mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-10-25 17:45:53 +01:00
fix(sandbox): make the pre-READY duplex newline scan incremental
The pre-READY readiness gate searched the full accumulated buffer with
buffer.indexOf("\\n") for every chunk. String#indexOf is linear in the
searched string, so many small newline-less chunks made the cumulative
scan work quadratic in the bytes received before the cap fired. An
untrusted provider could monopolize a host worker CPU.
Record the buffer length before each append and search from that offset
with buffer.indexOf("\\n", from). A newline can only be inside the new
chunk, so the search is correct and the total scan work is linear. Reset
the offset to 0 after the blank-line skip consumes a line.
Add a regression test that counts the scan work through a test-only seam
and asserts it stays proportional to the bytes received. The O(1) buffer
cap, the protocol_contamination result, the settled byte-drop guard, the
nonce check, and the file-bridge fallback stay unchanged.
Co-authored-by: Paperclip <noreply@paperclip.ing>
This commit is contained in:
2 files changed
+94
-2
No files matched your search
@@ -14,6 +14,7 @@ import {
|
||||
} from "./sandbox-callback-bridge.js";
|
||||
|
||||
import {
|
||||
__duplexReadinessTesting,
|
||||
DEFAULT_REMOTE_SANDBOX_ADAPTER_TIMEOUT_SEC,
|
||||
adapterExecutionTargetDuplexTelemetryRecorder,
|
||||
adapterExecutionTargetEnablesSandboxDuplexBridge,
|
||||
@@ -4050,6 +4051,62 @@ describe("sandbox adapter execution targets", () => {
|
||||
}
|
||||
}, 20000);
|
||||
|
||||
it("bounds the pre-READY newline-scan work by the bytes received", async () => {
|
||||
const rootDir = await mkdtemp(path.join(os.tmpdir(), "paperclip-duplex-scan-"));
|
||||
cleanupDirs.push(rootDir);
|
||||
const remoteCwd = path.join(rootDir, "workspace");
|
||||
await mkdir(remoteCwd, { recursive: true });
|
||||
const api = await startRecordingApiServer();
|
||||
// An adversarial provider sends many small newline-less chunks before the
|
||||
// cap fires. Each chunk must scan only the new bytes, not the whole buffer,
|
||||
// so the total newline-scan work stays linear in the bytes received. A
|
||||
// per-chunk full rescan makes the work quadratic.
|
||||
const readinessBufferCapBytes = DEFAULT_MAX_DUPLEX_FRAME_BYTES + 4_096;
|
||||
const smallChunk = "x".repeat(64);
|
||||
const chunkCount = Math.ceil(readinessBufferCapBytes / smallChunk.length) + 1;
|
||||
const totalBytes = chunkCount * smallChunk.length;
|
||||
const { runner, control } = makeDuplexSelectionRunner((ctx) => {
|
||||
for (let i = 0; i < chunkCount; i += 1) {
|
||||
ctx.emitRaw(smallChunk);
|
||||
}
|
||||
});
|
||||
const { recorder } = createRecordingDuplexRecorder();
|
||||
const target: AdapterSandboxExecutionTarget = {
|
||||
kind: "remote",
|
||||
transport: "sandbox",
|
||||
providerKey: "daytona",
|
||||
remoteCwd,
|
||||
timeoutMs: 30_000,
|
||||
runner,
|
||||
effectiveCapabilities: duplexCapabilities(true),
|
||||
};
|
||||
|
||||
__duplexReadinessTesting.resetNewlineScanUnits();
|
||||
const bridge = await startAdapterExecutionTargetPaperclipBridge({
|
||||
runId: "run-scan-bound",
|
||||
target,
|
||||
runtimeRootDir: path.join(remoteCwd, ".paperclip-runtime", "codex"),
|
||||
adapterKey: "codex",
|
||||
hostApiToken: "real-run-jwt",
|
||||
hostApiUrl: api.origin,
|
||||
enableSandboxDuplexBridge: true,
|
||||
duplexReadinessTimeoutMs: 5_000,
|
||||
duplexTelemetryRecorder: recorder,
|
||||
});
|
||||
try {
|
||||
expect(bridge).not.toBeNull();
|
||||
expect(control.openCount).toBe(1);
|
||||
const scanUnits = __duplexReadinessTesting.readNewlineScanUnits();
|
||||
// Linear scan work reads each byte one time, so the count stays near
|
||||
// totalBytes. A per-chunk full rescan is quadratic (about
|
||||
// totalBytes^2 / (2 * chunkSize)), far above this bound.
|
||||
expect(scanUnits).toBeLessThanOrEqual(4 * totalBytes);
|
||||
} 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);
|
||||
|
||||
@@ -2402,6 +2402,35 @@ async function ensureSandboxRunLogDirectory(input: {
|
||||
* fails the handshake. The gate never dispatches a request; the broker does that
|
||||
* after readiness passes.
|
||||
*/
|
||||
// The count of the pre-READY newline-scan work, in UTF-16 code units. Each
|
||||
// search adds the number of code units it can read. A test reads this count to
|
||||
// prove the scan work stays linear in the bytes received. Production code never
|
||||
// reads this count.
|
||||
let duplexReadinessNewlineScanUnits = 0;
|
||||
|
||||
/**
|
||||
* Find the first newline in `buffer` at or after index `from` and return its
|
||||
* index, or -1. `String#indexOf` reads at most `buffer.length - from` code
|
||||
* units, so the search from the end of the prior bytes keeps the pre-READY scan
|
||||
* work linear in the bytes received. This helper counts that work for a test.
|
||||
*/
|
||||
function findNewlineFrom(buffer: string, from: number): number {
|
||||
duplexReadinessNewlineScanUnits += buffer.length - from;
|
||||
return buffer.indexOf("\n", from);
|
||||
}
|
||||
|
||||
/**
|
||||
* Test-only surface for the pre-READY readiness gate. A test reads the scan
|
||||
* count to prove the newline search work stays linear in the bytes received.
|
||||
* Production code does not use this object.
|
||||
*/
|
||||
export const __duplexReadinessTesting = {
|
||||
readNewlineScanUnits: (): number => duplexReadinessNewlineScanUnits,
|
||||
resetNewlineScanUnits: (): void => {
|
||||
duplexReadinessNewlineScanUnits = 0;
|
||||
},
|
||||
};
|
||||
|
||||
interface DuplexReadinessGate {
|
||||
/** Resolves with the handshake outcome. It never rejects. */
|
||||
readonly ready: Promise<DuplexReadinessResult>;
|
||||
@@ -2460,9 +2489,13 @@ function createDuplexReadinessGate(
|
||||
// never grows the buffer after the gate settles.
|
||||
return;
|
||||
}
|
||||
// The prior bytes hold no newline, so a newline can only be inside the new
|
||||
// chunk. Record the current length as the scan start, then search from it.
|
||||
// The total scan work stays linear in the bytes received.
|
||||
let searchFrom = buffer.length;
|
||||
buffer += chunk;
|
||||
for (;;) {
|
||||
const newlineIndex = buffer.indexOf("\n");
|
||||
const newlineIndex = findNewlineFrom(buffer, searchFrom);
|
||||
if (newlineIndex === -1) {
|
||||
// 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
|
||||
@@ -2483,8 +2516,10 @@ function createDuplexReadinessGate(
|
||||
const line = buffer.slice(0, newlineIndex);
|
||||
const rest = buffer.slice(newlineIndex + 1);
|
||||
if (line.length === 0) {
|
||||
// Skip a leading blank line, the same as the frame decoder.
|
||||
// Skip a leading blank line, the same as the frame decoder. The `rest`
|
||||
// is a fresh buffer, so search it from offset 0.
|
||||
buffer = rest;
|
||||
searchFrom = 0;
|
||||
continue;
|
||||
}
|
||||
const decoded = decodeDuplexLine(line);
|
||||
|
||||
Reference in new issue
Block a user