diff --git a/.design/project-log/2026-06-02-fix-test-hub-credential-leak.md b/.design/project-log/2026-06-02-fix-test-hub-credential-leak.md new file mode 100644 index 000000000..0678b1276 --- /dev/null +++ b/.design/project-log/2026-06-02-fix-test-hub-credential-leak.md @@ -0,0 +1,25 @@ +# Fix: Test suite leaking Hub credentials (issue #123) + +**Date**: 2026-06-02 +**PR**: #125 +**Issue**: #123 + +## Problem + +When `go test` runs inside an agent container, tests inherit live Hub env vars. `TestInitCommand_Integration` builds and spawns a real sciontool binary that inherits these vars, causing the child process to report status to the real Hub and corrupt agent state (resetting phase to "starting"). This is how dev-issue-71b got stuck. + +## Fix + +1. Added `scrubHubEnv(t)` helpers using `t.Setenv` for automatic cleanup in: + - `cmd/sciontool/commands/init_test.go` (primary subprocess fix) + - `pkg/sciontool/hooks/handlers/hub_test.go` (env var hygiene) + - `pkg/sciontool/hub/client_test.go` (env var hygiene) + +2. Added `filterHubEnv(env)` to explicitly strip Hub vars from subprocess environments. + +3. Converted all `os.Setenv`/`os.Unsetenv` patterns to `t.Setenv` in hub-related test files for crash-safe env isolation. + +## Observations + +- The Hub env var list (`SCION_HUB_ENDPOINT`, `SCION_HUB_URL`, `SCION_AUTH_TOKEN`, `SCION_AGENT_ID`, `SCION_AGENT_MODE`) is defined in `pkg/sciontool/hub/client.go:45-56`. The `scrubHubEnv` helpers are inlined in each test file rather than shared, to avoid importing `testing` into production code. +- Pre-existing CI issue: `pkg/hub/resource_import_handler_test.go` has an undefined `mockRoundTripper` symbol that causes `go vet ./...` to fail — not related to this change. diff --git a/cmd/sciontool/commands/init_test.go b/cmd/sciontool/commands/init_test.go index e24da9ac3..ca6d0282e 100644 --- a/cmd/sciontool/commands/init_test.go +++ b/cmd/sciontool/commands/init_test.go @@ -15,6 +15,45 @@ import ( "testing" ) +// hubEnvVars lists the environment variables used by the Hub client. +// Leaking these to a subprocess (e.g., sciontool init) causes the child +// to talk to the real Hub and corrupt agent state. See issue #123. +var hubEnvVars = []string{ + "SCION_HUB_ENDPOINT", + "SCION_HUB_URL", + "SCION_AUTH_TOKEN", + "SCION_AGENT_ID", + "SCION_AGENT_MODE", +} + +// scrubHubEnv clears all Hub-related environment variables for the +// duration of the test, preventing accidental communication with a +// real Hub when tests run inside an agent container. +func scrubHubEnv(t *testing.T) { + t.Helper() + for _, key := range hubEnvVars { + t.Setenv(key, "") + } +} + +// filterHubEnv returns a copy of the environment with all Hub-related +// variables removed. Use when constructing exec.Cmd.Env to prevent +// credential leakage to child processes. +func filterHubEnv(env []string) []string { + blocked := make(map[string]bool, len(hubEnvVars)) + for _, key := range hubEnvVars { + blocked[key] = true + } + var filtered []string + for _, e := range env { + key, _, _ := strings.Cut(e, "=") + if !blocked[key] { + filtered = append(filtered, e) + } + } + return filtered +} + // TestInitProjectDataIsolation is a canary test that verifies sciontool source code // does NOT import the pkg/config package, which contains project path resolution logic. // This is a compile-time guarantee that in-container code cannot access project data paths. @@ -135,14 +174,20 @@ func TestInitCommand_Integration(t *testing.T) { t.Skip("skipping integration test in short mode") } + // Clear Hub env vars so the subprocess cannot talk to the real Hub + // and corrupt agent state. See issue #123. + scrubHubEnv(t) + // Build sciontool if needed for integration testing cmd := exec.Command("go", "build", "-buildvcs=false", "-o", "/tmp/sciontool-test", "../") if err := cmd.Run(); err != nil { t.Skipf("failed to build sciontool for integration test: %v", err) } - // Test running a simple command + // Test running a simple command — filter Hub env vars from the + // subprocess environment as belt-and-suspenders protection. testCmd := exec.Command("/tmp/sciontool-test", "init", "--", "echo", "hello") + testCmd.Env = filterHubEnv(os.Environ()) output, err := testCmd.CombinedOutput() if err != nil { t.Errorf("init command failed: %v\nOutput: %s", err, output) diff --git a/pkg/sciontool/hooks/handlers/hub_test.go b/pkg/sciontool/hooks/handlers/hub_test.go index 1bd5da5e7..817cd31a7 100644 --- a/pkg/sciontool/hooks/handlers/hub_test.go +++ b/pkg/sciontool/hooks/handlers/hub_test.go @@ -16,6 +16,22 @@ import ( "github.com/GoogleCloudPlatform/scion/pkg/sciontool/hooks" ) +// scrubHubEnv clears all Hub-related environment variables for the +// duration of the test, preventing accidental communication with a +// real Hub when tests run inside an agent container. See issue #123. +func scrubHubEnv(t *testing.T) { + t.Helper() + for _, key := range []string{ + "SCION_HUB_ENDPOINT", + "SCION_HUB_URL", + "SCION_AUTH_TOKEN", + "SCION_AGENT_ID", + "SCION_AGENT_MODE", + } { + t.Setenv(key, "") + } +} + // TestHubHandler_EventMapping tests that events are correctly mapped to Hub status updates. func TestHubHandler_EventMapping(t *testing.T) { tests := []struct { @@ -90,9 +106,7 @@ func TestHubHandler_EventMapping(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { tmpHome := t.TempDir() - oldHome := os.Getenv("HOME") - os.Setenv("HOME", tmpHome) - defer os.Setenv("HOME", oldHome) + t.Setenv("HOME", tmpHome) var receivedStatus string var mu sync.Mutex @@ -121,16 +135,11 @@ func TestHubHandler_EventMapping(t *testing.T) { })) defer server.Close() - // Set environment variables for the Hub client - os.Setenv("SCION_HUB_ENDPOINT", server.URL) - os.Setenv("SCION_AUTH_TOKEN", "test-token") - os.Setenv("SCION_AGENT_ID", "test-agent-id") - defer func() { - os.Unsetenv("SCION_HUB_ENDPOINT") - os.Unsetenv("SCION_HUB_URL") - os.Unsetenv("SCION_AUTH_TOKEN") - os.Unsetenv("SCION_AGENT_ID") - }() + // Clear real Hub env, then point at the test server (issue #123). + scrubHubEnv(t) + t.Setenv("SCION_HUB_ENDPOINT", server.URL) + t.Setenv("SCION_AUTH_TOKEN", "test-token") + t.Setenv("SCION_AGENT_ID", "test-agent-id") // Create handler handler := NewHubHandler() @@ -172,11 +181,8 @@ func TestHubHandler_EventMapping(t *testing.T) { // TestHubHandler_NotConfigured tests that nil handler doesn't panic. func TestHubHandler_NotConfigured(t *testing.T) { - // Clear environment to ensure client is not configured - os.Unsetenv("SCION_HUB_ENDPOINT") - os.Unsetenv("SCION_HUB_URL") - os.Unsetenv("SCION_AUTH_TOKEN") - os.Unsetenv("SCION_AGENT_ID") + // Clear environment to ensure client is not configured (issue #123). + scrubHubEnv(t) handler := NewHubHandler() if handler != nil { @@ -206,15 +212,11 @@ func TestHubHandler_ReportMethods(t *testing.T) { })) defer server.Close() - os.Setenv("SCION_HUB_ENDPOINT", server.URL) - os.Setenv("SCION_AUTH_TOKEN", "test-token") - os.Setenv("SCION_AGENT_ID", "test-agent-id") - defer func() { - os.Unsetenv("SCION_HUB_ENDPOINT") - os.Unsetenv("SCION_HUB_URL") - os.Unsetenv("SCION_AUTH_TOKEN") - os.Unsetenv("SCION_AGENT_ID") - }() + // Clear real Hub env, then point at the test server (issue #123). + scrubHubEnv(t) + t.Setenv("SCION_HUB_ENDPOINT", server.URL) + t.Setenv("SCION_AUTH_TOKEN", "test-token") + t.Setenv("SCION_AGENT_ID", "test-agent-id") handler := NewHubHandler() if handler == nil { @@ -377,9 +379,7 @@ func TestHubHandler_StickyStatus(t *testing.T) { os.WriteFile(tmpDir+"/agent-info.json", data, 0644) // Point HOME to the temp dir so readLocalActivity finds our file - origHome := os.Getenv("HOME") - os.Setenv("HOME", tmpDir) - defer os.Setenv("HOME", origHome) + t.Setenv("HOME", tmpDir) var mu sync.Mutex callCount := 0 @@ -400,15 +400,11 @@ func TestHubHandler_StickyStatus(t *testing.T) { })) defer server.Close() - os.Setenv("SCION_HUB_ENDPOINT", server.URL) - os.Setenv("SCION_AUTH_TOKEN", "test-token") - os.Setenv("SCION_AGENT_ID", "test-agent-id") - defer func() { - os.Unsetenv("SCION_HUB_ENDPOINT") - os.Unsetenv("SCION_HUB_URL") - os.Unsetenv("SCION_AUTH_TOKEN") - os.Unsetenv("SCION_AGENT_ID") - }() + // Clear real Hub env, then point at the test server (issue #123). + scrubHubEnv(t) + t.Setenv("SCION_HUB_ENDPOINT", server.URL) + t.Setenv("SCION_AUTH_TOKEN", "test-token") + t.Setenv("SCION_AGENT_ID", "test-agent-id") handler := NewHubHandler() if handler == nil { @@ -447,11 +443,8 @@ func TestHubHandler_StickyStatus(t *testing.T) { // TestHubHandler_ModeBehavior verifies behavior differences between local and hub modes. func TestHubHandler_ModeBehavior(t *testing.T) { t.Run("local mode: HubHandler is nil", func(t *testing.T) { - // Clear hub env vars to simulate local mode - os.Unsetenv("SCION_HUB_ENDPOINT") - os.Unsetenv("SCION_HUB_URL") - os.Unsetenv("SCION_AUTH_TOKEN") - os.Unsetenv("SCION_AGENT_ID") + // Clear hub env vars to simulate local mode (issue #123). + scrubHubEnv(t) handler := NewHubHandler() if handler != nil { @@ -463,15 +456,10 @@ func TestHubHandler_ModeBehavior(t *testing.T) { // Even without a hub, the StatusHandler must write to agent-info.json // for local observability (defense-in-depth). tmpHome := t.TempDir() - origHome := os.Getenv("HOME") - os.Setenv("HOME", tmpHome) - defer os.Setenv("HOME", origHome) + t.Setenv("HOME", tmpHome) - // Clear hub env to ensure local mode - os.Unsetenv("SCION_HUB_ENDPOINT") - os.Unsetenv("SCION_HUB_URL") - os.Unsetenv("SCION_AUTH_TOKEN") - os.Unsetenv("SCION_AGENT_ID") + // Clear hub env to ensure local mode (issue #123). + scrubHubEnv(t) statusHandler := NewStatusHandler() event := &hooks.Event{ @@ -497,9 +485,7 @@ func TestHubHandler_ModeBehavior(t *testing.T) { t.Run("hub mode: HubHandler is active and sends updates", func(t *testing.T) { tmpHome := t.TempDir() - origHome := os.Getenv("HOME") - os.Setenv("HOME", tmpHome) - defer os.Setenv("HOME", origHome) + t.Setenv("HOME", tmpHome) callCount := 0 var mu sync.Mutex @@ -513,14 +499,11 @@ func TestHubHandler_ModeBehavior(t *testing.T) { })) defer server.Close() - os.Setenv("SCION_HUB_ENDPOINT", server.URL) - os.Setenv("SCION_AUTH_TOKEN", "test-token") - os.Setenv("SCION_AGENT_ID", "test-agent") - defer func() { - os.Unsetenv("SCION_HUB_ENDPOINT") - os.Unsetenv("SCION_AUTH_TOKEN") - os.Unsetenv("SCION_AGENT_ID") - }() + // Clear real Hub env, then point at the test server (issue #123). + scrubHubEnv(t) + t.Setenv("SCION_HUB_ENDPOINT", server.URL) + t.Setenv("SCION_AUTH_TOKEN", "test-token") + t.Setenv("SCION_AGENT_ID", "test-agent") handler := NewHubHandler() if handler == nil { @@ -546,9 +529,7 @@ func TestHubHandler_ModeBehavior(t *testing.T) { t.Run("hub mode: StatusHandler still writes agent-info.json", func(t *testing.T) { // In hub mode, StatusHandler should still write locally for defense-in-depth. tmpHome := t.TempDir() - origHome := os.Getenv("HOME") - os.Setenv("HOME", tmpHome) - defer os.Setenv("HOME", origHome) + t.Setenv("HOME", tmpHome) server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { w.WriteHeader(http.StatusOK) @@ -556,14 +537,11 @@ func TestHubHandler_ModeBehavior(t *testing.T) { })) defer server.Close() - os.Setenv("SCION_HUB_ENDPOINT", server.URL) - os.Setenv("SCION_AUTH_TOKEN", "test-token") - os.Setenv("SCION_AGENT_ID", "test-agent") - defer func() { - os.Unsetenv("SCION_HUB_ENDPOINT") - os.Unsetenv("SCION_AUTH_TOKEN") - os.Unsetenv("SCION_AGENT_ID") - }() + // Clear real Hub env, then point at the test server (issue #123). + scrubHubEnv(t) + t.Setenv("SCION_HUB_ENDPOINT", server.URL) + t.Setenv("SCION_AUTH_TOKEN", "test-token") + t.Setenv("SCION_AGENT_ID", "test-agent") statusHandler := NewStatusHandler() event := &hooks.Event{ @@ -594,9 +572,7 @@ func TestHubHandler_ModeBehavior(t *testing.T) { func TestHubHandler_AssistantTextForwarding(t *testing.T) { t.Run("forwards assistant text to outbound-message endpoint", func(t *testing.T) { tmpHome := t.TempDir() - origHome := os.Getenv("HOME") - os.Setenv("HOME", tmpHome) - defer os.Setenv("HOME", origHome) + t.Setenv("HOME", tmpHome) var mu sync.Mutex var outboundMsg string @@ -624,15 +600,11 @@ func TestHubHandler_AssistantTextForwarding(t *testing.T) { })) defer server.Close() - os.Setenv("SCION_HUB_ENDPOINT", server.URL) - os.Setenv("SCION_AUTH_TOKEN", "test-token") - os.Setenv("SCION_AGENT_ID", "test-agent-id") - defer func() { - os.Unsetenv("SCION_HUB_ENDPOINT") - os.Unsetenv("SCION_HUB_URL") - os.Unsetenv("SCION_AUTH_TOKEN") - os.Unsetenv("SCION_AGENT_ID") - }() + // Clear real Hub env, then point at the test server (issue #123). + scrubHubEnv(t) + t.Setenv("SCION_HUB_ENDPOINT", server.URL) + t.Setenv("SCION_AUTH_TOKEN", "test-token") + t.Setenv("SCION_AGENT_ID", "test-agent-id") handler := NewHubHandler() if handler == nil { @@ -662,9 +634,7 @@ func TestHubHandler_AssistantTextForwarding(t *testing.T) { t.Run("truncates assistant text exceeding 64KB", func(t *testing.T) { tmpHome := t.TempDir() - origHome := os.Getenv("HOME") - os.Setenv("HOME", tmpHome) - defer os.Setenv("HOME", origHome) + t.Setenv("HOME", tmpHome) var mu sync.Mutex var outboundMsg string @@ -685,15 +655,11 @@ func TestHubHandler_AssistantTextForwarding(t *testing.T) { })) defer server.Close() - os.Setenv("SCION_HUB_ENDPOINT", server.URL) - os.Setenv("SCION_AUTH_TOKEN", "test-token") - os.Setenv("SCION_AGENT_ID", "test-agent-id") - defer func() { - os.Unsetenv("SCION_HUB_ENDPOINT") - os.Unsetenv("SCION_HUB_URL") - os.Unsetenv("SCION_AUTH_TOKEN") - os.Unsetenv("SCION_AGENT_ID") - }() + // Clear real Hub env, then point at the test server (issue #123). + scrubHubEnv(t) + t.Setenv("SCION_HUB_ENDPOINT", server.URL) + t.Setenv("SCION_AUTH_TOKEN", "test-token") + t.Setenv("SCION_AGENT_ID", "test-agent-id") handler := NewHubHandler() if handler == nil { @@ -734,9 +700,7 @@ func TestHubHandler_AssistantTextForwarding(t *testing.T) { func TestHubHandler_AssistantTextVisibilityTagging(t *testing.T) { t.Run("tags outbound message with verbose visibility", func(t *testing.T) { tmpHome := t.TempDir() - origHome := os.Getenv("HOME") - os.Setenv("HOME", tmpHome) - defer os.Setenv("HOME", origHome) + t.Setenv("HOME", tmpHome) var mu sync.Mutex var outboundPayload map[string]interface{} @@ -757,15 +721,11 @@ func TestHubHandler_AssistantTextVisibilityTagging(t *testing.T) { })) defer server.Close() - os.Setenv("SCION_HUB_ENDPOINT", server.URL) - os.Setenv("SCION_AUTH_TOKEN", "test-token") - os.Setenv("SCION_AGENT_ID", "test-agent-id") - defer func() { - os.Unsetenv("SCION_HUB_ENDPOINT") - os.Unsetenv("SCION_HUB_URL") - os.Unsetenv("SCION_AUTH_TOKEN") - os.Unsetenv("SCION_AGENT_ID") - }() + // Clear real Hub env, then point at the test server (issue #123). + scrubHubEnv(t) + t.Setenv("SCION_HUB_ENDPOINT", server.URL) + t.Setenv("SCION_AUTH_TOKEN", "test-token") + t.Setenv("SCION_AGENT_ID", "test-agent-id") handler := NewHubHandler() if handler == nil { @@ -800,9 +760,7 @@ func TestHubHandler_AssistantTextVisibilityTagging(t *testing.T) { t.Run("sets has_thinking metadata when thinking content was filtered", func(t *testing.T) { tmpHome := t.TempDir() - origHome := os.Getenv("HOME") - os.Setenv("HOME", tmpHome) - defer os.Setenv("HOME", origHome) + t.Setenv("HOME", tmpHome) var mu sync.Mutex var outboundPayload map[string]interface{} @@ -823,15 +781,11 @@ func TestHubHandler_AssistantTextVisibilityTagging(t *testing.T) { })) defer server.Close() - os.Setenv("SCION_HUB_ENDPOINT", server.URL) - os.Setenv("SCION_AUTH_TOKEN", "test-token") - os.Setenv("SCION_AGENT_ID", "test-agent-id") - defer func() { - os.Unsetenv("SCION_HUB_ENDPOINT") - os.Unsetenv("SCION_HUB_URL") - os.Unsetenv("SCION_AUTH_TOKEN") - os.Unsetenv("SCION_AGENT_ID") - }() + // Clear real Hub env, then point at the test server (issue #123). + scrubHubEnv(t) + t.Setenv("SCION_HUB_ENDPOINT", server.URL) + t.Setenv("SCION_AUTH_TOKEN", "test-token") + t.Setenv("SCION_AGENT_ID", "test-agent-id") handler := NewHubHandler() if handler == nil { diff --git a/pkg/sciontool/hub/client_test.go b/pkg/sciontool/hub/client_test.go index bd129dd3b..63839263b 100644 --- a/pkg/sciontool/hub/client_test.go +++ b/pkg/sciontool/hub/client_test.go @@ -19,7 +19,6 @@ import ( "encoding/json" "net/http" "net/http/httptest" - "os" "testing" "time" @@ -28,53 +27,52 @@ import ( "github.com/stretchr/testify/require" ) +// scrubHubEnv clears all Hub-related environment variables for the +// duration of the test, preventing accidental communication with a +// real Hub when tests run inside an agent container. See issue #123. +func scrubHubEnv(t *testing.T) { + t.Helper() + for _, key := range []string{ + EnvHubEndpoint, + EnvHubURL, + EnvHubToken, + EnvAgentID, + EnvAgentMode, + } { + t.Setenv(key, "") + } +} + func TestNewClient_FromEnvironment(t *testing.T) { - // Save and restore env vars - origEndpoint := os.Getenv(EnvHubEndpoint) - origURL := os.Getenv(EnvHubURL) - origToken := os.Getenv(EnvHubToken) - origAgentID := os.Getenv(EnvAgentID) - defer func() { - os.Setenv(EnvHubEndpoint, origEndpoint) - os.Setenv(EnvHubURL, origURL) - os.Setenv(EnvHubToken, origToken) - os.Setenv(EnvAgentID, origAgentID) - }() + // Clear Hub env vars to prevent leakage from the container (issue #123). + scrubHubEnv(t) t.Run("missing env vars returns nil", func(t *testing.T) { - os.Unsetenv(EnvHubEndpoint) - os.Unsetenv(EnvHubURL) - os.Unsetenv(EnvHubToken) - os.Unsetenv(EnvAgentID) - + scrubHubEnv(t) client := NewClient() assert.Nil(t, client) }) t.Run("missing token returns nil", func(t *testing.T) { - os.Unsetenv(EnvHubEndpoint) - os.Setenv(EnvHubURL, "http://hub.example.com") - os.Unsetenv(EnvHubToken) - os.Unsetenv(EnvAgentID) - + scrubHubEnv(t) + t.Setenv(EnvHubURL, "http://hub.example.com") client := NewClient() assert.Nil(t, client) }) t.Run("missing agentID returns nil", func(t *testing.T) { - os.Setenv(EnvHubEndpoint, "http://hub.example.com") - os.Setenv(EnvHubToken, "test-token") - os.Unsetenv(EnvAgentID) - + scrubHubEnv(t) + t.Setenv(EnvHubEndpoint, "http://hub.example.com") + t.Setenv(EnvHubToken, "test-token") client := NewClient() assert.Nil(t, client, "should not create client without agent ID (local agent scenario)") }) t.Run("with all env vars returns client", func(t *testing.T) { - os.Unsetenv(EnvHubEndpoint) - os.Setenv(EnvHubURL, "http://hub.example.com") - os.Setenv(EnvHubToken, "test-token") - os.Setenv(EnvAgentID, "agent-123") + scrubHubEnv(t) + t.Setenv(EnvHubURL, "http://hub.example.com") + t.Setenv(EnvHubToken, "test-token") + t.Setenv(EnvAgentID, "agent-123") client := NewClient() require.NotNil(t, client) @@ -82,10 +80,11 @@ func TestNewClient_FromEnvironment(t *testing.T) { }) t.Run("prefers SCION_HUB_ENDPOINT over SCION_HUB_URL", func(t *testing.T) { - os.Setenv(EnvHubEndpoint, "http://endpoint.example.com") - os.Setenv(EnvHubURL, "http://url.example.com") - os.Setenv(EnvHubToken, "test-token") - os.Setenv(EnvAgentID, "agent-123") + scrubHubEnv(t) + t.Setenv(EnvHubEndpoint, "http://endpoint.example.com") + t.Setenv(EnvHubURL, "http://url.example.com") + t.Setenv(EnvHubToken, "test-token") + t.Setenv(EnvAgentID, "agent-123") client := NewClient() require.NotNil(t, client) @@ -93,10 +92,10 @@ func TestNewClient_FromEnvironment(t *testing.T) { }) t.Run("falls back to SCION_HUB_URL when SCION_HUB_ENDPOINT not set", func(t *testing.T) { - os.Unsetenv(EnvHubEndpoint) - os.Setenv(EnvHubURL, "http://url.example.com") - os.Setenv(EnvHubToken, "test-token") - os.Setenv(EnvAgentID, "agent-123") + scrubHubEnv(t) + t.Setenv(EnvHubURL, "http://url.example.com") + t.Setenv(EnvHubToken, "test-token") + t.Setenv(EnvAgentID, "agent-123") client := NewClient() require.NotNil(t, client) @@ -261,31 +260,25 @@ func TestClient_Heartbeat(t *testing.T) { } func TestIsHostedMode(t *testing.T) { - origMode := os.Getenv(EnvAgentMode) - defer os.Setenv(EnvAgentMode, origMode) - t.Run("not hosted mode", func(t *testing.T) { - os.Unsetenv(EnvAgentMode) + t.Setenv(EnvAgentMode, "") assert.False(t, IsHostedMode()) - os.Setenv(EnvAgentMode, "solo") + t.Setenv(EnvAgentMode, "solo") assert.False(t, IsHostedMode()) }) t.Run("hosted mode", func(t *testing.T) { - os.Setenv(EnvAgentMode, "hosted") + t.Setenv(EnvAgentMode, "hosted") assert.True(t, IsHostedMode()) }) } func TestGetAgentID(t *testing.T) { - origID := os.Getenv(EnvAgentID) - defer os.Setenv(EnvAgentID, origID) - - os.Setenv(EnvAgentID, "test-agent-id") + t.Setenv(EnvAgentID, "test-agent-id") assert.Equal(t, "test-agent-id", GetAgentID()) - os.Unsetenv(EnvAgentID) + t.Setenv(EnvAgentID, "") assert.Equal(t, "", GetAgentID()) } @@ -677,16 +670,6 @@ func TestClient_StartTokenRefresh(t *testing.T) { } func TestOperatingMode(t *testing.T) { - // Save and restore env vars - origEndpoint := os.Getenv(EnvHubEndpoint) - origURL := os.Getenv(EnvHubURL) - origMode := os.Getenv(EnvAgentMode) - defer func() { - os.Setenv(EnvHubEndpoint, origEndpoint) - os.Setenv(EnvHubURL, origURL) - os.Setenv(EnvAgentMode, origMode) - }() - tests := []struct { name string endpoint string @@ -747,17 +730,16 @@ func TestOperatingMode(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - os.Unsetenv(EnvHubEndpoint) - os.Unsetenv(EnvHubURL) - os.Unsetenv(EnvAgentMode) + // Clear Hub env, then set test values (issue #123). + scrubHubEnv(t) if tt.endpoint != "" { - os.Setenv(EnvHubEndpoint, tt.endpoint) + t.Setenv(EnvHubEndpoint, tt.endpoint) } if tt.hubURL != "" { - os.Setenv(EnvHubURL, tt.hubURL) + t.Setenv(EnvHubURL, tt.hubURL) } if tt.agentMode != "" { - os.Setenv(EnvAgentMode, tt.agentMode) + t.Setenv(EnvAgentMode, tt.agentMode) } mode := OperatingMode() @@ -768,20 +750,8 @@ func TestOperatingMode(t *testing.T) { } func TestOperatingMode_Defaults(t *testing.T) { - // Save and restore env vars - origEndpoint := os.Getenv(EnvHubEndpoint) - origURL := os.Getenv(EnvHubURL) - origMode := os.Getenv(EnvAgentMode) - defer func() { - os.Setenv(EnvHubEndpoint, origEndpoint) - os.Setenv(EnvHubURL, origURL) - os.Setenv(EnvAgentMode, origMode) - }() - - // Clear all relevant env vars - os.Unsetenv(EnvHubEndpoint) - os.Unsetenv(EnvHubURL) - os.Unsetenv(EnvAgentMode) + // Clear all relevant env vars (issue #123). + scrubHubEnv(t) mode := OperatingMode() assert.Equal(t, ModeLocal, mode, "should default to ModeLocal when no env vars are set") @@ -818,18 +788,11 @@ func TestNewClient_UsesTokenFile(t *testing.T) { cleanup := SetTokenHome(t.TempDir()) defer cleanup() - origEndpoint := os.Getenv(EnvHubEndpoint) - origToken := os.Getenv(EnvHubToken) - origAgentID := os.Getenv(EnvAgentID) - defer func() { - os.Setenv(EnvHubEndpoint, origEndpoint) - os.Setenv(EnvHubToken, origToken) - os.Setenv(EnvAgentID, origAgentID) - }() - - os.Setenv(EnvHubEndpoint, "http://hub.example.com") - os.Setenv(EnvHubToken, "original-env-token") - os.Setenv(EnvAgentID, "agent-123") + // Clear Hub env, then set test values (issue #123). + scrubHubEnv(t) + t.Setenv(EnvHubEndpoint, "http://hub.example.com") + t.Setenv(EnvHubToken, "original-env-token") + t.Setenv(EnvAgentID, "agent-123") t.Run("uses env token when no file exists", func(t *testing.T) { client := NewClient() @@ -974,27 +937,21 @@ func TestGitHubTokenFile_WriteAndRead(t *testing.T) { } func TestIsGitHubAppEnabled(t *testing.T) { - orig := os.Getenv(EnvGitHubAppEnabled) - defer os.Setenv(EnvGitHubAppEnabled, orig) - - os.Unsetenv(EnvGitHubAppEnabled) + t.Setenv(EnvGitHubAppEnabled, "") assert.False(t, IsGitHubAppEnabled()) - os.Setenv(EnvGitHubAppEnabled, "false") + t.Setenv(EnvGitHubAppEnabled, "false") assert.False(t, IsGitHubAppEnabled()) - os.Setenv(EnvGitHubAppEnabled, "true") + t.Setenv(EnvGitHubAppEnabled, "true") assert.True(t, IsGitHubAppEnabled()) } func TestGitHubTokenPath(t *testing.T) { - orig := os.Getenv(EnvGitHubTokenPath) - defer os.Setenv(EnvGitHubTokenPath, orig) - - os.Unsetenv(EnvGitHubTokenPath) + t.Setenv(EnvGitHubTokenPath, "") assert.Equal(t, DefaultGitHubTokenPath, GitHubTokenPath()) - os.Setenv(EnvGitHubTokenPath, "/custom/path/token") + t.Setenv(EnvGitHubTokenPath, "/custom/path/token") assert.Equal(t, "/custom/path/token", GitHubTokenPath()) }