Re: [PATCH v2 4/4] sideband: add options to allow more control sequences to be passed through

Hi Junio,

I disagree with making sideband sanitization opt-in or weakening it based on a "trusted remote" heuristic. In this context, emitting untrusted bytes to a terminal without proper sanitization is a security-relevant bug; safe-by-default should be the baseline.

On Tue, 20 Jan 2026, Junio C Hamano wrote:

If the goal is to mitigate terminal escape injection from remote-controlled output, then shipping it disabled by default does not mitigate the default case. Most users will not discover or enable a hardening knob until after an incident.

I don't think we can safely infer "trusted enough to write to my terminal" from "I fetch from there often". A previously-trusted remote can be compromised, after all. Which means that a trust-based default is a foot-gun: it creates a path where users believe they're protected while the program is intentionally passing through attacker-controlled escape sequences.

Besides, allowing "colorful output from their hooks" _is already allowed by default_ in the proposed patch series. The config variable `sideband.allowControlCharacters` isn't an "all or nothing" setting, after all.

Color is already a narrowly-scoped exception. Cursor movement / erase sequences are in a different category because they can rewrite prior output and hide what actually happened. If we want to allow them, it should remain explicit opt-in on the client, not something we enable automatically based on repository state.

If the argument is "setting `sideband.allowControlCharacters` to `color` by default breaks common workflows on established remotes", can you point to a concrete repro (hook snippet + terminal + escape sequences relied on) or a public example? Without that, I don't think we should bias the default toward pass-through of higher-risk sequences.

Absent such evidence, the best way to proceed is to keep sanitization enabled by default for sideband output (modulo color), with the clearly documented escape hatch for users who knowingly want additional sequences. If there is a strong need for per-remote behavior, there is `sideband.<url>.allowControlCharacters`, as per v3), i.e. users _do_ have that option _after_ stating that they trust that particular remote not to wreak havoc with their terminal.

Also keep in mind that this patch series' scope is the sideband channel; The fact that SSH-based transports patch through `stderr` (completely side-stepping sideband) is out of scope.

Ciao, Johannes

P.S.: Junio: if we continue to discuss "opt-in"/"opt-out", I think we need to be more explicit about which behavior we mean. We now have multiple levels in `sideband.allowControlCharacters` (default allows color; `cursor`, `erase`, `false` and `true` allow more fine-grained levels).

If the proposal is "full pass-through of all control characters is opt-in", or "full sanitizing of all control characters is opt-in", I whole-heartedly agree: That is already opt-in via setting `sideband.allowControlCharacters` to `false` or `true`, respectively.

If the proposal is "keep the historical behavior (verbatim sideband payload, no sanitization) as the default, and make sanitization opt-in", I am firmly opposed: This makes the sideband payload remote-controlled; A security hardening that is off by default will not protect the default user population.

Can you confirm which of these two meanings you intend when you say "opt-in" here? Once that's clarified, we can discuss whether the default should remain at "color-only" (today's default) with explicit opt-in for riskier sequences, or whether you're arguing for no filtering at all by default.