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:

[...] forcing this filtering on everybody [...]  unless it is enabled by
default. [...]

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.

Two levels of defaults [...]  trusted daily remotes vs new remotes.
[...]

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.

[...] you shouldn't have to manually configure "I accept colors from
them". [...]

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.