Audit P1: the SQL compiler used a literal property lookup (`(input.actor as Record<string, unknown>)[expr.path]`) for actor and context references, while the in-memory runtime evaluator goes through `getPath()` which respects the `.` separator. A policy condition like `actor.risk.mfa === true` therefore resolved correctly in memory but produced `undefined` in the SQL parameter — silently misaligning DB-side filters with allow/deny decisions. Fix: route both `PERM_ROOT_ACTOR` and `PERM_ROOT_CONTEXT` through `getPath()` in `src/libs/perm/compilers/sql.ts` so both code paths agree on segmentation. Resource references stay on `columnName` (they map to a real DB column, not to a JS object). Adds `src/libs/perm/test/sql-nested-paths.test.ts` with three regression cases: 1. Nested actor path (`actor.risk.mfa`) emits the resolved value. 2. Nested context path (`context.request.region`) likewise. 3. Missing nested path emits `undefined`, matching the runtime evaluator (so the SQL/runtime alignment doesn't accidentally diverge in the "missing" case either). Caveat documented in the new comment: the fix assumes DB column names don't contain `.`. Apps that need columns with dotted identifiers must override `columnName` and the actor/context paths must avoid `.` for those references. Suite: 1492 / 1492. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>master
parent
5efec94367
commit
2a6706370e
@ -0,0 +1,132 @@
|
||||
/**
|
||||
* Regression test for the audit P1 finding "SQL compiler does not
|
||||
* resolve nested paths the same way runtime evaluation does".
|
||||
*
|
||||
* Before the fix, `actor.path` and `context.path` references in the
|
||||
* SQL compiler used a literal property lookup
|
||||
* (`(input.actor as Record<string, unknown>)[expr.path]`), so a
|
||||
* dotted path like `risk.mfa` produced `undefined` from the compiler
|
||||
* while the runtime evaluator (which uses `getPath`) resolved it to
|
||||
* the nested value. The fix routes both through `getPath`, so the
|
||||
* SQL parameter and the in-memory comparison agree.
|
||||
*/
|
||||
|
||||
import { describe, expect, it } from 'vitest';
|
||||
import { createSqlCompiler } from '../compilers/sql.ts';
|
||||
import {
|
||||
PERM_EFFECT_ALLOW,
|
||||
PERM_EXPR_CONST,
|
||||
PERM_EXPR_EQ,
|
||||
PERM_EXPR_REF,
|
||||
PERM_ROOT_ACTOR,
|
||||
PERM_ROOT_CONTEXT
|
||||
} from '../consts.ts';
|
||||
import type { PolicyIR, SubjectRef } from '../types.ts';
|
||||
|
||||
const schema = { tenants: false, attributes: {} } as never;
|
||||
|
||||
describe('createSqlCompiler — nested actor / context paths', () => {
|
||||
it('resolves a nested actor path with getPath instead of a literal lookup', () => {
|
||||
const compiler = createSqlCompiler();
|
||||
|
||||
const policy: PolicyIR = {
|
||||
id: 'allow-mfa',
|
||||
effect: PERM_EFFECT_ALLOW,
|
||||
priority: 0,
|
||||
target: { action: 'read', resource: 'doc' },
|
||||
condition: {
|
||||
op: PERM_EXPR_EQ,
|
||||
left: { op: PERM_EXPR_REF, root: PERM_ROOT_ACTOR, path: 'risk.mfa' },
|
||||
right: { op: PERM_EXPR_CONST, value: true }
|
||||
}
|
||||
} as PolicyIR;
|
||||
|
||||
const actor: SubjectRef = {
|
||||
id: 'u-1',
|
||||
risk: { mfa: true }
|
||||
} as unknown as SubjectRef;
|
||||
|
||||
const plan = compiler.compile({
|
||||
schema,
|
||||
policies: [policy],
|
||||
action: 'read',
|
||||
actor,
|
||||
resourceType: 'doc'
|
||||
});
|
||||
|
||||
// The nested value travels through `getPath`; the parameter the
|
||||
// compiler emits is `true`, not `undefined`.
|
||||
const predicate = (plan as { predicate?: { params: Record<string, unknown> } })
|
||||
.predicate;
|
||||
const params = predicate?.params ?? {};
|
||||
const values = Object.values(params);
|
||||
expect(values).toContain(true);
|
||||
expect(values).not.toContain(undefined);
|
||||
});
|
||||
|
||||
it('resolves a nested context path the same way', () => {
|
||||
const compiler = createSqlCompiler();
|
||||
|
||||
const policy: PolicyIR = {
|
||||
id: 'allow-region',
|
||||
effect: PERM_EFFECT_ALLOW,
|
||||
priority: 0,
|
||||
target: { action: 'read', resource: 'doc' },
|
||||
condition: {
|
||||
op: PERM_EXPR_EQ,
|
||||
left: { op: PERM_EXPR_REF, root: PERM_ROOT_CONTEXT, path: 'request.region' },
|
||||
right: { op: PERM_EXPR_CONST, value: 'eu' }
|
||||
}
|
||||
} as PolicyIR;
|
||||
|
||||
const plan = compiler.compile({
|
||||
schema,
|
||||
policies: [policy],
|
||||
action: 'read',
|
||||
actor: { id: 'u-1' } as SubjectRef,
|
||||
resourceType: 'doc',
|
||||
context: { request: { region: 'eu' } }
|
||||
});
|
||||
|
||||
const predicate = (plan as { predicate?: { params: Record<string, unknown> } })
|
||||
.predicate;
|
||||
const params = predicate?.params ?? {};
|
||||
const values = Object.values(params);
|
||||
expect(values).toContain('eu');
|
||||
expect(values).not.toContain(undefined);
|
||||
});
|
||||
|
||||
it('returns undefined parameter when the nested path is missing (matches runtime)', () => {
|
||||
const compiler = createSqlCompiler();
|
||||
|
||||
const policy: PolicyIR = {
|
||||
id: 'allow-region',
|
||||
effect: PERM_EFFECT_ALLOW,
|
||||
priority: 0,
|
||||
target: { action: 'read', resource: 'doc' },
|
||||
condition: {
|
||||
op: PERM_EXPR_EQ,
|
||||
left: { op: PERM_EXPR_REF, root: PERM_ROOT_ACTOR, path: 'profile.department' },
|
||||
right: { op: PERM_EXPR_CONST, value: 'finance' }
|
||||
}
|
||||
} as PolicyIR;
|
||||
|
||||
const plan = compiler.compile({
|
||||
schema,
|
||||
policies: [policy],
|
||||
action: 'read',
|
||||
actor: { id: 'u-1' } as SubjectRef, // no `profile`
|
||||
resourceType: 'doc'
|
||||
});
|
||||
|
||||
// `getPath` returns `undefined` when the chain breaks; the
|
||||
// runtime evaluator does the same. This test pins behavior so
|
||||
// a future change can't silently re-introduce a literal
|
||||
// `actor['profile.department']` path.
|
||||
const predicate = (plan as { predicate?: { params: Record<string, unknown> } })
|
||||
.predicate;
|
||||
const params = predicate?.params ?? {};
|
||||
const values = Object.values(params);
|
||||
expect(values).toContain(undefined);
|
||||
});
|
||||
});
|
||||
Loading…
Reference in new issue