Merge TASK-055: sanitize default 500 body / DR-009 Revision 1

Author: etr

README.md

@@ -639,6 +639,19 @@ build, `webserver(create_webserver{}.use_ssl(true))` throws
   dispatch. The handler signature is
   `http_response(const http_request&, std::string_view message)`. Load-bearing:
   see [Error propagation](#error-propagation).
+* **`.expose_exception_messages(bool = true)`** — restores the v1
+  behaviour of including the originating exception message in the
+  **default** 500 response body. Default is `false`: the default body
+  is the fixed string `"Internal Server Error"`
+  ([DR-009 Revision 1](specs/architecture/11-decisions/DR-009.md),
+  CWE-209 information-disclosure fix). The configured `log_error`
+  callback continues to receive the verbatim message regardless of
+  this flag — only the HTTP response body is affected.
+  **Warning:** exception messages routinely contain file paths, SQL
+  fragments, internal identifiers, and attacker-influenced input.
+  Enable only in development or behind an explicit `#ifndef NDEBUG`
+  guard. A configured `internal_error_handler` is unaffected — it
+  always receives the message and can build any body it wants.
 
 A worked example:
 
@@ -1664,9 +1677,20 @@ Distilled from
    invokes `internal_error_handler(request, e.what())`. The response
    returned by `internal_error_handler` is sent to the client; if no
    custom `internal_error_handler` is configured, a default 500 with
-   the message in the body is sent.
+   the **fixed body `"Internal Server Error"`** is sent
+   (DR-009 Revision 1, CWE-209 fix — exception text often contains
+   file paths, SQL fragments, or attacker-influenced input and must
+   not cross a process boundary to an untrusted client). To restore
+   the v1 behaviour of including the exception message in the body
+   for development, set `.expose_exception_messages(true)` on the
+   builder. The verbatim message is still surfaced via the
+   `log_error` callback and to any configured
+   `internal_error_handler` regardless of the flag.
 2. **A handler that throws something other than `std::exception`** is
    also caught, with `"unknown exception"` substituted for the message.
+   The same default body (`"Internal Server Error"`) applies; the
+   `"unknown exception"` sentinel reaches the wire only when
+   `.expose_exception_messages(true)` is set.
 3. **Library-internal failures during dispatch** (allocation, body
    materialisation) flow through the same `internal_error_handler` path.
 4. **If `internal_error_handler` itself throws**, the library logs and
@@ -1698,7 +1722,13 @@ httpserver::webserver ws{cfg};
 ```
 
 See [`examples/custom_error.cpp`](examples/custom_error.cpp) for a
-worked example.
+worked example. Note that the snippet above — which echoes `what`
+verbatim to the wire — is illustrative of the explicit-handler case;
+the **default** (no `internal_error_handler` configured) path now
+sends a fixed body, and only the `log_error` callback receives the
+verbatim message. To restore the v1 verbose-body default for
+development, chain `.expose_exception_messages(true)` on the builder
+(see [Custom error handlers](#custom-error-handlers)).
 
 [Back to TOC](#table-of-contents)
 

RELEASE_NOTES.md

@@ -196,6 +196,19 @@ and see the v2 replacement.
   fluent assembly.
 - **`webserver(create_webserver const&)` is `explicit`.** Direct-init
   `webserver ws{cw};` rather than copy-init `webserver ws = cw;`.
+- **Default internal-error response body is now a fixed string.** v1's
+  default 500 body included the originating exception's `e.what()`
+  text. v2.0
+  ([DR-009 Revision 1](specs/architecture/11-decisions/DR-009.md))
+  sends `"Internal Server Error"` instead, eliminating a CWE-209
+  information-disclosure surface (exception messages routinely embed
+  file paths, SQL fragments, internal identifiers, and
+  attacker-influenced input). The `log_error` callback continues to
+  receive the verbatim message. To restore the v1 verbose-body
+  behaviour for development, set `.expose_exception_messages(true)`
+  on the builder. Configured `internal_error_handler` callbacks are
+  unaffected — they still receive the message and can build any body
+  they want.
 
 ## Threading
 
@@ -226,7 +239,11 @@ for the full statement. The load-bearing points for porters:
 - A handler that throws lands at the configured
   `internal_error_handler` (a `std::function<http_response(const http_request&, std::string_view)>`).
   The `string_view` carries `what()` from the caught exception. Default
-  behaviour: log and return `500`.
+  behaviour: log and return `500`. When no `internal_error_handler` is
+  configured, the default response body is the fixed string
+  `"Internal Server Error"` (DR-009 Revision 1, CWE-209 fix); set
+  `.expose_exception_messages(true)` on the builder to restore the v1
+  message-in-body behaviour for development.
 - Calling an API entry point whose backend feature is disabled at
   configure time (e.g. `ssl_cert(...)` on a `HAVE_GNUTLS=no` build)
   throws `feature_unavailable` — a public `std::runtime_error` subclass.

specs/architecture/05-cross-cutting.md

@@ -16,9 +16,9 @@
 
 ### 5.2 Error propagation
 
-**Contract (committed in DR-9):**
-1. Handler throws `std::exception` → caught, logged via the `log_error` callback, `internal_error_handler` invoked with `e.what()`, response sent (default 500).
-2. Handler throws non-`std::exception` → caught with `catch (...)`, logged generically via `log_error`, `internal_error_handler` invoked with `"unknown exception"`.
+**Contract (committed in DR-9, revised 2026-05-29 — Revision 1):**
+1. Handler throws `std::exception` → caught, logged via the `log_error` callback, `internal_error_handler` invoked with `e.what()`, response sent. **Default 500** body is the fixed string `"Internal Server Error"` (DR-009 Revision 1 / CWE-209). The verbatim message is still surfaced via the `log_error` callback and to any configured `internal_error_handler`; the v1 behaviour of including it in the default body is opt-in via `create_webserver::expose_exception_messages(true)` (development only).
+2. Handler throws non-`std::exception` → caught with `catch (...)`, logged generically via `log_error`, `internal_error_handler` invoked with `"unknown exception"`. Default body is the same fixed string `"Internal Server Error"`; the `"unknown exception"` sentinel is on the wire only when `expose_exception_messages(true)` is set.
 3. Library-internal exception in dispatch (allocation failure, body materialization error) → same path as (1)/(2).
 4. `internal_error_handler` itself throws → library logs via `log_error` and sends a hardcoded 500 with empty body.
 5. `feature_unavailable` is a normal `std::runtime_error`; no special status mapping. Users who care translate it explicitly.

specs/architecture/11-decisions/DR-009.md

@@ -1,6 +1,6 @@
 ### DR-009: Handler error-propagation contract
 
-**Status:** Accepted
+**Status:** Accepted; revised 2026-05-29 (Revision 1)
 **Date:** 2026-04-30
 **Context:** With DR-4 (return-by-value), null-return is impossible. Two cases remain: handler throws, library-internal exception during dispatch.
 
@@ -19,3 +19,72 @@
 - `internal_error_handler` is the single user-overridable error escape hatch.
 
 ---
+
+### Revision 1 (2026-05-29) — Sanitize the default 500 body
+
+**Context:** The original decision had `internal_error_page` surface
+`e.what()` in the default-handler 500 body for debugging ergonomics.
+This was flagged by two security reviewers on TASK-031 (task-031 #3,
+task-031 #4) and one Pass-1 sweep on TASK-036 (task-036 #44) as a
+CWE-209 information-disclosure surface: handler exception text
+routinely embeds file paths, SQL fragments, internal identifiers, and
+attacker-influenced input that must not cross a process boundary to an
+untrusted client.
+
+**Revised decision:** The default body of the no-handler-configured 500
+path is the fixed string `"Internal Server Error"`. The originating
+exception message is still:
+
+1. Forwarded verbatim to a user-configured `internal_error_handler` (if
+   one is wired) — that handler can render any body it wants.
+2. Forwarded verbatim to the user-configured `log_error` callback (the
+   server-side log).
+
+Only the **default** HTTP response body is sanitized; everything else
+is unchanged. A new opt-in builder flag,
+`create_webserver::expose_exception_messages(bool)`, restores the
+pre-revision behaviour of including the message in the default body.
+The flag is documented as development-only (see the `@warning` block
+on the setter).
+
+**Contract points 1–6 in §5.2 stand; only the default-body clause in
+point 1 is amended.**
+
+**Consequences (Revision 1):**
+- The regression tests
+  `dr009_runtime_error_message_surfaces_in_default_body` and
+  `dr009_non_std_exception_yields_unknown_exception_in_default_body`
+  are renamed/split into a "fixed body" pair (the new contract) plus a
+  "verbose body" pair (the opt-in round-trip). See
+  `test/integ/basic.cpp`.
+- The legacy integration tests in `basic_suite` that pre-date this
+  revision and assert on the message-in-body (`exception_forces_500`,
+  `untyped_error_forces_500`, `file_serving_resource_missing`,
+  `file_serving_resource_dir`, `long_error_message`,
+  `dr009_feature_unavailable_lands_as_generic_500`) opt the
+  suite-shared `ws` into the verbose mode via
+  `.expose_exception_messages(true)` so their original assertion
+  intent — "the dispatcher's message-forwarding path works" — is
+  preserved without diluting coverage.
+- No request-id concept is introduced as part of this revision. The
+  original action item mentioned "with the request id (if any)
+  appended"; the codebase has no request-id plumbing today and adding
+  one is out of scope for a security-fix revision. The fixed-body
+  alone satisfies CWE-209. A request-id concept can be added later
+  without re-opening this decision.
+- `log_dispatch_error` continues to log `e.what()` verbatim regardless
+  of the flag. The error log is the canonical destination for the
+  verbatim exception text. Handlers that may throw exceptions
+  containing sensitive data (DB URLs, credentials, attacker-influenced
+  input) SHOULD catch and sanitize the message before re-throwing if
+  those values must not appear in the server log either; see the
+  Doxygen note on `webserver_impl::log_dispatch_error`.
+- ABI: this revision is shipping on `feature/v2.0` before v2.0 cuts;
+  SOVERSION is already 2 and there is no released ABI to preserve.
+  The added `const bool expose_exception_messages` on `webserver` and
+  the `bool _expose_exception_messages` on `create_webserver` recompile
+  cleanly for any consumer TU.
+
+**Related findings:** task-031 #3, task-031 #4, task-036 #44, task-034 #24.
+
+---

specs/tasks/v2-deferred-backlog-plan.md

@@ -162,22 +162,24 @@ untrusted client. The decision needs a coordinated revision: the default
 behaviour should be sanitized; the verbose form should be opt-in.
 
 **Action Items:**
-- [ ] Draft DR-009-rev1 in `specs/architecture/03-decisions.md` (or
-  equivalent file): default body is a fixed string ("Internal Server
-  Error") with the request id (if any) appended; verbose body is
-  opt-in via a `webserver_builder.expose_exception_messages(true)` flag
-  intended for development environments only.
-- [ ] Update `webserver_impl::internal_error_page` to consult the flag.
-- [ ] Update `log_dispatch_error` to log `e.what()` verbatim to the
+- [x] Draft DR-009-rev1 in `specs/architecture/11-decisions/DR-009.md`:
+  default body is a fixed string ("Internal Server Error"); the
+  "with the request id (if any) appended" clause was deferred (no
+  request-id concept exists in the codebase today; documented in
+  DR-009 Revision 1 "Consequences"). Verbose body is opt-in via
+  `create_webserver::expose_exception_messages(bool)` flag intended
+  for development environments only.
+- [x] Update `webserver_impl::internal_error_page` to consult the flag.
+- [x] Update `log_dispatch_error` to log `e.what()` verbatim to the
   server log (where it belongs) regardless of the flag; add a Doxygen
   note that handlers throwing exceptions with sensitive data should
   catch+sanitize before throwing.
-- [ ] Update the two regression tests that pin the current behaviour
+- [x] Update the two regression tests that pin the current behaviour
   (`dr009_runtime_error_message_surfaces_in_default_body` and sibling)
   to split into:
   - `dr009_default_body_is_fixed_string` (new contract)
   - `dr009_verbose_body_surfaces_message_when_opted_in`
-- [ ] Update README "Error handling" section + RELEASE_NOTES.md
+- [x] Update README "Error handling" section + RELEASE_NOTES.md
   "Behaviour change" note.
 
 **Dependencies:**
@@ -196,6 +198,8 @@ behaviour should be sanitized; the verbose form should be opt-in.
 **Related Findings:** task-031 #3, task-031 #4, task-036 #44, task-034 #24
 **Related Decisions:** DR-009 (revised), CWE-209
 
+**Status:** Done
+
 ---
 
 ## TASK-056 — Hash-DoS hardening + prefix-route disambiguation in radix tree