Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions docs/deviations.md
Original file line number Diff line number Diff line change
Expand Up @@ -482,6 +482,8 @@ frozen tree and is amended only deliberately, by hand. When §10 is next amended
| **`XCUT-16`'s replay guard is keyed on whether the hop was guarded, not on whether the replacement looks credentialed.** `XCUT-16` and `AUTH-28` say the guard applies "on any path where a credential will be attached", and carve out "a deliberately credential-free re-issue MAY proceed over any scheme". Deciding which of the two a challenge replacement is cannot be done by reading header names: the step's own `ApiKeyCredentialConfig.headerName` stamps whatever header the caller names, and a `challengeHook` may invent a carrier this step has never been told about. The port therefore reads "a credential will be attached" as a property of the HOP — if the outbound pass ran the HTTPS guard, so does the replay, whatever URL and headers the hook chose. **Strictly wider than the requirement's letter**, and knowingly so: it refuses a downgraded replacement that carries no credential at all, on a hop that is credentialed. The carve-out is preserved where it is observable — a `NO_AUTH` hop is never guarded outbound, and its replay is guarded only when the replacement carries `Authorization` or `Proxy-Authorization`, which is the previous rule kept as a second arm. *Rejected:* deriving the credential-carrying header names from configuration, which misses the hook-invented carrier and is the shape that let the reported leak through | audit #67 / #71 | 2026-09-04 | `docs/product-spec/19-cross-cutting-invariants-and-policies.md:44` is the requirement and its carve-out. `packages/core/src/auth/auth-step.ts:389` sets `OutboundPlan.guarded`; `:564-575` is `guardReplayScheme` and its two arms. Pinned by "a replacement carrying a NON-standard credential header over plaintext is refused" and "a header-free replacement over plaintext is refused too" in `packages/core/src/auth/auth-step.test.ts`, and by the "XCUT-16: a guarded hop stays guarded across a challenge replay" block in `tests/conformance/xcut/security-by-default.conformance.test.ts` | not yet in §10 |
| **`ASYNC-21`'s "MUST NOT close the caller-owned source on any termination" is not honoured: the RxJS SSE adapter takes ownership and closes.** `sseEvents$` and `typedSse$` pass `() => stream.close()` as `fromAsyncIterable`'s `release`, and RxJS runs a subscriber's finalizer on *every* termination — unsubscription, end-of-source and a source error alike, which is the complete list the clause names. **Kept, deliberately, on two grounds.** (1) **The clause has no subject on this platform.** It presumes a source whose iterator return leaves the source open; this port's `SseStream` is deliberately not that one. `#iterate`'s `finally` calls `#releaseQuietly()`, so the resource is released whenever the runtime drives `return()` — which `fromAsyncIterable` must do exactly once (`ASYNC-6`), and which a plain `for await` with `break` does too. Removing the callback would change which channel reports a release failure and when the release runs, not whether the caller-owned source ends up closed. (2) **The ordering is load-bearing.** The release runs *ahead of* `iterator.return()` because an async generator's `return()` queues behind a suspended `next()`, and an SSE stream idling between events is parked in exactly that pull — so without the callback an `unsubscribe()` stays pending until the server next sends a byte, holding the socket open indefinitely. Measured: deleting the two `release` arguments turns four cases red — the two suspended-pull ones, as "the teardown did not settle within 500ms", and the two pre-existing idle-unsubscribe assertions — while every exactly-once release count stays green, which is the shape of the claim. Pagination attaches no release for the complementary reason: a `Paginator`'s pulls are bounded HTTP exchanges, never a wait on a server that may never answer. *Rejected:* dropping the callback to match the letter (reintroduces the hang for no change in what closes). *Rejected:* a caller-facing `{ownership}` option (two behaviours to document for a case with one correct answer). The public TSDoc and `packages/rx/README.md` now state the transfer outright — subscribing hands the stream over, do not close it yourself and do not iterate it afterwards — rather than leaving the `ASYNC-21` citation on the doc comment's first line to read as satisfied | audit #67 / #75 | 2026-09-05 | `packages/rx/src/sse.ts:46` and `:73-75` are the two `release` arguments; `packages/rx/src/from-async-iterable.ts:103-108` is the teardown that runs one on every termination; `docs/product-spec/18-asynchronous-runtime-adapter-contract.md:42` is the requirement. Ground 1: `packages/core/src/sse/stream.ts:136-139` (`#iterate`'s `finally` → `#releaseQuietly()`) with `:117-121` (`close()` memoized, `SSE-28`). Ground 2: `packages/rx/src/from-async-iterable.ts:44-48` states the ordering and why. Pinned by the two `resource ownership` blocks in `packages/rx/src/sse.test.ts`, which count the release the OWNED resource sees rather than `SseStream.close()` calls — the facade memoizes, so a facade-level count reads "once" however many paths call it — and by "SSE ownership transfer releases once on Node" in `tests/node-conformance/rx-bridge.test.mjs`. Phase 8b marked `ASYNC-21` ✅ with this clause dropped from its gist (`docs/work/mvp/phase8/phase8b/2026-07-28-phase8b-async-runtime-checklist.md:67`); that is a dated record and is left as written. The other half is `SSE-41`'s own "documented source ownership" clause, which the same checklist marked ✅ (`:74`) on the strength of documentation that named unsubscription only — completed by the TSDoc and README rewrite this row accompanies | not yet in §10 |
| **`AUTH-22`'s "emit cnonce/nc/qop only when qop is negotiated" is not applied to `cnonce` for a `-sess` algorithm.** A `-sess` HA1 is `H(H(user:realm:pass):nonce:cnonce)` (RFC 7616 §3.4.2), so the client nonce is an *input to the hash* for `MD5-sess` and `SHA-256-sess` whatever `qop` the challenge offered. The port implemented AUTH-22 to the letter: it drew a fresh cnonce, folded it into HA1, and then omitted it from the header whenever `qop` was absent — a response no server can verify, because it has no way to reconstruct HA1. AUTH-30 bounds the re-challenge replay to one 401, so every such exchange simply failed. **`cnonce` is now emitted for any `-sess` algorithm; `nc` and `qop` stay conditional exactly as AUTH-22 says**, because RFC 2069's response input is `H(HA1:nonce:HA2)` and carries no nonce count, so emitting one would advertise a count the response was not computed over. RFC 7616 §3.4 states the wider rule outright — "cnonce: This parameter MUST be used by all implementations". AUTH-22's clause is RFC 2617's RFC 2069-compatibility form, written before `-sess` existed, and the requirement's own AUTH-15 mandates both `-sess` algorithms, so the two sentences cannot both be followed. *Rejected:* declining a `-sess`-without-`qop` challenge instead, which turns every such server into a guaranteed 401 for no security gain, when the value the server needs has already been computed | audit #67 / #74 | 2026-09-05 | `packages/core/src/auth/digest.ts:345-350` (the `-sess` HA1 that consumes the cnonce) against `:405-408` (`buildHeaderValue`, where the `else if` now emits it); `docs/product-spec/11-authentication.md:18` and `docs/product-spec/appendix-c-consolidated-normative-requirement-index.md:357` are AUTH-22's wording. Pinned by the `digestHandler -sess without qop (AUTH-17/AUTH-22)` block in `packages/core/src/auth/digest.test.ts` — one row asserting the header carries `cnonce` and neither `nc` nor `qop`, one recomputing the response from the header's OWN cnonce so a value drawn twice would fail — and by the `MD5-sess, no qop` vector in the same file | not yet in §10 |
| **`HTTP-35`'s timeout check is read as the FULL range `AbortSignal.timeout()` accepts, not the lower bound the requirement enumerates.** `HTTP-35` says the options builder "MUST reject a non-null timeout that is zero or negative". `RequestOptionsBuilder.timeoutMs` rejects three more classes: non-finite (shipped unledgered before this audit), non-integer, and anything above `2**32 - 1`. **Strictly stricter than the letter, and deliberately so.** The field has exactly one consumer — `composeSignal` hands it to `AbortSignal.timeout()` — so a value this setter admits and that function refuses is `HTTP-35`'s own failure mode with the seam moved: the error surfaces inside a transport, as an unwrapped platform `RangeError`, one frame away from the call that supplied it. The earlier reading accepted `1.5` and argued in TSDoc that "a timeout is a duration and a fractional millisecond is meaningful"; no consumer of the field can express one. **The range checked is Node's, and that is the point:** `AbortSignal.timeout(1.5)` and `AbortSignal.timeout(2 ** 32)` raise `RangeError` on Node and are ACCEPTED on Bun, and a negative delay is `RangeError` on Node against `TypeError` on Bun (measured 2026-09-05), so leaving the check to the runtime would make an SDK-level contract depend on which runtime the caller happens to be on. *Rejected:* rounding with `Math.ceil` and clamping inside `composeSignal`, which hides the caller's mistake in the one place `HTTP-35` exists to surface it. `composeSignal` is documented as still able to raise, because a transport's own `defaultTimeoutMs` construction option bypasses this setter and is not validated by core — recorded for #81/#82, not fixed here | audit #67 / #76 | 2026-09-05 | `docs/product-spec/04-core-http-domain-model.md:48` is `HTTP-35`'s wording. `packages/core/src/http/request-options.ts:12` (`MAX_TIMEOUT_MS`) and `:204-214` (the check and the rewritten TSDoc paragraph); `packages/core/src/seams/transport.ts:86-92` is `composeSignal`'s new `@throws`, which states the two-runtime divergence rather than naming one error class. Pinned by "rejects a fractional timeout, which no transport deadline can honor" (`packages/core/src/http/request-options.test.ts:128`, the FLIPPED case — it pinned acceptance until this audit), "rejects a timeout above AbortSignal.timeout()'s ceiling of 2**32 - 1" (`:134`), "accepts the ceiling itself" (`:143`) and the `every accepted timeout is an integer in 1..2**32 - 1` property (`:157`); the Node half is `composeSignal timeout range on Node (HTTP-35)` in `tests/node-conformance/seams.test.mjs:105`, which cannot live in `bun test` because Bun accepts both rejected values | not yet in §10 |
| **`HTTP-31`'s "falls back to raw text rather than throwing" is satisfied for an unpaired surrogate by SUBSTITUTING U+FFFD, not by keeping the raw text.** `HTTP-31` (MUST) makes `QueryParams.parse` lenient and enumerates the lenient cases, ending with "malformed percent-encoding falling back to raw text rather than throwing". An unpaired surrogate is a fourth kind of malformed input the enumeration does not name, and the fallback it prescribes is not available for it: the raw text has no UTF-8 form, so keeping it produces a `QueryParams` whose `encode()` throws `URIError` — the throw merely deferred out of `parse` and into an accessor that documents no throw at all. **The port repairs instead.** `parse` runs `toWellFormed()` over each decoded name and value, so every instance it returns is encodable, which is what "parsing MUST invert encode" needs to mean. The strict half of the rule is unaffected and is where `#76` puts the rejection: `QueryParamsBuilder.add` throws `UrlConstructionError` for the same input, and `substitutePathParams` throws `OperationAssemblyError`. That asymmetry is not new to the query model — it is exactly the outbound/inbound split `Headers` already draws for `HTTP-18` against `HTTP-19`, applied to the one requirement pair that needs it here. Substitution matches the platform rather than inventing a policy: `new URL('https://x/?a=\uD800').search` is `?a=%EF%BF%BD` (measured 2026-09-05). *Rejected:* letting `parse` throw the builder's error, which breaks a MUST. *Rejected:* dropping the offending parameter, which loses a name the caller may be matching on | audit #67 / #76 | 2026-09-05 | `docs/product-spec/04-core-http-domain-model.md:42` carries `HTTP-31`'s wording (shared with `HTTP-30`). `packages/core/src/http/rfc3986.ts:17-18` are the two patterns, `:31` `hasLoneSurrogate` (strict) and `:44` `toWellFormed` (lenient) — one rule, two entry points, so no caller can pick the wrong one; `packages/core/src/http/query-params.ts:144-150` is `parse`'s repair with the `HTTP-18`/`HTTP-19` comparison stated inline, against `:44-50` and `:240-241` for the strict `add` path; `packages/core/src/seams/operation.ts:139-144` is the path-param half. `/\p{Surrogate}/u` rather than `String.prototype.isWellFormed()` because the latter is ES2024 and `tsconfig.base.json:5-11` pins `lib: ES2023`, though the `engines.node >= 20.3` runtime has it. Pinned by the `lone surrogates are rejected where they are supplied (HTTP-29, HTTP-31)` block in `packages/core/src/http/query-params.test.ts:170` — "parse() stays lenient and substitutes U+FFFD, because HTTP-31 forbids throwing" (`:197`) and the `no anything escapes parse()` property (`:233`) | not yet in §10 |

