From 1dc1d5af4fbe5c644e7996785bbe9ad0c528877f Mon Sep 17 00:00:00 2001 From: "glm-5.3-flash" Date: Sat, 29 Aug 2026 10:46:18 +0000 Subject: [PATCH] docs(tasks): review-001 response-decode fidelity (FWD-07/08/10/12) complete --- .../review-001-response-decode-fidelity.md | 71 ++++++++++++++++--- 1 file changed, 61 insertions(+), 10 deletions(-) diff --git a/tasks/adapters/review-001-response-decode-fidelity.md b/tasks/adapters/review-001-response-decode-fidelity.md index 14b616a..90c17ce 100644 --- a/tasks/adapters/review-001-response-decode-fidelity.md +++ b/tasks/adapters/review-001-response-decode-fidelity.md @@ -1,7 +1,7 @@ --- id: review-001-forward-response-fidelity name: Upstream response decode — vendored JSON, size caps, loud auth errors, error bodies (FWD-07, FWD-08, FWD-10, FWD-12) -status: pending +status: completed depends_on: [] scope: moderate risk: medium @@ -44,14 +44,14 @@ Review 001 findings on `src/adapters/forward.rs`'s response-decode paths ## Acceptance Criteria -- [ ] `application/vnd.api+json` (and `application/problem+json`) decode as JSON, not byte arrays (test) -- [ ] Response size cap enforced on JSON/text/binary read paths (test) -- [ ] Non-SSE content-type on a Sub op produces a loud error, not an empty stream (test) -- [ ] Invalid credential values fail loudly at call time (test), no credential material logged -- [ ] Upstream 4xx/5xx bodies surface bounded in the error envelope (test) -- [ ] Duplicate `value_to_*` helpers collapsed; `text/` buffering branch removed or safe; reload atomic -- [ ] `forward.rs` COV-01 uncovered regions (binary branch, auth arms, non-2xx mapping, content-type branches) substantially covered -- [ ] `cargo test` and `cargo clippy --all-targets -- -D warnings` pass +- [x] `application/vnd.api+json` (and `application/problem+json`) decode as JSON, not byte arrays (test) +- [x] Response size cap enforced on JSON/text/binary read paths (test) +- [x] Non-SSE content-type on a Sub op produces a loud error, not an empty stream (test) +- [x] Invalid credential values fail loudly at call time (test), no credential material logged +- [x] Upstream 4xx/5xx bodies surface bounded in the error envelope (test) +- [x] Duplicate `value_to_*` helpers collapsed; `text/` buffering branch removed or safe; reload atomic +- [x] `forward.rs` COV-01 uncovered regions (binary branch, auth arms, non-2xx mapping, content-type branches) substantially covered +- [x] `cargo test` and `cargo clippy --all-targets -- -D warnings` pass ## References @@ -63,6 +63,57 @@ Review 001 findings on `src/adapters/forward.rs`'s response-decode paths > review-001-forward-url-safety (request side) and > review-001-sse-parser — sequence those first or coordinate. +FWD-07: JSON dispatch is now mime-essence based +(`is_json_content_type`: type `application`, subtype `json` or `+json` +suffix; hand-rolled — `mime` is in the lock file only as a transitive +dep of reqwest, not a direct dependency, so no new crate). Read paths +(JSON/text/binary) are byte-capped by `read_body_capped` at +`RESPONSE_BODY_CAP` = 16 MiB; over-cap surfaces `HTTP_413`. +`forward_stream` checks the `Content-Type` essence (`text/event-stream`) +and a non-SSE 2xx yields one `INVALID_RESPONSE_TYPE` error envelope +instead of an empty stream. FWD-08: every silently-dropped header — +default headers (`build_request`), and the Bearer/ApiKey/Basic +credential arms — now returns a loud `CallError::internal` ("refusing +to send the request unauthenticated"); error text names the header or +namespace but never echoes credential material (tested). FWD-10: +non-2xx in both `forward` and `forward_stream` goes through +`error_envelope`: `HTTP_` mapping unchanged (ADR-023) plus a +bounded echo of the upstream body — read once, capped at +`ERROR_BODY_ECHO_CAP` = 4 KiB, control characters stripped (newline/tab +kept), `[truncated]` marker when the read hit `STATUS_BODY_DRAIN` +(64 KiB) without EOF, in which case the connection is dropped rather +than drained further (bounded connection-reuse trade). FWD-12: +`value_to_path_segment`/`value_to_query` collapsed onto one +`scalar_value_to_string` extractor (the raw scalar is shared; path +rendering applies `PATH_VALUE_ENCODE_SET`, query emission stays raw for +`query_pairs_mut` — they were byte-identical pre-FWD-01 but the roles +diverged, so a single raw-then-encode function was wrong for the query +arm and the pre-existing `+`-encoding test proved it); `forward` lost +its `op_type` parameter (the Sub branch only toggled ACCEPT and the +`text/` buffering branch is gone — Sub ops route through +`forward_stream`); request-body serialization failure is a loud +`CallError::internal` instead of a silent `null` body (both handlers); +`SharedHttpClient` now holds client+config in one +`ArcSwap` so `reload` swaps both in a single atomic +store (minimal touch to `src/client/http_client.rs`, public API +`client()`/`config()` unchanged). COV-01 regions covered: vendor/`+json` +content-type branches, binary branch, all three auth arms (loud error +paths + the pre-existing success tests), non-2xx mapping with error-body +echo (2/404/429/500, truncated and exact), cap trips on all three read +paths, non-SSE Sub stream, SSE happy path through `forward_stream`. + ## Summary -> Filled on completion. \ No newline at end of file +FWD-07/08/10/12 remediated in `src/adapters/forward.rs` (+ +one-spot `SharedHttpClient::reload` atomicity fix in +`src/client/http_client.rs`): vendor-JSON mime-essence decode, 16 MiB +`RESPONSE_BODY_CAP` on all buffered reads (`HTTP_413` on breach), +loud `INVALID_RESPONSE_TYPE` on non-SSE Sub-op streams, loud call-time +errors for invalid credential/default-header values (no secret echo), +bounded 4 KiB error-body echo on non-2xx with a 64 KiB +`STATUS_BODY_DRAIN` read cap, duplicate scalar helpers collapsed, +`forward`'s dead `op_type` Sub branch removed, serialization failure +made loud, and the hot-reload swap made atomic (joint client+config +holder). 23 tests in `forward.rs::tests` (was 9) covering the previously +uncovered binary/auth/non-2xx/content-type regions (COV-01); full +suite 265 passing. Commits 9bb8487 (core) and 7ac2dc5 (tests). \ No newline at end of file