Skip to content

Adds Chrome-style certificate bypass for local development against HTTPS servers with self-signed or unknown-root certificates - #1184

Open
assaf758 wants to merge 13 commits into
manaflow-ai:mainfrom
assaf758:ignore.cert
Open

assaf758 wants to merge 13 commits into
manaflow-ai:mainfrom
assaf758:ignore.cert

Conversation

@assaf758

@assaf758 assaf758 commented Mar 11, 2026 •

Copy link
Copy Markdown

Summary

Adds Chrome-style certificate bypass for local development against HTTPS servers with self-signed or unknown-root certificates.

  • --ignore-certificate-errors launch flag sets the runtime override at startup (not persisted)
  • browser cert-bypass get | set <true|false> CLI/socket command toggles at runtime (not persisted)
  • User default browserIgnoreCertificateErrors for persistent default

Usage

Launch with bypass active:
cmux --ignore-certificate-errors

Toggle at runtime:
cmux browser cert-bypass set true
cmux browser cert-bypass get

Persist across restarts:
defaults write ai.manaflow.cmux browserIgnoreCertificateErrors -bool true

Implementation notes

  • BrowserCertBypassSettings enum owns the flag state: in-memory runtimeOverride (session-only) with a UserDefaults fallback for persistence
  • TLS bypass is scoped to NSURLAuthenticationMethodServerTrust challenges only — client cert flows, MDM/Entra CA, and SSO extensions are unaffected

Testing

  • Socket regression test in tests_v2/test_browser_cert_bypass.py
  • manually testing:
    • changing bypass state from socket
    • using --ignore-certificate-errors at launch
    • changing user default, verifying that socket and flag has priority over user default.

Demo Video

NA

Review Trigger (Copy/Paste as PR comment)

@codex review
@coderabbitai review
@greptile-apps review
@cubic-dev-ai review

Checklist

  • I tested the change locally
  • I added or updated tests for behavior changes
  • I updated docs/changelog if needed
  • I requested bot reviews after my latest commit (copy/paste block above or equivalent)
  • All code review bot comments are resolved
  • All human review comments are resolved

Summary by cubic

Adds a Chrome-style certificate bypass so the embedded browser can load self-signed or unknown-root HTTPS servers during local development. Only NSURLAuthenticationMethodServerTrust is bypassed; client cert flows, MDM/Entra CA trust, and SSO extensions are unaffected.

  • New Features

    • --ignore-certificate-errors launch flag enables session-only bypass.
    • browser cert-bypass get|set <true|false> CLI/socket toggle at runtime (session-only).
    • Persistent default via browserIgnoreCertificateErrors in UserDefaults; runtime/flag take precedence.
    • WebKit trusts server certs when enabled; regression test added in tests_v2/test_browser_cert_bypass.py.
  • Migration

    • Launch with bypass: cmux --ignore-certificate-errors
    • Toggle at runtime: cmux browser cert-bypass set true and cmux browser cert-bypass get
    • Persist across restarts: defaults write ai.manaflow.cmux browserIgnoreCertificateErrors -bool true

Written for commit 14d2322. Summary will update on new commits.

Summary by CodeRabbit

  • New Features

    • Added certificate bypass feature for WebKit TLS validation, controllable via the --ignore-certificate-errors CLI flag at startup or the browser.cert_bypass socket command at runtime (get/set actions).
  • Documentation

    • Added feature documentation and design specifications for the certificate bypass functionality.

@vercel

vercel Bot commented Mar 11, 2026

Copy link
Copy Markdown

@assaf758 is attempting to deploy a commit to the Manaflow Team on Vercel.

A member of the Team first needs to authorize it.

@chatgpt-codex-connector

Copy link
Copy Markdown

To use Codex here, create an environment for this repo.

@coderabbitai

coderabbitai Bot commented Mar 11, 2026 •

Copy link
Copy Markdown
📝 Walkthrough

Walkthrough

The PR introduces a WebKit TLS certificate bypass feature comprising a runtime-configurable BrowserCertBypassSettings enum, CLI startup flag --ignore-certificate-errors, socket command browser.cert_bypass for get/set actions, and browser delegate modifications to skip server trust validation when enabled.

