fix(web): treat configured urls with uppercase schemes as secure - #8005
Conversation
resolveConfiguredPrimaryTarget classified VITE_WS_URL/VITE_HTTP_URL with case-sensitive startsWith checks while the URL parser accepts any scheme casing. A configured "WSS://host" was therefore swapped to plaintext http (and "HTTPS://host" to plaintext ws), silently downgrading TLS for the affected base URL. ox-alpha via opencode
|
@codex review |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
Included review availability: Your plan provides up to 10 included reviews per hour; 9 remain after this review. 📝 WalkthroughWalkthroughThe target URL logic now normalizes configured schemes before detection. Tests cover uppercase ChangesSecure scheme normalization
Estimated code review effort: 2 (Simple) | ~10 minutes Merge Risk: ⚪ Minimal · up to This localized change prevents uppercase HTTPS/WSS configuration values from being downgraded to insecure derived URLs, with regression tests covering both cases. No actionable merge-blocking risk remains beyond normal checks and review. Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
Codex Review: Didn't find any major issues. You're on a roll. Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
|
@coderabbitai review |
✅ Action performedReview finished.
|
ApprovabilityVerdict: Skipped Macroscope did not run approvability analysis for this PR. Macroscope could not determine whether this PR modifies its approvability configuration, so the PR was not approved automatically. A PR that may change the rules that govern approval is never approved automatically. |
…gdotgg#8005) Co-authored-by: Simone <185146821+Lucenx9@users.noreply.github.com>
## What's Changed * fix(web): treat configured urls with uppercase schemes as secure by @Lucenx9 in pingdotgg/t3code#8005 * fix(desktop): keep release notes visible while downloading by @extoci in pingdotgg/t3code#6412 **Full Changelog**: pingdotgg/t3code@v0.0.34-nightly.20260823.1171...v0.0.34-nightly.20260824.1172 Upstream release: https://github.com/pingdotgg/t3code/releases/tag/v0.0.34-nightly.20260824.1172
Fast-forwards main from f70eeee to 7c6163c (16 commits, 65 files) and merges it into dev. Small in commits, heavy in fork contact: 38 of those files carried fork delta, but that produced only 7 conflicts across 9 hunks. Upstream lands a redesigned skills menu behind a new showSkillsInSlashMenu setting (pingdotgg#8009), Codex app access approval prompts (pingdotgg#8058), usage views that hide providers with no usage (pingdotgg#7563), four timeline and tool-call fixes (pingdotgg#7971, pingdotgg#8052, pingdotgg#7999, pingdotgg#8006), uppercase URL schemes treated as secure (pingdotgg#8005), and a CI and Windows packaging speed pass (pingdotgg#7975). Notable resolutions. Upstream's new slash-menu skill helpers auto-merged reading raw selectedProviderStatus.skills. They now take the fork's resolved effectiveProviderSkills, so the $ picker and the / menu still agree about project-scoped skills on a thread whose cwd differs from the server's. That same raw read has landed in a composer three syncs running and compiles cleanly every time. ComposerCommandMenu keeps the fork's Manual marker beside upstream's new source badge. ChatView.logic takes upstream's MessageId but not the ProjectId it re-added, because the fork's v2 storage key is scoped per environment. ChatView keeps its single route-scoped visit selector and thread-keyed background stop while adopting upstream's running-turn anchor release. One thing to know about acceptAlways: it is a new persisted approval decision, so an approval answered on this build does not decode on an older one. vp check, typecheck, and focused tests pass. The full suite is green with no failures at all this time, PortScanner included. lint:mobile was required because the @legendapp/list patch changed, and passes. Reviewed and merged by Claude Opus 5 in Claude Code, with gpt-5.6-sol subagents for the behavioral reviews and post-merge audits.
…gdotgg#8005) Co-authored-by: Simone <185146821+Lucenx9@users.noreply.github.com> (cherry picked from commit 69e5ad8)
Problem
resolveConfiguredPrimaryTargetclassifiesVITE_WS_URL/VITE_HTTP_URLwith case-sensitivestartsWith("wss:")/startsWith("https:")checks, while the URL parser folds any scheme casing (new URL("WSS://host")is valid). A configuredWSS://hosttherefore falls into the plaintext branch and the derived HTTP base URL becomeshttp://host— silently downgrading TLS for that deployment (same forHTTPS://host→ plainws://).Fix
Both raw-string scheme checks are case-folded before classification, so uppercase variants land in the secure branch like their lowercase equivalents.
Verification
bootstrap.test.ts(WSS://andHTTPS://configured URLs must resolve to https/wss targets); both fail against the old code and pass now.environments/primarysuite green,tsgo --noEmitclean.ox-alpha via opencode
Note
Fix
resolveConfiguredPrimaryTargetto treat uppercaseHTTPS/WSSschemes as secureWhen only one of
VITE_HTTP_URLorVITE_WS_URLis set, the other is derived from it. Previously, uppercase secure schemes likeHTTPS:orWSS:were not recognized as secure, causing the derived URL to downgrade tohttporws. The fix in target.ts normalizes the scheme viatoLowerCase()before the security check. Tests in bootstrap.test.ts cover uppercaseHTTPSandWSSenv values.Macroscope summarized 83fe887.
Note
Medium Risk
Touches how configured environment URLs are classified as TLS vs plaintext, which previously could silently downgrade derived http/ws bases. The change itself is a small, well-tested case-fold.
Overview
Stops a silent TLS downgrade when only one of
VITE_HTTP_URLorVITE_WS_URLis set with an uppercase scheme (HTTPS:///WSS://).resolveConfiguredPrimaryTargetnow case-folds the raw scheme before deciding whether to derivehttps/wssvshttp/ws. Regression tests cover both uppercase env values resolving to secure targets.Reviewed by Cursor Bugbot for commit 83fe887. Bugbot is set up for automated code reviews on this repo. Configure here.
Summary by CodeRabbit
HTTPSandWSSURL schemes.httpsandwsstargets instead of being downgraded.