Skip to content

Commit ab41c3e

Browse files
PrincesseuhPrincesseuh
authored
fix(errors): Only style valid URLs in the error overlay (#15324)
* fix(errors): Only style valid URLs in the error overlay * chore: changeset * feat: unit tests
1 parent 02ee3c7 commit ab41c3e

3 files changed

Lines changed: 155 additions & 9 deletions

File tree

‎.changeset/three-lizards-wish.md‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
'astro': patch
3+
---
4+
5+
Fixes an issue where certain unauthorized links could be rendered as clickable in the error overlay

‎packages/astro/src/core/errors/dev/utils.ts‎

Lines changed: 26 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -246,19 +246,43 @@ const boldRegex = /\*\*(.+)\*\*/g;
246246
const urlRegex = / ((?:https?|ftp):\/\/[-\w+&@#\\/%?=~|!:,.;]*[-\w+&@#\\/%=~|])/gi;
247247
const codeRegex = /`([^`]+)`/g;
248248

249+
function isAllowedUrl(url: string): boolean {
250+
const trimmedUrl = url.trim();
251+
if (!trimmedUrl) return false;
252+
253+
try {
254+
const parsedUrl = new URL(trimmedUrl);
255+
return ['http:', 'https:'].includes(parsedUrl.protocol);
256+
} catch {
257+
return false;
258+
}
259+
}
260+
249261
/**
250262
* Render a subset of Markdown to HTML or a CLI output
251263
*/
252264
export function renderErrorMarkdown(markdown: string, target: 'html' | 'cli') {
253265
if (target === 'html') {
254266
return escape(markdown)
255-
.replace(linkRegex, `<a href="$2" target="_blank">$1</a>`)
267+
.replace(linkRegex, (_match, text, url) => {
268+
if (!isAllowedUrl(url)) {
269+
return text;
270+
}
271+
272+
return `<a href="${url}" target="_blank">${text}</a>`;
273+
})
256274
.replace(boldRegex, '<b>$1</b>')
257275
.replace(urlRegex, ' <a href="$1" target="_blank">$1</a>')
258276
.replace(codeRegex, '<code>$1</code>');
259277
} else {
260278
return markdown
261-
.replace(linkRegex, (_, m1, m2) => `${colors.bold(m1)} ${colors.underline(m2)}`)
279+
.replace(linkRegex, (_, m1, m2) => {
280+
if (!isAllowedUrl(m2)) {
281+
return `${colors.bold(m1)} ${m2}`;
282+
}
283+
284+
return `${colors.bold(m1)} ${colors.underline(m2)}`;
285+
})
262286
.replace(urlRegex, (fullMatch) => ` ${colors.underline(fullMatch.trim())}`)
263287
.replace(boldRegex, (_, m1) => `${colors.bold(m1)}`);
264288
}

‎packages/astro/test/units/errors/dev-utils.test.js‎

Lines changed: 124 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -51,26 +51,26 @@ describe('renderErrorMarkdown', () => {
5151
it('handles link with parentheses followed by more content', () => {
5252
// This is the bug case from issue #15068
5353
// The link [text](url) should not consume content after it
54-
const input = 'use [text](url) for links';
54+
const input = 'use [text](https://example.com) for links';
5555
const result = renderErrorMarkdown(input, 'html');
56-
assert.equal(result, 'use <a href="url" target="_blank">text</a> for links');
56+
assert.equal(result, 'use <a href="https://example.com" target="_blank">text</a> for links');
5757
});
5858

5959
it('handles link followed by closing parenthesis', () => {
6060
// Edge case: link inside parentheses like "(use [text](url))"
61-
const input = '(use [text](url))';
61+
const input = '(use [text](https://example.com))';
6262
const result = renderErrorMarkdown(input, 'html');
63-
// The link should only capture 'url', not 'url)'
64-
assert.equal(result, '(use <a href="url" target="_blank">text</a>)');
63+
// The link should only capture the URL, not the closing paren
64+
assert.equal(result, '(use <a href="https://example.com" target="_blank">text</a>)');
6565
});
6666

6767
it('handles escaped HTML followed by link syntax', () => {
6868
// This simulates the MDX error message case
69-
const input = 'use <code>[text](url)</code>';
69+
const input = 'use <code>[text](https://example.com)</code>';
7070
const result = renderErrorMarkdown(input, 'html');
7171
// After HTML escaping, <code> becomes &lt;code&gt;
7272
// The link should still be parsed correctly without consuming &gt;)
73-
assert.ok(result.includes('<a href="url" target="_blank">text</a>'));
73+
assert.ok(result.includes('<a href="https://example.com" target="_blank">text</a>'));
7474
assert.ok(result.includes('&lt;code&gt;'));
7575
assert.ok(result.includes('&lt;/code&gt;'));
7676
});
@@ -83,6 +83,84 @@ describe('renderErrorMarkdown', () => {
8383
});
8484
});
8585

86+
describe('only allows proper links in the dev overlay', () => {
87+
it('blocks javascript: URLs in links', () => {
88+
const input = '[click me](javascript:alert(1))';
89+
const result = renderErrorMarkdown(input, 'html');
90+
// Should not create a link
91+
assert.ok(!result.includes('<a'));
92+
assert.ok(!result.includes('javascript:'));
93+
// Should contain the link text
94+
assert.ok(result.includes('click me'));
95+
});
96+
97+
it('blocks data: URLs in links', () => {
98+
const input = '[click me](data:text/html,<script>alert(1)</script>)';
99+
const result = renderErrorMarkdown(input, 'html');
100+
// Should not create a link
101+
assert.ok(!result.includes('<a'));
102+
assert.ok(!result.includes('data:'));
103+
// Should contain the link text
104+
assert.ok(result.includes('click me'));
105+
});
106+
107+
it('blocks file: URLs in links', () => {
108+
const input = '[click me](file:///etc/passwd)';
109+
const result = renderErrorMarkdown(input, 'html');
110+
// Should render as plain text, not a link
111+
assert.equal(result, 'click me');
112+
assert.ok(!result.includes('file:'));
113+
assert.ok(!result.includes('<a'));
114+
});
115+
116+
it('blocks relative URLs in links', () => {
117+
const input = '[home](/index.html)';
118+
const result = renderErrorMarkdown(input, 'html');
119+
// Should render as plain text, not a link
120+
assert.equal(result, 'home');
121+
assert.ok(!result.includes('/index.html'));
122+
assert.ok(!result.includes('<a'));
123+
});
124+
125+
it('blocks malformed URLs in links', () => {
126+
const input = '[click me](not-a-valid-url)';
127+
const result = renderErrorMarkdown(input, 'html');
128+
// Should render as plain text, not a link
129+
assert.equal(result, 'click me');
130+
assert.ok(!result.includes('<a'));
131+
});
132+
133+
it('allows http: URLs in links', () => {
134+
const input = '[example](http://example.com)';
135+
const result = renderErrorMarkdown(input, 'html');
136+
assert.equal(result, '<a href="http://example.com" target="_blank">example</a>');
137+
});
138+
139+
it('allows https: URLs in links', () => {
140+
const input = '[example](https://example.com)';
141+
const result = renderErrorMarkdown(input, 'html');
142+
assert.equal(result, '<a href="https://example.com" target="_blank">example</a>');
143+
});
144+
145+
it('handles mixed safe and unsafe links', () => {
146+
const input = '[safe](https://example.com) and [unsafe](javascript:alert(1))';
147+
const result = renderErrorMarkdown(input, 'html');
148+
assert.ok(result.includes('<a href="https://example.com" target="_blank">safe</a>'));
149+
assert.ok(result.includes('unsafe'));
150+
assert.ok(!result.includes('javascript:'));
151+
});
152+
153+
it('blocks case-insensitive javascript: URLs', () => {
154+
const input = '[click me](JavaScript:alert(1))';
155+
const result = renderErrorMarkdown(input, 'html');
156+
// Should not create a link
157+
assert.ok(!result.includes('<a'));
158+
assert.ok(!result.includes('JavaScript:'));
159+
// Should contain the link text
160+
assert.ok(result.includes('click me'));
161+
});
162+
});
163+
86164
describe('cli target', () => {
87165
it('formats markdown links for CLI output', () => {
88166
const input = 'Check the [documentation](https://docs.astro.build)';
@@ -104,4 +182,43 @@ describe('renderErrorMarkdown', () => {
104182
assert.ok(result.includes('https://astro.build'));
105183
});
106184
});
185+
186+
describe('only allows proper links in the terminal', () => {
187+
it('blocks javascript: URLs in links', () => {
188+
const input = '[click me](javascript:alert(1))';
189+
const result = renderErrorMarkdown(input, 'cli');
190+
// Should render with plain text, URL shown but not styled
191+
assert.ok(result.includes('click me'));
192+
assert.ok(result.includes('javascript:alert(1)'));
193+
});
194+
195+
it('blocks data: URLs in links', () => {
196+
const input = '[click me](data:text/html,<script>alert(1)</script>)';
197+
const result = renderErrorMarkdown(input, 'cli');
198+
assert.ok(result.includes('click me'));
199+
assert.ok(result.includes('data:text/html'));
200+
});
201+
202+
it('blocks relative URLs in links', () => {
203+
const input = '[home](/index.html)';
204+
const result = renderErrorMarkdown(input, 'cli');
205+
assert.ok(result.includes('home'));
206+
assert.ok(result.includes('/index.html'));
207+
});
208+
209+
it('allows safe URLs in links', () => {
210+
const input = '[docs](https://docs.astro.build)';
211+
const result = renderErrorMarkdown(input, 'cli');
212+
assert.ok(result.includes('docs'));
213+
assert.ok(result.includes('https://docs.astro.build'));
214+
});
215+
216+
it('handles mixed safe and unsafe links', () => {
217+
const input = '[safe](https://example.com) and [unsafe](javascript:alert(1))';
218+
const result = renderErrorMarkdown(input, 'cli');
219+
assert.ok(result.includes('https://example.com'));
220+
assert.ok(result.includes('unsafe'));
221+
assert.ok(result.includes('javascript:alert(1)'));
222+
});
223+
});
107224
});

0 commit comments

Comments
 (0)