From f650ebd3fb55f0f863921c1fa373469471a5a076 Mon Sep 17 00:00:00 2001 From: Jonathan LoBue Date: Fri, 14 Aug 2026 06:27:47 -0700 Subject: [PATCH 01/10] Fetch ipfs:// NFT resources through an HTTPS gateway Some NFTs are minted with bare ipfs:/// data, metadata, or license URIs instead of an HTTPS gateway URL. Every such URI failed validation in the GUI cache layer with "Invalid URL: ipfs://...": validator's isURL applies an FQDN check to the host, and a CID has no top-level domain, so listing 'ipfs' as an allowed protocol never actually accepted anything. Even when a caller ignored the validation error, Electron's net stack cannot request the ipfs scheme, so the media could never be fetched, verified, or cached. Translate ipfs:// URIs to their HTTPS gateway form (https://ipfs.io/ipfs//) in one shared helper and apply it - in the electron isValidURL, which now validates the gateway form (the URL that is actually requested), - at the outgoing request sites (downloadFile, fetchBuffer, fetchJSON) right where the URL reaches net.request, - at the single-NFT download handler, whose Chromium downloadURL cannot fetch the ipfs scheme either, - in the oversized-image direct-URL fallback of the dapp dialog, whose CSP only allows https: and data: images, - in the NFTHashStatus badge so ipfs URIs are no longer flagged as invalid in the renderer. The original on-chain URI remains the cache key everywhere, so existing cache entries, cache-info sidecars, and renderer lookups are unaffected. The redundant ipfs://ipfs/ form produced by some minting tools is tolerated, and CID case is preserved (CIDv0 is case-sensitive base58). The gateway does not need to be trusted for integrity: everything the cache serves is verified against the NFT's on-chain hash. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01Vai5yNzPZwUSgid2nhQvys --- .../gui/src/components/nfts/NFTHashStatus.tsx | 5 +- packages/gui/src/electron/CacheManager.ts | 3 +- .../src/electron/api/nftGetMetadata.test.ts | 15 ++++++ .../gui/src/electron/api/nftGetMetadata.ts | 5 +- packages/gui/src/electron/main.tsx | 5 +- .../gui/src/electron/utils/downloadFile.ts | 6 ++- .../gui/src/electron/utils/fetchBuffer.ts | 5 +- packages/gui/src/electron/utils/fetchJSON.ts | 6 ++- .../gui/src/electron/utils/isValidURL.test.ts | 25 +++++++++ packages/gui/src/electron/utils/isValidURL.ts | 8 ++- packages/gui/src/util/ipfs.test.ts | 53 +++++++++++++++++++ packages/gui/src/util/ipfs.ts | 33 ++++++++++++ 12 files changed, 160 insertions(+), 9 deletions(-) create mode 100644 packages/gui/src/electron/utils/isValidURL.test.ts create mode 100644 packages/gui/src/util/ipfs.test.ts create mode 100644 packages/gui/src/util/ipfs.ts diff --git a/packages/gui/src/components/nfts/NFTHashStatus.tsx b/packages/gui/src/components/nfts/NFTHashStatus.tsx index 884d8ecfc8..367a6d4cda 100644 --- a/packages/gui/src/components/nfts/NFTHashStatus.tsx +++ b/packages/gui/src/components/nfts/NFTHashStatus.tsx @@ -8,6 +8,7 @@ import React, { useMemo } from 'react'; import useNFT from '../../hooks/useNFT'; import useNFTVerifyHash from '../../hooks/useNFTVerifyHash'; +import ipfsToGatewayUrl from '../../util/ipfs'; export type NFTHashStatusProps = { nftId: string; @@ -39,7 +40,9 @@ export default function NFTHashStatus(props: NFTHashStatusProps) { } if (nftPreview.uri) { - return isValidURL(nftPreview.uri); + // ipfs:// URIs are served through an HTTPS gateway by the cache layer, + // so validate the gateway form instead of flagging them as invalid. + return isValidURL(ipfsToGatewayUrl(nftPreview.uri)); } return false; diff --git a/packages/gui/src/electron/CacheManager.ts b/packages/gui/src/electron/CacheManager.ts index 745cd4899d..ef22205f71 100644 --- a/packages/gui/src/electron/CacheManager.ts +++ b/packages/gui/src/electron/CacheManager.ts @@ -7,7 +7,6 @@ import path from 'node:path'; import { Readable } from 'node:stream'; import debug from 'debug'; -import isURL from 'validator/lib/isURL'; import type CacheInfo from '../@types/CacheInfo'; import type CacheInfoBase from '../@types/CacheInfoBase'; @@ -435,7 +434,7 @@ export default class CacheManager extends EventEmitter { const normalizedURL = decodeURI(url) === url ? encodeURI(url) : url; - if (!isURL(normalizedURL)) { + if (!isValidURL(normalizedURL)) { throw new Error(`Invalid URL: ${normalizedURL}`); } diff --git a/packages/gui/src/electron/api/nftGetMetadata.test.ts b/packages/gui/src/electron/api/nftGetMetadata.test.ts index 10f560d3fc..7a1f30dd2f 100644 --- a/packages/gui/src/electron/api/nftGetMetadata.test.ts +++ b/packages/gui/src/electron/api/nftGetMetadata.test.ts @@ -124,6 +124,21 @@ describe('nftGetImageDataUrl', () => { ); }); + it('falls back to the gateway URL for an oversized ipfs image when unverified previews are enabled', async () => { + mockAllowUnverifiedNftPreviews.mockReturnValue(true); + mockFetchBuffer.mockRejectedValue( + new MaxSizeExceededError({ + 'content-type': 'image/gif', + }), + ); + + // the dialog CSP only allows https: and data: images, so the raw ipfs + // URI would render as a broken image + await expect(nftGetImageDataUrl('ipfs://bafybeigdyrztest/large.gif', '00')).resolves.toBe( + 'https://ipfs.io/ipfs/bafybeigdyrztest/large.gif', + ); + }); + it('rejects an oversized response that is not an image even when unverified previews are enabled', async () => { mockAllowUnverifiedNftPreviews.mockReturnValue(true); mockFetchBuffer.mockRejectedValue( diff --git a/packages/gui/src/electron/api/nftGetMetadata.ts b/packages/gui/src/electron/api/nftGetMetadata.ts index f1f1d4a481..099955a027 100644 --- a/packages/gui/src/electron/api/nftGetMetadata.ts +++ b/packages/gui/src/electron/api/nftGetMetadata.ts @@ -2,6 +2,7 @@ import crypto from 'node:crypto'; import type Headers from '../../@types/Headers'; import compareChecksums from '../../util/compareChecksums'; +import ipfsToGatewayUrl from '../../util/ipfs'; import allowUnverifiedNftPreviews from '../utils/allowUnverifiedNftPreviews'; import fetchBuffer, { MaxSizeExceededError } from '../utils/fetchBuffer'; @@ -103,8 +104,10 @@ export async function nftGetImageDataUrl( // behavior for these files. Off by default: the response's size and type // claims are attacker-controlled, so the fallback can be triggered // deliberately to place unverified content in a confirmation dialog. + // The CSP does not allow the ipfs: scheme either, so ipfs URIs fall back + // to their gateway form. if (error instanceof MaxSizeExceededError && getImageContentType(error.headers) && allowUnverifiedNftPreviews()) { - return imageUri; + return ipfsToGatewayUrl(imageUri); } // image previews are best effort — the confirmation dialog has a fallback diff --git a/packages/gui/src/electron/main.tsx b/packages/gui/src/electron/main.tsx index 80e442814b..5b70a3a4e8 100644 --- a/packages/gui/src/electron/main.tsx +++ b/packages/gui/src/electron/main.tsx @@ -29,6 +29,7 @@ import type { PermissionsNotificationPayload } from '../@types/PermissionsServic import { WcError, WcErrorCode, encodeWcErrorForIpc } from '../@types/WcError'; import AppIcon from '../assets/img/chia64x64.png'; import { i18n } from '../config/locales'; +import ipfsToGatewayUrl from '../util/ipfs'; import CacheManager, { CACHE_PROTOCOL } from './CacheManager'; import { checkNFTOwnership } from './api/checkNFTOwnership'; @@ -575,7 +576,9 @@ if (ensureSingleInstance() && ensureCorrectEnvironment()) { return; } - mainWindow.webContents.downloadURL(urlLocal); + // Chromium's downloader cannot fetch the ipfs: scheme; download ipfs + // URIs through the HTTPS gateway like every other network path. + mainWindow.webContents.downloadURL(ipfsToGatewayUrl(urlLocal)); }); ipcMainHandle(AppAPI.START_MULTIPLE_DOWNLOAD, async (tasks: { url: string; filename: string }[]) => { diff --git a/packages/gui/src/electron/utils/downloadFile.ts b/packages/gui/src/electron/utils/downloadFile.ts index 7520fdb5ac..960c89c071 100644 --- a/packages/gui/src/electron/utils/downloadFile.ts +++ b/packages/gui/src/electron/utils/downloadFile.ts @@ -4,6 +4,7 @@ import { promises as fs, createWriteStream, type WriteStream } from 'node:fs'; import debug from 'debug'; import type Headers from '../../@types/Headers'; +import ipfsToGatewayUrl from '../../util/ipfs'; import fileExists from './fileExists'; import isValidURL from './isValidURL'; @@ -100,7 +101,10 @@ export default async function downloadFile( } const tempFilePath = `${localPath}.tmp`; - const request = net.request(url); + // ipfs:// URIs are fetched through an HTTPS gateway — Electron's net stack + // cannot request the ipfs scheme. Only this outgoing request uses the + // translated URL; callers keep the original URI as the cache key. + const request = net.request(ipfsToGatewayUrl(url)); const outputStream = new WriteStreamPromise(tempFilePath, overrideFile); // set when we abort the request ourselves, so abort events can be reported diff --git a/packages/gui/src/electron/utils/fetchBuffer.ts b/packages/gui/src/electron/utils/fetchBuffer.ts index cad731bac0..9c52960425 100644 --- a/packages/gui/src/electron/utils/fetchBuffer.ts +++ b/packages/gui/src/electron/utils/fetchBuffer.ts @@ -1,6 +1,7 @@ import { net, type IncomingMessage } from 'electron'; import type Headers from '../../@types/Headers'; +import ipfsToGatewayUrl from '../../util/ipfs'; import isValidURL from './isValidURL'; @@ -41,7 +42,9 @@ export default async function fetchBuffer( const request = net.request({ method: 'GET', - url, + // ipfs:// URIs are fetched through an HTTPS gateway — Electron's net + // stack cannot request the ipfs scheme. + url: ipfsToGatewayUrl(url), headers, }); diff --git a/packages/gui/src/electron/utils/fetchJSON.ts b/packages/gui/src/electron/utils/fetchJSON.ts index 982c010fdb..9bd44b3c90 100644 --- a/packages/gui/src/electron/utils/fetchJSON.ts +++ b/packages/gui/src/electron/utils/fetchJSON.ts @@ -1,5 +1,7 @@ import { net, IncomingMessage } from 'electron'; +import ipfsToGatewayUrl from '../../util/ipfs'; + import isValidURL from './isValidURL'; const DEFAULT_TIMEOUT = 10 * 60 * 1000; // 10 minutes @@ -17,7 +19,9 @@ export default async function fetchJSON( const request = net.request({ method, - url, + // ipfs:// URIs are fetched through an HTTPS gateway — Electron's net + // stack cannot request the ipfs scheme. + url: ipfsToGatewayUrl(url), headers, }); diff --git a/packages/gui/src/electron/utils/isValidURL.test.ts b/packages/gui/src/electron/utils/isValidURL.test.ts new file mode 100644 index 0000000000..e08e98eb39 --- /dev/null +++ b/packages/gui/src/electron/utils/isValidURL.test.ts @@ -0,0 +1,25 @@ +import isValidURL from './isValidURL'; + +describe('isValidURL', () => { + it('accepts https URLs', () => { + expect(isValidURL('https://example.com/image.png')).toBe(true); + }); + + it('requires the protocol and rejects non-https schemes', () => { + expect(isValidURL('example.com/image.png')).toBe(false); + expect(isValidURL('http://example.com/image.png')).toBe(false); + expect(isValidURL('ftp://example.com/image.png')).toBe(false); + }); + + it('accepts ipfs:// URIs with a CID host', () => { + // validator's isURL rejects CID hosts (no TLD), so these pass only via + // the gateway translation + expect(isValidURL('ipfs://bafybeiceg2gltyhlkukwetn26k7t2zdvthg4u4c6uj23rpni2adzgvo5si/020.png')).toBe(true); + expect(isValidURL('ipfs://ipfs/QmPK1s3pNYLi9ERiq3BDxKa4XosgWwFRQUydHUtz4YgpqB')).toBe(true); + }); + + it('rejects a bare ipfs scheme and non-strings', () => { + expect(isValidURL('ipfs://')).toBe(false); + expect(isValidURL(undefined as unknown as string)).toBe(false); + }); +}); diff --git a/packages/gui/src/electron/utils/isValidURL.ts b/packages/gui/src/electron/utils/isValidURL.ts index 53153a77e4..c1b4c1c38d 100644 --- a/packages/gui/src/electron/utils/isValidURL.ts +++ b/packages/gui/src/electron/utils/isValidURL.ts @@ -1,9 +1,15 @@ import isURL from 'validator/lib/isURL'; +import ipfsToGatewayUrl from '../../util/ipfs'; + export default function isValidURL(url: string) { if (typeof url !== 'string') { return false; } - return isURL(url, { protocols: ['https', 'ipfs'], require_protocol: true }); + // isURL applies an FQDN check to the host, which every ipfs:// URI + // fails (a CID has no top-level domain), so listing 'ipfs' as an allowed + // protocol is not enough. Validate the HTTPS gateway form instead — it is + // also the URL the network layer will actually request. + return isURL(ipfsToGatewayUrl(url), { protocols: ['https'], require_protocol: true }); } diff --git a/packages/gui/src/util/ipfs.test.ts b/packages/gui/src/util/ipfs.test.ts new file mode 100644 index 0000000000..a802405544 --- /dev/null +++ b/packages/gui/src/util/ipfs.test.ts @@ -0,0 +1,53 @@ +import ipfsToGatewayUrl, { IPFS_GATEWAY_BASE, getIpfsPath, isIpfsUrl } from './ipfs'; + +// CID taken from a real mainnet NFT whose on-chain data URI is ipfs:// +const CID_V1 = 'bafybeiceg2gltyhlkukwetn26k7t2zdvthg4u4c6uj23rpni2adzgvo5si'; +const CID_V0 = 'QmPK1s3pNYLi9ERiq3BDxKa4XosgWwFRQUydHUtz4YgpqB'; + +describe('isIpfsUrl', () => { + it('matches the ipfs scheme case-insensitively', () => { + expect(isIpfsUrl(`ipfs://${CID_V1}/020.png`)).toBe(true); + expect(isIpfsUrl(`IPFS://${CID_V1}`)).toBe(true); + }); + + it('does not match other schemes', () => { + expect(isIpfsUrl(`https://ipfs.io/ipfs/${CID_V1}`)).toBe(false); + expect(isIpfsUrl('')).toBe(false); + }); +}); + +describe('getIpfsPath', () => { + it('returns the CID and path', () => { + expect(getIpfsPath(`ipfs://${CID_V1}/020.png`)).toBe(`${CID_V1}/020.png`); + expect(getIpfsPath(`ipfs://${CID_V1}`)).toBe(CID_V1); + }); + + it('strips the redundant ipfs/ prefix some minting tools produce', () => { + expect(getIpfsPath(`ipfs://ipfs/${CID_V1}/020.png`)).toBe(`${CID_V1}/020.png`); + }); + + it('preserves the case of CIDv0 base58 hashes', () => { + expect(getIpfsPath(`ipfs://${CID_V0}/image.png`)).toBe(`${CID_V0}/image.png`); + }); + + it('returns undefined for non-ipfs URLs and a bare scheme', () => { + expect(getIpfsPath(`https://example.com/${CID_V1}`)).toBeUndefined(); + expect(getIpfsPath('ipfs://')).toBeUndefined(); + }); +}); + +describe('ipfsToGatewayUrl', () => { + it('translates ipfs:// URIs to the HTTPS gateway', () => { + expect(ipfsToGatewayUrl(`ipfs://${CID_V1}/020.png`)).toBe(`${IPFS_GATEWAY_BASE}${CID_V1}/020.png`); + expect(ipfsToGatewayUrl(`ipfs://ipfs/${CID_V1}`)).toBe(`${IPFS_GATEWAY_BASE}${CID_V1}`); + }); + + it('returns non-ipfs URLs unchanged', () => { + const url = 'https://example.com/image.png?size=large'; + expect(ipfsToGatewayUrl(url)).toBe(url); + }); + + it('returns an unusable bare scheme unchanged so validation rejects it', () => { + expect(ipfsToGatewayUrl('ipfs://')).toBe('ipfs://'); + }); +}); diff --git a/packages/gui/src/util/ipfs.ts b/packages/gui/src/util/ipfs.ts new file mode 100644 index 0000000000..e58a5e1304 --- /dev/null +++ b/packages/gui/src/util/ipfs.ts @@ -0,0 +1,33 @@ +// The public HTTPS gateway used to serve ipfs:// resources. Electron's net +// stack has no IPFS support, so ipfs:// URIs are fetched through a gateway. +// The gateway does not need to be trusted for integrity: everything the cache +// serves is checked against the NFT's on-chain hash before it is shown. +export const IPFS_GATEWAY_BASE = 'https://ipfs.io/ipfs/'; + +const IPFS_SCHEME = /^ipfs:\/\//i; + +export function isIpfsUrl(url: string): boolean { + return typeof url === 'string' && IPFS_SCHEME.test(url); +} + +// Returns the `[/path]` part of an ipfs:// URI, tolerating the redundant +// `ipfs://ipfs/` form produced by some minting tools. CIDv0 hashes are +// case-sensitive base58, so the value is never case-normalized. +export function getIpfsPath(url: string): string | undefined { + if (!isIpfsUrl(url)) { + return undefined; + } + + const ipfsPath = url.replace(IPFS_SCHEME, '').replace(/^ipfs\//i, ''); + + return ipfsPath.length > 0 ? ipfsPath : undefined; +} + +// Translates an ipfs:// URI to its HTTPS gateway equivalent. Anything else +// (including an unusable bare `ipfs://`) is returned unchanged, so this can +// wrap any URL right where it reaches the network layer. +export default function ipfsToGatewayUrl(url: string): string { + const ipfsPath = getIpfsPath(url); + + return ipfsPath === undefined ? url : `${IPFS_GATEWAY_BASE}${ipfsPath}`; +} From f79fe8a3d7426b7d10b40bb1db92cfc4fb9023b5 Mon Sep 17 00:00:00 2001 From: jlobue10 Date: Thu, 20 Aug 2026 16:29:16 +0000 Subject: [PATCH 02/10] Make IPFS gateway fetching a user-selectable option Review feedback on the gateway feature: the gateway URL is technically not the URI recorded on chain, so translating ipfs:// URIs to https://ipfs.io/ipfs/... should be something the user opts into rather than automatic behavior. - New 'Fetch IPFS content through a gateway' switch in Settings > NFT, off by default. While off, ipfs:// URIs behave as before the gateway feature: they fail URL validation and are never fetched, and the NFTHashStatus badge flags them again. - The preference is stored as nftIpfsGateway via the existing prefs.yaml round-trip; the main process reads the persisted value through electron/utils/ipfsGateway.ts (fail-closed when the store is unreadable) at every site that translated URLs: isValidURL, downloadFile, fetchBuffer, fetchJSON, the single-NFT download handler, and the dapp dialog's oversized-image fallback. The ipfs scheme check runs before the preference read so non-ipfs requests never touch the store. - The oversized-image fallback returns no preview (instead of a CSP-blocked raw ipfs URI) while the option is off. - Nothing is persisted for a rejected ipfs URL (the outer isValidURL guard throws before the cache-info error sidecar is written), so enabling the option retries previously failing NFTs cleanly. - util/ipfs.ts stays a pure translation helper shared with the renderer; the preference gate lives only in the electron layer and the useIpfsGateway hook. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01LdfCqRSBWwMpCDh1SdE24e --- .../gui/src/components/nfts/NFTHashStatus.tsx | 12 ++-- .../src/components/settings/SettingsNFT.tsx | 26 ++++++++ .../src/electron/api/nftGetMetadata.test.ts | 40 ++++++++++- .../gui/src/electron/api/nftGetMetadata.ts | 9 ++- packages/gui/src/electron/main.tsx | 9 +-- .../gui/src/electron/utils/downloadFile.ts | 11 ++-- .../gui/src/electron/utils/fetchBuffer.ts | 8 +-- packages/gui/src/electron/utils/fetchJSON.ts | 9 ++- .../src/electron/utils/ipfsGateway.test.ts | 66 +++++++++++++++++++ .../gui/src/electron/utils/ipfsGateway.ts | 32 +++++++++ .../gui/src/electron/utils/isValidURL.test.ts | 25 ++++++- packages/gui/src/electron/utils/isValidURL.ts | 10 +-- packages/gui/src/hooks/useIpfsGateway.ts | 13 ++++ 13 files changed, 238 insertions(+), 32 deletions(-) create mode 100644 packages/gui/src/electron/utils/ipfsGateway.test.ts create mode 100644 packages/gui/src/electron/utils/ipfsGateway.ts create mode 100644 packages/gui/src/hooks/useIpfsGateway.ts diff --git a/packages/gui/src/components/nfts/NFTHashStatus.tsx b/packages/gui/src/components/nfts/NFTHashStatus.tsx index 367a6d4cda..ecf3aea1c1 100644 --- a/packages/gui/src/components/nfts/NFTHashStatus.tsx +++ b/packages/gui/src/components/nfts/NFTHashStatus.tsx @@ -6,6 +6,7 @@ import { Chip, Typography } from '@mui/material'; import CircularProgress from '@mui/material/CircularProgress'; import React, { useMemo } from 'react'; +import useIpfsGateway from '../../hooks/useIpfsGateway'; import useNFT from '../../hooks/useNFT'; import useNFTVerifyHash from '../../hooks/useNFTVerifyHash'; import ipfsToGatewayUrl from '../../util/ipfs'; @@ -28,6 +29,7 @@ export default function NFTHashStatus(props: NFTHashStatusProps) { }); const { nft, isLoading: isLoadingNFT, error: errorNFT } = useNFT(nftId); + const [ipfsGateway] = useIpfsGateway(); const isLoading = isLoadingNFTVerifyHash || isLoadingNFT; const isVerified = preview ? nftPreview?.isVerified : data?.isVerified; @@ -40,13 +42,15 @@ export default function NFTHashStatus(props: NFTHashStatusProps) { } if (nftPreview.uri) { - // ipfs:// URIs are served through an HTTPS gateway by the cache layer, - // so validate the gateway form instead of flagging them as invalid. - return isValidURL(ipfsToGatewayUrl(nftPreview.uri)); + // While the user has IPFS gateway fetching enabled, ipfs:// URIs are + // served through an HTTPS gateway by the cache layer, so validate the + // gateway form instead of flagging them as invalid. With the option + // off they are not fetchable and stay flagged. + return isValidURL(ipfsGateway ? ipfsToGatewayUrl(nftPreview.uri) : nftPreview.uri); } return false; - }, [nftPreview]); + }, [nftPreview, ipfsGateway]); const icon = useMemo(() => { if (hideIcon) { diff --git a/packages/gui/src/components/settings/SettingsNFT.tsx b/packages/gui/src/components/settings/SettingsNFT.tsx index db3e58185a..f793e9bfa8 100644 --- a/packages/gui/src/components/settings/SettingsNFT.tsx +++ b/packages/gui/src/components/settings/SettingsNFT.tsx @@ -17,6 +17,7 @@ import React from 'react'; import useAllowUnverifiedNFTPreviews from '../../hooks/useAllowUnverifiedNFTPreviews'; import useCache from '../../hooks/useCache'; import useHideObjectionableContent from '../../hooks/useHideObjectionableContent'; +import useIpfsGateway from '../../hooks/useIpfsGateway'; import useNFTImageFittingMode from '../../hooks/useNFTImageFittingMode'; import { useNFTVideoLoopGlobal } from '../../hooks/useNFTVideoLoop'; @@ -41,6 +42,7 @@ export default function SettingsGeneral() { const { cacheSize, clearCache, cacheDirectory, setCacheDirectory } = useCache(); const [nftImageFittingMode, setNFTImageFittingMode] = useNFTImageFittingMode(); const [nftVideoLoop, setNFTVideoLoop] = useNFTVideoLoopGlobal(); + const [ipfsGateway, setIpfsGateway] = useIpfsGateway(); const [allowUnverifiedPreviews, setAllowUnverifiedPreviews] = useAllowUnverifiedNFTPreviews(); // const [, setCacheFolder] = usePrefs('cacheFolder', ''); const openDialog = useOpenDialog(); @@ -53,6 +55,10 @@ export default function SettingsGeneral() { setNFTVideoLoop(event.target.checked); } + function handleChangeIpfsGateway(event: React.ChangeEvent) { + setIpfsGateway(event.target.checked); + } + function handleChangeAllowUnverifiedPreviews(event: React.ChangeEvent) { setAllowUnverifiedPreviews(event.target.checked); } @@ -150,6 +156,26 @@ export default function SettingsGeneral() { + + + + Fetch IPFS content through a gateway + + + + } /> + + + + + NFT files published with ipfs:// addresses will be downloaded through the public ipfs.io HTTPS gateway. + The requested URL differs from the address recorded on chain, but downloaded content is still verified + against the NFT's on-chain hash. When disabled, ipfs:// files are not fetched. + + + + + diff --git a/packages/gui/src/electron/api/nftGetMetadata.test.ts b/packages/gui/src/electron/api/nftGetMetadata.test.ts index 7a1f30dd2f..4c481ef17d 100644 --- a/packages/gui/src/electron/api/nftGetMetadata.test.ts +++ b/packages/gui/src/electron/api/nftGetMetadata.test.ts @@ -11,6 +11,13 @@ jest.mock('../utils/fetchBuffer', () => ({ jest.requireActual('../utils/fetchBuffer').MaxSizeExceededError, })); +const mockMaybeIpfsToGatewayUrl = jest.fn(); + +jest.mock('../utils/ipfsGateway', () => ({ + __esModule: true, + default: mockMaybeIpfsToGatewayUrl, +})); + const mockAllowUnverifiedNftPreviews = jest.fn(); jest.mock('../utils/allowUnverifiedNftPreviews', () => ({ @@ -20,6 +27,8 @@ jest.mock('../utils/allowUnverifiedNftPreviews', () => ({ const { MaxSizeExceededError } = jest.requireActual('../utils/fetchBuffer'); +const ipfsToGatewayUrl = jest.requireActual('../../util/ipfs').default; + const { nftGetImageDataUrl, nftGetMetadata } = jest.requireActual('./nftGetMetadata'); @@ -66,6 +75,9 @@ describe('nftGetMetadata', () => { describe('nftGetImageDataUrl', () => { beforeEach(() => { mockFetchBuffer.mockReset(); + mockMaybeIpfsToGatewayUrl.mockReset(); + // gateway option off: URLs pass through untranslated + mockMaybeIpfsToGatewayUrl.mockImplementation((url) => url); mockAllowUnverifiedNftPreviews.mockReset(); mockAllowUnverifiedNftPreviews.mockReturnValue(false); }); @@ -124,8 +136,9 @@ describe('nftGetImageDataUrl', () => { ); }); - it('falls back to the gateway URL for an oversized ipfs image when unverified previews are enabled', async () => { + it('falls back to the gateway URL for an oversized ipfs image when both options are on', async () => { mockAllowUnverifiedNftPreviews.mockReturnValue(true); + mockMaybeIpfsToGatewayUrl.mockImplementation(ipfsToGatewayUrl); mockFetchBuffer.mockRejectedValue( new MaxSizeExceededError({ 'content-type': 'image/gif', @@ -139,6 +152,31 @@ describe('nftGetImageDataUrl', () => { ); }); + it('omits the preview for an oversized ipfs image while the gateway option is off', async () => { + mockAllowUnverifiedNftPreviews.mockReturnValue(true); + mockFetchBuffer.mockRejectedValue( + new MaxSizeExceededError({ + 'content-type': 'image/gif', + }), + ); + + // an untranslated ipfs URI would be blocked by the dialog CSP, so no + // preview is returned at all even though unverified previews are allowed + await expect(nftGetImageDataUrl('ipfs://bafybeigdyrztest/large.gif', '00')).resolves.toBeUndefined(); + }); + + it('omits the preview for an oversized ipfs image while unverified previews are off', async () => { + mockMaybeIpfsToGatewayUrl.mockImplementation(ipfsToGatewayUrl); + mockFetchBuffer.mockRejectedValue( + new MaxSizeExceededError({ + 'content-type': 'image/gif', + }), + ); + + // the gateway option alone does not opt into unverified fallbacks + await expect(nftGetImageDataUrl('ipfs://bafybeigdyrztest/large.gif', '00')).resolves.toBeUndefined(); + }); + it('rejects an oversized response that is not an image even when unverified previews are enabled', async () => { mockAllowUnverifiedNftPreviews.mockReturnValue(true); mockFetchBuffer.mockRejectedValue( diff --git a/packages/gui/src/electron/api/nftGetMetadata.ts b/packages/gui/src/electron/api/nftGetMetadata.ts index 099955a027..f6c4cf38fb 100644 --- a/packages/gui/src/electron/api/nftGetMetadata.ts +++ b/packages/gui/src/electron/api/nftGetMetadata.ts @@ -2,9 +2,10 @@ import crypto from 'node:crypto'; import type Headers from '../../@types/Headers'; import compareChecksums from '../../util/compareChecksums'; -import ipfsToGatewayUrl from '../../util/ipfs'; +import { isIpfsUrl } from '../../util/ipfs'; import allowUnverifiedNftPreviews from '../utils/allowUnverifiedNftPreviews'; import fetchBuffer, { MaxSizeExceededError } from '../utils/fetchBuffer'; +import maybeIpfsToGatewayUrl from '../utils/ipfsGateway'; const METADATA_TIMEOUT = 10_000; const METADATA_MAX_SIZE = 5 * 1024 * 1024; @@ -105,9 +106,11 @@ export async function nftGetImageDataUrl( // claims are attacker-controlled, so the fallback can be triggered // deliberately to place unverified content in a confirmation dialog. // The CSP does not allow the ipfs: scheme either, so ipfs URIs fall back - // to their gateway form. + // to their gateway form, and only when the user has also enabled the + // gateway — otherwise they get no preview rather than a CSP-blocked URL. if (error instanceof MaxSizeExceededError && getImageContentType(error.headers) && allowUnverifiedNftPreviews()) { - return ipfsToGatewayUrl(imageUri); + const directUrl = maybeIpfsToGatewayUrl(imageUri); + return isIpfsUrl(directUrl) ? undefined : directUrl; } // image previews are best effort — the confirmation dialog has a fallback diff --git a/packages/gui/src/electron/main.tsx b/packages/gui/src/electron/main.tsx index 5b70a3a4e8..6758864bf2 100644 --- a/packages/gui/src/electron/main.tsx +++ b/packages/gui/src/electron/main.tsx @@ -29,7 +29,6 @@ import type { PermissionsNotificationPayload } from '../@types/PermissionsServic import { WcError, WcErrorCode, encodeWcErrorForIpc } from '../@types/WcError'; import AppIcon from '../assets/img/chia64x64.png'; import { i18n } from '../config/locales'; -import ipfsToGatewayUrl from '../util/ipfs'; import CacheManager, { CACHE_PROTOCOL } from './CacheManager'; import { checkNFTOwnership } from './api/checkNFTOwnership'; @@ -62,6 +61,7 @@ import { dispatchPairRequest } from './utils/dispatchPairRequest'; import downloadFile from './utils/downloadFile'; import fetchJSON from './utils/fetchJSON'; import ipcMainHandle from './utils/ipcMainHandle'; +import maybeIpfsToGatewayUrl from './utils/ipfsGateway'; import isValidURL from './utils/isValidURL'; import { loadConfig, checkConfigFileExists } from './utils/loadConfig'; import { getDefaultLogPath, LogPathValidationError, resolveTrustedLogPath } from './utils/logPath'; @@ -576,9 +576,10 @@ if (ensureSingleInstance() && ensureCorrectEnvironment()) { return; } - // Chromium's downloader cannot fetch the ipfs: scheme; download ipfs - // URIs through the HTTPS gateway like every other network path. - mainWindow.webContents.downloadURL(ipfsToGatewayUrl(urlLocal)); + // Chromium's downloader cannot fetch the ipfs: scheme; when the user + // has enabled the gateway, download ipfs URIs through it like every + // other network path. + mainWindow.webContents.downloadURL(maybeIpfsToGatewayUrl(urlLocal)); }); ipcMainHandle(AppAPI.START_MULTIPLE_DOWNLOAD, async (tasks: { url: string; filename: string }[]) => { diff --git a/packages/gui/src/electron/utils/downloadFile.ts b/packages/gui/src/electron/utils/downloadFile.ts index 960c89c071..19159cda18 100644 --- a/packages/gui/src/electron/utils/downloadFile.ts +++ b/packages/gui/src/electron/utils/downloadFile.ts @@ -4,9 +4,9 @@ import { promises as fs, createWriteStream, type WriteStream } from 'node:fs'; import debug from 'debug'; import type Headers from '../../@types/Headers'; -import ipfsToGatewayUrl from '../../util/ipfs'; import fileExists from './fileExists'; +import maybeIpfsToGatewayUrl from './ipfsGateway'; import isValidURL from './isValidURL'; const log = debug('chia-gui:downloadFile'); @@ -101,10 +101,11 @@ export default async function downloadFile( } const tempFilePath = `${localPath}.tmp`; - // ipfs:// URIs are fetched through an HTTPS gateway — Electron's net stack - // cannot request the ipfs scheme. Only this outgoing request uses the - // translated URL; callers keep the original URI as the cache key. - const request = net.request(ipfsToGatewayUrl(url)); + // ipfs:// URIs are fetched through an HTTPS gateway when the user has + // enabled it — Electron's net stack cannot request the ipfs scheme. Only + // this outgoing request uses the translated URL; callers keep the original + // URI as the cache key. + const request = net.request(maybeIpfsToGatewayUrl(url)); const outputStream = new WriteStreamPromise(tempFilePath, overrideFile); // set when we abort the request ourselves, so abort events can be reported diff --git a/packages/gui/src/electron/utils/fetchBuffer.ts b/packages/gui/src/electron/utils/fetchBuffer.ts index 9c52960425..686d5e152a 100644 --- a/packages/gui/src/electron/utils/fetchBuffer.ts +++ b/packages/gui/src/electron/utils/fetchBuffer.ts @@ -1,8 +1,8 @@ import { net, type IncomingMessage } from 'electron'; import type Headers from '../../@types/Headers'; -import ipfsToGatewayUrl from '../../util/ipfs'; +import maybeIpfsToGatewayUrl from './ipfsGateway'; import isValidURL from './isValidURL'; const DEFAULT_TIMEOUT = 10 * 60 * 1000; // 10 minutes @@ -42,9 +42,9 @@ export default async function fetchBuffer( const request = net.request({ method: 'GET', - // ipfs:// URIs are fetched through an HTTPS gateway — Electron's net - // stack cannot request the ipfs scheme. - url: ipfsToGatewayUrl(url), + // ipfs:// URIs are fetched through an HTTPS gateway when the user has + // enabled it — Electron's net stack cannot request the ipfs scheme. + url: maybeIpfsToGatewayUrl(url), headers, }); diff --git a/packages/gui/src/electron/utils/fetchJSON.ts b/packages/gui/src/electron/utils/fetchJSON.ts index 9bd44b3c90..1149069fb7 100644 --- a/packages/gui/src/electron/utils/fetchJSON.ts +++ b/packages/gui/src/electron/utils/fetchJSON.ts @@ -1,7 +1,6 @@ import { net, IncomingMessage } from 'electron'; -import ipfsToGatewayUrl from '../../util/ipfs'; - +import maybeIpfsToGatewayUrl from './ipfsGateway'; import isValidURL from './isValidURL'; const DEFAULT_TIMEOUT = 10 * 60 * 1000; // 10 minutes @@ -19,9 +18,9 @@ export default async function fetchJSON( const request = net.request({ method, - // ipfs:// URIs are fetched through an HTTPS gateway — Electron's net - // stack cannot request the ipfs scheme. - url: ipfsToGatewayUrl(url), + // ipfs:// URIs are fetched through an HTTPS gateway when the user has + // enabled it — Electron's net stack cannot request the ipfs scheme. + url: maybeIpfsToGatewayUrl(url), headers, }); diff --git a/packages/gui/src/electron/utils/ipfsGateway.test.ts b/packages/gui/src/electron/utils/ipfsGateway.test.ts new file mode 100644 index 0000000000..4ef6da6db9 --- /dev/null +++ b/packages/gui/src/electron/utils/ipfsGateway.test.ts @@ -0,0 +1,66 @@ +const mockReadPrefs = jest.fn, []>(); + +jest.mock('../prefs', () => ({ + readPrefs: mockReadPrefs, +})); + +const { + default: maybeIpfsToGatewayUrl, + ipfsGatewayEnabled, + NFT_IPFS_GATEWAY_PREF, +} = jest.requireActual('./ipfsGateway'); + +describe('ipfsGatewayEnabled', () => { + beforeEach(() => { + mockReadPrefs.mockReset(); + }); + + it('is disabled when the preference has never been set', () => { + mockReadPrefs.mockReturnValue({}); + + expect(ipfsGatewayEnabled()).toBe(false); + }); + + it('is enabled only by an explicit boolean true', () => { + mockReadPrefs.mockReturnValue({ [NFT_IPFS_GATEWAY_PREF]: true }); + expect(ipfsGatewayEnabled()).toBe(true); + + mockReadPrefs.mockReturnValue({ [NFT_IPFS_GATEWAY_PREF]: 'true' }); + expect(ipfsGatewayEnabled()).toBe(false); + }); + + it('fails closed when the preferences store cannot be read', () => { + mockReadPrefs.mockImplementation(() => { + throw new Error('userDataDir needs to be initialized'); + }); + + expect(ipfsGatewayEnabled()).toBe(false); + }); +}); + +describe('maybeIpfsToGatewayUrl', () => { + beforeEach(() => { + mockReadPrefs.mockReset(); + }); + + it('never consults the preferences store for non-ipfs URLs', () => { + expect(maybeIpfsToGatewayUrl('https://example.com/image.png')).toBe('https://example.com/image.png'); + expect(mockReadPrefs).not.toHaveBeenCalled(); + }); + + it('leaves ipfs URIs untranslated while the gateway option is off', () => { + mockReadPrefs.mockReturnValue({}); + + expect(maybeIpfsToGatewayUrl('ipfs://QmPK1s3pNYLi9ERiq3BDxKa4XosgWwFRQUydHUtz4YgpqB')).toBe( + 'ipfs://QmPK1s3pNYLi9ERiq3BDxKa4XosgWwFRQUydHUtz4YgpqB', + ); + }); + + it('translates ipfs URIs when the gateway option is on', () => { + mockReadPrefs.mockReturnValue({ [NFT_IPFS_GATEWAY_PREF]: true }); + + expect(maybeIpfsToGatewayUrl('ipfs://QmPK1s3pNYLi9ERiq3BDxKa4XosgWwFRQUydHUtz4YgpqB/img.png')).toBe( + 'https://ipfs.io/ipfs/QmPK1s3pNYLi9ERiq3BDxKa4XosgWwFRQUydHUtz4YgpqB/img.png', + ); + }); +}); diff --git a/packages/gui/src/electron/utils/ipfsGateway.ts b/packages/gui/src/electron/utils/ipfsGateway.ts new file mode 100644 index 0000000000..15c3a0eb05 --- /dev/null +++ b/packages/gui/src/electron/utils/ipfsGateway.ts @@ -0,0 +1,32 @@ +import ipfsToGatewayUrl, { isIpfsUrl } from '../../util/ipfs'; +import { readPrefs } from '../prefs'; + +// Preference key shared with the renderer's useIpfsGateway hook. The renderer +// persists it through PreferencesAPI.SAVE into prefs.yaml, which is the copy +// consulted here in the main process. +export const NFT_IPFS_GATEWAY_PREF = 'nftIpfsGateway'; + +// Whether ipfs:// NFT resources may be fetched through the public HTTPS +// gateway. Off by default: the gateway URL is not the URI recorded on chain, +// so the translation is a user-selectable opt-in. Fails closed when the +// preferences store is unreadable (e.g. before userData is initialized). +export function ipfsGatewayEnabled(): boolean { + try { + return readPrefs()[NFT_IPFS_GATEWAY_PREF] === true; + } catch { + return false; + } +} + +// Translates an ipfs:// URI to its HTTPS gateway equivalent only when the +// user has enabled gateway fetching; every other URL — and every ipfs URI +// while the option is off — is returned unchanged, so this can wrap any URL +// right where it reaches the network layer. The ipfs check runs first so the +// hot non-ipfs paths never touch the preferences store. +export default function maybeIpfsToGatewayUrl(url: string): string { + if (!isIpfsUrl(url) || !ipfsGatewayEnabled()) { + return url; + } + + return ipfsToGatewayUrl(url); +} diff --git a/packages/gui/src/electron/utils/isValidURL.test.ts b/packages/gui/src/electron/utils/isValidURL.test.ts index e08e98eb39..66d8ff5b18 100644 --- a/packages/gui/src/electron/utils/isValidURL.test.ts +++ b/packages/gui/src/electron/utils/isValidURL.test.ts @@ -1,6 +1,18 @@ -import isValidURL from './isValidURL'; +const mockReadPrefs = jest.fn, []>(); + +jest.mock('../prefs', () => ({ + readPrefs: mockReadPrefs, +})); + +const isValidURL = jest.requireActual('./isValidURL').default; +const { NFT_IPFS_GATEWAY_PREF } = jest.requireActual('./ipfsGateway'); describe('isValidURL', () => { + beforeEach(() => { + mockReadPrefs.mockReset(); + mockReadPrefs.mockReturnValue({}); + }); + it('accepts https URLs', () => { expect(isValidURL('https://example.com/image.png')).toBe(true); }); @@ -11,14 +23,23 @@ describe('isValidURL', () => { expect(isValidURL('ftp://example.com/image.png')).toBe(false); }); - it('accepts ipfs:// URIs with a CID host', () => { + it('accepts ipfs:// URIs with a CID host when the gateway option is on', () => { + mockReadPrefs.mockReturnValue({ [NFT_IPFS_GATEWAY_PREF]: true }); + // validator's isURL rejects CID hosts (no TLD), so these pass only via // the gateway translation expect(isValidURL('ipfs://bafybeiceg2gltyhlkukwetn26k7t2zdvthg4u4c6uj23rpni2adzgvo5si/020.png')).toBe(true); expect(isValidURL('ipfs://ipfs/QmPK1s3pNYLi9ERiq3BDxKa4XosgWwFRQUydHUtz4YgpqB')).toBe(true); }); + it('rejects ipfs:// URIs while the gateway option is off', () => { + expect(isValidURL('ipfs://bafybeiceg2gltyhlkukwetn26k7t2zdvthg4u4c6uj23rpni2adzgvo5si/020.png')).toBe(false); + expect(isValidURL('ipfs://ipfs/QmPK1s3pNYLi9ERiq3BDxKa4XosgWwFRQUydHUtz4YgpqB')).toBe(false); + }); + it('rejects a bare ipfs scheme and non-strings', () => { + mockReadPrefs.mockReturnValue({ [NFT_IPFS_GATEWAY_PREF]: true }); + expect(isValidURL('ipfs://')).toBe(false); expect(isValidURL(undefined as unknown as string)).toBe(false); }); diff --git a/packages/gui/src/electron/utils/isValidURL.ts b/packages/gui/src/electron/utils/isValidURL.ts index c1b4c1c38d..064649f6aa 100644 --- a/packages/gui/src/electron/utils/isValidURL.ts +++ b/packages/gui/src/electron/utils/isValidURL.ts @@ -1,6 +1,6 @@ import isURL from 'validator/lib/isURL'; -import ipfsToGatewayUrl from '../../util/ipfs'; +import maybeIpfsToGatewayUrl from './ipfsGateway'; export default function isValidURL(url: string) { if (typeof url !== 'string') { @@ -9,7 +9,9 @@ export default function isValidURL(url: string) { // isURL applies an FQDN check to the host, which every ipfs:// URI // fails (a CID has no top-level domain), so listing 'ipfs' as an allowed - // protocol is not enough. Validate the HTTPS gateway form instead — it is - // also the URL the network layer will actually request. - return isURL(ipfsToGatewayUrl(url), { protocols: ['https'], require_protocol: true }); + // protocol is not enough. When the user has enabled gateway fetching, + // validate the HTTPS gateway form instead — it is also the URL the network + // layer will actually request; while the option is off, ipfs URIs stay + // invalid and are never fetched. + return isURL(maybeIpfsToGatewayUrl(url), { protocols: ['https'], require_protocol: true }); } diff --git a/packages/gui/src/hooks/useIpfsGateway.ts b/packages/gui/src/hooks/useIpfsGateway.ts new file mode 100644 index 0000000000..fcdbaed063 --- /dev/null +++ b/packages/gui/src/hooks/useIpfsGateway.ts @@ -0,0 +1,13 @@ +import { usePrefs } from '@chia-network/api-react'; + +// When enabled, NFT resources published with ipfs:// URIs are fetched through +// the public HTTPS gateway; the requested URL then differs from the URI +// recorded on chain, which is why this is a user-selectable opt-in (off by +// default — ipfs:// resources are simply not fetched). Downloaded content is +// still verified against the NFT's on-chain hash either way. The main process +// reads the persisted copy of this preference at every network call site +// (electron/utils/ipfsGateway.ts); keep the key in sync with +// NFT_IPFS_GATEWAY_PREF there. +export default function useIpfsGateway() { + return usePrefs('nftIpfsGateway', false); +} From 8a453be8e380ccd266e743545bb595697b74484d Mon Sep 17 00:00:00 2001 From: Jonathan LoBue Date: Thu, 20 Aug 2026 13:35:17 -0700 Subject: [PATCH 03/10] Gate only IPFS fetching on the gateway option, not cached content MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit isValidURL treated ipfs:// URIs as invalid whenever the gateway option was off, and CacheManager consults that check before every cache path lookup. Content that was downloaded and hash-verified while the option was on therefore became unservable the moment it was switched off — the cached bytes could not be served, checksummed, or evicted even though serving a local file involves no gateway request (Bugbot, PR #3029). - isValidURL is now a structural check only: ipfs URIs are validated via their gateway form regardless of the preference. - Fetching is gated where it happens instead: downloadFile, fetchBuffer, and fetchJSON resolve their request URL through a new toFetchableUrl, which throws IpfsGatewayDisabledError for ipfs URIs while the option is off. - CacheManager rethrows that error instead of persisting it as a cache ERROR entry, so flipping the option on retries cleanly - a persisted 'disabled' error would have poisoned the entry (only transient errors are ever retried). - The single-download IPC handler drops ipfs URLs while the option is off instead of handing Chromium a URL it silently fails on. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_017NXWAjaHb9SFafLeguTHd8 --- packages/gui/src/electron/CacheManager.ts | 10 +++++++ packages/gui/src/electron/main.tsx | 12 ++++++-- .../gui/src/electron/utils/downloadFile.ts | 7 +++-- .../gui/src/electron/utils/fetchBuffer.ts | 7 +++-- packages/gui/src/electron/utils/fetchJSON.ts | 7 +++-- .../src/electron/utils/ipfsGateway.test.ts | 29 +++++++++++++++++++ .../gui/src/electron/utils/ipfsGateway.ts | 24 +++++++++++++++ .../gui/src/electron/utils/isValidURL.test.ts | 18 ++++-------- packages/gui/src/electron/utils/isValidURL.ts | 17 +++++++---- 9 files changed, 102 insertions(+), 29 deletions(-) diff --git a/packages/gui/src/electron/CacheManager.ts b/packages/gui/src/electron/CacheManager.ts index ef22205f71..4e5e595720 100644 --- a/packages/gui/src/electron/CacheManager.ts +++ b/packages/gui/src/electron/CacheManager.ts @@ -19,6 +19,7 @@ import downloadFile, { MAX_FILE_SIZE_EXCEEDED_ERROR } from './utils/downloadFile import ensureDirectoryExists from './utils/ensureDirectoryExists'; import getChecksum from './utils/getChecksum'; import ipcMainHandle from './utils/ipcMainHandle'; +import { IpfsGatewayDisabledError } from './utils/ipfsGateway'; import isValidURL from './utils/isValidURL'; import sanitizeFilename from './utils/sanitizeFilename'; import sanitizeNumber from './utils/sanitizeNumber'; @@ -506,6 +507,15 @@ export default class CacheManager extends EventEmitter { return await this.#downloadLimit(() => limitedRemoteFileDownload()); } catch (error) { + // Not a property of the URL, just of the current preference: while + // the IPFS gateway option is off the fetch is refused before it + // starts. Persisting that as a cache error would keep the entry + // poisoned after the user turns the option on, so it propagates + // instead — already-cached content was served above regardless. + if (error instanceof IpfsGatewayDisabledError) { + throw error; + } + const currentError = (error as Error) ?? new Error('Unknown fetchRemoteContent error'); return await this.setCacheInfo(url, { diff --git a/packages/gui/src/electron/main.tsx b/packages/gui/src/electron/main.tsx index 6758864bf2..c48ca8dad9 100644 --- a/packages/gui/src/electron/main.tsx +++ b/packages/gui/src/electron/main.tsx @@ -29,6 +29,7 @@ import type { PermissionsNotificationPayload } from '../@types/PermissionsServic import { WcError, WcErrorCode, encodeWcErrorForIpc } from '../@types/WcError'; import AppIcon from '../assets/img/chia64x64.png'; import { i18n } from '../config/locales'; +import { isIpfsUrl } from '../util/ipfs'; import CacheManager, { CACHE_PROTOCOL } from './CacheManager'; import { checkNFTOwnership } from './api/checkNFTOwnership'; @@ -578,8 +579,15 @@ if (ensureSingleInstance() && ensureCorrectEnvironment()) { // Chromium's downloader cannot fetch the ipfs: scheme; when the user // has enabled the gateway, download ipfs URIs through it like every - // other network path. - mainWindow.webContents.downloadURL(maybeIpfsToGatewayUrl(urlLocal)); + // other network path. With the option off there is nothing the + // downloader could fetch, so the request is dropped instead of handing + // Chromium a URL it silently fails on. + const downloadUrl = maybeIpfsToGatewayUrl(urlLocal); + if (isIpfsUrl(downloadUrl)) { + return; + } + + mainWindow.webContents.downloadURL(downloadUrl); }); ipcMainHandle(AppAPI.START_MULTIPLE_DOWNLOAD, async (tasks: { url: string; filename: string }[]) => { diff --git a/packages/gui/src/electron/utils/downloadFile.ts b/packages/gui/src/electron/utils/downloadFile.ts index 19159cda18..d69d5b679f 100644 --- a/packages/gui/src/electron/utils/downloadFile.ts +++ b/packages/gui/src/electron/utils/downloadFile.ts @@ -6,7 +6,7 @@ import debug from 'debug'; import type Headers from '../../@types/Headers'; import fileExists from './fileExists'; -import maybeIpfsToGatewayUrl from './ipfsGateway'; +import { toFetchableUrl } from './ipfsGateway'; import isValidURL from './isValidURL'; const log = debug('chia-gui:downloadFile'); @@ -102,10 +102,11 @@ export default async function downloadFile( const tempFilePath = `${localPath}.tmp`; // ipfs:// URIs are fetched through an HTTPS gateway when the user has - // enabled it — Electron's net stack cannot request the ipfs scheme. Only + // enabled it — Electron's net stack cannot request the ipfs scheme, and + // with the option off toFetchableUrl refuses the fetch outright. Only // this outgoing request uses the translated URL; callers keep the original // URI as the cache key. - const request = net.request(maybeIpfsToGatewayUrl(url)); + const request = net.request(toFetchableUrl(url)); const outputStream = new WriteStreamPromise(tempFilePath, overrideFile); // set when we abort the request ourselves, so abort events can be reported diff --git a/packages/gui/src/electron/utils/fetchBuffer.ts b/packages/gui/src/electron/utils/fetchBuffer.ts index 686d5e152a..0efef32118 100644 --- a/packages/gui/src/electron/utils/fetchBuffer.ts +++ b/packages/gui/src/electron/utils/fetchBuffer.ts @@ -2,7 +2,7 @@ import { net, type IncomingMessage } from 'electron'; import type Headers from '../../@types/Headers'; -import maybeIpfsToGatewayUrl from './ipfsGateway'; +import { toFetchableUrl } from './ipfsGateway'; import isValidURL from './isValidURL'; const DEFAULT_TIMEOUT = 10 * 60 * 1000; // 10 minutes @@ -43,8 +43,9 @@ export default async function fetchBuffer( const request = net.request({ method: 'GET', // ipfs:// URIs are fetched through an HTTPS gateway when the user has - // enabled it — Electron's net stack cannot request the ipfs scheme. - url: maybeIpfsToGatewayUrl(url), + // enabled it — Electron's net stack cannot request the ipfs scheme, and + // with the option off toFetchableUrl refuses the fetch outright. + url: toFetchableUrl(url), headers, }); diff --git a/packages/gui/src/electron/utils/fetchJSON.ts b/packages/gui/src/electron/utils/fetchJSON.ts index 1149069fb7..df5465e527 100644 --- a/packages/gui/src/electron/utils/fetchJSON.ts +++ b/packages/gui/src/electron/utils/fetchJSON.ts @@ -1,6 +1,6 @@ import { net, IncomingMessage } from 'electron'; -import maybeIpfsToGatewayUrl from './ipfsGateway'; +import { toFetchableUrl } from './ipfsGateway'; import isValidURL from './isValidURL'; const DEFAULT_TIMEOUT = 10 * 60 * 1000; // 10 minutes @@ -19,8 +19,9 @@ export default async function fetchJSON( const request = net.request({ method, // ipfs:// URIs are fetched through an HTTPS gateway when the user has - // enabled it — Electron's net stack cannot request the ipfs scheme. - url: maybeIpfsToGatewayUrl(url), + // enabled it — Electron's net stack cannot request the ipfs scheme, and + // with the option off toFetchableUrl refuses the fetch outright. + url: toFetchableUrl(url), headers, }); diff --git a/packages/gui/src/electron/utils/ipfsGateway.test.ts b/packages/gui/src/electron/utils/ipfsGateway.test.ts index 4ef6da6db9..bcdf0b1cf4 100644 --- a/packages/gui/src/electron/utils/ipfsGateway.test.ts +++ b/packages/gui/src/electron/utils/ipfsGateway.test.ts @@ -7,6 +7,8 @@ jest.mock('../prefs', () => ({ const { default: maybeIpfsToGatewayUrl, ipfsGatewayEnabled, + toFetchableUrl, + IpfsGatewayDisabledError, NFT_IPFS_GATEWAY_PREF, } = jest.requireActual('./ipfsGateway'); @@ -64,3 +66,30 @@ describe('maybeIpfsToGatewayUrl', () => { ); }); }); + +describe('toFetchableUrl', () => { + beforeEach(() => { + mockReadPrefs.mockReset(); + }); + + it('passes non-ipfs URLs through without consulting the preferences store', () => { + expect(toFetchableUrl('https://example.com/image.png')).toBe('https://example.com/image.png'); + expect(mockReadPrefs).not.toHaveBeenCalled(); + }); + + it('translates ipfs URIs when the gateway option is on', () => { + mockReadPrefs.mockReturnValue({ [NFT_IPFS_GATEWAY_PREF]: true }); + + expect(toFetchableUrl('ipfs://QmPK1s3pNYLi9ERiq3BDxKa4XosgWwFRQUydHUtz4YgpqB/img.png')).toBe( + 'https://ipfs.io/ipfs/QmPK1s3pNYLi9ERiq3BDxKa4XosgWwFRQUydHUtz4YgpqB/img.png', + ); + }); + + it('refuses ipfs URIs while the gateway option is off', () => { + mockReadPrefs.mockReturnValue({}); + + expect(() => toFetchableUrl('ipfs://QmPK1s3pNYLi9ERiq3BDxKa4XosgWwFRQUydHUtz4YgpqB')).toThrow( + IpfsGatewayDisabledError, + ); + }); +}); diff --git a/packages/gui/src/electron/utils/ipfsGateway.ts b/packages/gui/src/electron/utils/ipfsGateway.ts index 15c3a0eb05..8c06be7269 100644 --- a/packages/gui/src/electron/utils/ipfsGateway.ts +++ b/packages/gui/src/electron/utils/ipfsGateway.ts @@ -30,3 +30,27 @@ export default function maybeIpfsToGatewayUrl(url: string): string { return ipfsToGatewayUrl(url); } + +// Thrown instead of attempting a fetch that cannot happen: with the gateway +// option off there is no URL Electron's net stack could request for an +// ipfs:// URI. CacheManager treats this error as non-persistent — flipping +// the option on must retry cleanly, so it never poisons a cache entry. +export class IpfsGatewayDisabledError extends Error { + constructor() { + super('IPFS gateway fetching is disabled'); + this.name = 'IpfsGatewayDisabledError'; + } +} + +// The URL the network layer may actually request. The gateway option gates +// only fetching: structural URL validation and serving already-cached content +// stay independent of it, so every network call site funnels through here +// instead of checking the option itself. +export function toFetchableUrl(url: string): string { + const requestUrl = maybeIpfsToGatewayUrl(url); + if (isIpfsUrl(requestUrl)) { + throw new IpfsGatewayDisabledError(); + } + + return requestUrl; +} diff --git a/packages/gui/src/electron/utils/isValidURL.test.ts b/packages/gui/src/electron/utils/isValidURL.test.ts index 66d8ff5b18..d9e38a0259 100644 --- a/packages/gui/src/electron/utils/isValidURL.test.ts +++ b/packages/gui/src/electron/utils/isValidURL.test.ts @@ -5,7 +5,6 @@ jest.mock('../prefs', () => ({ })); const isValidURL = jest.requireActual('./isValidURL').default; -const { NFT_IPFS_GATEWAY_PREF } = jest.requireActual('./ipfsGateway'); describe('isValidURL', () => { beforeEach(() => { @@ -23,23 +22,18 @@ describe('isValidURL', () => { expect(isValidURL('ftp://example.com/image.png')).toBe(false); }); - it('accepts ipfs:// URIs with a CID host when the gateway option is on', () => { - mockReadPrefs.mockReturnValue({ [NFT_IPFS_GATEWAY_PREF]: true }); - + it('accepts ipfs:// URIs with a CID host regardless of the gateway option', () => { // validator's isURL rejects CID hosts (no TLD), so these pass only via - // the gateway translation + // the gateway-form translation. The check is structural on purpose: the + // gateway option gates fetching (toFetchableUrl), not validity — cache + // lookups for already-downloaded ipfs content must keep working while + // the option is off. expect(isValidURL('ipfs://bafybeiceg2gltyhlkukwetn26k7t2zdvthg4u4c6uj23rpni2adzgvo5si/020.png')).toBe(true); expect(isValidURL('ipfs://ipfs/QmPK1s3pNYLi9ERiq3BDxKa4XosgWwFRQUydHUtz4YgpqB')).toBe(true); - }); - - it('rejects ipfs:// URIs while the gateway option is off', () => { - expect(isValidURL('ipfs://bafybeiceg2gltyhlkukwetn26k7t2zdvthg4u4c6uj23rpni2adzgvo5si/020.png')).toBe(false); - expect(isValidURL('ipfs://ipfs/QmPK1s3pNYLi9ERiq3BDxKa4XosgWwFRQUydHUtz4YgpqB')).toBe(false); + expect(mockReadPrefs).not.toHaveBeenCalled(); }); it('rejects a bare ipfs scheme and non-strings', () => { - mockReadPrefs.mockReturnValue({ [NFT_IPFS_GATEWAY_PREF]: true }); - expect(isValidURL('ipfs://')).toBe(false); expect(isValidURL(undefined as unknown as string)).toBe(false); }); diff --git a/packages/gui/src/electron/utils/isValidURL.ts b/packages/gui/src/electron/utils/isValidURL.ts index 064649f6aa..85ba088331 100644 --- a/packages/gui/src/electron/utils/isValidURL.ts +++ b/packages/gui/src/electron/utils/isValidURL.ts @@ -1,7 +1,15 @@ import isURL from 'validator/lib/isURL'; -import maybeIpfsToGatewayUrl from './ipfsGateway'; +import ipfsToGatewayUrl, { isIpfsUrl } from '../../util/ipfs'; +// Structural validation only — deliberately independent of the IPFS gateway +// preference. CacheManager consults this check before every cache path +// lookup, so tying it to the preference would strand content that was +// downloaded and hash-verified while the option was on: the cached bytes +// could no longer be served, checksummed, or evicted after switching it off, +// even though serving a local file involves no gateway request. Whether an +// ipfs URI may actually be FETCHED is decided at the network call sites via +// toFetchableUrl (electron/utils/ipfsGateway.ts). export default function isValidURL(url: string) { if (typeof url !== 'string') { return false; @@ -9,9 +17,6 @@ export default function isValidURL(url: string) { // isURL applies an FQDN check to the host, which every ipfs:// URI // fails (a CID has no top-level domain), so listing 'ipfs' as an allowed - // protocol is not enough. When the user has enabled gateway fetching, - // validate the HTTPS gateway form instead — it is also the URL the network - // layer will actually request; while the option is off, ipfs URIs stay - // invalid and are never fetched. - return isURL(maybeIpfsToGatewayUrl(url), { protocols: ['https'], require_protocol: true }); + // protocol is not enough — validate the HTTPS gateway form instead. + return isURL(isIpfsUrl(url) ? ipfsToGatewayUrl(url) : url, { protocols: ['https'], require_protocol: true }); } From 26f6dba211a5da908b9b2b524d3f752c020350fe Mon Sep 17 00:00:00 2001 From: Jonathan LoBue Date: Thu, 20 Aug 2026 13:38:34 -0700 Subject: [PATCH 04/10] React to gateway option changes without a remount Enabling 'Fetch IPFS content through a gateway' had no effect on NFTs already on screen: useNFTVerifyHash never re-ran (nothing depended on the preference), and a failed ipfs metadata fetch stayed cached in the NFT provider, so those NFTs kept looking broken until a full app reload (Bugbot, PR #3029). - Both verification effects in useNFTVerifyHash now list the preference as a dependency, so flipping it re-checks data and preview URIs immediately. - useMetadataData retries cached metadata failures when the preference flips - only failures: successfully fetched metadata is hash-verified content and unaffected by how it was fetched. The retry goes through invalidate, whose refetch notifies mounted subscribers. The cache layer needs no matching change: gateway-disabled fetch refusals are never persisted, so the re-run's fresh requests go through cleanly. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_017NXWAjaHb9SFafLeguTHd8 --- .../nfts/provider/hooks/useMetadataData.ts | 27 ++++++++++++++++++- packages/gui/src/hooks/useNFTVerifyHash.ts | 10 +++++-- 2 files changed, 34 insertions(+), 3 deletions(-) diff --git a/packages/gui/src/components/nfts/provider/hooks/useMetadataData.ts b/packages/gui/src/components/nfts/provider/hooks/useMetadataData.ts index c25ade0370..f55e7ce70f 100644 --- a/packages/gui/src/components/nfts/provider/hooks/useMetadataData.ts +++ b/packages/gui/src/components/nfts/provider/hooks/useMetadataData.ts @@ -2,12 +2,13 @@ import { EventEmitter } from 'events'; import { type NFTInfo } from '@chia-network/api'; import debug from 'debug'; -import { useState, useCallback, useMemo } from 'react'; +import { useState, useCallback, useEffect, useMemo, useRef } from 'react'; import type Metadata from '../../../../@types/Metadata'; import type MetadataOnDemand from '../../../../@types/MetadataOnDemand'; import type MetadataState from '../../../../@types/MetadataState'; import useFetchAndProcessMetadata from '../../../../hooks/useFetchAndProcessMetadata'; +import useIpfsGateway from '../../../../hooks/useIpfsGateway'; import getNFTId from '../../../../util/getNFTId'; const log = debug('chia-gui:NFTProvider:useMetadataData'); @@ -159,6 +160,30 @@ export default function useMetadataData(props: UseMetadataDataProps) { [getMetadata /* immutable */, metadatasOnDemand /* immutable */], ); + const [ipfsGateway] = useIpfsGateway(); + const lastIpfsGatewayRef = useRef(ipfsGateway); + + useEffect(() => { + if (lastIpfsGatewayRef.current === ipfsGateway) { + return; + } + lastIpfsGatewayRef.current = ipfsGateway; + + // Flipping the gateway option changes which URIs the main process will + // fetch, so cached failures are stale — without this, a failed ipfs + // metadata fetch stayed cached here and its NFT kept looking broken + // after enabling the option, until a full app reload. Only failures are + // retried: successfully fetched metadata is hash-verified content and + // unaffected by how it was fetched. + metadatasOnDemand.forEach((metadataOnDemand, nftId) => { + if (metadataOnDemand.error) { + invalidate(nftId).catch((e) => { + log(`Error retrying metadata for nftId: ${nftId}`, e); + }); + } + }); + }, [ipfsGateway, invalidate /* immutable */, metadatasOnDemand /* immutable */]); + // immutable function const subscribeToMetadataChanges = useCallback( (id: string | undefined, callback: (nftState: MetadataState) => void) => { diff --git a/packages/gui/src/hooks/useNFTVerifyHash.ts b/packages/gui/src/hooks/useNFTVerifyHash.ts index 4d54602a1b..4d2790a6dc 100644 --- a/packages/gui/src/hooks/useNFTVerifyHash.ts +++ b/packages/gui/src/hooks/useNFTVerifyHash.ts @@ -7,6 +7,7 @@ import compareChecksums from '../util/compareChecksums'; import selectNFTPreviewState, { type NFTPreviewState } from './selectNFTPreviewState'; import useCache from './useCache'; +import useIpfsGateway from './useIpfsGateway'; import useNFT from './useNFT'; import useNFTMetadata from './useNFTMetadata'; @@ -21,6 +22,11 @@ export default function useNFTVerifyHash(nftId?: string, options: UseNFTVerifyHa const { preview = false, ignoreSizeLimit = false } = options; const { getChecksum } = useCache(); + // Not read directly: the value changes which URIs the main process will + // fetch at all, so both verification effects list it as a dependency and + // re-run when the user flips the option — without this, NFTs already on + // screen would keep their failed state until a remount. + const [ipfsGateway] = useIpfsGateway(); const { nft, isLoading: isLoadingNFT, error: errorNFT } = useNFT(nftId); const { isLoading: isLoadingMetadata, metadata, error: errorMetadata } = useNFTMetadata(nftId); @@ -201,7 +207,7 @@ export default function useNFTVerifyHash(nftId?: string, options: UseNFTVerifyHa dataGeneration.current += 1; } }; - }, [nft, isLoadingNFT, validateData]); + }, [nft, isLoadingNFT, validateData, ipfsGateway]); useEffect(() => { const generation = previewGeneration.current + 1; @@ -227,7 +233,7 @@ export default function useNFTVerifyHash(nftId?: string, options: UseNFTVerifyHa previewGeneration.current += 1; } }; - }, [preview, nft, metadata, isLoadingNFT, isLoadingMetadata, validatePreview]); + }, [preview, nft, metadata, isLoadingNFT, isLoadingMetadata, validatePreview, ipfsGateway]); const previewState = useMemo( () => From 89fa21389aff88338def058581ca6db81c1c6723 Mon Sep 17 00:00:00 2001 From: Jonathan LoBue Date: Thu, 20 Aug 2026 13:38:49 -0700 Subject: [PATCH 05/10] Fix the hash badge's unreachable IPFS URL check The gateway-aware validity check in NFTHashStatus sat behind 'originalUri' in nftPreview, but NFTPreviewState has no originalUri field, so the guard always returned early: isValidURI stayed true, the 'URL is not valid' badge never showed for unfetchable ipfs URIs, and the ipfsToGatewayUrl path was dead code (Bugbot, PR #3029). The check now validates nftPreview.uri directly. Message precedence is unchanged: a file that already verified from the cache still reports 'Hash matches' - the URL branch is only reached for unverified states, which is exactly when an unfetchable URI is the thing worth reporting. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_017NXWAjaHb9SFafLeguTHd8 --- .../gui/src/components/nfts/NFTHashStatus.tsx | 19 +++++++++---------- 1 file changed, 9 insertions(+), 10 deletions(-) diff --git a/packages/gui/src/components/nfts/NFTHashStatus.tsx b/packages/gui/src/components/nfts/NFTHashStatus.tsx index ecf3aea1c1..abed5e7aaa 100644 --- a/packages/gui/src/components/nfts/NFTHashStatus.tsx +++ b/packages/gui/src/components/nfts/NFTHashStatus.tsx @@ -37,19 +37,18 @@ export default function NFTHashStatus(props: NFTHashStatusProps) { const failedFetch = preview ? nftPreview?.failedFetch : data?.failedFetch; const isValidURI = useMemo(() => { - if (!nftPreview || !('originalUri' in nftPreview)) { + const uri = nftPreview?.uri; + if (!uri) { + // nothing to validate — other branches cover the missing-preview cases return true; } - if (nftPreview.uri) { - // While the user has IPFS gateway fetching enabled, ipfs:// URIs are - // served through an HTTPS gateway by the cache layer, so validate the - // gateway form instead of flagging them as invalid. With the option - // off they are not fetchable and stay flagged. - return isValidURL(ipfsGateway ? ipfsToGatewayUrl(nftPreview.uri) : nftPreview.uri); - } - - return false; + // While the user has IPFS gateway fetching enabled, ipfs:// URIs are + // served through an HTTPS gateway by the cache layer, so validate the + // gateway form instead of flagging them as invalid. With the option off + // they are not fetchable and stay flagged — unless the file already + // verified from the cache, which the message branches above this check. + return isValidURL(ipfsGateway ? ipfsToGatewayUrl(uri) : uri); }, [nftPreview, ipfsGateway]); const icon = useMemo(() => { From 3bcb027e45d7e5113b24e2a479fb498b6bec2459 Mon Sep 17 00:00:00 2001 From: Jonathan LoBue Date: Thu, 20 Aug 2026 19:14:48 -0700 Subject: [PATCH 06/10] Retry in-flight metadata that fails under the old gateway preference MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The gateway-flip retry effect only invalidated cache entries that had already failed. An entry whose fetch was still in flight was skipped — so a request started while the option was off could reject after the toggle had run, caching a failure that nothing would ever retry until remount (Bugbot, PR #3029). In-flight entries now get a rejection handler: if the pending fetch fails, it is invalidated and refetched under the new preference, while a result that arrives successfully is kept instead of being thrown away and refetched. Repeated toggles can stack handlers on one promise, but each retry goes through invalidate, so the worst case is a redundant refetch, not an inconsistent cache. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_017NXWAjaHb9SFafLeguTHd8 --- .../nfts/provider/hooks/useMetadataData.ts | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/packages/gui/src/components/nfts/provider/hooks/useMetadataData.ts b/packages/gui/src/components/nfts/provider/hooks/useMetadataData.ts index f55e7ce70f..b84ba7bb37 100644 --- a/packages/gui/src/components/nfts/provider/hooks/useMetadataData.ts +++ b/packages/gui/src/components/nfts/provider/hooks/useMetadataData.ts @@ -174,12 +174,20 @@ export default function useMetadataData(props: UseMetadataDataProps) { // metadata fetch stayed cached here and its NFT kept looking broken // after enabling the option, until a full app reload. Only failures are // retried: successfully fetched metadata is hash-verified content and - // unaffected by how it was fetched. + // unaffected by how it was fetched. A fetch that is still in flight + // started under the old preference and may fail because of it — after + // this effect has run, nothing else would retry that failure — so it is + // retried on rejection; a result that arrives successfully is kept. metadatasOnDemand.forEach((metadataOnDemand, nftId) => { - if (metadataOnDemand.error) { + const retry = () => invalidate(nftId).catch((e) => { log(`Error retrying metadata for nftId: ${nftId}`, e); }); + + if (metadataOnDemand.error) { + retry(); + } else if (metadataOnDemand.promise) { + metadataOnDemand.promise.catch(retry); } }); }, [ipfsGateway, invalidate /* immutable */, metadatasOnDemand /* immutable */]); From e465ab5ffed5f37597027738eca19710c4a239b4 Mon Sep 17 00:00:00 2001 From: Jonathan LoBue Date: Thu, 20 Aug 2026 19:24:58 -0700 Subject: [PATCH 07/10] Retry each metadata failure exactly once per gateway flip The gateway-flip retry effect had two churn paths (Bugbot, PR #3029): retrying an errored entry re-inserts its key with a fresh in-flight promise synchronously, and Map.forEach revisits keys re-added during the pass - so the effect attached a rejection retry to the very fetch it had just started, double-fetching a failure. And rapid toggles could stack rejection handlers on one promise; when it rejected, each handler invalidated in turn, the later ones discarding the refetch the first had started - even a successful one. The effect now iterates a snapshot of the map, and a rejection handler retries only the failure it saw: the fetch's own catch stores its rejection as the entry's error, so an entry that has moved on - already retried by a stacked handler, or settled successfully - is left alone. Each failure is retried exactly once per flip and successful results are never dropped. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_017NXWAjaHb9SFafLeguTHd8 --- .../nfts/provider/hooks/useMetadataData.ts | 18 ++++++++++++++++-- 1 file changed, 16 insertions(+), 2 deletions(-) diff --git a/packages/gui/src/components/nfts/provider/hooks/useMetadataData.ts b/packages/gui/src/components/nfts/provider/hooks/useMetadataData.ts index b84ba7bb37..73b8e71b6a 100644 --- a/packages/gui/src/components/nfts/provider/hooks/useMetadataData.ts +++ b/packages/gui/src/components/nfts/provider/hooks/useMetadataData.ts @@ -178,7 +178,12 @@ export default function useMetadataData(props: UseMetadataDataProps) { // started under the old preference and may fail because of it — after // this effect has run, nothing else would retry that failure — so it is // retried on rejection; a result that arrives successfully is kept. - metadatasOnDemand.forEach((metadataOnDemand, nftId) => { + // + // Iterate a snapshot: retrying an errored entry re-inserts its key with a + // fresh in-flight promise synchronously, and Map.forEach revisits keys + // re-added during the pass — the live map would attach a rejection retry + // to the very fetch this effect just started, double-fetching a failure. + Array.from(metadatasOnDemand.entries()).forEach(([nftId, metadataOnDemand]) => { const retry = () => invalidate(nftId).catch((e) => { log(`Error retrying metadata for nftId: ${nftId}`, e); @@ -187,7 +192,16 @@ export default function useMetadataData(props: UseMetadataDataProps) { if (metadataOnDemand.error) { retry(); } else if (metadataOnDemand.promise) { - metadataOnDemand.promise.catch(retry); + metadataOnDemand.promise.catch((e) => { + // Retry only the failure this handler saw. The fetch's own catch + // stores its rejection as the entry's error, so anything else here + // means the entry has moved on — a stacked handler from another + // toggle already retried it, or a newer fetch succeeded — and a + // retry would discard that state and fetch again for nothing. + if (metadatasOnDemand.get(nftId)?.error === e) { + retry(); + } + }); } }); }, [ipfsGateway, invalidate /* immutable */, metadatasOnDemand /* immutable */]); From 46ca7ecb9e98dbe0de5ecd72c7a95d66ec5b1c48 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 10 Aug 2026 18:50:31 +0000 Subject: [PATCH 08/10] Retry timed-out NFT downloads once per session MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Settling download timeouts as permanent cache errors fixed the retry-a-stalled-host-on-every-access loop, but overshot: the timeout message is not in the transient-error list, so one slow first byte from a cold gateway — or a briefly offline machine — wrote an error that survived restarts. The NFT showed "Preview is not available" forever, until the user cleared the entire NFT cache. A persisted timeout is now retried once per app session: within the session the URL settles after its first timeout (a stalled host still cannot hold a download slot on every gallery visit), but a restart gets a fresh attempt, so a one-off network problem no longer bricks the preview. Timeout detection matches the messages persisted by earlier sessions' -info files, so existing poisoned caches also recover. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01FUAhsJYiWuD36Hq4h49gLg --- .../gui/src/electron/CacheManager.test.ts | 28 +++++++++++++++++++ packages/gui/src/electron/CacheManager.ts | 17 +++++++++-- .../gui/src/electron/utils/downloadFile.ts | 13 +++++++-- 3 files changed, 54 insertions(+), 4 deletions(-) diff --git a/packages/gui/src/electron/CacheManager.test.ts b/packages/gui/src/electron/CacheManager.test.ts index 00a5371b25..3de3202a2d 100644 --- a/packages/gui/src/electron/CacheManager.test.ts +++ b/packages/gui/src/electron/CacheManager.test.ts @@ -17,6 +17,7 @@ jest.mock('./utils/downloadFile', () => ({ __esModule: true, default: mockDownloadFile, MAX_FILE_SIZE_EXCEEDED_ERROR: 'Maximum file size exceeded', + isDownloadTimeoutError: jest.requireActual('./utils/downloadFile').isDownloadTimeoutError, })); jest.mock('./utils/ipcMainHandle', () => ({ @@ -125,6 +126,33 @@ describe('CacheManager eviction', () => { expect(mockDownloadFile).toHaveBeenCalledTimes(1); }); + it('retries a timeout persisted by a previous session', async () => { + const payload = Buffer.from('cached payload'); + mockDownloadFile.mockRejectedValue(new Error('Request timed out after 30000ms of inactivity')); + + const firstSession = new CacheManager({ + cacheDirectory, + maxCacheSize: 1024, + }); + await firstSession.init(); + await expect(firstSession.getContent('https://example.com/nft.png')).rejects.toThrow('Request timed out'); + + mockDownloadFile.mockReset(); + mockDownloadFile.mockImplementation(async (_url, localPath) => { + await fs.writeFile(localPath, payload); + return { + 'content-type': 'image/png', + }; + }); + + const secondSession = new CacheManager({ + cacheDirectory, + maxCacheSize: 1024, + }); + await secondSession.init(); + await expect(secondSession.getContent('https://example.com/nft.png')).resolves.toEqual(payload); + }); + it('retries an aborted download on the next access', async () => { const payload = Buffer.from('cached payload'); mockDownloadFile.mockRejectedValueOnce(new Error('Request aborted')).mockImplementation(async (_url, localPath) => { diff --git a/packages/gui/src/electron/CacheManager.ts b/packages/gui/src/electron/CacheManager.ts index 4e5e595720..04dd430c64 100644 --- a/packages/gui/src/electron/CacheManager.ts +++ b/packages/gui/src/electron/CacheManager.ts @@ -15,7 +15,7 @@ import CacheState from '../constants/CacheState'; import limit from '../util/limit'; import CacheAPI from './constants/CacheAPI'; -import downloadFile, { MAX_FILE_SIZE_EXCEEDED_ERROR } from './utils/downloadFile'; +import downloadFile, { MAX_FILE_SIZE_EXCEEDED_ERROR, isDownloadTimeoutError } from './utils/downloadFile'; import ensureDirectoryExists from './utils/ensureDirectoryExists'; import getChecksum from './utils/getChecksum'; import ipcMainHandle from './utils/ipcMainHandle'; @@ -114,6 +114,11 @@ export default class CacheManager extends EventEmitter { } > = new Map(); + // URLs whose download timed out during this session. A persisted timeout is + // retried once per session — the set keeps a stalled host from being retried + // (and holding a download slot) on every access within the same session. + private timedOutUrls: Set = new Set(); + constructor( options: { cacheDirectory?: string; @@ -449,10 +454,14 @@ export default class CacheManager extends EventEmitter { log(`Url already downloaded with error: ${cacheInfo.error}`, url); const isTransientError = ['Response aborted', 'Request aborted'].includes(cacheInfo.error); + // A persisted timeout settles for the rest of the session, but is + // retried in later sessions — a one-off network problem must not + // disable the preview until the whole cache is cleared. + const isRetriableTimeout = isDownloadTimeoutError(cacheInfo.error) && !this.timedOutUrls.has(url); // A persisted size-limit error is only retried when the caller lifts // the limit, so oversized files are not re-downloaded on every visit. const isSizeLimitLifted = cacheInfo.error === MAX_FILE_SIZE_EXCEEDED_ERROR && maxSize <= 0; - if (!isTransientError && !isSizeLimitLifted) { + if (!isTransientError && !isRetriableTimeout && !isSizeLimitLifted) { return cacheInfo; } @@ -518,6 +527,10 @@ export default class CacheManager extends EventEmitter { const currentError = (error as Error) ?? new Error('Unknown fetchRemoteContent error'); + if (isDownloadTimeoutError(currentError.message)) { + this.timedOutUrls.add(url); + } + return await this.setCacheInfo(url, { state: CacheState.ERROR, error: currentError.message, diff --git a/packages/gui/src/electron/utils/downloadFile.ts b/packages/gui/src/electron/utils/downloadFile.ts index d69d5b679f..899984760c 100644 --- a/packages/gui/src/electron/utils/downloadFile.ts +++ b/packages/gui/src/electron/utils/downloadFile.ts @@ -66,6 +66,15 @@ class WriteStreamPromise { export const MAX_FILE_SIZE_EXCEEDED_ERROR = 'Maximum file size exceeded'; +const INACTIVITY_TIMEOUT_ERROR_PREFIX = 'Request timed out after'; +const DOWNLOAD_DEADLINE_ERROR_PREFIX = 'Request exceeded the'; + +/** Matches the messages of both timeout errors below, including messages that + * earlier sessions persisted into cache `-info` files. */ +export function isDownloadTimeoutError(message: string): boolean { + return message.startsWith(INACTIVITY_TIMEOUT_ERROR_PREFIX) || message.startsWith(DOWNLOAD_DEADLINE_ERROR_PREFIX); +} + type DownloadFileOptions = { timeout?: number; maxDuration?: number; // absolute cap on the whole transfer @@ -136,7 +145,7 @@ export default async function downloadFile( } timeoutId = setTimeout( - () => abortWithError(new Error(`Request timed out after ${timeout}ms of inactivity`)), + () => abortWithError(new Error(`${INACTIVITY_TIMEOUT_ERROR_PREFIX} ${timeout}ms of inactivity`)), timeout, ); } @@ -144,7 +153,7 @@ export default async function downloadFile( // absolute deadline for the whole transfer — the inactivity timeout alone // would let a host trickling bytes hold a download slot forever const maxDurationTimeoutId = setTimeout( - () => abortWithError(new Error(`Request exceeded the ${maxDuration}ms download deadline`)), + () => abortWithError(new Error(`${DOWNLOAD_DEADLINE_ERROR_PREFIX} ${maxDuration}ms download deadline`)), maxDuration, ); From f68778eb58f75a687cfd911e92fa60a4601c215a Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 10 Aug 2026 18:51:08 +0000 Subject: [PATCH 09/10] Bound confirmation-preview resolution with one overall deadline MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit resolveNftPreviewUrl walks three ordered URI lists per NFT (image data URIs, metadata URIs, extensionless data URIs) with a 10-second timeout per fetch and no overall limit. NFT URI lists are on-chain data that nft_add_uri can extend, and the WalletConnect confirmation dialog only opens after parsing settles — so a take_offer referencing NFTs with many dead or slow hosts kept the security dialog off screen for 10s x URIs x lists, minutes in the worst case, while the request could expire. All fallbacks for one NFT now share a 20-second budget: each fetch gets the remaining time (capped at its own per-fetch timeout) and the walk stops when the budget is spent. Previews degrade to the placeholder in that case — the dialog itself is never held up by more than the budget. NFTs resolve concurrently, so the budget bounds the whole parse regardless of how many NFTs an offer references. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01FUAhsJYiWuD36Hq4h49gLg --- .../gui/src/electron/api/nftGetMetadata.ts | 10 ++-- .../commands/parseCommandDisplay.test.ts | 55 ++++++++++++++++--- .../electron/commands/parseCommandDisplay.ts | 30 +++++++++- 3 files changed, 81 insertions(+), 14 deletions(-) diff --git a/packages/gui/src/electron/api/nftGetMetadata.ts b/packages/gui/src/electron/api/nftGetMetadata.ts index f6c4cf38fb..ddf5e99c51 100644 --- a/packages/gui/src/electron/api/nftGetMetadata.ts +++ b/packages/gui/src/electron/api/nftGetMetadata.ts @@ -41,8 +41,9 @@ function hasExpectedChecksum(data: Buffer, expectedHash: string): boolean { export async function nftGetMetadata( metadataUri: string, expectedHash: string | undefined, + timeoutBudget: number = Number.POSITIVE_INFINITY, ): Promise { - if (!expectedHash) { + if (!expectedHash || timeoutBudget <= 0) { return undefined; } @@ -51,7 +52,7 @@ export async function nftGetMetadata( headers: { Accept: 'application/json', }, - timeout: METADATA_TIMEOUT, + timeout: Math.min(METADATA_TIMEOUT, timeoutBudget), maxSize: METADATA_MAX_SIZE, }); @@ -74,8 +75,9 @@ export async function nftGetMetadata( export async function nftGetImageDataUrl( imageUri: string, expectedHash: string | undefined, + timeoutBudget: number = Number.POSITIVE_INFINITY, ): Promise { - if (!expectedHash) { + if (!expectedHash || timeoutBudget <= 0) { return undefined; } @@ -84,7 +86,7 @@ export async function nftGetImageDataUrl( headers: { Accept: 'image/*', }, - timeout: IMAGE_TIMEOUT, + timeout: Math.min(IMAGE_TIMEOUT, timeoutBudget), maxSize: IMAGE_MAX_SIZE, }); diff --git a/packages/gui/src/electron/commands/parseCommandDisplay.test.ts b/packages/gui/src/electron/commands/parseCommandDisplay.test.ts index 3dca419d55..4335e2c39b 100644 --- a/packages/gui/src/electron/commands/parseCommandDisplay.test.ts +++ b/packages/gui/src/electron/commands/parseCommandDisplay.test.ts @@ -210,7 +210,7 @@ describe('parseCommandDisplay', () => { }, }); expect(mockNftGetInfo).toHaveBeenCalledWith(nftLauncherId); - expect(mockNftGetImageDataUrl).toHaveBeenCalledWith('https://example.com/nft.png', 'data-hash'); + expect(mockNftGetImageDataUrl).toHaveBeenCalledWith('https://example.com/nft.png', 'data-hash', expect.any(Number)); }); it('uses the metadata preview image for a video NFT instead of the video data uri', async () => { @@ -256,8 +256,16 @@ describe('parseCommandDisplay', () => { ], }, }); - expect(mockNftGetMetadata).toHaveBeenCalledWith('https://example.com/nft.json', 'metadata-hash'); - expect(mockNftGetImageDataUrl).toHaveBeenCalledWith('https://example.com/nft-preview.png', 'preview-image-hash'); + expect(mockNftGetMetadata).toHaveBeenCalledWith( + 'https://example.com/nft.json', + 'metadata-hash', + expect.any(Number), + ); + expect(mockNftGetImageDataUrl).toHaveBeenCalledWith( + 'https://example.com/nft-preview.png', + 'preview-image-hash', + expect.any(Number), + ); }); it('tries later metadata URIs when an earlier fallback cannot be verified', async () => { @@ -277,11 +285,36 @@ describe('parseCommandDisplay', () => { ).resolves.toBe('data:image/png;base64,cHJldmlldw=='); expect(mockNftGetMetadata.mock.calls).toEqual([ - ['https://example.com/unavailable.json', 'metadata-hash'], - ['https://example.com/verified.json', 'metadata-hash'], + ['https://example.com/unavailable.json', 'metadata-hash', expect.any(Number)], + ['https://example.com/verified.json', 'metadata-hash', expect.any(Number)], ]); }); + it('stops trying preview fallbacks once the overall resolution budget is spent', async () => { + let now = 1_700_000_000_000_000; + const nowSpy = jest.spyOn(Date, 'now').mockImplementation(() => now); + try { + mockNftGetMetadata.mockImplementation(async () => { + now += 25_000; // a slow host consumes the whole budget + return undefined; + }); + + await expect( + resolveNftPreviewUrl( + [], + undefined, + ['https://example.com/slow.json', 'https://example.com/never-tried.json'], + 'metadata-hash', + ), + ).resolves.toBeUndefined(); + + expect(mockNftGetMetadata).toHaveBeenCalledTimes(1); + expect(mockNftGetImageDataUrl).not.toHaveBeenCalled(); + } finally { + nowSpy.mockRestore(); + } + }); + it('does not fetch confirmation previews that have no expected on-chain hash', async () => { await expect( resolveNftPreviewUrl( @@ -327,7 +360,11 @@ describe('parseCommandDisplay', () => { const line = result.walletDelta!.spending[0] as { kind: string; previewUrl?: string }; expect(line.kind).toBe('nft'); expect(line.previewUrl).toBeUndefined(); - expect(mockNftGetMetadata).toHaveBeenCalledWith('https://example.com/nft.json', 'metadata-hash'); + expect(mockNftGetMetadata).toHaveBeenCalledWith( + 'https://example.com/nft.json', + 'metadata-hash', + expect.any(Number), + ); }); it('uses an extensionless data uri as preview when metadata has no preview image', async () => { @@ -370,7 +407,11 @@ describe('parseCommandDisplay', () => { }, }); expect(mockNftGetMetadata).not.toHaveBeenCalled(); - expect(mockNftGetImageDataUrl).toHaveBeenCalledWith('https://ipfs.example.com/bafybeigdyrztest', 'data-hash'); + expect(mockNftGetImageDataUrl).toHaveBeenCalledWith( + 'https://ipfs.example.com/bafybeigdyrztest', + 'data-hash', + expect.any(Number), + ); }); it('shows the take-offer fungible total with NFT creator royalties', async () => { diff --git a/packages/gui/src/electron/commands/parseCommandDisplay.ts b/packages/gui/src/electron/commands/parseCommandDisplay.ts index 41671d8ce0..e0781ffbc5 100644 --- a/packages/gui/src/electron/commands/parseCommandDisplay.ts +++ b/packages/gui/src/electron/commands/parseCommandDisplay.ts @@ -49,16 +49,31 @@ function hexToNftId(hex: string): string { } } -async function resolveVerifiedImage(uris: string[], expectedHash: string | undefined): Promise { +// The confirmation dialog is not shown until preview resolution settles, so +// every URI fallback for one NFT shares a single deadline — a long list of +// dead or slow hosts must not hold the security dialog off screen for the +// full per-fetch timeout each. +const NFT_PREVIEW_RESOLUTION_BUDGET_MS = 20_000; + +async function resolveVerifiedImage( + uris: string[], + expectedHash: string | undefined, + deadline: number, +): Promise { if (!expectedHash) { return undefined; } for (const uri of uris) { + const timeLeft = deadline - Date.now(); + if (timeLeft <= 0) { + return undefined; + } + if (isValidURL(uri)) { // URI lists are ordered fallbacks for the same content. // eslint-disable-next-line no-await-in-loop -- Fallbacks must be tried in their declared order. - const dataUrl = await nftGetImageDataUrl(uri, expectedHash); + const dataUrl = await nftGetImageDataUrl(uri, expectedHash, timeLeft); if (dataUrl) { return dataUrl; } @@ -78,11 +93,13 @@ export async function resolveNftPreviewUrl( metadataUris: string[], metadataHash: string | undefined, ): Promise { + const deadline = Date.now() + NFT_PREVIEW_RESOLUTION_BUDGET_MS; const validDataUris = dataUris.filter((uri) => isValidURL(uri)); const imageDataUrl = await resolveVerifiedImage( validDataUris.filter((uri) => getFileType(uri) === FileType.IMAGE), dataHash, + deadline, ); if (imageDataUrl) { return imageDataUrl; @@ -90,15 +107,21 @@ export async function resolveNftPreviewUrl( if (metadataHash) { for (const metadataUri of metadataUris) { + const timeLeft = deadline - Date.now(); + if (timeLeft <= 0) { + break; + } + if (isValidURL(metadataUri)) { // Metadata URIs are ordered fallbacks for the same on-chain hash. // eslint-disable-next-line no-await-in-loop -- Fallbacks must be tried in their declared order. - const metadata = await nftGetMetadata(metadataUri, metadataHash); + const metadata = await nftGetMetadata(metadataUri, metadataHash, timeLeft); if (metadata) { // eslint-disable-next-line no-await-in-loop -- Resolve each verified metadata fallback before moving on. const previewDataUrl = await resolveVerifiedImage( metadata.preview_image_uris ?? [], metadata.preview_image_hash, + deadline, ); if (previewDataUrl) { return previewDataUrl; @@ -114,6 +137,7 @@ export async function resolveNftPreviewUrl( return resolveVerifiedImage( validDataUris.filter((uri) => getFileType(uri) === FileType.UNKNOWN), dataHash, + deadline, ); } From 72a9b8d580bf6ef5102ab727a8de6db753fb0fd9 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 10 Aug 2026 18:51:37 +0000 Subject: [PATCH 10/10] Drop redundant buffer copies and a dead hook export The verified-preview path copied every downloaded byte twice for no gain: fetchBuffer re-allocated each response chunk element-by-element via Uint8Array.from before collecting it (Electron's net module delivers fresh Buffers, so retaining them is safe and Buffer.concat already performs the single final copy), and the metadata checksum round-tripped the whole buffer through a latin1 string before hashing, which allocates an up-to-10MB string to produce the digest hashing the Buffer directly yields. Also removes the unused useNFTVideoLoop default export: both consumers import the named hooks and derive the effective global-or-per-video state themselves, so the wrapper only added surface that could drift from the real logic. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01FUAhsJYiWuD36Hq4h49gLg --- packages/gui/src/electron/api/nftGetMetadata.ts | 2 +- packages/gui/src/electron/utils/fetchBuffer.ts | 7 +++---- packages/gui/src/hooks/useNFTVideoLoop.tsx | 11 +++-------- 3 files changed, 7 insertions(+), 13 deletions(-) diff --git a/packages/gui/src/electron/api/nftGetMetadata.ts b/packages/gui/src/electron/api/nftGetMetadata.ts index ddf5e99c51..c04b05af38 100644 --- a/packages/gui/src/electron/api/nftGetMetadata.ts +++ b/packages/gui/src/electron/api/nftGetMetadata.ts @@ -20,7 +20,7 @@ export type NftMetadata = Record & { }; function checksum(data: Buffer): string { - return crypto.createHash('sha256').update(data.toString('latin1'), 'latin1').digest('hex'); + return crypto.createHash('sha256').update(data).digest('hex'); } function getImageContentType(headers: Headers): string | undefined { diff --git a/packages/gui/src/electron/utils/fetchBuffer.ts b/packages/gui/src/electron/utils/fetchBuffer.ts index 0efef32118..f3d7a14d16 100644 --- a/packages/gui/src/electron/utils/fetchBuffer.ts +++ b/packages/gui/src/electron/utils/fetchBuffer.ts @@ -102,7 +102,7 @@ export default async function fetchBuffer( } } - const chunks: Uint8Array[] = []; + const chunks: Buffer[] = []; let dataSize = 0; response.on('data', (chunk: Buffer) => { @@ -110,14 +110,13 @@ export default async function fetchBuffer( return; } - const buffer = Uint8Array.from(chunk); - dataSize += buffer.byteLength; + dataSize += chunk.byteLength; if (maxSize > 0 && dataSize > maxSize) { abortWith(new MaxSizeExceededError(response.headers as Headers)); return; } - chunks.push(buffer); + chunks.push(chunk); }); response.on('end', () => { diff --git a/packages/gui/src/hooks/useNFTVideoLoop.tsx b/packages/gui/src/hooks/useNFTVideoLoop.tsx index 159765d338..9ee7d5e0fe 100644 --- a/packages/gui/src/hooks/useNFTVideoLoop.tsx +++ b/packages/gui/src/hooks/useNFTVideoLoop.tsx @@ -34,11 +34,6 @@ export function useNFTVideoLoopForNFT(nftId: string): [boolean, (loopVideo: bool return [loop, setLoop]; } -// The effective looping state for one NFT video: the global preference forces -// looping on when set, but never disables a video's own loop preference. -export default function useNFTVideoLoop(nftId: string): boolean { - const [globalLoop] = useNFTVideoLoopGlobal(); - const [videoLoop] = useNFTVideoLoopForNFT(nftId); - - return globalLoop || videoLoop; -} +// The effective looping state for one NFT video is `global || perVideo`: +// the global preference forces looping on when set, but never disables a +// video's own loop preference. Consumers combine the two hooks above.