TASK-036 minors: address 43 unchecked minor review items

Fix or document all 43 pending minor findings from the 2026-05-18
review pass. No behavioral changes — cosmetic, documentation, and
low-risk structural improvements only.

Key changes:
- Fix typo in webserver_callbacks.cpp (finding 22: 'Asssume/enought')
- Add `*~` to .gitignore to suppress editor/autoconf backup files (34)
- Add class-level comment to modded_request (18)
- Trim verbose DR-010 comment on response field to one line (28)
- Update lambda_resource: note render_connect/render_trace fallback (27),
  add slot-type doc comment (17), add invoke_() doc comment (26)
- Trim duplicate inline comment from http_resource::render() (29)
- Add MHD refcounting comment near MHD_destroy_response for deferred
  responses explaining why the immediate destroy is safe (15)
- Rename materialize_current -> try_materialize (23)
- Extract emplace_and_materialize helper in get_raw_response_with_fallback
  to fold repeated emplace+materialize pattern (30)
- Wrap ws_upgrade_data in unique_ptr RAII guard in websocket upgrade
  path; release to MHD after queue_response (42 / CWE-401)
- Document CWE-532 contract on log_dispatch_error (44)
- Document auth oracle design choice (found==true gate, 43 / CWE-200)
- Reduce thread_safety test sleep from 10s to 2s and add liveness
  comment on tautological assertion (49)
- Rename deferred_response_suite test -> deferred_response_with_prefix_content (50)
- Add on_post_lambda_returns_value integration test to guard non-GET
  lambda dispatch path (51)
- Condense lengthy AC-3 comment to observable contract (52)
- Add double-stop idempotency note in AC-3 test (25)
- Replace default_render_returns_empty unit test with
  default_render_returns_sentinel which tests the actual -1 sentinel
  contract introduced by TASK-036 (53)
- Add route_entry::lambda_handler dispatch note for PRD-RSP-REQ-007 (47)
- Update http-resource.md arch doc: render_* return by value, DR-004 (36)
- Add per-slot copy behaviour comment in commit_handlers_to_shim (39)

Deferred (unchanged): 13, 37, 38, 40, 41, 45, 46 (already documented
or design-level decisions); 20, 21, 31, 32, 33 (already addressed by
code refactoring in prior commits); 16, 35 (no action needed).

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

Author: etr

.gitignore

@@ -1,5 +1,6 @@
 *.sw*
 *.*~
+*~
 *.in
 *.php
 *.pm

specs/architecture/04-components/http-resource.md

@@ -2,12 +2,12 @@
 
 **Responsibility:** Stateful handler base for cases where state is shared across HTTP methods of one resource (counter, cache, DB handle, auth context).
 
