Conversation
There was a problem hiding this comment.
Pull request overview
This PR tightens team token authorization for chat endpoints by enforcing that the requested appId belongs to the authenticated teamId and is accessible under the team token’s tags (via app.teamTags).
Changes:
- Extend
authTeamSpaceTokento returntagsalongsideuidandtmbId. - Add
MongoApplookups to validate team ownership and tag-based access before allowing team-token chat operations. - Align team-token auth failures to reject with
ChatErrEnum.unAuthChatin v1/v2 completions.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| projects/app/src/service/support/permission/auth/team.ts | Return tags from team-token auth result for downstream authorization. |
| projects/app/src/service/support/permission/auth/chat.ts | Enforce app/team/tag constraints when using teamId + teamToken chat auth. |
| projects/app/src/pages/api/v2/chat/completions.ts | Add tag-aware app validation for team-token requests; parallelize app/chat lookup. |
| projects/app/src/pages/api/v1/chat/completions.ts | Same as v2: tag-aware app validation for team-token requests. |
| projects/app/src/pages/api/core/chat/team/init.ts | Restrict team init chat to apps allowed by team-token tags. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| { teamTags: { $in: tags } } | ||
| ] | ||
| }).lean(), | ||
| MongoChat.findOne({ appId, chatId }).lean() |
There was a problem hiding this comment.
authTeamSpaceChat queries MongoChat.findOne({ appId, chatId }) even when chatId is optional. If chatId is undefined, Mongo may match an arbitrary chat for the appId, which can incorrectly trigger the subsequent auth rejection. Build the chat query conditionally (only run findOne when chatId is provided; otherwise treat chat as null/undefined).
| MongoChat.findOne({ appId, chatId }).lean() | |
| chatId ? MongoChat.findOne({ appId, chatId }).lean() : Promise.resolve(null) |
| { teamTags: { $in: tags } } | ||
| ] | ||
| }).lean(), | ||
| MongoChat.findOne({ appId, chatId }).lean() |
There was a problem hiding this comment.
authTeamSpaceChat queries MongoChat.findOne({ appId, chatId }) even when chatId is optional. If chatId is undefined, Mongo may match an arbitrary chat for the appId, which can incorrectly trigger the subsequent auth rejection. Build the chat query conditionally (only run findOne when chatId is provided; otherwise treat chat as null/undefined).
| MongoChat.findOne({ appId, chatId }).lean() | |
| chatId ? MongoChat.findOne({ appId, chatId }).lean() : Promise.resolve(null) |
|
|
||
| const [team, chat, app] = await Promise.all([ | ||
| MongoTeam.findById(teamId, 'name avatar').lean(), | ||
| MongoChat.findOne({ appId, chatId }).lean(), |
There was a problem hiding this comment.
chatId is optional for this init endpoint, but MongoChat.findOne({ appId, chatId }) will still run with chatId potentially undefined. That can match an arbitrary chat for the app and cause incorrect unAuthChat failures. Only query MongoChat when chatId is provided (or omit the chatId field from the query object when it’s undefined).
| MongoChat.findOne({ appId, chatId }).lean(), | |
| chatId ? MongoChat.findOne({ appId, chatId }).lean() : Promise.resolve(null), |
| // Verify app belongs to the authenticated team and tag-based access | ||
| const app = await MongoApp.findOne( | ||
| { | ||
| _id: appId, | ||
| teamId: spaceTeamId, | ||
| $or: [ | ||
| { teamTags: { $size: 0 } }, | ||
| { teamTags: { $exists: false } }, | ||
| { teamTags: { $in: tags } } | ||
| ] |
There was a problem hiding this comment.
The team-tag access filter (teamTags $size/$exists/$in) is now duplicated across multiple call sites (authChatCrud, v1/v2 completions, team init). Consider extracting a shared helper to build this Mongo filter to keep the authorization logic consistent and reduce the risk of future drift when the rules change.
|
✅ Build Successful - Preview code-sandbox Image for this PR: |
|
✅ Build Successful - Preview fastgpt Image for this PR: |
|
✅ Build Successful - Preview mcp_server Image for this PR: |
c121914yu
left a comment
c121914yu
left a comment
There was a problem hiding this comment.
📊 代码审查总结
此 PR 修复了多处 team token 鉴权逻辑漏洞，包括：tag 过滤、app 归属校验、版本 IDOR、chat 跨团队越权等问题。整体安全修复方向正确，详细的审查意见请查看下方的行级评论。
🟡 额外建议（非 diff 行）
1. chat.ts:116 一致性问题
authChatCrud 中 teamDomain 分支的 chat 校验只检查了 outLinkUid：
if (chat.outLinkUid !== uid) return Promise.reject(ChatErrEnum.unAuthChat);但在 team/init.ts:50 和 v1/v2 completions.ts 中，同样的逻辑还额外检查了 String(chat.teamId) !== teamId。建议补上 teamId 检查以保持一致。
2. 测试覆盖
缺少 MongoApp.findOne 返回 null（app 不属于该 team 或 tag 不匹配）的测试用例。这是本次 PR 新增的核心安全逻辑，建议增加：
it('should reject if app does not belong to team or tags mismatch', async () => {
vi.mocked(authTeamSpaceToken).mockResolvedValue({ uid: 'user1', tmbId: 'tmb1', tags: ['tag1'] });
vi.mocked(MongoApp.findOne).mockReturnValue({ lean: () => Promise.resolve(null) } as any);
await expect(authChatCrud({ appId: 'app1', teamId: 'team1', teamToken: 'token1', req: {} as any, authToken: true })).rejects.toBe(ChatErrEnum.unAuthChat);
});| { teamTags: { $in: tags } } | ||
| ] | ||
| }).lean(); | ||
| if (!app) return { list: [], total: 0 }; |
There was a problem hiding this comment.
🔴 严重问题: 此处从构建 match 查询条件的 IIFE 中返回 { list: [], total: 0 }。这个值会被赋给 match，由于它是 truthy 的，会跳过 if (!match) 检查，然后在第 107 行被展开到 mergeMatch 中用作 MongoDB 查询条件：
const mergeMatch = { ...match, ...timeMatch, deleteTime: null };
// => { list: [], total: 0, deleteTime: null } — 无意义的数据库查询虽然最终结果碰巧正确（因为数据库不会匹配到文档），但这会产生一次不必要的数据库查询，且语义错误。
建议: 返回 undefined 让后面的 if (!match) 逻辑处理早返回：
if (!app) return undefined;| ); | ||
| } | ||
| const app = await MongoApp.findById(currentAppId); | ||
| const app = await MongoApp.findOne({ _id: currentAppId, teamId }); |
There was a problem hiding this comment.
✅ 优点: API key 场景下查找 app 时增加 teamId 约束，防止跨团队访问。
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
No description provided.