From a12ab5eabc0ca5e8c1dd7e41383f1daf867512e0 Mon Sep 17 00:00:00 2001 From: Sean Gearin Date: Wed, 29 Jul 2026 16:07:42 -0700 Subject: [PATCH] fix(import): quote frontmatter values and omit absent conversation id (#3600) Review follow-up to #3549. An envelope is a third-party file, so interpolating source_provider raw let a provider string carrying a newline close the scalar and inject arbitrary frontmatter keys. title: on the line above was already quoted; source: now matches it. memvelope_conversation_id emitted the literal string undefined when a conversation carried no id, which asserts a value rather than reporting absence: every id-less conversation claims the same id, so anything grouping or deduping on that key merges unrelated pages. The key is now omitted. Filename and frontmatter read one hasId predicate so they cannot disagree about whether an id exists. Tests add both cases; the injection case parses emitted frontmatter with js-yaml rather than substring-matching it. --- scripts/envelope-to-gbrain.mjs | 17 ++++++-- test/envelope-to-gbrain.test.ts | 69 ++++++++++++++++++++++++++++++++- 2 files changed, 82 insertions(+), 4 deletions(-) diff --git a/scripts/envelope-to-gbrain.mjs b/scripts/envelope-to-gbrain.mjs index 84721ef1e..ac83a042c 100644 --- a/scripts/envelope-to-gbrain.mjs +++ b/scripts/envelope-to-gbrain.mjs @@ -64,7 +64,11 @@ for (const [i, c] of conversations.entries()) { // other. The date only leads as a human/chronological sort prefix; the id // carries uniqueness. Positional fallback keeps names unique and deterministic // when an envelope omits an id. - const convId = (typeof c.id === 'string' && c.id.trim()) ? c.id.trim() : `conv-${i + 1}`; + // One predicate for "this conversation carries its own id", shared by the + // filename and the frontmatter below. Keeping it in a single place is what + // stops the two from disagreeing about whether an id exists. + const hasId = typeof c.id === 'string' && c.id.trim() !== ''; + const convId = hasId ? c.id.trim() : `conv-${i + 1}`; const name = `${date || '0000-00-00'}-${slug(convId, `conv-${i + 1}`)}.md`; // gbrain reads YAML frontmatter + markdown body; keep provenance in frontmatter. // Emit `type: conversation` so gbrain stores these as conversation pages rather @@ -77,8 +81,15 @@ for (const [i, c] of conversations.entries()) { 'type: conversation', `title: ${JSON.stringify(c.title || 'Untitled conversation')}`, `date: ${date || 'null'}`, - `source: ${env.meta?.source_provider || 'unknown'}`, - `memvelope_conversation_id: ${JSON.stringify(c.id)}`, + // Every interpolated value is quoted. An envelope is a third-party file, so + // a provider string carrying a newline would otherwise close this scalar and + // inject arbitrary frontmatter keys into the page gbrain ingests. + `source: ${JSON.stringify(env.meta?.source_provider || 'unknown')}`, + // Omit the key entirely when the envelope carries no id, rather than + // emitting the literal `undefined` or a synthesized `conv-N` — the positional + // fallback names the file, but it is not a memvelope conversation id and + // must not be recorded as one. + ...(hasId ? [`memvelope_conversation_id: ${JSON.stringify(convId)}`] : []), 'origin: memvelope/envelope-v0', '---', '', diff --git a/test/envelope-to-gbrain.test.ts b/test/envelope-to-gbrain.test.ts index 733e683e9..29361216c 100644 --- a/test/envelope-to-gbrain.test.ts +++ b/test/envelope-to-gbrain.test.ts @@ -6,6 +6,9 @@ import { afterAll, describe, expect, test } from 'bun:test'; import { mkdtempSync, rmSync, readdirSync, readFileSync, writeFileSync } from 'node:fs'; import { tmpdir } from 'node:os'; import { join } from 'node:path'; +// The same parser gbrain uses to ingest frontmatter (src/core/markdown.ts), so +// the injection test asserts against the real consumer rather than a substring. +import { safeLoad as yamlSafeLoad } from 'js-yaml'; const SCRIPT_PATH = join(import.meta.dir, '..', 'scripts', 'envelope-to-gbrain.mjs'); const FIXTURE_PATH = join(import.meta.dir, 'fixtures', 'memvelope', 'sample.mve.json'); @@ -70,7 +73,7 @@ describe('envelope-to-gbrain importer', () => { expect(page).toContain('type: conversation'); expect(page).toContain('title: "Onboarding Checklist Draft"'); expect(page).toContain('date: 2025-11-02'); - expect(page).toContain('source: chatgpt'); + expect(page).toContain('source: "chatgpt"'); expect(page).toContain('memvelope_conversation_id: "c-3f9a2b"'); expect(page).toContain('origin: memvelope/envelope-v0'); }); @@ -156,4 +159,68 @@ describe('envelope-to-gbrain importer', () => { expect(result.exitCode).toBe(0); expect(markdownFiles(result.outDir)).toEqual(['2025-11-02-conv-1.md']); }); + + test('missing conversation id omits the provenance key rather than emitting a value', async () => { + const inputDir = tempDir(); + const envelopePath = join(inputDir, 'missing-id-frontmatter.mve.json'); + writeFileSync(envelopePath, JSON.stringify({ + memvelope: 'envelope-v0', + meta: { source_provider: 'chatgpt' }, + conversations: [ + { + title: 'Missing id example', + created_at: '2025-11-02T14:22:51.000Z', + messages: [{ id: 'm1', role: 'user', ts: '2025-11-02T14:22:51.000Z', text: 'alice-example asked about frontmatter.' }], + }, + ], + })); + + const result = await runImporter(envelopePath); + const page = readOnlyMarkdown(result.outDir); + + expect(result.exitCode).toBe(0); + // Absent means absent: never the literal string `undefined`, and never the + // positional filename fallback masquerading as a real conversation id. + expect(page).not.toContain('memvelope_conversation_id'); + expect(page).not.toContain('undefined'); + expect(page).toContain('source: "chatgpt"'); + }); + + test('a provider string carrying a newline cannot inject frontmatter keys', async () => { + const inputDir = tempDir(); + const envelopePath = join(inputDir, 'injecting-provider.mve.json'); + writeFileSync(envelopePath, JSON.stringify({ + memvelope: 'envelope-v0', + meta: { source_provider: 'chatgpt\ntype: injected\nowner: attacker' }, + conversations: [ + { + id: 'c-inject', + title: 'Injection attempt', + created_at: '2025-11-02T14:22:51.000Z', + messages: [{ id: 'm1', role: 'user', ts: '2025-11-02T14:22:51.000Z', text: 'alice-example sent a hostile provider string.' }], + }, + ], + })); + + const result = await runImporter(envelopePath); + const page = readOnlyMarkdown(result.outDir); + const frontmatter = page.split('---')[1] ?? ''; + const parsed = yamlSafeLoad(frontmatter) as Record; + + expect(result.exitCode).toBe(0); + // The newline is escaped inside a quoted scalar, so the hostile text stays + // one value instead of becoming keys. Asserted structurally: a substring + // check cannot tell a real key from the same characters inside a quoted + // value, and would pass for the wrong reason. + expect(Object.keys(parsed).sort()).toEqual([ + 'date', + 'memvelope_conversation_id', + 'origin', + 'source', + 'title', + 'type', + ]); + expect(parsed.type).toBe('conversation'); + expect(parsed.source).toBe('chatgpt\ntype: injected\nowner: attacker'); + }); });