mirror of
https://github.com/garrytan/gbrain.git
synced 2026-07-27 22:15:33 +00:00
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 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Fable 5
parent
0612b0daa8
commit
d6fc78e5b6
@@ -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)
|
||||
|
||||
@@ -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,
|
||||
|
||||
Reference in New Issue
Block a user