diff --git a/.gitea/workflows/test-windows.yml b/.gitea/workflows/test-windows.yml new file mode 100644 index 00000000..f5c92458 --- /dev/null +++ b/.gitea/workflows/test-windows.yml @@ -0,0 +1,21 @@ +name: windows +on: + schedule: + - cron: '0 3 * * 1' + workflow_dispatch: + +env: + DOCKER_CONFIG: /tmp/docker-noauth # see test.yml + +jobs: + test: + name: test on windows + runs-on: ubuntu-latest + timeout-minutes: 180 + steps: + - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7 + - uses: ./.gitea/actions/go-setup + - name: prepare anonymous docker config + run: mkdir -p "$DOCKER_CONFIG" && echo '{}' > "$DOCKER_CONFIG/config.json" + - name: test on windows + run: make test-windows diff --git a/.goreleaser.yaml b/.goreleaser.yaml index bd0d651d..cc3f6ab7 100644 --- a/.goreleaser.yaml +++ b/.goreleaser.yaml @@ -34,8 +34,6 @@ builds: goarch: s390x - goos: windows goarch: arm - - goos: windows - goarch: arm64 - goos: freebsd goarch: s390x - goos: freebsd diff --git a/DEVELOPMENT.md b/DEVELOPMENT.md index a126c951..4aff42aa 100644 --- a/DEVELOPMENT.md +++ b/DEVELOPMENT.md @@ -69,3 +69,12 @@ Docker and Kubernetes. The suite shares one Gitea, regular runner and k3s cluster. Cache and ephemeral scenarios use isolated repository runners. A run path is `@` and a log row is `Z `. + +## Windows tests + +`make test-windows` cross-compiles the test binaries, installs Windows Server 2025 Core in a +[dockur/windows](https://github.com/dockur/windows) QEMU VM on the docker host and runs them there. +The host must be amd64 and allow privileged containers. `WINDOWS_TEST_TIMEOUT` sets the minutes +allowed for installing Windows and running the tests, 165 by default. The VM uses KVM when the host +has `/dev/kvm`, otherwise Windows is emulated and much slower. Docker-backed tests skip in the VM +and the race detector is off, it needs cgo. CI runs this weekly and on demand. diff --git a/Makefile b/Makefile index c4ef99e2..b123bf5c 100644 --- a/Makefile +++ b/Makefile @@ -8,6 +8,7 @@ E2E_JOB_IMAGE ?= docker.gitea.com/runner-images:ubuntu-latest@sha256:657378e5cfd SERVICE_IMAGE ?= nginx:1.31.7-alpine@sha256:892bb2152464f0c5d8972593a5f5afaf8054552580b699d95f8ff4af08c2d33d # renovate: datasource=docker E2E_GITEA_IMAGE ?= gitea/gitea:main-nightly E2E_K3S_IMAGE ?= rancher/k3s:v1.37.1-k3s1@sha256:ca7f37d993d82ef0dcdcfecb2e0e2618ea541dbaffc620c8cedebe01a82acd0d # renovate: datasource=docker +WINDOWS_TEST_IMAGE ?= dockurr/windows:6.06@sha256:16fd7c39a3e48bb946d29c59dc6015942e5b900a1fa5d7e5552053bfc48ee2ea # renovate: datasource=docker GOFILES := $(shell find . -type f -name "*.go" -o -name "go.mod" ! -name "generated.*") @@ -150,6 +151,10 @@ coverage-report: ## turn coverage.txt from `make test` into .tmp/coverage.md test-dind: ## run the daemon-facing tests against the built dind image or podman (TARGET=dind|dind-rootless|podman) @PODMAN_TEST_IMAGE=$(PODMAN_TEST_IMAGE) ./scripts/test-dind.sh $(TARGET) +.PHONY: test-windows +test-windows: ## run the tests on Windows Server Core in a QEMU VM on the docker host + @WINDOWS_TEST_IMAGE=$(WINDOWS_TEST_IMAGE) ./scripts/test-windows.sh + E2E_CONCURRENCY ?= 8 .PHONY: test-e2e diff --git a/README.md b/README.md index abaeb53b..92c3b8a5 100644 --- a/README.md +++ b/README.md @@ -2,6 +2,8 @@ ## Installation +The runner runs on Linux, macOS and Windows. + ### Prerequisites On the runner: @@ -89,6 +91,23 @@ If the registry succeed, it will run immediately. Next time, you could run the r ./gitea-runner daemon ``` +### Windows service + +Register the runner first, keeping `gitea-runner.exe`, `config.yaml` and the `.runner` registration file in the same directory. From an elevated Command Prompt, install and start the service with absolute paths: + +```bat +sc.exe create gitea-runner binPath= "\"C:\gitea-runner\gitea-runner.exe\" daemon --config \"C:\gitea-runner\config.yaml\"" start= auto +sc.exe start gitea-runner +``` + +The service runs from the executable's directory and appends its logs to `gitea-runner.log` there. Its account needs write access to that directory and access to the tools used by jobs. Use `sc.exe stop gitea-runner` to stop it, allowing running jobs to finish according to `runner.shutdown_timeout`. + +On shutdown or reboot, Windows waits for the service's preshutdown timeout at most, so set it above `runner.shutdown_timeout`, here six minutes for `5m`. It takes effect after the next reboot: + +```bat +reg.exe add HKLM\SYSTEM\CurrentControlSet\Services\gitea-runner /v PreshutdownTimeout /t REG_DWORD /d 360000 /f +``` + ### Run with docker The recommended Docker-in-Docker image runs its own Docker daemon for jobs. Jobs never get the host's Docker socket, which is equivalent to root access on the host. @@ -233,7 +252,9 @@ The `daemon` command also accepts `--labels` (which defaults to the `GITEA_RUNNE Whenever the resulting labels differ from the ones in the registration file, they are written back to it and re-declared to the Gitea instance on startup. -> **Note:** A runner that only exposes `host` labels still needs access to a Docker daemon (e.g. a mounted `/var/run/docker.sock`) whenever a job uses a `docker://` action or a service container. `host` labels only change where the job's own steps run; container-based steps and actions are still executed with Docker. +> **Note:** A runner that only exposes `host` labels still needs access to a Docker daemon (e.g. a mounted `/var/run/docker.sock`) whenever a job uses a `docker://` action or a service container. `host` labels only change where the job's own steps run; container-based steps and actions are still executed with Docker. Windows hosts do not support container actions and `docker://` steps in host jobs, like GitHub's Windows runners. + +On Windows hosts, a step's background processes end with the job unless it opts out, see [host-mode background processes](docs/host-mode.md). #### Service containers diff --git a/act/artifacts/server_test.go b/act/artifacts/server_test.go index f4fafa9a..7d63dd3c 100644 --- a/act/artifacts/server_test.go +++ b/act/artifacts/server_test.go @@ -281,7 +281,7 @@ func TestSafeResolve(t *testing.T) { for name, tc := range tests { t.Run(name, func(t *testing.T) { - require.Equal(t, tc.want, safeResolve(baseDir, tc.input)) + require.Equal(t, filepath.FromSlash(tc.want), safeResolve(baseDir, tc.input)) }) } } diff --git a/act/common/git/fetch_action.go b/act/common/git/fetch_action.go index bbd77aa0..0d67778e 100644 --- a/act/common/git/fetch_action.go +++ b/act/common/git/fetch_action.go @@ -10,6 +10,7 @@ import ( "context" "encoding/json" "errors" + "fmt" "io" "math/rand/v2" "net" @@ -29,6 +30,10 @@ import ( var actionFetches sync.Map +var symlink = (*os.Root).Symlink + +var symlinkFallback = runtime.GOOS == "windows" + const treeSizes = ".sizes" // beside the tree rather than in it, which is copied into job containers as it is type treeFile struct { @@ -229,6 +234,8 @@ func extractTree(ctx context.Context, archive io.Reader, dir string, strip bool) sha := "" sizes := map[string]int64{} + links := map[string]string{} + var deferred []string reader := tar.NewReader(archive) for { header, err := reader.Next() @@ -257,9 +264,8 @@ func extractTree(ctx context.Context, archive io.Reader, dir string, strip bool) case tar.TypeReg: sizes[name], err = writeTreeFile(root, name, header.Mode, reader) case tar.TypeSymlink: - if err = root.Symlink(header.Linkname, name); err != nil && runtime.GOOS == "windows" { - sizes[name], err = writeTreeFile(root, name, 0, strings.NewReader(header.Linkname)) - } + links[filepath.Clean(name)] = header.Linkname + deferred = append(deferred, filepath.Clean(name)) } if err != nil { return "", err @@ -268,12 +274,166 @@ func extractTree(ctx context.Context, archive io.Reader, dir string, strip bool) if !isFullObjectID(sha) { return "", errors.New("the archive names no commit") } + linksByParent := map[string][]string{} + if symlinkFallback { + for name := range links { + linksByParent[filepath.Dir(name)] = append(linksByParent[filepath.Dir(name)], filepath.Base(name)) + } + } + for requireTarget := symlinkFallback; len(deferred) > 0; { + unresolved := deferred[:0] + for _, name := range deferred { + if err := ctx.Err(); err != nil { + return "", err + } + if requireTarget { + if _, err := root.Stat(filepath.Dir(name) + string(filepath.Separator) + links[name]); err != nil { + unresolved = append(unresolved, name) + continue + } + } + if err := symlink(root, links[name], name); err == nil || !symlinkFallback { + if err != nil { + return "", err + } + continue + } + if err := copyTreeLink(ctx, root, name, links, linksByParent, sizes); err != nil { + return "", fmt.Errorf("materialize symlink %q: %w", filepath.ToSlash(name), err) + } + } + if len(unresolved) == len(deferred) { + requireTarget = false + } + deferred = unresolved + } if err := root.Close(); err != nil { return "", err } return sha, installTree(ctx, dir, sha, temp, sizes) } +func resolveTreeLink(name string, links map[string]string) (string, bool) { + if !filepath.IsLocal(name) { + return "", false + } + parts := strings.Split(name, string(filepath.Separator)) + var resolved []string + for count := 0; len(parts) > 0; { + part := parts[0] + parts = parts[1:] + switch part { + case "", ".": + continue + case "..": + if len(resolved) == 0 { + return "", false + } + resolved = resolved[:len(resolved)-1] + continue + } + target, ok := links[filepath.Join(append(resolved, part)...)] + if !ok { + resolved = append(resolved, part) + continue + } + count++ + if target == "" || os.IsPathSeparator(target[0]) || filepath.VolumeName(target) != "" || count > 255 { + return "", false + } + parts = append(strings.Split(filepath.FromSlash(target), string(filepath.Separator)), parts...) + } + return filepath.Join(append([]string{"."}, resolved...)...), true +} + +var ( + errTreeLinkTarget = errors.New("unmaterializable symlink target") + errTreeLinkCycle = fmt.Errorf("cyclic %w", errTreeLinkTarget) +) + +func copyTreeLink(ctx context.Context, root *os.Root, name string, links map[string]string, linksByParent map[string][]string, sizes map[string]int64) error { + writeLinkText := func(destination, target string) error { // as git writes links it cannot create + if err := root.RemoveAll(destination); err != nil { + return err + } + for written := range sizes { + if written == destination || strings.HasPrefix(written, destination+string(filepath.Separator)) { + delete(sizes, written) + } + } + size, err := writeTreeFile(root, destination, 0o644, strings.NewReader(target)) + sizes[destination] = size + return err + } + active := map[string]bool{} + var copyTarget func(string, string) error + copyTarget = func(source, destination string) error { + if err := ctx.Err(); err != nil { + return err + } + source, ok := resolveTreeLink(source, links) + if !ok { + return errTreeLinkTarget + } + file, err := root.Open(source) + if err != nil { + if errors.Is(err, os.ErrNotExist) { + return errTreeLinkTarget + } + return err + } + defer file.Close() + info, err := file.Stat() + if err != nil { + return err + } + if info.Mode().IsRegular() { + sizes[destination], err = writeTreeFile(root, destination, int64(info.Mode().Perm()), file) + return err + } + if !info.IsDir() { + return errTreeLinkTarget + } + if active[source] { + return errTreeLinkCycle + } + active[source] = true + defer delete(active, source) + entries, err := file.ReadDir(-1) + if err != nil { + return err + } + children := map[string]bool{} + for _, entry := range entries { + children[entry.Name()] = true + } + for _, child := range linksByParent[source] { + children[child] = true + } + if err := root.MkdirAll(destination, info.Mode().Perm()); err != nil { + return err + } + for child := range children { + childSource, childDestination := filepath.Join(source, child), filepath.Join(destination, child) + err := copyTarget(childSource, childDestination) + if target, isLink := links[childSource]; isLink && errors.Is(err, errTreeLinkTarget) && !errors.Is(err, errTreeLinkCycle) { + err = writeLinkText(childDestination, target) + } + if err != nil { + return err + } + } + return nil + } + if err := copyTarget(name, name); !errors.Is(err, errTreeLinkTarget) { + return err + } + if err := ctx.Err(); err != nil { + return err + } + return writeLinkText(name, links[name]) +} + var lockTree = AcquireCloneLock // replaced in tests to see installTree wait for a job holding the tree // installTree moves temp into place as dir/sha, replacing a tree that failed treeIntact. diff --git a/act/common/git/fetch_action_test.go b/act/common/git/fetch_action_test.go index 85a68bec..913d22fc 100644 --- a/act/common/git/fetch_action_test.go +++ b/act/common/git/fetch_action_test.go @@ -8,6 +8,7 @@ import ( "bytes" "context" "encoding/base64" + "errors" "fmt" "net/http" "net/http/cgi" @@ -40,6 +41,7 @@ func TestFetchAction(t *testing.T) { require.NoError(t, os.WriteFile(filepath.Join(remote, "run.sh"), []byte("#!/bin/sh\n"), 0o755)) require.NoError(t, os.Symlink("action.yml", filepath.Join(remote, "link"))) require.NoError(t, gitCmd("-C", remote, "add", ".")) + require.NoError(t, gitCmd("-C", remote, "update-index", "--chmod=+x", "run.sh")) require.NoError(t, gitCmd("-C", remote, "commit", "-m", "initial")) require.NoError(t, gitCmd("-C", remote, "tag", "v1")) expectedSha := gitRevParse(t, remote, "main") @@ -192,7 +194,9 @@ func TestFetchAction(t *testing.T) { assert.NoDirExists(t, filepath.Join(tree, ".git")) info, err := os.Stat(filepath.Join(tree, "run.sh")) require.NoError(t, err) - assert.Equal(t, os.FileMode(0o755), info.Mode().Perm()) + if runtime.GOOS != "windows" { + assert.EqualValues(t, 0o755, info.Mode().Perm()) + } link, err := os.Readlink(filepath.Join(tree, "link")) require.NoError(t, err) assert.Equal(t, "action.yml", link) @@ -391,6 +395,79 @@ func TestExtractTreeConfinesEntries(t *testing.T) { } } +func TestExtractTreeCopiesDeferredLinks(t *testing.T) { + originalSymlink, originalFallback := symlink, symlinkFallback + symlink = func(_root *os.Root, _target, _name string) error { return errors.New("symlink privilege unavailable") } + symlinkFallback = true + t.Cleanup(func() { symlink, symlinkFallback = originalSymlink, originalFallback }) + + for _, test := range []struct { + name string + links [][2]string + files []string + text bool + }{ + {name: "file chain", links: [][2]string{{"index.js", "entry.js"}, {"entry.js", "dist/index.js"}}, files: []string{"index.js", "entry.js"}}, + {name: "directory chain", links: [][2]string{{"lib", "bundle"}, {"bundle", "dist"}, {"dist/entry.js", "index.js"}}, files: []string{"lib/index.js", "lib/entry.js", "lib/nested/index.js", "bundle/index.js", "bundle/entry.js", "bundle/nested/index.js"}}, + {name: "directory in target path", links: [][2]string{{"index.js", "bundle/nested/index.js"}, {"bundle", "dist"}}, files: []string{"index.js", "bundle/nested/index.js"}}, + {name: "parent after directory link", links: [][2]string{{"index.js", "bundle/../index.js"}, {"bundle", "dist/nested"}}, files: []string{"index.js", "bundle/index.js"}}, + {name: "outside", links: [][2]string{{"index.js", "../outside.js"}}, text: true}, + {name: "absolute", links: [][2]string{{"index.js", t.TempDir()}}, text: true}, + {name: "dangling", links: [][2]string{{"index.js", "missing.js"}}, text: true}, + {name: "dangling child of directory link", links: [][2]string{{"lib", "dist"}, {"dist/optional", "missing.js"}}, files: []string{"lib/index.js", "lib/nested/index.js"}}, + {name: "cycle", links: [][2]string{{"index.js", "entry.js"}, {"entry.js", "index.js"}}, text: true}, + {name: "directory cycle", links: [][2]string{{"dist/loop", "."}}, text: true}, + } { + t.Run(test.name, func(t *testing.T) { + sha := strings.Repeat("a", 40) + var archive bytes.Buffer + writer := tar.NewWriter(&archive) + require.NoError(t, writer.WriteHeader(&tar.Header{Typeflag: tar.TypeXGlobalHeader, PAXRecords: map[string]string{"comment": sha}})) + require.NoError(t, writer.WriteHeader(&tar.Header{Name: "dist", Typeflag: tar.TypeDir, Mode: 0o755})) + require.NoError(t, writer.WriteHeader(&tar.Header{Name: "dist/nested", Typeflag: tar.TypeDir, Mode: 0o755})) + for _, link := range test.links { + require.NoError(t, writer.WriteHeader(&tar.Header{Name: link[0], Typeflag: tar.TypeSymlink, Linkname: link[1]})) + } + content := "console.log('action');\n" + for _, name := range []string{"dist/index.js", "dist/nested/index.js"} { + require.NoError(t, writer.WriteHeader(&tar.Header{Name: name, Typeflag: tar.TypeReg, Mode: 0o755, Size: int64(len(content))})) + _, err := writer.Write([]byte(content)) + require.NoError(t, err) + } + require.NoError(t, writer.Close()) + + dir := t.TempDir() + _, err := extractTree(t.Context(), &archive, dir, false) + require.NoError(t, err) + assert.True(t, treeIntact(dir, sha)) + if test.text { + for _, link := range test.links { + data, err := os.ReadFile(filepath.Join(dir, sha, link[0])) + require.NoError(t, err) + assert.Equal(t, link[1], string(data)) + info, err := os.Lstat(filepath.Join(dir, sha, link[0])) + require.NoError(t, err) + assert.True(t, info.Mode().IsRegular()) + } + } + for _, name := range test.files { + data, err := os.ReadFile(filepath.Join(dir, sha, name)) + require.NoError(t, err) + assert.Equal(t, content, string(data)) + info, err := os.Lstat(filepath.Join(dir, sha, name)) + require.NoError(t, err) + assert.True(t, info.Mode().IsRegular()) + if runtime.GOOS != "windows" { + assert.Equal(t, os.FileMode(0o755), info.Mode().Perm()) + } + require.NoError(t, os.WriteFile(filepath.Join(dir, sha, name), nil, 0o644)) + assert.False(t, treeIntact(dir, sha), name) + require.NoError(t, os.WriteFile(filepath.Join(dir, sha, name), []byte(content), 0o755)) + } + }) + } +} + func TestTreeIntactFindsNamesThatAreNotUTF8(t *testing.T) { if runtime.GOOS != "linux" { t.Skip("macOS and Windows store no name that is not UTF-8") @@ -410,3 +487,76 @@ func TestTreeIntactFindsNamesThatAreNotUTF8(t *testing.T) { require.FileExists(t, filepath.Join(dir, sha, "caf\xe9.js")) assert.True(t, treeIntact(dir, sha)) } + +func TestExtractTreeCreatesLinksInPlatformOrder(t *testing.T) { + if runtime.GOOS == "windows" { + t.Skip("requires symlink privileges") + } + original, originalFallback := symlink, symlinkFallback + var created []string + symlink = func(root *os.Root, target, name string) error { + created = append(created, name) + return root.Symlink(target, name) + } + t.Cleanup(func() { symlink, symlinkFallback = original, originalFallback }) + + var archive bytes.Buffer + writer := tar.NewWriter(&archive) + require.NoError(t, writer.WriteHeader(&tar.Header{Typeflag: tar.TypeXGlobalHeader, PAXRecords: map[string]string{"comment": strings.Repeat("a", 40)}})) + for _, entry := range []*tar.Header{ + {Name: "a", Typeflag: tar.TypeSymlink, Linkname: "b"}, + {Name: "b", Typeflag: tar.TypeSymlink, Linkname: "dist"}, + {Name: "dangling", Typeflag: tar.TypeSymlink, Linkname: "missing"}, + {Name: "dist", Typeflag: tar.TypeDir, Mode: 0o755}, + } { + require.NoError(t, writer.WriteHeader(entry)) + } + require.NoError(t, writer.Close()) + for _, test := range []struct { + name string + fallback bool + want []string + }{ + {name: "native", want: []string{"a", "b", "dangling"}}, + {name: "fallback", fallback: true, want: []string{"b", "a", "dangling"}}, + } { + t.Run(test.name, func(t *testing.T) { + symlinkFallback = test.fallback + created = nil + _, err := extractTree(t.Context(), bytes.NewReader(archive.Bytes()), t.TempDir(), false) + require.NoError(t, err) + assert.Equal(t, test.want, created) + }) + } +} + +func TestCopyTreeLinkCancellationBetweenEntries(t *testing.T) { + root, err := os.OpenRoot(t.TempDir()) + require.NoError(t, err) + defer root.Close() + require.NoError(t, root.Mkdir("source", 0o755)) + for _, name := range []string{"first", "second"} { + _, err := writeTreeFile(root, filepath.Join("source", name), 0o644, strings.NewReader("content")) + require.NoError(t, err) + } + sizes := map[string]int64{} + ctx, cancel := context.WithCancel(t.Context()) + defer cancel() + observed := &treeCopyContext{Context: ctx, check: func() { + if len(sizes) == 1 { + cancel() + } + }} + require.ErrorIs(t, copyTreeLink(observed, root, "copy", map[string]string{"copy": "source"}, nil, sizes), context.Canceled) + assert.Len(t, sizes, 1) +} + +type treeCopyContext struct { + context.Context + check func() +} + +func (ctx *treeCopyContext) Err() error { + ctx.check() + return ctx.Context.Err() +} diff --git a/act/common/git/git.go b/act/common/git/git.go index 889f86c0..0db6724f 100644 --- a/act/common/git/git.go +++ b/act/common/git/git.go @@ -5,6 +5,7 @@ package git import ( + "bytes" "context" "crypto/sha256" "encoding/base64" @@ -27,6 +28,7 @@ import ( "gitea.com/gitea/runner/act/common" "gitea.com/gitea/runner/act/common/logging" "gitea.com/gitea/runner/internal/pkg/lock" + "gitea.com/gitea/runner/internal/pkg/process" ) var ( @@ -387,6 +389,17 @@ func RemoteConfig(token string, insecureSkipTLS bool) []string { return config } +func ClientCertConfig(prefix, certFile, keyFile, executionOS string) []string { + if certFile == "" { + return nil + } + config := []string{prefix + "sslCert=" + certFile, prefix + "sslKey=" + keyFile} + if strings.EqualFold(executionOS, "windows") { + config = append(config, prefix+"sslBackend=openssl") + } + return config +} + // ConfigEnv applies `-c` style entries through the environment, keeping secrets out of argv. func ConfigEnv(config ...string) map[string]string { env := map[string]string{"GIT_CONFIG_COUNT": strconv.Itoa(len(config))} @@ -444,9 +457,7 @@ func runGitWithStdin(ctx context.Context, dir string, input *NewGitCloneExecutor config := []string{"core.autocrlf=false", "core.eol=lf"} // keep action files LF if input != nil { config = append(config, RemoteConfig(input.Token, input.InsecureSkipTLS)...) - if input.ClientCertFile != "" { - config = append(config, "http.sslCert="+input.ClientCertFile, "http.sslKey="+input.ClientKeyFile) - } + config = append(config, ClientCertConfig("http.", input.ClientCertFile, input.ClientKeyFile, runtime.GOOS)...) } if dir != "" { args = append([]string{"-C", dir}, args...) @@ -461,15 +472,28 @@ func runGitWithStdin(ctx context.Context, dir string, input *NewGitCloneExecutor cmd.Env = append(cmd.Env, key+"="+value) } cmd.Env = append(cmd.Env, "GIT_TERMINAL_PROMPT=0") - if runtime.GOOS != "windows" { + var treeKill *process.TreeKill + if runtime.GOOS == "windows" { + treeKill = process.NewTreeKill(cmd) // a cancelled git leaves its remote helper holding the repository otherwise + } else { cmd.Cancel = func() error { return cmd.Process.Signal(os.Interrupt) } // lets git remove its lock files } cmd.WaitDelay = time.Second + var stdout bytes.Buffer var stderr strings.Builder + cmd.Stdout = &stdout cmd.Stderr = &stderr - output, err := cmd.Output() - if err != nil { - return output, fmt.Errorf("git %s: %w: %s", strings.Join(args, " "), err, strings.TrimSpace(stderr.String())) + err := cmd.Start() + if err == nil { + if treeKill != nil { + if killer, captureErr := treeKill.Capture(cmd.Process); captureErr == nil { + defer killer.Close() + } + } + err = cmd.Wait() } - return output, nil + if err != nil { + return stdout.Bytes(), fmt.Errorf("git %s: %w: %s", strings.Join(args, " "), err, strings.TrimSpace(stderr.String())) + } + return stdout.Bytes(), nil } diff --git a/act/common/git/git_test.go b/act/common/git/git_test.go index ca852edc..f077ca1f 100644 --- a/act/common/git/git_test.go +++ b/act/common/git/git_test.go @@ -54,6 +54,22 @@ func TestMain(m *testing.M) { os.Exit(code) } +func TestClientCertConfig(t *testing.T) { + for _, testcase := range []struct { + prefix, certFile, executionOS string + expected []string + }{ + {"http.", "cert.pem", "windows", []string{"http.sslCert=cert.pem", "http.sslKey=key.pem", "http.sslBackend=openssl"}}, + {"http.https://example.com/.", "cert.pem", "Windows", []string{"http.https://example.com/.sslCert=cert.pem", "http.https://example.com/.sslKey=key.pem", "http.https://example.com/.sslBackend=openssl"}}, + {"http.", "cert.pem", "Linux", []string{"http.sslCert=cert.pem", "http.sslKey=key.pem"}}, + {"http.", "", "Windows", nil}, + } { + t.Run(testcase.prefix+testcase.certFile+testcase.executionOS, func(t *testing.T) { + assert.Equal(t, testcase.expected, ClientCertConfig(testcase.prefix, testcase.certFile, "key.pem", testcase.executionOS)) + }) + } +} + func TestFindGitSlug(t *testing.T) { for _, testcase := range []struct { remoteURL string @@ -185,7 +201,8 @@ func TestGitFindRef(t *testing.T) { func TestGitCloneExecutor(t *testing.T) { remoteDir, workDir := newGitRemote(t) require.NoError(t, os.WriteFile(filepath.Join(workDir, "action.sh"), []byte("#!/bin/sh\necho hi\n"), 0o755)) - require.NoError(t, gitCmd("-C", workDir, "add", "action.sh")) + require.NoError(t, os.WriteFile(filepath.Join(workDir, ".gitattributes"), []byte("*.sh text\n"), 0o644)) + require.NoError(t, gitCmd("-C", workDir, "add", "action.sh", ".gitattributes")) require.NoError(t, gitCmd("-C", workDir, "commit", "-m", "initial")) require.NoError(t, gitCmd("-C", workDir, "tag", "v2")) require.NoError(t, gitCmd("-C", workDir, "push", "-u", "origin", "main")) @@ -200,7 +217,7 @@ func TestGitCloneExecutor(t *testing.T) { require.NoError(t, gitCmd("-C", workDir, "push", "origin", "main:refs/heads/"+fullSha[:4])) hostConfig := filepath.Join(t.TempDir(), "gitconfig") - require.NoError(t, os.WriteFile(hostConfig, []byte("[core]\n\tautocrlf = true\n[url \""+remoteDir+"\"]\n\tinsteadOf = https://example.invalid/action\n"), 0o644)) + require.NoError(t, os.WriteFile(hostConfig, []byte("[core]\n\tautocrlf = true\n\teol = crlf\n[url "+strconv.Quote(remoteDir)+"]\n\tinsteadOf = https://example.invalid/action\n"), 0o644)) t.Setenv("GIT_CONFIG_GLOBAL", hostConfig) t.Setenv("GIT_DIR", t.TempDir()) @@ -676,8 +693,8 @@ func TestNewGitCloneExecutorFetchSendsTokenAndHonoursContext(t *testing.T) { select { case <-reached: - case <-time.After(10 * time.Second): - t.Fatal("the executor never reached the remote") + case err := <-done: + t.Fatalf("the executor finished before reaching the remote: %v", err) } assert.Equal(t, "Basic "+base64.StdEncoding.EncodeToString([]byte("token:secret")), authorization) cancel() diff --git a/act/common/lookup_fold.go b/act/common/lookup_fold.go new file mode 100644 index 00000000..d17f1f76 --- /dev/null +++ b/act/common/lookup_fold.go @@ -0,0 +1,20 @@ +// Copyright 2026 The Gitea Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package common + +import "strings" + +// LookupFold finds name in values, ignoring case where the environment does, as GitHub's inputs always do. +func LookupFold[V any](values map[string]V, name string, caseInsensitive bool) (V, bool) { + if value, ok := values[name]; ok || !caseInsensitive { + return value, ok + } + for key, value := range values { + if strings.EqualFold(key, name) { + return value, true + } + } + var zero V + return zero, false +} diff --git a/act/container/container_types.go b/act/container/container_types.go index 50bc4a86..c7deae30 100644 --- a/act/container/container_types.go +++ b/act/container/container_types.go @@ -10,6 +10,7 @@ import ( "fmt" "io" "path" + "runtime" "strings" "sync" "sync/atomic" @@ -20,6 +21,8 @@ import ( "github.com/moby/moby/api/types/container" ) +var hostOS = runtime.GOOS // replaced in tests + // ExitCodeError reports a non-zero process exit code from a container command. type ExitCodeError int diff --git a/act/container/docker_build.go b/act/container/docker_build.go index 8dcb3137..4f261699 100644 --- a/act/container/docker_build.go +++ b/act/container/docker_build.go @@ -125,6 +125,10 @@ func createBuildContext(ctx context.Context, contextDir, relDockerfile string) ( } func writeBuildContext(ctx context.Context, w io.Writer, contextDir string, includes []string, pm *patternmatcher.PatternMatcher) error { + contextDir, err := filepath.EvalSymlinks(contextDir) + if err != nil { + return err + } tw := tar.NewWriter(w) seen := map[string]bool{} hardlinks := map[uint64]string{} @@ -132,8 +136,7 @@ func writeBuildContext(ctx context.Context, w io.Writer, contextDir string, incl for _, include := range includes { include = filepath.FromSlash(include) var parents []parentMatch - root := strings.TrimSuffix(contextDir, string(filepath.Separator)) + string(filepath.Separator) + include // unlike filepath.Join, follows a symlinked context dir as docker build does - err := filepath.WalkDir(root, func(file string, entry fs.DirEntry, err error) error { + err := filepath.WalkDir(filepath.Join(contextDir, include), func(file string, entry fs.DirEntry, err error) error { if err != nil { return leaveOut(ctx, file, err) } @@ -210,7 +213,7 @@ func addBuildContextEntry(ctx context.Context, tw *tar.Writer, file, rel string, return leaveOut(ctx, file, err) } } - header, err := tar.FileInfoHeader(noNameLookups{info}, link) + header, err := tar.FileInfoHeader(noNameLookups{info}, filepath.ToSlash(link)) if err != nil { return err } diff --git a/act/container/docker_build_test.go b/act/container/docker_build_test.go index 2546880d..4f5b2981 100644 --- a/act/container/docker_build_test.go +++ b/act/container/docker_build_test.go @@ -50,7 +50,7 @@ func TestCreateBuildContextFollowsDockerignoreAndSymlinkedRootButKeepsDockerfile contexts := []string{dir} if link := filepath.Join(t.TempDir(), "link"); os.Symlink(dir, link) == nil { - contexts = append(contexts, link+string(filepath.Separator)) + contexts = append(contexts, link, link+string(filepath.Separator)) } for _, contextDir := range contexts { diff --git a/act/container/docker_cli.go b/act/container/docker_cli.go index 10c73b21..c3d29cea 100644 --- a/act/container/docker_cli.go +++ b/act/container/docker_cli.go @@ -351,7 +351,7 @@ type containerConfig struct { // parse parses the args for the specified command and generates a Config, // a HostConfig and returns them with the specified command. // If the specified args are not valid, it will return an error. -func parse(flags *pflag.FlagSet, copts *containerOptions, serverOS string) (*containerConfig, error) { //nolint:gocyclo,unparam // verbatim copy from docker/cli +func parse(flags *pflag.FlagSet, copts *containerOptions, serverOS string) (*containerConfig, error) { //nolint:gocyclo // verbatim copy from docker/cli var ( attachStdin = copts.attach.Get("stdin") attachStdout = copts.attach.Get("stdout") @@ -724,6 +724,10 @@ func parse(flags *pflag.FlagSet, copts *containerOptions, serverOS string) (*con }, nil } +func isUserDefinedNetwork(mode container.NetworkMode) bool { // Docker's helper uses the host OS instead of the daemon's. + return !mode.IsDefault() && mode != network.NetworkBridge && mode != network.NetworkHost && !mode.IsNone() && !mode.IsContainer() +} + // parseNetworkOpts converts --network advanced options to endpoint-specs, and combines // them with the old --network-alias and --links. If returns an error if conflicting options // are found. @@ -752,7 +756,7 @@ func parseNetworkOpts(copts *containerOptions) (map[string]*network.EndpointSett } for i, n := range copts.netMode.Value() { - if container.NetworkMode(n.Target).IsUserDefined() { + if isUserDefinedNetwork(container.NetworkMode(n.Target)) { hasUserDefined = true } else { hasNonUserDefined = true @@ -814,7 +818,7 @@ func applyContainerOptions(n *opts.NetworkAttachmentOpts, copts *containerOption } // For a user-defined network, "--link" is an endpoint option, it creates an alias. But, // for the default bridge it defines a legacy-link. - if container.NetworkMode(n.Target).IsUserDefined() && copts.links.Len() > 0 { + if isUserDefinedNetwork(container.NetworkMode(n.Target)) && copts.links.Len() > 0 { n.Links = slices.Clone(copts.links.GetSlice()) } if copts.ipv4Address != nil { @@ -840,7 +844,7 @@ func parseNetworkAttachmentOpt(ep opts.NetworkAttachmentOpts) (*network.Endpoint if strings.TrimSpace(ep.Target) == "" { return nil, errors.New("no name set for network") } - if !container.NetworkMode(ep.Target).IsUserDefined() { + if !isUserDefinedNetwork(container.NetworkMode(ep.Target)) { if len(ep.Aliases) > 0 { return nil, errors.New("network-scoped aliases are only supported for user-defined networks") } diff --git a/act/container/docker_proxy_test.go b/act/container/docker_proxy_test.go index 7c18d7c0..966f8e85 100644 --- a/act/container/docker_proxy_test.go +++ b/act/container/docker_proxy_test.go @@ -484,6 +484,10 @@ type probeClient struct { containers []container.Summary } +func (c *probeClient) Info(_ctx context.Context, _opts mobyclient.InfoOptions) (mobyclient.SystemInfoResult, error) { + return mobyclient.SystemInfoResult{}, nil +} + func (c *probeClient) Close() error { return nil } @@ -766,6 +770,9 @@ func TestDaemonSeesDir(t *testing.T) { } func TestRunnerContainerWorkdir(t *testing.T) { + if runtime.GOOS == "windows" { + t.Skip("a runner container is Linux") + } workdir := t.TempDir() t.Chdir(workdir) volume := container.MountPoint{Type: mount.TypeVolume, Source: "/daemon/_data", Destination: filepath.Dir(workdir)} diff --git a/act/container/docker_run.go b/act/container/docker_run.go index fb731541..06640432 100644 --- a/act/container/docker_run.go +++ b/act/container/docker_run.go @@ -366,6 +366,7 @@ type containerReference struct { id string input *NewContainerInput pullPolicy string + daemonOS string UID int GID int // attachDone is closed by the attach() streaming goroutine once it has @@ -375,13 +376,18 @@ type containerReference struct { LinuxContainerEnvironmentExtensions } -func GetDockerClient(ctx context.Context) (cli client.APIClient, err error) { +func GetDockerClient(ctx context.Context) (client.APIClient, error) { + cli, _, err := newDockerClient(ctx) + return cli, err +} + +func newDockerClient(ctx context.Context) (cli client.APIClient, daemonOS string, err error) { dockerHost := os.Getenv("DOCKER_HOST") if strings.HasPrefix(dockerHost, "ssh://") { var args []string if args, err = sshArgs(dockerHost); err != nil { - return nil, err + return nil, "", err } cli, err = client.New( client.WithHost("http://docker.example.com"), // the dialer below ignores the address @@ -391,16 +397,17 @@ func GetDockerClient(ctx context.Context) (cli client.APIClient, err error) { cli, err = client.New(client.FromEnv) } if err != nil { - return nil, fmt.Errorf("failed to connect to docker daemon: %w", err) + return nil, "", fmt.Errorf("failed to connect to docker daemon: %w", err) } // Best-effort API version negotiation, matching the old client.NegotiateAPIVersion // behaviour. Ping failures here are non-fatal: the connection is exercised again on // the first real call, and the client falls back to the daemon's default API version. - if _, err := cli.Ping(ctx, client.PingOptions{NegotiateAPIVersion: true}); err != nil { + ping, err := cli.Ping(ctx, client.PingOptions{NegotiateAPIVersion: true}) + if err != nil { common.Logger(ctx).Warnf("docker daemon ping during version negotiation failed, continuing: %v", err) } - return cli, nil + return cli, ping.OSType, nil } func GetHostInfo(ctx context.Context) (system.Info, error) { @@ -440,16 +447,32 @@ func RunnerArch(ctx context.Context) string { return info.Architecture } +func (cr *containerReference) targetOS(ctx context.Context) string { + if cr.daemonOS == "" { + cr.daemonOS = "linux" + if cr.cli != nil { + if result, err := cr.cli.Info(ctx, client.InfoOptions{}); err == nil && result.Info.OSType != "" { + cr.daemonOS = result.Info.OSType + } + } + } + return cr.daemonOS +} + +func (cr *containerReference) markExecutable(ctx context.Context) bool { + return hostOS == "windows" && cr.targetOS(ctx) == "linux" +} + func (cr *containerReference) connect() common.Executor { return func(ctx context.Context) error { if cr.cli != nil { return nil } - cli, err := GetDockerClient(ctx) + cli, daemonOS, err := newDockerClient(ctx) if err != nil { return err } - cr.cli = cli + cr.cli, cr.daemonOS = cli, daemonOS return nil } } @@ -459,6 +482,7 @@ func (cr *containerReference) Close() common.Executor { if cr.cli != nil { err := cr.cli.Close() cr.cli = nil + cr.daemonOS = "" if err != nil { return fmt.Errorf("failed to close client: %w", err) } @@ -603,7 +627,8 @@ func (cr *containerReference) mergeContainerConfigs(ctx context.Context, config copts.privileged = false } - containerConfig, err := parse(flags, copts, runtime.GOOS) + serverOS := cr.targetOS(ctx) + containerConfig, err := parse(flags, copts, serverOS) if err != nil { return fmt.Errorf("cannot process container options: '%s': '%w'", options, err) } @@ -618,7 +643,7 @@ func (cr *containerReference) mergeContainerConfigs(ctx context.Context, config // For Gitea, forcing --privileged off is not enough, other options reach the host too if !hostConfig.Privileged { - trusted, err := parseOptionsHostConfig(cr.input.RunnerOptions) + trusted, err := parseOptionsHostConfig(cr.input.RunnerOptions, serverOS) if err != nil { return err } @@ -731,9 +756,7 @@ func (cr *containerReference) create(capAdd, capDrop []string) common.Executor { cr.sanitizeConfig(ctx, hostConfig) var networkingConfig *network.NetworkingConfig - n := hostConfig.NetworkMode - // IsUserDefined and IsHost are broken on windows - if n.IsUserDefined() && n != "host" && len(input.NetworkAliases) > 0 { + if isUserDefinedNetwork(hostConfig.NetworkMode) && len(input.NetworkAliases) > 0 { networkingConfig = &network.NetworkingConfig{ EndpointsConfig: map[string]*network.EndpointSettings{ input.NetworkMode: {Aliases: input.NetworkAliases}, @@ -919,7 +942,7 @@ func (cr *containerReference) copyDir(dstPath, srcPath string, useGitIgnore, ski if actPath := cr.GetActPath(); strings.HasPrefix(dstPath+"/", actPath+"/") { extractionRoot = actPath // keeps tar entries from crossing the image's /var/run symlink } - if err := writeDirTar(ctx, tarFile, path.Join("/", strings.TrimPrefix(dstPath, extractionRoot)), srcPath, useGitIgnore, skipGitDir, cr.UID, cr.GID); err != nil { + if err := writeDirTar(ctx, tarFile, path.Join("/", strings.TrimPrefix(dstPath, extractionRoot)), srcPath, useGitIgnore, skipGitDir, cr.UID, cr.GID, cr.markExecutable(ctx)); err != nil { return err } @@ -1090,12 +1113,12 @@ func resetOption[T any](logger *logging.Logger, option string, field *T, trusted // parseOptionsHostConfig parses one options string on its own, to see what it alone asks for. // Even "" goes through the parser, or its empty slices and maps would differ from a real parse. -func parseOptionsHostConfig(options string) (*container.HostConfig, error) { +func parseOptionsHostConfig(options, serverOS string) (*container.HostConfig, error) { flags, copts, _, err := parseContainerOptions(options) if err != nil { return nil, err } - containerConfig, err := parse(flags, copts, runtime.GOOS) + containerConfig, err := parse(flags, copts, serverOS) if err != nil { return nil, fmt.Errorf("cannot process container options: '%s': '%w'", options, err) } diff --git a/act/container/docker_run_test.go b/act/container/docker_run_test.go index 4633ecc4..635b1e34 100644 --- a/act/container/docker_run_test.go +++ b/act/container/docker_run_test.go @@ -27,10 +27,12 @@ import ( "gitea.com/gitea/runner/act/common/logging/logtest" cerrdefs "github.com/containerd/errdefs" + "github.com/kballard/go-shellquote" "github.com/moby/moby/api/pkg/stdcopy" "github.com/moby/moby/api/types/container" "github.com/moby/moby/api/types/mount" "github.com/moby/moby/api/types/network" + "github.com/moby/moby/api/types/system" mobyclient "github.com/moby/moby/client" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/mock" @@ -65,6 +67,14 @@ func TestDockerEnvironmentUsesContainerPath(t *testing.T) { type mockDockerClient struct { mobyclient.APIClient mock.Mock + info system.Info + infoErr error + infoCalls int +} + +func (m *mockDockerClient) Info(_ctx context.Context, _opts mobyclient.InfoOptions) (mobyclient.SystemInfoResult, error) { + m.infoCalls++ + return mobyclient.SystemInfoResult{Info: m.info}, m.infoErr } func (m *mockDockerClient) ServerVersion(ctx context.Context, opts mobyclient.ServerVersionOptions) (mobyclient.ServerVersionResult, error) { @@ -689,11 +699,11 @@ func TestMergeContainerConfigsDropsVolumeDriversFromWorkflows(t *testing.T) { } func TestMergeContainerConfigsKeepsTheRunnersFilesAndEnvToItself(t *testing.T) { - hostFile := filepath.Join(t.TempDir(), "host.env") + hostFile := filepath.Join(t.TempDir(), "host file.env") require.NoError(t, os.WriteFile(hostFile, []byte("STOLEN=from-the-host\n"), 0o600)) t.Setenv("RUNNER_SECRET", "s3cr3t") - for _, option := range []string{"--env-file " + hostFile, "--label-file " + hostFile} { + for _, option := range []string{shellquote.Join("--env-file", hostFile), shellquote.Join("--label-file", hostFile)} { logger := logging.New(logtest.New()) cr := &containerReference{input: &NewContainerInput{NetworkMode: "bridge", WorkflowOptions: option}} @@ -933,6 +943,7 @@ func TestContainerInfoFromInspect(t *testing.T) { "/etc/hosts": "", "/etc/hostname": "", }, info.Mounts) + assert.Equal(t, "/var/lib/docker/volumes/job/_data/subdir", info.DaemonPath("/workspace/owner/repo/subdir")) }) } @@ -998,6 +1009,39 @@ func TestDockerCreateKeepsInputEntrypoint(t *testing.T) { } } +func TestDockerCreateNetworkAliases(t *testing.T) { + for _, test := range []struct { + networkMode string + wantAliases bool + }{ + {"job-network", true}, + {"host", false}, + {"bridge", false}, + {"default", false}, + {"none", false}, + {"container:x", false}, + } { + t.Run(test.networkMode, func(t *testing.T) { + cr := &containerReference{ + input: &NewContainerInput{NetworkMode: test.networkMode, NetworkAliases: []string{"service"}}, + cli: &probeClient{create: func(_ctx context.Context, opts mobyclient.ContainerCreateOptions) (mobyclient.ContainerCreateResult, error) { + assert.Equal(t, container.NetworkMode(test.networkMode), opts.HostConfig.NetworkMode) + if test.wantAliases { + assert.Equal(t, &network.NetworkingConfig{EndpointsConfig: map[string]*network.EndpointSettings{ + "job-network": {Aliases: []string{"service"}}, + }}, opts.NetworkingConfig) + } else { + assert.Nil(t, opts.NetworkingConfig) + } + return mobyclient.ContainerCreateResult{ID: "created"}, nil + }}, + } + require.NoError(t, cr.create(nil, nil)(t.Context())) + assert.Equal(t, "created", cr.id) + }) + } +} + func TestMergeContainerConfigsKeepsNetworkAliasesFromOptions(t *testing.T) { logger := logging.New(logtest.New()) cr := &containerReference{input: &NewContainerInput{ @@ -1019,3 +1063,92 @@ func TestSupportsContainerImagePlatformDaemonError(t *testing.T) { require.ErrorContains(t, err, "cannot connect to the Docker daemon") assert.False(t, supported) } + +func TestContainerOptionsUseDaemonOS(t *testing.T) { + for _, testcase := range []struct { + name string + serverOS string + infoErr error + device string + expected container.DeviceMapping + }{ + {name: "Linux daemon", serverOS: "linux", device: "/dev/kvm:/dev/kvm:rwm", expected: container.DeviceMapping{PathOnHost: "/dev/kvm", PathInContainer: "/dev/kvm", CgroupPermissions: "rwm"}}, + {name: "unknown daemon", device: "/dev/kvm:/dev/kvm:rwm", expected: container.DeviceMapping{PathOnHost: "/dev/kvm", PathInContainer: "/dev/kvm", CgroupPermissions: "rwm"}}, + {name: "unavailable daemon info", infoErr: errors.New("unavailable"), device: "/dev/kvm:/dev/kvm:rwm", expected: container.DeviceMapping{PathOnHost: "/dev/kvm", PathInContainer: "/dev/kvm", CgroupPermissions: "rwm"}}, + {name: "Windows daemon", serverOS: "windows", device: "class/01234567-89ab-cdef-0123-456789abcdef", expected: container.DeviceMapping{PathOnHost: "class/01234567-89ab-cdef-0123-456789abcdef"}}, + } { + t.Run(testcase.name, func(t *testing.T) { + cli := &mockDockerClient{info: system.Info{OSType: testcase.serverOS}, infoErr: testcase.infoErr} + cr := &containerReference{cli: cli, input: &NewContainerInput{RunnerOptions: "--device=" + testcase.device, WorkflowOptions: "--device=" + testcase.device, NetworkMode: "bridge"}} + for range 2 { + hostConfig := &container.HostConfig{} + require.NoError(t, cr.mergeContainerConfigs(t.Context(), &container.Config{}, hostConfig)) + assert.Equal(t, []container.DeviceMapping{testcase.expected}, hostConfig.Devices) + } + assert.Equal(t, 1, cli.infoCalls) + }) + } +} + +func TestConnectTakesDaemonOSFromPing(t *testing.T) { + var infos atomic.Int32 + daemon := httptest.NewServer(http.HandlerFunc(func(writer http.ResponseWriter, request *http.Request) { + writer.Header().Set("API-Version", "1.47") + writer.Header().Set("OSType", "windows") + if strings.HasSuffix(request.URL.Path, "/info") { + infos.Add(1) + } + })) + t.Cleanup(daemon.Close) + t.Setenv("DOCKER_HOST", daemon.URL) + cr := &containerReference{input: &NewContainerInput{}} + require.NoError(t, cr.connect()(t.Context())) + assert.Equal(t, "windows", cr.targetOS(t.Context())) + assert.Zero(t, infos.Load()) +} + +func TestContainerCopyModes(t *testing.T) { + for _, testcase := range []struct { + hostOS, serverOS string + expected int64 + }{ + {"windows", "linux", 0o755}, + {"windows", "windows", 0o666}, + {"linux", "linux", 0o666}, + } { + t.Run(testcase.hostOS+"-"+testcase.serverOS, func(t *testing.T) { + source := t.TempDir() + require.NoError(t, os.WriteFile(filepath.Join(source, "run.sh"), []byte("#!/bin/sh\n"), 0o666)) + require.NoError(t, os.Chmod(filepath.Join(source, "run.sh"), 0o666)) + file := &FileEntry{Name: "run.sh", Mode: 0o600, Body: "#!/bin/sh\n"} + original := hostOS + hostOS = testcase.hostOS + t.Cleanup(func() { hostOS = original }) + cli := &mockDockerClient{info: system.Info{OSType: testcase.serverOS}} + var modes []int64 + cli.On("CopyToContainer", mock.Anything, "job", mock.Anything). + Run(func(args mock.Arguments) { + reader := tar.NewReader(args.Get(2).(mobyclient.CopyToContainerOptions).Content) + header, err := reader.Next() + require.NoError(t, err) + modes = append(modes, header.Mode) + assert.Equal(t, "run.sh", filepath.Base(header.Name)) + content, err := io.ReadAll(reader) + require.NoError(t, err) + assert.Equal(t, file.Body, string(content)) + _, err = reader.Next() + require.ErrorIs(t, err, io.EOF) + }).Return(mobyclient.CopyToContainerResult{}, nil).Twice() + cr := &containerReference{cli: cli, id: "job", input: &NewContainerInput{}} + require.NoError(t, cr.copyContent("/workspace", file)(t.Context())) + require.NoError(t, cr.copyDir("/workspace", source+string(filepath.Separator), false, true)(t.Context())) + assert.Equal(t, []int64{0o600, testcase.expected}, modes) + if testcase.hostOS == "windows" { + assert.Equal(t, 1, cli.infoCalls) + } else { + assert.Zero(t, cli.infoCalls) + } + cli.AssertExpectations(t) + }) + } +} diff --git a/act/container/executions_environment.go b/act/container/executions_environment.go index 9023018a..e34a0156 100644 --- a/act/container/executions_environment.go +++ b/act/container/executions_environment.go @@ -4,7 +4,18 @@ package container -import "context" +import ( + "context" + "path" + "strings" +) + +func JoinPath(elem ...string) string { // path.Join would collapse the leading // of a UNC act path on a Windows host + if len(elem) > 0 && strings.HasPrefix(elem[0], "//") { + return "/" + path.Join(elem...) + } + return path.Join(elem...) +} type ExecutionsEnvironment interface { Container diff --git a/act/container/host_environment.go b/act/container/host_environment.go index 8ed565f9..725a36b7 100644 --- a/act/container/host_environment.go +++ b/act/container/host_environment.go @@ -40,7 +40,8 @@ type HostEnvironment struct { // when Workdir points at a caller-owned checkout (e.g. `act` local mode). CleanWorkdir bool ActPath string - CleanUp func() + Root string + TrackingID string // RUNNER_TRACKING_ID of the job, a step with another value keeps its leftovers StdOut io.Writer AllocatePTY bool // allocate a pseudo-TTY for each step's process @@ -261,6 +262,7 @@ func (e *HostEnvironment) exec(ctx context.Context, command []string, cmdline st // Flush any buffered, not-yet-newline-terminated trailing line, as the docker backend // does in waitForCommand, so the final line of a command's output is not lost. defer common.FlushWriter(e.StdOut) + process.UTF8Console() f, err := lookupPathHost(command[0], env, e.StdOut) if err != nil { @@ -270,7 +272,12 @@ func (e *HostEnvironment) exec(ctx context.Context, command []string, cmdline st cmd.Path = f cmd.Args = command cmd.Stdout = e.StdOut - cmd.Env = getEnvListFromMap(env) + trackingID := e.TrackingID + if value, ok := common.LookupFold(env, "RUNNER_TRACKING_ID", e.IsEnvironmentCaseInsensitive()); ok { + trackingID = value + } + tracked := trackingID == e.TrackingID + cmd.Env = append(getEnvListFromMap(env), "RUNNER_TRACKING_ID="+trackingID) cmd.Stderr = e.StdOut cmd.Dir = wd cmd.SysProcAttr = process.SysProcAttr(cmdline, false) @@ -278,6 +285,7 @@ func (e *HostEnvironment) exec(ctx context.Context, command []string, cmdline st // Kills the step's whole tree on cancellation and bounds the post-exit I/O // wait, so an orphan holding cmd's stdout pipe cannot hang cmd.Wait(). treeKill := process.NewTreeKill(cmd) + treeKill.KeepLeftovers = !tracked var ppty *os.File var tty *os.File @@ -311,10 +319,10 @@ func (e *HostEnvironment) exec(ctx context.Context, command []string, cmdline st if err := cmd.Start(); err != nil { return err } - // Assign before the step's Killer so the step's job nests inside the group's; - // cancellation still scopes to this step's tree. - if err := e.processGroup(ctx).Assign(cmd.Process); err != nil { - common.Logger(ctx).Warnf("could not assign the step's process to the job's process group; a process it leaves behind may outlive the job: %v", err) + if tracked { + if err := e.processGroup(ctx).Assign(cmd.Process); err != nil { + common.Logger(ctx).Warnf("could not assign the step's process to the job's process group, a process it leaves behind may outlive the job: %v", err) + } } if k, kerr := treeKill.Capture(cmd.Process); kerr != nil { common.Logger(ctx).Warnf("process tree kill setup failed, falling back to single-process kill: %v", kerr) @@ -365,9 +373,7 @@ func (e *HostEnvironment) UpdateFromEnv(srcPath string, env *map[string]string) // removeAll is a var so tests can substitute a blocking stub. var removeAll = os.RemoveAll -// removeAllWithContext returns once the delete finishes or ctx is cancelled. On -// cancellation the goroutine leaks: a delete inside a syscall cannot be -// interrupted (see runWithTimeout). +// Returns on cancel while an uninterruptible delete keeps running, freeing the runner's capacity slot. func removeAllWithContext(ctx context.Context, path string) error { done := make(chan error, 1) go func() { done <- removeAll(path) }() @@ -409,35 +415,6 @@ func removePathWithRetry(ctx context.Context, path string) error { return lastErr } -// buildWindowsWorkspaceKillScript builds a PowerShell command that taskkills -// every process tree whose ExecutablePath or CommandLine references one of the -// given workspace dirs, releasing file handles for cleanup. Win32_Process -// exposes both fields (Get-Process doesn't, wmic is deprecated); matching is on -// the dir+separator prefix via ordinal String methods, so a name-prefix sibling -// (job1 vs job10) is spared and path metacharacters stay literal. -func buildWindowsWorkspaceKillScript(dirs []string) string { - quoted := make([]string, len(dirs)) - for i, d := range dirs { - // Single-quoted PowerShell literal; escape ' by doubling it. - quoted[i] = "'" + strings.ReplaceAll(d, "'", "''") + "'" - } - - return `$paths = @(` + strings.Join(quoted, ",") + `) -$selfPid = $PID -Get-CimInstance Win32_Process -ErrorAction SilentlyContinue | Where-Object { - if ($_.ProcessId -eq $selfPid) { return $false } - foreach ($p in $paths) { - $prefix = $p + '\' - if ($_.ExecutablePath -and $_.ExecutablePath.StartsWith($prefix, [System.StringComparison]::OrdinalIgnoreCase)) { return $true } - if ($_.CommandLine -and $_.CommandLine.IndexOf($prefix, [System.StringComparison]::OrdinalIgnoreCase) -ge 0) { return $true } - } - return $false -} | ForEach-Object { - & taskkill.exe /PID $_.ProcessId /T /F 2>$null | Out-Null -} -` -} - func (e *HostEnvironment) terminateRunningProcesses(ctx context.Context) { if runtime.GOOS != "windows" { return @@ -471,20 +448,10 @@ func (e *HostEnvironment) terminateRunningProcesses(ctx context.Context) { return } - script := buildWindowsWorkspaceKillScript(dirs) - - cmd := exec.CommandContext(killCtx, "powershell.exe", "-NoProfile", "-NonInteractive", "-Command", script) - out, err := cmd.CombinedOutput() - if err != nil { - logger.Debugf("workspace process-tree kill via PowerShell failed: %v output=%s", err, strings.TrimSpace(string(out))) - } - - // Win32_Process exposes no working directory, so the scan above misses a - // process that merely runs in a workspace dir while pinning a handle on it. - if killed, err := process.KillProcessesWithCWDUnder(killCtx, dirs); err != nil { - logger.Debugf("workspace process kill by working directory reported errors: %v", err) + if killed, err := process.KillProcessesWithPaths(killCtx, dirs, e.TrackingID); err != nil { + logger.Debugf("workspace process cleanup reported errors: %v", err) } else if killed > 0 { - logger.Debugf("terminated %d leftover process(es) by workspace working directory", killed) + logger.Debugf("terminated %d leftover workspace process(es)", killed) } } @@ -492,26 +459,6 @@ func (e *HostEnvironment) terminateRunningProcesses(ctx context.Context) { // wedge the runner slot. A var so tests can shrink it. var hostCleanupTimeout = 30 * time.Second -// runWithTimeout returns context.DeadlineExceeded once timeout elapses, leaking -// the goroutine: a delete blocked in a syscall (AV filter driver, dead network -// mount) cannot be interrupted, and leaking scratch state beats losing the -// runner's capacity slot forever. The idle stale-dir sweep reclaims it later. -func runWithTimeout(fn func(), timeout time.Duration) error { - done := make(chan struct{}) - go func() { - defer close(done) - fn() - }() - timer := time.NewTimer(timeout) - defer timer.Stop() - select { - case <-done: - return nil - case <-timer.C: - return context.DeadlineExceeded - } -} - func (e *HostEnvironment) Remove() common.Executor { return func(ctx context.Context) error { logger := common.Logger(ctx) @@ -523,25 +470,14 @@ func (e *HostEnvironment) Remove() common.Executor { } e.terminateRunningProcesses(ctx) - // Removes per-job misc state only, never the toolcache root. Bounded because - // CleanUp is a caller-supplied, typically unbounded os.RemoveAll. - if e.CleanUp != nil { - logger.Debugf("running host environment cleanup callback") - if err := runWithTimeout(e.CleanUp, hostCleanupTimeout); err != nil { - logger.Warnf("host environment cleanup did not finish within %s; continuing job completion, scratch state may be leaked and is reclaimed by the idle stale-dir sweep", hostCleanupTimeout) - } else { - logger.Debugf("host environment cleanup callback finished") - } - } - // Detach: a cancelled ctx would skip removePathWithRetry's retries, // which absorb Windows file-handle release lag after the kill above. rmCtx, rmCancel := context.WithTimeout(context.Background(), hostCleanupTimeout) defer rmCancel() var errs []error - if err := removePathWithRetry(rmCtx, e.Path); err != nil { - logger.Warnf("failed to remove host misc state %s: %v", e.Path, err) + if err := removePathWithRetry(rmCtx, cmp.Or(e.Root, e.Path)); err != nil { + logger.Warnf("failed to remove host misc state %s: %v", cmp.Or(e.Root, e.Path), err) errs = append(errs, err) } if e.CleanWorkdir { diff --git a/act/container/host_environment_test.go b/act/container/host_environment_test.go index 177b4d4b..a3d244b0 100644 --- a/act/container/host_environment_test.go +++ b/act/container/host_environment_test.go @@ -161,10 +161,12 @@ func TestHostEnvironmentRemoveWorkdir(t *testing.T) { Path: filepath.Join(miscRoot, "hostexecutor"), Workdir: workdir, CleanWorkdir: owned, - CleanUp: func() { _ = os.RemoveAll(miscRoot) }, + Root: miscRoot, } require.NoError(t, os.MkdirAll(e.Path, 0o700)) + require.NoError(t, os.MkdirAll(filepath.Join(miscRoot, "tmp"), 0o700)) require.NoError(t, e.Remove()(t.Context())) + assert.NoDirExists(t, miscRoot) _, err := os.Stat(workdir) if owned { assert.ErrorIs(t, err, os.ErrNotExist) @@ -201,87 +203,38 @@ func TestRemoveAllWithContextDoesNotHangOnStuckDelete(t *testing.T) { } func TestHostEnvironmentRemoveDoesNotHang(t *testing.T) { - for _, phase := range []string{"CleanUp callback", "path removal"} { - t.Run(phase, func(t *testing.T) { - synctest.Test(t, func(t *testing.T) { - origTimeout := hostCleanupTimeout - hostCleanupTimeout = 100 * time.Millisecond - t.Cleanup(func() { hostCleanupTimeout = origTimeout }) - release, stubDone := make(chan struct{}), make(chan struct{}) - block := func() { - defer close(stubDone) - <-release - } - e := &HostEnvironment{Path: filepath.Join(t.TempDir(), "misc", "hostexecutor"), StdOut: os.Stdout} - if phase == "CleanUp callback" { - e.CleanUp = block - } else { - origRemoveAll := removeAll - removeAll = func(string) error { - block() - return nil - } - t.Cleanup(func() { removeAll = origRemoveAll }) - } - t.Cleanup(func() { - close(release) - <-stubDone - }) - require.NoError(t, os.MkdirAll(e.Path, 0o700)) - done := make(chan error, 1) - go func() { done <- e.Remove()(common.WithLogger(t.Context(), logging.Default())) }() - select { - case err := <-done: - require.NoError(t, err) - case <-time.After(10 * time.Second): - t.Fatalf("Remove() hung on a stuck %s", phase) - } - }) + synctest.Test(t, func(t *testing.T) { + origTimeout, origRemoveAll := hostCleanupTimeout, removeAll + hostCleanupTimeout = 100 * time.Millisecond + release, stubDone := make(chan struct{}), make(chan struct{}) + removeAll = func(string) error { + defer close(stubDone) + <-release + return nil + } + t.Cleanup(func() { + close(release) + <-stubDone + hostCleanupTimeout, removeAll = origTimeout, origRemoveAll }) + e := &HostEnvironment{Path: filepath.Join(t.TempDir(), "misc", "hostexecutor"), StdOut: os.Stdout} + require.NoError(t, os.MkdirAll(e.Path, 0o700)) + done := make(chan error, 1) + go func() { done <- e.Remove()(common.WithLogger(t.Context(), logging.Default())) }() + select { + case err := <-done: + require.NoError(t, err) + case <-time.After(10 * time.Second): + t.Fatal("Remove() hung on a stuck path removal") + } + }) +} + +func TestJoinPath(t *testing.T) { + assert.Equal(t, "//server/share/act/workflow/envs.txt", JoinPath("//server/share/act", "workflow/envs.txt")) + assert.Equal(t, "/var/run/act/workflow/envs.txt", JoinPath("/var/run/act/", "workflow", "envs.txt")) + for _, root := range []string{"//server/share/act", "/var/run/act", "C:/act"} { + assert.Equal(t, root+"/actions/key/main.go.out", JoinPath(root+"/actions/key", "main.go.out")) + assert.Equal(t, root+"/workflow", JoinPath(root+"/workflow/outputcmd.txt", "..")) } } - -func TestBuildWindowsWorkspaceKillScript(t *testing.T) { - t.Run("single dir", func(t *testing.T) { - s := buildWindowsWorkspaceKillScript([]string{`C:\workspace\job1`}) - assert.Contains(t, s, `$paths = @('C:\workspace\job1')`) - assert.Contains(t, s, "$selfPid = $PID") - assert.Contains(t, s, "$_.ProcessId -eq $selfPid") - assert.Contains(t, s, `$prefix = $p + '\'`) - assert.Contains(t, s, "$_.ExecutablePath.StartsWith($prefix") - assert.Contains(t, s, "$_.CommandLine.IndexOf($prefix") - assert.Contains(t, s, "taskkill.exe /PID $_.ProcessId /T /F") - }) - - t.Run("multiple dirs comma-separated", func(t *testing.T) { - s := buildWindowsWorkspaceKillScript([]string{ - `C:\work\path`, - `C:\work\workdir`, - `C:\Users\runner\AppData\Local\Temp\job-42`, - }) - assert.Contains(t, s, `'C:\work\path'`) - assert.Contains(t, s, `'C:\work\workdir'`) - assert.Contains(t, s, `'C:\Users\runner\AppData\Local\Temp\job-42'`) - assert.Contains(t, s, `'C:\work\path','C:\work\workdir',`) - }) - - t.Run("path with single quote is escaped", func(t *testing.T) { - s := buildWindowsWorkspaceKillScript([]string{`C:\work\it's\path`}) - assert.Contains(t, s, `'C:\work\it''s\path'`) - assert.NotContains(t, s, `'C:\work\it's\path'`) - }) - - t.Run("path with wildcard metacharacters is matched literally", func(t *testing.T) { - s := buildWindowsWorkspaceKillScript([]string{`C:\work\[job]?1`}) - assert.Contains(t, s, `'C:\work\[job]?1'`) - assert.NotContains(t, s, "-like") - assert.Contains(t, s, "StartsWith") - assert.Contains(t, s, "IndexOf") - }) - - t.Run("empty dir list still produces a valid script", func(t *testing.T) { - s := buildWindowsWorkspaceKillScript(nil) - assert.Contains(t, s, "$paths = @()") - assert.Contains(t, s, "Get-CimInstance Win32_Process") - }) -} diff --git a/act/container/host_environment_windows_test.go b/act/container/host_environment_windows_test.go new file mode 100644 index 00000000..bf5b3f68 --- /dev/null +++ b/act/container/host_environment_windows_test.go @@ -0,0 +1,138 @@ +// Copyright 2026 The Gitea Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package container + +import ( + "bufio" + "context" + "fmt" + "io" + "net" + "os" + "os/exec" + "path/filepath" + "strings" + "testing" + "time" + "unsafe" + + "github.com/stretchr/testify/require" + "golang.org/x/sys/windows" +) + +func TestHostEnvironmentTrackingCleanup(t *testing.T) { + for _, tracking := range []string{"default", "", "shared-helper"} { + t.Run("tracking="+tracking, func(t *testing.T) { + ctx, cancel := context.WithTimeout(t.Context(), 30*time.Second) + defer cancel() + listener, err := net.Listen("tcp4", "127.0.0.1:0") + require.NoError(t, err) + defer listener.Close() + stop := context.AfterFunc(ctx, func() { _ = listener.Close() }) + defer stop() + workspace := t.TempDir() + environment := &HostEnvironment{Path: t.TempDir(), TmpDir: workspace, Workdir: workspace, TrackingID: "gitea_test", StdOut: io.Discard} + env := map[string]string{} + for _, entry := range os.Environ() { + if name, value, ok := strings.Cut(entry, "="); ok && !strings.EqualFold(name, "RUNNER_TRACKING_ID") { + env[name] = value + } + } + env["GITEA_TRACKING_HELPER"] = "step" + env["GITEA_TRACKING_WORKSPACE"] = workspace + env["GITEA_TRACKING_ADDRESS"] = listener.Addr().String() + if tracking == "default" { + env["RUNNER_TRACKING_ID"] = environment.TrackingID + env["runner_tracking_id"] = "shared-helper" + } else { + env["runner_tracking_id"] = tracking + } + result := make(chan error, 1) + go func() { + result <- environment.Exec([]string{os.Args[0], "-test.run=^TestHostTrackingHelper$"}, env, "", workspace)(ctx) + }() + step, err := listener.Accept() + require.NoError(t, err) + defer step.Close() + require.NoError(t, step.SetDeadline(time.Now().Add(20*time.Second))) + var stepPID int + _, err = fmt.Fscanln(step, &stepPID) + require.NoError(t, err) + stepHandle, err := windows.OpenProcess(windows.PROCESS_QUERY_LIMITED_INFORMATION, false, uint32(stepPID)) + require.NoError(t, err) + defer func() { _ = windows.CloseHandle(stepHandle) }() + require.Eventually(t, func() bool { + var assigned int32 + result, _, _ := windows.NewLazySystemDLL("kernel32.dll").NewProc("IsProcessInJob").Call(uintptr(stepHandle), 0, uintptr(unsafe.Pointer(&assigned))) + return result != 0 && assigned != 0 + }, 5*time.Second, 10*time.Millisecond) + _, err = step.Write([]byte{1}) + require.NoError(t, err) + var helpers []windows.Handle + for range 2 { + connection, err := listener.Accept() + require.NoError(t, err) + t.Cleanup(func() { _ = connection.Close() }) + require.NoError(t, connection.SetReadDeadline(time.Now().Add(20*time.Second))) + var pid int + _, err = fmt.Fscanln(connection, &pid) + require.NoError(t, err) + handle, err := windows.OpenProcess(windows.SYNCHRONIZE|windows.PROCESS_TERMINATE, false, uint32(pid)) + require.NoError(t, err) + t.Cleanup(func() { + _ = windows.TerminateProcess(handle, 1) + _, _ = windows.WaitForSingleObject(handle, 5000) + _ = windows.CloseHandle(handle) + }) + helpers = append(helpers, handle) + } + select { + case err := <-result: + require.NoError(t, err) + case <-ctx.Done(): + t.Fatal(ctx.Err()) + } + require.NoError(t, environment.Remove()(ctx)) + for _, handle := range helpers { + if tracking == "default" { + result, err := windows.WaitForSingleObject(handle, 5000) + require.NoError(t, err) + require.EqualValues(t, windows.WAIT_OBJECT_0, result) + } else { + result, err := windows.WaitForSingleObject(handle, 0) + require.NoError(t, err) + require.EqualValues(t, windows.WAIT_TIMEOUT, result) + } + } + }) + } +} + +func TestHostTrackingHelper(t *testing.T) { + mode := os.Getenv("GITEA_TRACKING_HELPER") + if mode == "" { + return + } + connection, err := net.Dial("tcp4", os.Getenv("GITEA_TRACKING_ADDRESS")) + require.NoError(t, err) + _, err = fmt.Fprintln(connection, os.Getpid()) + require.NoError(t, err) + if mode == "step" { + _, err := bufio.NewReader(connection).ReadByte() + require.NoError(t, err) + } + if mode != "descendant" { + child := exec.Command(os.Args[0], "-test.run=^TestHostTrackingHelper$", filepath.Join(os.Getenv("GITEA_TRACKING_WORKSPACE"), "tracking-reference")) + child.Env = append(os.Environ(), "GITEA_TRACKING_HELPER="+map[string]string{"step": "helper", "helper": "descendant"}[mode]) + require.NoError(t, child.Start()) + if mode == "step" { + require.NoError(t, connection.Close()) + os.Exit(0) + } + } + _, err = io.Copy(io.Discard, connection) + require.NoError(t, err) + require.NoError(t, connection.Close()) + os.Exit(0) +} diff --git a/act/container/kubernetes.go b/act/container/kubernetes.go index 401a0253..a7953d42 100644 --- a/act/container/kubernetes.go +++ b/act/container/kubernetes.go @@ -557,7 +557,7 @@ func (c *kubernetesContainer) Copy(destPath string, files ...*FileEntry) common. func (c *kubernetesContainer) CopyDir(destPath, srcPath string, useGitIgnore, skipGitDir bool) common.Executor { return func(ctx context.Context) error { return c.extract(ctx, destPath, true, func(writer io.Writer) error { // directories are actions and workspaces, megabytes of compressible sources - return writeDirTar(ctx, writer, "/", srcPath, useGitIgnore, skipGitDir, 0, 0) + return writeDirTar(ctx, writer, "/", srcPath, useGitIgnore, skipGitDir, 0, 0, hostOS == "windows") }) } } diff --git a/act/container/parse_env_file.go b/act/container/parse_env_file.go index ee02fba9..a80dc698 100644 --- a/act/container/parse_env_file.go +++ b/act/container/parse_env_file.go @@ -21,7 +21,12 @@ import ( // 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 ParseEnvFile(e, srcPath, func(name, value string) { + (*env)[name] = value + }, windows) +} + +func ParseEnvFile(e Container, srcPath string, setEnv func(name, value string), windows bool) common.Executor { return func(ctx context.Context) error { envTar, err := e.GetContainerArchive(ctx, srcPath) if err != nil { @@ -64,7 +69,7 @@ func parseEnvFile(e Container, srcPath string, env *map[string]string, windows b multiLineEnv := strings.Index(line, "<<") switch { case singleLineEnv != -1 && (multiLineEnv == -1 || singleLineEnv < multiLineEnv): - localEnv[line[:singleLineEnv]] = line[singleLineEnv+1:] + setEnv(line[:singleLineEnv], line[singleLineEnv+1:]) case multiLineEnv != -1: var multiLineEnvContent []string multiLineEnvDelimiter := line[multiLineEnv+2:] @@ -86,7 +91,7 @@ func parseEnvFile(e Container, srcPath string, env *map[string]string, windows b if !delimiterFound { return fmt.Errorf("invalid format delimiter '%v' not found before end of file", multiLineEnvDelimiter) } - localEnv[line[:multiLineEnv]] = trimEnvLineEnding(strings.Join(multiLineEnvContent, ""), windows) + setEnv(line[:multiLineEnv], trimEnvLineEnding(strings.Join(multiLineEnvContent, ""), windows)) default: return fmt.Errorf("invalid format '%v', expected a line with '=' or '<<'", line) } diff --git a/act/container/parse_env_file_test.go b/act/container/parse_env_file_test.go index e28974fa..e011bdb1 100644 --- a/act/container/parse_env_file_test.go +++ b/act/container/parse_env_file_test.go @@ -34,6 +34,7 @@ func TestParseEnvFile(t *testing.T) { want map[string]string }{ {"single line", "FOO=bar\nBAZ=qux\n", map[string]string{"FOO": "bar", "BAZ": "qux"}}, + {"repeated names", "FOO=first\nfoo=middle\nFOO=last\n", map[string]string{"FOO": "last", "foo": "middle"}}, {"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"}}, @@ -57,6 +58,17 @@ func TestParseEnvFile(t *testing.T) { assert.Equal(t, test.want, env) }) } + t.Run("ordered entries preserve repeated names", func(t *testing.T) { + hostEnv, envPath := newTestHostEnv(t) + require.NoError(t, os.WriteFile(envPath, []byte("FOO=first\r\nfoo< limit + if split { // the read can end inside a secret, so keep only what masks alike without the lookahead + unsplit := masker.Replace(string(body[:maxJobSummaryBytes])) + kept := 0 + for kept < len(unsplit) && kept < len(masked) && unsplit[kept] == masked[kept] { + kept++ + } + for kept > 0 && kept < len(masked) && !utf8.RuneStart(masked[kept]) { + kept-- + } + masked = masked[:kept] + } + body = truncateJobSummary(ctx, []byte(masked), summaryPath, maxJobSummaryBytes, split) + uploadJobSummary(ctx, client, base+strconv.Itoa(i)+"/summary", runtimeToken, body) } } @@ -690,26 +713,26 @@ func readSingleFileFromContainerArchive(ctx context.Context, env container.Execu if entryName != requestedPath && entryName != path.Base(requestedPath) { continue } - // Summaries larger than the limit are truncated rather than dropped, so the - // user still gets the leading content (mirroring how GitHub caps oversized - // step summaries instead of discarding them). Read one extra byte so an - // over-limit file is detected from the actual stream rather than trusting - // header.Size, then cap the returned content at maxBytes. - b, err := io.ReadAll(io.LimitReader(tr, maxBytes+1)) + b, err := io.ReadAll(io.LimitReader(transform.NewReader(tr, unicodeencoding.BOMOverride(unicodeencoding.UTF8.NewDecoder())), maxBytes)) if err != nil { return nil, false } - if int64(len(b)) > maxBytes { - // Reserve room for the marker so the marked-up result still fits in maxBytes. - marker := []byte(jobSummaryTruncationMarker) - keep := max(maxBytes-int64(len(marker)), 0) - b = append(b[:keep], marker...) - common.Logger(ctx).Warnf("job summary truncated: path=%s max=%d", p, maxBytes) - } return b, true } } +func truncateJobSummary(ctx context.Context, body []byte, summaryPath string, maxBytes int, split bool) []byte { + if !split && len(body) <= maxBytes { + return body + } + keep := max(min(len(body), maxBytes-len(jobSummaryTruncationMarker)), 0) + for keep > 0 && keep < len(body) && !utf8.RuneStart(body[keep]) { + keep-- + } + common.Logger(ctx).Warnf("job summary truncated: path=%s max=%d", summaryPath, maxBytes) + return append(body[:keep], jobSummaryTruncationMarker...) +} + // useJobStageLogger tags the log entries of a job level executor with the stage they belong to. func useJobStageLogger(stage string, executor common.Executor) common.Executor { return func(ctx context.Context) error { diff --git a/act/runner/job_executor_test.go b/act/runner/job_executor_test.go index d109c7a9..f5c513aa 100644 --- a/act/runner/job_executor_test.go +++ b/act/runner/job_executor_test.go @@ -34,6 +34,8 @@ import ( "github.com/stretchr/testify/mock" "github.com/stretchr/testify/require" yaml "go.yaml.in/yaml/v4" + "golang.org/x/text/encoding" + "golang.org/x/text/encoding/unicode" ) func TestJobExecutor(t *testing.T) { @@ -932,49 +934,42 @@ func TestExtractJobIDFromRuntimeToken(t *testing.T) { assert.Equal(t, int64(0), extractJobIDFromRuntimeToken("")) } -func TestReadSingleFileFromContainerArchiveFindsMatchingRegularFile(t *testing.T) { - ctx := t.Context() - cm := &containerMock{} - cm.On("GetContainerArchive", ctx, "/var/run/act/workflow/SUMMARY.md").Return( - io.NopCloser(bytes.NewReader(tarArchive(t, - tarEntry{name: "workflow", typeflag: tar.TypeDir}, - tarEntry{name: "other.md", body: "wrong"}, - tarEntry{name: "SUMMARY.md", body: "right"}, - ))), - nil, - ).Once() - - body, ok := readSingleFileFromContainerArchive(ctx, cm, "/var/run/act/workflow/SUMMARY.md", 1024) - - assert.True(t, ok) - assert.Equal(t, []byte("right"), body) - cm.AssertExpectations(t) -} - -func TestReadSingleFileFromContainerArchiveSizeLimit(t *testing.T) { +func TestReadSingleFileFromContainerArchive(t *testing.T) { + retainedBytes := 200 - len(jobSummaryTruncationMarker) for name, tt := range map[string]struct { content string maxBytes int64 want string }{ - "truncates oversized summary": {strings.Repeat("a", 300), 200, strings.Repeat("a", 200-len(jobSummaryTruncationMarker)) + jobSummaryTruncationMarker}, - "keeps exact limit": {"abc", 3, "abc"}, + "truncates oversized summary without reading entire file": {strings.Repeat("a", 100000), 200, strings.Repeat("a", 200-len(jobSummaryTruncationMarker)) + jobSummaryTruncationMarker}, + "keeps exact limit": {"abc", 3, "abc"}, + "strips UTF-8 BOM before limit": {"\xef\xbb\xbfabc", 3, "abc"}, + "decodes UTF-16 before limit": {"\xff\xfea\x00b\x00c\x00", 3, "abc"}, + "limits expanded UTF-16": {"\xff\xfe" + strings.Repeat("\x2d\x4e", 150), 200, strings.Repeat("中", retainedBytes/3) + jobSummaryTruncationMarker}, + "keeps complete UTF-8 characters": {strings.Repeat("a", retainedBytes-1) + "€" + strings.Repeat("a", 200), 200, strings.Repeat("a", retainedBytes-1) + jobSummaryTruncationMarker}, } { t.Run(name, func(t *testing.T) { logger, hook := logtest.NewLogger() ctx := common.WithLogger(t.Context(), logger) cm := &containerMock{} + archive := bytes.NewReader(tarArchive(t, + tarEntry{name: "workflow", typeflag: tar.TypeDir}, + tarEntry{name: "other.md", body: "wrong"}, + tarEntry{name: "SUMMARY.md", body: tt.content}, + )) cm.On("GetContainerArchive", ctx, "/var/run/act/workflow/SUMMARY.md").Return( - io.NopCloser(bytes.NewReader(tarArchive(t, tarEntry{name: "SUMMARY.md", body: tt.content}))), + io.NopCloser(archive), nil, ).Once() - body, ok := readSingleFileFromContainerArchive(ctx, cm, "/var/run/act/workflow/SUMMARY.md", tt.maxBytes) + body, ok := readSingleFileFromContainerArchive(ctx, cm, "/var/run/act/workflow/SUMMARY.md", tt.maxBytes+1) + assert.LessOrEqual(t, int64(len(body)), tt.maxBytes+1) + body = truncateJobSummary(ctx, body, "/var/run/act/workflow/SUMMARY.md", int(tt.maxBytes), false) assert.True(t, ok) assert.Equal(t, []byte(tt.want), body) assert.LessOrEqual(t, int64(len(body)), tt.maxBytes) - if int64(len(tt.content)) > tt.maxBytes { + if strings.HasSuffix(tt.want, jobSummaryTruncationMarker) { if assert.Len(t, hook.AllRecords(), 1) { assert.Contains(t, hook.LastRecord().Message, "job summary truncated") } @@ -1113,35 +1108,75 @@ func TestJobSetContinueOnError(t *testing.T) { } func TestTryUploadJobSummaryMasksSecrets(t *testing.T) { - var got string - server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - body, err := io.ReadAll(r.Body) - assert.NoError(t, err) - got = string(body) - w.WriteHeader(http.StatusNoContent) - })) - defer server.Close() + const prefix = "deployed true with s3cr3t and runtime-added via pr0xypw 😀" + const maskedPrefix = "deployed true with *** and *** via *** 😀" + padding := strings.Repeat("a", maxJobSummaryBytes-len(jobSummaryTruncationMarker)-len(prefix)-2) + content := prefix + padding + "s3cr3t" + strings.Repeat("中", 100) + want := maskedPrefix + padding + "***" + strings.Repeat("中", (len(prefix)-len(maskedPrefix)-1)/3) + jobSummaryTruncationMarker + longSecret := "PREFIX" + strings.Repeat("xy", 100) + "Z" + readLimit := maxJobSummaryBytes + base64.StdEncoding.EncodedLen(len(longSecret)) + 1 + shrinkingPrefix := strings.Repeat(longSecret, (readLimit-len(longSecret)+1)/len(longSecret)) + for name, testcase := range map[string]struct { + encoding encoding.Encoding + content, want string + masks []string + }{ + "UTF-8": {unicode.UTF8, content, want, nil}, + "UTF-8 BOM": {unicode.UTF8BOM, content, want, nil}, + "UTF-16LE BOM": {unicode.UTF16(unicode.LittleEndian, unicode.UseBOM), content, want, nil}, + "UTF-16BE BOM": {unicode.UTF16(unicode.BigEndian, unicode.UseBOM), content, want, nil}, + "secret split by the read": {unicode.UTF8, strings.Repeat("s3cr3t", maxJobSummaryBytes/3), "", nil}, + "split read keeps every masked secret before the limit": { + unicode.UTF8, + "# public\n" + strings.Repeat(longSecret, readLimit/len(longSecret)+1), + "# public\n" + strings.Repeat("***", (maxJobSummaryBytes-len("# public\n"))/len(longSecret)) + jobSummaryTruncationMarker, + []string{longSecret}, + }, + "nested secret expands the split tail": { + unicode.UTF8, + shrinkingPrefix + strings.Repeat("*", readLimit-len(shrinkingPrefix)-len(longSecret)+1) + longSecret, + "", + []string{"xy", longSecret}, + }, + } { + t.Run(name, func(t *testing.T) { + var got string + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + body, err := io.ReadAll(r.Body) + assert.NoError(t, err) + got = string(body) + w.WriteHeader(http.StatusNoContent) + })) + defer server.Close() - cm := &containerMock{} - cm.On("GetContainerArchive", mock.Anything, "/var/run/act/workflow/step-summary-0.md").Return( - io.NopCloser(bytes.NewReader(tarArchive(t, tarEntry{ - name: "step-summary-0.md", body: "deployed true with s3cr3t and runtime-added via pr0xypw", - }))), - nil, - ).Once() + content, err := testcase.encoding.NewEncoder().Bytes([]byte(testcase.content)) + require.NoError(t, err) + cm := &containerMock{} + cm.On("GetContainerArchive", mock.Anything, "/var/run/act/workflow/step-summary-0.md").Return( + io.NopCloser(bytes.NewReader(tarArchive(t, tarEntry{ + name: "step-summary-0.md", body: string(content), + }))), + nil, + ).Once() - rc := newJobSummaryRC(map[string]string{ - "GITEA_ACTIONS_CAPABILITIES": "job-summary", - "ACTIONS_RUNTIME_URL": server.URL, - "ACTIONS_RUNTIME_TOKEN": fakeRuntimeToken(34), - "GITEA_RUN_ID": "12", - }, cm, 1) - rc.Config.Secrets = map[string]string{"TOK": "s3cr3t", "ACTIONS_STEP_DEBUG": "true"} - rc.Config.ExtraMasks = []string{"pr0xypw"} - rc.Masks = []string{"runtime-added"} + rc := newJobSummaryRC(map[string]string{ + "GITEA_ACTIONS_CAPABILITIES": "job-summary", + "ACTIONS_RUNTIME_URL": server.URL, + "ACTIONS_RUNTIME_TOKEN": fakeRuntimeToken(34), + "GITEA_RUN_ID": "12", + }, cm, 1) + rc.Config.Secrets = map[string]string{"TOK": "s3cr3t", "ACTIONS_STEP_DEBUG": "true"} + rc.Config.ExtraMasks = append([]string{"pr0xypw"}, testcase.masks...) + rc.Masks = []string{"runtime-added"} - tryUploadJobSummary(t.Context(), rc) + tryUploadJobSummary(t.Context(), rc) - assert.Equal(t, "deployed true with *** and *** via ***", got) - cm.AssertExpectations(t) + if testcase.want == "" { + assert.Equal(t, jobSummaryTruncationMarker, strings.TrimLeft(got, "*")) + } else { + assert.Equal(t, testcase.want, got) + } + cm.AssertExpectations(t) + }) + } } diff --git a/act/runner/job_hooks.go b/act/runner/job_hooks.go index f32485b2..062ff975 100644 --- a/act/runner/job_hooks.go +++ b/act/runner/job_hooks.go @@ -82,8 +82,8 @@ func (rc *RunContext) runJobHook(ctx context.Context, hookPath, name string) err // the first one wrote. func (rc *RunContext) setupHookFileCommands(ctx context.Context, env map[string]string) error { actPath := rc.JobContainer.GetActPath() - env["GITHUB_ENV"] = path.Join(actPath, hookEnvFileCommand) - env["GITHUB_PATH"] = path.Join(actPath, hookPathFileCommand) + env["GITHUB_ENV"] = container.JoinPath(actPath, hookEnvFileCommand) + env["GITHUB_PATH"] = container.JoinPath(actPath, hookPathFileCommand) aliasGiteaEnv(env) return rc.JobContainer.Copy(actPath, @@ -94,7 +94,7 @@ func (rc *RunContext) setupHookFileCommands(ctx context.Context, env map[string] func (rc *RunContext) processHookFileCommands(ctx context.Context) error { err := processRunnerEnvFileCommand(ctx, hookEnvFileCommand, rc, rc.setEnvFile) - if pathErr := rc.UpdateExtraPath(ctx, path.Join(rc.JobContainer.GetActPath(), hookPathFileCommand)); pathErr != nil && err == nil { + if pathErr := rc.UpdateExtraPath(ctx, container.JoinPath(rc.JobContainer.GetActPath(), hookPathFileCommand)); pathErr != nil && err == nil { err = pathErr } return err @@ -109,7 +109,7 @@ func hookCommand(hookPath string) (cmd []string, shell string) { case ".sh": return []string{"bash", "-e", hookPath}, "bash -e {0}" case ".ps1": - return []string{"pwsh", "-command", ". '" + hookPath + "'"}, `pwsh -command ". '{0}'"` + return []string{"pwsh", "-command", ". '" + expressionStringEscape(hookPath) + "'"}, `pwsh -command ". '{0}'"` default: return []string{hookPath}, "" } diff --git a/act/runner/job_hooks_test.go b/act/runner/job_hooks_test.go index e07b1081..56ab5ab5 100644 --- a/act/runner/job_hooks_test.go +++ b/act/runner/job_hooks_test.go @@ -246,9 +246,10 @@ func TestHookCommand(t *testing.T) { cmd []string shell string }{ - "/hooks/started.sh": {[]string{"bash", "-e", "/hooks/started.sh"}, "bash -e {0}"}, - "/hooks/started.PS1": {[]string{"pwsh", "-command", ". '/hooks/started.PS1'"}, `pwsh -command ". '{0}'"`}, - "/hooks/started": {[]string{"/hooks/started"}, ""}, + "/hooks/started.sh": {[]string{"bash", "-e", "/hooks/started.sh"}, "bash -e {0}"}, + "/hooks/started.PS1": {[]string{"pwsh", "-command", ". '/hooks/started.PS1'"}, `pwsh -command ". '{0}'"`}, + "/hooks/started": {[]string{"/hooks/started"}, ""}, + `C:\hooks\O'Brien\started.ps1`: {[]string{"pwsh", "-command", `. 'C:\hooks\O''Brien\started.ps1'`}, `pwsh -command ". '{0}'"`}, } { cmd, shell := hookCommand(hookPath) assert.Equal(t, want.cmd, cmd, hookPath) diff --git a/act/runner/logger.go b/act/runner/logger.go index 3c673c1a..ceafa3ce 100644 --- a/act/runner/logger.go +++ b/act/runner/logger.go @@ -479,11 +479,15 @@ func (f textFormatter) isColored() bool { // such as a container name or a job summary. Masks added at runtime count, so a summary written // after ::add-mask:: is covered too. func (rc *RunContext) maskSecrets(value string) string { + return NewSecretReplacer(rc.secretMaskers()).Replace(value) +} + +func (rc *RunContext) secretMaskers() []string { oldnew := rc.Config.maskers() for _, mask := range rc.Masks { oldnew = AppendSecretMasker(oldnew, mask) } - return NewSecretReplacer(oldnew).Replace(value) + return oldnew } // maskers is every value this job's configuration says to hide, whatever the sink. diff --git a/act/runner/patch_actions_test.go b/act/runner/patch_actions_test.go index 9ca93b77..dc57d2b4 100644 --- a/act/runner/patch_actions_test.go +++ b/act/runner/patch_actions_test.go @@ -256,7 +256,7 @@ func TestActionScriptPaths(t *testing.T) { } func TestPatchActionsAtTheContainerCopy(t *testing.T) { - copiedBundles := func(t *testing.T, noPatch bool) ([]string, []*container.FileEntry) { + copiedBundles := func(t *testing.T, noPatch bool) ([]string, []*container.FileEntry, int64) { t.Helper() cm := &containerMock{} @@ -274,6 +274,8 @@ func TestPatchActionsAtTheContainerCopy(t *testing.T) { require.NoError(t, os.MkdirAll(filepath.Dir(script), 0o755)) require.NoError(t, os.WriteFile(filepath.Join(filepath.Dir(script), "index.js"), []byte("require('./cache.js')"), 0o600)) require.NoError(t, os.WriteFile(script, []byte(gateTSC), 0o600)) + info, err := os.Stat(script) + require.NoError(t, err) var calls []string var copied []*container.FileEntry @@ -291,22 +293,22 @@ func TestPatchActionsAtTheContainerCopy(t *testing.T) { source, err := os.ReadFile(script) require.NoError(t, err) assert.Equal(t, gateTSC, string(source), "the shared copy is never written") - return calls, copied + return calls, copied, int64(info.Mode().Perm()) } t.Run("patched over the container's copy", func(t *testing.T) { - calls, copied := copiedBundles(t, false) + calls, copied, mode := copiedBundles(t, false) assert.Equal(t, []string{"CopyDir", "Copy"}, calls) require.Len(t, copied, 1) assert.Equal(t, "dist/cache.js", copied[0].Name) - assert.Equal(t, int64(0o600), copied[0].Mode) + assert.Equal(t, mode, copied[0].Mode) assert.True(t, gateOpened(copied[0].Body)) }) // The escape hatch, for an action the edit breaks: the artifact actions refuse again, and the // cache client keeps to v1. t.Run("as shipped when the runner is told not to patch", func(t *testing.T) { - calls, copied := copiedBundles(t, true) + calls, copied, _ := copiedBundles(t, true) assert.Equal(t, []string{"CopyDir"}, calls) assert.Empty(t, copied) }) diff --git a/act/runner/reusable_workflow_test.go b/act/runner/reusable_workflow_test.go index f4ee714a..f4ae8d30 100644 --- a/act/runner/reusable_workflow_test.go +++ b/act/runner/reusable_workflow_test.go @@ -160,8 +160,9 @@ func TestCloneRemoteReusableWorkflowSendsInstanceTokenAndClientCert(t *testing.T Run: &model.Run{JobID: "j1", Workflow: &model.Workflow{Jobs: map[string]*model.Job{"j1": {}}}}, } - require.Error(t, cloneRemoteReusableWorkflow(rc, server.URL+"/owner/repo", "main", t.TempDir())(t.Context())) - require.True(t, gotCert) + err = cloneRemoteReusableWorkflow(rc, server.URL+"/owner/repo", "main", t.TempDir())(t.Context()) + require.Error(t, err) + require.True(t, gotCert, "clone failed before receiving a client certificate: %v", err) require.Equal(t, "token", gotToken) } diff --git a/act/runner/run_context.go b/act/runner/run_context.go index 4573294d..a58641d3 100644 --- a/act/runner/run_context.go +++ b/act/runner/run_context.go @@ -81,6 +81,7 @@ type RunContext struct { jobRunDefaults model.RunDefaults // defaults.run, resolved once at job setup as GitHub does platformImage string // container.image or the runs-on pick, resolved once by isEnabled jobIndex, jobTotal int // strategy.job-index and job-total, total 0 when unknown + trackingID string // RUNNER_TRACKING_ID of the job, generated on first use runtimeToken string // ACTIONS_RUNTIME_TOKEN of the workflow run, shared with its called workflows and composite actions jobEnv map[string]string // workflow and job env as evaluated for the container and service specs @@ -191,7 +192,7 @@ func (rc *RunContext) GetEnv() map[string]string { if rc.Run != nil && rc.Run.Workflow != nil && rc.Config != nil { job := rc.Run.Job() if job != nil { - rc.Env = mergeMaps(rc.Run.Workflow.Env, job.Environment(), rc.Config.Env) + rc.Env = rc.mergeEnv(rc.Run.Workflow.Env, job.Environment(), rc.Config.Env) } } if rc.runtimeToken != "" { @@ -429,27 +430,58 @@ func (rc *RunContext) startHostEnvironment() common.Executor { Workdir: rc.Config.Workdir, CleanWorkdir: rc.Config.CleanWorkdir, ActPath: actPath, - CleanUp: func() { - os.RemoveAll(miscpath) - }, - StdOut: logWriter, - AllocatePTY: rc.Config.AllocatePTY, + Root: miscpath, + TrackingID: rc.runnerTrackingID(), + StdOut: logWriter, + AllocatePTY: rc.Config.AllocatePTY, } rc.cleanUpJobContainer = rc.JobContainer.Remove() rc.setRunnerContextEnv(ctx) - for _, env := range os.Environ() { - if k, v, ok := strings.Cut(env, "="); ok { - // don't override - if _, ok := rc.Env[k]; !ok { - rc.Env[k] = v - } - } - } + rc.importHostEnv(os.Environ()) return rc.copyWorkflowFiles()(ctx) } } +func (rc *RunContext) runnerTrackingID() string { + job := rc.topLevelRunContext() + if job.trackingID == "" { + job.trackingID = "gitea_" + rand.Text() + } + return job.trackingID +} + +func (rc *RunContext) isEnvCaseInsensitive() bool { + if rc.JobContainer != nil { + return rc.JobContainer.IsEnvironmentCaseInsensitive() + } + return runtime.GOOS == "windows" && rc.IsHostEnv() +} + +func (rc *RunContext) importHostEnv(environ []string) { + for _, env := range environ { + if key, value, ok := strings.Cut(env, "="); ok { + if key == "RUNNER_TRACKING_ID" || rc.isEnvCaseInsensitive() && strings.EqualFold(key, "RUNNER_TRACKING_ID") { + continue + } + _, exists := common.LookupFold(rc.Env, key, rc.isEnvCaseInsensitive()) + if !exists { + rc.Env[key] = value + } + } + } +} + +func (rc *RunContext) mergeEnv(envs ...map[string]string) map[string]string { + if rc.isEnvCaseInsensitive() { + env := map[string]string{} + mergeIntoMapCaseInsensitive(env, envs...) + return env + } else { + return mergeMaps(envs...) + } +} + func (rc *RunContext) setRunnerContextEnv(ctx context.Context) { for k, v := range rc.getRunnerContext(ctx) { if v, ok := v.(string); ok { @@ -460,6 +492,15 @@ func (rc *RunContext) setRunnerContextEnv(ctx context.Context) { var errKubernetesDocker = errors.New("docker actions and docker:// steps are not supported on Kubernetes") +var errWindowsHostContainer = errors.New("container actions and docker:// steps are not supported in Windows host jobs") + +var hostOS = runtime.GOOS // replaced in tests + +func (rc *RunContext) isWindowsHost() bool { + _, host := rc.JobContainer.(*container.HostEnvironment) + return host && hostOS == "windows" +} + func (rc *RunContext) startKubernetesPod() common.Executor { return func(ctx context.Context) error { if rc.containerSpec.Options != "" || len(rc.containerSpec.Volumes) > 0 || rc.containerSpec.Credentials != nil { @@ -1311,6 +1352,10 @@ func (rc *RunContext) resolvePlatformImage(ctx context.Context) error { } image, rc.kubernetes = strings.CutPrefix(image, "kubernetes://") rc.platformImage = cmp.Or(rc.containerSpec.Image, image) + if rc.isEnvCaseInsensitive() { + rc.Env = nil // Rebuild scopes after the host platform is known. + rc.GetEnv() + } return nil } @@ -1509,7 +1554,6 @@ func (rc *RunContext) getGithubContext(ctx context.Context) *model.GithubContext RepositoryOwner: rc.Config.Env["GITHUB_REPOSITORY_OWNER"], RetentionDays: cmp.Or(rc.Config.Env["GITHUB_RETENTION_DAYS"], "0"), RunnerPerflog: cmp.Or(rc.Config.Env["RUNNER_PERFLOG"], "/dev/null"), - RunnerTrackingID: rc.Config.Env["RUNNER_TRACKING_ID"], Repository: rc.Config.Env["GITHUB_REPOSITORY"], Ref: rc.Config.Env["GITHUB_REF"], Sha: rc.Config.Env["SHA_REF"], @@ -1523,6 +1567,8 @@ func (rc *RunContext) getGithubContext(ctx context.Context) *model.GithubContext ghc.EventPath = rc.JobContainer.GetActPath() + "/workflow/event.json" } + ghc.RunnerTrackingID = rc.runnerTrackingID() + if preset := rc.Config.PresetGitHubContext; preset != nil { merged := *preset merged.Workflow, merged.Job, merged.Workspace, merged.EventPath = ghc.Workflow, ghc.Job, ghc.Workspace, ghc.EventPath @@ -1645,7 +1691,9 @@ func (rc *RunContext) withGithubEnv(ctx context.Context, github *model.GithubCon env["GITHUB_REPOSITORY_OWNER"] = github.RepositoryOwner env["GITHUB_RETENTION_DAYS"] = github.RetentionDays env["RUNNER_PERFLOG"] = github.RunnerPerflog - env["RUNNER_TRACKING_ID"] = github.RunnerTrackingID + if _, ok := common.LookupFold(env, "RUNNER_TRACKING_ID", rc.isEnvCaseInsensitive()); !ok { + env["RUNNER_TRACKING_ID"] = github.RunnerTrackingID + } env["GITHUB_BASE_REF"] = github.BaseRef env["GITHUB_HEAD_REF"] = github.HeadRef env["GITHUB_SERVER_URL"] = github.ServerURL diff --git a/act/runner/run_context_test.go b/act/runner/run_context_test.go index e84e3141..957ae2ed 100644 --- a/act/runner/run_context_test.go +++ b/act/runner/run_context_test.go @@ -19,6 +19,7 @@ import ( "path/filepath" "runtime" "slices" + "strconv" "strings" "sync" "testing" @@ -1082,6 +1083,51 @@ func TestRunContextGetEnv(t *testing.T) { } } +func TestRunContextEnvCaseSensitivity(t *testing.T) { + for _, caseInsensitive := range []bool{false, true} { + t.Run(strconv.FormatBool(caseInsensitive), func(t *testing.T) { + rc := createIfTestRunContext(map[string]*model.Job{ + "job1": createJob(t, "runs-on: self-hosted\nenv:\n build_mode: job\n config_value: job", ""), + }) + rc.Run.Workflow.Env = map[string]string{"BUILD_MODE": "workflow", "CONFIG_VALUE": "workflow", "PATH": "workflow-path", "runner_tool_cache": "workflow-cache"} + rc.Config.Env = map[string]string{"CONFIG_VALUE": "config", "RUNNER_TOOL_CACHE": "configured-cache"} + rc.Config.DisableActEnv, rc.Config.NoSkipCheckout = true, true + rc.Env = nil + rc.GetEnv() + rc.JobContainer = fakeContainer{ExecutionsEnvironment: &container.HostEnvironment{}, caseInsensitive: caseInsensitive} + require.NoError(t, rc.resolvePlatformImage(t.Context())) + if caseInsensitive { + assert.Equal(t, map[string]string{"build_mode": "job", "CONFIG_VALUE": "config", "PATH": "workflow-path", "RUNNER_TOOL_CACHE": "configured-cache"}, rc.GetEnv()) + } else { + assert.Equal(t, map[string]string{ + "BUILD_MODE": "workflow", "build_mode": "job", "CONFIG_VALUE": "config", + "config_value": "job", "PATH": "workflow-path", "runner_tool_cache": "workflow-cache", "RUNNER_TOOL_CACHE": "configured-cache", + }, rc.GetEnv()) + } + want := maps.Clone(rc.GetEnv()) + rc.Run.Workflow.Env = nil + require.NoError(t, rc.Run.Workflow.RawEnv.Encode(`${{ fromJSON('{"BUILD_MODE":"workflow","CONFIG_VALUE":"workflow","PATH":"workflow-path","runner_tool_cache":"workflow-cache"}') }}`)) + rc.Env = nil + evaluated, err := evaluateJobEnv(t.Context(), rc) + require.NoError(t, err) + assert.Equal(t, want, evaluated) + rc.Env = evaluated + + rc.importHostEnv([]string{"Path=host-path", "HOST_ONLY=value", "RUNNER_TRACKING_ID=parent", "runner_tracking_id=parent-lower"}) + assert.NotContains(t, rc.Env, "RUNNER_TRACKING_ID") + assert.Equal(t, "workflow-path", rc.Env["PATH"]) + assert.Equal(t, "value", rc.Env["HOST_ONLY"]) + if caseInsensitive { + assert.NotContains(t, rc.Env, "Path") + assert.NotContains(t, rc.Env, "runner_tracking_id") + } else { + assert.Equal(t, "host-path", rc.Env["Path"]) + assert.Equal(t, "parent-lower", rc.Env["runner_tracking_id"]) + } + }) + } +} + func TestSetActionEnvRefreshesInputs(t *testing.T) { rc := &RunContext{} rc.setActionEnv(map[string]string{"INPUT_MSG": "pre"}) @@ -1405,13 +1451,14 @@ func TestGetJobContextReportsContainers(t *testing.T) { } func TestCaptureJobContainerInfoExportsDockerWorkspace(t *testing.T) { + workdir := t.TempDir() job := &containerMock{} job.On("Inspect", mock.Anything).Return(&container.Info{ ID: "job-container-id", - Mounts: map[string]string{"/workspace": "/var/lib/docker/volumes/job/_data"}, + Mounts: map[string]string{job.ToContainerPath(workdir): "/var/lib/docker/volumes/job/_data"}, }, nil) rc := &RunContext{ - Config: &Config{Workdir: "/workspace/owner/repo/"}, + Config: &Config{Workdir: filepath.Join(workdir, "owner", "repo") + string(filepath.Separator)}, Env: map[string]string{}, JobContainer: job, } @@ -1421,7 +1468,7 @@ func TestCaptureJobContainerInfoExportsDockerWorkspace(t *testing.T) { assert.Equal(t, "job-container-id", rc.jobContainerID) assert.Equal(t, "/var/lib/docker/volumes/job/_data/owner/repo", rc.Env["GITEA_DOCKER_WORKSPACE"]) - rc.Config.Workdir = "/elsewhere" + rc.Config.Workdir = filepath.Dir(workdir) rc.Env = map[string]string{} require.NoError(t, rc.captureJobContainerInfo()(t.Context())) assert.NotContains(t, rc.Env, "GITEA_DOCKER_WORKSPACE") diff --git a/act/runner/runner_windows_test.go b/act/runner/runner_windows_test.go new file mode 100644 index 00000000..adac22c4 --- /dev/null +++ b/act/runner/runner_windows_test.go @@ -0,0 +1,43 @@ +// Copyright 2026 The Gitea Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package runner + +import ( + "strings" + "testing" + + "gitea.com/gitea/runner/act/common/logging" + "gitea.com/gitea/runner/act/common/logging/logtest" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestRunEventHostEnvironmentUTF8(t *testing.T) { + handler := logtest.New() + t.Cleanup(func() { + if t.Failed() { + for _, record := range handler.AllRecords() { + stepIDs, _ := record.Attr("stepID") + t.Logf("stepID=%v: %s", stepIDs, record.Message) + } + } + }) + table := TestJobFileInfo{ + workdir: workdir, + workflowPath: "windows-utf8", + eventName: "push", + platforms: map[string]string{"windows-latest": "-self-hosted"}, + } + table.runTest(WithJobLoggerFactory(t.Context(), factoryFunc(func() logging.Handler { return handler })), t, &Config{Env: testCheckoutEnv()}) + var outputSteps []string + for _, record := range handler.AllRecords() { + if strings.TrimRight(record.Message, "\r\n") == "你好，世界" { + stepIDs, ok := record.Attr("stepID") + require.True(t, ok) + outputSteps = append(outputSteps, stepIDs.([]string)...) + } + } + assert.Equal(t, []string{"cmd", "pwsh", "powershell"}, outputSteps) +} diff --git a/act/runner/step.go b/act/runner/step.go index 0b6f611b..a86794c2 100644 --- a/act/runner/step.go +++ b/act/runner/step.go @@ -69,10 +69,15 @@ func (s stepStage) String() string { } func processRunnerEnvFileCommand(ctx context.Context, fileName string, rc *RunContext, setter func(context.Context, map[string]string, string)) error { + if rc.JobContainer.IsEnvironmentCaseInsensitive() { + return container.ParseEnvFile(rc.JobContainer, container.JoinPath(rc.JobContainer.GetActPath(), fileName), func(name, value string) { + setter(ctx, map[string]string{"name": name}, value) + }, true)(ctx) + } env := map[string]string{} - err := rc.JobContainer.UpdateFromEnv(path.Join(rc.JobContainer.GetActPath(), fileName), &env)(ctx) - for k, v := range env { - setter(ctx, map[string]string{"name": k}, v) + err := rc.JobContainer.UpdateFromEnv(container.JoinPath(rc.JobContainer.GetActPath(), fileName), &env)(ctx) + for key, value := range env { + setter(ctx, map[string]string{"name": key}, value) } return err } @@ -139,16 +144,16 @@ func runStepExecutor(step step, stage stepStage, executor common.Executor) commo actPath := rc.JobContainer.GetActPath() outputFileCommand := path.Join("workflow", "outputcmd.txt") - (*step.getEnv())["GITHUB_OUTPUT"] = path.Join(actPath, outputFileCommand) + (*step.getEnv())["GITHUB_OUTPUT"] = container.JoinPath(actPath, outputFileCommand) stateFileCommand := path.Join("workflow", "statecmd.txt") - (*step.getEnv())["GITHUB_STATE"] = path.Join(actPath, stateFileCommand) + (*step.getEnv())["GITHUB_STATE"] = container.JoinPath(actPath, stateFileCommand) pathFileCommand := path.Join("workflow", "pathcmd.txt") - (*step.getEnv())["GITHUB_PATH"] = path.Join(actPath, pathFileCommand) + (*step.getEnv())["GITHUB_PATH"] = container.JoinPath(actPath, pathFileCommand) envFileCommand := path.Join("workflow", "envs.txt") - (*step.getEnv())["GITHUB_ENV"] = path.Join(actPath, envFileCommand) + (*step.getEnv())["GITHUB_ENV"] = container.JoinPath(actPath, envFileCommand) // Per-step summary file. Composite sub-steps share the outer job step's index // via the Parent chain so all writes from within a composite action accumulate @@ -156,7 +161,7 @@ func runStepExecutor(step step, stage stepStage, executor common.Executor) commo topRC := rc.topLevelRunContext() stepSummaryIndex := topRC.CurrentStepIndex summaryFileCommand := path.Join("workflow", "step-summary-"+strconv.Itoa(stepSummaryIndex)+".md") - (*step.getEnv())["GITHUB_STEP_SUMMARY"] = path.Join(actPath, summaryFileCommand) + (*step.getEnv())["GITHUB_STEP_SUMMARY"] = container.JoinPath(actPath, summaryFileCommand) aliasGiteaEnv(*step.getEnv()) // Reset the per-phase file-command files. GITHUB_STEP_SUMMARY is intentionally @@ -203,7 +208,7 @@ func runStepExecutor(step step, stage stepStage, executor common.Executor) commo if fileErr := processRunnerEnvFileCommand(fileCommandCtx, outputFileCommand, rc, rc.setOutput); fileErr != nil && err == nil { err = fileErr } - if fileErr := rc.UpdateExtraPath(fileCommandCtx, path.Join(actPath, pathFileCommand)); fileErr != nil && err == nil { + if fileErr := rc.UpdateExtraPath(fileCommandCtx, container.JoinPath(actPath, pathFileCommand)); fileErr != nil && err == nil { err = fileErr } } @@ -351,17 +356,14 @@ func mergeIntoMapCaseInsensitive(target map[string]string, maps ...map[string]st for k := range target { foldKeys[strings.ToLower(k)] = k } - toKey := func(s string) string { - foldKey := strings.ToLower(s) - if k, ok := foldKeys[foldKey]; ok { - return k - } - foldKeys[foldKey] = s - return s - } for _, m := range maps { - for k, v := range m { - target[toKey(k)] = v + for key, value := range m { + foldKey := strings.ToLower(key) + if previous, ok := foldKeys[foldKey]; ok { + delete(target, previous) + } + foldKeys[foldKey] = key + target[key] = value } } } diff --git a/act/runner/step_docker.go b/act/runner/step_docker.go index dd7533ae..679f78ac 100644 --- a/act/runner/step_docker.go +++ b/act/runner/step_docker.go @@ -52,6 +52,9 @@ func (sd *stepDocker) runUsesContainer() common.Executor { step := sd.Step return func(ctx context.Context) error { + if rc.isWindowsHost() { + return errWindowsHostContainer + } if rc.kubernetes { return errKubernetesDocker } diff --git a/act/runner/step_docker_test.go b/act/runner/step_docker_test.go index 78b28a4f..24a7762f 100644 --- a/act/runner/step_docker_test.go +++ b/act/runner/step_docker_test.go @@ -159,3 +159,33 @@ func TestStepDockerNewStepContainer(t *testing.T) { }) } } + +func TestWindowsHostContainerActions(t *testing.T) { + for _, test := range []struct { + name, hostOS string + jobContainer container.ExecutionsEnvironment + wantError bool + }{ + {name: "Windows host", hostOS: "windows", jobContainer: &container.HostEnvironment{}, wantError: true}, + {name: "Linux host", hostOS: "linux", jobContainer: &container.HostEnvironment{}}, + {name: "macOS host", hostOS: "darwin", jobContainer: &container.HostEnvironment{}}, + {name: "Windows with Linux job container", hostOS: "windows", jobContainer: &containerMock{}}, + } { + t.Run(test.name, func(t *testing.T) { + original := hostOS + hostOS = test.hostOS + t.Cleanup(func() { hostOS = original }) + rc := &RunContext{JobContainer: test.jobContainer, kubernetes: true} + wantError := errKubernetesDocker.Error() + if test.wantError { + wantError = errWindowsHostContainer.Error() + } + t.Run("docker step", func(t *testing.T) { + require.EqualError(t, (&stepDocker{RunContext: rc}).runUsesContainer()(t.Context()), wantError) + }) + t.Run("container action", func(t *testing.T) { + require.EqualError(t, execAsDocker(t.Context(), &stepActionRemote{RunContext: rc}, "action", "", "", false, stepStageMain), wantError) + }) + }) + } +} diff --git a/act/runner/step_run.go b/act/runner/step_run.go index ec82e95e..2dc65fc4 100644 --- a/act/runner/step_run.go +++ b/act/runner/step_run.go @@ -100,7 +100,7 @@ func printRunActionHeader(ctx context.Context, step *model.Step, action *model.A maps.Copy(inputs, step.With) if action != nil { for name := range action.Inputs { - if _, provided := lookupFold(step.With, name, true); !provided { + if _, provided := common.LookupFold(step.With, name, true); !provided { inputs[name] = env[inputEnvKey(name)] } } @@ -292,7 +292,7 @@ func (sr *stepRun) setupShellCommand(ctx context.Context) (name, script string, name += ".sh" case "pwsh", "powershell": name += ".ps1" - runPrepend = "$ErrorActionPreference = 'stop'" + runPrepend = "$ErrorActionPreference = 'stop'\n$OutputEncoding = [System.Text.UTF8Encoding]::new($false)\ntry { [Console]::OutputEncoding = $OutputEncoding } catch {}" runAppend = "if ((Test-Path -LiteralPath variable:/LASTEXITCODE)) { exit $LASTEXITCODE }" case "cmd": name += ".cmd" @@ -302,6 +302,12 @@ func (sr *stepRun) setupShellCommand(ctx context.Context) (name, script string, } script = fmt.Sprintf("%s\n%s\n%s", runPrepend, script, runAppend) + switch shellCommand { + case "powershell": + script = "\ufeff" + script + case "cmd": + script = strings.ReplaceAll(strings.ReplaceAll(script, "\r\n", "\n"), "\n", "\r\n") + } if !strings.Contains(script, "::add-mask::") && !sr.RunContext.Config.InsecureSecrets { logger.Debugf("Wrote command \n%s\n to '%s'", script, name) @@ -311,7 +317,30 @@ func (sr *stepRun) setupShellCommand(ctx context.Context) (name, script string, rc := sr.getRunContext() scriptPath := fmt.Sprintf("%s/%s", rc.JobContainer.GetActPath(), name) + if step.Shell == "pwsh" || step.Shell == "powershell" { + scriptPath = expressionStringEscape(scriptPath) + } + if step.Shell == "bash" || step.Shell == "sh" || step.Shell == "python" { + if rc.isWindowsHost() { + scriptPath = `"` + scriptPath + `"` + } else { + scriptPath = shellquote.Join(scriptPath) + } + } sr.cmdline = strings.ReplaceAll(scCmd, `{0}`, scriptPath) + if rc.isWindowsHost() { + command := strings.TrimSpace(sr.cmdline) + if strings.HasPrefix(command, `"`) { + executable, _, closed := strings.Cut(command[1:], `"`) + if !closed || executable == "" { + return "", "", fmt.Errorf("invalid shell option %q: expected a double-quoted executable", step.Shell) + } + sr.cmd = []string{executable} + } else { + sr.cmd = strings.Fields(command)[:1] + } + return name, script, nil + } sr.cmd, err = shellquote.Split(sr.cmdline) return name, script, err diff --git a/act/runner/step_run_test.go b/act/runner/step_run_test.go index d8e08c2f..48a70fec 100644 --- a/act/runner/step_run_test.go +++ b/act/runner/step_run_test.go @@ -11,6 +11,7 @@ import ( "os" "path/filepath" "runtime" + "strings" "testing" "gitea.com/gitea/runner/act/container" @@ -80,14 +81,17 @@ func TestStepRun(t *testing.T) { } func TestStepRunShellParity(t *testing.T) { - tests := []struct { + const powershellScript = "$ErrorActionPreference = 'stop'\n$OutputEncoding = [System.Text.UTF8Encoding]::new($false)\ntry { [Console]::OutputEncoding = $OutputEncoding } catch {}\necho 你好，世界\nif ((Test-Path -LiteralPath variable:/LASTEXITCODE)) { exit $LASTEXITCODE }" + for _, test := range []struct { name, run, shell, workingDir string + hostOS, wantCmdline string actPath string env map[string]string host bool probeErr error wantExt string wantCmd []string + wantScript string wantErr string }{ { @@ -105,24 +109,79 @@ func TestStepRunShellParity(t *testing.T) { wantCmd: []string{"sh", "-e", "/var/run/act/workflow/1.sh"}, }, { - name: "builtin pwsh", - shell: "pwsh", - actPath: "/var/run/act with spaces", - wantExt: ".ps1", - wantCmd: []string{"pwsh", "-command", ". '/var/run/act with spaces/workflow/1.ps1'"}, + name: "builtin pwsh", + run: "echo 你好，世界", + shell: "pwsh", + actPath: "/var/run/act with spaces/O'Brien", + wantExt: ".ps1", + wantCmd: []string{"pwsh", "-command", ". '/var/run/act with spaces/O''Brien/workflow/1.ps1'"}, + wantScript: powershellScript, }, { - name: "builtin powershell", - shell: "powershell", - wantExt: ".ps1", - wantCmd: []string{"powershell", "-command", ". '/var/run/act/workflow/1.ps1'"}, + name: "builtin powershell", + actPath: "/var/run/act/O'Brien", + run: "echo 你好，世界", + shell: "powershell", + wantExt: ".ps1", + wantCmd: []string{"powershell", "-command", ". '/var/run/act/O''Brien/workflow/1.ps1'"}, + wantScript: "\ufeff" + powershellScript, }, { - name: "custom pwsh template", - shell: "pwsh -NoProfile -File {0}", - wantExt: ".ps1", - wantCmd: []string{"pwsh", "-NoProfile", "-File", "/var/run/act/workflow/1.ps1"}, + name: "custom pwsh template", + run: "echo 你好，世界", + shell: "pwsh -NoProfile -File {0}", + wantExt: ".ps1", + wantCmd: []string{"pwsh", "-NoProfile", "-File", "/var/run/act/workflow/1.ps1"}, + wantScript: powershellScript, }, + { + name: "builtin cmd reads and emits UTF-8 without a BOM", + run: "echo 你好，世界", + shell: "cmd", + wantExt: ".cmd", + wantCmd: []string{"cmd", "/D", "/E:ON", "/V:OFF", "/S", "/C", "CALL /var/run/act/workflow/1.cmd"}, + wantScript: "@echo off\r\necho 你好，世界\r\n", + }, + { + name: "Windows custom executable preserves backslashes and raw arguments", + shell: `C:\Python313\python.exe -X 'utf8' "{0}"`, + actPath: `C:\runner\O'Brien`, + hostOS: "windows", + wantCmd: []string{`C:\Python313\python.exe`}, + wantCmdline: `C:\Python313\python.exe -X 'utf8' "{0}"`, + }, + { + name: "Windows quoted custom executable preserves spaces and backslashes", + shell: `"C:\Program Files\Python313\python.exe" "{0}"`, + actPath: `C:\runner`, + hostOS: "windows", + wantCmd: []string{`C:\Program Files\Python313\python.exe`}, + wantCmdline: `"C:\Program Files\Python313\python.exe" "{0}"`, + }, + { + name: "Windows PowerShell escapes apostrophes in the raw command line", + run: "echo 你好，世界", + shell: "pwsh", + actPath: `C:\runner\O'Brien`, + hostOS: "windows", + wantExt: ".ps1", + wantCmd: []string{"pwsh"}, + wantCmdline: `pwsh -command ". '{0}'"`, + wantScript: powershellScript, + }, + { + name: "Windows unclosed executable quote", + shell: `"C:\Program Files\python.exe {0}`, + actPath: `C:\runner`, + hostOS: "windows", + wantErr: "expected a double-quoted executable", + }, + {name: "linux builtin bash quotes script path", shell: "bash", actPath: `C:/Users/Jane Doe/O'Brien`, wantCmd: []string{"bash", "--noprofile", "--norc", "-e", "-o", "pipefail", `C:/Users/Jane Doe/O'Brien/workflow/1.sh`}}, + {name: "linux builtin sh quotes script path", shell: "sh", actPath: `C:/Users/Jane Doe/O'Brien`, wantCmd: []string{"sh", "-e", `C:/Users/Jane Doe/O'Brien/workflow/1.sh`}}, + {name: "linux builtin python quotes script path", shell: "python", actPath: `C:/Users/Jane Doe/O'Brien`, wantExt: ".py", wantCmd: []string{"python", `C:/Users/Jane Doe/O'Brien/workflow/1.py`}}, + {name: "windows builtin bash quotes script path", shell: "bash", hostOS: "windows", actPath: `C:\Users\Jane Doe\O'Brien`, wantExt: ".sh", wantCmd: []string{"bash"}, wantCmdline: `bash --noprofile --norc -e -o pipefail "{0}"`}, + {name: "windows builtin sh quotes script path", shell: "sh", hostOS: "windows", actPath: `C:\Users\Jane Doe\O'Brien`, wantExt: ".sh", wantCmd: []string{"sh"}, wantCmdline: `sh -e "{0}"`}, + {name: "windows builtin python quotes script path", shell: "python", hostOS: "windows", actPath: `C:\Users\Jane Doe\O'Brien`, wantExt: ".py", wantCmd: []string{"python"}, wantCmdline: `python "{0}"`}, { name: "missing placeholder", shell: "bash -e", @@ -156,9 +215,7 @@ func TestStepRunShellParity(t *testing.T) { workingDir: "${{ DIR }}", wantErr: "unable to interpolate the working directory:", }, - } - - for _, test := range tests { + } { t.Run(test.name, func(t *testing.T) { cm := &containerMock{} var jobContainer container.ExecutionsEnvironment = &shellContainerMock{cm} @@ -189,22 +246,28 @@ func TestStepRunShellParity(t *testing.T) { env: test.env, } + original := hostOS + hostOS = cmp.Or(test.hostOS, "linux") + t.Cleanup(func() { hostOS = original }) name, script, err := sr.setupShellCommand(t.Context()) if test.wantErr != "" { require.ErrorContains(t, err, test.wantErr) return } require.NoError(t, err) - if test.wantExt == "" { + if test.wantExt == "" && test.hostOS != "windows" { test.wantExt = ".sh" } - wantScript := "\necho hi\n" - if test.wantExt == ".ps1" { - wantScript = "$ErrorActionPreference = 'stop'\necho hi\nif ((Test-Path -LiteralPath variable:/LASTEXITCODE)) { exit $LASTEXITCODE }" - } assert.Equal(t, "workflow/1"+test.wantExt, name) - assert.Equal(t, wantScript, script) + assert.Equal(t, cmp.Or(test.wantScript, "\necho hi\n"), script) assert.Equal(t, test.wantCmd, sr.cmd) + if test.wantCmdline != "" { + scriptPath := jobContainer.GetActPath() + "/workflow/1" + test.wantExt + if test.shell == "pwsh" || test.shell == "powershell" { + scriptPath = strings.ReplaceAll(scriptPath, "'", "''") + } + assert.Equal(t, strings.ReplaceAll(test.wantCmdline, "{0}", scriptPath), sr.cmdline) + } assert.Equal(t, test.env["DIR"], sr.WorkingDirectory) cm.AssertExpectations(t) }) diff --git a/act/runner/step_test.go b/act/runner/step_test.go index a369ad88..c9b29fbd 100644 --- a/act/runner/step_test.go +++ b/act/runner/step_test.go @@ -188,8 +188,9 @@ func TestSetupEnv(t *testing.T) { "RC_KEY": "rcvalue", "RUNNER_ENVIRONMENT": "self-hosted", "RUNNER_PERFLOG": "/dev/null", - "RUNNER_TRACKING_ID": "", + "RUNNER_TRACKING_ID": env["RUNNER_TRACKING_ID"], } + assert.Regexp(t, `^gitea_[A-Z2-7]{26}$`, env["RUNNER_TRACKING_ID"]) for name, value := range expected { if suffix, ok := strings.CutPrefix(name, "GITHUB_"); ok { expected["GITEA_"+suffix] = value @@ -197,6 +198,14 @@ func TestSetupEnv(t *testing.T) { } assert.Equal(t, expected, env) + t.Run("keeps an inherited RUNNER_TRACKING_ID opt-out", func(t *testing.T) { + rc.Env["RUNNER_TRACKING_ID"] = "" + optedOut := map[string]string{} + require.NoError(t, setupEnv(t.Context(), &stepRun{RunContext: rc, Step: step, env: optedOut})) + assert.Contains(t, optedOut, "RUNNER_TRACKING_ID") + assert.Empty(t, optedOut["RUNNER_TRACKING_ID"]) + }) + for _, expression := range []string{"inputs.args", "env.ARGS"} { t.Run("deferred inputs from "+expression, func(t *testing.T) { workflow, err := model.ReadWorkflow(strings.NewReader(` @@ -410,6 +419,29 @@ func (c *fileCommandTestContainer) GetContainerArchive(ctx context.Context, srcP return c.HostEnvironment.GetContainerArchive(ctx, srcPath) } +type windowsFileCommandTestContainer struct { + *container.HostEnvironment +} + +func (*windowsFileCommandTestContainer) IsEnvironmentCaseInsensitive() bool { return true } + +func TestProcessRunnerEnvFileCommandOrder(t *testing.T) { + rc := &RunContext{ + JobContainer: &windowsFileCommandTestContainer{&container.HostEnvironment{ActPath: t.TempDir()}}, + } + require.NoError(t, os.WriteFile(filepath.Join(rc.JobContainer.GetActPath(), "envs.txt"), []byte("BUILD_MODE=first\nbuild_mode< \"" + outFile + "\"\n" + if runtime.GOOS == "windows" { + scriptPath = filepath.Join(dir, "post-task.cmd") + script = "@echo off\r\n \"" + outFile + "\"\r\nexit /b 0\r\n" + } require.NoError(t, os.WriteFile(scriptPath, []byte(script), 0o700)) cfg, err := config.LoadDefault("") diff --git a/internal/app/run/runner.go b/internal/app/run/runner.go index 61c5d74c..4f78f542 100644 --- a/internal/app/run/runner.go +++ b/internal/app/run/runner.go @@ -168,7 +168,7 @@ func (r *Runner) OnIdle(ctx context.Context) { } // Bind-workdir mode: reclaim stale per-task workspace dirs (numeric task IDs). if r.cfg.Container.BindWorkdir { - r.cleanupStaleDirs(ctx, filepath.FromSlash("/"+strings.TrimLeft(r.cfg.Container.WorkdirParent, "/")), isTaskIDDir) + r.cleanupStaleDirs(ctx, containerWorkdirRoot(r.cfg.Container.WorkdirParent), isTaskIDDir) } // Host mode: reclaim per-job scratch dirs left behind when HostEnvironment // cleanup timed out (e.g. a delete stalled by an AV/EDR filter driver). They @@ -557,12 +557,12 @@ func (r *Runner) run(ctx context.Context, task *runnerv1.Task, reporter *report. actionCloneDepth = 0 } - workdirParent := strings.TrimLeft(r.cfg.Container.WorkdirParent, "/") + workdirParent := containerWorkdirRoot(r.cfg.Container.WorkdirParent) if r.cfg.Container.BindWorkdir { // Append the task ID to isolate concurrent jobs from the same repo. - workdirParent = fmt.Sprintf("%s/%d", workdirParent, task.Id) + workdirParent = filepath.Join(workdirParent, strconv.FormatInt(task.Id, 10)) } - workdir := filepath.FromSlash(fmt.Sprintf("/%s/%s", workdirParent, preset.Repository)) + workdir := filepath.Join(workdirParent, filepath.FromSlash(preset.Repository)) if runtime.GOOS == "windows" { if abs, err := filepath.Abs(workdir); err == nil { workdir = abs @@ -648,10 +648,8 @@ func (r *Runner) run(ctx context.Context, task *runnerv1.Task, reporter *report. reporter.SetOutputs(job.Outputs) if r.cfg.Container.BindWorkdir { - // Remove the entire task-specific directory (e.g. /workspace/). - taskDir := filepath.FromSlash("/" + workdirParent) - if err := os.RemoveAll(taskDir); err != nil { - logging.Warnf("failed to clean up workspace %s: %v", taskDir, err) + if err := os.RemoveAll(workdirParent); err != nil { + logging.Warnf("failed to clean up workspace %s: %v", workdirParent, err) } } @@ -821,7 +819,7 @@ func (r *Runner) setHealthStatus(ready bool, reason string) { func checkFreeDisk(cfg *config.Config) (bool, string) { root := cfg.Host.WorkdirParent if cfg.Container.BindWorkdir { - root = filepath.FromSlash("/" + strings.TrimLeft(cfg.Container.WorkdirParent, "/")) + root = containerWorkdirRoot(cfg.Container.WorkdirParent) } root = nearestExistingPath(root) available, err := disk.FreeBytes(root) @@ -835,6 +833,13 @@ func checkFreeDisk(cfg *config.Config) (bool, string) { return true, "ok" } +func containerWorkdirRoot(parent string) string { + if filepath.IsAbs(parent) { + return filepath.Clean(parent) + } + return filepath.Join(string(filepath.Separator), parent) +} + func nearestExistingPath(path string) string { for path != "" { if _, err := os.Stat(path); err == nil { diff --git a/internal/app/run/runner_idle_cleanup_test.go b/internal/app/run/runner_idle_cleanup_test.go index 0de30b0f..07c68972 100644 --- a/internal/app/run/runner_idle_cleanup_test.go +++ b/internal/app/run/runner_idle_cleanup_test.go @@ -241,9 +241,6 @@ func TestRunnerShouldRunIdleCleanupSkipsWhenDisabled(t *testing.T) { }) } -// TestRunnerOnIdleIntegratesCleanup wires the full OnIdle entry point and -// confirms it walks workdir_parent (after the leading-slash trim that -// matches the production path construction) and removes stale numeric dirs. func TestRunnerOnIdleIntegratesCleanup(t *testing.T) { now := time.Date(2026, time.April, 29, 20, 0, 0, 0, time.UTC) root := t.TempDir() @@ -255,7 +252,7 @@ func TestRunnerOnIdleIntegratesCleanup(t *testing.T) { cfg: &config.Config{ Container: config.Container{ BindWorkdir: true, - WorkdirParent: root, // leading slash absent, OnIdle reattaches it + WorkdirParent: root, }, Runner: config.Runner{ WorkdirCleanupAge: 24 * time.Hour, @@ -371,3 +368,16 @@ func TestRunnerOnIdleRemovesOrphanNetworks(t *testing.T) { assert.Equal(t, []string{"a1b2c3d4e5f6", ""}, detached) assert.Equal(t, []string{"runner-1"}, sweptPods) } + +func TestContainerWorkdirRoot(t *testing.T) { + absolute := t.TempDir() + rooted := filepath.Join(string(filepath.Separator), "workspace") + for input, want := range map[string]string{ + "workspace": rooted, + "/workspace/": rooted, + absolute: absolute, + absolute + "/nested": filepath.Join(absolute, "nested"), + } { + assert.Equal(t, want, containerWorkdirRoot(input), input) + } +} diff --git a/internal/pkg/action/checkout/checkout.go b/internal/pkg/action/checkout/checkout.go index dd609d18..2d42734b 100644 --- a/internal/pkg/action/checkout/checkout.go +++ b/internal/pkg/action/checkout/checkout.go @@ -42,7 +42,10 @@ func Main(ctx context.Context, c *action.Context) (err error) { return fmt.Errorf("checkout: unsupported input %q", name) } } - dst, err := workspaceDestination(c.Workspace, c.Inputs["path"]) + runnerContext := c.Container.GetRunnerContext(ctx) + executionOS, _ := runnerContext["os"].(string) + windows := strings.EqualFold(executionOS, "windows") + dst, err := workspaceDestination(c.Workspace, c.Inputs["path"], windows) if err != nil { return err } @@ -63,8 +66,8 @@ func Main(ctx context.Context, c *action.Context) (err error) { return err } } - if output := c.Env["GITHUB_OUTPUT"]; output != "" { - return c.Container.Copy(path.Dir(output), &container.FileEntry{Name: path.Base(output), Mode: 0o666, Body: fmt.Sprintf("ref=%s\ncommit=%s\n", ghc.Ref, ghc.Sha)})(ctx) + if output := filepath.ToSlash(c.Env["GITHUB_OUTPUT"]); output != "" { + return c.Container.Copy(container.JoinPath(output, ".."), &container.FileEntry{Name: path.Base(output), Mode: 0o666, Body: fmt.Sprintf("ref=%s\ncommit=%s\n", ghc.Ref, ghc.Sha)})(ctx) } return nil } @@ -72,6 +75,7 @@ func Main(ctx context.Context, c *action.Context) (err error) { co := &checkout{ url: strings.TrimSuffix(ghc.ServerURL, "/") + "/" + repository, dst: dst, + windows: windows, ref: ref, depth: depth, fetchTags: boolInput(c.Inputs, "fetch-tags", false), @@ -114,10 +118,11 @@ func Main(ctx context.Context, c *action.Context) (err error) { } // named like actions/checkout's, which create-pull-request knows to hide, and gone with the job - runnerTemp, _ := c.Container.GetRunnerContext(ctx)["temp"].(string) + runnerTemp, _ := runnerContext["temp"].(string) + runnerTemp = filepath.ToSlash(runnerTemp) digest := sha256.Sum256([]byte(dst)) credentials := fmt.Sprintf("git-credentials-%x.config", digest[:8]) - co.credentials = path.Join(runnerTemp, credentials) + co.credentials = container.JoinPath(runnerTemp, credentials) persist := boolInput(c.Inputs, "persist-credentials", true) config, remoteConfig := serverConfig(ghc.ServerURL, cmp.Or(c.Inputs["token"], ghc.Token), c.InsecureSkipTLS, co.submodules != "" && sshKey == "") if boolInput(c.Inputs, "set-safe-directory", true) { @@ -140,22 +145,21 @@ func Main(ctx context.Context, c *action.Context) (err error) { if err := c.Container.Copy(runnerTemp, &container.FileEntry{Name: cert, Mode: 0o600, Body: string(certPEM)}, &container.FileEntry{Name: key, Mode: 0o600, Body: string(keyPEM)})(ctx); err != nil { return err } - scope := "http." + strings.TrimSuffix(ghc.ServerURL, "/") + "/." - remoteConfig = append(remoteConfig, scope+"sslCert="+path.Join(runnerTemp, cert), scope+"sslKey="+path.Join(runnerTemp, key)) + remoteConfig = append(remoteConfig, git.ClientCertConfig("http."+strings.TrimSuffix(ghc.ServerURL, "/")+"/.", container.JoinPath(runnerTemp, cert), container.JoinPath(runnerTemp, key), executionOS)...) } env := gitEnv(c, remoteConfig) env["GIT_LFS_SKIP_SMUDGE"] = strconv.FormatBool(!co.lfs || len(co.sparse) == 0) // lfs pull fetches the objects of a full checkout in one go if sshKey != "" { key := fmt.Sprintf("git-ssh-key-%x", digest[:8]) files := []*container.FileEntry{{Name: key, Mode: 0o600, Body: sshKey + "\n"}} - ssh := []string{"ssh", "-i", path.Join(runnerTemp, key)} + ssh := []string{"ssh", "-i", container.JoinPath(runnerTemp, key)} if boolInput(c.Inputs, "ssh-strict", true) { ssh = append(ssh, "-o", "StrictHostKeyChecking=yes", "-o", "CheckHostIP=no") } if knownHosts := strings.TrimSpace(c.Inputs["ssh-known-hosts"]); knownHosts != "" { name := fmt.Sprintf("git-ssh-known-hosts-%x", digest[:8]) files = append(files, &container.FileEntry{Name: name, Mode: 0o600, Body: knownHosts + "\n"}) - ssh = append(ssh, "-o", "UserKnownHostsFile=~/.ssh/known_hosts "+strconv.Quote(path.Join(runnerTemp, name))) + ssh = append(ssh, "-o", "UserKnownHostsFile=~/.ssh/known_hosts "+strconv.Quote(container.JoinPath(runnerTemp, name))) } if !persist { defer func() { @@ -218,7 +222,7 @@ func gitEnv(c *action.Context, config []string) map[string]string { env := map[string]string{"GIT_TERMINAL_PROMPT": "0"} maps.Copy(env, c.Env) maps.DeleteFunc(env, func(name, _ string) bool { return slices.Contains(git.LocalEnvVars, name) }) - maps.Copy(env, git.ConfigEnv(append([]string{"protocol.version=2", "init.defaultBranch=main", "advice.detachedHead=false", "maintenance.auto=false"}, config...)...)) + maps.Copy(env, git.ConfigEnv(append([]string{"protocol.version=2", "init.defaultBranch=main", "advice.detachedHead=false", "maintenance.auto=false", "core.longpaths=true"}, config...)...)) return env } @@ -301,7 +305,7 @@ type checkout struct { filter, submodules string sparse []string depth int - fetchTags, clean, lfs, progress, cone bool + fetchTags, clean, lfs, progress, cone, windows bool } const fullDepth = "2147483647" // git's INFINITE_DEPTH, also deepens a shallow dst @@ -315,6 +319,9 @@ func (co *checkout) commands() (commands [][]string, fetched int) { repo("config", "remote.origin.fetch", "+refs/heads/*:refs/remotes/origin/*"), repo("config", "--replace-all", "--fixed-value", credentialsInclude, co.credentials, co.credentials), } + if co.windows { + commands = append(commands, repo("config", "core.longpaths", "true")) // later steps run git on the checkout too + } if co.lfs { commands = append(commands, repo("lfs", "install", "--local")) } @@ -451,9 +458,13 @@ func boolInput(inputs map[string]string, name string, fallback bool) bool { return strings.EqualFold(value, "true") || value == "" && fallback } -func workspaceDestination(workspace, checkoutPath string) (string, error) { - cleanPath := path.Clean(strings.TrimSpace(checkoutPath)) - if path.IsAbs(cleanPath) || cleanPath == ".." || strings.HasPrefix(cleanPath, "../") { +func workspaceDestination(workspace, checkoutPath string, windows bool) (string, error) { + cleanPath := strings.TrimSpace(checkoutPath) + if windows { + cleanPath = strings.ReplaceAll(cleanPath, `\`, "/") + } + cleanPath = path.Clean(cleanPath) + if path.IsAbs(cleanPath) || cleanPath == ".." || strings.HasPrefix(cleanPath, "../") || windows && len(cleanPath) > 1 && cleanPath[1] == ':' { return "", fmt.Errorf("checkout: path %q must be relative to the workspace", checkoutPath) } return path.Join(workspace, cleanPath), nil diff --git a/internal/pkg/action/checkout/checkout_test.go b/internal/pkg/action/checkout/checkout_test.go index f6b8b020..32a5596c 100644 --- a/internal/pkg/action/checkout/checkout_test.go +++ b/internal/pkg/action/checkout/checkout_test.go @@ -13,9 +13,11 @@ import ( "io" "net/http" "net/http/httptest" + "net/url" "os" "os/exec" "path/filepath" + "runtime" "strings" "testing" @@ -50,6 +52,7 @@ func gitLog(dir string) []string { func TestCheckout(t *testing.T) { server := t.TempDir() + serverURL := (&url.URL{Scheme: "file", Path: "/" + strings.TrimPrefix(filepath.ToSlash(server), "/")}).String() var sha string _, lfsErr := exec.LookPath("git-lfs") require.True(t, t.Run("fixtures", func(t *testing.T) { @@ -192,13 +195,16 @@ exec sh -c "$(printf '%s' "$last" | sed "s#git-upload-pack '/\(.*\)\.git'#git up if tc.inputs["lfs"] == "true" && lfsErr != nil { t.Skip("needs git-lfs") } + if tc.inputs["ssh-key"] != "" && runtime.GOOS == "windows" { + t.Skip("Git for Windows puts its own ssh ahead of the fake one on PATH") + } workspace, runnerTemp := t.TempDir(), t.TempDir() output := filepath.Join(runnerTemp, "output") checkout := func(inputs map[string]string) error { return Main(t.Context(), &action.Context{ Container: &container.HostEnvironment{Path: workspace, TmpDir: runnerTemp, StdOut: io.Discard}, Github: &model.GithubContext{ - Repository: "org/repo", Ref: cmp.Or(tc.eventRef, "refs/heads/main"), Sha: sha, ServerURL: "file://" + server, Token: "tok", Event: tc.event, + Repository: "org/repo", Ref: cmp.Or(tc.eventRef, "refs/heads/main"), Sha: sha, ServerURL: serverURL, Token: "tok", Event: tc.event, }, Inputs: inputs, HostWorkdir: host, @@ -207,7 +213,7 @@ exec sh -c "$(printf '%s' "$last" | sed "s#git-upload-pack '/\(.*\)\.git'#git up NoSkipCheckout: tc.noSkip, Env: map[string]string{ "PATH": tools + string(os.PathListSeparator) + os.Getenv("PATH"), "GIT_CONFIG_GLOBAL": globalConfig, "GIT_CONFIG_NOSYSTEM": "1", - "GIT_DIR": host, "GITHUB_OUTPUT": output, "SSH_SERVER": server, "SSH_ARGS": filepath.Join(runnerTemp, "ssh-args"), + "GIT_DIR": host, "GITHUB_OUTPUT": output, "SSH_SERVER": filepath.ToSlash(server), "SSH_ARGS": filepath.Join(runnerTemp, "ssh-args"), }, }) } @@ -270,7 +276,7 @@ exec sh -c "$(printf '%s' "$last" | sed "s#git-upload-pack '/\(.*\)\.git'#git up for _, repo := range repos { assert.Equal(t, tc.inputs["clean"] == "false", fileExists(filepath.Join(repo, "untracked")), repo) assert.Len(t, strings.Fields(gitOutput(repo, "config", "--local", "--get-all", credentialsInclude)), 1, repo) - headers, _ := exec.Command("git", "-C", repo, "config", "--get-all", "http.file://"+server+"/.extraheader").Output() + headers, _ := exec.Command("git", "-C", repo, "config", "--get-all", "http."+serverURL+"/.extraheader").Output() assert.Equal(t, persisted, string(headers) == "\nAuthorization: Basic dG9rZW46dG9r\n", repo) } if tc.inputs["ssh-key"] != "" { @@ -354,3 +360,25 @@ func fileExists(name string) bool { _, err := os.Stat(name) return err == nil } + +func TestWorkspaceDestination(t *testing.T) { + for _, tc := range []struct { + path string + windows bool + want string + }{ + {`..\x`, true, ""}, + {`C:x`, true, ""}, + {`a\b`, true, "/w/a/b"}, + {`..\x`, false, `/w/..\x`}, + {`a:b`, false, "/w/a:b"}, + } { + got, err := workspaceDestination("/w", tc.path, tc.windows) + if tc.want == "" { + assert.ErrorContains(t, err, "must be relative", tc.path) + } else { + require.NoError(t, err, tc.path) + assert.Equal(t, tc.want, got, tc.path) + } + } +} diff --git a/internal/pkg/config/config.example.yaml b/internal/pkg/config/config.example.yaml index 0b8891dc..9313b7c7 100644 --- a/internal/pkg/config/config.example.yaml +++ b/internal/pkg/config/config.example.yaml @@ -327,8 +327,8 @@ health_check: # Minimum free space required on the filesystem holding runner workspaces. # Defaults to 1024 MiB when omitted or set to zero. #min_free_disk_space_mb: 1024 - # Optional additional executable. A non-zero exit, timeout, or startup failure - # marks the runner unavailable. + # Optional additional executable, or a .bat or .cmd file on Windows. A non-zero + # exit, timeout, or startup failure marks the runner unavailable. #script: '' # How long a script result is cached and its maximum execution time. #interval: 30s diff --git a/internal/pkg/config/config_test.go b/internal/pkg/config/config_test.go index 3650a82a..61446c6f 100644 --- a/internal/pkg/config/config_test.go +++ b/internal/pkg/config/config_test.go @@ -109,7 +109,7 @@ cache: endpoint: "http://minio:9000" bucket: "caches" prefix: "/runners/" - access_key_file: "` + accessPath + `" + access_key_file: '` + accessPath + `' secret_key: "secret" ` require.NoError(t, os.WriteFile(path, []byte(content), 0o600)) @@ -341,20 +341,20 @@ func TestLoadDefault_ExternalCacheSecret(t *testing.T) { {name: "without secret", yaml: externalCache, wantErr: "external_secret"}, {name: "inline secret", yaml: externalCache + " external_secret: \"shh\"\n"}, { - name: "from file", yaml: externalCache + " external_secret_file: \"$DIR/cache.secret\"\n", + name: "from file", yaml: externalCache + " external_secret_file: '$DIR/cache.secret'\n", fileContent: " s3cr3t\n", wantSecret: "s3cr3t", }, { - name: "from file when disabled", yaml: "\ncache:\n enabled: false\n external_secret_file: \"$DIR/cache.secret\"\n", + name: "from file when disabled", yaml: "\ncache:\n enabled: false\n external_secret_file: '$DIR/cache.secret'\n", fileContent: "s3cr3t", wantSecret: "s3cr3t", }, { - name: "inline and file conflict", yaml: externalCache + " external_secret: \"inline\"\n external_secret_file: \"$DIR/cache.secret\"\n", + name: "inline and file conflict", yaml: externalCache + " external_secret: \"inline\"\n external_secret_file: '$DIR/cache.secret'\n", fileContent: "s3cr3t", wantErr: "both set", }, - {name: "missing file", yaml: externalCache + " external_secret_file: \"$DIR/absent.secret\"\n", wantErr: "read cache.external_secret_file"}, + {name: "missing file", yaml: externalCache + " external_secret_file: '$DIR/absent.secret'\n", wantErr: "read cache.external_secret_file"}, { - name: "empty file", yaml: externalCache + " external_secret_file: \"$DIR/cache.secret\"\n", + name: "empty file", yaml: externalCache + " external_secret_file: '$DIR/cache.secret'\n", fileContent: "\n \n", wantErr: "contains no secret", }, } { diff --git a/internal/pkg/process/console_others.go b/internal/pkg/process/console_others.go new file mode 100644 index 00000000..3b3d4862 --- /dev/null +++ b/internal/pkg/process/console_others.go @@ -0,0 +1,8 @@ +// Copyright 2026 The Gitea Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +//go:build !windows + +package process + +func UTF8Console() {} diff --git a/internal/pkg/process/console_windows.go b/internal/pkg/process/console_windows.go new file mode 100644 index 00000000..16f13bcb --- /dev/null +++ b/internal/pkg/process/console_windows.go @@ -0,0 +1,11 @@ +// Copyright 2026 The Gitea Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package process + +import "golang.org/x/sys/windows" + +func UTF8Console() { // per step, as GitHub's runner runs chcp 65001 before its steps + _ = windows.SetConsoleCP(65001) + _ = windows.SetConsoleOutputCP(65001) +} diff --git a/internal/pkg/process/group_windows.go b/internal/pkg/process/group_windows.go index 6c9e719a..5f5bf232 100644 --- a/internal/pkg/process/group_windows.go +++ b/internal/pkg/process/group_windows.go @@ -11,9 +11,7 @@ import ( "golang.org/x/sys/windows" ) -// Group is a job-scoped Windows Job Object holding every process the job's steps -// start; closing it terminates whatever is still assigned, reaching orphans the -// per-step Killer misses (a completed step's leftover has no parent to walk from). +// Group holds tracked step processes in a job-scoped Windows Job Object. type Group struct { mu sync.Mutex job windows.Handle diff --git a/internal/pkg/process/group_windows_test.go b/internal/pkg/process/group_windows_test.go index 7f4a1a38..fd0ecbdb 100644 --- a/internal/pkg/process/group_windows_test.go +++ b/internal/pkg/process/group_windows_test.go @@ -4,69 +4,25 @@ package process import ( - "fmt" - "os" "os/exec" - "path/filepath" - "strconv" - "strings" "testing" "time" "github.com/stretchr/testify/require" ) -// startOrphanMaker starts a process that spawns a detached, long-lived child and -// exits, leaving the child parentless. Returns the cmd and the child's PID file. -func startOrphanMaker(t *testing.T) (*exec.Cmd, string) { - t.Helper() - - pidFile := filepath.Join(t.TempDir(), "child.pid") - script := fmt.Sprintf( - `$c = Start-Process powershell -PassThru -ArgumentList '-NoProfile','-Command','Start-Sleep -Seconds 600'; `+ - `Set-Content -LiteralPath %q -Value $c.Id`, pidFile) - - cmd := exec.Command("powershell.exe", "-NoProfile", "-Command", script) - require.NoError(t, cmd.Start()) - t.Cleanup(func() { _ = cmd.Process.Kill() }) - - return cmd, pidFile -} - -// awaitChildPID waits until the spawned child has reported its PID and is running. -func awaitChildPID(t *testing.T, pidFile string) int { - t.Helper() - - var childPID int - require.Eventually(t, func() bool { - b, err := os.ReadFile(pidFile) - if err != nil { - return false - } - s := strings.TrimSpace(string(b)) - if s == "" { - return false - } - childPID, _ = strconv.Atoi(s) - return childPID > 0 && processAlive(childPID) - }, 20*time.Second, 200*time.Millisecond, "child process should start") - - return childPID -} - -// TestGroupClosePropagatesToOrphan covers what the per-step Killer cannot: a -// completed step's orphan, reachable only by closing the job object. func TestGroupClosePropagatesToOrphan(t *testing.T) { group, err := NewGroup() require.NoError(t, err) t.Cleanup(func() { _ = group.Close() }) - cmd, pidFile := startOrphanMaker(t) + cmd, ready := newTestProcess(t, "orphan", false) + require.NoError(t, cmd.Start()) require.NoError(t, group.Assign(cmd.Process)) - childPID := awaitChildPID(t, pidFile) + childPID := ready() - require.NoError(t, cmd.Wait()) // the step process exits cleanly, like a passing step + require.NoError(t, cmd.Wait()) require.Eventually(t, func() bool { return !processAlive(cmd.Process.Pid) }, 20*time.Second, 200*time.Millisecond, "the step process should have exited on its own") diff --git a/internal/pkg/process/killer_windows_test.go b/internal/pkg/process/killer_windows_test.go index 04f315a9..0aed87c6 100644 --- a/internal/pkg/process/killer_windows_test.go +++ b/internal/pkg/process/killer_windows_test.go @@ -4,9 +4,6 @@ package process import ( - "fmt" - "os/exec" - "path/filepath" "testing" "time" @@ -29,20 +26,18 @@ func processAlive(pid int) bool { } func TestKillerKillsTree(t *testing.T) { - pidFile := filepath.Join(t.TempDir(), "child.pid") - - script := fmt.Sprintf( - `$c = Start-Process powershell -PassThru -ArgumentList '-NoProfile','-Command','Start-Sleep -Seconds 600'; `+ - `Set-Content -LiteralPath %q -Value $c.Id; Start-Sleep -Seconds 600`, pidFile) - cmd := exec.Command("powershell.exe", "-NoProfile", "-Command", script) + cmd, ready := newTestProcess(t, "parent", false) require.NoError(t, cmd.Start()) - t.Cleanup(func() { _ = cmd.Process.Kill() }) killer, err := NewKiller(cmd.Process) require.NoError(t, err) - defer killer.Close() + t.Cleanup(func() { + _ = killer.Kill() + _ = killer.Close() + }) - childPID := awaitChildPID(t, pidFile) + childPID := ready() + require.True(t, processAlive(childPID), "child process should start") require.NoError(t, killer.Kill()) diff --git a/internal/pkg/process/leftover_others.go b/internal/pkg/process/leftover_others.go index 24778310..789e4189 100644 --- a/internal/pkg/process/leftover_others.go +++ b/internal/pkg/process/leftover_others.go @@ -7,8 +7,6 @@ package process import "context" -// KillProcessesWithCWDUnder is a no-op outside Windows: an open handle does not -// block os.RemoveAll there. -func KillProcessesWithCWDUnder(_ context.Context, _ []string) (int, error) { +func KillProcessesWithPaths(_ctx context.Context, _dirs []string, _trackingID string) (int, error) { return 0, nil } diff --git a/internal/pkg/process/leftover_windows.go b/internal/pkg/process/leftover_windows.go index 46c1d450..9de40724 100644 --- a/internal/pkg/process/leftover_windows.go +++ b/internal/pkg/process/leftover_windows.go @@ -8,16 +8,14 @@ import ( "errors" "fmt" "os" + "strings" "unsafe" "golang.org/x/sys/windows" ) -// KillProcessesWithCWDUnder terminates every process in the runner's session -// whose cwd is one of dirs or below, catching leftovers the path scan misses -// (Win32_Process exposes no cwd) that still pin a handle. Best-effort. -func KillProcessesWithCWDUnder(ctx context.Context, dirs []string) (int, error) { - if len(dirs) == 0 { +func KillProcessesWithPaths(ctx context.Context, dirs []string, trackingID string) (int, error) { + if len(dirs) == 0 || trackingID == "" { return 0, nil } @@ -38,7 +36,6 @@ func KillProcessesWithCWDUnder(ctx context.Context, dirs []string) (int, error) if ctx.Err() != nil { break } - // Never touch ourselves, System Idle (0) or System (4). if pid == self || pid == 0 || pid == 4 { continue } @@ -46,7 +43,7 @@ func KillProcessesWithCWDUnder(ctx context.Context, dirs []string) (int, error) if err := windows.ProcessIdToSessionId(pid, &session); err != nil || session != ownSession { continue } - ok, err := terminateIfCWDUnder(pid, dirs) + ok, err := terminateIfPathsMatch(pid, dirs, trackingID) if err != nil { errs = append(errs, err) continue @@ -59,29 +56,50 @@ func KillProcessesWithCWDUnder(ctx context.Context, dirs []string) (int, error) return killed, errors.Join(errs...) } -// terminateIfCWDUnder opens pid once and both reads its cwd and terminates it -// through that one handle, so a PID recycled between the two cannot be hit. -func terminateIfCWDUnder(pid uint32, dirs []string) (bool, error) { - h, err := windows.OpenProcess(windows.PROCESS_QUERY_INFORMATION|windows.PROCESS_VM_READ|windows.PROCESS_TERMINATE, false, pid) +func terminateIfPathsMatch(pid uint32, dirs []string, trackingID string) (bool, error) { + handle, err := windows.OpenProcess(windows.PROCESS_QUERY_INFORMATION|windows.PROCESS_VM_READ|windows.PROCESS_TERMINATE, false, pid) if err != nil { - return false, nil // best-effort: gone, higher integrity, or another user - } - defer func() { _ = windows.CloseHandle(h) }() - - cwd, err := processCWDHandle(h) - if err != nil || cwd == "" { return false, nil } + defer func() { _ = windows.CloseHandle(handle) }() - for _, d := range dirs { - if dirContainsPath(d, cwd) { - if err := windows.TerminateProcess(h, 1); err != nil { - return false, fmt.Errorf("terminate pid %d: %w", pid, err) + params, err := processParametersHandle(handle) + if err != nil { + return false, nil + } + matches := false +fields: + for _, field := range []struct { + offset uintptr + wow64 uintptr + command bool + }{ + {unsafe.Offsetof(windows.RTL_USER_PROCESS_PARAMETERS{}.CurrentDirectory), 0x24, false}, + {unsafe.Offsetof(windows.RTL_USER_PROCESS_PARAMETERS{}.ImagePathName), 0x38, false}, + {unsafe.Offsetof(windows.RTL_USER_PROCESS_PARAMETERS{}.CommandLine), 0x40, true}, + } { + value, err := params.unicodeString(handle, field.offset, field.wow64) + if err != nil { + continue + } + for _, dir := range dirs { + if !field.command && dirContainsPath(dir, value) || field.command && strings.Contains(strings.ToLower(value), strings.ToLower(strings.TrimRight(dir, `\/`)+`\`)) { + matches = true + break fields } - return true, nil } } - return false, nil + if !matches { + return false, nil + } + value, err := params.environmentVariable(handle, "RUNNER_TRACKING_ID") + if err != nil || value != trackingID { + return false, nil + } + if err := windows.TerminateProcess(handle, 1); err != nil { + return false, fmt.Errorf("terminate pid %d: %w", pid, err) + } + return true, nil } // listProcessIDs returns the PIDs of every process in a Toolhelp snapshot. @@ -107,65 +125,135 @@ func listProcessIDs() ([]uint32, error) { return pids, nil } -// processCWDHandle reads a process's working directory from its PEB (walking -// PEB -> ProcessParameters -> CurrentDirectory.DosPath). h must carry -// PROCESS_QUERY_INFORMATION|PROCESS_VM_READ. -func processCWDHandle(h windows.Handle) (string, error) { +type remoteParameters struct { + address uintptr + wow64 bool +} + +func processParametersHandle(handle windows.Handle) (remoteParameters, error) { var pbi windows.PROCESS_BASIC_INFORMATION var retLen uint32 - if err := windows.NtQueryInformationProcess(h, windows.ProcessBasicInformation, unsafe.Pointer(&pbi), uint32(unsafe.Sizeof(pbi)), &retLen); err != nil { - return "", err - } - if pbi.PebBaseAddress == nil { - return "", errors.New("nil PEB base address") + if err := windows.NtQueryInformationProcess(handle, windows.ProcessBasicInformation, unsafe.Pointer(&pbi), uint32(unsafe.Sizeof(pbi)), &retLen); err != nil { + return remoteParameters{}, err } pebAddr := uintptr(unsafe.Pointer(pbi.PebBaseAddress)) - - paramsAddr, err := readRemotePtr(h, pebAddr+unsafe.Offsetof(windows.PEB{}.ProcessParameters)) - if err != nil || paramsAddr == 0 { - return "", err + wow64 := false + if unsafe.Sizeof(uintptr(0)) == 8 { + var peb32 uintptr + if err := windows.NtQueryInformationProcess(handle, windows.ProcessWow64Information, unsafe.Pointer(&peb32), uint32(unsafe.Sizeof(peb32)), &retLen); err != nil { + return remoteParameters{}, err + } + if peb32 != 0 { + pebAddr = peb32 + wow64 = true + } } + if pebAddr == 0 { + return remoteParameters{}, errors.New("nil PEB base address") + } + params := remoteParameters{wow64: wow64} + address, err := params.pointer(handle, pebAddr+params.offset(unsafe.Offsetof(windows.PEB{}.ProcessParameters), 0x10)) + if err != nil { + return remoteParameters{}, err + } + if address == 0 { + return remoteParameters{}, errors.New("nil process parameters") + } + params.address = address + return params, nil +} - // DosPath is the leading NTUnicodeString of CURDIR, so it shares CurrentDirectory's address. - dosPathAddr := paramsAddr + unsafe.Offsetof(windows.RTL_USER_PROCESS_PARAMETERS{}.CurrentDirectory) - length, err := readRemoteU16(h, dosPathAddr+unsafe.Offsetof(windows.NTUnicodeString{}.Length)) +func (p remoteParameters) offset(native, wow64 uintptr) uintptr { + if p.wow64 { + return wow64 + } + return native +} + +func (p remoteParameters) pointer(handle windows.Handle, address uintptr) (uintptr, error) { + var value uintptr + size := unsafe.Sizeof(value) + if p.wow64 { + size = 4 + } + var read uintptr + if err := windows.ReadProcessMemory(handle, address, (*byte)(unsafe.Pointer(&value)), size, &read); err != nil { + return 0, err + } + if read != size { + return 0, errors.New("short pointer read") + } + return value, nil +} + +func (p remoteParameters) unicodeString(handle windows.Handle, native, wow64 uintptr) (string, error) { + address := p.address + p.offset(native, wow64) + length, err := readRemoteU16(handle, address) if err != nil || length == 0 { return "", err } - bufAddr, err := readRemotePtr(h, dosPathAddr+unsafe.Offsetof(windows.NTUnicodeString{}.Buffer)) - if err != nil || bufAddr == 0 { + buffer, err := p.pointer(handle, address+p.offset(unsafe.Offsetof(windows.NTUnicodeString{}.Buffer), 4)) + if err != nil { return "", err } - - u16 := make([]uint16, (length+1)/2) // round up so an odd Length cannot overflow the buffer - var n uintptr - if err := windows.ReadProcessMemory(h, bufAddr, (*byte)(unsafe.Pointer(&u16[0])), uintptr(length), &n); err != nil { + value := make([]uint16, (int(length)+1)/2) + var read uintptr + if err := windows.ReadProcessMemory(handle, buffer, (*byte)(unsafe.Pointer(&value[0])), uintptr(length), &read); err != nil { return "", err } - if n != uintptr(length) { - return "", errors.New("short read of working directory") + if read != uintptr(length) { + return "", errors.New("short string read") } - return windows.UTF16ToString(u16), nil + return windows.UTF16ToString(value), nil } -// readRemotePtr reads a single pointer-sized value from another process. -func readRemotePtr(h windows.Handle, addr uintptr) (uintptr, error) { - var v uintptr - var n uintptr - if err := windows.ReadProcessMemory(h, addr, (*byte)(unsafe.Pointer(&v)), unsafe.Sizeof(v), &n); err != nil { - return 0, err +func (p remoteParameters) environmentVariable(handle windows.Handle, name string) (string, error) { + address, err := p.pointer(handle, p.address+p.offset(unsafe.Offsetof(windows.RTL_USER_PROCESS_PARAMETERS{}.Environment), 0x48)) + if err != nil || address == 0 { + return "", err } - if n != unsafe.Sizeof(v) { - return 0, errors.New("short pointer read") + var entry []uint16 + for consumed := uintptr(0); consumed < 32*1024*1024; { + var region windows.MemoryBasicInformation + if err := windows.VirtualQueryEx(handle, address+consumed, ®ion, unsafe.Sizeof(region)); err != nil { + return "", err + } + size := min(uintptr(4096), region.BaseAddress+region.RegionSize-address-consumed) + if size < 2 { + return "", errors.New("invalid environment region") + } + buffer := make([]uint16, size/2) + var read uintptr + if err := windows.ReadProcessMemory(handle, address+consumed, (*byte)(unsafe.Pointer(&buffer[0])), uintptr(len(buffer)*2), &read); err != nil { + return "", err + } + if read != uintptr(len(buffer)*2) { + return "", errors.New("short environment read") + } + for _, value := range buffer { + if value != 0 { + entry = append(entry, value) + continue + } + if len(entry) == 0 { + return "", nil + } + key, value, ok := strings.Cut(windows.UTF16ToString(entry), "=") + if ok && strings.EqualFold(key, name) { + return value, nil + } + entry = entry[:0] + } + consumed += read } - return v, nil + return "", errors.New("environment exceeds read limit") } // readRemoteU16 reads a single uint16 from another process. -func readRemoteU16(h windows.Handle, addr uintptr) (uint16, error) { +func readRemoteU16(handle windows.Handle, addr uintptr) (uint16, error) { var v uint16 var n uintptr - if err := windows.ReadProcessMemory(h, addr, (*byte)(unsafe.Pointer(&v)), unsafe.Sizeof(v), &n); err != nil { + if err := windows.ReadProcessMemory(handle, addr, (*byte)(unsafe.Pointer(&v)), unsafe.Sizeof(v), &n); err != nil { return 0, err } if n != unsafe.Sizeof(v) { diff --git a/internal/pkg/process/leftover_windows_test.go b/internal/pkg/process/leftover_windows_test.go index 5f899244..d9374a07 100644 --- a/internal/pkg/process/leftover_windows_test.go +++ b/internal/pkg/process/leftover_windows_test.go @@ -7,21 +7,21 @@ import ( "os" "os/exec" "path/filepath" + "runtime" "syscall" "testing" "time" + "unsafe" "github.com/stretchr/testify/require" "golang.org/x/sys/windows" ) -// startSleeperIn launches a long-running child whose cwd is dir but whose -// executable lives outside it — what the path scan cannot see. -func startSleeperIn(t *testing.T, dir string) *exec.Cmd { +func startSleeperIn(t *testing.T, system, dir, trackingID string) *exec.Cmd { t.Helper() - ping := filepath.Join(os.Getenv("SystemRoot"), "System32", "ping.exe") - cmd := exec.Command(ping, "-n", "60", "127.0.0.1") + cmd := exec.Command(filepath.Join(os.Getenv("SystemRoot"), system, "ping.exe"), "-n", "60", "127.0.0.1") cmd.Dir = dir + cmd.Env = append(os.Environ(), "RUNNER_TRACKING_ID="+trackingID) cmd.SysProcAttr = &syscall.SysProcAttr{HideWindow: true} require.NoError(t, cmd.Start()) t.Cleanup(func() { @@ -31,19 +31,19 @@ func startSleeperIn(t *testing.T, dir string) *exec.Cmd { return cmd } -// processCWD opens pid and reads its working directory. Test-only; the reaper -// shares one handle across read and kill via processCWDHandle. func processCWD(pid uint32) (string, error) { h, err := windows.OpenProcess(windows.PROCESS_QUERY_INFORMATION|windows.PROCESS_VM_READ, false, pid) if err != nil { return "", err } defer func() { _ = windows.CloseHandle(h) }() - return processCWDHandle(h) + params, err := processParametersHandle(h) + if err != nil { + return "", err + } + return params.unicodeString(h, unsafe.Offsetof(windows.RTL_USER_PROCESS_PARAMETERS{}.CurrentDirectory), 0x24) } -// awaitCWD waits until pid's PEB reports a readable cwd (not populated the -// instant the process starts) and returns it. func awaitCWD(t *testing.T, pid int) string { t.Helper() var cwd string @@ -55,36 +55,52 @@ func awaitCWD(t *testing.T, pid int) string { return cwd } -func TestProcessCWDReadsWorkingDirectory(t *testing.T) { - dir := t.TempDir() - cmd := startSleeperIn(t, dir) - cwd := awaitCWD(t, cmd.Process.Pid) - require.True(t, dirContainsPath(dir, cwd), "cwd %q should be within %q", cwd, dir) +func TestKillProcessesWithPathsTracking(t *testing.T) { + for _, system := range []string{"System32", "SysWOW64"} { + for _, trackingID := range []string{"leftover-test", "", "another-job"} { + t.Run(system+"/tracking="+trackingID, func(t *testing.T) { + if system == "SysWOW64" { + if runtime.GOARCH != "amd64" && runtime.GOARCH != "arm64" { + t.Skip("needs a 64-bit runner") + } + if _, err := os.Stat(filepath.Join(os.Getenv("SystemRoot"), system, "ping.exe")); err != nil { + t.Skip("32-bit Windows tools unavailable") + } + } + dir := t.TempDir() + cmd := startSleeperIn(t, system, dir, trackingID) + require.True(t, dirContainsPath(dir, awaitCWD(t, cmd.Process.Pid))) + killed, err := KillProcessesWithPaths(t.Context(), []string{t.TempDir()}, "leftover-test") + require.NoError(t, err) + require.Zero(t, killed) + killed, err = KillProcessesWithPaths(t.Context(), []string{dir}, "leftover-test") + require.NoError(t, err) + if trackingID == "leftover-test" { + require.Equal(t, 1, killed) + require.Eventually(t, func() bool { + return !processAlive(cmd.Process.Pid) + }, 5*time.Second, 50*time.Millisecond) + } else { + require.Zero(t, killed) + require.True(t, processAlive(cmd.Process.Pid)) + } + }) + } + } } -func TestKillProcessesWithCWDUnderTerminatesMatch(t *testing.T) { - dir := t.TempDir() - // Executable outside dir and arguments carry no workspace path, so only the - // working directory links this process to dir. - cmd := startSleeperIn(t, dir) - awaitCWD(t, cmd.Process.Pid) - - killed, err := KillProcessesWithCWDUnder(t.Context(), []string{dir}) +func TestKillProcessesWithPathsCommandLine(t *testing.T) { + workspace := t.TempDir() + cmd, ready := newTestProcess(t, "child", false) + cmd.Dir = t.TempDir() + cmd.Args = append(cmd.Args, filepath.Join(workspace, "reference")) + cmd.Env = append(cmd.Env, "RUNNER_TRACKING_ID=command-test") + require.NoError(t, cmd.Start()) + ready() + killed, err := KillProcessesWithPaths(t.Context(), []string{workspace}, "command-test") require.NoError(t, err) - require.GreaterOrEqual(t, killed, 1) - + require.Equal(t, 1, killed) require.Eventually(t, func() bool { return !processAlive(cmd.Process.Pid) - }, 5*time.Second, 50*time.Millisecond, "matched process should exit") -} - -func TestKillProcessesWithCWDUnderSparesUnrelated(t *testing.T) { - workspace := t.TempDir() - elsewhere := t.TempDir() - cmd := startSleeperIn(t, elsewhere) - awaitCWD(t, cmd.Process.Pid) - - _, err := KillProcessesWithCWDUnder(t.Context(), []string{workspace}) - require.NoError(t, err) - require.True(t, processAlive(cmd.Process.Pid), "process outside owned dirs must be spared") + }, 5*time.Second, 50*time.Millisecond) } diff --git a/internal/pkg/process/treekill.go b/internal/pkg/process/treekill.go index ca2c029a..f1ec8d43 100644 --- a/internal/pkg/process/treekill.go +++ b/internal/pkg/process/treekill.go @@ -32,7 +32,8 @@ const treeKillWaitDelay = 10 * time.Second // Callers still set cmd.SysProcAttr (via SysProcAttr) themselves, because the // value differs between the plain and PTY execution paths. type TreeKill struct { - killer atomic.Pointer[Killer] + killer atomic.Pointer[Killer] + KeepLeftovers bool } // NewTreeKill sets cmd.Cancel and cmd.WaitDelay. Call it before cmd.Start, then @@ -72,13 +73,13 @@ func (t *TreeKill) Capture(p *os.Process) (*Killer, error) { return k, nil } -// Wait is cmd.Wait, but like GitHub's runner a leftover holding a successful command's output does not fail it, and is killed only on Windows. +// Wait is cmd.Wait, but like GitHub's runner a leftover holding a successful command's output does not fail it. func (t *TreeKill) Wait(ctx context.Context, cmd *exec.Cmd) error { err := cmd.Wait() if !errors.Is(err, exec.ErrWaitDelay) || ctx.Err() != nil { return err } - if runtime.GOOS == "windows" { + if runtime.GOOS == "windows" && !t.KeepLeftovers { _ = t.killer.Load().Kill() } return nil diff --git a/internal/pkg/process/treekill_test.go b/internal/pkg/process/treekill_test.go index 099963fb..97c1b1ca 100644 --- a/internal/pkg/process/treekill_test.go +++ b/internal/pkg/process/treekill_test.go @@ -5,8 +5,11 @@ package process import ( "context" + "fmt" "io" + "os" "os/exec" + "os/signal" "runtime" "testing" "time" @@ -60,11 +63,10 @@ func TestTreeKillWaitWithLeftoverHoldingOutput(t *testing.T) { } func TestTreeKillCaptureStoresKiller(t *testing.T) { - cmd := exec.CommandContext(t.Context(), "sleep", "10") - cmd.SysProcAttr = SysProcAttr("", false) + cmd, ready := newTestProcess(t, "child", false) tk := NewTreeKill(cmd) require.NoError(t, cmd.Start()) - defer func() { _ = cmd.Wait() }() + ready() killer, err := tk.Capture(cmd.Process) require.NoError(t, err) @@ -72,3 +74,84 @@ func TestTreeKillCaptureStoresKiller(t *testing.T) { require.NoError(t, cmd.Cancel()) require.NoError(t, killer.Close()) } + +func newTestProcess(t *testing.T, mode string, copiedOutput bool) (*exec.Cmd, func() int) { + t.Helper() + ctx, cancel := context.WithTimeout(t.Context(), 20*time.Second) + t.Cleanup(cancel) + cmd := exec.CommandContext(ctx, os.Args[0], "-test.run=^TestProcessHelper$") + cmd.Env = append(os.Environ(), "GITEA_PROCESS_HELPER="+mode) + cmd.SysProcAttr = SysProcAttr("", false) + cmd.Stderr = os.Stderr + input, err := cmd.StdinPipe() + require.NoError(t, err) + var output io.ReadCloser + if copiedOutput { + var writer *io.PipeWriter + output, writer = io.Pipe() + cmd.Stdout = writer + } else { + output, err = cmd.StdoutPipe() + require.NoError(t, err) + } + stop := context.AfterFunc(ctx, func() { + _ = input.Close() + _ = output.Close() + }) + t.Cleanup(func() { + stop() + _ = input.Close() + _ = output.Close() + if cmd.Process != nil { + _ = cmd.Process.Kill() + _ = cmd.Wait() + } + }) + return cmd, func() int { + t.Helper() + _, err := input.Write([]byte{1}) + require.NoError(t, err) + var childPID int + _, err = fmt.Fscan(output, &childPID) + require.NoError(t, err, "process should report its PID") + require.Positive(t, childPID) + return childPID + } +} + +func TestProcessHelper(t *testing.T) { + mode := os.Getenv("GITEA_PROCESS_HELPER") + switch mode { + case "": + return + case "orphan-child": + interrupt := make(chan os.Signal, 1) + signal.Notify(interrupt, os.Interrupt) + _, err := fmt.Fprintln(os.Stdout, os.Getpid()) + require.NoError(t, err) + <-interrupt + case "child": + _, err := fmt.Fprintln(os.Stdout, os.Getpid()) + require.NoError(t, err) + _, err = io.Copy(io.Discard, os.Stdin) + require.NoError(t, err) + default: + var start [1]byte + _, err := io.ReadFull(os.Stdin, start[:]) + require.NoError(t, err) + child := exec.Command(os.Args[0], "-test.run=^TestProcessHelper$") + child.Env = append(os.Environ(), "GITEA_PROCESS_HELPER=child") + if mode == "orphan" { + child.Env = append(os.Environ(), "GITEA_PROCESS_HELPER=orphan-child") + } + child.Stdin = os.Stdin + child.Stdout = os.Stdout + child.Stderr = os.Stderr + require.NoError(t, child.Start()) + if mode == "orphan" { + os.Exit(0) + } + require.NoError(t, child.Wait()) + } + os.Exit(0) +} diff --git a/internal/pkg/process/treekill_windows_test.go b/internal/pkg/process/treekill_windows_test.go new file mode 100644 index 00000000..9deb3d3c --- /dev/null +++ b/internal/pkg/process/treekill_windows_test.go @@ -0,0 +1,45 @@ +// Copyright 2026 The Gitea Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package process + +import ( + "testing" + "time" + + "github.com/stretchr/testify/require" + "golang.org/x/sys/windows" +) + +func TestTreeKillWaitTrackingLeftoverOutput(t *testing.T) { + for _, keep := range []bool{false, true} { + t.Run(map[bool]string{false: "tracked", true: "opted out"}[keep], func(t *testing.T) { + cmd, ready := newTestProcess(t, "orphan", true) + tree := NewTreeKill(cmd) + tree.KeepLeftovers = keep + cmd.WaitDelay = 50 * time.Millisecond + require.NoError(t, cmd.Start()) + killer, err := tree.Capture(cmd.Process) + require.NoError(t, err) + defer killer.Close() + childPID := ready() + handle, err := windows.OpenProcess(windows.SYNCHRONIZE|windows.PROCESS_TERMINATE, false, uint32(childPID)) + require.NoError(t, err) + t.Cleanup(func() { + _ = windows.TerminateProcess(handle, 1) + _, _ = windows.WaitForSingleObject(handle, 5000) + _ = windows.CloseHandle(handle) + }) + require.NoError(t, tree.Wait(t.Context(), cmd)) + if keep { + result, err := windows.WaitForSingleObject(handle, 0) + require.NoError(t, err) + require.EqualValues(t, windows.WAIT_TIMEOUT, result) + } else { + result, err := windows.WaitForSingleObject(handle, 5000) + require.NoError(t, err) + require.EqualValues(t, windows.WAIT_OBJECT_0, result) + } + }) + } +} diff --git a/scripts/test-windows-setup.ps1 b/scripts/test-windows-setup.ps1 new file mode 100644 index 00000000..ed1f4a3f --- /dev/null +++ b/scripts/test-windows-setup.ps1 @@ -0,0 +1,29 @@ +$ErrorActionPreference = 'Stop' +$ProgressPreference = 'SilentlyContinue' +$shared = '\\host.lan\Data' +try { + Write-Output '==> Setting up Windows' + Set-ItemProperty HKLM:\SYSTEM\CurrentControlSet\Control\FileSystem LongPathsEnabled 1 + foreach ($tool in @( + @('git.7z.exe', 'https://github.com/git-for-windows/git/releases/download/v2.56.0.windows.3/PortableGit-2.56.0.3-64-bit.7z.exe', '4fc27b779829a36df77911e3f4dea09c66c790dd6c5f7dfa3b6a5479f1fe354f'), + @('node.zip', 'https://nodejs.org/dist/v24.22.1/node-v24.22.1-win-x64.zip', 'd2046d28c2c3dcd211ac5145c0bf30e2047ef9dfb306693c8c6e2eccd8b95fa5'), + @('pwsh.zip', 'https://github.com/PowerShell/PowerShell/releases/download/v7.6.8/PowerShell-7.6.8-win-x64.zip', 'b9f074f923c499b51ec99d313da8e9ae90d17a9f3adf00c73d6189f34d908528'))) { + Write-Output "==> Downloading $($tool[1])" + Invoke-WebRequest -UseBasicParsing $tool[1] -OutFile "$env:TEMP\$($tool[0])" + if ((Get-FileHash "$env:TEMP\$($tool[0])").Hash -ne $tool[2]) { throw "checksum mismatch for $($tool[1])" } + } + Write-Output '==> Extracting git, node and pwsh' + New-Item -ItemType Directory C:\pwsh | Out-Null + @( + Start-Process "$env:TEMP\git.7z.exe" -ArgumentList '-oC:\git', '-y' -PassThru + Start-Process tar.exe -ArgumentList '-xf', "$env:TEMP\node.zip", '-C', 'C:\' -PassThru -NoNewWindow + Start-Process tar.exe -ArgumentList '-xf', "$env:TEMP\pwsh.zip", '-C', 'C:\pwsh' -PassThru -NoNewWindow + ) | Wait-Process + Rename-Item (Resolve-Path C:\node-*-win-x64).Path C:\node + $env:Path = "C:\git\cmd;C:\git\bin;C:\pwsh;C:\node;$env:Path" + Get-Command git, bash, node, pwsh | Out-Null +} catch { + $_ | Out-String | Tee-Object "$shared\error.txt" + Set-Content "$shared\exitcode" 1 + exit 1 +} diff --git a/scripts/test-windows.ps1 b/scripts/test-windows.ps1 new file mode 100644 index 00000000..e5b1fa17 --- /dev/null +++ b/scripts/test-windows.ps1 @@ -0,0 +1,31 @@ +$ErrorActionPreference = 'Stop' +$ProgressPreference = 'SilentlyContinue' +$results = '\\host.lan\Data' +$root = $PSScriptRoot +try { + New-Item -ItemType Directory "$root\logs" | Out-Null + $env:Path = "C:\git\cmd;C:\git\bin;C:\pwsh;C:\node;$env:Path" + git config --global --add safe.directory C:/OEM/src # setup copied it as SYSTEM, the tests run as the logged-on user + + $failed = 0 + Write-Output "==> Running $((Get-Content "$root\tests.txt").Count) test binaries on $([Environment]::ProcessorCount) cores" + Get-Content "$root\tests.txt" | ForEach-Object -ThrottleLimit ([Environment]::ProcessorCount) -Parallel { + $binary, $package = $_ -split ' ', 2 + $started = Get-Date + Set-Location "$using:root\src\$package" + cmd.exe /d /c "$using:root\bin\$binary -test.v -test.timeout=60m -test.parallel=8 > $using:root\logs\$($package -replace '/', '-').log 2>&1" + [pscustomobject]@{ Package = $package; ExitCode = $LASTEXITCODE; Seconds = [int]((Get-Date) - $started).TotalSeconds } + } | ForEach-Object { + $line = "ok $($_.Package) $($_.Seconds)s" + if ($_.ExitCode -ne 0) { + $line = "FAIL $($_.Package) $($_.Seconds)s exit $($_.ExitCode)" + $failed = 1 + } + Add-Content "$results\summary.txt" $line + } + Copy-Item "$root\logs\*" $results +} catch { + $_ | Out-String | Tee-Object "$results\error.txt" + $failed = 1 +} +Set-Content "$results\exitcode" $failed diff --git a/scripts/test-windows.sh b/scripts/test-windows.sh new file mode 100755 index 00000000..4fae56c8 --- /dev/null +++ b/scripts/test-windows.sh @@ -0,0 +1,90 @@ +#!/usr/bin/env bash +set -euo pipefail + +image="${WINDOWS_TEST_IMAGE:?}" +timeout="${WINDOWS_TEST_TIMEOUT:-165}" +name="gitea-runner-windows-test-$$" +work="$(mktemp -d)" + +cleanup() { + docker rm -fv "$name" >/dev/null 2>&1 || true + rm -rf "$work" +} +trap cleanup EXIT + +if [ "$(docker info -f '{{.Architecture}}')" != x86_64 ]; then + echo "error: the Windows test VM needs an amd64 docker host" >&2 + exit 1 +fi + +echo "==> Building Windows test binaries" +oem="$work/oem" +mkdir -p "$oem/bin" "$oem/src" "$work/shared" +module="$(go list -m)" +packages="$(GOOS=windows go list -f '{{if or .TestGoFiles .XTestGoFiles}}{{.ImportPath}}{{end}}' ./...)" +GOOS=windows GOARCH=amd64 go test -c -ldflags='-s -w' -o "$oem/bin/" $packages +for package in $packages; do + echo "${package##*/}.test.exe ${package#"$module"/}" +done > "$oem/tests.txt" +git ls-files -z --cached --others --exclude-standard | while IFS= read -r -d '' file; do + if [ -e "$file" ]; then printf '%s\0' "$file"; fi +done | tar --null -T - -cf - | tar -xf - -C "$oem/src" +git -C "$oem/src" init -q -b main # a repository like the CI checkout, for the tests that read the github context from it +git -C "$oem/src" add -Af . +git -C "$oem/src" -c user.name=test -c user.email=test@localhost -c commit.gpgsign=false -c core.hooksPath=/dev/null commit -qm test +git -C "$oem/src" remote add origin https://gitea.com/gitea/runner +cp scripts/test-windows-setup.ps1 scripts/test-windows.ps1 "$oem/" +sed 's/$/\r/' > "$oem/install.bat" <<'EOF' # a policy turns off real-time scanning, AMSI blocks scripts that do it +@echo off +set log=\\host.lan\Data\install.log +echo ==^> install.bat started at %TIME% >> %log% +reg.exe add "HKLM\SOFTWARE\Policies\Microsoft\Windows Defender\Real-Time Protection" /v DisableRealtimeMonitoring /t REG_DWORD /d 1 /f > nul +powershell.exe -NoProfile -ExecutionPolicy Bypass -File C:\OEM\test-windows-setup.ps1 >> %log% 2>&1 +echo ==^> setup exited with %ERRORLEVEL% >> %log% +if %ERRORLEVEL% equ 0 C:\pwsh\pwsh.exe -NoProfile -ExecutionPolicy Bypass -File C:\OEM\test-windows.ps1 >> %log% 2>&1 +if not exist \\host.lan\Data\exitcode (echo 1)> \\host.lan\Data\exitcode +shutdown.exe /s /t 600 +EOF + +kvm=Y cores="$(nproc)" +if ! docker run --rm --privileged --entrypoint sh "$image" -c 'test -w /dev/kvm'; then + kvm=N cores=4 # more emulated vCPUs slow Windows down + echo "==> The docker host has no usable /dev/kvm, Windows runs emulated and much slower" +fi + +echo "==> Installing Windows" +docker create --rm --name "$name" --privileged -e VERSION=2025 -e EDITION=core -e KVM="$kvm" -e CPU_CORES="$cores" -e RAM_SIZE=8G \ + -e DISK_SIZE=32G -e DISK_CACHE=unsafe -e DISK_IO=threads --entrypoint timeout "$image" "$((timeout + 10))m" /usr/bin/tini -s /run/entry.sh >/dev/null # the timeout stops the VM even when this script is killed before its cleanup runs +docker cp "$oem" "$name:/oem" +docker cp "$work/shared" "$name:/shared" +docker start "$name" >/dev/null +docker logs -f "$name" & +while sleep 10 && docker container inspect "$name" >/dev/null 2>&1; do + if ((SECONDS % 300 < 10)); then + echo "==> $((SECONDS / 60))m elapsed, VM disk $(docker exec "$name" du -sh /storage | cut -f1)" + fi +done & +docker exec "$name" sh -c 'cd /shared && touch install.log summary.txt && tail -qf install.log summary.txt' & + +if ! timeout "${timeout}m" docker exec "$name" sh -c 'until [ -f /shared/exitcode ]; do sleep 10; done'; then + echo "Windows stopped or did not finish the tests within $timeout minutes" >&2 + exit 1 +fi +results="$work/results" +docker cp "$name:/shared" "$results" + +tr -d '\r' < "$results/summary.txt" | while read -r result package _; do + if [ "$result" = FAIL ]; then + echo "::group::$package" + cat "$results/${package//\//-}.log" + echo "::endgroup::" + fi +done +if [ -f "$results/error.txt" ]; then + cat "$results/error.txt" >&2 +fi +echo "==> Slowest tests" +find "$results" -name '*.log' -exec cat {} + | tr -d '\r' | sed -nE 's/^--- (PASS|FAIL): (\S+) \(([0-9.]+)s\)$/\3 \2/p' | sort -rn | awk 'NR <= 20' +echo "==> Packages" +tr -d '\r' < "$results/summary.txt" +exit "$(tr -d '\r\n' < "$results/exitcode")"