Skip to content

fix(plugin-security): explain's read verdict on a controlled_by_parent record takes the read door's answer on the master leg - #22813

Merged
objectstack-fleet[bot] merged 3 commits into
mainfrom
claude/issue-22792-explain-read-parity
Oct 11, 2026
Merged

objectstack-fleet[bot] merged 3 commits into
mainfrom
claude/issue-22792-explain-read-parity

Conversation

@objectstack-fleet

Copy link
Copy Markdown
Contributor

Part of #22792
This PR delivers item 1 of the card; item 2 (ruling A on #22795) stays on the card, and #22792 remains open for item 2.
Clause-②: no

What this changes

Class: explain-versus-door parity, the read verb. The family's triage record on #22530 pinned every write verb; this widens that pin to read. Position: applyRecordAttribution's read branch in packages/plugins/plugin-security/src/explain-engine.ts.

For a controlled_by_parent record, the read door scopes the find by the record's master as well as by its own row-level security (ADR-0055). explain's read branch modelled the record's own row-level security only, so its record verdict could disagree with the read door. It now asks the read door's own by-id read for that leg, and its verdict equals the door's answer:

  • Asked when: a record-grained read (or export, which streams the same find) of a record that exists, on a controlled_by_parent object, past the capability and object gates, where every leg the report already models (the tenant wall, the record's business row-level security, the sharing read filter) admits the record.
  • Asked how: through the existing ExplainEngineDeps.recordAbsentToCaller, the caller-context by-id read the write doors already ask, with the EXPLAINED context. No second copy of the master derivation.
  • The answer: a record the door withholds is visible: false, decided by the sharing layer (the layer the write verbs' master check names), with that layer's record excluded. A record the door returns leaves the report byte-identical. A rejection is reported fail-closed (not_evaluated, no predicate), as every other dependency fault on the record path is.
  • Unchanged: every object that is not controlled_by_parent, every write verb (the read-absent path of the by-id writes is not touched), object-level reports, allowed, and the answer for an id no row carries. The verdict keeps a decider; whether it should take the nonexistent-id shape is item 2's round, not this PR.

Why the existing dependency and not a new one

The PM route pointed at the read door's master-aware read (the security plugin's master-derived read filter). Reaching it directly would add an optional key to the exported ExplainEngineDeps type. On this family's own precedent (the update half, which added checkControlledByParentWrite) that is a widening of a published type, Clause-②: yes (widening) with a minor changeset. The claim and this dispatch declare Clause-②: no with a patch. recordAbsentToCaller is the read door itself, so asking it is parity by construction, and no public type moves. Only its docblock changes, to say it is now asked for such a read too.

Pins

pin where what it holds
Family enumeration, REST (the pin the #22530 triage record named, widened in place) packages/qa/dogfood/test/cbp-explain-master-write.dogfood.test.ts The per-verb door table now gives read a by-id read door (GET) on the fixture's private-master detail object. Three cells, each beside that door: a record under a master the principal cannot read (door 404 RECORD_NOT_FOUND, explain visible: false, decided by sharing); CONTROL, a record under a master the principal reads (door 200, explain visible: true); CONTROL, an id no row carries (door 404 RECORD_NOT_FOUND, explain unchanged, no decider). The table stays total against ExplainOperationSchema.
Engine, per verb (the engine's verb classification, widened in place) packages/plugins/plugin-security/src/explain-controlled-by-parent-write.test.ts read and export are classified read_door_asked. For each: a withheld record is not visible on sharing; a returned one is byte-identical to the report without the question; a rejection is fail-closed with no predicate; asked once with the explained context, and the write check is not asked; not asked where the record's own RLS or the tenant wall already decides; not asked on a private or public_read_write object, nor for an object-level request or a missing record; create is not asked.

No new test file. The registered-service enumeration (controlled-by-parent-write-member.test.ts) is left as it was; see Acceptance notes.

Measurement and ablation

  • Before (a temporary, uncommitted plugin-level harness over a real ObjectQL, SQL driver and SharingService, on 179f7bf6c, which equals 680a86b4c for plugin-security): the read door withheld the record and explain's read verdict disagreed with it. PM assumption 1 holds. After (same harness, the changed source): the verdict equals the door for the subject and both controls.
  • Ablation (one leg, scripts/ablation-replace.mjs in wrap mode, anchor 1 to 0, blob changed, under an outer trap restoring to HEAD on EXIT, INT and TERM): the read master leg switched off behind a marker. Then pnpm --filter @objectstack/plugin-security build, then scripts/ablation-dist-preflight.mjs read the marker in 2 built files. Predicted: the engine's withheld, rejection and asked-once cells red for read and export, and the REST subject cell red, with every control green. Observed: engine 6 red, 67 green; REST 1 red (the subject cell), 17 green. Restore leg: blob equals HEAD, git diff HEAD empty, rebuilt, and the preflight with --absent read the marker absent from all 6 built files with the tree clean.

Local verification (head eb4274251, after merging origin/main 098481744)

  • pnpm --filter @objectstack/plugin-security exec vitest run: 199 files, 4053 passed, 45 skipped (run on 5ad8ac3f5, before the merge, which touched no file of this package). On eb4274251: the three explain and master-check files, 107 passed; the widened dogfood file, 18 passed.
  • pnpm --filter @objectstack/plugin-security typecheck: exit 0. pnpm --filter @objectstack/dogfood typecheck: exit 0, after a full workspace build.
  • node scripts/pm/dispatch-gates.mjs --commands: 70 derived commands, all run on eb4274251, all exit 0. Two first answered PREREQUISITE NOT MET (exit 3, not a measurement) and were re-run green after the full build. --ran: 70 derived, 70 run, 0 NOT-MEASURED.
  • Lint, narrowed to the three touched TypeScript files and proved. All three are in eslint's population (--print-config resolves each), the changeset is outside its files globs, and --format json counts 3 files with 0 errors and 0 warnings. eslint.config.mjs enables no type-aware linting, so this diff cannot move the verdict on any file it does not touch.
  • Not run locally, declared to CI: the repository-wide pnpm lint, the full dogfood suite (only the touched file ran), and the path-scheduled CI jobs.

Acceptance notes

  • Delegated reads. On an on-behalf-of read the door also composes the delegator's scope. The record path still does not model the delegator's row-level security on its own, so when the door withholds such a record the sharing layer names the master and says that the delegator's scope is composed in too. The visible verdict is the door's either way. Not filed: no measurement.
  • The registered-service enumeration (controlled-by-parent-write-member.test.ts, VERB_ROWS) still classifies read as no_by_id_write, which stays literally true. Its fixture has no private master, so a read-door row there needs a fixture change. The family's REST and engine enumerations carry read. Carrier: none.
  • The object-level readFilter on a read explanation is the rls layer's artifact (computeRlsFilter). The service's getReadFilter also ANDs the master-derived scope and the sharing filter. This is an observation from reading code, not measured at a door, and it is the object-level question, not this card's record verdict. Not filed. Carrier: none.
  • Wiring comment. The comment beside the recordAbsentToCaller wiring in security-plugin.ts still calls it the write path's read question. It is accurate, but it no longer covers every caller. Left alone to keep the diff inside the declared territory.

Generated by Claude Code

…t record takes the read door's answer on the master leg

A record-grained read (and export) of a controlled_by_parent record now
asks the read door's own by-id read, with the explained context, where
every leg the report models admits the record. A record the door
withholds is reported not visible on the sharing layer; a record it
returns leaves the report unchanged; a rejection is reported fail-closed.

Claude-Session: https://claude.ai/code/session_01CBAfsWMSfM3EToQGVStEcp
Co-authored-by: Claude <noreply@anthropic.com>
…d verb beside the by-id read door

The family's per-verb table classifies read against its by-id GET door on
a controlled_by_parent detail of a private master: a record under a master
the principal cannot read, a record under one it reads, and an id no row
carries, each beside that door's answer. Adds the patch changeset.

Claude-Session: https://claude.ai/code/session_01CBAfsWMSfM3EToQGVStEcp
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-security, touching 4 documentable anchor(s).

11 hand-written doc(s) NAME something this change touched and may need an implementation-accuracy re-verification:

  • content/docs/api/error-catalog.mdx (via controlled_by_parent (literal, a string literal in applyRecordAttribution))
  • content/docs/data-modeling/objects.mdx (via controlled_by_parent (literal, a string literal in applyRecordAttribution))
  • content/docs/getting-started/common-patterns.mdx (via controlled_by_parent (literal, a string literal in applyRecordAttribution))
  • content/docs/kernel/runtime-services/sharing-service.mdx (via controlled_by_parent (literal, a string literal in applyRecordAttribution))
  • content/docs/permissions/authorization.mdx (via controlled_by_parent (literal, a string literal in applyRecordAttribution))
  • content/docs/permissions/index.mdx (via controlled_by_parent (literal, a string literal in applyRecordAttribution))
  • content/docs/permissions/permissions-matrix.mdx (via controlled_by_parent (literal, a string literal in applyRecordAttribution))
  • content/docs/permissions/rls.mdx (via controlled_by_parent (literal, a string literal in applyRecordAttribution))
  • content/docs/permissions/sharing-rules.mdx (via controlled_by_parent (literal, a string literal in applyRecordAttribution))
  • content/docs/protocol/kernel/error-handling.mdx (via controlled_by_parent (literal, a string literal in applyRecordAttribution))
  • content/docs/protocol/objectql/security.mdx (via controlled_by_parent (literal, a string literal in applyRecordAttribution))

⛔ 6 release-owned page(s) also name something this change touched. These are read-only:

  • content/docs/releases/implementation-status.mdx (via controlled_by_parent (literal, a string literal in applyRecordAttribution))
  • content/docs/releases/v15.mdx (via controlled_by_parent (literal, a string literal in applyRecordAttribution))
  • content/docs/releases/v17/17-1.mdx (via controlled_by_parent (literal, a string literal in applyRecordAttribution))
  • content/docs/releases/v17/17-2.mdx (via controlled_by_parent (literal, a string literal in applyRecordAttribution))
  • content/docs/releases/v17/17-3.mdx (via controlled_by_parent (literal, a string literal in applyRecordAttribution))
  • content/docs/releases/v17/index.mdx (via controlled_by_parent (literal, a string literal in applyRecordAttribution))

content/docs/releases/ is RELEASE-OWNED (AGENTS.md "Documentation Guardrails"): release
notes are written centrally at release time, and a code PR that edits them is the exact PR
that guardrail exists to stop. They are still audited — read-only. If one of them is actually
wrong, file an issue or open a dedicated docs-only PR; do not edit it here.

What this run could not see
  • 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 — 15 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 14f4912390d7761a4b91a6e2ed1f1ca1fb95adfe → packageMentionDocs.

Which tree this was computed on

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

node scripts/docs-audit/affected-docs.mjs --json 14f4912390d7761a4b91a6e2ed1f1ca1fb95adfe

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

@github-actions github-actions Bot added documentation Improvements or additions to documentation tests tooling labels Oct 11, 2026
@objectstack-fleet
objectstack-fleet Bot marked this pull request as ready for review October 11, 2026 09:32
@objectstack-fleet
objectstack-fleet Bot enabled auto-merge October 11, 2026 09:32
@objectstack-fleet
objectstack-fleet Bot added this pull request to the merge queue Oct 11, 2026
Merged via the queue into main with commit c11b758 Oct 11, 2026
37 checks passed
@objectstack-fleet
objectstack-fleet Bot deleted the claude/issue-22792-explain-read-parity branch October 11, 2026 09:53
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/m tests tooling

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants