From d6fc78e5b64a70ffa87bede97c7dedcc0964e327 Mon Sep 17 00:00:00 2001 From: Garry Tan Date: Tue, 21 Jul 2026 14:35:55 -0700 Subject: [PATCH] fix(write-through): guard case-insensitive filesystem collisions before atomic write (#2831) On macOS/Windows (case-folding filesystems), the write-through rename silently clobbered a differently-cased file already occupying the target path (uncontrolled repo files like README.md vs slug readme, or unicode normalization variants between slugs). Refuse with skipped: 'case_insensitive_collision' when the path exists on disk but no exactly-named directory entry does; exact-case updates fall through and case-sensitive filesystems are unaffected. Fixes #2831 Co-Authored-By: Claude Fable 5 --- src/core/write-through.ts | 34 ++++++++++++++++++++++++++++--- test/write-through.test.ts | 41 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 72 insertions(+), 3 deletions(-) diff --git a/src/core/write-through.ts b/src/core/write-through.ts index 02f792a3f..e8ff4e71e 100644 --- a/src/core/write-through.ts +++ b/src/core/write-through.ts @@ -21,8 +21,8 @@ * only does "row exists + repo is a real dir → render + atomic write". */ -import { existsSync, statSync, mkdirSync, writeFileSync, renameSync, unlinkSync } from 'fs'; -import { dirname, join } from 'path'; +import { existsSync, statSync, mkdirSync, writeFileSync, renameSync, unlinkSync, readdirSync } from 'fs'; +import { basename, dirname, join } from 'path'; import { randomBytes } from 'crypto'; import type { BrainEngine } from './engine.ts'; import { serializePageToMarkdown, resolvePageFilePath } from './markdown.ts'; @@ -56,8 +56,12 @@ export interface WriteThroughResult { * DB write failed or targeted a different source). * - path_escapes_source_root: the computed file path resolves outside the * source's working tree (hostile slug row / symlinked subtree) — refused. + * - case_insensitive_collision: on a case-insensitive filesystem + * (macOS/Windows default), the target directory already holds a + * differently-cased entry that the FS folds onto this page's file, so + * writing would silently clobber the OTHER slug's file (#2831) — refused. */ - skipped?: 'no_repo_configured' | 'repo_not_found' | 'source_repo_belongs_to_other_source' | 'page_not_found_after_write' | 'path_escapes_source_root'; + skipped?: 'no_repo_configured' | 'repo_not_found' | 'source_repo_belongs_to_other_source' | 'page_not_found_after_write' | 'path_escapes_source_root' | 'case_insensitive_collision'; /** Set when the render/write/rename itself threw (EACCES, ENOTDIR, disk full). */ error?: string; } @@ -146,6 +150,30 @@ export async function writePageThrough( frontmatterOverrides: opts.frontmatterOverrides, }); + // #2831: two distinct DB slugs differing only by case (FOO vs foo) resolve + // to the SAME file on a case-insensitive filesystem — the second write + // would silently clobber the first slug's artifact. Refuse when the target + // dir holds a differently-cased entry that the FS folds onto our path: + // exact-case entry present → normal update, falls through; on a + // case-sensitive FS the variant path doesn't exist, so the guard is a + // no-op there. + const dir = dirname(filePath); + if (existsSync(dir)) { + const base = basename(filePath); + const entries = readdirSync(dir); + if (!entries.includes(base) && existsSync(filePath)) { + // The path exists on disk but no exactly-named entry does → the FS + // folded the name (case, or unicode normalization on APFS) onto a + // different slug's file. + const clash = + entries.find((e) => e.toLowerCase() === base.toLowerCase()) ?? '(normalization variant)'; + opts.logger?.warn( + `[write-through] case-insensitive collision for ${slug}: '${clash}' already occupies ${filePath} — file not written (DB row is intact)`, + ); + return { written: false, skipped: 'case_insensitive_collision' }; + } + } + 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 238082a60..2d4943e02 100644 --- a/test/write-through.test.ts +++ b/test/write-through.test.ts @@ -172,6 +172,47 @@ describe('writePageThrough', () => { expect(walkFiles(globalDir).some((f) => f.endsWith('.md'))).toBe(false); }); + test('[REGRESSION #2831] differently-cased entry occupying the target → skipped case_insensitive_collision, existing file untouched', async () => { + await engine.setConfig('sync.repo_path', brainDir); + const slug = 'wiki/ideas/note'; + await seedPage(slug); + + // A differently-cased file already occupies the target path's fold slot + // (e.g. an uncontrolled repo file, or another slug's normalization + // variant). On macOS/Windows the FS resolves `note.md` to it. + const dir = path.join(brainDir, 'wiki', 'ideas'); + fs.mkdirSync(dir, { recursive: true }); + fs.writeFileSync(path.join(dir, 'NOTE.md'), 'precious existing content'); + + // Detect whether THIS filesystem folds case (macOS/Windows: yes; Linux + // CI: no — there the two names are distinct files and no guard fires). + const caseInsensitiveFs = fs.existsSync(path.join(dir, 'note.md')); + + const res = await writePageThrough(engine, slug, { sourceId: 'default' }); + + if (caseInsensitiveFs) { + expect(res).toEqual({ written: false, skipped: 'case_insensitive_collision' }); + // The pre-existing file was NOT clobbered — the whole point of #2831. + expect(fs.readFileSync(path.join(dir, 'NOTE.md'), 'utf8')).toBe('precious existing content'); + } else { + expect(res.written).toBe(true); + expect(fs.readFileSync(path.join(dir, 'NOTE.md'), 'utf8')).toBe('precious existing content'); + expect(fs.existsSync(path.join(dir, 'note.md'))).toBe(true); + } + }); + + test('[#2831] exact-case rewrite of the same slug still updates (guard falls through)', async () => { + await engine.setConfig('sync.repo_path', brainDir); + const slug = 'wiki/ideas/rewrite-me'; + await seedPage(slug); + + const first = await writePageThrough(engine, slug, { sourceId: 'default' }); + expect(first.written).toBe(true); + const second = await writePageThrough(engine, slug, { sourceId: 'default' }); + expect(second.written).toBe(true); + expect(second.path).toBe(first.path); + }); + 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,