Skip to content

Commit 2df4c93

Browse files
gitstergitster
committed
Merge branch 'jc/neuter-sideband-fixup' into jch
Try to resurrect and reboot a stalled "avoid sending risky escape sequences taken from sideband" topic. Comments? * jc/neuter-sideband-fixup: sideband: conditional documentation fix sideband: delay sanitizing by default to Git v3.0 sideband: drop 'default' configuration sideband: offer to configure sanitizing on a per-URL basis sideband: add options to allow more control sequences to be passed through sideband: do allow ANSI color sequences by default sideband: introduce an "escape hatch" to allow control characters sideband: mask control characters
2 parents bd631b5 + 9754b0a commit 2df4c93

6 files changed

Lines changed: 335 additions & 2 deletions

File tree

‎Documentation/config.adoc‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -523,6 +523,8 @@ include::config/sequencer.adoc[]
523523

524524
include::config/showbranch.adoc[]
525525

526+
include::config/sideband.adoc[]
527+
526528
include::config/sparse.adoc[]
527529

528530
include::config/splitindex.adoc[]

‎Documentation/config/sideband.adoc‎

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
sideband.allowControlCharacters::
2+
ifdef::with-breaking-changes[]
3+
By default, control characters that are delivered via the sideband
4+
are masked, except ANSI color sequences. This prevents potentially
5+
unwanted ANSI escape sequences from being sent to the terminal.
6+
endif::with-breaking-changes[]
7+
ifndef::with-breaking-changes[]
8+
By default, no control characters delivered via the sideband
9+
are masked. This is unsafe and will change in Git v3.* to only
10+
allow ANSI color sequences by default, preventing potentially
11+
unwanted ANSI escape sequences from being sent to the terminal.
12+
endif::with-breaking-changes[]
13+
Use this config setting to override this behavior (the value can be
14+
a comma-separated list of the following keywords):
15+
+
16+
--
17+
`color`::
18+
Allow ANSI color sequences, line feeds and horizontal tabs,
19+
but mask all other control characters. This is the default.
20+
`cursor:`:
21+
Allow control sequences that move the cursor. This is
22+
disabled by default.
23+
`erase`::
24+
Allow control sequences that erase charactrs. This is
25+
disabled by default.
26+
`false`::
27+
Mask all control characters other than line feeds and
28+
horizontal tabs.
29+
`true`::
30+
Allow all control characters to be sent to the terminal.
31+
--
32+
33+
sideband.<url>.*::
34+
Apply the `sideband.*` option selectively to specific URLs. The
35+
same URL matching logic applies as for `http.<url>.*` settings.

‎sideband.c‎

Lines changed: 181 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
#include "help.h"
1111
#include "pkt-line.h"
1212
#include "write-or-die.h"
13+
#include "urlmatch.h"
1314

