From 25052164c0971497f9177f88446b110e8ca91ce2 Mon Sep 17 00:00:00 2001
From: "饺子w (Yumechi)" <35571479+eternal-flame-AD@users.noreply.github.com>
Date: Sun, 23 Feb 2025 04:21:34 -0600
Subject: Merge commit from fork

* fix(backend): Fix an issue where the origin of ActivityPub lookup response was not validated correctly.

[GHSA-6w2c-vf6f-xf26](https://github.com/misskey-dev/misskey/security/advisories/GHSA-6w2c-vf6f-xf26)

Signed-off-by: eternal-flame-AD <yume@yumechi.jp>

* Enhance: Add configuration option to disable all external redirects when responding to an ActivityPub lookup (config.disallowExternalApRedirect)

Signed-off-by: eternal-flame-AD <yume@yumechi.jp>

* fixup! fix(backend): Fix an issue where the origin of ActivityPub lookup response was not validated correctly.

* docs & one edge case

Signed-off-by: eternal-flame-AD <yume@yumechi.jp>

* apply suggestions

Signed-off-by: eternal-flame-AD <yume@yumechi.jp>

* remove stale frontend reference to _responseInvalidIdHostNotMatch

Signed-off-by: eternal-flame-AD <yume@yumechi.jp>

* apply suggestions

Signed-off-by: eternal-flame-AD <yume@yumechi.jp>

---------

Signed-off-by: eternal-flame-AD <yume@yumechi.jp>
---
 packages/backend/src/core/HttpRequestService.ts | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

(limited to 'packages/backend/src/core/HttpRequestService.ts')

diff --git a/packages/backend/src/core/HttpRequestService.ts b/packages/backend/src/core/HttpRequestService.ts
index 083153940a..8085bbf961 100644
--- a/packages/backend/src/core/HttpRequestService.ts
+++ b/packages/backend/src/core/HttpRequestService.ts
@@ -16,7 +16,7 @@ import type { Config } from '@/config.js';
 import { StatusError } from '@/misc/status-error.js';
 import { bindThis } from '@/decorators.js';
 import { validateContentTypeSetAsActivityPub } from '@/core/activitypub/misc/validator.js';
-import { assertActivityMatchesUrls } from '@/core/activitypub/misc/check-against-url.js';
+import { assertActivityMatchesUrls, FetchAllowSoftFailMask } from '@/core/activitypub/misc/check-against-url.js';
 import type { IObject } from '@/core/activitypub/type.js';
 import type { Response } from 'node-fetch';
 import type { URL } from 'node:url';
@@ -215,7 +215,7 @@ export class HttpRequestService {
 	}
 
 	@bindThis
-	public async getActivityJson(url: string, isLocalAddressAllowed = false): Promise<IObject> {
+	public async getActivityJson(url: string, isLocalAddressAllowed = false, allowSoftfail: FetchAllowSoftFailMask = FetchAllowSoftFailMask.Strict): Promise<IObject> {
 		const res = await this.send(url, {
 			method: 'GET',
 			headers: {
@@ -232,7 +232,7 @@ export class HttpRequestService {
 		const finalUrl = res.url; // redirects may have been involved
 		const activity = await res.json() as IObject;
 
-		assertActivityMatchesUrls(activity, [finalUrl]);
+		assertActivityMatchesUrls(url, activity, [finalUrl], allowSoftfail);
 
 		return activity;
 	}
-- 
cgit v1.2.3-freya


From 495db2743318d43961f2a7fea0084ccae1cd9dea Mon Sep 17 00:00:00 2001
From: おさむのひと <46447427+samunohito@users.noreply.github.com>
Date: Wed, 26 Feb 2025 10:48:38 +0900
Subject: fix(backend):
 カスタム絵文字の一括インポートをした時にHTTPプロキシの除外設定が効かないのを修正
 (#15431)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

* pxory

* fix

* fix CHANGELOG.md

* allow localAddress

---------

Co-authored-by: syuilo <4439005+syuilo@users.noreply.github.com>
---
 CHANGELOG.md                                    |  1 +
 packages/backend/src/core/DownloadService.ts    |  4 +--
 packages/backend/src/core/HttpRequestService.ts | 47 +++++++++++++++++++++----
 3 files changed, 43 insertions(+), 9 deletions(-)

(limited to 'packages/backend/src/core/HttpRequestService.ts')

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 8b5579db4c..8ffce0beee 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -24,6 +24,7 @@
 - Enhance: 成り済まし対策として、ActivityPub照会された時にリモートのリダイレクトを拒否できるように (config.disallowExternalApRedirect)
 - Fix: `following/invalidate`でフォロワーを解除しようとしているユーザーの情報を返すように
 - Fix: オブジェクトストレージの設定でPrefixを設定していなかった場合nullまたは空文字になる問題を修正
+- Fix: HTTPプロキシとその除外設定を行った状態でカスタム絵文字の一括インポートをしたとき、除外設定が効かないのを修正( #8766 )
 - Fix: pgroongaでの検索時にはじめのキーワードのみが検索に使用される問題を修正  
   (Cherry-picked from https://activitypub.software/TransFem-org/Sharkey/-/merge_requests/886)
 - Fix: メールアドレスの形式が正しくなければ以降の処理を行わないように
diff --git a/packages/backend/src/core/DownloadService.ts b/packages/backend/src/core/DownloadService.ts
index 2e78e6d877..a2b74d1ab2 100644
--- a/packages/backend/src/core/DownloadService.ts
+++ b/packages/backend/src/core/DownloadService.ts
@@ -60,8 +60,8 @@ export class DownloadService {
 				request: operationTimeout,	// whole operation timeout
 			},
 			agent: {
-				http: this.httpRequestService.httpAgent,
-				https: this.httpRequestService.httpsAgent,
+				http: this.httpRequestService.getAgentForHttp(urlObj, true),
+				https: this.httpRequestService.getAgentForHttps(urlObj, true),
 			},
 			http2: false,	// default
 			retry: {
diff --git a/packages/backend/src/core/HttpRequestService.ts b/packages/backend/src/core/HttpRequestService.ts
index 8085bbf961..13d8f7f43b 100644
--- a/packages/backend/src/core/HttpRequestService.ts
+++ b/packages/backend/src/core/HttpRequestService.ts
@@ -115,32 +115,32 @@ export class HttpRequestService {
 	/**
 	 * Get http non-proxy agent (without local address filtering)
 	 */
-	private httpNative: http.Agent;
+	private readonly httpNative: http.Agent;
 
 	/**
 	 * Get https non-proxy agent (without local address filtering)
 	 */
-	private httpsNative: https.Agent;
+	private readonly httpsNative: https.Agent;
 
 	/**
 	 * Get http non-proxy agent
 	 */
-	private http: http.Agent;
+	private readonly http: http.Agent;
 
 	/**
 	 * Get https non-proxy agent
 	 */
-	private https: https.Agent;
+	private readonly https: https.Agent;
 
 	/**
 	 * Get http proxy or non-proxy agent
 	 */
-	public httpAgent: http.Agent;
+	public readonly httpAgent: http.Agent;
 
 	/**
 	 * Get https proxy or non-proxy agent
 	 */
-	public httpsAgent: https.Agent;
+	public readonly httpsAgent: https.Agent;
 
 	constructor(
 		@Inject(DI.config)
@@ -197,7 +197,8 @@ export class HttpRequestService {
 	/**
 	 * Get agent by URL
 	 * @param url URL
-	 * @param bypassProxy Allways bypass proxy
+	 * @param bypassProxy Always bypass proxy
+	 * @param isLocalAddressAllowed
 	 */
 	@bindThis
 	public getAgentByUrl(url: URL, bypassProxy = false, isLocalAddressAllowed = false): http.Agent | https.Agent {
@@ -214,6 +215,38 @@ export class HttpRequestService {
 		}
 	}
 
+	/**
+	 * Get agent for http by URL
+	 * @param url URL
+	 * @param isLocalAddressAllowed
+	 */
+	@bindThis
+	public getAgentForHttp(url: URL, isLocalAddressAllowed = false): http.Agent {
+		if ((this.config.proxyBypassHosts ?? []).includes(url.hostname)) {
+			return isLocalAddressAllowed
+				? this.httpNative
+				: this.http;
+		} else {
+			return this.httpAgent;
+		}
+	}
+
+	/**
+	 * Get agent for https by URL
+	 * @param url URL
+	 * @param isLocalAddressAllowed
+	 */
+	@bindThis
+	public getAgentForHttps(url: URL, isLocalAddressAllowed = false): https.Agent {
+		if ((this.config.proxyBypassHosts ?? []).includes(url.hostname)) {
+			return isLocalAddressAllowed
+				? this.httpsNative
+				: this.https;
+		} else {
+			return this.httpsAgent;
+		}
+	}
+
 	@bindThis
 	public async getActivityJson(url: string, isLocalAddressAllowed = false, allowSoftfail: FetchAllowSoftFailMask = FetchAllowSoftFailMask.Strict): Promise<IObject> {
 		const res = await this.send(url, {
-- 
cgit v1.2.3-freya


From e5d117dc98b725f908402638754f8643bbbeef90 Mon Sep 17 00:00:00 2001
From: "饺子w (Yumechi)" <35571479+eternal-flame-AD@users.noreply.github.com>
Date: Wed, 12 Mar 2025 12:39:24 +0000
Subject: fix(backend): tighten an overly relaxed criteria and remove
 capability of matching multiple final URLs in URL authority checking (#15655)

Signed-off-by: eternal-flame-AD <yume@yumechi.jp>
---
 CHANGELOG.md                                       |  2 +-
 packages/backend/src/core/HttpRequestService.ts    |  4 +-
 .../src/core/activitypub/ApRequestService.ts       |  4 +-
 .../src/core/activitypub/misc/check-against-url.ts | 22 ++++---
 packages/backend/test/unit/ap-request.ts           | 75 +++++++++-------------
 5 files changed, 49 insertions(+), 58 deletions(-)

(limited to 'packages/backend/src/core/HttpRequestService.ts')

diff --git a/CHANGELOG.md b/CHANGELOG.md
index d02b37cbdb..a8b7f47a29 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -15,7 +15,7 @@
 
 ### Server
 - Fix: プロフィール追加情報で無効なURLに入力された場合に照会エラーを出るのを修正
-
+- Fix: ActivityPubリクエストURLチェック実装は仕様に従っていないのを修正
 
 ## 2025.3.1
 
diff --git a/packages/backend/src/core/HttpRequestService.ts b/packages/backend/src/core/HttpRequestService.ts
index 13d8f7f43b..3ddfe52045 100644
--- a/packages/backend/src/core/HttpRequestService.ts
+++ b/packages/backend/src/core/HttpRequestService.ts
@@ -16,7 +16,7 @@ import type { Config } from '@/config.js';
 import { StatusError } from '@/misc/status-error.js';
 import { bindThis } from '@/decorators.js';
 import { validateContentTypeSetAsActivityPub } from '@/core/activitypub/misc/validator.js';
-import { assertActivityMatchesUrls, FetchAllowSoftFailMask } from '@/core/activitypub/misc/check-against-url.js';
+import { assertActivityMatchesUrl, FetchAllowSoftFailMask } from '@/core/activitypub/misc/check-against-url.js';
 import type { IObject } from '@/core/activitypub/type.js';
 import type { Response } from 'node-fetch';
 import type { URL } from 'node:url';
@@ -265,7 +265,7 @@ export class HttpRequestService {
 		const finalUrl = res.url; // redirects may have been involved
 		const activity = await res.json() as IObject;
 
-		assertActivityMatchesUrls(url, activity, [finalUrl], allowSoftfail);
+		assertActivityMatchesUrl(url, activity, finalUrl, allowSoftfail);
 
 		return activity;
 	}
diff --git a/packages/backend/src/core/activitypub/ApRequestService.ts b/packages/backend/src/core/activitypub/ApRequestService.ts
index 6c29cce325..61d328ccac 100644
--- a/packages/backend/src/core/activitypub/ApRequestService.ts
+++ b/packages/backend/src/core/activitypub/ApRequestService.ts
@@ -17,7 +17,7 @@ import { LoggerService } from '@/core/LoggerService.js';
 import { bindThis } from '@/decorators.js';
 import type Logger from '@/logger.js';
 import { validateContentTypeSetAsActivityPub } from '@/core/activitypub/misc/validator.js';
-import { assertActivityMatchesUrls, FetchAllowSoftFailMask as FetchAllowSoftFailMask } from '@/core/activitypub/misc/check-against-url.js';
+import { assertActivityMatchesUrl, FetchAllowSoftFailMask as FetchAllowSoftFailMask } from '@/core/activitypub/misc/check-against-url.js';
 import type { IObject } from './type.js';
 
 type Request = {
@@ -258,7 +258,7 @@ export class ApRequestService {
 		const finalUrl = res.url; // redirects may have been involved
 		const activity = await res.json() as IObject;
 
-		assertActivityMatchesUrls(url, activity, [finalUrl], allowSoftfail);
+		assertActivityMatchesUrl(url, activity, finalUrl, allowSoftfail);
 
 		return activity;
 	}
diff --git a/packages/backend/src/core/activitypub/misc/check-against-url.ts b/packages/backend/src/core/activitypub/misc/check-against-url.ts
index dfcfb1943e..bbfe57f9fa 100644
--- a/packages/backend/src/core/activitypub/misc/check-against-url.ts
+++ b/packages/backend/src/core/activitypub/misc/check-against-url.ts
@@ -75,7 +75,7 @@ function normalizeSynonymousSubdomain(url: URL | string): URL {
 	return new URL(urlParsed.toString().replace(host, normalizedHost));
 }
 
-export function assertActivityMatchesUrls(requestUrl: string | URL, activity: IObject, candidateUrls: (string | URL)[], allowSoftfail: FetchAllowSoftFailMask): FetchAllowSoftFailMask {
+export function assertActivityMatchesUrl(requestUrl: string | URL, activity: IObject, finalUrl: string | URL, allowSoftfail: FetchAllowSoftFailMask): FetchAllowSoftFailMask {
 	// must have a unique identifier to verify authority
 	if (!activity.id) {
 		throw new Error('bad Activity: missing id field');
@@ -95,26 +95,32 @@ export function assertActivityMatchesUrls(requestUrl: string | URL, activity: IO
 	const requestUrlParsed = normalizeSynonymousSubdomain(requestUrl);
 	const idParsed = normalizeSynonymousSubdomain(activity.id);
 
-	const candidateUrlsParsed = candidateUrls.map(it => normalizeSynonymousSubdomain(it));
+	const finalUrlParsed = normalizeSynonymousSubdomain(finalUrl);
+
+	// mastodon sends activities with hash in the URL
+	// currently it only happens with likes, deletes etc.
+	// but object ID never has hash
+	requestUrlParsed.hash = '';
+	finalUrlParsed.hash = '';
 
 	const requestUrlSecure = requestUrlParsed.protocol === 'https:';
-	const finalUrlSecure = candidateUrlsParsed.every(it => it.protocol === 'https:');
+	const finalUrlSecure = finalUrlParsed.protocol === 'https:';
 	if (requestUrlSecure && !finalUrlSecure) {
 		throw new Error(`bad Activity: id(${activity.id}) is not allowed to have http:// in the url`);
 	}
 
 	// Compare final URL to the ID
-	if (!candidateUrlsParsed.some(it => it.href === idParsed.href)) {
-		requireSoftfail(FetchAllowSoftFailMask.NonCanonicalId, `bad Activity: id(${activity.id}) does not match response url(${candidateUrlsParsed.map(it => it.toString())})`);
+	if (finalUrlParsed.href !== idParsed.href) {
+		requireSoftfail(FetchAllowSoftFailMask.NonCanonicalId, `bad Activity: id(${activity.id}) does not match response url(${finalUrlParsed.toString()})`);
 
 		// at lease host need to match exactly (ActivityPub requirement)
-		if (!candidateUrlsParsed.some(it => idParsed.host === it.host)) {
-			throw new Error(`bad Activity: id(${activity.id}) does not match response host(${candidateUrlsParsed.map(it => it.host)})`);
+		if (idParsed.host !== finalUrlParsed.host) {
+			throw new Error(`bad Activity: id(${activity.id}) does not match response host(${finalUrlParsed.host})`);
 		}
 	}
 
 	// Compare request URL to the ID
-	if (!requestUrlParsed.href.includes(idParsed.href)) {
+	if (requestUrlParsed.href !== idParsed.href) {
 		requireSoftfail(FetchAllowSoftFailMask.NonCanonicalId, `bad Activity: id(${activity.id}) does not match request url(${requestUrlParsed.toString()})`);
 
 		// if cross-origin lookup is allowed, we can accept some variation between the original request URL to the final object ID (but not between the final URL and the object ID)
diff --git a/packages/backend/test/unit/ap-request.ts b/packages/backend/test/unit/ap-request.ts
index 0426de8e19..f8b2a697f2 100644
--- a/packages/backend/test/unit/ap-request.ts
+++ b/packages/backend/test/unit/ap-request.ts
@@ -8,7 +8,7 @@ import httpSignature from '@peertube/http-signature';
 
 import { genRsaKeyPair } from '@/misc/gen-key-pair.js';
 import { ApRequestCreator } from '@/core/activitypub/ApRequestService.js';
-import { assertActivityMatchesUrls, FetchAllowSoftFailMask } from '@/core/activitypub/misc/check-against-url.js';
+import { assertActivityMatchesUrl, FetchAllowSoftFailMask } from '@/core/activitypub/misc/check-against-url.js';
 import { IObject } from '@/core/activitypub/type.js';
 
 export const buildParsedSignature = (signingString: string, signature: string, algorithm: string) => {
@@ -66,23 +66,26 @@ describe('ap-request', () => {
 	});
 
 	test('rejects non matching domain', () => {
-		assert.doesNotThrow(() => assertActivityMatchesUrls(
+		assert.doesNotThrow(() => assertActivityMatchesUrl(
 			'https://alice.example.com/abc',
 			{ id: 'https://alice.example.com/abc' } as IObject,
-			[
-				'https://alice.example.com/abc',
-			],
+			'https://alice.example.com/abc',
 			FetchAllowSoftFailMask.Strict,
 		), 'validation should pass base case');
-		assert.throws(() => assertActivityMatchesUrls(
+		assert.throws(() => assertActivityMatchesUrl(
 			'https://alice.example.com/abc',
 			{ id: 'https://bob.example.com/abc' } as IObject,
-			[
-				'https://alice.example.com/abc',
-			],
+			'https://alice.example.com/abc',
 			FetchAllowSoftFailMask.Any,
 		), 'validation should fail no matter what if the response URL is inconsistent with the object ID');
 		
+		assert.doesNotThrow(() => assertActivityMatchesUrl(
+			'https://alice.example.com/abc#test',
+			{ id: 'https://alice.example.com/abc' } as IObject,
+			'https://alice.example.com/abc',
+			FetchAllowSoftFailMask.Strict,
+		), 'validation should pass with hash in request URL');
+
 		// fix issues like threads
 		// https://github.com/misskey-dev/misskey/issues/15039
 		const withOrWithoutWWW = [
@@ -97,89 +100,71 @@ describe('ap-request', () => {
 			),
 			withOrWithoutWWW,
 		).forEach(([[a, b], c]) => {
-			assert.doesNotThrow(() => assertActivityMatchesUrls(
+			assert.doesNotThrow(() => assertActivityMatchesUrl(
 				a,
 				{ id: b } as IObject,
-				[
-					c,
-				],
+				c,
 				FetchAllowSoftFailMask.Strict,
 			), 'validation should pass with or without www. subdomain');
 		});
 	});
 
 	test('cross origin lookup', () => {
-		assert.doesNotThrow(() => assertActivityMatchesUrls(
+		assert.doesNotThrow(() => assertActivityMatchesUrl(
 			'https://alice.example.com/abc',
 			{ id: 'https://bob.example.com/abc' } as IObject,
-			[
-				'https://bob.example.com/abc',
-			],
+			'https://bob.example.com/abc',
 			FetchAllowSoftFailMask.CrossOrigin | FetchAllowSoftFailMask.NonCanonicalId,
 		), 'validation should pass if the response is otherwise consistent and cross-origin is allowed');
-		assert.throws(() => assertActivityMatchesUrls(
+		assert.throws(() => assertActivityMatchesUrl(
 			'https://alice.example.com/abc',
 			{ id: 'https://bob.example.com/abc' } as IObject,
-			[
-				'https://bob.example.com/abc',
-			],
+			'https://bob.example.com/abc',
 			FetchAllowSoftFailMask.Strict,
 		), 'validation should fail if the response is otherwise consistent and cross-origin is not allowed');
 	});
 
 	test('rejects non-canonical ID', () => {
-		assert.throws(() => assertActivityMatchesUrls(
+		assert.throws(() => assertActivityMatchesUrl(
 			'https://alice.example.com/@alice',
 			{ id: 'https://alice.example.com/users/alice' } as IObject,
-			[
-				'https://alice.example.com/users/alice'
-			],
+			'https://alice.example.com/users/alice',
 			FetchAllowSoftFailMask.Strict,
 		), 'throws if the response ID did not exactly match the expected ID');
-		assert.doesNotThrow(() => assertActivityMatchesUrls(
+		assert.doesNotThrow(() => assertActivityMatchesUrl(
 			'https://alice.example.com/@alice',
 			{ id: 'https://alice.example.com/users/alice' } as IObject,
-			[
-				'https://alice.example.com/users/alice',
-			],
+			'https://alice.example.com/users/alice',
 			FetchAllowSoftFailMask.NonCanonicalId,
 		), 'does not throw if non-canonical ID is allowed');
 	});
 
 	test('origin relaxed alignment', () => {
-		assert.doesNotThrow(() => assertActivityMatchesUrls(
+		assert.doesNotThrow(() => assertActivityMatchesUrl(
 			'https://alice.example.com/abc',
 			{ id: 'https://ap.alice.example.com/abc' } as IObject,
-			[
-				'https://ap.alice.example.com/abc',
-			],
+			'https://ap.alice.example.com/abc',
 			FetchAllowSoftFailMask.MisalignedOrigin | FetchAllowSoftFailMask.NonCanonicalId,
 		), 'validation should pass if response is a subdomain of the expected origin');
-		assert.throws(() => assertActivityMatchesUrls(
+		assert.throws(() => assertActivityMatchesUrl(
 			'https://alice.multi-tenant.example.com/abc',
 			{ id: 'https://alice.multi-tenant.example.com/abc' } as IObject,
-			[
-				'https://bob.multi-tenant.example.com/abc',
-			],
+			'https://bob.multi-tenant.example.com/abc',
 			FetchAllowSoftFailMask.MisalignedOrigin | FetchAllowSoftFailMask.NonCanonicalId,
 		), 'validation should fail if response is a disjoint domain of the expected origin');
-		assert.throws(() => assertActivityMatchesUrls(
+		assert.throws(() => assertActivityMatchesUrl(
 			'https://alice.example.com/abc',
 			{ id: 'https://ap.alice.example.com/abc' } as IObject,
-			[
-				'https://ap.alice.example.com/abc',
-			],
+			'https://ap.alice.example.com/abc',
 			FetchAllowSoftFailMask.Strict,
 		), 'throws if relaxed origin is forbidden');
 	});
 
 	test('resist HTTP downgrade', () => {
-		assert.throws(() => assertActivityMatchesUrls(
+		assert.throws(() => assertActivityMatchesUrl(
 			'https://alice.example.com/abc',
 			{ id: 'https://alice.example.com/abc' } as IObject,
-			[
-				'http://alice.example.com/abc',
-			],
+			'http://alice.example.com/abc',
 			FetchAllowSoftFailMask.Strict,
 		), 'throws if HTTP downgrade is detected');
 	});
-- 
cgit v1.2.3-freya


From f88430aebc1e5a0151375fe50458b099af18196f Mon Sep 17 00:00:00 2001
From: Hazelnoot <acomputerdog@gmail.com>
Date: Sun, 2 Mar 2025 21:08:05 -0500
Subject: add IObjectWithId type for APIs that work with objects required to
 have an ID.

---
 packages/backend/src/core/HttpRequestService.ts    |  6 +++---
 .../src/core/activitypub/ApRequestService.ts       |  6 +++---
 .../src/core/activitypub/ApResolverService.ts      | 24 ++++++++++++----------
 packages/backend/src/core/activitypub/type.ts      |  4 ++++
 4 files changed, 23 insertions(+), 17 deletions(-)

(limited to 'packages/backend/src/core/HttpRequestService.ts')

diff --git a/packages/backend/src/core/HttpRequestService.ts b/packages/backend/src/core/HttpRequestService.ts
index 19992a7597..1aa62a9879 100644
--- a/packages/backend/src/core/HttpRequestService.ts
+++ b/packages/backend/src/core/HttpRequestService.ts
@@ -16,7 +16,7 @@ import type { Config } from '@/config.js';
 import { StatusError } from '@/misc/status-error.js';
 import { bindThis } from '@/decorators.js';
 import { validateContentTypeSetAsActivityPub } from '@/core/activitypub/misc/validator.js';
-import { IObject } from '@/core/activitypub/type.js';
+import type { IObject, IObjectWithId } from '@/core/activitypub/type.js';
 import { ApUtilityService } from './activitypub/ApUtilityService.js';
 import type { Response } from 'node-fetch';
 import type { URL } from 'node:url';
@@ -217,7 +217,7 @@ export class HttpRequestService {
 	}
 
 	@bindThis
-	public async getActivityJson(url: string, isLocalAddressAllowed = false): Promise<IObject> {
+	public async getActivityJson(url: string, isLocalAddressAllowed = false): Promise<IObjectWithId> {
 		const res = await this.send(url, {
 			method: 'GET',
 			headers: {
@@ -237,7 +237,7 @@ export class HttpRequestService {
 		// The caller (ApResolverService) will verify the ID against the original / entry URL, which ensures that all three match.
 		this.apUtilityService.assertIdMatchesUrlAuthority(activity, res.url);
 
-		return activity;
+		return activity as IObjectWithId;
 	}
 
 	@bindThis
diff --git a/packages/backend/src/core/activitypub/ApRequestService.ts b/packages/backend/src/core/activitypub/ApRequestService.ts
index b63d4eb2ab..952d1f5219 100644
--- a/packages/backend/src/core/activitypub/ApRequestService.ts
+++ b/packages/backend/src/core/activitypub/ApRequestService.ts
@@ -17,7 +17,7 @@ import { LoggerService } from '@/core/LoggerService.js';
 import { bindThis } from '@/decorators.js';
 import type Logger from '@/logger.js';
 import { validateContentTypeSetAsActivityPub } from '@/core/activitypub/misc/validator.js';
-import type { IObject } from './type.js';
+import type { IObject, IObjectWithId } from './type.js';
 
 type Request = {
 	url: string;
@@ -185,7 +185,7 @@ export class ApRequestService {
 	 * @param followAlternate
 	 */
 	@bindThis
-	public async signedGet(url: string, user: { id: MiUser['id'] }, followAlternate?: boolean): Promise<IObject> {
+	public async signedGet(url: string, user: { id: MiUser['id'] }, followAlternate?: boolean): Promise<IObjectWithId> {
 		const _followAlternate = followAlternate ?? true;
 		const keypair = await this.userKeypairService.getUserKeypair(user.id);
 
@@ -273,6 +273,6 @@ export class ApRequestService {
 		// The caller (ApResolverService) will verify the ID against the original / entry URL, which ensures that all three match.
 		this.apUtilityService.assertIdMatchesUrlAuthority(activity, res.url);
 
-		return activity;
+		return activity as IObjectWithId;
 	}
 }
diff --git a/packages/backend/src/core/activitypub/ApResolverService.ts b/packages/backend/src/core/activitypub/ApResolverService.ts
index c5ac8a944d..12c3202af1 100644
--- a/packages/backend/src/core/activitypub/ApResolverService.ts
+++ b/packages/backend/src/core/activitypub/ApResolverService.ts
@@ -19,7 +19,7 @@ import { fromTuple } from '@/misc/from-tuple.js';
 import { IdentifiableError } from '@/misc/identifiable-error.js';
 import { ApLogService, calculateDurationSince, extractObjectContext } from '@/core/ApLogService.js';
 import { ApUtilityService } from '@/core/activitypub/ApUtilityService.js';
-import { getApId, getNullableApId, isCollectionOrOrderedCollection } from './type.js';
+import { getApId, getNullableApId, IObjectWithId, isCollectionOrOrderedCollection } from './type.js';
 import { ApDbResolverService } from './ApDbResolverService.js';
 import { ApRendererService } from './ApRendererService.js';
 import { ApRequestService } from './ApRequestService.js';
@@ -82,7 +82,7 @@ export class Resolver {
 	 * In all other cases, the object is re-fetched from remote by input string or object ID.
 	 */
 	@bindThis
-	public async secureResolve(input: ApObject, sentFromUri: string): Promise<IObject> {
+	public async secureResolve(input: ApObject, sentFromUri: string): Promise<IObjectWithId> {
 		// Unpack arrays to get the value element.
 		const value = fromTuple(input);
 		if (value == null) {
@@ -96,13 +96,15 @@ export class Resolver {
 		// Our security requires that the object ID matches the host authority that sent it, otherwise it can't be trusted.
 		// A mismatch isn't necessarily malicious, it just means we can't use the object we were given.
 		if (typeof(value) === 'object' && this.apUtilityService.haveSameAuthority(id, sentFromUri)) {
-			return value;
+			return value as IObjectWithId;
 		}
 
 		// If the checks didn't pass, then we must fetch the object and use that.
 		return await this.resolve(id);
 	}
 
+	public async resolve(value: string | [string]): Promise<IObjectWithId>;
+	public async resolve(value: string | IObject | [string | IObject]): Promise<IObject>;
 	@bindThis
 	public async resolve(value: string | IObject | [string | IObject]): Promise<IObject> {
 		// eslint-disable-next-line no-param-reassign
@@ -120,7 +122,7 @@ export class Resolver {
 		}
 	}
 
-	private async _resolveLogged(requestUri: string, host: string): Promise<IObject> {
+	private async _resolveLogged(requestUri: string, host: string): Promise<IObjectWithId> {
 		const startTime = process.hrtime.bigint();
 
 		const log = await this.apLogService.createFetchLog({
@@ -149,7 +151,7 @@ export class Resolver {
 		}
 	}
 
-	private async _resolve(value: string, host: string, log?: SkApFetchLog): Promise<IObject> {
+	private async _resolve(value: string, host: string, log?: SkApFetchLog): Promise<IObjectWithId> {
 		if (value.includes('#')) {
 			// URLs with fragment parts cannot be resolved correctly because
 			// the fragment part does not get transmitted over HTTP(S).
@@ -168,7 +170,7 @@ export class Resolver {
 		this.history.add(value);
 
 		if (this.utilityService.isSelfHost(host)) {
-			return await this.resolveLocal(value);
+			return await this.resolveLocal(value) as IObjectWithId;
 		}
 
 		if (!this.utilityService.isFederationAllowedHost(host)) {
@@ -180,8 +182,8 @@ export class Resolver {
 		}
 
 		const object = (this.user
-			? await this.apRequestService.signedGet(value, this.user) as IObject
-			: await this.httpRequestService.getActivityJson(value)) as IObject;
+			? await this.apRequestService.signedGet(value, this.user)
+			: await this.httpRequestService.getActivityJson(value));
 
 		if (log) {
 			const { object: objectOnly, context, contextHash } = extractObjectContext(object);
@@ -226,7 +228,7 @@ export class Resolver {
 	}
 
 	@bindThis
-	private resolveLocal(url: string): Promise<IObject> {
+	private resolveLocal(url: string): Promise<IObjectWithId> {
 		const parsed = this.apDbResolverService.parseUri(url);
 		if (!parsed.local) throw new IdentifiableError('02b40cd0-fa92-4b0c-acc9-fb2ada952ab8', `resolveLocal - not a local URL: ${url}`);
 
@@ -241,7 +243,7 @@ export class Resolver {
 						} else {
 							return this.apRendererService.renderNote(note, author);
 						}
-					});
+					}) as Promise<IObjectWithId>;
 			case 'users':
 				return this.usersRepository.findOneByOrFail({ id: parsed.id })
 					.then(user => this.apRendererService.renderPerson(user as MiLocalUser));
@@ -251,7 +253,7 @@ export class Resolver {
 					this.notesRepository.findOneByOrFail({ id: parsed.id }),
 					this.pollsRepository.findOneByOrFail({ noteId: parsed.id }),
 				])
-					.then(([note, poll]) => this.apRendererService.renderQuestion({ id: note.userId }, note, poll));
+					.then(([note, poll]) => this.apRendererService.renderQuestion({ id: note.userId }, note, poll)) as Promise<IObjectWithId>;
 			case 'likes':
 				return this.noteReactionsRepository.findOneByOrFail({ id: parsed.id }).then(async reaction =>
 					this.apRendererService.addContext(await this.apRendererService.renderLike(reaction, { uri: null })));
diff --git a/packages/backend/src/core/activitypub/type.ts b/packages/backend/src/core/activitypub/type.ts
index a319c0b6ea..e7459a57d2 100644
--- a/packages/backend/src/core/activitypub/type.ts
+++ b/packages/backend/src/core/activitypub/type.ts
@@ -39,6 +39,10 @@ export interface IObject {
 	sensitive?: boolean;
 }
 
+export interface IObjectWithId extends IObject {
+	id: string;
+}
+
 /**
  * Get array of ActivityStreams Objects id
  */
-- 
cgit v1.2.3-freya


From 7ea710b31442cd05ac75a971212a0fa09f69645a Mon Sep 17 00:00:00 2001
From: Hazelnoot <acomputerdog@gmail.com>
Date: Tue, 25 Mar 2025 17:19:38 -0400
Subject: remerge: remove FetchAllowSoftFailMask in favor of our same-authority
 checks

---
 packages/backend/src/core/HttpRequestService.ts    |  5 +--
 .../backend/src/core/activitypub/ApInboxService.ts |  2 +-
 .../src/core/activitypub/ApRequestService.ts       |  9 ++---
 .../src/core/activitypub/ApResolverService.ts      | 21 +++++-----
 .../src/core/activitypub/misc/check-against-url.ts | 38 ------------------
 .../src/core/activitypub/models/ApNoteService.ts   | 31 ++++++++-------
 .../src/core/activitypub/models/ApPersonService.ts | 45 +++++++++++++++++-----
 .../backend/src/server/api/endpoints/ap/show.ts    | 40 +++++++------------
 packages/backend/test/unit/activitypub.ts          |  1 -
 packages/backend/test/unit/ap-request.ts           |  5 ++-
 10 files changed, 86 insertions(+), 111 deletions(-)
 delete mode 100644 packages/backend/src/core/activitypub/misc/check-against-url.ts

(limited to 'packages/backend/src/core/HttpRequestService.ts')

diff --git a/packages/backend/src/core/HttpRequestService.ts b/packages/backend/src/core/HttpRequestService.ts
index e8b0326e66..12047346fb 100644
--- a/packages/backend/src/core/HttpRequestService.ts
+++ b/packages/backend/src/core/HttpRequestService.ts
@@ -16,7 +16,6 @@ import type { Config } from '@/config.js';
 import { StatusError } from '@/misc/status-error.js';
 import { bindThis } from '@/decorators.js';
 import { validateContentTypeSetAsActivityPub } from '@/core/activitypub/misc/validator.js';
-import { FetchAllowSoftFailMask } from '@/core/activitypub/misc/check-against-url.js';
 import type { IObject, IObjectWithId } from '@/core/activitypub/type.js';
 import { ApUtilityService } from './activitypub/ApUtilityService.js';
 import type { Response } from 'node-fetch';
@@ -250,7 +249,7 @@ export class HttpRequestService {
 	}
 
 	@bindThis
-	public async getActivityJson(url: string, isLocalAddressAllowed = false, allowSoftfail: FetchAllowSoftFailMask = FetchAllowSoftFailMask.Strict): Promise<IObjectWithId> {
+	public async getActivityJson(url: string, isLocalAddressAllowed = false): Promise<IObjectWithId> {
 		const res = await this.send(url, {
 			method: 'GET',
 			headers: {
@@ -268,7 +267,7 @@ export class HttpRequestService {
 
 		// Make sure the object ID matches the final URL (which is where it actually exists).
 		// The caller (ApResolverService) will verify the ID against the original / entry URL, which ensures that all three match.
-		this.apUtilityService.assertIdMatchesUrlAuthority(activity, res.url, allowSoftfail);
+		this.apUtilityService.assertIdMatchesUrlAuthority(activity, res.url);
 
 		return activity as IObjectWithId;
 	}
diff --git a/packages/backend/src/core/activitypub/ApInboxService.ts b/packages/backend/src/core/activitypub/ApInboxService.ts
index ac4a408fa6..37b01a6a40 100644
--- a/packages/backend/src/core/activitypub/ApInboxService.ts
+++ b/packages/backend/src/core/activitypub/ApInboxService.ts
@@ -354,7 +354,7 @@ export class ApInboxService {
 			try {
 				// The target ID is verified by secureResolve, so we know it shares host authority with the actor who sent it.
 				// This means we can pass that ID to resolveNote and avoid an extra fetch, which will fail if the note is private.
-				renote = await this.apNoteService.resolveNote(target, { resolver, sentFrom: new URL(getApId(target)) });
+				renote = await this.apNoteService.resolveNote(target, { resolver, sentFrom: getApId(target) });
 				if (renote == null) return 'announce target is null';
 			} catch (err) {
 				// 対象が4xxならスキップ
diff --git a/packages/backend/src/core/activitypub/ApRequestService.ts b/packages/backend/src/core/activitypub/ApRequestService.ts
index 5caee21610..7118ce1e02 100644
--- a/packages/backend/src/core/activitypub/ApRequestService.ts
+++ b/packages/backend/src/core/activitypub/ApRequestService.ts
@@ -17,7 +17,6 @@ import { LoggerService } from '@/core/LoggerService.js';
 import { bindThis } from '@/decorators.js';
 import type Logger from '@/logger.js';
 import { validateContentTypeSetAsActivityPub } from '@/core/activitypub/misc/validator.js';
-import { FetchAllowSoftFailMask as FetchAllowSoftFailMask } from '@/core/activitypub/misc/check-against-url.js';
 import type { IObject, IObjectWithId } from './type.js';
 
 type Request = {
@@ -186,7 +185,7 @@ export class ApRequestService {
 	 * @param followAlternate
 	 */
 	@bindThis
-	public async signedGet(url: string, user: { id: MiUser['id'] }, allowSoftfail: FetchAllowSoftFailMask = FetchAllowSoftFailMask.Strict, followAlternate?: boolean): Promise<IObjectWithId> {
+	public async signedGet(url: string, user: { id: MiUser['id'] }, followAlternate?: boolean): Promise<IObjectWithId> {
 		const _followAlternate = followAlternate ?? true;
 		const keypair = await this.userKeypairService.getUserKeypair(user.id);
 
@@ -255,10 +254,10 @@ export class ApRequestService {
 				if (alternate) {
 					const href = alternate.getAttribute('href');
 					if (href && this.apUtilityService.haveSameAuthority(url, href)) {
-						return await this.signedGet(href, user, allowSoftfail, false);
+						return await this.signedGet(href, user, false);
 					}
 				}
-			} catch (e) {
+			} catch {
 				// something went wrong parsing the HTML, ignore the whole thing
 			} finally {
 				happyDOM.close().catch(err => {});
@@ -272,7 +271,7 @@ export class ApRequestService {
 
 		// Make sure the object ID matches the final URL (which is where it actually exists).
 		// The caller (ApResolverService) will verify the ID against the original / entry URL, which ensures that all three match.
-		this.apUtilityService.assertIdMatchesUrlAuthority(activity, res.url, allowSoftfail);
+		this.apUtilityService.assertIdMatchesUrlAuthority(activity, res.url);
 
 		return activity as IObjectWithId;
 	}
diff --git a/packages/backend/src/core/activitypub/ApResolverService.ts b/packages/backend/src/core/activitypub/ApResolverService.ts
index 27dcf2372b..5e58f848c0 100644
--- a/packages/backend/src/core/activitypub/ApResolverService.ts
+++ b/packages/backend/src/core/activitypub/ApResolverService.ts
@@ -23,7 +23,6 @@ import { getApId, getNullableApId, IObjectWithId, isCollectionOrOrderedCollectio
 import { ApDbResolverService } from './ApDbResolverService.js';
 import { ApRendererService } from './ApRendererService.js';
 import { ApRequestService } from './ApRequestService.js';
-import { FetchAllowSoftFailMask } from './misc/check-against-url.js';
 import type { IObject, ICollection, IOrderedCollection, ApObject } from './type.js';
 
 export class Resolver {
@@ -104,10 +103,10 @@ export class Resolver {
 		return await this.resolve(id);
 	}
 
-	public async resolve(value: string | [string], allowSoftfail?: FetchAllowSoftFailMask): Promise<IObjectWithId>;
-	public async resolve(value: string | IObject | [string | IObject], allowSoftfail?: FetchAllowSoftFailMask): Promise<IObject>;
+	public async resolve(value: string | [string]): Promise<IObjectWithId>;
+	public async resolve(value: string | IObject | [string | IObject]): Promise<IObject>;
 	@bindThis
-	public async resolve(value: string | IObject | [string | IObject], allowSoftfail: FetchAllowSoftFailMask = FetchAllowSoftFailMask.Strict): Promise<IObject> {
+	public async resolve(value: string | IObject | [string | IObject]): Promise<IObject> {
 		value = fromTuple(value);
 
 		if (typeof value !== 'string') {
@@ -116,13 +115,13 @@ export class Resolver {
 
 		const host = this.utilityService.extractDbHost(value);
 		if (this.config.activityLogging.enabled && !this.utilityService.isSelfHost(host)) {
-			return await this._resolveLogged(value, host, allowSoftfail);
+			return await this._resolveLogged(value, host);
 		} else {
-			return await this._resolve(value, host, allowSoftfail);
+			return await this._resolve(value, host);
 		}
 	}
 
-	private async _resolveLogged(requestUri: string, host: string, allowSoftfail: FetchAllowSoftFailMask): Promise<IObjectWithId> {
+	private async _resolveLogged(requestUri: string, host: string): Promise<IObjectWithId> {
 		const startTime = process.hrtime.bigint();
 
 		const log = await this.apLogService.createFetchLog({
@@ -131,7 +130,7 @@ export class Resolver {
 		});
 
 		try {
-			const result = await this._resolve(requestUri, host, allowSoftfail, log);
+			const result = await this._resolve(requestUri, host, log);
 
 			log.accepted = true;
 			log.result = 'ok';
@@ -151,7 +150,7 @@ export class Resolver {
 		}
 	}
 
-	private async _resolve(value: string, host: string, allowSoftfail: FetchAllowSoftFailMask, log?: SkApFetchLog): Promise<IObjectWithId> {
+	private async _resolve(value: string, host: string, log?: SkApFetchLog): Promise<IObjectWithId> {
 		if (value.includes('#')) {
 			// URLs with fragment parts cannot be resolved correctly because
 			// the fragment part does not get transmitted over HTTP(S).
@@ -182,8 +181,8 @@ export class Resolver {
 		}
 
 		const object = (this.user
-			? await this.apRequestService.signedGet(value, this.user, allowSoftfail) as IObject
-			: await this.httpRequestService.getActivityJson(value, allowSoftfail)) as IObject;
+			? await this.apRequestService.signedGet(value, this.user)
+			: await this.httpRequestService.getActivityJson(value));
 
 		if (log) {
 			const { object: objectOnly, context, contextHash } = extractObjectContext(object);
diff --git a/packages/backend/src/core/activitypub/misc/check-against-url.ts b/packages/backend/src/core/activitypub/misc/check-against-url.ts
deleted file mode 100644
index 282859907d..0000000000
--- a/packages/backend/src/core/activitypub/misc/check-against-url.ts
+++ /dev/null
@@ -1,38 +0,0 @@
-/*
- * SPDX-FileCopyrightText: dakkar and sharkey-project
- * SPDX-License-Identifier: AGPL-3.0-only
- */
-import type { IObject } from '../type.js';
-
-export enum FetchAllowSoftFailMask {
-	// Allow no softfail flags
-	Strict = 0,
-	// The values in tuple (requestUrl, finalUrl, objectId) are not all identical
-	//
-	// This condition is common for user-initiated lookups but should not be allowed in federation loop
-	//
-	// Allow variations:
-	//   good example: https://alice.example.com/@user -> https://alice.example.com/user/:userId
-	//   problematic example: https://alice.example.com/redirect?url=https://bad.example.com/ -> https://bad.example.com/ -> https://alice.example.com/somethingElse
-	NonCanonicalId = 1 << 0,
-	// Allow the final object to be at most one subdomain deeper than the request URL, similar to SPF relaxed alignment
-	//
-	// Currently no code path allows this flag to be set, but is kept in case of future use as some niche deployments do this, and we provide a pre-reviewed mechanism to opt-in.
-	//
-	// Allow variations:
-	//   good example: https://example.com/@user -> https://activitypub.example.com/@user { id: 'https://activitypub.example.com/@user' }
-	//   problematic example: https://example.com/@user -> https://untrusted.example.com/@user { id: 'https://untrusted.example.com/@user' }
-	MisalignedOrigin = 1 << 1,
-	// The requested URL has a different host than the returned object ID, although the final URL is still consistent with the object ID
-	//
-	// This condition is common for user-initiated lookups using an intermediate host but should not be allowed in federation loops
-	//
-	// Allow variations:
-	//   good example: https://alice.example.com/@user@bob.example.com -> https://bob.example.com/@user { id: 'https://bob.example.com/@user' }
-	//   problematic example: https://alice.example.com/definitelyAlice -> https://bob.example.com/@somebodyElse { id: 'https://bob.example.com/@somebodyElse' }
-	CrossOrigin = 1 << 2 | MisalignedOrigin,
-	// Allow all softfail flags
-	//
-	// do not use this flag on released code
-	Any = ~0,
-}
diff --git a/packages/backend/src/core/activitypub/models/ApNoteService.ts b/packages/backend/src/core/activitypub/models/ApNoteService.ts
index 63f9887a8d..5d168e623b 100644
--- a/packages/backend/src/core/activitypub/models/ApNoteService.ts
+++ b/packages/backend/src/core/activitypub/models/ApNoteService.ts
@@ -550,29 +550,32 @@ export class ApNoteService {
 	 * リモートサーバーからフェッチしてMisskeyに登録しそれを返します。
 	 */
 	@bindThis
-	public async resolveNote(value: string | IObject, options: { sentFrom?: URL, resolver?: Resolver } = {}): Promise<MiNote | null> {
+	public async resolveNote(value: string | IObject, options: { sentFrom?: string, resolver?: Resolver } = {}): Promise<MiNote | null> {
 		const uri = getApId(value);
 
 		if (!this.utilityService.isFederationAllowedUri(uri)) {
+			// TODO convert to identifiable error
 			throw new StatusError(`blocked host: ${uri}`, 451, 'blocked host');
 		}
 
-		const unlock = await this.appLockService.getApLock(uri);
+		//#region このサーバーに既に登録されていたらそれを返す
+		const exist = await this.fetchNote(uri);
+		if (exist) return exist;
+		//#endregion
 
-		try {
-			//#region このサーバーに既に登録されていたらそれを返す
-			const exist = await this.fetchNote(uri);
-			if (exist) return exist;
-			//#endregion
+		// Bail if local URI doesn't exist
+		if (this.utilityService.isUriLocal(uri)) {
+			// TODO convert to identifiable error
+			throw new StatusError(`cannot resolve local note: ${uri}`, 400, 'cannot resolve local note');
+		}
 
-			if (this.utilityService.isUriLocal(uri)) {
-				throw new StatusError(`cannot resolve local note: ${uri}`, 400, 'cannot resolve local note');
-			}
+		const unlock = await this.appLockService.getApLock(uri);
 
-			// リモートサーバーからフェッチしてきて登録
-			// ここでuriの代わりに添付されてきたNote Objectが指定されていると、サーバーフェッチを経ずにノートが生成されるが
-			// 添付されてきたNote Objectは偽装されている可能性があるため、常にuriを指定してサーバーフェッチを行う。
-			const createFrom = options.sentFrom?.origin === new URL(uri).origin ? value : uri;
+		try {
+			// Optimization: we can avoid re-fetching the value *if and only if* it matches the host authority that it was sent from.
+			// Instances can create any object within their host authority, but anything outside of that MUST be untrusted.
+			const haveSameAuthority = options.sentFrom && this.apUtilityService.haveSameAuthority(options.sentFrom, uri);
+			const createFrom = haveSameAuthority ? value : uri;
 			return await this.createNote(createFrom, undefined, options.resolver, true);
 		} finally {
 			unlock();
diff --git a/packages/backend/src/core/activitypub/models/ApPersonService.ts b/packages/backend/src/core/activitypub/models/ApPersonService.ts
index 59f1d73fb0..1c394a9276 100644
--- a/packages/backend/src/core/activitypub/models/ApPersonService.ts
+++ b/packages/backend/src/core/activitypub/models/ApPersonService.ts
@@ -40,6 +40,7 @@ import { RoleService } from '@/core/RoleService.js';
 import { DriveFileEntityService } from '@/core/entities/DriveFileEntityService.js';
 import type { AccountMoveService } from '@/core/AccountMoveService.js';
 import { ApUtilityService } from '@/core/activitypub/ApUtilityService.js';
+import { AppLockService } from '@/core/AppLockService.js';
 import { getApId, getApType, isActor, isCollection, isCollectionOrOrderedCollection, isPropertyValue } from '../type.js';
 import { extractApHashtags } from './tag.js';
 import type { OnModuleInit } from '@nestjs/common';
@@ -107,6 +108,7 @@ export class ApPersonService implements OnModuleInit {
 
 		private roleService: RoleService,
 		private readonly apUtilityService: ApUtilityService,
+		private readonly appLockService: AppLockService,
 	) {
 	}
 
@@ -314,12 +316,17 @@ export class ApPersonService implements OnModuleInit {
 			throw new StatusError(`cannot resolve local user: ${uri}`, 400, 'cannot resolve local user');
 		}
 
+		return await this._createPerson(uri, resolver);
+	}
+
+	private async _createPerson(value: string | IObject, resolver?: Resolver): Promise<MiRemoteUser> {
+		const uri = getApId(value);
+		const host = this.utilityService.punyHost(uri);
+
 		// eslint-disable-next-line no-param-reassign
 		if (resolver == null) resolver = this.apResolverService.createResolver();
 
-		const object = await resolver.resolve(uri);
-		if (object.id == null) throw new UnrecoverableError(`null object.id in ${uri}`);
-
+		const object = await resolver.resolve(value);
 		const person = this.validateActor(object, uri);
 
 		this.logger.info(`Creating the Person: ${person.id}`);
@@ -685,16 +692,36 @@ export class ApPersonService implements OnModuleInit {
 	 * リモートサーバーからフェッチしてMisskeyに登録しそれを返します。
 	 */
 	@bindThis
-	public async resolvePerson(uri: string, resolver?: Resolver): Promise<MiLocalUser | MiRemoteUser> {
+	public async resolvePerson(value: string | IObject, resolver?: Resolver, sentFrom?: string): Promise<MiLocalUser | MiRemoteUser> {
+		const uri = getApId(value);
+
+		if (!this.utilityService.isFederationAllowedUri(uri)) {
+			// TODO convert to identifiable error
+			throw new StatusError(`blocked host: ${uri}`, 451, 'blocked host');
+		}
+
 		//#region このサーバーに既に登録されていたらそれを返す
 		const exist = await this.fetchPerson(uri);
 		if (exist) return exist;
 		//#endregion
 
-		// リモートサーバーからフェッチしてきて登録
-		// eslint-disable-next-line no-param-reassign
-		if (resolver == null) resolver = this.apResolverService.createResolver();
-		return await this.createPerson(uri, resolver);
+		// Bail if local URI doesn't exist
+		if (this.utilityService.isUriLocal(uri)) {
+			// TODO convert to identifiable error
+			throw new StatusError(`cannot resolve local person: ${uri}`, 400, 'cannot resolve local person');
+		}
+
+		const unlock = await this.appLockService.getApLock(uri);
+
+		try {
+			// Optimization: we can avoid re-fetching the value *if and only if* it matches the host authority that it was sent from.
+			// Instances can create any object within their host authority, but anything outside of that MUST be untrusted.
+			const haveSameAuthority = sentFrom && this.apUtilityService.haveSameAuthority(sentFrom, uri);
+			const createFrom = haveSameAuthority ? value : uri;
+			return await this._createPerson(createFrom, resolver);
+		} finally {
+			unlock();
+		}
 	}
 
 	@bindThis
@@ -748,7 +775,7 @@ export class ApPersonService implements OnModuleInit {
 			.slice(0, maxPinned)
 			.map(item => limit(() => this.apNoteService.resolveNote(item, {
 				resolver: _resolver,
-				sentFrom: new URL(user.uri),
+				sentFrom: user.uri,
 			}))));
 
 		await this.db.transaction(async transactionalEntityManager => {
diff --git a/packages/backend/src/server/api/endpoints/ap/show.ts b/packages/backend/src/server/api/endpoints/ap/show.ts
index 8cf7747d5f..8ba18a3b8d 100644
--- a/packages/backend/src/server/api/endpoints/ap/show.ts
+++ b/packages/backend/src/server/api/endpoints/ap/show.ts
@@ -7,7 +7,7 @@ import { Inject, Injectable } from '@nestjs/common';
 import { Endpoint } from '@/server/api/endpoint-base.js';
 import type { MiNote } from '@/models/Note.js';
 import type { MiLocalUser, MiUser } from '@/models/User.js';
-import { isActor, isPost, getApId, getNullableApId } from '@/core/activitypub/type.js';
+import { isActor, isPost, getApId } from '@/core/activitypub/type.js';
 import type { SchemaType } from '@/misc/json-schema.js';
 import { ApResolverService } from '@/core/activitypub/ApResolverService.js';
 import { ApDbResolverService } from '@/core/activitypub/ApDbResolverService.js';
@@ -18,10 +18,9 @@ import { NoteEntityService } from '@/core/entities/NoteEntityService.js';
 import { UtilityService } from '@/core/UtilityService.js';
 import { bindThis } from '@/decorators.js';
 import { ApRequestService } from '@/core/activitypub/ApRequestService.js';
-import { InstanceActorService } from '@/core/InstanceActorService.js';
+import { SystemAccountService } from '@/core/SystemAccountService.js';
 import { ApiError } from '../../error.js';
 import { IdentifiableError } from '@/misc/identifiable-error.js';
-import { FetchAllowSoftFailMask } from '@/core/activitypub/misc/check-against-url.js';
 
 export const meta = {
 	tags: ['federation'],
@@ -119,7 +118,7 @@ export default class extends Endpoint<typeof meta, typeof paramDef> { // eslint-
 		private apPersonService: ApPersonService,
 		private apNoteService: ApNoteService,
 		private readonly apRequestService: ApRequestService,
-		private readonly instanceActorService: InstanceActorService,
+		private readonly systemAccountService: SystemAccountService,
 	) {
 		super(meta, paramDef, async (ps, me) => {
 			const object = await this.fetchAny(ps.uri, me);
@@ -140,7 +139,7 @@ export default class extends Endpoint<typeof meta, typeof paramDef> { // eslint-
 			throw new ApiError(meta.errors.federationNotAllowed);
 		}
 
-		let local = await this.mergePack(me, ...await Promise.all([
+		const local = await this.mergePack(me, ...await Promise.all([
 			this.apDbResolverService.getUserFromApId(uri),
 			this.apDbResolverService.getNoteFromApId(uri),
 		]));
@@ -149,7 +148,7 @@ export default class extends Endpoint<typeof meta, typeof paramDef> { // eslint-
 		// No local object found with that uri.
 		// Before we fetch, resolve the URI in case it has a cross-origin redirect or anything like that.
 		// Resolver.resolve() uses strict verification, which is overly paranoid for a user-provided lookup.
-		uri = await this.resolveCanonicalUri(uri); // eslint-disable-line no-param-reassign
+		uri = await this.resolveCanonicalUri(uri);
 		if (!this.utilityService.isFederationAllowedUri(uri)) {
 			throw new ApiError(meta.errors.federationNotAllowed);
 		}
@@ -161,8 +160,7 @@ export default class extends Endpoint<typeof meta, typeof paramDef> { // eslint-
 
 		// リモートから一旦オブジェクトフェッチ
 		const resolver = this.apResolverService.createResolver();
-		// allow ap/show exclusively to lookup URLs that are cross-origin or non-canonical (like https://alice.example.com/@bob@bob.example.com -> https://bob.example.com/@bob)
-		const object = await resolver.resolve(uri, FetchAllowSoftFailMask.CrossOrigin | FetchAllowSoftFailMask.NonCanonicalId).catch((err) => {
+		const object = await resolver.resolve(uri).catch((err) => {
 			if (err instanceof IdentifiableError) {
 				switch (err.id) {
 					// resolve
@@ -190,25 +188,13 @@ export default class extends Endpoint<typeof meta, typeof paramDef> { // eslint-
 			throw new ApiError(meta.errors.requestFailed);
 		});
 
-		if (object.id == null) {
-			throw new ApiError(meta.errors.responseInvalid);
-		}
-
-		// /@user のような正規id以外で取得できるURIが指定されていた場合、ここで初めて正規URIが確定する
-		// これはDBに存在する可能性があるため再度DB検索
-		if (uri !== object.id) {
-			local = await this.mergePack(me, ...await Promise.all([
-				this.apDbResolverService.getUserFromApId(object.id),
-				this.apDbResolverService.getNoteFromApId(object.id),
-			]));
-			if (local != null) return local;
-		}
-
-		// 同一ユーザーの情報を再度処理するので、使用済みのresolverを再利用してはいけない
+		// Object is already validated to have a valid id (URI).
+		// We can pass it through with the same resolver and sentFrom to avoid a duplicate fetch.
+		// The resolve* methods automatically check for locally cached copies.
 		return await this.mergePack(
 			me,
-			isActor(object) ? await this.apPersonService.createPerson(getApId(object)) : null,
-			isPost(object) ? await this.apNoteService.createNote(getApId(object), undefined, undefined, true) : null,
+			isActor(object) ? await this.apPersonService.resolvePerson(object, resolver, uri) : null,
+			isPost(object) ? await this.apNoteService.resolveNote(object, { resolver, sentFrom: uri }) : null,
 		);
 	}
 
@@ -239,8 +225,8 @@ export default class extends Endpoint<typeof meta, typeof paramDef> { // eslint-
 	 * Resolves an arbitrary URI to its canonical, post-redirect form.
 	 */
 	private async resolveCanonicalUri(uri: string): Promise<string> {
-		const user = await this.instanceActorService.getInstanceActor();
+		const user = await this.systemAccountService.getInstanceActor();
 		const res = await this.apRequestService.signedGet(uri, user, true);
-		return getNullableApId(res) ?? uri;
+		return getApId(res);
 	}
 }
diff --git a/packages/backend/test/unit/activitypub.ts b/packages/backend/test/unit/activitypub.ts
index 8f1e792829..6f6d4c4121 100644
--- a/packages/backend/test/unit/activitypub.ts
+++ b/packages/backend/test/unit/activitypub.ts
@@ -2,7 +2,6 @@
  * SPDX-FileCopyrightText: syuilo and misskey-project
  * SPDX-License-Identifier: AGPL-3.0-only
  */
-
 process.env.NODE_ENV = 'test';
 
 import * as assert from 'assert';
diff --git a/packages/backend/test/unit/ap-request.ts b/packages/backend/test/unit/ap-request.ts
index f8b2a697f2..11364e1735 100644
--- a/packages/backend/test/unit/ap-request.ts
+++ b/packages/backend/test/unit/ap-request.ts
@@ -8,7 +8,6 @@ import httpSignature from '@peertube/http-signature';
 
 import { genRsaKeyPair } from '@/misc/gen-key-pair.js';
 import { ApRequestCreator } from '@/core/activitypub/ApRequestService.js';
-import { assertActivityMatchesUrl, FetchAllowSoftFailMask } from '@/core/activitypub/misc/check-against-url.js';
 import { IObject } from '@/core/activitypub/type.js';
 
 export const buildParsedSignature = (signingString: string, signature: string, algorithm: string) => {
@@ -65,6 +64,7 @@ describe('ap-request', () => {
 		assert.deepStrictEqual(result, true);
 	});
 
+	/*
 	test('rejects non matching domain', () => {
 		assert.doesNotThrow(() => assertActivityMatchesUrl(
 			'https://alice.example.com/abc',
@@ -78,7 +78,7 @@ describe('ap-request', () => {
 			'https://alice.example.com/abc',
 			FetchAllowSoftFailMask.Any,
 		), 'validation should fail no matter what if the response URL is inconsistent with the object ID');
-		
+
 		assert.doesNotThrow(() => assertActivityMatchesUrl(
 			'https://alice.example.com/abc#test',
 			{ id: 'https://alice.example.com/abc' } as IObject,
@@ -168,4 +168,5 @@ describe('ap-request', () => {
 			FetchAllowSoftFailMask.Strict,
 		), 'throws if HTTP downgrade is detected');
 	});
+	*/
 });
-- 
cgit v1.2.3-freya


From fb63167d854f277ffeb82da720580b3e99f9cc8c Mon Sep 17 00:00:00 2001
From: Hazelnoot <acomputerdog@gmail.com>
Date: Sun, 11 May 2025 23:22:41 -0400
Subject: allow private IP ranges to specify allowed ports

---
 packages/backend/src/config.ts                     |  33 ++++++-
 packages/backend/src/core/HttpRequestService.ts    |  68 ++++++--------
 .../backend/test/unit/core/HttpRequestService.ts   | 103 +++++++++++++++++++++
 3 files changed, 161 insertions(+), 43 deletions(-)
 create mode 100644 packages/backend/test/unit/core/HttpRequestService.ts

(limited to 'packages/backend/src/core/HttpRequestService.ts')

diff --git a/packages/backend/src/config.ts b/packages/backend/src/config.ts
index 92fc2b8a13..2a3184f9b4 100644
--- a/packages/backend/src/config.ts
+++ b/packages/backend/src/config.ts
@@ -8,9 +8,11 @@ import { fileURLToPath } from 'node:url';
 import { dirname, resolve } from 'node:path';
 import * as yaml from 'js-yaml';
 import { globSync } from 'glob';
+import ipaddr from 'ipaddr.js';
 import type * as Sentry from '@sentry/node';
 import type * as SentryVue from '@sentry/vue';
 import type { RedisOptions } from 'ioredis';
+import type { IPv4, IPv6 } from 'ipaddr.js';
 
 type RedisOptionsSource = Partial<RedisOptions> & {
 	host?: string;
@@ -152,6 +154,33 @@ type Source = {
 	}
 };
 
+export type PrivateNetwork = {
+	/**
+	 * CIDR IP/netmask definition of the IP range to match.
+	 */
+	cidr: [ip: IPv4 | IPv6, mask: number];
+
+	/**
+	 * List of ports to match.
+	 * If undefined, then all ports match.
+	 * If empty, then NO ports match.
+	 */
+	ports?: number[];
+};
+
+export function parsePrivateNetworks(patterns: string[]): PrivateNetwork[];
+export function parsePrivateNetworks(patterns: undefined): undefined;
+export function parsePrivateNetworks(patterns: string[] | undefined): PrivateNetwork[] | undefined;
+export function parsePrivateNetworks(patterns: string[] | undefined): PrivateNetwork[] | undefined {
+	return patterns?.map(e => {
+		const [ip, ports] = e.split('#') as [string, ...(string | undefined)[]];
+		return {
+			cidr: ipaddr.parseCIDR(ip),
+			ports: ports?.split(',').map(p => parseInt(p)),
+		};
+	});
+}
+
 export type Config = {
 	url: string;
 	port: number;
@@ -190,7 +219,7 @@ export type Config = {
 	proxy: string | undefined;
 	proxySmtp: string | undefined;
 	proxyBypassHosts: string[] | undefined;
-	allowedPrivateNetworks: string[] | undefined;
+	allowedPrivateNetworks: PrivateNetwork[] | undefined;
 	disallowExternalApRedirect: boolean;
 	maxFileSize: number;
 	maxNoteLength: number;
@@ -382,7 +411,7 @@ export function loadConfig(): Config {
 		proxy: config.proxy,
 		proxySmtp: config.proxySmtp,
 		proxyBypassHosts: config.proxyBypassHosts,
-		allowedPrivateNetworks: config.allowedPrivateNetworks,
+		allowedPrivateNetworks: parsePrivateNetworks(config.allowedPrivateNetworks),
 		disallowExternalApRedirect: config.disallowExternalApRedirect ?? false,
 		maxFileSize: config.maxFileSize ?? 262144000,
 		maxNoteLength: config.maxNoteLength ?? 3000,
diff --git a/packages/backend/src/core/HttpRequestService.ts b/packages/backend/src/core/HttpRequestService.ts
index 12047346fb..7c086c9976 100644
--- a/packages/backend/src/core/HttpRequestService.ts
+++ b/packages/backend/src/core/HttpRequestService.ts
@@ -12,7 +12,7 @@ import fetch from 'node-fetch';
 import { HttpProxyAgent, HttpsProxyAgent } from 'hpagent';
 import { Inject, Injectable } from '@nestjs/common';
 import { DI } from '@/di-symbols.js';
-import type { Config } from '@/config.js';
+import type { Config, PrivateNetwork } from '@/config.js';
 import { StatusError } from '@/misc/status-error.js';
 import { bindThis } from '@/decorators.js';
 import { validateContentTypeSetAsActivityPub } from '@/core/activitypub/misc/validator.js';
@@ -20,12 +20,36 @@ import type { IObject, IObjectWithId } from '@/core/activitypub/type.js';
 import { ApUtilityService } from './activitypub/ApUtilityService.js';
 import type { Response } from 'node-fetch';
 import type { URL } from 'node:url';
+import type { Socket } from 'node:net';
 
 export type HttpRequestSendOptions = {
 	throwErrorWhenResponseNotOk: boolean;
 	validators?: ((res: Response) => void)[];
 };
 
+export function isPrivateIp(allowedPrivateNetworks: PrivateNetwork[] | undefined, ip: string, port?: number): boolean {
+	const parsedIp = ipaddr.parse(ip);
+
+	for (const { cidr, ports } of allowedPrivateNetworks ?? []) {
+		if (cidr[0].kind() === parsedIp.kind() && parsedIp.match(cidr)) {
+			if (port == null || ports == null || ports.includes(port)) {
+				return false;
+			}
+		}
+	}
+
+	return parsedIp.range() !== 'unicast';
+}
+
+export function validateSocketConnect(allowedPrivateNetworks: PrivateNetwork[] | undefined, socket: Socket): void {
+	const address = socket.remoteAddress;
+	if (address && ipaddr.isValid(address)) {
+		if (isPrivateIp(allowedPrivateNetworks, address, socket.remotePort)) {
+			socket.destroy(new Error(`Blocked address: ${address}`));
+		}
+	}
+}
+
 declare module 'node:http' {
 	interface Agent {
 		createConnection(options: net.NetConnectOpts, callback?: (err: unknown, stream: net.Socket) => void): net.Socket;
@@ -44,31 +68,12 @@ class HttpRequestServiceAgent extends http.Agent {
 	public createConnection(options: net.NetConnectOpts, callback?: (err: unknown, stream: net.Socket) => void): net.Socket {
 		const socket = super.createConnection(options, callback)
 			.on('connect', () => {
-				const address = socket.remoteAddress;
 				if (process.env.NODE_ENV === 'production') {
-					if (address && ipaddr.isValid(address)) {
-						if (this.isPrivateIp(address)) {
-							socket.destroy(new Error(`Blocked address: ${address}`));
-						}
-					}
+					validateSocketConnect(this.config.allowedPrivateNetworks, socket);
 				}
 			});
 		return socket;
 	}
-
-	@bindThis
-	private isPrivateIp(ip: string): boolean {
-		const parsedIp = ipaddr.parse(ip);
-
-		for (const net of this.config.allowedPrivateNetworks ?? []) {
-			const cidr = ipaddr.parseCIDR(net);
-			if (cidr[0].kind() === parsedIp.kind() && parsedIp.match(ipaddr.parseCIDR(net))) {
-				return false;
-			}
-		}
-
-		return parsedIp.range() !== 'unicast';
-	}
 }
 
 class HttpsRequestServiceAgent extends https.Agent {
@@ -83,31 +88,12 @@ class HttpsRequestServiceAgent extends https.Agent {
 	public createConnection(options: net.NetConnectOpts, callback?: (err: unknown, stream: net.Socket) => void): net.Socket {
 		const socket = super.createConnection(options, callback)
 			.on('connect', () => {
-				const address = socket.remoteAddress;
 				if (process.env.NODE_ENV === 'production') {
-					if (address && ipaddr.isValid(address)) {
-						if (this.isPrivateIp(address)) {
-							socket.destroy(new Error(`Blocked address: ${address}`));
-						}
-					}
+					validateSocketConnect(this.config.allowedPrivateNetworks, socket);
 				}
 			});
 		return socket;
 	}
-
-	@bindThis
-	private isPrivateIp(ip: string): boolean {
-		const parsedIp = ipaddr.parse(ip);
-
-		for (const net of this.config.allowedPrivateNetworks ?? []) {
-			const cidr = ipaddr.parseCIDR(net);
-			if (cidr[0].kind() === parsedIp.kind() && parsedIp.match(ipaddr.parseCIDR(net))) {
-				return false;
-			}
-		}
-
-		return parsedIp.range() !== 'unicast';
-	}
 }
 
 @Injectable()
diff --git a/packages/backend/test/unit/core/HttpRequestService.ts b/packages/backend/test/unit/core/HttpRequestService.ts
new file mode 100644
index 0000000000..3185b91567
--- /dev/null
+++ b/packages/backend/test/unit/core/HttpRequestService.ts
@@ -0,0 +1,103 @@
+/*
+ * SPDX-FileCopyrightText: hazelnoot and other Sharkey contributors
+ * SPDX-License-Identifier: AGPL-3.0-only
+ */
+
+import { 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 { parsePrivateNetworks } from '@/config.js';
+
+describe(HttpRequestService, () => {
+	let allowedPrivateNetworks: PrivateNetwork[] | undefined;
+
+	beforeEach(() => {
+		allowedPrivateNetworks = parsePrivateNetworks([
+			'10.0.0.1/32',
+			'127.0.0.1/32#1',
+			'127.0.0.1/32#3,4,5',
+		]);
+	});
+
+	describe('isPrivateIp', () => {
+		it('should return false when ip public', () => {
+			const result = isPrivateIp(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);
+			expect(result).toBeFalsy();
+		});
+
+		it('should return true when ip private and no ports specified', () => {
+			const result = isPrivateIp(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);
+			expect(result).toBeTruthy();
+		});
+	});
+
+	describe('validateSocketConnect', () => {
+		let fakeSocket: Socket;
+		let fakeSocketMutable: {
+			remoteAddress: string | undefined;
+			remotePort: number | undefined;
+			destroy: Mock<(error?: Error) => void>;
+		};
+
+		beforeEach(() => {
+			fakeSocketMutable = {
+				remoteAddress: '74.125.127.100',
+				remotePort: 80,
+				destroy: jest.fn<(error?: Error) => void>(),
+			};
+			fakeSocket = fakeSocketMutable as unknown as Socket;
+		});
+
+		it('should accept when IP is empty', () => {
+			fakeSocketMutable.remoteAddress = undefined;
+
+			validateSocketConnect(allowedPrivateNetworks, fakeSocket);
+
+			expect(fakeSocket.destroy).not.toHaveBeenCalled();
+		});
+
+		it('should accept when IP is invalid', () => {
+			fakeSocketMutable.remoteAddress = 'AB939ajd9jdajsdja8jj';
+
+			validateSocketConnect(allowedPrivateNetworks, fakeSocket);
+
+			expect(fakeSocket.destroy).not.toHaveBeenCalled();
+		});
+
+		it('should accept when IP is valid', () => {
+			validateSocketConnect(allowedPrivateNetworks, fakeSocket);
+
+			expect(fakeSocket.destroy).not.toHaveBeenCalled();
+		});
+
+		it('should accept when IP is private and port match', () => {
+			fakeSocketMutable.remoteAddress = '127.0.0.1';
+			fakeSocketMutable.remotePort = 1;
+
+			validateSocketConnect(allowedPrivateNetworks, fakeSocket);
+
+			expect(fakeSocket.destroy).not.toHaveBeenCalled();
+		});
+
+		it('should reject when IP is private and port no match', () => {
+			fakeSocketMutable.remoteAddress = '127.0.0.1';
+			fakeSocketMutable.remotePort = 2;
+
+			validateSocketConnect(allowedPrivateNetworks, fakeSocket);
+
+			expect(fakeSocket.destroy).toHaveBeenCalled();
+		});
+	});
+});
-- 
cgit v1.2.3-freya


From ebd4ccdd55a509e02fd8964061b90361d6c93924 Mon Sep 17 00:00:00 2001
From: Hazelnoot <acomputerdog@gmail.com>
Date: Tue, 13 May 2025 22:22:40 -0400
Subject: enforce port restrictions against requests that happen to be missing
 the port

---
 packages/backend/src/core/HttpRequestService.ts | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

(limited to 'packages/backend/src/core/HttpRequestService.ts')

diff --git a/packages/backend/src/core/HttpRequestService.ts b/packages/backend/src/core/HttpRequestService.ts
index 7c086c9976..2951691129 100644
--- a/packages/backend/src/core/HttpRequestService.ts
+++ b/packages/backend/src/core/HttpRequestService.ts
@@ -32,7 +32,7 @@ export function isPrivateIp(allowedPrivateNetworks: PrivateNetwork[] | undefined
 
 	for (const { cidr, ports } of allowedPrivateNetworks ?? []) {
 		if (cidr[0].kind() === parsedIp.kind() && parsedIp.match(cidr)) {
-			if (port == null || ports == null || ports.includes(port)) {
+			if (ports == null || (port != null && ports.includes(port))) {
 				return false;
 			}
 		}
-- 
cgit v1.2.3-freya


From 7f5e43530f10ed8f7515aa87b0598637b8361ded Mon Sep 17 00:00:00 2001
From: Hazelnoot <acomputerdog@gmail.com>
Date: Sun, 25 May 2025 00:04:27 -0400
Subject: enforce HTTPS for all federation

---
 packages/backend/src/core/HttpRequestService.ts    |  2 ++
 .../src/core/activitypub/ApRequestService.ts       |  4 +++
 .../src/core/activitypub/ApUtilityService.ts       | 38 ++++++++++++++++++++--
 .../src/core/activitypub/models/ApNoteService.ts   |  1 +
 .../src/core/activitypub/models/ApPersonService.ts |  4 +++
 5 files changed, 46 insertions(+), 3 deletions(-)

(limited to 'packages/backend/src/core/HttpRequestService.ts')

diff --git a/packages/backend/src/core/HttpRequestService.ts b/packages/backend/src/core/HttpRequestService.ts
index 2951691129..5c271b81e3 100644
--- a/packages/backend/src/core/HttpRequestService.ts
+++ b/packages/backend/src/core/HttpRequestService.ts
@@ -236,6 +236,8 @@ export class HttpRequestService {
 
 	@bindThis
 	public async getActivityJson(url: string, isLocalAddressAllowed = false): Promise<IObjectWithId> {
+		this.apUtilityService.assertApUrl(url);
+
 		const res = await this.send(url, {
 			method: 'GET',
 			headers: {
diff --git a/packages/backend/src/core/activitypub/ApRequestService.ts b/packages/backend/src/core/activitypub/ApRequestService.ts
index 7118ce1e02..b665b51700 100644
--- a/packages/backend/src/core/activitypub/ApRequestService.ts
+++ b/packages/backend/src/core/activitypub/ApRequestService.ts
@@ -155,6 +155,8 @@ export class ApRequestService {
 
 	@bindThis
 	public async signedPost(user: { id: MiUser['id'] }, url: string, object: unknown, digest?: string): Promise<void> {
+		this.apUtilityService.assertApUrl(url);
+
 		const body = typeof object === 'string' ? object : JSON.stringify(object);
 
 		const keypair = await this.userKeypairService.getUserKeypair(user.id);
@@ -186,6 +188,8 @@ export class ApRequestService {
 	 */
 	@bindThis
 	public async signedGet(url: string, user: { id: MiUser['id'] }, followAlternate?: boolean): Promise<IObjectWithId> {
+		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 ae6e4997e4..c3958cdf42 100644
--- a/packages/backend/src/core/activitypub/ApUtilityService.ts
+++ b/packages/backend/src/core/activitypub/ApUtilityService.ts
@@ -77,16 +77,48 @@ 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 contains a fragment
+	 * @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`);
+			}
+		}
+
+		// Hash component breaks federation
+		if (url.hash) {
+			throw new IdentifiableError('0bedd29b-e3bf-4604-af51-d3352e2518af', `invalid AP url ${url}: contains a fragment (#)`);
+		}
+
+		// 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): boolean {
+	private checkHttps(url: string | URL): boolean {
 		const isNonProd = this.envService.env.NODE_ENV !== 'production';
 
-		// noinspection HttpUrlsUsage
-		return url.startsWith('https://') || (url.startsWith('http://') && isNonProd);
+		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/models/ApNoteService.ts b/packages/backend/src/core/activitypub/models/ApNoteService.ts
index f6152e3888..7811b81795 100644
--- a/packages/backend/src/core/activitypub/models/ApNoteService.ts
+++ b/packages/backend/src/core/activitypub/models/ApNoteService.ts
@@ -95,6 +95,7 @@ export class ApNoteService {
 		actor?: MiRemoteUser,
 		user?: MiRemoteUser,
 	): Error | null {
+		this.apUtilityService.assertApUrl(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 4b685f7e1b..5c6716a0b8 100644
--- a/packages/backend/src/core/activitypub/models/ApPersonService.ts
+++ b/packages/backend/src/core/activitypub/models/ApPersonService.ts
@@ -153,6 +153,7 @@ export class ApPersonService implements OnModuleInit, OnApplicationShutdown {
 	 */
 	@bindThis
 	private validateActor(x: IObject, uri: string): IActor {
+		this.apUtilityService.assertApUrl(uri);
 		const expectHost = this.utilityService.punyHostPSLDomain(uri);
 
 		if (!isActor(x)) {
@@ -167,6 +168,7 @@ export class ApPersonService implements OnModuleInit, OnApplicationShutdown {
 			throw new UnrecoverableError(`invalid Actor ${uri} - wrong inbox type`);
 		}
 
+		this.apUtilityService.assertApUrl(x.inbox);
 		const inboxHost = this.utilityService.punyHostPSLDomain(x.inbox);
 		if (inboxHost !== expectHost) {
 			throw new UnrecoverableError(`invalid Actor ${uri} - wrong inbox ${inboxHost}`);
@@ -175,6 +177,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);
 			if (!(typeof sharedInbox === 'string' && sharedInbox.length > 0 && this.utilityService.punyHostPSLDomain(sharedInbox) === expectHost)) {
 				throw new UnrecoverableError(`invalid Actor ${uri} - wrong shared inbox ${sharedInbox}`);
 			}
@@ -185,6 +188,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);
 					if (this.utilityService.punyHostPSLDomain(collectionUri) !== expectHost) {
 						throw new UnrecoverableError(`invalid Actor ${uri} - wrong ${collection} ${collectionUri}`);
 					}
-- 
cgit v1.2.3-freya