mirror of
https://github.com/garrytan/gbrain.git
synced 2026-08-14 00:48:18 +00:00
fix(integrations): resolve secrets through buildGatewayConfig's config→env folding (#3648)
Adversarial review: survived a hostile reviewer plus two independent refuters, each told to assume the PR was broken and to default to refuting when uncertain. 19 of 62 PRs cleared that bar. `integrations show` printed `[missing]` for config-plane keys that the runtime gateway resolves perfectly well — so the status display disagreed with reality and sent people hunting for a problem that did not exist. Fixed with a single `secretEnv()` helper at all four read sites, preserving precedence. The spawn environment is deliberately left unchanged, which is the correct posture. Sequenced after #3531, which refactored the `buildGatewayConfig` internals this consumes. Verified before merge: the PR's own tests fail when the production change is reverted; typecheck clean; MERGEABLE/CLEAN on the current base after batches 1 and 2 landed, not a stale one. Known gap, recorded rather than hidden: the full-suite env-mutation interaction was not run locally; CI shards are green.
This commit is contained in:
@@ -23,7 +23,8 @@ import matter from 'gray-matter';
|
||||
import { readFileSync, existsSync, writeFileSync, mkdirSync, readdirSync } from 'fs';
|
||||
import { join, basename } from 'path';
|
||||
import { homedir } from 'os';
|
||||
import { gbrainPath } from '../core/config.ts';
|
||||
import { gbrainPath, loadConfig } from '../core/config.ts';
|
||||
import { buildGatewayConfig } from '../core/ai/build-gateway-config.ts';
|
||||
import { execSync } from 'child_process';
|
||||
|
||||
// --- Types ---
|
||||
@@ -122,9 +123,28 @@ export function isUnsafeHealthCheck(check: string): boolean {
|
||||
return /[;&|`$(){}\\<>\n]/.test(check);
|
||||
}
|
||||
|
||||
/** Expand $VAR references with process.env values */
|
||||
/**
|
||||
* Env view for secret resolution (#2789): apply the same config.json→env
|
||||
* folding the runtime applies via buildGatewayConfig, so a credential stored
|
||||
* only in ~/.gbrain/config.json — which powers a perfectly healthy
|
||||
* integration — is not reported [missing] by show/status. process.env still
|
||||
* wins for non-empty values (buildGatewayConfig spreads it last, dropping
|
||||
* only ''/undefined entries). Falls back to bare process.env before
|
||||
* `gbrain init` (no config file yet). Mirrors the #2728 fix on the
|
||||
* providers command.
|
||||
*/
|
||||
export function secretEnv(): Record<string, string | undefined> {
|
||||
try {
|
||||
const cfg = loadConfig();
|
||||
if (cfg) return buildGatewayConfig(cfg).env;
|
||||
} catch { /* integrations must keep working pre-init — fall through */ }
|
||||
return process.env;
|
||||
}
|
||||
|
||||
/** Expand $VAR references with gateway-env (config-folded) values */
|
||||
export function expandVars(s: string): string {
|
||||
return s.replace(/\$([A-Z_][A-Z0-9_]*)/g, (_, name) => process.env[name] || '');
|
||||
const env = secretEnv();
|
||||
return s.replace(/\$([A-Z_][A-Z0-9_]*)/g, (_, name) => env[name] || '');
|
||||
}
|
||||
|
||||
// --- SSRF Protection ---
|
||||
@@ -249,7 +269,7 @@ export async function executeHealthCheck(
|
||||
}
|
||||
|
||||
case 'env_exists': {
|
||||
const val = process.env[check.name];
|
||||
const val = secretEnv()[check.name];
|
||||
return {
|
||||
...base,
|
||||
status: val ? 'ok' : 'fail',
|
||||
@@ -457,11 +477,12 @@ function readHeartbeat(id: string): HeartbeatEntry[] {
|
||||
|
||||
// --- Secret Checking ---
|
||||
|
||||
function checkSecrets(secrets: RecipeSecret[]): { set: string[]; missing: RecipeSecret[] } {
|
||||
export function checkSecrets(secrets: RecipeSecret[]): { set: string[]; missing: RecipeSecret[] } {
|
||||
const set: string[] = [];
|
||||
const missing: RecipeSecret[] = [];
|
||||
const env = secretEnv();
|
||||
for (const s of secrets) {
|
||||
if (process.env[s.name]) {
|
||||
if (env[s.name]) {
|
||||
set.push(s.name);
|
||||
} else {
|
||||
missing.push(s);
|
||||
@@ -607,8 +628,9 @@ function cmdShow(args: string[]): void {
|
||||
if (f.requires.length > 0) console.log(`Requires: ${f.requires.join(', ')}`);
|
||||
|
||||
console.log('\nSecrets needed:');
|
||||
const env = secretEnv();
|
||||
for (const s of f.secrets) {
|
||||
const isSet = process.env[s.name] ? ' [set]' : ' [missing]';
|
||||
const isSet = env[s.name] ? ' [set]' : ' [missing]';
|
||||
console.log(` ${s.name}${isSet}`);
|
||||
console.log(` ${s.description}`);
|
||||
console.log(` Get it: ${s.where}`);
|
||||
|
||||
@@ -1,9 +1,13 @@
|
||||
import { describe, test, expect, beforeAll } from 'bun:test';
|
||||
import { describe, test, expect, beforeAll, beforeEach, afterEach } from 'bun:test';
|
||||
import { mkdtempSync, mkdirSync, writeFileSync, rmSync } from 'fs';
|
||||
import { tmpdir } from 'os';
|
||||
import { join } from 'path';
|
||||
import {
|
||||
parseRecipe,
|
||||
isUnsafeHealthCheck,
|
||||
expandVars,
|
||||
executeHealthCheck,
|
||||
checkSecrets,
|
||||
parseOctet,
|
||||
hostnameToOctets,
|
||||
isPrivateIpv4,
|
||||
@@ -679,3 +683,77 @@ describe('getRecipeDirs (B1 trust boundary)', () => {
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
// --- #2789: secret resolution folds the config plane (buildGatewayConfig seam) ---
|
||||
|
||||
describe('secret resolution folds config plane (#2789)', () => {
|
||||
let dir: string;
|
||||
let savedHome: string | undefined;
|
||||
let savedKey: string | undefined;
|
||||
|
||||
beforeEach(() => {
|
||||
savedHome = process.env.GBRAIN_HOME;
|
||||
savedKey = process.env.OPENAI_API_KEY;
|
||||
dir = mkdtempSync(join(tmpdir(), 'gbrain-integrations-2789-'));
|
||||
mkdirSync(join(dir, '.gbrain'), { recursive: true });
|
||||
process.env.GBRAIN_HOME = dir;
|
||||
delete process.env.OPENAI_API_KEY;
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
if (savedHome === undefined) delete process.env.GBRAIN_HOME;
|
||||
else process.env.GBRAIN_HOME = savedHome;
|
||||
if (savedKey === undefined) delete process.env.OPENAI_API_KEY;
|
||||
else process.env.OPENAI_API_KEY = savedKey;
|
||||
rmSync(dir, { recursive: true, force: true });
|
||||
});
|
||||
|
||||
function writeConfig(cfg: Record<string, unknown>) {
|
||||
writeFileSync(join(dir, '.gbrain', 'config.json'), JSON.stringify(cfg));
|
||||
}
|
||||
|
||||
const secret = { name: 'OPENAI_API_KEY', description: 'test key', where: 'https://example.com' };
|
||||
|
||||
test('checkSecrets sees a key stored only in config.json', () => {
|
||||
writeConfig({ engine: 'pglite', openai_api_key: 'sk-test-config-only' });
|
||||
const { set, missing } = checkSecrets([secret]);
|
||||
expect(set).toEqual(['OPENAI_API_KEY']);
|
||||
expect(missing).toHaveLength(0);
|
||||
});
|
||||
|
||||
test('checkSecrets still reports missing when the key is nowhere', () => {
|
||||
writeConfig({ engine: 'pglite' });
|
||||
const { set, missing } = checkSecrets([secret]);
|
||||
expect(set).toHaveLength(0);
|
||||
expect(missing.map(m => m.name)).toEqual(['OPENAI_API_KEY']);
|
||||
});
|
||||
|
||||
test('expandVars expands a config-folded key', () => {
|
||||
writeConfig({ engine: 'pglite', openai_api_key: 'sk-from-config' });
|
||||
expect(expandVars('Bearer $OPENAI_API_KEY')).toBe('Bearer sk-from-config');
|
||||
});
|
||||
|
||||
test('non-empty process.env still wins over the config plane', () => {
|
||||
writeConfig({ engine: 'pglite', openai_api_key: 'sk-from-config' });
|
||||
process.env.OPENAI_API_KEY = 'sk-from-env';
|
||||
expect(expandVars('$OPENAI_API_KEY')).toBe('sk-from-env');
|
||||
});
|
||||
|
||||
test('env_exists health check sees a config-folded key', async () => {
|
||||
writeConfig({ engine: 'pglite', openai_api_key: 'sk-from-config' });
|
||||
const result = await executeHealthCheck(
|
||||
{ type: 'env_exists', name: 'OPENAI_API_KEY', label: 'key present' },
|
||||
'test-id',
|
||||
true,
|
||||
);
|
||||
expect(result.status).toBe('ok');
|
||||
expect(result.output).toContain('set');
|
||||
});
|
||||
|
||||
test('falls back to process.env when no config file exists (pre-init)', () => {
|
||||
// No config.json written — pre-`gbrain init` shape.
|
||||
process.env.OPENAI_API_KEY = 'sk-env-only';
|
||||
const { set } = checkSecrets([secret]);
|
||||
expect(set).toEqual(['OPENAI_API_KEY']);
|
||||
});
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user