admin/gbrain
1
0
Fork 0
mirror of https://github.com/garrytan/gbrain.git synced 2026-07-28 14:59:47 +00:00
Code Issues Packages Projects Releases Wiki Activity

fix(v0.34 W0a): source-routing leak across query + two-pass

Browse Source 
Codex outside-voice review on the v0.34 plan caught two load-bearing
sites where sourceId was advertised but never applied — multi-source
brains silently cross-contaminated structural retrieval:

* operations.ts ~323 — `query` op handler called hybridSearch without
  threading ctx.sourceId. Multi-source agents querying with a
  --source flag got cross-source results.
* two-pass.ts:81 (nearSymbol lookup) and two-pass.ts:131 (unresolved
  edge resolution) — TwoPassOpts.sourceId was declared and threaded
  through hybridSearch's expandAnchors call, but the actual SQL ignored
  it. The walk window crossed source boundaries every time.

Fix:
* `query` op now reads ctx.sourceId AND accepts a new `source_id`
  param (with '__all__' as the explicit force-cross-source escape
  hatch). Per-call param wins over ctx context.
* two-pass.ts both lookups join through pages.source_id when
  opts.sourceId is set; omitted opts.sourceId preserves the legacy
  cross-source contract for callers who want it.

Regression test: test/e2e/source-routing.test.ts seeds two sources
with the same `parseMarkdown` symbol + a cross-source caller edge.
Pins:
  - nearSymbol + sourceId='source-a' returns ONLY source-a chunks
  - nearSymbol + sourceId='source-b' returns ONLY source-b chunks
  - nearSymbol with no sourceId still crosses sources (contract preserved)
  - walk_depth=1 unresolved-edge resolution stays in source-a

