Skip to content

Commit 7908770

Browse files
CopilotCopilot
authored
test: add regression tests for cli-proxy validated fixes from #1820 (#1826)
* Initial plan * test: add regression tests for cli-proxy validated fixes Agent-Logs-Url: https://github.com/github/gh-aw-firewall/sessions/86274f46-937b-46bd-a5c4-81ff4b46ce9f * fix: add source reference comment for duplicated regex Agent-Logs-Url: https://github.com/github/gh-aw-firewall/sessions/86274f46-937b-46bd-a5c4-81ff4b46ce9f * fix: freeze PROTECTED_ENV_KEYS and update comment Agent-Logs-Url: https://github.com/github/gh-aw-firewall/sessions/35b7845a-c062-4423-8ee3-5d260968c289 * refactor: remove redundant PROTECTED_KEYS alias Agent-Logs-Url: https://github.com/github/gh-aw-firewall/sessions/35b7845a-c062-4423-8ee3-5d260968c289 --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
1 parent 9071254 commit 7908770

File tree

4 files changed

+175
-5
lines changed

4 files changed

+175
-5
lines changed

‎containers/cli-proxy/server.js‎

Lines changed: 17 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,20 @@ const CLI_PROXY_PORT = parseInt(process.env.AWF_CLI_PROXY_PORT || '11000', 10);
2626
const COMMAND_TIMEOUT_MS = parseInt(process.env.AWF_CLI_PROXY_TIMEOUT_MS || '30000', 10);
2727
const MAX_OUTPUT_BYTES = parseInt(process.env.AWF_CLI_PROXY_MAX_OUTPUT_BYTES || String(10 * 1024 * 1024), 10);
2828

29+
// Environment keys that agents are not allowed to override via the /exec env field.
30+
// GH_HOST / GH_TOKEN / GITHUB_TOKEN — prevent auth/routing hijack.
31+
// NODE_EXTRA_CA_CERTS / SSL_CERT_FILE — prevent TLS trust-store bypass.
32+
const _PROTECTED_ENV_KEYS = new Set(['GH_HOST', 'GH_TOKEN', 'GITHUB_TOKEN', 'NODE_EXTRA_CA_CERTS', 'SSL_CERT_FILE']);
33+
const PROTECTED_ENV_KEYS = Object.freeze({
34+
has(key) { return _PROTECTED_ENV_KEYS.has(key); },
35+
get size() { return _PROTECTED_ENV_KEYS.size; },
36+
values() { return _PROTECTED_ENV_KEYS.values(); },
37+
keys() { return _PROTECTED_ENV_KEYS.keys(); },
38+
entries() { return _PROTECTED_ENV_KEYS.entries(); },
39+
forEach(callback, thisArg) { return _PROTECTED_ENV_KEYS.forEach(callback, thisArg); },
40+
[Symbol.iterator]() { return _PROTECTED_ENV_KEYS[Symbol.iterator](); },
41+
});
42+
2943
// --- Structured logging to /var/log/cli-proxy/access.log ---
3044

