Skip to content

[Bug]: Native-main refresh publication can overwrite external Codex writers #2999

Closed

Description

@MarcTCruz

Client or integration

Codex CLI

Area

Authentication and account pool

Summary

Current dev (9cd4e4b8efc055ec8c8f7f9fd76d7c556a065fa6) contains the native-main refresh implementation merged for #2221, but its publication and coordination protocol still has two credential-safety races.

The publisher hashes auth.json, checks it again, and then renames a staged file over it. A Codex writer that replaces auth.json between the final check and rename can still be overwritten. Native-main refresh also reuses the pool refresh lock under OPENCODEX_HOME; two OpenCodex instances with different OPENCODEX_HOME values but the same canonical CODEX_HOME therefore do not coordinate the file they both write.

Expected behavior: native-main refresh coordinates by canonical CODEX_HOME, preserves an external writer atomically, recovers deterministically after process death, and never removes the canonical auth.json target during publication.

Reproduction

  1. Check out current dev and install the frozen dependency tree:

    git checkout dev
    git pull --ff-only origin dev
    bun install --frozen-lockfile
  2. Create one temporary CODEX_HOME/auth.json containing an expired access token and a valid refresh token.

  3. Start two child processes with that same CODEX_HOME and distinct OPENCODEX_HOME directories. Have both request native-main refresh while counting OAuth refresh callbacks.

  4. Observe that the existing pool-lock namespace does not serialize the two processes by canonical Codex home.

  5. At the existing final pre-rename test hook in src/codex/main-account.ts, atomically replace auth.json from a simulated Codex writer. Resume the native-main publisher.

  6. Observe that the staged credential can overwrite the external writer after the last hash check.

A deterministic regression is included in the linked fix branch:

./node_modules/bun/bin/bun.exe test --preload ./tests/preload.ts \
  tests/atomic-file-preserving-replace.test.ts \
  tests/codex-main-account-refresh.test.ts \
  tests/native-main-refresh-process.test.ts

The test asserts one physical refresh across different OPENCODEX_HOME values, byte-for-byte preservation of an external writer, process-death recovery, and stable canonical target presence.

Version

dev at 9cd4e4b8efc055ec8c8f7f9fd76d7c556a065fa6

Operating system

Ubuntu 24.04 (Linux 6.8); macOS and Windows replacement adapters require CI runtime coverage

Provider and model

OpenAI native main account / gpt-5.5

Logs or error output

No token values are logged. The failure is observed as either more than one refresh callback for one CODEX_HOME or an auth.json SHA-256 different from the external writer's expected SHA-256.

Screenshots and supporting files

Not applicable. The linked regression uses temporary homes, child processes, SHA-256 identities, and loopback-only fixtures.

Redacted configuration

{
  "CODEX_HOME": "/tmp/ocx-native-main-repro/codex",
  "OPENCODEX_HOME_A": "/tmp/ocx-native-main-repro/ocx-a",
  "OPENCODEX_HOME_B": "/tmp/ocx-native-main-repro/ocx-b"
}

Checks

  • I searched existing issues and documentation.
  • I removed secrets, tokens, account details, request credentials, and personal data.

