unworked validation sweep: fix all remaining major findings (tasks 020-031)

Closes the last batch of major-severity unworked findings. The 43
majors in the backlog at the start of this sweep are now all
addressed (resolved by later commits and marked, fixed in this
sweep, or closed with explicit "deferred-until-profile" rationale
where the recommendation is a hot-path optimisation that the new
bench_route_lookup harness should drive).

Behavioural / structural changes
--------------------------------
- src/detail/webserver_routes.cpp: extract `make_non_prefix_entry`
  helper used by both branches of `insert_fresh_v2_entry`. The
  other two construction sites stay open-coded because their
  inputs don't match the helper's contract (set_all + variable
  is_prefix, or merge-into-existing). [manual-validation #6]

Documentation / comment trims
-----------------------------
- src/httpserver/detail/route_cache.hpp: file-level comment
  trimmed to the WHY-only portion (mutex choice + lock order),
  cross-references architecture §4.7. [task-027 #7]
- src/httpserver/detail/webserver_impl.hpp: drop the tier-order
  prose; keep only the lock-order paragraph plus the (newer)
  CWE-407 paragraph and a pointer to lookup_v2's implementation
  comment for the walk-order rationale. [task-027 #9]
- src/httpserver/webserver.hpp: collapse the 9-line top-of-file
  comment into 4 lines that explain only the include-firewall
  rationale and point to the per-method Doxygen blocks for ABI
  details. [task-020 #4]

Already-fixed findings, marked with a pointer to the resolving
commit:
- task-027 #14 (radix tree hash-DoS) → fixed by TASK-056
- task-027 #18 (terminus collision) → fixed by TASK-056
- task-031 #3, #4 (CWE-209 in default error body) → fixed by
  TASK-055 / DR-009 Revision 1
- task-036 #3 (dual dispatch branches) → removed by TASK-046+
- task-049 #2 (snapshot reserve) → resolved by TASK-048
- manual-validation #10 (PMD sha256 pin) → fixed by TASK-059

Closed with deferred-until-profile rationale (hot-path
optimisations the bench harness should drive, not speculative
rewrites):
- task-027 #11 (best_prefix_caps copy)
- task-027 #12 (cache_value copy-on-hit)
- task-027 #13 (normalize_path allocation)
- task-025 #2 (lambda fast path bypassing virtual dispatch)
- manual-validation #7 (canonicalize_lookup_path allocation)
- manual-validation #8 (normalize_path pre-normalisation)
- manual-validation #9 (serialize_allow_methods caching)

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

Author: etr

specs/unworked_review_issues/2026-05-07_185613_task-020.md

@@ -21,10 +21,10 @@
    *Recommendation:* Remove the `struct sockaddr;` and `struct sockaddr_storage;` forward declarations from `src/httpserver/webserver.hpp` (lines 52-53). The declarations in `http_utils.hpp` propagate transitively. If `webserver.hpp` ever stops including `http_utils.hpp`, a follow-up can restore them.
    *Status:* Resolved — webserver.hpp no longer redeclares `struct sockaddr;` / `struct sockaddr_storage;`. The header comment at lines 54-57 explicitly notes they are reached transitively via `http_utils.hpp`. The only `struct sockaddr` mentions in webserver.hpp are typed member fields and parameter types (lines 329, 402-403).
 
-4. [ ] **code-simplifier** | `src/httpserver/webserver.hpp:40` | code-structure
+4. [x] **code-simplifier** | `src/httpserver/webserver.hpp:40` | code-structure
    The block comment introducing `get_fdset` and `add_connection` (lines 40-52) and the per-method Doxygen comments (lines 157-197) both explain the `void*`/`unsigned int` ABI workaround, producing duplicate prose. The top-level block comment is long enough to be a distraction at the class level; the per-method comments immediately below are the natural place for callers to look.
    *Recommendation:* Shorten the top-level block comment to a single sentence (e.g. `// Socket-layer types are forward-declared; see individual method comments for the void*/unsigned-int ABI rationale.`) and keep the full explanation only in the Doxygen blocks for `get_fdset` and `add_connection`.
-   *Status:* deferred — the top-level comment at webserver.hpp:49-57 is still ~8 lines and overlaps with the per-method Doxygen blocks; a future cleanup pass should collapse it to one sentence with a "see individual method comments" pointer.
+   *Fixed:* Top-level comment collapsed to 4 lines that explain only the include-firewall rationale and point to the per-method Doxygen blocks for ABI details. Duplication with the get_fdset / add_connection docs is removed.
 
 5. [x] **security-reviewer** | `src/httpserver/webserver.hpp:163` | insecure-design
    get_fdset() now accepts three void* parameters in place of fd_set*. Callers that accidentally pass an incompatible pointer type (e.g. a pipe-fd array, a raw int buffer) will compile silently because any data pointer converts to void*. The original fd_set* signature produced a compiler error at misuse sites; the new signature moves the error to runtime, potentially corrupting file-descriptor set state and allowing an attacker to influence which sockets are monitored. CWE-704 (Incorrect Type Conversion).

specs/unworked_review_issues/2026-05-10_191459_task-025.md

@@ -11,10 +11,10 @@
    *Recommendation:* After the validation loop concludes, persist the consolidated non-blocking review findings to specs/unworked_review_issues/ with a filename following the pattern YYYY-MM-DD_HHMMSS_task-025.md, consistent with how TASK-024's findings were persisted.
    *Status:* Addressed — this file IS the persisted unworked_review_issues file for TASK-025 (2026-05-10_191459_task-025.md).
 
-2. [ ] **performance-reviewer** | `src/webserver.cpp:359` | missing-caching
+2. [x] **performance-reviewer** | `src/webserver.cpp:359` | missing-caching
    on_method_() uses std::dynamic_pointer_cast<detail::lambda_resource> on every registration call to distinguish a lambda shim from a class-based resource. This is registration-time only (not per-request), so the cast cost is not a hot-path problem. However, once the lambda_resource shim is inserted into registered_resources as a shared_ptr<http_resource>, the dispatch path (finalize_answer) will also re-traverse it via the virtual render_* mechanism, paying the vtable indirection cost that was avoidable if lambda routes had a separate lookup tier. This is a latent issue that will only matter when TASK-027 wires route_entry into the real dispatch; flagged now so the 3-tier design can accommodate a fast lambda branch.
    *Recommendation:* When TASK-027 integrates route_entry into the dispatch path, maintain a separate fast path for the lambda_handler arm of the variant (std::get<0>(entry.handler)(req)) that bypasses the virtual dispatch chain (render_get -> invoke_ -> slot lookup). This is the main motivation for route_entry carrying a variant in the first place.
-   *Status:* Deferred — TASK-027 is complete and wired the v2 3-tier table. The fast lambda variant arm (bypassing virtual dispatch) is a TASK-036 or follow-up concern. The v1 shim dispatch path remains for backward compatibility and is not hot-path.
+   *Acknowledged-deferred:* TASK-027 wired the v2 3-tier table and TASK-053 cut dispatch over to lookup_v2. The fast-lambda variant arm (bypassing virtual dispatch) is a design change that needs `bench_route_lookup` (TASK-053 step 5) to first show vtable cost as a measured contributor in the warm-cache profile. The architecture intentionally keeps the lambda-shim path going through `http_resource::invoke_` so the dispatch surface stays uniform; carving out a variant arm trades that uniformity for an unmeasured win. Closing the finding with a deferred-until-profile note; the bench harness is now in place to retire the deferral when the data supports it.
 
 ## Minor