Skip to content

Commit a8d2d84

Browse files
committed
Fix env behaviour and dont let host envs leak into container unless allowlisted
1 parent 08db59b commit a8d2d84

1 file changed

Lines changed: 87 additions & 23 deletions

File tree

nodes/docker-run@v1.go

Lines changed: 87 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -74,29 +74,18 @@ func (n *DockerNode) ExecuteImpl(c *core.ExecutionState, inputId core.InputId, p
7474
return err
7575
}
7676

77-
// build env map from context and input env vars.
78-
// In contrary to Docker, env vars in GitHub are automatically included, see code.
79-
// See: github.com/actions/runner/blob/main/src/Runner.Worker/Handlers/ContainerActionHandler.cs
80-
// I just take this behaviour for the entire system.
81-
// TODO: (Seb) Add an option to override this
82-
currentEnvMap := c.GetContextEnvironMapCopy()
83-
84-
// filter out env variables that would break Linux containers when running on Windows:
85-
// 1. Empty keys or keys starting with '=' - Windows per-drive CWD tracking variables
86-
// (eg =C:=, =D:=, =::=) are parsed by strings.Cut as empty-key entries
87-
// 2. PATH - Windows PATH contains Windows paths that break Linux container commands
88-
for key := range currentEnvMap {
89-
if key == "" || strings.HasPrefix(key, "=") || key == "PATH" {
90-
delete(currentEnvMap, key)
91-
}
92-
}
93-
for _, env := range envs {
94-
if idx := strings.Index(env, "="); idx > 0 {
95-
currentEnvMap[env[:idx]] = env[idx+1:]
96-
}
97-
// KEY without `=` is a Docker feature, but here a no-op since
98-
// we pass the full env to Docker anyway
99-
}
77+
// Build container environment following GitHub Actions approach:
78+
// Don't inherit host OS environment wholesale. Instead, build explicitly from:
79+
// 1. Allowlisted GITHUB_* variables (when in GitHub workflow mode)
80+
// 2. Hardcoded CI=true, GITHUB_ACTIONS=true
81+
// 3. User-defined env vars from node inputs
82+
// 4. Proxy variables (HTTP_PROXY, HTTPS_PROXY, NO_PROXY)
83+
//
84+
// References:
85+
// - GitHubContext.cs allowlist: https://github.com/actions/runner/blob/main/src/Runner.Worker/GitHubContext.cs#L106-L146
86+
// - DockerCommandManager.cs CI/GITHUB_ACTIONS: https://github.com/actions/runner/blob/main/src/Runner.Worker/Container/DockerCommandManager.cs#L329-L336
87+
// - ContainerActionHandler.cs HOME: https://github.com/actions/runner/blob/main/src/Runner.Worker/Handlers/ContainerActionHandler.cs#L176
88+
currentEnvMap := buildDockerEnvironment(c, envs)
10089

10190
// parse image reference. docker:// prefix means registry, otherwise Dockerfile
10291
isRegistry, imageRef := parseImageReference(image)
@@ -410,6 +399,81 @@ func parseVolume(vol string) (core.Volume, error) {
410399
return v, nil
411400
}
412401

402+
// githubContextAllowlist defines the GitHub context keys that are exposed as GITHUB_* env vars.
403+
// This matches the allowlist in the GitHub Actions runner:
404+
// https://github.com/actions/runner/blob/main/src/Runner.Worker/GitHubContext.cs#L106-L146
405+
var githubContextAllowlist = map[string]bool{
406+
"action": true, "action_path": true, "action_ref": true, "action_repository": true,
407+
"actor": true, "actor_id": true, "api_url": true, "base_ref": true,
408+
"env": true, "event_name": true, "event_path": true, "graphql_url": true,
409+
"head_ref": true, "job": true, "output": true, "path": true,
410+
"ref": true, "ref_name": true, "ref_protected": true, "ref_type": true,
411+
"repository": true, "repository_id": true, "repository_owner": true, "repository_owner_id": true,
412+
"retention_days": true, "run_attempt": true, "run_id": true, "run_number": true,
413+
"server_url": true, "sha": true, "state": true, "step_summary": true,
414+
"triggering_actor": true, "workflow": true, "workflow_ref": true, "workflow_sha": true,
415+
"workspace": true,
416+
}
417+
418+
// buildDockerEnvironment builds the container environment based on the GitHub Actions impl
419+
// so I just borrow tha, *partially* even for non-gh graphs. See also here:
420+
// - GitHubContext.cs: https://github.com/actions/runner/blob/main/src/Runner.Worker/GitHubContext.cs#L106-L146
421+
// - DockerCommandManager.cs: https://github.com/actions/runner/blob/main/src/Runner.Worker/Container/DockerCommandManager.cs#L329-L336
422+
// - ContainerActionHandler.cs: https://github.com/actions/runner/blob/main/src/Runner.Worker/Handlers/ContainerActionHandler.cs#L176
423+
func buildDockerEnvironment(c *core.ExecutionState, userEnvs []string) map[string]string {
424+
env := make(map[string]string)
425+
contextEnv := c.GetContextEnvironMapCopy()
426+
427+
// Add allowlisted GITHUB_* vars from context
428+
// https://github.com/actions/runner/blob/main/src/Runner.Worker/GitHubContext.cs#L148-L160
429+
if c.IsGitHubWorkflow {
430+
for key := range githubContextAllowlist {
431+
envKey := "GITHUB_" + strings.ToUpper(key)
432+
if val, ok := contextEnv[envKey]; ok {
433+
env[envKey] = val
434+
}
435+
}
436+
437+
// also add RUNNER_* vars if present
438+
for key, val := range contextEnv {
439+
if strings.HasPrefix(key, "RUNNER_") {
440+
env[key] = val
441+
}
442+
}
443+
}
444+
445+
// add hardcoded variables (always set for Docker containers)
446+
// https://github.com/actions/runner/blob/main/src/Runner.Worker/Container/DockerCommandManager.cs#L329-L336
447+
env["GITHUB_ACTIONS"] = "true"
448+
if _, exists := env["CI"]; !exists {
449+
env["CI"] = "true"
450+
}
451+
452+
// set HOME to container path (GitHub Actions sets this to /github/home)
453+
// https://github.com/actions/runner/blob/main/src/Runner.Worker/Handlers/ContainerActionHandler.cs#L176
454+
if c.IsGitHubWorkflow {
455+
env["HOME"] = "/github/home"
456+
}
457+
458+
// 4. add proxy variables if set in host environment
459+
// https://github.com/actions/runner/blob/main/src/Runner.Worker/Container/ContainerInfo.cs#L105-L130
460+
proxyVars := []string{"HTTP_PROXY", "http_proxy", "HTTPS_PROXY", "https_proxy", "NO_PROXY", "no_proxy"}
461+
for _, proxyVar := range proxyVars {
462+
if val, ok := contextEnv[proxyVar]; ok && val != "" {
463+
env[proxyVar] = val
464+
}
465+
}
466+
467+
// 5. add user-defined env vars from node inputs. Highest prio, can override above
468+
for _, e := range userEnvs {
469+
if idx := strings.Index(e, "="); idx > 0 {
470+
env[e[:idx]] = e[idx+1:]
471+
}
472+
}
473+
474+
return env
475+
}
476+
413477
func init() {
414478
err := core.RegisterNodeFactory(dockerDefinition, func(ctx any, parent core.NodeBaseInterface, parentId string, nodeDef map[string]any, validate bool, opts core.RunOpts) (core.NodeBaseInterface, []error) {
415479
return &DockerNode{}, nil

0 commit comments

Comments
 (0)