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>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/Documentation/config/sideband.adoc b/Documentation/config/sideband.adoc
index 2bf04262840b020e9a46283eb0a0796527bb8a6c..32088bbf2f0a40e2a6b926e07df2e748067c1082 100644 (file)
--- a/Documentation/config/sideband.adoc
+++ b/Documentation/config/sideband.adoc
@@ -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 0b420ca3193888f4e1acd5d8f4f0a2bc887243ca..a90db9e2880cba40a1ac071cfe115ac06888b75b 100644 (file)
--- a/sideband.c
+++ b/sideband.c
@@ -10,6 +10,7 @@
 #include "help.h"
 #include "pkt-line.h"
 #include "write-or-die.h"
+#include "urlmatch.h"
 
 struct keyword_entry {
        /*
@@ -27,13 +28,14 @@ static struct keyword_entry keywords[] = {
 };
 
 static enum {
-       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_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_CONTROL_SEQUENCES_UNSET;
 
 static inline int skip_prefix_in_csv(const char *value, const char *prefix,
                                     const char **out)
@@ -45,8 +47,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))
@@ -62,9 +75,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);
+       repo_config(the_repository, 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). */
@@ -80,20 +121,12 @@ static enum git_colorbool use_sideband_colors(void)
        if (use_sideband_colors_cached != GIT_COLOR_UNKNOWN)
                return use_sideband_colors_cached;
 
-       switch (repo_config_get_maybe_bool(the_repository, "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 (repo_config_get_string_tmp(the_repository, "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 (!repo_config_get_value(the_repository, "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 (!repo_config_get_string_tmp(the_repository, key, &value))

diff --git a/sideband.h b/sideband.h
index 5a25331be55d30dbc6b4d777da3cf015118dcf3d..d15fa4015fa0a38f731291da4e13872dd3f7ab30 100644 (file)
--- 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 896e790bf955cd1bccf1a253349ac826a23df4b2..3010913bb113e4fe37722aa2f1dab6211a3bf11d 100755 (executable)
--- a/t/t5409-colorize-remote-messages.sh
+++ b/t/t5409-colorize-remote-messages.sh
@@ -166,4 +166,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 c7f06a7382e605a82316747836300446e5ce74c9..1602065953a54e2c2c42544b60d87609c153303e 100644 (file)
--- a/transport.c
+++ b/transport.c
@@ -29,6 +29,7 @@
 #include "object-name.h"
 #include "color.h"
 #include "bundle-uri.h"
+#include "sideband.h"
 
 static enum git_colorbool transport_use_color = GIT_COLOR_UNKNOWN;
 static char transport_colors[][COLOR_MAXLEN] = {
@@ -1245,6 +1246,8 @@ struct transport *transport_get(struct remote *remote, const char *url)
 
        ret->hash_algo = &hash_algos[GIT_HASH_SHA1_LEGACY];
 
+       sideband_apply_url_config(ret->url);
+
        return ret;
 }