Files
alkhttp/docs/reviews/002-post-remediation-review.md
T
glm-5.3-flash e2c255d40c docs(tasks): decompose review-002 into 24 tasks (23 implementation + 1 bracketed follow-up)
Decomposition of docs/reviews/002-post-remediation-review.md per its
5-unit remediation plan:

- Unit 1 (security-critical): gw15-publish-body-cap,
  prj16-schema-via-call (CF-004 filed alkcall-side), fwd13-dot-segments,
  fwd16-missing-capability, oai11-ref-memoization
- Unit 2 (timeout/terminality): ws13-idle-progress,
  fwd15-stream-timeout, cli01-retry-after-budget, con17-mcp-pagination,
  con18-wss-sweep-exit
- Unit 3 (projection/docs): projection-truthfulness, mcp-batch-cap,
  gw16-status-drift
- Unit 4 (spec-import): yaml-normalization, oai13-path-item-wildcards,
  import-loudness-cluster, js01-placeholder-check,
  fwd17-19-contract-decisions
- Unit 5 (WS polish + tests): con18b-ws-polish,
  client-policy-wire-tests, cov-deployment-knobs, cov13-dead-code,
  srv11-srv12-router-ordering
- review-002-bracketed-followup: tentatively planned post-bulk pass
  (stale-check, OQA-18 enforcement decision, CON-08/09 close() lever,
  cross-crate re-checks) — deliberately not serialized against the
  bulk

Also: review-002 numbering repair (CON-14 was double-booked; MCP
pagination now CON-14, from_wss monitor renumbered CON-18, missing
CON-14 section added).

taskgraph: 66 valid, no cycles; 24 pending (all review-002);
gen-1/gen-2 parallel waves identified; workflow-cost hotspots are
prj16 (12.8) and ws13 (11.1), both carrying the reviewed slicing
guidance in their Notes.
2026-08-30 10:50:34 +00:00

46 KiB
Raw Blame History

Review 002 — Post-remediation review (after the review-001 sweep)

Status

Verified, open for remediation.

Scope

Consolidated review of the post-remediation tree at 91483a7 — the state after all 42 review-001 tasks (20 primary + 4 follow-up, plus re-scoped sub-task splits) were completed across the two remediation sessions. Review-001's findings clustered at the seams (auth/visibility, outbound forwarding, projection fidelity, WS edges); the remediation rewrote or heavily reworked most of that surface (~2.5k added lines net, large rewrites in forward.rs, byte_adapter.rs, routes.rs publish path, the to_openapi projection, and the WS pump consolidation). The purpose of this pass is threefold:

  1. Regression check: verify every review-001 fix holds and did not introduce new defects.
  2. New-code review: the remediation created substantial new subsystems and rewrote existing ones under time pressure — the highest-risk code in the crate is code that has never been reviewed at all (schema_cache, BufferedLines/NdjsonChunkStream, the generic pump, the retry/redirect/timeout policy stack, the $ref resolver).
  3. Fresh-eyes: anything review-001's lens missed.

The review consolidates six passes (server+gateway, WebSocket+from_wss, forwarding core+client host, from_openapi/from_jsonschema/openapi_spec, to_openapi/to_mcp/from_mcp, and a coverage pass) plus a consolidating pass that re-verified the consequential findings directly in source (and where agents disagreed, read the code to decide). Cross-crate findings are flagged as such.

Baseline verification (this pass)

cargo test --all-features            → 411 passed, 0 failed (370 unit + 41 integration)
cargo clippy --all-features --all-targets -- -D warnings → clean
cargo fmt --check                    → clean
cargo llvm-cov --all-features        → 94.87% regions / 94.63% lines / 92.22% functions

The suite is green, hygiene gates (deny(missing_docs), clippy, fmt) hold, and coverage is high. The findings below are overwhelmingly new code under review and residual gaps in remediated code — not a return of review-001's classes.

Verdict

  • The remediation held. Every review-001 finding was re-verified against its fix; 76 of 82 land FIXED-VERIFIED. The residuals are tracked below as new findings (SRV-11/12, WS-13..19, CON-14..18, OAI-10..19, JS-01..03, PRJ-16..24, CLI-01..03, FWD-13..19, GW-15..17) — none is a reversion of the original defect; each is a gap one layer short of the fix's goal, or a new-mechanism bug.
  • The one critical-class regression-adjacent defect: GW-15 — /publish moved from a buffered extractor to a raw-Body NDJSON stream (the GW-06 streaming fix) and in doing so lost both claimed memory bounds: axum's DefaultBodyLimit does not apply to raw-Body handlers (it is an extractor-side extension), and the per-line cap check only runs when a \n arrives — a never-newline chunked upload grows the heap unboundedly (the SRV-03 class on a different route).
  • One new borderline-critical defect: OAI-11 — the OAI-01 fix (cycle guard + depth budget) bounds stack, not work; an acyclic shared-$ref chain (doubly-linked-list-shaped schema, valid and common) expands exponentially and can hang import() indefinitely — the same "valid spec kills the process" class OAI-01 found, slower.
  • The dominant theme of the new findings is policy-vs-progress timeouts and terminality: the WS idle timer resets on message arrival (so the dribble stall it was built for defeats it — WS-13); the SSE subscription path inherits a 30 s total-request timeout that kills healthy long-lived subscriptions (FWD-15) while its terminal arms are untested; from_wss's new pending-sweep never exits (CON-18, a leak per dead session); and the MCP pagination loop trusts the remote to eventually stop (CON-17).
  • Projection fidelity is much improved (runtime-truthful /call, /publish, /batch verified against the wire) but two disclosure bugs remain: Internal op specs are readable through the services/schema op dispatched via /call (PRJ-16 — the GET /schema fix has a sibling hole through the op path), and the published document carries a dangling $ref (PRJ-16b) plus several wrong-status drifts (PRJ-17..21).

