fix(cli/mock): #19 codex P2 fixes — credential cache + audit DENIED row

P2-1 (cli): cmd_run was issuing two GET /credential/read calls when an
--env override targeted a service that auto-injection had already
fetched. Cache successful reads in a HashMap<service, ciphertext> and
reuse the cached value during --env resolution. Halves credential RTTs
for the common "auto-inject + override" flow.

P2-2 (mock): list_credentials returned 403 on cross-agent attempts
without leaving an audit trail, so probing through the new
/credential/list endpoint stayed invisible. Mirror the read_credential
contract by inserting a 'list' / 'DENIED' audit row (service_name='*')
before the 403 return. Extend integration test
list_credentials_ownership_enforced to assert the DENIED row appears.

Test: cargo test -p agentkeys-cli -p agentkeys-mock-server -- --test-threads=1
mock-server: 40 passed; cli: 18 passed.

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

Author: hanwencheng

crates/agentkeys-cli/src/lib.rs

@@ -183,6 +183,12 @@ pub async fn cmd_run(
         }
     }
 
+    // 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<String, String> =
+        std::collections::HashMap::new();
     let mut env_vars: Vec<(String, String)> = Vec::new();
     let mut credential_errors: Vec<String> = Vec::new();
     for service in &services_to_try {
@@ -191,6 +197,7 @@ pub async fn cmd_run(
             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) => {
@@ -215,12 +222,26 @@ pub async fn cmd_run(
         })?;
         let env_key = raw[..eq_pos].to_string();
         let service = &raw[eq_pos + 1..];
-        let service_name = ServiceName(service.to_string());
-        let bytes = backend
-            .read_credential(&session, &agent_id, &service_name)
-            .await
-            .map_err(wrap_backend_error)?;
-        let value = String::from_utf8_lossy(&bytes).to_string();
+
+        // 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 {

crates/agentkeys-mock-server/src/handlers/credential.rs

@@ -201,6 +201,18 @@ pub async fn list_credentials(
     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

crates/agentkeys-mock-server/tests/integration.rs

@@ -1349,6 +1349,23 @@ async fn list_credentials_ownership_enforced() {
     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, &path, &session_b).await;
+    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}"
+    );
 }