Skip to content

Commit e5331aa

Browse files
claudeclaude
committed
fix(gate-manifest): reject shadowed bindings and value-changing provenance chains
Addresses both P2s from the re-review at 9f06c00. Block/catch/loop shadowing. The proof modelled only function parameters, so a `const entry` in a nested block — or a catch parameter, or a destructured binding — shadowed the ownership callback parameter invisibly, and resolution fell back to the ownership binding and exempted a dynamic rule. Rather than model every scoping construct, shadowsParameter refuses the exemption whenever the name is re-declared ANYWHERE inside the callback, by any construct, including destructuring patterns. That over-refuses in principle — a re-declaration in a sibling block cannot reach the call site — and fails closed, which is the right direction for a proof whose job is to be conservative. Provenance. iteratesOwnershipTable accepted any chain rooted at the table, so `BUILD_OWNERSHIP.map(transform).map((entry) => entry.rule)` qualified even though `transform` can return anything and the downstream elements are no longer table entries. yieldsOwnershipEntries now walks the chain and requires every intervening method to be value-preserving (filter/slice/reverse/sort/toSorted/ toReversed). `.map(transform)` mid-chain breaks the proof; the real `BUILD_OWNERSHIP.filter(...).map(cb)` still holds. Tests 46 → 50: nested-block re-declaration, catch parameter and destructured re-declaration, a value-changing chain, and a value-preserving `.filter().slice()` chain proving the legitimate case still resolves and loses no category. rootIdentifier is gone — yieldsOwnershipEntries replaced it. Co-Authored-By: Claude Opus 5 <[email protected]> Claude-Session: https://claude.ai/code/session_01SkS4S8XXrfkJ8TD1VBKkvJ
1 parent 9f06c00 commit e5331aa

2 files changed

Lines changed: 150 additions & 21 deletions

File tree

‎scripts/gate-manifest/selector-rules.test.ts‎

Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -114,6 +114,69 @@ test('the real ownership loop stays exempt when no impostor shares its name', ()
114114
assert.deepEqual(selectorRuleIds('m.ts', source), ['gate:lint', 'own:swift']);
115115
});
116116

