mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-10-28 04:02:18 +01:00
## Thinking Path > - Paperclip is an open-source AI-agent management platform; agents run tasks inside sandboxed environments (Daytona, Kubernetes, E2B, etc.) > - The control-plane ↔ sandbox file-transfer path flows through the `environmentExecute` seam in `protocol.ts` — the only verb available to plugins — which forces a base64-over-exec chunked loop for every file move: workspace files, assets, Codex home sync > - This transport is correct and safe, but it bypasses provider-native bulk/streaming APIs (Daytona `uploadFiles`, K8s `FastUploadInterceptor` / volume mounts), leaving significant throughput on the table for large workspaces > - The right fix is an opt-in seam extension: providers with faster native transfer declare two optional verbs; providers that do not opt in stay on the existing fallback with zero code or behavior change required > - This PR adds the first layer of that extension — two optional verbs (`environmentSyncIn` / `environmentSyncOut`) in the plugin SDK, the runtime plumbing to prefer the native path for the two clean destroy-then-replace cases, and a doc for the contract > - The core correctness invariant is byte-identical fallback: if no provider opts in, execution is exactly what ships today; `assertSyncOperationsConfined` enforces host-side path confinement for providers that do opt in > - No provider advertises the verbs yet → zero production behavior change; future PRs wire up Daytona and K8s providers against this contract ## Linked Issues or Issue Description No public GitHub issue exists for this feature. Description follows the `feature_request` issue template: **Subsystem affected:** packages/plugins — plugin system; packages/adapter-utils — adapter runtime; server/ — EnvironmentRuntimeService **Problem or motivation:** Sandbox file transfers currently always use a base64-over-exec chunked loop regardless of what the underlying provider supports. For workspaces larger than a few MB this becomes the dominant wall-clock cost of every sandbox run, and it bypasses bulk/stream APIs that providers like Daytona already expose natively. **Proposed solution:** Add two optional, opt-in plugin hooks — `onEnvironmentSyncIn` / `onEnvironmentSyncOut` — to the plugin SDK. When a provider defines both hooks and both are advertised via the existing `supportedMethods` negotiation, the runtime prefers the native path for the two clean destroy-then-replace transfer cases; all other cases fall back to the existing byte-identical base64 transport. **Alternatives considered:** An unconditional verb would require every provider to implement or stub the verb. The opt-in / `METHOD_NOT_IMPLEMENTED` pattern (already used by `environmentExecute`) preserves backward compatibility with zero provider changes required. **Roadmap alignment:** Consistent with the ✅ "Cloud / Sandbox agents" and ✅ "Plugin system" milestones; extends the plugin seam rather than adding control-plane-level logic. **Additional context:** Searched open pull requests and issues for duplicate sandbox file-sync / native-transfer work; none found. ## What Changed - **`packages/plugins/sdk`** - `protocol.ts`: two new optional `HostToWorkerMethods` — `environmentSyncIn` / `environmentSyncOut` — plus generic `SyncOperation`, `SyncFileMapping`, and `SyncOutcome` types - `define-plugin.ts`: optional `onEnvironmentSyncIn` / `onEnvironmentSyncOut` fields on `PluginDefinition`; worker advertises each verb only when its hook is defined (else `METHOD_NOT_IMPLEMENTED`, mirroring `environmentExecute`) - `worker-rpc-host.ts`: route new verbs to plugin hooks - `index.ts`: re-export new public types - **`packages/adapter-utils`** - `command-managed-runtime.ts`: expose optional `syncIn` / `syncOut` on `CommandManagedRuntimeRunner` (available only when both verbs are advertised); add `assertSyncOperationsConfined` host-side path-confinement guard - `sandbox-managed-runtime.ts`: `SandboxManagedRuntimeClient` gains optional `syncIn` / `syncOut`; orchestrator prefers native path for default-provision asset inbound and workspace-download-into-fresh-dir outbound; all other paths keep the existing base64 fallback - `sandbox-file-sync.test.ts` (new): 234-line characterization suite — native-opt-in branch, fallback branch, `assertSyncOperationsConfined` escape-path rejection, `followSymlinks` → tar `-h` - `command-managed-runtime.test.ts`: negotiation + native-sync + confinement tests - **`server/src/services/environment-runtime.ts`**: `EnvironmentRuntimeService` delegates to `syncIn` / `syncOut`, gated on advertised support - **`server/src/services/environment-execution-target.ts`**: minor typing fix alongside the new verbs - **`doc/plugins/SANDBOX_FILE_SYNC_HOOKS.md`** (new): documents the full contract — opt-in / no-op guarantee, operation ordering, provider-may-tar, atomicity, `followSymlinks`, secret modes (0600, no window), path confinement, `operationId` opacity, resource bounds, shell-quoting ## Verification ```bash # SDK suite pnpm --filter packages/plugins/sdk test # Adapter-utils suite (includes new sandbox-file-sync characterization tests) pnpm --filter packages/adapter-utils test # Expected: 255 pass / 4 skip # Type-check across affected packages pnpm --filter packages/plugins/sdk typecheck pnpm --filter packages/adapter-utils typecheck # Server changed-file spot check: cd server && npx tsc --noEmit --skipLibCheck 2>&1 | grep -E "environment-(runtime|execution-target)" | head -20 ``` Key behavioral invariant to spot-check: with no provider opting in (the current state), run any sandbox task and confirm file-transfer behavior is byte-for-byte identical to what the pre-PR code produces. The characterization tests assert this at the unit level. ## Risks - **Zero production risk today**: no provider advertises `environmentSyncIn` / `environmentSyncOut`, so the new code paths are unreachable in production; all real traffic stays on the existing base64 fallback - **Path confinement**: `assertSyncOperationsConfined` rejects any `targetPath` that escapes the declared root — this is the primary security boundary for future providers. The test suite covers escape-path rejection - **Atomicity**: the contract delegates atomicity to providers; the doc explicitly calls out that directory-level ops are not guaranteed atomic - **Secret transport**: credential assets (e.g., Codex `auth.json`, directory mappings) continue to use the existing tar path — they do not go through the new verbs in any current provider > For core feature work, check [`ROADMAP.md`](ROADMAP.md) first and discuss it in `#dev` before opening the PR. Feature PRs that overlap with planned core work may need to be redirected — check the roadmap first. See `CONTRIBUTING.md`. ## Model Used Provider: Anthropic Model: `claude-sonnet-4-6` (Claude Sonnet 4.6) Context window: 200 K tokens Capabilities: extended tool use, multi-file code generation, agentic reasoning via the Paperclip agent framework ## 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>
235 lines
11 KiB
TypeScript
235 lines
11 KiB
TypeScript
import { lstat, mkdir, mkdtemp, readFile, readdir, rm, symlink, writeFile } from "node:fs/promises";
|
|
import os from "node:os";
|
|
import path from "node:path";
|
|
import { execFile as execFileCallback } from "node:child_process";
|
|
import { promisify } from "node:util";
|
|
import { afterEach, describe, expect, it } from "vitest";
|
|
|
|
import {
|
|
assertSyncOperationsConfined,
|
|
prepareSandboxManagedRuntime,
|
|
type SandboxManagedRuntimeClient,
|
|
type SandboxSyncOperation,
|
|
} from "./sandbox-managed-runtime.js";
|
|
|
|
const execFile = promisify(execFileCallback);
|
|
|
|
interface RecordingClient {
|
|
client: SandboxManagedRuntimeClient;
|
|
syncInOps: SandboxSyncOperation[][];
|
|
syncOutOps: SandboxSyncOperation[][];
|
|
}
|
|
|
|
// A filesystem-backed client that additionally exposes native syncIn/syncOut,
|
|
// mirroring a provider that opted into the sync verbs. The native transfer is a
|
|
// faithful destroy-then-replace directory copy honoring followSymlinks.
|
|
function makeNativeClient(): RecordingClient {
|
|
const syncInOps: SandboxSyncOperation[][] = [];
|
|
const syncOutOps: SandboxSyncOperation[][] = [];
|
|
|
|
const transferDirectory = async (
|
|
sourcePath: string,
|
|
targetPath: string,
|
|
followSymlinks: boolean | undefined,
|
|
): Promise<number> => {
|
|
await rm(targetPath, { recursive: true, force: true });
|
|
await mkdir(targetPath, { recursive: true });
|
|
// followSymlinks true dereferences to bytes (like tar -h); falsy preserves links.
|
|
const copyArgs = followSymlinks ? ["-RL"] : ["-a"];
|
|
await execFile("cp", [...copyArgs, `${sourcePath}/.`, targetPath]);
|
|
const entries = await readdir(targetPath, { withFileTypes: true }).catch(() => []);
|
|
return entries.length;
|
|
};
|
|
|
|
const applyOperations = async (operations: SandboxSyncOperation[]) => ({
|
|
operations: await Promise.all(operations.map(async (operation) => {
|
|
let filesTransferred = 0;
|
|
for (const mapping of operation.files) {
|
|
if (mapping.kind === "directory") {
|
|
filesTransferred += await transferDirectory(mapping.sourcePath, mapping.targetPath, mapping.followSymlinks);
|
|
} else {
|
|
await mkdir(path.dirname(mapping.targetPath), { recursive: true });
|
|
await writeFile(mapping.targetPath, await readFile(mapping.sourcePath));
|
|
filesTransferred += 1;
|
|
}
|
|
}
|
|
return { operationId: operation.operationId, filesTransferred, bytesTransferred: 0 };
|
|
})),
|
|
});
|
|
|
|
const client: SandboxManagedRuntimeClient = {
|
|
makeDir: async (remotePath) => { await mkdir(remotePath, { recursive: true }); },
|
|
writeFile: async (remotePath, bytes) => {
|
|
await mkdir(path.dirname(remotePath), { recursive: true });
|
|
await writeFile(remotePath, Buffer.from(bytes));
|
|
},
|
|
readFile: async (remotePath) => await readFile(remotePath),
|
|
listFiles: async (remotePath) => {
|
|
const entries = await readdir(remotePath, { withFileTypes: true }).catch(() => []);
|
|
return entries.filter((e) => e.isFile()).map((e) => e.name).sort();
|
|
},
|
|
remove: async (remotePath) => { await rm(remotePath, { recursive: true, force: true }); },
|
|
run: async (command) => { await execFile("sh", ["-c", command], { maxBuffer: 32 * 1024 * 1024 }); },
|
|
syncIn: async (operations) => { syncInOps.push(operations); return applyOperations(operations); },
|
|
syncOut: async (operations) => { syncOutOps.push(operations); return applyOperations(operations); },
|
|
};
|
|
|
|
return { client, syncInOps, syncOutOps };
|
|
}
|
|
|
|
describe("sandbox native file sync", () => {
|
|
const cleanupDirs: string[] = [];
|
|
afterEach(async () => {
|
|
while (cleanupDirs.length > 0) {
|
|
const dir = cleanupDirs.pop();
|
|
if (dir) await rm(dir, { recursive: true, force: true }).catch(() => undefined);
|
|
}
|
|
});
|
|
|
|
it("prefers the native path for default-provision asset inbound and workspace outbound", async () => {
|
|
const rootDir = await mkdtemp(path.join(os.tmpdir(), "paperclip-native-sync-"));
|
|
cleanupDirs.push(rootDir);
|
|
const localWorkspaceDir = path.join(rootDir, "local-workspace");
|
|
const remoteWorkspaceDir = path.join(rootDir, "remote-workspace");
|
|
const localAssetsDir = path.join(rootDir, "local-assets");
|
|
await mkdir(localWorkspaceDir, { recursive: true });
|
|
await mkdir(localAssetsDir, { recursive: true });
|
|
await writeFile(path.join(localWorkspaceDir, "README.md"), "local workspace\n", "utf8");
|
|
await writeFile(path.join(localAssetsDir, "skill.md"), "skill body\n", "utf8");
|
|
|
|
const { client, syncInOps, syncOutOps } = makeNativeClient();
|
|
const prepared = await prepareSandboxManagedRuntime({
|
|
spec: { transport: "sandbox", provider: "test", sandboxId: "s1", remoteCwd: remoteWorkspaceDir, timeoutMs: 30_000, apiKey: null },
|
|
adapterKey: "test-adapter",
|
|
client,
|
|
workspaceLocalDir: localWorkspaceDir,
|
|
assets: [{ key: "skills", localDir: localAssetsDir }],
|
|
});
|
|
|
|
// The default-provision asset was transferred through syncIn as a single
|
|
// directory mapping with an opaque operationId; the file landed in place.
|
|
const inboundOps = syncInOps.flat();
|
|
expect(inboundOps.length).toBe(1);
|
|
const assetOp = inboundOps[0];
|
|
expect(assetOp.operationId).toMatch(/^sync-op-\d+$/);
|
|
expect(assetOp.operationId).not.toContain("skills");
|
|
expect(assetOp.files).toEqual([
|
|
{ sourcePath: localAssetsDir, targetPath: prepared.assetDirs.skills, kind: "directory", exclude: undefined, followSymlinks: undefined },
|
|
]);
|
|
expect(await readFile(path.join(prepared.assetDirs.skills, "skill.md"), "utf8")).toBe("skill body\n");
|
|
|
|
// Mutate the sandbox workspace, then restore through the native outbound path.
|
|
await writeFile(path.join(remoteWorkspaceDir, "README.md"), "remote workspace\n", "utf8");
|
|
await writeFile(path.join(remoteWorkspaceDir, "new.txt"), "added\n", "utf8");
|
|
await prepared.restoreWorkspace();
|
|
|
|
const outboundOps = syncOutOps.flat();
|
|
expect(outboundOps.length).toBe(1);
|
|
expect(outboundOps[0].files[0]).toMatchObject({ sourcePath: remoteWorkspaceDir, kind: "directory" });
|
|
expect(await readFile(path.join(localWorkspaceDir, "README.md"), "utf8")).toBe("remote workspace\n");
|
|
expect(await readFile(path.join(localWorkspaceDir, "new.txt"), "utf8")).toBe("added\n");
|
|
});
|
|
|
|
it("keeps a custom-provision asset on the tar fallback even when native sync is available", async () => {
|
|
const rootDir = await mkdtemp(path.join(os.tmpdir(), "paperclip-native-custom-"));
|
|
cleanupDirs.push(rootDir);
|
|
const localWorkspaceDir = path.join(rootDir, "local-workspace");
|
|
const remoteWorkspaceDir = path.join(rootDir, "remote-workspace");
|
|
const localAssetsDir = path.join(rootDir, "local-assets");
|
|
await mkdir(localWorkspaceDir, { recursive: true });
|
|
await mkdir(localAssetsDir, { recursive: true });
|
|
await writeFile(path.join(localWorkspaceDir, "README.md"), "ws\n", "utf8");
|
|
await writeFile(path.join(localAssetsDir, "cred.txt"), "secret\n", "utf8");
|
|
|
|
const { client, syncInOps } = makeNativeClient();
|
|
const prepared = await prepareSandboxManagedRuntime({
|
|
spec: { transport: "sandbox", provider: "test", sandboxId: "s1", remoteCwd: remoteWorkspaceDir, timeoutMs: 30_000, apiKey: null },
|
|
adapterKey: "test-adapter",
|
|
client,
|
|
workspaceLocalDir: localWorkspaceDir,
|
|
assets: [{
|
|
key: "creds",
|
|
localDir: localAssetsDir,
|
|
// A bespoke extract command (e.g. a credential merge) cannot be a generic
|
|
// file mapping, so the orchestrator keeps it on the tar path.
|
|
provision: { extractCommand: ({ assetTarPath, assetDir }) =>
|
|
`rm -rf ${assetDir} && mkdir -p ${assetDir} && tar -xf ${assetTarPath} -C ${assetDir} && rm -f ${assetTarPath}` },
|
|
}],
|
|
});
|
|
|
|
// No syncIn operation for the custom asset; it still materializes via tar.
|
|
expect(syncInOps.flat().length).toBe(0);
|
|
expect(await readFile(path.join(prepared.assetDirs.creds, "cred.txt"), "utf8")).toBe("secret\n");
|
|
});
|
|
|
|
it("dereferences symlinks only when followSymlinks is true (native honors the flag)", async () => {
|
|
const rootDir = await mkdtemp(path.join(os.tmpdir(), "paperclip-native-symlink-"));
|
|
cleanupDirs.push(rootDir);
|
|
const localWorkspaceDir = path.join(rootDir, "local-workspace");
|
|
const remoteWorkspaceDir = path.join(rootDir, "remote-workspace");
|
|
const assetsPreserve = path.join(rootDir, "assets-preserve");
|
|
const assetsDeref = path.join(rootDir, "assets-deref");
|
|
const target = path.join(rootDir, "target.md");
|
|
await mkdir(localWorkspaceDir, { recursive: true });
|
|
await mkdir(assetsPreserve, { recursive: true });
|
|
await mkdir(assetsDeref, { recursive: true });
|
|
await writeFile(path.join(localWorkspaceDir, "README.md"), "ws\n", "utf8");
|
|
await writeFile(target, "link body\n", "utf8");
|
|
await symlink(target, path.join(assetsPreserve, "link.md"));
|
|
await symlink(target, path.join(assetsDeref, "link.md"));
|
|
|
|
const { client } = makeNativeClient();
|
|
const prepared = await prepareSandboxManagedRuntime({
|
|
spec: { transport: "sandbox", provider: "test", sandboxId: "s1", remoteCwd: remoteWorkspaceDir, timeoutMs: 30_000, apiKey: null },
|
|
adapterKey: "test-adapter",
|
|
client,
|
|
workspaceLocalDir: localWorkspaceDir,
|
|
assets: [
|
|
{ key: "preserve", localDir: assetsPreserve, followSymlinks: false },
|
|
{ key: "deref", localDir: assetsDeref, followSymlinks: true },
|
|
],
|
|
});
|
|
|
|
expect((await lstat(path.join(prepared.assetDirs.preserve, "link.md"))).isSymbolicLink()).toBe(true);
|
|
const dereffed = await lstat(path.join(prepared.assetDirs.deref, "link.md"));
|
|
expect(dereffed.isSymbolicLink()).toBe(false);
|
|
expect(await readFile(path.join(prepared.assetDirs.deref, "link.md"), "utf8")).toBe("link body\n");
|
|
});
|
|
});
|
|
|
|
describe("assertSyncOperationsConfined", () => {
|
|
const op = (targetPath: string, sourcePath = "/host/src"): SandboxSyncOperation[] => [
|
|
{ operationId: "sync-op-1", files: [{ sourcePath, targetPath, kind: "directory" }] },
|
|
];
|
|
|
|
it("accepts targets within an allowed root", () => {
|
|
expect(() => assertSyncOperationsConfined(op("/remote/ws/sub"), {
|
|
sourceRoots: ["/host/src"], targetRoots: ["/remote/ws"],
|
|
})).not.toThrow();
|
|
});
|
|
|
|
it("rejects a relative target", () => {
|
|
expect(() => assertSyncOperationsConfined(op("relative/path"), {
|
|
sourceRoots: ["/host/src"], targetRoots: ["/remote/ws"],
|
|
})).toThrow(/confined absolute path/);
|
|
});
|
|
|
|
it("rejects a parent-traversal escape", () => {
|
|
expect(() => assertSyncOperationsConfined(op("/remote/ws/../etc/passwd"), {
|
|
sourceRoots: ["/host/src"], targetRoots: ["/remote/ws"],
|
|
})).toThrow(/confined absolute path|escapes its confinement root/);
|
|
});
|
|
|
|
it("rejects an absolute target outside every root", () => {
|
|
expect(() => assertSyncOperationsConfined(op("/etc/passwd"), {
|
|
sourceRoots: ["/host/src"], targetRoots: ["/remote/ws"],
|
|
})).toThrow(/escapes its confinement root/);
|
|
});
|
|
|
|
it("rejects a source outside every source root", () => {
|
|
expect(() => assertSyncOperationsConfined(op("/remote/ws/ok", "/etc/shadow"), {
|
|
sourceRoots: ["/host/src"], targetRoots: ["/remote/ws"],
|
|
})).toThrow(/escapes its confinement root/);
|
|
});
|
|
});
|