From 0e9ff022dd79a416fede80461697aafb6ffbed0b Mon Sep 17 00:00:00 2001 From: Nick Bobrowski <39348559+nicko-ai@users.noreply.github.com> Date: Fri, 12 Jun 2026 01:20:54 +0100 Subject: [PATCH 1/2] refactor(core): align filesystem containment helper - use upstream separator-aware contains predicate shape - cover boundary path containment cases --- packages/core/src/filesystem.ts | 6 +++--- packages/core/test/filesystem/filesystem.test.ts | 4 ++++ 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/packages/core/src/filesystem.ts b/packages/core/src/filesystem.ts index 7f0c047de0..b0bb207884 100644 --- a/packages/core/src/filesystem.ts +++ b/packages/core/src/filesystem.ts @@ -1,5 +1,5 @@ import { NodeFileSystem } from "@effect/platform-node" -import { dirname, isAbsolute, join, relative, resolve as pathResolve } from "path" +import { dirname, isAbsolute, join, relative, resolve as pathResolve, sep } from "path" import { realpathSync } from "fs" import * as NFS from "fs/promises" import { lookup } from "mime-types" @@ -239,7 +239,7 @@ export namespace AppFileSystem { } export function contains(parent: string, child: string) { - const rel = relative(parent, child) - return rel === "" || (!rel.startsWith("..") && !isAbsolute(rel)) + const result = relative(parent, child) + return result === "" || (!isAbsolute(result) && result !== ".." && !result.startsWith(`..${sep}`)) } } diff --git a/packages/core/test/filesystem/filesystem.test.ts b/packages/core/test/filesystem/filesystem.test.ts index 1d9405333d..d3f973727e 100644 --- a/packages/core/test/filesystem/filesystem.test.ts +++ b/packages/core/test/filesystem/filesystem.test.ts @@ -354,7 +354,11 @@ describe("AppFileSystem", () => { test("contains checks path containment", () => { expect(AppFileSystem.contains("/a/b", "/a/b/c")).toBe(true) + expect(AppFileSystem.contains("/a/b", "/a/b")).toBe(true) + expect(AppFileSystem.contains("/a/b", "/a/b/..c")).toBe(true) expect(AppFileSystem.contains("/a/b", "/a/c")).toBe(false) + expect(AppFileSystem.contains("/a/b", "/a")).toBe(false) + expect(AppFileSystem.contains("/a/b", "/a/bc")).toBe(false) }) test("overlaps detects overlapping paths", () => { From 9f10678ec9417055d0ff785615f07223f3478fd2 Mon Sep 17 00:00:00 2001 From: Nick Bobrowski <39348559+nicko-ai@users.noreply.github.com> Date: Fri, 12 Jun 2026 19:05:58 +0100 Subject: [PATCH 2/2] fix(core): complete filesystem containment alignment Route overlap checks through the separator-aware containment helper in both core and opencode utilities. Cover child names beginning with dot-dot plus sibling and parent boundary cases. --- packages/core/src/filesystem.ts | 4 +-- .../core/test/filesystem/filesystem.test.ts | 4 +++ packages/opencode/src/util/filesystem.ts | 9 +++---- .../opencode/test/file/path-traversal.test.ts | 25 +++++++++++++++++++ 4 files changed, 34 insertions(+), 8 deletions(-) diff --git a/packages/core/src/filesystem.ts b/packages/core/src/filesystem.ts index b0bb207884..477122f12f 100644 --- a/packages/core/src/filesystem.ts +++ b/packages/core/src/filesystem.ts @@ -233,9 +233,7 @@ export namespace AppFileSystem { } export function overlaps(a: string, b: string) { - const relA = relative(a, b) - const relB = relative(b, a) - return !relA || !relA.startsWith("..") || !relB || !relB.startsWith("..") + return contains(a, b) || contains(b, a) } export function contains(parent: string, child: string) { diff --git a/packages/core/test/filesystem/filesystem.test.ts b/packages/core/test/filesystem/filesystem.test.ts index d3f973727e..973d2eacd5 100644 --- a/packages/core/test/filesystem/filesystem.test.ts +++ b/packages/core/test/filesystem/filesystem.test.ts @@ -363,8 +363,12 @@ describe("AppFileSystem", () => { test("overlaps detects overlapping paths", () => { expect(AppFileSystem.overlaps("/a/b", "/a/b/c")).toBe(true) + expect(AppFileSystem.overlaps("/a/b", "/a/b")).toBe(true) + expect(AppFileSystem.overlaps("/a/b", "/a/b/..c")).toBe(true) expect(AppFileSystem.overlaps("/a/b/c", "/a/b")).toBe(true) + expect(AppFileSystem.overlaps("/a/b", "/a")).toBe(true) expect(AppFileSystem.overlaps("/a", "/b")).toBe(false) + expect(AppFileSystem.overlaps("/a/b", "/a/bc")).toBe(false) }) }) }) diff --git a/packages/opencode/src/util/filesystem.ts b/packages/opencode/src/util/filesystem.ts index 696603adbb..6317cf1155 100644 --- a/packages/opencode/src/util/filesystem.ts +++ b/packages/opencode/src/util/filesystem.ts @@ -1,7 +1,7 @@ import { chmod, mkdir, readFile, stat as statFile, writeFile } from "fs/promises" import { createWriteStream, existsSync, statSync } from "fs" import { realpathSync } from "fs" -import { dirname, isAbsolute, join, relative, resolve as pathResolve, win32 } from "path" +import { dirname, isAbsolute, join, relative, resolve as pathResolve, sep, win32 } from "path" import { Readable } from "stream" import { pipeline } from "stream/promises" import { Glob } from "@opencode-ai/core/util/glob" @@ -163,13 +163,12 @@ export function windowsPath(p: string): string { ) } export function overlaps(a: string, b: string) { - const relA = relative(a, b) - const relB = relative(b, a) - return !relA || !relA.startsWith("..") || !relB || !relB.startsWith("..") + return contains(a, b) || contains(b, a) } export function contains(parent: string, child: string) { - return !relative(parent, child).startsWith("..") + const result = relative(parent, child) + return result === "" || (!isAbsolute(result) && result !== ".." && !result.startsWith(`..${sep}`)) } export async function findUp( diff --git a/packages/opencode/test/file/path-traversal.test.ts b/packages/opencode/test/file/path-traversal.test.ts index 1b0dec5aad..6850339e9c 100644 --- a/packages/opencode/test/file/path-traversal.test.ts +++ b/packages/opencode/test/file/path-traversal.test.ts @@ -25,6 +25,7 @@ describe("Filesystem.contains", () => { 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) + expect(Filesystem.contains("/project", "/project/..config")).toBe(true) }), ) @@ -33,6 +34,7 @@ describe("Filesystem.contains", () => { 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) + expect(Filesystem.contains("/project", "/")).toBe(false) }), ) @@ -52,6 +54,29 @@ describe("Filesystem.contains", () => { ) }) +describe("Filesystem.overlaps", () => { + it.effect("detects containment in either direction", () => + Effect.sync(() => { + expect(Filesystem.overlaps("/project", "/project/src")).toBe(true) + expect(Filesystem.overlaps("/project/src", "/project")).toBe(true) + expect(Filesystem.overlaps("/project", "/project")).toBe(true) + }), + ) + + it.effect("keeps child names beginning with .. inside the parent", () => + Effect.sync(() => { + expect(Filesystem.overlaps("/project", "/project/..config")).toBe(true) + }), + ) + + it.effect("rejects unrelated sibling and parent-only paths", () => + Effect.sync(() => { + expect(Filesystem.overlaps("/project", "/project-other")).toBe(false) + expect(Filesystem.overlaps("/project/src", "/etc")).toBe(false) + }), + ) +}) + /* * Integration tests for read() and list() path traversal protection. *