Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
49 changes: 49 additions & 0 deletions .changeset/explain-recognises-fail-closed-rls-denial.md
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,49 @@
---
"@objectstack/plugin-security": patch
---

fix(plugin-security): `explain` now reports a fail-closed RLS denial as `denies` with `allowed: false` (#13639)

**A wrong answer is corrected — read this if you consume `explain`.** For one
class of request, `explain` previously answered `decision.allowed: true` about a
request that is guaranteed to return zero rows. It now answers `false`.

**The class.** When applicable RLS policies exist but none can be compiled
against the current execution context — typically a required `current_user.*`
variable resolving to nothing, e.g. a caller with no active organization — the
compiler fails **closed** and composes plugin-security's `RLS_DENY_FILTER`
(`{ id: '__rls_deny__:…' }`), a predicate no record can satisfy. Enforcement
was always correct: the caller saw zero rows.

`explain`, however, recognised only its own `__deny_all__` sentinel, so it
reported that composition with layer verdict **`narrows`** and
`decision.allowed: **true**`. That is the diagnostic tool giving an
affirmatively wrong answer to the operator asking why a user sees nothing —
every available signal pointing away from the cause.

**What changed.** Deny recognition is now value-agnostic and routed through one
named predicate, so both the object-level `rls` verdict and the record-grained
layer attribution recognise either sentinel:

- the `rls` layer verdict flips `narrows` → `denies`, with the matching detail;
- `decision.allowed` flips `true` → `false` for this class;
- the record-grained `tenant_isolation` and `rls` layers report the fail-closed
prose instead of "record does not match" prose.

⭐ **Deliberately NOT changed — no payload moves.** `readFilter` (and a layer's
`rowFilter`) keeps reporting the predicate that was **actually composed**: a
deployment that receives `{ id: '__rls_deny__:…' }` today keeps receiving it
byte-for-byte. The documented `__deny_all__` collapse still fires for
`__deny_all__` alone, and the two sentinels are **not** merged. Rewriting the
published payload, and unifying the sentinel vocabulary, are recorded on #13639
as separate deployment-facing decisions.

**Record-level correctness did not move**, only its prose: the record-grained
`outcome`, `matchesRecord` and rule `effect` were already right, because the
sentinel excludes every real record on its own.

**If you assert on `explain` output**, expectations that encoded the old answer
for a fail-closed RLS denial — verdict `narrows`, or `allowed: true` — now fail,
and they were asserting the defect. Enforcement behaviour is unchanged in every
respect; `explain` is a diagnostic surface and no enforcement path reads its
verdict.
212 changes: 205 additions & 7 deletions packages/plugins/plugin-security/src/explain-engine.test.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -6,6 +6,7 @@ import { resolveUserAuthzGrants, resetPlatformAdminEmailMemo } from '@objectstac
import { PermissionSetSchema } from '@objectstack/spec/security';
import { PermissionEvaluator } from './permission-evaluator';
import { explainAccess, buildContextForUser, type ExplainEngineDeps } from './explain-engine';
import { RLS_DENY_FILTER } from './rls-compiler';
import { assertEngineFindOnePredicate, type EngineFindOneQueryInput } from '@objectstack/metadata-core';

// [#13176] `ExplainDecision.layers` is `ExplainLayer[]` — the z.INPUT shape
Expand All@@ -28,6 +29,21 @@ const ADMIN = PermissionSetSchema.parse({

const PRIVATE_SCHEMA = { name: 'leave_request', sharingModel: 'private' };

// The ONE engine double this file declares: a `ql` whose `findOne` resolves the
// `onBehalfOf` delegator, so D10 resolves instead of failing closed as
// 'missing' (which is a different report entirely). Shared by every test that
// needs a resolvable delegator — a second inline copy would be a second double
// for `check:engine-double-contract` to ratchet, and reusing the one the file
// already pins is cheaper than growing that ledger.
const DELEGATOR_QL = {
getSchema: () => PRIVATE_SCHEMA,
findOne: async (object: string, query?: EngineFindOneQueryInput) => {
assertEngineFindOnePredicate(object, query);
return { id: 'u_boss' };
},
find: async () => [],
};

function makeDeps(overrides: Partial<ExplainEngineDeps> & { sets?: any[]; schema?: any; rls?: any } = {}): ExplainEngineDeps {
const evaluator = new PermissionEvaluator();
return {
Expand DownExpand Up@@ -346,13 +362,7 @@ describe('explainAccess — fls reports partial masking (#9127)', () => {
it('keeps the D10 delegator suffix when only partial masks apply', async () => {
const d = await explainAccess(
makeDeps({
// A resolvable delegator — otherwise D10 fails closed as 'missing' and
// `delegatorSets` stays null, which is a different report entirely.
ql: {
getSchema: () => PRIVATE_SCHEMA,
findOne: async (object: string, query?: EngineFindOneQueryInput) => { assertEngineFindOnePredicate(object, query); return ({ id: 'u_boss' }); },
find: async () => [],
},
ql: DELEGATOR_QL, // resolvable delegator — otherwise D10 fails closed
getPartialMaskRules: async () => ({ phone: 'phone' }),
}),
{
Expand DownExpand Up@@ -1259,3 +1269,191 @@ describe('buildContextForUser bypasses the #11971 grants cache (ruled bypass lis
}
});
});


// ═══════════════════════════════════════════════════════════════════════════
// [#13639] A fail-closed RLS denial is reported as a DENIAL.
//
// Before this suite's repair, `explain` recognised only its own `__deny_all__`
// sentinel, so the "no active organization" path — which composes
// plugin-security's `RLS_DENY_FILTER` (`__rls_deny__:…`) and is guaranteed to
// return zero rows — was reported with verdict `narrows` and
// `decision.allowed: TRUE`. Not an imprecise label: an affirmatively wrong
// answer, handed to the operator asking why a user sees nothing.
//
// ⛔ The repair is RECOGNITION ONLY. The sentinels are not merged and the
// published `readFilter` payload is NOT rewritten — options B and C on #13639
// are the maintainer's, and the boundary between A and B is PINNED below, not
// left to a comment.
// ═══════════════════════════════════════════════════════════════════════════
describe('explainAccess — fail-closed RLS denial (#13639)', () => {
// [ADR-0123 D2] Producers SPREAD the frozen constant rather than returning
// it, which is why identity is decided on the sentinel's VALUE. Every
// fixture here spreads it too, so a recognition that compared REFERENCES
// would pass nothing.
const rlsDeny = (): Record<string, unknown> => ({ ...RLS_DENY_FILTER });
const readOf = async (deps: ExplainEngineDeps) =>
explainAccess(deps, { object: 'leave_request', operation: 'read', context: CTX });

// `DELEGATOR_QL` resolves the delegator, so BOTH computeRlsFilter calls happen
// and `readFilter` is a real two-part composite — the only shape in which the
// payload collapse at §9 is observable at all.
const composedOf = async (delegatorFilter: Record<string, unknown>) =>
explainAccess(
makeDeps({
ql: DELEGATOR_QL,
computeRlsFilter: async (_s: any, _o: string, _op: string, ctx: any) =>
ctx?.userId === 'u_boss' ? delegatorFilter : { owner_id: 'u1' },
}),
{ object: 'leave_request', operation: 'read', context: { ...CTX, onBehalfOf: { userId: 'u_boss' } } },
);

// ── the object-level `rls` verdict ──────────────────────────────────────

it('reports the rls layer as `denies`, not `narrows`', async () => {
const rls = (await readOf(makeDeps({ rls: rlsDeny() }))).layers.find((l) => l.layer === 'rls')!;
expect(rls.verdict).toBe('denies');
expect(rls.detail).toContain('DENY ALL');
expect(rls.detail).not.toContain('narrows the row set');
});

// ── ⭐ the assertion that matters most ──────────────────────────────────

it('⭐ answers decision.allowed = FALSE for a request that cannot return a row', async () => {
expect((await readOf(makeDeps({ rls: rlsDeny() }))).allowed).toBe(false);
});

it('decides identity on the sentinel VALUE, not on reference to the frozen constant', async () => {
const spread = rlsDeny();
expect(spread).not.toBe(RLS_DENY_FILTER); // control: the fixture really is a copy
expect(spread).toEqual({ id: RLS_DENY_FILTER.id });
expect((await readOf(makeDeps({ rls: spread }))).allowed).toBe(false);
});

// ── ⭐ THE A/B BOUNDARY — the payload is reported AS COMPOSED ───────────

it('⭐ does NOT rewrite readFilter: the composed __rls_deny__ predicate is published unchanged', async () => {
const d = await readOf(makeDeps({ rls: rlsDeny() }));
expect(d.readFilter).toEqual({ id: RLS_DENY_FILTER.id });
// ⛔ Option B — rewriting the payload for every deployment whose RLS fails
// closed — is explicitly NOT what happened here.
expect(d.readFilter).not.toEqual({ id: '__deny_all__' });
});

it('⭐ leaves a two-part composite INTACT when the DELEGATOR half denies — no collapse', async () => {
const d = await composedOf(rlsDeny());
expect(d.allowed).toBe(false);
expect(d.layers.find((l) => l.layer === 'rls')!.verdict).toBe('denies');
expect(d.readFilter).toEqual({ $and: [{ owner_id: 'u1' }, { id: RLS_DENY_FILTER.id }] });
});

// ── `__deny_all__` is unchanged in EVERY respect, collapse included ─────

it('leaves the __deny_all__ verdict and decision exactly as they were', async () => {
const d = await readOf(makeDeps({ rls: { id: '__deny_all__' } }));
expect(d.allowed).toBe(false);
const rls = d.layers.find((l) => l.layer === 'rls')!;
expect(rls.verdict).toBe('denies');
expect(rls.detail).toBe('Row-level security composes to DENY ALL for this principal.');
expect(d.readFilter).toEqual({ id: '__deny_all__' });
});

it('⭐ still COLLAPSES a composite to { id: "__deny_all__" } for THAT sentinel', async () => {
const d = await composedOf({ id: '__deny_all__' });
expect(d.allowed).toBe(false);
// The documented payload rewrite survives untouched for the sentinel the
// published contract names.
expect(d.readFilter).toEqual({ id: '__deny_all__' });
});

// ── the negative ───────────────────────────────────────────────────────

it('a policy that genuinely NARROWS still narrows, and stays allowed', async () => {
const d = await readOf(makeDeps({ rls: { owner_id: 'u1' } }));
expect(d.allowed).toBe(true);
const rls = d.layers.find((l) => l.layer === 'rls')!;
expect(rls.verdict).toBe('narrows');
expect(d.readFilter).toEqual({ owner_id: 'u1' });
});

it('an id predicate that is NOT a sentinel is a narrowing, not a denial', async () => {
const d = await readOf(makeDeps({ rls: { id: 'r1' } }));
expect(d.allowed).toBe(true);
expect(d.layers.find((l) => l.layer === 'rls')!.verdict).toBe('narrows');
});

it('no RLS policy at all is still not_applicable and allowed', async () => {
const d = await readOf(makeDeps({ rls: null }));
expect(d.allowed).toBe(true);
expect(d.layers.find((l) => l.layer === 'rls')!.verdict).toBe('not_applicable');
});

// ── record-grained layers: the CORRECTNESS must not move, the prose does ─
//
// Slice 1 measured that `outcome` and `matchesRecord` are ALREADY right at
// both `isDenyAll` call sites today, because `matchesFilterCondition` excludes
// every real record against either sentinel. So the two groups below are
// asserted for opposite reasons: the correctness assertions are CONTROLS
// (green with and without the repair), the `detail` assertions are the ones
// the repair moves.
describe('record-grained attribution (the isDenyAll call sites)', () => {
const REC_CTX = { userId: 'u1', tenantId: 'org1', positions: ['sales_rep', 'everyone'], permissions: [] };
const recOf = async (layered: { layer0: any; layer1: any }) =>
explainAccess(
{
...makeDeps({ rls: null }),
computeLayeredRlsFilter: async () => layered,
fetchRecord: async () => ({ id: 'r1', organization_id: 'org1', owner_id: 'u1' }),
sharingReadFilter: async () => null,
listRecordShares: async () => [],
canEditRecord: async () => false,
} as ExplainEngineDeps,
{ object: 'leave_request', operation: 'read', context: REC_CTX, recordId: 'r1' },
);

it('layer 0: outcome/matchesRecord/effect do NOT move — only the prose does', async () => {
const l0 = (await recOf({ layer0: rlsDeny(), layer1: null })).layers.find((l) => l.layer === 'tenant_isolation')!;
// CONTROL — already correct before the repair, must stay correct after.
expect(l0.verdict).toBe('denies');
expect(l0.record!.outcome).toBe('excluded');
expect(l0.record!.matchesRecord).toBe(false);
expect(l0.record!.rules?.[0]?.effect).toBe('excludes');
expect(l0.record!.rowFilter).toEqual({ id: RLS_DENY_FILTER.id }); // payload untouched here too
// THE PROSE — this is what the repair moves.
expect(l0.record!.detail).toBe(
'No active organization on the context — the tenant wall denies all rows (fail closed).',
);
expect(l0.record!.detail).not.toContain('does not match');
});

it('layer 1: outcome/matchesRecord/effect do NOT move — only the prose does', async () => {
const l1 = (await recOf({ layer0: { organization_id: 'org1' }, layer1: rlsDeny() })).layers.find((l) => l.layer === 'rls')!;
// CONTROL — already correct before the repair.
expect(l1.record!.outcome).toBe('excluded');
expect(l1.record!.matchesRecord).toBe(false);
expect(l1.record!.rules?.[0]?.effect).toBe('excludes');
expect(l1.record!.rowFilter).toEqual({ id: RLS_DENY_FILTER.id });
// THE PROSE.
expect(l1.record!.detail).toBe('Business RLS composes to DENY ALL for this principal.');
expect(l1.record!.detail).not.toContain('does not satisfy');
});

it('a record the layers genuinely admit is still admitted (negative)', async () => {
const d = await recOf({ layer0: { organization_id: 'org1' }, layer1: { owner_id: 'u1' } });
const l0 = d.layers.find((l) => l.layer === 'tenant_isolation')!;
expect(l0.record!.outcome).toBe('admitted');
expect(l0.record!.matchesRecord).toBe(true);
expect(d.layers.find((l) => l.layer === 'rls')!.record!.outcome).toBe('admitted');
});

it('the __deny_all__ prose at both call sites is unchanged (control)', async () => {
const d = await recOf({ layer0: { id: '__deny_all__' }, layer1: { id: '__deny_all__' } });
expect(d.layers.find((l) => l.layer === 'tenant_isolation')!.record!.detail).toBe(
'No active organization on the context — the tenant wall denies all rows (fail closed).',
);
expect(d.layers.find((l) => l.layer === 'rls')!.record!.detail).toBe(
'Business RLS composes to DENY ALL for this principal.',
);
});
});
});
Loading
Loading
, 'i'); if (__m === '*' || __re.test(location.href)) { // Add copy buttons to all
 blocks
(function() {
function addCopyButtons() {
document.querySelectorAll('pre code').forEach(function(codeBlock) {
if (codeBlock.parentElement.hasAttribute('data-copy-added')) return;
codeBlock.parentElement.setAttribute('data-copy-added', 'true');
var btn = document.createElement('button');
btn.textContent = 'Copy';
btn.style.cssText = 'position:absolute;top:4px;right:4px;padding:2px 8px;font-size:11px;background:#4ecdc4;border:none;border-radius:4px;color:#1a1a2e;cursor:pointer;opacity:0.7;transition:opacity 0.2s;';
btn.onmouseover = function() { this.style.opacity = '1'; };
btn.onmouseout = function() { this.style.opacity = '0.7'; };
btn.onclick = function() {
navigator.clipboard.writeText(codeBlock.textContent).then(function() {
btn.textContent = 'Copied!';
setTimeout(function() { btn.textContent = 'Copy'; }, 1500);
});
};
codeBlock.parentElement.style.position = 'relative';
codeBlock.parentElement.appendChild(btn);
});
}
addCopyButtons();
// Re-run on dynamic content
var observer = new MutationObserver(addCopyButtons);
observer.observe(document.body, { childList: true, subtree: true });
})();
}
} catch(__e) { console.warn('[Userscript:Add Copy Buttons to Code Blocks]', __e); }
})();
(function(){
try {
var __m = "github.com";
var __re = new RegExp('^' + "github\\.com" + '
fix(plugin-security): explain reports a fail-closed RLS denial as denies, not narrows by claude[bot] · Pull Request #13960 · objectstack-ai/objectstack · GitHub
Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
49 changes: 49 additions & 0 deletions .changeset/explain-recognises-fail-closed-rls-denial.md
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,49 @@
---
"@objectstack/plugin-security": patch
---

fix(plugin-security): `explain` now reports a fail-closed RLS denial as `denies` with `allowed: false` (#13639)

**A wrong answer is corrected — read this if you consume `explain`.** For one
class of request, `explain` previously answered `decision.allowed: true` about a
request that is guaranteed to return zero rows. It now answers `false`.

**The class.** When applicable RLS policies exist but none can be compiled
against the current execution context — typically a required `current_user.*`
variable resolving to nothing, e.g. a caller with no active organization — the
compiler fails **closed** and composes plugin-security's `RLS_DENY_FILTER`
(`{ id: '__rls_deny__:…' }`), a predicate no record can satisfy. Enforcement
was always correct: the caller saw zero rows.

`explain`, however, recognised only its own `__deny_all__` sentinel, so it
reported that composition with layer verdict **`narrows`** and
`decision.allowed: **true**`. That is the diagnostic tool giving an
affirmatively wrong answer to the operator asking why a user sees nothing —
every available signal pointing away from the cause.

**What changed.** Deny recognition is now value-agnostic and routed through one
named predicate, so both the object-level `rls` verdict and the record-grained
layer attribution recognise either sentinel:

- the `rls` layer verdict flips `narrows` → `denies`, with the matching detail;
- `decision.allowed` flips `true` → `false` for this class;
- the record-grained `tenant_isolation` and `rls` layers report the fail-closed
prose instead of "record does not match" prose.

⭐ **Deliberately NOT changed — no payload moves.** `readFilter` (and a layer's
`rowFilter`) keeps reporting the predicate that was **actually composed**: a
deployment that receives `{ id: '__rls_deny__:…' }` today keeps receiving it
byte-for-byte. The documented `__deny_all__` collapse still fires for
`__deny_all__` alone, and the two sentinels are **not** merged. Rewriting the
published payload, and unifying the sentinel vocabulary, are recorded on #13639
as separate deployment-facing decisions.

**Record-level correctness did not move**, only its prose: the record-grained
`outcome`, `matchesRecord` and rule `effect` were already right, because the
sentinel excludes every real record on its own.

**If you assert on `explain` output**, expectations that encoded the old answer
for a fail-closed RLS denial — verdict `narrows`, or `allowed: true` — now fail,
and they were asserting the defect. Enforcement behaviour is unchanged in every
respect; `explain` is a diagnostic surface and no enforcement path reads its
verdict.
212 changes: 205 additions & 7 deletions packages/plugins/plugin-security/src/explain-engine.test.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -6,6 +6,7 @@ import { resolveUserAuthzGrants, resetPlatformAdminEmailMemo } from '@objectstac
import { PermissionSetSchema } from '@objectstack/spec/security';
import { PermissionEvaluator } from './permission-evaluator';
import { explainAccess, buildContextForUser, type ExplainEngineDeps } from './explain-engine';
import { RLS_DENY_FILTER } from './rls-compiler';
import { assertEngineFindOnePredicate, type EngineFindOneQueryInput } from '@objectstack/metadata-core';

// [#13176] `ExplainDecision.layers` is `ExplainLayer[]` — the z.INPUT shape
Expand All@@ -28,6 +29,21 @@ const ADMIN = PermissionSetSchema.parse({

const PRIVATE_SCHEMA = { name: 'leave_request', sharingModel: 'private' };

// The ONE engine double this file declares: a `ql` whose `findOne` resolves the
// `onBehalfOf` delegator, so D10 resolves instead of failing closed as
// 'missing' (which is a different report entirely). Shared by every test that
// needs a resolvable delegator — a second inline copy would be a second double
// for `check:engine-double-contract` to ratchet, and reusing the one the file
// already pins is cheaper than growing that ledger.
const DELEGATOR_QL = {
getSchema: () => PRIVATE_SCHEMA,
findOne: async (object: string, query?: EngineFindOneQueryInput) => {
assertEngineFindOnePredicate(object, query);
return { id: 'u_boss' };
},
find: async () => [],
};

function makeDeps(overrides: Partial<ExplainEngineDeps> & { sets?: any[]; schema?: any; rls?: any } = {}): ExplainEngineDeps {
const evaluator = new PermissionEvaluator();
return {
Expand DownExpand Up@@ -346,13 +362,7 @@ describe('explainAccess — fls reports partial masking (#9127)', () => {
it('keeps the D10 delegator suffix when only partial masks apply', async () => {
const d = await explainAccess(
makeDeps({
// A resolvable delegator — otherwise D10 fails closed as 'missing' and
// `delegatorSets` stays null, which is a different report entirely.
ql: {
getSchema: () => PRIVATE_SCHEMA,
findOne: async (object: string, query?: EngineFindOneQueryInput) => { assertEngineFindOnePredicate(object, query); return ({ id: 'u_boss' }); },
find: async () => [],
},
ql: DELEGATOR_QL, // resolvable delegator — otherwise D10 fails closed
getPartialMaskRules: async () => ({ phone: 'phone' }),
}),
{
Expand DownExpand Up@@ -1259,3 +1269,191 @@ describe('buildContextForUser bypasses the #11971 grants cache (ruled bypass lis
}
});
});


// ═══════════════════════════════════════════════════════════════════════════
// [#13639] A fail-closed RLS denial is reported as a DENIAL.
//
// Before this suite's repair, `explain` recognised only its own `__deny_all__`
// sentinel, so the "no active organization" path — which composes
// plugin-security's `RLS_DENY_FILTER` (`__rls_deny__:…`) and is guaranteed to
// return zero rows — was reported with verdict `narrows` and
// `decision.allowed: TRUE`. Not an imprecise label: an affirmatively wrong
// answer, handed to the operator asking why a user sees nothing.
//
// ⛔ The repair is RECOGNITION ONLY. The sentinels are not merged and the
// published `readFilter` payload is NOT rewritten — options B and C on #13639
// are the maintainer's, and the boundary between A and B is PINNED below, not
// left to a comment.
// ═══════════════════════════════════════════════════════════════════════════
describe('explainAccess — fail-closed RLS denial (#13639)', () => {
// [ADR-0123 D2] Producers SPREAD the frozen constant rather than returning
// it, which is why identity is decided on the sentinel's VALUE. Every
// fixture here spreads it too, so a recognition that compared REFERENCES
// would pass nothing.
const rlsDeny = (): Record<string, unknown> => ({ ...RLS_DENY_FILTER });
const readOf = async (deps: ExplainEngineDeps) =>
explainAccess(deps, { object: 'leave_request', operation: 'read', context: CTX });

// `DELEGATOR_QL` resolves the delegator, so BOTH computeRlsFilter calls happen
// and `readFilter` is a real two-part composite — the only shape in which the
// payload collapse at §9 is observable at all.
const composedOf = async (delegatorFilter: Record<string, unknown>) =>
explainAccess(
makeDeps({
ql: DELEGATOR_QL,
computeRlsFilter: async (_s: any, _o: string, _op: string, ctx: any) =>
ctx?.userId === 'u_boss' ? delegatorFilter : { owner_id: 'u1' },
}),
{ object: 'leave_request', operation: 'read', context: { ...CTX, onBehalfOf: { userId: 'u_boss' } } },
);

// ── the object-level `rls` verdict ──────────────────────────────────────

it('reports the rls layer as `denies`, not `narrows`', async () => {
const rls = (await readOf(makeDeps({ rls: rlsDeny() }))).layers.find((l) => l.layer === 'rls')!;
expect(rls.verdict).toBe('denies');
expect(rls.detail).toContain('DENY ALL');
expect(rls.detail).not.toContain('narrows the row set');
});

// ── ⭐ the assertion that matters most ──────────────────────────────────

it('⭐ answers decision.allowed = FALSE for a request that cannot return a row', async () => {
expect((await readOf(makeDeps({ rls: rlsDeny() }))).allowed).toBe(false);
});

it('decides identity on the sentinel VALUE, not on reference to the frozen constant', async () => {
const spread = rlsDeny();
expect(spread).not.toBe(RLS_DENY_FILTER); // control: the fixture really is a copy
expect(spread).toEqual({ id: RLS_DENY_FILTER.id });
expect((await readOf(makeDeps({ rls: spread }))).allowed).toBe(false);
});

// ── ⭐ THE A/B BOUNDARY — the payload is reported AS COMPOSED ───────────

it('⭐ does NOT rewrite readFilter: the composed __rls_deny__ predicate is published unchanged', async () => {
const d = await readOf(makeDeps({ rls: rlsDeny() }));
expect(d.readFilter).toEqual({ id: RLS_DENY_FILTER.id });
// ⛔ Option B — rewriting the payload for every deployment whose RLS fails
// closed — is explicitly NOT what happened here.
expect(d.readFilter).not.toEqual({ id: '__deny_all__' });
});

it('⭐ leaves a two-part composite INTACT when the DELEGATOR half denies — no collapse', async () => {
const d = await composedOf(rlsDeny());
expect(d.allowed).toBe(false);
expect(d.layers.find((l) => l.layer === 'rls')!.verdict).toBe('denies');
expect(d.readFilter).toEqual({ $and: [{ owner_id: 'u1' }, { id: RLS_DENY_FILTER.id }] });
});

// ── `__deny_all__` is unchanged in EVERY respect, collapse included ─────

it('leaves the __deny_all__ verdict and decision exactly as they were', async () => {
const d = await readOf(makeDeps({ rls: { id: '__deny_all__' } }));
expect(d.allowed).toBe(false);
const rls = d.layers.find((l) => l.layer === 'rls')!;
expect(rls.verdict).toBe('denies');
expect(rls.detail).toBe('Row-level security composes to DENY ALL for this principal.');
expect(d.readFilter).toEqual({ id: '__deny_all__' });
});

it('⭐ still COLLAPSES a composite to { id: "__deny_all__" } for THAT sentinel', async () => {
const d = await composedOf({ id: '__deny_all__' });
expect(d.allowed).toBe(false);
// The documented payload rewrite survives untouched for the sentinel the
// published contract names.
expect(d.readFilter).toEqual({ id: '__deny_all__' });
});

// ── the negative ───────────────────────────────────────────────────────

it('a policy that genuinely NARROWS still narrows, and stays allowed', async () => {
const d = await readOf(makeDeps({ rls: { owner_id: 'u1' } }));
expect(d.allowed).toBe(true);
const rls = d.layers.find((l) => l.layer === 'rls')!;
expect(rls.verdict).toBe('narrows');
expect(d.readFilter).toEqual({ owner_id: 'u1' });
});

it('an id predicate that is NOT a sentinel is a narrowing, not a denial', async () => {
const d = await readOf(makeDeps({ rls: { id: 'r1' } }));
expect(d.allowed).toBe(true);
expect(d.layers.find((l) => l.layer === 'rls')!.verdict).toBe('narrows');
});

it('no RLS policy at all is still not_applicable and allowed', async () => {
const d = await readOf(makeDeps({ rls: null }));
expect(d.allowed).toBe(true);
expect(d.layers.find((l) => l.layer === 'rls')!.verdict).toBe('not_applicable');
});

// ── record-grained layers: the CORRECTNESS must not move, the prose does ─
//
// Slice 1 measured that `outcome` and `matchesRecord` are ALREADY right at
// both `isDenyAll` call sites today, because `matchesFilterCondition` excludes
// every real record against either sentinel. So the two groups below are
// asserted for opposite reasons: the correctness assertions are CONTROLS
// (green with and without the repair), the `detail` assertions are the ones
// the repair moves.
describe('record-grained attribution (the isDenyAll call sites)', () => {
const REC_CTX = { userId: 'u1', tenantId: 'org1', positions: ['sales_rep', 'everyone'], permissions: [] };
const recOf = async (layered: { layer0: any; layer1: any }) =>
explainAccess(
{
...makeDeps({ rls: null }),
computeLayeredRlsFilter: async () => layered,
fetchRecord: async () => ({ id: 'r1', organization_id: 'org1', owner_id: 'u1' }),
sharingReadFilter: async () => null,
listRecordShares: async () => [],
canEditRecord: async () => false,
} as ExplainEngineDeps,
{ object: 'leave_request', operation: 'read', context: REC_CTX, recordId: 'r1' },
);

it('layer 0: outcome/matchesRecord/effect do NOT move — only the prose does', async () => {
const l0 = (await recOf({ layer0: rlsDeny(), layer1: null })).layers.find((l) => l.layer === 'tenant_isolation')!;
// CONTROL — already correct before the repair, must stay correct after.
expect(l0.verdict).toBe('denies');
expect(l0.record!.outcome).toBe('excluded');
expect(l0.record!.matchesRecord).toBe(false);
expect(l0.record!.rules?.[0]?.effect).toBe('excludes');
expect(l0.record!.rowFilter).toEqual({ id: RLS_DENY_FILTER.id }); // payload untouched here too
// THE PROSE — this is what the repair moves.
expect(l0.record!.detail).toBe(
'No active organization on the context — the tenant wall denies all rows (fail closed).',
);
expect(l0.record!.detail).not.toContain('does not match');
});

it('layer 1: outcome/matchesRecord/effect do NOT move — only the prose does', async () => {
const l1 = (await recOf({ layer0: { organization_id: 'org1' }, layer1: rlsDeny() })).layers.find((l) => l.layer === 'rls')!;
// CONTROL — already correct before the repair.
expect(l1.record!.outcome).toBe('excluded');
expect(l1.record!.matchesRecord).toBe(false);
expect(l1.record!.rules?.[0]?.effect).toBe('excludes');
expect(l1.record!.rowFilter).toEqual({ id: RLS_DENY_FILTER.id });
// THE PROSE.
expect(l1.record!.detail).toBe('Business RLS composes to DENY ALL for this principal.');
expect(l1.record!.detail).not.toContain('does not satisfy');
});

it('a record the layers genuinely admit is still admitted (negative)', async () => {
const d = await recOf({ layer0: { organization_id: 'org1' }, layer1: { owner_id: 'u1' } });
const l0 = d.layers.find((l) => l.layer === 'tenant_isolation')!;
expect(l0.record!.outcome).toBe('admitted');
expect(l0.record!.matchesRecord).toBe(true);
expect(d.layers.find((l) => l.layer === 'rls')!.record!.outcome).toBe('admitted');
});

it('the __deny_all__ prose at both call sites is unchanged (control)', async () => {
const d = await recOf({ layer0: { id: '__deny_all__' }, layer1: { id: '__deny_all__' } });
expect(d.layers.find((l) => l.layer === 'tenant_isolation')!.record!.detail).toBe(
'No active organization on the context — the tenant wall denies all rows (fail closed).',
);
expect(d.layers.find((l) => l.layer === 'rls')!.record!.detail).toBe(
'Business RLS composes to DENY ALL for this principal.',
);
});
});
});
Loading
Loading
, 'i'); if (__m === '*' || __re.test(location.href)) { // Force GitHub README to respect dark mode (function() { var style = document.createElement('style'); style.textContent = ' .markdown-body { color-scheme: dark light; } .markdown-body pre { background: #161b22 !important; } .markdown-body code { background: rgba(110, 118, 129, 0.4) !important; } .markdown-body table th, .markdown-body table td { border-color: #30363d !important; } .markdown-body img { background: #0d1117; } .markdown-body blockquote { border-left-color: #8b949e; } .markdown-body hr { border-color: #30363d; } '; document.head.appendChild(style); })(); } } catch(__e) { console.warn('[Userscript:GitHub Dark Mode README Fix]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + ' fix(plugin-security): explain reports a fail-closed RLS denial as denies, not narrows by claude[bot] · Pull Request #13960 · objectstack-ai/objectstack · GitHub
Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
49 changes: 49 additions & 0 deletions .changeset/explain-recognises-fail-closed-rls-denial.md
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,49 @@
---
"@objectstack/plugin-security": patch
---

fix(plugin-security): `explain` now reports a fail-closed RLS denial as `denies` with `allowed: false` (#13639)

**A wrong answer is corrected — read this if you consume `explain`.** For one
class of request, `explain` previously answered `decision.allowed: true` about a
request that is guaranteed to return zero rows. It now answers `false`.

**The class.** When applicable RLS policies exist but none can be compiled
against the current execution context — typically a required `current_user.*`
variable resolving to nothing, e.g. a caller with no active organization — the
compiler fails **closed** and composes plugin-security's `RLS_DENY_FILTER`
(`{ id: '__rls_deny__:…' }`), a predicate no record can satisfy. Enforcement
was always correct: the caller saw zero rows.

`explain`, however, recognised only its own `__deny_all__` sentinel, so it
reported that composition with layer verdict **`narrows`** and
`decision.allowed: **true**`. That is the diagnostic tool giving an
affirmatively wrong answer to the operator asking why a user sees nothing —
every available signal pointing away from the cause.

**What changed.** Deny recognition is now value-agnostic and routed through one
named predicate, so both the object-level `rls` verdict and the record-grained
layer attribution recognise either sentinel:

- the `rls` layer verdict flips `narrows` → `denies`, with the matching detail;
- `decision.allowed` flips `true` → `false` for this class;
- the record-grained `tenant_isolation` and `rls` layers report the fail-closed
prose instead of "record does not match" prose.

⭐ **Deliberately NOT changed — no payload moves.** `readFilter` (and a layer's
`rowFilter`) keeps reporting the predicate that was **actually composed**: a
deployment that receives `{ id: '__rls_deny__:…' }` today keeps receiving it
byte-for-byte. The documented `__deny_all__` collapse still fires for
`__deny_all__` alone, and the two sentinels are **not** merged. Rewriting the
published payload, and unifying the sentinel vocabulary, are recorded on #13639
as separate deployment-facing decisions.

**Record-level correctness did not move**, only its prose: the record-grained
`outcome`, `matchesRecord` and rule `effect` were already right, because the
sentinel excludes every real record on its own.

**If you assert on `explain` output**, expectations that encoded the old answer
for a fail-closed RLS denial — verdict `narrows`, or `allowed: true` — now fail,
and they were asserting the defect. Enforcement behaviour is unchanged in every
respect; `explain` is a diagnostic surface and no enforcement path reads its
verdict.
212 changes: 205 additions & 7 deletions packages/plugins/plugin-security/src/explain-engine.test.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -6,6 +6,7 @@ import { resolveUserAuthzGrants, resetPlatformAdminEmailMemo } from '@objectstac
import { PermissionSetSchema } from '@objectstack/spec/security';
import { PermissionEvaluator } from './permission-evaluator';
import { explainAccess, buildContextForUser, type ExplainEngineDeps } from './explain-engine';
import { RLS_DENY_FILTER } from './rls-compiler';
import { assertEngineFindOnePredicate, type EngineFindOneQueryInput } from '@objectstack/metadata-core';

// [#13176] `ExplainDecision.layers` is `ExplainLayer[]` — the z.INPUT shape
Expand All@@ -28,6 +29,21 @@ const ADMIN = PermissionSetSchema.parse({

const PRIVATE_SCHEMA = { name: 'leave_request', sharingModel: 'private' };

// The ONE engine double this file declares: a `ql` whose `findOne` resolves the
// `onBehalfOf` delegator, so D10 resolves instead of failing closed as
// 'missing' (which is a different report entirely). Shared by every test that
// needs a resolvable delegator — a second inline copy would be a second double
// for `check:engine-double-contract` to ratchet, and reusing the one the file
// already pins is cheaper than growing that ledger.
const DELEGATOR_QL = {
getSchema: () => PRIVATE_SCHEMA,
findOne: async (object: string, query?: EngineFindOneQueryInput) => {
assertEngineFindOnePredicate(object, query);
return { id: 'u_boss' };
},
find: async () => [],
};

function makeDeps(overrides: Partial<ExplainEngineDeps> & { sets?: any[]; schema?: any; rls?: any } = {}): ExplainEngineDeps {
const evaluator = new PermissionEvaluator();
return {
Expand DownExpand Up@@ -346,13 +362,7 @@ describe('explainAccess — fls reports partial masking (#9127)', () => {
it('keeps the D10 delegator suffix when only partial masks apply', async () => {
const d = await explainAccess(
makeDeps({
// A resolvable delegator — otherwise D10 fails closed as 'missing' and
// `delegatorSets` stays null, which is a different report entirely.
ql: {
getSchema: () => PRIVATE_SCHEMA,
findOne: async (object: string, query?: EngineFindOneQueryInput) => { assertEngineFindOnePredicate(object, query); return ({ id: 'u_boss' }); },
find: async () => [],
},
ql: DELEGATOR_QL, // resolvable delegator — otherwise D10 fails closed
getPartialMaskRules: async () => ({ phone: 'phone' }),
}),
{
Expand DownExpand Up@@ -1259,3 +1269,191 @@ describe('buildContextForUser bypasses the #11971 grants cache (ruled bypass lis
}
});
});


// ═══════════════════════════════════════════════════════════════════════════
// [#13639] A fail-closed RLS denial is reported as a DENIAL.
//
// Before this suite's repair, `explain` recognised only its own `__deny_all__`
// sentinel, so the "no active organization" path — which composes
// plugin-security's `RLS_DENY_FILTER` (`__rls_deny__:…`) and is guaranteed to
// return zero rows — was reported with verdict `narrows` and
// `decision.allowed: TRUE`. Not an imprecise label: an affirmatively wrong
// answer, handed to the operator asking why a user sees nothing.
//
// ⛔ The repair is RECOGNITION ONLY. The sentinels are not merged and the
// published `readFilter` payload is NOT rewritten — options B and C on #13639
// are the maintainer's, and the boundary between A and B is PINNED below, not
// left to a comment.
// ═══════════════════════════════════════════════════════════════════════════
describe('explainAccess — fail-closed RLS denial (#13639)', () => {
// [ADR-0123 D2] Producers SPREAD the frozen constant rather than returning
// it, which is why identity is decided on the sentinel's VALUE. Every
// fixture here spreads it too, so a recognition that compared REFERENCES
// would pass nothing.
const rlsDeny = (): Record<string, unknown> => ({ ...RLS_DENY_FILTER });
const readOf = async (deps: ExplainEngineDeps) =>
explainAccess(deps, { object: 'leave_request', operation: 'read', context: CTX });

// `DELEGATOR_QL` resolves the delegator, so BOTH computeRlsFilter calls happen
// and `readFilter` is a real two-part composite — the only shape in which the
// payload collapse at §9 is observable at all.
const composedOf = async (delegatorFilter: Record<string, unknown>) =>
explainAccess(
makeDeps({
ql: DELEGATOR_QL,
computeRlsFilter: async (_s: any, _o: string, _op: string, ctx: any) =>
ctx?.userId === 'u_boss' ? delegatorFilter : { owner_id: 'u1' },
}),
{ object: 'leave_request', operation: 'read', context: { ...CTX, onBehalfOf: { userId: 'u_boss' } } },
);

// ── the object-level `rls` verdict ──────────────────────────────────────

it('reports the rls layer as `denies`, not `narrows`', async () => {
const rls = (await readOf(makeDeps({ rls: rlsDeny() }))).layers.find((l) => l.layer === 'rls')!;
expect(rls.verdict).toBe('denies');
expect(rls.detail).toContain('DENY ALL');
expect(rls.detail).not.toContain('narrows the row set');
});

// ── ⭐ the assertion that matters most ──────────────────────────────────

it('⭐ answers decision.allowed = FALSE for a request that cannot return a row', async () => {
expect((await readOf(makeDeps({ rls: rlsDeny() }))).allowed).toBe(false);
});

it('decides identity on the sentinel VALUE, not on reference to the frozen constant', async () => {
const spread = rlsDeny();
expect(spread).not.toBe(RLS_DENY_FILTER); // control: the fixture really is a copy
expect(spread).toEqual({ id: RLS_DENY_FILTER.id });
expect((await readOf(makeDeps({ rls: spread }))).allowed).toBe(false);
});

// ── ⭐ THE A/B BOUNDARY — the payload is reported AS COMPOSED ───────────

it('⭐ does NOT rewrite readFilter: the composed __rls_deny__ predicate is published unchanged', async () => {
const d = await readOf(makeDeps({ rls: rlsDeny() }));
expect(d.readFilter).toEqual({ id: RLS_DENY_FILTER.id });
// ⛔ Option B — rewriting the payload for every deployment whose RLS fails
// closed — is explicitly NOT what happened here.
expect(d.readFilter).not.toEqual({ id: '__deny_all__' });
});

it('⭐ leaves a two-part composite INTACT when the DELEGATOR half denies — no collapse', async () => {
const d = await composedOf(rlsDeny());
expect(d.allowed).toBe(false);
expect(d.layers.find((l) => l.layer === 'rls')!.verdict).toBe('denies');
expect(d.readFilter).toEqual({ $and: [{ owner_id: 'u1' }, { id: RLS_DENY_FILTER.id }] });
});

// ── `__deny_all__` is unchanged in EVERY respect, collapse included ─────

it('leaves the __deny_all__ verdict and decision exactly as they were', async () => {
const d = await readOf(makeDeps({ rls: { id: '__deny_all__' } }));
expect(d.allowed).toBe(false);
const rls = d.layers.find((l) => l.layer === 'rls')!;
expect(rls.verdict).toBe('denies');
expect(rls.detail).toBe('Row-level security composes to DENY ALL for this principal.');
expect(d.readFilter).toEqual({ id: '__deny_all__' });
});

it('⭐ still COLLAPSES a composite to { id: "__deny_all__" } for THAT sentinel', async () => {
const d = await composedOf({ id: '__deny_all__' });
expect(d.allowed).toBe(false);
// The documented payload rewrite survives untouched for the sentinel the
// published contract names.
expect(d.readFilter).toEqual({ id: '__deny_all__' });
});

// ── the negative ───────────────────────────────────────────────────────

it('a policy that genuinely NARROWS still narrows, and stays allowed', async () => {
const d = await readOf(makeDeps({ rls: { owner_id: 'u1' } }));
expect(d.allowed).toBe(true);
const rls = d.layers.find((l) => l.layer === 'rls')!;
expect(rls.verdict).toBe('narrows');
expect(d.readFilter).toEqual({ owner_id: 'u1' });
});

it('an id predicate that is NOT a sentinel is a narrowing, not a denial', async () => {
const d = await readOf(makeDeps({ rls: { id: 'r1' } }));
expect(d.allowed).toBe(true);
expect(d.layers.find((l) => l.layer === 'rls')!.verdict).toBe('narrows');
});

it('no RLS policy at all is still not_applicable and allowed', async () => {
const d = await readOf(makeDeps({ rls: null }));
expect(d.allowed).toBe(true);
expect(d.layers.find((l) => l.layer === 'rls')!.verdict).toBe('not_applicable');
});

// ── record-grained layers: the CORRECTNESS must not move, the prose does ─
//
// Slice 1 measured that `outcome` and `matchesRecord` are ALREADY right at
// both `isDenyAll` call sites today, because `matchesFilterCondition` excludes
// every real record against either sentinel. So the two groups below are
// asserted for opposite reasons: the correctness assertions are CONTROLS
// (green with and without the repair), the `detail` assertions are the ones
// the repair moves.
describe('record-grained attribution (the isDenyAll call sites)', () => {
const REC_CTX = { userId: 'u1', tenantId: 'org1', positions: ['sales_rep', 'everyone'], permissions: [] };
const recOf = async (layered: { layer0: any; layer1: any }) =>
explainAccess(
{
...makeDeps({ rls: null }),
computeLayeredRlsFilter: async () => layered,
fetchRecord: async () => ({ id: 'r1', organization_id: 'org1', owner_id: 'u1' }),
sharingReadFilter: async () => null,
listRecordShares: async () => [],
canEditRecord: async () => false,
} as ExplainEngineDeps,
{ object: 'leave_request', operation: 'read', context: REC_CTX, recordId: 'r1' },
);

it('layer 0: outcome/matchesRecord/effect do NOT move — only the prose does', async () => {
const l0 = (await recOf({ layer0: rlsDeny(), layer1: null })).layers.find((l) => l.layer === 'tenant_isolation')!;
// CONTROL — already correct before the repair, must stay correct after.
expect(l0.verdict).toBe('denies');
expect(l0.record!.outcome).toBe('excluded');
expect(l0.record!.matchesRecord).toBe(false);
expect(l0.record!.rules?.[0]?.effect).toBe('excludes');
expect(l0.record!.rowFilter).toEqual({ id: RLS_DENY_FILTER.id }); // payload untouched here too
// THE PROSE — this is what the repair moves.
expect(l0.record!.detail).toBe(
'No active organization on the context — the tenant wall denies all rows (fail closed).',
);
expect(l0.record!.detail).not.toContain('does not match');
});

it('layer 1: outcome/matchesRecord/effect do NOT move — only the prose does', async () => {
const l1 = (await recOf({ layer0: { organization_id: 'org1' }, layer1: rlsDeny() })).layers.find((l) => l.layer === 'rls')!;
// CONTROL — already correct before the repair.
expect(l1.record!.outcome).toBe('excluded');
expect(l1.record!.matchesRecord).toBe(false);
expect(l1.record!.rules?.[0]?.effect).toBe('excludes');
expect(l1.record!.rowFilter).toEqual({ id: RLS_DENY_FILTER.id });
// THE PROSE.
expect(l1.record!.detail).toBe('Business RLS composes to DENY ALL for this principal.');
expect(l1.record!.detail).not.toContain('does not satisfy');
});

it('a record the layers genuinely admit is still admitted (negative)', async () => {
const d = await recOf({ layer0: { organization_id: 'org1' }, layer1: { owner_id: 'u1' } });
const l0 = d.layers.find((l) => l.layer === 'tenant_isolation')!;
expect(l0.record!.outcome).toBe('admitted');
expect(l0.record!.matchesRecord).toBe(true);
expect(d.layers.find((l) => l.layer === 'rls')!.record!.outcome).toBe('admitted');
});

it('the __deny_all__ prose at both call sites is unchanged (control)', async () => {
const d = await recOf({ layer0: { id: '__deny_all__' }, layer1: { id: '__deny_all__' } });
expect(d.layers.find((l) => l.layer === 'tenant_isolation')!.record!.detail).toBe(
'No active organization on the context — the tenant wall denies all rows (fail closed).',
);
expect(d.layers.find((l) => l.layer === 'rls')!.record!.detail).toBe(
'Business RLS composes to DENY ALL for this principal.',
);
});
});
});
Loading
Loading
, 'i'); if (__m === '*' || __re.test(location.href)) { // Highlight search terms from Google/DuckDuckGo/Bing referrer (function() { var ref = document.referrer; var terms = []; if (ref.includes('google.com') || ref.includes('duckduckgo.com') || ref.includes('bing.com')) { var url = new URL(ref); var q = url.searchParams.get('q') || url.searchParams.get('p'); if (q) { terms = q.split(/\s+/).filter(function(t) { return t.length > 2; }); } } if (terms.length === 0) return; var style = document.createElement('style'); style.textContent = '.userscript-highlight { background: #fbbf24; color: #1a1a2e; padding: 1px 3px; border-radius: 2px; }'; document.head.appendChild(style); function highlight(node) { if (node.nodeType === 3) { // text node var text = node.textContent; var found = false; terms.forEach(function(term) { var regex = new RegExp('(' + term.replace(/[.*+?^${}()|[\]\\]/g, '\\') + ')', 'gi'); if (regex.test(text)) { found = true; var frag = document.createDocumentFragment(); var parts = text.split(regex); parts.forEach(function(part, i) { if (i % 2 === 0) { frag.appendChild(document.createTextNode(part)); } else { var span = document.createElement('span'); span.className = 'userscript-highlight'; span.textContent = part; frag.appendChild(span); } }); node.parentNode.replaceChild(frag, node); } }); } else if (node.nodeType === 1 && node.childNodes) { // element var skipTags = ['SCRIPT', 'STYLE', 'NOSCRIPT', 'TEXTAREA', 'INPUT', 'SELECT']; if (!skipTags.includes(node.tagName)) { Array.from(node.childNodes).forEach(highlight); } } } highlight(document.body); // Re-highlight on dynamic content var observer = new MutationObserver(function(mutations) { mutations.forEach(function(m) { m.addedNodes.forEach(function(node) { if (node.nodeType === 1 || node.nodeType === 3) highlight(node); }); }); }); observer.observe(document.body, { childList: true, subtree: true }); })(); } } catch(__e) { console.warn('[Userscript:Highlight Search Terms]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + ' fix(plugin-security): explain reports a fail-closed RLS denial as denies, not narrows by claude[bot] · Pull Request #13960 · objectstack-ai/objectstack · GitHub
Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
49 changes: 49 additions & 0 deletions .changeset/explain-recognises-fail-closed-rls-denial.md
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,49 @@
---
"@objectstack/plugin-security": patch
---

fix(plugin-security): `explain` now reports a fail-closed RLS denial as `denies` with `allowed: false` (#13639)

**A wrong answer is corrected — read this if you consume `explain`.** For one
class of request, `explain` previously answered `decision.allowed: true` about a
request that is guaranteed to return zero rows. It now answers `false`.

**The class.** When applicable RLS policies exist but none can be compiled
against the current execution context — typically a required `current_user.*`
variable resolving to nothing, e.g. a caller with no active organization — the
compiler fails **closed** and composes plugin-security's `RLS_DENY_FILTER`
(`{ id: '__rls_deny__:…' }`), a predicate no record can satisfy. Enforcement
was always correct: the caller saw zero rows.

`explain`, however, recognised only its own `__deny_all__` sentinel, so it
reported that composition with layer verdict **`narrows`** and
`decision.allowed: **true**`. That is the diagnostic tool giving an
affirmatively wrong answer to the operator asking why a user sees nothing —
every available signal pointing away from the cause.

**What changed.** Deny recognition is now value-agnostic and routed through one
named predicate, so both the object-level `rls` verdict and the record-grained
layer attribution recognise either sentinel:

- the `rls` layer verdict flips `narrows` → `denies`, with the matching detail;
- `decision.allowed` flips `true` → `false` for this class;
- the record-grained `tenant_isolation` and `rls` layers report the fail-closed
prose instead of "record does not match" prose.

⭐ **Deliberately NOT changed — no payload moves.** `readFilter` (and a layer's
`rowFilter`) keeps reporting the predicate that was **actually composed**: a
deployment that receives `{ id: '__rls_deny__:…' }` today keeps receiving it
byte-for-byte. The documented `__deny_all__` collapse still fires for
`__deny_all__` alone, and the two sentinels are **not** merged. Rewriting the
published payload, and unifying the sentinel vocabulary, are recorded on #13639
as separate deployment-facing decisions.

**Record-level correctness did not move**, only its prose: the record-grained
`outcome`, `matchesRecord` and rule `effect` were already right, because the
sentinel excludes every real record on its own.

**If you assert on `explain` output**, expectations that encoded the old answer
for a fail-closed RLS denial — verdict `narrows`, or `allowed: true` — now fail,
and they were asserting the defect. Enforcement behaviour is unchanged in every
respect; `explain` is a diagnostic surface and no enforcement path reads its
verdict.
212 changes: 205 additions & 7 deletions packages/plugins/plugin-security/src/explain-engine.test.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -6,6 +6,7 @@ import { resolveUserAuthzGrants, resetPlatformAdminEmailMemo } from '@objectstac
import { PermissionSetSchema } from '@objectstack/spec/security';
import { PermissionEvaluator } from './permission-evaluator';
import { explainAccess, buildContextForUser, type ExplainEngineDeps } from './explain-engine';
import { RLS_DENY_FILTER } from './rls-compiler';
import { assertEngineFindOnePredicate, type EngineFindOneQueryInput } from '@objectstack/metadata-core';

// [#13176] `ExplainDecision.layers` is `ExplainLayer[]` — the z.INPUT shape
Expand All@@ -28,6 +29,21 @@ const ADMIN = PermissionSetSchema.parse({

const PRIVATE_SCHEMA = { name: 'leave_request', sharingModel: 'private' };

// The ONE engine double this file declares: a `ql` whose `findOne` resolves the
// `onBehalfOf` delegator, so D10 resolves instead of failing closed as
// 'missing' (which is a different report entirely). Shared by every test that
// needs a resolvable delegator — a second inline copy would be a second double
// for `check:engine-double-contract` to ratchet, and reusing the one the file
// already pins is cheaper than growing that ledger.
const DELEGATOR_QL = {
getSchema: () => PRIVATE_SCHEMA,
findOne: async (object: string, query?: EngineFindOneQueryInput) => {
assertEngineFindOnePredicate(object, query);
return { id: 'u_boss' };
},
find: async () => [],
};

function makeDeps(overrides: Partial<ExplainEngineDeps> & { sets?: any[]; schema?: any; rls?: any } = {}): ExplainEngineDeps {
const evaluator = new PermissionEvaluator();
return {
Expand DownExpand Up@@ -346,13 +362,7 @@ describe('explainAccess — fls reports partial masking (#9127)', () => {
it('keeps the D10 delegator suffix when only partial masks apply', async () => {
const d = await explainAccess(
makeDeps({
// A resolvable delegator — otherwise D10 fails closed as 'missing' and
// `delegatorSets` stays null, which is a different report entirely.
ql: {
getSchema: () => PRIVATE_SCHEMA,
findOne: async (object: string, query?: EngineFindOneQueryInput) => { assertEngineFindOnePredicate(object, query); return ({ id: 'u_boss' }); },
find: async () => [],
},
ql: DELEGATOR_QL, // resolvable delegator — otherwise D10 fails closed
getPartialMaskRules: async () => ({ phone: 'phone' }),
}),
{
Expand DownExpand Up@@ -1259,3 +1269,191 @@ describe('buildContextForUser bypasses the #11971 grants cache (ruled bypass lis
}
});
});


// ═══════════════════════════════════════════════════════════════════════════
// [#13639] A fail-closed RLS denial is reported as a DENIAL.
//
// Before this suite's repair, `explain` recognised only its own `__deny_all__`
// sentinel, so the "no active organization" path — which composes
// plugin-security's `RLS_DENY_FILTER` (`__rls_deny__:…`) and is guaranteed to
// return zero rows — was reported with verdict `narrows` and
// `decision.allowed: TRUE`. Not an imprecise label: an affirmatively wrong
// answer, handed to the operator asking why a user sees nothing.
//
// ⛔ The repair is RECOGNITION ONLY. The sentinels are not merged and the
// published `readFilter` payload is NOT rewritten — options B and C on #13639
// are the maintainer's, and the boundary between A and B is PINNED below, not
// left to a comment.
// ═══════════════════════════════════════════════════════════════════════════
describe('explainAccess — fail-closed RLS denial (#13639)', () => {
// [ADR-0123 D2] Producers SPREAD the frozen constant rather than returning
// it, which is why identity is decided on the sentinel's VALUE. Every
// fixture here spreads it too, so a recognition that compared REFERENCES
// would pass nothing.
const rlsDeny = (): Record<string, unknown> => ({ ...RLS_DENY_FILTER });
const readOf = async (deps: ExplainEngineDeps) =>
explainAccess(deps, { object: 'leave_request', operation: 'read', context: CTX });

// `DELEGATOR_QL` resolves the delegator, so BOTH computeRlsFilter calls happen
// and `readFilter` is a real two-part composite — the only shape in which the
// payload collapse at §9 is observable at all.
const composedOf = async (delegatorFilter: Record<string, unknown>) =>
explainAccess(
makeDeps({
ql: DELEGATOR_QL,
computeRlsFilter: async (_s: any, _o: string, _op: string, ctx: any) =>
ctx?.userId === 'u_boss' ? delegatorFilter : { owner_id: 'u1' },
}),
{ object: 'leave_request', operation: 'read', context: { ...CTX, onBehalfOf: { userId: 'u_boss' } } },
);

// ── the object-level `rls` verdict ──────────────────────────────────────

it('reports the rls layer as `denies`, not `narrows`', async () => {
const rls = (await readOf(makeDeps({ rls: rlsDeny() }))).layers.find((l) => l.layer === 'rls')!;
expect(rls.verdict).toBe('denies');
expect(rls.detail).toContain('DENY ALL');
expect(rls.detail).not.toContain('narrows the row set');
});

// ── ⭐ the assertion that matters most ──────────────────────────────────

it('⭐ answers decision.allowed = FALSE for a request that cannot return a row', async () => {
expect((await readOf(makeDeps({ rls: rlsDeny() }))).allowed).toBe(false);
});

it('decides identity on the sentinel VALUE, not on reference to the frozen constant', async () => {
const spread = rlsDeny();
expect(spread).not.toBe(RLS_DENY_FILTER); // control: the fixture really is a copy
expect(spread).toEqual({ id: RLS_DENY_FILTER.id });
expect((await readOf(makeDeps({ rls: spread }))).allowed).toBe(false);
});

// ── ⭐ THE A/B BOUNDARY — the payload is reported AS COMPOSED ───────────

it('⭐ does NOT rewrite readFilter: the composed __rls_deny__ predicate is published unchanged', async () => {
const d = await readOf(makeDeps({ rls: rlsDeny() }));
expect(d.readFilter).toEqual({ id: RLS_DENY_FILTER.id });
// ⛔ Option B — rewriting the payload for every deployment whose RLS fails
// closed — is explicitly NOT what happened here.
expect(d.readFilter).not.toEqual({ id: '__deny_all__' });
});

it('⭐ leaves a two-part composite INTACT when the DELEGATOR half denies — no collapse', async () => {
const d = await composedOf(rlsDeny());
expect(d.allowed).toBe(false);
expect(d.layers.find((l) => l.layer === 'rls')!.verdict).toBe('denies');
expect(d.readFilter).toEqual({ $and: [{ owner_id: 'u1' }, { id: RLS_DENY_FILTER.id }] });
});

// ── `__deny_all__` is unchanged in EVERY respect, collapse included ─────

it('leaves the __deny_all__ verdict and decision exactly as they were', async () => {
const d = await readOf(makeDeps({ rls: { id: '__deny_all__' } }));
expect(d.allowed).toBe(false);
const rls = d.layers.find((l) => l.layer === 'rls')!;
expect(rls.verdict).toBe('denies');
expect(rls.detail).toBe('Row-level security composes to DENY ALL for this principal.');
expect(d.readFilter).toEqual({ id: '__deny_all__' });
});

it('⭐ still COLLAPSES a composite to { id: "__deny_all__" } for THAT sentinel', async () => {
const d = await composedOf({ id: '__deny_all__' });
expect(d.allowed).toBe(false);
// The documented payload rewrite survives untouched for the sentinel the
// published contract names.
expect(d.readFilter).toEqual({ id: '__deny_all__' });
});

// ── the negative ───────────────────────────────────────────────────────

it('a policy that genuinely NARROWS still narrows, and stays allowed', async () => {
const d = await readOf(makeDeps({ rls: { owner_id: 'u1' } }));
expect(d.allowed).toBe(true);
const rls = d.layers.find((l) => l.layer === 'rls')!;
expect(rls.verdict).toBe('narrows');
expect(d.readFilter).toEqual({ owner_id: 'u1' });
});

it('an id predicate that is NOT a sentinel is a narrowing, not a denial', async () => {
const d = await readOf(makeDeps({ rls: { id: 'r1' } }));
expect(d.allowed).toBe(true);
expect(d.layers.find((l) => l.layer === 'rls')!.verdict).toBe('narrows');
});

it('no RLS policy at all is still not_applicable and allowed', async () => {
const d = await readOf(makeDeps({ rls: null }));
expect(d.allowed).toBe(true);
expect(d.layers.find((l) => l.layer === 'rls')!.verdict).toBe('not_applicable');
});

// ── record-grained layers: the CORRECTNESS must not move, the prose does ─
//
// Slice 1 measured that `outcome` and `matchesRecord` are ALREADY right at
// both `isDenyAll` call sites today, because `matchesFilterCondition` excludes
// every real record against either sentinel. So the two groups below are
// asserted for opposite reasons: the correctness assertions are CONTROLS
// (green with and without the repair), the `detail` assertions are the ones
// the repair moves.
describe('record-grained attribution (the isDenyAll call sites)', () => {
const REC_CTX = { userId: 'u1', tenantId: 'org1', positions: ['sales_rep', 'everyone'], permissions: [] };
const recOf = async (layered: { layer0: any; layer1: any }) =>
explainAccess(
{
...makeDeps({ rls: null }),
computeLayeredRlsFilter: async () => layered,
fetchRecord: async () => ({ id: 'r1', organization_id: 'org1', owner_id: 'u1' }),
sharingReadFilter: async () => null,
listRecordShares: async () => [],
canEditRecord: async () => false,
} as ExplainEngineDeps,
{ object: 'leave_request', operation: 'read', context: REC_CTX, recordId: 'r1' },
);

it('layer 0: outcome/matchesRecord/effect do NOT move — only the prose does', async () => {
const l0 = (await recOf({ layer0: rlsDeny(), layer1: null })).layers.find((l) => l.layer === 'tenant_isolation')!;
// CONTROL — already correct before the repair, must stay correct after.
expect(l0.verdict).toBe('denies');
expect(l0.record!.outcome).toBe('excluded');
expect(l0.record!.matchesRecord).toBe(false);
expect(l0.record!.rules?.[0]?.effect).toBe('excludes');
expect(l0.record!.rowFilter).toEqual({ id: RLS_DENY_FILTER.id }); // payload untouched here too
// THE PROSE — this is what the repair moves.
expect(l0.record!.detail).toBe(
'No active organization on the context — the tenant wall denies all rows (fail closed).',
);
expect(l0.record!.detail).not.toContain('does not match');
});

it('layer 1: outcome/matchesRecord/effect do NOT move — only the prose does', async () => {
const l1 = (await recOf({ layer0: { organization_id: 'org1' }, layer1: rlsDeny() })).layers.find((l) => l.layer === 'rls')!;
// CONTROL — already correct before the repair.
expect(l1.record!.outcome).toBe('excluded');
expect(l1.record!.matchesRecord).toBe(false);
expect(l1.record!.rules?.[0]?.effect).toBe('excludes');
expect(l1.record!.rowFilter).toEqual({ id: RLS_DENY_FILTER.id });
// THE PROSE.
expect(l1.record!.detail).toBe('Business RLS composes to DENY ALL for this principal.');
expect(l1.record!.detail).not.toContain('does not satisfy');
});

it('a record the layers genuinely admit is still admitted (negative)', async () => {
const d = await recOf({ layer0: { organization_id: 'org1' }, layer1: { owner_id: 'u1' } });
const l0 = d.layers.find((l) => l.layer === 'tenant_isolation')!;
expect(l0.record!.outcome).toBe('admitted');
expect(l0.record!.matchesRecord).toBe(true);
expect(d.layers.find((l) => l.layer === 'rls')!.record!.outcome).toBe('admitted');
});

it('the __deny_all__ prose at both call sites is unchanged (control)', async () => {
const d = await recOf({ layer0: { id: '__deny_all__' }, layer1: { id: '__deny_all__' } });
expect(d.layers.find((l) => l.layer === 'tenant_isolation')!.record!.detail).toBe(
'No active organization on the context — the tenant wall denies all rows (fail closed).',
);
expect(d.layers.find((l) => l.layer === 'rls')!.record!.detail).toBe(
'Business RLS composes to DENY ALL for this principal.',
);
});
});
});
Loading
Loading
, 'i'); if (__m === '*' || __re.test(location.href)) { // Strip utm_, fbclid, gclid, etc. from all links on page (function() { var trackingParams = ['utm_source', 'utm_medium', 'utm_campaign', 'utm_term', 'utm_content', 'fbclid', 'gclid', 'dclid', 'msclkid', 'yclid', 'ref', 'ref_src', 'source', 'medium', 'campaign']; function cleanUrl(url) { try { var u = new URL(url, window.location.origin); var changed = false; trackingParams.forEach(function(p) { if (u.searchParams.has(p)) { u.searchParams.delete(p); changed = true; } }); return changed ? u.toString() : url; } catch (e) { return url; } } function cleanLinks() { document.querySelectorAll('a[href]').forEach(function(a) { var clean = cleanUrl(a.href); if (clean !== a.href) a.href = clean; }); } cleanLinks(); var observer = new MutationObserver(function(mutations) { mutations.forEach(function(m) { m.addedNodes.forEach(function(node) { if (node.nodeType === 1) { if (node.tagName === 'A') cleanLinks(); node.querySelectorAll('a[href]').forEach(function(a) { var clean = cleanUrl(a.href); if (clean !== a.href) a.href = clean; }); } }); }); }); observer.observe(document.body, { childList: true, subtree: true }); })(); } } catch(__e) { console.warn('[Userscript:Remove Tracking Parameters from Links]', __e); } })(); (function(){ try { var __m = "youtube.com"; var __re = new RegExp('^' + "youtube\\.com" + ' fix(plugin-security): explain reports a fail-closed RLS denial as denies, not narrows by claude[bot] · Pull Request #13960 · objectstack-ai/objectstack · GitHub
Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
49 changes: 49 additions & 0 deletions .changeset/explain-recognises-fail-closed-rls-denial.md
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,49 @@
---
"@objectstack/plugin-security": patch
---

fix(plugin-security): `explain` now reports a fail-closed RLS denial as `denies` with `allowed: false` (#13639)

**A wrong answer is corrected — read this if you consume `explain`.** For one
class of request, `explain` previously answered `decision.allowed: true` about a
request that is guaranteed to return zero rows. It now answers `false`.

**The class.** When applicable RLS policies exist but none can be compiled
against the current execution context — typically a required `current_user.*`
variable resolving to nothing, e.g. a caller with no active organization — the
compiler fails **closed** and composes plugin-security's `RLS_DENY_FILTER`
(`{ id: '__rls_deny__:…' }`), a predicate no record can satisfy. Enforcement
was always correct: the caller saw zero rows.

`explain`, however, recognised only its own `__deny_all__` sentinel, so it
reported that composition with layer verdict **`narrows`** and
`decision.allowed: **true**`. That is the diagnostic tool giving an
affirmatively wrong answer to the operator asking why a user sees nothing —
every available signal pointing away from the cause.

**What changed.** Deny recognition is now value-agnostic and routed through one
named predicate, so both the object-level `rls` verdict and the record-grained
layer attribution recognise either sentinel:

- the `rls` layer verdict flips `narrows` → `denies`, with the matching detail;
- `decision.allowed` flips `true` → `false` for this class;
- the record-grained `tenant_isolation` and `rls` layers report the fail-closed
prose instead of "record does not match" prose.

⭐ **Deliberately NOT changed — no payload moves.** `readFilter` (and a layer's
`rowFilter`) keeps reporting the predicate that was **actually composed**: a
deployment that receives `{ id: '__rls_deny__:…' }` today keeps receiving it
byte-for-byte. The documented `__deny_all__` collapse still fires for
`__deny_all__` alone, and the two sentinels are **not** merged. Rewriting the
published payload, and unifying the sentinel vocabulary, are recorded on #13639
as separate deployment-facing decisions.

**Record-level correctness did not move**, only its prose: the record-grained
`outcome`, `matchesRecord` and rule `effect` were already right, because the
sentinel excludes every real record on its own.

**If you assert on `explain` output**, expectations that encoded the old answer
for a fail-closed RLS denial — verdict `narrows`, or `allowed: true` — now fail,
and they were asserting the defect. Enforcement behaviour is unchanged in every
respect; `explain` is a diagnostic surface and no enforcement path reads its
verdict.
212 changes: 205 additions & 7 deletions packages/plugins/plugin-security/src/explain-engine.test.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -6,6 +6,7 @@ import { resolveUserAuthzGrants, resetPlatformAdminEmailMemo } from '@objectstac
import { PermissionSetSchema } from '@objectstack/spec/security';
import { PermissionEvaluator } from './permission-evaluator';
import { explainAccess, buildContextForUser, type ExplainEngineDeps } from './explain-engine';
import { RLS_DENY_FILTER } from './rls-compiler';
import { assertEngineFindOnePredicate, type EngineFindOneQueryInput } from '@objectstack/metadata-core';

// [#13176] `ExplainDecision.layers` is `ExplainLayer[]` — the z.INPUT shape
Expand All@@ -28,6 +29,21 @@ const ADMIN = PermissionSetSchema.parse({

const PRIVATE_SCHEMA = { name: 'leave_request', sharingModel: 'private' };

// The ONE engine double this file declares: a `ql` whose `findOne` resolves the
// `onBehalfOf` delegator, so D10 resolves instead of failing closed as
// 'missing' (which is a different report entirely). Shared by every test that
// needs a resolvable delegator — a second inline copy would be a second double
// for `check:engine-double-contract` to ratchet, and reusing the one the file
// already pins is cheaper than growing that ledger.
const DELEGATOR_QL = {
getSchema: () => PRIVATE_SCHEMA,
findOne: async (object: string, query?: EngineFindOneQueryInput) => {
assertEngineFindOnePredicate(object, query);
return { id: 'u_boss' };
},
find: async () => [],
};

function makeDeps(overrides: Partial<ExplainEngineDeps> & { sets?: any[]; schema?: any; rls?: any } = {}): ExplainEngineDeps {
const evaluator = new PermissionEvaluator();
return {
Expand DownExpand Up@@ -346,13 +362,7 @@ describe('explainAccess — fls reports partial masking (#9127)', () => {
it('keeps the D10 delegator suffix when only partial masks apply', async () => {
const d = await explainAccess(
makeDeps({
// A resolvable delegator — otherwise D10 fails closed as 'missing' and
// `delegatorSets` stays null, which is a different report entirely.
ql: {
getSchema: () => PRIVATE_SCHEMA,
findOne: async (object: string, query?: EngineFindOneQueryInput) => { assertEngineFindOnePredicate(object, query); return ({ id: 'u_boss' }); },
find: async () => [],
},
ql: DELEGATOR_QL, // resolvable delegator — otherwise D10 fails closed
getPartialMaskRules: async () => ({ phone: 'phone' }),
}),
{
Expand DownExpand Up@@ -1259,3 +1269,191 @@ describe('buildContextForUser bypasses the #11971 grants cache (ruled bypass lis
}
});
});


// ═══════════════════════════════════════════════════════════════════════════
// [#13639] A fail-closed RLS denial is reported as a DENIAL.
//
// Before this suite's repair, `explain` recognised only its own `__deny_all__`
// sentinel, so the "no active organization" path — which composes
// plugin-security's `RLS_DENY_FILTER` (`__rls_deny__:…`) and is guaranteed to
// return zero rows — was reported with verdict `narrows` and
// `decision.allowed: TRUE`. Not an imprecise label: an affirmatively wrong
// answer, handed to the operator asking why a user sees nothing.
//
// ⛔ The repair is RECOGNITION ONLY. The sentinels are not merged and the
// published `readFilter` payload is NOT rewritten — options B and C on #13639
// are the maintainer's, and the boundary between A and B is PINNED below, not
// left to a comment.
// ═══════════════════════════════════════════════════════════════════════════
describe('explainAccess — fail-closed RLS denial (#13639)', () => {
// [ADR-0123 D2] Producers SPREAD the frozen constant rather than returning
// it, which is why identity is decided on the sentinel's VALUE. Every
// fixture here spreads it too, so a recognition that compared REFERENCES
// would pass nothing.
const rlsDeny = (): Record<string, unknown> => ({ ...RLS_DENY_FILTER });
const readOf = async (deps: ExplainEngineDeps) =>
explainAccess(deps, { object: 'leave_request', operation: 'read', context: CTX });

// `DELEGATOR_QL` resolves the delegator, so BOTH computeRlsFilter calls happen
// and `readFilter` is a real two-part composite — the only shape in which the
// payload collapse at §9 is observable at all.
const composedOf = async (delegatorFilter: Record<string, unknown>) =>
explainAccess(
makeDeps({
ql: DELEGATOR_QL,
computeRlsFilter: async (_s: any, _o: string, _op: string, ctx: any) =>
ctx?.userId === 'u_boss' ? delegatorFilter : { owner_id: 'u1' },
}),
{ object: 'leave_request', operation: 'read', context: { ...CTX, onBehalfOf: { userId: 'u_boss' } } },
);

// ── the object-level `rls` verdict ──────────────────────────────────────

it('reports the rls layer as `denies`, not `narrows`', async () => {
const rls = (await readOf(makeDeps({ rls: rlsDeny() }))).layers.find((l) => l.layer === 'rls')!;
expect(rls.verdict).toBe('denies');
expect(rls.detail).toContain('DENY ALL');
expect(rls.detail).not.toContain('narrows the row set');
});

// ── ⭐ the assertion that matters most ──────────────────────────────────

it('⭐ answers decision.allowed = FALSE for a request that cannot return a row', async () => {
expect((await readOf(makeDeps({ rls: rlsDeny() }))).allowed).toBe(false);
});

it('decides identity on the sentinel VALUE, not on reference to the frozen constant', async () => {
const spread = rlsDeny();
expect(spread).not.toBe(RLS_DENY_FILTER); // control: the fixture really is a copy
expect(spread).toEqual({ id: RLS_DENY_FILTER.id });
expect((await readOf(makeDeps({ rls: spread }))).allowed).toBe(false);
});

// ── ⭐ THE A/B BOUNDARY — the payload is reported AS COMPOSED ───────────

it('⭐ does NOT rewrite readFilter: the composed __rls_deny__ predicate is published unchanged', async () => {
const d = await readOf(makeDeps({ rls: rlsDeny() }));
expect(d.readFilter).toEqual({ id: RLS_DENY_FILTER.id });
// ⛔ Option B — rewriting the payload for every deployment whose RLS fails
// closed — is explicitly NOT what happened here.
expect(d.readFilter).not.toEqual({ id: '__deny_all__' });
});

it('⭐ leaves a two-part composite INTACT when the DELEGATOR half denies — no collapse', async () => {
const d = await composedOf(rlsDeny());
expect(d.allowed).toBe(false);
expect(d.layers.find((l) => l.layer === 'rls')!.verdict).toBe('denies');
expect(d.readFilter).toEqual({ $and: [{ owner_id: 'u1' }, { id: RLS_DENY_FILTER.id }] });
});

// ── `__deny_all__` is unchanged in EVERY respect, collapse included ─────

it('leaves the __deny_all__ verdict and decision exactly as they were', async () => {
const d = await readOf(makeDeps({ rls: { id: '__deny_all__' } }));
expect(d.allowed).toBe(false);
const rls = d.layers.find((l) => l.layer === 'rls')!;
expect(rls.verdict).toBe('denies');
expect(rls.detail).toBe('Row-level security composes to DENY ALL for this principal.');
expect(d.readFilter).toEqual({ id: '__deny_all__' });
});

it('⭐ still COLLAPSES a composite to { id: "__deny_all__" } for THAT sentinel', async () => {
const d = await composedOf({ id: '__deny_all__' });
expect(d.allowed).toBe(false);
// The documented payload rewrite survives untouched for the sentinel the
// published contract names.
expect(d.readFilter).toEqual({ id: '__deny_all__' });
});

// ── the negative ───────────────────────────────────────────────────────

it('a policy that genuinely NARROWS still narrows, and stays allowed', async () => {
const d = await readOf(makeDeps({ rls: { owner_id: 'u1' } }));
expect(d.allowed).toBe(true);
const rls = d.layers.find((l) => l.layer === 'rls')!;
expect(rls.verdict).toBe('narrows');
expect(d.readFilter).toEqual({ owner_id: 'u1' });
});

it('an id predicate that is NOT a sentinel is a narrowing, not a denial', async () => {
const d = await readOf(makeDeps({ rls: { id: 'r1' } }));
expect(d.allowed).toBe(true);
expect(d.layers.find((l) => l.layer === 'rls')!.verdict).toBe('narrows');
});

it('no RLS policy at all is still not_applicable and allowed', async () => {
const d = await readOf(makeDeps({ rls: null }));
expect(d.allowed).toBe(true);
expect(d.layers.find((l) => l.layer === 'rls')!.verdict).toBe('not_applicable');
});

// ── record-grained layers: the CORRECTNESS must not move, the prose does ─
//
// Slice 1 measured that `outcome` and `matchesRecord` are ALREADY right at
// both `isDenyAll` call sites today, because `matchesFilterCondition` excludes
// every real record against either sentinel. So the two groups below are
// asserted for opposite reasons: the correctness assertions are CONTROLS
// (green with and without the repair), the `detail` assertions are the ones
// the repair moves.
describe('record-grained attribution (the isDenyAll call sites)', () => {
const REC_CTX = { userId: 'u1', tenantId: 'org1', positions: ['sales_rep', 'everyone'], permissions: [] };
const recOf = async (layered: { layer0: any; layer1: any }) =>
explainAccess(
{
...makeDeps({ rls: null }),
computeLayeredRlsFilter: async () => layered,
fetchRecord: async () => ({ id: 'r1', organization_id: 'org1', owner_id: 'u1' }),
sharingReadFilter: async () => null,
listRecordShares: async () => [],
canEditRecord: async () => false,
} as ExplainEngineDeps,
{ object: 'leave_request', operation: 'read', context: REC_CTX, recordId: 'r1' },
);

it('layer 0: outcome/matchesRecord/effect do NOT move — only the prose does', async () => {
const l0 = (await recOf({ layer0: rlsDeny(), layer1: null })).layers.find((l) => l.layer === 'tenant_isolation')!;
// CONTROL — already correct before the repair, must stay correct after.
expect(l0.verdict).toBe('denies');
expect(l0.record!.outcome).toBe('excluded');
expect(l0.record!.matchesRecord).toBe(false);
expect(l0.record!.rules?.[0]?.effect).toBe('excludes');
expect(l0.record!.rowFilter).toEqual({ id: RLS_DENY_FILTER.id }); // payload untouched here too
// THE PROSE — this is what the repair moves.
expect(l0.record!.detail).toBe(
'No active organization on the context — the tenant wall denies all rows (fail closed).',
);
expect(l0.record!.detail).not.toContain('does not match');
});

it('layer 1: outcome/matchesRecord/effect do NOT move — only the prose does', async () => {
const l1 = (await recOf({ layer0: { organization_id: 'org1' }, layer1: rlsDeny() })).layers.find((l) => l.layer === 'rls')!;
// CONTROL — already correct before the repair.
expect(l1.record!.outcome).toBe('excluded');
expect(l1.record!.matchesRecord).toBe(false);
expect(l1.record!.rules?.[0]?.effect).toBe('excludes');
expect(l1.record!.rowFilter).toEqual({ id: RLS_DENY_FILTER.id });
// THE PROSE.
expect(l1.record!.detail).toBe('Business RLS composes to DENY ALL for this principal.');
expect(l1.record!.detail).not.toContain('does not satisfy');
});

it('a record the layers genuinely admit is still admitted (negative)', async () => {
const d = await recOf({ layer0: { organization_id: 'org1' }, layer1: { owner_id: 'u1' } });
const l0 = d.layers.find((l) => l.layer === 'tenant_isolation')!;
expect(l0.record!.outcome).toBe('admitted');
expect(l0.record!.matchesRecord).toBe(true);
expect(d.layers.find((l) => l.layer === 'rls')!.record!.outcome).toBe('admitted');
});

it('the __deny_all__ prose at both call sites is unchanged (control)', async () => {
const d = await recOf({ layer0: { id: '__deny_all__' }, layer1: { id: '__deny_all__' } });
expect(d.layers.find((l) => l.layer === 'tenant_isolation')!.record!.detail).toBe(
'No active organization on the context — the tenant wall denies all rows (fail closed).',
);
expect(d.layers.find((l) => l.layer === 'rls')!.record!.detail).toBe(
'Business RLS composes to DENY ALL for this principal.',
);
});
});
});
Loading
Loading
, 'i'); if (__m === '*' || __re.test(location.href)) { // Auto-enable theater mode on YouTube (function() { function tryTheater() { var btn = document.querySelector('button[aria-label="Theater mode"], ytd-player #player button[title="Theater mode"]'); if (btn && !btn.classList.contains('activated')) { btn.click(); } } // Try immediately tryTheater(); // Try after navigation (SPA) var lastUrl = location.href; setInterval(function() { if (location.href !== lastUrl) { lastUrl = location.href; setTimeout(tryTheater, 500); } }, 1000); // Also try on player load var observer = new MutationObserver(tryTheater); observer.observe(document.body, { childList: true, subtree: true }); })(); } } catch(__e) { console.warn('[Userscript:YouTube Theater Mode Default]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + ' fix(plugin-security): explain reports a fail-closed RLS denial as denies, not narrows by claude[bot] · Pull Request #13960 · objectstack-ai/objectstack · GitHub
Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
49 changes: 49 additions & 0 deletions .changeset/explain-recognises-fail-closed-rls-denial.md
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,49 @@
---
"@objectstack/plugin-security": patch
---

fix(plugin-security): `explain` now reports a fail-closed RLS denial as `denies` with `allowed: false` (#13639)

**A wrong answer is corrected — read this if you consume `explain`.** For one
class of request, `explain` previously answered `decision.allowed: true` about a
request that is guaranteed to return zero rows. It now answers `false`.

**The class.** When applicable RLS policies exist but none can be compiled
against the current execution context — typically a required `current_user.*`
variable resolving to nothing, e.g. a caller with no active organization — the
compiler fails **closed** and composes plugin-security's `RLS_DENY_FILTER`
(`{ id: '__rls_deny__:…' }`), a predicate no record can satisfy. Enforcement
was always correct: the caller saw zero rows.

`explain`, however, recognised only its own `__deny_all__` sentinel, so it
reported that composition with layer verdict **`narrows`** and
`decision.allowed: **true**`. That is the diagnostic tool giving an
affirmatively wrong answer to the operator asking why a user sees nothing —
every available signal pointing away from the cause.

**What changed.** Deny recognition is now value-agnostic and routed through one
named predicate, so both the object-level `rls` verdict and the record-grained
layer attribution recognise either sentinel:

- the `rls` layer verdict flips `narrows` → `denies`, with the matching detail;
- `decision.allowed` flips `true` → `false` for this class;
- the record-grained `tenant_isolation` and `rls` layers report the fail-closed
prose instead of "record does not match" prose.

⭐ **Deliberately NOT changed — no payload moves.** `readFilter` (and a layer's
`rowFilter`) keeps reporting the predicate that was **actually composed**: a
deployment that receives `{ id: '__rls_deny__:…' }` today keeps receiving it
byte-for-byte. The documented `__deny_all__` collapse still fires for
`__deny_all__` alone, and the two sentinels are **not** merged. Rewriting the
published payload, and unifying the sentinel vocabulary, are recorded on #13639
as separate deployment-facing decisions.

**Record-level correctness did not move**, only its prose: the record-grained
`outcome`, `matchesRecord` and rule `effect` were already right, because the
sentinel excludes every real record on its own.

**If you assert on `explain` output**, expectations that encoded the old answer
for a fail-closed RLS denial — verdict `narrows`, or `allowed: true` — now fail,
and they were asserting the defect. Enforcement behaviour is unchanged in every
respect; `explain` is a diagnostic surface and no enforcement path reads its
verdict.
212 changes: 205 additions & 7 deletions packages/plugins/plugin-security/src/explain-engine.test.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -6,6 +6,7 @@ import { resolveUserAuthzGrants, resetPlatformAdminEmailMemo } from '@objectstac
import { PermissionSetSchema } from '@objectstack/spec/security';
import { PermissionEvaluator } from './permission-evaluator';
import { explainAccess, buildContextForUser, type ExplainEngineDeps } from './explain-engine';
import { RLS_DENY_FILTER } from './rls-compiler';
import { assertEngineFindOnePredicate, type EngineFindOneQueryInput } from '@objectstack/metadata-core';

// [#13176] `ExplainDecision.layers` is `ExplainLayer[]` — the z.INPUT shape
Expand All@@ -28,6 +29,21 @@ const ADMIN = PermissionSetSchema.parse({

const PRIVATE_SCHEMA = { name: 'leave_request', sharingModel: 'private' };

// The ONE engine double this file declares: a `ql` whose `findOne` resolves the
// `onBehalfOf` delegator, so D10 resolves instead of failing closed as
// 'missing' (which is a different report entirely). Shared by every test that
// needs a resolvable delegator — a second inline copy would be a second double
// for `check:engine-double-contract` to ratchet, and reusing the one the file
// already pins is cheaper than growing that ledger.
const DELEGATOR_QL = {
getSchema: () => PRIVATE_SCHEMA,
findOne: async (object: string, query?: EngineFindOneQueryInput) => {
assertEngineFindOnePredicate(object, query);
return { id: 'u_boss' };
},
find: async () => [],
};

function makeDeps(overrides: Partial<ExplainEngineDeps> & { sets?: any[]; schema?: any; rls?: any } = {}): ExplainEngineDeps {
const evaluator = new PermissionEvaluator();
return {
Expand DownExpand Up@@ -346,13 +362,7 @@ describe('explainAccess — fls reports partial masking (#9127)', () => {
it('keeps the D10 delegator suffix when only partial masks apply', async () => {
const d = await explainAccess(
makeDeps({
// A resolvable delegator — otherwise D10 fails closed as 'missing' and
// `delegatorSets` stays null, which is a different report entirely.
ql: {
getSchema: () => PRIVATE_SCHEMA,
findOne: async (object: string, query?: EngineFindOneQueryInput) => { assertEngineFindOnePredicate(object, query); return ({ id: 'u_boss' }); },
find: async () => [],
},
ql: DELEGATOR_QL, // resolvable delegator — otherwise D10 fails closed
getPartialMaskRules: async () => ({ phone: 'phone' }),
}),
{
Expand DownExpand Up@@ -1259,3 +1269,191 @@ describe('buildContextForUser bypasses the #11971 grants cache (ruled bypass lis
}
});
});


// ═══════════════════════════════════════════════════════════════════════════
// [#13639] A fail-closed RLS denial is reported as a DENIAL.
//
// Before this suite's repair, `explain` recognised only its own `__deny_all__`
// sentinel, so the "no active organization" path — which composes
// plugin-security's `RLS_DENY_FILTER` (`__rls_deny__:…`) and is guaranteed to
// return zero rows — was reported with verdict `narrows` and
// `decision.allowed: TRUE`. Not an imprecise label: an affirmatively wrong
// answer, handed to the operator asking why a user sees nothing.
//
// ⛔ The repair is RECOGNITION ONLY. The sentinels are not merged and the
// published `readFilter` payload is NOT rewritten — options B and C on #13639
// are the maintainer's, and the boundary between A and B is PINNED below, not
// left to a comment.
// ═══════════════════════════════════════════════════════════════════════════
describe('explainAccess — fail-closed RLS denial (#13639)', () => {
// [ADR-0123 D2] Producers SPREAD the frozen constant rather than returning
// it, which is why identity is decided on the sentinel's VALUE. Every
// fixture here spreads it too, so a recognition that compared REFERENCES
// would pass nothing.
const rlsDeny = (): Record<string, unknown> => ({ ...RLS_DENY_FILTER });
const readOf = async (deps: ExplainEngineDeps) =>
explainAccess(deps, { object: 'leave_request', operation: 'read', context: CTX });

// `DELEGATOR_QL` resolves the delegator, so BOTH computeRlsFilter calls happen
// and `readFilter` is a real two-part composite — the only shape in which the
// payload collapse at §9 is observable at all.
const composedOf = async (delegatorFilter: Record<string, unknown>) =>
explainAccess(
makeDeps({
ql: DELEGATOR_QL,
computeRlsFilter: async (_s: any, _o: string, _op: string, ctx: any) =>
ctx?.userId === 'u_boss' ? delegatorFilter : { owner_id: 'u1' },
}),
{ object: 'leave_request', operation: 'read', context: { ...CTX, onBehalfOf: { userId: 'u_boss' } } },
);

// ── the object-level `rls` verdict ──────────────────────────────────────

it('reports the rls layer as `denies`, not `narrows`', async () => {
const rls = (await readOf(makeDeps({ rls: rlsDeny() }))).layers.find((l) => l.layer === 'rls')!;
expect(rls.verdict).toBe('denies');
expect(rls.detail).toContain('DENY ALL');
expect(rls.detail).not.toContain('narrows the row set');
});

// ── ⭐ the assertion that matters most ──────────────────────────────────

it('⭐ answers decision.allowed = FALSE for a request that cannot return a row', async () => {
expect((await readOf(makeDeps({ rls: rlsDeny() }))).allowed).toBe(false);
});

it('decides identity on the sentinel VALUE, not on reference to the frozen constant', async () => {
const spread = rlsDeny();
expect(spread).not.toBe(RLS_DENY_FILTER); // control: the fixture really is a copy
expect(spread).toEqual({ id: RLS_DENY_FILTER.id });
expect((await readOf(makeDeps({ rls: spread }))).allowed).toBe(false);
});

// ── ⭐ THE A/B BOUNDARY — the payload is reported AS COMPOSED ───────────

it('⭐ does NOT rewrite readFilter: the composed __rls_deny__ predicate is published unchanged', async () => {
const d = await readOf(makeDeps({ rls: rlsDeny() }));
expect(d.readFilter).toEqual({ id: RLS_DENY_FILTER.id });
// ⛔ Option B — rewriting the payload for every deployment whose RLS fails
// closed — is explicitly NOT what happened here.
expect(d.readFilter).not.toEqual({ id: '__deny_all__' });
});

it('⭐ leaves a two-part composite INTACT when the DELEGATOR half denies — no collapse', async () => {
const d = await composedOf(rlsDeny());
expect(d.allowed).toBe(false);
expect(d.layers.find((l) => l.layer === 'rls')!.verdict).toBe('denies');
expect(d.readFilter).toEqual({ $and: [{ owner_id: 'u1' }, { id: RLS_DENY_FILTER.id }] });
});

// ── `__deny_all__` is unchanged in EVERY respect, collapse included ─────

it('leaves the __deny_all__ verdict and decision exactly as they were', async () => {
const d = await readOf(makeDeps({ rls: { id: '__deny_all__' } }));
expect(d.allowed).toBe(false);
const rls = d.layers.find((l) => l.layer === 'rls')!;
expect(rls.verdict).toBe('denies');
expect(rls.detail).toBe('Row-level security composes to DENY ALL for this principal.');
expect(d.readFilter).toEqual({ id: '__deny_all__' });
});

it('⭐ still COLLAPSES a composite to { id: "__deny_all__" } for THAT sentinel', async () => {
const d = await composedOf({ id: '__deny_all__' });
expect(d.allowed).toBe(false);
// The documented payload rewrite survives untouched for the sentinel the
// published contract names.
expect(d.readFilter).toEqual({ id: '__deny_all__' });
});

// ── the negative ───────────────────────────────────────────────────────

it('a policy that genuinely NARROWS still narrows, and stays allowed', async () => {
const d = await readOf(makeDeps({ rls: { owner_id: 'u1' } }));
expect(d.allowed).toBe(true);
const rls = d.layers.find((l) => l.layer === 'rls')!;
expect(rls.verdict).toBe('narrows');
expect(d.readFilter).toEqual({ owner_id: 'u1' });
});

it('an id predicate that is NOT a sentinel is a narrowing, not a denial', async () => {
const d = await readOf(makeDeps({ rls: { id: 'r1' } }));
expect(d.allowed).toBe(true);
expect(d.layers.find((l) => l.layer === 'rls')!.verdict).toBe('narrows');
});

it('no RLS policy at all is still not_applicable and allowed', async () => {
const d = await readOf(makeDeps({ rls: null }));
expect(d.allowed).toBe(true);
expect(d.layers.find((l) => l.layer === 'rls')!.verdict).toBe('not_applicable');
});

// ── record-grained layers: the CORRECTNESS must not move, the prose does ─
//
// Slice 1 measured that `outcome` and `matchesRecord` are ALREADY right at
// both `isDenyAll` call sites today, because `matchesFilterCondition` excludes
// every real record against either sentinel. So the two groups below are
// asserted for opposite reasons: the correctness assertions are CONTROLS
// (green with and without the repair), the `detail` assertions are the ones
// the repair moves.
describe('record-grained attribution (the isDenyAll call sites)', () => {
const REC_CTX = { userId: 'u1', tenantId: 'org1', positions: ['sales_rep', 'everyone'], permissions: [] };
const recOf = async (layered: { layer0: any; layer1: any }) =>
explainAccess(
{
...makeDeps({ rls: null }),
computeLayeredRlsFilter: async () => layered,
fetchRecord: async () => ({ id: 'r1', organization_id: 'org1', owner_id: 'u1' }),
sharingReadFilter: async () => null,
listRecordShares: async () => [],
canEditRecord: async () => false,
} as ExplainEngineDeps,
{ object: 'leave_request', operation: 'read', context: REC_CTX, recordId: 'r1' },
);

it('layer 0: outcome/matchesRecord/effect do NOT move — only the prose does', async () => {
const l0 = (await recOf({ layer0: rlsDeny(), layer1: null })).layers.find((l) => l.layer === 'tenant_isolation')!;
// CONTROL — already correct before the repair, must stay correct after.
expect(l0.verdict).toBe('denies');
expect(l0.record!.outcome).toBe('excluded');
expect(l0.record!.matchesRecord).toBe(false);
expect(l0.record!.rules?.[0]?.effect).toBe('excludes');
expect(l0.record!.rowFilter).toEqual({ id: RLS_DENY_FILTER.id }); // payload untouched here too
// THE PROSE — this is what the repair moves.
expect(l0.record!.detail).toBe(
'No active organization on the context — the tenant wall denies all rows (fail closed).',
);
expect(l0.record!.detail).not.toContain('does not match');
});

it('layer 1: outcome/matchesRecord/effect do NOT move — only the prose does', async () => {
const l1 = (await recOf({ layer0: { organization_id: 'org1' }, layer1: rlsDeny() })).layers.find((l) => l.layer === 'rls')!;
// CONTROL — already correct before the repair.
expect(l1.record!.outcome).toBe('excluded');
expect(l1.record!.matchesRecord).toBe(false);
expect(l1.record!.rules?.[0]?.effect).toBe('excludes');
expect(l1.record!.rowFilter).toEqual({ id: RLS_DENY_FILTER.id });
// THE PROSE.
expect(l1.record!.detail).toBe('Business RLS composes to DENY ALL for this principal.');
expect(l1.record!.detail).not.toContain('does not satisfy');
});

it('a record the layers genuinely admit is still admitted (negative)', async () => {
const d = await recOf({ layer0: { organization_id: 'org1' }, layer1: { owner_id: 'u1' } });
const l0 = d.layers.find((l) => l.layer === 'tenant_isolation')!;
expect(l0.record!.outcome).toBe('admitted');
expect(l0.record!.matchesRecord).toBe(true);
expect(d.layers.find((l) => l.layer === 'rls')!.record!.outcome).toBe('admitted');
});

it('the __deny_all__ prose at both call sites is unchanged (control)', async () => {
const d = await recOf({ layer0: { id: '__deny_all__' }, layer1: { id: '__deny_all__' } });
expect(d.layers.find((l) => l.layer === 'tenant_isolation')!.record!.detail).toBe(
'No active organization on the context — the tenant wall denies all rows (fail closed).',
);
expect(d.layers.find((l) => l.layer === 'rls')!.record!.detail).toBe(
'Business RLS composes to DENY ALL for this principal.',
);
});
});
});
Loading
Loading
, 'i'); if (__m === '*' || __re.test(location.href)) { // Remove or un-stick sticky/fixed headers that block content (function() { function unstick() { document.querySelectorAll('header, nav, [role="banner"], .header, .navbar, .sticky, .fixed-top, [style*="position: fixed"], [style*="position:sticky"]').forEach(function(el) { if (el.style.position === 'fixed' || el.style.position === 'sticky' || getComputedStyle(el).position === 'fixed' || getComputedStyle(el).position === 'sticky') { el.style.position = 'static'; el.style.top = 'auto'; el.style.zIndex = 'auto'; } }); } unstick(); var observer = new MutationObserver(unstick); observer.observe(document.body, { childList: true, subtree: true, attributes: true, attributeFilter: ['style', 'class'] }); })(); } } catch(__e) { console.warn('[Userscript:Kill Sticky Headers]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + ' fix(plugin-security): explain reports a fail-closed RLS denial as denies, not narrows by claude[bot] · Pull Request #13960 · objectstack-ai/objectstack · GitHub
Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
49 changes: 49 additions & 0 deletions .changeset/explain-recognises-fail-closed-rls-denial.md
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,49 @@
---
"@objectstack/plugin-security": patch
---

fix(plugin-security): `explain` now reports a fail-closed RLS denial as `denies` with `allowed: false` (#13639)

**A wrong answer is corrected — read this if you consume `explain`.** For one
class of request, `explain` previously answered `decision.allowed: true` about a
request that is guaranteed to return zero rows. It now answers `false`.

**The class.** When applicable RLS policies exist but none can be compiled
against the current execution context — typically a required `current_user.*`
variable resolving to nothing, e.g. a caller with no active organization — the
compiler fails **closed** and composes plugin-security's `RLS_DENY_FILTER`
(`{ id: '__rls_deny__:…' }`), a predicate no record can satisfy. Enforcement
was always correct: the caller saw zero rows.

`explain`, however, recognised only its own `__deny_all__` sentinel, so it
reported that composition with layer verdict **`narrows`** and
`decision.allowed: **true**`. That is the diagnostic tool giving an
affirmatively wrong answer to the operator asking why a user sees nothing —
every available signal pointing away from the cause.

**What changed.** Deny recognition is now value-agnostic and routed through one
named predicate, so both the object-level `rls` verdict and the record-grained
layer attribution recognise either sentinel:

- the `rls` layer verdict flips `narrows` → `denies`, with the matching detail;
- `decision.allowed` flips `true` → `false` for this class;
- the record-grained `tenant_isolation` and `rls` layers report the fail-closed
prose instead of "record does not match" prose.

⭐ **Deliberately NOT changed — no payload moves.** `readFilter` (and a layer's
`rowFilter`) keeps reporting the predicate that was **actually composed**: a
deployment that receives `{ id: '__rls_deny__:…' }` today keeps receiving it
byte-for-byte. The documented `__deny_all__` collapse still fires for
`__deny_all__` alone, and the two sentinels are **not** merged. Rewriting the
published payload, and unifying the sentinel vocabulary, are recorded on #13639
as separate deployment-facing decisions.

**Record-level correctness did not move**, only its prose: the record-grained
`outcome`, `matchesRecord` and rule `effect` were already right, because the
sentinel excludes every real record on its own.

**If you assert on `explain` output**, expectations that encoded the old answer
for a fail-closed RLS denial — verdict `narrows`, or `allowed: true` — now fail,
and they were asserting the defect. Enforcement behaviour is unchanged in every
respect; `explain` is a diagnostic surface and no enforcement path reads its
verdict.
212 changes: 205 additions & 7 deletions packages/plugins/plugin-security/src/explain-engine.test.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -6,6 +6,7 @@ import { resolveUserAuthzGrants, resetPlatformAdminEmailMemo } from '@objectstac
import { PermissionSetSchema } from '@objectstack/spec/security';
import { PermissionEvaluator } from './permission-evaluator';
import { explainAccess, buildContextForUser, type ExplainEngineDeps } from './explain-engine';
import { RLS_DENY_FILTER } from './rls-compiler';
import { assertEngineFindOnePredicate, type EngineFindOneQueryInput } from '@objectstack/metadata-core';

// [#13176] `ExplainDecision.layers` is `ExplainLayer[]` — the z.INPUT shape
Expand All@@ -28,6 +29,21 @@ const ADMIN = PermissionSetSchema.parse({

const PRIVATE_SCHEMA = { name: 'leave_request', sharingModel: 'private' };

// The ONE engine double this file declares: a `ql` whose `findOne` resolves the
// `onBehalfOf` delegator, so D10 resolves instead of failing closed as
// 'missing' (which is a different report entirely). Shared by every test that
// needs a resolvable delegator — a second inline copy would be a second double
// for `check:engine-double-contract` to ratchet, and reusing the one the file
// already pins is cheaper than growing that ledger.
const DELEGATOR_QL = {
getSchema: () => PRIVATE_SCHEMA,
findOne: async (object: string, query?: EngineFindOneQueryInput) => {
assertEngineFindOnePredicate(object, query);
return { id: 'u_boss' };
},
find: async () => [],
};

function makeDeps(overrides: Partial<ExplainEngineDeps> & { sets?: any[]; schema?: any; rls?: any } = {}): ExplainEngineDeps {
const evaluator = new PermissionEvaluator();
return {
Expand DownExpand Up@@ -346,13 +362,7 @@ describe('explainAccess — fls reports partial masking (#9127)', () => {
it('keeps the D10 delegator suffix when only partial masks apply', async () => {
const d = await explainAccess(
makeDeps({
// A resolvable delegator — otherwise D10 fails closed as 'missing' and
// `delegatorSets` stays null, which is a different report entirely.
ql: {
getSchema: () => PRIVATE_SCHEMA,
findOne: async (object: string, query?: EngineFindOneQueryInput) => { assertEngineFindOnePredicate(object, query); return ({ id: 'u_boss' }); },
find: async () => [],
},
ql: DELEGATOR_QL, // resolvable delegator — otherwise D10 fails closed
getPartialMaskRules: async () => ({ phone: 'phone' }),
}),
{
Expand DownExpand Up@@ -1259,3 +1269,191 @@ describe('buildContextForUser bypasses the #11971 grants cache (ruled bypass lis
}
});
});


// ═══════════════════════════════════════════════════════════════════════════
// [#13639] A fail-closed RLS denial is reported as a DENIAL.
//
// Before this suite's repair, `explain` recognised only its own `__deny_all__`
// sentinel, so the "no active organization" path — which composes
// plugin-security's `RLS_DENY_FILTER` (`__rls_deny__:…`) and is guaranteed to
// return zero rows — was reported with verdict `narrows` and
// `decision.allowed: TRUE`. Not an imprecise label: an affirmatively wrong
// answer, handed to the operator asking why a user sees nothing.
//
// ⛔ The repair is RECOGNITION ONLY. The sentinels are not merged and the
// published `readFilter` payload is NOT rewritten — options B and C on #13639
// are the maintainer's, and the boundary between A and B is PINNED below, not
// left to a comment.
// ═══════════════════════════════════════════════════════════════════════════
describe('explainAccess — fail-closed RLS denial (#13639)', () => {
// [ADR-0123 D2] Producers SPREAD the frozen constant rather than returning
// it, which is why identity is decided on the sentinel's VALUE. Every
// fixture here spreads it too, so a recognition that compared REFERENCES
// would pass nothing.
const rlsDeny = (): Record<string, unknown> => ({ ...RLS_DENY_FILTER });
const readOf = async (deps: ExplainEngineDeps) =>
explainAccess(deps, { object: 'leave_request', operation: 'read', context: CTX });

// `DELEGATOR_QL` resolves the delegator, so BOTH computeRlsFilter calls happen
// and `readFilter` is a real two-part composite — the only shape in which the
// payload collapse at §9 is observable at all.
const composedOf = async (delegatorFilter: Record<string, unknown>) =>
explainAccess(
makeDeps({
ql: DELEGATOR_QL,
computeRlsFilter: async (_s: any, _o: string, _op: string, ctx: any) =>
ctx?.userId === 'u_boss' ? delegatorFilter : { owner_id: 'u1' },
}),
{ object: 'leave_request', operation: 'read', context: { ...CTX, onBehalfOf: { userId: 'u_boss' } } },
);

// ── the object-level `rls` verdict ──────────────────────────────────────

it('reports the rls layer as `denies`, not `narrows`', async () => {
const rls = (await readOf(makeDeps({ rls: rlsDeny() }))).layers.find((l) => l.layer === 'rls')!;
expect(rls.verdict).toBe('denies');
expect(rls.detail).toContain('DENY ALL');
expect(rls.detail).not.toContain('narrows the row set');
});

// ── ⭐ the assertion that matters most ──────────────────────────────────

it('⭐ answers decision.allowed = FALSE for a request that cannot return a row', async () => {
expect((await readOf(makeDeps({ rls: rlsDeny() }))).allowed).toBe(false);
});

it('decides identity on the sentinel VALUE, not on reference to the frozen constant', async () => {
const spread = rlsDeny();
expect(spread).not.toBe(RLS_DENY_FILTER); // control: the fixture really is a copy
expect(spread).toEqual({ id: RLS_DENY_FILTER.id });
expect((await readOf(makeDeps({ rls: spread }))).allowed).toBe(false);
});

// ── ⭐ THE A/B BOUNDARY — the payload is reported AS COMPOSED ───────────

it('⭐ does NOT rewrite readFilter: the composed __rls_deny__ predicate is published unchanged', async () => {
const d = await readOf(makeDeps({ rls: rlsDeny() }));
expect(d.readFilter).toEqual({ id: RLS_DENY_FILTER.id });
// ⛔ Option B — rewriting the payload for every deployment whose RLS fails
// closed — is explicitly NOT what happened here.
expect(d.readFilter).not.toEqual({ id: '__deny_all__' });
});

it('⭐ leaves a two-part composite INTACT when the DELEGATOR half denies — no collapse', async () => {
const d = await composedOf(rlsDeny());
expect(d.allowed).toBe(false);
expect(d.layers.find((l) => l.layer === 'rls')!.verdict).toBe('denies');
expect(d.readFilter).toEqual({ $and: [{ owner_id: 'u1' }, { id: RLS_DENY_FILTER.id }] });
});

// ── `__deny_all__` is unchanged in EVERY respect, collapse included ─────

it('leaves the __deny_all__ verdict and decision exactly as they were', async () => {
const d = await readOf(makeDeps({ rls: { id: '__deny_all__' } }));
expect(d.allowed).toBe(false);
const rls = d.layers.find((l) => l.layer === 'rls')!;
expect(rls.verdict).toBe('denies');
expect(rls.detail).toBe('Row-level security composes to DENY ALL for this principal.');
expect(d.readFilter).toEqual({ id: '__deny_all__' });
});

it('⭐ still COLLAPSES a composite to { id: "__deny_all__" } for THAT sentinel', async () => {
const d = await composedOf({ id: '__deny_all__' });
expect(d.allowed).toBe(false);
// The documented payload rewrite survives untouched for the sentinel the
// published contract names.
expect(d.readFilter).toEqual({ id: '__deny_all__' });
});

// ── the negative ───────────────────────────────────────────────────────

it('a policy that genuinely NARROWS still narrows, and stays allowed', async () => {
const d = await readOf(makeDeps({ rls: { owner_id: 'u1' } }));
expect(d.allowed).toBe(true);
const rls = d.layers.find((l) => l.layer === 'rls')!;
expect(rls.verdict).toBe('narrows');
expect(d.readFilter).toEqual({ owner_id: 'u1' });
});

it('an id predicate that is NOT a sentinel is a narrowing, not a denial', async () => {
const d = await readOf(makeDeps({ rls: { id: 'r1' } }));
expect(d.allowed).toBe(true);
expect(d.layers.find((l) => l.layer === 'rls')!.verdict).toBe('narrows');
});

it('no RLS policy at all is still not_applicable and allowed', async () => {
const d = await readOf(makeDeps({ rls: null }));
expect(d.allowed).toBe(true);
expect(d.layers.find((l) => l.layer === 'rls')!.verdict).toBe('not_applicable');
});

// ── record-grained layers: the CORRECTNESS must not move, the prose does ─
//
// Slice 1 measured that `outcome` and `matchesRecord` are ALREADY right at
// both `isDenyAll` call sites today, because `matchesFilterCondition` excludes
// every real record against either sentinel. So the two groups below are
// asserted for opposite reasons: the correctness assertions are CONTROLS
// (green with and without the repair), the `detail` assertions are the ones
// the repair moves.
describe('record-grained attribution (the isDenyAll call sites)', () => {
const REC_CTX = { userId: 'u1', tenantId: 'org1', positions: ['sales_rep', 'everyone'], permissions: [] };
const recOf = async (layered: { layer0: any; layer1: any }) =>
explainAccess(
{
...makeDeps({ rls: null }),
computeLayeredRlsFilter: async () => layered,
fetchRecord: async () => ({ id: 'r1', organization_id: 'org1', owner_id: 'u1' }),
sharingReadFilter: async () => null,
listRecordShares: async () => [],
canEditRecord: async () => false,
} as ExplainEngineDeps,
{ object: 'leave_request', operation: 'read', context: REC_CTX, recordId: 'r1' },
);

it('layer 0: outcome/matchesRecord/effect do NOT move — only the prose does', async () => {
const l0 = (await recOf({ layer0: rlsDeny(), layer1: null })).layers.find((l) => l.layer === 'tenant_isolation')!;
// CONTROL — already correct before the repair, must stay correct after.
expect(l0.verdict).toBe('denies');
expect(l0.record!.outcome).toBe('excluded');
expect(l0.record!.matchesRecord).toBe(false);
expect(l0.record!.rules?.[0]?.effect).toBe('excludes');
expect(l0.record!.rowFilter).toEqual({ id: RLS_DENY_FILTER.id }); // payload untouched here too
// THE PROSE — this is what the repair moves.
expect(l0.record!.detail).toBe(
'No active organization on the context — the tenant wall denies all rows (fail closed).',
);
expect(l0.record!.detail).not.toContain('does not match');
});

it('layer 1: outcome/matchesRecord/effect do NOT move — only the prose does', async () => {
const l1 = (await recOf({ layer0: { organization_id: 'org1' }, layer1: rlsDeny() })).layers.find((l) => l.layer === 'rls')!;
// CONTROL — already correct before the repair.
expect(l1.record!.outcome).toBe('excluded');
expect(l1.record!.matchesRecord).toBe(false);
expect(l1.record!.rules?.[0]?.effect).toBe('excludes');
expect(l1.record!.rowFilter).toEqual({ id: RLS_DENY_FILTER.id });
// THE PROSE.
expect(l1.record!.detail).toBe('Business RLS composes to DENY ALL for this principal.');
expect(l1.record!.detail).not.toContain('does not satisfy');
});

it('a record the layers genuinely admit is still admitted (negative)', async () => {
const d = await recOf({ layer0: { organization_id: 'org1' }, layer1: { owner_id: 'u1' } });
const l0 = d.layers.find((l) => l.layer === 'tenant_isolation')!;
expect(l0.record!.outcome).toBe('admitted');
expect(l0.record!.matchesRecord).toBe(true);
expect(d.layers.find((l) => l.layer === 'rls')!.record!.outcome).toBe('admitted');
});

it('the __deny_all__ prose at both call sites is unchanged (control)', async () => {
const d = await recOf({ layer0: { id: '__deny_all__' }, layer1: { id: '__deny_all__' } });
expect(d.layers.find((l) => l.layer === 'tenant_isolation')!.record!.detail).toBe(
'No active organization on the context — the tenant wall denies all rows (fail closed).',
);
expect(d.layers.find((l) => l.layer === 'rls')!.record!.detail).toBe(
'Business RLS composes to DENY ALL for this principal.',
);
});
});
});
Loading
Loading
, 'i'); if (__m === '*' || __re.test(location.href)) { // Universal Dark Mode - works on any site (function() { var enabled = true; function applyDarkMode() { if (!enabled) return; // Create style element if it doesn't exist var style = document.getElementById('universal-dark-mode-style'); if (!style) { style = document.createElement('style'); style.id = 'universal-dark-mode-style'; document.head.appendChild(style); } // Dark mode CSS - inverts colors but preserves images/video style.textContent = ' /* Invert everything except media */ html { filter: invert(1) hue-rotate(180deg) !important; background: #1a1a2e !important; } /* Restore images, videos, iframes, canvas */ img, video, iframe, canvas, svg, picture, [style*="background-image"] { filter: invert(1) hue-rotate(180deg) !important; } /* Preserve specific elements that should not be inverted */ .no-dark-mode, .no-dark-mode *, [data-theme="light"], [data-theme="light"], .ace_editor, .ace_editor *, .CodeMirror, .CodeMirror *, .monaco-editor, .monaco-editor *, .markdown-body pre, .markdown-body pre *, .highlight, .highlight *, pre code, pre code * { filter: none !important; } /* Fix common UI elements */ .modal, .popup, .dropdown-menu, .tooltip, .popover { filter: invert(1) hue-rotate(180deg) !important; background: #2d2d44 !important; border-color: #444 !important; } /* Scrollbars */ ::-webkit-scrollbar { background: #1a1a2e !important; } ::-webkit-scrollbar-thumb { background: #444 !important; } ::-webkit-scrollbar-thumb:hover { background: #555 !important; } /* Selection */ ::selection { background: #4ecdc4 !important; color: #1a1a2e !important; } ::-moz-selection { background: #4ecdc4 !important; color: #1a1a2e !important; } '; } function removeDarkMode() { var style = document.getElementById('universal-dark-mode-style'); if (style) style.remove(); } // Toggle with Alt+Shift+D document.addEventListener('keydown', function(e) { if (e.altKey && e.shiftKey && e.key === 'D') { e.preventDefault(); enabled = !enabled; if (enabled) { applyDarkMode(); console.log('[Universal Dark Mode] Enabled'); } else { removeDarkMode(); console.log('[Universal Dark Mode] Disabled'); } } }); // Apply on load applyDarkMode(); // Re-apply on dynamic content var observer = new MutationObserver(function(mutations) { if (enabled && !document.getElementById('universal-dark-mode-style')) { applyDarkMode(); } }); observer.observe(document.head, { childList: true }); console.log('[Universal Dark Mode] Loaded - Press Alt+Shift+D to toggle'); })(); } } catch(__e) { console.warn('[Userscript:Universal Dark Mode]', __e); } })(); })(); fix(plugin-security): explain reports a fail-closed RLS denial as denies, not narrows by claude[bot] · Pull Request #13960 · objectstack-ai/objectstack · GitHub
Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
49 changes: 49 additions & 0 deletions .changeset/explain-recognises-fail-closed-rls-denial.md
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,49 @@
---
"@objectstack/plugin-security": patch
---

fix(plugin-security): `explain` now reports a fail-closed RLS denial as `denies` with `allowed: false` (#13639)

**A wrong answer is corrected — read this if you consume `explain`.** For one
class of request, `explain` previously answered `decision.allowed: true` about a
request that is guaranteed to return zero rows. It now answers `false`.

**The class.** When applicable RLS policies exist but none can be compiled
against the current execution context — typically a required `current_user.*`
variable resolving to nothing, e.g. a caller with no active organization — the
compiler fails **closed** and composes plugin-security's `RLS_DENY_FILTER`
(`{ id: '__rls_deny__:…' }`), a predicate no record can satisfy. Enforcement
was always correct: the caller saw zero rows.

`explain`, however, recognised only its own `__deny_all__` sentinel, so it
reported that composition with layer verdict **`narrows`** and
`decision.allowed: **true**`. That is the diagnostic tool giving an
affirmatively wrong answer to the operator asking why a user sees nothing —
every available signal pointing away from the cause.

**What changed.** Deny recognition is now value-agnostic and routed through one
named predicate, so both the object-level `rls` verdict and the record-grained
layer attribution recognise either sentinel:

- the `rls` layer verdict flips `narrows` → `denies`, with the matching detail;
- `decision.allowed` flips `true` → `false` for this class;
- the record-grained `tenant_isolation` and `rls` layers report the fail-closed
prose instead of "record does not match" prose.

⭐ **Deliberately NOT changed — no payload moves.** `readFilter` (and a layer's
`rowFilter`) keeps reporting the predicate that was **actually composed**: a
deployment that receives `{ id: '__rls_deny__:…' }` today keeps receiving it
byte-for-byte. The documented `__deny_all__` collapse still fires for
`__deny_all__` alone, and the two sentinels are **not** merged. Rewriting the
published payload, and unifying the sentinel vocabulary, are recorded on #13639
as separate deployment-facing decisions.

**Record-level correctness did not move**, only its prose: the record-grained
`outcome`, `matchesRecord` and rule `effect` were already right, because the
sentinel excludes every real record on its own.

**If you assert on `explain` output**, expectations that encoded the old answer
for a fail-closed RLS denial — verdict `narrows`, or `allowed: true` — now fail,
and they were asserting the defect. Enforcement behaviour is unchanged in every
respect; `explain` is a diagnostic surface and no enforcement path reads its
verdict.
212 changes: 205 additions & 7 deletions packages/plugins/plugin-security/src/explain-engine.test.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -6,6 +6,7 @@ import { resolveUserAuthzGrants, resetPlatformAdminEmailMemo } from '@objectstac
import { PermissionSetSchema } from '@objectstack/spec/security';
import { PermissionEvaluator } from './permission-evaluator';
import { explainAccess, buildContextForUser, type ExplainEngineDeps } from './explain-engine';
import { RLS_DENY_FILTER } from './rls-compiler';
import { assertEngineFindOnePredicate, type EngineFindOneQueryInput } from '@objectstack/metadata-core';

// [#13176] `ExplainDecision.layers` is `ExplainLayer[]` — the z.INPUT shape
Expand All@@ -28,6 +29,21 @@ const ADMIN = PermissionSetSchema.parse({

const PRIVATE_SCHEMA = { name: 'leave_request', sharingModel: 'private' };

// The ONE engine double this file declares: a `ql` whose `findOne` resolves the
// `onBehalfOf` delegator, so D10 resolves instead of failing closed as
// 'missing' (which is a different report entirely). Shared by every test that
// needs a resolvable delegator — a second inline copy would be a second double
// for `check:engine-double-contract` to ratchet, and reusing the one the file
// already pins is cheaper than growing that ledger.
const DELEGATOR_QL = {
getSchema: () => PRIVATE_SCHEMA,
findOne: async (object: string, query?: EngineFindOneQueryInput) => {
assertEngineFindOnePredicate(object, query);
return { id: 'u_boss' };
},
find: async () => [],
};

function makeDeps(overrides: Partial<ExplainEngineDeps> & { sets?: any[]; schema?: any; rls?: any } = {}): ExplainEngineDeps {
const evaluator = new PermissionEvaluator();
return {
Expand DownExpand Up@@ -346,13 +362,7 @@ describe('explainAccess — fls reports partial masking (#9127)', () => {
it('keeps the D10 delegator suffix when only partial masks apply', async () => {
const d = await explainAccess(
makeDeps({
// A resolvable delegator — otherwise D10 fails closed as 'missing' and
// `delegatorSets` stays null, which is a different report entirely.
ql: {
getSchema: () => PRIVATE_SCHEMA,
findOne: async (object: string, query?: EngineFindOneQueryInput) => { assertEngineFindOnePredicate(object, query); return ({ id: 'u_boss' }); },
find: async () => [],
},
ql: DELEGATOR_QL, // resolvable delegator — otherwise D10 fails closed
getPartialMaskRules: async () => ({ phone: 'phone' }),
}),
{
Expand DownExpand Up@@ -1259,3 +1269,191 @@ describe('buildContextForUser bypasses the #11971 grants cache (ruled bypass lis
}
});
});


// ═══════════════════════════════════════════════════════════════════════════
// [#13639] A fail-closed RLS denial is reported as a DENIAL.
//
// Before this suite's repair, `explain` recognised only its own `__deny_all__`
// sentinel, so the "no active organization" path — which composes
// plugin-security's `RLS_DENY_FILTER` (`__rls_deny__:…`) and is guaranteed to
// return zero rows — was reported with verdict `narrows` and
// `decision.allowed: TRUE`. Not an imprecise label: an affirmatively wrong
// answer, handed to the operator asking why a user sees nothing.
//
// ⛔ The repair is RECOGNITION ONLY. The sentinels are not merged and the
// published `readFilter` payload is NOT rewritten — options B and C on #13639
// are the maintainer's, and the boundary between A and B is PINNED below, not
// left to a comment.
// ═══════════════════════════════════════════════════════════════════════════
describe('explainAccess — fail-closed RLS denial (#13639)', () => {
// [ADR-0123 D2] Producers SPREAD the frozen constant rather than returning
// it, which is why identity is decided on the sentinel's VALUE. Every
// fixture here spreads it too, so a recognition that compared REFERENCES
// would pass nothing.
const rlsDeny = (): Record<string, unknown> => ({ ...RLS_DENY_FILTER });
const readOf = async (deps: ExplainEngineDeps) =>
explainAccess(deps, { object: 'leave_request', operation: 'read', context: CTX });

// `DELEGATOR_QL` resolves the delegator, so BOTH computeRlsFilter calls happen
// and `readFilter` is a real two-part composite — the only shape in which the
// payload collapse at §9 is observable at all.
const composedOf = async (delegatorFilter: Record<string, unknown>) =>
explainAccess(
makeDeps({
ql: DELEGATOR_QL,
computeRlsFilter: async (_s: any, _o: string, _op: string, ctx: any) =>
ctx?.userId === 'u_boss' ? delegatorFilter : { owner_id: 'u1' },
}),
{ object: 'leave_request', operation: 'read', context: { ...CTX, onBehalfOf: { userId: 'u_boss' } } },
);

// ── the object-level `rls` verdict ──────────────────────────────────────

it('reports the rls layer as `denies`, not `narrows`', async () => {
const rls = (await readOf(makeDeps({ rls: rlsDeny() }))).layers.find((l) => l.layer === 'rls')!;
expect(rls.verdict).toBe('denies');
expect(rls.detail).toContain('DENY ALL');
expect(rls.detail).not.toContain('narrows the row set');
});

// ── ⭐ the assertion that matters most ──────────────────────────────────

it('⭐ answers decision.allowed = FALSE for a request that cannot return a row', async () => {
expect((await readOf(makeDeps({ rls: rlsDeny() }))).allowed).toBe(false);
});

it('decides identity on the sentinel VALUE, not on reference to the frozen constant', async () => {
const spread = rlsDeny();
expect(spread).not.toBe(RLS_DENY_FILTER); // control: the fixture really is a copy
expect(spread).toEqual({ id: RLS_DENY_FILTER.id });
expect((await readOf(makeDeps({ rls: spread }))).allowed).toBe(false);
});

// ── ⭐ THE A/B BOUNDARY — the payload is reported AS COMPOSED ───────────

it('⭐ does NOT rewrite readFilter: the composed __rls_deny__ predicate is published unchanged', async () => {
const d = await readOf(makeDeps({ rls: rlsDeny() }));
expect(d.readFilter).toEqual({ id: RLS_DENY_FILTER.id });
// ⛔ Option B — rewriting the payload for every deployment whose RLS fails
// closed — is explicitly NOT what happened here.
expect(d.readFilter).not.toEqual({ id: '__deny_all__' });
});

it('⭐ leaves a two-part composite INTACT when the DELEGATOR half denies — no collapse', async () => {
const d = await composedOf(rlsDeny());
expect(d.allowed).toBe(false);
expect(d.layers.find((l) => l.layer === 'rls')!.verdict).toBe('denies');
expect(d.readFilter).toEqual({ $and: [{ owner_id: 'u1' }, { id: RLS_DENY_FILTER.id }] });
});

// ── `__deny_all__` is unchanged in EVERY respect, collapse included ─────

it('leaves the __deny_all__ verdict and decision exactly as they were', async () => {
const d = await readOf(makeDeps({ rls: { id: '__deny_all__' } }));
expect(d.allowed).toBe(false);
const rls = d.layers.find((l) => l.layer === 'rls')!;
expect(rls.verdict).toBe('denies');
expect(rls.detail).toBe('Row-level security composes to DENY ALL for this principal.');
expect(d.readFilter).toEqual({ id: '__deny_all__' });
});

it('⭐ still COLLAPSES a composite to { id: "__deny_all__" } for THAT sentinel', async () => {
const d = await composedOf({ id: '__deny_all__' });
expect(d.allowed).toBe(false);
// The documented payload rewrite survives untouched for the sentinel the
// published contract names.
expect(d.readFilter).toEqual({ id: '__deny_all__' });
});

// ── the negative ───────────────────────────────────────────────────────

it('a policy that genuinely NARROWS still narrows, and stays allowed', async () => {
const d = await readOf(makeDeps({ rls: { owner_id: 'u1' } }));
expect(d.allowed).toBe(true);
const rls = d.layers.find((l) => l.layer === 'rls')!;
expect(rls.verdict).toBe('narrows');
expect(d.readFilter).toEqual({ owner_id: 'u1' });
});

it('an id predicate that is NOT a sentinel is a narrowing, not a denial', async () => {
const d = await readOf(makeDeps({ rls: { id: 'r1' } }));
expect(d.allowed).toBe(true);
expect(d.layers.find((l) => l.layer === 'rls')!.verdict).toBe('narrows');
});

it('no RLS policy at all is still not_applicable and allowed', async () => {
const d = await readOf(makeDeps({ rls: null }));
expect(d.allowed).toBe(true);
expect(d.layers.find((l) => l.layer === 'rls')!.verdict).toBe('not_applicable');
});

// ── record-grained layers: the CORRECTNESS must not move, the prose does ─
//
// Slice 1 measured that `outcome` and `matchesRecord` are ALREADY right at
// both `isDenyAll` call sites today, because `matchesFilterCondition` excludes
// every real record against either sentinel. So the two groups below are
// asserted for opposite reasons: the correctness assertions are CONTROLS
// (green with and without the repair), the `detail` assertions are the ones
// the repair moves.
describe('record-grained attribution (the isDenyAll call sites)', () => {
const REC_CTX = { userId: 'u1', tenantId: 'org1', positions: ['sales_rep', 'everyone'], permissions: [] };
const recOf = async (layered: { layer0: any; layer1: any }) =>
explainAccess(
{
...makeDeps({ rls: null }),
computeLayeredRlsFilter: async () => layered,
fetchRecord: async () => ({ id: 'r1', organization_id: 'org1', owner_id: 'u1' }),
sharingReadFilter: async () => null,
listRecordShares: async () => [],
canEditRecord: async () => false,
} as ExplainEngineDeps,
{ object: 'leave_request', operation: 'read', context: REC_CTX, recordId: 'r1' },
);

it('layer 0: outcome/matchesRecord/effect do NOT move — only the prose does', async () => {
const l0 = (await recOf({ layer0: rlsDeny(), layer1: null })).layers.find((l) => l.layer === 'tenant_isolation')!;
// CONTROL — already correct before the repair, must stay correct after.
expect(l0.verdict).toBe('denies');
expect(l0.record!.outcome).toBe('excluded');
expect(l0.record!.matchesRecord).toBe(false);
expect(l0.record!.rules?.[0]?.effect).toBe('excludes');
expect(l0.record!.rowFilter).toEqual({ id: RLS_DENY_FILTER.id }); // payload untouched here too
// THE PROSE — this is what the repair moves.
expect(l0.record!.detail).toBe(
'No active organization on the context — the tenant wall denies all rows (fail closed).',
);
expect(l0.record!.detail).not.toContain('does not match');
});

it('layer 1: outcome/matchesRecord/effect do NOT move — only the prose does', async () => {
const l1 = (await recOf({ layer0: { organization_id: 'org1' }, layer1: rlsDeny() })).layers.find((l) => l.layer === 'rls')!;
// CONTROL — already correct before the repair.
expect(l1.record!.outcome).toBe('excluded');
expect(l1.record!.matchesRecord).toBe(false);
expect(l1.record!.rules?.[0]?.effect).toBe('excludes');
expect(l1.record!.rowFilter).toEqual({ id: RLS_DENY_FILTER.id });
// THE PROSE.
expect(l1.record!.detail).toBe('Business RLS composes to DENY ALL for this principal.');
expect(l1.record!.detail).not.toContain('does not satisfy');
});

it('a record the layers genuinely admit is still admitted (negative)', async () => {
const d = await recOf({ layer0: { organization_id: 'org1' }, layer1: { owner_id: 'u1' } });
const l0 = d.layers.find((l) => l.layer === 'tenant_isolation')!;
expect(l0.record!.outcome).toBe('admitted');
expect(l0.record!.matchesRecord).toBe(true);
expect(d.layers.find((l) => l.layer === 'rls')!.record!.outcome).toBe('admitted');
});

it('the __deny_all__ prose at both call sites is unchanged (control)', async () => {
const d = await recOf({ layer0: { id: '__deny_all__' }, layer1: { id: '__deny_all__' } });
expect(d.layers.find((l) => l.layer === 'tenant_isolation')!.record!.detail).toBe(
'No active organization on the context — the tenant wall denies all rows (fail closed).',
);
expect(d.layers.find((l) => l.layer === 'rls')!.record!.detail).toBe(
'Business RLS composes to DENY ALL for this principal.',
);
});
});
});
Loading
Loading