Persist HTTP basic auth credentials for git operations after clone - #7144
Open
pujitha24 wants to merge 2 commits into
Open
Persist HTTP basic auth credentials for git operations after clone#7144pujitha24 wants to merge 2 commits into
pujitha24 wants to merge 2 commits into
Conversation
Author
|
Just checking in on this — it's still green and rebased, happy to make any changes if something would help move review along. |
✅ Deploy Preview for pipecd-site canceled.
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #7144 +/- ##
===========================================
+ Coverage 29.67% 46.21% +16.53%
===========================================
Files 601 76 -525
Lines 64392 7543 -56849
===========================================
- Hits 19109 3486 -15623
+ Misses 43789 3780 -40009
+ Partials 1494 277 -1217
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Contributor
There was a problem hiding this comment.
Pull request overview
This PR updates PipeCD’s internal Git client/worktree implementation to persist HTTP Basic Auth credentials (derived from configured username/password) into the checked-out repository config so that subsequent Git operations (pull/fetch/push) continue to authenticate after the initial clone.
Changes:
- Add
repo.setHTTPAuthHeaderto persisthttp.extraHeaderinto the repo’s git config and carry it acrossCopyToModify. - Refactor Basic Auth header construction into a helper (
basicAuthHeader) and persist it duringClient.Clone. - Add unit tests to verify
http.extraHeaderpersistence and propagation.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| pkg/git/repo.go | Adds persistent HTTP auth header support and propagates it across CopyToModify. |
| pkg/git/repo_test.go | Adds tests for setHTTPAuthHeader and for propagation in CopyToModify. |
| pkg/git/client.go | Introduces basicAuthHeader helper and persists HTTP auth config after worktree clone. |
| pkg/git/client_test.go | Adds coverage to ensure Clone persists http.extraHeader into the checked-out repo. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+482
to
+488
| func (r *repo) setHTTPAuthHeader(ctx context.Context, header string) error { | ||
| if out, err := r.runGitCommand(ctx, "config", "http.extraHeader", header); err != nil { | ||
| return formatCommandError(err, out) | ||
| } | ||
| r.httpAuthHeader = header | ||
| return nil | ||
| } |
Contributor
|
@pujitha24 The required Go checks are currently failing due to Test_reporter_flush, which appears unrelated to this PR and is already fixed on current master by #7221. Could you please rebase onto the latest master and rerun the checks ? |
Motivation: When a piped git repository is configured with HTTPS username/password (PAT) authentication, pkg/git/client.go's Client.Clone() only passed the Authorization header as a one-off `-c http.extraHeader=...` flag to the `git clone --mirror` / `git fetch` commands used to populate its internal repo cache. That flag is never persisted, so the Repo object handed back to callers (a worktree checked out from the cache, with origin rewritten to the real HTTPS remote URL) carries no credentials at all. Components such as the event watcher call repo.Pull() repeatedly on that same object over time, and Pull/Push/ MergeRemoteBranch/CheckoutPullRequest in pkg/git/repo.go issue git commands straight at the remote with no auth args, so every one of them fails with "fatal: could not read Username for 'https://github.com': No such device or address" -- reproducing exactly the error from the report. Approach: After Client.Clone() checks out the destination repo, persist the basic-auth header into that repo's git config via a new repo.setHTTPAuthHeader method (`git config http.extraHeader <value>`), so every later git command run in that directory picks up the credentials automatically instead of relying on a one-off CLI flag. repo.CopyToModify (used to make a scratch copy for committing and pushing changes) does a plain local `git clone`, which doesn't carry over custom config keys, so the header is propagated there too before CopyToModify's initial fetch. Validation: Ran `go build ./...` (whole main module, passes) and `go test ./pkg/git/...` (passes, including three new/extended tests: TestClonePersistsHTTPAuthHeader, Test_setHTTPAuthHeader, and an extended TestCopyToModify). Confirmed TestClonePersistsHTTPAuthHeader fails against the pre-fix code: with the new persistence call temporarily removed from Clone(), the test fails because `git config --get http.extraHeader` finds nothing in the checked-out repo; restoring the fix makes it pass again. This is a local git-config assertion, not a live HTTPS clone against a real PAT-protected remote, but it directly demonstrates the exact defect: credentials silently dropped after the initial clone. Report: pipe-cd#6453 Signed-off-by: Pujitha Paladugu <10557236+pujitha24@users.noreply.github.com>
Signed-off-by: Pujitha Paladugu <10557236+pujitha24@users.noreply.github.com>
pujitha24
force-pushed
the
auto/issue-6453
branch
from
August 26, 2026 21:54
29269ba to
dab9546
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
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.
What this PR does:
Persists the HTTP Basic-Auth header (built from the piped git
username/passwordconfig) into the checked-out repository's git config, so that git operations run after the initial clone (pull,fetch,push, etc.) keep using the configured credentials.Why we need it:
When a piped git repository is configured with
username/password(a PAT) for HTTPS auth,pkg/git/client.go'sClient.Clone()only passed theAuthorizationheader as a one-off-c http.extraHeader=...flag to thegit clone --mirror/git fetchcommands used to populate its internal cache. That flag is never persisted, and theRepoobject handed back to callers (a worktree checked out from the cache, withoriginrewritten to the real HTTPS remote URL) carries no credentials at all. Components such as the event watcher (pkg/app/piped/eventwatcher) callrepo.Pull()repeatedly on that same object over time;Pull/Push/MergeRemoteBranch/CheckoutPullRequestinpkg/git/repo.goissue git commands straight at the remote with no auth, so every one of them fails withfatal: could not read Username for 'https://github.com': No such device or address— reproducing exactly the error reported in the issue.Which issue(s) this PR fixes:
Fixes #
Does this PR introduce a user-facing change?:
username/passwordgit auth (a common CI/CD PAT pattern) now keep working after the initial clone — previously, background git operations like the event watcher's periodic pull and re-clone would fail indefinitely with a credential error. Deployments using SSH auth are unaffected.AI assistance: this change was drafted with Claude Code.
Fixes #6453