-**Implementation:** Public abstract base. Subclasses override one of `render_get / render_post / render_put / render_delete / render_patch / render_options / render_head` (renamed from v1's `render_GET` etc., to comply with PRD-NAM-REQ-001 snake_case). The default `render(...)` falls back when the method-specific override is not provided.
+**Implementation:** Public abstract base. Subclasses override one of `render_get / render_post / render_put / render_delete / render_patch / render_options / render_head` (renamed from v1's `render_GET` etc., to comply with PRD-NAM-REQ-001 snake_case). Each `render_*` override returns `http_response` **by value** (DR-004 / PRD-RSP-REQ-007); the webserver moves the returned value into the per-connection `modded_request::response` anchor. The default `render(...)` falls back when the method-specific override is not provided; it returns a default-constructed `http_response` whose `status_code_ == -1` sentinel routes through `internal_error_page`.
 
 The allow-mask (formerly `std::map<std::string, bool> method_state`) becomes `method_set methods_allowed_;` — a `uint32_t` bitmask wrapper (DR-6). `is_allowed(http_method)` and `get_allowed_methods()` are `const` and return without allocation.
 
 **Lifetime:** owned by the `webserver` via `unique_ptr` or `shared_ptr` (PRD-HDL-REQ-003). Raw-pointer registration is gone (PRD-HDL-REQ-005).
 
-**Related requirements:** PRD-HDL-REQ-003, PRD-HDL-REQ-005, PRD-REQ-REQ-002, PRD-REQ-REQ-003.
+**Related requirements:** DR-004, PRD-HDL-REQ-003, PRD-HDL-REQ-005, PRD-REQ-REQ-002, PRD-REQ-REQ-003, PRD-RSP-REQ-007.
 
 ---

src/detail/webserver_aliases.cpp

@@ -178,6 +178,15 @@ void webserver::install_default_alias_hooks_() {
     // webserver_impl::apply_auth_short_circuit inline path: it respects
     // auth_skip_paths, calls the user-supplied auth_handler, and
     // short-circuits with the returned response when auth fails.
+    //
+    // Design note (security-reviewer-iter1-7 / CWE-200): auth_handler
+    // is registered as a before_handler hook, which fires only for
+    // matched routes (found==true). Requests to unregistered paths
+    // receive a 404 without consulting auth_handler. This means 401 vs
+    // 404 distinguishes registered from unregistered routes (auth oracle).
+    // Callers who need uniform authentication on all requests — including
+    // 404s — should add a catch-all fallback route or register a
+    // not_found_handler that applies equivalent auth logic.
     // ----------------------------------------------------------------
     if (auth_handler != nullptr) {
         // Capture both the webserver* (for auth_handler callable) and the

src/detail/webserver_callbacks.cpp

@@ -309,7 +309,7 @@ void webserver_impl::error_log(void* cls, const char* fmt, va_list ap) {
     webserver* dws = static_cast<webserver*>(cls);
 
     std::string msg;
-    msg.resize(80);  // Asssume one line will be enought most of the time.
+    msg.resize(80);  // Assume one line will be enough most of the time.
 
     va_list va;
     va_copy(va, ap);  // Stash a copy in case we need to try again.

src/detail/webserver_error_pages.cpp

@@ -98,6 +98,12 @@ void webserver_impl::log_dispatch_error(std::string_view msg) const {
     if (parent->log_error == nullptr) {
         return;
     }
+    // Framework contract (CWE-532): msg may contain e.what() text from
+    // a handler exception, which could include sensitive data (DB connection
+    // strings, file paths, user-supplied input that triggered the exception).
+    // Application code should sanitize or wrap exceptions that might expose
+    // sensitive information before re-throwing them.
+    //
     // A misbehaving user logger must not poison the catch from inside
     // the catch. Swallow any exception it throws; we have no recovery
     // beyond dropping the log line.

src/detail/webserver_request.cpp

@@ -194,41 +194,44 @@ struct MHD_Response* webserver_impl::get_raw_response_with_fallback(detail::modd
     // emplace(std::move(...)); the optional owns the value and the
     // deferred-body trampoline keeps a pointer into it for the lifetime
     // of the modded_request.
-    auto materialize_current = [&]() -> struct MHD_Response* {
+    auto try_materialize = [&]() -> struct MHD_Response* {
         return materialize_response(mr->response ? &*mr->response : nullptr);
     };
+    // Helper: emplace a new response and immediately materialize it.
+    // Each catch arm sets mr->response then returns the raw MHD handle.
+    auto emplace_and_materialize = [&](http_response r) -> struct MHD_Response* {
+        mr->response.emplace(std::move(r));
+        return try_materialize();
+    };
     try {
-        struct MHD_Response* raw = materialize_current();
+        struct MHD_Response* raw = try_materialize();
         if (raw == nullptr) {
             // TASK-031: no exception was thrown, but the body materializer
             // returned null. Route through the safe internal-error path so
             // a misbehaving user handler can't escape.
-            mr->response.emplace(run_internal_error_handler_safely(
+            return emplace_and_materialize(run_internal_error_handler_safely(
                 mr, "materialize_response returned null"));
-            raw = materialize_current();
         }
         return raw;
     } catch(const std::invalid_argument&) {
         try {
-            mr->response.emplace(not_found_page(mr));
-            return materialize_current();
+            return emplace_and_materialize(not_found_page(mr));
         } catch(...) {
             return nullptr;
         }
     } catch(const std::exception& e) {
         log_dispatch_error(std::string("materialize threw: ") + e.what());
         try {
-            mr->response.emplace(run_internal_error_handler_safely(mr, e.what()));
-            return materialize_current();
+            return emplace_and_materialize(
+                run_internal_error_handler_safely(mr, e.what()));
         } catch(...) {
             return nullptr;
         }
     } catch(...) {
         log_dispatch_error("materialize threw unknown exception");
         try {
-            mr->response.emplace(run_internal_error_handler_safely(mr,
-                                                         "unknown exception"));
-            return materialize_current();
+            return emplace_and_materialize(run_internal_error_handler_safely(
+                mr, "unknown exception"));
         } catch(...) {
             return nullptr;
         }
@@ -260,6 +263,14 @@ MHD_Result webserver_impl::materialize_and_queue_response(MHD_Connection* connec
     // queued bytes -- only the http_response in mr->response matters
     // for the hook ctx pointer.
     fire_response_sent_gated(mr);
+    // MHD reference-counting note: for MHD_create_response_from_callback
+    // responses (deferred/streaming), MHD increments its own internal
+    // refcount during MHD_queue_response, so this MHD_destroy_response only
+    // releases the *caller's* reference. MHD keeps the streaming callback
+    // (deferred_body::trampoline) alive — and therefore the cls pointer into
+    // mr->response — until request_completed fires and MHD releases its own
+    // reference. The modded_request (and mr->response) are destroyed only in
+    // the request_completed callback, which is after MHD is done streaming.
     MHD_destroy_response(raw_response);
     return (MHD_Result) to_ret;
 }

src/detail/webserver_routes.cpp

@@ -170,8 +170,11 @@ void webserver_impl::commit_handlers_to_shim(detail::lambda_resource& shim,
         method_set methods,
         const std::function<::httpserver::http_response(
             const ::httpserver::http_request&)>& handler) {
-    // The shared std::function copies cheaply (type-erased callable),
-    // so each slot owns its own copy.
+    // Each slot gets its own copy of the std::function. For small captures
+    // (within SBO, ~16-32 bytes on libstdc++/libc++) the copies are cheap.
+    // For large-capture handlers registered across many methods, each copy
+    // allocates once. Registration-time only; not a hot-path concern.
+    // (performance-reviewer-iter1-3: document the per-slot copy behaviour.)
     for_each_requested_method(methods, [&](http_method m) {
         shim.set_slot(m, handler);
     });

src/detail/webserver_websocket.cpp

@@ -212,11 +212,15 @@ webserver_impl::complete_websocket_upgrade(MHD_Connection* connection,
     std::shared_ptr<websocket_handler> handler_sp = ws_it->second;
     lock.unlock();
 
-    auto* data = new ws_upgrade_data{this, std::move(handler_sp)};
+    // CWE-401: RAII guard so data is freed if MHD_create_response_for_upgrade
+    // returns null. Ownership is transferred to MHD (via release()) only after
+    // the queue call — upgrade_handler receives data_guard.get() as cls and
+    // wraps it in unique_ptr for cleanup (see upgrade_handler below).
+    std::unique_ptr<ws_upgrade_data> data_guard(
+        new ws_upgrade_data{this, std::move(handler_sp)});
     struct MHD_Response* response = MHD_create_response_for_upgrade(
-        &webserver_impl::upgrade_handler, data);
+        &webserver_impl::upgrade_handler, data_guard.get());
     if (response == nullptr) {
-        delete data;
         return std::nullopt;
     }
     MHD_add_response_header(response, MHD_HTTP_HEADER_UPGRADE, "websocket");
@@ -231,6 +235,7 @@ webserver_impl::complete_websocket_upgrade(MHD_Connection* connection,
                                                        MHD_HTTP_SWITCHING_PROTOCOLS,
                                                        response);
     MHD_destroy_response(response);
+    data_guard.release();  // MHD now owns; upgrade_handler will delete.
     return to_ret;
 }
 

src/httpserver/detail/lambda_resource.hpp

@@ -64,7 +64,10 @@ namespace detail {
 
 // Tiny adapter that wraps a single lambda_handler as an http_resource
 // virtual override. We keep one slot per method enum and dispatch in
-// each render_* override.
+// each render_* override. Each slot holds a
+//   std::function<http_response(const http_request&)>
+// (see route_entry.hpp::lambda_handler). Slots are installed via
+// set_slot() and invoked by the render_* overrides below.
 class lambda_resource final : public ::httpserver::http_resource {
  public:
     lambda_resource() {
@@ -90,6 +93,9 @@ class lambda_resource final : public ::httpserver::http_resource {
     // Each differs only by the http_method enum constant forwarded.
     // They are required by the http_resource base-class interface and
     // cannot be collapsed further without changing that interface.
+    // render_connect and render_trace intentionally fall back to the
+    // base-class default (no slot wired, no set_allowing call); they
+    // return the -1 sentinel and route through internal_error_page.
     ::httpserver::http_response
     render_get(const ::httpserver::http_request& r) override {
         return invoke_(http_method::get, r);
@@ -120,6 +126,9 @@ class lambda_resource final : public ::httpserver::http_resource {
     }
 
  private:
+    // Dispatch to the registered slot for method `m`, returning the
+    // handler's http_response by value. Callers must ensure is_allowed(m)
+    // is true before calling (the 405 guard in finalize_answer enforces this).
     ::httpserver::http_response
     invoke_(http_method m, const ::httpserver::http_request& r) {
         auto& slot = slots_[static_cast<std::size_t>(m)];

src/httpserver/detail/modded_request.hpp

@@ -41,6 +41,11 @@ namespace httpserver {
 
 namespace detail {
 
+// modded_request adapts the raw MHD connection data into the
+// libhttpserver request model, accumulating per-connection state
+// (parsed headers, upload stream, method callback, staged response)
+// across MHD's repeated answer_to_connection invocations for one
+// HTTP request until finalize_answer queues the response.
 struct modded_request {
     struct MHD_PostProcessor *pp = nullptr;
     std::string complete_uri;
@@ -63,8 +68,7 @@ struct modded_request {
     httpserver::http_method method_enum = httpserver::http_method::count_;
 
     std::unique_ptr<http_request> dhr = nullptr;
-    // DR-010 / §5.3: anchor kept alive until request_completed.
-    // See webserver_impl.hpp and specs/architecture for full contract.
+    // DR-010 / §5.3: anchor kept alive until request_completed. See webserver_impl.hpp for full contract.
     std::optional<http_response> response;
     bool has_body = false;
     // TASK-047: set by a pre-handler hook short-circuit (request_received