fix: align runner behaviours (#1338)

Contains a number of small bug fixes where jobs behaved differently from GitHub or failed silently.

1. Hooks fail on a refused `::set-env::` or `::add-path::` and no longer inherit the last step's identity
1. Docker build, exec and wait errors are returned instead of swallowed, token-only registry credentials work, logs of `--tty` services decode
1. Env files split lines like GitHub on each OS, so a carriage return is kept on Linux and CRLF ends a line on Windows, empty heredoc names or delimiters are rejected
1. Post steps run after their pre step even when main was skipped, composites see undeclared inputs, `runs.args` wins without `with.args` being parsed
1. Docker actions skip evaluating defaults of inputs given in `with` and read `entryPoint` and `args` in any case, a lowercase `dockerfile` is found, undeclared inputs get GitHub's warning
1. A user-set `CI` is kept, `GITHUB_REPOSITORY_ID` and the other missing `GITHUB_*` variables are exported
1. `timeout-minutes` can read the step's own `env`, a step-level `ACTIONS_ALLOW_UNSECURE_COMMANDS: false` beats a job-level `true` while the runner's own opt-in still wins, `pwsh` quotes the script path
1. A step `env:` given as one expression such as `${{ fromJSON(matrix.env) }}` is applied, `hashFiles` fails with GitHub's messages on script errors and after 120 seconds, a job image without node still gets an empty hash
1. A step whose command files cannot be reset fails instead of importing stale ones, a timeout between steps fails the job instead of reporting success
1. One named volume can be mounted at several paths, host-mode PTY steps keep their last output when they fail, host scratch directories are removed when job setup fails
1. Cancelling a step right after it starts kills its whole process tree
1. Cache prefix restores pick the newest entry created within the same second, also on S3, cache commits with a wrong size are rejected
1. Artifact chunks are written at their offset, download URLs are escaped, listing a run without artifacts no longer crashes
1. Poll backoff reaches `fetch_interval_max`, proxy passwords are masked in more forms, secrets are also masked in step names and in their quoted form in the runner log, a failed final report is logged
1. On Windows hosts a differently cased unsecure-commands opt-in or proxy variable counts, and an early cancel no longer leaks a job object handle
1. Interactive `register` prompts for a missing token and stops on SIGTERM, `exec --list --detect-event` no longer panics without events, `config set` works on `cache.s3` keys

Reviewed-on: https://gitea.com/gitea/runner/pulls/1338
Reviewed-by: bircni <[email protected]>
Co-authored-by: silverwind <[email protected]>

act/container/docker_build.go

@@ -74,8 +74,10 @@ func NewDockerBuildExecutor(input NewDockerBuildExecutorInput) common.Executor {
 
 		logger.Debugf("Creating image from context dir '%s' with tag '%s' and platform '%s'", input.ContextDir, input.ImageTag, input.Platform)
 		resp, err := cli.ImageBuild(ctx, buildContext, options)
-
-		return logDockerResponse(logger, resp.Body, err != nil)
+		if err != nil {
+			return err
+		}
+		return logDockerResponse(logger, resp.Body, false)
 	}
 }
 

act/container/docker_build_test.go

@@ -8,13 +8,35 @@ package container
 import (
 	"archive/tar"
 	"io"
+	"net/http"
+	"net/http/httptest"
 	"os"
 	"path/filepath"
+	"strings"
 	"testing"
 
 	"github.com/stretchr/testify/require"
 )
 
