From bc972f55f727035b7736d11ae84422b8eb06b718 Mon Sep 17 00:00:00 2001 From: Sawyer Cutler Date: Thu, 1 Oct 2026 21:04:17 -0700 Subject: [PATCH 1/2] fix(exec): prompt on stdin TTY even with stdout piped --- docs/ARCHITECTURE.md | 2 +- src/exec/runner.test.ts | 65 ++++++++++++++++++++++++++++++++++++++++- src/exec/runner.ts | 35 ++++++++++++++++++++-- 3 files changed, 97 insertions(+), 5 deletions(-) diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index f43e23846..6f155638f 100644 --- a/docs/ARCHITECTURE.md +++ b/docs/ARCHITECTURE.md @@ -131,7 +131,7 @@ A worker deny-on-ask registers a harness-owned denied-call envelope (`src/permis - Bootstrap consumes the shared session assembly (`src/session/assemble-runtime.ts`, layered over `src/session/runtime-assembly.ts`) instead of hand-wiring a second copy. Intentional deltas vs TUI: - No workflow controller (`isWorkflowActive` is always false) - Non-interactive permission gate by default - - `ask_operator` is unmounted when stdin/stdout are not TTYs (no cancel stub on the wire); TTY exec still prompts on stdin + - `ask_operator` is unmounted when stdin is not a TTY (no cancel stub on the wire); TTY-stdin exec still prompts on stdin even with stdout piped - MCP connect is awaited before workflow resume and first inference, abort-capped at 15s so a hung handshake cannot block the run. A rejected batch does not disarm that abort while sibling dials are still in flight. A 1s log fires if connect is still in progress; remaining dials keep running until settle or abort. The TUI still fire-and-forgets connect (no rewrite). `tool_search` does not treat an empty catalog as a definitive miss while servers are still connecting. - Entry: `corbits exec "prompt"` (alias `corbits run`); `loadConfig` sets `command: "exec"` - Streams assistant text deltas to stdout; lifecycle errors to stderr diff --git a/src/exec/runner.test.ts b/src/exec/runner.test.ts index d68518340..9f02e0b78 100644 --- a/src/exec/runner.test.ts +++ b/src/exec/runner.test.ts @@ -1,4 +1,4 @@ -import { describe, expect, test } from "bun:test"; +import { describe, expect, spyOn, test } from "bun:test"; import type { AgentTool } from "@intx/agent"; import { submitOutputDefinition } from "../agent/director.js"; import { DIRECTOR_REGISTRY } from "../agent/directors/registry.js"; @@ -19,6 +19,7 @@ import { } from "../auth/codex/session.js"; import type { Config } from "../config/index.js"; import { CREDENTIAL_FAILURE_USER_MESSAGE } from "../inference-error-message.js"; +import type { PermissionRequest } from "../permission/types.js"; import { clearActiveRun, getActiveRun, @@ -44,9 +45,11 @@ import { createExecToolPromoter, execUserFailureMessage, isExecOverlayToolAllowed, + promptPermission, refreshSelectedProviderCredential, resolveExecDirectorOverlay, resolveExecDirectorOverlayForPackage, + resolveExecInteractive, runExec, } from "./runner.js"; @@ -815,3 +818,63 @@ describe("exec tool call gate and promoter", () => { ).not.toContain("list_dir"); }); }); + +describe("exec permission prompt gating (CL-9002)", () => { + test("piped stdout with a stdin TTY still prompts", () => { + expect(resolveExecInteractive({ stdinTTY: true, stdoutTTY: false })).toBe( + true, + ); + }); + + test("full TTY prompts", () => { + expect(resolveExecInteractive({ stdinTTY: true, stdoutTTY: true })).toBe( + true, + ); + }); + + test("headless stdin never prompts, whatever stdout is", () => { + expect(resolveExecInteractive({ stdinTTY: false, stdoutTTY: false })).toBe( + false, + ); + expect(resolveExecInteractive({ stdinTTY: false, stdoutTTY: true })).toBe( + false, + ); + expect( + resolveExecInteractive({ stdinTTY: undefined, stdoutTTY: true }), + ).toBe(false); + }); + + test("headless permission seam fails closed with an explicit stderr denial", async () => { + const request: PermissionRequest = { + tool: "run_shell", + action: "Run shell", + subject: "rm -rf /tmp/work", + scopes: [], + }; + const errWrites: string[] = []; + const outWrites: string[] = []; + const stderrSpy = spyOn(process.stderr, "write").mockImplementation((( + chunk: unknown, + ) => { + errWrites.push(String(chunk)); + return true; + }) as typeof process.stderr.write); + const stdoutSpy = spyOn(process.stdout, "write").mockImplementation((( + chunk: unknown, + ) => { + outWrites.push(String(chunk)); + return true; + }) as typeof process.stdout.write); + try { + const outcome = await promptPermission(request, false); + expect(outcome).toEqual({ allow: false }); + } finally { + stderrSpy.mockRestore(); + stdoutSpy.mockRestore(); + } + const denial = errWrites.join(""); + expect(denial).toContain("Run shell"); + expect(denial).toContain("--dangerously-skip-permissions"); + expect(outWrites).toEqual([]); + }); +}); diff --git a/src/exec/runner.ts b/src/exec/runner.ts index d4854dbd7..4e4f5ffea 100644 --- a/src/exec/runner.ts +++ b/src/exec/runner.ts @@ -670,7 +670,10 @@ export async function runExec(config: Config): Promise { resolveSessionMode(config.settings, localSettingsForMode) ?? "orchestrator"; - const interactive = input.isTTY === true && output.isTTY === true; + const interactive = resolveExecInteractive({ + stdinTTY: input.isTTY, + stdoutTTY: output.isTTY, + }); if (config.skipPermissionsFromSettings) { stderr.write( @@ -1407,6 +1410,21 @@ export async function runExec(config: Config): Promise { } } +/** + * Whether exec may prompt the operator. Permission and operator prompts read + * stdin and write stderr, so a piped stdout must not disable them: stdin TTY + * alone means an operator can answer. Fully headless (stdin not a TTY) fails + * closed — denials name the action and the remedy on stderr instead of + * prompting. stdoutTTY rides along so the call site stays explicit that a + * piped stdout is a supported prompting configuration, not an oversight. + */ +export function resolveExecInteractive(stdio: { + stdinTTY: boolean | undefined; + stdoutTTY: boolean | undefined; +}): boolean { + return stdio.stdinTTY === true; +} + async function promptOperator( question: string, options: string[], @@ -1432,11 +1450,22 @@ async function promptOperator( } } -async function promptPermission( +export async function promptPermission( request: PermissionRequest, interactive: boolean, ): Promise { - if (!interactive) return { allow: false }; + if (!interactive) { + // Fail closed and say so on stderr (never stdout: piped stdout may feed + // JSON consumers — see printResumeHint). A headless ask has no operator + // to answer, so the denial names the action and the bypass instead of + // vanishing into a generic block. Same sentence as the gate's + // non-interactive deny reasons (src/permission/gate.ts) by intent; the + // gate stays canonical for model-facing text, this seam for the operator. + stderr.write( + `Permission denied: ${request.action} requires operator approval, which is unavailable in a non-interactive run. Re-run with --dangerously-skip-permissions to bypass, or narrow the action.\n`, + ); + return { allow: false }; + } // Same freeze-while-deciding contract as the TUI gate: the tool wall-clock // budget must not run down while the operator reads the prompt. const budget = getToolApprovalBudget(); From 96e54da72db50a66c8cc4d397f39491e935ea673 Mon Sep 17 00:00:00 2001 From: Sawyer Cutler Date: Thu, 1 Oct 2026 21:22:24 -0700 Subject: [PATCH 2/2] fix(exec): surface headless gate denials on stderr via onHeadlessDeny --- src/exec/runner.test.ts | 114 +++++++++++++++++++++++++++----- src/exec/runner.ts | 29 ++++---- src/permission/gate.ts | 9 +++ src/session/assemble-runtime.ts | 7 ++ 4 files changed, 127 insertions(+), 32 deletions(-) diff --git a/src/exec/runner.test.ts b/src/exec/runner.test.ts index 9f02e0b78..e741ce07b 100644 --- a/src/exec/runner.test.ts +++ b/src/exec/runner.test.ts @@ -1,4 +1,5 @@ import { describe, expect, spyOn, test } from "bun:test"; +import type { ToolCall } from "@intx/types/runtime"; import type { AgentTool } from "@intx/agent"; import { submitOutputDefinition } from "../agent/director.js"; import { DIRECTOR_REGISTRY } from "../agent/directors/registry.js"; @@ -19,7 +20,7 @@ import { } from "../auth/codex/session.js"; import type { Config } from "../config/index.js"; import { CREDENTIAL_FAILURE_USER_MESSAGE } from "../inference-error-message.js"; -import type { PermissionRequest } from "../permission/types.js"; +import { createPermissionGate } from "../permission/gate.js"; import { clearActiveRun, getActiveRun, @@ -45,7 +46,6 @@ import { createExecToolPromoter, execUserFailureMessage, isExecOverlayToolAllowed, - promptPermission, refreshSelectedProviderCredential, resolveExecDirectorOverlay, resolveExecDirectorOverlayForPackage, @@ -844,13 +844,22 @@ describe("exec permission prompt gating (CL-9002)", () => { ).toBe(false); }); - test("headless permission seam fails closed with an explicit stderr denial", async () => { - const request: PermissionRequest = { - tool: "run_shell", - action: "Run shell", - subject: "rm -rf /tmp/work", - scopes: [], - }; + // Exec-identical headless gate wiring: interactive:false with the prompt seam + // armed to throw, so a test passes only when decide() denies on the real + // production path (the old direct-seam test never went through decide()). + const headlessGate = (onDeny: (reason: string) => void) => + createPermissionGate({ + approvals: [], + requestApproval: async () => { + throw new Error("headless run must never reach the prompt seam"); + }, + interactive: false, + onHeadlessDeny: onDeny, + skipPermissions: false, + reactorGated: true, + }); + + const captureStdio = () => { const errWrites: string[] = []; const outWrites: string[] = []; const stderrSpy = spyOn(process.stderr, "write").mockImplementation((( @@ -865,16 +874,89 @@ describe("exec permission prompt gating (CL-9002)", () => { outWrites.push(String(chunk)); return true; }) as typeof process.stdout.write); + return { + errWrites, + outWrites, + restore: () => { + stderrSpy.mockRestore(); + stdoutSpy.mockRestore(); + }, + }; + }; + + test("headless gate deny names the action and remedy on stderr, stdout clean", async () => { + const stdio = captureStdio(); try { - const outcome = await promptPermission(request, false); - expect(outcome).toEqual({ allow: false }); + // Same shape as runner.ts: the denial surfaces on stderr, never stdout. + const gate = headlessGate((reason) => { + process.stderr.write(`Permission denied: ${reason}\n`); + }); + const authorized = await gate.authorizeCall({ + id: "h1", + name: "web_fetch", + arguments: { url: "https://example.com/docs", format: "markdown" }, + } as ToolCall); + expect(authorized.effect).toBe("deny"); + const evaluated = await gate.evaluate({ + id: "h2", + name: "web_fetch", + arguments: { url: "https://example.com/other", format: "markdown" }, + } as ToolCall); + expect(evaluated.allowed).toBe(false); } finally { - stderrSpy.mockRestore(); - stdoutSpy.mockRestore(); + stdio.restore(); } - const denial = errWrites.join(""); - expect(denial).toContain("Run shell"); + const denial = stdio.errWrites.join(""); + expect(denial).toContain("Permission denied:"); + expect(denial).toContain("requires operator approval"); expect(denial).toContain("--dangerously-skip-permissions"); - expect(outWrites).toEqual([]); + expect(stdio.outWrites).toEqual([]); + }); + + test("secret-path headless deny names the action without bypass coaching", async () => { + const stdio = captureStdio(); + try { + const gate = headlessGate((reason) => { + process.stderr.write(`Permission denied: ${reason}\n`); + }); + const verdict = await gate.evaluate({ + id: "s1", + name: "run_shell", + arguments: { command: "cat .env" }, + } as ToolCall); + expect(verdict.allowed).toBe(false); + } finally { + stdio.restore(); + } + const denial = stdio.errWrites.join(""); + expect(denial).toContain("sensitive path"); + expect(denial).not.toContain("--dangerously-skip-permissions"); + expect(stdio.outWrites).toEqual([]); + }); + + test("piped stdout with a stdin TTY keeps the ask advertised and round-trips", async () => { + expect(resolveExecInteractive({ stdinTTY: true, stdoutTTY: false })).toBe( + true, + ); + const seen: string[] = []; + const gate = createPermissionGate({ + approvals: [], + requestApproval: async (request) => { + seen.push(request.subject); + return { allow: true }; + }, + interactive: true, + skipPermissions: false, + reactorGated: true, + }); + const call = { + id: "i1", + name: "web_fetch", + arguments: { url: "https://example.com/docs", format: "markdown" }, + } as ToolCall; + const authorized = await gate.authorizeCall(call); + expect(authorized.effect).toBe("ask"); + expect(await gate.evaluate(call)).toEqual({ allowed: true }); + expect(seen).toEqual(["https://example.com/docs"]); }); }); diff --git a/src/exec/runner.ts b/src/exec/runner.ts index 4e4f5ffea..cd725fc20 100644 --- a/src/exec/runner.ts +++ b/src/exec/runner.ts @@ -688,7 +688,7 @@ export async function runExec(config: Config): Promise { model: config.model, telemetry: liveTelemetry, requestApproval: (request: PermissionRequest): Promise => - promptPermission(request, interactive), + promptPermission(request), getActiveProviderModel: () => `${config.providerName}:${config.model}`, onPersistNotice: (text) => { stderr.write(`${text}\n`); @@ -697,6 +697,14 @@ export async function runExec(config: Config): Promise { stderr.write(`${text}\n`); }, interactive, + // Headless operator surface: the gate denies without an operator before + // ever reaching requestApproval, so the explicit denial lives here — + // naming the action and the remedy on stderr (never stdout: piped stdout + // may feed JSON consumers). The reason already omits the bypass remedy + // on sensitive-path denials; emit it verbatim. + onHeadlessDeny: (reason) => { + stderr.write(`Permission denied: ${reason}\n`); + }, skipPermissions: config.dangerouslySkipPermissions, auto: config.auto, // Main session: gating rides the reactor's approval-suspend seam. In @@ -1415,7 +1423,9 @@ export async function runExec(config: Config): Promise { * stdin and write stderr, so a piped stdout must not disable them: stdin TTY * alone means an operator can answer. Fully headless (stdin not a TTY) fails * closed — denials name the action and the remedy on stderr instead of - * prompting. stdoutTTY rides along so the call site stays explicit that a + * prompting (gate onHeadlessDeny wiring above, not the prompt seam: the gate + * denies headless asks before requestApproval is ever reached). stdoutTTY + * rides along so the call site stays explicit that a * piped stdout is a supported prompting configuration, not an oversight. */ export function resolveExecInteractive(stdio: { @@ -1450,22 +1460,9 @@ async function promptOperator( } } -export async function promptPermission( +async function promptPermission( request: PermissionRequest, - interactive: boolean, ): Promise { - if (!interactive) { - // Fail closed and say so on stderr (never stdout: piped stdout may feed - // JSON consumers — see printResumeHint). A headless ask has no operator - // to answer, so the denial names the action and the bypass instead of - // vanishing into a generic block. Same sentence as the gate's - // non-interactive deny reasons (src/permission/gate.ts) by intent; the - // gate stays canonical for model-facing text, this seam for the operator. - stderr.write( - `Permission denied: ${request.action} requires operator approval, which is unavailable in a non-interactive run. Re-run with --dangerously-skip-permissions to bypass, or narrow the action.\n`, - ); - return { allow: false }; - } // Same freeze-while-deciding contract as the TUI gate: the tool wall-clock // budget must not run down while the operator reads the prompt. const budget = getToolApprovalBudget(); diff --git a/src/permission/gate.ts b/src/permission/gate.ts index 019539492..a95a30846 100644 --- a/src/permission/gate.ts +++ b/src/permission/gate.ts @@ -395,6 +395,13 @@ export interface PermissionGateOptions { // No operator is attached (headless). An unresolved "ask" becomes a denial // unless skipPermissions is set. interactive: boolean; + // Headless operator surface: fired with the deny reason when decide() denies + // because no operator is attached (the !interactive exits above). Exec wires + // it to stderr so the denial names the action and the remedy; gates without + // an operator surface omit it and stay silent by construction. The reason + // already carries the secret-path distinction (no bypass remedy on + // sensitive-path denials), so callers emit it verbatim. + onHeadlessDeny?: ((reason: string) => void) | undefined; // The --dangerously-skip-permissions escape hatch: auto-allow anything the // authorization layer did not already deny. skipPermissions: boolean; @@ -1023,6 +1030,7 @@ export function createPermissionGate( ? `${request.action} references a sensitive path and requires operator approval, which is unavailable in a non-interactive run.` : `${request.action} requires operator approval, which is unavailable in a non-interactive run. Re-run with --dangerously-skip-permissions to bypass, or narrow the action.`; denialMemory.record(stableId, reason); + options.onHeadlessDeny?.(reason); return { kind: "deny", reason }; } @@ -1067,6 +1075,7 @@ export function createPermissionGate( recordAutoDecision(request.tool, "non-interactive", "deny"); const reason = `${request.action} requires operator approval, which is unavailable in a non-interactive run. Re-run with --dangerously-skip-permissions to bypass, or narrow the action.`; denialMemory.record(stableId, reason); + options.onHeadlessDeny?.(reason); return { kind: "deny", reason }; } diff --git a/src/session/assemble-runtime.ts b/src/session/assemble-runtime.ts index 8e7960c5c..df4c9ac1d 100644 --- a/src/session/assemble-runtime.ts +++ b/src/session/assemble-runtime.ts @@ -265,6 +265,12 @@ export interface SessionGateArgs { */ onPendingProjectGrants?: ((text: string) => void) | undefined; interactive: boolean; + /** + * Headless operator surface (see PermissionGateOptions.onHeadlessDeny): + * fired with the deny reason when the gate denies for want of an operator. + * Exec writes it to stderr; the TUI omits it. + */ + onHeadlessDeny?: ((reason: string) => void) | undefined; skipPermissions: boolean; auto?: boolean | undefined; /** Route this gate's decisions through the reactor authz seam (main session). */ @@ -308,6 +314,7 @@ export async function assembleSessionGate( ), approvalLog: createApprovalLog(sessionDir(args.cwd, args.sessionId)), interactive: args.interactive, + onHeadlessDeny: args.onHeadlessDeny, skipPermissions: args.skipPermissions, auto: args.auto, reactorGated: args.reactorGated,