mirror of
https://code.forgejo.org/forgejo/runner.git
synced 2025-09-05 18:40:59 +00:00
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 <earl-warren@noreply.code.forgejo.org> Co-authored-by: Mathieu Fenniak <mathieu@fenniak.net> Co-committed-by: Mathieu Fenniak <mathieu@fenniak.net>
This commit is contained in:
parent
022d5ad3e7
commit
fa42b8394e
1 changed files with 9 additions and 9 deletions
|
@ -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
|
||||
|
|
Loading…
Add table
Add a link
Reference in a new issue