test(file): migrate path traversal tests to Effect runner (#27098)

2026-05-13 15:44:56 +00:00 · 2026-05-12 12:42:49 -04:00
parent ec4fdaf8e9
commit 8115004c73
1 changed files with 133 additions and 162 deletions
--- 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 = <A, E>(eff: Effect.Effect<A, E, File.Service>) =>
-  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 = <A, E, R>(effect: Effect.Effect<A, E, R>) =>
+  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)
+    }),
+  )
 })