diff --git a/internal/cli/postreview.go b/internal/cli/postreview.go index 6ef89a7ae..819760710 100644 --- a/internal/cli/postreview.go +++ b/internal/cli/postreview.go @@ -13,6 +13,7 @@ import ( "github.com/fullsend-ai/fullsend/internal/forge" gh "github.com/fullsend-ai/fullsend/internal/forge/github" + "github.com/fullsend-ai/fullsend/internal/security" "github.com/fullsend-ai/fullsend/internal/sticky" "github.com/fullsend-ai/fullsend/internal/ui" ) @@ -87,6 +88,12 @@ has moved, a stale-head failure is posted instead.`, return fmt.Errorf("parsing review result: %w", err) } + // Sanitize review content through the output security + // pipeline before posting to the forge. This redacts + // leaked secrets and normalizes zero-width unicode + // obfuscation that could bypass pattern-based redaction. + parsed = sanitizeReviewResult(parsed, printer) + // CLI flag takes precedence over JSON field. if headSHA != "" { parsed.HeadSHA = headSHA @@ -527,6 +534,58 @@ func minimizeStaleReviews(ctx context.Context, client forge.Client, user string, printer.StepDone("Stale reviews minimized") } +// sanitizeReviewResult runs the security output pipeline over all +// user-visible text fields in a ReviewResult. This catches leaked +// secrets and zero-width–obfuscated tokens before they reach the +// forge API. +func sanitizeReviewResult(r ReviewResult, printer *ui.Printer) ReviewResult { + pipeline := security.OutputPipeline() + + // Sanitize the main body. + if r.Body != "" { + result := pipeline.Scan(r.Body) + if result.Sanitized != "" { + r.Body = result.Sanitized + printer.StepWarn(fmt.Sprintf("Sanitized review body (%d finding(s))", len(result.Findings))) + for _, f := range result.Findings { + printer.StepWarn(fmt.Sprintf(" %s: %s", f.Name, f.Detail)) + } + } + } + + // Sanitize finding fields — severity, category, description, and + // remediation are all interpolated into Markdown posted to the + // forge and could carry secrets from agent output. + for i := range r.Findings { + if r.Findings[i].Severity != "" { + result := pipeline.Scan(r.Findings[i].Severity) + if result.Sanitized != "" { + r.Findings[i].Severity = result.Sanitized + } + } + if r.Findings[i].Category != "" { + result := pipeline.Scan(r.Findings[i].Category) + if result.Sanitized != "" { + r.Findings[i].Category = result.Sanitized + } + } + if r.Findings[i].Description != "" { + result := pipeline.Scan(r.Findings[i].Description) + if result.Sanitized != "" { + r.Findings[i].Description = result.Sanitized + } + } + if r.Findings[i].Remediation != "" { + result := pipeline.Scan(r.Findings[i].Remediation) + if result.Sanitized != "" { + r.Findings[i].Remediation = result.Sanitized + } + } + } + + return r +} + // parseReviewResult attempts to parse the body as a JSON ReviewResult. // If parsing fails, treats the entire input as a plain-text body. // Returns an error if the JSON is valid but the body field is empty diff --git a/internal/cli/postreview_test.go b/internal/cli/postreview_test.go index 5be6ac4be..3e571dec3 100644 --- a/internal/cli/postreview_test.go +++ b/internal/cli/postreview_test.go @@ -1044,6 +1044,117 @@ func TestFormatFindingComment(t *testing.T) { }) } +func TestSanitizeReviewResult_RedactsSecretsInBody(t *testing.T) { + printer := ui.New(io.Discard) + secret := "ghp_FAKEtesttoken000000000000000000000000" + r := ReviewResult{ + Body: "Found this token: " + secret + " in the code.", + Action: "comment", + } + + sanitized := sanitizeReviewResult(r, printer) + assert.NotContains(t, sanitized.Body, "ghp_FAKEtest", "secret should be redacted from body") + assert.Contains(t, sanitized.Body, "Found this token:", "non-secret text should remain") +} + +func TestSanitizeReviewResult_RedactsSecretsInFindings(t *testing.T) { + printer := ui.New(io.Discard) + secret := "ghp_FAKEtesttoken000000000000000000000000" + r := ReviewResult{ + Body: "Review body without secrets.", + Action: "request-changes", + Findings: []ReviewFinding{ + { + Severity: "high", + Category: "security", + File: "main.go", + Line: 10, + Description: "Hardcoded token: " + secret, + Remediation: "Remove " + secret + " and use env var.", + }, + }, + } + + sanitized := sanitizeReviewResult(r, printer) + assert.NotContains(t, sanitized.Findings[0].Description, "ghp_FAKEtest", "secret should be redacted from finding description") + assert.NotContains(t, sanitized.Findings[0].Remediation, "ghp_FAKEtest", "secret should be redacted from finding remediation") + assert.Contains(t, sanitized.Findings[0].Description, "Hardcoded token:", "non-secret text should remain") +} + +func TestSanitizeReviewResult_RedactsSecretsInSeverityAndCategory(t *testing.T) { + printer := ui.New(io.Discard) + secret := "ghp_FAKEtesttoken000000000000000000000000" + r := ReviewResult{ + Body: "Review body without secrets.", + Action: "request-changes", + Findings: []ReviewFinding{ + { + Severity: "high " + secret, + Category: "security " + secret, + File: "main.go", + Line: 10, + Description: "Clean description.", + }, + }, + } + + sanitized := sanitizeReviewResult(r, printer) + assert.NotContains(t, sanitized.Findings[0].Severity, "ghp_FAKEtest", "secret should be redacted from finding severity") + assert.NotContains(t, sanitized.Findings[0].Category, "ghp_FAKEtest", "secret should be redacted from finding category") +} + +func TestSanitizeReviewResult_ZeroWidthObfuscatedSecret(t *testing.T) { + printer := ui.New(io.Discard) + plain := "ghp_FAKEtesttoken000000000000000000000000" + // Interleave zero-width non-joiner characters to obfuscate the token. + var obfuscated string + for _, c := range plain { + obfuscated += string(c) + "\u200c" + } + r := ReviewResult{ + Body: "Token: " + obfuscated, + Action: "comment", + } + + sanitized := sanitizeReviewResult(r, printer) + assert.NotContains(t, sanitized.Body, "ghp_FAKEtest", "zero-width obfuscated secret should be caught after normalization") +} + +func TestSanitizeReviewResult_NoSecretsPassesThrough(t *testing.T) { + printer := ui.New(io.Discard) + r := ReviewResult{ + Body: "Looks good! No issues found.", + Action: "approve", + Findings: []ReviewFinding{ + { + Severity: "low", + Category: "style", + File: "main.go", + Line: 5, + Description: "Consider renaming variable.", + }, + }, + } + + sanitized := sanitizeReviewResult(r, printer) + assert.Equal(t, "Looks good! No issues found.", sanitized.Body, "clean body should pass through unchanged") + assert.Equal(t, "Consider renaming variable.", sanitized.Findings[0].Description, "clean finding should pass through unchanged") + assert.Equal(t, "low", sanitized.Findings[0].Severity, "clean severity should pass through unchanged") + assert.Equal(t, "style", sanitized.Findings[0].Category, "clean category should pass through unchanged") +} + +func TestSanitizeReviewResult_EmptyBody(t *testing.T) { + printer := ui.New(io.Discard) + r := ReviewResult{ + Body: "", + Action: "failure", + Reason: "tool-failure", + } + + sanitized := sanitizeReviewResult(r, printer) + assert.Empty(t, sanitized.Body, "empty body should remain empty") +} + func TestPostApprovedFollowUpIssues_DisabledIsNoop(t *testing.T) { // Issue creation is disabled (#1137). Verify the function is a no-op for // approve actions with actionable findings.