### Proposed erratum for `PIPE-40` (drafted 2026-09-04, not applied)

Expand Down
5 changes: 3 additions & 2 deletions docs/sdk-documentation/http.md
Original file line number Diff line number Diff line change
Expand Up @@ -110,8 +110,9 @@ const options = RequestOptions.newBuilder()
`RequestOptions.EMPTY` is the shared no-op instance. A step reads it as `ctx.options`, and a
transport receives it as `send()`'s second argument. Both range checks are the **full** range, not
only the lower bound: `maxRetries` rejects anything that is not a non-negative integer, and
`timeoutMs` rejects zero, negatives, `Infinity` and `NaN` alike (`HTTP-35`). A fractional
`timeoutMs` is accepted — a timeout is a duration, not a count.
`timeoutMs` rejects zero, negatives, `Infinity`, `NaN`, a fractional value and anything above
`2**32 - 1` (`HTTP-35`) — the range is `AbortSignal.timeout()`'s, the only one a transport can
honor.

`auth` on the builder is the **per-call** auth tier, the highest-precedence one; see
[`auth.md`](./auth.md).
Expand Down
16 changes: 16 additions & 0 deletions packages/core/src/http/builder.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,22 @@ export interface Builder<T> {
build(): T;
}

/**
* The one frozen empty list every multi-value accessor returns for an absent name.
*
* Shared, not allocated per miss, and frozen for the same reason the present-name lists are:
* HTTP-5's accessors "MUST NOT let a caller mutate the model through the returned value", and the
* TSDoc on `Headers.getAll` and `QueryParams.getAll` promises a frozen list on every path. Both
* returned a fresh `[]` on a miss, which was neither (audit #67 / #76). Sharing one instance is
* safe precisely because it is frozen — there is no state in it to alias, and no caller can add
* any.
*
* Lives here rather than in either model because both need it and the two models deliberately
* import nothing from each other; this module is already the shared construction helper they both
* import from.
*/
export const EMPTY_VALUE_LIST: readonly string[] = Object.freeze([]);

