security: use crypto/rand for agent API keys (fixes C-1)

Replaces math/rand-based agent API key generation in internal/service/agent.go
with crypto/rand.Read over a 32-byte buffer encoded with base64.RawURLEncoding,
yielding a 43-character URL-safe unpadded ASCII string (256 bits of entropy).

generateAPIKey now returns (string, error); Register and RegisterAgent propagate
entropy-source failures. hashAPIKey is unchanged — the SHA-256 hashed-at-rest
invariant is preserved.

Fixes C-1 (CWE-338: Use of Cryptographically Weak Pseudo-Random Number Generator)
from certctl-audit-report.md.

Changes:
- internal/service/agent.go: new imports (crypto/rand, encoding/base64);
  generateAPIKey rewritten to return (string, error); Register and RegisterAgent
  updated to propagate the error.
- internal/service/agent_test.go: TestGenerateAPIKey_Properties regression test
  (non-empty, length 43, valid base64url, 32 decoded bytes, no collisions over
  64 calls). No entropy-failure test — Go 1.24+ (issue #66821) makes crypto/rand
  errors fatal, so that branch is defensively unreachable.

Verification:
- go build ./cmd/server/... ./cmd/agent/... ./cmd/mcp-server/... ./cmd/cli/... → pass
- go vet ./... → pass
- go test -race (CI scope, 43 packages) → pass
- golangci-lint v2.11.4 run ./... → 0 issues
- govulncheck ./... → 0 vulnerabilities in certctl code
- Coverage: service 68.9% / handler 83.6% / domain 82.0% / middleware 63.8%
  (all above CI gates 55/60/40/30)
- grep math/rand in internal/ and cmd/ → zero production hits
- No caller assumes the old 32-char length or legacy charset

internal/service/agent.go

@@ -2,11 +2,12 @@ package service
 
 import (
 	"context"
+	"crypto/rand"
 	"crypto/sha256"
+	"encoding/base64"
 	"encoding/hex"
 	"fmt"
 	"log/slog"
-	"math/rand"
 	"time"
 
 	"github.com/shankar0123/certctl/internal/domain"
@@ -57,8 +58,11 @@ func (s *AgentService) Register(ctx context.Context, name string, hostname strin
 		return nil, "", fmt.Errorf("agent name and hostname are required")
 	}
 
-	// Generate API key
-	apiKey := generateAPIKey()
+	// Generate API key. crypto/rand failure is non-recoverable — propagate immediately.
+	apiKey, err := generateAPIKey()
+	if err != nil {
+		return nil, "", fmt.Errorf("failed to generate agent api key: %w", err)
+	}
 	apiKeyHash := hashAPIKey(apiKey)
 
 	now := time.Now()
@@ -380,7 +384,10 @@ func (s *AgentService) GetAgent(ctx context.Context, id string) (*domain.Agent,
 // RegisterAgent creates and registers a new agent (handler interface method).
 func (s *AgentService) RegisterAgent(ctx context.Context, agent domain.Agent) (*domain.Agent, error) {
 	agent.ID = generateID("agent")
-	apiKey := generateAPIKey()
+	apiKey, err := generateAPIKey()
+	if err != nil {
+		return nil, fmt.Errorf("failed to generate agent api key: %w", err)
+	}
 	agent.APIKeyHash = hashAPIKey(apiKey)
 	agent.Status = domain.AgentStatusOnline
 	now := time.Now()
@@ -487,14 +494,17 @@ func (s *AgentService) CertificatePickup(ctx context.Context, agentID, certID st
 	return string(certPEM), nil
 }
 
-// generateAPIKey creates a random API key for an agent.
-func generateAPIKey() string {
-	const charset = "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789"
+// generateAPIKey creates a cryptographically secure random API key for an agent.
+// It fills a 32-byte buffer from crypto/rand (256 bits of entropy) and encodes it with
+// base64.RawURLEncoding, yielding a 43-character URL-safe, unpadded ASCII string.
+// The plaintext key is shown to the caller exactly once; only its SHA-256 hash is stored.
+// Fixes C-1 (CWE-338: previously used math/rand, which is not cryptographically secure).
+func generateAPIKey() (string, error) {
 	b := make([]byte, 32)
-	for i := range b {
-		b[i] = charset[rand.Intn(len(charset))]
+	if _, err := rand.Read(b); err != nil {
+		return "", fmt.Errorf("generate agent api key: %w", err)
 	}
-	return string(b)
+	return base64.RawURLEncoding.EncodeToString(b), nil
 }
 
 // hashAPIKey hashes an API key using SHA256.

internal/service/agent_test.go

@@ -2,6 +2,7 @@ package service
 
 import (
 	"context"
+	"encoding/base64"
 	"log/slog"
 	"testing"
 	"time"
@@ -594,3 +595,44 @@ func TestListAgents(t *testing.T) {
 		t.Errorf("expected total 2, got %d", total)
 	}
 }
+
+// TestGenerateAPIKey_Properties is the core regression test for C-1 (CWE-338).
+// It verifies that generateAPIKey produces cryptographically random,
+// unpadded base64url-encoded, 32-byte (256-bit) keys that never collide
+// across consecutive calls. Exact length and alphabet are verified against
+// base64.RawURLEncoding so any silent change to entropy or encoding fails
+// fast.
+//
+// Note on the error branch: since Go 1.24 (issue #66821) crypto/rand.Read
+// treats entropy-source failures as fatal — the process is terminated
+// rather than returning an error. The defensive `if err != nil` branch
+// in generateAPIKey is therefore unreachable from tests on modern Go.
+// It is kept to preserve the documented (string, error) contract and
+// to remain correct on older Go toolchains or future changes.
+func TestGenerateAPIKey_Properties(t *testing.T) {
+	seen := make(map[string]struct{}, 64)
+	for i := 0; i < 64; i++ {
+		k, err := generateAPIKey()
+		if err != nil {
+			t.Fatalf("generateAPIKey failed: %v", err)
+		}
+		if k == "" {
+			t.Fatal("expected non-empty API key")
+		}
+		// base64.RawURLEncoding of 32 bytes yields exactly 43 chars.
+		if got, want := len(k), 43; got != want {
+			t.Fatalf("expected key length %d, got %d (%q)", want, got, k)
+		}
+		decoded, err := base64.RawURLEncoding.DecodeString(k)
+		if err != nil {
+			t.Fatalf("key %q not valid base64url: %v", k, err)
+		}
+		if len(decoded) != 32 {
+			t.Fatalf("expected 32 decoded bytes (256 bits entropy), got %d", len(decoded))
+		}
+		if _, dup := seen[k]; dup {
+			t.Fatalf("collision detected after %d calls; weak PRNG?", i+1)
+		}
+		seen[k] = struct{}{}
+	}
+}