fix(executor): validate image references, service names and container ids (GO-005)

commit: 6c6f5f6b7c2bdba4ec3ff83909bc2fb6c9f17337
author: brice · 8d ago · 2026-10-14 06:18 UTC
parent: 77f4a19
signature: GPG VerifiedCD88F9E564E0102646FB6077A1A07C7C7C3E349D · arch-gpg Verified for the committer email

4 files · +81 -0

M internal/executor/containers_fake_test.go +1 -0


@@ -37,6 +37,7 @@ network) ;;
37	37		login) cat > $D/login_stdin ;;
38	38		pull) if [ -f $D/pull_fail ]; then n=$(cat $D/pull_fail); if [ "$n" -gt 0 ]; then echo $((n-1)) > $D/pull_fail; echo "pull denied"; exit 1; fi; fi; echo "pulled $2" ;;
39	39		create) [ -f $D/create_fail ] && { echo "unknown flag"; exit 125; }
	40	+	[ -f $D/create_out ] && { cat $D/create_out; exit 0; }
40	41		n=$(cat $D/seq 2>/dev/null \|\| echo 0); n=$((n+1)); echo $n > $D/seq; printf "%064d\n" $n ;;
41	42		start) echo "$2" ;;
42	43		inspect) case "$*" in


@@ -37,6 +37,7 @@ network) ;;
37	login) cat > $D/login_stdin ;;	37		login) cat > $D/login_stdin ;;
38	pull) if [ -f $D/pull_fail ]; then n=$(cat $D/pull_fail); if [ "$n" -gt 0 ]; then echo $((n-1)) > $D/pull_fail; echo "pull denied"; exit 1; fi; fi; echo "pulled $2" ;;	38		pull) if [ -f $D/pull_fail ]; then n=$(cat $D/pull_fail); if [ "$n" -gt 0 ]; then echo $((n-1)) > $D/pull_fail; echo "pull denied"; exit 1; fi; fi; echo "pulled $2" ;;
39	create) [ -f $D/create_fail ] && { echo "unknown flag"; exit 125; }	39		create) [ -f $D/create_fail ] && { echo "unknown flag"; exit 125; }
		40	+	[ -f $D/create_out ] && { cat $D/create_out; exit 0; }
40	n=$(cat $D/seq 2>/dev/null \|\| echo 0); n=$((n+1)); echo $n > $D/seq; printf "%064d\n" $n ;;	41		n=$(cat $D/seq 2>/dev/null \|\| echo 0); n=$((n+1)); echo $n > $D/seq; printf "%064d\n" $n ;;
41	start) echo "$2" ;;	42		start) echo "$2" ;;
42	inspect) case "$*" in	43		inspect) case "$*" in

M internal/executor/containers_spec.go +29 -0


