feat: pin external images referenced by COPY --from=

[azu]

closes #53
closes #47

Pins the external image in a COPY --from= the same way FROM is already pinned, so an image copied from at build time gets the same supply-chain guarantee as a base image.

This re-does the stalled #47 on current main, keeping its approach and addressing the review comments left there. Credit to @jon4hz for the original, and @hairmare for triaging the review feedback.

Result

Input from #53:

# Dockerfile
FROM ubuntu:24.04
COPY --from=nginx:1.27 /etc/nginx /etc/nginx
COPY --from=0 /a /b

dockerfile-pin run -f Dockerfile --write, run against the real registry:

# Dockerfile
FROM ubuntu:24.04@sha256:33ceb71981b602c1a7443a53469e4dba065f7503eab3078a2d7a57a2ab987517
COPY --from=nginx:1.27@sha256:6784fb0834aa7dbbe12e3d7471e69c290df3e6ba810dc38b34ae33d3c1c05f7d /etc/nginx /etc/nginx
COPY --from=0 /a /b

What is pinned, and why

--from accepts an image, a build stage, or a named context, so each rule follows BuildKit's own reading of the flag rather than a guess.

Written as	Behavior	Reason
--from=image:tag	Pinned
--from=image:tag@sha256:…	Skipped; re-resolved with --update	matches FROM
--from=stage-name	Skipped	it is a stage, not an image
--from=0	Skipped	BuildKit classifies the value with strconv.Atoi first, so a numeric value always selects a stage by position
--from=scratch, --from=	Skipped	nothing to resolve
a bare name matching no stage	Pinned as an image	what BuildKit does, and what FROM ubuntu already does here
--from=image:$TAG	Skipped	see below
ONBUILD COPY --from=image:tag	Pinned	the trigger names a real image; stage names from this file do not apply to it
# escape= directive	Honored	it changes the line-continuation character
ADD --from=, RUN --mount=…,from=, --FROM=	Ignored	none is a COPY --from flag; docker build rejects the last two outright

Two cases are worth calling out because #47 handled them differently:

