[0/4] Redact unsafe URLs in the Trace2 output

Message

Philippe Blain via GitGitGadget Nov. 22, 2023, 7:18 p.m. UTC

The Trace2 output can contain secrets when a user issues a Git command with
sensitive information in the command-line. A typical (if highly discouraged)
example is: git clone https://user:[e-mail]/.

With this PR, the Trace2 output redacts passwords in such URLs by default.

This series also includes a commit to temporarily disable leak checking on
t0210,t0211 because the tests uncover other unrelated bugs in Git.

These patches were integrated into Microsoft's fork of Git, as
https://github.com/microsoft/git/pull/616, and have been cooking there ever
since.

Jeff Hostetler (3):
  trace2: fix signature of trace2_def_param() macro
  t0211: test URL redacting in PERF format
  t0212: test URL redacting in EVENT format

Johannes Schindelin (1):
  trace2: redact passwords from https:// URLs by default

 t/helper/test-trace2.c   |  55 ++++++++++++++++++
 t/t0210-trace2-normal.sh |  20 ++++++-
 t/t0211-trace2-perf.sh   |  21 ++++++-
 t/t0212-trace2-event.sh  |  40 +++++++++++++
 trace2.c                 | 120 ++++++++++++++++++++++++++++++++++++++-
 trace2.h                 |   4 +-
 6 files changed, 253 insertions(+), 7 deletions(-)


base-commit: 564d0252ca632e0264ed670534a51d18a689ef5d
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1616%2Fdscho%2Ftrace2-redact-credentials-in-https-urls-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1616/dscho/trace2-redact-credentials-in-https-urls-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/1616

Comments

Elijah Newren Nov. 23, 2023, 7:08 p.m. UTC | #1

On Wed, Nov 22, 2023 at 11:18 AM Johannes Schindelin via GitGitGadget
<[e-mail]> wrote:
>
> The Trace2 output can contain secrets when a user issues a Git command with
> sensitive information in the command-line. A typical (if highly discouraged)
> example is: git clone https://user:[e-mail]/.
>
> With this PR, the Trace2 output redacts passwords in such URLs by default.
>
> This series also includes a commit to temporarily disable leak checking on
> t0210,t0211 because the tests uncover other unrelated bugs in Git.
>
> These patches were integrated into Microsoft's fork of Git, as
> https://github.com/microsoft/git/pull/616, and have been cooking there ever
> since.

Thanks for making these changes.  Makes me wonder, back when we were
logging trace2 data, if we had some of these leaks.  Eek.

As I commented in patch 2, I think this is a good start, but I'm
curious if others would be willing to turn clone/fetch of such bad
URLs into warnings for now and errors later.  The prevalence of
AI-assist add-ons for various IDEs and the number of developers opting
to use those IDEs and add-ons, and the fact that these tools sometimes
include repository URLs in what they send off to third parties, makes
me wonder if our recent infosec fire drill is soon going to be a
widely shared experience by lots of other companies and individuals.
Training users to not do bad things is hard, and it might be worth
saving them from themselves.  Thoughts?