fix(executor): composite step names see the action inputs, step names masked once

commit: 2e816bd8950ffc9e2f9924ab838fe695524ea632
author: brice · 8d ago · 2026-10-14 13:08 UTC
committer: bnema · 8d ago · 2026-10-14 13:08 UTC
parent: 9382a0f
signature: SSH UnverifiedSHA256:VOCA5lYARHcbKDZ7GhTV45VmcjWnJ6ZQIBazj17LivA The key is unknown

5 files · +131 -4

M internal/executor/actions_composite.go +1 -1


@@ -216,7 +216,7 @@ func (j job) runEmbedded(parent stepRun, c *node, scope string) {
216	216		j.mu.Unlock()
217	217		env, envErr := j.stepEnvironment(s)
218	218		s.env = env
219		-	s.name = j.displayName(c.st, env)
	219	+	s.name = j.displayNameOf(c.st, env, true)
220	220		// The display name is masked before it is cut and escaped. The runner masks
221	221		// the finished line (ExecutionContext.Write), after SanitizeDisplayName and
222	222		// EscapeValue, so a secret holding ";", "]" or "%" (escaped by then) or cut by


@@ -216,7 +216,7 @@ func (j job) runEmbedded(parent stepRun, c *node, scope string) {
216		j.mu.Unlock()	216		j.mu.Unlock()
217		env, envErr := j.stepEnvironment(s)	217		env, envErr := j.stepEnvironment(s)
218		s.env = env	218		s.env = env
219	-	s.name = j.displayName(c.st, env)	219	+	s.name = j.displayNameOf(c.st, env, true)
220		// The display name is masked before it is cut and escaped. The runner masks	220		// The display name is masked before it is cut and escaped. The runner masks
221		// the finished line (ExecutionContext.Write), after SanitizeDisplayName and	221		// the finished line (ExecutionContext.Write), after SanitizeDisplayName and
222		// EscapeValue, so a secret holding ";", "]" or "%" (escaped by then) or cut by	222		// EscapeValue, so a secret holding ";", "]" or "%" (escaped by then) or cut by

M internal/executor/actions_test.go +59 -0


@@ -1241,3 +1241,62 @@ runs:
1241	1241		t.Errorf("end marker:\n%s", log)
1242	1242		}
1243	1243		}
	1244	+
	1245	+	// act-composite-basic: the start marker of an inner step carries its display
	1246	+	// name evaluated with the contexts of the step, the inputs of the composite
	1247	+	// included (CompositeActionHandler.cs:314-318 passes step.ExecutionContext's
	1248	+	// ExpressionValues, not the job message's, which hold the inputs of the
	1249	+	// workflow): recorded `Run echo "greeting=hello world" >> "$GITHUB_OUTPUT"`.
	1250	+	func TestCompositeMarkerNameSeesTheInputsOfTheAction(t *testing.T) {
	1251	+	tree := files{".github/actions/basic/action.yml": `name: basic
	1252	+	inputs:
	1253	+	who:
	1254	+	default: world
	1255	+	runs:
	1256	+	using: composite
	1257	+	steps:
	1258	+	- id: greet
	1259	+	shell: bash
	1260	+	run: \|
	1261	+	echo "greeting=hello ${{ inputs.who }}" >> "$GITHUB_OUTPUT"
	1262	+	`}
	1263	+	rec, res := actionRun(t, tree, " - id: basic\n uses: ./.github/actions/basic\n - id: given\n uses: ./.github/actions/basic\n with:\n who: Ermite\n", nil)
	1264	+	if res.Conclusion != ConclusionSuccess {
	1265	+	t.Fatalf("%+v\n%s", res, logOf(rec.steps[1]))
	1266	+	}
	1267	+	for i, w := range []string{
	1268	+	`##[start-action display=Run echo "greeting=hello world" >> "$GITHUB_OUTPUT";id=basic.greet]`,
	1269	+	`##[start-action display=Run echo "greeting=hello Ermite" >> "$GITHUB_OUTPUT";id=given.greet]`,
	1270	+	} {
	1271	+	if got := logOf(rec.steps[1+i]); !strings.Contains(got, w) {
	1272	+	t.Errorf("step %d: missing %q:\n%s", i+1, w, got)
	1273	+	}
	1274	+	}
	1275	+	}
	1276	+
	1277	+	// act-pre-post, act-post-on-failure: the Post step of a local action prints the
	1278	+	// warning about `pre` too, before "Post job cleanup." (ActionRunner.cs:104-109
	1279	+	// does not look at the stage). GitHub does the same: its jobs carry six such
	1280	+	// annotations (three uses, three Post steps) and five of the six Post logs hold
	1281	+	// the line. The recorded first Post step lacks it only because the harness
	1282	+	// attributes it to the injected dump step, whose log it drops
	1283	+	// (ermite-conformance, movePreHeaderLines).
	1284	+	func TestPostStageOfALocalActionWarnsAboutPreToo(t *testing.T) {
	1285	+	act := "name: pp\nruns:\n using: node20\n pre: pre.sh\n main: main.sh\n post: post.sh\n"
	1286	+	tree := files{".github/actions/pp/action.yml": act,
	1287	+	".github/actions/pp/pre.sh": "echo pre\n", ".github/actions/pp/main.sh": "echo main\n", ".github/actions/pp/post.sh": "echo post\n"}
	1288	+	rec, res := actionRun(t, tree, " - uses: ./.github/actions/pp\n", nil)
	1289	+	if res.Conclusion != ConclusionSuccess {
	1290	+	t.Fatalf("%+v %v", res, rec.names())
	1291	+	}
	1292	+	warn := "##[warning]`pre` execution is not supported for local action from './.github/actions/pp'"
	1293	+	for _, name := range []string{"Run ./.github/actions/pp", "Post Run ./.github/actions/pp"} {
	1294	+	log := rec.step(t, name).Log
	1295	+	if strings.Count(strings.Join(log, "\n"), warn) != 1 {
	1296	+	t.Errorf("%s: want one warning:\n%v", name, log)
	1297	+	}
	1298	+	}
	1299	+	if log := rec.step(t, "Post Run ./.github/actions/pp").Log; len(log) < 2 \|\| log[0] != warn \|\| log[1] != "Post job cleanup." {
	1300	+	t.Errorf("post log starts %v", log[:min(2, len(log))])
	1301	+	}
	1302	+	}


