Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
41 changes: 40 additions & 1 deletion AGENTS.md
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
# AgentTap - Development Guide

> **IMPORTANT**: Do NOT update this file unless the user explicitly says to.
> **IMPORTANT**: Update this file with key learnings as they are discovered during development. Keep entries brief and actionable.

## Project Overview

Expand Down Expand Up @@ -86,4 +86,43 @@ groq: api.groq.com
- All proxy operations must handle TLS errors gracefully
- Traces must be queryable by provider, model, timestamp, and source app

## Rollback & Restore

**Kill proxy:** `pkill -f AgentTap` or `lsof -ti:8443 | xargs kill -9`
**Remove CA:** `rm -rf ~/Library/Application\ Support/AgentTap/ca/`
**Remove CA from system keychain:** `security delete-certificate -c "AgentTap CA" /Library/Keychains/System.keychain`
**Remove pf rules:** `sudo pfctl -f /etc/pf.conf` (restores defaults)
**Verify no lingering listeners:** `lsof -i :8443 -i :18443`
**Uninstall app:** `rm -rf ~/Library/Application\ Support/AgentTap/`

## Key Learnings

### Bun TLS Limitations (CRITICAL)

Bun's `node:tls` has significant limitations for MITM proxy use cases:

- **`tls.TLSSocket` wrapping with `isServer: true`**: data events never fire on the wrapped socket
- **`pipe()` between TLS sockets**: drops the TLS ClientHello — data never arrives
- **`SNICallback` on `tls.createServer`**: handshake completes but cleartext data events don't fire on piped connections

**Working pattern**: Per-domain `tls.createServer` with **static certs** (no SNICallback) + manual ClientHello capture via `once("data")` after sending CONNECT 200 + `net.connect` to loopback TLS server with explicit `write()` forwarding (not pipe). See `proxy-server.ts`.

### HTTP Response Completion

HTTP/1.1 keep-alive means upstream connections don't close after sending a response. Never rely on `end`/`close` events for response completion. Instead detect completion by:
- **Content-Length**: track body bytes received vs header value
- **Chunked**: scan for `0\r\n\r\n` terminator
- **SSE**: finalize only on connection close (server closes when stream ends)

### Dependencies

- `@peculiar/x509` requires `reflect-metadata` polyfill (`import "reflect-metadata"` at top of cert-generator.ts) — tsyringe dependency
- Use `globalThis.crypto` (Bun built-in WebCrypto), NOT `@peculiar/webcrypto`
- `bun-types` has known `Buffer`/`ArrayBufferLike` type mismatches with strict TS — safe to ignore, works at runtime

### ElectroBun

- Tray click handlers are sync — use `.then()/.catch()` for async operations, not `await`
- `exitOnLastWindowClosed: false` required for tray-only apps

*Generated by [LynxPrompt](https://lynxprompt.com) CLI*
38 changes: 38 additions & 0 deletions bun.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 2 additions & 0 deletions electrobun.config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,8 @@ export default {
"src/views/mainview/index.css": "views/mainview/index.css",
"src/views/mainview/assets/tray-icon-template.png":
"views/assets/tray-icon-template.png",
"src/views/mainview/assets/tray-icon-template@2x.png":
"views/assets/tray-icon-template@2x.png",
},

useAsar: false,
Expand Down
4 changes: 4 additions & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -18,5 +18,9 @@
"devDependencies": {
"electrobun": "^1.16.0",
"typescript": "^5.8.3"
},
"dependencies": {
"@peculiar/x509": "^2.0.0",
"reflect-metadata": "^0.2.2"
}
}
83 changes: 83 additions & 0 deletions src/bun/ca/ca-manager.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,83 @@
import { existsSync } from "node:fs";
import { mkdir } from "node:fs/promises";
import { join } from "node:path";
import { generateRootCA, type KeyCertPair } from "./cert-generator";

export type CAStatus =
| "not-generated"
| "generated"
| "installed"
| "expired";

