From 616cccf2511337fc181d0b6aa693b7091c7ba57b Mon Sep 17 00:00:00 2001 From: syuilo <4439005+syuilo@users.noreply.github.com> Date: Sun, 2 Mar 2025 20:06:20 +0900 Subject: enhance(backend): refine system account (#15530) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * wip * wip * wip * Update SystemAccountService.ts * Update 1740121393164-system-accounts.js * Update DeleteAccountService.ts * wip * wip * wip * wip * Update 1740121393164-system-accounts.js * Update RepositoryModule.ts * wip * wip * wip * Update ApRendererService.ts * wip * wip * Update SystemAccountService.ts * fix tests * fix tests * fix tests * fix tests * fix tests * fix tests * add print logs * ログが長すぎて出てないかもしれない * fix migration * refactor * fix fed-tests * Update RelayService.ts * merge * Update user.test.ts * chore: emit log * fix: tweak sleep duration * fix: exit 1 * fix: wait for misskey processes to become healthy * fix: longer sleep for user deletion * fix: make sleep longer again * デッドロック解消の試み https://github.com/misskey-dev/misskey/issues/15005 * Revert "デッドロック解消の試み" This reverts commit 266141f66fb584371bbb56ef7eba04e14bcff94d. * wip * Update SystemAccountService.ts --------- Co-authored-by: おさむのひと <46447427+samunohito@users.noreply.github.com> Co-authored-by: zyoshoka <107108195+zyoshoka@users.noreply.github.com> --- packages/backend/src/server/api/ApiCallService.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'packages/backend/src/server/api/ApiCallService.ts') diff --git a/packages/backend/src/server/api/ApiCallService.ts b/packages/backend/src/server/api/ApiCallService.ts index aad833f126..9399aa61b0 100644 --- a/packages/backend/src/server/api/ApiCallService.ts +++ b/packages/backend/src/server/api/ApiCallService.ts @@ -371,7 +371,7 @@ export class ApiCallService implements OnApplicationShutdown { } } - if ((ep.meta.requireModerator || ep.meta.requireAdmin) && !user!.isRoot) { + if ((ep.meta.requireModerator || ep.meta.requireAdmin) && (this.meta.rootUserId !== user!.id)) { const myRoles = await this.roleService.getUserRoles(user!.id); if (ep.meta.requireModerator && !myRoles.some(r => r.isModerator || r.isAdministrator)) { throw new ApiError({ @@ -391,7 +391,7 @@ export class ApiCallService implements OnApplicationShutdown { } } - if (ep.meta.requireRolePolicy != null && !user!.isRoot) { + if (ep.meta.requireRolePolicy != null && (this.meta.rootUserId !== user!.id)) { const myRoles = await this.roleService.getUserRoles(user!.id); const policies = await this.roleService.getUserPolicies(user!.id); if (!policies[ep.meta.requireRolePolicy] && !myRoles.some(r => r.isAdministrator)) { -- cgit v1.2.3-freya From 08b131ec33413e13c9cf8e043116b601c72b713f Mon Sep 17 00:00:00 2001 From: syuilo <4439005+syuilo@users.noreply.github.com> Date: Sat, 22 Mar 2025 08:16:15 +0900 Subject: refactor(backend): better prop name --- packages/backend/src/server/api/ApiCallService.ts | 4 ++-- packages/backend/src/server/api/endpoints.ts | 2 +- .../src/server/api/endpoints/admin/avatar-decorations/create.ts | 2 +- .../src/server/api/endpoints/admin/avatar-decorations/delete.ts | 2 +- .../backend/src/server/api/endpoints/admin/avatar-decorations/list.ts | 2 +- .../src/server/api/endpoints/admin/avatar-decorations/update.ts | 2 +- .../backend/src/server/api/endpoints/admin/emoji/add-aliases-bulk.ts | 2 +- packages/backend/src/server/api/endpoints/admin/emoji/add.ts | 2 +- packages/backend/src/server/api/endpoints/admin/emoji/copy.ts | 2 +- packages/backend/src/server/api/endpoints/admin/emoji/delete-bulk.ts | 2 +- packages/backend/src/server/api/endpoints/admin/emoji/delete.ts | 2 +- packages/backend/src/server/api/endpoints/admin/emoji/import-zip.ts | 2 +- packages/backend/src/server/api/endpoints/admin/emoji/list-remote.ts | 2 +- packages/backend/src/server/api/endpoints/admin/emoji/list.ts | 2 +- .../src/server/api/endpoints/admin/emoji/remove-aliases-bulk.ts | 2 +- .../backend/src/server/api/endpoints/admin/emoji/set-aliases-bulk.ts | 2 +- .../backend/src/server/api/endpoints/admin/emoji/set-category-bulk.ts | 2 +- .../backend/src/server/api/endpoints/admin/emoji/set-license-bulk.ts | 2 +- packages/backend/src/server/api/endpoints/admin/emoji/update.ts | 2 +- packages/backend/src/server/api/endpoints/i/import-antennas.ts | 2 +- packages/backend/src/server/api/endpoints/i/import-blocking.ts | 2 +- packages/backend/src/server/api/endpoints/i/import-following.ts | 2 +- packages/backend/src/server/api/endpoints/i/import-muting.ts | 2 +- packages/backend/src/server/api/endpoints/i/import-user-lists.ts | 2 +- packages/backend/src/server/api/endpoints/invite/create.ts | 2 +- packages/backend/src/server/api/endpoints/invite/delete.ts | 2 +- packages/backend/src/server/api/endpoints/invite/limit.ts | 2 +- packages/backend/src/server/api/endpoints/invite/list.ts | 2 +- packages/backend/src/server/api/endpoints/v2/admin/emoji/list.ts | 2 +- 29 files changed, 30 insertions(+), 30 deletions(-) (limited to 'packages/backend/src/server/api/ApiCallService.ts') diff --git a/packages/backend/src/server/api/ApiCallService.ts b/packages/backend/src/server/api/ApiCallService.ts index 9399aa61b0..a42fdaf730 100644 --- a/packages/backend/src/server/api/ApiCallService.ts +++ b/packages/backend/src/server/api/ApiCallService.ts @@ -391,10 +391,10 @@ export class ApiCallService implements OnApplicationShutdown { } } - if (ep.meta.requireRolePolicy != null && (this.meta.rootUserId !== user!.id)) { + if (ep.meta.requiredRolePolicy != null && (this.meta.rootUserId !== user!.id)) { const myRoles = await this.roleService.getUserRoles(user!.id); const policies = await this.roleService.getUserPolicies(user!.id); - if (!policies[ep.meta.requireRolePolicy] && !myRoles.some(r => r.isAdministrator)) { + if (!policies[ep.meta.requiredRolePolicy] && !myRoles.some(r => r.isAdministrator)) { throw new ApiError({ message: 'You are not assigned to a required role.', code: 'ROLE_PERMISSION_DENIED', diff --git a/packages/backend/src/server/api/endpoints.ts b/packages/backend/src/server/api/endpoints.ts index 7f4ca9c0e0..03c729ed18 100644 --- a/packages/backend/src/server/api/endpoints.ts +++ b/packages/backend/src/server/api/endpoints.ts @@ -39,7 +39,7 @@ interface IEndpointMetaBase { */ readonly requireAdmin?: boolean; - readonly requireRolePolicy?: KeyOf<'RolePolicies'>; + readonly requiredRolePolicy?: KeyOf<'RolePolicies'>; /** * 引っ越し済みのユーザーによるリクエストを禁止するか diff --git a/packages/backend/src/server/api/endpoints/admin/avatar-decorations/create.ts b/packages/backend/src/server/api/endpoints/admin/avatar-decorations/create.ts index 87d80cbe80..0121c302ac 100644 --- a/packages/backend/src/server/api/endpoints/admin/avatar-decorations/create.ts +++ b/packages/backend/src/server/api/endpoints/admin/avatar-decorations/create.ts @@ -12,7 +12,7 @@ export const meta = { tags: ['admin'], requireCredential: true, - requireRolePolicy: 'canManageAvatarDecorations', + requiredRolePolicy: 'canManageAvatarDecorations', kind: 'write:admin:avatar-decorations', res: { diff --git a/packages/backend/src/server/api/endpoints/admin/avatar-decorations/delete.ts b/packages/backend/src/server/api/endpoints/admin/avatar-decorations/delete.ts index 3a5673d99d..13660d0b8c 100644 --- a/packages/backend/src/server/api/endpoints/admin/avatar-decorations/delete.ts +++ b/packages/backend/src/server/api/endpoints/admin/avatar-decorations/delete.ts @@ -13,7 +13,7 @@ export const meta = { tags: ['admin'], requireCredential: true, - requireRolePolicy: 'canManageAvatarDecorations', + requiredRolePolicy: 'canManageAvatarDecorations', kind: 'write:admin:avatar-decorations', errors: { }, diff --git a/packages/backend/src/server/api/endpoints/admin/avatar-decorations/list.ts b/packages/backend/src/server/api/endpoints/admin/avatar-decorations/list.ts index d785f085ac..d4d9a7235b 100644 --- a/packages/backend/src/server/api/endpoints/admin/avatar-decorations/list.ts +++ b/packages/backend/src/server/api/endpoints/admin/avatar-decorations/list.ts @@ -13,7 +13,7 @@ export const meta = { tags: ['admin'], requireCredential: true, - requireRolePolicy: 'canManageAvatarDecorations', + requiredRolePolicy: 'canManageAvatarDecorations', kind: 'read:admin:avatar-decorations', res: { diff --git a/packages/backend/src/server/api/endpoints/admin/avatar-decorations/update.ts b/packages/backend/src/server/api/endpoints/admin/avatar-decorations/update.ts index 34b3b5a11f..22476a6888 100644 --- a/packages/backend/src/server/api/endpoints/admin/avatar-decorations/update.ts +++ b/packages/backend/src/server/api/endpoints/admin/avatar-decorations/update.ts @@ -13,7 +13,7 @@ export const meta = { tags: ['admin'], requireCredential: true, - requireRolePolicy: 'canManageAvatarDecorations', + requiredRolePolicy: 'canManageAvatarDecorations', kind: 'write:admin:avatar-decorations', errors: { diff --git a/packages/backend/src/server/api/endpoints/admin/emoji/add-aliases-bulk.ts b/packages/backend/src/server/api/endpoints/admin/emoji/add-aliases-bulk.ts index a30a080e59..1459351d37 100644 --- a/packages/backend/src/server/api/endpoints/admin/emoji/add-aliases-bulk.ts +++ b/packages/backend/src/server/api/endpoints/admin/emoji/add-aliases-bulk.ts @@ -11,7 +11,7 @@ export const meta = { tags: ['admin'], requireCredential: true, - requireRolePolicy: 'canManageCustomEmojis', + requiredRolePolicy: 'canManageCustomEmojis', kind: 'write:admin:emoji', } as const; diff --git a/packages/backend/src/server/api/endpoints/admin/emoji/add.ts b/packages/backend/src/server/api/endpoints/admin/emoji/add.ts index 53256565f6..3852146177 100644 --- a/packages/backend/src/server/api/endpoints/admin/emoji/add.ts +++ b/packages/backend/src/server/api/endpoints/admin/emoji/add.ts @@ -16,7 +16,7 @@ export const meta = { tags: ['admin'], requireCredential: true, - requireRolePolicy: 'canManageCustomEmojis', + requiredRolePolicy: 'canManageCustomEmojis', kind: 'write:admin:emoji', errors: { diff --git a/packages/backend/src/server/api/endpoints/admin/emoji/copy.ts b/packages/backend/src/server/api/endpoints/admin/emoji/copy.ts index 87b58ff6f6..cf03859ce5 100644 --- a/packages/backend/src/server/api/endpoints/admin/emoji/copy.ts +++ b/packages/backend/src/server/api/endpoints/admin/emoji/copy.ts @@ -17,7 +17,7 @@ export const meta = { tags: ['admin'], requireCredential: true, - requireRolePolicy: 'canManageCustomEmojis', + requiredRolePolicy: 'canManageCustomEmojis', kind: 'write:admin:emoji', errors: { diff --git a/packages/backend/src/server/api/endpoints/admin/emoji/delete-bulk.ts b/packages/backend/src/server/api/endpoints/admin/emoji/delete-bulk.ts index cec9f700c3..7993edcc07 100644 --- a/packages/backend/src/server/api/endpoints/admin/emoji/delete-bulk.ts +++ b/packages/backend/src/server/api/endpoints/admin/emoji/delete-bulk.ts @@ -11,7 +11,7 @@ export const meta = { tags: ['admin'], requireCredential: true, - requireRolePolicy: 'canManageCustomEmojis', + requiredRolePolicy: 'canManageCustomEmojis', kind: 'write:admin:emoji', } as const; diff --git a/packages/backend/src/server/api/endpoints/admin/emoji/delete.ts b/packages/backend/src/server/api/endpoints/admin/emoji/delete.ts index 50c45b6ac5..87ed3f5f18 100644 --- a/packages/backend/src/server/api/endpoints/admin/emoji/delete.ts +++ b/packages/backend/src/server/api/endpoints/admin/emoji/delete.ts @@ -11,7 +11,7 @@ export const meta = { tags: ['admin'], requireCredential: true, - requireRolePolicy: 'canManageCustomEmojis', + requiredRolePolicy: 'canManageCustomEmojis', kind: 'write:admin:emoji', errors: { diff --git a/packages/backend/src/server/api/endpoints/admin/emoji/import-zip.ts b/packages/backend/src/server/api/endpoints/admin/emoji/import-zip.ts index 8e5f69c894..7ca931eb21 100644 --- a/packages/backend/src/server/api/endpoints/admin/emoji/import-zip.ts +++ b/packages/backend/src/server/api/endpoints/admin/emoji/import-zip.ts @@ -10,7 +10,7 @@ import { QueueService } from '@/core/QueueService.js'; export const meta = { secure: true, requireCredential: true, - requireRolePolicy: 'canManageCustomEmojis', + requiredRolePolicy: 'canManageCustomEmojis', } as const; export const paramDef = { diff --git a/packages/backend/src/server/api/endpoints/admin/emoji/list-remote.ts b/packages/backend/src/server/api/endpoints/admin/emoji/list-remote.ts index 0889ceb76f..b44007962d 100644 --- a/packages/backend/src/server/api/endpoints/admin/emoji/list-remote.ts +++ b/packages/backend/src/server/api/endpoints/admin/emoji/list-remote.ts @@ -16,7 +16,7 @@ export const meta = { tags: ['admin'], requireCredential: true, - requireRolePolicy: 'canManageCustomEmojis', + requiredRolePolicy: 'canManageCustomEmojis', kind: 'read:admin:emoji', res: { diff --git a/packages/backend/src/server/api/endpoints/admin/emoji/list.ts b/packages/backend/src/server/api/endpoints/admin/emoji/list.ts index ffb5dbf4b5..4342e178cc 100644 --- a/packages/backend/src/server/api/endpoints/admin/emoji/list.ts +++ b/packages/backend/src/server/api/endpoints/admin/emoji/list.ts @@ -16,7 +16,7 @@ export const meta = { tags: ['admin'], requireCredential: true, - requireRolePolicy: 'canManageCustomEmojis', + requiredRolePolicy: 'canManageCustomEmojis', kind: 'read:admin:emoji', res: { diff --git a/packages/backend/src/server/api/endpoints/admin/emoji/remove-aliases-bulk.ts b/packages/backend/src/server/api/endpoints/admin/emoji/remove-aliases-bulk.ts index 0fa119eabe..161c3b9f37 100644 --- a/packages/backend/src/server/api/endpoints/admin/emoji/remove-aliases-bulk.ts +++ b/packages/backend/src/server/api/endpoints/admin/emoji/remove-aliases-bulk.ts @@ -11,7 +11,7 @@ export const meta = { tags: ['admin'], requireCredential: true, - requireRolePolicy: 'canManageCustomEmojis', + requiredRolePolicy: 'canManageCustomEmojis', kind: 'write:admin:emoji', } as const; diff --git a/packages/backend/src/server/api/endpoints/admin/emoji/set-aliases-bulk.ts b/packages/backend/src/server/api/endpoints/admin/emoji/set-aliases-bulk.ts index d9ee18699c..2e700809d8 100644 --- a/packages/backend/src/server/api/endpoints/admin/emoji/set-aliases-bulk.ts +++ b/packages/backend/src/server/api/endpoints/admin/emoji/set-aliases-bulk.ts @@ -11,7 +11,7 @@ export const meta = { tags: ['admin'], requireCredential: true, - requireRolePolicy: 'canManageCustomEmojis', + requiredRolePolicy: 'canManageCustomEmojis', kind: 'write:admin:emoji', } as const; diff --git a/packages/backend/src/server/api/endpoints/admin/emoji/set-category-bulk.ts b/packages/backend/src/server/api/endpoints/admin/emoji/set-category-bulk.ts index dc25df2767..ee87858b0e 100644 --- a/packages/backend/src/server/api/endpoints/admin/emoji/set-category-bulk.ts +++ b/packages/backend/src/server/api/endpoints/admin/emoji/set-category-bulk.ts @@ -11,7 +11,7 @@ export const meta = { tags: ['admin'], requireCredential: true, - requireRolePolicy: 'canManageCustomEmojis', + requiredRolePolicy: 'canManageCustomEmojis', kind: 'write:admin:emoji', } as const; diff --git a/packages/backend/src/server/api/endpoints/admin/emoji/set-license-bulk.ts b/packages/backend/src/server/api/endpoints/admin/emoji/set-license-bulk.ts index 4ba99faab7..7ab5916951 100644 --- a/packages/backend/src/server/api/endpoints/admin/emoji/set-license-bulk.ts +++ b/packages/backend/src/server/api/endpoints/admin/emoji/set-license-bulk.ts @@ -11,7 +11,7 @@ export const meta = { tags: ['admin'], requireCredential: true, - requireRolePolicy: 'canManageCustomEmojis', + requiredRolePolicy: 'canManageCustomEmojis', kind: 'write:admin:emoji', } as const; diff --git a/packages/backend/src/server/api/endpoints/admin/emoji/update.ts b/packages/backend/src/server/api/endpoints/admin/emoji/update.ts index e3aaa051c1..6834a6d213 100644 --- a/packages/backend/src/server/api/endpoints/admin/emoji/update.ts +++ b/packages/backend/src/server/api/endpoints/admin/emoji/update.ts @@ -14,7 +14,7 @@ export const meta = { tags: ['admin'], requireCredential: true, - requireRolePolicy: 'canManageCustomEmojis', + requiredRolePolicy: 'canManageCustomEmojis', kind: 'write:admin:emoji', errors: { diff --git a/packages/backend/src/server/api/endpoints/i/import-antennas.ts b/packages/backend/src/server/api/endpoints/i/import-antennas.ts index bdf6c065e8..ccec96ffbb 100644 --- a/packages/backend/src/server/api/endpoints/i/import-antennas.ts +++ b/packages/backend/src/server/api/endpoints/i/import-antennas.ts @@ -16,7 +16,7 @@ import { ApiError } from '../../error.js'; export const meta = { secure: true, requireCredential: true, - requireRolePolicy: 'canImportAntennas', + requiredRolePolicy: 'canImportAntennas', prohibitMoved: true, limit: { diff --git a/packages/backend/src/server/api/endpoints/i/import-blocking.ts b/packages/backend/src/server/api/endpoints/i/import-blocking.ts index d7bb6bcd22..2fa450558b 100644 --- a/packages/backend/src/server/api/endpoints/i/import-blocking.ts +++ b/packages/backend/src/server/api/endpoints/i/import-blocking.ts @@ -15,7 +15,7 @@ import { ApiError } from '../../error.js'; export const meta = { secure: true, requireCredential: true, - requireRolePolicy: 'canImportBlocking', + requiredRolePolicy: 'canImportBlocking', prohibitMoved: true, limit: { diff --git a/packages/backend/src/server/api/endpoints/i/import-following.ts b/packages/backend/src/server/api/endpoints/i/import-following.ts index e03192d8c6..9186fca162 100644 --- a/packages/backend/src/server/api/endpoints/i/import-following.ts +++ b/packages/backend/src/server/api/endpoints/i/import-following.ts @@ -15,7 +15,7 @@ import { ApiError } from '../../error.js'; export const meta = { secure: true, requireCredential: true, - requireRolePolicy: 'canImportFollowing', + requiredRolePolicy: 'canImportFollowing', prohibitMoved: true, limit: { duration: ms('1hour'), diff --git a/packages/backend/src/server/api/endpoints/i/import-muting.ts b/packages/backend/src/server/api/endpoints/i/import-muting.ts index 76b285bb7e..b6dbacd371 100644 --- a/packages/backend/src/server/api/endpoints/i/import-muting.ts +++ b/packages/backend/src/server/api/endpoints/i/import-muting.ts @@ -15,7 +15,7 @@ import { ApiError } from '../../error.js'; export const meta = { secure: true, requireCredential: true, - requireRolePolicy: 'canImportMuting', + requiredRolePolicy: 'canImportMuting', prohibitMoved: true, limit: { diff --git a/packages/backend/src/server/api/endpoints/i/import-user-lists.ts b/packages/backend/src/server/api/endpoints/i/import-user-lists.ts index 76ecfd082c..5de0a70bbb 100644 --- a/packages/backend/src/server/api/endpoints/i/import-user-lists.ts +++ b/packages/backend/src/server/api/endpoints/i/import-user-lists.ts @@ -15,7 +15,7 @@ import { ApiError } from '../../error.js'; export const meta = { secure: true, requireCredential: true, - requireRolePolicy: 'canImportUserLists', + requiredRolePolicy: 'canImportUserLists', prohibitMoved: true, limit: { duration: ms('1hour'), diff --git a/packages/backend/src/server/api/endpoints/invite/create.ts b/packages/backend/src/server/api/endpoints/invite/create.ts index a70b587da7..f2e683ddf2 100644 --- a/packages/backend/src/server/api/endpoints/invite/create.ts +++ b/packages/backend/src/server/api/endpoints/invite/create.ts @@ -18,7 +18,7 @@ export const meta = { tags: ['meta'], requireCredential: true, - requireRolePolicy: 'canInvite', + requiredRolePolicy: 'canInvite', kind: 'write:invite-codes', errors: { diff --git a/packages/backend/src/server/api/endpoints/invite/delete.ts b/packages/backend/src/server/api/endpoints/invite/delete.ts index e960ff9f4e..06f47e90bc 100644 --- a/packages/backend/src/server/api/endpoints/invite/delete.ts +++ b/packages/backend/src/server/api/endpoints/invite/delete.ts @@ -14,7 +14,7 @@ export const meta = { tags: ['meta'], requireCredential: true, - requireRolePolicy: 'canInvite', + requiredRolePolicy: 'canInvite', kind: 'write:invite-codes', errors: { diff --git a/packages/backend/src/server/api/endpoints/invite/limit.ts b/packages/backend/src/server/api/endpoints/invite/limit.ts index 2ffd41ae28..0067dce231 100644 --- a/packages/backend/src/server/api/endpoints/invite/limit.ts +++ b/packages/backend/src/server/api/endpoints/invite/limit.ts @@ -15,7 +15,7 @@ export const meta = { tags: ['meta'], requireCredential: true, - requireRolePolicy: 'canInvite', + requiredRolePolicy: 'canInvite', kind: 'read:invite-codes', res: { diff --git a/packages/backend/src/server/api/endpoints/invite/list.ts b/packages/backend/src/server/api/endpoints/invite/list.ts index 23aefe83a2..a99974a91e 100644 --- a/packages/backend/src/server/api/endpoints/invite/list.ts +++ b/packages/backend/src/server/api/endpoints/invite/list.ts @@ -14,7 +14,7 @@ export const meta = { tags: ['meta'], requireCredential: true, - requireRolePolicy: 'canInvite', + requiredRolePolicy: 'canInvite', kind: 'read:invite-codes', res: { diff --git a/packages/backend/src/server/api/endpoints/v2/admin/emoji/list.ts b/packages/backend/src/server/api/endpoints/v2/admin/emoji/list.ts index 9426318e34..7139715293 100644 --- a/packages/backend/src/server/api/endpoints/v2/admin/emoji/list.ts +++ b/packages/backend/src/server/api/endpoints/v2/admin/emoji/list.ts @@ -12,7 +12,7 @@ export const meta = { tags: ['admin'], requireCredential: true, - requireRolePolicy: 'canManageCustomEmojis', + requiredRolePolicy: 'canManageCustomEmojis', kind: 'read:admin:emoji', res: { -- cgit v1.2.3-freya From 7e8cc4d7c0a86ad0bf71a727fb16132e8bc180a8 Mon Sep 17 00:00:00 2001 From: おさむのひと <46447427+samunohito@users.noreply.github.com> Date: Tue, 29 Apr 2025 08:15:09 +0900 Subject: fix: 添付ファイルのあるリクエストを受けたときの初動を改善 (#15896) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * wip * ロールポリシーの値も参照するように * エンドポイントのテストを追加 * fix review * add spdx * fix CHANGELOG.md * fix test * regenerate * add log * Revert "add log" This reverts commit 4b2bf59a609b85ca0bfcc9b71438db782f11983d. * add log * fix * Revert "add log" This reverts commit c5a73d57da0f30ec5215e08a8b4d78785cce48d1. --- CHANGELOG.md | 1 + packages/backend/package.json | 4 +- packages/backend/src/server/ServerService.ts | 7 +- packages/backend/src/server/api/ApiCallService.ts | 166 ++++++++++++++------- packages/backend/src/server/api/endpoint-base.ts | 10 +- packages/backend/test/e2e/api.ts | 4 +- .../test/unit/server/api/drive/files/create.ts | 108 ++++++++++++++ pnpm-lock.yaml | 158 ++++++++++++++++---- 8 files changed, 363 insertions(+), 95 deletions(-) create mode 100644 packages/backend/test/unit/server/api/drive/files/create.ts (limited to 'packages/backend/src/server/api/ApiCallService.ts') diff --git a/CHANGELOG.md b/CHANGELOG.md index 8151ed3184..f52bf43171 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -33,6 +33,7 @@ - Fix: システムアカウントの名前がサーバー名と同期されない問題を修正 - Fix: 大文字を含むユーザの URL で紹介された場合に 404 エラーを返す問題 #15813 - Fix: リードレプリカ設定時にレコードの追加・更新・削除を伴うクエリを発行した際はmasterノードで実行されるように調整( #10897 ) +- Fix: ファイルアップロード時の挙動を一部調整(#15895) ## 2025.4.0 diff --git a/packages/backend/package.json b/packages/backend/package.json index faf331ed85..c4de44df18 100644 --- a/packages/backend/package.json +++ b/packages/backend/package.json @@ -222,6 +222,7 @@ "@types/semver": "7.7.0", "@types/simple-oauth2": "5.0.7", "@types/sinonjs__fake-timers": "8.1.5", + "@types/supertest": "6.0.3", "@types/tinycolor2": "1.4.6", "@types/tmp": "0.2.6", "@types/vary": "1.1.3", @@ -238,6 +239,7 @@ "jest-mock": "29.7.0", "nodemon": "3.1.10", "pid-port": "1.0.2", - "simple-oauth2": "5.1.0" + "simple-oauth2": "5.1.0", + "supertest": "7.1.0" } } diff --git a/packages/backend/src/server/ServerService.ts b/packages/backend/src/server/ServerService.ts index 355d7ca08e..7decdd2c10 100644 --- a/packages/backend/src/server/ServerService.ts +++ b/packages/backend/src/server/ServerService.ts @@ -73,7 +73,7 @@ export class ServerService implements OnApplicationShutdown { } @bindThis - public async launch(): Promise { + public async launch() { const fastify = Fastify({ trustProxy: true, logger: false, @@ -133,8 +133,8 @@ export class ServerService implements OnApplicationShutdown { reply.header('content-type', 'text/plain; charset=utf-8'); reply.header('link', `<${encodeURI(location)}>; rel="canonical"`); done(null, [ - "Refusing to relay remote ActivityPub object lookup.", - "", + 'Refusing to relay remote ActivityPub object lookup.', + '', `Please remove 'application/activity+json' and 'application/ld+json' from the Accept header or fetch using the authoritative URL at ${location}.`, ].join('\n')); }); @@ -301,6 +301,7 @@ export class ServerService implements OnApplicationShutdown { } await fastify.ready(); + return fastify; } @bindThis diff --git a/packages/backend/src/server/api/ApiCallService.ts b/packages/backend/src/server/api/ApiCallService.ts index a42fdaf730..960c7b5476 100644 --- a/packages/backend/src/server/api/ApiCallService.ts +++ b/packages/backend/src/server/api/ApiCallService.ts @@ -6,8 +6,11 @@ import { randomUUID } from 'node:crypto'; import * as fs from 'node:fs'; import * as stream from 'node:stream/promises'; +import { Transform } from 'node:stream'; +import { type MultipartFile } from '@fastify/multipart'; import { Inject, Injectable } from '@nestjs/common'; import * as Sentry from '@sentry/node'; +import { AttachmentFile } from '@/server/api/endpoint-base.js'; import { DI } from '@/di-symbols.js'; import { getIpHash } from '@/misc/get-ip-hash.js'; import type { MiLocalUser, MiUser } from '@/models/User.js'; @@ -16,7 +19,7 @@ import type Logger from '@/logger.js'; import type { MiMeta, UserIpsRepository } from '@/models/_.js'; import { createTemp } from '@/misc/create-temp.js'; import { bindThis } from '@/decorators.js'; -import { RoleService } from '@/core/RoleService.js'; +import { type RolePolicies, RoleService } from '@/core/RoleService.js'; import type { Config } from '@/config.js'; import { ApiError } from './error.js'; import { RateLimiterService } from './RateLimiterService.js'; @@ -200,18 +203,6 @@ export class ApiCallService implements OnApplicationShutdown { return; } - const [path, cleanup] = await createTemp(); - await stream.pipeline(multipartData.file, fs.createWriteStream(path)); - - // ファイルサイズが制限を超えていた場合 - // なお truncated はストリームを読み切ってからでないと機能しないため、stream.pipeline より後にある必要がある - if (multipartData.file.truncated) { - cleanup(); - reply.code(413); - reply.send(); - return; - } - const fields = {} as Record; for (const [k, v] of Object.entries(multipartData.fields)) { fields[k] = typeof v === 'object' && 'value' in v ? v.value : undefined; @@ -226,10 +217,7 @@ export class ApiCallService implements OnApplicationShutdown { return; } this.authenticateService.authenticate(token).then(([user, app]) => { - this.call(endpoint, user, app, fields, { - name: multipartData.filename, - path: path, - }, request).then((res) => { + this.call(endpoint, user, app, fields, multipartData, request).then((res) => { this.send(reply, res); }).catch((err: ApiError) => { this.#sendApiError(reply, err); @@ -294,10 +282,7 @@ export class ApiCallService implements OnApplicationShutdown { user: MiLocalUser | null | undefined, token: MiAccessToken | null | undefined, data: any, - file: { - name: string; - path: string; - } | null, + multipartFile: MultipartFile | null, request: FastifyRequest<{ Body: Record | undefined, Querystring: Record }>, ) { const isSecure = user != null && token == null; @@ -371,6 +356,37 @@ export class ApiCallService implements OnApplicationShutdown { } } + // Cast non JSON input + if ((ep.meta.requireFile || request.method === 'GET') && ep.params.properties) { + for (const k of Object.keys(ep.params.properties)) { + const param = ep.params.properties![k]; + if (['boolean', 'number', 'integer'].includes(param.type ?? '') && typeof data[k] === 'string') { + try { + data[k] = JSON.parse(data[k]); + } catch (e) { + throw new ApiError({ + message: 'Invalid param.', + code: 'INVALID_PARAM', + id: '0b5f1631-7c1a-41a6-b399-cce335f34d85', + }, { + param: k, + reason: `cannot cast to ${param.type}`, + }); + } + } + } + } + + if (token && ((ep.meta.kind && !token.permission.some(p => p === ep.meta.kind)) + || (!ep.meta.kind && (ep.meta.requireCredential || ep.meta.requireModerator || ep.meta.requireAdmin)))) { + throw new ApiError({ + message: 'Your app does not have the necessary permissions to use this endpoint.', + code: 'PERMISSION_DENIED', + kind: 'permission', + id: '1370e5b7-d4eb-4566-bb1d-7748ee6a1838', + }); + } + if ((ep.meta.requireModerator || ep.meta.requireAdmin) && (this.meta.rootUserId !== user!.id)) { const myRoles = await this.roleService.getUserRoles(user!.id); if (ep.meta.requireModerator && !myRoles.some(r => r.isModerator || r.isAdministrator)) { @@ -404,47 +420,89 @@ export class ApiCallService implements OnApplicationShutdown { } } - if (token && ((ep.meta.kind && !token.permission.some(p => p === ep.meta.kind)) - || (!ep.meta.kind && (ep.meta.requireCredential || ep.meta.requireModerator || ep.meta.requireAdmin)))) { - throw new ApiError({ - message: 'Your app does not have the necessary permissions to use this endpoint.', - code: 'PERMISSION_DENIED', - kind: 'permission', - id: '1370e5b7-d4eb-4566-bb1d-7748ee6a1838', - }); - } - - // Cast non JSON input - if ((ep.meta.requireFile || request.method === 'GET') && ep.params.properties) { - for (const k of Object.keys(ep.params.properties)) { - const param = ep.params.properties![k]; - if (['boolean', 'number', 'integer'].includes(param.type ?? '') && typeof data[k] === 'string') { - try { - data[k] = JSON.parse(data[k]); - } catch (e) { - throw new ApiError({ - message: 'Invalid param.', - code: 'INVALID_PARAM', - id: '0b5f1631-7c1a-41a6-b399-cce335f34d85', - }, { - param: k, - reason: `cannot cast to ${param.type}`, - }); - } - } - } + let attachmentFile: AttachmentFile | null = null; + let cleanup = () => {}; + if (ep.meta.requireFile && request.method === 'POST' && multipartFile) { + const policies = await this.roleService.getUserPolicies(user!.id); + const result = await this.handleAttachmentFile( + Math.min((policies.maxFileSizeMb * 1024 * 1024), this.config.maxFileSize), + multipartFile, + ); + attachmentFile = result.attachmentFile; + cleanup = result.cleanup; } // API invoking if (this.config.sentryForBackend) { return await Sentry.startSpan({ name: 'API: ' + ep.name, - }, () => ep.exec(data, user, token, file, request.ip, request.headers) - .catch((err: Error) => this.#onExecError(ep, data, err, user?.id))); + }, () => { + return ep.exec(data, user, token, attachmentFile, request.ip, request.headers) + .catch((err: Error) => this.#onExecError(ep, data, err, user?.id)) + .finally(() => cleanup()); + }); } else { - return await ep.exec(data, user, token, file, request.ip, request.headers) - .catch((err: Error) => this.#onExecError(ep, data, err, user?.id)); + return await ep.exec(data, user, token, attachmentFile, request.ip, request.headers) + .catch((err: Error) => this.#onExecError(ep, data, err, user?.id)) + .finally(() => cleanup()); + } + } + + @bindThis + private async handleAttachmentFile( + fileSizeLimit: number, + multipartFile: MultipartFile, + ) { + function createTooLongError() { + return new ApiError({ + httpStatusCode: 413, + kind: 'client', + message: 'File size is too large.', + code: 'FILE_SIZE_TOO_LARGE', + id: 'ff827ce8-9b4b-4808-8511-422222a3362f', + }); + } + + function createLimitStream(limit: number) { + let total = 0; + + return new Transform({ + transform(chunk, _, callback) { + total += chunk.length; + if (total > limit) { + callback(createTooLongError()); + } else { + callback(null, chunk); + } + }, + }); } + + const [path, cleanup] = await createTemp(); + try { + await stream.pipeline( + multipartFile.file, + createLimitStream(fileSizeLimit), + fs.createWriteStream(path), + ); + + // ファイルサイズが制限を超えていた場合 + // なお truncated はストリームを読み切ってからでないと機能しないため、stream.pipeline より後にある必要がある + if (multipartFile.file.truncated) { + throw createTooLongError(); + } + } catch (err) { + cleanup(); + throw err; + } + + return { + attachmentFile: { + name: multipartFile.filename, + path, + }, + cleanup, + }; } @bindThis diff --git a/packages/backend/src/server/api/endpoint-base.ts b/packages/backend/src/server/api/endpoint-base.ts index e061aa3a8e..b063487305 100644 --- a/packages/backend/src/server/api/endpoint-base.ts +++ b/packages/backend/src/server/api/endpoint-base.ts @@ -21,23 +21,23 @@ ajv.addFormat('misskey:id', /^[a-zA-Z0-9]+$/); export type Response = Record | void; -type File = { +export type AttachmentFile = { name: string | null; path: string; }; // TODO: paramsの型をT['params']のスキーマ定義から推論する type Executor = - (params: SchemaType, user: T['requireCredential'] extends true ? MiLocalUser : MiLocalUser | null, token: MiAccessToken | null, file?: File, cleanup?: () => any, ip?: string | null, headers?: Record | null) => - Promise>>; + (params: SchemaType, user: T['requireCredential'] extends true ? MiLocalUser : MiLocalUser | null, token: MiAccessToken | null, file?: AttachmentFile, cleanup?: () => any, ip?: string | null, headers?: Record | null) => + Promise>>; export abstract class Endpoint { - public exec: (params: any, user: T['requireCredential'] extends true ? MiLocalUser : MiLocalUser | null, token: MiAccessToken | null, file?: File, ip?: string | null, headers?: Record | null) => Promise; + public exec: (params: any, user: T['requireCredential'] extends true ? MiLocalUser : MiLocalUser | null, token: MiAccessToken | null, file?: AttachmentFile, ip?: string | null, headers?: Record | null) => Promise; constructor(meta: T, paramDef: Ps, cb: Executor) { const validate = ajv.compile(paramDef); - this.exec = (params: any, user: T['requireCredential'] extends true ? MiLocalUser : MiLocalUser | null, token: MiAccessToken | null, file?: File, ip?: string | null, headers?: Record | null) => { + this.exec = (params: any, user: T['requireCredential'] extends true ? MiLocalUser : MiLocalUser | null, token: MiAccessToken | null, file?: AttachmentFile, ip?: string | null, headers?: Record | null) => { let cleanup: undefined | (() => void) = undefined; if (meta.requireFile) { diff --git a/packages/backend/test/e2e/api.ts b/packages/backend/test/e2e/api.ts index 49c6a0636b..f9e65aaa84 100644 --- a/packages/backend/test/e2e/api.ts +++ b/packages/backend/test/e2e/api.ts @@ -159,8 +159,8 @@ describe('API', () => { user: { token: application3 }, }, { status: 403, - code: 'ROLE_PERMISSION_DENIED', - id: 'c3d38592-54c0-429d-be96-5636b0431a61', + code: 'PERMISSION_DENIED', + id: '1370e5b7-d4eb-4566-bb1d-7748ee6a1838', }); await failedApiCall({ diff --git a/packages/backend/test/unit/server/api/drive/files/create.ts b/packages/backend/test/unit/server/api/drive/files/create.ts new file mode 100644 index 0000000000..b98892fa03 --- /dev/null +++ b/packages/backend/test/unit/server/api/drive/files/create.ts @@ -0,0 +1,108 @@ +/* + * SPDX-FileCopyrightText: syuilo and misskey-project + * SPDX-License-Identifier: AGPL-3.0-only + */ + +import { S3Client } from '@aws-sdk/client-s3'; +import { Test, TestingModule } from '@nestjs/testing'; +import { mockClient } from 'aws-sdk-client-mock'; +import { FastifyInstance } from 'fastify'; +import request from 'supertest'; +import { CoreModule } from '@/core/CoreModule.js'; +import { RoleService } from '@/core/RoleService.js'; +import { DI } from '@/di-symbols.js'; +import { GlobalModule } from '@/GlobalModule.js'; +import { MiRole, UserProfilesRepository, UsersRepository } from '@/models/_.js'; +import { MiUser } from '@/models/User.js'; +import { ServerModule } from '@/server/ServerModule.js'; +import { ServerService } from '@/server/ServerService.js'; + +describe('/drive/files/create', () => { + let module: TestingModule; + let server: FastifyInstance; + const s3Mock = mockClient(S3Client); + let roleService: RoleService; + + let root: MiUser; + let role_tinyAttachment: MiRole; + + beforeAll(async () => { + module = await Test.createTestingModule({ + imports: [GlobalModule, CoreModule, ServerModule], + }).compile(); + module.enableShutdownHooks(); + + const serverService = module.get(ServerService); + server = await serverService.launch(); + + const usersRepository = module.get(DI.usersRepository); + root = await usersRepository.insert({ + id: 'root', + username: 'root', + usernameLower: 'root', + token: '1234567890123456', + }).then(x => usersRepository.findOneByOrFail(x.identifiers[0])); + + const userProfilesRepository = module.get(DI.userProfilesRepository); + await userProfilesRepository.insert({ + userId: root.id, + }); + + roleService = module.get(RoleService); + role_tinyAttachment = await roleService.create({ + name: 'test-role001', + description: 'Test role001 description', + target: 'manual', + policies: { + maxFileSizeMb: { + useDefault: false, + priority: 1, + // 10byte + value: 10 / 1024 / 1024, + }, + }, + }); + }); + + beforeEach(async () => { + s3Mock.reset(); + await roleService.unassign(root.id, role_tinyAttachment.id).catch(() => {}); + }); + + afterAll(async () => { + await server.close(); + await module.close(); + }); + + test('200 ok', async () => { + const result = await request(server.server) + .post('/api/drive/files/create') + .set('Content-Type', 'multipart/form-data') + .set('Authorization', `Bearer ${root.token}`) + .attach('file', Buffer.from('a'.repeat(1024 * 1024))); + expect(result.statusCode).toBe(200); + }); + + test('200 ok(with role)', async () => { + await roleService.assign(root.id, role_tinyAttachment.id); + + const result = await request(server.server) + .post('/api/drive/files/create') + .set('Content-Type', 'multipart/form-data') + .set('Authorization', `Bearer ${root.token}`) + .attach('file', Buffer.from('a'.repeat(10))); + expect(result.statusCode).toBe(200); + }); + + test('413 too large', async () => { + await roleService.assign(root.id, role_tinyAttachment.id); + + const result = await request(server.server) + .post('/api/drive/files/create') + .set('Content-Type', 'multipart/form-data') + .set('Authorization', `Bearer ${root.token}`) + .attach('file', Buffer.from('a'.repeat(11))); + expect(result.statusCode).toBe(413); + expect(result.body.error.code).toBe('FILE_SIZE_TOO_LARGE'); + }); +}); diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 717421d68e..5b3c95d703 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -547,6 +547,9 @@ importers: '@types/sinonjs__fake-timers': specifier: 8.1.5 version: 8.1.5 + '@types/supertest': + specifier: 6.0.3 + version: 6.0.3 '@types/tinycolor2': specifier: 1.4.6 version: 1.4.6 @@ -598,6 +601,9 @@ importers: simple-oauth2: specifier: 5.1.0 version: 5.1.0 + supertest: + specifier: 7.1.0 + version: 7.1.0 optionalDependencies: '@swc/core-android-arm64': specifier: 1.3.11 @@ -3139,6 +3145,9 @@ packages: peerDependencies: '@opentelemetry/api': ^1.1.0 + '@paralleldrive/cuid2@2.2.2': + resolution: {integrity: sha512-ZOBkgDwEdoYVlSeRbYYXs0S9MejQofiVYoTbKzy/6GQa39/q5tQU2IX46+shYnUkpEl3wc+J6wRlar7r2EK2xA==} + '@parcel/watcher-android-arm64@2.5.0': resolution: {integrity: sha512-qlX4eS28bUcQCdribHkg/herLe+0A9RyYC+mm2PXpncit8z5b3nSqGVzMNR3CmtAOgRutiZ02eIJJgP/b1iEFQ==} engines: {node: '>= 10.0.0'} @@ -4282,6 +4291,9 @@ packages: '@types/cookie@0.6.0': resolution: {integrity: sha512-4Kh9a6B2bQciAhf7FSuMRRkUWecJgJu9nPnx3yzpsfXX/c50REIqpHY4C82bXP90qrLtXtkDxTZosYO3UpOwlA==} + '@types/cookiejar@2.1.5': + resolution: {integrity: sha512-he+DHOWReW0nghN24E1WUqM0efK4kI9oTqDm6XmK8ZPe2djZ90BSNdGnIyCLzCPw7/pogPlGbzI2wHGGmi4O/Q==} + '@types/debug@4.1.12': resolution: {integrity: sha512-vIChWdVG3LG1SMxEvI/AK+FWJthlrqlTu7fbrlywTkkaONwk/UAGaULXRlf8vkzFBLVm0zkMdCquhL5aOjhXPQ==} @@ -4369,6 +4381,9 @@ packages: '@types/mdx@2.0.3': resolution: {integrity: sha512-IgHxcT3RC8LzFLhKwP3gbMPeaK7BM9eBH46OdapPA7yvuIUJ8H6zHZV53J8hGZcTSnt95jANt+rTBNUUc22ACQ==} + '@types/methods@1.1.4': + resolution: {integrity: sha512-ymXWVrDiCxTBE3+RIrrP533E70eA+9qu7zdWoHuOmGujkYtzf4HQF96b8nwHLqhuf4ykX61IGRIB38CC6/sImQ==} + '@types/micromatch@4.0.9': resolution: {integrity: sha512-7V+8ncr22h4UoYRLnLXSpTxjQrNUXtWHGeMPRJt1nULXI57G9bIcpyrHlmrQ7QK24EyyuXvYcSSWAM8GA9nqCg==} @@ -4507,6 +4522,12 @@ packages: '@types/statuses@2.0.4': resolution: {integrity: sha512-eqNDvZsCNY49OAXB0Firg/Sc2BgoWsntsLUdybGFOhAfCD6QJ2n9HXUIHGqt5qjrxmMv4wS8WLAw43ZkKcJ8Pw==} + '@types/superagent@8.1.9': + resolution: {integrity: sha512-pTVjI73witn+9ILmoJdajHGW2jkSaOzhiFYF1Rd3EQ94kymLqB9PjD9ISg7WaALC7+dCHT0FGe9T2LktLq/3GQ==} + + '@types/supertest@6.0.3': + resolution: {integrity: sha512-8WzXq62EXFhJ7QsH3Ocb/iKQ/Ty9ZVWnVzoTKc9tyyFRRF3a74Tk2+TLFgaFFw364Ere+npzHKEJ6ga2LzIL7w==} + '@types/tedious@4.0.14': resolution: {integrity: sha512-KHPsfX/FoVbUGbyYvk1q9MMQHLPeRZhRJZdO45Q4YjvFkv4hMNghCWTvy7rdKessBsmtz4euWCWAB6/tVpI1Iw==} @@ -5572,6 +5593,9 @@ packages: compare-versions@6.1.1: resolution: {integrity: sha512-4hm4VPpIecmlg59CHXnRDnqGplJFrbLG4aFEl5vl6cK1u76ws3LLvX7ikFnTDl5vo39sjWD6AaDPYodJp/NNHg==} + component-emitter@1.3.1: + resolution: {integrity: sha512-T0+barUSQRTUQASh8bx02dl+DhF54GtIDY13Y3m9oWTklKbb3Wv974meRpeZ3lp1JpLVECWWNHC4vaG2XHXouQ==} + compress-commons@6.0.2: resolution: {integrity: sha512-6FqVXeETqWPoGcfzrXb37E50NP0LXT8kAMu5ooZayhWWdgEY4lBEEcbQNXtkuKQsGduxiIcI4gOTsxTmuq/bSg==} engines: {node: '>= 14'} @@ -5629,6 +5653,9 @@ packages: resolution: {integrity: sha512-Xd8lFX4LM9QEEwxQpF9J9NTUh8pmdJO0cyRJhFiDoLTk2eH8FXlRv2IFGYVadZpqI3j8fhNrSdKCeYPxiAhLXw==} engines: {node: '>=18'} + cookiejar@2.1.4: + resolution: {integrity: sha512-LDx6oHrK+PhzLKJU9j5S7/Y3jM/mUHvD/DeI1WQmJn652iPC5Y4TBzC9l+5OMOXlyTTA+SmVUPm0HQUwpD5Jqw==} + core-js@3.29.1: resolution: {integrity: sha512-+jwgnhg6cQxKYIIjGtAHq2nwUOolo9eoFZ4sHfUH09BLXBgxnH4gA0zEd+t+BO2cNB8idaBtZFcFTRjQJRJmAw==} @@ -5938,6 +5965,9 @@ packages: devlop@1.1.0: resolution: {integrity: sha512-RWmIqhcFf1lRYBvNmr7qTNuyCt/7/ns2jbpp1+PalgE/rDQcBT0fioSMUpJ93irlUhC5hrg4cYqe6U+0ImW0rA==} + dezalgo@1.0.4: + resolution: {integrity: sha512-rXSP0bf+5n0Qonsb+SVVfNfIsimO4HEtmnIpPHY8Q1UCzKlQrDMfdobr8nJOOsRgWCyMRqeSBQzmWUMq7zvVig==} + diff-match-patch@1.0.5: resolution: {integrity: sha512-IayShXAgj/QMXgB0IWmKx+rOPuGMhqm5w6jvFxmVenXKIzRqTAAsbBPT3kWQeGANj3jGgvcvv4yK6SxqYmikgw==} @@ -6599,6 +6629,10 @@ packages: resolution: {integrity: sha512-buewHzMvYL29jdeQTVILecSaZKnt/RJWjoZCF5OW60Z67/GmSLBkOFM7qh1PI3zFNtJbaZL5eQu1vLfazOwj4g==} engines: {node: '>=12.20.0'} + formidable@3.5.4: + resolution: {integrity: sha512-YikH+7CUTOtP44ZTnUhR7Ic2UASBPOqmaRkRKxRbywPTe5VxF7RRCck4af9wutiZ/QKM5nME9Bie2fFaPz5Gug==} + engines: {node: '>=14.0.0'} + forwarded-parse@2.1.2: resolution: {integrity: sha512-alTFZZQDKMporBH77856pXgzhEzaUVmLCDk+egLgIgHst3Tpndzz8MnKe+GzRJRfvVdn69HhpW7cmXzvtLvJAw==} @@ -10017,6 +10051,14 @@ packages: peerDependencies: postcss: ^8.4.31 + superagent@9.0.2: + resolution: {integrity: sha512-xuW7dzkUpcJq7QnhOsnNUgtYp3xRwpt2F7abdRYIpCsAt0hhUqia0EdxyXZQQpNmGtsCzYHryaKSV3q3GJnq7w==} + engines: {node: '>=14.18.0'} + + supertest@7.1.0: + resolution: {integrity: sha512-5QeSO8hSrKghtcWEoPiO036fxH0Ii2wVQfFZSP0oqQhmjk8bOLhDFXr4JrvaFmPuEWUoq4znY3uSi8UzLKxGqw==} + engines: {node: '>=14.18.0'} + supports-color@5.5.0: resolution: {integrity: sha512-QjVjwdXIt408MIiAqCX4oUKsgU2EqAGzs2Ppkm4aQYbjm+ZEWEcW4SfFNTr4uMNZma0ey4f5lgLrkB0aX0QMow==} engines: {node: '>=4'} @@ -11530,7 +11572,7 @@ snapshots: '@babel/traverse': 7.24.7 '@babel/types': 7.25.6 convert-source-map: 2.0.0 - debug: 4.4.0(supports-color@5.5.0) + debug: 4.4.0(supports-color@8.1.1) gensync: 1.0.0-beta.2 json5: 2.2.3 semver: 6.3.1 @@ -11550,7 +11592,7 @@ snapshots: '@babel/traverse': 7.24.7 '@babel/types': 7.25.6 convert-source-map: 2.0.0 - debug: 4.4.0(supports-color@5.5.0) + debug: 4.4.0(supports-color@8.1.1) gensync: 1.0.0-beta.2 json5: 2.2.3 semver: 6.3.1 @@ -11772,7 +11814,7 @@ snapshots: '@babel/helper-split-export-declaration': 7.24.7 '@babel/parser': 7.25.6 '@babel/types': 7.25.6 - debug: 4.4.0(supports-color@5.5.0) + debug: 4.4.0(supports-color@8.1.1) globals: 11.12.0 transitivePeerDependencies: - supports-color @@ -12108,7 +12150,7 @@ snapshots: '@eslint/config-array@0.20.0': dependencies: '@eslint/object-schema': 2.1.6 - debug: 4.4.0(supports-color@5.5.0) + debug: 4.4.0(supports-color@8.1.1) minimatch: 3.1.2 transitivePeerDependencies: - supports-color @@ -12122,7 +12164,7 @@ snapshots: '@eslint/eslintrc@3.3.1': dependencies: ajv: 6.12.6 - debug: 4.4.0(supports-color@5.5.0) + debug: 4.4.0(supports-color@8.1.1) espree: 10.3.0 globals: 14.0.0 ignore: 5.3.1 @@ -13161,6 +13203,10 @@ snapshots: '@opentelemetry/api': 1.9.0 '@opentelemetry/core': 1.30.1(@opentelemetry/api@1.9.0) + '@paralleldrive/cuid2@2.2.2': + dependencies: + '@noble/hashes': 1.7.1 + '@parcel/watcher-android-arm64@2.5.0': optional: true @@ -14508,7 +14554,7 @@ snapshots: '@tokenizer/inflate@0.2.7': dependencies: - debug: 4.4.0(supports-color@5.5.0) + debug: 4.4.0(supports-color@8.1.1) fflate: 0.8.2 token-types: 6.0.0 transitivePeerDependencies: @@ -14588,6 +14634,8 @@ snapshots: '@types/cookie@0.6.0': {} + '@types/cookiejar@2.1.5': {} + '@types/debug@4.1.12': dependencies: '@types/ms': 0.7.34 @@ -14681,6 +14729,8 @@ snapshots: '@types/mdx@2.0.3': {} + '@types/methods@1.1.4': {} + '@types/micromatch@4.0.9': dependencies: '@types/braces': 3.0.1 @@ -14816,6 +14866,18 @@ snapshots: '@types/statuses@2.0.4': {} + '@types/superagent@8.1.9': + dependencies: + '@types/cookiejar': 2.1.5 + '@types/methods': 1.1.4 + '@types/node': 22.15.2 + form-data: 4.0.2 + + '@types/supertest@6.0.3': + dependencies: + '@types/methods': 1.1.4 + '@types/superagent': 8.1.9 + '@types/tedious@4.0.14': dependencies: '@types/node': 22.15.2 @@ -14880,7 +14942,7 @@ snapshots: '@typescript-eslint/types': 8.31.0 '@typescript-eslint/typescript-estree': 8.31.0(typescript@5.8.3) '@typescript-eslint/visitor-keys': 8.31.0 - debug: 4.4.0(supports-color@5.5.0) + debug: 4.4.0(supports-color@8.1.1) eslint: 9.25.1 typescript: 5.8.3 transitivePeerDependencies: @@ -14895,7 +14957,7 @@ snapshots: dependencies: '@typescript-eslint/typescript-estree': 8.31.0(typescript@5.8.3) '@typescript-eslint/utils': 8.31.0(eslint@9.25.1)(typescript@5.8.3) - debug: 4.4.0(supports-color@5.5.0) + debug: 4.4.0(supports-color@8.1.1) eslint: 9.25.1 ts-api-utils: 2.1.0(typescript@5.8.3) typescript: 5.8.3 @@ -14908,7 +14970,7 @@ snapshots: dependencies: '@typescript-eslint/types': 8.31.0 '@typescript-eslint/visitor-keys': 8.31.0 - debug: 4.4.0(supports-color@5.5.0) + debug: 4.4.0(supports-color@8.1.1) fast-glob: 3.3.3 is-glob: 4.0.3 minimatch: 9.0.5 @@ -14945,7 +15007,7 @@ snapshots: dependencies: '@ampproject/remapping': 2.3.0 '@bcoe/v8-coverage': 1.0.2 - debug: 4.4.0(supports-color@5.5.0) + debug: 4.4.0(supports-color@8.1.1) istanbul-lib-coverage: 3.2.2 istanbul-lib-report: 3.0.1 istanbul-lib-source-maps: 5.0.6 @@ -15256,14 +15318,14 @@ snapshots: agent-base@6.0.2: dependencies: - debug: 4.4.0(supports-color@5.5.0) + debug: 4.4.0(supports-color@8.1.1) transitivePeerDependencies: - supports-color optional: true agent-base@7.1.0: dependencies: - debug: 4.4.0(supports-color@5.5.0) + debug: 4.4.0(supports-color@8.1.1) transitivePeerDependencies: - supports-color @@ -16100,6 +16162,8 @@ snapshots: compare-versions@6.1.1: {} + component-emitter@1.3.1: {} + compress-commons@6.0.2: dependencies: crc-32: 1.2.2 @@ -16152,6 +16216,8 @@ snapshots: cookie@1.0.1: {} + cookiejar@2.1.4: {} + core-js@3.29.1: {} core-util-is@1.0.2: {} @@ -16534,6 +16600,11 @@ snapshots: dependencies: dequal: 2.0.3 + dezalgo@1.0.4: + dependencies: + asap: 2.0.6 + wrappy: 1.0.2 + diff-match-patch@1.0.5: {} diff-sequences@29.6.3: {} @@ -16836,7 +16907,7 @@ snapshots: esbuild-register@3.5.0(esbuild@0.25.3): dependencies: - debug: 4.4.0(supports-color@5.5.0) + debug: 4.4.0(supports-color@8.1.1) esbuild: 0.25.3 transitivePeerDependencies: - supports-color @@ -17013,7 +17084,7 @@ snapshots: ajv: 6.12.6 chalk: 4.1.2 cross-spawn: 7.0.6 - debug: 4.4.0(supports-color@5.5.0) + debug: 4.4.0(supports-color@8.1.1) escape-string-regexp: 4.0.0 eslint-scope: 8.3.0 eslint-visitor-keys: 4.2.0 @@ -17464,7 +17535,7 @@ snapshots: follow-redirects@1.15.9(debug@4.4.0): optionalDependencies: - debug: 4.4.0(supports-color@5.5.0) + debug: 4.4.0(supports-color@8.1.1) for-each@0.3.3: dependencies: @@ -17492,6 +17563,12 @@ snapshots: dependencies: fetch-blob: 3.2.0 + formidable@3.5.4: + dependencies: + '@paralleldrive/cuid2': 2.2.2 + dezalgo: 1.0.4 + once: 1.4.0 + forwarded-parse@2.1.2: {} forwarded@0.2.0: {} @@ -17906,7 +17983,7 @@ snapshots: http-proxy-agent@7.0.2: dependencies: agent-base: 7.1.3 - debug: 4.4.0(supports-color@5.5.0) + debug: 4.4.0(supports-color@8.1.1) transitivePeerDependencies: - supports-color @@ -17934,7 +18011,7 @@ snapshots: https-proxy-agent@5.0.1: dependencies: agent-base: 6.0.2 - debug: 4.4.0(supports-color@5.5.0) + debug: 4.4.0(supports-color@8.1.1) transitivePeerDependencies: - supports-color optional: true @@ -17942,14 +18019,14 @@ snapshots: https-proxy-agent@7.0.2: dependencies: agent-base: 7.1.0 - debug: 4.4.0(supports-color@5.5.0) + debug: 4.4.0(supports-color@8.1.1) transitivePeerDependencies: - supports-color https-proxy-agent@7.0.6: dependencies: agent-base: 7.1.3 - debug: 4.4.0(supports-color@5.5.0) + debug: 4.4.0(supports-color@8.1.1) transitivePeerDependencies: - supports-color @@ -18047,7 +18124,7 @@ snapshots: dependencies: '@ioredis/commands': 1.2.0 cluster-key-slot: 1.1.2 - debug: 4.4.0(supports-color@5.5.0) + debug: 4.4.0(supports-color@8.1.1) denque: 2.1.0 lodash.defaults: 4.2.0 lodash.isarguments: 3.1.0 @@ -18281,7 +18358,7 @@ snapshots: istanbul-lib-source-maps@4.0.1: dependencies: - debug: 4.4.0(supports-color@5.5.0) + debug: 4.4.0(supports-color@8.1.1) istanbul-lib-coverage: 3.2.2 source-map: 0.6.1 transitivePeerDependencies: @@ -18290,7 +18367,7 @@ snapshots: istanbul-lib-source-maps@5.0.6: dependencies: '@jridgewell/trace-mapping': 0.3.25 - debug: 4.4.0(supports-color@5.5.0) + debug: 4.4.0(supports-color@8.1.1) istanbul-lib-coverage: 3.2.2 transitivePeerDependencies: - supports-color @@ -19314,7 +19391,7 @@ snapshots: micromark@4.0.0: dependencies: '@types/debug': 4.1.12 - debug: 4.4.0(supports-color@5.5.0) + debug: 4.4.0(supports-color@8.1.1) decode-named-character-reference: 1.0.2 devlop: 1.1.0 micromark-core-commonmark: 2.0.0 @@ -20719,7 +20796,7 @@ snapshots: require-in-the-middle@7.3.0: dependencies: - debug: 4.4.0(supports-color@5.5.0) + debug: 4.4.0(supports-color@8.1.1) module-details-from-path: 1.0.3 resolve: 1.22.8 transitivePeerDependencies: @@ -21171,7 +21248,7 @@ snapshots: socks-proxy-agent@8.0.2: dependencies: agent-base: 7.1.3 - debug: 4.4.0(supports-color@5.5.0) + debug: 4.4.0(supports-color@8.1.1) socks: 2.7.1 transitivePeerDependencies: - supports-color @@ -21280,7 +21357,7 @@ snapshots: arg: 5.0.2 bluebird: 3.7.2 check-more-types: 2.24.0 - debug: 4.4.0(supports-color@5.5.0) + debug: 4.4.0(supports-color@8.1.1) execa: 5.1.1 lazy-ass: 1.6.0 ps-tree: 1.2.0 @@ -21472,6 +21549,27 @@ snapshots: postcss: 8.5.3 postcss-selector-parser: 6.1.2 + superagent@9.0.2: + dependencies: + component-emitter: 1.3.1 + cookiejar: 2.1.4 + debug: 4.4.0(supports-color@8.1.1) + fast-safe-stringify: 2.1.1 + form-data: 4.0.2 + formidable: 3.5.4 + methods: 1.1.2 + mime: 2.6.0 + qs: 6.14.0 + transitivePeerDependencies: + - supports-color + + supertest@7.1.0: + dependencies: + methods: 1.1.2 + superagent: 9.0.2 + transitivePeerDependencies: + - supports-color + supports-color@5.5.0: dependencies: has-flag: 3.0.0 @@ -21829,7 +21927,7 @@ snapshots: app-root-path: 3.1.0 buffer: 6.0.3 dayjs: 1.11.13 - debug: 4.4.0(supports-color@5.5.0) + debug: 4.4.0(supports-color@8.1.1) dotenv: 16.4.7 glob: 10.4.5 reflect-metadata: 0.2.2 @@ -22023,7 +22121,7 @@ snapshots: vite-node@3.1.2(@types/node@22.15.2)(sass@1.87.0)(terser@5.39.0)(tsx@4.19.3): dependencies: cac: 6.7.14 - debug: 4.4.0(supports-color@5.5.0) + debug: 4.4.0(supports-color@8.1.1) es-module-lexer: 1.6.0 pathe: 2.0.3 vite: 6.3.3(@types/node@22.15.2)(sass@1.87.0)(terser@5.39.0)(tsx@4.19.3) @@ -22072,7 +22170,7 @@ snapshots: '@vitest/spy': 3.1.2 '@vitest/utils': 3.1.2 chai: 5.2.0 - debug: 4.4.0(supports-color@5.5.0) + debug: 4.4.0(supports-color@8.1.1) expect-type: 1.2.1 magic-string: 0.30.17 pathe: 2.0.3 @@ -22159,7 +22257,7 @@ snapshots: vue-eslint-parser@10.1.3(eslint@9.25.1): dependencies: - debug: 4.4.0(supports-color@5.5.0) + debug: 4.4.0(supports-color@8.1.1) eslint: 9.25.1 eslint-scope: 8.3.0 eslint-visitor-keys: 4.2.0 -- cgit v1.2.3-freya From b91a67d74ec18b80ceaa8522e07bf28a628284d5 Mon Sep 17 00:00:00 2001 From: おさむのひと <46447427+samunohito@users.noreply.github.com> Date: Sat, 3 May 2025 16:23:06 +0900 Subject: Revert "fix: 添付ファイルのあるリクエストを受けたときの初動を改善 (#15896)" (#15927) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Revert "fix: 添付ファイルのあるリクエストを受けたときの初動を改善 (#15896)" This reverts commit 7e8cc4d7c0a86ad0bf71a727fb16132e8bc180a8. * fix CHANGELOG.md --- packages/backend/package.json | 4 +- packages/backend/src/server/ServerService.ts | 7 +- packages/backend/src/server/api/ApiCallService.ts | 166 +++++++-------------- packages/backend/src/server/api/endpoint-base.ts | 10 +- packages/backend/test/e2e/api.ts | 4 +- .../test/unit/server/api/drive/files/create.ts | 108 -------------- pnpm-lock.yaml | 98 ------------ 7 files changed, 65 insertions(+), 332 deletions(-) delete mode 100644 packages/backend/test/unit/server/api/drive/files/create.ts (limited to 'packages/backend/src/server/api/ApiCallService.ts') diff --git a/packages/backend/package.json b/packages/backend/package.json index b9cb0002ab..14ac79ae1b 100644 --- a/packages/backend/package.json +++ b/packages/backend/package.json @@ -229,7 +229,6 @@ "@types/semver": "7.7.0", "@types/simple-oauth2": "5.0.7", "@types/sinonjs__fake-timers": "8.1.5", - "@types/supertest": "6.0.3", "@types/tinycolor2": "1.4.6", "@types/tmp": "0.2.6", "@types/uuid": "^9.0.4", @@ -247,7 +246,6 @@ "jest-mock": "29.7.0", "nodemon": "3.1.10", "pid-port": "1.0.2", - "simple-oauth2": "5.1.0", - "supertest": "7.1.0" + "simple-oauth2": "5.1.0" } } diff --git a/packages/backend/src/server/ServerService.ts b/packages/backend/src/server/ServerService.ts index 5857b3059e..dce47e2290 100644 --- a/packages/backend/src/server/ServerService.ts +++ b/packages/backend/src/server/ServerService.ts @@ -75,7 +75,7 @@ export class ServerService implements OnApplicationShutdown { } @bindThis - public async launch() { + public async launch(): Promise { const fastify = Fastify({ trustProxy: true, logger: false, @@ -135,8 +135,8 @@ export class ServerService implements OnApplicationShutdown { reply.header('content-type', 'text/plain; charset=utf-8'); reply.header('link', `<${encodeURI(location)}>; rel="canonical"`); done(null, [ - 'Refusing to relay remote ActivityPub object lookup.', - '', + "Refusing to relay remote ActivityPub object lookup.", + "", `Please remove 'application/activity+json' and 'application/ld+json' from the Accept header or fetch using the authoritative URL at ${location}.`, ].join('\n')); }); @@ -304,7 +304,6 @@ export class ServerService implements OnApplicationShutdown { } await fastify.ready(); - return fastify; } @bindThis diff --git a/packages/backend/src/server/api/ApiCallService.ts b/packages/backend/src/server/api/ApiCallService.ts index 1b8d33f9c9..b22a8c1837 100644 --- a/packages/backend/src/server/api/ApiCallService.ts +++ b/packages/backend/src/server/api/ApiCallService.ts @@ -6,11 +6,8 @@ import { randomUUID } from 'node:crypto'; import * as fs from 'node:fs'; import * as stream from 'node:stream/promises'; -import { Transform } from 'node:stream'; -import { type MultipartFile } from '@fastify/multipart'; import { Inject, Injectable } from '@nestjs/common'; import * as Sentry from '@sentry/node'; -import { AttachmentFile } from '@/server/api/endpoint-base.js'; import { DI } from '@/di-symbols.js'; import { getIpHash } from '@/misc/get-ip-hash.js'; import type { MiLocalUser, MiUser } from '@/models/User.js'; @@ -19,7 +16,7 @@ import type Logger from '@/logger.js'; import type { MiMeta, UserIpsRepository } from '@/models/_.js'; import { createTemp } from '@/misc/create-temp.js'; import { bindThis } from '@/decorators.js'; -import { type RolePolicies, RoleService } from '@/core/RoleService.js'; +import { RoleService } from '@/core/RoleService.js'; import type { Config } from '@/config.js'; import { sendRateLimitHeaders } from '@/misc/rate-limit-utils.js'; import { SkRateLimiterService } from '@/server/SkRateLimiterService.js'; @@ -194,6 +191,18 @@ export class ApiCallService implements OnApplicationShutdown { return; } + const [path, cleanup] = await createTemp(); + await stream.pipeline(multipartData.file, fs.createWriteStream(path)); + + // ファイルサイズが制限を超えていた場合 + // なお truncated はストリームを読み切ってからでないと機能しないため、stream.pipeline より後にある必要がある + if (multipartData.file.truncated) { + cleanup(); + reply.code(413); + reply.send(); + return; + } + const fields = {} as Record; for (const [k, v] of Object.entries(multipartData.fields)) { fields[k] = typeof v === 'object' && 'value' in v ? v.value : undefined; @@ -208,7 +217,10 @@ export class ApiCallService implements OnApplicationShutdown { return; } this.authenticateService.authenticate(token).then(([user, app]) => { - this.call(endpoint, user, app, fields, multipartData, request, reply).then((res) => { + this.call(endpoint, user, app, fields, { + name: multipartData.filename, + path: path, + }, request, reply).then((res) => { this.send(reply, res); }).catch((err: ApiError) => { this.#sendApiError(reply, err); @@ -278,7 +290,10 @@ export class ApiCallService implements OnApplicationShutdown { user: MiLocalUser | null | undefined, token: MiAccessToken | null | undefined, data: any, - multipartFile: MultipartFile | null, + file: { + name: string; + path: string; + } | null, request: FastifyRequest<{ Body: Record | undefined, Querystring: Record }>, reply: FastifyReply, ) { @@ -354,37 +369,6 @@ export class ApiCallService implements OnApplicationShutdown { } } - // Cast non JSON input - if ((ep.meta.requireFile || request.method === 'GET') && ep.params.properties) { - for (const k of Object.keys(ep.params.properties)) { - const param = ep.params.properties![k]; - if (['boolean', 'number', 'integer'].includes(param.type ?? '') && typeof data[k] === 'string') { - try { - data[k] = JSON.parse(data[k]); - } catch (e) { - throw new ApiError({ - message: 'Invalid param.', - code: 'INVALID_PARAM', - id: '0b5f1631-7c1a-41a6-b399-cce335f34d85', - }, { - param: k, - reason: `cannot cast to ${param.type}`, - }); - } - } - } - } - - if (token && ((ep.meta.kind && !token.permission.some(p => p === ep.meta.kind)) - || (!ep.meta.kind && (ep.meta.requireCredential || ep.meta.requireModerator || ep.meta.requireAdmin)))) { - throw new ApiError({ - message: 'Your app does not have the necessary permissions to use this endpoint.', - code: 'PERMISSION_DENIED', - kind: 'permission', - id: '1370e5b7-d4eb-4566-bb1d-7748ee6a1838', - }); - } - if ((ep.meta.requireModerator || ep.meta.requireAdmin) && (this.meta.rootUserId !== user!.id)) { const myRoles = await this.roleService.getUserRoles(user!.id); if (ep.meta.requireModerator && !myRoles.some(r => r.isModerator || r.isAdministrator)) { @@ -418,89 +402,47 @@ export class ApiCallService implements OnApplicationShutdown { } } - let attachmentFile: AttachmentFile | null = null; - let cleanup = () => {}; - if (ep.meta.requireFile && request.method === 'POST' && multipartFile) { - const policies = await this.roleService.getUserPolicies(user!.id); - const result = await this.handleAttachmentFile( - Math.min((policies.maxFileSizeMb * 1024 * 1024), this.config.maxFileSize), - multipartFile, - ); - attachmentFile = result.attachmentFile; - cleanup = result.cleanup; + if (token && ((ep.meta.kind && !token.permission.some(p => p === ep.meta.kind)) + || (!ep.meta.kind && (ep.meta.requireCredential || ep.meta.requireModerator || ep.meta.requireAdmin)))) { + throw new ApiError({ + message: 'Your app does not have the necessary permissions to use this endpoint.', + code: 'PERMISSION_DENIED', + kind: 'permission', + id: '1370e5b7-d4eb-4566-bb1d-7748ee6a1838', + }); + } + + // Cast non JSON input + if ((ep.meta.requireFile || request.method === 'GET') && ep.params.properties) { + for (const k of Object.keys(ep.params.properties)) { + const param = ep.params.properties![k]; + if (['boolean', 'number', 'integer'].includes(param.type ?? '') && typeof data[k] === 'string') { + try { + data[k] = JSON.parse(data[k]); + } catch (e) { + throw new ApiError({ + message: 'Invalid param.', + code: 'INVALID_PARAM', + id: '0b5f1631-7c1a-41a6-b399-cce335f34d85', + }, { + param: k, + reason: `cannot cast to ${param.type}`, + }); + } + } + } } // API invoking if (this.config.sentryForBackend) { return await Sentry.startSpan({ name: 'API: ' + ep.name, - }, () => { - return ep.exec(data, user, token, attachmentFile, request.ip, request.headers) - .catch((err: Error) => this.#onExecError(ep, data, err, user?.id)) - .finally(() => cleanup()); - }); + }, () => ep.exec(data, user, token, file, request.ip, request.headers) + .catch((err: Error) => this.#onExecError(ep, data, err, user?.id))); } else { - return await ep.exec(data, user, token, attachmentFile, request.ip, request.headers) - .catch((err: Error) => this.#onExecError(ep, data, err, user?.id)) - .finally(() => cleanup()); - } - } - - @bindThis - private async handleAttachmentFile( - fileSizeLimit: number, - multipartFile: MultipartFile, - ) { - function createTooLongError() { - return new ApiError({ - httpStatusCode: 413, - kind: 'client', - message: 'File size is too large.', - code: 'FILE_SIZE_TOO_LARGE', - id: 'ff827ce8-9b4b-4808-8511-422222a3362f', - }); - } - - function createLimitStream(limit: number) { - let total = 0; - - return new Transform({ - transform(chunk, _, callback) { - total += chunk.length; - if (total > limit) { - callback(createTooLongError()); - } else { - callback(null, chunk); - } - }, - }); + return await ep.exec(data, user, token, file, request.ip, request.headers) + .catch((err: Error) => this.#onExecError(ep, data, err, user?.id)); } - - const [path, cleanup] = await createTemp(); - try { - await stream.pipeline( - multipartFile.file, - createLimitStream(fileSizeLimit), - fs.createWriteStream(path), - ); - - // ファイルサイズが制限を超えていた場合 - // なお truncated はストリームを読み切ってからでないと機能しないため、stream.pipeline より後にある必要がある - if (multipartFile.file.truncated) { - throw createTooLongError(); - } - } catch (err) { - cleanup(); - throw err; - } - - return { - attachmentFile: { - name: multipartFile.filename, - path, - }, - cleanup, - }; } @bindThis diff --git a/packages/backend/src/server/api/endpoint-base.ts b/packages/backend/src/server/api/endpoint-base.ts index b063487305..e061aa3a8e 100644 --- a/packages/backend/src/server/api/endpoint-base.ts +++ b/packages/backend/src/server/api/endpoint-base.ts @@ -21,23 +21,23 @@ ajv.addFormat('misskey:id', /^[a-zA-Z0-9]+$/); export type Response = Record | void; -export type AttachmentFile = { +type File = { name: string | null; path: string; }; // TODO: paramsの型をT['params']のスキーマ定義から推論する type Executor = - (params: SchemaType, user: T['requireCredential'] extends true ? MiLocalUser : MiLocalUser | null, token: MiAccessToken | null, file?: AttachmentFile, cleanup?: () => any, ip?: string | null, headers?: Record | null) => - Promise>>; + (params: SchemaType, user: T['requireCredential'] extends true ? MiLocalUser : MiLocalUser | null, token: MiAccessToken | null, file?: File, cleanup?: () => any, ip?: string | null, headers?: Record | null) => + Promise>>; export abstract class Endpoint { - public exec: (params: any, user: T['requireCredential'] extends true ? MiLocalUser : MiLocalUser | null, token: MiAccessToken | null, file?: AttachmentFile, ip?: string | null, headers?: Record | null) => Promise; + public exec: (params: any, user: T['requireCredential'] extends true ? MiLocalUser : MiLocalUser | null, token: MiAccessToken | null, file?: File, ip?: string | null, headers?: Record | null) => Promise; constructor(meta: T, paramDef: Ps, cb: Executor) { const validate = ajv.compile(paramDef); - this.exec = (params: any, user: T['requireCredential'] extends true ? MiLocalUser : MiLocalUser | null, token: MiAccessToken | null, file?: AttachmentFile, ip?: string | null, headers?: Record | null) => { + this.exec = (params: any, user: T['requireCredential'] extends true ? MiLocalUser : MiLocalUser | null, token: MiAccessToken | null, file?: File, ip?: string | null, headers?: Record | null) => { let cleanup: undefined | (() => void) = undefined; if (meta.requireFile) { diff --git a/packages/backend/test/e2e/api.ts b/packages/backend/test/e2e/api.ts index f9e65aaa84..49c6a0636b 100644 --- a/packages/backend/test/e2e/api.ts +++ b/packages/backend/test/e2e/api.ts @@ -159,8 +159,8 @@ describe('API', () => { user: { token: application3 }, }, { status: 403, - code: 'PERMISSION_DENIED', - id: '1370e5b7-d4eb-4566-bb1d-7748ee6a1838', + code: 'ROLE_PERMISSION_DENIED', + id: 'c3d38592-54c0-429d-be96-5636b0431a61', }); await failedApiCall({ diff --git a/packages/backend/test/unit/server/api/drive/files/create.ts b/packages/backend/test/unit/server/api/drive/files/create.ts deleted file mode 100644 index b98892fa03..0000000000 --- a/packages/backend/test/unit/server/api/drive/files/create.ts +++ /dev/null @@ -1,108 +0,0 @@ -/* - * SPDX-FileCopyrightText: syuilo and misskey-project - * SPDX-License-Identifier: AGPL-3.0-only - */ - -import { S3Client } from '@aws-sdk/client-s3'; -import { Test, TestingModule } from '@nestjs/testing'; -import { mockClient } from 'aws-sdk-client-mock'; -import { FastifyInstance } from 'fastify'; -import request from 'supertest'; -import { CoreModule } from '@/core/CoreModule.js'; -import { RoleService } from '@/core/RoleService.js'; -import { DI } from '@/di-symbols.js'; -import { GlobalModule } from '@/GlobalModule.js'; -import { MiRole, UserProfilesRepository, UsersRepository } from '@/models/_.js'; -import { MiUser } from '@/models/User.js'; -import { ServerModule } from '@/server/ServerModule.js'; -import { ServerService } from '@/server/ServerService.js'; - -describe('/drive/files/create', () => { - let module: TestingModule; - let server: FastifyInstance; - const s3Mock = mockClient(S3Client); - let roleService: RoleService; - - let root: MiUser; - let role_tinyAttachment: MiRole; - - beforeAll(async () => { - module = await Test.createTestingModule({ - imports: [GlobalModule, CoreModule, ServerModule], - }).compile(); - module.enableShutdownHooks(); - - const serverService = module.get(ServerService); - server = await serverService.launch(); - - const usersRepository = module.get(DI.usersRepository); - root = await usersRepository.insert({ - id: 'root', - username: 'root', - usernameLower: 'root', - token: '1234567890123456', - }).then(x => usersRepository.findOneByOrFail(x.identifiers[0])); - - const userProfilesRepository = module.get(DI.userProfilesRepository); - await userProfilesRepository.insert({ - userId: root.id, - }); - - roleService = module.get(RoleService); - role_tinyAttachment = await roleService.create({ - name: 'test-role001', - description: 'Test role001 description', - target: 'manual', - policies: { - maxFileSizeMb: { - useDefault: false, - priority: 1, - // 10byte - value: 10 / 1024 / 1024, - }, - }, - }); - }); - - beforeEach(async () => { - s3Mock.reset(); - await roleService.unassign(root.id, role_tinyAttachment.id).catch(() => {}); - }); - - afterAll(async () => { - await server.close(); - await module.close(); - }); - - test('200 ok', async () => { - const result = await request(server.server) - .post('/api/drive/files/create') - .set('Content-Type', 'multipart/form-data') - .set('Authorization', `Bearer ${root.token}`) - .attach('file', Buffer.from('a'.repeat(1024 * 1024))); - expect(result.statusCode).toBe(200); - }); - - test('200 ok(with role)', async () => { - await roleService.assign(root.id, role_tinyAttachment.id); - - const result = await request(server.server) - .post('/api/drive/files/create') - .set('Content-Type', 'multipart/form-data') - .set('Authorization', `Bearer ${root.token}`) - .attach('file', Buffer.from('a'.repeat(10))); - expect(result.statusCode).toBe(200); - }); - - test('413 too large', async () => { - await roleService.assign(root.id, role_tinyAttachment.id); - - const result = await request(server.server) - .post('/api/drive/files/create') - .set('Content-Type', 'multipart/form-data') - .set('Authorization', `Bearer ${root.token}`) - .attach('file', Buffer.from('a'.repeat(11))); - expect(result.statusCode).toBe(413); - expect(result.body.error.code).toBe('FILE_SIZE_TOO_LARGE'); - }); -}); diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index fe009057fa..d4836a8a91 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -652,9 +652,6 @@ importers: '@types/sinonjs__fake-timers': specifier: 8.1.5 version: 8.1.5 - '@types/supertest': - specifier: 6.0.3 - version: 6.0.3 '@types/tinycolor2': specifier: 1.4.6 version: 1.4.6 @@ -709,9 +706,6 @@ importers: simple-oauth2: specifier: 5.1.0 version: 5.1.0 - supertest: - specifier: 7.1.0 - version: 7.1.0 packages/frontend: dependencies: @@ -3034,9 +3028,6 @@ packages: peerDependencies: '@opentelemetry/api': ^1.1.0 - '@paralleldrive/cuid2@2.2.2': - resolution: {integrity: sha512-ZOBkgDwEdoYVlSeRbYYXs0S9MejQofiVYoTbKzy/6GQa39/q5tQU2IX46+shYnUkpEl3wc+J6wRlar7r2EK2xA==} - '@parcel/watcher-android-arm64@2.5.1': resolution: {integrity: sha512-KF8+j9nNbUN8vzOFDpRMsaKBHZ/mcjEjMToVMJOhTozkDonQFFrRcfdLWn6yWKCmJKmdVxSgHiYvTCef4/qcBA==} engines: {node: '>= 10.0.0'} @@ -4150,9 +4141,6 @@ packages: '@types/cookie@0.6.0': resolution: {integrity: sha512-4Kh9a6B2bQciAhf7FSuMRRkUWecJgJu9nPnx3yzpsfXX/c50REIqpHY4C82bXP90qrLtXtkDxTZosYO3UpOwlA==} - '@types/cookiejar@2.1.5': - resolution: {integrity: sha512-he+DHOWReW0nghN24E1WUqM0efK4kI9oTqDm6XmK8ZPe2djZ90BSNdGnIyCLzCPw7/pogPlGbzI2wHGGmi4O/Q==} - '@types/debug@4.1.12': resolution: {integrity: sha512-vIChWdVG3LG1SMxEvI/AK+FWJthlrqlTu7fbrlywTkkaONwk/UAGaULXRlf8vkzFBLVm0zkMdCquhL5aOjhXPQ==} @@ -4237,9 +4225,6 @@ packages: '@types/mdx@2.0.3': resolution: {integrity: sha512-IgHxcT3RC8LzFLhKwP3gbMPeaK7BM9eBH46OdapPA7yvuIUJ8H6zHZV53J8hGZcTSnt95jANt+rTBNUUc22ACQ==} - '@types/methods@1.1.4': - resolution: {integrity: sha512-ymXWVrDiCxTBE3+RIrrP533E70eA+9qu7zdWoHuOmGujkYtzf4HQF96b8nwHLqhuf4ykX61IGRIB38CC6/sImQ==} - '@types/micromatch@4.0.9': resolution: {integrity: sha512-7V+8ncr22h4UoYRLnLXSpTxjQrNUXtWHGeMPRJt1nULXI57G9bIcpyrHlmrQ7QK24EyyuXvYcSSWAM8GA9nqCg==} @@ -4372,12 +4357,6 @@ packages: '@types/statuses@2.0.4': resolution: {integrity: sha512-eqNDvZsCNY49OAXB0Firg/Sc2BgoWsntsLUdybGFOhAfCD6QJ2n9HXUIHGqt5qjrxmMv4wS8WLAw43ZkKcJ8Pw==} - '@types/superagent@8.1.9': - resolution: {integrity: sha512-pTVjI73witn+9ILmoJdajHGW2jkSaOzhiFYF1Rd3EQ94kymLqB9PjD9ISg7WaALC7+dCHT0FGe9T2LktLq/3GQ==} - - '@types/supertest@6.0.3': - resolution: {integrity: sha512-8WzXq62EXFhJ7QsH3Ocb/iKQ/Ty9ZVWnVzoTKc9tyyFRRF3a74Tk2+TLFgaFFw364Ere+npzHKEJ6ga2LzIL7w==} - '@types/tedious@4.0.14': resolution: {integrity: sha512-KHPsfX/FoVbUGbyYvk1q9MMQHLPeRZhRJZdO45Q4YjvFkv4hMNghCWTvy7rdKessBsmtz4euWCWAB6/tVpI1Iw==} @@ -5425,9 +5404,6 @@ packages: compare-versions@6.1.1: resolution: {integrity: sha512-4hm4VPpIecmlg59CHXnRDnqGplJFrbLG4aFEl5vl6cK1u76ws3LLvX7ikFnTDl5vo39sjWD6AaDPYodJp/NNHg==} - component-emitter@1.3.1: - resolution: {integrity: sha512-T0+barUSQRTUQASh8bx02dl+DhF54GtIDY13Y3m9oWTklKbb3Wv974meRpeZ3lp1JpLVECWWNHC4vaG2XHXouQ==} - compress-commons@6.0.2: resolution: {integrity: sha512-6FqVXeETqWPoGcfzrXb37E50NP0LXT8kAMu5ooZayhWWdgEY4lBEEcbQNXtkuKQsGduxiIcI4gOTsxTmuq/bSg==} engines: {node: '>= 14'} @@ -5486,9 +5462,6 @@ packages: resolution: {integrity: sha512-9Kr/j4O16ISv8zBBhJoi4bXOYNTkFLOqSL3UDB0njXxCXNezjeyVrJyGOWtgfs/q2km1gwBcfH8q1yEGoMYunA==} engines: {node: '>=18'} - cookiejar@2.1.4: - resolution: {integrity: sha512-LDx6oHrK+PhzLKJU9j5S7/Y3jM/mUHvD/DeI1WQmJn652iPC5Y4TBzC9l+5OMOXlyTTA+SmVUPm0HQUwpD5Jqw==} - core-util-is@1.0.2: resolution: {integrity: sha512-3lqz5YjWTYnW6dlDa5TLaTCcShfar1e40rmcJVwCBJC6mWlFuj0eCHIElmG1g5kyuJ/GD+8Wn4FFCcz4gJPfaQ==} @@ -5800,9 +5773,6 @@ packages: devlop@1.1.0: resolution: {integrity: sha512-RWmIqhcFf1lRYBvNmr7qTNuyCt/7/ns2jbpp1+PalgE/rDQcBT0fioSMUpJ93irlUhC5hrg4cYqe6U+0ImW0rA==} - dezalgo@1.0.4: - resolution: {integrity: sha512-rXSP0bf+5n0Qonsb+SVVfNfIsimO4HEtmnIpPHY8Q1UCzKlQrDMfdobr8nJOOsRgWCyMRqeSBQzmWUMq7zvVig==} - diff-match-patch@1.0.5: resolution: {integrity: sha512-IayShXAgj/QMXgB0IWmKx+rOPuGMhqm5w6jvFxmVenXKIzRqTAAsbBPT3kWQeGANj3jGgvcvv4yK6SxqYmikgw==} @@ -6446,10 +6416,6 @@ packages: resolution: {integrity: sha512-buewHzMvYL29jdeQTVILecSaZKnt/RJWjoZCF5OW60Z67/GmSLBkOFM7qh1PI3zFNtJbaZL5eQu1vLfazOwj4g==} engines: {node: '>=12.20.0'} - formidable@3.5.4: - resolution: {integrity: sha512-YikH+7CUTOtP44ZTnUhR7Ic2UASBPOqmaRkRKxRbywPTe5VxF7RRCck4af9wutiZ/QKM5nME9Bie2fFaPz5Gug==} - engines: {node: '>=14.0.0'} - forwarded-parse@2.1.2: resolution: {integrity: sha512-alTFZZQDKMporBH77856pXgzhEzaUVmLCDk+egLgIgHst3Tpndzz8MnKe+GzRJRfvVdn69HhpW7cmXzvtLvJAw==} @@ -9843,14 +9809,6 @@ packages: peerDependencies: postcss: ^8.4.31 - superagent@9.0.2: - resolution: {integrity: sha512-xuW7dzkUpcJq7QnhOsnNUgtYp3xRwpt2F7abdRYIpCsAt0hhUqia0EdxyXZQQpNmGtsCzYHryaKSV3q3GJnq7w==} - engines: {node: '>=14.18.0'} - - supertest@7.1.0: - resolution: {integrity: sha512-5QeSO8hSrKghtcWEoPiO036fxH0Ii2wVQfFZSP0oqQhmjk8bOLhDFXr4JrvaFmPuEWUoq4znY3uSi8UzLKxGqw==} - engines: {node: '>=14.18.0'} - supports-color@5.5.0: resolution: {integrity: sha512-QjVjwdXIt408MIiAqCX4oUKsgU2EqAGzs2Ppkm4aQYbjm+ZEWEcW4SfFNTr4uMNZma0ey4f5lgLrkB0aX0QMow==} engines: {node: '>=4'} @@ -12980,10 +12938,6 @@ snapshots: '@opentelemetry/api': 1.9.0 '@opentelemetry/core': 1.30.1(@opentelemetry/api@1.9.0) - '@paralleldrive/cuid2@2.2.2': - dependencies: - '@noble/hashes': 1.7.1 - '@parcel/watcher-android-arm64@2.5.1': optional: true @@ -14342,8 +14296,6 @@ snapshots: '@types/cookie@0.6.0': {} - '@types/cookiejar@2.1.5': {} - '@types/debug@4.1.12': dependencies: '@types/ms': 0.7.34 @@ -14434,8 +14386,6 @@ snapshots: '@types/mdx@2.0.3': {} - '@types/methods@1.1.4': {} - '@types/micromatch@4.0.9': dependencies: '@types/braces': 3.0.1 @@ -14568,18 +14518,6 @@ snapshots: '@types/statuses@2.0.4': {} - '@types/superagent@8.1.9': - dependencies: - '@types/cookiejar': 2.1.5 - '@types/methods': 1.1.4 - '@types/node': 22.15.2 - form-data: 4.0.2 - - '@types/supertest@6.0.3': - dependencies: - '@types/methods': 1.1.4 - '@types/superagent': 8.1.9 - '@types/tedious@4.0.14': dependencies: '@types/node': 22.15.2 @@ -15905,8 +15843,6 @@ snapshots: compare-versions@6.1.1: {} - component-emitter@1.3.1: {} - compress-commons@6.0.2: dependencies: crc-32: 1.2.2 @@ -15960,8 +15896,6 @@ snapshots: cookie@1.0.2: {} - cookiejar@2.1.4: {} - core-util-is@1.0.2: {} core-util-is@1.0.3: {} @@ -16347,11 +16281,6 @@ snapshots: dependencies: dequal: 2.0.3 - dezalgo@1.0.4: - dependencies: - asap: 2.0.6 - wrappy: 1.0.2 - diff-match-patch@1.0.5: {} diff-sequences@29.6.3: {} @@ -17246,12 +17175,6 @@ snapshots: dependencies: fetch-blob: 3.2.0 - formidable@3.5.4: - dependencies: - '@paralleldrive/cuid2': 2.2.2 - dezalgo: 1.0.4 - once: 1.4.0 - forwarded-parse@2.1.2: {} forwarded@0.2.0: {} @@ -21205,27 +21128,6 @@ snapshots: postcss: 8.5.3 postcss-selector-parser: 6.1.2 - superagent@9.0.2: - dependencies: - component-emitter: 1.3.1 - cookiejar: 2.1.4 - debug: 4.4.0(supports-color@8.1.1) - fast-safe-stringify: 2.1.1 - form-data: 4.0.2 - formidable: 3.5.4 - methods: 1.1.2 - mime: 2.6.0 - qs: 6.13.0 - transitivePeerDependencies: - - supports-color - - supertest@7.1.0: - dependencies: - methods: 1.1.2 - superagent: 9.0.2 - transitivePeerDependencies: - - supports-color - supports-color@5.5.0: dependencies: has-flag: 3.0.0 -- cgit v1.2.3-freya From b218251b94e7e0961145c57298f48102f4342818 Mon Sep 17 00:00:00 2001 From: Marie Date: Thu, 8 May 2025 19:46:42 +0000 Subject: added cleanup to more sections --- packages/backend/src/server/api/ApiCallService.ts | 3 +++ 1 file changed, 3 insertions(+) (limited to 'packages/backend/src/server/api/ApiCallService.ts') diff --git a/packages/backend/src/server/api/ApiCallService.ts b/packages/backend/src/server/api/ApiCallService.ts index b22a8c1837..0d2dafd556 100644 --- a/packages/backend/src/server/api/ApiCallService.ts +++ b/packages/backend/src/server/api/ApiCallService.ts @@ -213,6 +213,7 @@ export class ApiCallService implements OnApplicationShutdown { ? request.headers.authorization.slice(7) : fields['i']; if (token != null && typeof token !== 'string') { + cleanup(); reply.code(400); return; } @@ -223,6 +224,7 @@ export class ApiCallService implements OnApplicationShutdown { }, request, reply).then((res) => { this.send(reply, res); }).catch((err: ApiError) => { + cleanup(); this.#sendApiError(reply, err); }); @@ -230,6 +232,7 @@ export class ApiCallService implements OnApplicationShutdown { this.logIp(request, user); } }).catch(err => { + cleanup(); this.#sendAuthenticationError(reply, err); }); } -- cgit v1.2.3-freya