fix(executor): reuse resolutions across local composites and guard local composite recursion

commit: 6cc6c69786860cdfcf5dc16554659d7cde53788d
author: brice · 8d ago · 2026-10-14 03:04 UTC
parent: acae6da
signature: GPG VerifiedCD88F9E564E0102646FB6077A1A07C7C7C3E349D · arch-gpg Verified for the committer email

4 files · +107 -19

M internal/executor/actions_composite.go +18 -8


@@ -19,6 +19,11 @@ func compositeGitHubKeys() []string {
19	19		return keys
20	20		}
21	21
	22	+	// maxLocalDepth is the nesting of composite actions the executor allows. It is
	23	+	// ermite's guard against a local composite action that uses itself, not a limit
	24	+	// of GitHub's, whose runner has none for local actions.
	25	+	const maxLocalDepth = 100
	26	+
22	27		// prepareLocalComposite is the first part of ActionRunner.RunAsync for a local
23	28		// composite action (ActionRunner.cs:77-96): its steps are prepared when the
24	29		// step runs, in the log of the step, and the pulls and builds they need run

@@ -28,14 +33,19 @@ func (j job) prepareLocalComposite(s stepRun, def *actionDef, cancel <-chan st
28	33		if n.children != nil {
29	34		return nil // pre and post stages reuse the nodes of the main stage
30	35		}
31		-	// Depth restarts at 1 for the steps of a local composite action, as in the
32		-	// runner (ActionRunner.cs:77-96 calls PrepareActionsAsync with depth 1), so a
33		-	// chain of local composites has no depth limit here
34		-	// (TestLocalCompositeChainHasNoDepthLimit). Whether github.com limits it is
35		-	// to be settled by the recording of ermite-conformance
36		-	// corpus/act-composite-depth. Every level that needs a resolution is counted
37		-	// by the host's per-job budget (host/resolve.go), so the server is safe
38		-	// whatever is decided.
	36	+	// Local composite actions have no depth limit on GitHub: the recording of
	37	+	// ermite-conformance corpus/act-composite-depth has composites of 9, 10 and 12
	38	+	// levels, all of which succeed. The runner prepares the steps of a local
	39	+	// composite when its step runs, with the depth restarting at 1
	40	+	// (ActionRunner.cs:77-96), which is why. This port does the same, and the
	41	+	// resolutions it needs are cached and budgeted by the host (host/resolve.go).
	42	+	//
	43	+	// The one guard that is ermite's own, not GitHub's: a local composite that
	44	+	// uses itself (directly or in a cycle) would recurse until the executor
	45	+	// crashes, so the nesting of composite actions is capped at maxLocalDepth.
	46	+	if d := s.depth(); d >= maxLocalDepth {
	47	+	return ghErrorf("Composite action depth exceeded max depth %d", maxLocalDepth)
	48	+	}
39	49		kids, err := newNodes(def.act.Runs.Steps, n, n.scope)
40	50		if err != nil {
41	51		return err

 	return keys
 }
+// maxLocalDepth is the nesting of composite actions the executor allows. It is
+// ermite's guard against a local composite action that uses itself, not a limit
+// of GitHub's, whose runner has none for local actions.
+const maxLocalDepth = 100
 // prepareLocalComposite is the first part of ActionRunner.RunAsync for a local
 // composite action (ActionRunner.cs:77-96): its steps are prepared when the
 // step runs, in the log of the step, and the pulls and builds they need run
 	if n.children != nil {
 		return nil // pre and post stages reuse the nodes of the main stage
 	}
-	// Depth restarts at 1 for the steps of a local composite action, as in the
+	// Local composite actions have no depth limit on GitHub: the recording of
-	// runner (ActionRunner.cs:77-96 calls PrepareActionsAsync with depth 1), so a
+	// ermite-conformance corpus/act-composite-depth has composites of 9, 10 and 12
-	// chain of local composites has no depth limit here
+	// levels, all of which succeed. The runner prepares the steps of a local
-	// (TestLocalCompositeChainHasNoDepthLimit). Whether github.com limits it is
+	// composite when its step runs, with the depth restarting at 1
-	// to be settled by the recording of ermite-conformance
+	// (ActionRunner.cs:77-96), which is why. This port does the same, and the
-	// corpus/act-composite-depth. Every level that needs a resolution is counted
+	// resolutions it needs are cached and budgeted by the host (host/resolve.go).
-	// by the host's per-job budget (host/resolve.go), so the server is safe
+	//
-	// whatever is decided.
+	// The one guard that is ermite's own, not GitHub's: a local composite that
+	// uses itself (directly or in a cycle) would recurse until the executor
+	// crashes, so the nesting of composite actions is capped at maxLocalDepth.
+	if d := s.depth(); d >= maxLocalDepth {
+		return ghErrorf("Composite action depth exceeded max depth %d", maxLocalDepth)
+	}
 	kids, err := newNodes(def.act.Runs.Steps, n, n.scope)
 	if err != nil {
 		return err

M internal/executor/actions_prepare.go +19 -1


@@ -26,6 +26,9 @@ type actionState struct {
26	26		// resolved caches the download info by "name@ref" (ActionManager's legacy
27	27		// path asks again at every level; ermite's server caches per run).
28	28		downloaded map[string]bool
	29	+	// infos are the resolutions of the job by "Name@Ref": an action that a local
	30	+	// composite action brings again is not asked for again.
	31	+	infos map[string]ResolvedAction
29	32		}
30	33
31	34		type buildInfo struct{ repository, dockerfile, workDir string }

@@ -34,7 +37,7 @@ func newActionState() *actionState {
34	37		return &actionState{
35	38		pulled: map[string]string{}, built: map[string]string{}, buildInfo: map[string]buildInfo{},
36	39		pullUsers: map[string][]node{}, buildUsers: map[string][]node{}, pre: map[*node]bool{},
37		-	downloaded: map[string]bool{},
	40	+	downloaded: map[string]bool{}, infos: map[string]ResolvedAction{},
38	41		}
39	42		}
40	43

@@ -138,6 +141,20 @@ func (j job) downloadInfo(ctx context.Context, num int, nodes []node) (map[str
138	141		if len(uses) == 0 {
139	142		return out, nil
140	143		}
	144	+	// What the job resolved already is not asked again: the host would not call
	145	+	// the server for it either (host/resolve.go), but the round trip is saved.
	146	+	var askUses, askKeys []string
	147	+	for i, k := range keys {
	148	+	if ra, ok := j.actions.infos[k]; ok {
	149	+	out[k] = ra
	150	+	continue
	151	+	}
	152	+	askUses, askKeys = append(askUses, uses[i]), append(askKeys, k)
	153	+	}
	154	+	if len(askUses) == 0 {
	155	+	return out, nil
	156	+	}
	157	+	uses, keys = askUses, askKeys
141	158		if j.cfg.Actions.Resolver == nil {
142	159		return nil, ghErrorf("Failed to resolve action download info.")
143	160		}

@@ -158,6 +175,7 @@ func (j job) downloadInfo(ctx context.Context, num int, nodes []node) (map[str
158	175		ra.NameWithOwner = strings.SplitN(keys[i], "@", 2)[0]
159	176		}
160	177		out[keys[i]] = ra
	178	+	j.actions.infos[keys[i]] = ra
161	179		}
162	180		return out, nil
163	181		}

 	// resolved caches the download info by "name@ref" (ActionManager's legacy
 	// path asks again at every level; ermite's server caches per run).
 	downloaded map[string]bool
+	// infos are the resolutions of the job by "Name@Ref": an action that a local
+	// composite action brings again is not asked for again.
+	infos map[string]ResolvedAction
 }
 type buildInfo struct{ repository, dockerfile, workDir string }
 	return &actionState{
 		pulled: map[string]string{}, built: map[string]string{}, buildInfo: map[string]buildInfo{},
 		pullUsers: map[string][]*node{}, buildUsers: map[string][]*node{}, pre: map[*node]bool{},
-		downloaded: map[string]bool{},
+		downloaded: map[string]bool{}, infos: map[string]ResolvedAction{},
 	}
 }
 	if len(uses) == 0 {
 		return out, nil
 	}
+	// What the job resolved already is not asked again: the host would not call
+	// the server for it either (host/resolve.go), but the round trip is saved.
+	var askUses, askKeys []string
+	for i, k := range keys {
+		if ra, ok := j.actions.infos[k]; ok {
+			out[k] = ra
+			continue
+		}
+		askUses, askKeys = append(askUses, uses[i]), append(askKeys, k)
+	}
+	if len(askUses) == 0 {
+		return out, nil
+	}
+	uses, keys = askUses, askKeys
 	if j.cfg.Actions.Resolver == nil {
 		return nil, ghErrorf("Failed to resolve action download info.")
 	}
 			ra.NameWithOwner = strings.SplitN(keys[i], "@", 2)[0]
 		}
 		out[keys[i]] = ra
