test: make timing-sensitive tests deterministic

internal/security/mask_test.go

@@ -247,30 +247,78 @@ func TestMaskTextMasksBeyondTheDetectionCap(t *testing.T) {
 	}
 }
 
-// Removing the replacement cap makes "a payload stuffed with secrets" the
-// worst case, so it must not be quadratic: a full-size activity response
-// (64KB, the activity_max_response_size cap) of nothing but distinct tokens
-// still has to mask in well under a second.
-func TestMaskTextStaysCheapOnAPayloadFullOfSecrets(t *testing.T) {
-	d := NewDetector(nil)
-
+// secretsPayload builds text of at least targetLen bytes made entirely of
+// distinct GitHub-PAT-shaped tokens ("key<i>=ghp_<i zero-padded to 36 digits>"),
+// the worst case for masking: every token is a separate match, none share a
+// value, so no dedup/caching in the detector can shortcut the work.
+func secretsPayload(targetLen int) string {
 	var b strings.Builder
-	for i := 0; b.Len() < 64*1024; i++ {
+	for i := 0; b.Len() < targetLen; i++ {
 		fmt.Fprintf(&b, "key%d=ghp_%036d\n", i, i)
 	}
-	text := b.String()
+	return b.String()
+}
+
+// A full-size activity response made entirely of distinct tokens exercises the
+// worst-case masking shape without making correctness depend on host timing.
+func TestMaskTextHandlesAPayloadFullOfSecrets(t *testing.T) {
+	d := NewDetector(nil)
+
+	text := secretsPayload(64 * 1024)
 
-	start := time.Now()
 	masked, _ := d.MaskText(text)
-	elapsed := time.Since(start)
 
 	if strings.Contains(masked, "ghp_0000") {
 		t.Fatal("tokens survived masking")
 	}
-	if elapsed > 2*time.Second {
-		t.Fatalf("masking a %d-byte payload took %s", len(text), elapsed)
+}
+
+// TestMaskTextScalesLinearlyOnAPayloadFullOfSecrets guards against
+// reintroducing the O(n^2) behavior TestMaskTextStaysCheapOnAPayloadFullOfSecrets
+// used to catch (removed because an absolute wall-clock ceiling was flaky under
+// CI load — see BenchmarkMaskTextPayloadFullOfSecrets below for the profiling
+// counterpart). Instead of a fixed threshold, it compares masking a payload
+// against one 4x the size: linear work predicts ~4x the time, so a generous
+// growth ceiling still catches real quadratic blowups (~16x) while absorbing
+// routine scheduler/GC noise from the comparison running in the same process.
+func TestMaskTextScalesLinearlyOnAPayloadFullOfSecrets(t *testing.T) {
+	if testing.Short() {
+		t.Skip("timing-sensitive; skipped under -short")
+	}
+
+	measure := func(targetLen int) time.Duration {
+		d := NewDetector(nil)
+		text := secretsPayload(targetLen)
+		start := time.Now()
+		d.MaskText(text)
+		return time.Since(start)
+	}
+
+	// Warm up (first run pays for allocator/CPU-cache warmup, not algorithmic cost).
+	measure(16 * 1024)
+
+	small := measure(16 * 1024)
+	large := measure(64 * 1024) // 4x the payload
+
+	// The absolute floor keeps this from firing on noise when both runs are
+	// too fast for the ratio to mean anything.
+	if large > 10*small && large > 50*time.Millisecond {
+		t.Fatalf("masking a payload 4x the size took %s vs %s for the baseline — looks like a complexity regression, not scheduler noise", large, small)
+	}
+}
+
+func BenchmarkMaskTextPayloadFullOfSecrets(b *testing.B) {
+	d := NewDetector(nil)
+	text := secretsPayload(64 * 1024)
+
+	b.SetBytes(int64(len(text)))
+	b.ReportAllocs()
+	b.ResetTimer()
+	for range b.N {
+		if _, changed := d.MaskText(text); !changed {
+			b.Fatal("payload was not masked")
+		}
 	}
-	t.Logf("masked %d bytes in %s", len(text), elapsed)
 }
 
 // Turning detection off later must not retroactively serve the credentials in

internal/upstream/manager.go

@@ -46,6 +46,16 @@ var dockerResolverFn = func(logger *zap.Logger) (string, error) {
 	return shellwrap.ResolveDockerPath(logger)
 }
 
+func runDockerInfo(ctx context.Context, dockerBin string) error {
+	return exec.CommandContext(ctx, dockerBin, "info", "--format", "{{json .ServerVersion}}").Run()
+}
+
+// dockerInfoRunnerFn runs the resolved docker binary's `info` command.
+// Overridable in tests, mirroring dockerResolverFn above so the two sibling
+// fakes checkDockerAvailability needs (which binary, and running it) share one
+// injection idiom instead of two.
+var dockerInfoRunnerFn = runDockerInfo
+
 // Docker recovery constants - internal implementation defaults
 const (
 	dockerCheckInterval      = 30 * time.Second // How often to check Docker availability
@@ -3035,8 +3045,7 @@ func (m *Manager) checkDockerAvailability(ctx context.Context) error {
 		dockerBin = "docker"
 	}
 
-	cmd := exec.CommandContext(checkCtx, dockerBin, "info", "--format", "{{json .ServerVersion}}")
-	if err := cmd.Run(); err != nil {
+	if err := dockerInfoRunnerFn(checkCtx, dockerBin); err != nil {
 		return fmt.Errorf("docker unavailable: %w", err)
 	}
 	return nil

internal/upstream/manager_docker_recovery_test.go

@@ -15,90 +15,131 @@ import (
 	"github.com/smart-mcp-proxy/mcpproxy-go/internal/storage"
 )
 
+// TestRunDockerInfo_ExecutesResolvedBinary exercises runDockerInfo's real
+// exec.CommandContext(...).Run() invocation shape end-to-end against a real
+// subprocess, so a regression in the argument/flag shape (wrong dockerBin
+// position, broken --format string) is caught even though the tests below
+// inject dockerInfoRunnerFn and never call this function.
+func TestRunDockerInfo_ExecutesResolvedBinary(t *testing.T) {
+	if runtime.GOOS == "windows" {
+		t.Skip("/bin/sh-style fake docker is POSIX only")
+	}
+
+	dockerPath := filepath.Join(t.TempDir(), "docker")
+	// Requires the exact argv runDockerInfo passes (not just $1 == info), so a
+	// regression in argument count, the --format flag, or the Go template
+	// string fails this test instead of slipping through unnoticed.
+	script := `#!/bin/sh
+if [ "$#" -eq 3 ] && [ "$1" = "info" ] && [ "$2" = "--format" ] && [ "$3" = '{{json .ServerVersion}}' ]; then
+  printf '"24.0.0"\n'
+  exit 0
+fi
+exit 99
+`
+	if err := os.WriteFile(dockerPath, []byte(script), 0o755); err != nil {
+		t.Fatalf("write fake docker: %v", err)
+	}
+
+	ctx, cancel := context.WithTimeout(context.Background(), 2*time.Second)
+	defer cancel()
+
+	if err := runDockerInfo(ctx, dockerPath); err != nil {
+		t.Fatalf("expected fake docker info to succeed, got: %v", err)
+	}
+}
+
+// TestRunDockerInfo_PropagatesNonZeroExit ensures a real non-zero process exit
+// (not just a synthetic Go error) surfaces as an error from runDockerInfo.
+func TestRunDockerInfo_PropagatesNonZeroExit(t *testing.T) {
+	if runtime.GOOS == "windows" {
+		t.Skip("/bin/sh-style fake docker is POSIX only")
+	}
+
+	dockerPath := filepath.Join(t.TempDir(), "docker")
+	if err := os.WriteFile(dockerPath, []byte("#!/bin/sh\nexit 7\n"), 0o755); err != nil {
+		t.Fatalf("write fake docker: %v", err)
+	}
+
+	ctx, cancel := context.WithTimeout(context.Background(), 2*time.Second)
+	defer cancel()
+
+	if err := runDockerInfo(ctx, dockerPath); err == nil {
+		t.Fatal("expected non-zero exit to surface as an error")
+	}
+}
+
 // TestCheckDockerAvailability_UsesShellwrapResolver verifies that a launchd-style
 // minimal PATH (the situation when mcpproxy is launched from /Applications/...app
 // or a LoginItem) does not break docker lookup, because checkDockerAvailability
 // now goes through dockerResolverFn (shellwrap-backed) instead of relying on
 // $PATH for the bare "docker" binary.
 //
-// We exercise the real resolver indirectly: the test installs a fake docker
-// shim into a temp dir, points dockerResolverFn at it, drops $PATH to the
-// launchd minimum, and asserts that checkDockerAvailability still succeeds.
-// Without the fix, exec.Command("docker") would error with `executable file
-// not found in $PATH` regardless of how the resolver was wired.
+// dockerInfoRunnerFn is faked so the test asserts the resolved executable
+// directly without depending on process startup or a wall-clock deadline; the
+// real exec.CommandContext(...).Run() shape is covered separately by
+// TestRunDockerInfo_ExecutesResolvedBinary above.
 func TestCheckDockerAvailability_UsesShellwrapResolver(t *testing.T) {
 	if runtime.GOOS == "windows" {
 		t.Skip("launchd PATH scenario is macOS/Linux only")
 	}
 
-	tmpDir := t.TempDir()
-	dockerPath := filepath.Join(tmpDir, "docker")
-
-	// Fake docker that prints a server version when invoked with `info`.
-	// Anything else exits non-zero so we know the call shape was correct.
-	script := `#!/bin/sh
-case "$1" in
-  info) printf '"24.0.0"\n'; exit 0 ;;
-  *)    exit 99 ;;
-esac
-`
-	if err := os.WriteFile(dockerPath, []byte(script), 0o755); err != nil {
-		t.Fatalf("write fake docker: %v", err)
-	}
-
-	// Simulate launchd's minimal PATH — fake docker is NOT on it. If the
-	// production code resolved via os.Getenv("PATH") we would fail here.
-	t.Setenv("PATH", "/usr/bin:/bin:/usr/sbin:/sbin")
+	dockerPath := filepath.Join(t.TempDir(), "docker")
 
 	original := dockerResolverFn
 	t.Cleanup(func() { dockerResolverFn = original })
 	dockerResolverFn = func(*zap.Logger) (string, error) {
 		return dockerPath, nil
 	}
 
+	var resolvedBin string
+	originalRunner := dockerInfoRunnerFn
+	t.Cleanup(func() { dockerInfoRunnerFn = originalRunner })
+	dockerInfoRunnerFn = func(_ context.Context, dockerBin string) error {
+		resolvedBin = dockerBin
+		return nil
+	}
+
 	m := &Manager{logger: zap.NewNop()}
-	ctx, cancel := context.WithTimeout(context.Background(), 2*time.Second)
-	defer cancel()
 
-	if err := m.checkDockerAvailability(ctx); err != nil {
+	if err := m.checkDockerAvailability(context.Background()); err != nil {
 		t.Fatalf("expected docker to be reachable via resolver, got: %v", err)
 	}
+	if resolvedBin != dockerPath {
+		t.Fatalf("docker executable = %q, want resolved path %q", resolvedBin, dockerPath)
+	}
 }
 
 // TestCheckDockerAvailability_FallbackOnResolverFailure ensures that even when
-// the resolver itself errors out we still attempt a bare-name exec — preserving
+// the resolver itself errors out we still select the bare executable name — preserving
 // the original behaviour for hosts where the shellwrap probes legitimately
 // cannot find docker but it IS on the parent's PATH.
 func TestCheckDockerAvailability_FallbackOnResolverFailure(t *testing.T) {
 	if runtime.GOOS == "windows" {
 		t.Skip("/bin/sh-style fake docker is POSIX only")
 	}
 
-	tmpDir := t.TempDir()
-	dockerPath := filepath.Join(tmpDir, "docker")
-	script := `#!/bin/sh
-exit 0
-`
-	if err := os.WriteFile(dockerPath, []byte(script), 0o755); err != nil {
-		t.Fatalf("write fake docker: %v", err)
-	}
-
-	// Resolver fails — the fallback should still exec via bare PATH.
-	t.Setenv("PATH", tmpDir)
-
 	original := dockerResolverFn
 	t.Cleanup(func() { dockerResolverFn = original })
 	dockerResolverFn = func(*zap.Logger) (string, error) {
 		return "", errors.New("simulated resolver failure")
 	}
 
+	var resolvedBin string
+	originalRunner := dockerInfoRunnerFn
+	t.Cleanup(func() { dockerInfoRunnerFn = originalRunner })
+	dockerInfoRunnerFn = func(_ context.Context, dockerBin string) error {
+		resolvedBin = dockerBin
+		return nil
+	}
+
 	m := &Manager{logger: zap.NewNop()}
-	ctx, cancel := context.WithTimeout(context.Background(), 2*time.Second)
-	defer cancel()
 
-	if err := m.checkDockerAvailability(ctx); err != nil {
+	if err := m.checkDockerAvailability(context.Background()); err != nil {
 		t.Fatalf("expected fallback bare-name exec to succeed, got: %v", err)
 	}
+	if resolvedBin != "docker" {
+		t.Fatalf("docker executable = %q, want bare-name fallback", resolvedBin)
+	}
 }
 
 // TestFreshenLoadedDockerRecoveryState verifies that a state loaded from the