[codex] Improve REST 4xx error responses

cmd/hub/main.go

@@ -300,6 +300,8 @@ func runWithRuntime(ctx context.Context, rt runtimeDeps) error {
 	}
 
 	r := chi.NewRouter()
+	r.NotFound(handlers.WriteNotFound)
+	r.MethodNotAllowed(handlers.WriteMethodNotAllowed)
 	r.Use(observability.RequestLogger(logger))
 	r.Use(auth.Middleware(authenticator, cfg.Auth, registry, r))
 
@@ -323,7 +325,10 @@ func runWithRuntime(ctx context.Context, rt runtimeDeps) error {
 		RPC:                   rpcBridge,
 		RequestBodyLimitBytes: cfg.HTTPRequestBodyLimitBytes,
 	})
-	gen.HandlerFromMux(h, r)
+	gen.HandlerWithOptions(h, gen.ChiServerOptions{
+		BaseRouter:       r,
+		ErrorHandlerFunc: handlers.WriteRequestError,
+	})
 	wsHub := ws.New(
 		logger,
 		service,

docs/auth.md

@@ -153,9 +153,22 @@ For `*_OWN` permissions, the request path parameter must be present in the match
 
 ## Error Handling
 
+- REST errors use the standard JSON envelope:
+
+```json
+{
+  "type": "bad_request",
+  "code": 400,
+  "message": "invalid request body",
+  "details": ["JSON syntax error at byte offset 18: invalid character '}' looking for beginning of object key string"]
+}
+```
+
 - `401 Unauthorized`: missing bearer header, malformed token, invalid signature, bad issuer, bad audience, expired token, or other authentication failure
-- `403 Forbidden`: authenticated token lacks a matching permission or ownership claim
-- `403 Forbidden` on RPC also covers missing method-level discovery or invocation permission
+- `400 Bad Request`: invalid JSON, multiple JSON documents, oversized request bodies, invalid path parameters, or service validation failures; the `details` array identifies the field, parameter, byte offset, limit, or validation rule where possible
+- `403 Forbidden`: authenticated token lacks a matching permission or ownership claim; REST responses include the required permission pair, the matched route rule when one exists, and ownership claim details when `_OWN` permissions are involved
+- `403 Forbidden` on RPC and WebSocket policy checks also names the missing discovery, invocation, subscribe, or publish rule
+- `404 Not Found`: unknown REST paths name the requested path; missing resources return the resource-specific not-found message
 - WebSocket auth failures are returned as OMLOX wrapper `error` events with code `10004`
 
 Authentication failures return a `WWW-Authenticate: Bearer` header and the API error body.

internal/auth/errors.go

@@ -11,6 +11,7 @@ type authError struct {
 	status    int
 	typ       string
 	message   string
+	details   []string
 	challenge string
 }
 
@@ -20,6 +21,7 @@ func (e *authError) Type() string  { return e.typ }
 func (e *authError) Message() string {
 	return e.message
 }
+func (e *authError) Details() []string { return e.details }
 
 func unauthorized(message string) error {
 	return &authError{
@@ -30,11 +32,12 @@ func unauthorized(message string) error {
 	}
 }
 
-func forbidden(message string) error {
+func forbidden(message string, details ...string) error {
 	return &authError{
 		status:  http.StatusForbidden,
 		typ:     "authorization_failed",
 		message: message,
+		details: details,
 	}
 }
 
@@ -53,9 +56,13 @@ func writeAuthError(w http.ResponseWriter, err error) {
 	w.Header().Set("Content-Type", "application/json")
 	w.WriteHeader(authErr.status)
 	msg := authErr.message
-	_ = json.NewEncoder(w).Encode(gen.ErrorResponse{
+	body := gen.ErrorResponse{
 		Type:    authErr.typ,
 		Code:    authErr.status,
 		Message: &msg,
-	})
+	}
+	if len(authErr.details) > 0 {
+		body.Details = &authErr.details
+	}
+	_ = json.NewEncoder(w).Encode(body)
 }

internal/auth/permissions.go

@@ -261,13 +261,14 @@ func validateRule(pattern string, perms map[Permission]struct{}) error {
 func (r *Registry) Authorize(req *http.Request, principal *Principal) error {
 	requiredAny, requiredOwn, err := permissionsForMethod(req.Method)
 	if err != nil {
-		return forbidden(err.Error())
+		return forbidden("request method is not authorized", err.Error())
 	}
 	if principal == nil {
 		return unauthorized("missing authenticated principal")
 	}
 
 	matched := false
+	var matchedDetails []string
 	for _, role := range principal.Roles {
 		policy := r.roles[role]
 		rule, params, ok := bestMatchingRule(policy.Routes, req.URL.Path)
@@ -281,11 +282,15 @@ func (r *Registry) Authorize(req *http.Request, principal *Principal) error {
 		if _, ok := rule.Permissions[requiredOwn]; ok && ownsAll(principal, params) {
 			return nil
 		}
+		matchedDetails = append(matchedDetails, authorizationFailureDetails(rule, params, principal, requiredAny, requiredOwn)...)
 	}
 	if !matched {
-		return forbidden("token does not grant access to this endpoint")
+		return forbidden("token does not grant access to this endpoint",
+			fmt.Sprintf("no authorization rule matched %s %s for the token roles", req.Method, req.URL.Path),
+			fmt.Sprintf("the request requires %s or %s on a matching route", requiredAny, requiredOwn),
+		)
 	}
-	return forbidden("token does not grant access to this endpoint")
+	return forbidden("token does not satisfy the authorization rule for this endpoint", matchedDetails...)
 }
 
 // AuthorizeRPCDiscover checks whether the principal may list reachable RPC methods.
@@ -298,7 +303,7 @@ func (r *Registry) AuthorizeRPCDiscover(principal *Principal) error {
 			return nil
 		}
 	}
-	return forbidden("token does not grant access to rpc method discovery")
+	return forbidden("token does not grant access to rpc method discovery", "the token roles must grant rpc.discover=true")
 }
 
 // AuthorizeRPCInvoke checks whether the principal may invoke the supplied RPC method.
@@ -311,7 +316,7 @@ func (r *Registry) AuthorizeRPCInvoke(principal *Principal, method string) error
 			return nil
 		}
 	}
-	return forbidden("token does not grant access to rpc method invocation")
+	return forbidden("token does not grant access to rpc method invocation", fmt.Sprintf("no rpc.invoke rule matched method %q", method))
 }
 
 // AuthorizeWebSocketSubscribe checks whether the principal may subscribe to
