scorecard icon indicating copy to clipboard operation
scorecard copied to clipboard

Pinned dependencies check in Dockerfile does not handle build args

Open sudo-bmitch opened this issue 2 years ago • 11 comments

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.

sudo-bmitch avatar Nov 18 '23 15:11 sudo-bmitch

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

spencerschrock avatar Nov 21 '23 01:11 spencerschrock

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.

sudo-bmitch avatar Nov 21 '23 01:11 sudo-bmitch

@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.

spencerschrock avatar Jan 11 '24 18:01 spencerschrock

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?

laurentsimon avatar Jan 11 '24 18:01 laurentsimon

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.

tuminoid avatar Jun 17 '24 10:06 tuminoid

ARG for image versions also often used for reusability porposes in multistage build. Here example, tldr version:

ARG TAG=3.12.0-alpine3.17@sha256:fc34b07ec97a4f288bc17083d288374a803dd59800399c76b977016c9fe5b8f2
FROM python:${TAG} as builder
# ...

FROM python:${TAG} 

So currenty OSSF force us to or violate DRY and readability, or just ignore that warning

MaxymVlasov avatar Feb 14 '25 03:02 MaxymVlasov

Just logging that we (the Kokkos project) have been running into that same issue. In all our base images we do

ARG BASE=properly.pinned/image:tag@hash
FROM $BASE
# ...

and we sometimes overwrite that BASE argument in our automated testing via --build-arg BASE=... Dockerfile example here overwritten in our jenkinsfile here.

Is there any path towards resolving this issue? Perhaps some suppression directive we could use, a la # NO-SCORECARD-PINNED-DEPENDENCIES-NEXT-LINE(containerImage) or whatnot.

dalg24 avatar Feb 21 '25 19:02 dalg24

Back when this was originally filed I had this commit as a WIP which I just pushed to my fork to show, but the maintainers were split on the resolution.. We can add it to the next meeting agenda to re-poll.

https://github.com/spencerschrock/scorecard/commit/93a7c0da7cdc2112122d3771bd4bd3517da515a2

spencerschrock avatar Feb 21 '25 20:02 spencerschrock

Specifically in regards to multi-stage builds, can this be looked at as an issue that can be resolved by looking back to see if the source "FROM" statement was pinned? this takes away the debate on using build-args and focuses on what is indeed a widely used best practice.

EDIT: I didn't realise that this was actually already supported, thanks @spencerschrock for pointing me to this test case.

balteravishay avatar Feb 25 '25 16:02 balteravishay

Ah, so you propose to rework

ARG TAG=3.12.0-alpine3.17@sha256:fc34b07ec97a4f288bc17083d288374a803dd59800399c76b977016c9fe5b8f2
FROM python:${TAG} as builder
# ...

FROM python:${TAG} 

to

FROM python:3.12.0-alpine3.17@sha256:fc34b07ec97a4f288bc17083d288374a803dd59800399c76b977016c9fe5b8f2 AS python_base
FROM python_base as builder
# ...

FROM python_base

?

If that is a solution, don't see the reason why we can't do it (as we do not overwrite image version in CI, which is still the case for @dalg24)

MaxymVlasov avatar Feb 26 '25 05:02 MaxymVlasov

We discussed in the Scorecard meeting today and this seems like a reasonable improvement to introduce. If anyone is interested/available to work on this, @spencerschrock's draft commit is good reference material: https://github.com/spencerschrock/scorecard/commit/93a7c0da7cdc2112122d3771bd4bd3517da515a2

justaugustus avatar Mar 06 '25 21:03 justaugustus