Skip to content

fix(plugin-security): a controlled_by_parent detail's read scope requires object-level read on its master - #24985

Merged
objectstack-fleet[bot] merged 4 commits into
mainfrom
claude/issue-24962-cbp-master-object-read
Oct 20, 2026
Merged

objectstack-fleet[bot] merged 4 commits into
mainfrom
claude/issue-24962-cbp-master-object-read

Conversation

@objectstack-fleet

Copy link
Copy Markdown
Contributor

Fixes #24962
Clause-②: no

What this changes

ADR-0055 §3 says "Reading a detail requires read access to its master". §4 says a member who cannot read master M "can neither read nor by-id-write a detail D under M".

computeControlledByParentFilter builds a controlled_by_parent object's read scope from the master ids the principal may read. It scoped the master's rows: RLS, the sharing half, and the master's own derivation on a chain. It never asked the master's object-level read admission. Now it asks that first, and a principal the master's own read door refuses at the object gate gets the empty master set.

  • One predicate, two askers. Arms 3 to 6 of canReadObject move into a private objectReadAdmission(object, sets, delegatorSets), which works over sets the caller has already resolved. The arms are the posture, the ADR-0066 D3 capabilities, the allowRead grant, and the ADR-0090 D10 delegator's same questions. canReadObject resolves the context and asks it, as before. The derivation asks it of the master, for the principal whose sets it was handed. There is no second copy of the rule.
  • Every call site goes through it. These are the caller half, the D10 delegator half and getReadFilter (security-plugin.ts near :4390, :4408 and :5943 at f32d795ce5). The recursion asks the same question at every hop of a chain.
  • The write doors follow without a change of their own. A by-id update or delete asks the read door's question first: a row the caller cannot read answers the read door's not-found. A predicate write is scoped by the read scope.
  • No widening. The diff only adds an empty-set exit ahead of the existing scan. A principal the master object admits runs the same code as before.
  • Not touched: packages/objectql/** and packages/spec/**.

The two spellings now agree

The engine's ADR-0139 ReadableIds leaf asks canReadObject(parent) before it scopes a parent's rows (engine.ts, readableIdsScopeOf). That call now runs the same predicate the id-list derivation asks. Measured on the real stack with a probe read through the leaf, for a principal without object-level read on the master and for its control:

  • before this change: the leaf answered no master and the id list answered every master;
  • after it: both answer no master, and for the control both answer every master.

The probe was a one-off measurement and is not committed.

Verification (head 3dd33f7b78)

Pins. src/controlled-by-parent-master-object-read.test.ts runs on the real stack: ObjectQL over better-sqlite3, the real SecurityPlugin, and on-behalf-of contexts assembled by resolveAuthzContext and assembleExecutionContext. It has 23 cases, named by the rule:

  • for two principals the master's read door refuses, find, findOne and count read no detail, the getReadFilter scope admits none, and a detail two hops down is not read;
  • a detail whose master is itself controlled_by_parent is not read without object-level read on that master;
  • on an on-behalf-of read, the delegator's object-level read on the master is required too;
  • a by-id update or delete of such a detail answers RECORD_NOT_FOUND / 404 and writes nothing, an insert is refused, and a predicate update matches no detail;
  • the controls keep their answers: master read with rows scoped, master read with no master row in scope, master read unscoped, a super-user grant, a system context, and an on-behalf-of read whose delegator reads the master. So do the write controls: a by-id update, an insert and a predicate update by a principal that reads the master.

Package. At 438ef2dfde, pnpm --filter @objectstack/plugin-security exec vitest run --maxWorkers=2 gave 198 files passed, 4100 passed and 45 skipped, VERDICT command-exit 0. The one commit since then renames the new file's labels, and its 23 cases pass at 3dd33f7b78. pnpm --filter @objectstack/plugin-security typecheck gave VERDICT command-exit 0 at 3dd33f7b78. tsconfig.test.json includes src/**/*, so the new file is compiled.

Ablation. The fix was committed first. Each leg mutated security-plugin.ts through scripts/ablation-replace.mjs (anchor 1 to 0, blob changed) and ran the pin file. Each was then restored with git checkout HEAD -- PATH and proven by blob equal to HEAD (a774fe4e1ddf) and an empty git diff HEAD. The subject is imported from src/ by a relative path, so no dist/ leg applies.

  • A: the new guard removed. 12 failed, 11 passed. Red: every "without object-level read" read, scope, chain, on-behalf-of, by-id-write and predicate-write pin. Green: the two preconditions, the two insert pins and every control. The insert pin is refused by gates ahead of the read scope and stands either way, so it pins §4's third write rather than this change.
  • B: the guard skipped on recursive hops. 3 failed: the two two-hops-down pins and the on-behalf-of pin, whose chain read goes through the recursion.
  • C: the delegator half handed the caller's sets and context. 1 failed: the on-behalf-of pin. That pin measures the delegator half, not the caller half.

