Skip to content

Commit 15dfb55

Browse files
committed
fix(plugin-node-server): eliminate the dynamic property-write sink CodeQL flags
The blacklist guard added in the previous commit was not recognized as a sanitizer by CodeQL's js/remote-property-injection rule — the sink is the computed write itself (obj[userKey] = …). Build query and params via Object.fromEntries (own data properties only, no prototype-chain walk) so the sink no longer exists; the __proto__/constructor/prototype drop stays as defense in depth. Regression test unchanged and green (41/41). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01A1BtxNeUaGmvUYr8FwtUNu
1 parent 94eef6f commit 15dfb55

1 file changed

Lines changed: 18 additions & 11 deletions

File tree

packages/plugins/plugin-node-server/src/adapter.ts

Lines changed: 18 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -160,10 +160,12 @@ export class NodeHttpServer implements IHttpServer {
160160
if (route.method !== effective) continue;
161161
const m = route.regex.exec(normalized);
162162
if (!m) continue;
163-
// Null-prototype for symmetry with `query` — keys come from the
164-
// registered pattern (developer-controlled), values from the URL.
165-
const params: Record<string, string> = Object.create(null);
166-
route.keys.forEach((k, i) => { params[k] = decodeURIComponent(m[i + 1]); });
163+
// Own-data-property construction for symmetry with `query` — keys
164+
// come from the registered pattern (developer-controlled), values
165+
// from the URL.
166+
const params = Object.fromEntries(
167+
route.keys.map((k, i) => [k, decodeURIComponent(m[i + 1])]),
168+
) as Record<string, string>;
167169
return { route, params };
168170
}
169171
return undefined;
@@ -200,17 +202,22 @@ export class NodeHttpServer implements IHttpServer {
200202
}
201203

202204
// ── Query params (multi-value aware) ────────────────────────────────
203-
// Null-prototype object + dangerous-key filter: the property names
204-
// come straight from the request, so a plain `{}` would be open to
205-
// prototype pollution via `?__proto__=…` (CodeQL: remote property
206-
// injection).
207-
const query: Record<string, string | string[]> = Object.create(null);
205+
// The property names come straight from the request URL, so this map
206+
// must never be built via dynamic property writes (`obj[key] = …`) —
207+
// `?__proto__=…` would walk the prototype chain (CodeQL: remote
208+
// property injection). `Object.fromEntries` creates own data
209+
// properties only; the dangerous keys are dropped outright as
210+
// defense in depth.
211+
const seen = new Set<string>();
212+
const queryEntries: Array<[string, string | string[]]> = [];
208213
for (const key of url.searchParams.keys()) {
209214
if (key === '__proto__' || key === 'constructor' || key === 'prototype') continue;
210-
if (key in query) continue;
215+
if (seen.has(key)) continue;
216+
seen.add(key);
211217
const all = url.searchParams.getAll(key);
212-
query[key] = all.length > 1 ? all : all[0];
218+
queryEntries.push([key, all.length > 1 ? all : all[0]]);
213219
}
220+
const query = Object.fromEntries(queryEntries) as Record<string, string | string[]>;
214221

215222
// ── Body ────────────────────────────────────────────────────────────
216223
// JSON / urlencoded / text bodies are consumed eagerly (like the

0 commit comments

Comments
 (0)