Skip to content

Commit c755e0c

Browse files
jlobue10claude
andcommitted
Gate only IPFS fetching on the gateway option, not cached content
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 Chia-Network#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 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017NXWAjaHb9SFafLeguTHd8
1 parent 7d3f7a9 commit c755e0c

9 files changed

Lines changed: 102 additions & 29 deletions

File tree

packages/gui/src/electron/CacheManager.ts

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ import downloadFile, { MAX_FILE_SIZE_EXCEEDED_ERROR } from './utils/downloadFile
1919
import ensureDirectoryExists from './utils/ensureDirectoryExists';
2020
import getChecksum from './utils/getChecksum';
2121
import ipcMainHandle from './utils/ipcMainHandle';
22+
import { IpfsGatewayDisabledError } from './utils/ipfsGateway';
2223
import isValidURL from './utils/isValidURL';
2324
import sanitizeFilename from './utils/sanitizeFilename';
2425
import sanitizeNumber from './utils/sanitizeNumber';
@@ -506,6 +507,15 @@ export default class CacheManager extends EventEmitter {
506507

507508
return await this.#downloadLimit<CacheInfo>(() => limitedRemoteFileDownload());
508509
} catch (error) {
510+
// Not a property of the URL, just of the current preference: while
511+
// the IPFS gateway option is off the fetch is refused before it
512+
// starts. Persisting that as a cache error would keep the entry
513+
// poisoned after the user turns the option on, so it propagates
514+
// instead — already-cached content was served above regardless.
515+
if (error instanceof IpfsGatewayDisabledError) {
516+
throw error;
517+
}
518+
509519
const currentError = (error as Error) ?? new Error('Unknown fetchRemoteContent error');
510520

511521
return await this.setCacheInfo(url, {

packages/gui/src/electron/main.tsx

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@ import type { PermissionsNotificationPayload } from '../@types/PermissionsServic
2929
import { WcError, WcErrorCode, encodeWcErrorForIpc } from '../@types/WcError';
3030
import AppIcon from '../assets/img/chia64x64.png';
3131
import { i18n } from '../config/locales';
32+
import { isIpfsUrl } from '../util/ipfs';
3233

3334
import CacheManager, { CACHE_PROTOCOL } from './CacheManager';
3435
import { checkNFTOwnership } from './api/checkNFTOwnership';
@@ -578,8 +579,15 @@ if (ensureSingleInstance() && ensureCorrectEnvironment()) {
578579

579580
// Chromium's downloader cannot fetch the ipfs: scheme; when the user
580581
// has enabled the gateway, download ipfs URIs through it like every
581-
// other network path.
582-
mainWindow.webContents.downloadURL(maybeIpfsToGatewayUrl(urlLocal));
582+
// other network path. With the option off there is nothing the
583+
// downloader could fetch, so the request is dropped instead of handing
584+
// Chromium a URL it silently fails on.
585+
const downloadUrl = maybeIpfsToGatewayUrl(urlLocal);
586+
if (isIpfsUrl(downloadUrl)) {
587+
return;
588+
}
589+
590+
mainWindow.webContents.downloadURL(downloadUrl);
583591
});
584592

585593
ipcMainHandle(AppAPI.START_MULTIPLE_DOWNLOAD, async (tasks: { url: string; filename: string }[]) => {

packages/gui/src/electron/utils/downloadFile.ts

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ import debug from 'debug';
66
import type Headers from '../../@types/Headers';
77

88
import fileExists from './fileExists';
9-
import maybeIpfsToGatewayUrl from './ipfsGateway';
9+
import { toFetchableUrl } from './ipfsGateway';
1010
import isValidURL from './isValidURL';
1111

1212
const log = debug('chia-gui:downloadFile');
@@ -102,10 +102,11 @@ export default async function downloadFile(
102102

103103
const tempFilePath = `${localPath}.tmp`;
104104
// ipfs:// URIs are fetched through an HTTPS gateway when the user has
105-
// enabled it — Electron's net stack cannot request the ipfs scheme. Only
105+
// enabled it — Electron's net stack cannot request the ipfs scheme, and
106+
// with the option off toFetchableUrl refuses the fetch outright. Only
106107
// this outgoing request uses the translated URL; callers keep the original
107108
// URI as the cache key.
108-
const request = net.request(maybeIpfsToGatewayUrl(url));
109+
const request = net.request(toFetchableUrl(url));
109110
const outputStream = new WriteStreamPromise(tempFilePath, overrideFile);
110111

111112
// set when we abort the request ourselves, so abort events can be reported

packages/gui/src/electron/utils/fetchBuffer.ts

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ import { net, type IncomingMessage } from 'electron';
22

33
import type Headers from '../../@types/Headers';
44

5-
import maybeIpfsToGatewayUrl from './ipfsGateway';
5+
import { toFetchableUrl } from './ipfsGateway';
66
import isValidURL from './isValidURL';
77

88
const DEFAULT_TIMEOUT = 10 * 60 * 1000; // 10 minutes
@@ -43,8 +43,9 @@ export default async function fetchBuffer(
4343
const request = net.request({
4444
method: 'GET',
4545
// ipfs:// URIs are fetched through an HTTPS gateway when the user has
46-
// enabled it — Electron's net stack cannot request the ipfs scheme.
47-
url: maybeIpfsToGatewayUrl(url),
46+
// enabled it — Electron's net stack cannot request the ipfs scheme, and
47+
// with the option off toFetchableUrl refuses the fetch outright.
48+
url: toFetchableUrl(url),
4849
headers,
4950
});
5051

packages/gui/src/electron/utils/fetchJSON.ts

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import { net, IncomingMessage } from 'electron';
22

3-
import maybeIpfsToGatewayUrl from './ipfsGateway';
3+
import { toFetchableUrl } from './ipfsGateway';
44
import isValidURL from './isValidURL';
55

66
const DEFAULT_TIMEOUT = 10 * 60 * 1000; // 10 minutes
@@ -19,8 +19,9 @@ export default async function fetchJSON<TData>(
1919
const request = net.request({
2020
method,
2121
// ipfs:// URIs are fetched through an HTTPS gateway when the user has
22-
// enabled it — Electron's net stack cannot request the ipfs scheme.
23-
url: maybeIpfsToGatewayUrl(url),
22+
// enabled it — Electron's net stack cannot request the ipfs scheme, and
23+
// with the option off toFetchableUrl refuses the fetch outright.
24+
url: toFetchableUrl(url),
2425
headers,
2526
});
2627

packages/gui/src/electron/utils/ipfsGateway.test.ts

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,8 @@ jest.mock('../prefs', () => ({
77
const {
88
default: maybeIpfsToGatewayUrl,
99
ipfsGatewayEnabled,
10+
toFetchableUrl,
11+
IpfsGatewayDisabledError,
1012
NFT_IPFS_GATEWAY_PREF,
1113
} = jest.requireActual<typeof import('./ipfsGateway')>('./ipfsGateway');
1214

@@ -64,3 +66,30 @@ describe('maybeIpfsToGatewayUrl', () => {
6466
);
6567
});
6668
});
69+
70+
describe('toFetchableUrl', () => {
71+
beforeEach(() => {
72+
mockReadPrefs.mockReset();
73+
});
74+
75+
it('passes non-ipfs URLs through without consulting the preferences store', () => {
76+
expect(toFetchableUrl('https://example.com/image.png')).toBe('https://example.com/image.png');
77+
expect(mockReadPrefs).not.toHaveBeenCalled();
78+
});
79+
80+
it('translates ipfs URIs when the gateway option is on', () => {
81+
mockReadPrefs.mockReturnValue({ [NFT_IPFS_GATEWAY_PREF]: true });
82+
83+
expect(toFetchableUrl('ipfs://QmPK1s3pNYLi9ERiq3BDxKa4XosgWwFRQUydHUtz4YgpqB/img.png')).toBe(
84+
'https://ipfs.io/ipfs/QmPK1s3pNYLi9ERiq3BDxKa4XosgWwFRQUydHUtz4YgpqB/img.png',
85+
);
86+
});
87+
88+
it('refuses ipfs URIs while the gateway option is off', () => {
89+
mockReadPrefs.mockReturnValue({});
90+
91+
expect(() => toFetchableUrl('ipfs://QmPK1s3pNYLi9ERiq3BDxKa4XosgWwFRQUydHUtz4YgpqB')).toThrow(
92+
IpfsGatewayDisabledError,
93+
);
94+
});
95+
});

packages/gui/src/electron/utils/ipfsGateway.ts

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,3 +30,27 @@ export default function maybeIpfsToGatewayUrl(url: string): string {
3030

3131
return ipfsToGatewayUrl(url);
3232
}
33+
34+
// Thrown instead of attempting a fetch that cannot happen: with the gateway
35+
// option off there is no URL Electron's net stack could request for an
36+
// ipfs:// URI. CacheManager treats this error as non-persistent — flipping
37+
// the option on must retry cleanly, so it never poisons a cache entry.
38+
export class IpfsGatewayDisabledError extends Error {
39+
constructor() {
40+
super('IPFS gateway fetching is disabled');
41+
this.name = 'IpfsGatewayDisabledError';
42+
}
43+
}
44+
45+
// The URL the network layer may actually request. The gateway option gates
46+
// only fetching: structural URL validation and serving already-cached content
47+
// stay independent of it, so every network call site funnels through here
48+
// instead of checking the option itself.
49+
export function toFetchableUrl(url: string): string {
50+
const requestUrl = maybeIpfsToGatewayUrl(url);
51+
if (isIpfsUrl(requestUrl)) {
52+
throw new IpfsGatewayDisabledError();
53+
}
54+
55+
return requestUrl;
56+
}

packages/gui/src/electron/utils/isValidURL.test.ts

Lines changed: 6 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,6 @@ jest.mock('../prefs', () => ({
55
}));
66

77
const isValidURL = jest.requireActual<typeof import('./isValidURL')>('./isValidURL').default;
8-
const { NFT_IPFS_GATEWAY_PREF } = jest.requireActual<typeof import('./ipfsGateway')>('./ipfsGateway');
98

109
describe('isValidURL', () => {
1110
beforeEach(() => {
@@ -23,23 +22,18 @@ describe('isValidURL', () => {
2322
expect(isValidURL('ftp://example.com/image.png')).toBe(false);
2423
});
2524

26-
it('accepts ipfs:// URIs with a CID host when the gateway option is on', () => {
27-
mockReadPrefs.mockReturnValue({ [NFT_IPFS_GATEWAY_PREF]: true });
28-
25+
it('accepts ipfs:// URIs with a CID host regardless of the gateway option', () => {
2926
// validator's isURL rejects CID hosts (no TLD), so these pass only via
30-
// the gateway translation
27+
// the gateway-form translation. The check is structural on purpose: the
28+
// gateway option gates fetching (toFetchableUrl), not validity — cache
29+
// lookups for already-downloaded ipfs content must keep working while
30+
// the option is off.
3131
expect(isValidURL('ipfs://bafybeiceg2gltyhlkukwetn26k7t2zdvthg4u4c6uj23rpni2adzgvo5si/020.png')).toBe(true);
3232
expect(isValidURL('ipfs://ipfs/QmPK1s3pNYLi9ERiq3BDxKa4XosgWwFRQUydHUtz4YgpqB')).toBe(true);
33-
});
34-
35-
it('rejects ipfs:// URIs while the gateway option is off', () => {
36-
expect(isValidURL('ipfs://bafybeiceg2gltyhlkukwetn26k7t2zdvthg4u4c6uj23rpni2adzgvo5si/020.png')).toBe(false);
37-
expect(isValidURL('ipfs://ipfs/QmPK1s3pNYLi9ERiq3BDxKa4XosgWwFRQUydHUtz4YgpqB')).toBe(false);
33+
expect(mockReadPrefs).not.toHaveBeenCalled();
3834
});
3935

4036
it('rejects a bare ipfs scheme and non-strings', () => {
41-
mockReadPrefs.mockReturnValue({ [NFT_IPFS_GATEWAY_PREF]: true });
42-
4337
expect(isValidURL('ipfs://')).toBe(false);
4438
expect(isValidURL(undefined as unknown as string)).toBe(false);
4539
});
Lines changed: 11 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,17 +1,22 @@
11
import isURL from 'validator/lib/isURL';
22

3-
import maybeIpfsToGatewayUrl from './ipfsGateway';
3+
import ipfsToGatewayUrl, { isIpfsUrl } from '../../util/ipfs';
44

5+
// Structural validation only — deliberately independent of the IPFS gateway
6+
// preference. CacheManager consults this check before every cache path
7+
// lookup, so tying it to the preference would strand content that was
8+
// downloaded and hash-verified while the option was on: the cached bytes
9+
// could no longer be served, checksummed, or evicted after switching it off,
10+
// even though serving a local file involves no gateway request. Whether an
11+
// ipfs URI may actually be FETCHED is decided at the network call sites via
12+
// toFetchableUrl (electron/utils/ipfsGateway.ts).
513
export default function isValidURL(url: string) {
614
if (typeof url !== 'string') {
715
return false;
816
}
917

1018
// isURL applies an FQDN check to the host, which every ipfs://<CID> URI
1119
// fails (a CID has no top-level domain), so listing 'ipfs' as an allowed
12-
// protocol is not enough. When the user has enabled gateway fetching,
13-
// validate the HTTPS gateway form instead — it is also the URL the network
14-
// layer will actually request; while the option is off, ipfs URIs stay
15-
// invalid and are never fetched.
16-
return isURL(maybeIpfsToGatewayUrl(url), { protocols: ['https'], require_protocol: true });
20+
// protocol is not enough — validate the HTTPS gateway form instead.
21+
return isURL(isIpfsUrl(url) ? ipfsToGatewayUrl(url) : url, { protocols: ['https'], require_protocol: true });
1722
}

0 commit comments

Comments
 (0)