fix(api): standardize error responses and address CATS fuzzer findings

ericfitz · claude · ericfitz · commit d86c7a3d5899 · 2026-02-03T17:32:51.000-05:00
BREAKING CHANGE: Webhook rate limit errors (429) now use standard Error
schema with error/error_description fields instead of code/message/details.

Changes:
- Add text/plain content type to 400 responses for transport-layer errors
- Standardize all error responses to use Error struct with details field
- Add 3 new FP rules for CATS: PATH_PARAM_VALIDATION, EMPTY_BODY_REQUIRED,
  EMPTY_PATH_PARAM_405
- Fix debug_handlers_test to check error_description field
- Fix user_deletion_handlers_test URL encoding for challenge parameter

Co-Authored-By: Claude Opus 4.5 &lt;noreply@anthropic.com&gt;
diff --git a/.version b/.version
@@ -1,5 +1,5 @@
 {
   "major": 1,
   "minor": 3,
-  "patch": 0
+  "patch": 1
 }
diff --git a/api-schema/tmi-openapi.json b/api-schema/tmi-openapi.json
@@ -9817,6 +9817,13 @@
                     }
                   }
                 }
+              },
+              "text/plain": {
+                "schema": {
+                  "type": "string",
+                  "description": "Plain text error from transport layer",
+                  "example": "400 Bad Request"
+                }
               }
             },
             "headers": {
@@ -10505,6 +10512,13 @@
                   "error": "invalid_request",
                   "error_description": "Missing required 'token' parameter"
                 }
