Skip to content

Commit fe8710f

Browse files
map: keep the request namespace through renamed destructuring (#128)
Found while splitting extract.ts. The request namespace decides an input's runtime coordinate, but it was recorded as a SET of local names and then compared against the literal strings 'query'/'params'. With a renamed destructuring the local is the alias, so the comparison failed and the namespace was silently discarded: ({ query }) => query.doc -> get.doc (correct) ({ query: q }) => q.doc -> post.doc (WRONG: never matches a query-string attack) ({ params: p }) => p.id -> post.id (WORSE: a coordinate for a route param) The second case is the dangerous one: route params are not exposed by the runtime resolver at all, which is exactly why runtimeCoordinate returns null for them — and aliasing bypassed that guard, handing a rule compiler an address the engine can never resolve. This is the same failure class as attributing a sink by name: a coordinate that looks plausible and quietly does nothing. `sourceNames` is now a Map from local name to the namespace it was bound from, so an alias resolves to its true source. An aliased request-body read (`const b = await req.json()`) also keeps its precise source (json-body / form-body) instead of collapsing to a generic body. Covered for all five shapes plus the candidate consequence: the aliased route param yields no coordinate and therefore no candidate, while the four addressable ones still compile. Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
1 parent 409f358 commit fe8710f

2 files changed

Lines changed: 93 additions & 7 deletions

File tree

src/map/inputs.ts

Lines changed: 20 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -122,14 +122,20 @@ function requestMemberAccesses(
122122
const out = new Map<string, InputSource>();
123123
const p0 = params?.[0];
124124
const reqName = p0 && ts.isIdentifier(p0.name) ? p0.name.text : undefined;
125-
// Identifiers that ARE a request-input object (destructured `({ body })` param, `await req.json()`).
126-
const sourceNames = new Set<string>();
125+
// Identifiers that ARE a request-input object (destructured `({ body })` param, `await req.json()`),
126+
// mapped to the NAMESPACE each one came from. It has to be a map, not a set of names: with
127+
// `({ query: q })` the local is `q`, and matching the local against the literal 'query'/'params'
128+
// discards the namespace — which silently mis-addresses the input (`post.doc` for a query-string
129+
// field, and worse, a coordinate for a route param, which the resolver cannot address at all).
130+
const sourceNames = new Map<string, InputSource>();
127131
const payloadNames = new Set<string>();
128132
if (opts.payloadParam && p0 && ts.isIdentifier(p0.name)) payloadNames.add(p0.name.text);
129133
if (p0 && !reqName && ts.isObjectBindingPattern(p0.name)) {
130134
for (const el of p0.name.elements) {
131135
const key = bindingKey(el, ts);
132-
if (key && REQ_SOURCES.includes(key) && ts.isIdentifier(el.name)) sourceNames.add(el.name.text);
136+
if (key && REQ_SOURCES.includes(key) && ts.isIdentifier(el.name)) {
137+
sourceNames.set(el.name.text, namespaceSource(key));
138+
}
133139
}
134140
}
135141
const unwrap = (e: any): any => {
@@ -156,7 +162,7 @@ function requestMemberAccesses(
156162
if (ts.isVariableDeclaration(n) && n.initializer) {
157163
const init = unwrap(n.initializer);
158164
// const b = await request.json() → b is a request-input object from here on.
159-
if (ts.isIdentifier(n.name) && isBodyReadCall(n.initializer)) sourceNames.add(n.name.text);
165+
if (ts.isIdentifier(n.name) && isBodyReadCall(n.initializer)) sourceNames.set(n.name.text, bodyReadSource(n.initializer));
160166
// const { a, b } = <source> | await request.json()
161167
if (ts.isObjectBindingPattern(n.name) && (isReqSourceExpr(init) || isBodyReadCall(n.initializer))) {
162168
const src = isBodyReadCall(n.initializer) ? bodyReadSource(n.initializer) : sourceOfExpr(init);
@@ -180,13 +186,20 @@ function requestMemberAccesses(
180186
if (e.name.text === 'params') return 'route-param';
181187
if (e.name.text === 'body') return 'body';
182188
}
189+
// The recorded namespace, so an ALIAS resolves correctly (`({ query: q }) => q.id` → query).
183190
if (ts.isIdentifier(e)) {
184-
const key = [...sourceNames].includes(e.text) ? e.text : undefined;
185-
if (key === 'query') return 'query';
186-
if (key === 'params') return 'route-param';
191+
const recorded = sourceNames.get(e.text);
192+
if (recorded) return recorded;
187193
}
188194
return 'body';
189195
}
196+
197+
/** Map a request namespace key to the input source it implies. */
198+
function namespaceSource(key: string): InputSource {
199+
if (key === 'query') return 'query';
200+
if (key === 'params') return 'route-param';
201+
return 'body';
202+
}
190203
function bodyReadSource(init: any): InputSource {
191204
const t = init?.getText?.() ?? '';
192205
return /formData\s*\(/.test(t) ? 'form-body' : 'json-body';
Lines changed: 73 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,73 @@
1+
import { describe, it, expect, beforeAll, afterAll } from 'vitest';
2+
import { mkdtempSync, mkdirSync, writeFileSync, rmSync } from 'node:fs';
3+
import { tmpdir } from 'node:os';
4+
import { join } from 'node:path';
5+
import { buildInputMap } from '../../src/map/index.js';
6+
7+
// The request NAMESPACE decides the runtime coordinate, so it has to survive renamed destructuring.
8+
// `({ query: q })` binds the local `q`; matching that local against the literal 'query' discarded the
9+
// namespace, which mis-addressed the input two ways:
10+
// - a query-string field got `post.doc` → a rule that can never match
11+
// - an aliased ROUTE PARAM got `post.id` → a coordinate for something the resolver cannot address,
12+
// defeating the whole point of returning null for route params.
13+
let dir: string;
14+
beforeAll(() => {
15+
dir = mkdtempSync(join(tmpdir(), 'ps-alias-'));
16+
mkdirSync(join(dir, 'src'), { recursive: true });
17+
writeFileSync(join(dir, 'package.json'), JSON.stringify({ dependencies: { express: '4' } }));
18+
writeFileSync(join(dir, 'src', 'app.ts'), `
19+
import express from "express";
20+
import fs from "node:fs";
21+
const app = express();
22+
app.get("/plain", ({ query }, res) => { res.end(fs.readFileSync(query.doc)); });
23+
app.get("/renamed", ({ query: q }, res) => { res.end(fs.readFileSync(q.doc)); });
24+
app.get("/param/:id", ({ params: p }, res) => { res.end(fs.readFileSync(p.id)); });
25+
app.post("/bodyalias", ({ body: b }, res) => { res.end(fs.readFileSync(b.file)); });
26+
app.post("/nested", ({ query: q }, res) => { const { doc } = q; res.end(fs.readFileSync(doc)); });
27+
`);
28+
});
29+
afterAll(() => rmSync(dir, { recursive: true, force: true }));
30+
31+
const input = async (route: string, name: string) => {
32+
const { map } = await buildInputMap(dir);
33+
const ep = map!.endpoints.find((e) => e.route === route)!;
34+
return { ep, field: ep.inputs.find((i) => i.name === name)! };
35+
};
36+
37+
describe('aliased request namespaces', () => {
38+
it('keeps the namespace when the handler param is destructured plainly (control)', async () => {
39+
const { field } = await input('/plain', 'doc');
40+
expect(field).toMatchObject({ source: 'query', runtimeParameter: 'get.doc' });
41+
});
42+
43+
it('keeps the namespace through a RENAMED destructuring', async () => {
44+
const { field } = await input('/renamed', 'doc');
45+
expect(field).toMatchObject({ source: 'query', runtimeParameter: 'get.doc' });
46+
});
47+
48+
it('still refuses a coordinate for an aliased route param', async () => {
49+
const { ep, field } = await input('/param/:id', 'id');
50+
expect(field.source).toBe('route-param');
51+
expect(field.runtimeParameter).toBeNull();
52+
// …and therefore cannot become a candidate, however strong the flow evidence is.
53+
expect(ep.flows.filter((f) => f.input === 'id' && f.ruleGeneratable)).toEqual([]);
54+
});
55+
56+
it('keeps an aliased body namespace', async () => {
57+
const { field } = await input('/bodyalias', 'file');
58+
expect(field).toMatchObject({ source: 'body', runtimeParameter: 'post.file' });
59+
});
60+
61+
it('keeps the namespace when destructuring again from the alias', async () => {
62+
const { field } = await input('/nested', 'doc');
63+
expect(field).toMatchObject({ source: 'query', runtimeParameter: 'get.doc' });
64+
});
65+
66+
it('compiles candidates for the addressable ones only', async () => {
67+
const { map } = await buildInputMap(dir);
68+
const got = map!.endpoints
69+
.flatMap((e) => e.flows.filter((f) => f.ruleGeneratable).map((f) => `${e.route}:${f.input}`))
70+
.sort();
71+
expect(got).toEqual(['/bodyalias:file', '/nested:doc', '/plain:doc', '/renamed:doc']);
72+
});
73+
});

0 commit comments

Comments
 (0)