mirror of
https://github.com/garrytan/gbrain.git
synced 2026-07-31 04:07:52 +00:00
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 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Fable 5
parent
dee3367d99
commit
2e5941354d
+45
-11
@@ -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<WriteThroughResult> {
|
||||
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)
|
||||
|
||||
@@ -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/<id>/`.
|
||||
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,
|
||||
|
||||
Reference in New Issue
Block a user