Skip to content

[Repo Assist] refactor(auth): move generateRandomAPIKey to internal/auth package #3163

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
Apr 4, 2026
+44 −29
Merged

[Repo Assist] refactor(auth): move generateRandomAPIKey to internal/auth package #3163

Show file tree
Hide file tree
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
18 changes: 18 additions & 0 deletions internal/auth/apikey.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
package auth

import (
"crypto/rand"
"encoding/hex"
"fmt"
)

// GenerateRandomAPIKey generates a cryptographically random API key.
// Per spec §7.3, the gateway SHOULD generate a random API key on startup
// if none is provided. Returns a 32-byte hex-encoded string (64 chars).
func GenerateRandomAPIKey() (string, error) {
bytes := make([]byte, 32)
if _, err := rand.Read(bytes); err != nil {
return "", fmt.Errorf("failed to generate random API key: %w", err)
}
Comment on lines +12 to +16
Copy link

Copilot AI Apr 4, 2026

Choose a reason for hiding this comment

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

GenerateRandomAPIKey wraps the rand.Read error with "failed to generate random API key", and the caller in run() wraps again with the same message. If this error ever surfaces, it will read redundantly ("failed...: failed..."). Prefer returning the raw rand.Read error from GenerateRandomAPIKey (or using a more specific/sentinel error) and let callers add context once.

Copilot uses AI. Check for mistakes.
return hex.EncodeToString(bytes), nil
Comment on lines +13 to +17
Copy link

Copilot AI Apr 4, 2026

Choose a reason for hiding this comment

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

Minor naming: bytes is a very generic identifier and can be confused with the standard bytes package in Go codebases. Consider renaming it to something specific like keyBytes or buf to make the intent clearer.

Suggested change
bytes := make([]byte, 32)
if _, err := rand.Read(bytes); err != nil {
return "", fmt.Errorf("failed to generate random API key: %w", err)
}
return hex.EncodeToString(bytes), nil
keyBytes := make([]byte, 32)
if _, err := rand.Read(keyBytes); err != nil {
return "", fmt.Errorf("failed to generate random API key: %w", err)
}
return hex.EncodeToString(keyBytes), nil

Copilot uses AI. Check for mistakes.
}
24 changes: 24 additions & 0 deletions internal/auth/apikey_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
package auth_test

import (
"testing"

"github.com/github/gh-aw-mcpg/internal/auth"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)

// TestGenerateRandomAPIKey verifies that GenerateRandomAPIKey produces a
// non-empty, unique, hex-encoded string per spec §7.3.
func TestGenerateRandomAPIKey(t *testing.T) {
key, err := auth.GenerateRandomAPIKey()
require.NoError(t, err, "GenerateRandomAPIKey() should not fail")
assert.NotEmpty(t, key, "generated key should not be empty")
// 32 bytes encoded as hex = 64 characters
assert.Len(t, key, 64, "generated key should be 64 hex characters")

// Verify keys are unique across calls
key2, err := auth.GenerateRandomAPIKey()
require.NoError(t, err)
assert.NotEqual(t, key, key2, "successive calls should produce unique keys")
}
16 changes: 2 additions & 14 deletions internal/cmd/root.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,6 @@ package cmd
import (
"bufio"
"context"
"crypto/rand"
"encoding/hex"
"encoding/json"
"fmt"
"io"
Expand All @@ -17,6 +15,7 @@ import (
"syscall"
"time"

"github.com/github/gh-aw-mcpg/internal/auth"
"github.com/github/gh-aw-mcpg/internal/config"
"github.com/github/gh-aw-mcpg/internal/difc"
"github.com/github/gh-aw-mcpg/internal/logger"
Expand Down Expand Up @@ -303,7 +302,7 @@ func run(cmd *cobra.Command, args []string) error {
// The generated key is set in the config so it propagates to both the HTTP
// server authentication and the stdout configuration output (spec §5.4).
if cfg.GetAPIKey() == "" {
randomKey, err := generateRandomAPIKey()
randomKey, err := auth.GenerateRandomAPIKey()
if err != nil {
return fmt.Errorf("failed to generate random API key: %w", err)
}
Expand Down Expand Up @@ -587,17 +586,6 @@ func loadEnvFile(path string) error {
return scanner.Err()
}

// generateRandomAPIKey generates a cryptographically random API key.
// Per spec §7.3, the gateway SHOULD generate a random API key on startup
// if none is provided. Returns a 32-byte hex-encoded string (64 chars).
func generateRandomAPIKey() (string, error) {
bytes := make([]byte, 32)
if _, err := rand.Read(bytes); err != nil {
return "", fmt.Errorf("failed to generate random API key: %w", err)
}
return hex.EncodeToString(bytes), nil
}

// Execute runs the root command
func Execute() {
if err := rootCmd.Execute(); err != nil {
Expand Down
15 changes: 0 additions & 15 deletions internal/cmd/root_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -508,18 +508,3 @@ func TestPostRunCleanup(t *testing.T) {
assert.NotNil(t, rootCmd.PersistentPostRun, "PersistentPostRun should be set")
})
}

// TestGenerateRandomAPIKey verifies that generateRandomAPIKey produces a
// non-empty, unique, hex-encoded string per spec §7.3.
func TestGenerateRandomAPIKey(t *testing.T) {
key, err := generateRandomAPIKey()
require.NoError(t, err, "generateRandomAPIKey() should not fail")
assert.NotEmpty(t, key, "generated key should not be empty")
// 32 bytes encoded as hex = 64 characters
assert.Len(t, key, 64, "generated key should be 64 hex characters")

// Verify keys are unique across calls
key2, err := generateRandomAPIKey()
require.NoError(t, err)
assert.NotEqual(t, key, key2, "successive calls should produce unique keys")
}
Loading