mirror of
https://github.com/nearai/ironclaw.git
synced 2026-09-03 08:06:01 +08:00
* 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>
247 lines
8.8 KiB
Rust
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());
|
|
}
|