Skip to content

fix(plugin-audit): ask enable.files/enable.feeds on the update verb too - #10332

Merged
os-warren merged 3 commits into
mainfrom
claude/issue-10170-capability-gate-update-verb
Aug 21, 2026
Merged

fix(plugin-audit): ask enable.files/enable.feeds on the update verb too#10332
os-warren merged 3 commits into
mainfrom
claude/issue-10170-capability-gate-update-verb

Conversation

@os-warren

@os-warrenos-warren commented Aug 20, 2026

Copy link
Copy Markdown
Collaborator

Fixes#10170

⚠️ Clause-② — this PR narrows what the engine accepts. Do NOT auto-merge.

Accept → reject flip: an update of sys_attachment / sys_comment that re-points the row onto a capability-disabled target object goes from accepted today to 403. This PR was developed at claude-opus-5 under the maintainer's 2026-08-20 authorization (Clause-② cards may be developed one tier below the gate while fable is exhausted), and the compensating control is that triage reviews before merge. The PR is draft; auto-merge is not armed and must not be armed until triage has reviewed.

The measurement establishing the flip is below — it is a behavioural reproduction on this branch, not an assertion.

Premise, re-established behaviourally before anything changed

New real-engine harness (capability-gate-update-verb.test.ts) run against unmodifiedaudit-writers.ts. The file carries a control whose only job is to prove the harness is wired before any conclusion is read from it — the control passed, so enable really round-trips through the registry and both gates are live:

control (insert verb, identical shape) → rejects FILES_DISABLED / FEEDS_DISABLED, 403 ✓ passed
control (insert verb, enabled parent) → accepted ✓ passed

With the harness proven, the update verb on the same engine:

by-id re-point onto a files-disabled parent
AssertionError: promise resolved "{ parent_object: 'lead_walled', …(3) }" instead of rejecting
Received: { file_id: 'file-1', id: 'r_1', parent_id: 'rec-1', parent_object: 'lead_walled' }
predicate (multi) re-point onto a files-disabled parent
AssertionError: promise resolved "2" instead of rejecting ← two rows re-pointed in one call
by-id re-thread into a feeds-disabled object
Received: { body: 'hello', id: 'r_1', thread_id: 'lead_walled:rec-1' }
predicate re-thread into a feeds-disabled object
AssertionError: promise resolved "2" instead of rejecting

Tests 4 failed | 7 passed (11) — exactly the four update-verb re-points, on both dispatch shapes, for both capabilities. The row landed on the walled parent; this is not a missing error, it is a completed write.

An earlier run of this harness reported the predicate legs as resolving 0. That was a stub defect, not a measurement: driver.updateMany(object, AST, data, options) takes the compiled AST as its second argument, and stubbing it as (o, where, data) matches nothing and resolves 0 — which reads exactly like "accepted, touched no row". Fixed in the harness (with a comment recording it) and re-measured before any of the above was believed.

Verifying triage's reading of the declaration — it holds, with one nuance worth seeing

Triage read enable.files / enable.feeds as properties of the target object. Checked against packages/spec/src/data/object.zod.ts rather than inherited:

  • files (:269-281) — "permits sys_attachment rows to target this object"; the flag lives in the target object'senable block, not on the attachment.
  • feeds (:283-290) — "rejects new sys_comment rows targeting this object".

The reading holds. The nuance: both describe strings then narrow to the verb — "otherwise creation is rejected", "rejects new attachments". So the declaration says the state property in its contract sentence and the verb in its short description. That tension is precisely the card: the contract sentence is the one the block-level docs and the gate's own doc comment state, and it is a property of the object. Nothing in the spec says a row may become a target of a walled object as long as no creation happened.

What changed

Both existing enforcement functions are registered on beforeUpdate in addition to beforeInsert, scoped to one object each. No new error codeFILES_DISABLED / FEEDS_DISABLED are existing standard-catalog members already in error-code-ledger.zod.ts and already mapped to 403 in rest/src/error-response.ts; packages/spec is untouched.

Both functions already read only ctx.input.data, so:

