hyperguild#115: a session reported "old_str is empty" with old_str that was never empty on their end. Server-side round-trip re-verified clean (fresh brain_edit call, real content, succeeds) — the code path itself isn't broken, but its error left no way to tell "client sent empty" apart from "something between the client and here lost the field" without a second investigation. The refusal now names the raw argument byte count. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
321 lines
12 KiB
Go
321 lines
12 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_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)
|
|
}
|