fix: comprehensive audit — security, bugs, cleanup

AGENTS.md

@@ -9,51 +9,54 @@ AgentTap is a macOS (and later Linux) tool that captures AI agent API traces at
 ## Architecture
 
 ```
-Menu Bar App (SwiftUI) <-> Privileged Helper (root, XPC)
-         |                           |
-    DNS Resolver              pf firewall rules
-         |                    Custom CA management
-    Provider IPs              /etc/hosts
+Menu Bar App (ElectroBun/Bun) <-> Privileged Helper (root, Unix socket)
+         |                              |
+    DoH Resolver                  pf firewall rules
+         |                        /etc/hosts management
+    Real Provider IPs             DNS cache flush
          |
   Local MITM Proxy (port 8443)
          |
     TLS termination -> Log -> Forward to real API
-         |
+         |                    (via native forwarder + IP_BOUND_IF)
   Trace Storage (SQLite)
 ```
 
 ### Components
 
-1. **Menu Bar App** -- SwiftUI, user-space process. Toggle providers, view status, browse traces.
-2. **Privileged Helper** -- Runs as root via XPC. Manages pf rules, custom CA in system keychain, DNS cache flush.
-3. **DNS Resolver** -- Parallel resolution (dig + DoH + system) to track AI provider IPs. Refresh hourly.
+1. **Menu Bar App** -- ElectroBun (Bun-based), user-space process. Toggle providers, view status, browse traces.
+2. **Privileged Helper** -- Runs as root LaunchDaemon via Unix socket (JSON line protocol). Manages pf rules, /etc/hosts, DNS cache flush.
+3. **DNS Resolver** -- DoH resolver (Cloudflare + Google fallback) to resolve real provider IPs bypassing /etc/hosts.
 4. **MITM Proxy** -- Local TLS-terminating proxy. Decrypts, logs request/response, reassembles SSE streams, forwards to real API.
 5. **Trace Storage** -- SQLite database. Stores provider, model, full request/response, tokens, duration, source app.
 
-### Traffic Flow
+### Traffic Flow (DNS-based interception)
 
 1. App makes API call to `api.anthropic.com`
-2. DNS resolves normally to provider IP
-3. pf rule redirects traffic to that IP:443 -> 127.0.0.1:8443
-4. Local proxy presents cert signed by custom CA (installed in system keychain)
-5. Proxy decrypts, logs request, opens real TLS connection to provider
-6. Logs response, returns to app transparently
+2. System resolver hits `/etc/hosts` → sentinel IP (e.g. `127.0.1.1`)
+3. Single PF rdr rule: `127.0.1.0/24:443` → `127.0.0.1:8443`
+4. Transparent proxy extracts real domain from TLS SNI (ClientHello)
+5. Proxy presents leaf cert signed by custom CA (trusted in user keychain)
+6. Proxy decrypts, logs request, resolves real IP via DoH
+7. Native forwarder (`IP_BOUND_IF` on physical NIC) connects to real provider
+8. Logs response, returns to app transparently
 
 ### Reuse from VPN-Bypass
 
-- Two-process model (app + helper via XPC) -- same security pattern
-- Parallel DNS resolver engine
+- Two-process model (app + helper via Unix socket) -- same security pattern
 - Helper auto-install/update mechanism
-- Menu bar UI framework (SwiftUI)
 - Build pipeline (Makefile, GitHub Actions)
 
 ### New Components
 
