Skip to content

fix(print): handle carriage returns in --attach text output - #364

Open
jchak7 wants to merge 1 commit into
signadot:mainfrom
jchak7:fix-attach-carriage-returns
Open

fix(print): handle carriage returns in --attach text output#364
jchak7 wants to merge 1 commit into
signadot:mainfrom
jchak7:fix-attach-carriage-returns

Conversation

@jchak7

@jchak7 jchak7 commented Aug 24, 2026

Copy link
Copy Markdown

While reading the --attach output code I noticed the text formatter can leak raw carriage returns into terminal output.

In formatText, the log message is trimmed with strings.TrimRight(e.Msg, "\n"), which drops a trailing \n but leaves a trailing \r. And quoteIfNeeded forces quoting on " \t\n"=" but not \r. So a log line ending in \r\n prints as msg= followed by a bare \r, and an embedded \r inside a message prints raw too. On a terminal that \r sends the cursor back to the start of the line and the next write overwrites it. CRLF endings are common (Windows apps, plenty of containers), so this turns up in real logs.

The fix is small:

  • trim "\r\n" instead of "\n"
  • add \r to the set that forces quoting, so an embedded \r is escaped as \r instead of emitted raw

JSON mode already handled this, since json.Marshal escapes control characters, so this only affects the text output.

This package had no tests, so I added the first ones. They cover quoteIfNeeded across the special characters and the log path for trailing CRLF, embedded CR, and a plain message.

go test ./internal/print/
ok  github.com/signadot/cli/internal/print

I checked each assertion pins the fix: reverting the trim fails only TestFormatTextLogTrimsTrailingCRLF, and reverting the quoteIfNeeded change fails only the carriage-return cases. gofmt is clean.

The text formatter for --attach output trimmed a trailing newline but not
a carriage return, and quoteIfNeeded escaped \n but not \r. A log line
ending in \r\n emitted a raw carriage return, which overwrites the line on
a terminal. An embedded \r did the same.

Trim \r\n and add \r to the set of characters that force quoting. Also adds
the first tests for this package, covering quoteIfNeeded and the log
formatting path.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant