Redact unsafe URLs in the Trace2 output #616
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
jeffhostetler
force-pushed
the
redact-unsafe-urls-in-trace2
branch
from
719fffa
to
63e847b
Compare
|
Unrelated to Trace2, the same problem can be seen in GIT_TRACE_PERFORMANCE: |
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`). Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Jeff Hostetler <jeffhostetler@github.com>
Add `struct key_value_info` argument to `trace2_def_param()`. In dc90208 (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 <jeffhostetler@github.com>
jeffhostetler
force-pushed
the
redact-unsafe-urls-in-trace2
branch
from
63e847b
to
b489c62
Compare
Signed-off-by: Jeff Hostetler <jeffhostetler@github.com>
jeffhostetler
force-pushed
the
redact-unsafe-urls-in-trace2
branch
from
b489c62
to
23987cf
Compare
Turn off TEST_PASSES_SANITIZE_LEAK in t0210 and t0211 tests. The tests added in a previous commit to confirm that we redact URLs in the Trace2 output uncovered leaks in `builtin/clone.c` and `remote.c`. We observed that (1) `the_repository->remote_status` is not released properly. And (2) that we are using `url...insteadOf` and that runs into a code path where an allocated URL is replaced with another URL. And (3) `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 these leaks / bugs. Signed-off-by: Jeff Hostetler <jeffhostetler@github.com> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
|
I cancelled the job because the osx-clang job seemed to be stuck after almost 90 minutes. |
|
@dscho Should we update the workflow to eliminate the osx-gcc compile since we're now using osx-clang and universal builds. Wondering if it is completely redundant now. |
I'd love to keep it because upstream Git keeps it. |
|
Keeping the osc-gcc job is fine. |
dscho
pushed a commit
that referenced
this pull request
Nov 8, 2023
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:password@host.com/`. 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.
dscho
pushed a commit
that referenced
this pull request
Nov 14, 2023
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:password@host.com/`. 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.
dscho
pushed a commit
that referenced
this pull request
Nov 20, 2023
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:password@host.com/`. 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.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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:password@host.com/.With this PR, the Trace2 output redacts passwords in such URLs by default.