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
22 changes: 22 additions & 0 deletions .memory/edit-file-symlink-race-20261015.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
# Parent edit_file symlink-swap fix — 2026-10-15

Issue #423: the parent tool previously checked a canonical path, then used path-based `fs.writeFile` after a test seam. A target or ancestor symlink swap could redirect the final write outside the pinned workspace.

The parent tool now reads an opened regular file, records its exact device/inode and SHA-256, and passes the canonical root-relative path to the existing packaged cross-platform file-mutator helper. Its new `edit-file` operation opens the pinned root and every relative directory component with no-follow descriptors, checks the opened file's identity and content, then truncates and writes only through that descriptor. The ordinary in-root symlink policy remains: TypeScript resolves a stable link to its in-root target before the native traversal. Multiply linked files fail closed so an outside hard link cannot be changed. The helper accepts up to 256 MiB, beyond the existing subagent protocol's 200 KB cap.

The security claim is limited to preventing a symlink/path replacement from redirecting the write to a different inode. A local actor can rename the already-open file or its parent outside the root after the native name check; the descriptor then still writes that original inode at its new location. This is not a guarantee that every inode modified by the tool remains reachable inside the workspace throughout the write. Such a stronger guarantee would require control of concurrent namespace mutation, not another path recheck or a held parent descriptor.

The high-level tool test swaps the target or an ancestor between validation and commit and checks that the outside file is unchanged. Native tests exercise the same no-follow rule on macOS and Linux, and a separate test covers a stable in-root link and a file above 200 KB. The root test lane now builds the helper before coding-tools tests; the checked-in helper binaries and manifests were refreshed for macOS universal, Linux x64, and Linux arm64. No shared server protocol, native client, UI, onboarding, or plan status changed.

Local verification: native mutator suite, CI test registry, native prebuilt manifest suite, scoped ESLint, and TypeScript type-check pass. The full coding-tools suite had one independent glob/native-glob comparison failure on `**/*`; the new edit tests passed. Hosted exact-head checks and review remain PR delivery gates.

A review of PR #501 found the standalone CLI Bot route used the desktop development fallback (`<cwd>/build/native/…`) for the new native edit operation. The CLI now passes the helper staged beside `dist/app/cli.js` through the Bot file router and pinned tool factory. The packaged parity test changes into an external temporary project with an executable decoy at the old fallback path, edits a Bot file successfully, and verifies that the decoy did not run. CLI build, CLI and root type checks, scoped lint, and focused desktop and Bot tests passed.
The full CLI suite passed 99/99 tests after the helper lookup fix. The Bot file router suite passed 8/8, and the focused edit tests passed 4/4. PR #492 remained open when this update was made.

After PR #492 merged as c68462b97, merged origin/main into PR #501 as 46f30b2ce. The combined `edit_file` uses `indexOf`/slices for literal replacement while retaining the opened-file identity/hash checks and descriptor-bound native commit. On the merge head, focused edit/literal tests passed 5/5, Bot router tests 8/8, the CLI build/type-check and full CLI suite passed 99/99, and root type-check and scoped lint passed. The complete coding-tools suite passed 61/62; its sole failure is the previously documented independent foreground/native glob `**/*` mismatch.

Main advanced again through unrelated PR #491 to dd2c0d03c. Merged it into PR #501 as 6fff24413 without conflicts. The shared `package.json` test chain retained both branches' scripts; CI registry policy tests passed 62/62. The descriptor-bound edit and literal replacement remained intact. Focused edit/literal tests passed 5/5, Bot router tests 8/8, the rebuilt CLI suite passed 99/99, both root and CLI type checks passed, and scoped lint passed. The earlier full coding-tools glob mismatch remains documented above; no edit test failed.

