fix(brain): provenance as a post-condition on every model-reachable write
The PR #147 review forged origin: static as a JWT principal through three tools the first version never touched (brain_edit, brain_ingest_raw, ingest_raw + brain_promote), and through YAML spellings a key-name match missed (quoted, escaped, tagged, flow-mapped, merge keys, lone CR). - api.StampProvenance removes every spelling of origin/written_by from the frontmatter and appends the real values. A line is dropped if EITHER yaml.v3 (decoding quotes, escapes, tags, flow maps) or the brain's line-oriented readers would read it as provenance; complex keys, orphan values and merge keys are dropped outright. Lone CRs in frontmatter become LF. Principal values that are not bare words are quoted. - Applied as the last step of writeHallNote, UpdateNote, PromoteNote (re-attributed to the promoter), brain_edit (re-attributed to the editor) and both ingests (every page written or merged). REST /write and /promote carry no principal and stamp unknown. - Ingest: newlines in scalar fields fold to spaces; a page type must be a plain lower-case name (../raw wrote outside wiki/, pre-existing). - Audit: outcome is ok|error, never the error text, which echoed argument values. Tests: every escape from the review is a table case asserting the invariant on the file (parses as YAML, yaml.v3 and brain_get agree, exactly the stamped lines). Plus REST /write, REST capture AuthPath, body-line provenance in search, and quoted principals; two of the review's surviving flips now go red. Three tests renamed so names claim only what they assert. Refs #145 #144 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
@@ -35,12 +35,13 @@ func logToolCall(ctx context.Context, name string, args json.RawMessage, start t
|
||||
attrs = append(attrs, "content_bytes", len(a.Content))
|
||||
}
|
||||
}
|
||||
// The outcome is a class, never the error text: handler errors echo
|
||||
// argument values (a rejected half_life, a requested path), which would
|
||||
// make the audit line a copy of what the caller sent (PR #147 review E6).
|
||||
// The caller already received the full error in the tool response.
|
||||
outcome := "ok"
|
||||
if err != nil {
|
||||
outcome = err.Error()
|
||||
if len(outcome) > 200 {
|
||||
outcome = outcome[:200]
|
||||
}
|
||||
outcome = "error"
|
||||
}
|
||||
attrs = append(attrs, "outcome", outcome)
|
||||
slog.InfoContext(ctx, "mcp tool call", attrs...)
|
||||
|
||||
@@ -64,7 +64,7 @@ func TestToolsCallLogsErrorOutcome(t *testing.T) {
|
||||
authCall(t, srv, "Bearer "+capStaticTok, "brain_query", map[string]any{})
|
||||
})
|
||||
require.Len(t, recs, 1)
|
||||
assert.Contains(t, recs[0]["outcome"], "query is required")
|
||||
assert.Equal(t, "error", recs[0]["outcome"])
|
||||
}
|
||||
|
||||
// The audit line must not become a second copy of brain content.
|
||||
|
||||
@@ -393,8 +393,10 @@ func (s *Server) brainPromote(ctx context.Context, args json.RawMessage) (json.R
|
||||
if err := json.Unmarshal(args, &a); err != nil {
|
||||
return nil, fmt.Errorf("parse args: %w", err)
|
||||
}
|
||||
origin, writtenBy := provenanceFrom(ctx)
|
||||
relPath, err := api.PromoteNote(s.brainDir, api.PromoteOptions{
|
||||
Filename: a.Filename, Wing: a.Wing, Hall: a.Hall, Slug: a.Slug,
|
||||
Origin: origin, WrittenBy: writtenBy,
|
||||
})
|
||||
if err != nil {
|
||||
return nil, err
|
||||
@@ -510,6 +512,9 @@ func (s *Server) brainIngestRaw(ctx context.Context, args json.RawMessage) (json
|
||||
warnings = []string{}
|
||||
}
|
||||
if !a.DryRun {
|
||||
if err := s.stampPages(ctx, pages); err != nil {
|
||||
return nil, err
|
||||
}
|
||||
for _, p := range pages {
|
||||
s.indexInGraph(ctx, "brain_ingest_raw", p)
|
||||
}
|
||||
@@ -605,6 +610,9 @@ func (s *Server) runIngest(ctx context.Context, content, source string, dryRun b
|
||||
pages = []string{}
|
||||
}
|
||||
if !dryRun {
|
||||
if err := s.stampPages(ctx, pages); err != nil {
|
||||
return nil, err
|
||||
}
|
||||
for _, p := range pages {
|
||||
s.indexInGraph(ctx, "brain_ingest", p)
|
||||
}
|
||||
@@ -615,3 +623,16 @@ func (s *Server) runIngest(ctx context.Context, content, source string, dryRun b
|
||||
}
|
||||
return json.Marshal(map[string]any{"pages": pages, "warnings": warnings})
|
||||
}
|
||||
|
||||
// stampPages attributes every page an ingest wrote (or merged into) to the
|
||||
// calling principal (hyperguild#145). Ingest builds frontmatter itself, so
|
||||
// without this its pages carried no provenance at all.
|
||||
func (s *Server) stampPages(ctx context.Context, pages []string) error {
|
||||
origin, writtenBy := provenanceFrom(ctx)
|
||||
for _, p := range pages {
|
||||
if err := api.StampProvenanceFile(s.brainDir, p, origin, writtenBy); err != nil {
|
||||
return fmt.Errorf("stamp provenance: %w", err)
|
||||
}
|
||||
}
|
||||
return nil
|
||||
}
|
||||
|
||||
@@ -0,0 +1,227 @@
|
||||
package mcp_test
|
||||
|
||||
import (
|
||||
"bytes"
|
||||
"encoding/json"
|
||||
"log/slog"
|
||||
"os"
|
||||
"path/filepath"
|
||||
"regexp"
|
||||
"strings"
|
||||
"testing"
|
||||
|
||||
"github.com/stretchr/testify/assert"
|
||||
"github.com/stretchr/testify/require"
|
||||
"gopkg.in/yaml.v3"
|
||||
|
||||
"github.com/mathiasbq/hyperguild/ingestion/internal/audit"
|
||||
"github.com/mathiasbq/hyperguild/ingestion/internal/brainstore"
|
||||
"github.com/mathiasbq/hyperguild/ingestion/internal/capture"
|
||||
"github.com/mathiasbq/hyperguild/ingestion/internal/capturehttp"
|
||||
"github.com/mathiasbq/hyperguild/ingestion/internal/classification"
|
||||
"github.com/mathiasbq/hyperguild/ingestion/internal/mcp"
|
||||
)
|
||||
|
||||
// Escapes found by the PR #147 gatekeeper review. Each test drives the real
|
||||
// mcp.Server as a JWT principal "attacker" and asserts the provenance
|
||||
// invariant on the file every reader sees.
|
||||
|
||||
const jwtAuthz = "Bearer jwt-token"
|
||||
|
||||
func jwtServer(t *testing.T) (*mcp.Server, string) {
|
||||
t.Helper()
|
||||
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: "attacker"}, capStaticTok, "local-cli",
|
||||
capturehttp.NewOriginResolver(nil))
|
||||
return srv, dir
|
||||
}
|
||||
|
||||
var provenanceLine = regexp.MustCompile(`(?im)^\W*(origin|written_by)\W*:`)
|
||||
|
||||
// assertProvenance checks the invariant on a note: the frontmatter parses as
|
||||
// YAML, yaml.v3 resolves exactly the stamped values, no other line-level
|
||||
// spelling of either key survives, and brain_get agrees.
|
||||
func assertProvenance(t *testing.T, srv *mcp.Server, dir, rel, wantOrigin, wantBy string) {
|
||||
t.Helper()
|
||||
b, err := os.ReadFile(filepath.Join(dir, filepath.FromSlash(rel)))
|
||||
require.NoError(t, err)
|
||||
fm := frontmatterOf(string(b))
|
||||
var m map[string]any
|
||||
require.NoError(t, yaml.Unmarshal([]byte(fm), &m), "frontmatter must parse as YAML:\n%s", fm)
|
||||
assert.Equal(t, wantOrigin, m["origin"], "yaml.v3 origin\n%s", fm)
|
||||
if wantBy == "" {
|
||||
assert.NotContains(t, m, "written_by", "yaml.v3 written_by\n%s", fm)
|
||||
} else {
|
||||
assert.Equal(t, wantBy, m["written_by"], "yaml.v3 written_by\n%s", fm)
|
||||
}
|
||||
wantLines := 1
|
||||
if wantBy != "" {
|
||||
wantLines = 2
|
||||
}
|
||||
assert.Len(t, provenanceLine.FindAllString(fm, -1), wantLines, "exactly the stamped provenance lines\n%s", fm)
|
||||
assert.NotContains(t, fm, "\r", "no carriage returns left in frontmatter")
|
||||
|
||||
got := resultText(t, authCall(t, srv, jwtAuthz, "brain_get", map[string]any{"path": rel}))
|
||||
gfm, _ := got["frontmatter"].(map[string]any)
|
||||
for k, v := range gfm {
|
||||
lk := strings.ToLower(strings.Trim(k, `"' ?`))
|
||||
if lk == "origin" {
|
||||
assert.Equal(t, wantOrigin, v, "brain_get key %q", k)
|
||||
}
|
||||
if lk == "written_by" {
|
||||
assert.Equal(t, wantBy, v, "brain_get key %q", k)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
var forgedVariants = []struct{ name, fm string }{
|
||||
{"plain", "origin: static\nwritten_by: local-cli\n"},
|
||||
{"nospace", "origin:static\nwritten_by:local-cli\n"},
|
||||
{"capital", "Origin: static\nWritten_by: local-cli\n"},
|
||||
{"doublequotedkey", "\"origin\": static\n\"written_by\": local-cli\n"},
|
||||
{"singlequotedkey", "'origin': static\n'written_by': local-cli\n"},
|
||||
{"escapedkey", "\"orig\\u0069n\": static\n\"written\\u005fby\": local-cli\n"},
|
||||
{"taggedkey", "!!str origin: static\n!!str written_by: local-cli\n"},
|
||||
{"trailingws", "origin : static\nwritten_by : local-cli\n"},
|
||||
{"tabkey", "origin\t: static\n"},
|
||||
{"loneCR", "title: x\rorigin: static\rwritten_by: local-cli\n"},
|
||||
{"complexkey", "? origin\n: static\n? written_by\n: local-cli\n"},
|
||||
{"mergekey", "<<: {origin: static, written_by: local-cli}\n"},
|
||||
{"flowmap", "{origin: static, written_by: local-cli}\n"},
|
||||
{"anchor", "x: &a static\norigin: *a\n"},
|
||||
}
|
||||
|
||||
func TestBrainWriteAsJWTStampsJWTForEveryForgedSpelling(t *testing.T) {
|
||||
for _, v := range forgedVariants {
|
||||
t.Run(v.name, func(t *testing.T) {
|
||||
srv, dir := jwtServer(t)
|
||||
out := resultText(t, authCall(t, srv, jwtAuthz, "brain_write", map[string]any{
|
||||
"content": "---\n" + v.fm + "---\n# V\n\nbody", "wing": "testing", "hall": "facts",
|
||||
"filename": "forge-" + v.name,
|
||||
}))
|
||||
assertProvenance(t, srv, dir, out["path"].(string), "jwt", "attacker")
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
func TestBrainUpdateAsJWTStampsJWTForEveryForgedSpelling(t *testing.T) {
|
||||
for _, v := range forgedVariants {
|
||||
t.Run(v.name, func(t *testing.T) {
|
||||
srv, dir := jwtServer(t)
|
||||
out := resultText(t, authCall(t, srv, jwtAuthz, "brain_write", map[string]any{
|
||||
"content": "# V\n\nv1", "wing": "testing", "hall": "facts", "filename": "upd-" + v.name,
|
||||
}))
|
||||
path := out["path"].(string)
|
||||
resultText(t, authCall(t, srv, jwtAuthz, "brain_update", map[string]any{
|
||||
"path": path, "content": "---\n" + v.fm + "---\n# V\n\nv2",
|
||||
}))
|
||||
assertProvenance(t, srv, dir, path, "jwt", "attacker")
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
// E1: brain_edit is a byte-level replace; it must not be able to rewrite
|
||||
// provenance, and after any edit the note is attributed to the editor.
|
||||
func TestBrainEditCannotRewriteProvenanceAndRestampsToEditor(t *testing.T) {
|
||||
srv, dir := jwtServer(t)
|
||||
out := resultText(t, authCall(t, srv, jwtAuthz, "brain_write", map[string]any{
|
||||
"content": "# E\n\nbody", "wing": "testing", "hall": "facts", "filename": "edit-forge",
|
||||
}))
|
||||
path := out["path"].(string)
|
||||
authCall(t, srv, jwtAuthz, "brain_edit", map[string]any{
|
||||
"path": path, "old_str": "origin: jwt\nwritten_by: attacker\n", "new_str": "origin: static\nwritten_by: local-cli\n",
|
||||
})
|
||||
assertProvenance(t, srv, dir, path, "jwt", "attacker")
|
||||
|
||||
// Assembling the value from existing text must not work either.
|
||||
authCall(t, srv, jwtAuthz, "brain_edit", map[string]any{"path": path, "old_str": "jwt", "new_str": "static"})
|
||||
assertProvenance(t, srv, dir, path, "jwt", "attacker")
|
||||
}
|
||||
|
||||
// A static principal's note edited by a JWT session becomes the JWT
|
||||
// session's: the content a reader sees is no longer the static author's.
|
||||
func TestBrainEditByJWTReattributesAStaticNote(t *testing.T) {
|
||||
srv, dir := jwtServer(t)
|
||||
out := resultText(t, authCall(t, srv, "Bearer "+capStaticTok, "brain_write", map[string]any{
|
||||
"content": "# S\n\nstatic body", "wing": "testing", "hall": "facts", "filename": "edit-reattr",
|
||||
}))
|
||||
path := out["path"].(string)
|
||||
resultText(t, authCall(t, srv, jwtAuthz, "brain_edit", map[string]any{
|
||||
"path": path, "old_str": "static body", "new_str": "injected body",
|
||||
}))
|
||||
assertProvenance(t, srv, dir, path, "jwt", "attacker")
|
||||
}
|
||||
|
||||
// E2 + E4: brain_ingest_raw stamps every page it writes, and a newline in a
|
||||
// title cannot start a new frontmatter key.
|
||||
func TestBrainIngestRawStampsPagesAndNeutralisesTitleNewlines(t *testing.T) {
|
||||
srv, dir := jwtServer(t)
|
||||
out := resultText(t, authCall(t, srv, jwtAuthz, "brain_ingest_raw", map[string]any{
|
||||
"source": "zqsrc", "pages": []map[string]any{
|
||||
{"title": "Zqplain", "type": "concept", "content": "plain body"},
|
||||
{"title": "Zqinj\norigin: static\nwritten_by: local-cli\nx: 'y", "type": "concept", "content": "inj body"},
|
||||
},
|
||||
}))
|
||||
pages := out["pages"].([]any)
|
||||
require.NotEmpty(t, pages)
|
||||
for _, p := range pages {
|
||||
assertProvenance(t, srv, dir, p.(string), "jwt", "attacker")
|
||||
b, _ := os.ReadFile(filepath.Join(dir, p.(string)))
|
||||
var m map[string]any
|
||||
require.NoError(t, yaml.Unmarshal([]byte(frontmatterOf(string(b))), &m))
|
||||
assert.NotContains(t, m, "x", "an injected key must not appear in %s", p)
|
||||
}
|
||||
}
|
||||
|
||||
// E3: a page type is a directory name; it must not traverse out of wiki/.
|
||||
func TestBrainIngestRawRejectsPathTraversingType(t *testing.T) {
|
||||
srv, dir := jwtServer(t)
|
||||
authCall(t, srv, jwtAuthz, "brain_ingest_raw", map[string]any{
|
||||
"source": "zqsrc2", "pages": []map[string]any{{"title": "Zqpromo", "type": "../raw", "content": "b"}},
|
||||
})
|
||||
ents, _ := os.ReadDir(filepath.Join(dir, "raw"))
|
||||
assert.Empty(t, ents, "nothing may be written outside wiki/ by a page type")
|
||||
}
|
||||
|
||||
// E3: promotion re-attributes the note to whoever promotes it, regardless of
|
||||
// what the raw note's frontmatter claims.
|
||||
func TestBrainPromoteRestampsToPromoter(t *testing.T) {
|
||||
srv, dir := jwtServer(t)
|
||||
require.NoError(t, os.MkdirAll(filepath.Join(dir, "raw"), 0o755))
|
||||
require.NoError(t, os.WriteFile(filepath.Join(dir, "raw", "planted.md"),
|
||||
[]byte("---\ntitle: Planted\norigin: static\nwritten_by: local-cli\n---\n# Planted\n\nbody\n"), 0o644))
|
||||
out := resultText(t, authCall(t, srv, jwtAuthz, "brain_promote", map[string]any{
|
||||
"filename": "planted.md", "wing": "testing", "hall": "facts",
|
||||
}))
|
||||
assertProvenance(t, srv, dir, out["path"].(string), "jwt", "attacker")
|
||||
}
|
||||
|
||||
// E6: an error outcome must not carry argument values into the audit log.
|
||||
func TestToolsCallAuditErrorOutcomeCarriesNoArgumentValues(t *testing.T) {
|
||||
const sentinel = "SENTINEL-ERR-7d1e"
|
||||
srv, _ := jwtServer(t)
|
||||
var buf bytes.Buffer
|
||||
prev := slog.Default()
|
||||
slog.SetDefault(slog.New(slog.NewJSONHandler(&buf, nil)))
|
||||
t.Cleanup(func() { slog.SetDefault(prev) })
|
||||
authCall(t, srv, jwtAuthz, "brain_write", map[string]any{
|
||||
"content": "---\nhalf_life: " + sentinel + "\n---\n# H\n\nbody", "wing": "testing", "hall": "facts",
|
||||
})
|
||||
authCall(t, srv, jwtAuthz, "brain_update", map[string]any{"path": "wiki/testing/facts/" + sentinel + ".md", "content": "x"})
|
||||
var sawError bool
|
||||
for _, l := range strings.Split(buf.String(), "\n") {
|
||||
var rec map[string]any
|
||||
if json.Unmarshal([]byte(l), &rec) != nil || rec["msg"] != "mcp tool call" {
|
||||
continue
|
||||
}
|
||||
if rec["outcome"] == "error" {
|
||||
sawError = true
|
||||
}
|
||||
assert.NotContains(t, l, sentinel)
|
||||
}
|
||||
assert.True(t, sawError, "the failing calls must still be recorded as errors")
|
||||
}
|
||||
@@ -61,7 +61,7 @@ func TestBrainWriteWithoutPrincipalStampsOriginUnknown(t *testing.T) {
|
||||
|
||||
// The control: a caller cannot claim a more trusted origin by putting it in
|
||||
// its own frontmatter.
|
||||
func TestBrainWriteOverwritesCallerSuppliedProvenance(t *testing.T) {
|
||||
func TestBrainWriteUnauthenticatedDropsPlainForgedProvenance(t *testing.T) {
|
||||
srv, dir, _, _ := govServer(t)
|
||||
forged := "---\norigin: static\nwritten_by: local-cli\n---\n# F\n\nbody"
|
||||
out := resultText(t, authCall(t, srv, "", "brain_write", map[string]any{
|
||||
@@ -76,7 +76,7 @@ func TestBrainWriteOverwritesCallerSuppliedProvenance(t *testing.T) {
|
||||
|
||||
// brain_update replaces the content wholesale, so provenance follows the
|
||||
// last writer — and a forged fragment must not survive the merge either.
|
||||
func TestBrainUpdateRestampsProvenanceAndIgnoresForgedFragment(t *testing.T) {
|
||||
func TestBrainUpdateRestampsProvenanceToTheUpdater(t *testing.T) {
|
||||
srv, dir, _, _ := govServer(t)
|
||||
out := resultText(t, authCall(t, srv, "Bearer "+capStaticTok, "brain_write", map[string]any{
|
||||
"content": "# U\n\nv1", "wing": "testing", "hall": "facts", "filename": "prov-update",
|
||||
@@ -106,7 +106,7 @@ func TestBrainQueryReturnsProvenance(t *testing.T) {
|
||||
|
||||
// capture is the door a claude.ai connector session uses, so it is the one
|
||||
// whose provenance matters most: a JWT principal must be stamped as such.
|
||||
func TestCaptureStampsJWTProvenance(t *testing.T) {
|
||||
func TestMCPCaptureStampsJWTProvenance(t *testing.T) {
|
||||
srv, brainDir := captureServer(t, capFakeValidator{subject: "koala-cli"}, []string{"koala-cli"})
|
||||
resp := captureCall(t, srv, "Bearer jwt-token", map[string]any{
|
||||
"context": map[string]any{"harness": "claude-code", "actor": "mathias", "classification": "internal"},
|
||||
|
||||
@@ -5,6 +5,7 @@ import (
|
||||
"encoding/json"
|
||||
"errors"
|
||||
"fmt"
|
||||
"github.com/mathiasbq/hyperguild/ingestion/internal/api"
|
||||
"log/slog"
|
||||
"strings"
|
||||
"time"
|
||||
@@ -103,9 +104,19 @@ type brainEditArgs struct {
|
||||
// hard the change is to reconstruct. Auditing every note edit would also bury
|
||||
// the rare governance record under routine traffic, which is how an alert
|
||||
// stops being read.
|
||||
func (s *Server) brainEdit(_ context.Context, args json.RawMessage) (json.RawMessage, error) {
|
||||
_, out, err := s.editWithScope(args, notes.ScopeNotes)
|
||||
return out, err
|
||||
func (s *Server) brainEdit(ctx context.Context, args json.RawMessage) (json.RawMessage, error) {
|
||||
res, out, err := s.editWithScope(args, notes.ScopeNotes)
|
||||
if err != nil {
|
||||
return out, err
|
||||
}
|
||||
// A byte-level replace can rewrite any line, including provenance, and
|
||||
// whoever edits a note is the author of what a reader now sees: re-stamp
|
||||
// to the editor after every edit (PR #147 review, escape E1).
|
||||
origin, writtenBy := provenanceFrom(ctx)
|
||||
if serr := api.StampProvenanceFile(s.brainDir, res.Path, origin, writtenBy); serr != nil {
|
||||
return nil, fmt.Errorf("stamp provenance: %w", serr)
|
||||
}
|
||||
return out, nil
|
||||
}
|
||||
|
||||
// brainEditRoot edits a governance file (CLAUDE.md). De-surfaced AND gated on
|
||||
|
||||
@@ -66,8 +66,11 @@ func TestBrainReadAndEdit_RoundTripThroughJSONRPC(t *testing.T) {
|
||||
|
||||
got, err := os.ReadFile(filepath.Join(dir, "wiki/homelab/facts/n.md"))
|
||||
require.NoError(t, err)
|
||||
assert.Equal(t, "---\ntitle: 'n'\n---\n\nalpha\nBETA\n", string(got),
|
||||
"only the matched bytes may change")
|
||||
// Only the matched bytes change in the body; the one deliberate addition
|
||||
// is the editor's provenance stamp (hyperguild#145 — no principal here,
|
||||
// so origin: unknown).
|
||||
assert.Equal(t, "---\ntitle: 'n'\norigin: unknown\n---\n\nalpha\nBETA\n", string(got),
|
||||
"only the matched bytes and the provenance stamp may change")
|
||||
}
|
||||
|
||||
func TestBrainEdit_EmptyOldStrErrorNamesRawArgumentBytes(t *testing.T) {
|
||||
|
||||
Reference in New Issue
Block a user