Skip to content

Commit f974432

Browse files
committed
wip(lint): push-style cbp finding sites; table the merged-shape pass in the #5017 meta-tests
Co-authored-by: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01YRp5z4kK6vgYkrdH97ztsy
1 parent 5464db8 commit f974432

2 files changed

Lines changed: 83 additions & 46 deletions

File tree

‎packages/lint/src/validate-security-posture.test.ts‎

Lines changed: 36 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1480,6 +1480,12 @@ const NOT_SCHEMA_RECEIVERS: Record<string, string> = {
14801480
// scanned where they happen (in `object-graph.ts`, guarded by its own tests),
14811481
// not here; this rule never touches an authored field record directly.
14821482
surface: "a `GraphObject` — `object-graph.ts`'s resolved per-object surface, not an authored one.",
1483+
// [#23210] The merged-shape pass folds each `objectExtensions` entry into a
1484+
// `VisibleExtension` ({ carrier, path, priority, fields }) BEFORE it reads
1485+
// one; the authored reads behind it are scanned on `ext` and `f`.
1486+
visible: "a `VisibleExtension` — this file's own fold of one extension entry, not an authored surface.",
1487+
left: 'a `VisibleExtension`, the left operand of the fold-order comparator.',
1488+
right: 'a `VisibleExtension`, the right operand of the fold-order comparator.',
14831489
};
14841490

14851491
const READ_SURFACES: Array<{ receiver: string; expected: string[]; declaredBy: string; keys: () => string[] }> = [
@@ -1489,7 +1495,11 @@ const READ_SURFACES: Array<{ receiver: string; expected: string[]; declaredBy: s
14891495
// started passing the stack's own capability declarations to the predicate
14901496
// as `AnchorBindingContext.declaredCapabilities` — a declared token is the
14911497
// app's own gate, not a platform system permission.
1492-
expected: ['apps', 'books', 'capabilities', 'data', 'objects', 'permissions', 'positions'],
1498+
// [#23210] `objectExtensions` and `packages` joined when the
1499+
// controlled-by-parent rules started judging an object on its MERGED
1500+
// field set: the extensions a stack (or each package body of an artifact)
1501+
// declares are folded onto the objects it declares.
1502+
expected: ['apps', 'books', 'capabilities', 'data', 'objectExtensions', 'objects', 'packages', 'permissions', 'positions'],
14931503
declaredBy: 'ObjectStackSchema',
14941504
keys: () => Object.keys(ObjectStackSchema.shape),
14951505
},
@@ -1575,6 +1585,28 @@ const READ_SURFACES: Array<{ receiver: string; expected: string[]; declaredBy: s
15751585
declaredBy: 'ObjectStackSchema.data[]',
15761586
keys: () => shapeKeysOf(ObjectStackSchema.shape.data),
15771587
},
1588+
// [#23210] The merged-shape pass of the controlled-by-parent rules: an
1589+
// artifact entry (`packages[]`), its assembled body, and one
1590+
// `objectExtensions` entry — each read for the keys the registry's merge
1591+
// reads, and no other.
1592+
{
1593+
receiver: 'entry',
1594+
expected: ['manifest'],
1595+
declaredBy: 'ObjectStackSchema.packages[]',
1596+
keys: () => shapeKeysOf(ObjectStackSchema.shape.packages),
1597+
},
1598+
{
1599+
receiver: 'body',
1600+
expected: ['objectExtensions', 'objects'],
1601+
declaredBy: 'ObjectStackSchema.packages[].manifest',
1602+
keys: () => shapeKeysOf(shapeOf(ObjectStackSchema.shape.packages).manifest),
1603+
},
1604+
{
1605+
receiver: 'ext',
1606+
expected: ['extend', 'fields', 'priority'],
1607+
declaredBy: 'ObjectStackSchema.objectExtensions[]',
1608+
keys: () => shapeKeysOf(ObjectStackSchema.shape.objectExtensions),
1609+
},
15781610
// [#18535] The ADR-0066 D1 capability declarations the anchor rule reads: it
15791611
// wants their NAMES and hands the declarations themselves to the predicate,
15801612
// which reads `name` and ignores every other field — so nothing is
@@ -1629,6 +1661,9 @@ describe('validateSecurityPosture — reads only keys the spec declares (meta-te
16291661
'candidates', // #23100: `resolveCbpRelation`'s candidate list — `.length`, a JS property.
16301662
'cbpCandidates', // #23100: one object's required master_detail list — `.length` / `.map`, JS methods.
16311663
'declaredCapabilities', // #18535: the stack's own capability list — `.length`, a JS property.
1664+
// #23210: the merged-shape pass's own collections — JS Map / Set / Array methods.
1665+
'bodies', 'entries', 'targets', 'owners', 'extensions', 'byCarrier', 'writers', 'tiedWriters',
1666+
'merged', 'baseFields', 'blamed',
16321667
]);
16331668
expect(receivers.filter((r) => !tabled.has(r) && !PLUMBING.has(r))).toEqual([]);
16341669
});

