Skip to content

fix(plugin-audit): @mention notifications reach only mentioned users who can read the record - #24794

Merged
os-zhuang merged 6 commits into
mainfrom
claude/issue-24785-mention-wall
Oct 20, 2026
Merged

os-zhuang merged 6 commits into
mainfrom
claude/issue-24785-mention-wall

Conversation

@objectstack-fleet

Copy link
Copy Markdown
Contributor

Fixes #24785
Clause-②: no — the change narrows who a notification reaches; it is a runtime security-boundary change, so it sits on the human floor and its PR stays draft until the maintainer approves

Summary

@mention notification recipients are now filtered by read access to the comment's record. A collab.mention notification carries the comment's preview and a deep link to the record, so it now reaches only a mentioned user who can read that record. The author is never notified.

  • New mention-recipient-gate.ts (plugin-audit). Each mentions entry resolves to a user id. An email-shaped entry is looked up to its user; an audience expansion (role: / team: / owner_of:) is not a mention. Each user is then judged in their own envelope with no new ACL code. The envelope comes from resolveUserAuthzGrants, the resolver every request uses. The organization claim is vetted by vetOrganizationClaim under the posture in force (effectiveTenancyPosture over the tenancy service). The read is asked of resolveReadableParentIds, the evaluator every parent-derived read gate uses. So the tenant wall, sharing, RLS and object CRUD decide, the same way they decide when the user opens the deep link.
  • Withheld recipients get no notification. One warn line records the user id (when there is one), the reason and the comment id. It never logs the comment text.
  • Fail closed. A thread naming no record, a missing record, or a grant-resolution failure withholds the notification. installAuditWriters takes the gate as a new admitMentionRecipients option, and AuditPlugin wires the real one. A host calling installAuditWriters without it emits no mention notifications and is told once at warn.
  • service-messaging is untouched. Its RecipientResolver serves every topic (approvals, notify nodes and so on), so the narrowing stays on the mention path. The writer hands messaging only already-admitted user ids.
  • The fix lives at the producer, not as a row filter on sys_notification. A recipient reads their own notification, so no inbox-side read rule could tell a leaked copy from a legitimate one.

Tests

