Skip to content

Commit cd901d7

Browse files
fix(plugin-security,core): security/explain answers enforcement's refusal at the object level, and explains another user in their organization (#20604) (#20629)
Fixes #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 #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, #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 #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: #19963 (write depth), #19986 (read depth), #20002 (a throwing sharing read filter), #20431 (the record matcher under a cross-class comparison, both orderings), #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 #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. - #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 #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>
1 parent f1e921a commit cd901d7

7 files changed

Lines changed: 1130 additions & 44 deletions

File tree

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
---
2+
'@objectstack/plugin-security': patch
3+
'@objectstack/core': patch
4+
---
5+
6+
fix(plugin-security): `security/explain` answers enforcement's refusal at the object level too, and explains another user in the organization they are resolved in (#20604)
7+
8+
Clause-②: no
9+
10+
Two answers of `POST /api/v1/security/explain` disagreed with what the same principal's own request gets from enforcement.
11+
12+
**A row-level policy that compares two fields of no shared comparison class** (text against a number, or any field against a file field, a formula field, or a field that holds a list or an object). The SQL driver refuses to compile such a read, so the find answers `INVALID_FILTER` / 400. A by-id update or delete fails closed at its row-level gate, and an insert whose check judges the policy is refused with `INVALID_FILTER` / 400. An object-level explanation (no `recordId`) still answered `allowed: true`, the `rls` layer `narrows`, and the predicate as `readFilter`, for every operation. A `recordId` that no row carries was answered `visible: false` with no deciding layer. Both are now refused with the envelope a record-grained explanation already gives: `INVALID_FILTER` / 400, with the message that names the policy and both fields. A request that the capability gate or the CRUD grant denies is still explained as denied there.
13+
14+
**Another user explained by an administrator.** The explanation now carries the organization the user is resolved in, as enforcement's context for that user does. Before, a current member of the administrator's organization was explained with no organization. Under `isolated`, that member was reported denied on a tenant object their own find reads. Under every posture, a permission set that their organization authored (a `sys_permission_set` row scoped to that organization) was missing from the explanation and from the verdicts it decides.
15+
16+
`@objectstack/core`: the API-key arm of `resolveAuthzContext` asks `vetOrganizationClaim` for its membership rule, as the session arm does. This is a refactor with no behaviour change. A key whose owner is no longer a member of its organization is still refused.
17+
18+
Unchanged:
19+
20+
- Enforcement admits and refuses exactly what it did before.
21+
- A comparison between two fields of one class keeps its verdicts, at the object level and per record.
22+
- Explaining yourself.
23+
- A removed member's explanation (no organization, as enforcement resolves them).

‎packages/core/src/security/resolve-authz-context.ts‎

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -451,9 +451,13 @@ export async function resolveAuthzContext(input: ResolveAuthzInput): Promise<Res
451451
// Degrading would hand back exactly the `200 + total 0` silent-empty this
452452
// card exists to kill — an ex-member's automation would keep answering
453453
// success while reading nothing.
454-
if (keyPrincipal?.tenantId && input.tenancyPosture) {
455-
const posture = input.tenancyPosture;
456-
if (postureEnforcesWall(posture) && !grants.accessible_org_ids.includes(keyPrincipal.tenantId)) {
454+
//
455+
// [#20604] The membership rule is {@link vetOrganizationClaim}, the one the
456+
// session arm below asks: a walled posture, and no current membership backing
457+
// the key's organization. Only the CONSEQUENCE is this arm's own — the key is
458+
// refused (#15256 2A) where the session arm drops the claim.
459+
if (keyPrincipal?.tenantId) {
460+
if (vetOrganizationClaim(keyPrincipal.tenantId, grants.accessible_org_ids, input.tenancyPosture) === undefined) {
457461
// [#15256 / 2A] The `organization_membership_ended` decision point — AFTER
458462
// grants, because the membership set is what decides it. One line, here.
459463
warnApiKeyRefusal({
@@ -582,6 +586,10 @@ export async function resolveAuthzContext(input: ResolveAuthzInput): Promise<Res
582586
* [#20580] A second reader asks it: the permission explainer, about the user
583587
* it explains in the caller's organization. That is how `security/explain`
584588
* resolves that user in the organization enforcement would resolve them in.
589+
* [#20604] The API-key arm of {@link resolveAuthzContext} asks it too, about
590+
* the organization a key is stamped with. The rule is the same; the
591+
* consequence is that arm's own: a key IS its organization binding, so an
592+
* unbacked key is refused (#15256 2A) where a session's claim is dropped.
585593
* ⛔ Nothing else spells this rule — a caller that needs it calls this.
586594
*/
587595
export function vetOrganizationClaim(
Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
1+
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
2+
3+
import type { MatchesFilterOptions } from '@objectstack/formula';
4+
5+
/**
6+
* The declared columns of one object declaration, in the shape the record
7+
* matcher's comparison-class rule reads (`MatchesFilterOptions['fields']`, the
8+
* spec's `crossFieldComparisonVerdict` over each column's `type` and
9+
* `multiple`).
10+
*
11+
* ONE reading, used by the two judges that hand the matcher an object's
12+
* columns: the row-level write check (#20355), which judges the image a write
13+
* would store, and `security/explain` (#20431, #20604), which answers with the
14+
* refusal enforcement gives the same predicate. Where each gets the
15+
* declaration from is its own question; what the declaration SAYS about a
16+
* column is this function's, so the two cannot read one declaration two ways.
17+
*
18+
* A field map keyed by name and a list of `{ name, … }` entries read the same.
19+
* A declaration with no field map hands over no columns (`undefined`), and the
20+
* matcher then judges values only: a missing declaration never manufactures a
21+
* refusal. A column whose `type` is not a string is left out, so it is not
22+
* judged.
23+
*/
24+
export function declaredComparisonColumns(declaration: unknown): MatchesFilterOptions | undefined {
25+
const declared = (declaration as { fields?: unknown } | null | undefined)?.fields;
26+
if (!declared || typeof declared !== 'object') return undefined;
27+
const entries: Array<[string, unknown]> = Array.isArray(declared)
28+
? (declared as Array<{ name?: unknown }>).filter((f) => f?.name).map((f) => [String(f.name), f])
29+
: Object.entries(declared as Record<string, unknown>);
30+
const fields: Record<string, { type: string; multiple: boolean }> = {};
31+
for (const [name, decl] of entries) {
32+
if (!decl || typeof decl !== 'object') continue;
33+
const { type, multiple } = decl as { type?: unknown; multiple?: unknown };
34+
if (typeof type !== 'string') continue;
35+
fields[name] = { type, multiple: multiple === true };
36+
}
37+
return { fields };
38+
}

0 commit comments

Comments
 (0)