From df976f151b80c77d42b8de0cbafb000b0f2b4ec9 Mon Sep 17 00:00:00 2001 From: Factory Agent Date: Fri, 23 Oct 2026 13:33:46 +0700 Subject: [PATCH] Log redacted Codex registry mutations --- .../src-tauri/src/commands/codex_accounts.rs | 29 ++- rust/src/codex_accounts/account_manager.rs | 6 +- rust/src/codex_accounts/mod.rs | 2 + rust/src/codex_accounts/registry_log.rs | 178 ++++++++++++++++++ rust/src/codex_accounts/stores.rs | 27 +++ 5 files changed, 232 insertions(+), 10 deletions(-) create mode 100644 rust/src/codex_accounts/registry_log.rs diff --git a/apps/desktop-tauri/src-tauri/src/commands/codex_accounts.rs b/apps/desktop-tauri/src-tauri/src/commands/codex_accounts.rs index d568cdc2461..963681a6915 100644 --- a/apps/desktop-tauri/src-tauri/src/commands/codex_accounts.rs +++ b/apps/desktop-tauri/src-tauri/src/commands/codex_accounts.rs @@ -5,8 +5,8 @@ use uuid::Uuid; use codexbar::codex_accounts::{ AccountStore, CodexAccount, CodexAccountApi, CodexAccountManager, CodexAccountManagerError, - CodexAccountRuntime, CodexAccountSource, CodexApiError, CodexSwitchResult, SnapshotStore, - display_names_by_id, ordinals_by_id, restart_codex_desktop, + CodexAccountRuntime, CodexAccountSource, CodexApiError, CodexSwitchResult, RegistryOperation, + SnapshotStore, display_names_by_id, ordinals_by_id, restart_codex_desktop, }; use codexbar::notifications::CredentialAlertPolicy; use codexbar::settings::Settings; @@ -84,10 +84,18 @@ fn reconcile_codex_accounts( /// Persist the given accounts to the account store. pub(crate) fn persist_codex_accounts(accounts: &[CodexAccount]) -> Result<(), String> { + persist_codex_accounts_as(RegistryOperation::RegistryWrite, accounts) +} + +/// Persist the given accounts, tagging the registry-write log with `operation`. +pub(crate) fn persist_codex_accounts_as( + operation: RegistryOperation, + accounts: &[CodexAccount], +) -> Result<(), String> { let store = AccountStore::new(); let (_existing, removed) = store.load().map_err(|e| e.to_string())?; store - .save(accounts, Some(&removed)) + .save_with_operation(accounts, Some(&removed), operation) .map_err(|e| e.to_string()) } @@ -317,7 +325,7 @@ pub async fn codex_account_add(app: tauri::AppHandle) -> Result canonical_reauthenticated_account(&accounts, &authenticated), Err(e) => { // Credential replacement is already committed, but the reconciled @@ -392,7 +400,7 @@ pub fn codex_account_remove(app: tauri::AppHandle, id: String) -> Result<(), Str .into_iter() .filter(|account| account.id.to_string() != id) .collect(); - persist_codex_accounts(&remaining)?; + persist_codex_accounts_as(RegistryOperation::Remove, &remaining)?; crate::auto_resume::clear(&app, ProviderId::Codex); events::emit_settings_changed(&app); accounts_changed(&app); @@ -530,7 +538,7 @@ pub async fn codex_account_fetch( let _ = SnapshotStore::new().save(&snapshots); } - if refresh_persisted_accounts(app).is_err() { + if refresh_persisted_accounts(app, RegistryOperation::RegistryWrite).is_err() { // Non-fatal: the snapshot was still fetched. } Ok(snapshot) @@ -575,9 +583,12 @@ pub async fn codex_account_restart_desktop( /// /// Returns the post-reconciliation accounts, which is exactly the set that was /// persisted, so callers can report a canonical account from persisted state. -fn refresh_persisted_accounts(app: tauri::AppHandle) -> Result, String> { +fn refresh_persisted_accounts( + app: tauri::AppHandle, + operation: RegistryOperation, +) -> Result, String> { let accounts = load_codex_accounts()?; - persist_codex_accounts(&accounts)?; + persist_codex_accounts_as(operation, &accounts)?; events::emit_settings_changed(&app); accounts_changed(&app); Ok(accounts) diff --git a/rust/src/codex_accounts/account_manager.rs b/rust/src/codex_accounts/account_manager.rs index c5e9d01236f..c40b4227ab2 100644 --- a/rust/src/codex_accounts/account_manager.rs +++ b/rust/src/codex_accounts/account_manager.rs @@ -24,6 +24,7 @@ use super::file_locations::{ }; use super::login_runner::{CodexLoginOutcome, CodexLoginRunner, ManagedLoginProcess}; use super::models::{CodexAccount, CodexAccountSource, utc_now}; +use super::registry_log::{RegistryOperation, log_home_deletion}; /// Friendly account manager error. #[derive(Debug, Error)] @@ -77,7 +78,9 @@ impl CodexAccountManager { Err(error) => { // Best-effort teardown of the fresh managed home on failure; // a removal error cannot change the authentication outcome. - let _removed_home = fs::remove_dir_all(&home_path); + if fs::remove_dir_all(&home_path).is_ok() { + log_home_deletion(RegistryOperation::AddOrReauth); + } Err(error) } } @@ -127,6 +130,7 @@ impl CodexAccountManager { } if target.exists() { fs::remove_dir_all(&target)?; + log_home_deletion(RegistryOperation::Remove); } } Ok(()) diff --git a/rust/src/codex_accounts/mod.rs b/rust/src/codex_accounts/mod.rs index f8dfb8edd67..fd11dd9ad1d 100644 --- a/rust/src/codex_accounts/mod.rs +++ b/rust/src/codex_accounts/mod.rs @@ -21,6 +21,7 @@ pub mod file_locations; mod labels; pub mod login_runner; pub mod models; +mod registry_log; pub mod stores; mod switch_runtime; @@ -42,5 +43,6 @@ pub use models::{ AccountUsageSnapshot, CodexAccount, CodexAccountSource, CreditsBalanceSnapshot, RemovedAccountIdentity, UsageWindowSnapshot, utc_now, }; +pub use registry_log::RegistryOperation; pub use stores::{AccountStore, SnapshotStore}; pub use switch_runtime::CodexAccountRuntime; diff --git a/rust/src/codex_accounts/registry_log.rs b/rust/src/codex_accounts/registry_log.rs new file mode 100644 index 00000000000..9b3126e6a27 --- /dev/null +++ b/rust/src/codex_accounts/registry_log.rs @@ -0,0 +1,178 @@ +//! Redacted diagnostics for Codex account registry mutations. +//! +//! These lines help explain a saved account that disappears. They carry only a +//! fixed operation name, row counts and an outcome. They never carry account +//! ids, emails, managed-home paths, tokens or error text. + +/// Fixed operation names for registry mutations. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum RegistryOperation { + /// A managed account was added through login, or re-authenticated in place. + AddOrReauth, + /// A saved account was removed. + Remove, + /// Any other registry write, such as metadata reconciliation or switch bookkeeping. + RegistryWrite, +} + +impl RegistryOperation { + pub const fn as_str(self) -> &'static str { + match self { + Self::AddOrReauth => "add-or-reauth", + Self::Remove => "remove", + Self::RegistryWrite => "registry-write", + } + } +} + +/// Field values for one registry write attempt. +#[derive(Debug, Clone, PartialEq, Eq)] +pub(crate) struct RegistryWriteLog { + pub(crate) operation: &'static str, + pub(crate) rows_before: String, + pub(crate) rows_requested: String, + pub(crate) rows_written: String, + pub(crate) result: &'static str, +} + +/// Builds the fields for one registry write attempt. +/// +/// Takes only counts and the outcome, so no identifier, path or error text can +/// reach the log line. +pub(crate) fn registry_write_log( + operation: RegistryOperation, + rows_before: Option, + rows_requested: usize, + succeeded: bool, +) -> RegistryWriteLog { + RegistryWriteLog { + operation: operation.as_str(), + rows_before: count_field(rows_before), + rows_requested: rows_requested.to_string(), + rows_written: if succeeded { + rows_requested.to_string() + } else { + "unknown".to_string() + }, + result: if succeeded { "success" } else { "failed" }, + } +} + +fn count_field(count: Option) -> String { + count.map_or_else(|| "unknown".to_string(), |count| count.to_string()) +} + +/// Emits one registry-write log line built by [`registry_write_log`]. +pub(crate) fn log_registry_write(log: &RegistryWriteLog) { + if log.result == "success" { + tracing::info!( + operation = log.operation, + rows_before = %log.rows_before, + rows_requested = %log.rows_requested, + rows_written = %log.rows_written, + result = log.result, + "Managed Codex account registry write" + ); + } else { + tracing::error!( + operation = log.operation, + rows_before = %log.rows_before, + rows_requested = %log.rows_requested, + rows_written = %log.rows_written, + result = log.result, + "Managed Codex account registry write failed" + ); + } +} + +/// Emits one log line for a successful managed-home deletion. +pub(crate) fn log_home_deletion(operation: RegistryOperation) { + tracing::info!( + operation = operation.as_str(), + "Managed Codex account home deleted" + ); +} + +#[cfg(test)] +mod tests { + use super::*; + + const FORBIDDEN_FRAGMENTS: [&str; 7] = [ + "@", + "/", + "\\", + "example.com", + "managed-homes", + "auth.json", + "token", + ]; + + fn assert_only_fixed_vocabulary(log: &RegistryWriteLog) { + let rendered = format!("{log:?}"); + for fragment in FORBIDDEN_FRAGMENTS { + assert!( + !rendered.contains(fragment), + "registry log leaked `{fragment}`: {rendered}" + ); + } + for value in [&log.rows_before, &log.rows_requested, &log.rows_written] { + assert!( + value == "unknown" || value.chars().all(|c| c.is_ascii_digit()), + "count field is not a bare number or `unknown`: {value}" + ); + } + assert!(matches!(log.result, "success" | "failed")); + assert!(matches!( + log.operation, + "add-or-reauth" | "remove" | "registry-write" + )); + } + + #[test] + fn success_record_carries_only_counts_and_operation() { + let log = registry_write_log(RegistryOperation::Remove, Some(3), 2, true); + + assert_eq!(log.operation, "remove"); + assert_eq!(log.rows_before, "3"); + assert_eq!(log.rows_requested, "2"); + assert_eq!(log.rows_written, "2"); + assert_eq!(log.result, "success"); + assert_only_fixed_vocabulary(&log); + } + + #[test] + fn failed_write_reports_unknown_written_rows() { + let log = registry_write_log(RegistryOperation::AddOrReauth, None, 4, false); + + assert_eq!(log.operation, "add-or-reauth"); + assert_eq!(log.rows_before, "unknown"); + assert_eq!(log.rows_requested, "4"); + assert_eq!(log.rows_written, "unknown"); + assert_eq!(log.result, "failed"); + assert_only_fixed_vocabulary(&log); + } + + #[test] + fn registry_write_default_operation_name_is_fixed() { + let log = registry_write_log(RegistryOperation::RegistryWrite, Some(0), 0, true); + + assert_eq!(log.operation, "registry-write"); + assert_eq!(log.rows_before, "0"); + assert_only_fixed_vocabulary(&log); + } + + #[test] + fn every_operation_name_is_identity_free() { + for operation in [ + RegistryOperation::AddOrReauth, + RegistryOperation::Remove, + RegistryOperation::RegistryWrite, + ] { + let name = operation.as_str(); + assert!(!name.contains('@')); + assert!(!name.contains('/')); + assert!(!name.contains('\\')); + assert!(name.chars().all(|c| c.is_ascii_lowercase() || c == '-')); + } + } +} diff --git a/rust/src/codex_accounts/stores.rs b/rust/src/codex_accounts/stores.rs index 5983d057f03..dfb2c8d9692 100644 --- a/rust/src/codex_accounts/stores.rs +++ b/rust/src/codex_accounts/stores.rs @@ -15,6 +15,7 @@ use crate::secure_file; use super::file_locations::{accounts_file, snapshots_file}; use super::models::{CodexAccount, RemovedAccountIdentity}; +use super::registry_log::{RegistryOperation, log_registry_write, registry_write_log}; #[derive(Debug, serde::Serialize, serde::Deserialize)] struct AccountsFile { @@ -72,6 +73,32 @@ impl AccountStore { &self, accounts: &[CodexAccount], removed_accounts: Option<&[RemovedAccountIdentity]>, + ) -> io::Result<()> { + self.save_with_operation(accounts, removed_accounts, RegistryOperation::RegistryWrite) + } + + /// Write the accounts file and log one redacted registry-write entry. + pub fn save_with_operation( + &self, + accounts: &[CodexAccount], + removed_accounts: Option<&[RemovedAccountIdentity]>, + operation: RegistryOperation, + ) -> io::Result<()> { + let rows_before = self.load().ok().map(|(existing, _)| existing.len()); + let result = self.write_accounts_file(accounts, removed_accounts); + log_registry_write(®istry_write_log( + operation, + rows_before, + accounts.len(), + result.is_ok(), + )); + result + } + + fn write_accounts_file( + &self, + accounts: &[CodexAccount], + removed_accounts: Option<&[RemovedAccountIdentity]>, ) -> io::Result<()> { super::file_locations::ensure_directories()?; let removed = match removed_accounts {