From e6676ea080eae22453e856fa25608919d8bd9388 Mon Sep 17 00:00:00 2001 From: garrytan Date: Sat, 3 Oct 2026 20:40:07 +0000 Subject: [PATCH] H #2949: eval credential screen checks every name segment The hermetic eval env admits GITHUB_* and EVALS_* metadata but rejects credential-shaped names, judged only by the last underscore segment. A trailing qualifier carried a credential through: GITHUB_APP_PRIVATE_KEY_BASE64, GITHUB_TOKEN_1, EVALS_API_KEY_FALLBACK reached eval children. The screen now rejects a name when any segment is a credential word; segments, not substrings, so GITHUB_PATH / GITHUB_TOKENIZER / GITHUB_KEYRING stay metadata, and explicit runner admissions still win. Co-authored-by: Nguyen Thanh Dat --- test/helpers/hermetic-env.test.ts | 22 ++++++++++++++++++++++ test/helpers/hermetic-env.ts | 19 +++++++++++++++---- 2 files changed, 37 insertions(+), 4 deletions(-) diff --git a/test/helpers/hermetic-env.test.ts b/test/helpers/hermetic-env.test.ts index 6d18f7faf..2518f688e 100644 --- a/test/helpers/hermetic-env.test.ts +++ b/test/helpers/hermetic-env.test.ts @@ -144,6 +144,28 @@ describe('buildHermeticEnv allowlist', () => { ]) expect(result[name]).toBe(base[name]); }); + test('a trailing qualifier does not carry a credential past the prefix rule (#2949)', () => { + const qualified = { + GITHUB_APP_PRIVATE_KEY_BASE64: 'synthetic-pem-base64', + GITHUB_PRIVATE_KEY_PEM: 'synthetic-pem', + GITHUB_TOKEN_1: 'synthetic-first-token', + GITHUB_TOKEN_GHES: 'synthetic-enterprise-token', + GITHUB_CLIENT_SECRET_VALUE: 'synthetic-client-secret', + EVALS_API_KEY_FALLBACK: 'synthetic-eval-key', + }; + const result = buildHermeticEnv({ ...CONTAMINATED, ...qualified }, HERMETIC_VARS); + for (const name of Object.keys(qualified)) expect(result[name]).toBeUndefined(); + const serialized = JSON.stringify(result); + for (const value of Object.values(qualified)) expect(serialized.includes(value)).toBe(false); + + // Segments, not substrings: names that merely contain a credential word + // stay metadata, and an explicit runner admission still wins. + const metadata = { GITHUB_PATH: '/tmp/p', GITHUB_TOKENIZER: 't', GITHUB_KEYRING: 'k', GITHUB_REF_PROTECTED: 'false', EVALS_RUN_ID: 'r' }; + const kept = buildHermeticEnv({ ...CONTAMINATED, ...metadata, ...qualified }, HERMETIC_VARS, undefined, { extraAllow: ['GITHUB_TOKEN_1'] }); + for (const [name, value] of Object.entries(metadata)) expect(kept[name]).toBe(value); + expect(kept.GITHUB_TOKEN_1).toBe(qualified.GITHUB_TOKEN_1); + }); + test('explicit provider auth, runner admissions, and overrides still win', () => { const base = { ...CONTAMINATED, diff --git a/test/helpers/hermetic-env.ts b/test/helpers/hermetic-env.ts index cb6c8c1c2..6473f7828 100644 --- a/test/helpers/hermetic-env.ts +++ b/test/helpers/hermetic-env.ts @@ -71,15 +71,27 @@ const ALLOW_EXACT = new Set([ * VOYAGE_API_KEY) — CI doesn't have them and eval children have no business * using them. A test that legitimately needs one opts in via its own env * override; a provider runner (codex/gemini) re-admits its auth vars via - * opts.extraAllow. Prefix matches reject credential-shaped suffixes; exact + * opts.extraAllow. Prefix matches reject credential-shaped segments; exact * and explicit runner admissions still win. */ const ALLOW_PREFIXES = ['EVALS_', 'GITHUB_']; -const CREDENTIAL_SUFFIXES = new Set([ +const CREDENTIAL_SEGMENTS = new Set([ 'KEY', 'KEYS', 'TOKEN', 'TOKENS', 'SECRET', 'SECRETS', 'PASSWORD', 'PASSWD', 'PASS', 'CREDENTIAL', 'CREDENTIALS', 'AUTH', 'PAT', 'DSN', 'COOKIE', 'SESSION', 'PRIVATE', ]); +/** + * True when any underscore-separated segment of `name` is a credential word. + * Every segment, not just the last: a trailing qualifier moves the credential + * word off the end (`GITHUB_APP_PRIVATE_KEY_BASE64` is the PEM itself, + * `GITHUB_TOKEN_1` is a token). Segments, not substrings: `GITHUB_PATH` + * contains "PAT" and `GITHUB_TOKENIZER` contains "TOKEN", and both are + * metadata. + */ +function isCredentialShapedName(name: string): boolean { + return name.toUpperCase().split('_').some((segment) => CREDENTIAL_SEGMENTS.has(segment)); +} + export interface HermeticEnvOpts { /** Per-runner additional allowed names (exact match) or prefixes (entries * ending in '*'). Example: codex runner passes ['OPENAI_API_KEY', 'CODEX_*']. */ @@ -126,8 +138,7 @@ export function buildHermeticEnv( const allowed = ALLOW_EXACT.has(k) || extraExact.has(k) || - (ALLOW_PREFIXES.some((p) => k.startsWith(p)) && - !CREDENTIAL_SUFFIXES.has(k.slice(k.lastIndexOf('_') + 1).toUpperCase())) || + (ALLOW_PREFIXES.some((p) => k.startsWith(p)) && !isCredentialShapedName(k)) || extraPrefixes.some((p) => k.startsWith(p)); if (allowed) out[k] = v; }