Skip to content

fix(plugin-audit): a member removes their own comment reaction through an own-row unreact door - #26345

Merged
objectstack-fleet[bot] merged 6 commits into
mainfrom
claude/project-thread-tlk5tx
Oct 26, 2026
Merged

objectstack-fleet[bot] merged 6 commits into
mainfrom
claude/project-thread-tlk5tx

Conversation

@objectstack-fleet

Copy link
Copy Markdown
Contributor

Requested by Leehom · project thread

Fixes #26161

This is the server half of ruling 6422861164 (letter A). The console half (the reaction chip calls the new route, and feedReactionRemove stops reading a delete grant) is a separate domain:ui card that the seat files with Blocked-by: #26161.

Clause-②: yes (widening)
Metadata-protocol: none

What changed

AuditPlugin mounts an own-row unreact door on the kernel's HTTP server:

Method and path DELETE /api/v1/comments/reactions/:id (:id is the sys_comment_reaction row id)
Body none read; any query parameter is refused
200 { success: true, data: { id, deleted: true } }
400 VALIDATION_ERROR a query parameter, or an empty id
401 UNAUTHENTICATED the request names nobody
403 held-sign-in code an expired password or MFA not yet enrolled (the auth policy's own code)
403 PERMISSION_DENIED the row is another member's reaction; it stays
404 RECORD_NOT_FOUND no such row, or a row the caller cannot read (same answer, so nothing leaks)
503 SERVICE_UNAVAILABLE the permission store cannot be read, or no data engine is registered

How it decides, in order:

  1. The caller is resolved by the platform's one request-to-context composition, resolveRequestExecutionContext, with the settings door's choices: fail-closed for anonymous, the sign-in gate resolved onto the context, API key and session both admitted, nothing localized, no bearer carried.
  2. Anonymous, held sign-in and query-parameter refusals come next.
  3. The row is read as the caller, so the object grant, the organization wall and the reaction read gate (a reaction is readable exactly when its comment is) all decide. A row the caller cannot read answers the same 404 RECORD_NOT_FOUND as an unknown id. That is the answer the data door gives both.
  4. The row's created_by must be the caller. This is the column the platform's own-record floor (owner_only_deletes) reads, stamped by the engine with the reactor. Otherwise the door answers 403 PERMISSION_DENIED.
  5. Only then is the row removed by id, under a system context with attributedUserId set to the caller and the caller's tenantId. Step 4 is the only way in to that elevated write. If the driver reports that nothing matched (a race), the door answers 404.

What does not move: no permission set gains delete, member_default (default-permission-sets.ts) is untouched, and security-anchor-high-privilege (ADR-0090 D5) is untouched. The generic DELETE /api/v1/data/sys_comment_reaction/:id answers exactly what it did before.

Files:

  • packages/plugins/plugin-audit/src/comment-reaction-routes.ts: the door, removeOwnCommentReaction, and platformCallerResolver.
  • packages/plugins/plugin-audit/src/audit-plugin.ts: start() mounts the door on http.server (falling back to the http-server alias). With no HTTP server it warns once and mounts nothing.
  • packages/plugins/plugin-audit/src/comment-reaction-route-ledger.ts plus its .conformance.test.ts: the package-local route ledger, using the print-door model. The row is a gap because @objectstack/client has no method yet.
  • packages/plugins/plugin-audit/src/objects/sys-comment-reaction.object.ts: the header's Delete section now names the door.
  • scripts/check-single-authz-resolver.mjs: lists the door as a delegator of resolveRequestExecutionContext, so a later re-inline of the resolver goes red.
  • content/docs/permissions/tenant-audit-census.mdx and docs/audits/2026-08-tenant-audit-write-call-sites.counts.md: the census regenerated for the door's one system-context write (243→244 sites).
  • scripts/check-route-envelope.mjs: declares the new *-routes.ts module with { responses: 0, ok: 0, err: 0 }. Every body goes through the shared sendOk / sendError / sendServerFault.
  • .changeset/26161-own-row-unreact-door.md: patch on @objectstack/plugin-audit.

Tests

  • packages/plugins/plugin-security/src/comment-reaction-unreact-door.integration.test.ts runs on the real kernel, the real SecurityPlugin with the shipped sets, and the real AuditPlugin (object, read gate and mounted door). Caller resolution is real too: the platform resolver runs over seeded sys_user / sys_member rows and an auth service whose session read names the user. The member is a plain organization member with no position. Their baseline is member_default composed with an app every-member set that grants read and create (never delete) on comments and reactions. Cases:

    • own row: 200, and the row is gone on read-back;
    • foreign row: 403 PERMISSION_DENIED, and the row stays;
    • a reaction on a comment the caller cannot read: 404 RECORD_NOT_FOUND, the same status and code as an unknown id, and the row stays;
    • unknown id: 404 RECORD_NOT_FOUND;
    • anonymous: 401; a query parameter: 400. In both cases the row stays;
    • the generic data-door delete of the member's own row still answers 403 PERMISSION_DENIED, via the engine delete under the caller plus mapDataError, the data door's own path. The row stays.

    The test lives in plugin-security because plugin-security already has a devDependency on plugin-audit. The reverse edge would be a workspace cycle.

  • packages/plugins/plugin-audit/src/comment-reaction-routes.test.ts: the door's shell before the engine is asked (permission store outage 503, nobody 401, held sign-in 403, no engine 503, engine fault 500 INTERNAL_ERROR), plus the AuditPlugin mount with and without an HTTP server.

  • packages/plugins/plugin-audit/src/comment-reaction-route-ledger.conformance.test.ts: checks ledger and registrar in both directions.

Red first, on base c4968860 with the door not mounted: 6 of 8 integration cases red, each with "no DELETE /api/v1/comments/reactions/:id is mounted". The fixture case and the generic-delete 403 case were green, which is the defect itself.

Ablation of the reactor check, on 0f28b47607 (if (row.created_by !== callerId) throw … deleted through scripts/ablation-replace.mjs): plugin-audit was rebuilt and ablation-dist-preflight.mjs --absent reported the marker absent from all 6 dist files. Then 1 failed and 7 passed, and the failure was the foreign-row case alone. Restore: HEAD blob hash matched, the rebuild had the marker present in dist/index.js and dist/index.mjs, the tree was clean, and the run was 8 of 8.

Package runs on 0f28b47607: pnpm --filter @objectstack/plugin-audit test passed 64 files and 1023 tests. pnpm --filter @objectstack/plugin-security test passed 229 files, with 4942 passing and 45 skipped. The --project repo suites of core, objectql, rest, runtime, spec and types all passed. typecheck exited 0 for both. eslint --no-inline-config on both packages exited 0.

Gates on 0f28b47607: all 122 commands dispatch-gates.mjs --commands derives for this diff exited 0, and --ran reconciled 122 derived, 122 run, 0 NOT-MEASURED, 0 UNRUN.

Acceptance notes

  • The route is a gap on the SDK: @objectstack/client has no method for it. The console reaches the server through ObjectStackClient (ui/packages/data-objectstack), so the console half will want a client method or a direct call. That half is the seat's domain:ui card.
  • The dogfood live-mount parity gate (route-ledger-live-mount-parity.dogfood.test.ts) boots the showcase without AuditPlugin, so this ledger is not in its union, and this route is neither probed nor flagged there. The package-local conformance test is this route's guard. Adding AuditPlugin to that boot is outside this card.
  • An impersonation session may use the door, as it may use the generic data door to add a reaction. The removal is attributed to the session's user.

Generated by Claude Code

…h an own-row unreact door

`DELETE /api/v1/comments/reactions/:id` removes the caller's own
sys_comment_reaction row and only that: the row is read as the caller (so an
unreadable reaction answers 404 like an unknown id), its reactor (`created_by`)
must be the caller (else 403 PERMISSION_DENIED, row kept), and only then is it
removed under a system context attributed to the caller. No object-level
delete grant is read or widened; the generic data-door delete is unchanged.

Claude-Session: https://claude.ai/code/session_01TZQBvhyKHoq1oKrgnVx35Y
Co-authored-by: Claude <noreply@anthropic.com>
…changeset

The route-envelope gate audits every `*-routes.ts` module; the unreact door
writes every body through the shared sendOk/sendError pair, so it is declared
with zero local response builders.

Claude-Session: https://claude.ai/code/session_01TZQBvhyKHoq1oKrgnVx35Y
Co-authored-by: Claude <noreply@anthropic.com>
…ant-audit census

The door's removal is one more write call site threading a decidably
elevated context; the census and the page's quoted totals move 243 -> 244.

Claude-Session: https://claude.ai/code/session_01TZQBvhyKHoq1oKrgnVx35Y
Co-authored-by: Claude <noreply@anthropic.com>
… instead of casting them

Claude-Session: https://claude.ai/code/session_01TZQBvhyKHoq1oKrgnVx35Y
Co-authored-by: Claude <noreply@anthropic.com>
…olveRequestExecutionContext

The door hand-composed resolveAuthzContext and assembleExecutionContext.
It now calls the platform's one request-to-context composition with the
settings door's choices (fail-closed, gate resolved, API key and session,
optional session API, nothing localized). check-single-authz-resolver
lists it as a delegator.