const CA_DIR = join(
process.env.HOME ?? "~",
"Library",
"Application Support",
"AgentTap",
"ca",
);
const CA_KEY_PATH = join(CA_DIR, "ca-key.pem");
const CA_CERT_PATH = join(CA_DIR, "ca.pem");
Comment on lines +12 to +20
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Do not persist the CA private key under $HOME/Library/....

This stores the MITM root key outside the app container, which weakens the trust boundary around the most sensitive secret in the system. Based on learnings, Never store the custom CA private key outside the app's sandboxed container.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/bun/ca/ca-manager.ts` around lines 12 - 20, The CA private key is being
persisted under HOME via the CA_DIR/CA_KEY_PATH/CA_CERT_PATH constants; change
CA_DIR to a path inside the application's sandboxed container (e.g., the app's
data/userData directory or a dedicated secure storage directory provided by the
runtime) and update CA_KEY_PATH and CA_CERT_PATH to join against that sandboxed
CA_DIR instead of process.env.HOME; ensure the code that reads/writes the key
(references to CA_KEY_PATH and CA_CERT_PATH) uses the new location, add a
migration step to move existing keys into the sandbox if present, and enforce
strict file permissions when creating the key files.


let loaded: KeyCertPair | null = null;

export function getCAPaths() {
return { keyPath: CA_KEY_PATH, certPath: CA_CERT_PATH, dir: CA_DIR };
}

