diff --git a/internal/mcp/audit.go b/internal/mcp/audit.go new file mode 100644 index 0000000..80678dc --- /dev/null +++ b/internal/mcp/audit.go @@ -0,0 +1,46 @@ +package mcp + +import ( + "context" + "encoding/json" + "log/slog" + "time" + + chassisauth "git.d-ma.be/mathias/mcp-chassis/auth" + + "git.d-ma.be/mathias/gitea-mcp/internal/auth" + "git.d-ma.be/mathias/gitea-mcp/internal/gitea" +) + +// auditTargetKeys are the only argument values copied into the audit record: +// they identify what a call acted on without copying what it wrote. Every +// other argument (file content, issue bodies) is reduced to args_bytes so the +// audit trail never becomes a second copy of repo content (gitea-mcp#64). +var auditTargetKeys = []string{"owner", "repo", "number", "branch", "path", "tag"} + +// logToolCall emits one structured record per tools/call (OWASP MCP08). +func logToolCall(ctx context.Context, name string, args json.RawMessage, start time.Time, err error) { + attrs := []any{ + "tool", name, + "caller", auth.Caller(ctx), + "args_bytes", len(args), + "duration_ms", time.Since(start).Milliseconds(), + } + if tok, ok := gitea.TokenFromContext(ctx); ok { + attrs = append(attrs, "token_fp", chassisauth.Fingerprint(tok)) + } + var parsed map[string]any + if json.Unmarshal(args, &parsed) == nil { + for _, k := range auditTargetKeys { + if v, ok := parsed[k]; ok { + attrs = append(attrs, k, v) + } + } + } + outcome := "ok" + if err != nil { + outcome = err.Error() + } + attrs = append(attrs, "outcome", outcome) + slog.InfoContext(ctx, "mcp tool call", attrs...) +} diff --git a/internal/mcp/audit_test.go b/internal/mcp/audit_test.go new file mode 100644 index 0000000..313c01d --- /dev/null +++ b/internal/mcp/audit_test.go @@ -0,0 +1,123 @@ +package mcp_test + +import ( + "bytes" + "context" + "encoding/json" + "errors" + "log/slog" + "net/http" + "net/http/httptest" + "strings" + "testing" + + "git.d-ma.be/mathias/gitea-mcp/internal/auth" + "git.d-ma.be/mathias/gitea-mcp/internal/mcp" + "git.d-ma.be/mathias/gitea-mcp/internal/registry" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +type auditStubTool struct { + name string + err error +} + +func (s auditStubTool) Descriptor() registry.ToolDescriptor { + return registry.ToolDescriptor{Name: s.name, InputSchema: json.RawMessage(`{"type":"object"}`)} +} + +func (s auditStubTool) Call(context.Context, json.RawMessage) (json.RawMessage, error) { + if s.err != nil { + return nil, s.err + } + return json.RawMessage(`{"ok":true}`), nil +} + +// captureAudit routes slog to a buffer and returns the decoded +// "mcp tool call" records emitted while fn runs. +func captureAudit(t *testing.T, fn func()) []map[string]any { + t.Helper() + var buf bytes.Buffer + prev := slog.Default() + slog.SetDefault(slog.New(slog.NewJSONHandler(&buf, nil))) + t.Cleanup(func() { slog.SetDefault(prev) }) + + fn() + + var out []map[string]any + for _, l := range strings.Split(strings.TrimSpace(buf.String()), "\n") { + if l == "" { + continue + } + var rec map[string]any + require.NoError(t, json.Unmarshal([]byte(l), &rec)) + if rec["msg"] == "mcp tool call" { + out = append(out, rec) + } + } + return out +} + +func auditServer(tools ...registry.Tool) http.Handler { + reg := registry.New() + for _, tl := range tools { + reg.Register(tl) + } + srv := mcp.NewServer(mcp.ServerOptions{Registry: reg}) + return auth.CallerMiddleware(slog.Default(), srv) +} + +func callTool(t *testing.T, h http.Handler, name string, args any) { + t.Helper() + body := map[string]any{ + "jsonrpc": "2.0", "id": 1, "method": "tools/call", + "params": map[string]any{"name": name, "arguments": args}, + } + b, err := json.Marshal(body) + require.NoError(t, err) + req := httptest.NewRequest(http.MethodPost, "/", bytes.NewBuffer(b)) + req.Header.Set("Content-Type", "application/json") + req.Header.Set("X-Auth-Request-User", "alice") + h.ServeHTTP(httptest.NewRecorder(), req) +} + +// gitea-mcp#64: tools/call dispatched with no record at all. +func TestToolsCallLogsOneAuditRecordWithToolCallerAndTarget(t *testing.T) { + recs := captureAudit(t, func() { + callTool(t, auditServer(auditStubTool{name: "repo_delete"}), "repo_delete", + map[string]any{"owner": "mathias", "repo": "scratch", "confirm": "scratch"}) + }) + require.Len(t, recs, 1) + r := recs[0] + assert.Equal(t, "repo_delete", r["tool"]) + assert.Equal(t, "alice", r["caller"]) + assert.Equal(t, "mathias", r["owner"]) + assert.Equal(t, "scratch", r["repo"]) + assert.Equal(t, "ok", r["outcome"]) + assert.Contains(t, r, "duration_ms") +} + +func TestToolsCallAuditRecordsErrorOutcome(t *testing.T) { + recs := captureAudit(t, func() { + callTool(t, auditServer(auditStubTool{name: "pr_merge", err: errors.New("boom")}), + "pr_merge", map[string]any{"owner": "mathias", "repo": "x", "number": 3}) + }) + require.Len(t, recs, 1) + assert.Equal(t, "boom", recs[0]["outcome"]) + assert.EqualValues(t, 3, recs[0]["number"]) +} + +// The audit line must not become a second copy of repo content. +func TestToolsCallAuditOmitsArgumentValuesOtherThanTarget(t *testing.T) { + const sentinel = "SENTINEL-FILE-CONTENT-7f3a" + recs := captureAudit(t, func() { + callTool(t, auditServer(auditStubTool{name: "file_write_branch"}), "file_write_branch", + map[string]any{"owner": "mathias", "repo": "x", "path": "a.txt", "content": sentinel}) + }) + require.Len(t, recs, 1) + raw, err := json.Marshal(recs[0]) + require.NoError(t, err) + assert.NotContains(t, string(raw), sentinel) + assert.Contains(t, recs[0], "args_bytes") +} diff --git a/internal/mcp/server.go b/internal/mcp/server.go index 2fa3da9..2c2920e 100644 --- a/internal/mcp/server.go +++ b/internal/mcp/server.go @@ -4,6 +4,7 @@ import ( "encoding/json" "errors" "net/http" + "time" "git.d-ma.be/mathias/gitea-mcp/internal/registry" ) @@ -90,7 +91,9 @@ func (s *Server) handlePOST(w http.ResponseWriter, r *http.Request) { NewErrorResponse(req.ID, -32602, "invalid params", nil)) return } + start := time.Now() out, err := s.opts.Registry.Dispatch(r.Context(), p.Name, p.Arguments) + logToolCall(r.Context(), p.Name, p.Arguments, start, err) if err != nil { code := -32000 if errors.Is(err, registry.ErrToolNotFound) {