feat: close Mission Squad paid-agent gaps from the expansion spec by j4ys0n · Pull Request #5 · MissionSquad/x402-proxy

j4ys0n · 2026-07-09T19:47:56Z

Summary

A full audit of docs/x402-proxy-expansion-spec.md against main showed the spec ~90% implemented by the earlier expand PRs (resource model, stores, dynamic runtime, route patterns, streaming leases, WS alignment, diagnostics, discovery, SSRF controls, Express 4/5 metadata). This PR closes the remaining gaps so the proxy fully supports Mission Squad paid agents while staying product-agnostic.

Changes

`service-token` access mode is now functional

X402AccessMode was declared in the types but never applied — the proxy never injected the configured service credential. Now access: { mode: "service-token", serviceTokenHeader, serviceTokenValue } injects the header on the upstream request (buffered and streaming paths), replacing any client-supplied value for the same header. Validation requires both fields, rejects payment/hop-by-hop/host/content-length header names, and error messages never include the token value. The value is not exposed via diagnostics, discovery, or logs.

Header policy: exclude lists + spec-exact presets

New excludeRequestHeaders / excludeResponseHeaders on X402HeaderPolicy — the spec's "extend presets with explicit include/exclude lists" was only half-implemented (include only). Excludes apply to headers copied from the inbound request/upstream response; explicit add*Headers are unaffected.
api-auth and browser-auth are trimmed to the spec-exact lists. Behavioral change: x-webhook-secret, accept-language, x-request-id, idempotency-key are no longer forwarded by preset — opt in per resource via forwardRequestHeaders. This narrows the default forwarding surface to least-privilege.
HeaderPolicy is now an alias of X402HeaderPolicy (identical shapes previously maintained in parallel).

Bug fix: streaming client-disconnect propagation

The spec requires client disconnects to abort the upstream request via AbortController, and lists it as an integration scenario — writing that test exposed a real bug: the streaming proxy only listened on req.on("close"), which does not fire on a real mid-response client abort in Node 20 (existing unit tests emitted close synthetically). Upstream SSE requests were never aborted and would run to completion. Fixed by also listening on the response close event guarded by writableEnded, waking backpressure waits on it, and swallowing the resulting AbortError only when the client is gone.

Validation: placeholder position rule

upstreamUrl placeholders were validated against publicPath across the whole string, but interpolation only substitutes full path segments — a placeholder in a partial segment or the query string validated and then went upstream literally. Validation now rejects any placeholder occurrence that does not occupy a full path segment (HTTP kinds; WebSocket upstreams are connected verbatim).

Tests (197 passing, was 187)

Discovery generation for http-stream/websocket resources emits lease paths, never raw stream/WS paths or upstream URLs (spec §9 test gap).
Integration: expired stream lease rejected end-to-end; client disconnect aborts the upstream streaming request; service-token injection replaces client credentials; excludeRequestHeaders drops cookie from browser-auth.
Unit: exclude lists, spec-exact presets, service-token injection guards, placeholder-position validation (no token leakage in messages).

Notes for reviewers

The streaming response preset still lists connection per the spec, but the hop-by-hop filter strips it — documented in the README as spec parity; forwarding connection would be incorrect.
MongoDB resource-store adapter intentionally remains out of this package per the spec (interface defined; Mission Squad implements its own adapter).

Verification

yarn typecheck (src + tests), yarn build, yarn test — 21 files, 197 tests, all green.

🤖 Generated with Claude Code

Implements the remaining items from docs/x402-proxy-expansion-spec.md: - service-token access mode: inject access.serviceTokenHeader/Value on upstream requests (replacing client credentials), with validation that rejects payment/hop-by-hop header names and never leaks the token value - header policy excludeRequestHeaders/excludeResponseHeaders so consumers can drop preset-granted headers (e.g. cookie from browser-auth) - trim api-auth/browser-auth presets to the spec-exact lists; extras like x-webhook-secret, x-request-id, idempotency-key now require explicit forwardRequestHeaders opt-in - reject upstreamUrl placeholders that do not occupy a full path segment (previously they validated but were never interpolated) - fix streaming client-disconnect propagation: listen on the response "close" event (guarded by writableEnded); req "close" alone does not fire on a real mid-response abort in Node 20, so the upstream request was never aborted - tests: discovery generation for http-stream/websocket lease paths, expired-lease rejection end-to-end, client-disconnect abort propagation, service-token + exclude-list integration coverage Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

