Skip to content

[test-improver] Improve tests for auth package #271

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
lpcox merged 1 commit into from
Jan 16, 2026
Merged

[test-improver] Improve tests for auth package #271

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
117 changes: 101 additions & 16 deletions internal/auth/header_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,9 @@ package auth

import (
"testing"

"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)

func TestSanitizeForLogging(t *testing.T) {
Expand Down Expand Up @@ -40,13 +43,27 @@ func TestSanitizeForLogging(t *testing.T) {
input: "Bearer my-token-123",
want: "Bear...",
},
{
name: "Unicode characters",
input: "key-with-émojis-🔑",
want: "key-...",
},
{
name: "Very long API key",
input: "my-super-long-api-key-with-many-characters-12345678901234567890",
want: "my-s...",
},
{
name: "Special characters",
input: "key!@#$%^&*()",
want: "key!...",
},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
if got := sanitizeForLogging(tt.input); got != tt.want {
t.Errorf("sanitizeForLogging() = %v, want %v", got, tt.want)
}
got := sanitizeForLogging(tt.input)
assert.Equal(t, tt.want, got)
})
}
}
Expand Down Expand Up @@ -87,24 +104,69 @@ func TestParseAuthHeader(t *testing.T) {
wantAgentID: "agent-123",
wantErr: nil,
},
{
name: "Bearer with multiple spaces",
authHeader: "Bearer my-token",
wantAPIKey: " my-token",
wantAgentID: " my-token",
Comment on lines +110 to +111
Copy link

Copilot AI Jan 16, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The expected values include a leading space, but the implementation uses strings.TrimPrefix(authHeader, \"Bearer \") which removes only the prefix "Bearer " (with a single space). With input "Bearer my-token" (two spaces), the result will be " my-token" (one leading space), which matches the expectation. However, this test documents unexpected behavior - tokens/keys with leading whitespace are likely unintended and could cause authentication failures. Consider whether the implementation should trim all leading/trailing whitespace after prefix removal.

This issue also appears in the following locations of the same file:

  • line 124

Copilot uses AI. Check for mistakes.
wantErr: nil,
},
{
name: "Lowercase bearer (not supported)",
authHeader: "bearer my-token",
wantAPIKey: "bearer my-token",
wantAgentID: "bearer my-token",
wantErr: nil,
},
{
name: "Agent with multiple spaces",
authHeader: "Agent agent-id",
wantAPIKey: " agent-id",
wantAgentID: " agent-id",
wantErr: nil,
},
{
name: "Whitespace only header",
authHeader: " ",
wantAPIKey: " ",
wantAgentID: " ",
wantErr: nil,
Comment on lines +131 to +133
Copy link

Copilot AI Jan 16, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This test expects whitespace-only headers to be treated as valid authentication. However, this is problematic from a security perspective - whitespace-only values should likely be rejected or treated as empty. The implementation at line 59 only checks for authHeader == \"\", allowing whitespace-only strings to be used as API keys/agent IDs, which could lead to security issues.

This issue also appears in the following locations of the same file:

  • line 152
Suggested change
wantAPIKey: " ",
wantAgentID: " ",
wantErr: nil,
wantAPIKey: "",
wantAgentID: "",
wantErr: ErrMissingAuthHeader,

Copilot uses AI. Check for mistakes.
},
{
name: "API key with special characters",
authHeader: "key!@#$%^&*()",
wantAPIKey: "key!@#$%^&*()",
wantAgentID: "key!@#$%^&*()",
wantErr: nil,
},
{
name: "Very long API key",
authHeader: "my-super-long-api-key-with-many-characters-12345678901234567890",
wantAPIKey: "my-super-long-api-key-with-many-characters-12345678901234567890",
wantAgentID: "my-super-long-api-key-with-many-characters-12345678901234567890",
wantErr: nil,
},
{
name: "Bearer with trailing space",
authHeader: "Bearer my-token ",
wantAPIKey: "my-token ",
wantAgentID: "my-token ",
wantErr: nil,
},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
gotAPIKey, gotAgentID, gotErr := ParseAuthHeader(tt.authHeader)

if gotErr != tt.wantErr {
t.Errorf("ParseAuthHeader() error = %v, wantErr %v", gotErr, tt.wantErr)
return
}

if gotAPIKey != tt.wantAPIKey {
t.Errorf("ParseAuthHeader() gotAPIKey = %v, want %v", gotAPIKey, tt.wantAPIKey)
if tt.wantErr != nil {
require.ErrorIs(t, gotErr, tt.wantErr)
} else {
require.NoError(t, gotErr)
}

if gotAgentID != tt.wantAgentID {
t.Errorf("ParseAuthHeader() gotAgentID = %v, want %v", gotAgentID, tt.wantAgentID)
}
assert.Equal(t, tt.wantAPIKey, gotAPIKey)
assert.Equal(t, tt.wantAgentID, gotAgentID)
})
}
}
Expand Down Expand Up @@ -140,13 +202,36 @@ func TestValidateAPIKey(t *testing.T) {
expected: "required-key",
want: false,
},
{
name: "Both empty",
provided: "",
expected: "",
want: true,
},
{
name: "Case sensitive - should not match",
provided: "My-Secret-Key",
expected: "my-secret-key",
want: false,
},
{
name: "Keys with whitespace - exact match required",
provided: "key with spaces",
expected: "key with spaces",
want: true,
},
{
name: "Keys with whitespace - trailing space different",
provided: "my-key ",
expected: "my-key",
want: false,
},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
if got := ValidateAPIKey(tt.provided, tt.expected); got != tt.want {
t.Errorf("ValidateAPIKey() = %v, want %v", got, tt.want)
}
got := ValidateAPIKey(tt.provided, tt.expected)
assert.Equal(t, tt.want, got)
})
}
}
Loading