fix(executor): pass container variable values and PATH through the docker process environment (GO-004)
- commit
- 8e03ed6517d5bd05af4e8a9f21a9255269789285
- author
- brice · · 2026-10-14 06:22 UTC
- parent
- 63e2006
- signature
- GPG Verified
CD88F9E564E0102646FB6077A1A07C7C7C3E349D· arch-gpg Verified for the committer email
7 files · +130 -14
M internal/executor/actions_docker_cmd.go +10 -1
| @@ -8,6 +8,7 @@ import ( | |||
| 8 | 8 | "os" | |
| 9 | 9 | "os/exec" | |
| 10 | 10 | "regexp" | |
| 11 | + | "slices" | |
| 11 | 12 | "strings" | |
| 12 | 13 | ||
| 13 | 14 | "github.com/google/uuid" | |
| @@ -54,6 +55,10 @@ func escapeDockerArg(v string) string { | |||
| 54 | 55 | type dockerCmd struct { | |
| 55 | 56 | line string | |
| 56 | 57 | argv []string | |
| 58 | + | // env is added to the environment of the docker process, after its own: | |
| 59 | + | // "-e NAME" options read their values there, so that they are neither on | |
| 60 | + | // the command line nor in the echoed text. | |
| 61 | + | env []string | |
| 57 | 62 | } | |
| 58 | 63 | ||
| 59 | 64 | // newDockerCmd builds "docker <command> <options>" the way the runner does | |
| @@ -62,12 +67,14 @@ type dockerCmd struct { | |||
| 62 | 67 | func (j *job) newDockerCmd(command string, parts ...dockerPart) dockerCmd { | |
| 63 | 68 | lines := []string{} | |
| 64 | 69 | argv := []string{j.dockerPath(), command} | |
| 70 | + | var env []string | |
| 65 | 71 | for _, p := range parts { | |
| 66 | 72 | lines = append(lines, p.line) | |
| 67 | 73 | argv = append(argv, p.args...) | |
| 74 | + | env = append(env, p.env...) | |
| 68 | 75 | } | |
| 69 | 76 | line := strings.TrimSpace(command + " " + strings.Join(lines, " ")) | |
| 70 | - | return dockerCmd{line: j.dockerPath() + " " + line, argv: argv} | |
| 77 | + | return dockerCmd{line: j.dockerPath() + " " + line, argv: argv, env: env} | |
| 71 | 78 | } | |
| 72 | 79 | ||
| 73 | 80 | // dockerPart is one piece of a docker command: its text in the echoed line and | |
| @@ -75,6 +82,7 @@ func (j *job) newDockerCmd(command string, parts ...dockerPart) dockerCmd { | |||
| 75 | 82 | type dockerPart struct { | |
| 76 | 83 | line string | |
| 77 | 84 | args []string | |
| 85 | + | env []string // NAME=value for the process of docker, see dockerCmd.env | |
| 78 | 86 | } | |
| 79 | 87 | ||
| 80 | 88 | func raw(args ...string) dockerPart { return dockerPart{line: strings.Join(args, " "), args: args} } | |
| @@ -172,6 +180,7 @@ func (j *job) runDockerCmd(s *stepRun, c dockerCmd, dir string, env []string, ca | |||
| 172 | 180 | if env == nil { | |
| 173 | 181 | env = j.processEnv(newOrdered()) | |
| 174 | 182 | } | |
| 183 | + | env = append(slices.Clone(env), c.env...) | |
| 175 | 184 | res, err := runProcess(procSpec{ | |
| 176 | 185 | Argv: c.argv, Dir: dir, Env: env, Credential: j.cfg.Credential, | |
| 177 | 186 | OnLine: func(line string) { j.write(s.num, line) }, | |
| @@ -8,6 +8,7 @@ import ( | |||||
| 8 | "os" | 8 | "os" | ||
| 9 | "os/exec" | 9 | "os/exec" | ||
| 10 | "regexp" | 10 | "regexp" | ||
| 11 | + | "slices" | |||
| 11 | "strings" | 12 | "strings" | ||
| 12 | 13 | ||||
| 13 | "github.com/google/uuid" | 14 | "github.com/google/uuid" | ||
| @@ -54,6 +55,10 @@ func escapeDockerArg(v string) string { | |||||
| 54 | type dockerCmd struct { | 55 | type dockerCmd struct { | ||
| 55 | line string | 56 | line string | ||
| 56 | argv []string | 57 | argv []string | ||
| 58 | + | // env is added to the environment of the docker process, after its own: | |||
| 59 | + | // "-e NAME" options read their values there, so that they are neither on | |||
| 60 | + | // the command line nor in the echoed text. | |||
| 61 | + | env []string | |||
| 57 | } | 62 | } | ||
| 58 | 63 | ||||
| 59 | // newDockerCmd builds "docker <command> <options>" the way the runner does | 64 | // newDockerCmd builds "docker <command> <options>" the way the runner does | ||
| @@ -62,12 +67,14 @@ type dockerCmd struct { | |||||
| 62 | func (j *job) newDockerCmd(command string, parts ...dockerPart) dockerCmd { | 67 | func (j *job) newDockerCmd(command string, parts ...dockerPart) dockerCmd { | ||
| 63 | lines := []string{} | 68 | lines := []string{} | ||
| 64 | argv := []string{j.dockerPath(), command} | 69 | argv := []string{j.dockerPath(), command} | ||
| 70 | + | var env []string | |||
| 65 | for _, p := range parts { | 71 | for _, p := range parts { | ||
| 66 | lines = append(lines, p.line) | 72 | lines = append(lines, p.line) | ||
| 67 | argv = append(argv, p.args...) | 73 | argv = append(argv, p.args...) | ||
| 74 | + | env = append(env, p.env...) | |||
| 68 | } | 75 | } | ||
| 69 | line := strings.TrimSpace(command + " " + strings.Join(lines, " ")) | 76 | line := strings.TrimSpace(command + " " + strings.Join(lines, " ")) | ||
| 70 | - | return dockerCmd{line: j.dockerPath() + " " + line, argv: argv} | 77 | + | return dockerCmd{line: j.dockerPath() + " " + line, argv: argv, env: env} |
| 71 | } | 78 | } | ||
| 72 | 79 | ||||
| 73 | // dockerPart is one piece of a docker command: its text in the echoed line and | 80 | // dockerPart is one piece of a docker command: its text in the echoed line and | ||
| @@ -75,6 +82,7 @@ func (j *job) newDockerCmd(command string, parts ...dockerPart) dockerCmd { | |||||
| 75 | type dockerPart struct { | 82 | type dockerPart struct { | ||
| 76 | line string | 83 | line string | ||
| 77 | args []string | 84 | args []string | ||
| 85 | + | env []string // NAME=value for the process of docker, see dockerCmd.env | |||
| 78 | } | 86 | } | ||
| 79 | 87 | ||||
| 80 | func raw(args ...string) dockerPart { return dockerPart{line: strings.Join(args, " "), args: args} } | 88 | func raw(args ...string) dockerPart { return dockerPart{line: strings.Join(args, " "), args: args} } | ||
| @@ -172,6 +180,7 @@ func (j *job) runDockerCmd(s *stepRun, c dockerCmd, dir string, env []string, ca | |||||
| 172 | if env == nil { | 180 | if env == nil { | ||
| 173 | env = j.processEnv(newOrdered()) | 181 | env = j.processEnv(newOrdered()) | ||
| 174 | } | 182 | } | ||
| 183 | + | env = append(slices.Clone(env), c.env...) | |||
| 175 | res, err := runProcess(procSpec{ | 184 | res, err := runProcess(procSpec{ | ||
| 176 | Argv: c.argv, Dir: dir, Env: env, Credential: j.cfg.Credential, | 185 | Argv: c.argv, Dir: dir, Env: env, Credential: j.cfg.Credential, | ||
| 177 | OnLine: func(line string) { j.write(s.num, line) }, | 186 | OnLine: func(line string) { j.write(s.num, line) }, | ||
M internal/executor/containers_docker.go +49 -6
| @@ -23,14 +23,57 @@ func eqOpt(flag, value string) dockerPart { | |||
| 23 | 23 | return dockerPart{line: flag + `="` + value + `"`, args: []string{flag + "=" + value}} | |
| 24 | 24 | } | |
| 25 | 25 | ||
| 26 | + | // cliSensitive are the variables the docker CLI itself reads (its daemon, its | |
| 27 | + | // configuration, its proxy and TLS settings, the loader): handing one of them | |
| 28 | + | // to the process of docker as the value of a container variable would change | |
| 29 | + | // what the CLI does, so these are the only values that stay in the arguments. | |
| 30 | + | func cliSensitive(name string) bool { | |
| 31 | + | u := strings.ToUpper(name) | |
| 32 | + | switch u { | |
| 33 | + | case "HOME", "TMPDIR", "USER", "LOGNAME", "HTTP_PROXY", "HTTPS_PROXY", "NO_PROXY", "ALL_PROXY", | |
| 34 | + | "GODEBUG", "GOMAXPROCS", "GOGC", "GOMEMLIMIT", "GOTRACEBACK", "KUBECONFIG", ProcessTrackingID: | |
| 35 | + | return true | |
| 36 | + | } | |
| 37 | + | for _, pre := range []string{"DOCKER_", "XDG_", "LD_", "SSL_CERT", "BUILDKIT_", "COMPOSE_", "CONTAINERD_", "REGISTRY_"} { | |
| 38 | + | if strings.HasPrefix(u, pre) { | |
| 39 | + | return true | |
| 40 | + | } | |
| 41 | + | } | |
| 42 | + | return false | |
| 43 | + | } | |
| 44 | + | ||
| 45 | + | // envForCLI is how one container variable reaches `docker create`/`exec`: | |
| 46 | + | // "-e NAME" with the value in the environment of the docker process, so that | |
| 47 | + | // it is on no command line (visible to every user of the sandbox in /proc) — | |
| 48 | + | // except for the variables the CLI reads itself, unless their value is the one | |
| 49 | + | // the process has anyway. An empty value is set explicitly, so that the CLI | |
| 50 | + | // cannot inherit one from the executor. echo is the text the runner logs. | |
| 51 | + | func (j *job) envForCLI(key, value, echo string) dockerPart { | |
| 52 | + | if cliSensitive(key) { | |
| 53 | + | if v, ok := envLookup(j.baseEnv, key); ok && v == value { | |
| 54 | + | return dockerPart{line: echo, args: []string{"-e", key}} | |
| 55 | + | } | |
| 56 | + | return dockerPart{line: echo, args: []string{"-e", key + "=" + value}} | |
| 57 | + | } | |
| 58 | + | return dockerPart{line: echo, args: []string{"-e", key}, env: []string{key + "=" + value}} | |
| 59 | + | } | |
| 60 | + | ||
| 61 | + | func envLookup(env []string, key string) (string, bool) { | |
| 62 | + | for i := len(env) - 1; i >= 0; i-- { | |
| 63 | + | if k, v, ok := strings.Cut(env[i], "="); ok && k == key { | |
| 64 | + | return v, true | |
| 65 | + | } | |
| 66 | + | } | |
| 67 | + | return "", false | |
| 68 | + | } | |
| 69 | + | ||
| 26 | 70 | // envKV is DockerUtil.CreateEscapedOption("-e", key, value), or ("-e", key) | |
| 27 | - | // for an empty value (docker then reads it from its own environment). | |
| 28 | - | func envKV(key, value string) dockerPart { | |
| 71 | + | // for an empty value, as the runner echoes it. | |
| 72 | + | func (j *job) envKV(key, value string) dockerPart { | |
| 29 | 73 | if value == "" { | |
| 30 | - | return envOpt(key) | |
| 74 | + | return j.envForCLI(key, "", "-e "+escapeDockerArg(key)) | |
| 31 | 75 | } | |
| 32 | - | kv := key + "=" + value | |
| 33 | - | return dockerPart{line: "-e " + escapeDockerArg(kv), args: []string{"-e", kv}} | |
| 76 | + | return j.envForCLI(key, value, "-e "+escapeDockerArg(key+"="+value)) | |
| 34 | 77 | } | |
| 35 | 78 | ||
| 36 | 79 | // mountOpt is the -v option of DockerCommandManager.DockerCreate (:164-185). | |
| @@ -70,7 +113,7 @@ func (j *job) dockerList(s *stepRun, c dockerCmd, cancel <-chan struct{}) ([]str | |||
| 70 | 113 | j.out(s.num, "##[command]"+c.line) | |
| 71 | 114 | var lines []string | |
| 72 | 115 | res, err := runProcess(procSpec{ | |
| 73 | - | Argv: c.argv, Dir: dirOrCwd(j.githubValue("workspace")), Env: j.processEnv(newOrdered()), Credential: j.cfg.Credential, | |
| 116 | + | Argv: c.argv, Dir: dirOrCwd(j.githubValue("workspace")), Env: append(j.processEnv(newOrdered()), c.env...), Credential: j.cfg.Credential, | |
| 74 | 117 | OnLine: func(l string) { | |
| 75 | 118 | if l != "" { | |
| 76 | 119 | lines = append(lines, l) | |
| @@ -23,14 +23,57 @@ func eqOpt(flag, value string) dockerPart { | |||||
| 23 | return dockerPart{line: flag + `="` + value + `"`, args: []string{flag + "=" + value}} | 23 | return dockerPart{line: flag + `="` + value + `"`, args: []string{flag + "=" + value}} | ||
| 24 | } | 24 | } | ||
| 25 | 25 | ||||
| 26 | + | // cliSensitive are the variables the docker CLI itself reads (its daemon, its | |||
| 27 | + | // configuration, its proxy and TLS settings, the loader): handing one of them | |||
| 28 | + | // to the process of docker as the value of a container variable would change | |||
| 29 | + | // what the CLI does, so these are the only values that stay in the arguments. | |||
| 30 | + | func cliSensitive(name string) bool { | |||
| 31 | + | u := strings.ToUpper(name) | |||
| 32 | + | switch u { | |||
| 33 | + | case "HOME", "TMPDIR", "USER", "LOGNAME", "HTTP_PROXY", "HTTPS_PROXY", "NO_PROXY", "ALL_PROXY", | |||
| 34 | + | "GODEBUG", "GOMAXPROCS", "GOGC", "GOMEMLIMIT", "GOTRACEBACK", "KUBECONFIG", ProcessTrackingID: | |||
| 35 | + | return true | |||
| 36 | + | } | |||
| 37 | + | for _, pre := range []string{"DOCKER_", "XDG_", "LD_", "SSL_CERT", "BUILDKIT_", "COMPOSE_", "CONTAINERD_", "REGISTRY_"} { | |||
| 38 | + | if strings.HasPrefix(u, pre) { | |||
| 39 | + | return true | |||
| 40 | + | } | |||
| 41 | + | } | |||
| 42 | + | return false | |||
| 43 | + | } | |||
| 44 | + | ||||
| 45 | + | // envForCLI is how one container variable reaches `docker create`/`exec`: | |||
| 46 | + | // "-e NAME" with the value in the environment of the docker process, so that | |||
| 47 | + | // it is on no command line (visible to every user of the sandbox in /proc) — | |||
| 48 | + | // except for the variables the CLI reads itself, unless their value is the one | |||
| 49 | + | // the process has anyway. An empty value is set explicitly, so that the CLI | |||
| 50 | + | // cannot inherit one from the executor. echo is the text the runner logs. | |||
| 51 | + | func (j *job) envForCLI(key, value, echo string) dockerPart { | |||
| 52 | + | if cliSensitive(key) { | |||
| 53 | + | if v, ok := envLookup(j.baseEnv, key); ok && v == value { | |||
| 54 | + | return dockerPart{line: echo, args: []string{"-e", key}} | |||
| 55 | + | } | |||
| 56 | + | return dockerPart{line: echo, args: []string{"-e", key + "=" + value}} | |||
| 57 | + | } | |||
| 58 | + | return dockerPart{line: echo, args: []string{"-e", key}, env: []string{key + "=" + value}} | |||
| 59 | + | } | |||
| 60 | + | ||||
| 61 | + | func envLookup(env []string, key string) (string, bool) { | |||
| 62 | + | for i := len(env) - 1; i >= 0; i-- { | |||
| 63 | + | if k, v, ok := strings.Cut(env[i], "="); ok && k == key { | |||
| 64 | + | return v, true | |||
| 65 | + | } | |||
| 66 | + | } | |||
| 67 | + | return "", false | |||
| 68 | + | } | |||
| 69 | + | ||||
| 26 | // envKV is DockerUtil.CreateEscapedOption("-e", key, value), or ("-e", key) | 70 | // envKV is DockerUtil.CreateEscapedOption("-e", key, value), or ("-e", key) | ||
| 27 | - | // for an empty value (docker then reads it from its own environment). | 71 | + | // for an empty value, as the runner echoes it. |
| 28 | - | func envKV(key, value string) dockerPart { | 72 | + | func (j *job) envKV(key, value string) dockerPart { |
| 29 | if value == "" { | 73 | if value == "" { | ||
| 30 | - | return envOpt(key) | 74 | + | return j.envForCLI(key, "", "-e "+escapeDockerArg(key)) |
| 31 | } | 75 | } | ||
| 32 | - | kv := key + "=" + value | 76 | + | return j.envForCLI(key, value, "-e "+escapeDockerArg(key+"="+value)) |
| 33 | - | return dockerPart{line: "-e " + escapeDockerArg(kv), args: []string{"-e", kv}} | |||
| 34 | } | 77 | } | ||
| 35 | 78 | ||||
| 36 | // mountOpt is the -v option of DockerCommandManager.DockerCreate (:164-185). | 79 | // mountOpt is the -v option of DockerCommandManager.DockerCreate (:164-185). | ||
| @@ -70,7 +113,7 @@ func (j *job) dockerList(s *stepRun, c dockerCmd, cancel <-chan struct{}) ([]str | |||||
| 70 | j.out(s.num, "##[command]"+c.line) | 113 | j.out(s.num, "##[command]"+c.line) | ||
| 71 | var lines []string | 114 | var lines []string | ||
| 72 | res, err := runProcess(procSpec{ | 115 | res, err := runProcess(procSpec{ | ||
| 73 | - | Argv: c.argv, Dir: dirOrCwd(j.githubValue("workspace")), Env: j.processEnv(newOrdered()), Credential: j.cfg.Credential, | 116 | + | Argv: c.argv, Dir: dirOrCwd(j.githubValue("workspace")), Env: append(j.processEnv(newOrdered()), c.env...), Credential: j.cfg.Credential, |
| 74 | OnLine: func(l string) { | 117 | OnLine: func(l string) { | ||
| 75 | if l != "" { | 118 | if l != "" { | ||
| 76 | lines = append(lines, l) | 119 | lines = append(lines, l) | ||
M internal/executor/containers_exec.go +13 -3
| @@ -53,8 +53,15 @@ func (j *job) execArgv(c *container, workDir string, env *ordered, command strin | |||
| 53 | 53 | cenv := newOrdered() | |
| 54 | 54 | for _, k := range env.Keys() { | |
| 55 | 55 | v, _ := env.Get(k) | |
| 56 | - | argv = append(argv, "-e", k) | |
| 57 | - | cenv.Set(k, c.translateToContainer(v)) | |
| 56 | + | v = c.translateToContainer(v) | |
| 57 | + | // PATH is set below. The docker process is started by its absolute | |
| 58 | + | // path, so the variables of the container are not read by it, except | |
| 59 | + | // those the CLI itself reads. | |
| 60 | + | p := j.envForCLI(k, v, "") | |
| 61 | + | argv = append(argv, p.args...) | |
| 62 | + | for _, kv := range p.env { | |
| 63 | + | cenv.Set(k, strings.TrimPrefix(kv, k+"=")) | |
| 64 | + | } | |
| 58 | 65 | } | |
| 59 | 66 | j.mu.Lock() | |
| 60 | 67 | prepend := j.state.Prepend() | |
| @@ -68,7 +75,10 @@ func (j *job) execArgv(c *container, workDir string, env *ordered, command strin | |||
| 68 | 75 | if c.runtimePath != "" { | |
| 69 | 76 | full += ":" + c.runtimePath | |
| 70 | 77 | } | |
| 71 | - | argv = append(argv, "-e", "PATH="+full) | |
| 78 | + | // The value is in the environment of docker.exec only: the process is | |
| 79 | + | // started by absolute path and `docker exec` runs nothing of its own. | |
| 80 | + | argv = append(argv, "-e", "PATH") | |
| 81 | + | cenv.Set("PATH", full) | |
| 72 | 82 | } | |
| 73 | 83 | argv = append(argv, c.id, command) | |
| 74 | 84 | argv = append(argv, args...) | |
| @@ -53,8 +53,15 @@ func (j *job) execArgv(c *container, workDir string, env *ordered, command strin | |||||
| 53 | cenv := newOrdered() | 53 | cenv := newOrdered() | ||
| 54 | for _, k := range env.Keys() { | 54 | for _, k := range env.Keys() { | ||
| 55 | v, _ := env.Get(k) | 55 | v, _ := env.Get(k) | ||
| 56 | - | argv = append(argv, "-e", k) | 56 | + | v = c.translateToContainer(v) |
| 57 | - | cenv.Set(k, c.translateToContainer(v)) | 57 | + | // PATH is set below. The docker process is started by its absolute |
| 58 | + | // path, so the variables of the container are not read by it, except | |||
| 59 | + | // those the CLI itself reads. | |||
| 60 | + | p := j.envForCLI(k, v, "") | |||
| 61 | + | argv = append(argv, p.args...) | |||
| 62 | + | for _, kv := range p.env { | |||
| 63 | + | cenv.Set(k, strings.TrimPrefix(kv, k+"=")) | |||
| 64 | + | } | |||
| 58 | } | 65 | } | ||
| 59 | j.mu.Lock() | 66 | j.mu.Lock() | ||
| 60 | prepend := j.state.Prepend() | 67 | prepend := j.state.Prepend() | ||
| @@ -68,7 +75,10 @@ func (j *job) execArgv(c *container, workDir string, env *ordered, command strin | |||||
| 68 | if c.runtimePath != "" { | 75 | if c.runtimePath != "" { | ||
| 69 | full += ":" + c.runtimePath | 76 | full += ":" + c.runtimePath | ||
| 70 | } | 77 | } | ||
| 71 | - | argv = append(argv, "-e", "PATH="+full) | 78 | + | // The value is in the environment of docker.exec only: the process is |
| 79 | + | // started by absolute path and `docker exec` runs nothing of its own. | |||
| 80 | + | argv = append(argv, "-e", "PATH") | |||
| 81 | + | cenv.Set("PATH", full) | |||
| 72 | } | 82 | } | ||
| 73 | argv = append(argv, c.id, command) | 83 | argv = append(argv, c.id, command) | ||
| 74 | argv = append(argv, args...) | 84 | argv = append(argv, args...) | ||
M internal/executor/containers_exec_test.go +1 -1
| @@ -104,7 +104,7 @@ func TestNodeActionInContainerUsesExternalsAndAlpine(t *testing.T) { | |||
| 104 | 104 | if len(ex) != 2 { | |
| 105 | 105 | t.Fatalf("%s: exec calls %v", tc.node, ex) | |
| 106 | 106 | } | |
| 107 | - | if got := strings.Join(ex[0][len(ex[0])-3:], " "); got != "sh -c "+alpineCheck { | |
| 107 | + | if got := strings.Join(ex[0][len(ex[0])-4:len(ex[0])-1], " "); got != "sh -c "+alpineCheck { | |
| 108 | 108 | t.Errorf("probe: %v", ex[0]) | |
| 109 | 109 | } | |
| 110 | 110 | run := strings.Join(ex[1], "\n") | |
| @@ -104,7 +104,7 @@ func TestNodeActionInContainerUsesExternalsAndAlpine(t *testing.T) { | |||||
| 104 | if len(ex) != 2 { | 104 | if len(ex) != 2 { | ||
| 105 | t.Fatalf("%s: exec calls %v", tc.node, ex) | 105 | t.Fatalf("%s: exec calls %v", tc.node, ex) | ||
| 106 | } | 106 | } | ||
| 107 | - | if got := strings.Join(ex[0][len(ex[0])-3:], " "); got != "sh -c "+alpineCheck { | 107 | + | if got := strings.Join(ex[0][len(ex[0])-4:len(ex[0])-1], " "); got != "sh -c "+alpineCheck { |
| 108 | t.Errorf("probe: %v", ex[0]) | 108 | t.Errorf("probe: %v", ex[0]) | ||
| 109 | } | 109 | } | ||
| 110 | run := strings.Join(ex[1], "\n") | 110 | run := strings.Join(ex[1], "\n") | ||
M internal/executor/containers_fake_test.go +3 -2
| @@ -28,7 +28,7 @@ func newFakeCtr(t *testing.T) *fakeCtr { | |||
| 28 | 28 | f := &fakeCtr{dir: d, bin: filepath.Join(d, "docker"), log: filepath.Join(d, "calls")} | |
| 29 | 29 | script := `#!/bin/sh | |
| 30 | 30 | D=` + d + ` | |
| 31 | - | { echo "--"; for a in "$@"; do printf "%s\n" "$a"; done; } >> $D/calls | |
| 31 | + | { echo "--"; for a in "$@"; do printf "%s\n" "$a"; done; echo "ENV:CVAR=$CVAR:EMPTYV=${EMPTYV-unset}:PATH=$PATH"; } >> $D/calls | |
| 32 | 32 | [ "$1" = "--config" ] && { cat > $D/login_stdin_$$ 2>/dev/null </dev/null; shift 2; } | |
| 33 | 33 | case "$1" in | |
| 34 | 34 | version) echo "'1.48'" ;; | |
| @@ -47,7 +47,8 @@ inspect) case "$*" in | |||
| 47 | 47 | port) [ -f $D/ports ] && cat $D/ports ;; | |
| 48 | 48 | logs) echo "service log of $3" ;; | |
| 49 | 49 | rm) echo "$3" ;; | |
| 50 | - | exec) [ -f $D/exec_env ] && echo "ENV GITHUB_WORKSPACE=$GITHUB_WORKSPACE RUNNER_TEMP=$RUNNER_TEMP HOME=${HOME:-<unset>} GITHUB_OUTPUT=$GITHUB_OUTPUT GITHUB_EVENT_PATH=$GITHUB_EVENT_PATH" | |
| 50 | + | exec) if [ -f $D/add_path ]; then f=$(echo "$GITHUB_PATH" | sed "s#^/__w#$(cat $D/add_path)#"); echo /added >> "$f"; fi | |
| 51 | + | [ -f $D/exec_env ] && echo "ENV GITHUB_WORKSPACE=$GITHUB_WORKSPACE RUNNER_TEMP=$RUNNER_TEMP HOME=${HOME:-<unset>} GITHUB_OUTPUT=$GITHUB_OUTPUT GITHUB_EVENT_PATH=$GITHUB_EVENT_PATH" | |
| 51 | 52 | [ -f $D/hang_exec ] && { trap 'exit 130' INT TERM; while :; do sleep 0.05; done; } | |
| 52 | 53 | [ -f $D/exec_out ] && cat $D/exec_out | |
| 53 | 54 | if [ -f $D/exec_exit ]; then n=$(cat $D/exec_exit); [ "$n" != 0 ] && echo 0 > $D/exec_exit && exit $n; fi ;; | |
| @@ -28,7 +28,7 @@ func newFakeCtr(t *testing.T) *fakeCtr { | |||||
| 28 | f := &fakeCtr{dir: d, bin: filepath.Join(d, "docker"), log: filepath.Join(d, "calls")} | 28 | f := &fakeCtr{dir: d, bin: filepath.Join(d, "docker"), log: filepath.Join(d, "calls")} | ||
| 29 | script := `#!/bin/sh | 29 | script := `#!/bin/sh | ||
| 30 | D=` + d + ` | 30 | D=` + d + ` | ||
| 31 | - | { echo "--"; for a in "$@"; do printf "%s\n" "$a"; done; } >> $D/calls | 31 | + | { echo "--"; for a in "$@"; do printf "%s\n" "$a"; done; echo "ENV:CVAR=$CVAR:EMPTYV=${EMPTYV-unset}:PATH=$PATH"; } >> $D/calls |
| 32 | [ "$1" = "--config" ] && { cat > $D/login_stdin_$$ 2>/dev/null </dev/null; shift 2; } | 32 | [ "$1" = "--config" ] && { cat > $D/login_stdin_$$ 2>/dev/null </dev/null; shift 2; } | ||
| 33 | case "$1" in | 33 | case "$1" in | ||
| 34 | version) echo "'1.48'" ;; | 34 | version) echo "'1.48'" ;; | ||
| @@ -47,7 +47,8 @@ inspect) case "$*" in | |||||
| 47 | port) [ -f $D/ports ] && cat $D/ports ;; | 47 | port) [ -f $D/ports ] && cat $D/ports ;; | ||
| 48 | logs) echo "service log of $3" ;; | 48 | logs) echo "service log of $3" ;; | ||
| 49 | rm) echo "$3" ;; | 49 | rm) echo "$3" ;; | ||
| 50 | - | exec) [ -f $D/exec_env ] && echo "ENV GITHUB_WORKSPACE=$GITHUB_WORKSPACE RUNNER_TEMP=$RUNNER_TEMP HOME=${HOME:-<unset>} GITHUB_OUTPUT=$GITHUB_OUTPUT GITHUB_EVENT_PATH=$GITHUB_EVENT_PATH" | 50 | + | exec) if [ -f $D/add_path ]; then f=$(echo "$GITHUB_PATH" | sed "s#^/__w#$(cat $D/add_path)#"); echo /added >> "$f"; fi |
| 51 | + | [ -f $D/exec_env ] && echo "ENV GITHUB_WORKSPACE=$GITHUB_WORKSPACE RUNNER_TEMP=$RUNNER_TEMP HOME=${HOME:-<unset>} GITHUB_OUTPUT=$GITHUB_OUTPUT GITHUB_EVENT_PATH=$GITHUB_EVENT_PATH" | |||
| 51 | [ -f $D/hang_exec ] && { trap 'exit 130' INT TERM; while :; do sleep 0.05; done; } | 52 | [ -f $D/hang_exec ] && { trap 'exit 130' INT TERM; while :; do sleep 0.05; done; } | ||
| 52 | [ -f $D/exec_out ] && cat $D/exec_out | 53 | [ -f $D/exec_out ] && cat $D/exec_out | ||
| 53 | if [ -f $D/exec_exit ]; then n=$(cat $D/exec_exit); [ "$n" != 0 ] && echo 0 > $D/exec_exit && exit $n; fi ;; | 54 | if [ -f $D/exec_exit ]; then n=$(cat $D/exec_exit); [ "$n" != 0 ] && echo 0 > $D/exec_exit && exit $n; fi ;; | ||
M internal/executor/containers_init_test.go +53 -0
| @@ -48,3 +48,56 @@ func TestInitAndStopServiceOnly(t *testing.T) { | |||
| 48 | 48 | t.Errorf("job.services not in the context:\n%s", run) | |
| 49 | 49 | } | |
| 50 | 50 | } | |
| 51 | + | ||
| 52 | + | // GO-004: values of container variables are in the environment of the docker | |
| 53 | + | // process; the arguments carry "-e NAME". The echoed line is the runner's | |
| 54 | + | // (recorded: -e "NAME=value"), masked like any log. | |
| 55 | + | func TestContainerEnvValuesStayOffTheArguments(t *testing.T) { | |
| 56 | + | f := newFakeCtr(t) | |
| 57 | + | rec, res := ctrRun(t, f, " container:\n image: ubuntu:24.04\n env:\n CVAR: s3cret-value\n EMPTYV: \"\"\n DOCKER_HOST: tcp://x:1\n", | |
| 58 | + | " - name: p\n run: echo\n - name: q\n env:\n STEPVAR: s3cret-step\n run: echo\n", func(c *Config) { f.set(t, "add_path", c.Layout.WorkRoot) }) | |
| 59 | + | if res.Conclusion != ConclusionSuccess { | |
| 60 | + | t.Fatalf("%+v", res) | |
| 61 | + | } | |
| 62 | + | var create []string | |
| 63 | + | for _, c := range f.calls(t) { | |
| 64 | + | if c[0] == "create" { | |
| 65 | + | create = c | |
| 66 | + | } | |
| 67 | + | } | |
| 68 | + | joined := strings.Join(create, "\n") | |
| 69 | + | for _, w := range []string{"\n-e\nCVAR\n", "\n-e\nEMPTYV\n", "\n-e\nDOCKER_HOST=tcp://x:1\n", "ENV:CVAR=s3cret-value:EMPTYV=:"} { | |
| 70 | + | if !strings.Contains(joined, w) { | |
| 71 | + | t.Errorf("create lacks %q:\n%s", w, joined) | |
| 72 | + | } | |
| 73 | + | } | |
| 74 | + | if strings.Contains(joined, "CVAR=s3cret") && !strings.Contains(joined, "ENV:CVAR=s3cret") { | |
| 75 | + | t.Errorf("value on the arguments:\n%s", joined) | |
| 76 | + | } | |
| 77 | + | for _, a := range create { | |
| 78 | + | if strings.HasPrefix(a, "CVAR=") || strings.HasPrefix(a, "EMPTYV=") { | |
| 79 | + | t.Errorf("argument %q carries a value", a) | |
| 80 | + | } | |
| 81 | + | } | |
| 82 | + | // What the runner echoes (recorded in ctr-options). | |
| 83 | + | init := logOf(rec.step(t, "Initialize containers")) | |
| 84 | + | if !strings.Contains(init, ` -e "CVAR=s3cret-value" -e "EMPTYV" `) { | |
| 85 | + | t.Errorf("echo:\n%s", init) | |
| 86 | + | } | |
| 87 | + | // exec: no value of a step variable and no PATH on the arguments. | |
| 88 | + | for _, c := range f.execCalls(t) { | |
| 89 | + | for _, a := range c { | |
| 90 | + | if strings.HasPrefix(a, "ENV:") { | |
| 91 | + | continue // what the fake saw in its environment | |
| 92 | + | } | |
| 93 | + | if strings.Contains(a, "s3cret-step") || strings.HasPrefix(a, "PATH=") || strings.Contains(a, "/added") { | |
| 94 | + | t.Errorf("exec argument %q", a) | |
| 95 | + | } | |
| 96 | + | } | |
| 97 | + | } | |
| 98 | + | ex := f.execCalls(t) | |
| 99 | + | last := strings.Join(ex[len(ex)-1], "\n") | |
| 100 | + | if !strings.Contains(last, "\n-e\nPATH\n") || !strings.Contains(last, "ENV:") || !strings.Contains(last, "PATH=/added:") { | |
| 101 | + | t.Errorf("second exec:\n%s", last) | |
| 102 | + | } | |
| 103 | + | } | |
| @@ -48,3 +48,56 @@ func TestInitAndStopServiceOnly(t *testing.T) { | |||||
| 48 | t.Errorf("job.services not in the context:\n%s", run) | 48 | t.Errorf("job.services not in the context:\n%s", run) | ||
| 49 | } | 49 | } | ||
| 50 | } | 50 | } | ||
| 51 | + | ||||
| 52 | + | // GO-004: values of container variables are in the environment of the docker | |||
| 53 | + | // process; the arguments carry "-e NAME". The echoed line is the runner's | |||
| 54 | + | // (recorded: -e "NAME=value"), masked like any log. | |||
| 55 | + | func TestContainerEnvValuesStayOffTheArguments(t *testing.T) { | |||
| 56 | + | f := newFakeCtr(t) | |||
| 57 | + | rec, res := ctrRun(t, f, " container:\n image: ubuntu:24.04\n env:\n CVAR: s3cret-value\n EMPTYV: \"\"\n DOCKER_HOST: tcp://x:1\n", | |||
| 58 | + | " - name: p\n run: echo\n - name: q\n env:\n STEPVAR: s3cret-step\n run: echo\n", func(c *Config) { f.set(t, "add_path", c.Layout.WorkRoot) }) | |||
| 59 | + | if res.Conclusion != ConclusionSuccess { | |||
| 60 | + | t.Fatalf("%+v", res) | |||
| 61 | + | } | |||
| 62 | + | var create []string | |||
| 63 | + | for _, c := range f.calls(t) { | |||
| 64 | + | if c[0] == "create" { | |||
| 65 | + | create = c | |||
| 66 | + | } | |||
| 67 | + | } | |||
| 68 | + | joined := strings.Join(create, "\n") | |||
| 69 | + | for _, w := range []string{"\n-e\nCVAR\n", "\n-e\nEMPTYV\n", "\n-e\nDOCKER_HOST=tcp://x:1\n", "ENV:CVAR=s3cret-value:EMPTYV=:"} { | |||
| 70 | + | if !strings.Contains(joined, w) { | |||
| 71 | + | t.Errorf("create lacks %q:\n%s", w, joined) | |||
| 72 | + | } | |||
| 73 | + | } | |||
| 74 | + | if strings.Contains(joined, "CVAR=s3cret") && !strings.Contains(joined, "ENV:CVAR=s3cret") { | |||
| 75 | + | t.Errorf("value on the arguments:\n%s", joined) | |||
| 76 | + | } | |||
| 77 | + | for _, a := range create { | |||
| 78 | + | if strings.HasPrefix(a, "CVAR=") || strings.HasPrefix(a, "EMPTYV=") { | |||
| 79 | + | t.Errorf("argument %q carries a value", a) | |||
| 80 | + | } | |||
| 81 | + | } | |||
| 82 | + | // What the runner echoes (recorded in ctr-options). | |||
| 83 | + | init := logOf(rec.step(t, "Initialize containers")) | |||
| 84 | + | if !strings.Contains(init, ` -e "CVAR=s3cret-value" -e "EMPTYV" `) { | |||
| 85 | + | t.Errorf("echo:\n%s", init) | |||
| 86 | + | } | |||
| 87 | + | // exec: no value of a step variable and no PATH on the arguments. | |||
| 88 | + | for _, c := range f.execCalls(t) { | |||
| 89 | + | for _, a := range c { | |||
| 90 | + | if strings.HasPrefix(a, "ENV:") { | |||
| 91 | + | continue // what the fake saw in its environment | |||
| 92 | + | } | |||
| 93 | + | if strings.Contains(a, "s3cret-step") || strings.HasPrefix(a, "PATH=") || strings.Contains(a, "/added") { | |||
| 94 | + | t.Errorf("exec argument %q", a) | |||
| 95 | + | } | |||
| 96 | + | } | |||
| 97 | + | } | |||
| 98 | + | ex := f.execCalls(t) | |||
| 99 | + | last := strings.Join(ex[len(ex)-1], "\n") | |||
| 100 | + | if !strings.Contains(last, "\n-e\nPATH\n") || !strings.Contains(last, "ENV:") || !strings.Contains(last, "PATH=/added:") { | |||
| 101 | + | t.Errorf("second exec:\n%s", last) | |||
| 102 | + | } | |||
| 103 | + | } | |||
M internal/executor/containers_start.go +1 -1
| @@ -28,7 +28,7 @@ func (j *job) createCmd(c *container) dockerCmd { | |||
| 28 | 28 | parts = append(parts, optionsParts(c.options)) | |
| 29 | 29 | for _, k := range c.env.Keys() { | |
| 30 | 30 | v, _ := c.env.Get(k) | |
| 31 | - | parts = append(parts, envKV(k, v)) | |
| 31 | + | parts = append(parts, j.envKV(k, v)) | |
| 32 | 32 | } | |
| 33 | 33 | parts = append(parts, raw("-e", "GITHUB_ACTIONS=true")) | |
| 34 | 34 | if _, ok := c.env.Get("CI"); !ok { | |
| @@ -28,7 +28,7 @@ func (j *job) createCmd(c *container) dockerCmd { | |||||
| 28 | parts = append(parts, optionsParts(c.options)) | 28 | parts = append(parts, optionsParts(c.options)) | ||
| 29 | for _, k := range c.env.Keys() { | 29 | for _, k := range c.env.Keys() { | ||
| 30 | v, _ := c.env.Get(k) | 30 | v, _ := c.env.Get(k) | ||
| 31 | - | parts = append(parts, envKV(k, v)) | 31 | + | parts = append(parts, j.envKV(k, v)) |
| 32 | } | 32 | } | ||
| 33 | parts = append(parts, raw("-e", "GITHUB_ACTIONS=true")) | 33 | parts = append(parts, raw("-e", "GITHUB_ACTIONS=true")) | ||
| 34 | if _, ok := c.env.Get("CI"); !ok { | 34 | if _, ok := c.env.Get("CI"); !ok { | ||