From: Johannes Schindelin <Johannes.Schindelin@gmx•de>
To: Junio C Hamano <gitster@pobox•com>
Cc: Johannes Schindelin via GitGitGadget <gitgitgadget@gmail•com>,
git@vger•kernel.org,
"brian m. carlson" <sandals@crustytoothpaste•net>,
Phillip Wood <phillip.wood123@gmail•com>,
Andreas Schwab <schwab@linux-m68k•org>,
Ondrej Pohorelsky <opohorel@redhat•com>
Subject: Re: [PATCH v2 2/4] sideband: introduce an "escape hatch" to allow control characters
Date: Fri, 16 Jan 2026 20:25:42 +0100 (CET) [thread overview]
Message-ID: <2ff3b9a0-9c84-6e9e-d4fe-0a19adcdd215@gmx.de> (raw)
In-Reply-To: <xmqqpl8avbop.fsf@gitster.g>
Hi Junio,
On Fri, 19 Dec 2025, Junio C Hamano wrote:
> Johannes Schindelin <Johannes.Schindelin@gmx•de> writes:
>
> > Good idea! What do you think about something like this?
>
> It may be easier to hack up to piggyback on the http.*.variable
> infrastructure, but I do not like the smell of it very much, because
> the implementation ties it too tightly to the http transport; I
> think this should live in one layer up (transport?).
>
> > If this is the direction you're thinking, I'll polish it and integrate it
> > into v3.
>
> In other words, it would be more like sideband.allowEscapeSequences
> that is overridden by sideband.<url>.allowEscapeSequences was what I
> had in mind. Or even transfer.allowEscapeSequencesInSideband that
> is overridden by transfer.<url>.allowEscapeSequencesInSideband.
That was quite a bit trickier than I hoped for. But here it finally is:
-- snip --
From: Johannes Schindelin <johannes.schindelin@gmx•de>
Subject: [PATCH] sideband: offer to configure sanitizing on a per-URL basis
The main objection against sanitizing the sideband that was raised
during the review of the sideband sanitizing patches, first on the
git-security mailing list, then on the public mailing list, was that
there are some setups where server-side `pre-receive` hooks want to
error out, giving colorful messages to the users on the client side (if
they are not redirecting the output into a file, that is).
To avoid breaking such setups, the default chosen by the sideband
sanitizing patches is to pass through ANSI color sequences.
Still, there might be some use case out there where that is not enough.
Therefore the `sideband.allowControlCharacters` config setting allows
for configuring levels of sanitizing.
As Junio Hamano pointed out, to keep users safe by default, we need to
be able to scope this to some servers because while a user may trust
their company's Git server, the same might not apply to other Git
servers.
To allow for this, let's imitate the way `http.<url>.*` offers
to scope config settings to certain URLs, by letting users
override the `sideband.allowControlCharacters` setting via
`sideband.<url>.allowControlCharacters`.
Suggested-by: Junio Hamano <gitster@pobox•com>
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx•de>
---
Documentation/config/sideband.txt | 4 ++
sideband.c | 69 +++++++++++++++++++++--------
sideband.h | 14 ++++++
t/t5409-colorize-remote-messages.sh | 24 ++++++++++
transport.c | 3 ++
5 files changed, 96 insertions(+), 18 deletions(-)
diff --git a/Documentation/config/sideband.txt b/Documentation/config/sideband.txt
index 8eb7656cdd2..cc2cc463bef 100644
--- a/Documentation/config/sideband.txt
+++ b/Documentation/config/sideband.txt
@@ -22,3 +22,7 @@ sideband.allowControlCharacters::
true::
Allow all control characters to be sent to the terminal.
--
+
+sideband.<url>.*::
+ Apply the `sideband.*` option selectively to specific URLs. The
+ same URL matching logic applies as for `http.<url>.*` settings.
diff --git a/sideband.c b/sideband.c
index 725e24db0db..e856981ea55 100644
--- a/sideband.c
+++ b/sideband.c
@@ -9,6 +9,7 @@
#include "help.h"
#include "pkt-line.h"
#include "write-or-die.h"
+#include "urlmatch.h"
struct keyword_entry {
/*
@@ -26,13 +27,14 @@ static struct keyword_entry keywords[] = {
};
static enum {
+ ALLOW_CONTROL_SEQUENCES_UNSET = -1,
ALLOW_NO_CONTROL_CHARACTERS = 0,
ALLOW_ANSI_COLOR_SEQUENCES = 1<<0,
ALLOW_ANSI_CURSOR_MOVEMENTS = 1<<1,
ALLOW_ANSI_ERASE = 1<<2,
ALLOW_DEFAULT_ANSI_SEQUENCES = ALLOW_ANSI_COLOR_SEQUENCES,
ALLOW_ALL_CONTROL_CHARACTERS = 1<<3,
-} allow_control_characters = ALLOW_DEFAULT_ANSI_SEQUENCES;
+} allow_control_characters = ALLOW_CONTROL_SEQUENCES_UNSET;
static inline int skip_prefix_in_csv(const char *value, const char *prefix,
const char **out)
@@ -44,8 +46,19 @@ static inline int skip_prefix_in_csv(const char *value, const char *prefix,
return 1;
}
-static void parse_allow_control_characters(const char *value)
+int sideband_allow_control_characters_config(const char *var, const char *value)
{
+ switch (git_parse_maybe_bool(value)) {
+ case 0:
+ allow_control_characters = ALLOW_NO_CONTROL_CHARACTERS;
+ return 0;
+ case 1:
+ allow_control_characters = ALLOW_ALL_CONTROL_CHARACTERS;
+ return 0;
+ default:
+ break;
+ }
+
allow_control_characters = ALLOW_NO_CONTROL_CHARACTERS;
while (*value) {
if (skip_prefix_in_csv(value, "default", &value))
@@ -61,9 +74,37 @@ static void parse_allow_control_characters(const char *value)
else if (skip_prefix_in_csv(value, "false", &value))
allow_control_characters = ALLOW_NO_CONTROL_CHARACTERS;
else
- warning(_("unrecognized value for `sideband."
- "allowControlCharacters`: '%s'"), value);
+ warning(_("unrecognized value for '%s': '%s'"), var, value);
}
+ return 0;
+}
+
+static int sideband_config_callback(const char *var, const char *value,
+ const struct config_context *ctx UNUSED,
+ void *data UNUSED)
+{
+ if (!strcmp(var, "sideband.allowcontrolcharacters"))
+ return sideband_allow_control_characters_config(var, value);
+
+ return 0;
+}
+
+void sideband_apply_url_config(const char *url)
+{
+ struct urlmatch_config config = URLMATCH_CONFIG_INIT;
+ char *normalized_url;
+
+ if (!url)
+ BUG("must not call sideband_apply_url_config(NULL)");
+
+ config.section = "sideband";
+ config.collect_fn = sideband_config_callback;
+
+ normalized_url = url_normalize(url, &config.url);
+ git_config(urlmatch_config_entry, &config);
+ free(normalized_url);
+ string_list_clear(&config.vars, 1);
+ urlmatch_config_release(&config);
}
/* Returns a color setting (GIT_COLOR_NEVER, etc). */
@@ -79,20 +120,12 @@ static int use_sideband_colors(void)
if (use_sideband_colors_cached >= 0)
return use_sideband_colors_cached;
- switch (git_config_get_maybe_bool("sideband.allowcontrolcharacters", &i)) {
- case 0: /* Boolean value */
- allow_control_characters = i ? ALLOW_ALL_CONTROL_CHARACTERS :
- ALLOW_NO_CONTROL_CHARACTERS;
- break;
- case -1: /* non-Boolean value */
- if (git_config_get_string_tmp("sideband.allowcontrolcharacters",
- &value))
- ; /* huh? `get_maybe_bool()` returned -1 */
- else
- parse_allow_control_characters(value);
- break;
- default:
- break; /* not configured */
+ if (allow_control_characters == ALLOW_CONTROL_SEQUENCES_UNSET) {
+ if (!git_config_get_value("sideband.allowcontrolcharacters", &value))
+ sideband_allow_control_characters_config("sideband.allowcontrolcharacters", value);
+
+ if (allow_control_characters == ALLOW_CONTROL_SEQUENCES_UNSET)
+ allow_control_characters = ALLOW_DEFAULT_ANSI_SEQUENCES;
}
if (!git_config_get_string_tmp(key, &value))
diff --git a/sideband.h b/sideband.h
index 5a25331be55..d15fa4015fa 100644
--- a/sideband.h
+++ b/sideband.h
@@ -30,4 +30,18 @@ int demultiplex_sideband(const char *me, int status,
void send_sideband(int fd, int band, const char *data, ssize_t sz, int packet_max);
+/*
+ * Apply sideband configuration for the given URL. This should be called
+ * when a transport is created to allow URL-specific configuration of
+ * sideband behavior (e.g., sideband.<url>.allowControlCharacters).
+ */
+void sideband_apply_url_config(const char *url);
+
+/*
+ * Parse and set the sideband allow control characters configuration.
+ * The var parameter should be the key name (without section prefix).
+ * Returns 0 if the variable was recognized and handled, non-zero otherwise.
+ */
+int sideband_allow_control_characters_config(const char *var, const char *value);
+
#endif
diff --git a/t/t5409-colorize-remote-messages.sh b/t/t5409-colorize-remote-messages.sh
index c3e4e143627..1d039cbdafb 100755
--- a/t/t5409-colorize-remote-messages.sh
+++ b/t/t5409-colorize-remote-messages.sh
@@ -167,4 +167,28 @@ test_expect_success 'control sequences in sideband allowed by default' '
test_grep ! "\\^\\[\\[G" decoded
'
+test_expect_success 'allow all control sequences for a specific URL' '
+ write_script .git/eraser <<-\EOF &&
+ printf "error: Ohai!\\r\\033[K" >&2
+ exec "$@"
+ EOF
+ test_config_global uploadPack.packObjectsHook ./eraser &&
+ test_commit one-more-please &&
+
+ rm -rf throw-away &&
+ git clone --no-local . throw-away 2>stderr &&
+ test_decode_color <stderr >color-decoded &&
+ test_decode_csi <color-decoded >decoded &&
+ test_grep ! "CSI \\[K" decoded &&
+ test_grep "\\^\\[\\[K" decoded &&
+
+ rm -rf throw-away &&
+ git -c "sideband.file://.allowControlCharacters=true" \
+ clone --no-local "file://$PWD" throw-away 2>stderr &&
+ test_decode_color <stderr >color-decoded &&
+ test_decode_csi <color-decoded >decoded &&
+ test_grep "CSI \\[K" decoded &&
+ test_grep ! "\\^\\[\\[K" decoded
+'
+
test_done
diff --git a/transport.c b/transport.c
index 1098bbd60e4..e19536c9c6b 100644
--- a/transport.c
+++ b/transport.c
@@ -28,6 +28,7 @@
#include "object-name.h"
#include "color.h"
#include "bundle-uri.h"
+#include "sideband.h"
static int transport_use_color = -1;
static char transport_colors[][COLOR_MAXLEN] = {
@@ -1210,6 +1211,8 @@ struct transport *transport_get(struct remote *remote, const char *url)
ret->hash_algo = &hash_algos[GIT_HASH_SHA1];
+ sideband_apply_url_config(ret->url);
+
return ret;
}
-- snap --
That should address this particular concern of yours.
> >> - It may no longer matter but a remote repository that may send
> >> messages as strings encoded in ISO/IEC 2022 would need to set
> >> this, merely to make the messages human-readable. There may be
> >> other reasons the trusted repositories want to send "escape
> >> sequences".
> >
> > If the remote side has no way to determine whether the client side is
> > connected to a terminal or not (which we have already established in this
> > thread), it has even less chance to determine which character encoding is
> > in use...
>
> Then I think you need to re-read brian's
>
> https://lore.kernel.org/git/aS-D5lD2Kk6BHNIl@fruit.crustytoothpaste.net/
Oh, but brian described a scenario that is quite different: it is using
SSH. And there, the hook has quite literally the ability to verify that
the output goes to a terminal. It is also implicitly much more trustable
than a random HTTPS server because if you have SSH credentials to
authenticate with a server, there is already a much stronger trust
relationship here. That scenario is far away from some repository on
GitHub requiring a recursive clone that then points to some totally
untrustworthy HTTPS server hosted by the attacker.
> In any case, I do not think ISO/IEC 2022 matters as much as it used
> to back when the reencode_string_iconv() was written (which was the
> topic of another thread regarding the broken iconv on macOS wrt
> 2022). But even if we limit ourselves to UTF-8, brian's point that
> applications do assume certain characteristics on its clients and
> implements unportable stuff. A project targetting developers and/or
> users from certain locale may use their own hooks that assumes the
> clients understands strings in certain language in certain encoding.
>
> And to serve these projects better, classes like "pass colors",
> "pass cursor movements", might help than just "pass everything" vs
> "deny everything", but we probably want to try to keep it as simple
> as possible; trying to make it finer grained with extra complexity
> would only make our efforts look like whack-a-mole X-<.
Well, I only introduced that complexity because you asked for it on the
git-security mailing list. You offered concerns that an all-or-nothing
escape hatch was not fine-grained enough. It was hard enough for me to
tickle out what granularity exactly would be needed in addition to appease
the concerns, and obviously even what I did was not enough to be
accepted...
> >> It might even be a good idea to make the default setting of this
> >> variable "allow", except for the initial connections to repositories
> >> (i.e., "git clone $URL", and "git fetch/ls-remote $URL" with an
> >> explicit $URL without using a nickname recorded in our .git/config),
> >> as visiting a potentially malicious remote repository you are not
> >> familiar with may not be uncommon, and users may deserve protection
> >> over inconvenience.
> >>
> >> But once the user establishes a working relationship with a remote
> >> repository, would it be a lot more common to trust the contents
> >> there than be on the lookout that the repository may spew bad
> >> strings of bytes at your standard error stream, I have to wonder.
>
> > tl;dr remote servers don't get more trustworthy just by successfully
> > serving clones.
>
> The "successfully serving clone" has nothing to do with the reason
> why I suggested to deny by default in "clone" and anything that gets
> $URL not remote nickname. I am roughly equating the fact that the
> user cloned *and* *then* continues to interact with the project that
> is served from that remote repository (hence using the remote
> nickname) with the willingness by the user to trust that particular
> remote repository.
Such a willingness to trust might be only because nothing bad happened
during the clone, though. In which case we made things worse that way, not
better.
Ciao,
Johannes
next prev parent reply other threads:[~2026-01-16 19:25 UTC|newest]
Thread overview: 85+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-01-14 18:19 [PATCH 0/3] Sanitize sideband channel messages Johannes Schindelin via GitGitGadget
2025-01-14 18:19 ` [PATCH 1/3] sideband: mask control characters Johannes Schindelin via GitGitGadget
2025-01-15 14:49 ` Phillip Wood
2025-12-02 15:43 ` Johannes Schindelin
2025-01-15 15:17 ` Andreas Schwab
2025-01-15 16:24 ` Junio C Hamano
2025-01-14 18:19 ` [PATCH 2/3] sideband: introduce an "escape hatch" to allow " Johannes Schindelin via GitGitGadget
2025-01-14 18:19 ` [PATCH 3/3] sideband: do allow ANSI color sequences by default Johannes Schindelin via GitGitGadget
2025-01-14 22:50 ` [PATCH 0/3] Sanitize sideband channel messages brian m. carlson
2025-01-16 6:45 ` Junio C Hamano
2025-01-28 16:03 ` Ondrej Pohorelsky
2025-01-31 17:55 ` Junio C Hamano
2025-12-02 14:11 ` Johannes Schindelin
2025-12-03 0:47 ` brian m. carlson
2025-12-03 8:04 ` Johannes Schindelin
2025-01-15 14:49 ` Phillip Wood
2025-12-02 14:56 ` Johannes Schindelin
2025-12-17 14:23 ` [PATCH v2 0/4] " Johannes Schindelin via GitGitGadget
2025-12-17 14:23 ` [PATCH v2 1/4] sideband: mask control characters Johannes Schindelin via GitGitGadget
2026-01-09 12:38 ` Patrick Steinhardt
2026-01-16 19:29 ` Johannes Schindelin
2025-12-17 14:23 ` [PATCH v2 2/4] sideband: introduce an "escape hatch" to allow " Johannes Schindelin via GitGitGadget
2025-12-18 2:22 ` Junio C Hamano
2025-12-18 17:59 ` Johannes Schindelin
2025-12-19 13:33 ` Junio C Hamano
2026-01-16 19:25 ` Johannes Schindelin [this message]
2026-01-09 12:38 ` Patrick Steinhardt
2025-12-17 14:23 ` [PATCH v2 3/4] sideband: do allow ANSI color sequences by default Johannes Schindelin via GitGitGadget
2026-01-09 12:38 ` Patrick Steinhardt
2026-01-16 19:38 ` Johannes Schindelin
2025-12-17 14:23 ` [PATCH v2 4/4] sideband: add options to allow more control sequences to be passed through Johannes Schindelin via GitGitGadget
2026-01-09 12:38 ` Patrick Steinhardt
2026-01-10 17:26 ` brian m. carlson
2026-01-15 21:14 ` Jeff King
2026-01-15 21:36 ` Junio C Hamano
2026-01-15 23:12 ` Johannes Schindelin
2026-01-16 6:45 ` Patrick Steinhardt
2026-01-16 12:12 ` Ondrej Pohorelsky
2026-01-16 15:21 ` Junio C Hamano
2026-01-16 18:46 ` Johannes Schindelin
2026-01-16 19:24 ` Junio C Hamano
2026-01-19 7:20 ` Patrick Steinhardt
2026-01-19 22:16 ` brian m. carlson
2026-01-20 2:41 ` D. Ben Knoble
2026-01-20 17:05 ` Junio C Hamano
2026-01-20 19:31 ` Jeff King
2026-01-20 20:11 ` Junio C Hamano
2026-01-21 7:39 ` Patrick Steinhardt
2026-01-22 12:29 ` Johannes Schindelin
2026-01-22 17:58 ` Junio C Hamano
2026-01-15 23:10 ` brian m. carlson
2026-02-03 1:11 ` Junio C Hamano
2026-02-03 7:12 ` Johannes Schindelin
2026-02-03 19:00 ` Junio C Hamano
2026-02-04 19:35 ` Junio C Hamano
2026-01-16 19:47 ` Johannes Schindelin
2026-01-16 22:26 ` [PATCH v3 0/5] Sanitize sideband channel messages Johannes Schindelin via GitGitGadget
2026-01-16 22:26 ` [PATCH v3 1/5] sideband: mask control characters Johannes Schindelin via GitGitGadget
2026-01-16 22:26 ` [PATCH v3 2/5] sideband: introduce an "escape hatch" to allow " Johannes Schindelin via GitGitGadget
2026-01-16 22:26 ` [PATCH v3 3/5] sideband: do allow ANSI color sequences by default Johannes Schindelin via GitGitGadget
2026-01-16 22:26 ` [PATCH v3 4/5] sideband: add options to allow more control sequences to be passed through Johannes Schindelin via GitGitGadget
2026-01-16 22:26 ` [PATCH v3 5/5] sideband: offer to configure sanitizing on a per-URL basis Johannes Schindelin via GitGitGadget
2026-01-16 22:32 ` [PATCH v3 0/5] Sanitize sideband channel messages Johannes Schindelin
2026-02-03 10:17 ` [PATCH v4 0/6] " Johannes Schindelin via GitGitGadget
2026-02-03 10:17 ` [PATCH v4 1/6] sideband: mask control characters Johannes Schindelin via GitGitGadget
2026-02-03 10:17 ` [PATCH v4 2/6] sideband: introduce an "escape hatch" to allow " Johannes Schindelin via GitGitGadget
2026-02-03 10:17 ` [PATCH v4 3/6] sideband: do allow ANSI color sequences by default Johannes Schindelin via GitGitGadget
2026-02-03 10:18 ` [PATCH v4 4/6] sideband: add options to allow more control sequences to be passed through Johannes Schindelin via GitGitGadget
2026-02-03 10:18 ` [PATCH v4 5/6] sideband: offer to configure sanitizing on a per-URL basis Johannes Schindelin via GitGitGadget
2026-02-03 10:18 ` [PATCH v4 6/6] sideband: delay sanitizing by default to Git v3.0 Johannes Schindelin via GitGitGadget
2026-02-04 19:26 ` [PATCH v4 0/6] Sanitize sideband channel messages Junio C Hamano
2026-02-05 14:48 ` Junio C Hamano
2026-02-13 23:50 ` Junio C Hamano
2026-03-02 18:11 ` [PATCH 0/3] Sanitizing sideband output Junio C Hamano
2026-03-02 18:11 ` [PATCH 1/3] sideband: drop 'default' configuration Junio C Hamano
2026-03-02 18:11 ` [PATCH 2/3] sideband: delay sanitizing by default to Git v3.0 Junio C Hamano
2026-03-02 18:11 ` [PATCH 3/3] sideband: conditional documentation fix Junio C Hamano
2026-03-05 23:34 ` [PATCH v5 0/7] Sanitizing sideband output Junio C Hamano
2026-03-05 23:34 ` [PATCH v5 1/7] sideband: mask control characters Junio C Hamano
2026-03-05 23:34 ` [PATCH v5 2/7] sideband: introduce an "escape hatch" to allow " Junio C Hamano
2026-03-05 23:34 ` [PATCH v5 3/7] sideband: do allow ANSI color sequences by default Junio C Hamano
2026-03-05 23:34 ` [PATCH v5 4/7] sideband: add options to allow more control sequences to be passed through Junio C Hamano
2026-03-05 23:34 ` [PATCH v5 5/7] sideband: offer to configure sanitizing on a per-URL basis Junio C Hamano
2026-03-05 23:34 ` [PATCH v5 6/7] sideband: drop 'default' configuration Junio C Hamano
2026-03-05 23:34 ` [PATCH v5 7/7] sideband: delay sanitizing by default to Git v3.0 Junio C Hamano
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=2ff3b9a0-9c84-6e9e-d4fe-0a19adcdd215@gmx.de \
--to=johannes.schindelin@gmx$(echo .)de \
--cc=git@vger$(echo .)kernel.org \
--cc=gitgitgadget@gmail$(echo .)com \
--cc=gitster@pobox$(echo .)com \
--cc=opohorel@redhat$(echo .)com \
--cc=phillip.wood123@gmail$(echo .)com \
--cc=sandals@crustytoothpaste$(echo .)net \
--cc=schwab@linux-m68k$(echo .)org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox