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
58 changes: 58 additions & 0 deletions .changeset/explain-partial-mask-reporting.md
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,58 @@
---
"@objectstack/plugin-security": minor
---

fix(security): security explain reports partial masking — the field-mask layer gains the third state instead of calling gated fields hidden and gate-less rule fields readable (#9127)

<!-- adr-0087: not-required (no-migration-prescription) Nothing authorable
changes. No `packages/spec` property is added, renamed, retired or tombstoned:
`field.maskingRule` was minted by #8993 and is untouched here, and the explain
report's own schema (`ExplainLayerSchema`) keeps its exact shape — the fix
lands entirely in what the `fls` layer's existing `verdict` and `detail` say.
There is no stored data to migrate and no author-facing spelling to convert. -->

#8993 landed partial masking on the enforcement channel: a field declaring
`maskingRule` is no longer deleted from a masked caller's response, its value
is **replaced** (`13812345678` → `138****5678`), with the field's
`requiredPermissions` acting as the unmask gate. The access-explanation
engine's field-mask layer predates that and read only the binary mask, so on
the one surface whose whole job is to describe enforcement it stated two
things that were not true:

- a field with `maskingRule` **and** a `requiredPermissions` gate the caller
does not hold was listed under *"N field(s) masked from responses"* — an
admin reading the report concluded the key was absent, while the caller was
in fact receiving the partially masked value;
- a field with `maskingRule` and **no** gate was reported under *"No
field-level masking applies"* — invisible in the report, and masked for
every non-system caller in reality.

Both directions matter, and they fail opposite ways: the first overstates the
protection in place, the second hides that any applies at all.

The `fls` layer now reports the three states the enforcement path actually
produces — **hidden** (key deleted), **partially masked** (key served, value
replaced, the applicable rule named) and **readable** — and answers `narrows`
whenever either dimension bites, where a gate-less rule previously produced
`not_applicable`.

**Mirrored, not re-derived.** The composition deciding which rules apply to a
caller — `computePartialMaskRules` AND the explicit-deny exclusion that a
permission-set `readable: false` still wins outright — is lifted into one
method on the plugin (`computeReadPartialMaskRules`) that the result-masking
middleware, the readable-field projection and now explain all call. The
hidden/partial split in the report is `FieldMasker.maskResults`' own rule
(`!(field in rules)`), so the report cannot disagree with the masking it
describes. A second, independent derivation inside the explain engine is
exactly how this drift opened in the first place; `security-service.ts`'s
module contract claims explain *"matches enforcement by construction"*, and
this restores that for the partial-mask dimension.

**Breaking for direct embedders of the engine** (hence `minor`, not `patch`):
`ExplainEngineDeps` gains a **required** `getPartialMaskRules`. It is required
rather than optional on purpose — the field-mask decision has three outcomes
and the existing binary `getFieldMask` can express only two, so an engine
wired without it would silently reproduce both misreports above. A compile
error is the correct way for that omission to surface. Callers going through
`SecurityPlugin` / the `security` service's `explain()` — every consumer in
this repo — need no change.
148 changes: 148 additions & 0 deletions packages/plugins/plugin-security/src/explain-engine.test.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -36,6 +36,7 @@ function makeDeps(overrides: Partial<ExplainEngineDeps> & { sets?: any[]; schema
},
computeRlsFilter: async () => overrides.rls !== undefined ? overrides.rls : null,
getFieldMask: () => ({}),
getPartialMaskRules: async () => ({}),
baselinePermissionSets: ['member_default'],
...overrides,
};
Expand DownExpand Up@@ -171,6 +172,153 @@ describe('explainAccess (ADR-0090 D6)', () => {
});
});

