From c7f82db596429800673be2847f48667d0f02ae77 Mon Sep 17 00:00:00 2001 From: "brian.j.murdock@gmail.com" Date: Mon, 13 Jul 2026 16:56:22 -0500 Subject: [PATCH 01/13] Harden request normalization and retry cleanup - validate query containers and cross-realm URLSearchParams inputs\n- reject non-serializable JSON configuration consistently\n- cancel abandoned response bodies before HTTP retries\n- add regression coverage for the hardened behaviors --- src/internal/execute-request.ts | 9 +++ src/internal/hook-options.ts | 11 ++- src/internal/normalize-request.ts | 109 +++++++++++++++++++++++++----- test/hook-options.test.ts | 11 +++ test/hooks-and-retries.test.ts | 44 ++++++++++++ test/normalize-request.test.ts | 103 ++++++++++++++++++++++++++++ 6 files changed, 268 insertions(+), 19 deletions(-) diff --git a/src/internal/execute-request.ts b/src/internal/execute-request.ts index e160cfa..b24c511 100644 --- a/src/internal/execute-request.ts +++ b/src/internal/execute-request.ts @@ -390,6 +390,7 @@ async function parseWithHandling(params: { attempt, ) ) { + cancelResponseBody(response) await waitForRetry({ attempt, context }) throw new RetrySignal(new HttpError({ @@ -448,3 +449,11 @@ async function parseWithHandling(params: { throw normalized } } + +function cancelResponseBody(response: Response): void { + if (response.body === null) { + return + } + + void response.body.cancel().catch(() => undefined) +} diff --git a/src/internal/hook-options.ts b/src/internal/hook-options.ts index 339686a..b16d05c 100644 --- a/src/internal/hook-options.ts +++ b/src/internal/hook-options.ts @@ -74,7 +74,16 @@ export interface HookLifecycleMetadata { } function isURLSearchParams(value: unknown): value is URLSearchParams { - return value instanceof URLSearchParams + if (typeof value !== 'object' || value === null) { + return false + } + + try { + URLSearchParams.prototype.toString.call(value) + return true + } catch { + return false + } } function freezeQueryParams(query: QueryParams): QueryParams { diff --git a/src/internal/normalize-request.ts b/src/internal/normalize-request.ts index dc582ac..072bd9d 100644 --- a/src/internal/normalize-request.ts +++ b/src/internal/normalize-request.ts @@ -34,12 +34,16 @@ export function createBeforeRequestContext( options: RequestOptions = {}, attempt = 1, ): ExecutionBeforeRequestContext { - const url = resolveRequestURL(input, defaults.baseURL, options.query) const normalized = normalizeRequestOptions(defaults, options) + const queryString = serializeValidatedQueryParams(normalized.query) + const url = resolveRequestURLWithQueryString( + input, + defaults.baseURL, + queryString, + ) const body = resolveRequestBody(normalized) validateRetryableBody(body, normalized.retry) const maxAttempts = normalized.retry === false ? 1 : normalized.retry.attempts - const queryString = serializeQueryParams(normalized.query) const optionsView = createHookRequestOptions(normalized, { attempt, maxAttempts, @@ -117,11 +121,16 @@ export function normalizeRequestOptions( ) const headers = mergeHeaders(defaults.headers, options.headers) - if (options.body !== undefined && options.json !== undefined) { + const hasJson = Object.hasOwn(options, 'json') + + if (options.body !== undefined && hasJson) { throw new ConfigError('`body` and `json` cannot both be provided') } - if ((method === 'GET' || method === 'HEAD') && (options.body !== undefined || options.json !== undefined)) { + if ( + (method === 'GET' || method === 'HEAD') && + (options.body !== undefined || hasJson) + ) { throw new ConfigError(`\`${method}\` requests cannot include a request body`) } @@ -139,9 +148,7 @@ export function normalizeRequestOptions( } if (options.query !== undefined) { - if (!isURLSearchParams(options.query)) { - validateQueryParams(options.query) - } + validateQueryInput(options.query) normalized.query = options.query } @@ -149,7 +156,7 @@ export function normalizeRequestOptions( normalized.body = options.body } - if (options.json !== undefined) { + if (hasJson) { normalized.json = options.json } @@ -168,11 +175,23 @@ export function resolveRequestURL( input: string | URL, baseURL?: string | URL, query?: QueryInput, +): URL { + return resolveRequestURLWithQueryString( + input, + baseURL, + serializeQueryParams(query), + ) +} + +function resolveRequestURLWithQueryString( + input: string | URL, + baseURL: string | URL | undefined, + queryString: string, ): URL { const base = baseURL === undefined ? undefined : toAbsoluteURL(baseURL, 'Invalid base URL') const url = input instanceof URL ? new URL(input) : resolveInputURL(input, base) - applyQueryParams(url, query) + applyQueryString(url, queryString) return url } @@ -181,8 +200,17 @@ export function serializeQueryParams(query?: QueryInput): string { return '' } + validateQueryInput(query) + return serializeValidatedQueryParams(query) +} + +function serializeValidatedQueryParams(query?: QueryInput): string { + if (query === undefined) { + return '' + } + if (isURLSearchParams(query)) { - return query.toString() + return URLSearchParams.prototype.toString.call(query) } const params = new URLSearchParams() @@ -205,19 +233,26 @@ export function serializeQueryParams(query?: QueryInput): string { return params.toString() } -function applyQueryParams(url: URL, query?: QueryInput): void { - const serialized = serializeQueryParams(query) - - if (serialized === '') { +function applyQueryString(url: URL, queryString: string): void { + if (queryString === '') { return } - const suffix = url.search === '' ? serialized : `&${serialized}` + const suffix = url.search === '' ? queryString : `&${queryString}` url.search += suffix } function isURLSearchParams(value: unknown): value is URLSearchParams { - return value instanceof URLSearchParams + if (typeof value !== 'object' || value === null) { + return false + } + + try { + URLSearchParams.prototype.toString.call(value) + return true + } catch { + return false + } } function mergeHeaders( @@ -305,7 +340,7 @@ function toAbsoluteURL(value: string | URL, message: string): URL { function resolveRequestBody( options: Pick, ): BodyInit | null | undefined { - if (options.json === undefined) { + if (!Object.hasOwn(options, 'json')) { return options.body } @@ -313,7 +348,21 @@ function resolveRequestBody( options.headers.set('Content-Type', 'application/json') } - return JSON.stringify(options.json) + try { + const body = JSON.stringify(options.json) + if (body === undefined) { + throw new ConfigError('`json` must serialize to a JSON value') + } + return body + } catch (cause) { + if (cause instanceof ConfigError) { + throw cause + } + if (cause instanceof TypeError) { + throw new ConfigError('`json` must serialize to a JSON value', cause) + } + throw cause + } } function serializeScalarQueryValue( @@ -332,6 +381,30 @@ function validateQueryParams(query: QueryParams): void { } } +function validateQueryInput(query: unknown): asserts query is QueryInput { + if (isURLSearchParams(query)) { + return + } + + if (!isQueryParamsRecord(query)) { + throw new ConfigError('`query` must be a record or URLSearchParams') + } + + validateQueryParams(query) +} + +function isQueryParamsRecord(value: unknown): value is QueryParams { + if (typeof value !== 'object' || value === null || Array.isArray(value)) { + return false + } + + try { + return Object.prototype.toString.call(value) === '[object Object]' + } catch { + return false + } +} + function validateQueryValue(key: string, value: QueryParams[string]): void { if (value === undefined) { return diff --git a/test/hook-options.test.ts b/test/hook-options.test.ts index 98c7ba9..dfb83af 100644 --- a/test/hook-options.test.ts +++ b/test/hook-options.test.ts @@ -109,6 +109,17 @@ test('createHookRequestOptions exposes serialized URLSearchParams query metadata assert.equal(snapshot.queryString, 'tag=a&page=1&tag=b') }) +test('createHookRequestOptions recognizes cross-realm URLSearchParams metadata', () => { + const query = new URLSearchParams('tag=a&page=1&tag=b') + Object.setPrototypeOf(query, null) + const snapshot = createHookRequestOptions( + createOptions({ query: query as URLSearchParams }), + DEFAULT_METADATA, + ) + + assert.equal(Object.hasOwn(snapshot, 'query'), false) +}) + test('createHookRequestOptions freezes query metadata and query arrays when query is present', () => { const query = { page: 2, diff --git a/test/hooks-and-retries.test.ts b/test/hooks-and-retries.test.ts index 4708952..684efeb 100644 --- a/test/hooks-and-retries.test.ts +++ b/test/hooks-and-retries.test.ts @@ -759,6 +759,50 @@ test('retryable HTTP responses do not read body text before retrying', async () ) }) +test('retryable HTTP responses cancel abandoned response bodies', async () => { + let attempts = 0 + let cancelCalls = 0 + + const fetchImpl: typeof fetch = async () => { + attempts += 1 + + if (attempts === 1) { + return new Response(new ReadableStream({ + cancel() { + cancelCalls += 1 + }, + start(controller) { + controller.enqueue(new TextEncoder().encode('retry')) + }, + }), { + status: 503, + statusText: 'Service Unavailable', + }) + } + + return new Response(JSON.stringify({ ok: true })) + } + + await withMockedFetch(fetchImpl, async () => { + const result = await request<{ ok: boolean }>( + 'https://api.example.com/users', + { + retry: { + attempts: 2, + backoffMs: 1, + maxBackoffMs: 1, + multiplier: 1, + retryOnStatuses: [503], + retryOnMethods: ['GET'], + }, + }, + ) + + assert.deepEqual(result, { ok: true }) + assert.equal(cancelCalls, 1) + }) +}) + test('abort during HTTP retry backoff stops promptly with AbortRequestError', async () => { const originalFetch = globalThis.fetch const controller = new AbortController() diff --git a/test/normalize-request.test.ts b/test/normalize-request.test.ts index 2338bdc..0a300db 100644 --- a/test/normalize-request.test.ts +++ b/test/normalize-request.test.ts @@ -1,5 +1,6 @@ import assert from 'node:assert/strict' import test from 'node:test' +import { runInNewContext } from 'node:vm' import { ConfigError } from '../src/errors.js' import { @@ -31,6 +32,16 @@ test('serializeQueryParams preserves URLSearchParams ordering and duplicate keys assert.equal(serializeQueryParams(query), 'tag=a&page=1&tag=b') }) +test('serializeQueryParams accepts URLSearchParams values across realm boundaries', () => { + const query = new URLSearchParams('tag=a&page=1&tag=b') + Object.setPrototypeOf(query, null) + + assert.equal( + serializeQueryParams(query as URLSearchParams), + 'tag=a&page=1&tag=b', + ) +}) + test('resolveRequestURL appends URLSearchParams query input', () => { const query = new URLSearchParams('tag=a&page=1&tag=b') const url = resolveRequestURL( @@ -290,6 +301,41 @@ test('normalizeRequestOptions rejects unsupported query values', () => { ) }) +test('normalizeRequestOptions rejects invalid query containers with ConfigError', () => { + for (const query of [null, 42, []]) { + assert.throws( + () => + normalizeRequestOptions({}, { + query: query as never, + }), + (error) => + error instanceof ConfigError && + error.message === '`query` must be a record or URLSearchParams', + ) + } +}) + +test('normalizeRequestOptions accepts query records across realm boundaries', () => { + const query = runInNewContext('({ page: 2, tags: ["design", "types"] })') + const options = normalizeRequestOptions({}, { + query, + }) + + assert.equal(serializeQueryParams(options.query), 'page=2&tags=design&tags=types') +}) + +test('createBeforeRequestContext validates query input before URL serialization', () => { + assert.throws( + () => + createBeforeRequestContext('https://api.example.com/users', {}, { + query: null as never, + }), + (error) => + error instanceof ConfigError && + error.message === '`query` must be a record or URLSearchParams', + ) +}) + test('createBeforeRequestContext rejects streaming bodies when retry is enabled', () => { assert.throws( () => @@ -324,6 +370,63 @@ test('buildRequestFromContext serializes json and sets content-type when absent' assert.equal(context.body, JSON.stringify({ name: 'Brian' })) }) +test('createBeforeRequestContext rejects json values that serialize to undefined', () => { + for (const json of [undefined, Symbol('value'), () => undefined]) { + assert.throws( + () => + createBeforeRequestContext('https://api.example.com/users', {}, { + method: 'POST', + json, + }), + (error) => + error instanceof ConfigError && + error.message === '`json` must serialize to a JSON value', + ) + } +}) + +test('explicit undefined json still participates in option validation', () => { + assert.throws( + () => + normalizeRequestOptions({}, { + method: 'POST', + body: 'raw', + json: undefined, + } as unknown as RequestOptions), + (error) => + error instanceof ConfigError && + error.message === '`body` and `json` cannot both be provided', + ) + + assert.throws( + () => + normalizeRequestOptions({}, { + method: 'GET', + json: undefined, + } as unknown as RequestOptions), + (error) => + error instanceof ConfigError && + error.message === '`GET` requests cannot include a request body', + ) +}) + +test('createBeforeRequestContext wraps json serialization failures as ConfigError', () => { + const json: { self?: unknown } = {} + json.self = json + + assert.throws( + () => + createBeforeRequestContext('https://api.example.com/users', {}, { + method: 'POST', + json, + }), + (error) => + error instanceof ConfigError && + error.message === '`json` must serialize to a JSON value' && + error.cause instanceof TypeError, + ) +}) + test('buildRequestFromContext rejects invalid hook URL overrides', () => { const context = createBeforeRequestContext('https://api.example.com/users') From bbb78900062babf8ba28d39bf223bcc3a60c1e29 Mon Sep 17 00:00:00 2001 From: "brian.j.murdock@gmail.com" Date: Mon, 13 Jul 2026 16:56:32 -0500 Subject: [PATCH 02/13] Harden release publication safeguards - verify annotated release tags are reachable from main\n- add strict and rerunnable publish dry-run modes\n- extend CI coverage through Node.js 24\n- document the release-path check in contributor guidance --- .github/pull_request_template.md | 1 + .github/workflows/ci.yml | 6 +-- .github/workflows/release.yml | 27 +++++++++-- CONTRIBUTING.md | 1 + package.json | 1 + scripts/check-publish-dry-run.mjs | 74 +++++++++++++++++++++++++++++++ 6 files changed, 104 insertions(+), 6 deletions(-) create mode 100644 scripts/check-publish-dry-run.mjs diff --git a/.github/pull_request_template.md b/.github/pull_request_template.md index 244fc02..cb3142a 100644 --- a/.github/pull_request_template.md +++ b/.github/pull_request_template.md @@ -9,6 +9,7 @@ Describe the change and why it exists. - [ ] `npm run build` - [ ] `npm run check:package-metadata` - [ ] `npm run check:pack-smoke` +- [ ] `npm run check:publish-dry-run` when release behavior changes - [ ] dependency audit performed when dependencies or lockfiles change ## Notes diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index e78034f..11ccd3e 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -32,7 +32,7 @@ jobs: strategy: fail-fast: false matrix: - node-version: [18, 20, 22] + node-version: [18, 20, 22, 24] steps: - name: Check out repository @@ -66,7 +66,7 @@ jobs: - name: Set up Node.js uses: actions/setup-node@53b83947a5a98c8d113130e565377fae1a50d02f # v6.3.0 with: - node-version: 20 + node-version: 24 cache: npm - name: Install dependencies @@ -85,7 +85,7 @@ jobs: - name: Set up Node.js uses: actions/setup-node@53b83947a5a98c8d113130e565377fae1a50d02f # v6.3.0 with: - node-version: 20 + node-version: 24 cache: npm - name: Install dependencies diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 58652a6..bda4d6f 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -38,7 +38,14 @@ jobs: npm run build npm run check:package-metadata npm run check:pack-smoke - npm publish --dry-run + + - name: Check publishability for a release tag + if: github.event_name == 'push' + run: npm run check:publish-dry-run + + - name: Check publishability for manual validation + if: github.event_name == 'workflow_dispatch' + run: npm run check:publish-dry-run -- --allow-existing publish: if: github.event_name == 'push' @@ -52,6 +59,8 @@ jobs: steps: - name: Check out repository uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 + with: + fetch-depth: 0 - name: Set up Node.js uses: actions/setup-node@53b83947a5a98c8d113130e565377fae1a50d02f # v6.3.0 @@ -70,24 +79,36 @@ jobs: npm run build npm run check:package-metadata npm run check:pack-smoke - npm publish --dry-run + npm run check:publish-dry-run - name: Publish to npm with provenance env: TAG_NAME: ${{ github.ref_name }} run: | PACKAGE_VERSION="$(node -p "require('./package.json').version")" + CURRENT_GIT_HEAD="$(git rev-parse HEAD)" if [ "$TAG_NAME" != "v$PACKAGE_VERSION" ]; then echo "Release tag $TAG_NAME does not match package version v$PACKAGE_VERSION" >&2 exit 1 fi + TAG_OBJECT_TYPE="$(git cat-file -t "refs/tags/$TAG_NAME" 2>/dev/null || true)" + if [ "$TAG_OBJECT_TYPE" != "tag" ]; then + echo "Release tag $TAG_NAME must be annotated" >&2 + exit 1 + fi + + git fetch origin main:refs/remotes/origin/main --no-tags + if ! git merge-base --is-ancestor "$CURRENT_GIT_HEAD" origin/main; then + echo "Release commit $CURRENT_GIT_HEAD is not reachable from origin/main" >&2 + exit 1 + fi + PUBLISHED_VERSION="$(npm view "@gavoryn/clearfetch@$PACKAGE_VERSION" version --registry=https://registry.npmjs.org 2>/dev/null || true)" if [ "$PUBLISHED_VERSION" = "$PACKAGE_VERSION" ]; then PUBLISHED_GIT_HEAD="$(npm view "@gavoryn/clearfetch@$PACKAGE_VERSION" gitHead --registry=https://registry.npmjs.org 2>/dev/null || true)" - CURRENT_GIT_HEAD="$(git rev-parse HEAD)" if [ "$PUBLISHED_GIT_HEAD" != "$CURRENT_GIT_HEAD" ]; then echo "Published gitHead $PUBLISHED_GIT_HEAD does not match current tag commit $CURRENT_GIT_HEAD" >&2 diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 9711f40..ddfa4de 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -18,6 +18,7 @@ Thanks for the interest in improving `clearfetch`. - `npm run build` - `npm run check:package-metadata` - `npm run check:pack-smoke` +- `npm run check:publish-dry-run` for release-path changes For dependency changes, also run the relevant audit command, usually: diff --git a/package.json b/package.json index 047b737..488bd0c 100644 --- a/package.json +++ b/package.json @@ -22,6 +22,7 @@ "build": "node -e \"require('node:fs').rmSync('dist', { recursive: true, force: true })\" && tsc -p tsconfig.build.json", "check:package-metadata": "node scripts/check-package-metadata.mjs", "check:pack-smoke": "node scripts/check-pack-smoke.mjs", + "check:publish-dry-run": "node scripts/check-publish-dry-run.mjs", "lint": "tsc --noEmit -p tsconfig.json", "test": "tsx --test test/*.test.ts", "test:browser-like": "tsx --test test/browser-like.test.ts" diff --git a/scripts/check-publish-dry-run.mjs b/scripts/check-publish-dry-run.mjs new file mode 100644 index 0000000..b9a7d2a --- /dev/null +++ b/scripts/check-publish-dry-run.mjs @@ -0,0 +1,74 @@ +import { execFile } from 'node:child_process' +import { readFile } from 'node:fs/promises' +import process from 'node:process' +import { promisify } from 'node:util' + +const execFileAsync = promisify(execFile) +const registry = 'https://registry.npmjs.org' +const allowExisting = process.argv.includes('--allow-existing') +const packageConfig = JSON.parse(await readFile('package.json', 'utf8')) +const packageSpec = `${packageConfig.name}@${packageConfig.version}` +const published = await getPublishedMetadata(packageSpec) + +if (published === undefined) { + const { stderr, stdout } = await execFileAsync( + 'npm', + ['publish', '--dry-run', `--registry=${registry}`], + { maxBuffer: 10 * 1024 * 1024 }, + ) + process.stdout.write(stdout) + process.stderr.write(stderr) + console.log(`publish dry-run passed for unpublished version ${packageSpec}`) +} else { + if (allowExisting) { + console.log( + `${packageSpec} is already published; publish dry-run skipped for manual validation`, + ) + process.exit(0) + } + + const { stdout } = await execFileAsync('git', ['rev-parse', 'HEAD']) + const currentGitHead = stdout.trim() + + if (published.gitHead !== currentGitHead) { + throw new Error( + `published gitHead ${String(published.gitHead)} does not match current commit ${currentGitHead}`, + ) + } + + console.log( + `${packageSpec} is already published from the current commit; publish dry-run skipped`, + ) +} + +async function getPublishedMetadata(packageSpec) { + try { + const { stdout } = await execFileAsync( + 'npm', + [ + 'view', + packageSpec, + 'version', + 'gitHead', + '--json', + `--registry=${registry}`, + ], + { maxBuffer: 1024 * 1024 }, + ) + return JSON.parse(stdout) + } catch (error) { + if (isNotFoundError(error)) { + return undefined + } + throw error + } +} + +function isNotFoundError(error) { + if (typeof error !== 'object' || error === null) { + return false + } + + const stderr = 'stderr' in error ? String(error.stderr) : '' + return stderr.includes('E404') || stderr.includes('is not in this registry') +} From 073c280c12c0d11c1989d741a74827c74cc052ed Mon Sep 17 00:00:00 2001 From: "brian.j.murdock@gmail.com" Date: Mon, 13 Jul 2026 16:56:41 -0500 Subject: [PATCH 03/13] Align documentation with hardened behavior - record request validation and retry cleanup semantics\n- clarify package compatibility versus security support\n- document strict and rerunnable release validation\n- correct empty-response guidance for HTTP 304 --- CHANGELOG.md | 9 +++++++++ DESIGN.md | 3 ++- README.md | 10 ++++++++-- RELEASE.md | 12 ++++++++++-- SECURITY.md | 11 ++++++++--- 5 files changed, 37 insertions(+), 8 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index d7803f9..279b8ad 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,15 @@ Entries describe repository versions. A version is publicly released only after the matching `vX.Y.Z` tag publishes to npm and a GitHub Release exists. +## Unreleased + +- cancel abandoned response bodies before retrying eligible HTTP failures +- reject invalid query containers and JSON values for which `JSON.stringify` returns `undefined` or throws `TypeError` with `ConfigError` +- recognize `URLSearchParams` values across browser realm boundaries +- require release tags to be annotated and reachable from `main` before publishing +- keep release verification rerunnable when the package version already exists for the same commit +- distinguish Node.js package compatibility from upstream-backed security support + ## 1.0.6 - validate hook configuration consistently for client defaults and request options diff --git a/DESIGN.md b/DESIGN.md index a19ba92..8aa31c7 100644 --- a/DESIGN.md +++ b/DESIGN.md @@ -481,6 +481,7 @@ Rules: - `json` and `body` are mutually exclusive - when `json` is provided, the package serializes it with `JSON.stringify` - if `Content-Type` is not already set, it is set to `application/json` +- values for which `JSON.stringify` returns `undefined` or throws `TypeError` fail with `ConfigError` before network execution; other caller-owned exceptions encountered during serialization, including from `toJSON` or property access, propagate as-is The package should not perform schema validation or content introspection beyond what is necessary for consistent behavior. @@ -529,7 +530,7 @@ When `responseType` is `json`, an empty response body yields `undefined`. For this purpose, a response body is considered empty if reading it yields an empty string. -This applies to `204`, `205`, and `304` responses and to other successful responses whose body is empty. +This applies to `204`, `205`, and other successful responses whose body is empty. A `304` remains a non-2xx response and therefore throws `HttpError`. As a result, JSON responses are typed as `T | undefined` rather than `T` alone. diff --git a/README.md b/README.md index 76fe35a..fa50f25 100644 --- a/README.md +++ b/README.md @@ -102,6 +102,7 @@ If `json` is provided, clearfetch: - serializes the value with `JSON.stringify()` - sets `Content-Type: application/json` if it is not already present - rejects the request with `ConfigError` if `body` is also provided +- rejects values when `JSON.stringify()` returns `undefined` or throws `TypeError` with `ConfigError`; other caller-owned exceptions encountered during serialization, including from `toJSON()` or property access, propagate as-is Use `body` directly only when you want to send a raw payload such as `FormData`, `URLSearchParams`, or pre-serialized text. @@ -370,10 +371,13 @@ If you need end-to-end runtime safety, validate parsed data with a schema librar clearfetch currently supports: -- Node.js `18.x` and newer +- Node.js `18.x` and newer for package compatibility - modern browsers with native `fetch`, `Request`, `Response`, `Headers`, `URL`, and `AbortController` The package is ESM-only and does not target legacy runtimes or polyfill-driven environments. +For security-sensitive use, run clearfetch on a Node.js release line that is +still [supported upstream](https://nodejs.org/en/about/previous-releases); EOL +Node.js releases do not receive upstream security fixes. ## Security @@ -385,7 +389,7 @@ The package is ESM-only and does not target legacy runtimes or polyfill-driven e - CI lints GitHub Actions workflows before merge. - CI runs lint, test, and build checks on selected supported Node.js versions. -- CI also runs a lightweight browser-like test path using `happy-dom` on Node.js `20`. +- CI also runs a lightweight browser-like test path using `happy-dom` on Node.js `24`. - Dependency review is enforced for pull requests and supports manual base/head validation. - The release workflow supports a non-publishing dry-run path via manual dispatch. - npm publishing now uses npm trusted publishing from GitHub Actions instead of a long-lived publish token. @@ -399,6 +403,7 @@ The public package surface is intentionally narrow: - the root export provides the supported runtime API and public types - internal implementation modules are not part of the supported import contract +- the deprecated `NormalizedRequestOptions` type remains exported only for compatibility and is planned for removal in the next major version - the package includes no lifecycle scripts and is intended to publish only built `dist/` artifacts ## Development @@ -407,6 +412,7 @@ The public package surface is intentionally narrow: - `npm run build`: compile the package into `dist/` - `npm run check:package-metadata`: validate publish metadata and zero-runtime-dependency posture - `npm run check:pack-smoke`: smoke-test the packed tarball from a clean temporary install +- `npm run check:publish-dry-run`: dry-run unpublished versions or verify an already-published version came from the current commit; add `-- --allow-existing` only for non-publishing manual workflow validation - `npm run lint`: run TypeScript static checks - `npm test`: run the test suite - `npm run test:browser-like`: run browser-like package entrypoint coverage with `happy-dom` diff --git a/RELEASE.md b/RELEASE.md index b52ab5d..a545aaa 100644 --- a/RELEASE.md +++ b/RELEASE.md @@ -18,6 +18,9 @@ Local `npm publish` should not be used for normal releases. The tag must match the package version exactly, for example package version `1.2.3` must be released from tag `v1.2.3`. If a workflow rerun finds that exact package version already published on npm and the published `gitHead` matches the checked-out tag commit, it skips publishing and still creates or verifies the GitHub Release record. +Before publishing, the workflow also verifies that the release tag is annotated +and that its commit is reachable from `origin/main`. + Post-release verification: ```bash @@ -35,7 +38,11 @@ That dry-run path should verify: - install, lint, test, and build steps - package metadata with `npm run check:package-metadata` - packed artifact behavior with `npm run check:pack-smoke` -- publishability with `npm publish --dry-run --registry=https://registry.npmjs.org` +- publishability with `npm run check:publish-dry-run -- --allow-existing`, which uses the public npm registry explicitly, dry-runs unpublished versions, and permits manual validation to skip an existing version + +Tag-triggered verification uses strict `npm run check:publish-dry-run` instead. +For an already-published tag rerun, that mode requires npm `gitHead` to match +the checked-out tag commit before the publish job can continue. Use the dry-run path before relying on a first release or after making workflow changes that affect packaging or publishing. @@ -88,4 +95,5 @@ The release process must preserve the package’s public claims: - no lifecycle scripts - no built-in telemetry - no hidden network behavior beyond the caller's request -- support limited to Node.js `18+` and modern browsers +- package compatibility starting at Node.js `18+`, with security support limited to upstream-supported Node.js release lines +- modern browsers with the native web platform APIs documented in `README.md` diff --git a/SECURITY.md b/SECURITY.md index edae6f5..8ed0aa3 100644 --- a/SECURITY.md +++ b/SECURITY.md @@ -2,12 +2,17 @@ ## Supported runtimes -Security support is currently scoped to the actively supported runtime targets for this project: +Package compatibility starts at Node.js `18.x`, as declared in `package.json`. +Security support is narrower and applies only when clearfetch is used on a +Node.js release line that is still supported by the Node.js project: -- Node.js `18.x` and newer +- upstream-supported Node.js release lines - modern browsers with native `fetch`, `Request`, `Response`, `Headers`, `URL`, and `AbortController` -Legacy runtimes, polyfill-driven environments, and unsupported platform shims are out of scope. +See the [official Node.js release status](https://nodejs.org/en/about/previous-releases) +for the current lifecycle. Vulnerabilities caused by EOL Node.js releases, +legacy runtimes, polyfill-driven environments, and unsupported platform shims +are out of scope. ## Reporting a vulnerability From 87bc7dfac16248664dd6bd907f685b2ac1d74fb0 Mon Sep 17 00:00:00 2001 From: "brian.j.murdock@gmail.com" Date: Wed, 15 Jul 2026 20:56:55 -0500 Subject: [PATCH 04/13] Stabilize retry replay and client response typing - snapshot normalized request inputs and replayable bodies across attempts - handle cross-realm platform values and effective retry eligibility - reflect client response defaults in public types and regression coverage --- src/client.ts | 20 +- src/internal/execute-request.ts | 78 ++++++-- src/internal/normalize-request.ts | 303 ++++++++++++++++-------------- src/internal/platform-values.ts | 163 ++++++++++++++++ src/internal/query-params.ts | 136 ++++++++++++++ src/internal/retry-policy.ts | 19 +- src/types.ts | 101 ++++++++-- test/hooks-and-retries.test.ts | 212 ++++++++++++++++----- test/normalize-request.test.ts | 197 +++++++++++++++++++ test/retry-policy.test.ts | 14 +- test/type-signatures.ts | 76 ++++++++ 11 files changed, 1100 insertions(+), 219 deletions(-) create mode 100644 src/internal/platform-values.ts create mode 100644 src/internal/query-params.ts diff --git a/src/client.ts b/src/client.ts index 847bb2c..2af17a5 100644 --- a/src/client.ts +++ b/src/client.ts @@ -1,12 +1,28 @@ import { createClient as createClientInternal, } from './internal/execute-request.js' -import type { ClientDefaults, HttpClient } from './types.js' +import type { ClientDefaults, HttpClient, ResponseType } from './types.js' /** * Creates a reusable HTTP client with shared defaults such as `baseURL`, * headers, timeout, retry behavior, hooks, and JSON parsing behavior. */ -export function createClient(defaults: ClientDefaults = {}): HttpClient { +export function createClient( + defaults: Omit & { + responseType: DefaultResponseType + }, +): HttpClient + +export function createClient( + defaults?: Omit & { + responseType?: never + }, +): HttpClient<'json'> + +export function createClient(defaults: ClientDefaults): HttpClient + +export function createClient( + defaults: ClientDefaults = {}, +): HttpClient { return createClientInternal(defaults) } diff --git a/src/internal/execute-request.ts b/src/internal/execute-request.ts index b24c511..6106097 100644 --- a/src/internal/execute-request.ts +++ b/src/internal/execute-request.ts @@ -12,6 +12,7 @@ import type { OnErrorHook, RequestMethod, RequestOptions, + ResponseType, } from '../types.js' import { mergeClientDefaults, @@ -21,11 +22,14 @@ import { normalizeExecutionError } from './normalize-error.js' import { buildRequestFromContext, createBeforeRequestContext, + createBeforeRequestContextFromSnapshot, + snapshotBeforeRequestContext, type ExecutionBeforeRequestContext, } from './normalize-request.js' import { normalizeOnErrorHooks } from './hooks.js' import { parseResponse } from './parse-response.js' import { + getEffectiveRetryAttempts, getRetryDelay, shouldRetryError, shouldRetryStatus, @@ -65,10 +69,12 @@ export async function executeRequest( throw error } - const maxAttempts = - initialContext._internalOptions.retry === false - ? 1 - : initialContext._internalOptions.retry.attempts + const maxAttempts = getEffectiveRetryAttempts( + initialContext._internalOptions.method, + initialContext._internalOptions.retry, + ) + const contextSnapshot = + maxAttempts === 1 ? undefined : snapshotBeforeRequestContext(initialContext) let lastError: HttpClientError | undefined @@ -78,7 +84,10 @@ export async function executeRequest( context = initialContext } else { try { - context = createBeforeRequestContext(input, defaults, options, attempt) + if (contextSnapshot === undefined) { + throw new ConfigError('Retry context snapshot is unavailable') + } + context = createBeforeRequestContextFromSnapshot(contextSnapshot, attempt) } catch (error) { await runOnErrorHooks({ input, @@ -176,7 +185,9 @@ export async function executeRequest( throw lastError ?? new ConfigError('Request execution ended without a result') } -export function createClient(defaults: ClientDefaults = {}): HttpClient { +export function createClient( + defaults: ClientDefaults = {}, +): HttpClient { // Snapshot defaults once so client behavior does not drift if caller-owned // objects are mutated after client creation. const frozenDefaults = snapshotClientDefaults(defaults) @@ -193,13 +204,13 @@ export function createClient(defaults: ClientDefaults = {}): HttpClient { options: createMethodCaller(frozenDefaults, 'OPTIONS'), extend: (childDefaults: ClientDefaults) => createClient(mergeClientDefaults(frozenDefaults, childDefaults)), - } + } as HttpClient } function createMethodCaller( defaults: ClientDefaults, method: RequestMethod, -): HttpClient['get'] { +): HttpClient['get'] { return ( input: string | URL, options: RequestOptions = {}, @@ -281,6 +292,34 @@ async function waitForRetry(params: { } } +async function waitForRetryWithHandling(params: { + attempt: number + context: ExecutionBeforeRequestContext + input: string | URL + request: Request + response?: Response +}): Promise { + const { attempt, context, input, request, response } = params + + try { + await waitForRetry({ attempt, context }) + } catch (error) { + const errorContext: ErrorContext = { + input, + error, + options: context.options, + request, + } + + if (response !== undefined) { + errorContext.response = response + } + + await runOnErrorHooks(errorContext, context._internalOptions.hooks.onError) + throw error + } +} + function isSignalAbortReason(signal: AbortSignal, error: unknown): boolean { return signal.aborted && Object.is(error, signal.reason) } @@ -348,7 +387,12 @@ async function fetchWithHandling(params: { attempt, ) ) { - await waitForRetry({ attempt, context }) + await waitForRetryWithHandling({ + attempt, + context, + input, + request, + }) throw new RetrySignal(normalized) } @@ -391,7 +435,13 @@ async function parseWithHandling(params: { ) ) { cancelResponseBody(response) - await waitForRetry({ attempt, context }) + await waitForRetryWithHandling({ + attempt, + context, + input, + request, + response, + }) throw new RetrySignal(new HttpError({ status: response.status, @@ -432,7 +482,13 @@ async function parseWithHandling(params: { attempt, ) ) { - await waitForRetry({ attempt, context }) + await waitForRetryWithHandling({ + attempt, + context, + input, + request, + response, + }) throw new RetrySignal(normalized) } diff --git a/src/internal/normalize-request.ts b/src/internal/normalize-request.ts index 072bd9d..120bad7 100644 --- a/src/internal/normalize-request.ts +++ b/src/internal/normalize-request.ts @@ -3,16 +3,29 @@ import type { BeforeRequestContext, ClientDefaults, NormalizedRequestOptions, - PrimitiveQueryValue, QueryInput, - QueryParams, RequestMethod, RequestOptions, ResponseType, } from '../types.js' import { mergeHooks } from './hooks.js' import { createHookRequestOptions } from './hook-options.js' -import { REQUEST_METHODS, normalizeRetry } from './retry-policy.js' +import { + isReadableStream, + snapshotRequestBody, +} from './platform-values.js' +import { + applyQueryString, + serializeQueryParams, + serializeValidatedQueryParams, + snapshotQueryInput, + validateQueryInput, +} from './query-params.js' +import { + getEffectiveRetryAttempts, + REQUEST_METHODS, + normalizeRetry, +} from './retry-policy.js' const RESPONSE_TYPES = new Set([ 'json', @@ -28,6 +41,13 @@ export interface ExecutionBeforeRequestContext extends BeforeRequestContext { _internalOptions: NormalizedRequestOptions } +export interface BeforeRequestContextSnapshot { + input: string | URL + url: URL + options: NormalizedRequestOptions + queryString?: string +} + export function createBeforeRequestContext( input: string | URL, defaults: ClientDefaults = {}, @@ -41,9 +61,90 @@ export function createBeforeRequestContext( defaults.baseURL, queryString, ) - const body = resolveRequestBody(normalized) - validateRetryableBody(body, normalized.retry) - const maxAttempts = normalized.retry === false ? 1 : normalized.retry.attempts + const resolvedBody = resolveRequestBody(normalized) + const maxAttempts = getEffectiveRetryAttempts( + normalized.method, + normalized.retry, + ) + validateRetryableBody(resolvedBody, maxAttempts) + const body = + maxAttempts === 1 || resolvedBody === undefined + ? resolvedBody + : snapshotRequestBody(resolvedBody) + + return createExecutionBeforeRequestContext({ + attempt, + body, + input, + normalized, + queryString, + url, + }) +} + +export function snapshotBeforeRequestContext( + context: ExecutionBeforeRequestContext, +): BeforeRequestContextSnapshot { + const options = cloneNormalizedRequestOptions(context._internalOptions) + + delete options.json + if (context.body !== undefined) { + options.body = + options.hooks.beforeRequest.length === 0 + ? context.body + : snapshotRequestBody(context.body) + } else { + delete options.body + } + + const snapshot: BeforeRequestContextSnapshot = { + input: cloneRequestInput(context.input), + url: new URL(context.url), + options, + } + + if (context.options.queryString !== undefined) { + snapshot.queryString = context.options.queryString + } + + return snapshot +} + +export function createBeforeRequestContextFromSnapshot( + snapshot: BeforeRequestContextSnapshot, + attempt: number, +): ExecutionBeforeRequestContext { + const normalized = cloneNormalizedRequestOptions(snapshot.options) + if ( + normalized.body !== undefined && + normalized.hooks.beforeRequest.length > 0 + ) { + normalized.body = snapshotRequestBody(normalized.body) + } + + return createExecutionBeforeRequestContext({ + attempt, + body: normalized.body, + input: cloneRequestInput(snapshot.input), + normalized, + queryString: snapshot.queryString ?? '', + url: new URL(snapshot.url), + }) +} + +function createExecutionBeforeRequestContext(params: { + attempt: number + body: BodyInit | null | undefined + input: string | URL + normalized: NormalizedRequestOptions + queryString: string + url: URL +}): ExecutionBeforeRequestContext { + const { attempt, body, input, normalized, queryString, url } = params + const maxAttempts = getEffectiveRetryAttempts( + normalized.method, + normalized.retry, + ) const optionsView = createHookRequestOptions(normalized, { attempt, maxAttempts, @@ -79,6 +180,56 @@ export function createBeforeRequestContext( return context } +function cloneNormalizedRequestOptions( + options: NormalizedRequestOptions, +): NormalizedRequestOptions { + const snapshot: NormalizedRequestOptions = { + method: options.method, + headers: new Headers(options.headers), + responseType: options.responseType, + retry: + options.retry === false + ? false + : { + ...options.retry, + retryOnStatuses: [...options.retry.retryOnStatuses], + retryOnMethods: [...options.retry.retryOnMethods], + }, + hooks: { + beforeRequest: [...options.hooks.beforeRequest], + afterResponse: [...options.hooks.afterResponse], + onError: [...options.hooks.onError], + }, + parseJson: options.parseJson, + } + + if (options.query !== undefined) { + snapshot.query = snapshotQueryInput(options.query) + } + + if (options.body !== undefined) { + snapshot.body = options.body + } + + if (Object.hasOwn(options, 'json')) { + snapshot.json = options.json + } + + if (options.timeout !== undefined) { + snapshot.timeout = options.timeout + } + + if (options.signal !== undefined) { + snapshot.signal = options.signal + } + + return snapshot +} + +function cloneRequestInput(input: string | URL): string | URL { + return typeof input === 'string' ? input : new URL(String(input)) +} + export function buildRequestFromContext( context: ExecutionBeforeRequestContext, signal?: AbortSignal, @@ -94,6 +245,9 @@ export function buildRequestFromContext( if (context.body !== undefined) { init.body = context.body + if (isReadableStream(context.body)) { + Object.assign(init, { duplex: 'half' as const }) + } } if (signal !== undefined) { @@ -195,65 +349,7 @@ function resolveRequestURLWithQueryString( return url } -export function serializeQueryParams(query?: QueryInput): string { - if (query === undefined) { - return '' - } - - validateQueryInput(query) - return serializeValidatedQueryParams(query) -} - -function serializeValidatedQueryParams(query?: QueryInput): string { - if (query === undefined) { - return '' - } - - if (isURLSearchParams(query)) { - return URLSearchParams.prototype.toString.call(query) - } - - const params = new URLSearchParams() - - for (const [key, value] of Object.entries(query)) { - if (value === undefined) { - continue - } - - if (Array.isArray(value)) { - for (const item of value) { - params.append(key, serializeScalarQueryValue(item)) - } - continue - } - - params.append(key, serializeScalarQueryValue(value)) - } - - return params.toString() -} - -function applyQueryString(url: URL, queryString: string): void { - if (queryString === '') { - return - } - - const suffix = url.search === '' ? queryString : `&${queryString}` - url.search += suffix -} - -function isURLSearchParams(value: unknown): value is URLSearchParams { - if (typeof value !== 'object' || value === null) { - return false - } - - try { - URLSearchParams.prototype.toString.call(value) - return true - } catch { - return false - } -} +export { serializeQueryParams } from './query-params.js' function mergeHeaders( defaultHeaders?: HeadersInit, @@ -365,87 +461,16 @@ function resolveRequestBody( } } -function serializeScalarQueryValue( - value: PrimitiveQueryValue, -): string { - if (value === null) { - return 'null' - } - - return String(value) -} - -function validateQueryParams(query: QueryParams): void { - for (const [key, value] of Object.entries(query)) { - validateQueryValue(key, value) - } -} - -function validateQueryInput(query: unknown): asserts query is QueryInput { - if (isURLSearchParams(query)) { - return - } - - if (!isQueryParamsRecord(query)) { - throw new ConfigError('`query` must be a record or URLSearchParams') - } - - validateQueryParams(query) -} - -function isQueryParamsRecord(value: unknown): value is QueryParams { - if (typeof value !== 'object' || value === null || Array.isArray(value)) { - return false - } - - try { - return Object.prototype.toString.call(value) === '[object Object]' - } catch { - return false - } -} - -function validateQueryValue(key: string, value: QueryParams[string]): void { - if (value === undefined) { - return - } - - if (Array.isArray(value)) { - for (const item of value) { - validateQueryScalarValue(key, item) - } - return - } - - validateQueryScalarValue(key, value) -} - -function validateQueryScalarValue(key: string, value: unknown): void { - if ( - value === null || - typeof value === 'string' || - typeof value === 'number' || - typeof value === 'boolean' - ) { - return - } - - throw new ConfigError( - `Unsupported query value for \`${key}\`; only string, number, boolean, null, arrays, and undefined are allowed`, - ) -} - function validateRetryableBody( body: BodyInit | null | undefined, - retry: NormalizedRequestOptions['retry'], + maxAttempts: number, ): void { - if (retry === false || body === undefined || body === null) { + if (maxAttempts === 1 || body === undefined || body === null) { return } if ( - typeof ReadableStream !== 'undefined' && - body instanceof ReadableStream + isReadableStream(body) ) { throw new ConfigError( 'Retry is not supported for streaming request bodies', diff --git a/src/internal/platform-values.ts b/src/internal/platform-values.ts new file mode 100644 index 0000000..92ebe3d --- /dev/null +++ b/src/internal/platform-values.ts @@ -0,0 +1,163 @@ +import { ConfigError } from '../errors.js' + +export function snapshotRequestBody(body: BodyInit | null): BodyInit | null { + if (body === null || typeof body === 'string') { + return body + } + + if (isURLSearchParams(body)) { + return new URLSearchParams(URLSearchParams.prototype.toString.call(body)) + } + + if (ArrayBuffer.isView(body)) { + return new Uint8Array(body.buffer, body.byteOffset, body.byteLength).slice() + } + + const arrayBufferSnapshot = snapshotArrayBuffer(body) + if (arrayBufferSnapshot !== undefined) { + return arrayBufferSnapshot + } + + const formDataSnapshot = snapshotFormData(body) + if (formDataSnapshot !== undefined) { + return formDataSnapshot + } + + return body +} + +export function isURLSearchParams( + value: unknown, +): value is URLSearchParams { + if (typeof value !== 'object' || value === null) { + return false + } + + try { + URLSearchParams.prototype.toString.call(value) + return true + } catch { + return false + } +} + +export function isReadableStream(value: unknown): value is ReadableStream { + if ( + typeof ReadableStream === 'undefined' || + typeof value !== 'object' || + value === null || + Object.prototype.toString.call(value) !== '[object ReadableStream]' + ) { + return false + } + + const lockedGetter = Object.getOwnPropertyDescriptor( + ReadableStream.prototype, + 'locked', + )?.get + if (lockedGetter === undefined) { + return value instanceof ReadableStream + } + + try { + lockedGetter.call(value) + return true + } catch { + return false + } +} + +function snapshotArrayBuffer(body: BodyInit): ArrayBuffer | undefined { + if (Object.prototype.toString.call(body) !== '[object ArrayBuffer]') { + return undefined + } + + try { + return ArrayBuffer.prototype.slice.call(body as ArrayBuffer, 0) + } catch { + return undefined + } +} + +function snapshotFormData(body: BodyInit): FormData | undefined { + if (typeof FormData === 'undefined') { + return undefined + } + + const entries = getFormDataEntries(body) + if (entries === undefined) { + return undefined + } + + const snapshot = new FormData() + for (const [name, value] of entries) { + if (typeof value === 'string') { + snapshot.append(name, value) + continue + } + + const blobSnapshot = snapshotBlob(value) + if (blobSnapshot === undefined) { + throw new ConfigError( + 'Retry is not supported for FormData files that cannot be cloned safely', + ) + } + snapshot.append(name, blobSnapshot, value.name) + } + return snapshot +} + +function snapshotBlob(value: Blob): Blob | undefined { + if (typeof Blob === 'undefined') { + return undefined + } + + try { + return Blob.prototype.slice.call(value, 0, value.size, value.type) + } catch { + return undefined + } +} + +function getFormDataEntries( + body: BodyInit, +): IterableIterator<[string, FormDataEntryValue]> | undefined { + try { + return FormData.prototype.entries.call(body as FormData) + } catch { + // Some browser-like runtimes implement each realm with a distinct class + // whose platform brand cannot be checked by the current realm's intrinsic. + } + + if (!hasFormDataShape(body)) { + return undefined + } + + try { + return body.entries() + } catch { + return undefined + } +} + +function hasFormDataShape(value: unknown): value is FormData { + if (typeof value !== 'object' || value === null) { + return false + } + + const candidate = value as FormData + try { + return ( + candidate.constructor?.name === 'FormData' && + typeof candidate.append === 'function' && + typeof candidate.delete === 'function' && + typeof candidate.entries === 'function' && + typeof candidate.get === 'function' && + typeof candidate.getAll === 'function' && + typeof candidate.has === 'function' && + typeof candidate.set === 'function' + ) + } catch { + return false + } +} diff --git a/src/internal/query-params.ts b/src/internal/query-params.ts new file mode 100644 index 0000000..746f3bd --- /dev/null +++ b/src/internal/query-params.ts @@ -0,0 +1,136 @@ +import { ConfigError } from '../errors.js' +import type { + PrimitiveQueryValue, + QueryInput, + QueryParams, +} from '../types.js' +import { isURLSearchParams } from './platform-values.js' + +export function snapshotQueryInput(query: QueryInput): QueryInput { + if (isURLSearchParams(query)) { + return new URLSearchParams(URLSearchParams.prototype.toString.call(query)) + } + + const snapshot: QueryParams = {} + for (const [key, value] of Object.entries(query)) { + snapshot[key] = Array.isArray(value) ? [...value] : value + } + return snapshot +} + +export function serializeQueryParams(query?: QueryInput): string { + if (query === undefined) { + return '' + } + + validateQueryInput(query) + return serializeValidatedQueryParams(query) +} + +export function serializeValidatedQueryParams(query?: QueryInput): string { + if (query === undefined) { + return '' + } + + if (isURLSearchParams(query)) { + return URLSearchParams.prototype.toString.call(query) + } + + const params = new URLSearchParams() + + for (const [key, value] of Object.entries(query)) { + if (value === undefined) { + continue + } + + if (Array.isArray(value)) { + for (const item of value) { + params.append(key, serializeScalarQueryValue(item)) + } + continue + } + + params.append(key, serializeScalarQueryValue(value)) + } + + return params.toString() +} + +export function applyQueryString(url: URL, queryString: string): void { + if (queryString === '') { + return + } + + const suffix = url.search === '' ? queryString : `&${queryString}` + url.search += suffix +} + +export function validateQueryInput( + query: unknown, +): asserts query is QueryInput { + if (isURLSearchParams(query)) { + return + } + + if (!isQueryParamsRecord(query)) { + throw new ConfigError('`query` must be a record or URLSearchParams') + } + + validateQueryParams(query) +} + +function serializeScalarQueryValue(value: PrimitiveQueryValue): string { + if (value === null) { + return 'null' + } + + return String(value) +} + +function validateQueryParams(query: QueryParams): void { + for (const [key, value] of Object.entries(query)) { + validateQueryValue(key, value) + } +} + +function isQueryParamsRecord(value: unknown): value is QueryParams { + if (typeof value !== 'object' || value === null || Array.isArray(value)) { + return false + } + + try { + return Object.prototype.toString.call(value) === '[object Object]' + } catch { + return false + } +} + +function validateQueryValue(key: string, value: QueryParams[string]): void { + if (value === undefined) { + return + } + + if (Array.isArray(value)) { + for (const item of value) { + validateQueryScalarValue(key, item) + } + return + } + + validateQueryScalarValue(key, value) +} + +function validateQueryScalarValue(key: string, value: unknown): void { + if ( + value === null || + typeof value === 'string' || + typeof value === 'number' || + typeof value === 'boolean' + ) { + return + } + + throw new ConfigError( + `Unsupported query value for \`${key}\`; only string, number, boolean, null, arrays, and undefined are allowed`, + ) +} diff --git a/src/internal/retry-policy.ts b/src/internal/retry-policy.ts index 2522d89..7db7c78 100644 --- a/src/internal/retry-policy.ts +++ b/src/internal/retry-policy.ts @@ -60,8 +60,12 @@ function buildRetry(source: RetryOptions): Required { backoffMs: source.backoffMs ?? DEFAULT_RETRY.backoffMs, maxBackoffMs: source.maxBackoffMs ?? DEFAULT_RETRY.maxBackoffMs, multiplier: source.multiplier ?? DEFAULT_RETRY.multiplier, - retryOnStatuses: source.retryOnStatuses ?? DEFAULT_RETRY.retryOnStatuses, - retryOnMethods: source.retryOnMethods ?? DEFAULT_RETRY.retryOnMethods, + retryOnStatuses: [ + ...(source.retryOnStatuses ?? DEFAULT_RETRY.retryOnStatuses), + ], + retryOnMethods: [ + ...(source.retryOnMethods ?? DEFAULT_RETRY.retryOnMethods), + ], } } @@ -134,6 +138,17 @@ export function shouldRetryError( return error instanceof NetworkError } +export function getEffectiveRetryAttempts( + method: RequestMethod, + retry: false | Required, +): number { + if (retry === false || !retry.retryOnMethods.includes(method)) { + return 1 + } + + return retry.attempts +} + export function shouldRetryStatus( response: Response, method: RequestMethod, diff --git a/src/types.ts b/src/types.ts index 2f0ae2f..37de513 100644 --- a/src/types.ts +++ b/src/types.ts @@ -231,10 +231,15 @@ export interface NormalizedRequestOptions { /** * Reusable client API produced by `createClient()`. */ -export interface HttpClient { +export interface HttpClient { request( input: string | URL, - options?: JsonRequestOptions, + options?: DefaultRequestOptions, + ): Promise> + + request( + input: string | URL, + options: JsonRequestOptions, ): Promise request( @@ -259,7 +264,12 @@ export interface HttpClient { get( input: string | URL, - options?: JsonBodylessClientMethodOptions, + options?: DefaultBodylessClientMethodOptions, + ): Promise> + + get( + input: string | URL, + options: JsonBodylessClientMethodOptions, ): Promise get( @@ -284,7 +294,12 @@ export interface HttpClient { post( input: string | URL, - options?: JsonClientMethodOptions, + options?: DefaultClientMethodOptions, + ): Promise> + + post( + input: string | URL, + options: JsonClientMethodOptions, ): Promise post( @@ -309,7 +324,12 @@ export interface HttpClient { put( input: string | URL, - options?: JsonClientMethodOptions, + options?: DefaultClientMethodOptions, + ): Promise> + + put( + input: string | URL, + options: JsonClientMethodOptions, ): Promise put( @@ -334,7 +354,12 @@ export interface HttpClient { patch( input: string | URL, - options?: JsonClientMethodOptions, + options?: DefaultClientMethodOptions, + ): Promise> + + patch( + input: string | URL, + options: JsonClientMethodOptions, ): Promise patch( @@ -359,7 +384,12 @@ export interface HttpClient { delete( input: string | URL, - options?: JsonClientMethodOptions, + options?: DefaultClientMethodOptions, + ): Promise> + + delete( + input: string | URL, + options: JsonClientMethodOptions, ): Promise delete( @@ -384,7 +414,12 @@ export interface HttpClient { head( input: string | URL, - options?: JsonBodylessClientMethodOptions, + options?: DefaultBodylessClientMethodOptions, + ): Promise> + + head( + input: string | URL, + options: JsonBodylessClientMethodOptions, ): Promise head( @@ -409,7 +444,12 @@ export interface HttpClient { options( input: string | URL, - options?: JsonClientMethodOptions, + options?: DefaultClientMethodOptions, + ): Promise> + + options( + input: string | URL, + options: JsonClientMethodOptions, ): Promise options( @@ -432,15 +472,46 @@ export interface HttpClient { options: RawClientMethodOptions, ): Promise - extend(defaults: ClientDefaults): HttpClient + extend( + defaults: Omit & { + responseType: ChildResponseType + }, + ): HttpClient + + extend( + defaults: Omit & { + responseType?: never + }, + ): HttpClient + + extend(defaults: ClientDefaults): HttpClient +} + +type ResponseResult = + ResponseMode extends 'json' + ? T | undefined + : ResponseMode extends 'text' + ? string + : ResponseMode extends 'blob' + ? Blob + : ResponseMode extends 'arrayBuffer' + ? ArrayBuffer + : Response + +type DefaultRequestOptions = RequestOptions & { + responseType?: never } type JsonRequestOptions = RequestOptions & { - responseType?: 'json' + responseType: 'json' +} + +type DefaultClientMethodOptions = ClientMethodOptions & { + responseType?: never } type JsonClientMethodOptions = ClientMethodOptions & { - responseType?: 'json' + responseType: 'json' } type BodylessClientMethodOptions = RequestOptionsBase & { @@ -448,8 +519,12 @@ type BodylessClientMethodOptions = RequestOptionsBase & { json?: never } +type DefaultBodylessClientMethodOptions = BodylessClientMethodOptions & { + responseType?: never +} + type JsonBodylessClientMethodOptions = BodylessClientMethodOptions & { - responseType?: 'json' + responseType: 'json' } type TextRequestOptions = RequestOptions & { diff --git a/test/hooks-and-retries.test.ts b/test/hooks-and-retries.test.ts index 684efeb..4afa4fa 100644 --- a/test/hooks-and-retries.test.ts +++ b/test/hooks-and-retries.test.ts @@ -600,7 +600,7 @@ test('beforeRequest hooks can inspect serialized query metadata', async () => { } }) -test('retry attempts rebuild POST json bodies after the first attempt', async () => { +test('retry attempts reuse one serialized POST json body', async () => { const originalFetch = globalThis.fetch let attempts = 0 let stringifyCalls = 0 @@ -609,7 +609,7 @@ test('retry attempts rebuild POST json bodies after the first attempt', async () const payload = { toJSON() { stringifyCalls += 1 - return { ok: true } + return { serialization: stringifyCalls } }, } @@ -644,69 +644,167 @@ test('retry attempts rebuild POST json bodies after the first attempt', async () assert.deepEqual(result, { ok: true }) assert.equal(attempts, 2) - assert.equal(stringifyCalls, 2) - assert.deepEqual(seenBodies, ['{"ok":true}', '{"ok":true}']) + assert.equal(stringifyCalls, 1) + assert.deepEqual(seenBodies, [ + '{"serialization":1}', + '{"serialization":1}', + ]) } finally { globalThis.fetch = originalFetch } }) -test('onError observes retry attempt request rebuild failures before rethrow', async () => { +test('retry attempts do not reread mutable request headers or query', async () => { const originalFetch = globalThis.fetch - const observedErrors: unknown[] = [] + const headers = new Headers({ + 'X-Request-Version': 'initial', + }) + const query = { + version: 'initial', + } let attempts = 0 - let stringifyCalls = 0 + const seenRequests: Array<{ header: string | null; url: string }> = [] - const payload = { - toJSON() { - stringifyCalls += 1 - if (stringifyCalls === 2) { - throw new Error('cannot replay payload') - } - return { ok: true } - }, + globalThis.fetch = async (input) => { + attempts += 1 + const req = input as Request + seenRequests.push({ + header: req.headers.get('x-request-version'), + url: req.url, + }) + + if (attempts === 1) { + headers.set('X-Request-Version', 'mutated') + query.version = 'mutated' + + return new Response('retry', { + status: 503, + statusText: 'Service Unavailable', + }) + } + + return new Response(JSON.stringify({ ok: true })) } + try { + const result = await request<{ ok: boolean }>('https://api.example.com/users', { + headers, + query, + retry: { + attempts: 2, + backoffMs: 1, + maxBackoffMs: 1, + multiplier: 1, + retryOnStatuses: [503], + retryOnMethods: ['GET'], + }, + }) + + assert.deepEqual(result, { ok: true }) + assert.deepEqual(seenRequests, [ + { + header: 'initial', + url: 'https://api.example.com/users?version=initial', + }, + { + header: 'initial', + url: 'https://api.example.com/users?version=initial', + }, + ]) + } finally { + globalThis.fetch = originalFetch + } +}) + +test('retry decisions use an initial snapshot of caller-owned policy arrays', async () => { + const originalFetch = globalThis.fetch + const retryOnStatuses = [503] + const retryOnMethods: Array<'GET'> = ['GET'] + let attempts = 0 + globalThis.fetch = async () => { attempts += 1 - return new Response('retry', { - status: 503, - statusText: 'Service Unavailable', + + if (attempts === 1) { + retryOnStatuses[0] = 500 + retryOnMethods.length = 0 + + return new Response('retry', { + status: 503, + statusText: 'Service Unavailable', + }) + } + + return new Response(JSON.stringify({ ok: true })) + } + + try { + const result = await request<{ ok: boolean }>('https://api.example.com/users', { + retry: { + attempts: 2, + backoffMs: 1, + maxBackoffMs: 1, + multiplier: 1, + retryOnStatuses, + retryOnMethods, + }, }) + + assert.deepEqual(result, { ok: true }) + assert.equal(attempts, 2) + } finally { + globalThis.fetch = originalFetch + } +}) + +test('retry attempts isolate mutable raw bodies from prior hook mutations', async () => { + const originalFetch = globalThis.fetch + const seenBodies: string[] = [] + let attempts = 0 + + globalThis.fetch = async (input) => { + attempts += 1 + const req = input as Request + seenBodies.push(await req.clone().text()) + + if (attempts < 3) { + return new Response('retry', { + status: 503, + statusText: 'Service Unavailable', + }) + } + + return new Response(JSON.stringify({ ok: true })) } try { - await assert.rejects( - () => - request('https://api.example.com/users', { - method: 'POST', - json: payload, - hooks: { - onError: [ - async (context) => { - observedErrors.push(context.error) - }, - ], - }, - retry: { - attempts: 2, - backoffMs: 1, - maxBackoffMs: 1, - multiplier: 1, - retryOnStatuses: [503], - retryOnMethods: ['POST'], + const result = await request<{ ok: boolean }>('https://api.example.com/users', { + method: 'POST', + body: new URLSearchParams({ value: 'base' }), + hooks: { + beforeRequest: [ + (context) => { + assert.ok(context.body instanceof URLSearchParams) + context.body.append('hook', String(context.options.attempt)) }, - }), - (error) => - error instanceof Error && - error.message === 'cannot replay payload', - ) + ], + }, + retry: { + attempts: 3, + backoffMs: 1, + maxBackoffMs: 1, + multiplier: 1, + retryOnStatuses: [503], + retryOnMethods: ['POST'], + }, + }) - assert.equal(attempts, 1) - assert.equal(stringifyCalls, 2) - assert.equal(observedErrors.length, 1) - assert.ok(observedErrors[0] instanceof Error) - assert.equal((observedErrors[0] as Error).message, 'cannot replay payload') + assert.deepEqual(result, { ok: true }) + assert.deepEqual(seenBodies, [ + 'value=base&hook=1', + 'value=base&hook=2', + 'value=base&hook=3', + ]) } finally { globalThis.fetch = originalFetch } @@ -806,6 +904,7 @@ test('retryable HTTP responses cancel abandoned response bodies', async () => { test('abort during HTTP retry backoff stops promptly with AbortRequestError', async () => { const originalFetch = globalThis.fetch const controller = new AbortController() + const observedErrors: unknown[] = [] let attempts = 0 globalThis.fetch = async () => { @@ -819,6 +918,13 @@ test('abort during HTTP retry backoff stops promptly with AbortRequestError', as try { const promise = request('https://api.example.com/users', { signal: controller.signal, + hooks: { + onError: [ + (context) => { + observedErrors.push(context.error) + }, + ], + }, retry: { attempts: 2, backoffMs: 50, @@ -836,6 +942,8 @@ test('abort during HTTP retry backoff stops promptly with AbortRequestError', as (error) => error instanceof AbortRequestError, ) assert.equal(attempts, 1) + assert.equal(observedErrors.length, 1) + assert.ok(observedErrors[0] instanceof AbortRequestError) } finally { globalThis.fetch = originalFetch } @@ -998,6 +1106,7 @@ test('retry runs for network failures when method is eligible', async () => { test('abort during retry backoff stops promptly with AbortRequestError', async () => { const originalFetch = globalThis.fetch + const observedErrors: unknown[] = [] let attempts = 0 globalThis.fetch = async () => { @@ -1011,6 +1120,13 @@ test('abort during retry backoff stops promptly with AbortRequestError', async ( const promise = request('https://api.example.com/users', { signal: controller.signal, + hooks: { + onError: [ + (context) => { + observedErrors.push(context.error) + }, + ], + }, retry: { attempts: 3, backoffMs: 500, @@ -1031,6 +1147,8 @@ test('abort during retry backoff stops promptly with AbortRequestError', async ( ) assert.equal(attempts, 1) + assert.equal(observedErrors.length, 1) + assert.ok(observedErrors[0] instanceof AbortRequestError) assert.ok(Date.now() - startedAt < 250) } finally { globalThis.fetch = originalFetch diff --git a/test/normalize-request.test.ts b/test/normalize-request.test.ts index 0a300db..902b68e 100644 --- a/test/normalize-request.test.ts +++ b/test/normalize-request.test.ts @@ -2,6 +2,8 @@ import assert from 'node:assert/strict' import test from 'node:test' import { runInNewContext } from 'node:vm' +import { Window } from 'happy-dom' + import { ConfigError } from '../src/errors.js' import { buildRequestFromContext, @@ -344,6 +346,29 @@ test('createBeforeRequestContext rejects streaming bodies when retry is enabled' body: new ReadableStream(), retry: { attempts: 2, + retryOnMethods: ['POST'], + }, + }), + (error) => + error instanceof ConfigError && + error.message === 'Retry is not supported for streaming request bodies', + ) +}) + +test('createBeforeRequestContext rejects streaming bodies across realms', () => { + const body = new ReadableStream() + Object.setPrototypeOf(body, { + [Symbol.toStringTag]: 'ReadableStream', + }) + + assert.throws( + () => + createBeforeRequestContext('https://api.example.com/upload', {}, { + method: 'POST', + body, + retry: { + attempts: 2, + retryOnMethods: ['POST'], }, }), (error) => @@ -352,6 +377,178 @@ test('createBeforeRequestContext rejects streaming bodies when retry is enabled' ) }) +test('createBeforeRequestContext does not snapshot bodies for a single attempt', () => { + const body = new URLSearchParams({ value: 'original' }) + + const context = createBeforeRequestContext( + 'https://api.example.com/users', + {}, + { + method: 'POST', + body, + retry: { + attempts: 1, + }, + }, + ) + + assert.equal(context.body, body) +}) + +test('createBeforeRequestContext does not snapshot bodies for retry-ineligible methods', () => { + const body = new URLSearchParams({ value: 'original' }) + + const context = createBeforeRequestContext( + 'https://api.example.com/users', + {}, + { + method: 'POST', + body, + retry: { + attempts: 2, + }, + }, + ) + + assert.equal(context.body, body) + assert.equal(context.options.maxAttempts, 1) +}) + +test('buildRequestFromContext supports streams for retry-ineligible methods', () => { + const body = new ReadableStream() + const context = createBeforeRequestContext( + 'https://api.example.com/users', + {}, + { + method: 'POST', + body, + retry: { + attempts: 2, + }, + }, + ) + + assert.equal(context.body, body) + assert.equal(context.options.maxAttempts, 1) + assert.doesNotThrow(() => buildRequestFromContext(context)) +}) + +test('createBeforeRequestContext snapshots ArrayBuffer bodies across realms', async () => { + const body = runInNewContext('new ArrayBuffer(3)') as ArrayBuffer + new Uint8Array(body).set([65, 66, 67]) + + const context = createBeforeRequestContext( + 'https://api.example.com/users', + {}, + { + method: 'POST', + body, + retry: { + attempts: 2, + retryOnMethods: ['POST'], + }, + }, + ) + + assert.notEqual(context.body, body) + new Uint8Array(body).set([88, 89, 90]) + assert.equal(await buildRequestFromContext(context).text(), 'ABC') +}) + +test('createBeforeRequestContext snapshots FormData bodies across realms', () => { + const window = new Window() + + try { + const foreignBody = new window.FormData() + foreignBody.append('value', 'original') + const body = foreignBody as unknown as FormData + + const context = createBeforeRequestContext( + 'https://api.example.com/users', + {}, + { + method: 'POST', + body, + retry: { + attempts: 2, + retryOnMethods: ['POST'], + }, + }, + ) + + assert.notEqual(context.body, body) + foreignBody.append('value', 'mutated') + assert.deepEqual( + [...FormData.prototype.entries.call(context.body as FormData)], + [['value', 'original']], + ) + } finally { + window.close() + } +}) + +test('createBeforeRequestContext preserves FormData file contents and metadata', async () => { + const body = new FormData() + body.append( + 'file', + new Blob(['ABC'], { type: 'text/plain' }), + 'example.txt', + ) + + const context = createBeforeRequestContext( + 'https://api.example.com/users', + {}, + { + method: 'POST', + body, + retry: { + attempts: 2, + retryOnMethods: ['POST'], + }, + }, + ) + + const file = (context.body as FormData).get('file') + assert.ok(file instanceof Blob) + assert.equal((file as Blob & { name?: string }).name, 'example.txt') + assert.equal(file.type, 'text/plain') + assert.equal(await file.text(), 'ABC') +}) + +test('createBeforeRequestContext rejects uncloneable foreign FormData files', () => { + const window = new Window() + + try { + const body = new window.FormData() + body.append( + 'file', + new window.File(['ABC'], 'example.txt', { type: 'text/plain' }), + ) + + assert.throws( + () => + createBeforeRequestContext( + 'https://api.example.com/users', + {}, + { + method: 'POST', + body: body as unknown as FormData, + retry: { + attempts: 2, + retryOnMethods: ['POST'], + }, + }, + ), + (error) => + error instanceof ConfigError && + error.message === + 'Retry is not supported for FormData files that cannot be cloned safely', + ) + } finally { + window.close() + } +}) + test('buildRequestFromContext serializes json and sets content-type when absent', () => { const context = createBeforeRequestContext( 'https://api.example.com/users', diff --git a/test/retry-policy.test.ts b/test/retry-policy.test.ts index fb787a3..0441f50 100644 --- a/test/retry-policy.test.ts +++ b/test/retry-policy.test.ts @@ -24,7 +24,7 @@ test('normalizeRetry applies DEFAULT_RETRY values', () => { }) }) -test('normalizeRetry preserves retry array references', () => { +test('normalizeRetry snapshots retry arrays', () => { const retryOnStatuses = [503] const retryOnMethods: RetryOptions['retryOnMethods'] = ['GET'] @@ -34,16 +34,20 @@ test('normalizeRetry preserves retry array references', () => { }) assert.ok(retry !== false) - assert.equal(retry.retryOnStatuses, retryOnStatuses) - assert.equal(retry.retryOnMethods, retryOnMethods) + assert.notEqual(retry.retryOnStatuses, retryOnStatuses) + assert.notEqual(retry.retryOnMethods, retryOnMethods) + assert.deepEqual(retry.retryOnStatuses, retryOnStatuses) + assert.deepEqual(retry.retryOnMethods, retryOnMethods) const defaultRetry = normalizeRetry(undefined, { attempts: 2, }) assert.ok(defaultRetry !== false) - assert.equal(defaultRetry.retryOnStatuses, DEFAULT_RETRY.retryOnStatuses) - assert.equal(defaultRetry.retryOnMethods, DEFAULT_RETRY.retryOnMethods) + assert.notEqual(defaultRetry.retryOnStatuses, DEFAULT_RETRY.retryOnStatuses) + assert.notEqual(defaultRetry.retryOnMethods, DEFAULT_RETRY.retryOnMethods) + assert.deepEqual(defaultRetry.retryOnStatuses, DEFAULT_RETRY.retryOnStatuses) + assert.deepEqual(defaultRetry.retryOnMethods, DEFAULT_RETRY.retryOnMethods) }) test('normalizeRetry rejects invalid attempts with existing message', () => { diff --git a/test/type-signatures.ts b/test/type-signatures.ts index 43f3910..3c0c181 100644 --- a/test/type-signatures.ts +++ b/test/type-signatures.ts @@ -1,9 +1,20 @@ import { createClient, request, + type ClientDefaults, + type HttpClient, type NormalizedRequestOptions, + type ResponseType, } from '../src/index.js' +type Equal = + (() => Value extends Left ? 1 : 2) extends + (() => Value extends Right ? 1 : 2) + ? true + : false + +type Expect = Value + const client = createClient() type PublicNormalizedRequestOptions = NormalizedRequestOptions void (undefined as unknown as PublicNormalizedRequestOptions) @@ -46,6 +57,71 @@ const clientJsonPromise: Promise<{ ok: boolean } | undefined> = client.get<{ }>('https://api.example.com/users') void clientJsonPromise +const textDefaultClient = createClient({ responseType: 'text' }) +const defaultTextPromise: Promise = textDefaultClient.get( + 'https://api.example.com/text', +) +void defaultTextPromise + +const rawDefaultClient = createClient({ responseType: 'raw' }) +const defaultRawPromise: Promise = rawDefaultClient.get<{ + ignoredAtRuntime: boolean +}>( + 'https://api.example.com/raw', +) +void defaultRawPromise + +const blobDefaultClient = createClient({ responseType: 'blob' }) +const defaultBlobPromise: Promise = blobDefaultClient.get( + 'https://api.example.com/blob', +) +void defaultBlobPromise + +const arrayBufferDefaultClient = createClient({ responseType: 'arrayBuffer' }) +const defaultArrayBufferPromise: Promise = + arrayBufferDefaultClient.get('https://api.example.com/binary') +void defaultArrayBufferPromise + +const inheritedTextDefault = textDefaultClient.extend({ + headers: { Accept: 'text/plain' }, +}) +const inheritedTextPromise: Promise = inheritedTextDefault.get( + 'https://api.example.com/text', +) +void inheritedTextPromise + +const extendedRawDefault = textDefaultClient.extend({ responseType: 'raw' }) +const extendedRawPromise: Promise = extendedRawDefault.get( + 'https://api.example.com/raw', +) +void extendedRawPromise + +const dynamicDefaults: ClientDefaults = { responseType: 'text' } +const dynamicDefaultClient = createClient(dynamicDefaults) +type DynamicDefaultClient = Expect< + Equal> +> +void (undefined as unknown as DynamicDefaultClient) + +const dynamicExtendedDefaults: ClientDefaults = { responseType: 'raw' } +const dynamicExtendedClient = textDefaultClient.extend(dynamicExtendedDefaults) +type DynamicExtendedClient = Expect< + Equal> +> +void (undefined as unknown as DynamicExtendedClient) + +const explicitJsonFromTextDefault: Promise<{ ok: boolean } | undefined> = + textDefaultClient.get<{ ok: boolean }>('https://api.example.com/users', { + responseType: 'json', + }) +void explicitJsonFromTextDefault + +// @ts-expect-error an explicit client mode requires a matching runtime default +createClient<'text'>() + +// @ts-expect-error an explicit extended mode requires a matching runtime default +textDefaultClient.extend<'raw'>({}) + request('https://api.example.com/create', { method: 'POST', json: { ok: true }, From cef1e9ac6a744ad74c30ebb710779934dfd473dc Mon Sep 17 00:00:00 2001 From: "brian.j.murdock@gmail.com" Date: Wed, 15 Jul 2026 20:57:16 -0500 Subject: [PATCH 05/13] Harden package and release supply chain - validate lockfile origins, integrity, and install-script scope before installs - add browser, TypeScript-floor, and packed-artifact guardrails - publish one verified tarball with isolated authority and provenance checks --- .github/pull_request_template.md | 6 +- .github/workflows/ci.yml | 94 +++++++-- .github/workflows/dependency-review.yml | 9 +- .github/workflows/release.yml | 270 +++++++++++++++++++----- .github/workflows/supply-chain.yml | 46 ++++ .gitignore | 2 +- package-lock.json | 70 +++++- package.json | 17 +- scripts/check-lockfile.mjs | 73 +++++++ scripts/check-pack-smoke.mjs | 82 ++++++- scripts/check-publish-dry-run.mjs | 72 +++++-- test/browser-real.browser.ts | 243 +++++++++++++++++++++ test/tsconfig.types-compat.json | 12 ++ test/type-compatibility.ts | 29 +++ tsconfig.build.json | 2 +- tsconfig.json | 3 +- 16 files changed, 928 insertions(+), 102 deletions(-) create mode 100644 .github/workflows/supply-chain.yml create mode 100644 scripts/check-lockfile.mjs create mode 100644 test/browser-real.browser.ts create mode 100644 test/tsconfig.types-compat.json create mode 100644 test/type-compatibility.ts diff --git a/.github/pull_request_template.md b/.github/pull_request_template.md index cb3142a..ce9dba0 100644 --- a/.github/pull_request_template.md +++ b/.github/pull_request_template.md @@ -6,11 +6,15 @@ Describe the change and why it exists. - [ ] `npm run lint` - [ ] `npm test` +- [ ] `npm run test:browser-real` when browser or web-platform behavior changes +- [ ] `npm run test:types-compat` when public types change - [ ] `npm run build` +- [ ] `npm run check:lockfile` - [ ] `npm run check:package-metadata` - [ ] `npm run check:pack-smoke` - [ ] `npm run check:publish-dry-run` when release behavior changes -- [ ] dependency audit performed when dependencies or lockfiles change +- [ ] `npm run check:dependency-audit` performed when dependencies or lockfiles change +- [ ] `npm run check:dependency-signatures` performed when dependencies or lockfiles change ## Notes diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 11ccd3e..af5a75c 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -12,13 +12,20 @@ env: permissions: contents: read +concurrency: + group: ci-${{ github.event.pull_request.number || github.ref }} + cancel-in-progress: true + jobs: workflow-lint: runs-on: ubuntu-latest + timeout-minutes: 10 steps: - name: Check out repository - uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 + uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0 + with: + persist-credentials: false - name: Lint GitHub Actions workflows uses: reviewdog/action-actionlint@6fb7acc99f4a1008869fa8a0f09cfca740837d9d # v1.72.0 @@ -29,23 +36,30 @@ jobs: verify: runs-on: ubuntu-latest + timeout-minutes: 15 strategy: fail-fast: false matrix: - node-version: [18, 20, 22, 24] + node-version: [18, 20, 22, 24, 26] steps: - name: Check out repository - uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 + uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0 + with: + persist-credentials: false - name: Set up Node.js - uses: actions/setup-node@53b83947a5a98c8d113130e565377fae1a50d02f # v6.3.0 + uses: actions/setup-node@48b55a011bda9f5d6aeb4c2d9c7362e8dae4041e # v6.4.0 with: node-version: ${{ matrix.node-version }} cache: npm + registry-url: https://registry.npmjs.org + + - name: Validate lockfile before install + run: node scripts/check-lockfile.mjs - - name: Install dependencies - run: npm ci + - name: Install dependencies without lifecycle scripts + run: npm ci --ignore-scripts --no-audit --registry=https://registry.npmjs.org - name: Lint run: npm run lint @@ -58,42 +72,94 @@ jobs: browser-like: runs-on: ubuntu-latest + timeout-minutes: 10 steps: - name: Check out repository - uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 + uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0 + with: + persist-credentials: false - name: Set up Node.js - uses: actions/setup-node@53b83947a5a98c8d113130e565377fae1a50d02f # v6.3.0 + uses: actions/setup-node@48b55a011bda9f5d6aeb4c2d9c7362e8dae4041e # v6.4.0 with: node-version: 24 cache: npm + registry-url: https://registry.npmjs.org + + - name: Validate lockfile before install + run: node scripts/check-lockfile.mjs - - name: Install dependencies - run: npm ci + - name: Install dependencies without lifecycle scripts + run: npm ci --ignore-scripts --no-audit --registry=https://registry.npmjs.org - name: Run browser-like tests run: npm run test:browser-like + browser-real: + runs-on: ubuntu-latest + timeout-minutes: 20 + + steps: + - name: Check out repository + uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0 + with: + persist-credentials: false + + - name: Set up Node.js + uses: actions/setup-node@48b55a011bda9f5d6aeb4c2d9c7362e8dae4041e # v6.4.0 + with: + node-version: 24 + cache: npm + registry-url: https://registry.npmjs.org + + - name: Validate lockfile before install + run: node scripts/check-lockfile.mjs + + - name: Install dependencies without lifecycle scripts + run: npm ci --ignore-scripts --no-audit --registry=https://registry.npmjs.org + + - name: Install Chromium + run: node node_modules/playwright/cli.js install --with-deps chromium + + - name: Run real-browser tests + run: npm run test:browser-real + package-guardrails: runs-on: ubuntu-latest + timeout-minutes: 15 steps: - name: Check out repository - uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 + uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0 + with: + persist-credentials: false - name: Set up Node.js - uses: actions/setup-node@53b83947a5a98c8d113130e565377fae1a50d02f # v6.3.0 + uses: actions/setup-node@48b55a011bda9f5d6aeb4c2d9c7362e8dae4041e # v6.4.0 with: node-version: 24 cache: npm + registry-url: https://registry.npmjs.org + + - name: Validate lockfile before install + run: node scripts/check-lockfile.mjs - - name: Install dependencies - run: npm ci + - name: Install dependencies without lifecycle scripts + run: npm ci --ignore-scripts --no-audit --registry=https://registry.npmjs.org + + - name: Audit dependencies + run: npm run check:dependency-audit + + - name: Verify dependency signatures and attestations + run: npm run check:dependency-signatures - name: Build package run: npm run build + - name: Verify TypeScript compatibility + run: npm run test:types-compat + - name: Validate package metadata run: npm run check:package-metadata diff --git a/.github/workflows/dependency-review.yml b/.github/workflows/dependency-review.yml index 8874b0a..901d1a1 100644 --- a/.github/workflows/dependency-review.yml +++ b/.github/workflows/dependency-review.yml @@ -17,13 +17,20 @@ env: permissions: contents: read +concurrency: + group: dependency-review-${{ github.event.pull_request.number || github.ref }} + cancel-in-progress: true + jobs: dependency-review: runs-on: ubuntu-latest + timeout-minutes: 10 steps: - name: Check out repository - uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 + uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0 + with: + persist-credentials: false - name: Review dependency changes if: github.event_name == 'pull_request' diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index bda4d6f..8c277f4 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -11,117 +11,279 @@ env: permissions: contents: read - id-token: write + +concurrency: + group: release + cancel-in-progress: false jobs: verify-release: runs-on: ubuntu-latest + timeout-minutes: 20 + outputs: + artifact_id: ${{ steps.upload.outputs.artifact-id }} + tarball_integrity: ${{ steps.artifact.outputs.tarball_integrity }} + tarball_name: ${{ steps.artifact.outputs.tarball_name }} steps: - name: Check out repository - uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 + uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0 + with: + fetch-depth: 0 + persist-credentials: false + + - name: Validate release tag + if: github.event_name == 'push' + env: + TAG_NAME: ${{ github.ref_name }} + run: | + PACKAGE_VERSION="$(node -p "require('./package.json').version")" + CURRENT_GIT_HEAD="$(git rev-parse HEAD)" + + if [ "$TAG_NAME" != "v$PACKAGE_VERSION" ]; then + echo "Release tag $TAG_NAME does not match package version v$PACKAGE_VERSION" >&2 + exit 1 + fi + + TAG_OBJECT_TYPE="$(git cat-file -t "refs/tags/$TAG_NAME" 2>/dev/null || true)" + if [ "$TAG_OBJECT_TYPE" != "tag" ]; then + echo "Release tag $TAG_NAME must be annotated" >&2 + exit 1 + fi + + git fetch origin main:refs/remotes/origin/main --no-tags + if ! git merge-base --is-ancestor "$CURRENT_GIT_HEAD" origin/main; then + echo "Release commit $CURRENT_GIT_HEAD is not reachable from origin/main" >&2 + exit 1 + fi - name: Set up Node.js - uses: actions/setup-node@53b83947a5a98c8d113130e565377fae1a50d02f # v6.3.0 + uses: actions/setup-node@48b55a011bda9f5d6aeb4c2d9c7362e8dae4041e # v6.4.0 with: node-version: 24 - cache: npm + package-manager-cache: false registry-url: https://registry.npmjs.org - - name: Install dependencies - run: npm ci + - name: Validate lockfile before install + run: node scripts/check-lockfile.mjs - - name: Verify package + - name: Install dependencies without lifecycle scripts + run: npm ci --ignore-scripts --no-audit --registry=https://registry.npmjs.org + + - name: Verify package and dependency evidence run: | npm run lint npm test + npm run test:browser-like npm run build + npm run test:types-compat npm run check:package-metadata - npm run check:pack-smoke + npm run check:dependency-audit + npm run check:dependency-signatures + npm run check:pack-smoke -- --retain - - name: Check publishability for a release tag - if: github.event_name == 'push' - run: npm run check:publish-dry-run + - name: Check exact artifact publishability + run: npm run check:publish-dry-run -- release-artifact/*.tgz + + - name: Record artifact identity + id: artifact + run: | + TARBALLS=(release-artifact/*.tgz) + if [ "${#TARBALLS[@]}" -ne 1 ] || [ ! -f "${TARBALLS[0]}" ]; then + echo "Expected exactly one release tarball" >&2 + exit 1 + fi + + TARBALL="${TARBALLS[0]}" + TARBALL_NAME="$(basename "$TARBALL")" + TARBALL_INTEGRITY="$(TARBALL_PATH="$TARBALL" node --input-type=module <<'NODE' + import { createHash } from 'node:crypto' + import { readFileSync } from 'node:fs' + + const bytes = readFileSync(process.env.TARBALL_PATH) + process.stdout.write(`sha512-${createHash('sha512').update(bytes).digest('base64')}`) + NODE + )" - - name: Check publishability for manual validation - if: github.event_name == 'workflow_dispatch' - run: npm run check:publish-dry-run -- --allow-existing + echo "tarball_name=$TARBALL_NAME" >> "$GITHUB_OUTPUT" + echo "tarball_integrity=$TARBALL_INTEGRITY" >> "$GITHUB_OUTPUT" + + - name: Upload verified release artifact + id: upload + uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7.0.1 + with: + path: release-artifact/${{ steps.artifact.outputs.tarball_name }} + archive: false + if-no-files-found: error + retention-days: 7 publish: if: github.event_name == 'push' needs: verify-release runs-on: ubuntu-latest + timeout-minutes: 10 environment: npm permissions: - contents: write id-token: write steps: - - name: Check out repository - uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 - with: - fetch-depth: 0 - - name: Set up Node.js - uses: actions/setup-node@53b83947a5a98c8d113130e565377fae1a50d02f # v6.3.0 + uses: actions/setup-node@48b55a011bda9f5d6aeb4c2d9c7362e8dae4041e # v6.4.0 with: node-version: 24 - cache: npm + package-manager-cache: false registry-url: https://registry.npmjs.org - - name: Install dependencies - run: npm ci - - - name: Verify package - run: | - npm run lint - npm test - npm run build - npm run check:package-metadata - npm run check:pack-smoke - npm run check:publish-dry-run + - name: Download verified release artifact + uses: actions/download-artifact@3e5f45b2cfb9172054b4087a40e8e0b5a5461e7c # v8.0.1 + with: + artifact-ids: ${{ needs.verify-release.outputs.artifact_id }} + path: release-artifact + digest-mismatch: error - - name: Publish to npm with provenance + - name: Publish exact artifact with provenance env: + EXPECTED_INTEGRITY: ${{ needs.verify-release.outputs.tarball_integrity }} TAG_NAME: ${{ github.ref_name }} + TARBALL_NAME: ${{ needs.verify-release.outputs.tarball_name }} run: | - PACKAGE_VERSION="$(node -p "require('./package.json').version")" - CURRENT_GIT_HEAD="$(git rev-parse HEAD)" - - if [ "$TAG_NAME" != "v$PACKAGE_VERSION" ]; then - echo "Release tag $TAG_NAME does not match package version v$PACKAGE_VERSION" >&2 + TARBALL="./release-artifact/$TARBALL_NAME" + if [ ! -f "$TARBALL" ]; then + echo "Verified tarball $TARBALL was not downloaded" >&2 exit 1 fi - TAG_OBJECT_TYPE="$(git cat-file -t "refs/tags/$TAG_NAME" 2>/dev/null || true)" - if [ "$TAG_OBJECT_TYPE" != "tag" ]; then - echo "Release tag $TAG_NAME must be annotated" >&2 + ACTUAL_INTEGRITY="$(TARBALL_PATH="$TARBALL" node --input-type=module <<'NODE' + import { createHash } from 'node:crypto' + import { readFileSync } from 'node:fs' + + const bytes = readFileSync(process.env.TARBALL_PATH) + process.stdout.write(`sha512-${createHash('sha512').update(bytes).digest('base64')}`) + NODE + )" + if [ "$ACTUAL_INTEGRITY" != "$EXPECTED_INTEGRITY" ]; then + echo "Downloaded tarball integrity does not match verified artifact" >&2 exit 1 fi - git fetch origin main:refs/remotes/origin/main --no-tags - if ! git merge-base --is-ancestor "$CURRENT_GIT_HEAD" origin/main; then - echo "Release commit $CURRENT_GIT_HEAD is not reachable from origin/main" >&2 + PACKAGE_JSON="$(tar -xOf "$TARBALL" package/package.json)" + PACKAGE_NAME="$(PACKAGE_JSON="$PACKAGE_JSON" node -p "JSON.parse(process.env.PACKAGE_JSON).name")" + PACKAGE_VERSION="$(PACKAGE_JSON="$PACKAGE_JSON" node -p "JSON.parse(process.env.PACKAGE_JSON).version")" + + if [ "$PACKAGE_NAME" != "@gavoryn/clearfetch" ]; then + echo "Tarball package name $PACKAGE_NAME is not @gavoryn/clearfetch" >&2 exit 1 fi - PUBLISHED_VERSION="$(npm view "@gavoryn/clearfetch@$PACKAGE_VERSION" version --registry=https://registry.npmjs.org 2>/dev/null || true)" - - if [ "$PUBLISHED_VERSION" = "$PACKAGE_VERSION" ]; then - PUBLISHED_GIT_HEAD="$(npm view "@gavoryn/clearfetch@$PACKAGE_VERSION" gitHead --registry=https://registry.npmjs.org 2>/dev/null || true)" + if [ "$TAG_NAME" != "v$PACKAGE_VERSION" ]; then + echo "Release tag $TAG_NAME does not match tarball version v$PACKAGE_VERSION" >&2 + exit 1 + fi - if [ "$PUBLISHED_GIT_HEAD" != "$CURRENT_GIT_HEAD" ]; then - echo "Published gitHead $PUBLISHED_GIT_HEAD does not match current tag commit $CURRENT_GIT_HEAD" >&2 + PUBLISHED_INTEGRITY="$(npm view "$PACKAGE_NAME@$PACKAGE_VERSION" dist.integrity --registry=https://registry.npmjs.org 2>/dev/null || true)" + if [ -n "$PUBLISHED_INTEGRITY" ]; then + if [ "$PUBLISHED_INTEGRITY" != "$ACTUAL_INTEGRITY" ]; then + echo "Published integrity does not match the verified tarball" >&2 exit 1 fi - - echo "Version $PACKAGE_VERSION is already published; skipping npm publish." + echo "$PACKAGE_NAME@$PACKAGE_VERSION already has the verified bytes; skipping npm publish." else - npm publish + npm publish "$TARBALL" --ignore-scripts --provenance --registry=https://registry.npmjs.org + fi + + REGISTRY_INTEGRITY="$(npm view "$PACKAGE_NAME@$PACKAGE_VERSION" dist.integrity --registry=https://registry.npmjs.org)" + if [ "$REGISTRY_INTEGRITY" != "$ACTUAL_INTEGRITY" ]; then + echo "Registry integrity does not match the verified tarball after publish" >&2 + exit 1 fi + ATTESTATION_URL="$(npm view "$PACKAGE_NAME@$PACKAGE_VERSION" dist.attestations.url --registry=https://registry.npmjs.org)" + if [ -z "$ATTESTATION_URL" ]; then + echo "Published package has no npm attestation URL" >&2 + exit 1 + fi + + ATTESTATION_DIR="$(mktemp -d)" + trap 'rm -rf "$ATTESTATION_DIR"' EXIT + ( + cd "$ATTESTATION_DIR" + npm init --yes >/dev/null + npm install --ignore-scripts --no-audit --registry=https://registry.npmjs.org "$PACKAGE_NAME@$PACKAGE_VERSION" >/dev/null + npm audit signatures --registry=https://registry.npmjs.org + ) + + ATTESTATION_URL="$ATTESTATION_URL" \ + PACKAGE_NAME="$PACKAGE_NAME" \ + PACKAGE_VERSION="$PACKAGE_VERSION" \ + PACKAGE_INTEGRITY="$ACTUAL_INTEGRITY" \ + TAG_NAME="$TAG_NAME" \ + node --input-type=module <<'NODE' + const response = await fetch(process.env.ATTESTATION_URL) + if (!response.ok) { + throw new Error(`npm attestation request failed with ${response.status}`) + } + + const document = await response.json() + const provenance = document.attestations?.find( + (entry) => entry.predicateType === 'https://slsa.dev/provenance/v1', + ) + if (provenance === undefined) { + throw new Error('published package has no SLSA provenance attestation') + } + + const statement = JSON.parse( + Buffer.from(provenance.bundle.dsseEnvelope.payload, 'base64').toString('utf8'), + ) + const expectedDigest = Buffer.from( + process.env.PACKAGE_INTEGRITY.slice('sha512-'.length), + 'base64', + ).toString('hex') + const expectedSubject = `pkg:npm/${process.env.PACKAGE_NAME.replace(/^@/, '%40')}@${process.env.PACKAGE_VERSION}` + const subject = statement.subject?.find((entry) => entry.name === expectedSubject) + if (subject?.digest?.sha512 !== expectedDigest) { + throw new Error('SLSA subject does not match the published package bytes') + } + + const workflow = statement.predicate?.buildDefinition?.externalParameters?.workflow + const expectedRepository = `https://github.com/${process.env.GITHUB_REPOSITORY}` + const expectedRef = `refs/tags/${process.env.TAG_NAME}` + if ( + workflow?.repository !== expectedRepository || + workflow?.path !== '.github/workflows/release.yml' || + workflow?.ref !== expectedRef + ) { + throw new Error('SLSA provenance does not identify the expected release workflow') + } + + const expectedSource = `git+${expectedRepository}@${expectedRef}` + const source = statement.predicate?.buildDefinition?.resolvedDependencies?.find( + (entry) => entry.uri === expectedSource, + ) + if (source?.digest?.gitCommit !== process.env.GITHUB_SHA) { + throw new Error('SLSA provenance does not identify the release commit') + } + + if ( + statement.predicate?.buildDefinition?.internalParameters?.github?.event_name !== 'push' + ) { + throw new Error('SLSA provenance was not produced by a tag push') + } + + console.log('npm provenance matches the expected artifact, workflow, tag, and commit') + NODE + + github-release: + if: github.event_name == 'push' + needs: publish + runs-on: ubuntu-latest + timeout-minutes: 5 + permissions: + contents: write + + steps: - name: Create or verify GitHub Release env: + GH_REPO: ${{ github.repository }} GH_TOKEN: ${{ github.token }} TAG_NAME: ${{ github.ref_name }} run: | diff --git a/.github/workflows/supply-chain.yml b/.github/workflows/supply-chain.yml new file mode 100644 index 0000000..da5aa28 --- /dev/null +++ b/.github/workflows/supply-chain.yml @@ -0,0 +1,46 @@ +name: Supply Chain Audit + +on: + schedule: + - cron: '17 9 * * 1' + workflow_dispatch: + +env: + FORCE_JAVASCRIPT_ACTIONS_TO_NODE24: true + +permissions: + contents: read + +concurrency: + group: supply-chain-audit + cancel-in-progress: true + +jobs: + audit: + runs-on: ubuntu-latest + timeout-minutes: 10 + + steps: + - name: Check out repository + uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0 + with: + persist-credentials: false + + - name: Set up Node.js + uses: actions/setup-node@48b55a011bda9f5d6aeb4c2d9c7362e8dae4041e # v6.4.0 + with: + node-version: 24 + package-manager-cache: false + registry-url: https://registry.npmjs.org + + - name: Validate lockfile before install + run: node scripts/check-lockfile.mjs + + - name: Install dependencies without lifecycle scripts + run: npm ci --ignore-scripts --no-audit --registry=https://registry.npmjs.org + + - name: Audit dependencies + run: npm run check:dependency-audit + + - name: Verify dependency signatures and attestations + run: npm run check:dependency-signatures diff --git a/.gitignore b/.gitignore index 59f6bf5..91804cc 100644 --- a/.gitignore +++ b/.gitignore @@ -5,6 +5,7 @@ build/ coverage/ ignore/ docs/ +release-artifact/ *.tsbuildinfo npm-debug.log* @@ -19,4 +20,3 @@ pnpm-debug.log* AGENTS.md suggestions.md -PREPUBLIC_REVIEW.md diff --git a/package-lock.json b/package-lock.json index f4d8f34..6fa2c61 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,18 +1,20 @@ { "name": "@gavoryn/clearfetch", - "version": "1.0.6", + "version": "1.0.7", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "@gavoryn/clearfetch", - "version": "1.0.6", + "version": "1.0.7", "license": "MIT", "devDependencies": { "@types/node": "^24.5.2", "happy-dom": "^20.8.9", + "playwright": "1.61.1", "tsx": "^4.20.5", - "typescript": "^5.9.2" + "typescript": "^5.9.2", + "typescript-compat": "npm:typescript@5.0.4" }, "engines": { "node": ">=18" @@ -575,6 +577,53 @@ "node": ">=20.0.0" } }, + "node_modules/playwright": { + "version": "1.61.1", + "resolved": "https://registry.npmjs.org/playwright/-/playwright-1.61.1.tgz", + "integrity": "sha512-DWnY5o3YbLWK4GovuAVwpqL+1VwGNdUGrRr++8j8PtQQzvAVZUIMjKQ90fY689sEJZJBbZVw1rXaOKSTitkzPQ==", + "dev": true, + "license": "Apache-2.0", + "dependencies": { + "playwright-core": "1.61.1" + }, + "bin": { + "playwright": "cli.js" + }, + "engines": { + "node": ">=18" + }, + "optionalDependencies": { + "fsevents": "2.3.2" + } + }, + "node_modules/playwright-core": { + "version": "1.61.1", + "resolved": "https://registry.npmjs.org/playwright-core/-/playwright-core-1.61.1.tgz", + "integrity": "sha512-h7Qlt6m4REp25qvIdvbDtVmD4LqVXfpRxhORv9L0jzETM05p4fuPJ3dKyuSXQxDSbXnmS79HAgi9589lGSpLkg==", + "dev": true, + "license": "Apache-2.0", + "bin": { + "playwright-core": "cli.js" + }, + "engines": { + "node": ">=18" + } + }, + "node_modules/playwright/node_modules/fsevents": { + "version": "2.3.2", + "resolved": "https://registry.npmjs.org/fsevents/-/fsevents-2.3.2.tgz", + "integrity": "sha512-xiqMQR4xAeHTuB9uWm+fFRcIOgKBMiOBP+eXiyT7jsgVCq1bkVygt00oASowB7EdtpOHaaPgKt812P9ab+DDKA==", + "dev": true, + "hasInstallScript": true, + "license": "MIT", + "optional": true, + "os": [ + "darwin" + ], + "engines": { + "node": "^8.16.0 || ^10.6.0 || >=11.0.0" + } + }, "node_modules/tsx": { "version": "4.22.4", "resolved": "https://registry.npmjs.org/tsx/-/tsx-4.22.4.tgz", @@ -608,6 +657,21 @@ "node": ">=14.17" } }, + "node_modules/typescript-compat": { + "name": "typescript", + "version": "5.0.4", + "resolved": "https://registry.npmjs.org/typescript/-/typescript-5.0.4.tgz", + "integrity": "sha512-cW9T5W9xY37cc+jfEnaUvX91foxtHkza3Nw3wkoF4sSlKn0MONdkdEndig/qPBWXNkmplh3NzayQzCiHM4/hqw==", + "dev": true, + "license": "Apache-2.0", + "bin": { + "tsc": "bin/tsc", + "tsserver": "bin/tsserver" + }, + "engines": { + "node": ">=12.20" + } + }, "node_modules/undici-types": { "version": "7.16.0", "resolved": "https://registry.npmjs.org/undici-types/-/undici-types-7.16.0.tgz", diff --git a/package.json b/package.json index 488bd0c..c6b292d 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "@gavoryn/clearfetch", - "version": "1.0.6", + "version": "1.0.7", "description": "A dependency-free, fetch-native HTTP client for modern JavaScript and TypeScript runtimes.", "type": "module", "sideEffects": false, @@ -19,13 +19,18 @@ } }, "scripts": { - "build": "node -e \"require('node:fs').rmSync('dist', { recursive: true, force: true })\" && tsc -p tsconfig.build.json", + "build": "node -e \"require('node:fs').rmSync('dist', { recursive: true, force: true })\" && node node_modules/typescript/bin/tsc -p tsconfig.build.json", + "check:dependency-audit": "npm audit --audit-level=moderate --registry=https://registry.npmjs.org", + "check:dependency-signatures": "npm audit signatures --registry=https://registry.npmjs.org", + "check:lockfile": "node scripts/check-lockfile.mjs", "check:package-metadata": "node scripts/check-package-metadata.mjs", "check:pack-smoke": "node scripts/check-pack-smoke.mjs", "check:publish-dry-run": "node scripts/check-publish-dry-run.mjs", - "lint": "tsc --noEmit -p tsconfig.json", + "lint": "node node_modules/typescript/bin/tsc --noEmit -p tsconfig.json", "test": "tsx --test test/*.test.ts", - "test:browser-like": "tsx --test test/browser-like.test.ts" + "test:browser-like": "tsx --test test/browser-like.test.ts", + "test:browser-real": "npm run build && tsx --test test/browser-real.browser.ts", + "test:types-compat": "npm run build && node node_modules/typescript-compat/bin/tsc -p test/tsconfig.types-compat.json" }, "keywords": [ "fetch", @@ -47,8 +52,10 @@ "devDependencies": { "@types/node": "^24.5.2", "happy-dom": "^20.8.9", + "playwright": "1.61.1", "tsx": "^4.20.5", - "typescript": "^5.9.2" + "typescript": "^5.9.2", + "typescript-compat": "npm:typescript@5.0.4" }, "overrides": { "esbuild": "0.28.1", diff --git a/scripts/check-lockfile.mjs b/scripts/check-lockfile.mjs new file mode 100644 index 0000000..7796841 --- /dev/null +++ b/scripts/check-lockfile.mjs @@ -0,0 +1,73 @@ +import { readFile } from 'node:fs/promises' + +const lockfile = JSON.parse(await readFile('package-lock.json', 'utf8')) +const registryOrigin = 'https://registry.npmjs.org' +const allowedInstallScriptPackages = new Set([ + 'node_modules/esbuild', + 'node_modules/fsevents', + 'node_modules/playwright/node_modules/fsevents', +]) +const observedInstallScriptPackages = new Set() + +if (lockfile.lockfileVersion !== 3) { + throw new Error(`package-lock.json must use lockfileVersion 3, found ${lockfile.lockfileVersion}`) +} + +if (lockfile.packages === null || typeof lockfile.packages !== 'object') { + throw new Error('package-lock.json must contain a packages object') +} + +for (const [packagePath, packageEntry] of Object.entries(lockfile.packages)) { + if (packagePath === '') { + continue + } + + if (packageEntry === null || typeof packageEntry !== 'object') { + throw new Error(`lockfile entry ${packagePath} must be an object`) + } + + if (packageEntry.link === true) { + throw new Error(`lockfile entry ${packagePath} must not be a local link`) + } + + let resolved + try { + resolved = new URL(packageEntry.resolved) + } catch { + throw new Error(`lockfile entry ${packagePath} must have a valid resolved URL`) + } + + if ( + resolved.origin !== registryOrigin || + resolved.username !== '' || + resolved.password !== '' + ) { + throw new Error(`lockfile entry ${packagePath} must resolve from ${registryOrigin}`) + } + + if ( + typeof packageEntry.integrity !== 'string' || + !packageEntry.integrity.startsWith('sha512-') + ) { + throw new Error(`lockfile entry ${packagePath} must have sha512 integrity`) + } + + if (packageEntry.dev !== true) { + throw new Error(`lockfile entry ${packagePath} must remain development-only`) + } + + if (packageEntry.hasInstallScript === true) { + observedInstallScriptPackages.add(packagePath) + if (!allowedInstallScriptPackages.has(packagePath)) { + throw new Error(`lockfile entry ${packagePath} adds an unreviewed install script`) + } + } +} + +for (const packagePath of allowedInstallScriptPackages) { + if (!observedInstallScriptPackages.has(packagePath)) { + throw new Error(`install-script allowlist entry ${packagePath} is stale`) + } +} + +console.log('lockfile origin, integrity, and install-script checks passed') diff --git a/scripts/check-pack-smoke.mjs b/scripts/check-pack-smoke.mjs index 2b6dd35..3299a88 100644 --- a/scripts/check-pack-smoke.mjs +++ b/scripts/check-pack-smoke.mjs @@ -1,4 +1,4 @@ -import { mkdtemp, rm, writeFile } from 'node:fs/promises' +import { mkdir, mkdtemp, rename, rm, writeFile } from 'node:fs/promises' import os from 'node:os' import path from 'node:path' import process from 'node:process' @@ -7,18 +7,29 @@ import { promisify } from 'node:util' const execFileAsync = promisify(execFile) const rootDir = process.cwd() -const tscPath = path.join(rootDir, 'node_modules', '.bin', 'tsc') +const tscPath = path.join(rootDir, 'node_modules', 'typescript', 'bin', 'tsc') +const retainTarball = process.argv.includes('--retain') +const unexpectedArguments = process.argv.slice(2).filter((argument) => argument !== '--retain') +const MAX_PACKED_BYTES = 50_000 +const MAX_UNPACKED_BYTES = 175_000 +const MAX_PACKED_FILES = 65 -const { stdout } = await execFileAsync('npm', ['pack', '--json'], { +if (unexpectedArguments.length > 0) { + throw new Error(`unexpected arguments: ${unexpectedArguments.join(', ')}`) +} + +const { stdout } = await execFileAsync('npm', ['pack', '--json', '--ignore-scripts'], { cwd: rootDir, }) const [packResult] = JSON.parse(stdout) const tarballPath = path.join(rootDir, packResult.filename) assertPackedFiles(packResult.files) +assertPackedSize(packResult) const packageName = '@gavoryn/clearfetch' const tempDir = await mkdtemp(path.join(os.tmpdir(), 'clearfetch-pack-')) +let tarballRetained = false try { const importSmokeFile = path.join(tempDir, 'smoke-import.mjs') @@ -120,6 +131,23 @@ try { 'const jsonPromise: Promise<{ ok: boolean } | undefined> = client.get<{ ok: boolean }>(\'/users\')', 'void jsonPromise', '', + "const textClient = createClient({ baseURL: 'https://api.example.com', responseType: 'text' })", + "const defaultTextPromise: Promise = textClient.get('/health')", + 'void defaultTextPromise', + '', + "const rawClient = textClient.extend({ responseType: 'raw' })", + "const defaultRawPromise: Promise = rawClient.get('/download')", + 'void defaultRawPromise', + '', + "const explicitJsonPromise: Promise<{ ok: boolean } | undefined> = textClient.get<{ ok: boolean }>('/users', { responseType: 'json' })", + 'void explicitJsonPromise', + '', + '// @ts-expect-error an explicit client mode requires a matching runtime default', + "createClient<'text'>()", + '', + '// @ts-expect-error an explicit extended mode requires a matching runtime default', + "textClient.extend<'raw'>({})", + '', 'async function smokeRequestBodies() {', " await request('https://api.example.com/create', {", " method: 'POST',", @@ -197,9 +225,21 @@ try { ], { cwd: tempDir }, ) + + if (retainTarball) { + const artifactDir = path.join(rootDir, 'release-artifact') + const artifactPath = path.join(artifactDir, packResult.filename) + await rm(artifactDir, { recursive: true, force: true }) + await mkdir(artifactDir) + await rename(tarballPath, artifactPath) + tarballRetained = true + console.log(`retained verified tarball at ${artifactPath}`) + } } finally { await rm(tempDir, { recursive: true, force: true }) - await rm(tarballPath, { force: true }) + if (!tarballRetained) { + await rm(tarballPath, { force: true }) + } } console.log('packed artifact smoke checks passed') @@ -219,6 +259,40 @@ function assertPackedFiles(files) { if (unexpectedFiles.length > 0) { throw new Error(`unexpected files in packed artifact: ${unexpectedFiles.join(', ')}`) } + + const declarationMaps = files + .map((entry) => entry.path) + .filter((filePath) => filePath.endsWith('.d.ts.map')) + if (declarationMaps.length > 0) { + throw new Error( + `declaration maps must not ship without their TypeScript sources: ${declarationMaps.join(', ')}`, + ) + } +} + +function assertPackedSize(packResult) { + if (!Number.isFinite(packResult.size) || !Number.isFinite(packResult.unpackedSize)) { + throw new Error('npm pack did not report finite packed and unpacked byte counts') + } + + const fileCount = packResult.files.length + const violations = [] + + if (packResult.size > MAX_PACKED_BYTES) { + violations.push(`packed bytes ${packResult.size} > ${MAX_PACKED_BYTES}`) + } + if (packResult.unpackedSize > MAX_UNPACKED_BYTES) { + violations.push( + `unpacked bytes ${packResult.unpackedSize} > ${MAX_UNPACKED_BYTES}`, + ) + } + if (fileCount > MAX_PACKED_FILES) { + violations.push(`file count ${fileCount} > ${MAX_PACKED_FILES}`) + } + + if (violations.length > 0) { + throw new Error(`packed artifact exceeds its size budget: ${violations.join('; ')}`) + } } function shellEscape(value) { diff --git a/scripts/check-publish-dry-run.mjs b/scripts/check-publish-dry-run.mjs index b9a7d2a..4528f83 100644 --- a/scripts/check-publish-dry-run.mjs +++ b/scripts/check-publish-dry-run.mjs @@ -1,43 +1,67 @@ import { execFile } from 'node:child_process' +import { createHash } from 'node:crypto' import { readFile } from 'node:fs/promises' +import path from 'node:path' import process from 'node:process' import { promisify } from 'node:util' const execFileAsync = promisify(execFile) const registry = 'https://registry.npmjs.org' const allowExisting = process.argv.includes('--allow-existing') +const tarballArgument = process.argv + .slice(2) + .find((argument) => !argument.startsWith('--')) const packageConfig = JSON.parse(await readFile('package.json', 'utf8')) const packageSpec = `${packageConfig.name}@${packageConfig.version}` const published = await getPublishedMetadata(packageSpec) +const tarballIntegrity = tarballArgument === undefined + ? undefined + : await calculateIntegrity(tarballArgument) +const tarballConfig = tarballArgument === undefined + ? undefined + : await getTarballPackageConfig(tarballArgument) + +if ( + tarballConfig !== undefined && + (tarballConfig.name !== packageConfig.name || + tarballConfig.version !== packageConfig.version) +) { + throw new Error( + `tarball identity ${String(tarballConfig.name)}@${String(tarballConfig.version)} does not match workspace ${packageSpec}`, + ) +} if (published === undefined) { + const publishTarget = tarballArgument === undefined + ? '.' + : path.resolve(tarballArgument) const { stderr, stdout } = await execFileAsync( 'npm', - ['publish', '--dry-run', `--registry=${registry}`], + [ + 'publish', + publishTarget, + '--dry-run', + '--ignore-scripts', + `--registry=${registry}`, + ], { maxBuffer: 10 * 1024 * 1024 }, ) process.stdout.write(stdout) process.stderr.write(stderr) console.log(`publish dry-run passed for unpublished version ${packageSpec}`) -} else { - if (allowExisting) { - console.log( - `${packageSpec} is already published; publish dry-run skipped for manual validation`, - ) - process.exit(0) - } - - const { stdout } = await execFileAsync('git', ['rev-parse', 'HEAD']) - const currentGitHead = stdout.trim() - - if (published.gitHead !== currentGitHead) { +} else if (tarballIntegrity !== undefined) { + if (published['dist.integrity'] !== tarballIntegrity) { throw new Error( - `published gitHead ${String(published.gitHead)} does not match current commit ${currentGitHead}`, + `published integrity ${String(published['dist.integrity'])} does not match verified tarball ${tarballIntegrity}`, ) } - console.log( - `${packageSpec} is already published from the current commit; publish dry-run skipped`, + console.log(`${packageSpec} is already published with the verified tarball bytes`) +} else if (allowExisting) { + console.log(`${packageSpec} is already published; non-publishing validation skipped`) +} else { + throw new Error( + `${packageSpec} is already published; pass a verified tarball to compare integrity or use --allow-existing for non-publishing validation`, ) } @@ -49,7 +73,7 @@ async function getPublishedMetadata(packageSpec) { 'view', packageSpec, 'version', - 'gitHead', + 'dist.integrity', '--json', `--registry=${registry}`, ], @@ -64,6 +88,20 @@ async function getPublishedMetadata(packageSpec) { } } +async function calculateIntegrity(tarballPath) { + const tarball = await readFile(path.resolve(tarballPath)) + return `sha512-${createHash('sha512').update(tarball).digest('base64')}` +} + +async function getTarballPackageConfig(tarballPath) { + const { stdout } = await execFileAsync( + 'tar', + ['-xOf', path.resolve(tarballPath), 'package/package.json'], + { maxBuffer: 1024 * 1024 }, + ) + return JSON.parse(stdout) +} + function isNotFoundError(error) { if (typeof error !== 'object' || error === null) { return false diff --git a/test/browser-real.browser.ts b/test/browser-real.browser.ts new file mode 100644 index 0000000..f921bf2 --- /dev/null +++ b/test/browser-real.browser.ts @@ -0,0 +1,243 @@ +import assert from 'node:assert/strict' +import { readFile } from 'node:fs/promises' +import { createServer, type IncomingMessage, type ServerResponse } from 'node:http' +import path from 'node:path' +import test from 'node:test' +import { fileURLToPath } from 'node:url' + +import { chromium } from 'playwright' + +const rootDir = path.resolve(path.dirname(fileURLToPath(import.meta.url)), '..') +const distDir = path.join(rootDir, 'dist') + +test('real browser handles native values created in another realm', { + timeout: 30_000, +}, async (t) => { + const attempts = new Map() + const requestBodies = new Map() + const requestContentTypes = new Map() + const server = createServer(async (request, response) => { + try { + await handleRequest( + request, + response, + attempts, + requestBodies, + requestContentTypes, + ) + } catch (error) { + response.writeHead(500, { 'Content-Type': 'text/plain' }) + response.end(error instanceof Error ? error.stack : String(error)) + } + }) + + await new Promise((resolve, reject) => { + server.once('error', reject) + server.listen(0, '127.0.0.1', () => resolve()) + }) + t.after(() => new Promise((resolve, reject) => { + server.close((error) => error === undefined ? resolve() : reject(error)) + })) + + const address = server.address() + assert(address !== null && typeof address === 'object') + const origin = `http://127.0.0.1:${address.port}` + + const browser = await chromium.launch({ headless: true }) + t.after(() => browser.close()) + const page = await browser.newPage() + await page.goto(origin) + + let result: { + arrayBufferResult: { attempts: number; bodies: number[][] } + formDataResult: { attempts: number; bodies: string[]; contentTypes: string[] } + queryResult: { search: string } + streamResult: { body: string } + } + try { + result = await page.evaluate(async ({ origin }) => { + const { createClient } = await import(`${origin}/dist/index.js`) + const { + buildRequestFromContext, + createBeforeRequestContext, + } = await import(`${origin}/dist/internal/normalize-request.js`) + const iframe = document.createElement('iframe') + iframe.srcdoc = 'foreign realm' + const iframeLoaded = new Promise((resolve) => { + iframe.addEventListener('load', () => resolve(), { once: true }) + }) + document.body.append(iframe) + await iframeLoaded + + const foreignWindow = iframe.contentWindow + if (foreignWindow === null) { + throw new Error('iframe realm is unavailable') + } + const foreign = foreignWindow as Window & typeof globalThis + + const client = createClient({ baseURL: origin }) + const retry = { + attempts: 2, + backoffMs: 0, + maxBackoffMs: 0, + retryOnMethods: ['POST'] as const, + retryOnStatuses: [503], + } + + const query = new foreign.URLSearchParams('tag=admin&page=1&tag=editor') + const queryResult = await client.get('/query', { query }) + + const bytes = new foreign.ArrayBuffer(4) + new foreign.Uint8Array(bytes).set([1, 2, 3, 4]) + const arrayBufferResult = await client.post('/array-buffer', { + body: bytes, + retry, + }) + + const form = new foreign.FormData() + form.append('field', 'value') + form.append( + 'file', + new foreign.File(['file-contents'], 'avatar.txt', { type: 'text/plain' }), + ) + const formDataResult = await client.post('/form-data', { + body: form, + retry, + }) + + const stream = new foreign.ReadableStream({ + start(controller) { + controller.enqueue(new foreign.TextEncoder().encode('stream-body')) + controller.close() + }, + }) + const streamContext = createBeforeRequestContext(`${origin}/stream`, {}, { + method: 'POST', + body: stream, + }) + const streamRequest = buildRequestFromContext(streamContext) + const streamResult = { + body: await new Response(streamRequest.body).text(), + } + + iframe.remove() + return { + arrayBufferResult, + formDataResult, + queryResult, + streamResult, + } + }, { origin }) as typeof result + } catch (cause) { + throw new Error( + `browser request failed after attempts ${JSON.stringify(Object.fromEntries(attempts))} and bodies ${JSON.stringify([...requestBodies.keys()])}`, + { cause }, + ) + } + + assert.deepEqual(result.queryResult, { + search: '?tag=admin&page=1&tag=editor', + }) + assert.deepEqual(result.arrayBufferResult, { + attempts: 2, + bodies: [[1, 2, 3, 4], [1, 2, 3, 4]], + }) + assert.equal(result.formDataResult.attempts, 2) + assert.equal(result.formDataResult.bodies.length, 2) + assert.equal(result.formDataResult.contentTypes.length, 2) + for (const [index, body] of result.formDataResult.bodies.entries()) { + assert.match(result.formDataResult.contentTypes[index] ?? '', /^multipart\/form-data; boundary=/) + assert.match(body, /name="field"\r\n\r\nvalue/) + assert.match(body, /name="file"; filename="avatar.txt"/) + assert.match(body, /Content-Type: text\/plain/) + assert.match(body, /file-contents/) + } + assert.deepEqual(result.streamResult, { body: 'stream-body' }) +}) + +async function handleRequest( + request: IncomingMessage, + response: ServerResponse, + attempts: Map, + requestBodies: Map, + requestContentTypes: Map, +): Promise { + const url = new URL(request.url ?? '/', 'http://127.0.0.1') + + if (url.pathname.startsWith('/dist/')) { + const filePath = path.resolve(rootDir, `.${url.pathname}`) + if (!filePath.startsWith(`${distDir}${path.sep}`)) { + response.writeHead(403) + response.end() + return + } + response.writeHead(200, { 'Content-Type': 'text/javascript' }) + response.end(await readFile(filePath)) + return + } + + if (url.pathname === '/') { + response.writeHead(200, { 'Content-Type': 'text/html' }) + response.end('clearfetch browser test') + return + } + + if (url.pathname === '/query') { + sendJson(response, { search: url.search }) + return + } + + const body = await readRequestBody(request) + const bodies = requestBodies.get(url.pathname) ?? [] + bodies.push(body) + requestBodies.set(url.pathname, bodies) + const contentTypes = requestContentTypes.get(url.pathname) ?? [] + contentTypes.push(request.headers['content-type'] ?? '') + requestContentTypes.set(url.pathname, contentTypes) + + if (url.pathname === '/stream') { + sendJson(response, { body: body.toString('utf8') }) + return + } + + const attempt = (attempts.get(url.pathname) ?? 0) + 1 + attempts.set(url.pathname, attempt) + if (attempt === 1) { + response.writeHead(503, { 'Content-Type': 'text/plain' }) + response.end('retry') + return + } + + if (url.pathname === '/array-buffer') { + sendJson(response, { + attempts: attempt, + bodies: bodies.map((value) => [...value]), + }) + return + } + + if (url.pathname === '/form-data') { + sendJson(response, { + attempts: attempt, + bodies: bodies.map((value) => value.toString('utf8')), + contentTypes, + }) + return + } + + response.writeHead(404) + response.end() +} + +function sendJson(response: ServerResponse, value: unknown): void { + response.writeHead(200, { 'Content-Type': 'application/json' }) + response.end(JSON.stringify(value)) +} + +async function readRequestBody(request: IncomingMessage): Promise { + const chunks: Buffer[] = [] + for await (const chunk of request) { + chunks.push(Buffer.isBuffer(chunk) ? chunk : Buffer.from(chunk)) + } + return Buffer.concat(chunks) +} diff --git a/test/tsconfig.types-compat.json b/test/tsconfig.types-compat.json new file mode 100644 index 0000000..2c8178e --- /dev/null +++ b/test/tsconfig.types-compat.json @@ -0,0 +1,12 @@ +{ + "compilerOptions": { + "target": "ES2022", + "module": "NodeNext", + "moduleResolution": "NodeNext", + "lib": ["ES2022", "DOM", "DOM.Iterable"], + "types": [], + "strict": true, + "noEmit": true + }, + "files": ["type-compatibility.ts"] +} diff --git a/test/type-compatibility.ts b/test/type-compatibility.ts new file mode 100644 index 0000000..a6764bd --- /dev/null +++ b/test/type-compatibility.ts @@ -0,0 +1,29 @@ +import { + createClient, + request, + type HttpClient, +} from '../dist/index.js' + +const jsonClient: HttpClient = createClient() +const jsonResult: Promise<{ ok: boolean } | undefined> = + jsonClient.get<{ ok: boolean }>('https://api.example.com/status') + +const textClient = createClient({ responseType: 'text' }) +const textResult: Promise = textClient.get( + 'https://api.example.com/status', +) + +const rawClient = textClient.extend({ responseType: 'raw' }) +const rawResult: Promise = rawClient.get( + 'https://api.example.com/status', +) + +const requestResult: Promise = request( + 'https://api.example.com/data', + { responseType: 'arrayBuffer' }, +) + +void jsonResult +void textResult +void rawResult +void requestResult diff --git a/tsconfig.build.json b/tsconfig.build.json index 0b454ac..63673f5 100644 --- a/tsconfig.build.json +++ b/tsconfig.build.json @@ -4,7 +4,7 @@ "rootDir": "src", "outDir": "dist", "declaration": true, - "declarationMap": true, + "declarationMap": false, "sourceMap": true }, "include": ["src/**/*.ts"], diff --git a/tsconfig.json b/tsconfig.json index 8c68a37..298ed3c 100644 --- a/tsconfig.json +++ b/tsconfig.json @@ -14,5 +14,6 @@ "skipLibCheck": true, "rootDir": "." }, - "include": ["src/**/*.ts", "test/**/*.ts"] + "include": ["src/**/*.ts", "test/**/*.ts"], + "exclude": ["test/type-compatibility.ts"] } From d3dc4b52c06fb10632d695b58dcf637a4335ded8 Mon Sep 17 00:00:00 2001 From: "brian.j.murdock@gmail.com" Date: Wed, 15 Jul 2026 20:57:38 -0500 Subject: [PATCH 06/13] Document v1.0.7 hardening and release policy - describe stable retry replay, response typing, and runtime constraints - record package, supply-chain, and exact-artifact release guardrails - publish the 1.0.7 changelog and contributor verification guidance --- CHANGELOG.md | 17 ++++++++-- CONTRIBUTING.md | 16 +++++++--- DESIGN.md | 42 +++++++++++++++++++++++-- README.md | 71 +++++++++++++++++++++++++++++++++++------ RELEASE.md | 84 ++++++++++++++++++++++++++++++++++++++++--------- SECURITY.md | 18 +++++++++++ 6 files changed, 215 insertions(+), 33 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 279b8ad..4c8f8e3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,14 +3,27 @@ Entries describe repository versions. A version is publicly released only after the matching `vX.Y.Z` tag publishes to npm and a GitHub Release exists. -## Unreleased +## 1.0.7 +- snapshot retry inputs and serialize JSON once so every eligible attempt replays stable request values without copying bodies for excluded methods +- preserve retryable `FormData` file contents and metadata, reject files that cannot be cloned safely, and limit stream rejection to methods with multiple effective attempts +- clarify that retried multipart bodies preserve values but do not guarantee byte-identical boundary encoding +- reflect client-level response type defaults in TypeScript return types, including extended clients, and deprecate the internal `NormalizedRequestOptions` export +- route aborts during retry backoff through `onError` before rethrowing - cancel abandoned response bodies before retrying eligible HTTP failures - reject invalid query containers and JSON values for which `JSON.stringify` returns `undefined` or throws `TypeError` with `ConfigError` -- recognize `URLSearchParams` values across browser realm boundaries +- recognize `URLSearchParams`, `ArrayBuffer`, `FormData`, and request streams across browser realm boundaries +- add real Chromium cross-realm coverage and verify published declarations with TypeScript 5.0 +- split query serialization and web-platform value handling into focused internal modules +- omit declaration maps that reference unshipped source files and enforce packed artifact size budgets +- add Node.js 26 compatibility coverage plus workflow concurrency and timeout limits - require release tags to be annotated and reachable from `main` before publishing - keep release verification rerunnable when the package version already exists for the same commit - distinguish Node.js package compatibility from upstream-backed security support +- lock all executed TypeScript compilers and validate dependency origins, integrity, and install-script scope before installation +- disable dependency lifecycle scripts and persisted checkout credentials throughout CI and release automation +- audit advisories, registry signatures, and attestations in CI, before release, and on a weekly schedule +- publish the exact smoke-tested tarball from an OIDC-only job, verify its registry provenance, and isolate GitHub Release write authority in a separate job ## 1.0.6 diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index ddfa4de..d868273 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -11,19 +11,27 @@ Thanks for the interest in improving `clearfetch`. ## Development -- `npm install` +- `npm ci --ignore-scripts --registry=https://registry.npmjs.org` - `npm run lint` - `npm test` - `npm run test:browser-like` +- `node node_modules/playwright/cli.js install chromium` once before the first real-browser test +- `npm run test:browser-real` +- `npm run test:types-compat` - `npm run build` +- `npm run check:lockfile` - `npm run check:package-metadata` - `npm run check:pack-smoke` - `npm run check:publish-dry-run` for release-path changes -For dependency changes, also run the relevant audit command, usually: +For dependency or lockfile changes, also run: -- `npm audit --registry=https://registry.npmjs.org` -- `npm audit --omit=dev --registry=https://registry.npmjs.org` +- `npm run check:dependency-audit` +- `npm run check:dependency-signatures` + +Do not add a lockfile source outside the public npm registry or a new package +with an install script without an explicit security review. The automated +lockfile check deliberately fails those changes. Changes should keep the public API, docs, tests, and runtime behavior aligned. diff --git a/DESIGN.md b/DESIGN.md index 8aa31c7..639abca 100644 --- a/DESIGN.md +++ b/DESIGN.md @@ -161,12 +161,21 @@ This package is designed for modern JavaScript runtimes that provide native `fet The minimum supported Node.js version should be declared in `package.json` under `engines`. The implementation should target only runtimes that satisfy that requirement. +The published declaration surface supports TypeScript 5.0 and newer. CI must +compile a consumer fixture with the minimum supported TypeScript version so +type-surface changes do not silently raise that floor. + ### Module strategy The package is designed as a modern module-first library. Export behavior must be explicit and restricted through `package.json` export maps. The package should not expose internal implementation paths. +The npm artifact ships JavaScript source maps for mapped stack traces. It does +not ship declaration maps because TypeScript source files are not included in +the package. Packed bytes, unpacked bytes, and file count are bounded by +deliberate package-smoke budgets so the small-package goal remains measurable. + --- ## Public API philosophy @@ -510,6 +519,8 @@ The package supports explicit response parsing modes: The default response type should be `json` unless explicitly changed. This reflects the most common modern use case and offers practical convenience. This default must be clearly documented because it differs from raw fetch semantics. +Client types must track an explicitly configured default response type so calls +without a request-level override retain the correct result type. ### JSON parsing behavior @@ -715,7 +726,7 @@ Permitted uses: Runs after a failure has been classified but before it is re-thrown to the caller. -For transport, timeout, external abort, HTTP, and parse failures, `onError` receives the normalized package error after classification has occurred. +For transport, timeout, external abort, HTTP, and parse failures, `onError` receives the normalized package error after classification has occurred. This includes external aborts that occur during retry backoff. For hook failures and request-construction failures, `onError` receives the error as thrown. This preserves the package's explicit-failure posture without wrapping consumer hook bugs or configuration failures as misleading network-layer errors. @@ -799,11 +810,25 @@ When enabled, retries should default to safe cases such as: - selected HTTP statuses such as `429`, `502`, `503`, `504` - replayable request bodies only +Retry execution must snapshot the initially normalized URL, headers, retry +policy, and body before the first attempt. JSON is serialized once so later +attempts replay the same payload rather than re-reading caller-owned state. + ### Unsafe scenarios The package should not automatically retry unsafe methods such as `POST`, `PUT`, `PATCH`, or `DELETE` unless the caller explicitly configures that behavior. -Version 1 must also reject retry-enabled execution for streaming request bodies. Retries must not assume that all bodies can be replayed safely. +Version 1 must also reject retry-eligible execution for streaming request bodies. A configured retry policy does not make an excluded method retryable, and retries must not assume that all bodies can be replayed safely. + +Form data files must be copied without coercing their contents or metadata. If +the current runtime cannot safely clone a file value from another realm or +implementation, normalization must reject the retryable request rather than +silently replaying a different payload. + +Form data replay guarantees equivalent field values and file contents, names, +and media types. It does not guarantee byte-identical multipart encoding because +the platform may generate a new boundary for each `Request`. Consumers that sign +exact request bytes must pre-serialize the body or disable retries for it. ### Backoff behavior @@ -830,7 +855,7 @@ This is simpler to implement and reason about. If a future version introduces to Retry behavior should be visible to hook contexts where practical so consuming applications can log and understand repeated attempts. -Hooks expose the current attempt through `context.options.attempt` and the configured attempt ceiling through `context.options.maxAttempts`. When the request `query` option serializes to a non-empty string, hooks also receive `context.options.queryString` without a leading `?`; URL search parameters already present in the input remain visible through `context.url`. Applications own any logging, metrics, or tracing behavior built from that metadata. +Hooks expose the current attempt through `context.options.attempt` and the effective attempt ceiling after method eligibility through `context.options.maxAttempts`. When the request `query` option serializes to a non-empty string, hooks also receive `context.options.queryString` without a leading `?`; URL search parameters already present in the input remain visible through `context.url`. Applications own any logging, metrics, or tracing behavior built from that metadata. ### Retry classification @@ -891,6 +916,12 @@ The repository and release process should include: - CI-based publishing - npm 2FA - signed tags where practical +- full-SHA GitHub Actions pins and non-persisted checkout credentials +- lockfile origin and integrity validation before dependency installation +- lifecycle-script-free dependency installation in automation +- dependency advisory, signature, and attestation checks +- publication of the exact verified tarball with npm provenance +- separate npm publication and GitHub Release privileges - strict package export maps - files whitelisting in published package artifacts - a `SECURITY.md` disclosure policy @@ -933,6 +964,7 @@ Examples: * discourage simultaneous `body` and `json` * reject body shapes on `GET` and `HEAD` * constrain response-type values +* preserve client-level default response types in method return types * strongly type hook contexts and retry configuration Runtime validation still remains necessary, especially for JavaScript callers and intentionally invalid test inputs. Invalid body combinations should be guarded both by public TypeScript types and runtime validation. @@ -970,6 +1002,8 @@ Recommended internal responsibilities include: * option validation * URL construction +* query serialization +* platform-native value detection and body snapshotting * header merging * request construction * timeout and signal composition @@ -1007,6 +1041,7 @@ The following invariants are part of the design contract. * request-level options override client defaults * header merging is deterministic +* retry attempts do not re-read caller-owned request configuration * `json` and `body` are mutually exclusive * invalid configuration fails before network execution * timeout timers are always cleaned up @@ -1025,6 +1060,7 @@ The following invariants are part of the design contract. * hooks run in definition order * hook failures are not swallowed * `onError` runs after failure classification and before re-throw +* retry-backoff aborts run through `onError` exactly once * hook and request-construction failures are surfaced as thrown ### Retry invariants diff --git a/README.md b/README.md index fa50f25..66706ba 100644 --- a/README.md +++ b/README.md @@ -141,6 +141,10 @@ const authed = api.extend({ const profile = await authed.get('/me') ``` +Request-level options override client defaults. Request headers replace matching +client header values, while client hooks run before request hooks and each hook +list retains its definition order. + ### Conservative retries ```ts @@ -160,8 +164,12 @@ const api = createClient({ const response = await api.get('/status') ``` -Retries are disabled by default. When enabled, they are intentionally conservative and do not allow streaming request bodies. +Retries are disabled by default. When enabled, they are intentionally conservative. Streaming request bodies are rejected when the request method is eligible for multiple attempts. They are a convenience for bounded retry cases, not a general resilience framework. +Retried `FormData` preserves field values and file contents, names, and media +types, but native multipart boundary encoding is not guaranteed to be +byte-for-byte identical between attempts. Pre-serialize a body when exact bytes +are part of an application signature or idempotency scheme. ### Abort a request @@ -217,9 +225,10 @@ const api = createClient({ `beforeRequest` hook failures, request-normalization failures, retry rebuild failures, and request-construction failures propagate as-is and are observable -through `onError` before being re-thrown. `afterResponse` hooks receive a -cloned `Response`, so reading the body there does not consume the response used -for normal parsing or `HttpError` creation. +through `onError` before being re-thrown. Retry-backoff aborts are normalized to +`AbortRequestError`, passed through `onError`, and re-thrown. `afterResponse` +hooks receive a cloned `Response`, so reading the body there does not consume +the response used for normal parsing or `HttpError` creation. Hook scope is intentionally narrow: @@ -227,6 +236,9 @@ Hook scope is intentionally narrow: - `afterResponse` and `onError` are observational only apart from throwing - `context.options` is read-only hook metadata, not a supported mutation surface +Client hooks run before request hooks. Within each client or request hook list, +hooks run in definition order. + Cloned `afterResponse` inspection is intended for ordinary API payloads, not large streaming or heavy binary workflows. #### Safe diagnostic header logging @@ -298,8 +310,21 @@ const health = await api.get('/health', { const rawResponse = await api.get('/download', { responseType: 'raw', }) + +const textApi = createClient({ + baseURL: 'https://api.example.com', + responseType: 'text', +}) + +const typedHealth: string = await textApi.get('/health') +const jsonStatus = await textApi.get<{ ok: boolean }>('/status', { + responseType: 'json', +}) ``` +Client-level `responseType` defaults are reflected in the returned client type. +A request-level `responseType` still overrides the client default. + ### Runtime validation TypeScript generics describe the expected response shape, but they do not validate response data at runtime. @@ -323,6 +348,17 @@ const user = User.parse(data) If you need end-to-end runtime safety, validate parsed data with a schema library such as Zod or Valibot after the request resolves. +Because successful empty JSON bodies resolve as `undefined`, handle that case +before runtime validation when an endpoint may return no content: + +```ts +const data = await api.get('/users/123') +if (data === undefined) { + throw new Error('Expected a response body') +} +const user = User.parse(data) +``` + ## Behavior notes - Non-2xx responses throw `HttpError`. @@ -339,7 +375,7 @@ If you need end-to-end runtime safety, validate parsed data with a schema librar - `beforeRequest` may override the URL only with a final absolute URL. - `beforeRequest` may mutate headers, but hook option metadata is read-only. - Retry support is opt-in and conservative by default. -- Retry support does not allow streaming request bodies. +- Streaming request bodies are rejected only when the request method is eligible for multiple attempts. - The `json` helper serializes request bodies and sets `Content-Type: application/json` when absent. - `body` and `json` cannot be used together. - TypeScript rejects common invalid option combinations such as `body` plus `json`, and request bodies on `GET`/`HEAD` request shapes. Runtime validation still protects JavaScript callers. @@ -353,6 +389,9 @@ If you need end-to-end runtime safety, validate parsed data with a schema librar - Timeout aborts surface as `TimeoutError`. - External abort reasons are preserved as `AbortRequestError.cause` when the platform exposes them. - Retry backoff waits are abortable. +- Retry attempts reuse a snapshot of the initially normalized URL, headers, retry policy, and request body. JSON bodies are serialized once before the first attempt. +- Retryable `FormData` file values that the current runtime cannot clone safely are rejected instead of being coerced into different payloads. +- Retried `FormData` preserves semantic values but does not guarantee byte-identical multipart boundaries across attempts. - Timeout windows start after `beforeRequest` hooks complete. - Retry backoff waits do not consume per-attempt timeout windows. - If `beforeRequest` replaces `context.url`, that replacement is final. Previously resolved `baseURL` and query parameters are not reapplied to the replacement URL. @@ -373,8 +412,11 @@ clearfetch currently supports: - Node.js `18.x` and newer for package compatibility - modern browsers with native `fetch`, `Request`, `Response`, `Headers`, `URL`, and `AbortController` +- TypeScript `5.0` and newer for the published declaration surface The package is ESM-only and does not target legacy runtimes or polyfill-driven environments. +Features that accept `Blob`, `File`, `FormData`, `URLSearchParams`, or +`ReadableStream` require the corresponding native platform implementation. For security-sensitive use, run clearfetch on a Node.js release line that is still [supported upstream](https://nodejs.org/en/about/previous-releases); EOL Node.js releases do not receive upstream security fixes. @@ -388,12 +430,16 @@ Node.js releases do not receive upstream security fixes. ## Release and CI - CI lints GitHub Actions workflows before merge. -- CI runs lint, test, and build checks on selected supported Node.js versions. +- CI runs lint, test, and build checks across the declared Node.js compatibility matrix, including Node.js `26`. - CI also runs a lightweight browser-like test path using `happy-dom` on Node.js `24`. +- CI runs a focused real-Chromium test for native values created in another browser realm. +- CI verifies the published declaration surface with TypeScript `5.0`. - Dependency review is enforced for pull requests and supports manual base/head validation. +- CI rejects non-registry lockfile sources, missing SHA-512 integrity, and unreviewed install scripts before dependency installation. +- Automated installs disable dependency lifecycle scripts, and a weekly read-only audit checks advisories, registry signatures, and attestations. - The release workflow supports a non-publishing dry-run path via manual dispatch. - npm publishing now uses npm trusted publishing from GitHub Actions instead of a long-lived publish token. -- The release workflow publishes to npm with provenance and creates or verifies the matching GitHub Release record. +- The release workflow publishes the exact smoke-tested tarball with provenance from an OIDC-only job; a separate write-only job creates or verifies the matching GitHub Release record. - Normal releases are expected to publish from GitHub Actions, not from local machines. - Release and repository protection policy is documented in [RELEASE.md](./RELEASE.md). @@ -405,17 +451,24 @@ The public package surface is intentionally narrow: - internal implementation modules are not part of the supported import contract - the deprecated `NormalizedRequestOptions` type remains exported only for compatibility and is planned for removal in the next major version - the package includes no lifecycle scripts and is intended to publish only built `dist/` artifacts +- JavaScript source maps remain available for mapped stack traces; declaration maps are omitted because TypeScript source files are not shipped +- packed and unpacked artifact sizes and file counts are guarded by deliberate budgets ## Development -- `npm install`: install development dependencies +- `npm ci --ignore-scripts --registry=https://registry.npmjs.org`: install locked development dependencies without lifecycle scripts - `npm run build`: compile the package into `dist/` +- `npm run check:lockfile`: validate lockfile origins, integrity, development-only scope, and the reviewed install-script allowlist +- `npm run check:dependency-audit`: fail on moderate-or-higher known dependency advisories +- `npm run check:dependency-signatures`: verify installed-package registry signatures and attestations - `npm run check:package-metadata`: validate publish metadata and zero-runtime-dependency posture - `npm run check:pack-smoke`: smoke-test the packed tarball from a clean temporary install -- `npm run check:publish-dry-run`: dry-run unpublished versions or verify an already-published version came from the current commit; add `-- --allow-existing` only for non-publishing manual workflow validation +- `npm run check:publish-dry-run`: dry-run unpublished workspace versions; pass a retained `.tgz` to compare exact registry integrity for an existing version, or use `-- --allow-existing` only for non-publishing validation - `npm run lint`: run TypeScript static checks - `npm test`: run the test suite - `npm run test:browser-like`: run browser-like package entrypoint coverage with `happy-dom` +- `npm run test:browser-real`: build and run focused cross-realm coverage in Chromium; run `node node_modules/playwright/cli.js install chromium` once before the first local invocation +- `npm run test:types-compat`: build and compile a consumer fixture with the minimum supported TypeScript version ## Status diff --git a/RELEASE.md b/RELEASE.md index a545aaa..1c8a860 100644 --- a/RELEASE.md +++ b/RELEASE.md @@ -6,17 +6,22 @@ Releases should be cut only from `main` after CI is green. Expected flow: -1. Merge reviewed changes into `main`. -2. Confirm `CI` and dependency review checks are passing. -3. Optionally run the `Release` workflow manually to exercise the non-publishing dry-run path. -4. Create an annotated release tag in the form `vX.Y.Z`. -5. Push the tag to GitHub. -6. Let the `Release` GitHub Actions workflow publish the package and create or verify the matching GitHub Release record. -7. Confirm npm and GitHub Releases show the same current version. +1. Prepare the version with `npm version X.Y.Z --no-git-tag-version` and promote the matching changelog entry out of `Unreleased`. +2. Merge reviewed changes into `main`. +3. Confirm `CI` and dependency review checks are passing, including the package guardrails. +4. Optionally run the `Release` workflow manually to exercise the non-publishing dry-run path. +5. Create an annotated release tag in the form `vX.Y.Z`. +6. Push the tag to GitHub. +7. Let the `Release` workflow verify and upload one exact tarball, publish that artifact to npm, and create or verify the matching GitHub Release record. +8. Confirm npm and GitHub Releases show the same current version. Local `npm publish` should not be used for normal releases. -The tag must match the package version exactly, for example package version `1.2.3` must be released from tag `v1.2.3`. If a workflow rerun finds that exact package version already published on npm and the published `gitHead` matches the checked-out tag commit, it skips publishing and still creates or verifies the GitHub Release record. +The tag must match the package version exactly, for example package version +`1.2.3` must be released from tag `v1.2.3`. If a workflow rerun finds that exact +package version already published, it requires npm `dist.integrity` to match the +verified tarball byte-for-byte before it skips publishing and continues to the +GitHub Release record. Before publishing, the workflow also verifies that the release tag is annotated and that its commit is reachable from `origin/main`. @@ -25,6 +30,7 @@ Post-release verification: ```bash npm view @gavoryn/clearfetch version --registry=https://registry.npmjs.org +npm view @gavoryn/clearfetch dist.integrity --registry=https://registry.npmjs.org gh release list --limit 5 --json tagName,name,isDraft,isPrerelease,isLatest,createdAt,publishedAt ``` @@ -35,18 +41,42 @@ GitHub Actions `workflow_dispatch`. That dry-run path should verify: -- install, lint, test, and build steps +- lockfile validation before a lifecycle-script-free install +- lint, test, build, and minimum-TypeScript compatibility steps +- dependency advisory, registry-signature, and attestation checks - package metadata with `npm run check:package-metadata` -- packed artifact behavior with `npm run check:pack-smoke` -- publishability with `npm run check:publish-dry-run -- --allow-existing`, which uses the public npm registry explicitly, dry-runs unpublished versions, and permits manual validation to skip an existing version +- packed artifact behavior with `npm run check:pack-smoke -- --retain` +- exact-artifact publishability with `npm run check:publish-dry-run -- release-artifact/*.tgz`, which uses the public npm registry explicitly, dry-runs unpublished versions, and compares byte integrity for an existing version +- upload of the verified tarball as a short-lived immutable workflow artifact -Tag-triggered verification uses strict `npm run check:publish-dry-run` instead. -For an already-published tag rerun, that mode requires npm `gitHead` to match -the checked-out tag commit before the publish job can continue. +Manual dispatch never runs either privileged job. Tag-triggered publication +downloads the artifact by its immutable artifact ID, verifies its SHA-512 +integrity again, compares the same integrity with npm after publication, and +requires verified SLSA provenance for the expected repository, workflow, tag, +and commit. Use the dry-run path before relying on a first release or after making workflow changes that affect packaging or publishing. +Before committing release preparation, run the local confidence bundle: + +```bash +npm ci --ignore-scripts --no-audit --registry=https://registry.npmjs.org +npm run check:lockfile +npm run lint +npm test +npm run test:browser-like +npm run test:browser-real +npm run test:types-compat +npm run build +npm run check:dependency-audit +npm run check:dependency-signatures +npm run check:package-metadata +npm run check:pack-smoke -- --retain +npm run check:publish-dry-run -- release-artifact/*.tgz +git diff --check +``` + ## Repository protections The repository should enforce the following protections on `main` and any future release-bearing branches: @@ -56,6 +86,13 @@ The repository should enforce the following protections on `main` and any future - require the dependency review workflow to pass for pull requests - block force pushes - block branch deletion +- require full commit SHA pins for GitHub Actions +- restrict Actions to the GitHub-owned actions in use plus the pinned workflow linter +- enable secret push protection; enable validity checks and non-provider patterns when the repository plan supports them +- keep published releases immutable +- keep GitHub private vulnerability reporting enabled +- keep the `npm` environment and npm trusted publisher configuration aligned with `.github/workflows/release.yml` +- restrict the `npm` environment to release tags matching `v*` ## Tag policy @@ -76,6 +113,12 @@ The npm package settings for `@gavoryn/clearfetch` should define a trusted publi - workflow filename: `release.yml` - environment name: `npm` +When npm package administration is available, maintainers should also disable +traditional publish tokens. Staged publishing with a separate 2FA approval is a +strong additional option, but it must be adopted together with a deliberate +workflow change to `npm stage publish`; enabling it only in npm settings would +break the current release path. + ## GitHub Actions configuration The release workflow assumes: @@ -85,7 +128,17 @@ The release workflow assumes: - the npm package has a matching trusted publisher configured on npmjs.com - maintainers review changes to workflow files with the same care as runtime code -The release workflow uses `id-token: write` so npm can exchange the workflow identity for publish access. When trusted publishing is configured, npm also generates provenance automatically for public packages from public repositories. +The release workflow separates authority across three jobs: + +- `verify-release` has read-only repository access, disables dependency lifecycle scripts and caching, runs all package and dependency checks, and uploads the exact smoke-tested tarball +- `publish` has no checkout or development dependencies and receives only `id-token: write`; it downloads, re-verifies, and publishes the exact tarball, then verifies npm signatures and the expected provenance identity +- `github-release` receives only `contents: write` and creates or verifies the GitHub Release after npm publication succeeds + +No job holds both npm publication authority and repository-write authority. +When trusted publishing is configured, npm binds provenance to the public +repository and workflow identity. Because tarball publication may omit npm +`gitHead`, release reruns use `dist.integrity` for byte identity and provenance +for the source-workflow association. ## Runtime and security expectations @@ -97,3 +150,4 @@ The release process must preserve the package’s public claims: - no hidden network behavior beyond the caller's request - package compatibility starting at Node.js `18+`, with security support limited to upstream-supported Node.js release lines - modern browsers with the native web platform APIs documented in `README.md` +- TypeScript `5.0+` for the published declaration surface diff --git a/SECURITY.md b/SECURITY.md index 8ed0aa3..0f970d6 100644 --- a/SECURITY.md +++ b/SECURITY.md @@ -51,3 +51,21 @@ This package is intentionally designed to reduce attack surface: - a narrow public API and explicit runtime support policy These choices reduce risk, but they do not eliminate the need for careful review, secure release practices, and responsible disclosure. + +The development and release path adds several supply-chain controls without +changing the consumer package: + +- every locked package must resolve from the public npm registry with SHA-512 integrity +- CI installs dependencies with lifecycle scripts disabled +- the minimum supported TypeScript compiler is an exact lockfile dependency, not a dynamically downloaded tool +- dependency advisories, registry signatures, and attestations are checked in CI, on a weekly schedule, and before release +- GitHub Actions are pinned to full commit SHAs and checkout credentials are not persisted +- release verification hands one smoke-tested tarball to an OIDC-only publish job and verifies the registry provenance identifies the expected workflow, tag, and commit +- npm publication authority and GitHub Release write authority are held by separate jobs + +These controls limit opportunities to execute or replace unreviewed tooling. +They do not prove that reviewed source is benign: integrity verifies bytes, +signatures verify registry identity, provenance binds a published artifact to a +workflow, and audits cover only known advisories. Maintainers must still review +dependency, lockfile, workflow, and release-policy changes as security-sensitive +code. From e3028cc052ac3287dc34bf02645326ff3f96d22a Mon Sep 17 00:00:00 2001 From: "brian.j.murdock@gmail.com" Date: Thu, 16 Jul 2026 10:27:29 -0500 Subject: [PATCH 07/13] Harden hook execution and request snapshots - separate hook-visible request context from normalized execution state - give response hooks independent clones and abort abandoned attempts - preserve special query keys and cover lifecycle regressions --- DESIGN.md | 3 +- README.md | 12 +-- src/internal/execute-request.ts | 103 ++++++++++++++---------- src/internal/hook-options.ts | 14 ++-- src/internal/normalize-request.ts | 58 ++++++++------ src/internal/query-params.ts | 7 +- src/internal/timeout-controller.ts | 14 ++-- test/hook-options.test.ts | 17 ++++ test/hooks-and-retries.test.ts | 124 +++++++++++++++++++++++++++-- test/normalize-request.test.ts | 54 +++++++++---- test/timeout-controller.test.ts | 17 +++- 11 files changed, 309 insertions(+), 114 deletions(-) diff --git a/DESIGN.md b/DESIGN.md index 639abca..2482c20 100644 --- a/DESIGN.md +++ b/DESIGN.md @@ -714,7 +714,8 @@ Permitted uses: Runs immediately after a response is received and before success parsing is returned to the consumer. -`afterResponse` always receives the raw `Response`, including non-2xx responses that may later be classified as `HttpError`. +Each `afterResponse` hook receives its own clone of the raw `Response`, +including non-2xx responses that may later be classified as `HttpError`. Permitted uses: diff --git a/README.md b/README.md index 66706ba..b57e9ea 100644 --- a/README.md +++ b/README.md @@ -226,9 +226,10 @@ const api = createClient({ `beforeRequest` hook failures, request-normalization failures, retry rebuild failures, and request-construction failures propagate as-is and are observable through `onError` before being re-thrown. Retry-backoff aborts are normalized to -`AbortRequestError`, passed through `onError`, and re-thrown. `afterResponse` -hooks receive a cloned `Response`, so reading the body there does not consume -the response used for normal parsing or `HttpError` creation. +`AbortRequestError`, passed through `onError`, and re-thrown. Each +`afterResponse` hook receives its own cloned `Response`, so reading the body in +one hook does not consume the response used by another hook, normal parsing, or +`HttpError` creation. Hook scope is intentionally narrow: @@ -239,7 +240,8 @@ Hook scope is intentionally narrow: Client hooks run before request hooks. Within each client or request hook list, hooks run in definition order. -Cloned `afterResponse` inspection is intended for ordinary API payloads, not large streaming or heavy binary workflows. +Cloned `afterResponse` inspection is intended for ordinary API payloads, not +large streaming or heavy binary workflows. #### Safe diagnostic header logging @@ -370,7 +372,7 @@ const user = User.parse(data) - No default timeout is applied. Requests run until completion or external abort unless `timeout` is configured. - Invalid request configuration, including invalid hook lists, fails fast with `ConfigError`. - Hook, request-normalization, retry rebuild, and request-construction failures are not wrapped as `NetworkError`. -- `afterResponse` receives a cloned `Response` for safe inspection. +- Each `afterResponse` hook receives an independently readable cloned `Response` for safe inspection. - Relative request inputs require `baseURL`. - `beforeRequest` may override the URL only with a final absolute URL. - `beforeRequest` may mutate headers, but hook option metadata is read-only. diff --git a/src/internal/execute-request.ts b/src/internal/execute-request.ts index 6106097..3fc7ccc 100644 --- a/src/internal/execute-request.ts +++ b/src/internal/execute-request.ts @@ -70,8 +70,8 @@ export async function executeRequest( } const maxAttempts = getEffectiveRetryAttempts( - initialContext._internalOptions.method, - initialContext._internalOptions.retry, + initialContext.normalizedOptions.method, + initialContext.normalizedOptions.retry, ) const contextSnapshot = maxAttempts === 1 ? undefined : snapshotBeforeRequestContext(initialContext) @@ -92,7 +92,7 @@ export async function executeRequest( await runOnErrorHooks({ input, error, - }, initialContext._internalOptions.hooks.onError) + }, initialContext.normalizedOptions.hooks.onError) throw error } } @@ -104,14 +104,14 @@ export async function executeRequest( await runOnErrorHooks({ input, error, - options: context.options, - }, context._internalOptions.hooks.onError) + options: context.hookContext.options, + }, context.normalizedOptions.hooks.onError) throw error } const timeout = createTimeoutController( - context._internalOptions.signal, - context._internalOptions.timeout, + context.normalizedOptions.signal, + context.normalizedOptions.timeout, ) try { @@ -122,8 +122,8 @@ export async function executeRequest( await runOnErrorHooks({ input, error, - options: context.options, - }, context._internalOptions.hooks.onError) + options: context.hookContext.options, + }, context.normalizedOptions.hooks.onError) throw error } @@ -136,23 +136,30 @@ export async function executeRequest( timeout, }) - const afterResponseHooks = context._internalOptions.hooks.afterResponse + const afterResponseHooks = context.normalizedOptions.hooks.afterResponse if (afterResponseHooks.length > 0) { try { await runAfterResponseHooks({ input, request, - response: response.clone(), - options: context.options, + response, + options: context.hookContext.options, }, afterResponseHooks) } catch (error) { - await runOnErrorHooks({ - input, - error, - options: context.options, - request, - response, - }, context._internalOptions.hooks.onError) + try { + await runOnErrorHooks({ + input, + error, + options: context.hookContext.options, + request, + response, + }, context.normalizedOptions.hooks.onError) + } finally { + timeout.abort( + new DOMException('Response body abandoned', 'AbortError'), + ) + cancelResponseBody(response) + } throw error } } @@ -224,8 +231,8 @@ function createMethodCaller( async function runBeforeRequestHooks( context: ExecutionBeforeRequestContext, ): Promise { - for (const hook of context._internalOptions.hooks.beforeRequest) { - await hook(context) + for (const hook of context.normalizedOptions.hooks.beforeRequest) { + await hook(context.hookContext) } } @@ -234,7 +241,18 @@ async function runAfterResponseHooks( hooks: AfterResponseHook[], ): Promise { for (const hook of hooks) { - await hook(context) + const hookResponse = context.response.clone() + try { + await hook({ + ...context, + response: hookResponse, + }) + } finally { + // Cancellation is initiated immediately but cannot be awaited here: + // cloned response bodies share a tee with the original body, so the + // cancellation promise may remain pending until the original settles. + cancelResponseBody(hookResponse) + } } } @@ -270,11 +288,11 @@ async function waitForRetry(params: { context: ExecutionBeforeRequestContext }): Promise { const { attempt, context } = params - const externalSignal = context._internalOptions.signal + const externalSignal = context.normalizedOptions.signal try { await sleep( - getRetryDelay(context._internalOptions.retry, attempt), + getRetryDelay(context.normalizedOptions.retry, attempt), externalSignal, ) } catch (delayError) { @@ -307,7 +325,7 @@ async function waitForRetryWithHandling(params: { const errorContext: ErrorContext = { input, error, - options: context.options, + options: context.hookContext.options, request, } @@ -315,7 +333,7 @@ async function waitForRetryWithHandling(params: { errorContext.response = response } - await runOnErrorHooks(errorContext, context._internalOptions.hooks.onError) + await runOnErrorHooks(errorContext, context.normalizedOptions.hooks.onError) throw error } } @@ -365,12 +383,12 @@ async function fetchWithHandling(params: { return await fetchImpl(request) } catch (error) { const normalized = normalizeExecutionError( - context._internalOptions.timeout !== undefined && timeout.didTimeout() + context.normalizedOptions.timeout !== undefined && timeout.didTimeout() ? { aborted: isRequestAbort(request.signal, error), abortReason: request.signal.reason, error, - timeout: context._internalOptions.timeout, + timeout: context.normalizedOptions.timeout, } : { aborted: isRequestAbort(request.signal, error), @@ -382,8 +400,8 @@ async function fetchWithHandling(params: { if ( shouldRetryError( normalized, - context._internalOptions.method, - context._internalOptions.retry, + context.normalizedOptions.method, + context.normalizedOptions.retry, attempt, ) ) { @@ -399,7 +417,7 @@ async function fetchWithHandling(params: { const errorContext: ErrorContext = { input, error: normalized, - options: context.options, + options: context.hookContext.options, request, } @@ -409,7 +427,7 @@ async function fetchWithHandling(params: { errorContext.response = errorResponse } - await runOnErrorHooks(errorContext, context._internalOptions.hooks.onError) + await runOnErrorHooks(errorContext, context.normalizedOptions.hooks.onError) throw normalized } @@ -429,11 +447,12 @@ async function parseWithHandling(params: { !response.ok && shouldRetryStatus( response, - context._internalOptions.method, - context._internalOptions.retry, + context.normalizedOptions.method, + context.normalizedOptions.retry, attempt, ) ) { + timeout.abort(new DOMException('Response body abandoned', 'AbortError')) cancelResponseBody(response) await waitForRetryWithHandling({ attempt, @@ -455,17 +474,17 @@ async function parseWithHandling(params: { return (await parseResponse({ request, response, - responseType: context._internalOptions.responseType, - parseJson: context._internalOptions.parseJson, + responseType: context.normalizedOptions.responseType, + parseJson: context.normalizedOptions.parseJson, })) as T | Response | string | Blob | ArrayBuffer | undefined } catch (error) { const normalized = normalizeExecutionError( - context._internalOptions.timeout !== undefined && timeout.didTimeout() + context.normalizedOptions.timeout !== undefined && timeout.didTimeout() ? { aborted: isRequestAbort(request.signal, error), abortReason: request.signal.reason, error, - timeout: context._internalOptions.timeout, + timeout: context.normalizedOptions.timeout, } : { aborted: isRequestAbort(request.signal, error), @@ -477,8 +496,8 @@ async function parseWithHandling(params: { if ( shouldRetryError( normalized, - context._internalOptions.method, - context._internalOptions.retry, + context.normalizedOptions.method, + context.normalizedOptions.retry, attempt, ) ) { @@ -495,12 +514,12 @@ async function parseWithHandling(params: { const errorContext: ErrorContext = { input, error: normalized, - options: context.options, + options: context.hookContext.options, request, response: normalized instanceof HttpError ? normalized.response : response, } - await runOnErrorHooks(errorContext, context._internalOptions.hooks.onError) + await runOnErrorHooks(errorContext, context.normalizedOptions.hooks.onError) throw normalized } diff --git a/src/internal/hook-options.ts b/src/internal/hook-options.ts index b16d05c..71e3a3c 100644 --- a/src/internal/hook-options.ts +++ b/src/internal/hook-options.ts @@ -90,12 +90,14 @@ function freezeQueryParams(query: QueryParams): QueryParams { const snapshot: QueryParams = {} for (const [key, value] of Object.entries(query)) { - if (Array.isArray(value)) { - snapshot[key] = Object.freeze([...value]) as PrimitiveQueryValue[] - continue - } - - snapshot[key] = value + Object.defineProperty(snapshot, key, { + configurable: true, + enumerable: true, + value: Array.isArray(value) + ? Object.freeze([...value]) as PrimitiveQueryValue[] + : value, + writable: true, + }) } return Object.freeze(snapshot) diff --git a/src/internal/normalize-request.ts b/src/internal/normalize-request.ts index 120bad7..2d0af2a 100644 --- a/src/internal/normalize-request.ts +++ b/src/internal/normalize-request.ts @@ -37,8 +37,9 @@ const RESPONSE_TYPES = new Set([ const DEFAULT_PARSE_JSON = (text: string): unknown => JSON.parse(text) -export interface ExecutionBeforeRequestContext extends BeforeRequestContext { - _internalOptions: NormalizedRequestOptions +export interface ExecutionBeforeRequestContext { + readonly hookContext: BeforeRequestContext + readonly normalizedOptions: NormalizedRequestOptions } export interface BeforeRequestContextSnapshot { @@ -85,26 +86,27 @@ export function createBeforeRequestContext( export function snapshotBeforeRequestContext( context: ExecutionBeforeRequestContext, ): BeforeRequestContextSnapshot { - const options = cloneNormalizedRequestOptions(context._internalOptions) + const { hookContext } = context + const options = cloneNormalizedRequestOptions(context.normalizedOptions) delete options.json - if (context.body !== undefined) { + if (hookContext.body !== undefined) { options.body = options.hooks.beforeRequest.length === 0 - ? context.body - : snapshotRequestBody(context.body) + ? hookContext.body + : snapshotRequestBody(hookContext.body) } else { delete options.body } const snapshot: BeforeRequestContextSnapshot = { - input: cloneRequestInput(context.input), - url: new URL(context.url), + input: cloneRequestInput(hookContext.input), + url: new URL(hookContext.url), options, } - if (context.options.queryString !== undefined) { - snapshot.queryString = context.options.queryString + if (hookContext.options.queryString !== undefined) { + snapshot.queryString = hookContext.options.queryString } return snapshot @@ -151,18 +153,17 @@ function createExecutionBeforeRequestContext(params: { ...(queryString === '' ? {} : { queryString }), }) - const context: ExecutionBeforeRequestContext = { + const hookContext: BeforeRequestContext = { input, url, headers: normalized.headers, - _internalOptions: normalized, options: optionsView, } if (body !== undefined) { - // `body` remains readable to hooks, but execution uses `_internalOptions` - // so hook metadata cannot silently rewrite normalized behavior. - Object.defineProperty(context, 'body', { + // `body` remains readable to hooks, while execution keeps normalized + // behavior in the separate internal context record. + Object.defineProperty(hookContext, 'body', { configurable: false, enumerable: true, value: body, @@ -170,14 +171,17 @@ function createExecutionBeforeRequestContext(params: { }) } - Object.defineProperty(context, 'options', { + Object.defineProperty(hookContext, 'options', { configurable: false, enumerable: true, value: optionsView, writable: false, }) - return context + return Object.freeze({ + hookContext, + normalizedOptions: normalized, + }) } function cloneNormalizedRequestOptions( @@ -234,29 +238,31 @@ export function buildRequestFromContext( context: ExecutionBeforeRequestContext, signal?: AbortSignal, ): Request { - if (!(context.url instanceof URL)) { + const { hookContext, normalizedOptions } = context + + if (!(hookContext.url instanceof URL)) { throw new ConfigError('beforeRequest URL overrides must be absolute URLs') } const init: RequestInit = { - method: context._internalOptions.method, - headers: context.headers, + method: normalizedOptions.method, + headers: hookContext.headers, } - if (context.body !== undefined) { - init.body = context.body - if (isReadableStream(context.body)) { + if (hookContext.body !== undefined) { + init.body = hookContext.body + if (isReadableStream(hookContext.body)) { Object.assign(init, { duplex: 'half' as const }) } } if (signal !== undefined) { init.signal = signal - } else if (context._internalOptions.signal !== undefined) { - init.signal = context._internalOptions.signal + } else if (normalizedOptions.signal !== undefined) { + init.signal = normalizedOptions.signal } - return new Request(context.url, init) + return new Request(hookContext.url, init) } export function normalizeRequestOptions( diff --git a/src/internal/query-params.ts b/src/internal/query-params.ts index 746f3bd..86519fe 100644 --- a/src/internal/query-params.ts +++ b/src/internal/query-params.ts @@ -13,7 +13,12 @@ export function snapshotQueryInput(query: QueryInput): QueryInput { const snapshot: QueryParams = {} for (const [key, value] of Object.entries(query)) { - snapshot[key] = Array.isArray(value) ? [...value] : value + Object.defineProperty(snapshot, key, { + configurable: true, + enumerable: true, + value: Array.isArray(value) ? [...value] : value, + writable: true, + }) } return snapshot } diff --git a/src/internal/timeout-controller.ts b/src/internal/timeout-controller.ts index a30c46f..5e39cc0 100644 --- a/src/internal/timeout-controller.ts +++ b/src/internal/timeout-controller.ts @@ -1,15 +1,9 @@ export function createTimeoutController(signal?: AbortSignal, timeout?: number): { + abort: (reason?: unknown) => void cleanup: () => void didTimeout: () => boolean - signal?: AbortSignal + signal: AbortSignal } { - if (signal === undefined && timeout === undefined) { - return { - cleanup: () => undefined, - didTimeout: () => false, - } - } - const controller = new AbortController() let timedOut = false let timeoutId: ReturnType | undefined @@ -42,6 +36,10 @@ export function createTimeoutController(signal?: AbortSignal, timeout?: number): return { signal: controller.signal, didTimeout: () => timedOut, + abort: (reason?: unknown) => { + clearTimeoutId() + controller.abort(reason) + }, cleanup: () => { clearTimeoutId() diff --git a/test/hook-options.test.ts b/test/hook-options.test.ts index dfb83af..0724b93 100644 --- a/test/hook-options.test.ts +++ b/test/hook-options.test.ts @@ -139,6 +139,23 @@ test('createHookRequestOptions freezes query metadata and query arrays when quer assert.deepEqual(snapshot.query, query) }) +test('createHookRequestOptions preserves special query keys as own properties', () => { + const query = JSON.parse( + '{"__proto__":["admin","editor"],"constructor":"value"}', + ) as Exclude + + const snapshot = createHookRequestOptions( + createOptions({ query }), + DEFAULT_METADATA, + ) + + assert.equal(Object.getPrototypeOf(snapshot.query), Object.prototype) + assert.equal(Object.hasOwn(snapshot.query ?? {}, '__proto__'), true) + assert.equal(Object.hasOwn(snapshot.query ?? {}, 'constructor'), true) + assert.deepEqual(snapshot.query?.['__proto__'], ['admin', 'editor']) + assert.equal(snapshot.query?.constructor, 'value') +}) + test('createHookRequestOptions omits optional metadata keys when absent', () => { const snapshot = createHookRequestOptions(createOptions(), DEFAULT_METADATA) diff --git a/test/hooks-and-retries.test.ts b/test/hooks-and-retries.test.ts index 4afa4fa..b98a626 100644 --- a/test/hooks-and-retries.test.ts +++ b/test/hooks-and-retries.test.ts @@ -857,20 +857,26 @@ test('retryable HTTP responses do not read body text before retrying', async () ) }) -test('retryable HTTP responses cancel abandoned response bodies', async () => { +test('retryable HTTP responses cancel bodies after observational response hooks', async () => { let attempts = 0 - let cancelCalls = 0 + let abandonedAttempts = 0 - const fetchImpl: typeof fetch = async () => { + const fetchImpl: typeof fetch = async (input) => { attempts += 1 if (attempts === 1) { return new Response(new ReadableStream({ - cancel() { - cancelCalls += 1 - }, start(controller) { controller.enqueue(new TextEncoder().encode('retry')) + const request = input as Request + request.signal.addEventListener( + 'abort', + () => { + abandonedAttempts += 1 + controller.error(request.signal.reason) + }, + { once: true }, + ) }, }), { status: 503, @@ -885,6 +891,9 @@ test('retryable HTTP responses cancel abandoned response bodies', async () => { const result = await request<{ ok: boolean }>( 'https://api.example.com/users', { + hooks: { + afterResponse: [() => undefined], + }, retry: { attempts: 2, backoffMs: 1, @@ -897,7 +906,7 @@ test('retryable HTTP responses cancel abandoned response bodies', async () => { ) assert.deepEqual(result, { ok: true }) - assert.equal(cancelCalls, 1) + assert.equal(abandonedAttempts, 1) }) }) @@ -1268,6 +1277,49 @@ test('afterResponse hook failures propagate without NetworkError wrapping', asyn ) }) +test('afterResponse hook failures cancel the abandoned response body', async () => { + let abandonedAttempts = 0 + + await withMockedFetch( + async (input) => + new Response( + new ReadableStream({ + start(controller) { + controller.enqueue(new TextEncoder().encode('response')) + const request = input as Request + request.signal.addEventListener( + 'abort', + () => { + abandonedAttempts += 1 + controller.error(request.signal.reason) + }, + { once: true }, + ) + }, + }), + ), + async () => { + const client = createClient({ + hooks: { + afterResponse: [ + async () => { + throw new Error('afterResponse failure') + }, + ], + }, + }) + + await assert.rejects( + () => client.get('https://api.example.com/users'), + (error) => + error instanceof Error && error.message === 'afterResponse failure', + ) + + assert.equal(abandonedAttempts, 1) + }, + ) +}) + test('onError observes request construction failures as thrown', async () => { const observedErrors: unknown[] = [] @@ -1386,6 +1438,35 @@ test('afterResponse may read the response body without breaking json parsing', a ) }) +test('afterResponse hooks receive independently readable response bodies', async () => { + const seenBodies: string[] = [] + + await withMockedFetch( + async () => new Response(JSON.stringify({ ok: true })), + async () => { + const client = createClient({ + hooks: { + afterResponse: [ + async (context) => { + seenBodies.push(await context.response.text()) + }, + async (context) => { + seenBodies.push(await context.response.text()) + }, + ], + }, + }) + + const result = await client.get<{ ok: boolean }>( + 'https://api.example.com/users', + ) + + assert.deepEqual(seenBodies, ['{"ok":true}', '{"ok":true}']) + assert.deepEqual(result, { ok: true }) + }, + ) +}) + test('beforeRequest cannot mutate execution options through context.options', async () => { await withMockedFetch( async () => new Response(JSON.stringify({ ok: true })), @@ -1408,6 +1489,35 @@ test('beforeRequest cannot mutate execution options through context.options', as ) }) +test('beforeRequest hook contexts do not expose internal execution state', async () => { + let hasInternalOptions = true + let contextKeys: string[] = [] + + await withMockedFetch( + async () => new Response(JSON.stringify({ ok: true })), + async () => { + const client = createClient({ + hooks: { + beforeRequest: [ + async (context) => { + hasInternalOptions = Object.hasOwn(context, '_internalOptions') + contextKeys = Object.keys(context) + }, + ], + }, + }) + + const result = await client.get<{ ok: boolean }>( + 'https://api.example.com/users', + ) + + assert.equal(hasInternalOptions, false) + assert.equal(contextKeys.includes('_internalOptions'), false) + assert.deepEqual(result, { ok: true }) + }, + ) +}) + test('afterResponse cannot mutate parse behavior through context.options', async () => { await withMockedFetch( async () => new Response(JSON.stringify({ ok: true })), diff --git a/test/normalize-request.test.ts b/test/normalize-request.test.ts index 902b68e..938d3dd 100644 --- a/test/normalize-request.test.ts +++ b/test/normalize-request.test.ts @@ -12,7 +12,8 @@ import { resolveRequestURL, serializeQueryParams, } from '../src/internal/normalize-request.js' -import type { RequestOptions } from '../src/types.js' +import { snapshotQueryInput } from '../src/internal/query-params.js' +import type { QueryParams, RequestOptions } from '../src/types.js' test('serializeQueryParams repeats array keys and skips undefined', () => { const query = serializeQueryParams({ @@ -44,6 +45,21 @@ test('serializeQueryParams accepts URLSearchParams values across realm boundarie ) }) +test('snapshotQueryInput preserves special keys as own data properties', () => { + const query = JSON.parse( + '{"__proto__":["admin","editor"],"constructor":"value"}', + ) as QueryParams + + const snapshot = snapshotQueryInput(query) as QueryParams + + assert.equal(Object.getPrototypeOf(snapshot), Object.prototype) + assert.equal(Object.hasOwn(snapshot, '__proto__'), true) + assert.equal(Object.hasOwn(snapshot, 'constructor'), true) + assert.deepEqual(snapshot['__proto__'], ['admin', 'editor']) + assert.notEqual(snapshot['__proto__'], query['__proto__']) + assert.equal(snapshot.constructor, 'value') +}) + test('resolveRequestURL appends URLSearchParams query input', () => { const query = new URLSearchParams('tag=a&page=1&tag=b') const url = resolveRequestURL( @@ -91,12 +107,15 @@ test('createBeforeRequestContext resolves relative input with baseURL and merges ) assert.equal( - context.url.toString(), + context.hookContext.url.toString(), 'https://api.example.com/users?page=2&tags=design&tags=types', ) - assert.equal(context.headers.get('accept'), 'application/vnd.clearfetch+json') - assert.equal(context.options.method, 'GET') - assert.deepEqual(context.options.query, { + assert.equal( + context.hookContext.headers.get('accept'), + 'application/vnd.clearfetch+json', + ) + assert.equal(context.hookContext.options.method, 'GET') + assert.deepEqual(context.hookContext.options.query, { page: 2, tags: ['design', 'types'], }) @@ -392,7 +411,7 @@ test('createBeforeRequestContext does not snapshot bodies for a single attempt', }, ) - assert.equal(context.body, body) + assert.equal(context.hookContext.body, body) }) test('createBeforeRequestContext does not snapshot bodies for retry-ineligible methods', () => { @@ -410,8 +429,8 @@ test('createBeforeRequestContext does not snapshot bodies for retry-ineligible m }, ) - assert.equal(context.body, body) - assert.equal(context.options.maxAttempts, 1) + assert.equal(context.hookContext.body, body) + assert.equal(context.hookContext.options.maxAttempts, 1) }) test('buildRequestFromContext supports streams for retry-ineligible methods', () => { @@ -428,8 +447,8 @@ test('buildRequestFromContext supports streams for retry-ineligible methods', () }, ) - assert.equal(context.body, body) - assert.equal(context.options.maxAttempts, 1) + assert.equal(context.hookContext.body, body) + assert.equal(context.hookContext.options.maxAttempts, 1) assert.doesNotThrow(() => buildRequestFromContext(context)) }) @@ -450,7 +469,7 @@ test('createBeforeRequestContext snapshots ArrayBuffer bodies across realms', as }, ) - assert.notEqual(context.body, body) + assert.notEqual(context.hookContext.body, body) new Uint8Array(body).set([88, 89, 90]) assert.equal(await buildRequestFromContext(context).text(), 'ABC') }) @@ -476,10 +495,10 @@ test('createBeforeRequestContext snapshots FormData bodies across realms', () => }, ) - assert.notEqual(context.body, body) + assert.notEqual(context.hookContext.body, body) foreignBody.append('value', 'mutated') assert.deepEqual( - [...FormData.prototype.entries.call(context.body as FormData)], + [...FormData.prototype.entries.call(context.hookContext.body as FormData)], [['value', 'original']], ) } finally { @@ -508,7 +527,7 @@ test('createBeforeRequestContext preserves FormData file contents and metadata', }, ) - const file = (context.body as FormData).get('file') + const file = (context.hookContext.body as FormData).get('file') assert.ok(file instanceof Blob) assert.equal((file as Blob & { name?: string }).name, 'example.txt') assert.equal(file.type, 'text/plain') @@ -564,7 +583,10 @@ test('buildRequestFromContext serializes json and sets content-type when absent' const request = buildRequestFromContext(context) assert.equal(request.headers.get('content-type'), 'application/json') - assert.equal(context.body, JSON.stringify({ name: 'Brian' })) + assert.equal( + context.hookContext.body, + JSON.stringify({ name: 'Brian' }), + ) }) test('createBeforeRequestContext rejects json values that serialize to undefined', () => { @@ -627,7 +649,7 @@ test('createBeforeRequestContext wraps json serialization failures as ConfigErro test('buildRequestFromContext rejects invalid hook URL overrides', () => { const context = createBeforeRequestContext('https://api.example.com/users') - ;(context as { url: unknown }).url = '/relative' + ;(context.hookContext as { url: unknown }).url = '/relative' assert.throws( () => buildRequestFromContext(context), diff --git a/test/timeout-controller.test.ts b/test/timeout-controller.test.ts index 45a648f..3d33638 100644 --- a/test/timeout-controller.test.ts +++ b/test/timeout-controller.test.ts @@ -6,14 +6,27 @@ import { sleep, } from '../src/internal/timeout-controller.js' -test('createTimeoutController returns no signal when no abort inputs exist', () => { +test('createTimeoutController creates an attempt signal without user abort inputs', () => { const timeout = createTimeoutController() - assert.equal(timeout.signal, undefined) + assert.equal(timeout.signal.aborted, false) assert.equal(timeout.didTimeout(), false) assert.doesNotThrow(() => timeout.cleanup()) }) +test('createTimeoutController may abort an abandoned attempt', () => { + const timeout = createTimeoutController() + const reason = new DOMException('Response body abandoned', 'AbortError') + + timeout.abort(reason) + + assert.equal(timeout.signal.aborted, true) + assert.equal(timeout.signal.reason, reason) + assert.equal(timeout.didTimeout(), false) + + timeout.cleanup() +}) + test('createTimeoutController propagates external aborts without timeout state', () => { const controller = new AbortController() const timeout = createTimeoutController(controller.signal) From a0ae567faf570e8e07c5bf4c66ca76f6bab7888a Mon Sep 17 00:00:00 2001 From: "brian.j.murdock@gmail.com" Date: Thu, 16 Jul 2026 10:28:36 -0500 Subject: [PATCH 08/13] Isolate browser-only tests from Node compatibility suite - move happy-dom coverage outside the default test glob - keep cross-realm FormData cases in the dedicated browser-like suite --- package.json | 2 +- test/browser-like.browser.ts | 127 +++++++++++++++++++++++++++++++++ test/browser-like.test.ts | 59 --------------- test/normalize-request.test.ts | 68 ------------------ 4 files changed, 128 insertions(+), 128 deletions(-) create mode 100644 test/browser-like.browser.ts delete mode 100644 test/browser-like.test.ts diff --git a/package.json b/package.json index c6b292d..7bed4dc 100644 --- a/package.json +++ b/package.json @@ -28,7 +28,7 @@ "check:publish-dry-run": "node scripts/check-publish-dry-run.mjs", "lint": "node node_modules/typescript/bin/tsc --noEmit -p tsconfig.json", "test": "tsx --test test/*.test.ts", - "test:browser-like": "tsx --test test/browser-like.test.ts", + "test:browser-like": "tsx --test test/browser-like.browser.ts", "test:browser-real": "npm run build && tsx --test test/browser-real.browser.ts", "test:types-compat": "npm run build && node node_modules/typescript-compat/bin/tsc -p test/tsconfig.types-compat.json" }, diff --git a/test/browser-like.browser.ts b/test/browser-like.browser.ts new file mode 100644 index 0000000..2d75945 --- /dev/null +++ b/test/browser-like.browser.ts @@ -0,0 +1,127 @@ +import assert from 'node:assert/strict' +import test from 'node:test' + +import { Window } from 'happy-dom' + +import { ConfigError } from '../src/errors.js' +import { createClient } from '../src/index.js' +import { createBeforeRequestContext } from '../src/internal/normalize-request.js' + +test('public API works in a browser-like environment', async () => { + const window = new Window() + + const originalGlobals = { + AbortController: globalThis.AbortController, + DOMException: globalThis.DOMException, + Headers: globalThis.Headers, + Request: globalThis.Request, + Response: globalThis.Response, + URL: globalThis.URL, + fetch: globalThis.fetch, + } + + Object.assign(globalThis, { + AbortController: window.AbortController, + DOMException: window.DOMException, + Headers: window.Headers, + Request: window.Request, + Response: window.Response, + URL: window.URL, + fetch: async (input: RequestInfo | URL) => { + const request = input as Request + return new window.Response( + JSON.stringify({ + ok: true, + url: request.url, + }), + { + headers: { + 'Content-Type': 'application/json', + }, + }, + ) + }, + }) + + try { + const client = createClient({ + baseURL: 'https://api.example.com', + }) + + const result = await client.get<{ ok: boolean; url: string }>('/users') + + assert.deepEqual(result, { + ok: true, + url: 'https://api.example.com/users', + }) + } finally { + Object.assign(globalThis, originalGlobals) + window.close() + } +}) + +test('retry snapshots FormData bodies created in another realm', () => { + const window = new Window() + + try { + const foreignBody = new window.FormData() + foreignBody.append('value', 'original') + const body = foreignBody as unknown as FormData + + const context = createBeforeRequestContext( + 'https://api.example.com/users', + {}, + { + method: 'POST', + body, + retry: { + attempts: 2, + retryOnMethods: ['POST'], + }, + }, + ) + + assert.notEqual(context.hookContext.body, body) + foreignBody.append('value', 'mutated') + assert.deepEqual( + [...FormData.prototype.entries.call(context.hookContext.body as FormData)], + [['value', 'original']], + ) + } finally { + window.close() + } +}) + +test('retry rejects uncloneable foreign FormData files', () => { + const window = new Window() + + try { + const body = new window.FormData() + body.append( + 'file', + new window.File(['ABC'], 'example.txt', { type: 'text/plain' }), + ) + + assert.throws( + () => + createBeforeRequestContext( + 'https://api.example.com/users', + {}, + { + method: 'POST', + body: body as unknown as FormData, + retry: { + attempts: 2, + retryOnMethods: ['POST'], + }, + }, + ), + (error) => + error instanceof ConfigError && + error.message === + 'Retry is not supported for FormData files that cannot be cloned safely', + ) + } finally { + window.close() + } +}) diff --git a/test/browser-like.test.ts b/test/browser-like.test.ts deleted file mode 100644 index 476b6bf..0000000 --- a/test/browser-like.test.ts +++ /dev/null @@ -1,59 +0,0 @@ -import assert from 'node:assert/strict' -import test from 'node:test' - -import { Window } from 'happy-dom' - -import { createClient } from '../src/index.js' - -test('public API works in a browser-like environment', async () => { - const window = new Window() - - const originalGlobals = { - AbortController: globalThis.AbortController, - DOMException: globalThis.DOMException, - Headers: globalThis.Headers, - Request: globalThis.Request, - Response: globalThis.Response, - URL: globalThis.URL, - fetch: globalThis.fetch, - } - - Object.assign(globalThis, { - AbortController: window.AbortController, - DOMException: window.DOMException, - Headers: window.Headers, - Request: window.Request, - Response: window.Response, - URL: window.URL, - fetch: async (input: RequestInfo | URL) => { - const request = input as Request - return new window.Response( - JSON.stringify({ - ok: true, - url: request.url, - }), - { - headers: { - 'Content-Type': 'application/json', - }, - }, - ) - }, - }) - - try { - const client = createClient({ - baseURL: 'https://api.example.com', - }) - - const result = await client.get<{ ok: boolean; url: string }>('/users') - - assert.deepEqual(result, { - ok: true, - url: 'https://api.example.com/users', - }) - } finally { - Object.assign(globalThis, originalGlobals) - window.close() - } -}) diff --git a/test/normalize-request.test.ts b/test/normalize-request.test.ts index 938d3dd..4414540 100644 --- a/test/normalize-request.test.ts +++ b/test/normalize-request.test.ts @@ -2,8 +2,6 @@ import assert from 'node:assert/strict' import test from 'node:test' import { runInNewContext } from 'node:vm' -import { Window } from 'happy-dom' - import { ConfigError } from '../src/errors.js' import { buildRequestFromContext, @@ -474,38 +472,6 @@ test('createBeforeRequestContext snapshots ArrayBuffer bodies across realms', as assert.equal(await buildRequestFromContext(context).text(), 'ABC') }) -test('createBeforeRequestContext snapshots FormData bodies across realms', () => { - const window = new Window() - - try { - const foreignBody = new window.FormData() - foreignBody.append('value', 'original') - const body = foreignBody as unknown as FormData - - const context = createBeforeRequestContext( - 'https://api.example.com/users', - {}, - { - method: 'POST', - body, - retry: { - attempts: 2, - retryOnMethods: ['POST'], - }, - }, - ) - - assert.notEqual(context.hookContext.body, body) - foreignBody.append('value', 'mutated') - assert.deepEqual( - [...FormData.prototype.entries.call(context.hookContext.body as FormData)], - [['value', 'original']], - ) - } finally { - window.close() - } -}) - test('createBeforeRequestContext preserves FormData file contents and metadata', async () => { const body = new FormData() body.append( @@ -534,40 +500,6 @@ test('createBeforeRequestContext preserves FormData file contents and metadata', assert.equal(await file.text(), 'ABC') }) -test('createBeforeRequestContext rejects uncloneable foreign FormData files', () => { - const window = new Window() - - try { - const body = new window.FormData() - body.append( - 'file', - new window.File(['ABC'], 'example.txt', { type: 'text/plain' }), - ) - - assert.throws( - () => - createBeforeRequestContext( - 'https://api.example.com/users', - {}, - { - method: 'POST', - body: body as unknown as FormData, - retry: { - attempts: 2, - retryOnMethods: ['POST'], - }, - }, - ), - (error) => - error instanceof ConfigError && - error.message === - 'Retry is not supported for FormData files that cannot be cloned safely', - ) - } finally { - window.close() - } -}) - test('buildRequestFromContext serializes json and sets content-type when absent', () => { const context = createBeforeRequestContext( 'https://api.example.com/users', From 1076272487702aceb2b24bb0e0cfe0e2f306a290 Mon Sep 17 00:00:00 2001 From: "brian.j.murdock@gmail.com" Date: Thu, 16 Jul 2026 10:28:50 -0500 Subject: [PATCH 09/13] Document 1.0.7 request hardening --- CHANGELOG.md | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4c8f8e3..1ecac78 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,10 +10,12 @@ the matching `vX.Y.Z` tag publishes to npm and a GitHub Release exists. - clarify that retried multipart bodies preserve values but do not guarantee byte-identical boundary encoding - reflect client-level response type defaults in TypeScript return types, including extended clients, and deprecate the internal `NormalizedRequestOptions` export - route aborts during retry backoff through `onError` before rethrowing -- cancel abandoned response bodies before retrying eligible HTTP failures +- give every `afterResponse` hook an independently readable response clone and cancel abandoned bodies on hook failure or before retry +- keep internal execution options out of the public `beforeRequest` hook context +- preserve special query keys such as `__proto__` as ordinary own properties in retry and hook snapshots - reject invalid query containers and JSON values for which `JSON.stringify` returns `undefined` or throws `TypeError` with `ConfigError` - recognize `URLSearchParams`, `ArrayBuffer`, `FormData`, and request streams across browser realm boundaries -- add real Chromium cross-realm coverage and verify published declarations with TypeScript 5.0 +- isolate browser-only dependencies from the Node.js compatibility suite, add real Chromium cross-realm coverage, and verify published declarations with TypeScript 5.0 - split query serialization and web-platform value handling into focused internal modules - omit declaration maps that reference unshipped source files and enforce packed artifact size budgets - add Node.js 26 compatibility coverage plus workflow concurrency and timeout limits From 62092a1c41564f0a7fc26722a71322b350a0aa83 Mon Sep 17 00:00:00 2001 From: "brian.j.murdock@gmail.com" Date: Wed, 29 Jul 2026 14:27:04 -0500 Subject: [PATCH 10/13] Harden request validation and timeout handling Validate option and default containers, headers, retry fields, and timer bounds. Snapshot query and cross-realm URL inputs, preserve abort classification through response hooks, and cover the behavior across supported Node and browser runtimes. --- CHANGELOG.md | 4 + DESIGN.md | 13 +- README.md | 6 +- src/internal/client-defaults.ts | 106 ++++++++++---- src/internal/execute-request.ts | 78 +++++++++- src/internal/hooks.ts | 16 +- src/internal/normalize-request.ts | 81 ++++++++-- src/internal/query-params.ts | 48 +++--- src/internal/retry-policy.ts | 63 ++++++-- src/internal/timeout-controller.ts | 2 + test/browser-real.browser.ts | 40 +++++ test/client-defaults.test.ts | 119 ++++++++++++++- test/client.test.ts | 25 +++- test/hooks-and-retries.test.ts | 227 +++++++++++++++++++++++++++++ test/normalize-request.test.ts | 131 +++++++++++++++++ test/retry-policy.test.ts | 58 ++++++++ 16 files changed, 912 insertions(+), 105 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1ecac78..4c21443 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,10 @@ the matching `vX.Y.Z` tag publishes to npm and a GitHub Release exists. ## 1.0.7 +- reject timeout and retry-delay values above the maximum reliable platform timer delay and preserve timeout classification through `afterResponse` hooks +- reject invalid option/default containers, headers, and nested retry values with `ConfigError` instead of silently inheriting defaults or leaking platform errors +- accept absolute `URL` replacements created in another browser realm from `beforeRequest` hooks and snapshot cross-realm `baseURL` defaults +- snapshot accessor-backed query values once so request URLs and hook metadata cannot diverge - snapshot retry inputs and serialize JSON once so every eligible attempt replays stable request values without copying bodies for excluded methods - preserve retryable `FormData` file contents and metadata, reject files that cannot be cloned safely, and limit stream rejection to methods with multiple effective attempts - clarify that retried multipart bodies preserve values but do not guarantee byte-identical boundary encoding diff --git a/DESIGN.md b/DESIGN.md index 2482c20..b14d0ba 100644 --- a/DESIGN.md +++ b/DESIGN.md @@ -345,6 +345,9 @@ Client defaults may include: - default hooks - default JSON parser +Mutable default inputs are snapshotted when the client is created. This includes +`URL` values created in another browser realm. + ### Validation rules Validation must occur before the request is executed. @@ -354,10 +357,13 @@ Invalid configurations must fail fast with a configuration error. Examples of invalid configurations include: - both `body` and `json` provided +- invalid option or default containers +- invalid header names or values - negative timeout +- timeout or retry-delay values above the platform timer maximum of `2,147,483,647` milliseconds - malformed base URL - unsupported response type -- invalid retry values +- invalid retry values, including explicit `null` nested fields Strict validation is desirable. Silent coercion should be avoided unless it is trivial and unsurprising. @@ -660,6 +666,7 @@ A timeout means: - the package creates an internal abort controller - the controller aborts once the configured timeout elapses - timeout expiration produces a `TimeoutError` +- after the timer starts, timeout classification remains authoritative through `afterResponse` hooks and response parsing ### External abort model @@ -773,7 +780,7 @@ In particular: - hook metadata exposed through `context.options` is read-only and must not act as a hidden mutation surface - hook metadata includes current attempt counts for application-owned logging and metrics -If a `beforeRequest` hook replaces the URL, the replacement must be a fully resolved absolute URL. Relative replacement URLs are invalid and must fail with `ConfigError`. +If a `beforeRequest` hook replaces the URL, the replacement must be a fully resolved absolute `URL`. URLs from another browser realm are valid; relative replacement URLs are invalid and must fail with `ConfigError`. When a hook replaces the URL, that replacement becomes the final URL for the request and overrides any previously resolved URL, including query parameters. @@ -814,6 +821,8 @@ When enabled, retries should default to safe cases such as: Retry execution must snapshot the initially normalized URL, headers, retry policy, and body before the first attempt. JSON is serialized once so later attempts replay the same payload rather than re-reading caller-owned state. +Query values are likewise read and validated once during request normalization, +and the resulting snapshot supplies both URL serialization and hook metadata. ### Unsafe scenarios diff --git a/README.md b/README.md index b57e9ea..5a3bd35 100644 --- a/README.md +++ b/README.md @@ -370,11 +370,13 @@ const user = User.parse(data) - JSON mode returns `undefined` for empty response bodies. - In JSON mode, successful empty bodies resolve as `T | undefined`. - No default timeout is applied. Requests run until completion or external abort unless `timeout` is configured. +- Timeout and retry-delay values may not exceed `2,147,483,647` milliseconds, the maximum reliable platform timer delay. +- After the timeout window starts, expiration remains authoritative through `afterResponse` hooks and response parsing. - Invalid request configuration, including invalid hook lists, fails fast with `ConfigError`. - Hook, request-normalization, retry rebuild, and request-construction failures are not wrapped as `NetworkError`. - Each `afterResponse` hook receives an independently readable cloned `Response` for safe inspection. - Relative request inputs require `baseURL`. -- `beforeRequest` may override the URL only with a final absolute URL. +- `beforeRequest` may override the URL only with a final absolute `URL`, including a `URL` created in another browser realm. - `beforeRequest` may mutate headers, but hook option metadata is read-only. - Retry support is opt-in and conservative by default. - Streaming request bodies are rejected only when the request method is eligible for multiple attempts. @@ -392,6 +394,7 @@ const user = User.parse(data) - External abort reasons are preserved as `AbortRequestError.cause` when the platform exposes them. - Retry backoff waits are abortable. - Retry attempts reuse a snapshot of the initially normalized URL, headers, retry policy, and request body. JSON bodies are serialized once before the first attempt. +- Client defaults are snapshotted at client creation, including mutable `URL` values created in another browser realm. - Retryable `FormData` file values that the current runtime cannot clone safely are rejected instead of being coerced into different payloads. - Retried `FormData` preserves semantic values but does not guarantee byte-identical multipart boundaries across attempts. - Timeout windows start after `beforeRequest` hooks complete. @@ -399,6 +402,7 @@ const user = User.parse(data) - If `beforeRequest` replaces `context.url`, that replacement is final. Previously resolved `baseURL` and query parameters are not reapplied to the replacement URL. - Hook metadata includes `context.options.attempt` and `context.options.maxAttempts`. Non-retried requests report attempt `1` and max attempts `1`. - When `query` serializes to a non-empty string, hook metadata includes `context.options.queryString` without a leading `?`. Existing search parameters from the input URL remain visible on `context.url`. +- Query values are snapshotted once during normalization so retry and hook metadata do not re-read caller-owned accessors. ## Important limitations by design diff --git a/src/internal/client-defaults.ts b/src/internal/client-defaults.ts index 728117a..b513939 100644 --- a/src/internal/client-defaults.ts +++ b/src/internal/client-defaults.ts @@ -1,3 +1,4 @@ +import { ConfigError } from '../errors.js' import type { ClientDefaults, Hooks, @@ -9,11 +10,13 @@ import { normalizeBeforeRequestHooks, normalizeOnErrorHooks, } from './hooks.js' +import { normalizeRetry } from './retry-policy.js' export function mergeClientDefaults( parent: ClientDefaults, child: ClientDefaults, ): ClientDefaults { + validateNullDefaultValues(child) const merged: ClientDefaults = {} mergeScalarDefaults(merged, parent, child) @@ -23,7 +26,42 @@ export function mergeClientDefaults( return merged } +function validateNullDefaultValues(defaults: ClientDefaults): void { + if ( + typeof defaults !== 'object' || + defaults === null || + Array.isArray(defaults) + ) { + throw new ConfigError('`defaults` must be an object') + } + + const values = defaults as Record + + if (values.baseURL === null) { + throw new ConfigError('`baseURL` must be a string or URL') + } + if (values.headers === null) { + throw new ConfigError('`headers` must not be null') + } + if (values.timeout === null) { + throw new ConfigError('`timeout` must be a non-negative finite number') + } + if (values.responseType === null) { + throw new ConfigError('Unsupported responseType: null') + } + if (values.retry === null) { + throw new ConfigError('`retry` must be false or an object') + } + if (values.hooks === null) { + throw new ConfigError('`hooks` must be an object') + } + if (values.parseJson === null) { + throw new ConfigError('`parseJson` must be a function') + } +} + export function snapshotClientDefaults(defaults: ClientDefaults): ClientDefaults { + validateNullDefaultValues(defaults) const snapshot: ClientDefaults = {} snapshotBaseURL(snapshot, defaults) @@ -72,8 +110,8 @@ function mergeHeaderDefaults( parent: ClientDefaults, child: ClientDefaults, ): void { - const headers = new Headers(parent.headers) - const childHeaders = new Headers(child.headers) + const headers = createHeaders(parent.headers) + const childHeaders = createHeaders(child.headers) for (const [key, value] of childHeaders.entries()) { headers.set(key, value) } @@ -98,9 +136,21 @@ function snapshotBaseURL( snapshot: ClientDefaults, defaults: ClientDefaults, ): void { - if (defaults.baseURL !== undefined) { - snapshot.baseURL = - defaults.baseURL instanceof URL ? new URL(defaults.baseURL) : defaults.baseURL + if (defaults.baseURL === undefined) { + return + } + + if (typeof defaults.baseURL === 'string') { + snapshot.baseURL = defaults.baseURL + return + } + + try { + snapshot.baseURL = new URL( + URL.prototype.toString.call(defaults.baseURL), + ) + } catch (cause) { + throw new ConfigError('`baseURL` must be a string or URL', cause) } } @@ -109,7 +159,7 @@ function snapshotHeaders( defaults: ClientDefaults, ): void { if (defaults.headers !== undefined) { - snapshot.headers = new Headers(defaults.headers) + snapshot.headers = createHeaders(defaults.headers) } } @@ -158,47 +208,39 @@ function snapshotRetry(retry: false | RetryOptions): false | RetryOptions { return false } - const snapshot: RetryOptions = { - ...retry, - } - - copyRetryOnStatuses(snapshot, retry) - copyRetryOnMethods(snapshot, retry) - - return snapshot -} - -function copyRetryOnStatuses( - snapshot: RetryOptions, - retry: RetryOptions, -): void { - if (retry.retryOnStatuses !== undefined) { - snapshot.retryOnStatuses = retry.retryOnStatuses.slice() - } + return normalizeRetry(undefined, retry) } -function copyRetryOnMethods( - snapshot: RetryOptions, - retry: RetryOptions, -): void { - if (retry.retryOnMethods !== undefined) { - snapshot.retryOnMethods = retry.retryOnMethods.slice() +function createHeaders(headers?: HeadersInit): Headers { + try { + return new Headers(headers) + } catch (cause) { + if (cause instanceof TypeError) { + throw new ConfigError( + '`headers` must contain valid header names and values', + cause, + ) + } + throw cause } } function snapshotHooks(hooks: Hooks): Hooks { + const beforeRequest = normalizeBeforeRequestHooks(hooks) + const afterResponse = normalizeAfterResponseHooks(hooks) + const onError = normalizeOnErrorHooks(hooks) const snapshot: Hooks = {} if (hooks.beforeRequest !== undefined) { - snapshot.beforeRequest = normalizeBeforeRequestHooks(hooks) + snapshot.beforeRequest = beforeRequest } if (hooks.afterResponse !== undefined) { - snapshot.afterResponse = normalizeAfterResponseHooks(hooks) + snapshot.afterResponse = afterResponse } if (hooks.onError !== undefined) { - snapshot.onError = normalizeOnErrorHooks(hooks) + snapshot.onError = onError } return snapshot diff --git a/src/internal/execute-request.ts b/src/internal/execute-request.ts index 3fc7ccc..cbca617 100644 --- a/src/internal/execute-request.ts +++ b/src/internal/execute-request.ts @@ -1,7 +1,9 @@ import { + AbortRequestError, ConfigError, HttpClientError, HttpError, + TimeoutError, } from '../errors.js' import type { AfterResponseContext, @@ -146,10 +148,17 @@ export async function executeRequest( options: context.hookContext.options, }, afterResponseHooks) } catch (error) { + const propagatedError = + normalizeAttemptAbort({ + context, + error, + request, + timeout, + }) ?? error try { await runOnErrorHooks({ input, - error, + error: propagatedError, options: context.hookContext.options, request, response, @@ -160,7 +169,33 @@ export async function executeRequest( ) cancelResponseBody(response) } - throw error + throw propagatedError + } + + const abortError = normalizeAttemptAbort({ + context, + error: + request.signal.reason ?? + new DOMException('Request was aborted', 'AbortError'), + request, + timeout, + }) + if (abortError !== undefined) { + try { + await runOnErrorHooks({ + input, + error: abortError, + options: context.hookContext.options, + request, + response, + }, context.normalizedOptions.hooks.onError) + } finally { + timeout.abort( + new DOMException('Response body abandoned', 'AbortError'), + ) + cancelResponseBody(response) + } + throw abortError } } @@ -221,11 +256,16 @@ function createMethodCaller( return ( input: string | URL, options: RequestOptions = {}, - ) => executeRequest( - input, - defaults, - { ...options, method } as RequestOptions, - ) + ) => { + const methodOptions = + typeof options === 'object' && + options !== null && + !Array.isArray(options) + ? { ...options, method } as RequestOptions + : options + + return executeRequest(input, defaults, methodOptions) + } } async function runBeforeRequestHooks( @@ -349,6 +389,30 @@ function isRequestAbort(signal: AbortSignal, error: unknown): boolean { ) } +function normalizeAttemptAbort(params: { + context: ExecutionBeforeRequestContext + error: unknown + request: Request + timeout: ReturnType +}): HttpClientError | undefined { + const { context, error, request, timeout } = params + if (!request.signal.aborted) { + return undefined + } + + if ( + context.normalizedOptions.timeout !== undefined && + timeout.didTimeout() + ) { + return new TimeoutError(context.normalizedOptions.timeout, error) + } + + return new AbortRequestError( + 'Request was aborted', + request.signal.reason !== undefined ? request.signal.reason : error, + ) +} + function isAbortLikeError(error: unknown): boolean { if (error instanceof DOMException && error.name === 'AbortError') { return true diff --git a/src/internal/hooks.ts b/src/internal/hooks.ts index 48a8b43..e49cefa 100644 --- a/src/internal/hooks.ts +++ b/src/internal/hooks.ts @@ -29,18 +29,26 @@ export function mergeHooks( } export function normalizeBeforeRequestHooks(hooks?: Hooks): BeforeRequestHook[] { - return normalizeHookList(hooks?.beforeRequest, 'beforeRequest') as BeforeRequestHook[] + return normalizeHookList(hooks, 'beforeRequest') as BeforeRequestHook[] } export function normalizeAfterResponseHooks(hooks?: Hooks): AfterResponseHook[] { - return normalizeHookList(hooks?.afterResponse, 'afterResponse') as AfterResponseHook[] + return normalizeHookList(hooks, 'afterResponse') as AfterResponseHook[] } export function normalizeOnErrorHooks(hooks?: Hooks): OnErrorHook[] { - return normalizeHookList(hooks?.onError, 'onError') as OnErrorHook[] + return normalizeHookList(hooks, 'onError') as OnErrorHook[] } -function normalizeHookList(value: unknown, key: keyof Hooks): HookList { +function normalizeHookList(hooks: Hooks | undefined, key: keyof Hooks): HookList { + if ( + hooks !== undefined && + (typeof hooks !== 'object' || hooks === null || Array.isArray(hooks)) + ) { + throw new ConfigError('`hooks` must be an object') + } + + const value = hooks?.[key] if (value === undefined) { return [] } diff --git a/src/internal/normalize-request.ts b/src/internal/normalize-request.ts index 2d0af2a..a81137f 100644 --- a/src/internal/normalize-request.ts +++ b/src/internal/normalize-request.ts @@ -19,13 +19,13 @@ import { serializeQueryParams, serializeValidatedQueryParams, snapshotQueryInput, - validateQueryInput, } from './query-params.js' import { getEffectiveRetryAttempts, REQUEST_METHODS, normalizeRetry, } from './retry-policy.js' +import { MAX_TIMER_DELAY_MS } from './timeout-controller.js' const RESPONSE_TYPES = new Set([ 'json', @@ -240,9 +240,7 @@ export function buildRequestFromContext( ): Request { const { hookContext, normalizedOptions } = context - if (!(hookContext.url instanceof URL)) { - throw new ConfigError('beforeRequest URL overrides must be absolute URLs') - } + const requestURL = normalizeBeforeRequestURL(hookContext.url) const init: RequestInit = { method: normalizedOptions.method, @@ -262,22 +260,37 @@ export function buildRequestFromContext( init.signal = normalizedOptions.signal } - return new Request(hookContext.url, init) + return new Request(requestURL, init) } export function normalizeRequestOptions( defaults: ClientDefaults = {}, options: RequestOptions = {}, ): NormalizedRequestOptions { - const method = normalizeMethod(options.method ?? 'GET') - const timeout = normalizeTimeout(options.timeout ?? defaults.timeout) + validateOptionsContainer(defaults, 'defaults') + validateOptionsContainer(options, 'options') + + const method = normalizeMethod( + options.method !== undefined ? options.method : 'GET', + ) + const timeout = normalizeTimeout( + options.timeout !== undefined ? options.timeout : defaults.timeout, + ) const responseType = normalizeResponseType( - options.responseType ?? defaults.responseType ?? 'json', + options.responseType !== undefined + ? options.responseType + : defaults.responseType !== undefined + ? defaults.responseType + : 'json', ) const retry = normalizeRetry(defaults.retry, options.retry) const hooks = mergeHooks(defaults.hooks, options.hooks) const parseJson = normalizeParseJson( - options.parseJson ?? defaults.parseJson ?? DEFAULT_PARSE_JSON, + options.parseJson !== undefined + ? options.parseJson + : defaults.parseJson !== undefined + ? defaults.parseJson + : DEFAULT_PARSE_JSON, ) const headers = mergeHeaders(defaults.headers, options.headers) @@ -308,8 +321,7 @@ export function normalizeRequestOptions( } if (options.query !== undefined) { - validateQueryInput(options.query) - normalized.query = options.query + normalized.query = snapshotQueryInput(options.query) } if (options.body !== undefined) { @@ -361,10 +373,13 @@ function mergeHeaders( defaultHeaders?: HeadersInit, requestHeaders?: HeadersInit, ): Headers { - const headers = new Headers(defaultHeaders) + const headers = createHeaders(defaultHeaders) if (requestHeaders !== undefined) { - const overrideHeaders = new Headers(requestHeaders) + if (requestHeaders === null) { + throw new ConfigError('`headers` must not be null') + } + const overrideHeaders = createHeaders(requestHeaders) for (const [key, value] of overrideHeaders.entries()) { headers.set(key, value) @@ -374,6 +389,29 @@ function mergeHeaders( return headers } +function createHeaders(headers?: HeadersInit): Headers { + try { + return new Headers(headers) + } catch (cause) { + if (cause instanceof TypeError) { + throw new ConfigError( + '`headers` must contain valid header names and values', + cause, + ) + } + throw cause + } +} + +function validateOptionsContainer( + value: unknown, + name: 'defaults' | 'options', +): void { + if (typeof value !== 'object' || value === null || Array.isArray(value)) { + throw new ConfigError(`\`${name}\` must be an object`) + } +} + function normalizeMethod(method: unknown): RequestMethod { if (typeof method !== 'string') { throw new ConfigError('`method` must be a string') @@ -391,9 +429,26 @@ function normalizeTimeout(timeout?: number): number | undefined { throw new ConfigError('`timeout` must be a non-negative finite number') } + if (timeout > MAX_TIMER_DELAY_MS) { + throw new ConfigError( + `\`timeout\` must be no greater than ${MAX_TIMER_DELAY_MS}`, + ) + } + return timeout } +function normalizeBeforeRequestURL(value: unknown): URL { + try { + return new URL(URL.prototype.toString.call(value as URL)) + } catch (cause) { + throw new ConfigError( + 'beforeRequest URL overrides must be absolute URLs', + cause, + ) + } +} + function normalizeResponseType(responseType: unknown): NormalizedRequestOptions['responseType'] { if ( typeof responseType !== 'string' || diff --git a/src/internal/query-params.ts b/src/internal/query-params.ts index 86519fe..3e9a204 100644 --- a/src/internal/query-params.ts +++ b/src/internal/query-params.ts @@ -6,17 +6,21 @@ import type { } from '../types.js' import { isURLSearchParams } from './platform-values.js' -export function snapshotQueryInput(query: QueryInput): QueryInput { +export function snapshotQueryInput(query: unknown): QueryInput { if (isURLSearchParams(query)) { return new URLSearchParams(URLSearchParams.prototype.toString.call(query)) } + if (!isQueryParamsRecord(query)) { + throw new ConfigError('`query` must be a record or URLSearchParams') + } + const snapshot: QueryParams = {} for (const [key, value] of Object.entries(query)) { Object.defineProperty(snapshot, key, { configurable: true, enumerable: true, - value: Array.isArray(value) ? [...value] : value, + value: snapshotQueryValue(key, value), writable: true, }) } @@ -28,8 +32,7 @@ export function serializeQueryParams(query?: QueryInput): string { return '' } - validateQueryInput(query) - return serializeValidatedQueryParams(query) + return serializeValidatedQueryParams(snapshotQueryInput(query)) } export function serializeValidatedQueryParams(query?: QueryInput): string { @@ -70,20 +73,6 @@ export function applyQueryString(url: URL, queryString: string): void { url.search += suffix } -export function validateQueryInput( - query: unknown, -): asserts query is QueryInput { - if (isURLSearchParams(query)) { - return - } - - if (!isQueryParamsRecord(query)) { - throw new ConfigError('`query` must be a record or URLSearchParams') - } - - validateQueryParams(query) -} - function serializeScalarQueryValue(value: PrimitiveQueryValue): string { if (value === null) { return 'null' @@ -92,12 +81,6 @@ function serializeScalarQueryValue(value: PrimitiveQueryValue): string { return String(value) } -function validateQueryParams(query: QueryParams): void { - for (const [key, value] of Object.entries(query)) { - validateQueryValue(key, value) - } -} - function isQueryParamsRecord(value: unknown): value is QueryParams { if (typeof value !== 'object' || value === null || Array.isArray(value)) { return false @@ -110,22 +93,31 @@ function isQueryParamsRecord(value: unknown): value is QueryParams { } } -function validateQueryValue(key: string, value: QueryParams[string]): void { +function snapshotQueryValue( + key: string, + value: unknown, +): QueryParams[string] { if (value === undefined) { - return + return undefined } if (Array.isArray(value)) { + const snapshot: PrimitiveQueryValue[] = [] for (const item of value) { validateQueryScalarValue(key, item) + snapshot.push(item) } - return + return snapshot } validateQueryScalarValue(key, value) + return value } -function validateQueryScalarValue(key: string, value: unknown): void { +function validateQueryScalarValue( + key: string, + value: unknown, +): asserts value is PrimitiveQueryValue { if ( value === null || typeof value === 'string' || diff --git a/src/internal/retry-policy.ts b/src/internal/retry-policy.ts index 7db7c78..26d119b 100644 --- a/src/internal/retry-policy.ts +++ b/src/internal/retry-policy.ts @@ -1,6 +1,7 @@ import { ConfigError, HttpError, NetworkError } from '../errors.js' import type { HttpClientError } from '../errors.js' import type { RequestMethod, RetryOptions } from '../types.js' +import { MAX_TIMER_DELAY_MS } from './timeout-controller.js' export const REQUEST_METHODS = new Set([ 'GET', @@ -32,8 +33,6 @@ export function normalizeRetry( const retry = buildRetry(source) validateRetryNumbers(retry) - validateRetryOnStatuses(retry.retryOnStatuses) - validateRetryOnMethods(retry.retryOnMethods) return retry } @@ -42,30 +41,50 @@ function selectRetrySource( defaultRetry?: false | RetryOptions, requestRetry?: false | RetryOptions, ): false | RetryOptions { - if (requestRetry === false) { + const source = requestRetry !== undefined ? requestRetry : defaultRetry + if (source === undefined || source === false) { return false } - const source = requestRetry ?? defaultRetry - if (source === undefined || source === false) { - return false + if (typeof source !== 'object' || source === null || Array.isArray(source)) { + throw new ConfigError('`retry` must be false or an object') } return source } function buildRetry(source: RetryOptions): Required { + const retryOnStatuses = + source.retryOnStatuses === undefined + ? DEFAULT_RETRY.retryOnStatuses + : source.retryOnStatuses + const retryOnMethods = + source.retryOnMethods === undefined + ? DEFAULT_RETRY.retryOnMethods + : source.retryOnMethods + + validateRetryOnStatuses(retryOnStatuses) + validateRetryOnMethods(retryOnMethods) + return { - attempts: source.attempts ?? DEFAULT_RETRY.attempts, - backoffMs: source.backoffMs ?? DEFAULT_RETRY.backoffMs, - maxBackoffMs: source.maxBackoffMs ?? DEFAULT_RETRY.maxBackoffMs, - multiplier: source.multiplier ?? DEFAULT_RETRY.multiplier, - retryOnStatuses: [ - ...(source.retryOnStatuses ?? DEFAULT_RETRY.retryOnStatuses), - ], - retryOnMethods: [ - ...(source.retryOnMethods ?? DEFAULT_RETRY.retryOnMethods), - ], + attempts: + source.attempts === undefined + ? DEFAULT_RETRY.attempts + : source.attempts, + backoffMs: + source.backoffMs === undefined + ? DEFAULT_RETRY.backoffMs + : source.backoffMs, + maxBackoffMs: + source.maxBackoffMs === undefined + ? DEFAULT_RETRY.maxBackoffMs + : source.maxBackoffMs, + multiplier: + source.multiplier === undefined + ? DEFAULT_RETRY.multiplier + : source.multiplier, + retryOnStatuses: [...retryOnStatuses], + retryOnMethods: [...retryOnMethods], } } @@ -78,12 +97,24 @@ function validateRetryNumbers(retry: Required): void { throw new ConfigError('`retry.backoffMs` must be a non-negative finite number') } + if (retry.backoffMs > MAX_TIMER_DELAY_MS) { + throw new ConfigError( + `\`retry.backoffMs\` must be no greater than ${MAX_TIMER_DELAY_MS}`, + ) + } + if (!Number.isFinite(retry.maxBackoffMs) || retry.maxBackoffMs < 0) { throw new ConfigError( '`retry.maxBackoffMs` must be a non-negative finite number', ) } + if (retry.maxBackoffMs > MAX_TIMER_DELAY_MS) { + throw new ConfigError( + `\`retry.maxBackoffMs\` must be no greater than ${MAX_TIMER_DELAY_MS}`, + ) + } + if (!Number.isFinite(retry.multiplier) || retry.multiplier < 1) { throw new ConfigError('`retry.multiplier` must be a finite number >= 1') } diff --git a/src/internal/timeout-controller.ts b/src/internal/timeout-controller.ts index 5e39cc0..45f9296 100644 --- a/src/internal/timeout-controller.ts +++ b/src/internal/timeout-controller.ts @@ -1,3 +1,5 @@ +export const MAX_TIMER_DELAY_MS = 2_147_483_647 + export function createTimeoutController(signal?: AbortSignal, timeout?: number): { abort: (reason?: unknown) => void cleanup: () => void diff --git a/test/browser-real.browser.ts b/test/browser-real.browser.ts index f921bf2..cf633da 100644 --- a/test/browser-real.browser.ts +++ b/test/browser-real.browser.ts @@ -7,6 +7,8 @@ import { fileURLToPath } from 'node:url' import { chromium } from 'playwright' +import type { BeforeRequestContext } from '../src/types.js' + const rootDir = path.resolve(path.dirname(fileURLToPath(import.meta.url)), '..') const distDir = path.join(rootDir, 'dist') @@ -50,6 +52,8 @@ test('real browser handles native values created in another realm', { let result: { arrayBufferResult: { attempts: number; bodies: number[][] } + crossRealmBaseURLResult: { pathname: string } + crossRealmURLResult: { search: string } formDataResult: { attempts: number; bodies: string[]; contentTypes: string[] } queryResult: { search: string } streamResult: { body: string } @@ -87,6 +91,26 @@ test('real browser handles native values created in another realm', { const query = new foreign.URLSearchParams('tag=admin&page=1&tag=editor') const queryResult = await client.get('/query', { query }) + const crossRealmURLClient = createClient({ + hooks: { + beforeRequest: [ + (context: BeforeRequestContext) => { + context.url = new foreign.URL(`${origin}/query?source=foreign`) + }, + ], + }, + }) + const crossRealmURLResult = await crossRealmURLClient.get( + `${origin}/query?source=original`, + ) + + const foreignBaseURL = new foreign.URL(`${origin}/base-original/`) + const crossRealmBaseURLClient = createClient({ + baseURL: foreignBaseURL, + }) + foreignBaseURL.pathname = '/base-mutated/' + const crossRealmBaseURLResult = await crossRealmBaseURLClient.get('query') + const bytes = new foreign.ArrayBuffer(4) new foreign.Uint8Array(bytes).set([1, 2, 3, 4]) const arrayBufferResult = await client.post('/array-buffer', { @@ -123,6 +147,8 @@ test('real browser handles native values created in another realm', { iframe.remove() return { arrayBufferResult, + crossRealmBaseURLResult, + crossRealmURLResult, formDataResult, queryResult, streamResult, @@ -138,6 +164,12 @@ test('real browser handles native values created in another realm', { assert.deepEqual(result.queryResult, { search: '?tag=admin&page=1&tag=editor', }) + assert.deepEqual(result.crossRealmURLResult, { + search: '?source=foreign', + }) + assert.deepEqual(result.crossRealmBaseURLResult, { + pathname: '/base-original/query', + }) assert.deepEqual(result.arrayBufferResult, { attempts: 2, bodies: [[1, 2, 3, 4], [1, 2, 3, 4]], @@ -187,6 +219,14 @@ async function handleRequest( return } + if ( + url.pathname === '/base-original/query' || + url.pathname === '/base-mutated/query' + ) { + sendJson(response, { pathname: url.pathname }) + return + } + const body = await readRequestBody(request) const bodies = requestBodies.get(url.pathname) ?? [] bodies.push(body) diff --git a/test/client-defaults.test.ts b/test/client-defaults.test.ts index dbabd51..21a98a1 100644 --- a/test/client-defaults.test.ts +++ b/test/client-defaults.test.ts @@ -94,7 +94,14 @@ test('snapshotClientDefaults preserves property insertion order', () => { ]) }) -test('snapshotClientDefaults rejects invalid hook defaults', () => { +test('snapshotClientDefaults rejects invalid defaults', () => { + assert.throws( + () => snapshotClientDefaults(null as never), + (error) => + error instanceof ConfigError && + error.message === '`defaults` must be an object', + ) + assert.throws( () => snapshotClientDefaults({ @@ -118,6 +125,67 @@ test('snapshotClientDefaults rejects invalid hook defaults', () => { error instanceof ConfigError && error.message === '`hooks.onError` must be an array of functions', ) + + assert.throws( + () => + snapshotClientDefaults({ + hooks: null, + } as never), + (error) => + error instanceof ConfigError && error.message === '`hooks` must be an object', + ) + + assert.throws( + () => + snapshotClientDefaults({ + retry: null, + } as never), + (error) => + error instanceof ConfigError && + error.message === '`retry` must be false or an object', + ) + + for (const { defaults, message } of [ + { + defaults: { baseURL: null }, + message: '`baseURL` must be a string or URL', + }, + { + defaults: { headers: null }, + message: '`headers` must not be null', + }, + ]) { + assert.throws( + () => snapshotClientDefaults(defaults as never), + (error) => error instanceof ConfigError && error.message === message, + ) + } + + assert.throws( + () => + snapshotClientDefaults({ + headers: { + 'bad header': 'value', + }, + }), + (error) => + error instanceof ConfigError && + error.message === '`headers` must contain valid header names and values' && + error.cause instanceof TypeError, + ) + + assert.throws( + () => + snapshotClientDefaults({ + retry: { + retryOnStatuses: null, + }, + } as never), + (error) => + error instanceof ConfigError && + error.message === + '`retry.retryOnStatuses` must be an array of status codes', + ) }) test('mergeClientDefaults lets child scalar defaults override parent defaults', () => { @@ -145,6 +213,55 @@ test('mergeClientDefaults lets child scalar defaults override parent defaults', assert.deepEqual(merged.parseJson?.('ok'), { text: 'ok' }) }) +test('mergeClientDefaults rejects null child overrides', () => { + const parent = { + baseURL: 'https://parent.example.com', + headers: { accept: 'application/json' }, + timeout: 1_000, + responseType: 'text' as const, + retry: { attempts: 2 }, + hooks: { beforeRequest: [() => undefined] }, + parseJson: () => 42, + } + const cases = [ + { + child: { baseURL: null }, + message: '`baseURL` must be a string or URL', + }, + { + child: { headers: null }, + message: '`headers` must not be null', + }, + { + child: { timeout: null }, + message: '`timeout` must be a non-negative finite number', + }, + { + child: { responseType: null }, + message: 'Unsupported responseType: null', + }, + { + child: { retry: null }, + message: '`retry` must be false or an object', + }, + { + child: { hooks: null }, + message: '`hooks` must be an object', + }, + { + child: { parseJson: null }, + message: '`parseJson` must be a function', + }, + ] + + for (const { child, message } of cases) { + assert.throws( + () => mergeClientDefaults(parent, child as never), + (error) => error instanceof ConfigError && error.message === message, + ) + } +}) + test('mergeClientDefaults merges headers and appends hooks parent then child', () => { const parentBefore: BeforeRequestHook = () => {} const childBefore: BeforeRequestHook = () => {} diff --git a/test/client.test.ts b/test/client.test.ts index 0630827..b4dd6f2 100644 --- a/test/client.test.ts +++ b/test/client.test.ts @@ -1,7 +1,7 @@ import assert from 'node:assert/strict' import test from 'node:test' -import { HttpError, NetworkError } from '../src/errors.js' +import { ConfigError, HttpError, NetworkError } from '../src/errors.js' import { createClient } from '../src/index.js' import { request } from '../src/request.js' @@ -22,6 +22,29 @@ test('request parses json responses', async () => { } }) +test('public request surfaces reject invalid option containers', async () => { + const client = createClient() + + await assert.rejects( + () => request('https://api.example.com/users', null as never), + (error) => + error instanceof ConfigError && + error.message === '`options` must be an object', + ) + await assert.rejects( + () => client.request('https://api.example.com/users', null as never), + (error) => + error instanceof ConfigError && + error.message === '`options` must be an object', + ) + await assert.rejects( + () => client.get('https://api.example.com/users', null as never), + (error) => + error instanceof ConfigError && + error.message === '`options` must be an object', + ) +}) + test('createClient resolves baseURL and extend merges headers', async () => { const originalFetch = globalThis.fetch const requests: Request[] = [] diff --git a/test/hooks-and-retries.test.ts b/test/hooks-and-retries.test.ts index b98a626..24ad810 100644 --- a/test/hooks-and-retries.test.ts +++ b/test/hooks-and-retries.test.ts @@ -184,6 +184,233 @@ test('timeout starts after beforeRequest hooks complete', async () => { } }) +test('timeout expiration during afterResponse hooks surfaces TimeoutError', async () => { + const originalFetch = globalThis.fetch + let observedError: unknown + + globalThis.fetch = async () => + new Response(JSON.stringify({ ok: true })) + + try { + await assert.rejects( + () => + request('https://api.example.com/users', { + timeout: 5, + hooks: { + afterResponse: [ + async () => { + await new Promise((resolve) => setTimeout(resolve, 25)) + }, + ], + onError: [ + (context) => { + observedError = context.error + }, + ], + }, + }), + (error) => error instanceof TimeoutError && error.timeout === 5, + ) + + assert.ok(observedError instanceof TimeoutError) + } finally { + globalThis.fetch = originalFetch + } +}) + +test('timeout aborts from afterResponse body reads are normalized', async () => { + const originalFetch = globalThis.fetch + let observedError: unknown + + globalThis.fetch = async (input) => { + const request = input as Request + const body = new ReadableStream({ + start(controller) { + request.signal.addEventListener( + 'abort', + () => controller.error(new DOMException('Aborted', 'AbortError')), + { once: true }, + ) + }, + }) + return new Response(body) + } + + try { + await assert.rejects( + () => + request('https://api.example.com/users', { + timeout: 5, + hooks: { + afterResponse: [ + async (context) => { + await context.response.text() + }, + ], + onError: [ + (context) => { + observedError = context.error + }, + ], + }, + }), + (error) => error instanceof TimeoutError && error.timeout === 5, + ) + + assert.ok(observedError instanceof TimeoutError) + } finally { + globalThis.fetch = originalFetch + } +}) + +test('external aborts during afterResponse hooks stay AbortRequestError', async () => { + const originalFetch = globalThis.fetch + const controller = new AbortController() + const reason = new Error('stop response inspection') + let observedError: unknown + + globalThis.fetch = async (input) => { + const request = input as Request + const body = new ReadableStream({ + start(streamController) { + request.signal.addEventListener( + 'abort', + () => streamController.error(request.signal.reason), + { once: true }, + ) + }, + }) + return new Response(body) + } + + const abortId = setTimeout(() => controller.abort(reason), 5) + try { + await assert.rejects( + () => + request('https://api.example.com/users', { + signal: controller.signal, + hooks: { + afterResponse: [ + async (context) => { + await context.response.text() + }, + ], + onError: [ + (context) => { + observedError = context.error + }, + ], + }, + }), + (error) => + error instanceof AbortRequestError && + error.cause === reason, + ) + + assert.ok(observedError instanceof AbortRequestError) + assert.equal(observedError.cause, reason) + } finally { + clearTimeout(abortId) + globalThis.fetch = originalFetch + } +}) + +test('timeout classification overrides clearfetch errors thrown by afterResponse hooks', async () => { + const originalFetch = globalThis.fetch + const hookError = new ConfigError('late hook failure') + + globalThis.fetch = async () => + new Response(JSON.stringify({ ok: true })) + + try { + await assert.rejects( + () => + request('https://api.example.com/users', { + timeout: 5, + hooks: { + afterResponse: [ + async () => { + await new Promise((resolve) => setTimeout(resolve, 25)) + throw hookError + }, + ], + }, + }), + (error) => + error instanceof TimeoutError && + error.timeout === 5 && + error.cause === hookError, + ) + } finally { + globalThis.fetch = originalFetch + } +}) + +test('external abort classification overrides clearfetch errors thrown by afterResponse hooks', async () => { + const originalFetch = globalThis.fetch + const controller = new AbortController() + const reason = new Error('stop response inspection') + + globalThis.fetch = async () => + new Response(JSON.stringify({ ok: true })) + + const abortId = setTimeout(() => controller.abort(reason), 5) + try { + await assert.rejects( + () => + request('https://api.example.com/users', { + signal: controller.signal, + hooks: { + afterResponse: [ + async () => { + await new Promise((resolve) => setTimeout(resolve, 25)) + throw new ConfigError('late hook failure') + }, + ], + }, + }), + (error) => + error instanceof AbortRequestError && + error.cause === reason, + ) + } finally { + clearTimeout(abortId) + globalThis.fetch = originalFetch + } +}) + +test('afterResponse abort classification preserves an explicit null reason', async () => { + const originalFetch = globalThis.fetch + const controller = new AbortController() + + globalThis.fetch = async () => + new Response(JSON.stringify({ ok: true })) + + const abortId = setTimeout(() => controller.abort(null), 5) + try { + await assert.rejects( + () => + request('https://api.example.com/users', { + signal: controller.signal, + hooks: { + afterResponse: [ + async () => { + await new Promise((resolve) => setTimeout(resolve, 25)) + throw new ConfigError('late hook failure') + }, + ], + }, + }), + (error) => + error instanceof AbortRequestError && + error.cause === null, + ) + } finally { + clearTimeout(abortId) + globalThis.fetch = originalFetch + } +}) + test('external abort surfaces AbortRequestError', async () => { const originalFetch = globalThis.fetch globalThis.fetch = async (input) => diff --git a/test/normalize-request.test.ts b/test/normalize-request.test.ts index 4414540..bb6096f 100644 --- a/test/normalize-request.test.ts +++ b/test/normalize-request.test.ts @@ -58,6 +58,55 @@ test('snapshotQueryInput preserves special keys as own data properties', () => { assert.equal(snapshot.constructor, 'value') }) +test('createBeforeRequestContext reads accessor-backed query values once', () => { + let reads = 0 + const query = { + get token() { + reads += 1 + return reads === 1 ? 'stable' : `changed-${reads}` + }, + } + + const context = createBeforeRequestContext( + 'https://api.example.com/users', + {}, + { query }, + ) + + assert.equal(reads, 1) + assert.equal( + context.hookContext.url.toString(), + 'https://api.example.com/users?token=stable', + ) + assert.deepEqual(context.hookContext.options.query, { token: 'stable' }) +}) + +test('createBeforeRequestContext reads accessor-backed query array values once', () => { + let reads = 0 + const values: unknown[] = [] + Object.defineProperty(values, 0, { + enumerable: true, + get() { + reads += 1 + return reads === 1 ? 'stable' : { invalid: true } + }, + }) + values.length = 1 + + const context = createBeforeRequestContext( + 'https://api.example.com/users', + {}, + { query: { token: values as never } }, + ) + + assert.equal(reads, 1) + assert.equal( + context.hookContext.url.toString(), + 'https://api.example.com/users?token=stable', + ) + assert.deepEqual(context.hookContext.options.query, { token: ['stable'] }) +}) + test('resolveRequestURL appends URLSearchParams query input', () => { const query = new URLSearchParams('tag=a&page=1&tag=b') const url = resolveRequestURL( @@ -196,6 +245,88 @@ test('normalizeRequestOptions rejects non-function parseJson values', () => { ) }) +test('normalizeRequestOptions rejects null request overrides', () => { + const defaults = { + timeout: 5_000, + responseType: 'text' as const, + retry: { attempts: 2 }, + hooks: { beforeRequest: [() => undefined] }, + parseJson: () => 42, + } + const cases = [ + { + options: { headers: null }, + message: '`headers` must not be null', + }, + { + options: { timeout: null }, + message: '`timeout` must be a non-negative finite number', + }, + { + options: { responseType: null }, + message: 'Unsupported responseType: null', + }, + { + options: { retry: null }, + message: '`retry` must be false or an object', + }, + { + options: { hooks: null }, + message: '`hooks` must be an object', + }, + { + options: { parseJson: null }, + message: '`parseJson` must be a function', + }, + ] + + for (const { message, options } of cases) { + assert.throws( + () => normalizeRequestOptions(defaults, options as never), + (error) => error instanceof ConfigError && error.message === message, + ) + } +}) + +test('normalizeRequestOptions rejects invalid option containers and headers', () => { + assert.throws( + () => normalizeRequestOptions({}, null as never), + (error) => + error instanceof ConfigError && + error.message === '`options` must be an object', + ) + + assert.throws( + () => + normalizeRequestOptions({}, { + headers: { + 'bad header': 'value', + }, + }), + (error) => + error instanceof ConfigError && + error.message === '`headers` must contain valid header names and values' && + error.cause instanceof TypeError, + ) +}) + +test('normalizeRequestOptions rejects timeout values above the timer limit', () => { + assert.equal( + normalizeRequestOptions({}, { timeout: 2_147_483_647 }).timeout, + 2_147_483_647, + ) + + assert.throws( + () => + normalizeRequestOptions({}, { + timeout: 2_147_483_648, + }), + (error) => + error instanceof ConfigError && + error.message === '`timeout` must be no greater than 2147483647', + ) +}) + test('normalizeRequestOptions rejects invalid hook configuration', () => { assert.throws( () => diff --git a/test/retry-policy.test.ts b/test/retry-policy.test.ts index 0441f50..ce1e9b0 100644 --- a/test/retry-policy.test.ts +++ b/test/retry-policy.test.ts @@ -59,6 +59,64 @@ test('normalizeRetry rejects invalid attempts with existing message', () => { ) }) +test('normalizeRetry rejects delays above the timer limit', () => { + const maximumDelay = normalizeRetry(undefined, { + backoffMs: 2_147_483_647, + maxBackoffMs: 2_147_483_647, + }) + assert.ok(maximumDelay !== false) + assert.equal(maximumDelay.backoffMs, 2_147_483_647) + assert.equal(maximumDelay.maxBackoffMs, 2_147_483_647) + + for (const retry of [ + { backoffMs: 2_147_483_648 }, + { maxBackoffMs: 2_147_483_648 }, + ]) { + assert.throws( + () => normalizeRetry(undefined, retry), + (error) => + error instanceof ConfigError && + error.message.endsWith('must be no greater than 2147483647'), + ) + } +}) + +test('normalizeRetry rejects invalid nested retry field types', () => { + const cases = [ + { + retry: { attempts: null }, + message: '`retry.attempts` must be a positive integer', + }, + { + retry: { backoffMs: null }, + message: '`retry.backoffMs` must be a non-negative finite number', + }, + { + retry: { maxBackoffMs: null }, + message: '`retry.maxBackoffMs` must be a non-negative finite number', + }, + { + retry: { multiplier: null }, + message: '`retry.multiplier` must be a finite number >= 1', + }, + { + retry: { retryOnStatuses: null }, + message: '`retry.retryOnStatuses` must be an array of status codes', + }, + { + retry: { retryOnMethods: {} }, + message: '`retry.retryOnMethods` must be an array of methods', + }, + ] + + for (const { message, retry } of cases) { + assert.throws( + () => normalizeRetry(undefined, retry as never), + (error) => error instanceof ConfigError && error.message === message, + ) + } +}) + test('normalizeRetry rejects invalid methods with existing message', () => { assert.throws( () => normalizeRetry(undefined, { retryOnMethods: ['post'] as never }), From 2f7bfcdb1cc0d5236696e1dce7c963600529df08 Mon Sep 17 00:00:00 2001 From: "brian.j.murdock@gmail.com" Date: Wed, 29 Jul 2026 15:08:26 -0500 Subject: [PATCH 11/13] Rename TypeScript alias to unblock dependency review --- package-lock.json | 4 ++-- package.json | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/package-lock.json b/package-lock.json index 6fa2c61..878c7e2 100644 --- a/package-lock.json +++ b/package-lock.json @@ -14,7 +14,7 @@ "playwright": "1.61.1", "tsx": "^4.20.5", "typescript": "^5.9.2", - "typescript-compat": "npm:typescript@5.0.4" + "typescript-v5": "npm:typescript@5.0.4" }, "engines": { "node": ">=18" @@ -657,7 +657,7 @@ "node": ">=14.17" } }, - "node_modules/typescript-compat": { + "node_modules/typescript-v5": { "name": "typescript", "version": "5.0.4", "resolved": "https://registry.npmjs.org/typescript/-/typescript-5.0.4.tgz", diff --git a/package.json b/package.json index 7bed4dc..9c4ff97 100644 --- a/package.json +++ b/package.json @@ -30,7 +30,7 @@ "test": "tsx --test test/*.test.ts", "test:browser-like": "tsx --test test/browser-like.browser.ts", "test:browser-real": "npm run build && tsx --test test/browser-real.browser.ts", - "test:types-compat": "npm run build && node node_modules/typescript-compat/bin/tsc -p test/tsconfig.types-compat.json" + "test:types-compat": "npm run build && node node_modules/typescript-v5/bin/tsc -p test/tsconfig.types-compat.json" }, "keywords": [ "fetch", @@ -55,7 +55,7 @@ "playwright": "1.61.1", "tsx": "^4.20.5", "typescript": "^5.9.2", - "typescript-compat": "npm:typescript@5.0.4" + "typescript-v5": "npm:typescript@5.0.4" }, "overrides": { "esbuild": "0.28.1", From d8501360b5e163f9fef9a2bfa5d1cd21c359139d Mon Sep 17 00:00:00 2001 From: "brian.j.murdock@gmail.com" Date: Wed, 29 Jul 2026 16:00:16 -0500 Subject: [PATCH 12/13] Preserve HttpClient assignment compatibility - retain response-type inference for configured and extended clients - cover legacy HttpClient annotations on TypeScript 5.0 and current types --- src/client.ts | 6 ++++-- src/types.ts | 4 ++-- test/type-compatibility.ts | 4 ++++ test/type-signatures.ts | 8 ++++++-- 4 files changed, 16 insertions(+), 6 deletions(-) diff --git a/src/client.ts b/src/client.ts index 2af17a5..8e82c2a 100644 --- a/src/client.ts +++ b/src/client.ts @@ -11,7 +11,7 @@ export function createClient( defaults: Omit & { responseType: DefaultResponseType }, -): HttpClient +): HttpClient & HttpClient export function createClient( defaults?: Omit & { @@ -19,7 +19,9 @@ export function createClient( }, ): HttpClient<'json'> -export function createClient(defaults: ClientDefaults): HttpClient +export function createClient( + defaults: ClientDefaults, +): HttpClient & HttpClient export function createClient( defaults: ClientDefaults = {}, diff --git a/src/types.ts b/src/types.ts index 37de513..944b363 100644 --- a/src/types.ts +++ b/src/types.ts @@ -476,7 +476,7 @@ export interface HttpClient { defaults: Omit & { responseType: ChildResponseType }, - ): HttpClient + ): HttpClient & HttpClient extend( defaults: Omit & { @@ -484,7 +484,7 @@ export interface HttpClient { }, ): HttpClient - extend(defaults: ClientDefaults): HttpClient + extend(defaults: ClientDefaults): HttpClient & HttpClient } type ResponseResult = diff --git a/test/type-compatibility.ts b/test/type-compatibility.ts index a6764bd..bbad774 100644 --- a/test/type-compatibility.ts +++ b/test/type-compatibility.ts @@ -12,11 +12,13 @@ const textClient = createClient({ responseType: 'text' }) const textResult: Promise = textClient.get( 'https://api.example.com/status', ) +const legacyTextClient: HttpClient = textClient const rawClient = textClient.extend({ responseType: 'raw' }) const rawResult: Promise = rawClient.get( 'https://api.example.com/status', ) +const legacyRawClient: HttpClient = rawClient const requestResult: Promise = request( 'https://api.example.com/data', @@ -25,5 +27,7 @@ const requestResult: Promise = request( void jsonResult void textResult +void legacyTextClient void rawResult +void legacyRawClient void requestResult diff --git a/test/type-signatures.ts b/test/type-signatures.ts index 3c0c181..652a594 100644 --- a/test/type-signatures.ts +++ b/test/type-signatures.ts @@ -58,9 +58,11 @@ const clientJsonPromise: Promise<{ ok: boolean } | undefined> = client.get<{ void clientJsonPromise const textDefaultClient = createClient({ responseType: 'text' }) +const legacyTextDefaultClient: HttpClient = textDefaultClient const defaultTextPromise: Promise = textDefaultClient.get( 'https://api.example.com/text', ) +void legacyTextDefaultClient void defaultTextPromise const rawDefaultClient = createClient({ responseType: 'raw' }) @@ -91,22 +93,24 @@ const inheritedTextPromise: Promise = inheritedTextDefault.get( void inheritedTextPromise const extendedRawDefault = textDefaultClient.extend({ responseType: 'raw' }) +const legacyRawExtendedClient: HttpClient = extendedRawDefault const extendedRawPromise: Promise = extendedRawDefault.get( 'https://api.example.com/raw', ) +void legacyRawExtendedClient void extendedRawPromise const dynamicDefaults: ClientDefaults = { responseType: 'text' } const dynamicDefaultClient = createClient(dynamicDefaults) type DynamicDefaultClient = Expect< - Equal> + Equal & HttpClient> > void (undefined as unknown as DynamicDefaultClient) const dynamicExtendedDefaults: ClientDefaults = { responseType: 'raw' } const dynamicExtendedClient = textDefaultClient.extend(dynamicExtendedDefaults) type DynamicExtendedClient = Expect< - Equal> + Equal & HttpClient> > void (undefined as unknown as DynamicExtendedClient) From 048a322e4b73d3759c964711db64b4bd2902bbe6 Mon Sep 17 00:00:00 2001 From: "brian.j.murdock@gmail.com" Date: Wed, 29 Jul 2026 16:00:24 -0500 Subject: [PATCH 13/13] Hide stack traces in browser test server --- test/browser-real.browser.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/test/browser-real.browser.ts b/test/browser-real.browser.ts index cf633da..f825fc9 100644 --- a/test/browser-real.browser.ts +++ b/test/browser-real.browser.ts @@ -28,8 +28,9 @@ test('real browser handles native values created in another realm', { requestContentTypes, ) } catch (error) { + console.error(error) response.writeHead(500, { 'Content-Type': 'text/plain' }) - response.end(error instanceof Error ? error.stack : String(error)) + response.end('Internal test server error') } })