fix(server): answer every process-generated error with the envelope

docs/guides/API_GUIDE.md

@@ -373,6 +373,14 @@ rate-limited per client IP and return `429` with a `Retry-After` header over
 the limit. There is no `422` validation status: validation failures return
 `400` with the envelope above.
 
+Every error the OpenWatch process generates carries the envelope, including
+a `404` for an `/api/` path that does not exist, a `405`, and a `400` for a
+query parameter that is missing or fails to parse; the message names the
+parameter and never repeats the rejected value. A proxy or load balancer in
+front of OpenWatch produces its own `502`, `503` or `504` bodies, so a client
+should treat any non-2xx whose body is not JSON as an infrastructure error
+rather than fail on the parse.
+
 ---
 
 ## Operations: the CLI and systemd

internal/server/request_errors.go

@@ -0,0 +1,61 @@
+package server
+
+import (
+	"errors"
+	"net/http"
+
+	"github.com/Hanalyx/openwatch/internal/server/api"
+)
+
+// requestErrorHandler turns the generated router's parameter-binding
+// failures into the canonical envelope. The generator's default answers
+// text/plain with the parser's own error text, which repeats the rejected
+// value back to the client. The envelope names the parameter and nothing
+// else: the value is the client's own input and the parser's wording is
+// internal detail. Spec: system-http-server C-15.
+func requestErrorHandler(w http.ResponseWriter, _ *http.Request, err error) {
+	var (
+		required  *api.RequiredParamError
+		reqHeader *api.RequiredHeaderError
+		format    *api.InvalidParamFormatError
+		tooMany   *api.TooManyValuesForParamError
+		cookie    *api.UnescapedCookieParamError
+		unmarshal *api.UnmarshalingParamError
+	)
+	switch {
+	case errors.As(err, &required):
+		writeError(w, http.StatusBadRequest, "request.missing_parameter", "client",
+			"parameter "+required.ParamName+" is required", false)
+	case errors.As(err, &reqHeader):
+		writeError(w, http.StatusBadRequest, "request.missing_parameter", "client",
+			"header "+reqHeader.ParamName+" is required", false)
+	case errors.As(err, &format):
+		writeError(w, http.StatusBadRequest, "request.invalid_parameter", "client",
+			"parameter "+format.ParamName+" is not valid", false)
+	case errors.As(err, &tooMany):
+		writeError(w, http.StatusBadRequest, "request.invalid_parameter", "client",
+			"parameter "+tooMany.ParamName+" was given more than once", false)
+	case errors.As(err, &cookie):
+		writeError(w, http.StatusBadRequest, "request.invalid_parameter", "client",
+			"cookie "+cookie.ParamName+" is not valid", false)
+	case errors.As(err, &unmarshal):
+		writeError(w, http.StatusBadRequest, "request.invalid_parameter", "client",
+			"parameter "+unmarshal.ParamName+" is not valid", false)
+	default:
+		writeError(w, http.StatusBadRequest, "request.invalid", "client",
+			"the request could not be parsed", false)
+	}
+}
+
+// writeNotFound is the envelope for an /api/ path no route matches.
+func writeNotFound(w http.ResponseWriter) {
+	writeError(w, http.StatusNotFound, "request.not_found", "client",
+		"no such API route", false)
+}
+
+// writeMethodNotAllowed is the envelope for a route that does not accept
+// the request's method.
+func writeMethodNotAllowed(w http.ResponseWriter) {
+	writeError(w, http.StatusMethodNotAllowed, "request.method_not_allowed", "client",
+		"the route does not accept this method", false)
+}

internal/server/request_errors_test.go

