Skip to content

feat(s3): add proxy transfer mode with tokenized upload/download#6492

Open
xqvvu wants to merge 9 commits intolabring:v4.14.9-devfrom
xqvvu:refactor/double-way-to-upload
Open

feat(s3): add proxy transfer mode with tokenized upload/download#6492
xqvvu wants to merge 9 commits intolabring:v4.14.9-devfrom
xqvvu:refactor/double-way-to-upload

Conversation

@xqvvu
Copy link
Contributor

@xqvvu xqvvu commented Mar 3, 2026

No description provided.

@github-actions
Copy link

github-actions bot commented Mar 3, 2026

Preview sandbox Image:

registry.cn-hangzhou.aliyuncs.com/fastgpt/fastgpt-pr:fatsgpt_sandbox_638bd7240d86680dfd0934a3e7e1f519f8c85774

@github-actions
Copy link

github-actions bot commented Mar 3, 2026

Preview mcp_server Image:

registry.cn-hangzhou.aliyuncs.com/fastgpt/fastgpt-pr:fatsgpt_mcp_server_638bd7240d86680dfd0934a3e7e1f519f8c85774

@github-actions
Copy link

github-actions bot commented Mar 3, 2026

Docs Preview:


🚀 FastGPT Document Preview Ready!

🔗 👀 Click here to visit preview

@github-actions
Copy link

github-actions bot commented Mar 3, 2026

Preview fastgpt Image:

registry.cn-hangzhou.aliyuncs.com/fastgpt/fastgpt-pr:fatsgpt_638bd7240d86680dfd0934a3e7e1f519f8c85774

- `STORAGE_SECRET_ACCESS_KEY` Secret Access Key for the service credentials
- `STORAGE_PUBLIC_BUCKET` FastGPT public resource bucket name
- `STORAGE_PRIVATE_BUCKET` FastGPT private resource bucket name
- `STORAGE_TRANSFER_MODE` File transfer mode. Options:
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

填写 EXTERNAL_URL 就走 presigned 是不是就好了,好像不会有重叠

@c121914yu
Copy link
Collaborator

PR Review: feat(s3): add proxy transfer mode with tokenized upload/download

📊 变更概览

✅ 优点

  1. 架构设计合理: 引入 STORAGE_TRANSFER_MODE 环境变量,支持 proxypresigned 两种模式,向后兼容
  2. 安全性增强: 使用 JWT token 验证上传/下载请求,避免直接暴露 S3 凭证
  3. 代码组织良好: 新增 packages/service/common/s3/token.ts 统一管理 token 逻辑
  4. 测试覆盖完整: 新增和更新了相关测试用例
  5. 文档更新及时: 同步更新了中英文文档

⚠️ 问题汇总

🔴 严重问题 (3 个,必须修复)

1. 文件上传缺少 Content-Type 验证

位置: projects/app/src/pages/api/system/file/upload/[token].ts:842-844

问题: 上传接口直接使用客户端提供的 Content-Type,没有验证是否与 token 中声明的 contentType 一致,存在安全风险。

// 当前代码
await bucket.client.uploadObject({
  key: objectKey,
  body: sizeGuardStream,
  contentType: getContentTypeFromHeader(headerContentType || '') || contentType,  // 🔴 问题
  contentLength,
  metadata
});

风险: 攻击者可以绕过 token 中的 contentType 限制,上传任意类型的文件。

建议修复:

// 验证 Content-Type 是否匹配
const headerContentType = Array.isArray(req.headers['content-type'])
  ? req.headers['content-type'][0]
  : req.headers['content-type'];

const actualContentType = getContentTypeFromHeader(headerContentType || '');

// 严格验证:必须与 token 中声明的 contentType 一致
if (actualContentType && actualContentType !== contentType) {
  return Promise.reject('Content-Type mismatch');
}

await bucket.client.uploadObject({
  key: objectKey,
  body: sizeGuardStream,
  contentType: contentType,  // 使用 token 中的 contentType
  contentLength,
  metadata
});

2. 上传接口缺少错误日志

位置: projects/app/src/pages/api/system/file/upload/[token].ts:807-850

问题: 上传接口没有使用 logger 记录错误,难以排查生产环境问题。

