diff --git a/bun.lock b/bun.lock index 0495f7f8cd..3a64439932 100644 --- a/bun.lock +++ b/bun.lock @@ -114,6 +114,7 @@ "mime-types": "3.0.2", "minimatch": "10.2.5", "npm-package-arg": "13.0.2", + "open": "10.1.2", "semver": "^7.6.3", "turndown": "7.2.0", "venice-ai-sdk-provider": "2.0.2", diff --git a/packages/core/package.json b/packages/core/package.json index 19e76e778b..882e2f3f9d 100644 --- a/packages/core/package.json +++ b/packages/core/package.json @@ -116,6 +116,7 @@ "mime-types": "3.0.2", "minimatch": "10.2.5", "npm-package-arg": "13.0.2", + "open": "10.1.2", "semver": "^7.6.3", "turndown": "7.2.0", "venice-ai-sdk-provider": "2.0.2", diff --git a/packages/core/src/open.ts b/packages/core/src/open.ts new file mode 100644 index 0000000000..296982f3af --- /dev/null +++ b/packages/core/src/open.ts @@ -0,0 +1,8 @@ +import open from "open" + +export function openUrl(input: string) { + const url = URL.canParse(input) ? new URL(input) : undefined + if (!url || (url.protocol !== "http:" && url.protocol !== "https:")) + return Promise.reject(new Error(`Only http and https links can be opened in the browser: ${input}`)) + return open(url.href) +} diff --git a/packages/core/test/open.test.ts b/packages/core/test/open.test.ts new file mode 100644 index 0000000000..2c095846ca --- /dev/null +++ b/packages/core/test/open.test.ts @@ -0,0 +1,47 @@ +import { describe, expect, spyOn, test } from "bun:test" +import type { ChildProcess } from "node:child_process" +import { EventEmitter } from "node:events" +import * as browser from "open" +import { openUrl } from "@opencode-ai/core/open" + +describe("openUrl", () => { + test("opens normalized http(s) URLs and preserves the subprocess promise", async () => { + const subprocess = new EventEmitter() as ChildProcess + const promise = Promise.resolve(subprocess) + const opener = spyOn(browser, "default").mockReturnValue(promise) + try { + for (const input of ["HTTP://Example.COM:80/path", "https://example.com/a b"]) { + const result = openUrl(input) + expect(result).toBe(promise) + expect(await result).toBe(subprocess) + expect(opener).toHaveBeenLastCalledWith(new URL(input).href) + } + } finally { + opener.mockRestore() + } + }) + + test("rejects values that are not URLs", async () => { + const opener = spyOn(browser, "default").mockResolvedValue(new EventEmitter() as ChildProcess) + try { + await expect(openUrl("not a url")).rejects.toThrow("Only http and https links") + await expect(openUrl("")).rejects.toThrow("Only http and https links") + expect(opener).not.toHaveBeenCalled() + } finally { + opener.mockRestore() + } + }) + + test("rejects non-http schemes", async () => { + const opener = spyOn(browser, "default").mockResolvedValue(new EventEmitter() as ChildProcess) + try { + await expect(openUrl("file:///etc/hosts")).rejects.toThrow("Only http and https links") + await expect(openUrl("javascript:alert(1)")).rejects.toThrow("Only http and https links") + await expect(openUrl("ms-msdt:/id PCWDiagnostic")).rejects.toThrow("Only http and https links") + await expect(openUrl("\\\\server\\share\\file.html")).rejects.toThrow("Only http and https links") + expect(opener).not.toHaveBeenCalled() + } finally { + opener.mockRestore() + } + }) +}) diff --git a/packages/drivers/src/sign-in.ts b/packages/drivers/src/sign-in.ts index c8fa248097..d23c705bc9 100644 --- a/packages/drivers/src/sign-in.ts +++ b/packages/drivers/src/sign-in.ts @@ -57,6 +57,14 @@ export function setBrowserOpenerForTests(fn: (url: string) => void): () => void } function launchBrowser(url: string): void { + // The server/IdP supplies this URL. Refuse non-web protocol handlers; the notice still carries it for manual copy. + try { + const protocol = new URL(url).protocol + if (protocol !== "http:" && protocol !== "https:") return + } catch { + return + } + const [command, args] = process.platform === "win32" ? // No shell, so `&` in the SAML query needs no escaping (unlike `cmd /c start`). diff --git a/packages/drivers/test/snowflake-sign-in.test.ts b/packages/drivers/test/snowflake-sign-in.test.ts index f4c6617722..557f0139db 100644 --- a/packages/drivers/test/snowflake-sign-in.test.ts +++ b/packages/drivers/test/snowflake-sign-in.test.ts @@ -4,14 +4,22 @@ * sign-in that never came back is an actionable error, not the SDK's raw timeout. * Non-interactive logins get an upper bound instead of the SDK's 300 s retry floor. */ -import { afterEach, beforeEach, describe, expect, test } from "bun:test" +import { afterEach, beforeEach, describe, expect, spyOn, test } from "bun:test" +import * as childProcess from "child_process" +import { EventEmitter } from "node:events" import { connect, SSO_WAIT_MS } from "../src/snowflake" -import { onBrowserSignIn, redactSignInUrl, setBrowserOpenerForTests, type BrowserSignInNotice } from "../src/sign-in" +import { + onBrowserSignIn, + openInBrowser, + redactSignInUrl, + setBrowserOpenerForTests, + type BrowserSignInNotice, +} from "../src/sign-in" const SIGN_IN_URL = "https://idp.example.com/sso/saml?SAMLRequest=abc&RelayState=xyz" /** snowflake-sdk stand-in: `browser` decides what the SSO sign-in does; `connect` what a plain login does. */ -function fakeSdk(opts: { browser?: "complete" | "timeout" | "no-prompt"; connect?: "ok" | "never" }) { +function fakeSdk(opts: { browser?: "complete" | "timeout" | "no-prompt"; connect?: "ok" | "never"; url?: string }) { const created: Array<{ options: any; destroyed: boolean }> = [] const sdk = { configure() {}, @@ -20,7 +28,7 @@ function fakeSdk(opts: { browser?: "complete" | "timeout" | "no-prompt"; connect created.push(entry) return { async connectAsync(cb: (err: Error | null) => void) { - if (opts.browser !== "no-prompt") options.openExternalBrowserCallback?.(SIGN_IN_URL) + if (opts.browser !== "no-prompt") options.openExternalBrowserCallback?.(opts.url ?? SIGN_IN_URL) setTimeout(() => { if (opts.browser === "timeout") { cb(new Error("Error while getting SAML token: Browser action timed out after 120000 ms.")) @@ -63,6 +71,46 @@ afterEach(() => { }) describe("Snowflake browser sign-in", () => { + test.each(["file:///etc/hosts", "javascript:alert(1)", "ms-msdt:/id PCWDiagnostic", "not a url", "", "https://"])( + "refuses to launch %j but still announces it for manual copy", + async (url) => { + restoreOpener() + const spawn = spyOn(childProcess, "spawn").mockImplementation(() => { + throw new Error("Unexpected browser launch") + }) + try { + const { sdk } = fakeSdk({ browser: "complete", url }) + const c = await connect(sso, sdk) + await c.connect() + + expect(spawn).not.toHaveBeenCalled() + expect(notices).toEqual([ + { warehouse: "snowflake", account: "acme-xy123", phase: "waiting", url, timeoutMs: SSO_WAIT_MS }, + { warehouse: "snowflake", account: "acme-xy123", phase: "completed" }, + ]) + } finally { + spawn.mockRestore() + } + }, + ) + + test.each(["http://idp.example.com/sso", SIGN_IN_URL, "HTTPS://idp.example.com/sso"])( + "launches the browser for %s", + (url) => { + restoreOpener() + const child = Object.assign(new EventEmitter(), { unref() {} }) as childProcess.ChildProcess + const spawn = spyOn(childProcess, "spawn").mockReturnValue(child) + try { + openInBrowser(url) + + expect(spawn).toHaveBeenCalledTimes(1) + expect(spawn.mock.calls[0][1]).toContain(url) + } finally { + spawn.mockRestore() + } + }, + ) + test("the sign-in page is announced with its time limit, opened, and its completion reported", async () => { const { sdk, created } = fakeSdk({ browser: "complete" }) const c = await connect(sso, sdk) diff --git a/packages/opencode/src/altimate/plugin/altimate.ts b/packages/opencode/src/altimate/plugin/altimate.ts index e67ff13311..d60e90227d 100644 --- a/packages/opencode/src/altimate/plugin/altimate.ts +++ b/packages/opencode/src/altimate/plugin/altimate.ts @@ -1,7 +1,7 @@ import type { Hooks, PluginInput } from "@opencode-ai/plugin" import { createServer } from "http" import { randomBytes } from "crypto" -import open from "open" +import { openUrl } from "@opencode-ai/core/open" import { AltimateApi } from "../api/client" // altimate_change — onboarding telemetry for the gateway sign-in funnel import * as OnboardingTelemetry from "../telemetry/onboarding" @@ -444,7 +444,7 @@ export async function AltimateAuthPlugin(_input: PluginInput): Promise { // to the log file at the top of packages/opencode/src/cli/tui/ // worker.ts — so that write reached the log, never the terminal. // Removed to stop leaking state-bearing authorize URLs into logs. - await open(authorizeUrl).catch(() => undefined) + await openUrl(authorizeUrl).catch(() => undefined) // altimate_change start — onboarding funnel: gateway sign-in started. // Spec name is `gateway_device_code_issued`; this flow is a browser loopback OAuth diff --git a/packages/opencode/src/altimate/workspace/browser-handoff.ts b/packages/opencode/src/altimate/workspace/browser-handoff.ts index 1f9cb6468d..a4f092056b 100644 --- a/packages/opencode/src/altimate/workspace/browser-handoff.ts +++ b/packages/opencode/src/altimate/workspace/browser-handoff.ts @@ -19,7 +19,7 @@ // See docs `workspace-browser-handoff-plan-v3.md` for the design context. import { createServer, type Server } from "http" import { randomBytes } from "crypto" -import open from "open" +import { openUrl } from "@opencode-ai/core/open" import { AltimateApi } from "@/altimate/api/client" import { Log } from "@/altimate/util/log" @@ -424,7 +424,7 @@ export interface OpenBrowserHandoffInput { * error is expressed as ``{ok: false, reason}`` so the caller can toast the * appropriate message. */ export async function openWorkspaceBrowserHandoff(input: OpenBrowserHandoffInput): Promise { - return runHandoffWithOpener(input, (url) => open(url).then(() => undefined)) + return runHandoffWithOpener(input, (url) => openUrl(url).then(() => undefined)) } /** Same as ``openWorkspaceBrowserHandoff`` but takes the browser-open callback diff --git a/packages/opencode/src/cli/cmd/account.ts b/packages/opencode/src/cli/cmd/account.ts index 56066d49ca..6a67583a67 100644 --- a/packages/opencode/src/cli/cmd/account.ts +++ b/packages/opencode/src/cli/cmd/account.ts @@ -5,9 +5,13 @@ import { Account } from "@/account/account" import { AccountID, OrgID, PollExpired, type PollResult, type AccountError } from "@/account/schema" import { effectCmd } from "../effect-cmd" import * as Prompt from "../effect/prompt" -import open from "open" +// altimate_change start — upstream_fix: backport 29f07e0c69 restrict browser URLs to http(s) +import { openUrl } from "@opencode-ai/core/open" +// altimate_change end -const openBrowser = (url: string) => Effect.promise(() => open(url).catch(() => undefined)) +// altimate_change start — upstream_fix: backport 29f07e0c69 restrict browser URLs to http(s) +const openBrowser = (url: string) => Effect.promise(() => openUrl(url).catch(() => undefined)) +// altimate_change end const println = (msg: string) => Effect.sync(() => UI.println(msg)) diff --git a/packages/opencode/src/cli/cmd/debug/config.ts b/packages/opencode/src/cli/cmd/debug/config.ts index 65e230b1bf..1b9876f1a5 100644 --- a/packages/opencode/src/cli/cmd/debug/config.ts +++ b/packages/opencode/src/cli/cmd/debug/config.ts @@ -1,6 +1,9 @@ import { EOL } from "os" import { Effect } from "effect" import { effectCmd } from "../../effect-cmd" +// altimate_change start — upstream_fix: backport 82d4c89031 redact debug configuration credentials +import { redactConfig } from "./redact" +// altimate_change end export const ConfigCommand = effectCmd({ command: "config", @@ -9,6 +12,8 @@ export const ConfigCommand = effectCmd({ handler: Effect.fn("Cli.debug.config")(function* () { const { Config } = yield* Effect.promise(() => import("@/config/config")) const config = yield* Config.Service.use((cfg) => cfg.get()) - process.stdout.write(JSON.stringify(config, null, 2) + EOL) + // altimate_change start — upstream_fix: backport 82d4c89031 redact debug configuration credentials + process.stdout.write(JSON.stringify(redactConfig(config), null, 2) + EOL) + // altimate_change end }), }) diff --git a/packages/opencode/src/cli/cmd/debug/redact.ts b/packages/opencode/src/cli/cmd/debug/redact.ts new file mode 100644 index 0000000000..eb0412c10a --- /dev/null +++ b/packages/opencode/src/cli/cmd/debug/redact.ts @@ -0,0 +1,92 @@ +// Fork-only file (not in v1.17.9): extends upstream 82d4c89031. +// Redact only the debug output; the resolved configuration must remain usable by providers. +const secretName = + /(?:api.?key|access.?key|secret|password|passwd|token$|authorization$|cookie$|credential|private.?key|bearer|passphrase|(?:^|[_-])pwd$|(?:^|[_-])auth(?:[_-]|$)|dsn$|connection.?string)/i + +function redactURL(value: string): string { + const trimmed = value.trim() + // Prose (prompts, descriptions) is not a URL even when it mentions one. A value that starts with + // `//` or a scheme directly followed by a non-space (`https:/x`, `jdbc:...`) is still a URL, even + // with spaces later; `Review: check {file}` stays prose. + if (/\s/.test(trimmed) && !/^(?:\/\/|[a-z][a-z\d+.-]*:\S)/i.test(trimmed)) return value + // Also covers malformed authorities and opaque schemes such as jdbc:postgresql://. + const authority = /^(?:[^/?#]*:)?\/\/([^/?#]*)/.exec(trimmed)?.[1] + if (authority?.includes("@")) return "***" + // Fail closed for URL-like strings that cannot be safely parsed, regardless of scheme. + if (!URL.canParse(trimmed)) { + return authority !== undefined || /^[a-z][a-z\d+.-]*:/i.test(trimmed) ? "***" : value + } + const url = new URL(trimmed) + if (url.username || url.password) return "***" + + const redactParams = (params: string) => + params + .split("&") + .map((param) => (param ? `${param.split("=", 1)[0]}=***` : param)) + .join("&") + + // Preserve the original URL spelling and fragments that are only anchors. + return value + .replace(/^([^?#]*\?)([^#]*)/, (_, prefix, params) => prefix + redactParams(params)) + .replace(/#(.*)/s, (fragment, params) => (params.includes("=") ? `#${redactParams(params)}` : fragment)) +} + +// Endpoint URLs (MCP `url`, provider `api`/`baseURL`) can carry credentials in the path +// (`/mcp/u//...`, `/v1/sk-...`), so print only the origin. +const endpointKey = /^(?:url|baseURL|api|endpoint)$/i + +function redactServerURL(value: string): string { + const trimmed = value.trim() + // Explicit endpoint fields fail closed: a schemeless value can still carry a path token. + if (!URL.canParse(trimmed)) return "***" + const url = new URL(trimmed) + if (url.username || url.password || url.origin === "null") return "***" + const rest = url.pathname.replace(/^\/$/, "") + url.search + url.hash + return rest ? `${url.origin}/***` : url.origin +} + +// Free-form secret carriers: every environment value is masked, and argv keeps only the +// executable (`env TOKEN=x`, `--header "Authorization: ..."`, `-pSecret`, JDBC strings). +const environmentKey = /^(?:environment|env)$/i +const argvKey = /^(?:command|headersCommand|args)$/i + +function redactArgv(argv: unknown[]): unknown[] { + return argv.map((item, index) => + typeof item !== "string" || index === 0 ? item : "***", + ) +} + +export function redactConfig(value: unknown, headers = false): unknown { + if (typeof value === "string") return headers ? "***" : redactURL(value) + if (Array.isArray(value)) + return value.map((item, index) => { + if (typeof item === "string") { + const flag = /^--?([^=]+)=/.exec(item) + const previous = value[index - 1] + const bare = typeof previous === "string" ? /^--?([^=]+)$/.exec(previous) : null + if ((flag && secretName.test(flag[1])) || (bare && secretName.test(bare[1]))) return "***" + } + return redactConfig(item, headers) + }) + if (value === null || typeof value !== "object") return value + + return Object.fromEntries( + Object.entries(value).map(([key, item]) => { + if (typeof item === "string" && (headers || secretName.test(key))) return [key, "***"] + // Only address-like values (`.`, `/`, `\`, `:`): a permission entry for a tool named `api` holds `allow`/`deny`. + if (typeof item === "string" && endpointKey.test(key) && /[./\\:]/.test(item) && !headers) + return [key, redactServerURL(item)] + if (Array.isArray(item) && argvKey.test(key) && !headers) return [key, redactArgv(item)] + // headersCommand is a record of header name -> argv. + if (item && typeof item === "object" && /^headersCommand$/i.test(key) && !headers) + return [ + key, + Object.fromEntries( + Object.entries(item).map(([name, argv]) => [name, Array.isArray(argv) ? redactArgv(argv) : "***"]), + ), + ] + const secretScope = headers || /^headers$/i.test(key) || environmentKey.test(key) || secretName.test(key) + return [key, redactConfig(item, secretScope)] + }), + ) +} diff --git a/packages/opencode/src/cli/cmd/link.ts b/packages/opencode/src/cli/cmd/link.ts index dbfaa92241..f9eae07135 100644 --- a/packages/opencode/src/cli/cmd/link.ts +++ b/packages/opencode/src/cli/cmd/link.ts @@ -13,7 +13,7 @@ import { cmd } from "./cmd" import { UI } from "../ui" import * as prompts from "@clack/prompts" -import open from "open" +import { openUrl } from "@opencode-ai/core/open" import { AltimateApi } from "@/altimate/api/client" import { WorkspaceApi, @@ -735,7 +735,7 @@ export async function createThenBindOrRebind( if (opts.openBrowser === false) { // printed above; nothing to open } else if (isSafeHttpUrl(manageUrl)) { - await open(manageUrl).catch(() => undefined) + await openUrl(manageUrl).catch(() => undefined) } else { prompts.log.warn(`Skipped auto-open: manage_url is not an http/https URL.`) } diff --git a/packages/opencode/src/cli/cmd/web.ts b/packages/opencode/src/cli/cmd/web.ts index 6eea9e3e6a..7faa508adc 100644 --- a/packages/opencode/src/cli/cmd/web.ts +++ b/packages/opencode/src/cli/cmd/web.ts @@ -4,7 +4,9 @@ import { cmd } from "./cmd" import { withNetworkOptions, resolveNetworkOptions } from "../network" import { AppRuntime } from "../../effect/app-runtime" import { Flag } from "../../flag/flag" -import open from "open" +// altimate_change start — upstream_fix: backport 29f07e0c69 restrict browser URLs to http(s) +import { openUrl } from "@opencode-ai/core/open" +// altimate_change end import { networkInterfaces } from "os" // altimate_change start — auto-register Altimate Base before the server (and its provider state) starts import { FreeTier } from "../../altimate/free/client" @@ -81,11 +83,15 @@ export const WebCommand = cmd({ } // Open localhost in browser - open(localhostUrl.toString()).catch(() => {}) + // altimate_change start — upstream_fix: backport 29f07e0c69 restrict browser URLs to http(s) + openUrl(localhostUrl.toString()).catch(() => {}) + // altimate_change end } else { const displayUrl = server.url.toString() UI.println(UI.Style.TEXT_INFO_BOLD + " Web interface: ", UI.Style.TEXT_NORMAL, displayUrl) - open(displayUrl).catch(() => {}) + // altimate_change start — upstream_fix: backport 29f07e0c69 restrict browser URLs to http(s) + openUrl(displayUrl).catch(() => {}) + // altimate_change end } await new Promise(() => {}) diff --git a/packages/opencode/src/mcp/index.ts b/packages/opencode/src/mcp/index.ts index 5efa2c0538..1f7d8e9c0b 100644 --- a/packages/opencode/src/mcp/index.ts +++ b/packages/opencode/src/mcp/index.ts @@ -37,7 +37,9 @@ import { McpAuth } from "./auth" import { EventV2Bridge } from "@/event-v2-bridge" import { EventV2 } from "@opencode-ai/core/event" import { TuiEvent } from "@/server/tui-event" -import open from "open" +// altimate_change start — upstream_fix: backport 29f07e0c69 restrict browser URLs to http(s) +import { openUrl } from "@opencode-ai/core/open" +// altimate_change end import { Cause, Effect, Exit, Layer, Option, Context, Schema, Stream } from "effect" // altimate_change start — effect@4 renamed Either → Result (Effect.either → Effect.result) import * as Result from "effect/Result" @@ -1502,7 +1504,8 @@ export const layer = Layer.effect( const callbackPromise = McpOAuthCallback.waitForCallback(result.oauthState, mcpName) - yield* Effect.tryPromise(() => open(result.authorizationUrl)).pipe( + // altimate_change start — upstream_fix: backport 29f07e0c69 restrict browser URLs to http(s) + yield* Effect.tryPromise(() => openUrl(result.authorizationUrl)).pipe( Effect.flatMap((subprocess) => Effect.callback((resume) => { const timer = setTimeout(() => resume(Effect.void), 500) @@ -1522,6 +1525,7 @@ export const layer = Layer.effect( return events.publish(BrowserOpenFailed, { mcpName, url: result.authorizationUrl }).pipe(Effect.ignore) }), ) + // altimate_change end const code = yield* Effect.promise(() => callbackPromise) diff --git a/packages/opencode/src/mcp/oauth-provider.ts b/packages/opencode/src/mcp/oauth-provider.ts index 459dd6af8a..c1d7a8fe4c 100644 --- a/packages/opencode/src/mcp/oauth-provider.ts +++ b/packages/opencode/src/mcp/oauth-provider.ts @@ -128,6 +128,12 @@ export class McpOAuthProvider implements OAuthClientProvider { } async redirectToAuthorization(authorizationUrl: URL): Promise { + // altimate_change start — upstream_fix: backport 29f07e0c69 restrict MCP authorization URLs to http(s) + if (authorizationUrl.protocol !== "http:" && authorizationUrl.protocol !== "https:") + throw new Error( + `MCP server "${this.mcpName}" returned a ${authorizationUrl.protocol} authorization URL; only http and https are supported`, + ) + // altimate_change end await this.callbacks.onRedirect(authorizationUrl) } diff --git a/packages/opencode/src/plugin/digitalocean.ts b/packages/opencode/src/plugin/digitalocean.ts index af4dec2ec3..81e33ffc3c 100644 --- a/packages/opencode/src/plugin/digitalocean.ts +++ b/packages/opencode/src/plugin/digitalocean.ts @@ -2,7 +2,9 @@ import type { Hooks, PluginInput } from "@opencode-ai/plugin" import type { Model } from "@opencode-ai/sdk/v2" import { InstallationVersion } from "@opencode-ai/core/installation/version" import { createServer } from "http" -import open from "open" +// altimate_change start — upstream_fix: backport 29f07e0c69 restrict browser URLs to http(s) +import { openUrl } from "@opencode-ai/core/open" +// altimate_change end const DO_OAUTH_CLIENT_ID = "b1a6c5158156caac821fd1b30253ca8acb52454a48fa744420e41889cb589f82" const DO_AUTHORIZE_URL = "https://cloud.digitalocean.com/v1/oauth/authorize" @@ -339,7 +341,9 @@ export async function DigitalOceanAuthPlugin(input: PluginInput): Promise const state = generateState() const callbackPromise = waitForOAuthCallback(state) const url = buildAuthorizeUrl(state) - await open(url).catch(() => undefined) + // altimate_change start — upstream_fix: backport 29f07e0c69 restrict browser URLs to http(s) + await openUrl(url).catch(() => undefined) + // altimate_change end return { url, // altimate_change start — DigitalOcean OAuth instructions use Altimate Code brand diff --git a/packages/opencode/src/plugin/snowflake-cortex.ts b/packages/opencode/src/plugin/snowflake-cortex.ts index 81bfda5a39..53e6f37386 100644 --- a/packages/opencode/src/plugin/snowflake-cortex.ts +++ b/packages/opencode/src/plugin/snowflake-cortex.ts @@ -2,7 +2,12 @@ import type { Hooks, PluginInput } from "@opencode-ai/plugin" import { OAUTH_DUMMY_KEY } from "../auth" import { InstallationVersion } from "@opencode-ai/core/installation/version" import { createServer } from "http" -import open from "open" +// altimate_change start — upstream_fix: backport 29f07e0c69 restrict browser URLs to http(s) +import { openUrl } from "@opencode-ai/core/open" +// altimate_change end +// altimate_change start — upstream_fix: backport e8fea9e63a escape provider text in OAuth callback pages +import { escapeHtml } from "../util/html" +// altimate_change end const OAUTH_CLIENT_ID = "LOCAL_APPLICATION" const OAUTH_CALLBACK_HOST = "127.0.0.1" @@ -170,14 +175,14 @@ const HTML_SUCCESS = ` ` // altimate_change end -// altimate_change start — Snowflake OAuth error copy uses Altimate Code brand +// altimate_change start — upstream_fix: backport e8fea9e63a escape Snowflake OAuth errors, retaining Altimate Code branding const htmlError = (message: string) => ` Altimate Code - Snowflake Authorization Failed

Authorization Failed

-
${message}
+
${escapeHtml(message)}
` @@ -496,7 +501,9 @@ export async function SnowflakeCortexAuthPlugin(_input: PluginInput): Promise undefined) + // altimate_change start — upstream_fix: backport 29f07e0c69 restrict browser URLs to http(s) + await openUrl(url).catch(() => undefined) + // altimate_change end return { url, diff --git a/packages/opencode/src/plugin/tui/altimate/trace-viewer.tsx b/packages/opencode/src/plugin/tui/altimate/trace-viewer.tsx index 7a7fbc6ae0..39d05c5ade 100644 --- a/packages/opencode/src/plugin/tui/altimate/trace-viewer.tsx +++ b/packages/opencode/src/plugin/tui/altimate/trace-viewer.tsx @@ -12,7 +12,7 @@ import type { TuiPlugin, TuiPluginApi } from "@opencode-ai/plugin/tui" import type { BuiltinTuiPlugin } from "@opencode-ai/tui/builtins" import { createMemo, createResource, onMount } from "solid-js" import fsAsync from "fs/promises" -import open from "open" +import { openUrl } from "@opencode-ai/core/open" import { Trace } from "@/altimate/observability/tracing" import { renderTraceViewer } from "@/altimate/observability/viewer" import { Log } from "@/util/log" @@ -256,7 +256,7 @@ const tui: TuiPlugin = async (api) => { return } const url = getTraceViewerUrl(sessionID, tracesDir) - await open(url) + await openUrl(url) api.ui.toast({ variant: "info", message: `Trace viewer: ${url}`, duration: 6000 }) } catch (err) { Log.Default.error(`Failed to open trace viewer: ${err}`) diff --git a/packages/opencode/src/plugin/tui/altimate/workspace.tsx b/packages/opencode/src/plugin/tui/altimate/workspace.tsx index 0895f828d3..198701027f 100644 --- a/packages/opencode/src/plugin/tui/altimate/workspace.tsx +++ b/packages/opencode/src/plugin/tui/altimate/workspace.tsx @@ -25,7 +25,7 @@ import type { TuiPlugin, TuiPluginApi } from "@opencode-ai/plugin/tui" import type { BuiltinTuiPlugin } from "@opencode-ai/tui/builtins" import { createHash } from "node:crypto" import { existsSync } from "node:fs" -import open from "open" +import { openUrl } from "@opencode-ai/core/open" // altimate_change start - the /workspace action menu import * as Manage from "@/altimate/workspace/manage" import { describeSyncProblems, pendingSync } from "@/altimate/workspace/skill-sync" @@ -891,7 +891,7 @@ export function openManageUrl(api: TuiPluginApi, url: string) { }) return } - open(url).catch(() => { + openUrl(url).catch(() => { api.ui.toast({ variant: "warning", message: `Could not open browser. Copy this URL: ${url}`, diff --git a/packages/opencode/test/cli/debug-config.test.ts b/packages/opencode/test/cli/debug-config.test.ts new file mode 100644 index 0000000000..832929433b --- /dev/null +++ b/packages/opencode/test/cli/debug-config.test.ts @@ -0,0 +1,555 @@ +import { describe, expect, test } from "bun:test" +import { Effect, Schema } from "effect" +import { ConfigV1 } from "@opencode-ai/core/v1/config/config" +import { cliIt } from "../lib/cli-process" +import { redactConfig } from "@/cli/cmd/debug/redact" + +const config = { + provider: { + example: { + options: { + apiKey: "sk-example", + timeout: 1200, + headers: { Authorization: "Bearer example", "X-API-Key": "key" }, + }, + models: { demo: { variants: { fast: { api_key: "variant-secret" } } } }, + }, + }, + mcp: { + remote: { oauth: { clientSecret: "oauth-secret", clientId: "public" }, headers: { "x-custom": "opaque" } }, + local: { environment: { SERVICE_TOKEN: "env-secret", PATH: "/usr/bin" } }, + }, + url: "https://user:pass@example.com/path", + normal: { context_tokens: 200000, name: "example" }, +} + +describe("debug config redaction", () => { + cliIt.live("always masks resolved credentials", ({ opencode }) => + Effect.gen(function* () { + const content = JSON.stringify({ provider: config.provider }) + const env = { OPENCODE_CONFIG_CONTENT: content } + const result = yield* opencode.spawn(["debug", "config"], { env }) + opencode.expectExit(result, 0, "debug config") + expect(JSON.parse(result.stdout).provider.example.options).toMatchObject({ + apiKey: "***", + timeout: 1200, + headers: { Authorization: "***", "X-API-Key": "***" }, + }) + expect(result.stdout).not.toContain("sk-example") + expect(result.stdout).not.toContain("Bearer example") + }), + 60_000, + ) + + test("does not mutate the input and preserves unrelated settings", () => { + const before = structuredClone(config) + expect(redactConfig(config)).toEqual({ + provider: { + example: { + options: { apiKey: "***", timeout: 1200, headers: { Authorization: "***", "X-API-Key": "***" } }, + models: { demo: { variants: { fast: { api_key: "***" } } } }, + }, + }, + mcp: { + remote: { oauth: { clientSecret: "***", clientId: "public" }, headers: { "x-custom": "***" } }, + local: { environment: { SERVICE_TOKEN: "***", PATH: "***" } }, + }, + url: "***", + normal: config.normal, + }) + expect(config).toEqual(before) + }) + + test("masks all URL query values and malformed HTTP URLs", () => { + expect( + redactConfig({ + api: "https://example.com/path?api_key=secret", + session: "https://example.com/path?token=secret", + browser: "https://example.com/path?cookie=secret", + invalid: "https://[invalid", + public: "https://example.com/path?model=demo", + }), + ).toEqual({ + api: "https://example.com/***", + session: "https://example.com/path?token=***", + browser: "https://example.com/path?cookie=***", + invalid: "***", + public: "https://example.com/path?model=***", + }) + }) + + test.each([ + "https://[invalid/mcp?api_key=secret", + "https://[invalid/mcp?token=secret", + "https://user:pass@[invalid/mcp", + "https://user:pass@[invalid/mcp?token=secret", + "custom://[invalid/mcp?api_key=secret", + "custom://[invalid/mcp?token=secret", + "custom://user:pass@[invalid/mcp", + "custom://user:pass@[invalid/mcp?token=secret", + "//user:pass@[invalid/mcp?api_key=secret", + ])("fails closed for malformed URL %s with surrounding whitespace", (url) => { + for (const value of [url, ` ${url}`, `${url} `, ` \t${url}\r\n `]) { + expect(redactConfig({ endpoint: value })).toEqual({ endpoint: "***" }) + } + }) + + test.each(["key", "sig", "signature", "code"])("masks URL query value for %s", (key) => { + expect(redactConfig({ link: `https://example.com/path?${key}=secret` })).toEqual({ + link: `https://example.com/path?${key}=***`, + }) + }) + + test("masks every query and fragment value, including repeated and empty values and bare query flags", () => { + expect( + redactConfig({ + link: "https://example.com/path?model=demo&model=other&empty=&flag#state=ready&state=done&empty=", + }), + ).toEqual({ + link: "https://example.com/path?model=***&model=***&empty=***&flag=***#state=***&state=***&empty=***", + }) + }) + + test("preserves URLs without parameters and fragment-only anchors", () => { + const input = { + plain: "https://example.com/path", + plainJdbc: "jdbc:postgresql://example.com/database", + plainOpaque: "custom:database", + anchor: "https://example.com/path#section", + namedAnchor: "https://example.com/path#access_token", + jdbc: "jdbc:postgresql://example.com/database#section", + opaque: "custom:database#section", + } + expect(redactConfig(input)).toEqual(input) + }) + + test("handles whitespace around valid URLs without changing unrelated strings", () => { + const input = { + plain: " \thttps://example.com/path\r\n ", + query: " \thttps://example.com/path?api_key=secret", + jdbc: " \tjdbc:postgresql://user:pass@example.com/database\r\n ", + label: " padded text ", + } + expect(redactConfig(input)).toEqual({ + plain: input.plain, + query: " \thttps://example.com/path?api_key=***", + jdbc: "***", + label: input.label, + }) + }) + + test("redacts objects inside arrays and nested headers while preserving primitive values", () => { + expect( + redactConfig({ + providers: [{ apiKey: "array-secret", name: "demo" }], + HEADERS: { nested: [{ "x-custom": "header-secret" }] }, + settings: [null, 1200, true, "demo"], + }), + ).toEqual({ + providers: [{ apiKey: "***", name: "demo" }], + HEADERS: { nested: [{ "x-custom": "***" }] }, + settings: [null, 1200, true, "demo"], + }) + }) + + test.each(["postgresql", "mysql", "mongodb+srv", "snowflake", "redis", "https", "custom+db"])( + "masks %s URLs with username-only, password-only, or combined userinfo", + (scheme) => { + expect( + redactConfig({ + withUser: `${scheme}://user@example.com/database`, + withPass: `${scheme}://:pass@example.com/database`, + withBoth: `${scheme}://user:pass@example.com/database`, + public: `${scheme}://example.com/database?model=demo#section`, + }), + ).toEqual({ + withUser: "***", + withPass: "***", + withBoth: "***", + public: `${scheme}://example.com/database?model=***#section`, + }) + }, + ) + + test("masks JDBC URL credentials and all JDBC and opaque URL parameter values", () => { + expect( + redactConfig({ + jdbcUser: "jdbc:postgresql://user@example.com/database", + jdbcPass: "jdbc:mysql://:pass@example.com/database", + jdbcBoth: "jdbc:postgresql://user:pass@example.com/database", + jdbcQuery: "jdbc:postgresql://example.com/database?password=secret", + jdbcFragment: "jdbc:mysql://example.com/database#access_token=secret", + opaqueQuery: "custom:database?api_key=secret", + opaqueFragment: "custom:database#session_token=secret", + publicJdbc: "jdbc:postgresql://example.com/database?model=demo#section", + publicOpaque: "custom:database?model=demo#section", + }), + ).toEqual({ + jdbcUser: "***", + jdbcPass: "***", + jdbcBoth: "***", + jdbcQuery: "jdbc:postgresql://example.com/database?password=***", + jdbcFragment: "jdbc:mysql://example.com/database#access_token=***", + opaqueQuery: "custom:database?api_key=***", + opaqueFragment: "custom:database#session_token=***", + publicJdbc: "jdbc:postgresql://example.com/database?model=***#section", + publicOpaque: "custom:database?model=***#section", + }) + }) + + test.each([ + "postgresql://user:pass@[invalid/database", + "mysql://user:pass@example.com:invalid/database", + "mongodb+srv://user:pass@[invalid/database", + "snowflake://user:pass@[invalid/database", + "redis://user:pass@[invalid/database", + "https://user:pass@[invalid/path", + "custom+db://user:pass@[invalid/database", + "jdbc:postgresql://user:pass@[invalid/database", + ])("masks userinfo in malformed URL %s", (url) => { + expect(redactConfig({ endpoint: url })).toEqual({ endpoint: "***" }) + }) + + test("preserves URL structure with @ outside the authority", () => { + const input = { + path: "https://example.com/users/@public", + query: "mysql://example.com/database?email=public@example.com", + fragment: "custom://example.com/database#email=public@example.com", + jdbc: "jdbc:postgresql://example.com/database?email=public@example.com", + malformed: "custom://[invalid/path@public", + } + expect(redactConfig(input)).toEqual({ + path: input.path, + query: "mysql://example.com/database?email=***", + fragment: "custom://example.com/database#email=***", + jdbc: "jdbc:postgresql://example.com/database?email=***", + malformed: "***", + }) + }) + + test.each([ + "access_key", + "AWS_ACCESS_KEY_ID", + "AWS_SECRET_ACCESS_KEY", + "passwd", + "bearer", + "auth", + "auth_config", + "DOCKER_AUTH_CONFIG", + "NPM_AUTH", + "session_token", + "client_secret", + "dsn", + "connection_string", + "connectionString", + ])("masks %s string keys and URL parameters while keeping non-string values", (key) => { + expect(redactConfig({ [key]: "secret" })).toEqual({ [key]: "***" }) + expect(redactConfig({ [key]: 1200 })).toEqual({ [key]: 1200 }) + expect(redactConfig({ [key]: true })).toEqual({ [key]: true }) + expect( + redactConfig({ + query: `mysql://example.com/database?model=demo&${key}=secret`, + fragment: `https://example.com/path#state=demo&${key}=secret`, + }), + ).toEqual({ + query: `mysql://example.com/database?model=***&${key}=***`, + fragment: `https://example.com/path#state=***&${key}=***`, + }) + }) + + test.each(["max_tokens", "maxTokens", "context_tokens", "tokenizer", "author", "authorize_url"])( + "preserves non-secret %s strings in keys and argv while masking URL parameter values", + (key) => { + const input = { + [key]: "public-value", + query: `https://example.com/path?${key}=public-value`, + fragment: `https://example.com/path#${key}=public-value`, + argv: ["example", `--${key}=public-value`, `-${key}=public-value`, `--${key}`, "public-value"], + } + expect(redactConfig(input)).toEqual({ + ...input, + query: `https://example.com/path?${key}=***`, + fragment: `https://example.com/path#${key}=***`, + }) + }, + ) + + test("checks strings in arrays and nested arrays without mutating resolved values", () => { + const input = { + endpoints: [ + "postgresql://user:pass@example.com/database", + "redis://example.com/0?password=secret", + "https://example.com/path#access_token=secret", + "mysql://user:pass@[invalid/database", + "jdbc:postgresql://user:pass@example.com/database", + ["snowflake://user@example.com/database", "https://example.com/public"], + null, + 1200, + false, + "demo", + ], + headers: { custom: ["header-secret", 1200, true, null] }, + } + const before = structuredClone(input) + expect(redactConfig(input)).toEqual({ + endpoints: [ + "***", + "redis://example.com/0?password=***", + "https://example.com/path#access_token=***", + "***", + "***", + ["***", "https://example.com/public"], + null, + 1200, + false, + "demo", + ], + headers: { custom: ["***", 1200, true, null] }, + }) + expect(input).toEqual(before) + expect(redactConfig(["https://user:pass@example.com/path", "public"])).toEqual(["***", "public"]) + }) + + test.each([ + "token", + "password", + "api-key", + "access_key", + "AWS_ACCESS_KEY_ID", + "AWS_SECRET_ACCESS_KEY", + "passwd", + "bearer", + "auth", + "DOCKER_AUTH_CONFIG", + "NPM_AUTH", + "session_token", + "client_secret", + "dsn", + "connection_string", + "connectionString", + ])("masks inline and following values for secret argv flag %s", (flag) => { + const input = { + argv: ["example", `--${flag}=inline-secret`, `-${flag}=inline-secret`, `--${flag}`, "next-secret"], + nested: [[`-${flag}`, "next-secret", "public"]], + } + const before = structuredClone(input) + expect(redactConfig(input)).toEqual({ + argv: ["example", "***", "***", `--${flag}`, "***"], + nested: [[`-${flag}`, "***", "public"]], + }) + expect(input).toEqual(before) + }) + + test("preserves non-string argv values and limits bare flags to the following element", () => { + expect( + redactConfig({ + argv: ["example", "--token", 1200, "public", "--password", false, "public", "--api-key", null, "public"], + settings: ["--token", "secret", "public", "--password"], + consecutive: ["--token", "--password", "secret", "public"], + }), + ).toEqual({ + argv: ["example", "--token", 1200, "public", "--password", false, "public", "--api-key", null, "public"], + settings: ["--token", "***", "public", "--password"], + consecutive: ["--token", "***", "***", "public"], + }) + }) + + test("redacts every string nested under a secret-named key", () => { + expect( + redactConfig({ + token: ["opaque-secret"], + credentials: { value: "opaque-secret", nested: { deeper: "opaque-secret" } }, + model: { name: "public" }, + }), + ).toEqual({ + token: ["***"], + credentials: { value: "***", nested: { deeper: "***" } }, + model: { name: "public" }, + }) + }) + + test.each(["command", "headersCommand", "args"])( + "keeps only the first element and non-string values in %s", + (key) => { + expect( + redactConfig({ + [key]: ["example", "--max_tokens=1200", "public", "--token", "--verbose", "-v", 1200, false, null], + }), + ).toEqual({ + [key]: ["example", "***", "***", "***", "***", "***", 1200, false, null], + }) + }, + ) + + test("keeps only the executable in headersCommand records and masks non-array values", () => { + expect( + redactConfig({ + headersCommand: { + "X-Tenant": ["sh", "-c", "printf embedded-secret"], + "X-String": "embedded-secret", + "X-Number": 1200, + "X-Null": null, + }, + }), + ).toEqual({ + headersCommand: { + "X-Tenant": ["sh", "***", "***"], + "X-String": "***", + "X-Number": "***", + "X-Null": "***", + }, + }) + }) + + test("redacts schema-valid MCP URLs and headersCommand without changing custom commands", () => { + const command = { + review: { + template: "Review the changes in $ARGUMENTS", + model: "test/test-model", + agent: "build", + description: "Review local changes", + subtask: true, + }, + } + const input = Schema.decodeUnknownSync(ConfigV1.Info)({ + command, + mcp: { + query: { + type: "remote", + url: " https://[invalid/mcp?api_key=query-secret ", + headersCommand: { Authorization: ["sh", "-c", "printf header-secret"] }, + }, + userinfo: { type: "remote", url: " custom://user:pass@[invalid/mcp?token=query-secret " }, + }, + }) + expect(redactConfig(input)).toEqual({ + command, + mcp: { + query: { type: "remote", url: "***", headersCommand: { Authorization: ["sh", "***", "***"] } }, + userinfo: { type: "remote", url: "***" }, + }, + }) + }) + + test("masks secrets attached to short command flags", () => { + expect(redactConfig({ command: ["mysql", "-pSecret123"] })).toEqual({ command: ["mysql", "***"] }) + }) + + test.each(["environment", "env"])("masks nested strings under %s while preserving non-string values", (key) => { + expect( + redactConfig({ + [key]: { nested: ["debug", 1200, true, null], command: ["example", "--verbose"] }, + }), + ).toEqual({ + [key]: { nested: ["***", 1200, true, null], command: ["***", "***"] }, + }) + }) + + test("masks every MCP environment value and keeps only the executable in command arrays", () => { + expect( + redactConfig({ + mcp: { + db: { + type: "local", + command: ["env", "API_TOKEN=secret", "curl", "--header", "Authorization: Bearer secret", "--verbose"], + environment: { + DATABASE_URL: "jdbc:sqlserver://db;user=sa;password=Secret123", + ORACLE: "jdbc:oracle:thin:scott/tiger@db:1521/orcl", + KEY_PASSPHRASE: "also-secret", + DB_PWD: "hunter2", + LOG_LEVEL: "debug", + }, + }, + }, + other: { KEY_PASSPHRASE: "x", DB_PWD: "y", password_hint_url: "https://example.com/help" }, + }), + ).toEqual({ + mcp: { + db: { + type: "local", + command: ["env", "***", "***", "***", "***", "***"], + environment: { + DATABASE_URL: "***", + ORACLE: "***", + KEY_PASSPHRASE: "***", + DB_PWD: "***", + LOG_LEVEL: "***", + }, + }, + }, + other: { KEY_PASSPHRASE: "***", DB_PWD: "***", password_hint_url: "***" }, + }) + }) +}) + +describe("remote server URLs and prose", () => { + test("prints only the origin of a remote MCP url, since paths can carry credentials", () => { + expect( + redactConfig({ + mcp: { + make: { type: "remote", url: "https://eu2.make.com/mcp/u/path-secret/stateless" }, + query: { type: "remote", url: "https://mcp.example.com/sse?token=query-secret" }, + bare: { type: "remote", url: "https://mcp.example.com" }, + }, + }), + ).toEqual({ + mcp: { + make: { type: "remote", url: "https://eu2.make.com/***" }, + query: { type: "remote", url: "https://mcp.example.com/***" }, + bare: { type: "remote", url: "https://mcp.example.com" }, + }, + }) + }) + + test("still redacts a URL whose query contains spaces", () => { + expect(redactConfig({ options: { link: "https://example.com/v1?api_key=secret&label=hello world" } })).toEqual({ + options: { link: "https://example.com/v1?api_key=***&label=***" }, + }) + }) + + test.each([ + "https:/api.example.com/v1?api_key=secret&label=hello world", + "jdbc:postgresql://db.example.com/app?password=secret&app=my app", + ])("redacts scheme-prefixed value %s even with spaces", (url) => { + expect(JSON.stringify(redactConfig({ options: { baseURL: url } }))).not.toContain("secret") + }) + + test("keeps prose that starts with a label", () => { + const prompt = "Review: check {file} carefully, see https://example.com/docs?x=1" + expect(redactConfig({ agent: { review: { prompt } } })).toEqual({ agent: { review: { prompt } } }) + }) + + test("masks backslash-form endpoint URLs", () => { + expect(JSON.stringify(redactConfig({ mcp: { remote: { type: "remote", url: "https:localhost\\mcp\\path-secret" } } }))).not.toContain( + "path-secret", + ) + }) + + test("keeps permission actions for tools named like endpoint keys", () => { + const permission = { skill: { endpoint: "allow" }, api: "deny" } + expect(redactConfig({ permission })).toEqual({ permission }) + }) + + test("masks unparseable endpoint values entirely", () => { + expect(redactConfig({ mcp: { remote: { type: "remote", url: "mcp.example.com/mcp/u/path-secret" } } })).toEqual({ + mcp: { remote: { type: "remote", url: "***" } }, + }) + }) + + test("prints only the origin of provider endpoint URLs", () => { + expect( + redactConfig({ + provider: { + custom: { api: "https://gateway.example/v1/sk-secret", options: { baseURL: "https://proxy.example.com/path-secret/v1" } }, + }, + }), + ).toEqual({ + provider: { custom: { api: "https://gateway.example/***", options: { baseURL: "https://proxy.example.com/***" } } }, + }) + }) + + test("keeps prose that mentions a link", () => { + const prompt = "See https://example.com/docs before answering." + expect(redactConfig({ agent: { review: { prompt } } })).toEqual({ agent: { review: { prompt } } }) + }) +}) diff --git a/packages/opencode/test/mcp/oauth-browser.test.ts b/packages/opencode/test/mcp/oauth-browser.test.ts index 04ffb05f9f..daaf650531 100644 --- a/packages/opencode/test/mcp/oauth-browser.test.ts +++ b/packages/opencode/test/mcp/oauth-browser.test.ts @@ -1,6 +1,6 @@ -import { expect, mock, beforeEach } from "bun:test" +import { expect, mock, beforeEach, spyOn } from "bun:test" import { EventEmitter } from "events" -import { Deferred, Effect, Exit, Fiber, Layer, Option } from "effect" +import { Cause, Deferred, Effect, Exit, Fiber, Layer, Option } from "effect" import { awaitWithTimeout, testEffect } from "../lib/effect" import type { MCP as MCPNS } from "../../src/mcp/index" @@ -8,6 +8,7 @@ import type { MCP as MCPNS } from "../../src/mcp/index" let openShouldFail = false let openCalledWith: string | undefined let openDeferred: Deferred.Deferred | undefined +let authorizationUrl = "https://auth.example.com/authorize?client_id=test" // altimate_change start — token exchanges the transport performed let tokenExchanges = 0 // altimate_change end @@ -64,7 +65,7 @@ void mock.module("@modelcontextprotocol/sdk/client/streamableHttp.js", () => ({ async start() { // Simulate OAuth redirect by calling the authProvider's redirectToAuthorization if (this.authProvider?.redirectToAuthorization) { - await this.authProvider.redirectToAuthorization(new URL("https://auth.example.com/authorize?client_id=test")) + await this.authProvider.redirectToAuthorization(new URL(authorizationUrl)) } throw new MockUnauthorizedError() } @@ -116,6 +117,7 @@ beforeEach(() => { openShouldFail = false openCalledWith = undefined openDeferred = undefined + authorizationUrl = "https://auth.example.com/authorize?client_id=test" transportCalls.length = 0 // altimate_change start — see tokenExchanges tokenExchanges = 0 @@ -185,6 +187,31 @@ const authenticateScoped = (name: string) => ) }) +for (const url of ["file:///tmp/authorize", "ms-msdt:/id PCWDiagnostic"]) { + mcpTest.instance( + `refuses server-supplied authorization URL ${url} before opening a browser`, + () => + Effect.gen(function* () { + yield* withCallbackStop + const callbackServer = spyOn(McpOAuthCallback, "ensureRunning").mockResolvedValue(undefined) + yield* Effect.addFinalizer(() => Effect.sync(() => callbackServer.mockRestore())) + authorizationUrl = url + const mcp = yield* service + const result = yield* Effect.exit( + mcp.authenticate("test-unsafe-oauth-server").pipe(Effect.timeout("2 seconds")), + ) + + expect(Exit.isFailure(result)).toBe(true) + if (Exit.isFailure(result)) { + expect(Cause.pretty(result.cause)).toContain(`returned a ${new URL(url).protocol} authorization URL`) + } + expect(openCalledWith).toBeUndefined() + expect(tokenExchanges).toBe(0) + }), + { config: config("test-unsafe-oauth-server") }, + ) +} + mcpTest.instance( "BrowserOpenFailed event is published when open() throws", () => diff --git a/packages/opencode/test/mcp/oauth-provider.test.ts b/packages/opencode/test/mcp/oauth-provider.test.ts index 64c2cb6687..b86cc4e6f4 100644 --- a/packages/opencode/test/mcp/oauth-provider.test.ts +++ b/packages/opencode/test/mcp/oauth-provider.test.ts @@ -33,6 +33,44 @@ describe("McpOAuthProvider.redirectUrl", () => { }) }) +describe("McpOAuthProvider.redirectToAuthorization", () => { + test("forwards http and https authorization URLs", async () => { + const seen: string[] = [] + const provider = new McpOAuthProvider( + "test-server", + "https://mcp.example.com/mcp", + {}, + { onRedirect: (url) => void seen.push(url.href) }, + stubAuth, + ) + await provider.redirectToAuthorization(new URL("https://auth.example.com/authorize?state=1")) + await provider.redirectToAuthorization(new URL("http://127.0.0.1:8080/authorize")) + expect(seen).toEqual(["https://auth.example.com/authorize?state=1", "http://127.0.0.1:8080/authorize"]) + }) + + test.each(["file:///tmp/authorize", "custom-scheme://authorize", "javascript:alert(1)", "data:text/html,authorize"])( + "rejects authorization URL %s before invoking onRedirect", + async (input) => { + const seen: string[] = [] + const mcpName = "test-server" + const serverUrl = "https://fixture-user:fixture-password@mcp.example.com/mcp?token=fixture-token" + const provider = new McpOAuthProvider( + mcpName, + serverUrl, + {}, + { onRedirect: (url) => void seen.push(url.href) }, + stubAuth, + ) + const url = new URL(input) + const redirect = provider.redirectToAuthorization(url) + await expect(redirect).rejects.toThrow(`returned a ${url.protocol} authorization URL`) + await expect(redirect).rejects.toThrow(mcpName) + await expect(redirect).rejects.not.toThrow(serverUrl) + expect(seen).toEqual([]) + }, + ) +}) + describe("McpOAuthProvider.clientMetadata", () => { test("includes redirect_uris from redirectUrl", () => { const provider = makeProvider({ callbackPort: 6620 }) diff --git a/packages/opencode/test/plugin/snowflake-cortex.test.ts b/packages/opencode/test/plugin/snowflake-cortex.test.ts index de5ac28c3b..9eeae14122 100644 --- a/packages/opencode/test/plugin/snowflake-cortex.test.ts +++ b/packages/opencode/test/plugin/snowflake-cortex.test.ts @@ -1,4 +1,6 @@ -import { describe, expect, test } from "bun:test" +import { describe, expect, mock, spyOn, test } from "bun:test" +import * as http from "http" +import * as browser from "open" import { OAUTH_DUMMY_KEY } from "../../src/auth" import { oauthScope, SnowflakeCortexAuthPlugin } from "../../src/plugin/snowflake-cortex" @@ -32,6 +34,50 @@ function makeInput() { } describe("plugin.snowflake-cortex", () => { + test("escapes provider error_description markup in callback HTML", async () => { + const open = spyOn(browser, "default").mockRejectedValue(new Error("Browser launch disabled in tests")) + let handleRequest: http.RequestListener | undefined + const createServer = spyOn(http, "createServer").mockImplementation((listener) => { + handleRequest = listener as http.RequestListener + return { + listen: (_port: number, _host: string, ready: () => void) => ready(), + address: () => ({ port: 12345 }), + on: () => {}, + close: () => {}, + } as unknown as http.Server + }) + try { + const hooks = await SnowflakeCortexAuthPlugin({} as any) + const method = hooks.auth!.methods[0] + if (method.type !== "oauth") throw new Error("Expected the Snowflake OAuth method") + const authorization = await method.authorize({ account: "myorg-myaccount" }) + if (authorization.method !== "auto") throw new Error("Expected an automatic OAuth callback") + const callback = authorization.callback() + const authorizeUrl = new URL(authorization.url) + const callbackUrl = new URL(authorizeUrl.searchParams.get("redirect_uri")!) + const error = `` + callbackUrl.searchParams.set("state", authorizeUrl.searchParams.get("state")!) + callbackUrl.searchParams.set("error", "access_denied") + callbackUrl.searchParams.set("error_description", error) + + const response = { writeHead: mock(), end: mock() } + handleRequest!( + { headers: { host: callbackUrl.host }, url: callbackUrl.pathname + callbackUrl.search } as http.IncomingMessage, + response as unknown as http.ServerResponse, + ) + expect(await callback).toEqual({ type: "failed" }) + const body = String(response.end.mock.calls[0][0]) + + expect(response.writeHead).toHaveBeenCalledWith(200, { "Content-Type": "text/html" }) + expect(body).toContain("<script>alert("xss" & 'more')</script>") + expect(body).not.toContain(error) + expect(open).toHaveBeenCalledWith(authorization.url) + } finally { + createServer.mockRestore() + open.mockRestore() + } + }) + test("oauthScope uses Snowflake-compatible scope values", () => { expect(oauthScope(undefined)).toBe("refresh_token") expect(oauthScope("PUBLIC")).toBe("refresh_token session:role:PUBLIC") diff --git a/packages/tui/src/app.tsx b/packages/tui/src/app.tsx index 85ac8c470c..3ed3314245 100644 --- a/packages/tui/src/app.tsx +++ b/packages/tui/src/app.tsx @@ -89,7 +89,9 @@ import { isDefaultTitle } from "./util/session" import { KVProvider, useKV } from "./context/kv" import * as Model from "./util/model" import { ArgsProvider, useArgs, type Args } from "./context/args" -import open from "open" +// altimate_change start — upstream_fix: backport 29f07e0c69 restrict browser URLs to http(s) +import { openUrl } from "@opencode-ai/core/open" +// altimate_change end import { PromptRefProvider, usePromptRef } from "./context/prompt" import { TuiConfigProvider, useTuiConfig, type TuiConfig } from "./config" import { createTuiApiAdapters } from "./plugin/adapters" @@ -1208,8 +1210,8 @@ function App(props: { onSnapshot?: () => Promise; pluginHost: TuiPlugi name: "docs.open", title: "Open docs", run: () => { - // altimate_change start — altimate docs URL - open("https://help.altimate.ai/code").catch(() => {}) + // altimate_change start — altimate docs URL; upstream_fix: backport 29f07e0c69 restrict browser URLs to http(s) + openUrl("https://help.altimate.ai/code").catch(() => {}) // altimate_change end dialog.clear() }, diff --git a/packages/tui/src/component/dialog-retry-action.tsx b/packages/tui/src/component/dialog-retry-action.tsx index 5a7751c374..6596c906f4 100644 --- a/packages/tui/src/component/dialog-retry-action.tsx +++ b/packages/tui/src/component/dialog-retry-action.tsx @@ -1,5 +1,7 @@ import { RGBA, TextAttributes } from "@opentui/core" -import open from "open" +// altimate_change start — upstream_fix: backport 29f07e0c69 restrict browser URLs to http(s) +import { openUrl } from "@opencode-ai/core/open" +// altimate_change end import { createSignal } from "solid-js" import { selectedForeground, useTheme } from "../context/theme" import { useDialog, type DialogContext } from "../ui/dialog" @@ -21,7 +23,9 @@ export type DialogRetryActionProps = { } function runAction(props: DialogRetryActionProps, dialog: ReturnType) { - if (props.link) open(props.link).catch(() => {}) + // altimate_change start — upstream_fix: backport 29f07e0c69 restrict browser URLs to http(s) + if (props.link) openUrl(props.link).catch(() => {}) + // altimate_change end props.onClose?.() dialog.clear() } diff --git a/packages/tui/src/feature-plugins/sidebar/footer.tsx b/packages/tui/src/feature-plugins/sidebar/footer.tsx index 3f2a0a1e2f..ccd68f7f90 100644 --- a/packages/tui/src/feature-plugins/sidebar/footer.tsx +++ b/packages/tui/src/feature-plugins/sidebar/footer.tsx @@ -3,7 +3,9 @@ import type { BuiltinTuiPlugin } from "../builtins" // altimate_change — community/docs links: plain copyable text, underlined for // affordance; clicking the line opens the browser. (OSC-8 word-links are not // supported by the current @opentui/solid JSX layer — raw TextChunk children crash.) -import open from "open" +// altimate_change start — upstream_fix: backport 29f07e0c69 restrict browser URLs to http(s) +import { openUrl } from "@opencode-ai/core/open" +// altimate_change end const id = "internal:sidebar-footer" @@ -39,12 +41,12 @@ function View(props: { api: TuiPluginApi }) { open("https://altimate.studio/join-agentic-data-engineering-slack").catch(() => {})} + onMouseUp={() => openUrl("https://altimate.studio/join-agentic-data-engineering-slack").catch(() => {})} > Community · altimate.studio/join-agentic-data-engineering-slack - open("https://help.altimate.ai/code/").catch(() => {})}> + openUrl("https://help.altimate.ai/code/").catch(() => {})}> Docs · help.altimate.ai/code diff --git a/packages/tui/src/ui/link.tsx b/packages/tui/src/ui/link.tsx index cfd78bc333..033fd6fea0 100644 --- a/packages/tui/src/ui/link.tsx +++ b/packages/tui/src/ui/link.tsx @@ -1,6 +1,8 @@ import type { JSX } from "solid-js" import type { RGBA } from "@opentui/core" -import open from "open" +// altimate_change start — upstream_fix: backport 29f07e0c69 restrict browser URLs to http(s) +import { openUrl } from "@opencode-ai/core/open" +// altimate_change end export interface LinkProps { href: string @@ -25,7 +27,9 @@ export function Link(props: LinkProps) { width={props.width} wrapMode={props.wrapMode} onMouseUp={() => { - open(props.href).catch(() => {}) + // altimate_change start — upstream_fix: backport 29f07e0c69 restrict browser URLs to http(s) + openUrl(props.href).catch(() => {}) + // altimate_change end }} > {displayText}