/**
* Returns `value` when present, throwing a field-named error when it is `null` or `undefined`.
*
Expand Down
31 changes: 27 additions & 4 deletions packages/core/src/http/errors.ts
Original file line number Diff line number Diff line change
Expand Up @@ -127,16 +127,39 @@ export class MediaTypeParseError extends DexpaceError {}
export class ProtocolParseError extends DexpaceError {}

/**
* Thrown when a request URL is malformed or not absolute; the message carries the offending input
* and the underlying parse failure is chained as `cause` (HTTP-47).
* Thrown when a URL cannot be constructed from what a caller supplied.
*
* Three cases, all of them "this input has no URL form":
*
* - A request URL that is malformed or not absolute (HTTP-47). The message carries the offending
* input and the underlying parse failure is chained as `cause`.
* - A base URL handed to `buildRequest()` that is malformed, not absolute, or carries a fragment
* (SEAM-27).
* - A query-parameter name or value carrying an unpaired surrogate. Such a string has no UTF-8
* form, so RFC 3986 percent-encoding is undefined for it and `QueryParams.encode()` could only
* fail; `QueryParamsBuilder.add` rejects it at the call that supplied it instead. `cause` is not
* set on this path — nothing was caught, the input was inspected (HTTP-29, audit #67 / #76).
* `QueryParams.parse` does NOT throw it: HTTP-31 makes parsing lenient, so it substitutes U+FFFD.
*
* @public
*/
export class UrlConstructionError extends DexpaceError {}

/**
* Thrown when a per-call operational override is out of range — a non-null timeout that is zero or
* negative, or a negative max-retries (HTTP-35).
* Thrown when a per-call operational override is out of range (HTTP-35).
*
* The ranges checked are the FULL ranges, not the lower bounds the requirement's own wording names.
* HTTP-35's point is that an out-of-range override is a loud error at the call site that supplied
* it, never a value reinterpreted downstream, and a value that only *some* consumer refuses is the
* same failure moved one seam away:
*
* - `timeoutMs` must be an integer in `1 .. 2**32 - 1` — the range `AbortSignal.timeout()` accepts,
* which is the only one a transport can honour. Zero, negatives, `Infinity`, `NaN`, a fractional
* millisecond and anything above the ceiling are all rejected. Zero is rejected rather than
* reinterpreted: it means "no timeout" in one transport and is an error in another.
* - `maxRetries` must be a non-negative integer. `0` is accepted and means "disable retries for
* this call", distinct from `undefined`; `Infinity` and `NaN` are rejected because they make a
* retry driver's ceiling test permanently false and its loop unbounded.
*
* @public
*/
Expand Down
Loading
Loading