Skip to content

Commit 5140eab

Browse files
committed
fix(agent): enforce combined dispatch capture bound
Signed-off-by: Prekshi Vyas <prekshiv@nvidia.com>
1 parent 18a98f9 commit 5140eab

4 files changed

Lines changed: 53 additions & 22 deletions

File tree

src/lib/actions/sandbox/agent/passthrough-dispatch.test.ts

Lines changed: 29 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ import {
1212
runAgentDispatch,
1313
SILENT_AGENT_DISPATCH_EXIT_CODE,
1414
} from "./passthrough-dispatch";
15-
import type { SandboxExecSignalSource } from "../exec";
15+
import { computeExitCode, type SandboxExecSignalSource } from "../exec";
1616

1717
function dispatchHarness() {
1818
const childEvents = new EventEmitter();
@@ -79,9 +79,36 @@ describe("runAgentDispatch", () => {
7979

8080
const result = await pending;
8181
expect(harness.child.kill).toHaveBeenCalledWith("SIGTERM");
82-
expect(result.error).toEqual(new Error("agent stdout exceeded the 4-byte capture limit"));
82+
expect(result.error).toEqual(
83+
new Error("agent output exceeded the 4-byte combined capture limit"),
84+
);
85+
expect(computeExitCode(result)).toEqual({
86+
code: 1,
87+
errorMessage: "agent output exceeded the 4-byte combined capture limit",
88+
});
8389
expect(result.stdout).toBe("");
8490
});
91+
92+
it("enforces one capture bound across stdout and stderr", async () => {
93+
const harness = dispatchHarness();
94+
const pending = runAgentDispatch(
95+
"openshell",
96+
["sandbox", "exec", "--name", "alpha", "--", "openclaw", "agent"],
97+
{ maxBufferBytes: 6, stdinIsTty: false },
98+
{ signalSource: harness.signalSource, spawnChild: () => harness.child },
99+
);
100+
101+
harness.stdout.emit("data", "1234");
102+
harness.stderr.emit("data", "567");
103+
104+
const result = await pending;
105+
expect(harness.child.kill).toHaveBeenCalledWith("SIGTERM");
106+
expect(result.error).toEqual(
107+
new Error("agent output exceeded the 6-byte combined capture limit"),
108+
);
109+
expect(result.stdout).toBe("1234");
110+
expect(result.stderr).toBe("");
111+
});
85112
});
86113

