feat: pin images in COPY instructions#47
Conversation
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Adds support for discovering and pinning images referenced by COPY --from=... in Dockerfiles, alongside existing FROM pinning.
Changes:
- Extend Dockerfile parsing to return both
FROMandCOPY --fromimage references (with stage refs/index refs marked as skipped). - Add ref-resolution helper (
resolveRef) and parsing forCOPY --fromflags. - Add/extend tests to cover parsing and rewriting behavior for
COPY --from.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 7 comments.
| File | Description |
|---|---|
| internal/dockerfile/parse.go | Updates parsing to include COPY --from and ref-resolution factoring into resolveRef. |
| internal/dockerfile/parse_test.go | Adds tests asserting correct identification of external COPY --from images vs stage refs/index refs. |
| internal/dockerfile/rewrite_test.go | Adds tests validating digest insertion/update for COPY --from and end-to-end rewrite behavior. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // Parse reads a Dockerfile from r and returns all FROM and COPY --from instructions. | ||
| func Parse(r io.Reader) ([]FromInstruction, error) { |
| // Numeric index (e.g. --from=0) refers to a build stage, not an image. | ||
| if isNumeric(fromValue) { | ||
| inst.Skip = true | ||
| inst.SkipReason = "stage index reference" | ||
| return inst, true | ||
| } |
| // The image ref is the first token after FROM (node.Next) | ||
| if node.Next == nil { | ||
| inst.Skip = true | ||
| inst.SkipReason = "missing image reference" |
| // Numeric index (e.g. --from=0) refers to a build stage, not an image. | ||
| if isNumeric(fromValue) { | ||
| inst.Skip = true | ||
| inst.SkipReason = "stage index reference" |
| expanded, hasUnresolved := expandVars(rawRef, argDefaults) | ||
|
|
||
| if strings.ToLower(expanded) == "scratch" { | ||
| return expanded, "", "scratch image", true |
| refWithoutDigest = expanded[:atIdx] | ||
| } | ||
| if stageNames[strings.ToLower(refWithoutDigest)] { | ||
| return expanded, "", "stage reference", true |
| } | ||
|
|
||
| if hasUnresolved { | ||
| return expanded, "", "unresolved ARG variable", true |
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
| case "arg": | ||
| parseArgNode(node, argDefaults) |
There was a problem hiding this comment.
🟡 First-pass ARG collection breaks sequential scoping, causing incorrect expansion for later-defined ARGs
In the old single-pass code, parseArgNode was called sequentially alongside parseFromNode, so an ARG defined after a FROM would not be available for that FROM's variable expansion. The new two-pass code collects all ARGs in the first pass (parse.go:35-36) before any FROM/COPY instructions are processed in the second pass (parse.go:47-57). This means an ARG defined later in the file is now visible to earlier FROM instructions.
For example, in a Dockerfile like:
FROM python:${VERSION}
ARG VERSION=3.12
The old code correctly marked the FROM as "unresolved ARG variable" (skip), matching Docker's behavior (ARG after FROM is stage-scoped, not global). The new code expands it to python:3.12 and attempts to pin it — producing an incorrect rewrite for a Dockerfile that Docker wouldn't even expand.
The fix is to move case "arg": parseArgNode(...) from the first pass to the second pass, so ARGs are still collected sequentially. The first pass should only collect stage names.
Prompt for agents
The bug is in the Parse function in internal/dockerfile/parse.go. The first pass (lines 33-45) collects both ARG defaults and stage names. However, ARG defaults need to be collected sequentially (as they were in the old single-pass code) because Docker scoping means an ARG after a FROM is not visible to that FROM's variable expansion.
To fix: Remove the 'case "arg": parseArgNode(node, argDefaults)' from the first pass loop (lines 35-36) and add it to the second pass loop (lines 48-57). The first pass should ONLY collect stage names from FROM...AS instructions. The second pass should handle ARG, FROM, and COPY in order, preserving the old sequential ARG collection behavior.
The corrected second pass should look like:
for _, node := range result.AST.Children {
switch strings.ToLower(node.Value) {
case "arg":
parseArgNode(node, argDefaults)
case "from":
instructions = append(instructions, parseFromNode(node, argDefaults, stageNames))
case "copy":
if inst, ok := parseCopyFromNode(node, argDefaults, stageNames); ok {
instructions = append(instructions, inst)
}
}
}
Was this helpful? React with 👍 or 👎 to provide feedback.
Hi,
This PR adds support for pinning images that are referenced in
COPY --frominstructions.This PR was assisted by copilot using Claude Sonnet 4.6.