From 2e5941354def2edb73a8cdf17d587143f5f52137 Mon Sep 17 00:00:00 2001 From: Garry Tan Date: Thu, 11 Jun 2026 20:48:37 -0700 Subject: [PATCH] fix(write-through): mirror to the assigned source's local_path, never the global repo (#2018) put_page write-through resolved the disk target from the global sync.repo_path, so a default-source page (local_path NULL) got written into an unrelated federated source's working tree. Now it uses the assigned source's own local_path; NULL local_path skips (no leak); the global path is used only as a sole-source fallback. Co-Authored-By: Claude Fable 5 --- src/core/write-through.ts | 56 ++++++++++++++++++++++++++++++-------- test/write-through.test.ts | 52 +++++++++++++++++++++++++++++++++++ 2 files changed, 97 insertions(+), 11 deletions(-) diff --git a/src/core/write-through.ts b/src/core/write-through.ts index b21d2cb24..4051839a9 100644 --- a/src/core/write-through.ts +++ b/src/core/write-through.ts @@ -22,7 +22,7 @@ */ import { existsSync, statSync, mkdirSync, writeFileSync, renameSync, unlinkSync } from 'fs'; -import { dirname } from 'path'; +import { dirname, join } from 'path'; import { randomBytes } from 'crypto'; import type { BrainEngine } from './engine.ts'; import { serializePageToMarkdown, resolvePageFilePath } from './markdown.ts'; @@ -37,12 +37,16 @@ export interface WriteThroughResult { path?: string; /** * Non-error reasons the file was not written: - * - no_repo_configured: `sync.repo_path` is unset (DB-only by design). - * - repo_not_found: `sync.repo_path` set but missing / not a directory. + * - no_repo_configured: the resolved target (source `local_path` or, for a + * sole-source brain, `sync.repo_path`) is unset (DB-only by design). + * - repo_not_found: target set but missing / not a directory. + * - source_has_no_local_path: the assigned source has no `local_path` and + * this is NOT a sole-source brain — #2018: we refuse to fall back to the + * global `sync.repo_path`, which may be a sibling source's working tree. * - page_not_found_after_write: the DB row isn't readable back (the caller's * DB write failed or targeted a different source). */ - skipped?: 'no_repo_configured' | 'repo_not_found' | 'page_not_found_after_write'; + skipped?: 'no_repo_configured' | 'repo_not_found' | 'source_has_no_local_path' | 'page_not_found_after_write'; /** Set when the render/write/rename itself threw (EACCES, ENOTDIR, disk full). */ error?: string; } @@ -67,12 +71,43 @@ export async function writePageThrough( ): Promise { const sourceId = opts.sourceId ?? 'default'; try { - const repoPath = await engine.getConfig('sync.repo_path'); - if (!repoPath) { - return { written: false, skipped: 'no_repo_configured' }; - } - if (!existsSync(repoPath) || !statSync(repoPath).isDirectory()) { - return { written: false, skipped: 'repo_not_found' }; + // #2018: resolve the disk target from the ASSIGNED source's own working + // tree, NOT the global `sync.repo_path`. A source with its own `local_path` + // stores files at that tree's root (matching how `scanOneSource` reads them + // back); the global path may belong to an unrelated sibling source, and + // writing a page there silently pollutes that source's git repo. + let filePath: string; + const srcRows = await engine.executeRaw<{ local_path: string | null }>( + `SELECT local_path FROM sources WHERE id = $1`, + [sourceId], + ); + const sourceLocalPath = srcRows[0]?.local_path ?? null; + if (sourceLocalPath) { + if (!existsSync(sourceLocalPath) || !statSync(sourceLocalPath).isDirectory()) { + return { written: false, skipped: 'repo_not_found' }; + } + // Source's own tree → file at the root (never nested under `.sources/`). + filePath = join(sourceLocalPath, `${slug}.md`); + } else { + // No per-source local_path. Falling back to the global `sync.repo_path` + // is ONLY safe when this is the SOLE source — then that path is + // unambiguously this source's tree. With multiple sources it may be a + // sibling's tree, so skip rather than leak into it (#2018). + const cntRows = await engine.executeRaw<{ n: string }>( + `SELECT COUNT(*)::text AS n FROM sources`, + ); + const sourceCount = parseInt(cntRows[0]?.n ?? '1', 10); + if (sourceCount > 1) { + return { written: false, skipped: 'source_has_no_local_path' }; + } + const repoPath = await engine.getConfig('sync.repo_path'); + if (!repoPath) { + return { written: false, skipped: 'no_repo_configured' }; + } + if (!existsSync(repoPath) || !statSync(repoPath).isDirectory()) { + return { written: false, skipped: 'repo_not_found' }; + } + filePath = resolvePageFilePath(repoPath, slug, sourceId); } const writtenPage = await engine.getPage(slug, { sourceId }); @@ -85,7 +120,6 @@ export async function writePageThrough( frontmatterOverrides: opts.frontmatterOverrides, }); - const filePath = resolvePageFilePath(repoPath, slug, sourceId); mkdirSync(dirname(filePath), { recursive: true }); // Atomic write: unique temp sibling + rename. Unique name (pid + random) diff --git a/test/write-through.test.ts b/test/write-through.test.ts index 67ed55117..5ee249d8b 100644 --- a/test/write-through.test.ts +++ b/test/write-through.test.ts @@ -120,6 +120,58 @@ describe('writePageThrough', () => { expect(res).toEqual({ written: false, skipped: 'page_not_found_after_write' }); }); + test('[REGRESSION #2018] default page (null local_path) in a multi-source brain → skipped, no leak into a sibling source repo', async () => { + // A sibling federated source with its OWN working tree. + const siblingDir = path.join(tmpRoot, 'housefax'); + fs.mkdirSync(siblingDir, { recursive: true }); + await engine.executeRaw( + `INSERT INTO sources (id, name, local_path, config) VALUES ('housefax', 'Housefax', $1, '{}'::jsonb)`, + [siblingDir], + ); + // The leak trigger: global sync.repo_path points at the sibling's tree + // while the default source (re-seeded by resetPgliteState) has no + // local_path of its own. + await engine.setConfig('sync.repo_path', siblingDir); + + const slug = 'internal/cross-cutting-note'; + await seedPage(slug); // sourceId 'default' + + const res = await writePageThrough(engine, slug, { sourceId: 'default' }); + + expect(res).toEqual({ written: false, skipped: 'source_has_no_local_path' }); + // The sibling source's repo stays clean — the whole point of #2018. + expect(walkFiles(siblingDir).some((f) => f.endsWith('.md'))).toBe(false); + }); + + test('[#2018] page assigned to a source with its own local_path writes to that tree root, not the global path', async () => { + const alphaDir = path.join(tmpRoot, 'alpha-repo'); + fs.mkdirSync(alphaDir, { recursive: true }); + const globalDir = path.join(tmpRoot, 'global-repo'); + fs.mkdirSync(globalDir, { recursive: true }); + await engine.executeRaw( + `INSERT INTO sources (id, name, local_path, config) VALUES ('alpha', 'Alpha', $1, '{}'::jsonb)`, + [alphaDir], + ); + // Must NOT be used — the assigned source has its own tree. + await engine.setConfig('sync.repo_path', globalDir); + + const slug = 'notes/alpha-thing'; + await importFromContent(engine, slug, `---\ntitle: T\ntype: note\n---\n\n# Body\n`, { + noEmbed: true, + sourceId: 'alpha', + sourcePath: `${slug}.md`, + }); + + const res = await writePageThrough(engine, slug, { sourceId: 'alpha' }); + + expect(res.written).toBe(true); + // File at the source's tree ROOT, never nested under `.sources//`. + expect(res.path).toBe(path.join(alphaDir, `${slug}.md`)); + expect(fs.existsSync(path.join(alphaDir, `${slug}.md`))).toBe(true); + // The global repo path is untouched. + expect(walkFiles(globalDir).some((f) => f.endsWith('.md'))).toBe(false); + }); + test('[REGRESSION] mkdir ENOTDIR (parent is a file) → error, no partial .md, no .tmp', async () => { await engine.setConfig('sync.repo_path', brainDir); // Block the `wiki/` directory by putting a FILE named "wiki" under the repo,