From 4c378812ce69375c9961aa69d1461251f19dddb2 Mon Sep 17 00:00:00 2001 From: emersonbusson Date: Sat, 16 May 2026 18:06:54 -0300 Subject: [PATCH 1/2] feat(core): non-interactive env and PTY skip for Full Access shell exec MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two related fixes for Full Access mode (--full-access / --approval-mode= full_access) so shell commands don't hang on interactive sub-prompts. 1. Inject non-interactive env vars in shellExecutionService.ts so npm, npx, apt, pip, yarn, and git auto-confirm instead of waiting at "Ok to proceed? [y]". Pre-existing user values pass through via `?? 'default'` coalesce. Vars injected when approvalMode === YOLO: CI=1 npm_config_yes=true npm_config_fund=false npm_config_audit=false YARN_ENABLE_INTERACTIVE=false DEBIAN_FRONTEND=noninteractive NEEDRESTART_MODE=a GIT_TERMINAL_PROMPT=0 GH_PROMPT_DISABLED=1 GCM_INTERACTIVE=never PIP_DISABLE_PIP_VERSION_CHECK=1 2. Skip the PTY in Full Access (shell.ts passes shouldUseNodePty=false when approvalMode===YOLO). The child_process fallback uses stdio:['ignore','pipe','pipe'], so sudo reads EOF on stdin and exits with "a password is required" within ~1s instead of hanging on the TTY password prompt. Trade-off: TUI tools (vim, htop) no longer work inside Full Access. That matches the intent — Full Access is fire-and-forget execution; interactive TUI sessions don't belong in it. Mirrors prior art: Codex CLI's UNIFIED_EXEC_ENV block at codex-rs/ core/src/unified_exec/process_manager.rs:60-71 (always-on env block) and command.stdin(Stdio::null()) at codex-rs/utils/pty/src/pipe.rs:144 (stdin closure for fail-fast on sudo). Claude Code propagates a similar env allow-list from the user's shell. This PR is the hybrid: opinionated like Codex (auto-injects) but gated like a user opt-in (YOLO only). Non-YOLO flows are unchanged. Tests: - shellExecutionService.test.ts: YOLO injects env / DEFAULT does not / pre-existing user CI=0 and DEBIAN_FRONTEND=dialog preserved - shell.test.ts: PTY enabled with interactive + non-YOLO / PTY skipped in YOLO / PTY skipped when interactiveShell disabled --- .../services/shellExecutionService.test.ts | 117 ++++++++++++++++++ .../src/services/shellExecutionService.ts | 30 +++++ packages/core/src/tools/shell.test.ts | 53 ++++++++ packages/core/src/tools/shell.ts | 9 +- 4 files changed, 208 insertions(+), 1 deletion(-) diff --git a/packages/core/src/services/shellExecutionService.test.ts b/packages/core/src/services/shellExecutionService.test.ts index f1aa08f41f2..ed178c89b61 100644 --- a/packages/core/src/services/shellExecutionService.test.ts +++ b/packages/core/src/services/shellExecutionService.test.ts @@ -2192,4 +2192,121 @@ describe('ShellExecutionService environment variables', () => { vi.unstubAllEnvs(); }); + + it('should inject non-interactive env vars in Full Access (YOLO) mode', async () => { + vi.resetModules(); + vi.stubEnv('CI', undefined); + vi.stubEnv('npm_config_yes', undefined); + vi.stubEnv('DEBIAN_FRONTEND', undefined); + + const { ShellExecutionService } = await import( + './shellExecutionService.js' + ); + const { ApprovalMode } = await import('../policy/types.js'); + + mockGetPty.mockResolvedValue(null); // child_process fallback + await ShellExecutionService.execute( + 'test-cp-yolo-env', + '/', + vi.fn(), + new AbortController().signal, + true, // interactive UI session — YOLO is the new gate + { ...shellExecutionConfig, approvalMode: ApprovalMode.YOLO }, + ); + + expect(mockCpSpawn).toHaveBeenCalled(); + const cpEnv = mockCpSpawn.mock.calls[0][2].env; + expect(cpEnv).toHaveProperty('CI', '1'); + expect(cpEnv).toHaveProperty('npm_config_yes', 'true'); + expect(cpEnv).toHaveProperty('npm_config_fund', 'false'); + expect(cpEnv).toHaveProperty('npm_config_audit', 'false'); + expect(cpEnv).toHaveProperty('YARN_ENABLE_INTERACTIVE', 'false'); + expect(cpEnv).toHaveProperty('DEBIAN_FRONTEND', 'noninteractive'); + expect(cpEnv).toHaveProperty('NEEDRESTART_MODE', 'a'); + expect(cpEnv).toHaveProperty('GIT_TERMINAL_PROMPT', '0'); + expect(cpEnv).toHaveProperty('GH_PROMPT_DISABLED', '1'); + expect(cpEnv).toHaveProperty('GCM_INTERACTIVE', 'never'); + expect(cpEnv).toHaveProperty('PIP_DISABLE_PIP_VERSION_CHECK', '1'); + + mockChildProcess.emit('exit', 0, null); + mockChildProcess.emit('close', 0, null); + await new Promise(process.nextTick); + vi.unstubAllEnvs(); + }); + + it('should NOT inject Full Access env vars when approvalMode is DEFAULT', async () => { + vi.resetModules(); + vi.stubEnv('CI', undefined); + vi.stubEnv('npm_config_yes', undefined); + vi.stubEnv('DEBIAN_FRONTEND', undefined); + + const { ShellExecutionService } = await import( + './shellExecutionService.js' + ); + const { ApprovalMode } = await import('../policy/types.js'); + + mockGetPty.mockResolvedValue(null); + await ShellExecutionService.execute( + 'test-cp-default-env', + '/', + vi.fn(), + new AbortController().signal, + true, // interactive + { ...shellExecutionConfig, approvalMode: ApprovalMode.DEFAULT }, + ); + + expect(mockCpSpawn).toHaveBeenCalled(); + const cpEnv = mockCpSpawn.mock.calls[0][2].env; + expect(cpEnv).not.toHaveProperty('CI'); + expect(cpEnv).not.toHaveProperty('npm_config_yes'); + expect(cpEnv).not.toHaveProperty('DEBIAN_FRONTEND'); + expect(cpEnv).not.toHaveProperty('NEEDRESTART_MODE'); + + mockChildProcess.emit('exit', 0, null); + mockChildProcess.emit('close', 0, null); + await new Promise(process.nextTick); + vi.unstubAllEnvs(); + }); + + it('should preserve pre-existing user env values in Full Access mode (?? coalesce)', async () => { + vi.resetModules(); + vi.stubEnv('CI', '0'); + vi.stubEnv('DEBIAN_FRONTEND', 'dialog'); + + const { ShellExecutionService } = await import( + './shellExecutionService.js' + ); + const { ApprovalMode } = await import('../policy/types.js'); + + mockGetPty.mockResolvedValue(null); + await ShellExecutionService.execute( + 'test-cp-yolo-preserve', + '/', + vi.fn(), + new AbortController().signal, + true, + { + ...shellExecutionConfig, + approvalMode: ApprovalMode.YOLO, + sanitizationConfig: { + ...shellExecutionConfig.sanitizationConfig, + allowedEnvironmentVariables: ['CI', 'DEBIAN_FRONTEND'], + }, + }, + ); + + expect(mockCpSpawn).toHaveBeenCalled(); + const cpEnv = mockCpSpawn.mock.calls[0][2].env; + // User-set values take precedence over the YOLO defaults + expect(cpEnv).toHaveProperty('CI', '0'); + expect(cpEnv).toHaveProperty('DEBIAN_FRONTEND', 'dialog'); + // The non-conflicting Full Access defaults still get injected + expect(cpEnv).toHaveProperty('npm_config_yes', 'true'); + expect(cpEnv).toHaveProperty('NEEDRESTART_MODE', 'a'); + + mockChildProcess.emit('exit', 0, null); + mockChildProcess.emit('close', 0, null); + await new Promise(process.nextTick); + vi.unstubAllEnvs(); + }); }); diff --git a/packages/core/src/services/shellExecutionService.ts b/packages/core/src/services/shellExecutionService.ts index 5817ffd3381..2f56aa840c8 100644 --- a/packages/core/src/services/shellExecutionService.ts +++ b/packages/core/src/services/shellExecutionService.ts @@ -37,6 +37,7 @@ import { type SandboxPermissions, } from './sandboxManager.js'; import type { SandboxConfig } from '../config/config.js'; +import { ApprovalMode } from '../policy/types.js'; import { killProcessGroup } from '../utils/process-utils.js'; import { ExecutionLifecycleService, @@ -105,6 +106,10 @@ export interface ShellExecutionConfig { backgroundCompletionBehavior?: 'inject' | 'notify' | 'silent'; originalCommand?: string; sessionId?: string; + // When set to YOLO, prepareExecution injects non-interactive env vars + // (CI=1, npm_config_yes=true, DEBIAN_FRONTEND=noninteractive, etc.) so + // package managers and installers auto-confirm instead of hanging on stdin. + approvalMode?: ApprovalMode; } /** @@ -481,6 +486,31 @@ export class ShellExecutionService { }); } + // Full Access mode opts the user into "auto-everything". Make common + // installers / package managers / VCS clients run non-interactively so + // they don't hang on prompts like `npx`'s "Ok to proceed? [y]" or + // `apt`'s "Do you want to continue?". Pre-existing user values pass + // through (?? coalesce). Codex CLI does this unconditionally at exec + // (codex-rs `UNIFIED_EXEC_ENV`); Claude Code propagates these vars from + // the user's env. We gate on YOLO because outside Full Access the user + // has not opted in to skipping prompts. + if (shellExecutionConfig.approvalMode === ApprovalMode.YOLO) { + Object.assign(baseEnv, { + CI: baseEnv['CI'] ?? '1', + npm_config_yes: baseEnv['npm_config_yes'] ?? 'true', + npm_config_fund: baseEnv['npm_config_fund'] ?? 'false', + npm_config_audit: baseEnv['npm_config_audit'] ?? 'false', + YARN_ENABLE_INTERACTIVE: baseEnv['YARN_ENABLE_INTERACTIVE'] ?? 'false', + DEBIAN_FRONTEND: baseEnv['DEBIAN_FRONTEND'] ?? 'noninteractive', + NEEDRESTART_MODE: baseEnv['NEEDRESTART_MODE'] ?? 'a', + GIT_TERMINAL_PROMPT: baseEnv['GIT_TERMINAL_PROMPT'] ?? '0', + GH_PROMPT_DISABLED: baseEnv['GH_PROMPT_DISABLED'] ?? '1', + GCM_INTERACTIVE: baseEnv['GCM_INTERACTIVE'] ?? 'never', + PIP_DISABLE_PIP_VERSION_CHECK: + baseEnv['PIP_DISABLE_PIP_VERSION_CHECK'] ?? '1', + }); + } + // 3. Prepare Sandboxed Command const sandboxedCommand = await sandboxManager.prepareCommand({ command: resolvedExecutable, diff --git a/packages/core/src/tools/shell.test.ts b/packages/core/src/tools/shell.test.ts index e1dd6bdf84c..479eb46c551 100644 --- a/packages/core/src/tools/shell.test.ts +++ b/packages/core/src/tools/shell.test.ts @@ -52,6 +52,7 @@ import { } from './shell.js'; import { debugLogger } from '../index.js'; import { type Config } from '../config/config.js'; +import { ApprovalMode } from '../policy/types.js'; import { NoopSandboxManager } from '../services/sandboxManager.js'; import { type ShellExecutionResult, @@ -898,6 +899,58 @@ EOF`; await promise; }); }); + + describe('PTY routing vs Full Access (YOLO)', () => { + it('should enable PTY when interactive shell is on and approvalMode is not YOLO', async () => { + vi.mocked(mockConfig.getEnableInteractiveShell).mockReturnValue(true); + vi.mocked(mockConfig.getApprovalMode).mockReturnValue( + ApprovalMode.DEFAULT, + ); + + const invocation = shellTool.build({ command: 'echo hi' }); + const promise = invocation.execute({ abortSignal: mockAbortSignal }); + resolveShellExecution(); + await promise; + + const call = mockShellExecutionService.mock.calls.at(-1); + // 5th positional arg (index 4) is shouldUseNodePty. + expect(call?.[4]).toBe(true); + }); + + it('should skip PTY in Full Access (YOLO) so sudo cannot prompt for a password', async () => { + // PTY would give sudo a real TTY and let it hang forever waiting for + // a password. The child_process fallback uses stdio:['ignore', ...] + // so sudo fails fast with "a password is required" (matches Codex's + // Stdio::null() in codex-rs/utils/pty/src/pipe.rs:144). + vi.mocked(mockConfig.getEnableInteractiveShell).mockReturnValue(true); + vi.mocked(mockConfig.getApprovalMode).mockReturnValue( + ApprovalMode.YOLO, + ); + + const invocation = shellTool.build({ command: 'sudo apt update' }); + const promise = invocation.execute({ abortSignal: mockAbortSignal }); + resolveShellExecution(); + await promise; + + const call = mockShellExecutionService.mock.calls.at(-1); + expect(call?.[4]).toBe(false); + }); + + it('should still skip PTY when interactive shell is disabled, regardless of approvalMode', async () => { + vi.mocked(mockConfig.getEnableInteractiveShell).mockReturnValue(false); + vi.mocked(mockConfig.getApprovalMode).mockReturnValue( + ApprovalMode.DEFAULT, + ); + + const invocation = shellTool.build({ command: 'echo hi' }); + const promise = invocation.execute({ abortSignal: mockAbortSignal }); + resolveShellExecution(); + await promise; + + const call = mockShellExecutionService.mock.calls.at(-1); + expect(call?.[4]).toBe(false); + }); + }); }); describe('shouldConfirmExecute', () => { diff --git a/packages/core/src/tools/shell.ts b/packages/core/src/tools/shell.ts index 13965d94f6e..f6b2cc18ced 100644 --- a/packages/core/src/tools/shell.ts +++ b/packages/core/src/tools/shell.ts @@ -651,11 +651,18 @@ export class ShellToolInvocation extends BaseToolInvocation< } }, combinedController.signal, - this.context.config.getEnableInteractiveShell(), + // In Full Access (YOLO) skip the PTY: a real TTY lets `sudo` and + // similar prompt for a password indefinitely. The child_process + // fallback uses stdio:['ignore', ...] so sudo fails fast with + // "a password is required" (matches Codex's Stdio::null() in + // codex-rs/utils/pty/src/pipe.rs:144). + this.context.config.getEnableInteractiveShell() && + this.context.config.getApprovalMode() !== ApprovalMode.YOLO, { ...shellExecutionConfig, sessionId: this.context.config?.getSessionId?.() ?? 'default', pager: 'cat', + approvalMode: this.context.config.getApprovalMode(), sanitizationConfig: shellExecutionConfig?.sanitizationConfig ?? this.context.config.sanitizationConfig, From 73ab20c4645381499ab2f15f662b5cdf60a9abfe Mon Sep 17 00:00:00 2001 From: emersonbusson Date: Sat, 16 May 2026 19:23:43 -0300 Subject: [PATCH 2/2] fix(core): drop redundant non-interactive env vars from Full Access block MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Per @gemini-code-assist[bot]: in the real production flow, Full Access (YOLO) forces shouldUseNodePty=false (see shell.ts), so execute() runs the child_process path with isInteractive=false. The pre-existing `!isInteractive` block already sets GIT_TERMINAL_PROMPT, GH_PROMPT_DISABLED, and GCM_INTERACTIVE unconditionally — meaning the `??` coalesce in the YOLO block for those three was both redundant AND misleading (it would not actually preserve user values, since the earlier block overwrites them first). Remove the three duplicated entries from the YOLO block. The eight remaining vars (CI, npm_config_yes/fund/audit, YARN_ENABLE_INTERACTIVE, DEBIAN_FRONTEND, NEEDRESTART_MODE, PIP_DISABLE_PIP_VERSION_CHECK) are unique to this block; their `??` coalesce continues to respect pre-existing user values. Added inline comment documenting the composition with the `!isInteractive` block so future readers don't re-add the duplicates. Updated the YOLO test to assert the realistic composed environment (isInteractive=false, both blocks running) instead of the hypothetical "YOLO + interactive" case which shell.ts no longer produces. --- .../services/shellExecutionService.test.ts | 16 +++++++++--- .../src/services/shellExecutionService.ts | 25 +++++++++++-------- 2 files changed, 28 insertions(+), 13 deletions(-) diff --git a/packages/core/src/services/shellExecutionService.test.ts b/packages/core/src/services/shellExecutionService.test.ts index ed178c89b61..993c46cb319 100644 --- a/packages/core/src/services/shellExecutionService.test.ts +++ b/packages/core/src/services/shellExecutionService.test.ts @@ -2193,11 +2193,18 @@ describe('ShellExecutionService environment variables', () => { vi.unstubAllEnvs(); }); - it('should inject non-interactive env vars in Full Access (YOLO) mode', async () => { + it('should inject Full Access (YOLO) env vars on top of the !isInteractive block', async () => { + // Real production flow: shell.ts forces shouldUseNodePty=false when + // approvalMode === YOLO, so execute() runs the child_process path with + // isInteractive=false. Both env blocks compose: `!isInteractive` + // contributes GIT_TERMINAL_PROMPT / GH_PROMPT_DISABLED / GCM_INTERACTIVE + // (and GIT_CONFIG_* shaping); the YOLO block contributes the + // package-manager / installer vars on top. vi.resetModules(); vi.stubEnv('CI', undefined); vi.stubEnv('npm_config_yes', undefined); vi.stubEnv('DEBIAN_FRONTEND', undefined); + vi.stubEnv('GIT_CONFIG_COUNT', undefined); const { ShellExecutionService } = await import( './shellExecutionService.js' @@ -2210,12 +2217,13 @@ describe('ShellExecutionService environment variables', () => { '/', vi.fn(), new AbortController().signal, - true, // interactive UI session — YOLO is the new gate + false, // YOLO real flow: PTY skipped → isInteractive=false { ...shellExecutionConfig, approvalMode: ApprovalMode.YOLO }, ); expect(mockCpSpawn).toHaveBeenCalled(); const cpEnv = mockCpSpawn.mock.calls[0][2].env; + // From the YOLO block (this PR): expect(cpEnv).toHaveProperty('CI', '1'); expect(cpEnv).toHaveProperty('npm_config_yes', 'true'); expect(cpEnv).toHaveProperty('npm_config_fund', 'false'); @@ -2223,10 +2231,12 @@ describe('ShellExecutionService environment variables', () => { expect(cpEnv).toHaveProperty('YARN_ENABLE_INTERACTIVE', 'false'); expect(cpEnv).toHaveProperty('DEBIAN_FRONTEND', 'noninteractive'); expect(cpEnv).toHaveProperty('NEEDRESTART_MODE', 'a'); + expect(cpEnv).toHaveProperty('PIP_DISABLE_PIP_VERSION_CHECK', '1'); + // From the pre-existing !isInteractive block — included here so a + // regression that decouples the two blocks gets caught: expect(cpEnv).toHaveProperty('GIT_TERMINAL_PROMPT', '0'); expect(cpEnv).toHaveProperty('GH_PROMPT_DISABLED', '1'); expect(cpEnv).toHaveProperty('GCM_INTERACTIVE', 'never'); - expect(cpEnv).toHaveProperty('PIP_DISABLE_PIP_VERSION_CHECK', '1'); mockChildProcess.emit('exit', 0, null); mockChildProcess.emit('close', 0, null); diff --git a/packages/core/src/services/shellExecutionService.ts b/packages/core/src/services/shellExecutionService.ts index 2f56aa840c8..58973a07125 100644 --- a/packages/core/src/services/shellExecutionService.ts +++ b/packages/core/src/services/shellExecutionService.ts @@ -487,13 +487,21 @@ export class ShellExecutionService { } // Full Access mode opts the user into "auto-everything". Make common - // installers / package managers / VCS clients run non-interactively so - // they don't hang on prompts like `npx`'s "Ok to proceed? [y]" or - // `apt`'s "Do you want to continue?". Pre-existing user values pass - // through (?? coalesce). Codex CLI does this unconditionally at exec - // (codex-rs `UNIFIED_EXEC_ENV`); Claude Code propagates these vars from - // the user's env. We gate on YOLO because outside Full Access the user - // has not opted in to skipping prompts. + // installers / package managers run non-interactively so they don't hang + // on prompts like `npx`'s "Ok to proceed? [y]" or `apt`'s "Do you want + // to continue?". Pre-existing user values pass through (?? coalesce). + // + // GIT_TERMINAL_PROMPT, GH_PROMPT_DISABLED, GCM_INTERACTIVE are NOT set + // here: the `!isInteractive` block above already does (in YOLO we force + // shouldUseNodePty=false in shell.ts → isInteractive=false → that block + // runs and overwrites them unconditionally). Listing them here would be + // redundant and the `??` would be misleading — user values for those + // three are not preserved by either path. + // + // Codex CLI does the always-on equivalent at exec (codex-rs + // `UNIFIED_EXEC_ENV`); Claude Code propagates these vars from the user's + // env. We gate on YOLO because outside Full Access the user has not + // opted in to skipping prompts. if (shellExecutionConfig.approvalMode === ApprovalMode.YOLO) { Object.assign(baseEnv, { CI: baseEnv['CI'] ?? '1', @@ -503,9 +511,6 @@ export class ShellExecutionService { YARN_ENABLE_INTERACTIVE: baseEnv['YARN_ENABLE_INTERACTIVE'] ?? 'false', DEBIAN_FRONTEND: baseEnv['DEBIAN_FRONTEND'] ?? 'noninteractive', NEEDRESTART_MODE: baseEnv['NEEDRESTART_MODE'] ?? 'a', - GIT_TERMINAL_PROMPT: baseEnv['GIT_TERMINAL_PROMPT'] ?? '0', - GH_PROMPT_DISABLED: baseEnv['GH_PROMPT_DISABLED'] ?? '1', - GCM_INTERACTIVE: baseEnv['GCM_INTERACTIVE'] ?? 'never', PIP_DISABLE_PIP_VERSION_CHECK: baseEnv['PIP_DISABLE_PIP_VERSION_CHECK'] ?? '1', });