Harden collectible metadata fetch with bounded reads by CassioMG · Pull Request #2719 · stellar/freighter

CassioMG · 2026-04-23T20:37:25Z

Summary

Adds a dedicated fetchMetadataJson helper and routes fetchCollectibleMetadata through it so NFT metadata reads are HTTPS-only, bounded in time and size, and robust against redirect-driven scheme downgrades.

The helper enforces, in order:

HTTPS allowlist — input URL is rejected before any network call if the scheme is not https:; the rejection message includes the offending protocol for easier diagnosis.
Redirect re-check — after fetch resolves, response.url is re-validated so a 3xx redirect that lands on a non-https scheme (e.g. https:→http:) is rejected instead of silently followed.
5-second timeout — AbortController aborts the in-flight request so slow or unreachable hosts don't leave work hanging.
1 MB response cap — Content-Length is pre-checked; the body is then streamed via response.body.getReader() with a running byte counter, and the reader is cancelled if the accumulated bytes exceed the cap. Falls back to response.text() + byte-length check when getReader is unavailable (defense-in-depth; modern runtimes always expose it).
Body cleanup on early exits — non-OK responses, oversized Content-Length, and redirect-to-non-https all call response.body.cancel() before throwing so streaming runtimes don't continue downloading into a discarded response.

fetchCollectibleMetadata keeps its existing try/catch → return null wrapper, so the UI-facing contract is unchanged (null metadata still falls back to the default name / broken-image placeholder). The existing decision to skip captureException for metadata failures is preserved — third-party metadata hosts fail often enough that flooding Sentry is counter-productive.

Only the NFT collectible metadata path is touched. The sibling fetchCollectibles function (which talks to the internal Freighter BE indexer) and every other fetch call site in the extension are untouched; the helper is also not routed into them.

27 Jest cases cover the helper (URL validation, response handling, size limits via both Content-Length and streamed paths, fallback path, timeout, redirect re-check, body cancellation on all early exits). The sibling fetchCollectibles test is updated to mock the helper at its seam instead of at global.fetch, and an unexpected-URL fetch mock now rejects explicitly to surface drift.

Mirrors the sister hardening shipped in Harden collectible metadata fetch with bounded reads freighter-mobile#835.

Screen.Recording.2026-04-23.at.13.33.16.mov

When metadata fetching fails

Screen.Recording.2026-04-23.at.14.07.12.mov

🤖 Generated with Claude Code

Introduces a dedicated helper that wraps fetch() with URL scheme validation (https-only), an AbortController-backed 5-second timeout, a Content-Length pre-check, and a streaming byte counter so response bodies are read within a 1 MB cap. When the current runtime does not expose response.body.getReader, the helper falls back to response.text() followed by a byte-length check. Errors include the offending protocol when the scheme check rejects, for easier diagnosis. Cancellation-side rejections from reader.cancel() are swallowed so the original "too large" message always wins. 23 Jest tests cover URL validation, response handling, size limits via both Content-Length and streaming, the getReader-missing fallback, and timeout behavior. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

…Json - Validate the final response.url after fetch resolves, not just the input URL. If a 3xx redirect lands on a non-https scheme (e.g. https→http) the helper now rejects instead of silently following through. Covered by two new tests (redirect-bypass rejected, https→https redirect accepted). - On the early-exit failure paths (non-OK status, oversized Content-Length, redirect-to-non-https) call response.body?.cancel?.() before throwing so streaming implementations don't keep downloading into a discarded response. Covered by new tests asserting the cancel call. - JSDoc updated to describe the redirect check and the body-cancel behavior on early exits. Mirrors stellar/freighter-mobile#835 (e3dd3cf8). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

…ataJson Replaces the inline fetch()/response.json() pair inside fetchCollectibleMetadata with a single call through the new fetchMetadataJson<CollectibleMetadataResponse>(tokenUri) helper. The outer try/catch remains so any helper rejection is still surfaced as a null return — UI callers fall back to the existing default-name / broken-image placeholder behaviour unchanged. The accompanying Jest suite switches from mocking global.fetch for the metadata leg to mocking the helper via jest.mock. global.fetch remains spied only for the /collectibles indexer endpoint; any unexpected URL now explicitly rejects, which surfaces drift instead of masking it. Preserves the intentional decision not to report metadata failures to Sentry — third-party hosts fail often and flooding the tracker is counter-productive. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Copilot