Activity

  1. added
    bugSomething isn't working
    account-poolOAuth, credentials, Codex pool, quota, failover, plans
    on Aug 30, 2026
  2. lidge-jun commented on Aug 30, 2026

    @lidge-jun
    Owner

    리뷰 · 우선순위 64 / 80

    지금 dev HEAD는 9cd4e4b8e이고 패키지는 2.36.0입니다. #2848이 이미 들어가 있습니다. 그 작업은 #2221을 고치려고, OpenCodex가 Codex CLI가 쓰는 CODEX_HOME/auth.json을 직접 읽고, 만료된 액세스 토큰을 리프레시 토큰으로 갈아 끼운 뒤 같은 파일에 다시 쓰게 했습니다. 이 이슈는 그 살아 있는 길이 아직 두 곳에서 위험하다고 말합니다. 저는 HEAD 코드를 다시 봤고, 버그는 아직 그대로입니다. 고치는 새 파일은 HEAD에 없습니다.

    첫 번째 구멍은 파일을 덮어쓰는 순간입니다. src/codex/main-account.ts의 persistRefreshedMainAuthJson은 먼저 auth.json 전체를 읽어서 SHA-256을 기억합니다. 그다음 atomicWriteFile에 임시 파일을 만들고, beforeRename과 validateBeforeRename에서 해시가 같은지 다시 본 다음, 임시 파일 이름을 auth.json으로 바꿉니다. 마지막 확인과 이름 바꾸기 사이에 Codex CLI 같은 다른 프로그램이 그 파일을 통째로 바꿔 두면, OpenCodex는 새 내용을 보지 못합니다. 이름 바꾸기는 그냥 덮어씁니다. Codex가 방금 저장한 토큰이 사라지고, OpenCodex가 들고 있던 예전 스냅샷이 남습니다. 해시를 두 번 보는 것은 도움이 되지만, 확인과 rename 사이의 틈은 막지 못합니다.

    두 번째 구멍은 누가 같은 파일을 고치는지 줄을 세우는 잠금입니다. 리프레시는 withCodexRefreshFileLock으로 한 줄로 섭니다. 그 잠금 파일은 src/codex/account-store.ts의 codexRefreshLockPath가 getConfigDir() 아래에 만듭니다. getConfigDir()은 OPENCODEX_HOME이고, 없으면 ~/.opencodex입니다. 실제로 쓰는 파일은 CODEX_HOME/auth.json입니다. 그래서 CODEX_HOME은 같고 OPENCODEX_HOME만 다른 두 OpenCodex는 서로 다른 잠금 폴더를 봅니다. 같은 auth.json을 동시에 고칩니다. 리프레시가 두 번 나가거나, 한쪽이 다른 쪽이 쓴 결과를 덮어쓸 수 있습니다.

    이미 HEAD에 있는 src/codex/native-main-claim.ts, src/codex/native-main-owner.ts, src/codex/native-profile-store.ts는 이 네이티브 메인 리프레시 길에 아직 연결되어 있지 않습니다. 이슈가 말하는 고침 파일 src/codex/native-main-refresh-publication.ts와 src/lib/atomic-file-preserving-replace.ts는 HEAD에 없습니다. 연결된 고침은 초안 PR #3000 하나입니다. 예전 초안 #2497은 이미 닫혀 있습니다. 이 이슈도 #3000도 그 초안을 다시 열지 말라고 분명히 적혀 있습니다. 같은 주제로 새 브랜치를 또 열 필요는 없습니다.

    이 구멍은 계정 풀 레인의 자격 증명 안전 문제입니다. 문서 로케일 맞춤(#2980)이나 카탈로그 다운로드(#2979)보다, 지금 dev에서 실제로 auth.json을 쓰는 길을 단단히 하는 일이 먼저입니다. #2848이 연 길을 이 이슈가 이어서 막습니다. types.ts/config.ts 쪼개기와는 겹치지 않습니다. 미리보기 배포도 이 일과 무관합니다.

    라인 15 - src/codex/main-account.ts가 풀 계정용 withCodexRefreshFileLock을 그대로 가져옵니다. 네이티브 메인 파일과 잠금 위치가 다릅니다.
    라인 149-162 - persistRefreshedMainAuthJson이 atomicWriteFile의 beforeRename에서 해시를 보고 바로 rename합니다. 그 사이에 들어온 외부 작성자 파일을 보존하지 않습니다.
    라인 191-192 - 리프레시 잠금 키가 리프레시 토큰 지문이고, withCodexRefreshFileLock 호출이 그 키로 잠급니다. 잠금 파일은 OPENCODEX_HOME 아래에 생깁니다.
    경로 src/codex/account-store.ts codexRefreshLockPath / withCodexRefreshFileLock - 잠금은 getConfigDir() 아래 codex-refresh-*.lock입니다. CODEX_HOME이 아닙니다.
    경로 src/config/paths.ts getConfigDir - OPENCODEX_HOME이 잠금 집을 바꿉니다. CODEX_HOME은 바꾸지 않습니다.
    경로 src/codex/native-main-refresh-publication.ts, src/lib/atomic-file-preserving-replace.ts - 이슈가 말하는 고침 파일은 아직 HEAD에 없습니다.

    메인테이너의 판단이 필요한 지점

    • #3000을 이 이슈의 유일한 고침으로 둘지. 같은 레이스를 다른 브랜치로 다시 짜지 말지
    • #2497은 이미 닫혀 있습니다. 그 초안을 다시 열지 말지. 이 이슈와 fix(codex): harden native main refresh publication #3000 본문도 다시 열지 말라고 적혀 있습니다
    • #2848이 연 네이티브 메인 쓰기 길을, 외부 Codex 작성자를 보존하는 교환이 올 때까지 그대로 둘지
    • 라벨 bug / account-pool 은 이 내용과 맞습니다. 바꾸지 않는 쪽이 맞습니다

    너의 추천
    이슈를 연 채로 두고 고침은 #3000만 따라갑니다. 같은 주제로 새 PR을 열지 않습니다. #2497은 닫힌 채로 둡니다. 다시 열지 않습니다. 라벨은 그대로 둡니다. 머지는 #3000이 초안에서 나오고, CI와 메인테이너 보안 검토가 끝난 뒤에 하면 됩니다.

    이 댓글은 grok-bot이 작성했습니다

  3. lidge-jun commented on Sep 1, 2026

    @lidge-jun
    Owner

    Both races this issue named are now addressed, though the second one is narrowed rather than eliminated — so here is exactly where it stands.

    Coordination race — closed. #3112 (fecb77a9) made native-main refresh serialize on the canonical CODEX_HOME claim, so two OpenCodex instances with different OPENCODEX_HOME values but the same Codex home now coordinate on the file they both write.

    Publication race — narrowed by #3199 (c17bc94c2faa9b296a95d8529019579df177de02).

    Your diagnosis was right, and the reason it survived the existing guards is worth stating: the publisher had two pre-rename checks, and both compared rawSha256. They asked whether the content matched, never whether it was still the same file. So an external writer that rewrote auth.json with identical bytes owned the target — different inode, same hash — and the staged credential would overwrite it anyway.

    The snapshot now carries the target's dev/ino alongside the hash and compares both, failing closed when identity cannot be read on either side. The regression follows your reproduction, using the setMainAuthJsonBeforeRenameHookForTests seam you named in step 5: a different-bytes writer is preserved byte-for-byte, a same-bytes-new-inode writer is refused, the canonical target survives a refused publication with no residual temps, and an uncontested publication still succeeds. Removing just the identity check turns the same-bytes case red, which is the proof it is doing work the hash was not.

    Why this stays open. The check still runs before rename(2) rather than atomically with it — atomic-write.ts issues validateBeforeRename and rename as separate syscalls, and rename replaces unconditionally. Closing that last window needs a real compare-and-swap: renameat2(RENAME_EXCHANGE) on Linux or an equivalent elsewhere. Bun exposes neither, and there is no linkSync/O_EXCL primitive in the tree to build one from today.

    So I am leaving this open, re-scoped to that primitive alone. Your other two requirements — deterministic recovery after process death, and never removing the canonical target during publication — are covered by the claim work and by the residual-temp assertions respectively.

    Thank you for the report. Naming the exact hook, the exact window, and the test files that would prove it is what made both halves straightforward to act on.

  4. github-actions commented on Sep 1, 2026

    @github-actions
    Contributor

    Automated translation bookkeeping — detected language: English.

  5. added 5 commits that reference this issue on Sep 1, 2026
  6. MarcTCruz commented on Oct 9, 2026

    @MarcTCruz
    ContributorAuthor

    @lidge-jun
    I can't reopen this issue (it was closed by a maintainer), so I'm asking here: please reopen it. The native-main refresh path on current dev (93ed1a40b) can still lose a rotated grant, and the publication window this issue was left open for is still there.

    Rotated token discarded on cancel — fix in #6859. This is the failure the #3000 review called the more important one: "a caller that cancels late discards the only live refresh token... A cancelled wait must not decide the fate of a refresh that already succeeded." resolveMainAccountToken still does this in two ways:

    Either way, a client disconnect after the provider has rotated the grant leaves auth.json holding the retired token. #6859 makes caller cancellation stop only the waits before the exchange starts, publishes a successful result through the unchanged snapshot/identity guards, and reports the cancellation after the commit.

    Check-then-rename window — still open, not addressed. validateBeforeRename and rename(2) are still separate syscalls, with no compare-and-swap. #6859 deliberately leaves this alone and adds no FFI.

  7. Ingwannu commented on Oct 11, 2026

    @Ingwannu
    Owner

    Rechecked against current dev f189b33 and separating the two outcomes:

    • The cancellation/token-loss follow-up is now on dev: fix(codex): never discard a rotated native-main refresh token on cancel #6859 was merged by @lidge-jun on 2026-10-10 as fca9cbf, an ancestor of the checked head. resolveMainAccountToken now detaches the started exchange from caller cancellation and publishes a successful rotation before reporting that cancellation. The earlier 93ed1a4 observation predates this landing. This is a dev-landing statement, not a released-version or live-account verification.
    • The residual publication window described in the earlier maintainer comment remains separate and unaddressed by fix(codex): never discard a rotated native-main refresh token on cancel #6859. persistRefreshedMainAuthJson still supplies snapshot validation to atomicWriteFile; src/config/atomic-write.ts and src/lib/windows-atomic-replace.ts invoke validation before an ordinary rename. They do not make the external-writer comparison atomic with replacement.

    Reopening this existing canonical issue for that remaining publication/compare-and-replace question, as the earlier scope note intended. Coordination and successful-refresh cancellation are not being requested again. #3000 and #2497 remain closed; this does not revive their FFI design or authorize a replacement implementation, sponsorship, merge, or scan. No new live credential reproduction was run; the current check is source/ancestry verification of the already-public report. A fix still needs a separately reviewed cross-platform contract and meaningful external-writer regressions.

  8. lidge-jun commented on Oct 13, 2026

    @lidge-jun
    Owner

    Design proposal: who owns CODEX_HOME/auth.json, and how native-main refresh publishes into it

    Design only. No code change accompanies this comment. Checked against dev 6da277436 and Codex source 16845bb8 (2026-10-12).

    Problem, restated against current dev

    #3183, #3199, and #6859 closed the coordination race, the same-bytes/new-inode check, and late-cancel token loss. What remains is that OpenCodex is a second writer of a file it does not own, and the writer it races with does not cooperate:

    1. The check-then-rename gap. persistRefreshedMainAuthJson (src/codex/main-account.ts:284) passes assertMainAuthJsonSnapshotUnchanged as validateBeforeRename. atomicWriteFileToTarget (src/config/atomic-write.ts:321-323) runs that check and then a separate rename(2), which replaces the target unconditionally. persistNativeMainReauthTokens (:355) has the same gap.
    2. Codex does not write the way our guard assumes. FileAuthStorage::save (codex-rs/login/src/auth/storage.rs:214-230) opens auth.json with truncate(true) and writes into the same inode. No temp file, no rename, no lock. This has three consequences:
      • The dev/ino check from fix(codex): verify auth.json identity, not just content, before publishing #3199 never fires for a Codex write. Only the hash check catches it.
      • During Codex's truncate-then-write, the file can briefly be empty or half written. If our final check reads it then, we refuse to publish (fail closed), and the grant we just rotated is thrown away, even though the provider has already retired the old refresh token. (Inferred from source; not reproduced.)
      • A Codex process that opened the old inode before our rename and writes after it puts its bytes into an orphaned inode. That write is lost no matter how atomic our rename is. A compare-and-swap rename alone cannot protect an in-place writer.
    3. Grant coordination with Codex does not exist. Codex serializes refresh with an in-process semaphore only (AuthManager::refresh_token, manager.rs:2845). It does a guarded reload from disk first, but takes no file lock. OpenCodex's .opencodex-native-main.claim.sqlite is invisible to Codex. When both sides read the same refresh token before either writes, both send it, and the loser gets refresh_token_reused. Codex classifies that as permanent (manager.rs:1688).

    What a user would actually see: the guarded file gap is microseconds wide, so the bad outcome is rare, but when it happens it is silent. The harmful case is an external write that is not the same grant. Examples are codex login to a different account, or codex logout (delete_file_if_exists), landing between our check and our rename. The user's new login is reverted to the old grant, or their logout is undone. Two refreshes of the same grant mostly sort themselves out at the token endpoint, because only one succeeds.

    Options

    Option What it closes Cost / risk
    A. Status quo + fail soft on refusal Nothing new on the race. When publication is refused after a successful exchange, the in-flight request is served with the fresh access token instead of failing. Tiny. Does not touch who owns the file.
    B. Lossless exchange publication (renameat2(RENAME_EXCHANGE) on Linux, renamex_np(RENAME_SWAP) on macOS/APFS, ReplaceFileW with a backup file on Windows) Swap atomically, then inspect the displaced file. If it does not match the snapshot, swap back and keep the displaced file as a sidecar. Every rename-based external write survives at a known path. Needs bun:ffi on three platforms, which revives the FFI direction declined in #3000/#2497. Does not cover Codex's truncate-in-place writes: an orphaned-inode write can still land after we verify. Filesystem limits (non-APFS volumes, network mounts) need a fallback anyway.
    C. Ownership marker (a sidecar in OPENCODEX_HOME recording the hash and inode we last wrote; refresh only if the file is still "ours") Gives a precedence rule: external content always wins once it has diverged. Mostly duplicates the existing snapshot check. Cannot live inside auth.json, because Codex re-serializes AuthDotJson and drops unknown fields. Does not close any race.
    D. Read-only external mode; Codex refreshes OpenCodex never writes CODEX_HOME/auth.json. When the access token is expired or rejected, it asks Codex to refresh through app-server getAuthStatus { includeToken: true, refreshToken: true } (account_processor.rs:1103-1125), then rereads the file. Codex stays the only writer and uses its own guarded reload. Needs a compatible codex binary (Desktop ships one; resolveChatgptCodexBinary exists) and a process spawn per refresh, which happens roughly hourly. Refresh latency goes up. Codex-vs-Codex races remain, but they are upstream's existing behavior and no longer ours. Precedent: Orca links are already "read-only; Orca exclusively owns OAuth refresh" (src/types/accounts.ts:22).
    E. Separate store (OpenCodex gets its own grant through ocx login, i.e. a pool account, and never shares Codex's refresh token) Removes the shared rotating grant completely. Changes the product: "use my Codex login" stops being zero setup. Copying Codex's refresh token into our store is not this option; it would make the grant race worse.
    F. Upstream fix (Codex writes via temp + rename and takes a auth.json.lock advisory lock that we also honor) The real cross-tool fix. Out of our control and slow to arrive. Worth filing anyway.

    Recommendation

    Make D the default for native main, keep the current publisher as a guarded fallback, and ship A right away.

    • D is the only option that fixes ownership instead of narrowing a window. Codex owns auth.json, and OpenCodex reads it. It also handles in-place writes, logout, and account switches, which B cannot.
    • Keep the current publisher (claim + snapshot + identity guards) only for when no compatible codex binary is found, or when the user opts in explicitly. Report which mode is active in ocx doctor.
    • A is a small, separate PR that is useful on its own: when publication is refused after a successful exchange, return the fresh access token to the waiting request and do not persist it. This stops a refused write from also failing the user's request.
    • Do not pursue B unless D turns out to be unworkable. It adds FFI on three platforms and still does not protect the writer we actually race with.
    • File F upstream with the source references above.

    Migration

    • Native main: nativeMainRefresh: "codex" | "opencodex", default "codex" when a compatible binary resolves, otherwise "opencodex" with today's behavior. No change to auth.json format or contents, and no data migration.
    • Native-profile slots (native-profile-store, keyring and encrypted modes) are files OpenCodex owns, so they keep their own publisher. This proposal covers only the shared CODEX_HOME/auth.json.
    • cli_auth_credentials_store = "keyring" or "auto": D works the same way there, because Codex refreshes whichever store it uses. OpenCodex must then read through the existing keyring-aware path and never fall back to auth.json.
    • Rollout: put mode D behind the setting for one release, then flip the default.

    Test plan

    • A: a unit test using setMainAuthJsonBeforeRenameHookForTests to make publication refuse after a successful injected exchange. The request still gets the new access token, the file keeps the external bytes byte for byte, and the grant is not marked rejected.
    • D, contract: a fake codex app-server (loopback stdio fixture) that rotates the file. Assert that OpenCodex never opens auth.json for writing (spy on atomicWriteFile for that path), rereads after the delegated refresh, and maps Codex's permanent failure reasons, including refresh_token_reused, to needsReauth.
    • D, live: an opt-in smoke test against a real codex binary on a throwaway CODEX_HOME. It does not run in default CI.
    • Fallback-mode stress (child processes): an external writer that imitates Codex's in-place open(O_TRUNC)+write and one that imitates rename, looped against the publisher. Count lost external writes. This puts a number on the remaining risk of the fallback without claiming it is zero.
    • Remove the "identity check protects against Codex" wording from comments, since Codex writes keep the same inode.

    Open decisions for Jun

    1. Ownership: do you accept "Codex owns CODEX_HOME/auth.json; OpenCodex reads it and delegates refresh" (D) as the product contract for native main?
    2. Fallback: when no compatible codex binary is found, should OpenCodex keep today's guarded publisher, or go read-only and require codex login or ocx login once the token expires?
    3. FFI: keep B (exchange-swap via bun:ffi) off the table, consistent with closing fix(codex): harden native main refresh publication #3000/Fix native main token refresh and replay #2497?
    4. Option A scope: should a refused publication only serve the in-flight request, or also keep the rotated refresh token in a short-lived 0600 escrow under OPENCODEX_HOME for recovery? Escrow keeps one more secret on disk; my lean is no escrow.
    5. Upstream: may we file the temp+rename+lock request against openai/codex?

    Once 1-3 are decided, implementation can be split into three small PRs: A, the D adapter behind a setting, and the doctor/mode reporting.

  9. lidge-jun commented on Oct 13, 2026

    @lidge-jun
    Owner

    Fixed on dev in two steps:

    Remaining limits: the app-server process OpenCodex starts can still race a Codex CLI or Desktop that is already running (Codex's refresh lock is per process), and logins stored only in the keyring are still unsupported for native main.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    account-poolOAuth, credentials, Codex pool, quota, failover, plansbugSomething isn't working

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions