fix(plugin-security): an RLS fail-closed denial logs its reason, not nothing

[claude]

Part of #13639 — the non-forked half only. The sentinel question and any further !ok consumers are measured and reported below, not delivered here.

The defect

compileCelToFilter computes a precise detail on every refusal — which current_user.* path did not resolve, which member of a membership array came back null and at what index, which parse bound was overrun. RLSCompiler.compileExpression consumed !ok and threw the rest away at if (!result.ok) return null;, one line before the only site that could surface it. The drop-warn beside it is gated on !isSupportedRlsExpression(predicate) — a SHAPE-only test that answers "supported" for exactly the membership and equality shapes that refuse at runtime — so that branch logged nothing at all.

The resulting failure shape is the worst-shaped one an operator can be handed: the user sees zero rows, no error is raised, nothing appears in the log. And the denial is deliberate — fail-closed working as designed — which is precisely why it needs a trace: a correct refusal is indistinguishable from "the data genuinely doesn't match", and the information that ends that search had already been computed.

The repair

compileExpressionOutcome returns the filter or the cause; compileExpression delegates to it and keeps its published Record | null signature unchanged. The drop site then splits by what actually happened:

cause	line	when
shape is not pushdownable (unsupported / parse-error on a predicate isSupportedRlsExpression rejects)	the existing ADR-0056 D4 DROPPED (no enforcement) warn, now also carrying the compiler's detail	every drop — this is an authoring fault, true for any input
shape is fine, the REQUEST could not be evaluated (unresolved-variable)	new DENY (fail closed) line naming the variable path, the member index, the policy, the object, the clause, the predicate and the consequence	only when the clause actually denies
emptied pre-resolved membership (the compiler answers ok; isEmptyMembershipFilter then refuses)	the same line, cause empty-membership — it names itself, since there is no compiler detail to carry	only when the clause actually denies

Two deliberate restrictions, both because this seam runs on read paths:

• A dropped policy is not a denial. If a sibling policy still grants, the caller sees rows and has no mystery to debug, so nothing is logged. The line is emitted at the filters.length === 0 fail-closed site, not per dropped policy.
• Once per distinct cause, not once per request. The card asked for dedup only if the surrounding code gives a way. It does: this is cel-to-filter.ts's warnedOverLimit memo — the immediately-upstream module in this same call chain — at the same bound (500) and with the same clear-on-overflow, held per RLSCompiler instance (the plugin holds one long-lived compiler). No new mechanism was invented. Without it the "no active organization" denial, a persistent session state, would emit a line per query for as long as it lasts.

Nothing about the decision moves

