Skip to content

Commit

Permalink
history: only add proxy vars to history if specified
Browse files Browse the repository at this point in the history
`Buildkit/Docker` only adds whitelisted variables for e.g `proxy
variables` to history only if user explicitly specifies them in
Dockerfile using `ARG`.

By default pre-whitelisted variables e.g `proxy variables` will be used
normally but will not be leaked into `docker/OCI` history of images.

A test for following behviour is added with this commit and similar test
can be verified against `Docker/Buildkit`

Signed-off-by: Aditya R <[email protected]>
  • Loading branch information
flouthoc committed Feb 15, 2022
1 parent 5d93f6a commit 33fc349
Show file tree
Hide file tree
Showing 2 changed files with 66 additions and 1 deletion.
33 changes: 32 additions & 1 deletion imagebuildah/stage_executor.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import (
"github.com/containers/buildah/pkg/parse"
"github.com/containers/buildah/pkg/rusage"
"github.com/containers/buildah/util"
config "github.com/containers/common/pkg/config"
cp "github.com/containers/image/v5/copy"
"github.com/containers/image/v5/docker/reference"
"github.com/containers/image/v5/manifest"
Expand Down Expand Up @@ -64,6 +65,7 @@ type StageExecutor struct {
output string
containerIDs []string
stage *imagebuilder.Stage
processedArgs []string // contains args which are processed and were added implicitly in Containerfile using ARG
}

// Preserve informs the stage executor that from this point on, it needs to
Expand Down Expand Up @@ -1228,6 +1230,11 @@ func (s *StageExecutor) getCreatedBy(node *parser.Node, addedContentSummary stri
}
switch strings.ToUpper(node.Value) {
case "ARG":
for _, variable := range strings.Fields(node.Original) {
if variable != "ARG" {
s.processedArgs = append(s.processedArgs, variable)
}
}
buildArgs := s.getBuildArgsKey()
return "/bin/sh -c #(nop) ARG " + buildArgs
case "RUN":
Expand Down Expand Up @@ -1271,7 +1278,31 @@ func (s *StageExecutor) getBuildArgsResolvedForRun() string {
if inImage {
envs = append(envs, fmt.Sprintf("%s=%s", key, configuredEnvs[key]))
} else {
envs = append(envs, fmt.Sprintf("%s=%s", key, value))
// by default everything must be added to history
// this is configured to false only for special cases
addToHistory := true

// following value is being assigned from build-args
// check if this key belongs to any of the predefined whitelisted args e.g Proxy Variables
// and if that arg is not manually set in Containerfile/Dockerfile
// then don't write its value to history.
// following behaviour ensures parity with docker/buildkit behviour
for _, variable := range config.ProxyEnv {
if key == variable {
// found in predefined args
// so don't add to history
// unless user did implicit `ARG <some-predefined-proxy-variable>`
addToHistory = false
for _, processedArg := range s.processedArgs {
if key == processedArg {
addToHistory = true
}
}
}
}
if addToHistory {
envs = append(envs, fmt.Sprintf("%s=%s", key, value))
}
}
}
}
Expand Down
34 changes: 34 additions & 0 deletions tests/history.bats
Original file line number Diff line number Diff line change
Expand Up @@ -108,3 +108,37 @@ function testconfighistory() {
run_buildah inspect --format '{{range .Docker.History}}{{println .CreatedBy}}{{end}}' runimg
expect_output --substring "/bin/sh -c uname -a"
}

@test "history should not contain whitelisted vars unless set in ARG" {
_prefetch busybox
ctxdir=${TESTDIR}/bud
mkdir -p $ctxdir
cat >$ctxdir/Dockerfile <<EOF
FROM busybox
RUN echo \$HTTP_PROXY
EOF

run_buildah build --signature-policy ${TESTSDIR}/policy.json -t test --build-arg HTTP_PROXY="helloworld" ${ctxdir}
expect_output --substring 'helloworld'
run_buildah inspect --format '{{range .Docker.History}}{{println .CreatedBy}}{{end}}' test
# history should not contain value for HTTP_PROXY since it was not in Containerfile
assert "$output" !~ 'HTTP_PROXY=helloworld'
assert "$output" !~ 'helloworld'
}

@test "history should contain whitelisted vars when set in ARG" {
_prefetch busybox
ctxdir=${TESTDIR}/bud
mkdir -p $ctxdir
cat >$ctxdir/Dockerfile <<EOF
FROM busybox
ARG HTTP_PROXY
RUN echo \$HTTP_PROXY
EOF

run_buildah build --signature-policy ${TESTSDIR}/policy.json -t test --build-arg HTTP_PROXY="helloworld" ${ctxdir}
expect_output --substring 'helloworld'
run_buildah inspect --format '{{range .Docker.History}}{{println .CreatedBy}}{{end}}' test
# history should not contain value for HTTP_PROXY since it was not in Containerfile
expect_output --substring 'HTTP_PROXY=helloworld'
}

0 comments on commit 33fc349

Please sign in to comment.