From 1b859a37c56d12774e575282781d4688c1a7484b Mon Sep 17 00:00:00 2001 From: DarkSky <25152247+darkskygit@users.noreply.github.com> Date: Thu, 9 Oct 2025 16:04:18 +0800 Subject: [PATCH] feat: improve attachment headers (#13709) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Summary by CodeRabbit - **New Features** - Safer, consistent file downloads with automatic attachment headers and filenames. - Smarter MIME detection for uploads (avatars, workspace blobs, Copilot files/transcripts). - Sensible default buffer limit when reading uploads. - **Bug Fixes** - Prevents risky content from rendering inline by forcing downloads and adding no‑sniff protection. - More accurate content types when original metadata is missing or incorrect. --- Cargo.lock | 7 +-- Cargo.toml | 2 +- packages/backend/native/Cargo.toml | 1 + packages/backend/native/src/file_type.rs | 10 ++-- .../server/src/__tests__/copilot.e2e.ts | 5 +- .../server/src/__tests__/user/user.e2e.ts | 22 +++++--- .../server/src/__tests__/utils/blobs.ts | 4 ++ .../src/base/storage/providers/index.ts | 2 +- .../src/base/storage/providers/utils.ts | 50 +++++++++++++++++++ .../backend/server/src/base/utils/stream.ts | 3 +- .../server/src/core/user/controller.ts | 10 +++- .../backend/server/src/core/user/resolver.ts | 18 ++++--- .../server/src/core/workspaces/controller.ts | 9 ++++ .../src/plugins/copilot/context/resolver.ts | 7 ++- .../server/src/plugins/copilot/controller.ts | 5 ++ .../server/src/plugins/copilot/resolver.ts | 6 ++- .../src/plugins/copilot/transcript/service.ts | 6 ++- .../src/plugins/copilot/workspace/service.ts | 9 +++- 18 files changed, 143 insertions(+), 33 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index f99bbc88ee..4b8949e94b 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -161,6 +161,7 @@ dependencies = [ "affine_common", "chrono", "file-format", + "infer", "mimalloc", "napi", "napi-build", @@ -1504,9 +1505,9 @@ dependencies = [ [[package]] name = "file-format" -version = "0.26.0" +version = "0.28.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "e7ef3d5e8ae27277c8285ac43ed153158178ef0f79567f32024ca8140a0c7cd8" +checksum = "0eab8aa2fba5f39f494000a22f44bf3c755b7d7f8ffad3f36c6d507893074159" [[package]] name = "flate2" @@ -1913,7 +1914,7 @@ dependencies = [ "js-sys", "log", "wasm-bindgen", - "windows-core 0.57.0", + "windows-core 0.61.2", ] [[package]] diff --git a/Cargo.toml b/Cargo.toml index c75b0795dc..56dfa586ff 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -39,7 +39,7 @@ crossbeam-channel = "0.5" dispatch2 = "0.3" docx-parser = { git = "https://github.com/toeverything/docx-parser" } dotenvy = "0.15" -file-format = { version = "0.26", features = ["reader"] } +file-format = { version = "0.28", features = ["reader"] } homedir = "0.3" infer = { version = "0.19.0" } lasso = { version = "0.7", features = ["multi-threaded"] } diff --git a/packages/backend/native/Cargo.toml b/packages/backend/native/Cargo.toml index b3d8afcb5e..77ff7bdb5e 100644 --- a/packages/backend/native/Cargo.toml +++ b/packages/backend/native/Cargo.toml @@ -10,6 +10,7 @@ crate-type = ["cdylib"] affine_common = { workspace = true, features = ["doc-loader"] } chrono = { workspace = true } file-format = { workspace = true } +infer = { workspace = true } napi = { workspace = true, features = ["async"] } napi-derive = { workspace = true } rand = { workspace = true } diff --git a/packages/backend/native/src/file_type.rs b/packages/backend/native/src/file_type.rs index 4dc79192a7..9bd3d688ed 100644 --- a/packages/backend/native/src/file_type.rs +++ b/packages/backend/native/src/file_type.rs @@ -2,7 +2,11 @@ use napi_derive::napi; #[napi] pub fn get_mime(input: &[u8]) -> String { - file_format::FileFormat::from_bytes(input) - .media_type() - .to_string() + if let Some(kind) = infer::get(&input[..4096.min(input.len())]) { + kind.mime_type().to_string() + } else { + file_format::FileFormat::from_bytes(input) + .media_type() + .to_string() + } } diff --git a/packages/backend/server/src/__tests__/copilot.e2e.ts b/packages/backend/server/src/__tests__/copilot.e2e.ts index 34edcf8282..10b09b7d9a 100644 --- a/packages/backend/server/src/__tests__/copilot.e2e.ts +++ b/packages/backend/server/src/__tests__/copilot.e2e.ts @@ -30,6 +30,7 @@ import { createTestingApp, createWorkspace, inviteUser, + smallestPng, TestingApp, TestUser, } from './utils'; @@ -453,8 +454,6 @@ test('should create message correctly', async t => { randomUUID(), textPromptName ); - const smallestPng = - 'data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAAgAAAAIAQMAAAD+wSzIAAAABlBMVEX///+/v7+jQ3Y5AAAADklEQVQI12P4AIX8EAgALgAD/aNpbtEAAAAASUVORK5CYII'; const pngData = await fetch(smallestPng).then(res => res.arrayBuffer()); const messageId = await createCopilotMessage( app, @@ -475,8 +474,6 @@ test('should create message correctly', async t => { randomUUID(), textPromptName ); - const smallestPng = - 'data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAAgAAAAIAQMAAAD+wSzIAAAABlBMVEX///+/v7+jQ3Y5AAAADklEQVQI12P4AIX8EAgALgAD/aNpbtEAAAAASUVORK5CYII'; const pngData = await fetch(smallestPng).then(res => res.arrayBuffer()); const messageId = await createCopilotMessage( app, diff --git a/packages/backend/server/src/__tests__/user/user.e2e.ts b/packages/backend/server/src/__tests__/user/user.e2e.ts index 38a96ce15d..c6490d3d53 100644 --- a/packages/backend/server/src/__tests__/user/user.e2e.ts +++ b/packages/backend/server/src/__tests__/user/user.e2e.ts @@ -6,6 +6,8 @@ import ava from 'ava'; import { createTestingApp, getPublicUserById, + smallestGif, + smallestPng, TestingApp, updateAvatar, } from '../utils'; @@ -27,7 +29,9 @@ test('should be able to upload user avatar', async t => { const { app } = t.context; await app.signup(); - const avatar = Buffer.from('test'); + const avatar = await fetch(smallestPng) + .then(res => res.arrayBuffer()) + .then(b => Buffer.from(b)); const res = await updateAvatar(app, avatar); t.is(res.status, 200); @@ -36,19 +40,23 @@ test('should be able to upload user avatar', async t => { const avatarRes = await app.GET(new URL(avatarUrl).pathname); - t.deepEqual(avatarRes.body, Buffer.from('test')); + t.deepEqual(avatarRes.body, avatar); }); test('should be able to update user avatar, and invalidate old avatar url', async t => { const { app } = t.context; await app.signup(); - const avatar = Buffer.from('test'); + const avatar = await fetch(smallestPng) + .then(res => res.arrayBuffer()) + .then(b => Buffer.from(b)); let res = await updateAvatar(app, avatar); const oldAvatarUrl = res.body.data.uploadAvatar.avatarUrl; - const newAvatar = Buffer.from('new'); + const newAvatar = await fetch(smallestGif) + .then(res => res.arrayBuffer()) + .then(b => Buffer.from(b)); res = await updateAvatar(app, newAvatar); const newAvatarUrl = res.body.data.uploadAvatar.avatarUrl; @@ -58,14 +66,16 @@ test('should be able to update user avatar, and invalidate old avatar url', asyn t.is(avatarRes.status, 404); const newAvatarRes = await app.GET(new URL(newAvatarUrl).pathname); - t.deepEqual(newAvatarRes.body, Buffer.from('new')); + t.deepEqual(newAvatarRes.body, newAvatar); }); test('should be able to get public user by id', async t => { const { app } = t.context; const u1 = await app.signup(); - const avatar = Buffer.from('test'); + const avatar = await fetch(smallestPng) + .then(res => res.arrayBuffer()) + .then(b => Buffer.from(b)); await updateAvatar(app, avatar); const u2 = await app.signup(); diff --git a/packages/backend/server/src/__tests__/utils/blobs.ts b/packages/backend/server/src/__tests__/utils/blobs.ts index a73426b165..f92163af99 100644 --- a/packages/backend/server/src/__tests__/utils/blobs.ts +++ b/packages/backend/server/src/__tests__/utils/blobs.ts @@ -3,6 +3,10 @@ import { type Blob } from '@prisma/client'; import { TestingApp } from './testing-app'; import { TEST_LOG_LEVEL } from './utils'; +export const smallestPng = + 'data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAAgAAAAIAQMAAAD+wSzIAAAABlBMVEX///+/v7+jQ3Y5AAAADklEQVQI12P4AIX8EAgALgAD/aNpbtEAAAAASUVORK5CYII'; +export const smallestGif = 'data:image/gif;base64,R0lGODlhAQABAAAAACw='; + export async function listBlobs( app: TestingApp, workspaceId: string diff --git a/packages/backend/server/src/base/storage/providers/index.ts b/packages/backend/server/src/base/storage/providers/index.ts index fa8b868973..4617a0cdd7 100644 --- a/packages/backend/server/src/base/storage/providers/index.ts +++ b/packages/backend/server/src/base/storage/providers/index.ts @@ -135,4 +135,4 @@ export const StorageJSONSchema: JSONSchema = { }; export type * from './provider'; -export { autoMetadata, toBuffer } from './utils'; +export { applyAttachHeaders, autoMetadata, sniffMime, toBuffer } from './utils'; diff --git a/packages/backend/server/src/base/storage/providers/utils.ts b/packages/backend/server/src/base/storage/providers/utils.ts index 4f926e8201..9ded0cd5b6 100644 --- a/packages/backend/server/src/base/storage/providers/utils.ts +++ b/packages/backend/server/src/base/storage/providers/utils.ts @@ -1,6 +1,7 @@ import { Readable } from 'node:stream'; import { crc32 } from '@node-rs/crc32'; +import type { Response } from 'express'; import { getStreamAsBuffer } from 'get-stream'; import { getMime } from '../../../native'; @@ -43,4 +44,53 @@ export function autoMetadata( return metadata; } +const DANGEROUS_INLINE_MIME_PREFIXES = [ + 'text/html', + 'application/xhtml+xml', + 'image/svg+xml', + 'application/xml', + 'text/xml', + 'text/javascript', +]; + +export function isDangerousInlineMime(mime: string | undefined) { + if (!mime) return false; + const lower = mime.toLowerCase(); + return DANGEROUS_INLINE_MIME_PREFIXES.some(p => lower.startsWith(p)); +} + +export function applyAttachHeaders( + res: Response, + options: { filename?: string; buffer?: Buffer; contentType?: string } +) { + let { filename, buffer, contentType } = options; + res.setHeader('X-Content-Type-Options', 'nosniff'); + if (!contentType && buffer) contentType = sniffMime(buffer); + if (contentType && isDangerousInlineMime(contentType)) { + const safeName = (filename || 'download') + .replace(/[\r\n]/g, '') + .replace(/[^\w\s.-]/g, '_'); + res.setHeader( + 'Content-Disposition', + `attachment; filename="${encodeURIComponent(safeName)}"; filename*=UTF-8''${encodeURIComponent( + safeName + )}` + ); + } + if (!res.getHeader('Content-Type')) { + res.setHeader('Content-Type', contentType || 'application/octet-stream'); + } +} + +export function sniffMime( + buffer: Buffer, + declared?: string +): string | undefined { + try { + const detected = getMime(buffer); + if (detected) return detected; + } catch {} + return declared; +} + export const SIGNED_URL_EXPIRED = 60 * 60; // 1 hour diff --git a/packages/backend/server/src/base/utils/stream.ts b/packages/backend/server/src/base/utils/stream.ts index 123996eb8e..d1e8f3d52a 100644 --- a/packages/backend/server/src/base/utils/stream.ts +++ b/packages/backend/server/src/base/utils/stream.ts @@ -1,6 +1,7 @@ import { Readable } from 'node:stream'; import { BlobQuotaExceeded, StorageQuotaExceeded } from '../error'; +import { OneKB } from './unit'; export type CheckExceededResult = | { @@ -52,7 +53,7 @@ export async function readBuffer( export async function readBufferWithLimit( readable: Readable, - limit: number + limit: number = 500 * OneKB ): Promise { return readBuffer(readable, size => size > limit diff --git a/packages/backend/server/src/core/user/controller.ts b/packages/backend/server/src/core/user/controller.ts index 46b3575889..9e7d91bdff 100644 --- a/packages/backend/server/src/core/user/controller.ts +++ b/packages/backend/server/src/core/user/controller.ts @@ -1,7 +1,11 @@ import { Controller, Get, Param, Res } from '@nestjs/common'; import type { Response } from 'express'; -import { ActionForbidden, UserAvatarNotFound } from '../../base'; +import { + ActionForbidden, + applyAttachHeaders, + UserAvatarNotFound, +} from '../../base'; import { Public } from '../auth/guard'; import { AvatarStorage } from '../storage'; @@ -30,6 +34,10 @@ export class UserAvatarController { res.setHeader('last-modified', metadata.lastModified.toISOString()); res.setHeader('content-length', metadata.contentLength); } + applyAttachHeaders(res, { + contentType: metadata?.contentType, + filename: `${id}`, + }); body.pipe(res); } diff --git a/packages/backend/server/src/core/user/resolver.ts b/packages/backend/server/src/core/user/resolver.ts index 392e4d1676..8bee67565c 100644 --- a/packages/backend/server/src/core/user/resolver.ts +++ b/packages/backend/server/src/core/user/resolver.ts @@ -17,6 +17,8 @@ import { isNil, omitBy } from 'lodash-es'; import { CannotDeleteOwnAccount, type FileUpload, + readBufferWithLimit, + sniffMime, Throttle, UserNotFound, } from '../../base'; @@ -98,20 +100,20 @@ export class UserResolver { @Args({ name: 'avatar', type: () => GraphQLUpload }) avatar: FileUpload ) { - if (!avatar.mimetype.startsWith('image/')) { - throw new Error('Invalid file type'); - } - if (!user) { throw new UserNotFound(); } + const avatarBuffer = await readBufferWithLimit(avatar.createReadStream()); + const contentType = sniffMime(avatarBuffer, avatar.mimetype); + if (!contentType || !contentType.startsWith('image/')) { + throw new Error(`Invalid file type: ${contentType || 'unknown'}`); + } + const avatarUrl = await this.storage.put( `${user.id}-avatar-${Date.now()}`, - avatar.createReadStream(), - { - contentType: avatar.mimetype, - } + avatarBuffer, + { contentType } ); if (user.avatarUrl) { diff --git a/packages/backend/server/src/core/workspaces/controller.ts b/packages/backend/server/src/core/workspaces/controller.ts index 51e2e18591..319f6bdcf8 100644 --- a/packages/backend/server/src/core/workspaces/controller.ts +++ b/packages/backend/server/src/core/workspaces/controller.ts @@ -2,6 +2,7 @@ import { Controller, Get, Logger, Param, Query, Res } from '@nestjs/common'; import type { Response } from 'express'; import { + applyAttachHeaders, BlobNotFound, CallMetric, CommentAttachmentNotFound, @@ -83,6 +84,10 @@ export class WorkspacesController { } else { this.logger.warn(`Blob ${workspaceId}/${name} has no metadata`); } + applyAttachHeaders(res, { + contentType: metadata?.contentType, + filename: name, + }); res.setHeader('cache-control', 'public, max-age=2592000, immutable'); body.pipe(res); @@ -215,6 +220,10 @@ export class WorkspacesController { `Comment attachment ${workspaceId}/${docId}/${key} has no metadata` ); } + applyAttachHeaders(res, { + contentType: metadata?.contentType, + filename: key, + }); res.setHeader('cache-control', 'private, max-age=2592000, immutable'); body.pipe(res); diff --git a/packages/backend/server/src/plugins/copilot/context/resolver.ts b/packages/backend/server/src/plugins/copilot/context/resolver.ts index 504646b3e3..2fd7356f4c 100644 --- a/packages/backend/server/src/plugins/copilot/context/resolver.ts +++ b/packages/backend/server/src/plugins/copilot/context/resolver.ts @@ -31,6 +31,7 @@ import { EventBus, type FileUpload, RequestMutex, + sniffMime, Throttle, TooManyRequest, UserFriendlyError, @@ -671,7 +672,11 @@ export class CopilotContextResolver { const { filename, mimetype } = content; await this.storage.put(user.id, session.workspaceId, blobId, buffer); - const file = await session.addFile(blobId, filename, mimetype); + const file = await session.addFile( + blobId, + filename, + sniffMime(buffer, mimetype) || mimetype + ); await this.jobs.addFileEmbeddingQueue({ userId: user.id, diff --git a/packages/backend/server/src/plugins/copilot/controller.ts b/packages/backend/server/src/plugins/copilot/controller.ts index c54f76ecc9..88e9313893 100644 --- a/packages/backend/server/src/plugins/copilot/controller.ts +++ b/packages/backend/server/src/plugins/copilot/controller.ts @@ -32,6 +32,7 @@ import { } from 'rxjs'; import { + applyAttachHeaders, BlobNotFound, CallMetric, Config, @@ -795,6 +796,10 @@ export class CopilotController implements BeforeApplicationShutdown { } else { this.logger.warn(`Blob ${workspaceId}/${key} has no metadata`); } + applyAttachHeaders(res, { + contentType: metadata?.contentType, + filename: key, + }); res.setHeader('cache-control', 'public, max-age=2592000, immutable'); body.pipe(res); diff --git a/packages/backend/server/src/plugins/copilot/resolver.ts b/packages/backend/server/src/plugins/copilot/resolver.ts index 68e829d843..63561e3dae 100644 --- a/packages/backend/server/src/plugins/copilot/resolver.ts +++ b/packages/backend/server/src/plugins/copilot/resolver.ts @@ -30,6 +30,7 @@ import { Paginated, PaginationInput, RequestMutex, + sniffMime, Throttle, TooManyRequest, UserFriendlyError, @@ -806,7 +807,10 @@ export class CopilotResolver { filename, uploaded.buffer ); - attachments.push({ attachment, mimeType: blob.mimetype }); + attachments.push({ + attachment, + mimeType: sniffMime(uploaded.buffer, blob.mimetype) || blob.mimetype, + }); } } diff --git a/packages/backend/server/src/plugins/copilot/transcript/service.ts b/packages/backend/server/src/plugins/copilot/transcript/service.ts index 4dc0ec37e2..4b3c4c8f42 100644 --- a/packages/backend/server/src/plugins/copilot/transcript/service.ts +++ b/packages/backend/server/src/plugins/copilot/transcript/service.ts @@ -12,6 +12,7 @@ import { NoCopilotProviderAvailable, OnEvent, OnJob, + sniffMime, } from '../../../base'; import { Models } from '../../../models'; import { PromptService } from '../prompt'; @@ -85,7 +86,10 @@ export class CopilotTranscriptionService { `${blobId}-${idx}`, buffer ); - infos.push({ url, mimeType: blob.mimetype }); + infos.push({ + url, + mimeType: sniffMime(buffer, blob.mimetype) || blob.mimetype, + }); } const model = await this.getModel(userId); diff --git a/packages/backend/server/src/plugins/copilot/workspace/service.ts b/packages/backend/server/src/plugins/copilot/workspace/service.ts index 0091ad976a..abbcc001e6 100644 --- a/packages/backend/server/src/plugins/copilot/workspace/service.ts +++ b/packages/backend/server/src/plugins/copilot/workspace/service.ts @@ -2,7 +2,12 @@ import { createHash } from 'node:crypto'; import { Injectable, OnApplicationBootstrap } from '@nestjs/common'; -import { FileUpload, JobQueue, PaginationInput } from '../../../base'; +import { + FileUpload, + JobQueue, + PaginationInput, + sniffMime, +} from '../../../base'; import { ServerFeature, ServerService } from '../../../core'; import { Models } from '../../../models'; import { CopilotStorage } from '../storage'; @@ -64,7 +69,7 @@ export class CopilotWorkspaceService implements OnApplicationBootstrap { const file = await this.models.copilotWorkspace.addFile(workspaceId, { fileName, blobId, - mimeType: content.mimetype, + mimeType: sniffMime(buffer, content.mimetype) || content.mimetype, size: buffer.length, }); return { blobId, file };