Skip to content

fix(plugin-sharing): run the ADR-0111 D7 inert-grant guard for SYSTEM callers - #8311

Merged
os-zhuang merged 5 commits into
mainfrom
claude/issue-8207-system-caller-inert-grant-guard
Aug 13, 2026
Merged

fix(plugin-sharing): run the ADR-0111 D7 inert-grant guard for SYSTEM callers#8311
os-zhuang merged 5 commits into
mainfrom
claude/issue-8207-system-caller-inert-grant-guard

Conversation

@os-zhuang

@os-zhuangos-zhuang commented Aug 13, 2026

Copy link
Copy Markdown
Contributor

Fixes#8207

SharingService.grant skipped both of its pre-flights for a system context in one
block. The two halves ask different questions and only one may vary by caller: D1
(assertCanManageShares) is an AUTHORIZATION check, which a rule rightly skips because a
rule is not a principal; D7 is an INERTNESS check — "would any gate ever read a row on this
object?" — whose answer cannot depend on who asks, because the gates that would consult the
row never see the granter.

The inertness verdict is now computed caller-free (inertGrantReason) and refused for every
caller (assertNotInertGrant). The D1 management gate stays system-skipped. The EXISTENCE
check also stays non-system-only: an unresolvable object name is a caller's mistake
(NOT_FOUND), but for the evaluator it is a stored object_name against an engine that may
not have that schema registered yet, and absence of a schema is absence of evidence of
inertness rather than evidence of it.

Provenance. Two earlier agent runs were killed by container restarts before pushing or
reporting. Their work is commits 0c5b42afc and 1018d75fb, inherited unverified and
re-judged on the merits here. Everything below is measured on this branch, not carried over.

Ablation — prediction written before the mutation, then run

Both ablations were predicted in full (per test, not per total) and the prediction file was
written to disk before the production code was touched. An assertion whose expectation and
reality derive from the same source cannot fail; these can.

A — revert sharing-service.ts to origin/main, keep the tests

Baseline 24/24 green. Predicted 17 FAIL / 7 PASS; actual 17 FAIL / 7 PASS, and the
failing set matched test-for-test.

testpredictedactual
refuses a SYSTEM grant on %s (whiteboard, note, detail_item, ext_nostamp, sys_user)FAIL x5FAIL x5
the SYSTEM refusal on %s is the SAME verdict the user path gives (x5)FAIL x5FAIL x5
an engine with NO schema access still cannot answer, for either callerFAILFAIL
evaluating a rule on %s mints NOTHING (x5)FAIL x5FAIL x5
the boot backfill still COMPLETESFAIL, reconciled 6 not 1FAIL, expected 6 to be 1
the code is one the package DECLARES in the ADR-0112 ledgerPASSPASS
ANTI-VACUITY: a SYSTEM grant on an ordinary private object still succeedsPASSPASS
ANTI-VACUITY: the row that system grant wrote is LIVEPASSPASS
the D1 MANAGEMENT gate stays system-skippedPASSPASS
an UNRESOLVABLE object stays a user-only NOT_FOUNDPASSPASS
ANTI-VACUITY: the same rule shape on an ordinary object still materialisesPASSPASS
WITHDRAWAL still works on an inert objectPASSPASS

The non-obvious one is an engine with NO schema access — it fails not on its first
assertion but its second, because on main the whole-block system skip also bypasses the
bypass-object check, which needs no schema at all. Predicted as such, observed as such.

B — delete the SHARING_NOT_ENABLED 422 arm from rest-server.ts

Baseline 5/5 green. Predicted 2 FAIL / 3 PASS; actual 2 FAIL / 3 PASS.

testpredictedactual
answers 422 SHARING_NOT_ENABLEDFAIL, falls through to 500expected 500 to be 422, body {"code":"RULE_EVALUATE_FAILED", ...}
the message names the object and survives, minus the internal prefixFAIL on the 2nd assertion only (the prefix startsWith flips true); the 1st still passes because the 500 body carries the whole messageexpected true to be false — the 2nd assertion, as predicted
the code is one the platform DECLARESPASSPASS
ANTI-VACUITY: a successful evaluation still answers 200PASSPASS
ANTI-VACUITY: the sibling arms still map to their own statusesPASSPASS

Boot rule backfill — measured on a REAL engine, not a fake

The issue names this as the most likely way the change breaks something real. The committed
unit test covers it against a fake engine; that is not a boot. This was re-measured on a real
ObjectQL engine, a real SqlDriver on sqlite :memory:, real DDL through syncSchemas(),
and the realSysRecordShare / SysSharingRule platform object definitions, driving the
real backfillRuleGrants with two real rules — one ordinary, one on an inert object:

WARN boot rule backfill failed for rule {"rule":"share_whiteboard_gold","error":"SHARING_NOT_ENABLED: 'whiteboard' is not under record-sharing enforcement (public sharing model or no 'owner_id' field); a share row on it would never be consulted"}
INFO boot rule backfill done {"rules":2,"reconciled":1,"ms":8}
rows minted: [{"o":"account","r":"r1"}]

