Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
shuv1337 merged 2 commits into from
Jan 13, 2026
Merged

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

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
a09d300
fix: remove XSS vulnerability and duplicate variant tooltip
shuv1337 Jan 13, 2026
3ec0301
docs: add security warnings to hosted.ts url query param functions
shuv1337 Jan 13, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 7 additions & 8 deletions packages/app/src/app.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,26 +39,25 @@ declare global {
}

const defaultServerUrl = iife(() => {
// 1. Query parameter (highest priority)
const param = new URLSearchParams(document.location.search).get("url")
if (param) return param
// NOTE: The ?url= query parameter was intentionally removed due to CVE-2026-22813 (GHSA-c83v-7274-4vgp)
// Allowing arbitrary server URLs via query params enables XSS attacks on localhost:4096
Comment on lines +42 to +43
Copy link

@greptile-apps greptile-apps bot Jan 13, 2026

Choose a reason for hiding this comment

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

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.


// 2. Configured server URL (from desktop settings)
// 1. Configured server URL (from desktop settings)
if (window.__OPENCODE__?.serverUrl) return window.__OPENCODE__.serverUrl

// 3. Known production hosts -> localhost (same as upstream + shuv.ai)
// 2. Known production hosts -> localhost (same as upstream + shuv.ai)
if (location.hostname.includes("opencode.ai") || location.hostname.includes("shuv.ai")) return "http://localhost:4096"

// 4. Desktop app (Tauri) with injected port
// 3. Desktop app (Tauri) with injected port
if (window.__SHUVCODE__?.port) return `http://127.0.0.1:${window.__SHUVCODE__.port}`
if (window.__OPENCODE__?.port) return `http://127.0.0.1:${window.__OPENCODE__.port}`

// 5. Dev mode -> same-origin so Vite proxy handles LAN access + CORS
// 4. Dev mode -> same-origin so Vite proxy handles LAN access + CORS
if (import.meta.env.DEV) {
return `http://${import.meta.env.VITE_OPENCODE_SERVER_HOST ?? "localhost"}:${import.meta.env.VITE_OPENCODE_SERVER_PORT ?? "4096"}`
}

// 6. Default -> same origin (production web command)
// 5. Default -> same origin (production web command)
return window.location.origin
})

Expand Down
5 changes: 0 additions & 5 deletions packages/opencode/src/cli/cmd/tui/component/prompt/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1122,11 +1122,6 @@ export function Prompt(props: PromptProps) {
<text fg={theme.text}>
{keybind.print("command_list")} <span style={{ fg: theme.textMuted }}>commands</span>
</text>
<Show when={hasVariants()}>
<text fg={theme.text}>
{keybind.print("variant_cycle")} <span style={{ fg: theme.textMuted }}>cycle variants</span>
</text>
</Show>
</Match>
<Match when={store.mode === "shell"}>
<text fg={theme.text}>
Expand Down
13 changes: 13 additions & 0 deletions script/sync/fork-features.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -120,6 +120,19 @@
"Session migration for linked worktrees"
],
"note": "Upstream uses Project.sandboxes array to track worktrees. Main worktree is always Project.worktree."
},
{
"pr": "CVE-2026-22813",
"title": "Query parameter server URL override",
"author": "fork",
"removedDate": "2026-01-12",
"reason": "Critical XSS vulnerability (GHSA-c83v-7274-4vgp, CVE-2026-22813) - malicious websites could execute commands via ?url= parameter. Upstream fixed in v1.1.10 by removing this feature.",
"filesModified": ["packages/app/src/app.tsx"],
"codeRemoved": [
"const param = new URLSearchParams(document.location.search).get('url')",
"if (param) return param"
],
"note": "The ?url= query parameter allowed arbitrary server URL override, enabling XSS on localhost:4096. Attackers could inject malicious server URLs that return XSS payloads, then use /pty/ API to execute arbitrary commands."
}
],
"apiDependencies": [
Expand Down