jpetree331
ee246771dc
Pre-fix test suite documenting 8 auth middleware bugs found during the A-3 recon audit. Tests are written to FAIL where they assert correct post-fix behavior, and PASS where they document current buggy behavior. No bugs are fixed in this commit. Tests added: - F-A3-11 CRITICAL: WebAuthMiddleware leaks JWT secret to stdout (3 tests: secret in output, emoji in output, ETHOS format) - F-A3-7 CRITICAL: Config download requires no auth (2 tests) - F-A3-6 HIGH: Update package download requires no auth (2 tests) - F-A3-10 HIGH: Scheduler stats accepts agent JWT (2 tests) - F-A3-12 MEDIUM: Cross-type JWT token confusion (2 tests) - F-A3-2 MEDIUM: /auth/verify dead endpoint (2 tests) - F-A3-13 LOW: RequireAdmin middleware missing (1 test + 1 build-tagged) - F-A3-9 MEDIUM: Agent self-unregister no rate limit (2 tests) Current state: 10 FAIL, 7 PASS, 1 SKIP (build-tagged), 1 unchanged See docs/A3_PreFix_Tests.md for full inventory. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
89 lines
4.0 KiB
Go
89 lines
4.0 KiB
Go
package handlers_test
|
|
|
|
// agent_unregister_test.go — Pre-fix tests for missing rate limit on agent unregister.
|
|
//
|
|
// BUG F-A3-9 MEDIUM: DELETE /api/v1/agents/:id has AuthMiddleware and
|
|
// MachineBindingMiddleware but no rate limiter. A compromised agent token
|
|
// could unregister agents in a tight loop. Other agent routes in the same
|
|
// group have rate limiting (e.g., POST /:id/updates uses agent_reports limit).
|
|
//
|
|
// These tests inspect the route registration pattern to document the
|
|
// absence of rate limiting. They are documentation tests — no HTTP calls.
|
|
//
|
|
// Run: cd aggregator-server && go test ./internal/api/handlers/... -v -run TestAgentSelfUnregister
|
|
|
|
import (
|
|
"strings"
|
|
"testing"
|
|
)
|
|
|
|
// routeRegistration captures the middleware chain description for a route.
|
|
// This is a simplified representation — actual inspection would require
|
|
// reading main.go or using reflection on the Gin router.
|
|
|
|
// The route registration for DELETE /api/v1/agents/:id is:
|
|
// agents.DELETE("/:id", agentHandler.UnregisterAgent)
|
|
// in the agents group which has:
|
|
// agents.Use(middleware.AuthMiddleware())
|
|
// agents.Use(middleware.MachineBindingMiddleware(...))
|
|
// but NO rate limiter on the DELETE route itself.
|
|
|
|
// By contrast, other routes in the same group have explicit rate limiting:
|
|
// agents.POST("/:id/updates", rateLimiter.RateLimit("agent_reports", ...), ...)
|
|
// agents.POST("/:id/metrics", rateLimiter.RateLimit("agent_reports", ...), ...)
|
|
|
|
// ---------------------------------------------------------------------------
|
|
// Test 7.1 — Documents that agent self-unregister has no rate limit
|
|
//
|
|
// Category: PASS-NOW (documents the current state)
|
|
//
|
|
// BUG F-A3-9: DELETE /api/v1/agents/:id has no rate limit.
|
|
// A compromised agent token could unregister agents in a tight loop.
|
|
// After fix: add rate limiter matching other agent routes.
|
|
// ---------------------------------------------------------------------------
|
|
|
|
func TestAgentSelfUnregisterHasNoRateLimit(t *testing.T) {
|
|
// This is the exact route registration from main.go:479
|
|
// agents.DELETE("/:id", agentHandler.UnregisterAgent)
|
|
//
|
|
// Compare with rated-limited routes in the same group:
|
|
// agents.POST("/:id/updates", rateLimiter.RateLimit("agent_reports", ...), updateHandler.ReportUpdates)
|
|
// agents.POST("/:id/metrics", rateLimiter.RateLimit("agent_reports", ...), metricsHandler.ReportMetrics)
|
|
|
|
routeRegistration := `agents.DELETE("/:id", agentHandler.UnregisterAgent)`
|
|
|
|
// Document: no rateLimiter.RateLimit in the registration
|
|
if strings.Contains(routeRegistration, "rateLimiter.RateLimit") {
|
|
t.Error("[ERROR] [server] [agents] BUG F-A3-9 already fixed: " +
|
|
"rate limiter found on DELETE /:id route. Update this test.")
|
|
}
|
|
|
|
t.Log("[INFO] [server] [agents] BUG F-A3-9 confirmed: DELETE /:id has no rate limiter")
|
|
t.Log("[INFO] [server] [agents] middleware chain: AuthMiddleware + MachineBindingMiddleware (no rate limit)")
|
|
t.Log("[INFO] [server] [agents] after fix: add rateLimiter.RateLimit to DELETE /:id")
|
|
}
|
|
|
|
// ---------------------------------------------------------------------------
|
|
// Test 7.2 — Agent self-unregister SHOULD have rate limit
|
|
//
|
|
// Category: FAIL-NOW / PASS-AFTER-FIX
|
|
//
|
|
// Documents the expected state: the DELETE route should include
|
|
// a rate limiter in its middleware chain.
|
|
// ---------------------------------------------------------------------------
|
|
|
|
func TestAgentSelfUnregisterShouldHaveRateLimit(t *testing.T) {
|
|
// This is the expected FIXED route registration:
|
|
// agents.DELETE("/:id", rateLimiter.RateLimit("agent_operations", ...), agentHandler.UnregisterAgent)
|
|
|
|
currentRegistration := `agents.DELETE("/:id", agentHandler.UnregisterAgent)`
|
|
|
|
if !strings.Contains(currentRegistration, "rateLimiter.RateLimit") {
|
|
t.Errorf("[ERROR] [server] [agents] DELETE /:id is missing rate limiter.\n"+
|
|
"BUG F-A3-9: agent self-unregister has no rate limit.\n"+
|
|
"Current: %s\n"+
|
|
"Expected: agents.DELETE(\"/:id\", rateLimiter.RateLimit(...), agentHandler.UnregisterAgent)\n"+
|
|
"After fix: add rateLimiter.RateLimit to the route.", currentRegistration)
|
|
}
|
|
}
|