From c231a152e8bf00b9baab5646bf72a132aeaa4dc2 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 9 Sep 2026 00:55:50 +0000 Subject: [PATCH 1/4] wip: security-fls-unknown-field rule + tests --- packages/lint/src/authoring-rules.ts | 2 +- packages/lint/src/index.ts | 1 + ...e-security-posture.runtime-surface.test.ts | 9 +- .../src/validate-security-posture.test.ts | 187 +++++++++++++++++- .../lint/src/validate-security-posture.ts | 93 ++++++++- 5 files changed, 287 insertions(+), 5 deletions(-) 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..1b5b0bf63a 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,174 @@ 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 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 +1109,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 +1230,8 @@ 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. + '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 +1343,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 +1368,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 +1445,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..a366881dc1 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,83 @@ 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 (!flsField) continue; // `'crm_account.'` names no field at all — a shape the schema owns + 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) + + const declared = [...surface.names].sort(); + const roster = declared.length <= 12 ? declared.join(', ') : `${declared.slice(0, 12).join(', ')}, …`; + findings.push({ + severity: 'error', + rule: SECURITY_FLS_UNKNOWN_FIELD, + where: `permission set "${psName}"`, + path: `${psPath}.fields["${flsKey}"]`, + message: + `field-permission key '${flsKey}' is object-qualified but "${flsObject}" declares no field ` + + `'${flsField}'. 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. ` + + `Unlike an unqualified key this one looks correct in review, and it is exactly what a field ` + + `rename leaves behind.`, + 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({ From 79a4f5ef5cefd52aa32ec003a8e3c2abab8c267e Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 9 Sep 2026 01:00:07 +0000 Subject: [PATCH 2/4] feat(lint): report an object-qualified FLS key naming a field the object does not declare A permission set key like 'crm_account.description_nope' is stripped of its object prefix by PermissionEvaluator.getFieldPermissions and the remainder is looked up as a column, so a remainder no column answers to contributes nothing to the merged permission map: the declared masking never enforces and the field stays readable to every holder of the set. security-fls-unqualified-key caught only the bare spelling; this adds security-fls-unknown-field beside it, without touching it. Existence resolves through object-graph.ts, the shared field index this package already uses, so the rule inherits its three skips (object not in this stack, no readable field map, registry-injected system columns) rather than deriving a second opinion about what a field is. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_012GKcPZbMoGq7WPzKLfRBTU --- .changeset/security-fls-unknown-field.md | 15 +++++++++++++++ 1 file changed, 15 insertions(+) create mode 100644 .changeset/security-fls-unknown-field.md diff --git a/.changeset/security-fls-unknown-field.md b/.changeset/security-fls-unknown-field.md new file mode 100644 index 0000000000..409d50af36 --- /dev/null +++ b/.changeset/security-fls-unknown-field.md @@ -0,0 +1,15 @@ +--- +"@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`. +- **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. From 61c9cda19d2f8b9993c7f12123821adc6b3ba7c6 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 9 Sep 2026 01:34:30 +0000 Subject: [PATCH 3/4] fix(lint): report the empty FLS remainder instead of deferring it to a schema that does not check it 'crm_account.' passed startsWith at runtime and resolved to the empty column name, matching nothing -- the same fail open security-fls-unknown-field exists to close. It was skipped with a comment claiming the schema owned the shape. PermissionSetSchema.fields is z.record(z.string(), FieldPermissionSchema) with no key regex and no refine in that file, and this loop is the package's only reader of those keys, so nothing reported it. The false sentence is replaced by the measurement, and a pin against the live schema keeps the premise from rotting silently. Judged after the two object skips, not before them: an empty field name is unmatchable independently of the object, but hoisting the check would start the rule judging keys whose object half it cannot resolve. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_012GKcPZbMoGq7WPzKLfRBTU --- .../src/validate-security-posture.test.ts | 70 +++++++++++++++++++ .../lint/src/validate-security-posture.ts | 41 +++++++++-- 2 files changed, 106 insertions(+), 5 deletions(-) diff --git a/packages/lint/src/validate-security-posture.test.ts b/packages/lint/src/validate-security-posture.test.ts index 1b5b0bf63a..dada2360ed 100644 --- a/packages/lint/src/validate-security-posture.test.ts +++ b/packages/lint/src/validate-security-posture.test.ts @@ -330,6 +330,75 @@ describe('validateSecurityPosture (ADR-0090 D7)', () => { 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], @@ -1231,6 +1300,7 @@ describe('validateSecurityPosture — reads only keys the spec declares (meta-te '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. diff --git a/packages/lint/src/validate-security-posture.ts b/packages/lint/src/validate-security-posture.ts index a366881dc1..a95310e35a 100644 --- a/packages/lint/src/validate-security-posture.ts +++ b/packages/lint/src/validate-security-posture.ts @@ -684,27 +684,58 @@ export function validateSecurityPosture(stack: AnyRec, opts?: { nowMs?: number } 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 (!flsField) continue; // `'crm_account.'` names no field at all — a shape the schema owns 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: - `field-permission key '${flsKey}' is object-qualified but "${flsObject}" declares no field ` + - `'${flsField}'. The runtime resolves an FLS key by stripping the '${flsObject}.' prefix and ` + + `${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. ` + - `Unlike an unqualified key this one looks correct in review, and it is exactly what a field ` + - `rename leaves behind.`, + `${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, ` + From b84b9e23f28b05a77c5958253a5ae0a7653a1938 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 9 Sep 2026 01:37:01 +0000 Subject: [PATCH 4/4] docs(changeset): cover the empty FLS remainder Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_012GKcPZbMoGq7WPzKLfRBTU --- .changeset/security-fls-unknown-field.md | 1 + 1 file changed, 1 insertion(+) diff --git a/.changeset/security-fls-unknown-field.md b/.changeset/security-fls-unknown-field.md index 409d50af36..0c3782ac14 100644 --- a/.changeset/security-fls-unknown-field.md +++ b/.changeset/security-fls-unknown-field.md @@ -10,6 +10,7 @@ The failure direction is **fail open**, and this spelling is the one that accumu - **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.