Fix: fixed a potential vulnerability in /api/chat/get_file endpoint.
#1676
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
I have fixed a potential vulnerability in the `/api/chat/get_file` endpoint that could allow unauthorized access to files by ensuring the request has a jwt token.
后端安全组件的更新类图classDiagram
class ChatRoutes {
+get_file(self): QuartResponse
}
class Server {
-allowed_endpoints: String[]
#auth_middleware()
}
文件级别变更
针对关联问题的评估
可能关联的问题
提示和命令与 Sourcery 互动
自定义您的体验访问您的 dashboard 以：
获得帮助
Original review guide in EnglishReviewer's GuideThis PR secures the Updated Class Diagram for ChatPage.vueclassDiagram
class ChatPage {
+stagedImagesName: String[]
+stagedImagesUrl: String[]
+mediaCache: Object
+getMediaFile(filename: String): Promise~String~
+cleanupMediaCache(): void
+startListeningEvent(): void
+getConversationMessages(cid: String): void
+uploadImage(event: Event): void
+removeImage(index: Number): void
+sendMessage(): void
+beforeUnmount(): void
}
Updated Class Diagram for Backend Security ComponentsclassDiagram
class ChatRoutes {
+get_file(self): QuartResponse
}
class Server {
-allowed_endpoints: String[]
#auth_middleware()
}
File-Level Changes
Assessment against linked issues
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
嘿 @Soulter - 我已经查看了你的更改 - 这里有一些反馈：
- 在
get_file端点中，你手动剥离和验证文件名——考虑使用经过良好测试的助手函数，如 Werkzeug 的secure_filename或严格的允许文件名白名单，以简化和加强清理。 - 从
allowed_endpoints中删除/api/chat/get_file会强制执行 JWT 身份验证——仔细检查你的中间件排序是否在文件处理程序之前实际运行auth_middleware，并添加一个快速手动测试以进行未经身份验证的访问。 - 管理两个并行数组（
stagedImagesName和stagedImagesUrl）使 UI 逻辑难以理解——考虑将它们统一为单个对象数组{ filename, url }，这样你就可以更干净地映射和撤销 blob URL。
以下是我在审查期间查看的内容
- 🟡 一般问题：发现 4 个问题
- 🟢 安全性：一切看起来都不错
- 🟢 测试：一切看起来都不错
- 🟢 复杂性：一切看起来都不错
- 🟢 文档：一切看起来都不错
帮助我变得更有用！请点击每个评论上的 👍 或 👎，我将使用反馈来改进你的评论。
Original comment in English
Hey @Soulter - I've reviewed your changes - here's some feedback:
- In the
get_fileendpoint you manually strip and validate filenames—consider using a well-tested helper like Werkzeug’ssecure_filenameor a strict whitelist of allowed filenames to simplify and harden sanitization. - Removing
/api/chat/get_filefromallowed_endpointsenforces JWT auth—double-check that your middleware ordering actually runsauth_middlewarebefore the file handler and add a quick manual test for unauthenticated access. - Managing two parallel arrays (
stagedImagesNameandstagedImagesUrl) makes the UI logic hard to follow—consider unifying them into a single array of objects{ filename, url }so you can map and revoke blob URLs more cleanly.
Here's what I looked at during the review
- 🟡 General issues: 4 issues found
- 🟢 Security: all looks good
- 🟢 Testing: all looks good
- 🟢 Complexity: all looks good
- 🟢 Documentation: all looks good
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| }, | ||
|
|
||
| methods: { | ||
| async getMediaFile(filename) { |
There was a problem hiding this comment.
suggestion (performance): mediaCache 没有缓存驱逐机制，可能导致内存泄漏。
实施缓存驱逐策略或大小限制，以防止 mediaCache 增长导致无限制的内存使用。
建议的实现方式：
data() {
return {
// ...其他数据属性
mediaCache: new Map(), // 使用 Map 实现 LRU
mediaCacheMaxSize: 50, // 设置最大缓存大小
};
},
methods: {
async getMediaFile(filename) {
if (this.mediaCache.has(filename)) {
// 将访问过的项目移动到末尾（最近使用）
const value = this.mediaCache.get(filename);
this.mediaCache.delete(filename);
this.mediaCache.set(filename, value);
return value;
}
try {
const response = await axios.get('/api/chat/get_file', {
params: { filename },
responseType: 'blob'
});
const blobUrl = URL.createObjectURL(response.data);
// 如果缓存已满，则驱逐最近最少使用的（第一个）条目
if (this.mediaCache.size >= this.mediaCacheMaxSize) {
const firstKey = this.mediaCache.keys().next().value;
URL.revokeObjectURL(this.mediaCache.get(firstKey)); // 清理 blob
this.mediaCache.delete(firstKey);
}
this.mediaCache.set(filename, blobUrl);
- 更新任何假定
mediaCache是普通对象的代码，以使用Map。 - 确保
cleanupMediaCache()迭代Map并在清除之前撤销所有 blob URL。
Original comment in English
suggestion (performance): No cache eviction for mediaCache may lead to memory leaks.
Implement a cache eviction policy or size limit to prevent unbounded memory usage as mediaCache grows.
Suggested implementation:
data() {
return {
// ...other data properties
mediaCache: new Map(), // Use Map for LRU
mediaCacheMaxSize: 50, // Set a max cache size
};
},
methods: {
async getMediaFile(filename) {
if (this.mediaCache.has(filename)) {
// Move accessed item to the end (most recently used)
const value = this.mediaCache.get(filename);
this.mediaCache.delete(filename);
this.mediaCache.set(filename, value);
return value;
}
try {
const response = await axios.get('/api/chat/get_file', {
params: { filename },
responseType: 'blob'
});
const blobUrl = URL.createObjectURL(response.data);
// If cache is full, evict the least recently used (first) entry
if (this.mediaCache.size >= this.mediaCacheMaxSize) {
const firstKey = this.mediaCache.keys().next().value;
URL.revokeObjectURL(this.mediaCache.get(firstKey)); // Cleanup blob
this.mediaCache.delete(firstKey);
}
this.mediaCache.set(filename, blobUrl);
- Update any code that assumes
mediaCacheis a plain object to work with aMapinstead. - Ensure that
cleanupMediaCache()iterates over theMapand revokes all blob URLs before clearing.
| @@ -623,6 +662,15 @@ export default { | |||
| } | |||
| } | |||
| }, | |||
|
|
|||
| cleanupMediaCache() { | |||
There was a problem hiding this comment.
suggestion (bug_risk): cleanupMediaCache 仅在组件卸载时调用，而不是在图像/音频移除时调用。
仅在卸载时撤销 blob URL 可能会导致用户在发送之前移除媒体时发生内存泄漏。请在从暂存区移除媒体时撤销 blob URL。
建议的实现方式：
cleanupMediaCache() {
Object.values(this.mediaCache).forEach(url => {
if (url.startsWith('blob:')) {
URL.revokeObjectURL(url);
}
});
this.mediaCache = {};
},
removeMediaFromCache(key) {
const url = this.mediaCache[key];
if (url && url.startsWith('blob:')) {
URL.revokeObjectURL(url);
}
this.$delete(this.mediaCache, key);
},
现在，无论何时从暂存区移除媒体（例如，当用户在发送之前移除图像/音频时），都必须使用 removeMediaFromCache(key)，而不是直接从 mediaCache 中删除。
找到代码中所有执行类似 delete this.mediaCache[key] 或 this.$delete(this.mediaCache, key) 的地方，并将它们替换为 this.removeMediaFromCache(key)。
Original comment in English
suggestion (bug_risk): cleanupMediaCache only called on component unmount, not on image/audio removal.
Revoking blob URLs only on unmount can cause memory leaks if users remove media before sending. Please revoke blob URLs when media is removed from staging.
Suggested implementation:
cleanupMediaCache() {
Object.values(this.mediaCache).forEach(url => {
if (url.startsWith('blob:')) {
URL.revokeObjectURL(url);
}
});
this.mediaCache = {};
},
removeMediaFromCache(key) {
const url = this.mediaCache[key];
if (url && url.startsWith('blob:')) {
URL.revokeObjectURL(url);
}
this.$delete(this.mediaCache, key);
},
You must now use removeMediaFromCache(key) instead of directly deleting from mediaCache wherever media is removed from staging (e.g., when a user removes an image/audio before sending).
Find all places in your code where you do something like delete this.mediaCache[key] or this.$delete(this.mediaCache, key) and replace them with this.removeMediaFromCache(key).
Comment on lines
476
to
479
| removeImage(index) { | ||
| this.stagedImagesName.splice(index, 1); | ||
| this.stagedImagesUrl.splice(index, 1); | ||
| }, |
There was a problem hiding this comment.
suggestion (performance): 从 stagedImagesUrl 中移除图像不会撤销 blob URL。
请在 removeImage 中对已移除图像的 URL 调用 URL.revokeObjectURL 以释放内存。
Suggested change
| removeImage(index) { | |
| this.stagedImagesName.splice(index, 1); | |
| this.stagedImagesUrl.splice(index, 1); | |
| }, | |
| removeImage(index) { | |
| // 撤销 blob URL 以释放内存 | |
| const url = this.stagedImagesUrl[index]; | |
| if (url) { | |
| URL.revokeObjectURL(url); | |
| } | |
| this.stagedImagesName.splice(index, 1); | |
| this.stagedImagesUrl.splice(index, 1); | |
| }, |
Original comment in English
suggestion (performance): Removing images from stagedImagesUrl does not revoke blob URLs.
Please call URL.revokeObjectURL on the removed image's URL in removeImage to release memory.
Suggested change
| removeImage(index) { | |
| this.stagedImagesName.splice(index, 1); | |
| this.stagedImagesUrl.splice(index, 1); | |
| }, | |
| removeImage(index) { | |
| // Revoke the blob URL to release memory | |
| const url = this.stagedImagesUrl[index]; | |
| if (url) { | |
| URL.revokeObjectURL(url); | |
| } | |
| this.stagedImagesName.splice(index, 1); | |
| this.stagedImagesUrl.splice(index, 1); | |
| }, |
Soulter
commented
May 29, 2025
dashboard/src/views/ChatPage.vue
Outdated
| responseType: 'blob', | ||
| headers: { | ||
| 'Authorization': 'Bearer ' + localStorage.getItem('token') | ||
| } |
There was a problem hiding this comment.
Co-authored-by: anka-afk <[email protected]>
Raven95676
approved these changes
May 29, 2025
2 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
I have fixed a potential vulnerability in the
/api/chat/get_fileendpoint that could allow unauthorized access to files by ensuring the request has a jwt token.Fixes #1675
Motivation
The previous implementation did not enforce proper authentication for the /api/chat/get_file endpoint, which could allow unauthorized users to access sensitive files. This posed a security risk and needed to be addressed.
Modifications
Check
requirements.txt和pyproject.toml文件相应位置。好的，这是将 pull request 总结翻译成中文的结果：
Sourcery 总结
使用 JWT 认证和路径遍历检查来保护
/api/chat/get_file端点，并改进前端媒体处理，以使用具有适当生命周期管理的缓存 blob URL。新特性：
Bug 修复：
/api/chat/get_file需要有效的 JWT，并将其从未经身份验证的端点列表中删除/api/chat/get_file端点中的 filename 参数来防止路径遍历增强功能：
Original summary in English
Summary by Sourcery
Secure the
/api/chat/get_fileendpoint with JWT authentication and path traversal checks, and revamp the frontend media handling to use cached blob URLs with proper lifecycle management.New Features:
Bug Fixes:
/api/chat/get_fileand remove it from the unauthenticated endpoints list/api/chat/get_fileendpointEnhancements: