mirror of
https://github.com/garrytan/gbrain.git
synced 2026-07-27 22:15:33 +00:00
fix(spend): SSRF-harden estimator fetch + complete off/uncapped across reindex/enrich/onboard (#2139)
Ship-stage codex pre-landing review caught four P1s in the secondary cost gates: - The delta estimator's fetch-first ran `git fetch` through the plain git() helper, bypassing the GIT_SSRF_FLAGS + GIT_TERMINAL_PROMPT=0 hardening that real sync uses. Added `fetchRemote()` to git-remote.ts (same flags as pullRepo) and route the estimator through it — a cost preview / dry-run can no longer hit a remote through a less-protected path. - `reindex --max-cost off`, `enrich --max-usd off`, `onboard --auto --max-usd off` were parsed but didn't actually proceed/uncap. Now: explicit off (and spend.posture=tokenmax) proceed past the confirmation/missing-cap refusal AND run uncapped. enrich threads an Infinity sentinel mapped to "no BudgetTracker ceiling" (never raw Infinity → no null in audit rows); reindex/onboard use their native undefined=uncapped path. Spend still ledgered. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 4.8
parent
a9485784bb
commit
af1dc092bc
+33
-17
@@ -508,8 +508,13 @@ export async function runEnrichCore(
|
||||
// One tracker reference for both the run and the post-hoc overage check.
|
||||
// External tracker (cycle phase): used as-is, no withBudgetTracker wrap (that
|
||||
// would REPLACE not stack). Internal: capped at maxCostUsd ?? DEFAULT.
|
||||
// v0.42.42.0 (#2139): Infinity = explicit "uncapped" (off / tokenmax) → pass
|
||||
// `undefined` so BudgetTracker runs without a ceiling (NOT raw Infinity, which
|
||||
// would serialize to null in audit rows). undefined-when-unset still → DEFAULT.
|
||||
const resolvedCap =
|
||||
opts.maxCostUsd === Infinity ? undefined : (opts.maxCostUsd ?? DEFAULT_MAX_COST_USD);
|
||||
const tracker = opts.budgetTracker ?? new BudgetTracker({
|
||||
maxCostUsd: opts.maxCostUsd ?? DEFAULT_MAX_COST_USD,
|
||||
maxCostUsd: resolvedCap,
|
||||
label: `enrich:${sourceId}`,
|
||||
});
|
||||
try {
|
||||
@@ -622,8 +627,15 @@ export function parseArgs(args: string[]): ParsedArgs {
|
||||
continue;
|
||||
}
|
||||
if (a === '--max-usd' || a === '--max-cost-usd') {
|
||||
const n = parseFloat(args[++i] ?? '');
|
||||
if (Number.isFinite(n) && n > 0) out.maxCostUsd = n;
|
||||
const raw = args[++i] ?? '';
|
||||
// v0.42.42.0 (#2139): off/unlimited/none → run uncapped (Infinity sentinel;
|
||||
// mapped to "no BudgetTracker ceiling" in runEnrichCore). Spend still ledgered.
|
||||
if (['off', 'unlimited', 'none'].includes(raw.trim().toLowerCase())) {
|
||||
out.maxCostUsd = Infinity;
|
||||
} else {
|
||||
const n = parseFloat(raw);
|
||||
if (Number.isFinite(n) && n > 0) out.maxCostUsd = n;
|
||||
}
|
||||
continue;
|
||||
}
|
||||
if (a === '--min-context') {
|
||||
@@ -801,22 +813,24 @@ export async function runEnrich(engine: BrainEngine, args: string[]): Promise<vo
|
||||
process.exit(1);
|
||||
}
|
||||
|
||||
// v0.42.42.0 (#2139, D15A): spend.posture=tokenmax means "cost isn't the
|
||||
// constraint" — the missing-cap refusals below lift and enrich runs UNCAPPED
|
||||
// (maxCostUsd stays undefined → BudgetTracker runs without a ceiling). Spend
|
||||
// is still ledgered by the tracker; posture removes the ceiling, not the
|
||||
// accounting. An explicit --max-usd always wins (precedence: per-call > posture).
|
||||
// v0.42.42.0 (#2139, D15A): enrich runs UNCAPPED when the operator says cost
|
||||
// isn't the constraint — either explicit `--max-usd off` (parsed to Infinity)
|
||||
// or `spend.posture=tokenmax` with no per-call cap. Uncapped → the missing-cap
|
||||
// refusals lift AND runEnrichCore passes no ceiling to the BudgetTracker (spend
|
||||
// still ledgered; posture removes the ceiling, not the accounting). An explicit
|
||||
// finite --max-usd always wins (precedence: per-call > posture).
|
||||
const explicitOff = parsed.maxCostUsd === Infinity;
|
||||
const { resolveSpendPosture } = await import('../core/spend-posture.ts');
|
||||
const tokenmaxUncapped =
|
||||
!parsed.dryRun && parsed.maxCostUsd === undefined &&
|
||||
(await resolveSpendPosture(engine)) === 'tokenmax';
|
||||
if (tokenmaxUncapped) {
|
||||
console.error('spend.posture=tokenmax: running uncapped, spend ledgered. docs: docs/operations/spend-controls.md');
|
||||
const posture = parsed.dryRun ? 'gated' : await resolveSpendPosture(engine);
|
||||
const uncapped =
|
||||
!parsed.dryRun && (explicitOff || (parsed.maxCostUsd === undefined && posture === 'tokenmax'));
|
||||
if (uncapped) {
|
||||
console.error(`${explicitOff ? '--max-usd off' : 'spend.posture=tokenmax'}: running uncapped, spend ledgered. docs: docs/operations/spend-controls.md`);
|
||||
}
|
||||
|
||||
// Non-TTY execute without --max-usd or --yes is refused (cost guardrail).
|
||||
if (!parsed.dryRun && parsed.maxCostUsd === undefined && !parsed.yes && !process.stdout.isTTY && !tokenmaxUncapped) {
|
||||
console.error('Refusing to spend without a cap in a non-interactive context. Pass --max-usd <FLOAT> or --yes.');
|
||||
if (!parsed.dryRun && parsed.maxCostUsd === undefined && !parsed.yes && !process.stdout.isTTY && !uncapped) {
|
||||
console.error('Refusing to spend without a cap in a non-interactive context. Pass --max-usd <FLOAT> (or `off`), --yes, or set spend.posture=tokenmax.');
|
||||
process.exit(1);
|
||||
}
|
||||
|
||||
@@ -825,7 +839,7 @@ export async function runEnrich(engine: BrainEngine, args: string[]): Promise<vo
|
||||
: (await listSources(engine)).map((s) => s.id);
|
||||
|
||||
// Dry-run cost preview (TTY) before spending.
|
||||
if (!parsed.dryRun && process.stdout.isTTY && !parsed.yes && parsed.maxCostUsd === undefined && !tokenmaxUncapped) {
|
||||
if (!parsed.dryRun && process.stdout.isTTY && !parsed.yes && parsed.maxCostUsd === undefined && !uncapped) {
|
||||
const limit = parsed.limit ?? DEFAULT_LIMIT;
|
||||
const est = (limit * sourceIds.length * COST_ESTIMATE_PER_PAGE_USD).toFixed(2);
|
||||
console.error(`About to enrich up to ${limit} page(s) per source across ${sourceIds.length} source(s), est. ~$${est}. Re-run with --max-usd or --yes to confirm.`);
|
||||
@@ -847,7 +861,9 @@ export async function runEnrich(engine: BrainEngine, args: string[]): Promise<vo
|
||||
limit: parsed.limit,
|
||||
workers: parsed.workers,
|
||||
model: parsed.model,
|
||||
maxCostUsd: parsed.maxCostUsd,
|
||||
// uncapped (off / tokenmax) → Infinity sentinel; runEnrichCore maps it
|
||||
// to "no BudgetTracker ceiling".
|
||||
maxCostUsd: uncapped ? Infinity : parsed.maxCostUsd,
|
||||
minContextChars: parsed.minContextChars,
|
||||
thinThreshold: parsed.thinThreshold,
|
||||
reenrichAfterMs: parsed.reenrichAfterMs,
|
||||
|
||||
+11
-3
@@ -45,6 +45,13 @@ export async function runOnboard(engine: BrainEngine, args: string[]): Promise<v
|
||||
// No-op without a pack_upgrade_available finding.
|
||||
const explain = args.includes('--explain');
|
||||
const targetScore = parseInt10(args, '--target-score') ?? 90;
|
||||
// v0.42.42.0 (#2139): `--max-usd off`/`unlimited`/`none` → run uncapped. maxUsd
|
||||
// stays undefined, which runRemediation already treats as no ceiling (skips the
|
||||
// est-cost refusal + BudgetTracker runs uncapped); `maxUsdOff` lifts the
|
||||
// missing-cap refusal below. Spend is still ledgered.
|
||||
const maxUsdIdx = args.indexOf('--max-usd');
|
||||
const maxUsdVal = maxUsdIdx >= 0 ? (args[maxUsdIdx + 1] ?? '').trim().toLowerCase() : '';
|
||||
const maxUsdOff = ['off', 'unlimited', 'none'].includes(maxUsdVal);
|
||||
const maxUsdRaw = parseFloat10(args, '--max-usd');
|
||||
const maxUsd = maxUsdRaw === null ? undefined : maxUsdRaw;
|
||||
|
||||
@@ -96,14 +103,15 @@ export async function runOnboard(engine: BrainEngine, args: string[]): Promise<v
|
||||
// remediation budget tracker. An explicit --max-usd always wins.
|
||||
if (auto && maxUsd === undefined) {
|
||||
const { resolveSpendPosture } = await import('../core/spend-posture.ts');
|
||||
if ((await resolveSpendPosture(engine)) === 'tokenmax') {
|
||||
process.stderr.write('spend.posture=tokenmax: onboard --auto running uncapped, spend ledgered. docs: docs/operations/spend-controls.md\n');
|
||||
const tokenmax = (await resolveSpendPosture(engine)) === 'tokenmax';
|
||||
if (maxUsdOff || tokenmax) {
|
||||
process.stderr.write(`${maxUsdOff ? '--max-usd off' : 'spend.posture=tokenmax'}: onboard --auto running uncapped, spend ledgered. docs: docs/operations/spend-controls.md\n`);
|
||||
} else {
|
||||
process.stderr.write(
|
||||
`gbrain onboard --auto refuses without --max-usd N.\n` +
|
||||
`Set a cap to avoid surprise spend:\n` +
|
||||
` gbrain onboard --auto --max-usd 5\n` +
|
||||
`Or set spend.posture=tokenmax to run uncapped. docs: docs/operations/spend-controls.md\n`,
|
||||
`Or pass --max-usd off / set spend.posture=tokenmax to run uncapped. docs: docs/operations/spend-controls.md\n`,
|
||||
);
|
||||
process.exit(2);
|
||||
}
|
||||
|
||||
@@ -444,16 +444,19 @@ export async function runReindexCodeCli(engine: BrainEngine, args: string[]): Pr
|
||||
}
|
||||
|
||||
// F3: --max-cost / --max-cost-usd both accepted for symmetry with brainstorm.
|
||||
// v0.42.42.0 (#2139): `off`/`unlimited`/`none` → no cap (undefined). Numeric
|
||||
// must be positive; `0`/garbage is rejected.
|
||||
// v0.42.42.0 (#2139): `off`/`unlimited`/`none` → no runtime cap AND an explicit
|
||||
// "cost isn't the constraint" decision that proceeds past the confirmation gate
|
||||
// (like --yes). Numeric must be positive; `0`/garbage is rejected.
|
||||
let maxCostUsd: number | undefined;
|
||||
let maxCostOff = false;
|
||||
for (const flag of ['--max-cost', '--max-cost-usd']) {
|
||||
const idx = args.indexOf(flag);
|
||||
if (idx >= 0) {
|
||||
const v = args[idx + 1];
|
||||
const t = (v ?? '').trim().toLowerCase();
|
||||
if (['off', 'unlimited', 'none'].includes(t)) {
|
||||
maxCostUsd = undefined; // no cap
|
||||
maxCostUsd = undefined; // no runtime cap (reindex skips the tracker when unset)
|
||||
maxCostOff = true;
|
||||
break;
|
||||
}
|
||||
const n = v ? parseFloat(v) : NaN;
|
||||
@@ -505,11 +508,14 @@ export async function runReindexCodeCli(engine: BrainEngine, args: string[]): Pr
|
||||
// constraint). The spend is still ledgered by the runtime BudgetTracker.
|
||||
const { resolveSpendPosture } = await import('../core/spend-posture.ts');
|
||||
const posture = await resolveSpendPosture(engine);
|
||||
if (posture === 'tokenmax') {
|
||||
// An explicit `--max-cost off` is the same "cost isn't the constraint"
|
||||
// signal as spend.posture=tokenmax — proceed past the confirmation gate.
|
||||
if (posture === 'tokenmax' || maxCostOff) {
|
||||
const gate = maxCostOff ? 'max_cost_off' : 'posture_tokenmax';
|
||||
if (json) {
|
||||
console.log(JSON.stringify({ status: 'proceeding', gate: 'posture_tokenmax', codePages: preview.totalPages, totalTokens: preview.totalTokens, costUsd, model: getEmbeddingModelName() }));
|
||||
console.log(JSON.stringify({ status: 'proceeding', gate, codePages: preview.totalPages, totalTokens: preview.totalTokens, costUsd, model: getEmbeddingModelName() }));
|
||||
} else {
|
||||
console.log(`${previewMsg} spend.posture=tokenmax: proceeding (informational). docs: docs/operations/spend-controls.md`);
|
||||
console.log(`${previewMsg} ${maxCostOff ? '--max-cost off' : 'spend.posture=tokenmax'}: proceeding (informational). docs: docs/operations/spend-controls.md`);
|
||||
}
|
||||
} else {
|
||||
const isTTY = Boolean(process.stdout.isTTY) && Boolean(process.stdin.isTTY);
|
||||
|
||||
@@ -23,6 +23,7 @@ import {
|
||||
computeSyncDelta,
|
||||
buildDetachedWorkingTreeManifest,
|
||||
} from '../core/sync-delta.ts';
|
||||
import { fetchRemote } from '../core/git-remote.ts';
|
||||
import {
|
||||
parseUsdLimit,
|
||||
formatUsdLimit,
|
||||
@@ -311,7 +312,10 @@ function resolveEstimateTarget(localPath: string): { target: string; detached: b
|
||||
}
|
||||
if (branch && branch !== 'HEAD' && hasOriginRemote(localPath)) {
|
||||
try {
|
||||
git(localPath, ['fetch', 'origin', branch]);
|
||||
// v0.42.42.0 (#2139): route through the SSRF-hardened fetch (same flags +
|
||||
// no-prompt env as pullRepo) — a cost preview / dry-run must NOT hit a
|
||||
// remote through a less-protected path than real sync.
|
||||
fetchRemote(localPath, branch, { timeoutMs: 30_000 });
|
||||
} catch {
|
||||
// fail-open: offline, auth failure, no upstream — fall through to local HEAD.
|
||||
}
|
||||
|
||||
+26
-1
@@ -131,7 +131,7 @@ export interface CloneOpts {
|
||||
|
||||
export class GitOperationError extends Error {
|
||||
constructor(
|
||||
public op: 'clone' | 'pull' | 'remote_get_url',
|
||||
public op: 'clone' | 'pull' | 'fetch' | 'remote_get_url',
|
||||
message: string,
|
||||
public cause?: unknown,
|
||||
) {
|
||||
@@ -217,6 +217,31 @@ export function pullRepo(repoPath: string, opts: { timeoutMs?: number } = {}): v
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Fetch a single remote branch with the SAME SSRF-defensive flags + no-prompt
|
||||
* env as cloneRepo/pullRepo (GIT_SSRF_FLAGS, --no-recurse-submodules,
|
||||
* GIT_TERMINAL_PROMPT=0). Used by the sync cost-estimator's fetch-first path
|
||||
* (#2139) so a cost preview / dry-run never hits a remote through a
|
||||
* less-protected route than real sync. Throws GitOperationError on failure;
|
||||
* the estimator catches and falls back to local HEAD.
|
||||
*/
|
||||
export function fetchRemote(repoPath: string, branch: string, opts: { timeoutMs?: number } = {}): void {
|
||||
const args: string[] = ['-C', repoPath, ...GIT_SSRF_FLAGS, 'fetch', ...GIT_SSRF_SUBCOMMAND_FLAGS, 'origin', branch];
|
||||
try {
|
||||
execFileSync('git', args, {
|
||||
stdio: ['ignore', 'pipe', 'pipe'],
|
||||
timeout: opts.timeoutMs ?? 30_000,
|
||||
env: { ...process.env, ...GIT_ENV },
|
||||
});
|
||||
} catch (e) {
|
||||
throw new GitOperationError(
|
||||
'fetch',
|
||||
`git fetch failed in ${repoPath}: ${(e as Error).message}`,
|
||||
e,
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
export type RepoState =
|
||||
| 'healthy'
|
||||
| 'missing'
|
||||
|
||||
@@ -0,0 +1,53 @@
|
||||
/**
|
||||
* v0.42.42.0 (#2139) — `--max-usd off` / `--max-cost off` uncapped-switch pins
|
||||
* across enrich / onboard / reindex (the T6 secondary cost gates).
|
||||
*
|
||||
* The enrich arg parser carries the real logic (off → Infinity sentinel, mapped
|
||||
* to "no BudgetTracker ceiling" in runEnrichCore), so it gets a direct unit test.
|
||||
* reindex + onboard detect `off` inline at the CLI dispatch and proceed past the
|
||||
* confirmation / missing-cap refusal; those are pinned as source-level regression
|
||||
* guards (the codex pre-landing review found all three half-built — these keep
|
||||
* them from silently regressing without standing up full CLI+gateway harnesses).
|
||||
*/
|
||||
import { describe, test, expect } from 'bun:test';
|
||||
import { parseArgs } from '../src/commands/enrich.ts';
|
||||
|
||||
describe('enrich parseArgs — --max-usd off → uncapped (Infinity sentinel)', () => {
|
||||
test('off / unlimited / none (case-insensitive) → Infinity', () => {
|
||||
for (const v of ['off', 'OFF', 'unlimited', 'none', 'None']) {
|
||||
expect(parseArgs(['--max-usd', v]).maxCostUsd).toBe(Infinity);
|
||||
}
|
||||
// --max-cost-usd alias too.
|
||||
expect(parseArgs(['--max-cost-usd', 'off']).maxCostUsd).toBe(Infinity);
|
||||
});
|
||||
test('finite positive number passes through; absent → undefined; garbage → undefined', () => {
|
||||
expect(parseArgs(['--max-usd', '5']).maxCostUsd).toBe(5);
|
||||
expect(parseArgs([]).maxCostUsd).toBeUndefined();
|
||||
expect(parseArgs(['--max-usd', 'abc']).maxCostUsd).toBeUndefined();
|
||||
expect(parseArgs(['--max-usd', '0']).maxCostUsd).toBeUndefined(); // non-positive ignored
|
||||
});
|
||||
});
|
||||
|
||||
describe('reindex / onboard off-switch dispatch (regression guards)', () => {
|
||||
test('reindex-code: --max-cost off proceeds past the confirmation gate', async () => {
|
||||
const src = await Bun.file(new URL('../src/commands/reindex-code.ts', import.meta.url)).text();
|
||||
// off sets maxCostOff and the gate proceeds when (tokenmax || maxCostOff).
|
||||
expect(src).toMatch(/maxCostOff\s*=\s*true/);
|
||||
expect(src).toMatch(/posture === 'tokenmax' \|\| maxCostOff/);
|
||||
});
|
||||
|
||||
test('onboard: --max-usd off lifts the --auto missing-cap refusal', async () => {
|
||||
const src = await Bun.file(new URL('../src/commands/onboard.ts', import.meta.url)).text();
|
||||
expect(src).toMatch(/maxUsdOff\s*=/);
|
||||
// refusal skipped when (maxUsdOff || tokenmax).
|
||||
expect(src).toMatch(/maxUsdOff \|\| tokenmax/);
|
||||
});
|
||||
|
||||
test('enrich: uncapped path maps the Infinity sentinel to no BudgetTracker ceiling', async () => {
|
||||
const src = await Bun.file(new URL('../src/commands/enrich.ts', import.meta.url)).text();
|
||||
// The sentinel must become `undefined` at the tracker (never raw Infinity,
|
||||
// which serializes to null in audit rows).
|
||||
expect(src).toMatch(/opts\.maxCostUsd === Infinity \? undefined/);
|
||||
expect(src).toMatch(/uncapped \? Infinity : parsed\.maxCostUsd/);
|
||||
});
|
||||
});
|
||||
Reference in New Issue
Block a user