Skip to content

Commit 341b3ad

Browse files
Merge pull request #512 from corbitsdev/cl-6700-reject-empty-path-containment-roots
Reject empty roots in workspace containment prefix check
2 parents 35bffce + acc6ae2 commit 341b3ad

2 files changed

Lines changed: 21 additions & 1 deletion

File tree

‎src/permission/path-restriction.test.ts‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,22 @@ test("workspace-relative paths are unrestricted", () => {
4848
expect(r.isRestricted("src/index.ts", true)).toBe(false);
4949
});
5050

51+
test("an empty-string root does not turn containment into allow-all", () => {
52+
// Regression for CL-6700: root + sep === sep when root is "", which every
53+
// absolute path starts with. A sensitive absolute path resolved against
54+
// a provider that yields "" roots must still be denied.
55+
const r = createPathRestriction(cwd, () => [""], home);
56+
expect(r.isRestricted("/etc/passwd", false)).toBe(true);
57+
expect(r.isRestricted("/etc/passwd", true)).toBe(true);
58+
});
59+
60+
test("a root of exactly \"/\" is not the same bug: it is not an allow-all prefix", () => {
61+
// Documents the non-bug: "/" + sep is "//", which "/etc/passwd" does not
62+
// start with, so an unrelated absolute path stays outside the workspace.
63+
const r = createPathRestriction(cwd, () => ["/"], home);
64+
expect(r.isRestricted("/etc/passwd", false)).toBe(true);
65+
});
66+
5167
test("a directory sharing a string prefix with the workspace root is still restricted", async () => {
5268
// "<cwd>baz" shares a string prefix with cwd but is a distinct sibling
5369
// directory outside the workspace — the boundary check must not leak into

‎src/permission/path-restriction.ts‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -58,8 +58,12 @@ function realpathNearestOr(path: string): string {
5858
}
5959
}
6060

61+
// An empty root must never reach the prefix compare: `"" + sep` is just
62+
// `sep` (e.g. "/" on Unix), which every absolute path starts with, turning
63+
// containment into allow-all. A root of exactly `sep` itself is not this bug
64+
// — `startsWith(sep + sep)` correctly rejects unrelated absolute paths.
6165
const inKnownRoots = (real: string, roots: readonly string[]): boolean =>
62-
roots.some((root) => real === root || real.startsWith(root + sep));
66+
roots.some((root) => root.length > 0 && (real === root || real.startsWith(root + sep)));
6367

6468
// Resolves `path` (relative or absolute, possibly traversing `..`) against
6569
// `cwd` and checks it against the workspace boundary: `cwd` itself plus every

0 commit comments

Comments
 (0)