Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -289,7 +289,11 @@ operator `~/.claude` config, MCP servers (gbrain, Conductor), skills, `~/.gstack
decision logs, and `CONDUCTOR_*` env never leak into the child. The `GITHUB_`
and `EVALS_` prefix rules preserve CI metadata but reject credential-shaped
names such as `GITHUB_TOKEN`, `GITHUB_PERSONAL_ACCESS_TOKEN`, and
`GITHUB_APP_PRIVATE_KEY`. Named provider auth, runner `extraAllow` entries, and
`GITHUB_APP_PRIVATE_KEY`. The screen reads every underscore-separated segment,
so a trailing qualifier does not carry a name past it
(`GITHUB_APP_PRIVATE_KEY_BASE64`, `GITHUB_TOKEN_1`), while a segment that
merely contains a credential word stays metadata (`GITHUB_PATH`,
`GITHUB_TOKENIZER`). Named provider auth, runner `extraAllow` entries, and
per-test overrides are deliberate exceptions; a name-based rule cannot identify
a secret assigned to an arbitrary metadata name. This keeps local eval signal
aligned with CI instead of disagreeing for reasons unrelated to the code under
Expand Down
72 changes: 72 additions & 0 deletions test/helpers/hermetic-env.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import {
getHermeticDirs,
gcStaleHermeticDirs,
hermeticChildEnv,
isCredentialShapedName,
hermeticCeoPlanReadArgs,
hermeticDesignReadArgs,
} from './hermetic-env';
Expand Down Expand Up @@ -143,6 +144,77 @@ describe('buildHermeticEnv allowlist', () => {
]) expect(result[name]).toBe(base[name]);
});

test('a trailing qualifier does not carry a credential past the prefix rule', () => {
// Once there is more than one of something, the name grows a qualifier
// and the credential word stops being last. These are the ordinary
// spellings: the base64 form of the app PEM, a numbered token, an
// enterprise-server token.
const base = {
...CONTAMINATED,
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(base, HERMETIC_VARS);
for (const name of [
'GITHUB_APP_PRIVATE_KEY_BASE64', 'GITHUB_PRIVATE_KEY_PEM', 'GITHUB_TOKEN_1',
'GITHUB_TOKEN_GHES', 'GITHUB_CLIENT_SECRET_VALUE', 'EVALS_API_KEY_FALLBACK',
]) expect(result[name]).toBeUndefined();

// And no value survives under any other name either.
const serialized = JSON.stringify(result);
for (const value of [
'synthetic-pem-base64', 'synthetic-pem', 'synthetic-first-token',
'synthetic-enterprise-token', 'synthetic-client-secret', 'synthetic-eval-key',
]) expect(serialized.includes(value)).toBe(false);
});

test('screening is per segment, so near-miss metadata names still pass', () => {
// The whole documented runner set, plus the words that merely contain a
// credential word: GITHUB_PATH holds PAT, TOKENIZER holds TOKEN, KEYRING
// holds KEY. A substring screen would take all three.
const METADATA = [
'GITHUB_ACTION', 'GITHUB_ACTIONS', 'GITHUB_ACTOR', 'GITHUB_ACTOR_ID',
'GITHUB_API_URL', 'GITHUB_BASE_REF', 'GITHUB_ENV', 'GITHUB_EVENT_NAME',
'GITHUB_EVENT_PATH', 'GITHUB_GRAPHQL_URL', 'GITHUB_HEAD_REF', 'GITHUB_JOB',
'GITHUB_OUTPUT', 'GITHUB_PATH', 'GITHUB_REF', 'GITHUB_REF_NAME',
'GITHUB_REF_PROTECTED', 'GITHUB_REF_TYPE', 'GITHUB_REPOSITORY',
'GITHUB_REPOSITORY_ID', 'GITHUB_REPOSITORY_OWNER', 'GITHUB_RETENTION_DAYS',
'GITHUB_RUN_ATTEMPT', 'GITHUB_RUN_ID', 'GITHUB_RUN_NUMBER',
'GITHUB_SERVER_URL', 'GITHUB_SHA', 'GITHUB_STEP_SUMMARY',
'GITHUB_TRIGGERING_ACTOR', 'GITHUB_WORKFLOW', 'GITHUB_WORKFLOW_REF',
'GITHUB_WORKFLOW_SHA', 'GITHUB_WORKSPACE', 'GITHUB_TOKENIZER',
'GITHUB_KEYRING', 'EVALS_RUN_ID', 'EVALS_SELECTION_JSON',
];
const base = { ...CONTAMINATED } as NodeJS.ProcessEnv;
for (const name of METADATA) base[name] = `v-${name}`;
const result = buildHermeticEnv(base, HERMETIC_VARS);
for (const name of METADATA) expect(result[name]).toBe(`v-${name}`);
});

test('isCredentialShapedName reads segments, not substrings', () => {
for (const name of [
'GITHUB_TOKEN', 'GITHUB_APP_PRIVATE_KEY_BASE64', 'GITHUB_TOKEN_1',
'EVALS_API_KEY_FALLBACK', 'GITHUB_PAT',
]) expect(isCredentialShapedName(name)).toBe(true);
for (const name of [
'GITHUB_PATH', 'GITHUB_TOKENIZER', 'GITHUB_KEYRING', 'GITHUB_STEP_SUMMARY',
'EVALS_RUN_ID',
]) expect(isCredentialShapedName(name)).toBe(false);
});

test('a qualified runner credential is still re-admitted by extraAllow', () => {
// The screen governs prefix rules only; a runner that genuinely needs one
// of these names keeps saying so explicitly.
const base = { ...CONTAMINATED, GITHUB_TOKEN_1: 'synthetic-first-token' };
const result = buildHermeticEnv(base, HERMETIC_VARS, undefined, { extraAllow: ['GITHUB_TOKEN_1'] });
expect(result.GITHUB_TOKEN_1).toBe('synthetic-first-token');
expect(buildHermeticEnv(base, HERMETIC_VARS).GITHUB_TOKEN_1).toBeUndefined();
});

test('explicit provider auth, runner admissions, and overrides still win', () => {
const base = {
...CONTAMINATED,
Expand Down
20 changes: 18 additions & 2 deletions test/helpers/hermetic-env.ts
Original file line number Diff line number Diff line change
Expand Up @@ -70,12 +70,28 @@ const ALLOW_EXACT = new Set([
* opts.extraAllow. Prefix matches reject credential-shaped suffixes; 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 is the normal way
* these names are written once there is more than one of them, and it moves
* the credential word off the end. `GITHUB_APP_PRIVATE_KEY_BASE64` is the
* PEM itself, `GITHUB_TOKEN_1` is a token, and a tail-only screen admits
* both.
*
* Segments, not substrings: `GITHUB_PATH` is documented runner metadata and
* contains "PAT".
*/
export 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_*']. */
Expand Down Expand Up @@ -122,7 +138,7 @@ export function buildHermeticEnv(
ALLOW_EXACT.has(k) ||
extraExact.has(k) ||
(ALLOW_PREFIXES.some((p) => k.startsWith(p)) &&
!CREDENTIAL_SUFFIXES.has(k.slice(k.lastIndexOf('_') + 1).toUpperCase())) ||
!isCredentialShapedName(k)) ||
extraPrefixes.some((p) => k.startsWith(p));
if (allowed) out[k] = v;
}
Expand Down
10 changes: 9 additions & 1 deletion test/helpers/session-runner.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ test('runSkillTest launches a child without operator credentials', () => {
try {
const bin = path.join(root, 'claude');
fs.writeFileSync(bin, `#!/usr/bin/env node
const names = ['GITHUB_TOKEN', 'GITHUB_PERSONAL_ACCESS_TOKEN', 'GITHUB_APP_PRIVATE_KEY', 'GH_TOKEN', 'GITHUB_ACTIONS', 'GITHUB_PATH', 'GITHUB_TOKENIZER', 'EVALS_RUN_ID'];
const names = ['GITHUB_TOKEN', 'GITHUB_PERSONAL_ACCESS_TOKEN', 'GITHUB_APP_PRIVATE_KEY', 'GITHUB_APP_PRIVATE_KEY_BASE64', 'GITHUB_TOKEN_1', 'GH_TOKEN', 'GITHUB_ACTIONS', 'GITHUB_PATH', 'GITHUB_TOKENIZER', 'GITHUB_KEYRING', 'EVALS_RUN_ID'];
const present = Object.fromEntries(names.map(name => [name, Object.hasOwn(process.env, name)]));
console.log(JSON.stringify({type: 'result', subtype: 'success', result: JSON.stringify(present)}));
`, { mode: 0o700 });
Expand All @@ -29,10 +29,13 @@ console.log(JSON.stringify({exitReason: result.exitReason, child: JSON.parse(res
GITHUB_TOKEN: 'synthetic-token',
GITHUB_PERSONAL_ACCESS_TOKEN: 'synthetic-pat',
GITHUB_APP_PRIVATE_KEY: 'synthetic-private-key',
GITHUB_APP_PRIVATE_KEY_BASE64: 'synthetic-pem-base64',
GITHUB_TOKEN_1: 'synthetic-first-token',
GH_TOKEN: 'synthetic-gh-token',
GITHUB_ACTIONS: 'true',
GITHUB_PATH: '/tmp/actions-path',
GITHUB_TOKENIZER: 'metadata-tokenizer',
GITHUB_KEYRING: 'metadata-keyring',
EVALS_RUN_ID: 'synthetic-run',
},
});
Expand All @@ -43,10 +46,15 @@ console.log(JSON.stringify({exitReason: result.exitReason, child: JSON.parse(res
GITHUB_TOKEN: false,
GITHUB_PERSONAL_ACCESS_TOKEN: false,
GITHUB_APP_PRIVATE_KEY: false,
// A qualifier moves the credential word off the end of the name; it
// is still the PEM, and still a token.
GITHUB_APP_PRIVATE_KEY_BASE64: false,
GITHUB_TOKEN_1: false,
GH_TOKEN: false,
GITHUB_ACTIONS: true,
GITHUB_PATH: true,
GITHUB_TOKENIZER: true,
GITHUB_KEYRING: true,
EVALS_RUN_ID: true,
},
});
Expand Down