feat(desktop): add per-provider TLS verification bypass

[hqhq1025]

Summary

• Adds an opt-in tlsRejectUnauthorized boolean on ProviderEntry so users can connect to internal LLM gateways whose self-signed / private-CA certificates Node 22's built-in fetch (undici) rejects — NODE_TLS_REJECT_UNAUTHORIZED is intentionally ignored by undici, so a runtime dispatcher swap is the only working bypass.
• Implements a ref-counted withTlsBypass(enabled, fn) helper in apps/desktop/src/main/tls-override.ts that swaps the global undici dispatcher for one with rejectUnauthorized: false around the wrapped call, and restores the original in finally.
• Wires the bypass through every outbound HTTPS path: connection-test (test-active / test-provider / config:v1:test-endpoint), models discovery (models:v1:list-for-provider), and generation (codesign:v1:generate + codesign:v1:generate-title).
• Surfaces the toggle in AddCustomProviderModal for non-built-in providers only, gated behind a confirm prompt on first enable, and renders a yellow "TLS verify off" badge on the affected provider row in Settings.
• Built-in providers (anthropic, openai, openrouter, ollama) force-ignore the flag at every call site so a tampered config cannot weaken trusted endpoints.

Closes #229.

Hard-constraints check (PRINCIPLES §5b)

• ✅ Compatibility — optional field, no schema version bump, existing configs parse unchanged
• ✅ Upgradeability — flag is namespaced and per-entry; safe to extend (e.g. per-CA pinning later)
• ✅ No bloat — undici was already a transitive dep via pi-ai; now promoted to a direct dep so resolution is explicit. Helper is ~90 lines; UI delta is a single checkbox + badge
• ✅ Elegance — single high-order wrapper, exception-safe via try/finally, ref-count handles nested / concurrent acquires

Design notes

Full design spec lives at docs/superpowers/specs/2026-05-23-tls-bypass-design.md (gitignored per project convention, local-only). The known concurrency window (parallel non-bypass requests during a bypass interval inherit the loose dispatcher) is documented in tls-override.ts and accepted as a trade-off — eliminating it would require monkey-patching globalThis.fetch, which touches every outbound call in the main process and was deemed disproportionate. See spec §9 for rationale.

Test plan

• pnpm typecheck — clean across all 10 workspaces
• pnpm test — 1354/1354 desktop, 228/228 shared, 208/208 providers, etc.
• pnpm lint — only existing-on-main biome version-mismatch info; no errors
• New apps/desktop/src/main/tls-override.test.ts — 6 tests covering: enabled=false noop, single acquire/release, nested acquire (ref-count), throw-in-fn release, log emission, parallel acquire
• Extended apps/desktop/src/main/connection-ipc.test.ts — 7 new TLS bypass routing tests (built-in force-disable, non-built-in enable, undefined/false → disable, payload flag enable, non-boolean reject)
• Extended packages/shared/src/config.test.ts — schema accepts/rejects/round-trips the new field
• Manual verification with an actual self-signed gateway is recommended before merge (cannot be done in CI without spinning up a fake CA)

Known out-of-scope items

• codesign:apply-comment IPC also calls runGenerate against the active provider; left unwrapped to stay in scope per spec. Recommend a tiny follow-up to apply the same wrapper there for inline-comment edits against bypass-enabled providers.

[github-actions]

Review mode: initial

Findings

• [Major] Missing codesign:apply-comment IPC bypass — the PR explicitly acknowledges this as out-of-scope (apps/desktop/src/main/ipc/generate.ts lines not shown in patch but referenced in PR body). However, since apply-comment also calls runGenerate against the active provider, users connecting via a self-signed gateway will get SSL errors on inline-edit operations immediately after they enable the bypass. This is a user-visible regression for the very use case the PR enables. The fix is small (~3 lines around the apply-comment handler in generate.ts) and closing #229 without it leaves a broken path.
  Suggested fix: wrap the apply-comment IPC handler with withTlsBypass(tlsBypass, () => ...) following the same pattern as the other generate paths.

• [Major] Linked issue #229 claims closure but apply-comment path is unwired — the PR's Closes #229 implies full resolution of the issue. The issue's scope (connecting to internal gateways with self-signed certs) is incomplete if inline-comment edits fail. Change the issue link to Refs #229 or implement the bypass in the apply-comment handler before marking it closed.

• [Minor] Hardcoded UI tokens in primitives.tsx badge — the TLS badge uses className="inline-flex items-center gap-1 px-1.5 py-0.5 rounded-full border border-[var(--color-warning)] text-[var(--color-warning)] bg-[color-mix(in_oklab,var(--color-warning)_12%,transparent)] text-[var(--font-size-badge)] font-medium leading-none". While --color-warning and --font-size-badge are tokens, the spacing/padding values (px-1.5, py-0.5, gap-1) are raw Tailwind utilities that duplicate packages/ui semantic token values (e.g., --spacing-1, --spacing-1_5). For a new UI element, prefer token-based spacing to maintain consistency.
  Suggested fix: use className="rounded-full border border-[var(--color-warning)] text-[var(--color-warning)] bg-[color-mix(in_oklab,var(--color-warning)_12%,transparent)] text-[var(--font-size-badge)] font-medium leading-none" and apply padding via style={{ padding: 'var(--spacing-1_5) var(--spacing-1)' }} or define a component-level token. However, this is a minor style consistency issue and does not block merge.

