From a52f2527b02ad56fb24f7189ab5078cf21bc12c1 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sat, 7 Mar 2026 14:18:09 +0000 Subject: [PATCH 1/2] Initial plan From c12972161dd052b3553897de1ba4dcf01f4cec7c Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sat, 7 Mar 2026 14:30:59 +0000 Subject: [PATCH 2/2] fix: use GIT_CONFIG env vars for git auth to avoid writing secrets to disk Instead of writing the GitHub token to .git/config via `git config --local` and cleaning it up in a finally block, pass the auth header via GIT_CONFIG_COUNT/KEY/VALUE environment variables. This ensures the token is never written to disk, eliminating the window where an attacker monitoring file changes could read the secret. Update integration tests to verify the token is never written to git config (rather than verifying it was cleaned up after use). Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- actions/setup/js/generate_git_patch.cjs | 44 +++++++------------ .../setup/js/git_patch_integration.test.cjs | 8 ++-- 2 files changed, 19 insertions(+), 33 deletions(-) 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);