Skip to content

fix(plugin-dev): ask the published security service, in start(), whether anything is enforcing - #10092

Merged
os-elon merged 2 commits into
mainfrom
claude/issue-10036-security-warning-probe
Aug 20, 2026
Merged

fix(plugin-dev): ask the published security service, in start(), whether anything is enforcing#10092
os-elon merged 2 commits into
mainfrom
claude/issue-10036-security-warning-probe

Conversation

@os-elon

@os-elonos-elon commented Aug 20, 2026

Copy link
Copy Markdown
Collaborator

Fixes#10036

DevPlugin's "RBAC, row-level security and field masking are NOT enforced" warning could not fire in the state it describes. It probed security.permissions / security.rls / security.fieldMasker — and it probed them from init(). Both halves were wrong, in the direction that is hardest to notice: silence read as health.

Which fork this took, and why it is the honest fix rather than a settle

Triage attached an instruction: if the honest fix is SecurityPlugin withholding or poisoning its handles until start() succeeds, that lands in plugin-security and should be re-routed rather than guessed at from the consumer.

Measured: the consumer-side fix is the honest one, and the spec already says so.packages/spec/src/contracts/security-service.ts:20 rules on exactly this question:

Registered under the service name security by @objectstack/plugin-security … Related lower-level handles the same plugin registers — security.permissions, security.rls, security.fieldMasker — are implementation internals and deliberately NOT part of this contract.

So the published security service is the contract for "is anything enforcing?", and the three handles are internals that were never meant to carry that meaning. A consumer asking a capability question of an implementation internal is the contract violation; moving the reader onto the published contract is contract-first, not a patch over a producer bug. Option 2 would have had to re-shape an internal to carry a signal the spec deliberately withholds from it — that is the workaround, not this.

The second half the card did not predict: the phase, not just the name

