Run checkout auth cleanup when API commit signing is enabled (#1597)

* Run checkout auth cleanup when API commit signing is enabled

* Derive git-config test expectations from GITHUB_SERVER_URL

No-Verification-Needed: test-only change
This commit is contained in:
Ashwin Bhat 2026-08-06 10:18:34 -07:00 committed by GitHub
parent e1fc925862
commit 96e281f4d9
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
6 changed files with 391 additions and 4 deletions

View File

@ -42,6 +42,27 @@ export async function configureGitAuth(
await $`git config user.email "${botId}+${botName}@${noreplyDomain}"`;
console.log(`✓ Set git user as ${botName}`);
await replaceCheckoutCredentials(githubToken, context);
console.log("Git authentication configured successfully");
}
/**
* Replace the credential that actions/checkout persisted in the working tree.
*
* actions/checkout stores its token as an `http.<server>/.extraheader` entry
* in .git/config for the duration of the job. Claude and the tools it invokes
* run inside this working tree, so remove that entry and back git with the
* action's own token instead (a credential helper when non-write users are
* allowed, otherwise the origin URL). This applies to every mode, including API
* commit signing where no other git configuration is needed.
*/
export async function replaceCheckoutCredentials(
githubToken: string,
context: GitHubContext,
) {
const serverUrl = new URL(GITHUB_SERVER_URL);
// Remove the authorization header that actions/checkout sets
console.log("Removing existing git authentication headers...");
try {
@ -79,8 +100,6 @@ export async function configureGitAuth(
await $`git remote set-url origin ${remoteUrl}`;
console.log("✓ Updated remote URL with authentication token");
}
console.log("Git authentication configured successfully");
}
/**

View File

@ -3,6 +3,7 @@ import { prepareMcpConfig } from "../../mcp/install-mcp-server";
import { parseAllowedTools } from "./parse-tools";
import {
configureGitAuth,
replaceCheckoutCredentials,
setupSshSigning,
} from "../../github/operations/git-config";
import { checkHumanActor } from "../../github/validation/actor";
@ -62,6 +63,16 @@ export async function prepareAgentMode({
console.error("Failed to configure git authentication:", error);
// Continue anyway - git operations may still work with default config
}
} else {
// Commits go through the GitHub API, so no git user setup is needed, but
// the credential actions/checkout left in git config should still be
// replaced with the action's own.
try {
await replaceCheckoutCredentials(githubToken, context);
} catch (error) {
console.error("Failed to configure git credentials:", error);
// Continue anyway - git operations may still work with default config
}
}
// Create prompt directory. Clear any stale files from a prior invocation first —

View File

@ -3,6 +3,7 @@ import { createInitialComment } from "../../github/operations/comments/create-in
import { setupBranch } from "../../github/operations/branch";
import {
configureGitAuth,
replaceCheckoutCredentials,
setupSshSigning,
} from "../../github/operations/git-config";
import { prepareMcpConfig } from "../../mcp/install-mcp-server";
@ -98,6 +99,16 @@ export async function prepareTagMode({
console.error("Failed to configure git authentication:", error);
throw error;
}
} else {
// Commits go through the GitHub API, so no git user setup is needed, but
// the credential actions/checkout left in git config should still be
// replaced with the action's own.
try {
await replaceCheckoutCredentials(githubToken, context);
} catch (error) {
console.error("Failed to configure git credentials:", error);
throw error;
}
}
// Create prompt file

193
test/git-config.test.ts Normal file
View File

@ -0,0 +1,193 @@
import { afterEach, beforeEach, describe, expect, spyOn, test } from "bun:test";
import { execFileSync } from "child_process";
import { mkdtempSync, rmSync, statSync } from "fs";
import { tmpdir } from "os";
import { join } from "path";
import {
configureGitAuth,
replaceCheckoutCredentials,
} from "../src/github/operations/git-config";
import { GITHUB_SERVER_URL } from "../src/github/api/config";
import { createMockAutomationContext } from "./mockContext";
// Derive host-specific expectations from GITHUB_SERVER_URL so the suite passes
// on GHES runners (where Actions exports that variable) as well as github.com.
const SERVER = new URL(GITHUB_SERVER_URL);
const NOREPLY_DOMAIN =
SERVER.hostname === "github.com"
? "users.noreply.github.com"
: `users.noreply.${SERVER.hostname}`;
const EXTRAHEADER_KEY = `http.${GITHUB_SERVER_URL}/.extraheader`;
// git exports these into hooks (e.g. a pre-commit hook running the test
// suite); if inherited they would point every git command below at the
// enclosing repository instead of the temp repo.
const GIT_ENV_OVERRIDES = [
"GIT_DIR",
"GIT_WORK_TREE",
"GIT_INDEX_FILE",
"GIT_COMMON_DIR",
"GIT_PREFIX",
] as const;
// Pass an explicit env copy: unlike bun's `$`, execFileSync does not pick up
// deletions from process.env, so the GIT_* overrides removed in beforeEach
// would otherwise still reach the child process.
function runGit(args: string[], cwd?: string): string {
return execFileSync("git", args, {
cwd,
encoding: "utf8",
stdio: "pipe",
env: { ...process.env },
}).trim();
}
function gitConfigGetAll(key: string): string {
try {
return runGit(["config", "--local", "--get-all", key]);
} catch {
return "";
}
}
function remoteUrl(): string {
return runGit(["remote", "get-url", "origin"]);
}
describe("git-config", () => {
let originalCwd: string;
let tempDir: string;
let repoDir: string;
let originalActionPath: string | undefined;
let originalNonWriteUsers: string | undefined;
let originalGhToken: string | undefined;
let originalGitEnv: Record<string, string | undefined>;
let consoleLogSpy: any;
beforeEach(() => {
originalCwd = process.cwd();
originalActionPath = process.env.GITHUB_ACTION_PATH;
originalNonWriteUsers = process.env.ALLOWED_NON_WRITE_USERS;
originalGhToken = process.env.GH_TOKEN;
delete process.env.ALLOWED_NON_WRITE_USERS;
originalGitEnv = {};
for (const name of GIT_ENV_OVERRIDES) {
originalGitEnv[name] = process.env[name];
delete process.env[name];
}
tempDir = mkdtempSync(join(tmpdir(), "git-config-test-"));
repoDir = join(tempDir, "repo");
runGit(["init", repoDir]);
process.env.GITHUB_ACTION_PATH = tempDir;
process.chdir(repoDir);
git(["remote", "add", "origin", `https://${SERVER.host}/test/repo.git`]);
// Mimic the credential actions/checkout persists in the local config
git([
"config",
"--local",
"--add",
EXTRAHEADER_KEY,
"AUTHORIZATION: basic one",
]);
git([
"config",
"--local",
"--add",
EXTRAHEADER_KEY,
"AUTHORIZATION: basic two",
]);
git(["config", "--local", "user.name", "pre-existing"]);
consoleLogSpy = spyOn(console, "log").mockImplementation(() => {});
});
afterEach(() => {
process.chdir(originalCwd);
rmSync(tempDir, { recursive: true, force: true });
consoleLogSpy?.mockRestore();
restoreEnv("GITHUB_ACTION_PATH", originalActionPath);
restoreEnv("ALLOWED_NON_WRITE_USERS", originalNonWriteUsers);
restoreEnv("GH_TOKEN", originalGhToken);
for (const name of GIT_ENV_OVERRIDES) {
restoreEnv(name, originalGitEnv[name]);
}
});
describe("replaceCheckoutCredentials", () => {
test("removes the checkout extraheader and sets a token remote URL", async () => {
expect(gitConfigGetAll(EXTRAHEADER_KEY)).toContain("AUTHORIZATION");
await replaceCheckoutCredentials(
"test-token",
createMockAutomationContext(),
);
expect(gitConfigGetAll(EXTRAHEADER_KEY)).toBe("");
expect(remoteUrl()).toBe(
`https://x-access-token:test-token@${SERVER.host}/test-owner/test-repo.git`,
);
// Only the credential is touched — the git identity is left alone
expect(gitConfigGetAll("user.name")).toBe("pre-existing");
});
test("uses a credential helper when non-write users are allowed", async () => {
process.env.ALLOWED_NON_WRITE_USERS = "someone";
await replaceCheckoutCredentials(
"helper-token",
createMockAutomationContext(),
);
expect(gitConfigGetAll(EXTRAHEADER_KEY)).toBe("");
expect(remoteUrl()).toBe(
`https://${SERVER.host}/test-owner/test-repo.git`,
);
const helperPath = join(tempDir, ".git-credential-gh-token");
expect(gitConfigGetAll("credential.helper")).toBe(helperPath);
expect(statSync(helperPath).mode & 0o777).toBe(0o700);
expect(process.env.GH_TOKEN).toBe("helper-token");
});
test("succeeds when there is no checkout extraheader to remove", async () => {
git(["config", "--local", "--unset-all", EXTRAHEADER_KEY]);
await expect(
replaceCheckoutCredentials("test-token", createMockAutomationContext()),
).resolves.toBeUndefined();
expect(remoteUrl()).toContain("x-access-token:test-token@");
});
});
describe("configureGitAuth", () => {
test("configures the git user and replaces the checkout credential", async () => {
await configureGitAuth("test-token", createMockAutomationContext(), {
login: "claude[bot]",
id: 42,
});
expect(gitConfigGetAll("user.name")).toBe("claude[bot]");
expect(gitConfigGetAll("user.email")).toBe(
`42+claude[bot]@${NOREPLY_DOMAIN}`,
);
expect(gitConfigGetAll(EXTRAHEADER_KEY)).toBe("");
expect(remoteUrl()).toBe(
`https://x-access-token:test-token@${SERVER.host}/test-owner/test-repo.git`,
);
});
});
function git(args: string[]): void {
runGit(args, repoDir);
}
});
function restoreEnv(name: string, value: string | undefined): void {
if (value === undefined) {
delete process.env[name];
} else {
process.env[name] = value;
}
}

View File

@ -16,28 +16,35 @@ describe("Agent Mode", () => {
let exportVariableSpy: any;
let setOutputSpy: any;
let configureGitAuthSpy: any;
let replaceCheckoutCredentialsSpy: any;
beforeEach(() => {
exportVariableSpy = spyOn(core, "exportVariable").mockImplementation(
() => {},
);
setOutputSpy = spyOn(core, "setOutput").mockImplementation(() => {});
// Mock configureGitAuth to prevent actual git commands from running
// Mock git configuration to prevent actual git commands from running
configureGitAuthSpy = spyOn(
gitConfig,
"configureGitAuth",
).mockImplementation(async () => {
// Do nothing - prevent actual git config modifications
});
replaceCheckoutCredentialsSpy = spyOn(
gitConfig,
"replaceCheckoutCredentials",
).mockImplementation(async () => {});
});
afterEach(() => {
exportVariableSpy?.mockClear();
setOutputSpy?.mockClear();
configureGitAuthSpy?.mockClear();
replaceCheckoutCredentialsSpy?.mockClear();
exportVariableSpy?.mockRestore();
setOutputSpy?.mockRestore();
configureGitAuthSpy?.mockRestore();
replaceCheckoutCredentialsSpy?.mockRestore();
});
test("prepareAgentMode is exported as a function", () => {
@ -257,4 +264,59 @@ describe("Agent Mode", () => {
// Should be empty or just whitespace when no MCP servers are included
expect(result.claudeArgs).not.toContain("--mcp-config");
});
describe("git credential configuration", () => {
const mockOctokit = {
rest: {
users: {
getByUsername: mock(() =>
Promise.resolve({
data: { login: "test-user", id: 12345, type: "User" },
}),
),
},
},
} as any;
test("uses full git auth on the non-signing path", async () => {
const context = createMockAutomationContext({
eventName: "workflow_dispatch",
});
await prepareAgentMode({
context,
octokit: mockOctokit,
githubToken: "test-token",
});
expect(configureGitAuthSpy).toHaveBeenCalledTimes(1);
expect(configureGitAuthSpy).toHaveBeenCalledWith("test-token", context, {
login: context.inputs.botName,
id: parseInt(context.inputs.botId),
});
// configureGitAuth performs the credential replacement itself; the mock
// stands in for it here, so the standalone helper is not invoked.
expect(replaceCheckoutCredentialsSpy).not.toHaveBeenCalled();
});
test("still replaces the checkout credential when API commit signing is enabled", async () => {
const context = createMockAutomationContext({
eventName: "workflow_dispatch",
inputs: { useCommitSigning: true },
});
await prepareAgentMode({
context,
octokit: mockOctokit,
githubToken: "test-token",
});
expect(configureGitAuthSpy).not.toHaveBeenCalled();
expect(replaceCheckoutCredentialsSpy).toHaveBeenCalledTimes(1);
expect(replaceCheckoutCredentialsSpy).toHaveBeenCalledWith(
"test-token",
context,
);
});
});
});

View File

@ -1,8 +1,99 @@
import { describe, test, expect } from "bun:test";
import { describe, test, expect, beforeEach, afterEach, spyOn } from "bun:test";
import { prepareTagMode } from "../../src/modes/tag";
import { mockIssueCommentContext } from "../mockContext";
import * as actor from "../../src/github/validation/actor";
import * as createInitial from "../../src/github/operations/comments/create-initial";
import * as fetcher from "../../src/github/data/fetcher";
import * as branch from "../../src/github/operations/branch";
import * as createPrompt from "../../src/create-prompt";
import * as mcp from "../../src/mcp/install-mcp-server";
import * as gitConfig from "../../src/github/operations/git-config";
describe("Tag Mode", () => {
test("prepareTagMode is exported as a function", () => {
expect(typeof prepareTagMode).toBe("function");
});
describe("git credential configuration", () => {
let spies: Array<{ mockRestore: () => void }>;
let configureGitAuthSpy: any;
let replaceCheckoutCredentialsSpy: any;
beforeEach(() => {
configureGitAuthSpy = spyOn(
gitConfig,
"configureGitAuth",
).mockImplementation(async () => {});
replaceCheckoutCredentialsSpy = spyOn(
gitConfig,
"replaceCheckoutCredentials",
).mockImplementation(async () => {});
spies = [
configureGitAuthSpy,
replaceCheckoutCredentialsSpy,
spyOn(actor, "checkHumanActor").mockImplementation(async () => {}),
spyOn(createInitial, "createInitialComment").mockImplementation(
async () => ({ id: 42 }) as any,
),
spyOn(fetcher, "fetchGitHubData").mockImplementation(
async () => ({}) as any,
),
spyOn(branch, "setupBranch").mockImplementation(
async () =>
({
baseBranch: "main",
claudeBranch: "claude/test",
currentBranch: "claude/test",
}) as any,
),
spyOn(createPrompt, "createPrompt").mockImplementation(async () => {}),
spyOn(mcp, "prepareMcpConfig").mockImplementation(async () => "{}"),
];
});
afterEach(() => {
for (const spy of spies) {
spy.mockRestore();
}
});
test("uses full git auth on the non-signing path", async () => {
const context = { ...mockIssueCommentContext };
await prepareTagMode({
context,
octokit: {} as any,
githubToken: "test-token",
});
expect(configureGitAuthSpy).toHaveBeenCalledTimes(1);
expect(configureGitAuthSpy).toHaveBeenCalledWith("test-token", context, {
login: context.inputs.botName,
id: parseInt(context.inputs.botId),
});
// configureGitAuth performs the credential replacement itself; the mock
// stands in for it here, so the standalone helper is not invoked.
expect(replaceCheckoutCredentialsSpy).not.toHaveBeenCalled();
});
test("still replaces the checkout credential when API commit signing is enabled", async () => {
const context = {
...mockIssueCommentContext,
inputs: { ...mockIssueCommentContext.inputs, useCommitSigning: true },
};
await prepareTagMode({
context,
octokit: {} as any,
githubToken: "test-token",
});
expect(configureGitAuthSpy).not.toHaveBeenCalled();
expect(replaceCheckoutCredentialsSpy).toHaveBeenCalledTimes(1);
expect(replaceCheckoutCredentialsSpy).toHaveBeenCalledWith(
"test-token",
context,
);
});
});
});