diff --git a/actions/setup/js/generate_git_patch.cjs b/actions/setup/js/generate_git_patch.cjs index bedaef7b718..4482dfc236a 100644 --- a/actions/setup/js/generate_git_patch.cjs +++ b/actions/setup/js/generate_git_patch.cjs @@ -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) { @@ -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): diff --git a/actions/setup/js/git_patch_integration.test.cjs b/actions/setup/js/git_patch_integration.test.cjs index c9a1a7f8682..8a09f507cb2 100644 --- a/actions/setup/js/git_patch_integration.test.cjs +++ b/actions/setup/js/git_patch_integration.test.cjs @@ -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 }); @@ -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); @@ -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"); @@ -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);