Uh oh!
There was an error while loading. Please reload this page.
fix(security): grant members owner-scoped read on the personal inbox (#7344) - #7586
Merged
Conversation
…7344) OS-DEV-REPORT ============= status: done branch: claude/issue-7344-member-inbox-grants base: origin/main @ 9051802 (fetched fresh; measured against origin/main, not a stale tree) issue: #7344 ruling executed: comment 5248466727 (2026-08-11), Option A premise_still_valid: YES — re-verified on origin/main @ 9051802. `member_default` (packages/plugins/plugin-security/src/objects/default-permission-sets.ts:314) named the better-auth identity tables plus `sys_user_preference` (:363) and nothing else, while packages/platform-objects/src/apps/account.app.ts:88-99 declares the Notifications nav entry with `requiresObject: 'sys_inbox_message'` and the app declares no `requiredPermissions`. The mismatch the card describes is exactly what the code showed. STEP 1 — PRODUCER MEASUREMENT (the gate): PRODUCER EXISTS ⇒ Option A --------------------------------------------------------------------- A producer writes member-visible inbox messages today. Approval notifications are the writer, as the ruling expected. Full chain, each hop cited: 1. INSERT PATH (the rows themselves) - packages/services/service-messaging/src/inbox-channel.ts:112 `const created = await data.insert(objectName, row)` where `objectName` defaults to `sys_inbox_message` (:13, :47) and `row.user_id = delivery.recipient` (:95, :99). The row is keyed by the RECIPIENT's own user id — precisely the rows a plain member would see under a `user_id == current_user.id` scope. - packages/services/service-messaging/src/inbox-channel.ts:64 `await data.insert(receiptObject, { ... user_id: r.userId, channel: 'inbox', state: 'delivered' ... })` → `sys_notification_receipt` (:16). This is why the grant needs both objects: read-state lives on the receipt, not the inbox row (ADR-0030, inbox-channel.ts:119-120). 2. THE CHANNEL IS ALWAYS-ON, NOT OPT-IN - packages/services/service-messaging/src/messaging-service-plugin.ts:98 `registerInbox: true` is the constructor DEFAULT. - packages/services/service-messaging/src/messaging-service-plugin.ts:126-127 `if (this.options.registerInbox) service.registerChannel(createInboxChannel({ getData }))` 3. THE DEFAULT CHANNEL IS `inbox`, SO ANY UNQUALIFIED emit() MATERIALIZES A ROW - packages/services/service-messaging/src/messaging-service.ts:620 `const channels = input.channels?.length ? input.channels : ['inbox'];` - The preference filter at :621 only drops (recipient × channel) pairs the user MUTED and is fail-open on error (:617-618) — an ordinary member with no preference row is not filtered out. 4. THE PRODUCER: THE APPROVALS PIPELINE - packages/plugins/plugin-approvals/src/approval-service.ts:609-640 — `notify()` calls `this.messaging.emit({ severity: 'info', ...input, payload, audience })` at :632 with NO `channels` key ⇒ falls to the `['inbox']` default above. - PRODUCTION wiring (not just tests): packages/plugins/plugin-approvals/src/approvals-plugin.ts:188-191 `const messaging = ctx.getService<ApprovalMessagingSurface>('messaging');` `if (messaging && typeof messaging.emit === 'function') this.service.attachMessaging(messaging);` 5. REACHABILITY FOR A MEMBER PERSONA (what user action triggers it) Member-as-submitter (the strongest case — a plain member with no approver role): a plain member submits a record that enters an approval flow, so the request's `submitter_id` is that member's user id. An approver then clicks "Request info" (or comments, or the request is returned): requestInfo() → approval-service.ts:2872-2875 `notify({ topic: 'approval.request_info', audience: [String(raw.submitter_id)] })` → emit() with no `channels` → default `['inbox']` (messaging-service.ts:620) → preference filter passes → inbox channel `send()` → INSERT sys_inbox_message { user_id: <the member's OWN id> } (inbox-channel.ts:112) + INSERT sys_notification_receipt { user_id, channel:'inbox', state:'delivered' } (:64) Same shape via `approval.returned` (:2353-2355, :2395-2397) and `approval.comment` (:2915-2918, audience = the other side of the thread). Member-as-approver: an approval routes to the member; the submitter clicks "Remind" → `approval.reminder`, `audience: [approver]` (:2702-2705). Also `approval.reassigned` → the new approver (:2643-2645), `approval.ooo_substituted` → the covering approver (:1622-1624), `approval.escalated` (:3273-3275), `approval.sla_breached` (:3292-3295). 6. THE emit()→ROW HOP IS COVERED BY AN EXECUTED TEST, not just read statically - packages/services/service-messaging/src/notification-schema-conformance.test.ts:142-158 registers the REAL `createInboxChannel` against an in-memory engine and calls `service.emit({ topic, audience, payload })` with NO `channels`, then reads the rows back through `listInbox`. That pins the default-channel → inbox-row materialization. ZERO-HIT DISCIPLINE: one grep did return a suspicious zero — `attachMessaging` appeared only in `*.test.ts` on the first pass, which would have argued "wired in tests only, no real producer". I falsified it before concluding anything: a count probe returned 11 total occurrences vs 3 non-test, which surfaced the production call site at approvals-plugin.ts:190. No absence in this report rests on an unfalsified zero-hit. ⇒ The feature is NOT empty. Option A implemented; NOT flipped to Option C. STEP 2 — WHAT CHANGED --------------------- 1. packages/plugins/plugin-security/src/objects/default-permission-sets.ts `member_default.objects` (after the `sys_user_preference` precedent row): sys_inbox_message: { allowRead: true, allowCreate: false, allowEdit: false, allowDelete: false } sys_notification_receipt: { allowRead: true, allowCreate: false, allowEdit: false, allowDelete: false } `member_default.rowLevelSecurity` (after `sys_oauth_application_self`): { 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' } Shape rationale, measured rather than assumed: - `_self` policies are REQUIRED, not decorative. Neither object declares an `organization_id` field (inbox-message.object.ts:50-105, notification-receipt.object.ts — grep for `organization_id` returns no field declaration on either), so the ADR-0095 D1 Layer 0 tenant wall is inert on them, exactly as it is on `sys_oauth_application` (default-permission-sets.ts:501-504). Without these policies the new read bit would have been org-wide — the one outcome the ruling's "RLS-scoped to the caller" forbids. - `operation: 'select'`, not `'all'`. The `sys_user_preference_self` precedent uses `'all'` because that grant is read+write; ours is read-only, so it follows the read-only precedent in the same file (`sys_oauth_access_token_self` :486, `sys_team_member_self` :456). - READ ONLY, no create/edit/delete. The ruling says "read grants", and the measurement agrees that nothing in the flow needs more: rows are written by the inbox channel (inbox-channel.ts:112/:64) through the messaging service's own engine handle, and mark-read is served by POST /api/v1/notifications/read → `inbox.markRead(userId, ids)` (packages/runtime/src/domains/notifications.ts:18-19, :198, :205), NOT the generic data API. So no edit bit on `sys_notification_receipt` is needed to make the inbox usable. `allowDelete`/`allowExport` stay false, keeping the set bindable to the `everyone` anchor (ADR-0090 D5 / #2753). - ⛔ `sys_activity` NOT added, per the ruling. Pinned as an explicit negative test. - #5491 posture respected: no `'*'` key, no sentinel, no glob — two NAMED objects. 2. packages/plugins/plugin-security/src/objects/rbac-objects.test.ts:99 The existing sorted exact-list `toEqual` pin of `member_default`'s RLS policy names had to learn the two new names, or it fails. Updated (+2 entries, in sort order). Flagging this because it is a deliberate pin, not incidental churn — it is the gate that makes any future silent addition to this set visible. 3. packages/plugins/plugin-security/src/member-default-explicit-allow.test.ts New `[#7344]` describe block, 10 cases, mirroring the `sys_user_preference` precedent tests in the same file: - member CAN read own sys_inbox_message / sys_notification_receipt (object bit) - the grants are read-only (insert/update/delete all denied) on both objects - CANNOT read another user's rows: the real `RLSCompiler` compiles member_default's select policies for each object against a member context and must yield exactly `{ user_id: 'u_member' }` — a positive equality, asserted to be neither `null` nor `RLS_DENY_FILTER`, and the policy set for the object is asserted to be exactly `[<object>_self]` - an unidentified caller fails CLOSED (`RLS_DENY_FILTER`), not open - sys_activity remains DENIED on all four axes and has no `_self` policy (the denial idiom the repo already uses, applied as the ruling's explicit negative) - the additions stay anchor-safe (no allowDelete/allowExport/viewAllRecords/ modifyAllRecords) and no wildcard crept in alongside them 4. .changeset/member-default-personal-inbox-read.md — patch for @objectstack/plugin-security. TESTS + TYPECHECK (real output) ------------------------------- Deps built first (`turbo run build --filter=@objectstack/plugin-security^...`, 17 tasks successful) — without it vitest fails on `Cannot find package '@objectstack/spec/security'`. $ cd packages/plugins/plugin-security && npx vitest run src/member-default-explicit-allow.test.ts src/objects/rbac-objects.test.ts Test Files 2 passed (2) Tests 53 passed (53) Duration 3.11s $ cd packages/plugins/plugin-security && npx vitest run # full package suite Test Files 45 passed (45) Tests 941 passed (941) Duration 19.09s $ cd packages/plugins/plugin-security && npx tsc --noEmit === TYPECHECK EXIT: 0 === # no output, clean Broader security/RLS surfaces that reference `member_default`, run to catch cross-package pins: $ cd packages/spec && npx vitest run src/security/explain.test.ts src/system/book.test.ts Test Files 2 passed (2) Tests 53 passed (53) $ cd packages/rest && npx vitest run src/security-routes.test.ts Test Files 1 passed (1) Tests 8 passed (8) NOT run: packages/qa/dogfood (`showcase-default-profile`, `showcase-d7-default-profile`, `rls-multitenant`, …). They boot a real stack and were out of reach for a sensible cycle time here. Read statically instead: they use `GET /data/sys_user_preference` as the probe for "is member_default the governing fallback" and assert 200 under it — adding two unrelated objects cannot change that verdict. Recording the gap rather than implying coverage I did not run. CHANGESET --------- Added: .changeset/member-default-personal-inbox-read.md — `"@objectstack/plugin-security": patch` (package name read from packages/plugins/plugin-security/package.json:2). This changes shipped authorization behavior, so it is a required changeset, not an optional one. It carries the before/after grant table, the verbatim 403 the card measured, and the explicit statement that this is not a #5491 rollback. content/docs/releases/ NOT touched. OUT-OF-SCOPE FINDINGS --------------------- 1. `sys_activity` still 403s for a member — correct per the ruling, and it leaves NO nav dead end: `grep sys_activity packages/platform-objects/src/apps/account.app.ts` returns nothing, so the `GET /api/v1/data/sys_activity` in the card's evidence comes from a console component (objectui), not from an Account app nav entry. The ruling's "separate question if it ever matters" stands, and this PR does not create a new dangling entry by excluding it. 2. #7266 is NOT fixed by this and still matters. This removes the 403; the bell's "View all notifications" still routes a non-admin to `/apps/setup/sys_inbox_message?view=mine` (the wrong app). The card predicted exactly this compounding — after this PR a non-admin lands in the right permission state in the wrong app. 3. Mark-read is safe under read-only grants TODAY because it is served by /api/v1/notifications (runtime/src/domains/notifications.ts:198/:205), not the data API. If that ever moves to the generic data API, `sys_notification_receipt` will need an edit bit and a `check`-clause policy. Noted, deliberately not pre-granted — the ruling says read. 4. `sys_inbox_message` declares a `mine` list view already filtered to `{current_user_id}` (inbox-message.object.ts:36-47), which is what the nav entry points at. It now has a matching server-side grant + RLS, so the view is no longer a client-side filter over an object the caller cannot read at all. OPEN QUESTIONS -------------- None blocking. One for the PM to relay if the maintainer wants it on record: this session DID have read-only GitHub API access (issue body + all 5 comments were read directly, which is how the ruling text was verified verbatim rather than trusted from the dispatch brief). Write access was not attempted, per the E34 transport constraint. The E34 note may be worth narrowing from "no GitHub API tooling" to "no GitHub WRITE tooling" for future cloud dispatches. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017Prs32y86EcsRGzfdJJK1v
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
Contributor
📓 Docs Drift CheckThis PR changes 1 package(s): 12 hand-written doc(s) reference the affected code and may need an implementation-accuracy re-verification:
⛔ 1 release-owned page(s) also reference the affected code. These are read-only:
|
os-help
marked this pull request as ready for review
August 11, 2026 07:13
Uh oh!
There was an error while loading. Please reload this page.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes#7344
Executes the 2026-08-11 maintainer ruling on #7344 (Option A):
member_defaultgains owner-scoped, read-only grants forsys_inbox_messageandsys_notification_receipt;sys_activitydeliberately NOT included and pinned as an explicit negative.The gate: producer measurement — PRODUCER EXISTS ⇒ Option A, not C
The ruling made the work conditional on measuring a real producer of member-visible inbox rows. The commit report carries the full six-hop chain, each hop cited: the inbox channel inserts rows keyed by the recipient's own
user_id(inbox-channel.ts:112, receipts at:64); the channel is on by default (messaging-service-plugin.ts:98,126); an unqualifiedemit()defaults to['inbox'](messaging-service.ts:620); the approvals pipeline is the producer (approval-service.ts:632, production wiring atapprovals-plugin.ts:188-191); member-persona reachability holds in both directions (submitter gets request-info/returned/comment notices; approver gets reminders/reassignments/escalations); and the emit→row hop is covered by an executed conformance test, not just a static read. One suspicious zero-hit (attachMessagingseemingly test-only) was falsified before use.The change
Two named read-only object grants + two
_selfselect RLS policies (user_id == current_user.id), placed on thesys_user_preference/ read-only precedent rows indefault-permission-sets.ts. Shape rationale measured, not assumed: neither object declaresorganization_id, so the Layer-0 tenant wall is inert on them and the_selfpolicies are load-bearing; read-only suffices because rows are written by the messaging service's own engine handle and mark-read is served byPOST /api/v1/notifications/read, not the generic data API. No'*', no glob — #5491's explicit-allow posture kept.Tests: the
rbac-objectssorted exact-list pin learns the two new policy names (+2, the deliberate gate); a new 10-case[#7344]block mirrors the precedent tests — own-rows readable, cross-user compiled to a positive{ user_id }equality (neither null nor deny), unidentified caller fails closed, read-only on all write axes,sys_activitydenied on all four axes, anchor-safety preserved.Verification (from the commit report)
plugin-security: 45 files / 941 tests green; targeted 2 files / 53 green;
tsc --noEmitclean. Cross-package pins: specexplain/book53 green, restsecurity-routes8 green. Recorded gap: the QA dogfood boot suites were read statically, not run — theirmember_defaultprobe (sys_user_preference200) is unaffected by two added objects.Changeset:
.changeset/member-default-personal-inbox-read.md,@objectstack/plugin-security: patch— shipped authorization behavior.content/docs/releases/untouched.Out-of-scope findings (in the commit report)
sys_activityexclusion leaves no dangling nav entry (the Account app never pointed at it); #7266's wrong-app bell routing still stands and now becomes the sole remaining defect in that flow; a future move of mark-read onto the generic data API would need an edit bit + check-clause policy (deliberately not pre-granted).Generated by Claude Code