summaryrefslogtreecommitdiffhomepage
diff options
context:
space:
mode:
authorrari404 <[email protected]>2025-12-27 00:20:07 -0500
committerGitHub <[email protected]>2025-12-26 23:20:07 -0600
commit3c02d5d338698d382b02fbbf7eaa07a9b0f1ebc6 (patch)
treecaccd4850d7c5f50c80419bcc56010fdc70e93ea
parentbfb9787361af01cf2eb8c6dbb2dd7dd08e91496f (diff)
downloadopencode-3c02d5d338698d382b02fbbf7eaa07a9b0f1ebc6.tar.gz
opencode-3c02d5d338698d382b02fbbf7eaa07a9b0f1ebc6.zip
feat: add path traversal protection to File.read and File.list (#5985)
-rw-r--r--packages/opencode/src/file/index.ts15
-rw-r--r--packages/opencode/test/file/path-traversal.test.ts115
2 files changed, 130 insertions, 0 deletions
diff --git a/packages/opencode/src/file/index.ts b/packages/opencode/src/file/index.ts
index 148ab45cb..9462ec573 100644
--- a/packages/opencode/src/file/index.ts
+++ b/packages/opencode/src/file/index.ts
@@ -7,6 +7,7 @@ import path from "path"
import fs from "fs"
import ignore from "ignore"
import { Log } from "../util/log"
+import { Filesystem } from "../util/filesystem"
import { Instance } from "../project/instance"
import { Ripgrep } from "./ripgrep"
import fuzzysort from "fuzzysort"
@@ -235,6 +236,13 @@ export namespace File {
using _ = log.time("read", { file })
const project = Instance.project
const full = path.join(Instance.directory, file)
+
+ // TODO: Filesystem.contains is lexical only - symlinks inside the project can escape.
+ // TODO: On Windows, cross-drive paths bypass this check. Consider realpath canonicalization.
+ if (!Filesystem.contains(Instance.directory, full)) {
+ throw new Error(`Access denied: path escapes project directory`)
+ }
+
const bunFile = Bun.file(full)
if (!(await bunFile.exists())) {
@@ -288,6 +296,13 @@ export namespace File {
ignored = ig.ignores.bind(ig)
}
const resolved = dir ? path.join(Instance.directory, dir) : Instance.directory
+
+ // TODO: Filesystem.contains is lexical only - symlinks inside the project can escape.
+ // TODO: On Windows, cross-drive paths bypass this check. Consider realpath canonicalization.
+ if (!Filesystem.contains(Instance.directory, resolved)) {
+ throw new Error(`Access denied: path escapes project directory`)
+ }
+
const nodes: Node[] = []
for (const entry of await fs.promises
.readdir(resolved, {
diff --git a/packages/opencode/test/file/path-traversal.test.ts b/packages/opencode/test/file/path-traversal.test.ts
new file mode 100644
index 000000000..c20c76a2e
--- /dev/null
+++ b/packages/opencode/test/file/path-traversal.test.ts
@@ -0,0 +1,115 @@
+import { test, expect, describe } from "bun:test"
+import path from "path"
+import { Filesystem } from "../../src/util/filesystem"
+import { File } from "../../src/file"
+import { Instance } from "../../src/project/instance"
+import { tmpdir } from "../fixture/fixture"
+
+describe("Filesystem.contains", () => {
+ test("allows paths within project", () => {
+ expect(Filesystem.contains("/project", "/project/src")).toBe(true)
+ expect(Filesystem.contains("/project", "/project/src/file.ts")).toBe(true)
+ expect(Filesystem.contains("/project", "/project")).toBe(true)
+ })
+
+ test("blocks ../ traversal", () => {
+ expect(Filesystem.contains("/project", "/project/../etc")).toBe(false)
+ expect(Filesystem.contains("/project", "/project/src/../../etc")).toBe(false)
+ expect(Filesystem.contains("/project", "/etc/passwd")).toBe(false)
+ })
+
+ test("blocks absolute paths outside project", () => {
+ expect(Filesystem.contains("/project", "/etc/passwd")).toBe(false)
+ expect(Filesystem.contains("/project", "/tmp/file")).toBe(false)
+ expect(Filesystem.contains("/home/user/project", "/home/user/other")).toBe(false)
+ })
+
+ test("handles prefix collision edge cases", () => {
+ expect(Filesystem.contains("/project", "/project-other/file")).toBe(false)
+ expect(Filesystem.contains("/project", "/projectfile")).toBe(false)
+ })
+})
+
+/*
+ * Integration tests for File.read() and File.list() path traversal protection.
+ *
+ * These tests verify the HTTP API code path is protected. The HTTP endpoints
+ * in server.ts (GET /file/content, GET /file) call File.read()/File.list()
+ * directly - they do NOT go through ReadTool or the agent permission layer.
+ *
+ * This is a SEPARATE code path from ReadTool, which has its own checks.
+ */
+describe("File.read path traversal protection", () => {
+ test("rejects ../ traversal attempting to read /etc/passwd", async () => {
+ await using tmp = await tmpdir({
+ init: async (dir) => {
+ await Bun.write(path.join(dir, "allowed.txt"), "allowed content")
+ },
+ })
+
+ await Instance.provide({
+ directory: tmp.path,
+ fn: async () => {
+ await expect(File.read("../../../etc/passwd")).rejects.toThrow("Access denied: path escapes project directory")
+ },
+ })
+ })
+
+ test("rejects deeply nested traversal", async () => {
+ await using tmp = await tmpdir()
+
+ await Instance.provide({
+ directory: tmp.path,
+ fn: async () => {
+ await expect(File.read("src/nested/../../../../../../../etc/passwd")).rejects.toThrow(
+ "Access denied: path escapes project directory",
+ )
+ },
+ })
+ })
+
+ test("allows valid paths within project", async () => {
+ await using tmp = await tmpdir({
+ init: async (dir) => {
+ await Bun.write(path.join(dir, "valid.txt"), "valid content")
+ },
+ })
+
+ await Instance.provide({
+ directory: tmp.path,
+ fn: async () => {
+ const result = await File.read("valid.txt")
+ expect(result.content).toBe("valid content")
+ },
+ })
+ })
+})
+
+describe("File.list path traversal protection", () => {
+ test("rejects ../ traversal attempting to list /etc", async () => {
+ await using tmp = await tmpdir()
+
+ await Instance.provide({
+ directory: tmp.path,
+ fn: async () => {
+ await expect(File.list("../../../etc")).rejects.toThrow("Access denied: path escapes project directory")
+ },
+ })
+ })
+
+ test("allows valid subdirectory listing", async () => {
+ await using tmp = await tmpdir({
+ init: async (dir) => {
+ await Bun.write(path.join(dir, "subdir", "file.txt"), "content")
+ },
+ })
+
+ await Instance.provide({
+ directory: tmp.path,
+ fn: async () => {
+ const result = await File.list("subdir")
+ expect(Array.isArray(result)).toBe(true)
+ },
+ })
+ })
+})