fix(mcp): fold YAML block-scalar continuations before validating the envelope
CI / Lint / Test / Vet (pull_request) Successful in 33s
CI / Mirror to GitHub (pull_request) Skipped

Round-6 review found a bypass in a different mechanism than rounds 1-6's
fence-detection family: frontmatterScalars's indentation guard skips any
indented continuation line outright, so old_str="half_life: durable",
new_str="half_life:\n  garbage-value" turns the field into a YAML
block-scalar shape whose real value lives on the continuation line.
present()'s own "empty value = placeholder, not an assertion" contract then
treats the field as absent, and claim.ValidateEnvelope never sees the
invalid value at all — no encoding quirk needed, one ordinary brain_edit
call smuggles an out-of-enum value straight past the check.

Fold indented continuation lines into the value when a top-level key's
inline value is empty, so oneOf() still sees what was actually written. A
genuinely empty value (no indented line follows) still folds to "",
preserving the existing placeholder-not-assertion behavior for legitimately
unset fields.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This commit is contained in:
2026-09-23 10:51:09 +02:00
co-authored by Claude Sonnet 5
parent 1d67e16c0d
commit 42578c21b9
2 changed files with 60 additions and 2 deletions
+28 -2
View File
@@ -344,7 +344,9 @@ func frontmatterScalars(doc string) map[string]string {
if !ok { if !ok {
return out return out
} }
for _, line := range strings.Split(block, "\n") { lines := strings.Split(block, "\n")
for i := 0; i < len(lines); i++ {
line := lines[i]
if line != "" && (line[0] == ' ' || line[0] == '\t') { if line != "" && (line[0] == ' ' || line[0] == '\t') {
continue continue
} }
@@ -352,7 +354,31 @@ func frontmatterScalars(doc string) map[string]string {
if !ok { if !ok {
continue continue
} }
out[strings.TrimSpace(key)] = strings.TrimSpace(val) val = strings.TrimSpace(val)
if val == "" {
// Round 6: a colon with nothing after it is ambiguous — it can
// be a genuinely empty/omitted value, or the start of a YAML
// block scalar whose real value lives on the indented
// continuation line(s) that follow (e.g. "half_life:\n
// durable"). Treating the latter as absent let an edit smuggle
// an invalid value straight past ValidateEnvelope: the field
// read as unset while the disk actually carried a value. Fold
// the continuation in as a best-effort join so oneOf() still
// sees it. A genuinely empty value (no indented line follows)
// still folds to "", preserving present()'s existing
// placeholder-not-assertion contract.
var cont []string
for i+1 < len(lines) {
next := lines[i+1]
if next == "" || (next[0] != ' ' && next[0] != '\t') {
break
}
cont = append(cont, strings.TrimSpace(next))
i++
}
val = strings.Join(cont, " ")
}
out[strings.TrimSpace(key)] = val
} }
return out return out
} }
@@ -249,6 +249,38 @@ func TestBrainEdit_RejectsAnInvalidEnvelopeOnABOMPrefixedNote(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_RejectsAFoldedScalarThatHidesAnInvalidValue(t *testing.T) {
// Found in round-6 review: a different mechanism from the encoding
// family (rounds 1-6 were all about fence detection; this is the field
// extractor). old_str="half_life: durable", new_str="half_life:\n
// garbage-value" turns the field into a YAML block-scalar shape: the
// top-level line is now "half_life:" with an empty inline value, and
// the real value lives on the indented continuation line that follows.
// frontmatterScalars's indentation guard skips that continuation line
// outright, so the field reads as present-with-empty-value —
// present()'s own contract then treats that as absent (a placeholder,
// not an assertion) — and claim.ValidateEnvelope never sees the
// invalid value at all. No encoding quirk needed: one ordinary
// brain_edit call smuggles an out-of-enum value straight past the
// check the field was supposed to be validated by.
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": "half_life: durable",
"new_str": "half_life:\n garbage-value",
})
require.Contains(t, resp, "error", "%v", resp)
got, err := os.ReadFile(note)
require.NoError(t, err)
assert.Contains(t, string(got), "half_life: durable", "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")