fix(core): extract a shared image-reference parser

.changeset/shy-lemons-parse.md

@@ -0,0 +1,12 @@
+---
+"@docker-doctor/cli": patch
+---
+
+Fix image-reference parsing in `pin-image-version` and `prefer-slim-base`, which both used to split on the first `:` and got several common forms wrong:
+
+- Images with a registry port (`myregistry.example.com:5000/team/app`) are no longer mistaken for pinned/tagged just because the port number looks like a tag. `pin-image-version` now correctly flags these as unpinned.
+- Digest-pinned images (`node@sha256:...`) are no longer misread as an untagged full-OS image by `prefer-slim-base` — the strongest possible pin is now recognized and silently accepted by both rules.
+- Multi-stage build aliases (`FROM builder`, referencing an earlier `FROM ... AS builder`) are no longer flagged as unpinned or non-slim.
+- `${ARG}`-driven base images are skipped instead of producing a false diagnostic, since the actual image can't be determined statically.
+
+Image-reference parsing is now handled by a single shared parser (`parseImageRef`/`collectStageAliases` in `@docker-doctor/core`) instead of duplicated ad-hoc string splitting in each rule. Expect diagnostics to change on Dockerfiles that use registry ports, digest pins, or multi-stage aliases as their `FROM` target.

packages/core/src/parsers/image-ref.ts

@@ -0,0 +1,74 @@
+import type { DockerfileInstruction } from "../types/index";
+
+export interface ImageRef {
+  registry?: string;
+  name: string;
+  tag?: string;
+  digest?: string;
+  isVariable: boolean;
+}
+
+export const parseImageRef = (ref: string): ImageRef => {
+  if (ref.includes("${") || ref.startsWith("$")) {
+    return { isVariable: true, name: ref };
+  }
+
+  let remainder = ref;
+  let digest: string | undefined;
+
+  const atIndex = remainder.indexOf("@");
+  if (atIndex !== -1) {
+    digest = remainder.slice(atIndex + 1);
+    remainder = remainder.slice(0, atIndex);
+  }
+
+  let tag: string | undefined;
+  const lastColonIndex = remainder.lastIndexOf(":");
+  const lastSlashIndex = remainder.lastIndexOf("/");
+
+  if (lastColonIndex !== -1 && lastColonIndex > lastSlashIndex) {
+    tag = remainder.slice(lastColonIndex + 1);
+    remainder = remainder.slice(0, lastColonIndex);
+  }
+
+  let registry: string | undefined;
+  const firstSlashIndex = remainder.indexOf("/");
+  if (firstSlashIndex !== -1) {
+    const firstSegment = remainder.slice(0, firstSlashIndex);
+    if (
+      firstSegment.includes(".") ||
+      firstSegment.includes(":") ||
+      firstSegment === "localhost"
+    ) {
+      registry = firstSegment;
+      remainder = remainder.slice(firstSlashIndex + 1);
+    }
+  }
+
+  return {
+    digest,
+    isVariable: false,
+    name: remainder,
+    registry,
+    tag,
+  };
+};
+
+export const collectStageAliases = (
+  instructions: DockerfileInstruction[]
+): Set<string> => {
+  const aliases = new Set<string>();
+
+  for (const inst of instructions) {
+    if (inst.instruction !== "FROM") {
+      continue;
+    }
+
+    const match = /\sas\s+(?<alias>\S+)/iu.exec(inst.args);
+    if (match?.groups?.alias) {
+      aliases.add(match.groups.alias.toLowerCase());
+    }
+  }
+
+  return aliases;
+};

packages/core/src/rules/image-size.ts

