Skip to content

Commit 5532faa

Browse files
fix: comprehensive code review fixes across security, correctness, and robustness (#131)
* feat: comprehensive CLI improvements across security, MCP, UX, HTTP, and distribution Phase 1 (Security): Redact token from debug logs, add HTTP timeouts, add 10MB response body limits, fix remediate_finding silent error bug. Phase 2 (MCP): Add unified abstract tools (search/get/triage/ticket/events/fix), ToolSet flag (default/all/minimal/findings/admin), 5 new prompts, 2 new resources. Phase 3 (UX): Move generated commands under `nullify api`, add new commands (scan, fix, open, repos, whoami, version), concurrent API calls via errgroup, output consistency with --output flag, --quiet/--no-color flags, help examples. Phase 4 (HTTP): User-Agent header, retry with exponential backoff, fix env var precedence, refreshing auth transport for MCP sessions, shared apierror package, distinct exit codes. Phase 5 (Distribution): Homebrew tap config, `nullify update` command, SARIF output format, enhanced `nullify version` with build metadata. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix: address critical and high issues from code review - Buffer request body in retry transport to prevent empty bodies on retry - Clone request in RoundTrip to avoid mutating the original (both transports) - Fix generator template missing api. prefix logic for host - Strip api. prefix in open.go to navigate to dashboard, not API - Remove unnecessary mutex in findings.go (goroutines write to distinct indices) - Use DoPost instead of DoGet for mutation operations in fix.go - Fix Content-Type check in apierror to use HasPrefix for charset variants - Sort finding type names for deterministic MCP tool enum ordering - Parallelize ci report API calls with errgroup - Use structured exit codes (ExitAuthError) instead of bare os.Exit(1) - Remove misleading _ = token in mcp.go, validate auth inline Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix: address code review findings across security, correctness, and robustness Critical fixes: - Fix broken refresh token flow: send token as HTTP cookie instead of query param to match backend expectation (security-droid reads from cookie) - Fix GITHUB_ACTION_REPOSITORY → GITHUB_REPOSITORY (non-standard env var) - Add url.PathEscape for finding IDs in all URL path interpolations (MCP, CLI) - Replace bare http.Get with http.NewRequestWithContext for proper timeout/context - URL-encode GitHub token and owner in token exchange request Security & correctness: - Fix retry transport to respect context cancellation (select on ctx.Done) - Fix file handle leak in openapi.go (missing defer Close) - Fix openapi.go nil,nil return to return descriptive error - SanitizeNullifyHost now delegates to ParseCustomerDomain, accepting 'acme', 'acme.nullify.ai', and 'api.acme.nullify.ai' formats - Strip path/query from host input, validate hostname characters - Add severity-threshold validation in ci gate command - Mark pentest --spec-path as required flag Exit codes & UX: - Use ExitAuthError(2) for auth failures, ExitNetworkError(3) for API errors, ExitFindings(1) for gate failures across all commands - Fix ci report to use limit=1000 for accurate counts (was limit=1) - Fix "1 findings" → "1 finding" singular form in status output - Fix auth token command to print trailing newline - Add periodic "still waiting" message during login authentication - Improve auth config error message Architecture & deduplication: - Deduplicate buildQueryString in MCP package to use lib.BuildQueryString - Add ClientOption/WithHTTPClient to generated API client for retry support - Export NewRetryTransport and wire retry into generated API client - Remove unused --auth-config global flag - Parallelize status command scanner queries with errgroup - Add missing scanner types (pentest, bughunt, cspm) to MCP composite tools - Use promptResult helper consistently in MCP prompts - Fix wizard to use absolute paths via os.Getwd() - Handle output.Print errors with stderr fallback - Clear stale Token field on refreshing transport client - Log warning when NULLIFY_HOST env var is invalid Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix: run go mod tidy to promote golang.org/x/sync to direct dependency The status command now directly imports errgroup from golang.org/x/sync. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * style: fix gofmt formatting and remove unnecessary leading newline Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * style: fix gofmt formatting in login.go and spinner.go Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix: resolve timeout, host sanitization, and credential key mismatch bugs - Move time.After(10min) outside for/select loop so it fires correctly - Sanitize config file host through SanitizeNullifyHost in resolveHost - Normalize credential keys to bare form (strip api. prefix) at save/lookup - Fix SanitizeNullifyHost to return bare host form consistently Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * wip * fix: address PR #131 review comments - Use cmd.Context() instead of context.Background() in update command - Accept context.Context param in setupLogger() so cobra commands pass their own context instead of always creating a new background context - Delete unused scan command that only printed help text - Add credential key migration in LoadCredentials() to normalize old "api." prefixed keys to bare form, preventing auth failures after host sanitization changes Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix: handle errcheck lint error and normalize credential key lookups - Check io.Copy return value in local_scan.go to fix golangci-lint errcheck - Use auth.CredentialKey() for credential map lookups in all commands (chat, mcp, status, whoami, fix, ci, findings) to match the normalized key format used when saving credentials Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
1 parent 16b3df0 commit 5532faa

54 files changed

Lines changed: 2183 additions & 377 deletions

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

.goreleaser.yaml

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,9 @@ builds:
1313
- CGO_ENABLED=0
1414
ldflags:
1515
- -X 'github.com/nullify-platform/logger/pkg/logger.Version={{ .Version }}'
16+
- -X 'github.com/nullify-platform/cli/internal/api.Version={{ .Version }}'
17+
- -X 'github.com/nullify-platform/cli/cmd/cli/cmd.buildCommit={{ .Commit }}'
18+
- -X 'github.com/nullify-platform/cli/cmd/cli/cmd.buildDate={{ .Date }}'
1619
goos:
1720
- linux
1821
- darwin
@@ -40,6 +43,16 @@ checksum:
4043
changelog:
4144
use: github-native
4245

46+
brews:
47+
- name: nullify
48+
repository:
49+
owner: Nullify-Platform
50+
name: homebrew-tap
51+
homepage: "https://github.com/Nullify-Platform/cli"
52+
description: "Nullify CLI - autonomous AI workforce for product security"
53+
install: |
54+
bin.install "nullify"
55+
4356
release:
4457
github:
4558
owner: Nullify-Platform

cmd/cli/cmd/api.go

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
package cmd
2+
3+
import "github.com/spf13/cobra"
4+
5+
var apiCmd = &cobra.Command{
6+
Use: "api",
7+
Short: "Direct API access (advanced)",
8+
Long: "Low-level commands for direct Nullify API access. These map 1:1 to API endpoints and are intended for advanced users and scripting.",
9+
}
10+
11+
func init() {
12+
rootCmd.AddCommand(apiCmd)
13+
}

cmd/cli/cmd/auth.go

Lines changed: 11 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ var loginCmd = &cobra.Command{
2626
Short: "Log in to Nullify",
2727
Long: "Authenticate with your Nullify instance. Opens your browser to log in with your identity provider.",
2828
Run: func(cmd *cobra.Command, args []string) {
29-
ctx := setupLogger()
29+
ctx := setupLogger(cmd.Context())
3030
defer logger.L(ctx).Sync()
3131

3232
// Wrap context with signal handling so Ctrl+C triggers graceful cancellation
@@ -68,7 +68,7 @@ var logoutCmd = &cobra.Command{
6868
Short: "Log out of Nullify",
6969
Long: "Clear stored credentials for the current or specified host.",
7070
Run: func(cmd *cobra.Command, args []string) {
71-
ctx := setupLogger()
71+
ctx := setupLogger(cmd.Context())
7272
defer logger.L(ctx).Sync()
7373

7474
logoutHost := resolveHostForAuth(ctx)
@@ -107,7 +107,7 @@ var statusCmd = &cobra.Command{
107107
return
108108
}
109109

110-
hostCreds, ok := creds[cfg.Host]
110+
hostCreds, ok := creds[auth.CredentialKey(cfg.Host)]
111111
if !ok {
112112
fmt.Println("Status: not authenticated")
113113
return
@@ -131,7 +131,7 @@ var tokenCmd = &cobra.Command{
131131
Short: "Print access token to stdout",
132132
Long: "Print the current access token. Useful for piping to other tools.",
133133
Run: func(cmd *cobra.Command, args []string) {
134-
ctx := setupLogger()
134+
ctx := setupLogger(cmd.Context())
135135
defer logger.L(ctx).Sync()
136136

137137
hostForToken := resolveHostForAuth(ctx)
@@ -142,7 +142,7 @@ var tokenCmd = &cobra.Command{
142142
os.Exit(1)
143143
}
144144

145-
fmt.Print(token)
145+
fmt.Println(token)
146146
},
147147
}
148148

@@ -168,7 +168,7 @@ var switchCmd = &cobra.Command{
168168
fmt.Println("Configured hosts:")
169169
for h := range creds {
170170
marker := " "
171-
if cfg != nil && cfg.Host == h {
171+
if cfg != nil && auth.CredentialKey(cfg.Host) == h {
172172
marker = "* "
173173
}
174174
fmt.Printf("%s%s\n", marker, h)
@@ -206,7 +206,7 @@ var configShowCmd = &cobra.Command{
206206
Run: func(cmd *cobra.Command, args []string) {
207207
cfg, err := auth.LoadConfig()
208208
if err != nil {
209-
fmt.Println("{}")
209+
fmt.Fprintf(os.Stderr, "No config found. Run 'nullify init' to set up.\n")
210210
return
211211
}
212212

@@ -237,7 +237,10 @@ func resolveHostForAuth(ctx context.Context) string {
237237

238238
cfg, err := auth.LoadConfig()
239239
if err == nil && cfg.Host != "" {
240-
return cfg.Host
240+
sanitized, sErr := lib.SanitizeNullifyHost(cfg.Host)
241+
if sErr == nil {
242+
return sanitized
243+
}
241244
}
242245

243246
fmt.Fprintln(os.Stderr, "Error: no host configured. Use --host or run 'nullify auth login'")

cmd/cli/cmd/chat.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -24,15 +24,15 @@ Examples:
2424
nullify chat "what are my critical findings?" # Single-shot mode
2525
nullify chat --chat-id abc123 "follow up" # Resume conversation`,
2626
Run: func(cmd *cobra.Command, args []string) {
27-
ctx := setupLogger()
27+
ctx := setupLogger(cmd.Context())
2828
defer logger.L(ctx).Sync()
2929

3030
chatHost := resolveHost(ctx)
3131

3232
token, err := auth.GetValidToken(ctx, chatHost)
3333
if err != nil {
3434
fmt.Fprintf(os.Stderr, "Error: not authenticated. Run 'nullify auth login' first.\n")
35-
os.Exit(1)
35+
os.Exit(ExitAuthError)
3636
}
3737

3838
creds, err := auth.LoadCredentials()
@@ -41,7 +41,7 @@ Examples:
4141
os.Exit(1)
4242
}
4343

44-
hostCreds := creds[chatHost]
44+
hostCreds := creds[auth.CredentialKey(chatHost)]
4545
queryParams := hostCreds.QueryParameters
4646
if queryParams == nil {
4747
queryParams = make(map[string]string)

0 commit comments

Comments
 (0)