Skip to content

fix(http): gate streamRequest XSRF cookie read on withXSRFToken — closes queue #64#86

Closed
Goosterhof wants to merge 2 commits into
mainfrom
engineer/queue-64-streamrequest-xsrf-gate
Closed

fix(http): gate streamRequest XSRF cookie read on withXSRFToken — closes queue #64#86
Goosterhof wants to merge 2 commits into
mainfrom
engineer/queue-64-streamrequest-xsrf-gate

Conversation

@Goosterhof
Copy link
Copy Markdown
Contributor

Summary

Gate streamRequest's document.cookie XSRF read on options?.withXSRFToken ?? false, matching the default-resolution semantics applied to the axios methods at createHttpService:39. The streaming transport now honors the same withXSRFToken config as the standard request methods.

Closes enforcement queue #64 (war-room item — see enforcement/queue.md).

Why

packages/http/src/http.ts:108-128 (pre-fix) unconditionally read the XSRF-TOKEN cookie on every streaming POST and emitted X-XSRF-TOKEN, regardless of the service's withXSRFToken setting. This violated the library invariant established at line 39, where withXSRFToken: false (the factory default) gates the axios-method cookie read.

The asymmetry was surfaced by Sapper M3 Finding #1 during the 2026-05-15 staleness refresh. The same PR #62 hardening pass that wired withCredentials to honor consumer config in streamRequest did not apply the parallel discipline to withXSRFToken. Author comment at the original lines 116-119 documents the withCredentials discipline; this PR extends it to XSRF.

Severity recalibration (General-verified)

Sapper M3 originally rated this High conditioned on the consumer scan. General-led verification (grep -rn "streamRequest(" territories/{kendo,brick-inventory-orchestrator,the-laboratory,ublgenie,entreezuil,emmie}/) confirmed zero production call sites to streamRequest across the entire consumer fleet — only export/re-export, mock-server, and vi.fn() references. Severity drops to Medium correctness defect: real library invariant violation, no exploiting consumer today, would catch the first consumer to adopt the stream API without auditing the code path.

The library-side fix is warranted as defense-in-depth and to close the test gap that lets the defect persist undetected. Campaign report: campaigns/fs-packages/2026-05-15-m3-staleness-refresh-wave.md.

