Skip to content

Commit f951d8b

Browse files
committed
fix(engine): allow symlinks that resolve inside the project root
The ancestor-symlink check from #499 rejected any symlinked path component below the project root, regardless of where it pointed. That over-rejects: a link resolving back inside the project escapes nothing, and symlinked directories inside a repository are an ordinary layout. Workspace monorepos symlink node_modules/<pkg> to packages/<pkg> (pnpm, npm, yarn and bun all do this), and projects symlink shared source directories — so archgate refused to read files it is meant to govern, failing the rule with "access denied". The gate is now where a symlink POINTS rather than that one exists: each component below the root is resolved and rejected only when its target lands outside the root. The escape #499 closed stays closed — an outside-pointing link is still rejected at any component, including after an earlier hop that legitimately stayed inside. Comparison is realpath-against-realpath, never realpath-against-lexical: realpath case-canonicalizes on Windows and macOS, so mixing the two would reject case-mismatched-but-legitimate paths. Both sides are resolved so the same canonicalization applies to each, and the root's own realpath is memoized alongside the per-component memo. The component memo is keyed by (root, component) rather than component alone: the same component can sit under two roots with different verdicts, since a link leaving root A may land inside an enclosing root B. Note this also relaxes the pre-existing LEAF symlink check, which likewise rejected in-project links. Leaving the leaf strict while allowing ancestors would be arbitrary — a file reachable as packages/shared/index.ts should not become unreadable because it is also linked as index.ts. Signed-off-by: Rhuan Barreto <rhuan@barreto.work>
1 parent 9a114b3 commit f951d8b

2 files changed

Lines changed: 120 additions & 29 deletions

File tree

src/engine/safe-path.ts

Lines changed: 61 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
// SPDX-License-Identifier: Apache-2.0
22
// Copyright 2026 Archgate
3-
import { lstatSync } from "node:fs";
3+
import { lstatSync, realpathSync } from "node:fs";
44
import { resolve, isAbsolute, join, relative } from "node:path";
55

66
import { UserError } from "../helpers/user-error";
@@ -31,23 +31,45 @@ export function isWithinRoot(resolvedRoot: string, absPath: string): boolean {
3131
}
3232

3333
/**
34-
* Directories verified as real (non-symlink), memoized per process: safePath
35-
* runs once per glob result and siblings share every ancestor, so without this
36-
* the walk below re-lstats the same directories thousands of times per run.
34+
* Components already verified safe, memoized per process: safePath runs once
35+
* per glob result and siblings share every ancestor. Nested per root — the
36+
* same component under two roots can differ, since a link leaving root A may
37+
* land inside an enclosing root B.
3738
*/
38-
const verifiedRealDirs = new Set<string>();
39+
const verifiedComponents = new Map<string, Set<string>>();
40+
41+
/** Project roots resolved through `realpath`, memoized per process. */
42+
const realRoots = new Map<string, string>();
43+
44+
/**
45+
* The project root with symlinks in its own path resolved, so a link target
46+
* compares against it like-for-like. Falls back to the lexical root.
47+
*/
48+
function realRootOf(resolvedRoot: string): string {
49+
let hit = realRoots.get(resolvedRoot);
50+
if (hit === undefined) {
51+
try {
52+
hit = realpathSync(resolvedRoot);
53+
} catch {
54+
hit = resolvedRoot;
55+
}
56+
realRoots.set(resolvedRoot, hit);
57+
}
58+
return hit;
59+
}
3960

