fix: close out-of-scope server access in vulns, packages, run logs, and key revoke

server/internal/api/handlers.go

@@ -637,8 +637,14 @@ func assignKey(c *gin.Context) {
 func revokeAssignment(c *gin.Context) {
 	keyID := c.Param("id")
 	serverID := c.Param("serverId")
+	instanceID := auth.InstanceID(c)
 
-	if err := services.RevokeAssignment(auth.InstanceID(c), keyID, serverID); err != nil {
+	if _, err := services.GetServerScoped(instanceID, serverID, auth.ServerScope(c)); err != nil {
+		c.JSON(http.StatusNotFound, gin.H{"error": "server not found"})
+		return
+	}
+
+	if err := services.RevokeAssignment(instanceID, keyID, serverID); err != nil {
 		c.JSON(http.StatusInternalServerError, gin.H{"error": err.Error()})
 		return
 	}

server/internal/api/serverscope.go

@@ -9,7 +9,10 @@ import "fmt"
 // tomorrow that reads server data without filtering would leak a restricted
 // token's blind spot silently, and this turns that into a failure at boot.
 //
-// false means "deliberately fleet-wide" and requires a comment saying why.
+// false means the route is deliberately fleet-wide and requires a comment
+// explaining the deliberate reason. It must never mean "not scoped yet" —
+// an unresolved gap belongs on the fix list, not in this map, because a false
+// entry here is read as a considered decision, not a placeholder.
 var serverScopedRoutes = map[string]bool{
 	"GET /api/servers":                    true,
 	"GET /api/servers/:id":                true,
@@ -29,6 +32,24 @@ var serverScopedRoutes = map[string]bool{
 	"POST /api/servers/:id/workloads/:wid/action": true,
 	"GET /api/servers/:id/workloads/:wid/logs":    true,
 
+	// listServerVulnerabilities and getServerPackages now resolve the server
+	// through GetServerScoped before calling ListFindings/ListPackages, so an
+	// out-of-scope server ID reads as not-found before either function runs.
+	"GET /api/servers/:id/vulnerabilities": true,
+	"GET /api/servers/:id/packages":        true,
+
+	// getServerRunLog/streamServerRunLog resolve :serverId through
+	// GetServerScoped before reading anything from the log store, so a
+	// restricted token holding a valid runId still cannot read output from a
+	// server outside its scope.
+	"GET /api/runs/:runId/servers/:serverId/logs":        true,
+	"GET /api/runs/:runId/servers/:serverId/logs/stream": true,
+
+	// revokeAssignment resolves :serverId through GetServerScoped before
+	// calling RevokeAssignment, so a restricted token cannot mutate an
+	// assignment on a server outside its scope.
+	"DELETE /api/keys/:id/assign/:serverId": 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
@@ -36,42 +57,13 @@ var serverScopedRoutes = map[string]bool{
 	"GET /api/servers/new":  false,
 	"POST /api/servers/new": false,
 
-	// KnownTags aggregates the tag *vocabulary* in use across the fleet —
-	// keys and the values seen for them — never a server identifier or any
-	// other server attribute, so it does not let a restricted token enumerate
-	// which hosts exist. Scoping it would need ListServersFiltered-style
-	// plumbing through KnownTags for a leak this narrow; left fleet-wide for
-	// now.
+	// KnownTags aggregates the tag *vocabulary* in use across the fleet — keys
+	// and the values seen for them — never a server identifier or any other
+	// server attribute, so it does not let a restricted token enumerate which
+	// hosts exist. Filtering it would mean plumbing a selector through an
+	// aggregation query for a leak that carries no server identity; ruled
+	// acceptable to leave fleet-wide rather than take that on for this.
 	"GET /api/servers/tags": false,
-
-	// listServerVulnerabilities and getServerPackages read findings/package
-	// data keyed by server ID without checking the token's tag restriction —
-	// this task's interfaces (GetServerScoped, ResolveTargetsScoped) only
-	// wrap services.GetServer/ListServers/ResolveTargets, and these two
-	// handlers call services.ListFindings/ListPackages directly, so they are
-	// out of this task's chokepoints. This is a known, unresolved gap: a
-	// restricted token can currently read vulnerability/package data for a
-	// server outside its scope by ID. Flagged for a follow-up task rather
-	// than silently left off this map.
-	"GET /api/servers/:id/vulnerabilities": false,
-	"GET /api/servers/:id/packages":        false,
-
-	// getServerRunLog/streamServerRunLog read a run's per-server log by
-	// (runId, serverId) via services.HasServerRunLog/ReadServerRunLog, not
-	// through GetServer/ListServers/ResolveTargets, so — same as the
-	// vulnerabilities/packages routes above — they are outside this task's
-	// chokepoints. Known gap: a restricted token that already knows a runId
-	// can currently read log output for a serverId outside its scope.
-	// Flagged for a follow-up task.
-	"GET /api/runs/:runId/servers/:serverId/logs":        false,
-	"GET /api/runs/:runId/servers/:serverId/logs/stream": false,
-
-	// revokeAssignment calls services.RevokeAssignment(instanceID, keyID,
-	// serverID) directly and never resolves the server through GetServer, so
-	// it is also outside this task's chokepoints. Known gap: a restricted
-	// token could revoke a key assignment naming a serverId outside its
-	// scope. Flagged for a follow-up task.
-	"DELETE /api/keys/:id/assign/:serverId": false,
 }
 
 // AssertServerScopeMapComplete refuses to boot when a route touching server

server/internal/api/vulnerabilities.go

@@ -290,7 +290,14 @@ func writeFindingError(c *gin.Context, err error) {
 //	@Security		bearerAuth
 //	@Router			/servers/{id}/vulnerabilities [get]
 func listServerVulnerabilities(c *gin.Context) {
-	findings, err := services.ListFindings(c.Request.Context(), auth.InstanceID(c), c.Param("id"))
+	instanceID := auth.InstanceID(c)
+	id := c.Param("id")
+	if _, err := services.GetServerScoped(instanceID, id, auth.ServerScope(c)); err != nil {
+		c.JSON(http.StatusNotFound, gin.H{"error": "server not found"})
+		return
+	}
+
+	findings, err := services.ListFindings(c.Request.Context(), instanceID, id)
 	if err != nil {
 		c.JSON(http.StatusInternalServerError, gin.H{"error": err.Error()})
 		return
@@ -314,7 +321,14 @@ func listServerVulnerabilities(c *gin.Context) {
 //	@Security		bearerAuth
 //	@Router			/servers/{id}/packages [get]
 func getServerPackages(c *gin.Context) {
-	sp, err := services.ListPackages(auth.InstanceID(c), c.Param("id"))
+	instanceID := auth.InstanceID(c)
+	id := c.Param("id")
+	if _, err := services.GetServerScoped(instanceID, id, auth.ServerScope(c)); err != nil {
+		c.JSON(http.StatusNotFound, gin.H{"error": "server not found"})
+		return
+	}
+
+	sp, err := services.ListPackages(instanceID, id)
 	if err != nil {
 		c.JSON(http.StatusInternalServerError, gin.H{"error": err.Error()})
 		return

server/internal/api/workflows.go

@@ -67,6 +67,10 @@ func getServerRunLog(c *gin.Context) {
 		c.JSON(http.StatusBadRequest, gin.H{"error": "invalid id"})
 		return
 	}
+	if _, err := services.GetServerScoped(auth.InstanceID(c), serverID, auth.ServerScope(c)); err != nil {
+		c.JSON(http.StatusNotFound, gin.H{"error": "server not found"})
+		return
+	}
 	if !services.HasServerRunLog(runID, serverID) {
 		c.JSON(http.StatusNotFound, gin.H{"error": "no logs"})
 		return
@@ -117,6 +121,10 @@ func streamServerRunLog(c *gin.Context) {
 		c.JSON(http.StatusBadRequest, gin.H{"error": "invalid id"})
 		return
 	}
+	if _, err := services.GetServerScoped(auth.InstanceID(c), serverID, auth.ServerScope(c)); err != nil {
+		c.JSON(http.StatusNotFound, gin.H{"error": "server not found"})
+		return
+	}
 	c.Writer.Header().Set("Content-Type", "text/event-stream")
 	c.Writer.Header().Set("Cache-Control", "no-cache")
 	c.Writer.Header().Set("Connection", "keep-alive")