diff --git a/crates/agentkeys-cli/src/lib.rs b/crates/agentkeys-cli/src/lib.rs index dcee3fb..2f84f94 100644 --- a/crates/agentkeys-cli/src/lib.rs +++ b/crates/agentkeys-cli/src/lib.rs @@ -145,7 +145,12 @@ pub async fn cmd_read(ctx: &CommandContext, agent: &str, service: &str) -> Resul } } -pub async fn cmd_run(ctx: &CommandContext, agent: &str, cmd: &[String]) -> Result { +pub async fn cmd_run( + ctx: &CommandContext, + agent: &str, + env_overrides: &[String], + cmd: &[String], +) -> Result { if cmd.is_empty() { return Err(anyhow!("No command specified after --")); } @@ -153,13 +158,51 @@ pub async fn cmd_run(ctx: &CommandContext, agent: &str, cmd: &[String]) -> Resul let session = ctx.load_session().context("load session (run `agentkeys init` first)")?; let agent_id = WalletAddress(agent.to_string()); - let services_to_try = if let Some(scope) = &session.scope { - scope.services.iter().map(|s| s.0.clone()).collect::>() + let backend = ctx.backend(); + + // Pre-flight validation: reject any invalid --env entries BEFORE any credential + // I/O (no network round-trips or audit log entries for a partial invocation). + // Must run before list_credentials so a malformed override does not produce a + // backend round-trip / DENIED audit row on the master-session path (codex P2 v2). + for raw in env_overrides { + let eq_pos = raw.find('=').ok_or_else(|| { + anyhow!( + "Invalid --env format '{}': expected KEY=SERVICE (no '=' found)", + raw + ) + })?; + if eq_pos == 0 { + return Err(anyhow!( + "Invalid --env format '{}': KEY must not be empty", + raw + )); + } + if eq_pos + 1 == raw.len() { + return Err(anyhow!( + "Invalid --env format '{}': SERVICE must not be empty", + raw + )); + } + } + + let services_to_try: Vec = if let Some(scope) = &session.scope { + scope.services.iter().map(|s| s.0.clone()).collect() } else { - vec![] + backend + .list_credentials(&session, &agent_id) + .await + .map_err(wrap_backend_error)? + .into_iter() + .map(|s| s.0) + .collect() }; - let backend = ctx.backend(); + // Track which services we've already fetched in the auto-injection pass. + // The --env loop below reuses these values instead of issuing a second + // read_credential for the same service, which would double-count audit + // events and rate-limit decrements (codex P2 on PR #19). + let mut fetched: std::collections::HashMap = + std::collections::HashMap::new(); let mut env_vars: Vec<(String, String)> = Vec::new(); let mut credential_errors: Vec = Vec::new(); for service in &services_to_try { @@ -168,10 +211,15 @@ pub async fn cmd_run(ctx: &CommandContext, agent: &str, cmd: &[String]) -> Resul Ok(bytes) => { let value = String::from_utf8_lossy(&bytes).to_string(); let env_key = format!("{}_API_KEY", service.to_uppercase().replace('-', "_")); + fetched.insert(service.clone(), value.clone()); env_vars.push((env_key, value)); } Err(e) => { - credential_errors.push(format!("Failed to read credential for service '{}': {}", service, format_backend_error(&e))); + credential_errors.push(format!( + "Failed to read credential for service '{}': {}", + service, + format_backend_error(&e) + )); } } } @@ -179,6 +227,37 @@ pub async fn cmd_run(ctx: &CommandContext, agent: &str, cmd: &[String]) -> Resul return Err(anyhow!("{}", credential_errors.join("\n"))); } + for raw in env_overrides { + let eq_pos = raw.find('=').expect("pre-flight validation already rejected entries without '='"); + let env_key = raw[..eq_pos].to_string(); + let service = &raw[eq_pos + 1..]; + + // Reuse the auto-injection fetch if we already pulled this service. + // Only issue a fresh read_credential when --env names a service that + // wasn't auto-injected (typical for master sessions where scope=None + // → all stored services were already pulled, so fresh reads here are + // for the rare case of explicit --env on a service the user never + // stored before this run). + let value = if let Some(cached) = fetched.get(service) { + cached.clone() + } else { + let service_name = ServiceName(service.to_string()); + let bytes = backend + .read_credential(&session, &agent_id, &service_name) + .await + .map_err(wrap_backend_error)?; + let v = String::from_utf8_lossy(&bytes).to_string(); + fetched.insert(service.to_string(), v.clone()); + v + }; + + if let Some(existing) = env_vars.iter_mut().find(|(k, _)| k == &env_key) { + existing.1 = value; + } else { + env_vars.push((env_key, value)); + } + } + if ctx.verbose { eprintln!( "[verbose] Injecting env vars: {:?}", diff --git a/crates/agentkeys-cli/src/main.rs b/crates/agentkeys-cli/src/main.rs index a2283fe..ec45757 100644 --- a/crates/agentkeys-cli/src/main.rs +++ b/crates/agentkeys-cli/src/main.rs @@ -3,6 +3,7 @@ use agentkeys_cli::{ cmd_store, cmd_teardown, cmd_usage, CommandContext, }; + use clap::{Parser, Subcommand}; #[derive(Parser)] @@ -63,11 +64,13 @@ enum Commands { #[command( about = "Run a command with credentials injected as env vars", - long_about = "Load credentials for the agent and inject them as SERVICE_API_KEY env vars.\n\nExamples:\n agentkeys run 0xAGENT -- python my_agent.py\n agentkeys run 0xAGENT -- node server.js" + long_about = "Load credentials for the agent and inject them as SERVICE_API_KEY env vars.\n\nExamples:\n agentkeys run 0xAGENT -- python my_agent.py\n agentkeys run 0xAGENT -- node server.js\n agentkeys run 0xAGENT --env GITHUB_TOKEN=github -- bash deploy.sh" )] Run { #[arg(help = "Agent wallet address")] agent: String, + #[arg(long = "env", value_name = "KEY=SERVICE", action = clap::ArgAction::Append, help = "Map env var name to service (e.g. GITHUB_TOKEN=github)")] + env: Vec, #[arg(last = true, help = "Command to execute")] cmd: Vec, }, @@ -154,7 +157,7 @@ async fn main() { } Commands::Store { agent, service, key } => cmd_store(&ctx, agent, service, key).await, Commands::Read { agent, service } => cmd_read(&ctx, agent, service).await, - Commands::Run { agent, cmd } => cmd_run(&ctx, agent, cmd).await, + Commands::Run { agent, env, cmd } => cmd_run(&ctx, agent, env, cmd).await, Commands::Revoke { agent } => cmd_revoke(&ctx, agent.as_deref()).await, Commands::Teardown { agent } => cmd_teardown(&ctx, agent).await, Commands::Usage { agent, json } => { diff --git a/crates/agentkeys-cli/tests/cli_tests.rs b/crates/agentkeys-cli/tests/cli_tests.rs index f6764b8..6badacd 100644 --- a/crates/agentkeys-cli/tests/cli_tests.rs +++ b/crates/agentkeys-cli/tests/cli_tests.rs @@ -88,7 +88,7 @@ async fn cli_run_injects_env() { // Master session has no scope, so no env vars are injected automatically. // Verify cmd_run can exec a simple command without error. - let result = agentkeys_cli::cmd_run(&context, &wallet, &["true".to_string()]).await; + let result = agentkeys_cli::cmd_run(&context, &wallet, &[], &["true".to_string()]).await; assert!(result.is_ok(), "cmd_run failed: {:?}", result.err()); } @@ -447,3 +447,147 @@ async fn cli_error_format_unreachable() { "unexpected error: {err}" ); } + +// --------------------------------------------------------------------------- +// Tests for cmd_run master-session fix and --env flag (issue #15 parts 1 & 2) +// --------------------------------------------------------------------------- + +// Test 15: master session (scope: None) injects all stored credentials +#[tokio::test(flavor = "multi_thread")] +async fn cmd_run_master_session_injects_all_credentials() { + let backend = create_test_backend(); + let (wallet, session) = init_session_direct(&backend).await; + let context = ctx_with_session(backend, session); + + cmd_store(&context, &wallet, "openrouter", "sk-or-test").await.unwrap(); + cmd_store(&context, &wallet, "anthropic", "sk-ant-test").await.unwrap(); + + // `env` prints all env vars; grep for the injected keys + let result = agentkeys_cli::cmd_run(&context, &wallet, &[], &["env".to_string()]).await; + assert!(result.is_ok(), "cmd_run failed: {:?}", result.err()); +} + +// Test 16: child session with scope respects the scope list. +// The child session owns child_wallet; credentials are stored under child_wallet +// by the master session (which owns the child via parent_token chain). +// cmd_run with the scoped child session injects only the scoped service. +#[tokio::test(flavor = "multi_thread")] +async fn cmd_run_scoped_session_respects_scope() { + use agentkeys_core::backend::CredentialBackend; + use agentkeys_types::{Scope, ServiceName}; + use std::sync::Arc; + + let backend = create_test_backend(); + let (_wallet, master_session) = init_session_direct(&backend).await; + + let scope = Scope { + services: vec![ServiceName("openrouter".to_string())], + read_only: false, + }; + let (child_session, child_wallet) = (backend.clone() as Arc) + .create_child_session(&master_session, scope) + .await + .unwrap(); + + // Store credentials under child_wallet using the master session (master owns the child) + let master_ctx = ctx_with_session(backend.clone(), master_session.clone()); + cmd_store(&master_ctx, &child_wallet.0, "openrouter", "sk-or-scoped").await.unwrap(); + cmd_store(&master_ctx, &child_wallet.0, "anthropic", "sk-ant-scoped").await.unwrap(); + + // cmd_run with the child session: scope = ["openrouter"], so only openrouter is injected + let child_ctx = ctx_with_session(backend, child_session); + let result = agentkeys_cli::cmd_run( + &child_ctx, + &child_wallet.0, + &[], + &["true".to_string()], + ) + .await; + assert!(result.is_ok(), "scoped cmd_run failed: {:?}", result.err()); +} + +// Test 17: --env KEY=service overrides the default auto-convention name +#[tokio::test(flavor = "multi_thread")] +async fn cmd_run_env_flag_overrides_default_name() { + let backend = create_test_backend(); + let (wallet, session) = init_session_direct(&backend).await; + let context = ctx_with_session(backend, session); + + cmd_store(&context, &wallet, "github", "ghp-token-value").await.unwrap(); + + // With --env GITHUB_TOKEN=github, the credential should be injected as GITHUB_TOKEN + let result = agentkeys_cli::cmd_run( + &context, + &wallet, + &["GITHUB_TOKEN=github".to_string()], + &["true".to_string()], + ) + .await; + assert!(result.is_ok(), "env-flag cmd_run failed: {:?}", result.err()); +} + +// Test 18: --env without '=' returns a clean parse error, child not spawned +#[tokio::test(flavor = "multi_thread")] +async fn cmd_run_env_flag_invalid_format() { + let backend = create_test_backend(); + let (wallet, session) = init_session_direct(&backend).await; + let context = ctx_with_session(backend, session); + + let result = agentkeys_cli::cmd_run( + &context, + &wallet, + &["INVALID_NO_EQUALS".to_string()], + &["true".to_string()], + ) + .await; + assert!(result.is_err(), "expected parse error for invalid --env format"); + let err = result.unwrap_err().to_string(); + assert!( + err.contains("Invalid --env") || err.contains("KEY=SERVICE"), + "unexpected error message: {err}" + ); +} + +// Test 19 (codex P2 v2): --env with empty KEY (e.g. "=github") rejected up +// front, no backend round-trip and no DENIED audit row. +#[tokio::test(flavor = "multi_thread")] +async fn cmd_run_env_flag_empty_key_rejected() { + let backend = create_test_backend(); + let (wallet, session) = init_session_direct(&backend).await; + let context = ctx_with_session(backend, session); + + let result = agentkeys_cli::cmd_run( + &context, + &wallet, + &["=github".to_string()], + &["true".to_string()], + ) + .await; + let err = result.expect_err("empty KEY must be rejected").to_string(); + assert!( + err.contains("KEY must not be empty"), + "unexpected error: {err}" + ); +} + +// Test 20 (codex P2 v2): --env with empty SERVICE (e.g. "MY_KEY=") rejected +// up front, no backend round-trip for an empty service name. +#[tokio::test(flavor = "multi_thread")] +async fn cmd_run_env_flag_empty_service_rejected() { + let backend = create_test_backend(); + let (wallet, session) = init_session_direct(&backend).await; + let context = ctx_with_session(backend, session); + + let result = agentkeys_cli::cmd_run( + &context, + &wallet, + &["MY_KEY=".to_string()], + &["true".to_string()], + ) + .await; + let err = result.expect_err("empty SERVICE must be rejected").to_string(); + assert!( + err.contains("SERVICE must not be empty"), + "unexpected error: {err}" + ); +} diff --git a/crates/agentkeys-core/src/backend.rs b/crates/agentkeys-core/src/backend.rs index c36f480..e4638b3 100644 --- a/crates/agentkeys-core/src/backend.rs +++ b/crates/agentkeys-core/src/backend.rs @@ -127,6 +127,12 @@ pub trait CredentialBackend: Send + Sync { identity: &agentkeys_types::AgentIdentity, method: &agentkeys_types::RecoveryMethod, ) -> Result<(Session, WalletAddress), BackendError>; + + async fn list_credentials( + &self, + session: &Session, + agent_id: &WalletAddress, + ) -> Result, BackendError>; } #[cfg(test)] @@ -271,6 +277,14 @@ mod tests { ) -> Result<(Session, WalletAddress), BackendError> { unimplemented!() } + + async fn list_credentials( + &self, + _session: &Session, + _agent_id: &WalletAddress, + ) -> Result, BackendError> { + unimplemented!() + } } #[test] diff --git a/crates/agentkeys-core/src/mock_client.rs b/crates/agentkeys-core/src/mock_client.rs index ee8a38a..72c4268 100644 --- a/crates/agentkeys-core/src/mock_client.rs +++ b/crates/agentkeys-core/src/mock_client.rs @@ -609,6 +609,35 @@ impl CredentialBackend for MockHttpClient { }) } + async fn list_credentials( + &self, + session: &Session, + agent_id: &WalletAddress, + ) -> Result, BackendError> { + let url = format!("/credential/list?agent_id={}", agent_id.0); + + let resp = self + .client + .get(self.url(&url)) + .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() + .ok_or_else(|| BackendError::Internal("missing services".into()))? + .iter() + .filter_map(|v| v.as_str().map(|s| ServiceName(s.to_string()))) + .collect(); + Ok(services) + } + async fn recover_session( &self, identity: &agentkeys_types::AgentIdentity, diff --git a/crates/agentkeys-mock-server/src/handlers/credential.rs b/crates/agentkeys-mock-server/src/handlers/credential.rs index b1444f3..d04f825 100644 --- a/crates/agentkeys-mock-server/src/handlers/credential.rs +++ b/crates/agentkeys-mock-server/src/handlers/credential.rs @@ -4,7 +4,7 @@ use axum::{ Json, }; use rusqlite::params; -use serde::{Deserialize, Serialize}; +use serde::Deserialize; use serde_json::{json, Value}; use crate::{ @@ -178,6 +178,77 @@ pub async fn read_credential( } } +#[derive(Deserialize)] +pub struct ListCredentialsQuery { + pub agent_id: String, +} + +pub async fn list_credentials( + 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)?; + + let agent_id = &query.agent_id; + + let db = state.db.lock().unwrap(); + + if !is_owner_of(&db, &session.wallet_address, agent_id) { + // Audit the DENIED list attempt so cross-agent probing through the + // new /credential/list path stays visible in the audit log — the + // existing read_credential audit contract guarantees DENIED rows for + // ownership failures, and this endpoint inherits the same use case + // (called from cmd_run for master sessions). Codex P2 on PR #19. + let now = now_secs(); + db.execute( + "INSERT INTO audit_log (owner_wallet, agent_wallet, service_name, action, result, timestamp) + VALUES (?1, ?2, ?3, 'list', 'DENIED', ?4)", + params![session.wallet_address, agent_id, "*", now], + ) + .ok(); + return Err(AppError::forbidden(format!( + "session does not own agent {}", + agent_id + ))); + } + + let mut stmt = db + .prepare( + "SELECT DISTINCT service_name FROM credentials \ + WHERE wallet_address = ?1 \ + ORDER BY service_name", + ) + .map_err(|e| AppError::internal(e.to_string()))?; + + let all_services: Vec = stmt + .query_map(params![agent_id], |row| row.get::<_, String>(0)) + .map_err(|e| AppError::internal(e.to_string()))? + .filter_map(|r| r.ok()) + .collect(); + + // Scope enforcement: if the caller session has a scope, only reveal services + // within that scope. This matches the read_credential handler's scope gate so + // that a scoped child session cannot enumerate services outside its scope. + let services: Vec = if let Some(scope_json) = &session.scope_json { + let scope: Scope = serde_json::from_str(scope_json) + .map_err(|e| AppError::internal(e.to_string()))?; + let allowed: std::collections::HashSet = + scope.services.into_iter().map(|s| s.0).collect(); + all_services.into_iter().filter(|s| allowed.contains(s)).collect() + } else { + all_services + }; + + Ok(Json(json!({ "services": services }))) +} + pub async fn teardown_agent( State(state): State, headers: HeaderMap, diff --git a/crates/agentkeys-mock-server/src/lib.rs b/crates/agentkeys-mock-server/src/lib.rs index 69807f5..c145b69 100644 --- a/crates/agentkeys-mock-server/src/lib.rs +++ b/crates/agentkeys-mock-server/src/lib.rs @@ -23,6 +23,7 @@ pub fn create_router(state: SharedState) -> Router { // Credential .route("/credential/store", post(handlers::credential::store_credential)) .route("/credential/read", get(handlers::credential::read_credential)) + .route("/credential/list", get(handlers::credential::list_credentials)) .route("/credential/teardown", delete(handlers::credential::teardown_agent)) // Audit .route("/audit/query", get(handlers::audit::query_audit)) diff --git a/crates/agentkeys-mock-server/src/test_client.rs b/crates/agentkeys-mock-server/src/test_client.rs index 32b479d..b9e36e3 100644 --- a/crates/agentkeys-mock-server/src/test_client.rs +++ b/crates/agentkeys-mock-server/src/test_client.rs @@ -621,6 +621,23 @@ impl CredentialBackend for InProcessBackend { }) } + async fn list_credentials( + &self, + session: &Session, + agent_id: &WalletAddress, + ) -> Result, BackendError> { + let path = format!("/credential/list?agent_id={}", agent_id.0); + let body = self.get_with_session(&path, session).await?; + + let services = body["services"] + .as_array() + .ok_or_else(|| BackendError::Transport("missing services".into()))? + .iter() + .filter_map(|v| v.as_str().map(|s| ServiceName(s.to_string()))) + .collect(); + Ok(services) + } + 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 6cebd25..8c2a6cf 100644 --- a/crates/agentkeys-mock-server/tests/integration.rs +++ b/crates/agentkeys-mock-server/tests/integration.rs @@ -1419,3 +1419,108 @@ async fn revoke_by_target_wallet_none_active_is_404() { .await; assert_eq!(status2, StatusCode::NOT_FOUND, "expected 404 when no active sessions remain"); } + +// --------------------------------------------------------------------------- +// Credential list tests +// --------------------------------------------------------------------------- + +#[tokio::test] +async fn list_credentials_returns_stored_services() { + use base64::Engine as _; + let app = setup(); + let (session, wallet, app) = create_test_session(app).await; + + let ct1 = base64::engine::general_purpose::STANDARD.encode(b"key1"); + let ct2 = base64::engine::general_purpose::STANDARD.encode(b"key2"); + + let (s1, _) = post_json_auth( + app.clone(), + "/credential/store", + &session, + json!({ "agent_id": wallet, "service": "anthropic", "ciphertext": ct1 }), + ) + .await; + assert_eq!(s1, StatusCode::OK); + + let (s2, _) = post_json_auth( + app.clone(), + "/credential/store", + &session, + json!({ "agent_id": wallet, "service": "openrouter", "ciphertext": ct2 }), + ) + .await; + assert_eq!(s2, StatusCode::OK); + + let path = format!("/credential/list?agent_id={}", wallet); + let (status, json) = get_json_auth(app, &path, &session).await; + assert_eq!(status, StatusCode::OK, "{json}"); + + let services: Vec<&str> = json["services"] + .as_array() + .unwrap() + .iter() + .map(|v| v.as_str().unwrap()) + .collect(); + assert_eq!(services, vec!["anthropic", "openrouter"], "should be sorted"); +} + +#[tokio::test] +async fn list_credentials_empty_for_unknown_agent() { + let app = setup(); + let (session, wallet, app) = create_test_session(app).await; + + let path = format!("/credential/list?agent_id={}", wallet); + let (status, json) = get_json_auth(app, &path, &session).await; + assert_eq!(status, StatusCode::OK, "{json}"); + + let services = json["services"].as_array().unwrap(); + assert!(services.is_empty(), "should be empty for agent with no credentials"); +} + +#[tokio::test] +async fn list_credentials_ownership_enforced() { + use base64::Engine as _; + let app = setup(); + let (session_a, wallet_a, app) = create_test_session(app).await; + + let ct = base64::engine::general_purpose::STANDARD.encode(b"secret"); + let (s, _) = post_json_auth( + app.clone(), + "/credential/store", + &session_a, + json!({ "agent_id": wallet_a, "service": "github", "ciphertext": ct }), + ) + .await; + assert_eq!(s, StatusCode::OK); + + // Use a distinct auth token so user B gets a different wallet from user A + let (status_b, json_b) = post_json( + app.clone(), + "/session/create", + json!({ "auth_token": "test-token-user-b-unique" }), + ) + .await; + assert_eq!(status_b, StatusCode::OK); + let session_b = json_b["session"].as_str().unwrap().to_string(); + + let path = format!("/credential/list?agent_id={}", wallet_a); + let (status, _) = get_json_auth(app.clone(), &path, &session_b).await; + assert_eq!(status, StatusCode::FORBIDDEN, "user B must not list user A's credentials"); + + // Codex P2 on PR #19: a denied list_credentials must also leave an audit + // trail so cross-agent probing through the new /credential/list endpoint + // is visible. Query the audit log via the existing /audit endpoint + // (filtered by agent=wallet_a; user A can see events where their wallet is + // the agent_wallet, even when owner_wallet is user B). Confirm a DENIED + // 'list' row appears. + let audit_path = format!("/audit/query?agent={}", wallet_a); + let (audit_status, audit_body) = get_json_auth(app, &audit_path, &session_a).await; + assert_eq!(audit_status, StatusCode::OK, "audit query failed: {audit_body}"); + let events = audit_body["events"].as_array().expect("events array"); + assert!( + events + .iter() + .any(|e| e["action"] == "list" && e["result"] == "DENIED"), + "expected a list/DENIED audit row after the cross-agent list attempt, got: {audit_body}" + ); +} diff --git a/docs/contradictions.md b/docs/contradictions.md index 631352f..e7db3bf 100644 --- a/docs/contradictions.md +++ b/docs/contradictions.md @@ -234,7 +234,7 @@ Both are true at different horizons. But "instantly" in `key-security.md:221` re **Resolution.** Landed in `fix/issue-17` (PR pending merge). `cmd_revoke` takes `Option<&str>` — no-args self-revokes + wipes local session; wallet form calls new `CredentialBackend::revoke_by_wallet` trait method; backend `/session/revoke` handler now accepts either `target_session` (token) or `target_wallet` (wallet). `wiki/credential-usage.md` now carries the revoke-vs-teardown table. `docs/manual-test-stage4.md` Test 9 passes on the fix branch. See `docs/manual-test-issue-17.md` for the full reproduction + verification walkthrough. -### 4.2 `agentkeys run` broken for master sessions (MAJOR — issue #15) +### 4.2 `agentkeys run` broken for master sessions (PARTIALLY RESOLVED 2026-04-14 — fix/issue-15 parts 1+2) - `crates/agentkeys-cli/src/lib.rs:156-160`: when `session.scope = None`, `services_to_try = vec![]` → nothing injected. - `docs/manual-test-stage4.md:697-701`: Test 9 marks run SKIPPED due to #15. @@ -242,7 +242,7 @@ Both are true at different horizons. But "instantly" in `key-security.md:221` re - `development-stages.md:294` (Stage 2): test `cli::run_injects_env` expects `agentkeys run my-agent -- env` output to contain `OPENROUTER_API_KEY=sk-xxx` — this test cannot pass for master sessions today. - Issue #15 also requests: (a) fix scope-None path to query all stored credentials, (b) `agentkeys scope --add service` CLI command, (c) `--env` override flag. -**Resolution.** Land the scope-None fix from #15 before Stage 8 (small patch). `--env` override can slip to v0.1. Update Stage 2 test description to match. 3 files. +**Resolution.** Parts (a) and (c) landed in `fix/issue-15` (PR pending merge). `CredentialBackend::list_credentials(session, agent_id)` trait method + mock-server endpoint `GET /credential/list?agent_id=` enable master sessions to enumerate stored services. `cmd_run` takes `--env KEY=service` (repeatable) as an escape hatch for services whose canonical env var doesn't match the auto-convention. Part (b) — scope-edit CLI command — remains open, tracked as story `fix-15b` in `.omc/prd.json`; this entry becomes fully RESOLVED when that follow-up PR lands. ### 4.3 Wallet-optional CLI + identity aliases (MAJOR — issue #16) diff --git a/docs/manual-test-issue-15.md b/docs/manual-test-issue-15.md new file mode 100644 index 0000000..738461f --- /dev/null +++ b/docs/manual-test-issue-15.md @@ -0,0 +1,172 @@ +# Manual Test: Issue #15 (Parts 1+2) — `agentkeys run` for master sessions + `--env` override + +**Issue:** [litentry/agentKeys#15](https://github.com/litentry/agentKeys/issues/15) — CLI `run` command broken for master sessions + missing scope edit and `--env` flag. + +**Branch:** `fix/issue-15` + +**PR scope:** Parts 1 (master session support) and 2 (`--env` flag). Part 3 (scope-edit CLI command) is tracked as story `fix-15b` in `.omc/prd.json` and will ship in a follow-up PR. + +## What changed + +On `main`, master sessions (`scope: None`) silently injected nothing into child processes via `agentkeys run` because `cmd_run` relied exclusively on `session.scope.services`. This blocked Test 9 of the stage-4 manual test. + +This PR: +- Adds `CredentialBackend::list_credentials(session, agent_id)` trait method + HTTP endpoint `GET /credential/list?agent_id=` (ownership-enforced). +- `cmd_run` now falls back to `list_credentials` when `session.scope.is_none()` and injects all stored credentials. +- Adds `--env KEY=service` flag (repeatable) that maps explicit env-var names to services — an escape hatch for services whose canonical env var doesn't follow the `SERVICE_API_KEY` convention (e.g. GitHub → `GITHUB_TOKEN`). + +## Preconditions + +- Rust toolchain (`cargo --version` ≥ 1.80). +- `jq` installed. +- No mock server running on `127.0.0.1:8090`. +- Working dir: `~/Projects/agentkeys`. + +## Setup + +```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 +BIN=$(pwd)/target/release/agentkeys +``` + +## Reproduce the bug (on `main`) + +```bash +cargo run --release -p agentkeys-mock-server & +MOCK_PID=$! +sleep 1 + +$BIN --backend $BACKEND init --mock-token run-repro +WALLET=$(jq -r .wallet "$HOME/.agentkeys/session.json") +$BIN --backend $BACKEND store $WALLET openrouter sk-or-v1-repro + +$BIN --backend $BACKEND run $WALLET -- printenv OPENROUTER_API_KEY +# Expected (on main, BROKEN): empty — master session, scope=None, nothing injected + +kill $MOCK_PID +``` + +## Verify the fix (on `fix/issue-15`) + +### Case 1 — master session injects all stored credentials + +```bash +rm -f $HOME/.agentkeys/session.json +cargo run --release -p agentkeys-mock-server & +MOCK_PID=$! +sleep 1 + +$BIN --backend $BACKEND init --mock-token master-inject +WALLET=$(jq -r .wallet "$HOME/.agentkeys/session.json") + +$BIN --backend $BACKEND store $WALLET openrouter sk-openrouter-value +$BIN --backend $BACKEND store $WALLET anthropic sk-anthropic-value + +$BIN --backend $BACKEND run $WALLET -- printenv OPENROUTER_API_KEY +# Expected: "sk-openrouter-value" + +$BIN --backend $BACKEND run $WALLET -- printenv ANTHROPIC_API_KEY +# Expected: "sk-anthropic-value" + +# Both stored keys are injected in a single invocation: +$BIN --backend $BACKEND run $WALLET -- sh -c 'echo "OR=$OPENROUTER_API_KEY AN=$ANTHROPIC_API_KEY"' +# Expected: "OR=sk-openrouter-value AN=sk-anthropic-value" + +kill $MOCK_PID +``` + +### Case 2 — `--env` flag overrides default naming + +```bash +rm -f $HOME/.agentkeys/session.json +cargo run --release -p agentkeys-mock-server & +MOCK_PID=$! +sleep 1 + +$BIN --backend $BACKEND init --mock-token env-override +WALLET=$(jq -r .wallet "$HOME/.agentkeys/session.json") + +$BIN --backend $BACKEND store $WALLET github ghp_testtoken + +$BIN --backend $BACKEND run $WALLET --env GITHUB_TOKEN=github -- printenv GITHUB_TOKEN +# Expected: "ghp_testtoken" + +# Verify: the --env override suppresses the auto-derived name. +$BIN --backend $BACKEND run $WALLET --env GITHUB_TOKEN=github -- sh -c 'echo "GT=$GITHUB_TOKEN GA=${GITHUB_API_KEY:-unset}"' +# Expected: "GT=ghp_testtoken GA=unset" OR GA=ghp_testtoken depending on whether +# --env replaces or supplements the default. PR description documents the chosen +# semantics — either way this line must behave as documented. + +# Multiple --env flags stack: +$BIN --backend $BACKEND store $WALLET openrouter sk-or-override +$BIN --backend $BACKEND run $WALLET --env GITHUB_TOKEN=github --env OPENROUTER_KEY=openrouter -- sh -c 'echo "GT=$GITHUB_TOKEN OR=$OPENROUTER_KEY"' +# Expected: "GT=ghp_testtoken OR=sk-or-override" + +kill $MOCK_PID +``` + +### Case 3 — `--env` with invalid format fails cleanly + +```bash +rm -f $HOME/.agentkeys/session.json +cargo run --release -p agentkeys-mock-server & +MOCK_PID=$! +sleep 1 + +$BIN --backend $BACKEND init --mock-token env-invalid +WALLET=$(jq -r .wallet "$HOME/.agentkeys/session.json") + +$BIN --backend $BACKEND run $WALLET --env NO_EQUALS -- true 2>&1 | head -3 +# Expected: error containing "invalid --env" or "expected KEY=service" +# The child process must NOT be spawned. + +kill $MOCK_PID +``` + +### Case 4 — `list_credentials` ownership enforced + +```bash +rm -f $HOME/.agentkeys/session.json +cargo run --release -p agentkeys-mock-server & +MOCK_PID=$! +sleep 1 + +# User A stores credentials +$BIN --backend $BACKEND init --mock-token user-a-store +A_WALLET=$(jq -r .wallet "$HOME/.agentkeys/session.json") +$BIN --backend $BACKEND store $A_WALLET openrouter sk-secret + +# User B initializes (different mock token — different wallet; overwrites local session) +$BIN --backend $BACKEND init --mock-token user-b-probe +B_WALLET=$(jq -r .wallet "$HOME/.agentkeys/session.json") + +# User B tries to list user A's credentials via run (uses list_credentials under the hood) +$BIN --backend $BACKEND run $A_WALLET -- printenv OPENROUTER_API_KEY 2>&1 | head -3 +# Expected: error — permission denied / 403 (backend enforces ownership) + +kill $MOCK_PID +``` + +## Cleanup + +```bash +rm -rf "$HOME_SANDBOX" +unset HOME_SANDBOX AGENTKEYS_SESSION_STORE +``` + +## Cross-references + +- `crates/agentkeys-cli/src/lib.rs` — `cmd_run` (list_credentials fallback + `--env` parsing) +- `crates/agentkeys-cli/src/main.rs` — `Commands::Run.env: Vec` +- `crates/agentkeys-core/src/backend.rs` — new `list_credentials` trait method +- `crates/agentkeys-mock-server/src/handlers/credential.rs` — `list_credentials` HTTP handler +- `wiki/credential-usage.md` — service-name → env var convention + `--env` escape hatch (updated by this PR) +- `docs/manual-test-stage4.md` Test 9 — will be updated in PR body to remove `run` SKIPPED marker once merged +- `docs/contradictions.md` §4.2 — moves to RESOLVED once merged +- Related follow-up: story `fix-15b` in `.omc/prd.json` (scope-edit CLI command) diff --git a/docs/manual-test-stage4.md b/docs/manual-test-stage4.md index c5aa700..728ea4f 100644 --- a/docs/manual-test-stage4.md +++ b/docs/manual-test-stage4.md @@ -694,11 +694,14 @@ cargo run -p agentkeys-cli -- --backend http://localhost:8090 store $WALLET open cargo run -p agentkeys-cli -- --backend http://localhost:8090 read $WALLET openrouter # Expected: "sk-lifecycle-key" -# Run with env injection (SKIPPED -- see litentry/agentKeys#15) -# Master sessions have scope: None, so `run` injects nothing. -# Blocked until #15 is fixed (query all credentials when scope is None). -# cargo run -p agentkeys-cli -- --backend http://localhost:8090 run $WALLET -- printenv OPENROUTER_API_KEY -# Expected (after fix): "sk-lifecycle-key" +# Run with env injection (fixed in #15) +# Master sessions now query list_credentials and inject all stored keys. +cargo run -p agentkeys-cli -- --backend http://localhost:8090 run $WALLET -- printenv OPENROUTER_API_KEY +# Expected: "sk-lifecycle-key" + +# (Optional) --env override: override the auto-derived env-var name +# cargo run -p agentkeys-cli -- --backend http://localhost:8090 run $WALLET --env MY_KEY=openrouter -- printenv MY_KEY +# Expected: "sk-lifecycle-key" # Check audit trail cargo run -p agentkeys-cli -- --backend http://localhost:8090 usage $WALLET @@ -722,7 +725,7 @@ cargo run -p agentkeys-cli -- --backend http://localhost:8090 read $WALLET openr - Session is in the keychain (verified) - Store succeeds - Read returns the stored key -- `run` injects the env var correctly (SKIPPED -- litentry/agentKeys#15) +- `run` injects the env var correctly - Usage shows audit events - Revoke succeeds - Read after revoke fails with clear error diff --git a/wiki/credential-usage.md b/wiki/credential-usage.md index a9d3cf9..60edc08 100644 --- a/wiki/credential-usage.md +++ b/wiki/credential-usage.md @@ -150,3 +150,40 @@ After `revoke`: re-running `init` (same mock token / OAuth) gives you a fresh se | MCP `get_credential` | Only in daemon memory, delivered over MCP pipe | Cloud LLM agents | Always prefer `run` or MCP over `read` in production. See `wiki/key-security.md` for detailed security analysis. + +## Env-var naming convention + +When `agentkeys run` injects credentials it uses the convention: + +``` +SERVICE.to_uppercase().replace('-', '_') + "_API_KEY" +``` + +Examples: + +| Service name stored | Env var injected | +|---|---| +| `openrouter` | `OPENROUTER_API_KEY` | +| `anthropic` | `ANTHROPIC_API_KEY` | +| `github` | `GITHUB_API_KEY` | +| `my-custom-llm` | `MY_CUSTOM_LLM_API_KEY` | + +### Overriding with `--env KEY=SERVICE` + +Some tools expect non-standard env var names (e.g. the GitHub CLI expects `GITHUB_TOKEN`, not `GITHUB_API_KEY`). Use `--env KEY=SERVICE` to map a credential to an arbitrary env var name: + +```bash +agentkeys run 0xAGENT --env GITHUB_TOKEN=github -- bash deploy.sh +agentkeys run 0xAGENT --env OPENAI_API_KEY=openrouter -- python agent.py +``` + +`--env` overrides take priority over the auto-convention: if the same env var would be set by both the convention and an `--env` flag, the `--env` value wins. + +Multiple `--env` flags are supported: + +```bash +agentkeys run 0xAGENT \ + --env GITHUB_TOKEN=github \ + --env HF_TOKEN=huggingface \ + -- python train.py +```