fix(filesystem): resolve leading '/' paths relative to workspace in ROOTED mode

[mroldx]

AgentScope-Java Version

2.0.0-SNAPSHOT

Description

Fix #2046

In ROOTED mode, paths starting with "/" represent logical workspace-absolute paths (e.g. "/skills" means "/skills"), not host-machine absolute paths. This commit modifies resolveRooted() to strip the leading "/" and resolve relative to cwd, similar to resolveSandboxed().

Root cause:

• On Windows: Path.of("/skills").isAbsolute() returns false (no drive letter), causing cwd.resolve("/skills") to resolve to :\skills which doesn't exist.
• On Unix: Path.of("/skills").isAbsolute() returns true, triggering SecurityException.

Changes:

• Modified resolveRooted() to handle "/" prefixed paths as workspace-relative
• Added path traversal protection for "/../" patterns
• Added 5 regression tests for the fix

Checklist

Please check the following items before code is ready to be reviewed.

• Code has been formatted with mvn spotless:apply
• All tests are passing (mvn test)
• Javadoc comments are complete and follow project conventions
• Related documentation has been updated (e.g. links, examples, etc.)
• Code is ready for review

[chickenlj]

Thanks for the fix — the issue is real and the direction is correct. One security concern:

Missing startsWith(cwd) post-normalization check

resolveSandboxed() has a defense-in-depth pattern:

Path full = cwd.resolve(stripped).normalize();
if (!full.startsWith(cwd)) {
    throw new SecurityException("Path " + full + " outside root directory: " + cwd);
}
return full;

The new code in resolveRooted() only does a string-level contains("..") check but skips the post-normalize startsWith(cwd) validation. The string check is coarse — it might false-positive on legitimate names like some..dir, and more importantly it lacks the belt-and-suspenders guarantee that the final resolved path is actually within the workspace.

Please add the startsWith(cwd) check after normalize():

if (effectiveKey.startsWith("/")) {
    String stripped = effectiveKey.substring(1);
    if (stripped.isEmpty()) {
        return cwd;
    }
    if (stripped.contains("..") || stripped.startsWith("~")) {
        throw new SecurityException("Path traversal not allowed: " + effectiveKey);
    }
    Path full = cwd.resolve(stripped).normalize();
    if (!full.startsWith(cwd)) {
        throw new SecurityException(
                "Path " + full + " outside root directory: " + cwd);
    }
    return full;
}

This aligns with how resolveSandboxed() handles the same scenario and provides a final guarantee regardless of edge cases in path normalization.

Other than that, the fix and tests look good.

[mroldx]

Thanks for the fix — the issue is real and the direction is correct. One security concern:

Missing startsWith(cwd) post-normalization check

resolveSandboxed() has a defense-in-depth pattern:

Path full = cwd.resolve(stripped).normalize();
if (!full.startsWith(cwd)) {
    throw new SecurityException("Path " + full + " outside root directory: " + cwd);
}
return full;

The new code in resolveRooted() only does a string-level contains("..") check but skips the post-normalize startsWith(cwd) validation. The string check is coarse — it might false-positive on legitimate names like some..dir, and more importantly it lacks the belt-and-suspenders guarantee that the final resolved path is actually within the workspace.

Please add the startsWith(cwd) check after normalize():

if (effectiveKey.startsWith("/")) {
    String stripped = effectiveKey.substring(1);
    if (stripped.isEmpty()) {
        return cwd;
    }
    if (stripped.contains("..") || stripped.startsWith("~")) {
        throw new SecurityException("Path traversal not allowed: " + effectiveKey);
    }
    Path full = cwd.resolve(stripped).normalize();
    if (!full.startsWith(cwd)) {
        throw new SecurityException(
                "Path " + full + " outside root directory: " + cwd);
    }
    return full;
}

This aligns with how resolveSandboxed() handles the same scenario and provides a final guarantee regardless of edge cases in path normalization.

Other than that, the fix and tests look good.

Fixed. Added the post-normalization startsWith(cwd) guard for ROOTED leading-slash paths, and added a regression test to ensure literal ".." in path segment names is still allowed while traversal remains rejected.

[chickenlj]

Please check the test failures:

[INFO] 
Error:  Failures: 
Error:    LocalFilesystemModeTest.rooted_absolutePathLsNotCorruptedByNamespace:177 ls should find files in the project directory ==> expected: <false> but was: <true>
Error:    LocalFilesystemModeTest.rooted_absolutePathNotCorruptedByNamespace:158 absolute path should resolve correctly, got: File '/tmp/junit-13468257907594244212/Main.java' not found ==> expected: <true> but was: <false>
Error:    LocalFilesystemModeTest.rooted_acceptsAbsolutePathUnderPolicyRoot:70 expected success, got: File '/tmp/junit-17109832293948361478/AGENTS.md' not found ==> expected: <true> but was: <false>
Error:    LocalFilesystemModeTest.rooted_rejectsAbsolutePathOutsideAllRoots:85 Expected java.lang.SecurityException to be thrown, but nothing was thrown.
Error:    LocalFilesystemModeTest.rooted_workspaceItselfIsImplicitlyAllowed:115 expected: <true> but was: <false>
Error:    AtPathExpansionMiddlewareTest.expandsAbsolutePathInsidePolicy:78 expected: <true> but was: <false>
[INFO] 
Error:  Tests run: 615, Failures: 6, Errors: 0, Skipped: 0

[codecov]

Codecov Report

❌ Patch coverage is 50.00000% with 8 lines in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
...arness/agent/filesystem/local/LocalFilesystem.java	50.00%	4 Missing and 4 partials ⚠️

📢 Thoughts on this report? Let us know!