fix(security): add path traversal protection to File.read and File.list

This commit is contained in:
edlsh 2025-12-22 15:05:07 -05:00
parent 1b1b73b5b3
commit 7334d42488
2 changed files with 40 additions and 0 deletions

View file

@ -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, {

View file

@ -0,0 +1,25 @@
import { test, expect } from "bun:test"
import { Filesystem } from "../../src/util/filesystem"
test("Filesystem.contains blocks parent directory traversal", () => {
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("Filesystem.contains 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("Filesystem.contains 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("Filesystem.contains handles edge cases", () => {
expect(Filesystem.contains("/project", "/project-other/file")).toBe(false)
expect(Filesystem.contains("/project", "/projectfile")).toBe(false)
})