Skip to content

fix: pass git auth via environment variables instead of writing to .git/config #19963

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
pelikhan merged 2 commits into from
Mar 7, 2026
Merged

fix: pass git auth via environment variables instead of writing to .git/config #19963

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
a52f252
Initial plan
Copilot Mar 7, 2026
c129721
fix: use GIT_CONFIG env vars for git auth to avoid writing secrets to…
Copilot Mar 7, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
44 changes: 15 additions & 29 deletions actions/setup/js/generate_git_patch.cjs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -141,26 +141,27 @@ async function generateGitPatch(branchName, baseBranch, options = {}) {
// 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.
// SECURITY: The header is set immediately before the fetch and removed in a finally
// block to minimize the window during which the token is stored on disk. This is
// important because clean_git_credentials.sh runs before the agent starts — any
// credential written after that cleanup must be removed immediately after use so the
// agent cannot read the token from .git/config.
// SECURITY: The auth header is passed via GIT_CONFIG_* environment variables so it
// is never written to .git/config on disk. This prevents an attacker monitoring file
// changes from reading the secret.
const githubToken = process.env.GITHUB_TOKEN;
const githubServerUrl = process.env.GITHUB_SERVER_URL || "https://github.com";
const extraHeaderKey = `http.${githubServerUrl}/.extraheader`;
let authHeaderSet = false;
try {
if (githubToken) {
const tokenBase64 = Buffer.from(`x-access-token:${githubToken}`).toString("base64");
execGitSync(["config", "--local", extraHeaderKey, `Authorization: basic ${tokenBase64}`], { cwd });
authHeaderSet = true;
debugLog(`Strategy 1 (incremental): Configured git auth for ${githubServerUrl}`);
}

// Build environment for the fetch command with git config passed via env vars.
const fetchEnv = { ...process.env };
if (githubToken) {
const tokenBase64 = Buffer.from(`x-access-token:${githubToken}`).toString("base64");
fetchEnv.GIT_CONFIG_COUNT = "1";
fetchEnv.GIT_CONFIG_KEY_0 = extraHeaderKey;
fetchEnv.GIT_CONFIG_VALUE_0 = `Authorization: basic ${tokenBase64}`;
debugLog(`Strategy 1 (incremental): Configured git auth for ${githubServerUrl} via environment variables`);
}

try {
// Explicitly fetch origin/branchName to ensure we have the latest
// Use "--" to prevent branch names starting with "-" from being interpreted as options
execGitSync(["fetch", "origin", "--", `${branchName}:refs/remotes/origin/${branchName}`], { cwd });
execGitSync(["fetch", "origin", "--", `${branchName}:refs/remotes/origin/${branchName}`], { cwd, env: fetchEnv });
baseRef = `origin/${branchName}`;
debugLog(`Strategy 1 (incremental): Successfully fetched, baseRef=${baseRef}`);
} catch (fetchError) {
Expand All @@ -173,21 +174,6 @@ async function generateGitPatch(branchName, baseBranch, options = {}) {
error: errorMessage,
patchPath: patchPath,
};
} finally {
// SECURITY: Always remove the token from git config immediately after the fetch,
// regardless of success or failure. This prevents the agent from reading the
// credential out of .git/config for the remainder of its session.
if (authHeaderSet) {
try {
execGitSync(["config", "--local", "--unset-all", extraHeaderKey], { cwd });
debugLog(`Strategy 1 (incremental): Removed git auth header`);
} catch {
// Non-fatal: the header may already be absent or the repo state changed.
// However, a persistent failure here may leave the credential in .git/config
// for the remainder of the agent session and should be investigated.
debugLog(`Strategy 1 (incremental): Warning - failed to remove git auth header`);
}
}
}
} else {
// FULL MODE (for create_pull_request):
Expand Down
8 changes: 4 additions & 4 deletions actions/setup/js/git_patch_integration.test.cjs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -637,7 +637,7 @@ describe("git patch integration tests", () => {
};
}

it("should remove auth extraheader from git config after a successful fetch", async () => {
it("should not write auth extraheader to git config during a successful fetch", async () => {
// Set up a feature branch, push a first commit, then add a second commit so
// incremental mode has something new to patch.
execGit(["checkout", "-b", "auth-cleanup-success"], { cwd: workingRepo });
Expand All @@ -660,7 +660,7 @@ describe("git patch integration tests", () => {

expect(result.success).toBe(true);

// Verify the extraheader was removed from git config
// Verify the extraheader was never written to git config (auth is passed via env vars)
const configCheck = spawnSync("git", ["config", "--local", "--get", "http.https://github.example.com/.extraheader"], { cwd: workingRepo, encoding: "utf8" });
// exit status 1 means the key does not exist — that is what we want
expect(configCheck.status).toBe(1);
Comment on lines +663 to 666
Copy link

Copilot AI Mar 7, 2026

Choose a reason for hiding this comment

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

The new assertion only checks that the extraheader is absent after generateGitPatch finishes, which would also pass with the previous implementation that wrote the value to .git/config and then removed it in finally. To actually validate the “never written” behavior, add an assertion that .git/config was not modified during the call (e.g., record .git/config mtime/mtimeNs immediately before calling generateGitPatch and assert it is unchanged afterward).

This issue also appears on line 688 of the same file.

Copilot uses AI. Check for mistakes.
Expand All @@ -669,7 +669,7 @@ describe("git patch integration tests", () => {
}
});

it("should remove auth extraheader from git config even when fetch fails", async () => {
it("should not write auth extraheader to git config even when fetch fails", async () => {
// Create a local-only branch (fetch will fail because it's not on origin)
execGit(["checkout", "-b", "auth-cleanup-failure"], { cwd: workingRepo });
fs.writeFileSync(path.join(workingRepo, "auth-fail.txt"), "auth fail test\n");
Expand All @@ -685,7 +685,7 @@ describe("git patch integration tests", () => {
expect(result.success).toBe(false);
expect(result.error).toContain("Cannot generate incremental patch");

// Verify the extraheader was removed even though the fetch failed
// Verify the extraheader was never written to git config (auth is passed via env vars)
const configCheck = spawnSync("git", ["config", "--local", "--get", "http.https://github.example.com/.extraheader"], { cwd: workingRepo, encoding: "utf8" });
// exit status 1 means the key does not exist — that is what we want
expect(configCheck.status).toBe(1);
Expand Down
Loading