@@ -1241,3 +1241,62 @@ runs:
1241	t.Errorf("end marker:\n%s", log)	1241		t.Errorf("end marker:\n%s", log)
1242	}	1242		}
1243	}	1243		}
		1244	+
		1245	+	// act-composite-basic: the start marker of an inner step carries its display
		1246	+	// name evaluated with the contexts of the step, the inputs of the composite
		1247	+	// included (CompositeActionHandler.cs:314-318 passes step.ExecutionContext's
		1248	+	// ExpressionValues, not the job message's, which hold the inputs of the
		1249	+	// workflow): recorded `Run echo "greeting=hello world" >> "$GITHUB_OUTPUT"`.
		1250	+	func TestCompositeMarkerNameSeesTheInputsOfTheAction(t *testing.T) {
		1251	+	tree := files{".github/actions/basic/action.yml": `name: basic
		1252	+	inputs:
		1253	+	who:
		1254	+	default: world
		1255	+	runs:
		1256	+	using: composite
		1257	+	steps:
		1258	+	- id: greet
		1259	+	shell: bash
		1260	+	run: \|
		1261	+	echo "greeting=hello ${{ inputs.who }}" >> "$GITHUB_OUTPUT"
		1262	+	`}
		1263	+	rec, res := actionRun(t, tree, " - id: basic\n uses: ./.github/actions/basic\n - id: given\n uses: ./.github/actions/basic\n with:\n who: Ermite\n", nil)
		1264	+	if res.Conclusion != ConclusionSuccess {
		1265	+	t.Fatalf("%+v\n%s", res, logOf(rec.steps[1]))
		1266	+	}
		1267	+	for i, w := range []string{
		1268	+	`##[start-action display=Run echo "greeting=hello world" >> "$GITHUB_OUTPUT";id=basic.greet]`,
		1269	+	`##[start-action display=Run echo "greeting=hello Ermite" >> "$GITHUB_OUTPUT";id=given.greet]`,
		1270	+	} {
		1271	+	if got := logOf(rec.steps[1+i]); !strings.Contains(got, w) {
		1272	+	t.Errorf("step %d: missing %q:\n%s", i+1, w, got)
		1273	+	}
		1274	+	}
		1275	+	}
		1276	+
		1277	+	// act-pre-post, act-post-on-failure: the Post step of a local action prints the
		1278	+	// warning about `pre` too, before "Post job cleanup." (ActionRunner.cs:104-109
		1279	+	// does not look at the stage). GitHub does the same: its jobs carry six such
		1280	+	// annotations (three uses, three Post steps) and five of the six Post logs hold
		1281	+	// the line. The recorded first Post step lacks it only because the harness
		1282	+	// attributes it to the injected dump step, whose log it drops
		1283	+	// (ermite-conformance, movePreHeaderLines).
		1284	+	func TestPostStageOfALocalActionWarnsAboutPreToo(t *testing.T) {
		1285	+	act := "name: pp\nruns:\n using: node20\n pre: pre.sh\n main: main.sh\n post: post.sh\n"
		1286	+	tree := files{".github/actions/pp/action.yml": act,
		1287	+	".github/actions/pp/pre.sh": "echo pre\n", ".github/actions/pp/main.sh": "echo main\n", ".github/actions/pp/post.sh": "echo post\n"}
		1288	+	rec, res := actionRun(t, tree, " - uses: ./.github/actions/pp\n", nil)
		1289	+	if res.Conclusion != ConclusionSuccess {
		1290	+	t.Fatalf("%+v %v", res, rec.names())
		1291	+	}
		1292	+	warn := "##[warning]`pre` execution is not supported for local action from './.github/actions/pp'"
		1293	+	for _, name := range []string{"Run ./.github/actions/pp", "Post Run ./.github/actions/pp"} {
		1294	+	log := rec.step(t, name).Log
		1295	+	if strings.Count(strings.Join(log, "\n"), warn) != 1 {
		1296	+	t.Errorf("%s: want one warning:\n%v", name, log)
		1297	+	}
		1298	+	}
		1299	+	if log := rec.step(t, "Post Run ./.github/actions/pp").Log; len(log) < 2 \|\| log[0] != warn \|\| log[1] != "Post job cleanup." {
		1300	+	t.Errorf("post log starts %v", log[:min(2, len(log))])
		1301	+	}
		1302	+	}

