diff --git a/crates/agentkeys-cli/src/lib.rs b/crates/agentkeys-cli/src/lib.rs index 55be167..afa4d53 100644 --- a/crates/agentkeys-cli/src/lib.rs +++ b/crates/agentkeys-cli/src/lib.rs @@ -3,7 +3,9 @@ use std::sync::Arc; use agentkeys_core::backend::{BackendError, CredentialBackend}; use agentkeys_core::mock_client::MockHttpClient; pub use agentkeys_core::session_store; -use agentkeys_types::{AuditEvent, AuditFilter, AuthToken, ServiceName, Session, WalletAddress}; +use agentkeys_types::{ + AuditEvent, AuditFilter, AuthToken, Scope, ServiceName, Session, WalletAddress, +}; use anyhow::{anyhow, Context, Result}; use serde_json::json; @@ -68,7 +70,7 @@ impl CommandContext { self } - fn load_session(&self) -> Result { + pub fn load_session(&self) -> Result { if let Some(ref s) = self.session_override { return Ok(s.clone()); } @@ -629,6 +631,152 @@ pub async fn cmd_approve(ctx: &CommandContext, pair_code: &str, auto_yes: bool) Ok("Approved. Agent paired successfully.".to_string()) } +async fn resolve_agent_to_wallet( + ctx: &CommandContext, + session: &Session, + agent: &str, +) -> Result { + if agent.starts_with("0x") { + return Ok(agent.to_string()); + } + // Resolve alias or email via /identity/resolve + let (identity_type, identity_value) = if agent.contains('@') { + ("email", agent) + } else { + ("alias", agent) + }; + // reqwest's .query() builder percent-encodes per RFC 3986 so identities + // containing '+', '&', '=', '%', spaces (e.g. plus-addressed emails like + // "bot+prod@example.com") are sent intact to the server. + let http_client = reqwest::Client::new(); + let resp = http_client + .get(format!("{}/identity/resolve", ctx.backend_url)) + .query(&[("identity_type", identity_type), ("identity_value", identity_value)]) + .header("authorization", format!("Bearer {}", session.token)) + .send() + .await + .context("GET /identity/resolve")?; + if !resp.status().is_success() { + let status = resp.status(); + let body: serde_json::Value = resp.json().await.unwrap_or(serde_json::Value::Null); + let msg = body["message"].as_str().unwrap_or("not found"); + return Err(anyhow!("Error: HTTP {}: {}", status, msg)); + } + let body: serde_json::Value = resp.json().await.context("parse identity/resolve response")?; + let wallet = body["wallet_address"] + .as_str() + .ok_or_else(|| anyhow!("identity/resolve returned no wallet_address"))? + .to_string(); + Ok(wallet) +} + +pub async fn cmd_scope( + ctx: &CommandContext, + agent: &str, + add: &[String], + remove: &[String], + set: Option<&str>, + list: bool, +) -> Result { + if set.is_some() && (!add.is_empty() || !remove.is_empty()) { + return Err(anyhow!( + "Error: --set is mutually exclusive with --add and --remove. Use one or the other." + )); + } + + // --list is read-only. Combining it with mutating flags would silently + // drop the mutation (the --list early-return happens before the update + // path), so reject the combo up front with a clear error. + if list && (set.is_some() || !add.is_empty() || !remove.is_empty()) { + return Err(anyhow!( + "Error: --list is mutually exclusive with --add, --remove, and --set. Use --list alone to read the current scope." + )); + } + + // `--add foo --remove foo` would silently no-op after mutation + // (retain after push cancels) yet still issue a backend write with a + // misleading "Scope updated" message. Reject up front (codex PR #29 + // v2 P2). + if !add.is_empty() && !remove.is_empty() { + let add_set: std::collections::HashSet<&str> = add.iter().map(|s| s.as_str()).collect(); + let overlap: Vec<&str> = remove + .iter() + .map(|s| s.as_str()) + .filter(|s| add_set.contains(s)) + .collect(); + if !overlap.is_empty() { + return Err(anyhow!( + "Error: the following services appear in both --add and --remove: {}. Pass each service to only one flag.", + overlap.join(", ") + )); + } + } + + if !list && set.is_none() && add.is_empty() && remove.is_empty() { + return Err(anyhow!( + "No action specified. Use --add, --remove, --set, or --list.\nRun `agentkeys scope --help` for usage." + )); + } + + let session = ctx.load_session().context("load session (run `agentkeys init` first)")?; + let target_wallet = WalletAddress(resolve_agent_to_wallet(ctx, &session, agent).await?); + let backend = ctx.backend(); + + let current_scope = backend + .get_scope(&session, &target_wallet) + .await + .map_err(wrap_backend_error)? + .unwrap_or(Scope { services: vec![], read_only: false }); + + if list { + let service_names: Vec<&str> = + current_scope.services.iter().map(|s| s.0.as_str()).collect(); + return Ok(format!( + "Scope for agent {}:\n services: [{}]\n read_only: {}", + target_wallet.0, + service_names.join(", "), + current_scope.read_only + )); + } + + let mut new_scope = if let Some(set_val) = set { + let mut services: Vec = set_val + .split(',') + .map(|s| s.trim()) + .filter(|s| !s.is_empty()) + .map(|s| ServiceName(s.to_string())) + .collect(); + services.sort_by(|a, b| a.0.cmp(&b.0)); + Scope { services, read_only: current_scope.read_only } + } else { + let mut services: Vec = current_scope.services.clone(); + for svc in add { + let name = ServiceName(svc.clone()); + if !services.contains(&name) { + services.push(name); + } + } + services.retain(|s| !remove.contains(&s.0)); + services.sort_by(|a, b| a.0.cmp(&b.0)); + Scope { services, read_only: current_scope.read_only } + }; + + backend + .update_scope(&session, &target_wallet, &new_scope) + .await + .map_err(wrap_backend_error)?; + + // `new_scope.services` is already sorted — both the --set branch + // (line 749) and the --add/--remove branch (line 760) sort before + // the update_scope call. + let service_names: Vec<&str> = new_scope.services.iter().map(|s| s.0.as_str()).collect(); + Ok(format!( + "Scope updated for agent {}. New services: [{}]", + target_wallet.0, + service_names.join(", ") + )) +} + pub fn cmd_feedback() -> String { let url = "https://github.com/agentkeys/agentkeys/discussions"; let opened = std::process::Command::new("open").arg(url).status().is_ok() diff --git a/crates/agentkeys-cli/src/main.rs b/crates/agentkeys-cli/src/main.rs index 05efadd..05e29bb 100644 --- a/crates/agentkeys-cli/src/main.rs +++ b/crates/agentkeys-cli/src/main.rs @@ -1,6 +1,6 @@ use agentkeys_cli::{ cmd_approve, cmd_feedback, cmd_init, cmd_link, cmd_read, cmd_recover, cmd_revoke, cmd_run, - cmd_store, cmd_teardown, cmd_usage, CommandContext, + cmd_scope, cmd_store, cmd_teardown, cmd_usage, CommandContext, }; @@ -139,6 +139,23 @@ enum Commands { yes: bool, }, + #[command( + about = "Edit or list the scope of a child agent", + long_about = "Add, remove, replace, or list the services in a child agent's scope.\n\nExamples:\n agentkeys scope 0xAGENT --add openrouter\n agentkeys scope 0xAGENT --remove anthropic\n agentkeys scope 0xAGENT --set openrouter,anthropic\n agentkeys scope 0xAGENT --list" + )] + Scope { + #[arg(help = "Agent wallet address, alias, or email")] + agent: String, + #[arg(long, help = "Add a service to the scope (repeatable)")] + add: Vec, + #[arg(long, help = "Remove a service from the scope (repeatable)")] + remove: Vec, + #[arg(long, help = "Replace the entire scope with a comma-separated list of services")] + set: Option, + #[arg(long, help = "List the current scope without making changes")] + list: bool, + }, + #[command( about = "Open the feedback forum in your browser", long_about = "Open https://github.com/agentkeys/agentkeys/discussions in the default browser.\n\nExamples:\n agentkeys feedback" @@ -168,6 +185,9 @@ async fn main() { } Commands::Recover { identity, method } => cmd_recover(&ctx, identity, method).await, Commands::Approve { pair_code, yes } => cmd_approve(&ctx, pair_code, *yes).await, + Commands::Scope { agent, add, remove, set, list } => { + cmd_scope(&ctx, agent, add, remove, set.as_deref(), *list).await + } Commands::Feedback => Ok(cmd_feedback()), }; diff --git a/crates/agentkeys-cli/tests/cli_tests.rs b/crates/agentkeys-cli/tests/cli_tests.rs index 9108669..626c940 100644 --- a/crates/agentkeys-cli/tests/cli_tests.rs +++ b/crates/agentkeys-cli/tests/cli_tests.rs @@ -1,6 +1,9 @@ use std::sync::Arc; -use agentkeys_cli::{cmd_init, cmd_link, cmd_read, cmd_revoke, cmd_run, cmd_store, cmd_teardown, cmd_usage, CommandContext}; +use agentkeys_cli::{ + cmd_init, cmd_link, cmd_read, cmd_revoke, cmd_run, cmd_scope, cmd_store, cmd_teardown, + cmd_usage, CommandContext, +}; use agentkeys_cli::session_store; use agentkeys_core::backend::CredentialBackend; use agentkeys_mock_server::test_client::InProcessBackend; @@ -708,3 +711,278 @@ async fn cmd_read_unknown_identity_errors_cleanly() { "error message should mention agentkeys link: {err}" ); } + +// --------------------------------------------------------------------------- +// Scope tests (15-19): require a real TCP server (cmd_scope uses reqwest) +// --------------------------------------------------------------------------- + +async fn start_scope_test_server() -> (String, String, String) { + use agentkeys_mock_server::{create_router, db, state::AppState}; + + let conn = rusqlite::Connection::open_in_memory().unwrap(); + db::init_schema(&conn).unwrap(); + let state = std::sync::Arc::new(AppState::new(conn)); + let router = create_router(state); + let listener = tokio::net::TcpListener::bind("127.0.0.1:0").await.unwrap(); + let addr = listener.local_addr().unwrap(); + tokio::spawn(async move { + axum::serve(listener, router).await.unwrap(); + }); + let base_url = format!("http://127.0.0.1:{}", addr.port()); + + unsafe { std::env::set_var("AGENTKEYS_SESSION_STORE", "file"); } + let bare_ctx = CommandContext::new(&base_url, false, false); + let (output, _session) = cmd_init(&bare_ctx, Some("scope-test-unique".to_string())) + .await + .unwrap(); + let master_wallet = output.split("Wallet: ").nth(1).unwrap().trim().to_string(); + + // Create a child session with initial scope [a, b] + let http_client = reqwest::Client::new(); + let child_resp: serde_json::Value = http_client + .post(format!("{}/session/child", base_url)) + .header("authorization", format!("Bearer {}", _session.token)) + .json(&serde_json::json!({ "scope": { "services": ["a", "b"], "read_only": false } })) + .send() + .await + .unwrap() + .json() + .await + .unwrap(); + let child_wallet = child_resp["wallet"].as_str().unwrap().to_string(); + + (base_url, _session.token, child_wallet) +} + +// Test 15: --add appends a service +#[tokio::test(flavor = "multi_thread")] +async fn cmd_scope_add_appends_service() { + let (base_url, master_token, child_wallet) = start_scope_test_server().await; + + let master_session = agentkeys_types::Session { + token: master_token, + wallet: agentkeys_types::WalletAddress("unused".to_string()), + scope: None, + created_at: 0, + ttl_seconds: 86400, + }; + + let ctx = CommandContext::new(&base_url, false, false).with_session(master_session); + let result = cmd_scope(&ctx, &child_wallet, &["c".to_string()], &[], None, false).await; + assert!(result.is_ok(), "cmd_scope --add failed: {:?}", result.err()); + let out = result.unwrap(); + assert!(out.contains("c"), "output should mention new service: {out}"); + + // Verify scope via /session/scope + let http_client = reqwest::Client::new(); + let scope_resp: serde_json::Value = http_client + .get(format!("{}/session/scope?wallet={}", base_url, child_wallet)) + .header("authorization", format!("Bearer {}", ctx.load_session().unwrap().token)) + .send() + .await + .unwrap() + .json() + .await + .unwrap(); + let services: Vec = scope_resp["services"] + .as_array() + .unwrap() + .iter() + .filter_map(|v| v.as_str().map(String::from)) + .collect(); + assert!(services.contains(&"a".to_string()), "should still have a: {:?}", services); + assert!(services.contains(&"b".to_string()), "should still have b: {:?}", services); + assert!(services.contains(&"c".to_string()), "should have new c: {:?}", services); +} + +// Test 16: --remove drops a service +#[tokio::test(flavor = "multi_thread")] +async fn cmd_scope_remove_drops_service() { + let (base_url, master_token, child_wallet) = start_scope_test_server().await; + + let master_session = agentkeys_types::Session { + token: master_token, + wallet: agentkeys_types::WalletAddress("unused".to_string()), + scope: None, + created_at: 0, + ttl_seconds: 86400, + }; + + let ctx = CommandContext::new(&base_url, false, false).with_session(master_session); + let result = cmd_scope(&ctx, &child_wallet, &[], &["a".to_string()], None, false).await; + assert!(result.is_ok(), "cmd_scope --remove failed: {:?}", result.err()); + + let http_client = reqwest::Client::new(); + let scope_resp: serde_json::Value = http_client + .get(format!("{}/session/scope?wallet={}", base_url, child_wallet)) + .header("authorization", format!("Bearer {}", ctx.load_session().unwrap().token)) + .send() + .await + .unwrap() + .json() + .await + .unwrap(); + let services: Vec = scope_resp["services"] + .as_array() + .unwrap() + .iter() + .filter_map(|v| v.as_str().map(String::from)) + .collect(); + assert!(!services.contains(&"a".to_string()), "a should be removed: {:?}", services); + assert!(services.contains(&"b".to_string()), "b should remain: {:?}", services); +} + +// Test 17: --set replaces the entire scope +#[tokio::test(flavor = "multi_thread")] +async fn cmd_scope_set_replaces() { + let (base_url, master_token, child_wallet) = start_scope_test_server().await; + + let master_session = agentkeys_types::Session { + token: master_token, + wallet: agentkeys_types::WalletAddress("unused".to_string()), + scope: None, + created_at: 0, + ttl_seconds: 86400, + }; + + let ctx = CommandContext::new(&base_url, false, false).with_session(master_session); + let result = cmd_scope(&ctx, &child_wallet, &[], &[], Some("c,d"), false).await; + assert!(result.is_ok(), "cmd_scope --set failed: {:?}", result.err()); + + let http_client = reqwest::Client::new(); + let scope_resp: serde_json::Value = http_client + .get(format!("{}/session/scope?wallet={}", base_url, child_wallet)) + .header("authorization", format!("Bearer {}", ctx.load_session().unwrap().token)) + .send() + .await + .unwrap() + .json() + .await + .unwrap(); + let services: Vec = scope_resp["services"] + .as_array() + .unwrap() + .iter() + .filter_map(|v| v.as_str().map(String::from)) + .collect(); + assert_eq!(services.len(), 2, "should have exactly c and d: {:?}", services); + assert!(services.contains(&"c".to_string()), "should have c: {:?}", services); + assert!(services.contains(&"d".to_string()), "should have d: {:?}", services); + assert!(!services.contains(&"a".to_string()), "a should be gone: {:?}", services); + assert!(!services.contains(&"b".to_string()), "b should be gone: {:?}", services); +} + +// Test 18: --list prints current scope and does NOT modify DB +#[tokio::test(flavor = "multi_thread")] +async fn cmd_scope_list_prints_current() { + let (base_url, master_token, child_wallet) = start_scope_test_server().await; + + let master_session = agentkeys_types::Session { + token: master_token.clone(), + wallet: agentkeys_types::WalletAddress("unused".to_string()), + scope: None, + created_at: 0, + ttl_seconds: 86400, + }; + + let ctx = CommandContext::new(&base_url, false, false).with_session(master_session); + let result = cmd_scope(&ctx, &child_wallet, &[], &[], None, true).await; + assert!(result.is_ok(), "cmd_scope --list failed: {:?}", result.err()); + let out = result.unwrap(); + assert!(out.contains("a") && out.contains("b"), "list output missing services: {out}"); + + // Verify nothing changed — scope is still [a, b] + let http_client = reqwest::Client::new(); + let scope_resp: serde_json::Value = http_client + .get(format!("{}/session/scope?wallet={}", base_url, child_wallet)) + .header("authorization", format!("Bearer {}", master_token)) + .send() + .await + .unwrap() + .json() + .await + .unwrap(); + let services: Vec = scope_resp["services"] + .as_array() + .unwrap() + .iter() + .filter_map(|v| v.as_str().map(String::from)) + .collect(); + assert_eq!(services.len(), 2, "scope should be unchanged after --list: {:?}", services); +} + +// Test 19: combining --add and --set returns a clear conflict error +#[tokio::test(flavor = "multi_thread")] +async fn cmd_scope_add_and_set_conflict_errors() { + let (base_url, master_token, child_wallet) = start_scope_test_server().await; + + let master_session = agentkeys_types::Session { + token: master_token, + wallet: agentkeys_types::WalletAddress("unused".to_string()), + scope: None, + created_at: 0, + ttl_seconds: 86400, + }; + + let ctx = CommandContext::new(&base_url, false, false).with_session(master_session); + let result = cmd_scope(&ctx, &child_wallet, &["x".to_string()], &[], Some("y"), false).await; + assert!(result.is_err(), "expected error when --add and --set are combined"); + let err = result.unwrap_err().to_string(); + assert!( + err.contains("mutually exclusive") || err.contains("--set") || err.contains("--add"), + "error message should mention the conflict: {err}" + ); +} + +// Test 20: --list + --add rejected up front (claude re-review follow-up). +#[tokio::test(flavor = "multi_thread")] +async fn cmd_scope_list_and_add_conflict_errors() { + let (base_url, master_token, child_wallet) = start_scope_test_server().await; + + let master_session = agentkeys_types::Session { + token: master_token, + wallet: agentkeys_types::WalletAddress("unused".to_string()), + scope: None, + created_at: 0, + ttl_seconds: 86400, + }; + + let ctx = CommandContext::new(&base_url, false, false).with_session(master_session); + let result = cmd_scope(&ctx, &child_wallet, &["x".to_string()], &[], None, true).await; + let err = result.expect_err("--list + --add must be rejected").to_string(); + assert!( + err.contains("mutually exclusive"), + "error should flag the --list/--add combo: {err}" + ); +} + +// Test 21: --add X + --remove X overlap rejected with a clear error +// (claude re-review follow-up on the P2 overlap guard added in v3). +#[tokio::test(flavor = "multi_thread")] +async fn cmd_scope_add_remove_overlap_errors() { + let (base_url, master_token, child_wallet) = start_scope_test_server().await; + + let master_session = agentkeys_types::Session { + token: master_token, + wallet: agentkeys_types::WalletAddress("unused".to_string()), + scope: None, + created_at: 0, + ttl_seconds: 86400, + }; + + let ctx = CommandContext::new(&base_url, false, false).with_session(master_session); + let result = cmd_scope( + &ctx, + &child_wallet, + &["foo".to_string()], + &["foo".to_string()], + None, + false, + ) + .await; + let err = result.expect_err("--add X + --remove X must be rejected").to_string(); + assert!( + err.contains("both --add and --remove") && err.contains("foo"), + "error should name the overlapping service: {err}" + ); +} diff --git a/crates/agentkeys-core/src/backend.rs b/crates/agentkeys-core/src/backend.rs index dc839e1..d2813f2 100644 --- a/crates/agentkeys-core/src/backend.rs +++ b/crates/agentkeys-core/src/backend.rs @@ -141,6 +141,19 @@ pub trait CredentialBackend: Send + Sync { session: &Session, identifier: &str, ) -> Result; + + async fn get_scope( + &self, + session: &Session, + target_wallet: &WalletAddress, + ) -> Result, BackendError>; + + async fn update_scope( + &self, + session: &Session, + target_wallet: &WalletAddress, + new_scope: &Scope, + ) -> Result<(), BackendError>; } #[cfg(test)] @@ -301,6 +314,23 @@ mod tests { ) -> Result { unimplemented!() } + + async fn get_scope( + &self, + _session: &Session, + _target_wallet: &WalletAddress, + ) -> Result, BackendError> { + unimplemented!() + } + + async fn update_scope( + &self, + _session: &Session, + _target_wallet: &WalletAddress, + _new_scope: &Scope, + ) -> Result<(), BackendError> { + unimplemented!() + } } #[test] diff --git a/crates/agentkeys-core/src/mock_client.rs b/crates/agentkeys-core/src/mock_client.rs index c66a3ab..767a01e 100644 --- a/crates/agentkeys-core/src/mock_client.rs +++ b/crates/agentkeys-core/src/mock_client.rs @@ -627,20 +627,20 @@ impl CredentialBackend for MockHttpClient { session: &Session, agent_id: &WalletAddress, ) -> Result, BackendError> { - let url = format!("/credential/list?agent_id={}", agent_id.0); - + // Use reqwest's .query() builder for RFC 3986 percent-encoding so + // wallet strings with reserved chars (`&`, `#`, `%`, `+`, spaces) + // don't smuggle extra params or break the request. let resp = self .client - .get(self.url(&url)) + .get(self.url("/credential/list")) + .query(&[("agent_id", &agent_id.0)]) .header("authorization", format!("Bearer {}", session.token)) .send() .await .map_err(|e| BackendError::Transport(e.to_string()))?; - if !resp.status().is_success() { return Err(Self::map_error(resp).await); } - let body: Value = resp.json().await.map_err(|e| BackendError::Transport(e.to_string()))?; let services = body["services"] .as_array() @@ -674,11 +674,9 @@ impl CredentialBackend for MockHttpClient { .send() .await .map_err(|e| BackendError::Transport(e.to_string()))?; - if !resp.status().is_success() { return Err(Self::map_error(resp).await); } - let body: Value = resp.json().await.map_err(|e| BackendError::Transport(e.to_string()))?; let wallet_str = body["wallet_address"] .as_str() @@ -687,6 +685,65 @@ impl CredentialBackend for MockHttpClient { Ok(WalletAddress(wallet_str)) } + async fn get_scope( + &self, + session: &Session, + target_wallet: &WalletAddress, + ) -> Result, BackendError> { + // .query() builder percent-encodes per RFC 3986 so wallet strings + // with reserved chars don't break the request or smuggle params. + let resp = self + .client + .get(self.url("/session/scope")) + .query(&[("wallet", &target_wallet.0)]) + .header("authorization", format!("Bearer {}", session.token)) + .send() + .await + .map_err(|e| BackendError::Transport(e.to_string()))?; + if resp.status().as_u16() == 404 { + return Ok(None); + } + if !resp.status().is_success() { + return Err(Self::map_error(resp).await); + } + let body: Value = resp.json().await.map_err(|e| BackendError::Transport(e.to_string()))?; + if body["services"].is_null() { + return Ok(None); + } + let services: Vec = body["services"] + .as_array() + .unwrap_or(&vec![]) + .iter() + .filter_map(|v| v.as_str()) + .map(|s| ServiceName(s.to_string())) + .collect(); + let read_only = body["read_only"].as_bool().unwrap_or(false); + Ok(Some(Scope { services, read_only })) + } + + async fn update_scope( + &self, + session: &Session, + target_wallet: &WalletAddress, + new_scope: &Scope, + ) -> Result<(), BackendError> { + let resp = self + .client + .put(self.url("/session/scope")) + .header("authorization", format!("Bearer {}", session.token)) + .json(&serde_json::json!({ + "target_wallet": target_wallet.0, + "scope": new_scope, + })) + .send() + .await + .map_err(|e| BackendError::Transport(e.to_string()))?; + if !resp.status().is_success() { + return Err(Self::map_error(resp).await); + } + Ok(()) + } + async fn recover_session( &self, identity: &agentkeys_types::AgentIdentity, diff --git a/crates/agentkeys-mock-server/src/handlers/auth_request.rs b/crates/agentkeys-mock-server/src/handlers/auth_request.rs index 924d123..3f7766c 100644 --- a/crates/agentkeys-mock-server/src/handlers/auth_request.rs +++ b/crates/agentkeys-mock-server/src/handlers/auth_request.rs @@ -303,6 +303,14 @@ pub async fn fetch_auth_request( }))) } +// Note: the direct scope-update path lives on the backend trait as +// `update_scope` (see agentkeys-core::backend::CredentialBackend). PR #29's +// `cmd_scope` CLI command calls that directly via PUT /session/scope; the +// AuthRequestType::ScopeChange approve-flow upstream of this handler is +// not currently exercised end-to-end. The stub above returns an empty +// MintOutput so the dispatch compiles until a full approve-flow wiring +// lands. + pub async fn approve_auth_request( State(state): State, headers: HeaderMap, diff --git a/crates/agentkeys-mock-server/src/handlers/session.rs b/crates/agentkeys-mock-server/src/handlers/session.rs index 8c8440c..d541ae6 100644 --- a/crates/agentkeys-mock-server/src/handlers/session.rs +++ b/crates/agentkeys-mock-server/src/handlers/session.rs @@ -1,5 +1,5 @@ use axum::{ - extract::State, + extract::{Query, State}, http::HeaderMap, Json, }; @@ -301,3 +301,135 @@ pub async fn revoke_session( Ok(Json(json!({ "ok": true, "sessions_revoked": rows_affected }))) } } + +pub async fn update_scope( + State(state): State, + headers: HeaderMap, + Json(body): Json, +) -> AppResult> { + let token = headers + .get("authorization") + .and_then(|v| v.to_str().ok()) + .and_then(extract_bearer_token) + .ok_or_else(|| AppError::unauthorized("missing Authorization header"))?; + + let session = validate_session(&state, token)?; + + let target_wallet = body + .get("target_wallet") + .and_then(|v| v.as_str()) + .ok_or_else(|| AppError::bad_request("target_wallet required"))? + .to_string(); + + // `agentkeys scope` is for child agents. Allowing a master session to + // target its own wallet would let the master accidentally restrict itself + // (e.g. `agentkeys scope --agent --set openrouter` would flip + // the master's scope_json from NULL to ["openrouter"] and cause every + // subsequent `credential/read` outside that list to fail). Reject + // self-targeting explicitly before the ownership check. Case-insensitive + // so EIP-55 checksummed input matches the backend's lowercase storage. + if session.wallet_address.eq_ignore_ascii_case(&target_wallet) { + return Err(AppError::bad_request( + "agentkeys scope cannot target the master's own wallet — use it on child agent wallets only", + )); + } + + let db = state.db.lock().unwrap(); + + if !is_owner_of(&db, &session.wallet_address, &target_wallet) { + // Mirror the read_credential / list_credentials audit contract — + // cross-agent probing of scope endpoints must leave a DENIED row. + let now = now_secs(); + db.execute( + "INSERT INTO audit_log (owner_wallet, agent_wallet, service_name, action, result, timestamp) + VALUES (?1, ?2, ?3, 'scope_update', 'DENIED', ?4)", + rusqlite::params![session.wallet_address, target_wallet, "*", now], + ) + .ok(); + return Err(AppError::forbidden("session does not own the target wallet")); + } + + let new_scope: agentkeys_types::Scope = serde_json::from_value( + body.get("scope").cloned().ok_or_else(|| AppError::bad_request("scope required"))?, + ) + .map_err(|e| AppError::bad_request(e.to_string()))?; + + let scope_json = + serde_json::to_string(&new_scope).map_err(|e| AppError::internal(e.to_string()))?; + + // Mutate only the most recent active session for the target wallet. + // read-side `get_session_scope` uses `ORDER BY created_at DESC LIMIT 1`, + // so blanket updates across all active sessions would drift the + // read/write contract on wallets that happen to have multiple active + // sessions (e.g. one paired + one recovered). + let rows_affected = db + .execute( + "UPDATE sessions SET scope_json = ?1 \ + WHERE token = ( \ + SELECT token FROM sessions \ + WHERE wallet_address = ?2 AND revoked = 0 \ + ORDER BY created_at DESC LIMIT 1 \ + )", + rusqlite::params![scope_json, target_wallet], + ) + .map_err(|e| AppError::internal(e.to_string()))?; + + if rows_affected == 0 { + return Err(AppError::not_found("no active sessions for target wallet")); + } + + Ok(Json(serde_json::json!({ "ok": true, "updated": rows_affected }))) +} + +#[derive(serde::Deserialize)] +pub struct GetSessionScopeQuery { + pub wallet: String, +} + +pub async fn get_session_scope( + State(state): State, + headers: HeaderMap, + Query(query): Query, +) -> AppResult> { + let token = headers + .get("authorization") + .and_then(|v| v.to_str().ok()) + .and_then(extract_bearer_token) + .ok_or_else(|| AppError::unauthorized("missing Authorization header"))?; + + let session = validate_session(&state, token)?; + + // Only the master that owns the target wallet may query its scope. + let db = state.db.lock().unwrap(); + if !is_owner_of(&db, &session.wallet_address, &query.wallet) { + // Audit cross-agent scope probing to match the DENIED contract on + // other credential-path endpoints (codex PR #29 P1). + let now = now_secs(); + db.execute( + "INSERT INTO audit_log (owner_wallet, agent_wallet, service_name, action, result, timestamp) + VALUES (?1, ?2, ?3, 'scope_read', 'DENIED', ?4)", + rusqlite::params![session.wallet_address, query.wallet, "*", now], + ) + .ok(); + return Err(AppError::forbidden("session does not own the target wallet")); + } + + let scope_json: Option = db + .query_row( + "SELECT scope_json FROM sessions WHERE wallet_address = ?1 AND revoked = 0 ORDER BY created_at DESC LIMIT 1", + rusqlite::params![query.wallet], + |row| row.get(0), + ) + .ok() + .flatten(); + + let scope: agentkeys_types::Scope = match scope_json { + Some(ref s) => serde_json::from_str(s).unwrap_or(agentkeys_types::Scope { services: vec![], read_only: false }), + None => agentkeys_types::Scope { services: vec![], read_only: false }, + }; + + Ok(Json(serde_json::json!({ + "services": scope.services.iter().map(|s| &s.0).collect::>(), + "read_only": scope.read_only, + }))) +} diff --git a/crates/agentkeys-mock-server/src/lib.rs b/crates/agentkeys-mock-server/src/lib.rs index c145b69..6d0a2e2 100644 --- a/crates/agentkeys-mock-server/src/lib.rs +++ b/crates/agentkeys-mock-server/src/lib.rs @@ -7,7 +7,7 @@ pub mod test_client; use axum::{ Router, - routing::{delete, get, post}, + routing::{delete, get, post, put}, }; use std::sync::Arc; @@ -38,6 +38,9 @@ pub fn create_router(state: SharedState) -> Router { .route("/auth-request/fetch", get(handlers::auth_request::fetch_auth_request)) .route("/auth-request/approve", post(handlers::auth_request::approve_auth_request)) .route("/auth-request/await", get(handlers::auth_request::await_auth_decision)) + // Session scope + .route("/session/scope", get(handlers::session::get_session_scope)) + .route("/session/scope", put(handlers::session::update_scope)) // Identity .route("/identity/link", post(handlers::identity::link_identity)) .route("/identity/resolve", get(handlers::identity::resolve_identity)) diff --git a/crates/agentkeys-mock-server/src/test_client.rs b/crates/agentkeys-mock-server/src/test_client.rs index 975270d..9ea2d93 100644 --- a/crates/agentkeys-mock-server/src/test_client.rs +++ b/crates/agentkeys-mock-server/src/test_client.rs @@ -717,6 +717,58 @@ impl CredentialBackend for InProcessBackend { Ok(WalletAddress(wallet_str)) } + async fn get_scope( + &self, + session: &Session, + target_wallet: &WalletAddress, + ) -> Result, BackendError> { + // Percent-encode the wallet — matches the `.query()` pattern in + // `MockHttpClient::get_scope` and the `pct_encode` usage in + // `resolve_identity` above. Wallet strings are hex today so this is + // safe in practice, but the consistency matters for the + // `.github/REVIEW_GUIDELINES.md` URL-encoding invariant (pattern #3). + let path = format!("/session/scope?wallet={}", pct_encode(&target_wallet.0)); + let result = self.get_with_session(&path, session).await; + match result { + Err(BackendError::NotFound(_)) => Ok(None), + Err(e) => Err(e), + Ok(body) => { + if body["services"].is_null() { + return Ok(None); + } + let services: Vec = body["services"] + .as_array() + .unwrap_or(&vec![]) + .iter() + .filter_map(|v| v.as_str()) + .map(|s| ServiceName(s.to_string())) + .collect(); + let read_only = body["read_only"].as_bool().unwrap_or(false); + Ok(Some(Scope { services, read_only })) + } + } + } + + async fn update_scope( + &self, + session: &Session, + target_wallet: &WalletAddress, + new_scope: &Scope, + ) -> Result<(), BackendError> { + let auth = format!("Bearer {}", session.token); + self.do_request( + "PUT", + "/session/scope", + Some(json!({ + "target_wallet": target_wallet.0, + "scope": new_scope, + })), + vec![("authorization", auth)], + ) + .await?; + Ok(()) + } + async fn recover_session( &self, identity: &agentkeys_types::AgentIdentity, diff --git a/crates/agentkeys-mock-server/tests/integration.rs b/crates/agentkeys-mock-server/tests/integration.rs index dcea722..c1479c2 100644 --- a/crates/agentkeys-mock-server/tests/integration.rs +++ b/crates/agentkeys-mock-server/tests/integration.rs @@ -77,6 +77,20 @@ async fn delete_json_auth(app: Router, path: &str, token: &str, body: Value) -> (status, json) } +async fn put_json_auth(app: Router, path: &str, token: &str, body: Value) -> (StatusCode, Value) { + let req = Request::builder() + .method(Method::PUT) + .uri(path) + .header("content-type", "application/json") + .header("authorization", format!("Bearer {token}")) + .body(Body::from(serde_json::to_string(&body).unwrap())) + .unwrap(); + let resp = app.oneshot(req).await.unwrap(); + let status = resp.status(); + let json = body_json(resp.into_body()).await; + (status, json) +} + async fn create_test_session(app: Router) -> (String, String, Router) { let (status, json) = post_json( app.clone(), diff --git a/docs/manual-test-issue-15b.md b/docs/manual-test-issue-15b.md new file mode 100644 index 0000000..17c4e94 --- /dev/null +++ b/docs/manual-test-issue-15b.md @@ -0,0 +1,133 @@ +# Manual Test: Issue #15b — `agentkeys scope` CLI command (part 3 of #15) + +**Issue:** [litentry/agentKeys#15](https://github.com/litentry/agentKeys/issues/15) (part 3 of 3) + +**Branch:** `fix/issue-15b` + +## What this ships + +Adds `agentkeys scope` for editing a child agent's scope. Follow-up to PR #19 (parts 1+2 — `run` for master sessions + `--env`). + +``` +agentkeys scope --add [--add ...] +agentkeys scope --remove [--remove ...] +agentkeys scope --set +agentkeys scope --list +``` + +AGENT = `0x...` wallet, alias, or email (same resolver as `agentkeys store`/`read`/`run`). + +Under the hood: +1. Read current scope for the target agent. +2. Compute new scope (add/remove/set). +3. Open an `AuthRequest::ScopeChange` from the master session. +4. Auto-approve with the master session. +5. Print the resulting scope. + +## 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 +CLI=$(pwd)/target/release/agentkeys +``` + +## Case 1 — `--add` appends services + +```bash +cargo run --release -p agentkeys-mock-server & +MOCK_PID=$!; sleep 1 + +$CLI --backend $BACKEND init --mock-token scope-add +MASTER=$(jq -r .wallet "$HOME/.agentkeys/master/session.json") +# Pair a child (via daemon flow or test harness); record CHILD wallet. +CHILD= + +# Start with empty scope, add one service: +$CLI --backend $BACKEND scope --agent $CHILD --add openrouter +# Expected: "Scope updated for agent 0x... New services: [openrouter]" + +$CLI --backend $BACKEND scope --agent $CHILD --list +# Expected output includes: services=[openrouter] + +# Add a second: +$CLI --backend $BACKEND scope --agent $CHILD --add anthropic +$CLI --backend $BACKEND scope --agent $CHILD --list +# Expected: services=[anthropic, openrouter] (sorted) + +kill $MOCK_PID +``` + +## Case 2 — `--remove` drops services + +```bash +rm -rf $HOME_SANDBOX/.agentkeys +cargo run --release -p agentkeys-mock-server & +MOCK_PID=$!; sleep 1 + +$CLI --backend $BACKEND init --mock-token scope-remove +# (pair a child with scope=[a, b, c]) +CHILD=... + +$CLI --backend $BACKEND scope --agent $CHILD --remove a +$CLI --backend $BACKEND scope --agent $CHILD --list +# Expected: services=[b, c] + +$CLI --backend $BACKEND scope --agent $CHILD --remove b --remove c +$CLI --backend $BACKEND scope --agent $CHILD --list +# Expected: services=[] + +kill $MOCK_PID +``` + +## Case 3 — `--set` replaces + +```bash +rm -rf $HOME_SANDBOX/.agentkeys +cargo run --release -p agentkeys-mock-server & +MOCK_PID=$!; sleep 1 + +$CLI --backend $BACKEND init --mock-token scope-set +# (pair a child with scope=[openrouter]) +CHILD=... + +$CLI --backend $BACKEND scope --agent $CHILD --set anthropic,github +$CLI --backend $BACKEND scope --agent $CHILD --list +# Expected: services=[anthropic, github] + +kill $MOCK_PID +``` + +## Case 4 — conflict between `--add` / `--set` + +```bash +$CLI --backend $BACKEND scope --agent $CHILD --add x --set y 2>&1 | head -3 +# Expected: error — "--set is mutually exclusive with --add/--remove" +# Child wallet state unchanged. +``` + +## Case 5 — ownership enforcement + +```bash +# User A owns child C. User B tries to scope C → 403. +# ... +``` + +## Cleanup + +```bash +rm -rf "$HOME_SANDBOX" +unset HOME_SANDBOX AGENTKEYS_SESSION_STORE +``` + +## Cross-references + +- `crates/agentkeys-cli/src/lib.rs` — `cmd_scope` +- `crates/agentkeys-cli/src/main.rs` — `Commands::Scope` +- `crates/agentkeys-mock-server/src/handlers/auth_request.rs` — `mint_scope_change_session` (fleshed out by this PR) +- Related: PR #19 (parts 1+2), PR #20 (`resolve_agent` helper). +- `AuthRequestType::ScopeChange` already exists in `agentkeys-types`.