From 84437d5f950a6367e7c186d657449d67acc167d7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Bombeck?= Date: Thu, 3 Sep 2026 01:42:16 +0200 Subject: [PATCH] test: read every spelling of a dependency override pin MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The lockstep guard compared only the pins both matchers happened to recognise, and neither matcher said so when it recognised fewer. Workspace side: the key had to be quoted. Every entry in the file carries an `@` suffix and therefore needs quoting, so the matcher looked complete — but `mysql2: "^3.22.0"` is a valid pnpm override with no selector and no quotes, and it parsed to nothing. Unquoting that key and pointing the Dockerfile at `^1.0.0` left all six tests green. Dockerfile side: only `npm pkg set 'overrides.X=Y'` was recognised, so re-quoting the same line as `npm pkg set overrides.X=Y` removed the pin from the comparison without changing what the image installs. Both sides now parse line by line: quoted or bare keys and values on the workspace side, any shell quoting of the same assignment on the Dockerfile side, plus a raw JSON `"overrides"` block. What neither table can read is collected and asserted empty, so a future spelling fails loudly instead of shrinking the compared set in silence. --- ...dependency-override-lockstep-guard.test.ts | 313 +++++++++++++++--- 1 file changed, 269 insertions(+), 44 deletions(-) diff --git a/src/__tests__/dependency-override-lockstep-guard.test.ts b/src/__tests__/dependency-override-lockstep-guard.test.ts index 029e10fd6..9d5133b2c 100644 --- a/src/__tests__/dependency-override-lockstep-guard.test.ts +++ b/src/__tests__/dependency-override-lockstep-guard.test.ts @@ -20,16 +20,31 @@ * would encode noise. It asserts agreement where both files speak, which is * the failure that actually happened. * - * The matcher had a blind spot of its own until 2026-09-02: it refused any - * workspace key beginning with `@`, so every scoped package was invisible to - * it. `@hono/node-server` was pinned on one side and unpinned on the other and - * the guard stayed green, which is the same shape of green the guard exists to - * prevent. Scoped keys parse now, and a test below pins that they do. + * The matcher has now been widened twice, and both widenings were the same + * class of hole: a spelling the files accept and the regex did not. * - * Limit, written down so the next reader does not over-trust a green run: this - * compares declared ranges. It cannot see a NEW vulnerable transitive that - * neither file mentions. Only the image scan can, and that job runs on - * main-push, not on pull requests. + * - 2026-09-02: the workspace side refused any key beginning with `@`, so + * every scoped package was invisible. `@hono/node-server` was pinned on + * one side and unpinned on the other and the guard stayed green. + * - 2026-09-03: the workspace side demanded a QUOTED key, and the Dockerfile + * side demanded the exact spelling `npm pkg set 'overrides.X=Y'`. Every + * entry in the file happens to carry an `@` suffix, which needs + * quoting, so the matcher looked complete — but `mysql2: "^3.22.0"` is a + * legal pnpm override with no selector and no quotes, and it parsed to + * nothing. Unquoting that key and pointing the Dockerfile at `^1.0.0` + * left all six tests green. + * + * Both sides now parse line by line and collect what they could NOT read into + * an `unparsed` list, which is asserted empty. A future spelling neither + * matcher knows fails the guard loudly instead of shrinking the compared set + * in silence, which is the only structural defence against a third repeat. + * + * Limits, written down so the next reader does not over-trust a green run: + * this compares DECLARED ranges. It cannot see a NEW vulnerable transitive + * that neither file mentions — only the image scan can, and that job runs on + * main-push, not on pull requests. It also reads the Dockerfile as text, so a + * pin injected by a build ARG, an `npm config` call, or a package.json copied + * in from elsewhere is outside both matchers. */ import { readFileSync } from "node:fs"; import { join } from "node:path"; @@ -40,21 +55,19 @@ const ROOT = process.cwd(); const DOCKERFILE = readFileSync(join(ROOT, "Dockerfile"), "utf8"); const WORKSPACE = readFileSync(join(ROOT, "pnpm-workspace.yaml"), "utf8"); -/** `npm pkg set 'overrides.='` occurrences in the Dockerfile. */ -function dockerfileOverrides(): Map { - const found = new Map(); - const pattern = /npm\s+pkg\s+set\s+['"]overrides\.([^='"]+)=([^'"]+)['"]/g; - for (const match of DOCKERFILE.matchAll(pattern)) { - found.set(match[1].trim(), match[2].trim()); - } - return found; -} +/** What a side of the comparison parsed, and what it had to give up on. */ +type ParsedOverrides = { + /** Bare package name → declared range. */ + pins: Map; + /** Lines that name an override but no matcher could read. */ + unparsed: string[]; +}; /** - * Reduces an overrides key to the bare package name. The key carries a version - * selector, so `deepmerge-ts@<8.0.0` and a bare `deepmerge-ts` both land on the - * same name. A scoped name opens with its own `@`, so the selector starts at - * the SECOND one, not the first. + * Reduces an overrides key to the bare package name. The key MAY carry a + * version selector, so `deepmerge-ts@<8.0.0` and a bare `deepmerge-ts` both + * land on the same name. A scoped name opens with its own `@`, so the selector + * starts at the SECOND one, not the first. */ function packageNameOf(key: string): string { const trimmed = key.trim(); @@ -64,33 +77,172 @@ function packageNameOf(key: string): string { return (selectorAt === -1 ? trimmed : trimmed.slice(0, selectorAt)).trim(); } -/** - * `"@": ""` entries under the workspace `overrides:` - * block. - */ -function workspaceOverrides(): Map { - const found = new Map(); - const pattern = /^\s+"([^"]+)"\s*:\s*"([^"]+)"/gm; - const block = WORKSPACE.split(/^allowBuilds:/m)[0]; - for (const match of block.matchAll(pattern)) { - found.set(packageNameOf(match[1]), match[2].trim()); - } - return found; +/** Strips one layer of matching YAML/shell quotes, if present. */ +function unquote(raw: string): string { + const trimmed = raw.trim(); + const quoted = /^"([^"]*)"$/.exec(trimmed) ?? /^'([^']*)'$/.exec(trimmed); + return quoted ? quoted[1] : trimmed; } +/** + * Splits one `: ` mapping line. A QUOTED key may itself contain a + * colon, so the separator is looked for after the closing quote; an unquoted + * key cannot contain one, so the first colon is the separator. + */ +function splitMapping(line: string): [string, string] | null { + const body = line.trim(); + const quote = body.startsWith('"') ? '"' : body.startsWith("'") ? "'" : null; + if (quote) { + const close = body.indexOf(quote, 1); + if (close === -1) return null; + const rest = body.slice(close + 1).trimStart(); + if (!rest.startsWith(":")) return null; + return [body.slice(0, close + 1), rest.slice(1)]; + } + const colon = body.indexOf(":"); + if (colon === -1) return null; + return [body.slice(0, colon), body.slice(colon + 1)]; +} + +/** Reads a YAML scalar value, dropping any trailing `# comment`. */ +function scalarValue(raw: string): string { + const trimmed = raw.trim(); + const quoted = /^"([^"]*)"/.exec(trimmed) ?? /^'([^']*)'/.exec(trimmed); + if (quoted) return quoted[1]; + return trimmed.split(/\s+#/)[0].trim(); +} + +/** + * Entries under the workspace `overrides:` block. + * + * pnpm accepts the key with or without a version selector and YAML accepts + * either half quoted or bare, so all four spellings below are the same pin and + * all four must parse: + * + * "mysql2@<3.22.0": "^3.22.0" + * mysql2@<3.22.0: ^3.22.0 + * "mysql2": "^3.22.0" + * mysql2: ^3.22.0 + */ +export function parseWorkspaceOverrides(yaml: string): ParsedOverrides { + const pins = new Map(); + const unparsed: string[] = []; + const lines = yaml.split(/\r?\n/); + const start = lines.findIndex((line) => /^overrides:\s*$/.test(line)); + if (start === -1) { + return { pins, unparsed: ["no `overrides:` block found"] }; + } + + for (const line of lines.slice(start + 1)) { + if (line.trim() === "") continue; + // A line at column zero is the next top-level key; the block ends there. + if (!/^\s/.test(line)) break; + if (/^\s*#/.test(line)) continue; + + const mapping = splitMapping(line); + const range = mapping ? scalarValue(mapping[1]) : ""; + if (!mapping || range === "") { + unparsed.push(line.trim()); + continue; + } + pins.set(packageNameOf(unquote(mapping[0])), range); + } + return { pins, unparsed }; +} + +/** + * `overrides` entries written into `/opt/prisma-cli/package.json`. + * + * `npm pkg set` is indifferent to shell quoting, so the assignment may be + * quoted whole, quoted per half, or bare. Any non-comment Dockerfile line that + * names `overrides` and matches none of the accepted spellings is reported as + * unparsed rather than skipped — that is what stops a fresh spelling from + * quietly shrinking the compared set. + */ +export function parseDockerfileOverrides(dockerfile: string): ParsedOverrides { + const pins = new Map(); + const unparsed: string[] = []; + + // `npm pkg set 'overrides.='`, `"overrides.="`, + // `overrides.=`, and the per-half-quoted variants. A package + // name and a semver range never contain a quote character, so quoting is + // pure shell decoration here: strip it and every spelling collapses onto the + // same `overrides.=` token. + const npmPkgSet = /npm\s+pkg\s+set\s+overrides\.([^\s=]+)\s*=\s*([^\s&;|]+)/; + // A raw JSON field, e.g. inside a `cat > package.json` heredoc: + // `"": ""` on a line under an `"overrides"` key. + const jsonField = /"([^"]+)"\s*:\s*"([^"]+)"/; + + let insideJsonOverrides = false; + for (const rawLine of dockerfile.split(/\r?\n/)) { + const line = rawLine.trim(); + if (line.startsWith("#")) continue; + + if (insideJsonOverrides) { + if (line.startsWith("}")) { + insideJsonOverrides = false; + continue; + } + const field = jsonField.exec(line); + if (field) { + pins.set(packageNameOf(field[1]), field[2].trim()); + continue; + } + unparsed.push(line); + continue; + } + + if (!/\boverrides\b/.test(line)) continue; + + if (/"overrides"\s*:\s*\{\s*$/.test(line)) { + insideJsonOverrides = true; + continue; + } + + const set = npmPkgSet.exec(line.replace(/['"]/g, "")); + if (set) { + pins.set(packageNameOf(set[1]), set[2].trim()); + continue; + } + unparsed.push(line); + } + return { pins, unparsed }; +} + +const dockerfileOverrides = () => parseDockerfileOverrides(DOCKERFILE); +const workspaceOverrides = () => parseWorkspaceOverrides(WORKSPACE); + describe("dependency overrides — pnpm workspace and the npm install agree", () => { it("finds the Dockerfile's npm-side overrides at all", () => { const docker = dockerfileOverrides(); // A zero-match regex would make every assertion below vacuously true. // This is the check that keeps the guard honest about its own matcher. - expect(docker.size).toBeGreaterThan(0); - expect(docker.has("deepmerge-ts")).toBe(true); + expect(docker.pins.size).toBeGreaterThan(0); + expect(docker.pins.has("deepmerge-ts")).toBe(true); }); it("finds the workspace overrides at all", () => { const workspace = workspaceOverrides(); - expect(workspace.size).toBeGreaterThan(0); - expect(workspace.has("deepmerge-ts")).toBe(true); + expect(workspace.pins.size).toBeGreaterThan(0); + expect(workspace.pins.has("deepmerge-ts")).toBe(true); + }); + + it("refuses an overrides line it could not read on either side", () => { + // The failure this replaces: a matcher that silently narrows the compared + // set is indistinguishable from a matcher that found agreement. Anything + // neither spelling table covers has to surface here. + expect( + workspaceOverrides().unparsed, + "pnpm-workspace.yaml has an `overrides:` entry this guard cannot read. " + + "Teach `parseWorkspaceOverrides` the spelling — do not leave it " + + "unparsed, or the pin stops being compared against the Dockerfile.", + ).toEqual([]); + expect( + dockerfileOverrides().unparsed, + "The Dockerfile names `overrides` on a line this guard cannot read. " + + "Teach `parseDockerfileOverrides` the spelling — an unread pin is an " + + "uncompared pin.", + ).toEqual([]); }); it("reads scoped workspace keys rather than skipping them", () => { @@ -105,14 +257,87 @@ describe("dependency overrides — pnpm workspace and the npm install agree", () expect(packageNameOf("js-yaml@>=4.0.0 <4.3.1")).toBe("js-yaml"); expect(packageNameOf("deepmerge-ts")).toBe("deepmerge-ts"); - const workspace = workspaceOverrides(); - const scoped = [...workspace.keys()].filter((name) => name.startsWith("@")); + const scoped = [...workspaceOverrides().pins.keys()].filter((name) => + name.startsWith("@"), + ); expect(scoped.length).toBeGreaterThan(0); }); + it("reads a workspace pin that carries no selector and no quotes", () => { + // `mysql2: "^3.22.0"` is a valid pnpm override and valid YAML. The old + // matcher required a quoted key, so it parsed to nothing — and because + // every entry in the file happens to need quoting for its selector, the + // hole was invisible by inspection. All four spellings are one pin. + const parsed = parseWorkspaceOverrides( + [ + "overrides:", + " # a comment inside the block", + ' "quoted-selector@<2.0.0": "^2.0.0"', + " bare-selector@<2.0.0: ^2.0.0", + ' "quoted-plain": "^2.0.0"', + " bare-plain: ^2.0.0", + ' "@scoped/bare-value@<2.0.0": ^2.0.0', + " trailing-comment: ^2.0.0 # note", + "", + "allowBuilds:", + " prisma: true", + ].join("\n"), + ); + + expect(parsed.unparsed).toEqual([]); + expect(Object.fromEntries(parsed.pins)).toEqual({ + "quoted-selector": "^2.0.0", + "bare-selector": "^2.0.0", + "quoted-plain": "^2.0.0", + "bare-plain": "^2.0.0", + "@scoped/bare-value": "^2.0.0", + "trailing-comment": "^2.0.0", + }); + // The block ends at the next top-level key: `allowBuilds` is not a pin. + expect(parsed.pins.has("prisma")).toBe(false); + }); + + it("reads every spelling of the Dockerfile's npm-side pin", () => { + // The old matcher accepted `npm pkg set 'overrides.X=Y'` and nothing else, + // so re-quoting the same line removed the pin from the comparison without + // changing what the image installs. + const parsed = parseDockerfileOverrides( + [ + "# a comment mentioning the `overrides` block", + "RUN npm init -y && \\", + " npm pkg set 'overrides.single-quoted=^1.0.0' && \\", + ' npm pkg set "overrides.double-quoted=^1.0.0" && \\', + " npm pkg set overrides.unquoted=^1.0.0 && \\", + " npm pkg set 'overrides.@scoped/split'='^1.0.0' && \\", + " npm install --omit=dev prisma@7.8.0", + ].join("\n"), + ); + + expect(parsed.unparsed).toEqual([]); + expect(Object.fromEntries(parsed.pins)).toEqual({ + "single-quoted": "^1.0.0", + "double-quoted": "^1.0.0", + unquoted: "^1.0.0", + "@scoped/split": "^1.0.0", + }); + }); + + it("reports an overrides spelling neither matcher knows", () => { + // Written so the refusal itself is proven, not assumed. A guard that + // cannot fail is the failure mode this file exists to close. + expect( + parseWorkspaceOverrides(["overrides:", " - not-a-mapping"].join("\n")) + .unparsed, + ).toEqual(["- not-a-mapping"]); + expect( + parseDockerfileOverrides("RUN node -e \"pkg.overrides['x']='^1.0.0'\"") + .unparsed.length, + ).toBeGreaterThan(0); + }); + it("pins the same range wherever both files name the same package", () => { - const docker = dockerfileOverrides(); - const workspace = workspaceOverrides(); + const docker = dockerfileOverrides().pins; + const workspace = workspaceOverrides().pins; const shared = [...docker.keys()].filter((name) => workspace.has(name)); expect(shared.length).toBeGreaterThan(0); @@ -131,7 +356,7 @@ describe("dependency overrides — pnpm workspace and the npm install agree", () // These three were each reported by the image scan under // `/opt/prisma-cli`, reached through `prisma`. Losing a mirror is not a // style regression, it is the vulnerable version coming back. - const docker = dockerfileOverrides(); + const docker = dockerfileOverrides().pins; for (const name of ["deepmerge-ts", "@hono/node-server", "valibot"]) { expect( docker.has(name), @@ -145,7 +370,7 @@ describe("dependency overrides — pnpm workspace and the npm install agree", () it("keeps the override ahead of the version Prisma's config asks for", () => { // `@prisma/config` requests deepmerge-ts 7.x; the advisory is fixed in // 8.0.0. Anything that lets a 7.x back in re-opens CVE-2026-40345. - const range = dockerfileOverrides().get("deepmerge-ts"); + const range = dockerfileOverrides().pins.get("deepmerge-ts"); expect(range).toBeDefined(); expect(range).toMatch(/\^?8\./); });