Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
152 changes: 138 additions & 14 deletions packages/local-runtime/src/turns/file-changes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,14 @@ const PATCH_CONTEXT_LINES = 3;
* past this budget we fall back to the whole-file deletion+addition hunk.
*/
const MAX_DIFF_EDIT_LENGTH = 2000;
/**
* Upper bound on the symlink hops followed while resolving a mutation
* target's real path. A path that needs more hops than this — a symlink
* cycle, or a chain long enough to look like one — is refused rather than
* followed: this is a containment boundary, so a path that cannot be
* resolved is not a safe path.
*/
const MAX_REAL_PATH_SYMLINK_HOPS = 32;
const STRUCTURED_WRITE_TOOLS = new Set([
'edit',
'write',
Expand Down Expand Up @@ -406,9 +414,19 @@ export async function applyLocalTurnDiffSnapshotMutation(
*
* Safety first, ahead of the conflict check below: an operation that would
* write outside the workspace is refused without even reading the files. */
const workspaceRoot = path.resolve(record.workspaceDir);
/* The real workspace root, resolved once for the whole run: the
* containment check compares real paths, and the root does not change while
* the operation runs. A workspace that cannot be resolved refuses the run. */
const realWorkspaceRoot = await resolveRealPath(workspaceRoot);
if (!realWorkspaceRoot) return { success: false, reason: 'unsafe_path' };
const targets: string[] = [];
for (const undo of record.undo) {
const absolute = resolveMutationTargetPath(record.workspaceDir, undo.file);
const absolute = await resolveMutationTargetPath(
workspaceRoot,
realWorkspaceRoot,
undo.file,
);
if (!absolute) return { success: false, reason: 'unsafe_path' };
const target = undo[targetKey];
if (target.exists && target.content === undefined) {
Expand Down Expand Up @@ -446,29 +464,135 @@ export async function applyLocalTurnDiffSnapshotMutation(
* records out-of-workspace writes — so a truthy result is NOT proof of
* containment and the mutation has to check it. It returns `undefined` for
* the in-workspace paths the capture layer filters (`.git/`,
* `node_modules/`, the root itself), which is also a refusal here. The
* `.harness-def/`, the root itself), which is also a refusal here. The
* containment re-check after `path.resolve` is deliberate: this is a
* security boundary, so it does not rely on `normalizeCapturePath`'s
* contract holding forever.
*
* Known boundary (lexical containment only): a contained path that is a
* symlink pointing outside the workspace still resolves outside. Closing
* that needs `realpath` on every target with careful handling of
* not-yet-existing files, which is a larger change than this refusal.
* Containment is checked twice, and the second check is the one that binds:
*
* 1. Lexically, against the workspace root. It costs no syscall and
* refuses the plain escapes — an absolute path, a `..` climb, a sibling
* directory whose name starts with the workspace's name — before any
* path on disk is looked at.
* 2. Against the *real* paths, with symlinks resolved. Lexical containment
* is not containment: a path inside the workspace whose final component
* is a symlink pointing outside it is inside the workspace as a string
* and outside it as a file, and the write follows the link. The real
* path of the target is therefore compared with the real path of the
* workspace root, which also means a workspace that is itself reached
* through a symlink still contains its own files.
*
* `realWorkspaceRoot` is the already-resolved real root, passed in so a
* record with many entries does not resolve it once per entry.
*
* Known boundary (unchanged by this check): the real path is resolved before
* the write, so a symlink swapped between the check and the write is still
* a race. Closing that needs a write that cannot follow a link, which is a
* larger change than this refusal.
*/
function resolveMutationTargetPath(
workspaceDir: string,
async function resolveMutationTargetPath(
workspaceRoot: string,
realWorkspaceRoot: string,
file: string,
): string | undefined {
const normalized = normalizeCapturePath(workspaceDir, file);
): Promise<string | undefined> {
const normalized = normalizeCapturePath(workspaceRoot, file);
if (!normalized) return undefined;
if (path.isAbsolute(normalized) || path.win32.isAbsolute(normalized)) {
return undefined;
}
const root = path.resolve(workspaceDir);
const absolute = path.resolve(root, normalized);
const prefix = root.endsWith(path.sep) ? root : `${root}${path.sep}`;
return absolute === root || absolute.startsWith(prefix) ? absolute : undefined;
const absolute = path.resolve(workspaceRoot, normalized);
const prefix = workspaceRoot.endsWith(path.sep)
? workspaceRoot
: `${workspaceRoot}${path.sep}`;
if (absolute !== workspaceRoot && !absolute.startsWith(prefix)) return undefined;
const realTarget = await resolveRealPath(absolute);
if (!realTarget) return undefined;
return isInsideDirectory(realWorkspaceRoot, realTarget) ? absolute : undefined;
}

/**
* Resolve a path to the real location the filesystem uses for it, following
* symlinks.
*
* The shape follows the tool path guard
* (`agent-tools/src/desktop/path-guard.ts`): `realpath` the target, and when
* the target does not exist yet — a revert recreates a deleted file — resolve
* its nearest existing ancestor and re-attach the missing segments.
*
* One thing that guard does not have to handle and this one does: the target
* may be a *dangling* symlink. `realpath` fails on it the same way it fails
* on a missing file, but a write through a dangling link creates the file it
* points at, so the link is followed by hand instead of being treated as a
* missing path.
*
* Returns `undefined` when the path cannot be resolved at all — a symlink
* cycle, a permission error — so the caller refuses the target instead of
* guessing where it points.
*/
async function resolveRealPath(
absolutePath: string,
hops = 0,
): Promise<string | undefined> {
let candidate = path.resolve(absolutePath);
const suffixParts: string[] = [];
for (;;) {
try {
const ancestorReal = await fs.realpath(candidate);
return suffixParts.length === 0
? ancestorReal
: path.resolve(ancestorReal, ...suffixParts);
} catch (error) {
const code = (error as NodeJS.ErrnoException | undefined)?.code;
if (code !== 'ENOENT' && code !== 'ENOTDIR') return undefined;
const link = await readlinkOrUndefined(candidate);
if (link !== undefined) {
if (hops >= MAX_REAL_PATH_SYMLINK_HOPS) return undefined;
const realParent = await realpathOrUndefined(path.dirname(candidate));
if (!realParent) return undefined;
// A relative link is relative to the directory that holds it, which
// is the *real* directory when the link sits in a symlinked one.
const followed = path.isAbsolute(link)
? path.resolve(link)
: path.resolve(realParent, link);
const resolved = await resolveRealPath(followed, hops + 1);
if (!resolved) return undefined;
return suffixParts.length ? path.resolve(resolved, ...suffixParts) : resolved;
}
const parent = path.dirname(candidate);
if (parent === candidate) return undefined;
suffixParts.unshift(path.basename(candidate));
candidate = parent;
}
}
}

/**
* Whether `candidate` is `root` itself or a path inside it.
*
* The comparison follows the tool path guard
* (`agent-tools/src/desktop/path-guard.ts`): `path.relative` decides, and it
* is case-insensitive on Windows, where the filesystem is.
*/
function isInsideDirectory(root: string, candidate: string): boolean {
const rel = path.relative(root, candidate);
return !(rel === '..' || rel.startsWith(`..${path.sep}`) || path.isAbsolute(rel));
}

async function realpathOrUndefined(target: string): Promise<string | undefined> {
try {
return await fs.realpath(target);
} catch {
return undefined;
}
}

async function readlinkOrUndefined(target: string): Promise<string | undefined> {
try {
return await fs.readlink(target);
} catch {
return undefined;
}
}

function makeCaptureId(): string {
Expand Down
Loading
Loading