Skip to content

Keep Codex account runtime private across starts - #38

Closed
Tiyouw wants to merge 1 commit into
enowdev:mainfrom
Tiyouw:codex/account-upstream-adapt-20261105-8955
Closed

Tiyouw wants to merge 1 commit into
enowdev:mainfrom
Tiyouw:codex/account-upstream-adapt-20261105-8955

Conversation

@Tiyouw

@Tiyouw Tiyouw commented Nov 5, 2026

Copy link
Copy Markdown
Contributor

What and why

Codex account preparation links missing default-home runtime entries into another account home. After a physical account SQLite database closes and removes its sidecars, the next preparation can link the default database's WAL/SHM into that account. Share only known settings and conversations so databases, journals, auth, sockets, daemon state and unknown future names remain private. Refuse existing unsafe links without changing them or their targets.

Keep sessions, archived_sessions and thread-writer-locks in one canonical namespace, including first launch and concurrent starts within the daemon. Refuse conflicting namespaces or failure to establish required links. Honor the last explicit config CODEX_HOME before touching an unused generated home, using case-insensitive environment names on Windows. Explicit homes remain owner-managed; team-member isolation is unchanged.

Failed account preparation during restoration leaves the session stopped with exit code -1; continue propagates the failure. Adapted onto main cd071c6299c93b91c6e21643ca6c6c11156a39d7 after the restart changes in #31: retain keyAwaitsApp, restartVaultEnv, Lost reporting, TabAgent handling and the existing carry-on flow. The account guard runs after key-wait handling and before launching a restored session.

How it was tested

One signed-off commit: d62460b9fea93fa5de1d62e9234906e05a59f835. Only account-sharing code, its restoration guard, focused regressions and account documentation are changed.

Actual local Windows checks with Go 1.27.3, fresh synthetic homes and a private temporary/cache location:

  • Complete go test ./core/accounts -count=1 -timeout=2m -v: 19 top-level tests passed, 2.904s. Includes real SQLite close/prepare/reopen cycles for two account databases, row ownership/integrity, canonical aliases with a live OS lock, legacy/dangling link refusal and explicit-home precedence.
  • Focused daemon restore/continue/restart selection: passed, 0.915s. TestCodexRestoreRefusesUnsafeAccountHome passes for idle and mid-turn-tab records, including continue refusal, carry-on readiness refusal, unchanged journal/link and retained Lost reporting. TestContinueWithoutAConversation passes. Eleven existing restart tests that require sh skip on Windows; these skips are not native restart validation.
  • go vet ./core/accounts ./daemon: passed. git diff --check: passed.
  • The same SQLite regression on pristine upstream 048b78ece9b54ab5c2d5f32c1944eb738c82311c fails at cycle 0. The broad sharing path remains in the current base. This is a synthetic path-sharing reproduction, not historical corruption attribution.

This starts as Draft while CI runs for this exact head. Local checks do not establish full make test/make lint, native Linux/macOS flock/restart behavior, or authenticated lifecycle behavior.

  • make test and make lint pass on this head
  • Works the same for a session on this device and on a server (authenticated owner validation pending)
  • Docs updated where behavior changed
  • Commits signed off (git commit -s)

No protocol change, automatic legacy migration, recovery or private thread-index rebuilding is included. Real sign-in, fresh/resume/account switching, and owner-reviewed offline migration still need validation. Preserve matching physical databases/journals and canonical writer-lock identity during that separate review. No deployment is included.

Closes #37.

Share only known Codex settings and canonical rollout paths. Refuse legacy runtime links without unlinking targets, and retain one writer-lock namespace across repeated or concurrent starts.

Honor explicit config homes before preparing an unused account home, and keep restored sessions stopped when account setup fails. Add synthetic SQLite close/reopen and active-lock regressions.

Signed-off-by: Ahimsa Jenar Bramsaifstyo Kusuma <tio.rmx@gmail.com>
(cherry picked from commit 7d7596661a461f676aed5bc51a48cc9a39278c24)
(cherry picked from commit bd061596a044c39afa7855123f9a87f2ecc08217)
Signed-off-by: Ahimsa Jenar Bramsaifstyo Kusuma <tio.rmx@gmail.com>
@enowdev

enowdev commented Nov 5, 2026

Copy link
Copy Markdown
Owner

This is a real bug on main, and the fix is focused. For the record, from reading e306b3b:

  • core/accounts/accounts.go:43 lists only auth.json as the account's own, and share() at :334-344 links every other entry of ~/.codex, including state_5.sqlite and its -wal/-shm/-journal, with symlink failures ignored (_ = os.Symlink). On Windows that silently leaves an account with its own database, so the next preparation can link the default home's WAL beside it. Your reproduction matches the code.
  • daemon/persist.go:311-315 in main logs "could not restore a session's account" and starts the session anyway, without CODEX_HOME — i.e. as the host's own account. Moving that into an else-if that leaves the session stopped is right, and matches the configRunFor branch just above.

merge-tree against current main is clean. CI is red, but the three failures (TestPlatformE2E, TestHooksOutsideMore, TestHibernateLeavesATaskUnderWay) also fail on main's own v0.7.0 run — a rebase onto e306b3b should give a meaningful signal.

Three things I'd like before merging:

  1. Upgrade path. Accounts created by the current share() already contain runtime links, and shareCodex refuses such a home without touching it. That means every existing Codex account stops starting after this lands, with no way out from the app. Please either add a reviewed offline migration that only retires runtime links (targets untouched), or a UI affordance that names which home is refused and why.
  2. The refusal is invisible. A refused restore only sets exit code -1 plus a daemon log warning, so the user sees a stopped session with no reason. Please carry the reason onto the session, the way errNoVaultApp does on the key path.
  3. Env()'s explicit-home early return skips sharing entirely and keeps only the last CODEX_HOME entry. Worth a line in the code and docs saying the config's owner prepares that home — the docs already say it, the code should too.

The allowlist, the concurrent-start gate, the os.SameFile writer-lock check and the SQLite close/reopen regression all look good to me. Keep those as they are.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Keep Codex account preparation from sharing SQLite sidecars and runtime state

2 participants