背景
PR #422(chat-uploads 上传目录工作空间/Agent 感知化,已 merged)整体质量很高——安全模型扎实(owner 校验 + startsWith 防穿越 + tool 侧 getFileName() 取 basename)、向后兼容周到(双重查找 + 默认目录零行为变化)。本 issue 跟踪 review 中发现的一个回归和几个边角问题。
🟡 应修:upload 响应的 path 现在泄漏服务端绝对路径
ChatController.upload(约 L1107):
// 使用相对路径,避免暴露服务端绝对路径
response.setPath(uploadRoot.resolve(conversationId).resolve(storedName).toString());
注释声称"用相对路径避免暴露绝对路径",但本 PR 后 uploadRoot 来自 resolveUploadRoot,它永远返回绝对路径:
ChatUploadLocationResolver.defaultRoot() = Paths.get(baseDir).toAbsolutePath().normalize();
ChatUploadAutoConfiguration 进一步把 properties.baseDir 重写成了绝对路径(properties.setBaseDir(root.toString()),其中 root 已 toAbsolutePath());
- workspace/agent-scoped root 也
toAbsolutePath().normalize()。
后果
- 泄漏:
path 字段(返回给前端,并按 PR 描述会进入 MessageContentPart)现在携带形如 /abs/path/to/data/chat-uploads/... 的服务端绝对路径——正是这行注释想避免的。
- 可移植性:改部署目录后,历史消息里存的绝对路径失效(相对路径本来是可移植的)。
- PR 描述失真:PR 说"不改
MessageContentPart 的 path 语义",但 path 的取值从相对(改前 data/chat-uploads/...)变成了绝对,语义实际变了。
建议
path 字段存相对形式(如 defaultRoot().relativize(target),或直接拼 chat-uploads/{convId}/{storedName}),与服务端点 URL 契约 /api/v1/chat/files/{convId}/{storedName} 对齐。
注意:cleanAttachmentFiles 走的是候选根 + convId、不依赖该字段,所以改它不影响清理(已确认)。
🟢 边角 / 可跟进
1. tool 读取与写入用了两套"作用域来源"
- 下载器 / 上传 / serving 都按会话存储的 agentId 解析(
resolveUploadRoot(conversationId) → 缓存的 conversation → stored agentId)。
- 而静态
ChatUploadResolver(ReadFileTool / DocumentExtractTool)按请求态 agent 解析(ToolExecutionContext.workspaceBasePath())。
若同一会话被两个 workspaceBasePath override 不同的 agent 使用,文件写在 stored-agent 根下,而 tool 侧候选只有 [request-agent-scoped, default],会找不到。很窄(需要 per-agent override + 同会话换 agent;默认情况两者都落 default 所以重合),但两条路径从不同来源推导作用域,比较脆。建议加注释说明或统一到一个来源。
2. 会话创建前的首次上传永远落 default 根
/upload 端点按 convId 解析,会话行还不存在时回退 default——即使该 workspace 配了 basePath。读取靠候选根含 default 仍能命中,功能没问题,只是没被 workspace 隔离。该端点签名也没有 X-Workspace-Id(不像 /stream),否则可用 resolveUploadRoot(workspaceId, agentId) 重载让首传就归位。
3. 缓存只覆盖了一半热路径
- 只缓存了 conversation 行(5min);每次 resolve 仍调
workspaceService.getById + agentService.getAgent。若这两者没有各自的缓存,"避免热路径查库"只兑现了一半。
- 未知 convId 的查询不被 Caffeine 缓存(loader 返回 null 不写入),像
cron:<id> 这种会话每次 resolve 都打库。
均属小优化,非阻塞。
关联
背景
PR #422(chat-uploads 上传目录工作空间/Agent 感知化,已 merged)整体质量很高——安全模型扎实(owner 校验 +
startsWith防穿越 + tool 侧getFileName()取 basename)、向后兼容周到(双重查找 + 默认目录零行为变化)。本 issue 跟踪 review 中发现的一个回归和几个边角问题。🟡 应修:upload 响应的
path现在泄漏服务端绝对路径ChatController.upload(约 L1107):注释声称"用相对路径避免暴露绝对路径",但本 PR 后
uploadRoot来自resolveUploadRoot,它永远返回绝对路径:ChatUploadLocationResolver.defaultRoot()=Paths.get(baseDir).toAbsolutePath().normalize();ChatUploadAutoConfiguration进一步把properties.baseDir重写成了绝对路径(properties.setBaseDir(root.toString()),其中root已toAbsolutePath());toAbsolutePath().normalize()。后果
path字段(返回给前端,并按 PR 描述会进入MessageContentPart)现在携带形如/abs/path/to/data/chat-uploads/...的服务端绝对路径——正是这行注释想避免的。MessageContentPart的path语义",但path的取值从相对(改前data/chat-uploads/...)变成了绝对,语义实际变了。建议
path字段存相对形式(如defaultRoot().relativize(target),或直接拼chat-uploads/{convId}/{storedName}),与服务端点 URL 契约/api/v1/chat/files/{convId}/{storedName}对齐。注意:
cleanAttachmentFiles走的是候选根 + convId、不依赖该字段,所以改它不影响清理(已确认)。🟢 边角 / 可跟进
1. tool 读取与写入用了两套"作用域来源"
resolveUploadRoot(conversationId)→ 缓存的 conversation → stored agentId)。ChatUploadResolver(ReadFileTool/DocumentExtractTool)按请求态 agent 解析(ToolExecutionContext.workspaceBasePath())。若同一会话被两个
workspaceBasePathoverride 不同的 agent 使用,文件写在 stored-agent 根下,而 tool 侧候选只有[request-agent-scoped, default],会找不到。很窄(需要 per-agent override + 同会话换 agent;默认情况两者都落 default 所以重合),但两条路径从不同来源推导作用域,比较脆。建议加注释说明或统一到一个来源。2. 会话创建前的首次上传永远落 default 根
/upload端点按 convId 解析,会话行还不存在时回退 default——即使该 workspace 配了 basePath。读取靠候选根含 default 仍能命中,功能没问题,只是没被 workspace 隔离。该端点签名也没有X-Workspace-Id(不像/stream),否则可用resolveUploadRoot(workspaceId, agentId)重载让首传就归位。3. 缓存只覆盖了一半热路径
workspaceService.getById+agentService.getAgent。若这两者没有各自的缓存,"避免热路径查库"只兑现了一半。cron:<id>这种会话每次 resolve 都打库。均属小优化,非阻塞。
关联