Severity legend

  • [critical] — the protocol cannot function end-to-end as committed; or a decided spec invariant is violated in a way that corrupts data or causes silent permanent state damage.
  • [major] — a decided behavior is missing, wrong, or a real reliability hazard; works in the happy path but fails a spec-required edge case, or is a realistic security weakness.
  • [minor] — drift, convention violation, dead code, or a doc/spec inconsistency with no correctness impact.

Part A' — Server core (regression residuals + new findings)

Regression check: SRV-01..SRV-10 all FIXED-VERIFIED (evidence per finding consolidated in the remediation ledger below), with two residuals now filed as SRV-11/SRV-12.

SRV-11 [minor] — Router-wide bearer layer double-wraps the WS upgrade route and /mcp; the SRV-10 comment claims the opposite

Verified: YES (empirically, scratch harness replicating build_router against axum 0.8.9 semantics). src/server/adapter.rs:296-345. The comment says the WS route "is registered BEFORE the router-wide bearer_auth_middleware route_layer below … so the WS path resolves the token exactly once." axum's route_layer wraps routes registered before the call — the router-wide layer at :328 is applied after the WS route was registered, so it wraps it: both layers run on GET /alk/channels and the token resolves exactly twice. Same for /mcp (own layer at :288-292 + router-wide at :328). Functionally benign today (both stashes agree), but a provider with token-use side effects double-counts and the code documents the opposite of the behavior. Ironically this reintroduces the minor SRV-10 fold-in that the remediation claimed to remove. Fix: drop one of the two layers per route (or place the WS/MCP registration after the router-wide layer) and correct the comment; add a test counting token resolutions.

SRV-12 [minor] — Decoy 405 does not cover merged extra routes; wrong-method probes on custom paths return axum's bare 405

Verified: YES (empirically). adapter.rs:314-332method_not_allowed_fallback(decoy_method_not_allowed) is registered on the default router, then extras are merged. axum applies the 405 fallback only to method routers registered at the time of the call, so DELETE /<extra-path> returns a 405 with an empty body and no Server: nginx header in a decoy deployment with custom routes — the exact probe SRV-07's fix neutralized for the default surface (verified fixed there, adapter.rs:1184-1215 test). Fix: re-apply method_not_allowed_fallback after the extras merge.


Part B' — WebSocket subsystem + from_wss

Regression check: WS-02/04/05/06/07/08/09/10/11 FIXED-VERIFIED. WS-01 and the CON-02 sweep are the two residuals (WS-13, CON-18). The pump consolidation preserved behavior parity (diff-verified 4a825d3..HEAD) and the inbound memory bound dropped from ~4 GiB to 64 MiB/conn. No reversion found.

WS-13 [major] — The idle timeout cannot fire in the WS-01 stall shape, and legitimately slow streams are killed instead

Verified: YES. src/websocket/byte_adapter.rs:229-244. The 60 s timer wraps only the next-message await. WS-01's attack — declare a 16 MiB chunk, then deliver it one byte per WS message per minute — keeps arriving messages, so the deadline resets on every dribble while the demux still waits forever for the chunk to complete: the stall that WS-01's fix exists to bound persists, knob or no knob. Conversely, a legitimate >60 s-silent session (long compute, quiet subscription) is disconnected with 1001 mid-flight, because no keepalive/ping resets the timer and chunk-level progress does not. The integration test (idle_read_timeout_resets_on_traffic) enshrines reset-on-message as correct. Fix: drive the deadline off demux progress (reset when bytes are actually forwarded into read_tx), not message arrival; add a keepalive or document the knob as an application-silence bound, not an anti-dribble guarantee. Acceptance gates: a dribble-stall test that bounds demux progress, and a slow-but-alive session that survives.

WS-14 [minor] — Byte-cap violation is detected only after the mux has committed the write

Verified: YES. byte_adapter.rs:326-334, 585-586poll_write accepts into the bounded channel first; a single write above PENDING_BUFFER_CAP (only possible from a non-chunk-framed producer) is rejected after the mux believes it written → abrupt 1011 and a truncated chunk on the wire. Self-healing and unreachable for a correct mux. Fix: reject over-cap writes in poll_write before try_send.

WS-15 [minor] — Close frames carry the hardcoded "text messages not supported" reason regardless of cause

Verified: YES. byte_adapter.rs:316-324 — the same WriteMsg:: CloseWith(WS_GOING_AWAY) is used for idle-timeout (1001), inbound-size cap (1011), and text rejection (1002); the reason string lies in two of three cases. Numeric codes (what clients key on) are correct. Fix: carry the reason in the enum variant.

WS-16 [minor] — Withdrawn during verification (merged into CON-18)

WS-17 [minor] — Bare-registry/custom WS routes get no idle-timeout knob surface

Verified: YES. src/websocket/upgrade.rs:104-111SessionState::from_registry hardcodes Some(DEFAULT_WS_IDLE_TIMEOUT) and a private 64-session semaphore; a deployment assembling its own upgrade route cannot raise the cap or disable the timeout (no extension override, unlike ChannelsPolicy). Correct-but-inflexible; one authenticated client per custom route is bounded at 64 sessions/60 s. A long-lived silent subscription deployment needs None and cannot get it. Fix: a WsIdleTimeout/WsSessions request extension mirroring ChannelsPolicy, or document the fixed values.

WS-18 [minor] — Write-side stall is unbounded: a peer that stops reading parks the mux indefinitely