Changes

Cohort / File(s) Summary
Core Bypass Implementation
Sources/Panels/BrowserPanel.swift, Sources/TerminalController.swift, Sources/cmuxApp.swift
Introduces BrowserCertBypassSettings enum with runtimeOverride and isEnabled() logic; updates BrowserNavigationDelegate to bypass server trust challenges conditionally; adds v2 socket handler v2BrowserCertBypass for get/set actions; integrates --ignore-certificate-errors CLI flag at startup.
CLI Command Routing
CLI/cmux.swift
Adds browser cert-bypass subcommand handler accepting action (default "get") and optional value; routes to browser.cert_bypass v2 API call; outputs boolean payload or "OK"; returns early after processing.
Documentation
docs/superpowers/plans/2026-03-10-webkit-cert-bypass.md, docs/superpowers/specs/2026-03-10-webkit-cert-bypass-design.md
Comprehensive feature plan and design specification documenting configuration, integration points, socket command behavior, CLI flag usage, and end-to-end workflow.
Test Coverage
tests_v2/test_browser_cert_bypass.py
Python regression test script validating browser.cert_bypass socket command with get/set actions, state transitions, and persistence of initial state.

Sequence Diagram

sequenceDiagram
    participant WebView as WebView
    participant NavDelegate as BrowserNavigationDelegate
    participant Settings as BrowserCertBypassSettings
    participant TLSChallenge as TLS Challenge Handler

    WebView->>NavDelegate: didReceive(challenge)
    NavDelegate->>NavDelegate: Check authenticationMethod == ServerTrust
    NavDelegate->>Settings: isEnabled()
    alt Bypass Enabled
        Settings-->>NavDelegate: true
        NavDelegate->>NavDelegate: Extract serverTrust credential
        NavDelegate->>TLSChallenge: useCredential(serverTrust)
        TLSChallenge-->>WebView: Accept connection
    else Bypass Disabled
        Settings-->>NavDelegate: false
        NavDelegate->>TLSChallenge: Default handling
        TLSChallenge-->>WebView: Standard cert validation
    end
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~22 minutes

Possibly related PRs

  • manaflow-ai/cmux#806: Modifies BrowserNavigationDelegate.webView(_:didReceive:completionHandler:) to alter TLS challenge handling logic, directly related to server trust challenge processing.
  • manaflow-ai/cmux#936: Updates browser subcommand routing in CLI/cmux.swift and v2 command dispatch in TerminalController.swift, affecting the same command surfaces extended here.

Poem

🐰 A bunny hops through certificates so grand,
With trust flags waving across the land,
Bypass set true, TLS falls away,
Self-signed certs now welcome to play!
Development blooms where shadows once lay. ✨

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 18.18% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately summarizes the main change: introducing a Chrome-style certificate bypass for local development with self-signed or unknown-root certificates, which aligns with the changeset.
Description check ✅ Passed PR description covers all required sections: summary with why/what, testing approach with manual verification, and a complete checklist with most items addressed.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

Comment @coderabbitai help to get the list of available commands and usage tips.

@greptile-apps

greptile-apps Bot commented Mar 11, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR adds Chrome-style TLS certificate bypass for local development against HTTPS servers with self-signed or unknown-root certificates. It introduces BrowserCertBypassSettings (a @MainActor enum in BrowserPanel.swift) to hold a session-scoped runtimeOverride with a UserDefaults fallback, wires it to a --ignore-certificate-errors CLI flag at startup, exposes a browser.cert_bypass get|set socket command for runtime toggling, and hooks the bypass into the existing WKNavigationDelegate TLS challenge handler.

  • Core bypass logic (BrowserPanel.swift): BrowserCertBypassSettings is annotated @MainActor, but BrowserNavigationDelegate (where isEnabled() is called) is a plain NSObject subclass without any actor annotation — this is a Swift concurrency isolation violation that will become a compiler error under Swift 6 strict concurrency.
  • Socket handler (TerminalController.swift): v2BrowserCertBypass correctly uses v2MainSync{} to hop to the main actor before reading/writing @MainActor state; however there is no reset action to clear runtimeOverride back to nil and restore UserDefaults-based control for the live session.
  • Test teardown (test_browser_cert_bypass.py): The finally block restores state by calling set false (or set true), which sets runtimeOverride to a concrete value rather than nil. This permanently shadows the UserDefaults fallback for the rest of the session, which can mask a persistent browserIgnoreCertificateErrors=true default set by the user.