+func TestDockerBuildReportsDaemonError(t *testing.T) {
+	daemon := httptest.NewServer(http.HandlerFunc(func(writer http.ResponseWriter, request *http.Request) {
+		writer.Header().Set("API-Version", "1.47")
+		if strings.HasSuffix(request.URL.Path, "/build") {
+			writer.Header().Set("Content-Type", "application/json")
+			writer.WriteHeader(http.StatusInternalServerError)
+			_, _ = io.WriteString(writer, `{"message":"build rejected"}`)
+		}
+	}))
+	t.Cleanup(daemon.Close)
+	t.Setenv("DOCKER_HOST", daemon.URL)
+	t.Setenv("DOCKER_CONFIG", t.TempDir())
+	t.Setenv("DOCKER_AUTH_CONFIG", "")
+
+	require.ErrorContains(t, NewDockerBuildExecutor(NewDockerBuildExecutorInput{
+		ImageTag: "action:test", Dockerfile: "Dockerfile", BuildContext: strings.NewReader(""),
+	})(t.Context()), "build rejected")
+}
+
 func TestCreateBuildContextFollowsDockerignoreAndSymlinkedRootButKeepsDockerfile(t *testing.T) {
 	dir := t.TempDir()
 	for name, content := range map[string]string{

act/container/docker_logger.go

@@ -33,6 +33,7 @@ func logDockerResponse(logger *logging.Logger, dockerResponse io.ReadCloser, isE
 	defer dockerResponse.Close()
 
 	scanner := bufio.NewScanner(dockerResponse)
+	scanner.Buffer(nil, 64*1024*1024) // a build echoes long instructions in one record
 	for scanner.Scan() {
 		line := scanner.Bytes()
 		var msg dockerMessage
@@ -65,7 +66,7 @@ func logDockerResponse(logger *logging.Logger, dockerResponse io.ReadCloser, isE
 		}
 	}
 
-	return nil
+	return scanner.Err()
 }
 
 func writeLog(logger *logging.Logger, isError bool, format string, args ...any) {

act/container/docker_pull.go

@@ -114,7 +114,7 @@ func getImagePullOptions(ctx context.Context, input NewDockerPullExecutorInput)
 		if err != nil {
 			return imagePullOptions, err
 		}
-		if authConfig.Username == "" && authConfig.Password == "" {
+		if authConfig.Username == "" && authConfig.Password == "" && authConfig.IdentityToken == "" && authConfig.RegistryToken == "" {
 			return imagePullOptions, nil
 		}
 		logger.Info("using DockerAuthConfig authentication for docker pull")

act/container/docker_pull_test.go

@@ -6,12 +6,17 @@ package container
 
 import (
 	"io"
+	"os"
+	"path/filepath"
 	"strings"
 	"testing"
+	"testing/iotest"
 
 	"gitea.com/gitea/runner/act/common/logging"
 	"gitea.com/gitea/runner/act/common/logging/logtest"
 
+	"github.com/moby/moby/api/pkg/authconfig"
+	"github.com/moby/moby/api/types/registry"
 	assert "github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 )
@@ -64,6 +69,21 @@ func TestGetImagePullOptions(t *testing.T) {
 	})
 	assert.NoError(t, err, "Failed to create ImagePullOptions") //nolint:testifylint // pre-existing issue from nektos/act
 	assert.Equal(t, "eyJ1c2VybmFtZSI6InVzZXJuYW1lIiwicGFzc3dvcmQiOiJwYXNzd29yZFxuIiwic2VydmVyYWRkcmVzcyI6Imh0dHBzOi8vaW5kZXguZG9ja2VyLmlvL3YxLyJ9", options.RegistryAuth, "RegistryAuth should be taken from local docker config")
+
+	dir := t.TempDir()
+	require.NoError(t, os.WriteFile(filepath.Join(dir, "config.json"), []byte(`{"auths":{"identity.example.com":{"identitytoken":"identity-token"},"registry.example.com":{"registrytoken":"registry-token"}}}`), 0o600))
+	t.Setenv("DOCKER_CONFIG", dir)
+	for host, want := range map[string]registry.AuthConfig{
+		"identity.example.com": {IdentityToken: "identity-token", ServerAddress: "identity.example.com"},
+		"registry.example.com": {RegistryToken: "registry-token", ServerAddress: "registry.example.com"},
+	} {
+		options, err := getImagePullOptions(ctx, NewDockerPullExecutorInput{Image: host + "/app"})
+		require.NoError(t, err)
+		require.NotEmpty(t, options.RegistryAuth, host)
+		got, err := authconfig.Decode(options.RegistryAuth)
+		require.NoError(t, err)
+		assert.Equal(t, want, *got)
+	}
 }
 
 // A digest-pinned image is immutable, so its local copy is always current.
@@ -82,4 +102,8 @@ func TestLogDockerResponseError(t *testing.T) {
 	require.EqualError(t, streamErr(`{"error":"toomanyrequests: rate limit exceeded"}`), "toomanyrequests: rate limit exceeded")
 	require.EqualError(t, streamErr(`{"errorDetail":{"message":"unexpected EOF"}}`), "unexpected EOF")
 	require.NoError(t, streamErr(`{"status":"Downloading"}`))
+	require.NoError(t, streamErr(`{"stream":"`+strings.Repeat("x", 70000)+`"}`))
+	require.ErrorIs(t, logDockerResponse(logger, io.NopCloser(io.MultiReader(
+		strings.NewReader(`{"status":"Downloading"}`+"\n"), iotest.ErrReader(io.ErrUnexpectedEOF),
+	)), false), io.ErrUnexpectedEOF)
 }

act/container/docker_run.go

@@ -205,18 +205,30 @@ func (cr *containerReference) DumpLogs(ctx context.Context) error {
 	if cr.id == "" {
 		return cr.missingContainerError("read its logs")
 	}
+	tty, err := cr.containerTTY(ctx)
+	if err != nil {
+		return err
+	}
 
 	logs, err := cr.cli.ContainerLogs(ctx, cr.id, client.ContainerLogsOptions{ShowStdout: true, ShowStderr: true})
 	if err != nil {
 		return err
 	}
 	defer logs.Close()
-	return cr.copyOutput(logs)
+	return cr.copyOutput(logs, tty)
+}
+
+func (cr *containerReference) containerTTY(ctx context.Context) (bool, error) {
+	inspect, err := cr.cli.ContainerInspect(ctx, cr.id, client.ContainerInspectOptions{})
+	if err != nil {
+		return false, fmt.Errorf("failed to inspect container: %w", err)
+	}
+	return inspect.Container.Config != nil && inspect.Container.Config.Tty, nil
 }
 
 // copyOutput writes a container stream to the writers the container was created with,
 // demultiplexing it unless the container has a TTY, which sends a single raw stream.
-func (cr *containerReference) copyOutput(stream io.Reader) error {
+func (cr *containerReference) copyOutput(stream io.Reader, tty bool) error {
 	outWriter := cr.input.Stdout
 	if outWriter == nil {
 		outWriter = os.Stdout
@@ -227,7 +239,7 @@ func (cr *containerReference) copyOutput(stream io.Reader) error {
 	}
 
 	var err error
-	if !cr.input.AllocatePTY || os.Getenv("NORAW") != "" {
+	if !tty || os.Getenv("NORAW") != "" {
 		_, err = stdcopy.StdCopy(outWriter, errWriter, stream)
 	} else {
 		_, err = io.Copy(outWriter, stream)
@@ -296,7 +308,7 @@ func containerInfoFromInspect(inspect container.InspectResponse) *Info {
 }
 
 func (cr *containerReference) UpdateFromEnv(srcPath string, env *map[string]string) common.Executor {
-	return parseEnvFile(cr, srcPath, env).IfNot(common.Dryrun)
+	return parseEnvFile(cr, srcPath, env, false).IfNot(common.Dryrun)
 }
 
 func (cr *containerReference) UpdateFromImageEnv(env *map[string]string) common.Executor {
@@ -499,7 +511,7 @@ func (cr *containerReference) remove() common.Executor {
 		if idOrName == "" && cr.input != nil {
 			idOrName = cr.input.Name
 		}
-		if idOrName == "" {
+		if idOrName == "" || cr.cli == nil {
 			return nil
 		}
 
@@ -851,7 +863,7 @@ func (cr *containerReference) waitForCommand(ctx context.Context, resp client.Hi
 	cmdResponse := make(chan error, 1)
 
 	go func() {
-		cmdResponse <- cr.copyOutput(resp.Reader)
+		cmdResponse <- cr.copyOutput(resp.Reader, cr.input.AllocatePTY)
 	}()
 
 	select {
@@ -876,11 +888,7 @@ func (cr *containerReference) waitForCommand(ctx context.Context, resp client.Hi
 		// from executing
 		return ctx.Err()
 	case err := <-cmdResponse:
-		if err != nil {
-			logger.Error(err)
-		}
-
-		return nil
+		return err
 	}
 }
 
@@ -897,7 +905,7 @@ func (cr *containerReference) copyDir(dstPath, srcPath string, useGitIgnore, ski
 		}
 		logger.Debugf("Writing tarball %s from %s", tarFile.Name(), srcPath)
 		defer func() {
-			if err := tarFile.Close(); !errors.Is(err, os.ErrClosed) {
+			if err := tarFile.Close(); err != nil && !errors.Is(err, os.ErrClosed) {
 				logger.Error(err)
 			}
 			if err := os.Remove(tarFile.Name()); err != nil {
@@ -944,6 +952,10 @@ func (cr *containerReference) upload(ctx context.Context, dstPath string, conten
 
 func (cr *containerReference) attach() common.Executor {
 	return func(ctx context.Context) error {
+		tty, err := cr.containerTTY(ctx)
+		if err != nil {
+			return err
+		}
 		out, err := cr.cli.ContainerAttach(ctx, cr.id, client.ContainerAttachOptions{
 			Stream: true,
 			Stdout: true,
@@ -954,9 +966,12 @@ func (cr *containerReference) attach() common.Executor {
 		}
 		done := make(chan struct{})
 		cr.attachDone = done
+		stopClose := context.AfterFunc(ctx, out.Close)
 		go func() {
 			defer close(done)
-			if copyErr := cr.copyOutput(out.Reader); copyErr != nil {
+			defer out.Close()
+			defer stopClose()
+			if copyErr := cr.copyOutput(out.Reader, tty); copyErr != nil && ctx.Err() == nil {
 				common.Logger(ctx).Error(copyErr)
 			}
 		}()
@@ -991,6 +1006,9 @@ func (cr *containerReference) wait() common.Executor {
 				return fmt.Errorf("failed to wait for container: %w", err)
 			}
 		case status := <-waitResult.Result:
+			if status.Error != nil {
+				return fmt.Errorf("failed to wait for container: %s", status.Error.Message)
+			}
 			statusCode = status.StatusCode
 		}
 

act/container/docker_run_test.go

@@ -92,6 +92,16 @@ func (m *mockDockerClient) ContainerAttach(ctx context.Context, containerID stri
 	return args.Get(0).(mobyclient.ContainerAttachResult), args.Error(1)
 }
 
+func (m *mockDockerClient) ContainerLogs(ctx context.Context, containerID string, opts mobyclient.ContainerLogsOptions) (mobyclient.ContainerLogsResult, error) {
+	args := m.Called(ctx, containerID, opts)
+	return args.Get(0).(mobyclient.ContainerLogsResult), args.Error(1)
+}
+
+func (m *mockDockerClient) ContainerStart(ctx context.Context, containerID string, opts mobyclient.ContainerStartOptions) (mobyclient.ContainerStartResult, error) {
+	args := m.Called(ctx, containerID, opts)
+	return args.Get(0).(mobyclient.ContainerStartResult), args.Error(1)
+}
+
 func (m *mockDockerClient) ContainerWait(ctx context.Context, containerID string, opts mobyclient.ContainerWaitOptions) mobyclient.ContainerWaitResult {
 	args := m.Called(ctx, containerID, opts)
 	return args.Get(0).(mobyclient.ContainerWaitResult)
@@ -158,6 +168,7 @@ func (r *interruptReader) Read(_ []byte) (int, error) {
 type mockConn struct {
 	net.Conn
 	mock.Mock
+	closed atomic.Bool
 }
 
 func (m *mockConn) Write(b []byte) (n int, err error) {
@@ -166,6 +177,7 @@ func (m *mockConn) Write(b []byte) (n int, err error) {
 }
 
 func (m *mockConn) Close() (err error) {
+	m.closed.Store(true)
 	return nil
 }
 
@@ -236,6 +248,21 @@ func TestDockerExecFailure(t *testing.T) {
 	client.AssertExpectations(t)
 }
 
+func TestDockerExecStreamFailure(t *testing.T) {
+	ctx := t.Context()
+	client := &mockDockerClient{}
+	client.On("ExecCreate", ctx, "123", mock.AnythingOfType("client.ExecCreateOptions")).
+		Return(mobyclient.ExecCreateResult{ID: "id"}, nil)
+	client.On("ExecAttach", ctx, "id", mock.AnythingOfType("client.ExecAttachOptions")).
+		Return(mobyclient.ExecAttachResult{
+			Conn:   &mockConn{},
+			Reader: bufio.NewReader(bytes.NewReader(stdcopyFrame(stdcopy.Systemerr, "unable to start exec"))),
+		}, nil)
+	require.ErrorContains(t, (&containerReference{id: "123", cli: client, input: &NewContainerInput{}}).
+		exec([]string{"sh"}, nil, "", "")(ctx), "unable to start exec")
+	client.AssertExpectations(t)
+}
+
 // stdcopyFrame wraps payload in a single Docker multiplexed-stream frame, the
 // format StdCopy expects: an 8-byte header (stream type + 4-byte big-endian
 // length) followed by the payload.
@@ -248,60 +275,88 @@ func stdcopyFrame(stream stdcopy.StdType, payload string) []byte {
 }
 
 func TestDockerAttachFlushesTrailingLine(t *testing.T) {
-	ctx := t.Context()
+	for _, testcase := range []struct {
+		name string
+		tty  bool
+	}{
+		{name: "multiplexed"},
+		{name: "tty from container options", tty: true},
+	} {
+		t.Run(testcase.name, func(t *testing.T) {
+			ctx := t.Context()
+			output := []byte("line one\nlast line without newline")
+			if !testcase.tty {
+				output = stdcopyFrame(stdcopy.Stdout, string(output))
+			}
+			var lines []string
+			logWriter := common.NewLineWriter(func(line string) bool {
+				lines = append(lines, line)
+				return true
+			})
+			client := &mockDockerClient{}
+			client.On("ContainerInspect", ctx, "123", mobyclient.ContainerInspectOptions{}).
+				Return(mobyclient.ContainerInspectResult{Container: container.InspectResponse{Config: &container.Config{Tty: testcase.tty}}}, nil)
+			conn := &mockConn{}
+			client.On("ContainerAttach", ctx, "123", mock.AnythingOfType("client.ContainerAttachOptions")).
+				Return(mobyclient.ContainerAttachResult{
+					Conn:   conn,
+					Reader: bufio.NewReader(bytes.NewReader(output)),
+				}, nil)
+			client.On("ContainerLogs", ctx, "123", mobyclient.ContainerLogsOptions{ShowStdout: true, ShowStderr: true}).
+				Return(io.NopCloser(bytes.NewReader(output)), nil)
+			statusCh := make(chan container.WaitResponse, 1)
+			statusCh <- container.WaitResponse{StatusCode: 0}
+			client.On("ContainerWait", ctx, "123", mobyclient.ContainerWaitOptions{Condition: container.WaitConditionNotRunning}).
+				Return(mobyclient.ContainerWaitResult{Result: statusCh})
+			cr := &containerReference{id: "123", cli: client, input: &NewContainerInput{Stdout: logWriter, Stderr: logWriter}}
 
-	framed := bytes.NewBuffer(stdcopyFrame(stdcopy.Stdout, "line one\nlast line without newline"))
-
-	var lines []string
-	logWriter := common.NewLineWriter(func(s string) bool {
-		lines = append(lines, s)
-		return true
-	})
-
-	client := &mockDockerClient{}
-	client.On("ContainerAttach", ctx, "123", mock.AnythingOfType("client.ContainerAttachOptions")).
-		Return(mobyclient.ContainerAttachResult{
-			Conn:   &mockConn{},
-			Reader: bufio.NewReader(framed),
-		}, nil)
-
-	statusCh := make(chan container.WaitResponse, 1)
-	statusCh <- container.WaitResponse{StatusCode: 0}
-	client.On("ContainerWait", ctx, "123", mobyclient.ContainerWaitOptions{Condition: container.WaitConditionNotRunning}).
-		Return(mobyclient.ContainerWaitResult{Result: statusCh})
-
-	cr := &containerReference{
-		id:  "123",
-		cli: client,
-		input: &NewContainerInput{
-			Stdout: logWriter,
-			Stderr: logWriter,
-		},
+			require.NoError(t, cr.attach()(ctx))
+			require.NoError(t, cr.wait()(ctx))
+			assert.Equal(t, []string{"line one\n", "last line without newline"}, lines)
+			assert.True(t, conn.closed.Load())
+			require.NoError(t, cr.DumpLogs(ctx))
+			assert.Equal(t, []string{"line one\n", "last line without newline", "line one\n", "last line without newline"}, lines)
+			client.AssertExpectations(t)
+		})
 	}
+}
 
-	require.NoError(t, cr.attach()(ctx))
-	require.NoError(t, cr.wait()(ctx))
-
-	assert.Equal(t, []string{"line one\n", "last line without newline"}, lines)
+func TestDockerAttachStreamClosesWithItsContext(t *testing.T) {
+	conn, peer := net.Pipe()
+	t.Cleanup(func() { _ = conn.Close() })
+	t.Cleanup(func() { _ = peer.Close() })
+	client := &mockDockerClient{}
+	client.On("ContainerInspect", mock.Anything, "123", mobyclient.ContainerInspectOptions{}).
+		Return(mobyclient.ContainerInspectResult{Container: container.InspectResponse{Config: &container.Config{}}}, nil)
+	client.On("ContainerAttach", mock.Anything, "123", mock.AnythingOfType("client.ContainerAttachOptions")).
+		Return(mobyclient.ContainerAttachResult{HijackedResponse: mobyclient.NewHijackedResponse(conn, "")}, nil)
+	client.On("ContainerStart", mock.Anything, "123", mobyclient.ContainerStartOptions{}).
+		Return(mobyclient.ContainerStartResult{}, errors.New("container start failed"))
+	cr := &containerReference{id: "123", cli: client, input: &NewContainerInput{}}
 
+	ctx, cancel := context.WithCancel(t.Context())
+	require.ErrorContains(t, cr.Start(true)(ctx), "container start failed")
+	cancel()
+	<-cr.attachDone
 	client.AssertExpectations(t)
 }
 
 func TestDockerWaitFailure(t *testing.T) {
 	ctx := t.Context()
-
 	statusCh := make(chan container.WaitResponse, 1)
 	statusCh <- container.WaitResponse{StatusCode: 2}
 	client := &mockDockerClient{}
 	client.On("ContainerWait", ctx, "123", mobyclient.ContainerWaitOptions{Condition: container.WaitConditionNotRunning}).
 		Return(mobyclient.ContainerWaitResult{Result: statusCh})
+	cr := &containerReference{id: "123", cli: client}
 
-	err := (&containerReference{id: "123", cli: client}).wait()(ctx)
+	err := cr.wait()(ctx)
 	var exitErr ExitCodeError
 	require.ErrorAs(t, err, &exitErr)
 	assert.Equal(t, ExitCodeError(2), exitErr)
 	assert.Equal(t, "Process completed with exit code 2.", err.Error())
-
+	statusCh <- container.WaitResponse{Error: &container.WaitExitError{Message: "container wait failed"}}
+	require.EqualError(t, cr.wait()(ctx), "failed to wait for container: container wait failed")
 	client.AssertExpectations(t)
 }
 
@@ -370,6 +425,13 @@ func TestRemoveWithoutIDUsesName(t *testing.T) {
 	client.AssertExpectations(t)
 }
 
+func TestRemovePreservesClientCreationError(t *testing.T) {
+	t.Setenv("DOCKER_HOST", "://")
+
+	require.ErrorContains(t, (&containerReference{input: &NewContainerInput{Name: "job-1"}}).
+		Remove()(t.Context()), "failed to connect to docker daemon")
+}
+
 func TestFindRevalidatesStaleID(t *testing.T) {
 	ctx := t.Context()
 	inspectOpts := mobyclient.ContainerInspectOptions{}
@@ -477,7 +539,8 @@ func TestPublicCopyPipelineHandlesStaleID(t *testing.T) {
 		{name: "sibling prefix", directory: true, destination: "/var/run/actor/action", extractionRoot: "/", entry: "var/run/actor/action/x"},
 	} {
 		t.Run(testcase.name, func(t *testing.T) {
-			ctx := t.Context()
+			hook := logtest.NewWithLevel(logging.LevelError)
+			ctx := common.WithLogger(t.Context(), logging.New(hook))
 			client := &mockDockerClient{}
 			client.On("ContainerInspect", ctx, "stale", mobyclient.ContainerInspectOptions{}).
 				Return(mobyclient.ContainerInspectResult{}, cerrdefs.ErrNotFound.WithMessage("gone"))
@@ -501,6 +564,7 @@ func TestPublicCopyPipelineHandlesStaleID(t *testing.T) {
 				require.NoError(t, cr.Copy(testcase.destination, &FileEntry{Name: "x", Mode: 0o644})(ctx))
 			}
 			assert.Equal(t, "fresh", cr.id)
+			assert.Empty(t, hook.AllRecords())
 			client.AssertExpectations(t)
 		})
 	}

act/container/host_environment.go

@@ -318,21 +318,21 @@ func (e *HostEnvironment) exec(ctx context.Context, command []string, cmdline st
 		defer k.Close()
 	}
 	err = treeKill.Wait(ctx, cmd)
-	if err != nil {
-		if exitErr, ok := errors.AsType[*exec.ExitError](err); ok {
-			return ExitCodeError(exitErr.ExitCode())
-		}
-		return err
-	}
 	if tty != nil {
 		writer.AutoStop.Store(true)
 		if _, err := tty.WriteString("\x04"); err != nil {
 			common.Logger(ctx).Debug("Failed to write EOT")
 		}
-		<-logctx.Done()
+		select {
+		case <-logctx.Done():
+		case <-ctx.Done(): // a cancelled step does not wait on output nobody consumes
+		}
 		ppty.Close()
 		ppty = nil
 	}
+	if exitErr, ok := errors.AsType[*exec.ExitError](err); ok {
+		return ExitCodeError(exitErr.ExitCode())
+	}
 	return err
 }
 
@@ -355,7 +355,7 @@ func (e *HostEnvironment) ExecWithCmdLine(command []string, cmdline string, env
 }
 
 func (e *HostEnvironment) UpdateFromEnv(srcPath string, env *map[string]string) common.Executor {
-	return parseEnvFile(e, srcPath, env)
+	return parseEnvFile(e, srcPath, env, runtime.GOOS == "windows")
 }
 
 // removeAll is a var so tests can substitute a blocking stub.

act/container/host_environment_test.go

@@ -82,14 +82,36 @@ func TestHostEnvironmentExecExitCode(t *testing.T) {
 	if runtime.GOOS == "windows" {
 		t.Skip("uses POSIX shell")
 	}
-	ctx := t.Context()
-	e := &HostEnvironment{Path: t.TempDir(), StdOut: io.Discard}
+	for name, allocatePTY := range map[string]bool{"without PTY": false, "with PTY": true} {
+		t.Run(name, func(t *testing.T) {
+			reader, writer := io.Pipe()
+			defer reader.Close()
+			e := &HostEnvironment{Path: t.TempDir(), StdOut: writer, AllocatePTY: allocatePTY}
+			t.Cleanup(func() { assert.NoError(t, e.Remove()(t.Context())) })
 
-	err := e.Exec([]string{"sh", "-c", "exit 3"}, map[string]string{"PATH": os.Getenv("PATH")}, "", "")(ctx)
-	var exitErr ExitCodeError
-	require.ErrorAs(t, err, &exitErr)
-	assert.Equal(t, ExitCodeError(3), exitErr)
-	assert.Equal(t, "Process completed with exit code 3.", err.Error())
+			result := make(chan error, 1)
+			go func() {
+				defer writer.Close()
+				result <- e.Exec([]string{"sh", "-c", "printf 'failed\\n'; exit 3"}, map[string]string{"PATH": os.Getenv("PATH")}, "", "")(t.Context())
+			}()
+			first := make([]byte, 1)
+			_, err := io.ReadFull(reader, first)
+			require.NoError(t, err)
+			select {
+			case err := <-result:
+				t.Fatalf("Exec returned before its output was consumed: %v", err)
+			case <-time.After(100 * time.Millisecond):
+			}
+			remaining, err := io.ReadAll(reader)
+			require.NoError(t, err)
+			err = <-result
+			var exitErr ExitCodeError
+			require.ErrorAs(t, err, &exitErr)
+			assert.Equal(t, ExitCodeError(3), exitErr)
+			assert.Equal(t, "Process completed with exit code 3.", err.Error())
+			assert.Equal(t, "failed\n", string(first)+string(remaining))
+		})
+	}
 }
 
 func TestHostEnvironmentAllocatePTY(t *testing.T) {

act/container/kubernetes.go

@@ -500,7 +500,7 @@ func (c *kubernetesContainer) GetContainerArchive(ctx context.Context, srcPath s
 }
 
 func (c *kubernetesContainer) UpdateFromEnv(srcPath string, env *map[string]string) common.Executor {
-	return parseEnvFile(c, srcPath, env)
+	return parseEnvFile(c, srcPath, env, false)
 }
 
 func (c *kubernetesContainer) UpdateFromImageEnv(env *map[string]string) common.Executor {

act/container/parse_env_file.go

@@ -18,7 +18,8 @@ import (
 	"golang.org/x/text/transform"
 )
 
-func parseEnvFile(e Container, srcPath string, env *map[string]string) common.Executor {
+// parseEnvFile reads a GITHUB_ENV style file, splitting lines like GitHub's runner on Windows or elsewhere.
+func parseEnvFile(e Container, srcPath string, env *map[string]string, windows bool) common.Executor {
 	localEnv := *env
 	return func(ctx context.Context) error {
 		envTar, err := e.GetContainerArchive(ctx, srcPath)
@@ -38,8 +39,15 @@ func parseEnvFile(e Container, srcPath string, env *map[string]string) common.Ex
 		s := bufio.NewScanner(decoded)
 		// Default 64 KiB max token size is too small for realistic env-file lines; allow up to 16 MiB.
 		s.Buffer(make([]byte, 0, 64*1024), 16*1024*1024)
+		s.Split(func(data []byte, atEOF bool) (int, []byte, error) {
+			advance, token, err := bufio.ScanLines(data, atEOF)
+			if token != nil {
+				return advance, data[:advance], err
+			}
+			return advance, token, err
+		})
 		for s.Scan() {
-			line := s.Text()
+			line := trimEnvLineEnding(s.Text(), windows)
 			// GitHub's runner ignores blank lines
 			if strings.TrimSpace(line) == "" {
 				continue
@@ -52,10 +60,13 @@ func parseEnvFile(e Container, srcPath string, env *map[string]string) common.Ex
 			case multiLineEnv != -1:
 				var multiLineEnvContent []string
 				multiLineEnvDelimiter := line[multiLineEnv+2:]
+				if multiLineEnv == 0 || multiLineEnvDelimiter == "" {
+					return fmt.Errorf("invalid format '%v', name and delimiter must not be empty", line)
+				}
 				delimiterFound := false
 				for s.Scan() {
 					content := s.Text()
-					if content == multiLineEnvDelimiter {
+					if trimEnvLineEnding(content, windows) == multiLineEnvDelimiter {
 						delimiterFound = true
 						break
 					}
@@ -67,7 +78,7 @@ func parseEnvFile(e Container, srcPath string, env *map[string]string) common.Ex
 				if !delimiterFound {
 					return fmt.Errorf("invalid format delimiter '%v' not found before end of file", multiLineEnvDelimiter)
 				}
-				localEnv[line[:multiLineEnv]] = strings.Join(multiLineEnvContent, "\n")
+				localEnv[line[:multiLineEnv]] = trimEnvLineEnding(strings.Join(multiLineEnvContent, ""), windows)
 			default:
 				return fmt.Errorf("invalid format '%v', expected a line with '=' or '<<'", line)
 			}
@@ -78,3 +89,10 @@ func parseEnvFile(e Container, srcPath string, env *map[string]string) common.Ex
 		return nil
 	}
 }
+
+func trimEnvLineEnding(line string, windows bool) string {
+	if windows && strings.HasSuffix(line, "\r\n") {
+		return line[:len(line)-2]
+	}
+	return strings.TrimSuffix(line, "\n")
+}

act/container/parse_env_file_test.go

@@ -33,23 +33,36 @@ func TestParseEnvFile(t *testing.T) {
 		want          map[string]string
 	}{
 		{"single line", "FOO=bar\nBAZ=qux\n", map[string]string{"FOO": "bar", "BAZ": "qux"}},
+		{"single line CRLF and final bare CR", "FOO=bar\r\nBAZ=qux\r", map[string]string{"FOO": "bar\r", "BAZ": "qux\r"}},
+		{"single line without final newline", "FOO=bar", map[string]string{"FOO": "bar"}},
 		{"blank lines", "\nFOO=bar\n\n   \nBAZ=qux\n\n", map[string]string{"FOO": "bar", "BAZ": "qux"}},
 		{"multiline", "FOO<<EOF\nline1\nline2\nEOF\n", map[string]string{"FOO": "line1\nline2"}},
+		{"multiline without final newline", "FOO<<EOF\nline1\r\nline2\r\nEOF", map[string]string{"FOO": "line1\r\nline2\r"}},
+		{"empty multiline without final newline", "FOO<<EOF\nEOF", map[string]string{"FOO": ""}},
 		{"multiline blank lines", "FOO<<EOF\nline1\n\nline2\nEOF\n", map[string]string{"FOO": "line1\n\nline2"}},
 		{"multiline leading blank line", "FOO<<EOF\n\nline2\nEOF\n", map[string]string{"FOO": "\nline2"}},
+		{"multiline CRLF", "FOO<<EOF\r\nline1\r\nline2\r\nEOF\r\n", map[string]string{"FOO": "line1\r\nline2\r"}},
+		{"multiline mixed line endings", "FOO<<EOF\nline1\r\nline2\n\r\nEOF\n", map[string]string{"FOO": "line1\r\nline2\n\r"}},
 		{"large value within limit", "FOO<<EOF\n" + largeValue + "\nEOF\n", map[string]string{"FOO": largeValue}},
 		{"UTF8 BOM", "\xef\xbb\xbfFOO=bar\n", map[string]string{"FOO": "bar"}},
-		{"UTF16 little endian", encodeUTF16(unicode.LittleEndian), map[string]string{"FOO": "bar", "MULTI": "line1"}},
-		{"UTF16 big endian", encodeUTF16(unicode.BigEndian), map[string]string{"FOO": "bar", "MULTI": "line1"}},
+		{"UTF16 little endian", encodeUTF16(unicode.LittleEndian), map[string]string{"FOO": "bar\r", "MULTI": "line1\r"}},
+		{"UTF16 big endian", encodeUTF16(unicode.BigEndian), map[string]string{"FOO": "bar\r", "MULTI": "line1\r"}},
 	} {
 		t.Run(test.name, func(t *testing.T) {
 			e, envPath := newTestHostEnv(t)
 			require.NoError(t, os.WriteFile(envPath, []byte(test.content), 0o600))
 			env := map[string]string{}
-			require.NoError(t, parseEnvFile(e, envPath, &env)(t.Context()))
+			require.NoError(t, parseEnvFile(e, envPath, &env, false)(t.Context()))
 			assert.Equal(t, test.want, env)
 		})
 	}
+	t.Run("Windows line endings", func(t *testing.T) {
+		hostEnv, envPath := newTestHostEnv(t)
+		require.NoError(t, os.WriteFile(envPath, []byte("FOO=bar\r\nMULTI<<EOF\r\nline1\r\nline2\nEOF\r\n"), 0o600))
+		env := map[string]string{}
+		require.NoError(t, parseEnvFile(hostEnv, envPath, &env, true)(t.Context()))
+		assert.Equal(t, map[string]string{"FOO": "bar", "MULTI": "line1\r\nline2"}, env)
+	})
 }
 
 func TestParseEnvFileLineExceedsBufferReportsScannerError(t *testing.T) {
@@ -57,17 +70,27 @@ func TestParseEnvFileLineExceedsBufferReportsScannerError(t *testing.T) {
 	require.NoError(t, os.WriteFile(envPath, []byte("FOO<<EOF\n"+strings.Repeat("x", 17*1024*1024)+"\nEOF\n"), 0o600))
 
 	env := map[string]string{}
-	err := parseEnvFile(e, envPath, &env)(t.Context())
+	err := parseEnvFile(e, envPath, &env, false)(t.Context())
 	require.ErrorIs(t, err, bufio.ErrTooLong)
 	assert.Contains(t, err.Error(), "reading env file")
 }
 
-func TestParseEnvFileMissingDelimiter(t *testing.T) {
-	e, envPath := newTestHostEnv(t)
-	require.NoError(t, os.WriteFile(envPath, []byte("FOO<<EOF\nline1\nline2\n"), 0o600))
+func TestParseEnvFileInvalidHeredoc(t *testing.T) {
+	for name, content := range map[string]string{
+		"missing delimiter":              "FOO<<EOF\nline1\nline2\n",
+		"empty name":                     "<<EOF\nline1\nEOF\n",
+		"empty delimiter":                "FOO<<\nline1\n\n",
+		"delimiter with carriage return": "FOO<<EOF\nline1\nEOF\r\n",
+	} {
+		t.Run(name, func(t *testing.T) {
+			e, envPath := newTestHostEnv(t)
+			require.NoError(t, os.WriteFile(envPath, []byte(content), 0o600))
 
-	env := map[string]string{}
-	err := parseEnvFile(e, envPath, &env)(t.Context())
-	require.Error(t, err)
-	assert.Contains(t, err.Error(), "delimiter")
+			env := map[string]string{}
+			err := parseEnvFile(e, envPath, &env, false)(t.Context())
+			require.Error(t, err)
+			assert.Contains(t, err.Error(), "delimiter")
+			assert.Empty(t, env)
+		})
+	}
 }