mirror of
https://github.com/NeuralNomadsAI/CodeNomad.git
synced 2026-08-31 01:44:52 +00:00
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.
This commit is contained in:
parent
f03a17a434
commit
34179162df
13 changed files with 86 additions and 25 deletions
|
|
@ -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")
|
||||
})
|
||||
|
|
|
|||
|
|
@ -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"
|
||||
}
|
||||
|
|
|
|||
|
|
@ -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 }, {
|
||||
|
|
|
|||
|
|
@ -62,8 +62,8 @@ export class OpenCodeCliService implements OpenCodeServiceLifecycle {
|
|||
return this.endpoint(url, deadlineAt)
|
||||
}
|
||||
|
||||
private async endpoint(url: string, deadlineAt: number): Promise<Endpoint> {
|
||||
this.assertServiceUrl(url)
|
||||
private async endpoint(value: string, deadlineAt: number): Promise<Endpoint> {
|
||||
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 {
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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({
|
||||
|
|
|
|||
|
|
@ -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"
|
||||
|
|
|
|||
|
|
@ -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<R: Runtime>(webview: &Webview<R>, 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;
|
||||
|
|
|
|||
|
|
@ -13,11 +13,6 @@
|
|||
"withGlobalTauri": true,
|
||||
"windows": [],
|
||||
"security": {
|
||||
"assetProtocol": {
|
||||
"scope": [
|
||||
"**"
|
||||
]
|
||||
},
|
||||
"capabilities": [
|
||||
"main-window-native-dialogs",
|
||||
"remote-window-notifications"
|
||||
|
|
|
|||
|
|
@ -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 {
|
||||
|
|
|
|||
|
|
@ -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 ?? {}) }
|
||||
|
|
|
|||
|
|
@ -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)
|
||||
|
|
|
|||
|
|
@ -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()
|
||||
}
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue