Merge pull request #31584 from dave-tucker/ignore-default-build-args

Exclude “default” build-args from image history

builder/dockerfile/builder.go

@@ -38,6 +38,8 @@ var validCommitCommands = map[string]bool{
 }
 
 // BuiltinAllowedBuildArgs is list of built-in allowed build args
+// these args are considered transparent and are excluded from the image history.
+// Filtering from history is implemented in dispatchers.go
 var BuiltinAllowedBuildArgs = map[string]bool{
 	"HTTP_PROXY":  true,
 	"http_proxy":  true,

builder/dockerfile/dispatchers.go

@@ -411,6 +411,26 @@ func run(b *Builder, args []string, attributes map[string]bool, original string)
 	// have the build-time env vars in it (if any) so that future cache look-ups
 	// properly match it.
 	b.runConfig.Env = env
+
+	// remove BuiltinAllowedBuildArgs (see: builder.go)  from the saveCmd
+	// these args are transparent so resulting image should be the same regardless of the value
+	if len(cmdBuildEnv) > 0 {
+		saveCmd = config.Cmd
+		tmpBuildEnv := make([]string, len(cmdBuildEnv))
+		copy(tmpBuildEnv, cmdBuildEnv)
+		for i, env := range tmpBuildEnv {
+			key := strings.SplitN(env, "=", 2)[0]
+			if _, ok := BuiltinAllowedBuildArgs[key]; ok {
+				// If an built-in arg is explicitly added in the Dockerfile, don't prune it
+				if _, ok := b.allowedBuildArgs[key]; !ok {
+					tmpBuildEnv = append(tmpBuildEnv[:i], tmpBuildEnv[i+1:]...)
+				}
+			}
+		}
+		sort.Strings(tmpBuildEnv)
+		tmpEnv := append([]string{fmt.Sprintf("|%d", len(tmpBuildEnv))}, tmpBuildEnv...)
+		saveCmd = strslice.StrSlice(append(tmpEnv, saveCmd...))
+	}
 	b.runConfig.Cmd = saveCmd
 	return b.commit(cID, cmd, "run")
 }

docs/reference/builder.md

@@ -1396,6 +1396,35 @@ To use these, simply pass them on the command line using the flag:
 --build-arg <varname>=<value>
 ```
 
+By default, these pre-defined variables are excluded from the output of
+`docker history`. Excluding them reduces the risk of accidentally leaking
+sensitive authentication information in an `HTTP_PROXY` variable.
+
+For example, consider building the following Dockerfile using
+`--build-arg HTTP_PROXY=http://user:pass@proxy.lon.example.com`
+
+``` Dockerfile
+FROM ubuntu
+RUN echo "Hello World"
+```
+
+In this case, the value of the `HTTP_PROXY` variable is not available in the
+`docker history` and is not cached. If you were to change location, and your
+proxy server changed to `http://user:pass@proxy.sfo.example.com`, a subsequent
+build does not result in a cache miss.
+
+If you need to override this behaviour then you may do so by adding an `ARG`
+statement in the Dockerfile as follows:
+
+``` Dockerfile
+FROM ubuntu
+ARG HTTP_PROXY
+RUN echo "Hello World"
+```
+
+When building this Dockerfile, the `HTTP_PROXY` is preserved in the
+`docker history`, and changing its value invalidates the build cache.
+
 ### Impact on build caching
 
 `ARG` variables are not persisted into the built image as `ENV` variables are.
@@ -1404,6 +1433,8 @@ Dockerfile defines an `ARG` variable whose value is different from a previous
 build, then a "cache miss" occurs upon its first usage, not its definition. In
 particular, all `RUN` instructions following an `ARG` instruction use the `ARG`
 variable implicitly (as an environment variable), thus can cause a cache miss.
+All predefined `ARG` variables are exempt from caching unless there is a
+matching `ARG` statement in the `Dockerfile`.
 
 For example, consider these two Dockerfile:
 

integration-cli/docker_cli_build_test.go

@@ -4290,6 +4290,36 @@ func (s *DockerSuite) TestBuildBuildTimeArgHistory(c *check.C) {
 	}
 }
 
+func (s *DockerSuite) TestBuildTimeArgHistoryExclusions(c *check.C) {
+	imgName := "bldargtest"
+	envKey := "foo"
+	envVal := "bar"
+	proxy := "HTTP_PROXY=http://user:password@proxy.example.com"
+	explicitProxyKey := "http_proxy"
+	explicitProxyVal := "http://user:password@someproxy.example.com"
+	dockerfile := fmt.Sprintf(`FROM busybox
+		ARG %s
+		ARG %s
+		RUN echo "Testing Build Args!"`, envKey, explicitProxyKey)
+	buildImage(imgName,
+		withBuildFlags("--build-arg", fmt.Sprintf("%s=%s", envKey, envVal),
+			"--build-arg", fmt.Sprintf("%s=%s", explicitProxyKey, explicitProxyVal),
+			"--build-arg", proxy),
+		withDockerfile(dockerfile),
+	).Assert(c, icmd.Success)
+
+	out, _ := dockerCmd(c, "history", "--no-trunc", imgName)
+	if strings.Contains(out, proxy) {
+		c.Fatalf("failed to exclude proxy settings from history!")
+	}
+	if !strings.Contains(out, fmt.Sprintf("%s=%s", envKey, envVal)) {
+		c.Fatalf("explicitly defined ARG %s is not in output", explicitProxyKey)
+	}
+	if !strings.Contains(out, fmt.Sprintf("%s=%s", envKey, envVal)) {
+		c.Fatalf("missing build arguments from output")
+	}
+}
+
 func (s *DockerSuite) TestBuildBuildTimeArgCacheHit(c *check.C) {
 	imgName := "bldargtest"
 	envKey := "foo"