fix: coerce every expression value kind to a string and drop the format() panic (#1135)

`coerceToString` covered only a few kinds and returned the unconverted `reflect.Value` for the rest, so callers rendered contexts as `<*model.GithubContext Value>` and mangled sized integers and floats the same way. It now returns a string, which makes that placeholder unrepresentable, and is exported as `CoerceToString` so Gitea can drop its own diverging copy. It also takes an already reflected value, so internal callers need no conversion and no guard against the zero `Value`.

`format()` panicked whenever a lone `}` was followed by anything other than another `}`. Only a trailing `}` reached the existing unmatched-brace check, so an expression such as `format('a}b')`, which any workflow can write, took the process down instead. It now returns that same error.

Reviewed-on: https://gitea.com/gitea/runner/pulls/1135
Reviewed-by: Lunny Xiao <xiaolunwen@gmail.com>
Co-authored-by: silverwind <me@silverwind.io>

.golangci.yml

@@ -37,12 +37,8 @@ linters:
       rules:
         main:
           deny:
-            - pkg: io/ioutil
-              desc: use os or io instead
             - pkg: golang.org/x/exp
               desc: it's experimental and unreliable
-            - pkg: github.com/pkg/errors
-              desc: use builtin errors package instead
     nolintlint:
       allow-unused: false
       require-explanation: true

AGENTS.md

@@ -1,10 +1,18 @@
+- Never assume, verify before claiming
 - Use `make help` to find available development targets
-- Run `make fmt` to format `.go` files, and run `make lint-go` to lint them
+- PR descriptions: minimal, only what and why, no task lists or file listings
-- Run `make tidy` after any `go.mod` changes
+- Reference issues and PRs by full URL, not by number
-- Run single go unit tests with `go test -run '^TestName$' ./modulepath/`
+- Use Conventional Commits for commit messages and PR titles, plus the `enhance` type for user-facing enhancements
+- Add an `Assisted-by: AGENT_NAME:MODEL_VERSION` trailer to commit messages, never `Co-Authored-By` or `Signed-off-by`
+- Attribute agent authorship on one trailing line in issue and pull request comments, never as a PR description section
+- Never force-push, amend, or squash unless asked. Use new commits and normal push for pull request updates
+- Comments: write almost none, short and preferably same-line, explaining why for a future reader. Never narrate code, the change or the prompt. Preserve existing ones that still apply
 - Add the current year into the copyright header of new `.go` files
 - Ensure no trailing whitespace in edited files
-- Never force-push, amend, or squash unless asked. Use new commits and normal push for pull request updates
+- Run `make fmt` after `.go` edits, `make tidy` after `go.mod` edits, and `make checks` for the non-lint source checks
-- Preserve existing code comments, do not remove or rewrite comments that are still relevant
+- Lint what changed with `make lint-go`, and `make lint-go-windows` for Windows and platform-split files
-- Include authorship attribution in issue and pull request comments
+- Fix the cause rather than disabling a linter or weakening a test. Where unavoidable, use the narrowest scope with a trailing comment giving the reason
-- Add `Co-Authored-By` lines to all commits, indicating name and model used
+- Run single go tests with `go test -run '^TestName$' ./modulepath/`. `make test` self-skips the integration tests without docker or network, `make test-dind` runs the daemon-facing tests against the built dind image
+- Write the fewest, fastest tests covering the behavior, extending an existing one where possible. Prefer unit tests where logic is testable in isolation
+- Wait on a deterministic condition rather than `sleep`
+- Update the files under `docs/` when behavior documented there changes

act/container/docker_cli.go

@@ -4,8 +4,10 @@
 
 //go:build !(WITHOUT_DOCKER || !(linux || darwin || windows || netbsd))
 
-// This file is exact copy of https://github.com/docker/cli/blob/9a471180cb7d39c236d090399a9d362c3f5a8ebd/cli/command/container/opts.go
+// This file is exact copy of https://github.com/docker/cli/blob/dfc4efb1e2ab8c06d70d2a1366ad448d2f917e90/cli/command/container/opts.go with:
-// appended with license information.
+// * appended with license information
+// * regexp and loader.ParseVolume in place of the import-restricted internal/lazyregexp and internal/volumespec
+// * invalidParameter from the package's errors.go, and convertPortSet/convertPortMap for the callers in docker_run.go
 //
 // docker/cli is licensed under the Apache License, Version 2.0.
 // See DOCKER_LICENSE for the full license text.
@@ -30,6 +32,7 @@ import (
 	"strings"
 	"time"
 
+	cerrdefs "github.com/containerd/errdefs"
 	"github.com/docker/cli/cli/compose/loader"
 	"github.com/docker/cli/opts"
 	"github.com/docker/go-connections/nat"
@@ -380,7 +383,7 @@ func parse(flags *pflag.FlagSet, copts *containerOptions, serverOS string) (*con
 	var binds []string
 	volumes := copts.volumes.GetMap()
 	// add any bind targets to the list of container volumes
-	for bind := range copts.volumes.GetMap() {
+	for bind := range volumes {
 		parsed, err := loader.ParseVolume(bind)
 		if err != nil {
 			return nil, err
@@ -515,13 +518,13 @@ func parse(flags *pflag.FlagSet, copts *containerOptions, serverOS string) (*con
 	// collect all the environment variables for the container
 	envVariables, err := opts.ReadKVEnvStrings(copts.envFile.GetSlice(), copts.env.GetSlice())
 	if err != nil {
-		return nil, err
+		return nil, fmt.Errorf("--env-file: %w", err)
 	}
 
 	// collect all the labels for the container
 	labels, err := opts.ReadKVStrings(copts.labelsFile.GetSlice(), copts.labels.GetSlice())
 	if err != nil {
-		return nil, err
+		return nil, fmt.Errorf("--label-file: %w", err)
 	}
 
 	pidMode := container.PidMode(copts.pidMode)
@@ -1164,16 +1167,17 @@ func toNetipAddrSlice(ips []string) []netip.Addr {
 }
 
 // invalidParameter wraps an error to indicate it was caused by invalid input.
-// This is a local replacement for docker/docker/errdefs.InvalidParameter.
+// This is a copy of docker/cli's cli/command/container/errors.go, which is not importable.
-type invalidParameterError struct{ error }
+type invalidParameterErr struct{ error }
 
-func (e invalidParameterError) InvalidParameter() {}
+func (invalidParameterErr) InvalidParameter() {}
+func (e invalidParameterErr) Unwrap() error   { return e.error }
 
 func invalidParameter(err error) error {
-	if err == nil {
+	if err == nil || cerrdefs.IsInvalidArgument(err) {
-		return nil
+		return err
 	}
-	return invalidParameterError{err}
+	return invalidParameterErr{err}
 }
 
 func convertPortSet(ports nat.PortSet) (network.PortSet, error) {

act/container/docker_cli_test.go

@@ -2,20 +2,22 @@
 // Copyright 2022 The nektos/act Authors. All rights reserved.
 // SPDX-License-Identifier: MIT
 
-// This file is exact copy of https://github.com/docker/cli/blob/9ac8584acfd501c3f4da0e845e3a40ed15c85041/cli/command/container/opts_test.go with:
+// This file is exact copy of https://github.com/docker/cli/blob/dfc4efb1e2ab8c06d70d2a1366ad448d2f917e90/cli/command/container/opts_test.go with:
 // * appended with license information
-// * commented out case 'invalid-mixed-network-types' in test TestParseNetworkConfig
+// * added tests for the locally changed parseDevice, validateDevice and invalidParameter
 //
 // docker/cli is licensed under the Apache License, Version 2.0.
 // See DOCKER_LICENSE for the full license text.
 //
 
-//nolint:depguard,gocritic // verbatim copy from docker/cli tests
+//nolint:gocritic // verbatim copy from docker/cli tests
 package container
 
 import (
+	"errors"
 	"fmt"
 	"io"
+	"net"
 	"net/netip"
 	"os"
 	"runtime"
@@ -23,18 +25,23 @@ import (
 	"testing"
 	"time"
 
-	"github.com/docker/go-connections/nat"
-	"github.com/google/go-cmp/cmp"
 	"github.com/google/go-cmp/cmp/cmpopts"
 	"github.com/moby/moby/api/types/container"
 	networktypes "github.com/moby/moby/api/types/network"
-	"github.com/pkg/errors"
 	"github.com/spf13/pflag"
 	"gotest.tools/v3/assert"
 	is "gotest.tools/v3/assert/cmp"
 	"gotest.tools/v3/skip"
 )
 
+func mustParseMAC(s string) networktypes.HardwareAddr {
+	mac, err := net.ParseMAC(s)
+	if err != nil {
+		panic(err)
+	}
+	return networktypes.HardwareAddr(mac)
+}
+
 func TestValidateAttach(t *testing.T) {
 	valid := []string{
 		"stdin",
@@ -64,12 +71,12 @@ func parseRun(args []string) (*container.Config, *container.HostConfig, *network
 	if err := flags.Parse(args); err != nil {
 		return nil, nil, nil, err
 	}
-	// TODO: fix tests to accept ContainerConfig
+	// TODO(dnephin): fix tests to accept ContainerConfig; see https://github.com/moby/moby/pull/31621
-	containerConfig, err := parse(flags, copts, runtime.GOOS)
+	containerCfg, err := parse(flags, copts, runtime.GOOS)
 	if err != nil {
 		return nil, nil, nil, err
 	}
-	return containerConfig.Config, containerConfig.HostConfig, containerConfig.NetworkingConfig, err
+	return containerCfg.Config, containerCfg.HostConfig, containerCfg.NetworkingConfig, err
 }
 
 func setupRunFlags() (*pflag.FlagSet, *containerOptions) {
@@ -82,20 +89,81 @@ func setupRunFlags() (*pflag.FlagSet, *containerOptions) {
 
 func mustParse(t *testing.T, args string) (*container.Config, *container.HostConfig, *networktypes.NetworkingConfig) {
 	t.Helper()
-	config, hostConfig, networkingConfig, err := parseRun(append(strings.Split(args, " "), "ubuntu", "bash"))
+	config, hostConfig, nwConfig, err := parseRun(append(strings.Split(args, " "), "ubuntu", "bash"))
 	assert.NilError(t, err)
-	return config, hostConfig, networkingConfig
+	return config, hostConfig, nwConfig
 }
 
 func TestParseRunLinks(t *testing.T) {
-	if _, hostConfig, _ := mustParse(t, "--link a:b"); len(hostConfig.Links) == 0 || hostConfig.Links[0] != "a:b" {
+	tests := []struct {
-		t.Fatalf("Error parsing links. Expected []string{\"a:b\"}, received: %v", hostConfig.Links)
+		name               string
+		input              string
+		expHostConfigLinks []string
+		expNetConfigLinks  map[string][]string
+	}{
+		// Default bridge - legacy links ...
+		{
+			name:               "default/onelink",
+			input:              "--link a:b",
+			expHostConfigLinks: []string{"a:b"},
+			expNetConfigLinks:  map[string][]string{"default": nil},
+		},
+		{
+			name:               "default/twolinks",
+			input:              "--link a:b --link c:d",
+			expHostConfigLinks: []string{"a:b", "c:d"},
+			expNetConfigLinks:  map[string][]string{"default": nil},
+		},
+		{
+			name:               "bridge/onelink",
+			input:              "--network bridge --link a:b",
+			expHostConfigLinks: []string{"a:b"},
+			// expNetConfigLinks - no EndpointsConfig is created for a single named network with no options set.
+			// See the "For backward compatibility" comment in parseNetworkOpts().
+		},
+		{
+			name:              "default/nolinks",
+			expNetConfigLinks: map[string][]string{"default": nil},
+		},
+
+		// User-defined bridge - links become DNS aliases ...
+		{
+			name:               "userdefnet/onelink",
+			input:              "--network userdefnet --link a:b",
+			expHostConfigLinks: []string{"a:b"},
+			expNetConfigLinks:  map[string][]string{"userdefnet": {"a:b"}},
+		},
+		{
+			name:               "userdefnet/twolinks",
+			input:              "--network userdefnet --link a:b --link c:d",
+			expHostConfigLinks: []string{"a:b", "c:d"},
+			expNetConfigLinks:  map[string][]string{"userdefnet": {"a:b", "c:d"}},
+		},
+		{
+			name:  "userdefnet/nolinks",
+			input: "--network userdefnet",
+		},
+		{
+			// Link options are applied to the first network (and there's no "advanced syntax"
+			// link key, like "--network name=userdefnet,link=a:b").
+			name:               "links apply to the first network",
+			input:              "--network userdefnet --link a:b --network bar --link c:d",
+			expHostConfigLinks: []string{"a:b", "c:d"},
+			expNetConfigLinks:  map[string][]string{"userdefnet": {"a:b", "c:d"}, "bar": nil},
+		},
 	}
-	if _, hostConfig, _ := mustParse(t, "--link a:b --link c:d"); len(hostConfig.Links) < 2 || hostConfig.Links[0] != "a:b" || hostConfig.Links[1] != "c:d" {
+
-		t.Fatalf("Error parsing links. Expected []string{\"a:b\", \"c:d\"}, received: %v", hostConfig.Links)
+	for _, tc := range tests {
+		t.Run(tc.name, func(t *testing.T) {
+			_, hostConfig, netConfig := mustParse(t, tc.input)
+			assert.Check(t, is.DeepEqual(hostConfig.Links, tc.expHostConfigLinks))
+			assert.Check(t, is.Len(netConfig.EndpointsConfig, len(tc.expNetConfigLinks)))
+			for netName, expLinks := range tc.expNetConfigLinks {
+				nc, ok := netConfig.EndpointsConfig[netName]
+				assert.Assert(t, ok)
+				assert.Check(t, is.DeepEqual(nc.Links, expLinks))
 			}
-	if _, hostConfig, _ := mustParse(t, ""); len(hostConfig.Links) != 0 {
+		})
-		t.Fatalf("Error parsing links. No link expected, received: %v", hostConfig.Links)
 	}
 }
 
@@ -294,37 +362,7 @@ func compareRandomizedStrings(a, b, c, d string) error {
 	if a == d && b == c {
 		return nil
 	}
-	return errors.Errorf("strings don't match")
+	return errors.New("strings don't match")
-}
-
-func mustNetworkPort(t *testing.T, value string) networktypes.Port {
-	t.Helper()
-
-	port, err := networktypes.ParsePort(value)
-	if err != nil {
-		t.Fatalf("failed to parse network port %q: %v", value, err)
-	}
-	return port
-}
-
-func mustAddr(t *testing.T, value string) netip.Addr {
-	t.Helper()
-
-	addr, err := netip.ParseAddr(value)
-	if err != nil {
-		t.Fatalf("failed to parse address %q: %v", value, err)
-	}
-	return addr
-}
-
-func mustAddrs(t *testing.T, values ...string) []netip.Addr {
-	t.Helper()
-
-	addrs := make([]netip.Addr, 0, len(values))
-	for _, value := range values {
-		addrs = append(addrs, mustAddr(t, value))
-	}
-	return addrs
 }
 
 // Simple parse with MacAddress validation
@@ -334,10 +372,11 @@ func TestParseWithMacAddress(t *testing.T) {
 	if _, _, _, err := parseRun([]string{invalidMacAddress, "img", "cmd"}); err != nil && err.Error() != "invalidMacAddress is not a valid mac address" {
 		t.Fatalf("Expected an error with %v mac-address, got %v", invalidMacAddress, err)
 	}
-	_, hostConfig, networkingConfig := mustParse(t, validMacAddress)
+	_, hostConfig, nwConfig := mustParse(t, validMacAddress)
-	endpoint := networkingConfig.EndpointsConfig[string(hostConfig.NetworkMode)]
+	defaultNw := hostConfig.NetworkMode.NetworkName()
-	assert.Check(t, endpoint != nil)
+	if nwConfig.EndpointsConfig[defaultNw].MacAddress.String() != "92:d0:c6:0a:29:33" {
-	assert.Equal(t, "92:d0:c6:0a:29:33", endpoint.MacAddress.String())
+		t.Fatalf("Expected the default endpoint to have the MacAddress '92:d0:c6:0a:29:33' set, got '%v'", nwConfig.EndpointsConfig[defaultNw].MacAddress)
+	}
 }
 
 func TestRunFlagsParseWithMemory(t *testing.T) {
@@ -408,93 +447,144 @@ func TestParseHostnameDomainname(t *testing.T) {
 }
 
 func TestParseWithExpose(t *testing.T) {
-	invalids := []string{
+	t.Run("invalid", func(t *testing.T) {
-		":",
+		tests := map[string]string{
-		"8080:9090",
+			":":                   `invalid range format for --expose: invalid start port ':': invalid syntax`,
-		"/tcp",
+			"8080:9090":           `invalid range format for --expose: invalid start port '8080:9090': invalid syntax`,
-		"/udp",
+			"/tcp":                `invalid range format for --expose: invalid start port '': value is empty`,
-		"NaN/tcp",
+			"/udp":                `invalid range format for --expose: invalid start port '': value is empty`,
-		"NaN-NaN/tcp",
+			"NaN/tcp":             `invalid range format for --expose: invalid start port 'NaN': invalid syntax`,
-		"8080-NaN/tcp",
+			"NaN-NaN/tcp":         `invalid range format for --expose: invalid start port 'NaN': invalid syntax`,
-		"1234567890-8080/tcp",
+			"8080-NaN/tcp":        `invalid range format for --expose: invalid end port 'NaN': invalid syntax`,
+			"1234567890-8080/tcp": `invalid range format for --expose: invalid start port '1234567890': value out of range`,
 		}
-	valids := map[string][]nat.Port{
+		for expose, expectedError := range tests {
-		"8080/tcp":      {"8080/tcp"},
+			t.Run(expose, func(t *testing.T) {
-		"8080/udp":      {"8080/udp"},
+				_, _, _, err := parseRun([]string{fmt.Sprintf("--expose=%v", expose), "img", "cmd"})
-		"8080/ncp":      {"8080/ncp"},
+				assert.Error(t, err, expectedError)
-		"8080-8080/udp": {"8080/udp"},
+			})
-		"8080-8082/tcp": {"8080/tcp", "8081/tcp", "8082/tcp"},
 		}
-	for _, expose := range invalids {
+	})
-		if _, _, _, err := parseRun([]string{fmt.Sprintf("--expose=%v", expose), "img", "cmd"}); err == nil {
+	t.Run("valid", func(t *testing.T) {
-			t.Fatalf("Expected error with '--expose=%v', got none", expose)
+		tests := map[string][]networktypes.Port{
+			"8080/tcp":      {networktypes.MustParsePort("8080/tcp")},
+			"8080/udp":      {networktypes.MustParsePort("8080/udp")},
+			"8080/ncp":      {networktypes.MustParsePort("8080/ncp")},
+			"8080-8080/udp": {networktypes.MustParsePort("8080/udp")},
+			"8080-8082/tcp": {networktypes.MustParsePort("8080/tcp"), networktypes.MustParsePort("8081/tcp"), networktypes.MustParsePort("8082/tcp")},
 		}
-	}
+		for expose, exposedPorts := range tests {
-	for expose, exposedPorts := range valids {
+			t.Run(expose, func(t *testing.T) {
 				config, _, _, err := parseRun([]string{fmt.Sprintf("--expose=%v", expose), "img", "cmd"})
-		if err != nil {
+				assert.NilError(t, err)
-			t.Fatal(err)
-		}
-		if len(config.ExposedPorts) != len(exposedPorts) {
-			t.Fatalf("Expected %v exposed port, got %v", len(exposedPorts), len(config.ExposedPorts))
-		}
 				for _, port := range exposedPorts {
-			if _, ok := config.ExposedPorts[mustNetworkPort(t, string(port))]; !ok {
+					_, ok := config.ExposedPorts[port]
-				t.Fatalf("Expected %v, got %v", exposedPorts, config.ExposedPorts)
+					assert.Check(t, ok, "missing port %q in exposed ports: %#+v", port, config.ExposedPorts[port])
-			}
 				}
+			})
 		}
+	})
+
+	t.Run("merge with published", func(t *testing.T) {
 		// Merge with actual published port
 		config, _, _, err := parseRun([]string{"--publish=80", "--expose=80-81/tcp", "img", "cmd"})
-	if err != nil {
+		assert.NilError(t, err)
-		t.Fatal(err)
+		assert.Check(t, is.Len(config.ExposedPorts, 2))
-	}
+		ports := []networktypes.Port{networktypes.MustParsePort("80/tcp"), networktypes.MustParsePort("81/tcp")}
-	if len(config.ExposedPorts) != 2 {
-		t.Fatalf("Expected 2 exposed ports, got %v", config.ExposedPorts)
-	}
-	ports := []nat.Port{"80/tcp", "81/tcp"}
 		for _, port := range ports {
-		if _, ok := config.ExposedPorts[mustNetworkPort(t, string(port))]; !ok {
+			_, ok := config.ExposedPorts[port]
-			t.Fatalf("Expected %v, got %v", ports, config.ExposedPorts)
+			assert.Check(t, ok, "missing port %q in exposed ports: %#+v", port, config.ExposedPorts[port])
-		}
 		}
+	})
 }
 
 func TestParseDevice(t *testing.T) {
 	skip.If(t, runtime.GOOS != "linux") // Windows and macOS validate server-side
-	valids := map[string]container.DeviceMapping{
+	testCases := []struct {
-		"/dev/snd": {
+		devices        []string
+		deviceMapping  *container.DeviceMapping
+		deviceRequests []container.DeviceRequest
+	}{
+		{
+			devices: []string{"/dev/snd"},
+			deviceMapping: &container.DeviceMapping{
 				PathOnHost:        "/dev/snd",
 				PathInContainer:   "/dev/snd",
 				CgroupPermissions: "rwm",
 			},
-		"/dev/snd:rw": {
+		},
+		{
+			devices: []string{"/dev/snd:rw"},
+			deviceMapping: &container.DeviceMapping{
 				PathOnHost:        "/dev/snd",
 				PathInContainer:   "/dev/snd",
 				CgroupPermissions: "rw",
 			},
-		"/dev/snd:/something": {
+		},
+		{
+			devices: []string{"/dev/snd:/something"},
+			deviceMapping: &container.DeviceMapping{
 				PathOnHost:        "/dev/snd",
 				PathInContainer:   "/something",
 				CgroupPermissions: "rwm",
 			},
-		"/dev/snd:/something:rw": {
+		},
+		{
+			devices: []string{"/dev/snd:/something:rw"},
+			deviceMapping: &container.DeviceMapping{
 				PathOnHost:        "/dev/snd",
 				PathInContainer:   "/something",
 				CgroupPermissions: "rw",
 			},
+		},
+		{
+			devices:       []string{"vendor.com/class=name"},
+			deviceMapping: nil,
+			deviceRequests: []container.DeviceRequest{
+				{
+					Driver:    "cdi",
+					DeviceIDs: []string{"vendor.com/class=name"},
+				},
+			},
+		},
+		{
+			devices: []string{"vendor.com/class=name", "/dev/snd:/something:rw"},
+			deviceMapping: &container.DeviceMapping{
+				PathOnHost:        "/dev/snd",
+				PathInContainer:   "/something",
+				CgroupPermissions: "rw",
+			},
+			deviceRequests: []container.DeviceRequest{
+				{
+					Driver:    "cdi",
+					DeviceIDs: []string{"vendor.com/class=name"},
+				},
+			},
+		},
 	}
-	for device, deviceMapping := range valids {
+
-		_, hostconfig, _, err := parseRun([]string{fmt.Sprintf("--device=%v", device), "img", "cmd"})
+	for _, tc := range testCases {
-		if err != nil {
+		t.Run(fmt.Sprintf("%s", tc.devices), func(t *testing.T) {
-			t.Fatal(err)
+			var args []string
+			for _, d := range tc.devices {
+				args = append(args, fmt.Sprintf("--device=%v", d))
 			}
-		if len(hostconfig.Devices) != 1 {
+			args = append(args, "img", "cmd")
-			t.Fatalf("Expected 1 devices, got %v", hostconfig.Devices)
+
+			_, hostconfig, _, err := parseRun(args)
+
+			assert.NilError(t, err)
+
+			if tc.deviceMapping != nil {
+				if assert.Check(t, is.Len(hostconfig.Devices, 1)) {
+					assert.Check(t, is.DeepEqual(*tc.deviceMapping, hostconfig.Devices[0]))
 				}
-		if hostconfig.Devices[0] != deviceMapping {
+			} else {
-			t.Fatalf("Expected %v, got %v", deviceMapping, hostconfig.Devices)
+				assert.Check(t, is.Len(hostconfig.Devices, 0))
 			}
+
+			assert.Check(t, is.DeepEqual(tc.deviceRequests, hostconfig.DeviceRequests))
+		})
 	}
 }
 
@@ -576,20 +666,20 @@ func TestParseNetworkConfig(t *testing.T) {
 		name            string
 		flags           []string
 		expected        map[string]*networktypes.EndpointSettings
-		expectedCfg container.HostConfig
+		expectedHostCfg container.HostConfig
 		expectedErr     string
 	}{
 		{
 			name:            "single-network-legacy",
 			flags:           []string{"--network", "net1"},
 			expected:        map[string]*networktypes.EndpointSettings{},
-			expectedCfg: container.HostConfig{NetworkMode: "net1"},
+			expectedHostCfg: container.HostConfig{NetworkMode: "net1"},
 		},
 		{
 			name:            "single-network-advanced",
 			flags:           []string{"--network", "name=net1"},
 			expected:        map[string]*networktypes.EndpointSettings{},
-			expectedCfg: container.HostConfig{NetworkMode: "net1"},
+			expectedHostCfg: container.HostConfig{NetworkMode: "net1"},
 		},
 		{
 			name: "single-network-legacy-with-options",
@@ -607,15 +697,15 @@ func TestParseNetworkConfig(t *testing.T) {
 			expected: map[string]*networktypes.EndpointSettings{
 				"net1": {
 					IPAMConfig: &networktypes.EndpointIPAMConfig{
-						IPv4Address:  mustAddr(t, "172.20.88.22"),
+						IPv4Address:  netip.MustParseAddr("172.20.88.22"),
-						IPv6Address:  mustAddr(t, "2001:db8::8822"),
+						IPv6Address:  netip.MustParseAddr("2001:db8::8822"),
-						LinkLocalIPs: mustAddrs(t, "169.254.2.2", "fe80::169:254:2:2"),
+						LinkLocalIPs: []netip.Addr{netip.MustParseAddr("169.254.2.2"), netip.MustParseAddr("fe80::169:254:2:2")},
 					},
 					Links:   []string{"foo:bar", "bar:baz"},
 					Aliases: []string{"web1", "web2"},
 				},
 			},
-			expectedCfg: container.HostConfig{NetworkMode: "net1"},
+			expectedHostCfg: container.HostConfig{NetworkMode: "net1"},
 		},
 		{
 			name: "multiple-network-advanced-mixed",
@@ -631,14 +721,15 @@ func TestParseNetworkConfig(t *testing.T) {
 				"--network-alias", "web2",
 				"--network", "net2",
 				"--network", "name=net3,alias=web3,driver-opt=field3=value3,ip=172.20.88.22,ip6=2001:db8::8822",
+				"--network", "name=net4,mac-address=02:32:1c:23:00:04,link-local-ip=169.254.169.254",
 			},
 			expected: map[string]*networktypes.EndpointSettings{
 				"net1": {
 					DriverOpts: map[string]string{"field1": "value1"},
 					IPAMConfig: &networktypes.EndpointIPAMConfig{
-						IPv4Address:  mustAddr(t, "172.20.88.22"),
+						IPv4Address:  netip.MustParseAddr("172.20.88.22"),
-						IPv6Address:  mustAddr(t, "2001:db8::8822"),
+						IPv6Address:  netip.MustParseAddr("2001:db8::8822"),
-						LinkLocalIPs: mustAddrs(t, "169.254.2.2", "fe80::169:254:2:2"),
+						LinkLocalIPs: []netip.Addr{netip.MustParseAddr("169.254.2.2"), netip.MustParseAddr("fe80::169:254:2:2")},
 					},
 					Links:   []string{"foo:bar", "bar:baz"},
 					Aliases: []string{"web1", "web2"},
@@ -647,17 +738,23 @@ func TestParseNetworkConfig(t *testing.T) {
 				"net3": {
 					DriverOpts: map[string]string{"field3": "value3"},
 					IPAMConfig: &networktypes.EndpointIPAMConfig{
-						IPv4Address: mustAddr(t, "172.20.88.22"),
+						IPv4Address: netip.MustParseAddr("172.20.88.22"),
-						IPv6Address: mustAddr(t, "2001:db8::8822"),
+						IPv6Address: netip.MustParseAddr("2001:db8::8822"),
 					},
 					Aliases: []string{"web3"},
 				},
+				"net4": {
+					MacAddress: mustParseMAC("02:32:1c:23:00:04"),
+					IPAMConfig: &networktypes.EndpointIPAMConfig{
+						LinkLocalIPs: []netip.Addr{netip.MustParseAddr("169.254.169.254")},
 					},
-			expectedCfg: container.HostConfig{NetworkMode: "net1"},
+				},
+			},
+			expectedHostCfg: container.HostConfig{NetworkMode: "net1"},
 		},
 		{
 			name:  "single-network-advanced-with-options",
-			flags: []string{"--network", "name=net1,alias=web1,alias=web2,driver-opt=field1=value1,driver-opt=field2=value2,ip=172.20.88.22,ip6=2001:db8::8822"},
+			flags: []string{"--network", "name=net1,alias=web1,alias=web2,driver-opt=field1=value1,driver-opt=field2=value2,ip=172.20.88.22,ip6=2001:db8::8822,mac-address=02:32:1c:23:00:04"},
 			expected: map[string]*networktypes.EndpointSettings{
 				"net1": {
 					DriverOpts: map[string]string{
@@ -665,19 +762,31 @@ func TestParseNetworkConfig(t *testing.T) {
 						"field2": "value2",
 					},
 					IPAMConfig: &networktypes.EndpointIPAMConfig{
-						IPv4Address: mustAddr(t, "172.20.88.22"),
+						IPv4Address: netip.MustParseAddr("172.20.88.22"),
-						IPv6Address: mustAddr(t, "2001:db8::8822"),
+						IPv6Address: netip.MustParseAddr("2001:db8::8822"),
 					},
 					Aliases:    []string{"web1", "web2"},
+					MacAddress: mustParseMAC("02:32:1c:23:00:04"),
 				},
 			},
-			expectedCfg: container.HostConfig{NetworkMode: "net1"},
+			expectedHostCfg: container.HostConfig{NetworkMode: "net1"},
 		},
 		{
 			name:            "multiple-networks",
 			flags:           []string{"--network", "net1", "--network", "name=net2"},
 			expected:        map[string]*networktypes.EndpointSettings{"net1": {}, "net2": {}},
-			expectedCfg: container.HostConfig{NetworkMode: "net1"},
+			expectedHostCfg: container.HostConfig{NetworkMode: "net1"},
+		},
+		{
+			name:  "advanced-options-with-standalone-mac-address-flag",
+			flags: []string{"--network=name=net1,alias=foobar", "--mac-address", "52:0f:f3:dc:50:10"},
+			expected: map[string]*networktypes.EndpointSettings{
+				"net1": {
+					Aliases:    []string{"foobar"},
+					MacAddress: mustParseMAC("52:0f:f3:dc:50:10"),
+				},
+			},
+			expectedHostCfg: container.HostConfig{NetworkMode: "net1"},
 		},
 		{
 			name:        "conflict-network",
@@ -699,13 +808,26 @@ func TestParseNetworkConfig(t *testing.T) {
 			flags:       []string{"--network", "name=net1,ip=172.20.88.22,ip6=2001:db8::8822", "--ip6", "2001:db8::8822"},
 			expectedErr: `conflicting options: cannot specify both --ip6 and per-network IPv6 address`,
 		},
-		// case is skipped as it fails w/o any change
+		{
-		//
+			name:        "invalid-mixed-network-types",
-		//{
+			flags:       []string{"--network", "name=host", "--network", "net1"},
-		//	name:        "invalid-mixed-network-types",
+			expectedErr: `conflicting options: cannot attach both user-defined and non-user-defined network-modes`,
-		//	flags:       []string{"--network", "name=host", "--network", "net1"},
+		},
-		//	expectedErr: `conflicting options: cannot attach both user-defined and non-user-defined network-modes`,
+		{
-		//},
+			name:        "conflict-options-link-local-ip",
+			flags:       []string{"--network", "name=net1,link-local-ip=169.254.169.254", "--link-local-ip", "169.254.10.8"},
+			expectedErr: `conflicting options: cannot specify both --link-local-ip and per-network link-local IP addresses`,
+		},
+		{
+			name:        "conflict-options-mac-address",
+			flags:       []string{"--network", "name=net1,mac-address=02:32:1c:23:00:04", "--mac-address", "02:32:1c:23:00:04"},
+			expectedErr: `conflicting options: cannot specify both --mac-address and per-network MAC address`,
+		},
+		{
+			name:        "invalid-mac-address",
+			flags:       []string{"--network", "name=net1,mac-address=foobar"},
+			expectedErr: "foobar is not a valid mac address",
+		},
 	}
 
 	for _, tc := range tests {
@@ -718,10 +840,8 @@ func TestParseNetworkConfig(t *testing.T) {
 			}
 
 			assert.NilError(t, err)
-			assert.DeepEqual(t, hConfig.NetworkMode, tc.expectedCfg.NetworkMode)
+			assert.DeepEqual(t, hConfig.NetworkMode, tc.expectedHostCfg.NetworkMode)
-			if diff := cmp.Diff(tc.expected, nwConfig.EndpointsConfig, cmpopts.EquateComparable(netip.Addr{})); diff != "" {
+			assert.DeepEqual(t, nwConfig.EndpointsConfig, tc.expected, cmpopts.EquateComparable(netip.Addr{}))
-				t.Fatalf("unexpected endpoints (-want +got):\n%s", diff)
-			}
 		})
 	}
 }
@@ -770,42 +890,84 @@ func TestRunFlagsParseShmSize(t *testing.T) {
 }
 
 func TestParseRestartPolicy(t *testing.T) {
-	invalids := map[string]string{
+	tests := []struct {
-		"always:2:3":         "invalid restart policy format: maximum retry count must be an integer",
+		input       string
-		"on-failure:invalid": "invalid restart policy format: maximum retry count must be an integer",
+		expected    container.RestartPolicy
-	}
+		expectedErr string
-	valids := map[string]container.RestartPolicy{
+	}{
-		"": {},
+		{
-		"always": {
+			input: "",
-			Name:              "always",
-			MaximumRetryCount: 0,
 		},
-		"on-failure:1": {
+		{
-			Name:              "on-failure",
+			input: "no",
+			expected: container.RestartPolicy{
+				Name: container.RestartPolicyDisabled,
+			},
+		},
+		{
+			input:       ":1",
+			expectedErr: "invalid restart policy format: no policy provided before colon",
+		},
+		{
+			input: "always",
+			expected: container.RestartPolicy{
+				Name: container.RestartPolicyAlways,
+			},
+		},
+		{
+			input:       "always:2:3",
+			expectedErr: "invalid restart policy format: maximum retry count must be an integer",
+		},
+		{
+			input: "on-failure:1",
+			expected: container.RestartPolicy{
+				Name:              container.RestartPolicyOnFailure,
 				MaximumRetryCount: 1,
 			},
+		},
+		{
+			input:       "on-failure:invalid",
+			expectedErr: "invalid restart policy format: maximum retry count must be an integer",
+		},
+		{
+			input: "unless-stopped",
+			expected: container.RestartPolicy{
+				Name: container.RestartPolicyUnlessStopped,
+			},
+		},
+		{
+			input:       "unless-stopped:invalid",
+			expectedErr: "invalid restart policy format: maximum retry count must be an integer",
+		},
+
+		// Unknown / invalid combinations: validation is handled by the daemon>
+		{
+			input:    "anything:123",
+			expected: container.RestartPolicy{Name: "anything", MaximumRetryCount: 123},
+		},
+		{
+			input:    "negative:-123",
+			expected: container.RestartPolicy{Name: "negative", MaximumRetryCount: -123},
+		},
 	}
-	for restart, expectedError := range invalids {
+	for _, tc := range tests {
-		if _, _, _, err := parseRun([]string{"--restart=" + restart, "img", "cmd"}); err == nil || err.Error() != expectedError {
+		t.Run(tc.input, func(t *testing.T) {
-			t.Fatalf("Expected an error with message '%v' for %v, got %v", expectedError, restart, err)
+			_, hostConfig, _, err := parseRun([]string{"--restart=" + tc.input, "img", "cmd"})
-		}
+			if tc.expectedErr != "" {
-	}
+				assert.Check(t, is.Error(err, tc.expectedErr))
-	for restart, expected := range valids {
+				assert.Check(t, is.Nil(hostConfig))
-		_, hostconfig, _, err := parseRun([]string{fmt.Sprintf("--restart=%v", restart), "img", "cmd"})
+			} else {
-		if err != nil {
+				assert.NilError(t, err)
-			t.Fatal(err)
+				assert.Check(t, is.DeepEqual(hostConfig.RestartPolicy, tc.expected))
-		}
-		if hostconfig.RestartPolicy != expected {
-			t.Fatalf("Expected %v, got %v", expected, hostconfig.RestartPolicy)
 			}
+		})
 	}
 }
 
 func TestParseRestartPolicyAutoRemove(t *testing.T) {
 	_, _, _, err := parseRun([]string{"--rm", "--restart=always", "img", "cmd"}) //nolint:dogsled // verbatim copy from docker/cli tests
-	if err == nil {
+	const expected = "conflicting options: cannot specify both --restart and --rm"
-		t.Fatal("Expected error for conflicting --restart and --rm, but got none")
+	assert.Check(t, is.Error(err, expected))
-	}
 }
 
 func TestParseHealth(t *testing.T) {
@@ -841,8 +1003,8 @@ func TestParseHealth(t *testing.T) {
 	checkError("--no-healthcheck conflicts with --health-* options",
 		"--no-healthcheck", "--health-cmd=/check.sh -q", "img", "cmd")
 
-	health = checkOk("--health-timeout=2s", "--health-retries=3", "--health-interval=4.5s", "--health-start-period=5s", "img", "cmd")
+	health = checkOk("--health-timeout=2s", "--health-retries=3", "--health-interval=4.5s", "--health-start-period=5s", "--health-start-interval=1s", "img", "cmd")
-	if health.Timeout != 2*time.Second || health.Retries != 3 || health.Interval != 4500*time.Millisecond || health.StartPeriod != 5*time.Second {
+	if health.Timeout != 2*time.Second || health.Retries != 3 || health.Interval != 4500*time.Millisecond || health.StartPeriod != 5*time.Second || health.StartInterval != 1*time.Second {
 		t.Fatalf("--health-*: got %#v", health)
 	}
 }
@@ -863,13 +1025,13 @@ func TestParseLoggingOpts(t *testing.T) {
 }
 
 func TestParseEnvfileVariables(t *testing.T) { //nolint:dupl // verbatim copy from docker/cli tests
-	e := "open nonexistent: no such file or directory"
+	expErr := "--env-file: open nonexistent: no such file or directory"
 	if runtime.GOOS == "windows" {
-		e = "open nonexistent: The system cannot find the file specified."
+		expErr = "--env-file: open nonexistent: The system cannot find the file specified."
 	}
 	// env ko
-	if _, _, _, err := parseRun([]string{"--env-file=nonexistent", "img", "cmd"}); err == nil || err.Error() != e {
+	if _, _, _, err := parseRun([]string{"--env-file=nonexistent", "img", "cmd"}); err == nil || err.Error() != expErr {
-		t.Fatalf("Expected an error with message '%s', got %v", e, err)
+		t.Fatalf("Expected an error with message '%s', got %v", expErr, err)
 	}
 	// env ok
 	config, _, _, err := parseRun([]string{"--env-file=testdata/valid.env", "img", "cmd"})
@@ -905,7 +1067,7 @@ func TestParseEnvfileVariablesWithBOMUnicode(t *testing.T) {
 	}
 
 	// UTF16 with BOM
-	e := "invalid env file"
+	e := "invalid utf8 bytes at line"
 	if _, _, _, err := parseRun([]string{"--env-file=testdata/utf16.env", "img", "cmd"}); err == nil || !strings.Contains(err.Error(), e) {
 		t.Fatalf("Expected an error with message '%s', got %v", e, err)
 	}
@@ -916,13 +1078,13 @@ func TestParseEnvfileVariablesWithBOMUnicode(t *testing.T) {
 }
 
 func TestParseLabelfileVariables(t *testing.T) { //nolint:dupl // verbatim copy from docker/cli tests
-	e := "open nonexistent: no such file or directory"
+	expErr := "--label-file: open nonexistent: no such file or directory"
 	if runtime.GOOS == "windows" {
-		e = "open nonexistent: The system cannot find the file specified."
+		expErr = "--label-file: open nonexistent: The system cannot find the file specified."
 	}
 	// label ko
-	if _, _, _, err := parseRun([]string{"--label-file=nonexistent", "img", "cmd"}); err == nil || err.Error() != e {
+	if _, _, _, err := parseRun([]string{"--label-file=nonexistent", "img", "cmd"}); err == nil || err.Error() != expErr {
-		t.Fatalf("Expected an error with message '%s', got %v", e, err)
+		t.Fatalf("Expected an error with message '%s', got %v", expErr, err)
 	}
 	// label ok
 	config, _, _, err := parseRun([]string{"--label-file=testdata/valid.label", "img", "cmd"})
@@ -943,12 +1105,8 @@ func TestParseLabelfileVariables(t *testing.T) { //nolint:dupl // verbatim copy
 
 func TestParseEntryPoint(t *testing.T) {
 	config, _, _, err := parseRun([]string{"--entrypoint=anything", "cmd", "img"})
-	if err != nil {
+	assert.NilError(t, err)
-		t.Fatal(err)
+	assert.Check(t, is.DeepEqual(config.Entrypoint, []string{"anything"}))
-	}
-	if len(config.Entrypoint) != 1 && config.Entrypoint[0] != "anything" {
-		t.Fatalf("Expected entrypoint 'anything', got %v", config.Entrypoint)
-	}
 }
 
 func TestValidateDevice(t *testing.T) {
@@ -995,12 +1153,10 @@ func TestValidateDevice(t *testing.T) {
 	for path, expectedError := range invalid {
 		if _, err := validateDevice(path, runtime.GOOS); err == nil {
 			t.Fatalf("ValidateDevice(`%q`) should have failed validation", path)
-		} else {
+		} else if err.Error() != expectedError {
-			if err.Error() != expectedError {
 			t.Fatalf("ValidateDevice(`%q`) error should contain %q, got %q", path, expectedError, err.Error())
 		}
 	}
-	}
 }
 
 func TestValidateDeviceByServerOS(t *testing.T) {
@@ -1073,10 +1229,12 @@ func TestDeviceCgroupRulesAndInvalidParameter(t *testing.T) {
 	if invalidParameter(nil) != nil {
 		t.Fatal("invalidParameter(nil) should be nil")
 	}
-	err = invalidParameter(errors.New("bad input"))
+	cause := errors.New("bad input")
-	assert.Assert(t, err != nil)
+	err = invalidParameter(cause)
 	var invalid interface{ InvalidParameter() }
 	assert.Assert(t, errors.As(err, &invalid))
+	assert.Assert(t, errors.Is(err, cause))
+	assert.Equal(t, invalidParameter(err), err) // already invalid, so not wrapped twice
 }
 
 func TestParseSystemPaths(t *testing.T) {

act/container/docker_run.go

@@ -27,7 +27,6 @@ import (
 	"gitea.com/gitea/runner/act/filecollector"
 
 	"dario.cat/mergo"
-	"github.com/Masterminds/semver"
 	cerrdefs "github.com/containerd/errdefs"
 	"github.com/docker/cli/cli/compose/loader"
 	"github.com/docker/cli/cli/connhelper"
@@ -42,6 +41,7 @@ import (
 	"github.com/moby/moby/api/types/network"
 	"github.com/moby/moby/api/types/system"
 	"github.com/moby/moby/client"
+	"github.com/moby/moby/client/pkg/versions"
 	specs "github.com/opencontainers/image-spec/specs-go/v1"
 	"github.com/sirupsen/logrus"
 )
@@ -92,19 +92,11 @@ func (cr *containerReference) connectToNetwork(name string, aliases []string) co
 // supportsContainerImagePlatform returns true if the underlying Docker server
 // API version is 1.41 and beyond
 func supportsContainerImagePlatform(ctx context.Context, cli client.APIClient) bool {
-	logger := common.Logger(ctx)
 	ver, err := cli.ServerVersion(ctx, client.ServerVersionOptions{})
 	if err != nil {
-		logger.Panicf("Failed to get Docker API Version: %s", err)
+		common.Logger(ctx).Panicf("Failed to get Docker API Version: %s", err)
-		return false
 	}
-	sv, err := semver.NewVersion(ver.APIVersion)
+	return versions.GreaterThanOrEqualTo(ver.APIVersion, "1.41")
-	if err != nil {
-		logger.Panicf("Failed to unmarshal Docker Version: %s", err)
-		return false
-	}
-	constraint, _ := semver.NewConstraint(">= 1.41")
-	return constraint.Check(sv)
 }
 
 func (cr *containerReference) Create(capAdd, capDrop []string) common.Executor {
@@ -588,7 +580,7 @@ func (cr *containerReference) create(capAdd, capDrop []string) common.Executor {
 		}
 
 		var platSpecs *specs.Platform
-		if supportsContainerImagePlatform(ctx, cr.cli) && cr.input.Platform != "" {
+		if cr.input.Platform != "" && supportsContainerImagePlatform(ctx, cr.cli) {
 			platSpecs, err = parsePlatform(cr.input.Platform)
 			if err != nil {
 				return err

act/container/docker_stub.go

@@ -8,13 +8,13 @@ package container
 
 import (
 	"context"
+	"errors"
 	"runtime"
 	"time"
 
 	"gitea.com/gitea/runner/act/common"
 
 	"github.com/moby/moby/api/types/system"
-	"github.com/pkg/errors"
 )
 
 // ImageExistsLocally returns a boolean indicating if an image with the

act/exprparser/functions.go

@@ -28,8 +28,8 @@ func (impl *interperterImpl) contains(search, item reflect.Value) (bool, error)
 	switch search.Kind() {
 	case reflect.String, reflect.Int, reflect.Float64, reflect.Bool, reflect.Invalid:
 		return strings.Contains(
-			strings.ToLower(impl.coerceToString(search).String()),
+			strings.ToLower(CoerceToString(search)),
-			strings.ToLower(impl.coerceToString(item).String()),
+			strings.ToLower(CoerceToString(item)),
 		), nil
 
 	case reflect.Slice:
@@ -51,15 +51,15 @@ func (impl *interperterImpl) contains(search, item reflect.Value) (bool, error)
 
 func (impl *interperterImpl) startsWith(searchString, searchValue reflect.Value) (bool, error) { //nolint:unparam // pre-existing issue from nektos/act
 	return strings.HasPrefix(
-		strings.ToLower(impl.coerceToString(searchString).String()),
+		strings.ToLower(CoerceToString(searchString)),
-		strings.ToLower(impl.coerceToString(searchValue).String()),
+		strings.ToLower(CoerceToString(searchValue)),
 	), nil
 }
 
 func (impl *interperterImpl) endsWith(searchString, searchValue reflect.Value) (bool, error) { //nolint:unparam // pre-existing issue from nektos/act
 	return strings.HasSuffix(
-		strings.ToLower(impl.coerceToString(searchString).String()),
+		strings.ToLower(CoerceToString(searchString)),
-		strings.ToLower(impl.coerceToString(searchValue).String()),
+		strings.ToLower(CoerceToString(searchValue)),
 	), nil
 }
 
@@ -70,7 +70,7 @@ const (
 )
 
 func (impl *interperterImpl) format(str reflect.Value, replaceValue ...reflect.Value) (string, error) {
-	input := impl.coerceToString(str).String()
+	input := CoerceToString(str)
 	var output strings.Builder
 	replacementIndex := ""
 
@@ -108,7 +108,7 @@ func (impl *interperterImpl) format(str reflect.Value, replaceValue ...reflect.V
 					return "", fmt.Errorf("The following format string references more arguments than were supplied: '%s'", input)
 				}
 
-				output.WriteString(impl.coerceToString(replaceValue[index]).String())
+				output.WriteString(CoerceToString(replaceValue[index]))
 
 				state = passThrough
 
@@ -124,7 +124,7 @@ func (impl *interperterImpl) format(str reflect.Value, replaceValue ...reflect.V
 				state = passThrough
 
 			default:
-				panic("Invalid format parser state")
+				return "", fmt.Errorf("Closing bracket without opening one. The following format string is invalid: '%s'", input)
 			}
 		}
 	}
@@ -143,17 +143,17 @@ func (impl *interperterImpl) format(str reflect.Value, replaceValue ...reflect.V
 }
 
 func (impl *interperterImpl) join(array, sep reflect.Value) (string, error) { //nolint:unparam // pre-existing issue from nektos/act
-	separator := impl.coerceToString(sep).String()
+	separator := CoerceToString(sep)
 	switch array.Kind() {
 	case reflect.Slice:
 		var items []string
 		for i := 0; i < array.Len(); i++ {
-			items = append(items, impl.coerceToString(array.Index(i).Elem()).String())
+			items = append(items, CoerceToString(array.Index(i)))
 		}
 
 		return strings.Join(items, separator), nil
 	default:
-		return strings.Join([]string{impl.coerceToString(array).String()}, separator), nil
+		return strings.Join([]string{CoerceToString(array)}, separator), nil
 	}
 }
 

act/exprparser/functions_test.go

@@ -121,6 +121,7 @@ func TestFunctionJoin(t *testing.T) {
 		{"join(fromJSON('[\"a\", \"b\", null]'), null)", "ab", "join-number"},
 		{"join(fromJSON('[\"a\", \"b\"]'))", "a,b", "join-number"},
 		{"join(fromJSON('[\"a\", \"b\", null]'), 1)", "a1b1", "join-number"},
+		{"join(fromJSON('[1, true, null]'), '-')", "1-true-", "join-mixed-types"},
 	}
 
 	env := &EvaluationEnvironment{}
@@ -230,8 +231,10 @@ func TestFunctionFormat(t *testing.T) {
 		{`format('Hello "{0}" {1} {2} {3} {4}', null, true, -3.14, NaN, Infinity)`, `Hello "" true -3.14 NaN Infinity`, nil, "format-with-primitives"},
 		{`format('Hello "{0}" {1} {2}', fromJSON('[0, true, "abc"]'), fromJSON('[{"a":1}]'), fromJSON('{"a":{"b":1}}'))`, `Hello "Array" Array Object`, nil, "format-with-complex-types"},
 		{"format(true)", "true", nil, "format-with-primitive-args"},
+		{"format('{0}', github)", "Object", nil, "format-with-context"},
 		{"format('echo Hello {0} ${{Test}}', github.undefined_property)", "echo Hello  ${Test}", nil, "format-with-undefined-value"},
 		{"format('{0}}', '{1}', 'World')", nil, "Closing bracket without opening one. The following format string is invalid: '{0}}'", "format-invalid-format-string"},
+		{"format('a}b')", nil, "Closing bracket without opening one. The following format string is invalid: 'a}b'", "format-unmatched-closing-brace"},
 		{"format('{0', '{1}', 'World')", nil, "Unclosed brackets. The following format string is invalid: '{0'", "format-invalid-format-string"},
 		{"format('{2}', '{1}', 'World')", "", "The following format string references more arguments than were supplied: '{2}'", "format-invalid-replacement-reference"},
 		{"format('{2147483648}')", "", "The following format string is invalid: '{2147483648}'", "format-invalid-replacement-reference"},

act/exprparser/interpreter.go

@@ -10,6 +10,7 @@ import (
 	"fmt"
 	"math"
 	"reflect"
+	"strconv"
 	"strings"
 
 	"gitea.com/gitea/runner/act/model"
@@ -429,41 +430,54 @@ func (impl *interperterImpl) coerceToNumber(value reflect.Value) reflect.Value {
 	return reflect.ValueOf(math.NaN())
 }
 
-func (impl *interperterImpl) coerceToString(value reflect.Value) reflect.Value {
+// CoerceToString converts an evaluated expression value to a string the way GitHub does,
+// see https://docs.github.com/en/actions/reference/workflows-and-actions/expressions#operators
+// An already reflected value is accepted as-is, since Interface() would panic on an invalid one.
+func CoerceToString(v any) string {
+	value, ok := v.(reflect.Value)
+	if !ok {
+		value = reflect.ValueOf(v)
+	}
+
 	switch value.Kind() {
 	case reflect.Invalid:
-		return reflect.ValueOf("")
+		return ""
 
 	case reflect.Bool:
-		switch value.Bool() {
+		return strconv.FormatBool(value.Bool())
-		case true:
-			return reflect.ValueOf("true")
-		case false:
-			return reflect.ValueOf("false")
-		}
 
 	case reflect.String:
-		return value
+		return value.String()
 
-	case reflect.Int:
+	case reflect.Int, reflect.Int8, reflect.Int16, reflect.Int32, reflect.Int64:
-		return reflect.ValueOf(fmt.Sprint(value))
+		return strconv.FormatInt(value.Int(), 10)
 
-	case reflect.Float64:
+	case reflect.Uint, reflect.Uint8, reflect.Uint16, reflect.Uint32, reflect.Uint64:
+		return strconv.FormatUint(value.Uint(), 10)
+
+	case reflect.Float32, reflect.Float64:
 		if math.IsInf(value.Float(), 1) {
-			return reflect.ValueOf("Infinity")
+			return "Infinity"
 		} else if math.IsInf(value.Float(), -1) {
-			return reflect.ValueOf("-Infinity")
+			return "-Infinity"
 		}
-		return reflect.ValueOf(fmt.Sprintf("%.15G", value.Float()))
+		return fmt.Sprintf("%.15G", value.Float())
 
-	case reflect.Slice:
+	case reflect.Slice, reflect.Array:
-		return reflect.ValueOf("Array")
+		return "Array"
 
-	case reflect.Map:
+	// contexts such as `github` are pointers to structs, so they stringify as objects too
-		return reflect.ValueOf("Object")
+	case reflect.Map, reflect.Struct:
+		return "Object"
+
+	case reflect.Interface, reflect.Pointer:
+		if value.IsNil() {
+			return ""
+		}
+		return CoerceToString(value.Elem())
 	}
 
-	return value
+	return fmt.Sprintf("%v", value)
 }
 
 func (impl *interperterImpl) compareString(left, right string, kind actionlint.CompareOpNodeKind) (bool, error) {

act/exprparser/interpreter_test.go

@@ -6,6 +6,7 @@ package exprparser
 
 import (
 	"math"
+	"reflect"
 	"testing"
 
 	"gitea.com/gitea/runner/act/model"
@@ -633,3 +634,51 @@ func TestContexts(t *testing.T) {
 		})
 	}
 }
+
+func TestCoerceToString(t *testing.T) {
+	type object struct{ Name string }
+	obj := object{Name: "x"}
+	var nilPointer *object
+	var nilMap map[string]any
+	var nilSlice []any
+
+	table := []struct {
+		input    any
+		expected string
+		name     string
+	}{
+		{nil, "", "null"},
+		{true, "true", "true"},
+		{false, "false", "false"},
+		{"foo", "foo", "string"},
+		{"", "", "empty-string"},
+		{123, "123", "int"},
+		{int64(-9), "-9", "int64"},
+		{uint8(7), "7", "uint8"},
+		{1.0, "1", "float-integral"},
+		{-9.7, "-9.7", "float"},
+		{2.99e-2, "0.0299", "float-exponential"},
+		{1e21, "1E+21", "float-large"},
+		{float32(1.5), "1.5", "float32"},
+		{math.NaN(), "NaN", "nan"},
+		{math.Inf(1), "Infinity", "positive-infinity"},
+		{math.Inf(-1), "-Infinity", "negative-infinity"},
+		{[]any{1, 2}, "Array", "slice"},
+		{nilSlice, "Array", "nil-slice"},
+		{[2]int{1, 2}, "Array", "fixed-size-array"},
+		{map[string]any{"a": 1}, "Object", "map"},
+		{nilMap, "Object", "nil-map"},
+		{obj, "Object", "struct"},
+		{&obj, "Object", "pointer-to-struct"},
+		{nilPointer, "", "nil-pointer"},
+		{&model.GithubContext{Action: "push"}, "Object", "github-context"},
+		{reflect.ValueOf(42), "42", "reflected-value"},
+		{reflect.Value{}, "", "invalid-reflected-value"},
+	}
+
+	for _, tt := range table {
+		t.Run(tt.name, func(t *testing.T) {
+			assert.Equal(t, tt.expected, CoerceToString(tt.input))
+		})
+	}
+}

go.mod

@@ -6,7 +6,6 @@ require (
 	connectrpc.com/connect v1.20.0
 	dario.cat/mergo v1.0.2
 	gitea.dev/actions-proto-go v0.6.0
-	github.com/Masterminds/semver v1.5.0
 	github.com/avast/retry-go/v5 v5.0.0
 	github.com/containerd/errdefs v1.0.0
 	github.com/creack/pty v1.1.24
@@ -27,7 +26,6 @@ require (
 	github.com/moby/patternmatcher v0.6.1
 	github.com/opencontainers/image-spec v1.1.1
 	github.com/opencontainers/selinux v1.15.1
-	github.com/pkg/errors v0.9.1
 	github.com/prometheus/client_golang v1.24.0
 	github.com/prometheus/client_model v0.6.2
 	github.com/rhysd/actionlint v1.7.12

go.sum

@@ -8,8 +8,6 @@ gitea.dev/actions-proto-go v0.6.0 h1:gjllYQ5vmwlkqOeofTQu5qKTZpmf7kWsafoHvoPCSzY
 gitea.dev/actions-proto-go v0.6.0/go.mod h1:p4RX+D9oqiEEzzkPMXscw2CmaGuYFPWFc6xIOmDNDqs=
 github.com/AdaLogics/go-fuzz-headers v0.0.0-20240806141605-e8a1dd7889d6 h1:He8afgbRMd7mFxO99hRNu+6tazq8nFF9lIwo9JFroBk=
 github.com/AdaLogics/go-fuzz-headers v0.0.0-20240806141605-e8a1dd7889d6/go.mod h1:8o94RPi1/7XTJvwPpRSzSUedZrtlirdB3r9Z20bi2f8=
-github.com/Masterminds/semver v1.5.0 h1:H65muMkzWKEuNDnfl9d70GUjFniHKHRbFPGBuZ3QEww=
-github.com/Masterminds/semver v1.5.0/go.mod h1:MB6lktGJrhw8PrUyiEoblNEGEQ+RzHPF078ddwwvV3Y=
 github.com/Microsoft/go-winio v0.5.2/go.mod h1:WpS1mjBmmwHBEWmogvA2mj8546UReBk4v8QkMxJ6pZY=
 github.com/Microsoft/go-winio v0.6.2 h1:F2VQgta7ecxGYO8k3ZZz3RS8fVIXVxONVUPlNERoyfY=
 github.com/Microsoft/go-winio v0.6.2/go.mod h1:yd8OoFMLzJbo9gZq8j5qaps8bJ9aShtEA8Ipt1oGCvU=