diff --git a/ingestion/internal/mcp/tools_notes.go b/ingestion/internal/mcp/tools_notes.go index b2c5269..72ad383 100644 --- a/ingestion/internal/mcp/tools_notes.go +++ b/ingestion/internal/mcp/tools_notes.go @@ -275,10 +275,15 @@ func checkClaimEnvelope(brainDir, path, oldStr, newStr string) error { // — this function's job is the envelope, not read failures. return nil } - block, ok := frontmatterBlock(string(before)) - if !ok || !strings.Contains(block, oldStr) { - return nil // old_str isn't in the frontmatter block at all - } + // No gate on where old_str falls relative to the frontmatter fence: + // round 2 found that requiring old_str to be fully contained in the + // extracted block was itself bypassable by an old_str disambiguated + // across the closing "---" into the body. Simpler and bypass-proof: + // always compute what the edit would produce and validate that + // resulting state's envelope. A body-only edit leaves the frontmatter + // (and thus the validation outcome) unchanged, so this is a no-op for + // the common case and fail-closed for every case that touches the + // envelope, regardless of how old_str is phrased. after := strings.Replace(string(before), oldStr, newStr, 1) return claim.ValidateEnvelope(frontmatterScalars(after)) } diff --git a/ingestion/internal/mcp/tools_notes_test.go b/ingestion/internal/mcp/tools_notes_test.go index 5989cde..b32ae14 100644 --- a/ingestion/internal/mcp/tools_notes_test.go +++ b/ingestion/internal/mcp/tools_notes_test.go @@ -141,6 +141,33 @@ func TestBrainEdit_RejectsAValueOnlyEditThatBreaksTheEnvelope(t *testing.T) { assert.Contains(t, string(got), "claim_provenance: asserted", "rejected edit must leave the note byte-identical") } +func TestBrainEdit_RejectsAnOldStrThatCrossesTheClosingFence(t *testing.T) { + // Found in round-2 review: the position-based gate required old_str to + // be fully CONTAINED in the extracted frontmatter block (which excludes + // both "---" fence lines). An old_str disambiguated by extending past + // the closing fence into the body — a natural choice, since notes.Edit + // requires old_str to be unique in the file — fails that containment + // test, so checkClaimEnvelope skips validation entirely while + // notes.Edit still applies the replace verbatim. Same invalid-enum + // outcome as the original two bypasses, reached a third way. + dir := newBrainRepo(t) + note := filepath.Join(dir, "wiki/homelab/facts/n.md") + require.NoError(t, os.WriteFile(note, + []byte("---\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\n---\n\nbody", + "new_str": "guessed\n---\n\nbody", + }) + 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")