sideband: do allow ANSI color sequences by default

The preceding two commits introduced special handling of the sideband
channel to neutralize ANSI escape sequences before sending the payload
to the terminal, and `sideband.allowControlCharacters` to override that
behavior.

However, as reported by brian m. carlson, some `pre-receive` hooks that
are actively used in practice want to color their messages and therefore
rely on the fact that Git passes them through to the terminal, even
though they have no way to determine whether the receiving side can
actually handle Escape sequences (think e.g. about the practice
recommended by Git that third-party applications wishing to use Git
functionality parse the output of Git commands).

In contrast to other ANSI escape sequences, it is highly unlikely that
coloring sequences can be essential tools in attack vectors that mislead
Git users e.g. by hiding crucial information.

Therefore we can have both: Continue to allow ANSI coloring sequences to
be passed to the terminal by default, and neutralize all other ANSI
Escape sequences.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/Documentation/config/sideband.txt b/Documentation/config/sideband.txt
index 3fb5045cd7958166361abebf9cbc01154df5db18..e5b7383c7a391100b903c014c08e8951b9baffcf 100644 (file)
--- a/Documentation/config/sideband.txt
+++ b/Documentation/config/sideband.txt
@@ -1,5 +1,17 @@
 sideband.allowControlCharacters::
        By default, control characters that are delivered via the sideband
-       are masked, to prevent potentially unwanted ANSI escape sequences
-       from being sent to the terminal. Use this config setting to override
-       this behavior.
+       are masked, except ANSI color sequences. This prevents potentially
+       unwanted ANSI escape sequences from being sent to the terminal. Use
+       this config setting to override this behavior:
++
+--
+       default::
+       color::
+               Allow ANSI color sequences, line feeds and horizontal tabs,
+               but mask all other control characters. This is the default.
+       false::
+               Mask all control characters other than line feeds and
+               horizontal tabs.
+       true::
+               Allow all control characters to be sent to the terminal.
+--

diff --git a/sideband.c b/sideband.c
index 997430f2ea123b4d150db0f5e8672cf4a71a4617..fb43008ab7e7afbcbc0cfa06c0534cf575d3e80c 100644 (file)
--- a/sideband.c
+++ b/sideband.c
@@ -25,7 +25,12 @@ static struct keyword_entry keywords[] = {
        { "error",      GIT_COLOR_BOLD_RED },
 };
 
-static int allow_control_characters;
+static enum {
+       ALLOW_NO_CONTROL_CHARACTERS = 0,
+       ALLOW_ANSI_COLOR_SEQUENCES = 1<<0,
+       ALLOW_DEFAULT_ANSI_SEQUENCES = ALLOW_ANSI_COLOR_SEQUENCES,
+       ALLOW_ALL_CONTROL_CHARACTERS = 1<<1,
+} allow_control_characters = ALLOW_ANSI_COLOR_SEQUENCES;
 
 /* Returns a color setting (GIT_COLOR_NEVER, etc). */
 static int use_sideband_colors(void)
@@ -40,8 +45,26 @@ static int use_sideband_colors(void)
        if (use_sideband_colors_cached >= 0)
                return use_sideband_colors_cached;
 
-       git_config_get_bool("sideband.allowcontrolcharacters",
-                           &allow_control_characters);
+       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 if (!strcmp(value, "default"))
+                       allow_control_characters = ALLOW_DEFAULT_ANSI_SEQUENCES;
+               else if (!strcmp(value, "color"))
+                       allow_control_characters = ALLOW_ANSI_COLOR_SEQUENCES;
+               else
+                       warning(_("unrecognized value for `sideband."
+                                 "allowControlCharacters`: '%s'"), value);
+               break;
+       default:
+               break; /* not configured */
+       }
 
        if (!git_config_get_string_tmp(key, &value))
                use_sideband_colors_cached = git_config_colorbool(key, value);
@@ -70,9 +93,41 @@ void list_config_color_sideband_slots(struct string_list *list, const char *pref
                list_config_item(list, prefix, keywords[i].keyword);
 }
 
+static int handle_ansi_color_sequence(struct strbuf *dest, const char *src, int n)
+{
+       int i;
+
+       /*
+        * Valid ANSI color sequences are of the form
+        *
+        * ESC [ [<n> [; <n>]*] m
+        *
+        * These are part of the Select Graphic Rendition sequences which
+        * contain more than just color sequences, for more details see
+        * https://en.wikipedia.org/wiki/ANSI_escape_code#SGR.
+        */
+
+       if (allow_control_characters != ALLOW_ANSI_COLOR_SEQUENCES ||
+           n < 3 || src[0] != '\x1b' || src[1] != '[')
+               return 0;
+
+       for (i = 2; i < n; i++) {
+               if (src[i] == 'm') {
+                       strbuf_add(dest, src, i + 1);
+                       return i;
+               }
+               if (!isdigit(src[i]) && src[i] != ';')
+                       break;
+       }
+
+       return 0;
+}
+
 static void strbuf_add_sanitized(struct strbuf *dest, const char *src, int n)
 {
-       if (allow_control_characters) {
+       int i;
+
+       if (allow_control_characters == ALLOW_ALL_CONTROL_CHARACTERS) {
                strbuf_add(dest, src, n);
                return;
        }
@@ -81,7 +136,10 @@ static void strbuf_add_sanitized(struct strbuf *dest, const char *src, int n)
        for (; n && *src; src++, n--) {
                if (!iscntrl(*src) || *src == '\t' || *src == '\n')
                        strbuf_addch(dest, *src);
-               else {
+               else if ((i = handle_ansi_color_sequence(dest, src, n))) {
+                       src += i;
+                       n -= i;
+               } else {
                        strbuf_addch(dest, '^');
                        strbuf_addch(dest, *src == 0x7f ? '?' : 0x40 + *src);
                }

diff --git a/t/t5409-colorize-remote-messages.sh b/t/t5409-colorize-remote-messages.sh
index e8067df591131033f340e37e62f491865a15d87b..f34977b3323845e6ff07755cbeffef17286045de 100755 (executable)
--- a/t/t5409-colorize-remote-messages.sh
+++ b/t/t5409-colorize-remote-messages.sh
@@ -101,7 +101,7 @@ test_expect_success 'fallback to color.ui' '
 
 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?\\n" >&2
+       printf "error: Have you \\033[31mread\\033[m this?\\a\\n" >&2
        exec "$@"
        EOF
        test_config_global uploadPack.packObjectsHook ./color-me-surprised &&
@@ -109,12 +109,24 @@ test_expect_success 'disallow (color) control sequences in sideband' '
 
        git clone --no-local . throw-away 2>stderr &&
        test_decode_color <stderr >decoded &&
+       test_grep RED decoded &&
+       test_grep "\\^G" stderr &&
+       tr -dc "\\007" <stderr >actual &&
+       test_must_be_empty actual &&
+
+       rm -rf throw-away &&
+       git -c sideband.allowControlCharacters=false \
+               clone --no-local . throw-away 2>stderr &&
+       test_decode_color <stderr >decoded &&
        test_grep ! RED decoded &&
+       test_grep "\\^G" stderr &&
 
        rm -rf throw-away &&
        git -c sideband.allowControlCharacters clone --no-local . throw-away 2>stderr &&
        test_decode_color <stderr >decoded &&
-       test_grep RED decoded
+       test_grep RED decoded &&
+       tr -dc "\\007" <stderr >actual &&
+       test_file_not_empty actual
 '
 
 test_done