fix: audit refused and failed mcp write calls, scope key assignment to token

Every early return from a write-tool handler skipped both the tool's own
LogCall and transport.go's gated LogCall (which only fires for reads), so a
blocked mutation attempt left no audit trail. registerSDKTool now routes
every write-tool error through LogDenied (fan-out and tag-scope refusals,
by gate name) or LogFailure (everything else), keeping the successful-write
path logging its own resolved server count exactly as before.

Also close a live scope gap surfaced while reviewing this: POST
/api/keys/:id/assign called services.AssignKey with an unscoped GetServer
lookup, so a tag-restricted token could assign a key to a server outside
its restriction. The handler now resolves the target through
GetServerScoped first, matching its sibling revoke route, and the route is
recorded in serverScopedRoutes.

server/internal/api/handlers.go

@@ -623,6 +623,7 @@ func deleteKey(c *gin.Context) {
 //	@Router			/keys/{id}/assign [post]
 func assignKey(c *gin.Context) {
 	keyID := c.Param("id")
+	instanceID := auth.InstanceID(c)
 	var body struct {
 		ServerID string `json:"server_id" binding:"required"`
 	}
@@ -631,7 +632,12 @@ func assignKey(c *gin.Context) {
 		return
 	}
 
-	a, err := services.AssignKey(auth.InstanceID(c), keyID, body.ServerID)
+	if _, err := services.GetServerScoped(instanceID, body.ServerID, auth.ServerScope(c)); err != nil {
+		c.JSON(http.StatusNotFound, gin.H{"error": "server not found"})
+		return
+	}
+
+	a, err := services.AssignKey(instanceID, keyID, body.ServerID)
 	if err != nil {
 		c.JSON(http.StatusInternalServerError, gin.H{"error": err.Error()})
 		return

server/internal/api/serverscope.go

@@ -50,6 +50,16 @@ var serverScopedRoutes = map[string]bool{
 	// assignment on a server outside its scope.
 	"DELETE /api/keys/:id/assign/:serverId": true,
 
+	// assignKey resolves body.ServerID through GetServerScoped before calling
+	// services.AssignKey, which itself uses the unscoped GetServer — so a
+	// restricted token can no longer assign a key to a server outside its
+	// scope by naming it in the request body. This route's path carries
+	// neither "server" nor "console", so it is not swept in by
+	// serverTouchingRoutes and this entry is not boot-enforced; it is kept
+	// here anyway as the record of a considered decision, matching its
+	// sibling revoke route.
+	"POST /api/keys/:id/assign": true,
+
 	// Creating a server has no server to filter yet.
 	"POST /api/servers": false,
 	// The agent's own enrolment routes authenticate as the agent, not as a

server/internal/mcp/audit.go

@@ -14,10 +14,32 @@ import (
 // the whole fleet on one badly phrased instruction.
 const FanOutLimit = 25
 
+// Gate names for a write tool's own policy refusals, on top of GateMCPScope
+// and GateResourceScope in registry.go. These name a decision this package
+// made deliberately, so a human reading audit_logs can tell "the agent was
+// stopped by policy" from "the agent tried and the machine failed".
+const (
+	GateFanOut   = "fan_out"
+	GateTagScope = "tag_selector"
+)
+
 // ErrConfirmRequired is returned to the model as a tool error it can act on:
 // it says what would have happened and how to proceed deliberately.
 var ErrConfirmRequired = errors.New("confirmation required")
 
+// ErrOutOfScope wraps a write tool's refusal to act because the resolved (or,
+// for run_workflow, the workflow's configured) targets are not entirely
+// within the calling token's tag restriction. Handlers wrap this rather than
+// returning a bare error so registerSDKTool can tell a scope refusal apart
+// from an ordinary service failure and audit it as GateTagScope.
+var ErrOutOfScope = errors.New("targets outside token scope")
+
+// logEvent is services.LogEvent behind a package variable so tests can
+// observe what would have been audited without a live database connection —
+// services.LogEvent talks straight to Mongo via db.Col, which panics on a nil
+// client outside a real boot.
+var logEvent = services.LogEvent
+
 // CheckFanOut refuses a write that would touch more servers than FanOutLimit
 // unless the call passed confirm:true.
 func CheckFanOut(count int, args map[string]any) error {
@@ -84,12 +106,22 @@ func LogCall(c Caller, t Tool, args map[string]any, servers int) {
 	if servers > 0 {
 		detail += fmt.Sprintf(", %d server(s) affected", servers)
 	}
-	services.LogEvent(c.InstanceID, "mcp.tool_call", c.TokenName, "", "", detail)
+	logEvent(c.InstanceID, "mcp.tool_call", c.TokenName, "", "", detail)
 }
 
 // LogDenied records a refusal and which gate refused, which is what turns "the
 // agent said it could not" into a diagnosable event.
 func LogDenied(c Caller, toolName, gate string) {
-	services.LogEvent(c.InstanceID, "mcp.tool_denied", c.TokenName, "", "",
+	logEvent(c.InstanceID, "mcp.tool_denied", c.TokenName, "", "",
 		fmt.Sprintf("tool %s refused by %s", toolName, gate))
 }
+
+// LogFailure records a write tool call that reached a service and that
+// service returned an error — as opposed to LogDenied, which records a
+// policy refusal that never reached one. Distinguishing the two in
+// audit_logs is what lets a human reading it tell "the agent was stopped"
+// from "the agent tried and the machine failed".
+func LogFailure(c Caller, t Tool, args map[string]any, err error) {
+	logEvent(c.InstanceID, "mcp.tool_failed", c.TokenName, "", "",
+		fmt.Sprintf("tool %s (%s) failed: %v", t.Name, SummariseArgs(args), err))
+}

server/internal/mcp/tools_write.go

@@ -78,7 +78,7 @@ func init() {
 			}
 			scopedTargets, err := services.ResolveTargetsScoped(c.InstanceID, wf.TargetServerIDs, wf.TargetTags, c.TokenScope)
 			if err != nil || len(scopedTargets) != len(allTargets) {
-				return nil, fmt.Errorf("no servers visible to this token matched the request")
+				return nil, fmt.Errorf("%w: no servers visible to this token matched the request", ErrOutOfScope)
 			}
 
 			if err := CheckFanOut(len(scopedTargets), args); err != nil {
@@ -162,7 +162,7 @@ func init() {
 			ids := stringSliceArg(args, "server_ids")
 			targets, err := services.ResolveTargetsScoped(c.InstanceID, ids, tagArg(args), c.TokenScope)
 			if err != nil {
-				return nil, fmt.Errorf("no servers visible to this token matched the request")
+				return nil, fmt.Errorf("%w: no servers visible to this token matched the request", ErrOutOfScope)
 			}
 			if err := CheckFanOut(len(targets), args); err != nil {
 				return nil, err
@@ -209,7 +209,7 @@ func init() {
 			ids := stringSliceArg(args, "server_ids")
 			targets, err := services.ResolveTargetsScoped(c.InstanceID, ids, tagArg(args), c.TokenScope)
 			if err != nil {
-				return nil, fmt.Errorf("no servers visible to this token matched the request")
+				return nil, fmt.Errorf("%w: no servers visible to this token matched the request", ErrOutOfScope)
 			}
 			if err := CheckFanOut(len(targets), args); err != nil {
 				return nil, err
@@ -272,7 +272,7 @@ func init() {
 			ids := stringSliceArg(args, "server_ids")
 			targets, err := services.ResolveTargetsScoped(c.InstanceID, ids, tagArg(args), c.TokenScope)
 			if err != nil {
-				return nil, fmt.Errorf("no servers visible to this token matched the request")
+				return nil, fmt.Errorf("%w: no servers visible to this token matched the request", ErrOutOfScope)
 			}
 			if err := CheckFanOut(len(targets), args); err != nil {
 				return nil, err

server/internal/mcp/transport.go

@@ -71,24 +71,52 @@ func registerSDKTool(srv *sdk.Server, tool Tool, caller Caller) {
 		Name:        tool.Name,
 		Description: tool.Description,
 	}, func(ctx context.Context, req *sdk.CallToolRequest, args map[string]any) (*sdk.CallToolResult, any, error) {
-		if ok, gate := Allowed(tool, caller); !ok {
-			LogDenied(caller, tool.Name, gate)
-			return nil, nil, toolError(gate, tool)
-		}
-
-		out, err := tool.Handler(ctx, caller, args)
-		if err != nil {
-			return nil, nil, err
-		}
-		if !tool.Write {
-			// Write tools log their own call with a resolved server count,
-			// which this layer cannot know.
-			LogCall(caller, tool, args, 0)
-		}
-		return nil, out, nil
+		return callTool(ctx, tool, caller, args)
 	})
 }
 
+// callTool is the gate check, dispatch and audit write registerSDKTool wraps
+// onto the SDK's call signature. It is a separate function — rather than the
+// closure body inline — so it can be exercised directly in tests without
+// standing up an sdk.Server and driving a real MCP request through it.
+func callTool(ctx context.Context, tool Tool, caller Caller, args map[string]any) (*sdk.CallToolResult, any, error) {
+	if ok, gate := Allowed(tool, caller); !ok {
+		LogDenied(caller, tool.Name, gate)
+		return nil, nil, toolError(gate, tool)
+	}
+
+	out, err := tool.Handler(ctx, caller, args)
+	if err != nil {
+		// A write tool's own handler never gets a chance to audit its own
+		// refusal or failure — it returns before reaching its LogCall, and
+		// unlike a successful write, this layer does not know a resolved
+		// server count to pass along anyway. So every write failure is
+		// audited here instead: a policy refusal (fan-out or tag scope) as
+		// mcp.tool_denied naming the gate, everything else as
+		// mcp.tool_failed, so a human reading audit_logs can tell "the agent
+		// was stopped" from "the agent tried and the machine failed". Read
+		// tools are unaffected — a failed read was never going to change
+		// anything and carries no gate to name.
+		if tool.Write {
+			switch {
+			case errors.Is(err, ErrConfirmRequired):
+				LogDenied(caller, tool.Name, GateFanOut)
+			case errors.Is(err, ErrOutOfScope):
+				LogDenied(caller, tool.Name, GateTagScope)
+			default:
+				LogFailure(caller, tool, args, err)
+			}
+		}
+		return nil, nil, err
+	}
+	if !tool.Write {
+		// Write tools log their own call with a resolved server count, which
+		// this layer cannot know.
+		LogCall(caller, tool, args, 0)
+	}
+	return nil, out, nil
+}
+
 // toolError explains a refusal in words the model can act on. A transport-level
 // failure would be invisible to it; a tool error is something it can read and
 // relay to its user.

server/internal/mcp/transport_test.go

@@ -0,0 +1,115 @@
+package mcp
+
+import (
+	"context"
+	"testing"
+)
+
+// TestRefusedWriteIsAudited exercises the real dispatch path (callTool, which
+// registerSDKTool wraps) for a write tool whose handler refuses the call
+// before it ever reaches its own LogCall — a fan-out refusal, in this case,
+// which run_workflow, apply_updates, update_agent and assign_key all reach
+// the same way via CheckFanOut. The refusal must still produce an audit row:
+// a blocked mutation attempt is the single most audit-worthy event a write
+// tool produces, and until this test the only thing recording it was the
+// tool's own success path.
+func TestRefusedWriteIsAudited(t *testing.T) {
+	var got []string
+	restore := logEvent
+	logEvent = func(instanceID, eventType, actor, serverID, keyID, details string) {
+		got = append(got, eventType+": "+details)
+	}
+	defer func() { logEvent = restore }()
+
+	tool := Tool{
+		Name:  "test_write_tool",
+		Write: true,
+		Scope: "servers:write",
+		Handler: func(ctx context.Context, c Caller, args map[string]any) (any, error) {
+			return nil, CheckFanOut(200, args)
+		},
+	}
+
+	caller := Caller{Scopes: []string{"mcp:write", "servers:write"}}
+
+	_, _, err := callTool(context.Background(), tool, caller, nil)
+	if err == nil {
+		t.Fatal("callTool() = nil error, want the fan-out refusal")
+	}
+
+	if len(got) != 1 {
+		t.Fatalf("logEvent called %d times, want exactly 1 audit row for the refusal; got %v", len(got), got)
+	}
+	if want := "mcp.tool_denied: "; len(got[0]) < len(want) || got[0][:len(want)] != want {
+		t.Errorf("audit row %q does not record a denial", got[0])
+	}
+}
+
+// TestOutOfScopeWriteIsAudited covers the other write refusal shape: a
+// service-layer ErrOutOfScope wrap, as apply_updates/update_agent/assign_key
+// return when ResolveTargetsScoped finds nothing this token may touch.
+func TestOutOfScopeWriteIsAudited(t *testing.T) {
+	var got []string
+	restore := logEvent
+	logEvent = func(instanceID, eventType, actor, serverID, keyID, details string) {
+		got = append(got, eventType)
+	}
+	defer func() { logEvent = restore }()
+
+	tool := Tool{
+		Name:  "test_scoped_tool",
+		Write: true,
+		Scope: "servers:write",
+		Handler: func(ctx context.Context, c Caller, args map[string]any) (any, error) {
+			return nil, ErrOutOfScope
+		},
+	}
+
+	caller := Caller{Scopes: []string{"mcp:write", "servers:write"}}
+
+	if _, _, err := callTool(context.Background(), tool, caller, nil); err == nil {
+		t.Fatal("callTool() = nil error, want the scope refusal")
+	}
+
+	if len(got) != 1 || got[0] != "mcp.tool_denied" {
+		t.Errorf("audit events = %v, want exactly one mcp.tool_denied row", got)
+	}
+}
+
+// TestServiceFailureIsAuditedDistinctly makes sure a write tool failing for a
+// reason that is not a policy refusal — the underlying service call itself
+// erroring — is still audited, but as mcp.tool_failed rather than
+// mcp.tool_denied, so a human reading audit_logs can tell the two apart.
+func TestServiceFailureIsAuditedDistinctly(t *testing.T) {
+	var events []string
+	restore := logEvent
+	logEvent = func(instanceID, eventType, actor, serverID, keyID, details string) {
+		events = append(events, eventType)
+	}
+	defer func() { logEvent = restore }()
+
+	tool := Tool{
+		Name:  "test_failing_tool",
+		Write: true,
+		Scope: "servers:write",
+		Handler: func(ctx context.Context, c Caller, args map[string]any) (any, error) {
+			return nil, errFakeServiceFailure
+		},
+	}
+
+	caller := Caller{Scopes: []string{"mcp:write", "servers:write"}}
+
+	if _, _, err := callTool(context.Background(), tool, caller, nil); err == nil {
+		t.Fatal("callTool() = nil error, want the service failure")
+	}
+
+	if len(events) != 1 || events[0] != "mcp.tool_failed" {
+		t.Errorf("audit events = %v, want exactly one mcp.tool_failed row", events)
+	}
+}
+
+var errFakeServiceFailure = &fakeError{"the dispatcher refused the command"}
+
+type fakeError struct{ msg string }
+
+func (e *fakeError) Error() string { return e.msg }