Confidence Score: 3/5

  • Functionally working but has a Swift concurrency isolation violation that will break under Swift 6 strict concurrency and a test teardown side-effect that can mask persistent UserDefaults configuration.
  • The runtime behavior is correct on current Swift toolchain versions since WKNavigationDelegate callbacks always execute on the main thread. However, the @mainactor isolation violation in BrowserNavigationDelegate is a real architectural issue that will fail to compile under Swift 6 strict concurrency. The test teardown bug is a latent correctness issue that can affect anyone using the UserDefaults persistence path.
  • Sources/Panels/BrowserPanel.swift (actor isolation on the delegate) and tests_v2/test_browser_cert_bypass.py (teardown side-effect)

Important Files Changed

Filename Overview
Sources/Panels/BrowserPanel.swift Adds BrowserCertBypassSettings enum (@mainactor) and TLS bypass in BrowserNavigationDelegate; the delegate method accesses @mainactor state without being @mainactor itself — Swift 6 concurrency violation.
Sources/TerminalController.swift Adds browser.cert_bypass socket command dispatch and handler with correct v2MainSync usage for @mainactor state; missing a reset action to clear runtimeOverride back to nil.
Sources/cmuxApp.swift Adds --ignore-certificate-errors CLI flag check at app init; straightforward and correctly placed before UI load.
CLI/cmux.swift Adds cert-bypass subcommand to the browser command and registers it as verbsWithoutSurface; CLI wiring looks correct.
tests_v2/test_browser_cert_bypass.py Socket regression test with try/finally teardown; teardown sets runtimeOverride to False instead of nil, permanently masking UserDefaults for the session and potentially causing test interference.

Sequence Diagram

sequenceDiagram
    participant CLI as cmux CLI
    participant App as cmuxApp.init()
    participant TC as TerminalController
    participant CBS as BrowserCertBypassSettings<br/>(@MainActor)
    participant UD as UserDefaults
    participant ND as BrowserNavigationDelegate

    Note over App,CBS: Launch path
    CLI->>App: --ignore-certificate-errors
    App->>CBS: runtimeOverride = true

    Note over CLI,CBS: Runtime socket path
    CLI->>TC: browser.cert_bypass set true
    TC->>TC: v2MainSync { }
    TC->>CBS: runtimeOverride = true
    TC-->>CLI: {"enabled": true}

    CLI->>TC: browser.cert_bypass get
    TC->>TC: v2MainSync { }
    TC->>CBS: isEnabled()
    CBS->>UD: bool(forKey:) [fallback only]
    CBS-->>TC: true / false
    TC-->>CLI: {"enabled": true/false}

    Note over ND,CBS: TLS challenge path (always main thread)
    ND->>CBS: isEnabled() [⚠ no @MainActor on ND]
    CBS-->>ND: true
    ND-->>ND: completionHandler(.useCredential, trust)
Loading

Last reviewed commit: 14d2322

