Repository navigation
fix(web): fetch a DID's blobs only from a public PDS - #298
Merged
Merged
Conversation
The set player's audio route and fetchBlob (press kits' pictures) resolved a DID's PDS and fetched from it on the server, following redirects, so a DID document naming a private or local address had the site fetch it. Both now get the PDS through IdentityResolver.safePds, which refuses non-https and private addresses as the credentials code already did, and refuse redirects.
github-actions
Bot
added
web
Coverage Report for apps/web
File Coverage
SizeCompared with main. KiB are 1,024 bytes.
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
bnb
approved these changes
Oct 30, 2026
botandbang
added a commit
that referenced
this pull request
Nov 1, 2026
…etch bnb/profiles brings in main, whose #298 added IdentityResolver.safePds: the account's PDS once assertSafeEndpoint passed it, the same check before a separate fetch that this branch removes. main called it from core.fetchBlob, SpaceCredentialManager's mint, and the /media/audio route. Conflicts: - app.ts fetchBlob: the URL comes from identity.resolve, and the request goes through identity.serviceFetch with redirect 'error'. - credentials.ts mint: the authority's PDS from identity.resolve, as this branch had it. safePds is removed. The /media/audio route used it with SvelteKit's fetch: recordingAudio now takes the identity resolver in place of a pds lookup and a fetch, and reads the PDS with serviceFetch, so a caller can't hand it the plain fetch. audio.test.ts builds a real IdentityResolver over its fake PDS. serviceFetch drops its 60 second limit. safeFetchWrap's timeout pipes every body through a stream that reads ahead of the caller and kept the upstream open after the route cancelled a recording it refused, which the audio tests caught. Every caller already passes its own signal. no-unguarded-fetch also flags a fetch handed in as a dependency (request.fetch, this.fetch) in a function that reads a host, as the audio route did with request.pds(did); identity.fetch and identity.serviceFetch are left alone. It flags main's audio.ts and nothing on this tree. The DNS rebinding helper moves to testing/rebind.ts, where bnb/oauth-guarded-fetch put it, and a new route test streams a recording from a PDS whose name points at loopback when it connects: refused, with no connection. It reaches the loopback server on main's route.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What and why
The set player's audio route resolved a DJ's PDS from their DID document and fetched
com.atproto.sync.getBlobfrom it on the server, following redirects. Anyone can publish a recording record and point their DID document (a did:web is easy) at a private or link-local address, and the site would fetch it from inside its network.fetchBlobin app.ts, used for any press kit's pictures, had the same gap. The credentials code already checked PDS addresses withassertSafeEndpoint; these two paths didn't.Added:
IdentityResolver.safePds(did), which resolves the DID and refuses a PDS that isn't https or is a private, loopback or link-local address (localhost still allowed in dev).Changed: the audio route and
fetchBlobget their PDS throughsafePdsand fetch withredirect: 'error', so a refused PDS or a redirect ends as a 502 "Recording unavailable" with a warn log. The credentials code's resolve-then-check pair is now onesafePdscall, with no behaviour change.Not changed:
@atproto/identitystill fetches a did:web document from its domain without an address check. That's a blind GET inside the library.How it was tested
audio.test.ts: PDSes athttps://127.0.0.1,https://10.0.0.1,https://169.254.169.254,https://[::1]andhttp://pds.exampleget a 502 and nothing is fetched; a public PDS still serves a 206 range; a real local server answering 302 isn't followed. Breaking the guard or the redirect rule fails them.format:check,lint,check,knip, ast-grep,test(787) andtest:coverage(984 passed, thresholds met).Checklist
docs/and--helpstill say what the code does