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

From: Patrick Steinhardt <[email-masked]>
Date: Mon, 19 Jan 2026 08:20:41 +0100

On Fri, Jan 16, 2026 at 07:21:04AM -0800, Junio C Hamano wrote:
> Ondrej Pohorelsky <[email-masked]> writes:
> 
> > Hi, I just want to weight in from the downstream maintainer POV.
> > We've been carrying the patches Johannes has created in Fedora, CentOS
> > and RHEL for at least half a year now.
> > The only change I did is to make the new behavior opt-in by default
> > and give the RHEL customers a release note explaining it.
> 
> Thanks for your great input.  FWIW, I do not think anybody around
> here is against "opt-in with a note" approach at all.
> 
> > I think the patches proposed are making sense, and they should be
> > merged. Even having them as opt-in is better than not having them
> > merged at all.
> 
> I do not think anybody disagrees with this sentiment.  Back when the
> patches originally was discussed on the public list here, nobody was
> against adding it as an _optional_ feature to filter some byte
> sequences out of the end-user's data stream, and the review comments
> that led to the topic marked to be "expecting a reroll", if I recall
> correctly, were all about "why would we make this on by default?"
> Peff's message that reignited the topic this time around is also
> about the same.
> 
> We are still hearing from Dscho that he cannot think of a scenario
> where making this mandatory with opt-out would break existing
> legitimate setup people may have (I am paraphrasing [*]), but I
> think that is aiming in the wrong direction.  It does not matter if
> you consider the approach your users take is "broken by design"; as
> long as it works for them in their (limited) settings, it is a valid
> arrangement to send arbitrary byte sequence over the sideband even
> it happens to include ANSI escapes and other "curiosities".  We have
> in no position to unilaterally break them, telling them that we left
> a way open for them to disable.  That is not how to deliver features.

I think what I strongly disagree with is that this is considered to be a
feature. I myself don't consider this to be a feature though, but rather
a security fix for a bug that can lead to arbitrary code execution on
the client-side, for example via title bar injection.

It's not the first time that we change existing behaviour in a backwards
incompatible way because of a newly discovered attack vector. So I have
to wonder what's so different about this particular case here.

Patrick