Repository navigation
fix(discovery): compare contained-root identity at plain-stat precision (Windows #4834 flake) #6634
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 2 commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| 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. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,75 @@ | ||
| import { describe, expect, it } from "bun:test"; | ||
| import * as fs from "node:fs"; | ||
| 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("/tmp", "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("/tmp", "gjc-id-precision-1-")); | ||
| const tempDir2 = await fs.promises.mkdtemp(path.join("/tmp", "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 fs.promises.writeFile(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 two distinct inode values that round to the same double", () => { | ||
| // Demonstrate that two different bigint values can round to the same Number. | ||
| // At 2^60, rounding distance is 512. | ||
| // 2^60 + 1 and 2^60 + 2 round to the same double. | ||
| const large1: bigint = 2n ** 60n + 1n; | ||
| const large2: bigint = 2n ** 60n + 2n; | ||
|
|
||
| const double1 = Number(large1); | ||
| const double2 = Number(large2); | ||
|
|
||
| // They round to the same value | ||
| expect(double1).toBe(double2); | ||
| // But the bigints are different | ||
| expect(large1).not.toBe(large2); | ||
|
|
||
| // With exact bigint comparison, they should not match | ||
| expect(large1 === large2).toBe(false); | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
This assertion only proves that two bigint literals differ and never invokes AGENTS.md reference: AGENTS.md:L169-L173 Useful? React with 👍 / 👎. |
||
| }); | ||
| }); | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
On Windows hosts where the current drive has no
\tmpdirectory,mkdtemp(path.join("/tmp", ...))fails withENOENTbefore this regression runs; the second test repeats the same assumption. Useos.tmpdir()so the new suite can run on the platform whose file-identity behavior it targets.Useful? React with 👍 / 👎.