Gates. node scripts/pm/dispatch-gates.mjs --commands --repo objectstack-ai/objectstack derived 69 families at 3dd33f7b78. All 69 ran one at a time, each exit captured before any pipe. --ran reports "69 derived famil(ies) accounted for — 69 run, 0 NOT-MEASURED (a DERIVED zero — all 69 recorded an exit code and none of them is 3)".

  • An earlier run at 438ef2dfde had check:dual-build-cjs-loads and check:i18n exit 3, because the dist/ they read was missing. The trees were present at the second run, and both exited 0 there.

Acceptance notes

  • Error envelope. For some principals without object-level read on the master, a by-id update or delete of a detail now answers RECORD_NOT_FOUND / 404 where it answered PERMISSION_DENIED / 403 from the master-edit leg. The 404 is the write doors' existing rule for a row the caller cannot read, and that row is now unreadable. It is pinned.
  • plugin-security: a controlled_by_parent child's read loads every readable master id with no limit (twice per request), so its CPU grows with the master table and the read answers 500 "too many SQL variables" past ~32.8k masters #24932 works in this same function and is in the decision box. No ruling had landed on it at this head. This change does not move the id-list mechanism, so a later switch to the leaf keeps these rows by construction.
  • Administrators. A super-user grant is admitted to every master object, so it is unchanged, and that is pinned. The ADR-0090 D12 delegated-administration gate judges RBAC link-table writes only and is not on this path.
  • Base. origin/main was merged at b811c73696 (3 commits: service-datasource, docs, spec liveness data). This branch changes nothing under packages/spec. Main has since moved to ba69b168be (plugin-approvals labels, disjoint), and that is not merged.

Generated by Claude Code

…ires object-level read on its master

ADR-0055 §3 requires read access to the master to read a detail. The
master-derived read scope now asks the master's object-level read
admission before it scopes the master's rows, through one predicate
extracted from canReadObject (objectReadAdmission), so no second copy of
the rule exists. A principal the master's own read door refuses at the
object gate gets the empty master set at every door that composes the
scope: the caller half, the ADR-0090 D10 delegator half, getReadFilter and
every hop of a chain. The engine's ADR-0139 leaf already answered this way.

Pins on the real stack name the rule and its controls; a principal that
may read the master object keeps its previous answer.

Claude-Session: https://claude.ai/code/session_011TdZ8GjixNZFLz1xJD9zzK
Co-authored-by: Claude <noreply@anthropic.com>
…es; note the insert pin's gate

Claude-Session: https://claude.ai/code/session_011TdZ8GjixNZFLz1xJD9zzK
Co-authored-by: Claude <noreply@anthropic.com>
…ter door answers

Claude-Session: https://claude.ai/code/session_011TdZ8GjixNZFLz1xJD9zzK
Co-authored-by: Claude <noreply@anthropic.com>
@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-security, touching 4 documentable anchor(s).

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

  • content/docs/permissions/field-level-security.mdx (via SecurityPlugin (symbol, a top-level class))
  • content/docs/permissions/index.mdx (via SecurityPlugin (symbol, a top-level class))
  • content/docs/permissions/system-context.mdx (via canReadObject (symbol, a method of class SecurityPlugin))
  • content/docs/plugins/packages.mdx (via SecurityPlugin (symbol, a top-level class))
  • content/docs/ui/forms.mdx (via SecurityPlugin (symbol, a top-level class))

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

  • content/docs/releases/implementation-status.mdx (via SecurityPlugin (symbol, a top-level class))
  • content/docs/releases/v17/17-5.mdx (via canReadObject (symbol, a method of class SecurityPlugin))
  • content/docs/releases/v17/17-6.mdx (via canReadObject (symbol, a method of class SecurityPlugin))
  • content/docs/releases/v18/18-0.mdx (via SecurityPlugin (symbol, a top-level class))

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 — 16 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 0603bfa1cfb02a6e787f8ab2e389ce4752565ba9 → packageMentionDocs.

Which tree this was computed on

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

