Skip to content

Commit e5d4511

Browse files
authored
Merge pull request #284438 from microsoft/dev/dmitriv/fetch-tool-fixes
Allow redirects to trusted domains
2 parents d5b1147 + 1663fb5 commit e5d4511

File tree

15 files changed

+480
-22
lines changed

15 files changed

+480
-22
lines changed

src/vs/workbench/contrib/url/common/trustedDomains.ts renamed to src/vs/platform/url/common/trustedDomains.ts

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2,9 +2,9 @@
22
* Copyright (c) Microsoft Corporation. All rights reserved.
33
* Licensed under the MIT License. See License.txt in the project root for license information.
44
*--------------------------------------------------------------------------------------------*/
5-
import { URI } from '../../../../base/common/uri.js';
6-
import { testUrlMatchesGlob } from './urlGlob.js';
75

6+
import { URI } from '../../../base/common/uri.js';
7+
import { testUrlMatchesGlob } from './urlGlob.js';
88

99
/**
1010
* Check whether a domain like https://www.microsoft.com matches
@@ -14,7 +14,6 @@ import { testUrlMatchesGlob } from './urlGlob.js';
1414
* - There's no subdomain matching. For example https://microsoft.com doesn't match https://www.microsoft.com
1515
* - Star matches all subdomains. For example https://*.microsoft.com matches https://www.microsoft.com and https://foo.bar.microsoft.com
1616
*/
17-
1817
export function isURLDomainTrusted(url: URI, trustedDomains: string[]): boolean {
1918
url = URI.parse(normalizeURL(url));
2019
trustedDomains = trustedDomains.map(normalizeURL);
@@ -35,10 +34,10 @@ export function isURLDomainTrusted(url: URI, trustedDomains: string[]): boolean
3534

3635
return false;
3736
}
37+
3838
/**
3939
* Case-normalize some case-insensitive URLs, such as github.
4040
*/
41-
4241
export function normalizeURL(url: string | URI): string {
4342
const caseInsensitiveAuthorities = ['github.com'];
4443
try {
@@ -50,10 +49,10 @@ export function normalizeURL(url: string | URI): string {
5049
}
5150
} catch { return url.toString(); }
5251
}
52+
5353
const rLocalhost = /^(.+\.)?localhost(:\d+)?$/i;
5454
const r127 = /^127.0.0.1(:\d+)?$/;
5555

5656
export function isLocalhostAuthority(authority: string) {
5757
return rLocalhost.test(authority) || r127.test(authority);
5858
}
59-

src/vs/workbench/contrib/url/common/urlGlob.ts renamed to src/vs/platform/url/common/urlGlob.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
* Licensed under the MIT License. See License.txt in the project root for license information.
44
*--------------------------------------------------------------------------------------------*/
55

6-
import { URI } from '../../../../base/common/uri.js';
6+
import { URI } from '../../../base/common/uri.js';
77