@@ -0,0 +1,129 @@
+// @spec system-http-server
+package server
+
+import (
+	"encoding/json"
+	"io"
+	"net/http"
+	"os"
+	"path/filepath"
+	"regexp"
+	"strings"
+	"testing"
+
+	"github.com/Hanalyx/openwatch/internal/auth"
+)
+
+// Spec: specs/system/http-server.spec.yaml
+//
+//	AC-20  TestRequestErrors_EnvelopeEverywhere
+
+type envelopeBody struct {
+	Error struct {
+		Code          string `json:"code"`
+		Fault         string `json:"fault"`
+		HumanMessage  string `json:"human_message"`
+		CorrelationID string `json:"correlation_id"`
+	} `json:"error"`
+}
+
+func readEnvelope(t *testing.T, resp *http.Response) (envelopeBody, string) {
+	t.Helper()
+	raw, err := io.ReadAll(resp.Body)
+	if err != nil {
+		t.Fatalf("read body: %v", err)
+	}
+	var env envelopeBody
+	if err := json.Unmarshal(raw, &env); err != nil {
+		t.Fatalf("body is not the envelope: %v; body=%q", err, raw)
+	}
+	return env, string(raw)
+}
+
+// @ac AC-20
+// AC-20: every 4xx the process itself generates is the envelope, including
+// the three that no handler produces: an unmatched /api/ path, a method the
+// route does not accept, and a parameter that fails to bind. The parameter
+// message names the parameter and never repeats the rejected value.
+func TestRequestErrors_EnvelopeEverywhere(t *testing.T) {
+	t.Run("system-http-server/AC-20", func(t *testing.T) {
+		srv, _ := freshAPIServer(t)
+
+		cases := []struct {
+			name      string
+			req       *http.Request
+			status    int
+			code      string
+			mentions  string
+			forbidden string
+		}{
+			{
+				name:   "unmatched /api/ path is a 404 envelope",
+				req:    asRole(t, "GET", srv+"/api/v1/definitely-not-a-route", auth.RoleViewer, nil),
+				status: http.StatusNotFound, code: "request.not_found",
+			},
+			{
+				name:   "method the route does not accept is a 405 envelope",
+				req:    asRole(t, "DELETE", srv+"/api/v1/health", auth.RoleViewer, nil),
+				status: http.StatusMethodNotAllowed, code: "request.method_not_allowed",
+			},
+			{
+				name:   "parameter that fails to bind is a 400 envelope naming the parameter only",
+				req:    asRole(t, "GET", srv+"/api/v1/audit/events?limit=abc", auth.RoleViewer, nil),
+				status: http.StatusBadRequest, code: "request.invalid_parameter",
+				mentions: "limit", forbidden: "abc",
+			},
+		}
+		for _, tc := range cases {
+			t.Run(tc.name, func(t *testing.T) {
+				resp := doReq(t, tc.req)
+				defer resp.Body.Close()
+				if resp.StatusCode != tc.status {
+					t.Fatalf("status = %d, want %d", resp.StatusCode, tc.status)
+				}
+				if ct := resp.Header.Get("Content-Type"); !strings.HasPrefix(ct, "application/json") {
+					t.Errorf("Content-Type = %q, want application/json", ct)
+				}
+				if resp.Header.Get("X-Correlation-Id") == "" {
+					t.Errorf("no X-Correlation-Id header on the error response")
+				}
+				env, raw := readEnvelope(t, resp)
+				if env.Error.Code != tc.code {
+					t.Errorf("error.code = %q, want %q", env.Error.Code, tc.code)
+				}
+				if env.Error.Fault != "client" {
+					t.Errorf("error.fault = %q, want client", env.Error.Fault)
+				}
+				if tc.mentions != "" && !strings.Contains(env.Error.HumanMessage, tc.mentions) {
+					t.Errorf("human_message %q does not name %q", env.Error.HumanMessage, tc.mentions)
+				}
+				if tc.forbidden != "" && strings.Contains(raw, tc.forbidden) {
+					t.Errorf("body echoes the rejected value %q: %s", tc.forbidden, raw)
+				}
+			})
+		}
+
+		// Structural half: no bare http.Error in the package outside tests
+		// and generated code, so a new plain-text error cannot be added
+		// without this criterion noticing.
+		t.Run("no bare http.Error in internal/server", func(t *testing.T) {
+			files, err := filepath.Glob("*.go")
+			if err != nil {
+				t.Fatal(err)
+			}
+			bare := regexp.MustCompile(`\bhttp\.Error\(`)
+			for _, f := range files {
+				if strings.HasSuffix(f, "_test.go") {
+					continue
+				}
+				src, err := os.ReadFile(f)
+				if err != nil {
+					t.Fatal(err)
+				}
+				if bare.Match(src) {
+					t.Errorf("%s calls http.Error; use writeError so the response is the envelope (C-15)", f)
+				}
+			}
+		})
+	})
+}