建议修复:

import { getLogger, LogCategories } from '@fastgpt/service/common/logger';

const logger = getLogger(LogCategories.INFRA.FILE);

async function handler(req: NextApiRequest) {
  try {
    // ... 现有逻辑
  } catch (error) {
    logger.error('Proxy upload failed', {
      objectKey,
      bucketName,
      error
    });
    throw error;
  }
}

3. Token 验证错误处理不一致

位置: packages/service/common/s3/token.ts:352-364

问题: verifyToken 函数中,JWT 验证失败和 payload 验证失败都抛出 ERROR_ENUM.unAuthFile,但错误原因不同,难以调试。

const verifyToken = <T>(token: string, checker: (value: unknown) => value is T) => {
  return new Promise<T>((resolve, reject) => {
    jwt.verify(token, getTokenSecret(), (err, payload) => {
      if (err) {
        return reject(ERROR_ENUM.unAuthFile);  // JWT 验证失败
      }
      try {
        resolve(parsePayload(payload, checker));
      } catch (error) {
        reject(error);  // Payload 验证失败,也是 ERROR_ENUM.unAuthFile
      }
    });
  });
};

建议修复:

import { getLogger, LogCategories } from '@fastgpt/service/common/logger';

const logger = getLogger(LogCategories.INFRA.S3);

const verifyToken = <T>(token: string, checker: (value: unknown) => value is T) => {
  return new Promise<T>((resolve, reject) => {
    jwt.verify(token, getTokenSecret(), (err, payload) => {
      if (err) {
        logger.warn('JWT verification failed', { error: err.message });
        return reject(ERROR_ENUM.unAuthFile);
      }
      try {
        resolve(parsePayload(payload, checker));
      } catch (error) {
        logger.warn('Token payload validation failed', { payload, error });
        reject(error);
      }
    });
  });
};

🟡 建议改进 (5 个)

4. 文件名清理函数存在安全隐患

位置: projects/app/src/pages/api/system/file/[jwt].ts:668-670

问题: getContentDisposition 函数只替换了 "\,但没有处理其他特殊字符(如换行符、控制字符)。

