Skip to content

Commit 213d2f1

Browse files
committed
fix(plugin-security): explain reports the refusal enforcement gives a policy comparing two fields of no shared class
WIP: fix in explain-engine.ts; pins follow. Claude-Session: https://claude.ai/code/session_01XY5uCwTjZj7884yYtyur4H Co-authored-by: Claude <noreply@anthropic.com>
1 parent f11b5f2 commit 213d2f1

1 file changed

Lines changed: 115 additions & 19 deletions

File tree

‎packages/plugins/plugin-security/src/explain-engine.ts‎

Lines changed: 115 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,13 @@ import {
2626
derivePosture as deriveAdminPosture,
2727
resolveUserAuthzGrants,
2828
} from '@objectstack/core';
29-
import { matchesFilterCondition } from '@objectstack/formula';
29+
import {
30+
crossFieldClassRefusalCarriedBy,
31+
findCrossFieldClassRefusal,
32+
matchesFilterCondition,
33+
type CrossFieldClassRefusal,
34+
type MatchesFilterOptions,
35+
} from '@objectstack/formula';
3036
import { ORGANIZATION_ADMIN_GRANTS } from '@objectstack/spec';
3137
import type { FieldMaskingRule } from '@objectstack/spec/data';
3238
import type { PermissionSet } from '@objectstack/spec/security';
@@ -41,7 +47,7 @@ import type {
4147
import type { PermissionEvaluator } from './permission-evaluator.js';
4248
import { superuserBypassBitForOperation } from './permission-evaluator.js';
4349
import { ExplainObjectNotFoundError } from './errors.js';
44-
import { RLS_DENY_FILTER } from './rls-compiler.js';
50+
import { RLS_DENY_FILTER, compiledPolicyNameOf } from './rls-compiler.js';
4551
import {
4652
unresolvedPostureExplainDetail,
4753
type UnresolvedPostureCause,
@@ -843,6 +849,45 @@ function describeOwd(schema: any): { model: string; declared: boolean; effect: '
843849
return { model: `${String(m)} (unknown → private, fail-closed)`, declared: true, effect: 'private' };
844850
}
845851

852+
/**
853+
* [#20431] The object's declared columns, handed to the record matcher so it
854+
* applies the spec's cross-field comparison class (`crossFieldComparisonVerdict`)
855+
* — the rule driver-sql applies when it compiles the find this report explains,
856+
* and the rule the RLS write check applies to the same policy (`options.fields`
857+
* of `matchesFilterCondition`, #20355).
858+
*
859+
* Read off `ql.getSchema`, the schema the engine already reads for the OWD: the
860+
* ObjectQL registry is the declaration the find's driver compiles against. A
861+
* schema that cannot be read hands over no columns, and the matcher then judges
862+
* values only, as it did before — a missing schema never manufactures a refusal.
863+
*/
864+
function declaredColumnsOf(schema: any): MatchesFilterOptions | undefined {
865+
const fields = schema?.fields;
866+
if (!fields || typeof fields !== 'object' || Array.isArray(fields)) return undefined;
867+
return { fields };
868+
}
869+
870+
/**
871+
* [#20431] The names of the policies in a compiled business-RLS filter that
872+
* carry a refused field-to-field comparison — the attribution the RLS write
873+
* check logs for the same refusal. The composed filter is one policy's filter or
874+
* `{ $or: [...] }` of them, and `compiledPolicyNameOf` recognises each by
875+
* identity.
876+
*/
877+
function refusedPolicyNamesOf(
878+
filter: Record<string, unknown>,
879+
fields: NonNullable<MatchesFilterOptions['fields']>,
880+
): string[] {
881+
const members =
882+
compiledPolicyNameOf(filter) === undefined && Array.isArray((filter as { $or?: unknown }).$or)
883+
? ((filter as { $or: unknown[] }).$or)
884+
: [filter];
885+
const names = members
886+
.filter((m) => findCrossFieldClassRefusal(m as Record<string, unknown>, fields) !== null)
887+
.map((m) => compiledPolicyNameOf(m) ?? '(unnamed)');
888+
return [...new Set(names)];
889+
}
890+
846891
/**
847892
* [C2 / ADR-0095] Inputs the record-grained augmentation needs from the already
848893
* computed object-level pass — the row story is decomposed FROM the same facts,
@@ -869,6 +914,8 @@ interface RecordAttributionContext {
869914
vamaEffective: boolean;
870915
/** [#4647] The sets that carry the bypass, for the row-level detail text. */
871916
vamaSets: string[];
917+
/** [#20431] The object's declared columns ({@link declaredColumnsOf}); absent → values only. */
918+
declaredColumns: MatchesFilterOptions | undefined;
872919
}
873920

874921
/**
@@ -880,25 +927,39 @@ interface RecordAttributionContext {
880927
* - Layer 0 / Layer 1 filters come from `computeLayeredRlsFilter` (the middleware's
881928
* own `Layer0(tenant) AND Layer1(business)` split);
882929
* - "does THIS record satisfy the filter?" is `matchesFilterCondition` — the
883-
* third canonical backend for the same FilterCondition shape the query runs;
930+
* third canonical backend for the same FilterCondition shape the query runs —
931+
* handed the object's declared columns, so it refuses what the query refuses;
884932
* - the write verdict is the sharing service's own `canEdit`.
885933
* So the record story is explained by construction, exactly like the object-level pass.
886934
*/
887935
async function applyRecordAttribution(
888936
ra: RecordAttributionContext,
889937
): Promise<{ record: NonNullable<ExplainDecision['record']>; posture: AuthzPosture }> {
890-
const { deps, object, recordId, engineOp, context, sets, layers, owd, capsDeny, crudAllowed, vamaEffective, vamaSets } = ra;
938+
const {
939+
deps, object, recordId, engineOp, context, sets, layers, owd, capsDeny, crudAllowed, vamaEffective, vamaSets,
940+
declaredColumns,
941+
} = ra;
891942
const isRead = engineOp === 'find';
892943
const posture = derivePosture(context);
893944

894945
const record = deps.fetchRecord
895946
? await deps.fetchRecord(object, recordId, engineOp).catch(() => null)
896947
: null;
897948
const recordExists = record != null;
949+
// [#20431] With the declared columns, the matcher REFUSES (throws
950+
// `INVALID_FILTER` / 400) a field-to-field comparison between two columns of
951+
// no shared comparison class, for every record or for none — the comparison
952+
// the find's driver refuses to compile. Without them it compared the two raw
953+
// values, so the report answered `visible` true or false, depending on how
954+
// they happened to compare, for a read enforcement refuses outright. Only the
955+
// business-RLS site below catches that refusal: the tenant wall and the
956+
// sharing filter are platform-composed and compare no two columns, so a
957+
// refusal from either would fail this explanation loudly rather than be
958+
// answered.
898959
const matches = (filter: unknown): boolean | undefined => {
899960
if (!recordExists) return undefined;
900961
if (filter == null) return true;
901-
return matchesFilterCondition(record as Record<string, unknown>, filter as any);
962+
return matchesFilterCondition(record as Record<string, unknown>, filter as any, declaredColumns);
902963
};
903964

904965
// The composition enforcement runs before the query: when it throws, neither
@@ -1116,6 +1177,9 @@ async function applyRecordAttribution(
11161177
}
11171178

11181179
// ── rls: the business (Layer 1) predicate for this record ────────────────
1180+
// [#20431] Set when the matcher refused the predicate: the request it scopes
1181+
// is refused rather than answered, so there is no row judgement to report.
1182+
let rlsRefusal: CrossFieldClassRefusal | null = null;
11191183
const rlsLayer = layers.find((l) => l.layer === 'rls');
11201184
if (rlsLayer) {
11211185
if (!recordExists) {
@@ -1128,19 +1192,48 @@ async function applyRecordAttribution(
11281192
rlsLayer.record = { outcome: 'not_evaluated', rowFilter: null, rules: [], detail: 'No business RLS policy applies to this record.' };
11291193
} else {
11301194
const deny = isDenyAll(layer1);
1131-
const m = matches(layer1);
1132-
const excluded = deny || m === false;
1133-
rlsLayer.record = {
1134-
outcome: excluded ? 'excluded' : 'admitted',
1135-
rowFilter: layer1,
1136-
matchesRecord: deny ? false : m,
1137-
rules: [{ kind: 'rls_policy', name: 'business_rls', predicate: layer1, effect: excluded ? 'excludes' : 'admits' }],
1138-
detail: deny
1139-
? 'Business RLS composes to DENY ALL for this principal.'
1140-
: excluded
1141-
? 'The record does not satisfy the business row-level predicate.'
1142-
: 'The record satisfies the business row-level predicate.',
1143-
};
1195+
let m: boolean | undefined;
1196+
try {
1197+
m = matches(layer1);
1198+
} catch (e) {
1199+
rlsRefusal = crossFieldClassRefusalCarriedBy(e);
1200+
if (!rlsRefusal) throw e;
1201+
}
1202+
if (rlsRefusal) {
1203+
// The #20002 shape for a call enforcement cannot complete: the layer
1204+
// `not_evaluated`, no `matchesRecord` (there is no judgement behind
1205+
// it), the detail naming the refusal. `rowFilter` stays the predicate
1206+
// that was composed — the one the object-level `readFilter` reports.
1207+
const policies = declaredColumns?.fields ? refusedPolicyNamesOf(layer1, declaredColumns.fields) : [];
1208+
rlsLayer.record = {
1209+
outcome: 'not_evaluated',
1210+
rowFilter: layer1,
1211+
rules: [],
1212+
detail:
1213+
`Business RLS cannot be evaluated for this record: in ` +
1214+
(policies.length > 0 ? `policy ${policies.map((p) => `'${p}'`).join(', ')}` : 'a business RLS policy') +
1215+
`, ${rlsRefusal.diagnostic}. ` +
1216+
(isRead
1217+
? 'The find this predicate scopes is refused with INVALID_FILTER / 400 before any record is read, ' +
1218+
'so the record is reported NOT visible (fail closed), never judged by comparing the two values.'
1219+
: `The ${engineOp} this predicate scopes is refused rather than judged (the read under it is refused ` +
1220+
'with INVALID_FILTER, and a by-id update or delete fails closed at its row-level gate), so the ' +
1221+
'record is reported NOT writable (fail closed), never judged by comparing the two values.'),
1222+
};
1223+
} else {
1224+
const excluded = deny || m === false;
1225+
rlsLayer.record = {
1226+
outcome: excluded ? 'excluded' : 'admitted',
1227+
rowFilter: layer1,
1228+
matchesRecord: deny ? false : m,
1229+
rules: [{ kind: 'rls_policy', name: 'business_rls', predicate: layer1, effect: excluded ? 'excludes' : 'admits' }],
1230+
detail: deny
1231+
? 'Business RLS composes to DENY ALL for this principal.'
1232+
: excluded
1233+
? 'The record does not satisfy the business row-level predicate.'
1234+
: 'The record satisfies the business row-level predicate.',
1235+
};
1236+
}
11441237
}
11451238
}
11461239

@@ -1178,6 +1271,9 @@ async function applyRecordAttribution(
11781271
if (capsDeny) { visible = false; decidedBy = 'required_permissions'; }
11791272
else if (!crudAllowed) { visible = false; decidedBy = 'object_crud'; }
11801273
else if (!recordExists) { visible = false; decidedBy = undefined; }
1274+
// [#20431] A refused predicate fails the request before any row is read, the
1275+
// tenant wall's included: the driver compiles `Layer0 AND Layer1` as one query.
1276+
else if (rlsRefusal) { visible = false; decidedBy = 'rls'; }
11811277
else if (tenantExcluded) { visible = false; decidedBy = 'tenant_isolation'; }
11821278
else if (rlsExcluded) { visible = false; decidedBy = 'rls'; }
11831279
// A dependency that threw decides by failing the request, in the order the
@@ -1628,7 +1724,7 @@ export async function explainAccess(deps: ExplainEngineDeps, input: ExplainInput
16281724
if (input.recordId) {
16291725
const out = await applyRecordAttribution({
16301726
deps, object, recordId: input.recordId, engineOp: dataOp, context, sets, layers, owd, capsDeny, crudAllowed,
1631-
vamaEffective, vamaSets,
1727+
vamaEffective, vamaSets, declaredColumns: declaredColumnsOf(schema),
16321728
});
16331729
recordVerdict = out.record;
16341730
posture = out.posture;

0 commit comments

Comments
 (0)