From fa6c7d8e04772ca1aaad13ad13a036155d73f59f Mon Sep 17 00:00:00 2001 From: kite Date: Mon, 8 Jun 2026 20:21:28 +0800 Subject: [PATCH] refactor(llm): deduplicate and validate auth header normalization Export NormalizeAuthHeader from the llm package and remove the duplicate normalizeConfigAuthHeader in config_cmd.go. Invalid auth_header values now return an error at configuration time instead of being silently accepted and causing opaque 401s at runtime. --- cmd/opencodereview/config_cmd.go | 21 +++++++-------------- internal/llm/client.go | 4 ++-- internal/llm/resolver.go | 25 +++++++++++++++++++------ 3 files changed, 28 insertions(+), 22 deletions(-) diff --git a/cmd/opencodereview/config_cmd.go b/cmd/opencodereview/config_cmd.go index ccbd1df..6cf1aca 100644 --- a/cmd/opencodereview/config_cmd.go +++ b/cmd/opencodereview/config_cmd.go @@ -6,7 +6,8 @@ import ( "os" "path/filepath" "strconv" - "strings" + + "github.com/open-code-review/open-code-review/internal/llm" ) // Default config file location: ~/.opencodereview/config.json @@ -132,7 +133,11 @@ func setConfigValue(cfg *Config, key, value string) error { case "llm.auth_token", "llm.AuthToken": cfg.Llm.AuthToken = value case "llm.auth_header", "llm.AuthHeader": - cfg.Llm.AuthHeader = normalizeConfigAuthHeader(value) + normalized, err := llm.NormalizeAuthHeader(value) + if err != nil { + return err + } + cfg.Llm.AuthHeader = normalized case "llm.model", "llm.Model": cfg.Llm.Model = value case "llm.use_anthropic", "llm.UseAnthropic": @@ -175,18 +180,6 @@ func setConfigValue(cfg *Config, key, value string) error { return nil } -func normalizeConfigAuthHeader(header string) string { - header = strings.TrimSpace(header) - switch strings.ToLower(header) { - case "x-api-key": - return "x-api-key" - case "authorization", "bearer": - return "authorization" - default: - return header - } -} - func (c *Config) ensureTelemetry() { if c.Telemetry == nil { c.Telemetry = &TelemetryConfig{} diff --git a/internal/llm/client.go b/internal/llm/client.go index 5e6833b..de54322 100644 --- a/internal/llm/client.go +++ b/internal/llm/client.go @@ -182,7 +182,7 @@ type ClientConfig struct { URL string // Full API endpoint URL APIKey string // Bearer token / API key Model string // Default model override - AuthHeader string // Anthropic auth header: "x-api-key" or "authorization" + AuthHeader string // Auth header name: "x-api-key", "authorization", or empty for protocol default Timeout time.Duration // Request timeout ExtraBody map[string]any // Vendor-specific fields merged into every request body } @@ -484,7 +484,7 @@ func NewAnthropicClient(cfg ClientConfig) *AnthropicClient { } sdkBaseURL := strings.TrimSuffix(strings.TrimRight(cfg.URL, "/"), "/v1/messages") - authHeader := normalizeAuthHeader(cfg.AuthHeader) + authHeader, _ := NormalizeAuthHeader(cfg.AuthHeader) if authHeader == "" { authHeader = "authorization" } diff --git a/internal/llm/resolver.go b/internal/llm/resolver.go index 1e1269a..a5075a0 100644 --- a/internal/llm/resolver.go +++ b/internal/llm/resolver.go @@ -87,7 +87,11 @@ func tryOCREnv() (ResolvedEndpoint, bool, error) { var authHeader string if protocol == "anthropic" { - authHeader = normalizeAuthHeader(os.Getenv(envOCRLLMAuthHeader)) + var err error + authHeader, err = NormalizeAuthHeader(os.Getenv(envOCRLLMAuthHeader)) + if err != nil { + return ResolvedEndpoint{}, false, fmt.Errorf("OCR environment: %w", err) + } if authHeader == "" { authHeader = defaultAuthHeader(protocol) } @@ -141,7 +145,11 @@ func tryOCRConfig(path string) (ResolvedEndpoint, bool, error) { var authHeader string if protocol == "anthropic" { - authHeader = normalizeAuthHeader(cfg.Llm.AuthHeader) + var err error + authHeader, err = NormalizeAuthHeader(cfg.Llm.AuthHeader) + if err != nil { + return ResolvedEndpoint{}, false, fmt.Errorf("OCR config file: %w", err) + } if authHeader == "" { authHeader = defaultAuthHeader(protocol) } @@ -256,15 +264,20 @@ func defaultAuthHeader(protocol string) string { return "" } -func normalizeAuthHeader(header string) string { +// NormalizeAuthHeader normalizes an auth header value to a canonical form. +// It returns an error for unrecognized values. +func NormalizeAuthHeader(header string) (string, error) { header = strings.TrimSpace(header) + if header == "" { + return "", nil + } switch strings.ToLower(header) { case "x-api-key": - return "x-api-key" + return "x-api-key", nil case "authorization", "bearer": - return "authorization" + return "authorization", nil default: - return header + return "", fmt.Errorf("unsupported auth_header value %q; expected \"x-api-key\" or \"authorization\"", header) } }