The exact-head CI run 38825795960 for b5a6ba37 had Electron E2E shard 3 fail on three #491-origin tests: draft-agent-chats.spec.ts:205 failed twice because the workspace header remained keyboard focused and the plus button's intentional group-focus-within style kept opacity at 1, while chat-top-bar.spec.ts:236 and :327 failed once waiting for the worktree-send failure UI and passed retry. Both test and UI sources match main; this is classified as inherited baseline behavior, not an edit-path integration defect. The PR body records exact specs, symptoms, and run link. The run was still active when main advanced to #497, so no failed-job rerun was used on the stale head. Merged latest main 17d51d472 into PR #501 as dee27de64; this merge brought only Android simulator changes. Focused edit/literal tests passed 5/5, CLI Bot parity passed 1/1, and root type-check passed.

The separate #491 baseline test fix PR #506 merged to main as 896f31ae1. Merged that current main into PR #501 as b93b0b1e1 without conflicts; it changed the E2E focus expectation and native-client stream recovery, not the edit path or package scripts. On this merged checkout, the production build, root and E2E type checks, focused edit/literal tests (5/5), CLI Bot parity (1/1), and the corrected sidebar plus two worktree-send E2E specs repeated twice each (6/6 with fail-on-flaky-tests) passed.
7 changes: 5 additions & 2 deletions main/services/bot-file-tool-router.ts
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,8 @@ export interface BotFileToolRouterOptions {
* Defaults to the process-wide roots (`setBotFileToolProtectedRoots`).
*/
readonly protectedRoots?: () => readonly string[];
/** Installation-owned native helper used for edit_file. */
readonly fileMutatorBinary?: string;
}

/** File tools that change a file; only these are fenced from protected roots. */
Expand Down Expand Up @@ -146,11 +148,12 @@ function checkedLocation(location: BotFileToolLocation, role: string): BotFileTo
});
}

