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 <ntdat812@gmail.com>
This commit is contained in:
garrytanandNguyen Thanh Dat committed 2026-10-03 20:40:07 +00:00
1 parent 178724034b
commit e6676ea080
2 files changed
+37 -4

No files matched your search

+22
View File
@@ -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,
+15 -4
View File
@@ -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;
}