From 83dc35effafbeb9b0b5b53b40a72573a6a76697a Mon Sep 17 00:00:00 2001 From: Chris Fenton Date: Tue, 10 Mar 2026 08:01:05 -0700 Subject: [PATCH 1/8] docs: add implementation plan for login keychain CA trust fix Co-Authored-By: Claude Opus 4.6 --- .../2026-03-10-fix-ca-trust-login-keychain.md | 205 ++++++++++++++++++ 1 file changed, 205 insertions(+) create mode 100644 docs/plans/2026-03-10-fix-ca-trust-login-keychain.md diff --git a/docs/plans/2026-03-10-fix-ca-trust-login-keychain.md b/docs/plans/2026-03-10-fix-ca-trust-login-keychain.md new file mode 100644 index 0000000..6db2fda --- /dev/null +++ b/docs/plans/2026-03-10-fix-ca-trust-login-keychain.md @@ -0,0 +1,205 @@ +# 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. + +**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 +``` + +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 +``` + +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..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`. + +## Scope of User-Facing Message Changes + +The `init.rs` file has three places that reference the system keychain: +1. Line 363: `"trusting CA in system keychain (requires sudo)..."` — change to `"trusting CA in login keychain..."` +2. Line 372: fallback manual command with `sudo security add-trusted-cert ... /Library/Keychains/System.keychain` — update to new command (no sudo) +3. Line 549: "Next steps" fallback with same manual command — update to new command (no sudo) + +All three must be updated consistently. + +--- + +### Task 1: Change macOS trust to use login keychain in cert.rs + +**Files:** +- Modify: `src/proxy/cert.rs` + +**Step 1: Write a unit test that validates the security command arguments** + +The actual `security add-trusted-cert` call is a side-effecting system command and cannot be tested in CI. However, we can extract the logic that builds the keychain path and test that. Add a helper function `login_keychain_path()` and a test for it. + +Add to `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 { + let home = dirs::home_dir().context("could not determine home directory")?; + Ok(home.join("Library/Keychains/login.keychain-db")) +} +``` + +Add test in the existing `#[cfg(test)] mod tests`: + +```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.to_string_lossy().starts_with("/Users/")); +} +``` + +**Step 2: Update trust_ca_in_system to use login keychain** + +Replace the macOS block in `trust_ca_in_system` (lines 140-160) 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(()); +} +``` + +Key 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" since login keychain doesn't need it) + +**Step 3: Verify compilation and tests** + +```bash +cargo test --lib -- cert::tests +``` + +Expected: all cert tests pass, including the new `login_keychain_path_points_to_login_keychain` test. + +--- + +### Task 2: 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 fallback manual command (line 370-374)** + +Change the `#[cfg(target_os = "macos")]` fallback instructions from: +```rust +eprintln!( + " sudo security add-trusted-cert -d -r trustRoot -k /Library/Keychains/System.keychain {}", + ca_cert_path.display() +); +``` +To: +```rust +eprintln!( + " security add-trusted-cert -r trustRoot -k ~/Library/Keychains/login.keychain-db {}", + ca_cert_path.display() +); +``` + +**Step 3: Update the "Next steps" fallback (line 547-550)** + +Change the `#[cfg(target_os = "macos")]` next-steps instructions from: +```rust +eprintln!( + " sudo security add-trusted-cert -d -r trustRoot -k /Library/Keychains/System.keychain {}", + ca_cert_path.display() +); +``` +To: +```rust +eprintln!( + " security add-trusted-cert -r trustRoot -k ~/Library/Keychains/login.keychain-db {}", + ca_cert_path.display() +); +``` + +**Step 4: Verify compilation** + +```bash +cargo clippy --all-targets -- -D warnings +``` + +Expected: clean build, no warnings. + +--- + +### Task 3: 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. From 0e67a214d340a4de001ab97bb52d6990436c584a Mon Sep 17 00:00:00 2001 From: Chris Fenton Date: Tue, 10 Mar 2026 08:04:29 -0700 Subject: [PATCH 2/8] =?UTF-8?q?docs:=20improve=20CA=20trust=20plan=20?= =?UTF-8?q?=E2=80=94=20add=20missing=20message=20updates,=20commit=20steps?= =?UTF-8?q?,=20naming=20rationale?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-Authored-By: Claude Opus 4.6 --- .../2026-03-10-fix-ca-trust-login-keychain.md | 118 ++++++++++++++---- 1 file changed, 93 insertions(+), 25 deletions(-) diff --git a/docs/plans/2026-03-10-fix-ca-trust-login-keychain.md b/docs/plans/2026-03-10-fix-ca-trust-login-keychain.md index 6db2fda..821ed55 100644 --- a/docs/plans/2026-03-10-fix-ca-trust-login-keychain.md +++ b/docs/plans/2026-03-10-fix-ca-trust-login-keychain.md @@ -4,7 +4,7 @@ **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. +**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 @@ -35,27 +35,29 @@ Key differences from the current code: **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`. -## Scope of User-Facing Message Changes +**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 to something like `trust_ca` is tempting but would touch more code for no functional benefit. The doc comment already describes per-platform behavior. Keep the name as-is. -The `init.rs` file has three places that reference the system keychain: +## 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 372: fallback manual command with `sudo security add-trusted-cert ... /Library/Keychains/System.keychain` — update to new command (no sudo) -3. Line 549: "Next steps" fallback with same manual command — update to new command (no sudo) +2. Line 365: `"CA trusted in system keychain"` — change to `"CA trusted in login keychain"` +3. Line 369: `"run manually with sudo:"` — change to platform-conditional message (macOS no longer needs sudo; Linux still does) +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 three must be updated consistently. +All five must be updated consistently. --- -### Task 1: Change macOS trust to use login keychain in cert.rs +### Task 1: Add `login_keychain_path` helper and test in cert.rs **Files:** - Modify: `src/proxy/cert.rs` -**Step 1: Write a unit test that validates the security command arguments** - -The actual `security add-trusted-cert` call is a side-effecting system command and cannot be tested in CI. However, we can extract the logic that builds the keychain path and test that. Add a helper function `login_keychain_path()` and a test for it. +**Step 1: Add the helper function** -Add to `src/proxy/cert.rs`: +Add just above the existing `trust_ca_in_system` function (before line 135): ```rust /// Return the path to the current user's login keychain on macOS. @@ -66,7 +68,9 @@ fn login_keychain_path() -> Result { } ``` -Add test in the existing `#[cfg(test)] mod tests`: +**Step 2: Add a unit test** + +Add to the existing `#[cfg(test)] mod tests` block at the end of the file: ```rust #[cfg(target_os = "macos")] @@ -78,9 +82,31 @@ fn login_keychain_path_points_to_login_keychain() { } ``` -**Step 2: Update trust_ca_in_system to use login keychain** +**Step 3: Run the test** + +```bash +cargo test --lib -- cert::tests::login_keychain_path +``` + +Expected: PASS (on macOS). + +**Step 4: Commit** + +```bash +git add src/proxy/cert.rs +git commit -m "feat: add login_keychain_path helper for macOS CA trust" +``` + +--- -Replace the macOS block in `trust_ca_in_system` (lines 140-160) with: +### Task 2: Update `trust_ca_in_system` to use login keychain + +**Files:** +- Modify: `src/proxy/cert.rs` + +**Step 1: Replace the macOS block in trust_ca_in_system** + +Replace lines 140-160 (the `#[cfg(target_os = "macos")]` block inside `trust_ca_in_system`) with: ```rust #[cfg(target_os = "macos")] @@ -109,22 +135,29 @@ Replace the macOS block in `trust_ca_in_system` (lines 140-160) with: } ``` -Key changes from current code: +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" since login keychain doesn't need it) +- Updated error message (no more "may need sudo") -**Step 3: Verify compilation and tests** +**Step 2: Verify compilation and existing tests still pass** ```bash cargo test --lib -- cert::tests ``` -Expected: all cert tests pass, including the new `login_keychain_path_points_to_login_keychain` test. +Expected: all cert tests pass. + +**Step 3: Commit** + +```bash +git add src/proxy/cert.rs +git commit -m "fix: use login keychain for macOS CA trust (no sudo required)" +``` --- -### Task 2: Update user-facing messages in init.rs +### Task 3: Update user-facing messages in init.rs **Files:** - Modify: `src/commands/init.rs` @@ -140,10 +173,35 @@ To: eprintln!("trusting CA in login keychain..."); ``` -**Step 2: Update the fallback manual command (line 370-374)** +**Step 2: Update the success message (line 365)** -Change the `#[cfg(target_os = "macos")]` fallback instructions from: +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() @@ -151,16 +209,18 @@ eprintln!( ``` To: ```rust +#[cfg(target_os = "macos")] eprintln!( " security add-trusted-cert -r trustRoot -k ~/Library/Keychains/login.keychain-db {}", ca_cert_path.display() ); ``` -**Step 3: Update the "Next steps" fallback (line 547-550)** +**Step 5: Update the "Next steps" fallback (lines 547-550)** -Change the `#[cfg(target_os = "macos")]` next-steps instructions from: +Change: ```rust +#[cfg(target_os = "macos")] eprintln!( " sudo security add-trusted-cert -d -r trustRoot -k /Library/Keychains/System.keychain {}", ca_cert_path.display() @@ -168,13 +228,14 @@ eprintln!( ``` To: ```rust +#[cfg(target_os = "macos")] eprintln!( " security add-trusted-cert -r trustRoot -k ~/Library/Keychains/login.keychain-db {}", ca_cert_path.display() ); ``` -**Step 4: Verify compilation** +**Step 6: Verify compilation** ```bash cargo clippy --all-targets -- -D warnings @@ -182,9 +243,16 @@ 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 3: Run full test suite and verify +### Task 4: Run full test suite and verify **Files:** (no modifications) From 92c1ca740479b6d383b5ac3321e24ba8621f879e Mon Sep 17 00:00:00 2001 From: Chris Fenton Date: Tue, 10 Mar 2026 08:08:31 -0700 Subject: [PATCH 3/8] docs: improve CA trust login keychain plan for TDD compliance Reorder Task 1 to test-first (write failing test, verify compile error, then implement). Add doc comment update for trust_ca_in_system to accurately describe per-platform behavior after the fix. Co-Authored-By: Claude Opus 4.6 --- .../2026-03-10-fix-ca-trust-login-keychain.md | 75 +++++++++++++------ 1 file changed, 52 insertions(+), 23 deletions(-) diff --git a/docs/plans/2026-03-10-fix-ca-trust-login-keychain.md b/docs/plans/2026-03-10-fix-ca-trust-login-keychain.md index 821ed55..feb6382 100644 --- a/docs/plans/2026-03-10-fix-ca-trust-login-keychain.md +++ b/docs/plans/2026-03-10-fix-ca-trust-login-keychain.md @@ -35,14 +35,14 @@ Key differences from the current code: **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`. -**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 to something like `trust_ca` is tempting but would touch more code for no functional benefit. The doc comment already describes per-platform behavior. Keep the name as-is. +**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 platform-conditional message (macOS no longer needs sudo; Linux still does) +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) @@ -50,47 +50,55 @@ All five must be updated consistently. --- -### Task 1: Add `login_keychain_path` helper and test in cert.rs +### Task 1: Add `login_keychain_path` helper with TDD in cert.rs **Files:** - Modify: `src/proxy/cert.rs` -**Step 1: Add the helper function** +**Step 1: Write the failing test** -Add just above the existing `trust_ca_in_system` function (before line 135): +Add to the existing `#[cfg(test)] mod tests` block at the end of `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 { - let home = dirs::home_dir().context("could not determine home directory")?; - Ok(home.join("Library/Keychains/login.keychain-db")) +#[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.to_string_lossy().starts_with("/Users/")); } ``` -**Step 2: Add a unit test** +**Step 2: Run the test to verify it fails** -Add to the existing `#[cfg(test)] mod tests` block at the end of the file: +```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")] -#[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.to_string_lossy().starts_with("/Users/")); +fn login_keychain_path() -> Result { + let home = dirs::home_dir().context("could not determine home directory")?; + Ok(home.join("Library/Keychains/login.keychain-db")) } ``` -**Step 3: Run the test** +**Step 4: Run the test to verify it passes** ```bash cargo test --lib -- cert::tests::login_keychain_path ``` -Expected: PASS (on macOS). +Expected: PASS. -**Step 4: Commit** +**Step 5: Commit** ```bash git add src/proxy/cert.rs @@ -104,9 +112,30 @@ git commit -m "feat: add login_keychain_path helper for macOS CA trust" **Files:** - Modify: `src/proxy/cert.rs` -**Step 1: Replace the macOS block in trust_ca_in_system** +**Step 1: Update the doc comment on `trust_ca_in_system`** -Replace lines 140-160 (the `#[cfg(target_os = "macos")]` block inside `trust_ca_in_system`) with: +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")] @@ -140,7 +169,7 @@ Changes from current code: - Changed keychain from `/Library/Keychains/System.keychain` to dynamic login keychain path - Updated error message (no more "may need sudo") -**Step 2: Verify compilation and existing tests still pass** +**Step 3: Verify compilation and existing tests still pass** ```bash cargo test --lib -- cert::tests @@ -148,7 +177,7 @@ cargo test --lib -- cert::tests Expected: all cert tests pass. -**Step 3: Commit** +**Step 4: Commit** ```bash git add src/proxy/cert.rs From 6d269e05f5bb15f6eb9e5c7dde4cc1c8f7a434b3 Mon Sep 17 00:00:00 2001 From: Chris Fenton Date: Tue, 10 Mar 2026 08:11:15 -0700 Subject: [PATCH 4/8] docs: refine CA trust plan with UX note and robust test assertion Add documentation about the macOS Keychain Access password dialog that users will see (matches mkcert behavior). Replace fragile starts_with "/Users/" test assertion with is_absolute() check. Co-Authored-By: Claude Opus 4.6 --- docs/plans/2026-03-10-fix-ca-trust-login-keychain.md | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/docs/plans/2026-03-10-fix-ca-trust-login-keychain.md b/docs/plans/2026-03-10-fix-ca-trust-login-keychain.md index feb6382..c9a4d1e 100644 --- a/docs/plans/2026-03-10-fix-ca-trust-login-keychain.md +++ b/docs/plans/2026-03-10-fix-ca-trust-login-keychain.md @@ -35,6 +35,8 @@ Key differences from the current code: **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 @@ -65,7 +67,7 @@ Add to the existing `#[cfg(test)] mod tests` block at the end of `src/proxy/cert 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.to_string_lossy().starts_with("/Users/")); + assert!(path.is_absolute(), "path should be absolute: {}", path.display()); } ``` From 7e003eea5454bb8c75c662e43aceac0ad4407f70 Mon Sep 17 00:00:00 2001 From: Chris Fenton Date: Tue, 10 Mar 2026 08:16:33 -0700 Subject: [PATCH 5/8] test: add test plan for CA trust login keychain fix Co-Authored-By: Claude Opus 4.6 --- ...0-fix-ca-trust-login-keychain-test-plan.md | 70 +++++++++++++++++++ 1 file changed, 70 insertions(+) create mode 100644 docs/plans/2026-03-10-fix-ca-trust-login-keychain-test-plan.md diff --git a/docs/plans/2026-03-10-fix-ca-trust-login-keychain-test-plan.md b/docs/plans/2026-03-10-fix-ca-trust-login-keychain-test-plan.md new file mode 100644 index 0000000..61c6170 --- /dev/null +++ b/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 ` 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) From 0c61df5d387d91c923417634741ff310d6b84730 Mon Sep 17 00:00:00 2001 From: Chris Fenton Date: Tue, 10 Mar 2026 08:21:13 -0700 Subject: [PATCH 6/8] fix: use login keychain for macOS CA trust instead of system keychain The system keychain requires sudo, which always fails since devproxy runs as the current user via socket activation. Switch to the login keychain (no sudo, just a one-time Keychain Access dialog) and update all user-facing messages in init to match. Co-Authored-By: Claude Opus 4.6 --- src/commands/init.rs | 23 +++++---- src/proxy/cert.rs | 109 ++++++++++++++++++++++++++++++++++++++----- 2 files changed, 113 insertions(+), 19 deletions(-) diff --git a/src/commands/init.rs b/src/commands/init.rs index fa26fc0..4f07e9b 100644 --- a/src/commands/init.rs +++ b/src/commands/init.rs @@ -1,6 +1,6 @@ use crate::config::Config; use crate::proxy::cert; -use anyhow::{Context, Result, bail}; +use anyhow::{bail, Context, Result}; use colored::Colorize; use std::time::Duration; @@ -56,7 +56,10 @@ pub fn is_devproxy_process(pid: i32) -> bool { // On Linux, read /proc//exe symlink. let exe = std::fs::read_link(format!("/proc/{pid}/exe")); match exe { - Ok(path) => path.file_name().map(|n| n == "devproxy" || n == "devproxy-daemon").unwrap_or(false), + Ok(path) => path + .file_name() + .map(|n| n == "devproxy" || n == "devproxy-daemon") + .unwrap_or(false), Err(_) => false, } } @@ -328,7 +331,11 @@ pub fn install_daemon_binary(src: &std::path::Path, dest: &std::path::Path) -> R super::update::prepare_binary(dest)?; - eprintln!("{} daemon binary installed at {}", "ok:".green(), dest.display()); + eprintln!( + "{} daemon binary installed at {}", + "ok:".green(), + dest.display() + ); Ok(()) } @@ -360,16 +367,16 @@ pub fn run(domain: &str, port: u16, no_daemon: bool) -> Result<()> { eprintln!("{} CA certificate generated", "ok:".green()); // Trust the CA - eprintln!("trusting CA in system keychain (requires sudo)..."); + eprintln!("trusting CA in login keychain..."); match cert::trust_ca_in_system(&ca_cert_path) { - Ok(()) => eprintln!("{} CA trusted in system keychain", "ok:".green()), + Ok(()) => eprintln!("{} CA trusted in login keychain", "ok:".green()), Err(e) => { ca_trust_needed = true; eprintln!("{} could not trust CA automatically: {e}", "warn:".yellow()); - eprintln!(" run manually with sudo:"); + eprintln!(" run manually:"); #[cfg(target_os = "macos")] eprintln!( - " sudo security add-trusted-cert -d -r trustRoot -k /Library/Keychains/System.keychain {}", + " security add-trusted-cert -r trustRoot -k ~/Library/Keychains/login.keychain-db {}", ca_cert_path.display() ); #[cfg(target_os = "linux")] @@ -546,7 +553,7 @@ pub fn run(domain: &str, port: u16, no_daemon: bool) -> Result<()> { ); #[cfg(target_os = "macos")] eprintln!( - " sudo security add-trusted-cert -d -r trustRoot -k /Library/Keychains/System.keychain {}", + " security add-trusted-cert -r trustRoot -k ~/Library/Keychains/login.keychain-db {}", ca_cert_path.display() ); #[cfg(target_os = "linux")] diff --git a/src/proxy/cert.rs b/src/proxy/cert.rs index 93a2a13..5f7a999 100644 --- a/src/proxy/cert.rs +++ b/src/proxy/cert.rs @@ -132,28 +132,34 @@ pub fn load_tls_config(cert_path: &Path, key_path: &Path) -> Result Result { + let home = dirs::home_dir().context("could not determine home directory")?; + Ok(home.join("Library/Keychains/login.keychain-db")) +} + +/// Trust the CA certificate in the OS certificate store. /// -/// Supports macOS (security add-trusted-cert) and Linux (update-ca-certificates). +/// 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. pub fn trust_ca_in_system(ca_cert_path: &Path) -> Result<()> { #[cfg(target_os = "macos")] { + let keychain = login_keychain_path()?; let status = std::process::Command::new("security") - .args([ - "add-trusted-cert", - "-d", - "-r", - "trustRoot", - "-k", - "/Library/Keychains/System.keychain", - ]) + .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. You may need to run with sudo."); + anyhow::bail!( + "failed to trust CA cert in login keychain ({})", + keychain.display() + ); } return Ok(()); @@ -204,6 +210,20 @@ mod tests { assert!(key_pem.contains("BEGIN PRIVATE KEY")); } + #[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() + ); + } + #[test] fn tls_config_loads_from_generated_certs() { let (ca_cert, ca_key) = generate_ca().unwrap(); @@ -218,4 +238,71 @@ mod tests { let result = load_tls_config(&cert_path, &key_path); assert!(result.is_ok(), "TLS config should load: {:?}", result.err()); } + + /// Functional test: add a CA cert to the login keychain, verify it, then clean up. + /// + /// Marked `#[ignore]` because it touches the real login keychain and triggers + /// a macOS Keychain Access password dialog. Run manually with: + /// cargo test -- cert::tests::trust_ca_login_keychain_roundtrip --ignored + #[cfg(target_os = "macos")] + #[test] + #[ignore] + fn trust_ca_login_keychain_roundtrip() { + use std::path::PathBuf; + + /// Guard that removes the test certificate from the login keychain on drop. + struct KeychainCleanup { + cert_path: PathBuf, + keychain: PathBuf, + } + + impl Drop for KeychainCleanup { + fn drop(&mut self) { + // Remove trust setting + let _ = std::process::Command::new("security") + .args(["remove-trusted-cert", "-d"]) + .arg(&self.cert_path) + .status(); + + // Remove the certificate itself + let _ = std::process::Command::new("security") + .args(["delete-certificate", "-c", "devproxy Local CA"]) + .arg(&self.keychain) + .status(); + } + } + + // Generate a fresh CA cert + let (ca_cert_pem, _ca_key_pem) = generate_ca().unwrap(); + + // Write to a temp file + let dir = tempfile::tempdir().unwrap(); + let cert_path = dir.path().join("test-ca.pem"); + std::fs::write(&cert_path, &ca_cert_pem).unwrap(); + + // Resolve login keychain path + let home = dirs::home_dir().expect("could not determine home directory"); + let keychain = home.join("Library/Keychains/login.keychain-db"); + + // Set up cleanup guard before calling trust (runs even on panic) + let _cleanup = KeychainCleanup { + cert_path: cert_path.clone(), + keychain: keychain.clone(), + }; + + // Trust the cert in the login keychain + super::trust_ca_in_system(&cert_path).unwrap(); + + // Verify the cert is findable in the login keychain + let output = std::process::Command::new("security") + .args(["find-certificate", "-c", "devproxy Local CA", "-a"]) + .arg(&keychain) + .output() + .expect("failed to run security find-certificate"); + + assert!( + output.status.success(), + "certificate should be present in login keychain after trust_ca_in_system" + ); + } } From d7f505e65fb80996d91a82d7b26e66bbb410d8bd Mon Sep 17 00:00:00 2001 From: Chris Fenton Date: Tue, 10 Mar 2026 08:27:11 -0700 Subject: [PATCH 7/8] fix: remove -d flag from test cleanup and revert collateral formatting - Fix test cleanup to use `remove-trusted-cert` without `-d` flag, matching the user trust store where the cert was added (not admin store) - Revert unrelated formatting changes in init.rs to keep diff surgical Co-Authored-By: Claude Opus 4.6 --- src/commands/init.rs | 13 +++---------- src/proxy/cert.rs | 4 ++-- 2 files changed, 5 insertions(+), 12 deletions(-) diff --git a/src/commands/init.rs b/src/commands/init.rs index 4f07e9b..30e3137 100644 --- a/src/commands/init.rs +++ b/src/commands/init.rs @@ -1,6 +1,6 @@ use crate::config::Config; use crate::proxy::cert; -use anyhow::{bail, Context, Result}; +use anyhow::{Context, Result, bail}; use colored::Colorize; use std::time::Duration; @@ -56,10 +56,7 @@ pub fn is_devproxy_process(pid: i32) -> bool { // On Linux, read /proc//exe symlink. let exe = std::fs::read_link(format!("/proc/{pid}/exe")); match exe { - Ok(path) => path - .file_name() - .map(|n| n == "devproxy" || n == "devproxy-daemon") - .unwrap_or(false), + Ok(path) => path.file_name().map(|n| n == "devproxy" || n == "devproxy-daemon").unwrap_or(false), Err(_) => false, } } @@ -331,11 +328,7 @@ pub fn install_daemon_binary(src: &std::path::Path, dest: &std::path::Path) -> R super::update::prepare_binary(dest)?; - eprintln!( - "{} daemon binary installed at {}", - "ok:".green(), - dest.display() - ); + eprintln!("{} daemon binary installed at {}", "ok:".green(), dest.display()); Ok(()) } diff --git a/src/proxy/cert.rs b/src/proxy/cert.rs index 5f7a999..fa5844d 100644 --- a/src/proxy/cert.rs +++ b/src/proxy/cert.rs @@ -258,9 +258,9 @@ mod tests { impl Drop for KeychainCleanup { fn drop(&mut self) { - // Remove trust setting + // Remove trust setting from the user trust store let _ = std::process::Command::new("security") - .args(["remove-trusted-cert", "-d"]) + .args(["remove-trusted-cert"]) .arg(&self.cert_path) .status(); From 29861f55933faef87b711f9dc4f0e9d357b84789 Mon Sep 17 00:00:00 2001 From: Chris Fenton Date: Tue, 10 Mar 2026 08:33:03 -0700 Subject: [PATCH 8/8] fix: update docs/skills for login keychain, harden test isolation - Update docs/spec.md and skills/setup/SKILL.md to reference login keychain instead of system keychain (finding #1) - Use distinct CN "devproxy Test CA" in roundtrip test to avoid colliding with real devproxy CAs in the login keychain (finding #3) - Update e2e test to accept trust success (no longer requires sudo fallback output since login keychain trust succeeds without sudo) Co-Authored-By: Claude Opus 4.6 --- docs/spec.md | 2 +- skills/setup/SKILL.md | 2 +- src/proxy/cert.rs | 26 +++++++++++++++++++++----- tests/e2e.rs | 13 +++++++------ 4 files changed, 30 insertions(+), 13 deletions(-) diff --git a/docs/spec.md b/docs/spec.md index 137d24b..a34b3ab 100644 --- a/docs/spec.md +++ b/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`) diff --git a/skills/setup/SKILL.md b/skills/setup/SKILL.md index cd66c26..3d3acb2 100644 --- a/skills/setup/SKILL.md +++ b/skills/setup/SKILL.md @@ -70,7 +70,7 @@ devproxy init --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) diff --git a/src/proxy/cert.rs b/src/proxy/cert.rs index fa5844d..3ece712 100644 --- a/src/proxy/cert.rs +++ b/src/proxy/cert.rs @@ -250,6 +250,8 @@ mod tests { fn trust_ca_login_keychain_roundtrip() { use std::path::PathBuf; + const TEST_CN: &str = "devproxy Test CA"; + /// Guard that removes the test certificate from the login keychain on drop. struct KeychainCleanup { cert_path: PathBuf, @@ -266,19 +268,33 @@ mod tests { // Remove the certificate itself let _ = std::process::Command::new("security") - .args(["delete-certificate", "-c", "devproxy Local CA"]) + .args(["delete-certificate", "-c", TEST_CN]) .arg(&self.keychain) .status(); } } - // Generate a fresh CA cert - let (ca_cert_pem, _ca_key_pem) = generate_ca().unwrap(); + // Generate a CA cert with a distinct CN to avoid colliding with real devproxy CAs + let mut params = CertificateParams::default(); + params.is_ca = IsCa::Ca(BasicConstraints::Unconstrained); + params + .distinguished_name + .push(DnType::CommonName, TEST_CN); + params + .distinguished_name + .push(DnType::OrganizationName, "devproxy"); + params.key_usages = vec![KeyUsagePurpose::KeyCertSign, KeyUsagePurpose::CrlSign]; + params.not_before = + time::OffsetDateTime::now_utc() - std::time::Duration::from_secs(3600); + params.not_after = + time::OffsetDateTime::now_utc() + std::time::Duration::from_secs(3600); + let key_pair = KeyPair::generate().unwrap(); + let cert = params.self_signed(&key_pair).unwrap(); // Write to a temp file let dir = tempfile::tempdir().unwrap(); let cert_path = dir.path().join("test-ca.pem"); - std::fs::write(&cert_path, &ca_cert_pem).unwrap(); + std::fs::write(&cert_path, cert.pem()).unwrap(); // Resolve login keychain path let home = dirs::home_dir().expect("could not determine home directory"); @@ -295,7 +311,7 @@ mod tests { // Verify the cert is findable in the login keychain let output = std::process::Command::new("security") - .args(["find-certificate", "-c", "devproxy Local CA", "-a"]) + .args(["find-certificate", "-c", TEST_CN, "-a"]) .arg(&keychain) .output() .expect("failed to run security find-certificate"); diff --git a/tests/e2e.rs b/tests/e2e.rs index 6f3586c..4139bd0 100644 --- a/tests/e2e.rs +++ b/tests/e2e.rs @@ -974,14 +974,15 @@ fn test_init_output_includes_ca_trust_path() { assert!(output.status.success(), "init should succeed"); let stderr = String::from_utf8_lossy(&output.stderr); - // The CA cert path should appear somewhere in the output -- either in - // the trust failure remediation instructions or (if trust succeeded) - // in the "Next steps" CA trust section. Since tests run without sudo, - // trust will fail and the path appears in the warning. + // With the login keychain, trust should succeed without sudo. + // If it does, the success message appears. If it fails for some + // other reason, the CA cert path appears in the fallback instructions. let ca_cert_path = config_dir.join("ca-cert.pem"); + let trust_succeeded = stderr.contains("CA trusted in login keychain"); + let path_shown = stderr.contains(&ca_cert_path.display().to_string()); assert!( - stderr.contains(&ca_cert_path.display().to_string()), - "init output should include CA cert path '{}': {stderr}", + trust_succeeded || path_shown, + "init output should show trust success or CA cert path '{}': {stderr}", ca_cert_path.display() );