fix(agent): route set_identity through agent to preserve meta-tag integrity (#242)

* fix(agent): route set_identity through agent to preserve meta-tag integrity

set_identity previously called save_meta() directly from the CLI process,
modifying the .meta file without updating the HMAC meta-tag in the keychain.
This broke all subsequent sign operations because ensure_meta_integrity()
detected the file/tag mismatch and rejected it as tampering.

Route identity updates through a new SetIdentity agent IPC message (0xF7).
The agent modifies the .meta file and atomically re-stamps the meta-tag,
maintaining integrity. Also fix error swallowing in the sign path where
backend.get() failures were logged without the actual error message, and
update AGENTS.md with stronger CI/CD-only binary guidance.

Closes #240

* fix(agent): make meta-tag re-stamp non-fatal in SetIdentity

On platforms where the meta-integrity store is unavailable (e.g. Linux
without Secret Service), perform_migrate_meta fails. Since the identity
metadata was already saved successfully, log the re-stamp failure as a
warning instead of propagating it as an error.

* fix(e2e): start agent before sshenc identity in tests

sshenc identity now routes through the agent IPC, so the agent must
be running. Two e2e tests called sshenc identity without starting the
agent first: gitenc_config_signs_commit_and_verifies and
identity_persists_through_metadata.

Author: jgowdy-godaddy

AGENTS.md

@@ -19,18 +19,34 @@ Use your judgement on Windows/Linux: prefer the installed binary when one
 exists for parity reasons, but locally-built binaries are not a footgun on
 those platforms the way they are on macOS.
 
-## CRITICAL: Never Run Unsigned Binaries (macOS)
+## CRITICAL: Always Use CI/CD-Built Binaries (macOS)
 
-**DO NOT** run binaries from development builds (`cargo build`, `cargo run`, `~/.cargo/bin/*`) **on macOS** in production contexts or when they could access the Secure Enclave. (See the Platform Scope section above for Windows/Linux scoping.)
+**NEVER build, codesign, or install custom macOS binaries.** Always use the CI/CD release
+pipeline (`git tag vX.Y.Z` → GitHub Actions → Homebrew). This is non-negotiable.
 
-### Why This Matters
+The CI/CD pipeline performs steps that **cannot be replicated locally**:
 
-sshenc accesses hardware-backed cryptographic storage (macOS Secure Enclave, Windows TPM, Linux software keys). Running unsigned development builds as agents can:
+1. **Apple notarization** — AMFI (Apple Mobile File Integrity) only grants `keychain-access-groups`
+   entitlements to notarized app bundles. Without notarization, Secure Enclave key creation with
+   `.userPresence` access control fails with `-25308 (errSecInteractionNotAllowed)`.
+2. **Provisioning profile embedding** — required for the app bundle's entitlements to be honored.
+3. **Proper app bundle signing** — the entire `.app` bundle must be signed as a unit; signing
+   individual binaries and placing them in an existing bundle breaks the seal.
 
-1. **Poison the keychain** — unsigned agents create keychain entries that conflict with production agents
-2. **Trigger unexpected auth prompts** — users see Touch ID/password prompts from the wrong binary
-3. **Break signing** — the wrong agent binary services signing requests, causing failures
-4. **Leave stale processes** — development agents don't clean up properly when killed
+**DO NOT** run binaries from development builds (`cargo build`, `cargo run`, `~/.cargo/bin/*`)
+**on macOS** in production contexts or when they could access the Secure Enclave.
+(See the Platform Scope section above for Windows/Linux scoping.)
+
+### What Goes Wrong With Custom Builds
+
+sshenc accesses hardware-backed cryptographic storage (macOS Secure Enclave, Windows TPM, Linux software keys). Running non-CI/CD builds as agents can:
+
+1. **Fail silently on key generation** — AMFI rejects the entitlements, SE key creation with `.userPresence` returns `-25308`
+2. **Poison the keychain** — unsigned agents create keychain entries that conflict with production agents
+3. **Invalidate HMAC integrity** — unsigned binaries writing to `~/.sshenc/keys/*.meta` files corrupt the HMAC sidecar, breaking key operations for the production binary
+4. **Trigger unexpected auth prompts** — users see Touch ID/password prompts from the wrong binary
+5. **Break signing** — the wrong agent binary services signing requests, causing failures
+6. **Leave stale processes** — development agents don't clean up properly when killed
 
 ### Safe vs Unsafe Operations
 
@@ -132,15 +148,18 @@ cargo test --workspace
 cargo clippy --workspace --all-targets -- -D warnings
 cargo fmt --all
 
-# DO NOT run the built binaries as daemons
+# DO NOT run the built binaries as daemons or install them
 # DO NOT run: cargo run --bin sshenc-agent
 # DO NOT run: ~/.cargo/bin/sshenc agent
-
-# To test agent changes, install via homebrew first:
-cargo build --release
-# ... create PR, merge, release via CI ...
-brew upgrade sshenc
-# NOW it's safe to test agent functionality
+# DO NOT copy binaries into /opt/homebrew or any app bundle
+# DO NOT attempt to codesign binaries locally — notarization cannot be done locally
+
+# To test agent/signing changes, use the CI/CD release pipeline:
+# 1. Create PR, get it reviewed and merged
+# 2. Tag a new release: git tag vX.Y.Z && git push origin vX.Y.Z
+# 3. Wait for GitHub Actions release workflow to complete
+# 4. brew upgrade sshenc
+# 5. NOW it's safe to test agent functionality
 ```
 
 ### For Users Building From Source
@@ -190,8 +209,11 @@ git commit -S -m "your message"
 
 ## Summary
 
-- ✅ Build and test code with `cargo`
-- ✅ Use production agents from `/opt/homebrew/bin`
+- ✅ Build and test code with `cargo build`, `cargo test`, `cargo clippy`
+- ✅ Use CI/CD-built binaries from Homebrew (`brew upgrade sshenc`)
+- ✅ Use the release pipeline for any binary changes (tag → CI → Homebrew)
 - ❌ Don't run `~/.cargo/bin/sshenc-agent` as a daemon
+- ❌ Don't build, codesign, or install custom macOS binaries — notarization cannot be done locally
+- ❌ Don't copy binaries into Homebrew's Cellar or app bundles
 - ❌ Don't test signing with development builds
 - 🔍 Always verify which agent binary is running before signing operations

crates/sshenc-agent-proto/src/client.rs

@@ -28,7 +28,7 @@ use crate::message::{
     self, AgentRequest, AgentResponse, SSH_AGENTC_REQUEST_IDENTITIES, SSH_AGENTC_SIGN_REQUEST,
     SSH_AGENTC_SSHENC_CHECK_MIGRATION_MARKER, SSH_AGENTC_SSHENC_DELETE_KEY,
     SSH_AGENTC_SSHENC_GENERATE_KEY, SSH_AGENTC_SSHENC_MIGRATE_META, SSH_AGENTC_SSHENC_RENAME_KEY,
-    SSH_AGENTC_SSHENC_SET_MIGRATION_MARKER,
+    SSH_AGENTC_SSHENC_SET_IDENTITY, SSH_AGENTC_SSHENC_SET_MIGRATION_MARKER,
 };
 use std::io::{Read, Write};
 use std::path::Path;
@@ -351,6 +351,16 @@ pub fn try_set_migration_marker_via_socket(sock_path: &Path) -> Option<()> {
     request_set_migration_marker(&mut stream)
 }
 
+pub fn try_set_identity_via_socket(
+    sock_path: &Path,
+    label: &str,
+    name: &str,
+    email: &str,
+) -> Option<()> {
+    let mut stream = connect_agent(sock_path)?;
+    request_set_identity(&mut stream, label, name, email)
+}
+
 /// Best-effort lookup of the agent socket from the environment —
 /// `SSH_AUTH_SOCK` on Unix, or on Windows (cmd.exe, PowerShell, Git
 /// Bash, etc.) the same variable if set. If absent, returns `None`
@@ -554,6 +564,32 @@ fn request_migrate_meta<S: Read + Write>(stream: &mut S, label: &str) -> Option<
     }
 }
 
+fn request_set_identity<S: Read + Write>(
+    stream: &mut S,
+    label: &str,
+    name: &str,
+    email: &str,
+) -> Option<()> {
+    let payload = message::serialize_request(&AgentRequest::SetIdentity {
+        label: label.as_bytes().to_vec(),
+        name: name.as_bytes().to_vec(),
+        email: email.as_bytes().to_vec(),
+    });
+    debug_assert_eq!(payload[0], SSH_AGENTC_SSHENC_SET_IDENTITY);
+    send_framed(stream, &payload)?;
+    match recv_response(stream)? {
+        AgentResponse::Success => Some(()),
+        AgentResponse::Failure => {
+            tracing::debug!("agent proxy: set-identity returned FAILURE");
+            None
+        }
+        other => {
+            tracing::debug!(?other, "agent proxy: unexpected response to set-identity");
+            None
+        }
+    }
+}
+
 fn request_check_migration_marker<S: Read + Write>(stream: &mut S) -> Option<bool> {
     let payload = message::serialize_request(&AgentRequest::CheckMigrationMarker);
     debug_assert_eq!(payload[0], SSH_AGENTC_SSHENC_CHECK_MIGRATION_MARKER);

crates/sshenc-agent-proto/src/message.rs

@@ -59,6 +59,11 @@ pub const SSH_AGENTC_SSHENC_CHECK_MIGRATION_MARKER: u8 = 0xF5;
 /// migrate-meta calls succeed. After this, the agent's legacy_meta
 /// error variant switches to the strong-tamper-warning form.
 pub const SSH_AGENTC_SSHENC_SET_MIGRATION_MARKER: u8 = 0xF6;
+/// `SSH_AGENTC_SSHENC_SET_IDENTITY`: have the agent update the
+/// git identity (name + email) in a key's `.meta` and re-stamp the
+/// meta-tag. The CLI must NOT modify `.meta` directly — every meta
+/// mutation goes through the agent so the keychain tag stays in sync.
+pub const SSH_AGENTC_SSHENC_SET_IDENTITY: u8 = 0xF7;
 
 // sshenc-specific response: carries the public-key bytes of a
 // freshly-generated key back to the CLI. On failure the agent uses
@@ -168,6 +173,15 @@ pub enum AgentRequest {
     /// write the migrate-meta marker to the keychain. Sent by the
     /// CLI after all per-label migrations succeed.
     SetMigrationMarker,
+    /// `SSH_AGENTC_SSHENC_SET_IDENTITY` (sshenc extension):
+    /// update the git name and email in a key's `.meta` and re-stamp
+    /// the meta-tag atomically. The CLI delegates here instead of
+    /// writing `.meta` directly so the keychain tag is always in sync.
+    SetIdentity {
+        label: Vec<u8>,
+        name: Vec<u8>,
+        email: Vec<u8>,
+    },
     /// An unrecognized message type.
     Unknown(u8),
 }
@@ -271,6 +285,13 @@ pub fn parse_request(payload: &[u8]) -> Result<AgentRequest> {
         }
         SSH_AGENTC_SSHENC_CHECK_MIGRATION_MARKER => Ok(AgentRequest::CheckMigrationMarker),
         SSH_AGENTC_SSHENC_SET_MIGRATION_MARKER => Ok(AgentRequest::SetMigrationMarker),
+        SSH_AGENTC_SSHENC_SET_IDENTITY => {
+            let mut cursor = Cursor::new(body);
+            let label = wire::read_string(&mut cursor)?;
+            let name = wire::read_string(&mut cursor)?;
+            let email = wire::read_string(&mut cursor)?;
+            Ok(AgentRequest::SetIdentity { label, name, email })
+        }
         other => Ok(AgentRequest::Unknown(other)),
     }
 }
@@ -370,6 +391,13 @@ pub fn serialize_request(request: &AgentRequest) -> Vec<u8> {
         }
         AgentRequest::CheckMigrationMarker => vec![SSH_AGENTC_SSHENC_CHECK_MIGRATION_MARKER],
         AgentRequest::SetMigrationMarker => vec![SSH_AGENTC_SSHENC_SET_MIGRATION_MARKER],
+        AgentRequest::SetIdentity { label, name, email } => {
+            let mut buf = vec![SSH_AGENTC_SSHENC_SET_IDENTITY];
+            wire::write_string(&mut buf, label);
+            wire::write_string(&mut buf, name);
+            wire::write_string(&mut buf, email);
+            buf
+        }
         AgentRequest::Unknown(t) => vec![*t],
     }
 }
@@ -1199,4 +1227,34 @@ mod tests {
             "got {parsed:?}"
         );
     }
+
+    // --- sshenc SetIdentity extension ---
+
+    #[test]
+    fn roundtrip_set_identity_request() {
+        let original = AgentRequest::SetIdentity {
+            label: b"my-key".to_vec(),
+            name: b"Jay Gowdy".to_vec(),
+            email: b"jay@example.com".to_vec(),
+        };
+        let payload = serialize_request(&original);
+        assert_eq!(payload[0], SSH_AGENTC_SSHENC_SET_IDENTITY);
+        let parsed = parse_request(&payload).unwrap();
+        match parsed {
+            AgentRequest::SetIdentity { label, name, email } => {
+                assert_eq!(label, b"my-key");
+                assert_eq!(name, b"Jay Gowdy");
+                assert_eq!(email, b"jay@example.com");
+            }
+            other => panic!("expected SetIdentity, got {other:?}"),
+        }
+    }
+
+    #[test]
+    fn parse_rejects_truncated_set_identity_request() {
+        let mut payload = vec![SSH_AGENTC_SSHENC_SET_IDENTITY];
+        wire::write_string(&mut payload, b"label");
+        // Missing name and email
+        assert!(parse_request(&payload).is_err());
+    }
 }

crates/sshenc-agent/src/server.rs

@@ -1288,9 +1288,10 @@ async fn handle_request(
                 .await
             {
                 Ok(Ok(k)) => k,
-                Ok(Err(_)) => {
+                Ok(Err(e)) => {
                     tracing::warn!(
                         label = label.as_str(),
+                        error = %e,
                         "key lookup failed; evicting from cache"
                     );
                     blob_cache.remove(&key_blob);
@@ -1797,6 +1798,82 @@ async fn handle_request(
                 }
             }
         }
+        AgentRequest::SetIdentity { label, name, email } => {
+            let label_str = match std::str::from_utf8(&label) {
+                Ok(s) => s,
+                Err(_) => {
+                    tracing::warn!("set_identity: label not valid UTF-8");
+                    return Ok(AgentResponse::Failure);
+                }
+            };
+            let name_str = match std::str::from_utf8(&name) {
+                Ok(s) => s,
+                Err(_) => {
+                    tracing::warn!("set_identity: name not valid UTF-8");
+                    return Ok(AgentResponse::Failure);
+                }
+            };
+            let email_str = match std::str::from_utf8(&email) {
+                Ok(s) => s,
+                Err(_) => {
+                    tracing::warn!("set_identity: email not valid UTF-8");
+                    return Ok(AgentResponse::Failure);
+                }
+            };
+            tracing::info!(label = label_str, "handling set_identity request");
+
+            if !allowed_labels.is_empty() && !allowed_labels.contains(label_str) {
+                tracing::warn!(
+                    label = label_str,
+                    "set_identity: label not in agent's allowed list"
+                );
+                return Ok(AgentResponse::Failure);
+            }
+
+            let dir = sshenc_se::sshenc_keys_dir();
+            let started = Instant::now();
+
+            let result = (|| -> Result<(), String> {
+                let mut meta = sshenc_se::compat::load_sshenc_meta(&dir, label_str)
+                    .map_err(|e| format!("load meta: {e}"))?;
+                meta.set_app_field("git_name", name_str.to_string());
+                meta.set_app_field("git_email", email_str.to_string());
+                enclaveapp_core::metadata::save_meta(&dir, label_str, &meta)
+                    .map_err(|e| format!("save meta: {e}"))?;
+                if let Err(e) = perform_migrate_meta(&dir, label_str) {
+                    tracing::warn!(
+                        label = label_str,
+                        error = %e,
+                        "set_identity: meta-tag re-stamp failed (meta saved successfully)"
+                    );
+                }
+                Ok(())
+            })();
+
+            let error_msg = result
+                .as_ref()
+                .err()
+                .map(|e| format!("set_identity failed: {e}"));
+            crate::op_log::record(
+                "set_identity",
+                Some(label_str),
+                None,
+                started.elapsed(),
+                result.is_ok(),
+                error_msg.as_deref(),
+                None,
+            );
+            match result {
+                Ok(()) => {
+                    tracing::info!(label = label_str, "set_identity: succeeded");
+                    Ok(AgentResponse::Success)
+                }
+                Err(e) => {
+                    tracing::warn!(label = label_str, error = %e, "set_identity: failed");
+                    Ok(AgentResponse::Failure)
+                }
+            }
+        }
         AgentRequest::CheckMigrationMarker => {
             // Success = marker SET. Failure = marker NOT set or
             // keychain unreachable. The CLI treats both failure

crates/sshenc-cli/src/commands.rs

@@ -2130,21 +2130,16 @@ pub fn promote_to_default(label: &str) -> Result<()> {
 }
 
 #[allow(clippy::print_stdout)]
-pub fn set_identity(label: &str, name: &str, email: &str) -> Result<()> {
+pub fn set_identity(label: &str, name: &str, email: &str, socket_path: &Path) -> Result<()> {
     let _label = KeyLabel::new(label)?;
 
-    use enclaveapp_core::metadata;
-
-    let keys_dir = sshenc_keys_dir();
-
-    let mut meta = sshenc_se::compat::load_sshenc_meta(&keys_dir, label)
-        .map_err(|e| anyhow::anyhow!("failed to load metadata for '{label}': {e}"))?;
-    meta.set_app_field("git_name", name.to_string());
-    meta.set_app_field("git_email", email.to_string());
-
-    // Write updated metadata
-    metadata::save_meta(&keys_dir, label, &meta)
-        .map_err(|e| anyhow::anyhow!("failed to save metadata for '{label}': {e}"))?;
+    sshenc_agent_proto::client::try_set_identity_via_socket(socket_path, label, name, email)
+        .ok_or_else(|| {
+            anyhow::anyhow!(
+                "failed to set identity for '{label}': agent did not accept the request. \
+             Is the sshenc agent running?"
+            )
+        })?;
 
     println!("Set identity for key '{label}':");
     println!("  Name:  {name}");

crates/sshenc-cli/src/main.rs

@@ -385,15 +385,19 @@ fn main() -> Result<()> {
     // produce protocol-mismatch errors on benign setups.
     let socket_path = commands::client_socket_path(&config.socket_path);
     let backend: Box<dyn sshenc_se::KeyBackend> = Box::new(
-        sshenc_se::AgentProxyBackend::new(config.pub_dir.clone(), socket_path)
+        sshenc_se::AgentProxyBackend::new(config.pub_dir.clone(), socket_path.clone())
             .map_err(|e| anyhow::anyhow!("failed to initialize agent-proxy backend: {e}"))?,
     );
 
-    run_command(cli.command, &*backend)
+    run_command(cli.command, &*backend, &socket_path)
 }
 
 #[allow(clippy::print_stdout, clippy::print_stderr)]
-fn run_command(command: Commands, backend: &dyn sshenc_se::KeyBackend) -> Result<()> {
+fn run_command(
+    command: Commands,
+    backend: &dyn sshenc_se::KeyBackend,
+    socket_path: &std::path::Path,
+) -> Result<()> {
     match command {
         Commands::Keygen {
             label,
@@ -572,7 +576,9 @@ fn run_command(command: Commands, backend: &dyn sshenc_se::KeyBackend) -> Result
         },
         Commands::Install => commands::install(),
         Commands::Uninstall => commands::uninstall(),
-        Commands::Identity { label, name, email } => commands::set_identity(&label, &name, &email),
+        Commands::Identity { label, name, email } => {
+            commands::set_identity(&label, &name, &email, socket_path)
+        }
         Commands::Default { label } => commands::promote_to_default(&label),
         Commands::Ssh { label, ssh_args } => commands::ssh_wrapper(label.as_deref(), &ssh_args),
         #[cfg(windows)]