fix(plugin-dev): ask the published security service, in start(), whether anything is enforcing - #10092
Conversation
… 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
📓 Docs Drift CheckThis PR changes 1 package(s): 1 hand-written doc(s) NAME something this change touched and may need an implementation-accuracy re-verification:
⛔ 2 release-owned page(s) also name something this change touched. These are read-only:
What this run could not see
Coarse fallback — 3 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): Which tree this was computed onThis run read A worktree cut from an older # 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
|
… 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
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 frominit(). 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
SecurityPluginwithholding or poisoning its handles untilstart()succeeds, that lands inplugin-securityand 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:20rules on exactly this question:So the published
securityservice 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().securityis registered inSecurityPlugin.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 toDevPlugin.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)
plugin-security/src/security-plugin.ts:725-743init()registrations:794-812start()and its two early returns (:805,:810):1157securityservice:1166+ql.registerMiddlewarecallsplugin-dev/src/dev-plugin.ts:897Before/after — driven, not read from the source
New test file drives the real
SecurityPlugin(not a hand-written double) into the bailed-start state through the real DevPlugin assembly, and asserts the state before asserting the warning: all threeinit()handles resolve,securitydoes not, and the plugin logged its own bail.Against unmodified
main, both bail states were silent: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:That is the false positive the phase move exists to avoid. Restored from the commit (
git diff --quietclean) and re-run green. The subject resolves as intra-package source (./dev-plugin), so no rebuild is in that loop; the realSecurityPluginresolves throughexportstodist/, which was built before the measurement (it is already a registered unaliased import for this package incheck-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/andapps/— 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 branchspec/src/contracts/security-service.ts:20— the docblock quoted aboveplugin-security/src/security-plugin.ts:605,725-727—providesServices+ the registrations themselvessecurity.rls.predicateinpackages/specmigrations — a spec surface path, a different namespace, not a service nameNo new consumer appeared, so option 2's blast radius inside this repo is unchanged and still small.
Second commit: the
driver-memorycensus gate, disposed of by removalCI's
driver-memory census gateflagged the new test file as a thirteenth module binding on a frozen driver (#5499 / #5704 / #6664), because the census countsvi.mockas a DECLARATION deliberately.Disposition: the binding was dead weight, so it is gone — no ledger entry, no ruling assumed. DevPlugin imports
@objectstack/runtimeon 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 thedriver-memorymock factory printed 0 times across the whole file, while the same marker in the@objectstack/runtimefactory 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:
scripts/driver-memory-census.ledger.jsonis 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 commit897f83506, 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) · plusplugin-devtests (58/58) and typecheck.No gate weakened: no threshold moved, no baseline raised, no test skipped or retried.
Generated by Claude Code