diff --git a/crates/sign/tests/it/pdf.rs b/crates/sign/tests/it/pdf.rs index b7de2a4d3..3cad0fa75 100644 --- a/crates/sign/tests/it/pdf.rs +++ b/crates/sign/tests/it/pdf.rs @@ -665,12 +665,20 @@ trailer << /Root 1 0 R >> } } +/// Held by every test that changes the user's keychain search list. Each saves the list, changes +/// it and restores what it saved, so two at once could undo each other's temporary keychain or +/// put back one already deleted. +#[cfg(target_os = "macos")] +static KEYCHAIN_SEARCH_LIST: std::sync::Mutex<()> = std::sync::Mutex::new(()); + /// Signing with Keychain identities, in a throwaway keychain file. Ignored by default: creating /// a keychain touches the user's keychain search list (restored afterwards). #[cfg(target_os = "macos")] #[test] #[ignore = "creates a temporary macOS keychain; run with --ignored"] fn signing_with_keychain_identities() { + // A test that failed while holding it leaves the list restored, so a poisoned lock is fine. + let _list = KEYCHAIN_SEARCH_LIST.lock().unwrap_or_else(std::sync::PoisonError::into_inner); use std::process::Command; let dir = std::env::temp_dir().join(format!("pdfcraft-keychain-{}", std::process::id())); std::fs::create_dir_all(&dir).unwrap(); @@ -683,7 +691,7 @@ fn signing_with_keychain_identities() { let restore = || { let mut args = vec!["list-keychains", "-d", "user", "-s"]; args.extend(saved.iter().map(String::as_str)); - sec(&args); + sec(&args) }; restore(); // Everything that can fail runs before the clean-up below. @@ -705,9 +713,11 @@ fn signing_with_keychain_identities() { assert_eq!(s.status, Status::Valid, "{:?}", s.details); } })); + // The list first: it is what outlives the test if deleting the keychain goes wrong. + let restored = restore(); sec(&["delete-keychain", k]); - restore(); let _ = std::fs::remove_dir_all(&dir); + assert!(restored.status.success(), "the keychain search list wasn't restored: {}", String::from_utf8_lossy(&restored.stderr)); result.unwrap(); } @@ -718,20 +728,28 @@ fn signing_with_keychain_identities() { #[test] #[ignore = "creates a temporary macOS keychain; run with --ignored"] fn keychain_identities_carry_their_issuers() { + // A test that failed while holding it leaves the list restored, so a poisoned lock is fine. + let _list = KEYCHAIN_SEARCH_LIST.lock().unwrap_or_else(std::sync::PoisonError::into_inner); use std::process::Command; - let dir = std::env::temp_dir().join(format!("pdfcraft-keychain-chain-{}", std::process::id())); - std::fs::create_dir_all(&dir).unwrap(); + // The same path every run: trustd remembers which keychain file it found an issuer in, and + // after a run's keychain is deleted it keeps looking there, so a keychain at a new path + // would get an empty chain until that memory expires. + let dir = std::env::temp_dir().join("pdfcraft-keychain-chain"); let kc = dir.join("test.keychain-db"); let sec = |args: &[&str]| Command::new("security").args(args).output().unwrap(); + let k = kc.to_str().unwrap(); + // Left over by a run that was killed before its clean-up. + sec(&["delete-keychain", k]); + let _ = std::fs::remove_dir_all(&dir); + std::fs::create_dir_all(&dir).unwrap(); let list = String::from_utf8(sec(&["list-keychains", "-d", "user"]).stdout).unwrap(); let saved: Vec = list.lines().map(|l| l.trim().trim_matches('"').to_string()).filter(|l| !l.is_empty()).collect(); - let k = kc.to_str().unwrap(); assert!(sec(&["create-keychain", "-p", "pc-test", k]).status.success()); let set_list = |extra: Option<&str>| { let mut args = vec!["list-keychains", "-d", "user", "-s"]; args.extend(saved.iter().map(String::as_str)); args.extend(extra); - sec(&args); + sec(&args) }; // macOS finds an identity's issuers in the keychains on the search list. set_list(Some(k)); @@ -742,15 +760,29 @@ fn keychain_identities_carry_their_issuers() { assert!(out.status.success(), "chain-legacy.p12: {}", String::from_utf8_lossy(&out.stderr)); let listed = pdfcraft_sign::keychain::identities(Some(&kc)).unwrap(); assert_eq!(listed.len(), 1, "{listed:?}"); - let id = pdfcraft_sign::keychain::find(&pdfcraft_sign::keychain::reference(&listed[0].certificate)).unwrap(); + // The Security framework caches the search list in-process and learns of `security + // list-keychains` asynchronously, so after another Keychain test it takes a moment to look + // in the new keychain. + let reference = pdfcraft_sign::keychain::reference(&listed[0].certificate); + let mut found = pdfcraft_sign::keychain::find(&reference); + for _ in 0..50 { + if found.is_ok() { + break; + } + std::thread::sleep(std::time::Duration::from_millis(100)); + found = pdfcraft_sign::keychain::find(&reference); + } + let id = found.unwrap(); let issuers: Vec<_> = id.chain.iter().map(|c| c.subject.common_name()).collect(); assert_eq!(issuers, [Some("PrintCraft Test Root CA")]); let file = pkcs12::open(&data("chain.p12"), "test").unwrap(); assert_eq!(id.chain.iter().map(|c| &c.raw).collect::>(), file.chain.iter().map(|c| &c.raw).collect::>()); })); + // The list first: it is what outlives the test if deleting the keychain goes wrong. + let restored = set_list(None); sec(&["delete-keychain", k]); - set_list(None); let _ = std::fs::remove_dir_all(&dir); + assert!(restored.status.success(), "the keychain search list wasn't restored: {}", String::from_utf8_lossy(&restored.stderr)); result.unwrap(); }