a73x

c461e69f

Load repo trust policy without the machine's global file in tests

a73x   2026-09-06 08:48

Commit message
Load repo trust policy without the machine's global file in tests
src/trust.rs
Old New
@@ -137,12 +137,20 @@ fn merge_keys(earlier: Vec<TrustedKey>, later: Vec<TrustedKey>) -> Vec<TrustedKe
137 /// Returns `TrustPolicy::Configured(keys)` if either file exists (even if empty). 137 /// Returns `TrustPolicy::Configured(keys)` if either file exists (even if empty).
138 /// Global keys are loaded first, then repo-local keys are merged in. 138 /// Global keys are loaded first, then repo-local keys are merged in.
139 /// Repo-local keys take precedence for labels on duplicate keys. 139 /// Repo-local keys take precedence for labels on duplicate keys.
140 ///
141 /// **Not for tests.** This reads the real `~/.config/git-collab/trusted-keys`,
142 /// so a test that calls it asserts against whatever keys the machine running
143 /// the suite happens to trust — nine tests in this module did exactly that and
144 /// failed on any developer machine with a global file (fixed 2026-09-06). Use
145 /// `load_trust_policy_with_global` with an explicit path, or `None`.
140 pub fn load_trust_policy(repo: &Repository) -> Result<TrustPolicy, Error> { 146 pub fn load_trust_policy(repo: &Repository) -> Result<TrustPolicy, Error> {
141 let global_path = global_trusted_keys_path().ok(); 147 let global_path = global_trusted_keys_path().ok();
142 load_trust_policy_with_global(repo, global_path.as_deref()) 148 load_trust_policy_with_global(repo, global_path.as_deref())
143 } 149 }
144 150
145 /// Load trust policy with an explicit global path (for testing). 151 /// Load trust policy with an explicit global path: `Some(path)` to merge that
152 /// file in, `None` for the repository's keys alone. This is the seam tests use
153 /// — see the warning on `load_trust_policy`.
146 pub fn load_trust_policy_with_global( 154 pub fn load_trust_policy_with_global(
147 repo: &Repository, 155 repo: &Repository,
148 global_path: Option<&Path>, 156 global_path: Option<&Path>,
@@ -326,6 +334,20 @@ mod tests {
326 use git2::Oid; 334 use git2::Oid;
327 use tempfile::TempDir; 335 use tempfile::TempDir;
328 336
337 /// A repository's own trust policy, with the machine's global file left
338 /// out of it.
339 ///
340 /// `load_trust_policy` merges `~/.config/git-collab/trusted-keys` in, so a
341 /// unit test that calls it asserts against whatever keys the machine
342 /// running the suite happens to trust: nine tests here failed on any
343 /// developer machine that had a global file, and passed on one that did
344 /// not (fixed 2026-09-06). Tests use this; the merge itself is covered by
345 /// `load_trust_policy_merges_global_and_repo_keys`, which injects a temp
346 /// path instead of reading a real one.
347 fn repo_policy(repo: &Repository) -> TrustPolicy {
348 load_trust_policy_with_global(repo, None).unwrap()
349 }
350
329 fn test_repo() -> (TempDir, Repository) { 351 fn test_repo() -> (TempDir, Repository) {
330 let dir = TempDir::new().unwrap(); 352 let dir = TempDir::new().unwrap();
331 let repo = Repository::init(dir.path()).unwrap(); 353 let repo = Repository::init(dir.path()).unwrap();
@@ -397,7 +419,7 @@ mod tests {
397 #[test] 419 #[test]
398 fn load_trust_policy_returns_unconfigured_when_no_file() { 420 fn load_trust_policy_returns_unconfigured_when_no_file() {
399 let (_dir, repo) = test_repo(); 421 let (_dir, repo) = test_repo();
400 let policy = load_trust_policy(&repo).unwrap(); 422 let policy = repo_policy(&repo);
401 assert!(matches!(policy, TrustPolicy::Unconfigured)); 423 assert!(matches!(policy, TrustPolicy::Unconfigured));
402 } 424 }
403 425
@@ -407,7 +429,7 @@ mod tests {
407 let path = trusted_keys_path(&repo); 429 let path = trusted_keys_path(&repo);
408 fs::create_dir_all(path.parent().unwrap()).unwrap(); 430 fs::create_dir_all(path.parent().unwrap()).unwrap();
409 fs::write(&path, "").unwrap(); 431 fs::write(&path, "").unwrap();
410 let policy = load_trust_policy(&repo).unwrap(); 432 let policy = repo_policy(&repo);
411 match policy { 433 match policy {
412 TrustPolicy::Configured(keys) => assert!(keys.is_empty()), 434 TrustPolicy::Configured(keys) => assert!(keys.is_empty()),
413 _ => panic!("expected Configured"), 435 _ => panic!("expected Configured"),
@@ -422,7 +444,7 @@ mod tests {
422 let path = trusted_keys_path(&repo); 444 let path = trusted_keys_path(&repo);
423 fs::create_dir_all(path.parent().unwrap()).unwrap(); 445 fs::create_dir_all(path.parent().unwrap()).unwrap();
424 fs::write(&path, format!("# Comment\n{} Alice\n{}\n\n", pk1, pk2)).unwrap(); 446 fs::write(&path, format!("# Comment\n{} Alice\n{}\n\n", pk1, pk2)).unwrap();
425 let policy = load_trust_policy(&repo).unwrap(); 447 let policy = repo_policy(&repo);
426 match policy { 448 match policy {
427 TrustPolicy::Configured(keys) => { 449 TrustPolicy::Configured(keys) => {
428 assert_eq!(keys.len(), 2); 450 assert_eq!(keys.len(), 2);
@@ -442,7 +464,7 @@ mod tests {
442 let path = trusted_keys_path(&repo); 464 let path = trusted_keys_path(&repo);
443 fs::create_dir_all(path.parent().unwrap()).unwrap(); 465 fs::create_dir_all(path.parent().unwrap()).unwrap();
444 fs::write(&path, format!("garbage_not_base64\n{} Good key\n", pk)).unwrap(); 466 fs::write(&path, format!("garbage_not_base64\n{} Good key\n", pk)).unwrap();
445 let policy = load_trust_policy(&repo).unwrap(); 467 let policy = repo_policy(&repo);
446 match policy { 468 match policy {
447 TrustPolicy::Configured(keys) => { 469 TrustPolicy::Configured(keys) => {
448 assert_eq!(keys.len(), 1); 470 assert_eq!(keys.len(), 1);
@@ -459,7 +481,7 @@ mod tests {
459 let path = trusted_keys_path(&repo); 481 let path = trusted_keys_path(&repo);
460 fs::create_dir_all(path.parent().unwrap()).unwrap(); 482 fs::create_dir_all(path.parent().unwrap()).unwrap();
461 fs::write(&path, format!("{} First\n{} Second\n", pk, pk)).unwrap(); 483 fs::write(&path, format!("{} First\n{} Second\n", pk, pk)).unwrap();
462 let policy = load_trust_policy(&repo).unwrap(); 484 let policy = repo_policy(&repo);
463 match policy { 485 match policy {
464 TrustPolicy::Configured(keys) => { 486 TrustPolicy::Configured(keys) => {
465 assert_eq!(keys.len(), 1); 487 assert_eq!(keys.len(), 1);
@@ -477,7 +499,7 @@ mod tests {
477 let pk2 = valid_test_pubkey(); 499 let pk2 = valid_test_pubkey();
478 save_trusted_key(&repo, &pk1, Some("Alice")).unwrap(); 500 save_trusted_key(&repo, &pk1, Some("Alice")).unwrap();
479 save_trusted_key(&repo, &pk2, None).unwrap(); 501 save_trusted_key(&repo, &pk2, None).unwrap();
480 let policy = load_trust_policy(&repo).unwrap(); 502 let policy = repo_policy(&repo);
481 match policy { 503 match policy {
482 TrustPolicy::Configured(keys) => { 504 TrustPolicy::Configured(keys) => {
483 assert_eq!(keys.len(), 2); 505 assert_eq!(keys.len(), 2);
@@ -498,7 +520,7 @@ mod tests {
498 assert!(added, "first add should return true"); 520 assert!(added, "first add should return true");
499 let added = save_trusted_key(&repo, &pk, Some("Alice again")).unwrap(); 521 let added = save_trusted_key(&repo, &pk, Some("Alice again")).unwrap();
500 assert!(!added, "duplicate add should return false"); 522 assert!(!added, "duplicate add should return false");
501 let policy = load_trust_policy(&repo).unwrap(); 523 let policy = repo_policy(&repo);
502 match policy { 524 match policy {
503 TrustPolicy::Configured(keys) => { 525 TrustPolicy::Configured(keys) => {
504 assert_eq!(keys.len(), 1, "should not have duplicates"); 526 assert_eq!(keys.len(), 1, "should not have duplicates");
@@ -589,7 +611,7 @@ mod tests {
589 let removed = remove_trusted_key(&repo, &pk1).unwrap(); 611 let removed = remove_trusted_key(&repo, &pk1).unwrap();
590 assert_eq!(removed.pubkey, pk1); 612 assert_eq!(removed.pubkey, pk1);
591 assert_eq!(removed.label.as_deref(), Some("Alice")); 613 assert_eq!(removed.label.as_deref(), Some("Alice"));
592 let policy = load_trust_policy(&repo).unwrap(); 614 let policy = repo_policy(&repo);
593 match policy { 615 match policy {
594 TrustPolicy::Configured(keys) => { 616 TrustPolicy::Configured(keys) => {
595 assert_eq!(keys.len(), 1); 617 assert_eq!(keys.len(), 1);
@@ -620,7 +642,7 @@ mod tests {
620 save_trusted_key(&repo, &pk, None).unwrap(); 642 save_trusted_key(&repo, &pk, None).unwrap();
621 remove_trusted_key(&repo, &pk).unwrap(); 643 remove_trusted_key(&repo, &pk).unwrap();
622 // File should still exist (Configured) but empty 644 // File should still exist (Configured) but empty
623 let policy = load_trust_policy(&repo).unwrap(); 645 let policy = repo_policy(&repo);
624 match policy { 646 match policy {
625 TrustPolicy::Configured(keys) => assert!(keys.is_empty()), 647 TrustPolicy::Configured(keys) => assert!(keys.is_empty()),
626 _ => panic!("expected Configured (empty)"), 648 _ => panic!("expected Configured (empty)"),