diff --git a/packages/opencode/test/file/path-traversal.test.ts b/packages/opencode/test/file/path-traversal.test.ts index 5b59929ea5..28bd34978b 100644 --- a/packages/opencode/test/file/path-traversal.test.ts +++ b/packages/opencode/test/file/path-traversal.test.ts @@ -1,42 +1,55 @@ -import { test, expect, describe } from "bun:test" -import { Effect } from "effect" +import { expect, describe } from "bun:test" +import { Cause, Effect, Exit } from "effect" import path from "path" import fs from "fs/promises" import { Filesystem } from "@/util/filesystem" import { File } from "../../src/file" -import { Instance } from "../../src/project/instance" -import { WithInstance } from "../../src/project/with-instance" +import { InstanceState } from "../../src/effect/instance-state" import { containsPath } from "../../src/project/instance-context" -import { provideInstance, tmpdir } from "../fixture/fixture" +import { TestInstance } from "../fixture/fixture" +import { testEffect } from "../lib/effect" -const run = (eff: Effect.Effect) => - Effect.runPromise(provideInstance(Instance.directory)(eff.pipe(Effect.provide(File.defaultLayer)))) -const read = (file: string) => run(File.Service.use((svc) => svc.read(file))) -const list = (dir?: string) => run(File.Service.use((svc) => svc.list(dir))) +const it = testEffect(File.defaultLayer) +const read = (file: string) => File.Service.use((svc) => svc.read(file)) +const list = (dir?: string) => File.Service.use((svc) => svc.list(dir)) +const expectAccessDenied = (effect: Effect.Effect) => + Effect.gen(function* () { + const exit = yield* effect.pipe(Effect.exit) + if (Exit.isSuccess(exit)) throw new Error("expected access denied") + expect(Cause.squash(exit.cause)).toHaveProperty("message", "Access denied: path escapes project directory") + }) 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) - }) + it.effect("allows paths within project", () => + Effect.sync(() => { + 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) - }) + it.effect("blocks ../ traversal", () => + Effect.sync(() => { + 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) - }) + it.effect("blocks absolute paths outside project", () => + Effect.sync(() => { + 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) - }) + it.effect("handles prefix collision edge cases", () => + Effect.sync(() => { + expect(Filesystem.contains("/project", "/project-other/file")).toBe(false) + expect(Filesystem.contains("/project", "/projectfile")).toBe(false) + }), + ) }) /* @@ -49,158 +62,116 @@ describe("Filesystem.contains", () => { * 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") - }, - }) + it.instance("rejects ../ traversal attempting to read /etc/passwd", () => + Effect.gen(function* () { + const test = yield* TestInstance + yield* Effect.promise(() => Bun.write(path.join(test.directory, "allowed.txt"), "allowed content")) + yield* expectAccessDenied(read("../../../etc/passwd")) + }), + ) - await WithInstance.provide({ - directory: tmp.path, - fn: async () => { - await expect(read("../../../etc/passwd")).rejects.toThrow("Access denied: path escapes project directory") - }, - }) - }) + it.instance("rejects deeply nested traversal", () => + Effect.gen(function* () { + yield* expectAccessDenied(read("src/nested/../../../../../../../etc/passwd")) + }), + ) - test("rejects deeply nested traversal", async () => { - await using tmp = await tmpdir() + it.instance("allows valid paths within project", () => + Effect.gen(function* () { + const test = yield* TestInstance + yield* Effect.promise(() => Bun.write(path.join(test.directory, "valid.txt"), "valid content")) - await WithInstance.provide({ - directory: tmp.path, - fn: async () => { - await expect(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 WithInstance.provide({ - directory: tmp.path, - fn: async () => { - const result = await read("valid.txt") - expect(result.content).toBe("valid content") - }, - }) - }) + const result = yield* 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() + it.instance("rejects ../ traversal attempting to list /etc", () => + Effect.gen(function* () { + yield* expectAccessDenied(list("../../../etc")) + }), + ) - await WithInstance.provide({ - directory: tmp.path, - fn: async () => { - await expect(list("../../../etc")).rejects.toThrow("Access denied: path escapes project directory") - }, - }) - }) + it.instance("allows valid subdirectory listing", () => + Effect.gen(function* () { + const test = yield* TestInstance + yield* Effect.promise(() => Bun.write(path.join(test.directory, "subdir", "file.txt"), "content")) - 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 WithInstance.provide({ - directory: tmp.path, - fn: async () => { - const result = await list("subdir") - expect(Array.isArray(result)).toBe(true) - }, - }) - }) + const result = yield* list("subdir") + expect(Array.isArray(result)).toBe(true) + }), + ) }) describe("containsPath", () => { - test("returns true for path inside directory", async () => { - await using tmp = await tmpdir({ git: true }) + it.instance("returns true for path inside directory", () => + Effect.gen(function* () { + const test = yield* TestInstance + const ctx = yield* InstanceState.context + expect(containsPath(path.join(test.directory, "foo.txt"), ctx)).toBe(true) + expect(containsPath(path.join(test.directory, "src", "file.ts"), ctx)).toBe(true) + }), + { git: true }, + ) - await WithInstance.provide({ - directory: tmp.path, - fn: () => { - expect(containsPath(path.join(tmp.path, "foo.txt"), Instance.current)).toBe(true) - expect(containsPath(path.join(tmp.path, "src", "file.ts"), Instance.current)).toBe(true) - }, - }) - }) + it.instance( + "returns true for path inside worktree but outside directory (monorepo subdirectory scenario)", + () => + Effect.gen(function* () { + const test = yield* TestInstance + const subdir = path.join(test.directory, "packages", "lib") + yield* Effect.promise(() => fs.mkdir(subdir, { recursive: true })) + const ctx = { ...(yield* InstanceState.context), directory: subdir } - test("returns true for path inside worktree but outside directory (monorepo subdirectory scenario)", async () => { - await using tmp = await tmpdir({ git: true }) - const subdir = path.join(tmp.path, "packages", "lib") - await fs.mkdir(subdir, { recursive: true }) - - await WithInstance.provide({ - directory: subdir, - fn: () => { // .opencode at worktree root, but we're running from packages/lib - expect(containsPath(path.join(tmp.path, ".opencode", "state"), Instance.current)).toBe(true) + expect(containsPath(path.join(test.directory, ".opencode", "state"), ctx)).toBe(true) // sibling package should also be accessible - expect(containsPath(path.join(tmp.path, "packages", "other", "file.ts"), Instance.current)).toBe(true) + expect(containsPath(path.join(test.directory, "packages", "other", "file.ts"), ctx)).toBe(true) // worktree root itself - expect(containsPath(tmp.path, Instance.current)).toBe(true) - }, - }) - }) + expect(containsPath(test.directory, ctx)).toBe(true) + }), + { git: true }, + ) - test("returns false for path outside both directory and worktree", async () => { - await using tmp = await tmpdir({ git: true }) + it.instance("returns false for path outside both directory and worktree", () => + Effect.gen(function* () { + const ctx = yield* InstanceState.context + expect(containsPath("/etc/passwd", ctx)).toBe(false) + expect(containsPath("/tmp/other-project", ctx)).toBe(false) + }), + { git: true }, + ) - await WithInstance.provide({ - directory: tmp.path, - fn: () => { - expect(containsPath("/etc/passwd", Instance.current)).toBe(false) - expect(containsPath("/tmp/other-project", Instance.current)).toBe(false) - }, - }) - }) + it.instance("returns false for path with .. escaping worktree", () => + Effect.gen(function* () { + const test = yield* TestInstance + const ctx = yield* InstanceState.context + expect(containsPath(path.join(test.directory, "..", "escape.txt"), ctx)).toBe(false) + }), + { git: true }, + ) - test("returns false for path with .. escaping worktree", async () => { - await using tmp = await tmpdir({ git: true }) + it.instance("handles directory === worktree (running from repo root)", () => + Effect.gen(function* () { + const test = yield* TestInstance + const ctx = yield* InstanceState.context + expect(ctx.directory).toBe(ctx.worktree) + expect(containsPath(path.join(test.directory, "file.txt"), ctx)).toBe(true) + expect(containsPath("/etc/passwd", ctx)).toBe(false) + }), + { git: true }, + ) - await WithInstance.provide({ - directory: tmp.path, - fn: () => { - expect(containsPath(path.join(tmp.path, "..", "escape.txt"), Instance.current)).toBe(false) - }, - }) - }) - - test("handles directory === worktree (running from repo root)", async () => { - await using tmp = await tmpdir({ git: true }) - - await WithInstance.provide({ - directory: tmp.path, - fn: () => { - expect(Instance.directory).toBe(Instance.worktree) - expect(containsPath(path.join(tmp.path, "file.txt"), Instance.current)).toBe(true) - expect(containsPath("/etc/passwd", Instance.current)).toBe(false) - }, - }) - }) - - test("non-git project does not allow arbitrary paths via worktree='/'", async () => { - await using tmp = await tmpdir() // no git: true - - await WithInstance.provide({ - directory: tmp.path, - fn: () => { - // worktree is "/" for non-git projects, but containsPath should NOT allow all paths - expect(containsPath(path.join(tmp.path, "file.txt"), Instance.current)).toBe(true) - expect(containsPath("/etc/passwd", Instance.current)).toBe(false) - expect(containsPath("/tmp/other", Instance.current)).toBe(false) - }, - }) - }) + it.instance("non-git project does not allow arbitrary paths via worktree='/'", () => + Effect.gen(function* () { + const test = yield* TestInstance + const ctx = yield* InstanceState.context + // worktree is "/" for non-git projects, but containsPath should NOT allow all paths + expect(containsPath(path.join(test.directory, "file.txt"), ctx)).toBe(true) + expect(containsPath("/etc/passwd", ctx)).toBe(false) + expect(containsPath("/tmp/other", ctx)).toBe(false) + }), + ) })