Skip to content

Commit 903b3c5

Browse files
thymikeethymikee
authored
fix: restore open-url browser links (#2814)
1 parent 492d235 commit 903b3c5

4 files changed

Lines changed: 135 additions & 96 deletions

File tree

‎packages/cli-server-api/package.json‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,6 @@
1616
"open": "^6.2.0",
1717
"pretty-format": "^29.7.0",
1818
"serve-static": "^1.13.1",
19-
"strict-url-sanitise": "0.0.1",
2019
"ws": "^6.2.3"
2120
},
2221
"devDependencies": {

‎packages/cli-server-api/src/__tests__/openURLMiddleware.test.ts‎

Lines changed: 87 additions & 78 deletions
Original file line numberDiff line numberDiff line change
@@ -5,8 +5,11 @@ import openURLMiddleware from '../openURLMiddleware';
55

66
jest.mock('open');
77

8-
function createMockRequest(method: string, body: object): http.IncomingMessage {
9-
const bodyStr = JSON.stringify(body);
8+
function createMockRequest(
9+
method: string,
10+
body?: object,
11+
): http.IncomingMessage {
12+
const bodyStr = body == null ? '' : JSON.stringify(body);
1013
const readable = new Readable();
1114
readable.push(bodyStr);
1215
readable.push(null);
@@ -21,97 +24,103 @@ function createMockRequest(method: string, body: object): http.IncomingMessage {
2124
}) as unknown as http.IncomingMessage;
2225
}
2326

24-
describe('openURLMiddleware', () => {
25-
let res: jest.Mocked<http.ServerResponse>;
26-
let next: jest.Mock;
27-
28-
beforeEach(() => {
29-
res = {
30-
writeHead: jest.fn(),
31-
end: jest.fn(),
27+
type MiddlewareResponse = {
28+
body?: string;
29+
next: jest.Mock;
30+
statusCode?: number;
31+
};
32+
33+
function callOpenURLMiddleware(
34+
body?: object,
35+
method = 'POST',
36+
): Promise<MiddlewareResponse> {
37+
return new Promise((resolve, reject) => {
38+
const response: MiddlewareResponse = {
39+
next: jest.fn((error?: Error) => {
40+
if (error) {
41+
reject(error);
42+
return;
43+
}
44+
45+
resolve(response);
46+
}),
47+
};
48+
49+
const res = {
50+
writeHead: jest.fn((statusCode: number) => {
51+
response.statusCode = statusCode;
52+
}),
53+
end: jest.fn((message?: string) => {
54+
response.body = message;
55+
resolve(response);
56+
}),
3257
setHeader: jest.fn(),
3358
} as any;
3459

35-
next = jest.fn();
60+
openURLMiddleware(createMockRequest(method, body), res, response.next);
61+
});
62+
}
63+
64+
describe('openURLMiddleware', () => {
65+
beforeEach(() => {
3666
jest.clearAllMocks();
3767
});
3868

3969
afterEach(() => {
4070
jest.restoreAllMocks();
4171
});
4272

43-
test('should return 400 for non-string URL', (done) => {
44-
const req = createMockRequest('POST', {url: 123});
45-
46-
res.end = jest.fn(() => {
47-
try {
48-
expect(open).not.toHaveBeenCalled();
49-
expect(res.writeHead).toHaveBeenCalledWith(400);
50-
expect(res.end).toHaveBeenCalledWith('URL must be a string');
51-
done();
52-
} catch (error) {
53-
done(error);
54-
}
55-
}) as any;
56-
57-
openURLMiddleware(req, res, next);
58-
});
73+
test.each([
74+
'https://reactnative.dev/docs/tutorial',
75+
'https://reactnative.dev/docs/fast-refresh',
76+
'https://x.com/reactnative',
77+
])('should open React Native welcome screen URL %s', async (url) => {
78+
const response = await callOpenURLMiddleware({url});
5979

60-
// CVE-2025-11953
61-
test('should reject malicious URL with invalid hostname', (done) => {
62-
const maliciousUrl = 'https://www.$(calc.exe).com/foo';
63-
const req = createMockRequest('POST', {url: maliciousUrl});
64-
65-
res.end = jest.fn(() => {
66-
try {
67-
expect(open).not.toHaveBeenCalled();
68-
expect(res.writeHead).toHaveBeenCalledWith(400);
69-
expect(res.end).toHaveBeenCalledWith('Invalid URL');
70-
done();
71-
} catch (error) {
72-
done(error);
73-
}
74-
}) as any;
75-
76-
openURLMiddleware(req, res, next);
80+
expect(open).toHaveBeenCalledWith(url);
81+
expect(response.statusCode).toBe(200);
82+
expect(response.next).not.toHaveBeenCalled();
7783
});
7884

79-
// CVE-2025-11953
80-
test('should reject URL with Windows pipe separator', (done) => {
81-
const maliciousUrl = 'https://evil.com?|calc.exe';
82-
const req = createMockRequest('POST', {url: maliciousUrl});
83-
84-
res.end = jest.fn(() => {
85-
try {
86-
expect(open).not.toHaveBeenCalled();
87-
expect(res.writeHead).toHaveBeenCalledWith(400);
88-
expect(res.end).toHaveBeenCalledWith('Invalid URL');
89-
done();
90-
} catch (error) {
91-
done(error);
92-
}
93-
}) as any;
94-
95-
openURLMiddleware(req, res, next);
85+
test('should return 400 for non-string URL', async () => {
86+
const response = await callOpenURLMiddleware({url: 123});
87+
88+
expect(open).not.toHaveBeenCalled();
89+
expect(response.statusCode).toBe(400);
90+
expect(response.body).toBe('URL must be a string');
9691
});
9792

9893
// CVE-2025-11953
99-
test('should reject URL with Windows command exfiltration', (done) => {
100-
// Encodes to reveal %BETA% env var
101-
const maliciousUrl = 'https://example.com/?a=%¾TA%';
102-
const req = createMockRequest('POST', {url: maliciousUrl});
103-
104-
res.end = jest.fn(() => {
105-
try {
106-
expect(open).not.toHaveBeenCalled();
107-
expect(res.writeHead).toHaveBeenCalledWith(400);
108-
expect(res.end).toHaveBeenCalledWith('Invalid URL');
109-
done();
110-
} catch (error) {
111-
done(error);
112-
}
113-
}) as any;
114-
115-
openURLMiddleware(req, res, next);
94+
test.each([
95+
['JFrog bare executable command', 'calc.exe'],
96+
['JFrog nested cmd RCE command', 'cmd /c echo abc > c:\\temp\\pwned.txt'],
97+
['Windows command prefix', '& calc.exe'],
98+
[
99+
'URL followed by Windows command separator',
100+
'https://example.com & calc.exe',
101+
],
102+
['malicious URL with invalid hostname', 'https://www.$(calc.exe).com/foo'],
103+
['URL with Windows pipe separator', 'https://evil.com?|calc.exe'],
104+
['URL with Windows caret separator', 'https://example.com/?x=^calc'],
105+
['URL with Windows command exfiltration', 'https://example.com/?a=%¾TA%'],
106+
['URL with Windows delayed expansion', 'https://example.com/?x=!PATH!'],
107+
[
108+
'URL with Windows redirect metacharacter',
109+
'https://example.com/?x=>out.txt',
110+
],
111+
[
112+
'URL with Windows metacharacter in userinfo',
113+
'https://u:p|ss@example.com/',
114+
],
115+
['file URL scheme', 'file:///etc/passwd'],
116+
['javascript URL scheme', 'javascript:alert(1)'],
117+
['custom URL scheme', 'ms-msdt:/id'],
118+
['IPv6 hostname with injected metacharacter', 'https://[::1|x]/'],
119+
])('should reject %s', async (_name, url) => {
120+
const response = await callOpenURLMiddleware({url});
121+
122+
expect(open).not.toHaveBeenCalled();
123+
expect(response.statusCode).toBe(400);
124+
expect(response.body).toBe('Invalid URL');
116125
});
117126
});

‎packages/cli-server-api/src/openURLMiddleware.ts‎

Lines changed: 48 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,46 @@ import type {IncomingMessage, ServerResponse} from 'http';
1010
import {json} from 'body-parser';
1111
import connect from 'connect';
1212
import open from 'open';
13-
import {sanitizeUrl} from 'strict-url-sanitise';
13+
14+
// open@6 launches URLs through `cmd /c start` on Windows and only escapes `&`.
15+
// Reject the remaining cmd metacharacters, including `%` and `!` expansion,
16+
// even though this also rejects some otherwise-valid percent-encoded URLs.
17+
const WINDOWS_SHELL_SPECIAL_CHARS = /[|<>^%!]/;
18+
const INVALID_URL = 'Invalid URL';
19+
20+
function sendResponse(
21+
res: ServerResponse,
22+
statusCode: number,
23+
message?: string,
24+
) {
25+
res.writeHead(statusCode);
26+
res.end(message);
27+
}
28+
29+
function isSafeHostname(hostname: string) {
30+
return (
31+
(hostname.startsWith('[') && hostname.endsWith(']')) ||
32+
hostname === encodeURIComponent(hostname)
33+
);
34+
}
35+
36+
function validateURLForOpen(url: string) {
37+
const parsedUrl = new URL(url);
38+
39+
if (parsedUrl.protocol !== 'http:' && parsedUrl.protocol !== 'https:') {
40+
throw new Error('Invalid URL protocol');
41+
}
42+
43+
if (!isSafeHostname(parsedUrl.hostname)) {
44+
throw new Error('Invalid URL hostname');
45+
}
46+
47+
if (WINDOWS_SHELL_SPECIAL_CHARS.test(parsedUrl.href)) {
48+
throw new Error('Invalid URL characters');
49+
}
50+
51+
return parsedUrl.href;
52+
}
1453

1554
/**
1655
* Open a URL in the system browser.
@@ -25,32 +64,29 @@ async function openURLMiddleware(
2564
) {
2665
if (req.method === 'POST') {
2766
if (req.body == null) {
28-
res.writeHead(400);
29-
res.end('Missing request body');
67+
sendResponse(res, 400, 'Missing request body');
3068
return;
3169
}
3270

3371
const {url} = req.body as {url: string};
3472

3573
if (typeof url !== 'string') {
36-
res.writeHead(400);
37-
res.end('URL must be a string');
74+
sendResponse(res, 400, 'URL must be a string');
3875
return;
3976
}
4077

41-
let sanitizedUrl: string;
78+
let validatedUrl;
4279
try {
43-
sanitizedUrl = sanitizeUrl(url);
80+
validatedUrl = validateURLForOpen(url);
4481
} catch {
45-
res.writeHead(400);
46-
res.end('Invalid URL');
82+
sendResponse(res, 400, INVALID_URL);
4783
return;
4884
}
4985

50-
await open(sanitizedUrl);
86+
await open(validatedUrl);
5187

52-
res.writeHead(200);
53-
res.end();
88+
sendResponse(res, 200);
89+
return;
5490
}
5591

5692
next();

‎yarn.lock‎

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -9936,11 +9936,6 @@ stream-buffers@2.2.x:
99369936
resolved "https://registry.yarnpkg.com/stream-buffers/-/stream-buffers-2.2.0.tgz#91d5f5130d1cef96dcfa7f726945188741d09ee4"
99379937
integrity sha512-uyQK/mx5QjHun80FLJTfaWE7JtwfRMKBLkMne6udYOmvH0CawotVa7TfgYHzAnpphn4+TweIx1QKMnRIbipmUg==
99389938

9939-
strict-url-sanitise@0.0.1:
9940-
version "0.0.1"
9941-
resolved "https://registry.yarnpkg.com/strict-url-sanitise/-/strict-url-sanitise-0.0.1.tgz#10cfac63c9dfdd856d98ab9f76433dad5ce99e0c"
9942-
integrity sha512-nuFtF539K8jZg3FjaWH/L8eocCR6gegz5RDOsaWxfdbF5Jqr2VXWxZayjTwUzsWJDC91k2EbnJXp6FuWW+Z4hg==
9943-
99449939
string-argv@^0.3.1:
99459940
version "0.3.1"
99469941
resolved "https://registry.yarnpkg.com/string-argv/-/string-argv-0.3.1.tgz#95e2fbec0427ae19184935f816d74aaa4c5c19da"

0 commit comments

Comments
 (0)