Skip to content

Commit c2a26d3

Browse files
committed
Refuse queued run_shell after shell-guard dispose
Latch dispose so overlapping calls join one reap, and refuse queued shells that would spawn after the guard is gone.
1 parent 75df5a7 commit c2a26d3

2 files changed

Lines changed: 124 additions & 3 deletions

File tree

src/plugins/shell-guard-plugin.test.ts

Lines changed: 100 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -782,6 +782,106 @@ describe("shellGuardPlugin", () => {
782782
}
783783
});
784784

785+
test("dispose refuses a queued run_shell so it cannot stay running after reap", async () => {
786+
if (process.platform === "win32") return;
787+
const plugin = shellGuardPlugin(process.cwd());
788+
const handler = plugin.middleware!(fallback);
789+
const token1 = `ic_guard_queued1_${randomUUID()}`;
790+
const token2 = `ic_guard_queued2_${randomUUID()}`;
791+
const first = handler(
792+
{
793+
id: "q-live",
794+
name: "run_shell",
795+
arguments: { command: `IC_GUARD_TAG=${token1} sleep 600` },
796+
},
797+
neverAbort(),
798+
);
799+
try {
800+
const started = Date.now();
801+
while (Date.now() - started < 5_000) {
802+
const probe = spawnSync("pgrep", ["-f", token1], { encoding: "utf8" });
803+
if ((probe.stdout?.trim() ?? "").length > 0) break;
804+
await new Promise((r) => setTimeout(r, 50));
805+
}
806+
expect(
807+
spawnSync("pgrep", ["-f", token1], { encoding: "utf8" }).stdout?.trim() ?? "",
808+
).not.toBe("");
809+
810+
const queued = handler(
811+
{
812+
id: "q-wait",
813+
name: "run_shell",
814+
arguments: { command: `IC_GUARD_TAG=${token2} sleep 600` },
815+
},
816+
neverAbort(),
817+
);
818+
819+
let disposeError: unknown;
820+
try {
821+
await plugin.dispose!();
822+
} catch (err) {
823+
disposeError = err;
824+
}
825+
826+
await first;
827+
await Promise.race([queued, new Promise((r) => setTimeout(r, 400))]);
828+
await new Promise((r) => setTimeout(r, 200));
829+
830+
const leftover1 =
831+
spawnSync("pgrep", ["-f", token1], { encoding: "utf8" }).stdout?.trim() ?? "";
832+
const leftover2 =
833+
spawnSync("pgrep", ["-f", token2], { encoding: "utf8" }).stdout?.trim() ?? "";
834+
expect(leftover1).toBe("");
835+
expect(leftover2).toBe("");
836+
if (leftover2.length > 0) {
837+
expect(disposeError).toBeDefined();
838+
} else {
839+
const queuedResult = await queued;
840+
expect(queuedResult.isError).toBe(true);
841+
expect(String(queuedResult.content)).toMatch(/disposed/);
842+
expect(disposeError).toBeUndefined();
843+
}
844+
spawnSync("pkill", ["-9", "-f", token2]);
845+
await Promise.race([queued, new Promise((r) => setTimeout(r, 1_000))]);
846+
} finally {
847+
spawnSync("pkill", ["-9", "-f", token1]);
848+
spawnSync("pkill", ["-9", "-f", token2]);
849+
}
850+
}, 15_000);
851+
852+
test("overlapping dispose joins the in-flight reap", async () => {
853+
if (process.platform === "win32") return;
854+
const plugin = shellGuardPlugin(process.cwd());
855+
const handler = plugin.middleware!(fallback);
856+
const token = `ic_guard_join_${randomUUID()}`;
857+
const running = handler(
858+
{
859+
id: "join-live",
860+
name: "run_shell",
861+
arguments: { command: `IC_GUARD_TAG=${token} sleep 600` },
862+
},
863+
neverAbort(),
864+
);
865+
try {
866+
const started = Date.now();
867+
while (Date.now() - started < 5_000) {
868+
const probe = spawnSync("pgrep", ["-f", token], { encoding: "utf8" });
869+
if ((probe.stdout?.trim() ?? "").length > 0) break;
870+
await new Promise((r) => setTimeout(r, 50));
871+
}
872+
expect(plugin.dispose).toBeDefined();
873+
const first = plugin.dispose!();
874+
const second = plugin.dispose!();
875+
expect(second).toBe(first);
876+
await Promise.all([first, second]);
877+
await running;
878+
const after = spawnSync("pgrep", ["-f", token], { encoding: "utf8" });
879+
expect(after.stdout?.trim() ?? "").toBe("");
880+
} finally {
881+
spawnSync("pkill", ["-9", "-f", token]);
882+
}
883+
});
884+
785885
test("dispose fails when a child survives the reap window", async () => {
786886
const child = Object.assign(new EventEmitter(), {
787887
exitCode: null,

src/plugins/shell-guard-plugin.ts

Lines changed: 24 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -250,8 +250,12 @@ export async function runGuardedShell(
250250
args: RunShellArgs,
251251
signal: AbortSignal,
252252
liveChildren?: Set<ChildProcess>,
253+
isDisposed?: () => boolean,
253254
): Promise<GuardedShellResult> {
254255
signal.throwIfAborted();
256+
if (isDisposed?.()) {
257+
throw new Error("run_shell refused: shell guard disposed");
258+
}
255259

256260
// Arm setTimeout only when a positive timeout was resolved. No built-in default.
257261
const timeoutMs = args.timeout !== undefined && args.timeout > 0 ? args.timeout : undefined;
@@ -395,12 +399,19 @@ export function shellGuardPlugin(
395399
const sessionRoot = realpathSync(cwd);
396400
let retainedShellCwd = sessionRoot;
397401
const liveChildren = new Set<ChildProcess>();
402+
let disposed = false;
398403
let disposal: Promise<void> | undefined;
399404
// Serialize run_shell so concurrent tools cannot race retained cwd updates
400405
// (last-writer-wins or a non-cd call finishing after a cd and resetting cwd).
401406
let shellChain: Promise<unknown> = Promise.resolve();
402407
const enqueueShell = <T>(fn: () => Promise<T>): Promise<T> => {
403-
const run = shellChain.then(fn, fn);
408+
const runUnlessDisposed = (): Promise<T> => {
409+
if (disposed) {
410+
return Promise.reject(new Error("run_shell refused: shell guard disposed"));
411+
}
412+
return fn();
413+
};
414+
const run = shellChain.then(runUnlessDisposed, runUnlessDisposed);
404415
shellChain = run.then(
405416
() => undefined,
406417
() => undefined,
@@ -492,6 +503,7 @@ export function shellGuardPlugin(
492503
},
493504
signal,
494505
liveChildren,
506+
() => disposed,
495507
);
496508
const parsed = parsePwdProbeOutput(output);
497509
if (perCallCwdRaw === undefined && parsed.finalCwd !== undefined) {
@@ -523,7 +535,11 @@ export function shellGuardPlugin(
523535
isError: true,
524536
};
525537
}
526-
});
538+
}).catch((err: unknown) => ({
539+
callId: call.id,
540+
content: err instanceof Error ? err.message : String(err),
541+
isError: true,
542+
}));
527543
}
528544

529545
if (SEARCH_TOOLS.has(call.name)) {
@@ -579,7 +595,12 @@ export function shellGuardPlugin(
579595
},
580596
dispose: () => {
581597
if (disposal !== undefined) return disposal;
582-
disposal = reapLiveChildren(liveChildren);
598+
disposed = true;
599+
disposal = (async () => {
600+
await reapLiveChildren(liveChildren);
601+
await shellChain;
602+
await reapLiveChildren(liveChildren);
603+
})();
583604
return disposal;
584605
},
585606
};

0 commit comments

Comments
 (0)