+		j.actions.infos[keys[i]] = ra
 	}
 	return out, nil
 }

M internal/executor/actions_test.go +61 -10


@@ -11,6 +11,7 @@ import (
11	11		"net/http/httptest"
12	12		"os"
13	13		"path/filepath"
	14	+	"strconv"
14	15		"strings"
15	16		"sync"
16	17		"sync/atomic"

@@ -429,18 +430,68 @@ func TestCompositeDepthLimit(t *testing.T) {
429	430		}
430	431		}
431	432
432		-	func TestLocalCompositeChainHasNoDepthLimit(t *testing.T) {
433		-	tree := files{}
434		-	for i := 0; i < 12; i++ {
435		-	next := ""
436		-	if i < 11 {
437		-	next = " - uses: ./d" + string(rune('a'+i+1)) + "\n"
	433	+	// Local composites are not limited on GitHub (the recording act-composite-depth
	434	+	// has 9, 10 and 12 levels succeed); only ermite's guard against recursion stops
	435	+	// a composite that uses itself.
	436	+	func TestLocalCompositeDepth(t *testing.T) {
	437	+	chain := func(n int) files {
	438	+	tree := files{}
	439	+	for i := 0; i < n; i++ {
	440	+	next := ""
	441	+	if i < n-1 {
	442	+	next = " - uses: ./d" + strconv.Itoa(i+1) + "\n"
	443	+	}
	444	+	tree["d"+strconv.Itoa(i)+"/action.yml"] = "name: d\nruns:\n using: composite\n steps:\n - shell: bash\n run: echo level\n" + next
	445	+	}
	446	+	return tree
	447	+	}
	448	+	for _, n := range []int{9, 10, 12} {
	449	+	rec, res := actionRun(t, chain(n), " - uses: ./d0\n", nil)
	450	+	if res.Conclusion != ConclusionSuccess \|\| strings.Count(logOf(rec.step(t, "Run ./d0")), "\nlevel") != n {
	451	+	t.Fatalf("%d levels: %+v\n%s", n, res, logOf(rec.step(t, "Run ./d0")))
438	452		}
439		-	tree["d"+string(rune('a'+i))+"/action.yml"] = "name: d\nruns:\n using: composite\n steps:\n - shell: bash\n run: echo level\n" + next
440	453		}
441		-	rec, res := actionRun(t, tree, " - uses: ./da\n", nil)
442		-	if res.Conclusion != ConclusionSuccess \|\| strings.Count(logOf(rec.step(t, "Run ./da")), "\nlevel") != 12 {
443		-	t.Fatalf("%+v\n%s", res, logOf(rec.step(t, "Run ./da")))
	454	+	// Up to the guard.
	455	+	if _, res := actionRun(t, chain(maxLocalDepth), " - uses: ./d0\n", nil); res.Conclusion != ConclusionSuccess {
	456	+	t.Fatalf("%d levels: %+v", maxLocalDepth, res)
	457	+	}
	458	+	// A composite that uses itself fails cleanly, and the job goes on.
	459	+	self := files{"s/action.yml": "name: s\nruns:\n using: composite\n steps:\n - uses: ./s\n"}
	460	+	rec, res := actionRun(t, self, " - uses: ./s\n - if: always()\n run: echo after\n", nil)
	461	+	if res.Conclusion != ConclusionFailure {
	462	+	t.Fatalf("%+v", res)
	463	+	}
	464	+	if got := logOf(rec.step(t, "Run ./s")); strings.Count(got, "##[error]Composite action depth exceeded max depth 100") != 1 {
	465	+	t.Errorf("%s", got[:min(len(got), 600)])
	466	+	}
	467	+	if got := rec.step(t, "Run echo after").Conclusion; got != ConclusionSuccess {
	468	+	t.Errorf("the step after: %s", got)
	469	+	}
	470	+	// Two actions that use each other.
	471	+	cyc := files{
	472	+	"a/action.yml": "name: a\nruns:\n using: composite\n steps:\n - uses: ./b\n",
	473	+	"b/action.yml": "name: b\nruns:\n using: composite\n steps:\n - uses: ./a\n",
	474	+	}
	475	+	if _, res := actionRun(t, cyc, " - uses: ./a\n", nil); res.Conclusion != ConclusionFailure {
	476	+	t.Errorf("cycle: %+v", res)
	477	+	}
	478	+	}
	479	+
	480	+	// A repository composite used twice, and by a local composite used many times,
	481	+	// is resolved and downloaded once.
	482	+	func TestResolutionsAreReusedAcrossLocalComposites(t *testing.T) {
	483	+	forge := newForge(t, map[string][]byte{"/tarball/o/leaf/v1": tarOf(t, "o-leaf-x", files{"action.yml": nodeAction, "main.sh": "echo leaf\n"})})
	484	+	tree := files{"c/action.yml": "name: c\nruns:\n using: composite\n steps:\n - uses: o/leaf@v1\n"}
	485	+	var yaml string
	486	+	for i := 0; i < 5; i++ {
	487	+	yaml += " - uses: ./c\n"
	488	+	}
	489	+	rec, res := actionRun(t, tree, yaml, func(c *Config) { c.Actions.Resolver = forge.resolver("") })
	490	+	if res.Conclusion != ConclusionSuccess \|\| strings.Count(logOf(rec.steps[1])+logOf(rec.steps[2])+logOf(rec.steps[5]), "leaf") < 3 {
	491	+	t.Fatalf("%+v %v", res, rec.names())
	492	+	}
	493	+	if len(forge.asked) != 1 \|\| forge.hits.Load() != 1 {
	494	+	t.Errorf("asked %v, downloaded %d times: once is enough", forge.asked, forge.hits.Load())
444	495		}
445	496		}
446	497


@@ -11,6 +11,7 @@ import (
11		"net/http/httptest"	11		"net/http/httptest"
12		"os"	12		"os"
13		"path/filepath"	13		"path/filepath"
			14	+	"strconv"
14		"strings"	15		"strings"
15		"sync"	16		"sync"
16		"sync/atomic"	17		"sync/atomic"

@@ -429,18 +430,68 @@ func TestCompositeDepthLimit(t *testing.T) {
429		}	430		}
430		}	431		}
431			432
432	-	func TestLocalCompositeChainHasNoDepthLimit(t *testing.T) {	433	+	// Local composites are not limited on GitHub (the recording act-composite-depth
433	-	tree := files{}	434	+	// has 9, 10 and 12 levels succeed); only ermite's guard against recursion stops
434	-	for i := 0; i < 12; i++ {	435	+	// a composite that uses itself.
435	-	next := ""	436	+	func TestLocalCompositeDepth(t *testing.T) {
436	-	if i < 11 {	437	+	chain := func(n int) files {
437	-	next = " - uses: ./d" + string(rune('a'+i+1)) + "\n"	438	+	tree := files{}
			439	+	for i := 0; i < n; i++ {
			440	+	next := ""
			441	+	if i < n-1 {
			442	+	next = " - uses: ./d" + strconv.Itoa(i+1) + "\n"
			443	+	}
			444	+	tree["d"+strconv.Itoa(i)+"/action.yml"] = "name: d\nruns:\n using: composite\n steps:\n - shell: bash\n run: echo level\n" + next
			445	+	}
			446	+	return tree
			447	+	}
			448	+	for _, n := range []int{9, 10, 12} {
			449	+	rec, res := actionRun(t, chain(n), " - uses: ./d0\n", nil)
			450	+	if res.Conclusion != ConclusionSuccess \|\| strings.Count(logOf(rec.step(t, "Run ./d0")), "\nlevel") != n {
			451	+	t.Fatalf("%d levels: %+v\n%s", n, res, logOf(rec.step(t, "Run ./d0")))
438		}	452		}
439	-	tree["d"+string(rune('a'+i))+"/action.yml"] = "name: d\nruns:\n using: composite\n steps:\n - shell: bash\n run: echo level\n" + next
440		}	453		}
441	-	rec, res := actionRun(t, tree, " - uses: ./da\n", nil)	454	+	// Up to the guard.
442	-	if res.Conclusion != ConclusionSuccess \|\| strings.Count(logOf(rec.step(t, "Run ./da")), "\nlevel") != 12 {	455	+	if _, res := actionRun(t, chain(maxLocalDepth), " - uses: ./d0\n", nil); res.Conclusion != ConclusionSuccess {
443	-	t.Fatalf("%+v\n%s", res, logOf(rec.step(t, "Run ./da")))	456	+	t.Fatalf("%d levels: %+v", maxLocalDepth, res)
			457	+	}
			458	+	// A composite that uses itself fails cleanly, and the job goes on.
			459	+	self := files{"s/action.yml": "name: s\nruns:\n using: composite\n steps:\n - uses: ./s\n"}
			460	+	rec, res := actionRun(t, self, " - uses: ./s\n - if: always()\n run: echo after\n", nil)
			461	+	if res.Conclusion != ConclusionFailure {
			462	+	t.Fatalf("%+v", res)
			463	+	}
			464	+	if got := logOf(rec.step(t, "Run ./s")); strings.Count(got, "##[error]Composite action depth exceeded max depth 100") != 1 {
			465	+	t.Errorf("%s", got[:min(len(got), 600)])
			466	+	}
			467	+	if got := rec.step(t, "Run echo after").Conclusion; got != ConclusionSuccess {
			468	+	t.Errorf("the step after: %s", got)
			469	+	}
			470	+	// Two actions that use each other.
			471	+	cyc := files{
			472	+	"a/action.yml": "name: a\nruns:\n using: composite\n steps:\n - uses: ./b\n",
			473	+	"b/action.yml": "name: b\nruns:\n using: composite\n steps:\n - uses: ./a\n",
			474	+	}
			475	+	if _, res := actionRun(t, cyc, " - uses: ./a\n", nil); res.Conclusion != ConclusionFailure {
			476	+	t.Errorf("cycle: %+v", res)
			477	+	}
			478	+	}
			479	+
			480	+	// A repository composite used twice, and by a local composite used many times,
			481	+	// is resolved and downloaded once.
			482	+	func TestResolutionsAreReusedAcrossLocalComposites(t *testing.T) {
			483	+	forge := newForge(t, map[string][]byte{"/tarball/o/leaf/v1": tarOf(t, "o-leaf-x", files{"action.yml": nodeAction, "main.sh": "echo leaf\n"})})
			484	+	tree := files{"c/action.yml": "name: c\nruns:\n using: composite\n steps:\n - uses: o/leaf@v1\n"}
			485	+	var yaml string
			486	+	for i := 0; i < 5; i++ {
			487	+	yaml += " - uses: ./c\n"
			488	+	}
			489	+	rec, res := actionRun(t, tree, yaml, func(c *Config) { c.Actions.Resolver = forge.resolver("") })
			490	+	if res.Conclusion != ConclusionSuccess \|\| strings.Count(logOf(rec.steps[1])+logOf(rec.steps[2])+logOf(rec.steps[5]), "leaf") < 3 {
			491	+	t.Fatalf("%+v %v", res, rec.names())
			492	+	}
			493	+	if len(forge.asked) != 1 \|\| forge.hits.Load() != 1 {
			494	+	t.Errorf("asked %v, downloaded %d times: once is enough", forge.asked, forge.hits.Load())
444		}	495		}
445		}	496		}
446			497

M internal/executor/step.go +9 -0


@@ -47,6 +47,15 @@ type stepRun struct {
47	47		nodeVersion string
48	48		}
49	49
	50	+	// depth is the number of composite actions around the step.
	51	+	func (s *stepRun) depth() int {
	52	+	d := 0
	53	+	for p := s.parent; p != nil; p = p.parent {
	54	+	d++
	55	+	}
	56	+	return d
	57	+	}
	58	+
50	59		// procEnv is the environment of the process the step starts.
51	60		func (s *stepRun) procEnv() []string {
52	61		if s.rawEnvList != nil {


@@ -47,6 +47,15 @@ type stepRun struct {
47	nodeVersion string	47		nodeVersion string
48	}	48		}
49		49
		50	+	// depth is the number of composite actions around the step.
		51	+	func (s *stepRun) depth() int {
		52	+	d := 0
		53	+	for p := s.parent; p != nil; p = p.parent {
		54	+	d++
		55	+	}
		56	+	return d
		57	+	}
		58	+
50	// procEnv is the environment of the process the step starts.	59		// procEnv is the environment of the process the step starts.
51	func (s *stepRun) procEnv() []string {	60		func (s *stepRun) procEnv() []string {
52	if s.rawEnvList != nil {	61		if s.rawEnvList != nil {