Skip to content

Commit 767f724

Browse files
authored
fix: macos keychain overwriting refreshed token (#318)
* docker config * tests
1 parent 3db658f commit 767f724

1 file changed

Lines changed: 105 additions & 0 deletions

File tree

src/session/container_config.rs

Lines changed: 105 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -210,6 +210,29 @@ fn copy_dir_recursive(src: &Path, dest: &Path) -> Result<()> {
210210
Ok(())
211211
}
212212

213+
/// Parse the `expiresAt` timestamp from a Claude Code credential JSON string.
214+
/// Returns `None` if the JSON is malformed or the field is missing/wrong type.
215+
#[cfg(any(target_os = "macos", test))]
216+
fn parse_credential_expires_at(content: &str) -> Option<u64> {
217+
let value: serde_json::Value = serde_json::from_str(content).ok()?;
218+
value.get("claudeAiOauth")?.get("expiresAt")?.as_u64()
219+
}
220+
221+
/// Decide whether an incoming credential should overwrite the existing one,
222+
/// based on `expiresAt` timestamps. Returns `true` if the incoming credential
223+
/// should be written.
224+
#[cfg(any(target_os = "macos", test))]
225+
fn should_overwrite_credential(existing_content: &str, incoming_content: &str) -> bool {
226+
let existing_exp = parse_credential_expires_at(existing_content);
227+
let incoming_exp = parse_credential_expires_at(incoming_content);
228+
229+
match (existing_exp, incoming_exp) {
230+
(Some(existing), Some(incoming)) => incoming > existing,
231+
(Some(_), None) => false,
232+
_ => true,
233+
}
234+
}
235+
213236
/// Extract credentials from the macOS Keychain and write to a file.
214237
/// Returns Ok(true) if credentials were written, Ok(false) if not available.
215238
#[cfg(target_os = "macos")]
@@ -265,6 +288,19 @@ fn extract_keychain_credential(service: &str, dest: &Path) -> Result<bool> {
265288
return Ok(false);
266289
}
267290

291+
// Only overwrite if the keychain credential is fresher than what the sandbox already has.
292+
if dest.exists() {
293+
if let Ok(existing_content) = std::fs::read_to_string(dest) {
294+
if !should_overwrite_credential(&existing_content, trimmed) {
295+
tracing::debug!(
296+
"Keychain credential for '{}' is not fresher than sandbox, keeping sandbox",
297+
service,
298+
);
299+
return Ok(false);
300+
}
301+
}
302+
}
303+
268304
std::fs::write(dest, trimmed)?;
269305
tracing::debug!(
270306
"Extracted keychain credential for '{}' -> {}",
@@ -1187,4 +1223,73 @@ mod tests {
11871223
r#"{"token":"abc"}"#
11881224
);
11891225
}
1226+
1227+
// --- credential freshness tests ---
1228+
1229+
#[test]
1230+
fn test_parse_credential_expires_at_valid() {
1231+
let json = r#"{"claudeAiOauth":{"expiresAt":1700000000}}"#;
1232+
assert_eq!(parse_credential_expires_at(json), Some(1700000000));
1233+
}
1234+
1235+
#[test]
1236+
fn test_parse_credential_expires_at_missing_key() {
1237+
// Missing claudeAiOauth entirely.
1238+
assert_eq!(parse_credential_expires_at(r#"{"other":"data"}"#), None);
1239+
// Missing expiresAt inside claudeAiOauth.
1240+
assert_eq!(
1241+
parse_credential_expires_at(r#"{"claudeAiOauth":{"token":"abc"}}"#),
1242+
None
1243+
);
1244+
}
1245+
1246+
#[test]
1247+
fn test_parse_credential_expires_at_invalid_json() {
1248+
assert_eq!(parse_credential_expires_at("not json at all"), None);
1249+
assert_eq!(parse_credential_expires_at(""), None);
1250+
}
1251+
1252+
#[test]
1253+
fn test_parse_credential_expires_at_wrong_type() {
1254+
// expiresAt is a string instead of a number.
1255+
let json = r#"{"claudeAiOauth":{"expiresAt":"1700000000"}}"#;
1256+
assert_eq!(parse_credential_expires_at(json), None);
1257+
}
1258+
1259+
#[test]
1260+
fn test_should_not_overwrite_with_stale_keychain() {
1261+
let sandbox = r#"{"claudeAiOauth":{"expiresAt":2000}}"#;
1262+
let keychain = r#"{"claudeAiOauth":{"expiresAt":1000}}"#;
1263+
assert!(!should_overwrite_credential(sandbox, keychain));
1264+
}
1265+
1266+
#[test]
1267+
fn test_should_overwrite_with_fresh_keychain() {
1268+
let sandbox = r#"{"claudeAiOauth":{"expiresAt":1000}}"#;
1269+
let keychain = r#"{"claudeAiOauth":{"expiresAt":2000}}"#;
1270+
assert!(should_overwrite_credential(sandbox, keychain));
1271+
}
1272+
1273+
#[test]
1274+
fn test_should_not_overwrite_equal_timestamps() {
1275+
let cred = r#"{"claudeAiOauth":{"expiresAt":1000}}"#;
1276+
assert!(!should_overwrite_credential(cred, cred));
1277+
}
1278+
1279+
#[test]
1280+
fn test_should_not_overwrite_when_keychain_unparseable() {
1281+
let sandbox = r#"{"claudeAiOauth":{"expiresAt":1000}}"#;
1282+
assert!(!should_overwrite_credential(sandbox, "not-json"));
1283+
}
1284+
1285+
#[test]
1286+
fn test_should_overwrite_when_both_unparseable() {
1287+
assert!(should_overwrite_credential("bad", "also-bad"));
1288+
}
1289+
1290+
#[test]
1291+
fn test_should_overwrite_when_only_keychain_parseable() {
1292+
let keychain = r#"{"claudeAiOauth":{"expiresAt":1000}}"#;
1293+
assert!(should_overwrite_credential("not-json", keychain));
1294+
}
11901295
}

0 commit comments

Comments
 (0)