Changes

  • packages/http/src/http.ts:108-131 — gate cookie read on wantsXsrf = options?.withXSRFToken ?? false. One-line sibling comment naming the discipline (symmetric with axios methods at line 39).
  • packages/http/tests/http.spec.ts — five XSRF cases in the streamRequest block:
    • reads XSRF token from cookies when withXSRFToken: true (reworked)
    • omits XSRF header when withXSRFToken: true and no cookie present (reworked sanity-check; gate-open case)
    • decodes URL-encoded XSRF token when withXSRFToken: true (reworked)
    • omits XSRF header when withXSRFToken: false even with cookie present (new — the bypass case the test gap missed)
    • omits XSRF header by default (withXSRFToken unset) even with cookie present (new — confirms ?? false default semantics)
  • packages/http/package.json — bump 0.3.0 → 0.3.1 (additive guard on previously-undefined behavior; no API change).
  • packages/http/CHANGELOG.md — prepend ## 0.3.1 — 2026-05-15 entry.
  • package-lock.json — single-line version-string regeneration; no nested @script-development/* copies (verified).

Verification — 8 fs-packages CI gates

Gate Result Note
1. npm audit PASS 0 vulnerabilities, 605 deps
2. npm run format:check PASS 544 files, oxfmt clean
3. npm run lint PASS 0 warnings / 0 errors, 341 files
4. npm run build PASS 11 packages built (incl. fs-cached-adapter-store from rebased main)
5. npm run typecheck PASS exit 0 across all workspaces
6. npm run lint:pkg PRE-EXISTING FAIL on main publint 0.3.21 (PR #82 merged 2026-05-11) emits sideEffects suggestion on all 11 packages, including ones this PR does not touch — flagged below in Notes for follow-up
7. npm run test:coverage PASS 1722 tests / 76 files; fs-http 100% statements/branches/functions/lines
8. npx stryker run (fs-http) PASS 97.50% mutation score (78 killed / 2 survived, both on unregister helper line 29 — pre-existing equivalent mutants unrelated to this fix). Up from M2 baseline of 95.74% — the new tests killed two previously-surviving mutants on the XSRF read path.

Lockfile sanity: node -e "..." scan confirms zero nested node_modules/@script-development/* registry copies; all resolve to workspace links.

Out-of-scope coordination

  • Queue chore(deps-dev): bump the oxc group with 2 updates #21 (relative-baseURL guard): Not yet staged on a 0.3.1 bump. This PR's bump is 0.3.0 → 0.3.1. If queue chore(deps-dev): bump the oxc group with 2 updates #21's Medic order opens before this merges, the two CHANGELOG entries can be merged under the same 0.3.1 heading at PR-open time.
  • Cascade peer-range bumps: Not triggered. ^0.3.0 in fs-loading and fs-adapter-store peer ranges matches 0.3.1 under semver.
  • Consumer territories: No coordination needed (zero streamRequest(...) call sites in production code).
  • Docs page (docs/packages/http.md): Authentication & XSRF section (lines 102-123) does not currently mention streamRequest; skipped per orders.

Test plan

  • Source change preserves all existing behavior for withXSRFToken: true consumers
  • New test verifies bypass-case: withXSRFToken: false + cookie present → no X-XSRF-TOKEN header
  • New test verifies default-resolution: no options → no X-XSRF-TOKEN header even with cookie present
  • Reworked tests verify gate-open behavior unchanged: withXSRFToken: true + cookie → header present and URL-decoded
  • Stryker kills the new ?? false discriminator and if (wantsXsrf) branch
  • No nested registry copies in lockfile post-bump
  • All 8 CI gates evaluated; only pre-existing publint failure remains

…g (queue #64)

Previously, streamRequest unconditionally read document.cookie for
XSRF-TOKEN and emitted X-XSRF-TOKEN on every streaming POST, ignoring
the withXSRFToken flag that gates the axios methods at line 39. This
violated the library invariant that withXSRFToken: false (the default)
suppresses XSRF behavior across all transports.

Apply the same default-resolution semantics used at line 39
(options?.withXSRFToken ?? false) so streamRequest and the axios methods
share identical XSRF behavior under the same consumer config.

Tests rework the three existing XSRF cases to set withXSRFToken: true
(the gate-open condition they implicitly assumed) and add two new cases:
- omits XSRF header when withXSRFToken: false even with cookie present
- omits XSRF header by default (withXSRFToken unset) even with cookie present

Symmetry: createHttpService:39 already passes withXSRFToken: false as
the axios default. streamRequest now honors the same default.

Closes enforcement queue #64.
Patch bump for the streamRequest XSRF gate fix (queue #64). Additive
guard on previously-undefined behavior — no API change. Caret-range
^0.3.0 in fs-loading + fs-adapter-store peer ranges matches 0.3.1
under semver; no cascade widening required.
@cloudflare-workers-and-pages
Copy link
Copy Markdown

Deploying fs-packages with  Cloudflare Pages  Cloudflare Pages

Latest commit: df4af51
Status: ✅  Deploy successful!
Preview URL: https://c64e14ca.fs-packages.pages.dev
Branch Preview URL: https://engineer-queue-64-streamrequ.fs-packages.pages.dev

View logs

@Goosterhof
Copy link
Copy Markdown
Contributor Author

Superseded by an upcoming streamRequest removal PR targeting fs-http 0.4.0.

Why removal over the cookie-read gate this PR landed: the cross-stack review that followed PR #86 surfaced two additional option-honoring asymmetries on streamRequest beyond the XSRF one — headers (service-wide options.headers silently dropped at the streaming path) and timeout (Doctrine #8 silently bypassed because native fetch has no timeout primitive). Combined with the documented zero-consumer state of streamRequest across every adopter in our fleet, the honest structural answer is to remove the function rather than continue per-defect hardening.

What the replacement PR will ship:

  • streamRequest removed from the public API of @script-development/fs-http (breaking — minor bump 0.3.0 → 0.4.0 under pre-1.0 caret semver).
  • Test suite, type signature, and docs/packages/http.md references for streamRequest removed.
  • Peer-range cascade: fs-loading, fs-adapter-store, fs-cached-adapter-store widened to ^0.4.0 with patch-level bumps + CHANGELOG entries.
  • New rule in fs-packages CLAUDE.md § Conventions: every fs-http transport method must inherit option-honoring from the axios.create() instance; future transports built directly on fetch (or any non-axios transport) require deliberate audit against the full HttpServiceOptions matrix before merge. The matrix-audit discipline (Library-Config-Honor Surface Audit) is the standing checklist.

If streaming eventually becomes a real requirement, axios's native responseType: 'stream' via the standard methods inherits all HttpServiceOptions and per-call config for free. A purpose-built createStreamHttpService factory designed against the option-honoring matrix from the start is the other deliberate path. Re-adding an axios-bypassing transport is what we're explicitly designing out.

Closing here; the replacement PR will reference this for context.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant