feat(did): refuse redirects when resolving did:web by default by EfeDurmaz16 · Pull Request #133 · agentcommercekit/ack

EfeDurmaz16 · 2026-07-28T22:17:35Z

What

getResolver for did:web now sends redirect: "error" when fetching the DID document. A new followRedirects?: boolean option (default false) restores the previous behavior.

Why

allowedHttpHosts is applied to the URL built from the DID:

const url = isHttpAllowed(path, allowedHttpHosts)
  ? `http://${path}`
  : `https://${path}`

The fetch that follows it used the platform default, which follows redirects. So the allowlist only governs the first hop: a did:web resolved over https can answer 302 Location: http://…, and the resolver follows it to a scheme and host the check was there to reject.

flowchart LR
  D["did:web:issuer.example"] --> C{"allowedHttpHosts<br/>allows http for this host?"}
  C -- no --> U["fetch https://issuer.example/.well-known/did.json"]
  U --> H{"host answers 302<br/>Location: http://internal/…"}
  H -->|before: redirect followed| BAD["request lands on the scheme<br/>and host the check rejected"]
  H -->|after: redirect error| OK["resolution fails as notFound"]

  classDef bad stroke-dasharray: 4 3
  class BAD bad

Because the redirect target is chosen by the DID's own host, this also makes resolution an outbound request an untrusted party controls. That matters for anything that resolves a DID before verifying a signature, which is the normal order: you need the document to check the signature. A server verifying an ACK-ID proof from an arbitrary issuer will fetch whatever that issuer's host points it at.

did:web documents are served directly at /.well-known/did.json (or the configured docPath), so refusing redirects costs legitimate resolution nothing. Deployments that genuinely sit behind a redirect can opt in.

How

DidWebResolverOptions gains followRedirects?: boolean, documented with the reason, defaulting to false.
fetchDidDocumentAtUrl passes redirect: followRedirects ? "follow" : "error" alongside the existing mode: "cors".
Existing tests that assert the fetch options were updated to include the new field; two tests cover the default (refuses, resolves to notFound) and the opt-in (redirect: "follow").

Test plan

pnpm build
pnpm --filter @agentcommercekit/did test   # 72 passed

Found while building an ACK-ID identity-gated x402 demo, where a seller resolves buyer DIDs it has never seen before.

Implemented and tested by gpt 5.6-sol and fable 5, reviewed by me.

Summary by CodeRabbit

New Features
- Added optional followRedirects: true support for did:web and did:jwks resolution.
- Redirects are now blocked by default for safer resolution.
- Blocked redirects report the destination when available; otherwise, resolution returns a clear not-found or resolution error.
- Enabling redirect following preserves existing successful resolution behavior.
Documentation
- Documented redirect-handling behavior and the followRedirects option.

allowedHttpHosts is applied to the URL built from the DID, but the document fetch followed redirects, so the check only governed the first hop: a did:web served over https could redirect the resolver to plain http, or to a host the allowlist would have rejected. Because the redirect target is chosen by the DID's own host, this also turns resolution into an outbound request the issuer controls. did:web documents are served directly at a well-known path, so redirects are refused by default via redirect: "error". followRedirects: true restores the previous behavior for deployments that need it.

chatgpt-codex-connector · 2026-07-28T22:17:38Z

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.

coderabbitai · 2026-07-28T22:17:51Z

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 2e86e4b8-8a98-41d2-872d-3c8905595884

📥 Commits

Reviewing files that changed from the base of the PR and between b3be83c and e4efdce.

📒 Files selected for processing (5)

.changeset/wild-hounds-shave.md
packages/did/src/did-resolvers/get-did-resolver.test.ts
packages/did/src/did-resolvers/get-did-resolver.ts
packages/did/src/did-resolvers/web-did-resolver.test.ts
packages/did/src/did-resolvers/web-did-resolver.ts

💤 Files with no reviewable changes (1)

packages/did/src/did-resolvers/web-did-resolver.test.ts

🚧 Files skipped from review as they are similar to previous changes (2)

packages/did/src/did-resolvers/web-did-resolver.ts
.changeset/wild-hounds-shave.md

Included review availability: Your plan provides up to 2 included reviews per hour; 1 remains after this review.

Walkthrough

The did:web and did:jwks resolvers now reject redirects by default through manual redirect handling. The followRedirects option restores redirect following when enabled. Tests and a minor-release changeset document the behavior.

Changes

DID redirect handling

Layer / File(s)	Summary
Redirect policy and document fetching `packages/did/src/did-resolvers/web-did-resolver.ts`	Adds `followRedirects`, defaults it to `false`, and selects manual or automatic redirect handling for document fetching.
JWKS redirect integration `packages/did/src/did-resolvers/get-did-resolver.ts`	Applies the redirect policy to configured and global fetch implementations while preserving request options.
Redirect validation and release `packages/did/src/did-resolvers/*test.ts`, `.changeset/wild-hounds-shave.md`	Updates fetch expectations, tests redirect rejection and opt-in following, and documents the release behavior.

