Merge pull request #12 from openfetch-js/features/security-url-redaction-cache-warn

Features/security url redaction cache warn

Author: hamdymohamedak

CONTRIBUTING.md

@@ -1,82 +1,69 @@
-# Contributing to @hamdymohamedak/openfetch
+# Contributing on GitHub
 
-Thank you for taking the time to improve this project. The following guidelines keep reviews predictable and the package stable across runtimes.
+This document describes how to participate in **@hamdymohamedak/openfetch** through GitHub: issues, discussions, and pull requests.
 
-## Ground rules
+## What we look for in changes
+
+These rules keep reviews predictable and the library safe across runtimes:
 
 1. **Stay on `fetch`.** Do not add XMLHttpRequest, alternate fetch shims as required dependencies, or polyfills that assume a browser.
 2. **Avoid browser-only globals.** Do not reference `window`, `document`, `localStorage`, `sessionStorage`, `WebSocket`, or `EventSource` in library code.
 3. **Keep the public API small.** Prefer new behavior as optional middleware or clearly documented config rather than breaking existing callers.
-4. **TypeScript source of truth.** All implementation lives under `src/`; `dist/` is compiled output from `npm run build`.
-
-## How to contribute
+4. **TypeScript source of truth.** Implementation lives under `src/`; `dist/` is build output from `npm run build`.
 
-### Reporting issues
+## Reporting issues
 
