fix: trust CA in login keychain instead of system keychain

docs/plans/2026-03-10-fix-ca-trust-login-keychain-test-plan.md

@@ -0,0 +1,70 @@
+# Test Plan: Fix CA Trust to Use Login Keychain
+
+## Strategy reconciliation
+
+The implementation plan has four tasks: (1) add a `login_keychain_path` helper with a unit test, (2) update `trust_ca_in_system` to use the login keychain, (3) update user-facing messages in `init.rs`, and (4) run the full test suite.
+
+The approved testing strategy calls for:
+- **Unit test in cert.rs** for command argument verification
+- **Functional integration test marked `#[ignore]`** that touches the real login keychain with cleanup
+
+The implementation plan already includes a unit test for the `login_keychain_path` helper (Task 1). The strategy adds two things the plan does not cover: (a) a unit test verifying the `security` command arguments assembled by `trust_ca_in_system`, and (b) a functional integration test that actually adds/removes a cert from the login keychain.
+
+For (a), `trust_ca_in_system` calls `Command::new("security")` directly and returns a `Result` based on the exit code — there is no seam to intercept the command arguments without refactoring. Rather than introduce a trait/mock boundary for a one-off fix, we will verify the command arguments **indirectly** by confirming the `login_keychain_path` helper returns the correct path (unit test) and that the assembled command succeeds against a real keychain (functional test). This keeps the scope minimal and avoids speculative abstraction.
+
+For the init.rs message changes (Task 3), the messages are `eprintln!` output with no structured return value. The e2e test infrastructure runs `devproxy init` and captures stderr, but the init flow requires Docker and creates real daemon state. Adding a new e2e test just for message text would be heavy. Instead, we rely on code review of the five specific lines called out in the plan and verify compilation via `cargo clippy`.
+
+No changes to the approved strategy are needed.
+
+## Test plan
+
+### 1. `login_keychain_path_points_to_login_keychain` — helper returns correct absolute path
+
+- **Name**: `login_keychain_path` returns an absolute path ending in `Library/Keychains/login.keychain-db`
+- **Type**: unit
+- **Location**: `src/proxy/cert.rs` — `#[cfg(test)] mod tests` block
+- **Gate**: `#[cfg(target_os = "macos")]`
+- **Preconditions**: None (uses `dirs::home_dir()` which works in test context).
+- **Actions**: Call `super::login_keychain_path()`.
+- **Expected outcome**: Returns `Ok(path)` where `path.is_absolute()` is true and `path.to_string_lossy().ends_with("Library/Keychains/login.keychain-db")`.
+- **Source of truth**: Implementation plan Task 1 specifies this exact test.
+
+### 2. `trust_ca_login_keychain_roundtrip` — functional test adds and removes cert from real login keychain
+
+- **Name**: Generate a CA cert, trust it in the login keychain, verify it appears, then remove it
+- **Type**: functional / integration
+- **Location**: `tests/e2e.rs` (or a new `tests/keychain.rs` — see note below)
+- **Gate**: `#[cfg(target_os = "macos")]`, `#[ignore]` (requires interactive keychain access, touches real system state)
+- **Preconditions**: Running on macOS. Login keychain is unlocked (normal developer workstation). No pre-existing devproxy CA in the login keychain.
+- **Actions**:
+  1. Generate a fresh CA cert via `cert::generate_ca()` and write it to a temp file.
+  2. Call `cert::trust_ca_in_system(&temp_cert_path)` — this should use the new login keychain path.
+  3. Verify the cert is present by running `security find-certificate -c "devproxy Local CA" -a ~/Library/Keychains/login.keychain-db` and checking exit code 0.
+  4. **Cleanup** (in a `Drop` guard or `defer!`-style scope to ensure it runs even on assertion failure): run `security remove-trusted-cert <temp_cert_path>` and `security delete-certificate -c "devproxy Local CA" ~/Library/Keychains/login.keychain-db`.
+- **Expected outcome**: `trust_ca_in_system` returns `Ok(())`. The certificate is findable in the login keychain. Cleanup succeeds.
+- **Interactions**: This test will trigger the macOS Keychain Access password dialog. It is marked `#[ignore]` so it does not run in CI or `cargo test`. Run manually with `cargo test --test keychain -- --ignored`.
+- **Note on test file location**: A separate `tests/keychain.rs` is preferred over adding to `tests/e2e.rs` because: (a) e2e.rs has Docker/daemon dependencies baked into its helper functions, (b) this test has completely different prerequisites (macOS keychain, no Docker), and (c) it keeps the `#[ignore]` scope narrow. The file will be small (one test function plus cleanup).
+
+### 3. Verify no regressions — existing cert unit tests still pass
+
+- **Name**: Existing `cert::tests` pass after changes
+- **Type**: regression (existing tests)
+- **Location**: `src/proxy/cert.rs` — existing `#[cfg(test)] mod tests` block
+- **Actions**: Run `cargo test --lib -- cert::tests` after each task.
+- **Expected outcome**: All three existing tests (`generate_ca_produces_valid_pem`, `generate_wildcard_cert_produces_valid_pem`, `tls_config_loads_from_generated_certs`) pass.
+
+### 4. Verify compilation and lint — full clippy + test suite
+
+- **Name**: Full `cargo clippy` and `cargo test` pass
+- **Type**: regression / gate
+- **Location**: Entire crate
+- **Actions**: Run `cargo fmt -- --check && cargo clippy --all-targets -- -D warnings && cargo test`.
+- **Expected outcome**: Zero warnings, zero failures. This catches any compile errors in the `#[cfg(target_os = "macos")]` / `#[cfg(target_os = "linux")]` branches, and ensures init.rs message changes compile.
+
+## Test execution order
+
+1. Task 1 (impl) then **Test 1** — unit test for `login_keychain_path`
+2. Task 2 (impl) then **Test 3** — regression check existing cert tests
+3. Task 3 (impl) then **Test 4** — full clippy + test gate
+4. Task 4 (impl plan's final verification)
+5. **Test 2** — manual functional test (run once locally with `--ignored` before merging)

docs/plans/2026-03-10-fix-ca-trust-login-keychain.md

@@ -0,0 +1,304 @@
+# Fix CA Trust: Use Login Keychain on macOS
+
+> **For Claude:** REQUIRED SUB-SKILL: Use trycycle-executing to implement this plan task-by-task.
+
+**Goal:** Make devproxy's CA certificate trusted by all TLS clients (curl, reqwest/native-tls, browsers) on macOS without requiring sudo, by adding it to the login keychain instead of the system keychain.
+
+**Architecture:** The fix is entirely within `src/proxy/cert.rs` (the `trust_ca_in_system` function) and `src/commands/init.rs` (user-facing messages). No new commands, no new dependencies. The `dirs` crate is already in `Cargo.toml`.
+
+**Tech Stack:** Rust, macOS `security` CLI tool
+
+## Root Cause
+
+`trust_ca_in_system()` in `src/proxy/cert.rs` uses:
+```
+security add-trusted-cert -d -r trustRoot -k /Library/Keychains/System.keychain <cert>
+```
+
+This targets the **system keychain** (`/Library/Keychains/System.keychain`), which requires `sudo`. Since devproxy runs as the current user (socket activation, no sudo), this command always fails unless the user manually runs it with sudo. The error is caught and degraded to a warning, leaving the CA untrusted.
+
+## Fix
+
+Change the macOS trust command to target the **login keychain** instead:
+```
+security add-trusted-cert -r trustRoot -k ~/Library/Keychains/login.keychain-db <cert>
+```
+
+Key differences from the current code:
+1. **`-k login.keychain-db`** instead of `-k /Library/Keychains/System.keychain` — the login keychain is writable by the current user without sudo.
+2. **Drop the `-d` flag** — the `-d` flag means "add to the admin trust settings domain" which requires admin privileges. Without `-d`, the trust setting is stored in the user's trust settings, which is sufficient for all TLS clients running as the current user.
+3. **Resolve the path dynamically** using `dirs::home_dir()` to get `~/Library/Keychains/login.keychain-db`, since the `~` tilde is not expanded by `Command::new`.
+
+**Why login keychain and not system keychain:** The system keychain requires root. The login keychain is the user's default keychain, is unlocked when the user logs in, and is trusted by all user-space TLS clients including curl, Safari, Chrome, and native-tls/Security.framework. This matches how tools like mkcert work.
+
+**Why drop `-d`:** The `-d` flag writes to the admin cert store (`/Library/Preferences/com.apple.security.admin.plist`), which requires an admin authentication prompt or sudo. Without `-d`, the trust policy is written to `~/Library/Preferences/com.apple.security.trust-settings.<hash>.plist` — the per-user trust store. All TLS clients on macOS check per-user trust settings.
+
+**Why `login.keychain-db`:** Modern macOS (10.12+) uses the `-db` suffix. The `security` command accepts both `login.keychain` and `login.keychain-db`, but using the actual filename is more robust. We resolve the full path via `$HOME/Library/Keychains/login.keychain-db`.
+
+**User experience note:** `security add-trusted-cert` targeting the login keychain will trigger a macOS Keychain Access password dialog asking the user to confirm trusting the certificate. This is a one-time interactive prompt (not sudo) and is the same behavior as mkcert. The `security` command exits with status 0 after the user approves or non-zero if they cancel.
+
+**Why keep the function name `trust_ca_in_system`:** On Linux, this function still targets system-level CA trust (`/usr/local/share/ca-certificates`). Renaming would touch more code for no functional benefit. The doc comment will be updated to describe the per-platform behavior accurately.
+
+## Scope of User-Facing Message Changes in init.rs
+
+There are **five** places in `src/commands/init.rs` that reference the system keychain or sudo for macOS trust:
+1. Line 363: `"trusting CA in system keychain (requires sudo)..."` — change to `"trusting CA in login keychain..."`
+2. Line 365: `"CA trusted in system keychain"` — change to `"CA trusted in login keychain"`
+3. Line 369: `"run manually with sudo:"` — change to `"run manually:"` (the Linux fallback command includes `sudo` inline; the macOS command no longer needs it)
+4. Line 372: fallback manual command with `sudo security add-trusted-cert ... /Library/Keychains/System.keychain` — update to new command (no sudo)
+5. Line 548-549: "Next steps" fallback with same manual command — update to new command (no sudo)
+
+All five must be updated consistently.
+
+---
+
+### Task 1: Add `login_keychain_path` helper with TDD in cert.rs
+
+**Files:**
+- Modify: `src/proxy/cert.rs`
+
+**Step 1: Write the failing test**
+
+Add to the existing `#[cfg(test)] mod tests` block at the end of `src/proxy/cert.rs`:
+
+```rust
+#[cfg(target_os = "macos")]
+#[test]
+fn login_keychain_path_points_to_login_keychain() {
+    let path = super::login_keychain_path().unwrap();
+    assert!(path.to_string_lossy().ends_with("Library/Keychains/login.keychain-db"));
+    assert!(path.is_absolute(), "path should be absolute: {}", path.display());
+}
+```
+
+**Step 2: Run the test to verify it fails**
+
+```bash
+cargo test --lib -- cert::tests::login_keychain_path
+```
+
+Expected: FAIL — compile error because `login_keychain_path` does not exist yet.
+
+**Step 3: Add the helper function**
+
+Add just above the existing `trust_ca_in_system` function (before line 135 in `src/proxy/cert.rs`):
+
+```rust
+/// Return the path to the current user's login keychain on macOS.
+#[cfg(target_os = "macos")]
+fn login_keychain_path() -> Result<std::path::PathBuf> {
+    let home = dirs::home_dir().context("could not determine home directory")?;
+    Ok(home.join("Library/Keychains/login.keychain-db"))
+}
+```
+
+**Step 4: Run the test to verify it passes**
+
+```bash
+cargo test --lib -- cert::tests::login_keychain_path
+```
+
+Expected: PASS.
+
+**Step 5: Commit**
+
+```bash
+git add src/proxy/cert.rs
+git commit -m "feat: add login_keychain_path helper for macOS CA trust"
+```
+
+---
+
+### Task 2: Update `trust_ca_in_system` to use login keychain
+
+**Files:**
+- Modify: `src/proxy/cert.rs`
+
+**Step 1: Update the doc comment on `trust_ca_in_system`**
+
+Replace the doc comment block (lines 135-138 of `src/proxy/cert.rs`):
+
+```rust
+/// Trust the CA certificate in the system keychain.
+///
+/// Supports macOS (security add-trusted-cert) and Linux (update-ca-certificates).
+/// Warns on other platforms where automatic trust is not implemented.
+```
+
+With:
+
+```rust
+/// Trust the CA certificate in the OS certificate store.
+///
+/// On macOS, adds to the user's login keychain (no sudo required).
+/// On Linux, copies to /usr/local/share/ca-certificates and runs update-ca-certificates.
+/// Warns on other platforms where automatic trust is not implemented.
+```
+
+**Step 2: Replace the macOS block in `trust_ca_in_system`**
+
+Replace the `#[cfg(target_os = "macos")]` block inside `trust_ca_in_system` (the block starting at line 140 with `#[cfg(target_os = "macos")]` and ending at line 160 with `return Ok(());` and `}`) with:
+
+```rust
+#[cfg(target_os = "macos")]
+{
+    let keychain = login_keychain_path()?;
+    let status = std::process::Command::new("security")
+        .args([
+            "add-trusted-cert",
+            "-r",
+            "trustRoot",
+            "-k",
+        ])
+        .arg(&keychain)
+        .arg(ca_cert_path)
+        .status()
+        .context("failed to run security command")?;
+
+    if !status.success() {
+        anyhow::bail!(
+            "failed to trust CA cert in login keychain ({})",
+            keychain.display()
+        );
+    }
+
+    return Ok(());
+}
+```
+
+Changes from current code:
+- Removed `-d` flag (no admin trust domain)
+- Changed keychain from `/Library/Keychains/System.keychain` to dynamic login keychain path
+- Updated error message (no more "may need sudo")
+
+**Step 3: Verify compilation and existing tests still pass**
+
+```bash
+cargo test --lib -- cert::tests
+```
+
+Expected: all cert tests pass.
+
+**Step 4: Commit**
+
+```bash
+git add src/proxy/cert.rs
+git commit -m "fix: use login keychain for macOS CA trust (no sudo required)"
+```
+
+---
+
+### Task 3: Update user-facing messages in init.rs
+
+**Files:**
+- Modify: `src/commands/init.rs`
+
+**Step 1: Update the trust attempt message (line 363)**
+
+Change:
+```rust
+eprintln!("trusting CA in system keychain (requires sudo)...");
+```
+To:
+```rust
+eprintln!("trusting CA in login keychain...");
+```
+
+**Step 2: Update the success message (line 365)**
+
+Change:
+```rust
+Ok(()) => eprintln!("{} CA trusted in system keychain", "ok:".green()),
+```
+To:
+```rust
+Ok(()) => eprintln!("{} CA trusted in login keychain", "ok:".green()),
+```
+
+**Step 3: Update the fallback "run manually" text (line 369)**
+
+Change:
+```rust
+eprintln!("  run manually with sudo:");
+```
+To:
+```rust
+eprintln!("  run manually:");
+```
+
+Note: On Linux the fallback command still uses `sudo`, but the "run manually:" text itself doesn't need to say "with sudo" since the individual commands below show `sudo` where needed.
+
+**Step 4: Update the fallback manual command (lines 370-374)**
+
+Change:
+```rust
+#[cfg(target_os = "macos")]
+eprintln!(
+    "    sudo security add-trusted-cert -d -r trustRoot -k /Library/Keychains/System.keychain {}",
+    ca_cert_path.display()
+);
+```
+To:
+```rust
+#[cfg(target_os = "macos")]
+eprintln!(
+    "    security add-trusted-cert -r trustRoot -k ~/Library/Keychains/login.keychain-db {}",
+    ca_cert_path.display()
+);
+```
+
+**Step 5: Update the "Next steps" fallback (lines 547-550)**
+
+Change:
+```rust
+#[cfg(target_os = "macos")]
+eprintln!(
+    "     sudo security add-trusted-cert -d -r trustRoot -k /Library/Keychains/System.keychain {}",
+    ca_cert_path.display()
+);
+```
+To:
+```rust
+#[cfg(target_os = "macos")]
+eprintln!(
+    "     security add-trusted-cert -r trustRoot -k ~/Library/Keychains/login.keychain-db {}",
+    ca_cert_path.display()
+);
+```
+
+**Step 6: Verify compilation**
+
+```bash
+cargo clippy --all-targets -- -D warnings
+```
+
+Expected: clean build, no warnings.
+
+**Step 7: Commit**
+
+```bash
+git add src/commands/init.rs
+git commit -m "fix: update init messages to reflect login keychain trust (no sudo)"
+```
+
+---
+
+### Task 4: Run full test suite and verify
+
+**Files:** (no modifications)
+
+**Step 1: Run cargo fmt check**
+
+```bash
+cargo fmt -- --check
+```
+
+Expected: no formatting violations.
+
+**Step 2: Run full check**
+
+```bash
+cargo clippy --all-targets -- -D warnings && cargo test
+```
+
+Expected: all tests pass, no warnings.

docs/spec.md

@@ -25,7 +25,7 @@ devproxy init --domain mysite.dev
 ```
 
 - Generates a local CA and wildcard TLS cert using `rcgen` (pure Rust, no mkcert)
-- Trusts the CA in the system keychain (`security` on macOS, `update-ca-certificates` on Linux)
+- Trusts the CA in the OS certificate store (login keychain on macOS, `update-ca-certificates` on Linux)
 - Spawns the proxy daemon in the background
 - Prints instructions for wildcard DNS (dnsmasq or `/etc/hosts`)
 

skills/setup/SKILL.md

@@ -70,7 +70,7 @@ devproxy init --domain <chosen-domain>
 
 This will:
 - Generate a local CA and wildcard TLS certificate
-- Trust the CA in the system keychain (may prompt for password)
+- Trust the CA in the login keychain on macOS (prompts for keychain password once)
 - Install a LaunchAgent (macOS) or systemd unit (Linux) to run the daemon
 - Start the daemon via socket activation (no sudo needed for the daemon itself)