Skip to content

fix(git): use URL-rewrite for token-authenticated clone#1601

Open
WaliedA wants to merge 2 commits intoagent0ai:mainfrom
WaliedA:fix/git-clone-token-auth
Open

fix(git): use URL-rewrite for token-authenticated clone#1601
WaliedA wants to merge 2 commits intoagent0ai:mainfrom
WaliedA:fix/git-clone-token-auth

Conversation

@WaliedA
Copy link
Copy Markdown

@WaliedA WaliedA commented May 4, 2026

Problem

When git_token was supplied, the previous code used git -c http.extraHeader=Authorization: Basic <base64>. Git still ran its credential subsystem first and aborted with:

fatal: could not read Username for 'https://github.com': terminal prompts disabled

This broke any clone with a non-empty token, including public repos.

Fix

Replace the broken header approach with URL credential injection:

  • GitHub / GitLab / others: https://oauth2:<token>@host/...
  • Bitbucket: https://x-token-auth:<token>@bitbucket.org/...

After a successful clone the remote origin is immediately reset to the clean URL so the token is never in .git/config.

Security hardening

Change Why
_validate_git_url() regex allowlist Rejects URLs with shell metacharacters (command-injection guard)
_sanitize_token() on errors Token never appears in messages returned to the client
URL passed as argv element, never shell string No shell-interpolation risk
Token excluded from BasicProjectData (existing) Not written to project.json

Acceptance tests

Test Result
Anonymous clone of public repo ✅ ok: True
Public repo + bogus token (regression) ✅ ok: True (was failing before)
Invalid token → sanitized error ✅ ok: False, token redacted as ***
Command-injection URL rejected ✅ ok: False, validation error

All .git/config files confirmed token-free after clone.

Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: ce38afa30e

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread helpers/git.py Outdated
Replace the broken http.extraHeader approach with URL credential
injection. The old approach still triggered git's credential subsystem
which aborted with 'could not read Username' when GIT_TERMINAL_PROMPT=0.

Changes:
- Rewrite HTTPS clone URL to https://oauth2:<token>@host/... (GitHub/GitLab)
  or https://x-token-auth:<token>@bitbucket.org/... (Bitbucket)
- After clone, reset remote origin to the clean URL (token never on disk)
- Add _validate_git_url() to reject unsafe/non-http(s)/ssh URLs
- Sanitize token from error messages before re-raising
- Pass clone command as argv list, never via shell string concat

Fixes: public repos with any token, private repos with valid token.
All five acceptance tests pass.
@WaliedA WaliedA force-pushed the fix/git-clone-token-auth branch from ce38afa to 49a0ff9 Compare May 4, 2026 01:13
…r _validate_git_url with non-git SSH usernamesURL validation

Add regression tests for _validate_git_url to ensure it accepts safe SSH usernames and rejects unsafeGuards against the regression flagged by Codex review on PR agent0ai#1601 (commit ce38afa) where _SAFE_URL_RE only accepted the literal git@ SSH username. Commit 49a0ff9 already broadened the regex; this test locks that behavior in by parametrizing alice@host:org/repo.git, ssh://alice@host/..., and ssh://alice@host:2222/... alongside the existing git@ and https forms. Also covers a few unsafe URL classes that must continue to be rejected. URLs.
@WaliedA WaliedA closed this May 8, 2026
@WaliedA WaliedA reopened this May 8, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants