feat(mcp): audit-log every tools/call (OWASP MCP08)
tools/call dispatched with no record, so repo_delete, pr_merge and file_delete left no trail in gitea-mcp; only auth acceptance was logged. One slog record per call: tool, caller, token fingerprint, target (owner/repo/number/branch/path/tag), outcome, duration. Every other argument is reduced to args_bytes so the audit line never copies file content or issue bodies. Closes #64 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
@@ -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...)
|
||||||
|
}
|
||||||
@@ -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")
|
||||||
|
}
|
||||||
@@ -4,6 +4,7 @@ import (
|
|||||||
"encoding/json"
|
"encoding/json"
|
||||||
"errors"
|
"errors"
|
||||||
"net/http"
|
"net/http"
|
||||||
|
"time"
|
||||||
|
|
||||||
"git.d-ma.be/mathias/gitea-mcp/internal/registry"
|
"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))
|
NewErrorResponse(req.ID, -32602, "invalid params", nil))
|
||||||
return
|
return
|
||||||
}
|
}
|
||||||
|
start := time.Now()
|
||||||
out, err := s.opts.Registry.Dispatch(r.Context(), p.Name, p.Arguments)
|
out, err := s.opts.Registry.Dispatch(r.Context(), p.Name, p.Arguments)
|
||||||
|
logToolCall(r.Context(), p.Name, p.Arguments, start, err)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
code := -32000
|
code := -32000
|
||||||
if errors.Is(err, registry.ErrToolNotFound) {
|
if errors.Is(err, registry.ErrToolNotFound) {
|
||||||
|
|||||||
Reference in New Issue
Block a user