mirror of
https://github.com/garrytan/gbrain.git
synced 2026-07-30 19:49:14 +00:00
fix(sources): make the __all__ sentinel work in every resolution tier (#1712)
The `__all__` sentinel had two spellings and only one worked: as a
per-call source_id param, resolveRequestedScope understood it; as
--source __all__ or GBRAIN_SOURCE=__all__, SOURCE_ID_RE (which forbids
underscores) made all three resolver entry points throw. makeContext's
blanket catch then silently fell back to sourceId 'default' — making
the documented span-everything flag STRICTLY NARROWER than passing no
flag at all, because the catch also discarded the #2561/#3242 federated
widening:
unqualified read (federated brain) -> {"sourceIds":["default","src-a","src-b"]}
--source __all__ (pre-fix) -> {"sourceId":"default"}
--source __all__ (post-fix) -> {} (spans the brain)
Fix:
- src/core/source-id.ts: export ALL_SOURCES = '__all__'. SOURCE_ID_RE
itself is NOT loosened — it still guards source creation, lock ids,
and path joins, and its underscore rejection is what makes the
sentinel collision-free.
- src/core/source-resolver.ts: the explicit and env tiers of
resolveSourceId / resolveSourceIdEngineFree / resolveSourceWithTier
pass the sentinel through verbatim (skipping the regex and
assertSourceExists). Covers --source (#1712/#2289) and
GBRAIN_SOURCE (#2140), local and thin-client alike.
- src/core/operations.ts: sourceScopeOpts — the single choke point every
read-side scope helper delegates to — translates ctx.sourceId ===
ALL_SOURCES into {} for trusted local callers (strictly remote ===
false) and keeps the unsatisfiable literal for remote/untrusted
callers, so the sentinel can never widen past a caller's grant
(fail-closed). A federated grant still wins over the sentinel.
- src/cli.ts: makeContext's catch now rethrows when an explicit
--source was passed — a source that genuinely fails to resolve errors
loudly instead of silently becoming 'default' (the silent fallback is
what turned three bug reports into debugging sessions).
Tests (test/all-sources-sentinel.test.ts) fail on unmodified master
(9/13, behaviorally) and pass with the fix; the 4 that pass on both
sides pin invariants that must hold on both (remote fail-closed
literal, grant precedence, invalid-id rejection).
Closes #1712. #2289 and #2140 were closed as duplicates of it.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 5
parent
6920744dd8
commit
2e072d9e8c
+12
-6
@@ -872,7 +872,8 @@ export function applyThinClientSourceScope(
|
||||
params.source_id = resolved;
|
||||
}
|
||||
|
||||
async function makeContext(engine: BrainEngine, params: Record<string, unknown>): Promise<OperationContext> {
|
||||
// Exported for tests (same import-safety contract as applyThinClientSourceScope).
|
||||
export async function makeContext(engine: BrainEngine, params: Record<string, unknown>): Promise<OperationContext> {
|
||||
// v0.31.8 (D11): resolve sourceId via the canonical 6-tier chain. Honors
|
||||
// --source / GBRAIN_SOURCE / .gbrain-source / path-match / brain default /
|
||||
// 'default'. Wrapped in try/catch so a doctor / single-source brain that
|
||||
@@ -884,16 +885,21 @@ async function makeContext(engine: BrainEngine, params: Record<string, unknown>)
|
||||
// trusted local boundary) and consumed by federatedSearchScope in
|
||||
// operations.ts, which additionally gates on ctx.remote === false.
|
||||
let localFederated: string[] | undefined;
|
||||
// params.source is set when a CLI flag was parsed for the op (rare; most
|
||||
// CLI ops don't take --source). Falls through to env/dotfile/path-match.
|
||||
const explicit = (params.source as string | undefined) ?? null;
|
||||
try {
|
||||
const { resolveSourceWithTier, localFederatedSourceIds } = await import('./core/source-resolver.ts');
|
||||
// params.source is set when a CLI flag was parsed for the op (rare; most
|
||||
// CLI ops don't take --source). Falls through to env/dotfile/path-match.
|
||||
const explicit = (params.source as string | undefined) ?? null;
|
||||
const resolved = await resolveSourceWithTier(engine, explicit);
|
||||
sourceId = resolved.source_id;
|
||||
localFederated = await localFederatedSourceIds(engine, resolved.source_id, resolved.tier);
|
||||
} catch {
|
||||
// Source resolution failed (e.g. sources table doesn't exist on a fresh
|
||||
} catch (err) {
|
||||
// #1712: an EXPLICIT --source that fails to resolve (invalid id, or a
|
||||
// source that doesn't exist) must error loudly — the blanket swallow
|
||||
// turned `--source __all__` and typos into a silent `default` scope,
|
||||
// which is how three bug reports became debugging sessions.
|
||||
if (explicit) throw err;
|
||||
// Ambient resolution failed (e.g. sources table doesn't exist on a fresh
|
||||
// pre-init brain). Leave sourceId unset; engine read methods fall through
|
||||
// to the cross-source view (D16 back-compat path).
|
||||
sourceId = undefined;
|
||||
|
||||
+10
-1
@@ -28,6 +28,7 @@ import { isSearchMode } from './search/mode.ts';
|
||||
import { stampEvidence } from './search/evidence.ts';
|
||||
import type { SearchResult } from './types.ts';
|
||||
import { CJK_SLUG_CHARS, PAGE_SLUG_SEG } from './cjk.ts';
|
||||
import { ALL_SOURCES } from './source-id.ts';
|
||||
import * as db from './db.ts';
|
||||
import { VERSION } from '../version.ts';
|
||||
import {
|
||||
@@ -486,6 +487,14 @@ export function sourceScopeOpts(ctx: OperationContext): { sourceId?: string; sou
|
||||
// value of `[]` MUST NOT widen scope to "all sources" by being interpreted
|
||||
// as "no filter."
|
||||
if (allowed && allowed.length > 0) return { sourceIds: allowed };
|
||||
// #1712: the __all__ sentinel spans the brain — but ONLY for trusted local
|
||||
// callers (strictly `remote === false`). For remote/untrusted callers the
|
||||
// literal stays as-is: it can never match a real source id (underscores are
|
||||
// rejected at creation), so the read fail-closes to empty rather than
|
||||
// widening past the caller's grant. Do NOT "simplify" this to `{}`.
|
||||
if (ctx.sourceId === ALL_SOURCES) {
|
||||
return ctx.remote === false ? {} : { sourceId: ctx.sourceId };
|
||||
}
|
||||
if (ctx.sourceId) return { sourceId: ctx.sourceId };
|
||||
return {};
|
||||
}
|
||||
@@ -553,7 +562,7 @@ export function resolveRequestedScope(
|
||||
sourceIdParam: string | undefined,
|
||||
allSourcesParam = false,
|
||||
): { sourceId?: string; sourceIds?: string[] } {
|
||||
const wantsAll = allSourcesParam || sourceIdParam === '__all__';
|
||||
const wantsAll = allSourcesParam || sourceIdParam === ALL_SOURCES;
|
||||
if (wantsAll) {
|
||||
return ctx.remote === false ? {} : sourceScopeOpts(ctx);
|
||||
}
|
||||
|
||||
@@ -33,6 +33,17 @@
|
||||
|
||||
export const SOURCE_ID_RE = /^[a-z0-9](?:[a-z0-9-]{0,30}[a-z0-9])?$/;
|
||||
|
||||
/**
|
||||
* Sentinel meaning "span every source" (#1712). Deliberately NOT a valid
|
||||
* source id (underscores are rejected by SOURCE_ID_RE), so it can never
|
||||
* collide with a real source, be created via `sources add`, or leak into
|
||||
* lock ids / path joins. The resolver's explicit/env tiers pass it through
|
||||
* verbatim; `sourceScopeOpts` translates it to an unscoped read for trusted
|
||||
* local callers and keeps it as an unsatisfiable literal for remote callers
|
||||
* (fail-closed).
|
||||
*/
|
||||
export const ALL_SOURCES = '__all__';
|
||||
|
||||
/** Returns true if the string matches the canonical source_id regex. */
|
||||
export function isValidSourceId(s: unknown): s is string {
|
||||
return typeof s === 'string' && SOURCE_ID_RE.test(s);
|
||||
|
||||
@@ -17,9 +17,13 @@ import { readFileSync, lstatSync, type Stats } from 'fs';
|
||||
import { join, dirname, resolve } from 'path';
|
||||
import type { BrainEngine } from './engine.ts';
|
||||
import { isSourceFederated } from './sources-load.ts';
|
||||
import { SOURCE_ID_RE, isValidSourceId } from './source-id.ts';
|
||||
import { SOURCE_ID_RE, isValidSourceId, ALL_SOURCES } from './source-id.ts';
|
||||
import { isTrustedDotfile, realpathOrResolve } from './path-confine.ts';
|
||||
|
||||
// Re-export so scope-resolution call sites can import the sentinel from
|
||||
// either module (#1712).
|
||||
export { ALL_SOURCES };
|
||||
|
||||
const DOTFILE = '.gbrain-source';
|
||||
// Canonical SOURCE_ID_RE imported from `source-id.ts` (single source of truth).
|
||||
// Re-exported below as `__testing.SOURCE_ID_RE` for legacy test imports.
|
||||
@@ -83,8 +87,11 @@ export async function resolveSourceId(
|
||||
explicit: string | null | undefined,
|
||||
cwd: string = process.cwd(),
|
||||
): Promise<string> {
|
||||
// 1. Explicit flag wins.
|
||||
// 1. Explicit flag wins. The __all__ sentinel passes through verbatim
|
||||
// (#1712) — it is not a source id, so it skips both the regex and
|
||||
// assertSourceExists; sourceScopeOpts gives it span-everything semantics.
|
||||
if (explicit) {
|
||||
if (explicit === ALL_SOURCES) return ALL_SOURCES;
|
||||
if (!SOURCE_ID_RE.test(explicit)) {
|
||||
throw new Error(`Invalid --source value "${explicit}". Must match [a-z0-9-]{1,32}.`);
|
||||
}
|
||||
@@ -92,9 +99,10 @@ export async function resolveSourceId(
|
||||
return explicit;
|
||||
}
|
||||
|
||||
// 2. Env var.
|
||||
// 2. Env var. Same __all__ pass-through (#2140).
|
||||
const env = process.env.GBRAIN_SOURCE;
|
||||
if (env && env.length > 0) {
|
||||
if (env === ALL_SOURCES) return ALL_SOURCES;
|
||||
if (!SOURCE_ID_RE.test(env)) {
|
||||
throw new Error(`Invalid GBRAIN_SOURCE value "${env}". Must match [a-z0-9-]{1,32}.`);
|
||||
}
|
||||
@@ -173,6 +181,7 @@ export function resolveSourceIdEngineFree(
|
||||
cwd: string = process.cwd(),
|
||||
): string | null {
|
||||
if (explicit) {
|
||||
if (explicit === ALL_SOURCES) return ALL_SOURCES; // #1712 sentinel pass-through
|
||||
if (!SOURCE_ID_RE.test(explicit)) {
|
||||
throw new Error(`Invalid --source value "${explicit}". Must match [a-z0-9-]{1,32}.`);
|
||||
}
|
||||
@@ -180,6 +189,7 @@ export function resolveSourceIdEngineFree(
|
||||
}
|
||||
const env = process.env.GBRAIN_SOURCE;
|
||||
if (env && env.length > 0) {
|
||||
if (env === ALL_SOURCES) return ALL_SOURCES; // #2140 sentinel pass-through
|
||||
if (!SOURCE_ID_RE.test(env)) {
|
||||
throw new Error(`Invalid GBRAIN_SOURCE value "${env}". Must match [a-z0-9-]{1,32}.`);
|
||||
}
|
||||
@@ -315,8 +325,11 @@ export async function resolveSourceWithTier(
|
||||
explicit: string | null | undefined,
|
||||
cwd: string = process.cwd(),
|
||||
): Promise<{ source_id: string; tier: SourceTier; detail?: string }> {
|
||||
// 1. Explicit flag wins.
|
||||
// 1. Explicit flag wins. __all__ sentinel passes through verbatim (#1712).
|
||||
if (explicit) {
|
||||
if (explicit === ALL_SOURCES) {
|
||||
return { source_id: ALL_SOURCES, tier: 'flag', detail: `--source ${ALL_SOURCES} (spans all sources)` };
|
||||
}
|
||||
if (!SOURCE_ID_RE.test(explicit)) {
|
||||
throw new Error(`Invalid --source value "${explicit}". Must match [a-z0-9-]{1,32}.`);
|
||||
}
|
||||
@@ -324,9 +337,12 @@ export async function resolveSourceWithTier(
|
||||
return { source_id: explicit, tier: 'flag', detail: `--source ${explicit}` };
|
||||
}
|
||||
|
||||
// 2. Env var.
|
||||
// 2. Env var. Same __all__ pass-through (#2140).
|
||||
const env = process.env.GBRAIN_SOURCE;
|
||||
if (env && env.length > 0) {
|
||||
if (env === ALL_SOURCES) {
|
||||
return { source_id: ALL_SOURCES, tier: 'env', detail: `GBRAIN_SOURCE=${ALL_SOURCES} (spans all sources)` };
|
||||
}
|
||||
if (!SOURCE_ID_RE.test(env)) {
|
||||
throw new Error(`Invalid GBRAIN_SOURCE value "${env}". Must match [a-z0-9-]{1,32}.`);
|
||||
}
|
||||
|
||||
@@ -0,0 +1,176 @@
|
||||
/**
|
||||
* #1712 (dupes #2289, #2140) — the `__all__` sentinel must work in EVERY
|
||||
* resolution tier, not just as a per-call `source_id` param.
|
||||
*
|
||||
* The bug: SOURCE_ID_RE forbids underscores, so `--source __all__` and
|
||||
* `GBRAIN_SOURCE=__all__` threw in the resolver; the CLI's makeContext
|
||||
* blanket-caught that and silently fell back to `sourceId: 'default'` —
|
||||
* making the documented span-everything flag STRICTLY NARROWER than passing
|
||||
* no flag at all (the catch also discarded the #2561/#3242 federated
|
||||
* widening). Meanwhile sourceScopeOpts treated a ctx.sourceId of '__all__'
|
||||
* as an unsatisfiable literal.
|
||||
*
|
||||
* Uses the literal '__all__' (not the ALL_SOURCES constant) so these tests
|
||||
* load and run behaviorally against pre-fix trees.
|
||||
*/
|
||||
import { describe, test, expect } from 'bun:test';
|
||||
import { withEnv } from './helpers/with-env.ts';
|
||||
import {
|
||||
resolveSourceId,
|
||||
resolveSourceIdEngineFree,
|
||||
resolveSourceWithTier,
|
||||
} from '../src/core/source-resolver.ts';
|
||||
import {
|
||||
sourceScopeOpts,
|
||||
federatedSearchScope,
|
||||
type OperationContext,
|
||||
} from '../src/core/operations.ts';
|
||||
import type { BrainEngine } from '../src/core/engine.ts';
|
||||
|
||||
// Stub engine: registered sources + no local_path rows + no default config.
|
||||
function makeStub(registeredSources: string[]): BrainEngine {
|
||||
return {
|
||||
kind: 'pglite',
|
||||
executeRaw: async <T>(sql: string, params?: unknown[]): Promise<T[]> => {
|
||||
if (sql.includes('SELECT id FROM sources WHERE id = $1')) {
|
||||
const target = params?.[0];
|
||||
return registeredSources.includes(target as string)
|
||||
? [{ id: target } as unknown as T]
|
||||
: [];
|
||||
}
|
||||
return [];
|
||||
},
|
||||
getConfig: async () => null,
|
||||
} as unknown as BrainEngine;
|
||||
}
|
||||
|
||||
function ctxOf(overrides: Partial<OperationContext> = {}): OperationContext {
|
||||
return {
|
||||
engine: {} as any,
|
||||
config: {} as any,
|
||||
logger: console as any,
|
||||
dryRun: false,
|
||||
remote: true,
|
||||
sourceId: 'default',
|
||||
...overrides,
|
||||
};
|
||||
}
|
||||
|
||||
// ── Resolver tiers pass the sentinel through verbatim ──────────────────
|
||||
|
||||
describe('source-resolver — __all__ sentinel pass-through', () => {
|
||||
test('resolveSourceId: explicit --source __all__ resolves (no regex throw, no existence check)', async () => {
|
||||
// '__all__' is deliberately NOT in the registered set — the sentinel
|
||||
// must skip assertSourceExists (it is not a source id).
|
||||
const id = await resolveSourceId(makeStub(['default']), '__all__', '/nonexistent');
|
||||
expect(id).toBe('__all__');
|
||||
});
|
||||
|
||||
test('resolveSourceId: GBRAIN_SOURCE=__all__ resolves (#2140)', async () => {
|
||||
await withEnv({ GBRAIN_SOURCE: '__all__' }, async () => {
|
||||
const id = await resolveSourceId(makeStub(['default']), null, '/nonexistent');
|
||||
expect(id).toBe('__all__');
|
||||
});
|
||||
});
|
||||
|
||||
test('resolveSourceIdEngineFree: explicit + env __all__ (thin-client path)', async () => {
|
||||
expect(resolveSourceIdEngineFree('__all__', '/nonexistent')).toBe('__all__');
|
||||
await withEnv({ GBRAIN_SOURCE: '__all__' }, () => {
|
||||
expect(resolveSourceIdEngineFree(null, '/nonexistent')).toBe('__all__');
|
||||
});
|
||||
});
|
||||
|
||||
test('resolveSourceWithTier: flag and env tiers carry the sentinel', async () => {
|
||||
const flag = await resolveSourceWithTier(makeStub(['default']), '__all__', '/nonexistent');
|
||||
expect(flag).toMatchObject({ source_id: '__all__', tier: 'flag' });
|
||||
await withEnv({ GBRAIN_SOURCE: '__all__' }, async () => {
|
||||
const env = await resolveSourceWithTier(makeStub(['default']), null, '/nonexistent');
|
||||
expect(env).toMatchObject({ source_id: '__all__', tier: 'env' });
|
||||
});
|
||||
});
|
||||
|
||||
test('a genuinely invalid --source still throws (SOURCE_ID_RE not loosened)', async () => {
|
||||
await expect(resolveSourceId(makeStub(['default']), 'my_source', '/nonexistent'))
|
||||
.rejects.toThrow(/Invalid --source/);
|
||||
expect(() => resolveSourceIdEngineFree('my_source', '/nonexistent'))
|
||||
.toThrow(/Invalid --source/);
|
||||
});
|
||||
});
|
||||
|
||||
// ── sourceScopeOpts — the single read-scope choke point ─────────────────
|
||||
|
||||
describe('sourceScopeOpts — __all__ sentinel', () => {
|
||||
test('trusted local (remote === false): spans the whole brain (empty scope)', () => {
|
||||
expect(sourceScopeOpts(ctxOf({ remote: false, sourceId: '__all__' }))).toEqual({});
|
||||
});
|
||||
|
||||
test('remote: keeps the unsatisfiable literal — fail-closed, never widens', () => {
|
||||
expect(sourceScopeOpts(ctxOf({ remote: true, sourceId: '__all__' })))
|
||||
.toEqual({ sourceId: '__all__' });
|
||||
});
|
||||
|
||||
test('anything not strictly remote === false is untrusted (fail-closed)', () => {
|
||||
// undefined / missing remote must behave like remote, per the trust rule.
|
||||
const ctx = ctxOf({ sourceId: '__all__' });
|
||||
(ctx as any).remote = undefined;
|
||||
expect(sourceScopeOpts(ctx)).toEqual({ sourceId: '__all__' });
|
||||
});
|
||||
|
||||
test('a federated grant always wins over the sentinel', () => {
|
||||
const ctx = ctxOf({
|
||||
remote: true,
|
||||
sourceId: '__all__',
|
||||
auth: { token: 't', clientId: 'c', scopes: [], allowedSources: ['a', 'b'] } as any,
|
||||
});
|
||||
expect(sourceScopeOpts(ctx)).toEqual({ sourceIds: ['a', 'b'] });
|
||||
});
|
||||
});
|
||||
|
||||
// ── Never narrower than passing no flag (#2561 regression shape) ────────
|
||||
|
||||
describe('__all__ is never narrower than an unqualified read', () => {
|
||||
test('local __all__ spans the brain even when federated widening exists', () => {
|
||||
// Unqualified read on a federated brain widens to the federated array…
|
||||
const unqualified = ctxOf({
|
||||
remote: false,
|
||||
sourceId: 'default',
|
||||
localFederatedSourceIds: ['default', 'src-a', 'src-b'],
|
||||
});
|
||||
expect(federatedSearchScope(unqualified)).toEqual({
|
||||
sourceIds: ['default', 'src-a', 'src-b'],
|
||||
});
|
||||
// …and __all__ must be a superset of that: the whole brain ({}).
|
||||
const all = ctxOf({ remote: false, sourceId: '__all__' });
|
||||
expect(federatedSearchScope(all)).toEqual({});
|
||||
});
|
||||
});
|
||||
|
||||
// ── makeContext — explicit --source failures error loudly ───────────────
|
||||
|
||||
describe('cli makeContext — no silent default fallback for explicit --source', () => {
|
||||
test('--source __all__ produces ctx.sourceId __all__ (was: silent default)', async () => {
|
||||
const { makeContext } = await import('../src/cli.ts');
|
||||
const ctx = await makeContext(makeStub(['default']), { source: '__all__' });
|
||||
expect(ctx.sourceId).toBe('__all__');
|
||||
expect(ctx.remote).toBe(false);
|
||||
});
|
||||
|
||||
test('an explicit --source that fails to resolve throws instead of becoming default', async () => {
|
||||
const { makeContext } = await import('../src/cli.ts');
|
||||
await expect(makeContext(makeStub(['default']), { source: 'ghost' }))
|
||||
.rejects.toThrow(/not found/);
|
||||
await expect(makeContext(makeStub(['default']), { source: 'my_source' }))
|
||||
.rejects.toThrow(/Invalid --source/);
|
||||
});
|
||||
|
||||
test('ambient resolution failure still falls back silently (pre-init brains)', async () => {
|
||||
const { makeContext } = await import('../src/cli.ts');
|
||||
const broken = {
|
||||
kind: 'pglite',
|
||||
executeRaw: async () => { throw new Error('relation "sources" does not exist'); },
|
||||
getConfig: async () => { throw new Error('relation "config" does not exist'); },
|
||||
} as unknown as BrainEngine;
|
||||
const ctx = await makeContext(broken, {});
|
||||
expect(ctx.sourceId).toBe('default');
|
||||
});
|
||||
});
|
||||
Reference in New Issue
Block a user