Skip to content

fix(sdk): cookie-authenticated streams bypass the redirect guard #478

Description

@EricAndrechek

Area: sdk — streaming. Found reviewing #470; documented there as a known limitation rather than fixed.

The streaming transport refuses redirects when the request carries a credential, because a cross-origin hop strips Authorization and /v1/stream answers an unauthenticated caller with a default_role view instead of an error — a silent downgrade. The test is:

// clients/ts/src/stream/sse.ts:387-388
const credentialed =
  base.Authorization !== undefined || Object.keys(this._opts.headers ?? {}).length > 0;
// :417
redirect: credentialed ? "manual" : "follow",

Scope correction (from a later review round). The original text below said any cookie-authenticated stream downgrades on a redirect. That is too broad, and gets the mechanism wrong in a way that matters for the fix. Unlike Authorization, a cookie is not stripped on a redirect — it is re-derived from the cookie store at each hop, so a redirect carries it only while the hop stays inside both the request's origin and the cookie's own Path. A same-origin trailing-slash 301 keeps it; a same-origin rewrite from /v1/stream to /stream under a Path=/v1 cookie does not. (An earlier revision of this note offered a path rewrite as an example that carries the cookie — that was wrong.)

The gate is the request's origin, not the cookie's scope. Under the default same-origin credentials mode nothing is attached once a redirect goes cross-origin, whatever the cookie's Domain — so app.example.comapi.example.com sharing a Domain=example.com session cookie loses it, and so does an http→https upgrade (cross-origin by scheme and port) even though a non-Secure cookie's scope spans both. So the affected shape is: a browser stream on a same-origin baseURL under the default credentials mode, redirected cross-origin.

credentials: "include" is a separate story. Cross-origin against WaveHouse's own CORS it does not downgrade at all — WaveHouse never emits Access-Control-Allow-Credentials (AGENTS.md invariant 16, internal/api/router.go:278), so the browser blocks the response and the transport sees a TypeError → retryable SSE_NETWORK_ERROR, re-dialled forever. That is the failure mode sse.ts deliberately rejects redirect: "error" for, and option 1 would turn it into a terminal, inspectable SSE_REDIRECT instead — so it is worth having for that reason alone, quite apart from the downgrade.

This does not weaken option 1. credentials: "include" is not a synonym for a cross-origin baseURL: the SDK recommends credentials for a cookie-authenticated origin, and the only configuration where cookie auth against WaveHouse works at all is a same-origin baseURL — which works today, fails nothing loudly, and is exactly the slice option 1 detects. (An earlier revision of this note claimed the opposite; it was wrong.)

Cookies are a credential and are not in that test. In a browser init.credentials is preserved (sse.ts:428), so a stream authenticated by cookie — either fetchOptions: { credentials: "include" }, which the SDK documents for exactly this case, or the implicit same-origin default on a cookie-authenticated origin — computes credentialed === false and gets redirect: "follow". The browser will not send the source origin's cookies to a different origin, so the redirect arrives unauthenticated and the stream downgrades silently. That is the outcome the rule exists to prevent, reached through the one credential the rule cannot see.

Severity

Low. Browser-only (credentials is dropped elsewhere), and it needs a redirect on /v1/stream plus a target that answers unauthenticated. The cookie itself does not leak — a browser will not forward it cross-origin any more than it forwards Authorization. The harm is the silent role downgrade, identical to the bearer-token case.

Options

  1. Widen the test to include an explicit opt-in: treat fetchOptions?.credentials === "include" as credentialed. Cheap and correct as far as it goes, but it cannot cover the implicit same-origin default, which is indistinguishable from "this deployment has no cookies at all". So the documented caveat has to stay either way.
  2. Refuse redirects on /v1/stream unconditionally in a browser. Consistent and simple to reason about, but it gives up the uncredentialed cases the current rule deliberately supports — CDN canonicalization, geo/LB indirection, an http→https upgrade.
  3. Expose the choice as a streamRedirect: "follow" | "error" option, defaulting to today's heuristic. Most flexible, one more knob.
  4. Documentation only — what feat(sdk)!: stream over fetch so SSE authenticates by header #470 shipped. The caveat is now stated in clients/ts/src/types.ts, sse.ts, sdk/index.mdx, and the CHANGELOG.

(1) plus the existing caveat is probably the right increment; (3) is worth considering if anyone hits the uncredentialed-redirect case in practice.

Acceptance

  • A stream with fetchOptions: { credentials: "include" } and no bearer token refuses a redirect, or the decision to leave it following is recorded here
  • Whatever the outcome, the four documentation copies agree with the code
  • Pinned by a test asserting redirect for each combination of bearer token / configured headers / credentials

Related: #470 (introduced the rule and documented this gap), #471 (redirect handling on a resumption preflight).

Metadata

Metadata

Assignees

No one assigned

    Labels

    area/sdkTypeScript SDK (clients/ts/)bugSomething isn't working

    Type

    No type

    Projects

    Status
    Backlog

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions