From patchwork Wed Nov 22 19:18:34 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jeff Hostetler X-Patchwork-Id: 13465399 Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b="O40JWDG3" Received: from mail-wr1-x42b.google.com (mail-wr1-x42b.google.com [IPv6:2a00:1450:4864:20::42b]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 4D98710C for ; Wed, 22 Nov 2023 11:18:41 -0800 (PST) Received: by mail-wr1-x42b.google.com with SMTP id ffacd0b85a97d-32d81864e3fso39697f8f.2 for ; Wed, 22 Nov 2023 11:18:41 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1700680719; x=1701285519; darn=vger.kernel.org; h=cc:to:mime-version:content-transfer-encoding:fcc:subject:date:from :references:in-reply-to:message-id:from:to:cc:subject:date :message-id:reply-to; bh=rfUGHqGmJVI7mFma8+kdCMSVrhbvh3TgjSJ7W16vJpg=; b=O40JWDG3tSvNrsn+zg8fn1Nf10wRprE0MwvrIrOMDDG1sLm4nZLS50RjIQFpWO9naM vzxK58BU3DF/Y4evX+NW+U3CvrAuh+3DtQso2dBlsLwIGBVxdYCXXW5wyQDvmBb5BiPm igw/t8uNT5VZvtVze94MeHAHKUyyT7jFXZe/AN4ZP6M+Uy+RINvp05zj+101Jp17nWER PXP9sTzLNOq2I8F6PXVLRTqKhQz/khdzQVHhswapw+yRbLdcQgbOAT0RoqWjIEVyOLJX p9XokdeO8J7jMCgVmcPlwJ8nx7HBMEQPA+3N5nv5hbIR9rLD9zPpiK94PDH2hXPblGuB EfWQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1700680719; x=1701285519; h=cc:to:mime-version:content-transfer-encoding:fcc:subject:date:from :references:in-reply-to:message-id:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=rfUGHqGmJVI7mFma8+kdCMSVrhbvh3TgjSJ7W16vJpg=; b=r8v/9wkr14mOLIqd5q6BCNIUwWhb2eXzCxnfB8kmbbx5j/nYd9Dx1fl/1uczu+Xchv 7YQmH9hssSi1A5ksuviZ6JKo/xZ8shy9BzTK7GZ/nmhisHSQBcbjsn5GVJLKfx8Ul5Bh 0L1YcpmLWadGiApc4ZSnXm6jBQamcE0K7pHYEP7VPMuL/JXkceSOK9P7NEM+7gwGUenR cXGTcV1zEyApGQrW5oY1/TpxlUcSr37Hx3uO3kvzw9dABwjQyUxB5P4UjvNO4pjAoJ9r p+x4wMId0Gj//TjhVn6zxRRbLkmX0PHRNDK5GQsT6LNxoxIWLHxLXWQPpAtyOYqBRx9k +aUw== X-Gm-Message-State: AOJu0YxLy2wdv/7VRdpWn5J6GVG+v4ieWhTHZDTrgJuIGMn5IWa5HVc3 onbFpS88mI1sFpiUYSwpPlXf8h8EhMY= X-Google-Smtp-Source: AGHT+IEpWSbp0Fyt9Ia9IvwNwWPxBjQzRHXrqP2OBTs10zr1E2g8RGH+oZJ1tqNdkhAR0dbQtL5vSg== X-Received: by 2002:a05:6000:112:b0:331:6961:6cc7 with SMTP id o18-20020a056000011200b0033169616cc7mr2427953wrx.25.1700680718909; Wed, 22 Nov 2023 11:18:38 -0800 (PST) Received: from [127.0.0.1] ([13.74.141.28]) by smtp.gmail.com with ESMTPSA id g3-20020a5d46c3000000b00323287186aasm142471wrs.32.2023.11.22.11.18.38 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 22 Nov 2023 11:18:38 -0800 (PST) Message-ID: <97d17c22ff310c26c3ec391c7bf870e7e5bab4f8.1700680717.git.gitgitgadget@gmail.com> In-Reply-To: References: Date: Wed, 22 Nov 2023 19:18:34 +0000 Subject: [PATCH 1/4] trace2: fix signature of trace2_def_param() macro Fcc: Sent Precedence: bulk X-Mailing-List: git@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 To: git@vger.kernel.org Cc: Johannes Schindelin , Jeff Hostetler From: Jeff Hostetler From: Jeff Hostetler Add `struct key_value_info` argument to `trace2_def_param()`. In dc90208497 (trace2: plumb config kvi, 2023-06-28) a `kvi` argument was added to `trace2_def_param_fl()` but the macro was not up updated. Let's fix that. Signed-off-by: Jeff Hostetler --- trace2.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/trace2.h b/trace2.h index 40d8c2e02a5..1f0669bbd2d 100644 --- a/trace2.h +++ b/trace2.h @@ -337,8 +337,8 @@ struct key_value_info; void trace2_def_param_fl(const char *file, int line, const char *param, const char *value, const struct key_value_info *kvi); -#define trace2_def_param(param, value) \ - trace2_def_param_fl(__FILE__, __LINE__, (param), (value)) +#define trace2_def_param(param, value, kvi) \ + trace2_def_param_fl(__FILE__, __LINE__, (param), (value), (kvi)) /* * Tell trace2 about a newly instantiated repo object and assign From patchwork Wed Nov 22 19:18:35 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Johannes Schindelin X-Patchwork-Id: 13465401 Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b="EEoVChuR" Received: from mail-wm1-x336.google.com (mail-wm1-x336.google.com [IPv6:2a00:1450:4864:20::336]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 429F1112 for ; Wed, 22 Nov 2023 11:18:42 -0800 (PST) Received: by mail-wm1-x336.google.com with SMTP id 5b1f17b1804b1-4083f61322fso613305e9.1 for ; Wed, 22 Nov 2023 11:18:42 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1700680720; x=1701285520; darn=vger.kernel.org; h=cc:to:mime-version:content-transfer-encoding:fcc:subject:date:from :references:in-reply-to:message-id:from:to:cc:subject:date :message-id:reply-to; bh=HiLoCVeVr0figxewBdYEzSxmqCMtnQLP/5tvEUvlXWQ=; b=EEoVChuR61hbQKdPayEI+J/cXn7jRtDopUoK8jZiHj53DdUYjQSaxAzLAAk0xhWDgY SFU8cj4d1ErBeBeO5IcqhzBYOkjMN7W321P+PjH17ezjlgqPvIVy4vgIlmOrsePI1DSJ Zg6fKQLGtBHB1r0u/Tdx9bIMFTtaLcrhbqKJgS97LKo1USvGQ3yQp+K3P3ZlwrlgCKJy KuVup23/o0DOr7KbEy3qf+KxH19feOPv3VXzDemx4NXKLyzAGM9Fi7AYWyzEKW21T4LB Vo+RGOmiuTp1m2CSNhIIJowSC6xqrvmHE2DFMX3FHwVrQhDlWoTOQ0SJwR/Hg1zKDOEt aAcQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1700680720; x=1701285520; h=cc:to:mime-version:content-transfer-encoding:fcc:subject:date:from :references:in-reply-to:message-id:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=HiLoCVeVr0figxewBdYEzSxmqCMtnQLP/5tvEUvlXWQ=; b=Stl4ZrOapTdIWgc+NbeoyeKu2IP4ojtFU8B7AN5cZjOs7pM5Sj7a2Gu/uYW4GnMGhJ lz8OEeaSDff+5BqraFKBuJ4gKwd29qdiF9+teoiXUlQd2JNq5iX5pmuI3cAeusk+JQxR L5DpszaqS8IOXuB6xy4jO9qKLhIoiwK+tM+aK0wINpg9LT2yzVHLaJrBnb4CMK49+lF3 kxRlzFf+PVEGx2bTdBz0VDvQdqfdkSK4+XHv83nNDOuPSpgHNavZAmeBLHl2wUWk5tQv jaAKjII897FeL6rxVz3tPfv9X2tulAWO5N4pQBUngZZ0H4GnsFme4rLe20z9LYKA0dDa /N9w== X-Gm-Message-State: AOJu0Yz1yN77eq3uxMQG296uSmb0S/VNXkR0E70sCtQnC2zqlxgGWSk9 gHUYPse1CqWc9VvYBGnhWo2Ms6weZGw= X-Google-Smtp-Source: AGHT+IGaza+S7FwpiKhFKwqj9u5x9qtup1te05Z8+m4hc/raunV4c7G6IcS30wGd9+kjwjw8jWTHGg== X-Received: by 2002:a05:600c:46cd:b0:408:4083:fbf2 with SMTP id q13-20020a05600c46cd00b004084083fbf2mr2718833wmo.22.1700680720221; Wed, 22 Nov 2023 11:18:40 -0800 (PST) Received: from [127.0.0.1] ([13.74.141.28]) by smtp.gmail.com with ESMTPSA id l17-20020a05600c4f1100b004063c9f68f2sm297723wmq.26.2023.11.22.11.18.39 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 22 Nov 2023 11:18:39 -0800 (PST) Message-ID: In-Reply-To: References: Date: Wed, 22 Nov 2023 19:18:35 +0000 Subject: [PATCH 2/4] trace2: redact passwords from https:// URLs by default Fcc: Sent Precedence: bulk X-Mailing-List: git@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 To: git@vger.kernel.org Cc: Johannes Schindelin , Johannes Schindelin From: Johannes Schindelin From: Johannes Schindelin It is an unsafe practice to call something like git clone https://user:password@example.com/ This not only risks leaking the password "over the shoulder" or into the readline history of the current Unix shell, it also gets logged via Trace2 if enabled. Let's at least avoid logging such secrets via Trace2, much like we avoid logging secrets in `http.c`. Much like the code in `http.c` is guarded via `GIT_TRACE_REDACT` (defaulting to `true`), we guard the new code via `GIT_TRACE2_REDACT` (also defaulting to `true`). The new tests added in this commit uncover leaks in `builtin/clone.c` and `remote.c`. Therefore we need to turn off `TEST_PASSES_SANITIZE_LEAK`. The reasons: - We observed that `the_repository->remote_status` is not released properly. - We are using `url...insteadOf` and that runs into a code path where an allocated URL is replaced with another URL, and the original URL is never released. - `remote_states` contains plenty of `struct remote`s whose refspecs seem to be usually allocated by never released. More investigation is needed here to identify the exact cause and proper fixes for these leaks/bugs. Co-authored-by: Jeff Hostetler Signed-off-by: Jeff Hostetler Signed-off-by: Johannes Schindelin --- t/t0210-trace2-normal.sh | 20 ++++++- trace2.c | 120 ++++++++++++++++++++++++++++++++++++++- 2 files changed, 136 insertions(+), 4 deletions(-) diff --git a/t/t0210-trace2-normal.sh b/t/t0210-trace2-normal.sh index 80e76a4695e..c312657a12c 100755 --- a/t/t0210-trace2-normal.sh +++ b/t/t0210-trace2-normal.sh @@ -2,7 +2,7 @@ test_description='test trace2 facility (normal target)' -TEST_PASSES_SANITIZE_LEAK=true +TEST_PASSES_SANITIZE_LEAK=false . ./test-lib.sh # Turn off any inherited trace2 settings for this test. @@ -283,4 +283,22 @@ test_expect_success 'using global config with include' ' test_cmp expect actual ' +test_expect_success 'unsafe URLs are redacted by default' ' + test_when_finished \ + "rm -r trace.normal unredacted.normal clone clone2" && + + test_config_global \ + "url.$(pwd).insteadOf" https://user:pwd@example.com/ && + test_config_global trace2.configParams "core.*,remote.*.url" && + + GIT_TRACE2="$(pwd)/trace.normal" \ + git clone https://user:pwd@example.com/ clone && + ! grep user:pwd trace.normal && + + GIT_TRACE2_REDACT=0 GIT_TRACE2="$(pwd)/unredacted.normal" \ + git clone https://user:pwd@example.com/ clone2 && + grep "start .* clone https://user:pwd@example.com" unredacted.normal && + grep "remote.origin.url=https://user:pwd@example.com" unredacted.normal +' + test_done diff --git a/trace2.c b/trace2.c index 6dc74dff4c7..87d9a3a0361 100644 --- a/trace2.c +++ b/trace2.c @@ -20,6 +20,7 @@ #include "trace2/tr2_tmr.h" static int trace2_enabled; +static int trace2_redact = 1; static int tr2_next_child_id; /* modify under lock */ static int tr2_next_exec_id; /* modify under lock */ @@ -227,6 +228,8 @@ void trace2_initialize_fl(const char *file, int line) if (!tr2_tgt_want_builtins()) return; trace2_enabled = 1; + if (!git_env_bool("GIT_TRACE2_REDACT", 1)) + trace2_redact = 0; tr2_sid_get(); @@ -247,12 +250,93 @@ int trace2_is_enabled(void) return trace2_enabled; } +/* + * Redacts an argument, i.e. ensures that no password in + * https://user:password@host/-style URLs is logged. + * + * Returns the original if nothing needed to be redacted. + * Returns a pointer that needs to be `free()`d otherwise. + */ +static const char *redact_arg(const char *arg) +{ + const char *p, *colon; + size_t at; + + if (!trace2_redact || + (!skip_prefix(arg, "https://", &p) && + !skip_prefix(arg, "http://", &p))) + return arg; + + at = strcspn(p, "@/"); + if (p[at] != '@') + return arg; + + colon = memchr(p, ':', at); + if (!colon) + return arg; + + return xstrfmt("%.*s:%s", (int)(colon - arg), arg, p + at); +} + +/* + * Redacts arguments in an argument list. + * + * Returns the original if nothing needed to be redacted. + * Otherwise, returns a new array that needs to be released + * via `free_redacted_argv()`. + */ +static const char **redact_argv(const char **argv) +{ + int i, j; + const char *redacted = NULL; + const char **ret; + + if (!trace2_redact) + return argv; + + for (i = 0; argv[i]; i++) + if ((redacted = redact_arg(argv[i])) != argv[i]) + break; + + if (!argv[i]) + return argv; + + for (j = 0; argv[j]; j++) + ; /* keep counting */ + + ALLOC_ARRAY(ret, j + 1); + ret[j] = NULL; + + for (j = 0; j < i; j++) + ret[j] = argv[j]; + ret[i] = redacted; + for (++i; argv[i]; i++) { + redacted = redact_arg(argv[i]); + ret[i] = redacted ? redacted : argv[i]; + } + + return ret; +} + +static void free_redacted_argv(const char **redacted, const char **argv) +{ + int i; + + if (redacted != argv) { + for (i = 0; argv[i]; i++) + if (redacted[i] != argv[i]) + free((void *)redacted[i]); + free((void *)redacted); + } +} + void trace2_cmd_start_fl(const char *file, int line, const char **argv) { struct tr2_tgt *tgt_j; int j; uint64_t us_now; uint64_t us_elapsed_absolute; + const char **redacted; if (!trace2_enabled) return; @@ -260,10 +344,14 @@ void trace2_cmd_start_fl(const char *file, int line, const char **argv) us_now = getnanotime() / 1000; us_elapsed_absolute = tr2tls_absolute_elapsed(us_now); + redacted = redact_argv(argv); + for_each_wanted_builtin (j, tgt_j) if (tgt_j->pfn_start_fl) tgt_j->pfn_start_fl(file, line, us_elapsed_absolute, - argv); + redacted); + + free_redacted_argv(redacted, argv); } void trace2_cmd_exit_fl(const char *file, int line, int code) @@ -409,6 +497,7 @@ void trace2_child_start_fl(const char *file, int line, int j; uint64_t us_now; uint64_t us_elapsed_absolute; + const char **orig_argv = cmd->args.v; if (!trace2_enabled) return; @@ -419,10 +508,24 @@ void trace2_child_start_fl(const char *file, int line, cmd->trace2_child_id = tr2tls_locked_increment(&tr2_next_child_id); cmd->trace2_child_us_start = us_now; + /* + * The `pfn_child_start_fl` API takes a `struct child_process` + * rather than a simple `argv` for the child because some + * targets make use of the additional context bits/values. So + * temporarily replace the original argv (inside the `strvec`) + * with a possibly redacted version. + */ + cmd->args.v = redact_argv(orig_argv); + for_each_wanted_builtin (j, tgt_j) if (tgt_j->pfn_child_start_fl) tgt_j->pfn_child_start_fl(file, line, us_elapsed_absolute, cmd); + + if (cmd->args.v != orig_argv) { + free_redacted_argv(cmd->args.v, orig_argv); + cmd->args.v = orig_argv; + } } void trace2_child_exit_fl(const char *file, int line, struct child_process *cmd, @@ -493,6 +596,7 @@ int trace2_exec_fl(const char *file, int line, const char *exe, int exec_id; uint64_t us_now; uint64_t us_elapsed_absolute; + const char **redacted; if (!trace2_enabled) return -1; @@ -502,10 +606,14 @@ int trace2_exec_fl(const char *file, int line, const char *exe, exec_id = tr2tls_locked_increment(&tr2_next_exec_id); + redacted = redact_argv(argv); + for_each_wanted_builtin (j, tgt_j) if (tgt_j->pfn_exec_fl) tgt_j->pfn_exec_fl(file, line, us_elapsed_absolute, - exec_id, exe, argv); + exec_id, exe, redacted); + + free_redacted_argv(redacted, argv); return exec_id; } @@ -637,13 +745,19 @@ void trace2_def_param_fl(const char *file, int line, const char *param, { struct tr2_tgt *tgt_j; int j; + const char *redacted; if (!trace2_enabled) return; + redacted = redact_arg(value); + for_each_wanted_builtin (j, tgt_j) if (tgt_j->pfn_param_fl) - tgt_j->pfn_param_fl(file, line, param, value, kvi); + tgt_j->pfn_param_fl(file, line, param, redacted, kvi); + + if (redacted != value) + free((void *)redacted); } void trace2_def_repo_fl(const char *file, int line, struct repository *repo) From patchwork Wed Nov 22 19:18:36 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jeff Hostetler X-Patchwork-Id: 13465402 Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b="TRIdK3Pu" Received: from mail-wr1-x42a.google.com (mail-wr1-x42a.google.com [IPv6:2a00:1450:4864:20::42a]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 311B3C1 for ; Wed, 22 Nov 2023 11:18:43 -0800 (PST) Received: by mail-wr1-x42a.google.com with SMTP id ffacd0b85a97d-32faea0fa1fso85939f8f.1 for ; Wed, 22 Nov 2023 11:18:43 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1700680721; x=1701285521; darn=vger.kernel.org; h=cc:to:mime-version:content-transfer-encoding:fcc:subject:date:from :references:in-reply-to:message-id:from:to:cc:subject:date :message-id:reply-to; bh=Ot6cmyfDWJ74BZuAkV7cUFae3QAbrLchR33HOSRbevc=; b=TRIdK3Pu3840rOh8g2ssRAOyJk6musAE755tI35R3B3K9Aa+s+TyOumwRfO+LMQ/aR l+PcEOVvI1oHntzPperGlXuKqgb+talpceU1utfM/NtdFxYQzcX2pXgO0gwI+IcfCZAM iKQlrT1L9m0E7i75Wo50ZFIPzcLF+W1Z1MqGpAlSnjvtcC8IEegEzmVaD3kfC24ptuRz boM57siE+7wUUF0fMAHtq58d5rYthn8w4ebYEsJiLbqpNERigBjb6Ajr+1jpDrIlKqpB 2rqMYCnmLbWUCLC/qhy1fyI4a/nIqu7e81LCsjHmi01q5cnhCQ0Z60Fwz+lnSaWqeeiV 28FQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1700680721; x=1701285521; h=cc:to:mime-version:content-transfer-encoding:fcc:subject:date:from :references:in-reply-to:message-id:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=Ot6cmyfDWJ74BZuAkV7cUFae3QAbrLchR33HOSRbevc=; b=Z8CHV0v+9zcL4eS4Lc5G3MkfB4kdtZw+pjGCZGGdAjTpgm6EjNAi3wIBRh4ofUDWOh 8xTtZ5C5ym25Lcd/LRKYlSDIshLev9z9FAs5+1TMha7oiBGmhWKE/Db98zD1FUC7/hyH c7IshxiuEOEBBNCCzW89ZC7SAwFb3eXGFGdbLoiBeyg507hEIeT3UOsALbGOI7Kw0Kwm dD0B1PpihGjWT4tKITY96Pdbf5zR+OyiOXmZKYjh3jkMR6qDl/BGAjjMb7WS+w/mmL7e cD7WFo0qDPHW2Ha8iX3wc+4CSHr1TJIAfDN3FJRDNnqi6gWPsqaehXhpvxtqKvTgUM/C AXxA== X-Gm-Message-State: AOJu0YyTa7/GurHoCVPtZaIc//kDNJmB54C0Joju4PbggSJK16RlbwK9 5VbPfqTy/1vL8+GQ5gR7DjyC1fKaJxI= X-Google-Smtp-Source: AGHT+IH7+JyAEQaBbTyONMowOLvZEaL6NT5wK1n8oTJHqDq+gQmGgTwJvEAErF9S+6EpsAnVJAWxIw== X-Received: by 2002:adf:fd11:0:b0:332:c65a:8f1e with SMTP id e17-20020adffd11000000b00332c65a8f1emr309599wrr.19.1700680720788; Wed, 22 Nov 2023 11:18:40 -0800 (PST) Received: from [127.0.0.1] ([13.74.141.28]) by smtp.gmail.com with ESMTPSA id v15-20020a5d610f000000b00332cc24a59bsm120311wrt.109.2023.11.22.11.18.40 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 22 Nov 2023 11:18:40 -0800 (PST) Message-ID: In-Reply-To: References: Date: Wed, 22 Nov 2023 19:18:36 +0000 Subject: [PATCH 3/4] t0211: test URL redacting in PERF format Fcc: Sent Precedence: bulk X-Mailing-List: git@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 To: git@vger.kernel.org Cc: Johannes Schindelin , Jeff Hostetler From: Jeff Hostetler From: Jeff Hostetler This transmogrifies the test case that was just added to t0210, to also cover the `GIT_TRACE2_PERF` backend. Just like t0211, we now have to toggle the `TEST_PASSES_SANITIZE_LEAK` annotation. Signed-off-by: Jeff Hostetler --- t/t0211-trace2-perf.sh | 21 ++++++++++++++++++++- 1 file changed, 20 insertions(+), 1 deletion(-) diff --git a/t/t0211-trace2-perf.sh b/t/t0211-trace2-perf.sh index cfba6861322..290b6eaaab1 100755 --- a/t/t0211-trace2-perf.sh +++ b/t/t0211-trace2-perf.sh @@ -2,7 +2,7 @@ test_description='test trace2 facility (perf target)' -TEST_PASSES_SANITIZE_LEAK=true +TEST_PASSES_SANITIZE_LEAK=false . ./test-lib.sh # Turn off any inherited trace2 settings for this test. @@ -268,4 +268,23 @@ test_expect_success PTHREADS 'global counter test/test2' ' have_counter_event "main" "counter" "test" "test2" 60 actual ' +test_expect_success 'unsafe URLs are redacted by default' ' + test_when_finished \ + "rm -r actual trace.perf unredacted.perf clone clone2" && + + test_config_global \ + "url.$(pwd).insteadOf" https://user:pwd@example.com/ && + test_config_global trace2.configParams "core.*,remote.*.url" && + + GIT_TRACE2_PERF="$(pwd)/trace.perf" \ + git clone https://user:pwd@example.com/ clone && + ! grep user:pwd trace.perf && + + GIT_TRACE2_REDACT=0 GIT_TRACE2_PERF="$(pwd)/unredacted.perf" \ + git clone https://user:pwd@example.com/ clone2 && + perl "$TEST_DIRECTORY/t0211/scrub_perf.perl" actual && + grep "d0|main|start|.* clone https://user:pwd@example.com" actual && + grep "d0|main|def_param|.*|remote.origin.url:https://user:pwd@example.com" actual +' + test_done From patchwork Wed Nov 22 19:18:37 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jeff Hostetler X-Patchwork-Id: 13465403 Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b="koRMsAZl" Received: from mail-wr1-x42c.google.com (mail-wr1-x42c.google.com [IPv6:2a00:1450:4864:20::42c]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 57B3911F for ; Wed, 22 Nov 2023 11:18:43 -0800 (PST) Received: by mail-wr1-x42c.google.com with SMTP id ffacd0b85a97d-32df66c691dso39153f8f.3 for ; Wed, 22 Nov 2023 11:18:43 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1700680721; x=1701285521; darn=vger.kernel.org; h=cc:to:mime-version:content-transfer-encoding:fcc:subject:date:from :references:in-reply-to:message-id:from:to:cc:subject:date :message-id:reply-to; bh=gwi6GU9IrhbxpVTf408ITlx8/kBTgmpFzT72lxcYK4E=; b=koRMsAZlJTrPfOAPfteoGSUwa80zS7TaOD0W70u6zHLdkK53F1su5th0ABcDej5/3S cI4gWo2hYyh6ctfNvDL2yxNWx58vmZVTroQGlWipfQBe5oQegKYRoi5prAZ5MqSgh++B +5rWNTxusTbCVqvhUpEuxR6D+IvaPs/IuioMktQroiMkVvkooSsmBqpMUoIValRszRHf +OgXMTItrqj8pVruAjpbR/3aNlkylYZyEnNCJ8RFnhWdEijye11HcjOE7cku9DXVHw0e VUWcnu0rbQzAHFUnDGD4dB2Xly+F1JTYt5s27IkGG95rcIsVzS6Um9Epja0te7h7aP6E WrkQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1700680721; x=1701285521; h=cc:to:mime-version:content-transfer-encoding:fcc:subject:date:from :references:in-reply-to:message-id:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=gwi6GU9IrhbxpVTf408ITlx8/kBTgmpFzT72lxcYK4E=; b=OMdFa50xRY42uzg9UQCxiDBciGRPLO8i/A65w0XqzJAp8H7fiYFQum1KanNE6IDN+v wHIcOrZgDAAjWRZSWAoqDIuLCx5r4x9EbhY06i3xNsi4dJcGB72I71locff18pS3AXfy lGb++uauJoNIkZOLfFStYFgxYb63yFJHKcycX46OMqCItFYPC215ryE1exm20WFrsCo6 smGmIsvsIJbG2paZkDiz/daEwDpdDzAOhjzJItgNep9fDuWXGlb2PBCsJoYuJXjGeWaI OsdsDS20iEbiW42MkbzmXtJse6CsWEO6d/nKxgG1g9sPYZ/4x+Q50eAI2Q/BJeAwFt2S d5bA== X-Gm-Message-State: AOJu0YwQqClxFrVqDWWsZSEuD879Qqq4yTO6XNC1npFSQytkrnCxesni sXSgQVEWVbj7UclDRuy9CQbtEuvGbvo= X-Google-Smtp-Source: AGHT+IG0EAJzURPebhq2mLBcmZge5GAFpmyynneWQqBgIEc6C7noPhiit541lh29K4KZJmJjhQ545A== X-Received: by 2002:a5d:468d:0:b0:332:c9f5:e5e6 with SMTP id u13-20020a5d468d000000b00332c9f5e5e6mr2294848wrq.17.1700680721352; Wed, 22 Nov 2023 11:18:41 -0800 (PST) Received: from [127.0.0.1] ([13.74.141.28]) by smtp.gmail.com with ESMTPSA id dh10-20020a0560000a8a00b0032d8eecf901sm150856wrb.3.2023.11.22.11.18.41 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 22 Nov 2023 11:18:41 -0800 (PST) Message-ID: <9bfd00c5ecb26a25589c59a0282fce26f948b6a9.1700680717.git.gitgitgadget@gmail.com> In-Reply-To: References: Date: Wed, 22 Nov 2023 19:18:37 +0000 Subject: [PATCH 4/4] t0212: test URL redacting in EVENT format Fcc: Sent Precedence: bulk X-Mailing-List: git@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 To: git@vger.kernel.org Cc: Johannes Schindelin , Jeff Hostetler From: Jeff Hostetler From: Jeff Hostetler In the added tests cases, skip testing the `GIT_TRACE2_REDACT=0` case because we would need to exactly model the full JSON event stream like we did in the preceding basic tests and I do not think it is worth it. Furthermore, the Trace2 routines print the same content in normal, perf, or event format, and in t0210 and t0211 we already tested the basic functionality, so no need to repeat it here. In this test, we use the test-helper to unit test each of the event messages where URLs can appear and confirm that they are redacted in each event. Signed-off-by: Jeff Hostetler --- t/helper/test-trace2.c | 55 +++++++++++++++++++++++++++++++++++++++++ t/t0212-trace2-event.sh | 40 ++++++++++++++++++++++++++++++ 2 files changed, 95 insertions(+) diff --git a/t/helper/test-trace2.c b/t/helper/test-trace2.c index d5ca0046c89..1adac29a575 100644 --- a/t/helper/test-trace2.c +++ b/t/helper/test-trace2.c @@ -412,6 +412,56 @@ static int ut_201counter(int argc, const char **argv) return 0; } +static int ut_300redact_start(int argc, const char **argv) +{ + if (!argc) + die("expect "); + + trace2_cmd_start(argv); + + return 0; +} + +static int ut_301redact_child_start(int argc, const char **argv) +{ + struct child_process cmd = CHILD_PROCESS_INIT; + int k; + + if (!argc) + die("expect "); + + for (k = 0; argv[k]; k++) + strvec_push(&cmd.args, argv[k]); + + trace2_child_start(&cmd); + + strvec_clear(&cmd.args); + + return 0; +} + +static int ut_302redact_exec(int argc, const char **argv) +{ + if (!argc) + die("expect "); + + trace2_exec(argv[0], &argv[1]); + + return 0; +} + +static int ut_303redact_def_param(int argc, const char **argv) +{ + struct key_value_info kvi = KVI_INIT; + + if (argc < 2) + die("expect "); + + trace2_def_param(argv[0], argv[1], &kvi); + + return 0; +} + /* * Usage: * test-tool trace2 @@ -438,6 +488,11 @@ static struct unit_test ut_table[] = { { ut_200counter, "200counter", " [ [ [...]]]" }, { ut_201counter, "201counter", " " }, + + { ut_300redact_start, "300redact_start", "" }, + { ut_301redact_child_start, "301redact_child_start", "" }, + { ut_302redact_exec, "302redact_exec", " " }, + { ut_303redact_def_param, "303redact_def_param", " " }, }; /* clang-format on */ diff --git a/t/t0212-trace2-event.sh b/t/t0212-trace2-event.sh index 6d3374ff773..147643d5826 100755 --- a/t/t0212-trace2-event.sh +++ b/t/t0212-trace2-event.sh @@ -323,4 +323,44 @@ test_expect_success 'discard traces when there are too many files' ' head -n2 trace_target_dir/git-trace2-discard | tail -n1 | grep \"event\":\"too_many_files\" ' +# In the following "...redact..." tests, skip testing the GIT_TRACE2_REDACT=0 +# case because we would need to exactly model the full JSON event stream like +# we did in the basic tests above and I do not think it is worth it. + +test_expect_success 'unsafe URLs are redacted by default in cmd_start events' ' + test_when_finished \ + "rm -r trace.event" && + + GIT_TRACE2_EVENT="$(pwd)/trace.event" \ + test-tool trace2 300redact_start git clone https://user:pwd@example.com/ clone2 && + ! grep user:pwd trace.event +' + +test_expect_success 'unsafe URLs are redacted by default in child_start events' ' + test_when_finished \ + "rm -r trace.event" && + + GIT_TRACE2_EVENT="$(pwd)/trace.event" \ + test-tool trace2 301redact_child_start git clone https://user:pwd@example.com/ clone2 && + ! grep user:pwd trace.event +' + +test_expect_success 'unsafe URLs are redacted by default in exec events' ' + test_when_finished \ + "rm -r trace.event" && + + GIT_TRACE2_EVENT="$(pwd)/trace.event" \ + test-tool trace2 302redact_exec git clone https://user:pwd@example.com/ clone2 && + ! grep user:pwd trace.event +' + +test_expect_success 'unsafe URLs are redacted by default in def_param events' ' + test_when_finished \ + "rm -r trace.event" && + + GIT_TRACE2_EVENT="$(pwd)/trace.event" \ + test-tool trace2 303redact_def_param url https://user:pwd@example.com/ && + ! grep user:pwd trace.event +' + test_done