From 763b38ece3fd8b6959d79336efa0fea75701ba74 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Tue, 12 May 2026 18:50:25 +0000 Subject: [PATCH] fix: isolate per-task runner envs (#959) ## Summary - clone the runner environment map for each task before injecting runtime and OIDC tokens - keep the shared base environment immutable so concurrent jobs cannot hit `concurrent map writes` - add a unit test covering task-local env cloning Fixes #958 --------- Co-authored-by: Nicolas Reviewed-on: https://gitea.com/gitea/runner/pulls/959 Reviewed-by: Nicolas --- internal/app/run/runner.go | 19 ++++++--- internal/app/run/runner_env_test.go | 61 +++++++++++++++++++++++++++++ 2 files changed, 75 insertions(+), 5 deletions(-) create mode 100644 internal/app/run/runner_env_test.go diff --git a/internal/app/run/runner.go b/internal/app/run/runner.go index bb67ef9d..3b7b2b0e 100644 --- a/internal/app/run/runner.go +++ b/internal/app/run/runner.go @@ -218,6 +218,14 @@ func (r *Runner) Run(ctx context.Context, task *runnerv1.Task) error { return nil } +func (r *Runner) cloneEnvs() map[string]string { + // +3 reserves space for the per-task keys injected by run(): + // ACTIONS_ID_TOKEN_REQUEST_URL, ACTIONS_ID_TOKEN_REQUEST_TOKEN, ACTIONS_RUNTIME_TOKEN. + envs := make(map[string]string, len(r.envs)+3) + maps.Copy(envs, r.envs) + return envs +} + // getDefaultActionsURL // when DEFAULT_ACTIONS_URL == "https://github.com" and GithubMirror is not blank, // it should be set to GithubMirror first. @@ -251,6 +259,7 @@ func (r *Runner) run(ctx context.Context, task *runnerv1.Task, reporter *report. reporter.ResetSteps(len(job.Steps)) taskContext := task.Context.Fields + envs := r.cloneEnvs() log.Infof("task %v repo is %v %v %v", task.Id, taskContext["repository"].GetStringValue(), r.getDefaultActionsURL(task), @@ -281,9 +290,9 @@ func (r *Runner) run(ctx context.Context, task *runnerv1.Task, reporter *report. } if actionsIDTokenRequestURL := taskContext["actions_id_token_request_url"].GetStringValue(); actionsIDTokenRequestURL != "" { - r.envs["ACTIONS_ID_TOKEN_REQUEST_URL"] = actionsIDTokenRequestURL - r.envs["ACTIONS_ID_TOKEN_REQUEST_TOKEN"] = taskContext["actions_id_token_request_token"].GetStringValue() - task.Secrets["ACTIONS_ID_TOKEN_REQUEST_TOKEN"] = r.envs["ACTIONS_ID_TOKEN_REQUEST_TOKEN"] + envs["ACTIONS_ID_TOKEN_REQUEST_URL"] = actionsIDTokenRequestURL + envs["ACTIONS_ID_TOKEN_REQUEST_TOKEN"] = taskContext["actions_id_token_request_token"].GetStringValue() + task.Secrets["ACTIONS_ID_TOKEN_REQUEST_TOKEN"] = envs["ACTIONS_ID_TOKEN_REQUEST_TOKEN"] } giteaRuntimeToken := taskContext["gitea_runtime_token"].GetStringValue() @@ -291,7 +300,7 @@ func (r *Runner) run(ctx context.Context, task *runnerv1.Task, reporter *report. // use task token to action api token for previous Gitea Server Versions giteaRuntimeToken = preset.Token } - r.envs["ACTIONS_RUNTIME_TOKEN"] = giteaRuntimeToken + envs["ACTIONS_RUNTIME_TOKEN"] = giteaRuntimeToken // Mask the runtime token so it cannot be echoed in user step output; it is // now also the cache server's bearer credential and leaking it would let // any reader of the log impersonate this job against the cache. @@ -344,7 +353,7 @@ func (r *Runner) run(ctx context.Context, task *runnerv1.Task, reporter *report. ForceRebuild: r.cfg.Container.ForceRebuild, LogOutput: true, JSONLogger: false, - Env: r.envs, + Env: envs, Secrets: task.Secrets, GitHubInstance: strings.TrimSuffix(r.client.Address(), "/"), AutoRemove: true, diff --git a/internal/app/run/runner_env_test.go b/internal/app/run/runner_env_test.go new file mode 100644 index 00000000..27d9c8fc --- /dev/null +++ b/internal/app/run/runner_env_test.go @@ -0,0 +1,61 @@ +// Copyright 2026 The Gitea Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package run + +import ( + "sync" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestRunnerCloneEnvsReturnsTaskLocalCopy(t *testing.T) { + r := &Runner{ + envs: map[string]string{ + "ACTIONS_CACHE_URL": "http://cache.example", + "ACTIONS_RUNTIME_URL": "http://runner.example", + }, + } + + cloned := r.cloneEnvs() + require.Equal(t, r.envs, cloned) + + cloned["ACTIONS_RUNTIME_TOKEN"] = "task-token" + cloned["ACTIONS_ID_TOKEN_REQUEST_URL"] = "http://oidc.example" + + assert.NotContains(t, r.envs, "ACTIONS_RUNTIME_TOKEN") + assert.NotContains(t, r.envs, "ACTIONS_ID_TOKEN_REQUEST_URL") + assert.Equal(t, "http://cache.example", r.envs["ACTIONS_CACHE_URL"]) +} + +// Regression test for #958: concurrent tasks writing task-specific env keys +// used to race on the shared r.envs map and crash the runner with +// "fatal error: concurrent map writes". Each task must mutate its own clone. +func TestRunnerCloneEnvsConcurrentMutation(t *testing.T) { + r := &Runner{ + envs: map[string]string{ + "ACTIONS_CACHE_URL": "http://cache.example", + "ACTIONS_RUNTIME_URL": "http://runner.example", + }, + } + + const goroutines = 16 + var wg sync.WaitGroup + wg.Add(goroutines) + for range goroutines { + go func() { + defer wg.Done() + envs := r.cloneEnvs() + envs["ACTIONS_RUNTIME_TOKEN"] = "task-token" + envs["ACTIONS_ID_TOKEN_REQUEST_URL"] = "http://oidc.example" + envs["ACTIONS_ID_TOKEN_REQUEST_TOKEN"] = "oidc-token" + }() + } + wg.Wait() + + assert.NotContains(t, r.envs, "ACTIONS_RUNTIME_TOKEN") + assert.NotContains(t, r.envs, "ACTIONS_ID_TOKEN_REQUEST_URL") + assert.NotContains(t, r.envs, "ACTIONS_ID_TOKEN_REQUEST_TOKEN") +}