From a1229221ff2861c4b8e461d6cccf1fbd5e485840 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 2 Aug 2026 10:11:33 +0000 Subject: [PATCH 1/4] feat(rules): add five reproducibility rules for unpinned references The reproducibility rules only looked at a devcontainer.json's "image", "features", and "customizations", so the same moving reference went unreported wherever else it is written: a Dockerfile-based or Compose-based configuration escaped image pinning entirely, and a Feature's own dependencies were never checked. - no-dockerfile-image-latest and pin-dockerfile-image-digest read the Dockerfile named by "build.dockerfile" (or the legacy "dockerFile") and judge each FROM. A reference to an earlier stage, "scratch", and one containing a variable are left out: none names an image the configuration pins. - no-compose-image-latest reads the "image" of the Compose service the dev container runs in. A service that builds its own image, an image written as a variable, and a service no declared file defines are left out. - pin-depends-on-version checks a Feature's "dependsOn", where an unpinned reference installs a moving dependency into every project using the Feature, with no way for those projects to pin it. - pin-feature-exact-version requires a full "major.minor.patch", since the "major" and "major.minor" tags are reassigned on release. It stands to pin-feature-version as pin-image-digest stands to no-image-latest. A file another configuration file names is read through the directory the linted file was discovered in, so a path leading outside that boundary reports nothing rather than reaching for it. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01Y7ohBnPuPSezRvwrkzaRaA --- README.md | 2 +- go.mod | 2 +- rules/dockerfile.go | 88 +++++++++++ rules/no_compose_image_latest.go | 180 ++++++++++++++++++++++ rules/no_compose_image_latest_test.go | 134 ++++++++++++++++ rules/no_dockerfile_image_latest.go | 92 +++++++++++ rules/no_dockerfile_image_latest_test.go | 139 +++++++++++++++++ rules/pin_depends_on_version.go | 74 +++++++++ rules/pin_depends_on_version_test.go | 46 ++++++ rules/pin_dockerfile_image_digest.go | 84 ++++++++++ rules/pin_dockerfile_image_digest_test.go | 77 +++++++++ rules/pin_feature_exact_version.go | 89 +++++++++++ rules/pin_feature_exact_version_test.go | 91 +++++++++++ rules/pin_feature_version.go | 47 +----- rules/rules.go | 5 + rules/util.go | 97 ++++++++++++ 16 files changed, 1205 insertions(+), 42 deletions(-) create mode 100644 rules/dockerfile.go create mode 100644 rules/no_compose_image_latest.go create mode 100644 rules/no_compose_image_latest_test.go create mode 100644 rules/no_dockerfile_image_latest.go create mode 100644 rules/no_dockerfile_image_latest_test.go create mode 100644 rules/pin_depends_on_version.go create mode 100644 rules/pin_depends_on_version_test.go create mode 100644 rules/pin_dockerfile_image_digest.go create mode 100644 rules/pin_dockerfile_image_digest_test.go create mode 100644 rules/pin_feature_exact_version.go create mode 100644 rules/pin_feature_exact_version_test.go diff --git a/README.md b/README.md index 4e2f8ae..a378aec 100644 --- a/README.md +++ b/README.md @@ -155,7 +155,7 @@ runs without configuration; the rest are `off` until you enable them: | --- | --- | --- | | [`correctness`](https://bare-devcontainer.github.io/decolint/rules/#correctness) | `error` | 13 | | [`security`](https://bare-devcontainer.github.io/decolint/rules/#security) | `off` | 11 | -| [`reproducibility`](https://bare-devcontainer.github.io/decolint/rules/#reproducibility) | `off` | 4 | +| [`reproducibility`](https://bare-devcontainer.github.io/decolint/rules/#reproducibility) | `off` | 9 | | [`style`](https://bare-devcontainer.github.io/decolint/rules/#style) | `off` | 2 | diff --git a/go.mod b/go.mod index 91d6c2b..1475b0f 100644 --- a/go.mod +++ b/go.mod @@ -11,6 +11,7 @@ require ( github.com/opencontainers/image-spec v1.1.1 github.com/spf13/pflag v1.0.10 github.com/tailscale/hujson v0.0.0-20260302212456-ecc657c15afd + go.yaml.in/yaml/v3 v3.0.4 golang.org/x/sys v0.47.0 golang.org/x/term v0.45.0 oras.land/oras-go/v2 v2.6.2 @@ -200,7 +201,6 @@ require ( go.opentelemetry.io/otel/sdk/metric v1.44.0 // indirect go.opentelemetry.io/otel/trace v1.44.0 // indirect go.uber.org/automaxprocs v1.5.3 // indirect - go.yaml.in/yaml/v3 v3.0.4 // indirect go.yaml.in/yaml/v4 v4.0.0-rc.4 // indirect gocloud.dev v0.45.0 // indirect golang.org/x/crypto v0.53.0 // indirect diff --git a/rules/dockerfile.go b/rules/dockerfile.go new file mode 100644 index 0000000..d3c490c --- /dev/null +++ b/rules/dockerfile.go @@ -0,0 +1,88 @@ +package rules + +import ( + "bytes" + "strings" + + "github.com/bare-devcontainer/decolint/linter" + "github.com/moby/buildkit/frontend/dockerfile/instructions" + "github.com/moby/buildkit/frontend/dockerfile/parser" + "github.com/tailscale/hujson" +) + +// dockerfileRef locates the Dockerfile a devcontainer.json builds from: its path as written, and +// the byte offset of the value declaring it, which is where a rule reporting the Dockerfile's +// contents anchors its findings. +// +// The specification defines two mutually exclusive forms, the top-level "dockerFile" and the nested +// "build.dockerfile". The top-level one is preferred, as the reference implementation prefers it; +// the merge resolves the same two the same way, in feature's dockerfilePath. +func dockerfileRef(obj *hujson.Object) (path string, offset int, ok bool) { + if m := memberNamed(obj, "dockerFile"); m != nil { + if lit, isLit := m.Value.Value.(hujson.Literal); isLit && lit.Kind() == '"' { + return lit.String(), m.Value.StartOffset, true + } + } + if m := memberNamed(obj, "build"); m != nil { + if build, isObj := m.Value.Value.(*hujson.Object); isObj { + if d := memberNamed(build, "dockerfile"); d != nil { + if lit, isLit := d.Value.Value.(hujson.Literal); isLit && lit.Kind() == '"' { + return lit.String(), d.Value.StartOffset, true + } + } + } + } + return "", 0, false +} + +// dockerfileBaseImages returns the images the Dockerfile in src builds from, in the order its FROM +// instructions name them, keeping a repeated image once per FROM. It returns nothing for a +// Dockerfile that does not parse, leaving a rule with nothing to report rather than a guess. +// +// Only FROMs naming an image outside the build are returned. Left out are: +// - a reference to an earlier stage of the same Dockerfile, which is not an image at all; +// - "scratch", the empty base; +// - a reference containing a variable, whose value comes from "build.args" or an ARG default and +// is not the linter's to resolve. +func dockerfileBaseImages(src []byte) []string { + result, err := parser.Parse(bytes.NewReader(src)) + if err != nil { + return nil + } + // The linter argument reports the lint warnings buildkit itself defines; a nil one turns them + // off, which is what a caller reading the stages wants. + stages, _, err := instructions.Parse(result.AST, nil) + if err != nil { + return nil + } + + var images []string + stageNames := map[string]struct{}{} + for _, stage := range stages { + base := stage.BaseName + _, isStage := stageNames[strings.ToLower(base)] + if base != "" && base != "scratch" && !isStage && !strings.Contains(base, "$") { + images = append(images, base) + } + if stage.Name != "" { + stageNames[strings.ToLower(stage.Name)] = struct{}{} + } + } + return images +} + +// dockerfileBuildImages returns the images the Dockerfile that obj, a devcontainer.json, declares +// builds from (see [dockerfileBaseImages]), along with the Dockerfile's path as written and the +// offset to anchor findings at. ok is false when obj declares no Dockerfile, or when the file +// cannot be read (see [readConfigFile]). +func dockerfileBuildImages(dir linter.Dir, obj *hujson.Object) (images []string, path string, offset int, ok bool) { + path, offset, ok = dockerfileRef(obj) + if !ok { + return nil, "", 0, false + } + src, ok := readConfigFile(dir, path) + if !ok { + return nil, "", 0, false + } + return dockerfileBaseImages(src), path, offset, true +} diff --git a/rules/no_compose_image_latest.go b/rules/no_compose_image_latest.go new file mode 100644 index 0000000..9c31b1d --- /dev/null +++ b/rules/no_compose_image_latest.go @@ -0,0 +1,180 @@ +package rules + +import ( + "fmt" + "strings" + + "github.com/bare-devcontainer/decolint/linter" + "github.com/tailscale/hujson" + "go.yaml.in/yaml/v3" +) + +// NoComposeImageLatest reports the Compose service a devcontainer.json attaches to when it runs an +// image without an explicit tag or with the "latest" tag. It is [NoImageLatest] for the +// Compose-based form, where the container's image is named in a Compose file rather than in the +// "image" property. +var NoComposeImageLatest = &linter.Rule{ + ID: "no-compose-image-latest", + Description: `disallow a Compose service that runs an image without an explicit tag or with the "latest" tag`, + LongDescription: `The service named by "service" is the dev container: it is the one editors attach to and lifecycle +scripts run in. Its "image:" is therefore the environment the project works in, and an entry with no tag, +or with "latest", pulls whatever the publisher last released — a container that changes from one +"docker compose up" to the next while the repository stays the same.`, + References: []string{ + `https://containers.dev/implementors/spec/#docker-compose-based`, + `https://containers.dev/implementors/json_reference/#compose-specific`, + }, + Category: linter.CategoryReproducibility, + FileTypes: []linter.FileType{linter.Devcontainer}, + Paths: []string{""}, + Example: linter.Example{ + Bad: linter.Snippet{ + Files: []linter.ExampleFile{ + {Path: `devcontainer.json`, Content: `{ + "name": "api", + "dockerComposeFile": "docker-compose.yml", + "service": "app", + "workspaceFolder": "/workspace" +} +`}, + {Path: `docker-compose.yml`, Content: `services: + app: + image: mcr.microsoft.com/devcontainers/base:latest + command: sleep infinity +`}, + }, + }, + Good: linter.Snippet{ + Files: []linter.ExampleFile{ + {Path: `devcontainer.json`, Content: `{ + "name": "api", + "dockerComposeFile": "docker-compose.yml", + "service": "app", + "workspaceFolder": "/workspace" +} +`}, + {Path: `docker-compose.yml`, Content: `services: + app: + image: mcr.microsoft.com/devcontainers/base:ubuntu-24.04 + command: sleep infinity +`}, + }, + }, + Note: "Only the service the dev container runs in is checked. A service that builds its own\n" + + "image is left to the Dockerfile rules, and a service whose image is written as a\n" + + "`${...}` variable is not reported: the value is not in the configuration.", + }, + Check: checkNoComposeImageLatest, +} + +func checkNoComposeImageLatest(ctx *linter.Context, node *linter.Node) []linter.Finding { + obj, ok := node.Value.Value.(*hujson.Object) + if !ok { + return nil + } + paths, offset, ok := composeFilePaths(obj) + if !ok || len(paths) == 0 { + return nil + } + service, ok := stringMember(obj, "service") + if !ok { + return nil + } + image, ok := composeServiceImage(ctx.Dir, paths, service) + if !ok { + return nil + } + + tag, hasTag := refTag(image) + switch { + case !hasTag: + return []linter.Finding{{ + Message: fmt.Sprintf("compose service %q runs image %q, which has no explicit tag; pin a specific version", service, image), + Offset: offset, + }} + case tag == "latest": + return []linter.Finding{{ + Message: fmt.Sprintf("compose service %q runs image %q, which uses the \"latest\" tag; pin a specific version", service, image), + Offset: offset, + }} + } + return nil +} + +// composeFilePaths returns the Compose file paths obj declares, with the byte offset of the value +// declaring them. The property is a single path or an array of paths, later ones overriding earlier +// ones; the merge reads the same property in feature's composeFilePaths. +func composeFilePaths(obj *hujson.Object) (paths []string, offset int, ok bool) { + m := memberNamed(obj, "dockerComposeFile") + if m == nil { + return nil, 0, false + } + switch v := m.Value.Value.(type) { + case hujson.Literal: + if v.Kind() != '"' { + return nil, 0, false + } + paths = []string{v.String()} + case *hujson.Array: + for _, e := range v.Elements { + lit, isLit := e.Value.(hujson.Literal) + if !isLit || lit.Kind() != '"' { + return nil, 0, false + } + paths = append(paths, lit.String()) + } + default: + return nil, 0, false + } + return paths, m.Value.StartOffset, true +} + +// composeService is the part of a Compose service definition that says which image the service +// runs. +type composeService struct { + Image string `yaml:"image"` + Build any `yaml:"build"` +} + +// composeServiceImage returns the image the named Compose service runs, reading the files at paths +// in the order they are declared, each later one overriding the earlier ones as Compose merges them. +// +// ok is false whenever the answer is not in the files themselves, so that the caller reports +// nothing rather than reporting on a service it has only partly resolved: +// - a file that cannot be read (see [readConfigFile]) or does not parse; +// - a service none of the files defines, which "extends" or "include" may bring in from a file +// decolint does not follow; +// - a service that declares "build", whose "image" names what the build produces rather than what +// it starts from; +// - an image written with a "${...}" variable, whose value comes from the environment. +func composeServiceImage(dir linter.Dir, paths []string, service string) (string, bool) { + var image string + var found bool + for _, p := range paths { + src, ok := readConfigFile(dir, p) + if !ok { + return "", false + } + var doc struct { + Services map[string]composeService `yaml:"services"` + } + if err := yaml.Unmarshal(src, &doc); err != nil { + return "", false + } + svc, ok := doc.Services[service] + if !ok { + continue + } + found = true + if svc.Build != nil { + return "", false + } + if svc.Image != "" { + image = svc.Image + } + } + if !found || image == "" || strings.Contains(image, "${") { + return "", false + } + return image, true +} diff --git a/rules/no_compose_image_latest_test.go b/rules/no_compose_image_latest_test.go new file mode 100644 index 0000000..65715bc --- /dev/null +++ b/rules/no_compose_image_latest_test.go @@ -0,0 +1,134 @@ +package rules_test + +import ( + "testing" + "testing/fstest" + + "github.com/bare-devcontainer/decolint/linter" + "github.com/bare-devcontainer/decolint/rules" +) + +func TestNoComposeImageLatest(t *testing.T) { + t.Parallel() + + // Every case declares one Compose file, whose path starts at column 23, so the findings all + // anchor there. + const src = `{"dockerComposeFile": "docker-compose.yml", "service": "app"}` + issue := func(message string) []linter.Issue { + return []linter.Issue{{Path: "devcontainer.json", Line: 1, Col: 23, RuleID: "no-compose-image-latest", Message: message}} + } + + tests := []struct { + name string + compose string + want []linter.Issue + }{ + { + "untagged image", + "services:\n app:\n image: ubuntu\n", + issue(`compose service "app" runs image "ubuntu", which has no explicit tag; pin a specific version`), + }, + { + "latest image", + "services:\n app:\n image: ubuntu:latest\n", + issue(`compose service "app" runs image "ubuntu:latest", which uses the "latest" tag; pin a specific version`), + }, + {"pinned tag", "services:\n app:\n image: ubuntu:24.04\n", nil}, + {"pinned digest", "services:\n app:\n image: ubuntu@sha256:abc123\n", nil}, + { + // Only the service the dev container runs in is the container's image. + "another service is not the dev container", + "services:\n app:\n image: ubuntu:24.04\n db:\n image: postgres:latest\n", + nil, + }, + { + // A service that builds names in "image" what the build produces, not what it starts + // from; the Dockerfile rules cover the base image. + "a service that builds its own image reports nothing", + "services:\n app:\n build: .\n image: myapp:latest\n", + nil, + }, + { + "an image written as a variable is not resolved", + "services:\n app:\n image: ubuntu:${TAG}\n", + nil, + }, + {"a service defined in no file reports nothing", "services:\n web:\n image: ubuntu:latest\n", nil}, + {"a service without an image reports nothing", "services:\n app:\n command: sleep infinity\n", nil}, + {"a file that does not parse reports nothing", "services:\n app:\n image: [\n", nil}, + {"an empty file reports nothing", "", nil}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + dir := linter.Dir{FS: fstest.MapFS{"docker-compose.yml": {Data: []byte(tt.compose)}}} + assertIssuesInDir(t, rules.NoComposeImageLatest, linter.SeverityError, "devcontainer.json", linter.Devcontainer, src, dir, tt.want) + }) + } +} + +func TestNoComposeImageLatest_ComposeFileList(t *testing.T) { + t.Parallel() + + dir := linter.Dir{FS: fstest.MapFS{ + "docker-compose.yml": {Data: []byte("services:\n app:\n image: ubuntu:latest\n")}, + "docker-compose.override.yml": {Data: []byte("services:\n app:\n image: ubuntu:24.04\n")}, + "command.yml": {Data: []byte("services:\n app:\n command: sleep infinity\n")}, + }} + + tests := []struct { + name string + src string + want []linter.Issue + }{ + { + // Compose applies the files in order, so the last one to name an image wins. + "a later file overriding the image is the one read", + `{"dockerComposeFile": ["docker-compose.yml", "docker-compose.override.yml"], "service": "app"}`, + nil, + }, + { + "a later file leaving the image alone does not clear it", + `{"dockerComposeFile": ["docker-compose.yml", "command.yml"], "service": "app"}`, + []linter.Issue{{Path: "devcontainer.json", Line: 1, Col: 23, RuleID: "no-compose-image-latest", Message: `compose service "app" runs image "ubuntu:latest", which uses the "latest" tag; pin a specific version`}}, + }, + { + "an earlier file overridden by a later one is not reported", + `{"dockerComposeFile": ["docker-compose.override.yml", "docker-compose.yml"], "service": "app"}`, + []linter.Issue{{Path: "devcontainer.json", Line: 1, Col: 23, RuleID: "no-compose-image-latest", Message: `compose service "app" runs image "ubuntu:latest", which uses the "latest" tag; pin a specific version`}}, + }, + {"no dockerComposeFile property", `{"image": "ubuntu:latest", "service": "app"}`, nil}, + {"no service property", `{"dockerComposeFile": "docker-compose.yml"}`, nil}, + {"an empty file list reports nothing", `{"dockerComposeFile": [], "service": "app"}`, nil}, + {"a non-string entry reports nothing", `{"dockerComposeFile": [42], "service": "app"}`, nil}, + {"a non-string dockerComposeFile reports nothing", `{"dockerComposeFile": 42, "service": "app"}`, nil}, + {"an object dockerComposeFile reports nothing", `{"dockerComposeFile": {}, "service": "app"}`, nil}, + {"a non-string service reports nothing", `{"dockerComposeFile": "docker-compose.yml", "service": 42}`, nil}, + {"a document that is not an object reports nothing", `["docker-compose.yml"]`, nil}, + {"a missing Compose file reports nothing", `{"dockerComposeFile": "absent.yml", "service": "app"}`, nil}, + { + // Configuration under .devcontainer is read through a root confined to it, so a Compose + // file above that directory is not decolint's to open. + "a path leading outside the directory reports nothing", + `{"dockerComposeFile": "../docker-compose.yml", "service": "app"}`, + nil, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + assertIssuesInDir(t, rules.NoComposeImageLatest, linter.SeverityError, "devcontainer.json", linter.Devcontainer, tt.src, dir, tt.want) + }) + } + + t.Run("unreadable directory reports nothing", func(t *testing.T) { + t.Parallel() + src := `{"dockerComposeFile": "docker-compose.yml", "service": "app"}` + assertIssuesInDir(t, rules.NoComposeImageLatest, linter.SeverityError, "devcontainer.json", linter.Devcontainer, src, linter.Dir{FS: errFS{}}, nil) + }) + + t.Run("nil directory reports nothing", func(t *testing.T) { + t.Parallel() + assertIssues(t, rules.NoComposeImageLatest, linter.SeverityError, `{"dockerComposeFile": "docker-compose.yml", "service": "app"}`, nil) + }) +} diff --git a/rules/no_dockerfile_image_latest.go b/rules/no_dockerfile_image_latest.go new file mode 100644 index 0000000..d863132 --- /dev/null +++ b/rules/no_dockerfile_image_latest.go @@ -0,0 +1,92 @@ +package rules + +import ( + "fmt" + + "github.com/bare-devcontainer/decolint/linter" + "github.com/tailscale/hujson" +) + +// NoDockerfileImageLatest reports a FROM instruction of the Dockerfile a devcontainer.json builds +// from that names an image without an explicit tag or with the "latest" tag. It is [NoImageLatest] +// for the Dockerfile-based form, where the base image is named in the Dockerfile rather than in the +// "image" property. +var NoDockerfileImageLatest = &linter.Rule{ + ID: "no-dockerfile-image-latest", + Description: `disallow a Dockerfile that builds from an image without an explicit tag or with the "latest" tag`, + LongDescription: `A configuration that builds from a Dockerfile still starts from a base image, and pinning the +devcontainer.json says nothing about what that image is: a "FROM" with no tag, or with "latest", resolves +to whatever the publisher last released. The container then changes from one rebuild to the next while +every file in the repository stays the same. Name the version in the "FROM" the way you would in "image".`, + References: []string{ + `https://containers.dev/implementors/json_reference/#image-specific`, + `https://containers.dev/implementors/spec/#dockerfile-based`, + }, + Category: linter.CategoryReproducibility, + FileTypes: []linter.FileType{linter.Devcontainer}, + Paths: []string{""}, + Example: linter.Example{ + Bad: linter.Snippet{ + Files: []linter.ExampleFile{ + {Path: `devcontainer.json`, Content: `{ + "name": "api", + "build": { + "dockerfile": "Dockerfile" + } +} +`}, + {Path: `Dockerfile`, Content: `FROM mcr.microsoft.com/devcontainers/base:latest + +RUN apt-get update && apt-get install -y --no-install-recommends jq +`}, + }, + }, + Good: linter.Snippet{ + Files: []linter.ExampleFile{ + {Path: `devcontainer.json`, Content: `{ + "name": "api", + "build": { + "dockerfile": "Dockerfile" + } +} +`}, + {Path: `Dockerfile`, Content: `FROM mcr.microsoft.com/devcontainers/base:ubuntu-24.04 + +RUN apt-get update && apt-get install -y --no-install-recommends jq +`}, + }, + }, + Note: "The finding is reported at the property naming the Dockerfile, since that is what the\n" + + "devcontainer.json says about the image; the fix belongs in the Dockerfile.", + }, + Check: checkNoDockerfileImageLatest, +} + +func checkNoDockerfileImageLatest(ctx *linter.Context, node *linter.Node) []linter.Finding { + obj, ok := node.Value.Value.(*hujson.Object) + if !ok { + return nil + } + images, path, offset, ok := dockerfileBuildImages(ctx.Dir, obj) + if !ok { + return nil + } + + var findings []linter.Finding + for _, image := range images { + tag, hasTag := refTag(image) + switch { + case !hasTag: + findings = append(findings, linter.Finding{ + Message: fmt.Sprintf("Dockerfile %q builds from image %q, which has no explicit tag; pin a specific version", path, image), + Offset: offset, + }) + case tag == "latest": + findings = append(findings, linter.Finding{ + Message: fmt.Sprintf("Dockerfile %q builds from image %q, which uses the \"latest\" tag; pin a specific version", path, image), + Offset: offset, + }) + } + } + return findings +} diff --git a/rules/no_dockerfile_image_latest_test.go b/rules/no_dockerfile_image_latest_test.go new file mode 100644 index 0000000..9bc51c7 --- /dev/null +++ b/rules/no_dockerfile_image_latest_test.go @@ -0,0 +1,139 @@ +package rules_test + +import ( + "testing" + "testing/fstest" + + "github.com/bare-devcontainer/decolint/linter" + "github.com/bare-devcontainer/decolint/rules" +) + +func TestNoDockerfileImageLatest(t *testing.T) { + t.Parallel() + + // Every case declares the Dockerfile at "build.dockerfile", whose value starts at column 26, so + // the findings all anchor there. + const src = `{"build": {"dockerfile": "Dockerfile"}}` + issue := func(message string) []linter.Issue { + return []linter.Issue{{Path: "devcontainer.json", Line: 1, Col: 26, RuleID: "no-dockerfile-image-latest", Message: message}} + } + + tests := []struct { + name string + dockerfile string + want []linter.Issue + }{ + { + "untagged base image", + "FROM ubuntu\n", + issue(`Dockerfile "Dockerfile" builds from image "ubuntu", which has no explicit tag; pin a specific version`), + }, + { + "latest base image", + "FROM ubuntu:latest\n", + issue(`Dockerfile "Dockerfile" builds from image "ubuntu:latest", which uses the "latest" tag; pin a specific version`), + }, + {"pinned tag", "FROM ubuntu:24.04\n", nil}, + {"pinned digest", "FROM ubuntu@sha256:abc123\n", nil}, + {"scratch is not an image", "FROM scratch\nCOPY app /app\n", nil}, + { + "a later stage building on an earlier one is not an image", + "FROM golang:1.24 AS builder\nRUN go build\n\nFROM builder AS final\n", + nil, + }, + { + "a stage name is matched case-insensitively", + "FROM golang:1.24 AS Builder\n\nFROM builder\n", + nil, + }, + { + "an image reached through a variable is not resolved", + "ARG VARIANT=24.04\nFROM ubuntu:${VARIANT}\n", + nil, + }, + { + "each unpinned stage is reported", + "FROM golang:latest AS builder\nRUN go build\n\nFROM ubuntu\nCOPY --from=builder /app /app\n", + []linter.Issue{ + {Path: "devcontainer.json", Line: 1, Col: 26, RuleID: "no-dockerfile-image-latest", Message: `Dockerfile "Dockerfile" builds from image "golang:latest", which uses the "latest" tag; pin a specific version`}, + {Path: "devcontainer.json", Line: 1, Col: 26, RuleID: "no-dockerfile-image-latest", Message: `Dockerfile "Dockerfile" builds from image "ubuntu", which has no explicit tag; pin a specific version`}, + }, + }, + { + "the same unpinned image in several stages is reported once", + "FROM ubuntu:latest AS a\n\nFROM ubuntu:latest AS b\n", + issue(`Dockerfile "Dockerfile" builds from image "ubuntu:latest", which uses the "latest" tag; pin a specific version`), + }, + {"a Dockerfile whose instructions do not parse reports nothing", "FROM\n", nil}, + {"a Dockerfile that does not tokenize reports nothing", "FROM ubuntu\nRUN < maxConfigFileBytes { + return nil, false + } + return data, true +} + +// featureRef is an OCI Feature reference, as written for a key of a devcontainer.json "features" or +// a Feature's "dependsOn", with the byte offset of that key. +type featureRef struct { + ref string + offset int +} + +// ociFeatureRefs returns the OCI Feature references the members of v are keyed by, for a v that is +// an object of them. It returns none for a value that is not one. +// +// The local path and tarball URI forms are left out: neither carries a version to pin. See +// [isLocalFeature] and [isTarballFeature]. +func ociFeatureRefs(v *hujson.Value) []featureRef { + obj, ok := v.Value.(*hujson.Object) + if !ok { + return nil + } + var refs []featureRef + for _, m := range obj.Members { + name, ok := m.Name.Value.(hujson.Literal) + if !ok || name.Kind() != '"' { + continue + } + ref := name.String() + if isLocalFeature(ref) || isTarballFeature(ref) { + continue + } + refs = append(refs, featureRef{ref: ref, offset: m.Name.StartOffset}) + } + return refs +} + +// isLocalFeature reports whether ref names a Feature by a relative path, which has no version tag +// to pin. +func isLocalFeature(ref string) bool { + return strings.HasPrefix(ref, "./") || strings.HasPrefix(ref, "../") +} + +// isTarballFeature reports whether ref names a Feature by a direct HTTP(S) URI to a tarball, which +// has no version tag to pin. +func isTarballFeature(ref string) bool { + return strings.HasPrefix(ref, "http://") || strings.HasPrefix(ref, "https://") +} + +// unpinnedFeatureVersion describes how ref fails to name a specific Feature version, or "" if it +// names one. The text completes a message that begins with the reference, e.g. +// `feature "ghcr.io/devcontainers/features/go" has no explicit version; ...`. +func unpinnedFeatureVersion(ref string) string { + tag, hasTag := refTag(ref) + switch { + case !hasTag: + return "has no explicit version; pin a specific version" + case tag == "latest": + return `uses the "latest" version; pin a specific version` + default: + return "" + } +} + // refTag extracts the tag from an OCI-style reference, e.g. a container image or Feature reference. // A reference pinned by digest (e.g. "ref@sha256:...") is treated as tagged. The colon in a // registry host with a port (e.g. "localhost:5000/img") is not a tag separator. From 58ed5021c12b1d296198939d8ab5b806768e1ae5 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 4 Aug 2026 21:22:50 +0000 Subject: [PATCH 2/4] fix(rules): correct the Dockerfile and Compose readers Three defects found in review, each confirmed end-to-end: - instructions.Parse panics on a nil buildkit linter, which a Dockerfile reaches through a "# check=..." comment: the merge of that comment's config dereferences the receiver. Every rule reading such a Dockerfile reported "rule panicked" instead of its findings. Pass a linter whose Warn is nil, which reports nothing without being nil itself. - The Compose reader guarded only "${VAR}", so the bare "$VAR" form reached the tag check and was reported as an image with no tag. - "build.target" was ignored, so stages the build never reaches were reported. Only the target stage and what it builds on and copies from are read now, and with no target the last stage, as "docker build" does. The Compose reader also stops at "extends" and "include", which can define or override a service from a file it does not read: it now reports only what compose-go's full resolution (see feature's loadComposeService) would report too. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01Y7ohBnPuPSezRvwrkzaRaA --- rules/dockerfile.go | 132 +++++++++++++++++++--- rules/no_compose_image_latest.go | 40 ++++--- rules/no_compose_image_latest_test.go | 18 +++ rules/no_dockerfile_image_latest_test.go | 92 ++++++++++++++- rules/pin_dockerfile_image_digest_test.go | 2 +- rules/pin_feature_exact_version.go | 4 +- 6 files changed, 253 insertions(+), 35 deletions(-) diff --git a/rules/dockerfile.go b/rules/dockerfile.go index d3c490c..36d172d 100644 --- a/rules/dockerfile.go +++ b/rules/dockerfile.go @@ -2,10 +2,12 @@ package rules import ( "bytes" + "strconv" "strings" "github.com/bare-devcontainer/decolint/linter" "github.com/moby/buildkit/frontend/dockerfile/instructions" + dflinter "github.com/moby/buildkit/frontend/dockerfile/linter" "github.com/moby/buildkit/frontend/dockerfile/parser" "github.com/tailscale/hujson" ) @@ -35,42 +37,140 @@ func dockerfileRef(obj *hujson.Object) (path string, offset int, ok bool) { return "", 0, false } -// dockerfileBaseImages returns the images the Dockerfile in src builds from, in the order its FROM -// instructions name them, keeping a repeated image once per FROM. It returns nothing for a -// Dockerfile that does not parse, leaving a rule with nothing to report rather than a guess. +// buildTarget returns the stage "build.target" names, or "" when the configuration names none and +// the build produces the Dockerfile's last stage. +func buildTarget(obj *hujson.Object) string { + m := memberNamed(obj, "build") + if m == nil { + return "" + } + build, ok := m.Value.Value.(*hujson.Object) + if !ok { + return "" + } + target, _ := stringMember(build, "target") + return target +} + +// dockerfileBaseImages returns the images the Dockerfile in src builds from when target is built, +// in the order its FROM instructions name them, keeping a repeated image once per FROM. An empty +// target builds the last stage, as "docker build" does. // -// Only FROMs naming an image outside the build are returned. Left out are: +// Only the stages the build actually reaches are considered, since a stage nothing depends on is +// never built and its base image never pulled. Of those, only FROMs naming an image outside the +// build are returned. Left out are: // - a reference to an earlier stage of the same Dockerfile, which is not an image at all; // - "scratch", the empty base; // - a reference containing a variable, whose value comes from "build.args" or an ARG default and // is not the linter's to resolve. -func dockerfileBaseImages(src []byte) []string { +// +// It returns nothing for a Dockerfile that does not parse, or a target it does not define, leaving +// a rule with nothing to report rather than a guess. +func dockerfileBaseImages(src []byte, target string) []string { result, err := parser.Parse(bytes.NewReader(src)) if err != nil { return nil } - // The linter argument reports the lint warnings buildkit itself defines; a nil one turns them - // off, which is what a caller reading the stages wants. - stages, _, err := instructions.Parse(result.AST, nil) + // A Dockerfile may configure buildkit's own linter through a "# check=..." comment, which is + // merged onto the one passed here — a nil one is dereferenced, so pass a linter that reports + // nothing instead. Its zero Config leaves Warn nil, which is what turns the warnings off. + stages, _, err := instructions.Parse(result.AST, dflinter.New(&dflinter.Config{})) if err != nil { return nil } + built := builtStages(stages, target) var images []string - stageNames := map[string]struct{}{} - for _, stage := range stages { + for i, stage := range stages { + if !built[i] { + continue + } base := stage.BaseName - _, isStage := stageNames[strings.ToLower(base)] - if base != "" && base != "scratch" && !isStage && !strings.Contains(base, "$") { - images = append(images, base) + if base == "" || base == "scratch" || strings.Contains(base, "$") { + continue } - if stage.Name != "" { - stageNames[strings.ToLower(stage.Name)] = struct{}{} + if j, isStage := stageIndex(stages, base); isStage && j < i { + continue } + images = append(images, base) } return images } +// builtStages returns the indexes of the stages a build of target reaches: the target stage itself, +// the stages it builds on, and the ones it copies from, transitively. An empty target starts from +// the last stage, as "docker build" does. It returns nothing when target names no stage, since such +// a build does not run at all. +func builtStages(stages []instructions.Stage, target string) map[int]bool { + if len(stages) == 0 { + return nil + } + start := len(stages) - 1 + if target != "" { + i, ok := stageIndex(stages, target) + if !ok { + return nil + } + start = i + } + + built := map[int]bool{} + for queue := []int{start}; len(queue) > 0; queue = queue[1:] { + i := queue[0] + if built[i] { + continue + } + built[i] = true + for _, dep := range stageDeps(stages, i) { + queue = append(queue, dep) + } + } + return built +} + +// stageDeps returns the indexes of the stages the stage at i is built from: the one its FROM names, +// and the ones its instructions read through "--from", each only when it is a stage defined earlier +// rather than an image. +func stageDeps(stages []instructions.Stage, i int) []int { + var deps []int + add := func(ref string) { + if j, ok := stageIndex(stages, ref); ok && j < i { + deps = append(deps, j) + } + } + + add(stages[i].BaseName) + for _, cmd := range stages[i].Commands { + if copyCmd, ok := cmd.(*instructions.CopyCommand); ok { + add(copyCmd.From) + } + if runCmd, ok := cmd.(*instructions.RunCommand); ok { + for _, mount := range instructions.GetMounts(runCmd) { + add(mount.From) + } + } + } + return deps +} + +// stageIndex returns the index of the stage ref names, by its name or by its position, and reports +// whether it names one at all. Stage names are matched case-insensitively, as the Dockerfile parser +// matches them. +func stageIndex(stages []instructions.Stage, ref string) (int, bool) { + if ref == "" { + return 0, false + } + for i, stage := range stages { + if stage.Name != "" && strings.EqualFold(stage.Name, ref) { + return i, true + } + } + if i, err := strconv.Atoi(ref); err == nil && i >= 0 && i < len(stages) { + return i, true + } + return 0, false +} + // dockerfileBuildImages returns the images the Dockerfile that obj, a devcontainer.json, declares // builds from (see [dockerfileBaseImages]), along with the Dockerfile's path as written and the // offset to anchor findings at. ok is false when obj declares no Dockerfile, or when the file @@ -84,5 +184,5 @@ func dockerfileBuildImages(dir linter.Dir, obj *hujson.Object) (images []string, if !ok { return nil, "", 0, false } - return dockerfileBaseImages(src), path, offset, true + return dockerfileBaseImages(src, buildTarget(obj)), path, offset, true } diff --git a/rules/no_compose_image_latest.go b/rules/no_compose_image_latest.go index 9c31b1d..ca1efe6 100644 --- a/rules/no_compose_image_latest.go +++ b/rules/no_compose_image_latest.go @@ -130,23 +130,36 @@ func composeFilePaths(obj *hujson.Object) (paths []string, offset int, ok bool) } // composeService is the part of a Compose service definition that says which image the service -// runs. +// runs, or that the definition is not all in this file. type composeService struct { - Image string `yaml:"image"` - Build any `yaml:"build"` + Image string `yaml:"image"` + Build any `yaml:"build"` + Extends any `yaml:"extends"` +} + +// composeDoc is the part of a Compose file that defines the services, or pulls definitions in from +// files of its own. +type composeDoc struct { + Services map[string]composeService `yaml:"services"` + Include any `yaml:"include"` } // composeServiceImage returns the image the named Compose service runs, reading the files at paths // in the order they are declared, each later one overriding the earlier ones as Compose merges them. // -// ok is false whenever the answer is not in the files themselves, so that the caller reports -// nothing rather than reporting on a service it has only partly resolved: +// This reads the declared files and nothing else, which is narrower than the resolution the merge +// performs through compose-go (see feature's loadComposeService: it applies "extends" and "include" +// and interpolates variables, reading files outside the linted directory and an environment a rule +// does not have). ok is therefore false for everything this cannot settle from the files +// themselves, so that what it does report is what the full resolution would report too: +// // - a file that cannot be read (see [readConfigFile]) or does not parse; -// - a service none of the files defines, which "extends" or "include" may bring in from a file -// decolint does not follow; +// - a file declaring "include", or a service declaring "extends", either of which can define or +// override the service from a file not named here; +// - a service none of the files defines; // - a service that declares "build", whose "image" names what the build produces rather than what // it starts from; -// - an image written with a "${...}" variable, whose value comes from the environment. +// - an image written with a variable, whose value comes from the environment. func composeServiceImage(dir linter.Dir, paths []string, service string) (string, bool) { var image string var found bool @@ -155,10 +168,8 @@ func composeServiceImage(dir linter.Dir, paths []string, service string) (string if !ok { return "", false } - var doc struct { - Services map[string]composeService `yaml:"services"` - } - if err := yaml.Unmarshal(src, &doc); err != nil { + var doc composeDoc + if err := yaml.Unmarshal(src, &doc); err != nil || doc.Include != nil { return "", false } svc, ok := doc.Services[service] @@ -166,14 +177,15 @@ func composeServiceImage(dir linter.Dir, paths []string, service string) (string continue } found = true - if svc.Build != nil { + if svc.Build != nil || svc.Extends != nil { return "", false } if svc.Image != "" { image = svc.Image } } - if !found || image == "" || strings.Contains(image, "${") { + // Both "${VAR}" and the bare "$VAR" Compose accepts leave the image unresolved here. + if !found || image == "" || strings.Contains(image, "$") { return "", false } return image, true diff --git a/rules/no_compose_image_latest_test.go b/rules/no_compose_image_latest_test.go index 65715bc..c202263 100644 --- a/rules/no_compose_image_latest_test.go +++ b/rules/no_compose_image_latest_test.go @@ -53,6 +53,24 @@ func TestNoComposeImageLatest(t *testing.T) { "services:\n app:\n image: ubuntu:${TAG}\n", nil, }, + { + // Compose accepts the bare form as readily as "${VAR}". + "an image written as a bare variable is not resolved", + "services:\n app:\n image: $IMAGE\n", + nil, + }, + { + // The definition continues in a file this does not read, so what is here may not be the + // image the service ends up running. + "a service extending another reports nothing", + "services:\n app:\n extends:\n file: base.yml\n service: base\n image: ubuntu:latest\n", + nil, + }, + { + "a file pulling in others reports nothing", + "include:\n - other.yml\nservices:\n app:\n image: ubuntu:latest\n", + nil, + }, {"a service defined in no file reports nothing", "services:\n web:\n image: ubuntu:latest\n", nil}, {"a service without an image reports nothing", "services:\n app:\n command: sleep infinity\n", nil}, {"a file that does not parse reports nothing", "services:\n app:\n image: [\n", nil}, diff --git a/rules/no_dockerfile_image_latest_test.go b/rules/no_dockerfile_image_latest_test.go index 9bc51c7..f30c412 100644 --- a/rules/no_dockerfile_image_latest_test.go +++ b/rules/no_dockerfile_image_latest_test.go @@ -61,11 +61,46 @@ func TestNoDockerfileImageLatest(t *testing.T) { }, { "the same unpinned image in several stages is reported once", - "FROM ubuntu:latest AS a\n\nFROM ubuntu:latest AS b\n", + "FROM ubuntu:latest AS a\n\nFROM ubuntu:latest AS b\nCOPY --from=a /x /x\n", issue(`Dockerfile "Dockerfile" builds from image "ubuntu:latest", which uses the "latest" tag; pin a specific version`), }, {"a Dockerfile whose instructions do not parse reports nothing", "FROM\n", nil}, {"a Dockerfile that does not tokenize reports nothing", "FROM ubuntu\nRUN < Date: Tue, 4 Aug 2026 23:43:03 +0000 Subject: [PATCH 3/4] fix(rules): check every image a Dockerfile build pulls MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The Dockerfile rules read only each stage's FROM, so an image a "COPY --from" or a "RUN --mount=from" names went unreported. BuildKit pulls those too: a "--from" naming no stage becomes a dispatch state of its own, whose base image is resolved with the rest. A Dockerfile whose FROM is digest-pinned but which copies a tool from "ghcr.io/…/uv:latest" was reported clean. The stage lookup also matched more than BuildKit does. A FROM base is matched against the stages declared before it, as written, so "FROM Builder" after "AS builder" names an image; only a COPY's "--from" accepts a stage position, while "build.target" and a RUN --mount's "--from" are names BuildKit lower-cases first. Reporting the earlier stage's image for "FROM 0" named an image the build never pulls, and hid the one it does. Also state in the Compose rule's example what is left unchecked, rather than attributing it to the Dockerfile rules, which read a Compose service's "build" nowhere; and cover the two branches the suite reached by neither input: a Dockerfile of global ARGs alone, which parses to no stage at all, and a file over the size cap. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01Y7ohBnPuPSezRvwrkzaRaA --- rules/dockerfile.go | 169 ++++++++++++++++------ rules/no_compose_image_latest.go | 7 +- rules/no_dockerfile_image_latest.go | 24 +-- rules/no_dockerfile_image_latest_test.go | 81 ++++++++++- rules/pin_dockerfile_image_digest.go | 21 ++- rules/pin_dockerfile_image_digest_test.go | 15 ++ rules/util.go | 6 +- rules/util_test.go | 38 +++++ 8 files changed, 288 insertions(+), 73 deletions(-) create mode 100644 rules/util_test.go diff --git a/rules/dockerfile.go b/rules/dockerfile.go index 36d172d..c4acee5 100644 --- a/rules/dockerfile.go +++ b/rules/dockerfile.go @@ -52,21 +52,36 @@ func buildTarget(obj *hujson.Object) string { return target } -// dockerfileBaseImages returns the images the Dockerfile in src builds from when target is built, -// in the order its FROM instructions name them, keeping a repeated image once per FROM. An empty -// target builds the last stage, as "docker build" does. +// dockerfileImage is an image a build of a Dockerfile pulls, and the instruction form that reaches +// it, which is all a rule needs to name it in a finding. +type dockerfileImage struct { + ref string + // base distinguishes the image a stage's FROM builds on from one a COPY or a RUN --mount pulls + // through "--from". + base bool +} + +// verb describes how the Dockerfile reaches the image, for a message that continues with the image: +// `Dockerfile "Dockerfile" builds from image "ubuntu"`. +func (img dockerfileImage) verb() string { + if img.base { + return "builds from" + } + return "pulls" +} + +// dockerfilePulledImages returns the images a build of the Dockerfile in src pulls when target is +// built: the one each stage's FROM builds on, and the ones its COPY and RUN --mount instructions +// read through "--from". They come in the order the instructions name them, one entry per +// instruction. An empty target builds the last stage, as "docker build" does. // // Only the stages the build actually reaches are considered, since a stage nothing depends on is -// never built and its base image never pulled. Of those, only FROMs naming an image outside the -// build are returned. Left out are: -// - a reference to an earlier stage of the same Dockerfile, which is not an image at all; -// - "scratch", the empty base; -// - a reference containing a variable, whose value comes from "build.args" or an ARG default and -// is not the linter's to resolve. +// never built and its images never pulled. Within them, a reference naming another stage is left +// out, being no image at all, as are the references [isPulledImage] rejects. // // It returns nothing for a Dockerfile that does not parse, or a target it does not define, leaving // a rule with nothing to report rather than a guess. -func dockerfileBaseImages(src []byte, target string) []string { +func dockerfilePulledImages(src []byte, target string) []dockerfileImage { result, err := parser.Parse(bytes.NewReader(src)) if err != nil { return nil @@ -80,23 +95,33 @@ func dockerfileBaseImages(src []byte, target string) []string { } built := builtStages(stages, target) - var images []string - for i, stage := range stages { + var images []dockerfileImage + for i := range stages { if !built[i] { continue } - base := stage.BaseName - if base == "" || base == "scratch" || strings.Contains(base, "$") { - continue + if _, isStage := stageBase(stages, i); !isStage && isPulledImage(stages[i].BaseName) { + images = append(images, dockerfileImage{ref: stages[i].BaseName, base: true}) } - if j, isStage := stageIndex(stages, base); isStage && j < i { - continue + for _, from := range stageFroms(stages, i) { + if from.stage < 0 && isPulledImage(from.ref) { + images = append(images, dockerfileImage{ref: from.ref}) + } } - images = append(images, base) } return images } +// isPulledImage reports whether ref, a reference naming no stage, names an image the build pulls. +// Left out are: +// - the empty reference; +// - "scratch", the empty base, which BuildKit recognizes in that spelling alone; +// - a reference containing a variable, whose value comes from "build.args" or an ARG default and +// is not the linter's to resolve. +func isPulledImage(ref string) bool { + return ref != "" && ref != "scratch" && !strings.Contains(ref, "$") +} + // builtStages returns the indexes of the stages a build of target reaches: the target stage itself, // the stages it builds on, and the ones it copies from, transitively. An empty target starts from // the last stage, as "docker build" does. It returns nothing when target names no stage, since such @@ -107,7 +132,9 @@ func builtStages(stages []instructions.Stage, target string) map[int]bool { } start := len(stages) - 1 if target != "" { - i, ok := stageIndex(stages, target) + // A target names a stage and never a position, and BuildKit lower-cases it before the + // lookup, so "DEV" reaches the stage declared "AS dev". + i, ok := stageNamed(stages, strings.ToLower(target)) if !ok { return nil } @@ -128,54 +155,100 @@ func builtStages(stages []instructions.Stage, target string) map[int]bool { return built } -// stageDeps returns the indexes of the stages the stage at i is built from: the one its FROM names, -// and the ones its instructions read through "--from", each only when it is a stage defined earlier -// rather than an image. +// stageDeps returns the indexes of the stages the stage at i is built from: the one its FROM builds +// on, and the ones its instructions read through "--from", each only when it names a stage rather +// than an image. func stageDeps(stages []instructions.Stage, i int) []int { var deps []int - add := func(ref string) { - if j, ok := stageIndex(stages, ref); ok && j < i { - deps = append(deps, j) + if j, ok := stageBase(stages, i); ok { + deps = append(deps, j) + } + for _, from := range stageFroms(stages, i) { + if from.stage >= 0 { + deps = append(deps, from.stage) + } + } + return deps +} + +// stageFrom is a "--from" value of a COPY or a RUN --mount, resolved against the Dockerfile's +// stages: stage is the index of the stage it names, or -1 for a value naming an image, which the +// build pulls like a FROM base. +type stageFrom struct { + ref string + stage int +} + +// stageFroms returns the "--from" values the instructions of the stage at i read, in the order they +// are written. A value naming neither a stage nor an image — a COPY's position that is out of range, +// which fails the build — is left out. +// +// The two instructions resolve a value differently: a COPY's is a stage position when it parses as +// an integer, while a RUN --mount's is always a name. Both are matched against the stage names +// case-insensitively, and against every stage rather than only the earlier ones, since BuildKit +// resolves them once the whole Dockerfile is read. +func stageFroms(stages []instructions.Stage, i int) []stageFrom { + byName := func(ref string) stageFrom { + if j, ok := stageNamed(stages, strings.ToLower(ref)); ok { + return stageFrom{ref: ref, stage: j} } + return stageFrom{ref: ref, stage: -1} } - add(stages[i].BaseName) + var froms []stageFrom for _, cmd := range stages[i].Commands { - if copyCmd, ok := cmd.(*instructions.CopyCommand); ok { - add(copyCmd.From) - } - if runCmd, ok := cmd.(*instructions.RunCommand); ok { - for _, mount := range instructions.GetMounts(runCmd) { - add(mount.From) + switch c := cmd.(type) { + case *instructions.CopyCommand: + if c.From == "" { + continue + } + if j, err := strconv.Atoi(c.From); err == nil { + if j >= 0 && j < len(stages) { + froms = append(froms, stageFrom{ref: c.From, stage: j}) + } + continue + } + froms = append(froms, byName(c.From)) + case *instructions.RunCommand: + for _, mount := range instructions.GetMounts(c) { + if mount.From == "" { + continue + } + froms = append(froms, byName(mount.From)) } } } - return deps + return froms } -// stageIndex returns the index of the stage ref names, by its name or by its position, and reports -// whether it names one at all. Stage names are matched case-insensitively, as the Dockerfile parser -// matches them. -func stageIndex(stages []instructions.Stage, ref string) (int, bool) { - if ref == "" { - return 0, false - } +// stageBase returns the index of the stage the FROM of the stage at i builds on, and reports whether +// it names one rather than an image. BuildKit matches a base name against the stages declared before +// it only, and matches it as written against names the parser has already lower-cased — so +// "FROM Builder" after "AS builder" names an image, as its "repository name must be lowercase" +// failure shows. +func stageBase(stages []instructions.Stage, i int) (int, bool) { + return stageNamed(stages[:i], stages[i].BaseName) +} + +// stageNamed returns the index of the stage named ref and reports whether one is. A caller whose +// reference BuildKit lower-cases before the lookup passes it lower-cased; stage names need no +// folding, the parser having lower-cased them already. A name cannot begin with a digit, so no +// reference written as a position reaches a stage here. +func stageNamed(stages []instructions.Stage, ref string) (int, bool) { for i, stage := range stages { - if stage.Name != "" && strings.EqualFold(stage.Name, ref) { + // A stage left unnamed has no name to be reached by, whatever ref is. + if stage.Name != "" && stage.Name == ref { return i, true } } - if i, err := strconv.Atoi(ref); err == nil && i >= 0 && i < len(stages) { - return i, true - } return 0, false } -// dockerfileBuildImages returns the images the Dockerfile that obj, a devcontainer.json, declares -// builds from (see [dockerfileBaseImages]), along with the Dockerfile's path as written and the +// dockerfileBuildImages returns the images the build the Dockerfile that obj, a devcontainer.json, +// declares pulls (see [dockerfilePulledImages]), along with the Dockerfile's path as written and the // offset to anchor findings at. ok is false when obj declares no Dockerfile, or when the file // cannot be read (see [readConfigFile]). -func dockerfileBuildImages(dir linter.Dir, obj *hujson.Object) (images []string, path string, offset int, ok bool) { +func dockerfileBuildImages(dir linter.Dir, obj *hujson.Object) (images []dockerfileImage, path string, offset int, ok bool) { path, offset, ok = dockerfileRef(obj) if !ok { return nil, "", 0, false @@ -184,5 +257,5 @@ func dockerfileBuildImages(dir linter.Dir, obj *hujson.Object) (images []string, if !ok { return nil, "", 0, false } - return dockerfileBaseImages(src, buildTarget(obj)), path, offset, true + return dockerfilePulledImages(src, buildTarget(obj)), path, offset, true } diff --git a/rules/no_compose_image_latest.go b/rules/no_compose_image_latest.go index ca1efe6..7cfd25e 100644 --- a/rules/no_compose_image_latest.go +++ b/rules/no_compose_image_latest.go @@ -60,9 +60,10 @@ or with "latest", pulls whatever the publisher last released — a container tha `}, }, }, - Note: "Only the service the dev container runs in is checked. A service that builds its own\n" + - "image is left to the Dockerfile rules, and a service whose image is written as a\n" + - "`${...}` variable is not reported: the value is not in the configuration.", + Note: "Only the service the dev container runs in is checked, and only when it runs a\n" + + "published image: the base image of a service that builds its own image is not checked,\n" + + "and neither is an image written as a `${...}` variable, whose value is not in the\n" + + "configuration.", }, Check: checkNoComposeImageLatest, } diff --git a/rules/no_dockerfile_image_latest.go b/rules/no_dockerfile_image_latest.go index d863132..a43b7da 100644 --- a/rules/no_dockerfile_image_latest.go +++ b/rules/no_dockerfile_image_latest.go @@ -7,17 +7,19 @@ import ( "github.com/tailscale/hujson" ) -// NoDockerfileImageLatest reports a FROM instruction of the Dockerfile a devcontainer.json builds -// from that names an image without an explicit tag or with the "latest" tag. It is [NoImageLatest] -// for the Dockerfile-based form, where the base image is named in the Dockerfile rather than in the -// "image" property. +// NoDockerfileImageLatest reports an image the Dockerfile a devcontainer.json builds from pulls +// without an explicit tag or with the "latest" tag. It is [NoImageLatest] for the Dockerfile-based +// form, where the images are named in the Dockerfile rather than in the "image" property. var NoDockerfileImageLatest = &linter.Rule{ ID: "no-dockerfile-image-latest", - Description: `disallow a Dockerfile that builds from an image without an explicit tag or with the "latest" tag`, + Description: `disallow a Dockerfile that pulls an image without an explicit tag or with the "latest" tag`, LongDescription: `A configuration that builds from a Dockerfile still starts from a base image, and pinning the devcontainer.json says nothing about what that image is: a "FROM" with no tag, or with "latest", resolves to whatever the publisher last released. The container then changes from one rebuild to the next while -every file in the repository stays the same. Name the version in the "FROM" the way you would in "image".`, +every file in the repository stays the same. Name the version in the "FROM" the way you would in "image". + +A "COPY --from" or a "RUN --mount=from" naming an image pulls one just as a "FROM" does, and what it +brings into the container moves under an unpinned reference the same way, so those are named too.`, References: []string{ `https://containers.dev/implementors/json_reference/#image-specific`, `https://containers.dev/implementors/spec/#dockerfile-based`, @@ -56,7 +58,9 @@ RUN apt-get update && apt-get install -y --no-install-recommends jq `}, }, }, - Note: "The finding is reported at the property naming the Dockerfile, since that is what the\n" + + Note: "Every image a build of the Dockerfile pulls is checked: the base image of each stage the\n" + + "build reaches, and the images its `COPY --from` and `RUN --mount=from` instructions name.\n" + + "The finding is reported at the property naming the Dockerfile, since that is what the\n" + "devcontainer.json says about the image; the fix belongs in the Dockerfile.", }, Check: checkNoDockerfileImageLatest, @@ -74,16 +78,16 @@ func checkNoDockerfileImageLatest(ctx *linter.Context, node *linter.Node) []lint var findings []linter.Finding for _, image := range images { - tag, hasTag := refTag(image) + tag, hasTag := refTag(image.ref) switch { case !hasTag: findings = append(findings, linter.Finding{ - Message: fmt.Sprintf("Dockerfile %q builds from image %q, which has no explicit tag; pin a specific version", path, image), + Message: fmt.Sprintf("Dockerfile %q %s image %q, which has no explicit tag; pin a specific version", path, image.verb(), image.ref), Offset: offset, }) case tag == "latest": findings = append(findings, linter.Finding{ - Message: fmt.Sprintf("Dockerfile %q builds from image %q, which uses the \"latest\" tag; pin a specific version", path, image), + Message: fmt.Sprintf("Dockerfile %q %s image %q, which uses the \"latest\" tag; pin a specific version", path, image.verb(), image.ref), Offset: offset, }) } diff --git a/rules/no_dockerfile_image_latest_test.go b/rules/no_dockerfile_image_latest_test.go index f30c412..5eef590 100644 --- a/rules/no_dockerfile_image_latest_test.go +++ b/rules/no_dockerfile_image_latest_test.go @@ -42,10 +42,25 @@ func TestNoDockerfileImageLatest(t *testing.T) { nil, }, { - "a stage name is matched case-insensitively", + // The parser lower-cases every stage name, so a "FROM" reaches one only in lower case. + "a stage name is reached in the case the parser gives it", "FROM golang:1.24 AS Builder\n\nFROM builder\n", nil, }, + { + // BuildKit reads a base name it cannot match as an image, which is why "FROM BUILDER" + // fails with "repository name must be lowercase" rather than building on the stage. + "a base name in another case is an image", + "FROM golang:1.24 AS builder\n\nFROM BUILDER\n", + issue(`Dockerfile "Dockerfile" builds from image "BUILDER", which has no explicit tag; pin a specific version`), + }, + { + // A stage name cannot begin with a digit, so a "FROM" naming a position names an image; + // the stage at that position is not built and its own base never pulled. + "a base name written as a position is an image", + "FROM golang:latest\n\nFROM 0\n", + issue(`Dockerfile "Dockerfile" builds from image "0", which has no explicit tag; pin a specific version`), + }, { "an image reached through a variable is not resolved", "ARG VARIANT=24.04\nFROM ubuntu:${VARIANT}\n", @@ -95,12 +110,47 @@ func TestNoDockerfileImageLatest(t *testing.T) { "FROM golang:latest\n\nFROM ubuntu:24.04\nCOPY --from=0 /app /app\n", issue(`Dockerfile "Dockerfile" builds from image "golang:latest", which uses the "latest" tag; pin a specific version`), }, + { + "an image copied from is reported", + "FROM ubuntu:24.04\nCOPY --from=ghcr.io/astral-sh/uv:latest /uv /bin/\n", + issue(`Dockerfile "Dockerfile" pulls image "ghcr.io/astral-sh/uv:latest", which uses the "latest" tag; pin a specific version`), + }, + { + "an image mounted from is reported", + "FROM ubuntu:24.04\nRUN --mount=from=busybox,target=/b /b/bin/echo hi\n", + issue(`Dockerfile "Dockerfile" pulls image "busybox", which has no explicit tag; pin a specific version`), + }, + { + // A mount naming no source mounts the build context, and one naming no stage is matched + // by name alone, so neither reaches the stage at that position. + "a mount is not a position and needs no source", + "FROM golang:latest\n\nFROM ubuntu:24.04\nRUN --mount=target=/b --mount=from=0,target=/c true\n", + issue(`Dockerfile "Dockerfile" pulls image "0", which has no explicit tag; pin a specific version`), + }, + { + // Out of range, the position names no stage at all and the build fails on it, so there + // is no image to report either. + "a position no stage occupies is not an image", + "FROM ubuntu:24.04\nCOPY --from=9 /app /app\n", + nil, + }, + { + "an ordinary copy pulls nothing", + "FROM ubuntu:24.04\nCOPY app /app\nRUN --mount=type=cache,target=/c true\n", + nil, + }, + { + "a copy from a variable is not resolved", + "FROM ubuntu:24.04\nCOPY --from=$BUILDER /app /app\n", + nil, + }, { "a stage reached twice is read once", "FROM ubuntu:latest AS base\n\nFROM base AS mid\nCOPY --from=base /x /x\n", issue(`Dockerfile "Dockerfile" builds from image "ubuntu:latest", which uses the "latest" tag; pin a specific version`), }, {"a Dockerfile with no stage at all reports nothing", "# nothing to build here\n", nil}, + {"a Dockerfile of global ARGs alone reports nothing", "ARG VERSION=1.0\n", nil}, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { @@ -148,7 +198,19 @@ COPY --from=tools /go /go `{"build": {"dockerfile": "Dockerfile"}}`, toolsIssue, }, + { + "a target is matched case-insensitively", + `{"build": {"dockerfile": "Dockerfile", "target": "DEV"}}`, + toolsIssue, + }, {"a target naming no stage reports nothing", `{"build": {"dockerfile": "Dockerfile", "target": "absent"}}`, nil}, + { + // A target names a stage and never a position, and a stage name cannot begin with a + // digit, so the build fails rather than building the stage at that position. + "a target written as a position reports nothing", + `{"build": {"dockerfile": "Dockerfile", "target": "0"}}`, + nil, + }, {"a non-string target is no target", `{"build": {"dockerfile": "Dockerfile", "target": 42}}`, toolsIssue}, { // The legacy top-level property names the Dockerfile; a "build" beside it that is not @@ -166,6 +228,23 @@ COPY --from=tools /go /go } } +// TestNoDockerfileImageLatest_ForwardStageReference checks that a stage copied from before it is +// declared is read as the stage it names rather than as an image: BuildKit resolves a "--from" +// once the whole Dockerfile is parsed, so the order the two stages are written in does not matter. +func TestNoDockerfileImageLatest_ForwardStageReference(t *testing.T) { + t.Parallel() + + const dockerfile = `FROM ubuntu:24.04 AS dev +COPY --from=tools /go /go + +FROM golang:latest AS tools +` + dir := linter.Dir{FS: fstest.MapFS{"Dockerfile": {Data: []byte(dockerfile)}}} + src := `{"build": {"dockerfile": "Dockerfile", "target": "dev"}}` + want := []linter.Issue{{Path: "devcontainer.json", Line: 1, Col: 26, RuleID: "no-dockerfile-image-latest", Message: `Dockerfile "Dockerfile" builds from image "golang:latest", which uses the "latest" tag; pin a specific version`}} + assertIssuesInDir(t, rules.NoDockerfileImageLatest, linter.SeverityError, "devcontainer.json", linter.Devcontainer, src, dir, want) +} + func TestNoDockerfileImageLatest_DockerfileLocation(t *testing.T) { t.Parallel() diff --git a/rules/pin_dockerfile_image_digest.go b/rules/pin_dockerfile_image_digest.go index 936579f..58ee5cd 100644 --- a/rules/pin_dockerfile_image_digest.go +++ b/rules/pin_dockerfile_image_digest.go @@ -7,17 +7,20 @@ import ( "github.com/tailscale/hujson" ) -// PinDockerfileImageDigest reports a FROM instruction of the Dockerfile a devcontainer.json builds -// from that names an image without a content digest. It is [PinImageDigest] for the -// Dockerfile-based form, and stands to [NoDockerfileImageLatest] as that rule stands to -// [NoImageLatest]: any unpinned reference is reported, not only a missing or "latest" tag. +// PinDockerfileImageDigest reports an image the Dockerfile a devcontainer.json builds from pulls +// without a content digest. It is [PinImageDigest] for the Dockerfile-based form, and stands to +// [NoDockerfileImageLatest] as that rule stands to [NoImageLatest]: any unpinned reference is +// reported, not only a missing or "latest" tag. var PinDockerfileImageDigest = &linter.Rule{ ID: "pin-dockerfile-image-digest", - Description: `disallow a Dockerfile that builds from an image not pinned by content digest (e.g. "FROM image@sha256:...")`, + Description: `disallow a Dockerfile that pulls an image not pinned by content digest (e.g. "FROM image@sha256:...")`, LongDescription: `A "FROM" with a fixed tag still resolves through a mutable pointer: the publisher can move the tag to different bits, so two builds of the same Dockerfile can start from different images. Writing the digest ("FROM image:tag@sha256:...") names the content itself, and the build verifies what it pulled against it. -Keeping the tag alongside the digest leaves the reference readable.`, +Keeping the tag alongside the digest leaves the reference readable. + +An image a "COPY --from" or a "RUN --mount=from" names is pulled through the same mutable pointer, so +it takes a digest too.`, References: []string{ `https://containers.dev/implementors/spec/#dockerfile-based`, `https://github.com/opencontainers/image-spec/blob/main/descriptor.md#digests`, @@ -56,6 +59,8 @@ RUN apt-get update && apt-get install -y --no-install-recommends jq `}, }, }, + Note: "Every image a build of the Dockerfile pulls is checked: the base image of each stage the\n" + + "build reaches, and the images its `COPY --from` and `RUN --mount=from` instructions name.", }, Check: checkPinDockerfileImageDigest, } @@ -72,11 +77,11 @@ func checkPinDockerfileImageDigest(ctx *linter.Context, node *linter.Node) []lin var findings []linter.Finding for _, image := range images { - if digestSuffix.MatchString(image) { + if digestSuffix.MatchString(image.ref) { continue } findings = append(findings, linter.Finding{ - Message: fmt.Sprintf("Dockerfile %q builds from image %q, which is not pinned by digest; add an \"@sha256:...\" digest", path, image), + Message: fmt.Sprintf("Dockerfile %q %s image %q, which is not pinned by digest; add an \"@sha256:...\" digest", path, image.verb(), image.ref), Offset: offset, }) } diff --git a/rules/pin_dockerfile_image_digest_test.go b/rules/pin_dockerfile_image_digest_test.go index e54cab1..e1f8290 100644 --- a/rules/pin_dockerfile_image_digest_test.go +++ b/rules/pin_dockerfile_image_digest_test.go @@ -48,6 +48,21 @@ func TestPinDockerfileImageDigest(t *testing.T) { {Path: "devcontainer.json", Line: 1, Col: 26, RuleID: "pin-dockerfile-image-digest", Message: `Dockerfile "Dockerfile" builds from image "ubuntu:24.04", which is not pinned by digest; add an "@sha256:..." digest`}, }, }, + { + "an image copied from is reported", + "FROM ubuntu:24.04@sha256:abc123\nCOPY --from=ghcr.io/astral-sh/uv:0.9.7 /uv /bin/\n", + issue(`Dockerfile "Dockerfile" pulls image "ghcr.io/astral-sh/uv:0.9.7", which is not pinned by digest; add an "@sha256:..." digest`), + }, + { + "an image mounted from is reported", + "FROM ubuntu:24.04@sha256:abc123\nRUN --mount=from=busybox:1.37,target=/b /b/bin/echo hi\n", + issue(`Dockerfile "Dockerfile" pulls image "busybox:1.37", which is not pinned by digest; add an "@sha256:..." digest`), + }, + { + "an image copied from by digest is pinned", + "FROM ubuntu:24.04@sha256:abc123\nCOPY --from=busybox@sha256:def456 /bin/busybox /bin/\n", + nil, + }, {"a Dockerfile that does not parse reports nothing", "FROM\n", nil}, } for _, tt := range tests { diff --git a/rules/util.go b/rules/util.go index d08e919..d3ad4a2 100644 --- a/rules/util.go +++ b/rules/util.go @@ -238,9 +238,9 @@ const maxConfigFileBytes = 4 << 20 // 4 MB // // It reports false rather than an error because a rule reads such a file to say something about it, // and can say nothing when it is absent, unreadable, or too large (see maxConfigFileBytes). A path -// leading outside the directory is also not read: access is confined to the boundary the file was -// discovered through (see [discovery.VisitConfigs]), so a rule reports nothing on configuration -// that names a file decolint may not open. +// leading outside the directory is also not read: access is confined to the boundary discovery hands +// the rule the directory through, so a rule reports nothing on configuration that names a file +// decolint may not open. func readConfigFile(dir linter.Dir, name string) ([]byte, bool) { if dir.FS == nil { return nil, false diff --git a/rules/util_test.go b/rules/util_test.go new file mode 100644 index 0000000..32f92e8 --- /dev/null +++ b/rules/util_test.go @@ -0,0 +1,38 @@ +package rules + +import ( + "strings" + "testing" + "testing/fstest" + + "github.com/bare-devcontainer/decolint/linter" +) + +// TestReadConfigFile_SizeCap covers the boundary of the size cap: a file at it is read, and one over +// it is refused outright, so the rules reading a Dockerfile or a Compose file report nothing on it +// rather than on the part of it that fit. +func TestReadConfigFile_SizeCap(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + size int + want bool + }{ + {"at the cap", maxConfigFileBytes, true}, + {"over the cap", maxConfigFileBytes + 1, false}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + dir := linter.Dir{FS: fstest.MapFS{"Dockerfile": {Data: []byte(strings.Repeat("#", tt.size))}}} + src, ok := readConfigFile(dir, "Dockerfile") + if ok != tt.want { + t.Fatalf("readConfigFile of a %d-byte file: ok = %v, want %v", tt.size, ok, tt.want) + } + if ok && len(src) != tt.size { + t.Errorf("read %d bytes, want %d", len(src), tt.size) + } + }) + } +} From 825667cee6b09d6b55aea64fdba985cd7e3d0a2f Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 5 Aug 2026 00:09:04 +0000 Subject: [PATCH 4/4] fix(rules): reach the last stage sharing a name, as a build does MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Several stages may be declared under one name. BuildKit only warns about that and keeps one stage per name as it registers them in turn, so a reference reaches the last of them; the lookup here returned the first, which is a different stage whenever a name is repeated. It went wrong in both directions. A "COPY --from" naming a repeated stage read the wrong one, so a Dockerfile that copies a tool from an unpinned "AS tools" declared second was reported clean; a "build.target" or a FROM base naming one reported the image of a stage the build replaces. A differential run against BuildKit disagreed on 56 of 235 valid multi-stage Dockerfiles before, and on none after. The two Dockerfile rules also claimed in their examples that every image a build pulls is checked, while an image written with a "$" variable is deliberately left out — a gap that shows up in the "ARG VARIANT" form devcontainer templates use, and that was documented nowhere the reader can see. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01Y7ohBnPuPSezRvwrkzaRaA --- rules/dockerfile.go | 14 ++++--- rules/no_dockerfile_image_latest.go | 4 +- rules/no_dockerfile_image_latest_test.go | 50 +++++++++++++++++++++++- rules/pin_dockerfile_image_digest.go | 6 ++- 4 files changed, 63 insertions(+), 11 deletions(-) diff --git a/rules/dockerfile.go b/rules/dockerfile.go index c4acee5..51c06cf 100644 --- a/rules/dockerfile.go +++ b/rules/dockerfile.go @@ -230,14 +230,16 @@ func stageBase(stages []instructions.Stage, i int) (int, bool) { return stageNamed(stages[:i], stages[i].BaseName) } -// stageNamed returns the index of the stage named ref and reports whether one is. A caller whose -// reference BuildKit lower-cases before the lookup passes it lower-cased; stage names need no -// folding, the parser having lower-cased them already. A name cannot begin with a digit, so no -// reference written as a position reaches a stage here. +// stageNamed returns the index of the last stage named ref and reports whether one is. Several +// stages may share a name, which BuildKit only warns about, and it keeps one stage per name as it +// registers them in turn, so a reference reaches the last of them. A caller whose reference BuildKit +// lower-cases before the lookup passes it lower-cased; stage names need no folding, the parser +// having lower-cased them already. A name cannot begin with a digit, so no reference written as a +// position reaches a stage here. func stageNamed(stages []instructions.Stage, ref string) (int, bool) { - for i, stage := range stages { + for i := len(stages) - 1; i >= 0; i-- { // A stage left unnamed has no name to be reached by, whatever ref is. - if stage.Name != "" && stage.Name == ref { + if stages[i].Name != "" && stages[i].Name == ref { return i, true } } diff --git a/rules/no_dockerfile_image_latest.go b/rules/no_dockerfile_image_latest.go index a43b7da..768d3f9 100644 --- a/rules/no_dockerfile_image_latest.go +++ b/rules/no_dockerfile_image_latest.go @@ -58,8 +58,10 @@ RUN apt-get update && apt-get install -y --no-install-recommends jq `}, }, }, - Note: "Every image a build of the Dockerfile pulls is checked: the base image of each stage the\n" + + Note: "The images a build of the Dockerfile pulls are checked: the base image of each stage the\n" + "build reaches, and the images its `COPY --from` and `RUN --mount=from` instructions name.\n" + + "An image written with a `$` variable is not checked, since its value can come from\n" + + "`build.args`.\n" + "The finding is reported at the property naming the Dockerfile, since that is what the\n" + "devcontainer.json says about the image; the fix belongs in the Dockerfile.", }, diff --git a/rules/no_dockerfile_image_latest_test.go b/rules/no_dockerfile_image_latest_test.go index 5eef590..2bac41b 100644 --- a/rules/no_dockerfile_image_latest_test.go +++ b/rules/no_dockerfile_image_latest_test.go @@ -41,6 +41,12 @@ func TestNoDockerfileImageLatest(t *testing.T) { "FROM golang:1.24 AS builder\nRUN go build\n\nFROM builder AS final\n", nil, }, + { + // A base name reaching the last stage declared under it leaves the first one unbuilt. + "a base name reaches the last stage declared under it", + "FROM ubuntu:latest AS base\n\nFROM ubuntu:24.04 AS base\n\nFROM base AS final\n", + nil, + }, { // The parser lower-cases every stage name, so a "FROM" reaches one only in lower case. "a stage name is reached in the case the parser gives it", @@ -100,6 +106,11 @@ func TestNoDockerfileImageLatest(t *testing.T) { "FROM golang:latest AS builder\n\nFROM ubuntu:24.04\nCOPY --from=builder /app /app\n", issue(`Dockerfile "Dockerfile" builds from image "golang:latest", which uses the "latest" tag; pin a specific version`), }, + { + "a copy reaches the last stage declared under a shared name", + "FROM golang:1.24 AS tools\n\nFROM golang:latest AS tools\n\nFROM ubuntu:24.04\nCOPY --from=tools /go /go\n", + issue(`Dockerfile "Dockerfile" builds from image "golang:latest", which uses the "latest" tag; pin a specific version`), + }, { "a stage the last one mounts from is reported", "FROM golang:latest AS builder\n\nFROM ubuntu:24.04\nRUN --mount=from=builder,target=/app echo hi\n", @@ -228,9 +239,44 @@ COPY --from=tools /go /go } } +// TestNoDockerfileImageLatest_DuplicateStageName checks that a name several stages share reaches the +// last of them, both as a "build.target" and as a "--from" looked up across the whole Dockerfile. +func TestNoDockerfileImageLatest_DuplicateStageName(t *testing.T) { + t.Parallel() + + // Both cases build the "dev" stage, and "build.dockerfile" comes first, so its value starts at + // column 26. + const src = `{"build": {"dockerfile": "Dockerfile", "target": "dev"}}` + + tests := []struct { + name string + dockerfile string + want []linter.Issue + }{ + { + "a target reaches the last stage declared under its name", + "FROM golang:latest AS dev\n\nFROM ubuntu:24.04 AS dev\n", + nil, + }, + { + "a copy reaches the last stage declared under its name", + "FROM golang:1.24 AS tools\n\nFROM ubuntu:24.04 AS dev\nCOPY --from=tools /go /go\n\nFROM golang:latest AS tools\n", + []linter.Issue{{Path: "devcontainer.json", Line: 1, Col: 26, RuleID: "no-dockerfile-image-latest", Message: `Dockerfile "Dockerfile" builds from image "golang:latest", which uses the "latest" tag; pin a specific version`}}, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + dir := linter.Dir{FS: fstest.MapFS{"Dockerfile": {Data: []byte(tt.dockerfile)}}} + assertIssuesInDir(t, rules.NoDockerfileImageLatest, linter.SeverityError, "devcontainer.json", linter.Devcontainer, src, dir, tt.want) + }) + } +} + // TestNoDockerfileImageLatest_ForwardStageReference checks that a stage copied from before it is -// declared is read as the stage it names rather than as an image: BuildKit resolves a "--from" -// once the whole Dockerfile is parsed, so the order the two stages are written in does not matter. +// declared is read as the stage it names rather than as an image: BuildKit looks a "--from" up +// among every stage of the Dockerfile, wherever it is declared. Running such a build then fails, +// the copy naming a stage not yet built, but what the reference names is a stage all the same. func TestNoDockerfileImageLatest_ForwardStageReference(t *testing.T) { t.Parallel() diff --git a/rules/pin_dockerfile_image_digest.go b/rules/pin_dockerfile_image_digest.go index 58ee5cd..34e2a1d 100644 --- a/rules/pin_dockerfile_image_digest.go +++ b/rules/pin_dockerfile_image_digest.go @@ -59,8 +59,10 @@ RUN apt-get update && apt-get install -y --no-install-recommends jq `}, }, }, - Note: "Every image a build of the Dockerfile pulls is checked: the base image of each stage the\n" + - "build reaches, and the images its `COPY --from` and `RUN --mount=from` instructions name.", + Note: "The images a build of the Dockerfile pulls are checked: the base image of each stage the\n" + + "build reaches, and the images its `COPY --from` and `RUN --mount=from` instructions name.\n" + + "An image written with a `$` variable is not checked, since its value can come from\n" + + "`build.args`.", }, Check: checkPinDockerfileImageDigest, }