From 03cd52631b51506fa665239373c62882ced32aee Mon Sep 17 00:00:00 2001 From: Time Attakc <89218912+time-attack@users.noreply.github.com> Date: Thu, 23 Jul 2026 16:45:36 -0700 Subject: [PATCH] fix(thin-client): map --source scope onto source_id for remote-routed ops (#3086) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(thin-client): map --source/GBRAIN_SOURCE/.gbrain-source onto source_id for routed ops (#2098) The thin-client route short-circuits before makeContext, so the 6-tier source resolution never ran and `gbrain query --source X` against a remote brain sent the unknown `source` key verbatim — the server op ignored it and searched unscoped. applyThinClientSourceScope now runs the engine-free tiers (flag → env → dotfile; DB-backed tiers need an engine, and the server's grant scoping covers the rest) and sets the op's source_id wire param. Ops declaring their own `source` param are untouched; an explicit --source on an op with no source_id wire param errors loudly instead of silently dropping; explicit --source-id/--all-sources on the wire win over ambient tiers. Fixes #2098 Co-Authored-By: Claude Fable 5 * test(thin-client): use withEnv() instead of direct process.env mutation (test-isolation R1) CI verify failed on check:test-isolation — thin-client-source-scope.test.ts mutated process.env.GBRAIN_SOURCE via beforeEach/afterEach. Wrapped each test body in withEnv() from test/helpers/with-env.ts instead. Co-Authored-By: Claude Fable 5 * fix(thin-client): keep ambient scope out of get_skill's non-scope source_id param get_skill's source_id is a mode switch (host catalog vs brain-resident-pack lookup), not a read-scope filter. Ambient GBRAIN_SOURCE / .gbrain-source injection would silently reroute 'gbrain skill ' on thin clients to getResidentSkillDetail. Exclude it via NON_SCOPE_SOURCE_ID_OPS; explicit --source-id still passes through, explicit --source errors with a hint. Co-Authored-By: Claude Fable 5 --------- Co-authored-by: Sinabina Co-authored-by: Claude Fable 5 Co-authored-by: Garry Tan --- src/cli.ts | 64 ++++++++++++ src/core/source-resolver.ts | 27 +++++ test/thin-client-source-scope.test.ts | 142 ++++++++++++++++++++++++++ 3 files changed, 233 insertions(+) create mode 100644 test/thin-client-source-scope.test.ts diff --git a/src/cli.ts b/src/cli.ts index 49a96e6df..b12b776f0 100755 --- a/src/cli.ts +++ b/src/cli.ts @@ -24,6 +24,7 @@ import type { GBrainConfig } from './core/config.ts'; import type { AIGatewayConfig } from './core/ai/types.ts'; import type { BrainEngine } from './core/engine.ts'; import { operations, OperationError } from './core/operations.ts'; +import { resolveSourceIdEngineFree } from './core/source-resolver.ts'; import { formatVolunteeredPage } from './core/context/volunteer.ts'; import type { Operation, OperationContext } from './core/operations.ts'; import { shouldForceExitAfterMain, finishCliTeardown, flushThenExit, currentExitCode, setCliExitVerdict } from './core/cli-force-exit.ts'; @@ -384,6 +385,15 @@ async function main() { if (op.localOnly) { refuseThinClient(command, cfgPre!.remote_mcp!.mcp_url); } + // #2098: the local path resolves --source / GBRAIN_SOURCE / .gbrain-source + // inside makeContext (ctx.sourceId), which this route never reaches — so + // scope must be mapped onto the op's source_id wire param before the call. + try { + applyThinClientSourceScope(op, params); + } catch (e: unknown) { + console.error(e instanceof Error ? e.message : String(e)); + process.exit(1); + } await runThinClientRouted(op, params, cfgPre!, cliOpts); return; } @@ -804,6 +814,60 @@ export function parseOpArgs(op: Operation, args: string[]): Record, + cwd?: string, +): void { + if ('source' in op.params) return; // the op owns --source; not a scope flag + const explicit = typeof params.source === 'string' && params.source.length > 0 + ? (params.source as string) + : null; + delete params.source; // never a wire param on these ops — don't leak it + // Explicit per-call scope already on the wire wins over ambient tiers. + if (params.source_id !== undefined || params.all_sources === true) { + if (explicit) { + throw new Error('Pass either --source or --source-id/--all-sources, not both.'); + } + return; + } + const resolved = resolveSourceIdEngineFree(explicit, cwd); + if (!resolved) return; + if (!('source_id' in op.params) || NON_SCOPE_SOURCE_ID_OPS.has(op.name)) { + if (explicit) { + const hint = NON_SCOPE_SOURCE_ID_OPS.has(op.name) + ? `(its source_id parameter is not a scope filter; pass --source-id explicitly if you mean it)` + : `(the remote op has no source_id parameter; the server scopes it to your grant)`; + throw new Error( + `gbrain ${op.cliHints?.name || op.name} does not accept --source on a thin-client install ${hint}.`, + ); + } + return; // ambient env/dotfile scope with nowhere to send it + } + params.source_id = resolved; +} + async function makeContext(engine: BrainEngine, params: Record): Promise { // v0.31.8 (D11): resolve sourceId via the canonical 6-tier chain. Honors // --source / GBRAIN_SOURCE / .gbrain-source / path-match / brain default / diff --git a/src/core/source-resolver.ts b/src/core/source-resolver.ts index d81d3878f..03f9eb0c0 100644 --- a/src/core/source-resolver.ts +++ b/src/core/source-resolver.ts @@ -160,6 +160,33 @@ export async function resolveSourceId( return 'default'; } +/** + * Engine-free tiers (1-3) of the resolution chain: explicit flag → + * GBRAIN_SOURCE env → .gbrain-source dotfile walk. Used by the thin-client + * CLI path (#2098), which has no local engine to run tiers 4-6 or + * assertSourceExists against — the remote server enforces existence + grant. + * Returns null when no engine-free tier fires. + */ +export function resolveSourceIdEngineFree( + explicit: string | null | undefined, + cwd: string = process.cwd(), +): string | null { + if (explicit) { + if (!SOURCE_ID_RE.test(explicit)) { + throw new Error(`Invalid --source value "${explicit}". Must match [a-z0-9-]{1,32}.`); + } + return explicit; + } + const env = process.env.GBRAIN_SOURCE; + if (env && env.length > 0) { + if (!SOURCE_ID_RE.test(env)) { + throw new Error(`Invalid GBRAIN_SOURCE value "${env}". Must match [a-z0-9-]{1,32}.`); + } + return env; + } + return readDotfileWalk(cwd); +} + /** * Returns the id of the SINGLE registered non-default source with a * local_path, when exactly one such row exists. Returns null when: diff --git a/test/thin-client-source-scope.test.ts b/test/thin-client-source-scope.test.ts new file mode 100644 index 000000000..d366f319d --- /dev/null +++ b/test/thin-client-source-scope.test.ts @@ -0,0 +1,142 @@ +/** + * #2098: thin-client routing dropped --source / GBRAIN_SOURCE / .gbrain-source. + * + * The local CLI path resolves source scope in makeContext (ctx.sourceId); the + * thin-client route short-circuits before that and sent params verbatim, so + * `gbrain query --source X` against a remote brain silently searched unscoped + * (the server op ignores the unknown `source` key). + * + * applyThinClientSourceScope runs the engine-free tiers (flag → env → dotfile) + * and maps the result onto the op's `source_id` wire param. These tests fail + * without the fix (params.source_id stays undefined / params.source leaks). + */ + +import { describe, test, expect } from 'bun:test'; +import { mkdtempSync, rmSync, writeFileSync } from 'fs'; +import { join } from 'path'; +import { tmpdir } from 'os'; +import { applyThinClientSourceScope, parseOpArgs } from '../src/cli.ts'; +import { operationsByName } from '../src/core/operations.ts'; +import { withEnv } from './helpers/with-env.ts'; + +const queryOp = operationsByName.query; + +describe('applyThinClientSourceScope (#2098)', () => { + test('--source maps onto the query op wire param source_id', async () => { + await withEnv({ GBRAIN_SOURCE: undefined }, () => { + const params = parseOpArgs(queryOp, ['find things', '--source', 'wiki']); + expect(params.source).toBe('wiki'); // pre-fix state: wrong key + applyThinClientSourceScope(queryOp, params, '/'); + expect(params.source_id).toBe('wiki'); + expect('source' in params).toBe(false); // never leaks the unknown key + }); + }); + + test('GBRAIN_SOURCE env tier fires when no flag is passed', async () => { + await withEnv({ GBRAIN_SOURCE: 'gstack' }, () => { + const params = parseOpArgs(queryOp, ['find things']); + applyThinClientSourceScope(queryOp, params, '/'); + expect(params.source_id).toBe('gstack'); + }); + }); + + test('.gbrain-source dotfile tier fires when flag and env are absent', async () => { + await withEnv({ GBRAIN_SOURCE: undefined }, () => { + const tmp = mkdtempSync(join(tmpdir(), 'gbrain-thin-scope-')); + try { + writeFileSync(join(tmp, '.gbrain-source'), 'essays\n'); + const params = parseOpArgs(queryOp, ['find things']); + applyThinClientSourceScope(queryOp, params, tmp); + expect(params.source_id).toBe('essays'); + } finally { + rmSync(tmp, { recursive: true, force: true }); + } + }); + }); + + test('explicit --source-id on the wire wins over ambient env scope', async () => { + await withEnv({ GBRAIN_SOURCE: 'gstack' }, () => { + const params = parseOpArgs(queryOp, ['find things', '--source-id', 'wiki']); + applyThinClientSourceScope(queryOp, params, '/'); + expect(params.source_id).toBe('wiki'); + }); + }); + + test('--source together with --source-id is rejected loudly', async () => { + await withEnv({ GBRAIN_SOURCE: undefined }, () => { + const params = parseOpArgs(queryOp, ['q', '--source', 'a', '--source-id', 'b']); + expect(() => applyThinClientSourceScope(queryOp, params, '/')).toThrow(/not both/); + }); + }); + + test('invalid --source value is rejected loudly', async () => { + await withEnv({ GBRAIN_SOURCE: undefined }, () => { + const params = parseOpArgs(queryOp, ['q', '--source', 'Bad_Value!']); + expect(() => applyThinClientSourceScope(queryOp, params, '/')).toThrow(/Invalid --source/); + }); + }); + + test('--source on an op with no source_id wire param errors instead of silently dropping', async () => { + await withEnv({ GBRAIN_SOURCE: undefined }, () => { + const op = operationsByName.add_tag; + expect('source_id' in op.params).toBe(false); + const params = { slug: 'x', tag: 'y', source: 'wiki' }; + expect(() => applyThinClientSourceScope(op, params, '/')).toThrow(/--source/); + }); + }); + + test('ambient env scope on an op with no source_id wire param is ignored (no throw)', async () => { + await withEnv({ GBRAIN_SOURCE: 'wiki' }, () => { + const op = operationsByName.add_tag; + const params: Record = { slug: 'x', tag: 'y' }; + applyThinClientSourceScope(op, params, '/'); + expect(params.source_id).toBeUndefined(); + }); + }); + + test('ops that declare their OWN source param are left untouched', async () => { + await withEnv({ GBRAIN_SOURCE: undefined }, () => { + const op = operationsByName.put_raw_data; + expect('source' in op.params).toBe(true); + const params: Record = { slug: 'x', source: 'crustdata', data: {} }; + applyThinClientSourceScope(op, params, '/'); + expect(params.source).toBe('crustdata'); + expect(params.source_id).toBeUndefined(); + }); + }); + + test('get_skill: ambient scope never leaks into its non-scope source_id param', async () => { + await withEnv({ GBRAIN_SOURCE: 'wiki' }, () => { + const op = operationsByName.get_skill; + expect('source_id' in op.params).toBe(true); // has the param, but it is a mode switch + const params: Record = { name: 'ingest' }; + applyThinClientSourceScope(op, params, '/'); + expect(params.source_id).toBeUndefined(); // would flip host catalog → brain-pack lookup + }); + }); + + test('get_skill: explicit --source errors instead of masquerading as --source-id', async () => { + await withEnv({ GBRAIN_SOURCE: undefined }, () => { + const op = operationsByName.get_skill; + const params: Record = { name: 'ingest', source: 'wiki' }; + expect(() => applyThinClientSourceScope(op, params, '/')).toThrow(/--source-id/); + }); + }); + + test('get_skill: explicit --source-id passes through untouched', async () => { + await withEnv({ GBRAIN_SOURCE: 'gstack' }, () => { + const op = operationsByName.get_skill; + const params: Record = { name: 'ingest', source_id: 'wiki' }; + applyThinClientSourceScope(op, params, '/'); + expect(params.source_id).toBe('wiki'); + }); + }); + + test('no scope from any tier leaves params unchanged', async () => { + await withEnv({ GBRAIN_SOURCE: undefined }, () => { + const params = parseOpArgs(queryOp, ['find things']); + applyThinClientSourceScope(queryOp, params, '/'); + expect(params.source_id).toBeUndefined(); + }); + }); +});