Skip to content

fix: apply --sanitize=always when output is piped - #3995

Open
pucedoteth wants to merge 1 commit into
sharkdp:masterfrom
pucedoteth:fix/sanitize-applies-when-piped
Open

fix: apply --sanitize=always when output is piped#3995
pucedoteth wants to merge 1 commit into
sharkdp:masterfrom
pucedoteth:fix/sanitize-applies-when-piped

Conversation

@pucedoteth

Copy link
Copy Markdown

Summary

--sanitize=always does nothing when bat's output is piped.

$ printf 'a\xe2\x80\xaeb\n' | bat --sanitize=always | od -An -tx1
 61 e2 80 ae 62 0a          # U+202E RLO survives untouched

$ printf 'a\xe2\x80\xaeb\n' | bat --sanitize=always --color=always | od -An -tx1
 ... 61 ef bf bd 62 ...     # U+FFFD, as documented

sanitize() is only called from InteractivePrinter. The controller picks SimplePrinter whenever loop_through is set, and loop_through is true unless stdout is a terminal or one of --color=always, --decorations=always, --force-colorization, -n, -b was passed. SimplePrinter::print_line writes line_buffer straight to the handle.

So the flag is silently dropped in exactly the case it matters most. --sanitize exists to make untrusted bytes safe for a terminal to render (CVE-2021-42574, added in #3729), and redirecting into a file or a pager is precisely when those bytes get rendered by a terminal later rather than now. bat --sanitize=always untrusted.txt > out.txt reads as a request for safe output and does not produce one.

Changes

src/printer.rsSimplePrinter::print_line applies sanitize() when --sanitize=always was given:

} else if self.config.sanitize == StripAnsiMode::Always {
    let line = sanitize(&String::from_utf8_lossy(line_buffer));
    write!(handle, "{line}")?;
}

The passthrough output shape is unchanged — no header, no decorations, no line numbers — only the dangerous bytes are substituted:

$ printf 'a\xe2\x80\xaeb\n' | bat --sanitize=always | od -An -tx1
 61 ef bf bd 62 0a

Every other mode keeps the existing raw write_all fast path, so --sanitize=never and the default are byte-for-byte unaffected. Lossy UTF-8 decoding is confined to the sanitizing branch, which seemed acceptable there — the purpose of that branch is to make the bytes safe to display.

Scoped deliberately: --sanitize=auto is left alone. auto already declines to touch plain text, and making it act on piped output would be a behaviour change for people who never asked for it. Only the explicit always is honoured here. --strip-ansi=always has the same gap and is not changed — happy to do it in a follow-up if you want the two kept symmetric, but it is a wider blast radius than the security flag and felt like your call rather than mine.

Tests

Three cases in tests/integration_tests.rs. The existing sanitize tests all pass --decorations=always, which routes to InteractivePrinter and hides this entirely, so the new ones deliberately omit it:

  • sanitize_always_applies_when_output_is_piped
  • sanitize_always_strips_ansi_when_output_is_piped
  • sanitize_never_leaves_piped_output_untouched

Reverting only src/printer.rs and keeping the tests fails exactly the two that cover the fix:

test sanitize_always_applies_when_output_is_piped ... FAILED
test sanitize_always_strips_ansi_when_output_is_piped ... FAILED
test result: FAILED. 12 passed; 2 failed

The never case passes either way — it is there to catch the change over-reaching, not to catch the bug.

Full suite green:

$ cargo test --release
146 passed / 11 passed / 1 passed / 263 passed / 1 passed / 27 passed   (0 failed)

$ cargo clippy --all-targets -- -D warnings
(no warnings)

$ cargo fmt --check
(clean)

🤖 Generated with Claude Code

`SimplePrinter::print_line` writes the input bytes untouched, and the controller
selects it whenever `loop_through` is set -- which is the case whenever stdout is
not a terminal and none of `--color=always`, `--decorations=always`,
`--force-colorization`, `-n` or `-b` was passed.

So an explicit `--sanitize=always` did nothing at all when piping:

    $ printf 'a\xe2\x80\xaeb\n' | bat --sanitize=always | od -An -tx1
     61 e2 80 ae 62 0a          # U+202E survives

That is the case where it matters most. Sanitizing exists to make untrusted bytes
safe for a terminal to render, and redirecting into a file or a pager is exactly
when they get rendered by a terminal later rather than now.

Apply `sanitize()` in `SimplePrinter` when `--sanitize=always` was given. The
passthrough output shape is unchanged -- no header, no decorations -- only the
dangerous bytes are substituted. Every other mode keeps the raw fast path.

The existing sanitize tests all pass `--decorations=always`, which routes to
InteractivePrinter and hides this, so the three added cases deliberately omit it.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@pucedoteth
pucedoteth force-pushed the fix/sanitize-applies-when-piped branch from 1320e9e to b5dac2b Compare September 7, 2026 19:31
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