4061
/**
41-
* Reject a symlink anywhere in the path below the project root — the leaf OR
42-
* any ancestor, since a linked ancestor makes the leaf look like an ordinary
43-
* file to both `isWithinRoot` and a leaf `lstat`. Components at or above the
44-
* root are deliberately not inspected, and each test is a boolean `lstat`
45-
* rather than a `realpath` comparison.
62+
* Reject a path whose real target lies outside the project root, testing every
63+
* component below it — the leaf AND every ancestor, since a linked ancestor
64+
* makes the leaf look ordinary to both `isWithinRoot` and a leaf `lstat`. The
65+
* gate is where a link POINTS, not that one exists: links resolving back
66+
* inside the project are a normal layout (workspace monorepos, shared dirs).
4667
*
47-
* @throws {UserError} When any component below the root is a symbolic link.
48-
* @see ARCH-022 — why the walk stops at the root and avoids `realpath`
68+
* @throws {UserError} When a component resolves outside the project root.
69+
* @see ARCH-022 — why the walk stops at the root
70+
* @see .claude/agent-memory/archgate-developer/project_rules_engine_internals.md — why both sides are realpath'd
4971
*/
50-
function assertNoSymlinkInPath(
72+
function assertNoEscapingSymlink(
5173
resolvedRoot: string,
5274
absPath: string,
5375
userPath: string
@@ -57,10 +79,16 @@ function assertNoSymlinkInPath(
5779
if (!rel || rel.startsWith("..")) return;
5880

5981
const segments = rel.split(/[/\\]/u);
82+
let verified = verifiedComponents.get(resolvedRoot);
83+
if (!verified) {
84+
verified = new Set();
85+
verifiedComponents.set(resolvedRoot, verified);
86+
}
87+
6088
let current = resolvedRoot;
61-
for (const [index, segment] of segments.entries()) {
89+
for (const segment of segments) {
6290
current = join(current, segment);
63-
if (verifiedRealDirs.has(current)) continue;
91+
if (verified.has(current)) continue;
6492
let stat;
6593
try {
6694
stat = lstatSync(current);
@@ -70,20 +98,28 @@ function assertNoSymlinkInPath(
7098
return;
7199
}
72100
if (stat.isSymbolicLink()) {
73-
throw new UserError(
74-
index === segments.length - 1
75-
? `Path "${userPath}" is a symbolic link — access denied`
76-
: `Path "${userPath}" traverses symbolic link "${segment}" — access denied`
77-
);
101+
let target: string;
102+
try {
103+
target = realpathSync(current);
104+
} catch {
105+
return; // Broken link — resolves to nothing, so it reaches nothing.
106+
}
107+
if (!isWithinRoot(realRootOf(resolvedRoot), target)) {
108+
throw new UserError(
109+
`Path "${userPath}" resolves outside the project through symbolic link "${segment}" — access denied`
110+
);
111+
}
78112
}
79-
if (stat.isDirectory()) verifiedRealDirs.add(current);
113+
verified.add(current);
80114
}
81115
}
82116

83117
/**
84-
* Resolve a user-supplied path and ensure it stays within projectRoot.
85-
* Throws if the resolved path escapes the project boundary, is a symlink, or
86-
* reaches its target through a symlinked ancestor directory.
118+
* Resolve a user-supplied path and ensure it stays within projectRoot, either
119+
* lexically or after resolving symlinks at any component. Links resolving back
120+
* inside the project are allowed.
121+
*
122+
* @throws {UserError} When the path escapes the project root.
87123
*/
88124
export function safePath(resolvedRoot: string, userPath: string): string {
89125
const absPath = resolveUserPath(resolvedRoot, userPath);
@@ -92,6 +128,6 @@ export function safePath(resolvedRoot: string, userPath: string): string {
92128
`Path "${userPath}" escapes project root — access denied`
93129
);
94130
}
95-
assertNoSymlinkInPath(resolvedRoot, absPath, userPath);
131+
assertNoEscapingSymlink(resolvedRoot, absPath, userPath);
96132
return absPath;
97133
}

tests/engine/safe-path.test.ts

Lines changed: 59 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -79,7 +79,7 @@ describe("safe-path", () => {
7979
);
8080
});
8181

82-
test("throws when an ANCESTOR directory is a symlink", () => {
82+
test("throws when an ANCESTOR directory links OUTSIDE the project", () => {
8383
// The leaf is an ordinary file — only the parent is a link, which a
8484
// leaf-only lstat cannot see.
8585
const outsideDir = mkdtempSync(join(tmpdir(), "archgate-outside-"));
@@ -93,12 +93,34 @@ describe("safe-path", () => {
9393
return;
9494
}
9595
expect(() => safePath(tempDir, "linkdir/secret.txt")).toThrow(
96-
/traverses symbolic link "linkdir" access denied/u
96+
/resolves outside the project through symbolic link "linkdir" access denied/u
9797
);
9898
rmSync(outsideDir, { recursive: true, force: true });
9999
});
100100

101-
test("throws when the LEAF itself is a symlink", () => {
101+
test("ALLOWS an ancestor directory symlink that stays inside the project", () => {
102+
// The workspace-monorepo layout: apps/web/shared -> packages/shared.
103+
// This escapes nothing, so refusing it would refuse to govern ordinary
104+
// repositories (pnpm/npm/yarn/bun all symlink workspace packages).
105+
mkdirSync(join(tempDir, "packages", "shared"), { recursive: true });
106+
writeFileSync(
107+
join(tempDir, "packages", "shared", "index.ts"),
108+
"export {};"
109+
);
110+
mkdirSync(join(tempDir, "apps", "web"), { recursive: true });
111+
try {
112+
symlinkSync(
113+
join(tempDir, "packages", "shared"),
114+
join(tempDir, "apps", "web", "shared"),
115+
"junction"
116+
);
117+
} catch {
118+
return;
119+
}
120+
expect(() => safePath(tempDir, "apps/web/shared/index.ts")).not.toThrow();
121+
});
122+
123+
test("throws when the LEAF links OUTSIDE the project", () => {
102124
const outsideDir = mkdtempSync(join(tmpdir(), "archgate-outside-"));
103125
writeFileSync(join(outsideDir, "secret.txt"), "sensitive");
104126
try {
@@ -109,7 +131,40 @@ describe("safe-path", () => {
109131
return;
110132
}
111133
expect(() => safePath(tempDir, "link.txt")).toThrow(
112-
/is a symbolic link access denied/u
134+
/resolves outside the project through symbolic link "link.txt" access denied/u
135+
);
136+
rmSync(outsideDir, { recursive: true, force: true });
137+
});
138+
139+
test("ALLOWS a leaf symlink that stays inside the project", () => {
140+
mkdirSync(join(tempDir, "real"), { recursive: true });
141+
writeFileSync(join(tempDir, "real", "config.ts"), "export {};");
142+
try {
143+
symlinkSync(
144+
join(tempDir, "real", "config.ts"),
145+
join(tempDir, "config.ts")
146+
);
147+
} catch {
148+
return;
149+
}
150+
expect(() => safePath(tempDir, "config.ts")).not.toThrow();
151+
});
152+
153+
test("throws when a chain hops inside the project and then out", () => {
154+
// inside-link -> real dir; real dir contains escape -> outside.
155+
// Allowing the first hop must not stop the walk checking later ones.
156+
const outsideDir = mkdtempSync(join(tmpdir(), "archgate-outside-"));
157+
writeFileSync(join(outsideDir, "secret.txt"), "sensitive");
158+
mkdirSync(join(tempDir, "real"), { recursive: true });
159+
try {
160+
symlinkSync(join(tempDir, "real"), join(tempDir, "hop"), "junction");
161+
symlinkSync(outsideDir, join(tempDir, "real", "escape"), "junction");
162+
} catch {
163+
rmSync(outsideDir, { recursive: true, force: true });
164+
return;
165+
}
166+
expect(() => safePath(tempDir, "hop/escape/secret.txt")).toThrow(
167+
/resolves outside the project.*access denied/u
113168
);
114169
rmSync(outsideDir, { recursive: true, force: true });
115170
});

0 commit comments

Comments
 (0)