// ---------------------------------------------------------------------------
// [#9127] fls — the THREE-state field mask (hidden / partially masked /
// readable). #8993 landed partial masking on the enforcement channel; this
// layer reported only the binary mask, so a gated rule field read as fully
// hidden and a gate-less rule field as fully readable. `getPartialMaskRules`
// carries the enforcement composition in (never a re-derivation), and the
// hidden/partial split here is `FieldMasker.maskResults`' own.
// ---------------------------------------------------------------------------
describe('explainAccess — fls reports partial masking (#9127)', () => {
const flsOf = async (over: Partial<ExplainEngineDeps>) => {
const d = await explainAccess(makeDeps(over), {
object: 'leave_request',
operation: 'read',
context: CTX,
});
return d.layers.find((l) => l.layer === 'fls')!;
};

it('HIDDEN: a field the mask denies with no applicable rule is still reported deleted', async () => {
const fls = await flsOf({
getFieldMask: () => ({ ssn: { readable: false }, name: { readable: true } }),
getPartialMaskRules: async () => ({}),
});
expect(fls.verdict).toBe('narrows');
expect(fls.detail).toContain('1 field(s) masked from responses');
expect(fls.detail).toContain('ssn');
expect(fls.detail).not.toContain('PARTIALLY');
});

it('PARTIAL (gated): a rule field the caller has not unmasked is NOT reported as hidden', async () => {
// The requiredPermissions fold marks `phone` non-readable; the rule turns
// that deletion into a replacement, so the caller receives `138****5678`,
// not an absent key. Pre-#9127 this read "1 field(s) masked from
// responses: [phone]" — the first misreport on the card.
const fls = await flsOf({
getFieldMask: () => ({ phone: { readable: false } }),
getPartialMaskRules: async () => ({ phone: 'phone' }),
});
expect(fls.verdict).toBe('narrows');
expect(fls.detail).not.toContain('masked from responses');
expect(fls.detail).toContain('1 field(s) PARTIALLY masked');
expect(fls.detail).toContain('phone (phone)');
});

it('PARTIAL (gate-less): a rule with no requiredPermissions is reported, not passed over', async () => {
// No permission entry exists for `bank` — the binary mask is silent — yet
// every non-system caller sees it masked. Pre-#9127 this layer answered
// `not_applicable` / "No field-level masking applies": the second misreport.
const fls = await flsOf({
getFieldMask: () => ({}),
getPartialMaskRules: async () => ({ bank: 'bank_account' }),
});
expect(fls.verdict).toBe('narrows');
expect(fls.detail).not.toBe('No field-level masking applies.');
expect(fls.detail).toContain('1 field(s) PARTIALLY masked');
expect(fls.detail).toContain('bank (bank_account)');
});

it('READABLE: a field in neither set appears in neither list', async () => {
const fls = await flsOf({
getFieldMask: () => ({ ssn: { readable: false }, name: { readable: true } }),
getPartialMaskRules: async () => ({ phone: 'phone' }),
});
expect(fls.detail).toContain('masked from responses: [ssn]');
expect(fls.detail).toContain('PARTIALLY masked');
expect(fls.detail).not.toContain('name');
});

it('reports all three states together, splitting hidden from partially masked', async () => {
const fls = await flsOf({
getFieldMask: () => ({
ssn: { readable: false },
phone: { readable: false },
name: { readable: true },
}),
getPartialMaskRules: async () => ({ phone: 'phone', bank: 'bank_account' }),
});
expect(fls.verdict).toBe('narrows');
// `phone` is denied by the binary mask AND carries a rule → partial, not hidden.
expect(fls.detail).toContain('1 field(s) masked from responses: [ssn]');
expect(fls.detail).toContain('2 field(s) PARTIALLY masked');
expect(fls.detail).toContain('phone (phone)');
expect(fls.detail).toContain('bank (bank_account)');
});

it('an explicit permission-set DENY stays HIDDEN — explain follows enforcement, it does not assume', async () => {
// A rule never widens an explicit deny, so enforcement drops such fields
// from `partialRules` before this layer sees them. The engine must report
// whatever the composition hands it rather than re-deriving "has a rule ⇒
// partial" — that second derivation is how explain drifted in the first place.
const fls = await flsOf({
getFieldMask: () => ({ phone: { readable: false } }),
getPartialMaskRules: async () => ({}),
});
expect(fls.detail).toContain('masked from responses: [phone]');
expect(fls.detail).not.toContain('PARTIALLY');
});

it('names an explicit keepHead/keepTail span rather than a preset', async () => {
const fls = await flsOf({
getPartialMaskRules: async () => ({ code: { keepHead: 2, keepTail: 2 } }),
});
expect(fls.detail).toContain('code (keepHead 2, keepTail 2)');
});

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 () => ({ id: 'u_boss' }),
find: async () => [],
},
getPartialMaskRules: async () => ({ phone: 'phone' }),
}),
{
object: 'leave_request',
operation: 'read',
context: { ...CTX, onBehalfOf: { userId: 'u_boss' } },
},
);
const fls = d.layers.find((l) => l.layer === 'fls')!;
expect(fls.detail).toContain('PARTIALLY masked');
expect(fls.detail).toContain('D10');
});

it('stays not_applicable when neither dimension applies', async () => {
const fls = await flsOf({});
expect(fls.verdict).toBe('not_applicable');
expect(fls.detail).toBe('No field-level masking applies.');
});