Claude-Session: https://claude.ai/code/session_01TZQBvhyKHoq1oKrgnVx35Y
Co-authored-by: Claude <noreply@anthropic.com>
@github-actions

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

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

34 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 a8e7393dda4837823a88e906dd96eac4a175e554.

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

What this run could not see
  • 1 anchor(s) matched too much of the corpus to be a work list: PERMISSION_DENIED (literal, 43 pages)
  • 7 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 a8e7393dda4837823a88e906dd96eac4a175e554 → packageMentionDocs.

Which tree this was computed on

This run read content/docs from 75f40ffd86e7dd30263affa9b5560d120406d689 — the merge of head 0f28b4760718fb93247c8b0095c83246e8196b38 into base a8e7393dda4837823a88e906dd96eac4a175e554, 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 75f40ffd86e7dd30263affa9b5560d120406d689 && git checkout 75f40ffd86e7dd30263affa9b5560d120406d689
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin a8e7393dda4837823a88e906dd96eac4a175e554 0f28b4760718fb93247c8b0095c83246e8196b38 && git checkout -B drift-repro a8e7393dda4837823a88e906dd96eac4a175e554 && git merge --no-ff 0f28b4760718fb93247c8b0095c83246e8196b38

node scripts/docs-audit/affected-docs.mjs --json a8e7393dda4837823a88e906dd96eac4a175e554

