From 76c0b646b1927014eb899711f5c0ac23a6f9c629 Mon Sep 17 00:00:00 2001 From: dakkar Date: Wed, 2 Jul 2025 16:43:24 +0100 Subject: [PATCH 01/26] recalculate size&hash after web-optimising videos --- packages/backend/src/core/DriveService.ts | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/packages/backend/src/core/DriveService.ts b/packages/backend/src/core/DriveService.ts index b9be4e3039..8874a4d8af 100644 --- a/packages/backend/src/core/DriveService.ts +++ b/packages/backend/src/core/DriveService.ts @@ -154,8 +154,8 @@ export class DriveService { @bindThis private async save(file: MiDriveFile, path: string, name: string, info: FileInfo): Promise { const type = info.type.mime; - const hash = info.md5; - const size = info.size; + let hash = info.md5; + let size = info.size; // thunbnail, webpublic を必要なら生成 const alts = await this.generateAlts(path, type, !file.uri); @@ -163,6 +163,9 @@ export class DriveService { if (type && type.startsWith('video/')) { try { await this.videoProcessingService.webOptimizeVideo(path, type); + const newInfo = await this.fileInfoService.getFileInfo(path); + hash = newInfo.md5; + size = newInfo.size; } catch (err) { this.registerLogger.warn(`Video optimization failed: ${renderInlineError(err)}`); } From c927c3056767e955e57a6d5f411591688f7de239 Mon Sep 17 00:00:00 2001 From: dakkar Date: Thu, 3 Jul 2025 12:07:43 +0100 Subject: [PATCH 02/26] mark grouped notifs by oldest id - sort-of fix 1139 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Misskey's code does the same, but our groups behave differently enough that this may be not the best choice for example, let's say we have: - notifications 1-5 for reaction to note A - notifications 6-8 for reaction to note B - notifications 9-12 for reaction to note A - notification 13-19 for non-groupable events - notification 20 for reaction to note A and that events happened one every minute (so the last notification is from 20 minutes ago) client requests the most recent 10 notifications; we fetch notifications 1-10, and reply: - grouped id 6 for reactions 6-8 to note B - grouped id 10 for reactions 1-5, 9-10 to note A then the client requests 10 more notifications, untilId=10; we fetch notifications 11-20, and reply: - non-grouped notifications 13-19 - grouped id 20 for reactions 11,12,20 to note A because we sort by id, and also the `createdAt` marks the _newest_ event in each group, the client will then show: 6 reactions to note B, 6 minutes ago 4 reactions to note A, 1 minute ago notifications 13-19, 13 minutes to 19 minutes ago 3 reactions to note A, 11 minutes ago I don't know how to make this work better ☹ --- .../server/api/endpoints/i/notifications-grouped.ts | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/packages/backend/src/server/api/endpoints/i/notifications-grouped.ts b/packages/backend/src/server/api/endpoints/i/notifications-grouped.ts index 444734070f..3821b5a20e 100644 --- a/packages/backend/src/server/api/endpoints/i/notifications-grouped.ts +++ b/packages/backend/src/server/api/endpoints/i/notifications-grouped.ts @@ -116,7 +116,7 @@ export default class extends Endpoint { // eslint- // user has many notifications, the pagination will break the // groups - // scan `notifications` newest-to-oldest + // scan `notifications` newest-to-oldest (unless we have sinceId && !untilId, in which case it's oldest-to-newest) for (let i = 0; i < notifications.length; i++) { const notification = notifications[i]; @@ -135,7 +135,7 @@ export default class extends Endpoint { // eslint- if (prevReaction.type !== 'reaction:grouped') { prevReaction = groupedNotifications[reactionIdx] = { type: 'reaction:grouped', - id: prevReaction.id, // this will be the newest id in this group + id: '', createdAt: prevReaction.createdAt, noteId: prevReaction.noteId!, reactions: [{ @@ -149,6 +149,7 @@ export default class extends Endpoint { // eslint- userId: notification.notifierId!, reaction: notification.reaction!, }); + prevReaction.id = notification.id; // this will be the *oldest* id in this group (newest if sinceId && !untilId) continue; } @@ -167,7 +168,7 @@ export default class extends Endpoint { // eslint- if (prevRenote.type !== 'renote:grouped') { prevRenote = groupedNotifications[renoteIdx] = { type: 'renote:grouped', - id: prevRenote.id, // this will be the newest id in this group + id: '', createdAt: prevRenote.createdAt, noteId: prevRenote.noteId!, userIds: [prevRenote.notifierId!], @@ -175,6 +176,7 @@ export default class extends Endpoint { // eslint- } // add this new renote to the existing group (prevRenote as FilterUnionByProperty).userIds.push(notification.notifierId!); + prevRenote.id = notification.id; // this will be the *oldest* id in this group (newest if sinceId && !untilId) continue; } @@ -182,10 +184,12 @@ export default class extends Endpoint { // eslint- groupedNotifications.push(notification); } - // sort the groups by their id, newest first + // sort the groups by their id groupedNotifications.sort( (a, b) => a.id < b.id ? 1 : a.id > b.id ? -1 : 0, ); + // this matches the logic in NotificationService and it's what MkPagination expects + if (ps.sinceId && !ps.untilId) groupedNotifications.reverse(); return await this.notificationEntityService.packGroupedMany(groupedNotifications, me.id); }); From 38616ab246ae2a48e8299727faa77482eafb696a Mon Sep 17 00:00:00 2001 From: Hazelnoot Date: Fri, 4 Jul 2025 12:54:52 -0400 Subject: [PATCH 03/26] disable outgoing mastodon quotes --- packages/backend/src/core/activitypub/ApRendererService.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/packages/backend/src/core/activitypub/ApRendererService.ts b/packages/backend/src/core/activitypub/ApRendererService.ts index 623e7002cd..9f55be11ac 100644 --- a/packages/backend/src/core/activitypub/ApRendererService.ts +++ b/packages/backend/src/core/activitypub/ApRendererService.ts @@ -555,7 +555,8 @@ export class ApRendererService { quoteUrl: quote, quoteUri: quote, // https://codeberg.org/fediverse/fep/src/branch/main/fep/044f/fep-044f.md - quote: quote, + // Disabled since Mastodon hides the fallback link when this is set + // quote: quote, published: this.idService.parse(note.id).date.toISOString(), to, cc, From 215f095a3fc9d0bbffb39e26988f3176a7822e37 Mon Sep 17 00:00:00 2001 From: dakkar Date: Fri, 25 Jul 2025 19:16:03 +0100 Subject: [PATCH 04/26] sort notifications by creation time so groups are sorted newest first according to the displayed time --- .../frontend/src/components/MkNotifications.vue | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/packages/frontend/src/components/MkNotifications.vue b/packages/frontend/src/components/MkNotifications.vue index 46e98462dc..6cd55ffedd 100644 --- a/packages/frontend/src/components/MkNotifications.vue +++ b/packages/frontend/src/components/MkNotifications.vue @@ -23,7 +23,7 @@ SPDX-License-Identifier: AGPL-3.0-only :moveClass=" $style.transition_x_move" tag="div" > -
+
@@ -67,6 +67,20 @@ const pagination = computed(() => prefer.r.useGroupedNotifications.value ? { })), }); +// for pagination reasons, each notification group needs to have the +// id of the oldest notification inside it, but we want to show the +// groups sorted by the time of the *newest* notification; so we re-sort +// them here +function sortedByTime(notifications) { + return notifications.toSorted( + (a, b) => { + if (a.createdAt < b.createdAt) return 1; + if (a.createdAt > b.createdAt) return -1; + return 0; + } + ); +} + function onNotification(notification) { const isMuted = props.excludeTypes ? props.excludeTypes.includes(notification.type) : false; if (isMuted || window.document.visibilityState === 'visible') { From 982223ad38e428ca4e2269fff56bccd332ca0222 Mon Sep 17 00:00:00 2001 From: Hazelnoot Date: Fri, 4 Jul 2025 12:16:18 -0400 Subject: [PATCH 05/26] validate all URLs before fetch --- .../backend/src/core/HttpRequestService.ts | 8 ++- packages/backend/src/core/UtilityService.ts | 59 ++++++++++++++++- .../src/core/activitypub/ApRequestService.ts | 4 -- .../src/core/activitypub/ApUtilityService.ts | 63 +++++-------------- .../core/activitypub/models/ApNoteService.ts | 2 +- .../activitypub/models/ApPersonService.ts | 8 +-- .../unit/core/activitypub/ApUtilityService.ts | 30 ++++++--- 7 files changed, 101 insertions(+), 73 deletions(-) diff --git a/packages/backend/src/core/HttpRequestService.ts b/packages/backend/src/core/HttpRequestService.ts index 151097095d..046b0dc244 100644 --- a/packages/backend/src/core/HttpRequestService.ts +++ b/packages/backend/src/core/HttpRequestService.ts @@ -17,7 +17,8 @@ import { StatusError } from '@/misc/status-error.js'; import { bindThis } from '@/decorators.js'; import { validateContentTypeSetAsActivityPub } from '@/core/activitypub/misc/validator.js'; import type { IObject, IObjectWithId } from '@/core/activitypub/type.js'; -import { ApUtilityService } from './activitypub/ApUtilityService.js'; +import { UtilityService } from '@/core/UtilityService.js'; +import { ApUtilityService } from '@/core/activitypub/ApUtilityService.js'; import type { Response } from 'node-fetch'; import type { URL } from 'node:url'; import type { Socket } from 'node:net'; @@ -132,6 +133,7 @@ export class HttpRequestService { @Inject(DI.config) private config: Config, private readonly apUtilityService: ApUtilityService, + private readonly utilityService: UtilityService, ) { const cache = new CacheableLookup({ maxTtl: 3600, // 1hours @@ -236,8 +238,6 @@ export class HttpRequestService { @bindThis public async getActivityJson(url: string, isLocalAddressAllowed = false, allowAnonymous = false): Promise { - this.apUtilityService.assertApUrl(url); - const res = await this.send(url, { method: 'GET', headers: { @@ -311,6 +311,8 @@ export class HttpRequestService { ): Promise { const timeout = args.timeout ?? 5000; + this.utilityService.assertUrl(url); + const controller = new AbortController(); setTimeout(() => { controller.abort(); diff --git a/packages/backend/src/core/UtilityService.ts b/packages/backend/src/core/UtilityService.ts index 3098367392..281edccca3 100644 --- a/packages/backend/src/core/UtilityService.ts +++ b/packages/backend/src/core/UtilityService.ts @@ -10,7 +10,10 @@ import psl from 'psl'; import { DI } from '@/di-symbols.js'; import type { Config } from '@/config.js'; import { bindThis } from '@/decorators.js'; -import { MiMeta } from '@/models/Meta.js'; +import { MiMeta, SoftwareSuspension } from '@/models/Meta.js'; +import { MiInstance } from '@/models/Instance.js'; +import { IdentifiableError } from '@/misc/identifiable-error.js'; +import { EnvService } from '@/core/EnvService.js'; @Injectable() export class UtilityService { @@ -20,6 +23,8 @@ export class UtilityService { @Inject(DI.meta) private meta: MiMeta, + + private readonly envService: EnvService, ) { } @@ -181,8 +186,8 @@ export class UtilityService { } @bindThis - public punyHostPSLDomain(url: string): string { - const urlObj = new URL(url); + public punyHostPSLDomain(url: string | URL): string { + const urlObj = typeof(url) === 'object' ? url : new URL(url); const hostname = urlObj.hostname; const domain = this.specialSuffix(hostname) ?? psl.get(hostname) ?? hostname; const host = `${this.toPuny(domain)}${urlObj.port.length > 0 ? ':' + urlObj.port : ''}`; @@ -213,4 +218,52 @@ export class UtilityService { return ''; } } + + /** + * Verifies that a provided URL is in a format acceptable for federation. + * @throws {IdentifiableError} If URL cannot be parsed + * @throws {IdentifiableError} If URL is not HTTPS + * @throws {IdentifiableError} If URL contains credentials + */ + @bindThis + public assertUrl(url: string | URL): URL | never { + // If string, parse and validate + if (typeof(url) === 'string') { + try { + url = new URL(url); + } catch { + throw new IdentifiableError('0bedd29b-e3bf-4604-af51-d3352e2518af', `invalid url ${url}: not a valid URL`); + } + } + + // Must be HTTPS + if (!this.checkHttps(url)) { + throw new IdentifiableError('0bedd29b-e3bf-4604-af51-d3352e2518af', `invalid url ${url}: unsupported protocol ${url.protocol}`); + } + + // Must not have credentials + if (url.username || url.password) { + throw new IdentifiableError('0bedd29b-e3bf-4604-af51-d3352e2518af', `invalid url ${url}: contains embedded credentials`); + } + + return url; + } + + /** + * Checks if the URL contains HTTPS. + * Additionally, allows HTTP in non-production environments. + * Based on check-https.ts. + */ + @bindThis + public checkHttps(url: string | URL): boolean { + const isNonProd = this.envService.env.NODE_ENV !== 'production'; + + try { + const proto = new URL(url).protocol; + return proto === 'https:' || (proto === 'http:' && isNonProd); + } catch { + // Invalid URLs don't "count" as HTTPS + return false; + } + } } diff --git a/packages/backend/src/core/activitypub/ApRequestService.ts b/packages/backend/src/core/activitypub/ApRequestService.ts index e4db9b237c..7669ce9669 100644 --- a/packages/backend/src/core/activitypub/ApRequestService.ts +++ b/packages/backend/src/core/activitypub/ApRequestService.ts @@ -157,8 +157,6 @@ export class ApRequestService { @bindThis public async signedPost(user: { id: MiUser['id'] }, url: string, object: unknown, digest?: string): Promise { - this.apUtilityService.assertApUrl(url); - const body = typeof object === 'string' ? object : JSON.stringify(object); const keypair = await this.userKeypairService.getUserKeypair(user.id); @@ -191,8 +189,6 @@ export class ApRequestService { */ @bindThis public async signedGet(url: string, user: { id: MiUser['id'] }, allowAnonymous = false, followAlternate?: boolean): Promise { - this.apUtilityService.assertApUrl(url); - const _followAlternate = followAlternate ?? true; const keypair = await this.userKeypairService.getUserKeypair(user.id); diff --git a/packages/backend/src/core/activitypub/ApUtilityService.ts b/packages/backend/src/core/activitypub/ApUtilityService.ts index 227dc3b9b3..eede48f0c6 100644 --- a/packages/backend/src/core/activitypub/ApUtilityService.ts +++ b/packages/backend/src/core/activitypub/ApUtilityService.ts @@ -7,14 +7,12 @@ import { Injectable } from '@nestjs/common'; import { UtilityService } from '@/core/UtilityService.js'; import { IdentifiableError } from '@/misc/identifiable-error.js'; import { toArray } from '@/misc/prelude/array.js'; -import { EnvService } from '@/core/EnvService.js'; -import { getApId, getOneApHrefNullable, IObject } from './type.js'; +import { getApId, getOneApHrefNullable, IObject } from '@/core/activitypub/type.js'; @Injectable() export class ApUtilityService { constructor( private readonly utilityService: UtilityService, - private readonly envService: EnvService, ) {} /** @@ -39,8 +37,11 @@ export class ApUtilityService { public haveSameAuthority(url1: string, url2: string): boolean { if (url1 === url2) return true; - const authority1 = this.utilityService.punyHostPSLDomain(url1); - const authority2 = this.utilityService.punyHostPSLDomain(url2); + const parsed1 = this.utilityService.assertUrl(url1); + const parsed2 = this.utilityService.assertUrl(url2); + + const authority1 = this.utilityService.punyHostPSLDomain(parsed1); + const authority2 = this.utilityService.punyHostPSLDomain(parsed2); return authority1 === authority2; } @@ -63,12 +64,16 @@ export class ApUtilityService { : undefined, })) .filter(({ url, type }) => { - if (!url) return false; - if (!this.checkHttps(url)) return false; - if (!isAcceptableUrlType(type)) return false; + try { + if (!url) return false; + if (!isAcceptableUrlType(type)) return false; + const parsed = this.utilityService.assertUrl(url); - const urlAuthority = this.utilityService.punyHostPSLDomain(url); - return urlAuthority === targetAuthority; + const urlAuthority = this.utilityService.punyHostPSLDomain(parsed); + return urlAuthority === targetAuthority; + } catch { + return false; + } }) .sort((a, b) => { return rankUrlType(a.type) - rankUrlType(b.type); @@ -76,44 +81,6 @@ export class ApUtilityService { return acceptableUrls[0]?.url ?? null; } - - /** - * Verifies that a provided URL is in a format acceptable for federation. - * @throws {IdentifiableError} If URL cannot be parsed - * @throws {IdentifiableError} If URL is not HTTPS - */ - public assertApUrl(url: string | URL): void { - // If string, parse and validate - if (typeof(url) === 'string') { - try { - url = new URL(url); - } catch { - throw new IdentifiableError('0bedd29b-e3bf-4604-af51-d3352e2518af', `invalid AP url ${url}: not a valid URL`); - } - } - - // Must be HTTPS - if (!this.checkHttps(url)) { - throw new IdentifiableError('0bedd29b-e3bf-4604-af51-d3352e2518af', `invalid AP url ${url}: unsupported protocol ${url.protocol}`); - } - } - - /** - * Checks if the URL contains HTTPS. - * Additionally, allows HTTP in non-production environments. - * Based on check-https.ts. - */ - private checkHttps(url: string | URL): boolean { - const isNonProd = this.envService.env.NODE_ENV !== 'production'; - - try { - const proto = new URL(url).protocol; - return proto === 'https:' || (proto === 'http:' && isNonProd); - } catch { - // Invalid URLs don't "count" as HTTPS - return false; - } - } } function isAcceptableUrlType(type: string | undefined): boolean { diff --git a/packages/backend/src/core/activitypub/models/ApNoteService.ts b/packages/backend/src/core/activitypub/models/ApNoteService.ts index 2a28405121..a67b02a9dc 100644 --- a/packages/backend/src/core/activitypub/models/ApNoteService.ts +++ b/packages/backend/src/core/activitypub/models/ApNoteService.ts @@ -96,7 +96,7 @@ export class ApNoteService { actor?: MiRemoteUser, user?: MiRemoteUser, ): Error | null { - this.apUtilityService.assertApUrl(uri); + this.utilityService.assertUrl(uri); const expectHost = this.utilityService.extractDbHost(uri); const apType = getApType(object); diff --git a/packages/backend/src/core/activitypub/models/ApPersonService.ts b/packages/backend/src/core/activitypub/models/ApPersonService.ts index 29f7459219..b715d90a21 100644 --- a/packages/backend/src/core/activitypub/models/ApPersonService.ts +++ b/packages/backend/src/core/activitypub/models/ApPersonService.ts @@ -155,7 +155,7 @@ export class ApPersonService implements OnModuleInit, OnApplicationShutdown { */ @bindThis private validateActor(x: IObject, uri: string): IActor { - this.apUtilityService.assertApUrl(uri); + this.utilityService.assertUrl(uri); const expectHost = this.utilityService.punyHostPSLDomain(uri); if (!isActor(x)) { @@ -170,7 +170,7 @@ export class ApPersonService implements OnModuleInit, OnApplicationShutdown { throw new UnrecoverableError(`invalid Actor ${uri}: wrong inbox type`); } - this.apUtilityService.assertApUrl(x.inbox); + this.utilityService.assertUrl(x.inbox); const inboxHost = this.utilityService.punyHostPSLDomain(x.inbox); if (inboxHost !== expectHost) { throw new UnrecoverableError(`invalid Actor ${uri}: wrong inbox host ${inboxHost}`); @@ -179,7 +179,7 @@ export class ApPersonService implements OnModuleInit, OnApplicationShutdown { const sharedInboxObject = x.sharedInbox ?? (x.endpoints ? x.endpoints.sharedInbox : undefined); if (sharedInboxObject != null) { const sharedInbox = getApId(sharedInboxObject); - this.apUtilityService.assertApUrl(sharedInbox); + this.utilityService.assertUrl(sharedInbox); if (!(typeof sharedInbox === 'string' && sharedInbox.length > 0 && this.utilityService.punyHostPSLDomain(sharedInbox) === expectHost)) { throw new UnrecoverableError(`invalid Actor ${uri}: wrong shared inbox ${sharedInbox}`); } @@ -190,7 +190,7 @@ export class ApPersonService implements OnModuleInit, OnApplicationShutdown { if (xCollection != null) { const collectionUri = getApId(xCollection); if (typeof collectionUri === 'string' && collectionUri.length > 0) { - this.apUtilityService.assertApUrl(collectionUri); + this.utilityService.assertUrl(collectionUri); if (this.utilityService.punyHostPSLDomain(collectionUri) !== expectHost) { throw new UnrecoverableError(`invalid Actor ${uri}: wrong ${collection} host ${collectionUri}`); } diff --git a/packages/backend/test/unit/core/activitypub/ApUtilityService.ts b/packages/backend/test/unit/core/activitypub/ApUtilityService.ts index 325a94dc5a..a49ba35112 100644 --- a/packages/backend/test/unit/core/activitypub/ApUtilityService.ts +++ b/packages/backend/test/unit/core/activitypub/ApUtilityService.ts @@ -3,30 +3,40 @@ * SPDX-License-Identifier: AGPL-3.0-only */ -import type { UtilityService } from '@/core/UtilityService.js'; import type { IObject } from '@/core/activitypub/type.js'; import type { EnvService } from '@/core/EnvService.js'; +import type { MiMeta } from '@/models/Meta.js'; +import type { Config } from '@/config.js'; import { ApUtilityService } from '@/core/activitypub/ApUtilityService.js'; +import { UtilityService } from '@/core/UtilityService.js'; describe(ApUtilityService, () => { let serviceUnderTest: ApUtilityService; let env: Record; beforeEach(() => { - const utilityService = { - punyHostPSLDomain(input: string) { - const host = new URL(input).host; - const parts = host.split('.'); - return `${parts[parts.length - 2]}.${parts[parts.length - 1]}`; - }, - } as unknown as UtilityService; - env = {}; const envService = { env, } as unknown as EnvService; - serviceUnderTest = new ApUtilityService(utilityService, envService); + const config = { + host: 'example.com', + blockedHosts: [], + silencedHosts: [], + mediaSilencedHosts: [], + federationHosts: [], + bubbleInstances: [], + deliverSuspendedSoftware: [], + federation: 'all', + } as unknown as Config; + const meta = { + + } as MiMeta; + + const utilityService = new UtilityService(config, meta, envService); + + serviceUnderTest = new ApUtilityService(utilityService); }); describe('assertIdMatchesUrlAuthority', () => { From 3849e8c15aefd72e7fa2cea471f88143708f717e Mon Sep 17 00:00:00 2001 From: Hazelnoot Date: Fri, 4 Jul 2025 14:53:02 -0400 Subject: [PATCH 06/26] use shared URL verification in verifyLinkFields --- packages/backend/src/misc/verify-field-link.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/backend/src/misc/verify-field-link.ts b/packages/backend/src/misc/verify-field-link.ts index f9fc352806..f90b25248f 100644 --- a/packages/backend/src/misc/verify-field-link.ts +++ b/packages/backend/src/misc/verify-field-link.ts @@ -10,7 +10,7 @@ type Field = { name: string, value: string }; export async function verifyFieldLinks(fields: Field[], profile_url: string, httpRequestService: HttpRequestService): Promise { const verified_links = []; - for (const field_url of fields.filter(x => URL.canParse(x.value) && ['http:', 'https:'].includes((new URL(x.value).protocol)))) { + for (const field_url of fields) { try { const html = await httpRequestService.getHtml(field_url.value); From df0331ea04b7db7eed1f975d3b13fe60f7e8a6ba Mon Sep 17 00:00:00 2001 From: Hazelnoot Date: Fri, 4 Jul 2025 14:53:13 -0400 Subject: [PATCH 07/26] verify URLs in DownloadService --- packages/backend/src/core/DownloadService.ts | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/packages/backend/src/core/DownloadService.ts b/packages/backend/src/core/DownloadService.ts index cb5bdb6cb7..d1819f40a3 100644 --- a/packages/backend/src/core/DownloadService.ts +++ b/packages/backend/src/core/DownloadService.ts @@ -19,6 +19,7 @@ import type Logger from '@/logger.js'; import { bindThis } from '@/decorators.js'; import { renderInlineError } from '@/misc/render-inline-error.js'; +import { UtilityService } from '@/core/UtilityService.js'; @Injectable() export class DownloadService { @@ -30,6 +31,7 @@ export class DownloadService { private httpRequestService: HttpRequestService, private loggerService: LoggerService, + private readonly utilityService: UtilityService, ) { this.logger = this.loggerService.getLogger('download'); } @@ -38,6 +40,8 @@ export class DownloadService { public async downloadUrl(url: string, path: string, options: { timeout?: number, operationTimeout?: number, maxSize?: number } = {} ): Promise<{ filename: string; }> { + this.utilityService.assertUrl(url); + this.logger.debug(`Downloading ${chalk.cyan(url)} to ${chalk.cyanBright(path)} ...`); const timeout = options.timeout ?? 30 * 1000; From d3f672657ed9ec17b297d6836d3c1b80705b8561 Mon Sep 17 00:00:00 2001 From: Hazelnoot Date: Fri, 4 Jul 2025 14:56:32 -0400 Subject: [PATCH 08/26] re-use parsed URI in validateActor --- .../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 b715d90a21..1b29bed3ab 100644 --- a/packages/backend/src/core/activitypub/models/ApPersonService.ts +++ b/packages/backend/src/core/activitypub/models/ApPersonService.ts @@ -155,8 +155,8 @@ export class ApPersonService implements OnModuleInit, OnApplicationShutdown { */ @bindThis private validateActor(x: IObject, uri: string): IActor { - this.utilityService.assertUrl(uri); - const expectHost = this.utilityService.punyHostPSLDomain(uri); + const parsedUri = this.utilityService.assertUrl(uri); + const expectHost = this.utilityService.punyHostPSLDomain(parsedUri); if (!isActor(x)) { throw new UnrecoverableError(`invalid Actor ${uri}: unknown type '${x.type}'`); From d36b94c8cf6b1d4068857ded82e418f771c38803 Mon Sep 17 00:00:00 2001 From: Hazelnoot Date: Fri, 4 Jul 2025 15:17:09 -0400 Subject: [PATCH 09/26] fix URL errors from incorrect validation in validateActor --- .../activitypub/models/ApPersonService.ts | 99 +++++++++++++++---- 1 file changed, 80 insertions(+), 19 deletions(-) diff --git a/packages/backend/src/core/activitypub/models/ApPersonService.ts b/packages/backend/src/core/activitypub/models/ApPersonService.ts index 1b29bed3ab..05bd97a3e3 100644 --- a/packages/backend/src/core/activitypub/models/ApPersonService.ts +++ b/packages/backend/src/core/activitypub/models/ApPersonService.ts @@ -45,7 +45,8 @@ import { HttpRequestService } from '@/core/HttpRequestService.js'; import { verifyFieldLinks } from '@/misc/verify-field-link.js'; import { isRetryableError } from '@/misc/is-retryable-error.js'; import { renderInlineError } from '@/misc/render-inline-error.js'; -import { getApId, getApType, isActor, isCollection, isCollectionOrOrderedCollection, isPropertyValue } from '../type.js'; +import { IdentifiableError } from '@/misc/identifiable-error.js'; +import { getApId, getApType, getNullableApId, isActor, isCollection, isCollectionOrOrderedCollection, isPropertyValue } from '../type.js'; import { extractApHashtags } from './tag.js'; import type { OnModuleInit } from '@nestjs/common'; import type { ApNoteService } from './ApNoteService.js'; @@ -176,27 +177,85 @@ export class ApPersonService implements OnModuleInit, OnApplicationShutdown { throw new UnrecoverableError(`invalid Actor ${uri}: wrong inbox host ${inboxHost}`); } - const sharedInboxObject = x.sharedInbox ?? (x.endpoints ? x.endpoints.sharedInbox : undefined); - if (sharedInboxObject != null) { - const sharedInbox = getApId(sharedInboxObject); - this.utilityService.assertUrl(sharedInbox); - if (!(typeof sharedInbox === 'string' && sharedInbox.length > 0 && this.utilityService.punyHostPSLDomain(sharedInbox) === expectHost)) { - throw new UnrecoverableError(`invalid Actor ${uri}: wrong shared inbox ${sharedInbox}`); + // Sanitize sharedInbox + try { + if (x.sharedInbox) { + const sharedInbox = getNullableApId(x.sharedInbox); + if (sharedInbox) { + const parsed = this.utilityService.assertUrl(sharedInbox); + if (this.utilityService.punyHostPSLDomain(parsed) !== expectHost) { + this.logger.warn(`Excluding sharedInbox for actor ${uri}: wrong host in ${sharedInbox}`); + x.sharedInbox = undefined; + } + } else { + this.logger.warn(`Excluding sharedInbox for actor ${uri}: missing ID`); + x.sharedInbox = undefined; + } + } else { + // Collapse all falsy values to undefined + x.sharedInbox = undefined; + } + } catch { + // Shared inbox is unparseable - strip out + x.sharedInbox = undefined; + } + + // Sanitize endpoints object + if (typeof(x.endpoints) === 'object') { + x.endpoints = { + sharedInbox: x.endpoints.sharedInbox, + }; + } else { + x.endpoints = undefined; + } + + // Sanitize endpoints.sharedInbox + if (x.endpoints) { + try { + if (x.endpoints.sharedInbox) { + const sharedInbox = getNullableApId(x.endpoints.sharedInbox); + if (sharedInbox) { + const parsed = this.utilityService.assertUrl(sharedInbox); + if (this.utilityService.punyHostPSLDomain(parsed) !== expectHost) { + this.logger.warn(`Excluding endpoints.sharedInbox for actor ${uri}: wrong host in ${sharedInbox}`); + x.endpoints.sharedInbox = undefined; + } + } else { + this.logger.warn(`Excluding endpoints.sharedInbox for actor ${uri}: missing ID`); + x.endpoints.sharedInbox = undefined; + } + } else { + // Collapse all falsy values to undefined + x.endpoints.sharedInbox = undefined; + } + } catch { + // Shared inbox is unparseable - strip out + x.endpoints.sharedInbox = undefined; } } - for (const collection of ['outbox', 'followers', 'following'] as (keyof IActor)[]) { - const xCollection = (x as IActor)[collection]; - if (xCollection != null) { - const collectionUri = getApId(xCollection); - if (typeof collectionUri === 'string' && collectionUri.length > 0) { - this.utilityService.assertUrl(collectionUri); - if (this.utilityService.punyHostPSLDomain(collectionUri) !== expectHost) { - throw new UnrecoverableError(`invalid Actor ${uri}: wrong ${collection} host ${collectionUri}`); + // Sanitize collections + for (const collection of ['outbox', 'followers', 'following', 'featured'] as (keyof IActor)[]) { + try { + if (x[collection]) { + const collectionUri = getNullableApId(x[collection]); + if (collectionUri) { + const parsed = this.utilityService.assertUrl(collectionUri); + if (this.utilityService.punyHostPSLDomain(parsed) !== expectHost) { + this.logger.warn(`Excluding ${collection} for actor ${uri}: wrong host in ${collectionUri}`); + x[collection] = undefined; + } + } else { + this.logger.warn(`Excluding ${collection} for actor ${uri}: missing ID`); + x[collection] = undefined; } - } else if (collectionUri != null) { - throw new UnrecoverableError(`invalid Actor ${uri}: wrong ${collection} type`); + } else { + // Collapse all falsy values to undefined + x[collection] = undefined; } + } catch { + // Collection is unparseable - strip out + x[collection] = undefined; } } @@ -223,7 +282,8 @@ export class ApPersonService implements OnModuleInit, OnApplicationShutdown { x.summary = truncate(x.summary, summaryLength); } - const idHost = this.utilityService.punyHostPSLDomain(x.id); + const parsedId = this.utilityService.assertUrl(x.id); + const idHost = this.utilityService.punyHostPSLDomain(parsedId); if (idHost !== expectHost) { throw new UnrecoverableError(`invalid Actor ${uri}: wrong id ${x.id}`); } @@ -233,7 +293,8 @@ export class ApPersonService implements OnModuleInit, OnApplicationShutdown { throw new UnrecoverableError(`invalid Actor ${uri}: wrong publicKey.id type`); } - const publicKeyIdHost = this.utilityService.punyHostPSLDomain(x.publicKey.id); + const parsed = this.utilityService.assertUrl(x.publicKey.id); + const publicKeyIdHost = this.utilityService.punyHostPSLDomain(parsed); if (publicKeyIdHost !== expectHost) { throw new UnrecoverableError(`invalid Actor ${uri}: wrong publicKey.id ${x.publicKey.id}`); } From 283facdd31e1dc43fb457a97cdb8677d38e08bcf Mon Sep 17 00:00:00 2001 From: Hazelnoot Date: Tue, 17 Jun 2025 20:31:00 -0400 Subject: [PATCH 10/26] add workarounds for node-fetch crashes --- packages/backend/src/boot/entry.ts | 22 ++++++++++++++++++++-- 1 file changed, 20 insertions(+), 2 deletions(-) diff --git a/packages/backend/src/boot/entry.ts b/packages/backend/src/boot/entry.ts index afb48e526c..af4a813833 100644 --- a/packages/backend/src/boot/entry.ts +++ b/packages/backend/src/boot/entry.ts @@ -63,14 +63,32 @@ async function main() { }); } + process.on('uncaughtException', (err) => { + // Workaround for https://github.com/node-fetch/node-fetch/issues/954 + if (String(err).match(/^TypeError: .+ is an? url with embedded credentials.$/)) { + console.debug('Suppressed node-fetch issue#954, but the current job may fail.'); + return; + } + + // Workaround for https://github.com/node-fetch/node-fetch/issues/1845 + if (String(err) === 'TypeError: Cannot read properties of undefined (reading \'body\')') { + console.debug('Suppressed node-fetch issue#1845, but the current job may fail.'); + return; + } + + // Throw all other errors to avoid inconsistent state. + // (per NodeJS docs, it's unsafe to suppress arbitrary errors in an uncaughtException handler.) + throw err; + }); + // Display detail of uncaught exception - process.on('uncaughtExceptionMonitor', ((err, origin) => { + process.on('uncaughtExceptionMonitor', (err, origin) => { try { logger.error(`Uncaught exception (${origin}):`, err); } catch { console.error(`Uncaught exception (${origin}):`, err); } - })); + }); // Dying away... process.on('disconnect', () => { From bf455c2f7a3122ab72bf7ef7498d758936b1db91 Mon Sep 17 00:00:00 2001 From: Hazelnoot Date: Mon, 7 Jul 2025 11:44:48 -0400 Subject: [PATCH 11/26] use logger instead of console for uncaughtException debug lines --- packages/backend/src/boot/entry.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/backend/src/boot/entry.ts b/packages/backend/src/boot/entry.ts index af4a813833..530c75cc8e 100644 --- a/packages/backend/src/boot/entry.ts +++ b/packages/backend/src/boot/entry.ts @@ -66,13 +66,13 @@ async function main() { process.on('uncaughtException', (err) => { // Workaround for https://github.com/node-fetch/node-fetch/issues/954 if (String(err).match(/^TypeError: .+ is an? url with embedded credentials.$/)) { - console.debug('Suppressed node-fetch issue#954, but the current job may fail.'); + logger.debug('Suppressed node-fetch issue#954, but the current job may fail.'); return; } // Workaround for https://github.com/node-fetch/node-fetch/issues/1845 if (String(err) === 'TypeError: Cannot read properties of undefined (reading \'body\')') { - console.debug('Suppressed node-fetch issue#1845, but the current job may fail.'); + logger.debug('Suppressed node-fetch issue#1845, but the current job may fail.'); return; } From e8c71341237b21cde103c0c0a9a67647ba605bac Mon Sep 17 00:00:00 2001 From: Hazelnoot Date: Mon, 7 Jul 2025 11:45:01 -0400 Subject: [PATCH 12/26] remove unused console logging fallbacks --- packages/backend/src/boot/entry.ts | 30 +++++------------------------- 1 file changed, 5 insertions(+), 25 deletions(-) diff --git a/packages/backend/src/boot/entry.ts b/packages/backend/src/boot/entry.ts index 530c75cc8e..bbe6a57383 100644 --- a/packages/backend/src/boot/entry.ts +++ b/packages/backend/src/boot/entry.ts @@ -55,11 +55,7 @@ async function main() { // Display detail of unhandled promise rejection if (!envOption.quiet) { process.on('unhandledRejection', e => { - try { - logger.error('Unhandled rejection:', inspect(e)); - } catch { - console.error('Unhandled rejection:', inspect(e)); - } + logger.error('Unhandled rejection:', inspect(e)); }); } @@ -83,34 +79,18 @@ async function main() { // Display detail of uncaught exception process.on('uncaughtExceptionMonitor', (err, origin) => { - try { - logger.error(`Uncaught exception (${origin}):`, err); - } catch { - console.error(`Uncaught exception (${origin}):`, err); - } + logger.error(`Uncaught exception (${origin}):`, err); }); // Dying away... process.on('disconnect', () => { - try { - logger.warn('IPC channel disconnected! The process may soon die.'); - } catch { - console.warn('IPC channel disconnected! The process may soon die.'); - } + logger.warn('IPC channel disconnected! The process may soon die.'); }); process.on('beforeExit', code => { - try { - logger.warn(`Event loop died! Process will exit with code ${code}.`); - } catch { - console.warn(`Event loop died! Process will exit with code ${code}.`); - } + logger.warn(`Event loop died! Process will exit with code ${code}.`); }); process.on('exit', code => { - try { - logger.info(`The process is going to exit with code ${code}`); - } catch { - console.info(`The process is going to exit with code ${code}`); - } + logger.info(`The process is going to exit with code ${code}`); }); //#endregion From dc19b181123bfe2e92ca8f7edaee13215724c7fc Mon Sep 17 00:00:00 2001 From: Hazelnoot Date: Mon, 7 Jul 2025 11:46:35 -0400 Subject: [PATCH 13/26] add comment about validation in verify-field-link.ts --- packages/backend/src/misc/verify-field-link.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/backend/src/misc/verify-field-link.ts b/packages/backend/src/misc/verify-field-link.ts index f90b25248f..37161f16e5 100644 --- a/packages/backend/src/misc/verify-field-link.ts +++ b/packages/backend/src/misc/verify-field-link.ts @@ -12,6 +12,7 @@ export async function verifyFieldLinks(fields: Field[], profile_url: string, htt const verified_links = []; for (const field_url of fields) { try { + // getHtml validates the input URL, so we can safely pass in untrusted values const html = await httpRequestService.getHtml(field_url.value); const doc = cheerio(html); From 3dde7f25a6d1696c449b4c1d4e781697cf7e5b95 Mon Sep 17 00:00:00 2001 From: Hazelnoot Date: Tue, 8 Jul 2025 11:01:37 -0400 Subject: [PATCH 14/26] move CaptchaError to a separate file to avoid circular import errors --- packages/backend/src/core/CaptchaService.ts | 17 ++++------------- packages/backend/src/misc/captcha-error.ts | 18 ++++++++++++++++++ .../backend/src/misc/render-inline-error.ts | 2 +- 3 files changed, 23 insertions(+), 14 deletions(-) create mode 100644 packages/backend/src/misc/captcha-error.ts diff --git a/packages/backend/src/core/CaptchaService.ts b/packages/backend/src/core/CaptchaService.ts index c526a80aeb..020984a37f 100644 --- a/packages/backend/src/core/CaptchaService.ts +++ b/packages/backend/src/core/CaptchaService.ts @@ -9,7 +9,10 @@ import { bindThis } from '@/decorators.js'; import { MetaService } from '@/core/MetaService.js'; import { MiMeta } from '@/models/Meta.js'; import Logger from '@/logger.js'; -import { LoggerService } from './LoggerService.js'; +import { LoggerService } from '@/core/LoggerService.js'; +import { CaptchaError } from '@/misc/captcha-error.js'; + +export { CaptchaError } from '@/misc/captcha-error.js'; export const supportedCaptchaProviders = ['none', 'hcaptcha', 'mcaptcha', 'recaptcha', 'turnstile', 'fc', 'testcaptcha'] as const; export type CaptchaProvider = typeof supportedCaptchaProviders[number]; @@ -49,18 +52,6 @@ export type CaptchaSetting = { } }; -export class CaptchaError extends Error { - public readonly code: CaptchaErrorCode; - public readonly cause?: unknown; - - constructor(code: CaptchaErrorCode, message: string, cause?: unknown) { - super(message, cause ? { cause } : undefined); - this.code = code; - this.cause = cause; - this.name = 'CaptchaError'; - } -} - export type CaptchaSaveSuccess = { success: true; }; diff --git a/packages/backend/src/misc/captcha-error.ts b/packages/backend/src/misc/captcha-error.ts new file mode 100644 index 0000000000..217018ec68 --- /dev/null +++ b/packages/backend/src/misc/captcha-error.ts @@ -0,0 +1,18 @@ +/* + * SPDX-FileCopyrightText: syuilo and misskey-project + * SPDX-License-Identifier: AGPL-3.0-only + */ + +import type { CaptchaErrorCode } from '@/core/CaptchaService.js'; + +export class CaptchaError extends Error { + public readonly code: CaptchaErrorCode; + public readonly cause?: unknown; + + constructor(code: CaptchaErrorCode, message: string, cause?: unknown) { + super(message, cause ? { cause } : undefined); + this.code = code; + this.cause = cause; + this.name = 'CaptchaError'; + } +} diff --git a/packages/backend/src/misc/render-inline-error.ts b/packages/backend/src/misc/render-inline-error.ts index 07f9f3068e..886efcb86e 100644 --- a/packages/backend/src/misc/render-inline-error.ts +++ b/packages/backend/src/misc/render-inline-error.ts @@ -5,7 +5,7 @@ import { IdentifiableError } from '@/misc/identifiable-error.js'; import { StatusError } from '@/misc/status-error.js'; -import { CaptchaError } from '@/core/CaptchaService.js'; +import { CaptchaError } from '@/misc/captcha-error.js'; export function renderInlineError(err: unknown): string { const parts: string[] = []; From f937f2d3c677c6634c29d28de1e5f8dbe15d1176 Mon Sep 17 00:00:00 2001 From: Hazelnoot Date: Wed, 23 Jul 2025 15:45:32 -0400 Subject: [PATCH 15/26] fix error in UserSuspendService.freezeAll and UserSuspendService.unFreezeAll caused by TypeORM bug --- packages/backend/src/core/UserSuspendService.ts | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/packages/backend/src/core/UserSuspendService.ts b/packages/backend/src/core/UserSuspendService.ts index ddadab7022..0a592e5024 100644 --- a/packages/backend/src/core/UserSuspendService.ts +++ b/packages/backend/src/core/UserSuspendService.ts @@ -178,10 +178,8 @@ export class UserSuspendService { // Freeze follow relations with all remote users await this.followingsRepository .createQueryBuilder('following') - .orWhere({ - followeeId: user.id, - followerHost: Not(IsNull()), - }) + .andWhere('following."followeeId" = :id', { id: user.id }) + .andWhere('following."followerHost" IS NOT NULL') .update({ isFollowerHibernated: true, }) @@ -195,10 +193,8 @@ export class UserSuspendService { .createQueryBuilder('following') .innerJoin(MiUser, 'follower', 'user.id = following.followerId') .andWhere('follower.isHibernated = false') // Don't unfreeze if the follower is *actually* frozen - .andWhere({ - followeeId: user.id, - followerHost: Not(IsNull()), - }) + .andWhere('following."followeeId" = :id', { id: user.id }) + .andWhere('following."followerHost" IS NOT NULL') .update({ isFollowerHibernated: false, }) From 73f2ee4fb3209471869b9d1f5e7b4d438214d886 Mon Sep 17 00:00:00 2001 From: Hazelnoot Date: Wed, 23 Jul 2025 16:10:00 -0400 Subject: [PATCH 16/26] fix user suspension / unsuspension not updating caches --- packages/backend/src/core/UserSuspendService.ts | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/packages/backend/src/core/UserSuspendService.ts b/packages/backend/src/core/UserSuspendService.ts index 0a592e5024..1b54fb5e8a 100644 --- a/packages/backend/src/core/UserSuspendService.ts +++ b/packages/backend/src/core/UserSuspendService.ts @@ -21,6 +21,7 @@ import { LoggerService } from '@/core/LoggerService.js'; import type Logger from '@/logger.js'; import { renderInlineError } from '@/misc/render-inline-error.js'; import { trackPromise } from '@/misc/promise-tracker.js'; +import { InternalEventService } from '@/core/InternalEventService.js'; @Injectable() export class UserSuspendService { @@ -42,6 +43,7 @@ export class UserSuspendService { private apRendererService: ApRendererService, private moderationLogService: ModerationLogService, private readonly cacheService: CacheService, + private readonly internalEventService: InternalEventService, loggerService: LoggerService, ) { @@ -56,6 +58,8 @@ export class UserSuspendService { isSuspended: true, }); + await this.internalEventService.emit(user.host == null ? 'localUserUpdated' : 'remoteUserUpdated', { id: user.id }); + await this.moderationLogService.log(moderator, 'suspend', { userId: user.id, userUsername: user.username, @@ -74,6 +78,8 @@ export class UserSuspendService { isSuspended: false, }); + await this.internalEventService.emit(user.host == null ? 'localUserUpdated' : 'remoteUserUpdated', { id: user.id }); + await this.moderationLogService.log(moderator, 'unsuspend', { userId: user.id, userUsername: user.username, From ea9335bcc8f59b187140f7c9ea876389ac0af9a4 Mon Sep 17 00:00:00 2001 From: Hazelnoot Date: Wed, 23 Jul 2025 16:10:14 -0400 Subject: [PATCH 17/26] fix more freeze / unfreeze errors caused by TypeORM bugs --- .../backend/src/core/UserSuspendService.ts | 32 +++++++++++-------- 1 file changed, 19 insertions(+), 13 deletions(-) diff --git a/packages/backend/src/core/UserSuspendService.ts b/packages/backend/src/core/UserSuspendService.ts index 1b54fb5e8a..5868ba6678 100644 --- a/packages/backend/src/core/UserSuspendService.ts +++ b/packages/backend/src/core/UserSuspendService.ts @@ -4,7 +4,7 @@ */ import { Inject, Injectable } from '@nestjs/common'; -import { Not, IsNull } from 'typeorm'; +import { Not, IsNull, DataSource } from 'typeorm'; import type { FollowingsRepository, FollowRequestsRepository, UsersRepository } from '@/models/_.js'; import { MiUser } from '@/models/User.js'; import { QueueService } from '@/core/QueueService.js'; @@ -37,6 +37,9 @@ export class UserSuspendService { @Inject(DI.followRequestsRepository) private followRequestsRepository: FollowRequestsRepository, + @Inject(DI.db) + private db: DataSource, + private userEntityService: UserEntityService, private queueService: QueueService, private globalEventService: GlobalEventService, @@ -184,26 +187,29 @@ export class UserSuspendService { // Freeze follow relations with all remote users await this.followingsRepository .createQueryBuilder('following') - .andWhere('following."followeeId" = :id', { id: user.id }) - .andWhere('following."followerHost" IS NOT NULL') .update({ isFollowerHibernated: true, }) + .where({ + followeeId: user.id, + followerHost: Not(IsNull()), + }) .execute(); } @bindThis private async unFreezeAll(user: MiUser): Promise { // Restore follow relations with all remote users - await this.followingsRepository - .createQueryBuilder('following') - .innerJoin(MiUser, 'follower', 'user.id = following.followerId') - .andWhere('follower.isHibernated = false') // Don't unfreeze if the follower is *actually* frozen - .andWhere('following."followeeId" = :id', { id: user.id }) - .andWhere('following."followerHost" IS NOT NULL') - .update({ - isFollowerHibernated: false, - }) - .execute(); + + // TypeORM does not support UPDATE with JOIN: https://github.com/typeorm/typeorm/issues/564#issuecomment-310331468 + await this.db.query(` + UPDATE "following" + SET "isFollowerHibernated" = false + FROM "user" + WHERE "user"."id" = "following"."followerId" + AND "user"."isHibernated" = false -- Don't unfreeze if the follower is *actually* frozen + AND "followeeId" = $1 + AND "followeeHost" IS NOT NULL + `, [user.id]); } } From 2c8c422cb6d27515fdebf42f19f1d85a7fdac3fe Mon Sep 17 00:00:00 2001 From: Hazelnoot Date: Fri, 4 Jul 2025 10:00:40 -0400 Subject: [PATCH 18/26] include profile URI for link verification --- .../src/core/activitypub/models/ApPersonService.ts | 6 ++++-- packages/backend/src/misc/verify-field-link.ts | 4 ++-- packages/backend/src/server/api/endpoints/i/update.ts | 8 ++++++-- 3 files changed, 12 insertions(+), 6 deletions(-) diff --git a/packages/backend/src/core/activitypub/models/ApPersonService.ts b/packages/backend/src/core/activitypub/models/ApPersonService.ts index 29f7459219..bc602bbd5b 100644 --- a/packages/backend/src/core/activitypub/models/ApPersonService.ts +++ b/packages/backend/src/core/activitypub/models/ApPersonService.ts @@ -377,7 +377,8 @@ export class ApPersonService implements OnModuleInit, OnApplicationShutdown { const url = this.apUtilityService.findBestObjectUrl(person); - const verifiedLinks = url ? await verifyFieldLinks(fields, url, this.httpRequestService) : []; + const profileUrls = url ? [url, person.id] : [person.id]; + const verifiedLinks = await verifyFieldLinks(fields, profileUrls, this.httpRequestService); // Create user let user: MiRemoteUser | null = null; @@ -626,7 +627,8 @@ export class ApPersonService implements OnModuleInit, OnApplicationShutdown { const url = this.apUtilityService.findBestObjectUrl(person); - const verifiedLinks = url ? await verifyFieldLinks(fields, url, this.httpRequestService) : []; + const profileUrls = url ? [url, person.id] : [person.id]; + const verifiedLinks = await verifyFieldLinks(fields, profileUrls, this.httpRequestService); const updates = { lastFetchedAt: new Date(), diff --git a/packages/backend/src/misc/verify-field-link.ts b/packages/backend/src/misc/verify-field-link.ts index f9fc352806..6a3c950059 100644 --- a/packages/backend/src/misc/verify-field-link.ts +++ b/packages/backend/src/misc/verify-field-link.ts @@ -8,7 +8,7 @@ import type { HttpRequestService } from '@/core/HttpRequestService.js'; type Field = { name: string, value: string }; -export async function verifyFieldLinks(fields: Field[], profile_url: string, httpRequestService: HttpRequestService): Promise { +export async function verifyFieldLinks(fields: Field[], profileUrls: string[], httpRequestService: HttpRequestService): Promise { const verified_links = []; for (const field_url of fields.filter(x => URL.canParse(x.value) && ['http:', 'https:'].includes((new URL(x.value).protocol)))) { try { @@ -18,7 +18,7 @@ export async function verifyFieldLinks(fields: Field[], profile_url: string, htt const links = doc('a[rel~="me"][href], link[rel~="me"][href]').toArray(); - const includesProfileLinks = links.some(link => link.attribs.href === profile_url); + const includesProfileLinks = links.some(link => profileUrls.includes(link.attribs.href)); if (includesProfileLinks) { verified_links.push(field_url.value); } diff --git a/packages/backend/src/server/api/endpoints/i/update.ts b/packages/backend/src/server/api/endpoints/i/update.ts index 5767880531..65dcf6301f 100644 --- a/packages/backend/src/server/api/endpoints/i/update.ts +++ b/packages/backend/src/server/api/endpoints/i/update.ts @@ -603,11 +603,15 @@ export default class extends Endpoint { // eslint- this.globalEventService.publishInternalEvent('localUserUpdated', { id: user.id }); } - const verified_links = await verifyFieldLinks(newFields, `${this.config.url}/@${user.username}`, this.httpRequestService); + const profileUrls = [ + this.userEntityService.genLocalUserUri(user.id), + `${this.config.url}/@${user.username}`, + ]; + const verifiedLinks = await verifyFieldLinks(newFields, profileUrls, this.httpRequestService); await this.userProfilesRepository.update(user.id, { ...profileUpdates, - verifiedLinks: verified_links, + verifiedLinks, }); const iObj = await this.userEntityService.pack(user.id, user, { From 9ac58e6107b3178669bdd0b150a217c5bf8b51cb Mon Sep 17 00:00:00 2001 From: Hazelnoot Date: Sun, 6 Jul 2025 20:25:05 -0400 Subject: [PATCH 19/26] scale rate limit dripRate with factor --- .../src/server/SkRateLimiterService.md | 2 +- .../src/server/SkRateLimiterService.ts | 2 +- .../server/api/SkRateLimiterServiceTests.ts | 36 +++++++++++++------ 3 files changed, 28 insertions(+), 12 deletions(-) diff --git a/packages/backend/src/server/SkRateLimiterService.md b/packages/backend/src/server/SkRateLimiterService.md index 55786664e1..4ec097ea8e 100644 --- a/packages/backend/src/server/SkRateLimiterService.md +++ b/packages/backend/src/server/SkRateLimiterService.md @@ -81,7 +81,7 @@ The Atomic Leaky Bucket algorithm is described here, in pseudocode: # * Delta Timestamp - Difference between current and expected timestamp value # 0 - Calculations -dripRate = ceil(limit.dripRate ?? 1000); +dripRate = ceil((limit.dripRate ?? 1000) * factor); dripSize = ceil(limit.dripSize ?? 1); bucketSize = max(ceil(limit.size / factor), 1); maxExpiration = max(ceil((dripRate * ceil(bucketSize / dripSize)) / 1000), 1);; diff --git a/packages/backend/src/server/SkRateLimiterService.ts b/packages/backend/src/server/SkRateLimiterService.ts index 35e87b0fe8..a53c58ba5a 100644 --- a/packages/backend/src/server/SkRateLimiterService.ts +++ b/packages/backend/src/server/SkRateLimiterService.ts @@ -206,7 +206,7 @@ export class SkRateLimiterService { // 0 - Calculate const now = this.timeService.now; const bucketSize = Math.max(Math.ceil(limit.size / factor), 1); - const dripRate = Math.ceil(limit.dripRate ?? 1000); + const dripRate = Math.ceil((limit.dripRate ?? 1000) * factor); const dripSize = Math.ceil(limit.dripSize ?? 1); const fullResetMs = dripRate * Math.ceil(bucketSize / dripSize); const fullResetSec = Math.max(Math.ceil(fullResetMs / 1000), 1); diff --git a/packages/backend/test/unit/server/api/SkRateLimiterServiceTests.ts b/packages/backend/test/unit/server/api/SkRateLimiterServiceTests.ts index b1f100698b..f7250600e3 100644 --- a/packages/backend/test/unit/server/api/SkRateLimiterServiceTests.ts +++ b/packages/backend/test/unit/server/api/SkRateLimiterServiceTests.ts @@ -303,9 +303,12 @@ describe(SkRateLimiterService, () => { const i1 = await serviceUnderTest().limit(limit, actor); // 1 + 1 = 2 const i2 = await serviceUnderTest().limit(limit, actor); // 2 + 1 = 3 + mockTimeService.now += 500; // 3 - 1 = 2 (at 1/2 time) + const i3 = await serviceUnderTest().limit(limit, actor); expect(i1.blocked).toBeFalsy(); expect(i2.blocked).toBeTruthy(); + expect(i3.blocked).toBeFalsy(); }); it('should set counter expiration', async () => { @@ -563,11 +566,15 @@ describe(SkRateLimiterService, () => { mockDefaultUserPolicies.rateLimitFactor = 0.5; limitCounter = 1; limitTimestamp = 0; + + const i1 = await serviceUnderTest().limit(limit, actor); + const i2 = await serviceUnderTest().limit(limit, actor); mockTimeService.now += 500; + const i3 = await serviceUnderTest().limit(limit, actor); - const info = await serviceUnderTest().limit(limit, actor); - - expect(info.blocked).toBeFalsy(); + expect(i1.blocked).toBeFalsy(); + expect(i2.blocked).toBeTruthy(); + expect(i3.blocked).toBeFalsy(); }); it('should set counter expiration', async () => { @@ -738,12 +745,17 @@ describe(SkRateLimiterService, () => { it('should scale limit by factor', async () => { mockDefaultUserPolicies.rateLimitFactor = 0.5; - limitCounter = 10; + limitCounter = 1; limitTimestamp = 0; - const info = await serviceUnderTest().limit(limit, actor); // 10 + 1 = 11 + const i1 = await serviceUnderTest().limit(limit, actor); + const i2 = await serviceUnderTest().limit(limit, actor); + mockTimeService.now += 500; + const i3 = await serviceUnderTest().limit(limit, actor); - expect(info.blocked).toBeTruthy(); + expect(i1.blocked).toBeFalsy(); + expect(i2.blocked).toBeTruthy(); + expect(i3.blocked).toBeFalsy(); }); it('should set counter expiration', async () => { @@ -932,13 +944,17 @@ describe(SkRateLimiterService, () => { it('should scale limit and interval by factor', async () => { mockDefaultUserPolicies.rateLimitFactor = 0.5; - limitCounter = 5; + limitCounter = 19; limitTimestamp = 0; + + const i1 = await serviceUnderTest().limit(limit, actor); + const i2 = await serviceUnderTest().limit(limit, actor); mockTimeService.now += 500; + const i3 = await serviceUnderTest().limit(limit, actor); - const info = await serviceUnderTest().limit(limit, actor); - - expect(info.blocked).toBeFalsy(); + expect(i1.blocked).toBeFalsy(); + expect(i2.blocked).toBeTruthy(); + expect(i3.blocked).toBeFalsy(); }); it('should set counter expiration', async () => { From 84ca3621d8ff2566c4d37bb7c1a6b4a923faed91 Mon Sep 17 00:00:00 2001 From: Hazelnoot Date: Wed, 9 Jul 2025 19:03:13 -0400 Subject: [PATCH 20/26] fix users/report-abuse endpoint being really slow --- packages/backend/src/core/AbuseReportService.ts | 5 +++-- .../src/server/api/endpoints/users/report-abuse.ts | 10 ++++++---- 2 files changed, 9 insertions(+), 6 deletions(-) diff --git a/packages/backend/src/core/AbuseReportService.ts b/packages/backend/src/core/AbuseReportService.ts index bccb9f86f6..8b3c596f50 100644 --- a/packages/backend/src/core/AbuseReportService.ts +++ b/packages/backend/src/core/AbuseReportService.ts @@ -14,6 +14,7 @@ import { ApRendererService } from '@/core/activitypub/ApRendererService.js'; import { ModerationLogService } from '@/core/ModerationLogService.js'; import { SystemAccountService } from '@/core/SystemAccountService.js'; import { IdentifiableError } from '@/misc/identifiable-error.js'; +import { trackPromise } from '@/misc/promise-tracker.js'; import { IdService } from './IdService.js'; @Injectable() @@ -68,11 +69,11 @@ export class AbuseReportService { reports.push(report); } - return Promise.all([ + trackPromise(Promise.all([ this.abuseReportNotificationService.notifyAdminStream(reports), this.abuseReportNotificationService.notifySystemWebhook(reports, 'abuseReport'), this.abuseReportNotificationService.notifyMail(reports), - ]); + ])); } /** 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 81c0c526f0..fc2b57c4a5 100644 --- a/packages/backend/src/server/api/endpoints/users/report-abuse.ts +++ b/packages/backend/src/server/api/endpoints/users/report-abuse.ts @@ -9,6 +9,7 @@ import { GetterService } from '@/server/api/GetterService.js'; import { RoleService } from '@/core/RoleService.js'; import { AbuseReportService } from '@/core/AbuseReportService.js'; import { ApiError } from '../../error.js'; +import { CacheService } from '@/core/CacheService.js'; export const meta = { tags: ['users'], @@ -60,13 +61,14 @@ export default class extends Endpoint { // eslint- private getterService: GetterService, private roleService: RoleService, private abuseReportService: AbuseReportService, + private readonly cacheService: CacheService, ) { super(meta, paramDef, async (ps, me) => { // Lookup user - const targetUser = await this.getterService.getUser(ps.userId).catch(err => { - if (err.id === '15348ddd-432d-49c2-8a5a-8069753becff') throw new ApiError(meta.errors.noSuchUser); - throw err; - }); + const targetUser = await this.cacheService.findOptionalUserById(ps.userId); + if (!targetUser) { + throw new ApiError(meta.errors.noSuchUser); + } if (targetUser.id === me.id) { throw new ApiError(meta.errors.cannotReportYourself); From ec11092e8df36fd7f87f622e40ad1c49f5c34c97 Mon Sep 17 00:00:00 2001 From: Hazelnoot Date: Sat, 26 Jul 2025 18:47:33 -0400 Subject: [PATCH 21/26] fix cherry-pick error: restore CacheService.findOptionalUserById --- packages/backend/src/core/CacheService.ts | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/packages/backend/src/core/CacheService.ts b/packages/backend/src/core/CacheService.ts index 2d37cd6bab..84f6df3e21 100644 --- a/packages/backend/src/core/CacheService.ts +++ b/packages/backend/src/core/CacheService.ts @@ -350,6 +350,11 @@ export class CacheService implements OnApplicationShutdown { }) ?? null; } + @bindThis + public findOptionalUserById(userId: MiUser['id']) { + return this.userByIdCache.fetchMaybe(userId, async () => await this.usersRepository.findOneBy({ id: userId }) ?? undefined); + } + @bindThis public async getFollowStats(userId: MiUser['id']): Promise { return await this.userFollowStatsCache.fetch(userId, async () => { From 8994a95f1358df63050a0c81861eaab6b164bedd Mon Sep 17 00:00:00 2001 From: dakkar Date: Sun, 27 Jul 2025 18:50:22 +0100 Subject: [PATCH 22/26] bump release version --- package.json | 2 +- packages/misskey-js/package.json | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/package.json b/package.json index 24777d9d87..5055eff10f 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "sharkey", - "version": "2025.4.3", + "version": "2025.4.4", "codename": "shonk", "repository": { "type": "git", diff --git a/packages/misskey-js/package.json b/packages/misskey-js/package.json index 48d5912a07..059c7c2eae 100644 --- a/packages/misskey-js/package.json +++ b/packages/misskey-js/package.json @@ -1,7 +1,7 @@ { "type": "module", "name": "misskey-js", - "version": "2025.4.3", + "version": "2025.4.4", "description": "Misskey SDK for JavaScript", "license": "MIT", "main": "./built/index.js", From af967fe6bee8e06470f10bc1a0e274fc0db9d66c Mon Sep 17 00:00:00 2001 From: Hazelnoot Date: Tue, 8 Jul 2025 11:01:56 -0400 Subject: [PATCH 23/26] refactor actor validation to reduce code duplication --- packages/backend/src/core/UtilityService.ts | 4 +- .../src/core/activitypub/ApUtilityService.ts | 86 ++++++++++- .../activitypub/models/ApPersonService.ts | 143 +++++------------- packages/backend/src/core/activitypub/type.ts | 6 +- .../test/unit/FetchInstanceMetadataService.ts | 2 + .../unit/core/activitypub/ApUtilityService.ts | 112 +++++++++++++- 6 files changed, 243 insertions(+), 110 deletions(-) diff --git a/packages/backend/src/core/UtilityService.ts b/packages/backend/src/core/UtilityService.ts index 281edccca3..27d3cb7776 100644 --- a/packages/backend/src/core/UtilityService.ts +++ b/packages/backend/src/core/UtilityService.ts @@ -10,8 +10,8 @@ import psl from 'psl'; import { DI } from '@/di-symbols.js'; import type { Config } from '@/config.js'; import { bindThis } from '@/decorators.js'; -import { MiMeta, SoftwareSuspension } from '@/models/Meta.js'; -import { MiInstance } from '@/models/Instance.js'; +import type { MiMeta } from '@/models/Meta.js'; +import type { MiInstance } from '@/models/Instance.js'; import { IdentifiableError } from '@/misc/identifiable-error.js'; import { EnvService } from '@/core/EnvService.js'; diff --git a/packages/backend/src/core/activitypub/ApUtilityService.ts b/packages/backend/src/core/activitypub/ApUtilityService.ts index eede48f0c6..26ea0cd632 100644 --- a/packages/backend/src/core/activitypub/ApUtilityService.ts +++ b/packages/backend/src/core/activitypub/ApUtilityService.ts @@ -7,18 +7,29 @@ import { Injectable } from '@nestjs/common'; import { UtilityService } from '@/core/UtilityService.js'; import { IdentifiableError } from '@/misc/identifiable-error.js'; import { toArray } from '@/misc/prelude/array.js'; -import { getApId, getOneApHrefNullable, IObject } from '@/core/activitypub/type.js'; +import { getApId, getNullableApId, getOneApHrefNullable } from '@/core/activitypub/type.js'; +import type { IObject, IObjectWithId } from '@/core/activitypub/type.js'; +import { bindThis } from '@/decorators.js'; +import { renderInlineError } from '@/misc/render-inline-error.js'; +import { LoggerService } from '@/core/LoggerService.js'; +import type Logger from '@/logger.js'; @Injectable() export class ApUtilityService { + private readonly logger: Logger; + constructor( private readonly utilityService: UtilityService, - ) {} + loggerService: LoggerService, + ) { + this.logger = loggerService.getLogger('ap-utility'); + } /** * Verifies that the object's ID has the same authority as the provided URL. * Returns on success, throws on any validation error. */ + @bindThis public assertIdMatchesUrlAuthority(object: IObject, url: string): void { // This throws if the ID is missing or invalid, but that's ok. // Anonymous objects are impossible to verify, so we don't allow fetching them. @@ -34,6 +45,7 @@ export class ApUtilityService { /** * Checks if two URLs have the same host authority */ + @bindThis public haveSameAuthority(url1: string, url2: string): boolean { if (url1 === url2) return true; @@ -51,6 +63,7 @@ export class ApUtilityService { * @throws {IdentifiableError} if object does not have an ID * @returns the best URL, or null if none were found */ + @bindThis public findBestObjectUrl(object: IObject): string | null { const targetUrl = getApId(object); const targetAuthority = this.utilityService.punyHostPSLDomain(targetUrl); @@ -81,6 +94,75 @@ export class ApUtilityService { return acceptableUrls[0]?.url ?? null; } + + /** + * Sanitizes an inline / nested Object property within an AP object. + * + * Returns true if the property contains a valid string URL, object w/ valid ID, or an array containing one of those. + * Returns false and erases the property if it doesn't contain a valid value. + * + * Arrays are automatically flattened. + * Falsy values (including null) are collapsed to undefined. + * @param obj Object containing the property to validate + * @param key Key of the property in obj + * @param parentUri URI of the object that contains this inline object. + * @param parentHost PSL host of parentUri + * @param keyPath If obj is *itself* a nested object, set this to the property path from root to obj (including the trailing '.'). This does not affect the logic, but improves the clarity of logs. + */ + @bindThis + public sanitizeInlineObject(obj: Partial>, key: Key, parentUri: string | URL, parentHost: string, keyPath = ''): obj is Partial> { + let value: unknown = obj[key]; + + // Unpack arrays + if (Array.isArray(value)) { + value = value[0]; + } + + // Clear the value - we'll add it back once we have a confirmed ID + obj[key] = undefined; + + // Collapse falsy values to undefined + if (!value) { + return false; + } + + // Exclude nested arrays + if (Array.isArray(value)) { + this.logger.warn(`Excluding ${keyPath}${key} from object ${parentUri}: nested arrays are prohibited`); + return false; + } + + // Exclude incorrect types + if (typeof(value) !== 'string' && typeof(value) !== 'object') { + this.logger.warn(`Excluding ${keyPath}${key} from object ${parentUri}: incorrect type ${typeof(value)}`); + return false; + } + + const valueId = getNullableApId(value); + if (!valueId) { + // Exclude missing ID + this.logger.warn(`Excluding ${keyPath}${key} from object ${parentUri}: missing or invalid ID`); + return false; + } + + try { + const parsed = this.utilityService.assertUrl(valueId); + const parsedHost = this.utilityService.punyHostPSLDomain(parsed); + if (parsedHost !== parentHost) { + // Exclude wrong host + this.logger.warn(`Excluding ${keyPath}${key} from object ${parentUri}: wrong host in ${valueId} (got ${parsedHost}, expected ${parentHost})`); + return false; + } + } catch (err) { + // Exclude invalid URLs + this.logger.warn(`Excluding ${keyPath}${key} from object ${parentUri}: invalid URL ${valueId}: ${renderInlineError(err)}`); + return false; + } + + // Success - store the sanitized value and return + obj[key] = value as string | IObjectWithId; + return true; + } } function isAcceptableUrlType(type: string | undefined): boolean { diff --git a/packages/backend/src/core/activitypub/models/ApPersonService.ts b/packages/backend/src/core/activitypub/models/ApPersonService.ts index 05bd97a3e3..b3fd17e7a0 100644 --- a/packages/backend/src/core/activitypub/models/ApPersonService.ts +++ b/packages/backend/src/core/activitypub/models/ApPersonService.ts @@ -46,7 +46,7 @@ import { verifyFieldLinks } from '@/misc/verify-field-link.js'; import { isRetryableError } from '@/misc/is-retryable-error.js'; import { renderInlineError } from '@/misc/render-inline-error.js'; import { IdentifiableError } from '@/misc/identifiable-error.js'; -import { getApId, getApType, getNullableApId, isActor, isCollection, isCollectionOrOrderedCollection, isPropertyValue } from '../type.js'; +import { getApId, getApType, isActor, isCollection, isCollectionOrOrderedCollection, isPropertyValue } from '../type.js'; import { extractApHashtags } from './tag.js'; import type { OnModuleInit } from '@nestjs/common'; import type { ApNoteService } from './ApNoteService.js'; @@ -159,46 +159,32 @@ export class ApPersonService implements OnModuleInit, OnApplicationShutdown { const parsedUri = this.utilityService.assertUrl(uri); const expectHost = this.utilityService.punyHostPSLDomain(parsedUri); + // Validate type if (!isActor(x)) { throw new UnrecoverableError(`invalid Actor ${uri}: unknown type '${x.type}'`); } - if (!(typeof x.id === 'string' && x.id.length > 0)) { - throw new UnrecoverableError(`invalid Actor ${uri}: wrong id type`); + // Validate id + if (!x.id) { + throw new UnrecoverableError(`invalid Actor ${uri}: missing id`); + } + if (typeof(x.id) !== 'string') { + throw new UnrecoverableError(`invalid Actor ${uri}: wrong id type ${typeof(x.id)}`); + } + const parsedId = this.utilityService.assertUrl(x.id); + const idHost = this.utilityService.punyHostPSLDomain(parsedId); + if (idHost !== expectHost) { + throw new UnrecoverableError(`invalid Actor ${uri}: wrong host in id ${x.id} (got ${parsedId}, expected ${expectHost})`); } - if (!(typeof x.inbox === 'string' && x.inbox.length > 0)) { - throw new UnrecoverableError(`invalid Actor ${uri}: wrong inbox type`); - } - - this.utilityService.assertUrl(x.inbox); - const inboxHost = this.utilityService.punyHostPSLDomain(x.inbox); - if (inboxHost !== expectHost) { - throw new UnrecoverableError(`invalid Actor ${uri}: wrong inbox host ${inboxHost}`); + // Validate inbox + this.apUtilityService.sanitizeInlineObject(x, 'inbox', parsedUri, expectHost); + if (!x.inbox || typeof(x.inbox) !== 'string') { + throw new UnrecoverableError(`invalid Actor ${uri}: missing or invalid inbox ${x.inbox}`); } // Sanitize sharedInbox - try { - if (x.sharedInbox) { - const sharedInbox = getNullableApId(x.sharedInbox); - if (sharedInbox) { - const parsed = this.utilityService.assertUrl(sharedInbox); - if (this.utilityService.punyHostPSLDomain(parsed) !== expectHost) { - this.logger.warn(`Excluding sharedInbox for actor ${uri}: wrong host in ${sharedInbox}`); - x.sharedInbox = undefined; - } - } else { - this.logger.warn(`Excluding sharedInbox for actor ${uri}: missing ID`); - x.sharedInbox = undefined; - } - } else { - // Collapse all falsy values to undefined - x.sharedInbox = undefined; - } - } catch { - // Shared inbox is unparseable - strip out - x.sharedInbox = undefined; - } + this.apUtilityService.sanitizeInlineObject(x, 'sharedInbox', parsedUri, expectHost); // Sanitize endpoints object if (typeof(x.endpoints) === 'object') { @@ -211,94 +197,47 @@ export class ApPersonService implements OnModuleInit, OnApplicationShutdown { // Sanitize endpoints.sharedInbox if (x.endpoints) { - try { - if (x.endpoints.sharedInbox) { - const sharedInbox = getNullableApId(x.endpoints.sharedInbox); - if (sharedInbox) { - const parsed = this.utilityService.assertUrl(sharedInbox); - if (this.utilityService.punyHostPSLDomain(parsed) !== expectHost) { - this.logger.warn(`Excluding endpoints.sharedInbox for actor ${uri}: wrong host in ${sharedInbox}`); - x.endpoints.sharedInbox = undefined; - } - } else { - this.logger.warn(`Excluding endpoints.sharedInbox for actor ${uri}: missing ID`); - x.endpoints.sharedInbox = undefined; - } - } else { - // Collapse all falsy values to undefined - x.endpoints.sharedInbox = undefined; - } - } catch { - // Shared inbox is unparseable - strip out - x.endpoints.sharedInbox = undefined; + this.apUtilityService.sanitizeInlineObject(x.endpoints, 'sharedInbox', parsedUri, expectHost, 'endpoints.'); + + if (!x.endpoints.sharedInbox) { + x.endpoints = undefined; } } // Sanitize collections - for (const collection of ['outbox', 'followers', 'following', 'featured'] as (keyof IActor)[]) { - try { - if (x[collection]) { - const collectionUri = getNullableApId(x[collection]); - if (collectionUri) { - const parsed = this.utilityService.assertUrl(collectionUri); - if (this.utilityService.punyHostPSLDomain(parsed) !== expectHost) { - this.logger.warn(`Excluding ${collection} for actor ${uri}: wrong host in ${collectionUri}`); - x[collection] = undefined; - } - } else { - this.logger.warn(`Excluding ${collection} for actor ${uri}: missing ID`); - x[collection] = undefined; - } - } else { - // Collapse all falsy values to undefined - x[collection] = undefined; - } - } catch { - // Collection is unparseable - strip out - x[collection] = undefined; - } + for (const collection of ['outbox', 'followers', 'following', 'featured'] as const) { + this.apUtilityService.sanitizeInlineObject(x, collection, parsedUri, expectHost); } + // Validate username if (!(typeof x.preferredUsername === 'string' && x.preferredUsername.length > 0 && x.preferredUsername.length <= 128 && /^\w([\w-.]*\w)?$/.test(x.preferredUsername))) { throw new UnrecoverableError(`invalid Actor ${uri}: wrong username`); } + // Sanitize name // These fields are only informational, and some AP software allows these // fields to be very long. If they are too long, we cut them off. This way // we can at least see these users and their activities. - if (x.name) { - if (!(typeof x.name === 'string' && x.name.length > 0)) { - throw new UnrecoverableError(`invalid Actor ${uri}: wrong name`); - } - x.name = truncate(x.name, nameLength); - } else if (x.name === '') { - // Mastodon emits empty string when the name is not set. + if (!x.name) { x.name = undefined; + } else if (typeof(x.name) !== 'string') { + this.logger.warn(`Excluding name from object ${uri}: incorrect type ${typeof(x)}`); + x.name = undefined; + } else { + x.name = truncate(x.name, nameLength); } - if (x.summary) { - if (!(typeof x.summary === 'string' && x.summary.length > 0)) { - throw new UnrecoverableError(`invalid Actor ${uri}: wrong summary`); - } + + // Sanitize summary + if (!x.summary) { + x.summary = undefined; + } else if (typeof(x.summary) !== 'string') { + this.logger.warn(`Excluding summary from object ${uri}: incorrect type ${typeof(x)}`); + } else { x.summary = truncate(x.summary, summaryLength); } - const parsedId = this.utilityService.assertUrl(x.id); - const idHost = this.utilityService.punyHostPSLDomain(parsedId); - if (idHost !== expectHost) { - throw new UnrecoverableError(`invalid Actor ${uri}: wrong id ${x.id}`); - } - - if (x.publicKey) { - if (typeof x.publicKey.id !== 'string') { - throw new UnrecoverableError(`invalid Actor ${uri}: wrong publicKey.id type`); - } - - const parsed = this.utilityService.assertUrl(x.publicKey.id); - const publicKeyIdHost = this.utilityService.punyHostPSLDomain(parsed); - if (publicKeyIdHost !== expectHost) { - throw new UnrecoverableError(`invalid Actor ${uri}: wrong publicKey.id ${x.publicKey.id}`); - } - } + // Sanitize publicKey + this.apUtilityService.sanitizeInlineObject(x, 'publicKey', parsedUri, expectHost); return x; } diff --git a/packages/backend/src/core/activitypub/type.ts b/packages/backend/src/core/activitypub/type.ts index 554420d670..ebe29a4fe6 100644 --- a/packages/backend/src/core/activitypub/type.ts +++ b/packages/backend/src/core/activitypub/type.ts @@ -86,7 +86,7 @@ export function getOneApId(value: ApObject): string { /** * Get ActivityStreams Object id */ -export function getApId(value: string | IObject | [string | IObject], sourceForLogs?: string): string { +export function getApId(value: unknown | [unknown] | unknown[], sourceForLogs?: string): string { const id = getNullableApId(value); if (id == null) { @@ -102,7 +102,7 @@ export function getApId(value: string | IObject | [string | IObject], sourceForL /** * Get ActivityStreams Object id, or null if not present */ -export function getNullableApId(source: string | IObject | [string | IObject]): string | null { +export function getNullableApId(source: unknown | [unknown] | unknown[]): string | null { const value: unknown = fromTuple(source); if (value != null) { @@ -276,7 +276,7 @@ export interface IActor extends IObject { followers?: string | ICollection | IOrderedCollection; following?: string | ICollection | IOrderedCollection; featured?: string | IOrderedCollection; - outbox: string | IOrderedCollection; + outbox?: string | IOrderedCollection; endpoints?: { sharedInbox?: string; }; diff --git a/packages/backend/test/unit/FetchInstanceMetadataService.ts b/packages/backend/test/unit/FetchInstanceMetadataService.ts index 1e3605aafc..812ee38703 100644 --- a/packages/backend/test/unit/FetchInstanceMetadataService.ts +++ b/packages/backend/test/unit/FetchInstanceMetadataService.ts @@ -16,6 +16,7 @@ import { HttpRequestService } from '@/core/HttpRequestService.js'; import { LoggerService } from '@/core/LoggerService.js'; import { UtilityService } from '@/core/UtilityService.js'; import { IdService } from '@/core/IdService.js'; +import { EnvService } from '@/core/EnvService.js'; import { DI } from '@/di-symbols.js'; function mockRedis() { @@ -46,6 +47,7 @@ describe('FetchInstanceMetadataService', () => { LoggerService, UtilityService, IdService, + EnvService, ], }) .useMocker((token) => { diff --git a/packages/backend/test/unit/core/activitypub/ApUtilityService.ts b/packages/backend/test/unit/core/activitypub/ApUtilityService.ts index a49ba35112..7b564b1fdd 100644 --- a/packages/backend/test/unit/core/activitypub/ApUtilityService.ts +++ b/packages/backend/test/unit/core/activitypub/ApUtilityService.ts @@ -7,6 +7,8 @@ import type { IObject } from '@/core/activitypub/type.js'; import type { EnvService } from '@/core/EnvService.js'; import type { MiMeta } from '@/models/Meta.js'; import type { Config } from '@/config.js'; +import type { LoggerService } from '@/core/LoggerService.js'; +import Logger from '@/logger.js'; import { ApUtilityService } from '@/core/activitypub/ApUtilityService.js'; import { UtilityService } from '@/core/UtilityService.js'; @@ -36,7 +38,17 @@ describe(ApUtilityService, () => { const utilityService = new UtilityService(config, meta, envService); - serviceUnderTest = new ApUtilityService(utilityService); + const loggerService = { + getLogger(domain: string) { + const logger = new Logger(domain); + Object.defineProperty(logger, 'log', { + value: () => {}, + }); + return logger; + }, + } as unknown as LoggerService; + + serviceUnderTest = new ApUtilityService(utilityService, loggerService); }); describe('assertIdMatchesUrlAuthority', () => { @@ -361,4 +373,102 @@ describe(ApUtilityService, () => { expect(result).toBe('http://example.com/1'); }); }); + + describe('sanitizeInlineObject', () => { + it('should exclude nested arrays', () => { + const input = { + test: [[]] as unknown as string[], + }; + + const result = serviceUnderTest.sanitizeInlineObject(input, 'test', 'https://example.com/actor', 'example.com'); + + expect(result).toBe(false); + }); + + it('should exclude incorrect type', () => { + const input = { + test: 0 as unknown as string, + }; + + const result = serviceUnderTest.sanitizeInlineObject(input, 'test', 'https://example.com/actor', 'example.com'); + + expect(result).toBe(false); + }); + + it('should exclude missing ID', () => { + const input = { + test: { + id: undefined, + }, + }; + + const result = serviceUnderTest.sanitizeInlineObject(input, 'test', 'https://example.com/actor', 'example.com'); + + expect(result).toBe(false); + }); + + it('should exclude wrong host', () => { + const input = { + test: 'https://wrong.com/object', + }; + + const result = serviceUnderTest.sanitizeInlineObject(input, 'test', 'https://example.com/actor', 'example.com'); + + expect(result).toBe(false); + }); + + it('should exclude invalid URLs', () => { + const input = { + test: 'https://user@example.com/object', + }; + + const result = serviceUnderTest.sanitizeInlineObject(input, 'test', 'https://example.com/actor', 'example.com'); + + expect(result).toBe(false); + }); + + it('should accept string', () => { + const input = { + test: 'https://example.com/object', + }; + + const result = serviceUnderTest.sanitizeInlineObject(input, 'test', 'https://example.com/actor', 'example.com'); + + expect(result).toBe(true); + }); + + it('should accept array of string', () => { + const input = { + test: ['https://example.com/object'], + }; + + const result = serviceUnderTest.sanitizeInlineObject(input, 'test', 'https://example.com/actor', 'example.com'); + + expect(result).toBe(true); + }); + + it('should accept object', () => { + const input = { + test: { + id: 'https://example.com/object', + }, + }; + + const result = serviceUnderTest.sanitizeInlineObject(input, 'test', 'https://example.com/actor', 'example.com'); + + expect(result).toBe(true); + }); + + it('should accept array of object', () => { + const input = { + test: [{ + id: 'https://example.com/object', + }], + }; + + const result = serviceUnderTest.sanitizeInlineObject(input, 'test', 'https://example.com/actor', 'example.com'); + + expect(result).toBe(true); + }); + }); }); From 3c59a7ae01f7ea7094178cf961c3f4a668672f08 Mon Sep 17 00:00:00 2001 From: Hazelnoot Date: Tue, 8 Jul 2025 11:27:31 -0400 Subject: [PATCH 24/26] allow HTTP connections to private IPs --- .../backend/src/core/HttpRequestService.ts | 17 +++++++++--- packages/backend/src/core/UtilityService.ts | 8 +++--- .../test/unit/core/HttpRequestService.ts | 26 +++++++++++++++++-- 3 files changed, 42 insertions(+), 9 deletions(-) diff --git a/packages/backend/src/core/HttpRequestService.ts b/packages/backend/src/core/HttpRequestService.ts index 046b0dc244..bbc127fa86 100644 --- a/packages/backend/src/core/HttpRequestService.ts +++ b/packages/backend/src/core/HttpRequestService.ts @@ -20,7 +20,6 @@ import type { IObject, IObjectWithId } from '@/core/activitypub/type.js'; import { UtilityService } from '@/core/UtilityService.js'; import { ApUtilityService } from '@/core/activitypub/ApUtilityService.js'; import type { Response } from 'node-fetch'; -import type { URL } from 'node:url'; import type { Socket } from 'node:net'; export type HttpRequestSendOptions = { @@ -28,6 +27,15 @@ export type HttpRequestSendOptions = { validators?: ((res: Response) => void)[]; }; +export function isPrivateUrl(url: URL): boolean { + if (!ipaddr.isValid(url.hostname)) { + return false; + } + + const ip = ipaddr.parse(url.hostname); + return ip.range() !== 'unicast'; +} + export function isPrivateIp(allowedPrivateNetworks: PrivateNetwork[] | undefined, ip: string, port?: number): boolean { const parsedIp = ipaddr.parse(ip); @@ -303,6 +311,7 @@ export class HttpRequestService { timeout?: number, size?: number, isLocalAddressAllowed?: boolean, + allowHttp?: boolean, } = {}, extra: HttpRequestSendOptions = { throwErrorWhenResponseNotOk: true, @@ -311,7 +320,9 @@ export class HttpRequestService { ): Promise { const timeout = args.timeout ?? 5000; - this.utilityService.assertUrl(url); + const parsedUrl = new URL(url); + const allowHttp = args.allowHttp || isPrivateUrl(parsedUrl); + this.utilityService.assertUrl(parsedUrl, allowHttp); const controller = new AbortController(); setTimeout(() => { @@ -320,7 +331,7 @@ export class HttpRequestService { const isLocalAddressAllowed = args.isLocalAddressAllowed ?? false; - const res = await fetch(url, { + const res = await fetch(parsedUrl, { method: args.method ?? 'GET', headers: { 'User-Agent': this.config.userAgent, diff --git a/packages/backend/src/core/UtilityService.ts b/packages/backend/src/core/UtilityService.ts index 27d3cb7776..a90774cf59 100644 --- a/packages/backend/src/core/UtilityService.ts +++ b/packages/backend/src/core/UtilityService.ts @@ -226,7 +226,7 @@ export class UtilityService { * @throws {IdentifiableError} If URL contains credentials */ @bindThis - public assertUrl(url: string | URL): URL | never { + public assertUrl(url: string | URL, allowHttp?: boolean): URL | never { // If string, parse and validate if (typeof(url) === 'string') { try { @@ -237,7 +237,7 @@ export class UtilityService { } // Must be HTTPS - if (!this.checkHttps(url)) { + if (!this.checkHttps(url, allowHttp)) { throw new IdentifiableError('0bedd29b-e3bf-4604-af51-d3352e2518af', `invalid url ${url}: unsupported protocol ${url.protocol}`); } @@ -255,12 +255,12 @@ export class UtilityService { * Based on check-https.ts. */ @bindThis - public checkHttps(url: string | URL): boolean { + public checkHttps(url: string | URL, allowHttp = false): boolean { const isNonProd = this.envService.env.NODE_ENV !== 'production'; try { const proto = new URL(url).protocol; - return proto === 'https:' || (proto === 'http:' && isNonProd); + return proto === 'https:' || (proto === 'http:' && (isNonProd || allowHttp)); } catch { // Invalid URLs don't "count" as HTTPS return false; diff --git a/packages/backend/test/unit/core/HttpRequestService.ts b/packages/backend/test/unit/core/HttpRequestService.ts index a2f4604e7b..5aeeb1e26f 100644 --- a/packages/backend/test/unit/core/HttpRequestService.ts +++ b/packages/backend/test/unit/core/HttpRequestService.ts @@ -3,11 +3,11 @@ * SPDX-License-Identifier: AGPL-3.0-only */ -import { jest } from '@jest/globals'; +import { describe, jest } from '@jest/globals'; import type { Mock } from 'jest-mock'; import type { PrivateNetwork } from '@/config.js'; import type { Socket } from 'net'; -import { HttpRequestService, isPrivateIp, validateSocketConnect } from '@/core/HttpRequestService.js'; +import { HttpRequestService, isPrivateIp, isPrivateUrl, validateSocketConnect } from '@/core/HttpRequestService.js'; import { parsePrivateNetworks } from '@/config.js'; describe(HttpRequestService, () => { @@ -53,6 +53,28 @@ describe(HttpRequestService, () => { }); }); + describe('isPrivateUrl', () => { + it('should return false when URL is not an IP', () => { + const result = isPrivateUrl(new URL('https://example.com')); + expect(result).toBe(false); + }); + + it('should return false when IP is public', () => { + const result = isPrivateUrl(new URL('https://23.192.228.80')); + expect(result).toBe(false); + }); + + it('should return true when IP is private', () => { + const result = isPrivateUrl(new URL('https://127.0.0.1')); + expect(result).toBe(true); + }); + + it('should return true when IP is private with port and path', () => { + const result = isPrivateUrl(new URL('https://127.0.0.1:443/some/path')); + expect(result).toBe(true); + }); + }); + describe('validateSocketConnect', () => { let fakeSocket: Socket; let fakeSocketMutable: { From 25622b536c982f717c72b0cf039322d45c36a55b Mon Sep 17 00:00:00 2001 From: Hazelnoot Date: Fri, 25 Jul 2025 16:28:53 -0400 Subject: [PATCH 25/26] resolve domain names when checking for private URLs --- .../backend/src/core/HttpRequestService.ts | 32 ++++++--- .../activitypub/models/ApPersonService.ts | 1 - .../test/unit/core/HttpRequestService.ts | 66 +++++++++++++------ 3 files changed, 69 insertions(+), 30 deletions(-) diff --git a/packages/backend/src/core/HttpRequestService.ts b/packages/backend/src/core/HttpRequestService.ts index bbc127fa86..34aaada9f2 100644 --- a/packages/backend/src/core/HttpRequestService.ts +++ b/packages/backend/src/core/HttpRequestService.ts @@ -27,16 +27,27 @@ export type HttpRequestSendOptions = { validators?: ((res: Response) => void)[]; }; -export function isPrivateUrl(url: URL): boolean { - if (!ipaddr.isValid(url.hostname)) { - return false; - } - - const ip = ipaddr.parse(url.hostname); +export async function isPrivateUrl(url: URL, lookup: net.LookupFunction): Promise { + const ip = await resolveIp(url, lookup); return ip.range() !== 'unicast'; } -export function isPrivateIp(allowedPrivateNetworks: PrivateNetwork[] | undefined, ip: string, port?: number): boolean { +export async function resolveIp(url: URL, lookup: net.LookupFunction) { + if (ipaddr.isValid(url.hostname)) { + return ipaddr.parse(url.hostname); + } + + const resolvedIp = await new Promise((resolve, reject) => { + lookup(url.hostname, {}, (err, address) => { + if (err) reject(err); + else resolve(address as string); + }); + }); + + return ipaddr.parse(resolvedIp); +} + +export function isAllowedPrivateIp(allowedPrivateNetworks: PrivateNetwork[] | undefined, ip: string, port?: number): boolean { const parsedIp = ipaddr.parse(ip); for (const { cidr, ports } of allowedPrivateNetworks ?? []) { @@ -53,7 +64,7 @@ export function isPrivateIp(allowedPrivateNetworks: PrivateNetwork[] | undefined export function validateSocketConnect(allowedPrivateNetworks: PrivateNetwork[] | undefined, socket: Socket): void { const address = socket.remoteAddress; if (address && ipaddr.isValid(address)) { - if (isPrivateIp(allowedPrivateNetworks, address, socket.remotePort)) { + if (isAllowedPrivateIp(allowedPrivateNetworks, address, socket.remotePort)) { socket.destroy(new Error(`Blocked address: ${address}`)); } } @@ -142,6 +153,7 @@ export class HttpRequestService { private config: Config, private readonly apUtilityService: ApUtilityService, private readonly utilityService: UtilityService, + private readonly lookup: net.LookupFunction, ) { const cache = new CacheableLookup({ maxTtl: 3600, // 1hours @@ -149,6 +161,8 @@ export class HttpRequestService { lookup: false, // nativeのdns.lookupにfallbackしない }); + this.lookup = cache.lookup as unknown as net.LookupFunction; + const agentOption = { keepAlive: true, keepAliveMsecs: 30 * 1000, @@ -321,7 +335,7 @@ export class HttpRequestService { const timeout = args.timeout ?? 5000; const parsedUrl = new URL(url); - const allowHttp = args.allowHttp || isPrivateUrl(parsedUrl); + const allowHttp = args.allowHttp || await isPrivateUrl(parsedUrl, this.lookup); this.utilityService.assertUrl(parsedUrl, allowHttp); const controller = new AbortController(); diff --git a/packages/backend/src/core/activitypub/models/ApPersonService.ts b/packages/backend/src/core/activitypub/models/ApPersonService.ts index b3fd17e7a0..30124949bb 100644 --- a/packages/backend/src/core/activitypub/models/ApPersonService.ts +++ b/packages/backend/src/core/activitypub/models/ApPersonService.ts @@ -56,7 +56,6 @@ import type { ApLoggerService } from '../ApLoggerService.js'; import type { ApImageService } from './ApImageService.js'; import type { IActor, ICollection, IObject, IOrderedCollection } from '../type.js'; -import { IdentifiableError } from '@/misc/identifiable-error.js'; const nameLength = 128; const summaryLength = 2048; diff --git a/packages/backend/test/unit/core/HttpRequestService.ts b/packages/backend/test/unit/core/HttpRequestService.ts index 5aeeb1e26f..0759306666 100644 --- a/packages/backend/test/unit/core/HttpRequestService.ts +++ b/packages/backend/test/unit/core/HttpRequestService.ts @@ -7,8 +7,9 @@ import { describe, jest } from '@jest/globals'; import type { Mock } from 'jest-mock'; import type { PrivateNetwork } from '@/config.js'; import type { Socket } from 'net'; -import { HttpRequestService, isPrivateIp, isPrivateUrl, validateSocketConnect } from '@/core/HttpRequestService.js'; +import { HttpRequestService, isAllowedPrivateIp, isPrivateUrl, resolveIp, validateSocketConnect } from '@/core/HttpRequestService.js'; import { parsePrivateNetworks } from '@/config.js'; +import { IPv4 } from 'ipaddr.js'; describe(HttpRequestService, () => { let allowedPrivateNetworks: PrivateNetwork[] | undefined; @@ -21,56 +22,81 @@ describe(HttpRequestService, () => { ]); }); - describe('isPrivateIp', () => { + describe(isAllowedPrivateIp, () => { it('should return false when ip public', () => { - const result = isPrivateIp(allowedPrivateNetworks, '74.125.127.100', 80); + const result = isAllowedPrivateIp(allowedPrivateNetworks, '74.125.127.100', 80); expect(result).toBeFalsy(); }); it('should return false when ip private and port matches', () => { - const result = isPrivateIp(allowedPrivateNetworks, '127.0.0.1', 1); + const result = isAllowedPrivateIp(allowedPrivateNetworks, '127.0.0.1', 1); expect(result).toBeFalsy(); }); it('should return false when ip private and all ports undefined', () => { - const result = isPrivateIp(allowedPrivateNetworks, '10.0.0.1', undefined); + const result = isAllowedPrivateIp(allowedPrivateNetworks, '10.0.0.1', undefined); expect(result).toBeFalsy(); }); it('should return true when ip private and no ports specified', () => { - const result = isPrivateIp(allowedPrivateNetworks, '10.0.0.2', 80); + const result = isAllowedPrivateIp(allowedPrivateNetworks, '10.0.0.2', 80); expect(result).toBeTruthy(); }); it('should return true when ip private and port does not match', () => { - const result = isPrivateIp(allowedPrivateNetworks, '127.0.0.1', 80); + const result = isAllowedPrivateIp(allowedPrivateNetworks, '127.0.0.1', 80); expect(result).toBeTruthy(); }); it('should return true when ip private and port is null but ports are specified', () => { - const result = isPrivateIp(allowedPrivateNetworks, '127.0.0.1', undefined); + const result = isAllowedPrivateIp(allowedPrivateNetworks, '127.0.0.1', undefined); expect(result).toBeTruthy(); }); }); - describe('isPrivateUrl', () => { - it('should return false when URL is not an IP', () => { - const result = isPrivateUrl(new URL('https://example.com')); + const fakeLookup = (host: string, _: unknown, callback: (err: Error | null, ip: string) => void) => { + if (host === 'localhost') { + callback(null, '127.0.0.1'); + } else { + callback(null, '23.192.228.80'); + } + }; + + describe(resolveIp, () => { + it('should parse inline IPs', async () => { + const result = await resolveIp(new URL('https://10.0.0.1'), fakeLookup); + expect(result.toString()).toEqual('10.0.0.1'); + }); + + it('should resolve domain names', async () => { + const result = await resolveIp(new URL('https://localhost'), fakeLookup); + expect(result.toString()).toEqual('127.0.0.1'); + }); + }); + + describe(isPrivateUrl, () => { + it('should return false when URL is public host', async () => { + const result = await isPrivateUrl(new URL('https://example.com'), fakeLookup); expect(result).toBe(false); }); - it('should return false when IP is public', () => { - const result = isPrivateUrl(new URL('https://23.192.228.80')); - expect(result).toBe(false); - }); - - it('should return true when IP is private', () => { - const result = isPrivateUrl(new URL('https://127.0.0.1')); + it('should return true when URL is private host', async () => { + const result = await isPrivateUrl(new URL('https://localhost'), fakeLookup); expect(result).toBe(true); }); - it('should return true when IP is private with port and path', () => { - const result = isPrivateUrl(new URL('https://127.0.0.1:443/some/path')); + it('should return false when IP is public', async () => { + const result = await isPrivateUrl(new URL('https://23.192.228.80'), fakeLookup); + expect(result).toBe(false); + }); + + it('should return true when IP is private', async () => { + const result = await isPrivateUrl(new URL('https://127.0.0.1'), fakeLookup); + expect(result).toBe(true); + }); + + it('should return true when IP is private with port and path', async () => { + const result = await isPrivateUrl(new URL('https://127.0.0.1:443/some/path'), fakeLookup); expect(result).toBe(true); }); }); From db15ac0fbb8bb4474f365a42a1df9056828d26b3 Mon Sep 17 00:00:00 2001 From: Hazelnoot Date: Fri, 25 Jul 2025 16:32:54 -0400 Subject: [PATCH 26/26] fix DI error in HttpRequestService.ts --- packages/backend/src/core/HttpRequestService.ts | 6 +++++- packages/backend/test/unit/core/HttpRequestService.ts | 1 - 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/packages/backend/src/core/HttpRequestService.ts b/packages/backend/src/core/HttpRequestService.ts index 34aaada9f2..bd72fefe4f 100644 --- a/packages/backend/src/core/HttpRequestService.ts +++ b/packages/backend/src/core/HttpRequestService.ts @@ -148,12 +148,16 @@ export class HttpRequestService { */ public readonly httpsAgent: https.Agent; + /** + * Get shared DNS resolver + */ + public readonly lookup: net.LookupFunction; + constructor( @Inject(DI.config) private config: Config, private readonly apUtilityService: ApUtilityService, private readonly utilityService: UtilityService, - private readonly lookup: net.LookupFunction, ) { const cache = new CacheableLookup({ maxTtl: 3600, // 1hours diff --git a/packages/backend/test/unit/core/HttpRequestService.ts b/packages/backend/test/unit/core/HttpRequestService.ts index 0759306666..ccce32ffee 100644 --- a/packages/backend/test/unit/core/HttpRequestService.ts +++ b/packages/backend/test/unit/core/HttpRequestService.ts @@ -9,7 +9,6 @@ import type { PrivateNetwork } from '@/config.js'; import type { Socket } from 'net'; import { HttpRequestService, isAllowedPrivateIp, isPrivateUrl, resolveIp, validateSocketConnect } from '@/core/HttpRequestService.js'; import { parsePrivateNetworks } from '@/config.js'; -import { IPv4 } from 'ipaddr.js'; describe(HttpRequestService, () => { let allowedPrivateNetworks: PrivateNetwork[] | undefined;