From 1d67e16c0dd39e74af5e6f2148a97e266dd29211 Mon Sep 17 00:00:00 2001 From: Mathias Date: Wed, 23 Sep 2026 10:44:54 +0200 Subject: [PATCH] fix(mcp): strip a leading UTF-8 BOM before frontmatter fence detection MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Round-5 review found a sixth bypass, same mechanism as round 4's CRLF gap, one layer earlier: a leading UTF-8 BOM (EF BB BF) defeats strings.HasPrefix(doc, "---\n") before CRLF normalization even applies. Judged live-plausible rather than exotic — Notepad's default "UTF-8" save and PowerShell's pre-v6 Out-File/Set-Content both emit a BOM — so fixed inline at the same cost as the CRLF fix rather than deferred wholesale to hyperguild#138. Strip the BOM before the CRLF normalization in frontmatterBlock, so every caller (fence-presence detection and frontmatterScalars) agrees regardless of encoding. Drawing the line here: further encoding variants in this family (NEL line separators, UTF-16 BOM, lone-CR classic-Mac endings) are judged not live-plausible in a git-tracked UTF-8 markdown corpus and are tracked wholesale under hyperguild#138's parser consolidation rather than chased as individual rounds on this PR. Co-Authored-By: Claude Sonnet 5 --- ingestion/internal/mcp/tools_notes.go | 9 ++++++-- ingestion/internal/mcp/tools_notes_test.go | 26 ++++++++++++++++++++++ 2 files changed, 33 insertions(+), 2 deletions(-) diff --git a/ingestion/internal/mcp/tools_notes.go b/ingestion/internal/mcp/tools_notes.go index 043a217..5be4b4f 100644 --- a/ingestion/internal/mcp/tools_notes.go +++ b/ingestion/internal/mcp/tools_notes.go @@ -313,9 +313,14 @@ func frontmatterBlock(doc string) (block string, ok bool) { // Round 4 found a note with CRLF fence lines ("---\r\n") silently // defeats HasPrefix(doc, "---\n") on the precondition read — not an // adversarial old_str/new_str, just an encoding difference — which - // makes checkClaimEnvelope no-op entirely. Normalize once, here, so + // makes checkClaimEnvelope no-op entirely. Round 5 found the same + // mechanism one layer earlier: a leading UTF-8 BOM (a live, plausible + // byte sequence — Notepad's default "UTF-8" save and PowerShell's + // pre-v6 Out-File/Set-Content both emit one) defeats the same prefix + // check before CRLF normalization even applies. Strip both, here, so // every caller (hadFence/hasFence detection and frontmatterScalars) - // agrees regardless of line ending. + // agrees regardless of encoding. + doc = strings.TrimPrefix(doc, "\xef\xbb\xbf") doc = strings.ReplaceAll(doc, "\r\n", "\n") if !strings.HasPrefix(doc, "---\n") { return "", false diff --git a/ingestion/internal/mcp/tools_notes_test.go b/ingestion/internal/mcp/tools_notes_test.go index d991bf2..01b18a1 100644 --- a/ingestion/internal/mcp/tools_notes_test.go +++ b/ingestion/internal/mcp/tools_notes_test.go @@ -223,6 +223,32 @@ func TestBrainEdit_RejectsAnInvalidEnvelopeOnACRLFFencedNote(t *testing.T) { assert.Contains(t, string(got), "claim_provenance: asserted", "rejected edit must leave the note byte-identical") } +func TestBrainEdit_RejectsAnInvalidEnvelopeOnABOMPrefixedNote(t *testing.T) { + // Found in round-5 review: the same "encoding property empties + // frontmatterScalars's map" mechanism as the CRLF bypass, one layer + // earlier — a leading UTF-8 BOM (EF BB BF) defeats + // strings.HasPrefix(doc, "---\n") exactly like CRLF did, before that + // prefix is even reached. Notepad's default "UTF-8" save and + // PowerShell's pre-v6 Out-File/Set-Content both emit a BOM, so this is + // a live, plausible authoring path, not an exotic one — same severity + // and same one-line cost as the CRLF fix, fixed the same way rather + // than deferred to hyperguild#138. + dir := newBrainRepo(t) + note := filepath.Join(dir, "wiki/homelab/facts/n.md") + require.NoError(t, os.WriteFile(note, + []byte("\xef\xbb\xbf---\ntitle: 'n'\nhalf_life: durable\napplies: universal\nclaim_provenance: asserted\n---\n\nbody\n"), 0o644)) + srv := mcp.NewServer(dir, nil, nil, nil) + + resp := call(t, srv, "brain_edit", map[string]any{ + "path": "wiki/homelab/facts/n.md", "old_str": "asserted", "new_str": "guessed", + }) + require.Contains(t, resp, "error", "%v", resp) + + got, err := os.ReadFile(note) + require.NoError(t, err) + assert.Contains(t, string(got), "claim_provenance: asserted", "rejected edit must leave the note byte-identical") +} + func TestBrainEdit_AllowsAValidClaimEnvelopeEdit(t *testing.T) { dir := newBrainRepo(t) note := filepath.Join(dir, "wiki/homelab/facts/n.md")