Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
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
43 changes: 41 additions & 2 deletions packages/opencode/src/altimate/api/client.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import z from "zod"
import path from "path"
import { rm } from "node:fs/promises"
import { Global } from "../../global"
import { Filesystem } from "../../util/filesystem"

Expand Down Expand Up @@ -134,8 +135,18 @@ export namespace AltimateApi {
)
}

/** Remove the stored gateway credential file (sign out). No-op if absent. */
export async function clearCredentials(): Promise<void> {
await rm(credentialsPath(), { force: true })
}

const VALID_TENANT_REGEX = /^[a-z_][a-z0-9_-]*$/

/** True if `name` is a well-formed instance/tenant name. */
export function isValidInstanceName(name: string): boolean {
return VALID_TENANT_REGEX.test(name)
}

/** Validates credentials against the Altimate API.
* Mirrors AltimateSettingsHelper.validateSettings from altimate-mcp-engine. */
export async function validateCredentials(creds: {
Expand Down Expand Up @@ -185,6 +196,35 @@ export namespace AltimateApi {
}
}

/**
* Exchange a short-lived social `login_token` for the user's gateway
* `auth_token` via POST {altimateUrl}/auth/social/exchange. The token is
* one-time and short-lived, which keeps the raw api_key out of the loopback
* callback URL. Throws on a non-ok response or a missing `auth_token`.
*/
export async function exchangeSocialToken(altimateUrl: string, instance: string, token: string): Promise<string> {
const url = `${altimateUrl.replace(/\/+$/, "")}/auth/social/exchange`
// upstream_fix parity: bound the request so a network stall can't hang the callback.
const controller = new AbortController()
const timeout = setTimeout(() => controller.abort(), 15_000)
const res = await fetch(url, {
method: "POST",
headers: {
"x-tenant": instance,
"Content-Type": "application/json",
},
body: JSON.stringify({ token }),
signal: controller.signal,
}).finally(() => clearTimeout(timeout))
if (!res.ok) {
const body = await res.text().catch(() => "")
throw new Error(`Social token exchange failed (${res.status} ${res.statusText}) ${body}`)
}
const data = (await res.json()) as { auth_token?: string }
if (!data.auth_token) throw new Error("Social token exchange did not return an auth_token")
return data.auth_token
}

async function request(creds: AltimateCredentials, method: string, endpoint: string, body?: unknown) {
const url = `${creds.altimateUrl}${endpoint}`
const res = await fetch(url, {
Expand Down Expand Up @@ -278,8 +318,7 @@ export namespace AltimateApi {
const allIntegrations = await listIntegrations()
return integrationIds.map((id) => {
const def = allIntegrations.find((i) => i.id === id)
const tools =
def?.tools?.flatMap((t) => (t.enable_all ?? [t.key]).map((k) => ({ key: k }))) ?? []
const tools = def?.tools?.flatMap((t) => (t.enable_all ?? [t.key]).map((k) => ({ key: k }))) ?? []
return { id, tools }
})
}
Expand Down
218 changes: 217 additions & 1 deletion packages/opencode/src/altimate/plugin/altimate.ts
Original file line number Diff line number Diff line change
@@ -1,13 +1,229 @@
import type { Hooks, PluginInput } from "@opencode-ai/plugin"
import { createServer } from "http"
import { randomBytes } from "crypto"
import open from "open"
import { AltimateApi } from "../api/client"

// Loopback port the CLI listens on for the browser to deliver the gateway
// credential after sign-in. Must match the redirect the web authorize page posts
// back to. 7317 is otherwise unused in this codebase.
const CALLBACK_PORT = 7317

// Web app that hosts the signup/login (authorize) page. Overridable for
// dev/staging via ALTIMATE_WEB_URL.
const DEFAULT_WEB_URL = "https://app.myaltimate.com"
// Fallback gateway API base if the callback omits one.
const DEFAULT_API_URL = "https://api.myaltimate.com"

// Escape reflected values before interpolating them into the callback HTML — the
// error text originates from the URL query string, so it must not be trusted.
function escapeHtml(s: string): string {
return s.replace(
/[&<>"']/g,
(c) => ({ "&": "&amp;", "<": "&lt;", ">": "&gt;", '"': "&quot;", "'": "&#39;" })[c] as string,
)
}

// Neutral copy: the loopback returns this as soon as it RECEIVES the token, before
// the CLI has exchanged/persisted it — so don't claim "Signed in" (the terminal is
// the source of truth for actual success/failure).
const HTML_SUCCESS = `<!doctype html><meta charset="utf-8"><title>Altimate Code</title>
<body style="font-family:system-ui;text-align:center;padding:64px">
<h2>Authorization received</h2><p>Return to your terminal to finish connecting.</p>
<script>setTimeout(()=>window.close(),1500)</script></body>`

const HTML_ERROR = (msg: string) => `<!doctype html><meta charset="utf-8"><title>Altimate Code</title>

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The error page echoes attacker-controlled text straight into HTML

MAJORReflected, unescaped input in the callback error page (localhost XSS). HTML_ERROR(msg) interpolates msg into HTML with no escaping, and the value ultimately comes from url.searchParams (the error branch at L57, which runs before the state check). Any page that drives the browser to http://localhost:7317/callback?error=<script>... during the auth window yields reflected XSS on the localhost origin. Impact is limited (transient page, no secrets/cookies there) but it's avoidable.

Fix: HTML-escape all reflected values, and validate state before handling the error branch.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Fixed in e46fdd48b. Added escapeHtml() and applied it to the reflected value in HTML_ERROR (altimate.ts:20-25,37). The related root cause is also closed — state is now validated before the ?error= branch runs (see the abort-path comment), so untrusted input no longer reaches that branch without a valid state.

<body style="font-family:system-ui;text-align:center;padding:64px">
<h2>Connection failed</h2><p>${escapeHtml(msg)}</p><p>Please return to your terminal and try again.</p></body>`

interface CallbackResult {
api_url: string
instance: string
// Short-lived, one-time login_token delivered by the browser. Exchanged for
// the gateway auth_token in callback() — the raw api_key never rides in the URL.
token: string
}

interface Pending {
resolve: (creds: CallbackResult) => void
reject: (err: Error) => void
}

let server: ReturnType<typeof createServer> | undefined
// Pending flows keyed by the unguessable `state`. Registered synchronously in
// authorize() BEFORE the browser opens, so an instant redirect (an already
// signed-in user) is matched instead of dropped; keying by state also lets two
// concurrent /auth flows coexist without clobbering each other.
const pending = new Map<string, Pending>()

async function startCallbackServer(): Promise<void> {
if (server) return

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

If port 7317 is busy, sign-in silently hangs forever with no retry

MAJORPort collision leaves a broken, silently-failing server. server is assigned by createServer() before listen(). On EADDRINUSE the listen promise rejects (L94-96) but server is never reset to undefined, so this if (server) return early-returns on the next attempt and the flow proceeds as if listening — the callback never arrives and the user waits out the 5-minute timeout with no message.

Fix: in the error handler set server = undefined (and close it), and surface a clear "port 7317 in use" error to the TUI. Consider an ephemeral port if the redirect contract allows.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Fixed in e46fdd48b. On listen error we now reset server = undefined (clearing the if (server) return guard so a retry can re-bind) and throw an EADDRINUSE-specific message instead of hanging: "Port 7317 is already in use — close whatever is using it and try again." See altimate.ts:112-122.

server = createServer((req, res) => {
const url = new URL(req.url || "/", `http://localhost:${CALLBACK_PORT}`)
if (url.pathname !== "/callback") {
res.writeHead(404)
res.end("Not found")
return
}

const html = (status: number, body: string) => {
res.writeHead(status, { "Content-Type": "text/html" })
res.end(body)
}

// Validate `state` FIRST — before honoring `error` — so a request without a
// known state can neither cancel an in-progress flow nor deliver anything.
const state = url.searchParams.get("state")
const entry = state ? pending.get(state) : undefined
if (!state || !entry) {
html(400, HTML_ERROR("Invalid or unknown sign-in state"))
return
}
pending.delete(state)

const error = url.searchParams.get("error")
if (error) {
entry.reject(new Error(error))
html(200, HTML_ERROR(error))
return
}

const token = url.searchParams.get("token")
const instance = url.searchParams.get("instance")
const apiUrl = url.searchParams.get("url") || DEFAULT_API_URL
if (!token || !instance) {
const msg = "Missing credential in callback"
entry.reject(new Error(msg))
html(400, HTML_ERROR(msg))
return
}

entry.resolve({ api_url: apiUrl, instance, token })
html(200, HTML_SUCCESS)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The browser says "Signed in" before we've actually finished signing in

MINORNEW-6 — browser shows "Signed in ✓" before the token exchange/persist succeeds. The loopback returns HTML_SUCCESS here as soon as it receives the token; if exchangeSocialToken()/save then fails, the browser says success while the terminal reports failure.

Fix: use neutral "authorization received, return to terminal" copy, or delay success until exchange + persist succeed.

Flagged by: GPT, MiniMax

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Fixed in 40c80361c (NEW-6). HTML_SUCCESS is now neutral — "Authorization received / Return to your terminal to finish connecting." — so the browser no longer claims success before the exchange + persist complete. The terminal remains the source of truth for actual success/failure.

})

try {
await new Promise<void>((resolve, reject) => {
// Bind to loopback only — the credential/abort endpoints must not be reachable
// from the LAN.
server!.listen(CALLBACK_PORT, "127.0.0.1", () => resolve())
server!.on("error", reject)
})
} catch (err) {
// Reset so a retry isn't blocked by the `if (server) return` guard, and surface
// a clear reason (e.g. the port is already taken) instead of hanging.
server = undefined
const code = (err as NodeJS.ErrnoException)?.code
throw new Error(
code === "EADDRINUSE"
? `Port ${CALLBACK_PORT} is already in use — close whatever is using it and try again.`
: `Could not start the sign-in server: ${err instanceof Error ? err.message : String(err)}`,
)
}
}

function stopCallbackServer() {
if (server) {
server.close()
server = undefined
}
}

// Register a pending flow keyed by `state` and return its promise. Called
// synchronously in authorize() before the browser opens; the server handler
// resolves/rejects it by state, so a fast redirect is never lost.
function registerPending(state: string, timeoutMs = 5 * 60 * 1000): Promise<CallbackResult> {
return new Promise<CallbackResult>((resolve, reject) => {
const timeout = setTimeout(() => {
if (pending.delete(state)) reject(new Error("Timed out waiting for browser sign-in"))
}, timeoutMs)
pending.set(state, {
resolve: (creds) => {
clearTimeout(timeout)
resolve(creds)
},
reject: (err) => {
clearTimeout(timeout)
reject(err)
},
})
})
}

export async function AltimateAuthPlugin(_input: PluginInput): Promise<Hooks> {
return {
auth: {
provider: "altimate-backend",
methods: [
{
type: "oauth",
label: "Altimate LLM Gateway",
async authorize() {
const state = randomBytes(16).toString("hex")
await startCallbackServer()

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The browser can hand back the login before we're listening for it

MAJORRace — pending is registered too late. startCallbackServer() binds the port and starts accepting requests here inside authorize(), but the module-level pending (state + resolve/reject) is only set later when the framework calls the returned callback() (waitForCallback, ~L114). If the browser redirects before callback() runs (an already-signed-in user gets an instant redirect, or any scheduling gap), /callback sees pending === undefined and rejects as "Invalid state — possible CSRF", silently dropping the credential. The singleton also can't support two concurrent /auth flows — the second overwrites the first.

Fix: register pending keyed by state (e.g. a Map) synchronously in authorize() before opening the browser; have callback() await that promise, and buffer an early callback by state until it's awaited.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Fixed in e46fdd48b. registerPending(state) now runs synchronously inside authorize() before open(), keying the pending flow by the unguessable state. The /callback handler resolves/rejects by that state, so an instant redirect (already-signed-in user) is matched instead of dropped as CSRF. See altimate.ts:162-171.

// Register the pending flow BEFORE opening the browser so an instant
// redirect can be matched by state rather than dropped as CSRF.
const result = registerPending(state)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Backing out of sign-in leaves the server and pending flow running for 5 minutes

MINORNEW-3 (residual of prior #2) — dismissing the dialog doesn't cancel the server-side flow. The onCleanup+disposed guard fixes the UI leak, but nothing rejects this eagerly-created registerPending promise on dismiss, so callback() keeps awaiting, the loopback server stays up, and the pending entry lingers until the 5-minute timeout.

Fix: on dismiss, reject/cancel the pending flow by state (and stop the server if it was the last one). Also attach a no-op .catch() to this registerPending promise so a never-awaited rejection can't become an unhandled rejection.

Flagged by: Kimi (MAJOR), MiMo, Claude — consensus

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Partially addressed in 40c80361c (NEW-3). Added void result.catch(() => {}) so the eagerly-created registerPending promise can't surface as an unhandled rejection when the dialog is dismissed. The eager reject-by-state on dismiss isn't feasible from here: the TUI dismiss path never receives state, so it can't reach the loopback pending map. The backstop remains the 5-min timeout (rejects the pending entry) and callback()'s finally (stops the server when it was the last flow). Happy to open a follow-up if we want true immediate cancel — it'd require threading state back out to the dialog.

// If callback() is never awaited (e.g. the dialog is dismissed before it
// runs), the pending promise still rejects on timeout — swallow that here
// so it can't surface as an unhandled rejection. callback() awaits the
// same promise independently.
void result.catch(() => {})

const webUrl = (process.env.ALTIMATE_WEB_URL || DEFAULT_WEB_URL).replace(/\/+$/, "")
// Use 127.0.0.1 to match the loopback bind — a plain `localhost` redirect
// can resolve to ::1 first and hit a closed IPv6 port.
const redirect = `http://127.0.0.1:${CALLBACK_PORT}/callback`
// Land on the sign-up page and let the user choose how to authenticate
// (Google today, more providers later) rather than forcing Google.
const authorizeUrl =
`${webUrl}/register?client=altimate-code` +
`&redirect=${encodeURIComponent(redirect)}` +
`&state=${state}`

await open(authorizeUrl).catch(() => undefined)

return {
url: authorizeUrl,
instructions: "Complete sign-in in your browser to connect Altimate LLM Gateway.",
method: "auto",
async callback() {
try {
const creds = await result
// The instance name comes from the browser callback; validate it
// before persisting (the manual paste path validates too).
if (!AltimateApi.isValidInstanceName(creds.instance)) {
throw new Error(`invalid instance name from callback: ${creds.instance}`)
}
// Exchange the short-lived, one-time login_token for the gateway
// auth_token server-side — the raw api_key never rides in the URL.
const authToken = await AltimateApi.exchangeSocialToken(creds.api_url, creds.instance, creds.token)
// Persist to ~/.altimate/altimate.json — the provider loader
// reads this first (it carries the instance/tenant + api_url
// the generic auth.json store can't).
await AltimateApi.saveCredentials({

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Browser sign-in skips the tenant-name check that manual entry enforces

MINOROAuth callback skips the tenant-name validation the manual path enforces. The manual paste path runs validateCredentials() (tenant regex + API check) before saving; this OAuth path calls saveCredentials() directly on the callback instance. The source is the trusted web app, so risk is low, but the invariant differs between the two entry points.

Fix: validate instance against VALID_TENANT_REGEX before persisting, or document why the OAuth source is trusted.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Fixed in f5596b40a. The OAuth path now validates AltimateApi.isValidInstanceName(creds.instance) against VALID_TENANT_REGEX before exchange/persist, matching the manual paste path's invariant. altimate.ts:195.

altimateUrl: creds.api_url,
altimateInstanceName: creds.instance,
altimateApiKey: authToken,
})
return { type: "success", key: authToken, provider: "altimate-backend" }
} catch (err) {
// Log the reason (CSRF / timeout / invalid instance / …). Runs in the
// server process, so this goes to the log, not the TUI display.
console.error("[altimate] gateway sign-in failed:", err instanceof Error ? err.message : err)
return { type: "failed" }

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

When sign-in fails, we throw away the reason

MINORFailed-auth callback discards the error reason. The catch returns { type: "failed" } with no log and no reason, so CSRF/timeout/unknown failures are indistinguishable and undebuggable in the field.

Fix: log the caught error and thread a reason (e.g. csrf / timeout / unknown) through the failure result.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Fixed in f5596b40a (log) + 40c80361c (UX). callback()'s catch now logs the reason via console.error("[altimate] gateway sign-in failed:", ...), and per NEW-4 the reason is surfaced to the user through toast.error(...) in the AutoMethod path.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

When sign-in fails, the user sees a generic error with no reason

MINORNEW-4 (UX) — auth/callback failures are opaque to the user. The plugin callback() logs the reason via console.error but returns bare { type: "failed" }; OauthCallbackFailed carries no message (auth-service.ts), so the TUI shows a generic failure with no reason (port busy, timeout, exchange 401, invalid instance).

Fix: thread a reason string through the failed result (or surface it via toast.error in the callback path).

Flagged by: MiniMax (rated CRITICAL), GPT, Kimi, MiMo — consensus

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Fixed in 40c80361c (NEW-4). AutoMethod now calls toast.error(...) on result.error instead of clearing silently, so port-busy/timeout/exchange-401/invalid-instance failures are visible in the TUI. The precise reason is still logged server-side via console.error.

} finally {
// Keep the shared server up while another flow is still waiting.
if (pending.size === 0) stopCallbackServer()
}
},
}
},
},
{
// Fallback: paste an instance-name::api-key manually.
type: "api",
label: "Connect to Altimate",
label: "Paste API key",
},
],
},
Expand Down
3 changes: 3 additions & 0 deletions packages/opencode/src/cli/cmd/providers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -319,6 +319,9 @@ export const ProvidersLoginCommand = cmd({
})

const priority: Record<string, number> = {
// altimate_change start — surface the Altimate LLM Gateway first
"altimate-backend": -1,
// altimate_change end
opencode: 0,
openai: 1,
"github-copilot": 2,
Expand Down
Loading
Loading