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
3 changes: 3 additions & 0 deletions packages/coding-agent/changelog.d/win-file-id-precision.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
### Fixed

- File identity comparisons for user-agent and containment-root isolation now use exact bigint precision end-to-end: stat calls capture full 64-bit device and inode IDs, and comparisons are direct bigint equality without rounding. This prevents a security regression on Windows where replaced directories in the same double-rounding bucket could pass isolation checks.
46 changes: 27 additions & 19 deletions packages/coding-agent/src/capability/fs.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,8 @@ const dirCache = new Map<string, fs.Dirent[]>();
export type ReadScope = "project" | "user" | "native";

export interface FileIdentity {
readonly dev: number;
readonly ino: number;
readonly dev: bigint;
readonly ino: bigint;
}

export interface ReadFileOptions {
Expand Down Expand Up @@ -45,7 +45,7 @@ function isWithinOrEqual(root: string, candidate: string): boolean {
async function isHomeIdentityStable(options?: ReadFileOptions): Promise<boolean> {
if (!options?.isolatedHome || !options.home || !options.homeIdentity) return true;
try {
const current = await fs.promises.stat(options.home);
const current = await fs.promises.stat(options.home, { bigint: true });
return current.dev === options.homeIdentity.dev && current.ino === options.homeIdentity.ino;
} catch {
return false;
Expand All @@ -69,7 +69,7 @@ async function isUserAgentIdentityStable(options?: ReadFileOptions): Promise<boo
}
}
try {
const current = await fs.promises.stat(options.userAgentDir);
const current = await fs.promises.stat(options.userAgentDir, { bigint: true });
return current.dev === options.userAgentIdentity.dev && current.ino === options.userAgentIdentity.ino;
} catch {
return false;
Expand All @@ -79,7 +79,7 @@ async function isUserAgentIdentityStable(options?: ReadFileOptions): Promise<boo
async function isContainmentRootIdentityStable(options?: ReadFileOptions): Promise<boolean> {
if (!options?.isolatedHome || !options.containmentRoot || !options.containmentRootIdentity) return true;
try {
const current = await fs.promises.stat(options.containmentRoot);
const current = await fs.promises.stat(options.containmentRoot, { bigint: true });
return sameFileIdentity(current, options.containmentRootIdentity);
} catch {
return false;
Expand Down Expand Up @@ -152,8 +152,8 @@ async function isCurrentIsolatedPath(abs: string, options?: ReadFileOptions): Pr
}
}

function isSingleLinkRegularFile(stat: fs.Stats): boolean {
return stat.isFile() && !stat.isSymbolicLink() && stat.nlink === 1;
function isSingleLinkRegularFile(stat: fs.Stats | fs.BigIntStats): boolean {
return stat.isFile() && !stat.isSymbolicLink() && (stat.nlink === 1n || stat.nlink === 1);
}

/** Reject hard-linked executable or configuration files in isolated discovery. */
Expand All @@ -165,22 +165,30 @@ export async function isSingleLinkRegularFileAt(filePath: string): Promise<boole
}
}

function sameFileIdentity(left: Pick<fs.Stats, "dev" | "ino">, right: Pick<fs.Stats, "dev" | "ino">): boolean {
return left.dev === right.dev && left.ino === right.ino;
function sameFileIdentity(
left: Pick<fs.Stats, "dev" | "ino"> | Pick<fs.BigIntStats, "dev" | "ino">,
right: Pick<fs.Stats, "dev" | "ino"> | Pick<fs.BigIntStats, "dev" | "ino"> | FileIdentity,
): boolean {
// Convert to bigint if needed for comparison
const leftDev = typeof left.dev === "bigint" ? left.dev : BigInt(left.dev);
const leftIno = typeof left.ino === "bigint" ? left.ino : BigInt(left.ino);
const rightDev = typeof right.dev === "bigint" ? right.dev : BigInt(right.dev);
const rightIno = typeof right.ino === "bigint" ? right.ino : BigInt(right.ino);
return leftDev === rightDev && leftIno === rightIno;
}

/**
* Capture the identity of the currently-authorized path before opening it.
* Stat first and canonicalize second so a parent replacement in either seam
* is detected by the descriptor identity check below.
*/
async function statAuthorizedPath(abs: string, options?: ReadFileOptions): Promise<fs.Stats | null> {
async function statAuthorizedPath(abs: string, options?: ReadFileOptions): Promise<fs.Stats | fs.BigIntStats | null> {
if (!options?.isolatedHome) return null;
if (!(await isHomeIdentityStable(options))) return null;
if (!(await isUserAgentIdentityStable(options))) return null;
let authorized: fs.Stats;
let authorized: fs.Stats | fs.BigIntStats;
try {
authorized = await fs.promises.stat(abs);
authorized = await fs.promises.stat(abs, { bigint: true });
} catch {
return null;
}
Expand All @@ -191,18 +199,18 @@ async function validateOpenedPath(
handle: fsPromises.FileHandle,
abs: string,
options: ReadFileOptions | undefined,
authorized: fs.Stats | null,
authorized: fs.Stats | fs.BigIntStats | null,
kind: "file" | "directory",
): Promise<boolean> {
if (!options?.isolatedHome) return true;
if (!(await isHomeIdentityStable(options))) return false;
if (!(await isUserAgentIdentityStable(options))) return false;
try {
const opened = await handle.stat();
const opened = await handle.stat({ bigint: true });
const validKind =
kind === "file" ? isSingleLinkRegularFile(opened) : opened.isDirectory() && !opened.isSymbolicLink();
if (!validKind || authorized === null || !sameFileIdentity(opened, authorized)) return false;
const current = await fs.promises.stat(abs);
const current = await fs.promises.stat(abs, { bigint: true });
if (!sameFileIdentity(opened, current)) return false;
return await isCurrentIsolatedPath(abs, options);
} catch {
Expand All @@ -212,7 +220,7 @@ async function validateOpenedPath(

interface OpenedPath {
handle: fsPromises.FileHandle;
authorized: fs.Stats | null;
authorized: fs.Stats | fs.BigIntStats | null;
}

async function openIsolatedFile(abs: string, options?: ReadFileOptions): Promise<OpenedPath | null> {
Expand Down Expand Up @@ -279,7 +287,7 @@ export async function readFile(filePath: string, options?: ReadFileOptions): Pro
/** Capture a stable physical identity for an authorized containment root. */
export async function capturePathIdentity(filePath: string): Promise<FileIdentity | null> {
try {
const stat = await fs.promises.stat(path.resolve(filePath));
const stat = await fs.promises.stat(path.resolve(filePath), { bigint: true });
return { dev: stat.dev, ino: stat.ino };
} catch {
return null;
Expand Down Expand Up @@ -342,14 +350,14 @@ export async function readFileSize(filePath: string, options?: ReadFileOptions):
async function validateDirectoryPath(
abs: string,
options: ReadFileOptions | undefined,
authorized: fs.Stats | null,
authorized: fs.Stats | fs.BigIntStats | null,
): Promise<boolean> {
if (!options?.isolatedHome) return true;
if (authorized === null) return false;
if (!(await isHomeIdentityStable(options))) return false;
if (!(await isUserAgentIdentityStable(options))) return false;
try {
const current = await fs.promises.stat(abs);
const current = await fs.promises.stat(abs, { bigint: true });
return (
current.isDirectory() && sameFileIdentity(current, authorized) && (await isCurrentIsolatedPath(abs, options))
);
Expand Down
4 changes: 2 additions & 2 deletions packages/coding-agent/src/capability/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -482,7 +482,7 @@ export async function loadCapabilityForHome<T>(
// read another profile's SYSTEM/RULES/AGENTS, skills, commands, hooks,
// settings, or executable descriptors.
const canonicalHome = await canonicalizeThroughExistingAncestor(resolvedHome);
const homeStats = await fs.stat(canonicalHome);
const homeStats = await fs.stat(canonicalHome, { bigint: true });
const homeIdentity = { dev: homeStats.dev, ino: homeStats.ino };
const trustedHome = await canonicalizeThroughExistingAncestor(getTrustedHomeDir());
const isolatedHome = canonicalHome !== trustedHome;
Expand Down Expand Up @@ -510,7 +510,7 @@ export async function loadCapabilityForHome<T>(
"user agent directory",
)
: path.join(canonicalHome, getConfigDirName(), "agent");
const userAgentStats = await fs.stat(userAgentDir).catch(() => null);
const userAgentStats = await fs.stat(userAgentDir, { bigint: true }).catch(() => null);
const userAgentIdentity = userAgentStats ? { dev: userAgentStats.dev, ino: userAgentStats.ino } : null;
await assertExplicitHomeRoots(canonicalHome, cwd, Boolean(options.agentDir), effectiveProviderIds, isolatedHome);
const repoRootCandidate = await findRepoRoot(cwd, canonicalHome, {
Expand Down
2 changes: 1 addition & 1 deletion packages/coding-agent/src/discovery/agents-md.ts
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ async function readBoundedAgentsMdFile(
const homeIsStable = async (): Promise<boolean> => {
if (!options?.home || !options.homeIdentity) return true;
try {
const current = await fs.stat(options.home);
const current = await fs.stat(options.home, { bigint: true });
return current.dev === options.homeIdentity.dev && current.ino === options.homeIdentity.ino;
} catch {
return false;
Expand Down
3 changes: 1 addition & 2 deletions packages/coding-agent/src/discovery/helpers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1005,8 +1005,7 @@ export async function readContainedFile(
rootIdentity = capturedRoot;
if (
expectedRootIdentity !== undefined &&
(rootIdentity.dev !== BigInt(expectedRootIdentity.dev) ||
rootIdentity.ino !== BigInt(expectedRootIdentity.ino))
(rootIdentity.dev !== expectedRootIdentity.dev || rootIdentity.ino !== expectedRootIdentity.ino)
)
return null;
await SkillDiscoveryTestHooks.afterContainedRootValidated?.(root);
Expand Down
74 changes: 74 additions & 0 deletions packages/coding-agent/test/discovery-identity-precision.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,74 @@
import { describe, expect, it } from "bun:test";
import * as fs from "node:fs";
import * as os from "node:os";
import * as path from "node:path";
import { capturePathIdentity } from "../src/capability/fs";
import { readContainedFile } from "../src/discovery/helpers";

describe("File identity exact bigint precision", () => {
it("captures exact large file IDs that exceed 2^53", async () => {
// Use the actual filesystem to verify exact ID capture.
// Create a temporary directory and stat it with bigint: true.
const tempDir = await fs.promises.mkdtemp(path.join(os.tmpdir(), "gjc-id-precision-"));
try {
const stat = await fs.promises.stat(tempDir, { bigint: true });
const captured = await capturePathIdentity(tempDir);
expect(captured).not.toBeNull();
expect(captured!.dev).toBe(stat.dev);
expect(captured!.ino).toBe(stat.ino);
// Verify that the captured identity is bigint, not Number
expect(typeof captured!.dev).toBe("bigint");
expect(typeof captured!.ino).toBe("bigint");
} finally {
await fs.promises.rm(tempDir, { force: true, recursive: true });
}
});

it("rejects mismatched identities when comparing contained files", async () => {
// Create two temp directories and verify that mismatched identities are rejected.
const tempDir1 = await fs.promises.mkdtemp(path.join(os.tmpdir(), "gjc-id-precision-1-"));
const tempDir2 = await fs.promises.mkdtemp(path.join(os.tmpdir(), "gjc-id-precision-2-"));
try {
// Capture the true identity of tempDir1
const identity1 = await capturePathIdentity(tempDir1);
expect(identity1).not.toBeNull();

// Create a test file in tempDir2
const testFile = path.join(tempDir2, "test.txt");
await Bun.write(testFile, "test content");

// readContainedFile should reject a file when given the wrong directory's identity
const resultWrongIdentity = await readContainedFile(tempDir2, testFile, identity1);
expect(resultWrongIdentity).toBeNull();

// Capture the true identity of tempDir2 and verify it matches
const identity2 = await capturePathIdentity(tempDir2);
expect(identity2).not.toBeNull();
const resultCorrectIdentity = await readContainedFile(tempDir2, testFile, identity2);
expect(resultCorrectIdentity).toBe("test content");

// Verify that the identities are different (at least in inode)
expect(identity1!.ino).not.toBe(identity2!.ino);
} finally {
await fs.promises.rm(tempDir1, { force: true, recursive: true });
await fs.promises.rm(tempDir2, { force: true, recursive: true });
}
});

it("rejects an expected identity that differs from the root by a single inode", async () => {
// readContainedFile must compare bigint identities exactly: an off-by-one inode is a
// different directory even where a double would round both to the same value.
const root = await fs.promises.mkdtemp(path.join(os.tmpdir(), "gjc-id-precision-"));
try {
const file = path.join(root, "SYSTEM.md");
await Bun.write(file, "authorized");
const identity = await capturePathIdentity(root);
expect(identity).not.toBeNull();

expect(await readContainedFile(root, file, identity)).toBe("authorized");
expect(await readContainedFile(root, file, { dev: identity!.dev, ino: identity!.ino + 1n })).toBeNull();
} finally {
await fs.promises.rm(root, { force: true, recursive: true });
}
});
});
2 changes: 1 addition & 1 deletion packages/coding-agent/test/discovery/helpers.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -508,7 +508,7 @@ describe("safe discovery boundaries", () => {
await fs.mkdir(replacement, { recursive: true });
await fs.writeFile(path.join(agentDir, "SYSTEM.md"), "authorized");
await fs.writeFile(path.join(replacement, "SYSTEM.md"), "replacement");
const identity = await fs.stat(agentDir);
const identity = await fs.stat(agentDir, { bigint: true });
await fs.rm(agentDir, { recursive: true });
await fs.symlink(replacement, agentDir, "dir");

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -959,7 +959,7 @@ describe("PR #4834: loadCapabilityForHome never falls back to the process profil
await seedProfile(outsideAgentDir, "outside");
await fs.mkdir(path.dirname(homeAgentDir), { recursive: true });

const homeStats = await fs.stat(home);
const homeStats = await fs.stat(home, { bigint: true });
const result = await readFile(path.join(homeAgentDir, "SYSTEM.md"), {
isolatedHome: true,
home,
Expand Down
Loading