fix: strip control characters from info field values

[carfeii]

Fixes #1828

Version and Project (repo name) fall back to a package.json/Cargo.toml manifest field when git tags/remote are absent, and those values were styled and written to the terminal in InfoField::style_value with no filtering of control characters. A manifest field has no character restrictions, unlike a git tag/ref, so a crafted manifest could inject terminal escape sequences into onefetch's own output.

Change

Adds sanitize_for_display (src/info/utils/info_field.rs), which replaces control characters (other than \n, since some fields intentionally span multiple lines) with the Unicode replacement character. Applied inside style_value, the single choke point every field's value already passes through, so the fix covers the project-name fallback and any other manifest-derived field, not just the version field used to demonstrate this.

Testing

• Reproduced the issue with a git repo (no tags) and a package.json whose version field contained a JSON-escaped OSC title-set escape sequence, confirmed via a tmux pipe-pane capture (hex-dumped, not just visually inspected) that the decoded raw escape bytes reached the terminal adjacent to onefetch's own SGR color codes.
• Re-ran the same reproduction against this fix; the injected control bytes are gone (replaced with U+FFFD), and unrelated legitimate color codes nearby are untouched.
• Full test suite (132 tests) passes unchanged.

[o2sh]

can you add some unit tests for this?

[spenserblack]

This would only realistically happen with a repository that isn't trustworthy IMO, so I'd argue that this would be a symptom of a much greater problem, which is the user running onefetch on unsafe and malicious repositories.

Git has had a feature for repository trust/ownership for a while now. Should we perhaps rely on that to determine if it's safe to read and output repository contents?

Also, rather than clean the contents, perhaps it would be better to wrap the string in a result? Ok(s), where s is the original string, if it's safe and valid, Err otherwise. I think this makes sense since AFAIK control characters would violate most package manifest specs, anyway. It also avoids allocating a new string, so it's a bit more performant.

[carfeii]

Added unit tests for sanitize_for_display in 4af5377, covering the control-byte-to-replacement-character behavior and confirming existing SGR color codes and normal printable text are left untouched.

On the trust question: Perhaps a repo does not need to already look untrustworthy for this to matter. Running onefetch is often how someone forms that first impression, cloning a new dependency or browsing an unfamiliar repo before deciding whether to trust it. Escape sequences in a manifest field let the repo act on the terminal (change the window title, and depending on the emulator, worse) before that judgment ever gets made. git's safe.directory/ownership check is also a different boundary: it stops git from running config or hooks from a repo you don't own, not display of arbitrary file content like a manifest's version or description field.

On Result<String, _> vs cleaning: fair point on the allocation, and I'm not attached to the specific approach as long as the control bytes never reach the terminal. The behavioral difference is what happens when one field has a single bad byte in it: cleaning keeps the rest of the field and swaps out just the bad byte, Err would drop the whole field.

[spenserblack]

The behavioral difference is what happens when one field has a single bad byte in it: cleaning keeps the rest of the field and swaps out just the bad byte, Err would drop the whole field.

Unless there's a legitimate reason to have a bad byte, I think it's safe to assume the field is malformed at best, malicious at worst, and is an edge case where it's okay to just drop the whole field.