-- MITM proxy engine (Swift Network.framework + Security.framework)
-- Custom CA lifecycle (generate, install in keychain, remove)
-- pf firewall rules (instead of VPN-Bypass's /sbin/route)
-- Trace storage (SQLite)
+- MITM proxy engine (Bun `node:tls` + per-domain cert servers)
+- Custom CA lifecycle (generate, install in user keychain, remove)
+- DNS-based interception (/etc/hosts sentinel IPs + PF rdr on lo0)
+- Native TCP forwarder (C, `IP_BOUND_IF` for upstream bypass)
+- DoH resolver (bypasses /etc/hosts for real IP resolution)
+- Trace storage (SQLite with write batching)
 - SSE stream reassembly
+- Cost estimation
 - Trace viewer UI
 
 ## Tech Stack
@@ -63,7 +66,7 @@ Menu Bar App (SwiftUI) <-> Privileged Helper (root, XPC)
 - **Platform**: macOS first, Linux planned
 - **Proxy**: Bun-native TLS interception
 - **Storage**: SQLite
-- **IPC**: Swift FFI via Bun's FFI for macOS system APIs (pf, keychain, etc.)
+- **IPC**: Unix domain socket (JSON line protocol) for privileged helper; Swift FFI via Bun's FFI for macOS system APIs (keychain, system info)
 - **Build**: ElectroBun build system
 
 ## Provider Domains
@@ -77,6 +80,8 @@ cohere: api.cohere.ai
 groq: api.groq.com
 bedrock: bedrock-runtime.{us-east-1,us-west-2,eu-west-1}.amazonaws.com
 cursor: api2.cursor.sh
+deepseek: api.deepseek.com
+xai: api.x.ai
 ```
 
 ## Development Guidelines

native/Forwarder/agenttap-fwd.c

@@ -152,6 +152,15 @@ static void handle_client(int client_fd) {
     if (read_line(client_fd, host, MAX_LINE) < 0) goto done;
     if (read_line(client_fd, port_str, MAX_LINE) < 0) goto done;
 
+    /* Restrict to port 443 — the forwarder should only relay HTTPS upstream */
+    {
+        int requested_port = atoi(port_str);
+        if (requested_port != 443) {
+            dprintf(client_fd, "ERR port %d not allowed (only 443)\n", requested_port);
+            goto done;
+        }
+    }
+
     /* Resolve host */
     struct addrinfo hints, *res;
     memset(&hints, 0, sizeof(hints));

native/Helper/HelperTool.swift

@@ -109,6 +109,18 @@ class HelperTool {
         }
     }
 
+    // MARK: - Orphan Cleanup
+
+    /// Called by the orphan detection timer in main.swift when the app
+    /// appears to have crashed while /etc/hosts entries are still active.
+    /// Removes hosts entries and flushes pf anchor directly.
+    func cleanupOrphanState() {
+        _ = removeHostsEntries(id: "orphan", params: [:])
+        _ = clearRules(id: "orphan", params: [
+            "anchorName": "com.apple/300.AgentTap",
+        ])
+    }
+
     // MARK: - PF Rules
 
     private func applyPfRules(id: String, params: [String: Any]) -> [String: Any] {

native/Helper/main.swift

@@ -36,8 +36,8 @@ guard bindResult == 0 else {
     exit(1)
 }
 
-// Socket permissions: root owner, staff group (GID 20), rw for owner+group
-// Without chown, socket stays root:wheel and non-root users can't connect
+// Socket permissions: root owner, staff group (GID 20), rw for owner+group.
+// verifyClient() further restricts to the active console user via getpeereid().
 chmod(socketPath, 0o660)
 chown(socketPath, 0, 20) // root:staff — allows normal macOS users to connect
 
@@ -51,6 +51,44 @@ guard listen(serverFD, 5) == 0 else {
 signal(SIGTERM) { _ in unlink(socketPath); exit(0) }
 signal(SIGINT) { _ in unlink(socketPath); exit(0) }
 
+// Orphan detection: if /etc/hosts has AgentTap entries but no app is connected
+// for 2 consecutive checks (120s), auto-clean to prevent traffic blackhole.
+var orphanMissCount = 0
+let orphanTimer = DispatchSource.makeTimerSource(queue: DispatchQueue.global())
+orphanTimer.schedule(deadline: .now() + 60, repeating: 60)
+orphanTimer.setEventHandler {
+    // Check if AgentTap block exists in /etc/hosts
+    guard let hosts = try? String(contentsOfFile: "/etc/hosts", encoding: .utf8),
+          hosts.contains("# BEGIN AgentTap") else {
+        orphanMissCount = 0
+        return
+    }
+    // Try connecting to the proxy port (8443) — if it responds, app is alive
+    let probe = socket(AF_INET, SOCK_STREAM, 0)
+    guard probe >= 0 else { return }
+    var probeAddr = sockaddr_in()
+    probeAddr.sin_family = sa_family_t(AF_INET)
+    probeAddr.sin_port = UInt16(8443).bigEndian
+    probeAddr.sin_addr.s_addr = UInt32(0x7f000001).bigEndian // 127.0.0.1
+    let connected = withUnsafePointer(to: &probeAddr) { ptr in
+        ptr.withMemoryRebound(to: sockaddr.self, capacity: 1) {
+            Darwin.connect(probe, $0, socklen_t(MemoryLayout<sockaddr_in>.size))
+        }
+    }
+    close(probe)
+    if connected == 0 {
+        orphanMissCount = 0
+        return
+    }
+    orphanMissCount += 1
+    if orphanMissCount >= 2 {
+        fputs("[Helper] Orphan detected — app not running but /etc/hosts has entries. Cleaning up.\n", stderr)
+        helper.cleanupOrphanState()
+        orphanMissCount = 0
+    }
+}
+orphanTimer.resume()
+
 // Accept connections via GCD
 let acceptQueue = DispatchQueue(
     label: "com.geiserx.agenttap.helper.accept",

src/bun/ca/ca-manager.ts

@@ -32,13 +32,8 @@ export async function ensureCA(): Promise<KeyCertPair> {
     const key = await Bun.file(CA_KEY_PATH).text();
     const cert = await Bun.file(CA_CERT_PATH).text();
 
-    if (isCertExpired(cert)) {
-      console.log("[CA] Certificate expired, regenerating...");
-      return regenerateCA();
-    }
-
-    if (isCertExpiringSoon(cert)) {
-      console.log("[CA] Certificate expires within 30 days, regenerating...");
+    if (isCertExpired(cert) || isCertExpiringSoon(cert)) {
+      console.log("[CA] Certificate expired or expiring soon, regenerating...");
       return regenerateCA();
     }
 
@@ -77,25 +72,27 @@ export function getCAStatus(): CAStatus {
   return "not-generated";
 }
 
-function isCertExpired(pem: string): boolean {
+const ROTATION_THRESHOLD_MS = 30 * 24 * 60 * 60 * 1000; // 30 days
+
+function getCertExpiresAt(pem: string): number | null {
   try {
     const crypto = require("node:crypto");
     const cert = new crypto.X509Certificate(pem);
-    return new Date(cert.validTo) < new Date();
+    return new Date(cert.validTo).getTime();
   } catch {
-    return false;
+    return null;
   }
 }
 
-const ROTATION_THRESHOLD_MS = 30 * 24 * 60 * 60 * 1000; // 30 days
+function isCertExpired(pem: string): boolean {
+  const expiresAt = getCertExpiresAt(pem);
+  // If we can't parse the cert, treat it as expired to force regeneration
+  if (expiresAt === null) return true;
+  return expiresAt < Date.now();
+}
 
 function isCertExpiringSoon(pem: string): boolean {
-  try {
-    const crypto = require("node:crypto");
-    const cert = new crypto.X509Certificate(pem);
-    const expiresAt = new Date(cert.validTo).getTime();
-    return expiresAt - Date.now() < ROTATION_THRESHOLD_MS;
-  } catch {
-    return false;
-  }
+  const expiresAt = getCertExpiresAt(pem);
+  if (expiresAt === null) return true;
+  return expiresAt - Date.now() < ROTATION_THRESHOLD_MS;
 }

src/bun/index.ts

@@ -95,14 +95,18 @@ function statusLabel(status: CaptureStatus): string {
 }
 
 function updateTrayIndicator() {
-  const newImage = state.captureStatus === "active"
-    ? "views://assets/tray-icon-active-template.png"
-    : state.captureStatus === "error"
-      ? "views://assets/tray-icon-error-template.png"
-      : "views://assets/tray-icon-template.png";
-  // All icons are template images (monochrome, adapts to menu bar color).
-  // Different shapes convey state: closed eye = off, open eye = capturing, X = error.
-  // Using setImage() alone avoids the ElectroBun template-flag recreation bug.
+  let newImage: string;
+  switch (state.captureStatus) {
+    case "active":
+      newImage = "views://assets/tray-icon-active-template.png";
+      break;
+    case "error":
+      newImage = "views://assets/tray-icon-error-template.png";
+      break;
+    default:
+      newImage = "views://assets/tray-icon-template.png";
+  }
+  // setImage() alone avoids the ElectroBun template-flag recreation bug.
   tray.setImage(newImage);
   tray.setTitle("");
 }
@@ -117,13 +121,16 @@ function buildTrayMenu() {
   }));
 
   const redirectCount = getRedirectCount();
-  const toggleLabel = state.proxyStatus === "starting"
-    ? "Starting…"
-    : state.proxyStatus === "stopping"
-      ? "Stopping…"
-      : state.captureStatus === "active"
-        ? "Stop Capture"
-        : "Start Capture";
+  let toggleLabel: string;
+  if (state.proxyStatus === "starting") {
+    toggleLabel = "Starting\u2026";
+  } else if (state.proxyStatus === "stopping") {
+    toggleLabel = "Stopping\u2026";
+  } else if (state.captureStatus === "active") {
+    toggleLabel = "Stop Capture";
+  } else {
+    toggleLabel = "Start Capture";
+  }
 
   tray.setMenu([
     {
@@ -184,6 +191,14 @@ function buildTrayMenu() {
 
 // ── Capture Control ───────────────────────────────────────────────────────
 
+let captureTransitionLock: Promise<unknown> = Promise.resolve();
+
+function enqueueCaptureTransition<T>(action: () => Promise<T>): Promise<T> {
+  const run = captureTransitionLock.then(action, action);
+  captureTransitionLock = run.catch(() => {});
+  return run;
+}
+
 async function doStartCapture(): Promise<{ success: boolean; error?: string; redirectCount?: number }> {
   if (state.proxyStatus === "starting" || state.proxyStatus === "stopping") {
     return { success: false, error: "Transition in progress" };
@@ -256,11 +271,10 @@ tray.on("tray-clicked", (e) => {
     if (state.proxyStatus === "starting" || state.proxyStatus === "stopping") {
       return;
     }
-    if (state.captureStatus === "active") {
-      doStopCapture();
-    } else {
-      doStartCapture();
-    }
+    // Serialize start/stop to prevent concurrent state mutations from rapid clicks
+    void enqueueCaptureTransition(() =>
+      state.captureStatus === "active" ? doStopCapture() : doStartCapture()
+    );
     return;
   }
 
@@ -306,8 +320,8 @@ function openViewer(): void {
         getSessions: (params) => getRecentSessions(params.limit) as any,
         getStats: () => getStats() as any,
         getState: () => getViewerState(),
-        startCapture: () => doStartCapture(),
-        stopCapture: () => doStopCapture(),
+        startCapture: () => enqueueCaptureTransition(() => doStartCapture()),
+        stopCapture: () => enqueueCaptureTransition(() => doStopCapture()),
         purgeTraces: (params) => {
           const deleted = purgeTraces(params);
           state.tracesCount = getTraceCount();
@@ -374,17 +388,17 @@ function shellEscape(s: string): string {
 function generateCurl(trace: Record<string, unknown>): string {
   const method = trace.request_method as string;
   const url = trace.request_url as string;
-  let cmd = `curl -X ${method} ${shellEscape(url)}`;
+  let cmd = `curl -X ${shellEscape(method)} ${shellEscape(url)}`;
   try {
     const headers = JSON.parse(trace.request_headers as string) as Record<string, string>;
     for (const [key, value] of Object.entries(headers)) {
       if (key.toLowerCase() === "host") continue;
-      cmd += ` \\\n  -H '${key}: ${value.replace(/'/g, "'\\''")}'`;
+      cmd += ` \\\n  -H ${shellEscape(`${key}: ${value}`)}`;
     }
   } catch { /* malformed headers */ }
   const body = trace.request_body as string | null;
   if (body && method !== "GET") {
-    cmd += ` \\\n  -d '${body.replace(/'/g, "'\\''")}'`;
+    cmd += ` \\\n  -d ${shellEscape(body)}`;
   }
   return cmd;
 }
@@ -434,8 +448,7 @@ function getSafeTraceUrl(url: string): string {
 }
 
 function getOptionalStringParam(url: URL, key: string): string | undefined {
-  const value = url.searchParams.get(key)?.trim();
-  return value ? value : undefined;
+  return url.searchParams.get(key)?.trim() || undefined;
 }
 
 function parseOptionalTimestampParam(url: URL, key: string): string | undefined {
@@ -499,15 +512,15 @@ function startDebugServer() {
       }
 
       if (path === "/start") {
-        const result = await doStartCapture();
+        const result = await enqueueCaptureTransition(() => doStartCapture());
         if (!result.success && result.error === "Transition in progress") {
           return Response.json({ error: result.error }, { status: 409 });
         }
         return Response.json(result);
       }
 
       if (path === "/stop") {
-        return Response.json(await doStopCapture());
+        return Response.json(await enqueueCaptureTransition(() => doStopCapture()));
       }
 
       if (path === "/traces") {