node scripts/docs-audit/affected-docs.mjs --json 0603bfa1cfb02a6e787f8ab2e389ce4752565ba9

⚠️ 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 0603bfa1cfb02a6e787f8ab2e389ce4752565ba9 → 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

席位核对 · ACCEPT · domain:services seat 1 · 2026-10-20T22:01Z

os-justin · session_011TdZ8GjixNZFLz1xJD9zzK (seat post #6021). Reviewed against GitHub and origin/main 78ab6474ca, not against the report (os-dev report 6302124402 on #24962).

Disclosure read first (#9387, 5323310017):

Head under review:

3dd33f7b781f32525e32af20c9b7d72ab4e343b7
  1. Shape.
  2. The fix, read in the code (security-plugin.ts).
    • Arms 3–6 of canReadObject move verbatim into a private objectReadAdmission(object, sets, delegatorSets): required capabilities, then checkObjectPermission('find', …, { isPrivate }), for the caller and the delegator. canReadObject calls it after resolving its context. There is ONE copy of the rule.
    • computeControlledByParentFilter asks it of the master before scoping the master's rows. An unadmitted principal gets { [fk]: { $in: [] } }, and an admission that cannot be resolved denies (fail-closed, logged at error).
    • The delegator half (~:4408) composes the same derivation with the delegator's own sets. getReadFilter composes it too, and the recursion applies the rule at every hop.
    • packages/objectql and packages/spec are untouched. There is no widening.
  3. Premises.
    • H1 holds: no object-level ask existed.
    • H2 holds: all three call sites now answer no detail for an unadmitted principal.
    • H3 holds: system, super-user and an admitted delegator are unchanged, and so is every principal the master admits, row-scoped or not.
    • §4: the by-id write path shared the gap, and closes through the read door (the not-found shape) with no write-gate change.
    • The engine's ADR-0139 leaf and the id-list derivation now agree, measured.
  4. Pins and ablation.
    • controlled-by-parent-master-object-read.test.ts: 23 cases on a real ObjectQL + SqlDriver + the real SecurityPlugin, including on-behalf-of contexts.
    • Ablation leg A (guard removed): 12 red, every unadmitted read, scope, chain, OBO, by-id and predicate-write pin.
    • Ablation leg B (guard skipped on recursive hops): 3 red.
    • Ablation leg C (the delegator call site given the caller's sets): 1 red.
    • Each restore was proven by blob equality.
  5. Changeset, @objectstack/plugin-security patch, read sentence by sentence: it states the rule (ADR-0055 §3/§4), the doors, the write-side consequence and what is unchanged, in abstract terms. Each matches 2–3.
  6. Classification. This brings the runtime to a rule ADR-0055 already declares, an implementation gap (spec 声明 > 实现), not a contract change. That is the treatment plugin-security: an on-behalf-of by-id write can admit a row the delegator's own by-id write refuses, where an app-authored row-level write policy narrows the delegator (ADR-0090 D10) #24746 and group posture: a member whose session has no active organization gets 500 ERR_SYSTEM_WRITE_ORGANIZATION_REQUIRED on a tenant-scoped create, where isolated answers 403 naming the missing organization #24784 took this term. The error envelope moves from 403 to the read door's 404 for an unreadable detail; that is the doors' existing rule, and it is pinned.
  7. Gates.
    • At ACCEPT, every check-run on this head is success or an expected skip, among them TypeScript Type Check, Lint & Repo Gates, Test Core 1–6/6, Build Core, Governed Surface Queue Guard and Check Documentation Links.
    • Locally, 69 of 69 derived families exited 0; --ran reads 0 NOT MEASURED.
    • plugin-security: 198 files, 4100 tests passed.
  8. Accepted deviations.
    • One container restart before any code; the run was resumed from a clean worktree.
    • One merge of main.
    • A throwaway measurement harness, never committed.
    • The gate runner ran under the verify lock, per the dispatch.
    • The pin labels were reworded to stay abstract.
    • Model-free trailers.
  9. Out-of-scope disposition. An unmeasured source reading on the detail write gate's master-edit leg is filed, redacted to location and governing text, as plugin-security: the controlled_by_parent detail write gate's master-edit check may not ask the same question as the master's own write door (reach not measured; detail withheld under the disclosure ruling) #24986 (measure reach first).
  10. Landing clause. Tier S: not governed, under the human-merge threshold. Next step: ready + auto-merge through the relay once every check on the current head is green or an expected skip. A push resets this reading.

Generated by Claude Code

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

2 participants