refactor(api): require every /api route to declare its server-scope status

server/cmd/main.go

@@ -293,7 +293,7 @@ func serve() {
 		log.Fatalf("api scope map: %v", err)
 	}
 
-	if err := api.AssertServerScopeMapComplete(serverTouchingRoutes(r)); err != nil {
+	if err := api.AssertServerScopeMapComplete(apiRoutes(r)); err != nil {
 		log.Fatalf("api server scope map: %v", err)
 	}
 
@@ -359,41 +359,24 @@ func boolEnv(key string) bool {
 	return false
 }
 
-// serverTouchingRoutes restricts AssertServerScopeMapComplete to routes whose
-// pattern names server-derived data — "server", ":serverId", "console" or
-// "assign", or the exact "workflows/:id/run" — rather than every routeScopes
-// entry, so unrelated routes are never swept in and boot never fails for no
-// reason. ":serverId" and "assign" were added after POST /api/keys/:id/assign
-// turned out to resolve a named server (in its request body, not its path)
-// with no scope check and no entry in serverScopedRoutes at all — the
-// original three-way filter never saw it because nothing in its path pattern
-// said "server".
+// apiRoutes lists every registered /api route as "METHOD /path", which is the
+// whole input AssertServerScopeMapComplete now takes.
 //
-// A substring filter is the weak part of this design: it only catches a route
-// whose *path* names a server, and a route can act on a server named in its
-// body, a query parameter, or an ID a handler derives some other way, with a
-// path that says nothing about it — as the keys/assign route did. Each time
-// that happens the fix is another substring added here, which finds this
-// class of gap one instance late rather than by construction. The stronger
-// design would invert the model: every /api route declares itself in
-// serverScopedRoutes (or an adjacent map), with an explicit exemption list
-// for the handful that genuinely act on nothing server-scoped, so a new route
-// is checked by default rather than only if its path happens to match a
-// pattern someone thought to add. That inversion is a larger change than
-// this filter widening and is left as a follow-up, not done here.
-func serverTouchingRoutes(r *gin.Engine) []string {
+// It replaces a substring filter that fed in only routes whose path contained
+// "server", ":serverId", "console" or "assign". That filter could only ever
+// catch a route whose *path* named a server, and a route can act on one named
+// in its body, in a query parameter, or derived by the handler — it caught one
+// of the leaks found in the final review of the MCP feature, and none of the
+// eleven found during implementation. Declaring every route is more typing
+// once and no maintenance after: a new route fails boot until somebody answers
+// "does this touch server data?" for it.
+func apiRoutes(r *gin.Engine) []string {
 	var out []string
 	for _, route := range r.Routes() {
 		if !strings.HasPrefix(route.Path, "/api/") {
 			continue
 		}
-		if strings.Contains(route.Path, "server") ||
-			strings.Contains(route.Path, ":serverId") ||
-			strings.Contains(route.Path, "console") ||
-			strings.Contains(route.Path, "assign") ||
-			route.Path == "/api/workflows/:id/run" {
-			out = append(out, route.Method+" "+route.Path)
-		}
+		out = append(out, route.Method+" "+route.Path)
 	}
 	return out
 }