Skip to content

Security: GitHub token embedded in clone URL may leak credentials#340

Open
tuanaiseo wants to merge 1 commit into
zilliztech:masterfrom
tuanaiseo:contribai/fix/security/github-token-embedded-in-clone-url-may-l
Open

Security: GitHub token embedded in clone URL may leak credentials#340
tuanaiseo wants to merge 1 commit into
zilliztech:masterfrom
tuanaiseo:contribai/fix/security/github-token-embedded-in-clone-url-may-l

Conversation

@tuanaiseo

@tuanaiseo tuanaiseo commented Apr 25, 2026

Copy link
Copy Markdown

Problem

The repository cloning logic constructs an HTTPS URL containing the raw GitHub token (https://<token>@github.com/...). Tokens embedded in URLs can leak through process listings, crash logs, shell history, proxy logs, or be persisted in .git/config as the remote origin URL.

Severity: high
File: evaluation/utils/file_management.py

Solution

Avoid embedding credentials in URLs. Use Git credential helpers, GIT_ASKPASS, or authenticated headers/SSH deploy keys. If HTTPS tokens must be used, ensure they are not persisted in remotes and scrub/redact them from all logs.

Changes

  • evaluation/utils/file_management.py (modified)

Testing

  • Existing tests pass
  • Manual review completed
  • No new warnings/errors introduced

@tuanaiseo
fix(security): github token embedded in clone url may leak creden
d60aa06 
The repository cloning logic constructs an HTTPS URL containing the raw GitHub token (`https://<token>@github.com/...`). Tokens embedded in URLs can leak through process listings, crash logs, shell history, proxy logs, or be persisted in `.git/config` as the remote origin URL.

Affected files: file_management.py

Signed-off-by: tuanaiseo <221258316+tuanaiseo@users.noreply.github.com>
@zc277584121

zc277584121 commented Apr 29, 2026

Copy link
Copy Markdown
Collaborator

Thanks for flagging this. I agree that embedding the GitHub token in the clone URL is not ideal, especially because it can be persisted into the cloned repository's remote URL or appear in logs/errors.

I do not think this PR is ready to merge in its current form, though:

  1. The branch is far behind the current master and would remove a number of recently added tests, scripts, and MCP fixes. Please rebase on the latest master and keep the change narrowly scoped to evaluation/utils/file_management.py.

  2. The proposed http.extraheader approach avoids persisting the token in the remote URL, but it still places the token in the git command arguments via -c http.extraheader=..., which can still be visible through process listings on some systems. Since the PR description calls that out as part of the risk, it would be better to use a mechanism that does not expose the token in either the URL or argv, such as a temporary credential helper or GIT_ASKPASS setup scoped to this clone operation.

After those points are addressed, this should be much safer to review again.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@tuanaiseo @zc277584121