diff --git a/packages/adapter-utils/src/acpx-engine/execute.test.ts b/packages/adapter-utils/src/acpx-engine/execute.test.ts index 63c5fe4789..5575b37b46 100644 --- a/packages/adapter-utils/src/acpx-engine/execute.test.ts +++ b/packages/adapter-utils/src/acpx-engine/execute.test.ts @@ -38,6 +38,7 @@ import { } from "./execute.js"; import { runChildProcess } from "../server-utils.js"; import { setExpensiveWorkspaceGitExecutor } from "../git-workspace-sync.js"; +import { resolveReferencedSourceIgnore } from "../sandbox-managed-runtime.js"; import { getActiveStepContext, runWithRuntimeParent, @@ -1591,6 +1592,37 @@ describe("shared ACPX engine runtime behavior", () => { expect(signature).toBe("unreadable:git status timed out"); }); + it("never leaks a raw absolute path into the signature, even when the underlying scan embedded one", async () => { + const root = await makeTempRoot(); + const localPath = path.join(root, "does-not-exist"); + + // A raw toplevel string that makes `localPath` a non-descendant, carrying + // a sensitive absolute path — exactly the shape a caught Git diagnostic + // could embed. `resolveReferencedSourceIgnore` is the single choke point + // that must reduce it to the fixed category before the signature (which + // embeds `reason` verbatim as `unreadable:${reason}`) ever sees it. + const sensitivePath = "/home/alice/project"; + let ignoreResolution; + try { + setExpensiveWorkspaceGitExecutor(async (input) => { + if (input.operation === "referenced_source.toplevel") { + return { stdout: `${sensitivePath}\n`, stderr: "" }; + } + return { stdout: "", stderr: "" }; + }); + ignoreResolution = await resolveReferencedSourceIgnore(localPath); + } finally { + setExpensiveWorkspaceGitExecutor(null); + } + expect(ignoreResolution.kind).toBe("failed"); + + const signature = await referencedSourceContentSignature(localPath, ignoreResolution); + + expect(signature).toBe("unreadable:git-toplevel-not-descendant"); + expect(signature).not.toContain(sensitivePath); + expect(signature).not.toContain(localPath); + }); + it("skips a Git-ignored file (exact match) so its content never affects the signature", async () => { const root = await makeTempRoot(); const localPath = path.join(root, "project"); diff --git a/packages/adapter-utils/src/git-workspace-sync.test.ts b/packages/adapter-utils/src/git-workspace-sync.test.ts index 035d3203eb..31ac4590a8 100644 --- a/packages/adapter-utils/src/git-workspace-sync.test.ts +++ b/packages/adapter-utils/src/git-workspace-sync.test.ts @@ -14,7 +14,10 @@ import { integrateImportedGitHead, isMissingGitPrerequisiteError, readGitWorkspaceSnapshot, + ReferencedSourceIgnoreScanLimitExceededError, readReferencedSourceGitIgnoredPaths, + REFERENCED_SOURCE_IGNORE_MAX_ENTRY_COUNT, + REFERENCED_SOURCE_IGNORE_MAX_TOTAL_BYTES, runLocalGit, sanitizeGitRemoteUrl, setExpensiveWorkspaceGitExecutor, @@ -64,6 +67,44 @@ describe("git workspace sync", () => { ]); }); + it("keeps every filename byte for a padded name in each of the four anchor lanes", async () => { + const rootDir = await mkdtemp(path.join(os.tmpdir(), "paperclip-git-anchor-whitespace-")); + cleanupDirs.push(rootDir); + const repo = await createRepo(rootDir); + + // Deleted lane: commit the file first (in isolation, before anything else + // is staged), then remove it from the work tree. + const deletedName = " deleted padded "; + await writeFile(path.join(repo, deletedName), "deleted\n", "utf8"); + await git(repo, ["add", deletedName]); + await git(repo, ["commit", "-qm", "add deleted padded"]); + await rm(path.join(repo, deletedName)); + + // Overlay lane, staged-new half: `git diff --diff-filter=ACMRTUXB HEAD` + // reports a staged-but-uncommitted file as added. + const overlayName = " overlay padded "; + await writeFile(path.join(repo, overlayName), "overlay\n", "utf8"); + await git(repo, ["add", overlayName]); + + // Overlay lane, untracked half: `ls-files --others --exclude-standard`. + const untrackedName = " untracked padded "; + await writeFile(path.join(repo, untrackedName), "untracked\n", "utf8"); + + // Ignored lane: a double-wildcard pattern avoids the separate rule that + // Git trims an unescaped trailing space in a .gitignore PATTERN itself; + // the padding under test lives in the matched FILE name. + const ignoredName = " ignored padded "; + await writeFile(path.join(repo, ".gitignore"), "*ignored*padded*\n", "utf8"); + await writeFile(path.join(repo, ignoredName), "ignored\n", "utf8"); + + const snapshot = await readGitWorkspaceSnapshot(repo); + + expect(snapshot?.overlayPaths).toContain(overlayName); + expect(snapshot?.overlayPaths).toContain(untrackedName); + expect(snapshot?.deletedPaths).toContain(deletedName); + expect(snapshot?.ignoredPaths).toContain(ignoredName); + }); + async function createRepo(rootDir: string): Promise { const repo = path.join(rootDir, "repo"); await mkdir(repo, { recursive: true }); @@ -576,6 +617,157 @@ describe("git workspace sync", () => { expect(scan?.ignoredPaths).toEqual([paddedName]); }); + it("fails closed when the parsed ignored-entry count exceeds the bound", async () => { + const rootDir = await mkdtemp(path.join(os.tmpdir(), "paperclip-referenced-bound-count-")); + cleanupDirs.push(rootDir); + const repo = await createRepo(rootDir); + // Synthesize the `git ls-files --others --ignored -z` output directly, + // rather than creating ten thousand real files, by intercepting the + // scan at the executor seam. The parser must reject this before it + // sorts or re-relativizes the list. + const overLimitCount = REFERENCED_SOURCE_IGNORE_MAX_ENTRY_COUNT + 1; + const syntheticIgnored = `${Array.from({ length: overLimitCount }, (_, index) => `entry-${index}`).join("\0")}\0`; + setExpensiveWorkspaceGitExecutor(async (input) => { + if (input.operation === "referenced_source.ignored_files") { + return { stdout: syntheticIgnored, stderr: "" }; + } + return await runLocalGit(input.localDir, [...input.args], { + timeout: input.timeout, + maxBuffer: input.maxBuffer, + env: input.env, + }); + }); + + await expect(readReferencedSourceGitIgnoredPaths(repo)).rejects.toBeInstanceOf( + ReferencedSourceIgnoreScanLimitExceededError, + ); + }); + + it("fails closed when the summed UTF-8 byte size of ignored paths exceeds the bound", async () => { + const rootDir = await mkdtemp(path.join(os.tmpdir(), "paperclip-referenced-bound-bytes-")); + cleanupDirs.push(rootDir); + const repo = await createRepo(rootDir); + // One entry alone exceeds the byte bound, well under the entry-count bound. + const hugeEntry = "a".repeat(REFERENCED_SOURCE_IGNORE_MAX_TOTAL_BYTES + 1); + const syntheticIgnored = `${hugeEntry}\0`; + setExpensiveWorkspaceGitExecutor(async (input) => { + if (input.operation === "referenced_source.ignored_files") { + return { stdout: syntheticIgnored, stderr: "" }; + } + return await runLocalGit(input.localDir, [...input.args], { + timeout: input.timeout, + maxBuffer: input.maxBuffer, + env: input.env, + }); + }); + + await expect(readReferencedSourceGitIgnoredPaths(repo)).rejects.toBeInstanceOf( + ReferencedSourceIgnoreScanLimitExceededError, + ); + }); + + it("fails closed on the byte bound while it is still accumulating, before it would ever reach a later entry-count breach", async () => { + const rootDir = await mkdtemp(path.join(os.tmpdir(), "paperclip-referenced-bound-order-")); + cleanupDirs.push(rootDir); + const repo = await createRepo(rootDir); + // Three entries alone cross the byte bound. Many more small entries + // follow, so the FULL response also carries more than the entry-count + // bound. A parser that fully builds the list before checking either + // bound (post-parse) would report the entry-count breach, because it + // checks that bound first against the whole materialized list. A + // parser that checks both bounds while the list accumulates rejects on + // the byte bound instead, the moment the third entry crosses it, well + // before the count bound is ever reached. + const oversizedEntry = "a".repeat(Math.ceil(REFERENCED_SOURCE_IGNORE_MAX_TOTAL_BYTES / 2) + 1); + const bigEntries = Array.from({ length: 3 }, (_, index) => `${oversizedEntry}-${index}`); + const trailingEntries = Array.from( + { length: REFERENCED_SOURCE_IGNORE_MAX_ENTRY_COUNT + 10 }, + (_, index) => `trailing-${index}`, + ); + const syntheticIgnored = `${[...bigEntries, ...trailingEntries].join("\0")}\0`; + setExpensiveWorkspaceGitExecutor(async (input) => { + if (input.operation === "referenced_source.ignored_files") { + return { stdout: syntheticIgnored, stderr: "" }; + } + return await runLocalGit(input.localDir, [...input.args], { + timeout: input.timeout, + maxBuffer: input.maxBuffer, + env: input.env, + }); + }); + + await expect(readReferencedSourceGitIgnoredPaths(repo)).rejects.toThrow(/UTF-8 bytes/); + }); + + it("bounds the raw command-output allowance to the ignore-scan limits, not the general-purpose full-tree ceiling", async () => { + const rootDir = await mkdtemp(path.join(os.tmpdir(), "paperclip-referenced-raw-buffer-")); + cleanupDirs.push(rootDir); + const repo = await createRepo(rootDir); + let observedMaxBuffer: number | undefined; + setExpensiveWorkspaceGitExecutor(async (input) => { + if (input.operation === "referenced_source.ignored_files") { + observedMaxBuffer = input.maxBuffer; + } + return await runLocalGit(input.localDir, [...input.args], { + timeout: input.timeout, + maxBuffer: input.maxBuffer, + env: input.env, + }); + }); + + await readReferencedSourceGitIgnoredPaths(repo); + + // Enough headroom for a scan within bounds to complete, but a small + // multiple of the byte bound — not the far larger allowance the + // anchor workspace's general-purpose full-tree reads use. + expect(observedMaxBuffer).toBeGreaterThan(REFERENCED_SOURCE_IGNORE_MAX_TOTAL_BYTES); + expect(observedMaxBuffer).toBeLessThan(16 * 1024 * 1024); + }); + + it("does not fail closed on a huge amount of unrelated tracked-change and untracked noise, when the ignored set itself stays in bounds", async () => { + const rootDir = await mkdtemp(path.join(os.tmpdir(), "paperclip-referenced-mixed-status-")); + cleanupDirs.push(rootDir); + const repo = await createRepo(rootDir); + await writeFile(path.join(repo, ".gitignore"), "secret.env\n", "utf8"); + await writeFile(path.join(repo, "secret.env"), "TOKEN=abc\n", "utf8"); + + // Many long-named, untracked, NOT-ignored files at the repository root. + // `git status` reports one record per file (root-level files are never + // collapsed the way an entirely untracked directory is), so this alone + // makes the raw `git status --ignored` response exceed the raw buffer + // bound this scan used to apply to the WHOLE response, well before the + // parser ever got to discard these non-ignored records. The ignored set + // above stays a single small entry throughout. + const noiseNameLength = 220; + const noiseFileCount = 30_000; + const noiseNames = Array.from( + { length: noiseFileCount }, + (_, index) => `${"n".repeat(noiseNameLength - 6)}${String(index).padStart(6, "0")}`, + ); + const writeConcurrency = 200; + for (let start = 0; start < noiseNames.length; start += writeConcurrency) { + const batch = noiseNames.slice(start, start + writeConcurrency); + await Promise.all(batch.map((name) => writeFile(path.join(repo, name), "", "utf8"))); + } + + // Confirm this test actually reproduces the reported defect precondition: + // the raw `git status --ignored` response for this repository state is + // larger than the 4 MiB raw buffer bound the scan used to apply to the + // whole response, not just to the declared ignored-set limits. A large + // explicit maxBuffer is required here only to observe that raw size; + // the scan under test never issues this command. + const rawStatusResult = await runLocalGit( + repo, + ["status", "--ignored", "--porcelain=v1", "-z", "--untracked-files=normal"], + { maxBuffer: 16 * 1024 * 1024 }, + ); + expect(Buffer.byteLength(rawStatusResult.stdout, "utf8")).toBeGreaterThan(REFERENCED_SOURCE_IGNORE_MAX_TOTAL_BYTES * 2); + + const scan = await readReferencedSourceGitIgnoredPaths(repo); + + expect(scan?.ignoredPaths).toEqual(["secret.env"]); + }); + it("routes both scan commands through the registered scheduler instead of spawning git directly", async () => { const rootDir = await mkdtemp(path.join(os.tmpdir(), "paperclip-referenced-scheduler-")); cleanupDirs.push(rootDir); diff --git a/packages/adapter-utils/src/git-workspace-sync.ts b/packages/adapter-utils/src/git-workspace-sync.ts index b8a354c02c..0d80e76cd4 100644 --- a/packages/adapter-utils/src/git-workspace-sync.ts +++ b/packages/adapter-utils/src/git-workspace-sync.ts @@ -40,6 +40,19 @@ export type ExpensiveWorkspaceGitExecutor = ( let expensiveWorkspaceGitExecutor: ExpensiveWorkspaceGitExecutor | null = null; +/** + * The workspace Git scan scheduler's typed code for a saturated queue + * (`server/src/services/workspace-git-operation-scheduler.ts`, + * `WORKSPACE_GIT_SCAN_ERROR_CODES.saturated`). Declared again here because + * `adapter-utils` cannot import from `server` (the reverse direction is + * allowed, not this one); `server` carries a test that asserts the two + * literals stay equal. `resolveReferencedSourceIgnore` in + * `sandbox-managed-runtime.ts` reads this code off a caught error's `code` + * property, never off its message text, to retry only a saturated queue and + * fail closed on every other Git scan error. + */ +export const WORKSPACE_GIT_SCAN_SATURATED_CODE = "workspace_git_scan_saturated"; + /** * Lets a host process apply its process-wide admission policy to the adapter * package's full-tree Git walks. Standalone adapter-utils consumers retain the @@ -161,7 +174,16 @@ export async function readGitWorkspaceSnapshot(localDir: string): Promise value.split("\0").map((entry) => entry.trim()).filter(Boolean); + // `-z` already delimits each record with a NUL byte, so a leading or + // trailing space in a record is part of the path itself, not padding to + // remove — trimming it would resolve to a path that does not exist. A + // length check finds the one genuinely empty record `-z` appends after + // the last NUL, without eating a real path's own leading or trailing + // whitespace. This applies to all four NUL-delimited outputs below (the + // overlay diff, the untracked list, the deleted list, and the ignored + // list); `branchName` and `headCommit` come from non-`-z` commands and + // keep their own `.trim()` above and below, which is safe. + const splitNul = (value: string) => value.split("\0").filter((entry) => entry.length > 0); return { headCommit: headCommitResult.stdout.trim(), branchName: branchName && branchName !== "HEAD" ? branchName : null, @@ -180,7 +202,7 @@ export async function readGitWorkspaceSnapshot(localDir: string): Promise entry.length > 0) - .filter((entry) => entry.startsWith("!! ")) - .map((entry) => entry.slice(3).replace(/\/+$/, "")) - .filter((entry) => entry.length > 0) - .sort((left, right) => left.localeCompare(right)); + + // Read one NUL-delimited record at a time and enforce both bounds while the + // ignored-entry list accumulates, instead of splitting and mapping the + // whole response into a list first and only then checking its size. A + // pathologically large ignore set (a huge repository, or one crafted to + // hold many ignored entries) must fail closed the moment it breaches a + // bound, without this scan first retaining and transforming the full + // oversized response. + // + // Do not trim each entry: `-z` already delimits entries with a NUL byte, so + // a leading or trailing space in an entry is part of the path itself, not + // padding to remove. A length check finds the one genuinely empty record + // `-z` appends after the last NUL, without eating a real path's own + // leading or trailing whitespace. + const rawIgnored = ignoredResult.stdout; + const parsedIgnoredEntries: string[] = []; + let totalIgnoredBytes = 0; + let recordStart = 0; + while (recordStart < rawIgnored.length) { + const nulIndex = rawIgnored.indexOf("\0", recordStart); + const recordEnd = nulIndex === -1 ? rawIgnored.length : nulIndex; + const record = rawIgnored.slice(recordStart, recordEnd); + recordStart = nulIndex === -1 ? rawIgnored.length : nulIndex + 1; + + const entry = record.replace(/\/+$/, ""); + if (entry.length === 0) { + continue; + } + + if (parsedIgnoredEntries.length + 1 > REFERENCED_SOURCE_IGNORE_MAX_ENTRY_COUNT) { + throw new ReferencedSourceIgnoreScanLimitExceededError( + `referenced project ignore scan found more than ${REFERENCED_SOURCE_IGNORE_MAX_ENTRY_COUNT} ignored entries`, + ); + } + totalIgnoredBytes += Buffer.byteLength(entry, "utf8"); + if (totalIgnoredBytes > REFERENCED_SOURCE_IGNORE_MAX_TOTAL_BYTES) { + throw new ReferencedSourceIgnoreScanLimitExceededError( + `referenced project ignore scan exceeded ${REFERENCED_SOURCE_IGNORE_MAX_TOTAL_BYTES} UTF-8 bytes of ignored paths`, + ); + } + parsedIgnoredEntries.push(entry); + } + + // The list is bounded by both checks above, so sorting and re-relativizing + // it here never costs more than the accepted bounds allow. + const ignoredPaths = parsedIgnoredEntries.sort((left, right) => left.localeCompare(right)); return { toplevel, ignoredPaths }; } diff --git a/packages/adapter-utils/src/remote-managed-runtime.test.ts b/packages/adapter-utils/src/remote-managed-runtime.test.ts index 35f4134830..89659bb643 100644 --- a/packages/adapter-utils/src/remote-managed-runtime.test.ts +++ b/packages/adapter-utils/src/remote-managed-runtime.test.ts @@ -26,6 +26,8 @@ vi.mock("./ssh.js", () => ({ })); import { prepareRemoteManagedRuntime } from "./remote-managed-runtime.js"; +import { resolveReferencedSourceIgnore } from "./sandbox-managed-runtime.js"; +import { setExpensiveWorkspaceGitExecutor } from "./git-workspace-sync.js"; describe("remote managed runtime", () => { const cleanupDirs: string[] = []; @@ -262,4 +264,62 @@ describe("remote managed runtime", () => { expect(Object.keys(prepared.additionalSourceDirs)).toEqual(["healthy"]); expect(syncDirectoryToSsh).not.toHaveBeenCalledWith(expect.objectContaining({ localDir: failedDir })); }); + + it("never leaks a raw absolute path into the remote per-project staging warning", async () => { + const rootDir = await mkdtemp(path.join(os.tmpdir(), "paperclip-remote-runtime-redact-")); + cleanupDirs.push(rootDir); + const workspaceDir = path.join(rootDir, "workspace"); + const failedDir = path.join(rootDir, "referenced-failed"); + await mkdir(workspaceDir, { recursive: true }); + await mkdir(failedDir, { recursive: true }); + + // A raw toplevel string that makes `failedDir` a non-descendant, carrying + // a sensitive absolute path — exactly the shape a caught Git diagnostic + // could embed. `resolveReferencedSourceIgnore` is the single choke point + // that must reduce it to the fixed category before anything downstream + // (here, the remote lane's warning) ever sees it. + const sensitivePath = "/srv/alice/project"; + let ignoreResolution; + try { + setExpensiveWorkspaceGitExecutor(async (input) => { + if (input.operation === "referenced_source.toplevel") { + return { stdout: `${sensitivePath}\n`, stderr: "" }; + } + return { stdout: "", stderr: "" }; + }); + ignoreResolution = await resolveReferencedSourceIgnore(failedDir); + } finally { + setExpensiveWorkspaceGitExecutor(null); + } + expect(ignoreResolution.kind).toBe("failed"); + + const warnSpy = vi.spyOn(console, "warn").mockImplementation(() => undefined); + try { + await prepareRemoteManagedRuntime({ + spec: { + host: "127.0.0.1", + port: 2222, + username: "fixture", + remoteWorkspacePath: "/app", + remoteCwd: "/app", + privateKey: "PRIVATE KEY", + knownHosts: "KNOWN HOSTS", + strictHostKeyChecking: true, + }, + runId: "run-redact", + adapterKey: "codex", + workspaceLocalDir: workspaceDir, + workspaceRemoteDir: "/app", + syncWorkspace: false, + additionalSources: [{ localPath: failedDir, projectId: "failed", ignoreResolution }], + }); + + const warnedText = warnSpy.mock.calls.map((call) => call.join(" ")).join("\n"); + expect(warnedText).toContain("failed"); + expect(warnedText).not.toContain(sensitivePath); + expect(warnedText).not.toContain(failedDir); + } finally { + warnSpy.mockRestore(); + } + }); }); diff --git a/packages/adapter-utils/src/sandbox-managed-runtime.test.ts b/packages/adapter-utils/src/sandbox-managed-runtime.test.ts index a889f6b34f..be36585a1e 100644 --- a/packages/adapter-utils/src/sandbox-managed-runtime.test.ts +++ b/packages/adapter-utils/src/sandbox-managed-runtime.test.ts @@ -6,13 +6,19 @@ import path from "node:path"; import { execFile as execFileCallback, spawn } from "node:child_process"; import { promisify } from "node:util"; import { afterEach, describe, expect, it, vi } from "vitest"; -import { resetLocalGitIndexToHead } from "./git-workspace-sync.js"; +import { + resetLocalGitIndexToHead, + runLocalGit, + setExpensiveWorkspaceGitExecutor, + WORKSPACE_GIT_SCAN_SATURATED_CODE, +} from "./git-workspace-sync.js"; import { assertSyncOperationsConfined, escapeTarExcludeLiteral, mirrorDirectory, prepareSandboxManagedRuntime, + REFERENCED_SOURCE_IGNORE_FAILURE_REASONS, resolveReferencedSourceIgnore, type PreparedSandboxManagedRuntime, type ReferencedSourceIgnoreResolution, @@ -1003,6 +1009,65 @@ describe("sandbox managed runtime", () => { expect(downloadMembers.some((entry) => entry.includes("/node_modules/") || entry.endsWith("/node_modules"))).toBe(false); }); + it("excludes an anchor-workspace ignored file whose name has leading and trailing whitespace from the staged tree", async () => { + const rootDir = await mkdtemp(path.join(os.tmpdir(), "paperclip-sandbox-ignored-whitespace-")); + cleanupDirs.push(rootDir); + const workspaceLocalDir = path.join(rootDir, "workspace"); + const remoteWorkspaceDir = path.join(rootDir, "remote-workspace"); + await initGitRepo(workspaceLocalDir); + + // A double-wildcard pattern avoids the separate rule that Git trims an + // unescaped trailing space in a .gitignore PATTERN itself; the padding + // under test lives in the matched FILE name, proving the anchor `splitNul` + // parser keeps it instead of trimming it away and missing the exclude. + const ignoredName = " ignored padded "; + await writeFile(path.join(workspaceLocalDir, ".gitignore"), "*ignored*padded*\n", "utf8"); + await writeFile(path.join(workspaceLocalDir, ignoredName), "TOKEN=abc\n", "utf8"); + await writeFile(path.join(workspaceLocalDir, "kept.txt"), "kept\n", "utf8"); + + const uploadedTars: { remotePath: string; bytes: Buffer }[] = []; + const client: SandboxManagedRuntimeClient = { + makeDir: async (remotePath) => { + await mkdir(remotePath, { recursive: true }); + }, + writeFile: async (remotePath, bytes) => { + await mkdir(path.dirname(remotePath), { recursive: true }); + const buffer = Buffer.from(bytes); + if (remotePath.endsWith("-upload.tar")) uploadedTars.push({ remotePath, bytes: buffer }); + await writeFile(remotePath, buffer); + }, + readFile: async (remotePath) => await readFile(remotePath), + listFiles: async () => [], + remove: async (remotePath) => { + await rm(remotePath, { recursive: true, force: true }); + }, + run: async (command) => { + await execFile("sh", ["-c", command], { maxBuffer: 32 * 1024 * 1024 }); + }, + }; + attachFallbackSyncIn(client); + + await prepareSandboxManagedRuntime({ + spec: { + transport: "sandbox", + provider: "test", + sandboxId: "sandbox-1", + remoteCwd: remoteWorkspaceDir, + timeoutMs: 30_000, + apiKey: null, + }, + adapterKey: "test-adapter", + client, + workspaceLocalDir, + }); + + const workspaceUpload = uploadedTars.find((entry) => path.posix.basename(entry.remotePath) === "workspace-upload.tar"); + expect(workspaceUpload).toBeDefined(); + const members = await listTarMembers(rootDir, "ignored-whitespace-workspace-upload.tar", workspaceUpload!.bytes); + expect(members).not.toContain(ignoredName); + expect(members).toContain("kept.txt"); + }); + it("builds workspace/asset tarballs without a './' self-entry (so untar does not chmod/utime an unowned target dir)", async () => { const rootDir = await mkdtemp(path.join(os.tmpdir(), "paperclip-sandbox-tarself-")); cleanupDirs.push(rootDir); @@ -2460,8 +2525,272 @@ describe("sandbox managed runtime", () => { const resolution = await resolveReferencedSourceIgnore(repo); - expect(resolution.kind).toBe("failed"); - expect((resolution as { reason: string }).reason.length).toBeGreaterThan(0); + // The reason is the fixed category, never the caught error's own message + // (which would embed `repo`, an absolute host path). + expect(resolution).toEqual({ kind: "failed", reason: REFERENCED_SOURCE_IGNORE_FAILURE_REASONS.scanFailed }); + }); + + // Nadia's required probes: none of these three example absolute paths — + // an ordinary POSIX path, a home directory, and a Windows path — may ever + // reach `reason`, however they arrive (a caught Git error, or a raw + // toplevel string that makes `localPath` a non-descendant). + const SENSITIVE_PATH_PROBES = ["/srv/alice/project", "/home/alice/project", "C:\\Users\\alice\\project"]; + + for (const sensitivePath of SENSITIVE_PATH_PROBES) { + it(`redacts a caught Git error embedding ${sensitivePath} to the fixed category`, async () => { + const rootDir = await mkdtemp(path.join(os.tmpdir(), "paperclip-ignore-redact-caught-")); + cleanupDirs.push(rootDir); + const repo = path.join(rootDir, "repo"); + await initGitRepo(repo); + try { + setExpensiveWorkspaceGitExecutor(async (input) => { + if (input.operation === "referenced_source.ignored_files") { + throw Object.assign( + new Error(`fatal: unable to read tree object for ${sensitivePath}, pid 4242`), + { stderr: `fatal: unable to read tree object for ${sensitivePath}, pid 4242` }, + ); + } + return await runLocalGit(input.localDir, [...input.args], { + timeout: input.timeout, + maxBuffer: input.maxBuffer, + env: input.env, + }); + }); + + const resolution = await resolveReferencedSourceIgnore(repo); + + expect(resolution).toEqual({ kind: "failed", reason: REFERENCED_SOURCE_IGNORE_FAILURE_REASONS.scanFailed }); + } finally { + setExpensiveWorkspaceGitExecutor(null); + } + }); + + it(`redacts a non-descendant toplevel embedding ${sensitivePath} to the fixed category`, async () => { + const rootDir = await mkdtemp(path.join(os.tmpdir(), "paperclip-ignore-redact-nondescendant-")); + cleanupDirs.push(rootDir); + const localPath = path.join(rootDir, "referenced"); + await mkdir(localPath, { recursive: true }); + try { + // A toplevel string with no relation to `localPath` — the resolver + // must treat it as a non-descendant and fail closed with the fixed + // category, never a message built from `sensitivePath` or `localPath`. + setExpensiveWorkspaceGitExecutor(async (input) => { + if (input.operation === "referenced_source.toplevel") { + return { stdout: `${sensitivePath}\n`, stderr: "" }; + } + return { stdout: "", stderr: "" }; + }); + + const resolution = await resolveReferencedSourceIgnore(localPath); + + expect(resolution).toEqual({ kind: "failed", reason: REFERENCED_SOURCE_IGNORE_FAILURE_REASONS.toplevelNotDescendant }); + } finally { + setExpensiveWorkspaceGitExecutor(null); + } + }); + } + + it("fails closed with a fixed category when the parsed ignored-entry count exceeds the bound", async () => { + const rootDir = await mkdtemp(path.join(os.tmpdir(), "paperclip-ignore-bound-count-")); + cleanupDirs.push(rootDir); + const repo = path.join(rootDir, "repo"); + await initGitRepo(repo); + const overLimitCount = 10_001; + const syntheticIgnored = `${Array.from({ length: overLimitCount }, (_, index) => `!! entry-${index}`).join("\0")}\0`; + try { + setExpensiveWorkspaceGitExecutor(async (input) => { + if (input.operation === "referenced_source.ignored_files") { + return { stdout: syntheticIgnored, stderr: "" }; + } + return await runLocalGit(input.localDir, [...input.args], { + timeout: input.timeout, + maxBuffer: input.maxBuffer, + env: input.env, + }); + }); + + const resolution = await resolveReferencedSourceIgnore(repo); + + expect(resolution).toEqual({ kind: "failed", reason: REFERENCED_SOURCE_IGNORE_FAILURE_REASONS.limitExceeded }); + } finally { + setExpensiveWorkspaceGitExecutor(null); + } + }); + + it("fails closed with a fixed category when the total UTF-8 byte size of ignored paths exceeds the bound", async () => { + const rootDir = await mkdtemp(path.join(os.tmpdir(), "paperclip-ignore-bound-bytes-")); + cleanupDirs.push(rootDir); + const repo = path.join(rootDir, "repo"); + await initGitRepo(repo); + // One entry alone exceeds the 2 MiB bound, well under the entry-count bound. + const hugeEntry = "a".repeat(3 * 1024 * 1024); + const syntheticIgnored = `!! ${hugeEntry}\0`; + try { + setExpensiveWorkspaceGitExecutor(async (input) => { + if (input.operation === "referenced_source.ignored_files") { + return { stdout: syntheticIgnored, stderr: "" }; + } + return await runLocalGit(input.localDir, [...input.args], { + timeout: input.timeout, + maxBuffer: input.maxBuffer, + env: input.env, + }); + }); + + const resolution = await resolveReferencedSourceIgnore(repo); + + expect(resolution).toEqual({ kind: "failed", reason: REFERENCED_SOURCE_IGNORE_FAILURE_REASONS.limitExceeded }); + } finally { + setExpensiveWorkspaceGitExecutor(null); + } + }); + + it("stages no bytes for either a count-breach or a byte-breach project, and stages a healthy sibling", async () => { + const rootDir = await mkdtemp(path.join(os.tmpdir(), "paperclip-ignore-bound-staging-")); + cleanupDirs.push(rootDir); + const localWorkspaceDir = path.join(rootDir, "local-workspace"); + const remoteWorkspaceDir = path.join(rootDir, "remote-workspace"); + const healthyDir = path.join(rootDir, "referenced-healthy"); + const countBreachDir = path.join(rootDir, "referenced-count-breach"); + const byteBreachDir = path.join(rootDir, "referenced-byte-breach"); + await mkdir(localWorkspaceDir, { recursive: true }); + await mkdir(healthyDir, { recursive: true }); + await mkdir(countBreachDir, { recursive: true }); + await mkdir(byteBreachDir, { recursive: true }); + await writeFile(path.join(localWorkspaceDir, "README.md"), "anchor\n", "utf8"); + await writeFile(path.join(healthyDir, "notes.md"), "healthy\n", "utf8"); + await writeFile(path.join(countBreachDir, "should-never-ship.txt"), "must not stage\n", "utf8"); + await writeFile(path.join(byteBreachDir, "should-never-ship.txt"), "must not stage\n", "utf8"); + + const countBreachReason = { kind: "failed" as const, reason: REFERENCED_SOURCE_IGNORE_FAILURE_REASONS.limitExceeded }; + const byteBreachReason = { kind: "failed" as const, reason: REFERENCED_SOURCE_IGNORE_FAILURE_REASONS.limitExceeded }; + + const prepared = await prepareCommandManagedRuntime({ + runner: makeInlineSpawnRunner(), + spec: { remoteCwd: remoteWorkspaceDir, timeoutMs: 30_000 }, + adapterKey: "test-adapter", + workspaceLocalDir: localWorkspaceDir, + additionalSources: [ + { localPath: healthyDir, projectId: "healthy", ignoreResolution: { kind: "other" } }, + { localPath: countBreachDir, projectId: "count-breach", ignoreResolution: countBreachReason }, + { localPath: byteBreachDir, projectId: "byte-breach", ignoreResolution: byteBreachReason }, + ], + }); + + expect(Object.keys(prepared.additionalSourceDirs).sort()).toEqual(["healthy"]); + expect(prepared.additionalSourceFailures.map((failure) => failure.projectId).sort()).toEqual([ + "byte-breach", + "count-breach", + ]); + const runtimeRootDir = path.posix.join(remoteWorkspaceDir, ".paperclip-runtime", "test-adapter"); + await expect(readFile(path.join(runtimeRootDir, "project-count-breach", "should-never-ship.txt"), "utf8")).rejects + .toMatchObject({ code: "ENOENT" }); + await expect(readFile(path.join(runtimeRootDir, "project-byte-breach", "should-never-ship.txt"), "utf8")).rejects + .toMatchObject({ code: "ENOENT" }); + }); + + describe("saturation retry", () => { + afterEach(() => { + vi.useRealTimers(); + setExpensiveWorkspaceGitExecutor(null); + }); + + function throwSaturated(): never { + throw Object.assign( + new Error("Changed files are temporarily unavailable because the Git scan queue is full"), + { code: WORKSPACE_GIT_SCAN_SATURATED_CODE }, + ); + } + + // Every case here synthesizes BOTH scan operations at the executor seam + // instead of spawning real `git` — the property under test is the + // retry's own timing and attempt count, and a fake-timer-driven test + // must not also depend on a real child process's independent, real-time + // completion. `toplevel` need not exist on disk: `resolveReferencedSourceIgnore` + // falls back to a plain string compare when `fs.realpath` fails, and the + // fixture path is used unchanged on both sides of that compare. + const repo = "/fixture/referenced-project"; + + it("retries a saturated scan up to two times and succeeds on the third attempt", async () => { + let ignoredCallCount = 0; + setExpensiveWorkspaceGitExecutor(async (input) => { + if (input.operation === "referenced_source.toplevel") { + return { stdout: `${repo}\n`, stderr: "" }; + } + ignoredCallCount += 1; + if (ignoredCallCount <= 2) throwSaturated(); + return { stdout: "", stderr: "" }; + }); + + vi.useFakeTimers(); + const resolutionPromise = resolveReferencedSourceIgnore(repo); + // Bounded backoff: none before attempt 1, 1 s before attempt 2, 2 s + // before attempt 3. + await vi.advanceTimersByTimeAsync(1_000); + await vi.advanceTimersByTimeAsync(2_000); + const resolution = await resolutionPromise; + + expect(resolution).toEqual({ kind: "git", ignoredPaths: [] }); + expect(ignoredCallCount).toBe(3); + }); + + it("fails closed after three saturated attempts, with no further Git invocation", async () => { + let ignoredCallCount = 0; + setExpensiveWorkspaceGitExecutor(async (input) => { + if (input.operation === "referenced_source.toplevel") { + return { stdout: `${repo}\n`, stderr: "" }; + } + ignoredCallCount += 1; + throwSaturated(); + }); + + vi.useFakeTimers(); + const resolutionPromise = resolveReferencedSourceIgnore(repo); + await vi.advanceTimersByTimeAsync(1_000); + await vi.advanceTimersByTimeAsync(2_000); + const resolution = await resolutionPromise; + + expect(resolution).toEqual({ kind: "failed", reason: REFERENCED_SOURCE_IGNORE_FAILURE_REASONS.scanFailed }); + // Three total attempts (the first plus two retries) — no fourth, + // unscheduled invocation past the retry budget. + expect(ignoredCallCount).toBe(3); + }); + + it("makes exactly one attempt and fails closed on a timeout, never retrying it", async () => { + let ignoredCallCount = 0; + setExpensiveWorkspaceGitExecutor(async (input) => { + if (input.operation === "referenced_source.toplevel") { + return { stdout: `${repo}\n`, stderr: "" }; + } + ignoredCallCount += 1; + throw Object.assign(new Error("Workspace Git scan timed out after 8000ms"), { + code: "workspace_git_scan_timeout", + }); + }); + + const resolution = await resolveReferencedSourceIgnore(repo); + + expect(resolution).toEqual({ kind: "failed", reason: REFERENCED_SOURCE_IGNORE_FAILURE_REASONS.scanFailed }); + expect(ignoredCallCount).toBe(1); + }); + + it("makes exactly one attempt and fails closed on an output-limit breach, never retrying it", async () => { + let ignoredCallCount = 0; + setExpensiveWorkspaceGitExecutor(async (input) => { + if (input.operation === "referenced_source.toplevel") { + return { stdout: `${repo}\n`, stderr: "" }; + } + ignoredCallCount += 1; + throw Object.assign(new Error("Workspace Git scan exceeded its output limit"), { + code: "workspace_git_scan_output_limit", + }); + }); + + const resolution = await resolveReferencedSourceIgnore(repo); + + expect(resolution).toEqual({ kind: "failed", reason: REFERENCED_SOURCE_IGNORE_FAILURE_REASONS.scanFailed }); + expect(ignoredCallCount).toBe(1); + }); }); }); diff --git a/packages/adapter-utils/src/sandbox-managed-runtime.ts b/packages/adapter-utils/src/sandbox-managed-runtime.ts index bd5d5c5e71..d3de812500 100644 --- a/packages/adapter-utils/src/sandbox-managed-runtime.ts +++ b/packages/adapter-utils/src/sandbox-managed-runtime.ts @@ -14,9 +14,11 @@ import { GIT_ARCHIVE_EXCLUDES, integrateImportedGitHead, readGitWorkspaceSnapshot, + ReferencedSourceIgnoreScanLimitExceededError, readReferencedSourceGitIgnoredPaths, resetLocalGitIndexToHead, withShallowGitWorkspaceClone, + WORKSPACE_GIT_SCAN_SATURATED_CODE, } from "./git-workspace-sync.js"; import { captureDirectorySnapshot, mergeDirectoryWithBaseline } from "./workspace-restore-merge.js"; import { @@ -147,16 +149,39 @@ export interface SandboxManagedRuntimeAsset { * `resolveReferencedSourceIgnore`). * - `other`: `localPath` is not a Git work tree. The staging path keeps * today's fixed heavy-directory excludes. - * - `failed`: the Git read failed, timed out, or returned output the - * resolver could not parse or safely re-relativize. The project is NOT - * staged (fail closed) — every site records it as a per-project failure - * instead of shipping it unfiltered. + * - `failed`: the Git read failed, timed out, breached a parse bound, or + * returned output the resolver could not safely re-relativize. The project + * is NOT staged (fail closed) — every site records it as a per-project + * failure instead of shipping it unfiltered. `reason` is always one of + * {@link REFERENCED_SOURCE_IGNORE_FAILURE_REASONS} — never a raw Git or tar + * diagnostic, an absolute host path, or a basename. */ export type ReferencedSourceIgnoreResolution = | { kind: "git"; ignoredPaths: string[] } | { kind: "other" } | { kind: "failed"; reason: string }; +/** + * The fixed, allowlisted failure categories a `failed` + * {@link ReferencedSourceIgnoreResolution} reports as `reason`. This is the + * ENTIRE vocabulary: no absolute host path, no basename, no opaque token, and + * no raw Git or tar stderr ever reaches `reason` — only one of these three + * stable strings, chosen once at the single construction point in + * `resolveReferencedSourceIgnore`. A `failed` resolution always prevents + * staging and is always re-resolved before its next use, so two different + * underlying failures colliding on the same category (e.g. a timeout and a + * malformed-output error both reporting `scanFailed`) never weakens the + * fail-closed decision. + */ +export const REFERENCED_SOURCE_IGNORE_FAILURE_REASONS = { + /** A Git read failed, timed out, was cancelled, or returned malformed output — including a saturated scan queue that never recovered after its retries. */ + scanFailed: "git-ignore-scan-failed", + /** The parsed ignored-entry count or total UTF-8 byte size breached its bound (see `readReferencedSourceGitIgnoredPaths`). */ + limitExceeded: "git-ignore-scan-limit-exceeded", + /** The referenced project's `localPath` is not a descendant of its own Git top level. */ + toplevelNotDescendant: "git-toplevel-not-descendant", +} as const; + /** * A referenced (additional) project to stage into the run sandbox as a plain, * read-only tree. `localPath` is the host checkout directory. Upstream code @@ -240,7 +265,7 @@ function relativizeUnderGitToplevel(input: { toplevel: string; localPath: string } /** - * Re-relativize root-relative ignored paths (as `git status --ignored` + * Re-relativize root-relative ignored paths (as `readReferencedSourceGitIgnoredPaths` * reports them, from the repository toplevel) to `offset`, the position of * the referenced project's `localPath` under that toplevel. Keeps only the * entries that are `offset` itself or a descendant of it — an ignored path @@ -259,23 +284,78 @@ function reRelativizeIgnoredPathsToLocalPath(input: { ignoredPaths: string[]; of .filter(Boolean); } +/** + * Bounded backoff before each retry of a saturated Git scan: none before the + * first attempt, 1 second before the second, 2 seconds before the third. Three + * total attempts (the first plus these two retries) is a liveness parameter, + * not a security control — the retry only ever fires for the scheduler's + * typed saturation code (see {@link isWorkspaceGitScanSaturatedError}). + */ +const REFERENCED_SOURCE_IGNORE_SCAN_RETRY_DELAYS_MS = [1_000, 2_000] as const; + +async function delay(ms: number): Promise { + await new Promise((resolve) => setTimeout(resolve, ms)); +} + +/** + * True only when `error` carries the workspace Git scan scheduler's typed + * saturation code on its `code` property. Matches the code alone, never + * message text — a message can change wording without changing meaning, and + * matching text would silently stop retrying (or start retrying the wrong + * failure) the moment it did. + */ +function isWorkspaceGitScanSaturatedError(error: unknown): boolean { + return ( + typeof error === "object" && + error !== null && + "code" in error && + (error as { code?: unknown }).code === WORKSPACE_GIT_SCAN_SATURATED_CODE + ); +} + /** * Resolve a referenced project's Git-ignored paths ONCE, before any staging * site runs. Called once per project (see `execute.ts`); the sandbox lane, * the SSH lane, and the content-signature walk all consume this one result, * so they can never apply a different exclusion set to the same project. * - * Fails closed: a Git read error, a timeout, malformed output, or a `localPath` - * that is not a plain descendant of its own Git toplevel all return `failed`, - * never an empty ignore list — an empty list means "resolved, nothing extra to - * exclude", which is a different claim than "the resolution did not run". + * Fails closed: a Git read error, a timeout, malformed output, a parse-bound + * breach, or a `localPath` that is not a plain descendant of its own Git + * toplevel all return `failed`, never an empty ignore list — an empty list + * means "resolved, nothing extra to exclude", which is a different claim than + * "the resolution did not run". + * + * Retries ONLY a saturated scan queue (the shared workspace Git operation + * scheduler rejecting before spawn because it is at capacity) — a liveness + * condition, not an integrity one. Three attempts total, with the bounded + * backoff in {@link REFERENCED_SOURCE_IGNORE_SCAN_RETRY_DELAYS_MS}, retried + * through the same registered scheduler every time. No direct-spawn fallback + * exists: bypassing the scheduler would defeat the process-wide concurrency + * limit it enforces. Every other failure — timeout, cancellation, an output + * limit, a permission error, malformed output, a real Git failure, or a bound + * breach — makes exactly one attempt and fails closed immediately. */ export async function resolveReferencedSourceIgnore(localPath: string): Promise { - let scan: Awaited>; - try { - scan = await readReferencedSourceGitIgnoredPaths(localPath); - } catch (error) { - return { kind: "failed", reason: error instanceof Error ? error.message : String(error) }; + let scan: Awaited> = null; + let failureReason: string | null = null; + for (let attempt = 0; attempt <= REFERENCED_SOURCE_IGNORE_SCAN_RETRY_DELAYS_MS.length; attempt += 1) { + try { + scan = await readReferencedSourceGitIgnoredPaths(localPath); + failureReason = null; + break; + } catch (error) { + failureReason = error instanceof ReferencedSourceIgnoreScanLimitExceededError + ? REFERENCED_SOURCE_IGNORE_FAILURE_REASONS.limitExceeded + : REFERENCED_SOURCE_IGNORE_FAILURE_REASONS.scanFailed; + const isLastAttempt = attempt === REFERENCED_SOURCE_IGNORE_SCAN_RETRY_DELAYS_MS.length; + if (isLastAttempt || !isWorkspaceGitScanSaturatedError(error)) { + break; + } + await delay(REFERENCED_SOURCE_IGNORE_SCAN_RETRY_DELAYS_MS[attempt]!); + } + } + if (failureReason !== null) { + return { kind: "failed", reason: failureReason }; } if (!scan) { return { kind: "other" }; @@ -285,10 +365,7 @@ export async function resolveReferencedSourceIgnore(localPath: string): Promise< localPath: await physicalPath(localPath), }); if (offset === null) { - return { - kind: "failed", - reason: `referenced project path is not a descendant of its own Git top level: ${localPath} under ${scan.toplevel}`, - }; + return { kind: "failed", reason: REFERENCED_SOURCE_IGNORE_FAILURE_REASONS.toplevelNotDescendant }; } return { kind: "git", diff --git a/server/src/services/workspace-git-operation-scheduler.test.ts b/server/src/services/workspace-git-operation-scheduler.test.ts index abd9a36930..72fbb352a4 100644 --- a/server/src/services/workspace-git-operation-scheduler.test.ts +++ b/server/src/services/workspace-git-operation-scheduler.test.ts @@ -9,6 +9,7 @@ import { workspaceGitSchedulerOptionsFromEnv, type WorkspaceGitRunner, } from "./workspace-git-operation-scheduler.js"; +import { WORKSPACE_GIT_SCAN_SATURATED_CODE } from "@paperclipai/adapter-utils/git-workspace-sync"; const tempPaths: string[] = []; @@ -429,3 +430,13 @@ describe("WorkspaceGitOperationScheduler", () => { expect(scheduler.snapshot()).toMatchObject({ activeCount: 0, queuedCount: 0, inFlightCount: 0 }); }); }); + +describe("WORKSPACE_GIT_SCAN_SATURATED_CODE parity", () => { + it("stays equal to WORKSPACE_GIT_SCAN_ERROR_CODES.saturated", () => { + // `adapter-utils` cannot import this module (the reverse direction is + // allowed, not this one), so `resolveReferencedSourceIgnore` declares its + // own copy of the saturation code to key its retry off. This test is the + // one place both literals meet, so the two copies cannot drift apart. + expect(WORKSPACE_GIT_SCAN_SATURATED_CODE).toBe(WORKSPACE_GIT_SCAN_ERROR_CODES.saturated); + }); +});