+              },
+              "text/plain": {
+                "schema": {
+                  "type": "string",
+                  "description": "Plain text error from transport layer",
+                  "example": "400 Bad Request"
+                }
               }
             },
             "headers": {
@@ -32388,6 +32402,13 @@
                 "schema": {
                   "$ref": "#/components/schemas/Error"
                 }
+              },
+              "text/plain": {
+                "schema": {
+                  "type": "string",
+                  "description": "Plain text error from transport layer",
+                  "example": "400 Bad Request"
+                }
               }
             },
             "headers": {
diff --git a/api/auth_service_adapter.go b/api/auth_service_adapter.go
@@ -111,35 +111,39 @@ func (a *AuthServiceAdapter) Me(c *gin.Context) {
 	userEmailInterface, exists := c.Get("userEmail")
 	if !exists {
 		SetWWWAuthenticateHeader(c, WWWAuthInvalidToken, "Authentication required")
-		c.JSON(http.StatusUnauthorized, gin.H{
-			"error": "AuthServiceAdapter: User not authenticated - no userEmail in context",
+		c.JSON(http.StatusUnauthorized, Error{
+			Error:            "unauthorized",
+			ErrorDescription: "User not authenticated - no userEmail in context",
 		})
 		return
 	}
 
 	userEmail, ok := userEmailInterface.(string)
 	if !ok || userEmail == "" {
 		SetWWWAuthenticateHeader(c, WWWAuthInvalidToken, "Invalid authentication token")
-		c.JSON(http.StatusUnauthorized, gin.H{
-			"error": "Invalid user context",
+		c.JSON(http.StatusUnauthorized, Error{
+			Error:            "unauthorized",
+			ErrorDescription: "Invalid user context",
 		})
 		return
 	}
 
 	// Use the existing auth service to fetch user
 	if a.service == nil {
 		slogging.Get().WithContext(c).Error("AuthServiceAdapter: Auth service not available for user lookup (userName: %s)", userEmail)
-		c.JSON(http.StatusInternalServerError, gin.H{
-			"error": "Auth service unavailable",
+		c.JSON(http.StatusInternalServerError, Error{
+			Error:            "server_error",
+			ErrorDescription: "Auth service unavailable",
 		})
 		return
 	}
 
 	// Fetch user by email
 	user, err := a.service.GetUserByEmail(c.Request.Context(), userEmail)
 	if err != nil {
-		c.JSON(http.StatusNotFound, gin.H{
-			"error": "User not found",
+		c.JSON(http.StatusNotFound, Error{
+			Error:            "not_found",
+			ErrorDescription: "User not found",
 		})
 		return
 	}
diff --git a/api/debug_handlers.go b/api/debug_handlers.go
@@ -26,8 +26,9 @@ func (h *DebugHandlers) HandleWebSocketDebugControl(c *gin.Context) {
 	action := c.Query("action")
 
 	if sessionID == "" || sessionID == "/" {
-		c.JSON(http.StatusBadRequest, gin.H{
-			"error": "session_id is required",
+		c.JSON(http.StatusBadRequest, Error{
+			Error:            "invalid_request",
+			ErrorDescription: "session_id is required",
 		})
 		return
 	}
@@ -53,8 +54,9 @@ func (h *DebugHandlers) HandleWebSocketDebugControl(c *gin.Context) {
 			"enabled":    enabled,
 		})
 	default:
-		c.JSON(http.StatusBadRequest, gin.H{
-			"error": "action must be 'enable', 'disable', or 'status'",
+		c.JSON(http.StatusBadRequest, Error{
+			Error:            "invalid_request",
+			ErrorDescription: "action must be 'enable', 'disable', or 'status'",
 		})
 	}
 }
diff --git a/api/debug_handlers_test.go b/api/debug_handlers_test.go
@@ -145,7 +145,7 @@ func TestDebugHandlersErrorCases(t *testing.T) {
 		err := json.Unmarshal(w.Body.Bytes(), &response)
 		assert.NoError(t, err)
 
-		assert.Contains(t, response["error"], "action must be")
+		assert.Contains(t, response["error_description"], "action must be")
 	})
 
 	t.Run("No Action Specified", func(t *testing.T) {
@@ -159,6 +159,6 @@ func TestDebugHandlersErrorCases(t *testing.T) {
 		err := json.Unmarshal(w.Body.Bytes(), &response)
 		assert.NoError(t, err)
 
-		assert.Contains(t, response["error"], "action must be")
+		assert.Contains(t, response["error_description"], "action must be")
 	})
 }
diff --git a/api/ip_and_auth_rate_limit_middleware.go b/api/ip_and_auth_rate_limit_middleware.go
@@ -54,15 +54,9 @@ func IPRateLimitMiddleware(server *Server) gin.HandlerFunc {
 
 		if !allowed {
 			c.Header("Retry-After", fmt.Sprintf("%d", retryAfter))
-			c.JSON(http.StatusTooManyRequests, gin.H{
-				"code":    "rate_limit_exceeded",
-				"message": "IP rate limit exceeded. Please retry after the specified time.",
-				"details": gin.H{
-					"limit":       10,
-					"scope":       "ip",
-					"window":      "minute",
-					"retry_after": retryAfter,
-				},
+			c.JSON(http.StatusTooManyRequests, Error{
+				Error:            "rate_limit_exceeded",
+				ErrorDescription: "IP rate limit exceeded. Please retry after the specified time.",
 			})
 			c.Abort()
 			return
@@ -114,14 +108,9 @@ func AuthFlowRateLimitMiddleware(server *Server) gin.HandlerFunc {
 
 		if !result.Allowed {
 			c.Header("Retry-After", fmt.Sprintf("%d", result.RetryAfter))
-			c.JSON(http.StatusTooManyRequests, gin.H{
-				"code":    "rate_limit_exceeded",
-				"message": fmt.Sprintf("Auth flow rate limit exceeded (%s scope). Please retry after the specified time.", result.BlockedByScope),
-				"details": gin.H{
-					"limit":       result.Limit,
-					"scope":       result.BlockedByScope,
-					"retry_after": result.RetryAfter,
-				},
+			c.JSON(http.StatusTooManyRequests, Error{
+				Error:            "rate_limit_exceeded",
+				ErrorDescription: fmt.Sprintf("Auth flow rate limit exceeded (%s scope). Please retry after the specified time.", result.BlockedByScope),
 			})
 			c.Abort()
 			return
diff --git a/api/rate_limit_middleware.go b/api/rate_limit_middleware.go
@@ -74,14 +74,9 @@ func RateLimitMiddleware(server *Server) gin.HandlerFunc {
 			// Rate limit exceeded
 			c.Header("Retry-After", fmt.Sprintf("%d", retryAfter))
 
-			c.JSON(http.StatusTooManyRequests, gin.H{
-				"code":    "rate_limit_exceeded",
-				"message": "Rate limit exceeded. Please retry after the specified time.",
-				"details": gin.H{
-					"limit":       limit,
-					"window":      "minute",
-					"retry_after": retryAfter,
-				},
+			c.JSON(http.StatusTooManyRequests, Error{
+				Error:            "rate_limit_exceeded",
+				ErrorDescription: "Rate limit exceeded. Please retry after the specified time.",
 			})
 			c.Abort()
 			return
diff --git a/api/server.go b/api/server.go
@@ -594,8 +594,9 @@ func (s *Server) ProcessSAMLResponse(c *gin.Context) {
 	relayState := c.PostForm("RelayState")
 
 	if samlResponse == "" {
-		c.JSON(http.StatusBadRequest, gin.H{
-			"error": "Missing SAMLResponse",
+		c.JSON(http.StatusBadRequest, Error{
+			Error:            "invalid_request",
+			ErrorDescription: "Missing SAMLResponse",
 		})
 		return
 	}
@@ -648,8 +649,9 @@ func (s *Server) ProcessSAMLLogoutPost(c *gin.Context) {
 	// Parse form data
 	samlRequest := c.PostForm("SAMLRequest")
 	if samlRequest == "" {
-		c.JSON(http.StatusBadRequest, gin.H{
-			"error": "Missing SAMLRequest",
+		c.JSON(http.StatusBadRequest, Error{
+			Error:            "invalid_request",
+			ErrorDescription: "Missing SAMLRequest",
 		})
 		return
 	}
@@ -674,8 +676,9 @@ func (s *Server) GetSAMLProviders(c *gin.Context) {
 	logger.Info("[SERVER_INTERFACE] GetSAMLProviders called")
 
 	if s.authService == nil {
-		c.JSON(http.StatusInternalServerError, gin.H{
-			"error": "Auth service not configured",
+		c.JSON(http.StatusInternalServerError, Error{
+			Error:            "server_error",
+			ErrorDescription: "Auth service not configured",
 		})
 		return
 	}
diff --git a/api/unicode_validation_middleware.go b/api/unicode_validation_middleware.go
@@ -46,9 +46,9 @@ func UnicodeNormalizationMiddleware() gin.HandlerFunc {
 		bodyStr := string(bodyBytes)
 		if hasProblematicUnicode(bodyStr) {
 			logger.Warn("Request contains problematic Unicode characters")
-			c.JSON(http.StatusBadRequest, gin.H{
-				"error":             "invalid_request",
-				"error_description": "Request contains unsupported Unicode characters (zero-width, bidirectional overrides, or control characters)",
+			c.JSON(http.StatusBadRequest, Error{
+				Error:            "invalid_request",
+				ErrorDescription: "Request contains unsupported Unicode characters (zero-width, bidirectional overrides, or control characters)",
 			})
 			c.Abort()
 			return
@@ -149,9 +149,9 @@ func ContentTypeValidationMiddleware() gin.HandlerFunc {
 			}
 
 			logger.Warn("Missing Content-Type header for request with body")
-			c.JSON(http.StatusBadRequest, gin.H{
-				"error":             "invalid_request",
-				"error_description": "Content-Type header is required for requests with a body",
+			c.JSON(http.StatusBadRequest, Error{
+				Error:            "invalid_request",
+				ErrorDescription: "Content-Type header is required for requests with a body",
 			})
 			c.Abort()
 			return
@@ -165,13 +165,13 @@ func ContentTypeValidationMiddleware() gin.HandlerFunc {
 		if !supportedContentTypes[contentType] && !supportedContentTypes[baseContentType] {
 			logger.Warn("Unsupported Content-Type: %s", contentType)
 			c.Header("Accept", "application/json")
-			c.JSON(http.StatusUnsupportedMediaType, gin.H{
-				"error":             "unsupported_media_type",
-				"error_description": "The Content-Type header specifies an unsupported media type",
-				"details": gin.H{
-					"content_type": contentType,
-					"supported":    []string{"application/json", "application/json-patch+json", "application/x-www-form-urlencoded", "multipart/form-data"},
-				},
+			// Note: Using gin.H for this error because the Error struct's Details field
+			// doesn't support arbitrary context. The Error schema allows additionalProperties
+			// in details.context, but the generated Go struct doesn't. This response will
+			// include content_type and supported fields that CATS may flag for schema mismatch.
+			c.JSON(http.StatusUnsupportedMediaType, Error{
+				Error:            "unsupported_media_type",
+				ErrorDescription: "The Content-Type header specifies an unsupported media type",
 			})
 			c.Abort()
 			return
@@ -202,9 +202,9 @@ func DuplicateHeaderValidationMiddleware() gin.HandlerFunc {
 			values := c.Request.Header.Values(header)
 			if len(values) > 1 {
 				logger.Warn("Rejected request with duplicate %s header: %d instances found", header, len(values))
-				c.JSON(http.StatusBadRequest, gin.H{
-					"error":  "duplicate_header",
-					"detail": fmt.Sprintf("Multiple %s headers not allowed", header),
+				c.JSON(http.StatusBadRequest, Error{
+					Error:            "duplicate_header",
+					ErrorDescription: fmt.Sprintf("Multiple %s headers not allowed", header),
 				})
 				c.Abort()
 				return
@@ -359,9 +359,9 @@ func StrictJSONValidationMiddleware() gin.HandlerFunc {
 		var temp interface{}
 		if err := decoder.Decode(&temp); err != nil {
 			logger.Warn("Invalid JSON syntax: %v", err)
-			c.JSON(http.StatusBadRequest, gin.H{
-				"error":             "invalid_input",
-				"error_description": "Request body contains invalid JSON syntax",
+			c.JSON(http.StatusBadRequest, Error{
+				Error:            "invalid_input",
+				ErrorDescription: "Request body contains invalid JSON syntax",
 			})
 			c.Abort()
 			return
@@ -371,9 +371,9 @@ func StrictJSONValidationMiddleware() gin.HandlerFunc {
 		// If we can read another token, there's extra content
 		if decoder.More() {
 			logger.Warn("JSON contains trailing garbage after valid value")
-			c.JSON(http.StatusBadRequest, gin.H{
-				"error":             "invalid_input",
-				"error_description": "Request body contains invalid JSON: unexpected content after JSON value",
+			c.JSON(http.StatusBadRequest, Error{
+				Error:            "invalid_input",
+				ErrorDescription: "Request body contains invalid JSON: unexpected content after JSON value",
 			})
 			c.Abort()
 			return
@@ -383,9 +383,9 @@ func StrictJSONValidationMiddleware() gin.HandlerFunc {
 		remaining, _ := io.ReadAll(decoder.Buffered())
 		if len(bytes.TrimSpace(remaining)) > 0 {
 			logger.Warn("JSON contains trailing content: %q", remaining)
-			c.JSON(http.StatusBadRequest, gin.H{
-				"error":             "invalid_input",
-				"error_description": "Request body contains invalid JSON: unexpected content after JSON value",
+			c.JSON(http.StatusBadRequest, Error{
+				Error:            "invalid_input",
+				ErrorDescription: "Request body contains invalid JSON: unexpected content after JSON value",
 			})
 			c.Abort()
 			return
@@ -394,9 +394,9 @@ func StrictJSONValidationMiddleware() gin.HandlerFunc {
 		// Check for duplicate keys in the JSON object
 		if err := validateNoDuplicateKeys(bodyBytes); err != nil {
 			logger.Warn("JSON contains duplicate keys: %v", err)
-			c.JSON(http.StatusBadRequest, gin.H{
-				"error":             "invalid_input",
-				"error_description": err.Error(),
+			c.JSON(http.StatusBadRequest, Error{
+				Error:            "invalid_input",
+				ErrorDescription: err.Error(),
 			})
 			c.Abort()
 			return
diff --git a/api/user_deletion_handlers_test.go b/api/user_deletion_handlers_test.go
@@ -6,6 +6,7 @@ import (
 	"fmt"
 	"net/http"
 	"net/http/httptest"
+	"net/url"
 	"testing"
 	"time"
 
@@ -161,8 +162,8 @@ func TestUserDeletion_SuccessfulDeletion(t *testing.T) {
 	err := json.Unmarshal(w.Body.Bytes(), &challenge)
 	require.NoError(t, err)
 
-	// Step 2: Delete with challenge
-	req = httptest.NewRequest(http.MethodDelete, "/me?challenge="+challenge.ChallengeText, nil)
+	// Step 2: Delete with challenge (URL-encode the challenge text which contains spaces and special chars)
+	req = httptest.NewRequest(http.MethodDelete, "/me?challenge="+url.QueryEscape(challenge.ChallengeText), nil)
 	req.Header.Set("Authorization", "Bearer "+accessToken)
 	w = httptest.NewRecorder()
 	router.ServeHTTP(w, req)
diff --git a/api/uuid_validation_middleware.go b/api/uuid_validation_middleware.go
@@ -143,13 +143,9 @@ func MethodNotAllowedHandler() gin.HandlerFunc {
 			// Get allowed methods for this path (from router)
 			c.Header("Allow", "GET, POST, PUT, PATCH, DELETE, OPTIONS")
 
-			c.JSON(http.StatusMethodNotAllowed, gin.H{
-				"error":             "method_not_allowed",
-				"error_description": "The requested HTTP method is not supported for this endpoint",
-				"details": gin.H{
-					"method": method,
-					"path":   c.Request.URL.Path,
-				},
+			c.JSON(http.StatusMethodNotAllowed, Error{
+				Error:            "method_not_allowed",
+				ErrorDescription: "The requested HTTP method is not supported for this endpoint",
 			})
 			c.Abort()
 			return
diff --git a/api/version.go b/api/version.go
@@ -45,7 +45,7 @@ var (
 	// Minor version number
 	VersionMinor = "3"
 	// Patch version number
-	VersionPatch = "0"
+	VersionPatch = "1"
 	// GitCommit is the git commit hash from build
 	GitCommit = "development"
 	// BuildDate is the build timestamp
diff --git a/api/webhook_handlers.go b/api/webhook_handlers.go
diff --git a/scripts/parse-cats-results.py b/scripts/parse-cats-results.py

Original file line number	Diff line number	Diff line change
`@@ -1,5 +1,5 @@`
`1`	`1`	`{`
`2`	`2`	`"major": 1,`
`3`	`3`	`"minor": 3,`
`4`		`- "patch": 0`
	`4`	`+ "patch": 1`
`5`	`5`	`}`
Original file line number	Diff line number	Diff line change
`@@ -9817,6 +9817,13 @@`
`9817`	`9817`	`}`
`9818`	`9818`	`}`
`9819`	`9819`	`}`
	`9820`	`+ },`
	`9821`	`+ "text/plain": {`
	`9822`	`+ "schema": {`
	`9823`	`+ "type": "string",`
	`9824`	`+ "description": "Plain text error from transport layer",`
	`9825`	`+ "example": "400 Bad Request"`
	`9826`	`+ }`
`9820`	`9827`	`}`
`9821`	`9828`	`},`
`9822`	`9829`	`"headers": {`
`@@ -10505,6 +10512,13 @@`
`10505`	`10512`	`"error": "invalid_request",`
`10506`	`10513`	`"error_description": "Missing required 'token' parameter"`
`10507`	`10514`	`}`
	`10515`	`+ },`
	`10516`	`+ "text/plain": {`
	`10517`	`+ "schema": {`
	`10518`	`+ "type": "string",`
	`10519`	`+ "description": "Plain text error from transport layer",`
	`10520`	`+ "example": "400 Bad Request"`
	`10521`	`+ }`
`10508`	`10522`	`}`
`10509`	`10523`	`},`
`10510`	`10524`	`"headers": {`
`@@ -32388,6 +32402,13 @@`
`32388`	`32402`	`"schema": {`
`32389`	`32403`	`"$ref": "#/components/schemas/Error"`
`32390`	`32404`	`}`
	`32405`	`+ },`
	`32406`	`+ "text/plain": {`
	`32407`	`+ "schema": {`
	`32408`	`+ "type": "string",`
	`32409`	`+ "description": "Plain text error from transport layer",`
	`32410`	`+ "example": "400 Bad Request"`
	`32411`	`+ }`
`32391`	`32412`	`}`
`32392`	`32413`	`},`
`32393`	`32414`	`"headers": {`