Copilot

Pull request overview

This PR completes remaining gaps for “paid agent” support by implementing functional service-token access mode, adding header-policy exclude lists (and tightening presets to spec-exact allowlists), improving streaming client-disconnect propagation, and strengthening upstream placeholder validation; it also adds unit/integration coverage for these behaviors.

Changes:

Implement service-token access injection for buffered + streaming HTTP proxy paths, with validation and non-leaking errors.
Extend header policy with excludeRequestHeaders / excludeResponseHeaders, and trim api-auth / browser-auth presets to spec-exact lists.
Fix streaming disconnect handling and add validation rejecting upstream placeholders outside full path segments; add new unit + integration tests.

Reviewed changes

Copilot reviewed 12 out of 12 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
tests/unit/resourceStore.test.ts	Adds unit coverage for placeholder-position validation and service-token validation (no token leakage).
tests/unit/headerPolicy.test.ts	Updates preset expectations; adds tests for exclude lists and service-token header injection behavior.
tests/unit/discovery.test.ts	Ensures discovery output emits only lease endpoints for stream/WS resources (no upstream URLs/raw paths).
tests/integration/paid-agent-hardening.integration.test.ts	Adds end-to-end hardening scenarios: lease expiry, client disconnect abort, service-token replacement, header excludes.
src/types.ts	Unifies `HeaderPolicy` with `X402HeaderPolicy`; introduces `X402ResourceAccess` and wires it into `X402Resource`.
src/routePattern.ts	Adds `findMisplacedUpstreamPlaceholders` to detect placeholders that interpolation will never substitute.
src/resourceStore.ts	Adds access-mode validation and placeholder-position validation for HTTP resources.
src/resourceRuntime.ts	Propagates `access` into the runtime target passed to proxy handlers.
src/index.ts	Exports `X402ResourceAccess`, `applyServiceTokenAccess`, and route placeholder helpers.
src/httpProxy.ts	Applies service-token injection to outbound upstream requests; improves streaming abort on client disconnect.
src/headerPolicy.ts	Adds exclude-list support, tightens preset allowlists, and implements `applyServiceTokenAccess`.
README.md	Documents updated presets, exclude lists, and upstream access modes.

Comments suppressed due to low confidence (1)

README.md:196

The README now documents excludeResponseHeaders, but this paragraph still says responses "always" forward the default header set. Since applyUpstreamResponseHeaders applies excludeResponseHeaders after adding DEFAULT_RESPONSE_HEADERS, the defaults are forwarded only unless excluded.

Responses always forward a safe default set (`content-type`, `content-disposition`,
`content-language`, `content-range`, `accept-ranges`, `cache-control`, `etag`, `last-modified`,
`expires`, `vary`, `location`, `retry-after`, `www-authenticate`) plus anything in
`headers.forwardResponseHeaders`. `content-encoding` and `content-length` are never forwarded because
the proxy re-frames the body (`fetch` transparently decompresses upstream responses). Upstream `3xx`

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

j4ys0n · 2026-07-09T20:04:40Z

        await new Promise<void>((resolve) => {
          const settle = (): void => {
            input.res.off("drain", settle);
+            input.res.off("close", settle);
            input.req.off("close", settle);
            resolve();
          };
          input.res.once("drain", settle);
+          input.res.once("close", settle);
          input.req.once("close", settle);
        });


Verified against the code flow: the stated hang cannot occur today — clearTimeout(timeout) runs in the finally immediately after fetch() resolves with headers, so the timeout-driven controller.abort() can never fire during the relay/backpressure loop. The only mid-relay abort sources are the client-disconnect handlers, and both are driven by the same req/res close events the settle promise already listens for.

That said, the wait's liveness relied on that non-local invariant, so I've hardened it in 73861ff: the settle promise now also wakes on controller.signal abort, making it robust to any future abort source (e.g. an idle relay timeout).

The upstream timeout is cleared once response headers arrive, so today only the client-disconnect handlers (which also emit the close events the wait listens for) can abort mid-relay. Still, the wait's liveness depended on that non-local invariant; listening on the abort signal makes it robust to any future abort source (e.g. an idle relay timeout). Addresses Copilot review feedback on PR #5. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

Copilot

Pull request overview

Copilot reviewed 12 out of 12 changed files in this pull request and generated 3 comments.

j4ys0n · 2026-07-09T20:12:25Z

