ossf / scorecard

OpenSSF Scorecard - Security health metrics for Open Source
https://scorecard.dev
Apache License 2.0
4.24k stars 458 forks source link

Pinned dependencies check in Dockerfile does not handle build args #3684

Open sudo-bmitch opened 7 months ago

sudo-bmitch commented 7 months ago

Describe the bug The Pinned Dependency check is reporting a false positive for Dockerfile entries with a build arg and multi-stage build.

Reproduction steps Steps to reproduce the behavior:

With the following Dockerfile:

ARG REGISTRY=docker.io
ARG GO_VER=1.21-alpine@sha256:110b07af87238fbdc5f1df52b00927cf58ce3de358eeeb1854f10a8b5e5e1411

FROM ${REGISTRY}/library/golang:${GO_VER} as golang
#...

FROM golang as tool1
#...

FROM golang as tool2
#...

Each of the FROM lines are reported as non-pinned.

Expected behavior The described FROM lines should all indicate they are pinned.

Additional context The registry.example.org/repo:tag@digest syntax is a pinned dependency, the digest is used and tag will be ignored by runtimes pulling the image.

spencerschrock commented 7 months ago

Couple of questions:

  1. The values are just the defaults right? You could technically pass --build-arg GO_VER=latest and not pin? https://docs.docker.com/engine/reference/builder/#arg
  2. Do most people tend to use the ${foo} vs. $foo syntax? https://docs.docker.com/engine/reference/builder/#environment-replacement
sudo-bmitch commented 7 months ago

Couple of questions:

  1. The values are just the defaults right? You could technically pass --build-arg GO_VER=latest and not pin? https://docs.docker.com/engine/reference/builder/#arg

Yes, they are the default values. In my case, the build runs with the defaults, but it's also possible that injected values could be pinned and the default values are latest.

  1. Do most people tend to use the ${foo} vs. $foo syntax? https://docs.docker.com/engine/reference/builder/#environment-replacement

Both are valid/used, I'm not sure if either has more usage. When nested in a string, the ${foo} syntax is useful to denote the variable name from the rest of the string.

spencerschrock commented 5 months ago

@laurentsimon handled this partially in #710, and also brought up the --build-arg debate:

ARG is given via a command line. The ARG may be pinned, but dependabot is enable to update a pinned script command. So it seems like intended result. wdyt?

Laurent, do you still hold this view? To some extent, I think this falls under "maintainers aren't trying to game scorecard", but I'm not as familiar with Dockerfile semantics. In terms of how many people tend to build with defaults, vs having them but injecting build args anyway.

laurentsimon commented 5 months ago

I think so. It's impossible for scorecard to determine if the ARG was pinned or not. Maybe we need a specific probe for folks to turn off if they want to?

tuminoid commented 1 week ago

Pitching in with similar issue, with the difference that the default is pinned, but of course ARG could be overridden completely. This flexibility is allowing developers to override it but 99% of the cases it stays the default. IMO this kind of flexibility should not be punished.

     15 # Support FROM override
     16 ARG BUILD_IMAGE=docker.io/golang:1.22.4@sha256:969349b8121a56d51c74f4c273ab974c15b3a8ae246a5cffc1df7d28b66cf978
     17 ARG BASE_IMAGE=gcr.io/distroless/static:nonroot@sha256:9ecc53c269509f63c69a266168e4a687c7eb8c0cfd753bd8bfcaa4f58a90876f
     18 
     19 # Build the manager binary on golang image
     20 FROM $BUILD_IMAGE as builder

Line 20 gets penalized by scorecard right now.