fix: address 6 security and reliability issues

Security fixes:
- Auto-register now requires settings:write scope for API tokens
- X-Forwarded-For in auto-register only trusted from verified proxies
- Public URL capture requires authentication (no loopback bypass)
- Lockout reset now uses RequireAdmin for session users

Reliability fixes:
- Docker stop command expiration clears PendingUninstall flag
- Cancelled notifications get completed_at set and are cleaned up

internal/api/config_handlers.go

@@ -5010,13 +5010,21 @@ func (h *ConfigHandlers) HandleAutoRegister(w http.ResponseWriter, r *http.Reque
 	if authCode != "" {
 		matchedAPIToken := false
 		if h.getConfig(r.Context()).HasAPITokens() {
-			if _, ok := h.getConfig(r.Context()).ValidateAPIToken(authCode); ok {
-				authenticated = true
-				matchedAPIToken = true
-				log.Info().
-					Str("type", req.Type).
-					Str("host", req.Host).
-					Msg("Auto-register authenticated via direct API token")
+			if record, ok := h.getConfig(r.Context()).ValidateAPIToken(authCode); ok {
+				// Require settings:write scope for auto-registration
+				if record.HasScope(config.ScopeSettingsWrite) {
+					authenticated = true
+					matchedAPIToken = true
+					log.Info().
+						Str("type", req.Type).
+						Str("host", req.Host).
+						Msg("Auto-register authenticated via direct API token")
+				} else {
+					log.Warn().
+						Str("type", req.Type).
+						Str("host", req.Host).
+						Msg("Auto-register rejected: API token missing settings:write scope")
+				}
 			}
 		}
 
@@ -5077,9 +5085,13 @@ func (h *ConfigHandlers) HandleAutoRegister(w http.ResponseWriter, r *http.Reque
 	// If not authenticated via setup code, check API token if configured
 	if !authenticated && h.getConfig(r.Context()).HasAPITokens() {
 		apiToken := r.Header.Get("X-API-Token")
-		if _, ok := h.getConfig(r.Context()).ValidateAPIToken(apiToken); ok {
-			authenticated = true
-			log.Info().Msg("Auto-register authenticated via API token")
+		if record, ok := h.getConfig(r.Context()).ValidateAPIToken(apiToken); ok {
+			if record.HasScope(config.ScopeSettingsWrite) {
+				authenticated = true
+				log.Info().Msg("Auto-register authenticated via API token")
+			} else {
+				log.Warn().Msg("Auto-register rejected: API token missing settings:write scope")
+			}
 		}
 	}
 
@@ -5101,8 +5113,12 @@ func (h *ConfigHandlers) HandleAutoRegister(w http.ResponseWriter, r *http.Reque
 
 	// Log source IP for security auditing
 	clientIP := r.RemoteAddr
-	if forwarded := r.Header.Get("X-Forwarded-For"); forwarded != "" {
-		clientIP = forwarded
+	// Only trust X-Forwarded-For if request comes from a trusted proxy
+	peerIP := extractRemoteIP(clientIP)
+	if isTrustedProxyIP(peerIP) {
+		if forwarded := r.Header.Get("X-Forwarded-For"); forwarded != "" {
+			clientIP = forwarded
+		}
 	}
 	log.Info().Str("clientIP", clientIP).Msg("Auto-register request from")
 

internal/api/router.go

@@ -251,7 +251,7 @@ func (r *Router) setupRoutes() {
 	r.licenseHandlers = NewLicenseHandlers(r.multiTenant)
 	// Wire license service provider so middleware can access per-tenant license services
 	SetLicenseServiceProvider(r.licenseHandlers)
-	r.reportingHandlers = NewReportingHandlers()
+	r.reportingHandlers = NewReportingHandlers(r.mtMonitor)
 	r.logHandlers = NewLogHandlers(r.config, r.persistence)
 	rbacHandlers := NewRBACHandlers(r.config)
 
@@ -3907,9 +3907,11 @@ func canCapturePublicURL(cfg *config.Config, req *http.Request) bool {
 		return false
 	}
 
-	if isDirectLoopbackRequest(req) {
-		return true
-	}
+	// Security fix: Do not auto-trust loopback requests for public URL capture.
+	// We require authentication even for loopback to prevent "poisoning" attacks.
+	// if isDirectLoopbackRequest(req) {
+	// 	return true
+	// }
 
 	return isRequestAuthenticated(cfg, req)
 }
@@ -4572,52 +4574,52 @@ func (r *Router) handleResetLockout(w http.ResponseWriter, req *http.Request) {
 		return
 	}
 
-	if !CheckAuth(r.config, w, req) {
-		return
-	}
-	if !ensureSettingsWriteScope(w, req) {
-		return
-	}
+	// Use RequireAdmin to ensure proper admin checks (including proxy auth) for session users
+	RequireAdmin(r.config, func(w http.ResponseWriter, req *http.Request) {
+		if !ensureSettingsWriteScope(w, req) {
+			return
+		}
 
-	// Parse request
-	var resetReq struct {
-		Identifier string `json:"identifier"` // Can be username or IP
-	}
+		// Parse request
+		var resetReq struct {
+			Identifier string `json:"identifier"` // Can be username or IP
+		}
 
-	if err := json.NewDecoder(req.Body).Decode(&resetReq); err != nil {
-		writeErrorResponse(w, http.StatusBadRequest, "invalid_request",
-			"Invalid request body", nil)
-		return
-	}
+		if err := json.NewDecoder(req.Body).Decode(&resetReq); err != nil {
+			writeErrorResponse(w, http.StatusBadRequest, "invalid_request",
+				"Invalid request body", nil)
+			return
+		}
 
-	if resetReq.Identifier == "" {
-		writeErrorResponse(w, http.StatusBadRequest, "missing_identifier",
-			"Identifier (username or IP) is required", nil)
-		return
-	}
+		if resetReq.Identifier == "" {
+			writeErrorResponse(w, http.StatusBadRequest, "missing_identifier",
+				"Identifier (username or IP) is required", nil)
+			return
+		}
 
-	// Reset the lockout
-	ResetLockout(resetReq.Identifier)
+		// Reset the lockout
+		ResetLockout(resetReq.Identifier)
 
-	// Also clear failed login attempts
-	ClearFailedLogins(resetReq.Identifier)
+		// Also clear failed login attempts
+		ClearFailedLogins(resetReq.Identifier)
 
-	// Audit log the reset
-	LogAuditEventForTenant(GetOrgID(req.Context()), "lockout_reset", "admin", GetClientIP(req), req.URL.Path, true,
-		fmt.Sprintf("Lockout reset for: %s", resetReq.Identifier))
+		// Audit log the reset
+		LogAuditEventForTenant(GetOrgID(req.Context()), "lockout_reset", "admin", GetClientIP(req), req.URL.Path, true,
+			fmt.Sprintf("Lockout reset for: %s", resetReq.Identifier))
 
-	log.Info().
-		Str("identifier", resetReq.Identifier).
-		Str("reset_by", "admin").
-		Str("ip", GetClientIP(req)).
-		Msg("Account lockout manually reset")
+		log.Info().
+			Str("identifier", resetReq.Identifier).
+			Str("reset_by", "admin").
+			Str("ip", GetClientIP(req)).
+			Msg("Account lockout manually reset")
 
-	// Return success
-	w.Header().Set("Content-Type", "application/json")
-	json.NewEncoder(w).Encode(map[string]interface{}{
-		"success": true,
-		"message": fmt.Sprintf("Lockout reset for %s", resetReq.Identifier),
-	})
+		// Return success
+		w.Header().Set("Content-Type", "application/json")
+		json.NewEncoder(w).Encode(map[string]interface{}{
+			"success": true,
+			"message": fmt.Sprintf("Lockout reset for %s", resetReq.Identifier),
+		})
+	})(w, req)
 }
 
 // handleState handles state requests

internal/monitoring/docker_commands.go

@@ -301,6 +301,12 @@ func (m *Monitor) getDockerCommandPayload(hostID string) (map[string]any, *model
 		cmd.status.UpdatedAt = now
 		cmd.status.FailureReason = "command expired before agent acknowledged it"
 		m.state.SetDockerHostCommand(hostID, &cmd.status)
+
+		// If this was a stop command (uninstall), clear the pending flag so the host isn't stuck
+		if cmd.status.Type == DockerCommandTypeStop {
+			m.state.SetDockerHostPendingUninstall(hostID, false)
+		}
+
 		log.Warn().
 			Str("dockerHostID", hostID).
 			Str("commandID", cmd.status.ID).

internal/notifications/queue.go

@@ -248,7 +248,7 @@ func (nq *NotificationQueue) UpdateStatus(id string, status NotificationQueueSta
 
 	now := time.Now().Unix()
 	var completedAt *int64
-	if status == QueueStatusSent || status == QueueStatusFailed || status == QueueStatusDLQ {
+	if status == QueueStatusSent || status == QueueStatusFailed || status == QueueStatusDLQ || status == QueueStatusCancelled {
 		completedAt = &now
 	}
 
@@ -674,8 +674,8 @@ func (nq *NotificationQueue) performCleanup() {
 	completedCutoff := time.Now().Add(-7 * 24 * time.Hour).Unix()
 	dlqCutoff := time.Now().Add(-30 * 24 * time.Hour).Unix()
 
-	// Clean completed/sent/failed
-	query := `DELETE FROM notification_queue WHERE status IN ('sent', 'failed') AND completed_at < ?`
+	// Clean completed/sent/failed/cancelled
+	query := `DELETE FROM notification_queue WHERE status IN ('sent', 'failed', 'cancelled') AND completed_at < ?`
 	result, err := nq.db.Exec(query, completedCutoff)
 	if err != nil {
 		log.Error().Err(err).Msg("Failed to cleanup old notifications")