Pull request overview

This PR hardens third-party NFT metadata fetching by introducing a dedicated helper that enforces HTTPS-only requests and bounds reads by time and response size, then routing collectible metadata retrieval through that helper to preserve the existing UI contract (return null on failure).

Changes:

Add fetchMetadataJson helper with HTTPS validation (including post-redirect), 5s timeout, and 1MB response cap with streaming reads + cancellation.
Update fetchCollectibleMetadata to use the new helper while keeping the existing try/catch → null behavior.
Add comprehensive Jest coverage for the helper and update fetchCollectibles tests to mock at the helper seam.

Reviewed changes

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

File	Description
@shared/api/helpers/fetchMetadataJson.ts	New bounded/validated JSON fetch helper for NFT metadata.
@shared/api/helpers/fetchCollectibles.ts	Route collectible metadata fetch through `fetchMetadataJson`.
@shared/api/helpers/tests/fetchMetadataJson.test.ts	New test suite covering URL validation, redirects, timeout, and size limits.
@shared/api/helpers/tests/fetchCollectibles.test.ts	Update tests to mock `fetchMetadataJson` instead of `global.fetch` for metadata.

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

Addresses PR review feedback. If response.url is present but cannot be parsed we can't confirm the scheme, so we can't confirm we weren't redirected away from https — treat that as a rejection rather than silently proceeding. One new test covers the unparseable case. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Copilot

Pull request overview

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

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

piyalbasu · 2026-04-23T21:22:53Z

Code review

No issues found. Checked for bugs and CLAUDE.md compliance.

🤖 Generated with Claude Code

_{- If this code review was useful, please react with 👍. Otherwise, react with 👎.}

piyalbasu

Looks good to me. Confirmed on Slack that a user sees sane defaults and is not blocked when metadata cannot be parsed

https://stellarfoundation.slack.com/archives/C03347FNAHK/p1776976823841349

CassioMG and others added 3 commits April 23, 2026 13:34

Copilot AI review requested due to automatic review settings April 23, 2026 20:37

Copilot started reviewing on behalf of CassioMG April 23, 2026 20:37 View session

CassioMG changed the title ~~harden collectible metadata fetch with bounded reads~~ Harden collectible metadata fetch with bounded reads Apr 23, 2026

CassioMG self-assigned this Apr 23, 2026

CassioMG requested review from aristidesstaffieri, leofelix077 and piyalbasu April 23, 2026 20:41

Copilot AI reviewed Apr 23, 2026

View reviewed changes

Comment thread @shared/api/helpers/fetchMetadataJson.ts

Comment thread @shared/api/helpers/fetchMetadataJson.ts

CassioMG requested a review from Copilot April 23, 2026 20:54

Copilot started reviewing on behalf of CassioMG April 23, 2026 20:54 View session

Copilot AI reviewed Apr 23, 2026

View reviewed changes

Comment thread @shared/api/helpers/fetchMetadataJson.ts

piyalbasu approved these changes Apr 23, 2026

View reviewed changes

CassioMG merged commit 0a910b3 into master Apr 23, 2026
13 checks passed

CassioMG deleted the fix/metadata-fetch-hardening branch April 23, 2026 23:44

github-actions Bot mentioned this pull request Apr 24, 2026

v5.40.0 #2730

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Harden collectible metadata fetch with bounded reads#2719

Harden collectible metadata fetch with bounded reads#2719
CassioMG merged 4 commits intomasterfrom
fix/metadata-fetch-hardening

CassioMG commented Apr 23, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

piyalbasu commented Apr 23, 2026

Uh oh!

piyalbasu left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

CassioMG commented Apr 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

When metadata fetching fails

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

piyalbasu commented Apr 23, 2026

Code review

Uh oh!

piyalbasu left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

CassioMG commented Apr 23, 2026 •

edited

Loading