feat(rule): add rules for unpinned images in Compose and Dockerfile - #95
Open
nozaq wants to merge 7 commits into
Open
feat(rule): add rules for unpinned images in Compose and Dockerfile#95nozaq wants to merge 7 commits into
nozaq wants to merge 7 commits into
Conversation
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 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Y7ohBnPuPSezRvwrkzaRaA
nozaq
changed the title
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 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Y7ohBnPuPSezRvwrkzaRaA
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 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Y7ohBnPuPSezRvwrkzaRaA
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 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Y7ohBnPuPSezRvwrkzaRaA
pin-feature-exact-version declares both properties Feature references are written under, and a rule's paths are matched in every file type it applies to — so it also read a "features" member of a devcontainer-feature.json and a "dependsOn" member of a devcontainer.json, neither of which the specification defines. A Feature that happened to carry a "features" object was told to pin versions in it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Y7ohBnPuPSezRvwrkzaRaA
pin-feature-exact-version accepted a component with a leading zero, which semver forbids, so a reference no Feature can be published under passed as pinned to one. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Y7ohBnPuPSezRvwrkzaRaA
The stage lookup's backward scan is spelled with slices.Backward, and the dependency walk appends its slice in one call. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Y7ohBnPuPSezRvwrkzaRaA
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
This PR adds two new linting rules to detect unpinned container images in Compose-based and Dockerfile-based devcontainer configurations, complementing the existing
no-image-latestrule for direct image references.Key Changes
no-compose-image-latestrule: Reports when a Compose service (the one the dev container runs in) uses an image without an explicit tag or with the "latest" tag. Reads and parses referenced Compose files to extract the service's image configuration.no-dockerfile-image-latestrule: Reports when a Dockerfile that a devcontainer.json builds from has FROM instructions naming images without explicit tags or with "latest". Parses Dockerfile syntax to extract base image references.Shared utilities in
util.go:readConfigFile(): Safely reads referenced configuration files (Compose, Dockerfile) with size limits and path validationociFeatureRefs(): Extracts OCI Feature references from configuration objectsisLocalFeature()andisTarballFeature(): Helpers to identify Feature references that don't need version pinningunpinnedFeatureVersion(): Generates consistent messaging for unpinned Feature versionsdockerfile.go: New module providing Dockerfile parsing utilities:dockerfileRef(): Locates the Dockerfile path from devcontainer.json (handles both top-leveldockerFileand nestedbuild.dockerfile)dockerfileBaseImages(): Extracts base images from Dockerfile, filtering out stage references, scratch, and variable referencesdockerfileBuildImages(): Combines the above to get images with their source locationRefactored
pin_feature_version.go: Extracted common Feature reference handling logic into shared utilities inutil.goto reduce duplication and support the newpin-depends-on-versionandpin-feature-exact-versionrules.Comprehensive test coverage: Added test suites for all new rules covering edge cases like missing files, parse errors, variable references, and file merging behavior.
Implementation Details
Both new image rules follow the same pattern as
no-image-latest: they report at the configuration property that declares the image source (the Compose file path or Dockerfile path), making the finding location clear even though the fix belongs in the referenced file.Compose file handling respects the merge semantics: multiple files are read in order with later ones overriding earlier ones, matching Docker Compose's behavior.
Dockerfile parsing uses the buildkit parser to properly handle multi-stage builds, filtering out stage-to-stage references and variable substitutions that cannot be statically resolved.
File reading is bounded by
maxConfigFileBytes(4 MB) to prevent issues with truncated configuration analysis, and paths are validated to prevent directory traversal.https://claude.ai/code/session_01Y7ohBnPuPSezRvwrkzaRaA