From c1d3686f9a2bedffcca0bf532421342383a9f014 Mon Sep 17 00:00:00 2001
From: Cuong Manh Le <cuong@windscribe.com>
Date: Mon, 15 Jun 2026 15:50:12 +0700
Subject: [PATCH] fix: stop self-upgrade tests from fork-bombing the windows
 test runner

The test:windows CI job intermittently failed to clean up .testbin with
"Access to the path '...cmd_cli.test.exe' is denied". This was previously
attributed to Windows Defender scanning the large unsigned test binaries,
and mitigated with Defender exclusions and cleanup retries. That was
treating a symptom.

Root cause: performUpgrade() self-upgrades by running
exec.Command(os.Executable(), "upgrade", "prod", "-vv") as a detached,
windowless child. In the real ctrld binary this re-execs ctrld and is
correct. Under `go test`, os.Executable() is the test binary itself, and
`go test` stops flag parsing at the first positional arg ("upgrade") and
ignores the rest -- so the child silently re-runs the entire test suite.
That child hits the upgrade tests again and spawns more detached children,
recursively: a fork bomb of hidden processes that pins the runner's
CPU/memory and keeps the test binary's image file locked. Windows refuses
to delete the image of a running process, hence the "Access is denied"
during after_script. Whether any children are still alive when cleanup
runs is a timing race, which is why the failure was flaky.

Two tests reached this path: Test_performUpgrade (directly) and
Test_selfUpgradeCheck (via selfUpgradeCheck -> performUpgrade on the
"upgrade allowed" case).

Fix:
- prog.go: extract the command construction into a package-level
  newUpgradeCmd var. Production behavior is unchanged.
- main_test.go: stub newUpgradeCmd once in TestMain so the whole test
  binary self-execs with `-test.run=^$` (matches no tests, exits
  immediately) instead of re-running the suite. This covers every test
  that reaches performUpgrade, present and future, while still exercising
  the cmd.Start() success path.
---
 cmd/cli/main_test.go | 16 ++++++++++++++++
 cmd/cli/prog.go      | 16 ++++++++++++++--
 cmd/cli/prog_test.go |  2 ++
 3 files changed, 32 insertions(+), 2 deletions(-)

diff --git a/cmd/cli/main_test.go b/cmd/cli/main_test.go
index d0a1149..55de3b0 100644
--- a/cmd/cli/main_test.go
+++ b/cmd/cli/main_test.go
@@ -2,6 +2,7 @@ package cli
 
 import (
 	"os"
+	"os/exec"
 	"strings"
 	"testing"
 
@@ -28,5 +29,20 @@ func TestMain(m *testing.M) {
 	l := zap.New(core)
 
 	mainLog.Store(&ctrld.Logger{Logger: l})
+
+	// Stub the self-upgrade command builder for the whole test binary. The real
+	// builder execs os.Executable() — which under `go test` IS this test binary
+	// — with positional args ("upgrade", ...). `go test` stops flag parsing at
+	// the first positional arg and ignores the rest, so the child just re-runs
+	// the entire suite, hits the upgrade tests again, and spawns more children:
+	// a fork bomb of detached processes that stalls the host and (on Windows)
+	// holds the test binary's image locked, breaking CI artifact cleanup.
+	// Point it at the test binary with a no-match -test.run so any test that
+	// reaches performUpgrade still exercises the cmd.Start() success path while
+	// the child exits immediately without recursing.
+	newUpgradeCmd = func(exe string) *exec.Cmd {
+		return exec.Command(exe, "-test.run=^$")
+	}
+
 	os.Exit(m.Run())
 }
diff --git a/cmd/cli/prog.go b/cmd/cli/prog.go
index 98a34a8..66c2f15 100644
--- a/cmd/cli/prog.go
+++ b/cmd/cli/prog.go
@@ -1574,6 +1574,19 @@ func shouldUpgrade(vt string, cv *semver.Version, logger *ctrld.Logger) bool {
 	return true
 }
 
+// newUpgradeCmd builds the detached command used to self-upgrade. It is a
+// package-level variable so tests can stub it. With the real implementation a
+// *test* binary would re-exec itself — os.Executable() is the test binary, and
+// because `go test` stops flag parsing at the first positional arg ("upgrade")
+// it ignores the args and re-runs the entire suite. That child hits the same
+// upgrade test and spawns another child, recursively: a fork bomb of detached
+// processes that pins the host and locks the test binary's image file.
+var newUpgradeCmd = func(exe string) *exec.Cmd {
+	cmd := exec.Command(exe, "upgrade", "prod", "-vv")
+	cmd.SysProcAttr = sysProcAttrForDetachedChildProcess()
+	return cmd
+}
+
 // performUpgrade executes the self-upgrade command.
 // Returns true if upgrade was initiated successfully, false otherwise.
 func performUpgrade(vt string, logger *ctrld.Logger) bool {
@@ -1582,8 +1595,7 @@ func performUpgrade(vt string, logger *ctrld.Logger) bool {
 		logger.Error().Err(err).Msg("Failed to get executable path, skipped self-upgrade")
 		return false
 	}
-	cmd := exec.Command(exe, "upgrade", "prod", "-vv")
-	cmd.SysProcAttr = sysProcAttrForDetachedChildProcess()
+	cmd := newUpgradeCmd(exe)
 	if err := cmd.Start(); err != nil {
 		logger.Error().Err(err).Msg("Failed to start self-upgrade")
 		return false
diff --git a/cmd/cli/prog_test.go b/cmd/cli/prog_test.go
index eccc30b..f30dcb8 100644
--- a/cmd/cli/prog_test.go
+++ b/cmd/cli/prog_test.go
@@ -253,6 +253,8 @@ func Test_performUpgrade(t *testing.T) {
 		},
 	}
 
+	// newUpgradeCmd is stubbed in TestMain so performUpgrade does not re-exec
+	// (and fork-bomb) the test binary; see the comment there.
 	for _, tc := range tests {
 		tc := tc
 		t.Run(tc.name, func(t *testing.T) {