fix(cli): #19 codex P2 v2 — pre-flight --env validation before backend I/O

Two correctness issues from the second codex pass on PR #19:

1. Pre-flight --env validation ran AFTER the master-session
   list_credentials() call, so a malformed `--env NO_EQUALS` could still
   produce a backend round-trip and a list/DENIED audit row before the
   parse error fired. Move the validation loop above the
   list_credentials branch so the pre-flight guarantee in the comment
   actually holds for the master-session path.

2. The validation only checked for the presence of '=', so `--env =foo`
   (empty KEY) and `--env BAR=` (empty SERVICE) were accepted and then
   blew up later as opaque runtime errors (empty env-var name passed to
   process spawn / empty service name passed to read_credential).
   Reject both up front with a clear "KEY must not be empty" /
   "SERVICE must not be empty" message.

Tests: cmd_run_env_flag_empty_key_rejected,
cmd_run_env_flag_empty_service_rejected.

Test: cargo test -p agentkeys-cli -- --test-threads=1 → 20 passed.

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

Author: hanwencheng

crates/agentkeys-cli/src/lib.rs

@@ -160,6 +160,31 @@ pub async fn cmd_run(
 
     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<String> = if let Some(scope) = &session.scope {
         scope.services.iter().map(|s| s.0.clone()).collect()
     } else {
@@ -172,17 +197,6 @@ pub async fn cmd_run(
             .collect()
     };
 
-    // 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).
-    for raw in env_overrides {
-        if raw.find('=').is_none() {
-            return Err(anyhow!(
-                "Invalid --env format '{}': expected KEY=SERVICE (no '=' found)",
-                raw
-            ));
-        }
-    }
-
     // 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
@@ -214,12 +228,7 @@ pub async fn cmd_run(
     }
 
     for raw in env_overrides {
-        let eq_pos = raw.find('=').ok_or_else(|| {
-            anyhow!(
-                "Invalid --env format '{}': expected KEY=SERVICE (no '=' found)",
-                raw
-            )
-        })?;
+        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..];
 

crates/agentkeys-cli/tests/cli_tests.rs

@@ -367,3 +367,47 @@ async fn cmd_run_env_flag_invalid_format() {
         "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}"
+    );
+}