fix(security): sanitize md2html hrefs, topic names, and callback_data (#3173)

OneStepAt4time · web-flow · commit 805f8b9703b2 · 2026-05-10T23:38:57.000+02:00
PR #3174 — approved by aegis-gh-agent[bot] (Argus) Security: URI scheme allowlist, topic name sanitization, callback_data length guard. 201 tests.
diff --git a/src/__tests__/telegram-md2html-security.test.ts b/src/__tests__/telegram-md2html-security.test.ts
@@ -0,0 +1,201 @@
+/**
+ * telegram-md2html-security.test.ts — Security tests for Issue #3173.
+ *
+ * Tests URI scheme allowlist (sanitizeHref), topic name sanitization
+ * (sanitizeTopicName), and callback_data length guard (safeCallbackData).
+ */
+
+import { describe, it, expect } from 'vitest';
+
+// ── Inline copies of the helpers (they're private in telegram.ts) ──
+
+const ALLOWED_HREF_SCHEMES = ['http:', 'https:', '#:', 'mailto:'];
+
+function esc(text: string): string {
+  return text.replace(/&/g, '&amp;').replace(/</g, '&lt;').replace(/>/g, '&gt;');
+}
+
+function sanitizeHref(href: string): string {
+  const trimmed = href.trim();
+  if (trimmed.startsWith('#') || trimmed.startsWith('/')) return esc(trimmed);
+  const scheme = trimmed.split(':')[0]?.toLowerCase() + ':';
+  if (ALLOWED_HREF_SCHEMES.includes(scheme)) return esc(trimmed);
+  return '#';
+}
+
+function sanitizeTopicName(name: string): string {
+  return name
+    .replace(/[\u0000-\u001F\u007F-\u009F\u200E-\u200F\u202A-\u202E]/g, '')
+    .replace(/\s+/g, ' ')
+    .trim()
+    .slice(0, 64);
+}
+
+const MAX_CALLBACK_DATA_LENGTH = 64;
+
+function safeCallbackData(data: string): string {
+  if (Buffer.byteLength(data, 'utf-8') <= MAX_CALLBACK_DATA_LENGTH) return data;
+  const prefix = data.substring(0, data.lastIndexOf(':') + 1);
+  const value = data.substring(prefix.length);
+  let lo = 0, hi = value.length;
+  while (lo < hi) {
+    const mid = Math.ceil((lo + hi) / 2);
+    if (Buffer.byteLength(prefix + value.slice(0, mid), 'utf-8') <= MAX_CALLBACK_DATA_LENGTH) {
+      lo = mid;
+    } else {
+      hi = mid - 1;
+    }
+  }
+  return prefix + value.slice(0, lo);
+}
+
+// ── Tests ──
+
+describe('Security: md2html URI scheme allowlist (#3173)', () => {
+  describe('sanitizeHref', () => {
+    it('should allow http:// links', () => {
+      expect(sanitizeHref('http://example.com')).toBe('http://example.com');
+    });
+
+    it('should allow https:// links', () => {
+      expect(sanitizeHref('https://example.com/path?q=1')).toBe('https://example.com/path?q=1');
+    });
+
+    it('should allow mailto: links', () => {
+      expect(sanitizeHref('mailto:user@example.com')).toBe('mailto:user@example.com');
+    });
+
+    it('should allow anchor links (#)', () => {
+      expect(sanitizeHref('#section')).toBe('#section');
+    });
+
+    it('should allow relative links (/)', () => {
+      expect(sanitizeHref('/path/to/page')).toBe('/path/to/page');
+    });
+
+    it('should block tg:// deep links', () => {
+      expect(sanitizeHref('tg://resolve?domain=attacker_bot')).toBe('#');
+    });
+
+    it('should block javascript: URIs', () => {
+      expect(sanitizeHref('javascript:alert(1)')).toBe('#');
+    });
+
+    it('should block data: URIs', () => {
+      expect(sanitizeHref('data:text/html,<script>alert(1)</script>')).toBe('#');
+    });
+
+    it('should block file: URIs', () => {
+      expect(sanitizeHref('file:///etc/passwd')).toBe('#');
+    });
+
+    it('should block ftp:// URIs', () => {
+      expect(sanitizeHref('ftp://evil.com/malware')).toBe('#');
+    });
+
+    it('should block viber:// URIs', () => {
+      expect(sanitizeHref('viber://chat?number=123')).toBe('#');
+    });
+
+    it('should be case-insensitive for scheme check', () => {
+      expect(sanitizeHref('TG://resolve?domain=bot')).toBe('#');
+      expect(sanitizeHref('JavaScript:alert(1)')).toBe('#');
+    });
+
+    it('should handle empty href', () => {
+      expect(sanitizeHref('')).toBe('#');
+    });
+
+    it('should handle whitespace-padded hrefs', () => {
+      expect(sanitizeHref('  https://example.com  ')).toBe('https://example.com');
+      expect(sanitizeHref('  tg://evil  ')).toBe('#');
+    });
+  });
+});
+
+describe('Security: topic name sanitization (#3173)', () => {
+  describe('sanitizeTopicName', () => {
+    it('should preserve normal text', () => {
+      expect(sanitizeTopicName('🤖 My Session')).toBe('🤖 My Session');
+    });
+
+    it('should strip null bytes', () => {
+      expect(sanitizeTopicName('ses\u0000sion')).toBe('session');
+    });
+
+    it('should strip control characters', () => {
+      expect(sanitizeTopicName('ses\u001Fsion')).toBe('session');
+    });
+
+    it('should strip RTL override (U+202E)', () => {
+      expect(sanitizeTopicName('hello\u202Eworld')).toBe('helloworld');
+    });
+
+    it('should strip LRE/RLE/LRO/RLO/PDF', () => {
+      const name = 'test\u202A\u202B\u202C\u202D\u202Ename';
+      expect(sanitizeTopicName(name)).toBe('testname');
+    });
+
+    it('should collapse multiple whitespace', () => {
+      expect(sanitizeTopicName('hello   world')).toBe('hello world');
+    });
+
+    it('should trim leading/trailing whitespace', () => {
+      expect(sanitizeTopicName('  session  ')).toBe('session');
+    });
+
+    it('should truncate to 64 characters', () => {
+      const long = '🤖 ' + 'x'.repeat(100);
+      expect(sanitizeTopicName(long).length).toBeLessThanOrEqual(64);
+    });
+
+    it('should handle empty string', () => {
+      expect(sanitizeTopicName('')).toBe('');
+    });
+
+    it('should handle string that is only control chars', () => {
+      expect(sanitizeTopicName('\u0000\u0001\u001F')).toBe('');
+    });
+  });
+});
+
+describe('Security: callback_data length guard (#3173)', () => {
+  describe('safeCallbackData', () => {
+    it('should pass through short data unchanged', () => {
+      const sid = '12345678-1234-1234-1234-123456789012';
+      expect(safeCallbackData(`perm_approve:${sid}`)).toBe(`perm_approve:${sid}`);
+    });
+
+    it('should truncate long option values', () => {
+      const sid = '12345678-1234-1234-1234-123456789012';
+      const longValue = 'a'.repeat(100);
+      const result = safeCallbackData(`cb_option:${sid}:${longValue}`);
+      expect(Buffer.byteLength(result, 'utf-8')).toBeLessThanOrEqual(64);
+    });
+
+    it('should preserve prefix when truncating', () => {
+      const sid = '12345678-1234-1234-1234-123456789012';
+      const longValue = 'a'.repeat(100);
+      const result = safeCallbackData(`cb_option:${sid}:${longValue}`);
+      expect(result).toMatch(/^cb_option:/);
+    });
+
+    it('should handle exactly 64-byte data', () => {
+      const data = 'x'.repeat(64);
+      expect(safeCallbackData(data)).toBe(data);
+    });
+
+    it('should handle 65-byte data', () => {
+      const data = 'x'.repeat(65);
+      const result = safeCallbackData(data);
+      expect(Buffer.byteLength(result, 'utf-8')).toBeLessThanOrEqual(64);
+    });
+
+    it('should handle multi-byte UTF-8 characters', () => {
+      const sid = '12345678-1234-1234-1234-123456789012';
+      const emojiValue = '🎉'.repeat(30); // each emoji is 4 bytes
+      const result = safeCallbackData(`cb_option:${sid}:${emojiValue}`);
+      expect(Buffer.byteLength(result, 'utf-8')).toBeLessThanOrEqual(64);
+    });
+  });
+});
diff --git a/src/channels/telegram.ts b/src/channels/telegram.ts
@@ -252,6 +252,51 @@ function formatSubAgentTree(text: string): string | null {
  * Handles: **bold**, `code`, ```blocks```, [links](url), tables
  * Must be called BEFORE wrapping in blockquote/pre tags.
  */
+/** Allowlisted URI schemes for md2html() link hrefs. Prevents tg://, javascript:, data:, etc. */
+const ALLOWED_HREF_SCHEMES = ['http:', 'https:', '#:', 'mailto:'];
+
+function sanitizeHref(href: string): string {
+  const trimmed = href.trim();
+  // Allow relative/anchor links (no colon before slash/hash)
+  if (trimmed.startsWith('#') || trimmed.startsWith('/')) return esc(trimmed);
+  const scheme = trimmed.split(':')[0]?.toLowerCase() + ':';
+  if (ALLOWED_HREF_SCHEMES.includes(scheme)) return esc(trimmed);
+  // Block everything else — show the link text but drop the dangerous href
+  return '#';
+}
+
+/** Strip control chars, RTL overrides, and truncate topic names for Telegram forum topics. */
+function sanitizeTopicName(name: string): string {
+  return name
+    // Strip control characters (C0, C1, RTL overrides LRE/RLE/LRO/RLO/PDF)
+    .replace(/[\u0000-\u001F\u007F-\u009F\u200E-\u200F\u202A-\u202E]/g, '')
+    // Collapse whitespace
+    .replace(/\s+/g, ' ')
+    .trim()
+    .slice(0, 64);
+}
+
+/** Max callback_data length per Telegram Bot API (64 bytes). */
+const MAX_CALLBACK_DATA_LENGTH = 64;
+
+function safeCallbackData(data: string): string {
+  if (Buffer.byteLength(data, 'utf-8') <= MAX_CALLBACK_DATA_LENGTH) return data;
+  // Truncate value portion to fit within 64 bytes
+  const prefix = data.substring(0, data.lastIndexOf(':') + 1);
+  const value = data.substring(prefix.length);
+  // Binary search for max value length that fits
+  let lo = 0, hi = value.length;
+  while (lo < hi) {
+    const mid = Math.ceil((lo + hi) / 2);
+    if (Buffer.byteLength(prefix + value.slice(0, mid), 'utf-8') <= MAX_CALLBACK_DATA_LENGTH) {
+      lo = mid;
+    } else {
+      hi = mid - 1;
+    }
+  }
+  return prefix + value.slice(0, lo);
+}
+
 function md2html(md: string): string {
   let result = '';
   const lines = md.split('\n');
@@ -322,7 +367,7 @@ function md2html(md: string): string {
     processed = processed.replace(/(?<!\w)_([^_]+?)_(?!\w)/g, '<i>$1</i>');
 
     // Links: [text](url)
-    processed = processed.replace(/\[([^\]]+)\]\(([^)]+)\)/g, '<a href="$2">$1</a>');
+        processed = processed.replace(/\[([^\]]+)\]\(([^)]+)\)/g, (_, text, href) => '<a href="' + sanitizeHref(href) + '">' + text + '</a>');
 
     // List bullets
     processed = processed.replace(/^(\s*)[-*]\s+/, '$1• ');
