From ac32679316b83d60252923fed3c2d1fb983bd38d Mon Sep 17 00:00:00 2001 From: Amit Kumar Date: Fri, 24 Apr 2026 17:16:30 +0000 Subject: [PATCH] fix(security): block path traversal via symlinks in /api/file and read_file (RAN-8) Resolve symlinks with `Path.toRealPath()` and re-check the resolved path against the codebase root on both the REST `/api/file` endpoint and the MCP `read_file` tool. `Path.normalize()` is purely lexical and left symlinks inside the indexed repo usable for exfiltrating off-tree files (e.g. `link -> /etc/passwd`). - GraphController: canonicalize root, lexical guard, then toRealPath() and re-check; 404 on NoSuchFileException, 403 on out-of-root. - McpTools: same two-stage guard, returns "Path traversal detected". - Tests: positive (escape symlink rejected) + negative (in-repo symlink read succeeds) for both REST and MCP. Skip gracefully on filesystems without symlink support. Co-Authored-By: Paperclip Co-Authored-By: Claude Opus 4.7 (1M context) --- .../iq/api/GraphController.java | 33 +++++++++++--- .../randomcodespace/iq/mcp/McpTools.java | 12 ++++-- .../iq/api/GraphControllerTest.java | 43 +++++++++++++++++++ .../randomcodespace/iq/mcp/McpToolsTest.java | 43 +++++++++++++++++++ 4 files changed, 123 insertions(+), 8 deletions(-) diff --git a/src/main/java/io/github/randomcodespace/iq/api/GraphController.java b/src/main/java/io/github/randomcodespace/iq/api/GraphController.java index 75f189e2..2cd8c6cc 100644 --- a/src/main/java/io/github/randomcodespace/iq/api/GraphController.java +++ b/src/main/java/io/github/randomcodespace/iq/api/GraphController.java @@ -19,6 +19,7 @@ import java.io.IOException; import java.nio.charset.StandardCharsets; import java.nio.file.Files; +import java.nio.file.NoSuchFileException; import java.nio.file.Path; import java.util.Arrays; import java.util.List; @@ -257,18 +258,40 @@ public ResponseEntity readFile( @RequestParam String path, @RequestParam(required = false) Integer startLine, @RequestParam(required = false) Integer endLine) { - Path codebasePath = Path.of(config.getRootPath()).toAbsolutePath().normalize(); - Path resolved = codebasePath.resolve(path).normalize(); - if (!resolved.startsWith(codebasePath)) { + Path codebaseReal; + try { + codebaseReal = Path.of(config.getRootPath()).toRealPath(); + } catch (IOException e) { + return ResponseEntity.status(500) + .contentType(MediaType.TEXT_PLAIN) + .body("Failed to resolve codebase root: " + e.getMessage()); + } + Path candidate = codebaseReal.resolve(path).normalize(); + if (!candidate.startsWith(codebaseReal)) { + return ResponseEntity.status(403) + .contentType(MediaType.TEXT_PLAIN) + .body("Path traversal blocked"); + } + Path resolvedReal; + try { + resolvedReal = candidate.toRealPath(); + } catch (NoSuchFileException e) { + return ResponseEntity.notFound().build(); + } catch (IOException e) { + return ResponseEntity.status(500) + .contentType(MediaType.TEXT_PLAIN) + .body("Failed to resolve file: " + e.getMessage()); + } + if (!resolvedReal.startsWith(codebaseReal)) { return ResponseEntity.status(403) .contentType(MediaType.TEXT_PLAIN) .body("Path traversal blocked"); } - if (!Files.isRegularFile(resolved)) { + if (!Files.isRegularFile(resolvedReal)) { return ResponseEntity.notFound().build(); } try { - String content = Files.readString(resolved, StandardCharsets.UTF_8); + String content = Files.readString(resolvedReal, StandardCharsets.UTF_8); if (startLine != null || endLine != null) { String[] lines = content.split("\n", -1); int start = (startLine != null ? startLine : 1); diff --git a/src/main/java/io/github/randomcodespace/iq/mcp/McpTools.java b/src/main/java/io/github/randomcodespace/iq/mcp/McpTools.java index 6acfe81b..4089ee62 100644 --- a/src/main/java/io/github/randomcodespace/iq/mcp/McpTools.java +++ b/src/main/java/io/github/randomcodespace/iq/mcp/McpTools.java @@ -385,9 +385,15 @@ public String readFile( @McpToolParam(description = "Start line number, 1-based (optional — omit to read entire file)", required = false) Integer startLine, @McpToolParam(description = "End line number, 1-based inclusive (optional — omit to read to end)", required = false) Integer endLine) { try { - Path root = Path.of(config.getRootPath()).toAbsolutePath().normalize(); - Path resolved = root.resolve(filePath).normalize(); - // Path traversal protection + Path root = Path.of(config.getRootPath()).toRealPath(); + Path candidate = root.resolve(filePath).normalize(); + // Lexical traversal guard (rejects ../ before any filesystem touch) + if (!candidate.startsWith(root)) { + return toJson(Map.of(PROP_ERROR, "Path traversal detected")); + } + // Follow symlinks and re-check so an in-repo symlink pointing outside the + // codebase (e.g. link -> /etc/passwd) cannot be used to exfiltrate files. + Path resolved = candidate.toRealPath(); if (!resolved.startsWith(root)) { return toJson(Map.of(PROP_ERROR, "Path traversal detected")); } diff --git a/src/test/java/io/github/randomcodespace/iq/api/GraphControllerTest.java b/src/test/java/io/github/randomcodespace/iq/api/GraphControllerTest.java index 8455b2ef..b126423d 100644 --- a/src/test/java/io/github/randomcodespace/iq/api/GraphControllerTest.java +++ b/src/test/java/io/github/randomcodespace/iq/api/GraphControllerTest.java @@ -557,6 +557,49 @@ void readFileShouldReturnFullContentWithoutLineParams(@TempDir Path tempDir) thr .andExpect(content().string("aaa\nbbb\nccc")); } + @Test + void readFileShouldRejectSymlinkEscapingRoot(@TempDir Path tempDir) throws Exception { + Path target = Files.createTempFile("codeiq-escape-", ".txt"); + try { + Files.writeString(target, "TOP SECRET", StandardCharsets.UTF_8); + Path link = tempDir.resolve("leak.txt"); + try { + Files.createSymbolicLink(link, target.toAbsolutePath()); + } catch (UnsupportedOperationException | java.io.IOException unsupported) { + // Filesystem does not support symlinks (e.g. Windows without privilege) — skip. + return; + } + CodeIqConfigTestSupport.override(config).rootPath(tempDir.toAbsolutePath().toString()).done(); + var controller = new GraphController(queryService, config); + var fileMvc = MockMvcBuilders.standaloneSetup(controller).build(); + + fileMvc.perform(get("/api/file").param("path", "leak.txt")) + .andExpect(status().isForbidden()) + .andExpect(content().string("Path traversal blocked")); + } finally { + Files.deleteIfExists(target); + } + } + + @Test + void readFileShouldAllowInRepoSymlink(@TempDir Path tempDir) throws Exception { + Path real = tempDir.resolve("real.txt"); + Files.writeString(real, "in-repo", StandardCharsets.UTF_8); + Path link = tempDir.resolve("alias.txt"); + try { + Files.createSymbolicLink(link, real); + } catch (UnsupportedOperationException | java.io.IOException unsupported) { + return; + } + CodeIqConfigTestSupport.override(config).rootPath(tempDir.toAbsolutePath().toString()).done(); + var controller = new GraphController(queryService, config); + var fileMvc = MockMvcBuilders.standaloneSetup(controller).build(); + + fileMvc.perform(get("/api/file").param("path", "alias.txt")) + .andExpect(status().isOk()) + .andExpect(content().string("in-repo")); + } + // POST /api/analyze removed — API is read-only // --- /api/file-tree --- diff --git a/src/test/java/io/github/randomcodespace/iq/mcp/McpToolsTest.java b/src/test/java/io/github/randomcodespace/iq/mcp/McpToolsTest.java index 11fb6971..c62a2cea 100644 --- a/src/test/java/io/github/randomcodespace/iq/mcp/McpToolsTest.java +++ b/src/test/java/io/github/randomcodespace/iq/mcp/McpToolsTest.java @@ -524,4 +524,47 @@ void readFileShouldClampOutOfBoundsLineRange(@TempDir Path tempDir) throws IOExc assertEquals("line2\nline3", result); } + + @Test + void readFileShouldRejectSymlinkEscapingRoot(@TempDir Path tempDir) throws IOException { + CodeIqConfigTestSupport.override(config).rootPath(tempDir.toString()).done(); + + Path target = Files.createTempFile("codeiq-escape-", ".txt"); + try { + Files.writeString(target, "TOP SECRET"); + Path link = tempDir.resolve("leak.txt"); + try { + Files.createSymbolicLink(link, target.toAbsolutePath()); + } catch (UnsupportedOperationException | IOException unsupported) { + // Filesystem does not support symlinks (e.g. Windows without privilege) — skip. + return; + } + + String result = mcpTools.readFile("leak.txt", null, null); + + assertFalse(result.contains("TOP SECRET"), + "Symlink target contents must not leak through read_file"); + Map parsed = parseJson(result); + assertEquals("Path traversal detected", parsed.get("error")); + } finally { + Files.deleteIfExists(target); + } + } + + @Test + void readFileShouldAllowInRepoSymlink(@TempDir Path tempDir) throws IOException { + CodeIqConfigTestSupport.override(config).rootPath(tempDir.toString()).done(); + Path real = tempDir.resolve("real.txt"); + Files.writeString(real, "in-repo"); + Path link = tempDir.resolve("alias.txt"); + try { + Files.createSymbolicLink(link, real); + } catch (UnsupportedOperationException | IOException unsupported) { + return; + } + + String result = mcpTools.readFile("alias.txt", null, null); + + assertEquals("in-repo", result); + } }