diff --git a/src/api/controllers/map.controller.ts b/src/api/controllers/map.controller.ts index fbb89a7..b90fd08 100644 --- a/src/api/controllers/map.controller.ts +++ b/src/api/controllers/map.controller.ts @@ -186,13 +186,16 @@ export class MapController { return; } - await this.discoveryService.clearCache(url); + const keysCleared = await this.discoveryService.clearCache(url); - logger.info('Discovery cache cleared', { url }); + logger.info('Discovery cache cleared', { url, keysCleared }); res.status(200).json({ success: true, - message: 'Cache cleared successfully' + keysCleared, + message: keysCleared > 0 + ? `Cleared ${keysCleared} cached ${keysCleared === 1 ? 'entry' : 'entries'} for this URL` + : 'No cached entries found for this URL' }); } catch (error) { diff --git a/src/services/url-discovery.service.ts b/src/services/url-discovery.service.ts index 630cfc1..aa4c4e0 100644 --- a/src/services/url-discovery.service.ts +++ b/src/services/url-discovery.service.ts @@ -1069,10 +1069,13 @@ export class URLDiscoveryService { } } - /** Registrable base domain from a hostname (naive last-two-labels heuristic). */ + /** + * Registrable base domain from a hostname. Delegates to the same helper the + * result filter uses (URLValidationUtils.registrableDomain), so the subdomains + * this discovers and the subdomains the filter keeps can never disagree. + */ private baseDomainOf(hostname: string): string { - const labels = hostname.split('.').filter(Boolean); - return labels.length > 2 ? labels.slice(-2).join('.') : hostname; + return URLValidationUtils.registrableDomain(hostname); } /** @@ -1484,6 +1487,22 @@ export class URLDiscoveryService { /** * Generate cache key for discovery options */ + /** + * A stable per-URL token embedded in the cache key so `clearCache(url)` can + * find every cached variant (different maxUrls/options) for one URL. Normalized + * (scheme + lowercased host + path, no trailing slash) so trivial differences + * like a trailing slash still match. + */ + private urlKeyToken(url: string): string { + try { + const u = new URL(url); + const path = u.pathname.replace(/\/+$/, ''); + return encodeURIComponent(`${u.protocol}//${u.hostname.toLowerCase()}${path}`); + } catch { + return encodeURIComponent(url ?? ''); + } + } + private generateCacheKey(options: DiscoveryOptions): string { const keyData = { url: options.url, @@ -1498,7 +1517,8 @@ export class URLDiscoveryService { const keyString = JSON.stringify(keyData); const hash = require('crypto').createHash('sha256').update(keyString).digest('hex'); - return `${this.cachePrefix}:${hash}`; + // The URL token makes keys clearable by URL; the hash keeps per-option variants distinct. + return `${this.cachePrefix}:${this.urlKeyToken(options.url)}:${hash}`; } /** @@ -1537,25 +1557,31 @@ export class URLDiscoveryService { } /** - * Clear discovery cache for a specific URL + * Clear every cached discovery variant for a specific URL. Returns the number + * of cache entries removed. + * + * Cache keys are `url-discovery::`. The previous code + * matched keys with `.includes(encodeURIComponent(url))`, but the URL never + * appeared in the key (it was folded into the hash), so this always matched + * nothing and silently cleared zero entries. */ - async clearCache(url: string): Promise { + async clearCache(url: string): Promise { try { - const pattern = `${this.cachePrefix}:*`; - const keys = await redisClient.keys(pattern); - - // Filter keys that contain the URL - const urlKeys = keys.filter(key => key.includes(encodeURIComponent(url))); + const keys = await redisClient.keys(`${this.cachePrefix}:*`); + const prefix = `${this.cachePrefix}:${this.urlKeyToken(url)}:`; + const urlKeys = keys.filter(key => key.startsWith(prefix)); if (urlKeys.length > 0) { await redisClient.del(...urlKeys); - logger.info('Cleared discovery cache', { url, keysCleared: urlKeys.length }); } + logger.info('Cleared discovery cache', { url, keysCleared: urlKeys.length }); + return urlKeys.length; } catch (error) { logger.warn('Failed to clear discovery cache', { url, error: (error as Error).message }); + throw error; } } diff --git a/src/utils/url-validation.subdomain.spec.ts b/src/utils/url-validation.subdomain.spec.ts new file mode 100644 index 0000000..5a2897c --- /dev/null +++ b/src/utils/url-validation.subdomain.spec.ts @@ -0,0 +1,49 @@ +import { URLValidationUtils } from './url-validation.utils'; + +describe('URLValidationUtils.registrableDomain', () => { + const cases: [string, string][] = [ + ['www.sonarsource.com', 'sonarsource.com'], + ['docs.sonarsource.com', 'sonarsource.com'], + ['sonarsource.com', 'sonarsource.com'], + ['a.b.c.sonarsource.com', 'sonarsource.com'], + ['fuel-finder.uk', 'fuel-finder.uk'], + ['www.example.co.uk', 'example.co.uk'], + ['docs.example.co.uk', 'example.co.uk'], + ['shop.example.com.au', 'example.com.au'], + ['localhost', 'localhost'], + ['192.168.0.1', '192.168.0.1'], + ['WWW.SonarSource.COM', 'sonarsource.com'], + ]; + it.each(cases)('%s -> %s', (host, expected) => { + expect(URLValidationUtils.registrableDomain(host)).toBe(expected); + }); +}); + +describe('URLValidationUtils.matchesDomain (the includeSubdomains fix)', () => { + const seed = 'https://www.sonarsource.com'; + + it('keeps sibling subdomains when includeSubdomains is true (regression)', () => { + // This is the exact bug: a www. seed used to DROP docs./blog./community. + expect(URLValidationUtils.matchesDomain('https://docs.sonarsource.com/x', seed, true)).toBe(true); + expect(URLValidationUtils.matchesDomain('https://blog.sonarsource.com/y', seed, true)).toBe(true); + expect(URLValidationUtils.matchesDomain('https://www.sonarsource.com/z', seed, true)).toBe(true); + expect(URLValidationUtils.matchesDomain('https://sonarsource.com/', seed, true)).toBe(true); + }); + + it('still excludes unrelated / look-alike domains', () => { + expect(URLValidationUtils.matchesDomain('https://evilsonarsource.com', seed, true)).toBe(false); + expect(URLValidationUtils.matchesDomain('https://sonarsource.com.attacker.net', seed, true)).toBe(false); + expect(URLValidationUtils.matchesDomain('https://example.com', seed, true)).toBe(false); + }); + + it('respects a co.uk registrable boundary (no cross-site bleed)', () => { + const ukSeed = 'https://www.example.co.uk'; + expect(URLValidationUtils.matchesDomain('https://docs.example.co.uk/a', ukSeed, true)).toBe(true); + expect(URLValidationUtils.matchesDomain('https://other.co.uk/a', ukSeed, true)).toBe(false); + }); + + it('exact-host-only when includeSubdomains is false', () => { + expect(URLValidationUtils.matchesDomain('https://www.sonarsource.com/x', seed, false)).toBe(true); + expect(URLValidationUtils.matchesDomain('https://docs.sonarsource.com/x', seed, false)).toBe(false); + }); +}); diff --git a/src/utils/url-validation.utils.ts b/src/utils/url-validation.utils.ts index ade4f6c..d63f640 100644 --- a/src/utils/url-validation.utils.ts +++ b/src/utils/url-validation.utils.ts @@ -67,7 +67,51 @@ export class URLValidationUtils { } /** - * Check if URL matches domain constraints + * A curated set of common two-level public suffixes. Not a full Public Suffix + * List (that would need a dependency + periodic updates), but it covers the + * cases a blind "last two labels" gets wrong — without it, `docs.a.co.uk` + * would reduce to `co.uk` and then match every other .co.uk site. + */ + private static readonly MULTI_PART_TLDS = new Set([ + 'co.uk', 'org.uk', 'me.uk', 'ac.uk', 'gov.uk', 'net.uk', 'sch.uk', 'ltd.uk', 'plc.uk', + 'com.au', 'net.au', 'org.au', 'edu.au', 'gov.au', 'id.au', + 'co.nz', 'net.nz', 'org.nz', 'govt.nz', 'ac.nz', + 'co.za', 'org.za', 'web.za', 'gov.za', + 'co.jp', 'or.jp', 'ne.jp', 'ac.jp', 'go.jp', + 'com.br', 'net.br', 'org.br', 'gov.br', + 'com.cn', 'net.cn', 'org.cn', 'gov.cn', + 'co.in', 'net.in', 'org.in', 'gen.in', 'firm.in', + 'com.sg', 'com.hk', 'com.mx', 'com.tr', 'com.ar', 'com.tw', 'com.pl', 'com.ua', + ]); + + /** + * Best-effort registrable ("base") domain — the thing a subdomain filter must + * key on. `www.sonarsource.com` and `docs.sonarsource.com` both -> `sonarsource.com`. + * IP literals and single-label hosts are returned unchanged. + */ + static registrableDomain(hostname: string): string { + const host = (hostname || '').toLowerCase().replace(/\.$/, ''); + // IPv4 / IPv6 literal, or a single-label host (localhost): use as-is. + if (/^\d+\.\d+\.\d+\.\d+$/.test(host) || host.includes(':') || !host.includes('.')) { + return host; + } + const labels = host.split('.'); + if (labels.length <= 2) return host; + const lastTwo = labels.slice(-2).join('.'); + if (URLValidationUtils.MULTI_PART_TLDS.has(lastTwo)) { + return labels.slice(-3).join('.'); + } + return lastTwo; + } + + /** + * Check if URL matches domain constraints. + * + * With includeSubdomains, "same domain" means the same REGISTRABLE domain — so + * a seed of `www.sonarsource.com` keeps `docs.`, `blog.`, `community.` etc. + * Previously this compared against the full seed hostname, so a `www.` seed + * matched only `www.sonarsource.com` and dropped every sibling subdomain — the + * whole point of includeSubdomains — even though discovery had found them. */ static matchesDomain(url: string, baseUrl: string, includeSubdomains: boolean = true): boolean { try { @@ -75,13 +119,11 @@ export class URLValidationUtils { const baseObj = new URL(baseUrl); if (includeSubdomains) { - // Allow subdomains - return urlObj.hostname === baseObj.hostname || - urlObj.hostname.endsWith('.' + baseObj.hostname); - } else { - // Exact hostname match only - return urlObj.hostname === baseObj.hostname; + return URLValidationUtils.registrableDomain(urlObj.hostname) === + URLValidationUtils.registrableDomain(baseObj.hostname); } + // Exact hostname match only. + return urlObj.hostname === baseObj.hostname; } catch (error) { return false; }