From 3135025d0df0e73e6c639f2ba98d5ef0d23985a3 Mon Sep 17 00:00:00 2001 From: PunGrumpy <108584943+PunGrumpy@users.noreply.github.com> Date: Thu, 23 Jul 2026 01:30:47 +0700 Subject: [PATCH 1/9] test(cli): add fixture-driven integration tests for the published CLI Establishes a verification baseline for @docker-doctor/cli by spawning the built dist/cli.mjs against small Dockerfile fixtures and asserting on the three documented exit-code paths and the --json report contract. --- packages/docker-doctor/package.json | 1 + packages/docker-doctor/test/cli.test.ts | 119 ++++++++++++++++++ .../test/fixtures/clean/.dockerignore | 1 + .../test/fixtures/clean/Dockerfile | 8 ++ .../test/fixtures/empty/.gitkeep | 0 .../test/fixtures/with-error/Dockerfile | 4 + 6 files changed, 133 insertions(+) create mode 100644 packages/docker-doctor/test/cli.test.ts create mode 100644 packages/docker-doctor/test/fixtures/clean/.dockerignore create mode 100644 packages/docker-doctor/test/fixtures/clean/Dockerfile create mode 100644 packages/docker-doctor/test/fixtures/empty/.gitkeep create mode 100644 packages/docker-doctor/test/fixtures/with-error/Dockerfile diff --git a/packages/docker-doctor/package.json b/packages/docker-doctor/package.json index 7a1c718..c9dbb0e 100644 --- a/packages/docker-doctor/package.json +++ b/packages/docker-doctor/package.json @@ -54,6 +54,7 @@ "scripts": { "build": "NODE_OPTIONS='--max-old-space-size=4096' tsdown", "dev": "NODE_OPTIONS='--max-old-space-size=4096' tsdown --watch", + "test": "bun test", "typecheck": "tsc --noEmit", "clean": "git clean -xdf .turbo node_modules dist" }, diff --git a/packages/docker-doctor/test/cli.test.ts b/packages/docker-doctor/test/cli.test.ts new file mode 100644 index 0000000..f6f056b --- /dev/null +++ b/packages/docker-doctor/test/cli.test.ts @@ -0,0 +1,119 @@ +import { beforeAll, describe, expect, test } from "bun:test"; +import path from "node:path"; + +const CLI = path.join(import.meta.dir, "..", "dist", "cli.mjs"); +const fixture = (name: string) => path.join(import.meta.dir, "fixtures", name); + +const runCli = async (args: string[]) => { + const proc = Bun.spawn(["node", CLI, ...args], { + stderr: "pipe", + stdout: "pipe", + }); + const stdout = await new Response(proc.stdout).text(); + const stderr = await new Response(proc.stderr).text(); + const exitCode = await proc.exited; + return { exitCode, stderr, stdout }; +}; + +beforeAll(async () => { + const exists = await Bun.file(CLI).exists(); + if (!exists) { + throw new Error( + `Built CLI not found at ${CLI}. Run: bun run build --filter @docker-doctor/cli` + ); + } +}); + +describe("smoke", () => { + test("--version exits 0 and prints a semver-like string", async () => { + const { exitCode, stdout } = await runCli(["--version"]); + expect(exitCode).toBe(0); + expect(stdout.trim()).toMatch(/^\d+\.\d+\.\d+/u); + }); +}); + +describe("exit codes", () => { + test("clean fixture with --json exits 0", async () => { + const { exitCode } = await runCli([fixture("clean"), "--json"]); + expect(exitCode).toBe(0); + }); + + test("with-error fixture with --json exits 1", async () => { + const { exitCode } = await runCli([fixture("with-error"), "--json"]); + expect(exitCode).toBe(1); + }); + + test("clean fixture with --score exits 0", async () => { + const { exitCode } = await runCli([fixture("clean"), "--score"]); + expect(exitCode).toBe(0); + }); +}); + +describe("--json contract", () => { + test("with-error fixture produces parseable JSON", async () => { + const { stdout } = await runCli([fixture("with-error"), "--json"]); + expect(() => JSON.parse(stdout)).not.toThrow(); + }); + + test("report has exactly the documented top-level keys", async () => { + const { stdout } = await runCli([fixture("with-error"), "--json"]); + const report = JSON.parse(stdout); + expect(Object.keys(report).toSorted()).toEqual( + ["diagnostics", "label", "project", "score", "timestamp"].toSorted() + ); + }); + + test("score is a number in [0, 100]", async () => { + const { stdout } = await runCli([fixture("with-error"), "--json"]); + const report = JSON.parse(stdout); + expect(typeof report.score).toBe("number"); + expect(report.score).toBeGreaterThanOrEqual(0); + expect(report.score).toBeLessThanOrEqual(100); + }); + + test("every diagnostic has the required shape", async () => { + const { stdout } = await runCli([fixture("with-error"), "--json"]); + const report = JSON.parse(stdout); + expect(Array.isArray(report.diagnostics)).toBe(true); + for (const d of report.diagnostics) { + expect(typeof d.file).toBe("string"); + expect(typeof d.help).toBe("string"); + expect(typeof d.message).toBe("string"); + expect(typeof d.rule).toBe("string"); + expect(["error", "warning", "info"]).toContain(d.severity); + } + }); + + test("at least one diagnostic has severity error", async () => { + const { stdout } = await runCli([fixture("with-error"), "--json"]); + const report = JSON.parse(stdout); + expect( + report.diagnostics.some( + (d: { severity: string }) => d.severity === "error" + ) + ).toBe(true); + }); +}); + +describe("--score contract", () => { + test("clean fixture prints only an integer score", async () => { + const { stdout } = await runCli([fixture("clean"), "--score"]); + const trimmed = stdout.trim(); + expect(Number.isInteger(Number(trimmed))).toBe(true); + expect(trimmed.split("\n").length).toBe(1); + }); + + test("clean fixture score is >= 50", async () => { + const { stdout } = await runCli([fixture("clean"), "--score"]); + expect(Number(stdout.trim())).toBeGreaterThanOrEqual(50); + }); +}); + +describe("empty project", () => { + test("scanning an empty directory with --json exits 0 with no diagnostics", async () => { + const { exitCode, stdout } = await runCli([fixture("empty"), "--json"]); + expect(exitCode).toBe(0); + const report = JSON.parse(stdout); + expect(report.diagnostics).toEqual([]); + }); +}); diff --git a/packages/docker-doctor/test/fixtures/clean/.dockerignore b/packages/docker-doctor/test/fixtures/clean/.dockerignore new file mode 100644 index 0000000..3c3629e --- /dev/null +++ b/packages/docker-doctor/test/fixtures/clean/.dockerignore @@ -0,0 +1 @@ +node_modules diff --git a/packages/docker-doctor/test/fixtures/clean/Dockerfile b/packages/docker-doctor/test/fixtures/clean/Dockerfile new file mode 100644 index 0000000..84e0ae1 --- /dev/null +++ b/packages/docker-doctor/test/fixtures/clean/Dockerfile @@ -0,0 +1,8 @@ +FROM node:22.2.0-alpine +WORKDIR /app +COPY package.json ./ +RUN npm ci --omit=dev +COPY src ./src +USER node +HEALTHCHECK --interval=30s --timeout=3s CMD node -e "process.exit(0)" +CMD ["node", "src/index.js"] diff --git a/packages/docker-doctor/test/fixtures/empty/.gitkeep b/packages/docker-doctor/test/fixtures/empty/.gitkeep new file mode 100644 index 0000000..e69de29 diff --git a/packages/docker-doctor/test/fixtures/with-error/Dockerfile b/packages/docker-doctor/test/fixtures/with-error/Dockerfile new file mode 100644 index 0000000..8a448cc --- /dev/null +++ b/packages/docker-doctor/test/fixtures/with-error/Dockerfile @@ -0,0 +1,4 @@ +FROM node:22.2.0-alpine +ENV DB_PASSWORD=hunter2 +USER node +CMD ["node", "index.js"] From f51920e34e325a3ee2387a16b20464e209aa84ca Mon Sep 17 00:00:00 2001 From: PunGrumpy <108584943+PunGrumpy@users.noreply.github.com> Date: Thu, 23 Jul 2026 02:38:16 +0700 Subject: [PATCH 2/9] fix(core): reset no-root-user tracking at each build stage The no-root-user rule tracked the last USER instruction across the entire Dockerfile without resetting at each FROM, so a USER set in an earlier build stage (e.g. the builder) silently satisfied the check for a later stage that never sets one. Reset the tracked user to "root" on every FROM so only the final stage's effective user is evaluated. --- packages/core/src/rules/security.ts | 5 ++++- packages/core/test/rules.test.ts | 13 +++++++++++++ 2 files changed, 17 insertions(+), 1 deletion(-) diff --git a/packages/core/src/rules/security.ts b/packages/core/src/rules/security.ts index 53a01a7..95e319a 100644 --- a/packages/core/src/rules/security.ts +++ b/packages/core/src/rules/security.ts @@ -16,7 +16,10 @@ export const noRootUser: DockerfileRule = { let lastUserLine = 1; for (const inst of instructions) { - if (inst.instruction === "USER") { + if (inst.instruction === "FROM") { + lastUser = "root"; + lastUserLine = inst.line; + } else if (inst.instruction === "USER") { lastUser = inst.args.trim().toLowerCase(); lastUserLine = inst.line; } diff --git a/packages/core/test/rules.test.ts b/packages/core/test/rules.test.ts index aaa1c9a..191ed93 100644 --- a/packages/core/test/rules.test.ts +++ b/packages/core/test/rules.test.ts @@ -55,6 +55,19 @@ describe("Security Rules", () => { expect(diags2).toHaveLength(0); }); + test("no-root-user: multi-stage runtime without USER", () => { + const multiStageRootRuntime = parseDockerfile(` + FROM node:22-alpine AS build + USER node + RUN npm run build + FROM node:22-alpine + COPY --from=build /app/dist ./dist + CMD ["node", "dist/index.js"] + `); + const diags3 = noRootUser.check(multiStageRootRuntime, "Dockerfile"); + expect(diags3).toHaveLength(1); + }); + test("pin-image-version", () => { const unpinned = parseDockerfile(` FROM node From 2db2b6bbc07b16cf62f7342b49b1e0d3d15cfad0 Mon Sep 17 00:00:00 2001 From: PunGrumpy <108584943+PunGrumpy@users.noreply.github.com> Date: Thu, 23 Jul 2026 02:39:47 +0700 Subject: [PATCH 3/9] fix(core): anchor secret keyword matching to segment boundaries no-secrets-in-env matched keywords like /auth/ anywhere in the env key, so ENV AUTHOR=grumpy tripped the check even though "author" has no secret in it. Anchor each keyword to a SCREAMING_SNAKE_CASE segment boundary (_, -, or string start/end) so substring matches like AUTHOR and OAUTH_ISSUER_URL no longer false-positive, while genuine segments like DB_PASSWORD still match. --- packages/core/src/rules/security.ts | 12 ++++++------ packages/core/test/rules.test.ts | 16 ++++++++++++++++ 2 files changed, 22 insertions(+), 6 deletions(-) diff --git a/packages/core/src/rules/security.ts b/packages/core/src/rules/security.ts index 95e319a..ef6872a 100644 --- a/packages/core/src/rules/security.ts +++ b/packages/core/src/rules/security.ts @@ -51,12 +51,12 @@ export const noSecretsInEnv: DockerfileRule = { check(instructions, file) { const diagnostics: Diagnostic[] = []; const secretKeywords = [ - /password/iu, - /secret/iu, - /token/iu, - /api_key/iu, - /private_key/iu, - /auth/iu, + /(?:^|[_-])password(?:[_-]|$)/iu, + /(?:^|[_-])secret(?:[_-]|$)/iu, + /(?:^|[_-])token(?:[_-]|$)/iu, + /(?:^|[_-])api_key(?:[_-]|$)/iu, + /(?:^|[_-])private_key(?:[_-]|$)/iu, + /(?:^|[_-])auth(?:[_-]|$)/iu, ]; for (const inst of instructions) { diff --git a/packages/core/test/rules.test.ts b/packages/core/test/rules.test.ts index 191ed93..9765e49 100644 --- a/packages/core/test/rules.test.ts +++ b/packages/core/test/rules.test.ts @@ -111,6 +111,22 @@ describe("Security Rules", () => { expect(diagsSpaceNormal).toHaveLength(0); }); + test("no-secrets-in-env: AUTHOR is not a secret", () => { + const authorNotSecret = parseDockerfile(` + ENV AUTHOR=grumpy + `); + expect(noSecretsInEnv.check(authorNotSecret, "Dockerfile")).toHaveLength(0); + }); + + test("no-secrets-in-env: OAUTH_ISSUER_URL is not a secret", () => { + const oauthUrlNotSecret = parseDockerfile(` + ENV OAUTH_ISSUER_URL=https://example.com + `); + expect(noSecretsInEnv.check(oauthUrlNotSecret, "Dockerfile")).toHaveLength( + 0 + ); + }); + test("no-add-remote", () => { const remoteAdd = parseDockerfile(` ADD https://example.com/file.txt /app/ From 0b1cd1a1bf4aa8ee3816b3c9323d2a364dd7db80 Mon Sep 17 00:00:00 2001 From: PunGrumpy <108584943+PunGrumpy@users.noreply.github.com> Date: Thu, 23 Jul 2026 02:41:25 +0700 Subject: [PATCH 4/9] fix(core): read the first non-flag operand in ADD/COPY source rules no-add-remote, prefer-copy-over-add, and use-dockerignore all read parts[0] as the ADD/COPY source, so a leading flag like --chown or --from silently disabled them (e.g. ADD --chown=node:node https://... never tripped no-add-remote). Use the same parts.find((p) => !p.startsWith("--")) idiom already used by pin-image-version and order-layers. As a side effect, use-dockerignore would now fire on stage-to-stage COPY --from= . ./, which never reads the build context, so that instruction shape is explicitly skipped. --- packages/core/src/rules/best-practices.ts | 6 +++++- packages/core/src/rules/performance.ts | 12 +++++++++++- packages/core/src/rules/security.ts | 6 +++++- packages/core/test/rules.test.ts | 18 ++++++++++++++++++ 4 files changed, 39 insertions(+), 3 deletions(-) diff --git a/packages/core/src/rules/best-practices.ts b/packages/core/src/rules/best-practices.ts index 3b92d04..410b218 100644 --- a/packages/core/src/rules/best-practices.ts +++ b/packages/core/src/rules/best-practices.ts @@ -52,7 +52,11 @@ export const preferCopyOverAdd: DockerfileRule = { for (const inst of instructions) { if (inst.instruction === "ADD") { const parts = inst.args.split(/\s+/u); - const [src] = parts; + const src = parts.find((p) => !p.startsWith("--")); + + if (!src) { + continue; + } // If it's not a remote url (handled by security/no-add-remote) and not a compressed file const isRemote = diff --git a/packages/core/src/rules/performance.ts b/packages/core/src/rules/performance.ts index 1fb65a6..a44bbf0 100644 --- a/packages/core/src/rules/performance.ts +++ b/packages/core/src/rules/performance.ts @@ -160,8 +160,18 @@ export const useDockerignore: DockerfileRule = { // If copying everything, we definitely need .dockerignore const hasCopyAll = instructions.some((inst) => { if (inst.instruction === "COPY" || inst.instruction === "ADD") { + // COPY --from= reads from a previous build stage, not the + // build context, so .dockerignore is irrelevant to it. + const isStageCopy = /(?:^|\s)--from=/u.test(inst.args); + if (isStageCopy) { + return false; + } + const parts = inst.args.split(/\s+/u); - const [src] = parts; + const src = parts.find((p) => !p.startsWith("--")); + if (!src) { + return false; + } return src === "." || src === "./" || src === "*"; } return false; diff --git a/packages/core/src/rules/security.ts b/packages/core/src/rules/security.ts index ef6872a..ae84472 100644 --- a/packages/core/src/rules/security.ts +++ b/packages/core/src/rules/security.ts @@ -196,7 +196,11 @@ export const noAddRemote: DockerfileRule = { for (const inst of instructions) { if (inst.instruction === "ADD") { const parts = inst.args.split(/\s+/u); - const [src] = parts; + const src = parts.find((p) => !p.startsWith("--")); + + if (!src) { + continue; + } if (src.startsWith("http://") || src.startsWith("https://")) { diagnostics.push( diff --git a/packages/core/test/rules.test.ts b/packages/core/test/rules.test.ts index 9765e49..5e78766 100644 --- a/packages/core/test/rules.test.ts +++ b/packages/core/test/rules.test.ts @@ -134,6 +134,13 @@ describe("Security Rules", () => { const diags1 = noAddRemote.check(remoteAdd, "Dockerfile"); expect(diags1).toHaveLength(1); }); + + test("no-add-remote: remote URL with --chown flag", () => { + const remoteAddWithChown = parseDockerfile(` + ADD --chown=node:node https://example.com/file.txt /app/ + `); + expect(noAddRemote.check(remoteAddWithChown, "Dockerfile")).toHaveLength(1); + }); }); describe("Performance Rules", () => { @@ -182,6 +189,17 @@ describe("Performance Rules", () => { }); expect(diags2).toHaveLength(0); }); + + test("use-dockerignore ignores stage-to-stage copies", () => { + const stageCopy = parseDockerfile(` + FROM node:22-alpine AS build + FROM node:22-alpine + COPY --from=build . ./ + `); + expect( + useDockerignore.check(stageCopy, "Dockerfile", { projectFiles: [] }) + ).toHaveLength(0); + }); }); describe("Compose Rules", () => { From aa6706de39bc7b2c8214727308624b4aabbc64cc Mon Sep 17 00:00:00 2001 From: PunGrumpy <108584943+PunGrumpy@users.noreply.github.com> Date: Thu, 23 Jul 2026 02:42:38 +0700 Subject: [PATCH 5/9] fix(core): reset order-layers tracking per stage, tighten src match order-layers never reset copyAllLine on FROM, so a copy-everything in an earlier build stage falsely implicated an install command in a later, correctly-ordered stage. It also matched any path containing the substring "src" (e.g. /usr/src/lib, mysrcdir), not just an actual src directory. Reset the tracker per stage and replace the substring check with an exact match against "." / "./" / "*" / "src" / "src/*". --- packages/core/src/rules/performance.ts | 17 +++++++++++++---- packages/core/test/rules.test.ts | 21 +++++++++++++++++++++ 2 files changed, 34 insertions(+), 4 deletions(-) diff --git a/packages/core/src/rules/performance.ts b/packages/core/src/rules/performance.ts index a44bbf0..ca100db 100644 --- a/packages/core/src/rules/performance.ts +++ b/packages/core/src/rules/performance.ts @@ -55,6 +55,10 @@ export const orderLayers: DockerfileRule = { let copyAllLine = -1; for (const inst of instructions) { + if (inst.instruction === "FROM") { + copyAllLine = -1; + } + if (inst.instruction === "COPY" || inst.instruction === "ADD") { const parts = inst.args.split(/\s+/u); const src = parts.find((p) => !p.startsWith("--")); @@ -63,10 +67,15 @@ export const orderLayers: DockerfileRule = { continue; } - if ( - (src === "." || src === "./" || src === "*" || src.includes("src")) && - copyAllLine === -1 - ) { + const normalized = src.replace(/^\.\//u, ""); + const isCopyAll = + normalized === "." || + normalized === "" || + normalized === "*" || + normalized === "src" || + normalized.startsWith("src/"); + + if (isCopyAll && copyAllLine === -1) { copyAllLine = inst.line; } } diff --git a/packages/core/test/rules.test.ts b/packages/core/test/rules.test.ts index 5e78766..2a5ac8a 100644 --- a/packages/core/test/rules.test.ts +++ b/packages/core/test/rules.test.ts @@ -155,6 +155,27 @@ describe("Performance Rules", () => { expect(diags[0].rule).toBe("docker-doctor/order-layers"); }); + test("order-layers: correct multi-stage build", () => { + const correctMultiStage = parseDockerfile(` + FROM node:22-alpine AS build + COPY . . + RUN npm run build + FROM node:22-alpine + COPY package.json ./ + RUN npm ci --omit=dev + `); + expect(orderLayers.check(correctMultiStage, "Dockerfile")).toHaveLength(0); + }); + + test("order-layers: /usr/src path is not a copy-all", () => { + const usrSrcPath = parseDockerfile(` + FROM node:22-alpine + COPY /usr/src/lib /lib + RUN npm ci + `); + expect(orderLayers.check(usrSrcPath, "Dockerfile")).toHaveLength(0); + }); + test("use-multi-stage", () => { const singleStage = parseDockerfile(` FROM node:22 From 3d4d06bc597794c048c8ab65a40e61c52288c78e Mon Sep 17 00:00:00 2001 From: PunGrumpy <108584943+PunGrumpy@users.noreply.github.com> Date: Thu, 23 Jul 2026 02:44:08 +0700 Subject: [PATCH 6/9] chore(changeset): patch @docker-doctor/cli for rule-specificity fixes Bundled packages/core ships inside @docker-doctor/cli, and these four diagnostic changes are user-visible, so they require a changeset per this repo's convention. --- .changeset/four-quiet-rules.md | 12 ++++++++++++ 1 file changed, 12 insertions(+) create mode 100644 .changeset/four-quiet-rules.md diff --git a/.changeset/four-quiet-rules.md b/.changeset/four-quiet-rules.md new file mode 100644 index 0000000..7e45559 --- /dev/null +++ b/.changeset/four-quiet-rules.md @@ -0,0 +1,12 @@ +--- +"@docker-doctor/cli": patch +--- + +Fix four rule-specificity bugs in the Dockerfile rule engine: + +- `no-root-user` now resets its tracked user at each `FROM` (build stage), instead of letting a `USER` set in an earlier stage (e.g. a builder) silently satisfy the check for a later stage that never sets one. Multi-stage Dockerfiles may now surface a new diagnostic here. +- `no-add-remote`, `prefer-copy-over-add`, and `use-dockerignore` now read the first non-flag operand of `ADD`/`COPY` instead of always taking the first whitespace-separated token, so a leading flag like `--chown` or `--from` no longer silently disables the check (e.g. `ADD --chown=node:node https://...` now correctly trips `no-add-remote`). As a side effect, `use-dockerignore` now explicitly skips `COPY --from=` since that never reads the build context. These fixes may surface new diagnostics. +- `no-secrets-in-env` no longer matches secret keywords as an unanchored substring, so keys like `AUTHOR` or `OAUTH_ISSUER_URL` no longer false-positive as "potential secret found" (this is the only error-severity rule, so this previously failed CI on Dockerfiles with no secrets). Genuine segment matches like `DB_PASSWORD` still match. This removes diagnostics. +- `order-layers` now resets its copy-tracking at each build stage, so a copy-everything in an earlier stage no longer implicates an install command in a later, correctly-ordered stage, and no longer substring-matches `"src"` anywhere in a path (e.g. `/usr/src/lib`, `mysrcdir`). This removes diagnostics. + +These are bug fixes to rule specificity; expect docker-doctor scores to move on existing Dockerfiles as false negatives are corrected and false positives are removed. From 014aa6825e6e420d9184049cb90a5b28ab7a6a79 Mon Sep 17 00:00:00 2001 From: PunGrumpy <108584943+PunGrumpy@users.noreply.github.com> Date: Thu, 23 Jul 2026 09:05:54 +0700 Subject: [PATCH 7/9] fix(core): anchor Dockerfile instruction regex to known keywords The permissive case-insensitive [A-Z]+ regex treated any "word arg" line as an instruction, causing false parses. Anchor to the real Dockerfile keyword set (module-level, built once) while still accepting lowercase instructions. --- .../core/src/parsers/dockerfile-parser.ts | 38 ++++++++++++++----- packages/core/test/parser.test.ts | 19 ++++++++++ 2 files changed, 48 insertions(+), 9 deletions(-) diff --git a/packages/core/src/parsers/dockerfile-parser.ts b/packages/core/src/parsers/dockerfile-parser.ts index 6c4b587..fd69c42 100644 --- a/packages/core/src/parsers/dockerfile-parser.ts +++ b/packages/core/src/parsers/dockerfile-parser.ts @@ -1,5 +1,28 @@ import type { DockerfileInstruction } from "../types/index"; +const DOCKERFILE_KEYWORDS = new Set([ + "ADD", + "ARG", + "CMD", + "COPY", + "ENTRYPOINT", + "ENV", + "EXPOSE", + "FROM", + "HEALTHCHECK", + "LABEL", + "MAINTAINER", + "ONBUILD", + "RUN", + "SHELL", + "STOPSIGNAL", + "USER", + "VOLUME", + "WORKDIR", +]); + +const INSTRUCTION_LINE_RE = /^(?[A-Za-z]+)\s+(?.*)$/u; + export const parseDockerfile = (content: string): DockerfileInstruction[] => { const instructions: DockerfileInstruction[] = []; const lines = content.split(/\r?\n/u); @@ -37,17 +60,14 @@ export const parseDockerfile = (content: string): DockerfileInstruction[] => { } else { startLine = lineNum; // Match the first instruction word (e.g. FROM, RUN, COPY) - const match = lineContent.match(/^(?[A-Z]+)\s+(?.*)$/iu); - if (match?.groups) { - currentInstruction = match.groups.inst.toUpperCase(); - currentArgs = match.groups.args; + const match = lineContent.match(INSTRUCTION_LINE_RE); + const matchedWord = match?.groups?.inst.toUpperCase(); + if (matchedWord && DOCKERFILE_KEYWORDS.has(matchedWord)) { + currentInstruction = matchedWord; + currentArgs = match?.groups?.args ?? ""; } else { const word = lineContent.trim().toUpperCase(); - if ( - ["RUN", "CMD", "ENTRYPOINT", "EXPOSE", "USER", "WORKDIR"].includes( - word - ) - ) { + if (DOCKERFILE_KEYWORDS.has(word)) { currentInstruction = word; currentArgs = ""; } diff --git a/packages/core/test/parser.test.ts b/packages/core/test/parser.test.ts index a4487af..2e6107b 100644 --- a/packages/core/test/parser.test.ts +++ b/packages/core/test/parser.test.ts @@ -39,6 +39,25 @@ describe("Dockerfile Parser", () => { "apt-get update && apt-get install -y curl && rm -rf /var/lib/apt/lists/*" ); }); + + test("ignores non-instruction lines", () => { + const parsed = parseDockerfile(` + FROM node:22 + this is not an instruction + `); + expect(parsed).toHaveLength(1); + expect(parsed[0].instruction).toBe("FROM"); + }); + + test("accepts lowercase instructions", () => { + const parsed = parseDockerfile(` + from node:22 + run echo hi + `); + expect(parsed).toHaveLength(2); + expect(parsed[0].instruction).toBe("FROM"); + expect(parsed[1].instruction).toBe("RUN"); + }); }); describe("Compose Parser", () => { From 3062d1a2639830c452200b2287afc7850a5f53b8 Mon Sep 17 00:00:00 2001 From: PunGrumpy <108584943+PunGrumpy@users.noreply.github.com> Date: Thu, 23 Jul 2026 09:11:05 +0700 Subject: [PATCH 8/9] fix(core): parse heredoc bodies instead of treating them as instructions BuildKit heredocs (<[A-Za-z]+)\s+(?.*)$/u; +// Matches a heredoc opener like <['"]?)(?\w+)\k/gu; + +interface ParserState { + instructions: DockerfileInstruction[]; + currentInstruction: string; + currentArgs: string; + startLine: number; + rawAccumulator: string[]; + // FIFO of heredoc delimiters still open for the current instruction, in + // the order they were opened (closed in the same order). + heredocQueue: string[]; +} + +const createParserState = (): ParserState => ({ + currentArgs: "", + currentInstruction: "", + heredocQueue: [], + instructions: [], + rawAccumulator: [], + startLine: 0, +}); + +const closeInstruction = (state: ParserState): void => { + if (state.currentInstruction) { + state.instructions.push({ + args: state.currentArgs, + instruction: state.currentInstruction, + line: state.startLine, + raw: state.rawAccumulator.join("\n"), + }); + } + state.currentInstruction = ""; + state.currentArgs = ""; + state.rawAccumulator = []; +}; + +const matchInstructionKeyword = ( + lineContent: string +): { instruction: string; args: string } | null => { + const match = lineContent.match(INSTRUCTION_LINE_RE); + const matchedWord = match?.groups?.inst.toUpperCase(); + if (matchedWord && DOCKERFILE_KEYWORDS.has(matchedWord)) { + return { args: match?.groups?.args ?? "", instruction: matchedWord }; + } + + const word = lineContent.trim().toUpperCase(); + if (DOCKERFILE_KEYWORDS.has(word)) { + return { args: "", instruction: word }; + } + + return null; +}; + +const findHeredocDelimiters = (lineContent: string): string[] => + [...lineContent.matchAll(HEREDOC_OPENER_RE)].map( + (m) => m.groups?.delim ?? "" + ); + +// Heredoc body lines are never treated as instructions (not even comment +// lines, which are shell-comment content here) — they are folded verbatim +// into the owning instruction's args until the delimiter line closes the +// (possibly multiple) open heredoc(s), in the order they were opened. +const processHeredocLine = (state: ParserState, trimmed: string): void => { + if (trimmed === state.heredocQueue[0]) { + state.heredocQueue.shift(); + } else { + state.currentArgs += (state.currentArgs ? " " : "") + trimmed; + } + + if (state.heredocQueue.length === 0) { + // Last open heredoc just closed: the instruction ends here. + closeInstruction(state); + } +}; + +const processInstructionLine = ( + state: ParserState, + trimmed: string, + lineNum: number +): void => { + let lineContent = trimmed; + + // Inside a multi-line run, comment lines are ignored by docker parser + if (lineContent.startsWith("#")) { + return; + } + + const hasContinuation = lineContent.endsWith("\\"); + if (hasContinuation) { + lineContent = lineContent.slice(0, -1).trim(); + } + + if (state.currentInstruction) { + state.currentArgs += (state.currentArgs ? " " : "") + lineContent; + } else { + state.startLine = lineNum; + // Match the first instruction word (e.g. FROM, RUN, COPY) + const matched = matchInstructionKeyword(lineContent); + if (matched) { + state.currentInstruction = matched.instruction; + state.currentArgs = matched.args; + } + } + + if (state.currentInstruction) { + state.heredocQueue.push(...findHeredocDelimiters(lineContent)); + } + + if (state.heredocQueue.length > 0) { + // A heredoc opener implies continuation even without a trailing + // backslash — keep accumulating until every opened delimiter closes. + return; + } + + if (!hasContinuation) { + closeInstruction(state); + } +}; + export const parseDockerfile = (content: string): DockerfileInstruction[] => { - const instructions: DockerfileInstruction[] = []; + const state = createParserState(); const lines = content.split(/\r?\n/u); - let currentInstruction = ""; - let currentArgs = ""; - let startLine = 0; - let rawAccumulator: string[] = []; - for (const [i, rawLine] of lines.entries()) { const trimmed = rawLine.trim(); const lineNum = i + 1; + const insideHeredoc = state.heredocQueue.length > 0; // Skip empty lines or comment lines if not in multi-line block - if (!currentInstruction && (trimmed === "" || trimmed.startsWith("#"))) { + if ( + !state.currentInstruction && + !insideHeredoc && + (trimmed === "" || trimmed.startsWith("#")) + ) { continue; } - rawAccumulator.push(rawLine); - - let lineContent = trimmed; - - // Inside a multi-line run, comment lines are ignored by docker parser - if (lineContent.startsWith("#")) { - continue; - } + state.rawAccumulator.push(rawLine); - const hasContinuation = lineContent.endsWith("\\"); - if (hasContinuation) { - lineContent = lineContent.slice(0, -1).trim(); - } - - if (currentInstruction) { - currentArgs += (currentArgs ? " " : "") + lineContent; + if (insideHeredoc) { + processHeredocLine(state, trimmed); } else { - startLine = lineNum; - // Match the first instruction word (e.g. FROM, RUN, COPY) - const match = lineContent.match(INSTRUCTION_LINE_RE); - const matchedWord = match?.groups?.inst.toUpperCase(); - if (matchedWord && DOCKERFILE_KEYWORDS.has(matchedWord)) { - currentInstruction = matchedWord; - currentArgs = match?.groups?.args ?? ""; - } else { - const word = lineContent.trim().toUpperCase(); - if (DOCKERFILE_KEYWORDS.has(word)) { - currentInstruction = word; - currentArgs = ""; - } - } - } - - if (!hasContinuation) { - if (currentInstruction) { - instructions.push({ - args: currentArgs, - instruction: currentInstruction, - line: startLine, - raw: rawAccumulator.join("\n"), - }); - } - currentInstruction = ""; - currentArgs = ""; - rawAccumulator = []; + processInstructionLine(state, trimmed, lineNum); } } - if (currentInstruction) { - instructions.push({ - args: currentArgs, - instruction: currentInstruction, - line: startLine, - raw: rawAccumulator.join("\n"), - }); - } + // EOF: an unterminated heredoc (or a trailing backslash continuation) + // still emits whatever was accumulated so far, rather than dropping it. + closeInstruction(state); - return instructions; + return state.instructions; }; diff --git a/packages/core/test/parser.test.ts b/packages/core/test/parser.test.ts index 2e6107b..787a187 100644 --- a/packages/core/test/parser.test.ts +++ b/packages/core/test/parser.test.ts @@ -58,6 +58,60 @@ describe("Dockerfile Parser", () => { expect(parsed[0].instruction).toBe("FROM"); expect(parsed[1].instruction).toBe("RUN"); }); + + test("parses heredoc bodies into the owning instruction", () => { + const parsed = parseDockerfile(` +FROM node:22 +RUN < { + const parsed = parseDockerfile(` +FROM node:22 +RUN < i.instruction === "USER")).toBe(false); + }); + + test("handles quoted and dash heredoc delimiters", () => { + const parsed = parseDockerfile(` +FROM node:22 +RUN <<-'EOT' +echo hello +EOT + `); + expect(parsed).toHaveLength(2); + expect(parsed[1].args).toContain("echo hello"); + }); + + test("preserves heredoc body in raw", () => { + const parsed = parseDockerfile(` +FROM node:22 +RUN < { + const parsed = parseDockerfile("FROM node:22\r\nUSER node\r\n"); + expect(parsed).toHaveLength(2); + expect(parsed[1].instruction).toBe("USER"); + }); }); describe("Compose Parser", () => { diff --git a/packages/docker-doctor/test/cli.test.ts b/packages/docker-doctor/test/cli.test.ts index f6f056b..db15a18 100644 --- a/packages/docker-doctor/test/cli.test.ts +++ b/packages/docker-doctor/test/cli.test.ts @@ -117,3 +117,11 @@ describe("empty project", () => { expect(report.diagnostics).toEqual([]); }); }); + +describe("heredoc fixture", () => { + test("heredoc fixture with --json yields at least one diagnostic", async () => { + const { stdout } = await runCli([fixture("heredoc"), "--json"]); + const report = JSON.parse(stdout); + expect(report.diagnostics.length).toBeGreaterThanOrEqual(1); + }); +}); diff --git a/packages/docker-doctor/test/fixtures/heredoc/Dockerfile b/packages/docker-doctor/test/fixtures/heredoc/Dockerfile new file mode 100644 index 0000000..325ca17 --- /dev/null +++ b/packages/docker-doctor/test/fixtures/heredoc/Dockerfile @@ -0,0 +1,7 @@ +FROM node:22.2.0-alpine +RUN < Date: Thu, 23 Jul 2026 09:11:23 +0700 Subject: [PATCH 9/9] chore(changeset): patch @docker-doctor/cli for heredoc parsing packages/core is bundled into @docker-doctor/cli, and heredoc bodies now feed content-based rules, so this is a published behavior change. --- .changeset/dockerfile-heredoc-parsing.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/dockerfile-heredoc-parsing.md diff --git a/.changeset/dockerfile-heredoc-parsing.md b/.changeset/dockerfile-heredoc-parsing.md new file mode 100644 index 0000000..860cfb2 --- /dev/null +++ b/.changeset/dockerfile-heredoc-parsing.md @@ -0,0 +1,5 @@ +--- +"@docker-doctor/cli": patch +--- + +The Dockerfile parser now understands heredoc (`<