From 3e5c7a2fc81de006cda30116b9aa87cf2436e3b6 Mon Sep 17 00:00:00 2001 From: Cody Lee Date: Tue, 10 Feb 2026 09:19:27 -0600 Subject: [PATCH 1/2] fix(cmd): prevent panic in test command with empty API keys The test command would panic when trying to slice empty API keys for masking. This occurred when users authenticate via OAuth2 instead of API keys, since config validation only requires Site to be set. Changes: - Add length checks before slicing API/App keys in test command (root.go:210-227) - Display helpful messages for empty keys (OAuth2 auth) - Show warnings for keys that are too short to be valid - Add comprehensive tests for all edge cases (root_test.go:235-320) Fixes panic: runtime error: slice bounds out of range [:8] with length 0 Co-Authored-By: Claude Sonnet 4.5 --- cmd/root.go | 21 ++++++++++-- cmd/root_test.go | 89 ++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 108 insertions(+), 2 deletions(-) diff --git a/cmd/root.go b/cmd/root.go index 1893916b..f84473c6 100644 --- a/cmd/root.go +++ b/cmd/root.go @@ -207,8 +207,25 @@ var testCmd = &cobra.Command{ fmt.Println("Configuration is valid:") fmt.Printf(" Site: %s\n", cfg.Site) - fmt.Printf(" API Key: %s...%s\n", cfg.APIKey[:8], cfg.APIKey[len(cfg.APIKey)-4:]) - fmt.Printf(" App Key: %s...%s\n", cfg.AppKey[:8], cfg.AppKey[len(cfg.AppKey)-4:]) + + // Display API key info if present + if len(cfg.APIKey) >= 12 { + fmt.Printf(" API Key: %s...%s\n", cfg.APIKey[:8], cfg.APIKey[len(cfg.APIKey)-4:]) + } else if len(cfg.APIKey) > 0 { + fmt.Printf(" API Key: %s (too short - may be invalid)\n", cfg.APIKey) + } else { + fmt.Println(" API Key: (not set - using OAuth2 or will prompt)") + } + + // Display App key info if present + if len(cfg.AppKey) >= 12 { + fmt.Printf(" App Key: %s...%s\n", cfg.AppKey[:8], cfg.AppKey[len(cfg.AppKey)-4:]) + } else if len(cfg.AppKey) > 0 { + fmt.Printf(" App Key: %s (too short - may be invalid)\n", cfg.AppKey) + } else { + fmt.Println(" App Key: (not set - using OAuth2 or will prompt)") + } + fmt.Println("\nConnection test successful!") return nil diff --git a/cmd/root_test.go b/cmd/root_test.go index c171347d..d24bcbd0 100644 --- a/cmd/root_test.go +++ b/cmd/root_test.go @@ -9,6 +9,8 @@ import ( "errors" "strings" "testing" + + "github.com/DataDog/pup/pkg/config" ) func TestRootCmd_SilenceUsage(t *testing.T) { @@ -229,3 +231,90 @@ func TestFormatAPIError_AllStatusCodes(t *testing.T) { }) } } + +func TestTestCmd_EmptyKeys(t *testing.T) { + // Save original config + origCfg := cfg + + // Set up test config with empty keys + cfg = &config.Config{ + Site: "datadoghq.com", + APIKey: "", + AppKey: "", + } + defer func() { cfg = origCfg }() + + // Execute test command + err := testCmd.RunE(testCmd, []string{}) + + // Should not panic and should succeed + if err != nil { + t.Errorf("testCmd.RunE() with empty keys failed: %v", err) + } +} + +func TestTestCmd_ShortKeys(t *testing.T) { + // Save original config + origCfg := cfg + + // Set up test config with short keys + cfg = &config.Config{ + Site: "datadoghq.com", + APIKey: "short", + AppKey: "key", + } + defer func() { cfg = origCfg }() + + // Execute test command + err := testCmd.RunE(testCmd, []string{}) + + // Should not panic and should succeed + if err != nil { + t.Errorf("testCmd.RunE() with short keys failed: %v", err) + } +} + +func TestTestCmd_ValidKeys(t *testing.T) { + // Save original config + origCfg := cfg + + // Set up test config with valid length keys + cfg = &config.Config{ + Site: "datadoghq.com", + APIKey: "1234567890abcdef1234567890abcdef", + AppKey: "abcdefghijklmnopqrstuvwxyz123456", + } + defer func() { cfg = origCfg }() + + // Execute test command + err := testCmd.RunE(testCmd, []string{}) + + // Should not panic and should succeed + if err != nil { + t.Errorf("testCmd.RunE() with valid keys failed: %v", err) + } +} + +func TestTestCmd_InvalidSite(t *testing.T) { + // Save original config + origCfg := cfg + + // Set up test config with empty site + cfg = &config.Config{ + Site: "", + APIKey: "1234567890abcdef", + AppKey: "abcdefghijklmnop", + } + defer func() { cfg = origCfg }() + + // Execute test command + err := testCmd.RunE(testCmd, []string{}) + + // Should fail validation + if err == nil { + t.Error("testCmd.RunE() with empty site should fail") + } + if !strings.Contains(err.Error(), "DD_SITE") { + t.Errorf("testCmd.RunE() error should mention DD_SITE, got: %v", err) + } +} From 54c9eeed2f485d3ef1c986c32857eae60daef99d Mon Sep 17 00:00:00 2001 From: Cody Lee Date: Tue, 10 Feb 2026 09:24:06 -0600 Subject: [PATCH 2/2] fix(ci): make PR comment step non-blocking The CI was failing when the Comment on PR step couldn't authenticate, even though the actual tests passed successfully. This is a common issue with GitHub Actions permissions on PRs. Making this step non-blocking ensures CI passes when tests succeed, regardless of comment posting permissions. Co-Authored-By: Claude Sonnet 4.5 --- .github/workflows/ci.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 43c65f68..ca1d69e3 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -137,6 +137,7 @@ jobs: - name: Comment on PR if: github.event_name == 'pull_request' + continue-on-error: true # Don't fail CI if comment posting fails uses: actions/github-script@v8 env: COMMENT_BODY: ${{ steps.comment.outputs.comment_body }}