Skip to content

Commit b8a5299

Browse files
gitstergitster
committed
Merge branch 'jt/redact-all-cookies'
The interface to redact sensitive information in the trace output has been simplified. * jt/redact-all-cookies: http: redact all cookies, teach GIT_TRACE_REDACT=0
2 parents 113f734 + 827e7d4 commit b8a5299

File tree

3 files changed

+39
-43
lines changed

3 files changed

+39
-43
lines changed

‎Documentation/git.txt‎

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -775,11 +775,10 @@ for full details.
775775
See `GIT_TRACE2` for available trace output options and
776776
link:technical/api-trace2.html[Trace2 documentation] for full details.
777777

778-
`GIT_REDACT_COOKIES`::
779-
This can be set to a comma-separated list of strings. When a curl trace
780-
is enabled (see `GIT_TRACE_CURL` above), whenever a "Cookies:" header
781-
sent by the client is dumped, values of cookies whose key is in that
782-
list (case-sensitive) are redacted.
778+
`GIT_TRACE_REDACT`::
779+
By default, when tracing is activated, Git redacts the values of
780+
cookies, the "Authorization:" header, and the "Proxy-Authorization:"
781+
header. Set this variable to `0` to prevent this redaction.
783782

784783
`GIT_LITERAL_PATHSPECS`::
785784
Setting this variable to `1` will cause Git to treat all

‎http.c‎

Lines changed: 11 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@
1818

