From ffdcb2c8d4a31eb32d7d9ede01c45affe165653f Mon Sep 17 00:00:00 2001 From: Don Syme Date: Sun, 7 Jun 2026 14:26:52 +0100 Subject: [PATCH 1/3] Fix sample replay multi-repo lookup failing when "Configure Git credentials" rewrote the workspace remote (#37545) When a workflow uses `checkout:` to bring a non-workflow repository into `$GITHUB_WORKSPACE`, the compiler-emitted "Configure Git credentials" step runs after the side-repo checkout and unconditionally executes: git remote set-url origin \ "https://x-access-token:${GITHUB_TOKEN}@github.com/${GITHUB_REPOSITORY}.git" This clobbers `remote.origin.url` at the workspace root so it points back at the workflow's own repository, even though the on-disk contents are the side repo. `find_repo_checkout.cjs` then fails because it identifies repos by scanning `git config --get remote.origin.url`, and the URL no longer matches the slug being searched for. In sample-replay mode the safe-output handler reports "Repository '...' not found in workspace" and emits zero outputs. Fix by consulting the existing compile-time checkout manifest first. The "Build checkout manifest for safe-outputs handlers" step writes `$RUNNER_TEMP/gh-aw/checkout-manifest.json` mapping `lowercase(slug) -> { repository, path, default_branch }` before any credential rewriting happens. The manifest is therefore the authoritative source for cross-repo lookup paths and is unaffected by later `git remote set-url` operations. Changes: - `checkout_manifest.cjs`: export `loadAllCheckouts()` returning a Map of every parsed manifest entry. - `find_repo_checkout.cjs`: - `findRepoCheckout()` calls `lookupCheckout(targetSlug)` after the allowed-repos validation block; on hit, returns the resolved path directly and skips the git-remote scan. Falls through to the existing git-dir + workspace-root scan when the manifest has no entry (covers cases where no manifest is written, e.g. unit tests or non-multi-repo workflows). - `buildRepoCheckoutMap()` seeds the map from `loadAllCheckouts()` first, then only adds git-scan results for slugs not already present from the manifest. - New helper `resolveManifestPath()` handles the manifest's `path: ""` convention (= workspace root) and path joining. - `find_repo_checkout.test.cjs`: six new tests under `describe("checkout manifest fallback (issue #37545)")` covering the clobbered-origin regression scenario, subdirectory paths, case-insensitive slug matching, allowedRepos interaction with manifest hits, fallback when the manifest is empty, and `buildRepoCheckoutMap` seeding from manifest entries. --- ...patch-find-repo-checkout-manifest-first.md | 5 + actions/setup/js/checkout_manifest.cjs | 20 +++ actions/setup/js/find_repo_checkout.cjs | 44 +++++- actions/setup/js/find_repo_checkout.test.cjs | 135 ++++++++++++++++++ 4 files changed, 203 insertions(+), 1 deletion(-) create mode 100644 .changeset/patch-find-repo-checkout-manifest-first.md diff --git a/.changeset/patch-find-repo-checkout-manifest-first.md b/.changeset/patch-find-repo-checkout-manifest-first.md new file mode 100644 index 00000000000..72a2ed95dcd --- /dev/null +++ b/.changeset/patch-find-repo-checkout-manifest-first.md @@ -0,0 +1,5 @@ +--- +"gh-aw": patch +--- + +Fix safe-output handlers failing to locate side-repo checkouts when a "Configure Git credentials" step rewrote `remote.origin.url` at the workspace root. `findRepoCheckout` and `buildRepoCheckoutMap` now consult the compiler-written checkout manifest first, falling back to the existing `git config` scan only when the manifest has no entry for the requested slug. diff --git a/actions/setup/js/checkout_manifest.cjs b/actions/setup/js/checkout_manifest.cjs index dd3c8ea97f4..ac845a07f0a 100644 --- a/actions/setup/js/checkout_manifest.cjs +++ b/actions/setup/js/checkout_manifest.cjs @@ -93,7 +93,27 @@ function _resetCache() { cached = null; } +/** + * Return all checkout manifest entries as a Map keyed by lowercase repo slug. + * Each value has the shape { repository, path, default_branch }. + * + * @returns {Map} + */ +function loadAllCheckouts() { + const manifest = loadManifest(); + const map = new Map(); + for (const [key, entry] of Object.entries(manifest)) { + if (!entry || typeof entry !== "object") continue; + const repository = typeof entry.repository === "string" ? entry.repository : key; + const entryPath = typeof entry.path === "string" ? entry.path : ""; + const defaultBranch = typeof entry.default_branch === "string" ? entry.default_branch : ""; + map.set(key, { repository, path: entryPath, default_branch: defaultBranch }); + } + return map; +} + module.exports = { lookupCheckout, + loadAllCheckouts, _resetCache, }; diff --git a/actions/setup/js/find_repo_checkout.cjs b/actions/setup/js/find_repo_checkout.cjs index 4de72272f0c..b44f1b773df 100644 --- a/actions/setup/js/find_repo_checkout.cjs +++ b/actions/setup/js/find_repo_checkout.cjs @@ -4,6 +4,7 @@ const fs = require("fs"); const path = require("path"); const { execGitSync } = require("./git_helpers.cjs"); const { validateTargetRepo, parseAllowedRepos, getDefaultTargetRepo } = require("./repo_helpers.cjs"); +const { lookupCheckout, loadAllCheckouts } = require("./checkout_manifest.cjs"); /** * Debug logging helper - logs to stderr when DEBUG env var matches @@ -122,6 +123,24 @@ function getRemoteOriginUrl(repoPath) { } } +/** + * Resolve a workspace-relative path from the checkout manifest into an + * absolute path under the workspace. The manifest stores an empty string + * to represent the workspace root. + * @param {string} workspaceRoot + * @param {string} relPath + * @returns {string} + */ +function resolveManifestPath(workspaceRoot, relPath) { + if (!relPath || relPath === "" || relPath === ".") { + return workspaceRoot; + } + if (path.isAbsolute(relPath)) { + return relPath; + } + return path.join(workspaceRoot, relPath); +} + /** * Find the checkout directory for a given repo slug * Searches the workspace for git repos and matches by remote URL @@ -155,6 +174,22 @@ function findRepoCheckout(repoSlug, workspaceRoot, options = {}) { } } + // First, consult the checkout manifest written by the compiler-emitted + // "Build checkout manifest for safe-outputs handlers" step. This is the + // authoritative source for cross-repo checkout paths and does not depend + // on `git config --get remote.origin.url`, which a later "Configure Git + // credentials" step may have overwritten to point at the workflow repo. + const manifestEntry = lookupCheckout(targetSlug); + if (manifestEntry) { + const resolved = resolveManifestPath(ws, manifestEntry.path); + debugLog(`Found manifest entry for ${targetSlug}: ${resolved}`); + return { + success: true, + path: resolved, + repoSlug: targetSlug, + }; + } + // Find all git directories in the workspace const gitDirs = findGitDirectories(ws); debugLog(`Found ${gitDirs.length} git directories: ${gitDirs.join(", ")}`); @@ -211,6 +246,13 @@ function buildRepoCheckoutMap(workspaceRoot) { const ws = workspaceRoot || process.env.GITHUB_WORKSPACE || process.cwd(); const map = new Map(); + // Seed from the checkout manifest first so cross-repo entries survive even + // when a later "Configure Git credentials" step has rewritten remote.origin.url. + const manifest = loadAllCheckouts(); + for (const [slug, entry] of manifest) { + map.set(slug, resolveManifestPath(ws, entry.path)); + } + const gitDirs = findGitDirectories(ws); for (const repoPath of gitDirs) { @@ -218,7 +260,7 @@ function buildRepoCheckoutMap(workspaceRoot) { if (!remoteUrl) continue; const slug = extractRepoSlugFromUrl(remoteUrl); - if (slug) { + if (slug && !map.has(slug)) { map.set(slug, repoPath); } } diff --git a/actions/setup/js/find_repo_checkout.test.cjs b/actions/setup/js/find_repo_checkout.test.cjs index 8d97df99e56..ad5e3024858 100644 --- a/actions/setup/js/find_repo_checkout.test.cjs +++ b/actions/setup/js/find_repo_checkout.test.cjs @@ -2,6 +2,7 @@ const fs = require("fs"); const path = require("path"); const { extractRepoSlugFromUrl, normalizeRepoSlug, findGitDirectories, findRepoCheckout, buildRepoCheckoutMap } = require("./find_repo_checkout.cjs"); const { getPatchPathForBranchInRepo, sanitizeBranchNameForPatch, sanitizeRepoSlugForPatch } = require("./generate_git_patch.cjs"); +const { _resetCache: resetCheckoutManifestCache } = require("./checkout_manifest.cjs"); describe("find_repo_checkout", () => { describe("extractRepoSlugFromUrl", () => { @@ -168,6 +169,140 @@ describe("find_repo_checkout", () => { }); }); + describe("checkout manifest fallback (issue #37545)", () => { + let workspaceDir; + let manifestPath; + let originalManifestEnv; + + beforeEach(() => { + originalManifestEnv = process.env.GH_AW_CHECKOUT_MANIFEST; + workspaceDir = fs.mkdtempSync(path.join(require("os").tmpdir(), "test-manifest-")); + manifestPath = path.join(workspaceDir, "checkout-manifest.json"); + process.env.GH_AW_CHECKOUT_MANIFEST = manifestPath; + resetCheckoutManifestCache(); + }); + + afterEach(() => { + if (originalManifestEnv === undefined) { + delete process.env.GH_AW_CHECKOUT_MANIFEST; + } else { + process.env.GH_AW_CHECKOUT_MANIFEST = originalManifestEnv; + } + resetCheckoutManifestCache(); + try { + fs.rmSync(workspaceDir, { recursive: true, force: true }); + } catch { + // ignore + } + }); + + it("findRepoCheckout returns the manifest path even when remote.origin.url has been clobbered", () => { + // Reproduces the bug from #37545: the workflow checked out githubnext/gh-aw-side-repo + // at the workspace root, but a later "Configure Git credentials" step rewrote + // origin to point at githubnext/gh-aw-test. The manifest is the source of truth. + fs.writeFileSync( + manifestPath, + JSON.stringify({ + "githubnext/gh-aw-side-repo": { + repository: "githubnext/gh-aw-side-repo", + path: "", + default_branch: "main", + }, + }) + ); + + const result = findRepoCheckout("githubnext/gh-aw-side-repo", workspaceDir); + + expect(result.success).toBe(true); + expect(result.path).toBe(workspaceDir); + expect(result.repoSlug).toBe("githubnext/gh-aw-side-repo"); + }); + + it("findRepoCheckout resolves manifest entries with a non-empty path to a subdirectory", () => { + fs.writeFileSync( + manifestPath, + JSON.stringify({ + "owner/sub-repo": { + repository: "owner/sub-repo", + path: "repos/sub-repo", + default_branch: "main", + }, + }) + ); + + const result = findRepoCheckout("owner/sub-repo", workspaceDir); + + expect(result.success).toBe(true); + expect(result.path).toBe(path.join(workspaceDir, "repos/sub-repo")); + }); + + it("findRepoCheckout is case-insensitive on the repo slug", () => { + fs.writeFileSync( + manifestPath, + JSON.stringify({ + "githubnext/gh-aw-side-repo": { + repository: "githubnext/gh-aw-side-repo", + path: "", + default_branch: "main", + }, + }) + ); + + const result = findRepoCheckout("GithubNext/gh-aw-side-repo", workspaceDir); + + expect(result.success).toBe(true); + expect(result.path).toBe(workspaceDir); + }); + + it("findRepoCheckout still applies allowedRepos validation when the manifest matches", () => { + fs.writeFileSync( + manifestPath, + JSON.stringify({ + "owner/blocked": { repository: "owner/blocked", path: "blocked", default_branch: "main" }, + }) + ); + + const result = findRepoCheckout("owner/blocked", workspaceDir, { + allowedRepos: ["owner/allowed"], + }); + + expect(result.success).toBe(false); + expect(result.error).toBeDefined(); + }); + + it("findRepoCheckout falls back to the git-remote scan when the manifest has no entry", () => { + fs.writeFileSync(manifestPath, JSON.stringify({})); + + const result = findRepoCheckout("owner/missing", workspaceDir); + + expect(result.success).toBe(false); + expect(result.error).toContain("not found in workspace"); + }); + + it("buildRepoCheckoutMap seeds from manifest entries", () => { + fs.writeFileSync( + manifestPath, + JSON.stringify({ + "githubnext/gh-aw-side-repo": { + repository: "githubnext/gh-aw-side-repo", + path: "", + default_branch: "main", + }, + "owner/sub-repo": { + repository: "owner/sub-repo", + path: "repos/sub-repo", + default_branch: "main", + }, + }) + ); + + const map = buildRepoCheckoutMap(workspaceDir); + + expect(map.get("githubnext/gh-aw-side-repo")).toBe(workspaceDir); + expect(map.get("owner/sub-repo")).toBe(path.join(workspaceDir, "repos/sub-repo")); + }); + }); + describe("buildRepoCheckoutMap", () => { let testDir; From abaa7a5cb173bda6b9221f579f71def0c5dbb4dd Mon Sep 17 00:00:00 2001 From: Don Syme Date: Sun, 7 Jun 2026 14:41:58 +0100 Subject: [PATCH 2/3] Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> --- actions/setup/js/checkout_manifest.cjs | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/actions/setup/js/checkout_manifest.cjs b/actions/setup/js/checkout_manifest.cjs index ac845a07f0a..903fcdf1b43 100644 --- a/actions/setup/js/checkout_manifest.cjs +++ b/actions/setup/js/checkout_manifest.cjs @@ -104,10 +104,12 @@ function loadAllCheckouts() { const map = new Map(); for (const [key, entry] of Object.entries(manifest)) { if (!entry || typeof entry !== "object") continue; - const repository = typeof entry.repository === "string" ? entry.repository : key; + const slug = key.trim().toLowerCase(); + if (!slug) continue; + const repository = typeof entry.repository === "string" ? entry.repository : slug; const entryPath = typeof entry.path === "string" ? entry.path : ""; const defaultBranch = typeof entry.default_branch === "string" ? entry.default_branch : ""; - map.set(key, { repository, path: entryPath, default_branch: defaultBranch }); + map.set(slug, { repository, path: entryPath, default_branch: defaultBranch }); } return map; } From f5c633834f685feef7f46f73083c9de08f06c4f3 Mon Sep 17 00:00:00 2001 From: Don Syme Date: Sun, 7 Jun 2026 14:51:24 +0100 Subject: [PATCH 3/3] Address PR #37565 review feedback - resolveManifestPath: simplify redundant empty-string check; clamp result to workspace root, refusing absolute paths and .. traversal so a malformed or tampered manifest cannot redirect lookups outside $GITHUB_WORKSPACE. - findRepoCheckout: verify resolved manifest path exists on disk before returning success; fall back to git scan on stale/missing entries. - buildRepoCheckoutMap: only seed manifest entries whose resolved path is safe and exists, matching the guarantee provided by the git-scan branch. - Tests: cover unset/missing manifest env var, stale path fallback, absolute + .. traversal rejection, manifest-wins-over-git-scan priority, and the new buildRepoCheckoutMap existence guard. --- actions/setup/js/find_repo_checkout.cjs | 43 ++++-- actions/setup/js/find_repo_checkout.test.cjs | 137 +++++++++++++++++++ 2 files changed, 168 insertions(+), 12 deletions(-) diff --git a/actions/setup/js/find_repo_checkout.cjs b/actions/setup/js/find_repo_checkout.cjs index b44f1b773df..5ca51f1ad87 100644 --- a/actions/setup/js/find_repo_checkout.cjs +++ b/actions/setup/js/find_repo_checkout.cjs @@ -126,19 +126,26 @@ function getRemoteOriginUrl(repoPath) { /** * Resolve a workspace-relative path from the checkout manifest into an * absolute path under the workspace. The manifest stores an empty string - * to represent the workspace root. + * to represent the workspace root. Returns null when the manifest path is + * absolute or escapes the workspace root via `..` traversal, so a malformed + * or tampered manifest cannot redirect lookups outside of $GITHUB_WORKSPACE. * @param {string} workspaceRoot * @param {string} relPath - * @returns {string} + * @returns {string|null} */ function resolveManifestPath(workspaceRoot, relPath) { - if (!relPath || relPath === "" || relPath === ".") { + if (!relPath || relPath === ".") { return workspaceRoot; } if (path.isAbsolute(relPath)) { - return relPath; + return null; + } + const wsResolved = path.resolve(workspaceRoot); + const resolved = path.resolve(wsResolved, relPath); + if (resolved !== wsResolved && !resolved.startsWith(wsResolved + path.sep)) { + return null; } - return path.join(workspaceRoot, relPath); + return resolved; } /** @@ -179,15 +186,21 @@ function findRepoCheckout(repoSlug, workspaceRoot, options = {}) { // authoritative source for cross-repo checkout paths and does not depend // on `git config --get remote.origin.url`, which a later "Configure Git // credentials" step may have overwritten to point at the workflow repo. + // The manifest is written before `actions/checkout` runs, so fall back to + // the git scan when the resolved path is unsafe or does not exist on disk + // (failed checkout, workspace wiped, manifest stale). const manifestEntry = lookupCheckout(targetSlug); if (manifestEntry) { const resolved = resolveManifestPath(ws, manifestEntry.path); - debugLog(`Found manifest entry for ${targetSlug}: ${resolved}`); - return { - success: true, - path: resolved, - repoSlug: targetSlug, - }; + if (resolved && fs.existsSync(resolved)) { + debugLog(`Found manifest entry for ${targetSlug}: ${resolved}`); + return { + success: true, + path: resolved, + repoSlug: targetSlug, + }; + } + debugLog(`Manifest entry for ${targetSlug} unusable (path: ${manifestEntry.path}), falling back to git scan`); } // Find all git directories in the workspace @@ -248,9 +261,15 @@ function buildRepoCheckoutMap(workspaceRoot) { // Seed from the checkout manifest first so cross-repo entries survive even // when a later "Configure Git credentials" step has rewritten remote.origin.url. + // Only seed entries whose resolved path is safe (inside the workspace) and + // actually exists on disk, mirroring the guarantee that the git-scan branch + // provides for every entry it produces. const manifest = loadAllCheckouts(); for (const [slug, entry] of manifest) { - map.set(slug, resolveManifestPath(ws, entry.path)); + const resolved = resolveManifestPath(ws, entry.path); + if (resolved && fs.existsSync(resolved)) { + map.set(slug, resolved); + } } const gitDirs = findGitDirectories(ws); diff --git a/actions/setup/js/find_repo_checkout.test.cjs b/actions/setup/js/find_repo_checkout.test.cjs index ad5e3024858..41119a9789f 100644 --- a/actions/setup/js/find_repo_checkout.test.cjs +++ b/actions/setup/js/find_repo_checkout.test.cjs @@ -219,6 +219,7 @@ describe("find_repo_checkout", () => { }); it("findRepoCheckout resolves manifest entries with a non-empty path to a subdirectory", () => { + fs.mkdirSync(path.join(workspaceDir, "repos/sub-repo"), { recursive: true }); fs.writeFileSync( manifestPath, JSON.stringify({ @@ -280,6 +281,7 @@ describe("find_repo_checkout", () => { }); it("buildRepoCheckoutMap seeds from manifest entries", () => { + fs.mkdirSync(path.join(workspaceDir, "repos/sub-repo"), { recursive: true }); fs.writeFileSync( manifestPath, JSON.stringify({ @@ -301,6 +303,141 @@ describe("find_repo_checkout", () => { expect(map.get("githubnext/gh-aw-side-repo")).toBe(workspaceDir); expect(map.get("owner/sub-repo")).toBe(path.join(workspaceDir, "repos/sub-repo")); }); + + it("findRepoCheckout falls back to git-scan when GH_AW_CHECKOUT_MANIFEST is unset", () => { + // Pin the contract that loadManifest() returns an empty object (not + // throws) when the manifest env var is unset and no $RUNNER_TEMP fallback + // exists. Critical for backward compatibility with workflows that have no + // multi-repo checkout. + delete process.env.GH_AW_CHECKOUT_MANIFEST; + const originalRunnerTemp = process.env.RUNNER_TEMP; + delete process.env.RUNNER_TEMP; + resetCheckoutManifestCache(); + + try { + const result = findRepoCheckout("owner/missing", workspaceDir); + + expect(result.success).toBe(false); + expect(result.error).toContain("not found in workspace"); + } finally { + if (originalRunnerTemp !== undefined) { + process.env.RUNNER_TEMP = originalRunnerTemp; + } + } + }); + + it("findRepoCheckout falls back to git-scan when GH_AW_CHECKOUT_MANIFEST points to a non-existent file", () => { + process.env.GH_AW_CHECKOUT_MANIFEST = path.join(workspaceDir, "does-not-exist.json"); + resetCheckoutManifestCache(); + + const result = findRepoCheckout("owner/missing", workspaceDir); + + expect(result.success).toBe(false); + expect(result.error).toContain("not found in workspace"); + }); + + it("findRepoCheckout falls back to git-scan when the manifest path does not exist on disk", () => { + // A stale manifest entry (failed checkout, workspace wiped) must not be + // returned as a valid checkout — the git scan is authoritative for paths + // that actually exist on disk. + fs.writeFileSync( + manifestPath, + JSON.stringify({ + "owner/stale": { + repository: "owner/stale", + path: "never-checked-out", + default_branch: "main", + }, + }) + ); + + const result = findRepoCheckout("owner/stale", workspaceDir); + + expect(result.success).toBe(false); + expect(result.error).toContain("not found in workspace"); + }); + + it("findRepoCheckout rejects manifest entries with absolute paths", () => { + // resolveManifestPath() must refuse absolute paths so a tampered manifest + // cannot redirect lookups outside $GITHUB_WORKSPACE. + fs.writeFileSync( + manifestPath, + JSON.stringify({ + "owner/escape": { + repository: "owner/escape", + path: "/etc", + default_branch: "main", + }, + }) + ); + + const result = findRepoCheckout("owner/escape", workspaceDir); + + expect(result.success).toBe(false); + expect(result.error).toContain("not found in workspace"); + }); + + it("findRepoCheckout rejects manifest entries that escape the workspace via .. traversal", () => { + fs.writeFileSync( + manifestPath, + JSON.stringify({ + "owner/escape": { + repository: "owner/escape", + path: "../outside", + default_branch: "main", + }, + }) + ); + + const result = findRepoCheckout("owner/escape", workspaceDir); + + expect(result.success).toBe(false); + expect(result.error).toContain("not found in workspace"); + }); + + it("buildRepoCheckoutMap: manifest entry wins over git-scan for the same slug", () => { + // The !map.has(slug) guard added in buildRepoCheckoutMap is the core of + // the bug fix; without this test the guard could be silently removed and + // the regression would go undetected. + const manifestRepoPath = path.join(workspaceDir, "from-manifest"); + const gitScanRepoPath = path.join(workspaceDir, "from-git-scan"); + fs.mkdirSync(manifestRepoPath, { recursive: true }); + fs.mkdirSync(path.join(gitScanRepoPath, ".git"), { recursive: true }); + // Real git config so getRemoteOriginUrl resolves the same slug as the + // manifest entry, simulating the clobbered-origin scenario. + fs.writeFileSync(path.join(gitScanRepoPath, ".git", "config"), `[remote "origin"]\n\turl = https://github.com/owner/conflict.git\n`); + fs.writeFileSync( + manifestPath, + JSON.stringify({ + "owner/conflict": { + repository: "owner/conflict", + path: "from-manifest", + default_branch: "main", + }, + }) + ); + + const map = buildRepoCheckoutMap(workspaceDir); + + expect(map.get("owner/conflict")).toBe(manifestRepoPath); + }); + + it("buildRepoCheckoutMap skips manifest entries whose path does not exist", () => { + fs.writeFileSync( + manifestPath, + JSON.stringify({ + "owner/stale": { + repository: "owner/stale", + path: "missing-on-disk", + default_branch: "main", + }, + }) + ); + + const map = buildRepoCheckoutMap(workspaceDir); + + expect(map.has("owner/stale")).toBe(false); + }); }); describe("buildRepoCheckoutMap", () => {