From c13dc4fd29b3bb54f987f554e531f48a05009237 Mon Sep 17 00:00:00 2001 From: Tim Thacker Date: Tue, 26 May 2026 20:10:53 +1000 Subject: [PATCH] refactor(cli): RunE migration, auth dedup, TTY-aware spinner, non-interactive guards Command hygiene refactor for the hand-written CLI commands. Behavior (including exit codes and output streams) is preserved. - os.Exit -> RunE: convert bespoke command bodies (auth, fix, status, findings, ci, open, whoami, init, mcp, pentest, repos, update) to return errors instead of calling os.Exit mid-body, so deferred logger.Close runs and the commands are testable. Centralize error->exit-code mapping in main.go via a typed exitCodeError and ExitCodeForError; preserves ExitAuthError(2), ExitNetworkError(3), and ExitFindings(1). Set rootCmd.SilenceUsage=true / SilenceErrors=false so cobra prints each error once. - Auth boilerplate dedup: route fix/status/findings/ci through the existing resolveCommandAuth helper (now error-returning) instead of repeating resolveHost->GetNullifyToken->NewNullifyClient-> LoadCredentials. Added commandAuthContext.Client(). - TTY-aware spinner: NewSpinner stays silent when stderr is not a terminal, avoiding ANSI/braille noise in CI logs. - Non-interactive guards: auth login host prompt and the init wizard domain step now fail fast with a clear error when stdin is not a TTY instead of blocking on a read. The generated `api` commands and their getAPIClient factory are unchanged (resolveHost retains its os.Exit wrapper for that path). Co-Authored-By: Claude Opus 4.7 (1M context) --- cmd/cli/cmd/auth.go | 88 ++++++++++++++++++----------------- cmd/cli/cmd/ci.go | 74 +++++++++++------------------ cmd/cli/cmd/exitcodes.go | 67 ++++++++++++++++++++++++++ cmd/cli/cmd/exitcodes_test.go | 40 ++++++++++++++++ cmd/cli/cmd/findings.go | 32 ++++--------- cmd/cli/cmd/fix.go | 35 ++++---------- cmd/cli/cmd/init.go | 7 ++- cmd/cli/cmd/mcp.go | 25 +++------- cmd/cli/cmd/open.go | 12 +++-- cmd/cli/cmd/pentest.go | 19 ++++---- cmd/cli/cmd/repos.go | 6 +-- cmd/cli/cmd/root.go | 55 ++++++++++++++-------- cmd/cli/cmd/runtime_auth.go | 28 ++++++++++- cmd/cli/cmd/status.go | 39 ++++------------ cmd/cli/cmd/update.go | 9 ++-- cmd/cli/cmd/whoami.go | 12 +++-- cmd/cli/main.go | 4 +- internal/output/spinner.go | 14 +++++- internal/wizard/steps.go | 14 ++++++ 19 files changed, 339 insertions(+), 241 deletions(-) create mode 100644 cmd/cli/cmd/exitcodes_test.go diff --git a/cmd/cli/cmd/auth.go b/cmd/cli/cmd/auth.go index a6fa5a8..518bfde 100644 --- a/cmd/cli/cmd/auth.go +++ b/cmd/cli/cmd/auth.go @@ -1,7 +1,6 @@ package cmd import ( - "context" "encoding/json" "fmt" "os" @@ -25,7 +24,7 @@ var loginCmd = &cobra.Command{ Use: "login", Short: "Log in to Nullify", Long: "Authenticate with your Nullify instance. Opens your browser to log in with your identity provider.", - Run: func(cmd *cobra.Command, args []string) { + RunE: func(cmd *cobra.Command, args []string) error { ctx := setupLogger(cmd.Context()) defer logger.Close(ctx) @@ -43,23 +42,24 @@ var loginCmd = &cobra.Command{ } } - // If still no host, prompt + // If still no host, prompt (only when running interactively). if loginHost == "" { + if !stdinIsTTY() { + return fmt.Errorf("not a terminal; pass --host .nullify.ai") + } fmt.Print("Enter your Nullify instance (e.g., acme.nullify.ai): ") _, _ = fmt.Scanln(&loginHost) } sanitizedHost, err := lib.SanitizeNullifyHost(loginHost) if err != nil { - fmt.Fprintf(os.Stderr, "Error: invalid host %q - must be in the format .nullify.ai\n", loginHost) - os.Exit(1) + return fmt.Errorf("invalid host %q - must be in the format .nullify.ai", loginHost) } - err = auth.Login(ctx, sanitizedHost) - if err != nil { - fmt.Fprintf(os.Stderr, "Error: login failed: %v\n", err) - os.Exit(1) + if err := auth.Login(ctx, sanitizedHost); err != nil { + return fmt.Errorf("login failed: %w", err) } + return nil }, } @@ -67,19 +67,21 @@ var logoutCmd = &cobra.Command{ Use: "logout", Short: "Log out of Nullify", Long: "Clear stored credentials for the current or specified host.", - Run: func(cmd *cobra.Command, args []string) { + RunE: func(cmd *cobra.Command, args []string) error { ctx := setupLogger(cmd.Context()) defer logger.Close(ctx) - logoutHost := resolveHostForAuth(ctx) - - err := auth.Logout(logoutHost) + logoutHost, err := resolveHostForAuth() if err != nil { - fmt.Fprintf(os.Stderr, "Error: logout failed: %v\n", err) - os.Exit(1) + return err + } + + if err := auth.Logout(logoutHost); err != nil { + return fmt.Errorf("logout failed: %w", err) } fmt.Printf("Logged out from %s\n", logoutHost) + return nil }, } @@ -87,16 +89,16 @@ var statusCmd = &cobra.Command{ Use: "status", Short: "Show authentication status", Long: "Display the current authentication state including host, user, and token expiry.", - Run: func(cmd *cobra.Command, args []string) { + RunE: func(cmd *cobra.Command, args []string) error { cfg, err := auth.LoadConfig() if err != nil { fmt.Println("Not configured. Run 'nullify auth login --host .nullify.ai' to get started.") - return + return nil } if cfg.Host == "" { fmt.Println("No host configured. Run 'nullify auth login --host .nullify.ai'") - return + return nil } fmt.Printf("Host: %s\n", cfg.Host) @@ -104,13 +106,13 @@ var statusCmd = &cobra.Command{ creds, err := auth.LoadCredentials() if err != nil { fmt.Println("Status: not authenticated") - return + return nil } hostCreds, ok := creds[auth.CredentialKey(cfg.Host)] if !ok { fmt.Println("Status: not authenticated") - return + return nil } if hostCreds.ExpiresAt > 0 { @@ -123,6 +125,7 @@ var statusCmd = &cobra.Command{ } else { fmt.Println("Status: authenticated") } + return nil }, } @@ -130,19 +133,22 @@ var tokenCmd = &cobra.Command{ Use: "token", Short: "Print access token to stdout", Long: "Print the current access token. Useful for piping to other tools.", - Run: func(cmd *cobra.Command, args []string) { + RunE: func(cmd *cobra.Command, args []string) error { ctx := setupLogger(cmd.Context()) defer logger.Close(ctx) - hostForToken := resolveHostForAuth(ctx) + hostForToken, err := resolveHostForAuth() + if err != nil { + return err + } token, err := auth.GetValidToken(ctx, hostForToken) if err != nil { - fmt.Fprintf(os.Stderr, "Error: %v\n", err) - os.Exit(1) + return err } fmt.Println(token) + return nil }, } @@ -150,7 +156,7 @@ var switchCmd = &cobra.Command{ Use: "switch", Short: "Switch between configured hosts", Long: "Switch the default host when multiple Nullify instances are configured.", - Run: func(cmd *cobra.Command, args []string) { + RunE: func(cmd *cobra.Command, args []string) error { switchHost, _ := cmd.Flags().GetString("host") if switchHost == "" && host != "" { switchHost = host @@ -161,7 +167,7 @@ var switchCmd = &cobra.Command{ creds, err := auth.LoadCredentials() if err != nil || len(creds) == 0 { fmt.Println("No configured hosts. Run 'nullify auth login --host .nullify.ai'") - return + return nil } cfg, _ := auth.LoadConfig() @@ -174,13 +180,12 @@ var switchCmd = &cobra.Command{ fmt.Printf("%s%s\n", marker, h) } fmt.Println("\nUse 'nullify auth switch --host ' to switch.") - return + return nil } sanitizedHost, err := lib.SanitizeNullifyHost(switchHost) if err != nil { - fmt.Fprintf(os.Stderr, "Error: invalid host %q\n", switchHost) - os.Exit(1) + return fmt.Errorf("invalid host %q", switchHost) } cfg, err := auth.LoadConfig() @@ -189,13 +194,12 @@ var switchCmd = &cobra.Command{ } cfg.Host = sanitizedHost - err = auth.SaveConfig(cfg) - if err != nil { - fmt.Fprintf(os.Stderr, "Error: failed to save config: %v\n", err) - os.Exit(1) + if err := auth.SaveConfig(cfg); err != nil { + return fmt.Errorf("failed to save config: %w", err) } fmt.Printf("Switched to %s\n", sanitizedHost) + return nil }, } @@ -203,15 +207,16 @@ var configShowCmd = &cobra.Command{ Use: "config", Short: "Show current configuration", Long: "Display the current CLI configuration as JSON.", - Run: func(cmd *cobra.Command, args []string) { + RunE: func(cmd *cobra.Command, args []string) error { cfg, err := auth.LoadConfig() if err != nil { fmt.Fprintf(os.Stderr, "No config found. Run 'nullify init' to set up.\n") - return + return nil } data, _ := json.MarshalIndent(cfg, "", " ") fmt.Println(string(data)) + return nil }, } @@ -225,25 +230,22 @@ func init() { authCmd.AddCommand(configShowCmd) } -func resolveHostForAuth(ctx context.Context) string { +func resolveHostForAuth() (string, error) { if host != "" { sanitized, err := lib.SanitizeNullifyHost(host) if err != nil { - fmt.Fprintf(os.Stderr, "Error: invalid host %q\n", host) - os.Exit(1) + return "", fmt.Errorf("invalid host %q", host) } - return sanitized + return sanitized, nil } cfg, err := auth.LoadConfig() if err == nil && cfg.Host != "" { sanitized, sErr := lib.SanitizeNullifyHost(cfg.Host) if sErr == nil { - return sanitized + return sanitized, nil } } - fmt.Fprintln(os.Stderr, "Error: no host configured. Use --host or run 'nullify auth login'") - os.Exit(1) - return "" + return "", fmt.Errorf("no host configured. Use --host or run 'nullify auth login'") } diff --git a/cmd/cli/cmd/ci.go b/cmd/cli/cmd/ci.go index 84e2ab6..cff79f5 100644 --- a/cmd/cli/cmd/ci.go +++ b/cmd/cli/cmd/ci.go @@ -2,14 +2,11 @@ package cmd import ( "encoding/json" - "errors" "fmt" "os" "sync" "sync/atomic" - "github.com/nullify-platform/cli/internal/auth" - "github.com/nullify-platform/cli/internal/client" "github.com/nullify-platform/cli/internal/lib" "github.com/nullify-platform/cli/internal/logger" "github.com/spf13/cobra" @@ -41,30 +38,22 @@ Exit codes: # Check a specific repo nullify ci gate --repo my-org/my-repo`, - Run: func(cmd *cobra.Command, args []string) { + // SilenceErrors: this command writes its own gate-failure summary to + // stdout and must not have cobra echo the sentinel error to stderr. + // Auth/network errors are printed explicitly below to preserve the + // original stderr output. + SilenceErrors: true, + RunE: func(cmd *cobra.Command, args []string) error { ctx := setupLogger(cmd.Context()) defer logger.Close(ctx) - ciHost := resolveHost(ctx) - token, err := lib.GetNullifyToken(ctx, ciHost, nullifyToken, githubToken) + authCtx, err := resolveCommandAuth(ctx) if err != nil { - if errors.Is(err, lib.ErrNoToken) { - fmt.Fprintf(os.Stderr, "Error: not authenticated. Run 'nullify auth login' first.\n") - } else { - fmt.Fprintf(os.Stderr, "Error: %v\n", err) - } - os.Exit(ExitAuthError) - } - - nullifyClient := client.NewNullifyClient(ciHost, token) - - creds, err := auth.LoadCredentials() - queryParams := map[string]string{} - if err == nil { - if hostCreds, ok := creds[auth.CredentialKey(ciHost)]; ok && hostCreds.QueryParameters != nil { - queryParams = hostCreds.QueryParameters - } + fmt.Fprintf(os.Stderr, "Error: %v\n", err) + return err } + nullifyClient := authCtx.Client() + queryParams := authCtx.QueryParams severityThreshold, _ := cmd.Flags().GetString("severity-threshold") findingType, _ := cmd.Flags().GetString("type") @@ -79,8 +68,9 @@ Exit codes: } } if !validThreshold { - fmt.Fprintf(os.Stderr, "Error: invalid --severity-threshold %q. Valid values: critical, high, medium, low\n", severityThreshold) - os.Exit(1) + err := fmt.Errorf("invalid --severity-threshold %q. Valid values: critical, high, medium, low", severityThreshold) + fmt.Fprintf(os.Stderr, "Error: %v\n", err) + return withExitCode(1, err) } if repo == "" { @@ -138,16 +128,18 @@ Exit codes: _ = g.Wait() if apiErrors > 0 && apiErrors == totalRequests { - fmt.Fprintf(os.Stderr, "Error: all API requests failed, cannot determine gate status\n") - os.Exit(ExitNetworkError) + err := fmt.Errorf("all API requests failed, cannot determine gate status") + fmt.Fprintf(os.Stderr, "Error: %v\n", err) + return withExitCode(ExitNetworkError, err) } if totalFindings > 0 { fmt.Printf("\nGate failed: %d findings at or above %s severity\n", totalFindings, severityThreshold) - os.Exit(ExitFindings) + return withExitCode(ExitFindings, fmt.Errorf("gate failed: %d findings at or above %s severity", totalFindings, severityThreshold)) } fmt.Println("Gate passed: no findings above threshold") + return nil }, } @@ -157,30 +149,16 @@ var ciReportCmd = &cobra.Command{ Long: "Output a markdown summary of security findings suitable for PR comments. Shows counts by type and severity.", Example: ` nullify ci report nullify ci report --repo my-org/my-repo`, - Run: func(cmd *cobra.Command, args []string) { + RunE: func(cmd *cobra.Command, args []string) error { ctx := setupLogger(cmd.Context()) defer logger.Close(ctx) - ciHost := resolveHost(ctx) - token, err := lib.GetNullifyToken(ctx, ciHost, nullifyToken, githubToken) + authCtx, err := resolveCommandAuth(ctx) if err != nil { - if errors.Is(err, lib.ErrNoToken) { - fmt.Fprintf(os.Stderr, "Error: not authenticated. Run 'nullify auth login' first.\n") - } else { - fmt.Fprintf(os.Stderr, "Error: %v\n", err) - } - os.Exit(ExitAuthError) - } - - nullifyClient := client.NewNullifyClient(ciHost, token) - - creds, err := auth.LoadCredentials() - queryParams := map[string]string{} - if err == nil { - if hostCreds, ok := creds[auth.CredentialKey(ciHost)]; ok && hostCreds.QueryParameters != nil { - queryParams = hostCreds.QueryParameters - } + return err } + nullifyClient := authCtx.Client() + queryParams := authCtx.QueryParams repo, _ := cmd.Flags().GetString("repo") if repo == "" { @@ -235,8 +213,7 @@ var ciReportCmd = &cobra.Command{ _ = g.Wait() if successCount == 0 { - fmt.Fprintln(os.Stderr, "Error: all API requests failed, cannot generate report") - os.Exit(ExitNetworkError) + return networkError("all API requests failed, cannot generate report") } if apiErrors > 0 { fmt.Fprintf(os.Stderr, "Warning: %d API requests failed while generating the report\n", apiErrors) @@ -255,6 +232,7 @@ var ciReportCmd = &cobra.Command{ fmt.Println() fmt.Println("*Generated by [Nullify CLI](https://github.com/nullify-platform/cli)*") + return nil }, } diff --git a/cmd/cli/cmd/exitcodes.go b/cmd/cli/cmd/exitcodes.go index dc02b3d..9c0b84e 100644 --- a/cmd/cli/cmd/exitcodes.go +++ b/cmd/cli/cmd/exitcodes.go @@ -1,5 +1,10 @@ package cmd +import ( + "errors" + "fmt" +) + // Exit codes for the CLI. const ( ExitSuccess = 0 @@ -7,3 +12,65 @@ const ( ExitAuthError = 2 ExitNetworkError = 3 ) + +// exitCodeError wraps an error with a specific process exit code. RunE handlers +// return these so that exit-code mapping happens in exactly one place +// (main.go), instead of scattered os.Exit calls that would skip deferred +// cleanup such as logger.Close. +type exitCodeError struct { + code int + err error +} + +func (e *exitCodeError) Error() string { + if e.err == nil { + return "" + } + return e.err.Error() +} + +func (e *exitCodeError) Unwrap() error { + return e.err +} + +// ExitCode returns the process exit code associated with the error. +func (e *exitCodeError) ExitCode() int { + return e.code +} + +// withExitCode wraps err so that the CLI exits with the given code. The +// underlying error is preserved (and printed once by cobra). +func withExitCode(code int, err error) error { + if err == nil { + return nil + } + return &exitCodeError{code: code, err: err} +} + +// authError builds an exit-code error for authentication failures. +func authError(format string, args ...any) error { + return withExitCode(ExitAuthError, fmt.Errorf(format, args...)) +} + +// networkError builds an exit-code error for network/API failures. +func networkError(format string, args ...any) error { + return withExitCode(ExitNetworkError, fmt.Errorf(format, args...)) +} + +// findingsError builds an exit-code error indicating findings exceeded the gate. +func findingsError(format string, args ...any) error { + return withExitCode(ExitFindings, fmt.Errorf(format, args...)) +} + +// ExitCodeForError resolves the process exit code for a top-level error. +// Plain errors map to exit code 1; coded errors use their embedded code. +func ExitCodeForError(err error) int { + if err == nil { + return ExitSuccess + } + var coded *exitCodeError + if errors.As(err, &coded) { + return coded.code + } + return 1 +} diff --git a/cmd/cli/cmd/exitcodes_test.go b/cmd/cli/cmd/exitcodes_test.go new file mode 100644 index 0000000..e603289 --- /dev/null +++ b/cmd/cli/cmd/exitcodes_test.go @@ -0,0 +1,40 @@ +package cmd + +import ( + "errors" + "fmt" + "testing" + + "github.com/stretchr/testify/require" +) + +func TestExitCodeForError(t *testing.T) { + tests := []struct { + name string + err error + want int + }{ + {"nil", nil, ExitSuccess}, + {"plain error", errors.New("boom"), 1}, + {"auth error", authError("nope"), ExitAuthError}, + {"network error", networkError("nope"), ExitNetworkError}, + {"findings error", findingsError("nope"), ExitFindings}, + {"explicit code 1", withExitCode(1, errors.New("x")), 1}, + {"wrapped coded error", fmt.Errorf("ctx: %w", authError("nope")), ExitAuthError}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + require.Equal(t, tt.want, ExitCodeForError(tt.err)) + }) + } +} + +func TestWithExitCodePreservesMessage(t *testing.T) { + err := withExitCode(ExitNetworkError, errors.New("fetching metrics: timeout")) + require.Equal(t, "fetching metrics: timeout", err.Error()) + + var coded *exitCodeError + require.True(t, errors.As(err, &coded)) + require.Equal(t, ExitNetworkError, coded.ExitCode()) +} diff --git a/cmd/cli/cmd/findings.go b/cmd/cli/cmd/findings.go index 11268bc..dd27f8c 100644 --- a/cmd/cli/cmd/findings.go +++ b/cmd/cli/cmd/findings.go @@ -7,8 +7,6 @@ import ( "os" "strings" - "github.com/nullify-platform/cli/internal/auth" - "github.com/nullify-platform/cli/internal/client" "github.com/nullify-platform/cli/internal/lib" "github.com/nullify-platform/cli/internal/logger" "github.com/nullify-platform/cli/internal/output" @@ -33,26 +31,16 @@ Results are paginated automatically up to --limit total findings.`, # Fetch up to 500 findings nullify findings --limit 500`, - Run: func(cmd *cobra.Command, args []string) { + RunE: func(cmd *cobra.Command, args []string) error { ctx := setupLogger(cmd.Context()) defer logger.Close(ctx) - findingsHost := resolveHost(ctx) - token, err := lib.GetNullifyToken(ctx, findingsHost, nullifyToken, githubToken) + authCtx, err := resolveCommandAuth(ctx) if err != nil { - fmt.Fprintf(os.Stderr, "Error: not authenticated. Run 'nullify auth login' first.\n") - os.Exit(ExitAuthError) - } - - nullifyClient := client.NewNullifyClient(findingsHost, token) - - creds, err := auth.LoadCredentials() - queryParams := map[string]string{} - if err == nil { - if hostCreds, ok := creds[auth.CredentialKey(findingsHost)]; ok && hostCreds.QueryParameters != nil { - queryParams = hostCreds.QueryParameters - } + return err } + nullifyClient := authCtx.Client() + queryParams := authCtx.QueryParams severity, _ := cmd.Flags().GetString("severity") status, _ := cmd.Flags().GetString("status") @@ -136,8 +124,7 @@ Results are paginated automatically up to --limit total findings.`, reqBody := map[string]any{"query": query} bodyBytes, err := json.Marshal(reqBody) if err != nil { - fmt.Fprintf(os.Stderr, "Error: %v\n", err) - os.Exit(ExitNetworkError) + return networkError("%w", err) } if debug { @@ -147,8 +134,7 @@ Results are paginated automatically up to --limit total findings.`, respBody, err := lib.DoPostJSON(ctx, nullifyClient.HttpClient, nullifyClient.BaseURL, path, bytes.NewReader(bodyBytes)) if err != nil { - fmt.Fprintf(os.Stderr, "Error: %v\n", err) - os.Exit(ExitNetworkError) + return networkError("%w", err) } if debug { @@ -161,8 +147,7 @@ Results are paginated automatically up to --limit total findings.`, var resp unifiedResponse if err := json.Unmarshal([]byte(respBody), &resp); err != nil { - fmt.Fprintf(os.Stderr, "Error parsing response: %v\n", err) - os.Exit(ExitNetworkError) + return networkError("parsing response: %w", err) } allFindings = append(allFindings, resp.Findings...) @@ -190,6 +175,7 @@ Results are paginated automatically up to --limit total findings.`, if err := output.Print(cmd, out); err != nil { fmt.Fprintln(os.Stderr, string(out)) } + return nil }, } diff --git a/cmd/cli/cmd/fix.go b/cmd/cli/cmd/fix.go index d60ef8b..427ebb3 100644 --- a/cmd/cli/cmd/fix.go +++ b/cmd/cli/cmd/fix.go @@ -6,8 +6,6 @@ import ( "net/url" "os" - "github.com/nullify-platform/cli/internal/auth" - "github.com/nullify-platform/cli/internal/client" "github.com/nullify-platform/cli/internal/lib" "github.com/nullify-platform/cli/internal/logger" "github.com/nullify-platform/cli/internal/output" @@ -29,28 +27,18 @@ Supports SAST and SCA dependency findings.`, # Fix an SCA dependency finding nullify fix def456 --type sca`, Args: cobra.ExactArgs(1), - Run: func(cmd *cobra.Command, args []string) { + RunE: func(cmd *cobra.Command, args []string) error { ctx := setupLogger(cmd.Context()) defer logger.Close(ctx) findingID := args[0] - fixHost := resolveHost(ctx) - token, err := lib.GetNullifyToken(ctx, fixHost, nullifyToken, githubToken) + authCtx, err := resolveCommandAuth(ctx) if err != nil { - fmt.Fprintf(os.Stderr, "Error: not authenticated. Run 'nullify auth login' first.\n") - os.Exit(ExitAuthError) - } - - nullifyClient := client.NewNullifyClient(fixHost, token) - - creds, err := auth.LoadCredentials() - queryParams := map[string]string{} - if err == nil { - if hostCreds, ok := creds[auth.CredentialKey(fixHost)]; ok && hostCreds.QueryParameters != nil { - queryParams = hostCreds.QueryParameters - } + return err } + nullifyClient := authCtx.Client() + queryParams := authCtx.QueryParams findingType, _ := cmd.Flags().GetString("type") createPR, _ := cmd.Flags().GetBool("create-pr") @@ -62,8 +50,7 @@ Supports SAST and SCA dependency findings.`, case "sca": basePath = "/sca/dependencies/findings" default: - fmt.Fprintf(os.Stderr, "Error: --type must be 'sast' or 'sca'\n") - os.Exit(1) + return fmt.Errorf("--type must be 'sast' or 'sca'") } qs := lib.BuildQueryString(queryParams) @@ -75,16 +62,14 @@ Supports SAST and SCA dependency findings.`, _, err = lib.DoPost(ctx, nullifyClient.HttpClient, nullifyClient.BaseURL, fmt.Sprintf("%s/%s/autofix/fix%s", basePath, url.PathEscape(findingID), qs)) if err != nil { - fmt.Fprintf(os.Stderr, "Error generating fix: %v\n", err) - os.Exit(1) + return fmt.Errorf("generating fix: %w", err) } // Step 2: Get diff diffBody, err := lib.DoGet(ctx, nullifyClient.HttpClient, nullifyClient.BaseURL, fmt.Sprintf("%s/%s/autofix/cache/diff%s", basePath, url.PathEscape(findingID), qs)) if err != nil { - fmt.Fprintf(os.Stderr, "Error getting diff: %v\n", err) - os.Exit(1) + return fmt.Errorf("getting diff: %w", err) } result := map[string]any{ @@ -101,8 +86,7 @@ Supports SAST and SCA dependency findings.`, prBody, err := lib.DoPost(ctx, nullifyClient.HttpClient, nullifyClient.BaseURL, fmt.Sprintf("%s/%s/autofix/cache/create_pr%s", basePath, url.PathEscape(findingID), qs)) if err != nil { - fmt.Fprintf(os.Stderr, "Error creating PR: %v\n", err) - os.Exit(1) + return fmt.Errorf("creating PR: %w", err) } result["pr"] = json.RawMessage(prBody) } @@ -111,6 +95,7 @@ Supports SAST and SCA dependency findings.`, if err := output.Print(cmd, out); err != nil { fmt.Fprintln(os.Stderr, string(out)) } + return nil }, } diff --git a/cmd/cli/cmd/init.go b/cmd/cli/cmd/init.go index d908c8f..2eab8c5 100644 --- a/cmd/cli/cmd/init.go +++ b/cmd/cli/cmd/init.go @@ -2,7 +2,6 @@ package cmd import ( "fmt" - "os" "github.com/nullify-platform/cli/internal/logger" "github.com/nullify-platform/cli/internal/wizard" @@ -13,7 +12,7 @@ var initCmd = &cobra.Command{ Use: "init", Short: "Set up Nullify CLI for the first time", Long: "Interactive setup wizard that configures your Nullify domain, authentication, repository detection, and MCP integration.", - Run: func(cmd *cobra.Command, args []string) { + RunE: func(cmd *cobra.Command, args []string) error { ctx := setupLogger(cmd.Context()) defer logger.Close(ctx) @@ -31,9 +30,9 @@ var initCmd = &cobra.Command{ } if err := wizard.Run(ctx, steps); err != nil { - logger.L(ctx).Error("setup wizard failed", logger.Err(err)) - os.Exit(1) + return fmt.Errorf("setup wizard failed: %w", err) } + return nil }, } diff --git a/cmd/cli/cmd/mcp.go b/cmd/cli/cmd/mcp.go index f9cc7ac..6cbde2a 100644 --- a/cmd/cli/cmd/mcp.go +++ b/cmd/cli/cmd/mcp.go @@ -1,10 +1,7 @@ package cmd import ( - "errors" "fmt" - "os" - "strings" "github.com/nullify-platform/cli/internal/client" @@ -24,18 +21,13 @@ var mcpServeCmd = &cobra.Command{ Use: "serve", Short: "Start the MCP server", Long: "Start the Nullify MCP server over stdio. Configure your AI tool to run 'nullify mcp serve'.", - Run: func(cmd *cobra.Command, args []string) { + RunE: func(cmd *cobra.Command, args []string) error { ctx := setupLogger(cmd.Context()) defer logger.Close(ctx) authCtx, err := resolveCommandAuth(ctx) if err != nil { - if errors.Is(err, lib.ErrNoToken) { - fmt.Fprintf(os.Stderr, "Error: not authenticated. Run 'nullify auth login' first.\n") - } else { - fmt.Fprintf(os.Stderr, "Error: %v\n", err) - } - os.Exit(ExitAuthError) + return err } queryParams := authCtx.QueryParams @@ -59,8 +51,7 @@ var mcpServeCmd = &cobra.Command{ } } if !validSet { - fmt.Fprintf(os.Stderr, "Error: invalid --tools value %q. Valid values: %s\n", toolsFlag, strings.Join(validSets, ", ")) - os.Exit(1) + return fmt.Errorf("invalid --tools value %q. Valid values: %s", toolsFlag, strings.Join(validSets, ", ")) } // Create a refreshing client for long-running MCP sessions @@ -69,15 +60,13 @@ var mcpServeCmd = &cobra.Command{ } nullifyClient, clientErr := client.NewRefreshingNullifyClient(authCtx.Host, tokenProvider) if clientErr != nil { - fmt.Fprintf(os.Stderr, "Error: failed to create client: %v\n", clientErr) - os.Exit(1) + return fmt.Errorf("failed to create client: %w", clientErr) } - err = mcp.ServeWithClient(ctx, nullifyClient, queryParams, toolSet) - if err != nil { - logger.L(ctx).Error("MCP server error", logger.Err(err)) - os.Exit(1) + if err := mcp.ServeWithClient(ctx, nullifyClient, queryParams, toolSet); err != nil { + return fmt.Errorf("MCP server error: %w", err) } + return nil }, } diff --git a/cmd/cli/cmd/open.go b/cmd/cli/cmd/open.go index e3db4ae..07f3f02 100644 --- a/cmd/cli/cmd/open.go +++ b/cmd/cli/cmd/open.go @@ -13,11 +13,14 @@ import ( var openCmd = &cobra.Command{ Use: "open", Short: "Open the Nullify dashboard in your browser", - Run: func(cmd *cobra.Command, args []string) { + RunE: func(cmd *cobra.Command, args []string) error { ctx := setupLogger(cmd.Context()) defer logger.Close(ctx) - openHost := resolveHost(ctx) + openHost, err := resolveHostE(ctx) + if err != nil { + return err + } // Strip "api." prefix to get the dashboard URL dashboardHost := strings.TrimPrefix(openHost, "api.") url := "https://" + dashboardHost @@ -27,9 +30,10 @@ var openCmd = &cobra.Command{ } if err := auth.OpenBrowser(url); err != nil { - fmt.Fprintf(os.Stderr, "Error: could not open browser: %v\nVisit %s manually.\n", err, url) - os.Exit(1) + fmt.Fprintf(os.Stderr, "Visit %s manually.\n", url) + return fmt.Errorf("could not open browser: %w", err) } + return nil }, } diff --git a/cmd/cli/cmd/pentest.go b/cmd/cli/cmd/pentest.go index cf30865..3ce2d97 100644 --- a/cmd/cli/cmd/pentest.go +++ b/cmd/cli/cmd/pentest.go @@ -1,7 +1,7 @@ package cmd import ( - "os" + "fmt" "github.com/nullify-platform/cli/internal/logger" "github.com/nullify-platform/cli/internal/pentest" @@ -12,21 +12,20 @@ var pentestCmd = &cobra.Command{ Use: "pentest", Short: "Run a pentest scan against your API endpoints", Long: "Run pentest scans against your API endpoints. Supports local Docker-based scanning and cloud-based scanning.", - Run: func(cmd *cobra.Command, args []string) { + RunE: func(cmd *cobra.Command, args []string) error { ctx := setupLogger(cmd.Context()) defer logger.Close(ctx) pentestArgs := getPentestArgs(cmd) - nullifyClient := getNullifyClient(ctx) - - err := pentest.RunPentestScan(ctx, pentestArgs, nullifyClient, getLogLevel()) + nullifyClient, err := getNullifyClientE(ctx) if err != nil { - logger.L(ctx).Error( - "failed to run pentest scan", - logger.Err(err), - ) - os.Exit(1) + return err + } + + if err := pentest.RunPentestScan(ctx, pentestArgs, nullifyClient, getLogLevel()); err != nil { + return fmt.Errorf("failed to run pentest scan: %w", err) } + return nil }, } diff --git a/cmd/cli/cmd/repos.go b/cmd/cli/cmd/repos.go index 551ee08..b9fa9b1 100644 --- a/cmd/cli/cmd/repos.go +++ b/cmd/cli/cmd/repos.go @@ -14,7 +14,7 @@ var reposCmd = &cobra.Command{ Use: "repos", Short: "List monitored repositories", Example: " nullify repos\n nullify repos -o table", - Run: func(cmd *cobra.Command, args []string) { + RunE: func(cmd *cobra.Command, args []string) error { ctx := setupLogger(cmd.Context()) defer logger.Close(ctx) @@ -22,13 +22,13 @@ var reposCmd = &cobra.Command{ result, err := apiClient.ListContextRepositories(ctx, url.Values{}) if err != nil { - fmt.Fprintf(os.Stderr, "Error: %v\n", err) - os.Exit(1) + return err } if err := output.Print(cmd, result); err != nil { fmt.Fprintln(os.Stderr, string(result)) } + return nil }, } diff --git a/cmd/cli/cmd/root.go b/cmd/cli/cmd/root.go index 3691769..e8f2578 100644 --- a/cmd/cli/cmd/root.go +++ b/cmd/cli/cmd/root.go @@ -2,6 +2,7 @@ package cmd import ( "context" + "fmt" "net/http" "os" "time" @@ -47,6 +48,10 @@ var rootCmd = &cobra.Command{ } func Execute() error { + // Silence cobra's automatic usage dump on errors (it's noisy for runtime + // failures), but let cobra print the error message itself exactly once. + rootCmd.SilenceUsage = true + rootCmd.SilenceErrors = false return rootCmd.Execute() } @@ -137,25 +142,25 @@ func getLogLevel() string { return "warn" } -func resolveHost(ctx context.Context) string { +// resolveHostE resolves the Nullify host from flag, env var, or config file. +// It returns a coded error (exit code 1 for a malformed host, ExitAuthError +// when no host is configured) instead of calling os.Exit, so callers in RunE +// handlers can return it up the stack and let deferred cleanup run. +func resolveHostE(ctx context.Context) (string, error) { // 1. Flag takes priority if host != "" { sanitized, err := lib.SanitizeNullifyHost(host) if err != nil { - logger.L(ctx).Error( - "invalid host, must be in the format .nullify.ai", - logger.String("host", host), - ) - os.Exit(1) + return "", withExitCode(1, fmt.Errorf("invalid host %q, must be in the format .nullify.ai", host)) } - return sanitized + return sanitized, nil } // 2. Env var (takes precedence over config file) if envHost := os.Getenv("NULLIFY_HOST"); envHost != "" { sanitized, err := lib.SanitizeNullifyHost(envHost) if err == nil { - return sanitized + return sanitized, nil } logger.L(ctx).Warn("NULLIFY_HOST env var is invalid, falling through to config", logger.String("host", envHost), logger.Err(err)) } @@ -165,27 +170,37 @@ func resolveHost(ctx context.Context) string { if err == nil && cfg.Host != "" { sanitized, err := lib.SanitizeNullifyHost(cfg.Host) if err == nil { - return sanitized + return sanitized, nil } logger.L(ctx).Warn("config file host is invalid, ignoring", logger.String("host", cfg.Host), logger.Err(err)) } - logger.L(ctx).Error("no host configured. Run 'nullify init' to set up, or 'nullify auth login --host .nullify.ai' to configure.") - os.Exit(ExitAuthError) - return "" + return "", authError("no host configured. Run 'nullify init' to set up, or 'nullify auth login --host .nullify.ai' to configure.") +} + +// resolveHost is the os.Exit-on-error variant retained for the generated API +// commands' getAPIClient factory, which cannot easily thread errors back up. +func resolveHost(ctx context.Context) string { + nullifyHost, err := resolveHostE(ctx) + if err != nil { + logger.L(ctx).Error("failed to resolve host", logger.Err(err)) + os.Exit(ExitCodeForError(err)) + } + return nullifyHost } -func getNullifyClient(ctx context.Context) *client.NullifyClient { - nullifyHost := resolveHost(ctx) +// getNullifyClientE builds a NullifyClient, returning a coded error on auth +// failure instead of calling os.Exit. +func getNullifyClientE(ctx context.Context) (*client.NullifyClient, error) { + nullifyHost, err := resolveHostE(ctx) + if err != nil { + return nil, err + } token, err := lib.GetNullifyToken(ctx, nullifyHost, nullifyToken, githubToken) if err != nil { - logger.L(ctx).Error( - "failed to get token. Run 'nullify auth login' to authenticate.", - logger.Err(err), - ) - os.Exit(ExitAuthError) + return nil, authError("failed to get token. Run 'nullify auth login' to authenticate: %w", err) } - return client.NewNullifyClient(nullifyHost, token) + return client.NewNullifyClient(nullifyHost, token), nil } diff --git a/cmd/cli/cmd/runtime_auth.go b/cmd/cli/cmd/runtime_auth.go index 7725530..e663223 100644 --- a/cmd/cli/cmd/runtime_auth.go +++ b/cmd/cli/cmd/runtime_auth.go @@ -2,24 +2,48 @@ package cmd import ( "context" + "errors" "os" "github.com/nullify-platform/cli/internal/auth" + "github.com/nullify-platform/cli/internal/client" "github.com/nullify-platform/cli/internal/lib" ) +// stdinIsTTY reports whether stdin is connected to an interactive terminal. +// Commands that prompt for input use this to fail fast in non-interactive +// environments (CI, pipes) instead of blocking forever on a read. +func stdinIsTTY() bool { + info, err := os.Stdin.Stat() + if err != nil { + return false + } + return info.Mode()&os.ModeCharDevice != 0 +} + type commandAuthContext struct { Host string Token string QueryParams map[string]string } +// Client builds a NullifyClient for the resolved host and token. +func (c *commandAuthContext) Client() *client.NullifyClient { + return client.NewNullifyClient(c.Host, c.Token) +} + func resolveCommandAuth(ctx context.Context) (*commandAuthContext, error) { - commandHost := resolveHost(ctx) + commandHost, err := resolveHostE(ctx) + if err != nil { + return nil, err + } token, err := lib.GetNullifyToken(ctx, commandHost, nullifyToken, githubToken) if err != nil { - return nil, err + if errors.Is(err, lib.ErrNoToken) { + return nil, authError("not authenticated. Run 'nullify auth login' first") + } + return nil, authError("%w", err) } queryParams := map[string]string{} diff --git a/cmd/cli/cmd/status.go b/cmd/cli/cmd/status.go index 670253f..743ac67 100644 --- a/cmd/cli/cmd/status.go +++ b/cmd/cli/cmd/status.go @@ -2,15 +2,12 @@ package cmd import ( "encoding/json" - "errors" "fmt" "os" "sort" "strings" "text/tabwriter" - "github.com/nullify-platform/cli/internal/auth" - "github.com/nullify-platform/cli/internal/client" "github.com/nullify-platform/cli/internal/lib" "github.com/nullify-platform/cli/internal/logger" "github.com/nullify-platform/cli/internal/output" @@ -24,38 +21,23 @@ var securityStatusCmd = &cobra.Command{ Long: "Display a summary of your security posture across all scanner types. Quick morning check-in command.", Example: ` nullify status nullify status -o table`, - Run: func(cmd *cobra.Command, args []string) { + RunE: func(cmd *cobra.Command, args []string) error { ctx := setupLogger(cmd.Context()) defer logger.Close(ctx) - statusHost := resolveHost(ctx) - token, err := lib.GetNullifyToken(ctx, statusHost, nullifyToken, githubToken) + authCtx, err := resolveCommandAuth(ctx) if err != nil { - if errors.Is(err, lib.ErrNoToken) { - fmt.Fprintf(os.Stderr, "Error: not authenticated. Run 'nullify auth login' first.\n") - } else { - fmt.Fprintf(os.Stderr, "Error: %v\n", err) - } - os.Exit(ExitAuthError) - } - - nullifyClient := client.NewNullifyClient(statusHost, token) - - creds, err := auth.LoadCredentials() - queryParams := map[string]string{} - if err == nil { - if hostCreds, ok := creds[auth.CredentialKey(statusHost)]; ok && hostCreds.QueryParameters != nil { - queryParams = hostCreds.QueryParameters - } + return err } + nullifyClient := authCtx.Client() + queryParams := authCtx.QueryParams // Fetch metrics overview qs := lib.BuildQueryString(queryParams) overviewBody, err := lib.DoPostJSON(ctx, nullifyClient.HttpClient, nullifyClient.BaseURL, "/admin/metrics/overview"+qs, strings.NewReader(`{"query":{}}`)) if err != nil { - fmt.Fprintf(os.Stderr, "Error fetching metrics: %v\n", err) - os.Exit(ExitNetworkError) + return networkError("fetching metrics: %w", err) } var overview any @@ -106,20 +88,19 @@ var securityStatusCmd = &cobra.Command{ if format == "table" || !outputExplicit { if err := printStatusTable(statusOutput); err != nil { - fmt.Fprintf(os.Stderr, "Error: %v\n", err) - os.Exit(1) + return err } - return + return nil } out, err := json.Marshal(statusOutput) if err != nil { - fmt.Fprintf(os.Stderr, "Error: failed to encode status output: %v\n", err) - os.Exit(1) + return fmt.Errorf("failed to encode status output: %w", err) } if err := output.Print(cmd, out); err != nil { fmt.Fprintln(os.Stderr, string(out)) } + return nil }, } diff --git a/cmd/cli/cmd/update.go b/cmd/cli/cmd/update.go index e3e16ef..1693509 100644 --- a/cmd/cli/cmd/update.go +++ b/cmd/cli/cmd/update.go @@ -2,7 +2,6 @@ package cmd import ( "fmt" - "os" "runtime" "strings" @@ -14,14 +13,13 @@ import ( var updateCmd = &cobra.Command{ Use: "update", Short: "Check for CLI updates", - Run: func(cmd *cobra.Command, args []string) { + RunE: func(cmd *cobra.Command, args []string) error { ctx := cmd.Context() ghClient := github.NewClient(nil) release, _, err := ghClient.Repositories.GetLatestRelease(ctx, "Nullify-Platform", "cli") if err != nil { - fmt.Fprintf(os.Stderr, "Error checking for updates: %v\n", err) - os.Exit(1) + return fmt.Errorf("checking for updates: %w", err) } latestVersion := strings.TrimPrefix(release.GetTagName(), "v") @@ -29,7 +27,7 @@ var updateCmd = &cobra.Command{ if latestVersion == currentVersion || currentVersion == "dev" { fmt.Printf("You are running the latest version (%s)\n", logger.Version) - return + return nil } fmt.Printf("A new version is available: %s (current: %s)\n\n", latestVersion, logger.Version) @@ -46,6 +44,7 @@ var updateCmd = &cobra.Command{ fmt.Println(" # Direct download") fmt.Printf(" curl -sSL https://github.com/Nullify-Platform/cli/releases/download/v%s/nullify_%s_%s.tar.gz | tar xz\n", latestVersion, runtime.GOOS, runtime.GOARCH) + return nil }, } diff --git a/cmd/cli/cmd/whoami.go b/cmd/cli/cmd/whoami.go index cbde7c9..180892a 100644 --- a/cmd/cli/cmd/whoami.go +++ b/cmd/cli/cmd/whoami.go @@ -3,7 +3,6 @@ package cmd import ( "encoding/json" "fmt" - "os" "github.com/nullify-platform/cli/internal/auth" "github.com/nullify-platform/cli/internal/logger" @@ -14,11 +13,14 @@ import ( var whoamiCmd = &cobra.Command{ Use: "whoami", Short: "Show current authentication status", - Run: func(cmd *cobra.Command, args []string) { + RunE: func(cmd *cobra.Command, args []string) error { ctx := setupLogger(cmd.Context()) defer logger.Close(ctx) - whoamiHost := resolveHost(ctx) + whoamiHost, err := resolveHostE(ctx) + if err != nil { + return err + } info := map[string]any{ "host": whoamiHost, @@ -44,9 +46,9 @@ var whoamiCmd = &cobra.Command{ out, _ := json.MarshalIndent(info, "", " ") if err := output.Print(cmd, out); err != nil { - fmt.Fprintf(os.Stderr, "Error: %v\n", err) - os.Exit(1) + return err } + return nil }, } diff --git a/cmd/cli/main.go b/cmd/cli/main.go index b9876ca..a4fd15f 100644 --- a/cmd/cli/main.go +++ b/cmd/cli/main.go @@ -8,6 +8,8 @@ import ( func main() { if err := cmd.Execute(); err != nil { - os.Exit(1) + // cobra has already printed the error (SilenceErrors is false). + // Map it to the appropriate process exit code in one place. + os.Exit(cmd.ExitCodeForError(err)) } } diff --git a/internal/output/spinner.go b/internal/output/spinner.go index fc204d3..13e5c8b 100644 --- a/internal/output/spinner.go +++ b/internal/output/spinner.go @@ -14,14 +14,26 @@ type Spinner struct { once sync.Once } +// stderrIsTTY reports whether stderr is connected to a terminal. When it is +// not (e.g. CI logs, redirected output), the spinner stays silent to avoid +// polluting logs with ANSI escape sequences and braille frames. +func stderrIsTTY() bool { + info, err := os.Stderr.Stat() + if err != nil { + return false + } + return info.Mode()&os.ModeCharDevice != 0 +} + // NewSpinner starts a spinner with the given message. Call Stop() when done. // If quiet is true, no spinner is displayed but Stop() is still safe to call. +// The spinner is also suppressed when stderr is not a terminal. func NewSpinner(msg string, quiet bool) *Spinner { s := &Spinner{ msg: msg, done: make(chan struct{}), } - if quiet { + if quiet || !stderrIsTTY() { return s } diff --git a/internal/wizard/steps.go b/internal/wizard/steps.go index 78a0069..d903386 100644 --- a/internal/wizard/steps.go +++ b/internal/wizard/steps.go @@ -14,6 +14,17 @@ import ( "github.com/nullify-platform/cli/internal/logger" ) +// stdinIsTTY reports whether stdin is connected to an interactive terminal. +// Steps that prompt for input use this to fail fast in non-interactive +// environments (CI, pipes) instead of blocking forever on a read. +func stdinIsTTY() bool { + info, err := os.Stdin.Stat() + if err != nil { + return false + } + return info.Mode()&os.ModeCharDevice != 0 +} + // DomainStep checks if a valid host is configured and prompts the user if not. func DomainStep() Step { return Step{ @@ -23,6 +34,9 @@ func DomainStep() Step { return err == nil && cfg.Host != "" }, Execute: func(ctx context.Context) error { + if !stdinIsTTY() { + return fmt.Errorf("not a terminal; configure a host non-interactively with 'nullify auth login --host .nullify.ai'") + } reader := bufio.NewReader(os.Stdin) fmt.Print(" Enter your Nullify customer name (e.g., 'acme'): ") input, err := reader.ReadString('\n')