From 34179162df7c623fe016ded467668dce9ab5523e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pascal=20Andr=C3=A9?= Date: Fri, 21 Aug 2026 00:28:09 +0200 Subject: [PATCH] fix(v2): close current desktop parity gaps Load continuation message pages with the native cursor alone, hide deprecated models, and tolerate failed plugin records that do not carry an ID. Match official Desktop URL handling by limiting native external openers to web and mail schemes. Connect wildcard-bound shared services through loopback while preserving strict authentication and non-loopback rejection. Remove the inactive unrestricted Tauri asset scope and add focused regression coverage for cursor requests, model/plugin projection, URL schemes, and wildcard service discovery. Validated with focused UI/server tests, all 158 Electron native tests, all 104 Tauri tests, package typechecks, the UI production build, Rust formatting, and diff checks. --- .../electron/main/navigation-security.test.ts | 2 + .../electron/main/navigation-security.ts | 2 +- .../workspaces/host-opencode-service.test.ts | 17 ++++++++ .../src/workspaces/opencode-cli-service.ts | 9 +++-- .../src/workspaces/opencode-service.test.ts | 14 +++++++ .../server/src/workspaces/opencode-service.ts | 4 +- .../server/src/workspaces/service-state.ts | 1 + packages/tauri-app/src-tauri/src/main.rs | 40 +++++++++++++++---- packages/tauri-app/src-tauri/tauri.conf.json | 5 --- .../lib/hooks/use-instance-metadata.test.ts | 2 +- .../ui/src/lib/hooks/use-instance-metadata.ts | 2 +- packages/ui/src/stores/session-api.ts | 3 +- .../stores/session-request-authority.test.ts | 10 +++-- 13 files changed, 86 insertions(+), 25 deletions(-) diff --git a/packages/electron-app/electron/main/navigation-security.test.ts b/packages/electron-app/electron/main/navigation-security.test.ts index f74217f9..d06964ab 100644 --- a/packages/electron-app/electron/main/navigation-security.test.ts +++ b/packages/electron-app/electron/main/navigation-security.test.ts @@ -17,6 +17,8 @@ test("navigation allows registered origins and only the exact loading file", () assert.equal(decideNavigation("file:///opt/codenomad/index.html", origins, loading), "deny") assert.equal(decideNavigation("https://renderer.example/workspace", origins, loading), "allow") assert.equal(decideNavigation("https://outside.example/", origins, loading), "external") + assert.equal(decideNavigation("mailto:hello@example.com", origins, loading), "external") + assert.equal(decideNavigation("vscode://file/C:/workspace", origins, loading), "deny") assert.equal(decideNavigation("not a URL", origins, loading), "deny") assert.equal(decideNavigation("http://localhost:5173/loading.html", [], "http://localhost:5173/loading.html"), "allow") }) diff --git a/packages/electron-app/electron/main/navigation-security.ts b/packages/electron-app/electron/main/navigation-security.ts index df567f73..175c9d8c 100644 --- a/packages/electron-app/electron/main/navigation-security.ts +++ b/packages/electron-app/electron/main/navigation-security.ts @@ -21,5 +21,5 @@ export function decideNavigation( if (url.protocol === "http:" || url.protocol === "https:") { return allowedOrigins.includes(url.origin) ? "allow" : "external" } - return url.protocol === "file:" || url.protocol === "data:" || url.protocol === "javascript:" ? "deny" : "external" + return url.protocol === "mailto:" ? "external" : "deny" } diff --git a/packages/server/src/workspaces/host-opencode-service.test.ts b/packages/server/src/workspaces/host-opencode-service.test.ts index c2a4a820..174cd99a 100644 --- a/packages/server/src/workspaces/host-opencode-service.test.ts +++ b/packages/server/src/workspaces/host-opencode-service.test.ts @@ -51,6 +51,23 @@ describe("HostOpenCodeService", () => { await assert.rejects(malformed.discover(), /multiline/) }) + it("connects to wildcard services through loopback", async () => { + let healthUrl = "" + const service = createService([], {}, { + execFile: async (_file, args) => ({ + stdout: args.at(-1) === "password" ? "password\n" : "http://0.0.0.0:4321\n", + stderr: "", + }), + fetch: async (input) => { + healthUrl = String(input) + return Response.json({ healthy: true, version: "2.0.0", pid: 1 }) + }, + }) + + assert.equal((await service.discover())?.url, "http://127.0.0.1:4321/") + assert.equal(healthUrl, "http://127.0.0.1:4321/api/health") + }) + it("redacts startup environment values from failures and hashes identity", async () => { const secret = "DO_NOT_LEAK" const service = createService([], { TOKEN: secret }, { diff --git a/packages/server/src/workspaces/opencode-cli-service.ts b/packages/server/src/workspaces/opencode-cli-service.ts index a22f29e2..8466af28 100644 --- a/packages/server/src/workspaces/opencode-cli-service.ts +++ b/packages/server/src/workspaces/opencode-cli-service.ts @@ -62,8 +62,8 @@ export class OpenCodeCliService implements OpenCodeServiceLifecycle { return this.endpoint(url, deadlineAt) } - private async endpoint(url: string, deadlineAt: number): Promise { - this.assertServiceUrl(url) + private async endpoint(value: string, deadlineAt: number): Promise { + const url = this.assertServiceUrl(value) const password = this.singleLine( await this.run(["service", "get", "password"], false, deadlineAt), "password", @@ -153,9 +153,11 @@ export class OpenCodeCliService implements OpenCodeServiceLifecycle { } } - private assertServiceUrl(value: string): void { + private assertServiceUrl(value: string): string { let url: URL + let wildcard = false try { + wildcard = new URL(value).hostname === "0.0.0.0" url = assertLoopbackServiceUrl(value) } catch { throw new Error(`${this.options.label} OpenCode service returned an invalid loopback URL`) @@ -163,6 +165,7 @@ export class OpenCodeCliService implements OpenCodeServiceLifecycle { if (/[^\S\r\n]|[\x00-\x1f\x7f]/.test(value) || url.username || url.password || url.pathname !== "/" || url.search || url.hash) { throw new Error(`${this.options.label} OpenCode service returned an invalid loopback URL`) } + return wildcard ? url.toString() : value } private singleLine(value: string, label: string): string { diff --git a/packages/server/src/workspaces/opencode-service.test.ts b/packages/server/src/workspaces/opencode-service.test.ts index 67aa6085..b656119d 100644 --- a/packages/server/src/workspaces/opencode-service.test.ts +++ b/packages/server/src/workspaces/opencode-service.test.ts @@ -85,6 +85,20 @@ describe("OpenCodeSharedService", () => { }))), /must be loopback/) }) + it("normalizes wildcard lifecycle endpoints", async () => { + let baseUrl = "" + const service = createService({ + makeClient: (options) => { + baseUrl = options.baseUrl + return {} as OpenCodeClient + }, + }) + const wildcard = { ...endpoint, url: "http://0.0.0.0:4321" } + + assert.equal((await service.endpoint(lifecycleOptions("host:wildcard", lifecycleFor(wildcard)))).url, "http://127.0.0.1:4321/") + assert.equal(baseUrl, "http://127.0.0.1:4321/") + }) + it("formats auth, validates locations, and evicts through the official debug API", async () => { let clientHeaders: HeadersInit | undefined let evicted: unknown diff --git a/packages/server/src/workspaces/opencode-service.ts b/packages/server/src/workspaces/opencode-service.ts index 84b9e313..4328b5f8 100644 --- a/packages/server/src/workspaces/opencode-service.ts +++ b/packages/server/src/workspaces/opencode-service.ts @@ -150,7 +150,9 @@ export class OpenCodeSharedService { } private createConnection(endpoint: Endpoint, generation: number): ServiceConnection { - assertLoopbackServiceUrl(endpoint.url) + const wildcard = new URL(endpoint.url).hostname === "0.0.0.0" + const url = assertLoopbackServiceUrl(endpoint.url) + if (wildcard) endpoint = { ...endpoint, url: url.toString() } const connection = { endpoint, client: this.dependencies.makeClient({ diff --git a/packages/server/src/workspaces/service-state.ts b/packages/server/src/workspaces/service-state.ts index 5d9b1333..55c2cded 100644 --- a/packages/server/src/workspaces/service-state.ts +++ b/packages/server/src/workspaces/service-state.ts @@ -5,6 +5,7 @@ export function assertLoopbackServiceUrl(value: string): URL { if (url.protocol !== "http:" && url.protocol !== "https:") { throw new Error(`Unsupported OpenCode service protocol: ${url.protocol}`) } + if (url.hostname === "0.0.0.0") url.hostname = "127.0.0.1" const hostname = url.hostname.replace(/^\[|\]$/g, "").toLowerCase() const ipVersion = isIP(hostname) const loopback = hostname === "localhost" diff --git a/packages/tauri-app/src-tauri/src/main.rs b/packages/tauri-app/src-tauri/src/main.rs index 18319d16..bfaf23ca 100644 --- a/packages/tauri-app/src-tauri/src/main.rs +++ b/packages/tauri-app/src-tauri/src/main.rs @@ -379,18 +379,24 @@ fn should_allow_registered_origin(registered_origin: Option<&str>, url: &Url) -> should_allow_internal(url) } +fn should_open_external_url(url: &Url) -> bool { + matches!(url.scheme(), "http" | "https" | "mailto") +} + fn intercept_navigation(webview: &Webview, url: &Url) -> bool { let window_label = webview.label().to_string(); if should_allow_window_origin(&webview.app_handle(), &window_label, url) { return true; } - if let Err(err) = webview - .app_handle() - .opener() - .open_url(url.as_str(), None::<&str>) - { - eprintln!("[tauri] failed to open external link {}: {}", url, err); + if should_open_external_url(url) { + if let Err(err) = webview + .app_handle() + .opener() + .open_url(url.as_str(), None::<&str>) + { + eprintln!("[tauri] failed to open external link {}: {}", url, err); + } } false } @@ -1568,8 +1574,8 @@ fn build_about_metadata(version: &str, include_update_link: bool) -> AboutMetada mod menu_tests { use super::{ build_about_metadata, is_allowed_local_origin, require_http_url, run_update_with_fallback, - should_allow_registered_origin, should_recreate_remote_window, RemoteProfileIdentity, - WakeLockState, RELEASES_URL, REMOTE_WINDOW_CONTEXT_SCRIPT, + should_allow_registered_origin, should_open_external_url, should_recreate_remote_window, + RemoteProfileIdentity, WakeLockState, RELEASES_URL, REMOTE_WINDOW_CONTEXT_SCRIPT, }; use serde_json::json; use std::sync::atomic::{AtomicBool, Ordering}; @@ -1706,6 +1712,24 @@ mod menu_tests { } } + #[test] + fn external_navigation_allows_only_web_and_mail_urls() { + for value in [ + "https://example.com", + "http://example.com", + "mailto:hello@example.com", + ] { + assert!(should_open_external_url(&Url::parse(value).unwrap())); + } + for value in [ + "vscode://file/C:/workspace", + "ms-settings:privacy", + "unknown:target", + ] { + assert!(!should_open_external_url(&Url::parse(value).unwrap())); + } + } + #[test] fn remote_window_reuse_requires_exact_profile_identity() { let direct = RemoteProfileIdentity::Direct; diff --git a/packages/tauri-app/src-tauri/tauri.conf.json b/packages/tauri-app/src-tauri/tauri.conf.json index bafeeec5..8f7aea20 100644 --- a/packages/tauri-app/src-tauri/tauri.conf.json +++ b/packages/tauri-app/src-tauri/tauri.conf.json @@ -13,11 +13,6 @@ "withGlobalTauri": true, "windows": [], "security": { - "assetProtocol": { - "scope": [ - "**" - ] - }, "capabilities": [ "main-window-native-dialogs", "remote-window-notifications" diff --git a/packages/ui/src/lib/hooks/use-instance-metadata.test.ts b/packages/ui/src/lib/hooks/use-instance-metadata.test.ts index d858e719..462b99fc 100644 --- a/packages/ui/src/lib/hooks/use-instance-metadata.test.ts +++ b/packages/ui/src/lib/hooks/use-instance-metadata.test.ts @@ -13,7 +13,7 @@ describe("instance metadata", () => { list: async () => [{ id: "other", canonical: "/other", vcs: "hg" }, { id: "project-1", canonical: "/repo", vcs: "git" }], }, mcp: { list: async () => ({ location: { directory: "/repo", project: { id: "project-1", directory: "/repo", canonical: "/repo" } }, data: [] }) }, - plugin: { list: async () => ({ location: { directory: "/repo", project: { id: "project-1", directory: "/repo", canonical: "/repo" } }, data: [{ id: "opencode.plan" }, { id: "ponytail" }] }) }, + plugin: { list: async () => ({ location: { directory: "/repo", project: { id: "project-1", directory: "/repo", canonical: "/repo" } }, data: [{ id: "opencode.plan" }, { id: "ponytail" }, { status: "failed", error: "broken" }] }) }, } try { diff --git a/packages/ui/src/lib/hooks/use-instance-metadata.ts b/packages/ui/src/lib/hooks/use-instance-metadata.ts index 22739c69..10175437 100644 --- a/packages/ui/src/lib/hooks/use-instance-metadata.ts +++ b/packages/ui/src/lib/hooks/use-instance-metadata.ts @@ -45,7 +45,7 @@ export async function loadInstanceMetadata(instance: Instance, options?: { force ? { ...currentProject, ...(listedProject?.vcs ? { vcs: listedProject.vcs } : {}) } : undefined const plugins = pluginResult.status === "fulfilled" - ? pluginResult.value.data.map((plugin) => plugin.id).filter((id) => !id.startsWith("opencode.")) + ? pluginResult.value.data.flatMap((plugin) => typeof plugin.id === "string" && !plugin.id.startsWith("opencode.") ? [plugin.id] : []) : undefined const updates: Instance["metadata"] = { ...(currentMetadata ?? {}) } diff --git a/packages/ui/src/stores/session-api.ts b/packages/ui/src/stores/session-api.ts index 674e69bc..a72d703f 100644 --- a/packages/ui/src/stores/session-api.ts +++ b/packages/ui/src/stores/session-api.ts @@ -970,7 +970,7 @@ async function loadProviders(instanceId: string, location: LocationRef): Promise id: providerId, name: providersById.get(providerId)?.name ?? providerId, defaultModelId: defaultModel.data?.providerID === providerId ? defaultModel.data.id : undefined, - models: Array.from(modelsById.values()).filter((model) => model.providerID === providerId).sort((a, b) => a.id.localeCompare(b.id)).map((model) => ({ + models: Array.from(modelsById.values()).filter((model) => model.providerID === providerId && model.status !== "deprecated").sort((a, b) => a.id.localeCompare(b.id)).map((model) => ({ id: model.id, name: model.name, providerId, @@ -1235,7 +1235,6 @@ async function loadNextMessagePage(instanceId: string, sessionId: string, signal const response = await getRootClient(instanceId).message.list({ sessionID: sessionId, limit: 200, - order: "desc", cursor, }, signal ? { signal } : undefined) if (!instances().has(instanceId) diff --git a/packages/ui/src/stores/session-request-authority.test.ts b/packages/ui/src/stores/session-request-authority.test.ts index adff7f7d..e900d60a 100644 --- a/packages/ui/src/stores/session-request-authority.test.ts +++ b/packages/ui/src/stores/session-request-authority.test.ts @@ -175,6 +175,7 @@ describe("session request authority", () => { const requests: any[] = [] ;(client as any).message = { list: async (input: any) => { requests.push(input) + if (input.cursor && input.order !== undefined) throw new Error("cursor cannot be combined with order") if (input.cursor && failSecondPage) throw new Error("cursor failed") if (input.cursor && pendingSecondPage) return pendingSecondPage.promise return input.cursor @@ -204,7 +205,7 @@ describe("session request authority", () => { pendingSecondPage.resolve({ data: [apiMessage("old-2"), apiMessage("old-1")], cursor: {} }) await Promise.all([firstLoadMore, concurrentLoadMore]) assert.deepEqual(messageStoreBus.getOrCreate(instanceId).getSessionMessageIds(sessionId), ["old-1", "old-2", "new-1", "new-2"]) - assert.equal((requests.at(-1) as any)?.cursor, "page-2") + assert.deepEqual(requests.at(-1), { sessionID: sessionId, limit: 200, cursor: "page-2" }) assert.equal(hasMoreMessages(instanceId, sessionId), false) } finally { cleanup() @@ -241,7 +242,10 @@ describe("session request authority", () => { const calls = { provider: 0, model: 0, default: 0 } ;(client as any).provider = { list: async () => { calls.provider += 1; await response.promise; return { data: [{ id: "provider", name: "Provider" }] } } } ;(client as any).model = { - list: async () => { calls.model += 1; await response.promise; return { data: [{ id: "model", providerID: "provider", name: "Model", cost: [{}], limit: {}, variants: [] }] } }, + list: async () => { calls.model += 1; await response.promise; return { data: [ + { id: "model", providerID: "provider", name: "Model", cost: [{}], limit: {}, variants: [] }, + { id: "retired", providerID: "provider", name: "Retired", status: "deprecated", cost: [{}], limit: {}, variants: [] }, + ] } }, default: async () => { calls.default += 1; await response.promise; return { data: { id: "model", providerID: "provider", name: "Model", cost: [{}], limit: {}, variants: [] } } }, } @@ -251,7 +255,7 @@ describe("session request authority", () => { assert.deepEqual(calls, { provider: 1, model: 1, default: 1 }) response.resolve() assert.deepEqual(await Promise.all([first, second]), [true, true]) - assert.equal(providers().get(instanceId)?.[0]?.models[0]?.id, "model") + assert.deepEqual(providers().get(instanceId)?.[0]?.models.map((model) => model.id), ["model"]) } finally { cleanup() }