1919
static struct trace_key trace_curl = TRACE_KEY_INIT(CURL);
2020
static int trace_curl_data = 1;
21-
static struct string_list cookies_to_redact = STRING_LIST_INIT_DUP;
21+
static int trace_curl_redact = 1;
2222
#if LIBCURL_VERSION_NUM >= 0x070a08
2323
long int git_curl_ipresolve = CURL_IPRESOLVE_WHATEVER;
2424
#else
@@ -642,8 +642,9 @@ static void redact_sensitive_header(struct strbuf *header)
642642
{
643643
const char *sensitive_header;
644644

645-
if (skip_prefix(header->buf, "Authorization:", &sensitive_header) ||
646-
skip_prefix(header->buf, "Proxy-Authorization:", &sensitive_header)) {
645+
if (trace_curl_redact &&
646+
(skip_prefix(header->buf, "Authorization:", &sensitive_header) ||
647+
skip_prefix(header->buf, "Proxy-Authorization:", &sensitive_header))) {
647648
/* The first token is the type, which is OK to log */
648649
while (isspace(*sensitive_header))
649650
sensitive_header++;
@@ -652,20 +653,15 @@ static void redact_sensitive_header(struct strbuf *header)
652653
/* Everything else is opaque and possibly sensitive */
653654
strbuf_setlen(header, sensitive_header - header->buf);
654655
strbuf_addstr(header, " <redacted>");
655-
} else if (cookies_to_redact.nr &&
656+
} else if (trace_curl_redact &&
656657
skip_prefix(header->buf, "Cookie:", &sensitive_header)) {
657658
struct strbuf redacted_header = STRBUF_INIT;
658-
char *cookie;
659+
const char *cookie;
659660

660661
while (isspace(*sensitive_header))
661662
sensitive_header++;
662663

663-
/*
664-
* The contents of header starting from sensitive_header will
665-
* subsequently be overridden, so it is fine to mutate this
666-
* string (hence the assignment to "char *").
667-
*/
668-
cookie = (char *) sensitive_header;
664+
cookie = sensitive_header;
669665

670666
while (cookie) {
671667
char *equals;
@@ -678,14 +674,8 @@ static void redact_sensitive_header(struct strbuf *header)
678674
strbuf_addstr(&redacted_header, cookie);
679675
continue;
680676
}
681-
*equals = 0; /* temporarily set to NUL for lookup */
682-
if (string_list_lookup(&cookies_to_redact, cookie)) {
683-
strbuf_addstr(&redacted_header, cookie);
684-
strbuf_addstr(&redacted_header, "=<redacted>");
685-
} else {
686-
*equals = '=';
687-
strbuf_addstr(&redacted_header, cookie);
688-
}
677+
strbuf_add(&redacted_header, cookie, equals - cookie);
678+
strbuf_addstr(&redacted_header, "=<redacted>");
689679
if (semicolon) {
690680
/*
691681
* There are more cookies. (Or, for some
@@ -1003,11 +993,8 @@ static CURL *get_curl_handle(void)
1003993
setup_curl_trace(result);
1004994
if (getenv("GIT_TRACE_CURL_NO_DATA"))
1005995
trace_curl_data = 0;
1006-
if (getenv("GIT_REDACT_COOKIES")) {
1007-
string_list_split(&cookies_to_redact,
1008-
getenv("GIT_REDACT_COOKIES"), ',', -1);
1009-
string_list_sort(&cookies_to_redact);
1010-
}
996+
if (!git_env_bool("GIT_TRACE_REDACT", 1))
997+
trace_curl_redact = 0;
1011998

1012999
curl_easy_setopt(result, CURLOPT_USERAGENT,
10131000
user_agent ? user_agent : git_user_agent());

‎t/t5551-http-fetch-smart.sh‎

Lines changed: 24 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -209,6 +209,16 @@ test_expect_success 'GIT_CURL_VERBOSE redacts auth details' '
209209
grep "Authorization: Basic <redacted>" trace
210210
'
211211

212+
test_expect_success 'GIT_TRACE_CURL does not redact auth details if GIT_TRACE_REDACT=0' '
213+
rm -rf redact-auth trace &&
214+
set_askpass user@host pass@host &&
215+
GIT_TRACE_REDACT=0 GIT_TRACE_CURL="$(pwd)/trace" \
216+
git clone --bare "$HTTPD_URL/auth/smart/repo.git" redact-auth &&
217+
expect_askpass both user@host &&
218+
219+
grep "Authorization: Basic [0-9a-zA-Z+/]" trace
220+
'
221+
212222
test_expect_success 'disable dumb http on server' '
213223
git --git-dir="$HTTPD_DOCUMENT_ROOT_PATH/repo.git" \
214224
config http.getanyfile false
@@ -454,37 +464,37 @@ test_expect_success 'fetch by SHA-1 without tag following' '
454464
--no-tags origin $(cat bar_hash)
455465
'
456466

457-
test_expect_success 'GIT_REDACT_COOKIES redacts cookies' '
467+
test_expect_success 'cookies are redacted by default' '
458468
rm -rf clone &&
459469
echo "Set-Cookie: Foo=1" >cookies &&
460470
echo "Set-Cookie: Bar=2" >>cookies &&
461-
GIT_TRACE_CURL=true GIT_REDACT_COOKIES=Bar,Baz \
471+
GIT_TRACE_CURL=true \
462472
git -c "http.cookieFile=$(pwd)/cookies" clone \
463473
$HTTPD_URL/smart/repo.git clone 2>err &&
464-
grep "Cookie:.*Foo=1" err &&
474+
grep "Cookie:.*Foo=<redacted>" err &&
465475
grep "Cookie:.*Bar=<redacted>" err &&
476+
! grep "Cookie:.*Foo=1" err &&
466477
! grep "Cookie:.*Bar=2" err
467478
'
468479

469-
test_expect_success 'GIT_REDACT_COOKIES redacts cookies when GIT_CURL_VERBOSE=1' '
480+
test_expect_success 'empty values of cookies are also redacted' '
470481
rm -rf clone &&
471-
echo "Set-Cookie: Foo=1" >cookies &&
472-
echo "Set-Cookie: Bar=2" >>cookies &&
473-
GIT_CURL_VERBOSE=1 GIT_REDACT_COOKIES=Bar,Baz \
482+
echo "Set-Cookie: Foo=" >cookies &&
483+
GIT_TRACE_CURL=true \
474484
git -c "http.cookieFile=$(pwd)/cookies" clone \
475485
$HTTPD_URL/smart/repo.git clone 2>err &&
476-
grep "Cookie:.*Foo=1" err &&
477-
grep "Cookie:.*Bar=<redacted>" err &&
478-
! grep "Cookie:.*Bar=2" err
486+
grep "Cookie:.*Foo=<redacted>" err
479487
'
480488

481-
test_expect_success 'GIT_REDACT_COOKIES handles empty values' '
489+
test_expect_success 'GIT_TRACE_REDACT=0 disables cookie redaction' '
482490
rm -rf clone &&
483-
echo "Set-Cookie: Foo=" >cookies &&
484-
GIT_TRACE_CURL=true GIT_REDACT_COOKIES=Foo \
491+
echo "Set-Cookie: Foo=1" >cookies &&
492+
echo "Set-Cookie: Bar=2" >>cookies &&
493+
GIT_TRACE_REDACT=0 GIT_TRACE_CURL=true \
485494
git -c "http.cookieFile=$(pwd)/cookies" clone \
486495
$HTTPD_URL/smart/repo.git clone 2>err &&
487-
grep "Cookie:.*Foo=<redacted>" err
496+
grep "Cookie:.*Foo=1" err &&
497+
grep "Cookie:.*Bar=2" err
488498
'
489499

490500
test_expect_success 'GIT_TRACE_CURL_NO_DATA prevents data from being traced' '

0 commit comments

Comments
 (0)