PGLite in-memory, no DATABASE_URL needed. The fix proves out under
realistic structural retrieval not just a contrived unit test.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
Garry Tan
2026-05-11 12:26:19 -07:00
co-authored by Claude Opus 4.7
parent eb75e1b32b
commit d4046871d8
3 changed files with 288 additions and 8 deletions
Download Patch File Download Diff File Expand all files Collapse all files
src/core/operations.ts
+19
View File
@@ -1025,6 +1025,11 @@ const query: Operation = {
description:
"v0.29.1 — filter to effective_date <= this. Same format as `since`. Replaces deprecated `beforeDate`. YYYY-MM-DD lands at end-of-day.",
},
source_id: {
type: 'string',
description:
"v0.34: scope search to a single source. Defaults to OperationContext.sourceId (set from CLI --source / GBRAIN_SOURCE / .gbrain-source dotfile). Pass '__all__' to force cross-source search in multi-source brains.",
},
},
handler: async (ctx, p) => {
const startedAt = Date.now();
@@ -1057,7 +1062,20 @@ const query: Operation = {
// v0.25.0 — capture meta side-channel. hybridSearch's return contract
// stays SearchResult[] (Cathedral II callers depend on that); meta
// arrives via callback so eval capture can record what actually ran.
//
// v0.34 (Codex finding #2): thread ctx.sourceId so multi-source brains
// get source-scoped retrieval. Explicit `source_id` param wins over
// ctx.sourceId for callers that want to override (per-call multi-source
// search). When the param is the literal '__all__', force-allow
// cross-source mode (matches SearchOpts.sourceId contract).
let capturedMeta: HybridSearchMeta | null = null;
const sourceIdParam = typeof p.source_id === 'string' ? p.source_id : undefined;
const resolvedSourceId =
sourceIdParam !== undefined
? sourceIdParam === '__all__'
? undefined
: sourceIdParam
: ctx.sourceId;
const results = await hybridSearch(ctx.engine, queryText, {
limit: (p.limit as number) || 20,
offset: (p.offset as number) || 0,
@@ -1068,6 +1086,7 @@ const query: Operation = {
symbolKind: (p.symbol_kind as string) || undefined,
nearSymbol: (p.near_symbol as string) || undefined,
walkDepth: typeof p.walk_depth === 'number' ? (p.walk_depth as number) : undefined,
sourceId: resolvedSourceId,
// v0.29.1 — agent-explicit recency + salience. Omitted = heuristic defaults.
salience: p.salience as 'off' | 'on' | 'strong' | undefined,
recency: p.recency as 'off' | 'on' | 'strong' | undefined,
src/core/search/two-pass.ts
+35 -8
View File
@@ -75,12 +75,26 @@ export async function expandAnchors(
// --near-symbol: add chunks whose symbol_name_qualified matches as
// additional anchors. Best-effort — if none found, fall through.
//
// v0.34 (Codex finding #2): when opts.sourceId is set, scope the
// symbol lookup to that source. Pre-v0.34 this WAS unscoped despite the
// TwoPassOpts.sourceId field being declared — multi-source brains
// cross-contaminated structural retrieval. Now: opts.sourceId set →
// filter; undefined → cross-source (matches the documented contract).
if (opts.nearSymbol) {
try {
const rows = await engine.executeRaw<{ id: number }>(
`SELECT id FROM content_chunks WHERE symbol_name_qualified = $1 LIMIT 50`,
[opts.nearSymbol],
);
const rows = opts.sourceId
? await engine.executeRaw<{ id: number }>(
`SELECT cc.id FROM content_chunks cc
JOIN pages p ON p.id = cc.page_id
WHERE cc.symbol_name_qualified = $1 AND p.source_id = $2
LIMIT 50`,
[opts.nearSymbol, opts.sourceId],
)
: await engine.executeRaw<{ id: number }>(
`SELECT id FROM content_chunks WHERE symbol_name_qualified = $1 LIMIT 50`,
[opts.nearSymbol],
);
const baseScore = anchors.length > 0 ? anchors[0]!.score : 1.0;
for (const r of rows) {
if (!seen.has(r.id)) {
@@ -126,12 +140,25 @@ export async function expandAnchors(
}
// Resolve unresolved edges by looking up chunks whose
// symbol_name_qualified matches. One batch query per frontier node.
//
// v0.34 (Codex finding #2): scope by opts.sourceId when set. Pre-v0.34
// this lookup was unscoped, letting structural retrieval cross source
// boundaries silently in multi-source brains.
if (unresolvedTargets.length > 0) {
try {
const resolved = await engine.executeRaw<{ id: number }>(
`SELECT id FROM content_chunks WHERE symbol_name_qualified = ANY($1::text[]) LIMIT ${NEIGHBOR_CAP_PER_HOP}`,
[unresolvedTargets],
);
const resolved = opts.sourceId
? await engine.executeRaw<{ id: number }>(
`SELECT cc.id FROM content_chunks cc
JOIN pages p ON p.id = cc.page_id
WHERE cc.symbol_name_qualified = ANY($1::text[])
AND p.source_id = $2
LIMIT ${NEIGHBOR_CAP_PER_HOP}`,
[unresolvedTargets, opts.sourceId],
)
: await engine.executeRaw<{ id: number }>(
`SELECT id FROM content_chunks WHERE symbol_name_qualified = ANY($1::text[]) LIMIT ${NEIGHBOR_CAP_PER_HOP}`,
[unresolvedTargets],
);
for (const r of resolved) directChunkIds.push(r.id);
} catch {
// best-effort
test/e2e/source-routing.test.ts
+234
View File
@@ -0,0 +1,234 @@
/**
* v0.34 W0a — multi-source isolation E2E.
*
* Pre-v0.34 (Codex finding #2): `query` op didn't pass ctx.sourceId to
* hybridSearch; two-pass.ts:81 + :131 advertised TwoPassOpts.sourceId but
* never applied it to the nearSymbol lookup or unresolved-edge resolution.
* Multi-source brains silently cross-contaminated structural retrieval.
*
* This E2E pins the fix: seed two sources with the same symbol name in
* different files; assert near_symbol + walk_depth retrieval with
* sourceId='source-a' only returns chunks from source-a.
*
* PGLite in-memory — no DATABASE_URL needed, hermetic.
*/
import { describe, test, expect, beforeAll, afterAll } from 'bun:test';
import { PGLiteEngine } from '../../src/core/pglite-engine.ts';
import { expandAnchors } from '../../src/core/search/two-pass.ts';
import { resetPgliteState } from '../helpers/reset-pglite.ts';
let engine: PGLiteEngine;
beforeAll(async () => {
engine = new PGLiteEngine();
await engine.connect({});
await engine.initSchema();
});
afterAll(async () => {
await engine.disconnect();
});
describe('v0.34 W0a — multi-source isolation in two-pass retrieval', () => {
beforeAll(async () => {
await resetPgliteState(engine);
await seedTwoSourcesWithSharedSymbol(engine);
});
test('expandAnchors with nearSymbol + sourceId returns ONLY source-a chunks', async () => {
const result = await expandAnchors(engine, [], {
walkDepth: 0,
nearSymbol: 'parseMarkdown',
sourceId: 'source-a',
});
expect(result.length).toBeGreaterThan(0);
// Hydrate chunk_ids to verify they all belong to source-a
const chunkIds = result.map((r) => r.chunk_id);
const rows = await engine.executeRaw<{ chunk_id: number; source_id: string }>(
`SELECT cc.id AS chunk_id, p.source_id
FROM content_chunks cc
JOIN pages p ON p.id = cc.page_id
WHERE cc.id = ANY($1::int[])`,
[chunkIds],
);
expect(rows.length).toBe(chunkIds.length);
for (const r of rows) {
expect(r.source_id).toBe('source-a');
}
});
test('expandAnchors with nearSymbol + sourceId="source-b" returns ONLY source-b chunks', async () => {
const result = await expandAnchors(engine, [], {
walkDepth: 0,
nearSymbol: 'parseMarkdown',
sourceId: 'source-b',
});
expect(result.length).toBeGreaterThan(0);
const chunkIds = result.map((r) => r.chunk_id);
const rows = await engine.executeRaw<{ chunk_id: number; source_id: string }>(
`SELECT cc.id AS chunk_id, p.source_id
FROM content_chunks cc
JOIN pages p ON p.id = cc.page_id
WHERE cc.id = ANY($1::int[])`,
[chunkIds],
);
for (const r of rows) {
expect(r.source_id).toBe('source-b');
}
});
test('expandAnchors with nearSymbol and NO sourceId returns chunks from both sources (legacy cross-source mode preserved)', async () => {
const result = await expandAnchors(engine, [], {
walkDepth: 0,
nearSymbol: 'parseMarkdown',
// sourceId omitted — should cross sources (matches the documented contract)
});
expect(result.length).toBeGreaterThan(0);
const chunkIds = result.map((r) => r.chunk_id);
const rows = await engine.executeRaw<{ chunk_id: number; source_id: string }>(
`SELECT cc.id AS chunk_id, p.source_id
FROM content_chunks cc
JOIN pages p ON p.id = cc.page_id
WHERE cc.id = ANY($1::int[])`,
[chunkIds],
);
const sources = new Set(rows.map((r) => r.source_id));
expect(sources.has('source-a')).toBe(true);
expect(sources.has('source-b')).toBe(true);
});
test('unresolved-edge resolution within walkDepth respects sourceId', async () => {
// Seed a caller → callee edge so walk_depth=1 must resolve via
// symbol_name_qualified. We added one such edge in seedTwoSourcesWithSharedSymbol
// pointing at parseMarkdown in source-a only.
//
// Start anchor = the caller chunk in source-a; expansion of depth 1 should
// land on the source-a parseMarkdown definition, NOT the source-b one.
const callerChunk = await engine.executeRaw<{ id: number; score: number }>(
`SELECT cc.id, 1.0 AS score
FROM content_chunks cc
JOIN pages p ON p.id = cc.page_id
WHERE p.source_id = 'source-a' AND cc.symbol_name_qualified = 'callerInA'
LIMIT 1`,
[],
);
expect(callerChunk.length).toBe(1);
const anchors = [{
slug: 'src/foo.ts',
page_id: 0,
title: '',
type: 'code' as const,
chunk_text: '',
chunk_source: 'compiled_truth' as const,
chunk_id: callerChunk[0]!.id,
chunk_index: 0,
score: 1.0,
source_id: 'source-a',
stale: false,
}];
const result = await expandAnchors(engine, anchors, {
walkDepth: 1,
sourceId: 'source-a',
});
expect(result.length).toBeGreaterThan(1); // anchor + at least one neighbor
const chunkIds = result.map((r) => r.chunk_id);
const rows = await engine.executeRaw<{ chunk_id: number; source_id: string }>(
`SELECT cc.id AS chunk_id, p.source_id
FROM content_chunks cc
JOIN pages p ON p.id = cc.page_id
WHERE cc.id = ANY($1::int[])`,
[chunkIds],
);
for (const r of rows) {
expect(r.source_id).toBe('source-a');
}
});
});
// ─────────────────────────────────────────────────────────────────
// Fixture: two sources, each with a `parseMarkdown` symbol.
// Source A also has a `callerInA` function whose unresolved call edge
// points at "parseMarkdown" (testing the walk_depth resolution path).
// ─────────────────────────────────────────────────────────────────
async function seedTwoSourcesWithSharedSymbol(engine: PGLiteEngine): Promise<void> {
// Register two sources (schema: id PK, name UNIQUE NOT NULL, plus optional fields)
await engine.executeRaw(
`INSERT INTO sources (id, name, local_path, config, created_at)
VALUES ('source-a', 'source-a', '/fake/a', '{}'::jsonb, NOW())
ON CONFLICT (id) DO NOTHING`,
[],
);
await engine.executeRaw(
`INSERT INTO sources (id, name, local_path, config, created_at)
VALUES ('source-b', 'source-b', '/fake/b', '{}'::jsonb, NOW())
ON CONFLICT (id) DO NOTHING`,
[],
);
// Page A1: contains parseMarkdown in source-a
const pageA = await engine.executeRaw<{ id: number }>(
`INSERT INTO pages (slug, source_id, title, type, compiled_truth, frontmatter, updated_at, created_at)
VALUES ('code/src/markdown-a.ts', 'source-a', 'markdown-a.ts', 'code', 'export function parseMarkdown(s: string) { return s; }', '{}'::jsonb, NOW(), NOW())
RETURNING id`,
[],
);
// Page A2: contains callerInA, which references parseMarkdown
const pageA2 = await engine.executeRaw<{ id: number }>(
`INSERT INTO pages (slug, source_id, title, type, compiled_truth, frontmatter, updated_at, created_at)
VALUES ('code/src/caller-a.ts', 'source-a', 'caller-a.ts', 'code', 'export function callerInA() { return parseMarkdown(""); }', '{}'::jsonb, NOW(), NOW())
RETURNING id`,
[],
);
// Page B: contains parseMarkdown in source-b
const pageB = await engine.executeRaw<{ id: number }>(
`INSERT INTO pages (slug, source_id, title, type, compiled_truth, frontmatter, updated_at, created_at)
VALUES ('code/src/markdown-b.ts', 'source-b', 'markdown-b.ts', 'code', 'export function parseMarkdown(s: string) { return s; }', '{}'::jsonb, NOW(), NOW())
RETURNING id`,
[],
);
// Chunks
await engine.executeRaw(
`INSERT INTO content_chunks (page_id, chunk_index, chunk_text, chunk_source, language, symbol_name_qualified, symbol_type)
VALUES ($1, 0, 'export function parseMarkdown(s: string) { return s; }', 'compiled_truth', 'typescript', 'parseMarkdown', 'function')`,
[pageA[0]!.id],
);
await engine.executeRaw(
`INSERT INTO content_chunks (page_id, chunk_index, chunk_text, chunk_source, language, symbol_name_qualified, symbol_type)
VALUES ($1, 0, 'export function callerInA() { return parseMarkdown(""); }', 'compiled_truth', 'typescript', 'callerInA', 'function')`,
[pageA2[0]!.id],
);
await engine.executeRaw(
`INSERT INTO content_chunks (page_id, chunk_index, chunk_text, chunk_source, language, symbol_name_qualified, symbol_type)
VALUES ($1, 0, 'export function parseMarkdown(s: string) { return s; }', 'compiled_truth', 'typescript', 'parseMarkdown', 'function')`,
[pageB[0]!.id],
);
// Unresolved edge: callerInA → parseMarkdown (no to_chunk_id, must resolve via symbol_name_qualified)
const callerChunk = await engine.executeRaw<{ id: number }>(
`SELECT cc.id FROM content_chunks cc
JOIN pages p ON p.id = cc.page_id
WHERE p.source_id = 'source-a' AND cc.symbol_name_qualified = 'callerInA' LIMIT 1`,
[],
);
await engine.executeRaw(
`INSERT INTO code_edges_symbol (from_chunk_id, from_symbol_qualified, to_symbol_qualified, edge_type, source_id, edge_metadata)
VALUES ($1, 'callerInA', 'parseMarkdown', 'calls', 'source-a', '{}'::jsonb)`,
[callerChunk[0]!.id],
);
}