fix(plugin-security): security/explain answers enforcement's refusal for a row-level policy comparing two fields of no shared comparison class - #20598
Conversation
… policy comparing two fields of no shared class WIP: fix in explain-engine.ts; pins follow. Claude-Session: https://claude.ai/code/session_01XY5uCwTjZj7884yYtyur4H Co-authored-by: Claude <noreply@anthropic.com>
…ass field comparison Claude-Session: https://claude.ai/code/session_01XY5uCwTjZj7884yYtyur4H Co-authored-by: Claude <noreply@anthropic.com>
…usal report Claude-Session: https://claude.ai/code/session_01XY5uCwTjZj7884yYtyur4H Co-authored-by: Claude <noreply@anthropic.com>
…al as the find does, INVALID_FILTER / 400
The report-shaped answer added a second refusal dialect beside the one explain
already gives the matcher's other INVALID_FILTER refusals (a { $field }
comparison against a list-holding column, pinned in
rls-stored-list-ordering-fails-closed.test.ts). Explain now fails with the
matcher's envelope, the message naming the policy and both columns.
Claude-Session: https://claude.ai/code/session_01XY5uCwTjZj7884yYtyur4H
Co-authored-by: Claude <noreply@anthropic.com>
…note Claude-Session: https://claude.ai/code/session_01XY5uCwTjZj7884yYtyur4H Co-authored-by: Claude <noreply@anthropic.com>
…sed predicate Claude-Session: https://claude.ai/code/session_01XY5uCwTjZj7884yYtyur4H Co-authored-by: Claude <noreply@anthropic.com>
…plain-cross-class-refusal
📓 Docs Drift Check9 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
Coarse fallback — 15 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 bde09535620e3aa3330d88bf71a4c88065713447 && git checkout bde09535620e3aa3330d88bf71a4c88065713447
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin e666636fd99a363d76753f5455a5708b9c9876cb 5e48f52c2464ecb8c6b1cded5103f1f3151d695a && git checkout -B drift-repro e666636fd99a363d76753f5455a5708b9c9876cb && git merge --no-ff 5e48f52c2464ecb8c6b1cded5103f1f3151d695a
node scripts/docs-audit/affected-docs.mjs --json e666636fd99a363d76753f5455a5708b9c9876cb |
Contract reviewServed-tier: PR #20598 for card #20431, ① Derived judgments
② Semver level
③ Boundary flags
Implemented-by: VERDICT: PASS Generated by Claude Code |
…usal at the object level, and explains another user in their organization (objectstack-ai#20604) (objectstack-ai#20629) Fixes objectstack-ai#20604 Clause-②: no ## What was wrong `POST /api/v1/security/explain` gave answers that disagreed with what the same principal's own request gets from enforcement. Measured on `main` at `889139ce` with the pin this PR adds (45 of 103 rows red before any fix): - **Position 1.** A row-level policy compares two fields of no shared comparison class (`record.status != record.amount`, and the other ordering `record.amount > record.status`). The find refuses every read it scopes with `INVALID_FILTER` / 400. A by-id update or delete fails closed with 403, and an insert is refused with `INVALID_FILTER` / 400. An object-level explanation (no `recordId`) answered `allowed: true`, the `rls` layer `narrows`, and the predicate as `readFilter`, for read, update, delete and create. That is 10 rows. - **Position 2.** Under the same policy, a `recordId` that no row carries was answered `visible: false` with no `decidedBy`, while the find for that id refuses with `INVALID_FILTER` / 400. REACHED. - **Position 3.** An administrator explained a CURRENT member of their organization. The explained context carried no organization: - (i) Under `isolated`, on a tenant object, the explanation answered `allowed: false`, `rls` `denies`, and the fail-closed sentinel as `readFilter`. The member's own find returns their organization's row. REACHED under `isolated` only: under `group`, the wall reads `accessible_org_ids`, which the context already carried. - (ii) A permission set authored as a `sys_permission_set` row scoped to that organization did not load for the explained user, under `isolated`, `group` and `single`. Enforcement resolves it. REACHED under all three postures. ## What changed - **`explain-engine.ts`, the object-level pass.** Before any verdict is computed, the composed row filter (the caller's, and a delegator's) is judged the way the find is judged: by the record matcher, with the object's declared columns. A comparison the spec's classification refuses now fails the explanation with the matcher's own refusal. This is the same function, envelope (`INVALID_FILTER` / 400), message and `cause` that the record-grained pass has given since PR objectstack-ai#20598. No second refusal dialect is added. The matcher judges declared columns before it reads a record, so it is asked with no row. It is asked only when `findCrossFieldClassRefusal` finds a refused comparison, so no filter the find runs is evaluated there. The refusal runs before the record-grained pass, so position 2 gets the find's answer too. A request that the capability or CRUD gate denies is still explained as denied there, as enforcement denies it (see the boundary row). The helper that names the refused policy now also walks the `$and` that puts the tenant wall next to a policy. - **`security-plugin.ts`, `explainAccessForCaller`.** The explained context's `tenantId` is set to the organization `vetOrganizationClaim` resolved the user in. That is the same value, and the same assignment, that `resolveDelegatorContext` makes for a delegator. A removed member (claim dropped) keeps no organization, as enforcement resolves them. - **One column source (A5).** A new internal module, `declared-comparison-columns.ts` (not exported from the package), holds `declaredComparisonColumns`. It is now the one reading of an object's declared columns for both the row-level write check (`writeCheckFieldOptions`) and explain. - **`@objectstack/core`, cross-lane notice for `domain:engine`.** In `packages/core/src/security/resolve-authz-context.ts`, the API-key arm of `resolveAuthzContext` changes from `if (keyPrincipal?.tenantId && input.tenancyPosture) { if (postureEnforcesWall(posture) && !grants.accessible_org_ids.includes(keyPrincipal.tenantId))` to `if (keyPrincipal?.tenantId) { if (vetOrganizationClaim(keyPrincipal.tenantId, grants.accessible_org_ids, input.tenancyPosture) === undefined)`. This is a refactor with no behaviour change. The two conditions are equal term by term: a truthy claim, a posture present, a walled posture, and the claim absent from `accessible_org_ids`. The consequence (the refusal, objectstack-ai#15256 2A) is unchanged. The `vetOrganizationClaim` docblock names the key arm as a reader, so its sentence "Nothing else spells this rule" stays true. A `git grep` for `accessible_org_ids.includes` / `accessibleOrgIds.includes` in `packages/*/src` (tests excluded) finds one spelling, inside `vetOrganizationClaim`. No export, signature or error changed. The existing key-arm cases pass unchanged. - **The objectstack-ai#20580 keep-pin, re-ruled in place** (`explain-removed-member-principal.test.ts`). "A current member's explanation is unchanged" is now "a current member's explanation matches enforcement". It asserts that the member's sets equal enforcement's resolved sets (with enforcement's `tenantId` asserted as `org_alpha`), and that `allowed` equals whether the member's own read is admitted. Nothing is deleted. - **Changeset** `.changeset/20604-explain-enforce-closeout.md`: `patch` for `@objectstack/plugin-security` and `@objectstack/core`. ## The enumeration pin (`explain-enforce-parity.test.ts`) One table, 104 rows, one invariant function (`expectParity`). For each row, the explanation and the same principal's own request run through the real `SecurityPlugin`, a real `ObjectQL`, and better-sqlite3. Where needed, the real `SharingService` and its middleware run too. Each row also asserts enforcement's own outcome. The positions are: object-level `allowed`, object-level `readFilter` (compared by the rows it admits as a system read), record-grained `visible` for read / update / delete, and the principal's permission sets. The rows are the family's shapes: objectstack-ai#19963 (write depth), objectstack-ai#19986 (read depth), objectstack-ai#20002 (a throwing sharing read filter), objectstack-ai#20431 (the record matcher under a cross-class comparison, both orderings), objectstack-ai#20580 (removed member, under `isolated` / `group` / `single`), and this card's positions 1 to 3. There is also a boundary row: under the cross-class policy, a principal with no CRUD grant gets `allowed: false`, not a refusal, because enforcement denies at the CRUD gate. It is a test, not a gate. It runs on better-sqlite3 only; the per-driver coverage stays in `explain-cross-class-refusal.test.ts`. Two measured divergences are findings of this card. They are not fixed here. Their rows assert the disagreement itself, so each row turns red the day either face moves (see Acceptance notes). ## Ablations (fix removed, then restored) All three were run with `scripts/ablation-replace.mjs` in WRAP mode, inside a driver script with `trap 'git checkout HEAD -- ABS_PATH' EXIT INT TERM`, at `6d23ba1ca`. The three source blobs are unchanged at the final head. - **A: object-level refusal deleted** (`explain-engine.ts`). Anchor 1 → 0, blob `fe299ae87b2d` → `0f870a48cdda`. 11 rows red: the 10 position-1 rows and the position-2 row. First red row: `explain answered allowed: true, rls: narrows, readFilter: {"status":{"$ne":{"$field":"amount"}}}; enforcement {"kind":"refused","code":"INVALID_FILTER","status":400}: expected 'answered' to deeply equal { code: 'INVALID_FILTER', status: 400 }`. Restore: blob == HEAD `fe299ae87b2d`, `git diff HEAD` empty. - **B: explained organization deleted** (`security-plugin.ts`). Blob `faa10fb1e8ae` → `e98f01b20caf`. 21 rows red, all position-3 rows under `isolated`, `group`, `single`, plus the A4 current-member rows. Examples: `the member user's permission sets ... enforcement {"kind":"sets","names":["member_default","qa_parity_reader","qa_parity_alpha_notes"]}: expected [ Array(2) ] to deeply equal [...]`, and `the member user, object-level read of LEDGER · object.allowed: explain answered allowed: false, rls: denies ...; enforcement {"kind":"rows","ids":["l_alpha"]}`. Restore: blob == HEAD `faa10fb1e8ae`, diff empty. - **C: core key-arm predicate replaced by `false`.** Blob `cc4b63ba7b11` → `b8fe722cd614`. 5 existing key-arm cases red, among them "refuses a key whose owner is no longer a member of its organization" and "the same key under `group` is refused too". So those cases do exercise the refactored line. Restore: blob == HEAD `cc4b63ba7b11`, diff empty. These tests import plugin-security from `src` and core's suite imports core from `src`, so no ablation leg depends on a `dist/` build. ## Verification (head `571cf85e3`) - Build: `pnpm --filter '@objectstack/plugin-security...' build`, which includes `@objectstack/core`. Exit 0. The rebuilt core `dist/index.js` carries `vetOrganizationClaim(keyPrincipal.tenantId` (count 1). - `pnpm --filter @objectstack/plugin-security --filter @objectstack/core run typecheck`: exit 0. plugin-security: test layer `0 file(s) / 0 error(s)` held in the debt ledger. core: `4 file(s) / 4 error(s)`, unchanged. - `@objectstack/plugin-security` full vitest: 147 files, 3202 passed, 23 skipped. `@objectstack/core` full vitest: 59 files, 1570 passed. - Gates: `node scripts/pm/dispatch-gates.mjs --commands --repo objectstack-ai/objectstack` derived 67 commands against this diff, the same 67 as at dispatch. 64 exited 0. 3 are NOT MEASURED: `check:dual-build-cjs-loads`, `check:i18n` and `check:type-check-debt`. Each printed `PREREQUISITE NOT MET` (exit 3), because it reads a whole-tree build and this worktree built only the plugin-security closure; CI builds first. `--ran` reconciliation: `67 derived famil(ies) accounted for — 64 run, 3 NOT-MEASURED`, `0 UNRUN`. - Lint, narrowed: `eslint --no-inline-config --format json` over the 6 changed `.ts` files returned 6 file results, 0 errors and 0 warnings. None were ignored (an ignored file reports a warning). `eslint.config.mjs` enables no type-aware linting (its own note, near line 326: "never enables type-aware linting (no `parserOptions.project` ...)"), so this diff cannot change the verdict for any untouched file. The full `pnpm lint` is CI's. ## Measured, not assumed (A4, A5) - **A4, the posture-source asymmetry: REACHED in-process, and enforcement's side.** The composition is `org-scoping` without a `tenancy` service. Admission reads no posture, so a removed member's `org_alpha` claim is kept. Their own reads return `org_alpha`'s ledger row, the global probe, and the organization-authored set's object. Meanwhile, this plugin walls Layer 0 at `isolated`, and explain vets the claim under that walled posture and drops it. The defect is enforcement's: a claim is never dropped while Layer 0 enforces a wall. Per the dispatch it is not changed here. See Acceptance notes for its reach. - **A5, the column source: no verdict difference measured.** The ObjectQL registry never returns an array-shaped `fields`. A probe that registered an object with `fields: [...]` got back a keyed map (`"0"`, `"1"`, plus the injected system fields), so the write check's array branch cannot differ from explain's reading. The write check's metadata fallback is reached only when the registry holds no field map for the object, which is an object the find cannot compile against. With the shared function, both judges read one declaration one way. The objectstack-ai#20431 suite (`explain-cross-class-refusal.test.ts`) and the table's cross-class rows stay green on it. ## Acceptance notes - **`NATIVE_SCOPING_UNDER_SINGLE` (explain's side, not fixed).** Under `single`, the engine still stamps the context's organization on a tenant object's read (driver-native tenant scoping, `engine.ts`, the `hasTenant` branch). A caller whose context carries `org_alpha` does not see `org_beta`'s row. Explain's tenant layer contributes nothing under `single`, so it reports `readFilter: null` and `record.visible: true` for the `org_beta` row. This was measured for an administrator explaining another user, after this PR's position-3 change. The same holds by construction for a caller explaining themselves, which was not measured. It needs rows of more than one organization under `single`, and no producer of that data is named here. - **`CLAIM_KEPT_UNDER_A_WALL` (enforcement's side, not changed).** It was measured in-process only. I found no public door that reaches it: without plugin-auth there is no session (no `auth` service), and no API key, because `sys_api_key` is registered by plugin-auth's manifest. So no request carries an organization claim in that composition. - **The F3 residual that remains.** The refused-policy naming is still spelled twice: explain's `refusedPolicyNamesOf` and the write check's inline log attribution. Explain's now also walks `$and`. That affects log and message attribution only. The next PR that touches the write check in `security-plugin.ts` could carry it. - objectstack-ai#20603 remains open: the REST route answers explain's refusals as 500. After this PR the object-level refusals reach that same door. ## Patch round 1 (the seat's append; the dev writes a body only once) - **The red:** the at-tier record `5888953451` FAILed on `571cf85e`. `Lint & Repo Gates` was red at the error-code casing guard, because the objectstack-ai#20002 row of the parity table spelled an absent error envelope as the string code `'undefined'`. - **The fix,** in the test file only: - `Envelope.code` and `Envelope.status` are optional, and `envelopeOf` answers `undefined` for an error that carries no envelope. - The row states `enforced: { kind: 'refused', code: undefined }`, and both halves of an envelope are compared strictly, so "no envelope" is its own value. - An explain refusal that answers an un-enveloped failure must carry no envelope itself. - The row still asserts that the find fails with the sharing service's own error and that explain answers `visible: false` for it. - No guard exemption, and no placeholder code. - The red was reproduced first at `571cf85e` (exit 1 at `:841`). `pnpm check:error-code-casing` exits 0 at `b8fe3069`. - **A second test-only commit:** the RLS and sharing rigs collect lifecycle hooks unfired, so no bootstrap read races engine teardown. No row or expected outcome changed. - **At `b8fe3069`:** all 67 derived gates exit 0, including the 3 whole-tree gates, now measured after a full build. `--ran` reconciles 67 / 0 / 0. The three ⛔-marked roster families exit 0. The `Lint & Repo Gates` steps from the casing guard onward were run locally: 76 of 77 exit 0, and the last one reports on CI's own step outcomes, so it has no local run. `plugin-security` 3202 passed, `typecheck` exit 0. - **Not added:** a sharing-rule-criterion boundary row. A rule's criterion is read only when the rule materializes, never in a reader's find, and a refused criterion query grants nothing, which fails closed. So there is no refusal for the table to hold. --- _Generated by [Claude Code](https://claude.ai/code/session_01XY5uCwTjZj7884yYtyur4H)_ --------- Co-authored-by: Claude <noreply@anthropic.com>
Fixes #20431
Clause-②: no
What was wrong
A row-level policy can compare two fields that share no comparison class, for example a text field against a number field. Enforcement refuses every request such a policy scopes. The find answers
INVALID_FILTER/ 400. A by-id update or delete fails closed at its row-level gate (403), because that gate's pre-image read is the same refused read.The record-grained explanation judged the same predicate in-process without the object's declared columns. It compared the two raw values and reported a record verdict:
visible: truefor one ordering of a pair, andvisible: false(rlsexcluded) for the other. Both answers covered a request that enforcement refuses.What changed
The landing point is
packages/plugins/plugin-security/src/explain-engine.ts, as the dispatch expected; the other files are the pin file and the changeset. There are no changes tosecurity-plugin.ts,packages/formula,packages/spec, the REST layer, or enforcement.matchesFilterCondition) now receives the object's declared columns (options.fields), as the RLS write check does. They are read fromql.getSchema(object): the schema the engine already reads for the OWD, and the ObjectQL registry that the find's driver compiles against. A schema that cannot be read hands over no columns, and the matcher judges values only, as before.INVALID_FILTER/ 400 (the matcher's code and status, the envelope the find answers with), and no record verdict is reported. The message names the policy and both fields with their declared types. The matcher's own error rides ascause.readFilter, or therlslayer'srowFilter).Why a refusal and not a fail-closed report: dispatch assumption A3 did not hold
A3 said to reuse PR #20030's shape (layer
not_evaluated,record.visible: false) for "enforcement refuses this read". Measurement onmainsays otherwise:INVALID_FILTERrefusals as a refusal.rls-stored-list-ordering-fails-closed.test.ts(landed inde091b50, PR fix(formula)!: refuse an ordering comparison whose stored operand holds a list or an object #20310) pins it: "explain read 400 = find 400; explain update 400, the by-id update 403". One of its cells is a field-to-field comparison against a list-holding field.My first commit used the report shape. The full
plugin-securitysuite then turned 2 cells of that landed pin red, because its field-to-field cell is now caught first by the comparison-class rule. Keeping the report shape would have added the second refusal dialect the dispatch forbids. So this PR follows the ruling's intent: "the read is refused … both orderings answer the same refusal as find".Measurement: before and after (better-sqlite3, the same stack as the pins)
INVALID_FILTERPERMISSION_DENIEDvisible: true,decidedBy: 'rls', rlsadmittedINVALID_FILTERINVALID_FILTERPERMISSION_DENIEDvisible: false,decidedBy: 'rls', rlsexcludedINVALID_FILTERvisible: true,decidedBy: 'rls'Tests
New file:
packages/plugins/plugin-security/src/explain-cross-class-refusal.test.ts. It uses the realSecurityPlugin,ObjectQLand SQL drivers (better-sqlite3 and sqlite-wasm; PostgreSQL whenOS_TEST_POSTGRES_URLis set), on PR #20427's harness. Every refused cell asserts both halves with their envelopecodeandstatus: explain's answer, and the caller's real request.{ code: 'INVALID_FILTER', status: 400 }and its message names the policy and both fields. Find answersINVALID_FILTER/ 400, update and delete answerPERMISSION_DENIED/ 403, and nothing is stored.{ find: INVALID, explain: INVALID }.visible: true/admittedfor it andvisible: false/excludedfor the other row. The update is admitted and matches explain.Pre-fix run:
main'sexplain-engine.tsrestored from the base blob92716c91, under a trap whose restore is proven by the HEAD blob and an emptygit diff HEAD. Result:Tests 12 failed | 2 passed | 7 skipped (21). The 2 passes are the controls.Ablation: only the declared-columns argument was removed, through
scripts/ablation-replace.mjs. The anchor hit 1 → 0 and the blob wenta46456db→5a314958. Result:Tests 12 failed | 2 passed | 7 skipped (21). Every refused cell on both drivers failed:Restore:
ok restored: blob == HEAD (a46456db6b87) and git diff HEAD is empty.All figures below were measured at
5e48f52c, the head after mergingorigin/mainc876a742:pnpm --filter @objectstack/plugin-security exec vitest run --maxWorkers=2:Test Files 144 passed (144),Tests 3066 passed | 23 skipped (3089).pnpm --filter @objectstack/plugin-security typecheck: exit 0, with the test layer OK.tsc -p tsconfig.test.json --listFilescounts the new file once.node scripts/pm/dispatch-gates.mjs --commandsderived 64 commands, and all 64 ran with exit 0. Three first answered exit 3PREREQUISITE NOT MET(check:dual-build-cjs-loads,check:i18n,check:type-check-debt). I rebuilt withturbo run build --filter='./packages/*' --filter='./packages/*/*'(71/71 tasks) and re-ran them; all three answered exit 0.dispatch-gates --ran:64 derived, 64 run, 0 NOT-MEASURED, 0 UNRUN.eslint --no-inline-config --format jsonover the two touched.tsfiles gives 2 files, 0 errors, 0 warnings.eslint --print-configshows noparserOptions.project/projectService. Linting is not type-aware, so this diff cannot move any untouched file's verdict.Acceptance notes
PERMISSION_DENIED→ 403 andOBJECT_NOT_FOUND→ 404; every other throw becomes500 EXPLAIN_FAILED. I measured it through the real handler (security-explain-envelope.test.tsharness): a service refusal carryingINVALID_FILTER/ 400 comes back as{ status: 500, error: { code: 'EXPLAIN_FAILED', message: … } }. The refusal's message survives. PR fix(formula)!: refuse an ordering comparison whose stored operand holds a list or an object #20310's refusals were already answered this way. It lives inpackages/rest/src/rest-server.ts, outside this card's surface, so it is reported, not fixed here.recordIdruns no record matcher. For a read under such a policy, it still reportsallowed: trueand rlsnarrows, where the find answers 400. This PR does not change that; it is reported separately.visible: false, nodecidedBy) for a policy the find would refuse.refusedPolicyNamesOf) copies the RLS write check's attribution insecurity-plugin.ts. That file is held by security: with no active organization, resolvePermissionSetsForContext reads the principal's position names as permission-set names organization-less, and sys_permission_set.name has no reserved-identity guard (NOT MEASURED; from PR #20540's review) #20555, so one shared helper is left to whoever next touches both files.Generated by Claude Code