88
/**
99
* Normalizes a URL by removing trailing slashes and query/fragment components.
Lines changed: 126 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,126 @@
1+
/*---------------------------------------------------------------------------------------------
2+
* Copyright (c) Microsoft Corporation. All rights reserved.
3+
* Licensed under the MIT License. See License.txt in the project root for license information.
4+
*--------------------------------------------------------------------------------------------*/
5+
6+
import assert from 'assert';
7+
import { isLocalhostAuthority, isURLDomainTrusted, normalizeURL } from '../../common/trustedDomains.js';
8+
import { URI } from '../../../../base/common/uri.js';
9+
import { ensureNoDisposablesAreLeakedInTestSuite } from '../../../../base/test/common/utils.js';
10+
11+
suite('trustedDomains', () => {
12+
13+
ensureNoDisposablesAreLeakedInTestSuite();
14+
15+
suite('isURLDomainTrusted', () => {
16+
17+
test('localhost is always trusted', () => {
18+
assert.strictEqual(isURLDomainTrusted(URI.parse('http://localhost:3000'), []), true);
19+
assert.strictEqual(isURLDomainTrusted(URI.parse('http://127.0.0.1:3000'), []), true);
20+
assert.strictEqual(isURLDomainTrusted(URI.parse('http://subdomain.localhost'), []), true);
21+
});
22+
23+
test('wildcard (*) matches everything', () => {
24+
assert.strictEqual(isURLDomainTrusted(URI.parse('https://example.com'), ['*']), true);
25+
assert.strictEqual(isURLDomainTrusted(URI.parse('http://anything.org'), ['*']), true);
26+
assert.strictEqual(isURLDomainTrusted(URI.parse('https://github.com/microsoft'), ['*']), true);
27+
});
28+
29+
test('exact domain match', () => {
30+
assert.strictEqual(isURLDomainTrusted(URI.parse('https://example.com'), ['https://example.com']), true);
31+
assert.strictEqual(isURLDomainTrusted(URI.parse('https://example.com/path'), ['https://example.com']), true);
32+
assert.strictEqual(isURLDomainTrusted(URI.parse('http://example.com'), ['https://example.com']), false);
33+
});
34+
35+
test('subdomain wildcard matching', () => {
36+
assert.strictEqual(isURLDomainTrusted(URI.parse('https://api.github.com'), ['https://*.github.com']), true);
37+
assert.strictEqual(isURLDomainTrusted(URI.parse('https://github.com'), ['https://*.github.com']), true);
38+
assert.strictEqual(isURLDomainTrusted(URI.parse('https://sub.api.github.com'), ['https://*.github.com']), true);
39+
});
40+
41+
test('path matching', () => {
42+
assert.strictEqual(isURLDomainTrusted(URI.parse('https://example.com/api/v1'), ['https://example.com/api/*']), true);
43+
// Path without trailing content doesn't match a wildcard pattern requiring more path segments
44+
assert.strictEqual(isURLDomainTrusted(URI.parse('https://example.com/api'), ['https://example.com/api/*']), false);
45+
});
46+
47+
test('scheme must match', () => {
48+
assert.strictEqual(isURLDomainTrusted(URI.parse('https://example.com'), ['http://example.com']), false);
49+
assert.strictEqual(isURLDomainTrusted(URI.parse('http://example.com'), ['https://example.com']), false);
50+
});
51+
52+
test('not trusted when no match', () => {
53+
assert.strictEqual(isURLDomainTrusted(URI.parse('https://example.com'), ['https://other.com']), false);
54+
assert.strictEqual(isURLDomainTrusted(URI.parse('https://example.com'), []), false);
55+
});
56+
57+
test('multiple trusted domains', () => {
58+
const trusted = ['https://github.com', 'https://microsoft.com'];
59+
assert.strictEqual(isURLDomainTrusted(URI.parse('https://github.com'), trusted), true);
60+
assert.strictEqual(isURLDomainTrusted(URI.parse('https://microsoft.com'), trusted), true);
61+
assert.strictEqual(isURLDomainTrusted(URI.parse('https://google.com'), trusted), false);
62+
});
63+
64+
test('case normalization for github', () => {
65+
assert.strictEqual(isURLDomainTrusted(URI.parse('https://github.com/Microsoft/VSCode'), ['https://github.com/microsoft/vscode']), true);
66+
assert.strictEqual(isURLDomainTrusted(URI.parse('https://github.com/microsoft/vscode'), ['https://github.com/Microsoft/VSCode']), true);
67+
});
68+
});
69+
70+
suite('normalizeURL', () => {
71+
72+
test('normalizes github.com URLs to lowercase path', () => {
73+
assert.strictEqual(normalizeURL('https://github.com/Microsoft/VSCode'), 'https://github.com/microsoft/vscode');
74+
assert.strictEqual(normalizeURL('https://github.com/OWNER/REPO'), 'https://github.com/owner/repo');
75+
});
76+
77+
test('does not normalize non-github URLs', () => {
78+
assert.strictEqual(normalizeURL('https://example.com/Path/To/Resource'), 'https://example.com/Path/To/Resource');
79+
assert.strictEqual(normalizeURL('https://microsoft.com/Products'), 'https://microsoft.com/Products');
80+
});
81+
82+
test('handles URI objects', () => {
83+
const uri = URI.parse('https://github.com/Microsoft/VSCode');
84+
assert.strictEqual(normalizeURL(uri), 'https://github.com/microsoft/vscode');
85+
});
86+
87+
test('handles invalid URIs gracefully', () => {
88+
const result = normalizeURL('not-a-valid-uri');
89+
assert.strictEqual(typeof result, 'string');
90+
});
91+
});
92+
93+
suite('isLocalhostAuthority', () => {
94+
95+
test('recognizes localhost', () => {
96+
assert.strictEqual(isLocalhostAuthority('localhost'), true);
97+
assert.strictEqual(isLocalhostAuthority('localhost:3000'), true);
98+
assert.strictEqual(isLocalhostAuthority('localhost:8080'), true);
99+
});
100+
101+
test('recognizes subdomains of localhost', () => {
102+
assert.strictEqual(isLocalhostAuthority('subdomain.localhost'), true);
103+
assert.strictEqual(isLocalhostAuthority('api.localhost:3000'), true);
104+
assert.strictEqual(isLocalhostAuthority('a.b.c.localhost'), true);
105+
});
106+
107+
test('recognizes 127.0.0.1', () => {
108+
assert.strictEqual(isLocalhostAuthority('127.0.0.1'), true);
109+
assert.strictEqual(isLocalhostAuthority('127.0.0.1:3000'), true);
110+
assert.strictEqual(isLocalhostAuthority('127.0.0.1:8080'), true);
111+
});
112+
113+
test('case insensitive for localhost', () => {
114+
assert.strictEqual(isLocalhostAuthority('LOCALHOST'), true);
115+
assert.strictEqual(isLocalhostAuthority('LocalHost:3000'), true);
116+
assert.strictEqual(isLocalhostAuthority('SUB.LOCALHOST'), true);
117+
});
118+
119+
test('does not match non-localhost authorities', () => {
120+
assert.strictEqual(isLocalhostAuthority('example.com'), false);
121+
assert.strictEqual(isLocalhostAuthority('notlocalhost.com'), false);
122+
assert.strictEqual(isLocalhostAuthority('127.0.0.2'), false);
123+
assert.strictEqual(isLocalhostAuthority('192.168.1.1'), false);
124+
});
125+
});
126+
});
Lines changed: 107 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,107 @@
1+
/*---------------------------------------------------------------------------------------------
2+
* Copyright (c) Microsoft Corporation. All rights reserved.
3+
* Licensed under the MIT License. See License.txt in the project root for license information.
4+
*--------------------------------------------------------------------------------------------*/
5+
6+
import assert from 'assert';
7+
import { URI } from '../../../../base/common/uri.js';
8+
import { testUrlMatchesGlob } from '../../common/urlGlob.js';
9+
import { ensureNoDisposablesAreLeakedInTestSuite } from '../../../../base/test/common/utils.js';
10+
11+
suite('urlGlob', () => {
12+
13+
ensureNoDisposablesAreLeakedInTestSuite();
14+
15+
suite('testUrlMatchesGlob', () => {
16+
17+
test('exact match', () => {
18+
assert.strictEqual(testUrlMatchesGlob('https://example.com', 'https://example.com'), true);
19+
assert.strictEqual(testUrlMatchesGlob('http://example.com', 'http://example.com'), true);
20+
assert.strictEqual(testUrlMatchesGlob('https://example.com/path', 'https://example.com/path'), true);
21+
});
22+
23+
test('trailing slashes are ignored', () => {
24+
assert.strictEqual(testUrlMatchesGlob('https://example.com/', 'https://example.com'), true);
25+
assert.strictEqual(testUrlMatchesGlob('https://example.com', 'https://example.com/'), true);
26+
assert.strictEqual(testUrlMatchesGlob('https://example.com//', 'https://example.com'), true);
27+
assert.strictEqual(testUrlMatchesGlob('https://example.com/path/', 'https://example.com/path'), true);
28+
});
29+
30+
test('query and fragment are ignored', () => {
31+
assert.strictEqual(testUrlMatchesGlob('https://example.com?query=value', 'https://example.com'), true);
32+
assert.strictEqual(testUrlMatchesGlob('https://example.com#fragment', 'https://example.com'), true);
33+
assert.strictEqual(testUrlMatchesGlob('https://example.com?query=value#fragment', 'https://example.com'), true);
34+
});
35+
36+
test('scheme matching', () => {
37+
assert.strictEqual(testUrlMatchesGlob('https://example.com', 'https://example.com'), true);
38+
assert.strictEqual(testUrlMatchesGlob('http://example.com', 'https://example.com'), false);
39+
assert.strictEqual(testUrlMatchesGlob('ftp://example.com', 'https://example.com'), false);
40+
});
41+
42+
test('glob without scheme assumes http/https', () => {
43+
assert.strictEqual(testUrlMatchesGlob('https://example.com', 'example.com'), true);
44+
assert.strictEqual(testUrlMatchesGlob('http://example.com', 'example.com'), true);
45+
assert.strictEqual(testUrlMatchesGlob('ftp://example.com', 'example.com'), false);
46+
});
47+
48+
test('wildcard matching in path', () => {
49+
assert.strictEqual(testUrlMatchesGlob('https://example.com/anything', 'https://example.com/*'), true);
50+
assert.strictEqual(testUrlMatchesGlob('https://example.com/path/to/resource', 'https://example.com/*'), true);
51+
assert.strictEqual(testUrlMatchesGlob('https://example.com/path/to/resource', 'https://example.com/path/*'), true);
52+
assert.strictEqual(testUrlMatchesGlob('https://example.com/path/to/resource', 'https://example.com/path/*/resource'), true);
53+
});
54+
55+
test('subdomain wildcard matching', () => {
56+
assert.strictEqual(testUrlMatchesGlob('https://sub.example.com', 'https://*.example.com'), true);
57+
assert.strictEqual(testUrlMatchesGlob('https://sub.domain.example.com', 'https://*.example.com'), true);
58+
assert.strictEqual(testUrlMatchesGlob('https://example.com', 'https://*.example.com'), true);
59+
// *. matches any number of characters before the domain, including other domains
60+
assert.strictEqual(testUrlMatchesGlob('https://notexample.com', 'https://*.example.com'), true);
61+
});
62+
63+
test('port matching', () => {
64+
assert.strictEqual(testUrlMatchesGlob('https://example.com:8080', 'https://example.com:8080'), true);
65+
assert.strictEqual(testUrlMatchesGlob('https://example.com:8080', 'https://example.com:9090'), false);
66+
assert.strictEqual(testUrlMatchesGlob('https://example.com', 'https://example.com:8080'), false);
67+
});
68+
69+
test('wildcard port matching', () => {
70+
assert.strictEqual(testUrlMatchesGlob('https://example.com:8080', 'https://example.com:*'), true);
71+
assert.strictEqual(testUrlMatchesGlob('https://example.com:9090', 'https://example.com:*'), true);
72+
assert.strictEqual(testUrlMatchesGlob('https://example.com', 'https://example.com:*'), true);
73+
assert.strictEqual(testUrlMatchesGlob('https://example.com:8080/path', 'https://example.com:*/path'), true);
74+
});
75+
76+
test('root path glob', () => {
77+
assert.strictEqual(testUrlMatchesGlob('https://example.com', 'https://example.com/'), true);
78+
assert.strictEqual(testUrlMatchesGlob('https://example.com/', 'https://example.com/'), true);
79+
assert.strictEqual(testUrlMatchesGlob('https://example.com/path', 'https://example.com/'), true);
80+
});
81+
82+
test('mismatch cases', () => {
83+
assert.strictEqual(testUrlMatchesGlob('https://example.com/path', 'https://example.com/other'), false);
84+
assert.strictEqual(testUrlMatchesGlob('https://example.com', 'https://other.com'), false);
85+
assert.strictEqual(testUrlMatchesGlob('https://sub.example.com', 'https://example.com'), false);
86+
});
87+
88+
test('URI object input', () => {
89+
const uri = URI.parse('https://example.com/path');
90+
assert.strictEqual(testUrlMatchesGlob(uri, 'https://example.com/path'), true);
91+
assert.strictEqual(testUrlMatchesGlob(uri, 'https://example.com/*'), true);
92+
});
93+
94+
test('complex patterns', () => {
95+
assert.strictEqual(testUrlMatchesGlob('https://api.github.com/repos/microsoft/vscode', 'https://*.github.com/repos/*/*'), true);
96+
assert.strictEqual(testUrlMatchesGlob('https://github.com/microsoft/vscode', 'https://*.github.com/repos/*/*'), false);
97+
assert.strictEqual(testUrlMatchesGlob('https://api.github.com:443/repos/microsoft/vscode', 'https://*.github.com:*/repos/*/*'), true);
98+
});
99+
100+
test('edge cases', () => {
101+
// Wildcard after authority doesn't match without additional path
102+
assert.strictEqual(testUrlMatchesGlob('https://example.com', 'https://example.com*'), false);
103+
assert.strictEqual(testUrlMatchesGlob('https://example.com.extra', 'https://example.com*'), true);
104+
assert.strictEqual(testUrlMatchesGlob('https://example.com', '*'), true);
105+
});
106+
});
107+
});

