diff --git a/cmd/heygen/auth_status.go b/cmd/heygen/auth_status.go index 1151619..0b8b048 100644 --- a/cmd/heygen/auth_status.go +++ b/cmd/heygen/auth_status.go @@ -1,13 +1,11 @@ package main import ( - "errors" "net/url" "github.com/heygen-com/heygen-cli/gen" "github.com/heygen-com/heygen-cli/internal/client" "github.com/heygen-com/heygen-cli/internal/command" - clierrors "github.com/heygen-com/heygen-cli/internal/errors" "github.com/spf13/cobra" ) @@ -24,10 +22,6 @@ func newAuthStatusCmd(ctx *cmdContext) *cobra.Command { QueryParams: make(url.Values), }) if err != nil { - var cliErr *clierrors.CLIError - if errors.As(err, &cliErr) && cliErr.ExitCode == clierrors.ExitAuth { - cliErr.Hint = "Your API key is missing or invalid.\n" + authGuidance - } return err } return ctx.formatter.Data(result, client.APIDataField, nil) diff --git a/cmd/heygen/auth_status_test.go b/cmd/heygen/auth_status_test.go index 50867df..a63c4fa 100644 --- a/cmd/heygen/auth_status_test.go +++ b/cmd/heygen/auth_status_test.go @@ -62,7 +62,7 @@ func TestAuthStatus_InvalidKey(t *testing.T) { } // TestAuthStatus_AuthError_AddsHint verifies that a 401 from the API gets the -// actionable auth hint injected by auth_status.go. +// source-aware auth hint from centralized enrichment in the error path. func TestAuthStatus_AuthError_AddsHint(t *testing.T) { srv := setupTestServer(t, map[string]testHandler{ "GET /v3/users/me": { @@ -72,21 +72,23 @@ func TestAuthStatus_AuthError_AddsHint(t *testing.T) { }) defer srv.Close() + // runCommand passes apiKey via HEYGEN_API_KEY env var, so the hint + // should reference the environment variable source. res := runCommand(t, srv.URL, "bad-key", "auth", "status") if res.ExitCode != clierrors.ExitAuth { t.Fatalf("ExitCode = %d, want %d", res.ExitCode, clierrors.ExitAuth) } - if !strings.Contains(res.Stderr, "HEYGEN_API_KEY") { - t.Fatalf("stderr = %s, want HEYGEN_API_KEY in hint", res.Stderr) + if !strings.Contains(res.Stderr, "HEYGEN_API_KEY environment variable") { + t.Fatalf("stderr should mention env var source:\n%s", res.Stderr) } - if !strings.Contains(res.Stderr, "heygen auth login") { - t.Fatalf("stderr = %s, want heygen auth login in hint", res.Stderr) + if !strings.Contains(res.Stderr, "app.heygen.com/settings/api") { + t.Fatalf("stderr should contain key generation URL:\n%s", res.Stderr) } } // TestAuthStatus_NonAuthError_NoHintMutation verifies that non-auth errors are -// not mutated by the auth hint injection in auth_status.go. +// not mutated by the centralized auth hint enrichment. func TestAuthStatus_NonAuthError_NoHintMutation(t *testing.T) { srv := setupTestServer(t, map[string]testHandler{ "GET /v3/users/me": { diff --git a/cmd/heygen/context.go b/cmd/heygen/context.go index 7ba7e62..d7dbe74 100644 --- a/cmd/heygen/context.go +++ b/cmd/heygen/context.go @@ -1,15 +1,19 @@ package main import ( + "context" "errors" + "fmt" "net/url" "os" + "path/filepath" "github.com/heygen-com/heygen-cli/internal/auth" "github.com/heygen-com/heygen-cli/internal/client" "github.com/heygen-com/heygen-cli/internal/config" clierrors "github.com/heygen-com/heygen-cli/internal/errors" "github.com/heygen-com/heygen-cli/internal/output" + "github.com/heygen-com/heygen-cli/internal/paths" "github.com/spf13/cobra" ) @@ -49,6 +53,38 @@ func schemaFlagEnabled(cmd *cobra.Command, name string) bool { return err == nil && enabled } +type credSourceKey struct{} + +// credSourceFromCmd retrieves the credential source stored on the command +// context during initContext. Returns "" if not set (e.g. skipAuth commands +// or when credential resolution failed before storing). +func credSourceFromCmd(cmd *cobra.Command) auth.CredentialSource { + if cmd == nil { + return "" + } + src, _ := cmd.Context().Value(credSourceKey{}).(auth.CredentialSource) + return src +} + +// enrichAuthHint sets a source-aware hint on auth errors that don't already +// have one. Called once in the centralized error path (main.go / testutil), +// not scattered across individual commands. +func enrichAuthHint(cliErr *clierrors.CLIError, source auth.CredentialSource) { + if cliErr.ExitCode != clierrors.ExitAuth { + return + } + if cliErr.Hint != "" { + return + } + credPath := filepath.Join(paths.ConfigDir(), "credentials") + switch source { + case auth.SourceEnv: + cliErr.Hint = "The HEYGEN_API_KEY environment variable contains an invalid or expired key.\nGenerate a new key: https://app.heygen.com/settings/api" + case auth.SourceFile: + cliErr.Hint = fmt.Sprintf("The stored API key (%s) is invalid or expired.\nReplace it: heygen auth login\nGenerate a new key: https://app.heygen.com/settings/api", credPath) + } +} + // initContext sets up the config provider and, for commands that require // auth, resolves credentials and creates the HTTP client. func initContext(cmd *cobra.Command, version string, ctx *cmdContext) error { @@ -69,7 +105,7 @@ func initContext(cmd *cobra.Command, version string, ctx *cmdContext) error { &auth.FileCredentialResolver{}, }, } - apiKey, err := resolver.Resolve() + result, err := resolver.ResolveWithSource() if err != nil { // Enrich the generic cold-start auth error ("no API key found") // with the full auth guidance. Don't overwrite specific hints @@ -80,13 +116,14 @@ func initContext(cmd *cobra.Command, version string, ctx *cmdContext) error { } return err } + cmd.SetContext(context.WithValue(cmd.Context(), credSourceKey{}, result.Source)) baseURL := provider.BaseURL() if u, err := url.Parse(baseURL); err == nil && u.Scheme == "http" && os.Getenv("HEYGEN_ALLOW_HTTP") == "" { return clierrors.NewUsage("HEYGEN_API_BASE uses HTTP which transmits API keys in plaintext. Set HEYGEN_ALLOW_HTTP=1 to allow.") } - ctx.client = client.New(apiKey, + ctx.client = client.New(result.Key, client.WithBaseURL(baseURL), client.WithUserAgent("heygen-cli/"+version), ) diff --git a/cmd/heygen/context_test.go b/cmd/heygen/context_test.go new file mode 100644 index 0000000..ae027b8 --- /dev/null +++ b/cmd/heygen/context_test.go @@ -0,0 +1,96 @@ +package main + +import ( + "encoding/json" + "os" + "path/filepath" + "strings" + "testing" + + clierrors "github.com/heygen-com/heygen-cli/internal/errors" +) + +func TestEnrichAuthHint_EnvSource_401(t *testing.T) { + srv := setupTestServer(t, map[string]testHandler{ + "GET /v3/videos": { + StatusCode: 401, + Body: `{"error":{"code":"unauthorized","message":"invalid API key"}}`, + }, + }) + defer srv.Close() + + res := runCommand(t, srv.URL, "bad-key", "video", "list") + + if res.ExitCode != clierrors.ExitAuth { + t.Fatalf("ExitCode = %d, want %d\nstderr: %s", res.ExitCode, clierrors.ExitAuth, res.Stderr) + } + if !strings.Contains(res.Stderr, "HEYGEN_API_KEY environment variable") { + t.Fatalf("stderr should mention env var source:\n%s", res.Stderr) + } + if !strings.Contains(res.Stderr, "app.heygen.com/settings/api") { + t.Fatalf("stderr should contain key generation URL:\n%s", res.Stderr) + } +} + +func TestEnrichAuthHint_FileSource_401(t *testing.T) { + srv := setupTestServer(t, map[string]testHandler{ + "GET /v3/videos": { + StatusCode: 401, + Body: `{"error":{"code":"unauthorized","message":"invalid API key"}}`, + }, + }) + defer srv.Close() + + dir := t.TempDir() + if err := os.WriteFile(filepath.Join(dir, "credentials"), []byte("bad-key"), 0600); err != nil { + t.Fatal(err) + } + t.Setenv("HEYGEN_CONFIG_DIR", dir) + t.Setenv("HEYGEN_API_KEY", "") + + res := runCommand(t, srv.URL, "", "video", "list") + + if res.ExitCode != clierrors.ExitAuth { + t.Fatalf("ExitCode = %d, want %d\nstderr: %s", res.ExitCode, clierrors.ExitAuth, res.Stderr) + } + if !strings.Contains(res.Stderr, "heygen auth login") { + t.Fatalf("stderr should mention auth login for file source:\n%s", res.Stderr) + } + pathJSON, _ := json.Marshal(filepath.Join(dir, "credentials")) + escapedPath := string(pathJSON[1 : len(pathJSON)-1]) + if !strings.Contains(res.Stderr, escapedPath) { + t.Fatalf("stderr should mention the resolved credentials path:\n%s", res.Stderr) + } +} + +func TestEnrichAuthHint_NonAuthError_NoMutation(t *testing.T) { + srv := setupTestServer(t, map[string]testHandler{ + "GET /v3/videos": { + StatusCode: 500, + Body: `{"error":{"code":"internal_error","message":"server error"}}`, + }, + }) + defer srv.Close() + + res := runCommand(t, srv.URL, "test-key", "video", "list") + + if res.ExitCode != clierrors.ExitGeneral { + t.Fatalf("ExitCode = %d, want %d", res.ExitCode, clierrors.ExitGeneral) + } + if strings.Contains(res.Stderr, "HEYGEN_API_KEY environment variable") { + t.Fatalf("non-auth error should not get auth hint:\n%s", res.Stderr) + } +} + +func TestEnrichAuthHint_ExistingHint_Preserved(t *testing.T) { + t.Setenv("HEYGEN_CONFIG_DIR", t.TempDir()) + + res := runCommand(t, "http://example.invalid", "", "video", "list") + + if res.ExitCode != clierrors.ExitAuth { + t.Fatalf("ExitCode = %d, want %d\nstderr: %s", res.ExitCode, clierrors.ExitAuth, res.Stderr) + } + if !strings.Contains(res.Stderr, "Three ways to provide your API key") { + t.Fatalf("no-key error should preserve authGuidance hint:\n%s", res.Stderr) + } +} diff --git a/cmd/heygen/main.go b/cmd/heygen/main.go index 582e933..fe4dc73 100644 --- a/cmd/heygen/main.go +++ b/cmd/heygen/main.go @@ -29,6 +29,7 @@ func main() { if err != nil { var cliErr *clierrors.CLIError if errors.As(err, &cliErr) { + enrichAuthHint(cliErr, credSourceFromCmd(executedCmd)) formatter.Error(cliErr) exitCode = cliErr.ExitCode errorCode = cliErr.Code diff --git a/cmd/heygen/testutil_test.go b/cmd/heygen/testutil_test.go index a94fdc7..1ba1aa2 100644 --- a/cmd/heygen/testutil_test.go +++ b/cmd/heygen/testutil_test.go @@ -98,15 +98,16 @@ func runCommandWithInput(t *testing.T, serverURL, apiKey string, stdin io.Reader cmd.SetIn(stdin) } - err := cmd.Execute() + executedCmd, err := cmd.ExecuteC() var exitCode int if err != nil { // Render through formatter — same path as main() - // Mirror the classification logic from main() so tests - // see the same exit codes production emits. + // Mirror the classification and auth-hint logic from main() so + // tests see the same exit codes and hints production emits. var cliErr *clierrors.CLIError if errors.As(err, &cliErr) { + enrichAuthHint(cliErr, credSourceFromCmd(executedCmd)) formatter.Error(cliErr) exitCode = cliErr.ExitCode } else { diff --git a/internal/auth/chain_resolver.go b/internal/auth/chain_resolver.go index f80c881..5031f21 100644 --- a/internal/auth/chain_resolver.go +++ b/internal/auth/chain_resolver.go @@ -16,10 +16,21 @@ type ChainCredentialResolver struct { // Resolve returns the first successful credential. Absent sources are skipped; // broken sources surface immediately. func (c *ChainCredentialResolver) Resolve() (string, error) { + result, err := c.ResolveWithSource() + if err != nil { + return "", err + } + return result.Key, nil +} + +// ResolveWithSource returns the first successful credential along with which +// resolver provided it. The Source tag lets callers produce source-aware error +// messages (e.g. "your env var is invalid" vs "your stored key is invalid"). +func (c *ChainCredentialResolver) ResolveWithSource() (CredentialResult, error) { for _, r := range c.Resolvers { key, err := r.Resolve() if err == nil { - return key, nil + return CredentialResult{Key: key, Source: sourceForResolver(r)}, nil } var notConfigured *ErrNotConfigured @@ -28,11 +39,22 @@ func (c *ChainCredentialResolver) Resolve() (string, error) { } credPath := filepath.Join(paths.ConfigDir(), "credentials") - return "", clierrors.NewAuth(err.Error(), "Check the credentials file at "+credPath) + return CredentialResult{}, clierrors.NewAuth(err.Error(), "Check the credentials file at "+credPath) } - return "", clierrors.NewAuth( + return CredentialResult{}, clierrors.NewAuth( "no API key found", "Set HEYGEN_API_KEY env var or run: heygen auth login", ) } + +func sourceForResolver(r CredentialResolver) CredentialSource { + switch r.(type) { + case *EnvCredentialResolver: + return SourceEnv + case *FileCredentialResolver: + return SourceFile + default: + return "" + } +} diff --git a/internal/auth/chain_resolver_test.go b/internal/auth/chain_resolver_test.go index a6814f1..1be1670 100644 --- a/internal/auth/chain_resolver_test.go +++ b/internal/auth/chain_resolver_test.go @@ -2,6 +2,8 @@ package auth import ( "errors" + "os" + "path/filepath" "testing" clierrors "github.com/heygen-com/heygen-cli/internal/errors" @@ -105,3 +107,78 @@ func TestChainResolver_BrokenSource(t *testing.T) { t.Fatalf("Message = %q, want %q", cliErr.Message, "permission denied") } } + +func TestResolveWithSource_EnvWins(t *testing.T) { + t.Setenv("HEYGEN_API_KEY", "env-key") + dir := t.TempDir() + t.Setenv("HEYGEN_CONFIG_DIR", dir) + _ = os.WriteFile(filepath.Join(dir, "credentials"), []byte("file-key"), 0600) + + r := &ChainCredentialResolver{ + Resolvers: []CredentialResolver{ + &EnvCredentialResolver{}, + &FileCredentialResolver{}, + }, + } + + result, err := r.ResolveWithSource() + if err != nil { + t.Fatalf("ResolveWithSource: %v", err) + } + if result.Key != "env-key" { + t.Fatalf("Key = %q, want %q", result.Key, "env-key") + } + if result.Source != SourceEnv { + t.Fatalf("Source = %q, want %q", result.Source, SourceEnv) + } +} + +func TestResolveWithSource_FallsToFile(t *testing.T) { + t.Setenv("HEYGEN_API_KEY", "") + dir := t.TempDir() + t.Setenv("HEYGEN_CONFIG_DIR", dir) + _ = os.WriteFile(filepath.Join(dir, "credentials"), []byte("file-key"), 0600) + + r := &ChainCredentialResolver{ + Resolvers: []CredentialResolver{ + &EnvCredentialResolver{}, + &FileCredentialResolver{}, + }, + } + + result, err := r.ResolveWithSource() + if err != nil { + t.Fatalf("ResolveWithSource: %v", err) + } + if result.Key != "file-key" { + t.Fatalf("Key = %q, want %q", result.Key, "file-key") + } + if result.Source != SourceFile { + t.Fatalf("Source = %q, want %q", result.Source, SourceFile) + } +} + +func TestResolveWithSource_NoneConfigured(t *testing.T) { + t.Setenv("HEYGEN_API_KEY", "") + t.Setenv("HEYGEN_CONFIG_DIR", t.TempDir()) + + r := &ChainCredentialResolver{ + Resolvers: []CredentialResolver{ + &EnvCredentialResolver{}, + &FileCredentialResolver{}, + }, + } + + _, err := r.ResolveWithSource() + if err == nil { + t.Fatal("expected error, got nil") + } + + var cliErr *clierrors.CLIError + if !errors.As(err, &cliErr) { + t.Fatalf("expected *CLIError, got %T", err) + } + if cliErr.ExitCode != clierrors.ExitAuth { + t.Fatalf("ExitCode = %d, want %d", cliErr.ExitCode, clierrors.ExitAuth) + } +} diff --git a/internal/auth/resolver.go b/internal/auth/resolver.go index a296229..0f96b48 100644 --- a/internal/auth/resolver.go +++ b/internal/auth/resolver.go @@ -1,5 +1,19 @@ package auth +// CredentialSource identifies where a credential was resolved from. +type CredentialSource string + +const ( + SourceEnv CredentialSource = "env" + SourceFile CredentialSource = "file" +) + +// CredentialResult pairs a resolved API key with its source. +type CredentialResult struct { + Key string + Source CredentialSource +} + // CredentialResolver locates an API key from a credential source. // Implementations include environment and file-based resolvers. type CredentialResolver interface {