diff --git a/packages/core/src/providers/claude.test.ts b/packages/core/src/providers/claude.test.ts index b760837278..ecebbdcd0c 100644 --- a/packages/core/src/providers/claude.test.ts +++ b/packages/core/src/providers/claude.test.ts @@ -446,12 +446,15 @@ describe('ClaudeProvider', () => { ); }); - test('subprocess env passes through all process.env keys (no allowlist filtering)', async () => { - // With the allowlist removed, buildSubprocessEnv returns { ...process.env }. - // CWD .env leakage and CLAUDECODE markers are handled at entry point by - // stripCwdEnv(), not by buildSubprocessEnv(). See #1067, #1097. - const originalKey = process.env.CUSTOM_USER_KEY; + test('subprocess env keeps user keys but strips nested Claude markers', async () => { + const originalCustomKey = process.env.CUSTOM_USER_KEY; + const originalEntrypoint = process.env.CLAUDE_CODE_ENTRYPOINT; + const originalOauth = process.env.CLAUDE_CODE_OAUTH_TOKEN; + const originalNodeOptions = process.env.NODE_OPTIONS; process.env.CUSTOM_USER_KEY = 'user-trusted-value'; + process.env.CLAUDE_CODE_ENTRYPOINT = 'nested'; + process.env.CLAUDE_CODE_OAUTH_TOKEN = 'oauth-token'; + process.env.NODE_OPTIONS = '--inspect'; mockQuery.mockImplementation(async function* () { // Empty generator @@ -464,12 +467,23 @@ describe('ClaudeProvider', () => { const callArgs = mockQuery.mock.calls[0][0] as { options: { env: NodeJS.ProcessEnv } }; expect(callArgs.options.env.CUSTOM_USER_KEY).toBe('user-trusted-value'); - expect(callArgs.options.env.PATH).toBe(process.env.PATH); + expect(callArgs.options.env.PATH ?? callArgs.options.env.Path).toBe( + process.env.PATH ?? process.env.Path + ); expect(callArgs.options.env.HOME).toBe(process.env.HOME); + expect(callArgs.options.env.CLAUDE_CODE_ENTRYPOINT).toBeUndefined(); + expect(callArgs.options.env.CLAUDE_CODE_OAUTH_TOKEN).toBe('oauth-token'); + expect(callArgs.options.env.NODE_OPTIONS).toBeUndefined(); // Cleanup - if (originalKey !== undefined) process.env.CUSTOM_USER_KEY = originalKey; + if (originalCustomKey !== undefined) process.env.CUSTOM_USER_KEY = originalCustomKey; else delete process.env.CUSTOM_USER_KEY; + if (originalEntrypoint !== undefined) process.env.CLAUDE_CODE_ENTRYPOINT = originalEntrypoint; + else delete process.env.CLAUDE_CODE_ENTRYPOINT; + if (originalOauth !== undefined) process.env.CLAUDE_CODE_OAUTH_TOKEN = originalOauth; + else delete process.env.CLAUDE_CODE_OAUTH_TOKEN; + if (originalNodeOptions !== undefined) process.env.NODE_OPTIONS = originalNodeOptions; + else delete process.env.NODE_OPTIONS; }); test('classifies exit code errors as crash and retries up to 3 times', async () => { @@ -689,6 +703,32 @@ describe('ClaudeProvider', () => { expect(env.HOME).toBe('/custom/home'); }); + test('requestOptions.env cannot re-inject nested Claude markers', async () => { + mockQuery.mockImplementation(async function* () { + yield { type: 'result', session_id: 'sid' }; + }); + + // eslint-disable-next-line @typescript-eslint/no-unused-vars + for await (const _ of client.sendQuery('test', '/tmp', undefined, { + env: { + SAFE_KEY: 'safe', + CLAUDECODE: '1', + CLAUDE_CODE_ENTRYPOINT: 'nested', + CLAUDE_CODE_OAUTH_TOKEN: 'oauth-from-override', + }, + })) { + // consume + } + + expect(mockQuery).toHaveBeenCalledTimes(1); + const callArgs = mockQuery.mock.calls[0][0] as { options: Record }; + const env = callArgs.options.env as Record; + expect(env.SAFE_KEY).toBe('safe'); + expect(env.CLAUDECODE).toBeUndefined(); + expect(env.CLAUDE_CODE_ENTRYPOINT).toBeUndefined(); + expect(env.CLAUDE_CODE_OAUTH_TOKEN).toBe('oauth-from-override'); + }); + test('passes effort to SDK when provided', async () => { mockQuery.mockImplementation(async function* () { yield { type: 'result', session_id: 'sid' }; diff --git a/packages/core/src/providers/claude.ts b/packages/core/src/providers/claude.ts index 0d8c6d4596..400e2dbe92 100644 --- a/packages/core/src/providers/claude.ts +++ b/packages/core/src/providers/claude.ts @@ -35,10 +35,9 @@ import { type TokenUsage, } from '../types'; import { createLogger } from '@archon/paths'; -// No env filtering here — process.env is already clean: -// stripCwdEnv() at entry point stripped CWD .env keys + CLAUDECODE markers, -// then ~/.archon/.env was loaded as the trusted source. All keys the user sets -// in ~/.archon/.env are intentional and pass through to the subprocess. +// process.env is cleaned at entry point by stripCwdEnv(), then ~/.archon/.env +// is loaded as trusted source. We still sanitize the final subprocess env as +// defense-in-depth so requestOptions.env cannot re-inject nested Claude markers. import { scanPathForSensitiveKeys, EnvLeakError } from '../utils/env-leak-scanner'; import * as codebaseDb from '../db/codebases'; import { loadConfig } from '../config/config-loader'; @@ -50,6 +49,28 @@ function getLog(): ReturnType { return cachedLog; } +const CLAUDE_CODE_AUTH_VARS = new Set([ + 'CLAUDE_CODE_OAUTH_TOKEN', + 'CLAUDE_CODE_USE_BEDROCK', + 'CLAUDE_CODE_USE_VERTEX', +]); + +const BLOCKED_SUBPROCESS_ENV_KEYS = new Set([ + 'CLAUDECODE', + 'NODE_OPTIONS', + 'VSCODE_INSPECTOR_OPTIONS', +]); + +function sanitizeSubprocessEnv(env: NodeJS.ProcessEnv): NodeJS.ProcessEnv { + const sanitized: NodeJS.ProcessEnv = {}; + for (const [key, value] of Object.entries(env)) { + if (BLOCKED_SUBPROCESS_ENV_KEYS.has(key)) continue; + if (key.startsWith('CLAUDE_CODE_') && !CLAUDE_CODE_AUTH_VARS.has(key)) continue; + sanitized[key] = value; + } + return sanitized; +} + /** * Content block type for assistant messages * Represents text or tool_use blocks from Claude API responses @@ -93,12 +114,13 @@ function normalizeClaudeUsage(usage?: { * - No tokens → SDK uses `claude /login` credentials (global auth) * - User controls this by what they put in ~/.archon/.env * - * We log the detected mode for diagnostics but don't filter — the user's - * config is trusted. See coleam00/Archon#1067 for design rationale. + * We preserve user env keys, but always drop known nested-session/debugger + * markers to prevent subprocess deadlocks. */ function buildSubprocessEnv(): NodeJS.ProcessEnv { + const subprocessEnv = sanitizeSubprocessEnv(process.env); const hasExplicitTokens = Boolean( - process.env.CLAUDE_CODE_OAUTH_TOKEN ?? process.env.CLAUDE_API_KEY + subprocessEnv.CLAUDE_CODE_OAUTH_TOKEN ?? subprocessEnv.CLAUDE_API_KEY ); const authMode = hasExplicitTokens ? 'explicit' : 'global'; getLog().info( @@ -106,7 +128,7 @@ function buildSubprocessEnv(): NodeJS.ProcessEnv { authMode === 'global' ? 'using_global_auth' : 'using_explicit_tokens' ); - return { ...process.env }; + return subprocessEnv; } /** Max retries for transient subprocess failures (3 = 4 total attempts). @@ -327,12 +349,15 @@ export class ClaudeProvider implements IAgentProvider { ); } + const mergedSubprocessEnv = requestOptions?.env + ? { ...buildSubprocessEnv(), ...requestOptions.env } + : buildSubprocessEnv(); + const subprocessEnv = sanitizeSubprocessEnv(mergedSubprocessEnv); + const options: Options = { cwd, pathToClaudeCodeExecutable: cliPath, - env: requestOptions?.env - ? { ...buildSubprocessEnv(), ...requestOptions.env } - : buildSubprocessEnv(), + env: subprocessEnv, model: requestOptions?.model, abortController: controller, ...(requestOptions?.tools !== undefined ? { tools: requestOptions.tools } : {}), diff --git a/packages/core/src/utils/env-leak-scanner.test.ts b/packages/core/src/utils/env-leak-scanner.test.ts index 4d436bbc24..4538df73a0 100644 --- a/packages/core/src/utils/env-leak-scanner.test.ts +++ b/packages/core/src/utils/env-leak-scanner.test.ts @@ -116,6 +116,28 @@ describe('EnvLeakError', () => { expect(spawn).toContain('Cannot run workflow'); }); + it('formats remediation command with the offending single key', () => { + const report = { path: '/x', findings: [{ file: '.env', keys: ['OPENAI_API_KEY'] }] }; + const message = formatLeakError(report, 'register-ui'); + + expect(message).toContain("Remove the key from this repo's .env"); + expect(message).toContain("grep -vE '^(OPENAI_API_KEY)=' .env > .env.tmp && mv .env.tmp .env"); + }); + + it('formats remediation command with all offending keys', () => { + const report = { + path: '/x', + findings: [{ file: '.env', keys: ['GEMINI_API_KEY', 'OPENAI_API_KEY'] }], + }; + const message = formatLeakError(report, 'register-ui'); + + expect(message).toContain("Remove the keys from this repo's .env"); + expect(message).toContain( + "grep -vE '^(GEMINI_API_KEY|OPENAI_API_KEY)=' .env > .env.tmp && mv .env.tmp .env" + ); + expect(message).not.toContain("grep -v '^ANTHROPIC_API_KEY=' .env"); + }); + it('formats multiple findings', () => { const report = { path: '/test', diff --git a/packages/core/src/utils/env-leak-scanner.ts b/packages/core/src/utils/env-leak-scanner.ts index 48edc2c6b7..7b78e85041 100644 --- a/packages/core/src/utils/env-leak-scanner.ts +++ b/packages/core/src/utils/env-leak-scanner.ts @@ -119,11 +119,33 @@ function consentCopy(context: LeakErrorContext): string { } } +function escapeRegexForGrep(raw: string): string { + return raw.replace(/[\\^$.*+?()[\]{}|]/g, '\\$&'); +} + +function collectOffendingKeys(report: LeakReport): string[] { + const ordered = report.findings.flatMap(f => f.keys).filter(key => !key.startsWith('[')); + return [...new Set(ordered)]; +} + +function buildRemediationCommand(report: LeakReport): string { + const offendingKeys = collectOffendingKeys(report); + const keyPattern = + offendingKeys.length > 0 + ? offendingKeys.map(escapeRegexForGrep).join('|') + : 'ANTHROPIC_API_KEY'; + + return `grep -vE '^(${keyPattern})=' .env > .env.tmp && mv .env.tmp .env`; +} + export function formatLeakError( report: LeakReport, context: LeakErrorContext = 'register-ui' ): string { const fileList = report.findings.map(f => ` ${f.file} — ${f.keys.join(', ')}`).join('\n'); + const offendingKeys = collectOffendingKeys(report); + const keyWord = offendingKeys.length > 1 ? 'keys' : 'key'; + const remediationCommand = buildRemediationCommand(report); const header = context === 'spawn-existing' @@ -144,8 +166,8 @@ ${fileList} This can bill the wrong API account silently. Choose one: - 1. Remove the key from this repo's .env (recommended): - grep -v '^ANTHROPIC_API_KEY=' .env > .env.tmp && mv .env.tmp .env + 1. Remove the ${keyWord} from this repo's .env (recommended): + ${remediationCommand} 2. Rename to a non-auto-loaded file: mv .env .env.secrets