feat(config): api_key_cmd / auth_token_cmd — resolve LLM credential from a command (#236)

cmd/opencodereview/config_cmd.go

@@ -189,6 +189,7 @@ func deleteCustomProvider(cfg *Config, name string) (bool, error) {
 // ProviderEntry holds per-provider configuration in the providers map.
 type ProviderEntry struct {
 	APIKey       string            `json:"api_key,omitempty"`
+	APIKeyCmd    string            `json:"api_key_cmd,omitempty"` // shell command whose stdout is the api key; used when api_key is empty
 	URL          string            `json:"url,omitempty"`
 	Protocol     string            `json:"protocol,omitempty"`
 	Model        string            `json:"model,omitempty"`
@@ -222,6 +223,7 @@ type Config struct {
 type LlmConfig struct {
 	URL          string            `json:"url,omitempty"`
 	AuthToken    string            `json:"auth_token,omitempty"`
+	AuthTokenCmd string            `json:"auth_token_cmd,omitempty"` // shell command whose stdout is the auth token; used when auth_token is empty
 	AuthHeader   string            `json:"auth_header,omitempty"`
 	Model        string            `json:"model,omitempty"`
 	Protocol     string            `json:"protocol,omitempty"`      // canonical protocol name; takes priority over UseAnthropic
@@ -326,6 +328,8 @@ func setConfigValue(cfg *Config, key, value string) error {
 		cfg.Llm.URL = value
 	case "llm.auth_token", "llm.AuthToken":
 		cfg.Llm.AuthToken = value
+	case "llm.auth_token_cmd", "llm.AuthTokenCmd":
+		cfg.Llm.AuthTokenCmd = value
 	case "llm.auth_header", "llm.AuthHeader":
 		normalized, err := llm.NormalizeAuthHeader(value)
 		if err != nil {
@@ -400,7 +404,7 @@ func setConfigValue(cfg *Config, key, value string) error {
 		}
 		cfg.Llm.ExtraBody = m
 	default:
-		return fmt.Errorf("unknown config key: %s\nSupported keys: provider, model, providers.<name>.<field>, custom_providers.<name>.<field>, mcp_servers.<name>.<field>, llm.url, llm.auth_token, llm.auth_header, llm.model, llm.protocol, llm.use_anthropic, llm.extra_body, llm.extra_headers, language, telemetry.enabled, telemetry.exporter, telemetry.otlp_endpoint, telemetry.content_logging\nProvider fields: api_key, url, protocol, model, models, auth_header, extra_body, extra_headers\nProtocol values: anthropic, openai, openai-responses\nMCP server fields: command, args, env, tools, setup", key)
+		return fmt.Errorf("unknown config key: %s\nSupported keys: provider, model, providers.<name>.<field>, custom_providers.<name>.<field>, mcp_servers.<name>.<field>, llm.url, llm.auth_token, llm.auth_token_cmd, llm.auth_header, llm.model, llm.protocol, llm.use_anthropic, llm.extra_body, llm.extra_headers, language, telemetry.enabled, telemetry.exporter, telemetry.otlp_endpoint, telemetry.content_logging\nProvider fields: api_key, api_key_cmd, url, protocol, model, models, auth_header, extra_body, extra_headers\nProtocol values: anthropic, openai, openai-responses\nMCP server fields: command, args, env, tools, setup", key)
 	}
 	return nil
 }
@@ -409,6 +413,8 @@ func applyProviderField(entry *ProviderEntry, field, key, value string) error {
 	switch field {
 	case "api_key":
 		entry.APIKey = value
+	case "api_key_cmd":
+		entry.APIKeyCmd = value
 	case "url":
 		entry.URL = value
 	case "protocol":
@@ -444,7 +450,7 @@ func applyProviderField(entry *ProviderEntry, field, key, value string) error {
 		}
 		entry.ExtraHeaders = parsed
 	default:
-		return fmt.Errorf("unknown provider field %q: supported fields are api_key, url, protocol, model, models, auth_header, extra_body, extra_headers", field)
+		return fmt.Errorf("unknown provider field %q: supported fields are api_key, api_key_cmd, url, protocol, model, models, auth_header, extra_body, extra_headers", field)
 	}
 	return nil
 }

cmd/opencodereview/provider_tui.go

@@ -1181,6 +1181,7 @@ func (m providerTUIModel) applyCreateCustomProvider() (tea.Model, tea.Cmd) {
 func cloneProviderEntry(v ProviderEntry) ProviderEntry {
 	out := ProviderEntry{
 		APIKey:     v.APIKey,
+		APIKeyCmd:  v.APIKeyCmd,
 		URL:        v.URL,
 		Protocol:   v.Protocol,
 		Model:      v.Model,

cmd/opencodereview/provider_tui_funcs_test.go

@@ -198,6 +198,7 @@ func TestRenderListName_Inactive(t *testing.T) {
 func TestCloneProviderEntry_WithExtraBody(t *testing.T) {
 	orig := ProviderEntry{
 		APIKey:     "key",
+		APIKeyCmd:  "op read op://dev/anthropic/api-key",
 		URL:        "http://localhost",
 		Protocol:   "openai",
 		Model:      "gpt-4",
@@ -210,6 +211,9 @@ func TestCloneProviderEntry_WithExtraBody(t *testing.T) {
 	if clone.APIKey != orig.APIKey || clone.URL != orig.URL || clone.Protocol != orig.Protocol {
 		t.Error("basic fields not copied")
 	}
+	if clone.APIKeyCmd != orig.APIKeyCmd {
+		t.Errorf("APIKeyCmd not copied: got %q, want %q", clone.APIKeyCmd, orig.APIKeyCmd)
+	}
 	if len(clone.Models) != 2 || clone.Models[0] != "gpt-4" {
 		t.Errorf("Models not cloned: %v", clone.Models)
 	}

internal/llm/keycmd.go

@@ -0,0 +1,50 @@
+package llm
+
+import (
+	"context"
+	"fmt"
+	"os"
+	"strings"
+	"time"
+)
+
+// keyCmdTimeout bounds how long an api_key_cmd / auth_token_cmd may run.
+// It is a package var (not const) so tests can shrink it.
+var keyCmdTimeout = 60 * time.Second
+
+// resolveKeyCmd runs a credential-fetching shell command and returns its
+// trimmed, single-line stdout. label names the source (e.g.
+// `api_key_cmd for provider "x"`) and is used in error messages.
+//
+// The child's stderr is wired to the process stderr so interactive prompts
+// (pinentry, 1Password, `op`) stay visible. Any failure is a hard error, never
+// a silent fallback. The resolved credential is used in memory only and is
+// never written to config or logged.
+func resolveKeyCmd(cmd, label string) (string, error) {
+	ctx, cancel := context.WithTimeout(context.Background(), keyCmdTimeout)
+	defer cancel()
+
+	c := newKeyCmd(ctx, cmd)
+	c.Stderr = os.Stderr
+
+	out, err := c.Output()
+	if ctx.Err() == context.DeadlineExceeded {
+		return "", fmt.Errorf("%s timed out after %s", label, keyCmdTimeout)
+	}
+	if err != nil {
+		// Covers non-zero exit and command-not-found (the shell exits non-zero
+		// and prints its not-found message on the child's stderr).
+		return "", fmt.Errorf("%s failed: %w", label, err)
+	}
+
+	// Trim a trailing line break; multi-line output past that is ambiguous and refused.
+	trimmed := strings.TrimRight(string(out), "\r\n")
+	if strings.Contains(trimmed, "\n") {
+		return "", fmt.Errorf("%s produced multi-line output; expected a single credential", label)
+	}
+	key := strings.TrimSpace(trimmed)
+	if key == "" {
+		return "", fmt.Errorf("%s produced empty output", label)
+	}
+	return key, nil
+}

internal/llm/keycmd_test.go

@@ -0,0 +1,73 @@
+//go:build !windows
+
+package llm
+
+import (
+	"strings"
+	"testing"
+	"time"
+)
+
+func TestResolveKeyCmd(t *testing.T) {
+	tests := []struct {
+		name    string
+		cmd     string
+		want    string
+		wantErr string // substring the error must contain; "" means success
+	}{
+		{name: "success", cmd: "printf 'sk-test\\n'", want: "sk-test"},
+		{name: "trailing whitespace trimmed", cmd: "printf '  sk-test  \\n'", want: "sk-test"},
+		{name: "no trailing newline", cmd: "printf 'sk-test'", want: "sk-test"},
+		{name: "crlf line ending trimmed", cmd: "printf 'sk-crlf\\r\\n'", want: "sk-crlf"},
+		{name: "non-zero exit", cmd: "exit 3", wantErr: "failed: exit status 3"},
+		{name: "false", cmd: "false", wantErr: "failed:"},
+		{name: "empty output", cmd: "true", wantErr: "produced empty output"},
+		{name: "empty printf", cmd: "printf ''", wantErr: "produced empty output"},
+		{name: "whitespace-only output", cmd: "printf '  \\n'", wantErr: "produced empty output"},
+		{name: "multi-line output", cmd: "printf 'a\\nb\\n'", wantErr: "produced multi-line output"},
+		{name: "command not found", cmd: "this-cmd-does-not-exist-xyz", wantErr: "failed:"},
+	}
+	for _, tt := range tests {
+		tt := tt
+		t.Run(tt.name, func(t *testing.T) {
+			t.Parallel()
+			got, err := resolveKeyCmd(tt.cmd, "api_key_cmd for provider \"x\"")
+			if tt.wantErr != "" {
+				if err == nil {
+					t.Fatalf("expected error containing %q, got nil (output %q)", tt.wantErr, got)
+				}
+				if !strings.Contains(err.Error(), tt.wantErr) {
+					t.Fatalf("error %q does not contain %q", err.Error(), tt.wantErr)
+				}
+				return
+			}
+			if err != nil {
+				t.Fatalf("unexpected error: %v", err)
+			}
+			if got != tt.want {
+				t.Fatalf("got %q, want %q", got, tt.want)
+			}
+		})
+	}
+}
+
+func TestResolveKeyCmd_Timeout(t *testing.T) {
+	orig := keyCmdTimeout
+	keyCmdTimeout = 50 * time.Millisecond
+	t.Cleanup(func() { keyCmdTimeout = orig })
+
+	_, err := resolveKeyCmd("sleep 5", "api_key_cmd for provider \"x\"")
+	if err == nil {
+		t.Fatal("expected timeout error, got nil")
+	}
+	if !strings.Contains(err.Error(), "timed out after") {
+		t.Fatalf("error %q does not mention timeout", err.Error())
+	}
+}
+
+func TestResolveKeyCmd_LabelInError(t *testing.T) {
+	_, err := resolveKeyCmd("false", `auth_token_cmd for llm config`)
+	if err == nil || !strings.HasPrefix(err.Error(), "auth_token_cmd for llm config") {
+		t.Fatalf("expected label prefix in error, got %v", err)
+	}
+}

internal/llm/keycmd_unix.go

@@ -0,0 +1,14 @@
+//go:build !windows
+
+package llm
+
+import (
+	"context"
+	"os/exec"
+)
+
+// newKeyCmd builds the OS-specific shell invocation (sh -c on Unix) that runs a
+// credential command under ctx, so its timeout and cancellation are honored.
+func newKeyCmd(ctx context.Context, cmd string) *exec.Cmd {
+	return exec.CommandContext(ctx, "sh", "-c", cmd)
+}

internal/llm/keycmd_windows.go

@@ -0,0 +1,14 @@
+//go:build windows
+
+package llm
+
+import (
+	"context"
+	"os/exec"
+)
+
+// newKeyCmd builds the OS-specific shell invocation (cmd /C on Windows) that runs a
+// credential command under ctx, so its timeout and cancellation are honored.
+func newKeyCmd(ctx context.Context, cmd string) *exec.Cmd {
+	return exec.CommandContext(ctx, "cmd", "/C", cmd)
+}

internal/llm/resolver.go

@@ -212,16 +212,18 @@ type llmFileConfig struct {
 	AuthToken    string            `json:"auth_token,omitempty"`
 	AuthHeader   string            `json:"auth_header,omitempty"`
 	Model        string            `json:"model,omitempty"`
-	Protocol     string            `json:"protocol,omitempty"`      // anthropic|openai|openai-responses; takes priority over use_anthropic
-	UseAnthropic *bool             `json:"use_anthropic,omitempty"` // pointer to distinguish unset from false; legacy fallback when protocol is empty
-	TimeoutSec   int               `json:"timeout_sec,omitempty"`   // per-request HTTP timeout in seconds
+	AuthTokenCmd string            `json:"auth_token_cmd,omitempty"` // shell command whose stdout is the auth token; used when auth_token is empty
+	Protocol     string            `json:"protocol,omitempty"`       // anthropic|openai|openai-responses; takes priority over use_anthropic
+	UseAnthropic *bool             `json:"use_anthropic,omitempty"`  // pointer to distinguish unset from false; legacy fallback when protocol is empty
+	TimeoutSec   int               `json:"timeout_sec,omitempty"`    // per-request HTTP timeout in seconds
 	ExtraBody    map[string]any    `json:"extra_body,omitempty"`
 	ExtraHeaders map[string]string `json:"extra_headers,omitempty"`
 }
 
 // providerEntryConfig represents a single provider entry in config.json.
 type providerEntryConfig struct {
 	APIKey       string            `json:"api_key,omitempty"`
+	APIKeyCmd    string            `json:"api_key_cmd,omitempty"` // shell command whose stdout is the api key; used when api_key is empty
 	URL          string            `json:"url,omitempty"`
 	Protocol     string            `json:"protocol,omitempty"`
 	Model        string            `json:"model,omitempty"`
@@ -282,13 +284,24 @@ func tryProviderConfig(cfg configFile, modelOverride string) (ResolvedEndpoint,
 	}
 
 	apiKey := entry.APIKey
-	if apiKey == "" {
-		if isPreset && preset.EnvVar != "" {
-			apiKey = os.Getenv(preset.EnvVar)
+	switch {
+	case apiKey != "":
+		// Static api_key always wins. Warn (don't error) if a command is also set,
+		// so a config that keeps api_key_cmd as a deliberate fallback still works.
+		if entry.APIKeyCmd != "" {
+			fmt.Fprintf(os.Stderr, "warning: provider %q has both api_key and api_key_cmd set; using the static api_key\n", cfg.Provider)
+		}
+	case entry.APIKeyCmd != "":
+		resolved, err := resolveKeyCmd(entry.APIKeyCmd, fmt.Sprintf("api_key_cmd for provider %q", cfg.Provider))
+		if err != nil {
+			return ResolvedEndpoint{}, false, err
 		}
+		apiKey = resolved
+	case isPreset && preset.EnvVar != "":
+		apiKey = os.Getenv(preset.EnvVar)
 	}
 	if apiKey == "" {
-		return ResolvedEndpoint{}, false, fmt.Errorf("provider %q has no api_key configured and no environment variable fallback found", cfg.Provider)
+		return ResolvedEndpoint{}, false, fmt.Errorf("provider %q has no api_key or api_key_cmd configured and no environment variable fallback found", cfg.Provider)
 	}
 
 	var url, protocol, authHeader, model string
@@ -408,9 +421,27 @@ func tryLegacyLlmConfig(cfg configFile, modelOverride string) (ResolvedEndpoint,
 	if modelOverride != "" {
 		model = modelOverride
 	}
-	if cfg.Llm.URL == "" || cfg.Llm.AuthToken == "" || model == "" {
+	// Fall through to later strategies when the legacy block is incomplete. This
+	// includes the case where neither auth_token nor auth_token_cmd is set — and,
+	// critically, an incomplete block (e.g. missing url) never runs auth_token_cmd.
+	token := cfg.Llm.AuthToken
+	if cfg.Llm.URL == "" || model == "" || (token == "" && cfg.Llm.AuthTokenCmd == "") {
 		return ResolvedEndpoint{}, false, nil
 	}
+	switch {
+	case token != "":
+		// Static auth_token always wins; warn if a command is also set.
+		if cfg.Llm.AuthTokenCmd != "" {
+			fmt.Fprintf(os.Stderr, "warning: llm config has both auth_token and auth_token_cmd set; using the static auth_token\n")
+		}
+	case cfg.Llm.AuthTokenCmd != "":
+		// Otherwise-complete legacy block with a set-but-failing command is a hard error.
+		resolved, err := resolveKeyCmd(cfg.Llm.AuthTokenCmd, "auth_token_cmd for llm config")
+		if err != nil {
+			return ResolvedEndpoint{}, false, err
+		}
+		token = resolved
+	}
 
 	// llm.protocol (normalized) wins over use_anthropic when set.
 	protocol := ""
@@ -449,7 +480,7 @@ func tryLegacyLlmConfig(cfg configFile, modelOverride string) (ResolvedEndpoint,
 		return ResolvedEndpoint{}, false, fmt.Errorf("OCR config file: %w", err)
 	}
 
-	return ResolvedEndpoint{URL: cfg.Llm.URL, Token: cfg.Llm.AuthToken, Model: model, Protocol: protocol, AuthHeader: authHeader, Source: "OCR config file", ExtraBody: cfg.Llm.ExtraBody, ExtraHeaders: cfg.Llm.ExtraHeaders, Timeout: timeout}, true, nil
+	return ResolvedEndpoint{URL: cfg.Llm.URL, Token: token, Model: model, Protocol: protocol, AuthHeader: authHeader, Source: "OCR config file", ExtraBody: cfg.Llm.ExtraBody, ExtraHeaders: cfg.Llm.ExtraHeaders, Timeout: timeout}, true, nil
 }
 
 // tryCCEnv reads Claude Code environment variables.