Verified: YES. byte_adapter.rs:584-597 — the idle knob covers reads only. A peer that keeps the WS alive but stops reading (full TCP window) blocks the write pump in ws_sink.send, fills the 64 WriteMsg slots (bounded ~1 GiB worst case), and poll_write returns Pending — the connection's mux and handlers park until the peer resumes or an admin aborts via WsSessions. Post-auth, per-connection, bounded memory; unbounded time. Fix: a write-progress timeout in the write pump (same knob family as the read one).

WS-19 [minor] — Axum-flavor pump failure arms have no direct test; the cap-trip closes are verified on tungstenite only

Verified: YES. byte_adapter.rs:782-1206 — all unit tests instantiate TungsteniteFraming; the axum flavor's text→1002 and size-cap close are covered end-to-end via ws_upgrade_session.rs, but the write-cap (WS-04/14) and idle-close arms are exercised on one flavor only. The shared pump makes regression risk low; the flavor adapters are the untested seam. Fix: mirror one text-rejection and one cap-trip test over AxumFraming.

CON-18 [minor] — from_wss drop monitor loops forever after EOF: one 1 Hz task per dead session, permanently

Verified: YES. src/adapters/from_wss.rs:254-272 — once eof_observed holds, the sweep branch runs fail_all every second forever; there is no exit, and naive pending.is_empty() early-exit would be wrong (post-EOF registrations race fail_all — the sweep test proves they register and would hang without it). Each fire-and- forget import() whose peer dies leaves a spawned task + pending map + watch receiver alive for the process lifetime. Fix: mark the connection dead so post-EOF registrations fail fast (CONNECTION_CLOSED), then let the sweep exit after a bounded grace period.


Part C' — Gateway (regression residuals + new findings)

Regression check: GW-01..GW-14 all FIXED-VERIFIED except GW-05/GW-06 (PARTIAL) and the residuals below. The publish path's true streaming landed (NdjsonChunkStream), publish_schema validation is fail-closed with a compile-once cache, batch cap/request-id/Retry-After/SSE terminality/keep-alive all verified with real tests.

GW-15 [major] — /publish can buffer an unbounded request body despite two claimed caps

Verified: YES (source + axum-core semantics). src/gateway/routes.rs:249-253, 425-448.

  1. The handler takes raw axum::body::Body — axum's 2 MiB DefaultBodyLimit is implemented as a request extension that only extractors consult; a raw-Body handler passes the body through untouched. The doc's "in addition to axum's own 2 MiB default body limit" claim is false on this route.
  2. BufferedLines::next_line enforces MAX_PUBLISH_LINE_BYTES only when a \n is found; bytes accumulate in self.buffer unboundedly until then, and the trailing-EOF ::take(&mut self.buffer) path has no cap check at all. Concrete failure: POST /publish (pre-dispatch, permissive path) with chunked transfer-encoding streaming 'a' forever → heap grows with the upload until OOM; each poll also re-scans the whole buffer from index zero → O(n²) CPU on top. This is the SRV-03 class one route over. Fix: enforce the cap on self.buffer growth in the read loop (error once len > MAX_PUBLISH_LINE_BYTES even without a newline), and wrap the route in an explicit request-body-limit layer.

GW-16 [minor] — INVALID_INPUT status drift persists on the new hand-rolled paths (400 vs the documented 422)

Verified: YES. routes.rs:174-183, 257-278, 459-464 vs docs/architecture/http-server.md:361. The new pre-dispatch /publish and /batch rejection paths hand-build INVALID_INPUT responses with StatusCode::BAD_REQUEST (empty body, missing operation/chunk, invalid first line, line-cap, batch over-cap) while mid-stream chunk validation correctly emits the documented 422. Same error class, two statuses depending on where it fires — the GW-03 drift class, one table row down. Fix: route these through call_error_to_http_response_*, or sanction 400-for-body-shape explicitly in the doc.

GW-17 [minor] — The documented 30 s deadline does not cover the /publish final envelope; a hung sink handler holds the request forever

Verified: YES. src/gateway/dispatch.rs:132-145 — GW-05's timeout covers invoke only; invoke_sink has no deadline and the module doc justifies it ("bounded by the client's upload" — that bounds chunk arrival, not the handler's final await), while http-server.md:381-384 explicitly includes the final envelope. Fix: wrap the sink handler's completion in the same 30 s timeout, or amend the doc.

SRV-13 (recorded, non-finding) — CachedOpenAPIDoc/PublishSchemaCache staleness is structurally impossible

The registry is &mut self-registered before the Arc is shared, so post-construction mutation is impossible. The value-keyed cache invalidation is correct by inspection; the untested transition is Failed→corrected (see test gaps).


Part D' — Outbound forwarding core + client host

