From 938e094a1a7bf0f156883e46d538f3fb176dcc67 Mon Sep 17 00:00:00 2001 From: Hazelnoot Date: Mon, 5 May 2025 08:44:42 -0400 Subject: [PATCH 01/18] set summary.haveNoteLocally before caching summary --- packages/backend/src/server/web/UrlPreviewService.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/backend/src/server/web/UrlPreviewService.ts b/packages/backend/src/server/web/UrlPreviewService.ts index aa8fcd0c2a..6c4e95cabc 100644 --- a/packages/backend/src/server/web/UrlPreviewService.ts +++ b/packages/backend/src/server/web/UrlPreviewService.ts @@ -137,12 +137,12 @@ export class UrlPreviewService { summary.icon = this.wrap(summary.icon); summary.thumbnail = this.wrap(summary.thumbnail); - this.previewCache.set(key, summary); - if (summary.activityPub) { - summary.haveNoteLocally = !! await this.apDbResolverService.getNoteFromApId(summary.activityPub); + summary.haveNoteLocally = !!await this.apDbResolverService.getNoteFromApId(summary.activityPub); } + this.previewCache.set(key, summary); + // Cache 7days reply.header('Cache-Control', 'max-age=604800, immutable'); From 129dfa964946a079aba08a5fa21379cfcbec0d70 Mon Sep 17 00:00:00 2001 From: Hazelnoot Date: Mon, 5 May 2025 09:06:27 -0400 Subject: [PATCH 02/18] extract LocalSummalyResult type --- .../src/server/web/UrlPreviewService.ts | 28 +++++++++++-------- 1 file changed, 16 insertions(+), 12 deletions(-) diff --git a/packages/backend/src/server/web/UrlPreviewService.ts b/packages/backend/src/server/web/UrlPreviewService.ts index 6c4e95cabc..a9be094ce7 100644 --- a/packages/backend/src/server/web/UrlPreviewService.ts +++ b/packages/backend/src/server/web/UrlPreviewService.ts @@ -18,31 +18,35 @@ import { ApiError } from '@/server/api/error.js'; import { MiMeta } from '@/models/Meta.js'; import { RedisKVCache } from '@/misc/cache.js'; import { UtilityService } from '@/core/UtilityService.js'; -import type { FastifyRequest, FastifyReply } from 'fastify'; import { ApDbResolverService } from '@/core/activitypub/ApDbResolverService.js'; +import type { FastifyRequest, FastifyReply } from 'fastify'; + +export type LocalSummalyResult = SummalyResult & { + haveNoteLocally?: boolean; +}; @Injectable() export class UrlPreviewService { private logger: Logger; - private previewCache: RedisKVCache; + private previewCache: RedisKVCache; constructor( @Inject(DI.config) private config: Config, @Inject(DI.redis) - private redisClient: Redis.Redis, + private readonly redisClient: Redis.Redis, @Inject(DI.meta) - private meta: MiMeta, + private readonly meta: MiMeta, private httpRequestService: HttpRequestService, private loggerService: LoggerService, - private utilityService: UtilityService, - private apDbResolverService: ApDbResolverService, + private readonly utilityService: UtilityService, + private readonly apDbResolverService: ApDbResolverService, ) { this.logger = this.loggerService.getLogger('url-preview'); - this.previewCache = new RedisKVCache(this.redisClient, 'summaly', { + this.previewCache = new RedisKVCache(this.redisClient, 'summaly', { lifetime: 1000 * 60 * 60 * 24, // 1d memoryCacheLifetime: 1000 * 60 * 10, // 10m fetcher: () => { throw new Error('the UrlPreview cache should never fetch'); }, @@ -102,7 +106,7 @@ export class UrlPreviewService { } const key = `${url}@${lang}`; - const cached = await this.previewCache.get(key) as SummalyResult & { haveNoteLocally?: boolean }; + const cached = await this.previewCache.get(key); if (cached !== undefined) { this.logger.info(`Returning cache preview of ${key}`); // Cache 7days @@ -120,7 +124,7 @@ export class UrlPreviewService { : `Getting preview of ${key} ...`); try { - const summary: SummalyResult & { haveNoteLocally?: boolean } = this.meta.urlPreviewSummaryProxyUrl + const summary: LocalSummalyResult = this.meta.urlPreviewSummaryProxyUrl ? await this.fetchSummaryFromProxy(url, this.meta, lang) : await this.fetchSummary(url, this.meta, lang); @@ -162,7 +166,7 @@ export class UrlPreviewService { } } - private fetchSummary(url: string, meta: MiMeta, lang?: string): Promise { + private fetchSummary(url: string, meta: MiMeta, lang?: string): Promise { const agent = this.config.proxy ? { http: this.httpRequestService.httpAgent, @@ -181,7 +185,7 @@ export class UrlPreviewService { }); } - private fetchSummaryFromProxy(url: string, meta: MiMeta, lang?: string): Promise { + private fetchSummaryFromProxy(url: string, meta: MiMeta, lang?: string): Promise { const proxy = meta.urlPreviewSummaryProxyUrl!; const queryStr = query({ url: url, @@ -192,6 +196,6 @@ export class UrlPreviewService { contentLengthRequired: meta.urlPreviewRequireContentLength, }); - return this.httpRequestService.getJson(`${proxy}?${queryStr}`, 'application/json, */*', undefined, true); + return this.httpRequestService.getJson(`${proxy}?${queryStr}`, 'application/json, */*', undefined, true); } } From 2fb56bc4ead2196b65b900e915be5bc658e4934e Mon Sep 17 00:00:00 2001 From: Hazelnoot Date: Mon, 5 May 2025 09:06:45 -0400 Subject: [PATCH 03/18] fix eslint warning in UrlPreviewService.ts --- packages/backend/src/server/web/UrlPreviewService.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/backend/src/server/web/UrlPreviewService.ts b/packages/backend/src/server/web/UrlPreviewService.ts index a9be094ce7..8c1776568b 100644 --- a/packages/backend/src/server/web/UrlPreviewService.ts +++ b/packages/backend/src/server/web/UrlPreviewService.ts @@ -67,7 +67,7 @@ export class UrlPreviewService { @bindThis public async handle( - request: FastifyRequest<{ Querystring: { url: string; lang?: string; } }>, + request: FastifyRequest<{ Querystring: { url?: string; lang?: string; } }>, reply: FastifyReply, ): Promise { const url = request.query.url; From ab65f4b8b2d0cc15c8279467b6150b4f0e51f704 Mon Sep 17 00:00:00 2001 From: Hazelnoot Date: Mon, 5 May 2025 09:07:26 -0400 Subject: [PATCH 04/18] infer ActivityPub links from local DB --- .../src/server/web/UrlPreviewService.ts | 39 +++++++++++++++++++ 1 file changed, 39 insertions(+) diff --git a/packages/backend/src/server/web/UrlPreviewService.ts b/packages/backend/src/server/web/UrlPreviewService.ts index 8c1776568b..a38454fafc 100644 --- a/packages/backend/src/server/web/UrlPreviewService.ts +++ b/packages/backend/src/server/web/UrlPreviewService.ts @@ -20,6 +20,9 @@ import { RedisKVCache } from '@/misc/cache.js'; import { UtilityService } from '@/core/UtilityService.js'; import { ApDbResolverService } from '@/core/activitypub/ApDbResolverService.js'; import type { FastifyRequest, FastifyReply } from 'fastify'; +import type { NotesRepository } from '@/models/_.js'; +import { ApUtilityService } from '@/core/activitypub/ApUtilityService.js'; +import { IsNull, Not } from 'typeorm'; export type LocalSummalyResult = SummalyResult & { haveNoteLocally?: boolean; @@ -40,9 +43,13 @@ export class UrlPreviewService { @Inject(DI.meta) private readonly meta: MiMeta, + @Inject(DI.notesRepository) + private readonly notesRepository: NotesRepository, + private httpRequestService: HttpRequestService, private loggerService: LoggerService, private readonly utilityService: UtilityService, + private readonly apUtilityService: ApUtilityService, private readonly apDbResolverService: ApDbResolverService, ) { this.logger = this.loggerService.getLogger('url-preview'); @@ -143,6 +150,9 @@ export class UrlPreviewService { if (summary.activityPub) { summary.haveNoteLocally = !!await this.apDbResolverService.getNoteFromApId(summary.activityPub); + } else { + // Summaly cannot always detect links to a fedi post, so check if it matches anything we already have + await this.inferActivityPubLink(summary); } this.previewCache.set(key, summary); @@ -198,4 +208,33 @@ export class UrlPreviewService { return this.httpRequestService.getJson(`${proxy}?${queryStr}`, 'application/json, */*', undefined, true); } + + private async inferActivityPubLink(summary: LocalSummalyResult) { + // Match canonical URI first. + // This covers local and remote links. + const isCanonicalUri = !!await this.apDbResolverService.getNoteFromApId(summary.url); + if (isCanonicalUri) { + summary.activityPub = summary.url; + summary.haveNoteLocally = true; + } + + // Try public URL next. + // This is necessary for Mastodon and other software with a different public URL. + const urlMatches = await this.notesRepository.find({ + select: { + uri: true, + }, + where: { + url: summary.url, + uri: Not(IsNull()), + }, + }) as { uri: string }[]; + + // Older versions did not validate URL, so do it now to avoid impersonation. + const matchByUrl = urlMatches.find(({ uri }) => this.apUtilityService.haveSameAuthority(uri, summary.url)); + if (matchByUrl) { + summary.activityPub = matchByUrl.uri; + summary.haveNoteLocally = true; + } + } } From 1d2a4c6f5631ffa1155a284b58e41b72ed8b7cc2 Mon Sep 17 00:00:00 2001 From: Hazelnoot Date: Mon, 5 May 2025 09:14:09 -0400 Subject: [PATCH 05/18] infer ActivityPub links from signed GET --- .../src/server/web/UrlPreviewService.ts | 19 +++++++++++++++++-- 1 file changed, 17 insertions(+), 2 deletions(-) diff --git a/packages/backend/src/server/web/UrlPreviewService.ts b/packages/backend/src/server/web/UrlPreviewService.ts index a38454fafc..8f2ec5be00 100644 --- a/packages/backend/src/server/web/UrlPreviewService.ts +++ b/packages/backend/src/server/web/UrlPreviewService.ts @@ -7,6 +7,7 @@ import { Inject, Injectable } from '@nestjs/common'; import { summaly } from '@misskey-dev/summaly'; import { SummalyResult } from '@misskey-dev/summaly/built/summary.js'; import * as Redis from 'ioredis'; +import { IsNull, Not } from 'typeorm'; import { DI } from '@/di-symbols.js'; import type { Config } from '@/config.js'; import { HttpRequestService } from '@/core/HttpRequestService.js'; @@ -19,10 +20,11 @@ import { MiMeta } from '@/models/Meta.js'; import { RedisKVCache } from '@/misc/cache.js'; import { UtilityService } from '@/core/UtilityService.js'; import { ApDbResolverService } from '@/core/activitypub/ApDbResolverService.js'; -import type { FastifyRequest, FastifyReply } from 'fastify'; import type { NotesRepository } from '@/models/_.js'; import { ApUtilityService } from '@/core/activitypub/ApUtilityService.js'; -import { IsNull, Not } from 'typeorm'; +import { ApRequestService } from '@/core/activitypub/ApRequestService.js'; +import { SystemAccountService } from '@/core/SystemAccountService.js'; +import type { FastifyRequest, FastifyReply } from 'fastify'; export type LocalSummalyResult = SummalyResult & { haveNoteLocally?: boolean; @@ -51,6 +53,8 @@ export class UrlPreviewService { private readonly utilityService: UtilityService, private readonly apUtilityService: ApUtilityService, private readonly apDbResolverService: ApDbResolverService, + private readonly apRequestService: ApRequestService, + private readonly systemAccountService: SystemAccountService, ) { this.logger = this.loggerService.getLogger('url-preview'); this.previewCache = new RedisKVCache(this.redisClient, 'summaly', { @@ -216,6 +220,7 @@ export class UrlPreviewService { if (isCanonicalUri) { summary.activityPub = summary.url; summary.haveNoteLocally = true; + return; } // Try public URL next. @@ -235,6 +240,16 @@ export class UrlPreviewService { if (matchByUrl) { summary.activityPub = matchByUrl.uri; summary.haveNoteLocally = true; + return; + } + + // Finally, attempt a signed GET in case it's a direct link to an instance with authorized fetch. + const instanceActor = await this.systemAccountService.getInstanceActor(); + const remoteObject = await this.apRequestService.signedGet(summary.url, instanceActor).catch(() => null); + if (remoteObject) { + summary.activityPub = remoteObject.id; + summary.haveNoteLocally = false; + return; } } } From 05201f71ccffe7aa1b8faab447c66c598fd2b4e5 Mon Sep 17 00:00:00 2001 From: Hazelnoot Date: Mon, 5 May 2025 09:15:24 -0400 Subject: [PATCH 06/18] allow summaly previews to redirect --- .../src/server/web/UrlPreviewService.ts | 18 +++++++++++++++--- 1 file changed, 15 insertions(+), 3 deletions(-) diff --git a/packages/backend/src/server/web/UrlPreviewService.ts b/packages/backend/src/server/web/UrlPreviewService.ts index 8f2ec5be00..876c9a9674 100644 --- a/packages/backend/src/server/web/UrlPreviewService.ts +++ b/packages/backend/src/server/web/UrlPreviewService.ts @@ -104,8 +104,7 @@ export class UrlPreviewService { }; } - const host = new URL(url).host; - if (this.utilityService.isBlockedHost(this.meta.blockedHosts, host)) { + if (this.utilityService.isBlockedHost(this.meta.blockedHosts, new URL(url).host)) { reply.code(403); return { error: new ApiError({ @@ -139,6 +138,18 @@ export class UrlPreviewService { ? await this.fetchSummaryFromProxy(url, this.meta, lang) : await this.fetchSummary(url, this.meta, lang); + // Repeat check, since redirects are allowed. + if (this.utilityService.isBlockedHost(this.meta.blockedHosts, new URL(summary.url).host)) { + reply.code(403); + return { + error: new ApiError({ + message: 'URL is blocked', + code: 'URL_PREVIEW_BLOCKED', + id: '50294652-857b-4b13-9700-8e5c7a8deae8', + }), + }; + } + this.logger.succ(`Got preview of ${url}: ${summary.title}`); if (!(summary.url.startsWith('http://') || summary.url.startsWith('https://'))) { @@ -189,7 +200,7 @@ export class UrlPreviewService { : undefined; return summaly(url, { - followRedirects: false, + followRedirects: true, lang: lang ?? 'ja-JP', agent: agent, userAgent: meta.urlPreviewUserAgent ?? undefined, @@ -202,6 +213,7 @@ export class UrlPreviewService { private fetchSummaryFromProxy(url: string, meta: MiMeta, lang?: string): Promise { const proxy = meta.urlPreviewSummaryProxyUrl!; const queryStr = query({ + followRedirects: true, url: url, lang: lang ?? 'ja-JP', userAgent: meta.urlPreviewUserAgent ?? undefined, From 80819f03e7a9404cf603648abf4581d0352e5997 Mon Sep 17 00:00:00 2001 From: Hazelnoot Date: Mon, 5 May 2025 09:21:53 -0400 Subject: [PATCH 07/18] don't proxy local URLs --- .../backend/src/server/web/UrlPreviewService.ts | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/packages/backend/src/server/web/UrlPreviewService.ts b/packages/backend/src/server/web/UrlPreviewService.ts index 876c9a9674..0312dff16d 100644 --- a/packages/backend/src/server/web/UrlPreviewService.ts +++ b/packages/backend/src/server/web/UrlPreviewService.ts @@ -68,12 +68,16 @@ export class UrlPreviewService { @bindThis private wrap(url?: string | null): string | null { - return url != null - ? `${this.config.mediaProxy}/preview.webp?${query({ - url, - preview: '1', - })}` - : null; + if (url == null) return null; + + // Don't proxy our own media + if (this.utilityService.isUriLocal(url)) { + return url; + } + + // But proxy everything else! + const mediaQuery = query({ url, preview: '1' }); + return `${this.config.mediaProxy}/preview.webp?${mediaQuery}`; } @bindThis From 387efac23ffcb5743279ba96798fc5db26ce6807 Mon Sep 17 00:00:00 2001 From: Hazelnoot Date: Mon, 5 May 2025 09:24:28 -0400 Subject: [PATCH 08/18] add version specifier to URL preview cache --- .../backend/src/server/web/UrlPreviewService.ts | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/packages/backend/src/server/web/UrlPreviewService.ts b/packages/backend/src/server/web/UrlPreviewService.ts index 0312dff16d..fc09554ce8 100644 --- a/packages/backend/src/server/web/UrlPreviewService.ts +++ b/packages/backend/src/server/web/UrlPreviewService.ts @@ -30,6 +30,9 @@ export type LocalSummalyResult = SummalyResult & { haveNoteLocally?: boolean; }; +// Increment this to invalidate cached previews after a major change. +const cacheFormatVersion = 1; + @Injectable() export class UrlPreviewService { private logger: Logger; @@ -119,10 +122,10 @@ export class UrlPreviewService { }; } - const key = `${url}@${lang}`; - const cached = await this.previewCache.get(key); + const cacheKey = `${url}@${lang}@${cacheFormatVersion}`; + const cached = await this.previewCache.get(cacheKey); if (cached !== undefined) { - this.logger.info(`Returning cache preview of ${key}`); + this.logger.info(`Returning cache preview of ${cacheKey}`); // Cache 7days reply.header('Cache-Control', 'max-age=604800, immutable'); @@ -134,8 +137,8 @@ export class UrlPreviewService { } this.logger.info(this.meta.urlPreviewSummaryProxyUrl - ? `(Proxy) Getting preview of ${key} ...` - : `Getting preview of ${key} ...`); + ? `(Proxy) Getting preview of ${cacheKey} ...` + : `Getting preview of ${cacheKey} ...`); try { const summary: LocalSummalyResult = this.meta.urlPreviewSummaryProxyUrl @@ -174,7 +177,7 @@ export class UrlPreviewService { await this.inferActivityPubLink(summary); } - this.previewCache.set(key, summary); + this.previewCache.set(cacheKey, summary); // Cache 7days reply.header('Cache-Control', 'max-age=604800, immutable'); From 163be8d4a4a978d3fbaad37909f8c8f9be61e08c Mon Sep 17 00:00:00 2001 From: Hazelnoot Date: Mon, 5 May 2025 09:25:38 -0400 Subject: [PATCH 09/18] match preview cache duration for HTTP and Redis --- packages/backend/src/server/web/UrlPreviewService.ts | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/packages/backend/src/server/web/UrlPreviewService.ts b/packages/backend/src/server/web/UrlPreviewService.ts index fc09554ce8..7eeb6535b1 100644 --- a/packages/backend/src/server/web/UrlPreviewService.ts +++ b/packages/backend/src/server/web/UrlPreviewService.ts @@ -126,8 +126,8 @@ export class UrlPreviewService { const cached = await this.previewCache.get(cacheKey); if (cached !== undefined) { this.logger.info(`Returning cache preview of ${cacheKey}`); - // Cache 7days - reply.header('Cache-Control', 'max-age=604800, immutable'); + // Cache 1 day (matching redis) + reply.header('Cache-Control', 'public, max-age=86400'); if (cached.activityPub) { cached.haveNoteLocally = !! await this.apDbResolverService.getNoteFromApId(cached.activityPub); @@ -179,8 +179,8 @@ export class UrlPreviewService { this.previewCache.set(cacheKey, summary); - // Cache 7days - reply.header('Cache-Control', 'max-age=604800, immutable'); + // Cache 1 day (matching redis) + reply.header('Cache-Control', 'public, max-age=86400'); return summary; } catch (err) { From c23b1c3be77be82ef73d3dac47c58e771fea7eff Mon Sep 17 00:00:00 2001 From: Hazelnoot Date: Mon, 5 May 2025 09:33:32 -0400 Subject: [PATCH 10/18] reduce log spam from UrlPreviewService.ts --- packages/backend/src/server/web/UrlPreviewService.ts | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/packages/backend/src/server/web/UrlPreviewService.ts b/packages/backend/src/server/web/UrlPreviewService.ts index 7eeb6535b1..13e103c780 100644 --- a/packages/backend/src/server/web/UrlPreviewService.ts +++ b/packages/backend/src/server/web/UrlPreviewService.ts @@ -125,7 +125,6 @@ export class UrlPreviewService { const cacheKey = `${url}@${lang}@${cacheFormatVersion}`; const cached = await this.previewCache.get(cacheKey); if (cached !== undefined) { - this.logger.info(`Returning cache preview of ${cacheKey}`); // Cache 1 day (matching redis) reply.header('Cache-Control', 'public, max-age=86400'); @@ -136,10 +135,6 @@ export class UrlPreviewService { return cached; } - this.logger.info(this.meta.urlPreviewSummaryProxyUrl - ? `(Proxy) Getting preview of ${cacheKey} ...` - : `Getting preview of ${cacheKey} ...`); - try { const summary: LocalSummalyResult = this.meta.urlPreviewSummaryProxyUrl ? await this.fetchSummaryFromProxy(url, this.meta, lang) @@ -157,7 +152,7 @@ export class UrlPreviewService { }; } - this.logger.succ(`Got preview of ${url}: ${summary.title}`); + this.logger.info(`Got preview of ${url} in ${lang}: ${summary.title}`); if (!(summary.url.startsWith('http://') || summary.url.startsWith('https://'))) { throw new Error('unsupported schema included'); @@ -184,7 +179,7 @@ export class UrlPreviewService { return summary; } catch (err) { - this.logger.warn(`Failed to get preview of ${url}: ${err}`); + this.logger.warn(`Failed to get preview of ${url} for ${lang}: ${err}`); reply.code(422); reply.header('Cache-Control', 'max-age=86400, immutable'); From a1fcf554fa6bf823f0efca1c70a1e4adfd4aaae3 Mon Sep 17 00:00:00 2001 From: Hazelnoot Date: Mon, 5 May 2025 09:35:13 -0400 Subject: [PATCH 11/18] reduce caching for failed previews --- packages/backend/src/server/web/UrlPreviewService.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/backend/src/server/web/UrlPreviewService.ts b/packages/backend/src/server/web/UrlPreviewService.ts index 13e103c780..e192bae2a8 100644 --- a/packages/backend/src/server/web/UrlPreviewService.ts +++ b/packages/backend/src/server/web/UrlPreviewService.ts @@ -182,7 +182,7 @@ export class UrlPreviewService { this.logger.warn(`Failed to get preview of ${url} for ${lang}: ${err}`); reply.code(422); - reply.header('Cache-Control', 'max-age=86400, immutable'); + reply.header('Cache-Control', 'max-age=3600'); return { error: new ApiError({ message: 'Failed to get preview', From 23267a3a9648e5e713ac021bf4ebd6d8ed067934 Mon Sep 17 00:00:00 2001 From: Hazelnoot Date: Mon, 5 May 2025 09:36:17 -0400 Subject: [PATCH 12/18] await cache update to avoid hammering redis in UrlPreviewService.ts --- packages/backend/src/server/web/UrlPreviewService.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/packages/backend/src/server/web/UrlPreviewService.ts b/packages/backend/src/server/web/UrlPreviewService.ts index e192bae2a8..d6151b665a 100644 --- a/packages/backend/src/server/web/UrlPreviewService.ts +++ b/packages/backend/src/server/web/UrlPreviewService.ts @@ -172,7 +172,8 @@ export class UrlPreviewService { await this.inferActivityPubLink(summary); } - this.previewCache.set(cacheKey, summary); + // Await this to avoid hammering redis when a bunch of URLs are fetched at once + await this.previewCache.set(cacheKey, summary); // Cache 1 day (matching redis) reply.header('Cache-Control', 'public, max-age=86400'); From d6c2140821a4595862e063949d2f92530bd16cfd Mon Sep 17 00:00:00 2001 From: Hazelnoot Date: Mon, 5 May 2025 09:43:40 -0400 Subject: [PATCH 13/18] validate more URLs in UrlPreviewService.ts --- packages/backend/src/core/UtilityService.ts | 10 +++++ .../src/server/web/UrlPreviewService.ts | 45 +++++++++++++++---- 2 files changed, 47 insertions(+), 8 deletions(-) diff --git a/packages/backend/src/core/UtilityService.ts b/packages/backend/src/core/UtilityService.ts index f8d04c0592..170afc72dc 100644 --- a/packages/backend/src/core/UtilityService.ts +++ b/packages/backend/src/core/UtilityService.ts @@ -176,4 +176,14 @@ export class UtilityService { const host = this.extractDbHost(uri); return this.isFederationAllowedHost(host); } + + @bindThis + public getUrlScheme(url: string): string { + try { + // Returns in the format "https:" or an empty string + return new URL(url).protocol; + } catch { + return ''; + } + } } diff --git a/packages/backend/src/server/web/UrlPreviewService.ts b/packages/backend/src/server/web/UrlPreviewService.ts index d6151b665a..fe03583270 100644 --- a/packages/backend/src/server/web/UrlPreviewService.ts +++ b/packages/backend/src/server/web/UrlPreviewService.ts @@ -140,6 +140,8 @@ export class UrlPreviewService { ? await this.fetchSummaryFromProxy(url, this.meta, lang) : await this.fetchSummary(url, this.meta, lang); + this.validateUrls(summary); + // Repeat check, since redirects are allowed. if (this.utilityService.isBlockedHost(this.meta.blockedHosts, new URL(summary.url).host)) { reply.code(403); @@ -154,14 +156,6 @@ export class UrlPreviewService { this.logger.info(`Got preview of ${url} in ${lang}: ${summary.title}`); - if (!(summary.url.startsWith('http://') || summary.url.startsWith('https://'))) { - throw new Error('unsupported schema included'); - } - - if (summary.player.url && !(summary.player.url.startsWith('http://') || summary.player.url.startsWith('https://'))) { - throw new Error('unsupported schema included'); - } - summary.icon = this.wrap(summary.icon); summary.thumbnail = this.wrap(summary.thumbnail); @@ -228,6 +222,41 @@ export class UrlPreviewService { return this.httpRequestService.getJson(`${proxy}?${queryStr}`, 'application/json, */*', undefined, true); } + private validateUrls(summary: LocalSummalyResult) { + const urlScheme = this.utilityService.getUrlScheme(summary.url); + if (urlScheme !== 'http:' && urlScheme !== 'https:') { + throw new Error(`unsupported scheme in preview URL: "${urlScheme}"`); + } + + if (summary.player.url) { + const playerScheme = this.utilityService.getUrlScheme(summary.player.url); + if (playerScheme !== 'http:' && playerScheme !== 'https:') { + throw new Error(`unsupported scheme in player URL: "${playerScheme}"`); + } + } + + if (summary.icon) { + const iconScheme = this.utilityService.getUrlScheme(summary.icon); + if (iconScheme !== 'http:' && iconScheme !== 'https:') { + throw new Error(`unsupported scheme in icon URL: "${iconScheme}"`); + } + } + + if (summary.thumbnail) { + const thumbnailScheme = this.utilityService.getUrlScheme(summary.thumbnail); + if (thumbnailScheme !== 'http:' && thumbnailScheme !== 'https:') { + throw new Error(`unsupported scheme in thumbnail URL: "${thumbnailScheme}"`); + } + } + + if (summary.activityPub) { + const activityPubScheme = this.utilityService.getUrlScheme(summary.activityPub); + if (activityPubScheme !== 'http:' && activityPubScheme !== 'https:') { + throw new Error(`unsupported scheme in ActivityPub URL: "${activityPubScheme}"`); + } + } + } + private async inferActivityPubLink(summary: LocalSummalyResult) { // Match canonical URI first. // This covers local and remote links. From c05aa7a28181b64669fb9e23c5e40b162292bef2 Mon Sep 17 00:00:00 2001 From: Hazelnoot Date: Mon, 5 May 2025 09:56:28 -0400 Subject: [PATCH 14/18] softer URL preview validation: remove unsupported URLs instead of rejecting the whole preview --- packages/backend/src/server/web/UrlPreviewService.ts | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/packages/backend/src/server/web/UrlPreviewService.ts b/packages/backend/src/server/web/UrlPreviewService.ts index fe03583270..0bd3eff3ba 100644 --- a/packages/backend/src/server/web/UrlPreviewService.ts +++ b/packages/backend/src/server/web/UrlPreviewService.ts @@ -231,28 +231,32 @@ export class UrlPreviewService { if (summary.player.url) { const playerScheme = this.utilityService.getUrlScheme(summary.player.url); if (playerScheme !== 'http:' && playerScheme !== 'https:') { - throw new Error(`unsupported scheme in player URL: "${playerScheme}"`); + this.logger.warn(`Redacting preview for ${summary.url}: player URL has unsupported scheme "${playerScheme}"`); + summary.player.url = null; } } if (summary.icon) { const iconScheme = this.utilityService.getUrlScheme(summary.icon); if (iconScheme !== 'http:' && iconScheme !== 'https:') { - throw new Error(`unsupported scheme in icon URL: "${iconScheme}"`); + this.logger.warn(`Redacting preview for ${summary.url}: icon URL has unsupported scheme "${iconScheme}"`); + summary.icon = null; } } if (summary.thumbnail) { const thumbnailScheme = this.utilityService.getUrlScheme(summary.thumbnail); if (thumbnailScheme !== 'http:' && thumbnailScheme !== 'https:') { - throw new Error(`unsupported scheme in thumbnail URL: "${thumbnailScheme}"`); + this.logger.warn(`Redacting preview for ${summary.url}: thumbnail URL has unsupported scheme "${thumbnailScheme}"`); + summary.thumbnail = null; } } if (summary.activityPub) { const activityPubScheme = this.utilityService.getUrlScheme(summary.activityPub); if (activityPubScheme !== 'http:' && activityPubScheme !== 'https:') { - throw new Error(`unsupported scheme in ActivityPub URL: "${activityPubScheme}"`); + this.logger.warn(`Redacting preview for ${summary.url}: ActivityPub URL has unsupported scheme "${activityPubScheme}"`); + summary.activityPub = null; } } } From 70d75f1d57ccaa751778b13cf3b7c8e2e360c8aa Mon Sep 17 00:00:00 2001 From: Hazelnoot Date: Mon, 5 May 2025 10:12:21 -0400 Subject: [PATCH 15/18] check summary.haveNoteLocally after setting summary.activityPub to improve support for Akkoma --- .../src/server/web/UrlPreviewService.ts | 25 ++++++++----------- 1 file changed, 11 insertions(+), 14 deletions(-) diff --git a/packages/backend/src/server/web/UrlPreviewService.ts b/packages/backend/src/server/web/UrlPreviewService.ts index 0bd3eff3ba..2ae626f8b7 100644 --- a/packages/backend/src/server/web/UrlPreviewService.ts +++ b/packages/backend/src/server/web/UrlPreviewService.ts @@ -31,7 +31,7 @@ export type LocalSummalyResult = SummalyResult & { }; // Increment this to invalidate cached previews after a major change. -const cacheFormatVersion = 1; +const cacheFormatVersion = 2; @Injectable() export class UrlPreviewService { @@ -159,11 +159,11 @@ export class UrlPreviewService { summary.icon = this.wrap(summary.icon); summary.thumbnail = this.wrap(summary.thumbnail); + // Summaly cannot always detect links to a fedi post, so do some additional tests to try and find missed cases. + summary.activityPub ??= await this.inferActivityPubLink(summary); + if (summary.activityPub) { summary.haveNoteLocally = !!await this.apDbResolverService.getNoteFromApId(summary.activityPub); - } else { - // Summaly cannot always detect links to a fedi post, so check if it matches anything we already have - await this.inferActivityPubLink(summary); } // Await this to avoid hammering redis when a bunch of URLs are fetched at once @@ -261,14 +261,12 @@ export class UrlPreviewService { } } - private async inferActivityPubLink(summary: LocalSummalyResult) { + private async inferActivityPubLink(summary: LocalSummalyResult): Promise { // Match canonical URI first. // This covers local and remote links. const isCanonicalUri = !!await this.apDbResolverService.getNoteFromApId(summary.url); if (isCanonicalUri) { - summary.activityPub = summary.url; - summary.haveNoteLocally = true; - return; + return summary.url; } // Try public URL next. @@ -286,18 +284,17 @@ export class UrlPreviewService { // Older versions did not validate URL, so do it now to avoid impersonation. const matchByUrl = urlMatches.find(({ uri }) => this.apUtilityService.haveSameAuthority(uri, summary.url)); if (matchByUrl) { - summary.activityPub = matchByUrl.uri; - summary.haveNoteLocally = true; - return; + return matchByUrl.uri; } // Finally, attempt a signed GET in case it's a direct link to an instance with authorized fetch. const instanceActor = await this.systemAccountService.getInstanceActor(); const remoteObject = await this.apRequestService.signedGet(summary.url, instanceActor).catch(() => null); if (remoteObject) { - summary.activityPub = remoteObject.id; - summary.haveNoteLocally = false; - return; + return remoteObject.id; } + + // No match :( + return null; } } From 633718ffe90bb0eacb4013208416cc79a2eed09e Mon Sep 17 00:00:00 2001 From: Hazelnoot Date: Mon, 5 May 2025 10:32:06 -0400 Subject: [PATCH 16/18] avoid fetching notes twice in UrlPreviewService --- .../src/server/web/UrlPreviewService.ts | 23 +++++++++++-------- 1 file changed, 14 insertions(+), 9 deletions(-) diff --git a/packages/backend/src/server/web/UrlPreviewService.ts b/packages/backend/src/server/web/UrlPreviewService.ts index 2ae626f8b7..4c40496305 100644 --- a/packages/backend/src/server/web/UrlPreviewService.ts +++ b/packages/backend/src/server/web/UrlPreviewService.ts @@ -160,10 +160,13 @@ export class UrlPreviewService { summary.thumbnail = this.wrap(summary.thumbnail); // Summaly cannot always detect links to a fedi post, so do some additional tests to try and find missed cases. - summary.activityPub ??= await this.inferActivityPubLink(summary); + if (!summary.activityPub) { + await this.inferActivityPubLink(summary); + } if (summary.activityPub) { - summary.haveNoteLocally = !!await this.apDbResolverService.getNoteFromApId(summary.activityPub); + // Avoid duplicate checks in case inferActivityPubLink already set this. + summary.haveNoteLocally ||= !!await this.apDbResolverService.getNoteFromApId(summary.activityPub); } // Await this to avoid hammering redis when a bunch of URLs are fetched at once @@ -261,12 +264,14 @@ export class UrlPreviewService { } } - private async inferActivityPubLink(summary: LocalSummalyResult): Promise { + private async inferActivityPubLink(summary: LocalSummalyResult) { // Match canonical URI first. // This covers local and remote links. const isCanonicalUri = !!await this.apDbResolverService.getNoteFromApId(summary.url); if (isCanonicalUri) { - return summary.url; + summary.activityPub = summary.url; + summary.haveNoteLocally = true; + return; } // Try public URL next. @@ -284,17 +289,17 @@ export class UrlPreviewService { // Older versions did not validate URL, so do it now to avoid impersonation. const matchByUrl = urlMatches.find(({ uri }) => this.apUtilityService.haveSameAuthority(uri, summary.url)); if (matchByUrl) { - return matchByUrl.uri; + summary.activityPub = matchByUrl.uri; + summary.haveNoteLocally = true; + return; } // Finally, attempt a signed GET in case it's a direct link to an instance with authorized fetch. const instanceActor = await this.systemAccountService.getInstanceActor(); const remoteObject = await this.apRequestService.signedGet(summary.url, instanceActor).catch(() => null); if (remoteObject) { - return remoteObject.id; + summary.activityPub = remoteObject.id; + return; } - - // No match :( - return null; } } From 1ac9625eea5a33544f2424bac6de6e94ffe0a4ad Mon Sep 17 00:00:00 2001 From: Hazelnoot Date: Mon, 5 May 2025 10:37:04 -0400 Subject: [PATCH 17/18] add same-authority check between fetched note and summary url --- packages/backend/src/server/web/UrlPreviewService.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/backend/src/server/web/UrlPreviewService.ts b/packages/backend/src/server/web/UrlPreviewService.ts index 4c40496305..15a4fc946f 100644 --- a/packages/backend/src/server/web/UrlPreviewService.ts +++ b/packages/backend/src/server/web/UrlPreviewService.ts @@ -297,7 +297,7 @@ export class UrlPreviewService { // Finally, attempt a signed GET in case it's a direct link to an instance with authorized fetch. const instanceActor = await this.systemAccountService.getInstanceActor(); const remoteObject = await this.apRequestService.signedGet(summary.url, instanceActor).catch(() => null); - if (remoteObject) { + if (remoteObject && this.apUtilityService.haveSameAuthority(remoteObject.id, summary.url)) { summary.activityPub = remoteObject.id; return; } From 207915856aaf5b3fd8fd797cae5876951cd8566b Mon Sep 17 00:00:00 2001 From: Hazelnoot Date: Thu, 8 May 2025 11:06:06 -0400 Subject: [PATCH 18/18] fix return type of fetchSummary and fetchSummaryFromProxy --- packages/backend/src/server/web/UrlPreviewService.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/backend/src/server/web/UrlPreviewService.ts b/packages/backend/src/server/web/UrlPreviewService.ts index 15a4fc946f..0cab657c23 100644 --- a/packages/backend/src/server/web/UrlPreviewService.ts +++ b/packages/backend/src/server/web/UrlPreviewService.ts @@ -191,7 +191,7 @@ export class UrlPreviewService { } } - private fetchSummary(url: string, meta: MiMeta, lang?: string): Promise { + private fetchSummary(url: string, meta: MiMeta, lang?: string): Promise { const agent = this.config.proxy ? { http: this.httpRequestService.httpAgent, @@ -210,7 +210,7 @@ export class UrlPreviewService { }); } - private fetchSummaryFromProxy(url: string, meta: MiMeta, lang?: string): Promise { + private fetchSummaryFromProxy(url: string, meta: MiMeta, lang?: string): Promise { const proxy = meta.urlPreviewSummaryProxyUrl!; const queryStr = query({ followRedirects: true,