Files
ironclaw/tests/module_init_integration.rs
Illia Polosukhin 9cf37364af fix(staging): repair broken test build and macOS-incompatible SSRF tests (#2064)
* fix(staging): repair broken test build and macOS-incompatible SSRF tests

Staging tip (f9ed8152) was failing `cargo test` for several unrelated
reasons. This commit gets the test suite back to green on both Linux CI
and macOS.

1. **wrapper.rs:5595** — `PairingStore::new()` was called with zero args
   in `test_http_request_rejects_private_ip_targets`. The signature
   changed to `(db, cache)` in #1898 and the other test in the same file
   (line 5573) was updated to use `PairingStore::new_noop()`, but this
   one was missed. Switch to `new_noop()` to match.

2. **CLI snapshot** — clap's `render_long_help` for `--auto-approve`
   now emits an indented blank line between the short and long
   description (10 spaces, not empty). Update the snapshot to match the
   new output and refresh `assertion_line` (438 -> 461).

3. **validate_base_url IPv6 bracket bug** — `Url::host_str()` returns
   IPv6 literals WITH the surrounding brackets (e.g. `[::1]`), but
   `IpAddr::parse` does not accept brackets — it wants bare `::1`. As a
   result the IPv6 SSRF defense was effectively dead code: every `[…]`
   host failed to parse and fell through to the DNS-resolution path.
   This passed on Linux CI by accident (because `to_socket_addrs` on
   Linux also fails on bracketed strings), but broke on any host whose
   resolver returns a public IP for unresolvable lookups (ISP captive
   portals, ad-injecting DNS providers). Strip the brackets before
   parsing so the IPv6 detection actually works as intended.

4. **DNS-hijack-tolerant test guards** — two tests
   (`validate_base_url_rejects_dns_failure`,
   `test_validate_public_https_url_fails_closed_on_dns_error`) rely on
   RFC 6761's promise that `.invalid` lookups fail. On networks with
   DNS hijacking that promise doesn't hold and the lookups succeed
   (typically resolving to a public ad-server IP). Probe with
   `ironclaw-dns-hijack-probe.invalid` and skip the test with an
   eprintln on hijacked-DNS networks. Coverage on CI is unchanged.

5. **ExtensionManager test isolation** — the
   `extension_manager_with_process_manager_constructs` integration test
   passes `store: None`, which makes `list()` fall back to file-based
   `load_mcp_servers()` reading `~/.ironclaw/mcp-servers.json`. Any
   locally installed MCP server (e.g. notion) leaked into the test and
   broke the empty assertion. Set `IRONCLAW_BASE_DIR` to a fresh
   tempdir at the top of the test (before the LazyLock is initialized)
   to fully isolate.

After this commit, `./scripts/dev-setup.sh` runs end-to-end and
`cargo test` passes 4709/0 locally on macOS as well as CI.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* fix(staging): address PR review feedback

- helpers.rs: simplify IPv6 bracket stripping to `host.trim_matches(...)`
  per gemini-code-assist suggestion. Functionally equivalent to the
  chained strip_prefix/strip_suffix/unwrap_or but cleaner; for any host
  string from `Url::host_str()` (which only ever returns matched
  brackets), the result is identical.

- setup/channels.rs: replace blocking `std::net::ToSocketAddrs` probe
  with `tokio::time::timeout(2s, tokio::net::lookup_host(...))` per
  Copilot review. The previous synchronous lookup could block a tokio
  worker thread inside `#[tokio::test]` and stall the suite on slow or
  offline DNS. The async resolver with a hard 2-second cap eliminates
  both risks.

- module_init_integration.rs: drop the brittle `is_empty()` assertion
  and the env-var override entirely, addressing both Copilot's review
  comment about parallel test ordering and the reviewer's deeper
  concern about touching process env from an integration test that
  cannot access the crate-private ENV_MUTEX. The test's actual purpose
  is to verify that ExtensionManager constructs and `list()` returns
  Ok — that's exactly what `is_ok()` checks. The empty assertion was
  always brittle (the test creates empty TOOL/CHANNEL dirs but does not
  isolate ~/.ironclaw, so any locally installed MCP server leaks in)
  and trying to "fix" it by mutating IRONCLAW_BASE_DIR from inside an
  integration test introduces order-dependent behaviour that the user
  flagged: parallel tests in the same binary can race the LazyLock,
  and there is no integration-test-visible mutex to serialise env
  mutations. Removing the assertion is the principled fix.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-04-06 08:34:27 -07:00

247 lines
8.8 KiB
Rust

//! Integration test for module-owned initialization factories.
//!
//! Verifies that the refactored factory functions in `db`, `secrets`,
//! `orchestrator`, and `extensions` modules wire up correctly end-to-end,
//! ensuring nothing was lost when initialization logic was moved out of
//! `main.rs` and `app.rs` into owning modules.
use std::sync::Arc;
use ironclaw::db::DatabaseHandles;
use ironclaw::secrets::{CreateSecretParams, SecretsCrypto, SecretsStore};
// ---------------------------------------------------------------------------
// Helpers
// ---------------------------------------------------------------------------
/// Build a libsql DatabaseConfig pointing at a temp file.
#[cfg(feature = "libsql")]
fn libsql_config(path: &std::path::Path) -> ironclaw::config::DatabaseConfig {
ironclaw::config::DatabaseConfig {
backend: ironclaw::config::DatabaseBackend::LibSql,
url: secrecy::SecretString::from(String::new()),
pool_size: 1,
ssl_mode: ironclaw::config::SslMode::Prefer,
libsql_path: Some(path.to_path_buf()),
libsql_url: None,
libsql_auth_token: None,
}
}
/// Build a master-key crypto instance for tests.
fn test_crypto() -> Arc<SecretsCrypto> {
let key = secrecy::SecretString::from(ironclaw::secrets::keychain::generate_master_key_hex());
Arc::new(SecretsCrypto::new(key).expect("test crypto"))
}
// ---------------------------------------------------------------------------
// connect_with_handles: returns Database + populated handles
// ---------------------------------------------------------------------------
#[cfg(feature = "libsql")]
#[tokio::test]
async fn connect_with_handles_returns_db_and_libsql_handle() {
let dir = tempfile::tempdir().expect("tempdir");
let db_path = dir.path().join("test.db");
let config = libsql_config(&db_path);
let (db, handles) = ironclaw::db::connect_with_handles(&config)
.await
.expect("connect_with_handles");
// Database trait object works — run a trivial operation.
db.run_migrations().await.expect("migrations");
// Handle is populated.
assert!(
handles.libsql_db.is_some(),
"libsql handle should be Some after connect_with_handles"
);
}
// ---------------------------------------------------------------------------
// connect_from_config delegates to connect_with_handles
// ---------------------------------------------------------------------------
#[cfg(feature = "libsql")]
#[tokio::test]
async fn connect_from_config_produces_working_db() {
let dir = tempfile::tempdir().expect("tempdir");
let db_path = dir.path().join("test.db");
let config = libsql_config(&db_path);
// connect_from_config delegates to connect_with_handles internally.
let db = ironclaw::db::connect_from_config(&config)
.await
.expect("connect_from_config");
// Verify usable — migrations should be idempotent.
db.run_migrations().await.expect("migrations");
}
// ---------------------------------------------------------------------------
// secrets::create_secrets_store from DatabaseHandles
// ---------------------------------------------------------------------------
#[cfg(feature = "libsql")]
#[tokio::test]
async fn secrets_store_from_handles_round_trips() {
let dir = tempfile::tempdir().expect("tempdir");
let db_path = dir.path().join("test.db");
let config = libsql_config(&db_path);
let (_db, handles) = ironclaw::db::connect_with_handles(&config)
.await
.expect("connect");
let crypto = test_crypto();
let store = ironclaw::secrets::create_secrets_store(crypto, &handles)
.expect("create_secrets_store should return Some for libsql");
// Round-trip a secret to prove the store works.
store
.create("test", CreateSecretParams::new("test_key", "test_value"))
.await
.expect("create secret");
let decrypted = store
.get_decrypted("test", "test_key")
.await
.expect("get_decrypted");
assert_eq!(decrypted.expose(), "test_value");
}
// ---------------------------------------------------------------------------
// db::create_secrets_store (standalone CLI factory)
// ---------------------------------------------------------------------------
#[cfg(feature = "libsql")]
#[tokio::test]
async fn db_create_secrets_store_standalone_round_trips() {
let dir = tempfile::tempdir().expect("tempdir");
let db_path = dir.path().join("test.db");
let config = libsql_config(&db_path);
let crypto = test_crypto();
let store = ironclaw::db::create_secrets_store(&config, crypto)
.await
.expect("db::create_secrets_store");
store
.create(
"test",
CreateSecretParams::new("standalone_key", "standalone_value"),
)
.await
.expect("create secret");
let decrypted = store
.get_decrypted("test", "standalone_key")
.await
.expect("get_decrypted");
assert_eq!(decrypted.expose(), "standalone_value");
}
// ---------------------------------------------------------------------------
// Both secrets factories produce equivalent stores
// ---------------------------------------------------------------------------
#[cfg(feature = "libsql")]
#[tokio::test]
async fn both_secrets_factories_produce_compatible_stores() {
let dir = tempfile::tempdir().expect("tempdir");
let db_path = dir.path().join("test.db");
let config = libsql_config(&db_path);
let crypto = test_crypto();
// Factory 1: connect_with_handles + secrets::create_secrets_store
let (_db, handles) = ironclaw::db::connect_with_handles(&config)
.await
.expect("connect");
let store_a = ironclaw::secrets::create_secrets_store(Arc::clone(&crypto), &handles)
.expect("store from handles");
// Factory 2: db::create_secrets_store (standalone)
let store_b = ironclaw::db::create_secrets_store(&config, crypto)
.await
.expect("standalone store");
// Write with factory 1, read with factory 2.
store_a
.create(
"test",
CreateSecretParams::new("cross_factory", "shared_secret"),
)
.await
.expect("create via store_a");
let decrypted = store_b
.get_decrypted("test", "cross_factory")
.await
.expect("read via store_b");
assert_eq!(decrypted.expose(), "shared_secret");
}
// ---------------------------------------------------------------------------
// ExtensionManager constructs with McpProcessManager
// ---------------------------------------------------------------------------
#[tokio::test]
async fn extension_manager_with_process_manager_constructs() {
use ironclaw::extensions::ExtensionManager;
use ironclaw::secrets::InMemorySecretsStore;
use ironclaw::tools::ToolRegistry;
use ironclaw::tools::mcp::McpProcessManager;
use ironclaw::tools::mcp::McpSessionManager;
let crypto = test_crypto();
let secrets: Arc<dyn SecretsStore + Send + Sync> = Arc::new(InMemorySecretsStore::new(crypto));
let tools = Arc::new(ToolRegistry::new());
let tools_dir = tempfile::tempdir().expect("tools_dir");
let channels_dir = tempfile::tempdir().expect("channels_dir");
let manager = ExtensionManager::new(
Arc::new(McpSessionManager::new()),
Arc::new(McpProcessManager::new()),
secrets,
tools,
None,
None,
tools_dir.path().to_path_buf(),
channels_dir.path().to_path_buf(),
None,
"test".to_string(),
None,
Vec::new(),
);
// Verify the manager is functional — list returns Ok.
//
// We do NOT assert the result is empty: with `store: None`,
// ExtensionManager.list() falls back to file-based load_mcp_servers()
// which reads ~/.ironclaw/mcp-servers.json. Asserting empty would leak
// the developer's local MCP-server configuration into the test, and
// overriding IRONCLAW_BASE_DIR from an integration test is unsafe (it
// would mutate process-wide env state in a binary that has no access
// to the crate-private ENV_MUTEX). The only thing this test is meant
// to verify is that the constructor wires up correctly and list() can
// be called without erroring — which is exactly what is_ok() checks.
let result = manager.list(None, false, "test").await;
assert!(result.is_ok(), "list should succeed on empty manager");
}
// ---------------------------------------------------------------------------
// DatabaseHandles: default is empty
// ---------------------------------------------------------------------------
#[test]
fn database_handles_default_is_empty() {
let handles = DatabaseHandles::default();
#[cfg(feature = "postgres")]
assert!(handles.pg_pool.is_none());
#[cfg(feature = "libsql")]
assert!(handles.libsql_db.is_none());
}