Regression check: FWD-02/03/04/05/06/08/09/10/11/12 FIXED-VERIFIED, FWD-01/07 PARTIAL with residuals below. The URL pipeline is substantially hardened (single-pass render, encode-set covers ?/#/ %///\, origin equality check post-set_path, userinfo reject, redirect stop on scheme/host/port change, POST/PATCH not retried, 300 s Retry-After ceiling in both seconds and date forms, real incremental SSE parser with split-UTF8 and chunk-boundary unit tests).

FWD-13 [major] — Lone ./.. path values survive the encode set and are silently normalized by Url::set_path → path/tenant escape with credentials attached

Verified: YES (empirically against the locked url 2.5.8). src/adapters/forward.rs:340-352 (encode set lacks .), :519 (set_path), :520-522 (origin check cannot catch it). {tenant} = ".." with template /tenants/{tenant}/resources renders /tenants/../resources, which set_path normalizes to /resources — a different upstream endpoint than the template describes (typically the less-scoped list-everything route), with the namespace's injected credential attached. The percent-encoded spelling is also normalized (%2e%2e), so extending the encode set does not fix it. The dot-segment test only covers the multi-segment ../../admin form that the encoder does catch. Fix: reject rendered values that are exactly ./.. in value_to_path_segment (the two normalized spellings), plus a post-set_path segment-count invariant test.

FWD-14 [minor→major-class] — forward_stream has no response size accounting; a hostile SSE upstream is bounded only by the 30 s wall clock

Verified: YES. forward.rs:880-934 — the streaming branch feeds bytes_stream() straight into SseParser; the 1 MiB cap bounds a single line, not the stream; the buffered branches carry caps, the streaming branch carries none (reqwest gzip off, so no decompression multiplier; total+read timeouts verified to apply). 30 s of 1-MiB-line events is still ~GB/s into envelope allocation. Fix: byte counter across feed on the stream path (total cap) and check buf.len() before extending so the overshoot can't exceed cap+chunk.

FWD-15 [major] — SSE subscriptions inherit the 30 s total request timeout → healthy long-lived subscriptions are killed at 30 s

Verified: YES (verified against reqwest 0.13.4 source that timeout() rides into the response body stream). forward.rs:824-845 sends the Sub forward through the shared client whose DEFAULT_REQUEST_TIMEOUT is 30 s; the gateway deliberately runs subscriptions unbounded (dispatch.rs:28, ADR-021) but the outbound half dies at 30 s with SSE stream error: operation timed out — a keep-alive-emitting source still dies. The direct dual of the read- timeout doc claim (http_client.rs:149-150). Fix: for HandlerKind::Stream forwards send with request_timeout: None (connect + read timeouts retained) via a per-request extension or a derived client.

FWD-16 [major] — Missing capability ⇒ silently unauthenticated outbound request; only malformed credentials fail loudly

Verified: YES. forward.rs:200-238 — every FWD-08 loud-error arm requires the capability to exist and be malformed; the context.capabilities.get(namespace) == None arm (the most probable misconfiguration shape — wrong capability key prefix, forgotten registration) silently sends the request unauthenticated. Fix: when auth_scheme.is_some() and the capability is absent, return INTERNAL ("capability missing; refusing to send unauthenticated").

CLI-01 [major] — Retry-After sleep sits outside the retry budget and re-arms a full ceiling on every throttled retry

Verified: YES (middleware order: RetryGateMiddleware outside RetryAfterMiddleware, http_client.rs:511-520). One logical request that gets 429/Retry-After: 300 sleeps to the deadline per attempt and each 429 re-arms now + 300 s (clamped to a full new ceiling); TotalRetryBudget gates only its own backoff sleeps, and the 30 s request timeout never starts (the sleep happens before the reqwest request is constructed) → up to ~15 min wall time inside one forward() call. Fix: sleep only when a retry will actually happen, and clamp the sleep by the remaining retry budget (check max_total_retry_duration inside maybe_sleep_for).

FWD-17 [minor] — Non-JSON SSE payloads silently degrade to JSON strings; the parser's event: field is discarded

Verified: YES. forward.rs:774-781from_str().unwrap_or(String) erases payload shape (123 vs "123") and an upstream's SSE event: error convention is indistinguishable from data. Documented JSON-or-string contract or carry the raw payload + event type. Minor.

FWD-18 [minor] — Non-scalar path-placeholder values double-route: rendered into the path and emitted as query params

Verified: YES. forward.rs:144-157is_path_placeholder skips query emission only while the raw input also renders into the path; for an object/array value under a placeholder key with an additionalProperties-allowing schema, the JSON blob is embedded in the path and the key is appended to the query. Inconsistent routing, no error. Fix: when the key matched a placeholder, never also emit it as query regardless of value shape.

FWD-19 [info] — Preserved literal % in template/base text is upstream semantics-changing (documented trade-off)

The second encoding pass deliberately preserves % (so encoded values survive), which means literal %2F sequences in template text reach the upstream as %2F (different route per most stacks). Inputs are assembly-supplied (ADR-066 trust); document it.

CLI-02 [minor] — The FWD-03 redirect fix has zero wire-level regression tests

No test in tests/ sends any 3xx; the same-host/cross-host decision, hop cap, and "credentials do not ride a cross-host redirect" property are all regression-fragile (a future edit breaks the fix with all tests green). The harness seam already exists (spawn_responder). Fix: two wire tests (same-host 302 followed with header; cross-host 302 surfaced, second endpoint never contacted).

CLI-03 [minor] — FWD-04's method gate, budget, and attempt cap are untested on the wire

No test counts upstream hits on retries, no POST-bypass test, no budget-exhaustion test, no timeout-default behavioral test — review 001's gap list flagged exactly these; the remediation added the code but not the tests. Fix: counting-responder tests (POST ⇒ 1 hit; GET 500×2 ⇒ 3 hits; budget ⇒ ≤ budget + one attempt).


Part E' — from_openapi / from_jsonschema / openapi_spec

Regression check: OAI-01 PARTIAL (stack-safe, work-unbounded → OAI-11), OAI-03/04/05/07/08/09 FIXED-VERIFIED, OAI-02/06 PARTIAL with residuals. The cycle guard is shape-safe (array-items, allOf/oneOf, additionalProperties, mid-object re-entry all verified by code-reading; the tests for those shapes are missing — OAI-16). No unwrap/expect remains in the three files' non-test code (one assert_eq! in reject_collisions → JS-03).

OAI-11 [major, borderline critical] — The depth budget bounds stack, not work: acyclic shared-$ref chains expand exponentially and can hang import() indefinitely

Verified: YES (empirically, harness of the verbatim resolver). openapi_spec.rs:382-437. The OAI-01 fix bounds recursion depth and rejects cycles, but every $ref hop re-expands its target from a fresh clone — a shared-ref chain shaped like a doubly-linked list (S_i = {a: $ref S_{i+1}, b: $ref S_{i+1}}, valid, acyclic) expands as a full binary tree: measured 20 levels → 7.3M node visits / 3.6 s; ~2^N growth means 30 levels ≈ an hour and 40 ≈ days, with no stack overflow — just a wedged import (both guards never fire). import() is assembly-time today, but the same "valid document kills the process" class as OAI-01, one mitigation shy. Fix: memoize resolved cycle- free $ref targets per document (or cap total resolved node count with a clean error). Acceptance gate: a 30-level shared-chain import returns in bounded time.

OAI-12 [major] — YAML path silently corrupts specs: .infnull constraint, duplicate keys last-wins (vs JSON's hard error), merge keys unprocessed

Verified: YES (empirically through yaml_serde 0.10.7). openapi_spec.rs:153-158 — a single from_str::<Value> with zero post-parse normalization: maximum: .inf becomes Value::Null (a declared constraint silently vanishes from the advertised schema); duplicate mapping keys silently last-win via the Value path (JSON errors on the same document — the two input formats disagree, YAML's failure mode is silent); <<: *anchor survives as a literal "<<" property in the advertised schema. Parser-level bounds (alias bombs, 128-depth) verified working. Fix: post-parse walk on the YAML path: reject non-string keys, duplicate keys, and .inf/.nan numbers; invoke apply_merge() (or reject merge keys loudly).

OAI-13 [major] — Path-item-level parameters, 2XX/4XX/5XX wildcard responses, and webhooks are still silently mishandled

Verified: YES. openapi_spec.rs:276-336 (only HTTP-method keys read — the skip filter explicitly whitelists "parameters" out of the unsupported-methods warning), from_openapi.rs:136-145.

  1. Shared path-item parameters never merge into operations → {id} unbound → the whole import fails, with a misleading diagnosis (nothing names the actual cause) against extremely common real specs.
  2. "2XX" success keys are missed by the SSE detection sweep → the OAI-06 misbehavior (stream imports as giant-text Mutation) survives one spelling away; "4XX"/"5XX" error keys drop silently with the generic warn.
  3. Top-level webhooks in mixed docs silently vanish. Fix: merge path-item parameters (operation overrides), extend both sweeps with the wildcard keys, import-or-reject webhooks loudly.

OAI-18 [minor→major-class] — Enforcement is a key-allowlist: required, enum, pattern, and value types are advertised via /schema but never enforced at call time

Verified: YES. forward.rs:288-329. A required: [id] op accepts {}; an advertised-string property sent as an object serializes as JSON text into the query string; enum/min/pattern are never consulted. Advertise/enforce drift — the contract the gateway publishes is broader than the one it defends (the OAI-02 unknown-key vector is closed; this is the declared-but-unchecked remainder). The jsonschema crate is already a dependency (schema_cache uses it). Fix: compile-and- enforce the full input schema per op (reusing the cache pattern), or document the allowlist-only scope in ADR-066 and stop advertising what is not enforced.

OAI-10 [minor] — $ref sibling keys are silently discarded (OpenAPI 3.1 divergence, no version gate)

Verified: YES. openapi_spec.rs:396-410 — under 3.1, {"$ref": …, "minLength": 3} advertises a constraint through /schema that the resolved schema (used at call time) does not carry. There is no openapi: 3.1 gate anywhere. Fix: warn on $ref siblings or gate the version, at minimum documented.

OAI-14/OAI-15/OAI-16/OAI-17/OAI-19/JS-01/JS-02/JS-03 [minor] — cluster

  • OAI-14: top-level oneOf requestBodies (unconstraining body contract), discriminator, xml, callbacks, security blocks are silently ignored — the OAI-06 loudness matrix has holes; loudness placement inconsistent (some features fail import, these don't).
  • OAI-15: a self-$ref'd requestBody resolves to content-less → body-less op registered silently (import succeeds; every call INVALID_INPUTs on body).
  • OAI-16: cycle-guard tests miss array-items/allOf/additionalProperties shapes (guard correct by inspection — insurance gap).
  • OAI-17: import errors echo unbounded spec-derived strings (100k-path servers lists → multi-megabyte error message); no credential material echoed (verified).
  • OAI-19: declared in: header params silently lose to default_headers and credential headers (last insert wins) — peer-supplied header values never reach the upstream when names collide; no import-time collision warning.
  • JS-01 [major]: from_jsonschema placeholder-binding check is skipped when input_schema has no properties key (from_jsonschema.rs:166-188is_empty → return false), so new(spec_without_properties, "/widgets/{id}") passes construction and fails every call — the exact eager-validation promise OAI-09's fix made. The from_openapi equivalent handles the empty case correctly. Fix: drop the early return; a placeholder with no declared properties is unbound by definition.
  • JS-02: from_openapi templates are not validated for balanced braces at import; /x{open imports and fails per-call INTERNAL (the OAI-09 shape, one file over — reuse from_jsonschema's check).
  • JS-03: assert_eq! in library code (reject_collisions, from_openapi.rs:74-75) — panic-family in non-test code.

Part F' — Projections (regression residuals + new findings)

Regression check: PRJ-01/03/04/05/07/08/09/10/12/13/14/15 FIXED-VERIFIED. The regenerated document's envelope shape, 422 body, error-status annotations, /subscribe contract, and securitySchemes were verified against the wire tests and are runtime-truthful except where noted below. The MCP gateway's search filter, Pub exclusion, batch shape, retryable, and wrapping are all pinned by tests. Two disclosure/correctness residuals (PRJ-16, PRJ-16b) and a drift cluster.

PRJ-16 [major, security] — Internal/ACL-restricted op specs leak through services/schema invoked via POST /call (and the MCP call tool)

Verified: YES (alkcall source + alkhttp routes). src/gateway/routes.rs:120-133 + alkcall registry/discovery.rs:327-343. call_handler's pre-check guards only the outer op name (services/schema is External): registry.invoke dispatches to services_schema_handler, which does a bare registry.registration(name)spec_to_json with no visibility and no AccessControl check. An unauthenticated caller reads the complete input/output/error schemas and access_control.required_scopes of every Internal op — the exact disclosure GET /schema (:151-156) and the MCP schema tool (to_mcp.rs:292-308) were fixed to prevent. The regenerated doc asserts "Internal (hidden from HTTP discovery, GW-02)" — currently false. (Disclosure only; the op's handler is not executed.) Fix: enforce the same internal+ACL pre-checks inside the services/schema dispatch path (alkcall-side handler fix is the complete one; an alkhttp-side input check is the local one).

PRJ-16b [major] — The published document contains a dangling $ref: #/components/schemas/CallError

Verified: YES. to_openapi.rs:602 (BatchResultEntry.error refs CallError) vs :387-413 (components defines CallFailure + per-code variants, no CallError). The openapiv3 validation test runs on an empty registry and openapiv3 does not resolve refs on parse, so nothing trips. Any generated client or validator consuming the doc fails to resolve. Fix: define the generic CallError component or point at the existing per-code variants; re-run the parse test on a populated registry.

PRJ-17 [minor] — HTTP_-prefixed op errors declared at protocol statuses clobber the shared protocol responses

Verified: YES. to_openapi.rs:694, 237-239, 295-300 — an op declaring HTTP_404@404 replaces #/components/responses/NotFound on /call, losing NOT_FOUND (still emitted by the runtime), so every real 404 violates the documented schema. Fix: merge op codes into the shared response (oneOf append) instead of overwriting.

PRJ-18/PRJ-19/PRJ-20/PRJ-21/PRJ-23/PRJ-24 [minor] — projection drift cluster

  • PRJ-18: /publish 400 over-declares INVALID_OPERATION_TYPE; the runtime maps that condition to 401 without a token (the same document's 401 entry is the true one; two docs, one scenario).
  • PRJ-19: extractor plain-text 415 and data-error 422 bodies are undocumented on /call//batch//subscribe (axum 0.8 emits them).
  • PRJ-20: /call 401 schema under-declares INVALID_OPERATION_TYPE (Sub/Pub called unauthenticated → real 401 with a code the enum doesn't allow; /publish got this right).
  • PRJ-21: /batch documents a 500 the runtime never emits (all dispatch failures are in-band entries).
  • PRJ-23: x-operation-error-statuses is emitted inside components.schemas (extension keys are legal on components, not as a schema name) — OAS-invalid on any registry with op errors; the validator test only ever runs on an empty registry.
  • PRJ-24: dead search_filter parameter in handle_batch (computed then discarded — vestigial).

PRJ-22 [minor] — MCP batch tool lacks the 100-call cap its HTTP counterpart enforces

Verified: YES. to_mcp.rs:215-247 vs routes.rs:173-184 — a 10,000-entry calls array executes serially in the dispatch spine (up to the 30 s deadline each) while /batch rejects at 101. Fix: enforce MAX_BATCH_OPERATIONS in the MCP batch tool.

CON-14 [major] — from_mcp import hangs forever (and grows memory unboundedly) on a malicious or buggy server that never clears next_cursor

Verified: YES (against rmcp 1.8.0 source). from_mcp/mod.rs:114-119 delegates to rmcp's Peer::list_all_tools, which loops while cursor.is_some() with no page cap and no timeout. A server that always returns next_cursor: Some(...) (cycling or ignoring the cursor) hangs import() indefinitely while appending every page's tools → unbounded memory growth alongside the hang. Tools from completed pages are not lost (extend per page), but nothing in alkhttp bounds the loop. Fix: replace list_all_tools with a bounded loop (max pages + overall deadline), surfacing AdapterError::DiscoveryFailed when the budget trips. Acceptance gate: a cycling-cursor paging server test terminates with a clean error.

CON-15 [minor] — from_mcp tool-name sanitization is not injective; collisions resolve by silent last-write

Verified: YES. from_mcp/mod.rs:231-252 + alkcall registration.rs:108-127"a/b", "a b", "a\tb" all sanitize to "a b"; OperationRegistry::register is HashMap::insert, so the later remote tool silently replaces the earlier one. A remote server can shadow its own tools with no diagnostic. Fix: fail import naming the colliding tools (or reject whitespace-containing names — current rejection is / + whitespace after trim; padded variants still collide with their trimmed selves).

CON-16 [minor] — Dead Capabilities entry still carries the upstream MCP token; struct doc contradicts the corrected module doc

Verified: YES. from_mcp/mod.rs:176, 393-400 — the import-time token is duplicated into HandlerRegistration.capabilities where no code reads it (the handler pins the transport header at import; CON-05 fixed the module doc but mod.rs:56's struct doc still claims per-call credential injection). Secret-minimization violation in spirit; the token also propagates into every OperationContext clone. Fix: pass Capabilities::new() or actually source per-call auth; fix the struct doc.

CON-08 (status change) — the from_mcp session leak is now honestly documented, not fixed

std::mem::forget(running) remains (mod.rs:126); no close/teardown handle exists; mod.rs:16-22 documents it as an explicit limitation (remote session + SSE stream live until the remote times out; repeated imports accumulate). Accepted-for-v1 with contract documented — recording as PARTIAL-DOCUMENTED, remediation optional (an explicit close() would be the v1.1 lever, same note as review-001 CON-09).

CON-11 residual — JSON-RPC-path MCP_JRPC_* codes are not in the declared error_schemas; Cancelled → retryable=true is unsafe for Mutations

Verified: YES. from_mcp/mod.rs:211-229, 377-389 — the new MCP_TRANSPORT_ERROR is declared (good), but JSON-RPC-error-derived codes carry http_status: None and appear in no declared schema (projects nowhere in to_openapi; surfaces as runtime 500 — defensible under the PRJ-04 annotation model, but undeclared). And a Cancelled remote error classified retryable: true invites duplicate side effects on Mutation retries. Minor.


Part G' — Coverage (cargo-llvm-cov re-run + classification)

Overall: 94.87% regions / 94.63% lines / 92.22% functions (JSON + HTML-verified per line). The prior pass's gaps mostly closed: tungstenite pumps have 8 dedicated tests (COV-03 resolved), TLS/mTLS success paths run through the real constructors (COV-02 resolved), forward.rs rose to 91%. The residue:

COV-11 — client/http_client.rs 63.2% lines / 53.3% functions: the redirect policy closure is 100% dark

The FWD-03 fix (same_host_redirect_policy, :300-316) — the crate's headline credential-exfiltration defense — has zero runtime exercise; no test in the crate follows any 3xx. Also dark: the retry gate's POST-bypass (:362-366), the wall-clock budget stop (:381), the PEM parse-failure arms (:487-507), and the config() accessor (:279-281, public API, 0 uses — also the untested half of the FWD-12 atomicity story). This is the top coverage gap in the crate — see COV-09 for the wire tests to add (shared with CLI-02/CLI-03).

COV-10 — forward_stream's terminal arms are dark end-to-end

forward.rs:887-925 (SSE parse-overflow → terminal error, transport error mid-stream → terminal error, EOF-flush of the pending event) and the streaming build/send-failure arms (:811-814, :851-853) plus the Once-path transport-error and binary-decode arms (:638-643, :656-658, :719-722). The parser is unit-tested; the wiring around it never received multi-chunk or erroring input over wire. Subscriptions are unbounded by design, so these arms are the only thing that ends a stuck subscription — they are also untested. Fix with three responder tests (oversized line, aborted socket, EOF-with-pending) + dead-port transport test.

COV-11b — Deployment knobs dark in all binaries

with_ws_idle_timeout (adapter.rs:228-242) — the WS-13-relevant server-side knob — is called by no test; a regression silently disables the idle timer on the default surface while tungstenite-side unit tests stay green. Add the 150 ms-knob 1001-close integration test + a None variant. Likewise to_mcp.rs:459-468 (call_tool, the rmcp-entered production dispatch) is dark — everything MCP is tested through a bypass harness (invoke_tool), so the real routing shell has never run.

COV-12 — Small dark arms worth folding into planned tests

  • openapi_spec.rs:428-433 — the array-recursion arm never runs in tests (the code is correct — verified — but arrays-of-$ref schemas have no test; one-line addition to the existing $ref tests).
  • openapi_spec.rs:561-566style: simple + explode: true rejection arm (the one loudness gate with a test hole).
  • from_mcp/mod.rs:405-408 — non-object tool arguments wrapped as {"value": …} (real wire behavior, untested).
  • to_mcp.rs:175, 221 — missing-argument validation for schema and batch tools (two of the four tools' argument contracts untested).
  • routes.rs:263-266 — first-line-invalid-JSON /publish 400 (the header-phase variant; only the stream-phase variant is tested).
  • decoy.rs:133-138 — directory→index.html resolution.
  • routes.rs:430-431, 551-553 — body-read-failure → chunk error (client reset mid-upload).

COV-13 — Dead code confirmed by coverage (delete, don't test)

server/state.rs:75-83 (both FromRef impls — nothing extracts those types), gateway/dispatch.rs:74-76, 79-81 (two accessors — and resolve_bearer's doc promises an auth hook the middleware never calls: wire it or delete it), websocket/upgrade.rs:145-147, and to_mcp.rs's PRJ-24 dead parameter.

Leave-uncovered (verified unreachable-by-construction): the reserved- path probe handler, StreamError glue, serialize-failure guards on acyclic inputs, depth-guard duplicates, test-support helper arms, and the stat/read race guards. Full per-range table in the coverage JSON export retained at /tmp during this session.


Cross-cutting: what is solid (verified this pass)

  • Every review-001 security fix verified fixed with tests: extra- route auth layering (now merged-before-layer, opt-out tested), /schema internal+ACL pre-checks (404/403, anonymous tested), MCP body limit (declared-CL and chunked 413 tests), hyper timer + header read timeout + keep-alive knobs, reservation-collision construction panics, decoy 405 + decode fixes (%2B, UTF-8), /openapi.json build-once cache with generic 500, gateway publish_schema fail-closed validation (schema-rejection, first-chunk, spy-handler-zero-chunks, hot-reload tests), no-store+Vary on per-identity GETs, SSE terminality + 15 s keep-alive + retry:, batch cap/request-ids, WS EOF watch signal (retention-tested), WS byte caps (accounting verified sound), 1 MiB inbound WS caps (integration-tested), shutdown close-frame cascade (acceptance-tested), session registry + 503 cap (slot-freeing tested), from_wss plaintext-ws:// refusal (message- asserted), Secret<String> builders (redacted-Debug tested), typed 401 classification (port-40101 false positive tested).
  • Credential hygiene held: no tracing of secrets anywhere in the forwarding path; loud (non-echoing) malformed-credential errors on all four value arms; redirect host-equality check compares scheme+host+port with normalized hosts; no upstream credential material appears in the served OpenAPI document (verified — HandlerRegistration.capabilities is invisible to list_operations).
  • The no-env-vars invariant, Internal-by-default visibility, and HTTP_<status> discipline all still hold (negative tests present).
  • The to_openapi document is genuinely runtime-truthful now except the specific drifts filed above (PRJ-16..23): envelope shape, 422 body, error-status annotations, in-band SSE contract, and securitySchemes were each cross-checked against routes/error sources and their tests.
  • The WS memory story is bounded in-crate: read 64×1 MiB, write 64×16 MiB + 16 MiB pending, no new unbounded accumulators (the two remaining >16 MiB buffers are alkcall-owned, unchanged).

Test gaps that would have caught the findings (semantic)

  1. A never-newline /publish body (GW-15's unbounded heap) — the existing line-cap test exercises the 400 short-circuit, not the stream path its name claims.
  2. A wrong-method probe on an extra route in decoy mode (SRV-12).
  3. A token-resolution count on the WS route (SRV-11's double-resolve).
  4. A path-param value of exactly ../. (FWD-13 — the whole dot-segment test family uses multi-segment forms the encoder catches).
  5. The dangling-CallError ref (PRJ-16b) — the openapiv3 parse test never runs on a populated registry.
  6. HTTP_404@404-style op errors (PRJ-17's clobber); a 401-body conformance test for /call (PRJ-20).
  7. A cycling-cursor MCP server (CON-14); a sanitize-collision pair (CON-15).
  8. The 30-level shared-$ref chain (OAI-11 — a bounded-time assert).
  9. YAML duplicate keys / .inf / merge keys at the from_yaml seam (OAI-12).
  10. Advertised-vs-enforced round trip: construct a doc, import, assert /schema's input_schema is exactly what build_request enforces for every branch (JS-01 and OAI-18 both survived this missing chain test).
  11. The with-properties-absent placeholder case (JS-01) — the test covers only properties-present.
  12. services/schema invoked through /call with an Internal op name (PRJ-16) — the route-level tests cover GET /schema only.

Remediation plan

Sequenced by dependency and severity; each unit independently shippable with its acceptance gate = the test that would have caught its worst finding.

Unit 1 — Security-critical (GW-15, PRJ-16, FWD-13, FWD-16, OAI-11)

The five findings with real attack or availability impact.

  • GW-15: cap BufferedLines growth pre-newline + explicit body-limit layer on /publish. Gate: never-newline upload rejected at the cap.
  • PRJ-16: internal+ACL pre-check on the services/schema dispatch input (alkcall handler preferred; alkhttp-side input check as the local layer). Gate: /call on services/schema{internal-op} → 404.
  • FWD-13: reject ./.. rendered path values (post-normalization segment-count invariant also acceptable). Gate: lone-.. value → INVALID_INPUT.
  • FWD-16: missing capability + auth_scheme → INTERNAL. Gate: the silent arm's test.
  • OAI-11: memoize cycle-free $ref expansions (or node-count budget). Gate: 30-level chain imports in bounded time.

Unit 2 — Timeout/terminality coherence (WS-13, FWD-14, FWD-15, CLI-01, GW-17, CON-17, CON-18)

One coherent theme: the new policy knobs (idle, budget, ceiling, timeout) interact with progress and terminality incorrectly.

  • WS-13: progress-based deadline + keepalive decision + two acceptance tests (dribble bounded; silent-but-alive survives).
  • FWD-14/FWD-15: stream byte cap + subscription sends with request_timeout: None.
  • CLI-01: budget-aware Retry-After sleep.
  • GW-17: wrap sink completion in the 30 s deadline (or fix the doc).
  • CON-17: bounded MCP pagination loop (max pages + deadline).
  • CON-18: dead-connection fast-fail + bounded sweep exit.

Unit 3 — Projection + doc truthfulness (PRJ-16b, PRJ-17..24, WS-15) + gateway status drift (GW-16)

Mostly mechanical; golden-doc work regains its review-001 lesson: the validation tests must run on a populated registry.

Unit 4 — Spec-import gaps (OAI-12, OAI-13, OAI-10/14/15/16/17/19, JS-01/JS-02/JS-03, COV-12's openapi arms)

YAML normalization pass, path-item parameters + wildcard responses, and the loudness-matrix completion. JS-01 is a one-line fix with a one-line test — fold it into any passing commit.

Unit 5 — WS polish (WS-14..19) + test-gaps batch (CLI-02/CLI-03, COV-09..12)

Wire tests for the redirect/retry/timeout policy stack, the streaming terminal arms, the idle-knob integration test, plus the small arm tests. Also delete the COV-13 dead code.


Verification log (this pass)

  • All findings carry file:line references verified against tree 91483a7. The consolidating pass directly re-verified: GW-15 (BufferedLines unbounded growth, raw-Body handler, extractor-limit bypass), PRJ-16 (alkcall services_schema_handler bare-registration disclosure + alkhttp outer-name-only pre-check), FWD-13 (encode set
    • Url::set_path normalization against url 2.5.8), WS-13 (timer placement), JS-01 (is_empty → false early return), PRJ-16b (dangling ref vs components contents), and the array-$ref recursion arm (correct — one agent claim demoted to a coverage-only gap).
  • Empirical verifications (scratch harnesses outside the repo, no repo changes): SRV-11/SRV-12 router-shape probes (extra-route 405 shape, double token resolution), OAI-11 exponential-ref expansion (7.3M visits at 20 levels), OAI-12 YAML corruptions, FWD-13 dot-segment normalization, reqwest timeout-into-stream behavior.
  • Coverage figures: cargo llvm-cov --all-features (summary + JSON + HTML), 2026-08-30.
  • Six subsystem passes ran in parallel over disjoint file sets; two agent findings were rejected at consolidation (coverage-overreported dark array-$ref arm — code verified correct; WS-16 — duplicate of CON-18) and several "uncovered" flags were reclassified as cross-binary artifacts after HTML-line verification.