mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-10-10 20:50:08 +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>
266 lines
8.3 KiB
TypeScript
266 lines
8.3 KiB
TypeScript
import { describe, it, expect, vi, beforeEach } from "vitest";
|
|
|
|
// Mock the kube-client module so the plugin handlers run against injected
|
|
// fake API clients instead of a real cluster. h.clients is swapped per test.
|
|
const h = vi.hoisted(() => ({ clients: {} as Record<string, unknown> }));
|
|
|
|
vi.mock("../../src/kube-client.js", () => ({
|
|
createKubeConfig: vi.fn(() => ({})),
|
|
makeKubeClients: vi.fn(() => h.clients),
|
|
}));
|
|
|
|
import plugin from "../../src/plugin.js";
|
|
|
|
const CONFIG = { inCluster: true, backend: "sandbox-cr" };
|
|
|
|
function leaseMetadata(overrides: Record<string, unknown> = {}): Record<string, unknown> {
|
|
return {
|
|
namespace: "paperclip-acme",
|
|
jobName: "pc-abc",
|
|
podName: "pc-abc-pod",
|
|
secretName: "pc-abc-env",
|
|
phase: "Pending",
|
|
backend: "sandbox-cr",
|
|
...overrides,
|
|
};
|
|
}
|
|
|
|
function notFound(): Error {
|
|
return Object.assign(new Error("not found"), { code: 404 });
|
|
}
|
|
|
|
function readySandboxCr(podName: string): Record<string, unknown> {
|
|
return {
|
|
metadata: { uid: "uid-1" },
|
|
status: {
|
|
conditions: [{ type: "Ready", status: "True" }],
|
|
podName,
|
|
},
|
|
};
|
|
}
|
|
|
|
beforeEach(() => {
|
|
h.clients = {};
|
|
});
|
|
|
|
describe("onEnvironmentResumeLease", () => {
|
|
it("is implemented (Daytona feature parity)", () => {
|
|
expect(plugin.definition.onEnvironmentResumeLease).toBeTypeOf("function");
|
|
expect(plugin.definition.onEnvironmentDestroyLease).toBeTypeOf("function");
|
|
});
|
|
|
|
it("returns a valid lease handle for a live sandbox-cr lease", async () => {
|
|
h.clients = {
|
|
custom: {
|
|
getNamespacedCustomObject: vi.fn().mockResolvedValue(readySandboxCr("pc-abc-pod")),
|
|
},
|
|
core: {
|
|
readNamespacedPod: vi.fn().mockResolvedValue({
|
|
metadata: {},
|
|
status: { phase: "Running" },
|
|
}),
|
|
},
|
|
};
|
|
|
|
const lease = await plugin.definition.onEnvironmentResumeLease!({
|
|
driverKey: "kubernetes",
|
|
companyId: "acme",
|
|
environmentId: "env-1",
|
|
config: CONFIG,
|
|
providerLeaseId: "pc-abc",
|
|
leaseMetadata: leaseMetadata(),
|
|
});
|
|
|
|
expect(lease.providerLeaseId).toBe("pc-abc");
|
|
expect(lease.metadata).toEqual(
|
|
expect.objectContaining({
|
|
namespace: "paperclip-acme",
|
|
jobName: "pc-abc",
|
|
podName: "pc-abc-pod",
|
|
secretName: "pc-abc-env",
|
|
phase: "Running",
|
|
backend: "sandbox-cr",
|
|
resumedLease: true,
|
|
// sandbox-cr has a pod-exec channel, so native file sync stays enabled.
|
|
nativeFileSyncUnsupported: false,
|
|
}),
|
|
);
|
|
});
|
|
|
|
it("flags a resumed job-backend lease as native-sync-unsupported so the server keeps the base64 fallback", async () => {
|
|
h.clients = {
|
|
batch: {
|
|
readNamespacedJobStatus: vi.fn().mockResolvedValue({ status: { active: 1 } }),
|
|
},
|
|
core: {
|
|
listNamespacedPod: vi.fn().mockResolvedValue({
|
|
items: [{ metadata: { name: "pc-job-pod" }, status: { phase: "Running" } }],
|
|
}),
|
|
},
|
|
};
|
|
|
|
const lease = await plugin.definition.onEnvironmentResumeLease!({
|
|
driverKey: "kubernetes",
|
|
companyId: "acme",
|
|
environmentId: "env-1",
|
|
config: { inCluster: true, backend: "job" },
|
|
providerLeaseId: "pc-job",
|
|
leaseMetadata: leaseMetadata({ jobName: "pc-job", backend: "job", podName: "pc-job-pod" }),
|
|
});
|
|
|
|
expect(lease.providerLeaseId).toBe("pc-job");
|
|
expect(lease.metadata).toEqual(
|
|
expect.objectContaining({
|
|
backend: "job",
|
|
// The job backend has no exec channel; its native sync hook rejects, so
|
|
// the lease must fall back to the byte-identical base64 transport.
|
|
nativeFileSyncUnsupported: true,
|
|
}),
|
|
);
|
|
});
|
|
|
|
it("returns providerLeaseId null (expired) when the Sandbox CR is gone, so the caller falls back to acquireLease", async () => {
|
|
h.clients = {
|
|
custom: { getNamespacedCustomObject: vi.fn().mockRejectedValue(notFound()) },
|
|
core: { readNamespacedPod: vi.fn() },
|
|
};
|
|
|
|
const lease = await plugin.definition.onEnvironmentResumeLease!({
|
|
driverKey: "kubernetes",
|
|
companyId: "acme",
|
|
environmentId: "env-1",
|
|
config: CONFIG,
|
|
providerLeaseId: "pc-abc",
|
|
leaseMetadata: leaseMetadata(),
|
|
});
|
|
|
|
expect(lease.providerLeaseId).toBeNull();
|
|
expect(lease.metadata?.expired).toBe(true);
|
|
expect(lease.metadata?.reason).toMatch(/no longer exists/);
|
|
});
|
|
|
|
it("returns providerLeaseId null when the backing pod is gone", async () => {
|
|
h.clients = {
|
|
custom: {
|
|
getNamespacedCustomObject: vi.fn().mockResolvedValue(readySandboxCr("pc-abc-pod")),
|
|
},
|
|
core: { readNamespacedPod: vi.fn().mockRejectedValue(notFound()) },
|
|
};
|
|
|
|
const lease = await plugin.definition.onEnvironmentResumeLease!({
|
|
driverKey: "kubernetes",
|
|
companyId: "acme",
|
|
environmentId: "env-1",
|
|
config: CONFIG,
|
|
providerLeaseId: "pc-abc",
|
|
leaseMetadata: leaseMetadata(),
|
|
});
|
|
|
|
expect(lease.providerLeaseId).toBeNull();
|
|
expect(lease.metadata?.expired).toBe(true);
|
|
});
|
|
});
|
|
|
|
describe("onEnvironmentDestroyLease", () => {
|
|
it("deletes the Sandbox CR, pod, and per-run Secret", async () => {
|
|
const deleteCr = vi.fn().mockResolvedValue({});
|
|
const deletePod = vi.fn().mockResolvedValue({});
|
|
const deleteSecret = vi.fn().mockResolvedValue({});
|
|
h.clients = {
|
|
custom: { deleteNamespacedCustomObject: deleteCr },
|
|
core: { deleteNamespacedPod: deletePod, deleteNamespacedSecret: deleteSecret },
|
|
batch: { deleteNamespacedJob: vi.fn() },
|
|
};
|
|
|
|
await plugin.definition.onEnvironmentDestroyLease!({
|
|
driverKey: "kubernetes",
|
|
companyId: "acme",
|
|
environmentId: "env-1",
|
|
config: CONFIG,
|
|
providerLeaseId: "pc-abc",
|
|
leaseMetadata: leaseMetadata(),
|
|
});
|
|
|
|
expect(deleteCr).toHaveBeenCalledWith(
|
|
expect.objectContaining({ namespace: "paperclip-acme", name: "pc-abc" }),
|
|
);
|
|
expect(deletePod).toHaveBeenCalledWith({
|
|
namespace: "paperclip-acme",
|
|
name: "pc-abc-pod",
|
|
});
|
|
expect(deleteSecret).toHaveBeenCalledWith({
|
|
namespace: "paperclip-acme",
|
|
name: "pc-abc-env",
|
|
});
|
|
});
|
|
|
|
it("is idempotent: resolves cleanly when every resource is already gone (404)", async () => {
|
|
h.clients = {
|
|
custom: { deleteNamespacedCustomObject: vi.fn().mockRejectedValue(notFound()) },
|
|
core: {
|
|
deleteNamespacedPod: vi.fn().mockRejectedValue(notFound()),
|
|
deleteNamespacedSecret: vi.fn().mockRejectedValue(notFound()),
|
|
},
|
|
batch: { deleteNamespacedJob: vi.fn() },
|
|
};
|
|
|
|
await expect(
|
|
plugin.definition.onEnvironmentDestroyLease!({
|
|
driverKey: "kubernetes",
|
|
companyId: "acme",
|
|
environmentId: "env-1",
|
|
config: CONFIG,
|
|
providerLeaseId: "pc-abc",
|
|
leaseMetadata: leaseMetadata(),
|
|
}),
|
|
).resolves.toBeUndefined();
|
|
});
|
|
|
|
it("is a no-op when providerLeaseId is null", async () => {
|
|
const deleteCr = vi.fn();
|
|
h.clients = {
|
|
custom: { deleteNamespacedCustomObject: deleteCr },
|
|
core: { deleteNamespacedPod: vi.fn(), deleteNamespacedSecret: vi.fn() },
|
|
batch: { deleteNamespacedJob: vi.fn() },
|
|
};
|
|
|
|
await plugin.definition.onEnvironmentDestroyLease!({
|
|
driverKey: "kubernetes",
|
|
companyId: "acme",
|
|
environmentId: "env-1",
|
|
config: CONFIG,
|
|
providerLeaseId: null,
|
|
leaseMetadata: undefined,
|
|
});
|
|
|
|
expect(deleteCr).not.toHaveBeenCalled();
|
|
});
|
|
|
|
it("deletes the Job for job-backend leases", async () => {
|
|
const deleteJob = vi.fn().mockResolvedValue({});
|
|
const deleteCr = vi.fn();
|
|
h.clients = {
|
|
custom: { deleteNamespacedCustomObject: deleteCr },
|
|
core: {
|
|
deleteNamespacedPod: vi.fn().mockResolvedValue({}),
|
|
deleteNamespacedSecret: vi.fn().mockResolvedValue({}),
|
|
},
|
|
batch: { deleteNamespacedJob: deleteJob },
|
|
};
|
|
|
|
await plugin.definition.onEnvironmentDestroyLease!({
|
|
driverKey: "kubernetes",
|
|
companyId: "acme",
|
|
environmentId: "env-1",
|
|
config: { inCluster: true, backend: "job" },
|
|
providerLeaseId: "pc-job",
|
|
leaseMetadata: leaseMetadata({ jobName: "pc-job", backend: "job", podName: "pc-job-pod", secretName: "pc-job-env" }),
|
|
});
|
|
|
|
expect(deleteJob).toHaveBeenCalledWith(
|
|
expect.objectContaining({ namespace: "paperclip-acme", name: "pc-job" }),
|
|
);
|
|
expect(deleteCr).not.toHaveBeenCalled();
|
|
});
|
|
});
|