refactor(daytona): simplify the inbound file-sync path (#12329)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - Sandbox providers run agent work in isolated environments > - The Daytona inbound file-sync path writes selected sandbox data to host paths > - Its in-sandbox lexical and realpath guards do not protect host-write or Paperclip API authority > - This pull request removes those guards and simplifies the inbound file-sync commands > - The benefit is a smaller path with the same outbound controls and host extraction validation ## Linked Issues or Issue Description No public issue exists for this change. This pull request describes the enhancement. **What existing behavior does this improve?** It improves inbound file and directory synchronization for the Daytona sandbox provider. **Subsystem affected** `packages/plugins` — the Daytona sandbox provider. **Current behavior** The inbound path checks lexical and realpath confinement inside the sandbox. It uses file-descriptor-pinned commands for file promotion, archive extraction, and decompression. These checks do not protect host-write or Paperclip API authority. **Proposed behavior** Remove the inbound lexical and realpath checks. Use `mv -f` for file mappings, `tar -xf` for directory mappings, and `zstd -d -o` for decompression. Keep outbound source checks, atomic snapshot downloads, and tarball member validation. **Reason and benefit** The sandbox boundary protects the relevant authorities. The removed guards run inside that boundary and only produce early errors. The simpler commands reduce code and preserve the controls that protect the host boundary. **Breaking changes** The inbound path no longer rejects mappings because of sandbox-side lexical or realpath confinement. Outbound source validation and tarball member validation remain unchanged. ## What Changed - Remove lexical and realpath confinement guards from inbound file mappings, inbound directory mappings, and post-upload command working directories. - Replace file-descriptor-pinned promotion with one `mv -f` command per file mapping. - Replace file-descriptor-pinned extraction with one `tar -xf` command per directory mapping. - Replace retained-descriptor decompression with `zstd -d -o`. - Keep outbound source guards, atomic snapshot-and-download, and tarball member validation. - Update Daytona tests for the new command shapes and remove tests for the removed rejections. ## Verification - The author ran the Daytona package test suite: 232 tests passed and 6 tests skipped. - The author ran the Daytona package typecheck successfully. - Review the diff and confirm it changes only the three Daytona files named in this description. - Confirm the Storybook check may report `SKIPPED` as an expected repository state. ## Risks The inbound path now trusts the sandbox boundary for host-write protection. A later change that gives sandbox code host-write or Paperclip API authority could require new guards. Outbound source checks and archive member validation remain in place. No Kubernetes or core runtime file changes exist. ## Model Used OpenAI GPT-5; exact model version and context window were not provided; tool use and code review assistance. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes: #` / `Refs: #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [x] All Paperclip CI gates are green - [x] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge --------- Co-authored-by: Paperclip <noreply@paperclip.ing>
This commit is contained in:
parent
036600d922
commit
3d9be6f7fe
|
|
@ -157,6 +157,35 @@ describe("daytona file-sync inbound scratch cleanup", () => {
|
|||
);
|
||||
expect(standaloneRemoves).toHaveLength(0);
|
||||
});
|
||||
|
||||
it("writes an inbound file mapping to a sandbox path outside the workspace root", async () => {
|
||||
const hostDir = await fs.mkdtemp(path.join(os.tmpdir(), "paperclip-daytona-outside-root-"));
|
||||
cleanupDirs.push(hostDir);
|
||||
const sourcePath = path.join(hostDir, "source.txt");
|
||||
await fs.writeFile(sourcePath, "payload");
|
||||
|
||||
// A target outside the workspace remote dir. The inbound direction writes
|
||||
// host data into the sandbox, and the sandbox already has read/write
|
||||
// authority over its own filesystem, so the provider no longer confines
|
||||
// the target to the remote dir.
|
||||
const remoteDir = "/workspace";
|
||||
const targetPath = "/etc/paperclip-outside-root.txt";
|
||||
const uploadedDestinations: string[] = [];
|
||||
const commands: RecordedCommand[] = [];
|
||||
const sandbox = createMockSandbox({ uploadedDestinations, commands });
|
||||
|
||||
const operations: PluginSyncOperation[] = [{
|
||||
operationId: "sync-op-outside-root",
|
||||
files: [{ sourcePath, targetPath, kind: "file" }],
|
||||
}];
|
||||
|
||||
const result = await performSyncIn({ sandbox: sandbox as never, operations, remoteDir, timeoutSeconds: 30 });
|
||||
|
||||
expect(result.operations[0].filesTransferred).toBe(1);
|
||||
const promoteCommand = commands.map((entry) => entry.command).find((command) => command.includes("mv -f"));
|
||||
expect(promoteCommand).toBeDefined();
|
||||
expect(promoteCommand).toContain(targetPath);
|
||||
});
|
||||
});
|
||||
|
||||
// ---------------------------------------------------------------
|
||||
|
|
@ -191,24 +220,6 @@ async function writeIncompressibleFile(filePath: string, sizeBytes: number): Pro
|
|||
await fs.writeFile(filePath, crypto.randomBytes(sizeBytes));
|
||||
}
|
||||
|
||||
/**
|
||||
* Extract the raw scratch pathname from a promote-script command's own text.
|
||||
* The promote script embeds the path inside `shellQuote`, and the WHOLE
|
||||
* script is itself `shellQuote`d again for the outer `sh -c` wrapper — so the
|
||||
* literal `'...'` delimiters around the path get escaped and are not a
|
||||
* reliable anchor. The path text itself has no shell metacharacters (it is
|
||||
* `remoteDir` + a UUID-based scratch name), so it survives both quoting
|
||||
* passes unchanged and is matched directly instead. The script always emits
|
||||
* the raw scratch's `exec 9> ...` line before the `.zst` scratch's
|
||||
* `zstd -d -c -- ...` line, so the FIRST match is always the raw name.
|
||||
*/
|
||||
function extractRawScratchPath(command: string, remoteDir: string): string {
|
||||
const escapedRoot = remoteDir.replace(/[.*+?^${}()|[\]\\]/g, "\\$&");
|
||||
const match = command.match(new RegExp(`${escapedRoot}/\\.paperclip-upload-[0-9a-f-]+`));
|
||||
if (!match) throw new Error("test setup: could not find the raw scratch path in the promote script");
|
||||
return match[0];
|
||||
}
|
||||
|
||||
/**
|
||||
* A lightweight recording sandbox double for the fallback-condition tests: it
|
||||
* never runs a real shell, only records commands and reports a canned exit
|
||||
|
|
@ -251,27 +262,16 @@ function createRecordingSandbox(input: {
|
|||
* `setFilePermissions` apply real bytes/modes onto a real directory standing
|
||||
* in for the sandbox root. This proves the decompression/promotion script
|
||||
* for real, instead of only recording which commands the code would send.
|
||||
*
|
||||
* `beforeCommand` runs (and may `await` a filesystem mutation) immediately
|
||||
* before each real command executes. The race-regression test uses it to
|
||||
* plant a pre-created scratch name at exactly the moment a sandbox peer could
|
||||
* have observed the pathname (the R2 residual the design discloses), and the
|
||||
* parent-swap test uses it to swap a target's parent dir between the
|
||||
* confinement guard and the promotion round trip.
|
||||
*/
|
||||
function createRealExecSandbox(input?: {
|
||||
beforeCommand?: (command: string, index: number) => void | Promise<void>;
|
||||
uploadOverride?: (uploads: Array<{ source: string; destination: string }>) => Promise<boolean>;
|
||||
}) {
|
||||
const commands: RecordedCommand[] = [];
|
||||
let index = 0;
|
||||
return {
|
||||
commands,
|
||||
sandbox: {
|
||||
process: {
|
||||
executeCommand: async (command: string) => {
|
||||
await input?.beforeCommand?.(command, index);
|
||||
index += 1;
|
||||
commands.push({ command });
|
||||
const result = spawnSync("/bin/sh", ["-c", command], { encoding: "utf8", maxBuffer: 64 * 1024 * 1024 });
|
||||
return { exitCode: result.status ?? 1, result: (result.stdout ?? "") + (result.stderr ?? "") };
|
||||
|
|
@ -325,9 +325,9 @@ describe("daytona file-sync inbound zstd transport compression", () => {
|
|||
|
||||
expect(result.operations[0].filesTransferred).toBe(1);
|
||||
expect(result.operations[0].bytesTransferred).toBe(ZSTD_MIN_SOURCE_BYTES_FOR_TEST + 1024);
|
||||
// The promote script actually ran a real `zstd -d -c` — proves decompression
|
||||
// The promote script actually ran a real `zstd -d -o` — proves decompression
|
||||
// happened, not just that the code called `uploadFiles`.
|
||||
expect(commands.some((entry) => entry.command.includes("zstd -d -c"))).toBe(true);
|
||||
expect(commands.some((entry) => entry.command.includes("zstd -d -o"))).toBe(true);
|
||||
expect(await sha256OfFile(targetPath)).toBe(await sha256OfFile(sourcePath));
|
||||
|
||||
// Cleanup on success: no reserved scratch (raw or `.zst`) remains.
|
||||
|
|
@ -506,109 +506,7 @@ describe("daytona file-sync inbound zstd transport compression", () => {
|
|||
expect(remaining.filter((name) => name.includes(".paperclip-upload"))).toHaveLength(0); // scratch swept
|
||||
});
|
||||
|
||||
it("refuses a pre-created regular file at the raw scratch name (race regression, C1)", async () => {
|
||||
const remoteDir = await mkTempDir("paperclip-daytona-zstd-remote-");
|
||||
const hostDir = await mkTempDir("paperclip-daytona-zstd-host-");
|
||||
const sourcePath = path.join(hostDir, "workspace-upload.tar");
|
||||
await writeCompressibleFile(sourcePath, ZSTD_MIN_SOURCE_BYTES_FOR_TEST + 1024);
|
||||
const targetPath = path.posix.join(remoteDir, "target.bin");
|
||||
|
||||
let injected = false;
|
||||
const { sandbox } = createRealExecSandbox({
|
||||
beforeCommand: async (command) => {
|
||||
if (injected || !command.includes("zstd -d -c")) return;
|
||||
const rawScratchPath = extractRawScratchPath(command, remoteDir);
|
||||
injected = true;
|
||||
// A peer that reads this command's own text (the R2 residual) claims
|
||||
// the reserved name first, as a plain pre-existing file.
|
||||
await fs.writeFile(rawScratchPath, "attacker-controlled pre-existing content");
|
||||
},
|
||||
});
|
||||
const operations: PluginSyncOperation[] = [{
|
||||
operationId: "sync-op-1",
|
||||
files: [{ sourcePath, targetPath, kind: "file" }],
|
||||
}];
|
||||
|
||||
await expect(
|
||||
performSyncIn({ sandbox: sandbox as never, operations, remoteDir, timeoutSeconds: 30 }),
|
||||
).rejects.toThrow();
|
||||
|
||||
expect(injected).toBe(true);
|
||||
await expect(fs.stat(targetPath)).rejects.toThrow(); // never promoted
|
||||
});
|
||||
|
||||
it("refuses a pre-created symlink at the raw scratch name and never writes through it (race regression, C1)", async () => {
|
||||
const remoteDir = await mkTempDir("paperclip-daytona-zstd-remote-");
|
||||
const hostDir = await mkTempDir("paperclip-daytona-zstd-host-");
|
||||
const sourcePath = path.join(hostDir, "workspace-upload.tar");
|
||||
await writeCompressibleFile(sourcePath, ZSTD_MIN_SOURCE_BYTES_FOR_TEST + 1024);
|
||||
const targetPath = path.posix.join(remoteDir, "target.bin");
|
||||
const sentinelPath = path.join(hostDir, "outside-the-workspace-root.txt");
|
||||
const sentinelContent = "PRE-EXISTING CONTENT, MUST SURVIVE UNCHANGED\n";
|
||||
await fs.writeFile(sentinelPath, sentinelContent);
|
||||
|
||||
let injected = false;
|
||||
const { sandbox } = createRealExecSandbox({
|
||||
beforeCommand: async (command) => {
|
||||
if (injected || !command.includes("zstd -d -c")) return;
|
||||
const rawScratchPath = extractRawScratchPath(command, remoteDir);
|
||||
injected = true;
|
||||
// A peer that reads this command's own text (the R2 residual) claims
|
||||
// the reserved name first, as a symlink pointing OUTSIDE the workspace
|
||||
// root — the attack shape a create-with-mode-not-check-then-create
|
||||
// primitive must refuse.
|
||||
await fs.symlink(sentinelPath, rawScratchPath);
|
||||
},
|
||||
});
|
||||
const operations: PluginSyncOperation[] = [{
|
||||
operationId: "sync-op-1",
|
||||
files: [{ sourcePath, targetPath, kind: "file" }],
|
||||
}];
|
||||
|
||||
await expect(
|
||||
performSyncIn({ sandbox: sandbox as never, operations, remoteDir, timeoutSeconds: 30 }),
|
||||
).rejects.toThrow();
|
||||
|
||||
expect(injected).toBe(true);
|
||||
await expect(fs.stat(targetPath)).rejects.toThrow(); // never promoted
|
||||
// The symlink's target was never opened/written through: content unchanged,
|
||||
// and no write landed outside the workspace root.
|
||||
expect(await fs.readFile(sentinelPath, "utf8")).toBe(sentinelContent);
|
||||
});
|
||||
|
||||
it("still fails at the fd re-verification when a target's parent dir is swapped after the confinement guard", async () => {
|
||||
const remoteDir = await mkTempDir("paperclip-daytona-zstd-remote-");
|
||||
const hostDir = await mkTempDir("paperclip-daytona-zstd-host-");
|
||||
const outsideDir = await mkTempDir("paperclip-daytona-zstd-outside-");
|
||||
const realParentDir = path.posix.join(remoteDir, "nested");
|
||||
const targetPath = path.posix.join(realParentDir, "target.bin");
|
||||
const sourcePath = path.join(hostDir, "small.bin");
|
||||
await fs.writeFile(sourcePath, "small file, well below the compression threshold\n");
|
||||
|
||||
let swapped = false;
|
||||
const { sandbox } = createRealExecSandbox({
|
||||
beforeCommand: async (_command, index) => {
|
||||
// index 0 = mkdir+probe, index 1 = checkSymlinkEscape, index 2 = promote.
|
||||
if (index !== 2 || swapped) return;
|
||||
swapped = true;
|
||||
await fs.rm(realParentDir, { recursive: true, force: true });
|
||||
await fs.symlink(outsideDir, realParentDir);
|
||||
},
|
||||
});
|
||||
const operations: PluginSyncOperation[] = [{
|
||||
operationId: "sync-op-1",
|
||||
files: [{ sourcePath, targetPath, kind: "file" }],
|
||||
}];
|
||||
|
||||
await expect(
|
||||
performSyncIn({ sandbox: sandbox as never, operations, remoteDir, timeoutSeconds: 30 }),
|
||||
).rejects.toThrow(/ESCAPE|syncIn rename/);
|
||||
|
||||
expect(swapped).toBe(true);
|
||||
expect(await fs.readdir(outsideDir)).toHaveLength(0); // nothing landed outside the root
|
||||
});
|
||||
|
||||
it("applies mapping.mode via the retained descriptor, defaulting to 0600 when unset", async () => {
|
||||
it("applies mapping.mode via chmod before promotion, when set", async () => {
|
||||
const remoteDir = await mkTempDir("paperclip-daytona-zstd-remote-");
|
||||
const hostDir = await mkTempDir("paperclip-daytona-zstd-host-");
|
||||
const sourceNoMode = path.join(hostDir, "no-mode.tar");
|
||||
|
|
@ -618,7 +516,21 @@ describe("daytona file-sync inbound zstd transport compression", () => {
|
|||
const targetNoMode = path.posix.join(remoteDir, "no-mode.bin");
|
||||
const targetWithMode = path.posix.join(remoteDir, "with-mode.bin");
|
||||
|
||||
const { sandbox } = createRealExecSandbox();
|
||||
// `zstd -d -o` copies the mode of its INPUT (the uploaded `.zst`
|
||||
// scratch) onto its output. Widen every `.zst` scratch to 0644 here,
|
||||
// standing in for a Daytona upload that does not preserve a host-side
|
||||
// 0600 origin. A pass on the no-mode assertion below then proves the
|
||||
// promote script's OWN `chmod` forces 0600 — not that the scratch
|
||||
// happened to arrive owner-only already.
|
||||
const { sandbox } = createRealExecSandbox({
|
||||
uploadOverride: async (uploads) => {
|
||||
for (const upload of uploads) {
|
||||
await fs.copyFile(upload.source, upload.destination);
|
||||
if (upload.destination.endsWith(".zst")) await fs.chmod(upload.destination, 0o644);
|
||||
}
|
||||
return true;
|
||||
},
|
||||
});
|
||||
const operations: PluginSyncOperation[] = [{
|
||||
operationId: "sync-op-1",
|
||||
files: [
|
||||
|
|
@ -629,6 +541,11 @@ describe("daytona file-sync inbound zstd transport compression", () => {
|
|||
|
||||
await performSyncIn({ sandbox: sandbox as never, operations, remoteDir, timeoutSeconds: 30 });
|
||||
|
||||
// A mapping with no `mode` lands owner-only (0600): the promote script
|
||||
// always runs `chmod` on the decompressed file right after
|
||||
// `zstd -d -o`, and uses 0600 when the mapping sets no `mode` — even
|
||||
// though its `.zst` scratch arrived at 0644 above. A mapping with an
|
||||
// explicit `mode` gets that mode instead.
|
||||
expect((await fs.stat(targetNoMode)).mode & 0o777).toBe(0o600);
|
||||
expect((await fs.stat(targetWithMode)).mode & 0o777).toBe(0o640);
|
||||
});
|
||||
|
|
@ -1012,7 +929,7 @@ describe("daytona file-sync inbound zstd transport compression", () => {
|
|||
|
||||
await performSyncIn({ sandbox: sandbox as never, operations, remoteDir: "/workspace", timeoutSeconds: 123 });
|
||||
|
||||
expect(seenTimeouts.length).toBeGreaterThanOrEqual(4); // mkdir+probe, checkSymlinkEscape, transfer, promote
|
||||
expect(seenTimeouts.length).toBeGreaterThanOrEqual(3); // mkdir and probe, transfer, promote
|
||||
expect(seenTimeouts.every((timeout) => timeout === 123)).toBe(true);
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -593,11 +593,10 @@ async function sweepZstdScratchAfterSuccess(
|
|||
interface FileMappingPlan {
|
||||
mapping: PluginSyncFileMapping;
|
||||
sourceSize: number;
|
||||
dir: string;
|
||||
/** Reserved scratch name for the FINAL bytes at `targetPath`, a direct child
|
||||
* of `remoteDir`. For a raw mapping the host uploads directly to this name.
|
||||
* For a compressed mapping the host never creates this name — the in-sandbox
|
||||
* decompression step does (Security Condition C1). */
|
||||
* decompression step does. */
|
||||
rawScratch: string;
|
||||
compressed: null | {
|
||||
/** Reserved `.zst` scratch name, a direct child of `remoteDir`. */
|
||||
|
|
@ -623,21 +622,14 @@ async function syncInFileMappings(input: {
|
|||
let bytesTransferred = 0;
|
||||
const plans: FileMappingPlan[] = [];
|
||||
for (const mapping of mappings) {
|
||||
assertConfinedSandboxPath(remoteDir, mapping.targetPath, "target");
|
||||
const dir = path.posix.dirname(mapping.targetPath);
|
||||
parentDirs.add(dir);
|
||||
parentDirs.add(path.posix.dirname(mapping.targetPath));
|
||||
const sourceSize = (await fs.stat(mapping.sourcePath)).size;
|
||||
bytesTransferred += sourceSize;
|
||||
// Stage each upload to a reserved temp that is a DIRECT child of the workspace
|
||||
// root (`remoteDir`), never a sibling of the target. The target's parent dir is
|
||||
// sandbox-writable and can be swapped for a symlink to `/etc` (or any host path)
|
||||
// after validation but before the write opens the destination — rooting the
|
||||
// privileged write directly under `remoteDir` removes that swappable intermediate
|
||||
// component, so the upload cannot be redirected outside the root by a parent
|
||||
// swap. `remoteDir` and the target dir share the workspace filesystem, so the
|
||||
// closing `mv -f` is still an atomic same-fs rename and an interrupted upload
|
||||
// never leaves a truncated file at targetPath.
|
||||
plans.push({ mapping, sourceSize, dir, rawScratch: path.posix.join(remoteDir, scratchName()), compressed: null });
|
||||
// root (`remoteDir`). `remoteDir` and the target dir share the workspace
|
||||
// filesystem, so the closing `mv -f` is still an atomic same-fs rename and an
|
||||
// interrupted upload never leaves a truncated file at targetPath.
|
||||
plans.push({ mapping, sourceSize, rawScratch: path.posix.join(remoteDir, scratchName()), compressed: null });
|
||||
}
|
||||
|
||||
// Count the serial guard round trips before the transfer, so the transfer span
|
||||
|
|
@ -668,25 +660,6 @@ async function syncInFileMappings(input: {
|
|||
// identical to a sandbox that has no `zstd` binary.
|
||||
const sandboxHasZstd = mkdirOutput.includes(ZSTD_PROBE_MARKER);
|
||||
|
||||
// Defense-in-depth beyond the lexical `assertConfinedSandboxPath`: a sandbox
|
||||
// can replace a target parent with a symlink to `/etc` so the string check
|
||||
// passes but the upload + `mv -f` resolve through it. Canonicalize every parent
|
||||
// dir (now materialized) and fail closed if any escapes, BEFORE any bytes land.
|
||||
// `checkSymlinkEscape` span: re-check a path resolves inside the workspace root
|
||||
// before use.
|
||||
await withProviderSpan({
|
||||
name: "checkSymlinkEscape",
|
||||
run: () =>
|
||||
assertSandboxPathsConfined({
|
||||
sandbox,
|
||||
remoteDir,
|
||||
paths: [...parentDirs],
|
||||
timeoutSeconds,
|
||||
label: "inbound symlink-escape guard",
|
||||
}),
|
||||
});
|
||||
guardRoundTrips += 1;
|
||||
|
||||
// Host-side compression, gated on the probe AND a runtime feature check (a
|
||||
// declared `engines.node` floor is an assumption, not a guarantee — always
|
||||
// feature-detect). Every candidate at or above `ZSTD_MIN_SOURCE_BYTES` is
|
||||
|
|
@ -737,7 +710,7 @@ async function syncInFileMappings(input: {
|
|||
for (const plan of plans) {
|
||||
if (plan.compressed) {
|
||||
// Upload ONLY the `.zst` file. The host never creates the raw scratch
|
||||
// name — the in-sandbox decompression step below does (C1).
|
||||
// name — the in-sandbox decompression step below does.
|
||||
uploads.push({ source: plan.compressed.hostTempPath, destination: plan.compressed.zstdScratch });
|
||||
} else {
|
||||
uploads.push({ source: plan.mapping.sourcePath, destination: plan.rawScratch });
|
||||
|
|
@ -766,8 +739,7 @@ async function syncInFileMappings(input: {
|
|||
// scratch (some targets promoted, others not) — sweep every reserved name on
|
||||
// any error so a retry never accumulates stale `.paperclip-upload-*` scratch.
|
||||
// The private host temp directory is removed in `finally` regardless of
|
||||
// outcome (C2's "no temp remains after success or failure" applies host-side
|
||||
// too).
|
||||
// outcome — no temp remains after success or failure.
|
||||
try {
|
||||
// One batched bulk upload (single /files/bulk-upload) for all file mappings.
|
||||
// `transfer` span: the real byte upload — `sandbox.fs.uploadFiles`.
|
||||
|
|
@ -784,99 +756,52 @@ async function syncInFileMappings(input: {
|
|||
// Apply the requested mode on the RAW mapping's temp file BEFORE the rename
|
||||
// so the target never appears at a widened window. A compressed mapping's
|
||||
// raw scratch does not exist yet at this point — its mode (if any) is
|
||||
// applied inside the promotion script below, on the safe raw scratch (C3).
|
||||
// applied inside the promotion script below, on the raw scratch.
|
||||
for (const apply of modeApplies) {
|
||||
await sandbox.fs.setFilePermissions(apply.temp, { mode: toOctalModeString(apply.mode) });
|
||||
}
|
||||
|
||||
// Promote every mapping onto its final target. The `mv -f` traverses the
|
||||
// target's PARENT dir, which is sandbox-writable and could be swapped for a
|
||||
// symlink after the earlier parent guard ran but before the rename opens it —
|
||||
// redirecting the promotion outside the root. Bind the confinement re-check and
|
||||
// the rename into ONE sandbox invocation: for each target, re-canonicalize its
|
||||
// parent dir, confirm the resolved parent is still inside the workspace root,
|
||||
// then OPEN that dir as fd 8 and `mv` into `/proc/self/fd/8/<base>`. Two races
|
||||
// are closed:
|
||||
// - check→open (ancestor swap): `mv "$_pc_tgt_dir"/<base>` would re-walk the
|
||||
// parent path string and follow an ancestor the sandbox repointed to a
|
||||
// symlink after the `case` check. Opening fd 8 PINS the directory inode, and
|
||||
// an immediate re-canonicalize of `/proc/self/fd/8` confirms the pinned inode
|
||||
// is still in-root before any write — an ancestor swap before the open is
|
||||
// caught by this verify (fail closed, exit 42); a swap after the open cannot
|
||||
// change which inode fd 8 references.
|
||||
// - open→rename: `mv` targets `/proc/self/fd/8/<base>`, which resolves through
|
||||
// the already-open inode rather than the path string, so the rename lands in
|
||||
// the verified directory even if the path is repointed mid-command.
|
||||
//
|
||||
// A compressed mapping runs a decompression block, in the SAME `sh -c`
|
||||
// script, immediately before its own fd-pinned promote block (C1–C3):
|
||||
// - `umask 077` + `set -C` (POSIX noclobber) scoped to a subshell, then
|
||||
// `exec 9> rawScratch` — an atomic exclusive create: `set -C` opens
|
||||
// with `O_CREAT|O_EXCL`, which fails on any EXISTING name, including a
|
||||
// symlink (dangling or not), because `O_EXCL` fails on the name's own
|
||||
// lstat and never opens through it. The tests in this package verify
|
||||
// this for the shells and the platform under test; it is not a claim
|
||||
// of a portable `O_NOFOLLOW` guarantee across every POSIX shell. This
|
||||
// is the retained descriptor C1 requires — never a separate
|
||||
// existence/symlink test.
|
||||
// - `zstd -d -c` writes into that retained descriptor (`>&9`), so the
|
||||
// decompressed bytes land in the pinned inode even if the pathname is
|
||||
// fought over mid-command.
|
||||
// - the mapping's `mode`, if set, is applied through `/proc/self/fd/9`
|
||||
// (the still-open descriptor) — never by reopening the pathname —
|
||||
// before the descriptor closes (C3). `umask 077` also means a mapping
|
||||
// with NO explicit mode still never appears wider than `0600` in the
|
||||
// window between create and the (absent) chmod.
|
||||
// - a subshell failure at any step exits non-zero without reaching the
|
||||
// `mv` (C2); the decompressed name is a reserved DIRECT child of
|
||||
// `remoteDir`, matching the guarantee the raw path already has for its
|
||||
// scratch name.
|
||||
// Only after the decompression block succeeds does the existing fd-pinned
|
||||
// promote block run — unchanged — for every mapping, compressed or raw.
|
||||
const renameScript = [...canonicalizerPreamble(shellQuote(remoteDir))];
|
||||
// Promote every mapping onto its final target with one `mv -f` per mapping,
|
||||
// atomic on the shared workspace filesystem. A compressed mapping first
|
||||
// decompresses its `.zst` scratch to the raw scratch name with `zstd -d -o`,
|
||||
// applies the mapping's mode (if set) with `chmod`, then removes the `.zst`
|
||||
// scratch after the `mv -f` promotes the raw scratch.
|
||||
const renameScript: string[] = [];
|
||||
for (const plan of plans) {
|
||||
const parentDir = plan.dir;
|
||||
const base = path.posix.basename(plan.mapping.targetPath);
|
||||
if (plan.compressed) {
|
||||
const modeCommand =
|
||||
typeof plan.mapping.mode === "number"
|
||||
? `chmod ${toOctalModeString(plan.mapping.mode)} /proc/self/fd/9 || { echo "chmod failed"; exit 50; };`
|
||||
: "";
|
||||
// `zstd -d -o` copies the mode of its INPUT (the uploaded `.zst`
|
||||
// scratch) onto its output with its own `chmod` call. That call runs
|
||||
// AFTER creation, so it overrides any `umask` in effect — a mapping
|
||||
// with no explicit `mode` must not rely on the scratch file's mode
|
||||
// being owner-only already. Always `chmod` the decompressed file
|
||||
// right after decompression: to the mapping's `mode` when set, or to
|
||||
// owner-only (0600) otherwise. The pre-refactor decompression step
|
||||
// applied the same 0600 default. The raw (uncompressed) path above
|
||||
// applies no `chmod` when the mapping sets no `mode`, so the two
|
||||
// inbound branches do not use the same no-mode default today.
|
||||
const targetMode = typeof plan.mapping.mode === "number" ? plan.mapping.mode : 0o600;
|
||||
renameScript.push(
|
||||
"(",
|
||||
"umask 077;",
|
||||
"set -C;",
|
||||
`exec 9> ${shellQuote(plan.rawScratch)} || { echo "raw scratch create failed"; exit 48; };`,
|
||||
`zstd -d -c -- ${shellQuote(plan.compressed.zstdScratch)} >&9 || { echo "decompress failed"; exit 49; };`,
|
||||
modeCommand,
|
||||
"exec 9>&-;",
|
||||
") || exit $?;",
|
||||
`zstd -d -o ${shellQuote(plan.rawScratch)} ${shellQuote(plan.compressed.zstdScratch)} || { echo "decompress failed"; exit 49; };`,
|
||||
`chmod ${toOctalModeString(targetMode)} ${shellQuote(plan.rawScratch)} || { echo "chmod failed"; exit 50; };`,
|
||||
);
|
||||
}
|
||||
renameScript.push(
|
||||
`_pc_tgt_dir=$(_pc_resolve ${shellQuote(parentDir)}) || { echo "ESCAPE"; exit 42; };`,
|
||||
`case "$_pc_tgt_dir/" in "$_pc_root"/*) : ;; *) echo "ESCAPE"; exit 42 ;; esac;`,
|
||||
`exec 8<"$_pc_tgt_dir" || { echo "open failed"; exit 47; };`,
|
||||
`_pc_fd_dir=$(_pc_resolve /proc/self/fd/8) || { echo "ESCAPE"; exit 42; };`,
|
||||
`case "$_pc_fd_dir/" in "$_pc_root"/*) : ;; *) echo "ESCAPE"; exit 42 ;; esac;`,
|
||||
`mv -f ${shellQuote(plan.rawScratch)} /proc/self/fd/8/${shellQuote(base)} || { echo "rename failed"; exit 43; };`,
|
||||
`exec 8>&-;`,
|
||||
`mv -f ${shellQuote(plan.rawScratch)} ${shellQuote(plan.mapping.targetPath)} || { echo "rename failed"; exit 43; };`,
|
||||
);
|
||||
if (plan.compressed) {
|
||||
// Clean up the `.zst` scratch after a successful promotion (C2/step 7).
|
||||
// `|| true` keeps a cleanup failure from becoming the promote script's
|
||||
// own exit status — every target file is already in place by this
|
||||
// point, so a stray `.zst` scratch must never read back as a sync
|
||||
// failure.
|
||||
// Clean up the `.zst` scratch after a successful promotion. `|| true`
|
||||
// keeps a cleanup failure from becoming the promote script's own exit
|
||||
// status — every target file is already in place by this point, so a
|
||||
// stray `.zst` scratch must never read back as a sync failure.
|
||||
renameScript.push(`rm -f ${shellQuote(plan.compressed.zstdScratch)} || true;`);
|
||||
}
|
||||
}
|
||||
// `promote` span: atomically move the staged temp onto its target via a
|
||||
// pinned dir handle. When this batch decompressed at least one mapping,
|
||||
// this span also carries `transfer.decompress.wall_ms`. That value
|
||||
// measures the WHOLE promote command — the canonicalizer preamble, every
|
||||
// decompression, and every `mv` — not decompression alone. Treat it as an
|
||||
// upper bound on the decompress wall time, not an exact measurement.
|
||||
// `promote` span: move the staged temp onto its target. When this batch
|
||||
// decompressed at least one mapping, this span also carries
|
||||
// `transfer.decompress.wall_ms`. That value measures the WHOLE promote
|
||||
// command — every decompression and every `mv` — not decompression alone.
|
||||
// Treat it as an upper bound on the decompress wall time, not an exact
|
||||
// measurement.
|
||||
await withProviderSpan({
|
||||
name: "promote",
|
||||
wallMsAttr: hasCompressedMapping ? SPAN_ATTR.transferDecompressWallMs : undefined,
|
||||
|
|
@ -919,7 +844,6 @@ async function syncInDirectoryMapping(input: {
|
|||
timeoutSeconds: number;
|
||||
}): Promise<{ filesTransferred: number; bytesTransferred: number }> {
|
||||
const { sandbox, mapping, remoteDir, timeoutSeconds } = input;
|
||||
assertConfinedSandboxPath(remoteDir, mapping.targetPath, "target");
|
||||
return withHostTempDir(async (tmp) => {
|
||||
const archivePath = path.join(tmp, "sync-in.tar");
|
||||
// The pack step is host-local: it builds the tarball and makes no sandbox
|
||||
|
|
@ -942,10 +866,8 @@ async function syncInDirectoryMapping(input: {
|
|||
// Count the serial guard round trips before the transfer, so the transfer
|
||||
// span records how much of the wall time is guard cost.
|
||||
let guardRoundTrips = 0;
|
||||
// Materialize the target dir first so the realpath guard resolves real
|
||||
// components, then confirm it (and any existing parent) canonicalizes inside
|
||||
// the remote dir — `tar -C` would otherwise follow a sandbox-planted symlink
|
||||
// and extract our archive outside the workspace root.
|
||||
// Materialize the target dir before the upload so the extract step below has
|
||||
// somewhere to write.
|
||||
// `ensureDirectory` span: `mkdir -p` — ensure a directory exists before a write.
|
||||
await withProviderSpan({
|
||||
name: "ensureDirectory",
|
||||
|
|
@ -958,20 +880,6 @@ async function syncInDirectoryMapping(input: {
|
|||
),
|
||||
});
|
||||
guardRoundTrips += 1;
|
||||
// `checkSymlinkEscape` span: re-check a path resolves inside the workspace
|
||||
// root before use.
|
||||
await withProviderSpan({
|
||||
name: "checkSymlinkEscape",
|
||||
run: () =>
|
||||
assertSandboxPathsConfined({
|
||||
sandbox,
|
||||
remoteDir,
|
||||
paths: [mapping.targetPath],
|
||||
timeoutSeconds,
|
||||
label: "inbound symlink-escape guard",
|
||||
}),
|
||||
});
|
||||
guardRoundTrips += 1;
|
||||
// The uploaded scratch tar lands at the workspace root as a reserved
|
||||
// `.paperclip-upload-*` entry. The extract script below removes it only on
|
||||
// success. On an upload or extract failure the scratch tar can remain, and the
|
||||
|
|
@ -991,30 +899,10 @@ async function syncInDirectoryMapping(input: {
|
|||
run: () =>
|
||||
sandbox.fs.uploadFiles([{ source: archivePath, destination: remoteTar }], timeoutSeconds),
|
||||
});
|
||||
// Bind validation and extraction into ONE sandbox invocation, then extract into
|
||||
// an OPEN directory inode rather than a path string. `exec 9<"$_pc_real"` itself
|
||||
// walks every ancestor of `$_pc_real` during the `open()` syscall, so a sandbox
|
||||
// process that swaps an ancestor component for a symlink AFTER `_pc_resolve`
|
||||
// returns but BEFORE the `open()` resolves would leave fd 9 pointing at a
|
||||
// directory outside the workspace — the earlier `case` check on the resolved
|
||||
// string cannot see that. Close the gap with open-then-verify: open fd 9 (which
|
||||
// PINS whatever inode `open()` landed on), then re-canonicalize `/proc/self/fd/9`
|
||||
// — the pinned inode's own path — and confirm it is still inside `$_pc_root`
|
||||
// before extracting. If an ancestor swap redirected the open, the pinned inode
|
||||
// resolves outside the root and the verify fails closed (exit 42); once the
|
||||
// verify passes, the inode is fixed and `tar -C /proc/self/fd/9` chdir's through
|
||||
// the magic symlink to that exact inode, so a post-open ancestor swap cannot
|
||||
// redirect the write. (The initial `case` on `$_pc_real` still fails fast on a
|
||||
// pre-open escape; the fd re-verify is what makes the guarantee race-free.)
|
||||
// Extract the uploaded tarball onto the already-created target directory,
|
||||
// then remove the scratch tarball.
|
||||
const extractScript = [
|
||||
...canonicalizerPreamble(shellQuote(remoteDir)),
|
||||
`_pc_real=$(_pc_resolve ${shellQuote(mapping.targetPath)}) || { echo "ESCAPE"; exit 42; };`,
|
||||
`case "$_pc_real/" in "$_pc_root"/*) : ;; *) echo "ESCAPE"; exit 42 ;; esac;`,
|
||||
`exec 9<"$_pc_real" || { echo "open failed"; exit 46; };`,
|
||||
`_pc_fd_real=$(_pc_resolve /proc/self/fd/9) || { echo "ESCAPE"; exit 42; };`,
|
||||
`case "$_pc_fd_real/" in "$_pc_root"/*) : ;; *) echo "ESCAPE"; exit 42 ;; esac;`,
|
||||
`tar -xf ${shellQuote(remoteTar)} -C /proc/self/fd/9 || { echo "extract failed"; exit 43; };`,
|
||||
`exec 9>&-;`,
|
||||
`tar -xf ${shellQuote(remoteTar)} -C ${shellQuote(mapping.targetPath)} || { echo "extract failed"; exit 43; };`,
|
||||
`rm -f ${shellQuote(remoteTar)};`,
|
||||
].join("\n");
|
||||
// `extractTarball` span: one round trip — re-check the path, `tar -xf`, and
|
||||
|
|
@ -1040,17 +928,13 @@ async function syncInDirectoryMapping(input: {
|
|||
|
||||
/**
|
||||
* Execute an operation's ordered `postUploadCommands` in-sandbox AFTER its files
|
||||
* have landed (Phase 3 / Security Conditions C1–C4). Commands run in array order,
|
||||
* fail-fast: the first non-zero exit or timeout throws and stops the rest — no
|
||||
* silent partial fallback (C4). Each `command` string is executed VERBATIM via the
|
||||
* exec seam; the provider never rewrites, concatenates, or appends a shell fragment
|
||||
* to it (C1/C3) — the working directory rides `executeCommand`'s structured `cwd`
|
||||
* argument, never a `cd &&` prefix on the command. Before exec, a present `cwd` is
|
||||
* re-validated under the workspace remote dir with the same lexical
|
||||
* ({@link assertConfinedSandboxPath}) + realpath/symlink ({@link assertSandboxPathsConfined})
|
||||
* guards used for file placement (C2): `..`, absolute-escape, and symlink-escape
|
||||
* are rejected fail-closed before any command runs. An absent `cwd` defaults to the
|
||||
* provider-resolved remote dir — never a process default cwd.
|
||||
* have landed. Commands run in array order, fail-fast: the first non-zero exit
|
||||
* or timeout throws and stops the rest. Each `command` string is executed
|
||||
* VERBATIM via the exec seam; the provider never rewrites, concatenates, or
|
||||
* appends a shell fragment to it — the working directory rides
|
||||
* `executeCommand`'s structured `cwd` argument, never a `cd &&` prefix on the
|
||||
* command. An absent `cwd` defaults to the provider-resolved remote dir — never
|
||||
* a process default cwd.
|
||||
*
|
||||
* Shared by the file- and directory-mapping paths: it runs once per operation,
|
||||
* after every mapping of that operation has been placed.
|
||||
|
|
@ -1063,29 +947,10 @@ async function runPostUploadCommands(input: {
|
|||
}): Promise<void> {
|
||||
const { sandbox, commands, remoteDir, timeoutSeconds } = input;
|
||||
for (const command of commands) {
|
||||
// C2: re-confine the command cwd before exec. Absent → the remote dir (never a
|
||||
// process default cwd); the remote dir is the confinement root itself, so only
|
||||
// an explicit cwd carries untrusted input worth re-validating.
|
||||
let cwd = remoteDir;
|
||||
if (command.cwd != null) {
|
||||
assertConfinedSandboxPath(remoteDir, command.cwd, "post-upload command cwd");
|
||||
// `checkSymlinkEscape` span: re-check a path resolves inside the workspace
|
||||
// root before use.
|
||||
await withProviderSpan({
|
||||
name: "checkSymlinkEscape",
|
||||
run: () =>
|
||||
assertSandboxPathsConfined({
|
||||
sandbox,
|
||||
remoteDir,
|
||||
paths: [command.cwd as string],
|
||||
timeoutSeconds,
|
||||
label: "post-upload command cwd symlink-escape guard",
|
||||
}),
|
||||
});
|
||||
cwd = command.cwd;
|
||||
}
|
||||
// C1/C3: run the command VERBATIM with a structured cwd (no string rewrite).
|
||||
// C4: first non-zero exit or timeout throws and aborts the remaining commands.
|
||||
// Absent cwd defaults to the remote dir, never a process default cwd.
|
||||
const cwd = command.cwd ?? remoteDir;
|
||||
// Run the command VERBATIM with a structured cwd (no string rewrite). The
|
||||
// first non-zero exit or timeout throws and aborts the remaining commands.
|
||||
const commandTimeoutSeconds =
|
||||
command.timeoutMs != null ? toTimeoutSeconds(command.timeoutMs) : timeoutSeconds;
|
||||
// `postUploadCommand` span: run one caller-supplied post-upload command.
|
||||
|
|
@ -1138,8 +1003,7 @@ export async function performSyncIn(input: {
|
|||
}
|
||||
|
||||
// Run the operation's ordered post-upload commands AFTER every file/directory
|
||||
// mapping of this operation has landed (Phase 3 / C1–C4). Absent/empty → no
|
||||
// extra exec, byte-identical to a pre-contract operation.
|
||||
// mapping of this operation has landed. Absent/empty → no extra exec.
|
||||
await runPostUploadCommands({
|
||||
sandbox: input.sandbox,
|
||||
commands: operation.postUploadCommands ?? [],
|
||||
|
|
|
|||
|
|
@ -3460,21 +3460,10 @@ describe("daytona native file-sync hooks", () => {
|
|||
const mvCall = sandbox.process.executeCommand.mock.calls.find(([cmd]) => String(cmd).includes("mv -f"));
|
||||
expect(mvCall).toBeDefined();
|
||||
const mvCommand = String(mvCall?.[0]);
|
||||
// TOCTOU-hardened rename: each promotion re-canonicalizes the target's parent
|
||||
// dir, confirms it is still confined, OPENS that dir as fd 8, re-verifies the
|
||||
// pinned inode is in-root, then `mv`s into `/proc/self/fd/8/<base>` — all in ONE
|
||||
// sh invocation, so neither an ancestor swap before the open nor a path swap
|
||||
// after it can redirect the rename. The rename is wrapped in `sh -c '...'`, so
|
||||
// inner single-quotes are shell-escaped; assert on the un-escaped components.
|
||||
expect(mvCommand).toContain("_pc_resolve");
|
||||
// The parent dir is opened as fd 8 and its pinned inode re-verified in-root
|
||||
// before the rename, which targets the inode via /proc/self/fd/8 rather than the
|
||||
// literal (swappable) path string.
|
||||
// Each promotion is one plain `mv -f <scratch> <target>` command, batched
|
||||
// together in a single sandbox invocation.
|
||||
expect(mvCommand).toContain(secretTemp);
|
||||
expect(mvCommand).toContain('exec 8<"$_pc_tgt_dir"');
|
||||
expect(mvCommand).toContain("_pc_fd_dir=$(_pc_resolve /proc/self/fd/8)");
|
||||
expect(mvCommand).toContain("/proc/self/fd/8/");
|
||||
expect(mvCommand).toContain("auth.json");
|
||||
expect(mvCommand).toContain(`${REMOTE_DIR}/.secret/auth.json`);
|
||||
// Both temps are promoted (one mv line per rename).
|
||||
expect(mvCommand.match(/mv -f /g)).toHaveLength(2);
|
||||
|
||||
|
|
@ -3546,8 +3535,8 @@ describe("daytona native file-sync hooks", () => {
|
|||
const transfer = spans.find((span) => span.name === "transfer");
|
||||
expect(transfer).toBeDefined();
|
||||
expect(transfer!.ended).toBe(true);
|
||||
// The two serial guard round trips before the transfer: mkdir + confinement.
|
||||
expect(transfer!.attributes["paperclip.sandbox.startup.transfer.guard.count"]).toBe(2);
|
||||
// One serial guard round trip before the transfer: mkdir (with the zstd probe).
|
||||
expect(transfer!.attributes["paperclip.sandbox.startup.transfer.guard.count"]).toBe(1);
|
||||
expect(transfer!.attributes["paperclip.sandbox.startup.provider"]).toBe("daytona");
|
||||
expect(typeof transfer!.attributes["paperclip.sandbox.startup.transfer.wall_ms"]).toBe("number");
|
||||
// A bulk file upload builds no host tarball, so it opens no pack span.
|
||||
|
|
@ -3651,10 +3640,11 @@ describe("daytona native file-sync hooks", () => {
|
|||
|
||||
const transfer = spans.find((span) => span.name === "transfer");
|
||||
expect(transfer).toBeDefined();
|
||||
expect(transfer!.attributes["paperclip.sandbox.startup.transfer.guard.count"]).toBe(2);
|
||||
// One serial guard round trip before the transfer: mkdir.
|
||||
expect(transfer!.attributes["paperclip.sandbox.startup.transfer.guard.count"]).toBe(1);
|
||||
});
|
||||
|
||||
it("opens ensureDirectory, checkSymlinkEscape, transfer, promote spans in call order for a file-mapping sync", async () => {
|
||||
it("opens ensureDirectory, transfer, promote spans in call order for a file-mapping sync", async () => {
|
||||
const hostDir = await makeHostDir();
|
||||
const source = path.join(hostDir, "config.txt");
|
||||
await fs.writeFile(source, "plain");
|
||||
|
|
@ -3683,7 +3673,6 @@ describe("daytona native file-sync hooks", () => {
|
|||
|
||||
expect(spans.map((span) => span.name)).toEqual([
|
||||
"ensureDirectory",
|
||||
"checkSymlinkEscape",
|
||||
"transfer",
|
||||
"promote",
|
||||
]);
|
||||
|
|
@ -3695,12 +3684,11 @@ describe("daytona native file-sync hooks", () => {
|
|||
if (span.name !== "transfer") {
|
||||
expect(span.attributes["paperclip.sandbox.startup.ensureDirectory.wall_ms"]).toBeUndefined();
|
||||
expect(span.attributes["paperclip.sandbox.startup.promote.wall_ms"]).toBeUndefined();
|
||||
expect(span.attributes["paperclip.sandbox.startup.checkSymlinkEscape.wall_ms"]).toBeUndefined();
|
||||
}
|
||||
}
|
||||
});
|
||||
|
||||
it("opens pack, ensureDirectory, checkSymlinkEscape, transfer, extractTarball spans in call order for a directory-mapping sync", async () => {
|
||||
it("opens pack, ensureDirectory, transfer, extractTarball spans in call order for a directory-mapping sync", async () => {
|
||||
const hostDir = await makeHostDir();
|
||||
const sourceDir = path.join(hostDir, "assets");
|
||||
await fs.mkdir(sourceDir, { recursive: true });
|
||||
|
|
@ -3733,7 +3721,6 @@ describe("daytona native file-sync hooks", () => {
|
|||
expect(spans.map((span) => span.name)).toEqual([
|
||||
"pack",
|
||||
"ensureDirectory",
|
||||
"checkSymlinkEscape",
|
||||
"transfer",
|
||||
"extractTarball",
|
||||
]);
|
||||
|
|
@ -3778,7 +3765,7 @@ describe("daytona native file-sync hooks", () => {
|
|||
expect(pack!.attributes["paperclip.sandbox.startup.provider"]).toBe("daytona");
|
||||
});
|
||||
|
||||
it("opens a checkSymlinkEscape span and a postUploadCommand span in call order for a post-upload command with a working directory", async () => {
|
||||
it("opens a postUploadCommand span for a post-upload command with a working directory", async () => {
|
||||
const hostDir = await makeHostDir();
|
||||
const source = path.join(hostDir, "config.txt");
|
||||
await fs.writeFile(source, "plain");
|
||||
|
|
@ -3806,15 +3793,12 @@ describe("daytona native file-sync hooks", () => {
|
|||
restore();
|
||||
}
|
||||
|
||||
// The full order: the file mapping opens ensureDirectory, checkSymlinkEscape,
|
||||
// transfer, promote; the post-upload command then opens its own cwd
|
||||
// checkSymlinkEscape and the postUploadCommand span.
|
||||
// The full order: the file mapping opens ensureDirectory, transfer, promote;
|
||||
// the post-upload command then opens the postUploadCommand span.
|
||||
expect(spans.map((span) => span.name)).toEqual([
|
||||
"ensureDirectory",
|
||||
"checkSymlinkEscape",
|
||||
"transfer",
|
||||
"promote",
|
||||
"checkSymlinkEscape",
|
||||
"postUploadCommand",
|
||||
]);
|
||||
const provision = spans.find((span) => span.name === "postUploadCommand");
|
||||
|
|
@ -3873,8 +3857,7 @@ describe("daytona native file-sync hooks", () => {
|
|||
expect(capturedTarListing).not.toContain("skip.log");
|
||||
expect(capturedTarListing).toMatch(/link\.txt ->|link\.txt link to/);
|
||||
|
||||
// The target dir is created by its own mkdir command (so the realpath guard
|
||||
// that follows resolves real components), no longer inside the extract chain.
|
||||
// The target dir is created by its own mkdir command, before the upload.
|
||||
const mkdirCall = sandbox.process.executeCommand.mock.calls.find(
|
||||
([cmd]) =>
|
||||
String(cmd).includes("mkdir -p") &&
|
||||
|
|
@ -3882,26 +3865,14 @@ describe("daytona native file-sync hooks", () => {
|
|||
!String(cmd).includes("tar -xf"),
|
||||
);
|
||||
expect(mkdirCall).toBeDefined();
|
||||
// The realpath symlink-escape guard runs on the target before extraction.
|
||||
const inboundGuardCall = sandbox.process.executeCommand.mock.calls.find(([cmd]) =>
|
||||
String(cmd).includes("_pc_resolve"),
|
||||
);
|
||||
expect(inboundGuardCall).toBeDefined();
|
||||
|
||||
const extractCall = sandbox.process.executeCommand.mock.calls.find(([cmd]) => String(cmd).includes("tar -xf"));
|
||||
expect(extractCall).toBeDefined();
|
||||
const extractCommand = String(extractCall?.[0]);
|
||||
// The extract binds validation and extraction into one sandbox invocation: it
|
||||
// re-canonicalizes the target, opens the resolved dir as fd 9, re-verifies the
|
||||
// PINNED inode (`/proc/self/fd/9`) is still in-root — closing the ancestor-swap
|
||||
// race in the `open()` itself — then extracts via /proc/self/fd/9, binding
|
||||
// extraction to the directory inode rather than the path string.
|
||||
expect(extractCommand).toContain("_pc_resolve");
|
||||
// The extract is one plain `tar -xf <scratch-tar> -C <target>` command,
|
||||
// followed by removing the scratch tar.
|
||||
expect(extractCommand).toContain(".paperclip-runtime/assets");
|
||||
expect(extractCommand).toContain("tar -xf");
|
||||
expect(extractCommand).toContain('exec 9<"$_pc_real"');
|
||||
expect(extractCommand).toContain("_pc_fd_real=$(_pc_resolve /proc/self/fd/9)");
|
||||
expect(extractCommand).toContain("-C /proc/self/fd/9");
|
||||
expect(extractCommand).toMatch(/rm -f .*\.paperclip-upload-.*\.tar/);
|
||||
});
|
||||
|
||||
|
|
@ -4081,32 +4052,6 @@ describe("daytona native file-sync hooks", () => {
|
|||
await expect(fs.stat(badTarget)).rejects.toThrow();
|
||||
});
|
||||
|
||||
it("rejects a sync target path that escapes the workspace remote dir (path confinement)", async () => {
|
||||
const hostDir = await makeHostDir();
|
||||
const source = path.join(hostDir, "evil.txt");
|
||||
await fs.writeFile(source, "x");
|
||||
const sandbox = createMockSandbox();
|
||||
mockGet.mockResolvedValue(sandbox);
|
||||
|
||||
await expect(
|
||||
plugin.definition.onEnvironmentSyncIn?.({
|
||||
driverKey: "daytona",
|
||||
companyId: "company-1",
|
||||
environmentId: "env-1",
|
||||
config: { timeoutMs: 300000, reuseLease: false },
|
||||
lease: syncLease(),
|
||||
operations: [
|
||||
{
|
||||
operationId: "sync-op-escape",
|
||||
files: [{ sourcePath: source, targetPath: `${REMOTE_DIR}/../../etc/passwd`, kind: "file" }],
|
||||
},
|
||||
],
|
||||
}),
|
||||
).rejects.toThrow(/escapes the workspace remote dir|not a confined absolute path/);
|
||||
|
||||
expect(sandbox.fs.uploadFiles).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("syncOut rejects an outbound source whose in-sandbox realpath escapes the workspace remote dir, before any download", async () => {
|
||||
const hostDir = await makeHostDir();
|
||||
const sandbox = createMockSandbox();
|
||||
|
|
@ -4177,81 +4122,6 @@ describe("daytona native file-sync hooks", () => {
|
|||
await expect(fs.stat(target)).rejects.toThrow();
|
||||
});
|
||||
|
||||
it("syncIn rejects a file mapping whose in-sandbox target parent resolves outside the remote dir (symlinked-parent escape), before uploading", async () => {
|
||||
const hostDir = await makeHostDir();
|
||||
const source = path.join(hostDir, "auth.json");
|
||||
await fs.writeFile(source, "credential-material");
|
||||
|
||||
const sandbox = createMockSandbox();
|
||||
// The lexical path check passes (the target string is confined), but the
|
||||
// realpath guard on the materialized parent dir resolves outside the root:
|
||||
// report the escape exit (42) for the `_pc_resolve` probe, green otherwise.
|
||||
sandbox.process.executeCommand.mockImplementation(async (command: string) => {
|
||||
if (command.includes("_pc_resolve")) {
|
||||
return { exitCode: 42, result: `ESCAPE:${REMOTE_DIR}/.secret`, artifacts: { stdout: "" } };
|
||||
}
|
||||
return { exitCode: 0, result: "bash", artifacts: { stdout: "bash" } };
|
||||
});
|
||||
mockGet.mockResolvedValue(sandbox);
|
||||
|
||||
await expect(
|
||||
plugin.definition.onEnvironmentSyncIn?.({
|
||||
driverKey: "daytona",
|
||||
companyId: "company-1",
|
||||
environmentId: "env-1",
|
||||
config: { timeoutMs: 300000, reuseLease: false },
|
||||
lease: syncLease(),
|
||||
operations: [
|
||||
{
|
||||
operationId: "sync-op-in-escape",
|
||||
files: [{ sourcePath: source, targetPath: `${REMOTE_DIR}/.secret/auth.json`, kind: "file", mode: 0o600 }],
|
||||
},
|
||||
],
|
||||
}),
|
||||
).rejects.toThrow(/inbound symlink-escape guard command failed \(exit 42\)/);
|
||||
|
||||
// Fail-closed: the guard trips after mkdir but before any bytes are uploaded.
|
||||
expect(sandbox.fs.uploadFiles).not.toHaveBeenCalled();
|
||||
expect(sandbox.fs.setFilePermissions).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("syncIn rejects a directory mapping whose in-sandbox target resolves outside the remote dir (symlinked-dir extraction), before uploading the tarball", async () => {
|
||||
const hostDir = await makeHostDir();
|
||||
const sourceDir = path.join(hostDir, "assets");
|
||||
await fs.mkdir(sourceDir, { recursive: true });
|
||||
await fs.writeFile(path.join(sourceDir, "a.txt"), "alpha");
|
||||
|
||||
const sandbox = createMockSandbox();
|
||||
sandbox.process.executeCommand.mockImplementation(async (command: string) => {
|
||||
if (command.includes("_pc_resolve")) {
|
||||
return { exitCode: 42, result: `ESCAPE:${REMOTE_DIR}/assets`, artifacts: { stdout: "" } };
|
||||
}
|
||||
return { exitCode: 0, result: "bash", artifacts: { stdout: "bash" } };
|
||||
});
|
||||
mockGet.mockResolvedValue(sandbox);
|
||||
|
||||
await expect(
|
||||
plugin.definition.onEnvironmentSyncIn?.({
|
||||
driverKey: "daytona",
|
||||
companyId: "company-1",
|
||||
environmentId: "env-1",
|
||||
config: { timeoutMs: 300000, reuseLease: false },
|
||||
lease: syncLease(),
|
||||
operations: [
|
||||
{
|
||||
operationId: "sync-op-in-dir-escape",
|
||||
files: [{ sourcePath: sourceDir, targetPath: `${REMOTE_DIR}/assets`, kind: "directory" }],
|
||||
},
|
||||
],
|
||||
}),
|
||||
).rejects.toThrow(/inbound symlink-escape guard command failed \(exit 42\)/);
|
||||
|
||||
// Fail-closed: no tarball is uploaded and no in-sandbox extraction runs.
|
||||
expect(sandbox.fs.uploadFiles).not.toHaveBeenCalled();
|
||||
const extractCall = sandbox.process.executeCommand.mock.calls.find(([cmd]) => String(cmd).includes("tar -xf"));
|
||||
expect(extractCall).toBeUndefined();
|
||||
});
|
||||
|
||||
it("syncIn sweeps staged temps when the batched rename fails mid-promotion", async () => {
|
||||
const hostDir = await makeHostDir();
|
||||
const source = path.join(hostDir, "config.txt");
|
||||
|
|
@ -4649,42 +4519,6 @@ describe("daytona native file-sync hooks", () => {
|
|||
});
|
||||
});
|
||||
|
||||
it("rejects a merged operation when either tar mapping target escapes the remote dir, before uploading", async () => {
|
||||
const hostDir = await makeHostDir();
|
||||
const gitTar = path.join(hostDir, "git-workspace.tar");
|
||||
const overlayTar = path.join(hostDir, "workspace.tar");
|
||||
await fs.writeFile(gitTar, "git-bytes");
|
||||
await fs.writeFile(overlayTar, "overlay-bytes");
|
||||
const runtimeDir = `${REMOTE_DIR}/.paperclip-runtime/adapter`;
|
||||
|
||||
const sandbox = createMockSandbox();
|
||||
mockGet.mockResolvedValue(sandbox);
|
||||
|
||||
await expect(
|
||||
plugin.definition.onEnvironmentSyncIn?.({
|
||||
driverKey: "daytona",
|
||||
companyId: "company-1",
|
||||
environmentId: "env-1",
|
||||
config: { timeoutMs: 300000, reuseLease: false },
|
||||
lease: syncLease(),
|
||||
operations: [
|
||||
{
|
||||
operationId: "merged-escape",
|
||||
files: [
|
||||
{ sourcePath: gitTar, targetPath: `${runtimeDir}/git-workspace-upload.tar`, kind: "file" },
|
||||
// The overlay mapping target escapes the workspace remote dir.
|
||||
{ sourcePath: overlayTar, targetPath: `${REMOTE_DIR}/../../etc/workspace-upload.tar`, kind: "file" },
|
||||
],
|
||||
postUploadCommands: [{ command: "git-history-extract" }, { command: "workspace-overlay-extract" }],
|
||||
},
|
||||
],
|
||||
}),
|
||||
).rejects.toThrow(/escapes the workspace remote dir|not a confined absolute path/);
|
||||
|
||||
// Neither tar uploaded: the confine check on the escaping mapping trips first.
|
||||
expect(sandbox.fs.uploadFiles).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("stops the overlay and remove-deleted commands when the git extract fails (merged operation fail-fast)", async () => {
|
||||
const hostDir = await makeHostDir();
|
||||
const gitTar = path.join(hostDir, "git-workspace.tar");
|
||||
|
|
@ -4736,75 +4570,6 @@ describe("daytona native file-sync hooks", () => {
|
|||
expect(ran("remove-deleted-paths")).toBe(false);
|
||||
});
|
||||
|
||||
it("rejects a post-upload command cwd that escapes the remote dir lexically, before any exec (C2)", async () => {
|
||||
const hostDir = await makeHostDir();
|
||||
const source = path.join(hostDir, "config.txt");
|
||||
await fs.writeFile(source, "plain");
|
||||
|
||||
for (const badCwd of [`${REMOTE_DIR}/../etc`, "/etc"]) {
|
||||
const sandbox = createMockSandbox();
|
||||
mockGet.mockResolvedValue(sandbox);
|
||||
await expect(
|
||||
plugin.definition.onEnvironmentSyncIn?.({
|
||||
driverKey: "daytona",
|
||||
companyId: "company-1",
|
||||
environmentId: "env-1",
|
||||
config: { timeoutMs: 300000, reuseLease: false },
|
||||
lease: syncLease(),
|
||||
operations: [
|
||||
{
|
||||
operationId: "op-escape",
|
||||
files: [{ sourcePath: source, targetPath: `${REMOTE_DIR}/config.txt`, kind: "file" }],
|
||||
postUploadCommands: [{ command: "run-me", cwd: badCwd }],
|
||||
},
|
||||
],
|
||||
}),
|
||||
).rejects.toThrow(/not a confined absolute path|escapes the workspace remote dir/);
|
||||
// The command never ran — lexical confinement rejected it before exec.
|
||||
expect(sandbox.process.executeCommand.mock.calls.some(([c]) => c === "run-me")).toBe(
|
||||
false,
|
||||
);
|
||||
}
|
||||
});
|
||||
|
||||
it("rejects a post-upload command whose cwd resolves outside the root via a symlink (realpath guard, C2)", async () => {
|
||||
const hostDir = await makeHostDir();
|
||||
const source = path.join(hostDir, "config.txt");
|
||||
await fs.writeFile(source, "plain");
|
||||
|
||||
const cwd = `${REMOTE_DIR}/link`;
|
||||
const sandbox = createMockSandbox();
|
||||
// The in-sandbox realpath symlink-escape guard for THIS cwd fails closed (exit
|
||||
// 42), simulating a sandbox-planted symlink that resolves out of root.
|
||||
sandbox.process.executeCommand.mockImplementation(async (command: string) => {
|
||||
if (command.includes("_pc_resolve") && command.includes(cwd)) {
|
||||
return { exitCode: 42, result: "ESCAPE", artifacts: { stdout: "ESCAPE" } };
|
||||
}
|
||||
return { exitCode: 0, result: "", artifacts: { stdout: "" } };
|
||||
});
|
||||
mockGet.mockResolvedValue(sandbox);
|
||||
|
||||
await expect(
|
||||
plugin.definition.onEnvironmentSyncIn?.({
|
||||
driverKey: "daytona",
|
||||
companyId: "company-1",
|
||||
environmentId: "env-1",
|
||||
config: { timeoutMs: 300000, reuseLease: false },
|
||||
lease: syncLease(),
|
||||
operations: [
|
||||
{
|
||||
operationId: "op-symlink",
|
||||
files: [{ sourcePath: source, targetPath: `${REMOTE_DIR}/config.txt`, kind: "file" }],
|
||||
postUploadCommands: [{ command: "run-me", cwd }],
|
||||
},
|
||||
],
|
||||
}),
|
||||
).rejects.toThrow(/symlink-escape guard|command failed/i);
|
||||
expect(sandbox.process.executeCommand.mock.calls.some(([c]) => c === "run-me")).toBe(
|
||||
false,
|
||||
);
|
||||
});
|
||||
|
||||
it("issues no extra exec when an operation has no post-upload commands (backward-compat)", async () => {
|
||||
const hostDir = await makeHostDir();
|
||||
const source = path.join(hostDir, "config.txt");
|
||||
|
|
|
|||
Loading…
Reference in New Issue