@@ -816,7 +861,7 @@ export class TelegramChannel implements Channel {
   }
 
   async onSessionCreated(payload: SessionEventPayload): Promise<void> {
-    const topicName = `🤖 ${payload.session.name}`;
+        const topicName = sanitizeTopicName(`🤖 ${payload.session.name}`);
     const result = (await this.tgApi('createForumTopic', {
       chat_id: this.config.groupChatId,
       name: topicName,
@@ -1043,14 +1088,14 @@ export class TelegramChannel implements Channel {
           for (const opt of options) {
             buttons.push({
               text: opt.label,
-              callback_data: `cb_option:${payload.session.id}:${opt.value}`,
+              callback_data: safeCallbackData(`cb_option:${payload.session.id}:${opt.value}`),
             });
           }
         } else {
           // Fallback: generic approve/reject
           buttons.push(
-            { text: '✅ Approve', callback_data: `perm_approve:${payload.session.id}` },
-            { text: '❌ Reject', callback_data: `perm_reject:${payload.session.id}` },
+            { text: '✅ Approve', callback_data: safeCallbackData(`perm_approve:${payload.session.id}`) },
+            { text: '❌ Reject', callback_data: safeCallbackData(`perm_reject:${payload.session.id}`) },
           );
         }
 
@@ -1083,18 +1128,18 @@ export class TelegramChannel implements Channel {
           for (const opt of options) {
             buttons.push({
               text: opt.label,
-              callback_data: `cb_option:${payload.session.id}:${opt.value}`,
+              callback_data: safeCallbackData(`cb_option:${payload.session.id}:${opt.value}`),
             });
           }
           // Always add Skip for questions
           if (buttons.length < 4) {
-            buttons.push({ text: '🤷 Skip', callback_data: `cb_skip:${payload.session.id}` });
+            buttons.push({ text: '🤷 Skip', callback_data: safeCallbackData(`cb_skip:${payload.session.id}`) });
           }
         } else {
           buttons.push(
-            { text: '✅ Yes', callback_data: `cb_yes:${payload.session.id}` },
-            { text: '❌ No', callback_data: `cb_no:${payload.session.id}` },
-            { text: '🤷 Skip', callback_data: `cb_skip:${payload.session.id}` },
+            { text: '✅ Yes', callback_data: safeCallbackData(`cb_yes:${payload.session.id}`) },
+            { text: '❌ No', callback_data: safeCallbackData(`cb_no:${payload.session.id}`) },
+            { text: '🤷 Skip', callback_data: safeCallbackData(`cb_skip:${payload.session.id}`) },
           );
         }
 
@@ -1122,9 +1167,9 @@ export class TelegramChannel implements Channel {
           reply_markup: {
             inline_keyboard: [
               [
-                { text: '▶ Execute', callback_data: `plan_exec:${payload.session.id}` },
-                { text: '⚡ Execute All', callback_data: `plan_exec_all:${payload.session.id}` },
-                { text: '❌ Cancel', callback_data: `plan_cancel:${payload.session.id}` },
+                { text: '▶ Execute', callback_data: safeCallbackData(`plan_exec:${payload.session.id}`) },
+                { text: '⚡ Execute All', callback_data: safeCallbackData(`plan_exec_all:${payload.session.id}`) },
+                { text: '❌ Cancel', callback_data: safeCallbackData(`plan_cancel:${payload.session.id}`) },
               ],
             ],
           },
