Skip to content

Commit 8766d54

Browse files
committed
fix(run-store): guard carryUnknownKeys against prototype keys and scan imports on raw source
Use an own-property check and skip __proto__/constructor/prototype so an inherited-name field surfaces as a divergence and no key can pollute the prototype. Scan import statements on the raw source (line-anchored) so comment stripping cannot hide a real import.
1 parent 26b6e9a commit 8766d54

3 files changed

Lines changed: 37 additions & 9 deletions

File tree

internal-packages/run-store/src/snapshotComparator.isolation.test.ts

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -13,17 +13,17 @@ const here = dirname(fileURLToPath(import.meta.url));
1313
// Returns the module specifiers a file imports FOR VALUE (i.e. that survive to runtime). `import type`
1414
// declarations and named blocks whose specifiers are all inline `type` are erased and excluded.
1515
function valueImports(sourcePath: string): string[] {
16-
// Strip block AND line comments so a comment mentioning `import(` or `import ... from` cannot
17-
// produce a false positive.
18-
const src = readFileSync(sourcePath, "utf8")
19-
.replace(/\/\*[\s\S]*?\*\//g, "")
20-
.replace(/\/\/.*$/gm, "");
16+
const raw = readFileSync(sourcePath, "utf8");
2117
const out: string[] = [];
2218

23-
if (/(^|[^.\w])import\s*\(/.test(src)) out.push("<dynamic import()>");
19+
// Statements are scanned on RAW source, anchored to line start (`^\s*import`), so a `//` comment
20+
// line never matches and no stripping can hide a real import. Only the mid-line dynamic `import(`
21+
// check runs on comment-stripped source. The pin test below guarantees the scan catches a real import.
22+
const stripped = raw.replace(/\/\*[\s\S]*?\*\//g, "").replace(/\/\/.*$/gm, "");
23+
if (/(^|[^.\w])import\s*\(/.test(stripped)) out.push("<dynamic import()>");
2424

2525
const importRe = /^\s*import\b([\s\S]*?)\bfrom\s*["']([^"']+)["']/gm;
26-
for (let m = importRe.exec(src); m !== null; m = importRe.exec(src)) {
26+
for (let m = importRe.exec(raw); m !== null; m = importRe.exec(raw)) {
2727
const clause = m[1];
2828
const spec = m[2];
2929
if (/^\s*type\b/.test(clause)) continue; // `import type ... from`
@@ -36,7 +36,7 @@ function valueImports(sourcePath: string): string[] {
3636

3737
// Bare side-effect imports (`import "x"`) run the module.
3838
const bareRe = /^\s*import\s*["']([^"']+)["']/gm;
39-
for (let m = bareRe.exec(src); m !== null; m = bareRe.exec(src)) out.push(m[1]);
39+
for (let m = bareRe.exec(raw); m !== null; m = bareRe.exec(raw)) out.push(m[1]);
4040

4141
return out;
4242
}

internal-packages/run-store/src/snapshotComparator.test.ts

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -131,6 +131,29 @@ describe("diffLatest", () => {
131131
expect.objectContaining({ field: "mysteryField", class: "unknownField", redis: "surprise" }),
132132
]);
133133
});
134+
135+
it("surfaces an inherited-name key and does not pollute the prototype", () => {
136+
// JSON.parse produces OWN keys for `toString` and `__proto__` (unlike an object literal).
137+
const entry = JSON.parse(
138+
'{"engine":"V2","executionStatus":"RUN_CREATED","description":"d","runId":"r1",' +
139+
'"runStatus":"PENDING","createdAt":"2026-08-24T00:00:00.000Z","environmentId":"env",' +
140+
'"environmentType":"DEVELOPMENT","projectId":"p","organizationId":"o",' +
141+
'"toString":"surprise","__proto__":{"polluted":true}}'
142+
) as Record<string, unknown>;
143+
const read: SnapshotRead = { id: "s1", seq: 1, isValid: true, raw: "{}", entry };
144+
const n = normalizeFromRedis(read) as Record<string, unknown>;
145+
146+
expect(Object.prototype.hasOwnProperty.call(n, "toString")).toBe(true); // carried despite inherited name
147+
expect(n["toString"]).toBe("surprise");
148+
expect(Object.getPrototypeOf(n)).toBe(Object.prototype); // __proto__ skipped, no pollution
149+
expect("polluted" in {}).toBe(false);
150+
151+
const d = diffLatest(
152+
norm({ id: "s1", createdAt: n.createdAt as number, updatedAt: n.updatedAt as number }),
153+
n as NormalizedSnapshot
154+
);
155+
expect(d.some((x) => x.field === "toString" && x.class === "unknownField")).toBe(true);
156+
});
134157
});
135158

136159
describe("diffSince", () => {

internal-packages/run-store/src/snapshotComparator.ts

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -80,9 +80,14 @@ const KNOWN_KEYS = new Set<string>([
8080

8181
// Carry a source key normalization does not recognise onto the normalized object, so the
8282
// unknownField check sees it instead of it being silently dropped (a false clean comparison).
83+
// Uses an own-property check (not `in`, which sees the prototype chain and would hide keys like
84+
// `constructor`), and skips prototype-pollution keys.
85+
const DANGEROUS_KEYS = new Set(["__proto__", "constructor", "prototype"]);
86+
const hasOwn = (o: object, k: string): boolean => Object.prototype.hasOwnProperty.call(o, k);
8387
function carryUnknownKeys(target: NormalizedSnapshot, source: Record<string, unknown>): void {
8488
for (const k of Object.keys(source)) {
85-
if (!KNOWN_KEYS.has(k) && !(k in target)) target[k] = source[k];
89+
if (DANGEROUS_KEYS.has(k)) continue;
90+
if (!KNOWN_KEYS.has(k) && !hasOwn(target, k)) target[k] = source[k];
8691
}
8792
}
8893

0 commit comments

Comments
 (0)