Prevent secrets from leaking to Kubernetes API Server logs (#5305)

This commit is contained in:
Harri Avellan 2025-07-14 17:45:13 +03:00 committed by GitHub
parent b382287170
commit 5c00b9d74b
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
9 changed files with 369 additions and 15 deletions

View File

@ -233,6 +233,13 @@ func (e *kube) StartStep(ctx context.Context, step *types.Step, taskUUID string)
}
}
if needsStepSecret(step) {
err = startStepSecret(ctx, e, step)
if err != nil {
return err
}
}
log.Trace().Str("taskUUID", taskUUID).Msgf("starting step: %s", step.Name)
_, err = startPod(ctx, e, step, options)
return err
@ -398,6 +405,13 @@ func (e *kube) DestroyStep(ctx context.Context, step *types.Step, taskUUID strin
}
}
if needsStepSecret(step) {
err := stopStepSecret(ctx, e, step, defaultDeleteOptions)
if err != nil {
errs = append(errs, err)
}
}
err := stopPod(ctx, e, step, defaultDeleteOptions)
if err != nil {
errs = append(errs, err)

View File

@ -235,7 +235,15 @@ func podContainer(step *types.Step, podName, goos string, options BackendOptions
container.Command = step.Entrypoint
}
container.Env = mapToEnvVars(step.Environment)
stepSecret, err := stepSecretName(step)
if err != nil {
return container, err
}
// filter environment variables to non-secrets and secrets, refer secrets from step secrets
envs, secs := filterSecrets(step.Environment, step.SecretMapping)
envsFromSecrets := mapToEnvVarsFromStepSecrets(secs, stepSecret)
container.Env = append(mapToEnvVars(envs), envsFromSecrets...)
container.Resources, err = resourceRequirements(options.Resources)
if err != nil {
@ -254,6 +262,38 @@ func podContainer(step *types.Step, podName, goos string, options BackendOptions
return container, nil
}
func mapToEnvVarsFromStepSecrets(secs []string, stepSecretName string) []v1.EnvVar {
var ev []v1.EnvVar
for _, key := range secs {
ev = append(ev, v1.EnvVar{
Name: key,
ValueFrom: &v1.EnvVarSource{
SecretKeyRef: &v1.SecretKeySelector{
LocalObjectReference: v1.LocalObjectReference{
Name: stepSecretName,
},
Key: key,
},
},
})
}
return ev
}
func filterSecrets(environment, secrets map[string]string) (map[string]string, []string) {
ev := map[string]string{}
var secs []string
for k, v := range environment {
if _, found := secrets[k]; found {
secs = append(secs, k)
} else {
ev[k] = v
}
}
return ev, secs
}
func pvcVolumes(volumes []string) ([]v1.Volume, error) {
var vols []v1.Volume

View File

@ -177,6 +177,7 @@ func TestTinyPod(t *testing.T) {
pod, err := mkPod(&types.Step{
Name: "build-via-gradle",
Image: "gradle:8.4.0-jdk21",
UUID: "01he8bebctabr3kgk0qj36d2me-0",
WorkingDir: "/woodpecker/src",
Pull: false,
Privileged: false,
@ -415,6 +416,7 @@ func TestPodPrivilege(t *testing.T) {
return mkPod(&types.Step{
Name: "go-test",
Image: "golang:1.16",
UUID: "01he8bebctabr3kgk0qj36d2me-0",
Privileged: stepPrivileged,
}, &config{
Namespace: "woodpecker",
@ -525,6 +527,7 @@ func TestScratchPod(t *testing.T) {
pod, err := mkPod(&types.Step{
Name: "curl-google",
Image: "quay.io/curl/curl",
UUID: "01he8bebctabr3kgk0qj36d2me-0",
Entrypoint: []string{"/usr/bin/curl", "-v", "google.com"},
}, &config{
Namespace: "woodpecker",
@ -623,6 +626,7 @@ func TestSecrets(t *testing.T) {
pod, err := mkPod(&types.Step{
Name: "test-secrets",
Image: "alpine",
UUID: "01he8bebctabr3kgk0qj36d2me-0",
Environment: map[string]string{"CGO": "0"},
Volumes: []string{"workspace:/woodpecker/src"},
}, &config{
@ -657,3 +661,35 @@ func TestSecrets(t *testing.T) {
ja := jsonassert.New(t)
ja.Assertf(string(podJSON), expected)
}
func TestStepSecret(t *testing.T) {
const expected = `{
"metadata": {
"name": "wp-01he8bebctabr3kgk0qj36d2me-0-step-secret",
"namespace": "woodpecker",
"creationTimestamp": null
},
"type": "Opaque",
"stringData": {
"VERY_SECRET": "secret_value"
}
}`
secret, err := mkStepSecret(&types.Step{
UUID: "01he8bebctabr3kgk0qj36d2me-0",
Name: "go-test",
Image: "meltwater/drone-cache",
SecretMapping: map[string]string{
"VERY_SECRET": "secret_value",
},
}, &config{
Namespace: "woodpecker",
})
assert.NoError(t, err)
secretJSON, err := json.Marshal(secret)
assert.NoError(t, err)
ja := jsonassert.New(t)
ja.Assertf(string(secretJSON), expected)
}

View File

@ -308,3 +308,58 @@ func stopRegistrySecret(ctx context.Context, engine *kube, step *types.Step, del
}
return err
}
func needsStepSecret(step *types.Step) bool {
return len(step.SecretMapping) > 0
}
func startStepSecret(ctx context.Context, e *kube, step *types.Step) error {
secret, err := mkStepSecret(step, e.config)
if err != nil {
return err
}
log.Trace().Msgf("creating secret: %s", secret.Name)
_, err = e.client.CoreV1().Secrets(e.config.Namespace).Create(ctx, secret, meta_v1.CreateOptions{})
if err != nil {
return err
}
return nil
}
func mkStepSecret(step *types.Step, config *config) (*v1.Secret, error) {
name, err := stepSecretName(step)
if err != nil {
return nil, err
}
return &v1.Secret{
ObjectMeta: meta_v1.ObjectMeta{
Namespace: config.Namespace,
Name: name,
},
Type: v1.SecretTypeOpaque,
StringData: step.SecretMapping,
}, nil
}
func stepSecretName(step *types.Step) (string, error) {
name, err := stepToPodName(step)
if err != nil {
return "", err
}
return fmt.Sprintf("%s-step-secret", name), nil
}
func stopStepSecret(ctx context.Context, engine *kube, step *types.Step, deleteOpts meta_v1.DeleteOptions) error {
name, err := stepSecretName(step)
if err != nil {
return err
}
log.Trace().Str("name", name).Msg("deleting secret")
err = engine.client.CoreV1().Secrets(engine.config.Namespace).Delete(ctx, name, deleteOpts)
if errors.IsNotFound(err) {
return nil
}
return err
}

View File

@ -26,6 +26,7 @@ type Step struct {
WorkingDir string `json:"working_dir,omitempty"`
WorkspaceBase string `json:"workspace_base,omitempty"`
Environment map[string]string `json:"environment,omitempty"`
SecretMapping map[string]string `json:"secret_mapping,omitempty"`
Entrypoint []string `json:"entrypoint,omitempty"`
Commands []string `json:"commands,omitempty"`
ExtraHosts []HostAlias `json:"extra_hosts,omitempty"`

View File

@ -313,6 +313,120 @@ func TestCompilerCompile(t *testing.T) {
assert.Truef(t, s.Environment["VERBOSE"] == "true", "expected to get value of global set environment")
assert.Truef(t, len(s.Environment) > 10, "expected to have a lot of built-in variables")
s.Environment = nil
s.SecretMapping = nil
}
}
// check if we get an expected backend config based on a frontend config
assert.EqualValues(t, *test.backConf, *backConf)
}
})
}
}
func TestCompilerCompileWithFromSecret(t *testing.T) {
repoURL := "https://github.com/octocat/hello-world"
compiler := New(
WithMetadata(metadata.Metadata{
Repo: metadata.Repo{
Owner: "octacat",
Name: "hello-world",
Private: true,
ForgeURL: repoURL,
CloneURL: "https://github.com/octocat/hello-world.git",
},
}),
WithEnviron(map[string]string{
"VERBOSE": "true",
"COLORED": "true",
}),
WithSecret(Secret{
Name: "secret_name",
Value: "VERY_SECRET",
}),
WithPrefix("test"),
// we use "/test" as custom workspace base to ensure the enforcement of the pluginWorkspaceBase is applied
WithWorkspaceFromURL("/test", repoURL),
)
defaultNetwork := "test_default"
defaultVolume := "test_default"
defaultCloneStage := &backend_types.Stage{
Steps: []*backend_types.Step{{
Name: "clone",
Type: backend_types.StepTypeClone,
Image: constant.DefaultClonePlugin,
OnSuccess: true,
Failure: "fail",
WorkingDir: "/woodpecker/src/github.com/octocat/hello-world",
WorkspaceBase: "/woodpecker",
Volumes: []string{defaultVolume + ":/woodpecker"},
Networks: []backend_types.Conn{{Name: "test_default", Aliases: []string{"clone"}}},
ExtraHosts: []backend_types.HostAlias{},
}},
}
tests := []struct {
name string
fronConf *yaml_types.Workflow
backConf *backend_types.Config
expectedErr string
}{
{
name: "workflow with missing secret",
fronConf: &yaml_types.Workflow{Steps: yaml_types.ContainerList{ContainerList: []*yaml_types.Container{{
Name: "step",
Image: "bash",
Commands: []string{"env"},
Environment: yaml_base_types.EnvironmentMap{
"SECRET": map[string]any{"from_secret": "secret_name"},
},
}}}},
backConf: &backend_types.Config{
Stages: []*backend_types.Stage{defaultCloneStage, {
Steps: []*backend_types.Step{{
Name: "step",
Type: backend_types.StepTypeCommands,
Image: "bash",
Commands: []string{"env"},
OnSuccess: true,
Failure: "fail",
WorkingDir: "/test/src/github.com/octocat/hello-world",
WorkspaceBase: "/test",
Volumes: []string{defaultVolume + ":/test"},
Networks: []backend_types.Conn{{Name: "test_default", Aliases: []string{"step"}}},
ExtraHosts: []backend_types.HostAlias{},
SecretMapping: map[string]string{
"SECRET": "VERY_SECRET",
},
}},
}},
Volume: defaultVolume,
Network: defaultNetwork,
Secrets: []*backend_types.Secret{{
Name: "secret_name",
Value: "VERY_SECRET",
}},
},
expectedErr: "",
},
}
for _, test := range tests {
t.Run(test.name, func(t *testing.T) {
backConf, err := compiler.Compile(test.fronConf)
if test.expectedErr != "" {
assert.Error(t, err)
assert.Equal(t, test.expectedErr, err.Error())
} else {
// we ignore uuids in steps and only check if global env got set ...
for _, st := range backConf.Stages {
for _, s := range st.Steps {
s.UUID = ""
assert.Truef(t, s.Environment["VERBOSE"] == "true", "expected to get value of global set environment")
assert.Truef(t, len(s.Environment) > 10, "expected to have a lot of built-in variables")
s.Environment = nil
if len(s.SecretMapping) == 0 {
s.SecretMapping = nil
}
}
}
// check if we get an expected backend config based on a frontend config

View File

@ -114,11 +114,13 @@ func (c *Compiler) createProcess(container *yaml_types.Container, workflow *yaml
return secret.Value, nil
}
if err := settings.ParamsToEnv(container.Settings, environment, "PLUGIN_", true, getSecretValue); err != nil {
secretMapping := map[string]string{}
if err := settings.ParamsToEnv(container.Settings, environment, "PLUGIN_", true, getSecretValue, secretMapping); err != nil {
return nil, err
}
if err := settings.ParamsToEnv(container.Environment, environment, "", false, getSecretValue); err != nil {
if err := settings.ParamsToEnv(container.Environment, environment, "", false, getSecretValue, secretMapping); err != nil {
return nil, err
}
@ -165,6 +167,7 @@ func (c *Compiler) createProcess(container *yaml_types.Container, workflow *yaml
WorkingDir: workingDir,
WorkspaceBase: workspaceBase,
Environment: environment,
SecretMapping: secretMapping,
Commands: container.Commands,
Entrypoint: container.Entrypoint,
ExtraHosts: extraHosts,

View File

@ -26,7 +26,7 @@ import (
// ParamsToEnv uses reflection to convert a map[string]interface to a list
// of environment variables.
func ParamsToEnv(from map[string]any, to map[string]string, prefix string, upper bool, getSecretValue func(name string) (string, error)) (err error) {
func ParamsToEnv(from map[string]any, to map[string]string, prefix string, upper bool, getSecretValue func(name string) (string, error), secretMapping map[string]string) (err error) {
if to == nil {
return fmt.Errorf("no map to write to")
}
@ -34,10 +34,22 @@ func ParamsToEnv(from map[string]any, to map[string]string, prefix string, upper
if v == nil || len(k) == 0 {
continue
}
to[sanitizeParamKey(prefix, upper, k)], err = sanitizeParamValue(v, getSecretValue)
sanitizedParamKey := sanitizeParamKey(prefix, upper, k)
secretUsed := false
wrappedGetSecretValue := func(name string) (string, error) {
secretUsed = true
return getSecretValue(name)
}
to[sanitizedParamKey], err = sanitizeParamValue(v, wrappedGetSecretValue)
if err != nil {
return err
}
if secretUsed && secretMapping != nil {
secretMapping[sanitizedParamKey] = to[sanitizedParamKey]
}
}
return nil
}

View File

@ -68,13 +68,13 @@ func TestParamsToEnv(t *testing.T) {
return "", fmt.Errorf("secret %q not found or not allowed to be used", name)
}
assert.NoError(t, ParamsToEnv(from, got, "PLUGIN_", true, getSecretValue))
secretMapping := map[string]string{}
assert.NoError(t, ParamsToEnv(from, got, "PLUGIN_", true, getSecretValue, secretMapping))
assert.EqualValues(t, want, got, "Problem converting plugin parameters to environment variables")
// handle edge cases (#1609)
got = map[string]string{}
assert.NoError(t, ParamsToEnv(map[string]any{"a": []any{"a", nil}}, got, "PLUGIN_", true, nil))
assert.NoError(t, ParamsToEnv(map[string]any{"a": []any{"a", nil}}, got, "PLUGIN_", true, nil, nil))
assert.EqualValues(t, map[string]string{"PLUGIN_A": "a,"}, got)
}
@ -92,7 +92,7 @@ func TestParamsToEnvPrefix(t *testing.T) {
return "", fmt.Errorf("secret %q not found or not allowed to be used", name)
}
assert.NoError(t, ParamsToEnv(from, got, "PLUGIN_", true, getSecretValue))
assert.NoError(t, ParamsToEnv(from, got, "PLUGIN_", true, getSecretValue, nil))
assert.EqualValues(t, wantPrefixPlugin, got, "Problem converting plugin parameters to environment variables")
wantNoPrefix := map[string]string{
@ -102,7 +102,7 @@ func TestParamsToEnvPrefix(t *testing.T) {
// handle edge cases (#1609)
got = map[string]string{}
assert.NoError(t, ParamsToEnv(from, got, "", true, getSecretValue))
assert.NoError(t, ParamsToEnv(from, got, "", true, getSecretValue, nil))
assert.EqualValues(t, wantNoPrefix, got, "Problem converting plugin parameters to environment variables")
}
@ -166,8 +166,14 @@ list.map:
return "", fmt.Errorf("secret %q not found or not allowed to be used", name)
}
assert.NoError(t, ParamsToEnv(from, got, "PLUGIN_", true, getSecretValue))
gotSecretMapping := map[string]string{}
wantSecretMapping := map[string]string{
"PLUGIN_MY_SECRET": "FooBar",
"PLUGIN_MAP": `{"entry2":["a","b",3],"key":"value","secret":"FooBar"}`,
"PLUGIN_LIST_MAP": `[{"password":"geheim","registry":"https://codeberg.org","username":"6543"}]`,
}
assert.NoError(t, ParamsToEnv(from, got, "PLUGIN_", true, getSecretValue, gotSecretMapping))
assert.Equal(t, wantSecretMapping, gotSecretMapping, "Problem collecting secret mapping")
assert.EqualValues(t, want, got, "Problem converting plugin parameters to environment variables")
}
@ -191,7 +197,8 @@ func TestYAMLToParamsToEnvError(t *testing.T) {
return "", fmt.Errorf("secret %q not found or not allowed to be used", name)
}
assert.Error(t, ParamsToEnv(from, make(map[string]string), "PLUGIN_", true, getSecretValue))
secretMapping := map[string]string{}
assert.Error(t, ParamsToEnv(from, make(map[string]string), "PLUGIN_", true, getSecretValue, secretMapping))
}
func stringsToInterface(val ...string) []any {
@ -220,8 +227,80 @@ func TestSecretNotFound(t *testing.T) {
return "", fmt.Errorf("secret %q not found or not allowed to be used", name)
}
got := map[string]string{}
secretMapping := map[string]string{}
assert.ErrorContains(t,
ParamsToEnv(from, got, "PLUGIN_", true, getSecretValue),
ParamsToEnv(from, got, "PLUGIN_", true, getSecretValue, secretMapping),
fmt.Sprintf("secret %q not found or not allowed to be used", "secret_token"))
}
func TestSecretMappingSimpleSecret(t *testing.T) {
from := map[string]any{
"simple_secret": map[string]any{"from_secret": "my_token"},
"regular_var": "no_secret_here",
}
secrets := map[string]string{
"my_token": "secret_value_123",
}
getSecretValue := func(name string) (string, error) {
name = strings.ToLower(name)
secret, ok := secrets[name]
if ok {
return secret, nil
}
return "", fmt.Errorf("secret %q not found", name)
}
got := map[string]string{}
secretMapping := map[string]string{}
assert.NoError(t, ParamsToEnv(from, got, "PLUGIN_", true, getSecretValue, secretMapping))
assert.Equal(t, "secret_value_123", got["PLUGIN_SIMPLE_SECRET"])
assert.Equal(t, "no_secret_here", got["PLUGIN_REGULAR_VAR"])
assert.Equal(t, "secret_value_123", secretMapping["PLUGIN_SIMPLE_SECRET"])
assert.NotContains(t, secretMapping, "PLUGIN_REGULAR_VAR")
}
func TestSecretMappingComplexMapWithSecrets(t *testing.T) {
from := map[string]any{
"config": map[string]any{
"database": map[string]any{
"host": "localhost",
"password": map[string]any{"from_secret": "db_password"},
"port": 5432,
},
"api_key": map[string]any{"from_secret": "api_secret"},
"timeout": 30,
},
"simple_var": "no_secrets",
}
secrets := map[string]string{
"db_password": "super_secret_db_pass",
"api_secret": "api_key_12345",
}
getSecretValue := func(name string) (string, error) {
name = strings.ToLower(name)
secret, ok := secrets[name]
if ok {
return secret, nil
}
return "", fmt.Errorf("secret %q not found", name)
}
got := map[string]string{}
secretMapping := map[string]string{}
assert.NoError(t, ParamsToEnv(from, got, "PLUGIN_", true, getSecretValue, secretMapping))
expectedJSON := `{"api_key":"api_key_12345","database":{"host":"localhost","password":"super_secret_db_pass","port":5432},"timeout":30}`
assert.Equal(t, expectedJSON, got["PLUGIN_CONFIG"])
assert.Equal(t, "no_secrets", got["PLUGIN_SIMPLE_VAR"])
assert.Equal(t, expectedJSON, secretMapping["PLUGIN_CONFIG"])
assert.NotContains(t, secretMapping, "PLUGIN_SIMPLE_VAR")
}