Skip to content

Fix git fetch auth in generateGitPatch after clean_git_credentials.sh #20581

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 3 commits into from
Mar 11, 2026
Merged

Fix git fetch auth in generateGitPatch after clean_git_credentials.sh #20581

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
e031505
Initial plan
Copilot Mar 11, 2026
89a8fcd
Fix git fetch auth after clean_git_credentials.sh for push_to_pull_re…
Copilot Mar 11, 2026
d63c437
Merge branch 'main' into copilot/fix-git-fetch-auth-issue
pelikhan Mar 11, 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
5 changes: 5 additions & 0 deletions .changeset/patch-fix-git-fetch-auth-after-clean-credentials.md
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

14 changes: 12 additions & 2 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 @@ -89,6 +89,8 @@ function getPatchPathForRepo(branchName, repoSlug) {
* Use this for multi-repo scenarios where repos are checked out to subdirectories.
* @param {string} [options.repoSlug] - Repository slug (owner/repo) to include in patch filename for disambiguation.
* Required for multi-repo scenarios to prevent patch file collisions.
* @param {string} [options.token] - GitHub token for git authentication. Falls back to GITHUB_TOKEN env var.
* Use this for cross-repo scenarios where a custom PAT with access to the target repo is needed.
* @returns {Promise<Object>} Object with patch info or error
*/
async function generateGitPatch(branchName, baseBranch, options = {}) {
Expand Down Expand Up @@ -146,9 +148,10 @@ async function generateGitPatch(branchName, baseBranch, options = {}) {
// Configure git authentication via GIT_CONFIG_* environment variables.
// This ensures the fetch works when .git/config credentials are unavailable
// (e.g. after clean_git_credentials.sh) and on GitHub Enterprise Server (GHES).
// Use options.token when provided (cross-repo PAT), falling back to GITHUB_TOKEN.
// SECURITY: The auth header is passed via env vars so it is never written to
// .git/config on disk, preventing file-monitoring attacks.
const fetchEnv = { ...process.env, ...getGitAuthEnv() };
const fetchEnv = { ...process.env, ...getGitAuthEnv(options.token) };

try {
// Explicitly fetch origin/branchName to ensure we have the latest
Expand Down Expand Up @@ -192,8 +195,15 @@ async function generateGitPatch(branchName, baseBranch, options = {}) {
// origin/<defaultBranch> doesn't exist locally, try to fetch it
debugLog(`Strategy 1 (full): origin/${defaultBranch} not found locally, attempting fetch`);
try {
// Configure git authentication via GIT_CONFIG_* environment variables.
// This ensures the fetch works when .git/config credentials are unavailable
// (e.g. after clean_git_credentials.sh) and on GitHub Enterprise Server (GHES).
// Use options.token when provided (cross-repo PAT), falling back to GITHUB_TOKEN.
// SECURITY: The auth header is passed via env vars so it is never written to
// .git/config on disk, preventing file-monitoring attacks.
const fullFetchEnv = { ...process.env, ...getGitAuthEnv(options.token) };
// Use "--" to prevent branch names starting with "-" from being interpreted as options
execGitSync(["fetch", "origin", "--", defaultBranch], { cwd });
execGitSync(["fetch", "origin", "--", defaultBranch], { cwd, env: fullFetchEnv });
hasLocalDefaultBranch = true;
debugLog(`Strategy 1 (full): Successfully fetched origin/${defaultBranch}`);
} catch (fetchErr) {
Expand Down
36 changes: 36 additions & 0 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 @@ -703,6 +703,42 @@ describe("git patch integration tests", () => {
}
});

it("should use options.token instead of GITHUB_TOKEN when provided", async () => {
// Set up a feature branch with a commit to push
execGit(["checkout", "-b", "token-option-test"], { cwd: workingRepo });
fs.writeFileSync(path.join(workingRepo, "token-test.txt"), "token option test\n");
execGit(["add", "token-test.txt"], { cwd: workingRepo });
execGit(["commit", "-m", "Token option base commit"], { cwd: workingRepo });
execGit(["push", "-u", "origin", "token-option-test"], { cwd: workingRepo });

// Add a second commit that will become the incremental patch
fs.writeFileSync(path.join(workingRepo, "token-test2.txt"), "token option test 2\n");
execGit(["add", "token-test2.txt"], { cwd: workingRepo });
execGit(["commit", "-m", "Token option new commit"], { cwd: workingRepo });

// Delete the tracking ref so generateGitPatch has to re-fetch
execGit(["update-ref", "-d", "refs/remotes/origin/token-option-test"], { cwd: workingRepo });

const restore = setTestEnv(workingRepo);
try {
// Pass a custom token via options.token — the local git server ignores auth so the
// fetch still succeeds, but we verify no credentials are written to disk.
const result = await generateGitPatch("token-option-test", "main", {
mode: "incremental",
token: "ghs_custom_token_for_cross_repo",
});

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

// Verify the extraheader was never written to git config (auth is passed via env vars only)
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);
} finally {
Comment on lines +723 to +737
Copy link

Copilot AI Mar 11, 2026

Choose a reason for hiding this comment

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

This test name/assertion claims to verify that options.token is used, but it never actually proves the override happened. Because the test repo uses a local file:// origin, the fetch succeeds regardless of auth, and the only assertion checks that .extraheader was not written to disk (which is already covered by the earlier tests). Consider asserting token usage by mocking/spying on getGitAuthEnv or execGitSync (e.g., via vi.resetModules() + vi.doMock('./git_helpers.cjs', ...)) and verifying the fetch call receives an env built from the provided token (and not the GITHUB_TOKEN).

Suggested change
try {
// Pass a custom token via options.token — the local git server ignores auth so the
// fetch still succeeds, but we verify no credentials are written to disk.
const result = await generateGitPatch("token-option-test", "main", {
mode: "incremental",
token: "ghs_custom_token_for_cross_repo",
});
expect(result.success).toBe(true);
// Verify the extraheader was never written to git config (auth is passed via env vars only)
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);
} finally {
const originalGithubToken = process.env.GITHUB_TOKEN;
process.env.GITHUB_TOKEN = "default_env_token";
// Capture environments passed to execGitSync (from git_helpers.cjs)
const execGitEnvs = [];
// Re-load modules with a mocked git_helpers.cjs that wraps execGitSync
vi.resetModules();
await vi.doMock("./git_helpers.cjs", async () => {
const actual = await vi.importActual("./git_helpers.cjs");
return {
...actual,
execGitSync: (...args) => {
const [gitArgs, options] = args;
if (options && options.env) {
execGitEnvs.push({ args: gitArgs, env: options.env });
}
return actual.execGitSync(...args);
},
};
});
const { generateGitPatch: generateGitPatchWithSpy } = await import("./generate_git_patch.cjs");
try {
// Pass a custom token via options.token — the local git server ignores auth so the
// fetch still succeeds, but we now also verify the env passed to git.
const result = await generateGitPatchWithSpy("token-option-test", "main", {
mode: "incremental",
token: "ghs_custom_token_for_cross_repo",
});
expect(result.success).toBe(true);
// Verify that git fetch (or any git command using auth env) received the custom token
const fetchEnvEntries = execGitEnvs.filter(
entry => Array.isArray(entry.args) && entry.args[0] === "fetch"
);
expect(fetchEnvEntries.length).toBeGreaterThan(0);
for (const entry of fetchEnvEntries) {
expect(entry.env).toBeDefined();
expect(entry.env.GITHUB_TOKEN).toBe("ghs_custom_token_for_cross_repo");
expect(entry.env.GITHUB_TOKEN).not.toBe("default_env_token");
}
// Verify the extraheader was never written to git config (auth is passed via env vars only)
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);
} finally {
process.env.GITHUB_TOKEN = originalGithubToken;

Copilot uses AI. Check for mistakes.
restore();
}
});

it("should include all commits in full mode even when origin/branch exists", async () => {
// Create a feature branch with first commit
execGit(["checkout", "-b", "full-mode-branch"], { cwd: workingRepo });
Expand Down
13 changes: 12 additions & 1 deletion actions/setup/js/safe_outputs_handlers.cjs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -319,6 +319,11 @@ function createHandlers(server, appendSafeOutput, config = {}) {
if (repoSlug) {
patchOptions.repoSlug = repoSlug;
}
// Pass per-handler token so cross-repo PATs are used for git fetch when configured.
// Falls back to GITHUB_TOKEN if not set.
if (prConfig["github-token"]) {
patchOptions.token = prConfig["github-token"];
}
const patchResult = await generateGitPatch(entry.branch, baseBranch, patchOptions);

if (!patchResult.success) {
Expand Down Expand Up @@ -423,7 +428,13 @@ function createHandlers(server, appendSafeOutput, config = {}) {
// Incremental mode only includes commits since origin/branchName,
// preventing patches that include already-existing commits
server.debug(`Generating incremental patch for push_to_pull_request_branch with branch: ${entry.branch}, baseBranch: ${baseBranch}`);
const patchResult = await generateGitPatch(entry.branch, baseBranch, { mode: "incremental" });
// Pass per-handler token so cross-repo PATs are used for git fetch when configured.
// Falls back to GITHUB_TOKEN if not set.
const pushPatchOptions = { mode: "incremental" };
if (pushConfig["github-token"]) {
pushPatchOptions.token = pushConfig["github-token"];
}
const patchResult = await generateGitPatch(entry.branch, baseBranch, pushPatchOptions);

if (!patchResult.success) {
// Patch generation failed or patch is empty
Expand Down
Loading