merge: Emit log messages with correct level (!1097)

View MR for information: https://activitypub.software/TransFem-org/Sharkey/-/merge_requests/1097

Approved-by: dakkar <dakkar@thenautilus.net>
Approved-by: Marie <github@yuugi.dev>

5 files changed, 68 insertions, 45 deletions

diff --git a/packages/backend/src/core/activitypub/ApRendererService.ts b/packages/backend/src/core/activitypub/ApRendererService.ts
index 23a52a248a..6068d707de 100644
--- a/packages/backend/src/core/activitypub/ApRendererService.ts
+++ b/packages/backend/src/core/activitypub/ApRendererService.ts
@@ -960,9 +960,7 @@ export class ApRendererService {
 
 		const keypair = await this.userKeypairService.getUserKeypair(user.id);
 
-		const jsonLd = this.jsonLdService.use();
-		jsonLd.debug = false;
-		activity = await jsonLd.signRsaSignature2017(activity, keypair.privateKey, `${this.config.url}/users/${user.id}#main-key`);
+		activity = await this.jsonLdService.signRsaSignature2017(activity, keypair.privateKey, `${this.config.url}/users/${user.id}#main-key`);
 
 		return activity;
 	}
diff --git a/packages/backend/src/core/activitypub/JsonLdService.ts b/packages/backend/src/core/activitypub/JsonLdService.ts
index 1db5df6ad9..8f150ab201 100644
--- a/packages/backend/src/core/activitypub/JsonLdService.ts
+++ b/packages/backend/src/core/activitypub/JsonLdService.ts
@@ -8,26 +8,61 @@ import { Injectable } from '@nestjs/common';
 import { UnrecoverableError } from 'bullmq';
 import { HttpRequestService } from '@/core/HttpRequestService.js';
 import { bindThis } from '@/decorators.js';
+import Logger from '@/logger.js';
+import { LoggerService } from '@/core/LoggerService.js';
 import { StatusError } from '@/misc/status-error.js';
 import { CONTEXT, PRELOADED_CONTEXTS } from './misc/contexts.js';
 import { validateContentTypeSetAsJsonLD } from './misc/validator.js';
-import type { JsonLdDocument } from 'jsonld';
+import type { ContextDefinition, JsonLdDocument } from 'jsonld';
 import type { JsonLd as JsonLdObject, RemoteDocument } from 'jsonld/jsonld-spec.js';
 
+// https://stackoverflow.com/a/66252656
+type RemoveIndex<T> = {
+	[ K in keyof T as string extends K
+		? never
+		: number extends K
+			? never
+			: symbol extends K
+				? never
+				: K
+	] : T[K];
+};
+
+export type Document = RemoveIndex<JsonLdDocument>;
+
+export type Signature = {
+	id?: string;
+	type: string;
+	creator: string;
+	domain?: string;
+	nonce: string;
+	created: string;
+	signatureValue: string;
+};
+
+export type Signed<T extends Document> = T & {
+	signature: Signature;
+};
+
+export function isSigned<T extends Document>(doc: T): doc is Signed<T> {
+	return 'signature' in doc && typeof(doc.signature) === 'object';
+}
+
 // RsaSignature2017 implementation is based on https://github.com/transmute-industries/RsaSignature2017
 
