[PATCH 2/3] sideband: delay sanitizing by default to Git v3.0

[Junio C Hamano <gitster@pobox.com>]

From: Johannes Schindelin <johannes.schindelin@gmx.de>

The sideband sanitization patches allow ANSI color sequences through
by default, preserving compatibility with pre-receive hooks that
provide colored output during `git push`.

Even so, there is concern that changing any default behavior in a
minor release may have unforeseen consequences. To accommodate this,
defer the secure-by-default behavior to Git v3.0, where breaking
changes are expected.

This gives users and tooling time to prepare, while committing to
address CVE-2024-52005 in Git v3.0.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
[jc: adjusted for the removal of 'default' value]
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 Documentation/config/sideband.adoc  |  7 +++++++
 sideband.c                          |  6 +++++-
 t/t5409-colorize-remote-messages.sh | 18 +++++++++++++-----
 3 files changed, 25 insertions(+), 6 deletions(-)

diff --git a/Documentation/config/sideband.adoc b/Documentation/config/sideband.adoc
index 96fade7f5f..85205477b7 100644
--- a/Documentation/config/sideband.adoc
+++ b/Documentation/config/sideband.adoc
@@ -1,6 +1,13 @@
 sideband.allowControlCharacters::
+ifdef::with-breaking-changes[]
 	By default, control characters that are delivered via the sideband
 	are masked, except ANSI color sequences. This prevents potentially
+endif::with-breaking-changes[]
+ifndef::with-breaking-changes[]
+	By default, no control characters delivered via the sideband
+	are masked. This is unsafe and will change in Git v3.* to only
+	allow ANSI color sequences by default, preventing potentially
+endif::with-breaking-changes[]
 	unwanted ANSI escape sequences from being sent to the terminal. Use
 	this config setting to override this behavior (the value can be
 	a comma-separated list of the following keywords):
diff --git a/sideband.c b/sideband.c
index 04282a568e..5fb60e52bf 100644
--- a/sideband.c
+++ b/sideband.c
@@ -34,7 +34,11 @@ static enum {
 	ALLOW_ANSI_CURSOR_MOVEMENTS   = 1<<1,
 	ALLOW_ANSI_ERASE              = 1<<2,
 	ALLOW_ALL_CONTROL_CHARACTERS  = 1<<3,
-	ALLOW_DEFAULT_ANSI_SEQUENCES  = ALLOW_ANSI_COLOR_SEQUENCES
+#ifdef WITH_BREAKING_CHANGES
+	ALLOW_DEFAULT_ANSI_SEQUENCES  = ALLOW_ANSI_COLOR_SEQUENCES,
+#else
+	ALLOW_DEFAULT_ANSI_SEQUENCES  = ALLOW_ALL_CONTROL_CHARACTERS,
+#endif
 } allow_control_characters = ALLOW_CONTROL_SEQUENCES_UNSET;
 
 static inline int skip_prefix_in_csv(const char *value, const char *prefix,
diff --git a/t/t5409-colorize-remote-messages.sh b/t/t5409-colorize-remote-messages.sh
index 3010913bb1..07cbc62736 100755
--- a/t/t5409-colorize-remote-messages.sh
+++ b/t/t5409-colorize-remote-messages.sh
@@ -98,6 +98,13 @@ test_expect_success 'fallback to color.ui' '
 	grep "<BOLD;RED>error<RESET>: error" decoded
 '
 
+if test_have_prereq WITH_BREAKING_CHANGES
+then
+	TURN_ON_SANITIZING=already.turned=on
+else
+	TURN_ON_SANITIZING=sideband.allowControlCharacters=color
+fi
+
 test_expect_success 'disallow (color) control sequences in sideband' '
 	write_script .git/color-me-surprised <<-\EOF &&
 	printf "error: Have you \\033[31mread\\033[m this?\\a\\n" >&2
@@ -106,7 +113,7 @@ test_expect_success 'disallow (color) control sequences in sideband' '
 	test_config_global uploadPack.packObjectsHook ./color-me-surprised &&
 	test_commit need-at-least-one-commit &&
 
-	git clone --no-local . throw-away 2>stderr &&
+	git -c $TURN_ON_SANITIZING clone --no-local . throw-away 2>stderr &&
 	test_decode_color <stderr >decoded &&
 	test_grep RED decoded &&
 	test_grep "\\^G" stderr &&
@@ -138,7 +145,7 @@ test_decode_csi() {
 	}'
 }
 
-test_expect_success 'control sequences in sideband allowed by default' '
+test_expect_success 'control sequences in sideband allowed by default (in Git v3.8)' '
 	write_script .git/color-me-surprised <<-\EOF &&
 	printf "error: \\033[31mcolor\\033[m\\033[Goverwrite\\033[Gerase\\033[K\\033?25l\\n" >&2
 	exec "$@"
@@ -147,7 +154,7 @@ test_expect_success 'control sequences in sideband allowed by default' '
 	test_commit need-at-least-one-commit-at-least &&
 
 	rm -rf throw-away &&
-	git clone --no-local . throw-away 2>stderr &&
+	git -c $TURN_ON_SANITIZING clone --no-local . throw-away 2>stderr &&
 	test_decode_color <stderr >color-decoded &&
 	test_decode_csi <color-decoded >decoded &&
 	test_grep ! "CSI \\[K" decoded &&
@@ -175,14 +182,15 @@ test_expect_success 'allow all control sequences for a specific URL' '
 	test_commit one-more-please &&
 
 	rm -rf throw-away &&
-	git clone --no-local . throw-away 2>stderr &&
+	git -c $TURN_ON_SANITIZING 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" \
+	git -c sideband.allowControlCharacters=false \
+		-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 &&
-- 
2.53.0-549-g863838a955