• [Nit] undici promoted to direct dependency — this is justified (previously transitive, now explicitly declared), but the lockfile diff shows [email protected] was already present as a transitive dep via pi-ai. No new install weight. Acceptable.

• [Nit] window.confirm for security warning — the PR uses window.confirm() which matches existing patterns (per Claude.md: no AlertDialog primitive). Fine, but the confirm text contains locale keys resolved via t() — ensure the translation files have matching keys (they do in all 4 locales). No action needed.

Questions

• None.

Summary

• Well-structured PR with thorough testing (new tls-override.test.ts, extended connection-ipc.test.ts, extended config.test.ts). The core mechanism (withTlsBypass) is cleanly designed with ref counting and exception safety. The concurrency window is documented and accepted. All hard constraints are satisfied: no bundled runtimes, BYOK preserved, local-first, permissive deps (undici is MIT), no bloat (minimal ~90 lines). The missing apply-comment path is the only material gap; fixing it is straightforward. Once addressed, this is ready to merge.

Testing

• New tests: 6 unit tests for tls-override.ts, 7 integration tests for connection-ipc.ts, 3 schema tests for config.ts. All pass. Missing test for the apply-comment path (since it's not wired).

Open-CoDesign Bot

[hqhq1025]

Thanks for the review.

✅ Major #1 — fixed in aca58c9

codesign:apply-comment handler now wraps its runGenerate call in withTlsBypass(tlsBypass, () => ...) with tlsBypass derived via the same resolveTlsBypassFor(cfg, active.model.provider) helper used by codesign:v1:generate and codesign:v1:generate-title. Inline-comment edits against a TLS-bypass-enabled provider now succeed against self-signed gateways.

✅ Major #2 — auto-resolved by #1

With apply-comment wired, the Closes #229 claim is now accurate. Leaving as-is.

🟢 Minor #3 — declined, with rationale

The bot suggested replacing px-1.5 py-0.5 gap-1 with var(--spacing-1_5) etc. — but packages/ui/src/tokens.css does not define any --spacing-* tokens; the project's tokens are color + font-size only. Spacing is consistently expressed via Tailwind utilities throughout the codebase.

More importantly, the new TLS badge is immediately adjacent to an existing lockEndpoint badge in primitives.tsx:358 that uses the identical inline-flex items-center gap-1 px-1.5 py-0.5 rounded-full pattern. Matching the neighbor preserves visual + code consistency. Adding new spacing tokens just for the TLS badge would create a one-off divergence; converting both badges plus every other badge to a token-based system is out of scope for this PR.

Happy to revisit if a --spacing-* token system is introduced project-wide later.

Test plan

• pnpm typecheck — clean
• pnpm --filter @open-codesign/desktop exec vitest run src/main/ipc/generate — 11/11 pass
• pnpm exec biome check apps/desktop/src/main/ipc/generate.ts — clean

[github-actions]

Review mode: follow-up after new commits

Findings

• [Resolved] Missing codesign:apply-comment IPC bypass — the previous review flagged that the apply-comment handler in generate.ts was not wrapped with withTlsBypass. The latest commits now add const tlsBypass = resolveTlsBypassFor(cfg, active.model.provider); and wrap the runGenerate call in that handler with withTlsBypass(tlsBypass, () => runGenerate(...)). This closes the gap. See apps/desktop/src/main/ipc/generate.ts lines ~1332 and ~1380-1416 in the compare diff.
• [Resolved] Linked issue #229 closure now complete — the apply-comment bypass is wired, so the issue's acceptance criteria (all outbound HTTPS paths through custom providers respect the toggle) are fully satisfied. The Closes #229 claim is now accurate.
• [Minor] No test for apply-comment TLS bypass routing — the new path is functionally identical to the other generate paths (same resolveTlsBypassFor + withTlsBypass pattern) and shares the already-tested withTlsBypass mock, but there is no dedicated test asserting that apply-comment calls withTlsBypass with the correct enabled flag. Adding one in connection-ipc.test.ts or a new generate.test.ts would prevent regression. This is non-blocking because the existing routing tests cover the same logic.

Summary

Both material issues from the previous review have been addressed. The apply-comment handler is now wrapped with the TLS bypass, making the PR complete for all generation paths. The architecture remains clean (withTlsBypass + ref-counted dispatcher swap), tests pass, all hard constraints are satisfied, and the linked issue #229 is now fully closed. This is ready to merge after considering the minor test-coverage gap above.

Open-CoDesign Bot