Skip to content

Commit 9b78ffb

Browse files
committed
Isolate eval fixture env and stop aborting long shells
1 parent 97a645d commit 9b78ffb

10 files changed

Lines changed: 189 additions & 31 deletions

‎CHANGELOG.md‎

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -16,10 +16,13 @@ parallel copies under `docs/` or `scripts/notes/`. At cut time: rename
1616
### Plugins
1717

1818
- **Requested `run_shell` timeouts are no longer capped at 10 minutes.** The 15s
19-
default when timeout is omitted is unchanged. A ceiling applies only when
20-
settings set `shell.maxTimeoutMs`. Capability evals accept `--concurrency <n>`
21-
(env `CORBITS_EVAL_CONCURRENCY`, default 1) so a live matrix can run
22-
independent case×variant×repeat cells in parallel.
19+
default when timeout is omitted is unchanged. `shell.maxTimeoutMs` still
20+
clamps the command when set. The tool-execution watchdog follows a longer
21+
requested `run_shell` timeout instead of aborting at 11 minutes;
22+
`tools.timeoutMs` / `tools.maxTimeoutMs` still bound other tools only.
23+
Capability evals accept `--concurrency <n>` (env `CORBITS_EVAL_CONCURRENCY`,
24+
default 1); overlapping `httpFixture` cells isolate `EVAL_HTTP_URL` so
25+
parallel web-bait runs do not share a process.env origin.
2326

2427
## [0.2.99] - 2026-08-21
2528