internal/server/server.go

@@ -307,7 +307,7 @@ func New(cfg *config.Config, pool *pgxpool.Pool) *Server {
 	// fallback) for everything else.
 	r.NotFound(newSPAHandler().ServeHTTP)
 	r.MethodNotAllowed(func(w http.ResponseWriter, _ *http.Request) {
-		http.Error(w, "405 method not allowed", http.StatusMethodNotAllowed)
+		writeMethodNotAllowed(w) // C-15: the envelope, not text/plain
 	})
 
 	// Mount the Stage-0 API routes via oapi-codegen's HandlerFromMux.
@@ -341,7 +341,12 @@ func New(cfg *config.Config, pool *pgxpool.Pool) *Server {
 		}
 		primeCancel()
 	}
-	api.HandlerFromMux(apiHandlers, r)
+	// ErrorHandlerFunc replaces the generator's text/plain default for a
+	// parameter that is missing or fails to parse (C-15).
+	api.HandlerWithOptions(apiHandlers, api.ChiServerOptions{
+		BaseRouter:       r,
+		ErrorHandlerFunc: requestErrorHandler,
+	})
 	_ = license.PremiumDiagnostics // ensure import is exercised
 
 	// OpenAPI spec + Swagger UI. The handlers do not call

internal/server/spa.go