The backfill completes. The inert rule is refused with a reason naming the object, the
operator gets it by rule name, and the sibling ordinary rule still materialises its grant.
That harness was a scratch file, run and deleted; it is not part of this diff.

Worth recording: no shipped example app declares a sharing rule at all, so booting
app-showcase cannot reach this log line (backfillRuleGrants only logs when
rules.length > 0). That matches the issue's own "dormant as far as I measured" framing, and
is why the measurement above builds the rules explicitly.

The check:route-envelope ratchet claim in 1018d75fb — verified, not assumed

The commit claims the sibling arms are declared debt held by a down-only ratchet. All three
halves measured:

  1. The ratchet exists.scripts/check-route-envelope.mjs, wired as check:route-envelope,
    declaring siblingCode: 69 for rest-server.ts.
  2. The flat dialect would have been new debt. Reverting this arm to
    res.status(422).json({ code, error }) reproduces the commit's quoted line verbatim:
    siblingCode: found 70, declared 69 — a NEW non-conforming body.
  3. The sibling arms are genuinely counted. Converting one sibling (RULE_NOT_FOUND) onto
    the shared envelope drops the count: siblingCode: found 68, declared 69 — 1 fewer than pinned. So those arms really are inside the 69.

One correction to the framing, in the PR rather than the code: the gate is an exact-equality
check, so it reddens in both directions. "Ticks down only" is the policy for lowering the
declared number, not the gate's behaviour — which is precisely why converting the siblings here
was not an option: banking that progress requires editing the declared number, a maintainer
action. The comment's substance is correct and stands.

Merits judgement on the inherited commits

Both hold up. Specific things checked rather than assumed:

  • The assertSharingEnforced refactor is behaviour-preserving on the non-system path. Old
    order: bypass throws first, then getSchema missing returns, then !schema is NOT_FOUND.
    New order: the NOT_FOUND probe is guarded by !bypassObjects.has(object), so a bypass
    object still yields SHARING_NOT_ENABLED rather than NOT_FOUND. Same verdicts, same
    precedence. It has exactly one caller (the non-system grant path), so nothing else moved.
  • The "every system caller treats a per-rule throw as best-effort" claim is real:
    backfillRuleGrants wraps each evaluateRule in try/catch, warns, and continues, and the
    boot rule backfill done line is outside that loop.

Two consequences worth stating, neither a defect:

  • On the non-system path getSchema is now called twice (once for existence, once inside
    inertGrantReason). Trivial cost, and the alternative re-entangles the two verdicts.
  • A rule on an inert object now aborts reconcile at the first grant, so that rule's
    revoke-stale phase no longer runs. Rows an older build minted on such an object therefore
    stay until deleteRule or a deactivated-rule evaluation purges them — both untouched and
    both pinned by the WITHDRAWAL test. No access consequence: those rows are inert by
    definition.

Gate fix folded in

check:engine-double-contract was red on the inherited test file (reported under the
ESLint job, which runs a battery). Its fake engine's update was loose in two ways: it
mirrored the DRIVER arity update(object, id, data), which IDataEngine does not have, and
its engine arm dispatched on a hand-derived data.id with no rejection surface. It now opens
with assertEngineUpdateDispatch(data, options) from @objectstack/metadata-core — already a
dependency here, and already the source of the assertEngineDeleteDispatch two lines below.
No baseline entry was added; that file is shrink-only and raising it is a maintainer action.

Probed while in there, per the gate's own dormancy methodology: this suite never calls that
update member (a stderr marker printed 0 times, while the same marker on insert printed 20
times in the same run). So the looseness was unexercised — the fix is correct and required, but
it hardens a currently-dormant path rather than repairing a live false green.

Verification run

gate / suiteresult
plugin-sharing full suite21 files, 569 passed
rest full suite110 files, 1814 passed
typecheck (plugin-sharing + rest)pass, both
check:engine-double-contractpass — 186 pinned, 133 DEBT, 2 exempt
check:route-envelopepass — stringError 44, siblingCode 69
check:nul-bytespass
check:error-code-casingpass

Not run locally, left to CI: the rest of the lint-workflow gate farm, and the full-workspace
build/test. One pre-existing eslint finding at rest-server.ts:4595
(ban-ts-comment rule not found) reproduces unchanged on origin/main and is an artifact of
invoking npx eslint on single files outside the repo's lint entrypoint — not from this diff.


Generated by Claude Code


Generated by Claude Code