87114
describe("isSilentAgentDispatch", () => {

src/lib/actions/sandbox/agent/passthrough-dispatch.ts

Lines changed: 15 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -84,6 +84,11 @@ type AgentDispatchReadable = {
8484
on(event: "data", listener: (chunk: Buffer | string) => void): unknown;
8585
};
8686

87+
type AgentDispatchCaptureBudget = {
88+
bytes: number;
89+
overflowed: boolean;
90+
};
91+
8792
export type AgentDispatchChild = SandboxExecChild & {
8893
stderr: AgentDispatchReadable | null;
8994
stdout: AgentDispatchReadable | null;
@@ -116,26 +121,25 @@ const defaultAgentDispatchSpawner: AgentDispatchSpawner = (binary, args, stdio)
116121

117122
function captureAgentDispatchStream(
118123
stream: AgentDispatchReadable | null,
119-
streamName: "stderr" | "stdout",
120124
child: AgentDispatchChild,
121125
chunks: Buffer[],
122126
maxBufferBytes: number,
127+
budget: AgentDispatchCaptureBudget,
123128
setOverflowError: (error: Error) => void,
124129
): void {
125-
let size = 0;
126-
let overflowed = false;
127130
stream?.on("data", (chunk) => {
128-
if (overflowed) return;
131+
if (budget.overflowed) return;
129132
const data = Buffer.isBuffer(chunk) ? chunk : Buffer.from(chunk);
130-
size += data.byteLength;
131-
if (size > maxBufferBytes) {
132-
overflowed = true;
133+
const nextSize = budget.bytes + data.byteLength;
134+
if (nextSize > maxBufferBytes) {
135+
budget.overflowed = true;
133136
setOverflowError(
134-
new Error(`agent ${streamName} exceeded the ${maxBufferBytes}-byte capture limit`),
137+
new Error(`agent output exceeded the ${maxBufferBytes}-byte combined capture limit`),
135138
);
136139
if (child.exitCode === null && child.signalCode === null) child.kill("SIGTERM");
137140
return;
138141
}
142+
budget.bytes = nextSize;
139143
chunks.push(data);
140144
});
141145
}
@@ -155,6 +159,7 @@ export async function runAgentDispatch(
155159
): Promise<AgentDispatchResult> {
156160
const stderrChunks: Buffer[] = [];
157161
const stdoutChunks: Buffer[] = [];
162+
const captureBudget: AgentDispatchCaptureBudget = { bytes: 0, overflowed: false };
158163
let overflowError: Error | undefined;
159164
const maxBufferBytes = options.maxBufferBytes ?? DEFAULT_AGENT_DISPATCH_MAX_BUFFER_BYTES;
160165
const spawnChild = deps.spawnChild ?? defaultAgentDispatchSpawner;
@@ -173,18 +178,18 @@ export async function runAgentDispatch(
173178
};
174179
captureAgentDispatchStream(
175180
child.stdout,
176-
"stdout",
177181
child,
178182
stdoutChunks,
179183
maxBufferBytes,
184+
captureBudget,
180185
setOverflowError,
181186
);
182187
captureAgentDispatchStream(
183188
child.stderr,
184-
"stderr",
185189
child,
186190
stderrChunks,
187191
maxBufferBytes,
192+
captureBudget,
188193
setOverflowError,
189194
);
190195
return child;

src/lib/actions/sandbox/agent/passthrough.test.ts

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -35,15 +35,12 @@ const isTerminalAgentMock = vi.hoisted(() =>
3535
vi.fn((agent: { runtime?: { kind?: string } }) => agent.runtime?.kind === "terminal"),
3636
);
3737

38-
vi.mock("../exec", () => ({
38+
vi.mock("../exec", async (importOriginal) => ({
39+
...(await importOriginal<typeof import("../exec")>()),
3940
execSandbox: execMock,
4041
buildOpenshellExecArgs: vi.fn((_sb: string, cmd: readonly string[]) => cmd),
4142
wrapExecCommandWithRuntimeEnv: vi.fn((cmd: readonly string[]) => cmd),
4243
wrapOpenClawAgentCommandWithRuntimeEnv: vi.fn((cmd: readonly string[]) => cmd),
43-
computeExitCode: vi.fn((result: { signal?: NodeJS.Signals | null; status: number | null }) => ({
44-
code: result.status ?? (result.signal === "SIGTERM" ? 143 : 1),
45-
errorMessage: null,
46-
})),
4744
}));
4845
vi.mock("../gateway-state", () => ({ ensureLiveSandboxOrExit: ensureLiveMock }));
4946
vi.mock("../../../state/registry", () => ({ getSandbox: getSandboxMock }));

src/lib/core/process-exit.test.ts

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -9,14 +9,16 @@ describe("spawnExitCode", () => {
99
["zero status", { status: 0 }, 0],
1010
["nonzero status", { status: 42 }, 42],
1111
["status before signal", { status: 7, signal: "SIGTERM" }, 7],
12+
["SIGINT", { status: null, signal: "SIGINT" }, 130],
1213
["SIGTERM", { status: null, signal: "SIGTERM" }, 143],
1314
["SIGKILL", { status: null, signal: "SIGKILL" }, 137],
1415
["missing signal", { status: null }, 1],
1516
["null signal", { status: null, signal: null }, 1],
1617
["unknown signal", { status: null, signal: "SIGBOGUS" as NodeJS.Signals }, 1],
17-
] satisfies Array<
18-
[string, Parameters<typeof spawnExitCode>[0], number]
19-
>)("normalizes %s (#5936)", (_label, result, expected) => {
20-
expect(spawnExitCode(result)).toBe(expected);
21-
});
18+
] satisfies Array<[string, Parameters<typeof spawnExitCode>[0], number]>)(
19+
"normalizes %s (#5936)",
20+
(_label, result, expected) => {
21+
expect(spawnExitCode(result)).toBe(expected);
22+
},
23+
);
2224
});

0 commit comments

Comments
 (0)