Repository navigation
fix(cli): link https in --print-handoff when the web UI is served over TLS - #2773
Merged
Merged
Conversation
…r TLS buildHandoff() hardcoded http:// for the deep link, so with both INSPECTOR_TLS_KEY_PATH and INSPECTOR_TLS_CERT_PATH set (#2446) the link pointed at a listener that serves https only. It now follows the same both-set, blank-is-unset rule as resolveTlsConfig(). A half pair stays http, since the web server refuses to start on one. Closes #2770 Co-Authored-By: Claude Opus 5.5 (1M context) <[email protected]> Signed-off-by: cliffhall <[email protected]>
There was a problem hiding this comment.
🟢 Approval recommended
The focused change matches the intended TLS behavior, includes regression coverage, and has no identified blocking issues.
No open issues 🎉
What changed in this PR
Fixes the CLI’s OAuth handoff link for HTTPS web listeners, addressing #2770 without changing port forwarding.
Changes:
- Selects HTTPS when both TLS paths are non-blank; otherwise retains HTTP.
- Adds regression tests for complete, partial, and blank TLS configuration.
- Documents HTTPS handoff behavior.
| File | Description |
|---|---|
clients/cli/src/cli.ts |
Selects the handoff link scheme from TLS configuration. |
clients/cli/README.md |
Documents when handoff links use HTTPS. |
clients/cli/__tests__/stored-auth.test.ts |
Adds four TLS scheme regression cases. |
Review effort: Balanced
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #2770
Problem
With
INSPECTOR_TLS_KEY_PATHandINSPECTOR_TLS_CERT_PATHboth set (#2446), the web UI listener serves https only.buildHandoff()inclients/cli/src/cli.tsstill built--print-handoff's deep link ashttp://…, so the link it printed could not load. Copilot found this while reviewing the v2.12.0 milestone merge (#2769).Change
clients/cli/src/cli.ts: the deep link's scheme ishttpswhen both TLS paths are set and non-blank, the same rule asresolveTlsConfig()inclients/web/server/tls.ts, andhttpotherwise. A half pair stayshttp, because the web server refuses to start on one, so there is no https listener to link. The port-forward command is unchanged: it forwards ports, and the sandbox and app-origin listeners stay plain http under TLS by design.clients/cli/README.md: the--print-handoffrow says when the link ishttps.Tests
Four new
--print-handoffcases instored-auth.test.ts: both paths set (https), key only, cert only, and a blank key path (http). All 69 tests in the file pass. Mutation check: putting back the hardcodedhttp://fails exactly the both-set case (1 failed | 68 passed).npm run local:gate: green (exit 0).No UI change, so no screenshots.
🤖 Generated with Claude Code