All on head 2a048ca9 unless noted (the last commit is a comment-only edit to the gate's header; the test runs below are on 43bbd4e4, byte-identical code).

  • src/mention-recipient-gate.integration.test.ts (new, 10 cases). It runs a real kernel with ObjectQL, a real SQLite driver, AuditPlugin, the platform's sys_user / sys_member definitions and the real core resolver. Comments are inserted through the engine as their author, so the comment write gates and the afterInsert writer run. plugin-security's read layer is played by one middleware that composes the posture's tenant clause and an owner-only rule into the read's WHERE. Messaging is played by an emit() recorder.
    • isolated: a user of another organization gets nothing, in either spelling a mention takes. A fellow member who can read the record is notified, with source set to the record (the deep link) and the record's organization. An email of a member resolves to their id. The author gets nothing (by id or email). Audience expansions are no mention.
    • group: a non-member of the record's organization gets nothing; a member is notified.
    • single: a same-organization member that the record's own sharing excludes gets nothing; the same member is notified on a record they can read.
    • The withheld log line carries { reason, userId?, commentId } only.
  • audit-writers.test.ts (+3). With no gate composed, the writer emits no mention. With a gate that throws, it emits none. With a gate, it emits only the admitted ids, and the gate receives the expected input. The existing mention unit tests now pass a pass-through gate.
  • audit-hook-object-scope.test.ts: the registration-scope pin passes a pass-through gate.
  • pnpm --filter @objectstack/plugin-audit typecheck && pnpm --filter @objectstack/plugin-audit exec vitest run --maxWorkers=1 → Test Files 60 passed (60) · Tests 976 passed (976), exit 0. The new test file was re-run after its final typing edit: Tests 10 passed (10), typecheck exit 0.

Ablations (committed fix, scripts/ablation-replace.mjs, restored byte-identical, git status clean)

  1. Organization-claim vetting removed (const tenantId = claimedOrganization): 4 of the 10 cases go red (the isolated cross-organization pins and the log pin). Restored blob cd95ead9997d == HEAD.
  2. Writer bypasses the gate (userIds = mentioned): 8 of 10 go red. Restored blob e0ccbab62207 == HEAD.

End-to-end on a walled boot

This used a scratch copy of examples/app-showcase on OS_TENANCY_POSTURE=isolated with the organizations package, the real plugin-security and the real messaging inbox. One comment's mentions covered a user outside the record's organization, a fellow member, and the author.

  • With this fix (full build at 43bbd4e4): the outside user's inbox was unchanged (unreadCount 0 → 0). The fellow member got one collab.mention with actionUrl set to the record's page (unreadCount 0 → 1) and could read the record (200). The author got nothing.
  • Control leg with the gate bypassed (ablation 2, rebuilt into dist/, marker proven present, then restored and rebuilt with the marker proven absent): the outside-user assertion fails. That shows the probe discriminates.

Gates

node scripts/pm/dispatch-gates.mjs --commands derived 70 families; reconciliation: 70 derived, 70 run, 0 NOT-MEASURED, 0 UNRUN. All exited 0, with the notes below:

  • pnpm check:dts-closure and pnpm check:dual-build-cjs-loads first ran while the full build was still in progress (PREREQUISITE NOT MET). Both were re-run after the build: exit 0.
  • pnpm check:query-options-erasure went red on the first pass: 3 new untyped option sites in the new test. Those were typed, and the re-run gave exit 0 (test surface: 236 site(s), at the ceiling).
  • node scripts/check-plugin-teardown-shape.mjs --self-test exited 3: this shallow clone cannot reach its pinned positive-control commit. NOT MEASURED here; CI runs it.
  • cross-package-test-inputs, test-source-alias and nul-bytes were re-run on the final code: exit 0.

Acceptance notes

  • Same-organization member without read access. Such a member no longer receives the preview: the rule is "can read the record", applied in every posture.
  • Cross-organization platform operators. No special case is made. A mentioned operator is notified exactly when they can read the record in their own envelope.
  • role: / team: / owner_of: entries in mentions. These no longer fan out. The field is documented as "JSON array of the user ids @mentioned".
  • Not in this PR's surface. A dogfood case driving this over HTTP with the real plugin-security belongs to packages/qa/dogfood, which is outside this card's file surface. The walled-boot run above is the manual equivalent.

Generated by Claude Code

@github-actions github-actions Bot added size/l documentation Improvements or additions to documentation tests tooling labels Oct 20, 2026
@github-actions

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 1 package(s): @objectstack/plugin-audit, touching 30 documentable anchor(s).

17 hand-written doc(s) name something this change touched — list omitted above 15 rows. Re-derive on the tree named below: node scripts/docs-audit/affected-docs.mjs --json 46bb5e3e5405fad31ccba6fe571e98b9234aac78.

⛔ 5 release-owned page(s) also affected — read-only, see AGENTS.md Documentation Guardrails.

What this run could not see
  • 2 anchor(s) matched too much of the corpus to be a work list: organization_id (literal, 35 pages), sys_user (literal, 41 pages)
  • 5 name(s) were too generic to anchor anything (single lowercase words)
  • the SDK route bridge reached 54 of 206 client-bound route-ledger rows — the other 152 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run. Of those 152: 0 are remediable by widening that discovery convention (an in-repo file declares the path; the convention did not scan it); 55 are structural — on a ledger where NOT ONE row is declared in-repo, so no discovery change reaches them at any price; 97 are undecided (no in-repo declaration, on a ledger that has other in-repo registrars — absence and an unreadable spelling are not distinguishable here). The rows themselves: node scripts/docs-audit/affected-docs.mjs --bridge-coverage
  • a page that states a rule by its inputs shares no identifier with the emitter that implements the rule, so an emitter-only diff cannot list it — not on this run and not on any run. Measured on fix(driver-sql): emit varchar(maxLength) for a text field a declared index keys on #11430: content/docs/protocol/objectql/types.mdx documents the text-family column mapping by the ObjectQL type names it maps FROM (text / textarea / html) while the diff changed createColumn; it went unlisted, and it was the page that diff falsified, in four places. No shared token exists to detect this on, so a rule your change carries has to be re-read by hand in the pages that restate it.
  • a key NAME is not a key, so the hand re-read the line above prescribes can land on the wrong schema. The same spelling is authorable on one governed type and a [REMOVED] tombstone on another for each of active, aria, joins, objects, template, tools and version (censused on [finding] tools is a key on BOTH AgentSchema (tombstoned, dead) and SkillSchema (live, cloud-attested), so a name-based search attributes skill examples to the agent key — it produced a false stop-the-line alarm on PR #19059 #19093 over the liveness ledger's governed types, top-level keys); nothing in a search result distinguishes the two, so a grep hit on a LIVE example reads as evidence about the DEAD key. Measured on fix(spec): the agent.tools liveness row says dead — it claimed live on a key the schema tombstoned #19059: content/docs/ai/agents.mdx was reported as contradicting the agent.tools tombstone over its tools: example at :161, which is inside the defineSkill({ block opened at :155 — the page was already correct. Settle ownership by PARSING the value against both schemas, never by the name: that literal PASSES SkillSchema, and as an AgentSchema it FAILS at tools with the tombstone prescription. ⛔ These names are not the whole class — a key retired through a .strict() guidance map leaves no tombstone in the walked shape and none of them here (tool.category, live as AIToolDefinition.category).

Coarse fallback — 10 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): node scripts/docs-audit/affected-docs.mjs --json 46bb5e3e5405fad31ccba6fe571e98b9234aac78 → packageMentionDocs.

Which tree this was computed on

This run read content/docs from 65912646cecf67e118c03759b963b561064201bf — the merge of head 2a048ca9b7fcdee57f89f59ab966e91f3a362274 into base 46bb5e3e5405fad31ccba6fe571e98b9234aac78, which is what actions/checkout gives a pull_request run. Not the PR head.

A worktree cut from an older main holds a different content/docs, so re-deriving there can legitimately return a different list — that is a different tree, not a wrong row. To answer on the same tree:

# while this PR is open — GitHub drops the merge commit once it closes
git fetch origin 65912646cecf67e118c03759b963b561064201bf && git checkout 65912646cecf67e118c03759b963b561064201bf
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 46bb5e3e5405fad31ccba6fe571e98b9234aac78 2a048ca9b7fcdee57f89f59ab966e91f3a362274 && git checkout -B drift-repro 46bb5e3e5405fad31ccba6fe571e98b9234aac78 && git merge --no-ff 2a048ca9b7fcdee57f89f59ab966e91f3a362274

node scripts/docs-audit/affected-docs.mjs --json 46bb5e3e5405fad31ccba6fe571e98b9234aac78

⚠️ That checkout carried uncommitted changes, so the commit above does not fully identify what was read.

Advisory only, and a precision-first one (#9192): a page is listed because it names a
symbol, wire route or SDK method this diff touched — not because it mentions a changed
package. Each row says which anchor put it there, so a wrong row is reportable rather than
merely annoying. To re-verify, run the docs-accuracy-audit workflow scoped to these files:
node scripts/docs-audit/affected-docs.mjs 46bb5e3e5405fad31ccba6fe571e98b9234aac78 → pass the list as
args.docs, on the commit named under Which tree this was computed on.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation size/l tests tooling

Projects

None yet

3 participants