diff --git a/.changeset/member-default-personal-inbox-read.md b/.changeset/member-default-personal-inbox-read.md new file mode 100644 index 0000000000..b0e9cfbd44 --- /dev/null +++ b/.changeset/member-default-personal-inbox-read.md @@ -0,0 +1,50 @@ +--- +"@objectstack/plugin-security": patch +--- + +fix(security): `member_default` grants owner-scoped READ on the personal inbox (#7344) + +The Account app's **Inbox → Notifications** entry was a dead end for every +non-admin. The app declares no `requiredPermissions`, so it is reachable by +design for every authenticated user, but the object behind that entry was named +by no shipped permission set — so the read came back `403 PERMISSION_DENIED`, +verbatim from the browser-measured run: + +``` +[Security] Access denied: operation 'find' on object 'sys_inbox_message' +is not permitted for positions [org_member, contributor, finance, everyone] +``` + +Two consequences were measured: the Notifications entry never rendered anything +for the audience the Account app exists for, and the console bell's notification +half was structurally **0** for any non-admin (a badge reading `2` was +`0 notifications + 2 pending approvals`). + +`member_default` now NAMES both halves of the personal inbox, read-only: + +| object | read | create | edit | delete | row scoping | +|:---|:---:|:---:|:---:|:---:|:---| +| `sys_inbox_message` | ✅ | ❌ | ❌ | ❌ | `sys_inbox_message_self` — `user_id == current_user.id` | +| `sys_notification_receipt` | ✅ | ❌ | ❌ | ❌ | `sys_notification_receipt_self` — `user_id == current_user.id` | + +`sys_notification_receipt` is not an extra: read-state lives on the receipt, not +on the inbox row (ADR-0030), so the entry needs both to render. + +**This is not a rollback of #5491.** The baseline stays explicit-allow — no +wildcard returns, and these are two NAMED objects in exactly the shape +`sys_user_preference` already uses there: an object grant plus a `_self` RLS +carve-out. Neither object declares `organization_id`, so Layer 0 is inert on +them (as it is on `sys_oauth_application`) and the `_self` policies ARE the row +scoping — without them the read bit would have been org-wide. A member reads +their own rows and only their own; an unidentified caller fails closed to +`RLS_DENY_FILTER` rather than open. + +The grants are READ-only because nothing in the flow needs more: rows are +written by the always-on `inbox` messaging channel keyed on the recipient, and +mark-read is served by `POST /api/v1/notifications/read` rather than the generic +data API. `allowDelete`/`allowExport` stay false, so the set remains bindable to +the `everyone` anchor (ADR-0090 D5). + +`sys_activity` is deliberately **not** included, per the maintainer ruling — it +is not a per-user-scoped shape, and it is a separate question if it ever +matters. It is pinned as an explicit negative in the tests. diff --git a/packages/plugins/plugin-security/src/member-default-explicit-allow.test.ts b/packages/plugins/plugin-security/src/member-default-explicit-allow.test.ts index 9100a4f371..199ae32850 100644 --- a/packages/plugins/plugin-security/src/member-default-explicit-allow.test.ts +++ b/packages/plugins/plugin-security/src/member-default-explicit-allow.test.ts @@ -27,6 +27,7 @@ import { describe, it, expect } from 'vitest'; import { PermissionSetSchema } from '@objectstack/spec/security'; import type { PermissionSet } from '@objectstack/spec/security'; import { PermissionEvaluator } from './permission-evaluator.js'; +import { RLSCompiler, RLS_DENY_FILTER } from './rls-compiler.js'; import { defaultPermissionSets, BETTER_AUTH_MANAGED_OBJECTS } from './objects/default-permission-sets.js'; const evaluator = new PermissionEvaluator(); @@ -151,6 +152,85 @@ describe('[#5491] what the baseline still declares, it still enforces', () => { }); }); +// [#7344] Maintainer ruling (2026-08-11): extend `member_default` with +// owner-scoped READ grants for `sys_inbox_message` and +// `sys_notification_receipt`, RLS-scoped to the caller. `sys_activity` is +// deliberately NOT included — it is not a per-user-scoped shape. +// +// The measured defect: the Account app declares a Notifications nav entry with +// `requiresObject: 'sys_inbox_message'` and no `requiredPermissions`, so every +// authenticated member can reach the app but no shipped set named the object — +// `[Security] Access denied: operation 'find' on object 'sys_inbox_message'`. +// +// This mirrors the `sys_user_preference` precedent above: an explicit object +// grant PLUS a `_self` RLS policy, because the grant alone would be org-wide. +// Both halves are asserted here — the read bit is worthless if the scoping is +// missing, and the scoping is what makes the grant safe to ship on the +// `everyone` anchor. +const INBOX_OBJECTS = ['sys_inbox_message', 'sys_notification_receipt'] as const; + +const MEMBER = { userId: 'u_member', tenantId: 'org_1', positions: ['org_member'] } as any; +const rls = new RLSCompiler(); + +/** The policies `member_default` contributes for one object × operation. */ +const policiesFor = (object: string, operation: string) => + (MEMBER_DEFAULT.rowLevelSecurity ?? []).filter( + (p: any) => (p.object === object || p.object === '*') && (p.operation === operation || p.operation === 'all'), + ); + +describe('[#7344] the personal inbox is readable by a member, scoped to their own rows', () => { + it.each(INBOX_OBJECTS)('%s: the baseline NAMES it, so a member with no app profile can read', (object) => { + expect(allows('find', [MEMBER_DEFAULT], object)).toBe(true); + }); + + it.each(INBOX_OBJECTS)('%s: the grant is READ-ONLY — the writer is the inbox channel, not the member', (object) => { + expect(allows('insert', [MEMBER_DEFAULT], object), `${object} insert`).toBe(false); + expect(allows('update', [MEMBER_DEFAULT], object), `${object} update`).toBe(false); + expect(allows('delete', [MEMBER_DEFAULT], object), `${object} delete`).toBe(false); + }); + + it.each(INBOX_OBJECTS)('%s: a read is RLS-narrowed to the caller — another user\'s rows are unreachable', (object) => { + const policies = policiesFor(object, 'select'); + expect(policies.map((p: any) => p.name)).toEqual([`${object}_self`]); + const filter = rls.compileFilter(policies as any, MEMBER); + // The scoping is a positive `user_id` equality, not a fail-closed sentinel: + // the member sees their own rows and ONLY their own. A row belonging to + // another user cannot satisfy this filter, which is the "cannot read another + // user's rows" half of the ruling. + expect(filter).not.toBeNull(); + expect(filter).not.toEqual(RLS_DENY_FILTER); + expect(filter).toEqual({ user_id: 'u_member' }); + }); + + it.each(INBOX_OBJECTS)('%s: an anonymous/unidentified caller fails CLOSED, not open', (object) => { + // No `current_user.id` to compile against ⇒ the sentinel, i.e. zero rows. + expect(rls.compileFilter(policiesFor(object, 'select') as any, {} as any)).toEqual(RLS_DENY_FILTER); + }); + + it('`sys_activity` is deliberately NOT granted — the ruling excludes it', () => { + // Named as an explicit negative so a future "while we are here" sweep has to + // argue with the ruling rather than quietly widen past it. It is not a + // per-user-scoped shape; a separate question if it ever matters. + for (const { operation } of AXES) { + expect(allows(operation, [MEMBER_DEFAULT], 'sys_activity'), `sys_activity ${operation}`).toBe(false); + } + const names = (MEMBER_DEFAULT.rowLevelSecurity ?? []).map((p: any) => p.name); + expect(names).not.toContain('sys_activity_self'); + }); + + it('the additions stay anchor-safe and explicit (no wildcard crept in with them)', () => { + for (const object of INBOX_OBJECTS) { + const perm = (MEMBER_DEFAULT.objects as any)[object]; + expect(perm, `${object} is named`).toBeTruthy(); + expect(perm.allowDelete ?? false).toBe(false); + expect(perm.allowExport ?? false).toBe(false); + expect(perm.viewAllRecords ?? false).toBe(false); + expect(perm.modifyAllRecords ?? false).toBe(false); + } + expect(Object.keys(MEMBER_DEFAULT.objects ?? {})).not.toContain('*'); + }); +}); + describe('[#5491] the admin sets keep their wildcards (this is a BASELINE change only)', () => { it.each(['admin_full_access', 'organization_admin', 'viewer_readonly'])( '%s still carries a `*` entry', diff --git a/packages/plugins/plugin-security/src/objects/default-permission-sets.ts b/packages/plugins/plugin-security/src/objects/default-permission-sets.ts index 9260a9a154..1c9dfdb329 100644 --- a/packages/plugins/plugin-security/src/objects/default-permission-sets.ts +++ b/packages/plugins/plugin-security/src/objects/default-permission-sets.ts @@ -361,6 +361,30 @@ const baseDefaultPermissionSets: PermissionSet[] = [ // implicit; making it explicit is the migration, not a widening — the // effective access for a member is byte-identical. sys_user_preference: { allowRead: true, allowCreate: true, allowEdit: true, allowDelete: false }, + // [#7344] The personal inbox, READ-ONLY. Same reasoning as the line above, + // applied to the other pair of platform objects a platform app points every + // authenticated member at: the Account app's `Inbox` group declares a + // Notifications entry with `requiresObject: 'sys_inbox_message'` + // (`packages/platform-objects/src/apps/account.app.ts`) and declares no + // `requiredPermissions`, so the app is reachable by design while the object + // behind the entry was named by no shipped set — every non-admin got + // `403 PERMISSION_DENIED` and the bell's notification half was structurally + // zero. `sys_notification_receipt` rides along because read-state lives + // there (ADR-0030), not on the inbox row, so the entry needs both. + // + // Maintainer ruling (2026-08-11): READ grants only. Rows are produced by + // the always-on `inbox` messaging channel keyed on the recipient + // (`service-messaging/src/inbox-channel.ts`) under the service's own engine + // access, and mark-read is served by `/api/v1/notifications` rather than the + // generic data API — so no create/edit bit is needed by the flow, and + // `allowDelete` stays false like everything else in this anchor-bound set. + // `sys_activity` is deliberately NOT included: it is not a per-user-scoped + // shape (no `user_id` to scope by), and it is a separate question if it ever + // matters. Two NAMED additions in #5491's explicit-allow shape — the + // `_self` policies below scope both to the caller — not a widening pattern + // and not a step back toward a wildcard. + sys_inbox_message: { allowRead: true, allowCreate: false, allowEdit: false, allowDelete: false }, + sys_notification_receipt: { allowRead: true, allowCreate: false, allowEdit: false, allowDelete: false }, }, rowLevelSecurity: [ // [ADR-0095 D1] The wildcard `tenant_isolation` policy RETIRED here — the @@ -508,6 +532,25 @@ const baseDefaultPermissionSets: PermissionSet[] = [ operation: 'all', using: 'user_id == current_user.id', }, + // [#7344] The personal inbox (Account → Inbox → Notifications, and the + // console bell). Neither object declares `organization_id`, so Layer 0 is + // inert on them exactly as it is on `sys_oauth_application` above and these + // `_self` policies ARE their row scoping — without them the read bit added + // to `objects` would be org-wide, which is the one outcome the ruling's + // "RLS-scoped to the caller" forbids. `select` (not `all`) because the + // grants are read-only; the writer is the inbox channel, not the member. + { + name: 'sys_inbox_message_self', + object: 'sys_inbox_message', + operation: 'select', + using: 'user_id == current_user.id', + }, + { + name: 'sys_notification_receipt_self', + object: 'sys_notification_receipt', + operation: 'select', + using: 'user_id == current_user.id', + }, ], }), PermissionSetSchema.parse({ diff --git a/packages/plugins/plugin-security/src/objects/rbac-objects.test.ts b/packages/plugins/plugin-security/src/objects/rbac-objects.test.ts index c3f3a000e7..dc6f6726bf 100644 --- a/packages/plugins/plugin-security/src/objects/rbac-objects.test.ts +++ b/packages/plugins/plugin-security/src/objects/rbac-objects.test.ts @@ -102,6 +102,11 @@ describe('default permission sets', () => { 'sys_account_self', 'sys_api_key_self', 'sys_device_code_self', + // [#7344] The personal-inbox pair — not better-auth tables, but the same + // `_self` shape for the same reason: no `organization_id`, so Layer 0 is + // inert and these policies are the row scoping for their read grants. + 'sys_inbox_message_self', + 'sys_notification_receipt_self', 'sys_oauth_access_token_self', 'sys_oauth_application_self', 'sys_oauth_consent_self',