function underlyingTools(location: BotFileToolLocation): ReadonlyMap<BotFileToolName, AgentTool> {
function underlyingTools(location: BotFileToolLocation, fileMutatorBinary?: string): ReadonlyMap<BotFileToolName, AgentTool> {
const selected = buildPinnedCodingTools(
location.root,
undefined,
location.expectedIdentity,
{ fileMutatorBinary },
).filter((tool) => TOOL_NAMES.has(tool.name));
const map = new Map<BotFileToolName, AgentTool>();
for (const tool of selected) {
Expand Down Expand Up @@ -252,7 +255,7 @@ export function buildBotFileTools(options: BotFileToolRouterOptions): AgentTool[

const prepared = locations.map<PreparedLocation>((location) => ({
...location,
tools: underlyingTools(location),
tools: underlyingTools(location, options.fileMutatorBinary),
}));
const byId = new Map(prepared.map((location) => [location.id, location] as const));
const schemaLocations = prepared.map(({ id, label }) => ({ id, label }));
Expand Down
72 changes: 72 additions & 0 deletions main/services/coding-tools.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import { promisify } from "node:util";
import test from "node:test";
import {
buildCodingTools,
buildPinnedCodingTools,
buildSubagentCodingTools,
DISCLOSURE_APPROVAL_TOOL_NAMES,
runCommandEnv,
Expand Down Expand Up @@ -1330,6 +1331,77 @@ test("filesystem tools do not follow workspace symlinks outside the root", async
}
});

test("pinned edit_file cannot overwrite an outside file after a target or ancestor swap", async () => {
for (const swappedPart of ["target", "ancestor"] as const) {
const root = await fs.mkdtemp(path.join(os.tmpdir(), "aiden-edit-root-"));
const outside = await fs.mkdtemp(path.join(os.tmpdir(), "aiden-edit-outside-"));
try {
const directory = path.join(root, "nested");
await fs.mkdir(directory);
const target = path.join(directory, "edit.txt");
const outsideFile = path.join(outside, "edit.txt");
await fs.writeFile(target, "before\n");
await fs.writeFile(outsideFile, "outside\n");
const edit = buildPinnedCodingTools(root, {
beforeWriteCommit: async () => {
if (swappedPart === "target") {
await fs.rename(target, path.join(directory, "original.txt"));
await fs.symlink(outsideFile, target);
} else {
await fs.rename(directory, path.join(root, "original-directory"));
await fs.symlink(outside, directory);
}
},
}).find((tool) => tool.name === "edit_file");
assert.ok(edit);
await assert.rejects(
edit.execute("swap", { path: "nested/edit.txt", old_string: "before", new_string: "after" }),
/changed before the edit could be committed/u,
);
assert.equal(await fs.readFile(outsideFile, "utf8"), "outside\n");
const original = swappedPart === "target"
? path.join(directory, "original.txt")
: path.join(root, "original-directory", "edit.txt");
assert.equal(await fs.readFile(original, "utf8"), "before\n");
} finally {
await fs.rm(root, { recursive: true, force: true });
await fs.rm(outside, { recursive: true, force: true });
}
}
});

test("pinned edit_file follows a stable in-root file symlink", async () => {
const root = await fs.mkdtemp(path.join(os.tmpdir(), "aiden-edit-in-root-"));
try {
const target = path.join(root, "actual.txt");
const alias = path.join(root, "alias.txt");
await fs.writeFile(target, "before\n");
await fs.symlink(target, alias);
const edit = buildPinnedCodingTools(root).find((tool) => tool.name === "edit_file");
assert.ok(edit);
await edit.execute("alias", { path: "alias.txt", old_string: "before", new_string: "after" });
assert.equal(await fs.readFile(target, "utf8"), "after\n");
assert.equal((await fs.lstat(alias)).isSymbolicLink(), true);
} finally {
await fs.rm(root, { recursive: true, force: true });
}
});

test("pinned edit_file keeps ordinary files above the subagent content cap editable", async () => {
const root = await fs.mkdtemp(path.join(os.tmpdir(), "aiden-edit-large-"));
try {
const target = path.join(root, "large.txt");
const original = `${"x".repeat(220_000)}before\n`;
await fs.writeFile(target, original);
const edit = buildPinnedCodingTools(root).find((tool) => tool.name === "edit_file");
assert.ok(edit);
await edit.execute("large", { path: "large.txt", old_string: "before", new_string: "after" });
assert.equal(await fs.readFile(target, "utf8"), original.replace("before", "after"));
} finally {
await fs.rm(root, { recursive: true, force: true });
}
});

test("run_command reports real exit failures through Pi and codemode without re-executing effects", async () => {
const root = await fs.mkdtemp(path.join(os.tmpdir(), "aiden-command-outcome-"));
try {
Expand Down
111 changes: 99 additions & 12 deletions main/services/coding-tools.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,8 @@
// Tool inputs use typebox schemas (pi's AgentTool.parameters), matching tools.ts.

import { isImageAspectRatio } from "../../renderer/shared/image-generation/aspect-ratio.js";
import { spawn } from "node:child_process";
import { execFile, spawn } from "node:child_process";
import { createHash } from "node:crypto";
import { ForegroundReadOperations, ForegroundReadCleanupError, closeForegroundResource } from "./foreground-read-scope.js";
import { StringDecoder } from "node:string_decoder";
import { CodingToolMatchTimeout, withCodingToolGlob, withCodingToolMatcher } from "./coding-tool-matcher.js";
Expand All @@ -28,6 +29,7 @@ import type { RE2 as RE2Matcher } from "re2-wasm";
import { declarePiRuntimeReplay } from "./pi-runtime-tool.js";
import { boundedToolOutput } from "./tool-output-context.js";
import { parseProducedFile, type ProducedFile } from "../../renderer/shared/produced-file.js";
import { resolveSubagentFileMutatorBinary } from "./subagents/subagent-file-mutator-io.js";

const MAX_READ_BYTES = 200_000;
const MAX_OUTPUT_CHARS = 20_000;
Expand All @@ -42,6 +44,7 @@ const MAX_GREP_ENTRIES = 10_000;
const MAX_GREP_BYTES = 10 * 1024 * 1024;
const MAX_GREP_DURATION_MS = 5_000;
const MAX_SEARCH_PATTERN_CHARS = 1_000;
const MAX_PARENT_EDIT_BYTES = 256 * 1024 * 1024;
const SKIP_DIRS = new Set([".git", "node_modules", "dist", "build", ".next", ".cache"]);
let re2Constructor: typeof import("re2-wasm").RE2 | undefined;

Expand Down Expand Up @@ -256,6 +259,7 @@ interface WorkspaceRootGuard {
lexical: string;
canonical: string | null;
identity: Stats | null;
exactIdentity: PinnedWorkspaceRootIdentity | null;
testObserver?: {
beforeDirectoryOpen?(directoryPath: string): void | Promise<void>;
afterDirectoryOpen?(directoryPath: string): void | Promise<void>;
Expand All @@ -281,6 +285,7 @@ function createParentWorkspaceRoot(
lexical: path.resolve(root),
canonical: null,
identity: null,
exactIdentity: null,
testObserver,
};
}
Expand All @@ -293,17 +298,21 @@ function pinWorkspaceRoot(
const lexical = path.resolve(root);
const canonical = realpathSync(lexical);
const identity = statSync(canonical);
const exactStat = statSync(canonical, { bigint: true });
const exactIdentity = {
device: exactStat.dev.toString(),
inode: exactStat.ino.toString(),
};
if (!identity.isDirectory()) throw new Error("The workspace root is not a directory.");
if (expectedIdentity) {
const exactIdentity = statSync(canonical, { bigint: true });
if (
exactIdentity.dev.toString() !== expectedIdentity.device ||
exactIdentity.ino.toString() !== expectedIdentity.inode
exactIdentity.device !== expectedIdentity.device ||
exactIdentity.inode !== expectedIdentity.inode
) {
throw new Error("The authorized workspace root changed before this generation started.");
}
}
return { lexical, canonical, identity, testObserver };
return { lexical, canonical, identity, exactIdentity, testObserver };
}

async function verifyWorkspaceRoot(
Expand Down Expand Up @@ -1076,7 +1085,50 @@ function makeWriteFile(workspace: WorkspaceRootGuard): AgentTool {
};
}

function makeEditFile(workspace: WorkspaceRootGuard): AgentTool {
async function commitPinnedEdit(input: {
binary?: string;
root: string;
rootIdentity: PinnedWorkspaceRootIdentity;
relativePath: string;
fileIdentity: PinnedWorkspaceRootIdentity;
originalSha256: string;
updated: Buffer;
}): Promise<void> {
if (input.updated.length > MAX_PARENT_EDIT_BYTES) {
throw new Error("The edited file exceeds the safe write limit.");
}
const args = [
"edit-file",
input.root,
input.rootIdentity.device,
input.rootIdentity.inode,
input.relativePath,
input.fileIdentity.device,
input.fileIdentity.inode,
input.originalSha256,
String(input.updated.length),
];
await new Promise<void>((resolve, reject) => {
const child = execFile(input.binary ?? resolveSubagentFileMutatorBinary(), args, {
encoding: "utf8",
maxBuffer: 1024,
timeout: 120_000,
}, (error, stdout, stderr) => {
if (error || stdout !== "edited\n") {
reject(new Error(stderr.trim() === "conflict"
? "The workspace file changed before the edit could be committed."
: "The workspace edit could not be completed safely."));
} else {
resolve();
}
});
// The callback owns child errors, including an early native rejection.
child.stdin?.on("error", () => undefined);
child.stdin?.end(input.updated);
});
}

function makeEditFile(workspace: WorkspaceRootGuard, binary?: string): AgentTool {
return {
name: "edit_file",
label: "Edit File",
Expand All @@ -1099,8 +1151,24 @@ function makeEditFile(workspace: WorkspaceRootGuard): AgentTool {
old_string: string;
new_string: string;
};
const { full } = await resolveExistingInRoot(workspace, p, signal);
const original = await fs.readFile(full, "utf-8");
const { root, full } = await resolveExistingInRoot(workspace, p, signal);
const handle = await fs.open(
full,
fsConstants.O_RDONLY | (fsConstants.O_NOFOLLOW ?? 0) | (fsConstants.O_NONBLOCK ?? 0),
);
let source: Buffer;
let fileIdentity: PinnedWorkspaceRootIdentity;
try {
const stat = await handle.stat({ bigint: true });
if (!stat.isFile() || stat.nlink !== 1n) {
throw new Error(`Path "${p}" is not an exclusively linked regular file.`);
}
source = await handle.readFile();
fileIdentity = { device: stat.dev.toString(), inode: stat.ino.toString() };
} finally {
await handle.close();
}
const original = source.toString("utf8");
throwIfAborted(signal, "File edit cancelled.");
const count = original.split(old_string).length - 1;
if (count === 0) throw new Error(`old_string not found in ${p}.`);
Expand All @@ -1112,8 +1180,24 @@ function makeEditFile(workspace: WorkspaceRootGuard): AgentTool {
const updated = original.slice(0, start) + new_string + original.slice(start + old_string.length);
const details = fileMutationDetails(original, updated);
throwIfAborted(signal, "File edit cancelled.");
details.producedFile = parseProducedFile({ relativePath: path.relative(await fs.realpath(workspace.lexical), path.join(await fs.realpath(path.dirname(full)), path.basename(full))).split(path.sep).join("/"), operation: "edited", bytes: Buffer.byteLength(updated) }, "edit_file");
await fs.writeFile(full, updated, "utf-8");
const relativePath = path.relative(root, full).split(path.sep).join("/");
const updatedBytes = Buffer.from(updated, "utf8");
details.producedFile = parseProducedFile({ relativePath, operation: "edited", bytes: updatedBytes.length }, "edit_file");
const rootStat = workspace.exactIdentity === null
? await fs.stat(root, { bigint: true })
: null;
await commitPinnedEdit({
binary,
root,
rootIdentity: workspace.exactIdentity ?? {
device: rootStat!.dev.toString(),
inode: rootStat!.ino.toString(),
},
relativePath,
fileIdentity,
originalSha256: createHash("sha256").update(source).digest("hex"),
updated: updatedBytes,
});
return fileMutationResult(`Edited ${p}.`, details);
},
};
Expand Down Expand Up @@ -1793,6 +1877,8 @@ function makeParentGrep(workspace: WorkspaceRootGuard): AgentTool {
*/
export interface CodingToolOptions {
pathPrefix?: string | (() => string | null | undefined);
/** Trusted native helper from the application installation, if packaged outside Electron. */
fileMutatorBinary?: string;
}

export function runCommandEnv(
Expand Down Expand Up @@ -1962,7 +2048,7 @@ function buildParentCodingToolSet(workspace: WorkspaceRootGuard, options: Coding
declarePiRuntimeReplay(withForegroundReadScope(makeParentListDir(workspace)), "safe"),
declarePiRuntimeReplay(withForegroundReadScope(makeParentGlob(workspace)), "safe"),
declarePiRuntimeReplay(withForegroundReadScope(makeParentGrep(workspace)), "safe"),
declarePiRuntimeReplay(makeEditFile(workspace), "never"),
declarePiRuntimeReplay(makeEditFile(workspace, options.fileMutatorBinary), "never"),
declarePiRuntimeReplay(makeWriteFile(workspace), "never"),
declarePiRuntimeReplay(makeRunCommand(workspace, options), "never"),
];
Expand All @@ -1988,8 +2074,9 @@ export function buildPinnedCodingTools(
testObserver?: WorkspaceRootGuard["testObserver"],
/** Optional authority-proven identity captured before the tool set is built. */
expectedIdentity?: PinnedWorkspaceRootIdentity,
options: CodingToolOptions = {},
): AgentTool[] {
return buildParentCodingToolSet(pinWorkspaceRoot(root, testObserver, expectedIdentity));
return buildParentCodingToolSet(pinWorkspaceRoot(root, testObserver, expectedIdentity), options);
}

/**
Expand Down
Loading
Loading