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..903fcdf1b43 100644 --- a/actions/setup/js/checkout_manifest.cjs +++ b/actions/setup/js/checkout_manifest.cjs @@ -93,7 +93,29 @@ 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 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(slug, { 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..5ca51f1ad87 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,31 @@ 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. 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|null} + */ +function resolveManifestPath(workspaceRoot, relPath) { + if (!relPath || relPath === ".") { + return workspaceRoot; + } + if (path.isAbsolute(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 resolved; +} + /** * Find the checkout directory for a given repo slug * Searches the workspace for git repos and matches by remote URL @@ -155,6 +181,28 @@ 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. + // 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); + 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 const gitDirs = findGitDirectories(ws); debugLog(`Found ${gitDirs.length} git directories: ${gitDirs.join(", ")}`); @@ -211,6 +259,19 @@ 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. + // 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) { + const resolved = resolveManifestPath(ws, entry.path); + if (resolved && fs.existsSync(resolved)) { + map.set(slug, resolved); + } + } + const gitDirs = findGitDirectories(ws); for (const repoPath of gitDirs) { @@ -218,7 +279,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..41119a9789f 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,277 @@ 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.mkdirSync(path.join(workspaceDir, "repos/sub-repo"), { recursive: true }); + 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.mkdirSync(path.join(workspaceDir, "repos/sub-repo"), { recursive: true }); + 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")); + }); + + 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", () => { let testDir;