fix(codex): trust extended-length resume paths

[yoke233]

Summary

Fix Codex cold resume on Windows when a persisted rollout uses the extended-length spelling \\?\C:\... but its trusted Codex home uses the equivalent C:\... spelling.

The provenance check now creates a comparison-only copy for strict extended drive paths. File probing and the returned transcript path still use the original path. Other Win32 device namespaces remain rejected.

The final factory stack also preserves the newer fail-closed provenance guard: broad Codex-layout recognition remains separate from the stricter trusted-rollout grammar, so ADS-shaped rollout paths are reported as rejected Codex provenance but cannot become trusted resume sources.

Screenshots

No visual change.

Testing

• pnpm lint (full-repository lint not run; changed-file oxlint and max-lines passed)
• pnpm typecheck (full multi-target typecheck not run; node typecheck passed)
• pnpm test (full suite not run; decisive focused suites and headed E2E passed)
• pnpm build (production build not run; both headed lanes rebuilt in E2E mode)
• pnpm typecheck:node
• Changed-file oxfmt --check and oxlint
• node config/scripts/check-max-lines-ratchet.mjs
• git diff --check
• Native Windows extended-path/security resolver matrix: 9 passed
• Resume argv-drop suite: 10 passed
• IPC unstrippable/drop-resume security cases: 4 passed
• Renderer cold-restore/fresh-banner cases: 2 passed
• Headed Electron cold-resume factory lane: \\?\C:\... rollout resumed after daemon death/restart and the provider session ID was visible in the terminal
• Identical headed disabled oracle: the same rollout started fresh and the user-visible unavailable-resume banner appeared
• Current-main baseline and disabled oracle both reproduce the fresh-session outcome; the factory changes it to resume

The broader resolver/preparation run still has 11 native-Windows failures from inherited POSIX-path fixtures; the same failures reproduce unchanged on pristine main. All decisive extended-path, ADS/device rejection, real-file, argv-drop, IPC fail-closed, and renderer resume cases pass.

AI Review Report

Reviewed the complete four-file diff for ownership, security, performance, cleanup, native Windows behavior, SSH and folder-workspace neutrality, provider scope, and Git 2.25 compatibility. The Codex provenance resolver is the authoritative owner: the comparison fold belongs at the trust boundary, not in global path normalization or renderer/session transport.

The change adds no scanning, commands, IPC, network access, dependencies, or production file mutation. Non-namespace paths are returned unchanged, so macOS/Linux, SSH, remote runtimes, folder workspaces, and Git worktrees keep their existing behavior. No Git command or provider-review behavior changes.

Security Audit

Only strict \\?\<drive>:\... paths can lose the prefix for containment comparison. \\.\..., \\?\GLOBALROOT\..., UNC/volume/device namespaces, different roots, traversal, invalid rollout layouts, ADS-shaped filenames, and missing/non-regular files remain rejected. Rejected originating provenance cannot fall back to scanning another home, and the original transcript path is retained for the regular-file check and resume result.

No workflow, action, package manifest, lockfile, dependency, download, credential, or permission surface changed.

Notes

Windows-specific fix; no behavior change is intended on macOS or Linux. \\?\UNC\... remains intentionally unsupported at this boundary. The headed test uses a hermetic command stub rather than an authenticated Codex CLI, while exercising the real Electron persistence, daemon termination, restart, trust, and user-visible terminal path.

X (Twitter) handle: N/A

ELI5

Windows can spell the same local file as both C:\... and \\?\C:\.... Orca now compares those spellings as the same trusted Codex rollout without weakening the checks that reject device paths, network namespaces, or alternate data streams.

[coderabbitai]

📝 Walkthrough

Walkthrough

The change adds trusted normalization for Windows extended-length and device-style paths before Codex rollout containment checks. Tests cover accepted extended-length paths, unsafe namespace paths, missing or non-regular files, compressed rollouts, scan filtering, and prevention of fallback session scanning. The PTY connection persistence test now includes a Windows extended-length transcript path in stored provider session data and the expected resume request.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Title check	✅ Passed	The title clearly and concisely identifies the extended-length Codex resume path fix.
Description check	✅ Passed	The description covers the required summary, screenshots, testing, AI review, security audit, and platform-specific notes.

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[OrcaWin]

Taking a look!

[OrcaWin]

Reviewed on real Windows 11 + WSL. The approach is right and I didn't change it: folding only the strict \\?\<drive>:\ spelling for comparison, while keeping the original path for the file probe and the returned transcript path, is the correct shape. Normalizing toward the namespaced form would break the containment check's deliberate host-OS independence (it has to hold on POSIX hosts and for SSH/remote too).

Why it's safe: the relative path is slash-folded before ROLLOUT_RELATIVE_PATH, which forces exactly four components (\d{4}/\d{2}/\d{2}/rollout-….jsonl[.zst]). So .. cannot survive the check, and it can't hide in the prefix either, because the prefix must string-equal sessionsRoot modulo case and separators. Every mismatch I could construct failed closed, never open.

