Address review: sanitize what is printed, and drop CR from one-shot text

main

Commit

Soph2w ago

Two findings from Bugbot, both confirmed against the code.

Sanitizing RefRejectedError.Reason did not change what users see. Error() returns the wrapped error's message when one is present, which in production it always is, so the CLI printed the server's raw status: verified that Reason came out clean while Error() still carried "\x1b[2K\r". Error() now filters its rendered output, and the non-per-ref path (an unpack failure, say) is wrapped in a sanitizedError that filters display while forwarding Unwrap, so errors.Is and errors.As keep matching exactly what they matched before.

sanitize.Text kept carriage return for git's progress output, but Text is used on strings that are not progress and not line-prefixed: HTTP bodies, diagnostic headers, ssh "remote:" text, rejection reasons. A bare CR rewrites the line without any escape sequence, so "rejected\rok refs/heads/main" still read as a success. The policy is now split by entry point: Writer keeps CR, because streamed progress depends on it and the "source:"/"target:" prefix bounds what a rewritten row can claim; Text drops it, because a one-shot message has no reason to move the cursor. The spoofing test asserted only that ESC was gone and so passed while allowing this — it now checks for both.

Co-Authored-By: Claude Opus 5 (1M context) noreply@anthropic.com Entire-Checkpoint: 01M0D4YTRH28GY8WHTCXSHCPKT

Checkpoints

Security Review and Vulnerability Fixes

Claude CodeOpus 5.[1m]
View session
Checkpoint 1