Repository navigation
fix(util): redact URL credentials in config values on the command line - #2299
Open
Keerthana-64 wants to merge 2 commits into
Open
Keerthana-64 wants to merge 2 commits into
Keerthana-64 wants to merge 2 commits into
Conversation
`remove_password_if_present` is applied to every command line that GitPython logs or stores in `GitCommandError` and the other `CommandError` subclasses. It redacted an argument only when the whole argument was a URL with userinfo, or when it was an `http.extraHeader=Authorization: ...` assignment. Two common forms were missed: - A URL that is the value of a configuration assignment, such as `-c http.proxy=http://user:pass@proxy:3128` or `-c remote.origin.url=https://user:token@host/repo`. The text before `://` contains `=`, so `urlsplit` finds no scheme and no userinfo, and the argument was kept verbatim. - The `--config=<key>=<value>` form, which is what `transform_kwargs` produces for a `config=...` keyword argument. The leading `--config=` defeated both checks, so even `--config=http.extraHeader=Authorization: Bearer <token>` was printed in full. For example, after `repo.git.set_persistent_git_options(c="http.proxy=http://user:secret@proxy:3128")` every failing command raised a `GitCommandError` whose message contained `user:secret`, and the same text reached the log when command tracing is on. The URL redaction moves into a small `_redact_url_credentials` helper. `remove_password_if_present` strips a leading `--config=` before the extra header check, and when the whole argument is not a URL with userinfo it tries the assignment after `--config=` (which covers `url.<base>.insteadOf` keys that carry credentials) and then the value after the first `=`. Only the userinfo is replaced, so the key and the rest of the URL stay readable, and arguments without a URL are returned unchanged. Arguments that were already redacted produce the same output as before. Adds `test_remove_password_from_config_values` to `test/test_util.py`. Its five redaction cases fail before this change and pass after it, and the existing redaction tests still pass. `ruff check`, `ruff format --check` and `mypy` are clean.
- [P1] Redact the value when the config key also contains a URL — /Users/byron/dev/github.com/gitpython-developers/GitPython.redact-config-url-c redentials/git/util.py:700-705 When a URL-specific config key and its value both contain credentials, such as `--config=http.https://alice@host.proxy=http://bob:token@proxy`, redacting the key sets `redacted`, so the value is never checked. `Git.execute` traces and `GitCommandError.command` still expose `bob:token`. Redact the key and value independently. - [P2] Recognize clone's compact -c configuration form — /Users/byron/dev/github.com/gitpython-developers/GitPython.redact-config-url-c redentials/git/util.py:686-688 When `Repo.clone_from` receives Git clone's valid `-c<key>=<value>` form through `multi_options`, a credentialed URL key such as `-curl.https://user:token@host/.insteadOf=https://old/` remains unchanged. The leading `-c` prevents URL parsing, while the fallback checks only the value, so traces and exceptions expose the token. Strip this prefix before checking the key. Assisted-by: GPT 6.0 Astra Assisted-by: GPT 6.1 Sol Co-authored-by: GPT 6.1 Sol <codex@openai.com>
Byron
force-pushed
the
redact-config-url-credentials
branch
from
October 21, 2026 13:37
c743518 to
f89c343
Compare
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
remove_password_if_presentonly redacted an argument that was itself a URL with userinfo, or a-c http.extraHeader=Authorization: ...assignment. a URL given as a config value, such as-c http.proxy=http://user:pass@proxy:3128or-c remote.origin.url=https://user:token@host/repo, has=before://, sourlsplitfinds no userinfo, and the--config=<key>=<value>form thattransform_kwargsproduces forconfig=...defeated both checks, the extra header one included. afterrepo.git.set_persistent_git_options(c="http.proxy=http://user:secret@proxy:3128"), every failing command raised aGitCommandErrorcontaininguser:secret. the fix stays inremove_password_if_presentbecause every logged or raised command line already goes through it: it now also tries the assignment after--config=and the value after the first=, and replaces only the userinfo.test_remove_password_from_config_valuesfails before the change and passes after it.