From f85110db2abcffbc7ebc93977fb58358413e34b1 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 5 Nov 2026 06:54:53 +0000 Subject: [PATCH 1/2] fix(lint): resolve FLS keys against the object as the registry serves it, extension fields included security-fls-unknown-field read an object's declared fields only, so a permission set naming a field that an objectExtensions entry adds (a sibling package's in a multi-package artifact, or the stack's own) was refused, although the runtime serves the field and the key masks it. The FLS-key index is now built over objectsAsServed(stack): the shared extension fold in object-graph.ts, mirroring the objectql registry (fields by name, one entry per package per object, extend verbatim). The controlled-by-parent merge (#23210) and the membership-key check read the same fold instead of their own copies. Co-Authored-By: Claude Claude-Session: https://claude.ai/code/session_01WGufzfKwxFVA5m6bAjSpxf --- .changeset/30303-fls-key-extension-fields.md | 12 ++ packages/lint/src/object-graph.ts | 128 ++++++++++++ packages/lint/src/rule-explanations.ts | 3 +- .../validate-membership-key-declarations.ts | 72 +------ ...ate-security-posture-fls-extension.test.ts | 184 ++++++++++++++++++ .../src/validate-security-posture.test.ts | 19 +- .../lint/src/validate-security-posture.ts | 67 +++---- 7 files changed, 372 insertions(+), 113 deletions(-) create mode 100644 .changeset/30303-fls-key-extension-fields.md create mode 100644 packages/lint/src/validate-security-posture-fls-extension.test.ts diff --git a/.changeset/30303-fls-key-extension-fields.md b/.changeset/30303-fls-key-extension-fields.md new file mode 100644 index 00000000000..8b15154863d --- /dev/null +++ b/.changeset/30303-fls-key-extension-fields.md @@ -0,0 +1,12 @@ +--- +'@objectstack/lint': patch +--- + +fix(lint): a permission set's field key on a field that an `objectExtensions` entry adds, a sibling package's in the same artifact included, is no longer refused as `security-fls-unknown-field` (#30303) + +Clause-②: yes (widening: the FLS-key check accepts extension-added fields) +Metadata-protocol: none + +- **Before.** The FLS-key check resolved `object.field` against the object's declared `fields` only. On a multi-package artifact, an app package's permission set naming `crm_contract.o2c_probe_note`, a field its module package adds to `crm_contract` through `objectExtensions`, failed `os build`, `os validate` and `os lint` with "crm_contract declares no field o2c_probe_note", although the runtime serves the field and the key masks it. A single-package stack's own extension fields were refused the same way. +- **Now.** The key resolves against the object as the registry serves it: its declared fields plus every field an `objectExtensions` entry of any body of the artifact adds to it, folded as `@objectstack/objectql` folds them (fields by name; one entry per package per object, a later entry replacing an earlier one; `extend` naming the object verbatim). A field that no body declares or adds keeps the same refusal, with the same rule id, severity and message, and so does a field an entry only patches through `fieldPatches`, which adds no field. The hint's nearest name reads the served fields too. +- **One fold.** The controlled-by-parent merge (#23210), the membership-key column check and the FLS-key check now read extensions through one shared fold in `object-graph.ts`. diff --git a/packages/lint/src/object-graph.ts b/packages/lint/src/object-graph.ts index b07923dd287..76abadb6999 100644 --- a/packages/lint/src/object-graph.ts +++ b/packages/lint/src/object-graph.ts @@ -365,6 +365,134 @@ export function indexObjectGraph(stack: unknown): ObjectGraph { return graph; } +/** + * One `objectExtensions` entry as the registry registers it — see + * {@link objectExtensionsByTarget}. + */ +export interface VisibleObjectExtension { + /** The position of the package that ships it: registration order across packages. */ + carrier: number; + /** The entry's path, e.g. `objectExtensions[0]` or `packages[1].manifest.objectExtensions[0]`. */ + path: string; + /** `ext.priority ?? 200`, the registry's default. */ + priority: number; + /** Its field records, name folded in ({@link recordsOf}). */ + fields: AnyRec[]; +} + +/** + * [#30303] Every `objectExtensions` entry of a stack, by the object it + * `extend`s, as the registry registers and folds them. This is the ONE place + * this package reads extensions for an object's merged field set: the + * `controlled_by_parent` merge of `validate-security-posture.ts` (#23210), + * the membership-key columns of `validate-membership-key-declarations.ts`, + * and the served field set {@link objectsAsServed} hands the FLS-key check. + * + * Read as the registry reads a manifest, since `@objectstack/lint` imports no + * runtime (`ObjectQL.registerApp`'s `objectExtensions` loop, + * `SchemaRegistry.registerObject`'s `extend` arm and `foldExtenders`, in + * `@objectstack/objectql`): + * + * - Source: with `packages[]`, every body's `objectExtensions` + * (`packages[j].manifest`, `carrier` = `j`, the artifact's registration + * order); without it, the stack's own (`carrier` 0). An artifact's top level + * repeats its bodies' collections, so it is not read beside them. + * - `extend` names its target verbatim. + * - ONE entry per package per target: registering an extension removes that + * package's earlier one, so a later entry of the same package replaces an + * earlier one whole, its fields included. + * - Fold order: ascending `priority` (default 200), equal priorities in + * package order. Each list is returned in that order. + * + * Which targets to judge is the caller's question: an entry is listed here + * whether or not the stack declares its target. + */ +export function objectExtensionsByTarget(stack: unknown): Map { + const byTarget = new Map(); + if (!isRec(stack)) return byTarget; + // The receivers are named as `validate-security-posture.test.ts`'s key-read + // meta-test tables them (`stack`, `entry`, `body`, `ext`, `obj`, `f`): that + // scan reads this fold as part of the rules that read through it. + const collect = (entries: unknown, carrier: number, at: string) => { + if (!Array.isArray(entries)) return; + entries.forEach((ext: unknown, i: number) => { + if (!isRec(ext) || typeof ext.extend !== 'string') return; + const kept = (byTarget.get(ext.extend) ?? []).filter((other) => other.carrier !== carrier); + kept.push({ + carrier, + path: `${at}objectExtensions[${i}]`, + priority: typeof ext.priority === 'number' ? ext.priority : 200, + fields: recordsOf(ext.fields), + }); + byTarget.set(ext.extend, kept); + }); + }; + if (Array.isArray(stack.packages)) { + stack.packages.forEach((entry: unknown, j: number) => { + const body = isRec(entry) ? entry.manifest : undefined; + if (isRec(body)) collect(body.objectExtensions, j, `packages[${j}].manifest.`); + }); + } else { + collect(stack.objectExtensions, 0, ''); + } + for (const list of byTarget.values()) { + list.sort((left, right) => left.priority - right.priority || left.carrier - right.carrier); + } + return byTarget; +} + +/** + * [#30303] A stack's `objects` as the registry SERVES them: each with every + * {@link objectExtensionsByTarget} entry that extends it folded in, the way + * `mergeObjectDefinitions` merges `fields` (by name, additively, a later + * writer replacing the whole field). An object nothing extends is returned as + * it is. + * + * Only the fields move. The registry plans an object's injected system + * columns over its base layer (`foldExtenders` reads + * `resolveInjectedSystemColumns(baseDefinition)`), and that plan reads no + * field, so {@link indexObjectGraph} over these objects injects what it + * injects over the declared ones. `fieldPatches` add no field (they patch the + * owner's own), so they are not read. + * + * A field two packages write at the priority that decides it is merged (its + * name is served whichever wins) and named in `tied`, for a caller whose + * verdict depends on which definition wins: registration order decides that, + * and a stack does not state it. + */ +export function objectsAsServed(stack: unknown): { objects: AnyRec[]; tied: Map> } { + const objects = isRec(stack) ? recordsOf(stack.objects) : []; + const tied = new Map>(); + const extensions = objectExtensionsByTarget(stack); + if (extensions.size === 0) return { objects, tied }; + + const served = objects.map((obj) => { + const name = strName(obj.name); + const exts = name ? extensions.get(name) : undefined; + if (!name || !exts) return obj; + const fields = new Map(); + for (const f of recordsOf(obj.fields)) { + const key = strName(f.name); + if (key) fields.set(key, f); + } + const lastWriter = new Map(); + const tiedKeys = new Set(); + for (const ext of exts) { + for (const f of ext.fields) { + const key = strName(f.name); + if (!key) continue; + if (lastWriter.get(key)?.priority === ext.priority) tiedKeys.add(key); + else tiedKeys.delete(key); + lastWriter.set(key, ext); + fields.set(key, f); + } + } + if (tiedKeys.size > 0) tied.set(name, tiedKeys); + return { ...obj, fields: [...fields.values()] }; + }); + return { objects: served, tied }; +} + /** What a path resolved to, or precisely how it failed. */ export type FieldPathVerdict = /** diff --git a/packages/lint/src/rule-explanations.ts b/packages/lint/src/rule-explanations.ts index 65fc06d36af..c97287e6e6b 100644 --- a/packages/lint/src/rule-explanations.ts +++ b/packages/lint/src/rule-explanations.ts @@ -819,7 +819,8 @@ const SECURITY_FLS_UNKNOWN_FIELD_EXPLANATION: RuleExplanation = { 'Unlike an unqualified key (`security-fls-unqualified-key`) this one looks correct in review, and ' + 'it is exactly what a field rename leaves behind; an empty remainder is what a half-finished edit ' + 'leaves. A key with more dots is judged on its whole remainder, because FLS keys address columns, ' + - 'never joins.', + 'never joins. The columns are the object\'s as the registry serves it: the fields it declares and ' + + 'every field an `objectExtensions` entry adds to it, from any package of the same artifact.', 'Not judged: an object this stack does not define, an object with no readable field map, and a ' + 'registry-injected system column, which is real and addressable. To fix it, point the key at a ' + 'field the object declares, or delete the entry: an entry that cannot match is not protection, ' + diff --git a/packages/lint/src/validate-membership-key-declarations.ts b/packages/lint/src/validate-membership-key-declarations.ts index 3a42a1c97bb..b16b64eac84 100644 --- a/packages/lint/src/validate-membership-key-declarations.ts +++ b/packages/lint/src/validate-membership-key-declarations.ts @@ -66,6 +66,7 @@ import { indexObjectGraph, isUnjudgeable, + objectsAsServed, recordsOf, resolveFieldPath, suggestName, @@ -205,75 +206,6 @@ export function stackObjectNamesOf(stack: unknown): Set { return names; } -/** One `objectExtensions` entry, as the registry folds it. */ -interface VisibleExtension { - /** Package order: registration order across packages. */ - carrier: number; - /** `priority ?? 200`, the registry's default. */ - priority: number; - fields: AnyRec[]; -} - -/** - * The stack's objects with every `objectExtensions` entry folded in, as the - * registry merges them (this file's header). A field two packages write at the - * priority that decides it is left out of the merged map and named in - * `tied`, so its verdict is "not judged" rather than either writer's. - */ -function mergedObjectsOf(stack: AnyRec): { objects: AnyRec[]; tied: Map> } { - const objects = recordsOf(stack.objects); - const tied = new Map>(); - const names = new Set(objects.map((obj) => str(obj.name)).filter(Boolean)); - const extensions = new Map(); - const collect = (entries: unknown, carrier: number) => { - if (!Array.isArray(entries)) return; - for (const entry of entries) { - if (!isRec(entry) || typeof entry.extend !== 'string' || !names.has(entry.extend)) continue; - const list = extensions.get(entry.extend) ?? []; - // One per package per target: the later entry wins. - const kept = list.filter((ext) => ext.carrier !== carrier); - kept.push({ - carrier, - priority: typeof entry.priority === 'number' ? entry.priority : 200, - fields: recordsOf(entry.fields), - }); - extensions.set(entry.extend, kept); - } - }; - if (Array.isArray(stack.packages)) { - stack.packages.forEach((pkg: unknown, j: number) => { - if (isRec(pkg) && isRec(pkg.manifest)) collect(pkg.manifest.objectExtensions, j); - }); - } else { - collect(stack.objectExtensions, 0); - } - if (extensions.size === 0) return { objects, tied }; - - const merged = objects.map((obj) => { - const name = str(obj.name); - const exts = extensions.get(name); - if (!exts) return obj; - const fields = new Map(); - for (const f of recordsOf(obj.fields)) if (str(f.name)) fields.set(str(f.name), f); - const lastWriter = new Map(); - const tiedKeys = new Set(); - for (const ext of [...exts].sort((a, b) => a.priority - b.priority || a.carrier - b.carrier)) { - for (const f of ext.fields) { - const key = str(f.name); - if (!key) continue; - const previous = lastWriter.get(key); - if (previous && previous.priority === ext.priority) tiedKeys.add(key); - else tiedKeys.delete(key); - lastWriter.set(key, ext); - fields.set(key, f); - } - } - if (tiedKeys.size > 0) tied.set(name, tiedKeys); - return { ...obj, fields: [...fields.values()] }; - }); - return { objects: merged, tied }; -} - /** U's verdict: `null` when it is a single-valued user reference, else the reason. */ function userFieldFault(meta: GraphField): string | null { const isUserRef = @@ -332,7 +264,7 @@ export function validateMembershipKeyDeclarations( const declared = declaredMembershipKeysOf(stack); if (declared.length === 0) return findings; - const { objects: merged, tied } = mergedObjectsOf(stack); + const { objects: merged, tied } = objectsAsServed(stack); const graph = indexObjectGraph({ objects: merged }); const stackNames = stackObjectNamesOf(stack); const owners = options.membershipKeyOwners; diff --git a/packages/lint/src/validate-security-posture-fls-extension.test.ts b/packages/lint/src/validate-security-posture-fls-extension.test.ts new file mode 100644 index 00000000000..283454decad --- /dev/null +++ b/packages/lint/src/validate-security-posture-fls-extension.test.ts @@ -0,0 +1,184 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#30303] `security-fls-unknown-field` resolves an `object.field` key against + * the object as the registry SERVES it: its declaration with every + * `objectExtensions` entry that extends it folded in, a sibling package body's + * included (`objectsAsServed`, the fold the controlled-by-parent merge of + * #23210 reads too). + * + * The card's shape: an app package owns `crm_contract` and the permission + * sets; a module package of the same artifact adds `o2c_probe_note` to + * `crm_contract` through `objectExtensions`. The runtime serves the field, so + * an app-package set may name it. A key no body declares or extends is still + * refused, with the message it always had, and so is a field an extension + * names in a way the registry does not merge. + * + * Each stack is parsed by `ObjectStackDefinitionSchema` first and the rule + * runs on the parsed tree, the tier it is registered on (`input: 'parsed'`). + */ +import { describe, it, expect } from 'vitest'; +import { ObjectStackDefinitionSchema } from '@objectstack/spec'; +import { validateSecurityPosture, SECURITY_FLS_UNKNOWN_FIELD } from './validate-security-posture.js'; + +type AnyRec = Record; + +const CONTRACT = { + name: 'crm_contract', + label: 'Contract', + sharingModel: 'private', + fields: { contract_no: { type: 'text', label: 'Contract No' } }, +}; + +const setNaming = (...keys: string[]) => ({ + name: 'finance_editor', + label: 'Finance Editor', + objects: { crm_contract: { allowRead: true, allowEdit: true } }, + fields: Object.fromEntries(keys.map((key) => [key, { readable: true, editable: true }])), +}); + +const NOTE = { o2c_probe_note: { type: 'textarea', label: 'O2C Probe Note' } }; + +const body = (id: string, type: string, extra: AnyRec) => ({ + manifest: { id, name: id, version: '1.0.0', type, ...extra }, +}); + +/** The app package: the object and the permission set. */ +const appBody = (...keys: string[]) => + body('app.objectstack.hotcrm', 'app', { objects: [CONTRACT], permissions: [setNaming(...keys)] }); + +/** The module package: its extensions of the app's object. */ +const moduleBody = (objectExtensions: AnyRec[]) => + body('app.objectstack.hotcrm.o2c', 'module', { objectExtensions }); + +const parsed = (stack: AnyRec): AnyRec => { + const result = ObjectStackDefinitionSchema.safeParse(stack); + expect(result.success, result.success ? '' : JSON.stringify(result.error.issues, null, 2)).toBe(true); + return result.data as AnyRec; +}; + +/** + * The two stacks the CLI hands the rule for one artifact + * (`packages/cli/src/utils/artifact-packages.ts`, `stack-collections.ts`): + * the per-package run over the app body (`packageBodyAsStack`: the body, with + * the artifact's `packages[]` as resolution context), and the union run (the + * bodies' collections folded onto the top level, `authoringRuleUnionStack`). + */ +const runsOf = (artifact: AnyRec): Array<[string, AnyRec]> => { + const packages = artifact.packages as Array<{ manifest: AnyRec }>; + const app = packages.find((p) => p.manifest.type === 'app')!.manifest; + const union: AnyRec = { ...artifact }; + for (const key of ['objects', 'permissions', 'objectExtensions']) { + const items = packages.flatMap((p) => (Array.isArray(p.manifest[key]) ? (p.manifest[key] as unknown[]) : [])); + if (items.length > 0) union[key] = items; + } + return [ + ['per-package (app body)', { ...app, manifest: app, packages }], + ['union', union], + ]; +}; + +const flsFindings = (stack: AnyRec) => + validateSecurityPosture(stack).filter((f) => f.rule === SECURITY_FLS_UNKNOWN_FIELD); + +describe('[#30303] security-fls-unknown-field reads the object as the registry serves it', () => { + it('the card\'s artifact: an app-package set naming a field a sibling module extends is accepted, on both runs', () => { + const artifact = parsed({ + packages: [appBody('crm_contract.o2c_probe_note'), moduleBody([{ extend: 'crm_contract', fields: NOTE }])], + }); + for (const [run, stack] of runsOf(artifact)) { + expect(flsFindings(stack), run).toEqual([]); + } + }); + + it('the same artifact without the module\'s extension still refuses the key (the control the pin above stands beside)', () => { + const artifact = parsed({ packages: [appBody('crm_contract.o2c_probe_note'), moduleBody([])] }); + for (const [run, stack] of runsOf(artifact)) { + expect(flsFindings(stack).map((f) => f.path), run).toEqual(['permissions[0].fields["crm_contract.o2c_probe_note"]']); + } + }); + + it('a key on a field no body declares or extends is still refused, in the words it always had', () => { + const artifact = parsed({ + packages: [ + appBody('crm_contract.o2c_probe_note', 'crm_contract.o2c_nowhere'), + moduleBody([{ extend: 'crm_contract', fields: NOTE }]), + ], + }); + // The control: one object, no extension anywhere, the same unknown key. + const control = flsFindings(parsed({ objects: [CONTRACT], permissions: [setNaming('crm_contract.o2c_nowhere')] })); + expect(control).toHaveLength(1); + for (const [run, stack] of runsOf(artifact)) { + const findings = flsFindings(stack); + expect(findings.map((f) => [f.severity, f.path]), run).toEqual([ + ['error', 'permissions[0].fields["crm_contract.o2c_nowhere"]'], + ]); + expect(findings[0].where, run).toBe(control[0].where); + expect(findings[0].message, run).toBe(control[0].message); + } + }); + + it('a single-package stack with an in-body extension: no finding; without the extension the key is refused', () => { + const stack = (objectExtensions: AnyRec[]) => + parsed({ objects: [CONTRACT], objectExtensions, permissions: [setNaming('crm_contract.o2c_probe_note')] }); + expect(flsFindings(stack([{ extend: 'crm_contract', fields: NOTE }]))).toEqual([]); + expect(flsFindings(stack([])).map((f) => f.path)).toEqual(['permissions[0].fields["crm_contract.o2c_probe_note"]']); + }); + + it('the hint\'s nearest name reads the served fields too', () => { + const artifact = parsed({ + packages: [appBody('crm_contract.o2c_probe_notes'), moduleBody([{ extend: 'crm_contract', fields: NOTE }])], + }); + for (const [run, stack] of runsOf(artifact)) { + expect(flsFindings(stack).map((f) => f.hint), run).toEqual([ + 'Point the key at a field "crm_contract" declares (\'crm_contract.o2c_probe_note\' is the nearest), or delete the entry if the field is gone', + ]); + } + }); +}); + +// ── Extensions the registry does not merge stay refused ────────────────── +// +// `ObjectQL.registerApp` registers each entry as an `extend` contribution +// under the name its `extend` spells (`SchemaRegistry.registerObject`), and a +// package's later contribution to one object replaces its earlier one; the +// fold merges each contribution's `fields`, and its `fieldPatches` only patch +// the owner's own fields. Each case below is a field the served object does +// not have. +describe('[#30303] a field the registry would not serve is still refused', () => { + const refusedOn = (objectExtensions: AnyRec[]) => { + const artifact = parsed({ packages: [appBody('crm_contract.o2c_probe_note'), moduleBody(objectExtensions)] }); + return runsOf(artifact).map(([run, stack]) => [run, flsFindings(stack).map((f) => f.path)]); + }; + const REFUSED = ['permissions[0].fields["crm_contract.o2c_probe_note"]']; + + it('a field one package\'s earlier entry adds, when a later entry of the same package extends the same object', () => { + const replaced = [ + { extend: 'crm_contract', fields: NOTE }, + { extend: 'crm_contract', fields: { o2c_other: { type: 'text', label: 'Other' } } }, + ]; + expect(refusedOn(replaced)).toEqual([ + ['per-package (app body)', REFUSED], + ['union', REFUSED], + ]); + // Its twin, the same two entries in the other order: the later one adds the field. + expect(refusedOn([...replaced].reverse())).toEqual([ + ['per-package (app body)', []], + ['union', []], + ]); + }); + + it('a field an extension only patches (fieldPatches add no field)', () => { + expect(refusedOn([{ extend: 'crm_contract', fieldPatches: { o2c_probe_note: { label: 'Probe' } } }])).toEqual([ + ['per-package (app body)', REFUSED], + ['union', REFUSED], + ]); + }); + + it('a field an extension adds to an object of another spelling', () => { + expect(refusedOn([{ extend: 'crm__crm_contract', fields: NOTE }])).toEqual([ + ['per-package (app body)', REFUSED], + ['union', REFUSED], + ]); + }); +}); diff --git a/packages/lint/src/validate-security-posture.test.ts b/packages/lint/src/validate-security-posture.test.ts index 7c93e71b8bb..ea802e06090 100644 --- a/packages/lint/src/validate-security-posture.test.ts +++ b/packages/lint/src/validate-security-posture.test.ts @@ -1648,7 +1648,21 @@ const RULE_CODE = RULE_SOURCE.replace(/\/\*[\s\S]*?\*\//g, '') // PLUMBING as though it were a local variable, which is a lie the next // author would have to re-derive. Strip the import block instead; nothing in // it is a read off a metadata record. - .replace(/^import[\s\S]*?from '[^']*';$/gm, ''); + .replace(/^import[\s\S]*?from '[^']*';$/gm, '') + // [#30303] The extension fold the controlled-by-parent merge and the FLS-key + // check read through (`objectExtensionsByTarget`, `objectsAsServed`) lives in + // `object-graph.ts`, and its reads off `stack`, `entry`, `body` and `ext` are + // this rule's reads: scanned here, so they stay held to the tables below. + + foldCode(); + +/** The shared extension fold's code in `object-graph.ts`, comments stripped. */ +function foldCode(): string { + const source = readFileSync(new URL('./object-graph.ts', import.meta.url), 'utf8'); + const start = source.indexOf('export interface VisibleObjectExtension'); + const end = source.indexOf('/** What a path resolved to'); + if (start < 0 || end <= start) throw new Error('object-graph.ts: the extension fold was not found; re-point this scan'); + return '\n' + source.slice(start, end).replace(/\/\*[\s\S]*?\*\//g, '').replace(/\/\/[^\n]*/g, ''); +} /** Distinct property names read off `receiver` in the rule's code. */ function keysReadOff(receiver: string): string[] { @@ -1966,6 +1980,9 @@ describe('validateSecurityPosture — reads only keys the spec declares (meta-te 'ownershipGap', // #29544: the inert-mask arm's list of unbound bits — `.filter` / `.join` / `.length`, JS methods. 'inertBits', + // #30303: the shared extension fold's own collections (`object-graph.ts`, scanned with this rule) — + // JS Map / Set / Array methods, and `other.carrier`, the fold's own record of a package's entry. + 'byTarget', 'kept', 'other', 'list', 'lastWriter', 'tiedKeys', 'tied', ]); expect(receivers.filter((r) => !tabled.has(r) && !PLUMBING.has(r))).toEqual([]); }); diff --git a/packages/lint/src/validate-security-posture.ts b/packages/lint/src/validate-security-posture.ts index ccd61918d86..cdfe1d906f6 100644 --- a/packages/lint/src/validate-security-posture.ts +++ b/packages/lint/src/validate-security-posture.ts @@ -124,7 +124,16 @@ import { parseCelToAst, sqlPredicateToCel, type CelAstNode } from '@objectstack/formula'; import { controlledByParentOwnerMayEdit, isControlledByParentMasterField, referenceCarrierOf } from '@objectstack/spec/data'; import { describeAnchorForbiddenBits, type AnchorBindingContext } from '@objectstack/spec/security'; -import { indexObjectGraph, nearestName, packagesOf, recordsOf, type ObjectGraph } from './object-graph.js'; +import { + indexObjectGraph, + nearestName, + objectExtensionsByTarget, + objectsAsServed, + packagesOf, + recordsOf, + type ObjectGraph, + type VisibleObjectExtension, +} from './object-graph.js'; import { injectedColumnsFor } from './system-fields.js'; import { PUBLIC_READ_WRITE_WAY_OUT, @@ -666,18 +675,6 @@ function pushCbpAmbiguousRelation( }); } -/** One `objectExtensions` entry as the registry registers it — see {@link pushExtensionMergedCbpFindings}. */ -interface VisibleExtension { - /** The position of the package that ships it: registration order across packages. */ - carrier: number; - /** The entry's path, e.g. `objectExtensions[0]`. */ - path: string; - /** `ext.priority ?? 200`, the registry's default. */ - priority: number; - /** Its field records, name folded in (`recordsOf`). */ - fields: AnyRec[]; -} - /** * [#23210 · ADR-0055 §1] `security-controlled-by-parent-no-relation` and * `security-controlled-by-parent-ambiguous-relation`, judged on the MERGED @@ -746,38 +743,16 @@ function pushExtensionMergedCbpFindings(findings: SecurityFinding[], stack: AnyR declare(recordsOf(stack.objects).filter((obj) => typeof obj.name === 'string' && !owners.has(obj.name)), bodies.length); } - // Target name → its extensions, one per package. - const extensions = new Map>(); - const collect = (entries: unknown, carrier: number, at: string) => { - if (!Array.isArray(entries)) return; - entries.forEach((entry: unknown, i: number) => { - if (!entry || typeof entry !== 'object' || Array.isArray(entry)) return; - const ext = entry as AnyRec; - if (typeof ext.extend !== 'string' || !targets.get(ext.extend)) return; - const byCarrier = extensions.get(ext.extend) ?? new Map(); - byCarrier.set(carrier, { - carrier, - path: `${at}objectExtensions[${i}]`, - priority: typeof ext.priority === 'number' ? ext.priority : 200, - fields: recordsOf(ext.fields), - }); - extensions.set(ext.extend, byCarrier); - }); - }; - if (!multi) collect(stack.objectExtensions, 0, ''); - else bodies.forEach((body, j) => collect(body.objectExtensions, j, `packages[${j}].manifest.`)); - - for (const [target, byCarrier] of extensions) { + // Target name → its extensions, one per package, in fold order: the shared + // fold (#30303), which the FLS-key check below reads too. + for (const [target, ordered] of objectExtensionsByTarget(stack)) { const obj = targets.get(target); if (!obj || owdOf(obj) !== 'controlled_by_parent') continue; if (cbpMasterCandidates(obj).length !== 1) continue; const baseFields = new Map(recordsOf(obj.fields).map((f) => [String(f.name ?? '?'), f] as const)); const merged = new Map(baseFields); - const ordered = [...byCarrier.values()].sort( - (left, right) => left.priority - right.priority || left.carrier - right.carrier, - ); - const writers = new Map(); + const writers = new Map(); // Per key, how many packages wrote it at the priority of its last writer. const tiedWriters = new Map(); for (const visible of ordered) { @@ -793,7 +768,7 @@ function pushExtensionMergedCbpFindings(findings: SecurityFinding[], stack: AnyR const candidates = cbpMasterCandidates({ ...obj, fields: [...merged.values()] }); if (candidates.length === 1) continue; - const blamed = new Set(); + const blamed = new Set(); for (const [key, visible] of writers) { if (merged.get(key)?.type === 'master_detail' || baseFields.get(key)?.type === 'master_detail') blamed.add(visible); } @@ -1031,7 +1006,17 @@ export function validateSecurityPosture(stack: AnyRec, opts?: { nowMs?: number } // 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); + // + // [#30303] …indexed over the objects as the registry SERVES them + // (`objectsAsServed`): with every `objectExtensions` entry that extends one + // folded in, a sibling body's included, through the same fold the + // controlled-by-parent merge above reads. The runtime evaluator matches a + // key against the served object's columns, so a key naming a field an + // extension adds is a key that enforces; read against the declaration + // alone, it was refused, and an app package could grant no access to a + // field its module package adds. A field no body declares or extends is + // still unknown, and the three skips are unchanged. + const flsGraph: ObjectGraph = indexObjectGraph({ objects: objectsAsServed(stack).objects }); // ── ADR-0066 / D5/D9: permission-set posture ───────────────────────── for (let i = 0; i < permissionSets.length; i++) { From 8aeb321249e63c295e811b973421491e1871d3c9 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 5 Nov 2026 08:00:03 +0000 Subject: [PATCH 2/2] test(lint): pin the shared extension fold on junk input; say it does not mirror the legacy-name fold objectExtensionsByTarget and objectsAsServed get the junk-input cases their neighbours have: a null stack, non-record packages entries and bodies, a non-array objectExtensions, non-record entries, junk fields and a string priority. The fold's docblock says it names the target verbatim and does not mirror the registry's short-name to legacy ns__name filing (#28858), so such an entry folds onto nothing and a key naming its field stays refused. Co-Authored-By: Claude Claude-Session: https://claude.ai/code/session_01WGufzfKwxFVA5m6bAjSpxf --- packages/lint/src/object-graph.test.ts | 89 ++++++++++++++++++++++++++ packages/lint/src/object-graph.ts | 9 ++- 2 files changed, 97 insertions(+), 1 deletion(-) diff --git a/packages/lint/src/object-graph.test.ts b/packages/lint/src/object-graph.test.ts index cd4d00a7d7d..99fd9f185a2 100644 --- a/packages/lint/src/object-graph.test.ts +++ b/packages/lint/src/object-graph.test.ts @@ -15,6 +15,8 @@ import { nearestName, listNames, packagesOf, + objectExtensionsByTarget, + objectsAsServed, RELATIONSHIP_FIELD_TYPES, } from './object-graph.js'; import { walkFilterFieldKeys, type FilterFieldKey } from './filter-walk.js'; @@ -345,6 +347,93 @@ describe('object-graph — packagesOf (#20206, ruling A on #15293 `5634034754`)' }); }); +/** + * [#30303] The shared extension fold on junk input, beside the + * `indexObjectGraph` and `packagesOf` cases above. Neither export refuses + * anything: a malformed `packages` is {@link packagesOf}'s refusal, met by a + * caller that reads the bodies through it (`validateSecurityPosture` does, + * before either fold runs). Each shape below reads as "nothing to fold" for + * the part that is junk, and never throws. + */ +describe('[#30303] object-graph — objectExtensionsByTarget / objectsAsServed on junk input', () => { + const OBJ = { name: 'crm_contract', fields: { contract_no: { type: 'text' } } }; + const EXT = { extend: 'crm_contract', fields: { note: { type: 'text' } } }; + const served = (stack: unknown) => + objectsAsServed(stack).objects.map((o) => [o.name, (o.fields as Array<{ name: string }> | Record)]); + + it('a null or non-record stack: no extensions, no objects', () => { + for (const stack of [null, undefined, 'junk', 42, []]) { + expect(objectExtensionsByTarget(stack).size, String(stack)).toBe(0); + expect(objectsAsServed(stack), String(stack)).toEqual({ objects: [], tied: new Map() }); + } + }); + + it('packages: [null] and other non-record entries or bodies contribute nothing; a record body beside them still does', () => { + expect(objectExtensionsByTarget({ packages: [null] }).size).toBe(0); + const stack = { + objects: [OBJ], + packages: [null, 'junk', 42, [], { manifest: null }, { manifest: 'junk' }, { manifest: { objectExtensions: [EXT] } }], + }; + expect([...objectExtensionsByTarget(stack).entries()].map(([t, list]) => [t, list.map((e) => [e.carrier, e.path])])).toEqual([ + ['crm_contract', [[6, 'packages[6].manifest.objectExtensions[0]']]], + ]); + expect(objectsAsServed(stack).objects[0].fields).toEqual([ + { name: 'contract_no', type: 'text' }, + { name: 'note', type: 'text' }, + ]); + }); + + it('a non-array objectExtensions folds nothing', () => { + for (const objectExtensions of [null, 'junk', 42, { crm_contract: EXT }]) { + const stack = { objects: [OBJ], objectExtensions }; + expect(objectExtensionsByTarget(stack).size, JSON.stringify(objectExtensions)).toBe(0); + expect(objectsAsServed(stack).objects, JSON.stringify(objectExtensions)).toEqual([OBJ]); + } + }); + + it('a non-record entry, or one whose extend is not a string, is skipped; the record entry beside it is read', () => { + const stack = { + objects: [OBJ], + objectExtensions: [null, 'junk', 42, [], { extend: 7, fields: { x: { type: 'text' } } }, { fields: { y: {} } }, EXT], + }; + expect([...objectExtensionsByTarget(stack).keys()]).toEqual(['crm_contract']); + expect(objectExtensionsByTarget(stack).get('crm_contract')!.map((e) => e.path)).toEqual(['objectExtensions[6]']); + expect(served(stack)).toEqual([['crm_contract', [{ name: 'contract_no', type: 'text' }, { name: 'note', type: 'text' }]]]); + }); + + it('junk fields add nothing: a non-collection, non-record members, a member with no name', () => { + for (const fields of [null, 'junk', 42, [null, 'x', 3, { type: 'text' }, { name: '', type: 'text' }]]) { + const stack = { objects: [OBJ], objectExtensions: [{ extend: 'crm_contract', fields }] }; + expect(() => objectsAsServed(stack), JSON.stringify(fields)).not.toThrow(); + expect(objectsAsServed(stack).objects[0].fields, JSON.stringify(fields)).toEqual([{ name: 'contract_no', type: 'text' }]); + } + // The array form is read by name, as the map form is. + const arrayForm = { objects: [OBJ], objectExtensions: [{ extend: 'crm_contract', fields: [{ name: 'note', type: 'text' }] }] }; + expect(served(arrayForm)).toEqual([['crm_contract', [{ name: 'contract_no', type: 'text' }, { name: 'note', type: 'text' }]]]); + }); + + // `ObjectExtensionSchema.priority` is a number, so a parsed stack never + // carries this shape; the fold reads a non-number as absent (the default 200, + // as `SchemaRegistry.registerObjectExtension` does) rather than guessing. + it('a string priority reads as the default 200 and never throws', () => { + const stack = { + objects: [OBJ], + packages: [ + { manifest: { objectExtensions: [{ extend: 'crm_contract', priority: '100', fields: { note: { type: 'text', label: 'A' } } }] } }, + { manifest: { objectExtensions: [{ extend: 'crm_contract', priority: 150, fields: { note: { type: 'text', label: 'B' } } }] } }, + ], + }; + expect(objectExtensionsByTarget(stack).get('crm_contract')!.map((e) => [e.carrier, e.priority])).toEqual([ + [1, 150], + [0, 200], + ]); + // So the string-priority entry folds LAST and its definition wins, untied. + const { objects, tied } = objectsAsServed(stack); + expect((objects[0].fields as Array>).find((f) => f.name === 'note')?.label).toBe('A'); + expect(tied.size).toBe(0); + }); +}); + /** * [#19289] A `{ type: 'user' }` field with no `reference` is TRAVERSABLE — the * third defect found by the implicit-target census, and the widest-reaching of diff --git a/packages/lint/src/object-graph.ts b/packages/lint/src/object-graph.ts index 76abadb6999..54c71089aec 100644 --- a/packages/lint/src/object-graph.ts +++ b/packages/lint/src/object-graph.ts @@ -397,7 +397,14 @@ export interface VisibleObjectExtension { * (`packages[j].manifest`, `carrier` = `j`, the artifact's registration * order); without it, the stack's own (`carrier` 0). An artifact's top level * repeats its bodies' collections, so it is not read beside them. - * - `extend` names its target verbatim. + * - `extend` names its target verbatim. ⚠️ The registry does one more thing + * this fold does not mirror (#28858, `SchemaRegistry.extensionTargetKey`): + * a short name that nothing is registered under but that `getObject` folds + * to a legacy `__` key is filed under that key, with a warning. + * Lint reads the stack's own object names and knows no namespace, so such + * an entry folds onto nothing here: a key naming its field reads as + * unknown, the refusing direction, and the warning's remedy (write the key + * the object is registered under) is also what makes the fold see it. * - ONE entry per package per target: registering an extension removes that * package's earlier one, so a later entry of the same package replaces an * earlier one whole, its fields included.