mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-10-12 08:19:54 +02:00
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - AI agents run in sandboxed execution environments (Kubernetes pods, Daytona workspaces, etc.) and need to sync files between the host and those environments — for workspace setup, asset delivery, and output retrieval > - The existing sync path for Kubernetes uses a base64-over-exec chunk loop: each ~4 MB chunk requires its own `execInPod` round-trip, so large syncs balloon into many exec calls with corresponding overhead > - `execInPod` supports piped stdin/stdout, meaning the full transfer can be done as a single exec that streams a raw `tar` archive over the data channel — one round-trip regardless of file size, with nothing base64-encoded and nothing buffered whole in memory on either side > - PR-1 (#10013, merged) added the `onEnvironmentSyncIn`/`onEnvironmentSyncOut` opt-in hook API to the sandbox provider interface and documented the protocol; PR-2 (#10028, merged) implemented these hooks for the Daytona provider > - This pull request implements the same two lifecycle hooks in the Kubernetes sandbox provider, so workspace/asset file sync streams through one `execInPod` per operation instead of the chunk loop > - The benefit is significantly fewer exec round-trips for large syncs and flat memory use on both host and pod, with security properties preserved: atomic replace, secret-mode enforcement, path confinement, TOCTOU-safe snapshot, and member-confinement on host-assembled archives from sandbox-authored tar output ## Linked Issues or Issue Description This is the third and final PR in a sequential series: - Refs #10013 — PR-1: opt-in sync hook API + provider docs (merged) - Refs #10028 — PR-2: native file-sync lifecycle hooks for Daytona provider (merged) **Feature:** Native single-exec file-sync lifecycle hooks for the Kubernetes sandbox provider. *Motivation:* The existing Kubernetes sync path encodes files as base64 and loops over `execInPod` one chunk at a time (~4 MB per exec). For large workspaces or asset sets this is slow and resource-intensive. The Kubernetes `execInPod` API supports piped stdin/stdout, enabling a raw-`tar` streaming transfer that needs only one exec regardless of file count or size and never buffers the whole payload in memory. *Proposed solution:* Implement `onEnvironmentSyncIn` and `onEnvironmentSyncOut` in the Kubernetes provider using a streaming `execInPod` with a tar pipeline — for syncIn the host builds the archive on disk and streams its raw bytes into the pod's stdin (`head -c <exact-size> | tar -x`, no base64); for syncOut in-pod `tar` writes to the exec's stdout and the host streams those bytes straight to a file. Path confinement, atomic replace, secret-mode enforcement, TOCTOU protection, and a streamed-bytes fail-closed guard are all enforced. ## What Changed - **New `src/file-sync.ts`** in `packages/plugins/sandbox-providers/kubernetes/` — `performSyncIn` and `performSyncOut` over an injected pod-exec closure, keeping transfer logic hermetically unit-testable - **New `execInPodStreaming` in `src/pod-exec.ts`** — a streaming exec primitive that binds a caller-supplied stdin readable and a stdout writable to the exec WebSocket data channel, added alongside the existing `execInPod` (which is unchanged). This lets a transfer stream raw bytes to/from disk instead of buffering the payload as a single string - **Updated `src/plugin.ts`** — registers `onEnvironmentSyncIn`/`onEnvironmentSyncOut`; resolves the `sandbox-cr` pod exactly like `onEnvironmentExecute` and delegates; `job` backend rejects file-sync calls explicitly (out of scope) - **syncIn path:** host builds the tarball to a temp file → streams its raw bytes over exec stdin, bounded in-pod by `head -c <exact-archive-size> | tar -x` (no base64 anywhere) → extract into a `/proc/self/fd`-pinned reserved `0700` staging dir → `chmod`-before-`mv -f` atomic replace per file (directory mappings use `followSymlinks`→`-h`) - **syncOut path:** in-pod validate + realpath-snapshot each source (closes the validation→copy TOCTOU window) → single-exec `tar -c` streamed over exec stdout → host streams that stdout straight to a temp file through a byte-counting transform → member-confined extraction of the sandbox-authored archive - **Security properties:** secret files land at requested mode with no widened window; every interpolated path is shell-quoted and confined lexically plus via in-pod `realpath`; the outbound stream is bounded by a **streamed-bytes disk guard** (`MAX_SYNC_OUTPUT_BYTES`, 8 GiB default, per-call overridable) that fails the transfer closed — writing no target file — if an untrusted pod emits more bytes than allowed. Neither host nor pod buffers the whole payload, so there is no in-memory size cap on the transfer - **No changes** to `execInPod`, `wrapCommandWithEnv`, or `FastUploadInterceptor` (the `environmentExecute` path is untouched) - **No dependency or lockfile changes** - **New tests** in `test/unit/file-sync.test.ts` (atomic-replace, `0600` secret mode, symlink preserve/deref, dir-mapping, exclude, path-confinement rejection, streamed-output guard fail-closed) and `test/unit/pod-exec.test.ts` (streaming stdin/stdout, caller-sink error fail-closed), plus extended `test/unit/plugin.test.ts` ## Follow-up: Legacy Job-Lease Base64 Fallback Fix Addresses the Greptile 4/5 blocking finding ("Handle existing job leases", `server/src/services/environment-runtime.ts`). Job leases provisioned before the `nativeFileSyncUnsupported` metadata flag existed carry `backend: "job"` but no flag, so `supportsSync()` treated them as native-capable and routed their sync to the pod-exec hook — which the job backend rejects (it has no exec channel) instead of using the byte-identical base64 fallback. The fix adds a belt-and-suspenders gate on the persisted `backend === "job"` field alongside the existing `nativeFileSyncUnsupported` flag check, so pre-existing job leases continue syncing via the base64 fallback after deployment. No behaviour change for `sandbox-cr` leases. ## Verification - `pnpm --filter @paperclipai/sandbox-provider-kubernetes test` — 19 files / 182 tests green, including the existing `upload-interceptor` and `pod-exec` suites - `tsc --noEmit` in the kubernetes package — 0 errors - The sync hooks are opt-in; existing `environmentExecute` behaviour is unaffected and tested by the unchanged existing suites ## Risks - **Opt-in only:** `onEnvironmentSyncIn`/`onEnvironmentSyncOut` are registered conditionally; providers that do not register them fall back to the existing chunk loop. No regression risk on the existing path. - **Shell-injection surface:** all path interpolation uses shell-quoting; paths are additionally confined lexically and via in-pod `realpath` before use. - **TOCTOU on syncOut:** the in-pod snapshot validates and records file metadata before the tar call, closing the window between validation and copy. - **Archive member confinement:** host-side reassembly rejects any tar member whose resolved path escapes the target directory, preventing a malicious in-pod tar from writing outside the intended destination. - **Untrusted-output volume:** an over-large outbound stream trips the streamed-bytes disk guard and fails closed (no target written and the temp sink is swept) rather than filling host disk or memory; the guard bounds disk unconditionally and bounds memory insofar as WebSocket write-backpressure holds. ## Model Used Anthropic Claude Sonnet 4.6 (`claude-sonnet-4-6`) — produced by a Claude-based AI agent using agentic tool use and multi-step code generation. 200K context window, extended reasoning, code execution and verification capabilities. ## 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: Harold Kim <harold@paperclip.ing> Co-authored-by: Paperclip <noreply@paperclip.ing>
239 lines
7.2 KiB
TypeScript
239 lines
7.2 KiB
TypeScript
import { describe, it, expect, vi } from "vitest";
|
|
import { Readable, Writable } from "node:stream";
|
|
import type { PassThrough } from "node:stream";
|
|
|
|
// Mock the k8s client so `execInPod` runs against a scripted WebSocket exec: the
|
|
// fake `Exec.exec` streams stdout chunks into the provided PassThrough and, for
|
|
// the success path, reports an exit status. This exercises the host-side stdout
|
|
// accumulation cap without a real cluster.
|
|
type StatusCb = (status: {
|
|
status: string;
|
|
details?: { causes?: { reason?: string; message?: string }[] };
|
|
}) => void;
|
|
|
|
let scriptedExec: (
|
|
stdout: PassThrough,
|
|
stderr: PassThrough,
|
|
statusCb: StatusCb,
|
|
stdin: PassThrough | null,
|
|
) => void = () => undefined;
|
|
|
|
vi.mock("@kubernetes/client-node", () => {
|
|
class Exec {
|
|
constructor(_kc: unknown) {}
|
|
async exec(
|
|
_namespace: string,
|
|
_podName: string,
|
|
_containerName: string,
|
|
_command: string[],
|
|
stdout: PassThrough,
|
|
stderr: PassThrough,
|
|
stdin: PassThrough | null,
|
|
_tty: boolean,
|
|
statusCb: StatusCb,
|
|
) {
|
|
// Defer so the caller has wired its stream listeners first.
|
|
setImmediate(() => scriptedExec(stdout, stderr, statusCb, stdin));
|
|
return { close() {} };
|
|
}
|
|
}
|
|
return { Exec };
|
|
});
|
|
|
|
const { execInPod, execInPodStreaming } = await import("../../src/pod-exec.js");
|
|
|
|
const KC = {} as never;
|
|
|
|
describe("execInPod stdout cap", () => {
|
|
it("fails closed when pod stdout exceeds the cap", async () => {
|
|
scriptedExec = (stdout) => {
|
|
// Emit more than the cap in a single chunk; the host must reject.
|
|
stdout.write(Buffer.alloc(64, 0x41));
|
|
};
|
|
await expect(
|
|
execInPod(KC, "ns", "pod", "agent", ["/bin/sh", "-c", ":"], undefined, 5_000, 16),
|
|
).rejects.toThrow(/cap|exceeded/i);
|
|
});
|
|
|
|
it("accepts stdout within the cap and returns the accumulated output", async () => {
|
|
scriptedExec = (stdout, stderr, statusCb) => {
|
|
stdout.write(Buffer.from("hello", "utf-8"));
|
|
stdout.end();
|
|
stderr.end();
|
|
statusCb({ status: "Success" });
|
|
};
|
|
const result = await execInPod(
|
|
KC,
|
|
"ns",
|
|
"pod",
|
|
"agent",
|
|
["/bin/sh", "-c", ":"],
|
|
undefined,
|
|
5_000,
|
|
1024,
|
|
);
|
|
expect(result).toEqual({ exitCode: 0, stdout: "hello", stderr: "" });
|
|
});
|
|
|
|
it("leaves stdout unbounded when no cap is provided", async () => {
|
|
scriptedExec = (stdout, stderr, statusCb) => {
|
|
stdout.write(Buffer.alloc(4096, 0x42));
|
|
stdout.end();
|
|
stderr.end();
|
|
statusCb({ status: "Success" });
|
|
};
|
|
const result = await execInPod(
|
|
KC,
|
|
"ns",
|
|
"pod",
|
|
"agent",
|
|
["/bin/sh", "-c", ":"],
|
|
undefined,
|
|
5_000,
|
|
);
|
|
expect(result.exitCode).toBe(0);
|
|
expect(result.stdout).toHaveLength(4096);
|
|
});
|
|
});
|
|
|
|
describe("execInPod stderr cap", () => {
|
|
it("fails closed when pod stderr exceeds the cap", async () => {
|
|
// stderr is equally pod-controlled: a malicious pod that floods stderr must
|
|
// not grow the host accumulator without bound. Same DoS class as stdout.
|
|
scriptedExec = (_stdout, stderr) => {
|
|
stderr.write(Buffer.alloc(64, 0x45));
|
|
};
|
|
await expect(
|
|
// maxStdoutBytes generous, maxStderrBytes = 16 -> stderr must trip.
|
|
execInPod(KC, "ns", "pod", "agent", ["/bin/sh", "-c", ":"], undefined, 5_000, 1024, 16),
|
|
).rejects.toThrow(/stderr.*cap|cap.*stderr|exceeded/i);
|
|
});
|
|
|
|
it("accepts stderr within the cap and returns the accumulated output", async () => {
|
|
scriptedExec = (stdout, stderr, statusCb) => {
|
|
stderr.write(Buffer.from("warn", "utf-8"));
|
|
stdout.end();
|
|
stderr.end();
|
|
statusCb({ status: "Success" });
|
|
};
|
|
const result = await execInPod(
|
|
KC,
|
|
"ns",
|
|
"pod",
|
|
"agent",
|
|
["/bin/sh", "-c", ":"],
|
|
undefined,
|
|
5_000,
|
|
1024,
|
|
1024,
|
|
);
|
|
expect(result).toEqual({ exitCode: 0, stdout: "", stderr: "warn" });
|
|
});
|
|
|
|
it("leaves stderr unbounded when no cap is provided", async () => {
|
|
scriptedExec = (stdout, stderr, statusCb) => {
|
|
stderr.write(Buffer.alloc(4096, 0x46));
|
|
stdout.end();
|
|
stderr.end();
|
|
statusCb({ status: "Success" });
|
|
};
|
|
const result = await execInPod(
|
|
KC,
|
|
"ns",
|
|
"pod",
|
|
"agent",
|
|
["/bin/sh", "-c", ":"],
|
|
undefined,
|
|
5_000,
|
|
1024,
|
|
);
|
|
expect(result.exitCode).toBe(0);
|
|
expect(result.stderr).toHaveLength(4096);
|
|
});
|
|
});
|
|
|
|
describe("execInPodStreaming", () => {
|
|
it("streams the command's stdout into the caller sink and resolves with exit code + stderr", async () => {
|
|
scriptedExec = (stdout, stderr, statusCb) => {
|
|
stdout.write(Buffer.from("chunk-one;"));
|
|
stdout.write(Buffer.from("chunk-two"));
|
|
stdout.end();
|
|
stderr.write(Buffer.from("warn"));
|
|
stderr.end();
|
|
statusCb({ status: "Success" });
|
|
};
|
|
const chunks: Buffer[] = [];
|
|
const sink = new Writable({
|
|
write(chunk: Buffer, _enc, cb) {
|
|
chunks.push(Buffer.from(chunk));
|
|
cb();
|
|
},
|
|
});
|
|
const result = await execInPodStreaming(KC, "ns", "pod", "agent", ["/bin/sh", "-c", ":"], {
|
|
stdout: sink,
|
|
timeoutMs: 5_000,
|
|
});
|
|
expect(result).toEqual({ exitCode: 0, stderr: "warn" });
|
|
expect(Buffer.concat(chunks).toString("utf-8")).toBe("chunk-one;chunk-two");
|
|
});
|
|
|
|
it("streams the caller stdin Readable into the pod command", async () => {
|
|
let received = Buffer.alloc(0);
|
|
scriptedExec = (stdout, stderr, statusCb, stdin) => {
|
|
if (stdin) {
|
|
stdin.on("data", (chunk: Buffer) => {
|
|
received = Buffer.concat([received, chunk]);
|
|
});
|
|
}
|
|
// Resolve once the source has been fully piped through.
|
|
setImmediate(() => {
|
|
stdout.end();
|
|
stderr.end();
|
|
statusCb({ status: "Success" });
|
|
});
|
|
};
|
|
const source = Readable.from([Buffer.from("payload-bytes")]);
|
|
const result = await execInPodStreaming(KC, "ns", "pod", "agent", ["/bin/sh", "-c", ":"], {
|
|
stdin: source,
|
|
timeoutMs: 5_000,
|
|
});
|
|
expect(result.exitCode).toBe(0);
|
|
expect(received.toString("utf-8")).toBe("payload-bytes");
|
|
});
|
|
|
|
it("fails closed when the pod floods stderr past the cap", async () => {
|
|
scriptedExec = (_stdout, stderr) => {
|
|
stderr.write(Buffer.alloc(64, 0x45));
|
|
};
|
|
const sink = new Writable({ write(_c, _e, cb) { cb(); } });
|
|
await expect(
|
|
execInPodStreaming(KC, "ns", "pod", "agent", ["/bin/sh", "-c", ":"], {
|
|
stdout: sink,
|
|
timeoutMs: 5_000,
|
|
maxStderrBytes: 16,
|
|
}),
|
|
).rejects.toThrow(/stderr.*cap|exceeded/i);
|
|
});
|
|
|
|
it("fails closed when the caller sink errors (e.g. a streamed-bytes disk guard trips)", async () => {
|
|
scriptedExec = (stdout, stderr, statusCb) => {
|
|
stdout.write(Buffer.alloc(4096, 0x42));
|
|
stdout.end();
|
|
stderr.end();
|
|
statusCb({ status: "Success" });
|
|
};
|
|
// A sink that rejects any write, standing in for the file-sync disk guard.
|
|
const sink = new Writable({
|
|
write(_chunk, _enc, cb) {
|
|
cb(new Error("streamed-output disk guard tripped"));
|
|
},
|
|
});
|
|
await expect(
|
|
execInPodStreaming(KC, "ns", "pod", "agent", ["/bin/sh", "-c", ":"], {
|
|
stdout: sink,
|
|
timeoutMs: 5_000,
|
|
}),
|
|
).rejects.toThrow(/disk guard/i);
|
|
});
|
|
});
|