3145
const LOG_DIR = process.env.AWF_CLI_PROXY_LOG_DIR || '/var/log/cli-proxy';
@@ -226,10 +240,9 @@ async function handleExec(req, res) {
226240
// Inherit server environment (includes GH_HOST, NODE_EXTRA_CA_CERTS, GH_REPO, etc.)
227241
const childEnv = Object.assign({}, process.env);
228242
if (extraEnv && typeof extraEnv === 'object') {
229-
// Only allow safe string env overrides; never allow overriding GH_HOST or GH_TOKEN
230-
const PROTECTED_KEYS = new Set(['GH_HOST', 'GH_TOKEN', 'GITHUB_TOKEN', 'NODE_EXTRA_CA_CERTS', 'SSL_CERT_FILE']);
243+
// Only allow safe string env overrides; never allow overriding keys in PROTECTED_ENV_KEYS.
231244
for (const [key, value] of Object.entries(extraEnv)) {
232-
if (typeof key === 'string' && typeof value === 'string' && !PROTECTED_KEYS.has(key)) {
245+
if (typeof key === 'string' && typeof value === 'string' && !PROTECTED_ENV_KEYS.has(key)) {
233246
childEnv[key] = value;
234247
}
235248
}
@@ -353,4 +366,4 @@ if (require.main === module) {
353366
});
354367
}
355368

356-
module.exports = { validateArgs, ALWAYS_DENIED_SUBCOMMANDS };
369+
module.exports = { validateArgs, ALWAYS_DENIED_SUBCOMMANDS, PROTECTED_ENV_KEYS };

‎containers/cli-proxy/server.test.js‎

Lines changed: 23 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,29 @@
66
* The server only enforces meta-command denial (auth, config, extension).
77
*/
88

9-
const { validateArgs, ALWAYS_DENIED_SUBCOMMANDS } = require('./server');
9+
const { validateArgs, ALWAYS_DENIED_SUBCOMMANDS, PROTECTED_ENV_KEYS } = require('./server');
10+
11+
describe('PROTECTED_ENV_KEYS', () => {
12+
it('should protect GH_HOST from agent override', () => {
13+
expect(PROTECTED_ENV_KEYS.has('GH_HOST')).toBe(true);
14+
});
15+
16+
it('should protect GH_TOKEN from agent override', () => {
17+
expect(PROTECTED_ENV_KEYS.has('GH_TOKEN')).toBe(true);
18+
});
19+
20+
it('should protect GITHUB_TOKEN from agent override', () => {
21+
expect(PROTECTED_ENV_KEYS.has('GITHUB_TOKEN')).toBe(true);
22+
});
23+
24+
it('should protect NODE_EXTRA_CA_CERTS from agent override', () => {
25+
expect(PROTECTED_ENV_KEYS.has('NODE_EXTRA_CA_CERTS')).toBe(true);
26+
});
27+
28+
it('should protect SSL_CERT_FILE from agent override (combined CA bundle for Go TLS)', () => {
29+
expect(PROTECTED_ENV_KEYS.has('SSL_CERT_FILE')).toBe(true);
30+
});
31+
});
1032

1133
describe('validateArgs', () => {
1234
describe('input validation', () => {

‎scripts/ci/postprocess-smoke-workflows.test.ts‎

Lines changed: 67 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,67 @@
1+
/**
2+
* Tests for postprocess-smoke-workflows.ts regex patterns.
3+
*
4+
* These tests verify that the install-step regex correctly handles both
5+
* quoted and unquoted paths, covering the fix for gh-aw compilers that
6+
* emit double-quoted ${RUNNER_TEMP}/... paths.
7+
*/
8+
9+
// The regex is module-internal in postprocess-smoke-workflows.ts (line 58-59)
10+
// and cannot be imported because the script performs file I/O at module load
11+
// time. If the source regex changes, these tests will catch regressions by
12+
// failing on the expected inputs below.
13+
const installStepRegex =
14+
/^(\s*)- name: Install [Aa][Ww][Ff] binary\n\1\s*run: bash "?(?:\/opt\/gh-aw|\$\{RUNNER_TEMP\}\/gh-aw)\/actions\/install_awf_binary\.sh"? v[0-9.]+\n/m;
15+
16+
describe('installStepRegex', () => {
17+
it('should match unquoted /opt/gh-aw path', () => {
18+
const input =
19+
' - name: Install awf binary\n' +
20+
' run: bash /opt/gh-aw/actions/install_awf_binary.sh v0.25.17\n';
21+
expect(installStepRegex.test(input)).toBe(true);
22+
});
23+
24+
it('should match unquoted ${RUNNER_TEMP}/gh-aw path', () => {
25+
const input =
26+
' - name: Install awf binary\n' +
27+
' run: bash ${RUNNER_TEMP}/gh-aw/actions/install_awf_binary.sh v0.25.17\n';
28+
expect(installStepRegex.test(input)).toBe(true);
29+
});
30+
31+
it('should match double-quoted ${RUNNER_TEMP}/gh-aw path', () => {
32+
const input =
33+
' - name: Install awf binary\n' +
34+
' run: bash "${RUNNER_TEMP}/gh-aw/actions/install_awf_binary.sh" v0.25.17\n';
35+
expect(installStepRegex.test(input)).toBe(true);
36+
});
37+
38+
it('should match double-quoted /opt/gh-aw path', () => {
39+
const input =
40+
' - name: Install awf binary\n' +
41+
' run: bash "/opt/gh-aw/actions/install_awf_binary.sh" v0.25.17\n';
42+
expect(installStepRegex.test(input)).toBe(true);
43+
});
44+
45+
it('should match case-insensitive AWF in step name', () => {
46+
const input =
47+
' - name: Install AWF binary\n' +
48+
' run: bash /opt/gh-aw/actions/install_awf_binary.sh v0.25.17\n';
49+
expect(installStepRegex.test(input)).toBe(true);
50+
});
51+
52+
it('should not match step with wrong name', () => {
53+
const input =
54+
' - name: Install something else\n' +
55+
' run: bash /opt/gh-aw/actions/install_awf_binary.sh v0.25.17\n';
56+
expect(installStepRegex.test(input)).toBe(false);
57+
});
58+
59+
it('should capture indentation for replacement', () => {
60+
const input =
61+
' - name: Install awf binary\n' +
62+
' run: bash "${RUNNER_TEMP}/gh-aw/actions/install_awf_binary.sh" v0.25.17\n';
63+
const match = input.match(installStepRegex);
64+
expect(match).not.toBeNull();
65+
expect(match![1]).toBe(' ');
66+
});
67+
});

‎src/docker-manager.test.ts‎

Lines changed: 68 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2919,6 +2919,74 @@ describe('docker-manager', () => {
29192919
expect(dependsOn['squid-proxy']).toBeDefined();
29202920
expect(dependsOn['cli-proxy-mcpg']).toBeUndefined();
29212921
});
2922+
2923+
it('should pass GH_TOKEN to cli-proxy environment when available', () => {
2924+
const originalGhToken = process.env.GH_TOKEN;
2925+
try {
2926+
process.env.GH_TOKEN = 'ghp_cli_proxy_test_token';
2927+
const configWithCliProxy = { ...mockConfig, difcProxyHost: 'host.docker.internal:18443' };
2928+
const result = generateDockerCompose(configWithCliProxy, mockNetworkConfigWithCliProxy);
2929+
const proxy = result.services['cli-proxy'];
2930+
const env = proxy.environment as Record<string, string>;
2931+
expect(env.GH_TOKEN).toBe('ghp_cli_proxy_test_token');
2932+
} finally {
2933+
if (originalGhToken !== undefined) {
2934+
process.env.GH_TOKEN = originalGhToken;
2935+
} else {
2936+
delete process.env.GH_TOKEN;
2937+
}
2938+
}
2939+
});
2940+
2941+
it('should fall back to GITHUB_TOKEN for cli-proxy when GH_TOKEN is absent', () => {
2942+
const originalGhToken = process.env.GH_TOKEN;
2943+
const originalGithubToken = process.env.GITHUB_TOKEN;
2944+
try {
2945+
delete process.env.GH_TOKEN;
2946+
process.env.GITHUB_TOKEN = 'ghp_github_token_fallback';
2947+
const configWithCliProxy = { ...mockConfig, difcProxyHost: 'host.docker.internal:18443' };
2948+
const result = generateDockerCompose(configWithCliProxy, mockNetworkConfigWithCliProxy);
2949+
const proxy = result.services['cli-proxy'];
2950+
const env = proxy.environment as Record<string, string>;
2951+
expect(env.GH_TOKEN).toBe('ghp_github_token_fallback');
2952+
} finally {
2953+
if (originalGhToken !== undefined) {
2954+
process.env.GH_TOKEN = originalGhToken;
2955+
} else {
2956+
delete process.env.GH_TOKEN;
2957+
}
2958+
if (originalGithubToken !== undefined) {
2959+
process.env.GITHUB_TOKEN = originalGithubToken;
2960+
} else {
2961+
delete process.env.GITHUB_TOKEN;
2962+
}
2963+
}
2964+
});
2965+
2966+
it('should not set GH_TOKEN in cli-proxy when neither GH_TOKEN nor GITHUB_TOKEN is set', () => {
2967+
const originalGhToken = process.env.GH_TOKEN;
2968+
const originalGithubToken = process.env.GITHUB_TOKEN;
2969+
try {
2970+
delete process.env.GH_TOKEN;
2971+
delete process.env.GITHUB_TOKEN;
2972+
const configWithCliProxy = { ...mockConfig, difcProxyHost: 'host.docker.internal:18443' };
2973+
const result = generateDockerCompose(configWithCliProxy, mockNetworkConfigWithCliProxy);
2974+
const proxy = result.services['cli-proxy'];
2975+
const env = proxy.environment as Record<string, string>;
2976+
expect(env.GH_TOKEN).toBeUndefined();
2977+
} finally {
2978+
if (originalGhToken !== undefined) {
2979+
process.env.GH_TOKEN = originalGhToken;
2980+
} else {
2981+
delete process.env.GH_TOKEN;
2982+
}
2983+
if (originalGithubToken !== undefined) {
2984+
process.env.GITHUB_TOKEN = originalGithubToken;
2985+
} else {
2986+
delete process.env.GITHUB_TOKEN;
2987+
}
2988+
}
2989+
});
29222990
});
29232991
});
29242992

0 commit comments

Comments
 (0)