Comment on lines +3824 to +3826
if challenge.protectionSpace.authenticationMethod == NSURLAuthenticationMethodServerTrust,
BrowserCertBypassSettings.isEnabled(),
let serverTrust = challenge.protectionSpace.serverTrust {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@MainActor isolation violation in delegate callback

BrowserCertBypassSettings is annotated @MainActor, meaning all its static members (runtimeOverride, isEnabled()) are main-actor-isolated. BrowserNavigationDelegate is a plain NSObject subclass without @MainActor isolation, so the call to BrowserCertBypassSettings.isEnabled() here is a Swift concurrency isolation violation.

While WKNavigationDelegate callbacks are always dispatched on the main thread at runtime, Swift's type system does not know this — there is no @MainActor annotation on the class or method to make it verifiable at compile time. Under Swift 6 strict concurrency (-strict-concurrency=complete), this will be a compiler error.

The fix is either to annotate BrowserNavigationDelegate (or at least this method) as @MainActor, or mark BrowserCertBypassSettings as nonisolated(unsafe) if the invariant is enforced by a comment:

// Option A — annotate the class or method:
@MainActor
func webView(
    _ webView: WKWebView,
    didReceive challenge: URLAuthenticationChallenge,
    completionHandler: @escaping (URLSession.AuthChallengeDisposition, URLCredential?) -> Void
) {

or add @MainActor to the BrowserNavigationDelegate class declaration itself.

Comment on lines +48 to +49
return 0

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Test teardown sets runtimeOverride to a concrete value instead of clearing it

The finally block restores runtimeOverride to True or False — never to its original state of None (the sentinel that causes isEnabled() to fall back to UserDefaults). If the app was started without --ignore-certificate-errors and without a UserDefaults entry (so initial == False comes from the UserDefaults.bool default of false, not from an explicit override), then after this test runtimeOverride is permanently set to False, shadowing any live UserDefaults change for the rest of the session.

This means a developer who has defaults write ai.manaflow.cmux browserIgnoreCertificateErrors -bool true set would find bypass silently disabled after the test suite runs.

A safer teardown would be to add a reset action to the socket command (and call it here), or accept that the test is destructive and document it clearly:

finally:
    # NOTE: this sets runtimeOverride to False, not None.
    # If the initial state relied on UserDefaults it will be shadowed
    # for the rest of the session.
    c._call("browser.cert_bypass", {"action": "set", "value": "true" if initial else "false"})

Comment on lines +7329 to +7358
private func v2BrowserCertBypass(params: [String: Any]) -> V2CallResult {
let action = (params["action"] as? String) ?? (params["args"] as? [String])?.first ?? ""
switch action {
case "get":
var result: V2CallResult = .err(code: "internal", message: "Unexpected", data: nil)
v2MainSync {
result = .ok(["enabled": BrowserCertBypassSettings.isEnabled()])
}
return result
case "set":
let rawValue = (params["args"] as? [String])?.dropFirst().first
?? (params["value"] as? String)
?? (params["enabled"] as? Bool).map { $0 ? "true" : "false" }
guard let rawValue else {
return .err(code: "invalid_params", message: "Missing value: use 'set true' or 'set false'", data: nil)
}
switch rawValue.lowercased() {
case "true", "1", "yes":
v2MainSync { BrowserCertBypassSettings.runtimeOverride = true }
return .ok(["enabled": true])
case "false", "0", "no":
v2MainSync { BrowserCertBypassSettings.runtimeOverride = false }
return .ok(["enabled": false])
default:
return .err(code: "invalid_params", message: "Invalid value '\(rawValue)': use 'true' or 'false'", data: nil)
}
default:
return .err(code: "invalid_params", message: "Unknown action '\(action)': use 'get' or 'set'", data: nil)
}
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

No reset action to clear runtimeOverride back to nil

Once runtimeOverride is set to false (either by cert-bypass set false or by the test), there is no way to restore the nil sentinel that defers to UserDefaults. A developer who has browserIgnoreCertificateErrors=true in UserDefaults and then runs cert-bypass set false to temporarily disable it cannot get back to "use UserDefaults" without restarting the app.

Consider adding a reset action:

case "reset":
    v2MainSync { BrowserCertBypassSettings.runtimeOverride = nil }
    var result: V2CallResult = .err(code: "internal", message: "Unexpected", data: nil)
    v2MainSync {
        result = .ok(["enabled": BrowserCertBypassSettings.isEnabled()])
    }
    return result

This would also solve the test teardown issue described above.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 7

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@CLI/cmux.swift`:
- Line 5054: The new help text "cert-bypass [get | set <true|false>]" in
CLI/cmux.swift is currently raw English; replace the literal string with a
localized call using String(localized: "cli.cert-bypass.help", defaultValue:
"cert-bypass [get | set <true|false>]") (or a similarly named key) and then add
that key and English value to Resources/Localizable.xcstrings; ensure you update
any call sites that display the help so they use the localized string (search
for the exact literal or the function that builds the help lines in
CLI/cmux.swift to locate where to substitute).
- Around line 3906-3919: Validate the argument count for the "cert-bypass"
subcommand before calling client.sendV2: check subArgs and ensure only allowed
forms ("get" with exactly 0 extra tokens, or "set" followed by exactly one value
and no additional tokens); if the arity is invalid, emit a clear error/usage
message via output and return without calling client.sendV2. Update the block
that sets action/params (references: subArgs, action, params, payload,
client.sendV2, output) to perform this validation and only populate
params["value"] and call client.sendV2 when the argument count matches the
expected "set <true|false>" pattern.

In `@docs/superpowers/plans/2026-03-10-webkit-cert-bypass.md`:
- Around line 230-240: The CLI examples use the old socket-style name and flags;
update them to the actual user-facing CLI syntax used by this PR: replace
occurrences like "cmux browser.cert_bypass --action get" and "cmux
browser.cert_bypass --action set --value true/false" with the correct forms
"cmux browser cert-bypass get" and "cmux browser cert-bypass set true" (and
"cmux browser cert-bypass set false"), and make the same changes for the other
affected block (lines ~350-359) so all examples show "cmux browser cert-bypass
get|set <true|false>".

In `@docs/superpowers/specs/2026-03-10-webkit-cert-bypass-design.md`:
- Around line 59-63: Add a language tag to the fenced code block that contains
the browser.cert_bypass examples (the block starting with "browser.cert_bypass
get → {"enabled": true|false}" and the two set lines) by changing the opening
triple backticks to include a tag like "text" so Markdownlint stops flagging it
and syntax highlighting is preserved.

In `@Sources/Panels/BrowserPanel.swift`:
- Around line 3820-3831: The unconditional server-trust bypass accepts all
NSURLAuthenticationMethodServerTrust challenges when
BrowserCertBypassSettings.isEnabled(), which is too broad; update the
conditional in the authentication challenge handler to also verify the host is a
development/loopback host or is present in an explicit allowlist before calling
completionHandler(.useCredential, URLCredential(trust: serverTrust)).
Specifically, keep the existing checks for NSURLAuthenticationMethodServerTrust
and serverTrust, but add a guard that inspects challenge.protectionSpace.host
(e.g., allow "localhost", "127.0.0.1", "::1", or configured dev hostnames) or
consults a BrowserCertBypassSettings.allowlist API, and only return
.useCredential when that host check passes.

In `@Sources/TerminalController.swift`:
- Around line 1984-1985: The dispatcher wires the method via the case
"browser.cert_bypass" and implements it in v2BrowserCertBypass, but
v2Capabilities() does not advertise "browser.cert_bypass", so clients
discovering methods via system.capabilities may not see it; update
v2Capabilities() to include "browser.cert_bypass" in the capability
list/response (the same collection returned by
v2Capabilities()/system.capabilities) so the advertised capabilities match the
implemented handler.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 99ae8758-dafc-445d-bbb9-0b69d8d7667a

📥 Commits

Reviewing files that changed from the base of the PR and between 0cdeb45 and 14d2322.

📒 Files selected for processing (7)
  • CLI/cmux.swift
  • Sources/Panels/BrowserPanel.swift
  • Sources/TerminalController.swift
  • Sources/cmuxApp.swift
  • docs/superpowers/plans/2026-03-10-webkit-cert-bypass.md
  • docs/superpowers/specs/2026-03-10-webkit-cert-bypass-design.md
  • tests_v2/test_browser_cert_bypass.py

Comment thread CLI/cmux.swift
Comment on lines +3906 to +3919
if subcommand == "cert-bypass" {
let action = subArgs.first ?? "get"
var params: [String: Any] = ["action": action]
if let value = subArgs.dropFirst().first {
params["value"] = value
}
let payload = try client.sendV2(method: "browser.cert_bypass", params: params)
if let enabled = payload["enabled"] as? Bool {
output(payload, fallback: enabled ? "true" : "false")
} else {
output(payload, fallback: "OK")
}
return
}

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 | 🟡 Minor

Validate cert-bypass arity before forwarding.

This branch silently ignores tokens after the first value. For example, cmux browser cert-bypass set true typo will still execute set true, which hides user mistakes and diverges from the documented get | set <true|false> contract.

🛠️ Suggested validation
         if subcommand == "cert-bypass" {
-            let action = subArgs.first ?? "get"
-            var params: [String: Any] = ["action": action]
-            if let value = subArgs.dropFirst().first {
-                params["value"] = value
-            }
+            let normalizedArgs = subArgs.map { $0.lowercased() }
+            let action = normalizedArgs.first ?? "get"
+            guard action == "get" || action == "set" else {
+                throw CLIError(message: "browser cert-bypass requires 'get' or 'set <true|false>'")
+            }
+            guard normalizedArgs.count <= 2 else {
+                throw CLIError(message: "browser cert-bypass accepts: get | set <true|false>")
+            }
+
+            var params: [String: Any] = ["action": action]
+            if action == "set" {
+                guard let value = normalizedArgs.dropFirst().first,
+                      parseBoolString(value) != nil else {
+                    throw CLIError(message: "browser cert-bypass set requires <true|false>")
+                }
+                params["value"] = value
+            } else if normalizedArgs.count > 1 {
+                throw CLIError(message: "browser cert-bypass get does not accept a value")
+            }
             let payload = try client.sendV2(method: "browser.cert_bypass", params: params)
             if let enabled = payload["enabled"] as? Bool {
                 output(payload, fallback: enabled ? "true" : "false")
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@CLI/cmux.swift` around lines 3906 - 3919, Validate the argument count for the
"cert-bypass" subcommand before calling client.sendV2: check subArgs and ensure
only allowed forms ("get" with exactly 0 extra tokens, or "set" followed by
exactly one value and no additional tokens); if the arity is invalid, emit a
clear error/usage message via output and return without calling client.sendV2.
Update the block that sets action/params (references: subArgs, action, params,
payload, client.sendV2, output) to perform this validation and only populate
params["value"] and call client.sendV2 when the argument count matches the
expected "set <true|false>" pattern.

Comment thread CLI/cmux.swift
viewport <width> <height>
geolocation|geo <latitude> <longitude>
offline <true|false>
cert-bypass [get | set <true|false>]

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 | 🟡 Minor

Localize the new browser help entry.

This adds new user-facing help text as raw English in Swift source. Please move it through the localization path and add the key to Resources/Localizable.xcstrings.

As per coding guidelines, "All user-facing strings must be localized using String(localized: "key.name", defaultValue: "English text") and added to Resources/Localizable.xcstrings."

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

In `@CLI/cmux.swift` at line 5054, The new help text "cert-bypass [get | set
<true|false>]" in CLI/cmux.swift is currently raw English; replace the literal
string with a localized call using String(localized: "cli.cert-bypass.help",
defaultValue: "cert-bypass [get | set <true|false>]") (or a similarly named key)
and then add that key and English value to Resources/Localizable.xcstrings;
ensure you update any call sites that display the help so they use the localized
string (search for the exact literal or the function that builds the help lines
in CLI/cmux.swift to locate where to substitute).

Comment on lines +230 to +240
**How callers invoke it via CLI:**
```bash
# Get current state
cmux browser.cert_bypass --action get

# Enable (session-only, does not persist)
cmux browser.cert_bypass --action set --value true

# Disable
cmux browser.cert_bypass --action set --value false
```

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 | 🟡 Minor

Use the actual CLI syntax in these examples.

These snippets are showing the socket method name and --action/--value params, but the user-facing CLI for this PR is cmux browser cert-bypass get|set <true|false>. Keeping the underscore form here will point readers at a command shape that doesn’t exist.

📝 Suggested fix
-# Get current state
-cmux browser.cert_bypass --action get
+# Get current state
+cmux browser cert-bypass get

-# Enable (session-only, does not persist)
-cmux browser.cert_bypass --action set --value true
+# Enable (session-only, does not persist)
+cmux browser cert-bypass set true

-# Disable
-cmux browser.cert_bypass --action set --value false
+# Disable
+cmux browser cert-bypass set false
-# Toggle at runtime (session-only)
-cmux browser.cert_bypass --action set --value true
+# Toggle at runtime (session-only)
+cmux browser cert-bypass set true

Also applies to: 350-359

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

In `@docs/superpowers/plans/2026-03-10-webkit-cert-bypass.md` around lines 230 -
240, The CLI examples use the old socket-style name and flags; update them to
the actual user-facing CLI syntax used by this PR: replace occurrences like
"cmux browser.cert_bypass --action get" and "cmux browser.cert_bypass --action
set --value true/false" with the correct forms "cmux browser cert-bypass get"
and "cmux browser cert-bypass set true" (and "cmux browser cert-bypass set
false"), and make the same changes for the other affected block (lines ~350-359)
so all examples show "cmux browser cert-bypass get|set <true|false>".

Comment on lines +59 to +63
```
browser.cert_bypass get → {"enabled": true|false} (reflects effective state)
browser.cert_bypass set true → {"enabled": true} (sets runtimeOverride, session only)
browser.cert_bypass set false → {"enabled": false} (sets runtimeOverride, session only)
```

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 | 🟡 Minor

Add a language tag to this fenced block.

Markdownlint is already flagging this block, and labeling it (text is enough here) will fix the warning and preserve syntax highlighting.

📝 Suggested fix
-```
+```text
 browser.cert_bypass get        → {"enabled": true|false}  (reflects effective state)
 browser.cert_bypass set true   → {"enabled": true}   (sets runtimeOverride, session only)
 browser.cert_bypass set false  → {"enabled": false}  (sets runtimeOverride, session only)

</details>

<details>
<summary>🧰 Tools</summary>

<details>
<summary>🪛 markdownlint-cli2 (0.21.0)</summary>

[warning] 59-59: Fenced code blocks should have a language specified

(MD040, fenced-code-language)

</details>

</details>

<details>
<summary>🤖 Prompt for AI Agents</summary>

Verify each finding against the current code and only fix it if needed.

In @docs/superpowers/specs/2026-03-10-webkit-cert-bypass-design.md around lines
59 - 63, Add a language tag to the fenced code block that contains the
browser.cert_bypass examples (the block starting with "browser.cert_bypass get →
{"enabled": true|false}" and the two set lines) by changing the opening triple
backticks to include a tag like "text" so Markdownlint stops flagging it and
syntax highlighting is preserved.


</details>

<!-- fingerprinting:phantom:poseidon:grasshopper -->

<!-- This is an auto-generated comment by CodeRabbit -->

Comment on lines +3820 to +3831
// When certificate bypass is active (--ignore-certificate-errors or browser.cert_bypass set true),
// unconditionally trust server certificates. This covers self-signed and unknown-root certs
// common in local development (e.g. https://localhost:8443). Only server trust challenges
// are bypassed; client cert and other challenge types still use default handling.
if challenge.protectionSpace.authenticationMethod == NSURLAuthenticationMethodServerTrust,
BrowserCertBypassSettings.isEnabled(),
let serverTrust = challenge.protectionSpace.serverTrust {
#if DEBUG
dlog("browser.cert_bypass: accepting server trust for \(challenge.protectionSpace.host)")
#endif
completionHandler(.useCredential, URLCredential(trust: serverTrust))
return

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

Scope the bypass to development hosts.

With the current condition, enabling this flag accepts every NSURLAuthenticationMethodServerTrust challenge, not just self-signed localhost/dev endpoints. Because the default can also come from UserDefaults, this silently disables certificate validation for arbitrary remote sites too, which is broader than the stated local-dev use case. Please gate this with an explicit loopback/dev-host check or a dedicated allowlist before returning .useCredential.

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

In `@Sources/Panels/BrowserPanel.swift` around lines 3820 - 3831, The
unconditional server-trust bypass accepts all
NSURLAuthenticationMethodServerTrust challenges when
BrowserCertBypassSettings.isEnabled(), which is too broad; update the
conditional in the authentication challenge handler to also verify the host is a
development/loopback host or is present in an explicit allowlist before calling
completionHandler(.useCredential, URLCredential(trust: serverTrust)).
Specifically, keep the existing checks for NSURLAuthenticationMethodServerTrust
and serverTrust, but add a guard that inspects challenge.protectionSpace.host
(e.g., allow "localhost", "127.0.0.1", "::1", or configured dev hostnames) or
consults a BrowserCertBypassSettings.allowlist API, and only return
.useCredential when that host check passes.

Comment on lines +1984 to +1985
case "browser.cert_bypass":
return v2Result(id: id, self.v2BrowserCertBypass(params: params))

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

Advertise browser.cert_bypass in system.capabilities.

Line 1984 wires the method into the dispatcher, but v2Capabilities() still omits it. Clients that discover methods through system.capabilities will assume this API is unavailable and never call it.

Suggested follow-up
         "browser.input_mouse",
         "browser.input_keyboard",
         "browser.input_touch",
+        "browser.cert_bypass",
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@Sources/TerminalController.swift` around lines 1984 - 1985, The dispatcher
wires the method via the case "browser.cert_bypass" and implements it in
v2BrowserCertBypass, but v2Capabilities() does not advertise
"browser.cert_bypass", so clients discovering methods via system.capabilities
may not see it; update v2Capabilities() to include "browser.cert_bypass" in the
capability list/response (the same collection returned by
v2Capabilities()/system.capabilities) so the advertised capabilities match the
implemented handler.

Comment on lines +7329 to +7358
private func v2BrowserCertBypass(params: [String: Any]) -> V2CallResult {
let action = (params["action"] as? String) ?? (params["args"] as? [String])?.first ?? ""
switch action {
case "get":
var result: V2CallResult = .err(code: "internal", message: "Unexpected", data: nil)
v2MainSync {
result = .ok(["enabled": BrowserCertBypassSettings.isEnabled()])
}
return result
case "set":
let rawValue = (params["args"] as? [String])?.dropFirst().first
?? (params["value"] as? String)
?? (params["enabled"] as? Bool).map { $0 ? "true" : "false" }
guard let rawValue else {
return .err(code: "invalid_params", message: "Missing value: use 'set true' or 'set false'", data: nil)
}
switch rawValue.lowercased() {
case "true", "1", "yes":
v2MainSync { BrowserCertBypassSettings.runtimeOverride = true }
return .ok(["enabled": true])
case "false", "0", "no":
v2MainSync { BrowserCertBypassSettings.runtimeOverride = false }
return .ok(["enabled": false])
default:
return .err(code: "invalid_params", message: "Invalid value '\(rawValue)': use 'true' or 'false'", data: nil)
}
default:
return .err(code: "invalid_params", message: "Unknown action '\(action)': use 'get' or 'set'", data: nil)
}
}

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 | 🟡 Minor

Localize the new response messages.

This handler adds new user-visible socket/CLI messages as hard-coded English strings. Please route them through String(localized:..., defaultValue:...) and add the keys to Resources/Localizable.xcstrings.

As per coding guidelines, "All user-facing strings must be localized using String(localized: "key.name", defaultValue: "English text") and added to Resources/Localizable.xcstrings."

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

No issues found across 7 files


Since this is your first cubic review, here's how it works:

  • cubic automatically reviews your code and comments on bugs and improvements
  • Teach cubic by replying to its comments. cubic learns from your replies and gets better over time
  • Add one-off context when rerunning by tagging @cubic-dev-ai with guidance or docs links (including llms.txt)
  • Ask questions if you need clarification on any suggestion

@teamleaderleo teamleaderleo added area: browser The embedded browser, web surfaces, inline VS Code S3: minor Wrong behavior with a workaround labels Sep 30, 2026

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area: browser The embedded browser, web surfaces, inline VS Code S3: minor Wrong behavior with a workaround

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants