From 100cd59194985d9a3ca26421258fb5c927fdfc65 Mon Sep 17 00:00:00 2001 From: tiennm99 Date: Fri, 23 Oct 2026 03:09:40 +0700 Subject: [PATCH] fix(refresh): run the refresh CLIs from the temp directory A GUI app starts with its working directory at the home folder, / or System32, and `codex exec .` refuses to run there because it is not a trusted directory or repository. Both CLIs now start from the temp directory so neither runs inside a project or loads its instructions and hooks, and Codex runs with --skip-git-repo-check in a read-only sandbox. The orchestrator takes its spawn step as an argument internally, so its refreshed, timeout and missing-CLI outcomes are now tested with a fake credential source, along with the locator's source order and the Claude credential file signature. --- src/creds/local_fs.rs | 32 ++++++++ src/usage/refresh.rs | 184 ++++++++++++++++++++++++++++++++++++++++-- 2 files changed, 211 insertions(+), 5 deletions(-) diff --git a/src/creds/local_fs.rs b/src/creds/local_fs.rs index 19d297f..a50a6c1 100644 --- a/src/creds/local_fs.rs +++ b/src/creds/local_fs.rs @@ -106,6 +106,38 @@ mod tests { assert_eq!(token.expires_at_unix_ms, None); } + #[test] + fn signature_changes_when_a_rewrite_changes_the_file_size() { + use crate::creds::CredentialSource; + + let path = std::env::temp_dir().join(format!( + "ccub-local-creds-{}-{}.json", + std::process::id(), + std::time::SystemTime::now() + .duration_since(UNIX_EPOCH) + .map(|d| d.as_nanos()) + .unwrap_or_default() + )); + let creds = LocalClaudeCreds { + id: format!("local:{}", path.display()), + path: path.clone(), + }; + assert_eq!(creds.signature(), None, "missing file has no signature"); + + std::fs::write(&path, r#"{"claudeAiOauth":{"accessToken":"a"}}"#).expect("write"); + let before = creds.signature().expect("file exists"); + assert_eq!( + Some(before.clone()), + creds.signature(), + "stable while unchanged" + ); + + std::fs::write(&path, r#"{"claudeAiOauth":{"accessToken":"rotated"}}"#).expect("rewrite"); + let after = creds.signature().expect("file exists"); + let _ = std::fs::remove_file(&path); + assert_ne!(before, after); + } + #[test] fn missing_fields_are_reported() { let err = parse_claude_json(&fixture("claude-credentials-missing-token.json")).unwrap_err(); diff --git a/src/usage/refresh.rs b/src/usage/refresh.rs index b13c871..0aa3e52 100644 --- a/src/usage/refresh.rs +++ b/src/usage/refresh.rs @@ -20,6 +20,17 @@ const EXTRA_HOME_DIRS: &[&str] = &[".local/bin", ".claude/local"]; #[cfg(unix)] const EXTRA_SYSTEM_DIRS: &[&str] = &["/opt/homebrew/bin", "/usr/local/bin"]; +/// `codex exec` refuses to run outside a trusted directory or repository, +/// and the refresh runs from the temp directory, so that check is skipped. +/// The read-only sandbox keeps the one-shot prompt from changing any file. +const CODEX_ARGS: &[&str] = &[ + "exec", + "--skip-git-repo-check", + "--sandbox", + "read-only", + ".", +]; + #[derive(Debug, Clone, Copy, PartialEq, Eq)] pub enum Outcome { Refreshed, @@ -42,9 +53,19 @@ impl Orchestrator { } pub fn refresh(&self, source: &dyn CredentialSource) -> Outcome { + self.refresh_with(source, spawn_cli) + } + + /// Spawn the refresh CLI through `spawn`, then wait for the credential + /// signature to change. `spawn` returns `false` when no CLI could start. + fn refresh_with( + &self, + source: &dyn CredentialSource, + spawn: impl FnOnce(&RefreshHint) -> bool, + ) -> Outcome { let initial_sig = source.signature(); let hint = source.refresh_hint(); - if !spawn_cli(&hint) { + if !spawn(&hint) { return Outcome::CliMissing; } let start = Instant::now(); @@ -73,7 +94,7 @@ fn cli_for(hint: &RefreshHint) -> (&'static [&'static str], &'static [&'static s RefreshHint::LocalClaudeCli => (&["claude.cmd", "claude.exe", "claude"], &["-p", "."]), RefreshHint::LocalCodexCli => ( &["codex.cmd", "codex.ps1", "codex.exe", "codex"], - &["exec", "."], + CODEX_ARGS, ), } } @@ -82,18 +103,22 @@ fn cli_for(hint: &RefreshHint) -> (&'static [&'static str], &'static [&'static s fn cli_for(hint: &RefreshHint) -> (&'static [&'static str], &'static [&'static str]) { match hint { RefreshHint::LocalClaudeCli => (&["claude"], &["-p", "."]), - RefreshHint::LocalCodexCli => (&["codex"], &["exec", "."]), + RefreshHint::LocalCodexCli => (&["codex"], CODEX_ARGS), } } /// Spawn the first candidate that starts. The child is detached: a reaper -/// thread waits on it so no zombie is left behind. +/// thread waits on it so no zombie is left behind. The child runs from the +/// temp directory rather than the app's own working directory (the home +/// folder, `/` or System32), so the CLI never runs inside a project or +/// loads its instructions and hooks. fn spawn_first(candidates: &[&str], args: &[&str]) -> bool { for name in candidates { let Some(mut cmd) = command_for(name, args) else { continue; }; - cmd.env_remove("CLAUDECODE") + cmd.current_dir(std::env::temp_dir()) + .env_remove("CLAUDECODE") .env_remove("CLAUDE_CODE_ENTRYPOINT") .stdin(Stdio::null()) .stdout(Stdio::null()) @@ -209,6 +234,129 @@ fn find_executable(name: &str, dirs: &[std::path::PathBuf]) -> Option, + } + + fn fake(id: &str, version: u32) -> (FakeSource, Arc) { + let version = Arc::new(AtomicU32::new(version)); + let source = FakeSource { + id: id.to_string(), + version: Arc::clone(&version), + }; + (source, version) + } + + impl CredentialSource for FakeSource { + fn id(&self) -> &str { + &self.id + } + + fn read(&self) -> Result { + Err(CredsError::Unavailable) + } + + fn signature(&self) -> Option { + match self.version.load(Ordering::SeqCst) { + 0 => None, + v => Some(format!("{}|{v}", self.id)), + } + } + + fn refresh_hint(&self) -> RefreshHint { + RefreshHint::LocalCodexCli + } + } + + fn fast_orchestrator() -> Orchestrator { + Orchestrator { + timeout: Duration::from_millis(30), + poll_interval: Duration::from_millis(2), + } + } + + #[test] + fn codex_refresh_skips_the_repository_check_in_a_read_only_sandbox() { + let (_, args) = cli_for(&RefreshHint::LocalCodexCli); + assert_eq!(args.first(), Some(&"exec")); + assert!(args.contains(&"--skip-git-repo-check"), "{args:?}"); + let sandbox = args + .iter() + .position(|a| *a == "--sandbox") + .expect("sandbox flag"); + assert_eq!(args.get(sandbox + 1), Some(&"read-only")); + } + + #[test] + fn signature_change_after_spawn_is_refreshed() { + let (source, version) = fake("fake", 1); + let outcome = fast_orchestrator().refresh_with(&source, |hint| { + assert!(matches!(hint, RefreshHint::LocalCodexCli)); + version.store(2, Ordering::SeqCst); + true + }); + assert_eq!(outcome, Outcome::Refreshed); + } + + #[test] + fn unchanged_signature_times_out() { + let (source, _) = fake("fake", 1); + let outcome = fast_orchestrator().refresh_with(&source, |_| true); + assert_eq!(outcome, Outcome::Timeout); + } + + #[test] + fn failed_spawn_is_cli_missing_without_waiting() { + let (source, _) = fake("fake", 1); + let orchestrator = Orchestrator { + timeout: Duration::from_secs(60), + poll_interval: Duration::from_secs(60), + }; + let start = Instant::now(); + assert_eq!( + orchestrator.refresh_with(&source, |_| false), + Outcome::CliMissing + ); + assert!(start.elapsed() < Duration::from_secs(5)); + } + + #[test] + fn first_available_skips_missing_sources_and_keeps_order() { + let (missing, _) = fake("missing", 0); + let (second, _) = fake("second", 1); + let (third, _) = fake("third", 1); + let locator = Locator::new(vec![Box::new(missing), Box::new(second), Box::new(third)]); + assert_eq!(locator.first_available().map(|s| s.id()), Some("second")); + + let (only_missing, _) = fake("missing", 0); + assert!(Locator::new(vec![Box::new(only_missing)]) + .first_available() + .is_none()); + assert!(Locator::new(Vec::new()).first_available().is_none()); + } + + #[test] + fn first_available_prefers_an_earlier_source_once_it_appears() { + let (first, first_version) = fake("first", 0); + let (second, _) = fake("second", 1); + let locator = Locator::new(vec![Box::new(first), Box::new(second)]); + assert_eq!(locator.first_available().map(|s| s.id()), Some("second")); + first_version.store(1, Ordering::SeqCst); + assert_eq!(locator.first_available().map(|s| s.id()), Some("first")); + } +} + #[cfg(all(test, unix))] mod tests { use std::ffi::OsString; @@ -308,6 +456,32 @@ mod tests { let _ = std::fs::remove_dir_all(dir); } + #[test] + fn spawned_child_runs_in_the_temp_directory() { + let dir = temp_dir("cwd"); + let script = dir.join("fake-cli"); + let marker = dir.join("cwd"); + std::fs::write( + &script, + format!("#!/bin/sh\npwd -P > '{}'\n", marker.display()), + ) + .expect("write script"); + std::fs::set_permissions(&script, std::fs::Permissions::from_mode(0o755)).expect("chmod"); + assert!(spawn_first(&[script.to_str().expect("utf8 path")], &[])); + // The file exists before `pwd` writes, so wait for its full line. + let read = || std::fs::read_to_string(&marker).unwrap_or_default(); + let deadline = Instant::now() + Duration::from_secs(10); + while !read().ends_with('\n') && Instant::now() < deadline { + std::thread::sleep(Duration::from_millis(20)); + } + let written = read(); + let expected = std::env::temp_dir() + .canonicalize() + .expect("temp dir resolves"); + assert_eq!(PathBuf::from(written.trim()), expected); + let _ = std::fs::remove_dir_all(dir); + } + #[test] fn child_path_resolves_env_interpreter_from_extra_dirs() { let tools = temp_dir("tools");