This is an observability change. Pinned in rls-denial-observability.test.ts: RLS_DENY_FILTER still lands with the same value, shape and single id key; the sentinel still excludes every record matched against it via matchesFilterCondition (the same predicate test explain's record-grained pass runs); compileExpression still returns null for a refusal and the identical filter for a resolving predicate; a resolving membership still compiles to the same $in.

Verification

All at 577e49e00, clean tree.

• pnpm --filter @objectstack/plugin-security test — 94 files, 1750 tests, 0 failed
• pnpm --filter @objectstack/plugin-security typecheck — exit 0. tsc --noEmit -p tsconfig.test.json --listFiles confirms all three edited files are inside the program (src/rls-compiler.ts, src/rls-denial-observability.test.ts, src/security-plugin.test.ts) — the typecheck is not green over files nothing read.
• pnpm lint (eslint . --no-inline-config, whole repo) — exit 0.
• Gate families re-derived from the ACTUAL diff with node scripts/pm/dispatch-gates.mjs (no path argument; it reads the change set itself). The model-tier path limb does not fire: "no path-derived mandate: the surface hits none of the 3 declared glob(s)". Green: check:cross-package-test-inputs, check:engine-double-contract, check:where-matcher, check:objectql-double-limit, check:test-source-alias, check:nul-bytes, check:logger-receiver-detach, check-keyed-text-bounds, check-undeclared-dep-imports, check:doc-authoring, check:i18n (after clearing its named build prerequisite), check:i18n-stale-fill, check:query-options-erasure, check:type-check-coverage, check:type-check-debt (incl. --re-measure: "29 ledger entries re-measured, 1531 raw tsc errors total, none above its recorded number"), check:changeset-gate-self-tests, check:objectui-changeset, check:pm-half-states, check:published-files, check:slot-lookup, check:page-declaration-shape, check:type-source-resolution, check:dual-build-cjs-loads (after turbo run build), check:shard-attestation, check-adr-0087-registration, check-changeset-no-major, check-empty-changeset, check-ci-filter-parity, check-comment-mask-adoption, check-plugin-teardown-shape, check-tenant-audit-census, check-cross-package-test-inputs, docs-audit/check-affected-docs, release-rehearsal-clone --self-test.
• NOT MEASURED, not green and not red: check-test-completeness.mjs exits 3 with no turbo run test log to parse ("the local reading for this gate is NOT MEASURED … it is not a red"); scripts/pm/check-half-states.mjs exits 2 on HTTP 502 from the GitHub API, an infrastructure fault unrelated to this diff.

Ablation

Direction predicted before running: reverting the gate fix leaves the drop silent again, so exactly the pins that assert the new line should redden — 6 in rls-denial-observability.test.ts and the 1 rewritten pin in security-plugin.test.ts — while every decision pin and every negative stays green.

The repair was committed first, so the restore leg has a real reference point. The mutation replaced the one emission line with a marker; the mutation was confirmed on disk before anything was measured, never by an editor's exit code: blob hash 3c54ac68… (equal to the HEAD blob) before, e0c6c524… after, removed-marker count 0, injected-marker count 1, git diff --stat showing 1 insertion / 1 deletion. The script carried trap … EXIT INT TERM with an absolute REPO_ROOT restore, and the restore is git checkout HEAD -- path — never the bare form, which restores from the index.

No rebuild leg is claimed or needed: the suite imports the subject relatively (./rls-compiler.js), which cannot resolve through a package exports map to dist/, so the mutated source is what ran. The measurement confirms it — the mutation reddened pins, which a stale-dist ablation cannot do.

Result — 7 failed, 250 passed, exactly the predicted 7:

× a membership shape whose member is null: the line names the VARIABLE PATH and the INDEX
× the SCALAR "no active organization" path names its variable too
× an EMPTY pre-resolved membership set names itself
× the `check` clause denies and reports under its own clause name (ADR-0058 D4)
× the SAME cause across repeated requests warns ONCE
× a DIFFERENT cause is a different line (dedup never swallows a new fact)
× a SUPPORTED shape whose context var is absent gets the DENY line, not the uncompilable one

Restore proven by state, not by the trap firing: on-disk blob back to 3c54ac68… = the HEAD blob, git diff HEAD empty, git status --porcelain empty, marker counts back to 0 / 1. Re-run on the restored tree: 257 passed, 0 failed.

Declared controls — green in BOTH directions, so they are controls and NOT ablation evidence: every pin in the DECISION is unchanged, plus a normal RESOLVING expression logs nothing, a shape that CORRECTLY logged before still logs — once, with its own message, a dropped policy that is NOT a denial stays silent, no applicable policy for the clause is NOT a denial, and a compiler with NO logger bound never throws. They hold the boundary of the change; they do not demonstrate it.

Out of scope — measured, not touched

1. The sentinel fork: are __deny_all__ and __rls_deny__ deliberately distinct?

explain's isDenyAll was NOT changed and the sentinels were NOT merged. One finding materially re-shapes the fork: the reported narrows verdict is not produced by isDenyAll at all. isDenyAll (explain-engine.ts:122) has exactly two call sites, both inside the record-grained augmentation (lines 728 and 897). The object-level rls layer verdict comes from a separate inline comparison at line 1312, filterParts.some((f) => f.id === '__deny_all__'), and that same denyAll also drives decision.allowed (line 1326) and replaces readFilter with { id: '__deny_all__' } (line 1313). Teaching only isDenyAll about __rls_deny__ would therefore not fix the symptom the card names. The full measurement, the two production/consumption censuses and the recommendation are in the report comment on #13639.

2. The other !ok consumers — census, with a reverse control

The filer marked this unmeasured. Every non-test call site of the compile-result family (compileCelToFilter, isPushdownableCel, lowerCelAst), excluding dist/, comments and the defining module:

call site	reads reason	reads detail
packages/plugins/plugin-security/src/rls-compiler.ts	✅ this PR	✅ this PR
packages/plugins/plugin-sharing/src/bootstrap-declared-sharing-rules.ts:132 (celToFilter)	❌	❌
packages/formula/src/rls-predicate.ts:59 (isSupportedRlsExpression)	❌	❌
packages/lint/scripts/check-doc-formula-expressions.mjs:962	✅	❌
packages/lint/src/validate-rls-predicate-enforceability.ts:283	✅	✅
packages/lint/src/validate-sharing-rule-enforceability.ts:438	✅	✅

Reverse control: the last two rows are the control that makes the zeroes readable — the same detector that reports celToFilter as discarding finds two consumers that carry detail into user-facing text, so "discards" is a reading and not a blind spot.

One consumer has the same shape as the repaired one and is NOT repaired here: celToFilter in the sharing seeder collapses { reason, detail } to null, and its caller logs the fact without the reason. It is materially less severe (boot-time, not per-request; the seeder does log a line naming the rule and its condition, so the operator is not left with nothing). It sizes a follow-up card rather than widening this one. isSupportedRlsExpression returns boolean by contract and is not a defect.

---

Generated by Claude Code

[github-actions]

📓 Docs Drift Check

10 anchor(s) derived from 1 changed package(s); no hand-written page names any of them, so this run has nothing to list — not a clean bill of health. This check sees only pages that NAME a derived anchor: one that documents this change in prose, or enumerates it in an authoring dialect, names none and stays invisible to it on every run.

What this run could not see

• 2 name(s) were too generic to anchor anything (single lowercase words)
• the SDK route bridge reached 47 of 219 client-bound route-ledger rows — the other 172 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run. Of those 172: 14 are remediable by widening that discovery convention (an in-repo file declares the path; the convention did not scan it); 56 are structural — on a ledger where NOT ONE row is declared in-repo, so no discovery change reaches them at any price; 102 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.

Coarse fallback — 14 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 dd3ea164879ec20b472945bc78814e9d98aaa0b0 → packageMentionDocs.

Which tree this was computed on

This run read content/docs from 818a510e9524e8ec95f0bc812529ca44021da95f — the merge of head 577e49e00266cf8b8f66082d6a38e9b763c79e64 into base dd3ea164879ec20b472945bc78814e9d98aaa0b0, 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 818a510e9524e8ec95f0bc812529ca44021da95f && git checkout 818a510e9524e8ec95f0bc812529ca44021da95f
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin dd3ea164879ec20b472945bc78814e9d98aaa0b0 577e49e00266cf8b8f66082d6a38e9b763c79e64 && git checkout -B drift-repro dd3ea164879ec20b472945bc78814e9d98aaa0b0 && git merge --no-ff 577e49e00266cf8b8f66082d6a38e9b763c79e64

node scripts/docs-audit/affected-docs.mjs --json dd3ea164879ec20b472945bc78814e9d98aaa0b0

⚠️ That checkout carried uncommitted changes, so the commit above does not fully identify what was read.