1415
struct keyword_entry {
1516
/*
@@ -26,6 +27,89 @@ static struct keyword_entry keywords[] = {
2627
{ "error", GIT_COLOR_BOLD_RED },
2728
};
2829

30+
static enum {
31+
ALLOW_CONTROL_SEQUENCES_UNSET = -1,
32+
ALLOW_NO_CONTROL_CHARACTERS = 0,
33+
ALLOW_ANSI_COLOR_SEQUENCES = 1<<0,
34+
ALLOW_ANSI_CURSOR_MOVEMENTS = 1<<1,
35+
ALLOW_ANSI_ERASE = 1<<2,
36+
ALLOW_ALL_CONTROL_CHARACTERS = 1<<3,
37+
#ifdef WITH_BREAKING_CHANGES
38+
ALLOW_DEFAULT_ANSI_SEQUENCES = ALLOW_ANSI_COLOR_SEQUENCES,
39+
#else
40+
ALLOW_DEFAULT_ANSI_SEQUENCES = ALLOW_ALL_CONTROL_CHARACTERS,
41+
#endif
42+
} allow_control_characters = ALLOW_CONTROL_SEQUENCES_UNSET;
43+
44+
static inline int skip_prefix_in_csv(const char *value, const char *prefix,
45+
const char **out)
46+
{
47+
if (!skip_prefix(value, prefix, &value) ||
48+
(*value && *value != ','))
49+
return 0;
50+
*out = value + !!*value;
51+
return 1;
52+
}
53+
54+
int sideband_allow_control_characters_config(const char *var, const char *value)
55+
{
56+
switch (git_parse_maybe_bool(value)) {
57+
case 0:
58+
allow_control_characters = ALLOW_NO_CONTROL_CHARACTERS;
59+
return 0;
60+
case 1:
61+
allow_control_characters = ALLOW_ALL_CONTROL_CHARACTERS;
62+
return 0;
63+
default:
64+
break;
65+
}
66+
67+
allow_control_characters = ALLOW_NO_CONTROL_CHARACTERS;
68+
while (*value) {
69+
if (skip_prefix_in_csv(value, "color", &value))
70+
allow_control_characters |= ALLOW_ANSI_COLOR_SEQUENCES;
71+
else if (skip_prefix_in_csv(value, "cursor", &value))
72+
allow_control_characters |= ALLOW_ANSI_CURSOR_MOVEMENTS;
73+
else if (skip_prefix_in_csv(value, "erase", &value))
74+
allow_control_characters |= ALLOW_ANSI_ERASE;
75+
else if (skip_prefix_in_csv(value, "true", &value))
76+
allow_control_characters = ALLOW_ALL_CONTROL_CHARACTERS;
77+
else if (skip_prefix_in_csv(value, "false", &value))
78+
allow_control_characters = ALLOW_NO_CONTROL_CHARACTERS;
79+
else
80+
warning(_("unrecognized value for '%s': '%s'"), var, value);
81+
}
82+
return 0;
83+
}
84+
85+
static int sideband_config_callback(const char *var, const char *value,
86+
const struct config_context *ctx UNUSED,
87+
void *data UNUSED)
88+
{
89+
if (!strcmp(var, "sideband.allowcontrolcharacters"))
90+
return sideband_allow_control_characters_config(var, value);
91+
92+
return 0;
93+
}
94+
95+
void sideband_apply_url_config(const char *url)
96+
{
97+
struct urlmatch_config config = URLMATCH_CONFIG_INIT;
98+
char *normalized_url;
99+
100+
if (!url)
101+
BUG("must not call sideband_apply_url_config(NULL)");
102+
103+
config.section = "sideband";
104+
config.collect_fn = sideband_config_callback;
105+
106+
normalized_url = url_normalize(url, &config.url);
107+
repo_config(the_repository, urlmatch_config_entry, &config);
108+
free(normalized_url);
109+
string_list_clear(&config.vars, 1);
110+
urlmatch_config_release(&config);
111+
}
112+
29113
/* Returns a color setting (GIT_COLOR_NEVER, etc). */
30114
static enum git_colorbool use_sideband_colors(void)
31115
{
@@ -39,6 +123,14 @@ static enum git_colorbool use_sideband_colors(void)
39123
if (use_sideband_colors_cached != GIT_COLOR_UNKNOWN)
40124
return use_sideband_colors_cached;
41125

126+
if (allow_control_characters == ALLOW_CONTROL_SEQUENCES_UNSET) {
127+
if (!repo_config_get_value(the_repository, "sideband.allowcontrolcharacters", &value))
128+
sideband_allow_control_characters_config("sideband.allowcontrolcharacters", value);
129+
130+
if (allow_control_characters == ALLOW_CONTROL_SEQUENCES_UNSET)
131+
allow_control_characters = ALLOW_DEFAULT_ANSI_SEQUENCES;
132+
}
133+
42134
if (!repo_config_get_string_tmp(the_repository, key, &value))
43135
use_sideband_colors_cached = git_config_colorbool(key, value);
44136
else if (!repo_config_get_string_tmp(the_repository, "color.ui", &value))
@@ -66,6 +158,93 @@ void list_config_color_sideband_slots(struct string_list *list, const char *pref
66158
list_config_item(list, prefix, keywords[i].keyword);
67159
}
68160

161+
static int handle_ansi_sequence(struct strbuf *dest, const char *src, int n)
162+
{
163+
int i;
164+
165+
/*
166+
* Valid ANSI color sequences are of the form
167+
*
168+
* ESC [ [<n> [; <n>]*] m
169+
*
170+
* These are part of the Select Graphic Rendition sequences which
171+
* contain more than just color sequences, for more details see
172+
* https://en.wikipedia.org/wiki/ANSI_escape_code#SGR.
173+
*
174+
* The cursor movement sequences are:
175+
*
176+
* ESC [ n A - Cursor up n lines (CUU)
177+
* ESC [ n B - Cursor down n lines (CUD)
178+
* ESC [ n C - Cursor forward n columns (CUF)
179+
* ESC [ n D - Cursor back n columns (CUB)
180+
* ESC [ n E - Cursor next line, beginning (CNL)
181+
* ESC [ n F - Cursor previous line, beginning (CPL)
182+
* ESC [ n G - Cursor to column n (CHA)
183+
* ESC [ n ; m H - Cursor position (row n, col m) (CUP)
184+
* ESC [ n ; m f - Same as H (HVP)
185+
*
186+
* The sequences to erase characters are:
187+
*
188+
*
189+
* ESC [ 0 J - Clear from cursor to end of screen (ED)
190+
* ESC [ 1 J - Clear from cursor to beginning of screen (ED)
191+
* ESC [ 2 J - Clear entire screen (ED)
192+
* ESC [ 3 J - Clear entire screen + scrollback (ED) - xterm extension
193+
* ESC [ 0 K - Clear from cursor to end of line (EL)
194+
* ESC [ 1 K - Clear from cursor to beginning of line (EL)
195+
* ESC [ 2 K - Clear entire line (EL)
196+
* ESC [ n M - Delete n lines (DL)
197+
* ESC [ n P - Delete n characters (DCH)
198+
* ESC [ n X - Erase n characters (ECH)
199+
*
200+
* For a comprehensive list of common ANSI Escape sequences, see
201+
* https://www.xfree86.org/current/ctlseqs.html
202+
*/
203+
204+
if (n < 3 || src[0] != '\x1b' || src[1] != '[')
205+
return 0;
206+
207+
for (i = 2; i < n; i++) {
208+
if (((allow_control_characters & ALLOW_ANSI_COLOR_SEQUENCES) &&
209+
src[i] == 'm') ||
210+
((allow_control_characters & ALLOW_ANSI_CURSOR_MOVEMENTS) &&
211+
strchr("ABCDEFGHf", src[i])) ||
212+
((allow_control_characters & ALLOW_ANSI_ERASE) &&
213+
strchr("JKMPX", src[i]))) {
214+
strbuf_add(dest, src, i + 1);
215+
return i;
216+
}
217+
if (!isdigit(src[i]) && src[i] != ';')
218+
break;
219+
}
220+
221+
return 0;
222+
}
223+
224+
static void strbuf_add_sanitized(struct strbuf *dest, const char *src, int n)
225+
{
226+
int i;
227+
228+
if ((allow_control_characters & ALLOW_ALL_CONTROL_CHARACTERS)) {
229+
strbuf_add(dest, src, n);
230+
return;
231+
}
232+
233+
strbuf_grow(dest, n);
234+
for (; n && *src; src++, n--) {
235+
if (!iscntrl(*src) || *src == '\t' || *src == '\n') {
236+
strbuf_addch(dest, *src);
237+
} else if (allow_control_characters != ALLOW_NO_CONTROL_CHARACTERS &&
238+
(i = handle_ansi_sequence(dest, src, n))) {
239+
src += i;
240+
n -= i;
241+
} else {
242+
strbuf_addch(dest, '^');
243+
strbuf_addch(dest, *src == 0x7f ? '?' : 0x40 + *src);
244+
}
245+
}
246+
}
247+
69248
/*
70249
* Optionally highlight one keyword in remote output if it appears at the start
71250
* of the line. This should be called for a single line only, which is
@@ -81,7 +260,7 @@ static void maybe_colorize_sideband(struct strbuf *dest, const char *src, int n)
81260
int i;
82261

83262
if (!want_color_stderr(use_sideband_colors())) {
84-
strbuf_add(dest, src, n);
263+
strbuf_add_sanitized(dest, src, n);
85264
return;
86265
}
87266

@@ -114,7 +293,7 @@ static void maybe_colorize_sideband(struct strbuf *dest, const char *src, int n)
114293
}
115294
}
116295

117-
strbuf_add(dest, src, n);
296+
strbuf_add_sanitized(dest, src, n);
118297
}
119298

120299

‎sideband.h‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,4 +30,18 @@ int demultiplex_sideband(const char *me, int status,
3030

3131
void send_sideband(int fd, int band, const char *data, ssize_t sz, int packet_max);
3232

33+
/*
34+
* Apply sideband configuration for the given URL. This should be called
35+
* when a transport is created to allow URL-specific configuration of
36+
* sideband behavior (e.g., sideband.<url>.allowControlCharacters).
37+
*/
38+
void sideband_apply_url_config(const char *url);
39+
40+
/*
41+
* Parse and set the sideband allow control characters configuration.
42+
* The var parameter should be the key name (without section prefix).
43+
* Returns 0 if the variable was recognized and handled, non-zero otherwise.
44+
*/
45+
int sideband_allow_control_characters_config(const char *var, const char *value);
46+
3347
#endif

‎t/t5409-colorize-remote-messages.sh‎

Lines changed: 100 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -98,4 +98,104 @@ test_expect_success 'fallback to color.ui' '
9898
grep "<BOLD;RED>error<RESET>: error" decoded
9999
'
100100

101+
if test_have_prereq WITH_BREAKING_CHANGES
102+
then
103+
TURN_ON_SANITIZING=already.turned=on
104+
else
105+
TURN_ON_SANITIZING=sideband.allowControlCharacters=color
106+
fi
107+
108+
test_expect_success 'disallow (color) control sequences in sideband' '
109+
write_script .git/color-me-surprised <<-\EOF &&
110+
printf "error: Have you \\033[31mread\\033[m this?\\a\\n" >&2
111+
exec "$@"
112+
EOF
113+
test_config_global uploadPack.packObjectsHook ./color-me-surprised &&
114+
test_commit need-at-least-one-commit &&
115+
116+
git -c $TURN_ON_SANITIZING clone --no-local . throw-away 2>stderr &&
117+
test_decode_color <stderr >decoded &&
118+
test_grep RED decoded &&
119+
test_grep "\\^G" stderr &&
120+
tr -dc "\\007" <stderr >actual &&
121+
test_must_be_empty actual &&
122+
123+
rm -rf throw-away &&
124+
git -c sideband.allowControlCharacters=false \
125+
clone --no-local . throw-away 2>stderr &&
126+
test_decode_color <stderr >decoded &&
127+
test_grep ! RED decoded &&
128+
test_grep "\\^G" stderr &&
129+
130+
rm -rf throw-away &&
131+
git -c sideband.allowControlCharacters clone --no-local . throw-away 2>stderr &&
132+
test_decode_color <stderr >decoded &&
133+
test_grep RED decoded &&
134+
tr -dc "\\007" <stderr >actual &&
135+
test_file_not_empty actual
136+
'
137+
138+
test_decode_csi() {
139+
awk '{
140+
while (match($0, /\033/) != 0) {
141+
printf "%sCSI ", substr($0, 1, RSTART-1);
142+
$0 = substr($0, RSTART + RLENGTH, length($0) - RSTART - RLENGTH + 1);
143+
}
144+
print
145+
}'
146+
}
147+
148+
test_expect_success 'control sequences in sideband allowed by default (in Git v3.8)' '
149+
write_script .git/color-me-surprised <<-\EOF &&
150+
printf "error: \\033[31mcolor\\033[m\\033[Goverwrite\\033[Gerase\\033[K\\033?25l\\n" >&2
151+
exec "$@"
152+
EOF
153+
test_config_global uploadPack.packObjectsHook ./color-me-surprised &&
154+
test_commit need-at-least-one-commit-at-least &&
155+
156+
rm -rf throw-away &&
157+
git -c $TURN_ON_SANITIZING clone --no-local . throw-away 2>stderr &&
158+
test_decode_color <stderr >color-decoded &&
159+
test_decode_csi <color-decoded >decoded &&
160+
test_grep ! "CSI \\[K" decoded &&
161+
test_grep ! "CSI \\[G" decoded &&
162+
test_grep "\\^\\[?25l" decoded &&
163+
164+
rm -rf throw-away &&
165+
git -c sideband.allowControlCharacters=erase,cursor,color \
166+
clone --no-local . throw-away 2>stderr &&
167+
test_decode_color <stderr >color-decoded &&
168+
test_decode_csi <color-decoded >decoded &&
169+
test_grep "RED" decoded &&
170+
test_grep "CSI \\[K" decoded &&
171+
test_grep "CSI \\[G" decoded &&
172+
test_grep ! "\\^\\[\\[K" decoded &&
173+
test_grep ! "\\^\\[\\[G" decoded
174+
'
175+
176+
test_expect_success 'allow all control sequences for a specific URL' '
177+
write_script .git/eraser <<-\EOF &&
178+
printf "error: Ohai!\\r\\033[K" >&2
179+
exec "$@"
180+
EOF
181+
test_config_global uploadPack.packObjectsHook ./eraser &&
182+
test_commit one-more-please &&
183+
184+
rm -rf throw-away &&
185+
git -c $TURN_ON_SANITIZING clone --no-local . throw-away 2>stderr &&
186+
test_decode_color <stderr >color-decoded &&
187+
test_decode_csi <color-decoded >decoded &&
188+
test_grep ! "CSI \\[K" decoded &&
189+
test_grep "\\^\\[\\[K" decoded &&
190+
191+
rm -rf throw-away &&
192+
git -c sideband.allowControlCharacters=false \
193+
-c "sideband.file://.allowControlCharacters=true" \
194+
clone --no-local "file://$PWD" throw-away 2>stderr &&
195+
test_decode_color <stderr >color-decoded &&
196+
test_decode_csi <color-decoded >decoded &&
197+
test_grep "CSI \\[K" decoded &&
198+
test_grep ! "\\^\\[\\[K" decoded
199+
'
200+
101201
test_done

0 commit comments

Comments
 (0)