fix(mcp): drop the containment gate, always validate the post-edit envelope
CI / Lint / Test / Vet (pull_request) Successful in 41s
CI / Mirror to GitHub (pull_request) Skipped

Round-2 review found a third bypass: checkClaimEnvelope required old_str to
be fully contained within the extracted frontmatter block (which excludes
both "---" fence lines). An old_str disambiguated across the closing fence
into the body — a natural choice, since notes.Edit requires old_str to be
unique — failed that containment test, so the gate skipped validation
entirely while notes.Edit applied the replace verbatim, reaching the same
invalid-enum outcome as the two bypasses already closed.

Remove the containment gate structurally instead of patching another edge:
checkClaimEnvelope now always computes the post-edit doc and validates its
resulting frontmatter, regardless of old_str's textual relationship to the
fence. A body-only edit leaves the frontmatter unchanged (still valid, since
the note started valid), so this is a no-op for the common case and
fail-closed for every case that touches the envelope, however old_str is
phrased.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This commit is contained in:
2026-09-23 10:27:30 +02:00
co-authored by Claude Sonnet 5
parent 062574dad8
commit 0f932d8f77
2 changed files with 36 additions and 4 deletions
+9 -4
View File
@@ -275,10 +275,15 @@ func checkClaimEnvelope(brainDir, path, oldStr, newStr string) error {
// — this function's job is the envelope, not read failures. // — this function's job is the envelope, not read failures.
return nil return nil
} }
block, ok := frontmatterBlock(string(before)) // No gate on where old_str falls relative to the frontmatter fence:
if !ok || !strings.Contains(block, oldStr) { // round 2 found that requiring old_str to be fully contained in the
return nil // old_str isn't in the frontmatter block at all // 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) after := strings.Replace(string(before), oldStr, newStr, 1)
return claim.ValidateEnvelope(frontmatterScalars(after)) return claim.ValidateEnvelope(frontmatterScalars(after))
} }
@@ -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") 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) { func TestBrainEdit_AllowsAValidClaimEnvelopeEdit(t *testing.T) {
dir := newBrainRepo(t) dir := newBrainRepo(t)
note := filepath.Join(dir, "wiki/homelab/facts/n.md") note := filepath.Join(dir, "wiki/homelab/facts/n.md")