Merge pull request #170 from gnanam1990/fix/security-issue-42

security: fix 5 findings from issue #42 — env leak, ant gate, depth DoS, URL parse, CA cert

src/bridge/sessionRunner.test.ts

@@ -0,0 +1,85 @@
+import { expect, test } from 'bun:test'
+import { buildChildEnv } from './sessionRunner.ts'
+
+// Finding #42-1: sessionRunner spreads the full parent process.env into the
+// child process environment, leaking API keys, DB credentials, proxy secrets.
+// Only CLAUDE_CODE_OAUTH_TOKEN was stripped. Fix: explicit allowlist.
+
+const baseOpts = {
+  accessToken: 'test-access-token',
+  useCcrV2: false as const,
+}
+
+test('buildChildEnv does not leak ANTHROPIC_API_KEY to child', () => {
+  const parentEnv = {
+    PATH: '/usr/bin',
+    HOME: '/home/user',
+    ANTHROPIC_API_KEY: 'sk-ant-secret-key',
+    CLAUDE_CODE_SESSION_ACCESS_TOKEN: 'will-be-overwritten',
+  }
+  const env = buildChildEnv(parentEnv, baseOpts)
+  expect(env.ANTHROPIC_API_KEY).toBeUndefined()
+})
+
+test('buildChildEnv does not leak OPENAI_API_KEY to child', () => {
+  const parentEnv = {
+    PATH: '/usr/bin',
+    HOME: '/home/user',
+    OPENAI_API_KEY: 'sk-openai-secret',
+  }
+  const env = buildChildEnv(parentEnv, baseOpts)
+  expect(env.OPENAI_API_KEY).toBeUndefined()
+})
+
+test('buildChildEnv does not leak arbitrary secrets to child', () => {
+  const parentEnv = {
+    PATH: '/usr/bin',
+    HOME: '/home/user',
+    DB_PASSWORD: 'super-secret',
+    AWS_SECRET_ACCESS_KEY: 'aws-secret',
+    GITHUB_TOKEN: 'ghp_token',
+  }
+  const env = buildChildEnv(parentEnv, baseOpts)
+  expect(env.DB_PASSWORD).toBeUndefined()
+  expect(env.AWS_SECRET_ACCESS_KEY).toBeUndefined()
+  expect(env.GITHUB_TOKEN).toBeUndefined()
+})
+
+test('buildChildEnv includes PATH and HOME from parent', () => {
+  const parentEnv = {
+    PATH: '/usr/bin:/usr/local/bin',
+    HOME: '/home/user',
+    ANTHROPIC_API_KEY: 'sk-secret',
+  }
+  const env = buildChildEnv(parentEnv, baseOpts)
+  expect(env.PATH).toBe('/usr/bin:/usr/local/bin')
+  expect(env.HOME).toBe('/home/user')
+})
+
+test('buildChildEnv sets CLAUDE_CODE_SESSION_ACCESS_TOKEN from opts', () => {
+  const env = buildChildEnv({ PATH: '/usr/bin' }, { ...baseOpts, accessToken: 'my-token' })
+  expect(env.CLAUDE_CODE_SESSION_ACCESS_TOKEN).toBe('my-token')
+})
+
+test('buildChildEnv sets CLAUDE_CODE_ENVIRONMENT_KIND to bridge', () => {
+  const env = buildChildEnv({ PATH: '/usr/bin' }, baseOpts)
+  expect(env.CLAUDE_CODE_ENVIRONMENT_KIND).toBe('bridge')
+})
+
+test('buildChildEnv does not pass CLAUDE_CODE_OAUTH_TOKEN to child', () => {
+  const parentEnv = {
+    PATH: '/usr/bin',
+    CLAUDE_CODE_OAUTH_TOKEN: 'oauth-token-to-strip',
+  }
+  const env = buildChildEnv(parentEnv, baseOpts)
+  expect(env.CLAUDE_CODE_OAUTH_TOKEN).toBeUndefined()
+})
+
+test('buildChildEnv sets CCR v2 vars when useCcrV2 is true', () => {
+  const env = buildChildEnv(
+    { PATH: '/usr/bin' },
+    { accessToken: 'tok', useCcrV2: true, workerEpoch: 42 },
+  )
+  expect(env.CLAUDE_CODE_USE_CCR_V2).toBe('1')
+  expect(env.CLAUDE_CODE_WORKER_EPOCH).toBe('42')
+})

