diff --git a/actions/setup/js/safe_outputs_handlers.cjs b/actions/setup/js/safe_outputs_handlers.cjs index 4ad3e221642..36c2f315a84 100644 --- a/actions/setup/js/safe_outputs_handlers.cjs +++ b/actions/setup/js/safe_outputs_handlers.cjs @@ -194,6 +194,39 @@ function resolvePatchWorkspacePath(workspacePath) { return { success: true, absolutePath: resolved }; } +/** + * Ensure the current git checkout path is trusted in this process context. + * Injects safe.directory via GIT_CONFIG_COUNT/KEY/VALUE env vars so that all + * subsequent git commands in this process inherit the trust without writing to + * ~/.gitconfig (no persistent global git config side effects). + * + * GIT_CONFIG_COUNT/KEY/VALUE is supported since git 2.31. + * + * @param {string} gitCwd + * @param {{ debug: (message: string) => void }} server + * @returns {void} + */ +function ensureSafeDirectoryTrust(gitCwd, server) { + if (!gitCwd) return; + + // Check if gitCwd is already present in the injected env-var config to avoid + // duplicate entries when the handler is called more than once in the same process. + // The `|| 0` guard converts NaN (from a malformed pre-existing GIT_CONFIG_COUNT) to 0, + // preventing GIT_CONFIG_KEY_NaN/VALUE_NaN entries that would corrupt the env-var config chain. + const existingCount = parseInt(process.env.GIT_CONFIG_COUNT || "0", 10) || 0; + for (let i = 0; i < existingCount; i++) { + if (process.env[`GIT_CONFIG_KEY_${i}`] === "safe.directory" && process.env[`GIT_CONFIG_VALUE_${i}`] === gitCwd) { + return; + } + } + + const idx = existingCount; + process.env.GIT_CONFIG_COUNT = String(existingCount + 1); + process.env[`GIT_CONFIG_KEY_${idx}`] = "safe.directory"; + process.env[`GIT_CONFIG_VALUE_${idx}`] = gitCwd; + server.debug(`Configured git safe.directory for bridge context: ${gitCwd}`); +} + /** * Create handlers for safe output tools * @param {Object} server - The MCP server instance for logging @@ -752,6 +785,7 @@ function createHandlers(server, appendSafeOutput, config = {}) { // This prevents TOCTOU races where the agent flips the ref between patch and bundle // generation, causing the two to represent different commit sets. const gitCwd = repoCwd || process.env.GITHUB_WORKSPACE || process.cwd(); + ensureSafeDirectoryTrust(gitCwd, server); let pinnedSha; try { pinnedSha = execGitSync(["rev-parse", "--verify", `refs/heads/${entry.branch}^{commit}`], { cwd: gitCwd }) @@ -1196,6 +1230,7 @@ function createHandlers(server, appendSafeOutput, config = {}) { // This prevents TOCTOU races where the agent flips the ref between patch and bundle // generation, causing the two to represent different commit sets. const pushGitCwd = repoCwd || process.env.GITHUB_WORKSPACE || process.cwd(); + ensureSafeDirectoryTrust(pushGitCwd, server); let pushPinnedSha; try { pushPinnedSha = execGitSync(["rev-parse", "--verify", `refs/heads/${entry.branch}^{commit}`], { cwd: pushGitCwd }) diff --git a/actions/setup/js/safe_outputs_handlers.test.cjs b/actions/setup/js/safe_outputs_handlers.test.cjs index 1c0434056b1..de6a1723d41 100644 --- a/actions/setup/js/safe_outputs_handlers.test.cjs +++ b/actions/setup/js/safe_outputs_handlers.test.cjs @@ -739,6 +739,39 @@ describe("safe_outputs_handlers", () => { expect(mockAppendSafeOutput).not.toHaveBeenCalled(); }); + it("injects GITHUB_WORKSPACE into GIT_CONFIG env vars for safe.directory before branch pinning", async () => { + const prevCount = process.env.GIT_CONFIG_COUNT; + const prevKeys = Object.fromEntries(Object.entries(process.env).filter(([k]) => /^GIT_CONFIG_(KEY|VALUE)_\d+$/.test(k))); + // Clear any pre-existing GIT_CONFIG_COUNT so the injection starts at index 0. + delete process.env.GIT_CONFIG_COUNT; + for (const key of Object.keys(prevKeys)) delete process.env[key]; + try { + await handlers.createPullRequestHandler({ + branch: "feature-branch", + title: "Test PR", + body: "Test description", + }); + + const count = parseInt(process.env.GIT_CONFIG_COUNT || "0", 10); + const injected = []; + for (let i = 0; i < count; i++) { + if (process.env[`GIT_CONFIG_KEY_${i}`] === "safe.directory") { + injected.push(process.env[`GIT_CONFIG_VALUE_${i}`]); + } + } + expect(injected).toContain(testWorkspaceDir); + } finally { + // Restore original env state. + if (prevCount === undefined) delete process.env.GIT_CONFIG_COUNT; + else process.env.GIT_CONFIG_COUNT = prevCount; + // Remove any GIT_CONFIG_KEY/VALUE vars added during the test, then restore originals. + for (const key of Object.keys(process.env).filter(k => /^GIT_CONFIG_(KEY|VALUE)_\d+$/.test(k))) { + delete process.env[key]; + } + for (const [key, value] of Object.entries(prevKeys)) process.env[key] = value; + } + }); + it("should include helpful details in error response", async () => { const args = { branch: "test-branch", @@ -1208,6 +1241,37 @@ describe("safe_outputs_handlers", () => { expect(mockAppendSafeOutput).not.toHaveBeenCalled(); }); + it("injects GITHUB_WORKSPACE into GIT_CONFIG env vars for safe.directory before push branch pinning", async () => { + const prevCount = process.env.GIT_CONFIG_COUNT; + const prevKeys = Object.fromEntries(Object.entries(process.env).filter(([k]) => /^GIT_CONFIG_(KEY|VALUE)_\d+$/.test(k))); + // Clear any pre-existing GIT_CONFIG_COUNT so the injection starts at index 0. + delete process.env.GIT_CONFIG_COUNT; + for (const key of Object.keys(prevKeys)) delete process.env[key]; + try { + await handlers.pushToPullRequestBranchHandler({ + branch: "feature-branch", + }); + + const count = parseInt(process.env.GIT_CONFIG_COUNT || "0", 10); + const injected = []; + for (let i = 0; i < count; i++) { + if (process.env[`GIT_CONFIG_KEY_${i}`] === "safe.directory") { + injected.push(process.env[`GIT_CONFIG_VALUE_${i}`]); + } + } + expect(injected).toContain(testWorkspaceDir); + } finally { + // Restore original env state. + if (prevCount === undefined) delete process.env.GIT_CONFIG_COUNT; + else process.env.GIT_CONFIG_COUNT = prevCount; + // Remove any GIT_CONFIG_KEY/VALUE vars added during the test, then restore originals. + for (const key of Object.keys(process.env).filter(k => /^GIT_CONFIG_(KEY|VALUE)_\d+$/.test(k))) { + delete process.env[key]; + } + for (const [key, value] of Object.entries(prevKeys)) process.env[key] = value; + } + }); + it("should require explicit pull_request_number when push_to_pull_request_branch target is '*'", async () => { const wildcardHandlers = createHandlers(mockServer, mockAppendSafeOutput, { push_to_pull_request_branch: {