From fa42b8394ee4afb1059f961b2ed23720a567fa92 Mon Sep 17 00:00:00 2001 From: Mathieu Fenniak Date: Wed, 20 Aug 2025 13:34:57 +0000 Subject: [PATCH] chore: fix data race in reusable workflows with inherited secrets (#875) Data race detection identified that `getWorkflowSecrets` is mutating `rc.caller.runContext.Config.Secrets` while interpolating values, in the case where secrets are inherited by a reusable workflow. This map is also mutated earlier in evaluation by `(*RunContext).handleCredentials`. It's possible that multiple goroutines performing mutation to this shared map could cause runtime panics (not observed). The issue is addressed creating a separate map to store interpolated secrets in `getWorkflowSecrets`, which was already the behavior in the non-inherited secret case. Automated testing for this issue will be provided by #861 when all data races are resolved. ``` ================== WARNING: DATA RACE Read at 0x00c0003a9620 by goroutine 2546: runtime.mapaccess1_faststr() /home/mfenniak/go/pkg/mod/golang.org/toolchain@v0.0.1-go1.24.6.linux-amd64/src/internal/runtime/maps/runtime_faststr_swiss.go:103 +0x0 code.forgejo.org/forgejo/runner/v9/act/runner.(*RunContext).handleCredentials() /.../forgejo-runner/act/runner/run_context.go:1395 +0xab code.forgejo.org/forgejo/runner/v9/act/runner.(*RunContext).prepareJobContainer() /.../forgejo-runner/act/runner/run_context.go:460 +0x2de code.forgejo.org/forgejo/runner/v9/act/runner.(*RunContext).startContainer.func1.(*RunContext).startJobContainer.2() /.../forgejo-runner/act/runner/run_context.go:610 +0x5e code.forgejo.org/forgejo/runner/v9/act/runner.(*RunContext).startContainer.func1() /.../forgejo-runner/act/runner/run_context.go:853 +0xf3 code.forgejo.org/forgejo/runner/v9/act/runner.newJobExecutor.NewPipelineExecutor.Executor.Then.func22() /.../forgejo-runner/act/common/executor.go:136 +0x57 ...snip... Previous write at 0x00c0003a9620 by goroutine 2440: runtime.mapassign_faststr() /home/mfenniak/go/pkg/mod/golang.org/toolchain@v0.0.1-go1.24.6.linux-amd64/src/internal/runtime/maps/runtime_faststr_swiss.go:263 +0x0 code.forgejo.org/forgejo/runner/v9/act/runner.getWorkflowSecrets() /.../forgejo-runner/act/runner/expression.go:578 +0x547 code.forgejo.org/forgejo/runner/v9/act/runner.(*RunContext).NewExpressionEvaluatorWithEnv() /.../forgejo-runner/act/runner/expression.go:85 +0x3fc code.forgejo.org/forgejo/runner/v9/act/common/git.FindGitRevision() /.../forgejo-runner/act/common/git/git.go:70 +0xe4 github.com/go-git/go-git/v5.PlainOpenWithOptions() /home/mfenniak/go/pkg/mod/github.com/go-git/go-git/v5@v5.16.2/repository.go:332 +0x7a6 code.forgejo.org/forgejo/runner/v9/act/common/git.FindGitRevision() /.../forgejo-runner/act/common/git/git.go:58 +0xc4 ...snip... ================== ``` Reviewed-on: https://code.forgejo.org/forgejo/runner/pulls/875 Reviewed-by: earl-warren Co-authored-by: Mathieu Fenniak Co-committed-by: Mathieu Fenniak --- act/runner/expression.go | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/act/runner/expression.go b/act/runner/expression.go index 4fa16e8f..1c52a424 100644 --- a/act/runner/expression.go +++ b/act/runner/expression.go @@ -564,21 +564,21 @@ func setupWorkflowInputs(ctx context.Context, inputs *map[string]any, rc *RunCon func getWorkflowSecrets(ctx context.Context, rc *RunContext) map[string]string { if rc.caller != nil { job := rc.caller.runContext.Run.Job() - secrets := job.Secrets() + rawSecrets := job.Secrets() - if secrets == nil && job.InheritSecrets() { - secrets = rc.caller.runContext.Config.Secrets + if rawSecrets == nil && job.InheritSecrets() { + rawSecrets = rc.caller.runContext.Config.Secrets } - if secrets == nil { - secrets = map[string]string{} + if rawSecrets == nil { + return map[string]string{} } - for k, v := range secrets { - secrets[k] = rc.caller.runContext.ExprEval.Interpolate(ctx, v) + interpolatedSecrets := make(map[string]string, len(rawSecrets)) + for k, v := range rawSecrets { + interpolatedSecrets[k] = rc.caller.runContext.ExprEval.Interpolate(ctx, v) } - - return secrets + return interpolatedSecrets } return rc.Config.Secrets