@@ -158,12 +158,12 @@ func contentTypeFor(p string) string {
 
 func (h *spaHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
 	if strings.HasPrefix(r.URL.Path, "/api/") {
-		http.Error(w, "404 page not found", http.StatusNotFound)
+		writeNotFound(w) // C-15: the envelope, not text/plain
 		return
 	}
 	if r.Method != http.MethodGet && r.Method != http.MethodHead {
 		w.Header().Set("Allow", "GET, HEAD")
-		http.Error(w, "method not allowed", http.StatusMethodNotAllowed)
+		writeMethodNotAllowed(w)
 		return
 	}
 

internal/server/sse_handler.go

@@ -50,7 +50,7 @@ func (h *handlers) GetEventsStream(w http.ResponseWriter, r *http.Request) {
 	}
 
 	if h.bus == nil {
-		http.Error(w, "events stream not wired", http.StatusServiceUnavailable)
+		writeError(w, http.StatusServiceUnavailable, "server.unavailable", "server", "events stream not wired", true)
 		return
 	}
 
@@ -59,13 +59,13 @@ func (h *handlers) GetEventsStream(w http.ResponseWriter, r *http.Request) {
 	// firehose by default). AC-02, AC-03.
 	topics := parseTopics(r.URL.Query().Get("topics"))
 	if len(topics) == 0 {
-		http.Error(w, "no topics requested", http.StatusBadRequest)
+		writeError(w, http.StatusBadRequest, "request.missing_parameter", "client", "parameter topics is required", false)
 		return
 	}
 
 	flusher, ok := w.(http.Flusher)
 	if !ok {
-		http.Error(w, "streaming unsupported", http.StatusInternalServerError)
+		writeError(w, http.StatusInternalServerError, "server.internal", "server", "streaming unsupported", false)
 		return
 	}
 

internal/server/sse_handler_test.go

@@ -77,8 +77,11 @@ func TestEventsStream_NoTopicsReturns400(t *testing.T) {
 		if w.Code != http.StatusBadRequest {
 			t.Errorf("status = %d, want 400", w.Code)
 		}
-		if !strings.Contains(w.Body.String(), "no topics requested") {
-			t.Errorf("body = %q, want 'no topics requested'", w.Body.String())
+		// The body is the canonical envelope (system-http-server C-15), so
+		// the message lives under error.human_message.
+		if !strings.Contains(w.Body.String(), `"request.missing_parameter"`) ||
+			!strings.Contains(w.Body.String(), "parameter topics is required") {
+			t.Errorf("body = %q, want the envelope naming the topics parameter", w.Body.String())
 		}
 	})
 }

specs/api/events-stream.spec.yaml

@@ -1,7 +1,7 @@
 spec:
   id: api-events-stream
   title: SSE live-events stream for the operator UI
-  version: "1.1.0"
+  version: "1.1.1"
   status: approved
   tier: 2
 
@@ -94,7 +94,7 @@ spec:
       priority: critical
       references_constraints: [C-01]
     - id: AC-02
-      description: GET /api/v1/events without ?topics (or with ?topics=) returns 400 with body containing "no topics requested".
+      description: GET /api/v1/events without ?topics (or with ?topics=) returns 400 carrying the canonical error envelope (system-http-server C-15) with code request.missing_parameter and a human_message naming the topics parameter.
       priority: critical
       references_constraints: [C-02]
     - id: AC-03

specs/system/http-server.spec.yaml

@@ -1,7 +1,7 @@
 spec:
   id: system-http-server
   title: HTTPS server with TLS hot-reload
-  version: "1.4.0"
+  version: "1.5.0"
   status: approved
   tier: 2
 
@@ -111,6 +111,26 @@ spec:
       type: security
       enforcement: error
 
+    - id: C-15
+      description: >-
+        Every 4xx or 5xx response the OpenWatch process itself generates MUST
+        carry the canonical error envelope (error.code, error.fault,
+        error.retryable, error.human_message, error.correlation_id) with
+        Content-Type application/json, including the responses that no
+        handler produces: an unmatched /api/ path (404, request.not_found),
+        a method the route does not accept (405,
+        request.method_not_allowed), and a request whose path, query, header
+        or cookie parameter is missing or fails to parse before the handler
+        runs (400, request.missing_parameter or request.invalid_parameter).
+        A parameter-binding message names the parameter and never echoes the
+        rejected value or the parser's internal error text, so a client
+        cannot be reflected into a body and no internal detail leaks.
+        Out of scope by construction: bodies produced by a proxy, load
+        balancer or the kernel in front of the process; a client MUST treat
+        a non-2xx whose body is not JSON as an infrastructure error rather
+        than a parse failure, and the API guide says so.
+      type: technical
+      enforcement: error
   acceptance_criteria:
     - id: AC-01
       description: Server.Run() binds to cfg.Server.Listen and accepts HTTPS connections using TLSCert and TLSKey via GetCertificate.
@@ -182,3 +202,16 @@ spec:
       description: A cookie-authenticated unsafe request (session cookie present) with no X-CSRF-Token, or a header that does not match the XSRF-TOKEN cookie, returns 403 authz.csrf_invalid; the same request with a matching X-CSRF-Token proceeds. A request carrying no session cookie, or an Authorization (Bearer) header, or to an /api/v1/auth/* path, is exempt. POST /auth/login Set-Cookies a non-HttpOnly XSRF-TOKEN.
       priority: critical
       references_constraints: [C-14]
+    - id: AC-20
+      description: >-
+        GET an unmatched /api/ path returns 404 with the envelope
+        (code request.not_found), DELETE /api/v1/health returns 405 with the
+        envelope (code request.method_not_allowed), and GET
+        /api/v1/audit/events?limit=abc as a viewer returns 400 with the
+        envelope (code request.invalid_parameter) whose human_message names
+        limit and does not contain abc; each carries Content-Type
+        application/json and an X-Correlation-Id header. A source walk of
+        internal/server finds no bare http.Error call outside tests, so a
+        new plain-text error cannot be added without failing this criterion.
+      priority: high
+      references_constraints: [C-15]