diff --git a/crates/agentkeys-core/src/backend.rs b/crates/agentkeys-core/src/backend.rs index dc839e1..20d0788 100644 --- a/crates/agentkeys-core/src/backend.rs +++ b/crates/agentkeys-core/src/backend.rs @@ -103,6 +103,7 @@ pub trait CredentialBackend: Send + Sync { child_pubkey: &PublicKey, request_type: AuthRequestType, request_details: &CanonicalBytes, + parent_wallet: Option<&WalletAddress>, ) -> Result; async fn fetch_auth_request( @@ -251,6 +252,7 @@ mod tests { _child_pubkey: &PublicKey, _request_type: AuthRequestType, _request_details: &CanonicalBytes, + _parent_wallet: Option<&WalletAddress>, ) -> Result { unimplemented!() } diff --git a/crates/agentkeys-core/src/mock_client.rs b/crates/agentkeys-core/src/mock_client.rs index c66a3ab..6861263 100644 --- a/crates/agentkeys-core/src/mock_client.rs +++ b/crates/agentkeys-core/src/mock_client.rs @@ -413,6 +413,7 @@ impl CredentialBackend for MockHttpClient { child_pubkey: &PublicKey, request_type: AuthRequestType, request_details: &CanonicalBytes, + parent_wallet: Option<&WalletAddress>, ) -> Result { let pubkey_b64 = base64::engine::general_purpose::STANDARD.encode(&child_pubkey.0); let details_b64 = base64::engine::general_purpose::STANDARD.encode(&request_details.0); @@ -441,6 +442,12 @@ impl CredentialBackend for MockHttpClient { request_body["identity_value"] = json!(identity_value); } + // --parent binding from the daemon's --parent flag (PR #22). Orthogonal + // to the Recover typed-identity fields above. + if let Some(pw) = parent_wallet { + request_body["parent_wallet"] = json!(pw.0); + } + let resp = self .client .post(self.url("/auth-request/open")) diff --git a/crates/agentkeys-daemon/Cargo.toml b/crates/agentkeys-daemon/Cargo.toml index 744bfef..86c01be 100644 --- a/crates/agentkeys-daemon/Cargo.toml +++ b/crates/agentkeys-daemon/Cargo.toml @@ -21,6 +21,7 @@ tracing-subscriber = { version = "0.3", features = ["env-filter"] } ed25519-dalek = { version = "2", features = ["rand_core"] } rand = "0.8" base64 = "0.22" +reqwest = { version = "0.12", features = ["json"] } [target.'cfg(target_os = "linux")'.dependencies] libc = "0.2" diff --git a/crates/agentkeys-daemon/src/main.rs b/crates/agentkeys-daemon/src/main.rs index c23b0f1..b73b919 100644 --- a/crates/agentkeys-daemon/src/main.rs +++ b/crates/agentkeys-daemon/src/main.rs @@ -39,6 +39,9 @@ struct Args { help = "Custom session namespace (default: derived from wallet address)" )] session_id: Option, + + #[arg(long, value_name = "ALIAS|WALLET", help = "Bind pair request to a specific master (alias or 0x... wallet)")] + parent: Option, } #[tokio::main] @@ -58,9 +61,15 @@ async fn main() -> anyhow::Result<()> { let backend = Arc::new(MockHttpClient::new(&args.backend)); + // --parent resolution is lazy: only the pair and master-approval recover + // paths use it, so resolving eagerly would crash non-pair startups when + // the backend is transiently down (codex PR #22 P3). Helper is called + // inside those branches only. + // 2. Determine session: env/file seam, pair flow, or recover flow let (sess, agent_id) = if let Some(token) = args.session { - // TEST SEAM: session injected directly (Stage 3 compatibility) + // TEST SEAM: session injected directly (Stage 3 compatibility). + // --parent is irrelevant here; no resolution is performed. let sess = session::build_session_from_token(token.clone()); let agent_id = WalletAddress(sess.wallet.0.clone()); let sid = args @@ -71,7 +80,7 @@ async fn main() -> anyhow::Result<()> { (sess, agent_id) } else if let Some(ref agent_identity) = args.recover { if let Some(ref method) = args.method { - // RECOVER VIA 2FA (no master approval needed) + // RECOVER VIA 2FA — no master approval, so --parent is unused. let result = pairing::run_recover_2fa_flow(&*backend, agent_identity, method) .await .context("2FA recover flow failed")?; @@ -86,10 +95,17 @@ async fn main() -> anyhow::Result<()> { .context("save recovered session")?; (result.session, agent_id) } else { - // RECOVER VIA MASTER APPROVAL - let result = pairing::run_recover_flow(&*backend, agent_identity, args.pair_timeout) - .await - .context("recover flow failed")?; + // RECOVER VIA MASTER APPROVAL — resolve --parent here, not at + // startup (codex P3). + let parent_wallet = resolve_parent_if_set(&args.backend, args.parent.as_deref()).await?; + let result = pairing::run_recover_flow( + &*backend, + agent_identity, + args.pair_timeout, + parent_wallet.as_ref(), + ) + .await + .context("recover flow failed")?; let agent_id = result.wallet.clone(); let sid = args .session_id @@ -181,11 +197,19 @@ async fn main() -> anyhow::Result<()> { (sess, agent_id) } None => { - // PAIR FLOW — no stored session found. Save only after pair - // succeeds and the wallet is known. - let result = pairing::run_pair_flow(&*backend, args.pair_timeout) - .await - .context("pair flow failed")?; + // PAIR FLOW — no stored session found. Resolve --parent lazily + // here (codex PR #22 P3) so transient backend failures on the + // --session / --recover --method paths don't crash startup. + // `--parent` binds the pair request to a specific master so + // the backend refuses approval from any other master. + let parent_wallet = resolve_parent_if_set(&args.backend, args.parent.as_deref()).await?; + let result = pairing::run_pair_flow( + &*backend, + args.pair_timeout, + parent_wallet.as_ref(), + ) + .await + .context("pair flow failed")?; let agent_id = result.wallet.clone(); let sid = args .session_id @@ -210,3 +234,101 @@ async fn main() -> anyhow::Result<()> { Ok(()) } + +/// True IFF `s` is a strict `0x` + 40 hex-digit wallet literal. Aliases like +/// `0x-office` or `0x+bar` (both legal per `cmd_link`) fail this check and +/// go through the identity-resolution path instead (codex PR #22 P2 — +/// 0x-prefix aliases misclassified as wallets). +fn looks_like_raw_wallet(s: &str) -> bool { + s.starts_with("0x") && s.len() == 42 && s[2..].chars().all(|c| c.is_ascii_hexdigit()) +} + +/// Resolve `--parent` to a wallet address if set, returning `Ok(None)` when +/// the flag is absent. +/// +/// Uses reqwest's `.query()` builder so aliases with reserved characters +/// (`+`, `&`, `%`, spaces) are percent-encoded per RFC 3986 (codex PR #22 +/// v1 P2 — URL encoding). +/// +/// All inputs — raw wallets included — go through `/identity/resolve` so +/// the backend can validate existence before the daemon opens a pair +/// request. Raw `0x...` wallets are normalized to lowercase first, which +/// matches the canonical form the backend stores; mixed-case checksummed +/// addresses therefore resolve cleanly instead of timing out at approval +/// (codex PR #22 v2 P2 — unknown wallet accepted + case mismatch). +async fn resolve_parent_if_set( + backend_url: &str, + parent: Option<&str>, +) -> anyhow::Result> { + let Some(raw) = parent else { + return Ok(None); + }; + + // Pick identity_type based on shape. Raw wallets get lowercased to + // match the backend's canonical storage form. + let (identity_type, identity_value) = if looks_like_raw_wallet(raw) { + ("wallet", raw.to_ascii_lowercase()) + } else { + ("alias", raw.to_string()) + }; + + let http = reqwest::Client::new(); + let resp = http + .get(format!("{backend_url}/identity/resolve")) + .query(&[ + ("identity_type", identity_type), + ("identity_value", identity_value.as_str()), + ]) + .send() + .await + .context("resolve --parent: HTTP request failed")?; + if !resp.status().is_success() { + anyhow::bail!( + "could not resolve --parent '{raw}' (identity_type={identity_type}): status={}", + resp.status() + ); + } + let body: serde_json::Value = resp + .json() + .await + .context("resolve --parent: JSON parse failed")?; + let wallet_str = body["wallet_address"] + .as_str() + .ok_or_else(|| anyhow::anyhow!("resolve --parent: missing wallet_address in response"))? + .to_string(); + Ok(Some(WalletAddress(wallet_str))) +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn looks_like_raw_wallet_accepts_canonical_hex() { + assert!(looks_like_raw_wallet( + "0x1234567890abcdef1234567890abcdef12345678" + )); + assert!(looks_like_raw_wallet( + "0xABCDEF1234567890ABCDEF1234567890ABCDEF12" + )); + } + + #[test] + fn looks_like_raw_wallet_rejects_0x_hyphen_alias() { + // `0x-office` is a valid alias per cmd_link; must NOT be treated as + // a literal wallet (codex PR #22 P2). + assert!(!looks_like_raw_wallet("0x-office")); + assert!(!looks_like_raw_wallet("0x+bar")); + } + + #[test] + fn looks_like_raw_wallet_rejects_short_or_non_hex() { + assert!(!looks_like_raw_wallet("0xdeadbeef")); // too short + assert!(!looks_like_raw_wallet( + "0x1234567890abcdef1234567890abcdef123456789" // 41 hex chars + )); + assert!(!looks_like_raw_wallet( + "0xGGGGGGGGGGGGGGGGGGGGGGGGGGGGGGGGGGGGGGGG" + )); // non-hex + } +} diff --git a/crates/agentkeys-daemon/src/pairing.rs b/crates/agentkeys-daemon/src/pairing.rs index f0682ff..55f257c 100644 --- a/crates/agentkeys-daemon/src/pairing.rs +++ b/crates/agentkeys-daemon/src/pairing.rs @@ -17,6 +17,7 @@ pub struct PairResult { pub async fn run_pair_flow( backend: &dyn CredentialBackend, poll_timeout_secs: u64, + parent_wallet: Option<&WalletAddress>, ) -> Result { let signing_key = ed25519_dalek::SigningKey::generate(&mut rand::rngs::OsRng); let pubkey_bytes = ed25519_dalek::VerifyingKey::from(&signing_key).to_bytes().to_vec(); @@ -28,7 +29,7 @@ pub async fn run_pair_flow( .map_err(|e| anyhow!("canonical_bytes failed: {e}"))?; let opened = backend - .open_auth_request(&child_pubkey, request_type, &request_details) + .open_auth_request(&child_pubkey, request_type, &request_details, parent_wallet) .await .context("open_auth_request failed")?; @@ -108,6 +109,7 @@ pub async fn run_recover_flow( backend: &dyn CredentialBackend, agent_identity_str: &str, poll_timeout_secs: u64, + parent_wallet: Option<&WalletAddress>, ) -> Result { let signing_key = ed25519_dalek::SigningKey::generate(&mut rand::rngs::OsRng); let pubkey_bytes = ed25519_dalek::VerifyingKey::from(&signing_key).to_bytes().to_vec(); @@ -127,7 +129,7 @@ pub async fn run_recover_flow( .map_err(|e| anyhow!("canonical_bytes failed: {e}"))?; let opened = backend - .open_auth_request(&child_pubkey, request_type, &request_details) + .open_auth_request(&child_pubkey, request_type, &request_details, parent_wallet) .await .context("open_auth_request (recover) failed")?; diff --git a/crates/agentkeys-daemon/tests/daemon_tests.rs b/crates/agentkeys-daemon/tests/daemon_tests.rs index ab8beb3..c566862 100644 --- a/crates/agentkeys-daemon/tests/daemon_tests.rs +++ b/crates/agentkeys-daemon/tests/daemon_tests.rs @@ -412,3 +412,79 @@ async fn mcp_tool_discovery() { assert!(tool["description"].is_string(), "tool {} must have description", tool["name"]); } } + +// --------------------------------------------------------------------------- +// Test 14: daemon_pair_with_parent_binds_correctly +// Opens a pair request pre-bound to master_a; master_a can approve it. +// --------------------------------------------------------------------------- +#[tokio::test] +async fn daemon_pair_with_parent_binds_correctly() { + use agentkeys_core::backend::CredentialBackend; + use agentkeys_types::{AuthRequestType, CanonicalBytes, PublicKey, Scope}; + + let backend = create_test_backend(); + + let (master_a_sess, master_a_wallet) = backend + .create_session(AuthToken::Mock("master-a".into())) + .await + .unwrap(); + + let signing_key = ed25519_dalek::SigningKey::generate(&mut rand::rngs::OsRng); + let child_pubkey = PublicKey(ed25519_dalek::VerifyingKey::from(&signing_key).to_bytes().to_vec()); + + let scope = Scope { services: vec![], read_only: false }; + let request_type = AuthRequestType::Pair { requested_scope: scope }; + let request_details = CanonicalBytes(serde_json::to_vec(&serde_json::json!({ "Pair": { "requested_scope": { "services": [], "read_only": false } } })).unwrap()); + + let opened = backend + .open_auth_request(&child_pubkey, request_type, &request_details, Some(&master_a_wallet)) + .await + .unwrap(); + + // master_a approves — should succeed + let result = backend.approve_auth_request(&master_a_sess, &opened.id).await; + assert!(result.is_ok(), "master_a should be able to approve its own bound request: {result:?}"); +} + +// --------------------------------------------------------------------------- +// Test 15: daemon_pair_wrong_parent_rejected +// Opens a pair request pre-bound to master_a; master_b tries to approve → rejected. +// --------------------------------------------------------------------------- +#[tokio::test] +async fn daemon_pair_wrong_parent_rejected() { + use agentkeys_core::backend::CredentialBackend; + use agentkeys_types::{AuthRequestType, CanonicalBytes, PublicKey, Scope}; + + let backend = create_test_backend(); + + let (_master_a_sess, master_a_wallet) = backend + .create_session(AuthToken::Mock("master-a-wrong".into())) + .await + .unwrap(); + + let (master_b_sess, _master_b_wallet) = backend + .create_session(AuthToken::Mock("master-b-wrong".into())) + .await + .unwrap(); + + let signing_key = ed25519_dalek::SigningKey::generate(&mut rand::rngs::OsRng); + let child_pubkey = PublicKey(ed25519_dalek::VerifyingKey::from(&signing_key).to_bytes().to_vec()); + + let scope = Scope { services: vec![], read_only: false }; + let request_type = AuthRequestType::Pair { requested_scope: scope }; + let request_details = CanonicalBytes(serde_json::to_vec(&serde_json::json!({ "Pair": { "requested_scope": { "services": [], "read_only": false } } })).unwrap()); + + let opened = backend + .open_auth_request(&child_pubkey, request_type, &request_details, Some(&master_a_wallet)) + .await + .unwrap(); + + // master_b tries to approve master_a's request — should be rejected + let result = backend.approve_auth_request(&master_b_sess, &opened.id).await; + assert!(result.is_err(), "master_b should not be able to approve master_a's bound request"); + let err_str = result.unwrap_err().to_string().to_lowercase(); + assert!( + err_str.contains("unauthorized") || err_str.contains("401") || err_str.contains("auth") || err_str.contains("session does not own"), + "error should indicate unauthorized: {err_str}" + ); +} diff --git a/crates/agentkeys-daemon/tests/pair_tests.rs b/crates/agentkeys-daemon/tests/pair_tests.rs index 93de72c..23939cb 100644 --- a/crates/agentkeys-daemon/tests/pair_tests.rs +++ b/crates/agentkeys-daemon/tests/pair_tests.rs @@ -59,7 +59,7 @@ async fn pair_full_loop() { let request_details = pair_canonical_bytes(&scope); let opened = backend - .open_auth_request(&child_pubkey, AuthRequestType::Pair { requested_scope: scope }, &request_details) + .open_auth_request(&child_pubkey, AuthRequestType::Pair { requested_scope: scope }, &request_details, None) .await .unwrap(); @@ -109,7 +109,7 @@ async fn pair_otp_matches() { let request_details = pair_canonical_bytes(&scope); let opened = backend - .open_auth_request(&child_pubkey, AuthRequestType::Pair { requested_scope: scope }, &request_details) + .open_auth_request(&child_pubkey, AuthRequestType::Pair { requested_scope: scope }, &request_details, None) .await .unwrap(); @@ -139,7 +139,7 @@ async fn pair_timeout_retry() { let request_details = pair_canonical_bytes(&scope); let opened = backend - .open_auth_request(&child_pubkey, AuthRequestType::Pair { requested_scope: scope }, &request_details) + .open_auth_request(&child_pubkey, AuthRequestType::Pair { requested_scope: scope }, &request_details, None) .await .unwrap(); @@ -224,7 +224,7 @@ async fn pair_replay_resistance() { let request_details = pair_canonical_bytes(&scope); let opened = backend - .open_auth_request(&child_pubkey, AuthRequestType::Pair { requested_scope: scope }, &request_details) + .open_auth_request(&child_pubkey, AuthRequestType::Pair { requested_scope: scope }, &request_details, None) .await .unwrap(); @@ -339,6 +339,7 @@ async fn recover_full_loop() { &child_pubkey, AuthRequestType::Pair { requested_scope: scope.clone() }, &request_details, + None, ) .await .unwrap(); @@ -392,6 +393,7 @@ async fn recover_full_loop() { new_daemon_pubkey: new_pubkey.0.clone(), }, &recover_details, + None, ) .await .unwrap(); @@ -451,6 +453,7 @@ async fn recover_unknown_identity() { new_daemon_pubkey: new_pubkey.0.clone(), }, &recover_details, + None, ) .await .unwrap(); @@ -485,6 +488,7 @@ async fn recover_old_pubkey_revoked() { &child_pubkey, AuthRequestType::Pair { requested_scope: scope }, &request_details, + None, ) .await .unwrap(); @@ -553,6 +557,7 @@ async fn recover_credentials_intact() { &child_pubkey, AuthRequestType::Pair { requested_scope: scope }, &request_details, + None, ) .await .unwrap(); @@ -596,6 +601,7 @@ async fn recover_credentials_intact() { new_daemon_pubkey: new_pubkey.0.clone(), }, &recover_details, + None, ) .await .unwrap(); diff --git a/crates/agentkeys-mock-server/src/test_client.rs b/crates/agentkeys-mock-server/src/test_client.rs index 975270d..c6f48a7 100644 --- a/crates/agentkeys-mock-server/src/test_client.rs +++ b/crates/agentkeys-mock-server/src/test_client.rs @@ -476,6 +476,7 @@ impl CredentialBackend for InProcessBackend { child_pubkey: &PublicKey, request_type: AuthRequestType, request_details: &CanonicalBytes, + parent_wallet: Option<&WalletAddress>, ) -> Result { let pubkey_b64 = base64::engine::general_purpose::STANDARD.encode(&child_pubkey.0); let details_b64 = base64::engine::general_purpose::STANDARD.encode(&request_details.0); @@ -504,12 +505,13 @@ impl CredentialBackend for InProcessBackend { request_body["identity_value"] = json!(identity_value); } - let body = self - .post( - "/auth-request/open", - request_body, - ) - .await?; + // --parent binding from PR #22 daemon flag — orthogonal to Recover + // typed-identity. + if let Some(pw) = parent_wallet { + request_body["parent_wallet"] = json!(pw.0); + } + + let body = self.post("/auth-request/open", request_body).await?; let id_str = body["id"] .as_str() diff --git a/docs/manual-test-issue-14.md b/docs/manual-test-issue-14.md new file mode 100644 index 0000000..c31d45f --- /dev/null +++ b/docs/manual-test-issue-14.md @@ -0,0 +1,101 @@ +# Manual Test: Issue #14 — Daemon `--parent` flag + +**Issue:** [litentry/agentKeys#14](https://github.com/litentry/agentKeys/issues/14) + +**Branch:** `fix/issue-14` + +## What changed + +Daemon now accepts `--parent ` which binds pair requests to a specific master. The backend refuses approvals from any other master. Without `--parent` the existing first-come-first-served behavior is preserved. + +## Preconditions + +```bash +cd ~/Projects/agentkeys +export AGENTKEYS_SESSION_STORE=file +export HOME_SANDBOX=$(mktemp -d) +export HOME=$HOME_SANDBOX +BACKEND=http://127.0.0.1:8090 +cargo build --release -p agentkeys-cli -p agentkeys-mock-server -p agentkeys-daemon +CLI=$(pwd)/target/release/agentkeys +DAEMON=$(pwd)/target/release/agentkeys-daemon +``` + +## Case 1 — daemon bound to a specific master via wallet + +```bash +cargo run --release -p agentkeys-mock-server & +MOCK_PID=$! +sleep 1 + +# Set up two masters +$CLI --backend $BACKEND init --mock-token master-a +A_WALLET=$(jq -r .wallet "$HOME/.agentkeys/session.json") +mv "$HOME/.agentkeys/session.json" "$HOME/.agentkeys/session-a.json" + +$CLI --backend $BACKEND init --mock-token master-b +B_WALLET=$(jq -r .wallet "$HOME/.agentkeys/session.json") +mv "$HOME/.agentkeys/session.json" "$HOME/.agentkeys/session-b.json" + +# Start daemon bound to master A +cp "$HOME/.agentkeys/session-a.json" "$HOME/.agentkeys/session.json" +$DAEMON --backend $BACKEND --parent "$A_WALLET" & +DAEMON_PID=$! +sleep 1 + +# Master B tries to approve the pair request → rejected +# (Expected: approve returns permission-denied / not-bound-to-this-master) + +kill $DAEMON_PID $MOCK_PID +``` + +## Case 2 — daemon bound via alias + +```bash +rm -f $HOME/.agentkeys/session.json +cargo run --release -p agentkeys-mock-server & +MOCK_PID=$! +sleep 1 + +$CLI --backend $BACKEND init --mock-token master-aliased +WALLET=$(jq -r .wallet "$HOME/.agentkeys/session.json") +$CLI --backend $BACKEND link $WALLET --alias office-mac + +$DAEMON --backend $BACKEND --parent office-mac & +DAEMON_PID=$! +sleep 1 +# Expected: daemon resolves "office-mac" to $WALLET, binds pair request. + +kill $DAEMON_PID $MOCK_PID +``` + +## Case 3 — no `--parent` (backward compatible) + +```bash +rm -f $HOME/.agentkeys/session.json +cargo run --release -p agentkeys-mock-server & +MOCK_PID=$! +sleep 1 + +$CLI --backend $BACKEND init --mock-token legacy-flow +$DAEMON --backend $BACKEND & +DAEMON_PID=$! +sleep 1 +# Expected: pair request has no parent_wallet; any master can claim (existing behavior). + +kill $DAEMON_PID $MOCK_PID +``` + +## Cleanup + +```bash +rm -rf "$HOME_SANDBOX" +``` + +## Cross-references + +- `crates/agentkeys-daemon/src/main.rs` — new `--parent` clap arg +- `crates/agentkeys-daemon/src/pairing.rs` — threads parent_wallet through +- `crates/agentkeys-core/src/backend.rs` — `open_auth_request` signature extended with `parent_wallet: Option<&WalletAddress>` +- `docs/manual-test-stage4.md` Test 8 — can be simplified to use `--parent` instead of raw curl once this lands (follow-up docs pass) +- Related: #13 (backend refactor — trait change stacks on top)