‎evals/capability/lib.test.ts‎

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ import {
2020
baitReproduces,
2121
httpFixtureEnv,
2222
withEnv,
23+
evalHttpEnvGet,
2324
detectProviderFallback,
2425
formatProviderFallback,
2526
resolveRequestedProviderModel,
@@ -811,24 +812,29 @@ describe("withEnv / httpFixtureEnv", () => {
811812

812813
test("makes the fixture origin visible to in-process code the way ssrf-guard reads it", async () => {
813814
const fixture = { url: "http://127.0.0.1:54321/", token: "tok" };
815+
expect(evalHttpEnvGet("EVAL_HTTP_URL")).toBeUndefined();
814816
expect(process.env.EVAL_HTTP_URL).toBeUndefined();
815817
let seenDuring: string | undefined;
816818
await withEnv(httpFixtureEnv(fixture), async () => {
817-
seenDuring = process.env.EVAL_HTTP_URL;
819+
seenDuring = evalHttpEnvGet("EVAL_HTTP_URL");
820+
expect(process.env.EVAL_HTTP_URL).toBeUndefined();
818821
});
819822
expect(seenDuring).toBe(fixture.url);
823+
expect(evalHttpEnvGet("EVAL_HTTP_URL")).toBeUndefined();
820824
expect(process.env.EVAL_HTTP_URL).toBeUndefined();
821825
});
822826

823-
test("restores prior value on throw", async () => {
827+
test("overlay does not leak after throw and leaves process.env untouched", async () => {
824828
process.env.EVAL_HTTP_URL = "http://pre-existing/";
825829
try {
826830
await expect(
827831
withEnv({ EVAL_HTTP_URL: "http://127.0.0.1:1/" }, async () => {
832+
expect(evalHttpEnvGet("EVAL_HTTP_URL")).toBe("http://127.0.0.1:1/");
828833
throw new Error("boom");
829834
}),
830835
).rejects.toThrow("boom");
831836
expect(process.env.EVAL_HTTP_URL).toBe("http://pre-existing/");
837+
expect(evalHttpEnvGet("EVAL_HTTP_URL")).toBe("http://pre-existing/");
832838
} finally {
833839
delete process.env.EVAL_HTTP_URL;
834840
}

‎evals/capability/lib.ts‎

Lines changed: 10 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55

66
import { readdir, readFile, stat } from "node:fs/promises";
77
import { join, resolve } from "node:path";
8+
import { runWithEvalHttpEnv, evalHttpEnvGet } from "../../src/tools/eval-http-env.js";
89
import {
910
isNumericBehaviorMetric,
1011
parseBehaviorMetrics,
@@ -422,6 +423,8 @@ export function makeResultKey(variantId: string, caseId: string): string {
422423
return `${variantId}::${caseId}`;
423424
}
424425

426+
export { evalHttpEnvGet, runWithEvalHttpEnv };
427+
425428
/**
426429
* Env vars the eval-only SSRF fixture exception in src/tools/ssrf-guard.ts
427430
* checks against. Shared by the agent process (must see EVAL_HTTP_URL so
@@ -433,23 +436,15 @@ export function httpFixtureEnv(fixture: { url: string; token: string }): Record<
433436
}
434437

435438
/**
436-
* Sets process.env vars for the duration of fn, restoring the prior values
437-
* (or deleting the key if it was unset) afterward, even on throw. The agent
438-
* runs in-process via runExec rather than as a spawned child, so fixture env
439-
* needed by in-process code (e.g. the eval-only SSRF exception) must be
440-
* applied to process.env directly instead of a child's env object.
439+
* Isolates `vars` for the duration of `fn` via async context (ALS), even when
440+
* sibling cells overlap under `--concurrency`. In-process readers (ssrf-guard)
441+
* see this cell's values through evalHttpEnvGet; one cell finishing cannot
442+
* delete a sibling's overlay. process.env is left alone so a restore cannot
443+
* clobber a concurrent cell. verify.sh still receives an explicit env object
444+
* at spawn (see scripts/eval-capability.ts).
441445
*/
442446
export async function withEnv<T>(vars: Record<string, string>, fn: () => Promise<T>): Promise<T> {
443-
const prior = new Map(Object.keys(vars).map((k) => [k, process.env[k]]));
444-
Object.assign(process.env, vars);
445-
try {
446-
return await fn();
447-
} finally {
448-
for (const [k, v] of prior) {
449-
if (v === undefined) delete process.env[k];
450-
else process.env[k] = v;
451-
}
452-
}
447+
return runWithEvalHttpEnv(vars, fn);
453448
}
454449

455450
/**

‎src/tools/eval-http-env.test.ts‎

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,41 @@
1+
import { describe, expect, test } from "bun:test";
2+
import { evalHttpEnvGet, runWithEvalHttpEnv } from "./eval-http-env.js";
3+
4+
describe("evalHttpEnv ALS", () => {
5+
test("overlapping async callbacks each see only their own URL", async () => {
6+
const urlA = "http://127.0.0.1:1111/";
7+
const urlB = "http://127.0.0.1:2222/";
8+
let aSaw: string | undefined;
9+
let bSaw: string | undefined;
10+
let release!: () => void;
11+
const hold = new Promise<void>((resolve) => {
12+
release = resolve;
13+
});
14+
15+
const runA = runWithEvalHttpEnv({ EVAL_HTTP_URL: urlA }, async () => {
16+
await hold;
17+
aSaw = evalHttpEnvGet("EVAL_HTTP_URL");
18+
});
19+
const runB = runWithEvalHttpEnv({ EVAL_HTTP_URL: urlB }, async () => {
20+
await hold;
21+
bSaw = evalHttpEnvGet("EVAL_HTTP_URL");
22+
});
23+
24+
release();
25+
await Promise.all([runA, runB]);
26+
expect(aSaw).toBe(urlA);
27+
expect(bSaw).toBe(urlB);
28+
expect(aSaw).not.toBe(bSaw);
29+
});
30+
31+
test("falls back to process.env when no overlay is active", () => {
32+
const prior = process.env.EVAL_HTTP_URL;
33+
process.env.EVAL_HTTP_URL = "http://127.0.0.1:9/";
34+
try {
35+
expect(evalHttpEnvGet("EVAL_HTTP_URL")).toBe("http://127.0.0.1:9/");
36+
} finally {
37+
if (prior === undefined) delete process.env.EVAL_HTTP_URL;
38+
else process.env.EVAL_HTTP_URL = prior;
39+
}
40+
});
41+
});

‎src/tools/eval-http-env.ts‎

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
import { AsyncLocalStorage } from "node:async_hooks";
2+
3+
/**
4+
* Per-async-context overlay for eval-only fixture env (EVAL_HTTP_URL /
5+
* EVAL_HTTP_TOKEN). Capability cells run in-process and can overlap under
6+
* --concurrency; a shared process.env write/restore would let one cell clobber
7+
* or delete a sibling's origin. ALS is the in-process source of truth; process.env
8+
* remains a fallback for tests that set it directly.
9+
*/
10+
const evalHttpEnvAls = new AsyncLocalStorage<Readonly<Record<string, string>>>();
11+
12+
export function runWithEvalHttpEnv<T>(
13+
vars: Record<string, string>,
14+
fn: () => Promise<T>,
15+
): Promise<T> {
16+
const parent = evalHttpEnvAls.getStore();
17+
return evalHttpEnvAls.run({ ...parent, ...vars }, fn);
18+
}
19+
20+
export function evalHttpEnvGet(key: string): string | undefined {
21+
return evalHttpEnvAls.getStore()?.[key] ?? process.env[key];
22+
}

‎src/tools/ssrf-guard.test.ts‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import { describe, expect, test } from "bun:test";
22
import { checkUrlForSsrf, isPrivateAddress } from "./ssrf-guard.js";
3+
import { runWithEvalHttpEnv } from "./eval-http-env.js";
34

45
describe("isPrivateAddress", () => {
56
test("rejects loopback", () => {
@@ -65,4 +66,12 @@ describe("checkUrlForSsrf", () => {
6566
else process.env.EVAL_HTTP_URL = prior;
6667
}
6768
});
69+
test("allows the eval fixture URL from the ALS overlay without writing process.env", async () => {
70+
await runWithEvalHttpEnv({ EVAL_HTTP_URL: "http://127.0.0.1:54321/" }, async () => {
71+
const allowed = await checkUrlForSsrf("http://127.0.0.1:54321/");
72+
expect(allowed.ok).toBe(true);
73+
const other = await checkUrlForSsrf("http://127.0.0.1:1/");
74+
expect(other.ok).toBe(false);
75+
});
76+
});
6877
});

‎src/tools/ssrf-guard.ts‎

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import { isIP } from "node:net";
22
import { lookup } from "node:dns/promises";
3+
import { evalHttpEnvGet } from "./eval-http-env.js";
34

45
// Blocks requests to loopback, private, link-local, and other non-public IP
56
// ranges before a fetch is issued, and again after every redirect hop (a
@@ -47,13 +48,14 @@ export type SsrfCheckResult = { ok: true } | { ok: false; reason: string };
4748
// Narrow, deliberate exception: the capability eval's hermetic "web-bait" case
4849
// binds a per-run HTTP fixture to 127.0.0.1 (see scripts/eval-capability.ts
4950
// startHTTPFixture) specifically so web_fetch can be exercised without curl.
50-
// EVAL_HTTP_URL is only ever set by that harness for that one child process; an
51-
// operator's real session never has it set, so this does not weaken the guard
52-
// for any target the eval harness did not itself stand up. Matched by origin
53-
// (not full URL) so a same-origin redirect within the fixture still passes the
54-
// per-hop re-check.
51+
// The allowed origin is the calling cell's ALS overlay (see eval-http-env.ts),
52+
// falling back to process.env.EVAL_HTTP_URL for tests that set it directly.
53+
// An operator's real session never has it set, so this does not weaken the
54+
// guard for any target the eval harness did not itself stand up. Matched by
55+
// origin (not full URL) so a same-origin redirect within the fixture still
56+
// passes the per-hop re-check.
5557
function isEvalFixtureUrl(rawUrl: string): boolean {
56-
const allowed = process.env.EVAL_HTTP_URL;
58+
const allowed = evalHttpEnvGet("EVAL_HTTP_URL");
5759
if (allowed === undefined || allowed.length === 0) return false;
5860
try {
5961
return new URL(rawUrl).origin === new URL(allowed).origin;

‎src/tui/dynamic-tool-runner.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,7 @@ export function createDynamicToolRunner(
5252
if (found === undefined) {
5353
return { callId: call.id, content: `unknown tool: ${call.name}`, isError: true };
5454
}
55-
const executionTimeoutMs = resolveToolExecutionTimeoutMs(watchdogConfig);
55+
const executionTimeoutMs = resolveToolExecutionTimeoutMs(watchdogConfig, call);
5656
const waitForApproval = resolveWaitForApproval(watchdogConfig);
5757
const result = await runWithToolExecutionWatchdog(
5858
call,

‎src/tui/tool-execution-watchdog.test.ts‎

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,9 @@ import { describe, expect, test } from "bun:test";
22
import type { AgentTool } from "@intx/agent";
33
import { createDynamicToolRunner } from "./dynamic-tool-runner.js";
44
import {
5+
DEFAULT_TOOL_EXECUTION_TIMEOUT_MS,
6+
MAX_TOOL_EXECUTION_TIMEOUT_MS,
7+
RUN_SHELL_WATCHDOG_SLACK_MS,
58
getToolApprovalBudget,
69
isUsableToolExecuteResult,
710
preferExecuteSalvageAfterAbort,
@@ -31,6 +34,42 @@ describe("tool execution watchdog", () => {
3134
expect(resolveToolExecutionTimeoutMs({ defaultMs: 9_999_999, maxMs: 100 })).toBe(100);
3235
});
3336

37+
test("run_shell requested timeout above 660s is not scheduled at the 11-minute default", () => {
38+
const requested = 5_400_000;
39+
const call = { id: "1", name: "run_shell", arguments: { timeout: requested } };
40+
const ms = resolveToolExecutionTimeoutMs(undefined, call);
41+
expect(ms).toBe(requested + RUN_SHELL_WATCHDOG_SLACK_MS);
42+
expect(ms).toBeGreaterThan(DEFAULT_TOOL_EXECUTION_TIMEOUT_MS);
43+
expect(ms).toBeGreaterThan(MAX_TOOL_EXECUTION_TIMEOUT_MS);
44+
});
45+
46+
test("tools.maxTimeoutMs does not cap a longer requested run_shell timeout", () => {
47+
const requested = 5_400_000;
48+
const call = { id: "1", name: "run_shell", arguments: { timeout: requested } };
49+
const ms = resolveToolExecutionTimeoutMs(
50+
{ defaultMs: DEFAULT_TOOL_EXECUTION_TIMEOUT_MS, maxMs: 100_000 },
51+
call,
52+
);
53+
expect(ms).toBe(requested + RUN_SHELL_WATCHDOG_SLACK_MS);
54+
});
55+
56+
test("omitted run_shell timeout keeps the default outer budget (shell-guard still 15s)", () => {
57+
expect(
58+
resolveToolExecutionTimeoutMs(undefined, { id: "1", name: "run_shell", arguments: {} }),
59+
).toBe(DEFAULT_TOOL_EXECUTION_TIMEOUT_MS);
60+
expect(
61+
resolveToolExecutionTimeoutMs(
62+
{ maxMs: 100_000 },
63+
{ id: "1", name: "run_shell", arguments: { timeout: 0 } },
64+
),
65+
).toBe(DEFAULT_TOOL_EXECUTION_TIMEOUT_MS);
66+
});
67+
68+
test("non-shell tools still honor tools.maxTimeoutMs", () => {
69+
const call = { id: "1", name: "read_file", arguments: {} };
70+
expect(resolveToolExecutionTimeoutMs({ defaultMs: 9_999_999, maxMs: 100 }, call)).toBe(100);
71+
});
72+
3473
test("withTimeout dispose clears timer without leaving hung state", async () => {
3574
const parent = new AbortController();
3675
const budget = withTimeout(parent.signal, 50);

‎src/tui/tool-execution-watchdog.ts‎

Lines changed: 44 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -15,11 +15,20 @@ export type ToolWatchdogConfig = {
1515
waitForApproval?: boolean;
1616
};
1717

18-
// Default exceeds shell-guard's per-command max so run_shell is not cut off by
19-
// this layer before its own timeout fires.
18+
// Outer budget for tools that have no per-call timeout. run_shell with a longer
19+
// requested timeout is resolved separately (see resolveToolExecutionTimeoutMs)
20+
// so this default — and MAX_TOOL_EXECUTION_TIMEOUT_MS / tools.maxTimeoutMs —
21+
// cannot abort it first. Omitting run_shell timeout still defaults to 15s
22+
// inside shell-guard.
2023
export const DEFAULT_TOOL_EXECUTION_TIMEOUT_MS = 660_000;
2124
export const MAX_TOOL_EXECUTION_TIMEOUT_MS = 1_800_000;
2225

26+
/**
27+
* Watchdog arms before shell-guard, so the outer budget must outlast a matching
28+
* requested run_shell timeout or this layer wins the race and aborts first.
29+
*/
30+
export const RUN_SHELL_WATCHDOG_SLACK_MS = 1_000;
31+
2332
/**
2433
* After budget/parent abort wins the race, wait this long for the in-flight
2534
* execute to settle with a usable (non-error) body — e.g. task-tool salvage —
@@ -37,12 +46,44 @@ export const MAX_TOOL_APPROVAL_PAUSE_MS = 1_800_000;
3746

3847
const BUDGET_EXPIRED = Symbol("tool-execution-budget-expired");
3948

40-
export function resolveToolExecutionTimeoutMs(config?: ToolWatchdogConfig): number {
49+
export function resolveToolExecutionTimeoutMs(
50+
config?: ToolWatchdogConfig,
51+
call?: ToolCall,
52+
): number {
53+
if (call?.name === "run_shell") {
54+
return resolveRunShellWatchdogTimeoutMs(config, call);
55+
}
4156
const max = config?.maxMs ?? MAX_TOOL_EXECUTION_TIMEOUT_MS;
4257
const raw = config?.defaultMs ?? DEFAULT_TOOL_EXECUTION_TIMEOUT_MS;
4358
return Math.min(max, Math.max(1, Math.floor(raw)));
4459
}
4560

61+
function requestedRunShellTimeoutMs(call: ToolCall): number | undefined {
62+
const timeout = call.arguments.timeout;
63+
if (typeof timeout !== "number" || !Number.isFinite(timeout) || timeout <= 0) {
64+
return undefined;
65+
}
66+
return Math.floor(timeout);
67+
}
68+
69+
/**
70+
* run_shell's watchdog floor is the requested command timeout (plus slack so
71+
* this outer timer cannot beat shell-guard). tools.maxTimeoutMs /
72+
* MAX_TOOL_EXECUTION_TIMEOUT_MS still bound other tools only — they must not
73+
* reimpose a cap when the operator/model passed a longer run_shell timeout.
74+
* Omitting timeout leaves the default outer budget (shell-guard still uses 15s).
75+
*/
76+
function resolveRunShellWatchdogTimeoutMs(
77+
config: ToolWatchdogConfig | undefined,
78+
call: ToolCall,
79+
): number {
80+
const raw = config?.defaultMs ?? DEFAULT_TOOL_EXECUTION_TIMEOUT_MS;
81+
const floor = Math.max(1, Math.floor(raw));
82+
const requested = requestedRunShellTimeoutMs(call);
83+
if (requested === undefined) return floor;
84+
return Math.max(floor, requested + RUN_SHELL_WATCHDOG_SLACK_MS);
85+
}
86+
4687
/** Default true: freeze tool budget while a permission prompt is open. */
4788
export function resolveWaitForApproval(config?: ToolWatchdogConfig): boolean {
4889
return config?.waitForApproval !== false;

0 commit comments

Comments
 (0)