From 006385fa605ee0c394ed9b754141560daa1d1e8b Mon Sep 17 00:00:00 2001 From: breken-ai <312387581+breken-ai@users.noreply.github.com> Date: Fri, 25 Sep 2026 18:20:39 -0700 Subject: [PATCH] bake: do not leak target overrides into inherited entries Targets that inherit from the same parent share the parent's output and secret entries. The push override set the "push" attribute on those shared entries, and the secret source override changed the shared secret, so "--set app.push=true" also pushed every sibling target and "--set app.secret.token=env=X" also changed the secret of siblings. Copy the entries before changing them. Assisted-By: Claude Signed-off-by: breken-ai <312387581+breken-ai@users.noreply.github.com> --- bake/bake.go | 39 +++++++++++++++++++++++++++------------ bake/bake_test.go | 45 +++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 72 insertions(+), 12 deletions(-) diff --git a/bake/bake.go b/bake/bake.go index d166f3b384ef..3a6eade0c320 100644 --- a/bake/bake.go +++ b/bake/bake.go @@ -1394,7 +1394,7 @@ func (t *Target) updateSecret(id, value string) error { return errors.Errorf("invalid format for secret, expecting secret.=") } - for _, s := range t.Secrets { + for i, s := range t.Secrets { if s.ID != id { continue } @@ -1407,8 +1407,11 @@ func (t *Target) updateSecret(id, value string) error { return errors.Errorf("secret override id %q does not match declared secret %q", next.ID, id) } - s.Env = next.Env - s.FilePath = next.FilePath + // t.Secrets can share entries with other targets through inherits, + // so the entry is replaced instead of changed in place. + secrets := slices.Clone(t.Secrets) + secrets[i] = &buildflags.Secret{ID: s.ID, FilePath: next.FilePath, Env: next.Env} + t.Secrets = secrets return nil } @@ -1887,28 +1890,29 @@ func removeDupesStr(s []string) []string { } func setPushOverride(outputs []*buildflags.ExportEntry, push bool) []*buildflags.ExportEntry { + // outputs can be shared with other targets through inherits, so they + // are copied before being changed. if !push { // Disable push for any relevant export types - for i := 0; i < len(outputs); { - output := outputs[i] + res := make([]*buildflags.ExportEntry, 0, len(outputs)) + for _, output := range outputs { switch output.Type { case "registry": // Filter out registry output type - outputs[i], outputs[len(outputs)-1] = outputs[len(outputs)-1], outputs[i] - outputs = outputs[:len(outputs)-1] continue case "image": // Override push attribute - output.Attrs["push"] = "false" + output = withExportAttr(output, "push", "false") } - i++ + res = append(res, output) } - return outputs + return res } // Force push to be enabled setPush := true - for _, output := range outputs { + outputs = slices.Clone(outputs) + for i, output := range outputs { if output.Type != "docker" { // If there is an output type that is not docker, don't set "push" setPush = false @@ -1916,7 +1920,7 @@ func setPushOverride(outputs []*buildflags.ExportEntry, push bool) []*buildflags // Set push attribute for image if output.Type == "image" { - output.Attrs["push"] = "true" + outputs[i] = withExportAttr(output, "push", "true") } } @@ -1932,6 +1936,17 @@ func setPushOverride(outputs []*buildflags.ExportEntry, push bool) []*buildflags return outputs } +// withExportAttr returns a copy of e with the attribute key set to value. +func withExportAttr(e *buildflags.ExportEntry, key, value string) *buildflags.ExportEntry { + out := *e + out.Attrs = maps.Clone(e.Attrs) + if out.Attrs == nil { + out.Attrs = map[string]string{} + } + out.Attrs[key] = value + return &out +} + func setLoadOverride(outputs []*buildflags.ExportEntry, load bool) []*buildflags.ExportEntry { if !load { return outputs diff --git a/bake/bake_test.go b/bake/bake_test.go index 97c2c0b2b8af..585a77fa8de4 100644 --- a/bake/bake_test.go +++ b/bake/bake_test.go @@ -483,6 +483,51 @@ func TestPushOverride(t *testing.T) { require.Equal(t, 1, len(m["bar"].Outputs)) require.Equal(t, []string{"type=image,push=true"}, stringify(m["bar"].Outputs)) }) + + t.Run("inherited outputs", func(t *testing.T) { + fp := File{ + Name: "docker-bake.hcl", + Data: []byte( + `target "_common" { + output = ["type=registry", "type=image,name=foo"] + } + target "api" { + inherits = ["_common"] + } + target "app" { + inherits = ["_common"] + }`), + } + m, _, err := ReadTargets(context.TODO(), []File{fp}, []string{"api", "app"}, []string{"app.push=true"}, nil, nil, &EntitlementConf{}) + require.NoError(t, err) + require.Equal(t, []string{"type=image,name=foo", "type=registry"}, stringify(m["api"].Outputs)) + require.Equal(t, []string{"type=image,name=foo,push=true", "type=registry"}, stringify(m["app"].Outputs)) + + m, _, err = ReadTargets(context.TODO(), []File{fp}, []string{"api", "app"}, []string{"app.push=false"}, nil, nil, &EntitlementConf{}) + require.NoError(t, err) + require.Equal(t, []string{"type=image,name=foo", "type=registry"}, stringify(m["api"].Outputs)) + require.Equal(t, []string{"type=image,name=foo,push=false"}, stringify(m["app"].Outputs)) + }) +} + +func TestSecretSourceOverrideInherited(t *testing.T) { + fp := File{ + Name: "docker-bake.hcl", + Data: []byte( + `target "_common" { + secret = ["id=token,env=COMMON_TOKEN"] + } + target "api" { + inherits = ["_common"] + } + target "app" { + inherits = ["_common"] + }`), + } + m, _, err := ReadTargets(context.TODO(), []File{fp}, []string{"api", "app"}, []string{"app.secret.token=env=APP_TOKEN"}, nil, nil, &EntitlementConf{}) + require.NoError(t, err) + require.Equal(t, []string{"id=token,env=COMMON_TOKEN"}, stringify(m["api"].Secrets)) + require.Equal(t, []string{"id=token,env=APP_TOKEN"}, stringify(m["app"].Secrets)) } func TestLoadOverride(t *testing.T) {