fix(tool-guard): resolve relative file paths against the workspace root, not process CWD (#494)

This commit is contained in:
matevip 2026-07-07 18:33:16 +08:00
parent 6802fc1c6a
commit c5805016a3
3 changed files with 62 additions and 4 deletions

View File

@ -204,14 +204,20 @@ public final class WorkspacePathGuard {
* transition window.
*/
public static Path validatePath(String rawPath, @Nullable ToolContext ctx) {
Path normalized = Paths.get(rawPath).toAbsolutePath().normalize();
String basePath = resolveBasePath(ctx);
if (basePath == null || basePath.isBlank()) {
return normalized; // 未配置活动目录不限制
// 未配置活动目录不限制此时相对路径仍按进程 CWD 解析遗留行为
return Paths.get(rawPath).toAbsolutePath().normalize();
}
Path root = Paths.get(basePath).toAbsolutePath().normalize();
// A relative path means "relative to the agent's workspace root", not
// the JVM's launch directory. Resolving against process CWD (via
// toAbsolutePath) sent a plain "./foo.html" outside the sandbox whenever
// the server ran from a directory other than the workspace, tripping a
// spurious "工作区越界" block (issue #494). This matches the shell
// scanner, which already resolves relative tokens against root.
Path normalized = resolveAgainstRoot(rawPath, root);
// 先用 normalize 检查再尝试 toRealPath 防符号链接逃逸
if (!normalized.startsWith(root) && !isExempt(normalized)) {
@ -333,13 +339,29 @@ public final class WorkspacePathGuard {
if (rawPath == null || rawPath.isBlank()) return null;
Path root = basePathToRoot(basePath);
if (root == null) return null;
Path normalized = Paths.get(rawPath).toAbsolutePath().normalize();
// Relative paths resolve against the workspace root (see validatePath /
// issue #494), so a plain "./foo.html" stays inside the sandbox
// regardless of the server's launch directory.
Path normalized = resolveAgainstRoot(rawPath, root);
if (!normalized.startsWith(root) && !isExempt(normalized)) {
return "Path is outside workspace boundary: " + normalized + ", allowed root: " + root;
}
return null;
}
/**
* Resolve a user-supplied path against the workspace {@code root}: absolute
* paths are taken as-is, relative paths (including {@code ./foo} and
* {@code ../foo}) are resolved against {@code root} and normalized. A
* traversal that climbs out of the workspace still normalizes to a path
* that fails the {@code startsWith(root)} check, so this only fixes the
* legitimate in-workspace relative case it does not weaken the boundary.
*/
private static Path resolveAgainstRoot(String rawPath, Path root) {
Path p = Paths.get(rawPath);
return (p.isAbsolute() ? p : root.resolve(p)).normalize();
}
/**
* Resolve a base-path string to a normalized root, falling back to the
* global sandbox root when blank. {@code null} only when neither is set.

View File

@ -87,6 +87,20 @@ class WorkspacePathGuardSandboxTest {
WorkspacePathGuard.validatePath(DEFAULT_ROOT + "/notes/new-file.txt"));
}
@Test
@DisplayName("validatePath: a relative path resolves into the default root, not the process CWD (issue #494)")
void validatePathRelative_resolvesIntoRoot() {
// A plain relative path must land inside the sandbox and return a
// path rooted there not one resolved against the JVM launch dir.
java.nio.file.Path resolved = WorkspacePathGuard.validatePath("./report.html");
org.junit.jupiter.api.Assertions.assertTrue(
resolved.startsWith(java.nio.file.Paths.get(DEFAULT_ROOT)),
"relative path should resolve inside the root, got: " + resolved);
// A relative traversal that climbs out is still rejected.
assertThrows(IllegalArgumentException.class, () ->
WorkspacePathGuard.validatePath("../escape.txt"));
}
@Test
@DisplayName("Per-conversation workspace still takes precedence over the default root")
void conversationWorkspace_wins() {

View File

@ -144,6 +144,28 @@ class WorkspaceBoundaryGuardianTest {
assertTrue(guardian.evaluate(write(WORKSPACE + "/notes.txt", WORKSPACE)).isEmpty());
}
@Test
@DisplayName("write_file with a relative path resolves against the workspace, not the process CWD (issue #494)")
void writeRelativePath_pass() {
// Regression for #494: a plain relative path like "./foo.html" was
// resolved against the JVM launch directory via toAbsolutePath(), so it
// fell outside the workspace whenever the server ran from elsewhere and
// tripped a spurious CRITICAL "工作区越界" block. It must resolve into
// the workspace and pass.
assertTrue(guardian.evaluate(write("./fiber-signal-architecture.html", WORKSPACE)).isEmpty());
assertTrue(guardian.evaluate(write("notes/todo.md", WORKSPACE)).isEmpty());
assertTrue(guardian.evaluate(write("report.txt", WORKSPACE)).isEmpty());
}
@Test
@DisplayName("write_file with a relative traversal that climbs out is still blocked")
void writeRelativeTraversal_blocked() {
// The fix must not weaken the boundary: "../escape.txt" normalizes to a
// path outside the workspace root and stays blocked.
assertBlocked(guardian.evaluate(write("../escape.txt", WORKSPACE)));
assertBlocked(guardian.evaluate(write("../../etc/evil.conf", WORKSPACE)));
}
// ==================== Default-root fallback & escape hatch ====================
@Test