ADD --checksum= is silently ignored for Git sources (no verification performed)

[thelooter]

Description

ADD --checksum=<digest> <git-url> <dest> parses successfully, validates that the checksum
is a well-formed digest, and then verifies nothing. The digest is never compared against
anything for Git sources.

This is a silent no-op on a security control. A user who pins a Git dependency by checksum
gets no error, no warning, and no verification — the build succeeds exactly as if the flag
had been correct.

For contrast, BuildKit's Dockerfile frontend has never allowed this to pass quietly:

• frontend 1.9.0 explicitly made ADD --checksum=<SUM> <GITSRC> <DST> a hard error
• frontend 1.16.0 implemented it properly, where the checksum is the commit SHA and may
  be given as a prefix

Buildah does neither.

Steps to reproduce the issue

1. Write a Containerfile pinning a Git source with a deliberately wrong, all-zeros checksum:

FROM scratch
ADD --checksum=sha256:0000000000000000000000000000000000000000000000000000000000000000 https://github.com/octocat/Hello-World.git /src

2. Build it:

$ buildah build --no-cache -t t-cksum .

Describe the results you received

The build succeeds. An all-zeros digest that cannot possibly match anything is accepted
without complaint:

STEP 1/2: FROM scratch
STEP 2/2: ADD --checksum=sha256:0000000000000000000000000000000000000000000000000000000000000000 https://github.com/octocat/Hello-World.git /src
COMMIT t-cksum
--> 5dcd7b30019d
Successfully tagged localhost/t-cksum:latest
5dcd7b30019d30e9edaa82c8fce8b28e5ccef20132b139c61c937e0df6d83684

Describe the results you expected

Either a verification failure — the digest is obviously wrong — or a clear error stating that
--checksum is unsupported for Git sources (which is what BuildKit's frontend did from 1.9
until it implemented the feature properly in 1.16).

Note that a syntactically invalid checksum is still rejected, by digest.Parse at
add.go:590. That makes the no-op harder to notice: the flag appears to be doing something,
because malformed values fail while well-formed-but-wrong values pass.

Root cause

add.go parses the checksum inside a branch that covers both remote and Git sources, but
only ever passes the resulting digest to the HTTP path.

add.go:585-594 — the shared branch, entered for either source type:

if urlsource.IsRemote(src) || urlsource.IsGit(src) {
    pipeReader, pipeWriter := io.Pipe()
    var srcDigest digest.Digest
    if options.Checksum != "" {
        srcDigest, err = digest.Parse(options.Checksum)
        if err != nil {
            return fmt.Errorf("invalid checksum flag: %w", err)
        }
    }

add.go:596-624 — the Git branch. It clones via define.TempDirForURL and streams the tree
through copier.Get. srcDigest is never referenced.

add.go:629 — the non-Git branch, the only consumer:

return getURL(src, chownFiles, mountPoint, renameTarget, pipeWriter,
    options.Chmod, srcDigest, options.CertPath, options.InsecureSkipTLSVerify,
    options.Timestamp)

Verification itself lives in getURL at add.go:207-250 (digester set up at :208-209,
compared at :248-249), which the Git path never calls.

Confirming the full set of references:

$ grep -n srcDigest add.go
132:func getURL(..., srcDigest digest.Digest, ...) error {
208:	if srcDigest != "" {
209:		digester = srcDigest.Algorithm().Digester()
248:		if responseDigest := digester.Digest(); responseDigest != srcDigest {
249:			return fmt.Errorf("unexpected response digest for %q: %s, want %s", src, responseDigest, srcDigest)
588:			var srcDigest digest.Digest
590:				srcDigest, err = digest.Parse(options.Checksum)
629:						return getURL(src, chownFiles, mountPoint, renameTarget, pipeWriter, options.Chmod, srcDigest, ...)

How this happened

Ordering, not oversight in a single change:

• --checksum shipped HTTP-only by design in Implement ADD --checksum flag #5152 (merged 2023-11-17). The release note
  read "The ADD command now supports the --checksum flag for HTTP sources."
• Git sources were added ten months later in add: add support for git sources #5438 (merged 2024-09-09), into a code path
  that already accepted --checksum.

Nobody re-checked the interaction, and there is no test covering it —
tests/bud/add-checksum/ and tests/bud.bats:4338,4347,4354 cover HTTP only.

Note on the stated principle

This is the exact failure mode @nalind described when reviewing the imagebuilder side of
--checksum (openshift/imagebuilder#265, 2023-11-06):

Can you have the dockerclient.ClientExecutor implementation [...] complain when its
methods are passed new fields which it doesn't know what to do with, sort of like the
Run() method complains about flags that it doesn't implement? If we don't remember to do
that, it'll just start quietly ignoring flags that it used to produce errors over before,
and users of the imagebuilder CLI might come to think that those options actually work.
That is an oversight that has come back to bite us before.

The guard was applied at the imagebuilder layer; this instance is one layer down, in buildah.

Suggested fix

Two options, in increasing order of usefulness:

1. Minimal / safe. Reject the combination explicitly, matching BuildKit 1.9–1.15
   behaviour. In the Git branch, error if options.Checksum != "".

2. Full parity with BuildKit ≥ 1.16. Treat the checksum as a commit SHA, accepting a
   prefix, and verify it after checkout. Note this needs a different value format than
   digest.Parse accepts — BuildKit takes a bare hex commit SHA, not sha256:<hex> — so the
   parse at add.go:588-593 must branch on source type rather than being shared.

Either way, a regression test in tests/bud.bats alongside the existing
tests/bud/add-checksum/ fixtures.

Related

• Implement ADD --checksum flag #5152 — Implement ADD --checksum flag (merged 2023-11-17, HTTP-only)
• add: add support for git sources #5438 — add: add support for git sources (merged 2024-09-09)
• Intelligent cache of ADD --checksum download #5685 — Intelligent cache of ADD --checksum download (open)
• ADD with git repository does not reuse cache #6165 — ADD with git repository does not reuse cache (open)
• builder,dispatcher: Parse ADD --checksum openshift/imagebuilder#265 — the quoted review principle

Environment

$ buildah version
Version:         1.45.0
Go Version:      go1.26.5-X:nodwarf5
Image Spec:      1.1.1
Runtime Spec:    1.3.0
image Version:   5.41.0
Git Commit:      7ae7d5a4021b24d02a5c281badca8c4d8ebbf442

$ pacman -Q buildah
buildah 1:1.45.0-1

Rootless, kernel 7.1.8-arch1-3.

Reproduced above on the released 1.45.0. The source analysis was done against main
@ b544ad829 (1.46.0-dev), where the same control flow is present — so this is not fixed in
main either.