Skip to content

fix(agent): pin the agent's secure-storage dir so no keychain login leaks #1191

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
posthog wants to merge 1 commit into
base: main
Choose a base branch
from
+72 −33
Draft

fix(agent): pin the agent's secure-storage dir so no keychain login leaks #1191

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
31 changes: 22 additions & 9 deletions src/lib/agent/__tests__/stored-login.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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', () => ({
Expand Down Expand Up @@ -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 {
Expand All @@ -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);
});
});
24 changes: 12 additions & 12 deletions src/lib/agent/agent-interface.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ import {
detectStoredClaudeLogin,
hasStoredClaudeLogin,
claudeConfigDir,
createIsolatedAgentConfigDir,
isolatedAgentCredentialEnv,
} from './stored-login';
import { sanitizeAgentSubprocessEnv } from './agent-env-isolation';

Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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.
Expand Down
9 changes: 4 additions & 5 deletions src/lib/agent/mcp-prompt-streaming.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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(),
Comment on lines +370 to +372

@posthog posthog Bot Sep 1, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fresh config directories break resumed tutorial sessions

should_fix bug

Why we think it's a valid issue
  • Checked: the pinned SDK itself. Downloaded and unpacked @anthropic-ai/claude-agent-sdk@0.3.169 (the version pinned at package.json:35) and read its typings and bundle, rather than reasoning from the snippet.
  • Found: the SDK ties resume to the config dir. sdk.d.ts documents persistSession as "When false, disables session persistence to disk. Sessions will not be saved to ~/.claude/projects/ and cannot be resumed later. @default true", and the sessionStore doc says "the subprocess still writes to CLAUDE_CONFIG_DIR". In sdk.mjs the config root resolves as (process.env.CLAUDE_CONFIG_DIR ?? join(homedir(), ".claude")).normalize("NFC"), and lookup failures raise Session ${id} not found in project directory / Session ${id} not found (no projects directory).
  • Found: isolatedAgentCredentialEnv() calls mkdtempSync on every invocation (stored-login.ts:57-60, stored-login.ts:90). The PR's own test asserts this at __tests__/stored-login.test.ts:129-131 — "returns a distinct directory on each call". So prompt 2 spawns against a different, empty root than prompt 1 wrote its transcript into.
  • Found: the resume path is a shipped feature, not a corner. McpSuggestedPromptsScreen.tsx:412 passes resumeSessionId: currentSessionIdRef.current, which mcp-prompt-streaming.ts:309 turns into resume. MAX_PROMPT_RUNS = 5 (McpSuggestedPromptsScreen.tsx:121) with a dedicated Phase.FollowUp, so every tutorial user who picks a second prompt hits it. The code states the intent it no longer delivers: mcp-prompt-streaming.ts:202-205 and :298-300 describe resumed follow-ups keeping prior turns as context.
  • Found: no mitigation applies. The SDK's resume-materialization temp dir (claude-resume-*) only runs on the sessionStore path, which this call site does not use.
  • Found: the root cause landed one commit earlier. git diff FETCH_HEAD HEAD shows main already had CLAUDE_CONFIG_DIR: createIsolatedAgentConfigDir() here, added by 919e7d1 (fix(agent): isolate CLAUDE_CONFIG_DIR so a stored Claude login cannot 401 the run #1180, v2.71.0). This PR replaces that line with the two-var spread at mcp-prompt-streaming.ts:372.
  • Impact: every follow-up prompt in the MCP tutorial resumes a session id whose transcript is unreachable. The run either errors — surfaced to the user through the error chunk at mcp-prompt-streaming.ts:400-403 and McpSuggestedPromptsScreen.tsx:430-433 — or silently starts fresh and drops the conversation the follow-up suggestions refer to. The trigger and the consequence are both concrete and deterministic, so this clears the bar even though the PR only re-expresses the line. It is the right moment to fix, because the change edits that exact line and adds a test that locks in the per-call fresh directory.
Issue description

The SDK stores resumable sessions under CLAUDE_CONFIG_DIR because persistSession defaults to true. This call creates a new directory for every prompt, including prompts with resumeSessionId. The resumed process cannot find the first prompt's transcript. Follow-up prompts can fail or lose their prior context.

Suggested fix

Create one isolated credential directory for each tutorial conversation. Reuse both environment values for resumed prompts. Create a new directory only when the user starts a new conversation. Add a test that checks path reuse for follow-ups and path replacement after reset.

Prompt to fix with AI (copy-paste) 
## Context
@src/lib/agent/mcp-prompt-streaming.ts#L370-372

<issue_description>
The SDK stores resumable sessions under `CLAUDE_CONFIG_DIR` because `persistSession` defaults to `true`. This call creates a new directory for every prompt, including prompts with `resumeSessionId`. The resumed process cannot find the first prompt's transcript. Follow-up prompts can fail or lose their prior context.
</issue_description>

<issue_validation>
- **Checked:** the pinned SDK itself. Downloaded and unpacked `@anthropic-ai/claude-agent-sdk@0.3.169` (the version pinned at `package.json:35`) and read its typings and bundle, rather than reasoning from the snippet.
- **Found:** the SDK ties resume to the config dir. `sdk.d.ts` documents `persistSession` as "When false, disables session persistence to disk. Sessions will not be saved to ~/.claude/projects/ and cannot be resumed later. @default true", and the `sessionStore` doc says "the subprocess still writes to CLAUDE_CONFIG_DIR". In `sdk.mjs` the config root resolves as `(process.env.CLAUDE_CONFIG_DIR ?? join(homedir(), ".claude")).normalize("NFC")`, and lookup failures raise `Session ${id} not found in project directory` / `Session ${id} not found (no projects directory)`.
- **Found:** `isolatedAgentCredentialEnv()` calls `mkdtempSync` on every invocation (`stored-login.ts:57-60`, `stored-login.ts:90`). The PR's own test asserts this at `__tests__/stored-login.test.ts:129-131` — "returns a distinct directory on each call". So prompt 2 spawns against a different, empty root than prompt 1 wrote its transcript into.
- **Found:** the resume path is a shipped feature, not a corner. `McpSuggestedPromptsScreen.tsx:412` passes `resumeSessionId: currentSessionIdRef.current`, which `mcp-prompt-streaming.ts:309` turns into `resume`. `MAX_PROMPT_RUNS = 5` (`McpSuggestedPromptsScreen.tsx:121`) with a dedicated `Phase.FollowUp`, so every tutorial user who picks a second prompt hits it. The code states the intent it no longer delivers: `mcp-prompt-streaming.ts:202-205` and `:298-300` describe resumed follow-ups keeping prior turns as context.
- **Found:** no mitigation applies. The SDK's resume-materialization temp dir (`claude-resume-*`) only runs on the `sessionStore` path, which this call site does not use.
- **Found:** the root cause landed one commit earlier. `git diff FETCH_HEAD HEAD` shows main already had `CLAUDE_CONFIG_DIR: createIsolatedAgentConfigDir()` here, added by `919e7d1` (#1180, v2.71.0). This PR replaces that line with the two-var spread at `mcp-prompt-streaming.ts:372`.
- **Impact:** every follow-up prompt in the MCP tutorial resumes a session id whose transcript is unreachable. The run either errors — surfaced to the user through the `error` chunk at `mcp-prompt-streaming.ts:400-403` and `McpSuggestedPromptsScreen.tsx:430-433` — or silently starts fresh and drops the conversation the follow-up suggestions refer to. The trigger and the consequence are both concrete and deterministic, so this clears the bar even though the PR only re-expresses the line. It is the right moment to fix, because the change edits that exact line and adds a test that locks in the per-call fresh directory.
</issue_validation>

## Task
Investigate the issue and solve it

<potential_solution>
Create one isolated credential directory for each tutorial conversation. Reuse both environment values for resumed prompts. Create a new directory only when the user starts a new conversation. Add a test that checks path reuse for follow-ups and path replacement after reset.
</potential_solution>

@posthog posthog Bot Sep 1, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Confirmed this is a real bug against the current head, and I'm escalating it for a human decision rather than fixing it unattended.

What happens: the MCP tutorial resumes sessions for follow-up prompts (it carries the previous run's session id forward as resumeSessionId, up to 5 prompts per session). But each run is handed a brand-new, empty per-run config directory — the isolation helper creates a fresh temp dir on every call, and a test in this PR deliberately locks that in ("a distinct directory on each call"). Because the SDK keeps resumable transcripts under that config directory, a resumed follow-up looks for the first prompt's transcript in a directory that never held it. The result is either a "session not found" error surfaced to the user or a silent fresh start that drops the conversation the follow-up suggestions refer to. The root cause actually shipped in v2.71.0 with the original config-dir isolation, but this PR edits the exact line, so it's a reasonable place to address it.

Why it needs a human: fixing it means reconciling two goals that currently conflict — per-run credential isolation wants a fresh, empty directory, while session resume wants a stable directory for the whole conversation. The natural fix is one isolated directory per tutorial conversation (created when the user picks a new prompt, reused for follow-ups, reset on a new conversation), which requires threading that directory through the screen → services → streaming runner, changing the isolation helper's contract, and updating its locking test. Two things put it beyond an unattended fix: (1) its correctness — that resume actually recovers the transcript once the directory is stable — can only be proven by running the real Agent SDK against a resumed session, which this pass can't do; and (2) the helper is credential-leak-prevention code, so changing how its directory is created and reused shouldn't be done without a human confirming it doesn't reopen a leak path.

Decision needed: adopt the per-conversation stable isolated directory (and confirm via a real resumed-session run that it fixes resume without reintroducing a stored-login leak), or decide the tutorial shouldn't resume at all (drop resume here and accept each prompt as standalone).

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.
Expand Down
41 changes: 34 additions & 7 deletions src/lib/agent/stored-login.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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-<hash of the dir>` 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.
Expand Down
Loading