From 28e9b40d58e74c510d7829f296257f6acbbb8d50 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sat, 27 Jun 2026 02:29:43 +0000 Subject: [PATCH 1/5] Initial plan From 6ab65a8c85c063be83611c400b1bbc69926072dd Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sat, 27 Jun 2026 02:37:09 +0000 Subject: [PATCH 2/5] Apply remaining changes Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- actions/setup/js/safe_outputs_handlers.cjs | 32 ++++++++++++ .../setup/js/safe_outputs_handlers.test.cjs | 52 +++++++++++++++++++ 2 files changed, 84 insertions(+) diff --git a/actions/setup/js/safe_outputs_handlers.cjs b/actions/setup/js/safe_outputs_handlers.cjs index 4ad3e221642..a5f31e66175 100644 --- a/actions/setup/js/safe_outputs_handlers.cjs +++ b/actions/setup/js/safe_outputs_handlers.cjs @@ -194,6 +194,36 @@ function resolvePatchWorkspacePath(workspacePath) { return { success: true, absolutePath: resolved }; } +/** + * Ensure the current git checkout path is trusted in this process HOME context. + * This prevents "detected dubious ownership in repository" failures when safe + * outputs run via a bridge process using a different HOME than the container. + * + * @param {string} gitCwd + * @param {{ debug: (message: string) => void }} server + * @returns {void} + */ +function ensureSafeDirectoryTrust(gitCwd, server) { + if (!gitCwd) return; + try { + const existingEntries = execGitSync(["config", "--global", "--get-all", "safe.directory"], { suppressLogs: true }) + .split("\n") + .map(value => value.trim()) + .filter(Boolean); + if (existingEntries.includes(gitCwd)) { + return; + } + } catch { + // `git config --get-all` exits non-zero when no values are configured yet. + } + try { + execGitSync(["config", "--global", "--add", "safe.directory", gitCwd], { suppressLogs: true }); + server.debug(`Configured git safe.directory for bridge context: ${gitCwd}`); + } catch (error) { + server.debug(`Failed to configure git safe.directory '${gitCwd}': ${getErrorMessage(error)}`); + } +} + /** * Create handlers for safe output tools * @param {Object} server - The MCP server instance for logging @@ -752,6 +782,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 +1227,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..2990f4fc716 100644 --- a/actions/setup/js/safe_outputs_handlers.test.cjs +++ b/actions/setup/js/safe_outputs_handlers.test.cjs @@ -739,6 +739,33 @@ describe("safe_outputs_handlers", () => { expect(mockAppendSafeOutput).not.toHaveBeenCalled(); }); + it("adds GITHUB_WORKSPACE to git safe.directory in the current HOME context before branch pinning", async () => { + const testHome = path.join(testWorkspaceDir, "home-safe-directory-create-pr"); + fs.mkdirSync(testHome, { recursive: true }); + const previousHome = process.env.HOME; + process.env.HOME = testHome; + try { + await handlers.createPullRequestHandler({ + branch: "feature-branch", + title: "Test PR", + body: "Test description", + }); + + const safeDirectories = execSync("git config --global --get-all safe.directory", { + env: { ...process.env, HOME: testHome }, + stdio: "pipe", + }) + .toString() + .split("\n") + .map(line => line.trim()) + .filter(Boolean); + expect(safeDirectories).toContain(testWorkspaceDir); + } finally { + if (previousHome === undefined) delete process.env.HOME; + else process.env.HOME = previousHome; + } + }); + it("should include helpful details in error response", async () => { const args = { branch: "test-branch", @@ -1208,6 +1235,31 @@ describe("safe_outputs_handlers", () => { expect(mockAppendSafeOutput).not.toHaveBeenCalled(); }); + it("adds GITHUB_WORKSPACE to git safe.directory in the current HOME context before push branch pinning", async () => { + const testHome = path.join(testWorkspaceDir, "home-safe-directory-push-pr-branch"); + fs.mkdirSync(testHome, { recursive: true }); + const previousHome = process.env.HOME; + process.env.HOME = testHome; + try { + await handlers.pushToPullRequestBranchHandler({ + branch: "feature-branch", + }); + + const safeDirectories = execSync("git config --global --get-all safe.directory", { + env: { ...process.env, HOME: testHome }, + stdio: "pipe", + }) + .toString() + .split("\n") + .map(line => line.trim()) + .filter(Boolean); + expect(safeDirectories).toContain(testWorkspaceDir); + } finally { + if (previousHome === undefined) delete process.env.HOME; + else process.env.HOME = previousHome; + } + }); + 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: { From 9c75a578807368f8158b19545aa8da5d64fbb2da Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sat, 27 Jun 2026 07:11:13 +0000 Subject: [PATCH 3/5] safe_outputs: use GIT_CONFIG_COUNT env vars for safe.directory trust Replace the git config --global approach with process-scoped GIT_CONFIG_COUNT/KEY/VALUE env var injection so that all subsequent git commands in the same process inherit the safe.directory trust without writing to ~/.gitconfig. This eliminates the persistent global git config side effect (no file writes) and removes the failure mode that was previously only surfaced at debug level. The env var approach is supported by git 2.31+ and carries over to all git calls made by generateGitPatch and generateGitBundle in the same process. Update tests to verify env var injection instead of checking the global git config file, and fix the finally-block cleanup to delete all GIT_CONFIG_KEY/VALUE entries added during the test before restoring originals. Co-authored-by: gh-aw-bot <259018956+gh-aw-bot@users.noreply.github.com> --- actions/setup/js/safe_outputs_handlers.cjs | 35 ++++----- .../setup/js/safe_outputs_handlers.test.cjs | 76 +++++++++++-------- 2 files changed, 62 insertions(+), 49 deletions(-) diff --git a/actions/setup/js/safe_outputs_handlers.cjs b/actions/setup/js/safe_outputs_handlers.cjs index a5f31e66175..d878ae3a0ec 100644 --- a/actions/setup/js/safe_outputs_handlers.cjs +++ b/actions/setup/js/safe_outputs_handlers.cjs @@ -195,9 +195,12 @@ function resolvePatchWorkspacePath(workspacePath) { } /** - * Ensure the current git checkout path is trusted in this process HOME context. - * This prevents "detected dubious ownership in repository" failures when safe - * outputs run via a bridge process using a different HOME than the container. + * 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 @@ -205,23 +208,21 @@ function resolvePatchWorkspacePath(workspacePath) { */ function ensureSafeDirectoryTrust(gitCwd, server) { if (!gitCwd) return; - try { - const existingEntries = execGitSync(["config", "--global", "--get-all", "safe.directory"], { suppressLogs: true }) - .split("\n") - .map(value => value.trim()) - .filter(Boolean); - if (existingEntries.includes(gitCwd)) { + + // 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. + const existingCount = parseInt(process.env.GIT_CONFIG_COUNT || "0", 10); + for (let i = 0; i < existingCount; i++) { + if (process.env[`GIT_CONFIG_KEY_${i}`] === "safe.directory" && process.env[`GIT_CONFIG_VALUE_${i}`] === gitCwd) { return; } - } catch { - // `git config --get-all` exits non-zero when no values are configured yet. - } - try { - execGitSync(["config", "--global", "--add", "safe.directory", gitCwd], { suppressLogs: true }); - server.debug(`Configured git safe.directory for bridge context: ${gitCwd}`); - } catch (error) { - server.debug(`Failed to configure git safe.directory '${gitCwd}': ${getErrorMessage(error)}`); } + + 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}`); } /** diff --git a/actions/setup/js/safe_outputs_handlers.test.cjs b/actions/setup/js/safe_outputs_handlers.test.cjs index 2990f4fc716..de6a1723d41 100644 --- a/actions/setup/js/safe_outputs_handlers.test.cjs +++ b/actions/setup/js/safe_outputs_handlers.test.cjs @@ -739,11 +739,12 @@ describe("safe_outputs_handlers", () => { expect(mockAppendSafeOutput).not.toHaveBeenCalled(); }); - it("adds GITHUB_WORKSPACE to git safe.directory in the current HOME context before branch pinning", async () => { - const testHome = path.join(testWorkspaceDir, "home-safe-directory-create-pr"); - fs.mkdirSync(testHome, { recursive: true }); - const previousHome = process.env.HOME; - process.env.HOME = testHome; + 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", @@ -751,18 +752,23 @@ describe("safe_outputs_handlers", () => { body: "Test description", }); - const safeDirectories = execSync("git config --global --get-all safe.directory", { - env: { ...process.env, HOME: testHome }, - stdio: "pipe", - }) - .toString() - .split("\n") - .map(line => line.trim()) - .filter(Boolean); - expect(safeDirectories).toContain(testWorkspaceDir); + 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 { - if (previousHome === undefined) delete process.env.HOME; - else process.env.HOME = previousHome; + // 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; } }); @@ -1235,28 +1241,34 @@ describe("safe_outputs_handlers", () => { expect(mockAppendSafeOutput).not.toHaveBeenCalled(); }); - it("adds GITHUB_WORKSPACE to git safe.directory in the current HOME context before push branch pinning", async () => { - const testHome = path.join(testWorkspaceDir, "home-safe-directory-push-pr-branch"); - fs.mkdirSync(testHome, { recursive: true }); - const previousHome = process.env.HOME; - process.env.HOME = testHome; + 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 safeDirectories = execSync("git config --global --get-all safe.directory", { - env: { ...process.env, HOME: testHome }, - stdio: "pipe", - }) - .toString() - .split("\n") - .map(line => line.trim()) - .filter(Boolean); - expect(safeDirectories).toContain(testWorkspaceDir); + 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 { - if (previousHome === undefined) delete process.env.HOME; - else process.env.HOME = previousHome; + // 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; } }); From 49d50203e42b293b269d5975eaa4bfdbe2c78a03 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sat, 27 Jun 2026 21:59:11 +0000 Subject: [PATCH 4/5] fix(safe_outputs): guard parseInt with || 0 to prevent NaN corruption of GIT_CONFIG_COUNT chain Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- actions/setup/js/safe_outputs_handlers.cjs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/actions/setup/js/safe_outputs_handlers.cjs b/actions/setup/js/safe_outputs_handlers.cjs index d878ae3a0ec..36c2f315a84 100644 --- a/actions/setup/js/safe_outputs_handlers.cjs +++ b/actions/setup/js/safe_outputs_handlers.cjs @@ -211,7 +211,9 @@ function ensureSafeDirectoryTrust(gitCwd, server) { // 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. - const existingCount = parseInt(process.env.GIT_CONFIG_COUNT || "0", 10); + // 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; From 43ea018b65677b6f06139091e46374bc7a3beb97 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sun, 28 Jun 2026 22:46:36 +0000 Subject: [PATCH 5/5] safe_outputs: add createHandlers-level guard + dedup test for ensureSafeDirectoryTrust Co-authored-by: gh-aw-bot <259018956+gh-aw-bot@users.noreply.github.com> --- actions/setup/js/safe_outputs_handlers.cjs | 5 +++ .../setup/js/safe_outputs_handlers.test.cjs | 37 +++++++++++++++++++ 2 files changed, 42 insertions(+) diff --git a/actions/setup/js/safe_outputs_handlers.cjs b/actions/setup/js/safe_outputs_handlers.cjs index 36c2f315a84..38fe3e5ed33 100644 --- a/actions/setup/js/safe_outputs_handlers.cjs +++ b/actions/setup/js/safe_outputs_handlers.cjs @@ -235,6 +235,11 @@ function ensureSafeDirectoryTrust(gitCwd, server) { * @returns {Object} An object containing all handler functions */ function createHandlers(server, appendSafeOutput, config = {}) { + // Ensure the workspace is trusted for the lifetime of this server process. + // This covers all current and future handlers automatically; per-handler calls + // additionally cover per-request checkout paths that differ from GITHUB_WORKSPACE. + ensureSafeDirectoryTrust(process.env.GITHUB_WORKSPACE || process.cwd(), server); + const TOKEN_THRESHOLD = 16000; /** diff --git a/actions/setup/js/safe_outputs_handlers.test.cjs b/actions/setup/js/safe_outputs_handlers.test.cjs index de6a1723d41..a1471501b5a 100644 --- a/actions/setup/js/safe_outputs_handlers.test.cjs +++ b/actions/setup/js/safe_outputs_handlers.test.cjs @@ -772,6 +772,43 @@ describe("safe_outputs_handlers", () => { } }); + it("does not add duplicate safe.directory entries on repeated handler calls with the same path", 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))); + delete process.env.GIT_CONFIG_COUNT; + for (const key of Object.keys(prevKeys)) delete process.env[key]; + try { + // Two calls simulate a retry or concurrent invocation with the same workspace path. + await handlers.createPullRequestHandler({ + branch: "feature-branch", + title: "Test PR", + body: "Test description", + }); + await handlers.createPullRequestHandler({ + branch: "feature-branch", + title: "Test PR", + body: "Test description", + }); + + const count = parseInt(process.env.GIT_CONFIG_COUNT || "0", 10); + const safeDirectories = []; + for (let i = 0; i < count; i++) { + if (process.env[`GIT_CONFIG_KEY_${i}`] === "safe.directory") { + safeDirectories.push(process.env[`GIT_CONFIG_VALUE_${i}`]); + } + } + // The workspace path must appear exactly once — no duplicate growth. + expect(safeDirectories.filter(d => d === testWorkspaceDir)).toHaveLength(1); + } finally { + if (prevCount === undefined) delete process.env.GIT_CONFIG_COUNT; + else process.env.GIT_CONFIG_COUNT = prevCount; + 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",