@@ -325,7 +330,7 @@ func (r *Registry) AuthorizeWebSocketSubscribe(principal *Principal, topic strin
 			return nil
 		}
 	}
-	return forbidden("token does not grant access to websocket topic subscription")
+	return forbidden("token does not grant access to websocket topic subscription", fmt.Sprintf("no websocket.subscribe rule matched topic %q", topic))
 }
 
 // AuthorizeWebSocketPublish checks whether the principal may publish to the
@@ -339,7 +344,54 @@ func (r *Registry) AuthorizeWebSocketPublish(principal *Principal, topic string)
 			return nil
 		}
 	}
-	return forbidden("token does not grant access to websocket topic publish")
+	return forbidden("token does not grant access to websocket topic publish", fmt.Sprintf("no websocket.publish rule matched topic %q", topic))
+}
+
+func authorizationFailureDetails(rule Rule, params map[string]string, principal *Principal, requiredAny, requiredOwn Permission) []string {
+	details := []string{
+		fmt.Sprintf("matched route pattern %q", rule.Pattern),
+		fmt.Sprintf("request requires %s, or %s with matching ownership claims", requiredAny, requiredOwn),
+		fmt.Sprintf("matched rule grants: %s", formatPermissions(rule.Permissions)),
+	}
+	if _, ok := rule.Permissions[requiredOwn]; ok {
+		details = append(details, ownershipFailureDetails(params, principal)...)
+	}
+	return details
+}
+
+func formatPermissions(perms map[Permission]struct{}) string {
+	if len(perms) == 0 {
+		return "<none>"
+	}
+	items := make([]string, 0, len(perms))
+	for perm := range perms {
+		items = append(items, string(perm))
+	}
+	sort.Strings(items)
+	return strings.Join(items, ", ")
+}
+
+func ownershipFailureDetails(params map[string]string, principal *Principal) []string {
+	if len(params) == 0 {
+		return []string{"ownership could not be evaluated because the route has no path parameters"}
+	}
+	details := make([]string, 0, len(params))
+	for name, value := range params {
+		key := ownedResourceKey(name)
+		values, ok := principal.OwnedResources[key]
+		switch {
+		case !ok:
+			details = append(details, fmt.Sprintf("ownership claim %q is missing for path parameter %q", key, name))
+		case values == nil:
+			details = append(details, fmt.Sprintf("ownership claim %q is empty for path parameter %q", key, name))
+		default:
+			if _, ok := values[value]; !ok {
+				details = append(details, fmt.Sprintf("ownership claim %q does not include requested %s value %q", key, name, value))
+			}
+		}
+	}
+	sort.Strings(details)
+	return details
 }
 
 func permissionsForMethod(method string) (Permission, Permission, error) {

internal/httpapi/handlers/handlers.go

@@ -4,12 +4,15 @@ import (
 	"context"
 	"encoding/json"
 	"errors"
+	"fmt"
 	"io"
 	"net/http"
+	"strings"
 
 	"github.com/formation-res/open-location-hub/internal/httpapi/gen"
 	"github.com/formation-res/open-location-hub/internal/hub"
 	"github.com/formation-res/open-location-hub/internal/observability"
+	"github.com/go-chi/chi/v5"
 	"go.uber.org/zap"
 )
 
@@ -618,15 +621,15 @@ func (h *Handler) PutRPC(w http.ResponseWriter, r *http.Request) {
 
 func decodeJSONBody(w http.ResponseWriter, r *http.Request, limit int64, dst any) error {
 	if err := decodeSingleJSONDocument(w, r, limit, dst); err != nil {
-		return &hub.HTTPError{Status: 400, Type: "bad_request", Message: "invalid request body"}
+		return badRequestError("invalid request body", describeBodyDecodeError(err, limit))
 	}
 	return nil
 }
 
 func readRawBody(w http.ResponseWriter, r *http.Request, limit int64) (json.RawMessage, error) {
 	var raw json.RawMessage
 	if err := decodeSingleJSONDocument(w, r, limit, &raw); err != nil {
-		return nil, &hub.HTTPError{Status: 400, Type: "bad_request", Message: "invalid request body"}
+		return nil, badRequestError("invalid request body", describeBodyDecodeError(err, limit))
 	}
 	return raw, nil
 }
@@ -666,13 +669,7 @@ func writeJSONOrError(w http.ResponseWriter, payload any, err error, successStat
 	if err != nil {
 		var httpErr *hub.HTTPError
 		if errors.As(err, &httpErr) {
-			w.Header().Set("Content-Type", "application/json")
-			w.WriteHeader(httpErr.Status)
-			_ = json.NewEncoder(w).Encode(gen.ErrorResponse{
-				Type:    httpErr.Type,
-				Code:    httpErr.Status,
-				Message: &httpErr.Message,
-			})
+			writeErrorResponse(w, httpErr.Status, httpErr.Type, httpErr.Message, httpErr.Details)
 			return
 		}
 		var authErr interface {
@@ -681,24 +678,18 @@ func writeJSONOrError(w http.ResponseWriter, payload any, err error, successStat
 			Message() string
 		}
 		if errors.As(err, &authErr) {
-			message := authErr.Message()
-			w.Header().Set("Content-Type", "application/json")
-			w.WriteHeader(authErr.Status())
-			_ = json.NewEncoder(w).Encode(gen.ErrorResponse{
-				Type:    authErr.Type(),
-				Code:    authErr.Status(),
-				Message: &message,
-			})
+			var detailed interface {
+				Details() []string
+			}
+			var details []string
+			if errors.As(err, &detailed) {
+				details = detailed.Details()
+			}
+			writeErrorResponse(w, authErr.Status(), authErr.Type(), authErr.Message(), details)
 			return
 		}
 		message := err.Error()
-		w.Header().Set("Content-Type", "application/json")
-		w.WriteHeader(http.StatusInternalServerError)
-		_ = json.NewEncoder(w).Encode(gen.ErrorResponse{
-			Type:    "internal_error",
-			Code:    http.StatusInternalServerError,
-			Message: &message,
-		})
+		writeErrorResponse(w, http.StatusInternalServerError, "internal_error", message, nil)
 		return
 	}
 	w.Header().Set("Content-Type", "application/json")
@@ -716,6 +707,83 @@ func writeStubNotImplemented(w http.ResponseWriter, message string) {
 	}, 0)
 }
 
+// WriteRequestError renders OpenAPI path and parameter validation failures as
+// the standard JSON error envelope.
+func WriteRequestError(w http.ResponseWriter, r *http.Request, err error) {
+	detail := err.Error()
+	var invalidParam *gen.InvalidParamFormatError
+	if errors.As(err, &invalidParam) {
+		detail = fmt.Sprintf("path parameter %q has invalid value %q: expected %s", invalidParam.ParamName, chiURLParam(r, invalidParam.ParamName), invalidParam.Err.Error())
+	}
+	writeErrorResponse(w, http.StatusBadRequest, "bad_request", "invalid request parameters", []string{detail})
+}
+
+// WriteNotFound renders unmatched routes as a JSON 404 that identifies the
+// requested method and path.
+func WriteNotFound(w http.ResponseWriter, r *http.Request) {
+	message := fmt.Sprintf("path %s does not exist", r.URL.Path)
+	details := []string{fmt.Sprintf("%s %s did not match any Open Location Hub REST route", r.Method, r.URL.Path)}
+	writeErrorResponse(w, http.StatusNotFound, "not_found", message, details)
+}
+
+// WriteMethodNotAllowed renders known paths with unsupported methods as JSON.
+func WriteMethodNotAllowed(w http.ResponseWriter, r *http.Request) {
+	message := fmt.Sprintf("method %s is not allowed for path %s", r.Method, r.URL.Path)
+	details := []string{"use one of the HTTP methods documented for this Open Location Hub REST path"}
+	writeErrorResponse(w, http.StatusMethodNotAllowed, "method_not_allowed", message, details)
+}
+
+func badRequestError(message string, details []string) error {
+	return &hub.HTTPError{Status: http.StatusBadRequest, Type: "bad_request", Message: message, Details: details}
+}
+
+func describeBodyDecodeError(err error, limit int64) []string {
+	switch {
+	case errors.Is(err, io.EOF):
+		return []string{"request body is empty; send a single JSON document"}
+	case strings.Contains(err.Error(), "http: request body too large"):
+		return []string{fmt.Sprintf("request body exceeds the configured limit of %d bytes", limit)}
+	}
+	var syntaxErr *json.SyntaxError
+	if errors.As(err, &syntaxErr) {
+		return []string{fmt.Sprintf("JSON syntax error at byte offset %d: %s", syntaxErr.Offset, syntaxErr.Error())}
+	}
+	var typeErr *json.UnmarshalTypeError
+	if errors.As(err, &typeErr) {
+		field := typeErr.Field
+		if field == "" {
+			field = typeErr.Value
+		}
+		return []string{fmt.Sprintf("JSON field %q has value type %q but must be %s", field, typeErr.Value, typeErr.Type.String())}
+	}
+	if err.Error() == "unexpected trailing JSON content" {
+		return []string{"request body must contain exactly one JSON document; remove trailing content after the first document"}
+	}
+	return []string{err.Error()}
+}
+
+func writeErrorResponse(w http.ResponseWriter, status int, typ, message string, details []string) {
+	w.Header().Set("Content-Type", "application/json")
+	w.WriteHeader(status)
+	body := gen.ErrorResponse{
+		Type:    typ,
+		Code:    status,
+		Message: &message,
+	}
+	if len(details) > 0 {
+		body.Details = &details
+	}
+	_ = json.NewEncoder(w).Encode(body)
+}
+
+func chiURLParam(r *http.Request, name string) string {
+	value := chi.URLParam(r, name)
+	if value == "" {
+		return "<empty>"
+	}
+	return value
+}
+
 func (h *Handler) extendedService() (extendedService, bool) {
 	svc, ok := h.deps.Service.(extendedService)
 	return svc, ok