export async function ensureCA(): Promise<KeyCertPair> {
if (loaded) return loaded;

if (existsSync(CA_KEY_PATH) && existsSync(CA_CERT_PATH)) {
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();
}

loaded = { key, cert };
console.log("[CA] Loaded existing CA from", CA_DIR);
return loaded;
Comment on lines +31 to +42
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Treat unreadable CA certs as invalid.

If ca.pem is corrupted, isCertExpired() returns false, so Lines 31-42 load broken material instead of regenerating it. That defers the failure into proxy startup and leaf issuance.

Suggested fix
 function isCertExpired(pem: string): boolean {
   try {
     const crypto = require("node:crypto");
     const cert = new crypto.X509Certificate(pem);
     return new Date(cert.validTo) < new Date();
   } catch {
-    return false;
+    return true;
   }
 }

Also applies to: 75-83

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/bun/ca/ca-manager.ts` around lines 31 - 42, The current load path reads
CA_KEY_PATH/CA_CERT_PATH and blindly returns them when isCertExpired(cert) is
false, which lets corrupted ca.pem be used; update the CA loading logic in
ca-manager.ts (the block using CA_KEY_PATH, CA_CERT_PATH, isCertExpired,
regenerateCA, loaded, CA_DIR) to validate readability and parseability of both
key and cert before accepting them: wrap the Bun.file(...).text() and
certificate validation in a try/catch (or otherwise check parse success), treat
any I/O or parse error as an expired/invalid cert and call regenerateCA(), and
apply the same defensive checks to the later similar block around lines 75-83 so
unreadable/corrupted files trigger regeneration instead of being loaded.

}

return regenerateCA();
}

export async function regenerateCA(): Promise<KeyCertPair> {
await mkdir(CA_DIR, { recursive: true });

console.log("[CA] Generating new root CA...");
const pair = await generateRootCA();

await Bun.write(CA_KEY_PATH, pair.key, { mode: 0o600 });
await Bun.write(CA_CERT_PATH, pair.cert);

loaded = pair;
console.log("[CA] Root CA written to", CA_DIR);
return loaded;
}

export function getCA(): KeyCertPair | null {
return loaded;
}

export function getCAStatus(): CAStatus {
if (!loaded) {
if (!existsSync(CA_CERT_PATH)) return "not-generated";
}
if (loaded && isCertExpired(loaded.cert)) return "expired";
if (loaded) return "generated";
return "not-generated";
}

function isCertExpired(pem: string): boolean {
try {
const crypto = require("node:crypto");
const cert = new crypto.X509Certificate(pem);
return new Date(cert.validTo) < new Date();
} catch {
return false;
}
}
56 changes: 56 additions & 0 deletions src/bun/ca/cert-cache.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
import type { KeyCertPair } from "./cert-generator";
import { generateLeafCert } from "./cert-generator";

const MAX_CACHE_SIZE = 100;

interface CacheEntry {
pair: KeyCertPair;
expiresAt: number;
lastUsed: number;
}

const cache = new Map<string, CacheEntry>();

export async function getOrGenerateLeafCert(
domain: string,
caCert: string,
caKey: string,
): Promise<KeyCertPair> {
const existing = cache.get(domain);
const now = Date.now();

if (existing && existing.expiresAt > now) {
existing.lastUsed = now;
return existing.pair;
}

const pair = await generateLeafCert(domain, caCert, caKey);

if (cache.size >= MAX_CACHE_SIZE) {
evictLRU();
}

cache.set(domain, {
pair,
expiresAt: now + 25 * 86400_000, // 25 days (leaf certs valid 30)
lastUsed: now,
});

return pair;
Comment on lines +19 to +39
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Deduplicate concurrent cache misses per domain.

Multiple simultaneous misses for the same domain will each generate a new cert. This is avoidable work on a hot path.

Proposed fix
 const cache = new Map<string, CacheEntry>();
+const inflight = new Map<string, Promise<KeyCertPair>>();
@@
   if (existing && existing.expiresAt > now) {
     existing.lastUsed = now;
     return existing.pair;
   }
 
-  const pair = await generateLeafCert(domain, caCert, caKey);
+  const pending = inflight.get(domain);
+  if (pending) return pending;
+
+  const generation = generateLeafCert(domain, caCert, caKey).finally(() => {
+    inflight.delete(domain);
+  });
+  inflight.set(domain, generation);
+  const pair = await generation;
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
const existing = cache.get(domain);
const now = Date.now();
if (existing && existing.expiresAt > now) {
existing.lastUsed = now;
return existing.pair;
}
const pair = await generateLeafCert(domain, caCert, caKey);
if (cache.size >= MAX_CACHE_SIZE) {
evictLRU();
}
cache.set(domain, {
pair,
expiresAt: now + 25 * 86400_000, // 25 days (leaf certs valid 30)
lastUsed: now,
});
return pair;
const cache = new Map<string, CacheEntry>();
const inflight = new Map<string, Promise<KeyCertPair>>();
const existing = cache.get(domain);
const now = Date.now();
if (existing && existing.expiresAt > now) {
existing.lastUsed = now;
return existing.pair;
}
const pending = inflight.get(domain);
if (pending) return pending;
const generation = generateLeafCert(domain, caCert, caKey).finally(() => {
inflight.delete(domain);
});
inflight.set(domain, generation);
const pair = await generation;
if (cache.size >= MAX_CACHE_SIZE) {
evictLRU();
}
cache.set(domain, {
pair,
expiresAt: now + 25 * 86400_000, // 25 days (leaf certs valid 30)
lastUsed: now,
});
return pair;
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/bun/ca/cert-cache.ts` around lines 19 - 39, Current code can trigger
multiple simultaneous generateLeafCert calls for the same domain; add a
deduplication layer by introducing an inFlight map (e.g., inFlightPromises:
Map<string, Promise<CertificatePair>>) and check it before calling
generateLeafCert in the cache-get path, so if inFlightPromises.get(domain)
exists you await and return that promise's result instead of starting a new
generation; when starting generation, set inFlightPromises.set(domain, promise),
await it, then remove the entry in finally and only then write the resolved pair
into cache (using the existing cache.set logic and evictLRU/MAX_CACHE_SIZE
handling); also ensure rejected promises are removed from inFlightPromises so
subsequent requests can retry.

}

export function clearCache(): void {
cache.clear();
}

function evictLRU(): void {
let oldest: string | null = null;
let oldestTime = Infinity;
for (const [domain, entry] of cache) {
if (entry.lastUsed < oldestTime) {
oldestTime = entry.lastUsed;
oldest = domain;
}
}
if (oldest) cache.delete(oldest);
}
Loading
Loading