From f9154237cdda783c79447043ffc8b5379d1ca1ff Mon Sep 17 00:00:00 2001 From: "posthog[bot]" <206114724+posthog[bot]@users.noreply.github.com> Date: Tue, 1 Sep 2026 19:28:30 +0000 Subject: [PATCH] fix(agent): pin the agent's secure-storage dir so no keychain login leaks The agent subprocess got an empty CLAUDE_CONFIG_DIR, which closed the file half of a stored Claude login. The macOS keychain half stayed open whenever a user shell set CLAUDE_SECURESTORAGE_CONFIG_DIR: that var sits outside the ANTHROPIC_*/CLAUDE_CODE_* namespace the wizard strips, and the binary derives the keychain service name from it. Both spawn sites now take one env pair from isolatedAgentCredentialEnv(), which points CLAUDE_CONFIG_DIR and CLAUDE_SECURESTORAGE_CONFIG_DIR at the same per-run temp dir. The keychain lookup then names an item that does not exist, so it finds nothing. Generated-By: PostHog Desktop Task-Id: 658d6c4a-2e6b-4936-9845-77c4fa863fd2 --- src/lib/agent/__tests__/stored-login.test.ts | 31 ++++++++++----- src/lib/agent/agent-interface.ts | 24 ++++++------ src/lib/agent/mcp-prompt-streaming.ts | 9 ++--- src/lib/agent/stored-login.ts | 41 ++++++++++++++++---- 4 files changed, 72 insertions(+), 33 deletions(-) diff --git a/src/lib/agent/__tests__/stored-login.test.ts b/src/lib/agent/__tests__/stored-login.test.ts index 6ef5b14e..7464fb5c 100644 --- a/src/lib/agent/__tests__/stored-login.test.ts +++ b/src/lib/agent/__tests__/stored-login.test.ts @@ -3,10 +3,11 @@ import * as os from 'os'; import path from 'path'; import { spawnSync } from 'node:child_process'; import { - createIsolatedAgentConfigDir, detectStoredClaudeLogin, hasStoredClaudeLogin, + isolatedAgentCredentialEnv, } from '../stored-login'; +import { isBlockedAgentEnvKey } from '../agent-env-isolation'; vi.mock('node:child_process', () => ({ spawnSync: vi.fn() })); vi.mock('@utils/analytics', () => ({ @@ -87,9 +88,15 @@ describe('detectStoredClaudeLogin', () => { }); }); -describe('createIsolatedAgentConfigDir', () => { +describe('isolatedAgentCredentialEnv', () => { const created: string[] = []; + const take = () => { + const env = isolatedAgentCredentialEnv(); + created.push(env.CLAUDE_CONFIG_DIR); + return env; + }; + afterEach(() => { for (const dir of created.splice(0)) { try { @@ -101,19 +108,25 @@ describe('createIsolatedAgentConfigDir', () => { }); it('creates a fresh, empty directory that holds no stored login', () => { - const dir = createIsolatedAgentConfigDir(); - created.push(dir); + const dir = take().CLAUDE_CONFIG_DIR; // Empty: no `.credentials.json` for the SDK to resolve a stored login from. expect(fs.existsSync(dir)).toBe(true); expect(fs.readdirSync(dir)).toEqual([]); }); - it('returns a distinct directory on each call so concurrent runs never share', () => { - const a = createIsolatedAgentConfigDir(); - const b = createIsolatedAgentConfigDir(); - created.push(a, b); + it('isolates the secure store too, so the macOS keychain lookup misses', () => { + const env = take(); - expect(a).not.toBe(b); + // The binary names the keychain item after this dir, so pointing it at the + // throwaway dir is what keeps `Claude Code-credentials` out of reach. + expect(env.CLAUDE_SECURESTORAGE_CONFIG_DIR).toBe(env.CLAUDE_CONFIG_DIR); + // Outside the ANTHROPIC_*/CLAUDE_CODE_* strip, so it must be set, not merely + // inherited — a shell value would otherwise undo the isolation. + expect(isBlockedAgentEnvKey('CLAUDE_SECURESTORAGE_CONFIG_DIR')).toBe(false); + }); + + it('returns a distinct directory on each call so concurrent runs never share', () => { + expect(take().CLAUDE_CONFIG_DIR).not.toBe(take().CLAUDE_CONFIG_DIR); }); }); diff --git a/src/lib/agent/agent-interface.ts b/src/lib/agent/agent-interface.ts index de6e8116..80d9f43b 100644 --- a/src/lib/agent/agent-interface.ts +++ b/src/lib/agent/agent-interface.ts @@ -66,7 +66,7 @@ import { detectStoredClaudeLogin, hasStoredClaudeLogin, claudeConfigDir, - createIsolatedAgentConfigDir, + isolatedAgentCredentialEnv, } from './stored-login'; import { sanitizeAgentSubprocessEnv } from './agent-env-isolation'; @@ -591,16 +591,17 @@ export async function initializeAgent( ); // A pre-existing Claude login (the SDK's "/login managed key") could outrank - // the gateway token and 401 the run. The subprocess now gets an isolated, - // empty CLAUDE_CONFIG_DIR at the spawn site (see below), so a stored login - // in `~/.claude` can no longer reach the gateway. Still detect + log it to - // measure how often the isolation saves a run. + // the gateway token and 401 the run. The subprocess now gets isolated, + // empty config and secure-storage dirs at the spawn site (see below), so + // neither a stored login in `~/.claude` nor one in the macOS keychain can + // reach the gateway. Still detect + log it to measure how often the + // isolation saves a run. const storedLogin = detectStoredClaudeLogin(); if (hasStoredClaudeLogin(storedLogin)) { logToFile( `Pre-existing Claude login detected (credentialsFile=${storedLogin.credentialsFile}, ` + - `keychain=${storedLogin.keychain}). The isolated CLAUDE_CONFIG_DIR keeps it out of ` + - `the agent run, so it no longer outranks the wizard's gateway token.`, + `keychain=${storedLogin.keychain}). The isolated config and secure-storage dirs keep ` + + `it out of the agent run, so it no longer outranks the wizard's gateway token.`, ); analytics.wizardCapture('claude stored login detected', { credentials_file: storedLogin.credentialsFile, @@ -1033,11 +1034,10 @@ export async function runAgent( ANTHROPIC_BASE_URL: agentConfig.gatewayAuth.gatewayUrl, ANTHROPIC_AUTH_TOKEN: agentConfig.gatewayAuth.token, CLAUDE_CODE_OAUTH_TOKEN: agentConfig.gatewayAuth.token, - // Point the binary at an empty config dir so it cannot resolve a - // stored Claude login (a `~/.claude/.credentials.json`) and send that - // to the gateway, which 401s it. The env token above is then the only - // credential it can find. See stored-login.ts. - CLAUDE_CONFIG_DIR: createIsolatedAgentConfigDir(), + // Per-run empty config + secure-storage dirs, so no stored Claude + // login reaches the gateway and 401s the run. The env token above is + // then the only credential the binary can find. See stored-login.ts. + ...isolatedAgentCredentialEnv(), CLAUDE_CODE_DISABLE_EXPERIMENTAL_BETAS: 'true', // The MCP config resolves this in the child; sending the value would // put it on the CLI's argv. diff --git a/src/lib/agent/mcp-prompt-streaming.ts b/src/lib/agent/mcp-prompt-streaming.ts index 986c59f6..de6050bd 100644 --- a/src/lib/agent/mcp-prompt-streaming.ts +++ b/src/lib/agent/mcp-prompt-streaming.ts @@ -19,7 +19,7 @@ import { logToFile } from '@utils/debug'; import { gatewayAuth } from '@lib/gateway-session'; import { buildAgentEnv, buildRunTags } from '@lib/agent/agent-interface'; import { sanitizeAgentSubprocessEnv } from '@lib/agent/agent-env-isolation'; -import { createIsolatedAgentConfigDir } from '@lib/agent/stored-login'; +import { isolatedAgentCredentialEnv } from '@lib/agent/stored-login'; import { analytics } from '@utils/analytics'; // Cached SDK module — first call pays the dynamic-import cost; later @@ -367,10 +367,9 @@ export async function* runMcpPromptViaSdk(args: { ANTHROPIC_BASE_URL: auth.gatewayUrl, ANTHROPIC_AUTH_TOKEN: auth.token, CLAUDE_CODE_OAUTH_TOKEN: auth.token, - // Point the binary at an empty config dir so it cannot resolve a - // stored Claude login and send that to the gateway, which 401s it. - // See stored-login.ts. - CLAUDE_CONFIG_DIR: createIsolatedAgentConfigDir(), + // Per-run empty config + secure-storage dirs, so no stored Claude + // login reaches the gateway and 401s the run. See stored-login.ts. + ...isolatedAgentCredentialEnv(), CLAUDE_CODE_DISABLE_EXPERIMENTAL_BETAS: 'true', // The MCP config resolves this in the child; sending the value would // put it on the CLI's argv. diff --git a/src/lib/agent/stored-login.ts b/src/lib/agent/stored-login.ts index 3a9ca812..7814a992 100644 --- a/src/lib/agent/stored-login.ts +++ b/src/lib/agent/stored-login.ts @@ -16,7 +16,7 @@ * is ever exposed. The locations checked mirror the SDK's own resolution * (npmjs.com/package/@anthropic-ai/claude-agent-sdk → sdk.mjs): a * `.credentials.json` under `CLAUDE_CONFIG_DIR` / `~/.claude`, and the macOS - * keychain item below. + * keychain item below. {@link isolatedAgentCredentialEnv} closes both. */ import * as fs from 'fs'; import * as os from 'os'; @@ -49,21 +49,48 @@ export const claudeConfigDir = (homeDir: string = os.homedir()): string => /** * Create a fresh, empty config dir for the agent subprocess and return its path. - * The spawn site sets it as `CLAUDE_CONFIG_DIR`, so the SDK's `claude` binary - * resolves credentials from this empty dir — never the user's `~/.claude`. With - * no `.credentials.json` to find, the binary cannot outrank the wizard's gateway - * token (see {@link detectStoredClaudeLogin}), so a stored login can no longer - * reach the PostHog gateway and 401. * * `mkdtempSync` gives each run its own dir, so concurrent runs never share one. * `/tmp` on macOS/Linux matches the agent sandbox's writable roots; Windows has * no `/tmp`, so fall back to the OS temp dir there. */ -export function createIsolatedAgentConfigDir(): string { +function createIsolatedAgentConfigDir(): string { const base = process.platform === 'win32' ? os.tmpdir() : '/tmp'; return fs.mkdtempSync(path.join(base, 'posthog-wizard-claude-')); } +/** + * The credential-isolation env the spawn sites hand the `claude` binary. Both + * vars point at one fresh, empty dir, which cuts the binary off from BOTH halves + * of a stored login: + * + * - `CLAUDE_CONFIG_DIR` is where the binary reads `.credentials.json` and + * `.claude.json` from. An empty dir has neither. + * - `CLAUDE_SECURESTORAGE_CONFIG_DIR` is where it reads the *secure* store from. + * On macOS that store is the keychain, and the binary derives the keychain + * service name from this dir — `Claude Code-credentials` for the default dir, + * `Claude Code-credentials-` otherwise. A per-run temp dir + * therefore names an item that does not exist, so the lookup finds nothing. + * + * Setting the second var explicitly matters twice over. It is outside the + * `ANTHROPIC_*` / `CLAUDE_CODE_*` namespace that `sanitizeAgentSubprocessEnv` + * strips, so a value in the user's shell would otherwise survive and point the + * keychain lookup back at the real `Claude Code-credentials` item. And it pins + * the keychain namespace directly instead of leaving it to the binary's + * undocumented fallback to `CLAUDE_CONFIG_DIR`. + * + * The gateway token alone is not enough: a stored login travels as an + * `x-api-key` header, which the binary resolves separately from — and does not + * suppress with — the `ANTHROPIC_AUTH_TOKEN` the wizard injects. + */ +export function isolatedAgentCredentialEnv(): { + CLAUDE_CONFIG_DIR: string; + CLAUDE_SECURESTORAGE_CONFIG_DIR: string; +} { + const dir = createIsolatedAgentConfigDir(); + return { CLAUDE_CONFIG_DIR: dir, CLAUDE_SECURESTORAGE_CONFIG_DIR: dir }; +} + /** * Look for a stored Claude login. `homeDir` / `platform` are injectable for * tests; production uses the real home dir and platform.