diff --git a/agentscope-harness/src/main/java/io/agentscope/harness/agent/filesystem/AbstractFilesystem.java b/agentscope-harness/src/main/java/io/agentscope/harness/agent/filesystem/AbstractFilesystem.java index 5f869b374f..881c0684b0 100644 --- a/agentscope-harness/src/main/java/io/agentscope/harness/agent/filesystem/AbstractFilesystem.java +++ b/agentscope-harness/src/main/java/io/agentscope/harness/agent/filesystem/AbstractFilesystem.java @@ -179,8 +179,11 @@ static void validatePath(String path) { if (path == null || path.isBlank()) { throw new IllegalArgumentException("Path must not be null or blank"); } - if (path.contains("..")) { - throw new IllegalArgumentException("Path traversal ('..') not allowed: " + path); + String portable = path.replace('\\', '/'); + for (String segment : portable.split("/")) { + if ("..".equals(segment)) { + throw new IllegalArgumentException("Path traversal ('..') not allowed: " + path); + } } } } diff --git a/agentscope-harness/src/main/java/io/agentscope/harness/agent/filesystem/local/LocalFilesystem.java b/agentscope-harness/src/main/java/io/agentscope/harness/agent/filesystem/local/LocalFilesystem.java index bc9e50c52c..606970d173 100644 --- a/agentscope-harness/src/main/java/io/agentscope/harness/agent/filesystem/local/LocalFilesystem.java +++ b/agentscope-harness/src/main/java/io/agentscope/harness/agent/filesystem/local/LocalFilesystem.java @@ -489,7 +489,7 @@ public List uploadFiles( responses.add(FileUploadResponse.success(filePath)); } catch (IOException e) { responses.add(FileUploadResponse.fail(filePath, e.getMessage())); - } catch (SecurityException e) { + } catch (SecurityException | IllegalArgumentException e) { responses.add(FileUploadResponse.fail(filePath, "permission_denied")); } } @@ -628,6 +628,7 @@ private static String stripWindowsDrive(String key) { } private Path resolveRooted(String effectiveKey) { + AbstractFilesystem.validatePath(effectiveKey); Path target = Path.of(effectiveKey); if (target.isAbsolute()) { Path normalized = target.normalize(); @@ -657,7 +658,11 @@ private Path resolveRooted(String effectiveKey) { return full; } - return cwd.resolve(target).normalize(); + Path full = cwd.resolve(target).normalize(); + if (!full.startsWith(cwd)) { + throw new SecurityException("Path " + full + " outside root directory: " + cwd); + } + return full; } private SecurityException rootAccessDenied(Path normalized) { diff --git a/agentscope-harness/src/main/java/io/agentscope/harness/agent/workspace/WorkspaceManager.java b/agentscope-harness/src/main/java/io/agentscope/harness/agent/workspace/WorkspaceManager.java index c40a47fdfe..1a1f4818d7 100644 --- a/agentscope-harness/src/main/java/io/agentscope/harness/agent/workspace/WorkspaceManager.java +++ b/agentscope-harness/src/main/java/io/agentscope/harness/agent/workspace/WorkspaceManager.java @@ -369,7 +369,7 @@ public void appendUtf8WorkspaceRelative( if (relativePath == null || content == null) { return; } - String normalized = normalizeRelativePath(relativePath); + String normalized = requireSafeRelativePath(relativePath); if (normalized.isEmpty()) { return; } @@ -706,7 +706,7 @@ private ObjectNode ensureSessionsObject(ObjectNode root) { } private String readWritableWorkspaceRelativeUtf8(RuntimeContext rc, String relativePath) { - String normalized = normalizeRelativePath(relativePath); + String normalized = requireSafeRelativePath(relativePath); if (normalized.isEmpty()) { return ""; } @@ -718,7 +718,7 @@ public void writeUtf8WorkspaceRelative(RuntimeContext rc, String relativePath, S if (relativePath == null || content == null) { return; } - String normalized = normalizeRelativePath(relativePath); + String normalized = requireSafeRelativePath(relativePath); if (normalized.isEmpty()) { return; } @@ -771,7 +771,7 @@ public void writeDraftSkillFile(RuntimeContext rc, String relativePath, String c if (relativePath == null || content == null) { return; } - String normalized = normalizeRelativePath(relativePath); + String normalized = requireSafeRelativePath(relativePath); if (normalized.isEmpty()) { return; } @@ -798,8 +798,8 @@ public boolean moveSkill(RuntimeContext rc, String fromRelative, String toRelati if (fromRelative == null || toRelative == null || filesystem == null) { return false; } - String src = normalizeRelativePath(fromRelative); - String dst = normalizeRelativePath(toRelative); + String src = requireSafeRelativePath(fromRelative); + String dst = requireSafeRelativePath(toRelative); if (src.isEmpty() || dst.isEmpty()) { return false; } @@ -929,6 +929,14 @@ static String normalizeRelativePath(String relativePath) { return s; } + private static String requireSafeRelativePath(String relativePath) { + String normalized = normalizeRelativePath(relativePath); + if (!normalized.isEmpty()) { + AbstractFilesystem.validatePath(normalized); + } + return normalized; + } + /** * Returns workspace-relative paths of all memory files ({@code MEMORY.md} and {@code * memory/*.md}). Unions results from the {@link AbstractFilesystem} layer and the local disk, diff --git a/agentscope-harness/src/test/java/io/agentscope/harness/agent/filesystem/LocalFilesystemModeTest.java b/agentscope-harness/src/test/java/io/agentscope/harness/agent/filesystem/LocalFilesystemModeTest.java index 56ebef91c5..695df37ce9 100644 --- a/agentscope-harness/src/test/java/io/agentscope/harness/agent/filesystem/LocalFilesystemModeTest.java +++ b/agentscope-harness/src/test/java/io/agentscope/harness/agent/filesystem/LocalFilesystemModeTest.java @@ -18,6 +18,7 @@ import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; import io.agentscope.core.agent.RuntimeContext; @@ -32,6 +33,7 @@ import java.nio.file.Files; import java.nio.file.Path; import java.util.List; +import java.util.Map; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.io.TempDir; @@ -285,8 +287,81 @@ void rooted_leadingSlashPathTraversalRejected(@TempDir Path workspace) { new LocalFilesystem(workspace, LocalFsMode.ROOTED, PathPolicy.empty(), 10, null); // Path traversal with leading "/" should be rejected - org.junit.jupiter.api.Assertions.assertThrows( - SecurityException.class, + assertThrows( + IllegalArgumentException.class, () -> fs.read(RuntimeContext.empty(), "/../etc/passwd", 0, 0)); } + + @Test + void rooted_relativePathTraversalRejected(@TempDir Path base) throws IOException { + Path workspace = base.resolve("workspace"); + Files.createDirectories(workspace); + Path outsideFile = base.resolve("secret.txt"); + Files.writeString(outsideFile, "secret", StandardCharsets.UTF_8); + LocalFilesystem fs = + new LocalFilesystem(workspace, LocalFsMode.ROOTED, PathPolicy.empty(), 10, null); + + assertThrows( + IllegalArgumentException.class, + () -> fs.read(RuntimeContext.empty(), "../secret.txt", 0, 0)); + } + + @Test + void rooted_relativeUploadTraversalRejected(@TempDir Path base) throws IOException { + Path workspace = base.resolve("workspace"); + Files.createDirectories(workspace); + Path escaped = base.resolve("escape.txt"); + LocalFilesystem fs = + new LocalFilesystem(workspace, LocalFsMode.ROOTED, PathPolicy.empty(), 10, null); + + var responses = + fs.uploadFiles( + RuntimeContext.empty(), + List.of( + Map.entry( + "../escape.txt", + "escape".getBytes(StandardCharsets.UTF_8)))); + + assertEquals(1, responses.size()); + assertFalse(responses.get(0).isSuccess()); + assertEquals("permission_denied", responses.get(0).error()); + assertFalse(Files.exists(escaped)); + } + + @Test + void rooted_relativeTraversalCannotEscapeUserNamespace(@TempDir Path workspace) + throws IOException { + Files.createDirectories(workspace.resolve("user-1")); + Files.createDirectories(workspace.resolve("user-2")); + LocalFilesystem fs = + new LocalFilesystem(workspace, LocalFsMode.ROOTED, PathPolicy.empty(), 10, USER_NS); + RuntimeContext rc = RuntimeContext.builder().userId("user-1").build(); + + var responses = + fs.uploadFiles( + rc, + List.of( + Map.entry( + "../user-2/escape.txt", + "escape".getBytes(StandardCharsets.UTF_8)))); + + assertEquals(1, responses.size()); + assertFalse(responses.get(0).isSuccess()); + assertFalse(Files.exists(workspace.resolve("user-2/escape.txt"))); + } + + @Test + void rooted_literalDoubleDotInRelativeSegmentAllowed(@TempDir Path workspace) + throws IOException { + Path file = workspace.resolve("some..dir/note.txt"); + Files.createDirectories(file.getParent()); + Files.writeString(file, "literal name", StandardCharsets.UTF_8); + LocalFilesystem fs = + new LocalFilesystem(workspace, LocalFsMode.ROOTED, PathPolicy.empty(), 10, null); + + ReadResult result = fs.read(RuntimeContext.empty(), "some..dir/note.txt", 0, 0); + + assertTrue(result.isSuccess()); + assertEquals("literal name", result.fileData().content()); + } } diff --git a/agentscope-harness/src/test/java/io/agentscope/harness/agent/workspace/WorkspaceManagerPathSafetyTest.java b/agentscope-harness/src/test/java/io/agentscope/harness/agent/workspace/WorkspaceManagerPathSafetyTest.java new file mode 100644 index 0000000000..06dcdd73c2 --- /dev/null +++ b/agentscope-harness/src/test/java/io/agentscope/harness/agent/workspace/WorkspaceManagerPathSafetyTest.java @@ -0,0 +1,92 @@ +/* + * Copyright 2024-2026 the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package io.agentscope.harness.agent.workspace; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertThrows; + +import io.agentscope.core.agent.RuntimeContext; +import io.agentscope.harness.agent.filesystem.local.LocalFilesystem; +import java.nio.charset.StandardCharsets; +import java.nio.file.Files; +import java.nio.file.Path; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.io.TempDir; + +class WorkspaceManagerPathSafetyTest { + + @Test + void workspaceWritesRejectRelativeTraversal(@TempDir Path root) throws Exception { + Path base = root.resolve("base"); + Path template = base.resolve("template"); + Path backend = base.resolve("backend"); + Files.createDirectories(template); + Files.createDirectories(backend); + RuntimeContext rc = RuntimeContext.empty(); + + try (WorkspaceManager manager = + new WorkspaceManager( + template, + new LocalFilesystem( + backend, LocalFsMode.ROOTED, PathPolicy.empty(), 10, null))) { + assertThrows( + IllegalArgumentException.class, + () -> manager.writeUtf8WorkspaceRelative(rc, "../escape.txt", "ESCAPE_WRITE")); + assertThrows( + IllegalArgumentException.class, + () -> + manager.appendUtf8WorkspaceRelative( + rc, "../../append-escape.txt", "ESCAPE_APPEND")); + assertThrows( + IllegalArgumentException.class, + () -> + manager.writeUtf8WorkspaceRelative( + rc, "..\\windows-escape.txt", "ESCAPE_WINDOWS")); + } + + assertFalse(Files.exists(base.resolve("escape.txt"))); + assertFalse(Files.exists(root.resolve("append-escape.txt"))); + assertFalse(Files.exists(base.resolve("windows-escape.txt"))); + } + + @Test + void workspaceWritesAllowLogicalAbsoluteAndLiteralDoubleDotNames(@TempDir Path root) + throws Exception { + Path template = root.resolve("template"); + Path backend = root.resolve("backend"); + Files.createDirectories(template); + Files.createDirectories(backend); + + try (WorkspaceManager manager = + new WorkspaceManager( + template, + new LocalFilesystem( + backend, LocalFsMode.ROOTED, PathPolicy.empty(), 10, null))) { + manager.writeUtf8WorkspaceRelative( + RuntimeContext.empty(), "/absolute-inside.txt", "ABSOLUTE"); + manager.writeUtf8WorkspaceRelative( + RuntimeContext.empty(), "some..dir/note.txt", "LITERAL"); + } + + assertEquals( + "ABSOLUTE", + Files.readString(backend.resolve("absolute-inside.txt"), StandardCharsets.UTF_8)); + assertEquals( + "LITERAL", + Files.readString(backend.resolve("some..dir/note.txt"), StandardCharsets.UTF_8)); + } +}