fix(brain): close round-3 review findings on provenance
CI / Lint / Test / Vet (pull_request) Successful in 37s
CI / Mirror to GitHub (pull_request) Skipped

- B1: search.extractProvenance locates the frontmatter with
  frontmatter.Split (fence at column 0), so an indented "---" inside a
  block scalar no longer hides provenance from brain_query.
- B2: brain_edit refuses to edit without a snapshot, and if the stamp
  cannot be verified restores the note and reports failure; a failed
  restore is reported as exactly that, never as "rolled back".
- B3: internal/provenance gets direct tests for the whole-document
  backstop (a stamp that cannot read back is refused), standalone group
  decoding, duplicates, lone CR, tab continuations and unclosable
  fences. Removing verify or the restore now fails the suite.
- Principals YAML would read as non-strings (true, 123, null, ~) are
  quoted instead of refused. An unclosable fence ("---x") is refused
  rather than demoted into the body. writeHallNote creates the hall
  directory only after the note passes, so a refused write leaves
  nothing behind (brain#21).
- MCP brain_ingest (stub LLM) asserted to stamp the caller.

Refs #145 #144

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
2026-10-01 23:03:09 +02:00
co-authored by Claude Opus 5.5
parent fcae16569d
commit 7afbd60ea2
8 changed files with 227 additions and 33 deletions
+6 -3
View File
@@ -174,9 +174,6 @@ func writeHallNote(brainDir string, opts WriteNoteOptions) (string, error) {
return "", fmt.Errorf("claim envelope: %w", err) return "", fmt.Errorf("claim envelope: %w", err)
} }
if err := os.MkdirAll(filepath.Dir(dest), 0o755); err != nil {
return "", fmt.Errorf("create hall dir: %w", err)
}
emitted := make(map[string]bool, 6) emitted := make(map[string]bool, 6)
var fm strings.Builder var fm strings.Builder
@@ -228,6 +225,12 @@ func writeHallNote(brainDir string, opts WriteNoteOptions) (string, error) {
if err != nil { if err != nil {
return "", err return "", err
} }
// The directory is created only once the note is known to be writable:
// a refused write leaves nothing behind, not even an empty hall dir
// (brain#21; provenance refusal joins the envelope refusal here).
if err := os.MkdirAll(filepath.Dir(dest), 0o755); err != nil {
return "", fmt.Errorf("create hall dir: %w", err)
}
if err := os.WriteFile(dest, []byte(final), 0o644); err != nil { if err := os.WriteFile(dest, []byte(final), 0o644); err != nil {
return "", fmt.Errorf("write: %w", err) return "", fmt.Errorf("write: %w", err)
} }
+12
View File
@@ -108,3 +108,15 @@ func TestRESTIngestStampsUnknown(t *testing.T) {
assert.Contains(t, strings.SplitN(string(b), "---\n", 3)[1], "origin: unknown\n", p) assert.Contains(t, strings.SplitN(string(b), "---\n", 3)[1], "origin: unknown\n", p)
} }
} }
// A refused write leaves nothing behind, not even an empty hall directory
// (brain#21's invariant, round 3 non-blocking finding).
func TestWriteNoteRefusedForProvenanceLeavesNoDirectory(t *testing.T) {
dir := t.TempDir()
_, err := api.WriteNote(dir, api.WriteNoteOptions{
Content: "# x\n\nbody", Wing: "freshwing", Hall: "facts", Filename: "x", Origin: "jwt", WrittenBy: "bad\xff",
})
require.Error(t, err)
_, statErr := os.Stat(filepath.Join(dir, "wiki", "freshwing"))
assert.True(t, os.IsNotExist(statErr), "no wing directory may be created by a refused write")
}
@@ -2,6 +2,7 @@ package mcp_test
import ( import (
"bytes" "bytes"
"context"
"encoding/json" "encoding/json"
"log/slog" "log/slog"
"os" "os"
@@ -20,6 +21,7 @@ import (
"github.com/mathiasbq/hyperguild/ingestion/internal/capturehttp" "github.com/mathiasbq/hyperguild/ingestion/internal/capturehttp"
"github.com/mathiasbq/hyperguild/ingestion/internal/classification" "github.com/mathiasbq/hyperguild/ingestion/internal/classification"
"github.com/mathiasbq/hyperguild/ingestion/internal/mcp" "github.com/mathiasbq/hyperguild/ingestion/internal/mcp"
"github.com/mathiasbq/hyperguild/ingestion/internal/pipeline"
) )
// Escapes found by the PR #147 gatekeeper review. Each test drives the real // Escapes found by the PR #147 gatekeeper review. Each test drives the real
@@ -260,3 +262,64 @@ func TestToolsCallAuditErrorOutcomeCarriesNoArgumentValues(t *testing.T) {
} }
assert.True(t, sawError, "the failing calls must still be recorded as errors") assert.True(t, sawError, "the failing calls must still be recorded as errors")
} }
// Round 3 (B1): brain_edit adding a block scalar containing an indented "---"
// must not hide provenance from brain_query's reader.
func TestBrainEditBlockScalarFenceDoesNotHideProvenanceFromQuery(t *testing.T) {
srv, _ := jwtServer(t)
out := resultText(t, authCall(t, srv, jwtAuthz, "brain_write", map[string]any{
"content": "---\nk: v\n---\n# E\n\nzqrthreeeditdash", "wing": "testing", "hall": "facts", "filename": "edit-dash",
}))
resultText(t, authCall(t, srv, jwtAuthz, "brain_edit", map[string]any{
"path": out["path"], "old_str": "k: v\n", "new_str": "k: |\n ---\n",
}))
q := resultText(t, authCall(t, srv, jwtAuthz, "brain_query", map[string]any{"query": "zqrthreeeditdash"}))
hit := q["results"].([]any)[0].(map[string]any)
assert.Equal(t, "jwt", hit["origin"])
assert.Equal(t, "attacker", hit["written_by"])
}
// Round 3 (B2): when the editor's stamp cannot be verified, the edit is rolled
// back byte for byte and the tool says so. A JWT subject with an invalid UTF-8
// byte cannot round-trip through YAML, which forces that path.
func TestBrainEditRollsBackWhenTheStampCannotBeVerified(t *testing.T) {
dir := newBrainRepo(t)
cfg, err := classification.Load(dir)
require.NoError(t, err)
svc := capture.NewService(brainstore.New(dir), capFakeTracker{}, cfg, audit.NewSlogSink(nil))
srv := mcp.NewServer(dir, nil, nil, nil)
srv.WithCapture(svc, capFakeValidator{subject: "bad\xff"}, capStaticTok, "local-cli", capturehttp.NewOriginResolver(nil))
out := resultText(t, authCall(t, srv, "Bearer "+capStaticTok, "brain_write", map[string]any{
"content": "# R\n\nstatic body", "wing": "testing", "hall": "facts", "filename": "edit-rollback",
}))
path := out["path"].(string)
before, err := os.ReadFile(filepath.Join(dir, path))
require.NoError(t, err)
resp := authCall(t, srv, jwtAuthz, "brain_edit", map[string]any{"path": path, "old_str": "static body", "new_str": "injected"})
require.NotNil(t, resp["error"], "the edit must be reported as failed")
assert.Contains(t, resp["error"].(map[string]any)["message"], "rolled back")
after, err := os.ReadFile(filepath.Join(dir, path))
require.NoError(t, err)
assert.Equal(t, string(before), string(after), "the note must be byte-identical to before the edit")
}
// MCP brain_ingest (the LLM path) attributes its pages to the caller.
func TestBrainIngestViaMCPStampsCaller(t *testing.T) {
dir := newBrainRepo(t)
cfg, err := classification.Load(dir)
require.NoError(t, err)
svc := capture.NewService(brainstore.New(dir), capFakeTracker{}, cfg, audit.NewSlogSink(nil))
stub := pipeline.Config{Complete: func(context.Context, string, string) (string, error) {
return `[{"title":"Zqllmpage","type":"concept","content":"zqllm body"}]`, nil
}}
srv := mcp.NewServer(dir, &stub, nil, nil)
srv.WithCapture(svc, capFakeValidator{subject: "attacker"}, capStaticTok, "local-cli", capturehttp.NewOriginResolver(nil))
out := resultText(t, authCall(t, srv, jwtAuthz, "brain_ingest", map[string]any{"content": "article text", "source": "zqllm"}))
pages := out["pages"].([]any)
require.NotEmpty(t, pages)
for _, p := range pages {
assertProvenance(t, srv, dir, p.(string), "jwt", "attacker")
}
}
+15 -6
View File
@@ -110,10 +110,17 @@ func (s *Server) brainEdit(ctx context.Context, args json.RawMessage) (json.RawM
// verified, the edit is rolled back rather than left on disk unattributed // verified, the edit is rolled back rather than left on disk unattributed
// (or carrying whatever provenance the edit wrote). // (or carrying whatever provenance the edit wrote).
var a brainEditArgs var a brainEditArgs
_ = json.Unmarshal(args, &a) if err := json.Unmarshal(args, &a); err != nil {
var prior []byte return nil, fmt.Errorf("parse args: %w", err)
if t, rerr := notes.Resolve(s.brainDir, a.Path, notes.ScopeNotes); rerr == nil { }
prior, _ = os.ReadFile(t.Abs) target, err := notes.Resolve(s.brainDir, a.Path, notes.ScopeNotes)
if err != nil {
return nil, err
}
// No snapshot, no edit: without one a failed stamp could not be undone.
prior, err := os.ReadFile(target.Abs)
if err != nil {
return nil, fmt.Errorf("snapshot before edit: %w", err)
} }
res, out, err := s.editWithScope(args, notes.ScopeNotes) res, out, err := s.editWithScope(args, notes.ScopeNotes)
if err != nil { if err != nil {
@@ -124,8 +131,10 @@ func (s *Server) brainEdit(ctx context.Context, args json.RawMessage) (json.RawM
// to the editor after every edit (PR #147 review, escape E1). // to the editor after every edit (PR #147 review, escape E1).
origin, writtenBy := provenanceFrom(ctx) origin, writtenBy := provenanceFrom(ctx)
if serr := api.StampProvenanceFile(s.brainDir, res.Path, origin, writtenBy); serr != nil { if serr := api.StampProvenanceFile(s.brainDir, res.Path, origin, writtenBy); serr != nil {
if t, rerr := notes.Resolve(s.brainDir, res.Path, notes.ScopeNotes); rerr == nil && prior != nil { if werr := os.WriteFile(target.Abs, prior, 0o644); werr != nil {
_ = os.WriteFile(t.Abs, prior, 0o644) // Say exactly what state the note is in: edited, unverified.
return nil, fmt.Errorf("stamp provenance failed (%v) AND the edit could NOT be rolled back: %w; "+
"%s carries the edit under its previous attribution", serr, werr, res.Path)
} }
return nil, fmt.Errorf("edit rolled back, stamp provenance: %w", serr) return nil, fmt.Errorf("edit rolled back, stamp provenance: %w", serr)
} }
+18 -5
View File
@@ -85,9 +85,15 @@ func Apply(content string, s Stamp) (string, error) {
content = foldFrontmatterCR(content) content = foldFrontmatterCR(content)
block, rest, ok := sharedfm.Split([]byte(content)) block, rest, ok := sharedfm.Split([]byte(content))
var out string var out string
if !ok { switch {
case !ok && strings.HasPrefix(strings.ReplaceAll(content, "\r\n", "\n"), "---\n"):
// A fence was opened but cannot be closed (e.g. a "---x" line).
// Prepending a second block would demote the original frontmatter
// into the body, so refuse instead (PR #147 review round 3).
return "", fmt.Errorf("%w: unclosed frontmatter fence", ErrUnverifiable)
case !ok:
out = "---\n" + stamp + "---\n" + content out = "---\n" + stamp + "---\n" + content
} else { default:
out = "---\n" + plainGroups(string(block)) + stamp + "---\n" + strings.TrimPrefix(string(rest), "\n") out = "---\n" + plainGroups(string(block)) + stamp + "---\n" + strings.TrimPrefix(string(rest), "\n")
} }
if err := verify(out, s); err != nil { if err := verify(out, s); err != nil {
@@ -203,11 +209,18 @@ func foldFrontmatterCR(content string) string {
return strings.ReplaceAll(s[:4+end], "\r", "\n") + s[4+end:] return strings.ReplaceAll(s[:4+end], "\r", "\n") + s[4+end:]
} }
// scalar renders a bare word as-is and anything else as a JSON string (valid // scalar renders a bare word as-is only when YAML reads it back as that same
// YAML double-quoted), so a principal name can never break the line. // string; anything else (true, 123, null, ~, punctuation, newlines) becomes
// a double-quoted string, so a principal name can never break the line or
// change type.
func scalar(v string) string { func scalar(v string) string {
if bareScalar.MatchString(v) { if bareScalar.MatchString(v) {
return v var back any
if yaml.Unmarshal([]byte(v), &back) == nil {
if s, ok := back.(string); ok && s == v {
return v
}
}
} }
return fmt.Sprintf("%q", v) return fmt.Sprintf("%q", v)
} }
@@ -0,0 +1,86 @@
package provenance
import (
"errors"
"strings"
"testing"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"gopkg.in/yaml.v3"
)
func fmOf(t *testing.T, doc string) map[string]any {
t.Helper()
parts := strings.SplitN(doc, "---\n", 3)
require.Len(t, parts, 3, doc)
var m map[string]any
require.NoError(t, yaml.Unmarshal([]byte(parts[1]), &m), parts[1])
return m
}
// The whole-document backstop: a stamp that would not read back as written
// is refused, never written. A principal with an invalid UTF-8 byte quotes
// to "\xff", which YAML decodes as U+00FF, so it cannot round-trip.
func TestApplyRefusesAStampThatDoesNotReadBack(t *testing.T) {
_, err := Apply("---\ntitle: t\n---\nbody\n", Stamp{Origin: "jwt", WrittenBy: "bad\xff"})
assert.True(t, errors.Is(err, ErrUnverifiable), "got %v", err)
}
// A principal that YAML would decode as a non-string (true, 123, null) is
// quoted, so the stamp reads back as the same string.
func TestApplyQuotesPrincipalsYAMLWouldNotReadAsStrings(t *testing.T) {
for _, p := range []string{"true", "123", "null", "1e3", "0x1F", "yes", "~"} {
out, err := Apply("---\ntitle: t\n---\nbody\n", Stamp{Origin: "jwt", WrittenBy: p})
require.NoError(t, err, p)
assert.Equal(t, p, fmOf(t, out)["written_by"], p)
}
}
// A group that does not decode on its own is dropped; its neighbours stay.
func TestApplyDropsGroupsThatDoNotDecodeAlone(t *testing.T) {
out, err := Apply("---\nkeep: 1\ntitle: a: b\nalso: 2\n---\nbody\n", Stamp{Origin: "static", WrittenBy: "local-cli"})
require.NoError(t, err)
m := fmOf(t, out)
assert.Equal(t, 1, m["keep"])
assert.Equal(t, 2, m["also"])
assert.NotContains(t, m, "title")
}
func TestApplyKeepsFirstOfDuplicateKeys(t *testing.T) {
out, err := Apply("---\ntitle: first\ntitle: second\nTitle: third\n---\nbody\n", Stamp{})
require.NoError(t, err)
m := fmOf(t, out)
assert.Equal(t, "first", m["title"])
assert.NotContains(t, m, "Title")
}
func TestApplyFoldsLoneCRSoItCannotHideALine(t *testing.T) {
out, err := Apply("---\ntitle: x\rorigin: static\n---\nbody\n", Stamp{Origin: "jwt", WrittenBy: "a"})
require.NoError(t, err)
assert.NotContains(t, out, "\r")
assert.Equal(t, "jwt", fmOf(t, out)["origin"])
assert.Equal(t, "x", fmOf(t, out)["title"])
}
func TestApplyDropsTabIndentedContinuations(t *testing.T) {
out, err := Apply("---\ntitle: x\n\torigin: static\nk: v\n---\nbody\n", Stamp{Origin: "jwt", WrittenBy: "a"})
require.NoError(t, err)
m := fmOf(t, out)
assert.Equal(t, "jwt", m["origin"])
assert.Equal(t, "v", m["k"])
}
// Content that opens a fence the shared splitter cannot close (e.g. a
// "---x" line) is refused rather than given a second, prepended block that
// demotes the original frontmatter into the body.
func TestApplyRefusesAnUnclosableFrontmatterFence(t *testing.T) {
_, err := Apply("---\ntitle: t\n---x\norigin: static\n---\nbody\n", Stamp{Origin: "jwt", WrittenBy: "a"})
assert.True(t, errors.Is(err, ErrUnverifiable), "got %v", err)
}
func TestApplyAddsFrontmatterToContentWithoutAny(t *testing.T) {
out, err := Apply("# plain\n\norigin: static is prose\n", Stamp{Origin: "static", WrittenBy: "local-cli"})
require.NoError(t, err)
assert.True(t, strings.HasPrefix(out, "---\norigin: static\nwritten_by: local-cli\n---\n# plain"))
}
@@ -19,3 +19,11 @@ func TestExtractProvenanceIgnoresBodyLines(t *testing.T) {
assert.Equal(t, "jwt", o) assert.Equal(t, "jwt", o)
assert.Equal(t, "attacker", w) assert.Equal(t, "attacker", w)
} }
// Round 3 (B1): an indented "---" inside a block scalar is content, not the
// closing fence — the same rule as frontmatter.Split.
func TestExtractProvenanceUsesTheSharedFenceRule(t *testing.T) {
o, w := extractProvenance("---\nk: |\n ---\norigin: jwt\nwritten_by: attacker\n---\nbody\n")
assert.Equal(t, "jwt", o)
assert.Equal(t, "attacker", w)
}
+19 -19
View File
@@ -5,6 +5,7 @@ import (
"bufio" "bufio"
"context" "context"
"fmt" "fmt"
sharedfm "github.com/mathiasbq/hyperguild/ingestion/internal/frontmatter"
"log/slog" "log/slog"
"os" "os"
"path/filepath" "path/filepath"
@@ -296,33 +297,32 @@ func hydrate(brainDir, relPath string) (Result, error) {
} }
// extractProvenance reads origin/written_by from the leading frontmatter // extractProvenance reads origin/written_by from the leading frontmatter
// block only — a body line that happens to start with "origin:" is not // block only, located with the shared fence rule (frontmatter.Split: a "---"
// provenance. // at column 0) so an indented "---" inside a block scalar is content, not a
// closing fence (PR #147 review round 3). A body line is never provenance.
func extractProvenance(content string) (origin, writtenBy string) { func extractProvenance(content string) (origin, writtenBy string) {
scanner := bufio.NewScanner(strings.NewReader(content)) block, _, ok := sharedfm.Split([]byte(content))
inFrontmatter := false if !ok {
for scanner.Scan() { return "", ""
line := scanner.Text() }
if strings.TrimSpace(line) == "---" { for _, line := range strings.Split(string(block), "\n") {
if !inFrontmatter { if line == "" || line[0] == ' ' || line[0] == '\t' {
inFrontmatter = true continue
continue
}
break
} }
if !inFrontmatter { key, val, found := strings.Cut(line, ":")
return "", "" if !found {
}
key, val, ok := strings.Cut(line, ":")
if !ok || strings.HasPrefix(line, " ") || strings.HasPrefix(line, "\t") {
continue continue
} }
v := strings.Trim(strings.TrimSpace(val), `"'`) v := strings.Trim(strings.TrimSpace(val), `"'`)
switch strings.TrimSpace(key) { switch strings.TrimSpace(key) {
case "origin": case "origin":
origin = v if origin == "" {
origin = v
}
case "written_by": case "written_by":
writtenBy = v if writtenBy == "" {
writtenBy = v
}
} }
} }
return origin, writtenBy return origin, writtenBy