dispatchUnscopedMultiWrite is deliberately NOT declared (reasoning in the source, pinned in a test). That flag serves guards refusing an operation shape; these read the payload, which the per-row fan-out delivers to every matched row — so an unscoped multi: true re-point is already refused on its first matched row. The only case the flag would add is a zero-match unscoped write, where nothing is written and nothing ever comes to target the walled object; refusing that is #4757/#9974's operation-shape policy, not this capability opt-in.

A landed pin had to be re-pointed — please look at this

audit-hook-object-scope.test.ts asserted "plugin-audit declares NO beforeUpdate/beforeDelete hook (#6656)" and this change turns it red. Triaged rather than edited to green:

The property #6656 pins is the retirement of captureBefore — an unscoped pre-image reader that made hasHooksFor answer true for EVERY object on beforeUpdate and bought a prior-row read on every update in the stack. The assertion expressed that by filtering on event name, and the case's own comment already recorded that as a proxy: it carved out the capability gates as "unrelated … on a single named object each" that "read no prior row", noting an assertion catching them "would fail on them while measuring nothing about this card". That carve-out is true of the new registrations too — the event-name filter just could not express it once the gates stopped being insert-only.

So the filter now selects global registrations (options.object === undefined), which is the invariant, and a second case adds the affirmative half: each gate is declared on beforeInsertandbeforeUpdate, each scoped to one object.

