Skip to content

fix(plugin-security): checkAuthoredRowWrite answers the declaration, not the caller's read scope (#7281) - #7400

Merged
os-zhuang merged 2 commits into
mainfrom
claude/issue-7281-authored-row-write-probe-scope
Aug 10, 2026
Merged

fix(plugin-security): checkAuthoredRowWrite answers the declaration, not the caller's read scope (#7281)#7400
os-zhuang merged 2 commits into
mainfrom
claude/issue-7281-authored-row-write-probe-scope

Conversation

@os-zhuang

@os-zhuang os-zhuang commented Aug 10, 2026

Copy link
Copy Markdown
Contributor

Fixes #7281

Implements the maintainer's 2026-08-10 ruling (reading 2) in the order it directs: the test half first and standing on its own, then the fix.

Branch point: d53bd0ba9bbe603632e6e9f55c2f202f06a7d541. Anchors re-derived there (the triage anchors still hold; security-plugin.ts moved in #7346 without disturbing them):

anchor at d53bd0ba9
checkAuthoredRowWrite service registration security-plugin.ts:749 / :754
the implementation security-plugin.ts:2660
the defective read security-plugin.ts:2703-2705
the by-id write pre-image gate security-plugin.ts:1301-1306
the deferral consumption sharing-plugin.ts:933-939
the fake-engine assertion row-write-widener-composition.test.ts:521

1. The test half

row-write-widener-composition.test.ts asserted checkAuthoredRowWrite(...) === 'admit' and then that the write lands, on crm_opportunity — declared sharingModel: 'private', with a row owned and created by somebody else. Its makeEngine() fake implements find as a direct row filter and registers no middleware, so no nested re-read in that file is ever scoped by plugin-sharing's read filter. Both of those nested reads — the probe's, and the pre-image gate's — decide their verdict on exactly that scoping. The double was looser than the producer on the one axis the outcome turns on.

What I did with it, and why. The real-stack measurement (below) says a private cross-owner by-id write is refused, so the { ok: true } end-to-end assertion measured behaviour the producer does not have and could not be corrected in place — that fake cannot represent read scoping at all. So:

  • the two verdict assertions are retained (checkEditdeny, checkAuthoredRowWriteadmit) — a fake can legitimately pin provenance composition against the real member_default seed;
  • the end-to-end write assertion is removed, with the measurement that killed it written into the case;
  • the retained admit assertion is flagged in the file as scope-blind — it is true on the real stack now, but it is green here for the fake's reason and would stay green if the producer regressed;
  • a file-header ⚠️ block states the double's blind spot, and plugin-sharing's deferral test, whose header named this file as "the REAL composition measured end-to-end", now points at the real-stack pin instead.

The replacement is packages/qa/dogfood/test/authored-row-write-scope.dogfood.test.ts, in the bulk-widener-probe.dogfood.test.ts idiom: real bootStack, real SecurityPlugin, real SharingServicePlugin, real ObjectQL engine, real middleware chain. Two objects identical in every respect except the OWD, one widener text, one principal, one cross-owner row shape.

2. The measurement

object OWD verdict before verdict after by-id PATCH before by-id PATCH after
wscope_note public_read admit admit 200, row changes 200, row changes
wscope_secret private abstain admit 403 403

Control, ruling out an inert declaration: on that same private object the caller's own admitted row answers admit on both sides.

3. The fix

The probe read moves to an elevated, principal-less context and is projected to id. The predicate is unchanged and is the whole of the question: {id} AND layer0(tenant wall) AND layer1(app-authored policies), both layers still compiled from the caller's own permission sets and tenant before the read. Elevating the scope therefore cannot widen the answer — it only stops a READ decision from being folded into a question about a declaration.

packages/spec's ISecurityService doc listed "the row is unreadable" among the abstain cases; that clause is the conflation the ruling removed, so it is corrected. Documentation only — no signature, shape or vocabulary change, still optional, still fail-closed.

⚠️ Measured, and NOT papered over: the widener is still dead end-to-end on private

The ruling's stated goal — "otherwise the by-id widener is structurally dead on private" — is not reached by the ruled fix alone, and the reason is a second gate with the identical shape. The by-id write pre-image gate (security-plugin.ts:1301-1306) performs its own findOne under opCtx.context, the caller's context, and is blind to the same cross-owner row for the same reason the probe was. Measured refusal on private, before and after:

403  {"error":"[Security] Access denied: not permitted to update this 'wscope_secret' record (row-level security)",
      "code":"PERMISSION_DENIED","object":"wscope_secret"}

That is the row-level gate's envelope, not the sharing middleware's FORBIDDEN: insufficient privileges — so on this posture the security gate refuses first and the deferral this verdict feeds is never even reached. (This also refines the card's own mechanism sketch, which attributed the 403 to the sharing middleware's refusal standing.)

I did not extend the fix to that gate. Its caller-scoped read is deliberate and documented in place ("a read denial … is itself a 'cannot touch this row' signal"), and changing it means answering may you write a row you cannot read — a public-contract question the ruling did not put to the maintainer. Filed as #7401 (unassigned, for triage) and raised in the report; both behaviours are pinned here so changing it later is a deliberate act with a red test to justify.

The no-leak claim, measured rather than assumed

"No leak either way: the final write gate still enforces" is the maintainer's argument; these are the measurements behind it. All are green on both sides of the change — that invariance is the claim.

  • A row the declaration does not admit is still abstain, on both postures — the elevated read never turns "invisible" into "permitted".
  • A principal with the SAME object CRUD and no declaration gains nothing — every row, both postures, abstain; and still refused end-to-end with the ADR-0112 envelope.
  • The elevation is confined to the probe — a unit case wraps every engine verb and asserts that every call the probe makes on the target object is a READ, carries isSystem, carries no principal, and is not the caller's own context object; the caller's context is deep-compared before/after and is unmutated; and during a real by-id UPDATE through the middleware every read of the target row still carries the caller's userId and no elevation.
  • The probe returns a verdict and nothing else — projected to ['id'], so no column of a row the caller may not read is ever materialised.
  • The tenant wall did not move — it lives in the query. Reverse-verified by mutation: dropping layer0 from the composed predicate turns the existing cross-tenant case red (expected 'admit' to be 'abstain').
  • The caller's read scope is unchanged after the probe runs — the row the verdict admits is still invisible to the caller as a read.

Reverse verification — direction predicted in writing before the first mutation

Predictions were committed as 7d7c358f3 before the producer was touched (file removed in the final commit; the commit stands as the timestamp). Predicted: the new real-stack pin is red against today's producer, green after; every no-leak case and both controls green on both sides; the end-to-end private PATCH still 403 after the fix, refused by the pre-image gate — "if it lands 2xx after, my reading of the pre-image gate is wrong and I must re-derive."

Measured, producer taken out with git checkout origin/main -- security-plugin.ts (never git stash), dist rebuilt each way:

BEFORE  × ⭐ [B private] … → admit          AssertionError: expected 'abstain' to be 'admit'
        × ⚠️ [E2E private] …                AssertionError: expected 'abstain' to be 'admit'
        Tests  2 failed | 10 passed (12)

AFTER   Tests  12 passed (12)

Two further mutations, since a case that cannot bite is not coverage:

M1  producer reverted, unit files:
    × the probe reads under an elevated, PRINCIPAL-LESS context — and reads only
    × the elevated scope carries the WHOLE question in the query
    Tests  2 failed | 26 passed (28)

M2  layer0 dropped from the composed predicate:
    × abstain — the matching row in ANOTHER tenant (Layer 0 stays AND-ed in)
      AssertionError: expected 'admit' to be 'abstain'
    Tests  2 failed | 15 passed (17)

Prediction and measurement agree case for case, including the one that matters most: the private PATCH is still 403 after the fix. Two retained cases cannot bite on this change — "the caller's context is not mutated" and "the elevation does not reach the write path" — and both are flagged as guards in the test file itself, not only here.

Gates

  • pnpm --filter @objectstack/dogfood exec vitest run test/authored-row-write-scope.dogfood.test.ts12 passed
  • pnpm --filter @objectstack/plugin-security test882 passed (43 files)
  • pnpm --filter @objectstack/plugin-sharing test418 passed (16 files)
  • typecheck — clean for spec, plugin-security, plugin-sharing, dogfood
  • eslint on all six changed files — clean
  • check:nul-bytes, check:authz-resolver, check:engine-double-contract, check:error-code-casing, check:route-envelope, check:query-options-erasure, check:verify-stand-in — all green
  • control-byte self-scan over every changed file — no hits

Changeset added (patch on plugin-security + spec): behaviour changes in a permission path, but only in the non-widening direction and with no end-to-end effect on any posture — nothing an app can observe as a new capability, and no contract shape moves.

claude added 2 commits August 10, 2026 09:20
…r an elevated scope (#7281)

The verdict answers "does the declared, app-authored widener admit this row",
which is a question about the row and the policy. It was resolved by re-reading
the row through the caller's own execution context, so plugin-sharing's READ
filter applied and a `private`-OWD cross-owner row was invisible to the question
asked about it — `abstain` for a row the declaration names. The by-id widener
surface was structurally dead on the posture #5493 built it for.

Ruled by the maintainer on 2026-08-10 (reading 2): resolve the probe read under
a scope that can see the row; the write decision stays with the pre-image gate.

The predicate — {id} AND layer0(tenant wall) AND layer1(authored policies) —
still carries the whole question and still compiles from the caller's own
permission sets and tenant, so nothing about the ANSWER widens with the scope.
The read is projected to `id`, so the probe can learn existence and nothing else.

Test half, landed independently and first per the ruling: a real-stack pin
(packages/qa/dogfood/test/authored-row-write-scope.dogfood.test.ts) that measures
the verdict on both OWD postures, plus six no-leak cases; and the fake-engine
unit file's end-to-end assertion — which asserted a `private` cross-owner write
LANDING, which the real stack refuses — removed, with the double's blind spot
written into the file.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01BM1tNf5U3nEbHKR4fo5qVQ
@vercel

vercel Bot commented Aug 10, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment
Project Deployment Actions Updated (UTC)
objectstack Ignored Ignored Aug 10, 2026 9:46am

Request Review

@github-actions

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 2 package(s): @objectstack/plugin-security, @objectstack/spec.

109 hand-written doc(s) reference the affected code and may need an implementation-accuracy re-verification:

  • content/docs/ai/agents.mdx (via @objectstack/spec)
  • content/docs/ai/skills-reference.mdx (via @objectstack/spec)
  • content/docs/ai/skills.mdx (via @objectstack/spec)
  • content/docs/api/client-sdk.mdx (via @objectstack/spec)
  • content/docs/api/environment-routing.mdx (via @objectstack/spec)
  • content/docs/api/error-catalog.mdx (via @objectstack/spec)
  • content/docs/api/error-handling-client.mdx (via @objectstack/spec)
  • content/docs/api/error-handling-server.mdx (via @objectstack/spec)
  • content/docs/api/index.mdx (via @objectstack/spec)
  • content/docs/automation/approvals.mdx (via @objectstack/spec)
  • content/docs/automation/connectors.mdx (via @objectstack/spec)
  • content/docs/automation/flows.mdx (via @objectstack/spec)
  • content/docs/automation/hook-bodies.mdx (via packages/spec)
  • content/docs/automation/hooks.mdx (via @objectstack/spec)
  • content/docs/automation/index.mdx (via @objectstack/spec)
  • content/docs/automation/webhooks.mdx (via @objectstack/spec)
  • content/docs/automation/workflows.mdx (via @objectstack/spec)
  • content/docs/concepts/architecture.mdx (via @objectstack/spec)
  • content/docs/concepts/design-principles.mdx (via packages/spec)
  • content/docs/concepts/index.mdx (via @objectstack/spec)
  • content/docs/concepts/metadata-driven.mdx (via @objectstack/spec)
  • content/docs/concepts/metadata-lifecycle.mdx (via packages/spec)
  • content/docs/concepts/north-star.mdx (via @objectstack/spec)
  • content/docs/data-modeling/analytics.mdx (via @objectstack/spec)
  • content/docs/data-modeling/drivers.mdx (via @objectstack/spec)
  • content/docs/data-modeling/external-datasources.mdx (via @objectstack/spec)
  • content/docs/data-modeling/field-types.mdx (via @objectstack/spec)
  • content/docs/data-modeling/fields.mdx (via @objectstack/spec)
  • content/docs/data-modeling/formulas.mdx (via @objectstack/spec)
  • content/docs/data-modeling/index.mdx (via @objectstack/spec)
  • content/docs/data-modeling/objects.mdx (via @objectstack/spec)
  • content/docs/data-modeling/queries.mdx (via @objectstack/spec)
  • content/docs/data-modeling/schema-design.mdx (via @objectstack/spec)
  • content/docs/data-modeling/seed-data.mdx (via @objectstack/spec)
  • content/docs/data-modeling/validation-rules.mdx (via @objectstack/spec)
  • content/docs/data-modeling/validation.mdx (via @objectstack/spec)
  • content/docs/deployment/cli.mdx (via @objectstack/plugin-security, @objectstack/spec)
  • content/docs/deployment/tenancy-modes.mdx (via @objectstack/spec)
  • content/docs/deployment/troubleshooting.mdx (via @objectstack/spec)
  • content/docs/deployment/validating-metadata.mdx (via @objectstack/spec)
  • content/docs/getting-started/build-with-claude-code.mdx (via @objectstack/spec)
  • content/docs/getting-started/common-patterns.mdx (via @objectstack/spec)
  • content/docs/getting-started/examples.mdx (via @objectstack/spec)
  • content/docs/getting-started/quick-reference.mdx (via @objectstack/spec)
  • content/docs/getting-started/quick-start.mdx (via @objectstack/spec)
  • content/docs/getting-started/your-first-project.mdx (via @objectstack/spec)
  • content/docs/kernel/cluster.mdx (via @objectstack/spec)
  • content/docs/kernel/contracts/auth-service.mdx (via packages/spec)
  • content/docs/kernel/contracts/cache-service.mdx (via packages/spec)
  • content/docs/kernel/contracts/data-engine.mdx (via @objectstack/spec)
  • content/docs/kernel/contracts/index.mdx (via @objectstack/spec)
  • content/docs/kernel/contracts/metadata-service.mdx (via packages/spec)
  • content/docs/kernel/contracts/storage-service.mdx (via @objectstack/spec)
  • content/docs/kernel/index.mdx (via packages/spec)
  • content/docs/kernel/runtime-services/data-service.mdx (via @objectstack/spec)
  • content/docs/kernel/runtime-services/email-service.mdx (via packages/spec)
  • content/docs/kernel/runtime-services/examples.mdx (via @objectstack/spec)
  • content/docs/kernel/runtime-services/index.mdx (via packages/spec)
  • content/docs/kernel/runtime-services/queue-service.mdx (via packages/spec)
  • content/docs/kernel/runtime-services/sharing-service.mdx (via @objectstack/plugin-security, @objectstack/spec)
  • content/docs/kernel/runtime-services/sms-service.mdx (via packages/spec)
  • content/docs/kernel/runtime-services/storage-service.mdx (via @objectstack/spec)
  • content/docs/kernel/services-checklist.mdx (via @objectstack/plugin-security, @objectstack/spec)
  • content/docs/kernel/services.mdx (via @objectstack/spec)
  • content/docs/permissions/access-recipes.mdx (via packages/plugins/plugin-security)
  • content/docs/permissions/authorization.mdx (via @objectstack/plugin-security, @objectstack/spec)
  • content/docs/permissions/explain.mdx (via @objectstack/plugin-security)
  • content/docs/permissions/permission-sets.mdx (via @objectstack/spec)
  • content/docs/permissions/permissions-matrix.mdx (via packages/plugins/plugin-security, @objectstack/spec)
  • content/docs/permissions/positions.mdx (via @objectstack/spec)
  • content/docs/permissions/rls.mdx (via @objectstack/spec)
  • content/docs/permissions/sharing-rules.mdx (via @objectstack/plugin-security, @objectstack/spec)
  • content/docs/permissions/system-context.mdx (via packages/spec)
  • content/docs/plugins/adding-a-metadata-type.mdx (via @objectstack/spec)
  • content/docs/plugins/development.mdx (via @objectstack/spec)
  • content/docs/plugins/index.mdx (via @objectstack/plugin-security, @objectstack/spec)
  • content/docs/plugins/packages.mdx (via @objectstack/plugin-security, @objectstack/spec)
  • content/docs/protocol/backward-compatibility.mdx (via @objectstack/spec)
  • content/docs/protocol/diagram.mdx (via packages/spec)
  • content/docs/protocol/kernel/config-resolution.mdx (via @objectstack/spec)
  • content/docs/protocol/kernel/http-protocol.mdx (via @objectstack/spec)
  • content/docs/protocol/kernel/i18n-standard.mdx (via @objectstack/spec)
  • content/docs/protocol/kernel/index.mdx (via @objectstack/spec)
  • content/docs/protocol/kernel/lifecycle.mdx (via @objectstack/spec)
  • content/docs/protocol/kernel/plugin-spec.mdx (via @objectstack/spec)
  • content/docs/protocol/knowledge.mdx (via @objectstack/spec)
  • content/docs/protocol/objectql/index.mdx (via @objectstack/spec)
  • content/docs/protocol/objectql/query-syntax.mdx (via @objectstack/spec)
  • content/docs/protocol/objectql/schema.mdx (via @objectstack/spec)
  • content/docs/protocol/objectql/security.mdx (via packages/spec)
  • content/docs/protocol/objectql/state-machine.mdx (via @objectstack/spec)
  • content/docs/protocol/objectui/actions.mdx (via @objectstack/spec)
  • content/docs/protocol/objectui/concept.mdx (via @objectstack/spec)
  • content/docs/protocol/objectui/index.mdx (via @objectstack/spec)
  • content/docs/protocol/objectui/layout-dsl.mdx (via @objectstack/spec)
  • content/docs/protocol/objectui/record-alert.mdx (via @objectstack/spec)
  • content/docs/protocol/objectui/widget-contract.mdx (via @objectstack/spec)
  • content/docs/ui/actions.mdx (via @objectstack/spec)
  • content/docs/ui/apps.mdx (via @objectstack/spec)
  • content/docs/ui/audience-based-interfaces.mdx (via packages/plugins/plugin-security)
  • content/docs/ui/create-vs-edit-form.mdx (via @objectstack/spec)
  • content/docs/ui/dashboards.mdx (via @objectstack/plugin-security, @objectstack/spec)
  • content/docs/ui/field-grouping-and-order.mdx (via @objectstack/spec)
  • content/docs/ui/forms.mdx (via @objectstack/spec)
  • content/docs/ui/index.mdx (via @objectstack/spec)
  • content/docs/ui/public-data-collection.mdx (via @objectstack/spec)
  • content/docs/ui/setup-app.mdx (via @objectstack/spec)
  • content/docs/ui/translations.mdx (via @objectstack/spec)
  • content/docs/ui/views.mdx (via @objectstack/spec)

7 release-owned page(s) also reference the affected code. These are read-only:

  • content/docs/releases/implementation-status.mdx (via @objectstack/plugin-security, @objectstack/spec)
  • content/docs/releases/index.mdx (via @objectstack/spec)
  • content/docs/releases/v12.mdx (via @objectstack/spec)
  • content/docs/releases/v13.mdx (via @objectstack/spec)
  • content/docs/releases/v16.mdx (via @objectstack/spec)
  • content/docs/releases/v17.mdx (via @objectstack/spec)
  • content/docs/releases/v9.mdx (via @objectstack/spec)

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.

Advisory only. To re-verify, run the docs-accuracy-audit workflow scoped to these files:
node scripts/docs-audit/affected-docs.mjs origin/main → pass the list as args.docs.

Copy link
Copy Markdown
Contributor Author

PM review — accepted. Verified from the diff rather than from the report.

The elevation is confined, which is the only thing that made this landable. AUTHORED_ROW_WRITE_PROBE_CONTEXT is principal-less (isSystem, no userId), spread at the single call site rather than passed by reference so no middleware can stamp state onto a shared object, and the read is projected to fields: ['id'] so the probe can learn existence and nothing else. The predicate is untouched — {id} AND layer0(tenant) AND layer1(authored) — and both layers still compile from the caller's own permission sets and tenant before the read. The scope moved; the question did not.

Also correct, and easy to have missed: the old JSDoc paragraph claiming the caller-scoped read was "the non-widening direction" is deleted, not left standing beside its own refutation. That sentence is exactly the kind of stale prose three cards in this lane were spent removing today.

The finding is the most valuable thing in this PR

The report does not claim the ruling achieved its purpose, and it is right not to. Measured: on private the 403 comes from the by-id write pre-image gate (security-plugin.ts:1301-1306, PERMISSION_DENIED, "(row-level security)"), which performs the same caller-scoped findOne and runs before the sharing middleware — so the deferral this verdict feeds is never reached on that posture, and the by-id write is still refused end-to-end, byte-identical envelope.

So the ruling's premise holds exactly (the verdict was answering the wrong question and now answers the right one), while its implied consequence — that fixing the probe revives the by-id widener on private — does not follow. That gap was predicted in writing before the first mutation ("if it lands 2xx after, my reading of the gate is wrong and I must re-derive"), which is what makes it a measurement rather than a rationalisation after a surprise.

Not extending the fix into that gate was the right call twice over: it is outside the ruled scope, and it answers a different public-contract question — may you write a row you cannot read — which the ruling never put to the maintainer. Filed as #7401 with both readings, the measurement, and today's behaviour pinned so either direction needs a deliberate change with a red test. I have escalated the gap on #7281 as well, so it does not live only inside a merged PR.

What I checked on the no-leak set

The claim under review was the maintainer's — "no leak either way: the final write gate still enforces" — and it is now measured rather than inherited. The ones that carry weight are the two that go red against the pre-fix producer (expected undefined to be true on the elevation, expected undefined to deeply equal [id] on the projection): a confinement assertion that passes both before and after would prove nothing. Alongside them: a row outside the declaration stays abstain on both postures; a principal with identical CRUD and no declaration gains nothing on either; the caller's context is deep-compared unmutated; every read during a real by-id UPDATE still carries the caller's userId; and M2 (dropping layer0) turns the cross-tenant case red, proving the tenant wall lives in the predicate and not in the scope that was just elevated.

On the fake-engine assertion

The disposition is right and the reasoning is the part I want on record. The end-to-end write assertion was removed rather than corrected, because makeEngine() registers no middleware and therefore cannot represent read scoping at all — it was green on precisely the axis the double is blind to. The verdict assertions stay, since a fake can legitimately pin provenance composition, and the retained admit is flagged in the file as scope-blind: true on the real stack today, but green here for the fake's reason and it would stay green if the producer regressed. Correcting plugin-sharing's deferral-test header — which claimed this file measured "the REAL composition end-to-end" — was necessary, not tidying: that sentence was the reason nobody looked.

Two retained cases that cannot bite on this change are flagged in the test file, not only in the report.

CI: 27 runs, all completed — 25 success + 2 skipped by design. ESLint success 09:50:07Z, TypeScript Type Check success 10:00:55Z.

Marking ready and landing through the merge queue. Landing this does not close the widener story on private; #7401 does, whichever way it is ruled.


Generated by Claude Code

@os-zhuang
os-zhuang marked this pull request as ready for review August 10, 2026 10:04
@os-zhuang
os-zhuang added this pull request to the merge queue Aug 10, 2026
Merged via the queue into main with commit 73f69dc Aug 10, 2026
28 checks passed
@os-zhuang
os-zhuang deleted the claude/issue-7281-authored-row-write-probe-scope branch August 10, 2026 10:21
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