M internal/executor/masker.go +29 -2


@@ -296,10 +296,26 @@ func (m *masker) has(s string) bool { return m != nil && m.mask(s) != s }
296	296		type maskReporter struct {
297	297		j *job
298	298		next Reporter
	299	+
	300	+	mu sync.Mutex
	301	+	names map[int]string // the masked name each step started with
299	302		}
300	303
	304	+	// StepStarted masks the name with the secrets known when the step starts, which
	305	+	// is when the runner masks it: ActionRunner.EvaluateDisplayName masks the name
	306	+	// once, as it evaluates it (ActionRunner.cs:343-347), and nothing masks the
	307	+	// name of the timeline record afterwards. A secret that the step itself
	308	+	// registers (add-mask, the token of stop-commands) is thus not applied to its
	309	+	// own name (recorded: cmd-stop-commands, "pause-logging allowed by step env").
301	310		func (r *maskReporter) StepStarted(n int, name string, at time.Time) {
302		-	r.next.StepStarted(n, r.j.mask(name), at)
	311	+	name = r.j.mask(name)
	312	+	r.mu.Lock()
	313	+	if r.names == nil {
	314	+	r.names = map[int]string{}
	315	+	}
	316	+	r.names[n] = name
	317	+	r.mu.Unlock()
	318	+	r.next.StepStarted(n, name, at)
303	319		}
304	320
305	321		func (r *maskReporter) Log(n int, at time.Time, line string) {

@@ -309,7 +325,18 @@ func (r *maskReporter) Log(n int, at time.Time, line string) {
309	325		func (r *maskReporter) StepCompleted(n int, name string, c Conclusion, at time.Time) {
310	326		// The results of the step (outcome, annotations, summary) go first.
311	327		r.j.flushStepResults(r.next, n, c)
312		-	r.next.StepCompleted(n, r.j.mask(name), c, at)
	328	+	// The name as reported at the start: the secrets a step registers while it
	329	+	// runs do not apply to its own name (see StepStarted).
	330	+	r.mu.Lock()
	331	+	started, ok := r.names[n]
	332	+	delete(r.names, n)
	333	+	r.mu.Unlock()
	334	+	if ok {
	335	+	name = started
	336	+	} else {
	337	+	name = r.j.mask(name)
	338	+	}
	339	+	r.next.StepCompleted(n, name, c, at)
313	340		}
314	341
315	342		func (r *maskReporter) JobCompleted(res Result) {


@@ -296,10 +296,26 @@ func (m *masker) has(s string) bool { return m != nil && m.mask(s) != s }
296		type maskReporter struct {	296		type maskReporter struct {
297		j *job	297		j *job
298		next Reporter	298		next Reporter
			299	+
			300	+	mu sync.Mutex
			301	+	names map[int]string // the masked name each step started with
299		}	302		}
300			303
			304	+	// StepStarted masks the name with the secrets known when the step starts, which
			305	+	// is when the runner masks it: ActionRunner.EvaluateDisplayName masks the name
			306	+	// once, as it evaluates it (ActionRunner.cs:343-347), and nothing masks the
			307	+	// name of the timeline record afterwards. A secret that the step itself
			308	+	// registers (add-mask, the token of stop-commands) is thus not applied to its
			309	+	// own name (recorded: cmd-stop-commands, "pause-logging allowed by step env").
301		func (r *maskReporter) StepStarted(n int, name string, at time.Time) {	310		func (r *maskReporter) StepStarted(n int, name string, at time.Time) {
302	-	r.next.StepStarted(n, r.j.mask(name), at)	311	+	name = r.j.mask(name)
			312	+	r.mu.Lock()
			313	+	if r.names == nil {
			314	+	r.names = map[int]string{}
			315	+	}
			316	+	r.names[n] = name
			317	+	r.mu.Unlock()
			318	+	r.next.StepStarted(n, name, at)
303		}	319		}
304			320
305		func (r *maskReporter) Log(n int, at time.Time, line string) {	321		func (r *maskReporter) Log(n int, at time.Time, line string) {

@@ -309,7 +325,18 @@ func (r *maskReporter) Log(n int, at time.Time, line string) {
309		func (r *maskReporter) StepCompleted(n int, name string, c Conclusion, at time.Time) {	325		func (r *maskReporter) StepCompleted(n int, name string, c Conclusion, at time.Time) {
310		// The results of the step (outcome, annotations, summary) go first.	326		// The results of the step (outcome, annotations, summary) go first.
311		r.j.flushStepResults(r.next, n, c)	327		r.j.flushStepResults(r.next, n, c)
312	-	r.next.StepCompleted(n, r.j.mask(name), c, at)	328	+	// The name as reported at the start: the secrets a step registers while it
			329	+	// runs do not apply to its own name (see StepStarted).
			330	+	r.mu.Lock()
			331	+	started, ok := r.names[n]
			332	+	delete(r.names, n)
			333	+	r.mu.Unlock()
			334	+	if ok {
			335	+	name = started
			336	+	} else {
			337	+	name = r.j.mask(name)
			338	+	}
			339	+	r.next.StepCompleted(n, name, c, at)
313		}	340		}
314			341
315		func (r *maskReporter) JobCompleted(res Result) {	342		func (r *maskReporter) JobCompleted(res Result) {

M internal/executor/masker_test.go +33 -0


@@ -198,3 +198,36 @@ jobs:
198	198		}
199	199		}
200	200		}
	201	+
	202	+	// cmd-stop-commands: the token of ::stop-commands:: is a secret from then on
	203	+	// (ActionCommandManager.cs:117-119) but the name of the step that registers it
	204	+	// is not masked: the runner masks a display name once, when it evaluates it
	205	+	// (ActionRunner.cs:343-347). Recorded: "pause-logging allowed by step env". A
	206	+	// secret that is known when the step starts is still masked in its name, and a
	207	+	// later step's name gets the new secret.
	208	+	func TestSecretRegisteredByAStepIsNotAppliedToItsOwnName(t *testing.T) {
	209	+	needTools(t, "bash")
	210	+	yaml := `on: push
	211	+	jobs:
	212	+	main:
	213	+	runs-on: ubuntu-24.04
	214	+	steps:
	215	+	- name: pause-logging allowed by step env
	216	+	env:
	217	+	ACTIONS_ALLOW_UNSECURE_STOPCOMMAND_TOKENS: 'true'
	218	+	run: \|
	219	+	echo "::stop-commands::pause-logging"
	220	+	echo "::pause-logging::"
	221	+	- name: after pause-logging
	222	+	run: echo hi
	223	+	- name: leak ${{ github.token }}
	224	+	run: echo hi
	225	+	`
	226	+	rec, res := runLocal(t, yaml, "main", func(c *Config) { c.ContextsJSON = tokenContexts() })
	227	+	if res.Conclusion != ConclusionSuccess {
	228	+	t.Fatalf("%+v", res)
	229	+	}
	230	+	rec.step(t, "pause-logging allowed by step env")
	231	+	rec.step(t, "after ***")
	232	+	rec.step(t, "leak ***")
	233	+	}


@@ -198,3 +198,36 @@ jobs:
198	}	198		}
199	}	199		}
200	}	200		}
		201	+
		202	+	// cmd-stop-commands: the token of ::stop-commands:: is a secret from then on
		203	+	// (ActionCommandManager.cs:117-119) but the name of the step that registers it
		204	+	// is not masked: the runner masks a display name once, when it evaluates it
		205	+	// (ActionRunner.cs:343-347). Recorded: "pause-logging allowed by step env". A
		206	+	// secret that is known when the step starts is still masked in its name, and a
		207	+	// later step's name gets the new secret.
		208	+	func TestSecretRegisteredByAStepIsNotAppliedToItsOwnName(t *testing.T) {
		209	+	needTools(t, "bash")
		210	+	yaml := `on: push
		211	+	jobs:
		212	+	main:
		213	+	runs-on: ubuntu-24.04
		214	+	steps:
		215	+	- name: pause-logging allowed by step env
		216	+	env:
		217	+	ACTIONS_ALLOW_UNSECURE_STOPCOMMAND_TOKENS: 'true'
		218	+	run: \|
		219	+	echo "::stop-commands::pause-logging"
		220	+	echo "::pause-logging::"
		221	+	- name: after pause-logging
		222	+	run: echo hi
		223	+	- name: leak ${{ github.token }}
		224	+	run: echo hi
		225	+	`
		226	+	rec, res := runLocal(t, yaml, "main", func(c *Config) { c.ContextsJSON = tokenContexts() })
		227	+	if res.Conclusion != ConclusionSuccess {
		228	+	t.Fatalf("%+v", res)
		229	+	}
		230	+	rec.step(t, "pause-logging allowed by step env")
		231	+	rec.step(t, "after ***")
		232	+	rec.step(t, "leak ***")
		233	+	}

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


