fix(executor): log parity for action steps (CI-2 compare)

commit: 9e5713165a5d241184245619f033a31a881177b3
author: brice · 8d ago · 2026-10-14 10:26 UTC
committer: bnema · 8d ago · 2026-10-14 10:26 UTC
parent: 819edf3
signature: SSH UnverifiedSHA256:VOCA5lYARHcbKDZ7GhTV45VmcjWnJ6ZQIBazj17LivA The key is unknown

11 files · +430 -28

M internal/executor/actions_composite.go +39 -3


@@ -50,6 +50,9 @@ func (j job) prepareLocalComposite(s stepRun, def *actionDef, cancel <-chan st
50	50		if err != nil {
51	51		return err
52	52		}
	53	+	for _, k := range kids {
	54	+	k.runtimePrepared = true
	55	+	}
53	56		n.children = kids
54	57		n.preSteps, n.postSteps = nil, nil
55	58		j.out(s.num, "Prepare all required actions")

@@ -150,7 +153,7 @@ func (j job) runComposite(s stepRun, def actionDef, in inputSet, env *ordere
150	153		ok := resSucceeded
151	154		s.result = &ok
152	155		for _, c := range kids {
153		-	j.runEmbedded(s, c)
	156	+	j.runEmbedded(s, c, scope)
154	157		}
155	158		// Outputs are evaluated in the steps context of the action and belong to the
156	159		// step that uses it (ExecutionContext.SetOutput of the composite step).

@@ -196,7 +199,10 @@ func (j job) compositeOutputs(s stepRun, def *actionDef) []Pair {
196	199		// step of the composite action runs inside the step that uses it, with its own
197	200		// environment, condition, cancellation watch and result. Its log lines belong
198	201		// to the outer step.
199		-	func (j job) runEmbedded(parent stepRun, c *node) {
	202	+	//
	203	+	// scope is the steps context of the action; with the id of the step it is the
	204	+	// id of the step's markers (the fully qualified context name).
	205	+	func (j job) runEmbedded(parent stepRun, c *node, scope string) {
200	206		s := &stepRun{j: j, num: parent.num, st: c.st, n: c, stage: parent.stage, parent: parent, fc: j.fc.Fork()}
201	207		// The issues, summaries and state of an inner step belong to the step that
202	208		// uses the action, whose collector counts them (10 per level, the outer

@@ -211,9 +217,26 @@ func (j job) runEmbedded(parent stepRun, c *node) {
211	217		env, envErr := j.stepEnvironment(s)
212	218		s.env = env
213	219		s.name = j.displayName(c.st, env)
	220	+	// The display name is masked before it is cut and escaped. The runner masks
	221	+	// the finished line (ExecutionContext.Write), after SanitizeDisplayName and
	222	+	// EscapeValue, so a secret holding ";", "]" or "%" (escaped by then) or cut by
	223	+	// the 1000-character limit would escape its masker. Masking first is stricter;
	224	+	// the line is masked again when it is written, as every line is.
	225	+	id := scope + "." + c.st.ID
214	226		if envErr != nil {
	227	+	// The error of the environment is logged and the step marked failed, then
	228	+	// the start marker is printed (CompositeActionHandler.cs:296-302, :308-324).
	229	+	// DIVERGENCE: the runner does not stop there. It falls through to the
	230	+	// condition (:372-390); success() of an embedded main step reads
	231	+	// github.action_status, set from the composite's result before the env was
	232	+	// evaluated (:246-248, SuccessFunction.cs:28), so the default condition still
	233	+	// holds and the runner RUNS the step, without the env that failed, and ends
	234	+	// with the marker of its failed result and the elapsed time. Here the step
	235	+	// does not run and the end marker says duration_ms=0.
215	236		j.errorf(s.num, "%s", templateNotValid(envErr, true))
	237	+	j.out(parent.num, startMarker(j.mask(s.name), id))
216	238		j.finishStep(s, resFailed)
	239	+	j.out(parent.num, endMarker(id, "failure", "failure", 0))
217	240		return
218	241		}
219	242		q := qstep{n: c, stage: parent.stage}

@@ -225,5 +248,18 @@ func (j job) runEmbedded(parent stepRun, c *node) {
225	248		case stagePost:
226	249		q.cond = c.postCond
227	250		}
228		-	j.runGuarded(s, q, true)
	251	+	j.out(parent.num, startMarker(j.mask(s.name), id))
	252	+	start := j.now()
	253	+	r := j.runGuarded(s, q, true)
	254	+	// Outcome is the result before continue-on-error, conclusion the final one;
	255	+	// a skipped step takes no time (CompositeActionHandler.cs:415, :448).
	256	+	outcome := r
	257	+	if s.outcome != nil {
	258	+	outcome = *s.outcome
	259	+	}
	260	+	ms := j.now().Sub(start).Milliseconds()
	261	+	if r == resSkipped {
	262	+	ms = 0
	263	+	}
	264	+	j.out(parent.num, endMarker(id, markerResult(outcome), markerResult(r), ms))
229	265		}


@@ -50,6 +50,9 @@ func (j job) prepareLocalComposite(s stepRun, def *actionDef, cancel <-chan st
50		if err != nil {	50		if err != nil {
51		return err	51		return err
52		}	52		}
			53	+	for _, k := range kids {
			54	+	k.runtimePrepared = true
			55	+	}
53		n.children = kids	56		n.children = kids
54		n.preSteps, n.postSteps = nil, nil	57		n.preSteps, n.postSteps = nil, nil
55		j.out(s.num, "Prepare all required actions")	58		j.out(s.num, "Prepare all required actions")

@@ -150,7 +153,7 @@ func (j job) runComposite(s stepRun, def actionDef, in inputSet, env *ordere
150		ok := resSucceeded	153		ok := resSucceeded
151		s.result = &ok	154		s.result = &ok
152		for _, c := range kids {	155		for _, c := range kids {
153	-	j.runEmbedded(s, c)	156	+	j.runEmbedded(s, c, scope)
154		}	157		}
155		// Outputs are evaluated in the steps context of the action and belong to the	158		// Outputs are evaluated in the steps context of the action and belong to the
156		// step that uses it (ExecutionContext.SetOutput of the composite step).	159		// step that uses it (ExecutionContext.SetOutput of the composite step).

@@ -196,7 +199,10 @@ func (j job) compositeOutputs(s stepRun, def *actionDef) []Pair {
196		// step of the composite action runs inside the step that uses it, with its own	199		// step of the composite action runs inside the step that uses it, with its own
197		// environment, condition, cancellation watch and result. Its log lines belong	200		// environment, condition, cancellation watch and result. Its log lines belong
198		// to the outer step.	201		// to the outer step.
199	-	func (j job) runEmbedded(parent stepRun, c *node) {	202	+	//
			203	+	// scope is the steps context of the action; with the id of the step it is the
			204	+	// id of the step's markers (the fully qualified context name).
			205	+	func (j job) runEmbedded(parent stepRun, c *node, scope string) {
200		s := &stepRun{j: j, num: parent.num, st: c.st, n: c, stage: parent.stage, parent: parent, fc: j.fc.Fork()}	206		s := &stepRun{j: j, num: parent.num, st: c.st, n: c, stage: parent.stage, parent: parent, fc: j.fc.Fork()}
201		// The issues, summaries and state of an inner step belong to the step that	207		// The issues, summaries and state of an inner step belong to the step that
202		// uses the action, whose collector counts them (10 per level, the outer	208		// uses the action, whose collector counts them (10 per level, the outer

@@ -211,9 +217,26 @@ func (j job) runEmbedded(parent stepRun, c *node) {
211		env, envErr := j.stepEnvironment(s)	217		env, envErr := j.stepEnvironment(s)
212		s.env = env	218		s.env = env
213		s.name = j.displayName(c.st, env)	219		s.name = j.displayName(c.st, env)
			220	+	// The display name is masked before it is cut and escaped. The runner masks
			221	+	// the finished line (ExecutionContext.Write), after SanitizeDisplayName and
			222	+	// EscapeValue, so a secret holding ";", "]" or "%" (escaped by then) or cut by
			223	+	// the 1000-character limit would escape its masker. Masking first is stricter;
			224	+	// the line is masked again when it is written, as every line is.
			225	+	id := scope + "." + c.st.ID
214		if envErr != nil {	226		if envErr != nil {
			227	+	// The error of the environment is logged and the step marked failed, then
			228	+	// the start marker is printed (CompositeActionHandler.cs:296-302, :308-324).
			229	+	// DIVERGENCE: the runner does not stop there. It falls through to the
			230	+	// condition (:372-390); success() of an embedded main step reads
			231	+	// github.action_status, set from the composite's result before the env was
			232	+	// evaluated (:246-248, SuccessFunction.cs:28), so the default condition still
			233	+	// holds and the runner RUNS the step, without the env that failed, and ends
			234	+	// with the marker of its failed result and the elapsed time. Here the step
			235	+	// does not run and the end marker says duration_ms=0.
215		j.errorf(s.num, "%s", templateNotValid(envErr, true))	236		j.errorf(s.num, "%s", templateNotValid(envErr, true))
			237	+	j.out(parent.num, startMarker(j.mask(s.name), id))
216		j.finishStep(s, resFailed)	238		j.finishStep(s, resFailed)
			239	+	j.out(parent.num, endMarker(id, "failure", "failure", 0))
217		return	240		return
218		}	241		}
219		q := qstep{n: c, stage: parent.stage}	242		q := qstep{n: c, stage: parent.stage}

@@ -225,5 +248,18 @@ func (j job) runEmbedded(parent stepRun, c *node) {
225		case stagePost:	248		case stagePost:
226		q.cond = c.postCond	249		q.cond = c.postCond
227		}	250		}
228	-	j.runGuarded(s, q, true)	251	+	j.out(parent.num, startMarker(j.mask(s.name), id))
			252	+	start := j.now()
			253	+	r := j.runGuarded(s, q, true)
			254	+	// Outcome is the result before continue-on-error, conclusion the final one;
			255	+	// a skipped step takes no time (CompositeActionHandler.cs:415, :448).
			256	+	outcome := r
			257	+	if s.outcome != nil {
			258	+	outcome = *s.outcome
			259	+	}
			260	+	ms := j.now().Sub(start).Milliseconds()
			261	+	if r == resSkipped {
			262	+	ms = 0
			263	+	}
			264	+	j.out(parent.num, endMarker(id, markerResult(outcome), markerResult(r), ms))
229		}	265		}

M internal/executor/actions_docker.go +2 -1


@@ -82,6 +82,7 @@ func (j *job) dockerBuildCmd(tag, dockerfile, context string) dockerCmd {
82	82
83	83		// buildImage builds an image from a Dockerfile, in the step.
84	84		func (j job) buildImage(s stepRun, tag, dockerfile, workDir string, cancel <-chan struct{}) error {
	85	+	// The context is Directory.GetParent(dockerfile).FullName (see buildActionImage).
85	86		c := j.dockerBuildCmd(tag, dockerfile, filepath.Dir(dockerfile))
86	87		code, err := j.runDockerCmd(s, c, dirOrCwd(workDir), nil, cancel)
87	88		if err != nil {

@@ -122,7 +123,7 @@ func (j job) runDocker(s stepRun, def actionDef, in inputSet, env *ordered,
122	123		if im.Kind == action.ImageRegistry {
123	124		image = im.Ref
124	125		} else {
125		-	build = filepath.Join(def.dir, filepath.FromSlash(im.Path))
	126	+	build = pathCombine(def.dir, filepath.FromSlash(im.Path))
126	127		}
127	128		}
128	129		if build != "" {

 // buildImage builds an image from a Dockerfile, in the step.
 func (j *job) buildImage(s *stepRun, tag, dockerfile, workDir string, cancel <-chan struct{}) error {
+	// The context is Directory.GetParent(dockerfile).FullName (see buildActionImage).
 	c := j.dockerBuildCmd(tag, dockerfile, filepath.Dir(dockerfile))
 	code, err := j.runDockerCmd(s, c, dirOrCwd(workDir), nil, cancel)
 	if err != nil {
 			if im.Kind == action.ImageRegistry {
 				image = im.Ref
 			} else {
-				build = filepath.Join(def.dir, filepath.FromSlash(im.Path))
+				build = pathCombine(def.dir, filepath.FromSlash(im.Path))
 			}
 		}
 		if build != "" {

M internal/executor/actions_docker_test.go +23 -0


@@ -229,3 +229,26 @@ func TestTranslatePath(t *testing.T) {
229	229		}
230	230		}
231	231		}
	232	+
	233	+	// The Dockerfile is passed as written (Path.Combine) and the build context is
	234	+	// Directory.GetParent(dockerFile).FullName, which Path.GetFullPath resolves
	235	+	// lexically: "." and ".." are folded, symlinks are not looked at. Recorded in
	236	+	// act-docker-pre-post: -f ".../work/r/r/./.github/actions/x/Dockerfile" with the
	237	+	// context ".../work/r/r/.github/actions/x".
	238	+	func TestDockerfileContextIsTheLexicalParent(t *testing.T) {
	239	+	tree := files{
	240	+	"act/action.yml": "name: d\nruns:\n using: docker\n image: sub/../Dockerfile\n",
	241	+	"act/Dockerfile": "FROM alpine\n",
	242	+	"act/sub/keep": "",
	243	+	}
	244	+	rec, res, cs, _ := dockerRun(t, tree, " - uses: ./act\n", 0)
	245	+	if res.Conclusion != ConclusionSuccess {
	246	+	t.Fatalf("%+v\n%s", res, logOf(rec.steps[1]))
	247	+	}
	248	+	if len(cs) < 1 \|\| cs[0][0] != "build" {
	249	+	t.Fatalf("calls %v", cs)
	250	+	}
	251	+	if f, c := cs[0][4], cs[0][5]; !strings.HasSuffix(f, "/./act/sub/../Dockerfile") \|\| !strings.HasSuffix(c, "/act") \|\| strings.Contains(c, "..") \|\| strings.Contains(c, "/./") {
	252	+	t.Errorf("-f %q, context %q", f, c)
	253	+	}
	254	+	}


@@ -229,3 +229,26 @@ func TestTranslatePath(t *testing.T) {
229	}	229		}
230	}	230		}
231	}	231		}
		232	+
		233	+	// The Dockerfile is passed as written (Path.Combine) and the build context is
		234	+	// Directory.GetParent(dockerFile).FullName, which Path.GetFullPath resolves
		235	+	// lexically: "." and ".." are folded, symlinks are not looked at. Recorded in
		236	+	// act-docker-pre-post: -f ".../work/r/r/./.github/actions/x/Dockerfile" with the
		237	+	// context ".../work/r/r/.github/actions/x".
		238	+	func TestDockerfileContextIsTheLexicalParent(t *testing.T) {
		239	+	tree := files{
		240	+	"act/action.yml": "name: d\nruns:\n using: docker\n image: sub/../Dockerfile\n",
		241	+	"act/Dockerfile": "FROM alpine\n",
		242	+	"act/sub/keep": "",
		243	+	}
		244	+	rec, res, cs, _ := dockerRun(t, tree, " - uses: ./act\n", 0)
		245	+	if res.Conclusion != ConclusionSuccess {
		246	+	t.Fatalf("%+v\n%s", res, logOf(rec.steps[1]))
		247	+	}
		248	+	if len(cs) < 1 \|\| cs[0][0] != "build" {
		249	+	t.Fatalf("calls %v", cs)
		250	+	}
		251	+	if f, c := cs[0][4], cs[0][5]; !strings.HasSuffix(f, "/./act/sub/../Dockerfile") \|\| !strings.HasSuffix(c, "/act") \|\| strings.Contains(c, "..") \|\| strings.Contains(c, "/./") {
		252	+	t.Errorf("-f %q, context %q", f, c)
		253	+	}
		254	+	}

M internal/executor/actions_load.go +29 -7


@@ -22,8 +22,8 @@ import (
22	22		func (j job) loadManifest(num int, dir string) (action.Action, string, error) {
23	23		var file string
24	24		for _, name := range []string{"action.yml", "action.yaml"} {
25		-	if _, err := os.Stat(filepath.Join(dir, name)); err == nil {
26		-	file = filepath.Join(dir, name)
	25	+	if _, err := os.Stat(pathCombine(dir, name)); err == nil {
	26	+	file = pathCombine(dir, name)
27	27		break
28	28		}
29	29		}

@@ -79,8 +79,8 @@ func (j job) loadDef(num int, dir string) (actionDef, error) {
79	79		return d, nil
80	80		}
81	81		for _, name := range []string{"Dockerfile", "dockerfile"} {
82		-	if _, err := os.Stat(filepath.Join(dir, name)); err == nil {
83		-	return &actionDef{kind: kindDocker, dir: dir, dockerfile: filepath.Join(dir, name)}, nil
	82	+	if _, err := os.Stat(pathCombine(dir, name)); err == nil {
	83	+	return &actionDef{kind: kindDocker, dir: dir, dockerfile: pathCombine(dir, name)}, nil
84	84		}
85	85		}
86	86		return nil, errNoAction

@@ -94,11 +94,11 @@ var errNoAction = fmt.Errorf("no action")
94	94		func (j job) actionDir(n node) string {
95	95		switch n.uses.Kind {
96	96		case action.UsesLocal:
97		-	return filepath.Join(j.state.GitHub("workspace"), filepath.FromSlash(strings.ReplaceAll(n.uses.Path, `\`, "/")))
	97	+	return pathCombine(j.state.GitHub("workspace"), filepath.FromSlash(strings.ReplaceAll(n.uses.Path, `\`, "/")))
98	98		case action.UsesRepository:
99		-	d := filepath.Join(j.state.Layout().Actions, filepath.FromSlash(n.uses.Name), n.uses.Ref)
	99	+	d := pathCombine(j.state.Layout().Actions, filepath.FromSlash(n.uses.Name), n.uses.Ref)
100	100		if n.uses.Path != "" {
101		-	d = filepath.Join(d, filepath.FromSlash(n.uses.Path))
	101	+	d = pathCombine(d, filepath.FromSlash(n.uses.Path))
102	102		}
103	103		return d
104	104		}

@@ -121,3 +121,25 @@ func refString(u action.Uses) string {
121	121		}
122	122		return ""
123	123		}
	124	+
	125	+	// pathCombine is .NET's Path.Combine: the parts are joined with a separator and
	126	+	// nothing is cleaned, so a "./" in the path of a local action stays in the
	127	+	// messages and in github.action_path (recorded: ".../work/repo/repo/./.github/
	128	+	// actions/basic"); a rooted part replaces what precedes it (an absolute `main`
	129	+	// is that file). Paths are not confined to the action directory, as in the
	130	+	// runner: ../ and symlinks may leave it.
	131	+	func pathCombine(parts ...string) string {
	132	+	out := ""
	133	+	for _, p := range parts {
	134	+	switch {
	135	+	case p == "":
	136	+	case out == "" \|\| filepath.IsAbs(p):
	137	+	out = p
	138	+	case strings.HasSuffix(out, string(filepath.Separator)):
	139	+	out += p
	140	+	default:
	141	+	out += string(filepath.Separator) + p
	142	+	}
	143	+	}
	144	+	return out
	145	+	}

 func (j *job) loadManifest(num int, dir string) (*action.Action, string, error) {
 	var file string
 	for _, name := range []string{"action.yml", "action.yaml"} {
-		if _, err := os.Stat(filepath.Join(dir, name)); err == nil {
+		if _, err := os.Stat(pathCombine(dir, name)); err == nil {
-			file = filepath.Join(dir, name)
+			file = pathCombine(dir, name)
 			break
 		}
 	}
 		return d, nil
 	}
 	for _, name := range []string{"Dockerfile", "dockerfile"} {
-		if _, err := os.Stat(filepath.Join(dir, name)); err == nil {
+		if _, err := os.Stat(pathCombine(dir, name)); err == nil {
-			return &actionDef{kind: kindDocker, dir: dir, dockerfile: filepath.Join(dir, name)}, nil
+			return &actionDef{kind: kindDocker, dir: dir, dockerfile: pathCombine(dir, name)}, nil
 		}
 	}
 	return nil, errNoAction
 func (j *job) actionDir(n *node) string {
 	switch n.uses.Kind {
 	case action.UsesLocal:
-		return filepath.Join(j.state.GitHub("workspace"), filepath.FromSlash(strings.ReplaceAll(n.uses.Path, `\`, "/")))
+		return pathCombine(j.state.GitHub("workspace"), filepath.FromSlash(strings.ReplaceAll(n.uses.Path, `\`, "/")))
 	case action.UsesRepository:
-		d := filepath.Join(j.state.Layout().Actions, filepath.FromSlash(n.uses.Name), n.uses.Ref)
+		d := pathCombine(j.state.Layout().Actions, filepath.FromSlash(n.uses.Name), n.uses.Ref)
 		if n.uses.Path != "" {
-			d = filepath.Join(d, filepath.FromSlash(n.uses.Path))
+			d = pathCombine(d, filepath.FromSlash(n.uses.Path))
 		}
 		return d
 	}
 	}
 	return ""
 }
+// pathCombine is .NET's Path.Combine: the parts are joined with a separator and
+// nothing is cleaned, so a "./" in the path of a local action stays in the
+// messages and in github.action_path (recorded: ".../work/repo/repo/./.github/
+// actions/basic"); a rooted part replaces what precedes it (an absolute `main`
+// is that file). Paths are not confined to the action directory, as in the
+// runner: ../ and symlinks may leave it.
+func pathCombine(parts ...string) string {
+	out := ""
+	for _, p := range parts {
+		switch {
+		case p == "":
+		case out == "" || filepath.IsAbs(p):
+			out = p
+		case strings.HasSuffix(out, string(filepath.Separator)):
+			out += p
+		default:
+			out += string(filepath.Separator) + p
+		}
+	}
+	return out
+}

A internal/executor/actions_markers.go +60 -0

@@ -0,0 +1,60 @@
	1	+	// SPDX-License-Identifier: MIT
	2	+
	3	+	package executor
	4	+
	5	+	import (
	6	+	"strconv"
	7	+	"strings"
	8	+	)
	9	+
	10	+	// The start-action and end-action markers the runner prints around each step of
	11	+	// a composite action (CompositeActionHandler.RunStepsAsync, :323 and :415-466).
	12	+	// GitHub's runner emits them when the EmitCompositeMarkers feature is on, which
	13	+	// is the case for the recorded runs, so they are always printed here.
	14	+
	15	+	// maxMarkerDisplayName is CompositeActionHandler.MaxDisplayNameLength.
	16	+	const maxMarkerDisplayName = 1000
	17	+
	18	+	// markerEscaper is ActionCommand.EscapeValue (ActionCommand.cs:211-225): the
	19	+	// escape mappings applied last to first, so that "%" goes first and nothing is
	20	+	// escaped twice.
	21	+	var markerEscaper = strings.NewReplacer("%", "%25", "]", "%5D", "\n", "%0A", "\r", "%0D", ";", "%3B")
	22	+
	23	+	// sanitizeDisplayName is CompositeActionHandler.SanitizeDisplayName (:552): the
	24	+	// first line without its leading white space, cut to 1000 UTF-16 code units.
	25	+	func sanitizeDisplayName(name string) string {
	26	+	r := firstLine(name)
	27	+	units := 0
	28	+	for i, c := range r {
	29	+	w := 1
	30	+	if c >= 0x10000 {
	31	+	w = 2
	32	+	}
	33	+	if units+w > maxMarkerDisplayName {
	34	+	return r[:i]
	35	+	}
	36	+	units += w
	37	+	}
	38	+	return r
	39	+	}
	40	+
	41	+	func startMarker(display, id string) string {
	42	+	return "##[start-action display=" + markerEscaper.Replace(sanitizeDisplayName(display)) + ";id=" + markerEscaper.Replace(id) + "]"
	43	+	}
	44	+
	45	+	func endMarker(id, outcome, conclusion string, ms int64) string {
	46	+	return "##[end-action id=" + markerEscaper.Replace(id) + ";outcome=" + outcome + ";conclusion=" + conclusion + ";duration_ms=" + strconv.FormatInt(ms, 10) + "]"
	47	+	}
	48	+
	49	+	// markerResult is TaskResult.ToActionResult in lower case.
	50	+	func markerResult(r stepResult) string {
	51	+	switch r {
	52	+	case resFailed:
	53	+	return "failure"
	54	+	case resCanceled:
	55	+	return "cancelled"
	56	+	case resSkipped:
	57	+	return "skipped"
	58	+	}
	59	+	return "success"
	60	+	}

@@ -0,0 +1,60 @@
	1	+	// SPDX-License-Identifier: MIT
	2	+
	3	+	package executor
	4	+
	5	+	import (
	6	+	"strconv"
	7	+	"strings"
	8	+	)
	9	+
	10	+	// The start-action and end-action markers the runner prints around each step of
	11	+	// a composite action (CompositeActionHandler.RunStepsAsync, :323 and :415-466).
	12	+	// GitHub's runner emits them when the EmitCompositeMarkers feature is on, which
	13	+	// is the case for the recorded runs, so they are always printed here.
	14	+
	15	+	// maxMarkerDisplayName is CompositeActionHandler.MaxDisplayNameLength.
	16	+	const maxMarkerDisplayName = 1000
	17	+
	18	+	// markerEscaper is ActionCommand.EscapeValue (ActionCommand.cs:211-225): the
	19	+	// escape mappings applied last to first, so that "%" goes first and nothing is
	20	+	// escaped twice.
	21	+	var markerEscaper = strings.NewReplacer("%", "%25", "]", "%5D", "\n", "%0A", "\r", "%0D", ";", "%3B")
	22	+
	23	+	// sanitizeDisplayName is CompositeActionHandler.SanitizeDisplayName (:552): the
	24	+	// first line without its leading white space, cut to 1000 UTF-16 code units.
	25	+	func sanitizeDisplayName(name string) string {
	26	+	r := firstLine(name)
	27	+	units := 0
	28	+	for i, c := range r {
	29	+	w := 1
	30	+	if c >= 0x10000 {
	31	+	w = 2
	32	+	}
	33	+	if units+w > maxMarkerDisplayName {
	34	+	return r[:i]
	35	+	}
	36	+	units += w
	37	+	}
	38	+	return r
	39	+	}
	40	+
	41	+	func startMarker(display, id string) string {
	42	+	return "##[start-action display=" + markerEscaper.Replace(sanitizeDisplayName(display)) + ";id=" + markerEscaper.Replace(id) + "]"
	43	+	}
	44	+
	45	+	func endMarker(id, outcome, conclusion string, ms int64) string {
	46	+	return "##[end-action id=" + markerEscaper.Replace(id) + ";outcome=" + outcome + ";conclusion=" + conclusion + ";duration_ms=" + strconv.FormatInt(ms, 10) + "]"
	47	+	}
	48	+
	49	+	// markerResult is TaskResult.ToActionResult in lower case.
	50	+	func markerResult(r stepResult) string {
	51	+	switch r {
	52	+	case resFailed:
	53	+	return "failure"
	54	+	case resCanceled:
	55	+	return "cancelled"
	56	+	case resSkipped:
	57	+	return "skipped"
	58	+	}
	59	+	return "success"
	60	+	}

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


@@ -177,7 +177,7 @@ func (j job) runNode(s stepRun, def actionDef, in inputSet, env *ordered, ca
177	177		if target == "" {
178	178		return fail(ghErrorf("Value cannot be null or empty. (Parameter 'target')"))
179	179		}
180		-	script := filepath.Join(def.dir, filepath.FromSlash(target))
	180	+	script := pathCombine(def.dir, filepath.FromSlash(target))
181	181		if st, err := os.Stat(script); err != nil \|\| st.IsDir() {
182	182		return fail(ghErrorf("File not found: '%s'", script))
183	183		}


@@ -177,7 +177,7 @@ func (j job) runNode(s stepRun, def actionDef, in inputSet, env *ordered, ca
177		if target == "" {	177		if target == "" {
178		return fail(ghErrorf("Value cannot be null or empty. (Parameter 'target')"))	178		return fail(ghErrorf("Value cannot be null or empty. (Parameter 'target')"))
179		}	179		}
180	-	script := filepath.Join(def.dir, filepath.FromSlash(target))	180	+	script := pathCombine(def.dir, filepath.FromSlash(target))
181		if st, err := os.Stat(script); err != nil \|\| st.IsDir() {	181		if st, err := os.Stat(script); err != nil \|\| st.IsDir() {
182		return fail(ghErrorf("File not found: '%s'", script))	182		return fail(ghErrorf("File not found: '%s'", script))
183		}	183		}

M internal/executor/actions_prepare.go +23 -3


@@ -247,7 +247,7 @@ func (j job) dockerImageOf(def actionDef) (pull, build string, err error) {
247	247		return "", "", err
248	248		}
249	249		if im.Kind == action.ImageDockerfile {
250		-	return "", def.dir + "/" + im.Path, nil
	250	+	return "", pathCombine(def.dir, im.Path), nil
251	251		}
252	252		return im.Ref, "", nil
253	253		}

@@ -275,7 +275,7 @@ func (j job) registerStages(n node, depth int) {
275	275		}
276	276		return
277	277		}
278		-	if p := n.parent; p != nil {
	278	+	if p := n.registrar(); p != nil {
279	279		if pre {
280	280		p.preSteps = append(p.preSteps, n)
281	281		}

@@ -285,9 +285,29 @@ func (j job) registerStages(n node, depth int) {
285	285		}
286	286		}
287	287
	288	+	// registrar is the node whose pre and post steps the node is registered with:
	289	+	// its parent, except for the steps of a local composite action prepared when
	290	+	// the action runs. The runner keys what it registers there by the id of the
	291	+	// execution context (ActionManager.cs:92-113 and :424-430, rootStepId =
	292	+	// ExecutionContext.Id), and an inner step's context shares the record, hence
	293	+	// the id, of the step of the job that contains it: everything lands on that
	294	+	// step, and the inner composite actions themselves never get a post of their
	295	+	// own (recorded: one "Post job cleanup." for act-composite-depth, not one per
	296	+	// level).
	297	+	func (n node) registrar() node {
	298	+	if n.runtimePrepared {
	299	+	for p := n.parent; p != nil; p = p.parent {
	300	+	if p.parent == nil {
	301	+	return p
	302	+	}
	303	+	}
	304	+	}
	305	+	return n.parent
	306	+	}
	307	+
288	308		// parentPost is the assumption for a local action inside a composite action.
289	309		func (n *node) parentPost() {
290		-	if p := n.parent; p != nil {
	310	+	if p := n.registrar(); p != nil {
291	311		p.postSteps = append(p.postSteps, n)
292	312		}
293	313		}

 		return "", "", err
 	}
 	if im.Kind == action.ImageDockerfile {
-		return "", def.dir + "/" + im.Path, nil
+		return "", pathCombine(def.dir, im.Path), nil
 	}
 	return im.Ref, "", nil
 }
 		}
 		return
 	}
-	if p := n.parent; p != nil {
+	if p := n.registrar(); p != nil {
 		if pre {
 			p.preSteps = append(p.preSteps, n)
 		}
 	}
 }
+// registrar is the node whose pre and post steps the node is registered with:
+// its parent, except for the steps of a local composite action prepared when
+// the action runs. The runner keys what it registers there by the id of the
+// execution context (ActionManager.cs:92-113 and :424-430, rootStepId =
+// ExecutionContext.Id), and an inner step's context shares the record, hence
+// the id, of the step of the job that contains it: everything lands on that
+// step, and the inner composite actions themselves never get a post of their
+// own (recorded: one "Post job cleanup." for act-composite-depth, not one per
+// level).
+func (n *node) registrar() *node {
+	if n.runtimePrepared {
+		for p := n.parent; p != nil; p = p.parent {
+			if p.parent == nil {
+				return p
+			}
+		}
+	}
+	return n.parent
+}
 // parentPost is the assumption for a local action inside a composite action.
 func (n *node) parentPost() {
-	if p := n.parent; p != nil {
+	if p := n.registrar(); p != nil {
 		p.postSteps = append(p.postSteps, n)
 	}
 }

M internal/executor/actions_setup.go +5 -11


@@ -4,7 +4,7 @@ package executor
4	4
5	5		import (
6	6		"fmt"
7		-	"os"
	7	+	"path/filepath"
8	8		"time"
9	9		)
10	10

@@ -83,7 +83,10 @@ func (j job) buildActionImage(s stepRun, info buildInfo, users []*node, cancel
83	83		tag := newImageTag(j.dockerLabel())
84	84		code := 0
85	85		for try := 1; try <= buildAttempts; try++ {
86		-	c := j.dockerBuildCmd(tag, info.dockerfile, dirOf(info.dockerfile))
	86	+	// Directory.GetParent(Dockerfile).FullName: the parent after Path.GetFullPath,
	87	+	// which folds "." and ".." lexically, as filepath.Dir does; the Dockerfile
	88	+	// itself is passed as written.
	89	+	c := j.dockerBuildCmd(tag, info.dockerfile, filepath.Dir(info.dockerfile))
87	90		var err error
88	91		code, err = j.runDockerCmd(s, c, dirOrCwd(info.workDir), nil, cancel)
89	92		if err == errCancelled {

@@ -118,15 +121,6 @@ func (j job) buildActionImage(s stepRun, info buildInfo, users []*node, cancel
118	121		return resSucceeded
119	122		}
120	123
121		-	func dirOf(p string) string {
122		-	for i := len(p) - 1; i >= 0; i-- {
123		-	if p[i] == os.PathSeparator {
124		-	return p[:i]
125		-	}
126		-	}
127		-	return "."
128		-	}
129		-
130	124		// pullBackoff is a random wait of 1 to 10 seconds.
131	125		func (j *job) pullBackoff() time.Duration {
132	126		if f := j.cfg.Actions.Backoff; f != nil {

 import (
 	"fmt"
-	"os"
+	"path/filepath"
 	"time"
 )
 	tag := newImageTag(j.dockerLabel())
 	code := 0
 	for try := 1; try <= buildAttempts; try++ {
-		c := j.dockerBuildCmd(tag, info.dockerfile, dirOf(info.dockerfile))
+		// Directory.GetParent(Dockerfile).FullName: the parent after Path.GetFullPath,
+		// which folds "." and ".." lexically, as filepath.Dir does; the Dockerfile
+		// itself is passed as written.
+		c := j.dockerBuildCmd(tag, info.dockerfile, filepath.Dir(info.dockerfile))
 		var err error
 		code, err = j.runDockerCmd(s, c, dirOrCwd(info.workDir), nil, cancel)
 		if err == errCancelled {
 	return resSucceeded
 }
-func dirOf(p string) string {
-	for i := len(p) - 1; i >= 0; i-- {
-		if p[i] == os.PathSeparator {
-			return p[:i]
-		}
-	}
-	return "."
-}
 // pullBackoff is a random wait of 1 to 10 seconds.
 func (j *job) pullBackoff() time.Duration {
 	if f := j.cfg.Actions.Backoff; f != nil {

M internal/executor/actions_test.go +244 -1


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

@@ -377,7 +378,7 @@ runs:
377	378		t.Fatalf("%+v", res)
378	379		}
379	380		got := logOf(rec.steps[1])
380		-	if !strings.Contains(got, "##[error]Process completed with exit code 3.") \|\| strings.Contains(got, "never") \|\|
	381	+	if !strings.Contains(got, "##[error]Process completed with exit code 3.") \|\| strings.Contains(got, "\nnever") \|\|
381	382		!strings.Contains(got, "status=failure") {
382	383		t.Errorf("%s", got)
383	384		}

@@ -948,3 +949,245 @@ func TestTokenAllowedOrigins(t *testing.T) {
948	949		t.Error("no token, nothing to allow")
949	950		}
950	951		}
	952	+
	953	+	// The markers of the runner around the steps of a composite action
	954	+	// (CompositeActionHandler.cs:323, :415-466), as recorded in act-composite-basic,
	955	+	// act-composite-nested and act-composite-depth: the id is the dotted context
	956	+	// name, a step without an id gets the id the workflow reader generated.
	957	+	func TestCompositeStepMarkers(t *testing.T) {
	958	+	tree := files{
	959	+	".github/actions/mid/action.yml": `name: mid
	960	+	runs:
	961	+	using: composite
	962	+	steps:
	963	+	- id: leaf-step
	964	+	shell: bash
	965	+	name: "multi %; ] line"
	966	+	run: echo leaf
	967	+	- uses: ./.github/actions/leaf
	968	+	`,
	969	+	".github/actions/leaf/action.yml": `name: leaf
	970	+	runs:
	971	+	using: composite
	972	+	steps:
	973	+	- shell: bash
	974	+	run: echo "level 2"
	975	+	- shell: bash
	976	+	if: false
	977	+	run: echo skipped
	978	+	- shell: bash
	979	+	continue-on-error: true
	980	+	run: exit 1
	981	+	`,
	982	+	}
	983	+	rec, res := actionRun(t, tree, " - id: call\n uses: ./.github/actions/mid\n - uses: ./.github/actions/leaf\n", nil)
	984	+	if res.Conclusion != ConclusionSuccess {
	985	+	t.Fatalf("%+v\n%s", res, logOf(rec.steps[1]))
	986	+	}
	987	+	var got []string
	988	+	for _, st := range rec.steps[1:3] {
	989	+	for _, l := range st.Log {
	990	+	if strings.HasPrefix(l, "##[start-action") \|\| strings.HasPrefix(l, "##[end-action") {
	991	+	if !strings.Contains(l, "outcome=skipped") {
	992	+	l = regexp.MustCompile(`duration_ms=\d+`).ReplaceAllString(l, "duration_ms=N")
	993	+	}
	994	+	got = append(got, l)
	995	+	}
	996	+	}
	997	+	}
	998	+	want := []string{
	999	+	"##[start-action display=multi %25%3B %5D line;id=call.leaf-step]",
	1000	+	"##[end-action id=call.leaf-step;outcome=success;conclusion=success;duration_ms=N]",
	1001	+	"##[start-action display=Run ./.github/actions/leaf;id=call.__self]",
	1002	+	`##[start-action display=Run echo "level 2";id=call.__self.__run]`,
	1003	+	"##[end-action id=call.__self.__run;outcome=success;conclusion=success;duration_ms=N]",
	1004	+	"##[start-action display=Run echo skipped;id=call.__self.__run_2]",
	1005	+	"##[end-action id=call.__self.__run_2;outcome=skipped;conclusion=skipped;duration_ms=0]",
	1006	+	"##[start-action display=Run exit 1;id=call.__self.__run_3]",
	1007	+	"##[end-action id=call.__self.__run_3;outcome=failure;conclusion=success;duration_ms=N]",
	1008	+	"##[end-action id=call.__self;outcome=success;conclusion=success;duration_ms=N]",
	1009	+	`##[start-action display=Run echo "level 2";id=__self.__run]`,
	1010	+	"##[end-action id=__self.__run;outcome=success;conclusion=success;duration_ms=N]",
	1011	+	"##[start-action display=Run echo skipped;id=__self.__run_2]",
	1012	+	"##[end-action id=__self.__run_2;outcome=skipped;conclusion=skipped;duration_ms=0]",
	1013	+	"##[start-action display=Run exit 1;id=__self.__run_3]",
	1014	+	"##[end-action id=__self.__run_3;outcome=failure;conclusion=success;duration_ms=N]",
	1015	+	}
	1016	+	if strings.Join(got, "\n") != strings.Join(want, "\n") {
	1017	+	t.Errorf("markers:\n%s\nwant:\n%s", strings.Join(got, "\n"), strings.Join(want, "\n"))
	1018	+	}
	1019	+	}
	1020	+
	1021	+	// A local composite action that uses other local composite actions has one post
	1022	+	// step, which prints "Post job cleanup." once (act-composite-depth: 9, 10 and 12
	1023	+	// levels, one line on GitHub). What the runner registers while it prepares the
	1024	+	// steps of a local composite is keyed by the id of the step of the job.
	1025	+	func TestNestedLocalCompositePostsOnce(t *testing.T) {
	1026	+	tree := files{
	1027	+	"c1/action.yml": "name: c1\nruns:\n using: composite\n steps:\n - shell: bash\n run: echo one\n - uses: ./c2\n",
	1028	+	"c2/action.yml": "name: c2\nruns:\n using: composite\n steps:\n - shell: bash\n run: echo two\n - uses: ./c3\n",
	1029	+	"c3/action.yml": "name: c3\nruns:\n using: composite\n steps:\n - shell: bash\n run: echo three\n",
	1030	+	}
	1031	+	rec, res := actionRun(t, tree, " - uses: ./c1\n", nil)
	1032	+	if res.Conclusion != ConclusionSuccess {
	1033	+	t.Fatalf("%+v", res)
	1034	+	}
	1035	+	post := logOf(rec.step(t, "Post Run ./c1"))
	1036	+	if post != "Post job cleanup." {
	1037	+	t.Errorf("post of c1: %q", post)
	1038	+	}
	1039	+	for _, n := range rec.names() {
	1040	+	if strings.HasPrefix(n, "Post ") && n != "Post Run ./c1" {
	1041	+	t.Errorf("unexpected step %q in %v", n, rec.names())
	1042	+	}
	1043	+	}
	1044	+	}
	1045	+
	1046	+	// A local action's directory is the workspace and the path of the step joined as
	1047	+	// .NET's Path.Combine does, without cleaning: github.action_path and the paths in
	1048	+	// the manifest errors keep the "./" of "uses: ./.github/actions/x" (recorded in
	1049	+	// act-composite-basic and act-manifest-errors).
	1050	+	func TestLocalActionPathKeepsDotSlash(t *testing.T) {
	1051	+	tree := files{
	1052	+	".github/actions/ok/action.yml": "name: ok\nruns:\n using: composite\n steps:\n - shell: bash\n run: echo \"path=$GITHUB_ACTION_PATH\"\n",
	1053	+	".github/actions/bad/action.yml": "name: bad\nruns:\n using: composite\n steps:\n - run: echo no shell\n",
	1054	+	}
	1055	+	rec, res := actionRun(t, tree, " - uses: ./.github/actions/ok\n - uses: ./.github/actions/bad\n", nil)
	1056	+	if res.Conclusion != ConclusionFailure {
	1057	+	t.Fatalf("%+v", res)
	1058	+	}
	1059	+	ws := regexp.MustCompile(`path=(.*)/\./\.github/actions/ok`).FindStringSubmatch(logOf(rec.steps[1]))
	1060	+	if ws == nil {
	1061	+	t.Fatalf("action_path lost its ./:\n%s", logOf(rec.steps[1]))
	1062	+	}
	1063	+	bad := logOf(rec.steps[2])
	1064	+	if !strings.Contains(bad, "##[error]"+ws[1]+"/./.github/actions/bad/action.yml (Line: 5, Col: 7): Required property is missing: shell") \|\|
	1065	+	!strings.Contains(bad, "##[error]Failed to load "+ws[1]+"/./.github/actions/bad/action.yml") {
	1066	+	t.Errorf("manifest errors:\n%s", bad)
	1067	+	}
	1068	+	}
	1069	+
	1070	+	// The markers of a composite step go through the masker like every log line
	1071	+	// (ExecutionContext.Write masks tag+message after the marker was built): a
	1072	+	// secret in the name of an inner step, from the configuration or from an
	1073	+	// ::add-mask:: of an earlier step, never reaches the log.
	1074	+	func TestCompositeMarkersAreMasked(t *testing.T) {
	1075	+	tree := files{
	1076	+	".github/actions/m/action.yml": `name: m
	1077	+	runs:
	1078	+	using: composite
	1079	+	steps:
	1080	+	- shell: bash
	1081	+	run: echo "::add-mask::late-secret-""value"
	1082	+	- name: "token ${{ github.token }} and late-secret-value"
	1083	+	shell: bash
	1084	+	run: echo hi
	1085	+	- name: "env ${{ env.TOK }}"
	1086	+	env:
	1087	+	TOK: late-secret-value
	1088	+	shell: bash
	1089	+	run: echo hi
	1090	+	`,
	1091	+	}
	1092	+	rec, res := actionRun(t, tree, " - uses: ./.github/actions/m\n", func(c *Config) { c.ContextsJSON = tokenContexts() })
	1093	+	if res.Conclusion != ConclusionSuccess {
	1094	+	t.Fatalf("%+v\n%s", res, logOf(rec.steps[1]))
	1095	+	}
	1096	+	log := logOf(rec.steps[1])
	1097	+	for _, secret := range []string{testToken, "late-secret-value"} {
	1098	+	if strings.Contains(log, secret) {
	1099	+	t.Errorf("%q is in the log:\n%s", secret, log)
	1100	+	}
	1101	+	}
	1102	+	// A secret with a character the marker escapes is masked before the escape.
	1103	+	if got := markerEscaper.Replace(sanitizeDisplayName("a;b")); got != "a%3Bb" {
	1104	+	t.Errorf("escape %q", got)
	1105	+	}
	1106	+	for _, w := range []string{"##[start-action display=token * and ;id=", "##[start-action display=env **;id="} {
	1107	+	if !strings.Contains(log, w) {
	1108	+	t.Errorf("missing %q:\n%s", w, log)
	1109	+	}
	1110	+	}
	1111	+	}
	1112	+
	1113	+	// An inner step whose env does not evaluate still gets its markers: the error
	1114	+	// is logged while the environment is set up, the start marker follows, then the
	1115	+	// end marker with the step failed (CompositeActionHandler.cs:296-302, :308-324).
	1116	+	// The runner would go on and run the step; this port does not (see runEmbedded).
	1117	+	func TestCompositeEnvErrorKeepsMarkers(t *testing.T) {
	1118	+	tree := files{
	1119	+	".github/actions/e/action.yml": `name: e
	1120	+	runs:
	1121	+	using: composite
	1122	+	steps:
	1123	+	- name: bad env
	1124	+	shell: bash
	1125	+	env:
	1126	+	X: ${{ fromJSON('{') }}
	1127	+	run: echo never
	1128	+	`,
	1129	+	}
	1130	+	rec, res := actionRun(t, tree, " - uses: ./.github/actions/e\n", nil)
	1131	+	if res.Conclusion != ConclusionFailure {
	1132	+	t.Fatalf("%+v", res)
	1133	+	}
	1134	+	var got []string
	1135	+	for _, l := range rec.steps[1].Log {
	1136	+	switch {
	1137	+	case strings.HasPrefix(l, "##[error]"):
	1138	+	got = append(got, "error")
	1139	+	case strings.HasPrefix(l, "##[start-action"), strings.HasPrefix(l, "##[end-action"):
	1140	+	got = append(got, l)
	1141	+	}
	1142	+	}
	1143	+	want := []string{"error", "##[start-action display=bad env;id=__self.__run]", "##[end-action id=__self.__run;outcome=failure;conclusion=failure;duration_ms=0]"}
	1144	+	if strings.Join(got, "\n") != strings.Join(want, "\n") \|\| strings.Contains(logOf(rec.steps[1]), "never") {
	1145	+	t.Errorf("markers:\n%s\n%s", strings.Join(got, "\n"), logOf(rec.steps[1]))
	1146	+	}
	1147	+	}
	1148	+
	1149	+	// A secret that the marker would escape, or that the cut at 1000 characters
	1150	+	// would split, is masked before: the display name of an inner step holding it
	1151	+	// leaves no part of it in the log.
	1152	+	func TestCompositeMarkerMasksBeforeEscapeAndCut(t *testing.T) {
	1153	+	secret := "sec;ret]%value"
	1154	+	long := strings.Repeat("x", 996) + secret // the cut falls inside the secret
	1155	+	tree := files{
	1156	+	".github/actions/m/action.yml": "name: m\nruns:\n using: composite\n steps:\n" +
	1157	+	" - name: \"pre ${{ env.S }}\"\n shell: bash\n run: echo hi\n" +
	1158	+	" - name: \"" + long + "\"\n shell: bash\n run: echo hi\n",
	1159	+	}
	1160	+	rec, res := actionRun(t, tree, " - run: echo \"::add-mask::"+secret+"\"\n - uses: ./.github/actions/m\n env:\n S: "+secret+"\n", nil)
	1161	+	if res.Conclusion != ConclusionSuccess {
	1162	+	t.Fatalf("%+v\n%s", res, logOf(rec.steps[2]))
	1163	+	}
	1164	+	log := logOf(rec.steps[2])
	1165	+	for _, frag := range []string{"sec;", "sec%3B", "value", "%5D%25", "xsec"} {
	1166	+	if strings.Contains(log, frag) {
	1167	+	t.Errorf("%q is in the log:\n%.1500s", frag, log)
	1168	+	}
	1169	+	}
	1170	+	if !strings.Contains(log, "##[start-action display=pre ***;id=") {
	1171	+	t.Errorf("marker:\n%s", log)
	1172	+	}
	1173	+	}
	1174	+
	1175	+	// pathCombine is Path.Combine: a rooted part replaces what precedes it, an empty
	1176	+	// part is skipped, nothing is cleaned.
	1177	+	func TestPathCombine(t *testing.T) {
	1178	+	for _, c := range []struct {
	1179	+	in []string
	1180	+	want string
	1181	+	}{
	1182	+	{[]string{"/w/r", "./.github/a"}, "/w/r/./.github/a"},
	1183	+	{[]string{"/w/r/", "x"}, "/w/r/x"},
	1184	+	{[]string{"/w/r/a", "/etc/x.js"}, "/etc/x.js"},
	1185	+	{[]string{"/w/r/a", "../../x.js"}, "/w/r/a/../../x.js"},
	1186	+	{[]string{"", "x"}, "x"},
	1187	+	{[]string{"/w", ""}, "/w"},
	1188	+	} {
	1189	+	if got := pathCombine(c.in...); got != c.want {
	1190	+	t.Errorf("%q: %q, want %q", c.in, got, c.want)
	1191	+	}
	1192	+	}
	1193	+	}


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

@@ -377,7 +378,7 @@ runs:
377		t.Fatalf("%+v", res)	378		t.Fatalf("%+v", res)
378		}	379		}
379		got := logOf(rec.steps[1])	380		got := logOf(rec.steps[1])
380	-	if !strings.Contains(got, "##[error]Process completed with exit code 3.") \|\| strings.Contains(got, "never") \|\|	381	+	if !strings.Contains(got, "##[error]Process completed with exit code 3.") \|\| strings.Contains(got, "\nnever") \|\|
381		!strings.Contains(got, "status=failure") {	382		!strings.Contains(got, "status=failure") {
382		t.Errorf("%s", got)	383		t.Errorf("%s", got)
383		}	384		}

@@ -948,3 +949,245 @@ func TestTokenAllowedOrigins(t *testing.T) {
948		t.Error("no token, nothing to allow")	949		t.Error("no token, nothing to allow")
949		}	950		}
950		}	951		}
			952	+
			953	+	// The markers of the runner around the steps of a composite action
			954	+	// (CompositeActionHandler.cs:323, :415-466), as recorded in act-composite-basic,
			955	+	// act-composite-nested and act-composite-depth: the id is the dotted context
			956	+	// name, a step without an id gets the id the workflow reader generated.
			957	+	func TestCompositeStepMarkers(t *testing.T) {
			958	+	tree := files{
			959	+	".github/actions/mid/action.yml": `name: mid
			960	+	runs:
			961	+	using: composite
			962	+	steps:
			963	+	- id: leaf-step
			964	+	shell: bash
			965	+	name: "multi %; ] line"
			966	+	run: echo leaf
			967	+	- uses: ./.github/actions/leaf
			968	+	`,
			969	+	".github/actions/leaf/action.yml": `name: leaf
			970	+	runs:
			971	+	using: composite
			972	+	steps:
			973	+	- shell: bash
			974	+	run: echo "level 2"
			975	+	- shell: bash
			976	+	if: false
			977	+	run: echo skipped
			978	+	- shell: bash
			979	+	continue-on-error: true
			980	+	run: exit 1
			981	+	`,
			982	+	}
			983	+	rec, res := actionRun(t, tree, " - id: call\n uses: ./.github/actions/mid\n - uses: ./.github/actions/leaf\n", nil)
			984	+	if res.Conclusion != ConclusionSuccess {
			985	+	t.Fatalf("%+v\n%s", res, logOf(rec.steps[1]))
			986	+	}
			987	+	var got []string
			988	+	for _, st := range rec.steps[1:3] {
			989	+	for _, l := range st.Log {
			990	+	if strings.HasPrefix(l, "##[start-action") \|\| strings.HasPrefix(l, "##[end-action") {
			991	+	if !strings.Contains(l, "outcome=skipped") {
			992	+	l = regexp.MustCompile(`duration_ms=\d+`).ReplaceAllString(l, "duration_ms=N")
			993	+	}
			994	+	got = append(got, l)
			995	+	}
			996	+	}
			997	+	}
			998	+	want := []string{
			999	+	"##[start-action display=multi %25%3B %5D line;id=call.leaf-step]",
			1000	+	"##[end-action id=call.leaf-step;outcome=success;conclusion=success;duration_ms=N]",
			1001	+	"##[start-action display=Run ./.github/actions/leaf;id=call.__self]",
			1002	+	`##[start-action display=Run echo "level 2";id=call.__self.__run]`,
			1003	+	"##[end-action id=call.__self.__run;outcome=success;conclusion=success;duration_ms=N]",
			1004	+	"##[start-action display=Run echo skipped;id=call.__self.__run_2]",
			1005	+	"##[end-action id=call.__self.__run_2;outcome=skipped;conclusion=skipped;duration_ms=0]",
			1006	+	"##[start-action display=Run exit 1;id=call.__self.__run_3]",
			1007	+	"##[end-action id=call.__self.__run_3;outcome=failure;conclusion=success;duration_ms=N]",
			1008	+	"##[end-action id=call.__self;outcome=success;conclusion=success;duration_ms=N]",
			1009	+	`##[start-action display=Run echo "level 2";id=__self.__run]`,
			1010	+	"##[end-action id=__self.__run;outcome=success;conclusion=success;duration_ms=N]",
			1011	+	"##[start-action display=Run echo skipped;id=__self.__run_2]",
			1012	+	"##[end-action id=__self.__run_2;outcome=skipped;conclusion=skipped;duration_ms=0]",
			1013	+	"##[start-action display=Run exit 1;id=__self.__run_3]",
			1014	+	"##[end-action id=__self.__run_3;outcome=failure;conclusion=success;duration_ms=N]",
			1015	+	}
			1016	+	if strings.Join(got, "\n") != strings.Join(want, "\n") {
			1017	+	t.Errorf("markers:\n%s\nwant:\n%s", strings.Join(got, "\n"), strings.Join(want, "\n"))
			1018	+	}
			1019	+	}
			1020	+
			1021	+	// A local composite action that uses other local composite actions has one post
			1022	+	// step, which prints "Post job cleanup." once (act-composite-depth: 9, 10 and 12
			1023	+	// levels, one line on GitHub). What the runner registers while it prepares the
			1024	+	// steps of a local composite is keyed by the id of the step of the job.
			1025	+	func TestNestedLocalCompositePostsOnce(t *testing.T) {
			1026	+	tree := files{
			1027	+	"c1/action.yml": "name: c1\nruns:\n using: composite\n steps:\n - shell: bash\n run: echo one\n - uses: ./c2\n",
			1028	+	"c2/action.yml": "name: c2\nruns:\n using: composite\n steps:\n - shell: bash\n run: echo two\n - uses: ./c3\n",
			1029	+	"c3/action.yml": "name: c3\nruns:\n using: composite\n steps:\n - shell: bash\n run: echo three\n",
			1030	+	}
			1031	+	rec, res := actionRun(t, tree, " - uses: ./c1\n", nil)
			1032	+	if res.Conclusion != ConclusionSuccess {
			1033	+	t.Fatalf("%+v", res)
			1034	+	}
			1035	+	post := logOf(rec.step(t, "Post Run ./c1"))
			1036	+	if post != "Post job cleanup." {
			1037	+	t.Errorf("post of c1: %q", post)
			1038	+	}
			1039	+	for _, n := range rec.names() {
			1040	+	if strings.HasPrefix(n, "Post ") && n != "Post Run ./c1" {
			1041	+	t.Errorf("unexpected step %q in %v", n, rec.names())
			1042	+	}
			1043	+	}
			1044	+	}
			1045	+
			1046	+	// A local action's directory is the workspace and the path of the step joined as
			1047	+	// .NET's Path.Combine does, without cleaning: github.action_path and the paths in
			1048	+	// the manifest errors keep the "./" of "uses: ./.github/actions/x" (recorded in
			1049	+	// act-composite-basic and act-manifest-errors).
			1050	+	func TestLocalActionPathKeepsDotSlash(t *testing.T) {
			1051	+	tree := files{
			1052	+	".github/actions/ok/action.yml": "name: ok\nruns:\n using: composite\n steps:\n - shell: bash\n run: echo \"path=$GITHUB_ACTION_PATH\"\n",
			1053	+	".github/actions/bad/action.yml": "name: bad\nruns:\n using: composite\n steps:\n - run: echo no shell\n",
			1054	+	}
			1055	+	rec, res := actionRun(t, tree, " - uses: ./.github/actions/ok\n - uses: ./.github/actions/bad\n", nil)
			1056	+	if res.Conclusion != ConclusionFailure {
			1057	+	t.Fatalf("%+v", res)
			1058	+	}
			1059	+	ws := regexp.MustCompile(`path=(.*)/\./\.github/actions/ok`).FindStringSubmatch(logOf(rec.steps[1]))
			1060	+	if ws == nil {
			1061	+	t.Fatalf("action_path lost its ./:\n%s", logOf(rec.steps[1]))
			1062	+	}
			1063	+	bad := logOf(rec.steps[2])
			1064	+	if !strings.Contains(bad, "##[error]"+ws[1]+"/./.github/actions/bad/action.yml (Line: 5, Col: 7): Required property is missing: shell") \|\|
			1065	+	!strings.Contains(bad, "##[error]Failed to load "+ws[1]+"/./.github/actions/bad/action.yml") {
			1066	+	t.Errorf("manifest errors:\n%s", bad)
			1067	+	}
			1068	+	}
			1069	+
			1070	+	// The markers of a composite step go through the masker like every log line
			1071	+	// (ExecutionContext.Write masks tag+message after the marker was built): a
			1072	+	// secret in the name of an inner step, from the configuration or from an
			1073	+	// ::add-mask:: of an earlier step, never reaches the log.
			1074	+	func TestCompositeMarkersAreMasked(t *testing.T) {
			1075	+	tree := files{
			1076	+	".github/actions/m/action.yml": `name: m
			1077	+	runs:
			1078	+	using: composite
			1079	+	steps:
			1080	+	- shell: bash
			1081	+	run: echo "::add-mask::late-secret-""value"
			1082	+	- name: "token ${{ github.token }} and late-secret-value"
			1083	+	shell: bash
			1084	+	run: echo hi
			1085	+	- name: "env ${{ env.TOK }}"
			1086	+	env:
			1087	+	TOK: late-secret-value
			1088	+	shell: bash
			1089	+	run: echo hi
			1090	+	`,
			1091	+	}
			1092	+	rec, res := actionRun(t, tree, " - uses: ./.github/actions/m\n", func(c *Config) { c.ContextsJSON = tokenContexts() })
			1093	+	if res.Conclusion != ConclusionSuccess {
			1094	+	t.Fatalf("%+v\n%s", res, logOf(rec.steps[1]))
			1095	+	}
			1096	+	log := logOf(rec.steps[1])
			1097	+	for _, secret := range []string{testToken, "late-secret-value"} {
			1098	+	if strings.Contains(log, secret) {
			1099	+	t.Errorf("%q is in the log:\n%s", secret, log)
			1100	+	}
			1101	+	}
			1102	+	// A secret with a character the marker escapes is masked before the escape.
			1103	+	if got := markerEscaper.Replace(sanitizeDisplayName("a;b")); got != "a%3Bb" {
			1104	+	t.Errorf("escape %q", got)
			1105	+	}
			1106	+	for _, w := range []string{"##[start-action display=token * and ;id=", "##[start-action display=env **;id="} {
			1107	+	if !strings.Contains(log, w) {
			1108	+	t.Errorf("missing %q:\n%s", w, log)
			1109	+	}
			1110	+	}
			1111	+	}
			1112	+
			1113	+	// An inner step whose env does not evaluate still gets its markers: the error
			1114	+	// is logged while the environment is set up, the start marker follows, then the
			1115	+	// end marker with the step failed (CompositeActionHandler.cs:296-302, :308-324).
			1116	+	// The runner would go on and run the step; this port does not (see runEmbedded).
			1117	+	func TestCompositeEnvErrorKeepsMarkers(t *testing.T) {
			1118	+	tree := files{
			1119	+	".github/actions/e/action.yml": `name: e
			1120	+	runs:
			1121	+	using: composite
			1122	+	steps:
			1123	+	- name: bad env
			1124	+	shell: bash
			1125	+	env:
			1126	+	X: ${{ fromJSON('{') }}
			1127	+	run: echo never
			1128	+	`,
			1129	+	}
			1130	+	rec, res := actionRun(t, tree, " - uses: ./.github/actions/e\n", nil)
			1131	+	if res.Conclusion != ConclusionFailure {
			1132	+	t.Fatalf("%+v", res)
			1133	+	}
			1134	+	var got []string
			1135	+	for _, l := range rec.steps[1].Log {
			1136	+	switch {
			1137	+	case strings.HasPrefix(l, "##[error]"):
			1138	+	got = append(got, "error")
			1139	+	case strings.HasPrefix(l, "##[start-action"), strings.HasPrefix(l, "##[end-action"):
			1140	+	got = append(got, l)
			1141	+	}
			1142	+	}
			1143	+	want := []string{"error", "##[start-action display=bad env;id=__self.__run]", "##[end-action id=__self.__run;outcome=failure;conclusion=failure;duration_ms=0]"}
			1144	+	if strings.Join(got, "\n") != strings.Join(want, "\n") \|\| strings.Contains(logOf(rec.steps[1]), "never") {
			1145	+	t.Errorf("markers:\n%s\n%s", strings.Join(got, "\n"), logOf(rec.steps[1]))
			1146	+	}
			1147	+	}
			1148	+
			1149	+	// A secret that the marker would escape, or that the cut at 1000 characters
			1150	+	// would split, is masked before: the display name of an inner step holding it
			1151	+	// leaves no part of it in the log.
			1152	+	func TestCompositeMarkerMasksBeforeEscapeAndCut(t *testing.T) {
			1153	+	secret := "sec;ret]%value"
			1154	+	long := strings.Repeat("x", 996) + secret // the cut falls inside the secret
			1155	+	tree := files{
			1156	+	".github/actions/m/action.yml": "name: m\nruns:\n using: composite\n steps:\n" +
			1157	+	" - name: \"pre ${{ env.S }}\"\n shell: bash\n run: echo hi\n" +
			1158	+	" - name: \"" + long + "\"\n shell: bash\n run: echo hi\n",
			1159	+	}
			1160	+	rec, res := actionRun(t, tree, " - run: echo \"::add-mask::"+secret+"\"\n - uses: ./.github/actions/m\n env:\n S: "+secret+"\n", nil)
			1161	+	if res.Conclusion != ConclusionSuccess {
			1162	+	t.Fatalf("%+v\n%s", res, logOf(rec.steps[2]))
			1163	+	}
			1164	+	log := logOf(rec.steps[2])
			1165	+	for _, frag := range []string{"sec;", "sec%3B", "value", "%5D%25", "xsec"} {
			1166	+	if strings.Contains(log, frag) {
			1167	+	t.Errorf("%q is in the log:\n%.1500s", frag, log)
			1168	+	}
			1169	+	}
			1170	+	if !strings.Contains(log, "##[start-action display=pre ***;id=") {
			1171	+	t.Errorf("marker:\n%s", log)
			1172	+	}
			1173	+	}
			1174	+
			1175	+	// pathCombine is Path.Combine: a rooted part replaces what precedes it, an empty
			1176	+	// part is skipped, nothing is cleaned.
			1177	+	func TestPathCombine(t *testing.T) {
			1178	+	for _, c := range []struct {
			1179	+	in []string
			1180	+	want string
			1181	+	}{
			1182	+	{[]string{"/w/r", "./.github/a"}, "/w/r/./.github/a"},
			1183	+	{[]string{"/w/r/", "x"}, "/w/r/x"},
			1184	+	{[]string{"/w/r/a", "/etc/x.js"}, "/etc/x.js"},
			1185	+	{[]string{"/w/r/a", "../../x.js"}, "/w/r/a/../../x.js"},
			1186	+	{[]string{"", "x"}, "x"},
			1187	+	{[]string{"/w", ""}, "/w"},
			1188	+	} {
			1189	+	if got := pathCombine(c.in...); got != c.want {
			1190	+	t.Errorf("%q: %q, want %q", c.in, got, c.want)
			1191	+	}
			1192	+	}
			1193	+	}

M internal/executor/actions_types.go +3 -0


@@ -158,6 +158,9 @@ type node struct {
158	158		// postRegistered is Root.StepsWithPostRegistered / EmbeddedStepsWithPostRegistered.
159	159		postRegistered bool
160	160		postCond string
	161	+	// runtimePrepared marks the steps of a local composite action, which are
	162	+	// prepared when the action runs (see registrar).
	163	+	runtimePrepared bool
161	164		}
162	165
163	166		// nodeSeq numbers the nodes, for their state keys.


@@ -158,6 +158,9 @@ type node struct {
158	// postRegistered is Root.StepsWithPostRegistered / EmbeddedStepsWithPostRegistered.	158		// postRegistered is Root.StepsWithPostRegistered / EmbeddedStepsWithPostRegistered.
159	postRegistered bool	159		postRegistered bool
160	postCond string	160		postCond string
		161	+	// runtimePrepared marks the steps of a local composite action, which are
		162	+	// prepared when the action runs (see registrar).
		163	+	runtimePrepared bool
161	}	164		}
162		165
163	// nodeSeq numbers the nodes, for their state keys.	166		// nodeSeq numbers the nodes, for their state keys.

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


@@ -108,7 +108,7 @@ func TestNodeActionInContainerUsesExternalsAndAlpine(t *testing.T) {
108	108		t.Errorf("probe: %v", ex[0])
109	109		}
110	110		run := strings.Join(ex[1], "\n")
111		-	if !strings.Contains(run, "\n/__e/"+tc.node+"/bin/node\n") \|\| !strings.Contains(run, "\n/__w/hello/hello/.github/actions/x/index.js") {
	111	+	if !strings.Contains(run, "\n/__e/"+tc.node+"/bin/node\n") \|\| !strings.Contains(run, "\n/__w/hello/hello/./.github/actions/x/index.js") {
112	112		t.Errorf("%s: node call:\n%s", tc.node, run)
113	113		}
114	114		if got := logOf(rec.step(t, "a")); !strings.Contains(got, "exec "+strings.Repeat("0", 63)+"1 sh -c \"cat /etc/*release \| grep ^ID\"") {


@@ -108,7 +108,7 @@ func TestNodeActionInContainerUsesExternalsAndAlpine(t *testing.T) {
108		t.Errorf("probe: %v", ex[0])	108		t.Errorf("probe: %v", ex[0])
109		}	109		}
110		run := strings.Join(ex[1], "\n")	110		run := strings.Join(ex[1], "\n")
111	-	if !strings.Contains(run, "\n/__e/"+tc.node+"/bin/node\n") \|\| !strings.Contains(run, "\n/__w/hello/hello/.github/actions/x/index.js") {	111	+	if !strings.Contains(run, "\n/__e/"+tc.node+"/bin/node\n") \|\| !strings.Contains(run, "\n/__w/hello/hello/./.github/actions/x/index.js") {
112		t.Errorf("%s: node call:\n%s", tc.node, run)	112		t.Errorf("%s: node call:\n%s", tc.node, run)
113		}	113		}
114		if got := logOf(rec.step(t, "a")); !strings.Contains(got, "exec "+strings.Repeat("0", 63)+"1 sh -c \"cat /etc/*release \| grep ^ID\"") {	114		if got := logOf(rec.step(t, "a")); !strings.Contains(got, "exec "+strings.Repeat("0", 63)+"1 sh -c \"cat /etc/*release \| grep ^ID\"") {