fix(compose): strip and resolve pre-deploy hook secrets like service env

internal/machine/store/container.go

@@ -92,6 +92,9 @@ func normaliseContainerForStore(ctr *api.ServiceContainer) {
 	// Remove the environment variables to avoid leaking secrets.
 	ctr.Config.Env = nil
 	ctr.ServiceSpec.Container.Env = nil
+	if ctr.ServiceSpec.PreDeploy != nil {
+		ctr.ServiceSpec.PreDeploy.Env = nil
+	}
 
 	// Docker returns Mounts in a non-deterministic order so sort them.
 	slices.SortFunc(ctr.Mounts, func(a, b container.MountPoint) int {

internal/machine/store/container_test.go

@@ -0,0 +1,67 @@
+package store
+
+import (
+	"testing"
+
+	"github.com/docker/docker/api/types/container"
+	"github.com/psviderski/uncloud/pkg/api"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+// TestNormaliseContainerForStore_StripsEnv pins the behaviour that all fields carrying environment variables
+// are cleared before a container record is written to the replicated store, regardless of which part of the
+// spec they live in. See https://github.com/psviderski/uncloud/issues/422.
+func TestNormaliseContainerForStore_StripsEnv(t *testing.T) {
+	t.Parallel()
+
+	newContainer := func(preDeploy *api.PreDeployHook) api.ServiceContainer {
+		return api.ServiceContainer{
+			Container: api.Container{
+				InspectResponse: container.InspectResponse{
+					Config: &container.Config{
+						Env: []string{"CONTROL_TOKEN=control-value-aaa"},
+					},
+				},
+			},
+			ServiceSpec: api.ServiceSpec{
+				Container: api.ContainerSpec{
+					Env: api.EnvVars{"CONTROL_TOKEN": "control-value-aaa"},
+				},
+				PreDeploy: preDeploy,
+			},
+		}
+	}
+
+	t.Run("service with pre-deploy hook", func(t *testing.T) {
+		t.Parallel()
+
+		ctr := newContainer(&api.PreDeployHook{
+			Command: []string{"/bin/true"},
+			Env:     api.EnvVars{"HOOK_TOKEN": "hook-value-bbb"},
+		})
+
+		normaliseContainerForStore(&ctr)
+
+		assert.Nil(t, ctr.Config.Env, "Config.Env should be stripped")
+		assert.Nil(t, ctr.ServiceSpec.Container.Env, "ServiceSpec.Container.Env should be stripped")
+		require.NotNil(t, ctr.ServiceSpec.PreDeploy, "PreDeploy itself should be left in place")
+		assert.Nil(t, ctr.ServiceSpec.PreDeploy.Env, "ServiceSpec.PreDeploy.Env should be stripped")
+		// Non-env fields of the hook must survive the normalisation.
+		assert.Equal(t, []string{"/bin/true"}, ctr.ServiceSpec.PreDeploy.Command)
+	})
+
+	t.Run("service without pre-deploy hook", func(t *testing.T) {
+		t.Parallel()
+
+		ctr := newContainer(nil)
+
+		require.NotPanics(t, func() {
+			normaliseContainerForStore(&ctr)
+		})
+
+		assert.Nil(t, ctr.Config.Env)
+		assert.Nil(t, ctr.ServiceSpec.Container.Env)
+		assert.Nil(t, ctr.ServiceSpec.PreDeploy)
+	})
+}

pkg/client/compose/secret.go

@@ -89,47 +89,78 @@ func ResolveSecrets(ctx context.Context, project *types.Project) error {
 		return v, nil
 	}
 
-	// Resolve only secret references set as values for environment variables in enabled services.
 	for _, service := range project.Services {
-		for k, v := range service.Environment {
-			if v == nil {
-				continue
-			}
-			secretName, ok := secretRefName(*v)
-			if !ok {
-				continue
-			}
-			value, err := resolve(secretName)
-			if err != nil {
+		// Resolve secret references set as values for environment variables in the service.
+		if err := resolveEnvSecrets(service.Environment, resolve); err != nil {
+			return err
+		}
+
+		// Resolve secret references set as values for the pre-deploy hook's environment variables, if any.
+		if hook, ok := service.Extensions[PreDeployHookExtensionKey].(PreDeployHook); ok {
+			if err := resolveEnvSecrets(hook.Environment, resolve); err != nil {
 				return err
 			}
-			service.Environment[k] = &value
 		}
 	}
 
 	return nil
 }
 
-// HasCommandSecretRefs reports whether any service environment references a secret that is resolved by running
-// a command.
+// resolveEnvSecrets resolves 'secret://name' references found as values in env to actual secret values using
+// resolve, setting each referenced variable to the secret's value in place. Variables that are not secret
+// references are left untouched.
+func resolveEnvSecrets(env types.MappingWithEquals, resolve func(string) (string, error)) error {
+	for k, v := range env {
+		if v == nil {
+			continue
+		}
+		secretName, ok := secretRefName(*v)
+		if !ok {
+			continue
+		}
+		value, err := resolve(secretName)
+		if err != nil {
+			return err
+		}
+		env[k] = &value
+	}
+	return nil
+}
+
+// HasCommandSecretRefs reports whether any service environment, including a pre-deploy hook's environment,
+// references a secret that is resolved by running a command.
 func HasCommandSecretRefs(project *types.Project) bool {
 	for _, service := range project.Services {
-		for _, v := range service.Environment {
-			if v == nil {
-				continue
-			}
-			name, ok := secretRefName(*v)
-			if !ok {
-				continue
-			}
-			if secret, ok := project.Secrets[name]; ok && secret.Driver == secretExecDriver {
+		if envHasCommandSecretRef(service.Environment, project) {
+			return true
+		}
+		if hook, ok := service.Extensions[PreDeployHookExtensionKey].(PreDeployHook); ok {
+			if envHasCommandSecretRef(hook.Environment, project) {
 				return true
 			}
 		}
 	}
 	return false
 }
 
+// envHasCommandSecretRef reports whether env contains a 'secret://name' reference to a secret resolved
+// by running a command.
+func envHasCommandSecretRef(env types.MappingWithEquals, project *types.Project) bool {
+	for _, v := range env {
+		if v == nil {
+			continue
+		}
+		name, ok := secretRefName(*v)
+		if !ok {
+			continue
+		}
+		if secret, ok := project.Secrets[name]; ok && secret.Driver == secretExecDriver {
+			return true
+		}
+	}
+	return false
+}
+
 // secretValue resolves a secret to its value depending on its source: an 'exec' driver command,
 // an environment variable, or a file. A single trailing newline ('\n' or '\r\n') is stripped from the command
 // output as command-line tools commonly append one. All other whitespace is kept. Environment and file values are

pkg/client/compose/secret_test.go

@@ -328,6 +328,74 @@ secrets:
 	assert.Equal(t, "run\n", string(runs), "command should run exactly once across services and repeated resolutions")
 }
 
+// TestResolveSecrets_PreDeployHook pins the behaviour that 'secret://name' references in the pre-deploy hook's
+// environment are resolved just like references in the service's own environment.
+// See https://github.com/psviderski/uncloud/issues/422.
+func TestResolveSecrets_PreDeployHook(t *testing.T) {
+	t.Parallel()
+
+	content := `
+services:
+  foo:
+    image: foo
+    environment:
+      CONTROL_TOKEN: secret://control_token
+    x-pre_deploy:
+      command: ["./migrate.sh"]
+      environment:
+        HOOK_TOKEN: secret://hook_token
+        PLAIN: hello
+secrets:
+  control_token:
+    x-command: printf control-value
+  hook_token:
+    x-command: printf hook-value
+`
+	project := loadProject(t, content)
+	require.NoError(t, ResolveSecrets(context.Background(), project))
+
+	assert.Equal(t, env(map[string]string{"CONTROL_TOKEN": "control-value"}), project.Services["foo"].Environment)
+
+	hook, ok := project.Services["foo"].Extensions[PreDeployHookExtensionKey].(PreDeployHook)
+	require.True(t, ok, "x-pre_deploy extension not found")
+	assert.Equal(t, env(map[string]string{"HOOK_TOKEN": "hook-value", "PLAIN": "hello"}), hook.Environment)
+}
+
+// TestResolveSecrets_PreDeployHook_ResolvedOnce verifies that a secret referenced from both the service's
+// environment and its pre-deploy hook's environment is only resolved once.
+func TestResolveSecrets_PreDeployHook_ResolvedOnce(t *testing.T) {
+	t.Parallel()
+
+	counter := filepath.Join(t.TempDir(), "runs")
+	content := fmt.Sprintf(`
+services:
+  foo:
+    image: foo
+    environment:
+      TOKEN: secret://token
+    x-pre_deploy:
+      command: ["./migrate.sh"]
+      environment:
+        TOKEN: secret://token
+secrets:
+  token:
+    x-command: "sh -c 'echo run >> %s; printf abc'"
+`, counter)
+
+	project := loadProject(t, content)
+	require.NoError(t, ResolveSecrets(context.Background(), project))
+
+	assert.Equal(t, "abc", *project.Services["foo"].Environment["TOKEN"])
+	hook, ok := project.Services["foo"].Extensions[PreDeployHookExtensionKey].(PreDeployHook)
+	require.True(t, ok, "x-pre_deploy extension not found")
+	assert.Equal(t, "abc", *hook.Environment["TOKEN"])
+
+	runs, err := os.ReadFile(counter)
+	require.NoError(t, err)
+	assert.Equal(t, "run\n", string(runs),
+		"command should run exactly once across the service and hook environments")
+}
+
 func TestHasCommandSecretRefs(t *testing.T) {
 	t.Parallel()
 
@@ -387,6 +455,22 @@ services:
 `,
 			want: false,
 		},
+		{
+			name: "command secret referenced from pre-deploy hook",
+			content: `
+services:
+  foo:
+    image: foo
+    x-pre_deploy:
+      command: ["./migrate.sh"]
+      environment:
+        TOKEN: secret://token
+secrets:
+  token:
+    x-command: printf abc
+`,
+			want: true,
+		},
 	}
 
 	for _, tt := range tests {