-class JsonLd {
-	public debug = false;
-	public preLoad = true;
-	public loderTimeout = 5000;
+@Injectable()
+export class JsonLdService {
+	private readonly logger: Logger;
 
 	constructor(
 		private httpRequestService: HttpRequestService,
+		loggerService: LoggerService,
 	) {
+		this.logger = loggerService.getLogger('json-ld');
 	}
 
 	@bindThis
-	public async signRsaSignature2017(data: any, privateKey: string, creator: string, domain?: string, created?: Date): Promise<any> {
+	public async signRsaSignature2017<T extends Document>(data: T, privateKey: string, creator: string, domain?: string, created?: Date): Promise<Signed<T>> {
 		const options: {
 			type: string;
 			creator: string;
@@ -63,7 +98,7 @@ class JsonLd {
 	}
 
 	@bindThis
-	public async verifyRsaSignature2017(data: any, publicKey: string): Promise<boolean> {
+	public async verifyRsaSignature2017(data: Signed<Document>, publicKey: string): Promise<boolean> {
 		const toBeSigned = await this.createVerifyData(data, data.signature);
 		const verifier = crypto.createVerify('sha256');
 		verifier.update(toBeSigned);
@@ -71,7 +106,7 @@ class JsonLd {
 	}
 
 	@bindThis
-	public async createVerifyData(data: any, options: any): Promise<string> {
+	public async createVerifyData<T extends Document>(data: T, options: Partial<Signature>): Promise<string> {
 		const transformedOptions = {
 			...options,
 			'@context': 'https://w3id.org/identity/v1',
@@ -81,17 +116,18 @@ class JsonLd {
 		delete transformedOptions['signatureValue'];
 		const canonizedOptions = await this.normalize(transformedOptions);
 		const optionsHash = this.sha256(canonizedOptions.toString());
-		const transformedData = { ...data };
+		const transformedData = { ...data } as T & { signature?: unknown };
 		delete transformedData['signature'];
 		const cannonidedData = await this.normalize(transformedData);
-		if (this.debug) console.debug(`cannonidedData: ${cannonidedData}`);
+		this.logger.debug('cannonidedData', cannonidedData);
 		const documentHash = this.sha256(cannonidedData.toString());
 		const verifyData = `${optionsHash}${documentHash}`;
 		return verifyData;
 	}
 
 	@bindThis
-	public async compact(data: any, context: any = CONTEXT): Promise<JsonLdDocument> {
+	// TODO our default CONTEXT isn't valid for the library, is this a bug?
+	public async compact(data: Document, context: ContextDefinition = CONTEXT as unknown as ContextDefinition): Promise<Document> {
 		const customLoader = this.getLoader();
 		// XXX: Importing jsonld dynamically since Jest frequently fails to import it statically
 		// https://github.com/misskey-dev/misskey/pull/9894#discussion_r1103753595
@@ -101,7 +137,7 @@ class JsonLd {
 	}
 
 	@bindThis
-	public async normalize(data: JsonLdDocument): Promise<string> {
+	public async normalize(data: Document): Promise<string> {
 		const customLoader = this.getLoader();
 		return (await import('jsonld')).default.normalize(data, {
 			documentLoader: customLoader,
@@ -113,9 +149,9 @@ class JsonLd {
 		return async (url: string): Promise<RemoteDocument> => {
 			if (!/^https?:\/\//.test(url)) throw new UnrecoverableError(`Invalid URL: ${url}`);
 
-			if (this.preLoad) {
+			{
 				if (url in PRELOADED_CONTEXTS) {
-					if (this.debug) console.debug(`HIT: ${url}`);
+					this.logger.debug(`Preload HIT: ${url}`);
 					return {
 						contextUrl: undefined,
 						document: PRELOADED_CONTEXTS[url],
@@ -124,7 +160,7 @@ class JsonLd {
 				}
 			}
 
-			if (this.debug) console.debug(`MISS: ${url}`);
+			this.logger.debug(`Preload MISS: ${url}`);
 			const document = await this.fetchDocument(url);
 			return {
 				contextUrl: undefined,
@@ -142,7 +178,6 @@ class JsonLd {
 				headers: {
 					Accept: 'application/ld+json, application/json',
 				},
-				timeout: this.loderTimeout,
 			},
 			{
 				throwErrorWhenResponseNotOk: false,
@@ -166,16 +201,3 @@ class JsonLd {
 		return hash.digest('hex');
 	}
 }
-
-@Injectable()
-export class JsonLdService {
-	constructor(
-		private httpRequestService: HttpRequestService,
-	) {
-	}
-
-	@bindThis
-	public use(): JsonLd {
-		return new JsonLd(this.httpRequestService);
-	}
-}
diff --git a/packages/backend/src/logger.ts b/packages/backend/src/logger.ts
index ca9b494ff2..4bf45fc76b 100644
--- a/packages/backend/src/logger.ts
+++ b/packages/backend/src/logger.ts
@@ -23,6 +23,14 @@ export type DataElement = DataObject | Error | string | null;
 // https://stackoverflow.com/questions/61148466/typescript-type-that-matches-any-object-but-not-arrays
 export type DataObject = Record<string, unknown> | (object & { length?: never; });
 
+const levelFuncs = {
+	error: 'error',
+	warning: 'warn',
+	success: 'info',
+	info: 'log',
+	debug: 'debug',
+} as const satisfies Record<Level, keyof typeof console>;
+
 // eslint-disable-next-line import/no-default-export
 export default class Logger {
 	private context: Context;
@@ -86,7 +94,7 @@ export default class Logger {
 		} else if (data != null) {
 			args.push(data);
 		}
-		console.log(...args);
+		console[levelFuncs[level]](...args);
 	}
 
 	@bindThis
diff --git a/packages/backend/src/queue/processors/InboxProcessorService.ts b/packages/backend/src/queue/processors/InboxProcessorService.ts
index 612b16dfbf..5f82d558b3 100644
--- a/packages/backend/src/queue/processors/InboxProcessorService.ts
+++ b/packages/backend/src/queue/processors/InboxProcessorService.ts
@@ -21,7 +21,7 @@ import { ApDbResolverService } from '@/core/activitypub/ApDbResolverService.js';
 import { StatusError } from '@/misc/status-error.js';
 import { UtilityService } from '@/core/UtilityService.js';
 import { ApPersonService } from '@/core/activitypub/models/ApPersonService.js';
-import { JsonLdService } from '@/core/activitypub/JsonLdService.js';
+import { isSigned, JsonLdService } from '@/core/activitypub/JsonLdService.js';
 import { ApInboxService } from '@/core/activitypub/ApInboxService.js';
 import { bindThis } from '@/decorators.js';
 import { IdentifiableError } from '@/misc/identifiable-error.js';
@@ -179,8 +179,8 @@ export class InboxProcessorService implements OnApplicationShutdown {
 		// また、signatureのsignerは、activity.actorと一致する必要がある
 		if (!httpSignatureValidated || authUser.user.uri !== actorId) {
 			// 一致しなくても、でもLD-Signatureがありそうならそっちも見る
-			const ldSignature = activity.signature;
-			if (ldSignature) {
+			if (isSigned(activity)) {
+				const ldSignature = activity.signature;
 				if (ldSignature.type !== 'RsaSignature2017') {
 					throw new Bull.UnrecoverableError(`skip: unsupported LD-signature type ${ldSignature.type}`);
 				}
@@ -202,24 +202,21 @@ export class InboxProcessorService implements OnApplicationShutdown {
 					throw new Bull.UnrecoverableError('skip: LD-SignatureのユーザーはpublicKeyを持っていませんでした');
 				}
 
-				const jsonLd = this.jsonLdService.use();
-
 				// LD-Signature検証
-				const verified = await jsonLd.verifyRsaSignature2017(activity, authUser.key.keyPem).catch(() => false);
+				const verified = await this.jsonLdService.verifyRsaSignature2017(activity, authUser.key.keyPem).catch(() => false);
 				if (!verified) {
 					throw new Bull.UnrecoverableError('skip: LD-Signatureの検証に失敗しました');
 				}
 
 				// アクティビティを正規化
-				delete activity.signature;
+				const copy = { ...activity, signature: undefined };
 				try {
-					activity = await jsonLd.compact(activity) as IActivity;
+					activity = await this.jsonLdService.compact(copy) as IActivity;
 				} catch (e) {
 					throw new Bull.UnrecoverableError(`skip: failed to compact activity: ${e}`);
 				}
 				// TODO: 元のアクティビティと非互換な形に正規化される場合は転送をスキップする
 				// https://github.com/mastodon/mastodon/blob/664b0ca/app/services/activitypub/process_collection_service.rb#L24-L29
-				activity.signature = ldSignature;
 
 				// もう一度actorチェック
 				if (authUser.user.uri !== actorId) {
diff --git a/packages/backend/test/unit/activitypub.ts b/packages/backend/test/unit/activitypub.ts
index 45975275d4..3ad9cdc4d5 100644
--- a/packages/backend/test/unit/activitypub.ts
+++ b/packages/backend/test/unit/activitypub.ts
@@ -479,8 +479,6 @@ describe('ActivityPub', () => {
 
 	describe('JSON-LD', () => {
 		test('Compaction', async () => {
-			const jsonLd = jsonLdService.use();
-
 			const object = {
 				'@context': [
 					'https://www.w3.org/ns/activitystreams',
@@ -499,7 +497,7 @@ describe('ActivityPub', () => {
 				unknown: 'test test bar',
 				undefined: 'test test baz',
 			};
-			const compacted = await jsonLd.compact(object);
+			const compacted = await jsonLdService.compact(object);
 
 			assert.deepStrictEqual(compacted, {
 				'@context': CONTEXT,