Mirrored from GitHub github.com/roostorg/coop
0

Configure Feed

Select the types of activity you want to include in your feed.

[Fix] Potential injection and safety consistency (#773)

* [Fix] Potential injection and safety consistency

* fix code review comments

* code review feedback

author
Juan Mrad
committer
GitHub
date (Jun 16, 2026, 7:38 PM -0500) commit 5bc204ca parent 2c84a321
+143 -6
+89
client/src/utils/contentUrlUtils.test.ts
··· 1 + import { afterEach, describe, expect, it, vi } from 'vitest'; 2 + 3 + import { shouldDisplayInIframe } from './contentUrlUtils'; 4 + 5 + function setPattern(value: string) { 6 + vi.stubEnv('VITE_CONTENT_URL_PATTERN', value); 7 + } 8 + 9 + describe('shouldDisplayInIframe', () => { 10 + afterEach(() => { 11 + vi.unstubAllEnvs(); 12 + }); 13 + 14 + it('returns false for non-URL strings', () => { 15 + setPattern('example.com'); 16 + expect(shouldDisplayInIframe('not a url')).toBe(false); 17 + expect(shouldDisplayInIframe('')).toBe(false); 18 + }); 19 + 20 + it('does not match patterns smuggled via path or query', () => { 21 + setPattern('example.com'); 22 + expect(shouldDisplayInIframe('https://evil.test/?ref=example.com')).toBe( 23 + false, 24 + ); 25 + expect(shouldDisplayInIframe('https://evil.test/example.com')).toBe(false); 26 + }); 27 + 28 + describe('domain-like patterns (containing a dot)', () => { 29 + it('matches the exact host', () => { 30 + setPattern('example.com'); 31 + expect(shouldDisplayInIframe('https://example.com/profile/foo')).toBe( 32 + true, 33 + ); 34 + }); 35 + 36 + it('matches subdomains of the pattern', () => { 37 + setPattern('example.com'); 38 + expect(shouldDisplayInIframe('https://www.example.com/foo')).toBe(true); 39 + }); 40 + 41 + it('does not match lookalike hosts', () => { 42 + setPattern('example.com'); 43 + expect(shouldDisplayInIframe('https://notexample.com/foo')).toBe(false); 44 + }); 45 + 46 + it('does not match hosts that merely contain the pattern as a subdomain prefix', () => { 47 + setPattern('example.com'); 48 + expect(shouldDisplayInIframe('https://example.com.evil.test/foo')).toBe( 49 + false, 50 + ); 51 + }); 52 + }); 53 + 54 + describe('legacy patterns (no dot)', () => { 55 + it('keeps substring matching for backward compatibility', () => { 56 + setPattern('wiki'); 57 + expect(shouldDisplayInIframe('https://wiki.example.com/page')).toBe(true); 58 + expect(shouldDisplayInIframe('https://team.wiki.example.org/x')).toBe( 59 + true, 60 + ); 61 + }); 62 + }); 63 + 64 + describe('URL-like patterns (scheme / port / path)', () => { 65 + it('normalizes a pattern with a scheme to its hostname', () => { 66 + setPattern('https://example.com'); 67 + expect(shouldDisplayInIframe('https://example.com/foo')).toBe(true); 68 + expect(shouldDisplayInIframe('https://notexample.com/foo')).toBe(false); 69 + }); 70 + 71 + it('normalizes a pattern with a path to its hostname', () => { 72 + setPattern('example.org/page'); 73 + expect(shouldDisplayInIframe('https://example.org/anything')).toBe(true); 74 + expect(shouldDisplayInIframe('https://www.example.org/x')).toBe(true); 75 + }); 76 + 77 + it('normalizes a pattern with a port to its hostname', () => { 78 + setPattern('localhost:3000'); 79 + expect(shouldDisplayInIframe('http://localhost/foo')).toBe(true); 80 + }); 81 + }); 82 + 83 + it('supports comma-separated patterns', () => { 84 + setPattern('wiki, example.com'); 85 + expect(shouldDisplayInIframe('https://wiki.example.org/page')).toBe(true); 86 + expect(shouldDisplayInIframe('https://example.com/foo')).toBe(true); 87 + expect(shouldDisplayInIframe('https://notexample.com/foo')).toBe(false); 88 + }); 89 + });
+51 -2
client/src/utils/contentUrlUtils.ts
··· 63 63 } 64 64 65 65 /** 66 + * Normalize a configured pattern to a bare host fragment, stripping any scheme, 67 + * port, or path so legacy full-URL configs keep working. Dotless fragments are 68 + * returned as-is for substring matching. 69 + */ 70 + function normalizePatternHost(pattern: string): string { 71 + const trimmed = pattern.trim().toLowerCase(); 72 + if (trimmed.length === 0) return ''; 73 + let host = trimmed; 74 + if (/[:/]/.test(trimmed)) { 75 + try { 76 + host = new URL(trimmed.includes('://') ? trimmed : `http://${trimmed}`) 77 + .hostname; 78 + } catch { 79 + // Not parseable as a URL; fall back to the raw fragment. 80 + } 81 + } 82 + return host.replace(/^\.+|\.+$/g, ''); 83 + } 84 + 85 + /** 86 + * Check whether a single pattern matches a hostname. 87 + * 88 + * Domain-like patterns (those containing a dot, e.g. `example.com`) are matched 89 + * on host boundaries so a pattern can't be smuggled via a lookalike or subdomain 90 + * (`notexample.com` and `example.com.evil.test` do NOT match `example.com`). 91 + * Legacy patterns without a dot keep substring matching for backward 92 + * compatibility with existing `VITE_CONTENT_URL_PATTERN` configs. 93 + */ 94 + function hostnameMatchesPattern(hostname: string, pattern: string): boolean { 95 + const normalized = normalizePatternHost(pattern); 96 + if (normalized.length === 0) return false; 97 + if (normalized.includes('.')) { 98 + return hostname === normalized || hostname.endsWith(`.${normalized}`); 99 + } 100 + return hostname.includes(normalized); 101 + } 102 + 103 + /** 66 104 * Check if a URL should be displayed in an iframe 105 + * 106 + * Patterns are matched against the URL's hostname only (not the full URL), so a 107 + * pattern can't be smuggled in via the path or query string (e.g. 108 + * `https://evil.test/?ref=example.com` does not match the `example.com` pattern). 109 + * Domain-like patterns are further matched on host boundaries; see 110 + * {@link hostnameMatchesPattern}. 67 111 * @param url The URL to check 68 112 * @returns True if the URL should be displayed in an iframe 69 113 */ 70 114 export function shouldDisplayInIframe(url: string): boolean { 115 + let hostname: string; 116 + try { 117 + hostname = new URL(url).hostname.toLowerCase(); 118 + } catch { 119 + return false; 120 + } 71 121 const patterns = getContentUrlPatterns(); 72 - const urlLower = url.toLowerCase(); 73 - return patterns.some((pattern) => urlLower.includes(pattern.toLowerCase())); 122 + return patterns.some((pattern) => hostnameMatchesPattern(hostname, pattern)); 74 123 } 75 124 76 125 /**
+3 -4
client/src/webpages/dashboard/mrt/manual_review_job/v2/ManualReviewJobContentView.tsx
··· 147 147 </div> 148 148 </div> 149 149 {'url' in payload.item.data && 150 - shouldDisplayInIframe(String(payload.item.data.url)) ? ( 151 - <IframeContentDisplayComponent 152 - contentUrl={String(payload.item.data.url)} 153 - /> 150 + typeof payload.item.data.url === 'string' && 151 + shouldDisplayInIframe(payload.item.data.url) ? ( 152 + <IframeContentDisplayComponent contentUrl={payload.item.data.url} /> 154 153 ) : null} 155 154 {!contentThread ? null : ( 156 155 <div className="my-6">