117+
test('a block-scoped re-declaration inside the ownership callback fails closed', () => {
118+
// `const entry = config` shadows the callback parameter without being a parameter itself,
119+
// so function-level resolution alone would fall back to the ownership binding and exempt a
120+
// rule that is not a collected literal.
121+
const source = `
122+
const BUILD_OWNERSHIP = [{ check: 'swift-runner', rule: 'own:swift', detail: 'd', owns: () => true }];
123+
const bad = BUILD_OWNERSHIP.map((entry) => {
124+
{
125+
const entry = config;
126+
return reason('lint', file, entry.rule, 'd');
127+
}
128+
});
129+
`;
130+
assert.throws(() => selectorRuleIds('m.ts', source), /rule argument is not a string literal/);
131+
});
132+
133+
test('a catch or destructured re-declaration is caught the same way', () => {
134+
const caught = `
135+
const BUILD_OWNERSHIP = [{ check: 'swift-runner', rule: 'own:swift', detail: 'd', owns: () => true }];
136+
const bad = BUILD_OWNERSHIP.map((entry) => {
137+
try { go(); } catch (entry) { return reason('lint', file, entry.rule, 'd'); }
138+
});
139+
`;
140+
assert.throws(() => selectorRuleIds('m.ts', caught), /rule argument is not a string literal/);
141+
142+
const destructured = `
143+
const BUILD_OWNERSHIP = [{ check: 'swift-runner', rule: 'own:swift', detail: 'd', owns: () => true }];
144+
const bad = BUILD_OWNERSHIP.map((entry) => {
145+
{
146+
const { entry } = config;
147+
return reason('lint', file, entry.rule, 'd');
148+
}
149+
});
150+
`;
151+
assert.throws(
152+
() => selectorRuleIds('m.ts', destructured),
153+
/rule argument is not a string literal/,
154+
);
155+
});
156+
157+
test('a value-changing chain is not proven provenance, even rooted at the table', () => {
158+
// `.map(transform)` can return anything, so the downstream elements are no longer ownership
159+
// entries and their `.rule` is not a literal this reader collected.
160+
const source = `
161+
const BUILD_OWNERSHIP = [{ check: 'swift-runner', rule: 'own:swift', detail: 'd', owns: () => true }];
162+
const bad = BUILD_OWNERSHIP.map(transform).map((entry) => reason('lint', file, entry.rule, 'd'));
163+
`;
164+
assert.throws(() => selectorRuleIds('m.ts', source), /rule argument is not a string literal/);
165+
});
166+
167+
test('a value-preserving chain still proves provenance', () => {
168+
const source = `
169+
const BUILD_OWNERSHIP = [
170+
{ check: 'swift-runner', rule: 'own:swift', detail: 'd', owns: () => true },
171+
];
172+
const good = BUILD_OWNERSHIP.filter((e) => e.owns(file)).slice(0).map((entry) =>
173+
reason(entry.check, file, entry.rule, entry.detail),
174+
);
175+
const a = reason('lint', file, 'gate:lint', 'd');
176+
`;
177+
assert.deepEqual(selectorRuleIds('m.ts', source), ['gate:lint', 'own:swift']);
178+
});
179+
117180
test('a computed rule argument fails closed rather than being skipped', () => {
118181
// A category this reader cannot see would bypass the representative-sample and reachability
119182
// checks entirely — the exact hole the derived universe exists to close.

‎scripts/gate-manifest/selector-rules.ts‎

Lines changed: 87 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -59,19 +59,6 @@ function isMemberAccess(node: Node): boolean {
5959
return node.type === 'MemberExpression' || node.type === 'StaticMemberExpression';
6060
}
6161

62-
/** Walks `a.b().c` down to its root identifier, or null when the root is not one. */
63-
function rootIdentifier(node: unknown): string | null {
64-
let current: unknown = node;
65-
for (let depth = 0; isNode(current) && depth < 32; depth++) {
66-
const name = identifierName(current);
67-
if (name !== null) return name;
68-
if (isMemberAccess(current)) current = current['object'];
69-
else if (current.type === 'CallExpression') current = current['callee'];
70-
else return null;
71-
}
72-
return null;
73-
}
74-
7562
/**
7663
* The body of the `reason(check, file, rule, detail)` factory itself.
7764
*
@@ -115,21 +102,50 @@ function ownershipTableNames(program: Node, factory: Span | null): Set<string> {
115102
}
116103

117104
/**
118-
* Whether a call iterates a collected ownership table — `BUILD_OWNERSHIP.filter(...)`, and the
119-
* `.map(...)` chained onto it.
105+
* Array methods that yield the SAME elements. An ownership entry stays an ownership entry
106+
* through `.filter(...)`; it does not through `.map(transform)`, whose callback can return
107+
* anything, so the elements downstream are no longer table entries and their `.rule` is no
108+
* longer a literal this reader has collected.
109+
*/
110+
const VALUE_PRESERVING_METHODS = new Set([
111+
'filter',
112+
'slice',
113+
'reverse',
114+
'sort',
115+
'toSorted',
116+
'toReversed',
117+
]);
118+
119+
/** Whether an expression evaluates to elements of a collected ownership table, unchanged. */
120+
function yieldsOwnershipEntries(node: unknown, tables: ReadonlySet<string>): boolean {
121+
let current: unknown = node;
122+
for (let depth = 0; isNode(current) && depth < 32; depth++) {
123+
const name = identifierName(current);
124+
if (name !== null) return tables.has(name);
125+
if (current.type !== 'CallExpression') return false;
126+
const callee = current['callee'];
127+
if (!isNode(callee) || !isMemberAccess(callee)) return false;
128+
const method = identifierName(callee['property']);
129+
if (method === null || !VALUE_PRESERVING_METHODS.has(method)) return false;
130+
current = callee['object'];
131+
}
132+
return false;
133+
}
134+
135+
/**
136+
* Whether a call iterates a collected ownership table — `BUILD_OWNERSHIP.filter(...).map(cb)`.
120137
*
121138
* This is what makes the forwarding exemption a proof rather than a pattern. Accepting any
122139
* member access named `.rule` would silently skip `reason(check, file, config.rule, …)`, whose
123140
* literal this reader never sees — a live category slipping past the derived universe while
124-
* the gate stays green. Only a binding whose origin is an ownership-table iteration qualifies,
125-
* because only then is the literal guaranteed to have been collected from the table already.
141+
* the gate stays green. Only a binding whose values are provably unchanged table entries
142+
* qualifies, because only then is the literal guaranteed to have been collected already.
126143
*/
127144
function iteratesOwnershipTable(node: Node, tables: ReadonlySet<string>): boolean {
128145
if (node.type !== 'CallExpression') return false;
129146
const callee = node['callee'];
130147
if (!isNode(callee) || !isMemberAccess(callee)) return false;
131-
const root = rootIdentifier(callee['object']);
132-
return root !== null && tables.has(root);
148+
return yieldsOwnershipEntries(callee['object'], tables);
133149
}
134150

135151
function isFunctionLike(node: Node): boolean {
@@ -147,6 +163,54 @@ function callbackFunctions(node: Node): Node[] {
147163
return args.filter((argument): argument is Node => isNode(argument) && isFunctionLike(argument));
148164
}
149165

166+
/** Every identifier a binding pattern introduces, including destructured ones. */
167+
function patternNames(node: unknown, into: Set<string>): void {
168+
if (!isNode(node)) return;
169+
const name = identifierName(node);
170+
if (name !== null) {
171+
into.add(name);
172+
return;
173+
}
174+
for (const child of children(node)) patternNames(child, into);
175+
}
176+
177+
/** Whether a node introduces `name` as a new binding of any kind. */
178+
function declaresName(node: Node, name: string): boolean {
179+
const names = new Set<string>();
180+
if (node.type === 'VariableDeclarator') patternNames(node['id'], names);
181+
else if (node.type === 'CatchClause') patternNames(node['param'], names);
182+
else if (isFunctionLike(node)) {
183+
patternNames(node['id'], names);
184+
const params = node['params'];
185+
if (Array.isArray(params)) for (const parameter of params) patternNames(parameter, names);
186+
}
187+
return names.has(name);
188+
}
189+
190+
/**
191+
* Any re-declaration of `name` inside `callback` other than the callback's own parameter.
192+
*
193+
* Resolving to the innermost FUNCTION binder is not enough on its own: a block, catch or loop
194+
* binding shadows the ownership parameter without being a function parameter at all, and the
195+
* resolver would otherwise fall back to the ownership binding and exempt a dynamic rule —
196+
*
197+
* BUILD_OWNERSHIP.map((entry) => { const entry = config; reason(…, entry.rule, …); })
198+
*
199+
* Rather than model every scoping construct, this refuses the exemption whenever the name is
200+
* re-declared anywhere inside the callback. That over-refuses in principle (a re-declaration in
201+
* a sibling block cannot reach the call site) and fails closed, which is the right direction.
202+
*/
203+
function shadowsParameter(program: Node, callback: Node, name: string): boolean {
204+
const span = spanOf(callback);
205+
if (span === null) return true;
206+
let shadowed = false;
207+
collect(program, (node) => {
208+
if (shadowed || node === callback) return;
209+
if (within(span, node) && declaresName(node, name)) shadowed = true;
210+
});
211+
return shadowed;
212+
}
213+
150214
/**
151215
* A name bound by a function's parameter list, with the source span it is visible in.
152216
*
@@ -170,10 +234,12 @@ function parameterBindings(program: Node, tables: ReadonlySet<string>): Paramete
170234
const span = isFunctionLike(node) ? spanOf(node) : null;
171235
const params = node['params'];
172236
if (span === null || !Array.isArray(params)) return;
173-
const iteratesOwnership = ownershipCallbacks.has(node);
237+
const owns = ownershipCallbacks.has(node);
174238
for (const parameter of params) {
175239
const name = identifierName(parameter);
176-
if (name !== null) bindings.push({ name, span, iteratesOwnership });
240+
if (name === null) continue;
241+
const iteratesOwnership = owns && !shadowsParameter(program, node, name);
242+
bindings.push({ name, span, iteratesOwnership });
177243
}
178244
});
179245
return bindings;

0 commit comments

Comments
 (0)