const getContentDisposition = (filename: string) => {
  const safeFilename = filename.replace(/["\\]/g, '_') || 'file';  // 🟡 不够安全
  return `inline; filename="${safeFilename}"; filename*=UTF-8''${encodeURIComponent(filename || 'file')}`;
};

建议修复:

const getContentDisposition = (filename: string) => {
  // 移除控制字符和危险字符
  const safeFilename = filename
    .replace(/[\x00-\x1F\x7F"\\]/g, '_')
    .substring(0, 255) || 'file';  // 限制长度

  return `inline; filename="${safeFilename}"; filename*=UTF-8''${encodeURIComponent(filename || 'file')}`;
};

5. 重复的 getContentDisposition 函数

位置:

  • projects/app/src/pages/api/system/file/[jwt].ts:668-670
  • projects/app/src/pages/api/system/file/download/[token].ts:708-710

问题: 相同的函数在两个文件中重复定义,违反 DRY 原则。

建议修复: 提取到共享工具函数

// packages/service/common/file/utils.ts
export const getContentDisposition = (filename: string) => {
  const safeFilename = filename
    .replace(/[\x00-\x1F\x7F"\\]/g, '_')
    .substring(0, 255) || 'file';

  return `inline; filename="${safeFilename}"; filename*=UTF-8''${encodeURIComponent(filename || 'file')}`;
};

6. parseRequestFilename 函数可以简化

位置: projects/app/src/pages/api/system/file/download/[token].ts:699-705

问题: 函数逻辑可以更简洁。

// 当前代码
const parseRequestFilename = (filename?: string) => {
  if (!filename) return '';
  try {
    return decodeURIComponent(filename);
  } catch {
    return filename;
  }
};

建议修复:

const parseRequestFilename = (filename?: string) => {
  if (!filename) return '';
  try {
    return decodeURIComponent(filename);
  } catch {
    return filename;  // 解码失败时返回原始值
  }
};

// 或者更简洁的写法
const parseRequestFilename = (filename?: string) =>
  filename ? (decodeURIComponent(filename).catch(() => filename)) : '';

7. 环境变量验证可以更严格

位置: packages/service/env.ts:586-597

问题: STORAGE_TRANSFER_MODE 使用默认值 'proxy',但没有验证 FILE_TOKEN_KEY 是否存在。在 proxy 模式下,FILE_TOKEN_KEY 是必需的。

建议修复:

// packages/service/common/s3/token.ts
const getTokenSecret = () => {
  const secret = process.env.FILE_TOKEN_KEY;
  if (!secret) {
    throw new Error('FILE_TOKEN_KEY environment variable is required for proxy transfer mode');
  }
  return secret;
};

8. 类型定义可以更精确

位置: packages/service/common/s3/token.ts:287-320

问题: metadata 字段类型为 Record<string, string>,但实际使用中可能包含 undefined 值。

type S3UploadTokenPayload = {
  objectKey: string;
  bucketName: string;
  maxSize: number;
  contentType: string;
  metadata?: Record<string, string>;  // 🟡 可能包含 undefined
  type: 'upload';
};

建议修复:

type S3UploadTokenPayload = {
  objectKey: string;
  bucketName: string;
  maxSize: number;
  contentType: string;
  metadata?: Record<string, string | undefined>;  // 更精确
  type: 'upload';
};

🟢 可选优化 (2 个)

9. 可以添加速率限制

位置: projects/app/src/pages/api/system/file/upload/[token].ts

建议: 为上传/下载接口添加速率限制,防止滥用。

import rateLimit from 'express-rate-limit';

const uploadLimiter = rateLimit({
  windowMs: 15 * 60 * 1000, // 15 分钟
  max: 100, // 限制 100 次请求
  message: 'Too many upload requests'
});

10. 可以添加上传进度监控

位置: projects/app/src/pages/api/system/file/upload/[token].ts:826-835

建议: 在 sizeGuardStream 中添加进度监控,便于排查大文件上传问题。

let uploadedBytes = 0;
const sizeGuardStream = new Transform({
  transform(chunk, _, callback) {
    uploadedBytes += chunk.length;
    if (uploadedBytes > maxSize) {
      logger.warn('Upload size exceeded', { uploadedBytes, maxSize, objectKey });
      return callback(new Error('EntityTooLarge'));
    }

    // 每 10MB 记录一次进度
    if (uploadedBytes % (10 * 1024 * 1024) === 0) {
      logger.debug('Upload progress', { uploadedBytes, maxSize, objectKey });
    }

    callback(null, chunk);
  }
});

🧪 测试建议

需要补充的测试场景

  1. 上传接口测试:

    • ✅ 已有: 文件大小超限测试
    • ❌ 缺少: Content-Type 不匹配测试
    • ❌ 缺少: Token 过期测试
    • ❌ 缺少: 并发上传测试
  2. 下载接口测试:

    • ✅ 已有: 基本下载功能
    • ❌ 缺少: HEAD 请求测试
    • ❌ 缺少: 文件名编码测试
    • ❌ 缺少: 大文件流式下载测试
  3. Token 验证测试:

    • ✅ 已有: 基本 token 验证
    • ❌ 缺少: Token 篡改测试
    • ❌ 缺少: 不同 bucket 的 token 混用测试

建议添加的测试:

// test/cases/service/common/s3/token.test.ts
describe('S3 Token Security', () => {
  it('should reject token with tampered payload', async () => {
    const token = jwtSignS3UploadToken({...});
    const tamperedToken = token.replace(/\w/, 'x');

    await expect(jwtVerifyS3UploadToken(tamperedToken))
      .rejects.toThrow('unAuthFile');
  });

  it('should reject expired token', async () => {
    const expiredTime = new Date(Date.now() - 1000); // 1秒前过期
    const token = jwtSignS3UploadToken({ expiredTime, ... });

    await new Promise(resolve => setTimeout(resolve, 1100));

    await expect(jwtVerifyS3UploadToken(token))
      .rejects.toThrow('unAuthFile');
  });
});

💬 总体评价

代码质量评分

  • 代码质量: ⭐⭐⭐⭐☆ (4/5) - 整体结构良好,但有安全细节需要完善
  • 安全性: ⭐⭐⭐☆☆ (3/5) - 引入了 token 验证,但上传验证不够严格
  • 性能: ⭐⭐⭐⭐☆ (4/5) - 使用流式处理,性能良好
  • 可维护性: ⭐⭐⭐⭐☆ (4/5) - 代码组织清晰,但有少量重复

架构评价

优秀的设计决策:

  1. 使用环境变量控制传输模式,灵活且向后兼容
  2. Token 化的上传/下载 URL,提升安全性
  3. 统一的 token 管理模块,便于维护
  4. 完整的测试覆盖和文档更新

⚠️ 需要改进的地方:

  1. 上传接口的 Content-Type 验证不够严格
  2. 错误日志不够完善,影响问题排查
  3. 存在代码重复(getContentDisposition 函数)

🚀 审查结论

建议: ⚠️ 需要修改后合并

必须修复的问题 (阻塞合并)

  1. 🔴 添加 Content-Type 验证(安全问题)
  2. 🔴 添加上传接口错误日志(可观测性)
  3. 🔴 改进 token 验证错误处理(调试体验)

建议修复的问题 (推荐但不阻塞)

  1. 🟡 改进文件名清理函数
  2. 🟡 消除重复代码
  3. 🟡 简化 parseRequestFilename
  4. 🟡 验证 FILE_TOKEN_KEY 环境变量
  5. 🟡 改进类型定义精确度

可选优化 (锦上添花)

  1. 🟢 添加速率限制
  2. 🟢 添加上传进度监控

📋 行级评论位置

以下位置需要添加具体的行级评论:

  1. projects/app/src/pages/api/system/file/upload/[token].ts:842 - Content-Type 验证
  2. projects/app/src/pages/api/system/file/upload/[token].ts:807 - 错误日志
  3. packages/service/common/s3/token.ts:352 - Token 验证错误处理
  4. projects/app/src/pages/api/system/file/[jwt].ts:668 - 文件名清理
  5. projects/app/src/pages/api/system/file/download/[token].ts:708 - 重复代码
  6. packages/service/env.ts:597 - 环境变量验证

审查人: Claude Opus 4.6
审查时间: 2026-03-05
审查标准: FastGPT PR Review Guidelines v1.0

Copy link
Collaborator

@c121914yu c121914yu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📍 详细代码评论

由于 GitHub API 限制,行级评论无法自动提交。以下是需要关注的关键代码位置:

🔴 严重问题

1. Content-Type 验证缺失 ()

  • 问题:直接使用客户端提供的 Content-Type,存在安全风险
  • 建议:验证 Content-Type 是否与 token 中声明的一致

2. 错误日志缺失 ()

  • 问题:上传接口没有使用 logger 记录错误
  • 建议:添加 logger.error 记录上传失败信息

3. Token 验证错误处理 ()

  • 问题:JWT 验证失败和 payload 验证失败都抛出相同错误,难以调试
  • 建议:添加 logger.warn 区分不同的验证失败原因

🟡 建议改进

4. 文件名清理 ()

  • 建议:处理控制字符并限制文件名长度

5. 重复代码 ()

  • 建议:将 getContentDisposition 提取到共享工具函数

6. 环境变量验证 ()

  • 建议:在 getTokenSecret 中验证 FILE_TOKEN_KEY 是否存在

7. 类型定义 ()

  • 建议:metadata 类型改为 Record<string, string | undefined>

完整的审查报告和修复建议请查看上方评论。

@c121914yu
Copy link
Collaborator

📍 详细代码评论

由于 GitHub API 限制,行级评论无法自动提交。以下是需要关注的关键代码位置:

🔴 严重问题

1. Content-Type 验证缺失

  • 文件:projects/app/src/pages/api/system/file/upload/[token].ts
  • 行号:842-844
  • 问题:直接使用客户端提供的 Content-Type,存在安全风险
  • 建议:验证 Content-Type 是否与 token 中声明的一致

2. 错误日志缺失

  • 文件:projects/app/src/pages/api/system/file/upload/[token].ts
  • 行号:807
  • 问题:上传接口没有使用 logger 记录错误
  • 建议:添加 logger.error 记录上传失败信息

3. Token 验证错误处理

  • 文件:packages/service/common/s3/token.ts
  • 行号:352-364
  • 问题:JWT 验证失败和 payload 验证失败都抛出相同错误,难以调试
  • 建议:添加 logger.warn 区分不同的验证失败原因

🟡 建议改进

4. 文件名清理

  • 文件:projects/app/src/pages/api/system/file/[jwt].ts
  • 行号:668-670
  • 建议:处理控制字符并限制文件名长度

5. 重复代码

  • 文件:projects/app/src/pages/api/system/file/download/[token].ts
  • 行号:708-710
  • 建议:将 getContentDisposition 提取到共享工具函数

6. 环境变量验证

  • 文件:packages/service/common/s3/token.ts
  • 行号:323
  • 建议:在 getTokenSecret 中验证 FILE_TOKEN_KEY 是否存在

7. 类型定义

  • 文件:packages/service/common/s3/token.ts
  • 行号:302
  • 建议:metadata 类型改为 Record<string, string | undefined>

完整的审查报告和修复建议请查看上方评论。

@c121914yu c121914yu force-pushed the v4.14.8-dev branch 2 times, most recently from ef98296 to f268dec Compare March 9, 2026 13:00
xqvvu and others added 7 commits March 12, 2026 09:56
* feat: encapsulate logger

* update engines

---------

Co-authored-by: archer <545436317@qq.com>
* docs: switch to docs layout and apply black theme (labring#6533)

* feat: add Gemini 3.1 models

- Add gemini-3.1-pro-preview (released February 19, 2026)
- Add gemini-3.1-flash-lite-preview (released March 3, 2026)

Both models support:
- 1M context window
- 64k max response
- Vision
- Tool choice

* docs: switch to docs layout and apply black theme

- Change layout from notebook to docs
- Update logo to icon + text format
- Apply fumadocs black theme
- Simplify global.css (keep only navbar and TOC styles)
- Fix icon components to properly accept className props
- Add mobile text overflow handling
- Update Node engine requirement to >=20.x

* doc

* doc

* lock

* fix: ts

* doc

* doc

---------

Co-authored-by: archer <archer@archerdeMac-mini.local>
Co-authored-by: archer <545436317@qq.com>

* Doc (labring#6493)

* cloud doc

* doc refactor

* doc move

* seo

* remove doc

* yml

* doc

* fix: tsconfig

* fix: tsconfig

* sandbox version (labring#6497)

* sandbox version

* add sandbox log

* update lock

* fix

* fix: sandbox

* doc

* add console

* i18n

* sandbxo in agent

* feat: agent sandbox

* lock

* feat: sandbox ui

* sandbox check exists

* env tempalte

* doc

* lock

* sandbox in chat window

* sandbox entry

* fix: test

* rename var

* sandbox config tip

* update sandbox lifecircle

* update prompt

* rename provider test

* sandbox logger

* yml

---------

Co-authored-by: Archer <archer@fastgpt.io>
Co-authored-by: archer <archer@archerdeMac-mini.local>
@xqvvu xqvvu marked this pull request as ready for review March 12, 2026 07:57
@xqvvu xqvvu force-pushed the refactor/double-way-to-upload branch from 8973c07 to b18e341 Compare March 12, 2026 07:59
@cla-assistant
Copy link

cla-assistant bot commented Mar 12, 2026

CLA assistant check
All committers have signed the CLA.

@cla-assistant
Copy link

cla-assistant bot commented Mar 12, 2026

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you all sign our Contributor License Agreement before we can accept your contribution.
3 out of 4 committers have signed the CLA.

✅ FinleyGe
✅ xqvvu
✅ c121914yu
❌ archer-claw
You have signed the CLA already but the status is still pending? Let us recheck it.

@xqvvu xqvvu force-pushed the refactor/double-way-to-upload branch from b18e341 to 8fe6fe6 Compare March 12, 2026 08:01
@xqvvu xqvvu changed the base branch from v4.14.8-dev to v4.14.9-dev March 12, 2026 08:02
@c121914yu c121914yu force-pushed the v4.14.9-dev branch 2 times, most recently from 579d76f to 7b20bfd Compare March 13, 2026 09:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants