From 7bec31000208c1f60e4a41862db52d907db782ff Mon Sep 17 00:00:00 2001 From: bircni Date: Wed, 22 Jul 2026 15:04:07 +0000 Subject: [PATCH] fix: stop host-mode jobs from leaking processes on Windows (#1080) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Problem On host-mode Windows runners, a job can leave processes running after it finishes, and those leftovers hold file handles that block deletion of the workspace. Today a step's tree is only torn down when the step is *cancelled* (`process.Killer`). A step that completes leaves whatever it spawned alive, and the existing workspace scan in `terminateRunningProcesses` misses two shapes of leftover: orphans whose parent already exited (no tree to walk, and their executable often lives outside the workspace), and processes that merely *run in* the workspace but reference no path from it — `Win32_Process` exposes no working directory, so the scan cannot match them. ## Solution Two additions in `internal/pkg/process`, both no-ops outside Windows: - **`process.Group`** — a job-scoped Windows Job Object created with `JOB_OBJECT_LIMIT_KILL_ON_JOB_CLOSE`. Step processes are assigned to it before they get their own `Killer`, so the step's job nests inside the job's: cancellation still kills exactly the step's tree, while `Remove` closing the group makes the kernel terminate everything still assigned, whatever its parentage. The kernel also drops the handle when the runner exits, so a crashed runner cannot strand processes. - **`process.KillProcessesWithCWDUnder`** — a best-effort net for processes that never joined the job (started via a service or scheduled task). It reads each process's working directory from its PEB and terminates those under a workspace dir. Processes it cannot open are skipped. --------- Co-authored-by: silverwind Reviewed-on: https://gitea.com/gitea/runner/pulls/1080 Reviewed-by: silverwind <2021+silverwind@noreply.gitea.com> --- act/container/host_environment.go | 104 +++++----- internal/pkg/process/group_others.go | 28 +++ internal/pkg/process/group_windows.go | 86 +++++++++ internal/pkg/process/group_windows_test.go | 113 +++++++++++ internal/pkg/process/leftover.go | 29 +++ internal/pkg/process/leftover_others.go | 14 ++ internal/pkg/process/leftover_test.go | 41 ++++ internal/pkg/process/leftover_windows.go | 182 ++++++++++++++++++ internal/pkg/process/leftover_windows_test.go | 91 +++++++++ 9 files changed, 645 insertions(+), 43 deletions(-) create mode 100644 internal/pkg/process/group_others.go create mode 100644 internal/pkg/process/group_windows.go create mode 100644 internal/pkg/process/group_windows_test.go create mode 100644 internal/pkg/process/leftover.go create mode 100644 internal/pkg/process/leftover_others.go create mode 100644 internal/pkg/process/leftover_test.go create mode 100644 internal/pkg/process/leftover_windows.go create mode 100644 internal/pkg/process/leftover_windows_test.go diff --git a/act/container/host_environment.go b/act/container/host_environment.go index bf52e09e..6937e39f 100644 --- a/act/container/host_environment.go +++ b/act/container/host_environment.go @@ -17,6 +17,7 @@ import ( "path/filepath" "runtime" "strings" + "sync" "sync/atomic" "time" @@ -43,6 +44,25 @@ type HostEnvironment struct { CleanUp func() StdOut io.Writer AllocatePTY bool // allocate a pseudo-TTY for each step's process + + // procGroup owns every process the job's steps start. Atomic: Remove may read + // it while a step is still starting. + procGroupOnce sync.Once + procGroup atomic.Pointer[process.Group] +} + +// processGroup returns the job-scoped process group, creating it on first use. +// Returns nil if the job object could not be created; Group is nil-safe. +func (e *HostEnvironment) processGroup(ctx context.Context) *process.Group { + e.procGroupOnce.Do(func() { + group, err := process.NewGroup() + if err != nil { + common.Logger(ctx).Warnf("could not create the job's process group; processes a step leaves behind can only be reclaimed by the workspace scan: %v", err) + return + } + e.procGroup.Store(group) + }) + return e.procGroup.Load() } func (e *HostEnvironment) Create(_, _ []string) common.Executor { @@ -324,11 +344,8 @@ func (e *HostEnvironment) exec(ctx context.Context, command []string, cmdline st cmd.Dir = wd cmd.SysProcAttr = process.SysProcAttr(cmdline, false) - // Kill the step's whole process tree on cancellation (a step often launches a - // shell that spawns further background or GUI children) and bound the post-exit - // I/O wait, so an orphan inheriting cmd's stdout/stderr pipe can never hang - // cmd.Wait() and the runner. See process.TreeKill. The PTY path below may - // override SysProcAttr, but never touches Cancel/WaitDelay. + // 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) var ppty *os.File @@ -360,6 +377,11 @@ 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 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) } else { @@ -407,13 +429,12 @@ func (e *HostEnvironment) UpdateFromEnv(srcPath string, env *map[string]string) return parseEnvFile(e, srcPath, env) } -// removeAll is the filesystem delete used by removeAllWithContext. A package -// var so tests can substitute a blocking stub without patching os.RemoveAll. +// removeAll is a var so tests can substitute a blocking stub. var removeAll = os.RemoveAll -// removeAllWithContext runs removeAll in a goroutine and returns once it -// finishes or ctx is cancelled. On cancellation the goroutine is left running — -// a delete blocked inside a syscall cannot be interrupted (see runWithTimeout). +// 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). func removeAllWithContext(ctx context.Context, path string) error { done := make(chan error, 1) go func() { done <- removeAll(path) }() @@ -455,17 +476,12 @@ func removePathWithRetry(ctx context.Context, path string) error { return lastErr } -// buildWindowsWorkspaceKillScript builds a PowerShell command that `taskkill -// /T /F`s every process tree whose ExecutablePath or CommandLine references one -// of the given absolute workspace dirs, releasing file handles for cleanup. -// -// Win32_Process is used because it exposes both ExecutablePath and CommandLine -// (Get-Process doesn't, wmic is deprecated). Both match the dir+separator -// prefix, so a sibling dir sharing a name prefix (job1 vs job10) is spared. -// Ordinal String methods, not -like, so path metacharacters ([ ] ? *) stay -// literal. -// -// Pure function so the quote-escaping can be unit-tested without PowerShell. +// 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 { @@ -501,9 +517,8 @@ func (e *HostEnvironment) terminateRunningProcesses(ctx context.Context) { logger := common.Logger(ctx) - // Workspace dirs we own. Any process running from or referencing one is a - // leftover job process. ToolCache is shared across jobs; Workdir only when - // we own it (else it's a caller-provided checkout, e.g. act local mode). + // Dirs we own; a process referencing one is a leftover. ToolCache is shared + // across jobs, and Workdir may be a caller-owned checkout. owned := []string{e.Path, e.TmpDir} if e.CleanWorkdir { owned = append(owned, e.Workdir) @@ -530,21 +545,24 @@ func (e *HostEnvironment) terminateRunningProcesses(ctx context.Context) { 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) + } else if killed > 0 { + logger.Debugf("terminated %d leftover process(es) by workspace working directory", killed) + } } -// hostCleanupTimeout bounds each filesystem-teardown phase of the host -// environment so a single stalled delete cannot wedge the runner slot forever. -// A var (not const) so tests can shrink it. +// hostCleanupTimeout bounds each teardown phase so one stalled delete cannot +// wedge the runner slot. A var so tests can shrink it. var hostCleanupTimeout = 30 * time.Second -// runWithTimeout runs fn in a goroutine and returns once it finishes or timeout -// elapses, whichever comes first. On timeout the goroutine is left running — an -// os.RemoveAll blocked inside a delete syscall (AV/EDR filter drivers, an -// unresponsive network mount, a dying disk) cannot be interrupted — and -// context.DeadlineExceeded is returned. Leaking the goroutine and the scratch -// state it was deleting is strictly better than blocking the caller forever and -// permanently losing the runner's capacity slot; the leaked scratch dir is -// reclaimed later by the runner's idle stale-dir sweep. +// 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() { @@ -565,14 +583,15 @@ func (e *HostEnvironment) Remove() common.Executor { return func(ctx context.Context) error { logger := common.Logger(ctx) - // Ensure any lingering child processes are ended before attempting - // to remove the workspace (Windows file locks otherwise prevent cleanup). + // End lingering processes before removing the workspace; on Windows their + // file locks block cleanup. Closing the group is deterministic, the scan a net. + if err := e.procGroup.Load().Close(); err != nil { + logger.Debugf("closing the job's process group failed: %v", err) + } e.terminateRunningProcesses(ctx) - // Only removes per-job misc state. Must not remove the cache/toolcache root. - // Bound it: CleanUp is a caller-supplied, typically unbounded os.RemoveAll, - // and a delete stalled by a filesystem filter driver would otherwise hang - // the job forever at "Cleaning up container" and hold the capacity slot. + // 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 { @@ -603,8 +622,7 @@ func (e *HostEnvironment) Remove() common.Executor { return errors.Join(errs...) } } - // Bounded teardown timed out; warnings already logged above. Do not - // fail job completion — leaked scratch is reclaimed by the idle sweep. + // Teardown timed out; warned above. Do not fail job completion over it. return nil } } diff --git a/internal/pkg/process/group_others.go b/internal/pkg/process/group_others.go new file mode 100644 index 00000000..11d2a9b6 --- /dev/null +++ b/internal/pkg/process/group_others.go @@ -0,0 +1,28 @@ +// Copyright 2026 The Gitea Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +//go:build !windows + +package process + +import "os" + +// Group is a no-op outside Windows: process groups already reclaim a step's +// tree, and an orphan does not block workspace deletion the way an open Windows +// handle does. +type Group struct{} + +// NewGroup returns a Group that owns nothing. +func NewGroup() (*Group, error) { + return &Group{}, nil +} + +// Assign does nothing. +func (g *Group) Assign(_ *os.Process) error { + return nil +} + +// Close does nothing. +func (g *Group) Close() error { + return nil +} diff --git a/internal/pkg/process/group_windows.go b/internal/pkg/process/group_windows.go new file mode 100644 index 00000000..6c9e719a --- /dev/null +++ b/internal/pkg/process/group_windows.go @@ -0,0 +1,86 @@ +// Copyright 2026 The Gitea Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package process + +import ( + "os" + "sync" + "unsafe" + + "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). +type Group struct { + mu sync.Mutex + job windows.Handle +} + +// NewGroup creates the job object. Closing it is what terminates the leftovers, +// so the caller must Close it when the job ends. +func NewGroup() (*Group, error) { + job, err := windows.CreateJobObject(nil, nil) + if err != nil { + return nil, err + } + + info := windows.JOBOBJECT_EXTENDED_LIMIT_INFORMATION{ + BasicLimitInformation: windows.JOBOBJECT_BASIC_LIMIT_INFORMATION{ + LimitFlags: windows.JOB_OBJECT_LIMIT_KILL_ON_JOB_CLOSE, + }, + } + if _, err := windows.SetInformationJobObject( + job, + windows.JobObjectExtendedLimitInformation, + uintptr(unsafe.Pointer(&info)), + uint32(unsafe.Sizeof(info)), + ); err != nil { + _ = windows.CloseHandle(job) + return nil, err + } + + return &Group{job: job}, nil +} + +// Assign adds a started process to the job; anything it spawns afterwards joins +// too. Call before NewKiller so the step's job nests inside this one. Nil-safe. +func (g *Group) Assign(p *os.Process) error { + if g == nil || p == nil { + return nil + } + + g.mu.Lock() + defer g.mu.Unlock() + if g.job == 0 { + return nil + } + + h, err := windows.OpenProcess(windows.PROCESS_SET_QUOTA|windows.PROCESS_TERMINATE, false, uint32(p.Pid)) + if err != nil { + return err + } + defer func() { _ = windows.CloseHandle(h) }() + + return windows.AssignProcessToJobObject(g.job, h) +} + +// Close drops the job handle, terminating every process still assigned to it. +// Nil-safe and idempotent. +func (g *Group) Close() error { + if g == nil { + return nil + } + + g.mu.Lock() + defer g.mu.Unlock() + if g.job == 0 { + return nil + } + + h := g.job + g.job = 0 + return windows.CloseHandle(h) +} diff --git a/internal/pkg/process/group_windows_test.go b/internal/pkg/process/group_windows_test.go new file mode 100644 index 00000000..7f4a1a38 --- /dev/null +++ b/internal/pkg/process/group_windows_test.go @@ -0,0 +1,113 @@ +// Copyright 2026 The Gitea Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +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) + require.NoError(t, group.Assign(cmd.Process)) + + childPID := awaitChildPID(t, pidFile) + + require.NoError(t, cmd.Wait()) // the step process exits cleanly, like a passing step + 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") + + require.True(t, processAlive(childPID), "orphan should outlive its parent") + + require.NoError(t, group.Close()) + require.Eventually(t, func() bool { + return !processAlive(childPID) + }, 20*time.Second, 200*time.Millisecond, "closing the job must terminate the orphan") +} + +// TestGroupNestsWithKiller: assigned to the group first and the step's Killer +// second, cancelling the step still kills exactly that step's tree. +func TestGroupNestsWithKiller(t *testing.T) { + group, err := NewGroup() + require.NoError(t, err) + t.Cleanup(func() { _ = group.Close() }) + + cmd := exec.Command("powershell.exe", "-NoProfile", "-Command", "Start-Sleep -Seconds 600") + require.NoError(t, cmd.Start()) + t.Cleanup(func() { _ = cmd.Process.Kill() }) + + require.NoError(t, group.Assign(cmd.Process)) + + // Nesting must be accepted, else cancellation falls back to a single-process kill. + killer, err := NewKiller(cmd.Process) + require.NoError(t, err, "the step's job object must nest inside the group's") + defer killer.Close() + + require.NoError(t, killer.Kill()) + require.Eventually(t, func() bool { + return !processAlive(cmd.Process.Pid) + }, 20*time.Second, 200*time.Millisecond, "cancelling the step should still kill its tree") +} + +// TestGroupNilSafe covers the fallback path: when the job object cannot be +// created the caller holds a nil *Group and must still be able to use it. +func TestGroupNilSafe(t *testing.T) { + var group *Group + require.NoError(t, group.Assign(nil)) + require.NoError(t, group.Close()) + require.NoError(t, group.Close()) +} diff --git a/internal/pkg/process/leftover.go b/internal/pkg/process/leftover.go new file mode 100644 index 00000000..396ab10b --- /dev/null +++ b/internal/pkg/process/leftover.go @@ -0,0 +1,29 @@ +// Copyright 2026 The Gitea Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package process + +import ( + "path/filepath" + "strings" +) + +// pathWithin is the OS-agnostic core of dirContainsPath: dir and target must +// already be cleaned, and fold folds case. +func pathWithin(dir, target string, fold bool) bool { + if dir == "" || target == "" || dir == "." || target == "." { + return false + } + if fold { + dir = strings.ToLower(dir) + target = strings.ToLower(target) + } + if dir == target { + return true + } + sep := string(filepath.Separator) + if !strings.HasSuffix(dir, sep) { + dir += sep + } + return strings.HasPrefix(target, dir) +} diff --git a/internal/pkg/process/leftover_others.go b/internal/pkg/process/leftover_others.go new file mode 100644 index 00000000..24778310 --- /dev/null +++ b/internal/pkg/process/leftover_others.go @@ -0,0 +1,14 @@ +// Copyright 2026 The Gitea Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +//go:build !windows + +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) { + return 0, nil +} diff --git a/internal/pkg/process/leftover_test.go b/internal/pkg/process/leftover_test.go new file mode 100644 index 00000000..b547a4ad --- /dev/null +++ b/internal/pkg/process/leftover_test.go @@ -0,0 +1,41 @@ +// Copyright 2026 The Gitea Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package process + +import ( + "path/filepath" + "testing" +) + +func TestPathWithin(t *testing.T) { + base := filepath.Join("base", "job1") + + cases := []struct { + name string + dir string + target string + fold bool + want bool + }{ + {"same dir", base, base, false, true}, + {"direct child", base, filepath.Join(base, "app.exe"), false, true}, + {"deep child", base, filepath.Join(base, "sub", "sub", "app.exe"), false, true}, + {"parent is not within child", filepath.Join(base, "sub"), base, false, false}, + {"name-prefix sibling spared", base, filepath.Join("base", "job10", "app.exe"), false, false}, + {"unrelated", base, filepath.Join("other", "app.exe"), false, false}, + {"empty dir", "", base, false, false}, + {"empty target", base, "", false, false}, + {"dot dir", ".", base, false, false}, + {"case-sensitive miss", base, filepath.Join("base", "JOB1", "app.exe"), false, false}, + {"case-insensitive hit", base, filepath.Join("base", "JOB1", "app.exe"), true, true}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + got := pathWithin(filepath.Clean(tc.dir), filepath.Clean(tc.target), tc.fold) + if got != tc.want { + t.Fatalf("pathWithin(%q, %q, fold=%v) = %v, want %v", tc.dir, tc.target, tc.fold, got, tc.want) + } + }) + } +} diff --git a/internal/pkg/process/leftover_windows.go b/internal/pkg/process/leftover_windows.go new file mode 100644 index 00000000..ae1b5f17 --- /dev/null +++ b/internal/pkg/process/leftover_windows.go @@ -0,0 +1,182 @@ +// Copyright 2026 The Gitea Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package process + +import ( + "context" + "errors" + "fmt" + "os" + "path/filepath" + "unsafe" + + "golang.org/x/sys/windows" +) + +// dirContainsPath reports whether target is dir or lives beneath it, +// case-insensitively, so a name-prefix sibling (job1 vs job10) is not a match. +func dirContainsPath(dir, target string) bool { + return pathWithin(filepath.Clean(dir), filepath.Clean(target), true) +} + +// 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 { + return 0, nil + } + + pids, err := listProcessIDs() + if err != nil { + return 0, err + } + + self := uint32(os.Getpid()) + var ownSession uint32 + if err := windows.ProcessIdToSessionId(self, &ownSession); err != nil { + return 0, fmt.Errorf("look up own session: %w", err) + } + + var killed int + var errs []error + for _, pid := range pids { + if ctx.Err() != nil { + break + } + // Never touch ourselves, System Idle (0) or System (4). + if pid == self || pid == 0 || pid == 4 { + continue + } + var session uint32 + if err := windows.ProcessIdToSessionId(pid, &session); err != nil || session != ownSession { + continue + } + ok, err := terminateIfCWDUnder(pid, dirs) + if err != nil { + errs = append(errs, err) + continue + } + if ok { + killed++ + } + } + + 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) + 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 + } + + 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) + } + return true, nil + } + } + return false, nil +} + +// listProcessIDs returns the PIDs of every process in a Toolhelp snapshot. +func listProcessIDs() ([]uint32, error) { + snap, err := windows.CreateToolhelp32Snapshot(windows.TH32CS_SNAPPROCESS, 0) + if err != nil { + return nil, err + } + defer func() { _ = windows.CloseHandle(snap) }() + + var entry windows.ProcessEntry32 + entry.Size = uint32(unsafe.Sizeof(entry)) + + pids := make([]uint32, 0, 256) + err = windows.Process32First(snap, &entry) + for err == nil { + pids = append(pids, entry.ProcessID) + err = windows.Process32Next(snap, &entry) + } + if !errors.Is(err, windows.ERROR_NO_MORE_FILES) { + return pids, err + } + 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) { + 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") + } + pebAddr := uintptr(unsafe.Pointer(pbi.PebBaseAddress)) + + paramsAddr, err := readRemotePtr(h, pebAddr+unsafe.Offsetof(windows.PEB{}.ProcessParameters)) + if err != nil || paramsAddr == 0 { + return "", err + } + + // 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)) + if err != nil || length == 0 { + return "", err + } + bufAddr, err := readRemotePtr(h, dosPathAddr+unsafe.Offsetof(windows.NTUnicodeString{}.Buffer)) + if err != nil || bufAddr == 0 { + 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 { + return "", err + } + if n != uintptr(length) { + return "", errors.New("short read of working directory") + } + return windows.UTF16ToString(u16), 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 + } + if n != unsafe.Sizeof(v) { + return 0, errors.New("short pointer read") + } + return v, nil +} + +// readRemoteU16 reads a single uint16 from another process. +func readRemoteU16(h 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 { + return 0, err + } + if n != unsafe.Sizeof(v) { + return 0, errors.New("short uint16 read") + } + return v, nil +} diff --git a/internal/pkg/process/leftover_windows_test.go b/internal/pkg/process/leftover_windows_test.go new file mode 100644 index 00000000..9748024a --- /dev/null +++ b/internal/pkg/process/leftover_windows_test.go @@ -0,0 +1,91 @@ +// Copyright 2026 The Gitea Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package process + +import ( + "context" + "os" + "os/exec" + "path/filepath" + "syscall" + "testing" + "time" + + "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 { + t.Helper() + ping := filepath.Join(os.Getenv("SystemRoot"), "System32", "ping.exe") + cmd := exec.Command(ping, "-n", "60", "127.0.0.1") + cmd.Dir = dir + cmd.SysProcAttr = &syscall.SysProcAttr{HideWindow: true} + require.NoError(t, cmd.Start()) + t.Cleanup(func() { + _ = cmd.Process.Kill() + _, _ = cmd.Process.Wait() + }) + 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) +} + +// 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 + require.Eventually(t, func() bool { + var err error + cwd, err = processCWD(uint32(pid)) + return err == nil && cwd != "" + }, 5*time.Second, 50*time.Millisecond, "process cwd should become readable") + 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 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(context.Background(), []string{dir}) + require.NoError(t, err) + require.GreaterOrEqual(t, killed, 1) + + 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(context.Background(), []string{workspace}) + require.NoError(t, err) + require.True(t, processAlive(cmd.Process.Pid), "process outside owned dirs must be spared") +}