Skip to content

Port upstream 0.67.0: stage credential writes and publish atomically - #669

Closed
Finesssee wants to merge 2 commits into
port/upstream-0.67.0from
port/micro-0.67.0-credential-write-hardening
Closed

Finesssee wants to merge 2 commits into
port/upstream-0.67.0from
port/micro-0.67.0-credential-write-hardening

Conversation

@Finesssee

Copy link
Copy Markdown
Collaborator

Summary

Credential-bearing file writes now stage next to the destination and publish atomically, so a failed or interrupted write leaves the previous file intact and a partly written secret is never the live file.

  • secure_file::write_string (settings, API keys, manual cookies, token accounts, Claude/Grok account stores, Codex account store, ...) builds the protected (DPAPI) bytes, writes them to a create_new sibling (mode 0600 set at creation and re-applied before any byte is written on unix), sync_all, then publishes through atomic_file::replace_staged (ReplaceFileW on Windows). New atomic_file::write_atomic_private provides this; write_atomic is unchanged for non-secret files. The old post-write restrict_file_permissions is gone (the staged file is private from creation).
  • CookieHeaderCache::store now writes through secure_file (DPAPI plus staged publish) instead of fs::write of plaintext JSON. load reads through secure_file::read_string, which still accepts legacy plaintext entries. clear is unchanged.
  • Codex account manager: the four fs::copy calls of auth.json (switch into the ambient home, materialize_as_managed for an existing and a fresh managed home, and the ambient backup) now go through a new credentials::copy_private_file, which reads the source and publishes with the existing exclusive-staged private writer (write_private_file, factored out of write_auth_contents). fs::copy preserved the source mode (for example 0644); the copy now always ends up 0600 on unix.
  • Non-secret files (stores.rs snapshots) are left alone.
  • Cookie-denial persistence needs no code: a test asserts cookie_source = "off" and openai_web_extras = false written by the Settings::save path are read back by the Settings::load path.

Upstream reference

  • Upstream 0.67.0 item 19 (credential file writes staged and published atomically at 0600).
  • Tag-pinned (v0.67.0) files: Sources/CodexBarCore/CredentialFileWriter.swift (writePrivate), tests Tests/CodexBarTests/CredentialFileWriterTests.swift (staging is private before writing, failed write or publish preserves destination, credential stores use private staging).

Ported / Deferred

Ported: all of (a) to (d) of the triage spec.

Deferred / intentionally different:

  • Upstream stages inside a task-owned 0700 directory and rename(2)s. Windows has no mode bits and the per-user config directory ACL already governs the staged sibling, so this port stages a create_new sibling in the destination directory (0600 on unix) and publishes with ReplaceFileW as atomic_file already does.
  • write_auth_contents still publishes with std::fs::rename (unchanged) rather than replace_staged, to avoid changing behavior against a live Codex CLI holding auth.json open. Not part of this item.
  • repairPermissions (chmod of pre-existing 0644 files on read) has no Windows analogue; on unix, every rewrite now republishes at 0600.
  • Upstream AntigravityOAuthCredentialsStore and GeminiStatusProbe callers: no matching production credential file writes exist in this port (Antigravity here only reads local sessions; the only fs::write hits there are tests), so nothing to convert.

Validation

  • cargo +1.98.0 fmt --all: clean
  • cargo +1.98.0 clippy --workspace --all-targets -- -D warnings: pass
  • cargo +1.98.0 test -p codexbar -- --test-threads=4 (full, shared code touched): 2170 passed, 0 failed, 1 ignored
  • New tests: secure_file (replace leaves only the destination, failed publish keeps destination and leaves no secret sibling, missing directory not created, unix 0600), cookie_cache (round trip and replace, failed store leaves no cookie bytes, legacy plaintext still loads), codex_accounts::credentials (copy replaces and leaves no staged file, unreadable source keeps destination, failed publish cleans staged secret, unix 0600 from a 0644 source), settings cookie-denial round trip. Existing secure_file DPAPI tests (windows_write_uses_protected_wrapper, round trip) still pass on Windows.
  • Tauri crate tests not run (apps/desktop-tauri/src-tauri not touched; workspace clippy passes).

