Uh oh!
There was an error while loading. Please reload this page.
fix: configure git auth for GHES in incremental patch fetch - #19685
Conversation
…de fetch Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
| // Configure git authentication using GITHUB_TOKEN and GITHUB_SERVER_URL. | ||
| // This ensures the fetch works on GitHub Enterprise Server (GHES) where | ||
| // the default credential helper may not be configured for the enterprise endpoint. | ||
| const githubToken = process.env.GITHUB_TOKEN; |
There was a problem hiding this comment.
Security review findings and fixes (d4cff95):
Issue found: token persisted in .git/config for duration of agent session
clean_git_credentials.sh runs before the agent starts as a security measure. The previous commit wrote GITHUB_TOKEN to .git/config via http.<serverUrl>/.extraheader during the agent session without cleanup — meaning the token remained readable from .git/config for the rest of the session (e.g., by Codex in its sandbox, or via other MCP tools with filesystem access).
Fix: Added a try/finally block around the fetch that calls git config --local --unset-all <key> to remove the header immediately after the fetch completes, regardless of success or failure. JavaScript finally runs even when catch contains a return, so cleanup is guaranteed in all code paths:
- Token set → fetch succeeds →
finallyremoves token - Token set → fetch fails →
catchprepares error →finallyremoves token → return fires GITHUB_TOKENabsent → no config write →finallyis a no-op
Other findings (no action needed):
execGitSyncdebug logging viacore.debugonly activates whencoreis defined (GitHub Actions context only); in the MCP server processcoreis undefined so the token value is never logged. GitHub Actions also masksGITHUB_TOKENin logs.GITHUB_SERVER_URLis set by GitHub Actions infrastructure, not user-controlled input, so there is no header-injection risk.--localflag scopes the config write to the specific repo atcwd, not the global git config.
…oken leakage Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
pelikhan
commented
Mar 5, 2026
There was a problem hiding this comment.
Pull request overview
Configures authenticated git fetch for incremental patch generation so push_to_pull_request_branch works in non-interactive environments (and on GHES) where Git cannot prompt for credentials.
Changes:
- Add temporary
http.<GITHUB_SERVER_URL>/.extraheaderconfiguration usingGITHUB_TOKENimmediately before the incrementalgit fetch. - Ensure the extraheader is removed in a
finallyblock after the fetch attempt (success or failure). - Add explanatory security-focused comments and debug logs around the incremental fetch flow.
Comments suppressed due to low confidence (1)
actions/setup/js/generate_git_patch.cjs:156
- The call to
execGitSyncwithAuthorization: basic ${tokenBase64}as an argument will cause the full base64-encodedGITHUB_TOKENto be emitted in logs viaexecGitSync's debug logging (core.debug/console.debugin the shim), leaking the token into MCP/server or Actions logs. Anyone with access to these logs could recover and reuse the token outside its intended context, effectively bypassing the protections provided byclean_git_credentials.sh. To fix this, avoid passing the raw authorization header through a logging helper (e.g., use a path that does not log arguments, or ensureexecGitSyncredactsAuthorizationvalues before logging) so that the token never appears in logs.
execGitSync(["config", "--local", extraHeaderKey, `Authorization: basic ${tokenBase64}`], { cwd });
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
Uh oh!
There was an error while loading. Please reload this page.
| const githubToken = process.env.GITHUB_TOKEN; | ||
| const githubServerUrl = process.env.GITHUB_SERVER_URL || "https://github.com"; | ||
| const extraHeaderKey = `http.${githubServerUrl}/.extraheader`; | ||
| let authHeaderSet = false; |
There was a problem hiding this comment.
This change adds new behavior in incremental mode (setting/unsetting http.<serverUrl>/.extraheader based on GITHUB_TOKEN). The existing vitest suite for generate_git_patch.cjs doesn’t appear to assert that the auth header is removed on both success and fetch failure. Adding a test that creates a temporary repo and verifies the config key is unset after the incremental fetch path would help prevent regressions, especially for the security boundary described in the comments.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
pelikhan
commented
Mar 5, 2026
@copilot address comments |
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Addressed in ca376d3:
|
Uh oh!
There was an error while loading. Please reload this page.
push_to_pull_request_branchMCP handler fails withCannot generate incremental patch: failed to fetchbecausegenerate_git_patch.cjsrunsgit fetch originviaspawnSyncwithout authentication. When the MCP server owns stdin (piped for protocol I/O), git cannot interactively prompt for credentials — failing withfatal: could not read Username for 'https://github.com': No such device or address— and on GHES the auth header configured byactions/checkouttargets the enterprise endpoint, notgithub.com.Changes
generate_git_patch.cjs— incremental mode fetch: Before callinggit fetch origin, configurehttp.<serverUrl>/.extraheaderusingGITHUB_TOKENandGITHUB_SERVER_URL, matching the pattern already used indynamic_checkout.cjs. The header is written immediately before the fetch and removed in afinallyblock immediately after, ensuring the token is never left in.git/configfor the duration of the agent session:This ensures the fetch uses the correct authenticated endpoint on both github.com and GHES without relying on interactive credential prompts.
git_patch_integration.test.cjs— auth header cleanup tests: Added two integration tests that verify the security boundary is upheld:http.<serverUrl>/.extraheaderkey is absent from.git/configafter a successful incremental fetch.finallyblock runs in all code paths.Security
The workflow's
clean_git_credentials.shruns before the agent starts to prevent the agent from accessing git credentials. Writing a token to.git/configafter that cleanup point and leaving it there for the rest of the session would undermine this security boundary — the agent could read the token from.git/configif it has filesystem access (e.g., Codex sandbox, other MCP tools with file access).The
try/finallypattern guarantees thehttp.<serverUrl>/.extraheaderis removed from.git/configimmediately after the fetch completes, regardless of success or failure. JavaScriptfinallysemantics ensure cleanup even whencatchcontains areturnstatement.Original prompt
✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.