@@ -1,3 +1,4 @@
+import { collectStageAliases, parseImageRef } from "../parsers/image-ref";
 import type { Diagnostic, DockerfileRule } from "../types/index";
 
 const createDiagnostic = (
@@ -13,6 +14,7 @@ export const preferSlimBase: DockerfileRule = {
   category: "Image Size",
   check(instructions, file) {
     const diagnostics: Diagnostic[] = [];
+    const stageAliases = collectStageAliases(instructions);
 
     for (const inst of instructions) {
       if (inst.instruction === "FROM") {
@@ -22,39 +24,39 @@ export const preferSlimBase: DockerfileRule = {
           continue;
         }
 
-        const colonIndex = imagePart.indexOf(":");
-        if (colonIndex !== -1) {
-          const tag = imagePart.slice(colonIndex + 1).toLowerCase();
-
-          // If the tag doesn't contain alpine, slim, distroless, or sha256
-          const isSlim =
-            tag.includes("alpine") ||
-            tag.includes("slim") ||
-            tag.includes("distroless");
-          const isSha = tag.startsWith("sha256:");
-
-          // Skip if it's a multi-stage builder step reference (e.g. FROM build AS publish)
-          const isStageReference = instructions.some(
-            (other) =>
-              other.line < inst.line &&
-              other.instruction === "FROM" &&
-              other.args
-                .toLowerCase()
-                .includes(` as ${imagePart.toLowerCase()}`)
-          );
+        const ref = parseImageRef(imagePart);
+
+        if (ref.isVariable || stageAliases.has(imagePart.toLowerCase())) {
+          continue;
+        }
+
+        // Digest pins are already fully deterministic; not our concern here.
+        if (ref.digest) {
+          continue;
+        }
+
+        // No tag: pin-image-version owns the untagged case, don't double-report.
+        if (!ref.tag) {
+          continue;
+        }
 
-          if (!isSlim && !isSha && !isStageReference) {
-            diagnostics.push(
-              createDiagnostic(
-                file,
-                this.key,
-                this.defaultSeverity as "error" | "warning" | "info",
-                `Base image '${imagePart}' may be a full-OS distribution. Consider using a slim or alpine alternative.`,
-                this.help,
-                inst.line
-              )
-            );
-          }
+        const tag = ref.tag.toLowerCase();
+        const isSlim =
+          tag.includes("alpine") ||
+          tag.includes("slim") ||
+          tag.includes("distroless");
+
+        if (!isSlim) {
+          diagnostics.push(
+            createDiagnostic(
+              file,
+              this.key,
+              this.defaultSeverity as "error" | "warning" | "info",
+              `Base image '${imagePart}' may be a full-OS distribution. Consider using a slim or alpine alternative.`,
+              this.help,
+              inst.line
+            )
+          );
         }
       }
     }

packages/core/src/rules/security.ts

@@ -1,3 +1,4 @@
+import { collectStageAliases, parseImageRef } from "../parsers/image-ref";
 import type { Diagnostic, DockerfileRule } from "../types/index";
 
 const createDiagnostic = (
@@ -136,6 +137,7 @@ export const pinImageVersion: DockerfileRule = {
   category: "Security",
   check(instructions, file) {
     const diagnostics: Diagnostic[] = [];
+    const stageAliases = collectStageAliases(instructions);
 
     for (const inst of instructions) {
       if (inst.instruction === "FROM") {
@@ -148,10 +150,13 @@ export const pinImageVersion: DockerfileRule = {
           continue;
         }
 
-        const colonIndex = imagePart.indexOf(":");
-        const atIndex = imagePart.indexOf("@");
+        const ref = parseImageRef(imagePart);
 
-        if (colonIndex === -1 && atIndex === -1) {
+        if (ref.isVariable || stageAliases.has(imagePart.toLowerCase())) {
+          continue;
+        }
+
+        if (!(ref.tag || ref.digest)) {
           diagnostics.push(
             createDiagnostic(
               file,
@@ -162,20 +167,17 @@ export const pinImageVersion: DockerfileRule = {
               inst.line
             )
           );
-        } else if (colonIndex !== -1) {
-          const tag = imagePart.slice(colonIndex + 1);
-          if (tag === "latest") {
-            diagnostics.push(
-              createDiagnostic(
-                file,
-                this.key,
-                this.defaultSeverity as "error" | "warning" | "info",
-                `Base image '${imagePart}' uses the mutable 'latest' tag. This makes builds non-deterministic.`,
-                this.help,
-                inst.line
-              )
-            );
-          }
+        } else if (ref.tag === "latest" && !ref.digest) {
+          diagnostics.push(
+            createDiagnostic(
+              file,
+              this.key,
+              this.defaultSeverity as "error" | "warning" | "info",
+              `Base image '${imagePart}' uses the mutable 'latest' tag. This makes builds non-deterministic.`,
+              this.help,
+              inst.line
+            )
+          );
         }
       }
     }

packages/core/test/image-ref.test.ts

@@ -0,0 +1,84 @@
+import { describe, expect, test } from "bun:test";
+
+import { parseDockerfile } from "../src/parsers/dockerfile-parser";
+import { collectStageAliases, parseImageRef } from "../src/parsers/image-ref";
+
+describe("parseImageRef", () => {
+  test("bare image name", () => {
+    const ref = parseImageRef("node");
+    expect(ref.name).toBe("node");
+    expect(ref.tag).toBeUndefined();
+    expect(ref.digest).toBeUndefined();
+  });
+
+  test("image with a version tag", () => {
+    const ref = parseImageRef("node:22.2.0-alpine");
+    expect(ref.tag).toBe("22.2.0-alpine");
+  });
+
+  test("image with the latest tag", () => {
+    const ref = parseImageRef("node:latest");
+    expect(ref.tag).toBe("latest");
+  });
+
+  test("registry with a port and no tag", () => {
+    const ref = parseImageRef("myregistry.example.com:5000/team/app");
+    expect(ref.registry).toBe("myregistry.example.com:5000");
+    expect(ref.name).toContain("team/app");
+    expect(ref.tag).toBeUndefined();
+  });
+
+  test("registry with a port and a tag", () => {
+    const ref = parseImageRef("myregistry.example.com:5000/team/app:1.2.3");
+    expect(ref.registry).toBe("myregistry.example.com:5000");
+    expect(ref.tag).toBe("1.2.3");
+  });
+
+  test("digest without a tag", () => {
+    const ref = parseImageRef("node@sha256:aaaa");
+    expect(ref.digest).toBe("sha256:aaaa");
+    expect(ref.tag).toBeUndefined();
+  });
+
+  test("tag and digest together", () => {
+    const ref = parseImageRef("node:22@sha256:aaaa");
+    expect(ref.tag).toBe("22");
+    expect(ref.digest).toBe("sha256:aaaa");
+  });
+
+  test("variable-driven image", () => {
+    const ref = parseImageRef(`\${NODE_IMAGE}`);
+    expect(ref.isVariable).toBe(true);
+  });
+
+  test("localhost registry", () => {
+    const ref = parseImageRef("localhost:5000/app");
+    expect(ref.registry).toBe("localhost:5000");
+    expect(ref.tag).toBeUndefined();
+  });
+});
+
+describe("collectStageAliases", () => {
+  test("collects an alias from a two-stage build", () => {
+    const instructions = parseDockerfile(`
+      FROM node:22-alpine AS build
+      FROM build
+    `);
+    expect(collectStageAliases(instructions)).toEqual(new Set(["build"]));
+  });
+
+  test("returns an empty set when no AS clause is present", () => {
+    const instructions = parseDockerfile(`
+      FROM node:22-alpine
+    `);
+    expect(collectStageAliases(instructions)).toEqual(new Set());
+  });
+
+  test("collects a lowercase as clause", () => {
+    const instructions = parseDockerfile(`
+      FROM node:22-alpine as build
+      FROM build
+    `);
+    expect(collectStageAliases(instructions)).toEqual(new Set(["build"]));
+  });
+});

packages/core/test/rules.test.ts

@@ -86,6 +86,30 @@ describe("Security Rules", () => {
     `);
     const diags3 = pinImageVersion.check(pinned, "Dockerfile");
     expect(diags3).toHaveLength(0);
+
+    const registryPortUntagged = parseDockerfile(`
+      FROM myregistry.example.com:5000/team/app
+    `);
+    expect(
+      pinImageVersion.check(registryPortUntagged, "Dockerfile")
+    ).toHaveLength(1);
+
+    const stageAlias = parseDockerfile(`
+      FROM node:22-alpine AS build
+      FROM build
+    `);
+    expect(pinImageVersion.check(stageAlias, "Dockerfile")).toHaveLength(0);
+
+    const argDriven = parseDockerfile(`
+      ARG NODE_IMAGE=node:22-alpine
+      FROM \${NODE_IMAGE}
+    `);
+    expect(pinImageVersion.check(argDriven, "Dockerfile")).toHaveLength(0);
+
+    const digestPinned = parseDockerfile(`
+      FROM node@sha256:aaaabbbbccccdddd
+    `);
+    expect(pinImageVersion.check(digestPinned, "Dockerfile")).toHaveLength(0);
   });
 
   test("no-secrets-in-env", () => {
@@ -287,6 +311,22 @@ describe("Image Size Rules", () => {
     `);
     const diags2 = preferSlimBase.check(slimBase, "Dockerfile");
     expect(diags2).toHaveLength(0);
+
+    const digestPinned = parseDockerfile(`
+      FROM node@sha256:aaaabbbbccccdddd
+    `);
+    expect(preferSlimBase.check(digestPinned, "Dockerfile")).toHaveLength(0);
+
+    const registryPort = parseDockerfile(`
+      FROM myregistry.example.com:5000/team/app
+    `);
+    expect(preferSlimBase.check(registryPort, "Dockerfile")).toHaveLength(0);
+
+    const stageAlias = parseDockerfile(`
+      FROM node:22-alpine AS build
+      FROM build
+    `);
+    expect(preferSlimBase.check(stageAlias, "Dockerfile")).toHaveLength(0);
   });
 
   test("clean-package-cache", () => {