⚠️ 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 a8e7393dda4837823a88e906dd96eac4a175e554 → pass the list as
args.docs, on the commit named under Which tree this was computed on.

@objectstack-fleet

Copy link
Copy Markdown
Contributor Author

Contract review

Served-tier: TIER_DEFAULT
Head-sha: 0f28b4760718fb93247c8b0095c83246e8196b38
Local-runs: none

The review is owed by the Clause-②: yes (widening) line alone. The diff touches no published schema (packages/spec/**) and no governed path (0 of 12 paths), and it declares no narrowing. Read on the net diff origin/main...origin/pr-26345, card #26161 (body, triage 6419159811, brief 6420295908, ruling 6422861164), the os-dev-report 6428370606, and the head's check-runs.

① Derived judgments

  • Accept-set widening, the one new door. DELETE /api/v1/comments/reactions/:id (comment-reaction-routes.ts), mounted by AuditPlugin on the kernel's HTTP server. It removes exactly one sys_comment_reaction row, and only when the row's created_by is the caller. RIGHT, and it is the ruling's letter A: authorization is structural (reactor == caller), not a grant.
  • Read as the caller first. engine.findOne runs under the caller's own context, so every read gate decides. A row the caller cannot read answers 404 RECORD_NOT_FOUND, the same as a missing id, so the door leaks no existence. RIGHT.
  • Foreign row. A row whose created_by is not the caller is refused 403 PERMISSION_DENIED before any write, and it stays. The ablation of this check turns the foreign-row pin red. RIGHT, and "refused loudly" as the ruling asks. The reactor column is never edited (Comment reactions (ruling A amended on #22505): a reaction is the reactor's own sys_comment_reaction record, and sys_comment.reactions retires with no aggregate and no data migration #22566), so there is no check-then-write race on ownership.
  • The write. The delete runs under { isSystem: true, attributedUserId: caller, tenantId: caller's }, addressed by the same id the caller's read admitted. That elevation is reached only after both checks, and the door accepts no other input (any query parameter is refused 400, no body is read). The new site is declared to check-single-authz-resolver (DELEGATORS row) and check-route-envelope (MODULES row), and the tenant-audit census is regenerated. RIGHT.
  • Door preamble. The caller is resolved by resolveRequestExecutionContext with anonymous: 'fail-closed', and the anonymous deny, the auth gate's own 403 and the permission store's 503 are answered as every door answers them. RIGHT.
  • Grant model unchanged. The generic DELETE /api/v1/data/sys_comment_reaction/:id still answers 403 for that member (pinned). member_default, the security-anchor-high-privilege rule and every permission set are untouched. Not B, not C. RIGHT.
  • Public surface. @objectstack/plugin-audit gains the route and the module's exports (registerCommentReactionRoutes, removeOwnCommentReaction, platformCallerResolver, COMMENT_REACTION_REMOVE_ROUTE). The route ledger (comment-reaction-route-ledger.ts) records the client gap (no @objectstack/client method yet), and its conformance test pins it. RIGHT.

② Semver level

  • .changeset/26161-own-row-unreact-door.md bumps @objectstack/plugin-audit at patch. That is the one package whose shipped source moves. scripts/** and the docs census are not published.
  • Clause-②: yes (widening) and Metadata-protocol: none match on the PR body and the changeset. A new door is a widening; nothing an author writes moves, so none takes patch. Nothing is refused that ran before, so no BREAKING banner or ADR-0087 disposition is owed. Check Changeset concluded success.
  • Prose-face sentences checked against the diff: "A reaction on a comment the caller cannot read answers 404 RECORD_NOT_FOUND, the same answer as an id that names no row" (the findOne miss path); "Anyone else's reaction is refused with 403 PERMISSION_DENIED and stays" (notYourReactionError before the delete); "The generic data-door delete answers exactly what it answered before" (pinned in the plugin-security integration test).

③ Boundary flags

  1. File surface beyond plugin-audit/**. The plugin-security integration test, the two gate registry rows and the regenerated tenant-audit census are each declared in the report. They are the door's own pins and the declarations its new site owes. Accepted.
  2. The UI half is filed as domain:ui sub-card Record feed: the reaction chip removes the caller's own reaction through the unreact door, so a member with no delete grant can take a reaction back (ui half of #26161) #26346 (Blocked-by: #26161), carrying the final route shape from the report. Until it lands, the route is reachable but nothing in the console calls it, as the changeset says.
  3. Client method gap. @objectstack/client has no method for the route, and the route ledger records it as a gap. The UI card names adding one as in scope. Not blocking.
  4. Open questions: none reported. Security handling: no advisory, no new issue, and no repro outside the test files, as the maintainer's standing word asks.
  5. Check-runs: 38 succeeded and 10 were expected skips on this head, 0 failed.

Implemented-by: claude/project-thread-tlk5tx
Reviewed-by: session_01R1q2xdtYgU6FKrZ7LgjPCC

VERDICT: PASS


Generated by Claude Code

@objectstack-fleet

Copy link
Copy Markdown
Contributor Author

席位核对 · ACCEPT · domain:server seat 1 · os-project-manager · session_01R1q2xdtYgU6FKrZ7LgjPCC · 2026-10-26T22:19Z

Head 0f28b47607, report 6428370606 (on #26161), claim 6424642090. The contract review owed by Clause-②: yes (widening) is on record: PASS at TIER_DEFAULT on this head (the record just above).

item reading
Shape Draft, base main, Fixes #26161, patch changeset for @objectstack/plugin-audit, mergeable clean. Not governed (0 of 12 paths), 1,075 changed lines.
Fix Ruling 6422861164 letter A. DELETE /api/v1/comments/reactions/:id reads the row as the caller (unreadable or missing gives 404, so nothing leaks). It refuses a foreign row with 403 PERMISSION_DENIED, and only then deletes under a system context attributed to the caller and kept in their organization. The grant model, member_default and security-anchor-high-privilege are unchanged, and the generic data-door delete still answers 403 (pinned).
Evidence 38 check runs succeeded and 10 were expected skips on this head, 0 failed. Red first: the own-row case fails on main. The ablation of the reactor check turns the foreign-row pin red. Derived gates: 122 of 122 exit 0.
Carried The UI half is filed as domain:ui sub-card #26346 (Blocked-by: #26161), with the final route shape. The @objectstack/client method gap is recorded in the route ledger and named on #26346.
Landing 「按照 pm-dispatch 规则复核过的都授权」 (Leehom, 2026-10-25T11:35Z): ready and auto-merge through the relay. Every check on this head is green or an expected skip, and the review PASS is on record.

Generated by Claude Code

@objectstack-fleet
objectstack-fleet Bot marked this pull request as ready for review October 26, 2026 22:21
@objectstack-fleet
objectstack-fleet Bot enabled auto-merge October 26, 2026 22:21
@objectstack-fleet
objectstack-fleet Bot added this pull request to the merge queue Oct 26, 2026
Merged via the queue into main with commit 1a298dd Oct 26, 2026
49 checks passed
@objectstack-fleet
objectstack-fleet Bot deleted the claude/project-thread-tlk5tx branch October 26, 2026 22:40
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/xl tests tooling Fix lands in tooling, not a product package — enters pm:queue only with Unblocks: or a named surface

Projects

None yet

2 participants