mirror of
https://gitea.com/gitea/act_runner.git
synced 2026-05-13 19:33:23 +02:00
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 <bircni@icloud.com> Reviewed-on: https://gitea.com/gitea/runner/pulls/959 Reviewed-by: Nicolas <bircni@icloud.com>
This commit is contained in:
@@ -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,
|
||||
|
||||
61
internal/app/run/runner_env_test.go
Normal file
61
internal/app/run/runner_env_test.go
Normal file
@@ -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")
|
||||
}
|
||||
Reference in New Issue
Block a user