The probe runs in DevPlugin.init(). security is registered in SecurityPlugin.start(). So swapping only the service name at the existing call site would have warned on every stack, healthy ones included — trading a false negative for a permanent false positive. The check therefore moves to DevPlugin.start(), after the child-start loop, into the boot banner (the placement #3900 already established for the production-override brand: the banner is the surface an operator actually reads).

The internal handles keep exactly one honest use — telling "never loaded" apart from "loaded, then failed to start" — so the warning names which state it saw and the operator is pointed at the right fix.

Anchors re-derived at the branch point (all held, to the line)

AnchorClaimHeld
plugin-security/src/security-plugin.ts:725-743the six init() registrationsyes
:794-812start() and its two early returns (:805, :810)yes — and they are the only two in the whole method
:1157published security serviceyes
:1166+ql.registerMiddleware callsyes
plugin-dev/src/dev-plugin.ts:897the probeyes

Before/after — driven, not read from the source

New test file drives the realSecurityPlugin (not a hand-written double) into the bailed-start state through the real DevPlugin assembly, and asserts the state before asserting the warning: all three init() handles resolve, security does not, and the plugin logged its own bail.

Against unmodified main, both bail states were silent:

× bail #1 (no objectql/metadata service) …
AssertionError: the dev assembly must warn that nothing is enforced: expected +0 to be 1
× bail #2 (engine cannot take middleware) …
AssertionError: expected +0 to be 1
Tests 2 failed | 56 passed (58)

After: Test Files 6 passed (6) · Tests 58 passed (58).

Reverse verification of the phase claim. Moving the call back into init() — the naive reading of "just swap the service name", keeping the corrected name — turns the healthy-stack control red, which is the whole point:

× does not fire when SecurityPlugin.start() completed and published the `security` service
AssertionError: expected [ Array(1) ] to deeply equal []
Tests 1 failed | 57 passed (58)

That is the false positive the phase move exists to avoid. Restored from the commit (git diff --quiet clean) and re-run green. The subject resolves as intra-package source (./dev-plugin), so no rebuild is in that loop; the real SecurityPlugin resolves through exports to dist/, which was built before the measurement (it is already a registered unaliased import for this package in check-test-source-alias.mjs).

Consumer sweep, re-run rather than inherited

The card claimed that after #10035 the three handles have no non-test consumers in this repo besides this probe. Re-run across packages/ and apps/claim holds. Every remaining hit is a test, a comment, the registrar, or an unrelated namespace:

  • plugin-dev/src/dev-plugin.ts — this probe (the card's subject)
  • plugin-hono-server/src/current-user-endpoints.ts:353-356 — prose only, the post-fix(plugin-hono-server): delegate permission-set resolution to the security service #10035 comment explaining the old degraded branch
  • spec/src/contracts/security-service.ts:20 — the docblock quoted above
  • plugin-security/src/security-plugin.ts:605,725-727providesServices + the registrations themselves
  • security.rls.predicate in packages/spec migrations — a spec surface path, a different namespace, not a service name

No new consumer appeared, so option 2's blast radius inside this repo is unchanged and still small.

Second commit: the driver-memory census gate, disposed of by removal

CI's driver-memory census gate flagged the new test file as a thirteenth module binding on a frozen driver (#5499 / #5704 / #6664), because the census counts vi.mock as a DECLARATION deliberately.

Disposition: the binding was dead weight, so it is gone — no ledger entry, no ruling assumed. DevPlugin imports @objectstack/runtime on the line before@objectstack/driver-memory, and that import is mocked to throw, so the driver import is never evaluated. Measured with a control rather than reasoned: a marker written from the driver-memory mock factory printed 0 times across the whole file, while the same marker in the @objectstack/runtime factory printed 8 times in the same run of the same harness. Removing the mock leaves the suite green at the same duration.

The census therefore stays at its 2 ruled consumers, which is the disposition that needs no maintainer ruling:

check-driver-memory-census: OK — every declaration is ledgered, every ledger entry is live,
and every ruled file states "#6664 census: 2 ruled consumers". Nothing here invests in the
driver (#5499 freeze).

scripts/driver-memory-census.ledger.json is untouched — the whole change set is 4 files, none of them a ledger, baseline, or gate script.

Gates

Derived with node scripts/pm/dispatch-gates.mjs (no paths passed — the script took its own change set from the merge base) against final commit 897f83506, working tree clean. All green, each read from the gate's own verdict line with the exit code captured before any pipe:

check:driver-memory-census (+ its self-test) · check:changeset-gate-self-tests · check:objectui-changeset · check:slot-lookup (ratchet holds, none new) · check:test-source-alias (72 packages) · check:type-source-resolution (76 packages) · check-adr-0087-registration · check-changeset-no-major · check-empty-changeset (1 declaring changeset) · check-affected-docs · check:query-options-erasure (none new) · check:type-check-coverage (64/77) · check:type-check-debt (--re-measure: 33 entries, none above its recorded number, surplus none) · check:engine-double-contract (325 pinned) · check:where-matcher (263 matchers) · pnpm lint (clean) · plus plugin-dev tests (58/58) and typecheck.

No gate weakened: no threshold moved, no baseline raised, no test skipped or retried.


Generated by Claude Code

… the "not enforced" warning can fire (#10036)
The warning probed `security.permissions` / `security.rls` /
`security.fieldMasker` from init(). Those are SecurityPlugin.init()
registrations that the spec contract names implementation internals; the
published `security` service is the contract, and it is registered only in
SecurityPlugin.start(), after both of that method's early returns and
alongside the enforcement middleware. A stack whose start() bailed holds all
three handles and enforces nothing, so the warning was silent in exactly the
state its text describes.
Probing `security` from init() would have been a permanent false positive
(start() has not run yet), so the check moves to DevPlugin.start(), after the
child-start loop and into the boot banner. The internal handles keep one
honest use: telling "never loaded" apart from "loaded, then failed to start".
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019bmVFqoQPq63zhKrxdYG1r
@github-actions

github-actionsBot commented Aug 20, 2026

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

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

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

  • content/docs/kernel/services-checklist.mdx(via DevPlugin (symbol), security.fieldMasker (literal), security.permissions (literal), security.rls (literal))

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

  • content/docs/releases/implementation-status.mdx(via security.fieldMasker (literal), security.permissions (literal), security.rls (literal))
  • content/docs/releases/v17.mdx(via DevPlugin (symbol))

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
  • 1 name(s) were too generic to anchor anything (single lowercase words)
  • 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 — 3 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 f80ccf90235cb7b2d1988961c963ad3c200d0fbbpackageMentionDocs.

Which tree this was computed on

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

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

… the new test (#10036)
The `driver-memory` census gate (#5499/#5704/#6664) flagged the new test file
as a thirteenth module binding on a frozen driver. It was dead weight, not a
consumer: DevPlugin imports `@objectstack/runtime` on the line BEFORE the
driver import, and that import is mocked to throw, so the driver import is
never evaluated.
Measured rather than reasoned, with a control: a marker written from the
`driver-memory` mock factory printed 0 times across the whole file, while the
same marker in the `@objectstack/runtime` factory printed 8 times in the same
run of the same harness. Removing the mock leaves the suite green (58/58) at
the same duration.
No ledger entry added and no ruling assumed — the census stays at 2 ruled
consumers, which is the disposition that needs no maintainer ruling.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019bmVFqoQPq63zhKrxdYG1r
@os-elon
os-elon marked this pull request as ready for review August 20, 2026 01:43
@os-elon
os-elon added this pull request to the merge queueAug 20, 2026
Merged via the queue into main with commit 7552e03Aug 20, 2026
29 checks passed
@os-elon
os-elon deleted the claude/issue-10036-security-warning-probe branch August 20, 2026 01:53
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentationImprovements or additions to documentationsize/mteststooling

Projects

None yet

2 participants

@os-elon@claude