From f2cc0f24a090ef242d76ab86480368b55bb88c4c Mon Sep 17 00:00:00 2001 From: Cascade Bot Date: Sat, 14 Mar 2026 18:20:11 +0000 Subject: [PATCH] feat(backends): extract shared env filtering into src/backends/shared/envFilter.ts --- src/backends/claude-code/env.ts | 87 ++------ src/backends/codex/env.ts | 83 ++------ src/backends/opencode/env.ts | 85 +++----- src/backends/shared/envFilter.ts | 119 +++++++++++ tests/unit/backends/shared-envFilter.test.ts | 200 +++++++++++++++++++ 5 files changed, 374 insertions(+), 200 deletions(-) create mode 100644 src/backends/shared/envFilter.ts create mode 100644 tests/unit/backends/shared-envFilter.test.ts diff --git a/src/backends/claude-code/env.ts b/src/backends/claude-code/env.ts index a3634921..418edc9e 100644 --- a/src/backends/claude-code/env.ts +++ b/src/backends/claude-code/env.ts @@ -6,28 +6,17 @@ * server-side secrets from leaking into agent environments. */ -import { - PR_SIDECAR_ENV_VAR, - PUSHED_CHANGES_SIDECAR_ENV_VAR, - REVIEW_SIDECAR_ENV_VAR, -} from '../../gadgets/sessionState.js'; import { buildNativeToolPath } from '../nativeToolRuntime.js'; -import { ENV_VAR_NAME as PROGRESS_COMMENT_ENV_VAR } from '../progressState.js'; -import { GITHUB_ACK_COMMENT_ID_ENV_VAR } from '../secretBuilder.js'; +import { + SHARED_ALLOWED_ENV_EXACT, + SHARED_ALLOWED_ENV_PREFIXES, + SHARED_BLOCKED_ENV_EXACT, + filterProcessEnv as sharedFilterProcessEnv, +} from '../shared/envFilter.js'; -/** Exact variable names to pass through. */ +/** Exact variable names to pass through (shared + Claude Code-specific). */ export const ALLOWED_ENV_EXACT = new Set([ - // System - 'HOME', - 'PATH', - 'SHELL', - 'TERM', - 'USER', - 'LOGNAME', - 'LANG', - 'TZ', - 'TMPDIR', - 'HOSTNAME', + ...SHARED_ALLOWED_ENV_EXACT, // Claude auth 'CLAUDE_CODE_OAUTH_TOKEN', @@ -35,51 +24,16 @@ export const ALLOWED_ENV_EXACT = new Set([ // Squint 'SQUINT_DB_PATH', - - // Progress comment state (pre-seeded ack comment ID) - PROGRESS_COMMENT_ENV_VAR, - - // GitHub ack comment ID for claude-code subprocess deletion after PR review - GITHUB_ACK_COMMENT_ID_ENV_VAR, - PR_SIDECAR_ENV_VAR, - PUSHED_CHANGES_SIDECAR_ENV_VAR, - REVIEW_SIDECAR_ENV_VAR, - - // Node - 'NODE_PATH', - 'NODE_EXTRA_CA_CERTS', - 'NODE_TLS_REJECT_UNAUTHORIZED', - - // Editor / color - 'EDITOR', - 'VISUAL', - 'PAGER', - 'FORCE_COLOR', - 'NO_COLOR', - 'TERM_PROGRAM', - 'COLORTERM', ]); /** Prefix patterns — any var starting with one of these passes through. */ -export const ALLOWED_ENV_PREFIXES = ['LC_', 'XDG_', 'GIT_', 'SSH_', 'GPG_', 'DOCKER_'] as const; +export const ALLOWED_ENV_PREFIXES = SHARED_ALLOWED_ENV_PREFIXES; /** * Defense-in-depth denylist. These are blocked even if a future allowlist * change accidentally matches them. */ -export const BLOCKED_ENV_EXACT = new Set([ - 'DATABASE_URL', - 'DATABASE_SSL', - 'REDIS_URL', - 'CREDENTIAL_MASTER_KEY', - 'JOB_ID', - 'JOB_TYPE', - 'JOB_DATA', - 'CASCADE_POSTGRES_HOST', - 'CASCADE_POSTGRES_PORT', - 'NODE_OPTIONS', - 'VSCODE_INSPECTOR_OPTIONS', -]); +export const BLOCKED_ENV_EXACT = SHARED_BLOCKED_ENV_EXACT; /** * Filter process.env to only include safe variables for agent subprocesses. @@ -93,21 +47,12 @@ export const BLOCKED_ENV_EXACT = new Set([ export function filterProcessEnv( processEnv: Record, ): Record { - const result: Record = {}; - - for (const [key, value] of Object.entries(processEnv)) { - if (value === undefined) continue; - if (BLOCKED_ENV_EXACT.has(key)) continue; - if (ALLOWED_ENV_EXACT.has(key)) { - result[key] = value; - continue; - } - if (ALLOWED_ENV_PREFIXES.some((prefix) => key.startsWith(prefix))) { - result[key] = value; - } - } - - return result; + return sharedFilterProcessEnv( + processEnv, + ALLOWED_ENV_EXACT, + ALLOWED_ENV_PREFIXES, + BLOCKED_ENV_EXACT, + ); } export function buildClaudeEnv( diff --git a/src/backends/codex/env.ts b/src/backends/codex/env.ts index f90444d7..b7ecd51e 100644 --- a/src/backends/codex/env.ts +++ b/src/backends/codex/env.ts @@ -5,87 +5,34 @@ * explicitly safe host variables, then layer project-scoped secrets on top. */ -import { - PR_SIDECAR_ENV_VAR, - PUSHED_CHANGES_SIDECAR_ENV_VAR, - REVIEW_SIDECAR_ENV_VAR, -} from '../../gadgets/sessionState.js'; import { buildNativeToolPath } from '../nativeToolRuntime.js'; -import { ENV_VAR_NAME as PROGRESS_COMMENT_ENV_VAR } from '../progressState.js'; -import { GITHUB_ACK_COMMENT_ID_ENV_VAR } from '../secretBuilder.js'; +import { + SHARED_ALLOWED_ENV_EXACT, + SHARED_ALLOWED_ENV_PREFIXES, + SHARED_BLOCKED_ENV_EXACT, + filterProcessEnv as sharedFilterProcessEnv, +} from '../shared/envFilter.js'; const ALLOWED_ENV_EXACT = new Set([ - // System - 'HOME', - 'PATH', - 'SHELL', - 'TERM', - 'USER', - 'LOGNAME', - 'LANG', - 'TZ', - 'TMPDIR', - 'HOSTNAME', + ...SHARED_ALLOWED_ENV_EXACT, // Codex auth 'OPENAI_API_KEY', - - // Progress/session bridge - PROGRESS_COMMENT_ENV_VAR, - GITHUB_ACK_COMMENT_ID_ENV_VAR, - PR_SIDECAR_ENV_VAR, - PUSHED_CHANGES_SIDECAR_ENV_VAR, - REVIEW_SIDECAR_ENV_VAR, - - // Node - 'NODE_PATH', - 'NODE_EXTRA_CA_CERTS', - 'NODE_TLS_REJECT_UNAUTHORIZED', - - // Editor / color - 'EDITOR', - 'VISUAL', - 'PAGER', - 'FORCE_COLOR', - 'NO_COLOR', - 'TERM_PROGRAM', - 'COLORTERM', ]); -const ALLOWED_ENV_PREFIXES = ['LC_', 'XDG_', 'GIT_', 'SSH_', 'GPG_', 'DOCKER_'] as const; +const ALLOWED_ENV_PREFIXES = SHARED_ALLOWED_ENV_PREFIXES; -const BLOCKED_ENV_EXACT = new Set([ - 'DATABASE_URL', - 'DATABASE_SSL', - 'REDIS_URL', - 'CREDENTIAL_MASTER_KEY', - 'JOB_ID', - 'JOB_TYPE', - 'JOB_DATA', - 'CASCADE_POSTGRES_HOST', - 'CASCADE_POSTGRES_PORT', - 'NODE_OPTIONS', - 'VSCODE_INSPECTOR_OPTIONS', -]); +const BLOCKED_ENV_EXACT = SHARED_BLOCKED_ENV_EXACT; export function filterProcessEnv( processEnv: Record, ): Record { - const result: Record = {}; - - for (const [key, value] of Object.entries(processEnv)) { - if (value === undefined) continue; - if (BLOCKED_ENV_EXACT.has(key)) continue; - if (ALLOWED_ENV_EXACT.has(key)) { - result[key] = value; - continue; - } - if (ALLOWED_ENV_PREFIXES.some((prefix) => key.startsWith(prefix))) { - result[key] = value; - } - } - - return result; + return sharedFilterProcessEnv( + processEnv, + ALLOWED_ENV_EXACT, + ALLOWED_ENV_PREFIXES, + BLOCKED_ENV_EXACT, + ); } export function buildEnv( diff --git a/src/backends/opencode/env.ts b/src/backends/opencode/env.ts index a9b927b7..14d5f225 100644 --- a/src/backends/opencode/env.ts +++ b/src/backends/opencode/env.ts @@ -1,77 +1,40 @@ -import { - PR_SIDECAR_ENV_VAR, - PUSHED_CHANGES_SIDECAR_ENV_VAR, - REVIEW_SIDECAR_ENV_VAR, -} from '../../gadgets/sessionState.js'; +/** + * Environment filtering for OpenCode CLI runs. + * + * Uses the same allowlist posture as other native-tool engines: keep only + * explicitly safe host variables, then layer project-scoped secrets on top. + */ + import { buildNativeToolPath } from '../nativeToolRuntime.js'; -import { ENV_VAR_NAME as PROGRESS_COMMENT_ENV_VAR } from '../progressState.js'; -import { GITHUB_ACK_COMMENT_ID_ENV_VAR } from '../secretBuilder.js'; +import { + SHARED_ALLOWED_ENV_EXACT, + SHARED_ALLOWED_ENV_PREFIXES, + SHARED_BLOCKED_ENV_EXACT, + filterProcessEnv as sharedFilterProcessEnv, +} from '../shared/envFilter.js'; const ALLOWED_ENV_EXACT = new Set([ - 'HOME', - 'PATH', - 'SHELL', - 'TERM', - 'USER', - 'LOGNAME', - 'LANG', - 'TZ', - 'TMPDIR', - 'HOSTNAME', + ...SHARED_ALLOWED_ENV_EXACT, + + // OpenCode auth 'OPENAI_API_KEY', 'ANTHROPIC_API_KEY', 'OPENROUTER_API_KEY', - PROGRESS_COMMENT_ENV_VAR, - GITHUB_ACK_COMMENT_ID_ENV_VAR, - PR_SIDECAR_ENV_VAR, - PUSHED_CHANGES_SIDECAR_ENV_VAR, - REVIEW_SIDECAR_ENV_VAR, - 'NODE_PATH', - 'NODE_EXTRA_CA_CERTS', - 'NODE_TLS_REJECT_UNAUTHORIZED', - 'EDITOR', - 'VISUAL', - 'PAGER', - 'FORCE_COLOR', - 'NO_COLOR', - 'TERM_PROGRAM', - 'COLORTERM', ]); -const ALLOWED_ENV_PREFIXES = ['LC_', 'XDG_', 'GIT_', 'SSH_', 'GPG_', 'DOCKER_'] as const; +const ALLOWED_ENV_PREFIXES = SHARED_ALLOWED_ENV_PREFIXES; -const BLOCKED_ENV_EXACT = new Set([ - 'DATABASE_URL', - 'DATABASE_SSL', - 'REDIS_URL', - 'CREDENTIAL_MASTER_KEY', - 'JOB_ID', - 'JOB_TYPE', - 'JOB_DATA', - 'CASCADE_POSTGRES_HOST', - 'CASCADE_POSTGRES_PORT', - 'NODE_OPTIONS', - 'VSCODE_INSPECTOR_OPTIONS', -]); +const BLOCKED_ENV_EXACT = SHARED_BLOCKED_ENV_EXACT; export function filterProcessEnv( processEnv: Record, ): Record { - const result: Record = {}; - - for (const [key, value] of Object.entries(processEnv)) { - if (value === undefined) continue; - if (BLOCKED_ENV_EXACT.has(key)) continue; - if (ALLOWED_ENV_EXACT.has(key)) { - result[key] = value; - continue; - } - if (ALLOWED_ENV_PREFIXES.some((prefix) => key.startsWith(prefix))) { - result[key] = value; - } - } - - return result; + return sharedFilterProcessEnv( + processEnv, + ALLOWED_ENV_EXACT, + ALLOWED_ENV_PREFIXES, + BLOCKED_ENV_EXACT, + ); } export function buildEnv( diff --git a/src/backends/shared/envFilter.ts b/src/backends/shared/envFilter.ts new file mode 100644 index 00000000..1f352ca3 --- /dev/null +++ b/src/backends/shared/envFilter.ts @@ -0,0 +1,119 @@ +/** + * Shared environment variable filtering utilities for native-tool engine subprocesses. + * + * Uses an allowlist approach: only explicitly approved variables pass through + * from the host process. This prevents DATABASE_URL, REDIS_URL, and other + * server-side secrets from leaking into agent environments. + * + * Each engine imports the shared sets and merges in its own engine-specific + * allowed variables before calling filterProcessEnv(). + */ + +import { + PR_SIDECAR_ENV_VAR, + PUSHED_CHANGES_SIDECAR_ENV_VAR, + REVIEW_SIDECAR_ENV_VAR, +} from '../../gadgets/sessionState.js'; +import { ENV_VAR_NAME as PROGRESS_COMMENT_ENV_VAR } from '../progressState.js'; +import { GITHUB_ACK_COMMENT_ID_ENV_VAR } from '../secretBuilder.js'; + +/** + * Defense-in-depth denylist. These are blocked even if a future allowlist + * change accidentally matches them. + */ +export const SHARED_BLOCKED_ENV_EXACT = new Set([ + 'DATABASE_URL', + 'DATABASE_SSL', + 'REDIS_URL', + 'CREDENTIAL_MASTER_KEY', + 'JOB_ID', + 'JOB_TYPE', + 'JOB_DATA', + 'CASCADE_POSTGRES_HOST', + 'CASCADE_POSTGRES_PORT', + 'NODE_OPTIONS', + 'VSCODE_INSPECTOR_OPTIONS', +]); + +/** + * Exact variable names shared across all engines. + * Engines extend this set with their own auth vars. + */ +export const SHARED_ALLOWED_ENV_EXACT = new Set([ + // System + 'HOME', + 'PATH', + 'SHELL', + 'TERM', + 'USER', + 'LOGNAME', + 'LANG', + 'TZ', + 'TMPDIR', + 'HOSTNAME', + + // Progress comment state (pre-seeded ack comment ID) + PROGRESS_COMMENT_ENV_VAR, + + // GitHub ack comment ID for subprocess deletion after PR review + GITHUB_ACK_COMMENT_ID_ENV_VAR, + PR_SIDECAR_ENV_VAR, + PUSHED_CHANGES_SIDECAR_ENV_VAR, + REVIEW_SIDECAR_ENV_VAR, + + // Node + 'NODE_PATH', + 'NODE_EXTRA_CA_CERTS', + 'NODE_TLS_REJECT_UNAUTHORIZED', + + // Editor / color + 'EDITOR', + 'VISUAL', + 'PAGER', + 'FORCE_COLOR', + 'NO_COLOR', + 'TERM_PROGRAM', + 'COLORTERM', +]); + +/** Prefix patterns — any var starting with one of these passes through. */ +export const SHARED_ALLOWED_ENV_PREFIXES = [ + 'LC_', + 'XDG_', + 'GIT_', + 'SSH_', + 'GPG_', + 'DOCKER_', +] as const; + +/** + * Filter process.env to only include safe variables for agent subprocesses. + * + * Resolution order per key: + * 1. If in blockedEnvExact → skip + * 2. If in allowedEnvExact → include + * 3. If matches any allowedEnvPrefixes → include + * 4. Otherwise → skip + */ +export function filterProcessEnv( + processEnv: Record, + allowedEnvExact: Set = SHARED_ALLOWED_ENV_EXACT, + allowedEnvPrefixes: ReadonlyArray = SHARED_ALLOWED_ENV_PREFIXES, + blockedEnvExact: Set = SHARED_BLOCKED_ENV_EXACT, +): Record { + const result: Record = {}; + + for (const [key, value] of Object.entries(processEnv)) { + if (value === undefined) continue; + if (blockedEnvExact.has(key)) continue; + if (allowedEnvExact.has(key)) { + result[key] = value; + continue; + } + if (allowedEnvPrefixes.some((prefix) => key.startsWith(prefix))) { + result[key] = value; + } + } + + return result; +} diff --git a/tests/unit/backends/shared-envFilter.test.ts b/tests/unit/backends/shared-envFilter.test.ts new file mode 100644 index 00000000..61ec6894 --- /dev/null +++ b/tests/unit/backends/shared-envFilter.test.ts @@ -0,0 +1,200 @@ +import { describe, expect, it } from 'vitest'; +import { GITHUB_ACK_COMMENT_ID_ENV_VAR } from '../../../src/backends/secretBuilder.js'; +import { + SHARED_ALLOWED_ENV_EXACT, + SHARED_ALLOWED_ENV_PREFIXES, + SHARED_BLOCKED_ENV_EXACT, + filterProcessEnv, +} from '../../../src/backends/shared/envFilter.js'; + +describe('filterProcessEnv (shared)', () => { + it('passes through exact-match shared allowed vars', () => { + const input: Record = { + HOME: '/home/user', + PATH: '/usr/bin', + SHELL: '/bin/bash', + TERM: 'xterm-256color', + USER: 'testuser', + LANG: 'en_US.UTF-8', + NODE_PATH: '/usr/lib/node', + EDITOR: 'vim', + }; + + const result = filterProcessEnv(input); + + for (const [key, value] of Object.entries(input)) { + expect(result[key]).toBe(value); + } + }); + + it('passes through prefix-matched vars', () => { + const input: Record = { + LC_ALL: 'en_US.UTF-8', + LC_CTYPE: 'UTF-8', + XDG_CONFIG_HOME: '/home/user/.config', + GIT_AUTHOR_NAME: 'Test User', + GIT_COMMITTER_EMAIL: 'test@example.com', + SSH_AUTH_SOCK: '/tmp/ssh-agent.sock', + SSH_AGENT_PID: '12345', + GPG_TTY: '/dev/pts/0', + DOCKER_HOST: 'unix:///var/run/docker.sock', + }; + + const result = filterProcessEnv(input); + + for (const [key, value] of Object.entries(input)) { + expect(result[key]).toBe(value); + } + }); + + it('blocks all SHARED_BLOCKED_ENV_EXACT vars by default', () => { + const input: Record = {}; + for (const key of SHARED_BLOCKED_ENV_EXACT) { + input[key] = 'some-value'; + } + + const result = filterProcessEnv(input); + + for (const key of SHARED_BLOCKED_ENV_EXACT) { + expect(result[key]).toBeUndefined(); + } + }); + + it('blocks DATABASE_URL specifically', () => { + const result = filterProcessEnv({ DATABASE_URL: 'postgres://user:pass@host:5432/db' }); + expect(result.DATABASE_URL).toBeUndefined(); + }); + + it('blocks REDIS_URL specifically', () => { + const result = filterProcessEnv({ REDIS_URL: 'redis://localhost:6379' }); + expect(result.REDIS_URL).toBeUndefined(); + }); + + it('blocks NODE_OPTIONS and VSCODE_INSPECTOR_OPTIONS', () => { + const result = filterProcessEnv({ + NODE_OPTIONS: '--inspect=9229', + VSCODE_INSPECTOR_OPTIONS: '{"some":"config"}', + }); + expect(result.NODE_OPTIONS).toBeUndefined(); + expect(result.VSCODE_INSPECTOR_OPTIONS).toBeUndefined(); + }); + + it('drops unknown vars not in any allowlist', () => { + const result = filterProcessEnv({ + MY_CUSTOM_SECRET: 'secret', + TRELLO_TOKEN: 'token123', + AWS_SECRET_ACCESS_KEY: 'aws-secret', + STRIPE_SECRET_KEY: 'sk_live_123', + }); + + expect(result.MY_CUSTOM_SECRET).toBeUndefined(); + expect(result.TRELLO_TOKEN).toBeUndefined(); + expect(result.AWS_SECRET_ACCESS_KEY).toBeUndefined(); + expect(result.STRIPE_SECRET_KEY).toBeUndefined(); + }); + + it('skips entries with undefined values', () => { + const result = filterProcessEnv({ + HOME: undefined as unknown as string, + PATH: '/usr/bin', + }); + + expect(result.HOME).toBeUndefined(); + expect(result.PATH).toBe('/usr/bin'); + }); + + it('returns empty object for empty input', () => { + expect(filterProcessEnv({})).toEqual({}); + }); + + it('blocked vars take precedence over allowed prefixes', () => { + const result = filterProcessEnv({ + DATABASE_URL: 'postgres://localhost', + DATABASE_SSL: 'false', + }); + expect(result.DATABASE_URL).toBeUndefined(); + expect(result.DATABASE_SSL).toBeUndefined(); + }); + + it('combines exact + prefix matches correctly', () => { + const result = filterProcessEnv({ + HOME: '/home/user', + PATH: '/usr/bin', + LC_ALL: 'C', + GIT_DIR: '/repo/.git', + DATABASE_URL: 'postgres://host/db', + MY_SECRET: 'hidden', + }); + + expect(Object.keys(result).sort()).toEqual(['GIT_DIR', 'HOME', 'LC_ALL', 'PATH']); + }); + + it('accepts custom allowedEnvExact to include engine-specific vars', () => { + const customAllowed = new Set([...SHARED_ALLOWED_ENV_EXACT, 'OPENAI_API_KEY']); + const result = filterProcessEnv( + { HOME: '/home/user', OPENAI_API_KEY: 'sk-test', MY_SECRET: 'hidden' }, + customAllowed, + ); + + expect(result.HOME).toBe('/home/user'); + expect(result.OPENAI_API_KEY).toBe('sk-test'); + expect(result.MY_SECRET).toBeUndefined(); + }); + + it('accepts custom blockedEnvExact to block additional vars', () => { + const customBlocked = new Set([...SHARED_BLOCKED_ENV_EXACT, 'HOME']); + const result = filterProcessEnv( + { HOME: '/home/user', PATH: '/usr/bin' }, + undefined, + undefined, + customBlocked, + ); + + expect(result.HOME).toBeUndefined(); + expect(result.PATH).toBe('/usr/bin'); + }); +}); + +describe('SHARED_ALLOWED_ENV_EXACT', () => { + it('does not overlap with SHARED_BLOCKED_ENV_EXACT', () => { + for (const key of SHARED_BLOCKED_ENV_EXACT) { + expect(SHARED_ALLOWED_ENV_EXACT.has(key)).toBe(false); + } + }); + + it('includes CASCADE_GITHUB_ACK_COMMENT_ID', () => { + expect(SHARED_ALLOWED_ENV_EXACT.has(GITHUB_ACK_COMMENT_ID_ENV_VAR)).toBe(true); + }); + + it('passes CASCADE_GITHUB_ACK_COMMENT_ID through filterProcessEnv', () => { + const result = filterProcessEnv({ [GITHUB_ACK_COMMENT_ID_ENV_VAR]: '12345' }); + expect(result[GITHUB_ACK_COMMENT_ID_ENV_VAR]).toBe('12345'); + }); +}); + +describe('SHARED_ALLOWED_ENV_PREFIXES', () => { + it('are all uppercase with trailing underscore', () => { + for (const prefix of SHARED_ALLOWED_ENV_PREFIXES) { + expect(prefix).toMatch(/^[A-Z_]+_$/); + } + }); + + it('includes LC_, XDG_, GIT_, SSH_, GPG_, DOCKER_', () => { + const prefixes = [...SHARED_ALLOWED_ENV_PREFIXES]; + expect(prefixes).toContain('LC_'); + expect(prefixes).toContain('XDG_'); + expect(prefixes).toContain('GIT_'); + expect(prefixes).toContain('SSH_'); + expect(prefixes).toContain('GPG_'); + expect(prefixes).toContain('DOCKER_'); + }); +}); + +describe('SHARED_BLOCKED_ENV_EXACT', () => { + it('contains critical server-side secrets', () => { + expect(SHARED_BLOCKED_ENV_EXACT.has('DATABASE_URL')).toBe(true); + expect(SHARED_BLOCKED_ENV_EXACT.has('REDIS_URL')).toBe(true); + expect(SHARED_BLOCKED_ENV_EXACT.has('CREDENTIAL_MASTER_KEY')).toBe(true); + expect(SHARED_BLOCKED_ENV_EXACT.has('NODE_OPTIONS')).toBe(true); + }); +});