@@ -127,6 +127,14 @@ func firstLine(contents string) string {
127	127		// ActionRunner.cs:363): the evaluated name, or "Run " and the first line of
128	128		// the script or the action reference.
129	129		func (j job) displayName(st workflow.Step, env *ordered) string {
	130	+	return j.displayNameOf(st, env, false)
	131	+	}
	132	+
	133	+	// displayNameOf is displayName; embedded is true for a step of a composite
	134	+	// action, whose name is evaluated only when the step starts
	135	+	// (CompositeActionHandler.cs:314-318), with the contexts of that step (the
	136	+	// inputs and steps of the action included), never with those of the job message.
	137	+	func (j job) displayNameOf(st workflow.Step, env *ordered, embedded bool) string {
130	138		tok := st.Name
131	139		prefix := ""
132	140		switch {

@@ -156,7 +164,7 @@ func (j job) displayName(st workflow.Step, env *ordered) string {
156	164		// with the contexts of the step, when the step starts.
157	165		var name string
158	166		var err error
159		-	if serverEvaluable(tok) {
	167	+	if !embedded && serverEvaluable(tok) {
160	168		j.mu.Lock()
161	169		sc := j.state.ServerContexts()
162	170		j.mu.Unlock()

 // ActionRunner.cs:363): the evaluated name, or "Run " and the first line of
 // the script or the action reference.
 func (j *job) displayName(st *workflow.Step, env *ordered) string {
+	return j.displayNameOf(st, env, false)
+}
+// displayNameOf is displayName; embedded is true for a step of a composite
+// action, whose name is evaluated only when the step starts
+// (CompositeActionHandler.cs:314-318), with the contexts of that step (the
+// inputs and steps of the action included), never with those of the job message.
+func (j *job) displayNameOf(st *workflow.Step, env *ordered, embedded bool) string {
 	tok := st.Name
 	prefix := ""
 	switch {
 	// with the contexts of the step, when the step starts.
 	var name string
 	var err error
-	if serverEvaluable(tok) {
+	if !embedded && serverEvaluable(tok) {
 		j.mu.Lock()
 		sc := j.state.ServerContexts()
 		j.mu.Unlock()