From 23c7b0eb1683da2c2dceae96634db42adea53b02 Mon Sep 17 00:00:00 2001 From: Garry Tan Date: Wed, 12 Aug 2026 11:18:32 -0700 Subject: [PATCH] fix(import): reject malformed YAML frontmatter (#3708) (#3923) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Wave-assembled from PR #3923 by @javieraldape. Co-Authored-By: Sofía González --- src/core/import-file.ts | 44 ++++++++- src/core/markdown.ts | 31 ++++++- test/commands/capture.test.ts | 8 ++ test/import-yaml-frontmatter.test.ts | 129 +++++++++++++++++++++++++++ 4 files changed, 206 insertions(+), 6 deletions(-) create mode 100644 test/import-yaml-frontmatter.test.ts diff --git a/src/core/import-file.ts b/src/core/import-file.ts index 010bd8a74..8c1ac1b3f 100644 --- a/src/core/import-file.ts +++ b/src/core/import-file.ts @@ -224,7 +224,7 @@ export interface ImportResult { * Parsed page content. Present for status='imported' AND status='skipped' * (skip happens when content is identical to existing page; auto-link still * needs to run for reconciliation in case links table drifted from page text). - * Absent only on status='error' (early payload-size rejection). + * Absent on early rejection before a page can be parsed. */ parsedPage?: ParsedPage; /** Content-quality gate (issue #1699): true when the page landed with a @@ -239,6 +239,13 @@ export interface ImportResult { const MAX_FILE_SIZE = 5_000_000; // 5MB +function invalidYamlFrontmatterError(parsed: ReturnType): string | null { + const yamlError = parsed.errors?.find((error) => error.code === 'YAML_PARSE'); + if (!yamlError) return null; + const detail = yamlError.message.replace(/^YAML parse failed:\s*/, '').trim(); + return `Invalid YAML frontmatter: ${detail}. Quote scalar values that contain ": " or fix the frontmatter block.`; +} + /** * Import content from a string. Core pipeline: * parse -> hash -> embed (external) -> transaction(version + putPage + tags + chunks) @@ -343,7 +350,14 @@ export async function importFromContent( }; } - const parsed = parseMarkdown(content, slug + '.md', { activePack: opts.activePack }); + const parsed = parseMarkdown(content, slug + '.md', { + validate: true, + ...(opts.activePack ? { activePack: opts.activePack } : {}), + }); + const frontmatterError = invalidYamlFrontmatterError(parsed); + if (frontmatterError) { + return { slug, status: 'error', chunks: 0, error: frontmatterError }; + } // v0.42 (#1699 trust boundary): strip gate-owned markers from UNTRUSTED // input. parseMarkdown preserves every frontmatter key except type/title/ @@ -1092,6 +1106,17 @@ export async function importFromFile( }); } + const preInferenceParsed = parseMarkdown(content, relativePath, { validate: true }); + const preInferenceFrontmatterError = invalidYamlFrontmatterError(preInferenceParsed); + if (preInferenceFrontmatterError) { + return { + slug: slugifyPath(relativePath), + status: 'skipped', + chunks: 0, + error: preInferenceFrontmatterError, + }; + } + // v0.22.8 — Frontmatter inference: if the file has no frontmatter and // inference is enabled, synthesize it from the filesystem path + content. // This turns bare markdown files into fully-typed, dated, tagged pages @@ -1106,7 +1131,11 @@ export async function importFromFile( } } - const parsed = parseMarkdown(content, relativePath, { activePack: opts.activePack }); + const parsed = parseMarkdown(content, relativePath, { + validate: true, + ...(opts.activePack ? { activePack: opts.activePack } : {}), + }); + const frontmatterError = invalidYamlFrontmatterError(parsed); // Enforce path-authoritative slug. parseMarkdown prefers frontmatter.slug over // the path-derived slug, so a mismatch here means the frontmatter is trying @@ -1119,6 +1148,15 @@ export async function importFromFile( let resolvedSlug = expectedSlug; let usedFrontmatterFallback = false; + if (frontmatterError) { + return { + slug: expectedSlug, + status: 'skipped', + chunks: 0, + error: frontmatterError, + }; + } + if (expectedSlug === '') { if (parsed.slug && parsed.slug.length > 0) { // v0.32.7 CJK wave (PR #598 + codex C1/C6): path-derived slug is empty diff --git a/src/core/markdown.ts b/src/core/markdown.ts index d2ccb68d0..6325c9829 100644 --- a/src/core/markdown.ts +++ b/src/core/markdown.ts @@ -315,11 +315,25 @@ function collectValidationErrors( } } - // 6. YAML_PARSE — gray-matter threw. - if (ctx.yamlParseError) { + const looksLikeFrontmatter = hasFrontmatterFieldSyntax(fmBody); + + // 6. YAML_PARSE — validate the fenced YAML directly. gray-matter normally + // throws for malformed frontmatter, but it can also return the whole file as + // body with empty data, so the validation surface must not depend only on + // gray-matter's parse path. Gate this on frontmatter-shaped fields so a + // leading Markdown thematic break / epigraph is preserved as body content. + let detectedYamlParseError = looksLikeFrontmatter ? ctx.yamlParseError : null; + if (!detectedYamlParseError && looksLikeFrontmatter) { + try { + yamlSafeLoad(fmBody); + } catch (e) { + detectedYamlParseError = e as Error; + } + } + if (detectedYamlParseError) { errors.push({ code: 'YAML_PARSE', - message: `YAML parse failed: ${ctx.yamlParseError.message}`, + message: `YAML parse failed: ${detectedYamlParseError.message}`, line: firstNonEmpty + 1, }); } @@ -351,6 +365,17 @@ function collectValidationErrors( } } +function hasFrontmatterFieldSyntax(fmBody: string): boolean { + for (const line of fmBody.split('\n')) { + const trimmed = line.trim(); + if (trimmed.length === 0 || trimmed.startsWith('#')) continue; + if (/^(?:['"][^'"]+['"]|[A-Za-z_][\w.-]*)\s*:/.test(trimmed)) { + return true; + } + } + return false; +} + /** * Split body content at the first recognized timeline sentinel. * Returns compiled_truth (before) and timeline (after). diff --git a/test/commands/capture.test.ts b/test/commands/capture.test.ts index d7bcbf5bb..cfda467eb 100644 --- a/test/commands/capture.test.ts +++ b/test/commands/capture.test.ts @@ -16,12 +16,19 @@ import { PGLiteEngine } from '../../src/core/pglite-engine.ts'; import { resetPgliteState } from '../helpers/reset-pglite.ts'; import matter from 'gray-matter'; import { runCapture, __testing } from '../../src/commands/capture.ts'; +import { configureGateway, resetGateway } from '../../src/core/ai/gateway.ts'; let engine: PGLiteEngine; let tmpRoot: string; let brainDir: string; beforeAll(async () => { + // Keep capture's put_page integration hermetic under CI's fake provider keys. + configureGateway({ + embedding_model: 'openai:text-embedding-3-large', + embedding_dimensions: 1536, + env: {}, + }); engine = new PGLiteEngine(); await engine.connect({}); await engine.initSchema(); @@ -29,6 +36,7 @@ beforeAll(async () => { afterAll(async () => { await engine.disconnect(); + resetGateway(); }); beforeEach(async () => { diff --git a/test/import-yaml-frontmatter.test.ts b/test/import-yaml-frontmatter.test.ts new file mode 100644 index 000000000..891e25057 --- /dev/null +++ b/test/import-yaml-frontmatter.test.ts @@ -0,0 +1,129 @@ +import { describe, expect, test } from 'bun:test'; +import { mkdtempSync, rmSync, writeFileSync } from 'fs'; +import { tmpdir } from 'os'; +import { join } from 'path'; +import type { BrainEngine } from '../src/core/engine.ts'; +import { importFile, importFromContent } from '../src/core/import-file.ts'; + +function mockEngine(): BrainEngine { + const calls: { method: string; args: any[] }[] = []; + const pages = new Map }>(); + + const engine = new Proxy({} as any, { + get(_, prop: string) { + if (prop === '_calls') return calls; + if (prop === 'getTags') return () => Promise.resolve([]); + if (prop === 'getPage') { + return (slug: string) => Promise.resolve(pages.get(slug) ?? null); + } + if (prop === 'putPage') { + return async (slug: string, page: { content_hash?: string; title?: string; type?: string; frontmatter?: Record }) => { + calls.push({ method: 'putPage', args: [slug, page] }); + pages.set(slug, { + slug, + content_hash: page.content_hash ?? '', + title: page.title ?? '', + type: page.type ?? '', + frontmatter: page.frontmatter ?? {}, + }); + }; + } + if (prop === 'transaction') return async (fn: (tx: BrainEngine) => Promise) => fn(engine); + return (...args: any[]) => { + calls.push({ method: String(prop), args }); + return Promise.resolve(null); + }; + }, + }); + + return engine as BrainEngine; +} + +describe('import YAML frontmatter validation', () => { + test('importFromContent rejects invalid YAML frontmatter instead of importing it as body', async () => { + const content = `--- +type: note +title: Re: October booking +--- + +Body text. +`; + + const engine = mockEngine(); + const result = await importFromContent(engine, 'emails/reply-october-booking', content, { noEmbed: true }); + + expect(result.status).toBe('error'); + expect(result.error).toContain('Invalid YAML frontmatter'); + expect(result.error).toContain('title: Re: October booking'); + expect((engine as any)._calls).toEqual([]); + }); + + test('importFile rejects invalid YAML before frontmatter inference can wrap it', async () => { + const dir = mkdtempSync(join(tmpdir(), 'gbrain-invalid-yaml-')); + try { + const filePath = join(dir, 'reply.md'); + writeFileSync(filePath, `--- +type: note +title: Re: October booking +--- + +Body text. +`); + + const engine = mockEngine(); + const result = await importFile(engine, filePath, 'emails/reply-october-booking.md', { noEmbed: true }); + + expect(result.status).toBe('skipped'); + expect(result.error).toContain('Invalid YAML frontmatter'); + expect(result.error).toContain('title: Re: October booking'); + expect((engine as any)._calls).toEqual([]); + } finally { + rmSync(dir, { recursive: true, force: true }); + } + }); + + test('quoted frontmatter values with colon-space still import', async () => { + const content = `--- +type: note +title: "Re: October booking" +--- + +Body text. +`; + + const engine = mockEngine(); + const result = await importFromContent(engine, 'emails/reply-october-booking', content, { noEmbed: true }); + + expect(result.status).toBe('imported'); + const putCall = (engine as any)._calls.find((call: any) => call.method === 'putPage'); + expect(putCall.args[1].title).toBe('Re: October booking'); + expect(putCall.args[1].compiled_truth).toBe('Body text.'); + }); + + test('importFile preserves leading thematic-break epigraphs as body content', async () => { + const dir = mkdtempSync(join(tmpdir(), 'gbrain-thematic-epigraph-')); + try { + const filePath = join(dir, 'epigraph.md'); + writeFileSync(filePath, `--- +> Re: quoted epigraph, not YAML frontmatter +--- + +# Epigraph Note + +Body text. +`); + + const engine = mockEngine(); + const result = await importFile(engine, filePath, 'notes/epigraph.md', { noEmbed: true }); + + expect(result.status).toBe('imported'); + expect(result.error).toBeUndefined(); + const putCall = (engine as any)._calls.find((call: any) => call.method === 'putPage'); + expect(putCall.args[1].title).toBe('Epigraph Note'); + expect(putCall.args[1].compiled_truth).toContain('> Re: quoted epigraph, not YAML frontmatter'); + expect(putCall.args[1].compiled_truth).toContain('# Epigraph Note'); + } finally { + rmSync(dir, { recursive: true, force: true }); + } + }); +});