Precedence: *
Matches: + *
Does NOT match {@code abc..xyz} (no slash before/after the {@code ..} — + * not a path segment) or absolute {@code /foo/../bar} (handled by + * {@link #ABS_PATH_TOKEN}). The token must be bounded by a shell separator + * or end-of-string on both sides. + */ + private static final Pattern RELATIVE_TRAVERSAL_TOKEN = Pattern.compile( + "(?:^|[\\s|&;<>(`\"'={}])((?:[^\\s|&;<>()\"'`{}=/]+/)*\\.\\.(?:/[^\\s|&;<>()\"'`{}=]*)?)(?=[\\s|&;<>)`\"'=}]|$)"); + /** Bare tilde or tilde at the start of a path token: {@code ~}, {@code ~/foo}, {@code "~/bar"}. */ private static final Pattern TILDE_REF = Pattern.compile( "(?:^|[\\s|&;<>(`\"'={}])~(?=[/\\s|&;<>)`\"'$]|$)"); diff --git a/mateclaw-server/src/test/java/vip/mate/agent/AgentGraphBuilderBasePathResolutionTest.java b/mateclaw-server/src/test/java/vip/mate/agent/AgentGraphBuilderBasePathResolutionTest.java index 01321c23..5af64342 100644 --- a/mateclaw-server/src/test/java/vip/mate/agent/AgentGraphBuilderBasePathResolutionTest.java +++ b/mateclaw-server/src/test/java/vip/mate/agent/AgentGraphBuilderBasePathResolutionTest.java @@ -39,21 +39,21 @@ class AgentGraphBuilderBasePathResolutionTest { } @Test - @DisplayName("Agent override is absolute → used verbatim, workspace ignored") + @DisplayName("Agent override is absolute and inside workspace → used verbatim") @DisabledOnOs(OS.WINDOWS) - void absoluteOverride_usedAsIs_unix() { - assertEquals("/opt/agents/code-review", - AgentGraphBuilder.resolveAgentBasePath("/opt/agents/code-review", "/srv/ws-root")); + void absoluteOverride_insideWs_usedAsIs_unix() { + assertEquals("/srv/ws-root/agents/code-review", + AgentGraphBuilder.resolveAgentBasePath("/srv/ws-root/agents/code-review", "/srv/ws-root")); assertEquals("/opt/agents/code-review", AgentGraphBuilder.resolveAgentBasePath("/opt/agents/code-review", null)); } @Test - @DisplayName("Agent override is absolute (Windows) → used verbatim") + @DisplayName("Agent override is absolute (Windows) and inside workspace → used verbatim") @EnabledOnOs(OS.WINDOWS) - void absoluteOverride_usedAsIs_windows() { - assertEquals("C:\\agents\\code-review", - AgentGraphBuilder.resolveAgentBasePath("C:\\agents\\code-review", "C:\\ws-root")); + void absoluteOverride_insideWs_usedAsIs_windows() { + assertEquals("C:\\ws-root\\agents\\code-review", + AgentGraphBuilder.resolveAgentBasePath("C:\\ws-root\\agents\\code-review", "C:\\ws-root")); } @Test @@ -79,4 +79,44 @@ class AgentGraphBuilderBasePathResolutionTest { assertEquals("agent-dir", AgentGraphBuilder.resolveAgentBasePath("agent-dir", " ")); } + + // ==================== Absolute override scoped to workspace root ==================== + + @Test + @DisplayName("Absolute override inside workspace root → allowed verbatim") + @DisabledOnOs(OS.WINDOWS) + void absoluteOverride_insideWorkspace_allowed() { + // /srv/ws-root/agents/code-review starts with /srv/ws-root → fine. + assertEquals("/srv/ws-root/agents/code-review", + AgentGraphBuilder.resolveAgentBasePath("/srv/ws-root/agents/code-review", "/srv/ws-root")); + // Identical to workspace root → trivially allowed. + assertEquals("/srv/ws-root", + AgentGraphBuilder.resolveAgentBasePath("/srv/ws-root", "/srv/ws-root")); + } + + @Test + @DisplayName("Absolute override outside workspace root → rejected (workspace-scoping bypass)") + @DisabledOnOs(OS.WINDOWS) + void absoluteOverride_outsideWorkspace_rejected() { + // A less-trusted admin could set / or another repo and bypass scoping — + // reject it so the override stays inside the team workspace. + org.junit.jupiter.api.Assertions.assertThrows(IllegalArgumentException.class, () -> + AgentGraphBuilder.resolveAgentBasePath("/etc", "/srv/ws-root")); + org.junit.jupiter.api.Assertions.assertThrows(IllegalArgumentException.class, () -> + AgentGraphBuilder.resolveAgentBasePath("/", "/srv/ws-root")); + org.junit.jupiter.api.Assertions.assertThrows(IllegalArgumentException.class, () -> + AgentGraphBuilder.resolveAgentBasePath("/Users/admin/other-project", + "/srv/ws-root")); + } + + @Test + @DisplayName("Absolute override with no workspace basePath → used verbatim (legacy)") + @DisabledOnOs(OS.WINDOWS) + void absoluteOverride_noWorkspace_allowed() { + // No workspace boundary to enforce — fall back to legacy behavior. + assertEquals("/Users/admin/scratch", + AgentGraphBuilder.resolveAgentBasePath("/Users/admin/scratch", null)); + assertEquals("/Users/admin/scratch", + AgentGraphBuilder.resolveAgentBasePath("/Users/admin/scratch", "")); + } } diff --git a/mateclaw-server/src/test/java/vip/mate/tool/guard/WorkspacePathGuardShellTest.java b/mateclaw-server/src/test/java/vip/mate/tool/guard/WorkspacePathGuardShellTest.java index 1f8139e4..7b694b2e 100644 --- a/mateclaw-server/src/test/java/vip/mate/tool/guard/WorkspacePathGuardShellTest.java +++ b/mateclaw-server/src/test/java/vip/mate/tool/guard/WorkspacePathGuardShellTest.java @@ -205,6 +205,65 @@ class WorkspacePathGuardShellTest { WorkspacePathGuard.validateShellCommand("read line < /dev/fd/3")); } + // ==================== Relative parent-directory traversal ==================== + + @Test + @DisplayName("Bare `cd ..` → rejected") + void cdDotDot_blocked() { + assertThrows(IllegalArgumentException.class, () -> + WorkspacePathGuard.validateShellCommand("cd .. && ls")); + assertThrows(IllegalArgumentException.class, () -> + WorkspacePathGuard.validateShellCommand("cd ..")); + } + + @Test + @DisplayName("Relative parent traversal `../foo` → rejected") + void relativeParentTraversal_blocked() { + assertThrows(IllegalArgumentException.class, () -> + WorkspacePathGuard.validateShellCommand("cat ../mateclaw/CLAUDE.md")); + assertThrows(IllegalArgumentException.class, () -> + WorkspacePathGuard.validateShellCommand("head -3 ../README.md")); + } + + @Test + @DisplayName("Symlink creation with relative outside-pointing target → rejected") + void relativeSymlinkEscape_blocked() { + assertThrows(IllegalArgumentException.class, () -> + WorkspacePathGuard.validateShellCommand("ln -sf ../mateclaw breakout")); + assertThrows(IllegalArgumentException.class, () -> + WorkspacePathGuard.validateShellCommand("ln -s ../../etc shortcut")); + } + + @Test + @DisplayName("Deeper relative traversal `foo/../../bar` → rejected") + void deepRelativeTraversal_blocked() { + assertThrows(IllegalArgumentException.class, () -> + WorkspacePathGuard.validateShellCommand("cat subdir/../../other/file.txt")); + } + + @Test + @DisplayName("In-workspace `..` traversal that normalizes back inside → allowed") + void inWorkspaceTraversal_pass() { + // subdir/../sibling → workspace/sibling, still inside. + assertDoesNotThrow(() -> + WorkspacePathGuard.validateShellCommand("cat subdir/../sibling.txt")); + // ./.. is at workspace root after normalize — still inside? No: ./.. is parent of cwd. + // We do want to reject that, which the regex will catch as escape. + assertDoesNotThrow(() -> + WorkspacePathGuard.validateShellCommand("ls foo/..")); // resolves to workspace root + } + + @Test + @DisplayName("Identifier with double-dot but no slash (e.g. `abc..xyz`) is not a path → allowed") + void doubleDotInIdentifier_pass() { + // "..foo" / "abc..xyz" should not be confused with parent traversal. + // These appear in version strings, env var values, etc. + assertDoesNotThrow(() -> + WorkspacePathGuard.validateShellCommand("echo version=1.2..3")); + } + + // ==================== Device-node negative cases ==================== + @Test @DisplayName("Non-allowlisted /dev/* paths still rejected") void devOther_blocked() {