diff --git a/.changeset/security-fls-unknown-field.md b/.changeset/security-fls-unknown-field.md new file mode 100644 index 0000000000..0c3782ac14 --- /dev/null +++ b/.changeset/security-fls-unknown-field.md @@ -0,0 +1,16 @@ +--- +"@objectstack/lint": minor +--- + +New gating rule `security-fls-unknown-field`: an object-qualified field-permission key naming a field the object does not declare is now an authoring-time `error`. + +`security-fls-unqualified-key` has always caught the *bare* spelling — `fields: { budget: … }` — because the runtime evaluator matches FLS keys by their `.` prefix and a bare key matches nothing. The qualified-but-dangling spelling (`fields: { 'crm_account.description_nope': { readable: false } }`) has the identical runtime consequence and was reported by nothing: `PermissionEvaluator.getFieldPermissions` strips the prefix and looks the remainder up as a column, so a remainder no column answers to contributes nothing to the merged permission map. The masking the author declared **never enforces**, and the field stays as readable and as editable as the object-level grant leaves it — for every holder of the set. + +The failure direction is **fail open**, and this spelling is the one that accumulates: unlike a bare key it looks correct in review, survives rename refactors invisibly, and is exactly what a field rename leaves behind. + +- **A second rule, not a widening of the first.** `security-fls-unqualified-key` is correct inside its declared scope and is untouched; the two defects have different prescriptions (add the object prefix / fix the field name) and suppressing one must not suppress the other. Two ids, two messages. +- **Where the existence answer comes from.** The rule resolves through `object-graph.ts`, the shared index every field-existence rule in this package already uses — no new input path. It therefore inherits that module's three skips, each of which is the difference between a finding and a false one: an object this stack does not define (it may be another installed package's), an object with no readable field map (an ADR-0015 `external` object, an introspected datasource), and registry-injected system columns such as `created_at` or `owner_id`, which are real at runtime and appear in no authored `fields`. +- **A truncated key is the same defect and is reported by the same rule.** `fields: { 'crm_account.': … }` passes the runtime's prefix test and resolves to the empty column name, so it matches nothing exactly as a dangling name does. `PermissionSetSchema.fields` is `z.record(z.string(), FieldPermissionSchema)` — a bare string key with no pattern and no refinement — and this rule is the only reader of those keys, so before this change nothing reported it at all. A key naming an object this stack does not declare still falls to skip 1, truncated or not. +- **It mirrors the evaluator, including on a multi-dot key.** Only the first dot separates object from field, because `ObjectSchema.name` is `/^[a-z_][a-z0-9_]*$/` and cannot contain one. `'crm_account.owner.name'` therefore asks for a column literally named `owner.name` and is reported: FLS keys address columns, never joins, and resolving that as a relationship hop would have been a fail-open divergence from the gate the rule mirrors. + +**What moves for consumers.** A stack carrying a dangling FLS key built clean before and now fails `os validate` / `os compile`, and is refused at the runtime publish door for `permission` and `object` writes (this rule joins the existing `validateSecurityPosture` registration; no new registry entry). That is the point — the key was never enforcing anything. A stack whose FLS keys all resolve is byte-identically clean: measured on the shipped showcase, whose six authored keys emit zero findings, with a firing control (one injected dangling key produces exactly one finding) beside the zero. diff --git a/packages/lint/src/authoring-rules.ts b/packages/lint/src/authoring-rules.ts index 0a5476b3a3..9757f22c1c 100644 --- a/packages/lint/src/authoring-rules.ts +++ b/packages/lint/src/authoring-rules.ts @@ -1410,7 +1410,7 @@ export const AUTHORING_RULES: readonly AuthoringRule[] = [ // rule therefore stays behind WHOLE (#8310's explicit call), as its own // entry. // - // This entry remains the rest of the D7 block (12 rule ids) as ONE + // This entry remains the rest of the D7 block (14 rule ids) as ONE // registration, not a per-rule split: the baseline/candidate differential is // what keeps a write of one declared type from leaking the other rules' // whole-stack findings — every finding derived from a sibling collection is diff --git a/packages/lint/src/index.ts b/packages/lint/src/index.ts index c0777e4113..8968a1e940 100644 --- a/packages/lint/src/index.ts +++ b/packages/lint/src/index.ts @@ -307,6 +307,7 @@ export { SECURITY_PRIVATE_NO_READSCOPE, SECURITY_MASTER_DETAIL_UNGRANTED, SECURITY_FLS_UNQUALIFIED_KEY, + SECURITY_FLS_UNKNOWN_FIELD, SECURITY_GRANT_EXPIRED_AT_AUTHORING, SECURITY_DELEGATION_MISSING_REASON, SECURITY_CBP_NO_RELATION, diff --git a/packages/lint/src/validate-security-posture.runtime-surface.test.ts b/packages/lint/src/validate-security-posture.runtime-surface.test.ts index b0995b6211..8f4b097d4a 100644 --- a/packages/lint/src/validate-security-posture.runtime-surface.test.ts +++ b/packages/lint/src/validate-security-posture.runtime-surface.test.ts @@ -175,10 +175,15 @@ describe('validateSecurityPosture at the runtime publish surface (#7576 → #830 expect(stackKeyForType('flow')).toBe('flows'); }); - it('[#8310] seed / permission / book / object all cross — the completed flip, on the whole 12-rule entry', () => { + it('[#8310] seed / permission / book / object all cross — the completed flip, on the whole 14-rule entry', () => { // The registration the #7891 programme was for: the - // `validateSecurityPosture` entry (12 rule ids — `security-role-word` is + // `validateSecurityPosture` entry (14 rule ids — `security-role-word` is // its own entry now, see below) declares all four mapped types. + // ⚠️ The count was written as 12 and had already drifted: it missed + // `security-cbp-ambiguous-relation` (#14747). Re-measured by counting the + // distinct `rule:` constants this function emits — 13 before + // `security-fls-unknown-field` (#16108), 14 with it. The number is prose, + // not an assertion; the pins below are what hold. // `permission`/`book` measured ZERO refusals when they crossed (PR // #8546); `object` crosses under the #8310 maintainer ruling with the // red suites repaired honestly (fixtures author their `sharingModel`). diff --git a/packages/lint/src/validate-security-posture.test.ts b/packages/lint/src/validate-security-posture.test.ts index 4c6afaec6d..dada2360ed 100644 --- a/packages/lint/src/validate-security-posture.test.ts +++ b/packages/lint/src/validate-security-posture.test.ts @@ -14,6 +14,7 @@ import { FieldSchema, ObjectSchema } from '@objectstack/spec/data'; import { ObjectPermissionSchema, PermissionSetSchema } from '@objectstack/spec/security'; import { SECURITY_FLS_UNQUALIFIED_KEY, + SECURITY_FLS_UNKNOWN_FIELD, validateSecurityPosture, validateSecurityRoleWord, SECURITY_OWD_UNSET, @@ -183,6 +184,243 @@ describe('validateSecurityPosture (ADR-0090 D7)', () => { ).toEqual([]); }); + // ── Rule: security-fls-unknown-field (#16108) ─────────────────────── + // + // The card's pair, both directions, on one object and one set: an FLS key + // that is object-qualified and names a REAL field passes; the same key + // naming a field the object does not declare reds. Before this rule the two + // were indistinguishable — both exit 0 — which is the fail-OPEN the rule + // closes: a mask that can never match leaves the field readable to every + // holder of the set, and nothing said so at author time or at runtime. + const ACCOUNT = { + name: 'crm_account', + label: 'Account', + sharingModel: 'public_read', + fields: { description: { type: 'text', label: 'Description' } }, + } as const; + + const flsStack = (key: string, perm: Record = { readable: false, editable: false }) => ({ + objects: [ACCOUNT], + permissions: [ + { name: 'sales_rep', label: 'Sales Rep', objects: { crm_account: { allowRead: true } }, fields: { [key]: perm } }, + ], + }); + + it('errors on a qualified key naming a field the object does not declare — the mask never enforces', () => { + const findings = validateSecurityPosture(flsStack('crm_account.description_nope')).filter( + (f) => f.rule === SECURITY_FLS_UNKNOWN_FIELD, + ); + expect(findings).toHaveLength(1); + expect(findings[0].severity).toBe('error'); + expect(findings[0].path).toBe('permissions[0].fields["crm_account.description_nope"]'); + // The message must name BOTH halves: what the author did wrong, and what + // it costs. "unknown field" alone would read as a typo notice on a key + // that is in fact an unenforced security control. + expect(findings[0].message).toContain("crm_account.description_nope"); + expect(findings[0].message).toContain("declares no field 'description_nope'"); + expect(findings[0].message).toMatch(/NEVER ENFORCES/); + expect(findings[0].message).toMatch(/stays as readable and as editable/); + // And the hint must be actionable: the fields that DO exist. + expect(findings[0].hint).toContain('description'); + }); + + it('accepts a qualified key naming a real field — the other direction of the same pair', () => { + expect(rulesOf(flsStack('crm_account.description'))).toEqual([]); + }); + + // The control stays in its own lane: two rules, two ids, two messages. A + // widening of `security-fls-unqualified-key` would have collapsed them. + it('the unqualified spelling still reds under security-fls-unqualified-key ONLY', () => { + const rules = rulesOf(flsStack('description')); + expect(rules).toEqual([SECURITY_FLS_UNQUALIFIED_KEY]); + expect(rules).not.toContain(SECURITY_FLS_UNKNOWN_FIELD); + }); + + // ── Negative controls: an implementation that always fires passes every + // positive above, so these are what make the positives readings. + it('an all-qualified, all-real permission set emits NOTHING new', () => { + expect( + rulesOf({ + objects: [ + { + ...ACCOUNT, + fields: { + description: { type: 'text', label: 'Description' }, + revenue: { type: 'number', label: 'Revenue' }, + }, + }, + ], + permissions: [ + { + name: 'sales_rep', + label: 'Sales Rep', + objects: { crm_account: { allowRead: true } }, + fields: { + 'crm_account.description': { readable: true, editable: true }, + 'crm_account.revenue': { readable: false, editable: false }, + }, + }, + ], + }), + ).toEqual([]); + }); + + it('an object with NO fields at all does not throw, and is not judged', () => { + // Skip 2: an object declaring no readable field map (ADR-0015 `external`, + // an introspected datasource) resolves its columns at runtime, so the + // linter cannot answer and must not guess. Both spellings of "no fields". + for (const fields of [undefined, {}]) { + const stack = { + objects: [{ name: 'crm_account', label: 'Account', sharingModel: 'public_read', ...(fields ? { fields } : {}) }], + permissions: [ + { name: 'ps', label: 'PS', objects: {}, fields: { 'crm_account.whatever': { readable: false } } }, + ], + }; + expect(() => validateSecurityPosture(stack)).not.toThrow(); + expect(rulesOf(stack)).toEqual([]); + } + }); + + it('skip 1: an object this stack does not define is never judged', () => { + // The set may legitimately mask a field of an object another installed + // package ships — the same silence `security-book-audience-unknown-set` + // keeps for a set it cannot see. + expect( + rulesOf({ + objects: [ACCOUNT], + permissions: [ + { name: 'ps', label: 'PS', objects: {}, fields: { 'not_in_this_stack.anything': { readable: false } } }, + ], + }), + ).toEqual([]); + }); + + it('skip 3: a registry-injected system column resolves, and is not a finding', () => { + // `created_at` appears in no authored `fields` and is real at runtime. + // Flagging it would be the false finding ADR-0072 D1 refuses — measured + // against a nonsense sibling in the same run, so the zero is a reading. + expect(rulesOf(flsStack('crm_account.created_at', { readable: true }))).toEqual([]); + expect(rulesOf(flsStack('crm_account.created_at_nope', { readable: true }))).toEqual([ + SECURITY_FLS_UNKNOWN_FIELD, + ]); + }); + + it('mirrors the evaluator on a multi-dot key: FLS addresses columns, never joins', () => { + // `getFieldPermissions` strips only the object prefix and looks the WHOLE + // remainder up as a column, so `crm_account.owner.name` asks for a column + // literally named `owner.name` and matches nothing. Resolving it as a + // relationship hop here would have been a fail-open divergence from the + // gate this rule mirrors. + const findings = validateSecurityPosture({ + objects: [ + { + ...ACCOUNT, + fields: { + description: { type: 'text', label: 'Description' }, + owner: { type: 'lookup', label: 'Owner', reference: 'sys_user' }, + }, + }, + { name: 'sys_user', label: 'User', isSystem: true, fields: { name: { type: 'text', label: 'Name' } } }, + ], + permissions: [ + { name: 'ps', label: 'PS', objects: {}, fields: { 'crm_account.owner.name': { readable: false } } }, + ], + }).filter((f) => f.rule === SECURITY_FLS_UNKNOWN_FIELD); + expect(findings).toHaveLength(1); + expect(findings[0].message).toContain("declares no field 'owner.name'"); + }); + + it('reports the EMPTY remainder — a truncated key masks nothing either', () => { + // ⚠️ This was skipped by an earlier revision with a comment claiming the + // schema owned the shape. It does not: `PermissionSetSchema.fields` is + // `z.record(z.string(), FieldPermissionSchema)` with a bare string key — + // pinned below against the LIVE schema, so the premise cannot rot silently + // — and this rule is the only reader of those keys in the package. The key + // passes `startsWith('crm_account.')` at runtime and resolves to the empty + // column name, so it is the identical fail open. + const findings = validateSecurityPosture(flsStack('crm_account.')).filter( + (f) => f.rule === SECURITY_FLS_UNKNOWN_FIELD, + ); + expect(findings).toHaveLength(1); + expect(findings[0].severity).toBe('error'); + expect(findings[0].path).toBe('permissions[0].fields["crm_account."]'); + expect(findings[0].message).toMatch(/names NO field/); + // The consequence half is the same sentence the dangling case carries — an + // author must be told what it costs, not merely that the key is malformed. + expect(findings[0].message).toMatch(/NEVER ENFORCES/); + expect(findings[0].message).toMatch(/stays as readable and as editable/); + // ⛔ And it must not read as the dangling-field message: there is no field + // name to quote, so the wrong branch would render `declares no field ''`. + expect(findings[0].message).not.toContain("declares no field"); + }); + + it('the empty-remainder premise: the schema really does NOT constrain the key', () => { + // The control that keeps the test above honest. If a future revision adds a + // key regex or a refine to `PermissionSetSchema.fields`, this pin goes red + // and the rule's justification is re-opened deliberately, instead of the + // rule quietly becoming a second opinion on the schema (Prime Directive + // #12). Read off the LIVE schema, with a control that the read is not + // vacuous. + const fieldsSchema = PermissionSetSchema.shape.fields; + expect(fieldsSchema, 'PermissionSetSchema must declare `fields` at all').toBeDefined(); + // A bare-string key accepts the truncated spelling: the schema parses it. + const parsed = PermissionSetSchema.safeParse({ + name: 'ps', + label: 'PS', + objects: {}, + fields: { 'crm_account.': { readable: false, editable: false } }, + }); + expect(parsed.success, 'the schema ACCEPTS a truncated FLS key — so this rule is its only reader').toBe(true); + // Control: the same shape with a key the schema also accepts, proving the + // `true` above is not a blanket accept-anything reading of `safeParse`. + expect( + PermissionSetSchema.safeParse({ name: 'ps', label: 'PS', objects: {}, notADeclaredKey: 1 }).success, + 'control — the schema is not accepting everything', + ).toBe(false); + }); + + it('skip 1 still holds for a truncated key naming an unknown object', () => { + // The deliberate non-widening: `'no_such_object.'` is unmatchable too, but + // judging it would mean judging a key whose OBJECT half this stack cannot + // resolve — the same disposition `'.description'` (empty object name) is + // left to. Named here so the silence is a decision on record, not an + // oversight, and measured against a firing control in the same run. + expect( + rulesOf({ + objects: [ACCOUNT], + permissions: [{ name: 'ps', label: 'PS', objects: {}, fields: { 'no_such_object.': { readable: false } } }], + }), + ).toEqual([]); + expect( + rulesOf({ + objects: [ACCOUNT], + permissions: [{ name: 'ps', label: 'PS', objects: {}, fields: { 'crm_account.': { readable: false } } }], + }), + ).toEqual([SECURITY_FLS_UNKNOWN_FIELD]); + }); + + it('reports every dangling key, and only the dangling ones', () => { + const findings = validateSecurityPosture({ + objects: [ACCOUNT], + permissions: [ + { + name: 'ps', + label: 'PS', + objects: {}, + fields: { + 'crm_account.description': { readable: true }, + 'crm_account.gone_one': { readable: false }, + 'crm_account.gone_two': { readable: false }, + }, + }, + ], + }).filter((f) => f.rule === SECURITY_FLS_UNKNOWN_FIELD); + expect(findings.map((f) => f.path)).toEqual([ + 'permissions[0].fields["crm_account.gone_one"]', + 'permissions[0].fields["crm_account.gone_two"]', + ]); + }); + // ── Rule: security-anchor-high-privilege (ADR-0090 D5/D9) ─────────── it('errors when an isDefault (everyone-suggested) set carries high-privilege bits', () => { const findings = validateSecurityPosture({ @@ -940,6 +1178,12 @@ const NOT_SCHEMA_RECEIVERS: Record = { cbpTier: "this file's own `cbpMasterCandidates` return type ({ tier, candidates }), not an authored surface.", winner: 'a `CbpRelation` — the winning-tier candidate this file already derived, not an authored surface.', cand: 'a `CbpRelation` — the same derived shape, one per candidate named in the ambiguity message.', + // [#16108] `surface` is a `GraphObject` from `object-graph.ts` — this + // package's own resolved shape ({ names, fields, injected }), built by that + // module from `ObjectSchema.fields`. The FieldSchema reads behind it are + // scanned where they happen (in `object-graph.ts`, guarded by its own tests), + // not here; this rule never touches an authored field record directly. + surface: "a `GraphObject` — `object-graph.ts`'s resolved per-object surface, not an authored one.", }; const READ_SURFACES: Array<{ receiver: string; expected: string[]; declaredBy: string; keys: () => string[] }> = [ @@ -1055,6 +1299,9 @@ describe('validateSecurityPosture — reads only keys the spec declares (meta-te const PLUMBING = new Set([ 'findings', 'objects', 'permissionSets', 'privateObjects', 'grantedObjects', 'stackSetNames', 'records', 'reason', 'until', 'setName', 'flsKey', 'opts', 'path', 'i', 'e', 'fields', 'crm_opportunity', + 'flsGraph', // #16108: the shared object index — `.has` / `.get`, JS Map methods. + 'flsField', // #16108: the key's remainder, a string — `.length`, a JS property. + 'declared', // #16108: one object's sorted field-name list — `.length` / `.slice` / `.join`. 'entries', // #7503: the rule's own field list — `.find`, a JS method. 'matched', // #14747: one tier's candidate list — `.length` / `.map`, JS methods. ]); @@ -1166,6 +1413,7 @@ const RULE_IDS: Record = { SECURITY_PRIVATE_NO_READSCOPE, SECURITY_MASTER_DETAIL_UNGRANTED, SECURITY_FLS_UNQUALIFIED_KEY, + SECURITY_FLS_UNKNOWN_FIELD, SECURITY_GRANT_EXPIRED_AT_AUTHORING, SECURITY_DELEGATION_MISSING_REASON, SECURITY_CBP_NO_RELATION, @@ -1190,6 +1438,13 @@ const REACHABILITY_CORPUS: Array<{ label: string; stack: Record stack: { objects: [objectFixture({ name: 'o', sharingModel: 'private', externalSharingModel: 'public_read_write' })] }, }, { label: 'fls-unqualified-key', stack: { permissions: [{ name: 'ps', label: 'PS', objects: {}, fields: { budget: { readable: true } } }] } }, + { + label: 'fls-unknown-field', + stack: { + objects: [objectFixture({ name: 'crm_account', sharingModel: 'public_read' })], + permissions: [{ name: 'ps', label: 'PS', objects: {}, fields: { 'crm_account.gone': { readable: false } } }], + }, + }, { label: 'wildcard-vama', stack: { permissions: [{ name: 'ps', label: 'PS', objects: { '*': { viewAllRecords: true } } }] } }, { label: 'anchor-high-privilege', @@ -1260,7 +1515,7 @@ describe('validateSecurityPosture — every branch is reachable without an undec }); it('maps every `findings.push` site in the source', () => { - expect(pushedRuleIds()).toHaveLength(17); + expect(pushedRuleIds()).toHaveLength(18); }); it('reaches every `findings.push` site from that corpus', () => { diff --git a/packages/lint/src/validate-security-posture.ts b/packages/lint/src/validate-security-posture.ts index 00069e9f36..a95310e35a 100644 --- a/packages/lint/src/validate-security-posture.ts +++ b/packages/lint/src/validate-security-posture.ts @@ -119,7 +119,7 @@ */ import { describeAnchorForbiddenBits } from '@objectstack/spec/security'; -import { recordsOf } from './object-graph.js'; +import { indexObjectGraph, recordsOf, type ObjectGraph } from './object-graph.js'; export const SECURITY_OWD_UNSET = 'security-owd-unset'; export const SECURITY_OWD_ALIAS = 'security-owd-alias'; @@ -131,6 +131,7 @@ export const SECURITY_BOOK_AUDIENCE_UNKNOWN_SET = 'security-book-audience-unknow export const SECURITY_PRIVATE_NO_READSCOPE = 'security-private-no-readscope'; export const SECURITY_MASTER_DETAIL_UNGRANTED = 'security-master-detail-ungranted'; export const SECURITY_FLS_UNQUALIFIED_KEY = 'security-fls-unqualified-key'; +export const SECURITY_FLS_UNKNOWN_FIELD = 'security-fls-unknown-field'; export const SECURITY_GRANT_EXPIRED_AT_AUTHORING = 'security-grant-expired-at-authoring'; export const SECURITY_DELEGATION_MISSING_REASON = 'security-delegation-missing-reason'; export const SECURITY_CBP_NO_RELATION = 'security-controlled-by-parent-no-relation'; @@ -592,6 +593,19 @@ export function validateSecurityPosture(stack: AnyRec, opts?: { nowMs?: number } } } + // [#16108] The object graph, for the FLS field-existence rule below. Built + // ONCE here rather than per permission set: `indexObjectGraph` walks every + // object's whole field map, and a stack with N sets would otherwise pay for + // that walk N times to answer the same question. + // + // ⚠️ This is the SHARED index every field-existence rule in this package + // resolves through (`object-graph.ts`), not a second field-set reader written + // for this rule — the three skips it encodes (an object this stack does not + // define, an object with no readable field map, registry-injected system + // columns) are exactly the three this rule must take, and re-deriving them + // here would be the drift that module exists to prevent. + const flsGraph: ObjectGraph = indexObjectGraph(stack); + // ── ADR-0066 / D5/D9: permission-set posture ───────────────────────── for (let i = 0; i < permissionSets.length; i++) { const ps = permissionSets[i]; @@ -621,6 +635,114 @@ export function validateSecurityPosture(stack: AnyRec, opts?: { nowMs?: number } }); } + // [#16108] …and the OTHER half of the same failure: a key that IS + // object-qualified but names a field the object does not have. + // + // Deliberately a SECOND rule beside `security-fls-unqualified-key` rather + // than a widening of it. That rule's own id says *unqualified*, and it is + // correct inside that scope; the two defects have different prescriptions + // (add the object prefix / fix the field name) and an author who suppresses + // one must not thereby suppress the other. Two ids, two messages, two + // loops over the same map, with disjoint guards. + // + // The runtime consequence is IDENTICAL, and it is the fail-OPEN direction. + // `PermissionEvaluator.getFieldPermissions` keeps a key only when + // `key.startsWith(`${objectName}.`)` and then reads the remainder as a + // column name (`key.substring(objectName.length + 1)`), so a remainder no + // column answers to contributes nothing to the merged map: the entry the + // author wrote to MASK a field masks nothing, and the field stays exactly + // as readable and as editable as the object-level grant leaves it. Silent + // at author time, silent at runtime, and — unlike the bare key — it LOOKS + // right in review. It is what a field rename leaves behind, which is why + // it accumulates rather than being caught once. + // + // Splitting on the FIRST dot mirrors that evaluator exactly, because an + // object name cannot contain one: `ObjectSchema.name` is + // `/^[a-z_][a-z0-9_]*$/`. So for any key the evaluator would attribute to + // object O, the head here IS O — and a key with more dots still + // (`crm_account.owner.name`) is judged on the whole remainder, which is + // also what the evaluator looks up: FLS keys address columns, never joins. + // + // `error`, on the inverse of the usual ADR-0049 argument and the same one + // `security-cbp-ambiguous-relation` above makes: there is no runtime + // refusal to mirror BECAUSE the runtime does not refuse — it silently + // ignores the key — so author time is the only place this can ever + // surface. It meets the same admissibility bar: decidable from the + // documents in front of the linter, no per-permission-set nuance to + // adjudicate, and no legitimate reading (a key that can never match is not + // an author saying which field they meant). + // + // The three skips are the graph's, not this rule's, and each is the + // difference between a finding and a false one (ADR-0072 D1): an object + // this stack does not define may be another installed package's; an object + // with no readable field map (ADR-0015 `external`, an introspected + // datasource) resolves its columns at runtime; and a registry-injected + // system column (`created_at`, `owner_id` where ownership provides one) is + // real and addressable while appearing in no authored `fields`. + for (const flsKey of Object.keys(flsMap)) { + const dot = flsKey.indexOf('.'); + if (dot < 0) continue; // the bare-key shape — judged by the rule above + const flsObject = flsKey.slice(0, dot); + const flsField = flsKey.slice(dot + 1); + if (!flsGraph.has(flsObject)) continue; // skip 1: not this stack's object + const surface = flsGraph.get(flsObject); + if (!surface) continue; // skip 2: no readable field map + if (surface.names.has(flsField) || surface.injected.has(flsField)) continue; // resolves (skip 3 included) + + // [#16108] The EMPTY remainder (`'crm_account.'`) is the same defect and + // is reported by this rule, not deferred to the schema. + // + // ⚠️ An earlier revision skipped it with the comment "a shape the schema + // owns". That was FALSE, and measured to be: `PermissionSetSchema.fields` + // is `z.record(z.string(), FieldPermissionSchema)` + // (`packages/spec/src/security/permission.zod.ts`) — a bare `z.string()` + // key with no `.regex`, and that file carries no `refine`/`superRefine` + // at all — and this loop is the only reader of permission-set `fields` + // keys in this package. So nothing anywhere reported it, while at runtime + // it passes `startsWith('crm_account.')` and resolves to the empty column + // name, matching no column: the same fail-open this rule exists to close. + // ⛔ A comment crediting coverage to a component that has none is how a + // real gap gets recorded as handled — the accounting trap this card's own + // downstream note is about, one level in. + // + // It is judged HERE, after the two object skips, and deliberately not + // before them. An empty field name is in fact unmatchable independently + // of the object — `FieldSchema.name` is `/^[a-z_][a-z0-9_]*$/`, so no + // object of any package can declare one — but hoisting the check above + // skip 1 would start this rule judging keys whose OBJECT half it cannot + // resolve, which is exactly the disposition `'.description'` (empty + // object name) and a mis-cased object name are deliberately left to. One + // story, one guard order: the object must be visible before the key is + // judged. `'no_such_object.'` therefore falls to skip 1, like every other + // key naming an object this stack does not define. + const emptyField = flsField.length === 0; + + const declared = [...surface.names].sort(); + const roster = declared.length <= 12 ? declared.join(', ') : `${declared.slice(0, 12).join(', ')}, …`; + const wrote = emptyField + ? `field-permission key '${flsKey}' names NO field — everything after the '${flsObject}.' prefix is empty` + : `field-permission key '${flsKey}' is object-qualified but "${flsObject}" declares no field '${flsField}'`; + const looksRight = emptyField + ? `A truncated key is what a half-finished edit leaves behind.` + : `Unlike an unqualified key this one looks correct in review, and it is exactly what a field rename leaves behind.`; + findings.push({ + severity: 'error', + rule: SECURITY_FLS_UNKNOWN_FIELD, + where: `permission set "${psName}"`, + path: `${psPath}.fields["${flsKey}"]`, + message: + `${wrote}. The runtime resolves an FLS key by stripping the '${flsObject}.' prefix and ` + + `looking the remainder up as a column, so this key matches NOTHING: the masking it declares ` + + `NEVER ENFORCES, and the field it was meant to cover stays as readable and as editable as the ` + + `object-level grant leaves it — for every holder of this set. Nothing reports that at runtime. ` + + `${looksRight}`, + hint: + `Point the key at a field "${flsObject}" really declares (${roster}), or delete the entry if the ` + + `field is gone — an entry that cannot match is not protection. If the masking is still wanted, ` + + `renaming the key is the fix; if the field was renamed, the mask has been off since that rename.`, + }); + } + const wildcard = objectsMap['*'] as AnyRec | undefined; if (wildcard && (wildcard.viewAllRecords === true || wildcard.modifyAllRecords === true)) { findings.push({