… callers (#8207)
`SharingService.grant` skipped both of its pre-flights for a system context in
one block. The two halves ask different questions and only one may vary by
caller: D1 (`assertCanManageShares`) is an AUTHORIZATION check, which a rule
rightly skips because a rule is not a principal; D7 is an INERTNESS check —
"would any gate ever read a row on this object?" — whose answer does not depend
on who asks, because the gates that would consult the row never see the granter.
Measured on origin/main @ a7e94e9: the rule evaluator does NOT independently
reject the inert object classes. One rule per class, defineRule then
evaluateRule under a system context, materialised a real sys_record_share row
for all five — public model, owner-less, controlled_by_parent, federated
phantom anchor (#8119), bypass object.
The inertness verdict is now computed caller-free (`inertGrantReason`) and
refused for every caller (`assertNotInertGrant`). The EXISTENCE check stays
non-system-only: an unresolvable object name is a caller's mistake (NOT_FOUND),
but for the evaluator it is a stored object_name against an engine that may not
have that schema registered yet, and absence of a schema is absence of evidence
of inertness rather than evidence of it.
`POST /sharing/rules/:idOrName/evaluate` maps the refusal to 422
SHARING_NOT_ENABLED — the same code/status pair the per-record shares routes
already publish — instead of burying the diagnosis in a 500.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01PEVB6w7D7uCszR9Mw1BL73
…nvelope (#8207)
The 422 arm added by the previous commit copied its three neighbours in
`registerSharingRuleEndpoints` — the flat `{ code, error: '<string>' }` dialect,
`code` beside `error` instead of inside it, so `body.error.code` reads
`undefined`. Those three are #7035's declared debt, held down by the
`check:route-envelope` ratchet's `siblingCode` count, which only ticks DOWN.
Measured on this branch: with the flat arm the gate reports
`siblingCode: found 70, declared 69 — a NEW non-conforming body`; through the
shared `sendError` (`sendEnvelopeError`, aliased at rest-server.ts:89) it is
green at 69. A new arm copying its neighbours' shape is exactly what that
ratchet exists to stop, and raising the declared number is a maintainer action,
not this card's.
The test file now pins the NESTED pair for the new arm (`body.error.code` +
`body.error.message`, `success: false`, no top-level `code`), and keeps reading
the sibling arms through the position-tolerant helper — they are asserted for
their STATUS only, which is all this card claims about them, so converting them
later under #8111's unfinished half does not go red here.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01PEVB6w7D7uCszR9Mw1BL73
@vercel

vercelBot commented Aug 13, 2026

Copy link
Copy Markdown

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

1 Skipped Deployment
ProjectDeploymentActionsUpdated (UTC)
objectstackIgnoredIgnoredAug 13, 2026 4:38am

Request Review

@github-actions

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

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

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

  • content/docs/ai/connect-mcp.mdx(via @objectstack/rest)
  • content/docs/api/error-handling-server.mdx(via @objectstack/rest)
  • content/docs/api/index.mdx(via @objectstack/rest)
  • content/docs/kernel/runtime-services/examples.mdx(via @objectstack/plugin-sharing)
  • content/docs/kernel/runtime-services/sharing-service.mdx(via @objectstack/plugin-sharing)
  • content/docs/kernel/services-checklist.mdx(via @objectstack/plugin-sharing)
  • content/docs/permissions/authentication.mdx(via @objectstack/rest)
  • content/docs/permissions/authorization.mdx(via packages/plugins/plugin-sharing)
  • content/docs/permissions/permissions-matrix.mdx(via packages/plugins/plugin-sharing)
  • content/docs/permissions/system-context.mdx(via packages/rest)
  • content/docs/plugins/index.mdx(via @objectstack/rest)
  • content/docs/plugins/packages.mdx(via @objectstack/plugin-sharing, @objectstack/rest)
  • content/docs/protocol/kernel/http-protocol.mdx(via @objectstack/rest)
  • content/docs/protocol/kernel/i18n-standard.mdx(via packages/rest)
  • content/docs/protocol/objectql/security.mdx(via packages/plugins/plugin-sharing)

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

  • content/docs/releases/implementation-status.mdx(via @objectstack/plugin-sharing, @objectstack/rest)
  • content/docs/releases/v12.mdx(via @objectstack/rest)
  • content/docs/releases/v17.mdx(via @objectstack/rest)

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.

@github-actionsgithub-actionsBot added documentation Improvements or additions to documentation tests tooling labels Aug 13, 2026
… dispatch contract
`check:engine-double-contract` reported the new test file's engine double as
`PINNED [update]: … declares 1 engine double(s) whose update() does not route
through assertEngineUpdateDispatch (line 88)`.
The double was loose in two ways, not one. It mirrored the DRIVER arity
(`update(object, id, data)`) alongside the engine one — a shape `IDataEngine`
does not have at all — and, on the engine arm, it dispatched on a hand-derived
`data.id` with no rejection surface, so it accepted the predicate updates
`ObjectQL.update` refuses. Both are the #4434 vacuity class the gate exists for:
a double that accepts what the producer rejects is a test that cannot fail.
`update` now opens with `assertEngineUpdateDispatch(data, options)` from
`@objectstack/metadata-core` — already a dependency of this package and already
the source of the `assertEngineDeleteDispatch` on the `delete` member two lines
below — and implements the returned `by-id` / `multi` verdicts separately.
`metadata-core` deliberately, not `@objectstack/objectql`: objectql depends on
this package's siblings and the reverse edge is a cycle turbo refuses.
No baseline entry added — `scripts/engine-double-contract.baseline.json` is
shrink-only and raising it is a maintainer action. Gate now reads
`OK — 186 pinned, 133 in the DEBT ledger, 2 exempt`; suite still 24/24.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01PEVB6w7D7uCszR9Mw1BL73
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

2 participants

@os-zhuang@claude