diff --git a/src/github/operations/git-config.ts b/src/github/operations/git-config.ts index 3df584ba..535a75c4 100644 --- a/src/github/operations/git-config.ts +++ b/src/github/operations/git-config.ts @@ -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./.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"); } /** diff --git a/src/modes/agent/index.ts b/src/modes/agent/index.ts index aca8d53c..d52ac0c8 100644 --- a/src/modes/agent/index.ts +++ b/src/modes/agent/index.ts @@ -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 — diff --git a/src/modes/tag/index.ts b/src/modes/tag/index.ts index c6de58dd..54e838a4 100644 --- a/src/modes/tag/index.ts +++ b/src/modes/tag/index.ts @@ -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 diff --git a/test/git-config.test.ts b/test/git-config.test.ts new file mode 100644 index 00000000..e697698b --- /dev/null +++ b/test/git-config.test.ts @@ -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; + 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; + } +} diff --git a/test/modes/agent.test.ts b/test/modes/agent.test.ts index 1404b0d6..8f0892d5 100644 --- a/test/modes/agent.test.ts +++ b/test/modes/agent.test.ts @@ -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, + ); + }); + }); }); diff --git a/test/modes/tag.test.ts b/test/modes/tag.test.ts index d68d7fc2..beac188c 100644 --- a/test/modes/tag.test.ts +++ b/test/modes/tag.test.ts @@ -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, + ); + }); + }); });