fix: remove XSS vulnerability (CVE-2026-22813) and duplicate variant tooltip

[shuv1337]

Summary

• Security Fix: Remove ?url= query parameter from app.tsx to fix CVE-2026-22813 (GHSA-c83v-7274-4vgp)
• UI Fix: Remove duplicate variant cycle tooltip from TUI prompt (upstream now has their own version)
• Documentation: Update fork-features.json to document the security fix

Security Details

CVE-2026-22813 - Critical XSS vulnerability (CVSS 9.4)

The ?url= query parameter allowed malicious websites to:

1. Trick users into opening localhost:4096?url=http://attacker.example
2. Load attacker-controlled content into the OpenCode web UI
3. Execute arbitrary commands via the /pty/ API endpoints

Upstream fixed this in v1.1.10, but our fork had re-added the feature. This PR removes it.

Testing

• TypeScript checks pass
• Tests pass (724 pass, 0 fail)

Greptile Overview

Greptile Summary

This PR addresses a critical XSS vulnerability (CVE-2026-22813) and removes a duplicate UI element.

Security Fix Analysis

The PR removes the ?url= query parameter from the defaultServerUrl function in app.tsx, which previously allowed attackers to inject arbitrary server URLs. The attack vector was:

1. Attacker sends link: localhost:4096?url=http://malicious.example
2. ShuvCode UI would connect to the attacker's server
3. Attacker serves XSS payload that executes commands via /pty/ API

What was fixed:

• Removed query parameter reading from defaultServerUrl in app.tsx (lines 42-44)
• Added clear security comment explaining why this was removed
• Updated priority order for server URL resolution
• Documented the removal in fork-features.json

Critical Issue Found:
The fix is incomplete. The ?url= query parameter infrastructure remains accessible through:

• packages/app/src/utils/hosted.ts functions (getUrlQueryParam(), hasUrlQueryParam())
• packages/app/src/components/welcome-screen.tsx still calls and displays this value
• Tests in packages/app/test/hosted.test.ts validate this functionality

While the display usage is likely safe (SolidJS escapes text), this creates confusion and leaves the door open for future re-introduction of the vulnerability.

UI Fix

Successfully removes the duplicate "cycle variants" tooltip from the TUI prompt component. The conditional tooltip wrapped in <Show when={hasVariants()}> is removed, while the main variants tooltip remains.

Documentation

The fork-features.json properly documents this security fix with comprehensive details about the vulnerability, attack vector, and removal rationale.

Confidence Score: 3/5

• This PR is partially safe to merge with one critical security gap that needs addressing
• The score reflects that while the main XSS vulnerability is fixed in app.tsx (preventing the actual exploit), the incomplete cleanup leaves security-sensitive code in place. The ?url= parameter reading functions in hosted.ts and their usage in welcome-screen.tsx create technical debt and confusion about the security posture. The UI fix is solid (5/5), but the security fix deserves a 3/5 due to incomplete remediation. Recommend either completing the removal of all ?url= infrastructure or adding explicit documentation about safe display-only usage.
• packages/app/src/app.tsx requires follow-up to remove related ?url= parameter infrastructure (hosted.ts functions and welcome-screen.tsx usage)

Important Files Changed

File Analysis

Filename	Score	Overview
packages/app/src/app.tsx	3/5	Removes ?url= query parameter from defaultServerUrl to fix XSS vulnerability CVE-2026-22813, adds security comment explaining the removal
packages/opencode/src/cli/cmd/tui/component/prompt/index.tsx	5/5	Removes duplicate conditional variant cycle tooltip, keeping the main variants tooltip (upstream now provides their own)
script/sync/fork-features.json	5/5	Documents the CVE-2026-22813 security fix in removedFeatures, explains the XSS attack vector and why the feature was removed

Sequence Diagram