(Correcting an earlier version of this comment: I first argued the safety came from \\?\ doing no normalization, so the probed path was the most literal reading. That premise is wrong — Node/libuv normalizes \\?\ paths before the syscall, and I confirmed lstatSync succeeds on \\?\…\20\..\20\…. The layout regex above is the load-bearing reason, not path literalness. Relatedly, preserving the original spelling is a sound conservative choice rather than a long-path necessity — a plain 315-character path stats fine here.)

Verified by executing it on Windows, not by reading it

• Reproduced the bug and confirmed the fix at the real seam. In a live dev build, resuming a real Codex rollout at \\?\C:\Users\<me>\.codex\sessions\...:

  • without this change → pty:spawn fails with "Orca could not verify the originating Codex session file…", thrown from prepareCodexSessionResumeForLaunch (src/main/index.ts:854)
  • with this change → resume spawns normally

  I checked the built main bundle on both runs to be sure the intended code was actually live.

• Negative controls stayed rejected in those same live runs: \\.\C:\… and a path outside every trusted home.

• Tried to break the normalization against real files: \\.\, \\?\GLOBALROOT\Device\…, \\?\Volume{GUID}\, \\?\UNC\localhost\C$\, \\.\UNC\…, plain \\localhost\C$\, \\?\\C:\, prefix-with-no-drive, and an ADS :stream suffix — all refused.

• Long paths: real 376/379-character paths on disk resolve through the extended form, including inside Electron's own runtime against the bundled module.

• Real data: 227 rollout files in my ~/.codex — both spellings resolve to the same home, and the legacy id scan over that tree runs in ~74ms. Measured separately, the added per-entry work is ~44ns against ~326µs per entry, and that ratio is fixed at every tree size, so it isn't a hot-path concern.

• WSL unaffected: a file created inside WSL and read from Windows via \\wsl.localhost\<distro>\… still resolves, and the wsl$ alias still folds. src/main/index.ts:838 also returns early for runtime === 'wsl'.

What I pushed

• e05932fb9 — tests only. codex-session-resume-home.test.ts now covers the parts of the fold that weren't pinned: the trusted-root side (extended-length home + normal-form path), the .zst sibling derived from an extended-length persisted path (the one place the returned path is derived rather than echoed), and the legacy id-scan call site (extended entries accepted, device namespaces refused). All three fail with the production change reverted.
• 0b8706b25 — comment accuracy only, no behavior. The helper's comment described the rejected spellings as "arbitrary device namespaces", but \\?\UNC\server\share\… is the extended-length form of \\server\share\… — a network share, not a device, and win32.toNamespacedPath produces it from ordinary UNC input. Reading it as a security decision hid the fact that UNC simply isn't folded yet, so it now says what's actually excluded. Also trimmed my two new test comments to one line to match the file.

I also checked the four tests this PR already added: the positive one fails on revert, and each of the three rejection tests is caught by a distinct plausible mis-implementation of the helper. None are vacuous.

Deliberately left alone

• The [\s\S]* in the regex is correct — don't "simplify" it. I tested the two arguments for narrowing it. Narrowing [\\/] to \\ would be a regression: \\?\C:/forward/slashes does open on Windows (libuv normalizes), so the current form accepts a spelling that genuinely works. And swapping [\s\S]* for .* doesn't silently truncate a trailing newline — it fails to match at all. The authored version is the right choice.
• Not renaming the helper. "Trusted"/"Copy" are fair to quibble with, but it's subjective naming churn on a security-sensitive function.
• Keeping the explicit return null rejections. They're arguably redundant today — a \\?\…/\\.\… candidate can never prefix-match a programmatically-joined plain trusted home — but on a trust boundary I'd rather the rejection be explicit than rely on that invariant holding elsewhere.
• Not hoisting the loop-invariant sessions-root conversion out of the scan loop (see the numbers above), and note codex-session-resume-home.ts:106-109 returns before scanning whenever a transcript path is set, so this PR's own scenario never reaches that loop.
• The renderer pty-connection.test.ts fixture change stays, though this one was genuinely split — one reviewer argued for reverting it, on the accurate grounds that the \\?\ value is inert there (the transport is mocked, so the main-process check never runs) and the assertion overlaps the sibling at pty-connection.test.ts:7656-7664. Recording that for the next reader: what it actually covers is renderer sleeping-record plumbing, not Win32 path handling — the renderer just forwards an opaque string. I left it because it still pins the precondition and removing it only deletes coverage.
• CodeRabbit left no line comments to action.

Known gaps / suggested follow-ups