Variables are not expanded. CopyCommand.Expand covers --chown, --chmod and the paths, but not From — BuildKit reads the value verbatim, so a variable there fails the build with failed to parse stage name (moby/buildkit#2374). Expanding it would write a digest onto a line docker can never build, and check would then call that line OK. It is reported as skipped instead. FROM does expand, and still does — that asymmetry is pinned down by a test.

A stage declared below the COPY is still a stage. Using one is a build error about stage order, not an unpinned image, so all stage names are collected up front and there is nothing to rewrite. FROM deliberately does not get that treatment: BuildKit resolves each FROM against the stages declared above it only, so a name defined later really is an image there.

Review comments from #47

• ARG scoping. The two-pass parse in feat: pin images in COPY instructions #47 collected every ARG before any FROM, so an ARG below a FROM would expand that FROM's ref — which docker does not do. ARG defaults are still collected in file order; only stage names are gathered up front. TestParse_ARGScopeIsSequential covers it.
• ImageRef left empty on skipped COPY refs — now always populated, so check reports the ref it skipped.
• Skip reasons duplicated as raw strings — now exported constants, which the tests and e2e assertions use instead of string literals.
• Parse returning more than FROM — kept as one function. internal/dockerfile is not importable outside the module and both callers (run, check) want both kinds, so an option flag would be dead weight. The type and function docs say what is returned, and IsCopyFrom distinguishes them.

Codex review, round 1 (fixed in f1177ec)

Each was reproduced first and confirmed against the BuildKit sources this project parses with.

• ONBUILD COPY --from= was invisible — it parsed to zero instructions. The parser puts the wrapped instruction in node.Next.Children[0], not in result.AST.Children, so check said nothing and run never pinned it. It is now unwrapped, taking the ONBUILD node's line span and source text since the parser leaves the child's unset.
• A reference split by a \ continuation was silently not rewritten — the digest resolved, the file was left unchanged, and run still counted the image as pinned. The rewrite now runs over the instruction as the parser joins it, mapping each byte back to the line and column it came from, so the digest lands correctly however the instruction is broken up: a split value, a split flag name, the FROM equivalent, or an existing digest spanning the continuation.
• A digest turned an image into a stage reference — BuildKit matches the whole value against its stage names, so nginx@sha256:… finds no stage named nginx. Stripping the digest before the lookup marked it as a stage, so check never verified it and run --update could not refresh it. This was equally wrong for FROM, and predates this PR; both share classifyRef, so one change fixes both.

Codex review, round 2 (fixed in 380d593)

• ONBUILD sources were scoped to the wrong file. This corrects a call made in round 1: the conservative choice there was to pass this file's stage names, on the grounds that the semantics were unclear. They are not. dispatchOnbuild only records the trigger into this image's config; dispatchOnBuildTriggers executes it later, inside whichever build uses the image as its base, resolving it against that Dockerfile's dispatch states. So ONBUILD COPY --from=nginx in a file that also declares AS nginx was skipped as a stage reference and left unpinned, when it is an image to resolve. ONBUILD sources are now classified with no stage names. A stage index and scratch do not depend on one, so those are still not images.
• The rewrite assumed a backslash continuation. A Dockerfile can change that character with the # escape= parser directive, and under a backtick escape a reference split across the continuation kept the backtick in the rebuilt instruction, so it was never found and the image went unpinned. The parser's EscapeToken now travels on the instruction and the rewrite strips that character. The reverse case has its own test: under a backtick escape a backslash is an ordinary character, so a Windows-style path in the same instruction must survive untouched.

The RewriteFileReport backstop added in round 1 is what kept the second one from being silent — the reference came back as unrewritten instead of being counted as pinned.

Other fixes that fall out of the same code

• Line continuations. A reference written after a \ is now pinned rather than silently reported as pinned and left unchanged. This also fixes FROM \ + newline, which had the bug before COPY existed.
• Anchored replacement. A COPY rewrite is anchored at the --from= flag, so the same text elsewhere on the line never gets the digest by mistake — e.g. COPY --chown=node:20 --from=node:20 /app /app.
• Honest reporting. RewriteFileReport returns the instructions handed a digest that could not be rewritten; run warns about those and leaves them out of its pinned N image(s) count, so this class of bug cannot be silent again.

Tests

Enriched at four levels; go test ./... -race, go vet and golangci-lint all clean locally.

• Parser — a table over every form above, plus ARG scoping, the FROM/COPY stage-resolution asymmetry, ONBUILD scoping, line spans, and isStageIndex.
• Rewriter — AddCopyFromDigest including the anchoring guard, plus whole-file rewrites: the Feature request: support pinning external images referenced via COPY --from= #53 reproduction compared byte for byte, a multi-stage file, line continuations and split references, the # escape= directive, --update, repeated refs, and skipped instructions staying untouched even when handed a digest. The split and escape cases re-parse the output and assert the digest really landed.
• Commands — parseFile collecting the right refs (never a stage name or index), --ignore-images applying to COPY refs, applyDockerfile writing to disk, and the status/line/original text check reports for each form.
• End to end — the Feature request: support pinning external images referenced via COPY --from= #53 reproduction and the --update path through the full pipeline, check statuses, and a testdata/copy_from.Dockerfile round trip that writes the file out, asserts every pinnable ref carries a digest, and re-runs to confirm a second pass changes nothing. Resolution there is strict, so a ref that should have been skipped fails the test instead of quietly reaching the registry.

Every behavior worth relying on was checked by reverting it and confirming a test goes red: the anchoring, the continuation span, the ARG ordering, and each of the five Codex findings.

Also verified by hand against the real registry: the run above, check --syntax-only before and after, idempotence on a second run, a continued COPY \ with --chown, an ONBUILD COPY --from, a split reference, and a file combining # escape= with a backtick-split reference.

[azu]

@codex review

---

Generated by Claude Code

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

[devin-ai-integration]

✅ Devin Review: No Issues Found

Devin Review analyzed this PR and found no bugs or issues to report.

[azu]

Note on CI: the test and lint checks are not reported on the current head (f1177ec).

The push that added that commit did not create a CI run — GitHub's managed CodeQL ran on it and passed, but .github/workflows/ci.yml did not start. The only ci.yml run on this branch is the one from when the PR was opened, against 23fd33f, which passed. Workflow runs are not created for synchronize events from this automation's token, so no further push from me will start one either; a push or re-run by a human will.

The commit is verified locally with the same commands the workflow runs:

• go test ./... -race — all packages pass
• go build ./... — clean
• golangci-lint run ./... — 0 issues
• gofmt -l . / go vet ./... — clean

Flagging it so the missing checks aren't read as a failure. Happy to be told otherwise if you'd rather I do something else about it.

---

Generated by Claude Code

[azu]

Correction to my previous comment: CI did run on f1177ec, and passed — test and lint both green (run #130). All five checks on the head are now successful.

The run simply started late: pushed at 15:52, run created at 16:13. I checked at ~15:56 and again at ~16:00, saw no run, and concluded from that gap that the synchronize event had not created one. That inference was wrong — it was queueing delay, not a suppressed trigger. Nothing needs doing about CI here.

---

Generated by Claude Code

[azu]

@codex review

[jon4hz]

Hey @azu, thank you for continuing the work here! My PR slipped my table a bit, sorry for that