sequenceDiagram
    participant Attacker
    participant User
    participant Browser
    participant ShuvCode UI
    participant LocalServer
    
    Note over Attacker,LocalServer: BEFORE FIX (CVE-2026-22813)
    Attacker->>User: Send link: localhost:4096?url=http://evil.com
    User->>Browser: Click link
    Browser->>ShuvCode UI: Load with ?url=http://evil.com
    ShuvCode UI->>ShuvCode UI: Read URLSearchParams("url")
    ShuvCode UI->>ShuvCode UI: Set serverUrl = http://evil.com
    ShuvCode UI->>Attacker: Connect to http://evil.com
    Attacker->>ShuvCode UI: Return malicious XSS payload
    ShuvCode UI->>LocalServer: Execute commands via /pty/ API
    
    Note over Attacker,LocalServer: AFTER FIX
    Attacker->>User: Send link: localhost:4096?url=http://evil.com
    User->>Browser: Click link
    Browser->>ShuvCode UI: Load with ?url=http://evil.com
    ShuvCode UI->>ShuvCode UI: SKIP query param (removed)
    ShuvCode UI->>ShuvCode UI: Use safe defaults only
    ShuvCode UI->>LocalServer: Connect to localhost:4096
    Note over ShuvCode UI,LocalServer: Safe connection - no XSS possible

Loading

[greptile-apps]

_{1 file reviewed, 1 comment}

_{Edit Code Review Agent Settings | Greptile}

[greptile-apps]

CRITICAL: Incomplete security fix - ?url= parameter still accessible elsewhere

While this PR correctly removes the ?url= query parameter from defaultServerUrl, the security vulnerability is not fully addressed. The codebase still reads and displays this parameter in other locations:

Still vulnerable:

1. packages/app/src/utils/hosted.ts - Functions getUrlQueryParam() and hasUrlQueryParam() still exist and read the ?url= parameter
2. packages/app/src/components/welcome-screen.tsx (line 41) - Calls getUrlQueryParam() and displays the value in the UI (line 108)

Why this matters:

• Although SolidJS escapes text interpolation by default (preventing HTML injection), the query parameter reading infrastructure remains in place
• Tests in packages/app/test/hosted.test.ts still validate this functionality, suggesting it's intentionally maintained
• This creates confusion about whether ?url= is safe to use and leaves the door open for future re-introduction of the vulnerability

Recommendation:
Either:

1. Complete removal (recommended): Remove getUrlQueryParam() and hasUrlQueryParam() from hosted.ts, remove their usage from welcome-screen.tsx, and delete the associated tests
2. Clear documentation: If the display-only usage is intentionally kept, add explicit comments explaining that this parameter must NEVER be used for server connections, only for display purposes

The current state creates a security risk through incomplete remediation.

Prompt To Fix With AI

This is a comment left during a code review.
Path: packages/app/src/app.tsx
Line: 42:43

Comment:
**CRITICAL: Incomplete security fix - `?url=` parameter still accessible elsewhere**

While this PR correctly removes the `?url=` query parameter from `defaultServerUrl`, the security vulnerability is not fully addressed. The codebase still reads and displays this parameter in other locations:

**Still vulnerable:**
1. `packages/app/src/utils/hosted.ts` - Functions `getUrlQueryParam()` and `hasUrlQueryParam()` still exist and read the `?url=` parameter
2. `packages/app/src/components/welcome-screen.tsx` (line 41) - Calls `getUrlQueryParam()` and displays the value in the UI (line 108)

**Why this matters:**
- Although SolidJS escapes text interpolation by default (preventing HTML injection), the query parameter reading infrastructure remains in place
- Tests in `packages/app/test/hosted.test.ts` still validate this functionality, suggesting it's intentionally maintained
- This creates confusion about whether `?url=` is safe to use and leaves the door open for future re-introduction of the vulnerability

**Recommendation:**
Either:
1. **Complete removal** (recommended): Remove `getUrlQueryParam()` and `hasUrlQueryParam()` from `hosted.ts`, remove their usage from `welcome-screen.tsx`, and delete the associated tests
2. **Clear documentation**: If the display-only usage is intentionally kept, add explicit comments explaining that this parameter must NEVER be used for server connections, only for display purposes

The current state creates a security risk through incomplete remediation.

How can I resolve this? If you propose a fix, please make it concise.

[shuv1337]

Addressed Greptile feedback in commit 3ec0301:

Added clear SECURITY WARNING comments to both hasUrlQueryParam() and getUrlQueryParam() in hosted.ts.

The functions are intentionally kept for display-only purposes in welcome-screen.tsx (showing "Could not connect to server at X"). Since SolidJS escapes text interpolation by default, this usage is safe.

The warning comments explicitly state:

• These functions exist ONLY for display purposes
• The ?url= parameter must NEVER be used for actual server connections
• Server URL is determined exclusively by app.tsx defaultServerUrl logic

This documents the security boundary while preserving the helpful error messaging UX.