src/bridge/sessionRunner.ts

@@ -16,6 +16,69 @@ import type {
 const MAX_ACTIVITIES = 10
 const MAX_STDERR_LINES = 10
 
+/**
+ * Safe OS and runtime variables that the child process needs to function.
+ * Everything else (API keys, DB passwords, proxy secrets, etc.) must not
+ * be inherited — the child authenticates via CLAUDE_CODE_SESSION_ACCESS_TOKEN.
+ */
+const CHILD_ENV_ALLOWLIST = new Set([
+  // System / shell
+  'PATH', 'HOME', 'USERPROFILE', 'HOMEPATH', 'HOMEDRIVE',
+  'USERNAME', 'USER', 'LOGNAME',
+  'TEMP', 'TMP', 'TMPDIR',
+  'SYSTEMROOT', 'SYSTEMDRIVE', 'COMSPEC', 'WINDIR',
+  'LANG', 'LC_ALL', 'LC_CTYPE',
+  // Node.js runtime
+  'NODE_OPTIONS', 'NODE_PATH', 'NODE_ENV',
+  // OpenClaude session / bridge (non-secret)
+  'CLAUDE_CODE_ENVIRONMENT_KIND',
+  'CLAUDE_CODE_FORCE_SANDBOX',
+  'CLAUDE_CODE_BUBBLEWRAP',
+  'CLAUDE_CODE_ENTRYPOINT',
+  'CLAUDE_CODE_COORDINATOR_MODE',
+  'CLAUDE_CODE_PERMISSIONS_VERSION',
+  'CLAUDE_CODE_PERMISSIONS_SETTING',
+  // Display / terminal
+  'TERM', 'COLORTERM', 'FORCE_COLOR', 'NO_COLOR',
+])
+
+type BuildChildEnvOpts = {
+  accessToken: string
+  useCcrV2: boolean
+  workerEpoch?: number
+  sandbox?: boolean
+}
+
+/**
+ * Build the environment for the child CC process from an explicit allowlist.
+ * This prevents the parent's API keys and credentials from leaking to the child.
+ */
+export function buildChildEnv(
+  parentEnv: NodeJS.ProcessEnv,
+  opts: BuildChildEnvOpts,
+): NodeJS.ProcessEnv {
+  // Start from allowlisted parent vars only
+  const env: NodeJS.ProcessEnv = {}
+  for (const key of Object.keys(parentEnv)) {
+    if (CHILD_ENV_ALLOWLIST.has(key)) {
+      env[key] = parentEnv[key]
+    }
+  }
+
+  // Bridge-required overrides
+  env.CLAUDE_CODE_OAUTH_TOKEN = undefined // explicitly strip
+  env.CLAUDE_CODE_ENVIRONMENT_KIND = 'bridge'
+  if (opts.sandbox) env.CLAUDE_CODE_FORCE_SANDBOX = '1'
+  env.CLAUDE_CODE_SESSION_ACCESS_TOKEN = opts.accessToken
+  env.CLAUDE_CODE_POST_FOR_SESSION_INGRESS_V2 = '1'
+  if (opts.useCcrV2) {
+    env.CLAUDE_CODE_USE_CCR_V2 = '1'
+    env.CLAUDE_CODE_WORKER_EPOCH = String(opts.workerEpoch)
+  }
+
+  return env
+}
+
 /**
  * Sanitize a session ID for use in file names.
  * Strips any characters that could cause path traversal (e.g. `../`, `/`)
@@ -303,24 +366,12 @@ export function createSessionSpawner(deps: SessionSpawnerDeps): SessionSpawner {
           : []),
       ]
 
-      const env: NodeJS.ProcessEnv = {
+      const env = buildChildEnv(deps.env, {
-        ...deps.env,
+        accessToken: opts.accessToken,
-        // Strip the bridge's OAuth token so the child CC process uses
+        useCcrV2: opts.useCcrV2,
-        // the session access token for inference instead.
+        workerEpoch: opts.workerEpoch,
-        CLAUDE_CODE_OAUTH_TOKEN: undefined,
+        sandbox: deps.sandbox,
-        CLAUDE_CODE_ENVIRONMENT_KIND: 'bridge',
+      })
-        ...(deps.sandbox && { CLAUDE_CODE_FORCE_SANDBOX: '1' }),
-        CLAUDE_CODE_SESSION_ACCESS_TOKEN: opts.accessToken,
-        // v1: HybridTransport (WS reads + POST writes) to Session-Ingress.
-        // Harmless in v2 mode — transportUtils checks CLAUDE_CODE_USE_CCR_V2 first.
-        CLAUDE_CODE_POST_FOR_SESSION_INGRESS_V2: '1',
-        // v2: SSETransport + CCRClient to CCR's /v1/code/sessions/* endpoints.
-        // Same env vars environment-manager sets in the container path.
-        ...(opts.useCcrV2 && {
-          CLAUDE_CODE_USE_CCR_V2: '1',
-          CLAUDE_CODE_WORKER_EPOCH: String(opts.workerEpoch),
-        }),
-      }
 
       deps.onDebug(
         `[bridge:session] Spawning sessionId=${opts.sessionId} sdkUrl=${opts.sdkUrl} accessToken=${opts.accessToken ? 'present' : 'MISSING'}`,

src/bridge/workSecret.test.ts

@@ -0,0 +1,36 @@
+import { expect, test } from 'bun:test'
+import { buildSdkUrl } from './workSecret.ts'
+
+// Finding #42-5: buildSdkUrl uses string.includes() on the full URL,
+// so a remote URL containing "localhost" in its path gets ws:// (unencrypted).
+
+test('buildSdkUrl uses wss for remote URL that contains localhost in path', () => {
+  const url = buildSdkUrl('https://remote.example.com/proxy/localhost/api', 'sess-1')
+  expect(url).toContain('wss://')
+  expect(url).not.toContain('ws://')
+})
+
+test('buildSdkUrl uses ws for actual localhost hostname', () => {
+  const url = buildSdkUrl('http://localhost:8080', 'sess-1')
+  expect(url).toContain('ws://')
+})
+
+test('buildSdkUrl uses ws for 127.0.0.1 hostname', () => {
+  const url = buildSdkUrl('http://127.0.0.1:3000', 'sess-1')
+  expect(url).toContain('ws://')
+})
+
+test('buildSdkUrl uses wss for regular remote hostname', () => {
+  const url = buildSdkUrl('https://api.example.com', 'sess-1')
+  expect(url).toContain('wss://')
+})
+
+test('buildSdkUrl uses v2 path for localhost', () => {
+  const url = buildSdkUrl('http://localhost:8080', 'sess-abc')
+  expect(url).toContain('/v2/session_ingress/ws/sess-abc')
+})
+
+test('buildSdkUrl uses v1 path for remote', () => {
+  const url = buildSdkUrl('https://api.example.com', 'sess-abc')
+  expect(url).toContain('/v1/session_ingress/ws/sess-abc')
+})

src/bridge/workSecret.ts

@@ -39,8 +39,8 @@ export function decodeWorkSecret(secret: string): WorkSecret {
  * and /v1/ for production (Envoy rewrites /v1/ → /v2/).
  */
 export function buildSdkUrl(apiBaseUrl: string, sessionId: string): string {
-  const isLocalhost =
+  const hostname = new URL(apiBaseUrl).hostname
-    apiBaseUrl.includes('localhost') || apiBaseUrl.includes('127.0.0.1')
+  const isLocalhost = hostname === 'localhost' || hostname === '127.0.0.1'
   const protocol = isLocalhost ? 'ws' : 'wss'
   const version = isLocalhost ? 'v2' : 'v1'
   const host = apiBaseUrl.replace(/^https?:\/\//, '').replace(/\/+$/, '')

src/memdir/memoryScan.test.ts

@@ -0,0 +1,59 @@
+import { afterEach, expect, test } from 'bun:test'
+import { mkdtemp, mkdir, writeFile, rm } from 'fs/promises'
+import { join } from 'path'
+import { tmpdir } from 'os'
+import { scanMemoryFiles } from './memoryScan.ts'
+
+// Finding #42-3: readdir({ recursive: true }) has no depth limit.
+// A deeply nested directory in the memory dir causes a full unbounded walk.
+
+let tempDir: string
+
+afterEach(async () => {
+  if (tempDir) {
+    await rm(tempDir, { recursive: true, force: true })
+  }
+})
+
+test('scanMemoryFiles finds .md files at shallow depth', async () => {
+  tempDir = await mkdtemp(join(tmpdir(), 'memoryScan-'))
+  await writeFile(join(tempDir, 'note.md'), '---\nname: test\ntype: user\n---\nContent')
+
+  const controller = new AbortController()
+  const result = await scanMemoryFiles(tempDir, controller.signal)
+
+  expect(result.length).toBe(1)
+  expect(result[0].filename).toBe('note.md')
+})
+
+test('scanMemoryFiles ignores MEMORY.md', async () => {
+  tempDir = await mkdtemp(join(tmpdir(), 'memoryScan-'))
+  await writeFile(join(tempDir, 'MEMORY.md'), '# index')
+  await writeFile(join(tempDir, 'user_role.md'), '---\nname: role\ntype: user\n---\nContent')
+
+  const controller = new AbortController()
+  const result = await scanMemoryFiles(tempDir, controller.signal)
+
+  expect(result.length).toBe(1)
+  expect(result[0].filename).toBe('user_role.md')
+})
+
+test('scanMemoryFiles does not return .md files nested beyond max depth', async () => {
+  tempDir = await mkdtemp(join(tmpdir(), 'memoryScan-'))
+
+  // Shallow file - should be found
+  await writeFile(join(tempDir, 'shallow.md'), '---\nname: shallow\ntype: user\n---\nContent')
+
+  // Deeply nested file (depth 5) - should be excluded
+  const deepDir = join(tempDir, 'd1', 'd2', 'd3', 'd4', 'd5')
+  await mkdir(deepDir, { recursive: true })
+  await writeFile(join(deepDir, 'deep.md'), '---\nname: deep\ntype: user\n---\nContent')
+
+  const controller = new AbortController()
+  const result = await scanMemoryFiles(tempDir, controller.signal)
+
+  const filenames = result.map(r => r.filename)
+  expect(filenames).toContain('shallow.md')
+  // The deeply nested file must not appear
+  expect(filenames.some(f => f.includes('deep.md'))).toBe(false)
+})

src/memdir/memoryScan.ts

@@ -38,8 +38,15 @@ export async function scanMemoryFiles(
 ): Promise<MemoryHeader[]> {
   try {
     const entries = await readdir(memoryDir, { recursive: true })
+    // Limit depth to 3 levels to prevent DoS from deep/symlinked directory trees.
+    // Relative paths from readdir use the OS separator, so count separators.
+    const sep = require('path').sep as string
+    const MAX_DEPTH = 3
     const mdFiles = entries.filter(
-      f => f.endsWith('.md') && basename(f) !== 'MEMORY.md',
+      f =>
+        f.endsWith('.md') &&
+        basename(f) !== 'MEMORY.md' &&
+        (f.split(sep).length - 1) < MAX_DEPTH,
     )
 
     const headerResults = await Promise.allSettled(

src/setup.ts

@@ -6,6 +6,7 @@ import {
   type AnalyticsMetadata_I_VERIFIED_THIS_IS_NOT_CODE_OR_FILEPATHS,
   logEvent,
 } from 'src/services/analytics/index.js'
+import { isAntEmployee } from 'src/utils/buildConfig.js'
 import { getCwd } from 'src/utils/cwd.js'
 import { checkForReleaseNotes } from 'src/utils/releaseNotes.js'
 import { setCwd } from 'src/utils/Shell.js'
@@ -334,7 +335,7 @@ export async function setup(
   // overhead. NOT an early-return: the --dangerously-skip-permissions safety
   // gate, tengu_started beacon, and apiKeyHelper prefetch below must still run.
   if (!isBareMode()) {
-    if (process.env.USER_TYPE === 'ant') {
+    if (isAntEmployee()) {
       // Prime repo classification cache for auto-undercover mode. Default is
       // undercover ON until proven internal; if this resolves to internal, clear
       // the prompt cache so the next turn picks up the OFF state.
@@ -414,7 +415,7 @@ export async function setup(
     }
 
     if (
-      process.env.USER_TYPE === 'ant' &&
+      isAntEmployee() &&
       // Skip for Desktop's local agent mode — same trust model as CCR/BYOC
       // (trusted Anthropic-managed launcher intentionally pre-approving everything).
       // Precedent: permissionSetup.ts:861, applySettingsChange.ts:55 (PR #19116)

src/state/onChangeAppState.ts

@@ -1,4 +1,5 @@
 import { setMainLoopModelOverride } from '../bootstrap/state.js'
+import { isAntEmployee } from '../utils/buildConfig.js'
 import {
   clearApiKeyHelperCache,
   clearAwsCredentialsCache,
@@ -140,7 +141,7 @@ export function onChangeAppState({
   }
 
   // tungstenPanelVisible (ant-only tmux panel sticky toggle)
-  if (process.env.USER_TYPE === 'ant') {
+  if (isAntEmployee()) {
     if (
       newState.tungstenPanelVisible !== oldState.tungstenPanelVisible &&
       newState.tungstenPanelVisible !== undefined &&

src/upstreamproxy/upstreamproxy.test.ts

@@ -0,0 +1,42 @@
+import { expect, test } from 'bun:test'
+import { isValidPemContent } from './upstreamproxy.ts'
+
+// Finding #42-6: The CA cert downloaded from the upstream proxy is written
+// to disk without validation. A compromised server could send arbitrary data.
+// Fix: validate it contains only valid PEM certificate blocks before writing.
+
+test('isValidPemContent returns true for a valid PEM certificate block', () => {
+  const pem = [
+    '-----BEGIN CERTIFICATE-----',
+    'MIICpDCCAYwCCQDU+pQ4pHgSpDANBgkqhkiG9w0BAQsFADAUMRIwEAYDVQQDDAls',
+    'b2NhbGhvc3QwHhcNMjMwMTAxMDAwMDAwWhcNMjQwMTAxMDAwMDAwWjAUMRIwEAYD',
+    'VQQDDAlsb2NhbGhvc3QwggEiMA0GCSqGSIb3DQEBAQUAA4IBDwAwggEKAoIBAQC7',
+    '-----END CERTIFICATE-----',
+  ].join('\n')
+
+  expect(isValidPemContent(pem)).toBe(true)
+})
+
+test('isValidPemContent returns true for multiple PEM blocks', () => {
+  const block = '-----BEGIN CERTIFICATE-----\nABCD\n-----END CERTIFICATE-----'
+  const pem = `${block}\n${block}`
+  expect(isValidPemContent(pem)).toBe(true)
+})
+
+test('isValidPemContent returns false for arbitrary text', () => {
+  expect(isValidPemContent('Hello world')).toBe(false)
+  expect(isValidPemContent('<html><body>error</body></html>')).toBe(false)
+  expect(isValidPemContent('{"error":"unauthorized"}')).toBe(false)
+})
+
+test('isValidPemContent returns false for empty string', () => {
+  expect(isValidPemContent('')).toBe(false)
+})
+
+test('isValidPemContent returns false for whitespace only', () => {
+  expect(isValidPemContent('   \n   ')).toBe(false)
+})
+
+test('isValidPemContent returns false for malformed PEM (no end marker)', () => {
+  expect(isValidPemContent('-----BEGIN CERTIFICATE-----\nABCD')).toBe(false)
+})

src/upstreamproxy/upstreamproxy.ts

@@ -203,6 +203,18 @@ export function resetUpstreamProxyForTests(): void {
   state = { enabled: false }
 }
 
+/**
+ * Validate that a string contains only well-formed PEM certificate blocks.
+ * Used to guard against a compromised upstream proxy sending arbitrary data
+ * that would be written into the system CA bundle.
+ */
+export function isValidPemContent(content: string): boolean {
+  if (!content || !content.trim()) return false
+  const pemBlockRegex =
+    /-----BEGIN CERTIFICATE-----[\s\S]+?-----END CERTIFICATE-----/g
+  return pemBlockRegex.test(content)
+}
+
 async function readToken(path: string): Promise<string | null> {
   try {
     const raw = await readFile(path, 'utf8')
@@ -271,6 +283,13 @@ async function downloadCaBundle(
       return false
     }
     const ccrCa = await resp.text()
+    if (!isValidPemContent(ccrCa)) {
+      logForDebugging(
+        `[upstreamproxy] ca-cert response is not valid PEM; proxy disabled`,
+        { level: 'warn' },
+      )
+      return false
+    }
     const systemCa = await readFile(systemCaPath, 'utf8').catch(() => '')
     await mkdir(join(outPath, '..'), { recursive: true })
     await writeFile(outPath, systemCa + '\n' + ccrCa, 'utf8')

src/utils/buildConfig.test.ts

@@ -0,0 +1,20 @@
+import { expect, test } from 'bun:test'
+import { isAntEmployee } from './buildConfig.ts'
+
+// Finding #42-2: process.env.USER_TYPE === 'ant' is checked directly in multiple
+// places, allowing any external user to activate Anthropic-internal code paths.
+// In OpenClaude, this must always be false regardless of env var.
+
+test('isAntEmployee always returns false in OpenClaude regardless of USER_TYPE env var', () => {
+  const original = process.env.USER_TYPE
+  process.env.USER_TYPE = 'ant'
+  expect(isAntEmployee()).toBe(false)
+  process.env.USER_TYPE = original
+})
+
+test('isAntEmployee returns false even when USER_TYPE is unset', () => {
+  const original = process.env.USER_TYPE
+  delete process.env.USER_TYPE
+  expect(isAntEmployee()).toBe(false)
+  process.env.USER_TYPE = original
+})

src/utils/buildConfig.ts

@@ -0,0 +1,18 @@
+/**
+ * OpenClaude build-time constants.
+ *
+ * These replace process.env checks that were only meaningful in Anthropic's
+ * internal build. In OpenClaude all such gates are permanently disabled so
+ * external users cannot activate internal code paths by setting env vars.
+ */
+
+/**
+ * Always false in OpenClaude.
+ * Replaces all `process.env.USER_TYPE === 'ant'` checks so that no external
+ * user can activate Anthropic-internal features (commit attribution hooks,
+ * system-prompt section clearing, dangerously-skip-permissions bypass, etc.)
+ * by setting USER_TYPE in their shell environment.
+ */
+export function isAntEmployee(): boolean {
+  return false
+}