+  // WebSocket upstream URLs are connected verbatim, so the position rule applies to HTTP kinds.
+  if (resource.kind !== "websocket") {
+    for (const placeholder of findMisplacedUpstreamPlaceholders(resource.upstreamUrl) ?? []) {
+      errors.push(`upstreamUrl placeholder [${placeholder}] must occupy a full path segment`);
+    }


Valid — fixed in 4b85f8b. validateUpstreamPlaceholders now rejects any placeholder in a websocket upstreamUrl outright (websocket upstreamUrl must not contain placeholder [name]), since WS upstreams are connected verbatim and the placeholder would reach the upstream literally. Unit test added.

j4ys0n · 2026-07-09T20:12:27Z

+  if (!isNonEmptyString(resource.access.serviceTokenHeader)) {
+    errors.push("access.serviceTokenHeader is required for service-token mode");
+  } else if (shouldDropProxyHeader(resource.access.serviceTokenHeader.toLowerCase())) {
+    errors.push("access.serviceTokenHeader must not be a payment, hop-by-hop, host, or content-length header");
+  }


Valid — fixed in 4b85f8b. Validation now requires serviceTokenHeader to be an RFC 9110 field-name token (rejects whitespace/invalid chars rather than silently trimming) and rejects CR/LF/NUL in serviceTokenValue (which is also a header-injection vector, not just a throw risk). Error messages still never echo the value. Unit tests added.

j4ys0n · 2026-07-09T20:12:30Z

+  const { serviceTokenHeader, serviceTokenValue } = access;
+  if (!serviceTokenHeader || !serviceTokenValue || shouldDropProxyHeader(serviceTokenHeader.toLowerCase())) {
+    return;
+  }
+  headers.set(serviceTokenHeader, serviceTokenValue);


Valid — fixed in 4b85f8b. applyServiceTokenAccess now independently refuses invalid header-name tokens and values containing CR/LF/NUL (skip, consistent with the existing protected-name behavior) so a custom store that bypasses validateX402Resource can neither crash the request via Headers.set nor inject headers. Chose validate-and-skip over trim-and-use so a misconfigured credential never silently mutates. Unit test added covering both cases.

…aceholders Addresses Copilot round-2 feedback on PR #5: - reject placeholders in websocket upstreamUrl outright — WS upstreams are connected verbatim, so a placeholder always reaches the upstream literally and is a misconfiguration - validate access.serviceTokenHeader as an RFC 9110 field-name token and access.serviceTokenValue against CR/LF/NUL; an invalid name would make Headers.set throw at request time, and a control character in the value is a header-injection vector - applyServiceTokenAccess independently refuses invalid names/values as defense in depth for custom resource stores that bypass validateX402Resource Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

Copilot

Pull request overview

Copilot reviewed 12 out of 12 changed files in this pull request and generated 2 comments.

j4ys0n · 2026-07-09T20:20:11Z

+  } catch (error: unknown) {
+    // Aborting the upstream fetch after a client disconnect rejects the pending read;
+    // there is no one left to answer, so swallow only that case.
+    if (!clientClosed) {
+      throw error;
+    }


Valid — fixed in 02058e9. Both streaming catch blocks now swallow only an AbortError observed after clientClosed (shape-based name === "AbortError" check, since undici's DOMException is not instanceof Error on every Node version); any other relay failure propagates even when the client is gone. Added unit tests for both sides: a generic reader failure after disconnect rejects, an abort rejection after disconnect resolves quietly.

j4ys0n · 2026-07-09T20:20:13Z

+Each resource may set `access.mode`:
+
+- `pass-through` (default): client credentials reach the upstream only per the header policy above.
+- `service-token`: the proxy injects `access.serviceTokenHeader: access.serviceTokenValue` on the
+  upstream request, replacing any client-supplied value for that header. Use this when the upstream
+  requires a private service credential the paying client does not have:


Valid — fixed in 02058e9. Went one step further than a docs clarification: validateX402Resource now rejects service-token mode on websocket resources outright (access.mode service-token is not supported for websocket resources), since the WS gateway relays without header forwarding/injection and the token would silently never apply. README scopes access modes to HTTP kinds and states the rejection. Unit test added.

…e-token Addresses Copilot round-3 feedback on PR #5: - streaming catch blocks now swallow only the AbortError produced by the proxy's own disconnect abort (shape-based check, since undici's DOMException is not instanceof Error on every Node version); any other relay failure propagates even after the client is gone - service-token mode is rejected by validation on websocket resources — the WS gateway relays without header forwarding/injection, so the token would never be applied; README scope clarified to HTTP kinds Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

Copilot

Pull request overview

Copilot reviewed 13 out of 13 changed files in this pull request and generated 1 comment.

j4ys0n · 2026-07-09T20:29:01Z

+/** CR, LF, and NUL in a field value enable header injection and make Headers.set throw. */
+const HEADER_VALUE_FORBIDDEN_REGEX = /[\r\n\0]/;
+
+export function isValidHttpHeaderName(name: string): boolean {
+  return HEADER_NAME_TOKEN_REGEX.test(name);
+}
+
+export function isValidHttpHeaderValue(value: string): boolean {
+  return !HEADER_VALUE_FORBIDDEN_REGEX.test(value);
+}


Valid — fixed in d1387b3. isValidHttpHeaderValue now rejects the full control range (0x00–0x1F and DEL, tab included) via an explicit char-code check, matching the documented "control characters" contract at both the validation and injection layers. Tests pin SOH and DEL rejection in addition to the CR/LF injection case.

Addresses Copilot round-4 feedback on PR #5: isValidHttpHeaderValue only rejected CR/LF/NUL while the error text and README promised "control characters". It now rejects 0x00-0x1F and DEL via an explicit char-code check (RFC 9110 forbids CTLs in field values; any CTL in a credential is a misconfiguration). Tests pin SOH and DEL rejection at both the validation and injection layers. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

Copilot

Pull request overview

Copilot reviewed 13 out of 13 changed files in this pull request and generated 1 comment.

j4ys0n · 2026-07-09T20:34:25Z

+  const misplaced = new Set<string>();
+  for (const segment of target.pathname.split("/")) {
+    if (PARAM_SEGMENT_REGEX.test(segment)) {
+      continue;
+    }
+    for (const name of extractRoutePlaceholders(segment)) {
+      misplaced.add(name);
+    }
+  }
+  for (const name of extractRoutePlaceholders(`${target.search}${target.hash}`)) {
+    misplaced.add(name);
+  }


Valid in substance — fixed in c7b6bc7. Note the hostname example (https://[tenant].example.com) actually fails WHATWG URL parsing outright (brackets are only valid for IPv6 literals), so it was already rejected by URL validation — but userinfo placeholders (https://[user]@host/...) do parse (percent-encoded) and slipped through. Rather than enumerating more URL parts, findMisplacedUpstreamPlaceholders now counts every raw placeholder occurrence against the full-pathname-segment occurrences interpolation actually rewrites; any surplus occurrence is misplaced wherever it appears, so the check is complete by construction. Userinfo test added.

Addresses Copilot round-5 feedback on PR #5: the part-enumeration check (partial path segments + query + hash) missed placeholders in the URL userinfo, which parse percent-encoded and reach the upstream literally. findMisplacedUpstreamPlaceholders now counts every raw placeholder occurrence against the full-pathname-segment occurrences interpolation actually rewrites — any surplus occurrence is misplaced regardless of which URL part it hides in, making the check complete by construction. (Hostname placeholders already fail WHATWG URL parsing and are reported by URL validation.) Test added for the userinfo case. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

Copilot

Pull request overview

Copilot reviewed 13 out of 13 changed files in this pull request and generated 1 comment.

j4ys0n · 2026-07-09T20:40:05Z

+/**
+ * Shape-based AbortError check: undici rejects aborted fetches/reads with a DOMException
+ * named "AbortError", which is not an `instanceof Error` on every Node version.
+ */
+function isAbortError(error: unknown): boolean {
+  return typeof error === "object" && error !== null && (error as { name?: unknown }).name === "AbortError";
+}


Valid — fixed in 32f991d. isAbortError is now exported and reused at both remaining abort-detection sites (createHttpProxyHandler and sendProxyErrorResponse in resourceRuntime.ts), so a DOMException-shaped abort maps to 504 consistently across all proxy paths. Added a unit test with a non-instanceof Error abort shape.

Addresses Copilot round-6 feedback on PR #5: createHttpProxyHandler and sendProxyErrorResponse still used instanceof Error && name checks, which miss undici's DOMException abort shape and would surface a timeout as a generic failure instead of 504. isAbortError is now exported and reused at both sites; test added for the DOMException-shaped abort. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

Copilot

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

Copilot