Cost check, measured not assumed: these registrations add zero new prior-row reads on any object. comment-access-hooks.ts (#4630, :447) and service-storage's attachment-access-hooks.ts (#10091, :347) already declare beforeUpdate scoped to sys_comment / sys_attachment, so hasHooksFor is already true for both wherever the access kits install.

Who is affected — measured, not asserted

A structural sweep of the 4 660 in-tree source files (git ls-files on ts/tsx/js/mjs, excluding dist/), each search validated against a known-present control term first:

PopulationResult
Callers issuing an update whose payload names parent_objectnone outside tests — every hit is an object definition, a generated translation, a hook reader, or a where clause
Callers re-pointing thread_id on an updatenone
Console (objectui @ 12841b6)only sys_attachment write is a create; the only sys_comment update writes reactions only
Dogfood suiteno update of either object anywhere — every hit is a create payload, a where clause, or a fixture
Objects declaring enable.files: true in-tree2 shipped app objects (showcase project, todo task) + dogfood fixtures
Objects declaring enable.feeds: false in-tree0 shipped app objects (test/dogfood fixtures only)

No existing row is newly refused. The narrowing is keyed on the payload, not on row state: a row already sitting on a walled parent is untouched, and renames/body edits/reaction writes on it keep working. Only a write that makes a row newly target a walled object is refused. This is pinned as its own test case, in both dispatch shapes, precisely so the narrowing cannot silently widen to grandfathered rows.

Tests

Real ObjectQL with a stub driver, not the hand-rolled fake in audit-writers.test.ts — the fake's registerHook ignores the { object } option, so a registration change is unobservable there, and only the real engine fans a predicate write out per row.

Two-directional on both dispatch shapes, every rejection asserting the ADR-0112 envelope (code AND status):

by-id (dispatch.moderecord)predicate (per-row)
files, walled parent403 FILES_DISABLED + row unchanged403 FILES_DISABLED + both rows unchanged
files, enabled parentsucceedssucceeds for every matched row
feeds, walled object403 FEEDS_DISABLED + row unchanged403 FEEDS_DISABLED
feeds, enabled objectsucceedssucceeds

Plus: unscoped multi: true refused on its first matched row; non-re-point updates unaffected; free-form/absent thread_id still allowed.

pnpm --filter @objectstack/plugin-audit test Test Files 19 passed (19) · Tests 313 passed (313)
pnpm --filter @objectstack/plugin-audit typecheck MARK_TYPECHECK_EXIT=0

Ablation — predicted first, then measured

Predicted before running: removing the two beforeUpdate registrations turns exactly 5 of 12 red — the four re-point cases plus the unscoped-multi pin — while the insert control and every "enabled parent still succeeds" case stays green.

Observed:Tests 5 failed | 7 passed (12), and the five were exactly the predicted five.

Rebuild statement, argued from the files. The ablation subject is audit-writers.ts, imported by the test as ./audit-writers.js — a relative specifier resolving to the sibling source file, so neutering it takes effect with no rebuild. The ObjectQL under it is a different matter: packages/plugins/plugin-audit/vitest.config.ts aliases only @objectstack/core, platform-objects/audit, spec* and types@objectstack/objectql matches none of those patterns (verified by evaluating the five regexes against the specifier), so the engine resolves through package exports to packages/objectql/dist/, which was built at the top of this run.

Restore proved byte-identical, both legs:

pristine git hash-object f7a8885d646b2fcc92eaa9ba3dbee08733c26c15 (== git rev-parse HEAD colon the same path)
ablated git hash-object 8e437f18cd7e54b698f895f07ad9a6d5010a2cc0 markers present at :1603, :1649
restored git hash-object f7a8885d646b2fcc92eaa9ba3dbee08733c26c15 markers: 0 occurrences
restored run → Tests 12 passed (12)

Gate union — derived, not recalled

node scripts/pm/dispatch-gates.mjs (no paths passed; the script derives the change set itself from the merge base) re-run after the final commit on a clean worktree, and the gate set was identical to the pre-commit derivation. Exit codes captured before any pipe; verdicts quoted from each gate's own line. Run at c226824d0:

check:changeset-gate-self-tests EXIT=0
check:cross-package-test-inputs EXIT=0 "OK: 12 package(s) read outside themselves, all declared"
check:objectui-changeset EXIT=0
check:slot-lookup EXIT=0
check:test-source-alias EXIT=0 "OK — 72 packages with tests scanned"
check:type-source-resolution EXIT=0
check:query-options-erasure EXIT=0 "ratchet holds … test surface: 240 site(s) — at the ceiling, none new"
check:engine-double-contract EXIT=0 "OK — 331 pinned, 133 in the DEBT ledger, 2 exempt"
check:where-matcher EXIT=0 "265 matcher(s) discovered, 265 answer the combinator battery correctly or refuse it"
check:i18n EXIT=0 "OK (9 package(s) — all bundles in sync)"
check:type-check-coverage EXIT=0 "OK — 64/77 workspace packages type-checked"
check:nul-bytes EXIT=0 "OK (scanned 6091 text file(s) … no raw ASCII control bytes)"

check:query-options-erasure first came back red — the new test file grew the test surface 240 → 246. Repaired the way the gate names (these inputs are on-contract, so they are typed with satisfies EngineQueryOptions / EngineUpdateOptions), not by raising the ceiling.

One gate declared unrun, with the reason:check:type-check-debt (the --re-measure ratchet half) refuses on this worktree — @objectstack/service-knowledge has no built type entry point, and the gate is explicit that measuring from here would silently measure a different world. It re-measures ledgered packages only, and @objectstack/plugin-audit is in neither DEBT nor TEST_DEBT (checked by parsing both ledger blocks out of scripts/check-type-check-coverage.mjs), so no number it records can move for a diff confined to this package. The structural half is green above and the package's own typecheck exits 0. CI runs it on the built closure regardless.


Generated by Claude Code

Both capability gates registered on beforeInsert only, so a re-point via
UPDATE landed rows on a parent object whose declaration refuses them: a
caller who could not create a sys_attachment on a files:false object could
move an existing one onto it, and a sys_comment could be re-threaded into a
feeds-disabled object's thread. The access kits authorize the re-point
(comment-access-hooks since #4630, attachment-access-hooks since #10091) —
those are access checks; the capability half was never asked.
enable.files/enable.feeds are properties of the TARGET object, so the verb
that made a row target it does not change the answer. Register the same two
enforcement functions on beforeUpdate. Both already read only input.data, so
an absent parent_object / thread_id means "not a re-point" and returns on
the first line — a rename or body edit on a grandfathered row is untouched.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01PnJHU45vPJj5UQrxe946Bx
…#6656 no-global-pre-image assertion
The #6656 case asserted "no beforeUpdate/beforeDelete registration" by
filtering on EVENT NAME. Its own comment recorded that as a proxy: the
property it pins is the retirement of captureBefore, an UNSCOPED pre-image
reader that made hasHooksFor true system-wide and bought a prior-row read on
every update in the stack. It carved the capability gates out by noting they
are "on a single named object each" and "read no prior row" — true of the new
beforeUpdate registrations too, which the event-name filter could not express.
Filter on GLOBAL registrations instead, and add the affirmative half: each
gate is declared on beforeInsert AND beforeUpdate, scoped to one object. The
demand gate costs nothing new on these two objects — comment-access-hooks
(#4630) and attachment-access-hooks (#10091) already declare beforeUpdate
scoped to sys_comment / sys_attachment.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01PnJHU45vPJj5UQrxe946Bx
…rasing them
check:query-options-erasure counts erased option bags in test code against a
shrink-only ceiling; the new file pushed the test surface 240 -> 246. These
inputs are all ON-contract (`where`, `multi`), so the gate's own remedy is to
type them, not to cast — the `as unknown as` escape names a contract being
bypassed deliberately, which is not what these do. Back to 240, at the
ceiling, none new.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01PnJHU45vPJj5UQrxe946Bx
@github-actions

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

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

18 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 75e9301372987d99b458d168d88d926bf09b9003.

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

What this run could not see
  • the SDK route bridge reached 45 of 221 client-bound route-ledger rows — the other 176 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run: node scripts/docs-audit/affected-docs.mjs --bridge-coverage

Coarse fallback — 7 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 75e9301372987d99b458d168d88d926bf09b9003packageMentionDocs.

Which tree this was computed on

This run read content/docs from 7096d599ebaa9ff74e6425bd85b5c3e800db92d9 — the merge of head c226824d043aca5cdc46c90a14cd5a792231ec24 into base 75e9301372987d99b458d168d88d926bf09b9003, 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 7096d599ebaa9ff74e6425bd85b5c3e800db92d9 && git checkout 7096d599ebaa9ff74e6425bd85b5c3e800db92d9
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 75e9301372987d99b458d168d88d926bf09b9003 c226824d043aca5cdc46c90a14cd5a792231ec24 && git checkout -B drift-repro 75e9301372987d99b458d168d88d926bf09b9003 && git merge --no-ff c226824d043aca5cdc46c90a14cd5a792231ec24
node scripts/docs-audit/affected-docs.mjs --json 75e9301372987d99b458d168d88d926bf09b9003

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

@github-actionsgithub-actionsBot added documentation Improvements or additions to documentation tests tooling labels Aug 20, 2026
@os-warren
os-warren marked this pull request as ready for review August 21, 2026 01:00
@os-warren
os-warren added this pull request to the merge queueAug 21, 2026
Merged via the queue into main with commit dd41df3Aug 21, 2026
28 checks passed
@os-warren
os-warren deleted the claude/issue-10170-capability-gate-update-verb branch August 21, 2026 01:55
os-elon pushed a commit that referenced this pull request Aug 21, 2026
…e.feeds describe strings (#10336)
Both `.describe()` strings said the flag rejects *creation*. Since #10170
(PR #10332) `plugin-audit` registers each capability gate on `beforeUpdate`
as well as `beforeInsert` (audit-writers.ts), so both refuse any write that
makes a row TARGET the walled object — a create and an update that re-points
a `sys_attachment` / re-threads a `sys_comment` alike, 403 FILES_DISABLED /
FEEDS_DISABLED. The strings now say that, which is what the docblocks
directly above them already said.
These strings are not inert prose: `.describe()` reaches the generated schema
output and the authoring surfaces built on it, so an author (or an LLM
authoring metadata) reading "otherwise creation is rejected" would conclude a
re-point is outside the flag's reach. It is not.
`enable.activities` is genuinely a mirror-on-write switch and is untouched.
Text only: no default, type, or key changes.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01B4h3medzvhB9rpfoja9jcw
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentationImprovements or additions to documentationsize/lteststooling

Projects

None yet

Development

Successfully merging this pull request may close these issues.

enforceFilesCapability / enforceFeedsCapability are beforeInsert-only — a re-point via update dodges the capability opt-in on the NEW parent object

2 participants

@os-warren@claude