diff --git a/docs/authentication-architecture.md b/docs/authentication-architecture.md index 39213c65f..f9e33392c 100644 --- a/docs/authentication-architecture.md +++ b/docs/authentication-architecture.md @@ -51,8 +51,8 @@ AWF uses a **3-container architecture** when API proxy mode is enabled: │ Ports: │ │ http://172.30.0.30:10000 │ │ - 10000 (OpenAI proxy) │◄──────│ ✓ COPILOT_API_URL= │ │ - 10001 (Anthropic proxy) │ │ http://172.30.0.30:10002 │ -│ - 10002 (Copilot proxy) │ │ ✓ GITHUB_TOKEN=ghp_... │ -│ - 10003 (Gemini proxy) │ │ (protected by one-shot-token) │ +│ - 10002 (Copilot proxy) │ │ ✗ GITHUB_TOKEN — excluded │ +│ - 10003 (Gemini proxy) │ │ (not present in agent env) │ │ Injects auth headers: │ │ User command execution: │ │ - x-api-key: sk-ant-... │ │ claude-code, copilot, etc. │ │ - Authorization: Bearer sk-... │ └──────────────────────────────────┘ @@ -125,8 +125,8 @@ agent: - COPILOT_API_URL=http://172.30.0.30:10002 - GOOGLE_GEMINI_BASE_URL=http://172.30.0.30:10003 - GEMINI_API_BASE_URL=http://172.30.0.30:10003 - # GitHub token for MCP servers (protected separately) - - GITHUB_TOKEN=ghp_... + # GITHUB_TOKEN / GH_TOKEN are NOT present — excluded by the API-proxy + # exclusion set to prevent credential extraction via /proc/self/environ networks: awf-net: ipv4_address: 172.30.0.20 @@ -307,7 +307,7 @@ The api-proxy connects to the real APIs (e.g., `api.openai.com`) over standard H **Source:** `containers/agent/one-shot-token/` -While API keys don't exist in the agent container, other tokens (like `GITHUB_TOKEN`) do. AWF uses an `LD_PRELOAD` library to protect these: +While API keys don't exist in the agent container, other tokens may still be present. AWF uses an `LD_PRELOAD` library as defense-in-depth for any token that does reach the container: ```c // Intercept getenv() calls @@ -329,9 +329,9 @@ char* getenv(const char* name) { ``` **Protected tokens by default:** -- `ANTHROPIC_API_KEY`, `CLAUDE_API_KEY` (though not passed to agent when api-proxy is enabled) +- `ANTHROPIC_API_KEY`, `CLAUDE_API_KEY` (not passed to agent when api-proxy is enabled) - `OPENAI_API_KEY`, `OPENAI_KEY` -- `GITHUB_TOKEN`, `GH_TOKEN`, `COPILOT_GITHUB_TOKEN` +- `GITHUB_TOKEN`, `GH_TOKEN`, `COPILOT_GITHUB_TOKEN` (not passed to agent when api-proxy is enabled) - `GITHUB_API_TOKEN`, `GITHUB_PAT`, `GH_ACCESS_TOKEN` - `CODEX_API_KEY` - `COPILOT_PROVIDER_API_KEY` (Copilot BYOK upstream provider key) diff --git a/src/services/agent-environment/env-passthrough.test.ts b/src/services/agent-environment/env-passthrough.test.ts new file mode 100644 index 000000000..a6c00ebde --- /dev/null +++ b/src/services/agent-environment/env-passthrough.test.ts @@ -0,0 +1,148 @@ +import { passthroughHostEnvironment } from './env-passthrough'; +import { WrapperConfig } from '../../types'; + +// Mock the logger to suppress output during tests +jest.mock('../../logger', () => ({ + logger: { + error: jest.fn(), + warn: jest.fn(), + info: jest.fn(), + debug: jest.fn(), + }, +})); + +function makeConfig(overrides: Partial = {}): WrapperConfig { + return { + allowedDomains: [], + ...overrides, + } as WrapperConfig; +} + +describe('passthroughHostEnvironment', () => { + let savedEnv: Record; + + beforeEach(() => { + // Save and clear relevant env vars before each test + savedEnv = {}; + }); + + afterEach(() => { + // Restore env vars + for (const [key, val] of Object.entries(savedEnv)) { + if (val === undefined) { + delete process.env[key]; + } else { + process.env[key] = val; + } + } + }); + + function withEnv(vars: Record, fn: () => void): void { + for (const [key, val] of Object.entries(vars)) { + savedEnv[key] = process.env[key]; + process.env[key] = val; + } + fn(); + } + + describe('alwaysForwardVars respect the exclusion set (root-cause fix)', () => { + it('does not forward GITHUB_TOKEN when it is in the exclusion set', () => { + const environment: Record = {}; + const excludedEnvVars = new Set(['GITHUB_TOKEN']); + + withEnv({ GITHUB_TOKEN: 'ghs_secret' }, () => { + passthroughHostEnvironment({ + config: makeConfig({ enableApiProxy: true }), + environment, + excludedEnvVars, + }); + }); + + expect(environment).not.toHaveProperty('GITHUB_TOKEN'); + }); + + it('does not forward GH_TOKEN when it is in the exclusion set', () => { + const environment: Record = {}; + const excludedEnvVars = new Set(['GH_TOKEN']); + + withEnv({ GH_TOKEN: 'ghs_secret' }, () => { + passthroughHostEnvironment({ + config: makeConfig({ enableApiProxy: true }), + environment, + excludedEnvVars, + }); + }); + + expect(environment).not.toHaveProperty('GH_TOKEN'); + }); + + it('does not forward GITHUB_PERSONAL_ACCESS_TOKEN when it is in the exclusion set', () => { + const environment: Record = {}; + const excludedEnvVars = new Set(['GITHUB_PERSONAL_ACCESS_TOKEN']); + + withEnv({ GITHUB_PERSONAL_ACCESS_TOKEN: 'ghp_secret' }, () => { + passthroughHostEnvironment({ + config: makeConfig({ enableApiProxy: true }), + environment, + excludedEnvVars, + }); + }); + + expect(environment).not.toHaveProperty('GITHUB_PERSONAL_ACCESS_TOKEN'); + }); + + it('forwards GITHUB_TOKEN when it is NOT in the exclusion set', () => { + const environment: Record = {}; + const excludedEnvVars = new Set(); + + withEnv({ GITHUB_TOKEN: 'ghs_allowed' }, () => { + passthroughHostEnvironment({ + config: makeConfig({ enableApiProxy: false }), + environment, + excludedEnvVars, + }); + }); + + expect(environment).toHaveProperty('GITHUB_TOKEN', 'ghs_allowed'); + }); + + it('forwards GH_TOKEN when it is NOT in the exclusion set', () => { + const environment: Record = {}; + const excludedEnvVars = new Set(); + + withEnv({ GH_TOKEN: 'ghs_allowed' }, () => { + passthroughHostEnvironment({ + config: makeConfig({ enableApiProxy: false }), + environment, + excludedEnvVars, + }); + }); + + expect(environment).toHaveProperty('GH_TOKEN', 'ghs_allowed'); + }); + + it('suppresses all three GitHub token aliases together when all are excluded', () => { + const environment: Record = {}; + const excludedEnvVars = new Set(['GITHUB_TOKEN', 'GH_TOKEN', 'GITHUB_PERSONAL_ACCESS_TOKEN']); + + withEnv( + { + GITHUB_TOKEN: 'ghs_tok', + GH_TOKEN: 'ghs_tok2', + GITHUB_PERSONAL_ACCESS_TOKEN: 'ghp_tok', + }, + () => { + passthroughHostEnvironment({ + config: makeConfig({ enableApiProxy: true }), + environment, + excludedEnvVars, + }); + }, + ); + + expect(environment).not.toHaveProperty('GITHUB_TOKEN'); + expect(environment).not.toHaveProperty('GH_TOKEN'); + expect(environment).not.toHaveProperty('GITHUB_PERSONAL_ACCESS_TOKEN'); + }); + }); +}); diff --git a/src/services/agent-environment/env-passthrough.ts b/src/services/agent-environment/env-passthrough.ts index d12153816..f23106eca 100644 --- a/src/services/agent-environment/env-passthrough.ts +++ b/src/services/agent-environment/env-passthrough.ts @@ -56,7 +56,7 @@ export function passthroughHostEnvironment(params: EnvPassthroughParams): void { ] as const; for (const v of alwaysForwardVars) { - if (process.env[v]) { + if (process.env[v] && !excludedEnvVars.has(v)) { environment[v] = process.env[v]!; } } diff --git a/src/services/agent-environment/excluded-vars.test.ts b/src/services/agent-environment/excluded-vars.test.ts index c9feac339..ef16688bb 100644 --- a/src/services/agent-environment/excluded-vars.test.ts +++ b/src/services/agent-environment/excluded-vars.test.ts @@ -105,6 +105,18 @@ describe('buildExclusionSet', () => { it('should exclude GOOGLE_VERTEX_BASE_URL (Vertex AI base URL)', () => { expect(buildExclusionSet(config).has('GOOGLE_VERTEX_BASE_URL')).toBe(true); }); + + it('should exclude GITHUB_TOKEN (credential isolation)', () => { + expect(buildExclusionSet(config).has('GITHUB_TOKEN')).toBe(true); + }); + + it('should exclude GH_TOKEN (credential isolation)', () => { + expect(buildExclusionSet(config).has('GH_TOKEN')).toBe(true); + }); + + it('should exclude GITHUB_PERSONAL_ACCESS_TOKEN (credential isolation)', () => { + expect(buildExclusionSet(config).has('GITHUB_PERSONAL_ACCESS_TOKEN')).toBe(true); + }); }); describe('when enableApiProxy is false', () => { @@ -125,6 +137,18 @@ describe('buildExclusionSet', () => { it('should NOT exclude GEMINI_API_KEY', () => { expect(buildExclusionSet(config).has('GEMINI_API_KEY')).toBe(false); }); + + it('should NOT exclude GITHUB_TOKEN', () => { + expect(buildExclusionSet(config).has('GITHUB_TOKEN')).toBe(false); + }); + + it('should NOT exclude GH_TOKEN', () => { + expect(buildExclusionSet(config).has('GH_TOKEN')).toBe(false); + }); + + it('should NOT exclude GITHUB_PERSONAL_ACCESS_TOKEN', () => { + expect(buildExclusionSet(config).has('GITHUB_PERSONAL_ACCESS_TOKEN')).toBe(false); + }); }); describe('when difcProxyHost is set (DIFC proxy security)', () => { @@ -137,10 +161,14 @@ describe('buildExclusionSet', () => { it('should exclude GH_TOKEN', () => { expect(buildExclusionSet(config).has('GH_TOKEN')).toBe(true); }); + + it('should exclude GITHUB_PERSONAL_ACCESS_TOKEN', () => { + expect(buildExclusionSet(config).has('GITHUB_PERSONAL_ACCESS_TOKEN')).toBe(true); + }); }); - describe('when difcProxyHost is not set', () => { - const config = makeConfig({ difcProxyHost: undefined }); + describe('when difcProxyHost is not set and enableApiProxy is false', () => { + const config = makeConfig({ difcProxyHost: undefined, enableApiProxy: false }); it('should NOT exclude GITHUB_TOKEN', () => { expect(buildExclusionSet(config).has('GITHUB_TOKEN')).toBe(false); @@ -149,6 +177,10 @@ describe('buildExclusionSet', () => { it('should NOT exclude GH_TOKEN', () => { expect(buildExclusionSet(config).has('GH_TOKEN')).toBe(false); }); + + it('should NOT exclude GITHUB_PERSONAL_ACCESS_TOKEN', () => { + expect(buildExclusionSet(config).has('GITHUB_PERSONAL_ACCESS_TOKEN')).toBe(false); + }); }); describe('when excludeEnv is set', () => { diff --git a/src/services/agent-environment/excluded-vars.ts b/src/services/agent-environment/excluded-vars.ts index 277c6615c..8469cb5e1 100644 --- a/src/services/agent-environment/excluded-vars.ts +++ b/src/services/agent-environment/excluded-vars.ts @@ -34,11 +34,20 @@ export function buildExclusionSet(config: WrapperConfig): Set { excludedEnvVars.add('GEMINI_API_BASE_URL'); excludedEnvVars.add('GOOGLE_API_KEY'); excludedEnvVars.add('GOOGLE_VERTEX_BASE_URL'); + // GitHub tokens are excluded when API proxy is enabled (strict mode): + // the agent must not hold live credentials that can be extracted via + // /proc/self/environ or environment inspection. + excludedEnvVars.add('GITHUB_TOKEN'); + excludedEnvVars.add('GH_TOKEN'); + excludedEnvVars.add('GITHUB_PERSONAL_ACCESS_TOKEN'); } if (config.difcProxyHost) { + // Redundant with enableApiProxy block above, kept for explicit documentation: + // when DIFC proxy handles GitHub auth, tokens must never reach the agent. excludedEnvVars.add('GITHUB_TOKEN'); excludedEnvVars.add('GH_TOKEN'); + excludedEnvVars.add('GITHUB_PERSONAL_ACCESS_TOKEN'); } if (config.excludeEnv && config.excludeEnv.length > 0) { diff --git a/src/services/agent-environment/github-actions-environment.ts b/src/services/agent-environment/github-actions-environment.ts index 23757c0f0..63801141e 100644 --- a/src/services/agent-environment/github-actions-environment.ts +++ b/src/services/agent-environment/github-actions-environment.ts @@ -37,7 +37,11 @@ export function buildGitHubActionsEnvironment(params: GitHubActionsEnvironmentPa } if (config.additionalEnv) { - Object.assign(environment, config.additionalEnv); + for (const [key, value] of Object.entries(config.additionalEnv)) { + if (!excludedEnvVars.has(key)) { + environment[key] = value; + } + } } if (environment.NO_PROXY !== environment.no_proxy) { diff --git a/tests/integration/token-unset.test.ts b/tests/integration/token-unset.test.ts index b0a944ee4..4b2325f1c 100644 --- a/tests/integration/token-unset.test.ts +++ b/tests/integration/token-unset.test.ts @@ -28,18 +28,18 @@ describe('Token Isolation from Agent Environment', () => { test('should never expose GITHUB_TOKEN in /proc/1/environ', async () => { const testToken = 'ghp_test_token_12345678901234567890'; + // Pass expected value via a non-sensitive env var so the script can compare + // without embedding the raw token (which would leak via entrypoint command echo). const command = ` - # Check that the real token value never appears in /proc/1/environ - if cat /proc/1/environ 2>/dev/null | tr "\\0" "\\n" | grep -q "${testToken}"; then + if cat /proc/1/environ 2>/dev/null | tr "\\0" "\\n" | grep -q "$AWF_TEST_EXPECT"; then echo "FAIL: Real GITHUB_TOKEN found in /proc/1/environ" exit 1 else echo "SUCCESS: Real GITHUB_TOKEN not in /proc/1/environ" fi - # Also check printenv doesn't show the real token TOKEN_VALUE=$(printenv GITHUB_TOKEN 2>/dev/null || echo "") - if [ "$TOKEN_VALUE" = "${testToken}" ]; then + if [ "$TOKEN_VALUE" = "$AWF_TEST_EXPECT" ]; then echo "FAIL: Real GITHUB_TOKEN visible via printenv" exit 1 else @@ -55,12 +55,14 @@ describe('Token Isolation from Agent Environment', () => { env: { GITHUB_TOKEN: testToken, }, + cliEnv: { + AWF_TEST_EXPECT: testToken, + }, }); expect(result).toSucceed(); expect(result.stdout).toContain('SUCCESS: Real GITHUB_TOKEN not in /proc/1/environ'); expect(result.stdout).toContain('SUCCESS: Real GITHUB_TOKEN not visible via printenv'); - // The real token must never appear in any output expect(result.stdout).not.toContain(testToken); }, 120000); @@ -68,7 +70,7 @@ describe('Token Isolation from Agent Environment', () => { const testToken = 'sk-test_openai_key_1234567890'; const command = ` - if cat /proc/1/environ 2>/dev/null | tr "\\0" "\\n" | grep -q "${testToken}"; then + if cat /proc/1/environ 2>/dev/null | tr "\\0" "\\n" | grep -q "$AWF_TEST_EXPECT"; then echo "FAIL: Real OPENAI_API_KEY found in /proc/1/environ" exit 1 else @@ -76,7 +78,7 @@ describe('Token Isolation from Agent Environment', () => { fi TOKEN_VALUE=$(printenv OPENAI_API_KEY 2>/dev/null || echo "") - if [ "$TOKEN_VALUE" = "${testToken}" ]; then + if [ "$TOKEN_VALUE" = "$AWF_TEST_EXPECT" ]; then echo "FAIL: Real OPENAI_API_KEY visible via printenv" exit 1 else @@ -92,6 +94,9 @@ describe('Token Isolation from Agent Environment', () => { env: { OPENAI_API_KEY: testToken, }, + cliEnv: { + AWF_TEST_EXPECT: testToken, + }, }); expect(result).toSucceed(); @@ -104,7 +109,7 @@ describe('Token Isolation from Agent Environment', () => { const testToken = 'sk-ant-test_key_1234567890'; const command = ` - if cat /proc/1/environ 2>/dev/null | tr "\\0" "\\n" | grep -q "${testToken}"; then + if cat /proc/1/environ 2>/dev/null | tr "\\0" "\\n" | grep -q "$AWF_TEST_EXPECT"; then echo "FAIL: Real ANTHROPIC_API_KEY found in /proc/1/environ" exit 1 else @@ -112,7 +117,7 @@ describe('Token Isolation from Agent Environment', () => { fi TOKEN_VALUE=$(printenv ANTHROPIC_API_KEY 2>/dev/null || echo "") - if [ "$TOKEN_VALUE" = "${testToken}" ]; then + if [ "$TOKEN_VALUE" = "$AWF_TEST_EXPECT" ]; then echo "FAIL: Real ANTHROPIC_API_KEY visible via printenv" exit 1 else @@ -128,6 +133,9 @@ describe('Token Isolation from Agent Environment', () => { env: { ANTHROPIC_API_KEY: testToken, }, + cliEnv: { + AWF_TEST_EXPECT: testToken, + }, }); expect(result).toSucceed(); @@ -141,15 +149,16 @@ describe('Token Isolation from Agent Environment', () => { const openaiKey = 'sk-multi_openai_test'; const anthropicKey = 'sk-ant-multi_test'; + // Pass expected values via non-sensitive env vars for comparison const command = ` FAIL=0 # Check /proc/1/environ for any real token values ENVIRON=$(cat /proc/1/environ 2>/dev/null | tr "\\0" "\\n") - echo "$ENVIRON" | grep -q "${ghToken}" && echo "FAIL: GITHUB_TOKEN in environ" && FAIL=1 - echo "$ENVIRON" | grep -q "${openaiKey}" && echo "FAIL: OPENAI_API_KEY in environ" && FAIL=1 - echo "$ENVIRON" | grep -q "${anthropicKey}" && echo "FAIL: ANTHROPIC_API_KEY in environ" && FAIL=1 + echo "$ENVIRON" | grep -q "$AWF_TEST_GH" && echo "FAIL: GITHUB_TOKEN in environ" && FAIL=1 + echo "$ENVIRON" | grep -q "$AWF_TEST_OAI" && echo "FAIL: OPENAI_API_KEY in environ" && FAIL=1 + echo "$ENVIRON" | grep -q "$AWF_TEST_ANT" && echo "FAIL: ANTHROPIC_API_KEY in environ" && FAIL=1 if [ $FAIL -eq 0 ]; then echo "SUCCESS: No real tokens found in /proc/1/environ" @@ -158,9 +167,9 @@ describe('Token Isolation from Agent Environment', () => { fi # Verify printenv doesn't return real values - [ "$(printenv GITHUB_TOKEN 2>/dev/null)" = "${ghToken}" ] && echo "FAIL: GITHUB_TOKEN via printenv" && exit 1 - [ "$(printenv OPENAI_API_KEY 2>/dev/null)" = "${openaiKey}" ] && echo "FAIL: OPENAI_API_KEY via printenv" && exit 1 - [ "$(printenv ANTHROPIC_API_KEY 2>/dev/null)" = "${anthropicKey}" ] && echo "FAIL: ANTHROPIC_API_KEY via printenv" && exit 1 + [ "$(printenv GITHUB_TOKEN 2>/dev/null)" = "$AWF_TEST_GH" ] && echo "FAIL: GITHUB_TOKEN via printenv" && exit 1 + [ "$(printenv OPENAI_API_KEY 2>/dev/null)" = "$AWF_TEST_OAI" ] && echo "FAIL: OPENAI_API_KEY via printenv" && exit 1 + [ "$(printenv ANTHROPIC_API_KEY 2>/dev/null)" = "$AWF_TEST_ANT" ] && echo "FAIL: ANTHROPIC_API_KEY via printenv" && exit 1 echo "SUCCESS: No real tokens visible via printenv" `; @@ -175,6 +184,11 @@ describe('Token Isolation from Agent Environment', () => { OPENAI_API_KEY: openaiKey, ANTHROPIC_API_KEY: anthropicKey, }, + cliEnv: { + AWF_TEST_GH: ghToken, + AWF_TEST_OAI: openaiKey, + AWF_TEST_ANT: anthropicKey, + }, }); expect(result).toSucceed(); @@ -189,7 +203,7 @@ describe('Token Isolation from Agent Environment', () => { const testToken = 'copilot_test_token_never_exposed'; const command = ` - if cat /proc/1/environ 2>/dev/null | tr "\\0" "\\n" | grep -q "${testToken}"; then + if cat /proc/1/environ 2>/dev/null | tr "\\0" "\\n" | grep -q "$AWF_TEST_EXPECT"; then echo "FAIL: Real COPILOT_GITHUB_TOKEN found in /proc/1/environ" exit 1 else @@ -197,7 +211,7 @@ describe('Token Isolation from Agent Environment', () => { fi TOKEN_VALUE=$(printenv COPILOT_GITHUB_TOKEN 2>/dev/null || echo "") - if [ "$TOKEN_VALUE" = "${testToken}" ]; then + if [ "$TOKEN_VALUE" = "$AWF_TEST_EXPECT" ]; then echo "FAIL: Real COPILOT_GITHUB_TOKEN visible via printenv" exit 1 else @@ -213,6 +227,9 @@ describe('Token Isolation from Agent Environment', () => { env: { COPILOT_GITHUB_TOKEN: testToken, }, + cliEnv: { + AWF_TEST_EXPECT: testToken, + }, }); expect(result).toSucceed();