fix(workspace): harden chat-upload base-path containment + cleanup guard

- resolveAgentBasePath: the relative-override branch now normalizes the
  resolved path and rejects values that escape the workspace root via "../"
  (the absolute branch already did this), keeping attachment/media/tool I/O
  contained when an agent's workspaceBasePath is a relative override.
- cleanAttachmentFiles: return early on a null/blank conversationId so a bare
  upload root can never be walked and deleted wholesale.
- Translate the chat-upload Javadoc/comments to English (cleanAttachmentFiles,
  BaseAgent image-path resolver) per code style.
- Add a resolver test for the relative-override escape fallback.
This commit is contained in:
matevip 2026-06-26 14:25:34 +08:00
parent fcb488c567
commit 26dd8a37d5
4 changed files with 43 additions and 9 deletions

View File

@ -1347,6 +1347,15 @@ public class AgentGraphBuilder {
return agentOverride; return agentOverride;
} }
if (hasWorkspace) { if (hasWorkspace) {
// Relative override resolves under the workspace root; reject any value
// that escapes it via "../" so attachment/media/tool I/O stays contained.
Path wsRoot = Paths.get(workspaceBase).toAbsolutePath().normalize();
Path resolved = wsRoot.resolve(agentOverride).normalize();
if (!resolved.startsWith(wsRoot)) {
throw new IllegalArgumentException(
"Agent workspaceBasePath override must stay inside the workspace root: "
+ resolved + " escapes " + wsRoot);
}
return Paths.get(workspaceBase).resolve(agentOverride).toString(); return Paths.get(workspaceBase).resolve(agentOverride).toString();
} }
return agentOverride; return agentOverride;

View File

@ -1218,12 +1218,14 @@ public abstract class BaseAgent {
} }
/** /**
* 解析图片文件的绝对路径 * Resolve the absolute path of an image file.
* <p> * <p>
* 上传文件的存储位置由 {@code ChatUploadLocationResolver} 按优先级解析 * The storage location of uploaded files is resolved by
* Agent workspaceBasePath Workspace basePath 可配置默认目录 * {@code ChatUploadLocationResolver} in priority order: the Agent's
* {@code mateclaw.chat.upload.base-dir}默认 {@code data/chat-uploads} * workspaceBasePath the Workspace's basePath a configurable default
* MCP 工具的工作目录可能不同所以这里直接解析为绝对路径 * directory ({@code mateclaw.chat.upload.base-dir}, default
* {@code data/chat-uploads}). An MCP tool's working directory may differ,
* so this resolves directly to an absolute path.
*/ */
/** /**
* 构建当前用户消息的 UserMessage multimodal 图片注入 * 构建当前用户消息的 UserMessage multimodal 图片注入

View File

@ -1665,13 +1665,19 @@ public class ConversationService {
} }
/** /**
* 清理会话关联的附件文件 * Clean up the attachment files associated with a conversation.
* <p> * <p>
* 遍历所有候选上传根workspace/agent 感知根 + 默认根逐一删除 * Walks every candidate upload root (the workspace/agent-aware root plus the
* 各根下该会话的附件目录这样无论附件落在新的工作空间目录还是 * default root) and deletes that conversation's attachment directory under
* 迁移前的默认目录都能被清理 * each, so attachments are removed whether they landed in the new workspace
* directory or the pre-migration default directory.
*/ */
public void cleanAttachmentFiles(String conversationId) { public void cleanAttachmentFiles(String conversationId) {
if (conversationId == null || conversationId.isBlank()) {
// A blank id would resolve to the upload root itself and wipe every
// conversation's attachments never walk/delete a bare root.
return;
}
boolean cleanedAny = false; boolean cleanedAny = false;
for (Path root : chatUploadLocationResolver.resolveCandidateUploadRoots(conversationId)) { for (Path root : chatUploadLocationResolver.resolveCandidateUploadRoots(conversationId)) {
Path dir; Path dir;

View File

@ -143,6 +143,23 @@ class ChatUploadLocationResolverTest {
.resolve(ChatUploadLocationResolver.UPLOAD_SUBDIR)); .resolve(ChatUploadLocationResolver.UPLOAD_SUBDIR));
} }
@Test
@DisplayName("relative agent override that escapes the workspace root via ../ falls back to workspace basePath")
void relativeAgentOverrideEscapingWorkspaceFallsBackToWorkspace() {
stubConversation("c5b", 7L, 99L);
Path wsBase = tempDir.resolve("ws-root");
when(workspaceService.getById(7L)).thenReturn(workspace(7L, wsBase.toString()));
// Relative override climbs out of the workspace root resolveAgentBasePath
// rejects it; the resolver falls back to the workspace basePath.
when(agentService.getAgent(99L)).thenReturn(agent(99L, "../../escape", 7L));
ChatUploadLocationResolver r = resolver(tempDir);
Path root = r.resolveUploadRoot("c5b");
assertThat(root).isEqualTo(wsBase.toAbsolutePath().normalize()
.resolve(ChatUploadLocationResolver.UPLOAD_SUBDIR));
}
@Test @Test
@DisplayName("candidate roots: workspace-scoped first, then default (dual-lookup order)") @DisplayName("candidate roots: workspace-scoped first, then default (dual-lookup order)")
void candidateRootsOrderedScopedThenDefault() { void candidateRootsOrderedScopedThenDefault() {