Compare commits

...
4 Commits
Author SHA1 Message Date
mathiasandClaude Opus 5.5 724a997002 feat(mcp): audit-log every tools/call (OWASP MCP08)
CD / Deploy via GitOps (push) Successful in 5s
CD / Lint / Test / Vet (push) Successful in 31s
CD / Build & Import (push) Successful in 14s
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>
2026-10-01 20:34:59 +02:00
mathiasandClaude Opus 5 59dabc9455 chore(context): adopt canonical context-sync.sh and re-sync adapters
CD / Lint / Test / Vet (push) Successful in 13s
CD / Build & Import (push) Successful in 17s
CD / Deploy via GitOps (push) Successful in 5s
infra's scripts/context-sync.sh is the estate canonical, and it moved in infra
262369b (a SIGPIPE fix on the placeholder-reporting path: `printf | grep -o |
head -1` became `head -1 < <(grep -o ...)`). Every other repo still carried the
old line, so ContextSyncScriptDrift and ContextAdapterDrift were both firing.

Also picks up the canonical AGENT.md change from local-dev a01ee80 — rule 5's
GitOps review lane and the new precedence rule — which every repo's adapters
need regenerated for regardless of the script.

Refs mathias/infra#470

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
2026-09-30 10:36:32 +02:00
mathiasandClaude Opus 5 6b57a8ba8a chore(context): regenerate derived adapters from the canonical root context
CD / Lint / Test / Vet (push) Successful in 41s
CD / Build & Import (push) Successful in 22s
CD / Deploy via GitOps (push) Successful in 10s
ContextAdapterDrift was firing for 13 repos. Picks up behaviour rule 7
("Finish what's closable"), which landed in ~/dev/.context/AGENT.md and never
reached any repo's derived files.

Where mcp.json changes, it also picks up root MCP config the repo was stale on:
brain/gitea/infra move from public hostnames to tailnet Serve URLs, and gitea's
bearer changes from GITEA_MCP_TOKEN to DMABE_GITEA_API_TOKEN — gitea-mcp
authenticates by pass-through and 401s on the former, so a harness syncing from
the old file had a dead gitea server and no signal saying so.

No literal secrets: mcp.json carries ${VAR} placeholders only.

CLAUDE.md deliberately unchanged — context-sync writes it from PROJECT.md alone,
since Claude Code walks up to ~/dev/CLAUDE.md for root context.

Refs mathias/infra ContextAdapterDrift

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
2026-09-25 07:34:24 +02:00
mathiasandClaude Opus 5 e1e9a2e30e feat(auth): every accepted request names the path that granted it
CD / Deploy via GitOps (push) Successful in 5s
CD / Lint / Test / Vet (push) Successful in 16s
CD / Build & Import (push) Successful in 16s
Closes #60. Closes #58 (fixed in mcp-chassis v0.7.0, consumed here).

Three auth paths reach this server and none of them said so on success.

PassthroughMiddleware logged nothing at all, success or failure. While verifying
#59 end to end there was no way to confirm from the logs whether calls were
using the new PAT pass-through — the caller's own Gitea PAT, the caller's own
identity — or falling through to the shared JWT/static path. The only available
reasoning was inference from which failures were absent.

    INFO mcp auth accepted auth_path=passthrough caller=alice token_fp=...
    INFO mcp auth accepted auth_path=jwt        caller=mathiasbq token_fp=...
    INFO mcp auth accepted auth_path=static     token_fp=...

The first line matters more than the other two. Pass-through is the path where a
request acts as a SPECIFIC Gitea user; an audit trail that cannot say which
identity performed a write is not an audit trail.

chassisauth.Fingerprint, not a local equivalent. The chassis emits token_fp on
its own accepted and rejected lines, and two different fingerprints for the same
token cannot be correlated — which is the only reason the field exists. Exported
from the chassis in v0.7.1 for exactly this.

A token that is not a valid PAT still falls through SILENTLY. The chassis owns
the verdict, and logging here as well would double-count every static-token
request, since all of them reach this middleware first. Asserted, because "logs
nothing on the failure path" is indistinguishable from "was never called".

Also lands #58 by consuming mcp-chassis v0.7.0: a JWT presented to a server with
no JWT auth configured now reports jwt_auth_not_configured rather than
static_token_mismatch. That log line is what sent a debugging session into
re-running the OAuth flow while the credentials were valid the whole time.

