From e615d6f144b849cc929761b5413ddb309b064305 Mon Sep 17 00:00:00 2001 From: Mathias Date: Thu, 1 Oct 2026 22:29:46 +0200 Subject: [PATCH] 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 --- ingestion/internal/api/frontmatter.go | 12 - ingestion/internal/api/handler.go | 14 +- ingestion/internal/api/promote.go | 7 +- ingestion/internal/api/provenance.go | 172 +++++++++++++ ingestion/internal/api/provenance_test.go | 62 +++++ ingestion/internal/api/update.go | 8 +- .../internal/capturehttp/provenance_test.go | 51 ++++ ingestion/internal/mcp/audit.go | 9 +- ingestion/internal/mcp/audit_test.go | 2 +- ingestion/internal/mcp/handlers.go | 21 ++ .../internal/mcp/provenance_escapes_test.go | 227 ++++++++++++++++++ ingestion/internal/mcp/provenance_test.go | 6 +- ingestion/internal/mcp/tools_notes.go | 17 +- ingestion/internal/mcp/tools_notes_test.go | 7 +- ingestion/internal/pipeline/build.go | 13 + ingestion/internal/search/provenance_test.go | 21 ++ 16 files changed, 609 insertions(+), 40 deletions(-) create mode 100644 ingestion/internal/api/provenance.go create mode 100644 ingestion/internal/api/provenance_test.go create mode 100644 ingestion/internal/capturehttp/provenance_test.go create mode 100644 ingestion/internal/mcp/provenance_escapes_test.go create mode 100644 ingestion/internal/search/provenance_test.go diff --git a/ingestion/internal/api/frontmatter.go b/ingestion/internal/api/frontmatter.go index 93a5432..f0614b4 100644 --- a/ingestion/internal/api/frontmatter.go +++ b/ingestion/internal/api/frontmatter.go @@ -95,18 +95,6 @@ func (f *frontmatter) set(key, value string) { f.lines = append(f.lines, fmLine{key: key, value: value}) } -// del removes every top-level line for key. Continuation lines of a -// multi-line value are not tracked per key, so del is only for scalars. -func (f *frontmatter) del(key string) { - kept := f.lines[:0] - for _, l := range f.lines { - if l.key != key { - kept = append(kept, l) - } - } - f.lines = kept -} - // render serialises the frontmatter back into a `---`-fenced block. An // empty frontmatter renders to the empty string so bodies without a // header stay header-less. diff --git a/ingestion/internal/api/handler.go b/ingestion/internal/api/handler.go index 860b525..ef73d29 100644 --- a/ingestion/internal/api/handler.go +++ b/ingestion/internal/api/handler.go @@ -194,11 +194,6 @@ func writeHallNote(brainDir string, opts WriteNoteOptions) (string, error) { fmt.Fprintf(&fm, "hall: %s\n", opts.Hall) fmt.Fprintf(&fm, "created_at: %s\n", time.Now().UTC().Format(time.RFC3339)) emitted["wing"], emitted["hall"], emitted["created_at"] = true, true, true - fmt.Fprintf(&fm, "origin: %s\n", provenanceOrigin(opts.Origin)) - emitted["origin"], emitted["written_by"] = true, true - if opts.WrittenBy != "" { - fmt.Fprintf(&fm, "written_by: %s\n", opts.WrittenBy) - } // writeField merges one key: opts.Content's own value (if the note already // carries this field in its own frontmatter) always wins over the fallback, @@ -238,7 +233,8 @@ func writeHallNote(brainDir string, opts WriteNoteOptions) (string, error) { } fm.WriteString("---\n") - if err := os.WriteFile(dest, []byte(fm.String()+body), 0o644); err != nil { + final := StampProvenance(fm.String()+body, opts.Origin, opts.WrittenBy) + if err := os.WriteFile(dest, []byte(final), 0o644); err != nil { return "", fmt.Errorf("write: %w", err) } rel, _ := filepath.Rel(brainDir, dest) @@ -613,7 +609,11 @@ func (h *Handler) Promote(w http.ResponseWriter, r *http.Request) { writeError(w, http.StatusBadRequest, "invalid JSON") return } - rel, err := PromoteNote(h.brainDir, PromoteOptions(req)) + // Field by field: this door carries no principal, so the promoted note is + // stamped origin: unknown and the body cannot set provenance (#145). + rel, err := PromoteNote(h.brainDir, PromoteOptions{ + Filename: req.Filename, Wing: req.Wing, Hall: req.Hall, Slug: req.Slug, + }) if err != nil { writeError(w, http.StatusBadRequest, err.Error()) return diff --git a/ingestion/internal/api/promote.go b/ingestion/internal/api/promote.go index 8def9e0..006f1b8 100644 --- a/ingestion/internal/api/promote.go +++ b/ingestion/internal/api/promote.go @@ -72,6 +72,10 @@ type PromoteOptions struct { Wing string Hall string Slug string // optional; defaults to Filename minus date prefix + .md + // Origin/WrittenBy re-attribute the promoted note to the promoter: the raw + // note's own frontmatter claims are not provenance (hyperguild#145). + Origin string + WrittenBy string } // PromoteNote moves a note from brain/raw/ into the structured wiki: it @@ -128,7 +132,8 @@ func PromoteNote(brainDir string, opts PromoteOptions) (string, error) { return "", fmt.Errorf("create wing dir: %w", err) } // Write-then-delete: the source survives any write failure. - if err := os.WriteFile(dest, []byte(fm.render()+body), 0o644); err != nil { + promoted := StampProvenance(fm.render()+body, opts.Origin, opts.WrittenBy) + if err := os.WriteFile(dest, []byte(promoted), 0o644); err != nil { return "", fmt.Errorf("write promoted note: %w", err) } if err := os.Remove(src); err != nil { diff --git a/ingestion/internal/api/provenance.go b/ingestion/internal/api/provenance.go new file mode 100644 index 0000000..56b92a5 --- /dev/null +++ b/ingestion/internal/api/provenance.go @@ -0,0 +1,172 @@ +package api + +import ( + "fmt" + "os" + "path/filepath" + "regexp" + "strings" + + "gopkg.in/yaml.v3" + + sharedfm "github.com/mathiasbq/hyperguild/ingestion/internal/frontmatter" +) + +// Provenance (hyperguild#145) is server-owned: origin and written_by say +// which authenticated principal last wrote a note, so a reader can discount +// what a connector session wrote after reading an untrusted page. +// +// The control is a post-condition, not a set of per-door rules: every write +// path a model can reach ends with StampProvenance, which removes EVERY +// spelling of the two keys from the frontmatter and appends the real values. +// Deciding what counts as a spelling is delegated to yaml.v3 itself — each +// frontmatter line is decoded the way a YAML reader would decode it — so +// quoted, escaped, tagged, flow-mapped and merged keys are all recognised +// (PR #147 review: each of those survived a key-name string match). + +var plainScalar = regexp.MustCompile(`^[A-Za-z0-9][A-Za-z0-9._@-]*$`) + +// provenanceKeys are compared after yaml decoding and lower-casing. +var provenanceKeys = map[string]bool{"origin": true, "written_by": true} + +// StampProvenance returns content with all provenance spellings removed from +// its frontmatter and origin (empty → "unknown") and written_by (omitted when +// empty) appended. Content without frontmatter gains a block. +func StampProvenance(content, origin, writtenBy string) string { + content = normaliseFenceNewlines(content) + stamp := "origin: " + scalar(provenanceOrigin(origin)) + "\n" + if writtenBy != "" { + stamp += "written_by: " + scalar(writtenBy) + "\n" + } + block, rest, ok := sharedfm.Split([]byte(content)) + if !ok { + return "---\n" + stamp + "---\n" + content + } + // block is the frontmatter lines without the opening fence, ending in the + // newline before the closing fence; rest starts with the newline after it. + var kept []string + if len(block) > 0 { + for _, l := range strings.Split(strings.TrimSuffix(string(block), "\n"), "\n") { + if !isProvenanceLine(l) { + kept = append(kept, l) + } + } + } + return "---\n" + joinLines(kept) + stamp + "---\n" + strings.TrimPrefix(string(rest), "\n") +} + +// StampProvenanceFile applies StampProvenance to a note on disk. +func StampProvenanceFile(brainDir, relPath, origin, writtenBy string) error { + _, abs, err := resolveWithin(brainDir, relPath) + if err != nil { + return err + } + b, err := os.ReadFile(abs) + if err != nil { + return fmt.Errorf("read for provenance: %w", err) + } + out := StampProvenance(string(b), origin, writtenBy) + if out == string(b) { + return nil + } + if err := os.WriteFile(abs, []byte(out), 0o644); err != nil { + return fmt.Errorf("write provenance %s: %w", filepath.ToSlash(relPath), err) + } + return nil +} + +// isProvenanceLine reports whether a frontmatter line could, to any YAML +// reader, set origin or written_by. It errs towards dropping: a line it drops +// wrongly loses attacker-shaped metadata; a line it keeps wrongly is a forgery. +func isProvenanceLine(line string) bool { + t := strings.TrimSpace(line) + switch { + case t == "": + return false + case strings.HasPrefix(t, "?"), strings.HasPrefix(t, ":"), strings.HasPrefix(t, "<<"): + // Complex keys, their orphaned values, and merge keys: none appear in + // frontmatter the brain writes, and each can introduce a key a + // line-oriented check cannot see. + return true + } + // Two readers, two verdicts, and a line is dropped if EITHER would see + // provenance: yaml.v3 decodes quoting, escapes, tags and flow maps, while + // the brain's line-oriented readers split on the first colon — to them + // "origin:static" is a key, to YAML it is a plain scalar. + return yamlSeesProvenance(t) || textSeesProvenance(t) +} + +func yamlSeesProvenance(t string) bool { + var v any + if err := yaml.Unmarshal([]byte(t), &v); err != nil { + return false + } + switch m := v.(type) { + case map[string]any: + for k := range m { + if provenanceKeys[strings.ToLower(strings.TrimSpace(k))] { + return true + } + } + case map[any]any: + for k := range m { + if provenanceKeys[strings.ToLower(strings.TrimSpace(fmt.Sprint(k)))] { + return true + } + } + } + return false +} + +// textSeesProvenance takes the text before the first colon, drops YAML tags, +// brackets and quotes, and compares case-insensitively. +func textSeesProvenance(t string) bool { + key, _, found := strings.Cut(t, ":") + if !found { + return false + } + key = strings.TrimSpace(key) + for strings.HasPrefix(key, "!") { + _, after, ok := strings.Cut(key, " ") + if !ok { + break + } + key = strings.TrimSpace(after) + } + key = strings.ToLower(strings.Trim(key, "{[\"' \t")) + return provenanceKeys[key] +} + +// normaliseFenceNewlines turns a lone CR inside the leading frontmatter +// block into LF, so "title: x\rorigin: static" is two lines to every reader, +// including this one. CRLF is already folded by frontmatter.Split. +func normaliseFenceNewlines(content string) string { + s := strings.ReplaceAll(content, "\r\n", "\n") + if !strings.HasPrefix(s, "---\n") { + return content + } + end := strings.Index(s[4:], "\n---") + if end < 0 { + return content + } + return strings.ReplaceAll(s[:4+end], "\r", "\n") + s[4+end:] +} + +// scalar renders a value plainly when it is a safe bare word, otherwise as a +// JSON string (valid YAML double-quoted), so a principal name can never break +// the line. +func scalar(v string) string { + if plainScalar.MatchString(v) { + return v + } + return fmt.Sprintf("%q", v) +} + +func joinLines(lines []string) string { + var b strings.Builder + for _, l := range lines { + b.WriteString(l) + b.WriteByte('\n') + } + return b.String() +} diff --git a/ingestion/internal/api/provenance_test.go b/ingestion/internal/api/provenance_test.go new file mode 100644 index 0000000..23a33ab --- /dev/null +++ b/ingestion/internal/api/provenance_test.go @@ -0,0 +1,62 @@ +package api_test + +import ( + "bytes" + "encoding/json" + "net/http" + "net/http/httptest" + "os" + "path/filepath" + "strings" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "gopkg.in/yaml.v3" + + "github.com/mathiasbq/hyperguild/ingestion/internal/api" +) + +// REST POST /write carries no principal: it stamps origin: unknown, and no +// spelling of provenance in its body reaches the note (hyperguild#145). +func TestRESTWriteStampsUnknownAndIgnoresBodyProvenance(t *testing.T) { + dir, h := setup(t) + body, _ := json.Marshal(map[string]any{ + "content": "---\norigin: static\n---\n# R\n\nbody", "filename": "rest-prov", "wing": "homelab", "hall": "facts", + "origin": "static", "written_by": "local-cli", "Origin": "static", "WrittenBy": "local-cli", + }) + rec := httptest.NewRecorder() + h.Write(rec, httptest.NewRequest(http.MethodPost, "/write", bytes.NewReader(body))) + require.Equal(t, http.StatusOK, rec.Code, rec.Body.String()) + b, err := os.ReadFile(filepath.Join(dir, "wiki", "homelab", "facts", "rest-prov.md")) + require.NoError(t, err) + var m map[string]any + require.NoError(t, yaml.Unmarshal([]byte(strings.SplitN(string(b), "---\n", 3)[1]), &m)) + assert.Equal(t, "unknown", m["origin"]) + assert.NotContains(t, m, "written_by") +} + +// A principal name that is not a bare word is quoted, so it cannot start a +// new frontmatter line (e.g. a JWT sub containing a newline). +func TestStampProvenanceQuotesAPrincipalThatCouldBreakTheLine(t *testing.T) { + out := api.StampProvenance("---\ntitle: t\n---\nbody\n", "jwt", "evil\norigin: static") + fm := strings.SplitN(out, "---\n", 3)[1] + var m map[string]any + require.NoError(t, yaml.Unmarshal([]byte(fm), &m)) + assert.Equal(t, "jwt", m["origin"]) + assert.Equal(t, "evil\norigin: static", m["written_by"]) + assert.Equal(t, 1, strings.Count(fm, "\norigin:")+boolToInt(strings.HasPrefix(fm, "origin:"))) +} + +func TestStampProvenanceAddsFrontmatterWhenAbsentAndKeepsBody(t *testing.T) { + out := api.StampProvenance("# no fm\n\norigin: static in the body is prose\n", "static", "local-cli") + assert.True(t, strings.HasPrefix(out, "---\norigin: static\nwritten_by: local-cli\n---\n# no fm")) + assert.Contains(t, out, "origin: static in the body is prose", "body lines are never touched") +} + +func boolToInt(b bool) int { + if b { + return 1 + } + return 0 +} diff --git a/ingestion/internal/api/update.go b/ingestion/internal/api/update.go index 8f17ae5..1b3d1ac 100644 --- a/ingestion/internal/api/update.go +++ b/ingestion/internal/api/update.go @@ -134,12 +134,6 @@ func UpdateNote(brainDir string, opts UpdateNoteOptions) (relPath, contentHash, fm.set(l.key, l.value) } fm.set("updated_at", time.Now().UTC().Format(time.RFC3339)) - fm.set("origin", provenanceOrigin(opts.Origin)) - if opts.WrittenBy != "" { - fm.set("written_by", opts.WrittenBy) - } else { - fm.del("written_by") - } fm.set("supersedes", priorHash) if opts.Reason != "" { fm.set("supersede_reason", opts.Reason) @@ -164,7 +158,7 @@ func UpdateNote(brainDir string, opts UpdateNoteOptions) (relPath, contentHash, return "", "", "", fmt.Errorf("claim envelope: %w", err) } - out := []byte(fm.render() + contentBody) + out := []byte(StampProvenance(fm.render()+contentBody, opts.Origin, opts.WrittenBy)) if err := os.WriteFile(abs, out, 0o644); err != nil { return "", "", "", fmt.Errorf("write: %w", err) } diff --git a/ingestion/internal/capturehttp/provenance_test.go b/ingestion/internal/capturehttp/provenance_test.go new file mode 100644 index 0000000..b03eb53 --- /dev/null +++ b/ingestion/internal/capturehttp/provenance_test.go @@ -0,0 +1,51 @@ +package capturehttp_test + +import ( + "os" + "path/filepath" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "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" +) + +// The REST capture door derives AuthPath itself (hyperguild#145); a body +// field cannot set it, and static vs JWT are told apart. +func TestRESTCaptureStampsAuthPathFromAuthentication(t *testing.T) { + for _, tc := range []struct { + name, authz, wantOrigin, wantBy string + v capturehttp.Validator + sovereign []string + }{ + {"static", "Bearer " + staticTok, "origin: static\n", "written_by: local-cli\n", nil, nil}, + {"jwt", "Bearer jwt", "origin: jwt\n", "written_by: koala-cli\n", fakeValidator{subject: "koala-cli"}, []string{"koala-cli"}}, + } { + t.Run(tc.name, func(t *testing.T) { + brainDir := t.TempDir() + cfg, err := classification.Load(t.TempDir()) + require.NoError(t, err) + svc := capture.NewService(brainstore.New(brainDir), fakeTracker{}, cfg, audit.NewSlogSink(nil)) + h := capturehttp.New(svc, tc.v, staticTok, "local-cli", capturehttp.NewOriginResolver(tc.sovereign)) + rr := do(t, h, tc.authz, map[string]any{ + "context": map[string]any{ + "harness": "claude-code", "actor": "mathias", "classification": "internal", + "AuthPath": "static", "auth_path": "static", "principal": "local-cli", + }, + "insights": []map[string]any{{"text": "rest provenance probe", "wing": "hyperguild", "hall": "facts"}}, + }) + require.Equal(t, 200, rr.Code, rr.Body.String()) + matches, _ := filepath.Glob(filepath.Join(brainDir, "wiki/hyperguild/facts/*.md")) + require.Len(t, matches, 1) + b, err := os.ReadFile(matches[0]) + require.NoError(t, err) + assert.Contains(t, string(b), tc.wantOrigin) + assert.Contains(t, string(b), tc.wantBy) + }) + } +} diff --git a/ingestion/internal/mcp/audit.go b/ingestion/internal/mcp/audit.go index 4d0540d..ea95680 100644 --- a/ingestion/internal/mcp/audit.go +++ b/ingestion/internal/mcp/audit.go @@ -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...) diff --git a/ingestion/internal/mcp/audit_test.go b/ingestion/internal/mcp/audit_test.go index 4e0f6b4..d374853 100644 --- a/ingestion/internal/mcp/audit_test.go +++ b/ingestion/internal/mcp/audit_test.go @@ -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. diff --git a/ingestion/internal/mcp/handlers.go b/ingestion/internal/mcp/handlers.go index c4c634c..4e40ec8 100644 --- a/ingestion/internal/mcp/handlers.go +++ b/ingestion/internal/mcp/handlers.go @@ -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 +} diff --git a/ingestion/internal/mcp/provenance_escapes_test.go b/ingestion/internal/mcp/provenance_escapes_test.go new file mode 100644 index 0000000..bcb4764 --- /dev/null +++ b/ingestion/internal/mcp/provenance_escapes_test.go @@ -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") +} diff --git a/ingestion/internal/mcp/provenance_test.go b/ingestion/internal/mcp/provenance_test.go index ad0c135..5f2571d 100644 --- a/ingestion/internal/mcp/provenance_test.go +++ b/ingestion/internal/mcp/provenance_test.go @@ -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"}, diff --git a/ingestion/internal/mcp/tools_notes.go b/ingestion/internal/mcp/tools_notes.go index 36ce113..f1f26ec 100644 --- a/ingestion/internal/mcp/tools_notes.go +++ b/ingestion/internal/mcp/tools_notes.go @@ -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 diff --git a/ingestion/internal/mcp/tools_notes_test.go b/ingestion/internal/mcp/tools_notes_test.go index 37dbf79..55842ef 100644 --- a/ingestion/internal/mcp/tools_notes_test.go +++ b/ingestion/internal/mcp/tools_notes_test.go @@ -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) { diff --git a/ingestion/internal/pipeline/build.go b/ingestion/internal/pipeline/build.go index 4ff005a..ef09d82 100644 --- a/ingestion/internal/pipeline/build.go +++ b/ingestion/internal/pipeline/build.go @@ -3,6 +3,7 @@ package pipeline import ( "fmt" + "regexp" "strings" "github.com/mathiasbq/hyperguild/ingestion/internal/wiki" @@ -16,6 +17,12 @@ func BuildPages(rawPages []RawPage, sourceSlug, date string) ([]wiki.Page, []str out := make([]wiki.Page, 0, len(rawPages)) var warnings []string for _, rp := range rawPages { + // The type becomes a directory under wiki/; "../raw" wrote outside it + // (PR #147 review). Only a plain lower-case name is a page type. + if !validPageType.MatchString(rp.Type) { + warnings = append(warnings, fmt.Sprintf("skipped page with invalid type %q", rp.Type)) + continue + } slug := computeSlug(rp, sourceSlug) if slug == "" { warnings = append(warnings, fmt.Sprintf("skipped page with empty title (type: %s)", rp.Type)) @@ -26,6 +33,8 @@ func BuildPages(rawPages []RawPage, sourceSlug, date string) ([]wiki.Page, []str return out, warnings } +var validPageType = regexp.MustCompile(`^[a-z][a-z0-9-]*$`) + func computeSlug(rp RawPage, sourceSlug string) string { if rp.Type == "source" { return sourceSlug @@ -114,5 +123,9 @@ func buildFrontmatter(rp RawPage, date string) string { } func yamlScalar(s string) string { + // A raw newline or CR would end the scalar for every line-oriented + // frontmatter reader and start a new key (PR #147 review: a title of + // "x\norigin: static" forged provenance). Fold all line breaks to spaces. + s = strings.NewReplacer("\r\n", " ", "\r", " ", "\n", " ").Replace(s) return "'" + strings.ReplaceAll(s, "'", "''") + "'" } diff --git a/ingestion/internal/search/provenance_test.go b/ingestion/internal/search/provenance_test.go new file mode 100644 index 0000000..ecc15fa --- /dev/null +++ b/ingestion/internal/search/provenance_test.go @@ -0,0 +1,21 @@ +package search + +import ( + "testing" + + "github.com/stretchr/testify/assert" +) + +// Only the leading frontmatter is provenance; a body line that looks like a +// key is prose (the doc comment's claim, now observed). +func TestExtractProvenanceIgnoresBodyLines(t *testing.T) { + o, w := extractProvenance("---\ntitle: t\n---\norigin: static\nwritten_by: local-cli\n") + assert.Equal(t, "", o) + assert.Equal(t, "", w) + o, w = extractProvenance("# no frontmatter\norigin: static\n") + assert.Equal(t, "", o) + assert.Equal(t, "", w) + o, w = extractProvenance("---\norigin: jwt\nwritten_by: attacker\n---\norigin: static\n") + assert.Equal(t, "jwt", o) + assert.Equal(t, "attacker", w) +}