fix(git): use URL-rewrite for token-authenticated clone#1601
Open
WaliedA wants to merge 2 commits intoagent0ai:mainfrom
Open
fix(git): use URL-rewrite for token-authenticated clone#1601WaliedA wants to merge 2 commits intoagent0ai:mainfrom
WaliedA wants to merge 2 commits intoagent0ai:mainfrom
Conversation
There was a problem hiding this comment.
💡 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".
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.
ce38afa to
49a0ff9
Compare
…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.
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
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.
Problem
When
git_tokenwas supplied, the previous code usedgit -c http.extraHeader=Authorization: Basic <base64>. Git still ran its credential subsystem first and aborted with:This broke any clone with a non-empty token, including public repos.
Fix
Replace the broken header approach with URL credential injection:
https://oauth2:<token>@host/...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
_validate_git_url()regex allowlist_sanitize_token()on errorsBasicProjectData(existing)project.jsonAcceptance tests
***All
.git/configfiles confirmed token-free after clone.