diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index e1494f2..c2fbaea 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -42,6 +42,14 @@ jobs: - name: Unit tests (series detection — keeps timelines out of the supersede band) run: node series-detection.test.mjs + - name: Unit tests (tag-leak repair — sibling fields leaked into content) + run: node tag-leak-repair.test.mjs + + - name: Integration test (tag-leak repair through store_memory) + env: + TOKENMEM_DB_PATH: ${{ runner.temp }}/mneme-ci-tag-leak.db + run: node tag-leak.integration.test.mjs + - name: Integration test (meta-gate write-gate) env: TOKENMEM_DB_PATH: ${{ runner.temp }}/mneme-ci-meta-gate.db diff --git a/mcp-server.mjs b/mcp-server.mjs index e709035..b24b794 100644 --- a/mcp-server.mjs +++ b/mcp-server.mjs @@ -48,6 +48,7 @@ import { } from './index.mjs' import { migrateVectorsToBlob } from './index.mjs' // Migration 013 background runner (below) import { parseHostTokens, resolveAuthMode, resolveHost } from './auth.mjs' +import { repairTagLeak } from './tag-leak-repair.mjs' import { recallClaudeMarkdownMemory } from './lib/claude-markdown-memory.mjs' // ── Load .env.local BEFORE initMemory() ──────────────────────────────── @@ -256,6 +257,19 @@ function createServer(hostId = DEFAULT_HOST) { async ({ content, summary, importance = 6, memory_type = 'long_term', memory_level = 'semi_abstract', category = 'general', tags = [], supersedes, event_time, is_anchor, is_pinned }) => { const out = {} + // Close-tag leak: sibling fields parsed into content/summary as raw XML + // while the real fields fell back to zod defaults. Split them back out + // before anything else sees the args — see tag-leak-repair.mjs. + const leak = repairTagLeak({ content, summary, importance, memory_type, memory_level, category, tags, supersedes, event_time, is_anchor, is_pinned }) + if (leak.repaired) { + ({ content, summary, importance, memory_type, memory_level, category, tags, supersedes, event_time, is_anchor, is_pinned } = leak.args) + } + const leakNote = leak.repaired + ? `\n🩹 close-tag leak repaired: ${leak.moved.join(', ') || '(no valid fields)'} moved back out of ${leak.from.join(' + ')} — a field was closed with the wrong tag; each must end with ` + : leak.suspect + ? `\n⚠️ possible close-tag leak: content/summary contains a closing tag followed by a field tag, but it didn't parse cleanly — recall_by_id this row and check its tail` + : '' + // v2.9: not-yet-trusted hosts write into quarantine — a separate table // the recall pool never reads. Requested supersedes are recorded but // execute only if the reviewer approves. @@ -273,6 +287,7 @@ function createServer(hostId = DEFAULT_HOST) { if (!qid) return { content: [{ type: 'text', text: 'Quarantine storage failed' }] } let qtext = `🔒 Quarantined (qid: ${qid}, host: ${hostId}) — pending review by '${PRIMARY_HOST}'. Not recallable until approved.` if (is_anchor || is_pinned) qtext += `\n(anchor/pinned flags are dropped for quarantined writes — the reviewer can re-add them after merge)` + qtext += leakNote if (out.encodingWarning) { const e = out.encodingWarning qtext += `\n⚠️ ENCODING DAMAGE: ${e.qmarkCount} '?' chars (longest run ${e.maxRun}). CJK was likely lost to a non-UTF-8 code page (cp936) — this is IRREVERSIBLE, not a display glitch. If you just wrote Chinese, it did NOT save; re-store via a UTF-8-safe path (codex exec / CC-side), not Codex Desktop.` @@ -309,7 +324,7 @@ function createServer(hostId = DEFAULT_HOST) { if (is_anchor && !out.quotaRejected?.find(q => q.flag === 'is_anchor')) flags.push('anchor') if (is_pinned && !out.quotaRejected?.find(q => q.flag === 'is_pinned')) flags.push('pinned') const flagStr = flags.length ? `, flags: [${flags.join(', ')}]` : '' - let text = `Stored memory (id: ${id}, importance: ${importance}, type: ${memory_type}, level: ${finalLevel}${flagStr})` + let text = `Stored memory (id: ${id}, importance: ${importance}, type: ${memory_type}, level: ${finalLevel}${flagStr})` + leakNote // First among the warnings on purpose: the others say a policy adjusted the // write, this one says the content that landed is already damaged. if (out.encodingWarning) { diff --git a/tag-leak-repair.mjs b/tag-leak-repair.mjs new file mode 100644 index 0000000..58dfaea --- /dev/null +++ b/tag-leak-repair.mjs @@ -0,0 +1,175 @@ +// tag-leak-repair.mjs — tool-call close-tag leak detection + repair +// +// When a caller closes a long `content` argument with the wrong tag +// (`` instead of ``, or opens a bare ``), +// the tool-call parser keeps reading and the sibling fields — summary, +// importance, category, tags — land inside `content` as raw XML. The +// fields themselves arrive empty, so the zod defaults (importance 6, +// category general) silently replace what the caller wrote. +// +// An instruction-level rule against this was in place for months and the +// leak kept happening, so the fix lives at the write path: +// detect the leak, split the tail back into its fields, report the repair. +// +// Detection is structural, not substring. The closing tag must be followed +// by a field tag AND the whole tail must parse as field tags to the end of +// the string. A memory that *describes* this bug quotes the same strings +// mid-prose (often in backticks) — that one must pass through untouched. + +const FIELDS = [ + 'summary', 'importance', 'category', 'tags', 'memory_type', 'memory_level', + 'supersedes', 'event_time', 'is_anchor', 'is_pinned', +] +const F = FIELDS.join('|') + +const ENUMS = { + category: ['general', 'people', 'project', 'decision', 'feedback', 'bug', 'relationship', 'skill', 'preference'], + memory_type: ['working', 'short_term', 'long_term', 'permanent'], + memory_level: ['concrete_trace', 'semi_abstract', 'meta_knowledge'], +} + +// zod defaults in the store_memory schema. A passed value equal to one of +// these may be the default standing in for a value that leaked. +export const DEFAULTS = { importance: 6, category: 'general', memory_type: 'long_term', memory_level: 'semi_abstract' } + +// Tail tokens, matched in place with sticky regexes — no slicing, tails can +// be tens of KB. Only field / `parameter` closers and the tool-call envelope +// (`invoke` / `function_calls`, optionally namespaced — real leaks usually +// end with it) count. A tail that ends in any other closer (``, +// ``) is markup, not a leak. +const WS = /\s*/y +const CLOSE = new RegExp(``, 'y') +const OPEN = new RegExp(`<(?:parameter name="(${F})"|(${F}))>`, 'y') +const VALUE_END = new RegExp(`||\\s*<(?:parameter name="(?:${F})"|(?:${F}))>`, 'g') + + // A leaked closer has no opener in the text — the caller's own opening tag + // was consumed by the tool-call parser. A closer that closes an element + // opened earlier (`…` in a quoted Atom entry, say) is + // markup. Open/close depth per name, advanced as matches move right. + const tagRe = new RegExp(`<(/?)(${closerNames})(?=[\\s>])`, 'g') + const depth = Object.fromEntries(closerNames.split('|').map(n => [n, 0])) + let scanned = 0 + const advanceTo = (to) => { + tagRe.lastIndex = scanned + let t + while ((t = tagRe.exec(text)) && t.index < to) depth[t[2]] += t[1] ? -1 : 1 + scanned = to + } + + let m + let suspect = false + while ((m = re.exec(text))) { + if (text[m.index - 1] === '`') continue // quoted in inline code + advanceTo(m.index) + if (depth[m[1]] > 0) continue + const r = parseTail(text, m.index + m[1].length + 3) + if (r.fields) return { head: text.slice(0, m.index).trimEnd(), fields: r.fields } + suspect = true + // Any later start before failAt runs into the same non-field token, so + // skip past it — keeps the scan linear on long, repetitive content. + re.lastIndex = Math.max(re.lastIndex, r.failAt) + } + return suspect ? { suspect: true } : null +} + +const isEmpty = v => v === undefined || v === null || v === '' || (Array.isArray(v) && v.length === 0) + +/** + * Repair a store_memory argument set whose fields leaked into content/summary. + * Pure: returns a new args object, never mutates the input. + * + * Merge rule: a leaked value only replaces a value the caller didn't really + * choose — an empty one, or one equal to the zod default (`defaults`). An + * explicitly passed non-default value always wins. When content and summary + * both leak the same field, the content-side value is kept. + * + * The text is only truncated when the split pays for itself: at least one + * field is recovered and something of the original text is left. Otherwise + * the args pass through unchanged and the result is flagged `suspect`. + * + * @returns {{ repaired: boolean, suspect: boolean, args: object, moved: string[], from: string[] }} + */ +export function repairTagLeak(args, defaults = DEFAULTS) { + const next = { ...args } + const moved = new Set() + const from = [] + let suspect = false + + const replaceable = (name) => isEmpty(next[name]) || (name in defaults && next[name] === defaults[name]) + + for (const [field, closerNames] of [['content', 'content|parameter'], ['summary', 'summary|parameter']]) { + const hit = splitLeak(next[field], closerNames) + if (!hit) continue + if (hit.suspect) { suspect = true; continue } + // `accounted`: tail fields that are valid and either applied now or + // already recovered from the other side — proof the tail is a real leak. + const updates = {} + let accounted = 0 + for (const [name, raw] of Object.entries(hit.fields)) { + if (name === field) continue + const v = coerce(name, raw) + if (v === undefined) continue + if (moved.has(name)) { accounted++; continue } + if (replaceable(name)) { updates[name] = v; accounted++ } + } + if (!accounted || !hit.head.trim()) { suspect = true; continue } + next[field] = hit.head + Object.assign(next, updates) + for (const k of Object.keys(updates)) moved.add(k) + from.push(field) + } + + return { repaired: from.length > 0, suspect, args: next, moved: [...moved], from } +} diff --git a/tag-leak-repair.test.mjs b/tag-leak-repair.test.mjs new file mode 100644 index 0000000..b9e47f7 --- /dev/null +++ b/tag-leak-repair.test.mjs @@ -0,0 +1,121 @@ +// Self-check for tag-leak-repair. Run: node tag-leak-repair.test.mjs +// Leak cases are shaped after real corrupted rows found in a production store. +import { repairTagLeak } from './tag-leak-repair.mjs' + +const base = { summary: undefined, importance: 6, category: 'general', tags: [], memory_type: 'long_term', memory_level: 'semi_abstract' } +let fail = 0 +const check = (label, cond, detail) => { + if (!cond) { fail++; console.log(`✗ ${label}`, detail ?? '') } else console.log(`✓ ${label}`) +} + +// 1. then an unclosed summary param (most common Aug shape) +{ + const r = repairTagLeak({ ...base, content: '测量工具介入系统就改变了系统。 性能测量前须采样空载基线' }) + check('content leak: summary moved out', r.repaired && r.args.summary === '性能测量前须采样空载基线', r) + check('content leak: content truncated', r.args.content === '测量工具介入系统就改变了系统。', r.args.content) +} + +// 2. bare / tags (Jun–Aug shape) +{ + const r = repairTagLeak({ ...base, content: '方案是假的。 备份凭据不能只存在被备份的机器上 8' }) + check('bare tags: summary + importance', r.args.summary === '备份凭据不能只存在被备份的机器上' && r.args.importance === 8, r.args) +} + +// 3. full sibling set with category / tags / level +{ + const content = '正文\n\nS\n9\ndecision\n["a","b"]\nmeta_knowledge' + const r = repairTagLeak({ ...base, content }) + check('full set: all fields', r.args.content === '正文' && r.args.summary === 'S' && r.args.importance === 9 + && r.args.category === 'decision' && r.args.tags.join() === 'a,b' && r.args.memory_level === 'meta_knowledge', r.args) +} + +// 4. summary-side leak (9/21 shape): importance stuck inside summary +{ + const r = repairTagLeak({ ...base, content: '内容无损', summary: '摘要文本7' }) + check('summary leak: importance recovered', r.args.summary === '摘要文本' && r.args.importance === 7 && r.args.content === '内容无损', r.args) +} + +// 4b. tail ending in the tool-call envelope closer (most summary-side rows) +{ + const r = repairTagLeak({ ...base, content: '正文', summary: '摘要\n9\nproject\n["a","b"]\n' }) + check('envelope closer at the end', r.repaired && r.args.summary === '摘要' && r.args.importance === 9 && r.args.category === 'project' && r.args.tags.join() === 'a,b', r.args) +} + +// 5. an existing non-empty summary is never overwritten by a leaked one +{ + const r = repairTagLeak({ ...base, summary: '已有摘要', content: 'x泄漏摘要bug' }) + check('keeps passed summary, takes leaked category', r.args.summary === '已有摘要' && r.args.category === 'bug' && r.args.content === 'x', r.args) +} + +// 6. prose that DESCRIBES the bug must pass untouched +for (const [label, content] of [ + ['prose: backticked closer + field', '尾巴上挂着一段 `…` 的裸 XML。\n\n## 分类判据\n关键是分清两种'], + ['prose: closer followed by Chinese', '把结尾 `` 手滑成 , 污染其后 importance'], + ['prose: field tag mid-paragraph then prose', '写 时要记得关 tag，然后继续写正文。这里还有更多说明文字。'], + ['clean content', '普通的一条记忆，没有任何标签。'], +]) { + const r = repairTagLeak({ ...base, content }) + check(label, !r.repaired && r.args.content === content, r) +} + +// 7. nothing valid recovered → no truncation, flagged suspect +{ + const content = 'xnonsense42' + const r = repairTagLeak({ ...base, content }) + check('invalid values only → untouched + suspect', !r.repaired && r.suspect && r.args.content === content && r.args.importance === 6, r) +} + +// 8. leak-shaped closer whose tail is prose → suspect, not repaired +{ + const content = '正文摘要 然后又接了一段正文' + const r = repairTagLeak({ ...base, content }) + check('unparseable tail → suspect only', !r.repaired && r.suspect && r.args.content === content, r) +} + +// 9. markup that happens to use field-named elements is not a leak +for (const [label, content] of [ + ['atom entry with wrapper', 'Atom feed 示例：\n\n 正文内容\n 摘要内容\n'], + ['atom elements at the very end', 'Atom 里正文和摘要这样写：\n正文内容\n摘要内容'], + ['jsx-ish', '组件结构：\nbar\nx\n'], + ['html with invalid enum', '这是页面结构：\n
\n 正文\n news\n
'], +]) { + const r = repairTagLeak({ ...base, content }) + check(`markup: ${label}`, !r.repaired && r.args.content === content && r.args.summary === undefined, r) +} + +// 10. an explicitly passed non-default value is never overridden +{ + const r = repairTagLeak({ ...base, importance: 10, category: 'decision', content: '示例正文3bugS' }) + check('explicit importance/category kept', r.args.importance === 10 && r.args.category === 'decision' && r.args.summary === 'S' && r.args.content === '示例正文', r.args) +} + +// 11. leak consumes the whole field → nothing left, don't store a blank row +{ + const content = 'S' + const r = repairTagLeak({ ...base, content }) + check('empty head → untouched + suspect', !r.repaired && r.suspect && r.args.content === content, r) +} + +// 12. content and summary both leak the same field → content side wins +{ + const r = repairTagLeak({ ...base, content: 'c3', summary: 's8' }) + check('content-side value wins on conflict', r.args.importance === 3 && r.args.summary === 's' && r.args.content === 'c', r.args) +} + +// 13. remaining fields: memory_type / supersedes / event_time / is_anchor +{ + const r = repairTagLeak({ ...base, content: 'xpermanent["12","34"]2026-01-02true' }) + check('other fields recovered', r.args.memory_type === 'permanent' && r.args.supersedes.join() === '12,34' && r.args.event_time === '2026-01-02' && r.args.is_anchor === true, r.args) +} + +// 14. long repetitive near-leak stays linear +{ + const content = 'Z' + 'Y'.repeat(8000) + '\u0000POISON' + const t0 = Date.now() + const r = repairTagLeak({ ...base, content }) + const ms = Date.now() - t0 + check(`${Math.round(content.length / 1000)}KB adversarial input in ${ms}ms (< 500)`, ms < 500 && !r.repaired, { ms, repaired: r.repaired }) +} + +console.log(fail ? `\n${fail} FAILED` : '\nall passed') +process.exit(fail ? 1 : 0) diff --git a/tag-leak.integration.test.mjs b/tag-leak.integration.test.mjs new file mode 100644 index 0000000..b5cb71b --- /dev/null +++ b/tag-leak.integration.test.mjs @@ -0,0 +1,90 @@ +// End-to-end: store_memory repairs sibling fields that leaked into content. +// +// The unit test covers the parser; this one covers the wiring — that the MCP +// handler runs the repair before the write on both the normal and the +// quarantine path, that repaired fields (not the zod defaults) are what land +// in the row, that an explicitly passed value survives, and that prose or +// markup is stored untouched. +// +// Run: TOKENMEM_DB_PATH=/tmp/x.db node tag-leak.integration.test.mjs +import { spawn } from 'node:child_process' +import { existsSync, unlinkSync } from 'node:fs' +import { dirname, resolve } from 'node:path' +import { fileURLToPath } from 'node:url' +import Database from 'better-sqlite3' +import { Client } from '@modelcontextprotocol/sdk/client/index.js' +import { StreamableHTTPClientTransport } from '@modelcontextprotocol/sdk/client/streamableHttp.js' + +const DB_PATH = process.env.TOKENMEM_DB_PATH +if (!DB_PATH) { console.error('FATAL: set TOKENMEM_DB_PATH'); process.exit(2) } +const Q_DB_PATH = DB_PATH.replace(/(\.db)?$/, '-quarantine.db') +for (const p of [DB_PATH, Q_DB_PATH]) for (const sfx of ['', '-shm', '-wal']) if (existsSync(p + sfx)) unlinkSync(p + sfx) + +const __dirname = dirname(fileURLToPath(import.meta.url)) + +let pass = 0, fail = 0 +const check = (label, cond, detail = '') => { + if (cond) { pass++; console.log(`✓ ${label}`) } + else { fail++; console.log(`✗ ${label}${detail ? ' — ' + detail : ''}`) } +} + +async function withServer(dbPath, extraEnv, fn) { + const port = 18960 + Math.floor(Math.random() * 30) + const srv = spawn(process.execPath, [resolve(__dirname, 'mcp-server.mjs'), '--transport=http', `--port=${port}`], { + env: { ...process.env, TOKENMEM_DB_PATH: dbPath, MNEME_AUTH: 'off', ...extraEnv }, stdio: 'ignore', + }) + try { + let up = false + for (let i = 0; i < 40 && !up; i++) { + try { up = (await fetch(`http://127.0.0.1:${port}/health`)).ok } catch {} + if (!up) await new Promise(r => setTimeout(r, 500)) + } + if (!up) throw new Error('server did not come up') + const client = new Client({ name: 'tag-leak-test', version: '0' }) + await client.connect(new StreamableHTTPClientTransport(new URL(`http://127.0.0.1:${port}/mcp`))) + const store = async (args) => (await client.callTool({ name: 'store_memory', arguments: args })).content[0].text + try { await fn(store) } finally { await client.close() } + } finally { + srv.kill() + } +} + +const LEAKED = 'body text.\nthe summary\n9\ndecision' +const PROSE = 'A write-up of the bug: the tail looked like `…` and then more prose follows.' +const MARKUP = 'An Atom entry for reference:\n\n entry body\n entry summary\n' + +try { + // ── normal path ── + await withServer(DB_PATH, {}, async (store) => { + const t1 = await store({ content: LEAKED }) + check('response reports the repair', /close-tag leak repaired: summary, importance, category/.test(t1), t1) + const t2 = await store({ content: PROSE, summary: 'write-up', importance: 7 }) + check('prose write is not flagged', !/leak/.test(t2), t2) + await store({ content: MARKUP, summary: 'atom example' }) + await store({ content: 'explicit.3S', importance: 10 }) + }) + const db = new Database(DB_PATH, { readonly: true }) + const [a, b, c, d] = db.prepare('SELECT content, summary, importance, category FROM memories ORDER BY rowid').all() + db.close() + check('leaked row: content truncated', a.content === 'body text.', a.content) + check('leaked row: fields restored over zod defaults', a.summary === 'the summary' && a.importance === 9 && a.category === 'decision', JSON.stringify(a)) + check('prose row: stored verbatim', b.content === PROSE && b.summary === 'write-up' && b.importance === 7, JSON.stringify(b)) + check('markup row: stored verbatim', c.content === MARKUP && c.summary === 'atom example', JSON.stringify(c)) + check('explicit importance survives a leaked one', d.importance === 10 && d.summary === 'S' && d.content === 'explicit.', JSON.stringify(d)) + + // ── quarantine path: repair runs before the write is routed ── + await withServer(Q_DB_PATH, { MNEME_QUARANTINE_HOSTS: 'cc', MNEME_DEFAULT_HOST: 'cc', MNEME_PRIMARY_HOST: 'reviewer' }, async (store) => { + const t = await store({ content: LEAKED }) + check('quarantined write reports the repair', /Quarantined/.test(t) && /close-tag leak repaired/.test(t), t) + }) + const qdb = new Database(Q_DB_PATH, { readonly: true }) + const q = qdb.prepare('SELECT content, summary, importance, category FROM memories_quarantine ORDER BY qid').get() + qdb.close() + check('quarantine row: repaired fields', q?.content === 'body text.' && q.summary === 'the summary' && q.importance === 9 && q.category === 'decision', JSON.stringify(q)) +} catch (e) { + fail++ + console.log(`✗ ${e.message}`) +} + +console.log(`\n${fail ? 'FAIL' : 'PASS'}: ${pass} passed / ${fail} failed`) +process.exitCode = fail ? 1 : 0