diff --git a/server/src/__tests__/heartbeat-workspace-session.test.ts b/server/src/__tests__/heartbeat-workspace-session.test.ts index 6524a27ee2a2..dee8f76fc24f 100644 --- a/server/src/__tests__/heartbeat-workspace-session.test.ts +++ b/server/src/__tests__/heartbeat-workspace-session.test.ts @@ -59,6 +59,8 @@ import { normalizeSessionParams, shouldResetTaskSessionForWake, mergeModelProfileAdapterConfig, + resolveContainedWorkspaceSubpath, + resolveRepoRelativeWorkspaceCwd, type ResolvedWorkspaceForRunSuccess, } from "../services/heartbeat.js"; import { applyRunScopeToBranchName } from "../services/workspace-runtime.js"; @@ -3938,6 +3940,156 @@ describe("isNonPrimaryWorkspaceTarget", () => { }); }); +describe("resolveContainedWorkspaceSubpath", () => { + // BLO-25415: a git_repo workspace may declare a repo-relative cwd such as + // "packages/iwa". Before the fix that raw string reached fs.stat() and was + // resolved against the API process's cwd, so the run failed + // `preferred_workspace_unrealizable` naming a path that looked correct while + // the real checkout sat on disk untouched. + it("joins a repo-relative subpath onto the realized checkout", async () => { + await expect( + resolveContainedWorkspaceSubpath("/managed/pim-multicast-gateway", "packages/iwa"), + ).resolves.toBe("/managed/pim-multicast-gateway/packages/iwa"); + }); + + it("normalizes redundant segments that stay inside the checkout", async () => { + await expect( + resolveContainedWorkspaceSubpath("/managed/repo", "./packages/../packages/iwa"), + ).resolves.toBe("/managed/repo/packages/iwa"); + }); + + it("allows the checkout root itself", async () => { + await expect(resolveContainedWorkspaceSubpath("/managed/repo", ".")).resolves.toBe("/managed/repo"); + }); + + it("refuses a subpath that escapes the checkout", async () => { + // cwd is operator-supplied config, so traversal must not point a run at an + // unrelated repo on the shared PVC. + await expect(resolveContainedWorkspaceSubpath("/managed/repo", "../other-repo")).rejects.toThrow( + /resolves outside its checkout/, + ); + await expect(resolveContainedWorkspaceSubpath("/managed/repo", "../../../etc")).rejects.toThrow( + /resolves outside its checkout/, + ); + }); + + it("refuses a sibling directory sharing the checkout name prefix", async () => { + // Guards the startsWith() containment check against "/managed/repo-evil" + // being treated as inside "/managed/repo". + await expect(resolveContainedWorkspaceSubpath("/managed/repo", "../repo-evil")).rejects.toThrow( + /resolves outside its checkout/, + ); + }); + + it("refuses a subpath that escapes via a symlink inside the checkout", async () => { + // The lexical check alone is not enough: the checkout's *contents* are + // repo-controlled, so a repo carrying `packages/iwa -> /etc` passes + // path.resolve() and then fs.stat() follows the link, launching the run + // outside the checkout on a PVC shared with every other repo. + const tmp = await fs.mkdtemp(path.join(os.tmpdir(), "blo25415-symlink-")); + try { + const checkout = path.join(tmp, "repo"); + const outside = path.join(tmp, "outside"); + await fs.mkdir(path.join(checkout, "packages"), { recursive: true }); + await fs.mkdir(outside, { recursive: true }); + await fs.symlink(outside, path.join(checkout, "packages", "iwa")); + + await expect(resolveContainedWorkspaceSubpath(checkout, "packages/iwa")).rejects.toThrow( + /resolves outside its checkout/, + ); + } finally { + await fs.rm(tmp, { recursive: true, force: true }); + } + }); + + it("allows a symlink that stays inside the checkout", async () => { + // Only escapes are refused — an in-repo symlink is legitimate. + const tmp = await fs.mkdtemp(path.join(os.tmpdir(), "blo25415-symlink-ok-")); + try { + const checkout = path.join(tmp, "repo"); + await fs.mkdir(path.join(checkout, "packages", "real"), { recursive: true }); + await fs.symlink(path.join(checkout, "packages", "real"), path.join(checkout, "iwa")); + + await expect(resolveContainedWorkspaceSubpath(checkout, "iwa")).resolves.toBe( + path.join(checkout, "iwa"), + ); + } finally { + await fs.rm(tmp, { recursive: true, force: true }); + } + }); + + it("allows a subpath when the checkout root itself sits behind a symlink", async () => { + // The managed dir may be reached through a symlink (e.g. a PVC mount + // indirection). Resolving root and target independently keeps that from + // failing containment for a perfectly legitimate subpath. + const tmp = await fs.mkdtemp(path.join(os.tmpdir(), "blo25415-symlink-root-")); + try { + const realCheckout = path.join(tmp, "real-repo"); + const linkedCheckout = path.join(tmp, "linked-repo"); + await fs.mkdir(path.join(realCheckout, "packages", "iwa"), { recursive: true }); + await fs.symlink(realCheckout, linkedCheckout); + + await expect(resolveContainedWorkspaceSubpath(linkedCheckout, "packages/iwa")).resolves.toBe( + path.join(linkedCheckout, "packages", "iwa"), + ); + } finally { + await fs.rm(tmp, { recursive: true, force: true }); + } + }); +}); + +describe("resolveRepoRelativeWorkspaceCwd", () => { + // BLO-25415: only a repo-backed workspace has a checkout for a relative cwd + // to be relative to. Redirecting other source types into a managed checkout + // dir would change their meaning — and, via ensureManagedProjectWorkspace's + // repoUrl-less branch, mkdir an empty directory on the shared PVC that the + // subsequent stat can never satisfy. + const repoUrl = "https://github.com/Blockcast/pim-multicast-gateway.git"; + + it("returns the subpath for a repo-backed workspace with a relative cwd", () => { + expect( + resolveRepoRelativeWorkspaceCwd({ cwd: "packages/iwa", sourceType: "git_repo", repoUrl }), + ).toBe("packages/iwa"); + }); + + it("accepts a repo-backed workspace whose source_type is not the canonical spelling", () => { + // source_type is an unconstrained text column; production carries a "git" + // row. An allowlist keyed on "git_repo" would silently skip it. + expect(resolveRepoRelativeWorkspaceCwd({ cwd: "packages/iwa", sourceType: "git", repoUrl })).toBe( + "packages/iwa", + ); + }); + + it("leaves an absolute cwd untouched", () => { + expect( + resolveRepoRelativeWorkspaceCwd({ cwd: "/managed/repo/packages/iwa", sourceType: "git_repo", repoUrl }), + ).toBeNull(); + }); + + it("leaves an empty cwd and the repo-only sentinel untouched", () => { + expect(resolveRepoRelativeWorkspaceCwd({ cwd: null, sourceType: "git_repo", repoUrl })).toBeNull(); + expect( + resolveRepoRelativeWorkspaceCwd({ cwd: "/__paperclip_repo_only__", sourceType: "git_repo", repoUrl }), + ).toBeNull(); + }); + + it("leaves a relative local_path / non_git_path / remote_managed cwd untouched", () => { + // These own their cwd outright — they must keep the prior semantics rather + // than being redirected into a managed checkout. + for (const sourceType of ["local_path", "non_git_path", "remote_managed"]) { + expect(resolveRepoRelativeWorkspaceCwd({ cwd: "packages/iwa", sourceType, repoUrl })).toBeNull(); + } + }); + + it("leaves a relative cwd untouched when the workspace has no repo", () => { + // Nothing for the path to be relative to; taking the managed-checkout + // branch would create an empty directory and still fail the stat. + expect( + resolveRepoRelativeWorkspaceCwd({ cwd: "packages/iwa", sourceType: "git_repo", repoUrl: null }), + ).toBeNull(); + }); +}); + describe("evaluatePreferredProjectWorkspaceRealization", () => { it("fails loud when an unrealized non-primary workspace is explicitly targeted", () => { // Mirrors BLO-8154: issue targets the trafficcontrol workspace but only the diff --git a/server/src/services/heartbeat.ts b/server/src/services/heartbeat.ts index 7d51fb2cc8bc..233637526941 100644 --- a/server/src/services/heartbeat.ts +++ b/server/src/services/heartbeat.ts @@ -2304,6 +2304,77 @@ function deriveRepoNameFromRepoUrl(repoUrl: string | null): string | null { } } +// Join a repo-relative workspace subpath onto its realized checkout, refusing +// anything that escapes the checkout root. `cwd` is operator-supplied config, +// so a value like "../../../etc" must not be able to point a run outside the +// repo it declared. See BLO-25415. +// +// Containment is enforced twice, because the two escapes are different: +// - lexically, against `..` traversal and sibling-prefix paths; and +// - after symlink resolution, because the checkout's *contents* are +// repo-controlled. A repo carrying `packages/iwa -> /etc` passes the +// lexical check and then fs.stat() follows the link, launching the run +// outside the checkout on a PVC shared with every other repo. +// +// The realpath pass is skipped when the target does not exist: there is +// nothing to follow, and the caller's own fs.stat() is what reports it +// missing. Resolving the root separately matters too — the managed dir may +// itself sit behind a symlink, which would otherwise fail containment for a +// perfectly legitimate subpath. +export async function resolveContainedWorkspaceSubpath( + checkoutDir: string, + subpath: string, +): Promise { + const root = path.resolve(checkoutDir); + const resolved = path.resolve(root, subpath); + assertPathContained(root, resolved, subpath); + + const realRoot = await fs.realpath(root).catch(() => null); + const realResolved = await fs.realpath(resolved).catch(() => null); + if (realRoot && realResolved) { + assertPathContained(realRoot, realResolved, subpath); + } + return resolved; +} + +function assertPathContained(root: string, candidate: string, subpath: string): void { + if (candidate !== root && !candidate.startsWith(`${root}${path.sep}`)) { + throw new Error( + `Project workspace cwd "${subpath}" resolves outside its checkout "${root}".`, + ); + } +} + +// Workspace source types that own their `cwd` outright: the path is a host +// location (or a remote provider's), not a subdirectory of a checkout we +// manage. A relative `cwd` on one of these keeps its prior meaning rather than +// being redirected into a managed checkout dir. +const NON_REPO_WORKSPACE_SOURCE_TYPES = new Set(["local_path", "non_git_path", "remote_managed"]); + +/** + * Decide whether a workspace's `cwd` is a repo-relative subdirectory that must + * be joined onto its managed checkout, returning that subpath (or null to use + * `cwd` as-is). + * + * Only repo-backed workspaces qualify. `repoUrl` is required rather than + * inferred from `sourceType` because `source_type` is an unconstrained text + * column — production carries at least one row typed `"git"` rather than + * `"git_repo"` — and because without a repo there is nothing for the path to + * be relative to: `ensureManagedProjectWorkspace` would mkdir an empty + * directory that the subsequent stat can never satisfy. See BLO-25415. + */ +export function resolveRepoRelativeWorkspaceCwd(workspace: { + cwd?: string | null; + sourceType?: string | null; + repoUrl?: string | null; +}): string | null { + const cwd = readNonEmptyString(workspace.cwd); + if (!cwd || cwd === REPO_ONLY_CWD_SENTINEL || path.isAbsolute(cwd)) return null; + if (NON_REPO_WORKSPACE_SOURCE_TYPES.has(readNonEmptyString(workspace.sourceType) ?? "")) return null; + if (!readNonEmptyString(workspace.repoUrl)) return null; + return cwd; +} + async function ensureManagedProjectWorkspace(input: { companyId: string; projectId: string; @@ -11418,14 +11489,26 @@ export function heartbeatService(db: Db, options: HeartbeatServiceOptions = {}) for (const workspace of realizationCandidates) { let projectCwd = readNonEmptyString(workspace.cwd); let managedWorkspaceWarning: string | null = null; - if (!projectCwd || projectCwd === REPO_ONLY_CWD_SENTINEL) { + // A workspace cwd is either an absolute host path or — for a repo-backed + // workspace — a repo-relative subdirectory such as "packages/iwa". The + // relative form only resolves once the repo is checked out, so realize + // the managed checkout first and join the subpath inside it. Without + // this the raw relative string reached the fs.stat() below and was + // resolved against the API process's own cwd, which never matches: the + // run then failed `preferred_workspace_unrealizable` quoting a path + // that looks correct, and no amount of cloning could satisfy it. See + // BLO-25415. + const repoRelativeCwd = resolveRepoRelativeWorkspaceCwd(workspace); + if (!projectCwd || projectCwd === REPO_ONLY_CWD_SENTINEL || repoRelativeCwd) { try { const managedWorkspace = await ensureManagedProjectWorkspace({ companyId: agent.companyId, projectId: workspaceProjectId ?? resolvedProjectId ?? workspace.projectId, repoUrl: readNonEmptyString(workspace.repoUrl), }); - projectCwd = managedWorkspace.cwd; + projectCwd = repoRelativeCwd + ? await resolveContainedWorkspaceSubpath(managedWorkspace.cwd, repoRelativeCwd) + : managedWorkspace.cwd; managedWorkspaceWarning = managedWorkspace.warning; } catch (error) { if (preferredWorkspace?.id === workspace.id) {