Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
54 changes: 47 additions & 7 deletions packages/core/src/providers/claude.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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 () => {
Expand Down Expand Up @@ -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<string, unknown> };
const env = callArgs.options.env as Record<string, string>;
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' };
Expand Down
47 changes: 36 additions & 11 deletions packages/core/src/providers/claude.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand All @@ -50,6 +49,28 @@ function getLog(): ReturnType<typeof createLogger> {
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;
}
Comment thread
LaplaceYoung marked this conversation as resolved.
return sanitized;
}

/**
* Content block type for assistant messages
* Represents text or tool_use blocks from Claude API responses
Expand Down Expand Up @@ -93,20 +114,21 @@ 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(
{ authMode },
authMode === 'global' ? 'using_global_auth' : 'using_explicit_tokens'
);

return { ...process.env };
return subprocessEnv;
}

/** Max retries for transient subprocess failures (3 = 4 total attempts).
Expand Down Expand Up @@ -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 } : {}),
Expand Down
22 changes: 22 additions & 0 deletions packages/core/src/utils/env-leak-scanner.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand Down
26 changes: 24 additions & 2 deletions packages/core/src/utils/env-leak-scanner.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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`;
}
Comment thread
LaplaceYoung marked this conversation as resolved.

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'
Expand All @@ -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
Expand Down