mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-10-26 02:00:52 +01:00
fix(sandbox): bound the pre-READY duplex buffer cap check in O(1)
The duplex readiness gate checked the pre-READY buffer cap with Buffer.byteLength, which is O(n) in the buffer length. A provider that controls the chunk size could send many small newline-less chunks, so the per-chunk byte scan made the pre-READY window quadratic in the bytes received. This was a CPU-exhaustion path on channel open. Gate on buffer.length, the UTF-16 code-unit count, which is O(1). The UTF-8 byte length is greater than or equal to the UTF-16 code-unit count for every string, so the check never fires before the byte cap is truly exceeded. It can fire late by at most a factor of 3, so peak buffer memory stays bounded near 3 MB. The failure reason and every other gate behavior stay unchanged. Add a test that feeds many small newline-less chunks past the cap and asserts the gate finishes with protocol contamination. Correct one telemetry README sentence. Counters carry no dimension labels; fallback_reason and loss_class fold into the counter metric name. The full closed dimension set rides only the spans and the transport event. Co-authored-by: Paperclip <noreply@paperclip.ing>
This commit is contained in:
3 files changed
+72
-3
No files matched your search
@@ -3993,6 +3993,63 @@ describe("sandbox adapter execution targets", () => {
|
||||
}
|
||||
}, 20000);
|
||||
|
||||
it("caps the pre-READY buffer under many small newline-less chunks", async () => {
|
||||
const rootDir = await mkdtemp(path.join(os.tmpdir(), "paperclip-duplex-cap-small-"));
|
||||
cleanupDirs.push(rootDir);
|
||||
const remoteCwd = path.join(rootDir, "workspace");
|
||||
await mkdir(remoteCwd, { recursive: true });
|
||||
const api = await startRecordingApiServer();
|
||||
// An adversarial provider controls the chunk size. It sends many small
|
||||
// newline-less chunks that together pass the cap. The gate must scan each
|
||||
// chunk in O(1) of the buffer length, so the pre-READY window stays bounded.
|
||||
// The gate caps the buffer, finishes with protocol contamination, and falls
|
||||
// back to the file bridge.
|
||||
const readinessBufferCapBytes = DEFAULT_MAX_DUPLEX_FRAME_BYTES + 4_096;
|
||||
const smallChunk = "x".repeat(64);
|
||||
const chunkCount = Math.ceil(readinessBufferCapBytes / smallChunk.length) + 1;
|
||||
const { runner, control } = makeDuplexSelectionRunner((ctx) => {
|
||||
for (let i = 0; i < chunkCount; i += 1) {
|
||||
ctx.emitRaw(smallChunk);
|
||||
}
|
||||
});
|
||||
const { recorder, counters } = createRecordingDuplexRecorder();
|
||||
const target: AdapterSandboxExecutionTarget = {
|
||||
kind: "remote",
|
||||
transport: "sandbox",
|
||||
providerKey: "daytona",
|
||||
remoteCwd,
|
||||
timeoutMs: 30_000,
|
||||
runner,
|
||||
effectiveCapabilities: duplexCapabilities(true),
|
||||
};
|
||||
|
||||
const bridge = await startAdapterExecutionTargetPaperclipBridge({
|
||||
runId: "run-cap-small",
|
||||
target,
|
||||
runtimeRootDir: path.join(remoteCwd, ".paperclip-runtime", "codex"),
|
||||
adapterKey: "codex",
|
||||
hostApiToken: "real-run-jwt",
|
||||
hostApiUrl: api.origin,
|
||||
enableSandboxDuplexBridge: true,
|
||||
// A long readiness timeout, so the buffer cap, not the timeout, drives the
|
||||
// failure.
|
||||
duplexReadinessTimeoutMs: 5_000,
|
||||
duplexTelemetryRecorder: recorder,
|
||||
});
|
||||
try {
|
||||
expect(bridge).not.toBeNull();
|
||||
expect(control.openCount).toBe(1);
|
||||
// The cap drove the failure, so the file bridge serves after the bounded cleanup.
|
||||
expect(bridge?.env.PAPERCLIP_API_BRIDGE_MODE).toBe("queue_v1");
|
||||
const fallback = counters.find((c) => c.metric === DUPLEX_COUNTER_FALLBACK_TOTAL);
|
||||
expect(fallback?.dimensions.fallback_reason).toBe("contaminated");
|
||||
expect(control.closeCount + control.stopCount).toBeGreaterThanOrEqual(1);
|
||||
} finally {
|
||||
await bridge?.stop();
|
||||
await api.close();
|
||||
}
|
||||
}, 20000);
|
||||
|
||||
it("drops a frame header outside the allowlist on the host duplex forward path", async () => {
|
||||
const rootDir = await mkdtemp(path.join(os.tmpdir(), "paperclip-duplex-hdr-"));
|
||||
cleanupDirs.push(rootDir);
|
||||
|
||||
@@ -2467,7 +2467,15 @@ function createDuplexReadinessGate(
|
||||
// No READY line yet. The gate reads untrusted bytes, so bound the
|
||||
// pre-READY buffer. Past the cap with no newline, the stream cannot be a
|
||||
// valid READY frame, so finish with protocol contamination.
|
||||
if (Buffer.byteLength(buffer, "utf8") > DUPLEX_READINESS_BUFFER_CAP_BYTES) {
|
||||
//
|
||||
// Gate on buffer.length, the UTF-16 code-unit count, which is O(1).
|
||||
// Buffer.byteLength is O(n), so a byte check on every newline-less chunk
|
||||
// makes the pre-READY window quadratic in the bytes received. The UTF-8
|
||||
// byte length is greater than or equal to the UTF-16 code-unit count for
|
||||
// every string, so this check never fires before the byte cap is truly
|
||||
// exceeded. It can fire late by at most a factor of 3, so peak buffer
|
||||
// memory stays bounded near 3 MB.
|
||||
if (buffer.length > DUPLEX_READINESS_BUFFER_CAP_BYTES) {
|
||||
finish({ ok: false, reason: "protocol_contamination" });
|
||||
}
|
||||
return;
|
||||
|
||||
@@ -352,8 +352,12 @@ field.
|
||||
|
||||
### Dimension keys
|
||||
|
||||
Every span attribute, counter label, and event field uses only these closed keys.
|
||||
A test asserts the exact set, so a new key never reaches a sink by accident.
|
||||
Counters carry no dimension labels. The guarded counter store keys each counter
|
||||
on `(companyId, metric)` with no label column, so the `fallback_reason` and
|
||||
`loss_class` values fold into the counter metric name instead. The full closed
|
||||
dimension set below rides only the spans and the `sandbox.duplex.transport`
|
||||
event, which use only these closed keys. A test asserts the exact set, so a new
|
||||
key never reaches a sink by accident.
|
||||
|
||||
| Key | Type | Optional | Value set |
|
||||
| --- | --- | --- | --- |
|
||||
|
||||
Reference in new issue
Block a user