-Open an issue with:
+[Open an issue](https://github.com/openfetch-js/OpenFetch/issues) on this repository with:
 
 - Runtime and version (Node, Bun, Deno, worker, browser).
 - Minimal code sample and expected vs actual behavior.
 - If relevant, the target URL shape (without secrets).
 
-### Suggesting features
+## Suggesting features
 
-Describe the use case and whether it can live in userland (middleware) vs core. Large features should be discussed in an issue before a heavy pull request.
+Open an issue first. Describe the use case and whether it can live in userland (middleware) vs core. Large features should be agreed in an issue before a large pull request.
 
-### Pull requests
+## Pull requests
 
-#### Branch naming
+### Branch naming
 
-Use **one branch per concern**: a single feature **or** a single bug fix, not unrelated changes in the same branch.
+Use **one branch per concern**: one feature **or** one bug fix, not unrelated changes together.
 
-- **Features:** `features/<short-description>` — for example `features/add-retry`, `features/cache-key-override`.
-- **Bug fixes:** `bugs/<short-description>` — for example `bugs/timeout-signal`, `bugs/cache-key-collision`.
+- **Features:** `features/<short-description>` — e.g. `features/add-retry`, `features/cache-key-override`.
+- **Bug fixes:** `bugs/<short-description>` — e.g. `bugs/timeout-signal`, `bugs/cache-key-collision`.
 
 Use **kebab-case** after the prefix. Keep the slug short but specific enough that reviewers can tell what the branch is for.
 
-1. **Fork** the repository and create a branch from `main` following the [branch naming](#branch-naming) rules above.
-2. **Implement** your change in `src/`. Match existing formatting and naming.
-3. **Build** locally: `npm run build` (must pass with no TypeScript errors).
+### Workflow
+
+1. **Fork** this repository and create a branch from `main` using the [branch naming](#branch-naming) rules above.
+2. **Implement** in `src/`. Match existing formatting and naming.
+3. **Build** locally: `npm run build` (no TypeScript errors).
 4. **Document** user-visible behavior in `README.md` and, if structural, in `docs/PROJECT_FLOW.md`.
-5. **Open a PR** into `main` with:
+5. **Open a pull request** into `main` with:
    - A clear title and description.
-   - What changed and why (motivation).
+   - What changed and why.
    - Any breaking changes called out explicitly.
 
-### Commit messages
-
-Use short, imperative subjects (for example `Add cache key override option`). Optional body for context. Consistent history helps maintainers and consumers.
-
-### Code review
-
-Maintainers may request tests, naming tweaks, or doc updates. Keeping changes scoped to one concern per PR speeds up merge.
+### Before you push
 
-## Local development
+From a clone of your fork:
 
 ```bash
 npm install
 npm run build
 npm run test:security
 ```
 
-`npm run test:security` runs the checks under `security-tests/`. `tsc` remains the compile gate for every change.
+`npm run test:security` runs `security-tests/`. The TypeScript compile step is the gate for every change.
 
-## Publishing
-
-Publishing to npm is reserved for maintainers after version bump and changelog review. Consumers should install from the registry or from a tagged release, not from unreviewed branches.
-
-npm **requires two-factor authentication** (or a granular access token with publish rights) for `npm publish`. Logging in with the browser (`npm login`) is not enough by itself.
+### Commit messages
 
-1. On [npmjs.com](https://www.npmjs.com/) go to **Account → Two-Factor Authentication** and enable **Authorization and writes** (authenticator app recommended).
-2. From the package root:
+Use short, imperative subjects (e.g. `Add cache key override option`). Add a body when extra context helps reviewers reading the PR on GitHub.
 
-```bash
-npm publish --access public --otp=123456
-```
-
-Use the current 6-digit code from your authenticator app. Do not commit or share OTPs.
+### Code review
 
-**Alternative:** create a **Granular Access Token** at npm with permission to publish this package (and “bypass 2FA” only if your org policy allows it), then configure npm to use that token for `registry.npmjs.org`.
+Maintainers may ask for tests, naming tweaks, or doc updates on the PR. Smaller, single-concern PRs are easier to review and merge.
 
 ## Code of conduct
 

README.md

@@ -85,7 +85,7 @@ Register **`retry` before `timeout`** so retries wrap the full inner stack. Use
 
 **Retry timing:** `retry.timeoutTotalMs` measures elapsed time with a monotonic clock (`performance.now()` when available), so the budget is not skewed by system clock changes. By default (`retry.enforceTotalTimeout !== false`), each attempt merges a deadline into the request `signal` so an in-flight `fetch` aborts when the budget runs out (`ERR_RETRY_TIMEOUT`). Set `retry.enforceTotalTimeout: false` to enforce the budget only between attempts. `retry.timeoutPerAttemptMs` sets `timeout` for every attempt inside the retry middleware. Each `dispatch` uses `clearTimeout` in a `finally` block so per-attempt timers are not left dangling.
 
-**Debug:** Default logs omit request headers. Use `debug({ includeRequestHeaders: true, maskHeaders: ["authorization"], maskStrategy: "partial" })` for values like `Bearer ****abcd`, or `maskStrategy: "hash"` for a short fingerprint. **`maskHeaderValues`** supports the same strategies when building your own logs.
+**Debug:** Default logs omit request headers. Logged URLs **redact common sensitive query parameters** (`token`, `code`, `password`, …); set `maskUrlQuery: false` to log raw URLs (avoid in production). Use `debug({ includeRequestHeaders: true, maskHeaders: ["authorization"], maskStrategy: "partial" })` for values like `Bearer ****abcd`, or `maskStrategy: "hash"` for a short fingerprint. **`maskHeaderValues`** supports the same strategies when building your own logs.
 
 ### Execution model
 
@@ -102,7 +102,7 @@ Understanding order helps avoid surprises with retries, timeouts, and escape hat
 
 ### Memory cache and authentication
 
-The default cache key is ``METHOD fullUrl``. For **authenticated or per-user** GETs, also pass header names that affect the response so entries do not leak across users:
+The default cache key is ``METHOD fullUrl``. The first request with **`Authorization` or `Cookie`** and no `varyHeaderNames` / custom `key` triggers a **one-time `console.warn`** (suppress with `suppressAuthCacheKeyWarning: true` if you only cache public data). For **authenticated or per-user** GETs, also pass header names that affect the response so entries do not leak across users:
 
 ```ts
 createCacheMiddleware(store, {
@@ -129,7 +129,7 @@ For URLs influenced by untrusted input, call `assertSafeHttpUrl(url)` before req
 
 ### Errors and logging
 
-`OpenFetchError.toShape()` omits `config.auth` but may still include **response `data` and `headers`**. For client-facing or shared logs, use `toShape({ includeResponseData: false, includeResponseHeaders: false })`. The error instance itself can still hold full `config`; do not expose it raw.
+`OpenFetchError.toShape()` omits `config.auth` and by default **redacts sensitive query parameters** in the `url` field; pass `redactSensitiveUrlQuery: false` only for trusted diagnostics. It may still include **response `data` and `headers`**. For client-facing or shared logs, use `toShape({ includeResponseData: false, includeResponseHeaders: false })`. The error instance itself can still hold full `config`; do not expose it raw.
 
 ## Documentation
 

SECURITY.md

@@ -7,7 +7,7 @@ openfetch is a thin `fetch` wrapper. Callers supply URLs, headers, and bodies. T
 - **Axios-class proxy CVEs (e.g. CVE-2025-62718 / `NO_PROXY` normalization)** — openfetch does **not** implement axios-style `HTTP_PROXY` / `HTTPS_PROXY` / `NO_PROXY` matching. Outbound routing follows the host runtime’s `fetch` (and any platform proxy). Those CVEs therefore do **not** map to openfetch code paths; policy still belongs at the app, proxy, or mesh layer.
 
 - **Network trust** — You choose endpoints. Blocking private IPs, metadata hosts, or open redirects is an **application** concern for partially trusted URLs.
-- **Secrets** — `toShape()` on `OpenFetchError` avoids echoing `config.auth`, but the full `Error` object may still carry `config` (including credentials). Response bodies and headers in `toShape()` may still contain tokens or PII; use `toShape({ includeResponseData: false, includeResponseHeaders: false })` when serializing for untrusted clients or broad logs. Never send raw errors to untrusted clients without redaction.
+- **Secrets** — `toShape()` on `OpenFetchError` avoids echoing `config.auth`, but the full `Error` object may still carry `config` (including credentials). Response bodies and headers in `toShape()` may still contain tokens or PII; use `toShape({ includeResponseData: false, includeResponseHeaders: false })` when serializing for untrusted clients or broad logs. By default, `toShape()` also **redacts common sensitive query parameters** in the serialized `url` (for example `token`, `code`, `password`); use `redactSensitiveUrlQuery: false` only for trusted diagnostics. The `debug()` plugin applies the same redaction to logged URLs. Never send raw errors to untrusted clients without redaction.
 - **Supply chain** — Install this package from npm or a verified Git tag; verify integrity with your package manager.
 
 ## Server-side usage and SSRF
@@ -34,6 +34,8 @@ Mitigations (combine as appropriate):
 
 Unauthenticated, fully public GETs may keep the default key.
 
+The middleware emits a **one-time `console.warn`** the first time it sees `Authorization` or `Cookie` on a cacheable request while `varyHeaderNames` is empty and no custom `key` is set. Suppress with `suppressAuthCacheKeyWarning: true` when you know the cache is safe (for example anonymous-only endpoints).
+
 ## Retry and non-idempotent methods
 
 By default, `createRetryMiddleware` retries network/parse failures and configured HTTP error statuses **only** for `GET`, `HEAD`, `OPTIONS`, and `TRACE`, to reduce duplicate side effects (for example double charges on `POST`).

dist/core/cache.d.ts

@@ -34,6 +34,13 @@ export type CacheMiddlewareOptions = {
      * Use for authenticated or personalized GETs, e.g. `["authorization", "cookie"]`, so entries do not leak across users.
      */
     varyHeaderNames?: string[];
+    /**
+     * When false (default), the first cached GET/HEAD that includes `Authorization` or `Cookie`
+     * without `varyHeaderNames` or a custom `key` triggers a one-time `console.warn` about
+     * possible cross-user cache leakage. Set true if you intentionally cache anonymous responses
+     * only or use another isolation mechanism.
+     */
+    suppressAuthCacheKeyWarning?: boolean;
 };
 /** Append a stable suffix from header values so cache keys differ per auth/cookie (etc.). */
 export declare function appendCacheKeyVaryHeaders(baseKey: string, headers: Record<string, string> | undefined, headerNames: string[]): string;

dist/core/cache.d.ts.map

@@ -33,6 +33,16 @@ function shallowCloneResponse(r) {
         data: r.data,
     };
 }
+function headersHaveAuthOrCookie(headers) {
+    if (!headers)
+        return false;
+    for (const k of Object.keys(headers)) {
+        const l = k.toLowerCase();
+        if (l === "authorization" || l === "cookie")
+            return true;
+    }
+    return false;
+}
 /** Append a stable suffix from header values so cache keys differ per auth/cookie (etc.). */
 export function appendCacheKeyVaryHeaders(baseKey, headers, headerNames) {
     if (!headerNames.length)
@@ -87,6 +97,7 @@ export function createCacheMiddleware(store, options) {
     const varyHeaderNames = options?.varyHeaderNames ?? [];
     const methods = new Set((options?.methods ?? ["GET", "HEAD"]).map((m) => m.toUpperCase()));
     const inflight = new Map();
+    let authCacheKeyWarningIssued = false;
     return async (ctx, next) => {
         if (ctx.request.memoryCache?.skip) {
             await next();
@@ -100,6 +111,16 @@ export function createCacheMiddleware(store, options) {
         const urlString = buildURL(ctx.request.url, ctx.request);
         const rawKey = options?.key?.({ request: ctx.request, url: urlString }) ??
             `${method} ${urlString}`;
+        if (!authCacheKeyWarningIssued &&
+            options?.suppressAuthCacheKeyWarning !== true &&
+            options?.key === undefined &&
+            varyHeaderNames.length === 0 &&
+            headersHaveAuthOrCookie(ctx.request.headers)) {
+            authCacheKeyWarningIssued = true;
+            if (typeof console !== "undefined" && typeof console.warn === "function") {
+                console.warn("[openfetch] createCacheMiddleware: request uses Authorization or Cookie but varyHeaderNames is empty and no custom key is set; cache entries may be shared across users. Use varyHeaderNames: [\"authorization\", \"cookie\"] or options.key, or set suppressAuthCacheKeyWarning: true if this is intentional.");
+            }
+        }
         const key = appendCacheKeyVaryHeaders(rawKey, ctx.request.headers, varyHeaderNames);
         const ttlMs = ctx.request.memoryCache?.ttlMs ?? defaultTtl;
         const swrMs = ctx.request.memoryCache?.staleWhileRevalidateMs ?? defaultSwr;

dist/core/cache.js

@@ -23,6 +23,13 @@ export type OpenFetchErrorToShapeOptions = {
      * Default true (backward compatible).
      */
     includeResponseHeaders?: boolean;
+    /**
+     * When true (default), replaces sensitive query parameter values in the serialized `url`
+     * (e.g. `token`, `code`, `api_key`). Set false only for trusted internal diagnostics.
+     */
+    redactSensitiveUrlQuery?: boolean;
+    /** Extra query parameter names to redact (case-insensitive); merged with the built-in list. */
+    sensitiveQueryParamNames?: string[];
 };
 export declare class OpenFetchError<T = unknown> extends Error {
     config?: OpenFetchConfig;