@@ -5,6 +5,7 @@ package executor
5	5		import (
6	6		"context"
7	7		"fmt"
	8	+	"regexp"
8	9		"strings"
9	10
10	11		"bnema.dev/ermite-actions/expr"

@@ -243,6 +244,28 @@ func containerStrings(v expr.Value, what string) ([]string, error) {
243	244		return out, nil
244	245		}
245	246
	247	+	// imageRefRe is the grammar of a docker image reference
	248	+	// (github.com/distribution/reference): [domain[:port]/]path[:tag][@digest].
	249	+	// Path components may be upper case here: docker words that error itself, as
	250	+	// it does for the image of the recorded ctr-errors scenario. What matters is
	251	+	// that the text is one argument that cannot be taken for an option (the first
	252	+	// character is a letter or a digit) and holds no space or control character.
	253	+	var imageRefRe = regexp.MustCompile(
	254	+	`^(?:(?:[A-Za-z0-9]\|[A-Za-z0-9][A-Za-z0-9-][A-Za-z0-9])(?:\.(?:[A-Za-z0-9]\|[A-Za-z0-9][A-Za-z0-9-][A-Za-z0-9]))*(?::[0-9]+)?/)?` +
	255	+	`[A-Za-z0-9]+(?:(?:[._]\|__\|-+)[A-Za-z0-9]+)(?:/[A-Za-z0-9]+(?:(?:[._]\|__\|-+)[A-Za-z0-9]+))*` +
	256	+	`(?::[A-Za-z0-9_][A-Za-z0-9_.-]{0,127})?` +
	257	+	`(?:@[A-Za-z][A-Za-z0-9](?:[-_+.][A-Za-z][A-Za-z0-9])*:[0-9A-Fa-f]{32,})?$`)
	258	+
	259	+	// validImageRef reports whether image can be passed to docker pull and create.
	260	+	func validImageRef(image string) bool { return len(image) <= 1024 && imageRefRe.MatchString(image) }
	261	+
	262	+	// aliasRe is the name of a service: it becomes a network alias and is never
	263	+	// taken for an option.
	264	+	var aliasRe = regexp.MustCompile(`^[A-Za-z0-9_][A-Za-z0-9_.-]{0,62}$`)
	265	+
	266	+	// containerIDRe is the id docker create prints.
	267	+	var containerIDRe = regexp.MustCompile(`^[0-9a-f]{64}$`)
	268	+
246	269		// serviceCommandAllowed is the feature flag of entrypoint: and command: of a
247	270		// service (ExecutionContext.cs:1545: actions_service_container_command or
248	271		// ACTIONS_SERVICE_CONTAINER_COMMAND; ermite has no feature flags).

@@ -299,6 +322,9 @@ func (j job) convertContainer(v expr.Value, service bool) (containerSpec, erro
299	322		if spec.image == "" {
300	323		return nil, nil
301	324		}
	325	+	if !validImageRef(spec.image) {
	326	+	return nil, ghErrorf("Invalid container image '%s'", spec.image)
	327	+	}
302	328		return spec, nil
303	329		}
304	330

@@ -376,6 +402,9 @@ func (j job) evalContainers() (job container, services []*container, err error
376	402		return nil, nil, ghErrorf("Unexpected type '%s' encountered while reading 'services'. The type 'MappingToken' was expected.", v.Kind())
377	403		}
378	404		for _, p := range d.Pairs() {
	405	+	if !aliasRe.MatchString(p.Key) {
	406	+	return nil, nil, ghErrorf("Invalid service name '%s': use letters, digits, '_', '.' and '-', starting with a letter, a digit or '_'", p.Key)
	407	+	}
379	408		spec, e := j.convertContainer(p.Value, true)
380	409		if e != nil {
381	410		return nil, nil, e


@@ -5,6 +5,7 @@ package executor
5	import (	5		import (
6	"context"	6		"context"
7	"fmt"	7		"fmt"
		8	+	"regexp"
8	"strings"	9		"strings"
9		10
10	"bnema.dev/ermite-actions/expr"	11		"bnema.dev/ermite-actions/expr"

@@ -243,6 +244,28 @@ func containerStrings(v expr.Value, what string) ([]string, error) {
243	return out, nil	244		return out, nil
244	}	245		}
245		246
		247	+	// imageRefRe is the grammar of a docker image reference
		248	+	// (github.com/distribution/reference): [domain[:port]/]path[:tag][@digest].
		249	+	// Path components may be upper case here: docker words that error itself, as
		250	+	// it does for the image of the recorded ctr-errors scenario. What matters is
		251	+	// that the text is one argument that cannot be taken for an option (the first
		252	+	// character is a letter or a digit) and holds no space or control character.
		253	+	var imageRefRe = regexp.MustCompile(
		254	+	`^(?:(?:[A-Za-z0-9]\|[A-Za-z0-9][A-Za-z0-9-][A-Za-z0-9])(?:\.(?:[A-Za-z0-9]\|[A-Za-z0-9][A-Za-z0-9-][A-Za-z0-9]))*(?::[0-9]+)?/)?` +
		255	+	`[A-Za-z0-9]+(?:(?:[._]\|__\|-+)[A-Za-z0-9]+)(?:/[A-Za-z0-9]+(?:(?:[._]\|__\|-+)[A-Za-z0-9]+))*` +
		256	+	`(?::[A-Za-z0-9_][A-Za-z0-9_.-]{0,127})?` +
		257	+	`(?:@[A-Za-z][A-Za-z0-9](?:[-_+.][A-Za-z][A-Za-z0-9])*:[0-9A-Fa-f]{32,})?$`)
		258	+
		259	+	// validImageRef reports whether image can be passed to docker pull and create.
		260	+	func validImageRef(image string) bool { return len(image) <= 1024 && imageRefRe.MatchString(image) }
		261	+
		262	+	// aliasRe is the name of a service: it becomes a network alias and is never
		263	+	// taken for an option.
		264	+	var aliasRe = regexp.MustCompile(`^[A-Za-z0-9_][A-Za-z0-9_.-]{0,62}$`)
		265	+
		266	+	// containerIDRe is the id docker create prints.
		267	+	var containerIDRe = regexp.MustCompile(`^[0-9a-f]{64}$`)
		268	+
246	// serviceCommandAllowed is the feature flag of entrypoint: and command: of a	269		// serviceCommandAllowed is the feature flag of entrypoint: and command: of a
247	// service (ExecutionContext.cs:1545: actions_service_container_command or	270		// service (ExecutionContext.cs:1545: actions_service_container_command or
248	// ACTIONS_SERVICE_CONTAINER_COMMAND; ermite has no feature flags).	271		// ACTIONS_SERVICE_CONTAINER_COMMAND; ermite has no feature flags).

@@ -299,6 +322,9 @@ func (j job) convertContainer(v expr.Value, service bool) (containerSpec, erro
299	if spec.image == "" {	322		if spec.image == "" {
300	return nil, nil	323		return nil, nil
301	}	324		}
		325	+	if !validImageRef(spec.image) {
		326	+	return nil, ghErrorf("Invalid container image '%s'", spec.image)
		327	+	}
302	return spec, nil	328		return spec, nil
303	}	329		}
304		330

@@ -376,6 +402,9 @@ func (j job) evalContainers() (job container, services []*container, err error
376	return nil, nil, ghErrorf("Unexpected type '%s' encountered while reading 'services'. The type 'MappingToken' was expected.", v.Kind())	402		return nil, nil, ghErrorf("Unexpected type '%s' encountered while reading 'services'. The type 'MappingToken' was expected.", v.Kind())
377	}	403		}
378	for _, p := range d.Pairs() {	404		for _, p := range d.Pairs() {
		405	+	if !aliasRe.MatchString(p.Key) {
		406	+	return nil, nil, ghErrorf("Invalid service name '%s': use letters, digits, '_', '.' and '-', starting with a letter, a digit or '_'", p.Key)
		407	+	}
379	spec, e := j.convertContainer(p.Value, true)	408		spec, e := j.convertContainer(p.Value, true)
380	if e != nil {	409		if e != nil {
381	return nil, nil, e	410		return nil, nil, e

M internal/executor/containers_spec_test.go +46 -0


@@ -95,3 +95,49 @@ func TestConvertContainerForms(t *testing.T) {
95	95		t.Errorf("%+v %v", s, err)
96	96		}
97	97		}
	98	+
	99	+	func TestImageReferenceGrammar(t *testing.T) {
	100	+	good := []string{"ubuntu", "ubuntu:24.04", "ghcr.io/o/img:1", "localhost:5000/a/b:tag", "a/b_c.d-e", "redis@sha256:" + strings.Repeat("a", 64),
	101	+	"r.io:443/x/y:1@sha256:" + strings.Repeat("0", 64), "Ubuntu:24.04"}
	102	+	bad := []string{"", "--help", "-x", "-", " a", "a b", "a\nb", "a;b", "a$(x)", "a:", "a:-x", "a@sha256:zz", "/a", "a//b", "a:b:c", strings.Repeat("a", 1100)}
	103	+	for _, g := range good {
	104	+	if !validImageRef(g) {
	105	+	t.Errorf("rejected %q", g)
	106	+	}
	107	+	}
	108	+	for _, b := range bad {
	109	+	if validImageRef(b) {
	110	+	t.Errorf("accepted %q", b)
	111	+	}
	112	+	}
	113	+	j := &job{}
	114	+	if _, err := j.convertContainer(mustValue(t, `"--help"`), false); err == nil \|\| !strings.Contains(err.Error(), "Invalid container image '--help'") {
	115	+	t.Errorf("container: --help: %v", err)
	116	+	}
	117	+	}
	118	+
	119	+	func TestServiceNamesAndContainerIDs(t *testing.T) {
	120	+	f := newFakeCtr(t)
	121	+	for _, name := range []string{"-x", ".y", "a b", "a/b"} {
	122	+	rec, res := ctrRun(t, f, " services:\n \""+name+"\":\n image: redis:7\n", " - run: echo never\n", nil)
	123	+	if res.Conclusion != ConclusionFailure \|\| !strings.Contains(logOf(rec.step(t, "Set up job")), "Invalid service name") {
	124	+	t.Errorf("%q: %+v", name, res)
	125	+	}
	126	+	}
	127	+	for id, ok := range map[string]bool{strings.Repeat("a", 64): true, strings.Repeat("0", 64): true, "Usage: docker create": false, strings.Repeat("a", 63): false, strings.Repeat("A", 64): false, "--help": false} {
	128	+	if containerIDRe.MatchString(id) != ok {
	129	+	t.Errorf("id %q", id)
	130	+	}
	131	+	}
	132	+	// A help text printed by create is not taken for an id: nothing is started.
	133	+	f.set(t, "create_out", "Usage: docker create [OPTIONS] IMAGE\n")
	134	+	rec, res := ctrRun(t, f, " container: ubuntu:24.04\n", " - run: echo never\n", nil)
	135	+	if res.Conclusion != ConclusionFailure \|\| !strings.Contains(logOf(rec.step(t, "Initialize containers")), "##[error]Docker create did not return a container id: 'Usage: docker create [OPTIONS] IMAGE'") {
	136	+	t.Errorf("%+v\n%s", res, logOf(rec.step(t, "Initialize containers")))
	137	+	}
	138	+	for _, c := range f.calls(t) {
	139	+	if c[0] == "start" {
	140	+	t.Errorf("started %v", c)
	141	+	}
	142	+	}
	143	+	}


@@ -95,3 +95,49 @@ func TestConvertContainerForms(t *testing.T) {
95	t.Errorf("%+v %v", s, err)	95		t.Errorf("%+v %v", s, err)
96	}	96		}
97	}	97		}
		98	+
		99	+	func TestImageReferenceGrammar(t *testing.T) {
		100	+	good := []string{"ubuntu", "ubuntu:24.04", "ghcr.io/o/img:1", "localhost:5000/a/b:tag", "a/b_c.d-e", "redis@sha256:" + strings.Repeat("a", 64),
		101	+	"r.io:443/x/y:1@sha256:" + strings.Repeat("0", 64), "Ubuntu:24.04"}
		102	+	bad := []string{"", "--help", "-x", "-", " a", "a b", "a\nb", "a;b", "a$(x)", "a:", "a:-x", "a@sha256:zz", "/a", "a//b", "a:b:c", strings.Repeat("a", 1100)}
		103	+	for _, g := range good {
		104	+	if !validImageRef(g) {
		105	+	t.Errorf("rejected %q", g)
		106	+	}
		107	+	}
		108	+	for _, b := range bad {
		109	+	if validImageRef(b) {
		110	+	t.Errorf("accepted %q", b)
		111	+	}
		112	+	}
		113	+	j := &job{}
		114	+	if _, err := j.convertContainer(mustValue(t, `"--help"`), false); err == nil \|\| !strings.Contains(err.Error(), "Invalid container image '--help'") {
		115	+	t.Errorf("container: --help: %v", err)
		116	+	}
		117	+	}
		118	+
		119	+	func TestServiceNamesAndContainerIDs(t *testing.T) {
		120	+	f := newFakeCtr(t)
		121	+	for _, name := range []string{"-x", ".y", "a b", "a/b"} {
		122	+	rec, res := ctrRun(t, f, " services:\n \""+name+"\":\n image: redis:7\n", " - run: echo never\n", nil)
		123	+	if res.Conclusion != ConclusionFailure \|\| !strings.Contains(logOf(rec.step(t, "Set up job")), "Invalid service name") {
		124	+	t.Errorf("%q: %+v", name, res)
		125	+	}
		126	+	}
		127	+	for id, ok := range map[string]bool{strings.Repeat("a", 64): true, strings.Repeat("0", 64): true, "Usage: docker create": false, strings.Repeat("a", 63): false, strings.Repeat("A", 64): false, "--help": false} {
		128	+	if containerIDRe.MatchString(id) != ok {
		129	+	t.Errorf("id %q", id)
		130	+	}
		131	+	}
		132	+	// A help text printed by create is not taken for an id: nothing is started.
		133	+	f.set(t, "create_out", "Usage: docker create [OPTIONS] IMAGE\n")
		134	+	rec, res := ctrRun(t, f, " container: ubuntu:24.04\n", " - run: echo never\n", nil)
		135	+	if res.Conclusion != ConclusionFailure \|\| !strings.Contains(logOf(rec.step(t, "Initialize containers")), "##[error]Docker create did not return a container id: 'Usage: docker create [OPTIONS] IMAGE'") {
		136	+	t.Errorf("%+v\n%s", res, logOf(rec.step(t, "Initialize containers")))
		137	+	}
		138	+	for _, c := range f.calls(t) {
		139	+	if c[0] == "start" {
		140	+	t.Errorf("started %v", c)
		141	+	}
		142	+	}
		143	+	}

M internal/executor/containers_start.go +5 -0


@@ -121,6 +121,11 @@ func (j job) startContainer(s stepRun, c *container, cancel <-chan struct{}) e
121	121		if len(out) == 0 \|\| out[0] == "" {
122	122		return ghErrorf("Value cannot be null. (Parameter 'container.ContainerId')")
123	123		}
	124	+	// docker create prints the id and nothing else; anything else (the help
	125	+	// text of an option in options:, say) is not an id to use or to remove.
	126	+	if !containerIDRe.MatchString(out[0]) {
	127	+	return ghErrorf("Docker create did not return a container id: '%s'", out[0])
	128	+	}
124	129		c.id = out[0]
125	130		code, err = j.dockerCode(s, j.newDockerCmd("start", raw(c.id)), cancel)
126	131		if err != nil {


@@ -121,6 +121,11 @@ func (j job) startContainer(s stepRun, c *container, cancel <-chan struct{}) e
121	if len(out) == 0 \|\| out[0] == "" {	121		if len(out) == 0 \|\| out[0] == "" {
122	return ghErrorf("Value cannot be null. (Parameter 'container.ContainerId')")	122		return ghErrorf("Value cannot be null. (Parameter 'container.ContainerId')")
123	}	123		}
		124	+	// docker create prints the id and nothing else; anything else (the help
		125	+	// text of an option in options:, say) is not an id to use or to remove.
		126	+	if !containerIDRe.MatchString(out[0]) {
		127	+	return ghErrorf("Docker create did not return a container id: '%s'", out[0])
		128	+	}
124	c.id = out[0]	129		c.id = out[0]
125	code, err = j.dockerCode(s, j.newDockerCmd("start", raw(c.id)), cancel)	130		code, err = j.dockerCode(s, j.newDockerCmd("start", raw(c.id)), cancel)
126	if err != nil {	131		if err != nil {