Affected areas

  • Rust backend / shared core (secure_file, atomic_file, browser::cookie_cache, codex_accounts)
  • Tauri shell
  • Frontend
  • Provider fetch/parse

UI proof

Not applicable

@coderabbitai

coderabbitai Bot commented Sep 29, 2026 •

Copy link
Copy Markdown

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Advanced

Run ID: 85aef3a7-2d76-42c8-9285-ff5a883f58c3

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review

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

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

@Finesssee

Copy link
Copy Markdown
Collaborator Author

Reviewed by Codex gpt-6-luna (xhigh); verified and validated by Claude

Thermo-nuclear review of this PR:

  • P2 codex_accounts/credentials.rs write_private_file: creation mode 0600 is filtered through umask and could drop owner access. Fix: restore exact 0600 before writing.
  • P2 codex_accounts/credentials.rs write_private_file: the staged credentials file was renamed without sync_all. Fix: sync before publishing.
  • P2 settings/tests.rs: the cookie-denial test hand-serialized JSON instead of using the persistence path. Fix: added Settings::save_to_path / load_from_path (used by save/load) and test through them.
  • P3 secure_file.rs, browser/cookie_cache.rs failure tests: they searched for plaintext, so a DPAPI-encrypted leftover would pass. Fix: assert leftover staged siblings are empty.

@Finesssee

Copy link
Copy Markdown
Collaborator Author

Follow-up: all four findings fixed, nothing left. Commands run: cargo +1.98.0 fmt --all --check; clippy --workspace --all-targets -D warnings; cargo test -p codexbar --lib (secure_file, cookie_cache, settings, credentials filters; 157 passed).

@Finesssee

Copy link
Copy Markdown
Collaborator Author

Adversarial validation (lane-B review)

Head validated: acaa2716 ("Address thermo review"). Spec: 0.67.0.md PR 4 (item 19, credential-write hardening).

Verdict: no blocking defects. Spec coverage verified in source at head:

  • (a) secure_file.rs write_string: protected bytes staged to a create_new sibling (atomic_file.rs), sync_all, then replace_staged (Windows ReplaceFileW path with directory sync); the staged file is truncated on failed publish ("failed publish must truncate the staged secret" test) so no partly written secret can become the live file.
  • (b) browser/cookie_cache.rs store: the cached header is now a secure-file-wrapped payload (no longer plaintext), staged and published atomically; failure keeps the previous entry ("failed store must truncate the staged cookie secret" test); clear behavior kept.
  • (c) codex_accounts/credentials.rs: the Codex auth copy writes to an exclusive staged private sibling (UUID-named .auth-*.tmp), then renames into place with staged cleanup on failure; account_manager.rs routes account writes through the hardened paths; settings.rs saves via secure_file::write_string (same staging).

Validation re-run at head (pinned 1.98.0, E-cores): cargo +1.98.0 fmt --all -- --check clean; cargo +1.98.0 clippy --workspace --all-targets -- -D warnings clean; cargo +1.98.0 test -p codexbar --lib 2170 passed / 0 failed (1 ignored; includes the new staged-write failure-injection tests); cargo +1.98.0 test -p codexbar-desktop-tauri 461 passed / 1 failed (the documented #684 bootstrap_payload_exposes_every_provider_variant baseline, untouched by this backend-only diff; no frontend files changed, so vitest is not affected).

@Finesssee

Copy link
Copy Markdown
Collaborator Author

Shipped in v0.70.0: this PR's head is included in main via #735 (merge commit 9d0a37a). Closing as integrated.

@Finesssee Finesssee closed this Oct 3, 2026
junglesub-bot Bot pushed a commit to junglesub/Win-CodexBar that referenced this pull request Oct 4, 2026
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.

1 participant