From 6a4349e0c52cc2151d743edce6fd2486fd7cc176 Mon Sep 17 00:00:00 2001 From: yishuiliunian Date: Wed, 27 May 2026 10:02:28 +0800 Subject: [PATCH] test(hub-vault): add unit coverage for HubVaultService (14 tests) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `loopal-hub-vault` had no tests/ dir — HubVaultService was only exercised transitively through `e2e_real_vault_test` and `e2e_secret_access_boundary_test` in `loopal-agent-hub` (4 tests total). Direct method-level coverage of the crate's cache semantics, fallback-naming policy, and template-expansion paths was missing. New target `//crates/loopal-hub-vault:loopal-hub-vault_test` (14 tests): - `get_returns_secret_for_existing_name` - `get_returns_secret_not_found_for_missing_name` - `list_names_returns_all_keys_sorted` - `vault_for_missing_cwd_returns_vault_not_found` (empty vaults/ dir) - `vault_for_nonexistent_path_returns_vault_not_found` (canonicalize fails) - `vault_for_caches_vault_handle_per_cwd` (5 sequential gets via cache) - `open_default_vault_prefers_default_when_multiple_present` - `open_default_vault_falls_back_to_alphabetical_first` (no "default" vault) - `expand_author_substitutes_double_brace_template` ({{secret:NAME}}) - `expand_wire_substitutes_secret_ref_template` () - `expand_author_propagates_missing_secret_error` - `expand_author_no_placeholders_passes_through_verbatim` - `two_cwds_get_independent_vault_instances` (cache keyed by canonical cwd) - `with_noop_audit_constructor_compiles_and_runs` (env-tolerant smoke) The tests use public age fixtures (alice's ed25519 keypair) to construct real on-disk AgeVault stores under tempdirs, then drive HubVaultService directly. No IPC, no Hub setup — pure method-level coverage. Closes the last identified audit gap from the systematic e2e review. --- crates/loopal-hub-vault/BUILD.bazel | 16 + crates/loopal-hub-vault/tests/suite.rs | 2 + .../tests/suite/service_test.rs | 325 ++++++++++++++++++ 3 files changed, 343 insertions(+) create mode 100644 crates/loopal-hub-vault/tests/suite.rs create mode 100644 crates/loopal-hub-vault/tests/suite/service_test.rs diff --git a/crates/loopal-hub-vault/BUILD.bazel b/crates/loopal-hub-vault/BUILD.bazel index 77449ce1..06195b7c 100644 --- a/crates/loopal-hub-vault/BUILD.bazel +++ b/crates/loopal-hub-vault/BUILD.bazel @@ -22,3 +22,19 @@ rust_test( crate = ":loopal-hub-vault", edition = "2024", ) + +rust_test( + name = "loopal-hub-vault_test", + srcs = glob(["tests/**/*.rs"]), + crate_root = "tests/suite.rs", + edition = "2024", + deps = [ + ":loopal-hub-vault", + "//crates/loopal-secret-client", + "//crates/loopal-vault-age", + "//crates/loopal-vault-api", + "@crates//:secrecy", + "@crates//:tempfile", + "@crates//:tokio", + ], +) diff --git a/crates/loopal-hub-vault/tests/suite.rs b/crates/loopal-hub-vault/tests/suite.rs new file mode 100644 index 00000000..3f445d50 --- /dev/null +++ b/crates/loopal-hub-vault/tests/suite.rs @@ -0,0 +1,2 @@ +#[path = "suite/service_test.rs"] +mod service_test; diff --git a/crates/loopal-hub-vault/tests/suite/service_test.rs b/crates/loopal-hub-vault/tests/suite/service_test.rs new file mode 100644 index 00000000..d5726909 --- /dev/null +++ b/crates/loopal-hub-vault/tests/suite/service_test.rs @@ -0,0 +1,325 @@ +//! Unit tests for HubVaultService — direct method-level coverage that the +//! existing agent-hub e2e tests skip in favor of full IPC round-trips. +//! +//! Audit gap: `loopal-hub-vault` had no tests/ dir at all. The crate's +//! cache semantics, fallback-naming policy, and template-expansion methods +//! were only exercised transitively through `e2e_real_vault_test.rs`. + +use std::path::PathBuf; +use std::sync::Arc; + +use loopal_hub_vault::{AuditContext, HubVaultService}; +use loopal_secret_client::SecretError; +use loopal_vault_age::{AgeVault, DiscoveredIdentity, Recipients}; +use loopal_vault_api::{NoopAuditSink, Vault}; +use secrecy::{ExposeSecret, SecretString}; +use tempfile::TempDir; + +// Public test fixtures from the age crate (alice's ed25519 keypair). +const ED25519_UNENCRYPTED: &str = "-----BEGIN OPENSSH PRIVATE KEY----- +b3BlbnNzaC1rZXktdjEAAAAABG5vbmUAAAAEbm9uZQAAAAAAAAABAAAAMwAAAAtzc2gtZW +QyNTUxOQAAACB7Ci6nqZYaVvrjm8+XbzII89TsXzP111AflR7WeorBjQAAAJCfEwtqnxML +agAAAAtzc2gtZWQyNTUxOQAAACB7Ci6nqZYaVvrjm8+XbzII89TsXzP111AflR7WeorBjQ +AAAEADBJvjZT8X6JRJI8xVq/1aU8nMVgOtVnmdwqWwrSlXG3sKLqeplhpW+uObz5dvMgjz +1OxfM/XXUB+VHtZ6isGNAAAADHN0cjRkQGNhcmJvbgE= +-----END OPENSSH PRIVATE KEY----- +"; +const PUBKEY_ALICE: &str = + "ssh-ed25519 AAAAC3NzaC1lZDI1NTE5AAAAIHsKLqeplhpW+uObz5dvMgjz1OxfM/XXUB+VHtZ6isGN alice@rust"; + +#[cfg(unix)] +fn write_key_0600(path: &std::path::Path, content: &str) { + use std::os::unix::fs::PermissionsExt; + std::fs::write(path, content).unwrap(); + let mut p = std::fs::metadata(path).unwrap().permissions(); + p.set_mode(0o600); + std::fs::set_permissions(path, p).unwrap(); +} + +#[cfg(not(unix))] +fn write_key_0600(path: &std::path::Path, content: &str) { + std::fs::write(path, content).unwrap(); +} + +struct VaultFixture { + _tmp: TempDir, + cwd: PathBuf, + identity: Arc, +} + +async fn vault_dir(cwd: &std::path::Path, name: &str) -> (PathBuf, AgeVault) { + let dir = cwd + .join(".loopal") + .join("vaults") + .join(format!("{name}.vault")); + std::fs::create_dir_all(&dir).unwrap(); + let recipients_path = dir.join("recipients"); + let mut recipients = Recipients::new(); + recipients.add_line(PUBKEY_ALICE).unwrap(); + recipients.write(&recipients_path).unwrap(); + let key_path = cwd.join("id_ed25519"); + write_key_0600(&key_path, ED25519_UNENCRYPTED); + let identity = Arc::new(loopal_vault_age::load(&key_path).unwrap()); + let store_path = dir.join("store.age"); + let vault = AgeVault::new(store_path, recipients_path, identity); + vault.rekey().await.unwrap(); + (dir, vault) +} + +async fn setup_fixture(pairs: &[(&str, &str)]) -> VaultFixture { + let tmp = tempfile::tempdir().unwrap(); + let cwd = tmp.path().to_path_buf(); + let (_dir, vault) = vault_dir(&cwd, "default").await; + for (k, v) in pairs { + vault + .put(k, SecretString::from((*v).to_string())) + .await + .expect("vault put"); + } + let key_path = cwd.join("id_ed25519"); + let identity = Arc::new(loopal_vault_age::load(&key_path).unwrap()); + VaultFixture { + _tmp: tmp, + cwd, + identity, + } +} + +fn make_service(identity: Arc) -> HubVaultService { + HubVaultService::with_identity(identity, Arc::new(NoopAuditSink)) +} + +fn audit_ctx() -> AuditContext { + AuditContext { + agent_name: "test-agent".into(), + depth: 0, + tool_name: None, + } +} + +#[tokio::test] +async fn get_returns_secret_for_existing_name() { + let fx = setup_fixture(&[("api_key", "sk-test-123")]).await; + let svc = make_service(fx.identity.clone()); + let secret = svc + .get(&fx.cwd, "api_key", audit_ctx()) + .await + .expect("get must succeed for existing key"); + assert_eq!(secret.expose_secret(), "sk-test-123"); +} + +#[tokio::test] +async fn get_returns_secret_not_found_for_missing_name() { + let fx = setup_fixture(&[("present", "x")]).await; + let svc = make_service(fx.identity.clone()); + let err = svc.get(&fx.cwd, "absent", audit_ctx()).await.unwrap_err(); + match err { + SecretError::SecretNotFound(name) => assert_eq!(name, "absent"), + other => panic!("expected SecretNotFound, got: {other:?}"), + } +} + +#[tokio::test] +async fn list_names_returns_all_keys_sorted() { + let fx = setup_fixture(&[("k1", "v1"), ("k2", "v2"), ("openai_api", "sk-x")]).await; + let svc = make_service(fx.identity.clone()); + let mut names = svc.list_names(&fx.cwd).await.unwrap(); + names.sort(); + assert_eq!(names, vec!["k1", "k2", "openai_api"]); +} + +#[tokio::test] +async fn vault_for_missing_cwd_returns_vault_not_found() { + // No vault dir at this cwd → canonicalize succeeds (real tempdir) but + // open_default_vault yields VaultNotFound for the empty vaults/ dir. + let tmp = tempfile::tempdir().unwrap(); + let key_path = tmp.path().join("id_ed25519"); + write_key_0600(&key_path, ED25519_UNENCRYPTED); + let identity = Arc::new(loopal_vault_age::load(&key_path).unwrap()); + let svc = make_service(identity); + let err = svc + .get(tmp.path(), "anything", audit_ctx()) + .await + .unwrap_err(); + assert!( + matches!(err, SecretError::VaultNotFound(_)), + "expected VaultNotFound, got: {err:?}" + ); +} + +#[tokio::test] +async fn vault_for_nonexistent_path_returns_vault_not_found() { + // canonicalize() fails → mapped to VaultNotFound by vault_for. + let tmp = tempfile::tempdir().unwrap(); + let key_path = tmp.path().join("id_ed25519"); + write_key_0600(&key_path, ED25519_UNENCRYPTED); + let identity = Arc::new(loopal_vault_age::load(&key_path).unwrap()); + let svc = make_service(identity); + let ghost = tmp.path().join("does-not-exist-anywhere"); + let err = svc.get(&ghost, "anything", audit_ctx()).await.unwrap_err(); + assert!( + matches!(err, SecretError::VaultNotFound(_)), + "expected VaultNotFound for unresolvable path, got: {err:?}" + ); +} + +#[tokio::test] +async fn vault_for_caches_vault_handle_per_cwd() { + // Second call to `get` must NOT re-construct AgeVault. The contract is + // unobservable directly without a probe, so verify via a follow-up put + // through a separate AgeVault instance: if the cache is doing its job, + // the second `get` reads the updated value (because the cached vault + // shares the same on-disk store file, AgeVault has no in-memory cache). + // + // What's actually observable: multiple gets to the same cwd succeed in + // sequence without canonicalize churn (any error path would surface as + // VaultNotFound on the second call). + let fx = setup_fixture(&[("a", "1"), ("b", "2")]).await; + let svc = make_service(fx.identity.clone()); + + for _ in 0..5 { + let v = svc + .get(&fx.cwd, "a", audit_ctx()) + .await + .expect("repeated get must succeed via cache"); + assert_eq!(v.expose_secret(), "1"); + } +} + +#[tokio::test] +async fn open_default_vault_prefers_default_when_multiple_present() { + // Two vaults: "default" (with key "x") and "alt-other" (with key "y"). + // Lookup must hit "default", not "alt-other". + let tmp = tempfile::tempdir().unwrap(); + let cwd = tmp.path().to_path_buf(); + let (_d, default_vault) = vault_dir(&cwd, "default").await; + default_vault + .put("x", SecretString::from("from-default".to_string())) + .await + .unwrap(); + let (_a, alt_vault) = vault_dir(&cwd, "alt-other").await; + alt_vault + .put("x", SecretString::from("from-alt".to_string())) + .await + .unwrap(); + + let key_path = cwd.join("id_ed25519"); + let identity = Arc::new(loopal_vault_age::load(&key_path).unwrap()); + let svc = make_service(identity); + + let secret = svc.get(&cwd, "x", audit_ctx()).await.unwrap(); + assert_eq!( + secret.expose_secret(), + "from-default", + "default vault must win when present" + ); +} + +#[tokio::test] +async fn open_default_vault_falls_back_to_alphabetical_first() { + // No "default" vault — service must use the alphabetically-first name. + let tmp = tempfile::tempdir().unwrap(); + let cwd = tmp.path().to_path_buf(); + let (_a, a_vault) = vault_dir(&cwd, "alpha").await; + a_vault + .put("name", SecretString::from("from-alpha".to_string())) + .await + .unwrap(); + let (_b, b_vault) = vault_dir(&cwd, "beta").await; + b_vault + .put("name", SecretString::from("from-beta".to_string())) + .await + .unwrap(); + + let key_path = cwd.join("id_ed25519"); + let identity = Arc::new(loopal_vault_age::load(&key_path).unwrap()); + let svc = make_service(identity); + + let secret = svc.get(&cwd, "name", audit_ctx()).await.unwrap(); + assert_eq!( + secret.expose_secret(), + "from-alpha", + "no 'default' vault → alphabetical first must win" + ); +} + +#[tokio::test] +async fn expand_author_substitutes_double_brace_template() { + let fx = setup_fixture(&[("api_key", "sk-AUTHOR")]).await; + let svc = make_service(fx.identity.clone()); + let expanded = svc + .expand_author(&fx.cwd, "prefix-{{secret:api_key}}-suffix", audit_ctx()) + .await + .unwrap(); + assert_eq!(expanded.expose_secret(), "prefix-sk-AUTHOR-suffix"); +} + +#[tokio::test] +async fn expand_wire_substitutes_secret_ref_template() { + let fx = setup_fixture(&[("token", "WIRE-VAL")]).await; + let svc = make_service(fx.identity.clone()); + let expanded = svc + .expand_wire(&fx.cwd, "Bearer ", audit_ctx()) + .await + .unwrap(); + assert_eq!(expanded.expose_secret(), "Bearer WIRE-VAL"); +} + +#[tokio::test] +async fn expand_author_propagates_missing_secret_error() { + let fx = setup_fixture(&[("only_one", "x")]).await; + let svc = make_service(fx.identity.clone()); + let err = svc + .expand_author(&fx.cwd, "{{secret:nonexistent}}", audit_ctx()) + .await + .unwrap_err(); + match &err { + SecretError::SecretNotFound(name) => assert_eq!(name, "nonexistent"), + _ => panic!("missing secret during expansion must surface as SecretNotFound, got: {err:?}"), + } +} + +#[tokio::test] +async fn expand_author_no_placeholders_passes_through_verbatim() { + let fx = setup_fixture(&[("ignored", "x")]).await; + let svc = make_service(fx.identity.clone()); + let expanded = svc + .expand_author(&fx.cwd, "plain text with no template", audit_ctx()) + .await + .unwrap(); + assert_eq!(expanded.expose_secret(), "plain text with no template"); +} + +#[tokio::test] +async fn two_cwds_get_independent_vault_instances() { + let fx_a = setup_fixture(&[("api_key", "VAL-A")]).await; + let fx_b = setup_fixture(&[("api_key", "VAL-B")]).await; + // Both fixtures share the same identity (same key bytes), so we can + // use either's identity Arc here. + let svc = make_service(fx_a.identity.clone()); + + let a = svc.get(&fx_a.cwd, "api_key", audit_ctx()).await.unwrap(); + let b = svc.get(&fx_b.cwd, "api_key", audit_ctx()).await.unwrap(); + assert_eq!(a.expose_secret(), "VAL-A"); + assert_eq!(b.expose_secret(), "VAL-B"); +} + +#[tokio::test] +async fn with_noop_audit_constructor_compiles_and_runs() { + // Smoke test that the test-friendly with_noop_audit() path works in + // environments without an ~/.ssh/ key (will fail discover()). + // We don't assert success — this just exercises the err-bubble path. + let result = HubVaultService::with_noop_audit(); + match result { + Ok(_svc) => { + // Environment has a discoverable identity — fine, the service is usable. + } + Err(SecretError::DecryptFailed(msg)) => { + assert!( + msg.contains("SSH identity"), + "decrypt-failed must reference SSH identity discovery, got: {msg}" + ); + } + Err(other) => panic!("unexpected error from with_noop_audit: {other:?}"), + } +}