From 8477909af208b7e0f3ce9357350be6b0a0fc783d Mon Sep 17 00:00:00 2001 From: Kio! Date: Sun, 3 Nov 2024 19:50:25 +0000 Subject: [PATCH 01/66] Update report-abuse.ts --- .../backend/src/server/api/endpoints/users/report-abuse.ts | 4 ---- 1 file changed, 4 deletions(-) diff --git a/packages/backend/src/server/api/endpoints/users/report-abuse.ts b/packages/backend/src/server/api/endpoints/users/report-abuse.ts index 5ff6de37d2..38ded8ee1e 100644 --- a/packages/backend/src/server/api/endpoints/users/report-abuse.ts +++ b/packages/backend/src/server/api/endpoints/users/report-abuse.ts @@ -66,10 +66,6 @@ export default class extends Endpoint { // eslint- throw new ApiError(meta.errors.cannotReportYourself); } - if (await this.roleService.isAdministrator(targetUser)) { - throw new ApiError(meta.errors.cannotReportAdmin); - } - await this.abuseReportService.report([{ targetUserId: targetUser.id, targetUserHost: targetUser.host, From c2c2120b768adbc8f710f65f7a0fe9658efe6683 Mon Sep 17 00:00:00 2001 From: CenTdemeern1 Date: Mon, 4 Nov 2024 22:50:56 +0100 Subject: [PATCH 02/66] Center SkModPlayer on big displays Authored-by: Freeplay Co-authored-by: Freeplay --- packages/frontend/src/components/SkModPlayer.vue | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/frontend/src/components/SkModPlayer.vue b/packages/frontend/src/components/SkModPlayer.vue index 58a96cfb63..6ba983645e 100644 --- a/packages/frontend/src/components/SkModPlayer.vue +++ b/packages/frontend/src/components/SkModPlayer.vue @@ -451,6 +451,7 @@ onDeactivated(() => { overflow: hidden; display: flex; flex-direction: column; + justify-content: center; > i { display: block; From 9fe5dc679aba9b0bb3b4d743ddd0ead341ccb8d6 Mon Sep 17 00:00:00 2001 From: dakkar Date: Tue, 5 Nov 2024 14:21:58 +0000 Subject: [PATCH 03/66] check harder for connectibility `allSettled` does not throw if a promise is rejected, so `check_connect` never actually failed --- packages/backend/scripts/check_connect.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/backend/scripts/check_connect.js b/packages/backend/scripts/check_connect.js index f33a450325..17b198ef62 100644 --- a/packages/backend/scripts/check_connect.js +++ b/packages/backend/scripts/check_connect.js @@ -57,4 +57,4 @@ const promises = Array connectToPostgres() ]); -await Promise.allSettled(promises); +await Promise.all(promises); From 83f780978c563781981d71455e346eae9d28ecaf Mon Sep 17 00:00:00 2001 From: Maciej Date: Thu, 7 Nov 2024 07:57:35 +0000 Subject: [PATCH 04/66] Change example config - db name and user consistent with docs --- .config/example.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.config/example.yml b/.config/example.yml index 0062b6670c..cf7b972de5 100644 --- a/.config/example.yml +++ b/.config/example.yml @@ -99,10 +99,10 @@ db: port: 5432 # Database name - db: misskey + db: sharkey # Auth - user: example-misskey-user + user: sharkey pass: example-misskey-pass # Whether disable Caching queries From 00ab7f5bd1c954119118707e839fc549733c3169 Mon Sep 17 00:00:00 2001 From: Rachel Y Date: Thu, 7 Nov 2024 20:09:01 +0000 Subject: [PATCH 05/66] Update file Dockerfile --- Dockerfile | 1 + 1 file changed, 1 insertion(+) diff --git a/Dockerfile b/Dockerfile index acef95deab..0e156ef003 100644 --- a/Dockerfile +++ b/Dockerfile @@ -40,6 +40,7 @@ RUN apk add ffmpeg tini jemalloc \ && corepack enable \ && addgroup -g "${GID}" sharkey \ && adduser -D -u "${UID}" -G sharkey -h /sharkey sharkey \ + && mkdir /sharkey/files \ && find / -type d -path /sys -prune -o -type d -path /proc -prune -o -type f -perm /u+s -exec chmod u-s {} \; \ && find / -type d -path /sys -prune -o -type d -path /proc -prune -o -type f -perm /g+s -exec chmod g-s {} \; From aebdbf07b4b11c8bc69d77b967222ec4a0bb0141 Mon Sep 17 00:00:00 2001 From: Rachel Y Date: Thu, 7 Nov 2024 20:09:52 +0000 Subject: [PATCH 06/66] creat and chown /sharkey/files in dockerfile --- Dockerfile | 1 + 1 file changed, 1 insertion(+) diff --git a/Dockerfile b/Dockerfile index 0e156ef003..abee7fb098 100644 --- a/Dockerfile +++ b/Dockerfile @@ -41,6 +41,7 @@ RUN apk add ffmpeg tini jemalloc \ && addgroup -g "${GID}" sharkey \ && adduser -D -u "${UID}" -G sharkey -h /sharkey sharkey \ && mkdir /sharkey/files \ + && chown sharkey:sharkey /sharkey/files \ && find / -type d -path /sys -prune -o -type d -path /proc -prune -o -type f -perm /u+s -exec chmod u-s {} \; \ && find / -type d -path /sys -prune -o -type d -path /proc -prune -o -type f -perm /g+s -exec chmod g-s {} \; From 101ca9e0f7e746f2188eac93d597fc42087e3662 Mon Sep 17 00:00:00 2001 From: tess Date: Tue, 12 Nov 2024 21:16:59 +0100 Subject: [PATCH 07/66] make sure popup position is never off screen to the left --- packages/frontend/src/scripts/popup-position.ts | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/packages/frontend/src/scripts/popup-position.ts b/packages/frontend/src/scripts/popup-position.ts index 3dad41a8b3..80ad91c48f 100644 --- a/packages/frontend/src/scripts/popup-position.ts +++ b/packages/frontend/src/scripts/popup-position.ts @@ -15,6 +15,8 @@ export function calcPopupPosition(el: HTMLElement, props: { const contentWidth = el.offsetWidth; const contentHeight = el.offsetHeight; + const LEFT_MARGIN = 4; + let rect: DOMRect; if (props.anchorElement) { @@ -39,6 +41,8 @@ export function calcPopupPosition(el: HTMLElement, props: { left = window.innerWidth - contentWidth + window.scrollX - 1; } + left = Math.max(LEFT_MARGIN, left); + return [left, top]; }; @@ -60,6 +64,8 @@ export function calcPopupPosition(el: HTMLElement, props: { left = window.innerWidth - contentWidth + window.scrollX - 1; } + left = Math.max(LEFT_MARGIN, left); + return [left, top]; }; @@ -75,6 +81,8 @@ export function calcPopupPosition(el: HTMLElement, props: { top = props.y; } + left = Math.max(LEFT_MARGIN, left); + top -= (el.offsetHeight / 2); if (top + contentHeight - window.scrollY > window.innerHeight) { @@ -106,6 +114,8 @@ export function calcPopupPosition(el: HTMLElement, props: { top -= (el.offsetHeight / 2); } + left = Math.max(LEFT_MARGIN, left); + if (top + contentHeight - window.scrollY > window.innerHeight) { top = window.innerHeight - contentHeight + window.scrollY - 1; } From 19be113cb4f62eb479061fe5274fce0655ee232b Mon Sep 17 00:00:00 2001 From: tess Date: Tue, 12 Nov 2024 21:29:22 +0100 Subject: [PATCH 08/66] Keep MkUserPopup from extending past left side of screen --- packages/frontend/src/components/MkUserPopup.vue | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/frontend/src/components/MkUserPopup.vue b/packages/frontend/src/components/MkUserPopup.vue index c6f4699b3e..e98a8b85e9 100644 --- a/packages/frontend/src/components/MkUserPopup.vue +++ b/packages/frontend/src/components/MkUserPopup.vue @@ -119,7 +119,7 @@ onMounted(() => { } const rect = props.source.getBoundingClientRect(); - const x = ((rect.left + (props.source.offsetWidth / 2)) - (300 / 2)) + window.scrollX; + const x = Math.max(2, ((rect.left + (props.source.offsetWidth / 2)) - (300 / 2)) + window.scrollX); const y = rect.top + props.source.offsetHeight + window.scrollY; top.value = y; From 6d6b03dfe262c2477284aacd3da85a944b6d55b0 Mon Sep 17 00:00:00 2001 From: tess Date: Tue, 12 Nov 2024 21:30:19 +0100 Subject: [PATCH 09/66] tweak popup left margin for consistency --- packages/frontend/src/scripts/popup-position.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/frontend/src/scripts/popup-position.ts b/packages/frontend/src/scripts/popup-position.ts index 80ad91c48f..187896b0e5 100644 --- a/packages/frontend/src/scripts/popup-position.ts +++ b/packages/frontend/src/scripts/popup-position.ts @@ -15,7 +15,7 @@ export function calcPopupPosition(el: HTMLElement, props: { const contentWidth = el.offsetWidth; const contentHeight = el.offsetHeight; - const LEFT_MARGIN = 4; + const LEFT_MARGIN = 2; let rect: DOMRect; From 68e5b5a84a8ea088f7375eb1056218c4bcf2b05a Mon Sep 17 00:00:00 2001 From: tess Date: Tue, 12 Nov 2024 22:09:37 +0100 Subject: [PATCH 10/66] Set horizontal margin for even better consistency --- packages/frontend/src/components/MkUserPopup.vue | 2 +- packages/frontend/src/scripts/popup-position.ts | 14 +++++++------- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/packages/frontend/src/components/MkUserPopup.vue b/packages/frontend/src/components/MkUserPopup.vue index e98a8b85e9..4ae23de62c 100644 --- a/packages/frontend/src/components/MkUserPopup.vue +++ b/packages/frontend/src/components/MkUserPopup.vue @@ -119,7 +119,7 @@ onMounted(() => { } const rect = props.source.getBoundingClientRect(); - const x = Math.max(2, ((rect.left + (props.source.offsetWidth / 2)) - (300 / 2)) + window.scrollX); + const x = Math.max(1, ((rect.left + (props.source.offsetWidth / 2)) - (300 / 2)) + window.scrollX); const y = rect.top + props.source.offsetHeight + window.scrollY; top.value = y; diff --git a/packages/frontend/src/scripts/popup-position.ts b/packages/frontend/src/scripts/popup-position.ts index 187896b0e5..be49532cf8 100644 --- a/packages/frontend/src/scripts/popup-position.ts +++ b/packages/frontend/src/scripts/popup-position.ts @@ -15,7 +15,7 @@ export function calcPopupPosition(el: HTMLElement, props: { const contentWidth = el.offsetWidth; const contentHeight = el.offsetHeight; - const LEFT_MARGIN = 2; + const HORIZONTAL_MARGIN = 1; let rect: DOMRect; @@ -38,10 +38,10 @@ export function calcPopupPosition(el: HTMLElement, props: { left -= (el.offsetWidth / 2); if (left + contentWidth - window.scrollX > window.innerWidth) { - left = window.innerWidth - contentWidth + window.scrollX - 1; + left = window.innerWidth - contentWidth + window.scrollX - HORIZONTAL_MARGIN; } - left = Math.max(LEFT_MARGIN, left); + left = Math.max(HORIZONTAL_MARGIN, left); return [left, top]; }; @@ -61,10 +61,10 @@ export function calcPopupPosition(el: HTMLElement, props: { left -= (el.offsetWidth / 2); if (left + contentWidth - window.scrollX > window.innerWidth) { - left = window.innerWidth - contentWidth + window.scrollX - 1; + left = window.innerWidth - contentWidth + window.scrollX - HORIZONTAL_MARGIN; } - left = Math.max(LEFT_MARGIN, left); + left = Math.max(HORIZONTAL_MARGIN, left); return [left, top]; }; @@ -81,7 +81,7 @@ export function calcPopupPosition(el: HTMLElement, props: { top = props.y; } - left = Math.max(LEFT_MARGIN, left); + left = Math.max(HORIZONTAL_MARGIN, left); top -= (el.offsetHeight / 2); @@ -114,7 +114,7 @@ export function calcPopupPosition(el: HTMLElement, props: { top -= (el.offsetHeight / 2); } - left = Math.max(LEFT_MARGIN, left); + left = Math.max(HORIZONTAL_MARGIN, left); if (top + contentHeight - window.scrollY > window.innerHeight) { top = window.innerHeight - contentHeight + window.scrollY - 1; From eaad96aae30d0e1ab1ae05b5c0fef908cd002d79 Mon Sep 17 00:00:00 2001 From: piuvas Date: Fri, 15 Nov 2024 13:40:53 -0300 Subject: [PATCH 11/66] edit query --- .../backend/src/server/api/endpoints/emojis.ts | 15 +++++---------- 1 file changed, 5 insertions(+), 10 deletions(-) diff --git a/packages/backend/src/server/api/endpoints/emojis.ts b/packages/backend/src/server/api/endpoints/emojis.ts index 46ef4eca1b..8054de3d95 100644 --- a/packages/backend/src/server/api/endpoints/emojis.ts +++ b/packages/backend/src/server/api/endpoints/emojis.ts @@ -50,16 +50,11 @@ export default class extends Endpoint { // eslint- private emojiEntityService: EmojiEntityService, ) { super(meta, paramDef, async (ps, me) => { - const emojis = await this.emojisRepository.find({ - where: { - host: IsNull(), - }, - order: { - category: 'ASC', - name: 'ASC', - }, - }); - + const emojis = await this.emojisRepository.createQueryBuilder() + .where('host IS NULL') + .orderBy('LOWER(category)', 'ASC') + .orderBy('LOWER(name)', 'ASC') + .getMany() return { emojis: await this.emojiEntityService.packSimpleMany(emojis), }; From d150e92f41807e83db3dfed3fb359cb685d8102b Mon Sep 17 00:00:00 2001 From: Hazelnoot Date: Tue, 19 Nov 2024 22:59:07 -0500 Subject: [PATCH 12/66] prevent DoS from spammed media proxy requests --- .../backend/src/server/FileServerService.ts | 89 +++++++++++++++++++ 1 file changed, 89 insertions(+) diff --git a/packages/backend/src/server/FileServerService.ts b/packages/backend/src/server/FileServerService.ts index 1a4d0cb48f..be196373c4 100644 --- a/packages/backend/src/server/FileServerService.ts +++ b/packages/backend/src/server/FileServerService.ts @@ -28,7 +28,11 @@ import { bindThis } from '@/decorators.js'; import { isMimeImage } from '@/misc/is-mime-image.js'; import { correctFilename } from '@/misc/correct-filename.js'; import { handleRequestRedirectToOmitSearch } from '@/misc/fastify-hook-handlers.js'; +import { RateLimiterService } from '@/server/api/RateLimiterService.js'; +import { getIpHash } from '@/misc/get-ip-hash.js'; +import { AuthenticateService } from '@/server/api/AuthenticateService.js'; import type { FastifyInstance, FastifyRequest, FastifyReply, FastifyPluginOptions } from 'fastify'; +import type Limiter from 'ratelimiter'; const _filename = fileURLToPath(import.meta.url); const _dirname = dirname(_filename); @@ -52,6 +56,8 @@ export class FileServerService { private videoProcessingService: VideoProcessingService, private internalStorageService: InternalStorageService, private loggerService: LoggerService, + private authenticateService: AuthenticateService, + private rateLimiterService: RateLimiterService, ) { this.logger = this.loggerService.getLogger('server', 'gray'); @@ -76,6 +82,8 @@ export class FileServerService { }); fastify.get<{ Params: { key: string; } }>('/files/:key', async (request, reply) => { + if (!await this.checkRateLimit(request, reply, `/files/${request.params.key}`)) return; + return await this.sendDriveFile(request, reply) .catch(err => this.errorHandler(request, reply, err)); }); @@ -89,6 +97,20 @@ export class FileServerService { Params: { url: string; }; Querystring: { url?: string; }; }>('/proxy/:url*', async (request, reply) => { + const url = 'url' in request.query ? request.query.url : 'https://' + request.params.url; + if (!url || !URL.canParse(url)) { + reply.code(400); + return; + } + + const keyUrl = new URL(url); + keyUrl.searchParams.forEach(k => keyUrl.searchParams.delete(k)); + keyUrl.hash = ''; + keyUrl.username = ''; + keyUrl.password = ''; + + if (!await this.checkRateLimit(request, reply, `/proxy/${keyUrl}`)) return; + return await this.proxyHandler(request, reply) .catch(err => this.errorHandler(request, reply, err)); }); @@ -572,4 +594,71 @@ export class FileServerService { path, }; } + + // Based on ApiCallService + private async checkRateLimit( + request: FastifyRequest<{ + Body?: Record | undefined, + Querystring?: Record | undefined, + Params?: Record | unknown, + }>, + reply: FastifyReply, + rateLimitKey: string, + ): Promise { + const body = request.method === 'GET' + ? request.query + : request.body; + + // https://datatracker.ietf.org/doc/html/rfc6750.html#section-2.1 (case sensitive) + const token = request.headers.authorization?.startsWith('Bearer ') + ? request.headers.authorization.slice(7) + : body?.['i']; + if (token != null && typeof token !== 'string') { + reply.code(400); + return false; + } + + // koa will automatically load the `X-Forwarded-For` header if `proxy: true` is configured in the app. + const [user] = await this.authenticateService.authenticate(token); + const actor = user?.id ?? getIpHash(request.ip); + + const limit = { + // Group by resource + key: rateLimitKey, + + // Maximum of 10 requests / 10 minutes + max: 10, + duration: 1000 * 60 * 10, + + // Minimum of 250 ms between each request + minInterval: 250, + }; + + // Rate limit proxy requests + try { + await this.rateLimiterService.limit(limit, actor); + return true; + } catch (err) { + // errはLimiter.LimiterInfoであることが期待される + reply.code(429); + + if (hasRateLimitInfo(err)) { + const cooldownInSeconds = Math.ceil((err.info.resetMs - Date.now()) / 1000); + // もしかするとマイナスになる可能性がなくはないのでマイナスだったら0にしておく + reply.header('Retry-After', Math.max(cooldownInSeconds, 0).toString(10)); + } + + reply.send({ + message: 'Rate limit exceeded. Please try again later.', + code: 'RATE_LIMIT_EXCEEDED', + id: 'd5826d14-3982-4d2e-8011-b9e9f02499ef', + }); + + return false; + } + } +} + +function hasRateLimitInfo(err: unknown): err is { info: Limiter.LimiterInfo } { + return err != null && typeof(err) === 'object' && 'info' in err; } From 680c2a071846b6051fbcee838522de33837f011f Mon Sep 17 00:00:00 2001 From: Julia Johannesen Date: Wed, 20 Nov 2024 00:09:56 -0500 Subject: [PATCH 13/66] Bump version --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index b312044921..2ba3881756 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "sharkey", - "version": "2024.9.1", + "version": "2024.9.2", "codename": "shonk", "repository": { "type": "git", From 41c500851b62b343ee47392f39cacbee3d1b20b8 Mon Sep 17 00:00:00 2001 From: Julia Johannesen Date: Wed, 20 Nov 2024 00:54:30 -0500 Subject: [PATCH 14/66] Bump develop version --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index 2ba3881756..6e84882440 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "sharkey", - "version": "2024.9.2", + "version": "2024.10.0-dev", "codename": "shonk", "repository": { "type": "git", From fb54546573f267701bd82175dc26a431518fb6c8 Mon Sep 17 00:00:00 2001 From: Julia Johannesen Date: Wed, 20 Nov 2024 01:17:24 -0500 Subject: [PATCH 15/66] Fix linter error in emojis endpoint --- packages/backend/src/server/api/endpoints/emojis.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/backend/src/server/api/endpoints/emojis.ts b/packages/backend/src/server/api/endpoints/emojis.ts index 8054de3d95..4dd3a2ed50 100644 --- a/packages/backend/src/server/api/endpoints/emojis.ts +++ b/packages/backend/src/server/api/endpoints/emojis.ts @@ -54,7 +54,7 @@ export default class extends Endpoint { // eslint- .where('host IS NULL') .orderBy('LOWER(category)', 'ASC') .orderBy('LOWER(name)', 'ASC') - .getMany() + .getMany(); return { emojis: await this.emojiEntityService.packSimpleMany(emojis), }; From d883934826122b64439170c2013d65606fd23184 Mon Sep 17 00:00:00 2001 From: Laura Hausmann Date: Thu, 24 Oct 2024 05:13:35 +0200 Subject: [PATCH 16/66] fix: primitive 2: acceptance of cross-origin alternate links --- packages/backend/src/core/activitypub/ApRequestService.ts | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/packages/backend/src/core/activitypub/ApRequestService.ts b/packages/backend/src/core/activitypub/ApRequestService.ts index 38c78cf900..35eb1a5c53 100644 --- a/packages/backend/src/core/activitypub/ApRequestService.ts +++ b/packages/backend/src/core/activitypub/ApRequestService.ts @@ -18,6 +18,7 @@ import type Logger from '@/logger.js'; import type { IObject } from './type.js'; import { validateContentTypeSetAsActivityPub } from '@/core/activitypub/misc/validator.js'; import { assertActivityMatchesUrls } from '@/core/activitypub/misc/check-against-url.js'; +import { UtilityService } from "@/core/UtilityService.js"; type Request = { url: string; @@ -147,6 +148,7 @@ export class ApRequestService { private userKeypairService: UserKeypairService, private httpRequestService: HttpRequestService, private loggerService: LoggerService, + private utilityService: UtilityService, ) { // eslint-disable-next-line @typescript-eslint/no-unnecessary-condition this.logger = this.loggerService?.getLogger('ap-request'); // なぜか TypeError: Cannot read properties of undefined (reading 'getLogger') と言われる @@ -241,7 +243,9 @@ export class ApRequestService { if (alternate) { const href = alternate.getAttribute('href'); if (href) { - return await this.signedGet(href, user, false); + if (this.utilityService.punyHost(url) === this.utilityService.punyHost(href)) { + return await this.signedGet(href, user, false); + } } } } catch (e) { From 9090b745e69c380847f97f107d39864d7ace0039 Mon Sep 17 00:00:00 2001 From: Laura Hausmann Date: Thu, 24 Oct 2024 04:04:56 +0200 Subject: [PATCH 17/66] fix: primitive 3: validation of non-final url --- packages/backend/src/core/HttpRequestService.ts | 2 +- packages/backend/src/core/activitypub/ApRequestService.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/backend/src/core/HttpRequestService.ts b/packages/backend/src/core/HttpRequestService.ts index bea5dee6ab..08e9f46b2d 100644 --- a/packages/backend/src/core/HttpRequestService.ts +++ b/packages/backend/src/core/HttpRequestService.ts @@ -129,7 +129,7 @@ export class HttpRequestService { const finalUrl = res.url; // redirects may have been involved const activity = await res.json() as IObject; - assertActivityMatchesUrls(activity, [url, finalUrl]); + assertActivityMatchesUrls(activity, [finalUrl]); return activity; } diff --git a/packages/backend/src/core/activitypub/ApRequestService.ts b/packages/backend/src/core/activitypub/ApRequestService.ts index 35eb1a5c53..eeff73385b 100644 --- a/packages/backend/src/core/activitypub/ApRequestService.ts +++ b/packages/backend/src/core/activitypub/ApRequestService.ts @@ -261,7 +261,7 @@ export class ApRequestService { const finalUrl = res.url; // redirects may have been involved const activity = await res.json() as IObject; - assertActivityMatchesUrls(activity, [url, finalUrl]); + assertActivityMatchesUrls(activity, [finalUrl]); return activity; } From 1e14612f0e4349b00f5016f3a9184ff3e40fc37e Mon Sep 17 00:00:00 2001 From: Laura Hausmann Date: Thu, 24 Oct 2024 04:11:35 +0200 Subject: [PATCH 18/66] fix: primitive 4: missing same-origin identifier validation of collection-wrapped activities --- packages/backend/src/core/activitypub/ApInboxService.ts | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/packages/backend/src/core/activitypub/ApInboxService.ts b/packages/backend/src/core/activitypub/ApInboxService.ts index d54c9544c3..5a6f6f083f 100644 --- a/packages/backend/src/core/activitypub/ApInboxService.ts +++ b/packages/backend/src/core/activitypub/ApInboxService.ts @@ -100,6 +100,10 @@ export class ApInboxService { const resolver = this.apResolverService.createResolver(); for (const item of toArray(isCollection(activity) ? activity.items : activity.orderedItems)) { const act = await resolver.resolve(item); + if (act.id == null || this.utilityService.extractDbHost(act.id) !== this.utilityService.extractDbHost(actor.uri)) { + this.logger.debug('skipping activity: activity id is null or mismatching'); + continue; + } try { results.push([getApId(item), await this.performOneActivity(actor, act)]); } catch (err) { From ad8e8793c7b0ecc08bb271cd83ba04f6f8be7036 Mon Sep 17 00:00:00 2001 From: Laura Hausmann Date: Thu, 24 Oct 2024 04:37:47 +0200 Subject: [PATCH 19/66] fix: primitives 5 & 8: reject activities with non-string identifiers --- packages/backend/src/queue/processors/InboxProcessorService.ts | 3 +++ 1 file changed, 3 insertions(+) diff --git a/packages/backend/src/queue/processors/InboxProcessorService.ts b/packages/backend/src/queue/processors/InboxProcessorService.ts index 11b00bb683..f453d7d1ae 100644 --- a/packages/backend/src/queue/processors/InboxProcessorService.ts +++ b/packages/backend/src/queue/processors/InboxProcessorService.ts @@ -193,6 +193,9 @@ export class InboxProcessorService implements OnApplicationShutdown { throw new Bull.UnrecoverableError(`skip: signerHost(${signerHost}) !== activity.id host(${activityIdHost}`); } } + else { + throw new Bull.UnrecoverableError('skip: activity id is not a string'); + } // Update stats this.federatedInstanceService.fetch(authUser.user.host).then(i => { From 174dfb83d09d13876c65b98c75769d01f5c0ec47 Mon Sep 17 00:00:00 2001 From: Laura Hausmann Date: Thu, 24 Oct 2024 04:28:43 +0200 Subject: [PATCH 20/66] fix: primitive 6: reject anonymous objects that were fetched by their id --- packages/backend/src/core/activitypub/ApResolverService.ts | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/packages/backend/src/core/activitypub/ApResolverService.ts b/packages/backend/src/core/activitypub/ApResolverService.ts index 5d5c61ce2c..a2c7ed19d8 100644 --- a/packages/backend/src/core/activitypub/ApResolverService.ts +++ b/packages/backend/src/core/activitypub/ApResolverService.ts @@ -121,7 +121,11 @@ export class Resolver { // `object.id` or `object.url` matches the URL used to fetch the // object after redirects; here we double-check that no redirects // bounced between hosts - if (object.id && (this.utilityService.punyHost(object.id) !== this.utilityService.punyHost(value))) { + if (object.id == null) { + throw new Error('invalid AP object: missing id'); + } + + if (this.utilityService.punyHost(object.id) !== this.utilityService.punyHost(value)) { throw new Error(`invalid AP object ${value}: id ${object.id} has different host`); } From 9ab25ede28f4f04ac2ae48c947e7668a9a6012b2 Mon Sep 17 00:00:00 2001 From: Laura Hausmann Date: Thu, 24 Oct 2024 04:40:33 +0200 Subject: [PATCH 21/66] fix: primitives 9, 10 & 11: http signature validation doesn't enforce required headers or specify auth header name --- packages/backend/src/server/ActivityPubServerService.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/backend/src/server/ActivityPubServerService.ts b/packages/backend/src/server/ActivityPubServerService.ts index 52592c47c6..f955329fd1 100644 --- a/packages/backend/src/server/ActivityPubServerService.ts +++ b/packages/backend/src/server/ActivityPubServerService.ts @@ -152,7 +152,7 @@ export class ActivityPubServerService { let signature; try { - signature = httpSignature.parseRequest(request.raw, { 'headers': [] }); + signature = httpSignature.parseRequest(request.raw, { 'headers': ['(request-target)', 'host', 'date'], authorizationHeaderName: 'signature' }); } catch (e) { // not signed, or malformed signature: refuse this.authlogger.warn(`${request.id} ${request.url} not signed, or malformed signature: refuse`); @@ -229,7 +229,7 @@ export class ActivityPubServerService { let signature; try { - signature = httpSignature.parseRequest(request.raw, { 'headers': [] }); + signature = httpSignature.parseRequest(request.raw, { 'headers': ['(request-target)', 'digest', 'host', 'date'], authorizationHeaderName: 'signature' }); } catch (e) { reply.code(401); return; From 1c7e05ce9ee6a4a5669ec73c940dcf57ecdc3db5 Mon Sep 17 00:00:00 2001 From: Julia Johannesen Date: Thu, 14 Nov 2024 19:57:29 -0500 Subject: [PATCH 22/66] fix: primitive 7 & 12: prevent poll spoofing --- .../src/core/activitypub/ApInboxService.ts | 2 +- .../activitypub/models/ApQuestionService.ts | 29 ++++++++++++++----- 2 files changed, 23 insertions(+), 8 deletions(-) diff --git a/packages/backend/src/core/activitypub/ApInboxService.ts b/packages/backend/src/core/activitypub/ApInboxService.ts index 5a6f6f083f..edd1041062 100644 --- a/packages/backend/src/core/activitypub/ApInboxService.ts +++ b/packages/backend/src/core/activitypub/ApInboxService.ts @@ -786,7 +786,7 @@ export class ApInboxService { await this.apPersonService.updatePerson(actor.uri, resolver, object); return 'ok: Person updated'; } else if (getApType(object) === 'Question') { - await this.apQuestionService.updateQuestion(object, resolver).catch(err => console.error(err)); + await this.apQuestionService.updateQuestion(object, actor, resolver).catch(err => console.error(err)); return 'ok: Question updated'; } else if (getApType(object) === 'Note') { await this.apNoteService.updateNote(object, resolver).catch(err => console.error(err)); diff --git a/packages/backend/src/core/activitypub/models/ApQuestionService.ts b/packages/backend/src/core/activitypub/models/ApQuestionService.ts index 9246398fde..c1aea15ece 100644 --- a/packages/backend/src/core/activitypub/models/ApQuestionService.ts +++ b/packages/backend/src/core/activitypub/models/ApQuestionService.ts @@ -5,16 +5,17 @@ import { Inject, Injectable } from '@nestjs/common'; import { DI } from '@/di-symbols.js'; -import type { NotesRepository, PollsRepository } from '@/models/_.js'; +import type { UsersRepository, NotesRepository, PollsRepository } from '@/models/_.js'; import type { Config } from '@/config.js'; import type { IPoll } from '@/models/Poll.js'; +import type { MiRemoteUser } from '@/models/User.js'; import type Logger from '@/logger.js'; import { bindThis } from '@/decorators.js'; -import { isQuestion } from '../type.js'; +import { getOneApId, isQuestion } from '../type.js'; import { ApLoggerService } from '../ApLoggerService.js'; import { ApResolverService } from '../ApResolverService.js'; import type { Resolver } from '../ApResolverService.js'; -import type { IObject, IQuestion } from '../type.js'; +import type { IObject } from '../type.js'; @Injectable() export class ApQuestionService { @@ -24,6 +25,9 @@ export class ApQuestionService { @Inject(DI.config) private config: Config, + @Inject(DI.usersRepository) + private usersRepository: UsersRepository, + @Inject(DI.notesRepository) private notesRepository: NotesRepository, @@ -65,7 +69,7 @@ export class ApQuestionService { * @returns true if updated */ @bindThis - public async updateQuestion(value: string | IObject, resolver?: Resolver): Promise { + public async updateQuestion(value: string | IObject, actor?: MiRemoteUser, resolver?: Resolver): Promise { const uri = typeof value === 'string' ? value : value.id; if (uri == null) throw new Error('uri is null'); @@ -78,15 +82,26 @@ export class ApQuestionService { const poll = await this.pollsRepository.findOneBy({ noteId: note.id }); if (poll == null) throw new Error('Question is not registered'); + + const user = await this.usersRepository.findOneBy({ id: poll.userId }); + if (user == null) throw new Error('Question is not registered'); //#endregion // resolve new Question object // eslint-disable-next-line no-param-reassign if (resolver == null) resolver = this.apResolverService.createResolver(); - const question = await resolver.resolve(value) as IQuestion; + const question = await resolver.resolve(value); this.logger.debug(`fetched question: ${JSON.stringify(question, null, 2)}`); - if (question.type !== 'Question') throw new Error('object is not a Question'); + if (!isQuestion(question)) throw new Error('object is not a Question'); + + const attribution = (question.attributedTo) ? getOneApId(question.attributedTo) : user.uri; + const attributionMatchesExisting = attribution === user.uri; + const actorMatchesAttribution = (actor) ? attribution === actor.uri : true; + + if (!attributionMatchesExisting || !actorMatchesAttribution) { + throw new Error('Refusing to ingest update for poll by different user'); + } const apChoices = question.oneOf ?? question.anyOf; if (apChoices == null) throw new Error('invalid apChoices: ' + apChoices); @@ -96,7 +111,7 @@ export class ApQuestionService { for (const choice of poll.choices) { const oldCount = poll.votes[poll.choices.indexOf(choice)]; const newCount = apChoices.filter(ap => ap.name === choice).at(0)?.replies?.totalItems; - if (newCount == null) throw new Error('invalid newCount: ' + newCount); + if (newCount == null || !(Number.isInteger(newCount) && newCount >= 0)) throw new Error('invalid newCount: ' + newCount); if (oldCount <= newCount) { changed = true; From 322b3b677ffd8fe893c6a94fbaf60768add095cc Mon Sep 17 00:00:00 2001 From: Laura Hausmann Date: Sat, 26 Oct 2024 19:51:11 +0200 Subject: [PATCH 23/66] fix: primitive 14: improper validation of outbox, followers, following & shared inbox collections --- .../src/core/activitypub/models/ApPersonService.ts | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/packages/backend/src/core/activitypub/models/ApPersonService.ts b/packages/backend/src/core/activitypub/models/ApPersonService.ts index 2046dad099..97b4dd27c9 100644 --- a/packages/backend/src/core/activitypub/models/ApPersonService.ts +++ b/packages/backend/src/core/activitypub/models/ApPersonService.ts @@ -154,13 +154,24 @@ export class ApPersonService implements OnModuleInit { throw new Error('invalid Actor: inbox has different host'); } + const sharedInboxObject = x.sharedInbox ?? (x.endpoints ? x.endpoints.sharedInbox : undefined); + if (sharedInboxObject != null) { + const sharedInbox = getApId(sharedInboxObject); + if (!(typeof sharedInbox === "string" && sharedInbox.length > 0 && this.utilityService.punyHost(sharedInbox) === expectHost)) { + throw new Error("invalid Actor: wrong shared inbox"); + } + } + for (const collection of ['outbox', 'followers', 'following'] as (keyof IActor)[]) { - const collectionUri = (x as IActor)[collection]; + const collectionUri = getApId((x as IActor)[collection]); if (typeof collectionUri === 'string' && collectionUri.length > 0) { if (this.utilityService.punyHost(collectionUri) !== expectHost) { throw new Error(`invalid Actor: ${collection} has different host`); } } + else if (collectionUri != null) { + throw new Error(`invalid Actor: wrong ${collection}`); + } } if (!(typeof x.preferredUsername === 'string' && x.preferredUsername.length > 0 && x.preferredUsername.length <= 128 && /^\w([\w-.]*\w)?$/.test(x.preferredUsername))) { From 4c432c07cbea935e06a839d897c3728c8964200d Mon Sep 17 00:00:00 2001 From: Julia Johannesen Date: Thu, 14 Nov 2024 20:21:17 -0500 Subject: [PATCH 24/66] fix: code style for primitive 14 --- .../backend/src/core/activitypub/models/ApPersonService.ts | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/packages/backend/src/core/activitypub/models/ApPersonService.ts b/packages/backend/src/core/activitypub/models/ApPersonService.ts index 97b4dd27c9..8ddd646f05 100644 --- a/packages/backend/src/core/activitypub/models/ApPersonService.ts +++ b/packages/backend/src/core/activitypub/models/ApPersonService.ts @@ -157,8 +157,8 @@ export class ApPersonService implements OnModuleInit { const sharedInboxObject = x.sharedInbox ?? (x.endpoints ? x.endpoints.sharedInbox : undefined); if (sharedInboxObject != null) { const sharedInbox = getApId(sharedInboxObject); - if (!(typeof sharedInbox === "string" && sharedInbox.length > 0 && this.utilityService.punyHost(sharedInbox) === expectHost)) { - throw new Error("invalid Actor: wrong shared inbox"); + if (!(typeof sharedInbox === 'string' && sharedInbox.length > 0 && this.utilityService.punyHost(sharedInbox) === expectHost)) { + throw new Error('invalid Actor: wrong shared inbox'); } } @@ -168,8 +168,7 @@ export class ApPersonService implements OnModuleInit { if (this.utilityService.punyHost(collectionUri) !== expectHost) { throw new Error(`invalid Actor: ${collection} has different host`); } - } - else if (collectionUri != null) { + } else if (collectionUri != null) { throw new Error(`invalid Actor: wrong ${collection}`); } } From ebea1a296228fb2a7694e9090e4fa8080cbaa1ec Mon Sep 17 00:00:00 2001 From: Laura Hausmann Date: Thu, 24 Oct 2024 05:07:58 +0200 Subject: [PATCH 25/66] fix: primitive 15: improper same-origin validation for note uri and url --- .../core/activitypub/models/ApNoteService.ts | 32 ++++++++++++++++--- 1 file changed, 28 insertions(+), 4 deletions(-) diff --git a/packages/backend/src/core/activitypub/models/ApNoteService.ts b/packages/backend/src/core/activitypub/models/ApNoteService.ts index f404a77fbb..146ccb11a2 100644 --- a/packages/backend/src/core/activitypub/models/ApNoteService.ts +++ b/packages/backend/src/core/activitypub/models/ApNoteService.ts @@ -141,14 +141,24 @@ export class ApNoteService { this.logger.debug(`Note fetched: ${JSON.stringify(note, null, 2)}`); - if (note.id && !checkHttps(note.id)) { + if (note.id == null) { + throw new Error('Refusing to create note without id'); + } + + if (!checkHttps(note.id)) { throw new Error('unexpected schema of note.id: ' + note.id); } const url = getOneApHrefNullable(note.url); - if (url && !checkHttps(url)) { - throw new Error('unexpected schema of note url: ' + url); + if (url != null) { + if (!checkHttps(url)) { + throw new Error('unexpected schema of note url: ' + url); + } + + if (this.utilityService.punyHost(url) !== this.utilityService.punyHost(note.id)) { + throw new Error(`note url <> uri host mismatch: ${url} <> ${note.id}`); + } } this.logger.info(`Creating the Note: ${note.id}`); @@ -366,7 +376,11 @@ export class ApNoteService { this.logger.debug(`Note fetched: ${JSON.stringify(note, null, 2)}`); - if (note.id && !checkHttps(note.id)) { + if (note.id == null) { + throw new Error('Refusing to update note without id'); + } + + if (!checkHttps(note.id)) { throw new Error('unexpected schema of note.id: ' + note.id); } @@ -376,6 +390,16 @@ export class ApNoteService { throw new Error('unexpected schema of note url: ' + url); } + if (url != null) { + if (!checkHttps(url)) { + throw new Error('unexpected schema of note url: ' + url); + } + + if (this.utilityService.punyHost(url) !== this.utilityService.punyHost(note.id)) { + throw new Error(`note url <> id host mismatch: ${url} <> ${note.id}`); + } + } + this.logger.info(`Creating the Note: ${note.id}`); // 投稿者をフェッチ From b74e2e91674ee56ef0b835daa31f5a72d02ab37d Mon Sep 17 00:00:00 2001 From: Laura Hausmann Date: Thu, 24 Oct 2024 05:11:16 +0200 Subject: [PATCH 26/66] fix: primitive 16: improper same-origin validation for user uri and url --- .../activitypub/models/ApPersonService.ts | 28 ++++++++++++++++--- 1 file changed, 24 insertions(+), 4 deletions(-) diff --git a/packages/backend/src/core/activitypub/models/ApPersonService.ts b/packages/backend/src/core/activitypub/models/ApPersonService.ts index 8ddd646f05..7a3bd57d43 100644 --- a/packages/backend/src/core/activitypub/models/ApPersonService.ts +++ b/packages/backend/src/core/activitypub/models/ApPersonService.ts @@ -337,8 +337,18 @@ export class ApPersonService implements OnModuleInit { const url = getOneApHrefNullable(person.url); - if (url && !checkHttps(url)) { - throw new Error('unexpected schema of person url: ' + url); + if (person.id == null) { + throw new Error('Refusing to create person without id'); + } + + if (url != null) { + if (!checkHttps(url)) { + throw new Error('unexpected schema of person url: ' + url); + } + + if (this.utilityService.punyHost(url) !== this.utilityService.punyHost(person.id)) { + throw new Error(`person url <> uri host mismatch: ${url} <> ${person.id}`); + } } // Create user @@ -539,8 +549,18 @@ export class ApPersonService implements OnModuleInit { const url = getOneApHrefNullable(person.url); - if (url && !checkHttps(url)) { - throw new Error('unexpected schema of person url: ' + url); + if (person.id == null) { + throw new Error('Refusing to update person without id'); + } + + if (url != null) { + if (!checkHttps(url)) { + throw new Error('unexpected schema of person url: ' + url); + } + + if (this.utilityService.punyHost(url) !== this.utilityService.punyHost(person.id)) { + throw new Error(`person url <> uri host mismatch: ${url} <> ${person.id}`); + } } const updates = { From 4d925fc08683a9415c9488b5bcc516ca8f43d4af Mon Sep 17 00:00:00 2001 From: Laura Hausmann Date: Thu, 24 Oct 2024 04:18:49 +0200 Subject: [PATCH 27/66] fix: primitive 17: note same-origin identifier validation can be bypassed by wrapping the id in an array --- packages/backend/src/core/activitypub/ApInboxService.ts | 3 +++ 1 file changed, 3 insertions(+) diff --git a/packages/backend/src/core/activitypub/ApInboxService.ts b/packages/backend/src/core/activitypub/ApInboxService.ts index edd1041062..b5a97d34c4 100644 --- a/packages/backend/src/core/activitypub/ApInboxService.ts +++ b/packages/backend/src/core/activitypub/ApInboxService.ts @@ -426,6 +426,9 @@ export class ApInboxService { return 'skip: host in actor.uri !== note.id'; } } + else { + return 'skip: note.id is not a string' + } } const unlock = await this.appLockService.getApLock(uri); From b9080da75dea7e9c5d38976814118a594be9e019 Mon Sep 17 00:00:00 2001 From: Julia Johannesen Date: Thu, 14 Nov 2024 20:28:50 -0500 Subject: [PATCH 28/66] fix: code style for primitive 17 --- packages/backend/src/core/activitypub/ApInboxService.ts | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/packages/backend/src/core/activitypub/ApInboxService.ts b/packages/backend/src/core/activitypub/ApInboxService.ts index b5a97d34c4..42c2007799 100644 --- a/packages/backend/src/core/activitypub/ApInboxService.ts +++ b/packages/backend/src/core/activitypub/ApInboxService.ts @@ -425,9 +425,8 @@ export class ApInboxService { if (this.utilityService.extractDbHost(actor.uri) !== this.utilityService.extractDbHost(note.id)) { return 'skip: host in actor.uri !== note.id'; } - } - else { - return 'skip: note.id is not a string' + } else { + return 'skip: note.id is not a string'; } } From c04f34404991d69103703d781f52658fe3214d15 Mon Sep 17 00:00:00 2001 From: Julia Johannesen Date: Thu, 14 Nov 2024 21:17:30 -0500 Subject: [PATCH 29/66] fix: primitive 13: check attribution against actor in notes --- .../src/core/activitypub/ApInboxService.ts | 4 +- .../core/activitypub/models/ApNoteService.ts | 71 ++++++++++++------- .../src/server/api/endpoints/ap/show.ts | 2 +- 3 files changed, 48 insertions(+), 29 deletions(-) diff --git a/packages/backend/src/core/activitypub/ApInboxService.ts b/packages/backend/src/core/activitypub/ApInboxService.ts index 42c2007799..0f56ad3f78 100644 --- a/packages/backend/src/core/activitypub/ApInboxService.ts +++ b/packages/backend/src/core/activitypub/ApInboxService.ts @@ -436,7 +436,7 @@ export class ApInboxService { const exist = await this.apNoteService.fetchNote(note); if (exist) return 'skip: note exists'; - await this.apNoteService.createNote(note, resolver, silent); + await this.apNoteService.createNote(note, actor, resolver, silent); return 'ok'; } catch (err) { if (err instanceof StatusError && !err.isRetryable) { @@ -791,7 +791,7 @@ export class ApInboxService { await this.apQuestionService.updateQuestion(object, actor, resolver).catch(err => console.error(err)); return 'ok: Question updated'; } else if (getApType(object) === 'Note') { - await this.apNoteService.updateNote(object, resolver).catch(err => console.error(err)); + await this.apNoteService.updateNote(object, actor, resolver).catch(err => console.error(err)); return 'ok: Note updated'; } else { return `skip: Unknown type: ${getApType(object)}`; diff --git a/packages/backend/src/core/activitypub/models/ApNoteService.ts b/packages/backend/src/core/activitypub/models/ApNoteService.ts index 146ccb11a2..7857bcc28c 100644 --- a/packages/backend/src/core/activitypub/models/ApNoteService.ts +++ b/packages/backend/src/core/activitypub/models/ApNoteService.ts @@ -6,7 +6,7 @@ import { forwardRef, Inject, Injectable } from '@nestjs/common'; import { In } from 'typeorm'; import { DI } from '@/di-symbols.js'; -import type { PollsRepository, EmojisRepository, NotesRepository, MiMeta } from '@/models/_.js'; +import type { UsersRepository, PollsRepository, EmojisRepository, NotesRepository, MiMeta } from '@/models/_.js'; import type { Config } from '@/config.js'; import type { MiRemoteUser } from '@/models/User.js'; import type { MiNote } from '@/models/Note.js'; @@ -49,6 +49,9 @@ export class ApNoteService { @Inject(DI.meta) private meta: MiMeta, + @Inject(DI.usersRepository) + private usersRepository: UsersRepository, + @Inject(DI.pollsRepository) private pollsRepository: PollsRepository, @@ -82,7 +85,13 @@ export class ApNoteService { } @bindThis - public validateNote(object: IObject, uri: string): Error | null { + public validateNote( + object: IObject, + uri: string, + actor?: MiRemoteUser, + user?: MiRemoteUser, + note?: MiNote, + ): Error | null { const expectHost = this.utilityService.extractDbHost(uri); const apType = getApType(object); @@ -99,10 +108,27 @@ export class ApNoteService { return new IdentifiableError('d450b8a9-48e4-4dab-ae36-f4db763fda7c', `invalid Note: attributedTo has different host. expected: ${expectHost}, actual: ${actualHost}`); } + if (actor) { + const attribution = (object.attributedTo) ? getOneApId(object.attributedTo) : actor.uri; + if (attribution !== actor.uri) { + return new IdentifiableError('d450b8a9-48e4-4dab-ae36-f4db763fda7c', `invalid Note: attribution does not match the actor that send it. attribution: ${attribution}, actor: ${actor.uri}`); + } + if (user && attribution !== user.uri) { + return new IdentifiableError('d450b8a9-48e4-4dab-ae36-f4db763fda7c', `invalid Note: updated attribution does not match original attribution. updated attribution: ${user.uri}, original attribution: ${attribution}`); + } + } + if (object.published && !this.idService.isSafeT(new Date(object.published).valueOf())) { return new IdentifiableError('d450b8a9-48e4-4dab-ae36-f4db763fda7c', 'invalid Note: published timestamp is malformed'); } + if (note) { + const url = (object.url) ? getOneApId(object.url) : note.url; + if (url && url !== note.url) { + return new IdentifiableError('d450b8a9-48e4-4dab-ae36-f4db763fda7c', `invalid Note: updated url does not match original url. updated url: ${url}, original url: ${note.url}`); + } + } + return null; } @@ -120,14 +146,14 @@ export class ApNoteService { * Noteを作成します。 */ @bindThis - public async createNote(value: string | IObject, resolver?: Resolver, silent = false): Promise { + public async createNote(value: string | IObject, actor?: MiRemoteUser, resolver?: Resolver, silent = false): Promise { // eslint-disable-next-line no-param-reassign if (resolver == null) resolver = this.apResolverService.createResolver(); const object = await resolver.resolve(value); const entryUri = getApId(value); - const err = this.validateNote(object, entryUri); + const err = this.validateNote(object, entryUri, actor); if (err) { this.logger.error(err.message, { resolver: { history: resolver.getHistory() }, @@ -171,8 +197,9 @@ export class ApNoteService { const uri = getOneApId(note.attributedTo); // ローカルで投稿者を検索し、もし凍結されていたらスキップ - const cachedActor = await this.apPersonService.fetchPerson(uri) as MiRemoteUser; - if (cachedActor && cachedActor.isSuspended) { + // eslint-disable-next-line no-param-reassign + actor ??= await this.apPersonService.fetchPerson(uri) as MiRemoteUser | undefined; + if (actor && actor.isSuspended) { throw new IdentifiableError('85ab9bd7-3a41-4530-959d-f07073900109', 'actor has been suspended'); } @@ -204,7 +231,8 @@ export class ApNoteService { } //#endregion - const actor = cachedActor ?? await this.apPersonService.resolvePerson(uri, resolver) as MiRemoteUser; + // eslint-disable-next-line no-param-reassign + actor ??= await this.apPersonService.resolvePerson(uri, resolver) as MiRemoteUser; // 解決した投稿者が凍結されていたらスキップ if (actor.isSuspended) { @@ -345,7 +373,7 @@ export class ApNoteService { * Noteを作成します。 */ @bindThis - public async updateNote(value: string | IObject, resolver?: Resolver, silent = false): Promise { + public async updateNote(value: string | IObject, actor?: MiRemoteUser, resolver?: Resolver, silent = false): Promise { const noteUri = typeof value === 'string' ? value : value.id; if (noteUri == null) throw new Error('uri is null'); @@ -356,6 +384,9 @@ export class ApNoteService { const UpdatedNote = await this.notesRepository.findOneBy({ uri: noteUri }); if (UpdatedNote == null) throw new Error('Note is not registered'); + const user = await this.usersRepository.findOneBy({ id: UpdatedNote.userId }) as MiRemoteUser | null; + if (user == null) throw new Error('Note is not registered'); + // eslint-disable-next-line no-param-reassign if (resolver == null) resolver = this.apResolverService.createResolver(); @@ -372,6 +403,10 @@ export class ApNoteService { throw err; } + // `validateNote` checks that the actor and user are one and the same + // eslint-disable-next-line no-param-reassign + actor ??= user; + const note = object as IPost; this.logger.debug(`Note fetched: ${JSON.stringify(note, null, 2)}`); @@ -402,16 +437,7 @@ export class ApNoteService { this.logger.info(`Creating the Note: ${note.id}`); - // 投稿者をフェッチ - if (note.attributedTo == null) { - throw new Error('invalid note.attributedTo: ' + note.attributedTo); - } - - const uri = getOneApId(note.attributedTo); - - // ローカルで投稿者を検索し、もし凍結されていたらスキップ - const cachedActor = await this.apPersonService.fetchPerson(uri) as MiRemoteUser; - if (cachedActor && cachedActor.isSuspended) { + if (actor.isSuspended) { throw new IdentifiableError('85ab9bd7-3a41-4530-959d-f07073900109', 'actor has been suspended'); } @@ -443,13 +469,6 @@ export class ApNoteService { } //#endregion - const actor = cachedActor ?? await this.apPersonService.resolvePerson(uri, resolver) as MiRemoteUser; - - // 投稿者が凍結されていたらスキップ - if (actor.isSuspended) { - throw new IdentifiableError('85ab9bd7-3a41-4530-959d-f07073900109', 'actor has been suspended'); - } - const noteAudience = await this.apAudienceService.parseAudience(actor, note.to, note.cc, resolver); let visibility = noteAudience.visibility; const visibleUsers = noteAudience.visibleUsers; @@ -610,7 +629,7 @@ export class ApNoteService { // ここでuriの代わりに添付されてきたNote Objectが指定されていると、サーバーフェッチを経ずにノートが生成されるが // 添付されてきたNote Objectは偽装されている可能性があるため、常にuriを指定してサーバーフェッチを行う。 const createFrom = options.sentFrom?.origin === new URL(uri).origin ? value : uri; - return await this.createNote(createFrom, options.resolver, true); + return await this.createNote(createFrom, undefined, options.resolver, true); } finally { unlock(); } diff --git a/packages/backend/src/server/api/endpoints/ap/show.ts b/packages/backend/src/server/api/endpoints/ap/show.ts index a877d1ce0d..4232bc6e39 100644 --- a/packages/backend/src/server/api/endpoints/ap/show.ts +++ b/packages/backend/src/server/api/endpoints/ap/show.ts @@ -140,7 +140,7 @@ export default class extends Endpoint { // eslint- return await this.mergePack( me, isActor(object) ? await this.apPersonService.createPerson(getApId(object)) : null, - isPost(object) ? await this.apNoteService.createNote(getApId(object), undefined, true) : null, + isPost(object) ? await this.apNoteService.createNote(getApId(object), undefined, undefined, true) : null, ); } From cbf8cc376e02e457a96d680dbbf0c110137d55f5 Mon Sep 17 00:00:00 2001 From: Julia Johannesen Date: Thu, 14 Nov 2024 21:23:27 -0500 Subject: [PATCH 30/66] fix: primitive 18: `ap/get` bypasses access checks One might argue that we could make this one actually preform access checks against the returned activity object, but I feel like that's a lot more work than just restricting it to administrators, since, to me at least, it seems more like a debugging tool than anything else. --- packages/backend/src/server/api/endpoints/ap/get.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/backend/src/server/api/endpoints/ap/get.ts b/packages/backend/src/server/api/endpoints/ap/get.ts index d8c55de7ec..14286bc23e 100644 --- a/packages/backend/src/server/api/endpoints/ap/get.ts +++ b/packages/backend/src/server/api/endpoints/ap/get.ts @@ -11,6 +11,7 @@ import { ApResolverService } from '@/core/activitypub/ApResolverService.js'; export const meta = { tags: ['federation'], + requireAdmin: true, requireCredential: true, kind: 'read:federation', From 408e782507837da4c9b2164266d6f6f3e48d1642 Mon Sep 17 00:00:00 2001 From: Julia Johannesen Date: Thu, 14 Nov 2024 21:38:17 -0500 Subject: [PATCH 31/66] fix: primitive 19 & 20: respect blocks and hide more Ideally, the user property should also be hidden (as leaving it in leaks information slightly), but given the schema of the note endpoint, I don't think that would be possible without introducing some kind of "ghost" user, who is attributed for posts by users who have you blocked. --- .../src/core/entities/NoteEntityService.ts | 43 ++++++++++++++++++- 1 file changed, 41 insertions(+), 2 deletions(-) diff --git a/packages/backend/src/core/entities/NoteEntityService.ts b/packages/backend/src/core/entities/NoteEntityService.ts index 4dd17c5af3..2855ae78f7 100644 --- a/packages/backend/src/core/entities/NoteEntityService.ts +++ b/packages/backend/src/core/entities/NoteEntityService.ts @@ -11,7 +11,7 @@ import type { Packed } from '@/misc/json-schema.js'; import { awaitAll } from '@/misc/prelude/await-all.js'; import type { MiUser } from '@/models/User.js'; import type { MiNote } from '@/models/Note.js'; -import type { UsersRepository, NotesRepository, FollowingsRepository, PollsRepository, PollVotesRepository, NoteReactionsRepository, ChannelsRepository, MiMeta } from '@/models/_.js'; +import type { BlockingsRepository, UsersRepository, NotesRepository, FollowingsRepository, PollsRepository, PollVotesRepository, NoteReactionsRepository, ChannelsRepository, MiMeta } from '@/models/_.js'; import { bindThis } from '@/decorators.js'; import { DebounceLoader } from '@/misc/loader.js'; import { IdService } from '@/core/IdService.js'; @@ -39,6 +39,9 @@ export class NoteEntityService implements OnModuleInit { @Inject(DI.meta) private meta: MiMeta, + @Inject(DI.blockingsRepository) + private blockingsRepository: BlockingsRepository, + @Inject(DI.usersRepository) private usersRepository: UsersRepository, @@ -142,6 +145,17 @@ export class NoteEntityService implements OnModuleInit { } } + if (!hide && meId && packedNote.userId !== meId) { + const isBlocked = await this.blockingsRepository.exists({ + where: { + blockeeId: meId, + blockerId: packedNote.userId, + }, + }); + + if (isBlocked) hide = true; + } + if (hide) { packedNote.visibleUserIds = undefined; packedNote.fileIds = []; @@ -149,6 +163,12 @@ export class NoteEntityService implements OnModuleInit { packedNote.text = null; packedNote.poll = undefined; packedNote.cw = null; + packedNote.repliesCount = 0; + packedNote.reactionAcceptance = null; + packedNote.reactionAndUserPairCache = undefined; + packedNote.reactionCount = 0; + packedNote.reactionEmojis = undefined; + packedNote.reactions = undefined; packedNote.isHidden = true; } } @@ -262,7 +282,13 @@ export class NoteEntityService implements OnModuleInit { return true; } else { // フォロワーかどうか - const [following, user] = await Promise.all([ + const [blocked, following, user] = await Promise.all([ + this.blockingsRepository.exists({ + where: { + blockeeId: meId, + blockerId: note.userId, + }, + }), this.followingsRepository.count({ where: { followeeId: note.userId, @@ -273,6 +299,8 @@ export class NoteEntityService implements OnModuleInit { this.usersRepository.findOneByOrFail({ id: meId }), ]); + if (blocked) return false; + /* If we know the following, everyhting is fine. But if we do not know the following, it might be that both the @@ -284,6 +312,17 @@ export class NoteEntityService implements OnModuleInit { } } + if (meId != null) { + const isBlocked = await this.blockingsRepository.exists({ + where: { + blockeeId: meId, + blockerId: note.userId, + }, + }); + + if (isBlocked) return false; + } + return true; } From 74565f67f77085c2885ece50e6a79b50548029df Mon Sep 17 00:00:00 2001 From: Julia Johannesen Date: Thu, 14 Nov 2024 21:53:16 -0500 Subject: [PATCH 32/66] fix: primitives 21, 22, and 23: reuse resolver This also increases the default `recursionLimit` for `Resolver`, as it theoretically will go higher that it previously would and could possibly fail on non-malicious collection activities. --- .../src/core/activitypub/ApInboxService.ts | 85 +++++++++++-------- .../src/core/activitypub/ApResolverService.ts | 7 +- 2 files changed, 55 insertions(+), 37 deletions(-) diff --git a/packages/backend/src/core/activitypub/ApInboxService.ts b/packages/backend/src/core/activitypub/ApInboxService.ts index 0f56ad3f78..90444a1af3 100644 --- a/packages/backend/src/core/activitypub/ApInboxService.ts +++ b/packages/backend/src/core/activitypub/ApInboxService.ts @@ -93,19 +93,26 @@ export class ApInboxService { } @bindThis - public async performActivity(actor: MiRemoteUser, activity: IObject): Promise { + public async performActivity(actor: MiRemoteUser, activity: IObject, resolver?: Resolver): Promise { let result = undefined as string | void; if (isCollectionOrOrderedCollection(activity)) { const results = [] as [string, string | void][]; - const resolver = this.apResolverService.createResolver(); - for (const item of toArray(isCollection(activity) ? activity.items : activity.orderedItems)) { + // eslint-disable-next-line no-param-reassign + resolver ??= this.apResolverService.createResolver(); + + const items = toArray(isCollection(activity) ? activity.items : activity.orderedItems); + if (items.length >= resolver.getRecursionLimit()) { + throw new Error(`skipping activity: collection would surpass recursion limit: ${this.utilityService.extractDbHost(actor.uri)}`); + } + + for (const item of items) { const act = await resolver.resolve(item); if (act.id == null || this.utilityService.extractDbHost(act.id) !== this.utilityService.extractDbHost(actor.uri)) { this.logger.debug('skipping activity: activity id is null or mismatching'); continue; } try { - results.push([getApId(item), await this.performOneActivity(actor, act)]); + results.push([getApId(item), await this.performOneActivity(actor, act, resolver)]); } catch (err) { if (err instanceof Error || typeof err === 'string') { this.logger.error(err); @@ -120,7 +127,7 @@ export class ApInboxService { result = results.map(([id, reason]) => `${id}: ${reason}`).join('\n'); } } else { - result = await this.performOneActivity(actor, activity); + result = await this.performOneActivity(actor, activity, resolver); } // ついでにリモートユーザーの情報が古かったら更新しておく @@ -135,37 +142,37 @@ export class ApInboxService { } @bindThis - public async performOneActivity(actor: MiRemoteUser, activity: IObject): Promise { + public async performOneActivity(actor: MiRemoteUser, activity: IObject, resolver?: Resolver): Promise { if (actor.isSuspended) return; if (isCreate(activity)) { - return await this.create(actor, activity); + return await this.create(actor, activity, resolver); } else if (isDelete(activity)) { return await this.delete(actor, activity); } else if (isUpdate(activity)) { - return await this.update(actor, activity); + return await this.update(actor, activity, resolver); } else if (isFollow(activity)) { return await this.follow(actor, activity); } else if (isAccept(activity)) { - return await this.accept(actor, activity); + return await this.accept(actor, activity, resolver); } else if (isReject(activity)) { - return await this.reject(actor, activity); + return await this.reject(actor, activity, resolver); } else if (isAdd(activity)) { - return await this.add(actor, activity); + return await this.add(actor, activity, resolver); } else if (isRemove(activity)) { - return await this.remove(actor, activity); + return await this.remove(actor, activity, resolver); } else if (isAnnounce(activity)) { - return await this.announce(actor, activity); + return await this.announce(actor, activity, resolver); } else if (isLike(activity)) { return await this.like(actor, activity); } else if (isUndo(activity)) { - return await this.undo(actor, activity); + return await this.undo(actor, activity, resolver); } else if (isBlock(activity)) { return await this.block(actor, activity); } else if (isFlag(activity)) { return await this.flag(actor, activity); } else if (isMove(activity)) { - return await this.move(actor, activity); + return await this.move(actor, activity, resolver); } else { return `unrecognized activity type: ${activity.type}`; } @@ -207,12 +214,13 @@ export class ApInboxService { } @bindThis - private async accept(actor: MiRemoteUser, activity: IAccept): Promise { + private async accept(actor: MiRemoteUser, activity: IAccept, resolver?: Resolver): Promise { const uri = activity.id ?? activity; this.logger.info(`Accept: ${uri}`); - const resolver = this.apResolverService.createResolver(); + // eslint-disable-next-line no-param-reassign + resolver ??= this.apResolverService.createResolver(); const object = await resolver.resolve(activity.object).catch(err => { this.logger.error(`Resolution failed: ${err}`); @@ -249,7 +257,7 @@ export class ApInboxService { } @bindThis - private async add(actor: MiRemoteUser, activity: IAdd): Promise { + private async add(actor: MiRemoteUser, activity: IAdd, resolver?: Resolver): Promise { if (actor.uri !== activity.actor) { return 'invalid actor'; } @@ -260,7 +268,7 @@ export class ApInboxService { if (activity.target === actor.featured) { const object = fromTuple(activity.object); - const note = await this.apNoteService.resolveNote(object); + const note = await this.apNoteService.resolveNote(object, { resolver }); if (note == null) return 'note not found'; await this.notePiningService.addPinned(actor, note.id); return; @@ -270,12 +278,13 @@ export class ApInboxService { } @bindThis - private async announce(actor: MiRemoteUser, activity: IAnnounce): Promise { + private async announce(actor: MiRemoteUser, activity: IAnnounce, resolver?: Resolver): Promise { const uri = getApId(activity); this.logger.info(`Announce: ${uri}`); - const resolver = this.apResolverService.createResolver(); + // eslint-disable-next-line no-param-reassign + resolver ??= this.apResolverService.createResolver(); const activityObject = fromTuple(activity.object); if (!activityObject) return 'skip: activity has no object property'; @@ -293,7 +302,7 @@ export class ApInboxService { } @bindThis - private async announceNote(actor: MiRemoteUser, activity: IAnnounce, target: IPost): Promise { + private async announceNote(actor: MiRemoteUser, activity: IAnnounce, target: IPost, resolver?: Resolver): Promise { const uri = getApId(activity); if (actor.isSuspended) { @@ -315,7 +324,7 @@ export class ApInboxService { // Announce対象をresolve let renote; try { - renote = await this.apNoteService.resolveNote(target); + renote = await this.apNoteService.resolveNote(target, { resolver }); if (renote == null) return 'announce target is null'; } catch (err) { // 対象が4xxならスキップ @@ -334,7 +343,7 @@ export class ApInboxService { this.logger.info(`Creating the (Re)Note: ${uri}`); - const activityAudience = await this.apAudienceService.parseAudience(actor, activity.to, activity.cc); + const activityAudience = await this.apAudienceService.parseAudience(actor, activity.to, activity.cc, resolver); const createdAt = activity.published ? new Date(activity.published) : null; if (createdAt && createdAt < this.idService.parse(renote.id).date) { @@ -372,7 +381,7 @@ export class ApInboxService { } @bindThis - private async create(actor: MiRemoteUser, activity: ICreate): Promise { + private async create(actor: MiRemoteUser, activity: ICreate, resolver?: Resolver): Promise { const uri = getApId(activity); this.logger.info(`Create: ${uri}`); @@ -398,7 +407,8 @@ export class ApInboxService { activityObject.attributedTo = activity.actor; } - const resolver = this.apResolverService.createResolver(); + // eslint-disable-next-line no-param-reassign + resolver ??= this.apResolverService.createResolver(); const object = await resolver.resolve(activityObject).catch(e => { this.logger.error(`Resolution failed: ${e}`); @@ -574,12 +584,13 @@ export class ApInboxService { } @bindThis - private async reject(actor: MiRemoteUser, activity: IReject): Promise { + private async reject(actor: MiRemoteUser, activity: IReject, resolver?: Resolver): Promise { const uri = activity.id ?? activity; this.logger.info(`Reject: ${uri}`); - const resolver = this.apResolverService.createResolver(); + // eslint-disable-next-line no-param-reassign + resolver ??= this.apResolverService.createResolver(); const object = await resolver.resolve(activity.object).catch(e => { this.logger.error(`Resolution failed: ${e}`); @@ -616,7 +627,7 @@ export class ApInboxService { } @bindThis - private async remove(actor: MiRemoteUser, activity: IRemove): Promise { + private async remove(actor: MiRemoteUser, activity: IRemove, resolver?: Resolver): Promise { if (actor.uri !== activity.actor) { return 'invalid actor'; } @@ -627,7 +638,7 @@ export class ApInboxService { if (activity.target === actor.featured) { const activityObject = fromTuple(activity.object); - const note = await this.apNoteService.resolveNote(activityObject); + const note = await this.apNoteService.resolveNote(activityObject, { resolver }); if (note == null) return 'note not found'; await this.notePiningService.removePinned(actor, note.id); return; @@ -637,7 +648,7 @@ export class ApInboxService { } @bindThis - private async undo(actor: MiRemoteUser, activity: IUndo): Promise { + private async undo(actor: MiRemoteUser, activity: IUndo, resolver?: Resolver): Promise { if (actor.uri !== activity.actor) { return 'invalid actor'; } @@ -646,7 +657,8 @@ export class ApInboxService { this.logger.info(`Undo: ${uri}`); - const resolver = this.apResolverService.createResolver(); + // eslint-disable-next-line no-param-reassign + resolver ??= this.apResolverService.createResolver(); const object = await resolver.resolve(activity.object).catch(e => { this.logger.error(`Resolution failed: ${e}`); @@ -770,14 +782,15 @@ export class ApInboxService { } @bindThis - private async update(actor: MiRemoteUser, activity: IUpdate): Promise { + private async update(actor: MiRemoteUser, activity: IUpdate, resolver?: Resolver): Promise { if (actor.uri !== activity.actor) { return 'skip: invalid actor'; } this.logger.debug('Update'); - const resolver = this.apResolverService.createResolver(); + // eslint-disable-next-line no-param-reassign + resolver ??= this.apResolverService.createResolver(); const object = await resolver.resolve(activity.object).catch(e => { this.logger.error(`Resolution failed: ${e}`); @@ -799,11 +812,11 @@ export class ApInboxService { } @bindThis - private async move(actor: MiRemoteUser, activity: IMove): Promise { + private async move(actor: MiRemoteUser, activity: IMove, resolver?: Resolver): Promise { // fetch the new and old accounts const targetUri = getApHrefNullable(activity.target); if (!targetUri) return 'skip: invalid activity target'; - return await this.apPersonService.updatePerson(actor.uri) ?? 'skip: nothing to do'; + return await this.apPersonService.updatePerson(actor.uri, resolver) ?? 'skip: nothing to do'; } } diff --git a/packages/backend/src/core/activitypub/ApResolverService.ts b/packages/backend/src/core/activitypub/ApResolverService.ts index a2c7ed19d8..25ccbdac60 100644 --- a/packages/backend/src/core/activitypub/ApResolverService.ts +++ b/packages/backend/src/core/activitypub/ApResolverService.ts @@ -42,7 +42,7 @@ export class Resolver { private apRendererService: ApRendererService, private apDbResolverService: ApDbResolverService, private loggerService: LoggerService, - private recursionLimit = 100, + private recursionLimit = 256, ) { this.history = new Set(); this.logger = this.loggerService.getLogger('ap-resolve'); @@ -53,6 +53,11 @@ export class Resolver { return Array.from(this.history); } + @bindThis + public getRecursionLimit(): number { + return this.recursionLimit; + } + @bindThis public async resolveCollection(value: string | IObject): Promise { const collection = typeof value === 'string' From 5764fa55cb7cb404fed3d029b658c495ec52ecaf Mon Sep 17 00:00:00 2001 From: Julia Johannesen Date: Thu, 14 Nov 2024 22:01:22 -0500 Subject: [PATCH 33/66] fix: primitives 25-33: proper local instance checks --- packages/backend/src/core/RemoteUserResolveService.ts | 4 ++-- packages/backend/src/core/UtilityService.ts | 5 +++++ .../backend/src/core/activitypub/ApDbResolverService.ts | 6 +++++- .../backend/src/core/activitypub/models/ApNoteService.ts | 2 +- .../src/core/activitypub/models/ApPersonService.ts | 9 ++++----- .../src/core/activitypub/models/ApQuestionService.ts | 4 +++- 6 files changed, 20 insertions(+), 10 deletions(-) diff --git a/packages/backend/src/core/RemoteUserResolveService.ts b/packages/backend/src/core/RemoteUserResolveService.ts index f5a55eb8bc..678da0cfa6 100644 --- a/packages/backend/src/core/RemoteUserResolveService.ts +++ b/packages/backend/src/core/RemoteUserResolveService.ts @@ -54,9 +54,9 @@ export class RemoteUserResolveService { }) as MiLocalUser; } - host = this.utilityService.toPuny(host); + host = this.utilityService.punyHost(host); - if (this.config.host === host) { + if (host === this.utilityService.toPuny(this.config.host)) { this.logger.info(`return local user: ${usernameLower}`); return await this.usersRepository.findOneBy({ usernameLower, host: IsNull() }).then(u => { if (u == null) { diff --git a/packages/backend/src/core/UtilityService.ts b/packages/backend/src/core/UtilityService.ts index 009dd4665f..4c6d539e16 100644 --- a/packages/backend/src/core/UtilityService.ts +++ b/packages/backend/src/core/UtilityService.ts @@ -34,6 +34,11 @@ export class UtilityService { return this.toPuny(this.config.host) === this.toPuny(host); } + @bindThis + public isUriLocal(uri: string): boolean { + return this.punyHost(uri) === this.toPuny(this.config.host); + } + @bindThis public isBlockedHost(blockedHosts: string[], host: string | null): boolean { if (host == null) return false; diff --git a/packages/backend/src/core/activitypub/ApDbResolverService.ts b/packages/backend/src/core/activitypub/ApDbResolverService.ts index 8c97cc8ce8..dd89716d34 100644 --- a/packages/backend/src/core/activitypub/ApDbResolverService.ts +++ b/packages/backend/src/core/activitypub/ApDbResolverService.ts @@ -10,6 +10,7 @@ import type { Config } from '@/config.js'; import { MemoryKVCache } from '@/misc/cache.js'; import type { MiUserPublickey } from '@/models/UserPublickey.js'; import { CacheService } from '@/core/CacheService.js'; +import { UtilityService } from '@/core/UtilityService.js'; import type { MiNote } from '@/models/Note.js'; import { bindThis } from '@/decorators.js'; import type { MiLocalUser, MiRemoteUser } from '@/models/User.js'; @@ -55,6 +56,7 @@ export class ApDbResolverService implements OnApplicationShutdown { private cacheService: CacheService, private apPersonService: ApPersonService, private apLoggerService: ApLoggerService, + private utilityService: UtilityService, ) { this.publicKeyCache = new MemoryKVCache(1000 * 60 * 60 * 12); // 12h this.publicKeyByUserIdCache = new MemoryKVCache(1000 * 60 * 60 * 12); // 12h @@ -65,7 +67,9 @@ export class ApDbResolverService implements OnApplicationShutdown { const separator = '/'; const uri = new URL(getApId(value)); - if (uri.origin !== this.config.url) return { local: false, uri: uri.href }; + if (this.utilityService.toPuny(uri.host) !== this.utilityService.toPuny(this.config.host)) { + return { local: false, uri: uri.href }; + } const [, type, id, ...rest] = uri.pathname.split(separator); return { diff --git a/packages/backend/src/core/activitypub/models/ApNoteService.ts b/packages/backend/src/core/activitypub/models/ApNoteService.ts index 7857bcc28c..a0ddc2075b 100644 --- a/packages/backend/src/core/activitypub/models/ApNoteService.ts +++ b/packages/backend/src/core/activitypub/models/ApNoteService.ts @@ -621,7 +621,7 @@ export class ApNoteService { if (exist) return exist; //#endregion - if (uri.startsWith(this.config.url)) { + if (this.utilityService.isUriLocal(uri)) { throw new StatusError('cannot resolve local note', 400, 'cannot resolve local note'); } diff --git a/packages/backend/src/core/activitypub/models/ApPersonService.ts b/packages/backend/src/core/activitypub/models/ApPersonService.ts index 7a3bd57d43..1c117795e9 100644 --- a/packages/backend/src/core/activitypub/models/ApPersonService.ts +++ b/packages/backend/src/core/activitypub/models/ApPersonService.ts @@ -296,7 +296,8 @@ export class ApPersonService implements OnModuleInit { public async createPerson(uri: string, resolver?: Resolver): Promise { if (typeof uri !== 'string') throw new Error('uri is not string'); - if (uri.startsWith(this.config.url)) { + const host = this.utilityService.punyHost(uri); + if (host === this.utilityService.toPuny(this.config.host)) { throw new StatusError('cannot resolve local user', 400, 'cannot resolve local user'); } @@ -310,8 +311,6 @@ export class ApPersonService implements OnModuleInit { this.logger.info(`Creating the Person: ${person.id}`); - const host = this.utilityService.punyHost(object.id); - const fields = this.analyzeAttachments(person.attachment ?? []); const tags = extractApHashtags(person.tag).map(normalizeForSearch).splice(0, 32); @@ -500,7 +499,7 @@ export class ApPersonService implements OnModuleInit { if (typeof uri !== 'string') throw new Error('uri is not string'); // URIがこのサーバーを指しているならスキップ - if (uri.startsWith(`${this.config.url}/`)) return; + if (this.utilityService.isUriLocal(uri)) return; //#region このサーバーに既に登録されているか const exist = await this.fetchPerson(uri) as MiRemoteUser | null; @@ -777,7 +776,7 @@ export class ApPersonService implements OnModuleInit { await this.updatePerson(src.movedToUri, undefined, undefined, [...movePreventUris, src.uri]); dst = await this.fetchPerson(src.movedToUri) ?? dst; } else { - if (src.movedToUri.startsWith(`${this.config.url}/`)) { + if (this.utilityService.isUriLocal(src.movedToUri)) { // ローカルユーザーっぽいのにfetchPersonで見つからないということはmovedToUriが間違っている return 'failed: movedTo is local but not found'; } diff --git a/packages/backend/src/core/activitypub/models/ApQuestionService.ts b/packages/backend/src/core/activitypub/models/ApQuestionService.ts index c1aea15ece..83a98d17f9 100644 --- a/packages/backend/src/core/activitypub/models/ApQuestionService.ts +++ b/packages/backend/src/core/activitypub/models/ApQuestionService.ts @@ -11,6 +11,7 @@ import type { IPoll } from '@/models/Poll.js'; import type { MiRemoteUser } from '@/models/User.js'; import type Logger from '@/logger.js'; import { bindThis } from '@/decorators.js'; +import { UtilityService } from '@/core/UtilityService.js'; import { getOneApId, isQuestion } from '../type.js'; import { ApLoggerService } from '../ApLoggerService.js'; import { ApResolverService } from '../ApResolverService.js'; @@ -36,6 +37,7 @@ export class ApQuestionService { private apResolverService: ApResolverService, private apLoggerService: ApLoggerService, + private utilityService: UtilityService, ) { this.logger = this.apLoggerService.logger; } @@ -74,7 +76,7 @@ export class ApQuestionService { if (uri == null) throw new Error('uri is null'); // URIがこのサーバーを指しているならスキップ - if (uri.startsWith(this.config.url + '/')) throw new Error('uri points local'); + if (this.utilityService.isUriLocal(uri)) throw new Error('uri points local'); //#region このサーバーに既に登録されているか const note = await this.notesRepository.findOneBy({ uri }); From cc4e99fdde690cea82c45f0ed9595b78810a1630 Mon Sep 17 00:00:00 2001 From: Julia Johannesen Date: Thu, 14 Nov 2024 23:43:19 -0500 Subject: [PATCH 34/66] fix: Try using `CacheService` to avoid excess db lookups This isn't perfect, theoretically if some massive number of users blocked the user making this request the set lookup could take a long amount of time, but eh, it works, and that scenario is highly unlikely. --- .../src/core/entities/NoteEntityService.ts | 29 +++++-------------- 1 file changed, 7 insertions(+), 22 deletions(-) diff --git a/packages/backend/src/core/entities/NoteEntityService.ts b/packages/backend/src/core/entities/NoteEntityService.ts index 2855ae78f7..985245aeb1 100644 --- a/packages/backend/src/core/entities/NoteEntityService.ts +++ b/packages/backend/src/core/entities/NoteEntityService.ts @@ -11,12 +11,13 @@ import type { Packed } from '@/misc/json-schema.js'; import { awaitAll } from '@/misc/prelude/await-all.js'; import type { MiUser } from '@/models/User.js'; import type { MiNote } from '@/models/Note.js'; -import type { BlockingsRepository, UsersRepository, NotesRepository, FollowingsRepository, PollsRepository, PollVotesRepository, NoteReactionsRepository, ChannelsRepository, MiMeta } from '@/models/_.js'; +import type { UsersRepository, NotesRepository, FollowingsRepository, PollsRepository, PollVotesRepository, NoteReactionsRepository, ChannelsRepository, MiMeta } from '@/models/_.js'; import { bindThis } from '@/decorators.js'; import { DebounceLoader } from '@/misc/loader.js'; import { IdService } from '@/core/IdService.js'; import { ReactionsBufferingService } from '@/core/ReactionsBufferingService.js'; import type { OnModuleInit } from '@nestjs/common'; +import type { CacheService } from '../CacheService.js'; import type { CustomEmojiService } from '../CustomEmojiService.js'; import type { ReactionService } from '../ReactionService.js'; import type { UserEntityService } from './UserEntityService.js'; @@ -27,6 +28,7 @@ import type { Config } from '@/config.js'; export class NoteEntityService implements OnModuleInit { private userEntityService: UserEntityService; private driveFileEntityService: DriveFileEntityService; + private cacheService: CacheService; private customEmojiService: CustomEmojiService; private reactionService: ReactionService; private reactionsBufferingService: ReactionsBufferingService; @@ -39,9 +41,6 @@ export class NoteEntityService implements OnModuleInit { @Inject(DI.meta) private meta: MiMeta, - @Inject(DI.blockingsRepository) - private blockingsRepository: BlockingsRepository, - @Inject(DI.usersRepository) private usersRepository: UsersRepository, @@ -78,6 +77,7 @@ export class NoteEntityService implements OnModuleInit { onModuleInit() { this.userEntityService = this.moduleRef.get('UserEntityService'); this.driveFileEntityService = this.moduleRef.get('DriveFileEntityService'); + this.cacheService = this.moduleRef.get('CacheService'); this.customEmojiService = this.moduleRef.get('CustomEmojiService'); this.reactionService = this.moduleRef.get('ReactionService'); this.reactionsBufferingService = this.moduleRef.get('ReactionsBufferingService'); @@ -146,12 +146,7 @@ export class NoteEntityService implements OnModuleInit { } if (!hide && meId && packedNote.userId !== meId) { - const isBlocked = await this.blockingsRepository.exists({ - where: { - blockeeId: meId, - blockerId: packedNote.userId, - }, - }); + const isBlocked = (await this.cacheService.userBlockedCache.fetch(meId)).has(packedNote.userId); if (isBlocked) hide = true; } @@ -283,12 +278,7 @@ export class NoteEntityService implements OnModuleInit { } else { // フォロワーかどうか const [blocked, following, user] = await Promise.all([ - this.blockingsRepository.exists({ - where: { - blockeeId: meId, - blockerId: note.userId, - }, - }), + this.cacheService.userBlockingCache.fetch(meId).then((ids) => ids.has(note.userId)), this.followingsRepository.count({ where: { followeeId: note.userId, @@ -313,12 +303,7 @@ export class NoteEntityService implements OnModuleInit { } if (meId != null) { - const isBlocked = await this.blockingsRepository.exists({ - where: { - blockeeId: meId, - blockerId: note.userId, - }, - }); + const isBlocked = (await this.cacheService.userBlockedCache.fetch(meId)).has(note.userId); if (isBlocked) return false; } From f36f4b5398561dcf7365729c530f5b1868c6b994 Mon Sep 17 00:00:00 2001 From: rectcoordsystem Date: Wed, 6 Nov 2024 05:31:11 +0900 Subject: [PATCH 35/66] fix(backend): check target IP before sending HTTP request --- packages/backend/src/core/DownloadService.ts | 22 ----- .../backend/src/core/HttpRequestService.ts | 90 ++++++++++++++++++- 2 files changed, 88 insertions(+), 24 deletions(-) diff --git a/packages/backend/src/core/DownloadService.ts b/packages/backend/src/core/DownloadService.ts index 0e992f05de..05b9e64a37 100644 --- a/packages/backend/src/core/DownloadService.ts +++ b/packages/backend/src/core/DownloadService.ts @@ -6,7 +6,6 @@ import * as fs from 'node:fs'; import * as stream from 'node:stream/promises'; import { Inject, Injectable } from '@nestjs/common'; -import ipaddr from 'ipaddr.js'; import chalk from 'chalk'; import got, * as Got from 'got'; import { parse } from 'content-disposition'; @@ -70,13 +69,6 @@ export class DownloadService { }, enableUnixSockets: false, }).on('response', (res: Got.Response) => { - if ((process.env.NODE_ENV === 'production' || process.env.NODE_ENV === 'test') && !this.config.proxy && res.ip) { - if (this.isPrivateIp(res.ip)) { - this.logger.warn(`Blocked address: ${res.ip}`); - req.destroy(); - } - } - const contentLength = res.headers['content-length']; if (contentLength != null) { const size = Number(contentLength); @@ -139,18 +131,4 @@ export class DownloadService { cleanup(); } } - - @bindThis - private isPrivateIp(ip: string): boolean { - const parsedIp = ipaddr.parse(ip); - - for (const net of this.config.allowedPrivateNetworks ?? []) { - const cidr = ipaddr.parseCIDR(net); - if (cidr[0].kind() === parsedIp.kind() && parsedIp.match(ipaddr.parseCIDR(net))) { - return false; - } - } - - return parsedIp.range() !== 'unicast'; - } } diff --git a/packages/backend/src/core/HttpRequestService.ts b/packages/backend/src/core/HttpRequestService.ts index 08e9f46b2d..6c013eacc0 100644 --- a/packages/backend/src/core/HttpRequestService.ts +++ b/packages/backend/src/core/HttpRequestService.ts @@ -6,6 +6,7 @@ import * as http from 'node:http'; import * as https from 'node:https'; import * as net from 'node:net'; +import ipaddr from 'ipaddr.js'; import CacheableLookup from 'cacheable-lookup'; import fetch from 'node-fetch'; import { HttpProxyAgent, HttpsProxyAgent } from 'hpagent'; @@ -25,6 +26,91 @@ export type HttpRequestSendOptions = { validators?: ((res: Response) => void)[]; }; +@Injectable() +class HttpRequestServiceAgent extends http.Agent { + constructor( + @Inject(DI.config) + private config: Config, + + options?: Object + ) { + super(options); + } + + @bindThis + public createConnection(options: Object, callback?: Function): net.Socket { + const socket = super.createConnection(options, callback) + .on('connect', ()=>{ + const address = socket.remoteAddress; + if (process.env.NODE_ENV === 'production' || process.env.NODE_ENV === 'test') { + if (address && ipaddr.isValid(address)) { + if (this.isPrivateIp(address)) { + socket.destroy(new Error(`Blocked address: ${address}`)); + } + } + } + }); + return socket; + }; + + @bindThis + private isPrivateIp(ip: string): boolean { + const parsedIp = ipaddr.parse(ip); + + for (const net of this.config.allowedPrivateNetworks ?? []) { + const cidr = ipaddr.parseCIDR(net); + if (cidr[0].kind() === parsedIp.kind() && parsedIp.match(ipaddr.parseCIDR(net))) { + return false; + } + } + + return parsedIp.range() !== 'unicast'; + } +} + +@Injectable() +class HttpsRequestServiceAgent extends https.Agent { + constructor( + @Inject(DI.config) + private config: Config, + + options?: Object + ) { + super(options); + } + + @bindThis + public createConnection(options: Object, callback?: Function): net.Socket { + const socket = super.createConnection(options, callback) + .on('connect', ()=>{ + const address = socket.remoteAddress; + if (process.env.NODE_ENV === 'production' || process.env.NODE_ENV === 'test') { + if (address && ipaddr.isValid(address)) { + if (this.isPrivateIp(address)) { + socket.destroy(new Error(`Blocked address: ${address}`)); + } + } + } + }); + return socket; + }; + + @bindThis + private isPrivateIp(ip: string): boolean { + const parsedIp = ipaddr.parse(ip); + + for (const net of this.config.allowedPrivateNetworks ?? []) { + const cidr = ipaddr.parseCIDR(net); + if (cidr[0].kind() === parsedIp.kind() && parsedIp.match(ipaddr.parseCIDR(net))) { + return false; + } + } + + return parsedIp.range() !== 'unicast'; + } +} + + @Injectable() export class HttpRequestService { /** @@ -57,14 +143,14 @@ export class HttpRequestService { lookup: false, // nativeのdns.lookupにfallbackしない }); - this.http = new http.Agent({ + this.http = new HttpRequestServiceAgent(config, { keepAlive: true, keepAliveMsecs: 30 * 1000, lookup: cache.lookup as unknown as net.LookupFunction, localAddress: config.outgoingAddress, }); - this.https = new https.Agent({ + this.https = new HttpsRequestServiceAgent(config, { keepAlive: true, keepAliveMsecs: 30 * 1000, lookup: cache.lookup as unknown as net.LookupFunction, From 7ccccf5545c1292c8de8b24bc426b1f9430528ef Mon Sep 17 00:00:00 2001 From: rectcoordsystem Date: Wed, 6 Nov 2024 06:33:44 +0900 Subject: [PATCH 36/66] fix(backend): allow accessing private IP when testing --- packages/backend/src/core/HttpRequestService.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/backend/src/core/HttpRequestService.ts b/packages/backend/src/core/HttpRequestService.ts index 6c013eacc0..8c5e3bdb91 100644 --- a/packages/backend/src/core/HttpRequestService.ts +++ b/packages/backend/src/core/HttpRequestService.ts @@ -42,7 +42,7 @@ class HttpRequestServiceAgent extends http.Agent { const socket = super.createConnection(options, callback) .on('connect', ()=>{ const address = socket.remoteAddress; - if (process.env.NODE_ENV === 'production' || process.env.NODE_ENV === 'test') { + if (process.env.NODE_ENV === 'production') { if (address && ipaddr.isValid(address)) { if (this.isPrivateIp(address)) { socket.destroy(new Error(`Blocked address: ${address}`)); @@ -84,7 +84,7 @@ class HttpsRequestServiceAgent extends https.Agent { const socket = super.createConnection(options, callback) .on('connect', ()=>{ const address = socket.remoteAddress; - if (process.env.NODE_ENV === 'production' || process.env.NODE_ENV === 'test') { + if (process.env.NODE_ENV === 'production') { if (address && ipaddr.isValid(address)) { if (this.isPrivateIp(address)) { socket.destroy(new Error(`Blocked address: ${address}`)); From 663c06be00d5b05d6c3e96559b170190805f38ed Mon Sep 17 00:00:00 2001 From: rectcoordsystem <37621004+rectcoordsystem@users.noreply.github.com> Date: Wed, 13 Nov 2024 03:06:22 +0900 Subject: [PATCH 37/66] Apply suggestions from code review Co-authored-by: anatawa12 --- .../backend/src/core/HttpRequestService.ts | 34 +++++++++---------- 1 file changed, 17 insertions(+), 17 deletions(-) diff --git a/packages/backend/src/core/HttpRequestService.ts b/packages/backend/src/core/HttpRequestService.ts index 8c5e3bdb91..0c8b5b4ef4 100644 --- a/packages/backend/src/core/HttpRequestService.ts +++ b/packages/backend/src/core/HttpRequestService.ts @@ -26,30 +26,33 @@ export type HttpRequestSendOptions = { validators?: ((res: Response) => void)[]; }; -@Injectable() +declare module 'node:http' { + interface Agent { + createConnection(options: net.NetConnectOpts, callback?: (err: unknown, stream: net.Socket) => void): net.Socket; + } +} + class HttpRequestServiceAgent extends http.Agent { constructor( - @Inject(DI.config) private config: Config, - - options?: Object + options?: http.AgentOptions, ) { super(options); } @bindThis - public createConnection(options: Object, callback?: Function): net.Socket { + public createConnection(options: net.NetConnectOpts, callback?: (err: unknown, stream: net.Socket) => void): net.Socket { const socket = super.createConnection(options, callback) - .on('connect', ()=>{ - const address = socket.remoteAddress; - if (process.env.NODE_ENV === 'production') { - if (address && ipaddr.isValid(address)) { - if (this.isPrivateIp(address)) { - socket.destroy(new Error(`Blocked address: ${address}`)); + .on('connect', () => { + const address = socket.remoteAddress; + if (process.env.NODE_ENV === 'production') { + if (address && ipaddr.isValid(address)) { + if (this.isPrivateIp(address)) { + socket.destroy(new Error(`Blocked address: ${address}`)); + } } } - } - }); + }); return socket; }; @@ -68,13 +71,10 @@ class HttpRequestServiceAgent extends http.Agent { } } -@Injectable() class HttpsRequestServiceAgent extends https.Agent { constructor( - @Inject(DI.config) private config: Config, - - options?: Object + options?: https.AgentOptions, ) { super(options); } From 360d71278a78169c2b2351bbcf518c1e1654b254 Mon Sep 17 00:00:00 2001 From: rectcoordsystem Date: Wed, 13 Nov 2024 03:27:52 +0900 Subject: [PATCH 38/66] fix(backend): lint and typecheck --- .../backend/src/core/HttpRequestService.ts | 19 +++++++++---------- 1 file changed, 9 insertions(+), 10 deletions(-) diff --git a/packages/backend/src/core/HttpRequestService.ts b/packages/backend/src/core/HttpRequestService.ts index 0c8b5b4ef4..0955d1f5bb 100644 --- a/packages/backend/src/core/HttpRequestService.ts +++ b/packages/backend/src/core/HttpRequestService.ts @@ -80,18 +80,18 @@ class HttpsRequestServiceAgent extends https.Agent { } @bindThis - public createConnection(options: Object, callback?: Function): net.Socket { + public createConnection(options: net.NetConnectOpts, callback?: (err: unknown, stream: net.Socket) => void): net.Socket { const socket = super.createConnection(options, callback) - .on('connect', ()=>{ - const address = socket.remoteAddress; - if (process.env.NODE_ENV === 'production') { - if (address && ipaddr.isValid(address)) { - if (this.isPrivateIp(address)) { - socket.destroy(new Error(`Blocked address: ${address}`)); + .on('connect', () => { + const address = socket.remoteAddress; + if (process.env.NODE_ENV === 'production') { + if (address && ipaddr.isValid(address)) { + if (this.isPrivateIp(address)) { + socket.destroy(new Error(`Blocked address: ${address}`)); + } } } - } - }); + }); return socket; }; @@ -110,7 +110,6 @@ class HttpsRequestServiceAgent extends https.Agent { } } - @Injectable() export class HttpRequestService { /** From 7b3e3f8e25a09430e250096ee01c0f913840623f Mon Sep 17 00:00:00 2001 From: rectcoordsystem Date: Wed, 13 Nov 2024 13:30:01 +0900 Subject: [PATCH 39/66] fix(backend): add isLocalAddressAllowed option to getAgentByUrl and send (HttpRequestService) --- .../backend/src/core/HttpRequestService.ts | 49 ++++++++++++++----- 1 file changed, 36 insertions(+), 13 deletions(-) diff --git a/packages/backend/src/core/HttpRequestService.ts b/packages/backend/src/core/HttpRequestService.ts index 0955d1f5bb..0ad5667049 100644 --- a/packages/backend/src/core/HttpRequestService.ts +++ b/packages/backend/src/core/HttpRequestService.ts @@ -112,6 +112,16 @@ class HttpsRequestServiceAgent extends https.Agent { @Injectable() export class HttpRequestService { + /** + * Get http non-proxy agent (without local address filtering) + */ + private httpNative: http.Agent; + + /** + * Get https non-proxy agent (without local address filtering) + */ + private httpsNative: https.Agent; + /** * Get http non-proxy agent */ @@ -142,19 +152,20 @@ export class HttpRequestService { lookup: false, // nativeのdns.lookupにfallbackしない }); - this.http = new HttpRequestServiceAgent(config, { + const agentOption = { keepAlive: true, keepAliveMsecs: 30 * 1000, lookup: cache.lookup as unknown as net.LookupFunction, localAddress: config.outgoingAddress, - }); + }; - this.https = new HttpsRequestServiceAgent(config, { - keepAlive: true, - keepAliveMsecs: 30 * 1000, - lookup: cache.lookup as unknown as net.LookupFunction, - localAddress: config.outgoingAddress, - }); + this.httpNative = new http.Agent(agentOption); + + this.httpsNative = new https.Agent(agentOption); + + this.http = new HttpRequestServiceAgent(config, agentOption); + + this.https = new HttpsRequestServiceAgent(config, agentOption); const maxSockets = Math.max(256, config.deliverJobConcurrency ?? 128); @@ -189,16 +200,22 @@ export class HttpRequestService { * @param bypassProxy Allways bypass proxy */ @bindThis - public getAgentByUrl(url: URL, bypassProxy = false): http.Agent | https.Agent { + public getAgentByUrl(url: URL, bypassProxy = false, isLocalAddressAllowed = false): http.Agent | https.Agent { if (bypassProxy || (this.config.proxyBypassHosts ?? []).includes(url.hostname)) { + if (isLocalAddressAllowed) { + return url.protocol === 'http:' ? this.httpNative : this.httpsNative; + } return url.protocol === 'http:' ? this.http : this.https; } else { + if (isLocalAddressAllowed && (!this.config.proxy)) { + return url.protocol === 'http:' ? this.httpNative : this.httpsNative; + } return url.protocol === 'http:' ? this.httpAgent : this.httpsAgent; } } @bindThis - public async getActivityJson(url: string): Promise { + public async getActivityJson(url: string, isLocalAddressAllowed = false): Promise { const res = await this.send(url, { method: 'GET', headers: { @@ -206,6 +223,7 @@ export class HttpRequestService { }, timeout: 5000, size: 1024 * 256, + isLocalAddressAllowed: isLocalAddressAllowed, }, { throwErrorWhenResponseNotOk: true, validators: [validateContentTypeSetAsActivityPub], @@ -220,7 +238,7 @@ export class HttpRequestService { } @bindThis - public async getJson(url: string, accept = 'application/json, */*', headers?: Record): Promise { + public async getJson(url: string, accept = 'application/json, */*', headers?: Record, isLocalAddressAllowed = false): Promise { const res = await this.send(url, { method: 'GET', headers: Object.assign({ @@ -228,19 +246,21 @@ export class HttpRequestService { }, headers ?? {}), timeout: 5000, size: 1024 * 256, + isLocalAddressAllowed: isLocalAddressAllowed, }); return await res.json() as T; } @bindThis - public async getHtml(url: string, accept = 'text/html, */*', headers?: Record): Promise { + public async getHtml(url: string, accept = 'text/html, */*', headers?: Record, isLocalAddressAllowed = false): Promise { const res = await this.send(url, { method: 'GET', headers: Object.assign({ Accept: accept, }, headers ?? {}), timeout: 5000, + isLocalAddressAllowed: isLocalAddressAllowed, }); return await res.text(); @@ -255,6 +275,7 @@ export class HttpRequestService { headers?: Record, timeout?: number, size?: number, + isLocalAddressAllowed?: boolean, } = {}, extra: HttpRequestSendOptions = { throwErrorWhenResponseNotOk: true, @@ -268,6 +289,8 @@ export class HttpRequestService { controller.abort(); }, timeout); + const isLocalAddressAllowed = args.isLocalAddressAllowed ?? false; + const res = await fetch(url, { method: args.method ?? 'GET', headers: { @@ -276,7 +299,7 @@ export class HttpRequestService { }, body: args.body, size: args.size ?? 10 * 1024 * 1024, - agent: (url) => this.getAgentByUrl(url), + agent: (url) => this.getAgentByUrl(url, false, isLocalAddressAllowed), signal: controller.signal, }); From 776f6fd1f568fc36e1b6b84d9b8e56611be58c5d Mon Sep 17 00:00:00 2001 From: rectcoordsystem Date: Wed, 13 Nov 2024 15:27:17 +0900 Subject: [PATCH 40/66] fix(backend): allow fetchSummaryFromProxy, trueMail to access local addresses --- packages/backend/src/core/EmailService.ts | 1 + packages/backend/src/server/web/UrlPreviewService.ts | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/packages/backend/src/core/EmailService.ts b/packages/backend/src/core/EmailService.ts index a176474b95..da198d0e42 100644 --- a/packages/backend/src/core/EmailService.ts +++ b/packages/backend/src/core/EmailService.ts @@ -312,6 +312,7 @@ export class EmailService { Accept: 'application/json', Authorization: truemailAuthKey, }, + isLocalAddressAllowed: true, }); const json = (await res.json()) as { diff --git a/packages/backend/src/server/web/UrlPreviewService.ts b/packages/backend/src/server/web/UrlPreviewService.ts index 981fbb4353..47cc09b067 100644 --- a/packages/backend/src/server/web/UrlPreviewService.ts +++ b/packages/backend/src/server/web/UrlPreviewService.ts @@ -170,6 +170,6 @@ export class UrlPreviewService { contentLengthRequired: meta.urlPreviewRequireContentLength, }); - return this.httpRequestService.getJson(`${proxy}?${queryStr}`); + return this.httpRequestService.getJson(`${proxy}?${queryStr}`, 'application/json, */*', undefined, true); } } From 8e90484b3e7ac255cc141000444e2ed6d6fa54f8 Mon Sep 17 00:00:00 2001 From: Julia Johannesen Date: Wed, 20 Nov 2024 19:21:57 -0500 Subject: [PATCH 41/66] Bump version --- package.json | 2 +- .../backend/src/server/FileServerService.ts | 89 ------------------- 2 files changed, 1 insertion(+), 90 deletions(-) diff --git a/package.json b/package.json index 2ba3881756..76cfbadb23 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "sharkey", - "version": "2024.9.2", + "version": "2024.9.3", "codename": "shonk", "repository": { "type": "git", diff --git a/packages/backend/src/server/FileServerService.ts b/packages/backend/src/server/FileServerService.ts index be196373c4..1a4d0cb48f 100644 --- a/packages/backend/src/server/FileServerService.ts +++ b/packages/backend/src/server/FileServerService.ts @@ -28,11 +28,7 @@ import { bindThis } from '@/decorators.js'; import { isMimeImage } from '@/misc/is-mime-image.js'; import { correctFilename } from '@/misc/correct-filename.js'; import { handleRequestRedirectToOmitSearch } from '@/misc/fastify-hook-handlers.js'; -import { RateLimiterService } from '@/server/api/RateLimiterService.js'; -import { getIpHash } from '@/misc/get-ip-hash.js'; -import { AuthenticateService } from '@/server/api/AuthenticateService.js'; import type { FastifyInstance, FastifyRequest, FastifyReply, FastifyPluginOptions } from 'fastify'; -import type Limiter from 'ratelimiter'; const _filename = fileURLToPath(import.meta.url); const _dirname = dirname(_filename); @@ -56,8 +52,6 @@ export class FileServerService { private videoProcessingService: VideoProcessingService, private internalStorageService: InternalStorageService, private loggerService: LoggerService, - private authenticateService: AuthenticateService, - private rateLimiterService: RateLimiterService, ) { this.logger = this.loggerService.getLogger('server', 'gray'); @@ -82,8 +76,6 @@ export class FileServerService { }); fastify.get<{ Params: { key: string; } }>('/files/:key', async (request, reply) => { - if (!await this.checkRateLimit(request, reply, `/files/${request.params.key}`)) return; - return await this.sendDriveFile(request, reply) .catch(err => this.errorHandler(request, reply, err)); }); @@ -97,20 +89,6 @@ export class FileServerService { Params: { url: string; }; Querystring: { url?: string; }; }>('/proxy/:url*', async (request, reply) => { - const url = 'url' in request.query ? request.query.url : 'https://' + request.params.url; - if (!url || !URL.canParse(url)) { - reply.code(400); - return; - } - - const keyUrl = new URL(url); - keyUrl.searchParams.forEach(k => keyUrl.searchParams.delete(k)); - keyUrl.hash = ''; - keyUrl.username = ''; - keyUrl.password = ''; - - if (!await this.checkRateLimit(request, reply, `/proxy/${keyUrl}`)) return; - return await this.proxyHandler(request, reply) .catch(err => this.errorHandler(request, reply, err)); }); @@ -594,71 +572,4 @@ export class FileServerService { path, }; } - - // Based on ApiCallService - private async checkRateLimit( - request: FastifyRequest<{ - Body?: Record | undefined, - Querystring?: Record | undefined, - Params?: Record | unknown, - }>, - reply: FastifyReply, - rateLimitKey: string, - ): Promise { - const body = request.method === 'GET' - ? request.query - : request.body; - - // https://datatracker.ietf.org/doc/html/rfc6750.html#section-2.1 (case sensitive) - const token = request.headers.authorization?.startsWith('Bearer ') - ? request.headers.authorization.slice(7) - : body?.['i']; - if (token != null && typeof token !== 'string') { - reply.code(400); - return false; - } - - // koa will automatically load the `X-Forwarded-For` header if `proxy: true` is configured in the app. - const [user] = await this.authenticateService.authenticate(token); - const actor = user?.id ?? getIpHash(request.ip); - - const limit = { - // Group by resource - key: rateLimitKey, - - // Maximum of 10 requests / 10 minutes - max: 10, - duration: 1000 * 60 * 10, - - // Minimum of 250 ms between each request - minInterval: 250, - }; - - // Rate limit proxy requests - try { - await this.rateLimiterService.limit(limit, actor); - return true; - } catch (err) { - // errはLimiter.LimiterInfoであることが期待される - reply.code(429); - - if (hasRateLimitInfo(err)) { - const cooldownInSeconds = Math.ceil((err.info.resetMs - Date.now()) / 1000); - // もしかするとマイナスになる可能性がなくはないのでマイナスだったら0にしておく - reply.header('Retry-After', Math.max(cooldownInSeconds, 0).toString(10)); - } - - reply.send({ - message: 'Rate limit exceeded. Please try again later.', - code: 'RATE_LIMIT_EXCEEDED', - id: 'd5826d14-3982-4d2e-8011-b9e9f02499ef', - }); - - return false; - } - } -} - -function hasRateLimitInfo(err: unknown): err is { info: Limiter.LimiterInfo } { - return err != null && typeof(err) === 'object' && 'info' in err; } From b0834ebf55d76979313378282d1dfc535b030e5d Mon Sep 17 00:00:00 2001 From: Hazelnoot Date: Tue, 19 Nov 2024 22:59:07 -0500 Subject: [PATCH 42/66] prevent DoS from spammed media proxy requests --- .../backend/src/server/FileServerService.ts | 89 +++++++++++++++++++ 1 file changed, 89 insertions(+) diff --git a/packages/backend/src/server/FileServerService.ts b/packages/backend/src/server/FileServerService.ts index 1a4d0cb48f..be196373c4 100644 --- a/packages/backend/src/server/FileServerService.ts +++ b/packages/backend/src/server/FileServerService.ts @@ -28,7 +28,11 @@ import { bindThis } from '@/decorators.js'; import { isMimeImage } from '@/misc/is-mime-image.js'; import { correctFilename } from '@/misc/correct-filename.js'; import { handleRequestRedirectToOmitSearch } from '@/misc/fastify-hook-handlers.js'; +import { RateLimiterService } from '@/server/api/RateLimiterService.js'; +import { getIpHash } from '@/misc/get-ip-hash.js'; +import { AuthenticateService } from '@/server/api/AuthenticateService.js'; import type { FastifyInstance, FastifyRequest, FastifyReply, FastifyPluginOptions } from 'fastify'; +import type Limiter from 'ratelimiter'; const _filename = fileURLToPath(import.meta.url); const _dirname = dirname(_filename); @@ -52,6 +56,8 @@ export class FileServerService { private videoProcessingService: VideoProcessingService, private internalStorageService: InternalStorageService, private loggerService: LoggerService, + private authenticateService: AuthenticateService, + private rateLimiterService: RateLimiterService, ) { this.logger = this.loggerService.getLogger('server', 'gray'); @@ -76,6 +82,8 @@ export class FileServerService { }); fastify.get<{ Params: { key: string; } }>('/files/:key', async (request, reply) => { + if (!await this.checkRateLimit(request, reply, `/files/${request.params.key}`)) return; + return await this.sendDriveFile(request, reply) .catch(err => this.errorHandler(request, reply, err)); }); @@ -89,6 +97,20 @@ export class FileServerService { Params: { url: string; }; Querystring: { url?: string; }; }>('/proxy/:url*', async (request, reply) => { + const url = 'url' in request.query ? request.query.url : 'https://' + request.params.url; + if (!url || !URL.canParse(url)) { + reply.code(400); + return; + } + + const keyUrl = new URL(url); + keyUrl.searchParams.forEach(k => keyUrl.searchParams.delete(k)); + keyUrl.hash = ''; + keyUrl.username = ''; + keyUrl.password = ''; + + if (!await this.checkRateLimit(request, reply, `/proxy/${keyUrl}`)) return; + return await this.proxyHandler(request, reply) .catch(err => this.errorHandler(request, reply, err)); }); @@ -572,4 +594,71 @@ export class FileServerService { path, }; } + + // Based on ApiCallService + private async checkRateLimit( + request: FastifyRequest<{ + Body?: Record | undefined, + Querystring?: Record | undefined, + Params?: Record | unknown, + }>, + reply: FastifyReply, + rateLimitKey: string, + ): Promise { + const body = request.method === 'GET' + ? request.query + : request.body; + + // https://datatracker.ietf.org/doc/html/rfc6750.html#section-2.1 (case sensitive) + const token = request.headers.authorization?.startsWith('Bearer ') + ? request.headers.authorization.slice(7) + : body?.['i']; + if (token != null && typeof token !== 'string') { + reply.code(400); + return false; + } + + // koa will automatically load the `X-Forwarded-For` header if `proxy: true` is configured in the app. + const [user] = await this.authenticateService.authenticate(token); + const actor = user?.id ?? getIpHash(request.ip); + + const limit = { + // Group by resource + key: rateLimitKey, + + // Maximum of 10 requests / 10 minutes + max: 10, + duration: 1000 * 60 * 10, + + // Minimum of 250 ms between each request + minInterval: 250, + }; + + // Rate limit proxy requests + try { + await this.rateLimiterService.limit(limit, actor); + return true; + } catch (err) { + // errはLimiter.LimiterInfoであることが期待される + reply.code(429); + + if (hasRateLimitInfo(err)) { + const cooldownInSeconds = Math.ceil((err.info.resetMs - Date.now()) / 1000); + // もしかするとマイナスになる可能性がなくはないのでマイナスだったら0にしておく + reply.header('Retry-After', Math.max(cooldownInSeconds, 0).toString(10)); + } + + reply.send({ + message: 'Rate limit exceeded. Please try again later.', + code: 'RATE_LIMIT_EXCEEDED', + id: 'd5826d14-3982-4d2e-8011-b9e9f02499ef', + }); + + return false; + } + } +} + +function hasRateLimitInfo(err: unknown): err is { info: Limiter.LimiterInfo } { + return err != null && typeof(err) === 'object' && 'info' in err; } From fa3cf6c2996741e642955c5e2fca8ad785e83205 Mon Sep 17 00:00:00 2001 From: Julia Johannesen Date: Wed, 20 Nov 2024 20:06:46 -0500 Subject: [PATCH 43/66] Fix type error in security fixes --- .../core/activitypub/models/ApPersonService.ts | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/packages/backend/src/core/activitypub/models/ApPersonService.ts b/packages/backend/src/core/activitypub/models/ApPersonService.ts index 1c117795e9..2119c41569 100644 --- a/packages/backend/src/core/activitypub/models/ApPersonService.ts +++ b/packages/backend/src/core/activitypub/models/ApPersonService.ts @@ -163,13 +163,16 @@ export class ApPersonService implements OnModuleInit { } for (const collection of ['outbox', 'followers', 'following'] as (keyof IActor)[]) { - const collectionUri = getApId((x as IActor)[collection]); - if (typeof collectionUri === 'string' && collectionUri.length > 0) { - if (this.utilityService.punyHost(collectionUri) !== expectHost) { - throw new Error(`invalid Actor: ${collection} has different host`); + const xCollection = (x as IActor)[collection]; + if (xCollection != null) { + const collectionUri = getApId(xCollection); + if (typeof collectionUri === 'string' && collectionUri.length > 0) { + if (this.utilityService.punyHost(collectionUri) !== expectHost) { + throw new Error(`invalid Actor: ${collection} has different host`); + } + } else if (collectionUri != null) { + throw new Error(`invalid Actor: wrong ${collection}`); } - } else if (collectionUri != null) { - throw new Error(`invalid Actor: wrong ${collection}`); } } From 1758f29364eca3cbd13dbb5c84909c93712b3b3b Mon Sep 17 00:00:00 2001 From: Julia Johannesen Date: Wed, 20 Nov 2024 20:16:43 -0500 Subject: [PATCH 44/66] Fix error in test function calls --- packages/backend/test/unit/activitypub.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/backend/test/unit/activitypub.ts b/packages/backend/test/unit/activitypub.ts index 53ced3dab3..73d6186edf 100644 --- a/packages/backend/test/unit/activitypub.ts +++ b/packages/backend/test/unit/activitypub.ts @@ -176,7 +176,7 @@ describe('ActivityPub', () => { resolver.register(actor.id, actor); resolver.register(post.id, post); - const note = await noteService.createNote(post.id, resolver, true); + const note = await noteService.createNote(post.id, undefined, resolver, true); assert.deepStrictEqual(note?.uri, post.id); assert.deepStrictEqual(note.visibility, 'public'); @@ -336,7 +336,7 @@ describe('ActivityPub', () => { resolver.register(actor.featured, featured); resolver.register(firstNote.id, firstNote); - const note = await noteService.createNote(firstNote.id as string, resolver); + const note = await noteService.createNote(firstNote.id as string, undefined, resolver); assert.strictEqual(note?.uri, firstNote.id); }); }); From 23c4aa25714af145098baa7edd74c1d217e51c1a Mon Sep 17 00:00:00 2001 From: Julia Johannesen Date: Wed, 20 Nov 2024 20:24:59 -0500 Subject: [PATCH 45/66] Fix style error --- packages/backend/src/core/HttpRequestService.ts | 10 +++++----- .../src/queue/processors/InboxProcessorService.ts | 3 +-- 2 files changed, 6 insertions(+), 7 deletions(-) diff --git a/packages/backend/src/core/HttpRequestService.ts b/packages/backend/src/core/HttpRequestService.ts index 0ad5667049..6dcd0cdff3 100644 --- a/packages/backend/src/core/HttpRequestService.ts +++ b/packages/backend/src/core/HttpRequestService.ts @@ -54,19 +54,19 @@ class HttpRequestServiceAgent extends http.Agent { } }); return socket; - }; + } @bindThis private isPrivateIp(ip: string): boolean { const parsedIp = ipaddr.parse(ip); - + for (const net of this.config.allowedPrivateNetworks ?? []) { const cidr = ipaddr.parseCIDR(net); if (cidr[0].kind() === parsedIp.kind() && parsedIp.match(ipaddr.parseCIDR(net))) { return false; } } - + return parsedIp.range() !== 'unicast'; } } @@ -98,14 +98,14 @@ class HttpsRequestServiceAgent extends https.Agent { @bindThis private isPrivateIp(ip: string): boolean { const parsedIp = ipaddr.parse(ip); - + for (const net of this.config.allowedPrivateNetworks ?? []) { const cidr = ipaddr.parseCIDR(net); if (cidr[0].kind() === parsedIp.kind() && parsedIp.match(ipaddr.parseCIDR(net))) { return false; } } - + return parsedIp.range() !== 'unicast'; } } diff --git a/packages/backend/src/queue/processors/InboxProcessorService.ts b/packages/backend/src/queue/processors/InboxProcessorService.ts index f453d7d1ae..102e835e24 100644 --- a/packages/backend/src/queue/processors/InboxProcessorService.ts +++ b/packages/backend/src/queue/processors/InboxProcessorService.ts @@ -192,8 +192,7 @@ export class InboxProcessorService implements OnApplicationShutdown { if (signerHost !== activityIdHost) { throw new Bull.UnrecoverableError(`skip: signerHost(${signerHost}) !== activity.id host(${activityIdHost}`); } - } - else { + } else { throw new Bull.UnrecoverableError('skip: activity id is not a string'); } From 36af07abe28bec670aaebf9f5af5694bb582c29a Mon Sep 17 00:00:00 2001 From: Julia Johannesen Date: Wed, 20 Nov 2024 20:31:22 -0500 Subject: [PATCH 46/66] Fix another style error --- packages/backend/src/core/HttpRequestService.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/backend/src/core/HttpRequestService.ts b/packages/backend/src/core/HttpRequestService.ts index 6dcd0cdff3..083153940a 100644 --- a/packages/backend/src/core/HttpRequestService.ts +++ b/packages/backend/src/core/HttpRequestService.ts @@ -93,7 +93,7 @@ class HttpsRequestServiceAgent extends https.Agent { } }); return socket; - }; + } @bindThis private isPrivateIp(ip: string): boolean { From 6027b516e1c82324d55d6e54d0e17cbd816feb42 Mon Sep 17 00:00:00 2001 From: Julia Johannesen Date: Wed, 20 Nov 2024 21:24:35 -0500 Subject: [PATCH 47/66] Fix `.punyHost` misuse --- packages/backend/src/core/RemoteUserResolveService.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/backend/src/core/RemoteUserResolveService.ts b/packages/backend/src/core/RemoteUserResolveService.ts index 678da0cfa6..098b5e1706 100644 --- a/packages/backend/src/core/RemoteUserResolveService.ts +++ b/packages/backend/src/core/RemoteUserResolveService.ts @@ -54,7 +54,7 @@ export class RemoteUserResolveService { }) as MiLocalUser; } - host = this.utilityService.punyHost(host); + host = this.utilityService.toPuny(host); if (host === this.utilityService.toPuny(this.config.host)) { this.logger.info(`return local user: ${usernameLower}`); From 59e160147fd38d49ebad27e0d705e3f2bf2613b4 Mon Sep 17 00:00:00 2001 From: Julia Johannesen Date: Wed, 20 Nov 2024 21:32:12 -0500 Subject: [PATCH 48/66] Bump develop version --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index 76cfbadb23..6e84882440 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "sharkey", - "version": "2024.9.3", + "version": "2024.10.0-dev", "codename": "shonk", "repository": { "type": "git", From 4e0f7ced842b538519c64769cd4a5adf010203ca Mon Sep 17 00:00:00 2001 From: Hazelnoot Date: Thu, 14 Nov 2024 18:10:14 -0500 Subject: [PATCH 49/66] preserve the raw URI in parseUri --- packages/backend/src/core/activitypub/ApDbResolverService.ts | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/packages/backend/src/core/activitypub/ApDbResolverService.ts b/packages/backend/src/core/activitypub/ApDbResolverService.ts index dd89716d34..f6b50ec704 100644 --- a/packages/backend/src/core/activitypub/ApDbResolverService.ts +++ b/packages/backend/src/core/activitypub/ApDbResolverService.ts @@ -66,9 +66,10 @@ export class ApDbResolverService implements OnApplicationShutdown { public parseUri(value: string | IObject | [string | IObject]): UriParseResult { const separator = '/'; - const uri = new URL(getApId(value)); + const apId = getApId(value); + const uri = new URL(apId); if (this.utilityService.toPuny(uri.host) !== this.utilityService.toPuny(this.config.host)) { - return { local: false, uri: uri.href }; + return { local: false, uri: apId }; } const [, type, id, ...rest] = uri.pathname.split(separator); From aabb1945e8a65af65437c8616b533491055e3c0d Mon Sep 17 00:00:00 2001 From: Hazelnoot Date: Sun, 17 Nov 2024 09:54:47 -0500 Subject: [PATCH 50/66] respect pinned note limit for remote users --- .../backend/src/core/activitypub/models/ApPersonService.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/packages/backend/src/core/activitypub/models/ApPersonService.ts b/packages/backend/src/core/activitypub/models/ApPersonService.ts index 2119c41569..6f8f3eca72 100644 --- a/packages/backend/src/core/activitypub/models/ApPersonService.ts +++ b/packages/backend/src/core/activitypub/models/ApPersonService.ts @@ -731,9 +731,10 @@ export class ApPersonService implements OnModuleInit { // Resolve and regist Notes const limit = promiseLimit(2); + const maxPinned = (await this.roleService.getUserPolicies(user.id)).pinLimit; const featuredNotes = await Promise.all(items .filter(item => getApType(item) === 'Note') // TODO: Noteでなくてもいいかも - .slice(0, 5) + .slice(0, maxPinned) .map(item => limit(() => this.apNoteService.resolveNote(item, { resolver: _resolver, sentFrom: new URL(user.uri), From 984cfe358d3558f75040ab2f7a043a454f6df84c Mon Sep 17 00:00:00 2001 From: Hazelnoot Date: Thu, 14 Nov 2024 18:47:28 -0500 Subject: [PATCH 51/66] reduce log spam from `updateFeatured` --- .../core/activitypub/models/ApPersonService.ts | 17 +++++++++++++++-- 1 file changed, 15 insertions(+), 2 deletions(-) diff --git a/packages/backend/src/core/activitypub/models/ApPersonService.ts b/packages/backend/src/core/activitypub/models/ApPersonService.ts index 2119c41569..d2f5b64119 100644 --- a/packages/backend/src/core/activitypub/models/ApPersonService.ts +++ b/packages/backend/src/core/activitypub/models/ApPersonService.ts @@ -7,6 +7,7 @@ import { Inject, Injectable } from '@nestjs/common'; import promiseLimit from 'promise-limit'; import { DataSource } from 'typeorm'; import { ModuleRef } from '@nestjs/core'; +import { AbortError } from 'node-fetch'; import { DI } from '@/di-symbols.js'; import type { FollowingsRepository, InstancesRepository, MiMeta, UserProfilesRepository, UserPublickeysRepository, UsersRepository } from '@/models/_.js'; import type { Config } from '@/config.js'; @@ -482,7 +483,13 @@ export class ApPersonService implements OnModuleInit { } //#endregion - await this.updateFeatured(user.id, resolver).catch(err => this.logger.error(err)); + await this.updateFeatured(user.id, resolver).catch(err => { + if (err instanceof AbortError || (err instanceof StatusError && err.isRetryable)) { + this.logger.warn(`Failed to update featured notes: ${err.name}: ${err.message}`); + } else { + this.logger.error('Failed to update featured notes:', err); + } + }); return user; } @@ -647,7 +654,13 @@ export class ApPersonService implements OnModuleInit { { followerSharedInbox: person.sharedInbox ?? person.endpoints?.sharedInbox }, ); - await this.updateFeatured(exist.id, resolver).catch(err => this.logger.error(err)); + await this.updateFeatured(exist.id, resolver).catch(err => { + if (err instanceof AbortError || (err instanceof StatusError && err.isRetryable)) { + this.logger.warn(`Failed to update featured notes: ${err.name}: ${err.message}`); + } else { + this.logger.error('Failed to update featured notes:', err); + } + }); const updated = { ...exist, ...updates }; From fedf0d7e20e615485b79b393e597ed2619577df0 Mon Sep 17 00:00:00 2001 From: Hazelnoot Date: Thu, 14 Nov 2024 20:32:59 -0500 Subject: [PATCH 52/66] further reduce log spam from `updateFeatured` errors --- .../activitypub/models/ApPersonService.ts | 26 ++++++++----------- 1 file changed, 11 insertions(+), 15 deletions(-) diff --git a/packages/backend/src/core/activitypub/models/ApPersonService.ts b/packages/backend/src/core/activitypub/models/ApPersonService.ts index d2f5b64119..6754c6c41b 100644 --- a/packages/backend/src/core/activitypub/models/ApPersonService.ts +++ b/packages/backend/src/core/activitypub/models/ApPersonService.ts @@ -483,13 +483,7 @@ export class ApPersonService implements OnModuleInit { } //#endregion - await this.updateFeatured(user.id, resolver).catch(err => { - if (err instanceof AbortError || (err instanceof StatusError && err.isRetryable)) { - this.logger.warn(`Failed to update featured notes: ${err.name}: ${err.message}`); - } else { - this.logger.error('Failed to update featured notes:', err); - } - }); + await this.updateFeatured(user.id, resolver).catch(err => console.error(err)); return user; } @@ -654,13 +648,7 @@ export class ApPersonService implements OnModuleInit { { followerSharedInbox: person.sharedInbox ?? person.endpoints?.sharedInbox }, ); - await this.updateFeatured(exist.id, resolver).catch(err => { - if (err instanceof AbortError || (err instanceof StatusError && err.isRetryable)) { - this.logger.warn(`Failed to update featured notes: ${err.name}: ${err.message}`); - } else { - this.logger.error('Failed to update featured notes:', err); - } - }); + await this.updateFeatured(exist.id, resolver).catch(err => console.error(err)); const updated = { ...exist, ...updates }; @@ -735,7 +723,15 @@ export class ApPersonService implements OnModuleInit { const _resolver = resolver ?? this.apResolverService.createResolver(); // Resolve to (Ordered)Collection Object - const collection = await _resolver.resolveCollection(user.featured); + const collection = await _resolver.resolveCollection(user.featured).catch(err => { + if (err instanceof AbortError || err instanceof StatusError) { + this.logger.warn(`Failed to update featured notes: ${err.name}: ${err.message}`); + } else { + this.logger.error('Failed to update featured notes:', err); + } + }); + if (!collection) return; + if (!isCollectionOrOrderedCollection(collection)) throw new Error('Object is not Collection or OrderedCollection'); // Resolve to Object(may be Note) arrays From dcd5b6d972809bf5d4f6f0ae3f1d7d1e9edab54b Mon Sep 17 00:00:00 2001 From: Hazelnoot Date: Sun, 17 Nov 2024 09:01:59 -0500 Subject: [PATCH 53/66] replace `console.error` with `this.logger.error` (merge error) --- .../backend/src/core/activitypub/models/ApPersonService.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/backend/src/core/activitypub/models/ApPersonService.ts b/packages/backend/src/core/activitypub/models/ApPersonService.ts index 6754c6c41b..b03d6b1853 100644 --- a/packages/backend/src/core/activitypub/models/ApPersonService.ts +++ b/packages/backend/src/core/activitypub/models/ApPersonService.ts @@ -483,7 +483,7 @@ export class ApPersonService implements OnModuleInit { } //#endregion - await this.updateFeatured(user.id, resolver).catch(err => console.error(err)); + await this.updateFeatured(user.id, resolver).catch(err => this.logger.error(err)); return user; } @@ -648,7 +648,7 @@ export class ApPersonService implements OnModuleInit { { followerSharedInbox: person.sharedInbox ?? person.endpoints?.sharedInbox }, ); - await this.updateFeatured(exist.id, resolver).catch(err => console.error(err)); + await this.updateFeatured(exist.id, resolver).catch(err => this.logger.error(err)); const updated = { ...exist, ...updates }; From a62e4f1cf2c99150d345917c290f13e5a48ab92e Mon Sep 17 00:00:00 2001 From: Hazelnoot Date: Thu, 14 Nov 2024 19:32:08 -0500 Subject: [PATCH 54/66] ignore `isNSFW` for pure renotes --- packages/backend/src/core/NoteCreateService.ts | 9 ++++++++- packages/backend/src/core/NoteEditService.ts | 9 ++++++++- 2 files changed, 16 insertions(+), 2 deletions(-) diff --git a/packages/backend/src/core/NoteCreateService.ts b/packages/backend/src/core/NoteCreateService.ts index 1bc4599a60..ccc5bc0214 100644 --- a/packages/backend/src/core/NoteCreateService.ts +++ b/packages/backend/src/core/NoteCreateService.ts @@ -146,6 +146,8 @@ type Option = { app?: MiApp | null; }; +type PureRenoteOption = Option & { renote: MiNote } & ({ text?: null } | { cw?: null } | { reply?: null } | { poll?: null } | { files?: null | [] }); + @Injectable() export class NoteCreateService implements OnApplicationShutdown { #shutdownController = new AbortController(); @@ -412,7 +414,7 @@ export class NoteCreateService implements OnApplicationShutdown { if (user.host && !data.cw) { await this.federatedInstanceService.fetch(user.host).then(async i => { - if (i.isNSFW) { + if (i.isNSFW && !this.isPureRenote(data)) { data.cw = 'Instance is marked as NSFW'; } }); @@ -821,6 +823,11 @@ export class NoteCreateService implements OnApplicationShutdown { if (!user.noindex) this.index(note); } + @bindThis + private isPureRenote(note: Option): note is PureRenoteOption { + return this.isRenote(note) && !this.isQuote(note); + } + @bindThis private isRenote(note: Option): note is Option & { renote: MiNote } { return note.renote != null; diff --git a/packages/backend/src/core/NoteEditService.ts b/packages/backend/src/core/NoteEditService.ts index d31958e5d4..6c456fb4a3 100644 --- a/packages/backend/src/core/NoteEditService.ts +++ b/packages/backend/src/core/NoteEditService.ts @@ -142,6 +142,8 @@ type Option = { editcount?: boolean | null; }; +type PureRenoteOption = Option & { renote: MiNote } & ({ text?: null } | { cw?: null } | { reply?: null } | { poll?: null } | { files?: null | [] }); + @Injectable() export class NoteEditService implements OnApplicationShutdown { #shutdownController = new AbortController(); @@ -442,7 +444,7 @@ export class NoteEditService implements OnApplicationShutdown { if (user.host && !data.cw) { await this.federatedInstanceService.fetch(user.host).then(async i => { - if (i.isNSFW) { + if (i.isNSFW && !this.isPureRenote(data)) { data.cw = 'Instance is marked as NSFW'; } }); @@ -787,6 +789,11 @@ export class NoteEditService implements OnApplicationShutdown { if (!user.noindex) this.index(note); } + @bindThis + private isPureRenote(note: Option): note is PureRenoteOption { + return this.isRenote(note) && !this.isQuote(note); + } + @bindThis private isRenote(note: Option): note is Option & { renote: MiNote } { return note.renote != null; From eb1e32681345aaa6184a540c7dba9ae4fd5a6e77 Mon Sep 17 00:00:00 2001 From: Hazelnoot Date: Thu, 14 Nov 2024 19:50:34 -0500 Subject: [PATCH 55/66] add script to fix hellspawns --- UPGRADE_NOTES.md | 33 +++++++++++++++++++++++++++++++++ 1 file changed, 33 insertions(+) diff --git a/UPGRADE_NOTES.md b/UPGRADE_NOTES.md index 8bebd4eb34..fb81a611b7 100644 --- a/UPGRADE_NOTES.md +++ b/UPGRADE_NOTES.md @@ -1,5 +1,38 @@ # Upgrade Notes +## 2024.10.0 + +### Hellspawns + +Sharkey versions before 2024.10 suffered from a bug in the "Mark instance as NSFW" feature. +When a user from such an instance boosted a note, the boost would be converted to a hellspawn (pure renote with Content Warning). +Hellspawns are buggy and do not properly federate, so it may be desirable to correct any that already exist in the database. +The following script will correct any local or remote hellspawns in the database. + +```postgresql +/* Remove "instance is marked as NSFW" hellspawns */ +UPDATE note +SET cw = null +WHERE + "renoteId" IS NOT NULL + AND "text" IS NULL + AND cw = 'Instance is marked as NSFW' + AND "replyId" IS NULL + AND "hasPoll" = false + AND "fileIds" = '{}'; + +/* Fix legacy / user-created hellspawns */ +UPDATE note +SET text = '.' +WHERE + "renoteId" IS NOT NULL + AND "text" IS NULL + AND cw IS NOT NULL + AND "replyId" IS NULL + AND "hasPoll" = false + AND "fileIds" = '{}'; +``` + ## 2024.9.0 ### Following Feed From c9934c379fcaaf02511f7d3ba63be44306feb722 Mon Sep 17 00:00:00 2001 From: Hazelnoot Date: Sun, 17 Nov 2024 09:31:17 -0500 Subject: [PATCH 56/66] remove duplicate `isPureRenote` method --- packages/backend/src/core/NoteCreateService.ts | 4 ++-- packages/backend/src/core/NoteEditService.ts | 9 +-------- 2 files changed, 3 insertions(+), 10 deletions(-) diff --git a/packages/backend/src/core/NoteCreateService.ts b/packages/backend/src/core/NoteCreateService.ts index ccc5bc0214..892a929c41 100644 --- a/packages/backend/src/core/NoteCreateService.ts +++ b/packages/backend/src/core/NoteCreateService.ts @@ -146,7 +146,7 @@ type Option = { app?: MiApp | null; }; -type PureRenoteOption = Option & { renote: MiNote } & ({ text?: null } | { cw?: null } | { reply?: null } | { poll?: null } | { files?: null | [] }); +export type PureRenoteOption = Option & { renote: MiNote } & ({ text?: null } | { cw?: null } | { reply?: null } | { poll?: null } | { files?: null | [] }); @Injectable() export class NoteCreateService implements OnApplicationShutdown { @@ -824,7 +824,7 @@ export class NoteCreateService implements OnApplicationShutdown { } @bindThis - private isPureRenote(note: Option): note is PureRenoteOption { + public isPureRenote(note: Option): note is PureRenoteOption { return this.isRenote(note) && !this.isQuote(note); } diff --git a/packages/backend/src/core/NoteEditService.ts b/packages/backend/src/core/NoteEditService.ts index 6c456fb4a3..e5e3c38cd3 100644 --- a/packages/backend/src/core/NoteEditService.ts +++ b/packages/backend/src/core/NoteEditService.ts @@ -142,8 +142,6 @@ type Option = { editcount?: boolean | null; }; -type PureRenoteOption = Option & { renote: MiNote } & ({ text?: null } | { cw?: null } | { reply?: null } | { poll?: null } | { files?: null | [] }); - @Injectable() export class NoteEditService implements OnApplicationShutdown { #shutdownController = new AbortController(); @@ -444,7 +442,7 @@ export class NoteEditService implements OnApplicationShutdown { if (user.host && !data.cw) { await this.federatedInstanceService.fetch(user.host).then(async i => { - if (i.isNSFW && !this.isPureRenote(data)) { + if (i.isNSFW && !this.noteCreateService.isPureRenote(data)) { data.cw = 'Instance is marked as NSFW'; } }); @@ -789,11 +787,6 @@ export class NoteEditService implements OnApplicationShutdown { if (!user.noindex) this.index(note); } - @bindThis - private isPureRenote(note: Option): note is PureRenoteOption { - return this.isRenote(note) && !this.isQuote(note); - } - @bindThis private isRenote(note: Option): note is Option & { renote: MiNote } { return note.renote != null; From cc394d9a4bd560097ddfa7c4d11f7abe9a993fa8 Mon Sep 17 00:00:00 2001 From: Hazelnoot Date: Sun, 17 Nov 2024 09:32:13 -0500 Subject: [PATCH 57/66] quote all symbols in hellspawn upgrade script --- UPGRADE_NOTES.md | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/UPGRADE_NOTES.md b/UPGRADE_NOTES.md index fb81a611b7..c941de6643 100644 --- a/UPGRADE_NOTES.md +++ b/UPGRADE_NOTES.md @@ -11,23 +11,23 @@ The following script will correct any local or remote hellspawns in the database ```postgresql /* Remove "instance is marked as NSFW" hellspawns */ -UPDATE note -SET cw = null +UPDATE "note" +SET "cw" = null WHERE "renoteId" IS NOT NULL AND "text" IS NULL - AND cw = 'Instance is marked as NSFW' + AND "cw" = 'Instance is marked as NSFW' AND "replyId" IS NULL AND "hasPoll" = false AND "fileIds" = '{}'; /* Fix legacy / user-created hellspawns */ -UPDATE note -SET text = '.' +UPDATE "note" +SET "text" = '.' WHERE "renoteId" IS NOT NULL AND "text" IS NULL - AND cw IS NOT NULL + AND "cw" IS NOT NULL AND "replyId" IS NULL AND "hasPoll" = false AND "fileIds" = '{}'; From 0f6d26e065731f70b75afda3542381409cbc9569 Mon Sep 17 00:00:00 2001 From: Hazelnoot Date: Thu, 14 Nov 2024 20:25:48 -0500 Subject: [PATCH 58/66] reduce log spam from charts --- packages/backend/src/core/chart/ChartManagementService.ts | 7 +++++++ packages/backend/src/core/chart/core.ts | 8 ++++---- 2 files changed, 11 insertions(+), 4 deletions(-) diff --git a/packages/backend/src/core/chart/ChartManagementService.ts b/packages/backend/src/core/chart/ChartManagementService.ts index 79681370a1..316feec6ee 100644 --- a/packages/backend/src/core/chart/ChartManagementService.ts +++ b/packages/backend/src/core/chart/ChartManagementService.ts @@ -6,6 +6,8 @@ import { Injectable } from '@nestjs/common'; import { bindThis } from '@/decorators.js'; +import { ChartLoggerService } from '@/core/chart/ChartLoggerService.js'; +import Logger from '@/logger.js'; import FederationChart from './charts/federation.js'; import NotesChart from './charts/notes.js'; import UsersChart from './charts/users.js'; @@ -24,6 +26,7 @@ import type { OnApplicationShutdown } from '@nestjs/common'; export class ChartManagementService implements OnApplicationShutdown { private charts; private saveIntervalId: NodeJS.Timeout; + private readonly logger: Logger; constructor( private federationChart: FederationChart, @@ -38,6 +41,7 @@ export class ChartManagementService implements OnApplicationShutdown { private perUserFollowingChart: PerUserFollowingChart, private perUserDriveChart: PerUserDriveChart, private apRequestChart: ApRequestChart, + private chartLoggerService: ChartLoggerService, ) { this.charts = [ this.federationChart, @@ -53,6 +57,7 @@ export class ChartManagementService implements OnApplicationShutdown { this.perUserDriveChart, this.apRequestChart, ]; + this.logger = chartLoggerService.logger; } @bindThis @@ -62,6 +67,7 @@ export class ChartManagementService implements OnApplicationShutdown { for (const chart of this.charts) { chart.save(); } + this.logger.info('All charts saved'); }, 1000 * 60 * 20); } @@ -72,6 +78,7 @@ export class ChartManagementService implements OnApplicationShutdown { await Promise.all( this.charts.map(chart => chart.save()), ); + this.logger.info('All charts saved'); } } diff --git a/packages/backend/src/core/chart/core.ts b/packages/backend/src/core/chart/core.ts index af5485a46e..234c1d63b4 100644 --- a/packages/backend/src/core/chart/core.ts +++ b/packages/backend/src/core/chart/core.ts @@ -368,7 +368,7 @@ export default abstract class Chart { // 初期ログデータを作成 data = this.getNewLog(null); - this.logger.info(`${this.name + (group ? `:${group}` : '')}(${span}): Initial commit created`); + this.logger.debug(`${this.name + (group ? `:${group}` : '')}(${span}): Initial commit created`); } const date = Chart.dateToTimestamp(current); @@ -398,7 +398,7 @@ export default abstract class Chart { ...columns, }) as RawRecord; - this.logger.info(`${this.name + (group ? `:${group}` : '')}(${span}): New commit created`); + this.logger.debug(`${this.name + (group ? `:${group}` : '')}(${span}): New commit created`); return log; } finally { @@ -418,7 +418,7 @@ export default abstract class Chart { @bindThis public async save(): Promise { if (this.buffer.length === 0) { - this.logger.info(`${this.name}: Write skipped`); + this.logger.debug(`${this.name}: Write skipped`); return; } @@ -519,7 +519,7 @@ export default abstract class Chart { .execute(), ]); - this.logger.info(`${this.name + (logHour.group ? `:${logHour.group}` : '')}: Updated`); + this.logger.debug(`${this.name + (logHour.group ? `:${logHour.group}` : '')}: Updated`); // TODO: この一連の処理が始まった後に新たにbufferに入ったものは消さないようにする this.buffer = this.buffer.filter(q => q.group != null && (q.group !== logHour.group)); From dff465000cfe10c8196ac6933e831eed5c7d687e Mon Sep 17 00:00:00 2001 From: Hazelnoot Date: Sun, 3 Nov 2024 16:49:58 -0500 Subject: [PATCH 59/66] fix import-order in ApInboxService --- packages/backend/src/core/activitypub/ApInboxService.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/backend/src/core/activitypub/ApInboxService.ts b/packages/backend/src/core/activitypub/ApInboxService.ts index 90444a1af3..45b3da5ca0 100644 --- a/packages/backend/src/core/activitypub/ApInboxService.ts +++ b/packages/backend/src/core/activitypub/ApInboxService.ts @@ -30,6 +30,7 @@ import type { MiRemoteUser } from '@/models/User.js'; import { GlobalEventService } from '@/core/GlobalEventService.js'; import { AbuseReportService } from '@/core/AbuseReportService.js'; import { FederatedInstanceService } from '@/core/FederatedInstanceService.js'; +import { fromTuple } from '@/misc/from-tuple.js'; import { getApHrefNullable, getApId, getApIds, getApType, isAccept, isActor, isAdd, isAnnounce, isBlock, isCollection, isCollectionOrOrderedCollection, isCreate, isDelete, isFlag, isFollow, isLike, isMove, isPost, isReject, isRemove, isTombstone, isUndo, isUpdate, validActor, validPost } from './type.js'; import { ApNoteService } from './models/ApNoteService.js'; import { ApLoggerService } from './ApLoggerService.js'; @@ -40,7 +41,6 @@ import { ApPersonService } from './models/ApPersonService.js'; import { ApQuestionService } from './models/ApQuestionService.js'; import type { Resolver } from './ApResolverService.js'; import type { IAccept, IAdd, IAnnounce, IBlock, ICreate, IDelete, IFlag, IFollow, ILike, IObject, IReject, IRemove, IUndo, IUpdate, IMove, IPost } from './type.js'; -import { fromTuple } from '@/misc/from-tuple.js'; @Injectable() export class ApInboxService { From 8f42e8434eaebe3aba5d1980c57f49dd8ad0de91 Mon Sep 17 00:00:00 2001 From: Hazelnoot Date: Sun, 3 Nov 2024 16:50:43 -0500 Subject: [PATCH 60/66] fix exception handling for Like activities --- .../backend/src/core/activitypub/ApInboxService.ts | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/packages/backend/src/core/activitypub/ApInboxService.ts b/packages/backend/src/core/activitypub/ApInboxService.ts index 45b3da5ca0..550dabf4de 100644 --- a/packages/backend/src/core/activitypub/ApInboxService.ts +++ b/packages/backend/src/core/activitypub/ApInboxService.ts @@ -31,6 +31,7 @@ import { GlobalEventService } from '@/core/GlobalEventService.js'; import { AbuseReportService } from '@/core/AbuseReportService.js'; import { FederatedInstanceService } from '@/core/FederatedInstanceService.js'; import { fromTuple } from '@/misc/from-tuple.js'; +import { IdentifiableError } from '@/misc/identifiable-error.js'; import { getApHrefNullable, getApId, getApIds, getApType, isAccept, isActor, isAdd, isAnnounce, isBlock, isCollection, isCollectionOrOrderedCollection, isCreate, isDelete, isFlag, isFollow, isLike, isMove, isPost, isReject, isRemove, isTombstone, isUndo, isUpdate, validActor, validPost } from './type.js'; import { ApNoteService } from './models/ApNoteService.js'; import { ApLoggerService } from './ApLoggerService.js'; @@ -204,13 +205,16 @@ export class ApInboxService { await this.apNoteService.extractEmojis(activity.tag ?? [], actor.host).catch(() => null); - return await this.reactionService.create(actor, note, activity._misskey_reaction ?? activity.content ?? activity.name).catch(err => { - if (err.id === '51c42bb4-931a-456b-bff7-e5a8a70dd298') { + try { + await this.reactionService.create(actor, note, activity._misskey_reaction ?? activity.content ?? activity.name); + return 'ok'; + } catch (err) { + if (err instanceof IdentifiableError && err.id === '51c42bb4-931a-456b-bff7-e5a8a70dd298') { return 'skip: already reacted'; } else { throw err; } - }).then(() => 'ok'); + } } @bindThis From cfc3ab4b045af0674122fa49176431860176358b Mon Sep 17 00:00:00 2001 From: Hazelnoot Date: Sun, 3 Nov 2024 16:50:54 -0500 Subject: [PATCH 61/66] fix exception handling for Announce activities --- packages/backend/src/core/activitypub/ApInboxService.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/backend/src/core/activitypub/ApInboxService.ts b/packages/backend/src/core/activitypub/ApInboxService.ts index 550dabf4de..ea01086368 100644 --- a/packages/backend/src/core/activitypub/ApInboxService.ts +++ b/packages/backend/src/core/activitypub/ApInboxService.ts @@ -297,7 +297,7 @@ export class ApInboxService { const target = await resolver.resolve(activityObject).catch(e => { this.logger.error(`Resolution failed: ${e}`); - return e; + throw e; }); if (isPost(target)) return await this.announceNote(actor, activity, target); From 0de7a084a99860b6f88bc2ce940dd6cb6d3543d5 Mon Sep 17 00:00:00 2001 From: Hazelnoot Date: Sun, 3 Nov 2024 16:51:12 -0500 Subject: [PATCH 62/66] fix exception handling for Undo activities --- packages/backend/src/core/activitypub/ApInboxService.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/backend/src/core/activitypub/ApInboxService.ts b/packages/backend/src/core/activitypub/ApInboxService.ts index ea01086368..97c8ef3a9d 100644 --- a/packages/backend/src/core/activitypub/ApInboxService.ts +++ b/packages/backend/src/core/activitypub/ApInboxService.ts @@ -666,7 +666,7 @@ export class ApInboxService { const object = await resolver.resolve(activity.object).catch(e => { this.logger.error(`Resolution failed: ${e}`); - return e; + throw e; }); // don't queue because the sender may attempt again when timeout From bcc20d6dc48590e60d63e8071c83986393470c1b Mon Sep 17 00:00:00 2001 From: Hazelnoot Date: Sun, 3 Nov 2024 16:26:00 -0500 Subject: [PATCH 63/66] allow Update activities for non-note posts --- packages/backend/src/core/activitypub/ApInboxService.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/backend/src/core/activitypub/ApInboxService.ts b/packages/backend/src/core/activitypub/ApInboxService.ts index 90444a1af3..dab8d98c23 100644 --- a/packages/backend/src/core/activitypub/ApInboxService.ts +++ b/packages/backend/src/core/activitypub/ApInboxService.ts @@ -803,7 +803,7 @@ export class ApInboxService { } else if (getApType(object) === 'Question') { await this.apQuestionService.updateQuestion(object, actor, resolver).catch(err => console.error(err)); return 'ok: Question updated'; - } else if (getApType(object) === 'Note') { + } else if (isPost(object)) { await this.apNoteService.updateNote(object, actor, resolver).catch(err => console.error(err)); return 'ok: Note updated'; } else { From c48faca707ca50c3f5ea05a4a6e1359b9fc7dcf6 Mon Sep 17 00:00:00 2001 From: Hazelnoot Date: Mon, 18 Nov 2024 10:33:32 -0500 Subject: [PATCH 64/66] fix lint errors in UrlPreviewService --- 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 47cc09b067..adb188b66f 100644 --- a/packages/backend/src/server/web/UrlPreviewService.ts +++ b/packages/backend/src/server/web/UrlPreviewService.ts @@ -6,6 +6,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 { DI } from '@/di-symbols.js'; import type { Config } from '@/config.js'; import { HttpRequestService } from '@/core/HttpRequestService.js'; @@ -15,9 +16,8 @@ import { LoggerService } from '@/core/LoggerService.js'; import { bindThis } from '@/decorators.js'; import { ApiError } from '@/server/api/error.js'; import { MiMeta } from '@/models/Meta.js'; -import type { FastifyRequest, FastifyReply } from 'fastify'; -import * as Redis from 'ioredis'; import { RedisKVCache } from '@/misc/cache.js'; +import type { FastifyRequest, FastifyReply } from 'fastify'; @Injectable() export class UrlPreviewService { @@ -41,7 +41,7 @@ export class UrlPreviewService { this.previewCache = new RedisKVCache(this.redisClient, 'summaly', { lifetime: 1000 * 60 * 60 * 24, // 1d memoryCacheLifetime: 1000 * 60 * 10, // 10m - fetcher: (key: string) => { throw new Error('the UrlPreview cache should never fetch'); }, + fetcher: () => { throw new Error('the UrlPreview cache should never fetch'); }, toRedisConverter: (value) => JSON.stringify(value), fromRedisConverter: (value) => JSON.parse(value), }); From 4c6cec552eb629f6c796bbc42db319e218f89515 Mon Sep 17 00:00:00 2001 From: Hazelnoot Date: Mon, 18 Nov 2024 10:41:18 -0500 Subject: [PATCH 65/66] verify that preview URL is valid --- 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 adb188b66f..26ea185586 100644 --- a/packages/backend/src/server/web/UrlPreviewService.ts +++ b/packages/backend/src/server/web/UrlPreviewService.ts @@ -65,7 +65,7 @@ export class UrlPreviewService { reply: FastifyReply, ): Promise { const url = request.query.url; - if (typeof url !== 'string') { + if (typeof url !== 'string' || !URL.canParse(url)) { reply.code(400); return; } From 2a4c432f41580cf2afa97bfd1a35153883b318db Mon Sep 17 00:00:00 2001 From: Hazelnoot Date: Mon, 18 Nov 2024 10:41:31 -0500 Subject: [PATCH 66/66] don't generate URL previews for blocked domains --- .../backend/src/server/web/UrlPreviewService.ts | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/packages/backend/src/server/web/UrlPreviewService.ts b/packages/backend/src/server/web/UrlPreviewService.ts index 26ea185586..19dac1dfb8 100644 --- a/packages/backend/src/server/web/UrlPreviewService.ts +++ b/packages/backend/src/server/web/UrlPreviewService.ts @@ -17,6 +17,7 @@ import { bindThis } from '@/decorators.js'; 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'; @Injectable() @@ -36,6 +37,7 @@ export class UrlPreviewService { private httpRequestService: HttpRequestService, private loggerService: LoggerService, + private utilityService: UtilityService, ) { this.logger = this.loggerService.getLogger('url-preview'); this.previewCache = new RedisKVCache(this.redisClient, 'summaly', { @@ -87,6 +89,18 @@ export class UrlPreviewService { }; } + const host = new URL(url).host; + if (this.utilityService.isBlockedHost(this.meta.blockedHosts, host)) { + reply.code(403); + return { + error: new ApiError({ + message: 'URL is blocked', + code: 'URL_PREVIEW_BLOCKED', + id: '50294652-857b-4b13-9700-8e5c7a8deae8', + }), + }; + } + const key = `${url}@${lang}`; const cached = await this.previewCache.get(key); if (cached !== undefined) {