From b0ba65a936278f7dc037176de0977b719d6ec252 Mon Sep 17 00:00:00 2001 From: Michael Bolin Date: Fri, 25 Apr 2025 17:37:41 -0700 Subject: [PATCH] fix: write logs to ~/.codex/log instead of /tmp (#669) Previously, the Rust TUI was writing log files to `/tmp`, which is world-readable and not available on Windows, so that isn't great. This PR tries to clean things up by adding a function that provides the path to the "Codex config dir," e.g., `~/.codex` (though I suppose we could support `$CODEX_HOME` to override this?) and then defines other paths in terms of the result of `codex_dir()`. For example, `log_dir()` returns the folder where log files should be written which is defined in terms of `codex_dir()`. I updated the TUI to use this function. On UNIX, we even go so far as to `chmod 600` the log file by default, though as noted in a comment, it's a bit tedious to do the equivalent on Windows, so we just let that go for now. This also changes the default logging level to `info` for `codex_core` and `codex_tui` when `RUST_LOG` is not specified. I'm not really sure if we should use a more verbose default (it may be helpful when debugging user issues), though if so, we should probably also set up log rotation? --- codex-rs/core/src/config.rs | 31 +++++++++++++++++++++++++++---- codex-rs/tui/src/lib.rs | 26 +++++++++++++++++++------- 2 files changed, 46 insertions(+), 11 deletions(-) diff --git a/codex-rs/core/src/config.rs b/codex-rs/core/src/config.rs index c094de54..b5574cea 100644 --- a/codex-rs/core/src/config.rs +++ b/codex-rs/core/src/config.rs @@ -1,3 +1,5 @@ +use std::path::PathBuf; + use dirs::home_dir; use serde::Deserialize; @@ -28,15 +30,36 @@ impl Config { } fn load_from_toml() -> Option { - let mut p = home_dir()?; - p.push(".codex/config.toml"); + let mut p = codex_dir().ok()?; + p.push("config.toml"); let contents = std::fs::read_to_string(&p).ok()?; toml::from_str(&contents).ok() } fn load_instructions() -> Option { - let mut p = home_dir()?; - p.push(".codex/instructions.md"); + let mut p = codex_dir().ok()?; + p.push("instructions.md"); std::fs::read_to_string(&p).ok() } } + +/// Returns the path to the Codex configuration directory, which is `~/.codex`. +/// Does not verify that the directory exists. +pub fn codex_dir() -> std::io::Result { + let mut p = home_dir().ok_or_else(|| { + std::io::Error::new( + std::io::ErrorKind::NotFound, + "Could not find home directory", + ) + })?; + p.push(".codex"); + Ok(p) +} + +/// Returns the path to the folder where Codex logs are stored. Does not verify +/// that the directory exists. +pub fn log_dir() -> std::io::Result { + let mut p = codex_dir()?; + p.push("log"); + Ok(p) +} diff --git a/codex-rs/tui/src/lib.rs b/codex-rs/tui/src/lib.rs index 4a063de0..d0f5f664 100644 --- a/codex-rs/tui/src/lib.rs +++ b/codex-rs/tui/src/lib.rs @@ -31,19 +31,31 @@ pub use cli::Cli; pub fn run_main(cli: Cli) -> std::io::Result<()> { assert_env_var_set(); + let log_dir = codex_core::config::log_dir()?; + std::fs::create_dir_all(&log_dir)?; // Open (or create) your log file, appending to it. - let file = OpenOptions::new() - .create(true) - .append(true) - .open("/tmp/codex-rs.log")?; + let mut log_file_opts = OpenOptions::new(); + log_file_opts.create(true).append(true); + + // Ensure the file is only readable and writable by the current user. + // Doing the equivalent to `chmod 600` on Windows is quite a bit more code + // and requires the Windows API crates, so we can reconsider that when + // Codex CLI is officially supported on Windows. + #[cfg(unix)] + { + use std::os::unix::fs::OpenOptionsExt; + log_file_opts.mode(0o600); + } + + let log_file = log_file_opts.open(log_dir.join("codex-tui.log"))?; // Wrap file in non‑blocking writer. - let (non_blocking, _guard) = non_blocking(file); + let (non_blocking, _guard) = non_blocking(log_file); - // use RUST_LOG env var, default to trace for codex crates. + // use RUST_LOG env var, default to info for codex crates. let env_filter = || { EnvFilter::try_from_default_env() - .unwrap_or_else(|_| EnvFilter::new("codex=trace,codex_tui=trace")) + .unwrap_or_else(|_| EnvFilter::new("codex_core=info,codex_tui=info")) }; // Build layered subscriber: