mirror of
https://gitea.com/gitea/act_runner.git
synced 2026-07-02 18:57:08 +08:00
fix: prevent exponential growth of RunContext masks in composite actions (#1059)
A composite action's `RunContext` is **seeded with its parent's `Masks` slice** in `newCompositeRunContext`. After the composite finished running, the *entire* `compositeRC.Masks` (the seeded parent masks plus any added during the run) was appended straight back into the parent: ```go rc.Masks = append(rc.Masks, compositeRC.Masks...) ``` Because the composite already contained the parent's masks, every composite action roughly **doubled** the parent's `Masks` length. With many nested/repeated composite actions this grows exponentially: ``` before=1152 after=2304 compositeLen=1152 before=2304 after=4608 compositeLen=2304 before=4608 after=9216 compositeLen=4608 ... ``` ### Impact The bloated `Masks` slice made per-log-line secret redaction (`strings.makeGenericReplacer`, rebuilt per line since #1001) extremely slow. Log writes fell behind enough to keep subprocess pipes open after the process had already exited, surfacing as seemingly random CI failures — most often in later steps of mask-heavy workflows: ``` Error: exec: WaitDelay expired before I/O complete ``` The split of `cmd.Run()` into `cmd.Start()` + `cmd.Wait()` (#883) is what turned this long-standing latency into a hard failure. ### Fix Only append masks that aren't already present in the parent, via a small `appendUniqueMasks` helper. This keeps the mask count bounded by the number of *distinct* secrets instead of growing exponentially. I kept `Masks` as a `[]string` rather than switching to a set type, since it's passed by pointer to loggers across `logger.go`, `run_context.go`, and `runner.go` — a type change would be a much wider, riskier refactor. The `O(n·m)` dedup is harmless precisely because `n` now stays small. Fixes #1054Reviewed-on: https://gitea.com/gitea/runner/pulls/1059 Reviewed-by: Lunny Xiao <xiaolunwen@gmail.com>
This commit is contained in:
@@ -8,6 +8,7 @@ import (
|
||||
"context"
|
||||
"errors"
|
||||
"regexp"
|
||||
"slices"
|
||||
"strconv"
|
||||
"strings"
|
||||
|
||||
@@ -85,6 +86,19 @@ func newCompositeRunContext(ctx context.Context, parent *RunContext, step action
|
||||
return compositerc
|
||||
}
|
||||
|
||||
// appendUniqueMasks appends the masks from src to dst, skipping any mask that
|
||||
// is already present in dst. This prevents the parent RunContext's Masks slice
|
||||
// from growing exponentially when composite actions are nested or repeated,
|
||||
// since each composite RunContext is seeded with its parent's masks.
|
||||
func appendUniqueMasks(dst, src []string) []string {
|
||||
for _, m := range src {
|
||||
if !slices.Contains(dst, m) {
|
||||
dst = append(dst, m)
|
||||
}
|
||||
}
|
||||
return dst
|
||||
}
|
||||
|
||||
func execAsComposite(step actionStep) common.Executor {
|
||||
rc := step.getRunContext()
|
||||
action := step.getActionModel()
|
||||
@@ -110,7 +124,11 @@ func execAsComposite(step actionStep) common.Executor {
|
||||
}, eval.Interpolate(ctx, output.Value))
|
||||
}
|
||||
|
||||
rc.Masks = append(rc.Masks, compositeRC.Masks...)
|
||||
// compositeRC.Masks is seeded with rc.Masks (see newCompositeRunContext)
|
||||
// and may have additional masks appended while the composite action runs.
|
||||
// Only append masks that are not already present, otherwise nested or
|
||||
// repeated composite actions grow rc.Masks exponentially.
|
||||
rc.Masks = appendUniqueMasks(rc.Masks, compositeRC.Masks)
|
||||
rc.ExtraPath = compositeRC.ExtraPath
|
||||
// compositeRC.Env is dirty, contains INPUT_ and merged step env, only rely on compositeRC.GlobalEnv
|
||||
mergeIntoMap := mergeIntoMapCaseSensitive
|
||||
|
||||
70
act/runner/action_composite_test.go
Normal file
70
act/runner/action_composite_test.go
Normal file
@@ -0,0 +1,70 @@
|
||||
// Copyright 2026 The Gitea Authors. All rights reserved.
|
||||
// SPDX-License-Identifier: MIT
|
||||
|
||||
package runner
|
||||
|
||||
import (
|
||||
"testing"
|
||||
|
||||
"github.com/stretchr/testify/assert"
|
||||
)
|
||||
|
||||
func TestAppendUniqueMasks(t *testing.T) {
|
||||
tests := []struct {
|
||||
name string
|
||||
dst []string
|
||||
src []string
|
||||
want []string
|
||||
}{
|
||||
{
|
||||
name: "appends new masks",
|
||||
dst: []string{"a"},
|
||||
src: []string{"b", "c"},
|
||||
want: []string{"a", "b", "c"},
|
||||
},
|
||||
{
|
||||
name: "skips masks already present",
|
||||
dst: []string{"a", "b"},
|
||||
src: []string{"a", "b"},
|
||||
want: []string{"a", "b"},
|
||||
},
|
||||
{
|
||||
name: "deduplicates within src",
|
||||
dst: []string{"a"},
|
||||
src: []string{"b", "b", "a"},
|
||||
want: []string{"a", "b"},
|
||||
},
|
||||
{
|
||||
name: "empty src leaves dst unchanged",
|
||||
dst: []string{"a"},
|
||||
src: nil,
|
||||
want: []string{"a"},
|
||||
},
|
||||
}
|
||||
|
||||
for _, tt := range tests {
|
||||
t.Run(tt.name, func(t *testing.T) {
|
||||
assert.Equal(t, tt.want, appendUniqueMasks(tt.dst, tt.src))
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
// TestAppendUniqueMasksNoExponentialGrowth reproduces the exponential growth of
|
||||
// the parent's Masks slice observed with nested/repeated composite actions. A
|
||||
// composite RunContext is seeded with its parent's masks and the whole seeded
|
||||
// slice was previously appended back into the parent, doubling its length on
|
||||
// every composite action.
|
||||
func TestAppendUniqueMasksNoExponentialGrowth(t *testing.T) {
|
||||
parentMasks := []string{"secret"}
|
||||
|
||||
for range 20 {
|
||||
// compositeRC.Masks starts as a copy of the parent's masks (it is
|
||||
// seeded with parent.Masks in newCompositeRunContext).
|
||||
compositeMasks := make([]string, len(parentMasks))
|
||||
copy(compositeMasks, parentMasks)
|
||||
|
||||
parentMasks = appendUniqueMasks(parentMasks, compositeMasks)
|
||||
}
|
||||
|
||||
assert.Equal(t, []string{"secret"}, parentMasks)
|
||||
}
|
||||
Reference in New Issue
Block a user