it('passes the delegator sets through to the enforcement composition (D10)', async () => {
let seen: unknown = 'not-called';
await flsOf({
getPartialMaskRules: async (_sets, _object, delegatorSets) => {
seen = delegatorSets;
return {};
},
});
// No on-behalf-of in CTX → the engine must pass null, not undefined.
expect(seen).toBeNull();
});
});

describe('explainAccess — record-grained (C2 / ADR-0095)', () => {
const REC_CTX = { userId: 'u1', tenantId: 'org1', positions: ['sales_rep', 'everyone'], permissions: [] };

Expand Down
72 changes: 68 additions & 4 deletions packages/plugins/plugin-security/src/explain-engine.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -27,6 +27,7 @@ import {
} from '@objectstack/core';
import { matchesFilterCondition } from '@objectstack/formula';
import { BUILTIN_IDENTITY_PLATFORM_ADMIN, ORGANIZATION_ADMIN_GRANTS } from '@objectstack/spec';
import type { FieldMaskingRule } from '@objectstack/spec/data';
import type { PermissionSet } from '@objectstack/spec/security';
import type {
AuthzPosture,
Expand DownExpand Up@@ -174,6 +175,28 @@ export interface ExplainEngineDeps {
object: string,
fieldRequiredPermissions: Record<string, string[]>,
) => Record<string, { readable?: boolean; editable?: boolean }>;
/**
* [#9127] The middleware's EFFECTIVE partial-mask set for this caller — the
* `partialRules` argument enforcement hands `FieldMasker.maskResults`, after
* the explicit-deny exclusion. Keyed by field, valued by the rule that will
* be applied.
*
* REQUIRED, deliberately. The field-mask decision has three outcomes
* (deleted / partially masked / served whole) and the binary
* {@link getFieldMask} can express only two, so an engine wired without this
* would report a partially-masked field as fully hidden and a gate-less
* rule field as fully readable — the exact misreport this dep exists to
* close. An optional dep would have let a new embedder re-open it silently;
* a required one makes the omission a compile error.
*
* Supply the enforcement composition, never a re-derivation of it: explain's
* module contract is that it "matches enforcement by construction".
*/
getPartialMaskRules: (
sets: PermissionSet[],
object: string,
delegatorSets: PermissionSet[] | null,
) => Promise<Record<string, FieldMaskingRule>>;
/**
* Configured additive baseline set NAMES (default `['member_default']`), for
* attribution.
Expand DownExpand Up@@ -504,6 +527,17 @@ export function intersectFieldMasks(
return out;
}

/**
* [#9127] Render a `maskingRule` for the report. PRESENTATION ONLY — it never
* decides whether the rule applies (that is enforcement's call, arriving via
* `deps.getPartialMaskRules`); it only names the rule the caller's masked
* value was produced by, so an admin reading "why is this `138****5678`" gets
* the preset or the explicit span instead of a bare field name.
*/
function describeMaskingRule(rule: FieldMaskingRule): string {
return typeof rule === 'string' ? rule : `keepHead ${rule.keepHead}, keepTail ${rule.keepTail}`;
}

/** D1-equivalent OWD reading (mirrors plugin-sharing's effectiveSharingModel). */
function describeOwd(schema: any): { model: string; declared: boolean; effect: 'private' | 'read' | 'public' } {
const m = schema?.sharingModel ?? schema?.security?.sharingModel;
Expand DownExpand Up@@ -1013,12 +1047,42 @@ export async function explainAccess(deps: ExplainEngineDeps, input: ExplainInput
const mask = delegatorSets
? intersectFieldMasks(agentMask, deps.getFieldMask(delegatorSets, object, secMeta.fieldRequiredPermissions))
: agentMask;
const hidden = Object.entries(mask).filter(([, p]) => p?.readable === false).map(([f]) => f);
// [#9127] The field-mask decision has THREE outcomes, not two — hidden
// (key deleted), PARTIALLY masked (key served, value replaced by the
// field's `maskingRule`), and readable. `deps.getPartialMaskRules` is the
// enforcement composition verbatim, never a second derivation of it: the
// binary mask below cannot express the middle state, and reading it alone
// is what made this layer call a partially-masked field fully hidden and a
// gate-less rule field fully readable.
//
// The hidden/partial split is `FieldMasker.maskResults`' own: it deletes a
// field when the binary mask denies it AND no rule applies, so a rule that
// survived the explicit-deny exclusion always demotes `hidden` to `partial`
// — including the capability-gated case, where the mask says `readable:
// false` precisely because the unmask gate is what the rule softens.
const partialRules = await deps.getPartialMaskRules(sets, object, delegatorSets);
const partial = Object.keys(partialRules);
const hidden = Object.entries(mask)
.filter(([f, p]) => p?.readable === false && !(f in partialRules))
.map(([f]) => f);
const listFields = (fields: string[], label: (f: string) => string): string =>
`[${fields.slice(0, 25).map(label).join(', ')}${fields.length > 25 ? ', …' : ''}]`;
const maskNarrows = hidden.length > 0 || partial.length > 0;
layers.push({
layer: 'fls',
verdict: hidden.length > 0 ? 'narrows' : 'not_applicable',
detail: hidden.length > 0
? `${hidden.length} field(s) masked from responses: [${hidden.slice(0, 25).join(', ')}${hidden.length > 25 ? ', …' : ''}]` +
verdict: maskNarrows ? 'narrows' : 'not_applicable',
detail: maskNarrows
? [
hidden.length > 0
? `${hidden.length} field(s) masked from responses: ${listFields(hidden, (f) => f)}`
: null,
partial.length > 0
? `${partial.length} field(s) PARTIALLY masked — the key is still served, its value ` +
`replaced: ${listFields(partial, (f) => `${f} (${describeMaskingRule(partialRules[f])})`)}`
: null,
]
.filter((s): s is string => s !== null)
.join('; ') +
(delegatorSets ? ' (intersection of agent + delegator masks, D10).' : '.')
: 'No field-level masking applies.',
contributors: [],
Expand Down
47 changes: 47 additions & 0 deletions packages/plugins/plugin-security/src/field-masking-rule.test.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -341,4 +341,51 @@ describe('SecurityPlugin — maskingRule middleware enforcement (#8993)', () =>
const readable = await svc.getReadableFields('contact', { userId: 'u1', positions: [], permissions: [] });
expect(readable).not.toContain('phone');
});

// -------------------------------------------------------------------------
// [#9127] explain ↔ enforcement, on THIS fixture. The suites above pin what
// the caller actually receives; these pin that `explain` says the same thing
// about the same (caller, object) pair. Same harness, same permission sets,
// same schema — so a future change that moves one channel without the other
// fails here rather than shipping an access report that contradicts the
// access. That is the module contract's own claim: explain "matches
// enforcement by construction".
// -------------------------------------------------------------------------
const explainFlsFor = async (sets: PermissionSet[], fallback: string, permissions: string[] = []) => {
const h = harnessFor(sets, fallback);
await h.plugin.init(h.ctx); await h.plugin.start(h.ctx);
const svc: any = h.ctx.registerService.mock.calls.find((c: any[]) => c[0] === 'security')?.[1];
const decision = await svc.explain(
{ object: 'contact', operation: 'read' },
{ userId: 'u1', positions: [], permissions },
);
return decision.layers.find((l: any) => l.layer === 'fls');
};

it('explain reports the partially masked fields the read path actually serves masked', async () => {
const fls = await explainFlsFor([setNoCap], 'msk_member');
// The enforcement test above serves this caller `138****5678` / `***…1234`
// — the keys ARE present, so neither may be reported deleted.
expect(fls.verdict).toBe('narrows');
expect(fls.detail).not.toContain('masked from responses');
expect(fls.detail).toContain('PARTIALLY masked');
expect(fls.detail).toContain('phone (phone)');
expect(fls.detail).toContain('bank (bank_account)');
});

it('explain drops the gated field from the masked set once the caller holds the unmask capability', async () => {
const fls = await explainFlsFor([setWithCap], 'msk_pii', ['msk_pii']);
// Enforcement serves `phone` whole and `bank` masked for this caller.
expect(fls.detail).not.toContain('phone');
expect(fls.detail).toContain('bank (bank_account)');
});

it('explain reports an explicit permission-set DENY as HIDDEN, not partially masked', async () => {
const fls = await explainFlsFor([setFieldDeny], 'msk_deny');
// Enforcement DELETES the key for this caller (a rule never widens an
// explicit deny), so the report must say deleted — not "value replaced".
expect(fls.detail).toContain('masked from responses');
expect(fls.detail).toContain('phone');
expect(fls.detail).not.toContain('phone (phone)');
});
});
Original file line numberDiff line numberDiff line change
Expand Up@@ -195,6 +195,7 @@ describe('[#3545] unresolvable object metadata — security posture fails closed
},
computeRlsFilter: async () => null,
getFieldMask: () => ({}),
getPartialMaskRules: async () => ({}),
baselinePermissionSets: ['member_default'],
}) as any;

Expand Down
Loading
Loading