Skip to content
37 changes: 37 additions & 0 deletions actions/setup/js/git_helpers.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,42 @@ function getGitAuthEnv(token) {
};
}

/**
* Ensure the given directory is trusted for git in the current 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).
*
* This is specifically needed for the "bridge" path (the Process Safe Outputs step
* running outside the Docker container) where HOME or the uid may differ from the
* in-container user that originally configured safe.directory in ~/.gitconfig.
*
* GIT_CONFIG_COUNT/KEY/VALUE is supported since git 2.31.
*
* @param {string} gitCwd - The directory to trust (e.g. GITHUB_WORKSPACE or a repo checkout path)
* @returns {void}
*/
function ensureSafeDirectoryTrust(gitCwd) {
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;
Comment thread
github-actions[bot] marked this conversation as resolved.
Outdated
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;
core.debug(`Configured git safe.directory for bridge context: ${gitCwd}`);
}

/**
* Safely execute git command using spawnSync with args array to prevent shell injection.
*
Expand Down Expand Up @@ -603,6 +639,7 @@ async function linearizeRangeAsCommit(baseRef, commitMessage, execApi, opts = {}
}

module.exports = {
ensureSafeDirectoryTrust,
execGitSync,
backfillCommitObjects,
ensureFullHistoryForBundle,
Expand Down
108 changes: 108 additions & 0 deletions actions/setup/js/git_helpers.test.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -282,6 +282,114 @@ describe("git_helpers.cjs", () => {
});
});

describe("ensureSafeDirectoryTrust", () => {
let originalEnv;

beforeEach(() => {
originalEnv = { ...process.env };
// Clean up GIT_CONFIG_* vars injected by a previous test
for (const key of Object.keys(process.env)) {
if (key.startsWith("GIT_CONFIG_")) {
delete process.env[key];
}
}
});

afterEach(() => {
// Restore to original, removing any vars added during the test
for (const key of Object.keys(process.env)) {
if (!(key in originalEnv)) {
delete process.env[key];
}
}
Object.assign(process.env, originalEnv);
});

it("should export ensureSafeDirectoryTrust function", async () => {
const { ensureSafeDirectoryTrust } = await import("./git_helpers.cjs");
expect(typeof ensureSafeDirectoryTrust).toBe("function");
});

it("should set GIT_CONFIG_* env vars for the given directory", async () => {
const { ensureSafeDirectoryTrust } = await import("./git_helpers.cjs");

ensureSafeDirectoryTrust("/workspace/repo");

expect(process.env.GIT_CONFIG_COUNT).toBe("1");
expect(process.env.GIT_CONFIG_KEY_0).toBe("safe.directory");
expect(process.env.GIT_CONFIG_VALUE_0).toBe("/workspace/repo");
});

it("should not add a duplicate entry when called twice with the same directory", async () => {
const { ensureSafeDirectoryTrust } = await import("./git_helpers.cjs");

ensureSafeDirectoryTrust("/workspace/repo");
ensureSafeDirectoryTrust("/workspace/repo");

expect(process.env.GIT_CONFIG_COUNT).toBe("1");
});

it("should append a new entry when called with a different directory", async () => {
const { ensureSafeDirectoryTrust } = await import("./git_helpers.cjs");

ensureSafeDirectoryTrust("/workspace/repo-a");
ensureSafeDirectoryTrust("/workspace/repo-b");

expect(process.env.GIT_CONFIG_COUNT).toBe("2");
expect(process.env.GIT_CONFIG_KEY_0).toBe("safe.directory");
expect(process.env.GIT_CONFIG_VALUE_0).toBe("/workspace/repo-a");
expect(process.env.GIT_CONFIG_KEY_1).toBe("safe.directory");
expect(process.env.GIT_CONFIG_VALUE_1).toBe("/workspace/repo-b");
});

it("should be a no-op when called with an empty string", async () => {
const { ensureSafeDirectoryTrust } = await import("./git_helpers.cjs");

ensureSafeDirectoryTrust("");

expect(process.env.GIT_CONFIG_COUNT).toBeUndefined();
});

it("should be a no-op when called with undefined/falsy", async () => {
const { ensureSafeDirectoryTrust } = await import("./git_helpers.cjs");

ensureSafeDirectoryTrust(undefined);

expect(process.env.GIT_CONFIG_COUNT).toBeUndefined();
});

it("should preserve existing GIT_CONFIG_* entries set by getGitAuthEnv", async () => {
const { ensureSafeDirectoryTrust, getGitAuthEnv } = await import("./git_helpers.cjs");

// Simulate what getGitAuthEnv returns being already applied via env
const authEnv = getGitAuthEnv("test-token");
Object.assign(process.env, authEnv);

ensureSafeDirectoryTrust("/workspace/repo");

// The count should be incremented by 1 (from 1 to 2)
expect(parseInt(process.env.GIT_CONFIG_COUNT, 10)).toBe(2);
// Existing auth entry preserved
expect(process.env.GIT_CONFIG_KEY_0).toBe(authEnv.GIT_CONFIG_KEY_0);
expect(process.env.GIT_CONFIG_VALUE_0).toBe(authEnv.GIT_CONFIG_VALUE_0);
// New safe.directory entry appended
expect(process.env.GIT_CONFIG_KEY_1).toBe("safe.directory");
expect(process.env.GIT_CONFIG_VALUE_1).toBe("/workspace/repo");
});

it("should handle a malformed GIT_CONFIG_COUNT gracefully", async () => {
const { ensureSafeDirectoryTrust } = await import("./git_helpers.cjs");

process.env.GIT_CONFIG_COUNT = "not-a-number";

ensureSafeDirectoryTrust("/workspace/repo");

expect(process.env.GIT_CONFIG_COUNT).toBe("1");
expect(process.env.GIT_CONFIG_KEY_0).toBe("safe.directory");
expect(process.env.GIT_CONFIG_VALUE_0).toBe("/workspace/repo");
});
});

describe("ensureFullHistoryForBundle", () => {
it("should unshallow the repository when the repository is shallow", async () => {
const { ensureFullHistoryForBundle } = await import("./git_helpers.cjs");
Expand Down
17 changes: 15 additions & 2 deletions actions/setup/js/push_to_pull_request_branch.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ const { checkFileProtection, checkFileProtectionPostApply } = require("./manifes
const { buildWorkflowRunUrl } = require("./workflow_metadata_helpers.cjs");
const { renderTemplateFromFile, buildProtectedFileList, getPromptPath } = require("./messages_core.cjs");
const { overridePersistedExtraheader, restorePersistedExtraheader } = require("./git_auth_helpers.cjs");
const { ensureFullHistoryForBundle, extractBundlePrerequisiteCommits, isShallowOrSparseCheckout, linearizeRangeAsCommit } = require("./git_helpers.cjs");
const { ensureFullHistoryForBundle, extractBundlePrerequisiteCommits, isShallowOrSparseCheckout, linearizeRangeAsCommit, ensureSafeDirectoryTrust } = require("./git_helpers.cjs");
const { normalizeCommitSHA } = require("./commit_sha_helpers.cjs");
const { findRepoCheckout } = require("./find_repo_checkout.cjs");
const { getThreatDetectedMarker } = require("./threat_detection_warning.cjs");
Expand Down Expand Up @@ -326,6 +326,13 @@ async function main(config = {}) {
const { defaultTargetRepo, allowedRepos } = resolveTargetRepoConfig(config);
const githubClient = await createAuthenticatedGitHubClient(config);

// Ensure the workspace is trusted in the bridge process (Process Safe Outputs step).
// The bridge runs outside the Docker container as a potentially different user/HOME,
// so the in-container `git config --global safe.directory` may not be visible here.
// Using GIT_CONFIG_* env vars avoids relying on ~/.gitconfig and covers cases where
// the conditional "Configure Git credentials" step was skipped or HOME differs.
ensureSafeDirectoryTrust(process.env.GITHUB_WORKSPACE || process.cwd());

// Git network operations authenticate using the credentials actions/checkout
// persisted into .git/config for the safe_outputs job (persist-credentials: true,
// using the resolved push token). We intentionally do NOT inject an additional
Expand Down Expand Up @@ -667,6 +674,12 @@ async function main(config = {}) {

// Base options for all git exec calls - includes cwd when running in a subdirectory checkout
const baseGitOpts = repoCwd ? { cwd: repoCwd } : {};

// For cross-repo checkouts, also trust the specific subdirectory. The factory-level call
// covers GITHUB_WORKSPACE; this per-message call covers subdirectory checkout paths.
if (repoCwd) {
ensureSafeDirectoryTrust(repoCwd);
}
let pullRequest;
try {
const response = await githubClient.rest.pulls.get({
Expand Down Expand Up @@ -1106,7 +1119,7 @@ async function main(config = {}) {
} catch {
// Ignore
}
return { success: false, error: "Failed to apply bundle" };
return { success: false, error: `Failed to apply bundle: ${getErrorMessage(bundleError)}` };
}
} else {
// Patch transport (non-default): git am --3way
Expand Down
41 changes: 4 additions & 37 deletions actions/setup/js/safe_outputs_handlers.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
const { lookupCheckout } = require("./checkout_manifest.cjs");
const { generateGitPatch } = require("./generate_git_patch.cjs");
const { generateGitBundle } = require("./generate_git_bundle.cjs");
const { hasMergeCommitsInRange, execGitSync } = require("./git_helpers.cjs");
const { hasMergeCommitsInRange, execGitSync, ensureSafeDirectoryTrust } = require("./git_helpers.cjs");
const { enforceCommentLimits } = require("./comment_limit_helpers.cjs");
const { getErrorMessage } = require("./error_helpers.cjs");
const { ERR_CONFIG, ERR_SYSTEM, ERR_VALIDATION } = require("./error_codes.cjs");
Expand Down Expand Up @@ -200,39 +200,6 @@
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
Expand All @@ -244,7 +211,7 @@
// 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);
ensureSafeDirectoryTrust(process.env.GITHUB_WORKSPACE || process.cwd());

Check failure on line 214 in actions/setup/js/safe_outputs_handlers.cjs

View workflow job for this annotation

GitHub Actions / JS Tests (shard 2/4)

safe_outputs_mcp_add_comment_constraints.test.cjs > Safe Outputs MCP Server - add_comment Constraint Enforcement > Valid Comments > should accept comment with valid body

ReferenceError: core is not defined ❯ ensureSafeDirectoryTrust git_helpers.cjs:73:3 ❯ Module.createHandlers safe_outputs_handlers.cjs:214:3 ❯ safe_outputs_mcp_add_comment_constraints.test.cjs:84:37

const TOKEN_THRESHOLD = 16000;

Expand Down Expand Up @@ -802,7 +769,7 @@
// 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);
ensureSafeDirectoryTrust(gitCwd);
let pinnedSha;
try {
pinnedSha = execGitSync(["rev-parse", "--verify", `refs/heads/${entry.branch}^{commit}`], { cwd: gitCwd })
Expand Down Expand Up @@ -1238,7 +1205,7 @@
// 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);
ensureSafeDirectoryTrust(pushGitCwd);
let pushPinnedSha;
try {
pushPinnedSha = execGitSync(["rev-parse", "--verify", `refs/heads/${entry.branch}^{commit}`], { cwd: pushGitCwd })
Expand Down
Loading