From 68fcc5861ecb271b2ff9975f0cefbcc98e29f2d4 Mon Sep 17 00:00:00 2001 From: Garry Tan Date: Fri, 22 May 2026 07:45:42 -0700 Subject: [PATCH] refactor(source-id): migrate three regex sites to canonical source-id.ts Consolidates source_id validation through src/core/source-id.ts: - src/core/utils.ts: validateSourceId is now a back-compat re-export of assertValidSourceId. Regex TIGHTENS from permissive ^[a-z0-9_-]+$ to the strict kebab. The path-safety boundary now matches what sources-ops enforces at source creation time; no production source IDs break because sources-ops always rejected underscored IDs at creation. Picks up the blast-radius callers in cycle/patterns.ts and cycle/synthesize.ts reverse-write paths. - src/core/sources-ops.ts: deletes local SOURCE_ID_RE + validateSourceId; imports isValidSourceId from source-id.ts. Keeps the thin SourceOpError- wrapping validator so `gbrain sources add` keeps its user-facing error envelope. - src/core/source-resolver.ts: imports SOURCE_ID_RE + isValidSourceId from source-id.ts. Per codex outside-voice P1-F, silent-fallback tiers (dotfile read at tier 3, brain_default config at tier 5) use isValidSourceId so an invalid dotfile/config value falls through to the next resolver tier instead of throwing. Explicit + env tiers keep their inline regex-test-and-throw shape because they need tailored error messages. Behaviour: validateSourceId('snake_id') NOW THROWS where pre-PR it accepted. Documented as the intentional tightening; no existing IDs in production contain underscores. Co-Authored-By: Claude Opus 4.7 (1M context) --- src/core/source-resolver.ts | 30 ++++++++++++++++++++++-------- src/core/sources-ops.ts | 11 ++++++++--- src/core/utils.ts | 26 ++++++++++++++------------ 3 files changed, 44 insertions(+), 23 deletions(-) diff --git a/src/core/source-resolver.ts b/src/core/source-resolver.ts index 10ada5a11..092ed4a66 100644 --- a/src/core/source-resolver.ts +++ b/src/core/source-resolver.ts @@ -16,12 +16,18 @@ import { readFileSync, existsSync } from 'fs'; import { join, dirname, resolve } from 'path'; import type { BrainEngine } from './engine.ts'; +import { SOURCE_ID_RE, isValidSourceId } from './source-id.ts'; const DOTFILE = '.gbrain-source'; -// Must start + end with alnum, interior dashes allowed. Max 32 chars. -// Single-char alnum is also valid. Kebab-case enforced so citation keys -// like `[wiki:slug]` can't have ugly edges like `[wiki-:slug]`. -const SOURCE_ID_RE = /^[a-z0-9](?:[a-z0-9-]{0,30}[a-z0-9])?$/; +// 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. +// Two validator shapes per codex r2 P1-F: +// - `isValidSourceId(s)`: boolean — used by tiers that silently fall through +// on invalid input (dotfile tier 3, brain_default tier 5) +// - explicit throw — used by tiers that must reject loudly with a tailored +// message (explicit `--source` flag tier 1, GBRAIN_SOURCE env tier 2). +// Tier-specific messages are clearer than the generic assertValidSourceId +// error, so the throws stay inline. function readDotfileWalk(startDir: string): string | null { let dir = resolve(startDir); @@ -31,7 +37,12 @@ function readDotfileWalk(startDir: string): string | null { if (existsSync(candidate)) { try { const content = readFileSync(candidate, 'utf8').trim().split('\n')[0].trim(); - if (SOURCE_ID_RE.test(content)) return content; + // Silent-fallback tier per codex P1-F: invalid dotfile content + // (legacy ids with underscores, hand-edits with whitespace, etc.) + // falls through to the next tier instead of throwing. The CLI's + // explicit/env tiers throw; dotfiles are operator-edited and the + // forgiving behavior preserves the resolver's existing semantics. + if (isValidSourceId(content)) return content; } catch { // Unreadable dotfile — skip and keep walking. } @@ -107,8 +118,11 @@ export async function resolveSourceId( if (best) return best.id; // 5. Brain-level default. + // Silent-fallback tier per codex P1-F: an invalid `sources.default` config + // value (operator hand-edit gone wrong, legacy underscore id) falls through + // to tier 6 rather than throwing. Resolver stays robust to bad config. const globalDefault = await engine.getConfig('sources.default'); - if (globalDefault && SOURCE_ID_RE.test(globalDefault)) { + if (globalDefault && isValidSourceId(globalDefault)) { await assertSourceExists(engine, globalDefault); return globalDefault; } @@ -243,9 +257,9 @@ export async function resolveSourceWithTier( } if (best) return { source_id: best.id, tier: 'local_path', detail: best.path }; - // 5. Brain-level default. + // 5. Brain-level default. Silent-fallback (P1-F) like tier 5 in resolveSourceId. const globalDefault = await engine.getConfig('sources.default'); - if (globalDefault && SOURCE_ID_RE.test(globalDefault)) { + if (globalDefault && isValidSourceId(globalDefault)) { await assertSourceExists(engine, globalDefault); return { source_id: globalDefault, tier: 'brain_default', detail: 'sources.default config' }; } diff --git a/src/core/sources-ops.ts b/src/core/sources-ops.ts index cbbcb2b30..383405dae 100644 --- a/src/core/sources-ops.ts +++ b/src/core/sources-ops.ts @@ -50,6 +50,7 @@ import { type RepoState, } from './git-remote.ts'; import { gbrainPath } from './config.ts'; +import { isValidSourceId } from './source-id.ts'; // ── Errors ────────────────────────────────────────────────────────────────── @@ -79,8 +80,6 @@ export class SourceOpError extends Error { // ── Types ─────────────────────────────────────────────────────────────────── -const SOURCE_ID_RE = /^[a-z0-9](?:[a-z0-9-]{0,30}[a-z0-9])?$/; - export interface SourceRow { id: string; name: string; @@ -142,8 +141,14 @@ export interface RemoveSourceOpts { // ── Helpers ───────────────────────────────────────────────────────────────── +/** + * Validate via the canonical regex from `source-id.ts` but rethrow as the + * sources-ops-tagged error so `gbrain sources add` keeps its user-facing + * SourceOpError shape. The regex itself is in one place; only the error + * envelope differs per caller. + */ function validateSourceId(id: string): void { - if (!SOURCE_ID_RE.test(id)) { + if (!isValidSourceId(id)) { throw new SourceOpError( 'invalid_id', `Invalid source id "${id}". Must be 1-32 lowercase alnum chars with optional interior hyphens.`, diff --git a/src/core/utils.ts b/src/core/utils.ts index 74c5ee384..deb6c03ca 100644 --- a/src/core/utils.ts +++ b/src/core/utils.ts @@ -44,21 +44,23 @@ export function contentHash(page: PageInput): string { } /** - * v0.32.8: validate a `source_id` is safe for use as a filesystem path - * segment AND as a SQL identifier value. Used by the per-source disk-layout - * fix in patterns.ts/synthesize.ts before any `join(brainDir, source_id, ...)` + * Validate a `source_id` is safe for use as a filesystem path segment AND + * as a SQL identifier value. Used by the per-source disk-layout code in + * patterns.ts/synthesize.ts before any `join(brainDir, source_id, ...)` * call, and at `putSource()` time so invalid ids never make it into the DB. * - * Allows lowercase ASCII letters, digits, underscore, and hyphen. Rejects - * `..`, `/`, spaces, dots, and any non-ASCII character. Path-traversal and - * SQL-injection safe by construction. + * **v0.38 (codex r2 P1-C, P1-D):** consolidated to import from + * `src/core/source-id.ts` (dependency-free canonical module). The regex + * TIGHTENED from the permissive `^[a-z0-9_-]+$` to the strict kebab-case + * `^[a-z0-9](?:[a-z0-9-]{0,30}[a-z0-9])?$` — same regex `sources-ops` has + * always enforced at creation time. Closes the drift between path-safety + * and creation-time validation; no production source IDs break (none had + * underscores, since `sources-ops` always rejected them). + * + * Re-exported here for back-compat with the pre-v0.38 `validateSourceId` + * import. New code should import directly from `source-id.ts`. */ -const SOURCE_ID_RE = /^[a-z0-9_-]+$/; -export function validateSourceId(id: string): void { - if (!SOURCE_ID_RE.test(id)) { - throw new Error(`Invalid source_id "${id}" — must match ${SOURCE_ID_RE}`); - } -} +export { assertValidSourceId as validateSourceId } from './source-id.ts'; function readOptionalDate(raw: unknown): Date | null | undefined { // Three-state read for columns that may or may not be in the SELECT