‎packages/lint/src/validate-security-posture.ts‎

Lines changed: 47 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -407,11 +407,11 @@ function grantsObjectAccess(p: AnyRec): boolean {
407407

408408
/**
409409
* `security-controlled-by-parent-no-relation` for one object, at `path`. One
410-
* builder for the declared object and the merged one ({@link
411-
* extensionMergedCbpFindings}), so the two read in one wording.
410+
* site for the declared object and the merged one
411+
* ({@link pushExtensionMergedCbpFindings}), so the two read in one wording.
412412
*/
413-
function cbpNoRelationFinding(objName: string, path: string): SecurityFinding {
414-
return {
413+
function pushCbpNoRelation(findings: SecurityFinding[], objName: string, path: string): void {
414+
findings.push({
415415
severity: 'error',
416416
rule: SECURITY_CBP_NO_RELATION,
417417
where: `object "${objName}"`,
@@ -428,16 +428,21 @@ function cbpNoRelationFinding(objName: string, path: string): SecurityFinding {
428428
hint:
429429
`Add the master relation, e.g. fields.parent: { type: 'master_detail', reference: '<master_object>', ` +
430430
`required: true }, or set its own sharingModel: 'private', 'public_read' or 'public_read_write'`,
431-
};
431+
});
432432
}
433433

434434
/**
435435
* `security-controlled-by-parent-ambiguous-relation` for one object, at
436-
* `path`, naming its candidates. Shared like {@link cbpNoRelationFinding}.
436+
* `path`, naming its candidates. Shared like {@link pushCbpNoRelation}.
437437
*/
438-
function cbpAmbiguousRelationFinding(objName: string, path: string, candidates: CbpRelation[]): SecurityFinding {
438+
function pushCbpAmbiguousRelation(
439+
findings: SecurityFinding[],
440+
objName: string,
441+
path: string,
442+
candidates: CbpRelation[],
443+
): void {
439444
const roster = candidates.map((cand) => `"${cand.field}" → ${cand.master}`).join(', ');
440-
return {
445+
findings.push({
441446
severity: 'error',
442447
rule: SECURITY_CBP_AMBIGUOUS_RELATION,
443448
where: `object "${objName}"`,
@@ -451,10 +456,10 @@ function cbpAmbiguousRelationFinding(objName: string, path: string, candidates:
451456
hint:
452457
`Keep master_detail on the ONE master this object's rows belong to, and declare every other parent ` +
453458
`as a lookup field`,
454-
};
459+
});
455460
}
456461

457-
/** One `objectExtensions` entry as the registry registers it — see {@link extensionMergedCbpFindings}. */
462+
/** One `objectExtensions` entry as the registry registers it — see {@link pushExtensionMergedCbpFindings}. */
458463
interface VisibleExtension {
459464
/** The position of the package that ships it: registration order across packages. */
460465
carrier: number;
@@ -503,14 +508,13 @@ interface VisibleExtension {
503508
* a `master_detail` in the declaration or in the merged set — the line that
504509
* added the second master, or demoted the one.
505510
*/
506-
function extensionMergedCbpFindings(stack: AnyRec): SecurityFinding[] {
507-
const findings: SecurityFinding[] = [];
508-
const bodies = packagesOf(stack).map((entry) =>
511+
function pushExtensionMergedCbpFindings(findings: SecurityFinding[], stack: AnyRec): void {
512+
const multi = Array.isArray(stack.packages);
513+
const bodies: AnyRec[] = packagesOf(stack).map((entry) =>
509514
entry.manifest && typeof entry.manifest === 'object' && !Array.isArray(entry.manifest)
510515
? (entry.manifest as AnyRec)
511516
: {},
512517
);
513-
const multi = Array.isArray(stack.packages);
514518

515519
// Target name → its declaration, or `null` when two packages claim it.
516520
const targets = new Map<string, AnyRec | null>();
@@ -542,58 +546,56 @@ function extensionMergedCbpFindings(stack: AnyRec): SecurityFinding[] {
542546
entries.forEach((entry: unknown, i: number) => {
543547
if (!entry || typeof entry !== 'object' || Array.isArray(entry)) return;
544548
const ext = entry as AnyRec;
545-
const target = ext.extend;
546-
if (typeof target !== 'string' || !targets.get(target)) return;
547-
const byCarrier = extensions.get(target) ?? new Map<number, VisibleExtension>();
549+
if (typeof ext.extend !== 'string' || !targets.get(ext.extend)) return;
550+
const byCarrier = extensions.get(ext.extend) ?? new Map<number, VisibleExtension>();
548551
byCarrier.set(carrier, {
549552
carrier,
550553
path: `${at}objectExtensions[${i}]`,
551554
priority: typeof ext.priority === 'number' ? ext.priority : 200,
552555
fields: recordsOf(ext.fields),
553556
});
554-
extensions.set(target, byCarrier);
557+
extensions.set(ext.extend, byCarrier);
555558
});
556559
};
557560
if (!multi) collect(stack.objectExtensions, 0, '');
558561
else bodies.forEach((body, j) => collect(body.objectExtensions, j, `packages[${j}].manifest.`));
559562

560563
for (const [target, byCarrier] of extensions) {
561-
const base = targets.get(target);
562-
if (!base || owdOf(base) !== 'controlled_by_parent') continue;
563-
if (cbpMasterCandidates(base).length !== 1) continue;
564+
const obj = targets.get(target);
565+
if (!obj || owdOf(obj) !== 'controlled_by_parent') continue;
566+
if (cbpMasterCandidates(obj).length !== 1) continue;
564567

565-
const baseFields = new Map(recordsOf(base.fields).map((f) => [String(f.name ?? '?'), f] as const));
568+
const baseFields = new Map(recordsOf(obj.fields).map((f) => [String(f.name ?? '?'), f] as const));
566569
const merged = new Map(baseFields);
567-
const ordered = [...byCarrier.values()].sort((a, b) => a.priority - b.priority || a.carrier - b.carrier);
570+
const ordered = [...byCarrier.values()].sort(
571+
(left, right) => left.priority - right.priority || left.carrier - right.carrier,
572+
);
568573
const writers = new Map<string, VisibleExtension>();
574+
// Per key, how many packages wrote it at the priority of its last writer.
569575
const tiedWriters = new Map<string, number>();
570-
for (const ext of ordered) {
571-
for (const field of ext.fields) {
572-
const key = String(field.name ?? '?');
573-
const previous = writers.get(key);
574-
tiedWriters.set(key, previous?.priority === ext.priority ? (tiedWriters.get(key) ?? 1) + 1 : 1);
575-
writers.set(key, ext);
576-
merged.set(key, field);
576+
for (const visible of ordered) {
577+
for (const f of visible.fields) {
578+
const key = String(f.name ?? '?');
579+
const tied = writers.get(key)?.priority === visible.priority;
580+
tiedWriters.set(key, tied ? (tiedWriters.get(key) ?? 1) + 1 : 1);
581+
writers.set(key, visible);
582+
merged.set(key, f);
577583
}
578584
}
579585
if ([...tiedWriters.values()].some((count) => count > 1)) continue;
580586

581-
const candidates = cbpMasterCandidates({ ...base, fields: [...merged.values()] });
587+
const candidates = cbpMasterCandidates({ ...obj, fields: [...merged.values()] });
582588
if (candidates.length === 1) continue;
583589
const blamed = new Set<VisibleExtension>();
584-
for (const [key, ext] of writers) {
585-
if (merged.get(key)?.type === 'master_detail' || baseFields.get(key)?.type === 'master_detail') blamed.add(ext);
590+
for (const [key, visible] of writers) {
591+
if (merged.get(key)?.type === 'master_detail' || baseFields.get(key)?.type === 'master_detail') blamed.add(visible);
586592
}
587-
for (const ext of ordered) {
588-
if (!blamed.has(ext)) continue;
589-
findings.push(
590-
candidates.length === 0
591-
? cbpNoRelationFinding(target, `${ext.path}.fields`)
592-
: cbpAmbiguousRelationFinding(target, `${ext.path}.fields`, candidates),
593-
);
593+
for (const visible of ordered) {
594+
if (!blamed.has(visible)) continue;
595+
if (candidates.length === 0) pushCbpNoRelation(findings, target, `${visible.path}.fields`);
596+
else pushCbpAmbiguousRelation(findings, target, `${visible.path}.fields`, candidates);
594597
}
595598
}
596-
return findings;
597599
}
598600

599601
/**
@@ -702,7 +704,7 @@ export function validateSecurityPosture(stack: AnyRec, opts?: { nowMs?: number }
702704
// who may author it.
703705
const cbpCandidates = owd === 'controlled_by_parent' ? cbpMasterCandidates(obj) : undefined;
704706
if (cbpCandidates && cbpCandidates.length === 0) {
705-
findings.push(cbpNoRelationFinding(objName, `${objPath}.sharingModel`));
707+
pushCbpNoRelation(findings, objName, `${objPath}.sharingModel`);
706708
}
707709

708710
// ── [#14747 · #23100] controlled_by_parent with TWO OR MORE masters ───
@@ -721,7 +723,7 @@ export function validateSecurityPosture(stack: AnyRec, opts?: { nowMs?: number }
721723
// not: the ambiguity is a property of the document, independent of who may
722724
// author it.
723725
if (cbpCandidates && cbpCandidates.length > 1) {
724-
findings.push(cbpAmbiguousRelationFinding(objName, `${objPath}.fields`, cbpCandidates));
726+
pushCbpAmbiguousRelation(findings, objName, `${objPath}.fields`, cbpCandidates);
725727
}
726728

727729
// D11: external dial present on any object (system included) must obey
@@ -761,8 +763,8 @@ export function validateSecurityPosture(stack: AnyRec, opts?: { nowMs?: number }
761763
// ── [#23210] the same two rules, on an object's MERGED field set ─────
762764
// An `objectExtensions` entry of this stack merges into its target at boot,
763765
// so an object that passes above as declared can run with two required
764-
// master_detail fields, or none. See {@link extensionMergedCbpFindings}.
765-
findings.push(...extensionMergedCbpFindings(stack));
766+
// master_detail fields, or none. See {@link pushExtensionMergedCbpFindings}.
767+
pushExtensionMergedCbpFindings(findings, stack);
766768

767769
// [#16108] The object graph, for the FLS field-existence rule below. Built
768770
// ONCE here rather than per permission set: `indexObjectGraph` walks every

0 commit comments

Comments
 (0)