src/vs/platform/webContentExtractor/common/webContentExtractor.ts

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,11 @@ export interface IWebContentExtractorOptions {
1717
* 'false' by default.
1818
*/
1919
followRedirects?: boolean;
20+
21+
/**
22+
* List of trusted domain patterns for redirect validation.
23+
*/
24+
trustedDomains?: string[];
2025
}
2126

2227
export type WebContentExtractResult =

src/vs/platform/webContentExtractor/electron-main/webContentExtractorService.ts

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import { BrowserWindow } from 'electron';
77
import { Limiter } from '../../../base/common/async.js';
88
import { URI } from '../../../base/common/uri.js';
99
import { ILogService } from '../../log/common/log.js';
10+
import { isURLDomainTrusted } from '../../url/common/trustedDomains.js';
1011
import { IWebContentExtractorOptions, IWebContentExtractorService, WebContentExtractResult } from '../common/webContentExtractor.js';
1112
import { WebContentCache } from './webContentCache.js';
1213
import { WebPageLoader } from './webPageLoader.js';
@@ -37,7 +38,13 @@ export class NativeWebContentExtractorService implements IWebContentExtractorSer
3738
return cached;
3839
}
3940

40-
const loader = new WebPageLoader((options) => new BrowserWindow(options), this._logger, uri, options);
41+
const loader = new WebPageLoader(
42+
(options) => new BrowserWindow(options),
43+
this._logger,
44+
uri,
45+
options,
46+
(uri) => isURLDomainTrusted(uri, options?.trustedDomains || []));
47+
4148
try {
4249
const result = await loader.load();
4350
this._webContentsCache.add(uri, options, result);

src/vs/platform/webContentExtractor/electron-main/webPageLoader.ts

Lines changed: 22 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,6 @@ import type { BeforeSendResponse, BrowserWindow, BrowserWindowConstructorOptions
77
import { Queue, raceTimeout, TimeoutTimer } from '../../../base/common/async.js';
88
import { createSingleCallFunction } from '../../../base/common/functional.js';
99
import { Disposable, toDisposable } from '../../../base/common/lifecycle.js';
10-
import { equalsIgnoreCase } from '../../../base/common/strings.js';
1110
import { URI } from '../../../base/common/uri.js';
1211
import { generateUuid } from '../../../base/common/uuid.js';
1312
import { ILogService } from '../../log/common/log.js';
@@ -44,7 +43,8 @@ export class WebPageLoader extends Disposable {
4443
browserWindowFactory: (options: BrowserWindowConstructorOptions) => BrowserWindow,
4544
private readonly _logger: ILogService,
4645
private readonly _uri: URI,
47-
private readonly _options?: IWebContentExtractorOptions,
46+
private readonly _options: IWebContentExtractorOptions | undefined,
47+
private readonly _isTrustedDomain: (uri: URI) => boolean,
4848
) {
4949
super();
5050

@@ -201,13 +201,30 @@ export class WebPageLoader extends Disposable {
201201
this.trace(`Received 'will-navigate' or 'will-redirect' event, url: ${url}`);
202202
if (!this._options?.followRedirects) {
203203
const toURI = URI.parse(url);
204-
if (!equalsIgnoreCase(toURI.authority, this._uri.authority)) {
205-
event.preventDefault();
206-
this._onResult({ status: 'redirect', toURI });
204+
205+
// Allow redirect if authority is the same when ignoring www prefix
206+
if (this.normalizeAuthority(toURI.authority) === this.normalizeAuthority(this._uri.authority)) {
207+
return;
208+
}
209+
210+
// Allow redirect if target is a trusted domain
211+
if (this._isTrustedDomain(toURI)) {
212+
return;
207213
}
214+
215+
// Otherwise, prevent redirect and report it
216+
event.preventDefault();
217+
this._onResult({ status: 'redirect', toURI });
208218
}
209219
}
210220

221+
/**
222+
* Normalizes an authority by removing the 'www.' prefix if present.
223+
*/
224+
private normalizeAuthority(authority: string): string {
225+
return authority.toLowerCase().replace(/^www\./, '');
226+
}
227+
211228
/**
212229
* Handles debugger messages related to network requests, tracking their lifecycle.
213230
* @note DO NOT add logging to this function, microsoft.com will freeze when too many logs are generated

0 commit comments

Comments
 (0)