Skip to content

fix(cmd): prevent panic in test command with empty API keys#20

Merged
platinummonkey merged 2 commits into
mainfrom
fix/test-command-panic-empty-keys
Feb 10, 2026
Merged

fix(cmd): prevent panic in test command with empty API keys#20
platinummonkey merged 2 commits into
mainfrom
fix/test-command-panic-empty-keys

Conversation

@platinummonkey
Copy link
Copy Markdown
Collaborator

Summary

Fixed a panic in the pup test command that occurred when API keys were not set (e.g., when using OAuth2 authentication).

Problem

The test command attempted to slice API keys for masking without checking if they existed or had sufficient length, causing a panic:

panic: runtime error: slice bounds out of range [:8] with length 0

This occurred because config validation only requires Site to be set, allowing empty API keys when OAuth2 authentication is used.

Changes

  • Added length checks before slicing API/App keys in test command (root.go:210-227)
  • Display appropriate messages based on key length:
    • Empty: Shows "(not set - using OAuth2 or will prompt)"
    • Too short (<12 chars): Shows key with warning "(too short - may be invalid)"
    • Valid (≥12 chars): Shows masked version "xxxxxxxx...yyyy"
  • Added comprehensive tests covering all edge cases (root_test.go:235-320):
    • TestTestCmd_EmptyKeys - Empty API keys don't panic
    • TestTestCmd_ShortKeys - Short keys display warning
    • TestTestCmd_ValidKeys - Valid keys are properly masked
    • TestTestCmd_InvalidSite - Validation still enforces required Site

Testing

# Empty keys (OAuth2 scenario)
./pup test
# Output: "API Key: (not set - using OAuth2 or will prompt)"

# Valid keys
DD_API_KEY="..." DD_APP_KEY="..." ./pup test
# Output: "API Key: 12345678...7890"

# Short keys
DD_API_KEY="short" DD_APP_KEY="key" ./pup test
# Output: "API Key: short (too short - may be invalid)"

# All tests pass
go test ./cmd/... -v -run TestTestCmd

🤖 Generated with Claude Code

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 <noreply@anthropic.com>
@platinummonkey platinummonkey requested a review from a team as a code owner February 10, 2026 15:19
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 <noreply@anthropic.com>
@github-actions
Copy link
Copy Markdown

📊 Test Coverage Report

Overall Coverage: 80.4% Coverage

Threshold: 80% ✅

Coverage by Package
## Coverage by Package

- github.com/DataDog/pup/pkg/auth/callback/server.go:40: 81.2%
- github.com/DataDog/pup/pkg/auth/dcr/client.go:28: 100.0%
- github.com/DataDog/pup/pkg/auth/dcr/types.go:24: 100.0%
- github.com/DataDog/pup/pkg/auth/oauth/client.go:22: 100.0%
- github.com/DataDog/pup/pkg/auth/oauth/pkce.go:24: 85.7%
- github.com/DataDog/pup/pkg/auth/storage/factory.go:53: 94.7%
- github.com/DataDog/pup/pkg/auth/storage/keychain.go:44: 42.9%
- github.com/DataDog/pup/pkg/auth/storage/storage.go:58: 71.4%
- github.com/DataDog/pup/pkg/auth/types/types.go:23: 100.0%
- github.com/DataDog/pup/pkg/client/client.go:32: 94.4%
- github.com/DataDog/pup/pkg/config/config.go:22: 100.0%
- github.com/DataDog/pup/pkg/formatter/formatter.go:31: 100.0%
- github.com/DataDog/pup/pkg/useragent/useragent.go:32: 100.0%
- github.com/DataDog/pup/pkg/util/time.go:20: 95.8%

## Summary

total:								(statements)		80.4%

📈 Coverage Status: ✅ PASSED - Coverage meets minimum threshold

Updated for commit 54c9eee

@platinummonkey platinummonkey merged commit 3c87368 into main Feb 10, 2026
4 checks passed
@platinummonkey platinummonkey deleted the fix/test-command-panic-empty-keys branch February 10, 2026 15:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant