hyperguild#123, the audit follow-up from #111: writeHallNote and UpdateNote both validate the claim envelope (half_life/applies/ claim_provenance) before writing; brain_edit didn't -- a byte-level string replace has no frontmatter awareness at all, so a caller could rewrite an existing envelope line (e.g. flip claim_provenance from asserted to guessed, or set half_life to a value outside the enum) straight past the validator. This was the one live, model-facing gap #123 identified; the ingestion pipeline write paths audited alongside it turned out not to touch these fields at all, so this closes the issue. Fixed by previewing what old_str->new_str would produce -- the exact strings.Replace(before, old_str, new_str, 1) formula notes.Edit itself uses internally -- and validating the envelope on that preview before the real edit is allowed to run. Fail-closed: an invalid result blocks the edit entirely, matching create/supersede's existing posture. Scoped to notes only (scope == notes.ScopeNotes) since governance files have no claim-envelope concept, and gated on a cheap substring check so an ordinary body edit that never mentions an envelope field name pays nothing extra. The preview doesn't replicate notes.Edit's uniqueness/CRLF/conflict- marker checks -- if old_str is absent or ambiguous, the preview may not match what Edit ultimately does, but Edit independently and correctly rejects those cases at write time regardless, so nothing about write safety rides on this preview's accuracy, only the envelope check itself. frontmatterScalars is a third, minimal implementation of the same indented-line-safe scalar extraction as the sibling packages (hyperguild#126/#128) -- deliberately not shared/exported across packages (each package's frontmatter handling has slightly different needs and none currently import each other), but built the same way from the start this time rather than needing a follow-up fix. Flip-tested: disabling the pre-check gate lets the invalid-envelope test fail (the edit silently succeeds); restoring it passes. Full module suite green. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
393 lines
15 KiB
Go
393 lines
15 KiB
Go
// ingestion/internal/mcp/tools_notes_test.go
|
|
package mcp_test
|
|
|
|
import (
|
|
"context"
|
|
"crypto/sha256"
|
|
"encoding/hex"
|
|
"encoding/json"
|
|
"net/http"
|
|
"net/http/httptest"
|
|
"os"
|
|
"path/filepath"
|
|
"strings"
|
|
"testing"
|
|
|
|
"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"
|
|
"github.com/stretchr/testify/assert"
|
|
"github.com/stretchr/testify/require"
|
|
)
|
|
|
|
// ---- brain#4: brain_read / brain_edit through the JSON-RPC surface --------
|
|
|
|
func newBrainRepo(t *testing.T) string {
|
|
t.Helper()
|
|
dir := t.TempDir()
|
|
require.NoError(t, os.MkdirAll(filepath.Join(dir, ".git"), 0o755))
|
|
require.NoError(t, os.MkdirAll(filepath.Join(dir, "wiki/homelab/facts"), 0o755))
|
|
require.NoError(t, os.WriteFile(filepath.Join(dir, "wiki/homelab/facts/n.md"),
|
|
[]byte("---\ntitle: 'n'\n---\n\nalpha\nbeta\n"), 0o644))
|
|
require.NoError(t, os.WriteFile(filepath.Join(dir, "CLAUDE.md"), []byte("# rules\n"), 0o644))
|
|
return dir
|
|
}
|
|
|
|
func call(t *testing.T, srv *mcp.Server, name string, args map[string]any) map[string]any {
|
|
t.Helper()
|
|
req := httptest.NewRequest(http.MethodPost, "/mcp", body(t, map[string]any{
|
|
"jsonrpc": "2.0", "id": 1, "method": "tools/call",
|
|
"params": map[string]any{"name": name, "arguments": args},
|
|
}))
|
|
rr := httptest.NewRecorder()
|
|
srv.ServeHTTP(rr, req)
|
|
var resp map[string]any
|
|
require.NoError(t, json.Unmarshal(rr.Body.Bytes(), &resp))
|
|
return resp
|
|
}
|
|
|
|
func TestBrainReadAndEdit_RoundTripThroughJSONRPC(t *testing.T) {
|
|
// The Go functions are tested in internal/notes; this pins the transport,
|
|
// because a tool that works in-process and is unreachable over JSON-RPC
|
|
// is indistinguishable from a working one in a unit test.
|
|
dir := newBrainRepo(t)
|
|
srv := mcp.NewServer(dir, nil, nil, nil)
|
|
|
|
resp := call(t, srv, "brain_read", map[string]any{"path": "wiki/homelab/facts/n.md"})
|
|
require.NotContains(t, resp, "error", "%v", resp["error"])
|
|
|
|
resp = call(t, srv, "brain_edit", map[string]any{
|
|
"path": "wiki/homelab/facts/n.md", "old_str": "beta", "new_str": "BETA",
|
|
})
|
|
require.NotContains(t, resp, "error", "%v", resp["error"])
|
|
|
|
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")
|
|
}
|
|
|
|
func TestBrainEdit_EmptyOldStrErrorNamesRawArgumentBytes(t *testing.T) {
|
|
// hyperguild#115: a session reported "old_str is empty" with an old_str
|
|
// that, on their end, was never empty. The server-side path was
|
|
// unreachable to reproduce (verified separately: a fresh brain_edit call
|
|
// with real content round-trips cleanly), which points at something
|
|
// between a specific client and this call losing the field. The fix this
|
|
// issue actually asked for isn't a behavior change — old_str=="" must
|
|
// still refuse — it's that the refusal should say how many raw argument
|
|
// bytes it received, so "client sent empty" is distinguishable from
|
|
// "server received empty after some hop lost it" without a second
|
|
// session's worth of guessing.
|
|
dir := newBrainRepo(t)
|
|
srv := mcp.NewServer(dir, nil, nil, nil)
|
|
|
|
resp := call(t, srv, "brain_edit", map[string]any{
|
|
"path": "wiki/homelab/facts/n.md", "old_str": "", "new_str": "BETA",
|
|
})
|
|
errMsg, ok := resp["error"].(map[string]any)["message"].(string)
|
|
require.True(t, ok, "%v", resp)
|
|
assert.Contains(t, errMsg, "old_str is empty")
|
|
assert.Contains(t, errMsg, "raw arguments were", "%v", errMsg)
|
|
}
|
|
|
|
func TestBrainEdit_RejectsAnInvalidClaimEnvelopeWrittenViaStringReplace(t *testing.T) {
|
|
// hyperguild#123: brain_write/brain_update both validate the claim
|
|
// envelope (half_life/applies/claim_provenance) before writing —
|
|
// brain_edit didn't, even though it's the one live, model-facing path
|
|
// that can rewrite an existing envelope line directly via raw string
|
|
// replace, no parsing involved. "same rules when a note changes as when
|
|
// it's born" (hyperguild#111's own framing) applies here too.
|
|
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": "claim_provenance: asserted", "new_str": "claim_provenance: guessed",
|
|
})
|
|
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) {
|
|
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: seasonal",
|
|
})
|
|
require.NotContains(t, resp, "error", "%v", resp)
|
|
|
|
got, err := os.ReadFile(note)
|
|
require.NoError(t, err)
|
|
assert.Contains(t, string(got), "half_life: seasonal")
|
|
}
|
|
|
|
func TestBrainEdit_UnrelatedEditsSkipTheEnvelopeCheckEntirely(t *testing.T) {
|
|
// The gate only engages when old_str/new_str actually mention an
|
|
// envelope field name — an ordinary body edit on a note that happens to
|
|
// carry a valid envelope must not pay for (or be affected by) a check
|
|
// that has nothing to do with it.
|
|
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\nold body\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": "old body", "new_str": "new body",
|
|
})
|
|
require.NotContains(t, resp, "error", "%v", resp)
|
|
|
|
got, err := os.ReadFile(note)
|
|
require.NoError(t, err)
|
|
assert.Contains(t, string(got), "new body")
|
|
}
|
|
|
|
func TestBrainEditRoot_GovernanceFilesAreNeverEnvelopeChecked(t *testing.T) {
|
|
// CLAUDE.md has no claim-envelope concept at all — the check must be
|
|
// scoped to notes (ScopeNotes), never applied on the governance path.
|
|
srv, _, _, _ := govServer(t)
|
|
|
|
resp := authCall(t, srv, "Bearer "+capStaticTok, "brain_edit_root", map[string]any{
|
|
"path": "CLAUDE.md", "old_str": "# rules", "new_str": "half_life: durable",
|
|
})
|
|
require.NotContains(t, resp, "error", "%v", resp)
|
|
}
|
|
|
|
func TestBrainEdit_RefusesGovernanceFilesFromTheModelFacingTool(t *testing.T) {
|
|
dir := newBrainRepo(t)
|
|
srv := mcp.NewServer(dir, nil, nil, nil)
|
|
|
|
resp := call(t, srv, "brain_edit", map[string]any{
|
|
"path": "CLAUDE.md", "old_str": "# rules", "new_str": "# rewritten",
|
|
})
|
|
require.Contains(t, resp, "error", "brain_edit must not reach CLAUDE.md")
|
|
|
|
got, err := os.ReadFile(filepath.Join(dir, "CLAUDE.md"))
|
|
require.NoError(t, err)
|
|
assert.Equal(t, "# rules\n", string(got))
|
|
}
|
|
|
|
func TestBrainEditRoot_RefusedWithoutTheLocalStaticPrincipal(t *testing.T) {
|
|
// No principal in context is the model-facing case (and the default in
|
|
// this test server): the privileged door must stay shut.
|
|
dir := newBrainRepo(t)
|
|
srv := mcp.NewServer(dir, nil, nil, nil)
|
|
|
|
resp := call(t, srv, "brain_edit_root", map[string]any{
|
|
"path": "CLAUDE.md", "old_str": "# rules", "new_str": "# rewritten",
|
|
})
|
|
require.Contains(t, resp, "error")
|
|
|
|
got, err := os.ReadFile(filepath.Join(dir, "CLAUDE.md"))
|
|
require.NoError(t, err)
|
|
assert.Equal(t, "# rules\n", string(got), "governance file must be untouched")
|
|
}
|
|
|
|
func TestBrainGet_CannotReachSessionTranscripts(t *testing.T) {
|
|
// Pre-existing hole, not introduced here: api.ReadNote resolves any
|
|
// brain-relative path and parseFrontmatter returns the WHOLE file as body
|
|
// when there is no frontmatter fence. archive/ holds verbatim Claude
|
|
// session transcripts (up to 500 KB) that BM25 never indexes, so they are
|
|
// invisible to search and trivially readable by path — and CLAUDE.md
|
|
// documents the layout.
|
|
dir := newBrainRepo(t)
|
|
require.NoError(t, os.MkdirAll(filepath.Join(dir, "archive/claude-sessions/koala"), 0o755))
|
|
require.NoError(t, os.WriteFile(
|
|
filepath.Join(dir, "archive/claude-sessions/koala/session-1.md"),
|
|
[]byte("verbatim transcript with tool output\n"), 0o644))
|
|
|
|
srv := mcp.NewServer(dir, nil, nil, nil)
|
|
resp := call(t, srv, "brain_get", map[string]any{"path": "archive/claude-sessions/koala/session-1.md"})
|
|
|
|
require.Contains(t, resp, "error", "brain_get must not read outside knowledge/ and wiki/")
|
|
assert.NotContains(t, string(mustJSON(t, resp)), "verbatim transcript")
|
|
}
|
|
|
|
func TestBrainGet_StillReadsNotes(t *testing.T) {
|
|
dir := newBrainRepo(t)
|
|
srv := mcp.NewServer(dir, nil, nil, nil)
|
|
|
|
resp := call(t, srv, "brain_get", map[string]any{"path": "wiki/homelab/facts/n.md"})
|
|
assert.NotContains(t, resp, "error", "%v", resp["error"])
|
|
}
|
|
|
|
func mustJSON(t *testing.T, v any) []byte {
|
|
t.Helper()
|
|
b, err := json.Marshal(v)
|
|
require.NoError(t, err)
|
|
return b
|
|
}
|
|
|
|
// ---- hyperguild#97: a governance rewrite must be observed, not just gated ----
|
|
|
|
// govSink captures audit records. Governance edits are rare, so the whole
|
|
// history is kept and asserted on rather than sampled.
|
|
type govSink struct {
|
|
entries []capture.AuditEntry
|
|
outcomes []capture.AuditOutcome
|
|
recErr error
|
|
}
|
|
|
|
func (s *govSink) Reserve(context.Context, classification.Level) (capture.AuditOutcome, error) {
|
|
return capture.AuditCentral, nil
|
|
}
|
|
|
|
func (s *govSink) Record(_ context.Context, e capture.AuditEntry, o capture.AuditOutcome) error {
|
|
s.entries = append(s.entries, e)
|
|
s.outcomes = append(s.outcomes, o)
|
|
return s.recErr
|
|
}
|
|
|
|
type govNotifier struct {
|
|
msgs []string
|
|
err error
|
|
}
|
|
|
|
func (n *govNotifier) Notify(_ context.Context, msg string) error {
|
|
n.msgs = append(n.msgs, msg)
|
|
return n.err
|
|
}
|
|
|
|
// govServer wires capture (which is what re-derives the static principal) plus
|
|
// the #97 observer, and returns the brain dir so the test can read the file.
|
|
func govServer(t *testing.T) (*mcp.Server, string, *govSink, *govNotifier) {
|
|
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, nil, capStaticTok, "local-cli", capturehttp.NewOriginResolver(nil))
|
|
sink, notifier := &govSink{}, &govNotifier{}
|
|
srv.WithGovernanceObserver(sink, notifier)
|
|
return srv, dir, sink, notifier
|
|
}
|
|
|
|
// authCall is call() with an Authorization header, so the static principal is
|
|
// present and brain_edit_root's gate opens.
|
|
func authCall(t *testing.T, srv *mcp.Server, authz, name string, args map[string]any) map[string]any {
|
|
t.Helper()
|
|
req := httptest.NewRequest(http.MethodPost, "/mcp", body(t, map[string]any{
|
|
"jsonrpc": "2.0", "id": 1, "method": "tools/call",
|
|
"params": map[string]any{"name": name, "arguments": args},
|
|
}))
|
|
if authz != "" {
|
|
req.Header.Set("Authorization", authz)
|
|
}
|
|
rr := httptest.NewRecorder()
|
|
srv.ServeHTTP(rr, req)
|
|
var resp map[string]any
|
|
require.NoError(t, json.Unmarshal(rr.Body.Bytes(), &resp))
|
|
return resp
|
|
}
|
|
|
|
func sha256hex(s string) string {
|
|
sum := sha256.Sum256([]byte(s))
|
|
return hex.EncodeToString(sum[:])
|
|
}
|
|
|
|
func TestBrainEditRoot_RecordsAnAuditEntryNamingPrincipalPathAndBothHashes(t *testing.T) {
|
|
// The gate proves a rewrite was ALLOWED. Only a record proves one HAPPENED
|
|
// — and brain-sync will commit it within 15 minutes authored `brain-sync`,
|
|
// so git history cannot tell it apart from routine note traffic.
|
|
srv, dir, sink, _ := govServer(t)
|
|
|
|
resp := authCall(t, srv, "Bearer "+capStaticTok, "brain_edit_root", map[string]any{
|
|
"path": "CLAUDE.md", "old_str": "# rules", "new_str": "# rewritten",
|
|
})
|
|
require.NotContains(t, resp, "error", "%v", resp["error"])
|
|
|
|
require.Len(t, sink.entries, 1, "exactly one audit record per governance rewrite")
|
|
e := sink.entries[0]
|
|
assert.Equal(t, "local-cli", e.Principal)
|
|
assert.Contains(t, e.Items, "CLAUDE.md", "the record must name what was rewritten")
|
|
|
|
joined := strings.Join(e.SecurityEvents, " ")
|
|
assert.Contains(t, joined, sha256hex("# rules\n"), "before-hash must be recorded")
|
|
assert.Contains(t, joined, sha256hex("# rewritten\n"), "after-hash must be recorded")
|
|
assert.NotZero(t, e.Timestamp)
|
|
|
|
got, err := os.ReadFile(filepath.Join(dir, "CLAUDE.md"))
|
|
require.NoError(t, err)
|
|
assert.Equal(t, "# rewritten\n", string(got))
|
|
}
|
|
|
|
func TestBrainEditRoot_NotifiesWithPathAndHashesButNeverContent(t *testing.T) {
|
|
srv, _, _, notifier := govServer(t)
|
|
|
|
resp := authCall(t, srv, "Bearer "+capStaticTok, "brain_edit_root", map[string]any{
|
|
"path": "CLAUDE.md", "old_str": "# rules", "new_str": "# rewritten",
|
|
})
|
|
require.NotContains(t, resp, "error", "%v", resp["error"])
|
|
|
|
require.Len(t, notifier.msgs, 1)
|
|
msg := notifier.msgs[0]
|
|
assert.Contains(t, msg, "CLAUDE.md")
|
|
assert.Contains(t, msg, "local-cli")
|
|
assert.Contains(t, msg, sha256hex("# rules\n"))
|
|
assert.Contains(t, msg, sha256hex("# rewritten\n"))
|
|
|
|
// The notification travels to a phone through a third-party topic. A hash
|
|
// identifies a rewrite; the bytes would leak whatever was written.
|
|
assert.NotContains(t, msg, "# rewritten", "no file content in the notification body")
|
|
assert.NotContains(t, msg, "# rules")
|
|
}
|
|
|
|
func TestBrainEditRoot_RefusedProducesNoRecordNoNotificationNoWrite(t *testing.T) {
|
|
srv, dir, sink, notifier := govServer(t)
|
|
|
|
// No Authorization header → no static principal → the gate stays shut.
|
|
resp := authCall(t, srv, "", "brain_edit_root", map[string]any{
|
|
"path": "CLAUDE.md", "old_str": "# rules", "new_str": "# rewritten",
|
|
})
|
|
require.Contains(t, resp, "error")
|
|
|
|
assert.Empty(t, sink.entries, "a refusal is not a rewrite — recording one would cry wolf")
|
|
assert.Empty(t, notifier.msgs)
|
|
got, err := os.ReadFile(filepath.Join(dir, "CLAUDE.md"))
|
|
require.NoError(t, err)
|
|
assert.Equal(t, "# rules\n", string(got))
|
|
}
|
|
|
|
func TestBrainEditRoot_FailedEditProducesNoRecordNoNotification(t *testing.T) {
|
|
// Authorised but no match: the door opened and nothing went through it.
|
|
srv, _, sink, notifier := govServer(t)
|
|
|
|
resp := authCall(t, srv, "Bearer "+capStaticTok, "brain_edit_root", map[string]any{
|
|
"path": "CLAUDE.md", "old_str": "absent-from-the-file", "new_str": "x",
|
|
})
|
|
require.Contains(t, resp, "error")
|
|
|
|
assert.Empty(t, sink.entries)
|
|
assert.Empty(t, notifier.msgs)
|
|
}
|
|
|
|
func TestBrainEdit_OnOrdinaryNotesIsNotAudited(t *testing.T) {
|
|
// The decided asymmetry (#97): note edits are covered by git history, which
|
|
// carries the same before/after bytes with a diff. Governance files get the
|
|
// extra trail because they change what later agents obey, not because they
|
|
// are harder to reconstruct.
|
|
srv, _, sink, notifier := govServer(t)
|
|
|
|
resp := authCall(t, srv, "Bearer "+capStaticTok, "brain_edit", map[string]any{
|
|
"path": "wiki/homelab/facts/n.md", "old_str": "beta", "new_str": "BETA",
|
|
})
|
|
require.NotContains(t, resp, "error", "%v", resp["error"])
|
|
|
|
assert.Empty(t, sink.entries, "note edits must not fill the governance audit trail")
|
|
assert.Empty(t, notifier.msgs)
|
|
}
|