Skip to content

Commit aae9d72

Browse files
authored
fix(cli-proxy): normalize exec exitCode and harden env key filtering
1 parent 4f4c6d2 commit aae9d72

3 files changed

Lines changed: 55 additions & 8 deletions

File tree

containers/cli-proxy/package-lock.json

Lines changed: 24 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

containers/cli-proxy/server.js

Lines changed: 22 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,8 @@ const PROTECTED_ENV_KEYS = Object.freeze({
4040
[Symbol.iterator]() { return _PROTECTED_ENV_KEYS[Symbol.iterator](); },
4141
});
4242

43+
const UNSAFE_ENV_KEYS = new Set(['__proto__', 'constructor', 'prototype']);
44+
4345
// --- Structured logging to /var/log/cli-proxy/access.jsonl ---
4446

4547
const LOG_DIR = process.env.AWF_CLI_PROXY_LOG_DIR || '/var/log/cli-proxy';
@@ -215,7 +217,12 @@ function buildExecEnv(extraEnv) {
215217
if (extraEnv && typeof extraEnv === 'object') {
216218
// Only allow safe string env overrides; never allow overriding keys in PROTECTED_ENV_KEYS.
217219
for (const [key, value] of Object.entries(extraEnv)) {
218-
if (typeof key === 'string' && typeof value === 'string' && !PROTECTED_ENV_KEYS.has(key)) {
220+
if (
221+
typeof key === 'string'
222+
&& typeof value === 'string'
223+
&& !PROTECTED_ENV_KEYS.has(key)
224+
&& !UNSAFE_ENV_KEYS.has(key)
225+
) {
219226
childEnv[key] = value;
220227
}
221228
}
@@ -236,6 +243,19 @@ function buildExecEnv(extraEnv) {
236243
* @returns {Promise<{stdout: string, stderr: string, exitCode: number}>}
237244
*/
238245
async function runGhCommand(args, childEnv, stdin) {
246+
const normalizeExitCode = code => {
247+
if (typeof code === 'number' && Number.isFinite(code)) {
248+
return code;
249+
}
250+
if (typeof code === 'string') {
251+
const parsedCode = Number.parseInt(code, 10);
252+
if (!Number.isNaN(parsedCode)) {
253+
return parsedCode;
254+
}
255+
}
256+
return 1;
257+
};
258+
239259
try {
240260
return await new Promise((resolve, reject) => {
241261
const child = execFile('gh', args, {
@@ -253,7 +273,7 @@ async function runGhCommand(args, childEnv, stdin) {
253273
resolve({
254274
stdout: childStdout || '',
255275
stderr: childStderr || '',
256-
exitCode: err ? (err.code || 1) : 0,
276+
exitCode: err ? normalizeExitCode(err.code) : 0,
257277
});
258278
});
259279

containers/cli-proxy/server.test.js

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -238,10 +238,14 @@ describe('buildExecEnv', () => {
238238
expect(env.GIT_SSL_CAINFO).toBe('/certs/git.crt');
239239
});
240240

241-
it('should silently skip non-string keys', () => {
242-
// Object.entries coerces, but we guard typeof key === 'string'
243-
const env = buildExecEnv({ MY_VAR: 'ok' });
241+
it('should ignore dangerous prototype keys from JSON payloads', () => {
242+
const extraEnv = JSON.parse('{"__proto__":"polluted","constructor":"polluted","prototype":"polluted","MY_VAR":"ok"}');
243+
const env = buildExecEnv(extraEnv);
244244
expect(env.MY_VAR).toBe('ok');
245+
expect(env.__proto__).toBe(Object.prototype);
246+
expect(env.constructor).toBe(Object);
247+
expect(env.prototype).toBeUndefined();
248+
expect({}.polluted).toBeUndefined();
245249
});
246250

247251
it('should silently skip non-string values', () => {
@@ -286,9 +290,8 @@ describe('runGhCommand', () => {
286290
process.env.PATH = '';
287291
try {
288292
const result = await runGhCommand(['--version'], process.env, null);
289-
// When the binary is not found, exitCode is the ENOENT error code (truthy, non-zero)
290-
expect(result.exitCode).toBeTruthy();
291-
expect(result.exitCode).not.toBe(0);
293+
expect(typeof result.exitCode).toBe('number');
294+
expect(result.exitCode).toBe(1);
292295
} finally {
293296
process.env.PATH = savedPath;
294297
}

0 commit comments

Comments
 (0)