Skip to content

Commit 3f1c1fe

Browse files
committed
fix(cli): correct verify --paths sandbox probe semantics
Install-test surfaced that sandbox-accept-home and sandbox-accept-tmp probes always fail with 'Access denied' when run outside a project context (e.g. from the filesystem root). The probe input paths did match resolvePath's documented sandbox definition; the real defect was in resolvePath's lookalike-sibling guard. When no project root is detected, resolvePath falls back to process.cwd() as the sandbox projectRoot. If cwd is a filesystem root such as 'C:\' or '/', the root already ends in a separator. isLookalikeSibling compared the target against the root character by character and inspected the character at base.length, which for any descendant of the root is path content (e.g. 'U' of 'Users'), not a separator. Every legitimate descendant was therefore mis-classified as a lookalike sibling and rejected, including paths under homedir() and tmpdir(). Fix strips the trailing separator from the normalized base before the sibling comparison and short-circuits to false when the stripped base is empty (POSIX root) or just a drive letter (Windows root), since a filesystem root cannot have lookalike siblings by construction. Non-root bases retain identical behavior. Regression coverage added: - test/paths.test.ts: resolvePath accepts home/tmp descendants when process.cwd() is the filesystem root. - test/codex-manager-verify-command.test.ts: verify --paths sandbox-accept-home and sandbox-accept-tmp probes succeed when cwd is the filesystem root and no project root is detected. Caught by: npm pack + npm install codex-multi-auth-1.3.0.tgz followed by manual CLI run in a non-project directory whose cwd walked up to a filesystem root.
1 parent 8b6b27a commit 3f1c1fe

3 files changed

Lines changed: 82 additions & 3 deletions

File tree

lib/storage/paths.ts

Lines changed: 16 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -377,13 +377,26 @@ function isWithinDirectory(baseDir: string, targetPath: string): boolean {
377377
* (not a descendant). A trailing separator after the base prefix must be present
378378
* to be considered a proper descendant; anything else (e.g. `home-evil/...`) is a
379379
* lookalike sibling and must be rejected to prevent path-guard bypass.
380+
*
381+
* Filesystem roots (POSIX `/`, Windows `C:\`) are stripped of their trailing
382+
* separator before comparison so that legitimate descendants such as
383+
* `C:\Users\neil\.codex\...` are not misclassified as siblings. A root has no
384+
* siblings by construction, so after the strip we return false early when the
385+
* base is empty (POSIX root) or just a drive letter (Windows root).
380386
*/
381387
function isLookalikeSibling(baseDir: string, targetPath: string): boolean {
382388
const normalizedBase = normalizePathForComparison(baseDir);
383389
const normalizedTarget = normalizePathForComparison(targetPath);
384-
if (normalizedTarget.length <= normalizedBase.length) return false;
385-
if (!normalizedTarget.startsWith(normalizedBase)) return false;
386-
const boundary = normalizedTarget.charAt(normalizedBase.length);
390+
const baseWithoutTrailingSep = normalizedBase.replace(/[\\/]+$/, "");
391+
if (
392+
baseWithoutTrailingSep === "" ||
393+
/^[a-z]:$/.test(baseWithoutTrailingSep)
394+
) {
395+
return false;
396+
}
397+
if (normalizedTarget.length <= baseWithoutTrailingSep.length) return false;
398+
if (!normalizedTarget.startsWith(baseWithoutTrailingSep)) return false;
399+
const boundary = normalizedTarget.charAt(baseWithoutTrailingSep.length);
387400
return boundary !== sep && boundary !== "/" && boundary !== "\\";
388401
}
389402

test/codex-manager-verify-command.test.ts

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -315,4 +315,44 @@ describe("sandbox-reject-escape probe with pathological cwd", () => {
315315

316316
cwdSpy.mockRestore();
317317
});
318+
319+
it("accepts sandbox-accept-home and sandbox-accept-tmp when cwd is the filesystem root", () => {
320+
// Regression for install-test: running `verify --paths` from a
321+
// directory with no detectable project root (where resolvePath falls
322+
// back to process.cwd() for the sandbox projectRoot) must still
323+
// accept paths under homedir() and tmpdir(). The prior lookalike
324+
// sibling check falsely rejected them when cwd was the filesystem
325+
// root because the trailing separator of the root made every
326+
// descendant appear to be a sibling.
327+
setStoragePath(null);
328+
329+
const rootCwd = process.platform === "win32" ? "C:\\" : "/";
330+
const cwdSpy = vi.spyOn(process, "cwd").mockReturnValue(rootCwd);
331+
332+
const deps: VerifyPathsDeps = {
333+
getCwd: () => rootCwd,
334+
findProjectRoot: () => null,
335+
resolveProjectStorageIdentityRoot: () => rootCwd,
336+
getProjectStorageKey: () => "project-rootcwd-probe",
337+
getProjectConfigDir: () => rootCwd,
338+
getProjectGlobalConfigDir: () =>
339+
join(homedir(), ".codex", "multi-auth", "projects", "root-probe"),
340+
resolvePath,
341+
};
342+
343+
const report = runVerifyPathsCheck(deps);
344+
345+
const homeAccept = report.sandboxTests.find(
346+
(test) => test.name === "sandbox-accept-home",
347+
);
348+
const tmpAccept = report.sandboxTests.find(
349+
(test) => test.name === "sandbox-accept-tmp",
350+
);
351+
expect(homeAccept?.rejected).toBe(false);
352+
expect(homeAccept?.ok).toBe(true);
353+
expect(tmpAccept?.rejected).toBe(false);
354+
expect(tmpAccept?.ok).toBe(true);
355+
356+
cwdSpy.mockRestore();
357+
});
318358
});

test/paths.test.ts

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -862,6 +862,32 @@ describe("Storage Paths Module", () => {
862862
expect(() => resolvePath(outsideLookalike)).toThrow("Access denied");
863863
});
864864

865+
it("accepts paths within home/tmp when cwd is the filesystem root", () => {
866+
// Regression: when process.cwd() is `C:\\` (or POSIX `/`) and no
867+
// project root is set, resolvePath used to wrongly classify any
868+
// path starting with the root drive/separator as a "lookalike
869+
// sibling" of the root. That falsely rejected legitimate
870+
// descendants such as `C:\\Users\\neil\\.codex\\...`.
871+
setStoragePathState({
872+
currentStoragePath: null,
873+
currentLegacyProjectStoragePath: null,
874+
currentLegacyWorktreeStoragePath: null,
875+
currentProjectRoot: null,
876+
});
877+
878+
const rootCwd = process.platform === "win32" ? "C:\\" : "/";
879+
const cwdSpy = vi.spyOn(process, "cwd").mockReturnValue(rootCwd);
880+
881+
try {
882+
const homePath = path.join(homedir(), ".codex", "verify-probe");
883+
const tmpPath = path.join(tmpdir(), "verify-probe.tmp");
884+
expect(() => resolvePath(homePath)).not.toThrow();
885+
expect(() => resolvePath(tmpPath)).not.toThrow();
886+
} finally {
887+
cwdSpy.mockRestore();
888+
}
889+
});
890+
865891
it("should handle tilde-only path", () => {
866892
const result = resolvePath("~");
867893
expect(result).toBe(homedir());

0 commit comments

Comments
 (0)