• \\?\UNC\… is still not folded, and that's the same bug for some users. Where homedir() is a UNC roaming profile or the Codex home sits on a network share, the trusted home is \\server\users\bob\.codex while a canonicalizing Codex would report \\?\UNC\server\users\bob\… — rejected before and after this PR, so that cohort still gets the hard "could not verify" error. Out of scope here, but the natural next step.
• Legacy shared-mirror migration silently no-ops for extended paths. src/main/index.ts:867 passes the still-\\?\-spelled transcript path into prepareLegacySharedCodexSessionResume; win32.relative(plainRoot, extPath) returns the absolute extended path unchanged, so isLegacyRolloutRelativePath (codex-legacy-session-resume.ts:156) sees resolve(rel) === rel and the call throws RETRYABLE_RESUME_ERROR, which index.ts:875-881 swallows with a warn. Net effect on that lane: resume works but never migrates to the real home, and warns every time. Still strictly better than the pre-PR hard failure, and it resumes against the provenance-correct home, so not a blocker — but a conscious follow-up. My live test used the real home, so it didn't reach this branch.
• My ~/.codex had no persisted \\?\ transcript path, so I confirmed the mechanism (hook payloads stored verbatim; rejected provenance hard-fails) rather than watching the CLI emit that spelling. One logged real transcript_path from a Windows Codex would settle it.
• Pre-existing and out of scope: three tests in this file fail on Windows because they use POSIX fixtures that get win32-joined. They fail identically on main, but it does mean part of this file's Windows coverage is inert.
• Junction traversal inside a trusted home behaves identically with and without this change.

Nice fix — the containment reasoning and the fail-closed handling of the other namespaces were already right, which made this easy to verify.

Ready to merge from my side — scope unchanged (3 files, production change still the one helper), CI green on 0b8706b25.

[greptile-apps]

Greptile Summary

This PR fixes Codex cold resume on Windows when a persisted rollout uses the extended-length path spelling \\?\C:\... but the trusted Codex home is recorded as the ordinary C:\... form. The comparison logic in isCodexRolloutInsideSessionsRoot now strips the \\?\ prefix from drive paths before comparing — while keeping the original path intact for file probing and the returned transcript value.

• Adds toCodexTrustedPathComparisonCopy, which normalises extended-length drive paths for containment comparison only, and returns null for \\.\ device namespaces or any non-drive \\?\ form (GLOBALROOT, Volume GUIDs, UNC), so those remain rejected.
• Splits DATED_ROLLOUT_TAIL into CLAIMED_CODEX_ROLLOUT_TAIL (for the layout-detection predicate that drives UX reporting) and TRUSTED_CODEX_ROLLOUT_TAIL (used in provenance trust), where the trusted variant additionally excludes : to block NTFS Alternate Data Stream paths.
• Expands the test suite in both the home-resolver and pty-connection test files to cover extended-length acceptance, ADS rejection, traversal, namespace rejection, fail-closed provenance, and the sleeping Codex cold-resume seam.

Confidence Score: 5/5

Safe to merge — the change is narrowly scoped to path comparison logic in the Codex resume provenance check, does not touch file I/O paths or returned values, and all rejection invariants are preserved and tested.

The production change is a single, well-contained helper that strips the Win32 extended-drive prefix only for the containment comparison, with no effect on POSIX paths. The regex correctly rejects every non-drive \?\ form and the \. device namespace. ADS blocking is sound via the [^/:] character class in the trusted tail regex. File probing and the returned transcript path continue to use the original spelling. The test suite covers every relevant rejection and acceptance branch, including the scan code path and fail-closed provenance behaviour.

Files Needing Attention: No files require special attention.

Important Files Changed

Filename	Overview
src/main/codex/codex-session-resume-home.ts	Adds toCodexTrustedPathComparisonCopy to strip the extended-length prefix from drive paths for comparison only; correctly rejects device namespaces and non-drive extended paths; splits rollout tail regex into claimed vs trusted variants to block ADS paths in provenance checks while preserving the original path for file I/O and the returned transcript value.
src/main/codex/codex-session-resume-home.test.ts	Adds comprehensive regression coverage for extended-length path acceptance, original path preservation, ADS/namespace/traversal rejection before file probing, fail-closed provenance, and the scan path.
src/main/codex/codex-session-resume-windows.test.ts	New Windows-native integration test exercising the full prepareCodexSessionResume path with a real filesystem and toNamespacedPath; gated behind process.platform === 'win32' so it is a no-op on POSIX CI.
src/renderer/src/components/terminal-pane/pty-connection.test.ts	Updates the sleeping-Codex cold-resume test to carry an extended-length transcriptPath in the providerSession, verifying the path is forwarded intact through the resume flow to transport.connect.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[persistedPath from sleeping session] --> B{toCodexTrustedPathComparisonCopy}
    B -->|starts with device namespace| C[return null - reject]
    B -->|starts with extended prefix| D{matches drive-letter regex?}
    D -->|No - GLOBALROOT / Volume / UNC| E[return null - reject]
    D -->|Yes| F[strip prefix to ordinary drive path]
    B -->|ordinary path| G[return unchanged]
    F --> H[relativePathInsideRoot on comparison copies]
    G --> H
    H -->|not inside sessions root| I[reject - no file probe]
    H -->|inside sessions root| J{ROLLOUT_RELATIVE_PATH regex}
    J -->|fails - ADS colon / traversal / wrong layout| I
    J -->|passes| K[resolveExistingRolloutPath on ORIGINAL path]
    K -->|file missing or non-regular| I
    K -->|file exists| L[return homePath + original transcriptPath]

Loading

_{Reviews (2): Last reviewed commit: "fix(codex): preserve resume provenance g..." | Re-trigger Greptile}