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

[Patrick Steinhardt <ps@pks.im>]

On Fri, Jan 16, 2026 at 12:12:47AM +0100, Johannes Schindelin wrote:
> Hi Junio, Jeff, and other interested parties,
> 
> On Thu, 15 Jan 2026, Junio C Hamano wrote:
> 
> > Jeff King <peff@peff.net> writes:
> > 
> > > Is there any reason we cannot introduce the new functionality as a
> > > config option but _not_ enable it by default?
> > >
> > > That gives people the tools to protect themselves if they want to bear
> > > the potential cost. It just feels a shame to deny them the tool because
> > > we can't agree on the default.
> > 
> > Yeah, I like the suggestion---making it opt-in would have much less
> > chance of breaking set-up people are relying on all of a sudden.
> 
> Can you help me understand how these existing use cases (which are not
> actually in wide-spread use) aren't broken by design, given that they have
> no chance to ensure that their ANSI sequences go to an actual terminal
> that can understand those sequences?
> 
> As such, it looks to me as if they have a valid goal, but go about it in a
> way that is easily improved: If they want color in their sideband output,
> then Git has to be taught about it, much in the same way as bf1a11f0a10
> (sideband: highlight keywords in remote sideband output, 2018-08-07)
> taught Git to highlight keywords in the remote sideband output. That is
> the actual correct way to do this, not by expecting Git to pass through
> all bytes to the terminal without sanitizing, which is a well-known worst
> practice (not even GNU tar does that when listing the contents of an
> archive, nor does cURL do that, just to list two of the command-line
> programs that sanitize properly what they pass on to the terminal).
> 
> Given that those use cases are rare (none of the popular Git forges
> support this!), and that it is a security issue, I still think that the
> default should be as I proposed: To pass through only a small subset of
> ANSI control sequences that you gentle people already agreed should be
> safe.

I have to agree with Johannes here. There's been way to many CVEs
assigned to terminal emulators out there that allowed arbitrary code
execution via ANSI escape sequences. Sure, you could argue that this is
an issue in the terminal emulator that needs to be fixed, and that is
certainly true. But we are significantly increasing the attack surface
if we don't sanitize escape sequences. And even when working as designed
I would claim that a lot of the escape sequences can cause active harm
[1][2][3].

So I would think that we should have behaviour in Git that is safe by
default, not safe if you know that the options happen to exist. Because
if we do the latter, then the majority of people will never enable it,
and I'm just not sure whether it's a good idea to increase the attack
surface for the majority of our users only to enable a small set of
niche edge cases. Doubly so when those niche edge cases can be made to
work again with an opt-out.

Patrick

[1]: https://cwe.mitre.org/data/definitions/150.html
[2]: https://www.infosecmatter.com/terminal-escape-injection/
[3]: https://www.cyberark.com/resources/threat-research-blog/dont-trust-this-title-abusing-terminal-emulators-with-ansi-escape-characters