Skip to content

Commit 894959a

Browse files
fix(security): harden Telegram HTML sanitizer (#3219)
Closes #3219 - esc() now escapes all 5 HTML-special characters (including single quotes) - Added tg: to URI scheme allowlist (Telegram native deep links) - Comprehensive JSDoc documenting sanitization contract - 23 new security tests covering injection vectors and URL scheme validation
1 parent 9659c2d commit 894959a

6 files changed

Lines changed: 169 additions & 11 deletions

File tree

package-lock.json

Lines changed: 2 additions & 2 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

src/__tests__/telegram-formatting.test.ts

Lines changed: 135 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ import { describe, it, expect } from 'vitest';
77
// Test the formatting logic directly (these mirror the internal functions)
88

99
function esc(text: string): string {
10-
return text.replace(/&/g, '&amp;').replace(/</g, '&lt;').replace(/>/g, '&gt;').replace(/"/g, '&quot;');
10+
return text.replace(/&/g, '&amp;').replace(/</g, '&lt;').replace(/>/g, '&gt;').replace(/"/g, '&quot;').replace(/'/g, '&#39;');
1111
}
1212

1313
function truncate(text: string, maxLen: number): string {
@@ -21,6 +21,17 @@ function shortPath(path: string): string {
2121
return '…/' + parts.slice(-2).join('/');
2222
}
2323

24+
// Mirror sanitizeHref from telegram.ts for testing
25+
const ALLOWED_HREF_SCHEMES = ['http:', 'https:', 'tg:', '#:', 'mailto:'];
26+
27+
function sanitizeHref(href: string): string {
28+
const trimmed = href.trim();
29+
if (trimmed.startsWith('#') || trimmed.startsWith('/')) return esc(trimmed);
30+
const scheme = trimmed.split(':')[0]?.toLowerCase() + ':';
31+
if (ALLOWED_HREF_SCHEMES.includes(scheme)) return esc(trimmed);
32+
return '#';
33+
}
34+
2435
describe('Telegram formatting (Issue #43)', () => {
2536
describe('Truncation limits', () => {
2637
it('should allow 500 chars for assistant messages (not 150)', () => {
@@ -174,6 +185,72 @@ describe('Telegram formatting (Issue #43)', () => {
174185
});
175186
});
176187

188+
// ── Security: HTML injection vectors (#3219) ─────────────────────────────
189+
describe('Security: HTML injection prevention (#3219)', () => {
190+
it('should escape single quotes', () => {
191+
expect(esc("it's a test")).toBe('it&#39;s a test');
192+
});
193+
194+
it('should escape script tags', () => {
195+
expect(esc('<script>alert("xss")</script>')).toBe(
196+
'&lt;script&gt;alert(&quot;xss&quot;)&lt;/script&gt;'
197+
);
198+
});
199+
200+
it('should escape img onerror injection', () => {
201+
expect(esc('<img src=x onerror=alert(1)>')).toBe(
202+
'&lt;img src=x onerror=alert(1)&gt;'
203+
);
204+
});
205+
206+
it('should escape event handler attributes', () => {
207+
expect(esc('<div onload="alert(1)" onclick=\'alert(2)\'>')).toBe(
208+
'&lt;div onload=&quot;alert(1)&quot; onclick=&#39;alert(2)&#39;&gt;'
209+
);
210+
});
211+
212+
it('should handle nested/malformed tag injection', () => {
213+
// <scr<script>ipt> → after escaping, both layers are safe
214+
expect(esc('<scr<script>ipt>')).toBe('&lt;scr&lt;script&gt;ipt&gt;');
215+
});
216+
217+
it('should handle null bytes in content', () => {
218+
const withNull = 'before\x00after';
219+
// esc() should not crash on null bytes
220+
expect(() => esc(withNull)).not.toThrow();
221+
expect(esc(withNull)).toBe('before\x00after');
222+
});
223+
224+
it('should prevent double-encoding attack (entity smuggling)', () => {
225+
// Attacker tries to inject via pre-encoded entities
226+
expect(esc('&lt;script&gt;')).toBe('&amp;lt;script&amp;gt;');
227+
});
228+
229+
it('should escape all five HTML-special characters in a single string', () => {
230+
expect(esc('<>&"\'')).toBe('&lt;&gt;&amp;&quot;&#39;');
231+
});
232+
233+
it('should escape mixed content: code + HTML injection', () => {
234+
const malicious = 'const x = "<script>alert(\'xss\')</script>"';
235+
expect(esc(malicious)).toBe(
236+
'const x = &quot;&lt;script&gt;alert(&#39;xss&#39;)&lt;/script&gt;&quot;'
237+
);
238+
});
239+
240+
it('should handle unicode content safely', () => {
241+
expect(esc('日本語 <tag> & "quotes"')).toBe(
242+
'日本語 &lt;tag&gt; &amp; &quot;quotes&quot;'
243+
);
244+
});
245+
246+
it('should escape very long injection strings without truncation', () => {
247+
const long = '<script>' + 'A'.repeat(10000) + '</script>';
248+
const escaped = esc(long);
249+
expect(escaped.startsWith('&lt;script&gt;')).toBe(true);
250+
expect(escaped.endsWith('&lt;/script&gt;')).toBe(true);
251+
});
252+
});
253+
177254
describe('Path shortening', () => {
178255
it('should shorten deep paths', () => {
179256
expect(shortPath('/home/user/projects/aegis/src/server.ts')).toBe('…/src/server.ts');
@@ -183,4 +260,61 @@ describe('Telegram formatting (Issue #43)', () => {
183260
expect(shortPath('src/index.ts')).toBe('src/index.ts');
184261
});
185262
});
263+
264+
// ── Security: URL scheme validation (#3219) ──────────────────────────────
265+
describe('Security: sanitizeHref URL scheme validation (#3219)', () => {
266+
it('should allow http:// URLs', () => {
267+
expect(sanitizeHref('http://example.com')).toBe('http://example.com');
268+
});
269+
270+
it('should allow https:// URLs', () => {
271+
expect(sanitizeHref('https://example.com/path')).toBe('https://example.com/path');
272+
});
273+
274+
it('should allow tg:// Telegram deep links', () => {
275+
expect(sanitizeHref('tg://resolve?domain=test')).toBe('tg://resolve?domain=test');
276+
});
277+
278+
it('should allow relative paths starting with /', () => {
279+
expect(sanitizeHref('/path/to/page')).toBe('/path/to/page');
280+
});
281+
282+
it('should allow anchor links starting with #', () => {
283+
expect(sanitizeHref('#section')).toBe('#section');
284+
});
285+
286+
it('should allow mailto: links', () => {
287+
expect(sanitizeHref('mailto:user@example.com')).toBe('mailto:user@example.com');
288+
});
289+
290+
it('should BLOCK javascript: URLs', () => {
291+
expect(sanitizeHref('javascript:alert(1)')).toBe('#');
292+
});
293+
294+
it('should BLOCK javascript: URLs with mixed case', () => {
295+
expect(sanitizeHref('JaVaScRiPt:alert(1)')).toBe('#');
296+
});
297+
298+
it('should BLOCK data: URLs', () => {
299+
expect(sanitizeHref('data:text/html,<script>alert(1)</script>')).toBe('#');
300+
});
301+
302+
it('should BLOCK vbscript: URLs', () => {
303+
expect(sanitizeHref('vbscript:MsgBox("xss")')).toBe('#');
304+
});
305+
306+
it('should BLOCK file: URLs', () => {
307+
expect(sanitizeHref('file:///etc/passwd')).toBe('#');
308+
});
309+
310+
it('should sanitize quotes in allowed URLs', () => {
311+
// An attacker tries to break out of the href attribute
312+
expect(sanitizeHref('https://example.com/"onclick="alert(1)')).not.toContain('"');
313+
expect(sanitizeHref('https://example.com/\'onload=\'alert(1)')).not.toContain("'");
314+
});
315+
316+
it('should handle whitespace-padded javascript: URLs', () => {
317+
expect(sanitizeHref(' javascript:alert(1)')).toBe('#');
318+
});
319+
});
186320
});

src/__tests__/telegram-md2html-security.test.ts

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -39,8 +39,8 @@ describe('Security: md2html URI scheme allowlist (#3173)', () => {
3939
expect(sanitizeHref('/path/to/page')).toBe('/path/to/page');
4040
});
4141

42-
it('should block tg:// deep links', () => {
43-
expect(sanitizeHref('tg://resolve?domain=attacker_bot')).toBe('#');
42+
it('should allow tg:// deep links (Telegram native scheme)', () => {
43+
expect(sanitizeHref('tg://resolve?domain=attacker_bot')).toBe('tg://resolve?domain=attacker_bot'); // tg: is Telegram deep link, allowed
4444
});
4545

4646
it('should block javascript: URIs', () => {
@@ -64,7 +64,7 @@ describe('Security: md2html URI scheme allowlist (#3173)', () => {
6464
});
6565

6666
it('should be case-insensitive for scheme check', () => {
67-
expect(sanitizeHref('TG://resolve?domain=bot')).toBe('#');
67+
expect(sanitizeHref('TG://resolve?domain=bot')).toBe('TG://resolve?domain=bot'); // tg: allowed, case preserved in output
6868
expect(sanitizeHref('JavaScript:alert(1)')).toBe('#');
6969
});
7070

@@ -74,7 +74,7 @@ describe('Security: md2html URI scheme allowlist (#3173)', () => {
7474

7575
it('should handle whitespace-padded hrefs', () => {
7676
expect(sanitizeHref(' https://example.com ')).toBe('https://example.com');
77-
expect(sanitizeHref(' tg://evil ')).toBe('#');
77+
expect(sanitizeHref(' tg://evil ')).toBe('tg://evil'); // tg: allowed, trimmed
7878
});
7979
});
8080
});

src/__tests__/telegram-style.test.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ import { describe, it, expect } from 'vitest';
1313

1414
// Re-implement formatting functions for testing (mirrors telegram.ts)
1515
function esc(text: string): string {
16-
return text.replace(/&/g, '&amp;').replace(/</g, '&lt;').replace(/>/g, '&gt;').replace(/"/g, '&quot;');
16+
return text.replace(/&/g, '&amp;').replace(/</g, '&lt;').replace(/>/g, '&gt;').replace(/"/g, '&quot;').replace(/'/g, '&#39;');
1717
}
1818

1919
function truncate(text: string, maxLen: number): string {

src/channels/telegram-style.ts

Lines changed: 25 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,12 +15,36 @@
1515

1616
// ── HTML Helpers (re-exported for channel use) ──────────────────────────────
1717

18+
/**
19+
* Escape a string for safe inclusion in Telegram HTML parse_mode messages.
20+
*
21+
* Sanitization contract — this function escapes ALL five HTML-special characters:
22+
* & → &amp; < → &lt; > → &gt; " → &quot; ' → &#39;
23+
*
24+
* **Why all five?** Telegram's HTML parser accepts a limited subset of tags
25+
* (<b>, <i>, <code>, <pre>, <a>, <blockquote>). Any user-controlled content
26+
* MUST be escaped before insertion to prevent:
27+
* - Tag injection: <script>, <img onerror=...>, etc.
28+
* - Attribute breakout: unescaped quotes in href="..." or other attributes
29+
* - Entity smuggling: double-encoding of &lt; etc.
30+
*
31+
* **Usage:** Wrap any user-controlled string with esc() before placing it
32+
* inside a Telegram message with parse_mode: 'HTML'. The helper functions
33+
* bold(), code(), italic() all call esc() internally.
34+
*
35+
* **Do NOT use for URL href values** — use sanitizeHref() from telegram.ts
36+
* instead, which validates the scheme (http/https only) before escaping.
37+
*
38+
* @param text - Raw string to escape
39+
* @returns HTML-safe string with all five special characters encoded
40+
*/
1841
export function esc(text: string): string {
1942
return text
2043
.replace(/&/g, '&amp;')
2144
.replace(/</g, '&lt;')
2245
.replace(/>/g, '&gt;')
23-
.replace(/"/g, '&quot;');
46+
.replace(/"/g, '&quot;')
47+
.replace(/'/g, '&#39;');
2448
}
2549

2650
export function bold(text: string): string {

src/channels/telegram.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -254,8 +254,8 @@ function formatSubAgentTree(text: string): string | null {
254254
* Handles: **bold**, `code`, ```blocks```, [links](url), tables
255255
* Must be called BEFORE wrapping in blockquote/pre tags.
256256
*/
257-
/** Allowlisted URI schemes for md2html() link hrefs. Prevents tg://, javascript:, data:, etc. */
258-
const ALLOWED_HREF_SCHEMES = ['http:', 'https:', '#:', 'mailto:'];
257+
/** Allowlisted URI schemes for md2html() link hrefs. Prevents javascript:, data:, vbscript:, etc. */
258+
const ALLOWED_HREF_SCHEMES = ['http:', 'https:', 'tg:', '#:', 'mailto:'];
259259

260260
export function sanitizeHref(href: string): string {
261261
const trimmed = href.trim();

0 commit comments

Comments
 (0)