Estimated code review effort: 3 (Moderate) | ~25 minutes

Merge Risk: ⚪ Minimal · up to e4efd

This change makes did:web resolution reject redirects by default while preserving opt-in support for deployments that require them. No actionable merge-blocking risk remains beyond normal checks and review.

Suggested reviewers: venables

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 75.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 4 functions across 4 files. (1 skipped: 1…	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly summarizes the primary change: `did:web` resolution refuses redirects by default. It is concise and specific, although it does not mention the related `did:jwks` behavior.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

Full details: Docstring Coverage

Explanation

Docstring coverage is 75.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 4 functions across 4 files. (1 skipped: 1 unsupported.)

Fix all pre-merge checks with AI

✨ Finishing Touches

🧪 Generate unit tests (beta)

Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands.}

domleboss97

One comment on the error handling, but I think this is the right move!

domleboss97 · 2026-07-30T14:17:46Z

-  const res = await fetch(url, { mode: "cors" })
+  const res = await fetch(url, {
+    mode: "cors",
+    redirect: followRedirects ? "follow" : "error",


i think it might be more useful to do manual here. with error this will throw with a TypeError, and then I think the resolver has nothing better ot report than notFound.

with manual, fetch resolves with the redirect repsonse (although in browsers i think it's opaque response), so we could throw a precise error - on node it could include the redirect target from the Location header.

Done in 22f6a5d: switched to redirect: "manual". The resolver now refuses any redirect response with an error naming the Location target on Node; browsers surface an opaque redirect so the same error fires without a target. Tests cover both cases, changeset updated.

Per review: redirect: "error" rejects with a bare TypeError, which the resolver can only surface as a generic notFound. With redirect: "manual" the redirect resolves as a response, so the resolver throws a precise error naming the Location target on Node; browsers surface an opaque redirect and get the same error without a target.

domleboss97

🚀

domleboss97 · 2026-07-30T14:37:30Z

+  ) {
+    const location = res.headers.get("location")
+    throw new Error(
+      `DID resolution refused a redirect${location ? ` to ${location}` : ""}`,


nit - might be nice to add an action here, i.e. Set followRedirects: true to allow redirects.

venables · 2026-08-04T15:11:14Z

@EfeDurmaz16 mind resolving the conflicts?

Resolves the conflicts between the redirect refusal in this branch and the did:web fetch timeout added upstream in agentcommercekit#129. Both options now apply to the same request: the resolver sends `redirect: "manual"` (unless `followRedirects` is set) and an `AbortSignal` built from `timeout`. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01ScNeLEEDvtbyDg8MQwRob3

EfeDurmaz16 · 2026-08-04T16:33:54Z

@venables resolved!

venables

Location: packages/did/src/did-resolvers/get-did-resolver.ts:34-37 —

ack/packages/did/src/did-resolvers/get-did-resolver.ts

Lines 34 to 37 in b3be83c

    
           const jwksResolver = getJwksDidResolver({ 
        
             ...webOptions, 
        
             fetch: webFetch ? (input, init) => webFetch(input, init) : globalThis.fetch, 
        
           })

getDidResolver spreads the same webOptions into jwks-did-resolver, but did-jwks calls fetchImpl(url) with no init (see node_modules/did-jwks/dist/fetch-*.js:125), so the did:jwks path still follows redirects after this change. A caller who sets webOptions.followRedirects (or relies on the new default) gets protection for did:web only, and the same allowedHttpHosts bypass remains for did:jwks.

Possible Solution: in the existing fetch wrapper at line 36, pass { ...init, redirect: webOptions.followRedirects ? "follow" : "manual" } (did-jwks treats a 3xx as !resp.ok and returns null), or state in the followRedirects docs that the option applies to did:web only.

venables · 2026-08-26T21:47:14Z

+  /**
+   * Whether to follow HTTP redirects while fetching the did document.
+   *
+   * The `allowedHttpHosts` check applies to the resolved URL only, so a
+   * followed redirect can move the request to a host or scheme that check
+   * would have rejected. did:web documents are served directly at a
+   * well-known path, so redirects are refused by default.
+   *
+   * @default false
+   */
+  followRedirects?: boolean


Small / Optional polish: The followRedirects doc does not say that a custom fetch must honour init.redirect. A custom fetch that ignores it silently follows redirects, and the 3xx check at line 100-103 never fires. The timeout option (line 62-63) already carries the equivalent note for init.signal.

Suggested change

/**

* Whether to follow HTTP redirects while fetching the did document.

*

* The `allowedHttpHosts` check applies to the resolved URL only, so a

* followed redirect can move the request to a host or scheme that check

* would have rejected. did:web documents are served directly at a

* well-known path, so redirects are refused by default.

*

* @default false

*/

followRedirects?: boolean

/**

* Whether to follow HTTP redirects while fetching the did document.

*

* The `allowedHttpHosts` check applies to the resolved URL only, so a

* followed redirect can move the request to a host or scheme that check

* would have rejected. did:web documents are served directly at a

* well-known path, so redirects are refused by default.

*

* A custom `fetch` must honour `init.redirect` for this to take effect.

*

* @default false

*/

getDidResolver spread webOptions into jwks-did-resolver, but did-jwks calls its fetch with no init, so did:jwks still followed redirects and the allowedHttpHosts bypass remained open there. Inject init.redirect in the shared fetch wrapper; a refused redirect fails the ok check and the resolution fails as notFound.

Match the timeout option's init.signal note, and shorten the redirect comments.

EfeDurmaz16 · 2026-08-26T21:56:52Z

Addressed in c0279ce: the redirect policy is now injected into the fetch passed to jwks-did-resolver, since did-jwks calls fetch with no init. A refused redirect fails the ok check and did:jwks resolution fails as notFound. Added tests for the default, followRedirects: true, and the global-fetch path, and updated the changeset. The followRedirects doc note about custom fetch honouring init.redirect landed in e4efdce.

coderabbitai Bot mentioned this pull request Jul 30, 2026

feat(did): add optional fetch timeout to the did:web resolver #129

Merged

domleboss97 previously approved these changes Jul 30, 2026

View reviewed changes

EfeDurmaz16 dismissed domleboss97’s stale review via 22f6a5d July 30, 2026 14:22

EfeDurmaz16 requested a review from domleboss97 July 30, 2026 14:24

domleboss97 previously approved these changes Jul 30, 2026

View reviewed changes

fix(did): point the redirect error at followRedirects

2ea4a25

EfeDurmaz16 dismissed domleboss97’s stale review via 2ea4a25 July 30, 2026 14:49

domleboss97 previously approved these changes Jul 30, 2026

View reviewed changes

EfeDurmaz16 dismissed domleboss97’s stale review via b3be83c August 4, 2026 15:34

venables requested changes Aug 26, 2026

View reviewed changes

EfeDurmaz16 added 2 commits August 27, 2026 00:56

docs(did): note a custom fetch must honour init.redirect

e4efdce

Match the timeout option's init.signal note, and shorten the redirect comments.

venables mentioned this pull request Aug 26, 2026

Prevent unauthorized redirects in fetch options #173

Closed

Navigation Menu

Uh oh!

feat(did): refuse redirects when resolving did:web by default - #133

feat(did): refuse redirects when resolving did:web by default#133
EfeDurmaz16 wants to merge 6 commits into
agentcommercekit:mainfrom
EfeDurmaz16:did-resolver-no-redirect

EfeDurmaz16 commented Jul 28, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

chatgpt-codex-connector Bot commented Jul 28, 2026

Uh oh!

coderabbitai Bot commented Jul 28, 2026 •

edited

Loading

❌ Failed checks (1 warning)

Uh oh!

domleboss97 left a comment

Uh oh!

domleboss97 Jul 30, 2026

Uh oh!

EfeDurmaz16 Jul 30, 2026

Uh oh!

domleboss97 left a comment

Uh oh!

domleboss97 Jul 30, 2026

Uh oh!

venables commented Aug 4, 2026

Uh oh!

EfeDurmaz16 commented Aug 4, 2026

Uh oh!

venables left a comment

Uh oh!

venables Aug 26, 2026

Uh oh!

EfeDurmaz16 commented Aug 26, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

	const jwksResolver = getJwksDidResolver({
	...webOptions,
	fetch: webFetch ? (input, init) => webFetch(input, init) : globalThis.fetch,
	})

Uh oh!

Conversation

EfeDurmaz16 commented Jul 28, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What

Why

How

Test plan

Summary by CodeRabbit

Uh oh!

chatgpt-codex-connector Bot commented Jul 28, 2026

Uh oh!

coderabbitai Bot commented Jul 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

❌ Failed checks (1 warning)

Uh oh!

domleboss97 left a comment

Choose a reason for hiding this comment

Uh oh!

domleboss97 Jul 30, 2026

Choose a reason for hiding this comment

Uh oh!

EfeDurmaz16 Jul 30, 2026

Choose a reason for hiding this comment

Uh oh!

domleboss97 left a comment

Choose a reason for hiding this comment

Uh oh!

domleboss97 Jul 30, 2026

Choose a reason for hiding this comment

Uh oh!

venables commented Aug 4, 2026

Uh oh!

EfeDurmaz16 commented Aug 4, 2026

Uh oh!

venables left a comment

Choose a reason for hiding this comment

Uh oh!

venables Aug 26, 2026

Choose a reason for hiding this comment

Uh oh!

EfeDurmaz16 commented Aug 26, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

EfeDurmaz16 commented Jul 28, 2026 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented Jul 28, 2026 •

edited

Loading