Verified: task check EXIT=0.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
2026-09-14 20:59:04 +02:00
10 changed files with 431 additions and 6 deletions
+72 -2
View File
@@ -78,6 +78,27 @@ These rules apply to every task across every project, regardless of harness.
when a human reviewer outside the project is required. Document the reason in
PROJECT.md.
**Exception — a GitOps repo where a branch cannot be reconciled.** In a repo whose
deployment watches one branch (`mathias/infra`: Flux watches `main`), a branch is
invisible to the thing that applies it. A `risk/medium` change to `alerts.yaml` or
`litellm-config.yaml` can be linted on a branch but never *exercised* there, so a
PR would buy a review of a state that was never deployed.
There, `risk/medium`+ work stays on `main` and instead requires a **recorded
adversarial review pass before the push** — `skills:pr-gatekeeper` or equivalent,
reviewing the staged diff. **The reviewer's verdict goes in the issue, not a PR
thread.** No verdict recorded means the lane was not used, and saying "the gate was
green" is not the same claim.
Be honest about what this is: a recorded *independent* pass, not four-eyes in the
sense `assessor-loop` §4.1 means for a regulated organisation, because the reviewer
is an agent. It is more than the repo had and less than the model it mirrors; do not
describe it as the stronger property. (infra#470, decided 2026-09-29.)
This lane depends on the `risk/` label being right. A label that triggers a heavier
lane is load-bearing, not cosmetic — if work that belongs in it is unlabelled, the
lane silently does not apply.
6. **Close the loop — every substantive task ends with the same ritual.** Shipping
the code is not the end of the task; capturing it is. Run this unprompted:
- **Tag + bump SemVer** on the change (annotated tag; minor for a feature or
@@ -106,6 +127,43 @@ These rules apply to every task across every project, regardless of harness.
revisit Y". Those are exactly the items that need a locator. If it is worth
saying in a summary, it is worth writing where it can be found again without you.
7. **Finish what's closable — only involve me for a genuine call.** If a
tracker issue, PR, or checklist item is demonstrably resolved — the fix
landed, the state changed, you verified it live — close/merge/check it off
in the same turn. Don't leave it open "for the record" or park it waiting
on me to confirm something that already happened; that's bookkeeping, not
a decision. Reserve asking me (`AskUserQuestion`, a `DECISION NEEDED`
comment, or otherwise pausing) for when it's genuinely mine to call:
irreversible or prod/host-impacting actions, a real judgment call with no
clearly-better option, ambiguous intent, or anything secret-writes/
destructive-git already gate. Before claiming something is closable,
verify it live (re-check the issue, re-run the command) rather than
trusting your own earlier summary (brain: `filed-issue-not-same-as-fixed`).
8. **Control code needs an adversary, not a checklist.** Code whose job is to
refuse, deny, gate or contain is not adequately verified by a green gate.
Rule 5 makes CI the quality gate and that holds for ordinary work — but a
passing suite only asserts what its author already thought of, and for a
control the author is precisely the person whose imagination just failed.
Measured on `dispatch#75`: five review rounds, five real escapes verified by
execution, and **CI was green on every one of them** while `git rm
././.dispatch-allow` deleted the file the control existed to protect. So
route this work through a review with authority to block, instructed to
verify by execution rather than by reading the implementation. Expect to be
blocked more than once; that is the control working, not the reviewer being
difficult. (brain: `a-green-gate-is-not-evidence-for-control-code`; the
practices live in the `pr-gatekeeper` agent.)
9. **A test's name must not claim more than its assertions prove.** Name a test
after the mechanism it observes, not the goal it serves.
`DoesNotInheritEnv` is honest; `IsContained` is a claim the test cannot
support. This is not pedantry about naming — when name and assertion
diverge, the name wins in the reader's head and the gap becomes invisible,
which is what lets the defect survive review. Found three times in one PR,
twice in tests written to fix the previous instance of it; each passed while
the thing it was named for was exploitable. If you cannot name the mechanism,
you do not yet know what the test observes.
## CAD sprint patterns
Two CAD sprint archetypes (defs: brain `wiki/homelab/decisions/continuous-agentic-development-cad-concept-2026-06-16`):
@@ -319,8 +377,20 @@ Workflow:
and missing-untracked adapters). A drift fails the check with a
message telling you to stage the regenerated files.
Behavior rules in this file and per-project rules in `PROJECT.md` apply
unconditionally on every host, every harness.
Behavior rules in this file and per-project rules in `PROJECT.md` apply on every
host and every harness.
**Where a project rule contradicts a rule here, the project rule governs — and it
must state its reason.** Both halves matter: the project file wins because it knows
something this file cannot (infra's Flux constraint is real and not visible from
here), and it must say why because a bare override is indistinguishable from drift.
This replaces the previous wording, which said rules here and in `PROJECT.md` both
apply *"unconditionally"*. That was not a tiebreak but a contradiction, and it went
unnoticed until rule 5 and infra's "never create feature branches" had to be obeyed
at the same time (infra#470). A standard can hold two individually sensible clauses
that are jointly unsatisfiable, and survive for as long as everyone reads it
charitably — the same failure the Inception Sprint Oath's G6 had.
## Engineering Skills
+72 -2
View File
@@ -73,6 +73,27 @@ These rules apply to every task across every project, regardless of harness.
when a human reviewer outside the project is required. Document the reason in
PROJECT.md.
**Exception — a GitOps repo where a branch cannot be reconciled.** In a repo whose
deployment watches one branch (`mathias/infra`: Flux watches `main`), a branch is
invisible to the thing that applies it. A `risk/medium` change to `alerts.yaml` or
`litellm-config.yaml` can be linted on a branch but never *exercised* there, so a
PR would buy a review of a state that was never deployed.
There, `risk/medium`+ work stays on `main` and instead requires a **recorded
adversarial review pass before the push** — `skills:pr-gatekeeper` or equivalent,
reviewing the staged diff. **The reviewer's verdict goes in the issue, not a PR
thread.** No verdict recorded means the lane was not used, and saying "the gate was
green" is not the same claim.
Be honest about what this is: a recorded *independent* pass, not four-eyes in the
sense `assessor-loop` §4.1 means for a regulated organisation, because the reviewer
is an agent. It is more than the repo had and less than the model it mirrors; do not
describe it as the stronger property. (infra#470, decided 2026-09-29.)
This lane depends on the `risk/` label being right. A label that triggers a heavier
lane is load-bearing, not cosmetic — if work that belongs in it is unlabelled, the
lane silently does not apply.
6. **Close the loop — every substantive task ends with the same ritual.** Shipping
the code is not the end of the task; capturing it is. Run this unprompted:
- **Tag + bump SemVer** on the change (annotated tag; minor for a feature or
@@ -101,6 +122,43 @@ These rules apply to every task across every project, regardless of harness.
revisit Y". Those are exactly the items that need a locator. If it is worth
saying in a summary, it is worth writing where it can be found again without you.
7. **Finish what's closable — only involve me for a genuine call.** If a
tracker issue, PR, or checklist item is demonstrably resolved — the fix
landed, the state changed, you verified it live — close/merge/check it off
in the same turn. Don't leave it open "for the record" or park it waiting
on me to confirm something that already happened; that's bookkeeping, not
a decision. Reserve asking me (`AskUserQuestion`, a `DECISION NEEDED`
comment, or otherwise pausing) for when it's genuinely mine to call:
irreversible or prod/host-impacting actions, a real judgment call with no
clearly-better option, ambiguous intent, or anything secret-writes/
destructive-git already gate. Before claiming something is closable,
verify it live (re-check the issue, re-run the command) rather than
trusting your own earlier summary (brain: `filed-issue-not-same-as-fixed`).
8. **Control code needs an adversary, not a checklist.** Code whose job is to
refuse, deny, gate or contain is not adequately verified by a green gate.
Rule 5 makes CI the quality gate and that holds for ordinary work — but a
passing suite only asserts what its author already thought of, and for a
control the author is precisely the person whose imagination just failed.
Measured on `dispatch#75`: five review rounds, five real escapes verified by
execution, and **CI was green on every one of them** while `git rm
././.dispatch-allow` deleted the file the control existed to protect. So
route this work through a review with authority to block, instructed to
verify by execution rather than by reading the implementation. Expect to be
blocked more than once; that is the control working, not the reviewer being
difficult. (brain: `a-green-gate-is-not-evidence-for-control-code`; the
practices live in the `pr-gatekeeper` agent.)
9. **A test's name must not claim more than its assertions prove.** Name a test
after the mechanism it observes, not the goal it serves.
`DoesNotInheritEnv` is honest; `IsContained` is a claim the test cannot
support. This is not pedantry about naming — when name and assertion
diverge, the name wins in the reader's head and the gap becomes invisible,
which is what lets the defect survive review. Found three times in one PR,
twice in tests written to fix the previous instance of it; each passed while
the thing it was named for was exploitable. If you cannot name the mechanism,
you do not yet know what the test observes.
## CAD sprint patterns
Two CAD sprint archetypes (defs: brain `wiki/homelab/decisions/continuous-agentic-development-cad-concept-2026-06-16`):
@@ -314,8 +372,20 @@ Workflow:
and missing-untracked adapters). A drift fails the check with a
message telling you to stage the regenerated files.
Behavior rules in this file and per-project rules in `PROJECT.md` apply
unconditionally on every host, every harness.
Behavior rules in this file and per-project rules in `PROJECT.md` apply on every
host and every harness.
**Where a project rule contradicts a rule here, the project rule governs — and it
must state its reason.** Both halves matter: the project file wins because it knows
something this file cannot (infra's Flux constraint is real and not visible from
here), and it must say why because a bare override is indistinguishable from drift.
This replaces the previous wording, which said rules here and in `PROJECT.md` both
apply *"unconditionally"*. That was not a tiebreak but a contradiction, and it went
unnoticed until rule 5 and infra's "never create feature branches" had to be obeyed
at the same time (infra#470). A standard can hold two individually sensible clauses
that are jointly unsatisfiable, and survive for as long as everyone reads it
charitably — the same failure the Inception Sprint Oath's G6 had.
## Engineering Skills
+1 -1
View File
@@ -3,7 +3,7 @@ module git.d-ma.be/mathias/gitea-mcp
go 1.26.2
require (
git.d-ma.be/mathias/mcp-chassis v0.6.0
git.d-ma.be/mathias/mcp-chassis v0.7.1
github.com/hashicorp/golang-lru/v2 v2.0.7
github.com/stretchr/testify v1.11.1
)
+4
View File
@@ -2,6 +2,10 @@ git.d-ma.be/mathias/mcp-chassis v0.5.0 h1:0w3dt4t4r8OtZBHIOoZkR6p6L3L6YDiqhMh9bT
git.d-ma.be/mathias/mcp-chassis v0.5.0/go.mod h1:Ks7EK2UnGAN0H3rJjKUxUagX8/ZBdtLrOlcUbv0RwH8=
git.d-ma.be/mathias/mcp-chassis v0.6.0 h1:/MuxT9IyzZbN26+COlI2FK0moTyXMLG/qXc/cm0i0Vg=
git.d-ma.be/mathias/mcp-chassis v0.6.0/go.mod h1:Ks7EK2UnGAN0H3rJjKUxUagX8/ZBdtLrOlcUbv0RwH8=
git.d-ma.be/mathias/mcp-chassis v0.7.0 h1:ct31v1F1PMhMCva/V0ErEbgMzwnT+27lYKbug2A/quE=
git.d-ma.be/mathias/mcp-chassis v0.7.0/go.mod h1:Ks7EK2UnGAN0H3rJjKUxUagX8/ZBdtLrOlcUbv0RwH8=
git.d-ma.be/mathias/mcp-chassis v0.7.1 h1:4h8TOBHTA0L/no4IPcr4tLcQtmKUlhE2/cXyPdNFGmM=
git.d-ma.be/mathias/mcp-chassis v0.7.1/go.mod h1:Ks7EK2UnGAN0H3rJjKUxUagX8/ZBdtLrOlcUbv0RwH8=
github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=
github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c=
github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=
+26
View File
@@ -2,9 +2,12 @@ package auth
import (
"context"
"log/slog"
"net/http"
"strings"
chassisauth "git.d-ma.be/mathias/mcp-chassis/auth"
"git.d-ma.be/mathias/gitea-mcp/internal/gitea"
)
@@ -25,11 +28,34 @@ func PassthroughMiddleware(validator TokenValidator, onValid, fallback http.Hand
token, hasBearer := strings.CutPrefix(authz, "Bearer ")
if hasBearer && token != "" {
if login, ok := validator.ValidateToken(r.Context(), token); ok {
// gitea-mcp#60. This middleware logged nothing at all, so there
// was no way to tell a PAT pass-through request from one that
// fell through to the shared JWT/static path — only inference
// from which failures were absent.
//
// It matters more here than on the other two paths: this is
// where a request acts as a SPECIFIC Gitea user. An audit trail
// that cannot say which identity performed a write is not an
// audit trail.
//
// chassisauth.Fingerprint, not a local equivalent: the chassis
// emits the same field on its own accepted/rejected lines, and
// two different fingerprints for one token cannot be correlated.
slog.Info("mcp auth accepted",
"auth_path", "passthrough",
"caller", login,
"token_fp", chassisauth.Fingerprint(token),
"path", r.URL.Path,
)
ctx := withCaller(r.Context(), login)
ctx = gitea.WithToken(ctx, token)
onValid.ServeHTTP(w, r.WithContext(ctx))
return
}
// A token that is not a valid PAT falls through SILENTLY. The
// chassis owns the verdict; logging here too would double-count
// every static-token request, since all of them reach this
// middleware first.
}
fallback.ServeHTTP(w, r)
})
+83
View File
@@ -0,0 +1,83 @@
package auth
import (
"bytes"
"context"
"encoding/json"
"log/slog"
"net/http"
"net/http/httptest"
"strings"
"testing"
"github.com/stretchr/testify/require"
)
type stubValidator struct{ login string }
func (s stubValidator) ValidateToken(context.Context, string) (string, bool) {
return s.login, s.login != ""
}
// gitea-mcp#60: PassthroughMiddleware logged nothing at all, success or failure.
//
// While verifying #59 end to end there was no way to confirm from the server
// logs whether calls were using the new PAT pass-through (caller's own Gitea
// PAT, own identity) or falling through to the shared JWT/static path. The only
// way to reason about it was inference. The chassis names its two paths as of
// v0.7.0; this is the third.
//
// This matters more than the other two: pass-through is the path where the
// request acts as a SPECIFIC Gitea user. An audit trail that cannot say which
// identity performed a write is not an audit trail.
func TestPassthroughMiddleware_LogsTheAcceptedPath(t *testing.T) {
var buf bytes.Buffer
prev := slog.Default()
slog.SetDefault(slog.New(slog.NewJSONHandler(&buf, &slog.HandlerOptions{Level: slog.LevelInfo})))
t.Cleanup(func() { slog.SetDefault(prev) })
reached := false
h := PassthroughMiddleware(stubValidator{login: "alice"},
http.HandlerFunc(func(http.ResponseWriter, *http.Request) { reached = true }),
http.HandlerFunc(func(http.ResponseWriter, *http.Request) { t.Fatal("must not fall back") }))
req := httptest.NewRequest(http.MethodGet, "/mcp", nil)
req.Header.Set("Authorization", "Bearer a-real-gitea-pat")
h.ServeHTTP(httptest.NewRecorder(), req)
require.True(t, reached)
var line map[string]any
require.NoError(t, json.Unmarshal([]byte(strings.TrimSpace(buf.String())), &line))
require.Equal(t, "mcp auth accepted", line["msg"])
require.Equal(t, "passthrough", line["auth_path"])
require.Equal(t, "alice", line["caller"])
require.NotEmpty(t, line["token_fp"], "correlatable without being a credential")
for k, v := range line {
require.NotContains(t, []any{v}, "a-real-gitea-pat",
"the PAT must never appear in any field, including %q", k)
}
}
// A token that is not a valid PAT must fall through silently to the chassis,
// which owns the rejection log. Logging here as well would double-count every
// static-token request — every one of them reaches this middleware first.
func TestPassthroughMiddleware_FallsThroughWithoutLogging(t *testing.T) {
var buf bytes.Buffer
prev := slog.Default()
slog.SetDefault(slog.New(slog.NewJSONHandler(&buf, &slog.HandlerOptions{Level: slog.LevelInfo})))
t.Cleanup(func() { slog.SetDefault(prev) })
fellBack := false
h := PassthroughMiddleware(stubValidator{},
http.HandlerFunc(func(http.ResponseWriter, *http.Request) { t.Fatal("must not accept") }),
http.HandlerFunc(func(http.ResponseWriter, *http.Request) { fellBack = true }))
req := httptest.NewRequest(http.MethodGet, "/mcp", nil)
req.Header.Set("Authorization", "Bearer not-a-pat")
h.ServeHTTP(httptest.NewRecorder(), req)
require.True(t, fellBack)
require.Empty(t, strings.TrimSpace(buf.String()),
"the chassis owns the verdict; logging here too would double-count every static-token call")
}
+46
View File
@@ -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...)
}
+123
View File
@@ -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")
}
+3
View File
@@ -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) {
+1 -1
View File
@@ -58,7 +58,7 @@ if [ "$FORCE" = false ]; then
while IFS= read -r _match; do
_lineno="${_match%%:*}"
_content="${_match#*:}"
_token=$(printf '%s' "$_content" | grep -o '{{[^}]*}}' | head -1)
_token=$(head -1 < <(grep -o '{{[^}]*}}' <<<"$_content"))
echo " $PROJECT_FILE:$_lineno: unfilled placeholder $_token" >&2
done <<< "$_placeholders"
echo "" >&2