From b47f0a89e43e8b24c01798e3be70cbc4fcc30856 Mon Sep 17 00:00:00 2001 From: Yash Datta Date: Wed, 9 Sep 2026 13:39:44 +0200 Subject: [PATCH 1/2] feat: sign in to a backend from codeoid, instead of pasting an API key MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Every backend codeoid runs already has a first-party login that mints a subscription credential — `claude setup-token`, `codex login`, qwen's OAuth. Each is an interactive terminal command, so using one meant having a shell on the machine the daemon runs on. When codeoid IS the whole surface — a hosted sandbox, a phone — nobody does, and an API key was the only way in. The gap was never that login is impossible; it was that it was unreachable from the only UI the user has. Settings → Claude now has it: open the link, paste back the code, done. ## Brokering, not reimplementing The daemon runs the vendor's own command. It does not implement anyone else's OAuth. That distinction is the design, not an implementation detail — scraping a client id out of someone's CLI and driving their token endpoint ourselves works right until they change it, and puts us in the business of maintaining another company's auth. Running the command they ship means their flow can change under us and keep working. Two steps, because that is the shape their commands already have: 1. `backend.login.start` — run the command, return the authorize URL it prints. The user opens it in THEIR browser; the daemon never needs one, which is what makes this work headless. 2. `backend.login.submit` — the vendor's callback page shows a code, the user pastes it back, the daemon hands it to the still-waiting command. No new dependency: the `claude` binary is already in `node_modules`, shipped by the Agent SDK's platform package, and is what every Claude turn already runs through. Verified end to end against it — real `setup-token`, real PKCE URL, 4.5s. ## The awkward part is the terminal These commands are TUIs. `claude setup-token` writes nothing at all to a pipe: it detects the absence of a TTY and waits forever (verified — zero bytes, hung). Under a pty it prints its URL in under two seconds. So it needs a pty, and codeoid has no pty dependency; node-pty is a native module, i.e. a build toolchain in every image for one feature. `script(1)` is the pty — util-linux on Linux (Essential, so present even in debian-slim), BSD script on macOS, a different argv on each. What comes back is raw terminal output: ANSI, OSC-8 hyperlinks, spinner redraws, an 80-column wrap that chops the URL across lines. Rather than strip escapes and hope, every pattern matches on a character class that control bytes cannot appear in — so escapes are simply not matchable — and longest-match wins, which is what picks the hyperlink's unwrapped copy of the URL over the visible truncated one. ## What the tests found They run a FAKE vendor command through the REAL pty and the real matchers, because the parts likely to be wrong here are not the state machine but the seam with a terminal. That caught a bug worth naming: a pty echoes its input, so a user who pastes something credential-shaped (the exact mistake made when "code" is confused with "key") had their own typo read back as the vendor's answer — success reported, garbage stored. Nothing a vendor returns for an exchange is a substring of the code that requested it, and that is now the rule. ## Storage, scope, hygiene The credential is written through the ordinary settings path, so it lands in the same 0600 `.env`, shows as set in the same snapshot, and is cleared by the same control as one that was typed — a credential the UI can create but not show or revoke is one nobody can reason about. Gated on `settings:write`, not a new scope: a completed login writes to the same file that scope already governs, so a separate one would imply a privilege boundary that does not exist and cost a token migration to express. The submitted code and the resulting credential are never logged, never echoed to a client, and never in an error; the transcript is dropped when the attempt ends. One attempt per backend, superseded on restart so a reloaded page is not locked out by its own abandonment, killed by process group on cancel and on daemon drain. Claude is wired. codex, gemini and qwen are a `LoginFlow` entry each — filed separately rather than guessed at, since each one's terminal output has to be read before its patterns can be written. --- CHANGELOG.md | 19 + docs/CONFIGURATION.md | 20 +- packages/protocol/src/backend-login.ts | 127 ++++++ packages/protocol/src/index.ts | 1 + packages/protocol/src/schemas.test.ts | 12 + packages/protocol/src/schemas.ts | 30 ++ packages/protocol/src/types.ts | 20 + src/daemon/auth/backend-login.ts | 545 +++++++++++++++++++++++ src/daemon/session-manager.ts | 114 +++++ src/daemon/settings/manifest.ts | 12 +- src/tests/backend-login.test.ts | 301 +++++++++++++ src/tests/protocol.test.ts | 6 + src/tests/settings.test.ts | 40 +- web/src/components/BackendLoginPanel.tsx | 178 ++++++++ web/src/components/SettingsDrawer.tsx | 19 + web/src/state/backend-login.test.ts | 133 ++++++ web/src/state/backend-login.ts | 151 +++++++ web/src/state/settings.ts | 11 + 18 files changed, 1736 insertions(+), 3 deletions(-) create mode 100644 packages/protocol/src/backend-login.ts create mode 100644 src/daemon/auth/backend-login.ts create mode 100644 src/tests/backend-login.test.ts create mode 100644 web/src/components/BackendLoginPanel.tsx create mode 100644 web/src/state/backend-login.test.ts create mode 100644 web/src/state/backend-login.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index 97ba8fc2..99e5300e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,25 @@ to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). ## [Unreleased] +### Added + +- **Sign in with Claude, instead of pasting an API key.** Settings → Claude now + has a two-step sign-in: codeoid runs `claude setup-token` for you, shows the + link to approve in your own browser, and takes back the code that page + displays. The subscription credential it mints is stored as + `CLAUDE_CODE_OAUTH_TOKEN` in `~/.codeoid/.env` like any other secret, and + applies to new sessions. + + This is for the case where codeoid is the whole surface — a hosted sandbox, a + phone — and there is no shell in which to run the vendor's login by hand. On a + machine with a shell, `claude login` still works and is still picked up. + + codeoid brokers the vendor's own command rather than implementing anyone + else's OAuth, and it runs the `claude` binary the Agent SDK already ships, so + there is no new dependency. Wire messages: `backend.login.start` / `.submit` / + `.cancel`, all gated on `settings:write`. Claude is wired today; the mechanism + is per-backend and the others follow. + ## [0.4.0] - 2026-07-29 codeoid moves to the Highflame npm org. npm has no way to transfer a package diff --git a/docs/CONFIGURATION.md b/docs/CONFIGURATION.md index cdc49d34..0fe43582 100644 --- a/docs/CONFIGURATION.md +++ b/docs/CONFIGURATION.md @@ -160,5 +160,23 @@ A variable already set in the real environment still wins. # ~/.codeoid/.env TELEGRAM_BOT_TOKEN=123456:AA... TELEGRAM_ALLOWED_USER_IDS=6714605885 -# ANTHROPIC_API_KEY= # only if not logged in via `claude login` +# CLAUDE_CODE_OAUTH_TOKEN= # written by Settings → Claude → "Sign in with Claude" +# ANTHROPIC_API_KEY= # only if you have neither a sign-in nor a Claude Code login ``` + +## Signing a backend in, instead of pasting a key + +Settings → **Claude** → **Sign in with Claude** runs the backend's own login +(`claude setup-token`) inside the daemon and stores the subscription credential +it mints as `CLAUDE_CODE_OAUTH_TOKEN` in the `.env` above. Two steps: open the +link it shows, then paste back the code the page displays. + +This exists for the case where codeoid IS the whole surface — a hosted sandbox, +a phone — and there is no shell in which to run the vendor's login by hand. On a +machine where you do have a shell, `claude login` still works and codeoid picks +that credential up unchanged; nothing here replaces it. + +The sign-in requires the `settings:write` scope, because it writes to the same +`.env` that scope already governs. It takes effect for **new sessions** — a +running session keeps the environment it started with. Only Claude is wired +today; the mechanism is per-backend and the others follow. diff --git a/packages/protocol/src/backend-login.ts b/packages/protocol/src/backend-login.ts new file mode 100644 index 00000000..0a2275ce --- /dev/null +++ b/packages/protocol/src/backend-login.ts @@ -0,0 +1,127 @@ +/** + * Interactive backend sign-in — the wire contract for logging a backend in from + * a codeoid client, instead of pasting a provider API key. + * + * WHY THIS EXISTS. Every backend codeoid runs already has a first-party login + * that mints a subscription credential — `claude setup-token`, `codex login`, + * qwen's OAuth. Each one is an interactive terminal command, so using it meant + * having a shell on the machine the daemon runs on. When codeoid IS the whole + * surface (a hosted sandbox, a phone), nobody does, and an API key was the only + * way in. That is the gap: not that login is impossible, but that it was + * unreachable from the only UI the user has. + * + * The daemon BROKERS the vendor's own command; it never reimplements the + * vendor's OAuth. That distinction is the whole design. Scraping a client id out + * of someone else's CLI and driving their token endpoint ourselves would work + * right up until they change it, and would put us in the business of + * maintaining another company's auth. Running the command they ship means their + * flow changes under us and keeps working. + * + * Two steps, because that is the shape the vendors' commands already use: + * + * 1. `backend.login.start` — the daemon runs the login command and returns the + * authorize URL it prints. The user opens that URL in THEIR browser; the + * daemon never needs one, which is what makes this work headless. + * 2. `backend.login.submit` — the vendor's callback page displays a code. The + * user pastes it back, the daemon hands it to the still-waiting command, + * and the credential the command produces is stored exactly like any other + * secret (`~/.codeoid/.env`, 0600) so the rest of codeoid needs no + * special case for it. + * + * A submitted code and the resulting credential are never echoed back to a + * client and never logged. + */ + +import type { SettingsSnapshot } from "./settings.js"; + +/** + * Backends with an interactive login wired end to end. + * + * Deliberately narrower than the backend catalog: a backend appears here only + * once its login command is driven and tested, so a client can offer "Sign in" + * without first asking the daemon whether it would work. Adding one is an entry + * here plus a flow in the daemon's broker. + */ +export type LoginBackend = "claude"; + +/** Runtime companion to {@link LoginBackend} — what a client renders a button for. */ +export const LOGIN_CAPABLE_BACKENDS: readonly LoginBackend[] = ["claude"] as const; + +/** A login the daemon has started and is holding open, waiting for a code. */ +export interface PendingBackendLogin { + /** Opaque handle for the in-flight attempt; required to submit or cancel. */ + loginId: string; + backend: LoginBackend; + /** The vendor URL the user opens in their OWN browser to approve. */ + verificationUrl: string; + /** Epoch ms after which the daemon abandons the attempt and kills the command. */ + expiresAt: number; + /** One line naming what the user brings back — wording is vendor-specific. */ + codeHint: string; +} + +// ── Messages (client → daemon) ──────────────────────────────────────────────── + +/** + * Begin an interactive login. Resolves only once the vendor command has printed + * its authorize URL, so a client gets a URL or an error — never a handle to an + * attempt it cannot show the user. Starting a login supersedes any attempt + * already in flight for that backend (a reloaded page must not be locked out by + * its own abandoned attempt). + */ +export interface BackendLoginStartMsg { + type: "backend.login.start"; + id: string; + backend: LoginBackend; +} + +/** + * Hand the vendor's code to the waiting command and wait for the exchange. + * Terminal either way: on failure the attempt is finished and the client must + * start a new one, because the command's own retry loop is not reachable + * through this protocol. + */ +export interface BackendLoginSubmitMsg { + type: "backend.login.submit"; + id: string; + loginId: string; + /** The code the vendor's callback page displayed. Never logged or echoed. */ + code: string; +} + +/** Abandon an in-flight attempt and kill the command. Idempotent. */ +export interface BackendLoginCancelMsg { + type: "backend.login.cancel"; + id: string; + loginId: string; +} + +// ── Messages (daemon → client) ──────────────────────────────────────────────── + +export interface BackendLoginStartResultMsg { + type: "backend.login.start.result"; + requestId: string; + login: PendingBackendLogin; +} + +export interface BackendLoginSubmitResultMsg { + type: "backend.login.submit.result"; + requestId: string; + ok: boolean; + /** + * Why it failed, safe to display — the vendor's own rejection, with anything + * credential-shaped stripped. Absent when `ok`. + */ + error?: string; + /** + * Settings AFTER the write, so the drawer reflects the new credential without + * a second round trip. The stored secret shows as set; its value never moves. + */ + snapshot: SettingsSnapshot; +} + +export interface BackendLoginCancelResultMsg { + type: "backend.login.cancel.result"; + requestId: string; + ok: boolean; +} diff --git a/packages/protocol/src/index.ts b/packages/protocol/src/index.ts index 5007fe5c..8643c244 100644 --- a/packages/protocol/src/index.ts +++ b/packages/protocol/src/index.ts @@ -1,3 +1,4 @@ export * from "./types.js"; export * from "./scopes.js"; export * from "./settings.js"; +export * from "./backend-login.js"; diff --git a/packages/protocol/src/schemas.test.ts b/packages/protocol/src/schemas.test.ts index 94b8998d..8c9b0b2e 100644 --- a/packages/protocol/src/schemas.test.ts +++ b/packages/protocol/src/schemas.test.ts @@ -127,6 +127,18 @@ const samples: { [T in ClientTypes]: Extract } = { nameOverride: "imported", }, "usage.daily": { type: "usage.daily", id: "r24", days: 30 }, + "backend.login.start": { type: "backend.login.start", id: "r25", backend: "claude" }, + "backend.login.submit": { + type: "backend.login.submit", + id: "r26", + loginId: "11111111-2222-3333-4444-555555555555", + code: "abc123#state", + }, + "backend.login.cancel": { + type: "backend.login.cancel", + id: "r27", + loginId: "11111111-2222-3333-4444-555555555555", + }, "settings.schema": { type: "settings.schema", id: "r29" }, "settings.get": { type: "settings.get", id: "r30" }, "settings.set": { diff --git a/packages/protocol/src/schemas.ts b/packages/protocol/src/schemas.ts index 3ccba0c2..64fa50eb 100644 --- a/packages/protocol/src/schemas.ts +++ b/packages/protocol/src/schemas.ts @@ -466,6 +466,33 @@ export const settingsSetSchema = z.object({ .max(256), }); +/** + * The backends a client may ask to sign in. An explicit enum, not a free + * string: `start` spawns a process chosen by this value, so the set of things + * it can name belongs in the validated surface rather than in a lookup that + * happens to miss. + */ +const loginBackendField = z.enum(["claude"]); + +export const backendLoginStartSchema = z.object({ + ...base, + type: z.literal("backend.login.start"), + backend: loginBackendField, +}); + +export const backendLoginSubmitSchema = z.object({ + ...base, + type: z.literal("backend.login.submit"), + loginId: z.string().min(1).max(128), + code: z.string().min(1).max(LIMITS.LOGIN_CODE_MAX), +}); + +export const backendLoginCancelSchema = z.object({ + ...base, + type: z.literal("backend.login.cancel"), + loginId: z.string().min(1).max(128), +}); + // ── The unions ──────────────────────────────────────────────────────────────── // ── SDLC pipeline ───────────────────────────────────────────────────────────── @@ -661,6 +688,9 @@ export const clientMessageSchema = z.discriminatedUnion("type", [ settingsSchemaSchema, settingsGetSchema, settingsSetSchema, + backendLoginStartSchema, + backendLoginSubmitSchema, + backendLoginCancelSchema, usageDailySchema, pipelineCreateSchema, pipelineListSchema, diff --git a/packages/protocol/src/types.ts b/packages/protocol/src/types.ts index 852129e3..cb5f3128 100644 --- a/packages/protocol/src/types.ts +++ b/packages/protocol/src/types.ts @@ -23,6 +23,14 @@ import type { SettingsGetResultMsg, SettingsSetResultMsg, } from "./settings.js"; +import type { + BackendLoginStartMsg, + BackendLoginSubmitMsg, + BackendLoginCancelMsg, + BackendLoginStartResultMsg, + BackendLoginSubmitResultMsg, + BackendLoginCancelResultMsg, +} from "./backend-login.js"; /** * Wire-protocol version. Bump on breaking changes (renamed/removed fields, @@ -184,6 +192,12 @@ export const LIMITS = { * fit comfortably under 8 KiB. */ SETTING_VALUE_MAX: 8192, + /** + * Max length of a `backend.login.submit` code. A vendor's out-of-band code is + * a short opaque string (a few hundred bytes with its state suffix); the cap + * is here so a blob never reaches a live pty, not to fit any real code. + */ + LOGIN_CODE_MAX: 1024, /** Max free-text length on a `session.ui_response` (`value`). */ UI_TEXT_MAX: 65_536, /** Max number of options on a `session.ui_request` select. */ @@ -968,6 +982,9 @@ export type ClientMessage = | SettingsSchemaMsg | SettingsGetMsg | SettingsSetMsg + | BackendLoginStartMsg + | BackendLoginSubmitMsg + | BackendLoginCancelMsg | UsageDailyMsg | PipelineCreateMsg | PipelineListMsg @@ -2479,6 +2496,9 @@ export type DaemonMessage = | SettingsSchemaResultMsg | SettingsGetResultMsg | SettingsSetResultMsg + | BackendLoginStartResultMsg + | BackendLoginSubmitResultMsg + | BackendLoginCancelResultMsg | PipelineSnapshotMsg | PipelineListResultMsg | PackListResultMsg diff --git a/src/daemon/auth/backend-login.ts b/src/daemon/auth/backend-login.ts new file mode 100644 index 00000000..bfff30fd --- /dev/null +++ b/src/daemon/auth/backend-login.ts @@ -0,0 +1,545 @@ +/** + * Backend login broker — runs a backend's OWN login command and brokers its two + * interactive steps to a codeoid client. + * + * See `packages/protocol/src/backend-login.ts` for why the daemon drives the + * vendor's command rather than implementing the vendor's OAuth. This file is + * the mechanism: a small amount of process wrangling around one awkward fact. + * + * THE AWKWARD FACT: these commands are terminal UIs. `claude setup-token` + * writes nothing at all to a pipe — it detects the absence of a TTY and waits + * forever. Verified: piped stdin/stdout produced zero bytes and hung; the same + * command under a pty printed its authorize URL in under two seconds. So the + * command needs a pty, and codeoid has no pty dependency (node-pty is a native + * module — a build toolchain in every image, for one feature). `script(1)` is + * the pty: util-linux on Linux (Essential, so present even in debian-slim), + * BSD script on macOS, different argv on each. + * + * WHAT WE READ BACK is raw terminal output — ANSI, OSC-8 hyperlinks, spinner + * redraws, an 80-column wrap that chops the URL across lines. Rather than strip + * escapes and hope, every pattern here is chosen to match on a character class + * that terminal control bytes cannot appear in, so the escapes are simply not + * matchable and the longest match wins (the OSC-8 copy of the URL is the + * unwrapped one). + * + * SECRET HYGIENE. The transcript holds a credential by the end, and the code + * the user submits is a bearer of one. Neither is ever logged, returned to a + * client, or included in an error; {@link redact} runs over anything that + * escapes this module. + */ + +import { spawn, type ChildProcess } from "node:child_process"; +import { randomUUID } from "node:crypto"; +import { createRequire } from "node:module"; +import type { LoginBackend, PendingBackendLogin } from "../../protocol/types.js"; +import { buildSubprocessEnv } from "../providers/env.js"; + +/** Longest we hold a started attempt open before killing it. */ +const ATTEMPT_TTL_MS = 10 * 60_000; +/** Longest we wait for the command to print its authorize URL. */ +const URL_TIMEOUT_MS = 45_000; +/** Longest we wait for the vendor to accept or reject a submitted code. */ +const EXCHANGE_TIMEOUT_MS = 90_000; +/** + * Transcript cap. We keep the TAIL (the outcome), never the whole stream: a + * spinner redrawing at 10 Hz for ten minutes is megabytes of nothing, and the + * URL is captured out of the stream the moment it appears rather than kept by + * holding the bytes it arrived in. + */ +const TRANSCRIPT_MAX = 64 * 1024; + +/** Reject a code that could not be one, before it reaches a live process. */ +const CODE_MAX = 1024; + +/** + * One backend's login command and how to read its terminal output. + * + * Adding a backend is an entry in {@link FLOWS}. The shape is deliberately + * declarative — no per-backend branching anywhere else in this file — because + * the parts that differ between vendors are exactly these five, and the parts + * that are hard (pty, timeouts, cancellation, redaction) are the same for all. + */ +export interface LoginFlow { + backend: LoginBackend; + /** + * Resolve the command to run, or `null` when this installation has no such + * binary — a clean "not available here" rather than a spawn failure. + */ + resolve(): { file: string; args: string[] } | null; + /** Matches the authorize URL in raw terminal output. */ + urlPattern: RegExp; + /** Matches the vendor's own rejection of a submitted code. */ + failurePattern: RegExp; + /** Shown to the user beside the code box; vendor-specific wording. */ + codeHint: string; + /** + * Pull the credential out of the finished transcript. + * + * `null` is NOT failure — some commands write their credential to disk and + * print nothing worth keeping. It means "nothing for codeoid to store", and + * the attempt still succeeds on the command's exit status. + */ + extractSecret(transcript: string): { key: string; value: string } | null; +} + +const require_ = createRequire(import.meta.url); + +/** + * The `claude` binary that the Agent SDK already ships. + * + * codeoid does not depend on the Claude Code CLI and does not need to: the + * Agent SDK's platform package (`@anthropic-ai/claude-agent-sdk-`) + * contains the real binary, and it is what every Claude turn already runs + * through. So this feature adds no dependency and no image bytes — it runs the + * executable that is by definition present wherever the Claude backend works. + * + * The musl variant is tried second so an Alpine install resolves; a plain + * `claude` on PATH is the last resort for an unusual layout. + */ +function resolveClaudeBinary(): string | null { + const base = `@anthropic-ai/claude-agent-sdk-${process.platform}-${process.arch}`; + for (const pkg of [base, `${base}-musl`]) { + try { + return require_.resolve(`${pkg}/claude`); + } catch { + // Not this variant — try the next. + } + } + return null; +} + +/** + * `sk-ant-oat01-…` — the long-lived OAuth token `setup-token` mints. Bounded + * character class so a trailing ANSI reset or newline is not swallowed into it. + */ +const CLAUDE_OAUTH_TOKEN = /sk-ant-oat01-[A-Za-z0-9_-]{20,}/; + +export const CLAUDE_LOGIN_FLOW: LoginFlow = { + backend: "claude", + resolve() { + const bin = resolveClaudeBinary(); + return bin ? { file: bin, args: ["setup-token"] } : null; + }, + // The character class stops at any terminal control byte, so the OSC-8 + // hyperlink wrapping this URL cannot bleed into the match — and the + // hyperlink's copy is the one that is not line-wrapped, hence longest-wins. + urlPattern: /https:\/\/claude\.com\/[A-Za-z0-9/_.~-]*oauth\/authorize\?[A-Za-z0-9_=&%.~+-]+/g, + // The command prints this and then offers "Press Enter to retry", i.e. it + // stays alive. We treat it as terminal anyway: a retry loop driven one + // round-trip at a time through a websocket is a worse experience than + // pressing the button again, and leaving the process alive after a failure we + // have already reported is a leak. + failurePattern: /OAuth error:/, + codeHint: "Approve in the browser, then paste the code the page shows.", + extractSecret(transcript) { + const m = CLAUDE_OAUTH_TOKEN.exec(transcript); + // No token in the output is not a failure: `setup-token` also writes + // ~/.claude/.credentials.json, which the Agent SDK reads directly. Storing + // the token when we can see it is strictly better — it survives a container + // whose home directory does not — but its absence just means the on-disk + // credential is the one doing the work. + return m ? { key: "CLAUDE_CODE_OAUTH_TOKEN", value: m[0] } : null; + }, +}; + +const FLOWS: Record = { + claude: CLAUDE_LOGIN_FLOW, +}; + +/** Anything credential-shaped, gone — applied to every string that leaves here. */ +export function redact(text: string): string { + return text + .replace(/sk-ant-[A-Za-z0-9]+-[A-Za-z0-9_-]{8,}/g, "«redacted»") + .replace(/sk-[A-Za-z0-9_-]{20,}/g, "«redacted»"); +} + +export class BackendLoginError extends Error {} + +interface Attempt { + loginId: string; + backend: LoginBackend; + flow: LoginFlow; + child: ChildProcess; + transcript: string; + /** + * Characters discarded off the FRONT by {@link capTail}, so an offset taken + * before a trim still addresses the same point in the stream. Without this a + * long-running attempt (a spinner redrawing for minutes) would silently shift + * the "everything since the code was submitted" window, and the exchange + * could be judged against output from before it. + */ + dropped: number; + verificationUrl: string | null; + expiresAt: number; + ttlTimer: ReturnType; + exited: boolean; + exitCode: number | null; + /** Woken on every chunk of output and on exit, so waiters can re-test. */ + wake: (() => void)[]; +} + +export interface LoginOutcome { + ok: boolean; + /** Present on success when the command emitted a credential worth storing. */ + secret?: { key: string; value: string }; + /** Redacted, user-safe. Present on failure. */ + error?: string; +} + +/** + * Holds at most one in-flight login per backend. + * + * `flows` is injectable so tests drive a fake login command through the REAL + * pty and the real matching — the parts that are easy to get wrong — without + * reaching a vendor. + */ +export class BackendLoginBroker { + readonly #flows: Record; + readonly #byId = new Map(); + readonly #byBackend = new Map(); + + constructor(flows: Record = FLOWS) { + this.#flows = flows; + } + + /** + * Run the backend's login command and resolve once it has printed its URL. + * + * Supersedes any attempt already in flight for this backend. A user who + * reloads the page must not be locked out by their own abandoned attempt, and + * two live logins for one backend would race to write the same credential. + */ + async start(backend: LoginBackend): Promise { + const flow = this.#flows[backend]; + if (!flow) throw new BackendLoginError(`No interactive login for backend '${backend}'.`); + + const resolved = flow.resolve(); + if (!resolved) { + throw new BackendLoginError( + `The ${backend} login command is not installed on this machine, so codeoid cannot sign in for you.`, + ); + } + + const superseded = this.#byBackend.get(backend); + if (superseded) this.cancel(superseded); + + const child = spawnUnderPty(resolved.file, resolved.args); + const loginId = randomUUID(); + const attempt: Attempt = { + loginId, + backend, + flow, + child, + transcript: "", + dropped: 0, + verificationUrl: null, + expiresAt: Date.now() + ATTEMPT_TTL_MS, + ttlTimer: setTimeout(() => this.cancel(loginId), ATTEMPT_TTL_MS), + exited: false, + exitCode: null, + wake: [], + }; + // `unref` so a forgotten attempt cannot hold the process open at shutdown. + attempt.ttlTimer.unref?.(); + this.#byId.set(loginId, attempt); + this.#byBackend.set(backend, loginId); + + const absorb = (buf: Buffer | string) => { + append(attempt, String(buf)); + if (!attempt.verificationUrl) { + attempt.verificationUrl = longestMatch(attempt.transcript, flow.urlPattern); + } + drain(attempt); + }; + child.stdout?.on("data", absorb); + child.stderr?.on("data", absorb); + child.on("error", (err) => { + append(attempt, `\n${err.message}`); + attempt.exited = true; + drain(attempt); + }); + child.on("exit", (code) => { + attempt.exited = true; + attempt.exitCode = code; + drain(attempt); + }); + + try { + await waitFor(attempt, URL_TIMEOUT_MS, () => attempt.verificationUrl !== null || attempt.exited); + } catch { + this.cancel(loginId); + throw new BackendLoginError( + `The ${backend} login command did not produce a sign-in link in time.`, + ); + } + if (!attempt.verificationUrl) { + const tail = redact(lastLine(attempt.transcript)); + this.cancel(loginId); + throw new BackendLoginError( + `The ${backend} login command exited before producing a sign-in link${tail ? `: ${tail}` : "."}`, + ); + } + + return { + loginId, + backend, + verificationUrl: attempt.verificationUrl, + expiresAt: attempt.expiresAt, + codeHint: flow.codeHint, + }; + } + + /** + * Hand the vendor's code to the waiting command and wait for the exchange. + * + * Terminal either way — the attempt is disposed before returning, so a + * rejected code means starting again rather than retrying into a process + * whose state we can no longer describe. + */ + async submit(loginId: string, code: string): Promise { + const attempt = this.#byId.get(loginId); + if (!attempt) { + return { ok: false, error: "That sign-in attempt is no longer open. Start again." }; + } + const trimmed = code.trim(); + // The code goes to a process's stdin, not a shell, so there is nothing to + // inject — but a newline would submit a second phantom answer to whatever + // the command asks next, and an unbounded blob is just a way to make a + // pty misbehave. Refuse both rather than sanitise into something the user + // did not type. + if (trimmed.length === 0 || trimmed.length > CODE_MAX || /[\r\n\x00]/.test(trimmed)) { + return { ok: false, error: "That does not look like a sign-in code." }; + } + if (attempt.exited) { + this.#dispose(attempt); + return { ok: false, error: "The sign-in command exited before the code arrived. Start again." }; + } + + // Mark where the answer begins, so a failure pattern already in the + // transcript (from the URL step) cannot be read as a rejection of THIS + // code. Stream-absolute, so a mid-exchange trim cannot move it. + const mark = attempt.dropped + attempt.transcript.length; + attempt.child.stdin?.write(`${trimmed}\r`); + + const sinceMark = () => attempt.transcript.slice(Math.max(0, mark - attempt.dropped)); + + /** + * The credential the VENDOR minted — never the one the user typed. + * + * A pty echoes its input, so everything submitted lands in the transcript + * a few bytes after the mark, indistinguishable from command output. Paste + * something credential-shaped (the exact mistake a user makes when they + * confuse "code" with "key") and the naive read is: match found, exchange + * succeeded, store it — a reported sign-in that authenticates as nothing. + * + * Containment rather than equality, because a wrapped echo can be broken + * across lines and match as a PREFIX of what was typed. Nothing a vendor + * returns for an exchange is a substring of the code that requested it. + */ + const mintedSecret = () => { + const found = attempt.flow.extractSecret(sinceMark()); + return found && !trimmed.includes(found.value) ? found : null; + }; + + const settled = () => { + if (attempt.flow.failurePattern.test(sinceMark())) return true; + if (mintedSecret()) return true; + return attempt.exited; + }; + + try { + await waitFor(attempt, EXCHANGE_TIMEOUT_MS, settled); + } catch { + this.#dispose(attempt); + return { ok: false, error: "The sign-in did not complete in time. Start again." }; + } + + const since = sinceMark(); + const secret = mintedSecret() ?? undefined; + const rejected = attempt.flow.failurePattern.test(since); + this.#dispose(attempt); + + if (secret && !rejected) return { ok: true, secret }; + if (rejected) { + return { ok: false, error: `${redact(vendorReason(since))} Start the sign-in again.` }; + } + // Exited with no credential in the output: trust the exit status. A command + // that stores its credential on disk (and prints nothing) lands here, and + // so does one that failed in a way we have no pattern for — the exit code + // is what separates them. + if (attempt.exitCode === 0) return { ok: true }; + return { + ok: false, + error: `The ${attempt.backend} sign-in command failed${ + attempt.exitCode === null ? "" : ` (exit ${attempt.exitCode})` + }. Start again.`, + }; + } + + /** Abandon an attempt and kill its command. Idempotent. */ + cancel(loginId: string): boolean { + const attempt = this.#byId.get(loginId); + if (!attempt) return false; + this.#dispose(attempt); + return true; + } + + /** Kill every in-flight attempt — daemon shutdown. */ + dispose(): void { + for (const id of [...this.#byId.keys()]) this.cancel(id); + } + + #dispose(attempt: Attempt): void { + clearTimeout(attempt.ttlTimer); + this.#byId.delete(attempt.loginId); + if (this.#byBackend.get(attempt.backend) === attempt.loginId) { + this.#byBackend.delete(attempt.backend); + } + killTree(attempt.child); + // Anything still awaiting this attempt must not hang on a dead process. + attempt.exited = true; + drain(attempt); + // Drop the transcript: it is the one place a credential sits in memory + // longer than the exchange needs it. + attempt.transcript = ""; + } +} + +// ── Process plumbing ────────────────────────────────────────────────────────── + +/** + * Spawn `file args…` attached to a pty, via `script(1)`. + * + * Two dialects, because the flag that means "run this command" is not the same + * one on both platforms and getting it wrong looks like the command hanging: + * - util-linux (Linux): `script -q -e -c "" /dev/null` — `-e` is what + * makes the child's exit status ours rather than script's. + * - BSD (macOS): `script -q /dev/null ` — argv, so no quoting. + * + * `detached` puts the child in its own process group: killing `script` alone + * would orphan the command it wrapped, which for a login command means an + * abandoned OAuth attempt still holding a pty. + */ +function spawnUnderPty(file: string, args: string[]): ChildProcess { + const env = { + ...buildSubprocessEnv({ prefixes: ["ANTHROPIC_", "CLAUDE_", "LC_"] }), + // A login command tries to open a browser. On the machine a daemon runs on + // that is at best useless and at worst a browser nobody asked for on + // someone's desktop — the user opens the URL we return, on their own + // machine. Every command tested prints the URL regardless. + BROWSER: "/bin/true", + // Terminal UIs size their output to the pty. Fixing it keeps the URL from + // being wrapped differently on a client with a different terminal, and + // keeps the transcript deterministic. + COLUMNS: "100", + LINES: "40", + TERM: "xterm-256color", + }; + const argv = + process.platform === "darwin" + ? ["-q", "/dev/null", file, ...args] + : ["-q", "-e", "-c", shellQuote([file, ...args]), "/dev/null"]; + return spawn("script", argv, { env, detached: true, stdio: ["pipe", "pipe", "pipe"] }); +} + +/** POSIX single-quoting — the command path is ours, but it may contain spaces. */ +function shellQuote(parts: string[]): string { + return parts.map((p) => `'${p.replaceAll("'", `'\\''`)}'`).join(" "); +} + +function killTree(child: ChildProcess): void { + if (child.exitCode !== null || child.signalCode !== null) return; + try { + // Negative pid = the whole process group (see `detached` above). + if (child.pid) process.kill(-child.pid, "SIGTERM"); + } catch { + // Already gone, or never started — nothing to clean up. + } + const hard = setTimeout(() => { + try { + if (child.pid) process.kill(-child.pid, "SIGKILL"); + } catch { + // Same. + } + }, 2_000); + hard.unref?.(); +} + +// ── Small helpers ───────────────────────────────────────────────────────────── + +function drain(attempt: Attempt): void { + const waiters = attempt.wake; + attempt.wake = []; + for (const w of waiters) w(); +} + +/** Resolve when `done()` holds; reject on timeout. Re-tested on every output chunk. */ +function waitFor(attempt: Attempt, timeoutMs: number, done: () => boolean): Promise { + if (done()) return Promise.resolve(); + return new Promise((resolve, reject) => { + const timer = setTimeout(() => reject(new Error("timeout")), timeoutMs); + timer.unref?.(); + const tick = () => { + if (!done()) { + attempt.wake.push(tick); + return; + } + clearTimeout(timer); + resolve(); + }; + attempt.wake.push(tick); + }); +} + +/** + * The longest match, not the first. + * + * The URL is printed twice: once inside an OSC-8 hyperlink (complete) and once + * as visible text (wrapped to the terminal width, so truncated at the first + * newline). Longest-wins picks the usable one without knowing which came first. + */ +function longestMatch(text: string, pattern: RegExp): string | null { + const re = new RegExp(pattern.source, pattern.flags.includes("g") ? pattern.flags : `${pattern.flags}g`); + let best: string | null = null; + for (const m of text.matchAll(re)) { + if (best === null || m[0].length > best.length) best = m[0]; + } + return best; +} + +/** Append to the transcript, trimming the front and accounting for what went. */ +function append(attempt: Attempt, chunk: string): void { + const grown = attempt.transcript + chunk; + if (grown.length <= TRANSCRIPT_MAX) { + attempt.transcript = grown; + return; + } + const cut = grown.length - TRANSCRIPT_MAX; + attempt.transcript = grown.slice(cut); + attempt.dropped += cut; +} + +/** The vendor's own last words, stripped of terminal noise, for an error we show. */ +function vendorReason(text: string): string { + const plain = stripAnsi(text); + const line = plain + .split("\n") + .map((l) => l.trim()) + .filter(Boolean) + .find((l) => /error/i.test(l)); + return line ?? "The provider rejected that code."; +} + +function lastLine(text: string): string { + const lines = stripAnsi(text) + .split("\n") + .map((l) => l.trim()) + .filter(Boolean); + return lines.at(-1) ?? ""; +} + +/** CSI/OSC sequences out. Display only — never used to decide anything. */ +function stripAnsi(text: string): string { + // oxlint-disable-next-line no-control-regex -- terminal output is the input + return text.replace(/\x1b\][^\x07\x1b]*(?:\x07|\x1b\\)/g, "").replace(/\x1b\[[0-9;?]*[a-zA-Z]/g, ""); +} diff --git a/src/daemon/session-manager.ts b/src/daemon/session-manager.ts index 69812a26..3ec3468e 100644 --- a/src/daemon/session-manager.ts +++ b/src/daemon/session-manager.ts @@ -22,6 +22,7 @@ import type { Store } from "./store.js"; import { createPushTransport, PushService } from "./push/index.js"; import { hasScope, SCOPES } from "../protocol/scopes.js"; import { applyPatches, getManifest, getSnapshot } from "./settings/store.js"; +import { BackendLoginBroker, BackendLoginError, redact } from "./auth/backend-login.js"; import { RateLimiter } from "./rate-limit.js"; import type { TranscriptMeta, TranscriptStore } from "./transcript.js"; import { @@ -325,6 +326,9 @@ export class SessionManager { readonly #blackboardTokens = new Map(); #mcpRegistry?: McpRegistry; #mcpHub?: McpHub; + /** In-flight interactive backend sign-ins. Daemon-wide, and at most one per + * backend — two live logins would race to write the same credential. */ + readonly #backendLogin = new BackendLoginBroker(); /** Live model catalogs by provider id (via each backend's supportedModels * equivalent), cached daemon-wide once any session of that provider * initializes. Empty until then. */ @@ -1024,6 +1028,12 @@ mcpHub: this.#mcpHub, return this.#settingsGet(msg, auth); case "settings.set": return this.#settingsSet(msg, auth); + case "backend.login.start": + return this.#backendLoginStart(msg, auth); + case "backend.login.submit": + return this.#backendLoginSubmit(msg, auth); + case "backend.login.cancel": + return this.#backendLoginCancel(msg, auth); case "usage.daily": return this.#usageDaily(msg, auth); case "pipeline.create": @@ -1581,6 +1591,107 @@ mcpHub: this.#mcpHub, }; } + // ── Interactive backend sign-in ───────────────────────────────────────────── + // + // Gated on `settings:write`, not a scope of its own. A completed login writes + // a credential to the same `.env` a caller with that scope can already write + // by hand — a separate scope would imply a privilege boundary that does not + // exist, and inventing one costs a token migration across every issuer. + + /** Start a backend's login command and return the URL the user must open. */ + async #backendLoginStart( + msg: Extract, + auth: AuthContext, + ): Promise { + if (!hasScope(auth.scopes as string[], SCOPES.SETTINGS_WRITE)) { + return this.#loginForbidden(msg.id); + } + try { + const login = await this.#backendLogin.start(msg.backend); + this.#store.audit(auth.sub, "backend.login.start", "", `backend=${msg.backend}`); + return { type: "backend.login.start.result", requestId: msg.id, login }; + } catch (err) { + return { + type: "response.error", + requestId: msg.id, + // The message is authored by the broker for display; redact anyway + // rather than depend on every future flow's error being clean. + error: redact(err instanceof Error ? err.message : String(err)), + code: err instanceof BackendLoginError ? "invalid_request" : "internal", + }; + } + } + + /** + * Hand the vendor's code to the waiting command, then store whatever + * credential it produced through the ordinary settings path — so the secret + * lands in the same 0600 `.env`, shows up as set in the same snapshot, and is + * cleared by the same control as one that was typed. + */ + async #backendLoginSubmit( + msg: Extract, + auth: AuthContext, + ): Promise { + if (!hasScope(auth.scopes as string[], SCOPES.SETTINGS_WRITE)) { + return this.#loginForbidden(msg.id); + } + try { + const outcome = await this.#backendLogin.submit(msg.loginId, msg.code); + let error = outcome.error; + if (outcome.ok && outcome.secret) { + const written = applyPatches([{ key: outcome.secret.key, value: outcome.secret.value }]); + if (!written.ok) { + // Signed in with the vendor but could not persist it: report the + // failure rather than a success the next session will not honour. + error = `Signed in, but the credential could not be saved: ${ + written.errors[0]?.message ?? "unknown error" + }`; + } + } + const ok = outcome.ok && error === undefined; + // Never the code, never the credential — only that an exchange happened. + this.#store.audit(auth.sub, "backend.login.submit", "", `ok=${ok}`); + return { + type: "backend.login.submit.result", + requestId: msg.id, + ok, + ...(error === undefined ? {} : { error }), + snapshot: { ...getSnapshot(), mcpServers: this.#mcpServerStatuses() }, + }; + } catch (err) { + return { + type: "response.error", + requestId: msg.id, + error: redact(err instanceof Error ? err.message : String(err)), + code: "internal", + }; + } + } + + /** Abandon an in-flight attempt. Idempotent — an unknown id is `ok: false`. */ + #backendLoginCancel( + msg: Extract, + auth: AuthContext, + ): DaemonMessage { + if (!hasScope(auth.scopes as string[], SCOPES.SETTINGS_WRITE)) { + return this.#loginForbidden(msg.id); + } + return { + type: "backend.login.cancel.result", + requestId: msg.id, + ok: this.#backendLogin.cancel(msg.loginId), + }; + } + + #loginForbidden(requestId: string): DaemonMessage { + return { + type: "response.error", + requestId, + error: "Missing scope: settings:write", + code: "forbidden", + }; + } + async #fsBrowseDir( msg: Extract, auth: AuthContext, @@ -1977,6 +2088,9 @@ mcpHub: this.#mcpHub, * idle or deadline. */ async drain(timeoutMs = 10_000): Promise { + // An abandoned login is a live pty holding an open OAuth attempt. Kill + // those first, and unconditionally — they are not work worth draining. + this.#backendLogin.dispose(); const deadline = Date.now() + timeoutMs; const systemAuth: AuthContext = { sub: "system:shutdown", diff --git a/src/daemon/settings/manifest.ts b/src/daemon/settings/manifest.ts index 93465094..ca503d42 100644 --- a/src/daemon/settings/manifest.ts +++ b/src/daemon/settings/manifest.ts @@ -322,12 +322,22 @@ const claude: SettingsTab = { id: "claude", title: "Claude", icon: "✳", - description: "The default in-process backend (Anthropic SDK). Always enabled. Credentials come from your Claude Code login or ANTHROPIC_API_KEY.", + description: "The default in-process backend (Anthropic SDK). Always enabled. Sign in with your Claude subscription, or set an API key.", groups: [ { id: "claude-auth", title: "Authentication", + description: "Sign in above to use a Claude subscription — no key to paste. A key is only needed instead of signing in, or for cluster labeling.", fields: [ + // Written by "Sign in with Claude", not typed. It is still a manifest + // field (and so still hand-editable, clearable, and visible as set) + // because a credential the UI can create but not show or revoke is a + // credential nobody can reason about. `next-session`, not `restart`: + // the store updates process.env live and each Claude turn builds its + // subprocess env fresh, so the next session picks it up. + secret("CLAUDE_CODE_OAUTH_TOKEN", "Claude subscription token", "Set by “Sign in with Claude”. A long-lived token from your Claude subscription, used instead of an API key.", { + applies: "next-session", + }), secret("ANTHROPIC_API_KEY", "Anthropic API key", "Used by the Claude backend when not signed in, and for cluster labeling."), env("CLAUDE_CODE_USE_BEDROCK", "Use Amazon Bedrock", "Route Claude through Amazon Bedrock (requires AWS credentials in the environment).", { kind: "boolean", diff --git a/src/tests/backend-login.test.ts b/src/tests/backend-login.test.ts new file mode 100644 index 00000000..4bf697b0 --- /dev/null +++ b/src/tests/backend-login.test.ts @@ -0,0 +1,301 @@ +/** + * Backend login broker. + * + * These run a FAKE vendor command through the REAL pty, the real matchers, and + * the real lifecycle. That combination is deliberate: the parts most likely to + * be wrong here are not the state machine but the seam with a terminal — + * whether `script(1)` is invoked correctly, whether the URL survives OSC-8 + * hyperlinks and an 80-column wrap, whether writing a line to stdin actually + * reaches a program blocked on `read`. Mocking the process would test the half + * that was never in doubt. + * + * The fake's output is modelled on a transcript captured from the real + * `claude setup-token`: same hyperlink wrapper around the URL, the same + * line-wrapped duplicate beside it, the same `OAuth error:` on rejection, and + * the same habit of staying alive afterwards to offer a retry. + */ + +import { afterEach, beforeAll, describe, expect, test } from "bun:test"; +import { chmodSync, mkdtempSync, readFileSync, writeFileSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { + BackendLoginBroker, + BackendLoginError, + CLAUDE_LOGIN_FLOW, + type LoginFlow, + redact, +} from "../daemon/auth/backend-login.js"; +import type { LoginBackend } from "../protocol/types.js"; + +const URL_ = "https://claude.com/cai/oauth/authorize?code=true&client_id=abc&state=xyz"; +const GOOD_CODE = "good-code#xyz"; +const TOKEN = "sk-ant-oat01-AAAABBBBCCCCDDDDEEEEFFFFGGGG"; + +let dir: string; +let fakeCmd: string; +let pidFile: string; + +beforeAll(() => { + dir = mkdtempSync(join(tmpdir(), "codeoid-login-")); + fakeCmd = join(dir, "fake-login.sh"); + pidFile = join(dir, "pid"); + // Note the two copies of the URL: the OSC-8 hyperlink carries it whole, the + // visible text is wrapped at 78 columns like a real terminal would. Only the + // longest-match rule recovers a usable URL from that. + const head = URL_.slice(0, 78); + const tail = URL_.slice(78); + writeFileSync( + fakeCmd, + `#!/usr/bin/env bash +echo $$ > "${pidFile}" +echo "Welcome to Fake CLI" +printf 'Opening browser to sign in\\xe2\\x80\\xa6\\n' +printf '\\033]8;id=abc;%s\\033\\\\%s\\033]8;;\\033\\\\\\n' "${URL_}" "${head}" +printf '%s\\n' "${tail}" +echo "Paste code here if prompted >" +read -r code +if [ "$code" = "${GOOD_CODE}" ]; then + echo "Success! Your token: ${TOKEN}" + exit 0 +fi +# Mirrors the real command: reports the error and stays alive offering a retry, +# so the broker must not wait for an exit to call this a failure. +echo "OAuth error: Request failed with status code 400" +echo "Press Enter to retry." +sleep 120 +`, + "utf8", + ); + chmodSync(fakeCmd, 0o755); +}); + +/** The real Claude matchers against a fake command — the matching is the point. */ +function fakeFlow(overrides: Partial = {}): Record { + return { + claude: { + ...CLAUDE_LOGIN_FLOW, + resolve: () => ({ file: fakeCmd, args: [] }), + ...overrides, + }, + }; +} + +const brokers: BackendLoginBroker[] = []; +function newBroker(flows = fakeFlow()): BackendLoginBroker { + const b = new BackendLoginBroker(flows); + brokers.push(b); + return b; +} + +afterEach(() => { + for (const b of brokers.splice(0)) b.dispose(); +}); + +describe("start", () => { + test("recovers the whole URL from hyperlinked, line-wrapped terminal output", async () => { + const login = await newBroker().start("claude"); + // The visible copy is truncated at the wrap; only the hyperlink's is whole. + // Asserting equality (not `contains`) is what pins longest-match. + expect(login.verificationUrl).toBe(URL_); + expect(login.backend).toBe("claude"); + expect(login.loginId.length).toBeGreaterThan(0); + expect(login.expiresAt).toBeGreaterThan(Date.now()); + }); + + test("an unavailable command is a clean refusal, not a spawn failure", async () => { + const broker = newBroker(fakeFlow({ resolve: () => null })); + const err = await broker.start("claude").catch((e) => e); + expect(err).toBeInstanceOf(BackendLoginError); + expect(String(err.message)).toContain("not installed"); + }); + + test("an unknown backend is refused", async () => { + const err = await newBroker() + .start("nope" as LoginBackend) + .catch((e) => e); + expect(err).toBeInstanceOf(BackendLoginError); + }); + + test("a second start supersedes the first, so a reloaded page is not locked out", async () => { + const broker = newBroker(); + const first = await broker.start("claude"); + const second = await broker.start("claude"); + expect(second.loginId).not.toBe(first.loginId); + // The superseded attempt is gone: its id no longer submits or cancels. + expect(broker.cancel(first.loginId)).toBe(false); + const stale = await broker.submit(first.loginId, GOOD_CODE); + expect(stale.ok).toBe(false); + expect(stale.error).toContain("no longer open"); + }); +}); + +describe("submit", () => { + test("the right code yields the credential the command printed", async () => { + const broker = newBroker(); + const login = await broker.start("claude"); + const out = await broker.submit(login.loginId, GOOD_CODE); + expect(out.ok).toBe(true); + expect(out.secret).toEqual({ key: "CLAUDE_CODE_OAUTH_TOKEN", value: TOKEN }); + expect(out.error).toBeUndefined(); + }); + + test("a rejected code fails without waiting for the command to exit", async () => { + const broker = newBroker(); + const login = await broker.start("claude"); + // The fake sleeps 120s after the error. Anything near that means the broker + // is waiting on exit rather than on the vendor's own rejection. + const started = Date.now(); + const out = await broker.submit(login.loginId, "wrong-code"); + expect(out.ok).toBe(false); + expect(Date.now() - started).toBeLessThan(20_000); + expect(out.secret).toBeUndefined(); + expect(out.error).toContain("OAuth error"); + expect(out.error).toContain("Start the sign-in again"); + }); + + test("a credential-shaped code is not mistaken for the vendor's answer", async () => { + // A pty echoes its input, so a token-shaped paste lands in the transcript + // looking exactly like output. Reading it back would report a successful + // sign-in and store the user's own typo as the credential. + const broker = newBroker(); + const login = await broker.start("claude"); + const looksLikeAToken = "sk-ant-oat01-LEAKYLEAKYLEAKYLEAKYLEAKY"; + const out = await broker.submit(login.loginId, looksLikeAToken); + expect(out.ok).toBe(false); + expect(out.secret).toBeUndefined(); + // …and the echo must not come back out in the error either. + expect(out.error ?? "").not.toContain("LEAKY"); + }); + + test("the attempt is terminal — the same id cannot be submitted twice", async () => { + const broker = newBroker(); + const login = await broker.start("claude"); + expect((await broker.submit(login.loginId, GOOD_CODE)).ok).toBe(true); + const again = await broker.submit(login.loginId, GOOD_CODE); + expect(again.ok).toBe(false); + expect(again.error).toContain("no longer open"); + }); + + test("an unknown login id is refused", async () => { + const out = await newBroker().submit("not-a-login", GOOD_CODE); + expect(out.ok).toBe(false); + }); + + test.each([ + ["empty", ""], + ["whitespace", " "], + ["embedded newline", "abc\ndef"], + ["carriage return", "abc\rdef"], + ["null byte", "abc\u0000def"], + ["oversized", "x".repeat(2000)], + ])("a code that cannot be one is refused before it reaches the pty: %s", async (_name, code) => { + const broker = newBroker(); + const login = await broker.start("claude"); + const out = await broker.submit(login.loginId, code); + expect(out.ok).toBe(false); + expect(out.error).toContain("does not look like"); + // Refused, not consumed: the attempt is still live and still usable. + const good = await broker.submit(login.loginId, GOOD_CODE); + expect(good.ok).toBe(true); + }); + + test("surrounding whitespace is tolerated — a pasted code often carries it", async () => { + const broker = newBroker(); + const login = await broker.start("claude"); + const out = await broker.submit(login.loginId, ` ${GOOD_CODE} `); + expect(out.ok).toBe(true); + }); + + test("a command that prints no credential succeeds on its exit status", async () => { + // Some vendors write the credential to disk and print nothing worth + // keeping. That is success with nothing for codeoid to store, not failure. + const broker = newBroker(fakeFlow({ extractSecret: () => null })); + const login = await broker.start("claude"); + const out = await broker.submit(login.loginId, GOOD_CODE); + expect(out.ok).toBe(true); + expect(out.secret).toBeUndefined(); + }); +}); + +describe("cancel", () => { + test("kills the command, so an abandoned attempt is not a live pty", async () => { + const broker = newBroker(); + const login = await broker.start("claude"); + const pid = Number(readFileSync(pidFile, "utf8").trim()); + expect(alive(pid)).toBe(true); + + expect(broker.cancel(login.loginId)).toBe(true); + await waitUntil(() => !alive(pid), 5_000); + expect(alive(pid)).toBe(false); + expect(broker.cancel(login.loginId)).toBe(false); // idempotent + }); + + test("dispose kills every attempt", async () => { + const broker = newBroker(); + const login = await broker.start("claude"); + const pid = Number(readFileSync(pidFile, "utf8").trim()); + broker.dispose(); + await waitUntil(() => !alive(pid), 5_000); + expect(alive(pid)).toBe(false); + expect((await broker.submit(login.loginId, GOOD_CODE)).ok).toBe(false); + }); +}); + +describe("redact", () => { + test.each([ + ["sk-ant-oat01-AAAABBBBCCCCDDDDEEEEFFFF"], + ["sk-ant-api03-ZZZZYYYYXXXXWWWWVVVVUUUU"], + ["sk-proj-0123456789abcdefghijklmnop"], + ])("removes %s", (credential) => { + const out = redact(`failed with ${credential} in the message`); + expect(out).not.toContain(credential); + expect(out).toContain("«redacted»"); + }); + + test("leaves ordinary text alone", () => { + expect(redact("OAuth error: status code 400")).toBe("OAuth error: status code 400"); + }); +}); + +describe("the Claude flow's matchers", () => { + test("extracts the token shape setup-token prints", () => { + expect(CLAUDE_LOGIN_FLOW.extractSecret(`Your token: ${TOKEN}\n`)).toEqual({ + key: "CLAUDE_CODE_OAUTH_TOKEN", + value: TOKEN, + }); + }); + + test("does not mistake an API key for a subscription token", () => { + // Only oat01 is the subscription credential; storing an api03 key under + // CLAUDE_CODE_OAUTH_TOKEN would authenticate as the wrong thing. + expect(CLAUDE_LOGIN_FLOW.extractSecret("sk-ant-api03-AAAABBBBCCCCDDDDEEEEFFFF")).toBeNull(); + }); + + test("the URL matcher stops at terminal control bytes", () => { + const esc = "\u001b"; + const bel = "\u0007"; + const raw = `${esc}]8;id=z;${URL_}${bel}${URL_.slice(0, 20)}${esc}]8;;${bel}`; + const matches = [...raw.matchAll(CLAUDE_LOGIN_FLOW.urlPattern)].map((m) => m[0]); + expect(matches).toContain(URL_); + // No match may carry an escape into itself. + for (const m of matches) expect(m).not.toContain(esc); + }); +}); + +function alive(pid: number): boolean { + try { + process.kill(pid, 0); + return true; + } catch { + return false; + } +} + +async function waitUntil(cond: () => boolean, timeoutMs: number): Promise { + const deadline = Date.now() + timeoutMs; + while (Date.now() < deadline) { + if (cond()) return; + await new Promise((r) => setTimeout(r, 50)); + } +} diff --git a/src/tests/protocol.test.ts b/src/tests/protocol.test.ts index a3e532e1..7b95e995 100644 --- a/src/tests/protocol.test.ts +++ b/src/tests/protocol.test.ts @@ -450,6 +450,12 @@ describe("DaemonMessage routing", () => { return `settings.get:${Object.keys(msg.snapshot.values).length}`; case "settings.set.result": return `settings.set:${msg.ok}`; + case "backend.login.start.result": + return `login.start:${msg.login.backend}`; + case "backend.login.submit.result": + return `login.submit:${msg.ok}`; + case "backend.login.cancel.result": + return `login.cancel:${msg.ok}`; case "pipeline.snapshot": return `pipeline:${msg.pipeline.id}`; case "pipeline.list.result": diff --git a/src/tests/settings.test.ts b/src/tests/settings.test.ts index e855f6dc..4d2fd582 100644 --- a/src/tests/settings.test.ts +++ b/src/tests/settings.test.ts @@ -15,7 +15,7 @@ import { SessionManager } from "../daemon/session-manager.js"; import { Store } from "../daemon/store.js"; import { TranscriptStore } from "../daemon/transcript.js"; import { SCOPES } from "../protocol/scopes.js"; -import type { McpServerStatus, SettingKind } from "../protocol/types.js"; +import type { ClientMessage, McpServerStatus, SettingKind } from "../protocol/types.js"; import { McpRegistry } from "../daemon/mcp/registry.js"; import { McpHub } from "../daemon/mcp/hub.js"; import type { RawMcpServerConfig } from "../config.js"; @@ -317,6 +317,44 @@ describe("settings RPC handlers", () => { expect(res.ok).toBe(true); expect(res.snapshot.values["memory.enabled"]?.value).toBe(false); }); + + // ── Interactive backend sign-in ───────────────────────────────────────────── + // Gated on settings:write, because a completed login writes a credential to + // the same `.env` that scope already governs. These pin the gate; the flow + // itself is covered in backend-login.test.ts against a real pty. + + it.each([ + ["backend.login.start", { type: "backend.login.start", backend: "claude" }], + ["backend.login.submit", { type: "backend.login.submit", loginId: "L", code: "c" }], + ["backend.login.cancel", { type: "backend.login.cancel", loginId: "L" }], + ] as const)("%s requires settings:write (read alone is not enough)", async (_name, msg) => { + const denied = await mgr().handle( + { ...msg, id: "1" } as ClientMessage, + auth([SCOPES.SETTINGS_READ]), + client, + ); + expect(denied).toMatchObject({ type: "response.error", code: "forbidden" }); + }); + + it("cancelling an unknown login is a clean no-op, not an error", async () => { + const res = await mgr().handle( + { type: "backend.login.cancel", id: "1", loginId: "never-existed" }, + auth([SCOPES.SETTINGS_WRITE]), + client, + ); + expect(res).toMatchObject({ type: "backend.login.cancel.result", ok: false }); + }); + + it("submitting against an unknown login reports it plainly, without writing", async () => { + const res = (await mgr().handle( + { type: "backend.login.submit", id: "1", loginId: "never-existed", code: "abc" }, + auth([SCOPES.SETTINGS_WRITE]), + client, + )) as { type: string; ok: boolean; error?: string }; + expect(res.type).toBe("backend.login.submit.result"); + expect(res.ok).toBe(false); + expect(res.error).toContain("no longer open"); + }); }); // ── Local helpers for the drift guard ────────────────────────────────────────── diff --git a/web/src/components/BackendLoginPanel.tsx b/web/src/components/BackendLoginPanel.tsx new file mode 100644 index 00000000..876c70e3 --- /dev/null +++ b/web/src/components/BackendLoginPanel.tsx @@ -0,0 +1,178 @@ +/** + * "Sign in with " — the settings-drawer face of the interactive login. + * + * Rendered above a backend tab's fields, because signing in is the thing most + * people want and pasting a key is the fallback. The panel walks the two steps + * the flow actually has, and never shows more than one of them at a time: + * + * idle → a button, plus whatever the last attempt failed with + * awaiting_code → the vendor URL to open, and a box for the code it hands back + * done → confirmation, with the honest caveat about when it applies + * + * The URL opens in a new tab AND is shown as copyable text: this daemon is + * often not on the machine holding the browser (a sandbox, a phone), and in + * that case the link is something the user carries across rather than clicks. + */ + +import { type Component, Show, createSignal } from "solid-js"; + +import { + backendLoginState, + cancelBackendLogin, + dismissBackendLogin, + startBackendLogin, + submitBackendLoginCode, +} from "../state/backend-login"; +import { settingsState } from "../state/settings"; +import type { LoginBackend } from "../protocol/types"; + +/** Which secret a completed sign-in fills in, per backend. */ +const CREDENTIAL_KEY: Record = { + claude: "CLAUDE_CODE_OAUTH_TOKEN", +}; + +const LABEL: Record = { + claude: "Claude", +}; + +export const BackendLoginPanel: Component<{ backend: LoginBackend }> = (props) => { + const [code, setCode] = createSignal(""); + const st = () => backendLoginState(); + const mine = () => st().backend === props.backend; + const phase = () => (mine() ? st().phase : "idle"); + const signedIn = () => + settingsState().snapshot?.secrets[CREDENTIAL_KEY[props.backend]]?.set === true; + + const submit = async () => { + const value = code().trim(); + if (value.length === 0) return; + const ok = await submitBackendLoginCode(value); + // Clear either way: on success it is spent, on failure the attempt is over + // and a stale code in the box only invites resubmitting it. + setCode(""); + if (!ok) return; + }; + + return ( +
+
+
+

Sign in with {LABEL[props.backend]}

+

+ Use your {LABEL[props.backend]} subscription instead of an API key. codeoid runs the + official sign-in for you — you approve it in your own browser. +

+
+ + + signed in + + +
+ + +
+ + + {st().error} + +
+ + + +

+ Starting the {LABEL[props.backend]} sign-in… this can take a few seconds. +

+ + + + {(login) => ( +
+
+
1 · Open this link and approve
+
+ + {login.verificationUrl} + + +
+
+ +
+
2 · Paste the code back here
+

{login.codeHint}

+
+ setCode(e.currentTarget.value)} + onKeyDown={(e) => { + if (e.key === "Enter") void submit(); + }} + /> + + +
+
+
+ )} + + + +
+ Signed in. + {/* Honest about when it bites: existing sessions keep the environment + they were started with. */} + New sessions will use it. + +
+ +
+ ); +}; diff --git a/web/src/components/SettingsDrawer.tsx b/web/src/components/SettingsDrawer.tsx index a5c9af79..d5270fcd 100644 --- a/web/src/components/SettingsDrawer.tsx +++ b/web/src/components/SettingsDrawer.tsx @@ -26,7 +26,10 @@ import { createStore, produce } from "solid-js/store"; import { fetchSettings, saveSettings, settingsState } from "../state/settings"; import { relativeTime } from "../lib/format"; +import { BackendLoginPanel } from "./BackendLoginPanel"; +import { LOGIN_CAPABLE_BACKENDS } from "../protocol/types"; import type { + LoginBackend, McpServerStatus, SecretStatus, SettingField, @@ -35,6 +38,15 @@ import type { SettingValue, } from "../protocol/types"; +/** + * A backend tab whose id names a backend codeoid can sign in interactively. + * Returns the narrowed id (so the panel is typed) or `null` — every other tab + * renders exactly as before. + */ +function isLoginBackend(tabId: string): LoginBackend | null { + return LOGIN_CAPABLE_BACKENDS.includes(tabId as LoginBackend) ? (tabId as LoginBackend) : null; +} + const [openSignal, setOpenSignal] = createSignal(false); const [activeTab, setActiveTab] = createSignal(""); const [showAdvanced, setShowAdvanced] = createSignal(false); @@ -204,6 +216,13 @@ const SettingsDrawer: Component = () => { + {/* Signing in is the primary path for a backend that has + one, so it sits above the fields — a pasted key is the + fallback, not the default. */} + + {(backend) => } + + {(g) => { const visible = () => diff --git a/web/src/state/backend-login.test.ts b/web/src/state/backend-login.test.ts new file mode 100644 index 00000000..05c0ca42 --- /dev/null +++ b/web/src/state/backend-login.test.ts @@ -0,0 +1,133 @@ +// @vitest-environment jsdom +import { describe, it, expect, afterEach, vi } from "vitest"; + +const clientRequestMock = vi.hoisted(() => vi.fn()); +vi.mock("./connection", () => ({ + newRequestId: () => "r", + getClient: () => ({ request: clientRequestMock }), +})); + +import { + backendLoginState, + cancelBackendLogin, + dismissBackendLogin, + startBackendLogin, + submitBackendLoginCode, + _resetBackendLoginForTest, +} from "./backend-login"; +import { settingsState, _resetSettingsForTest } from "./settings"; + +afterEach(() => { + _resetBackendLoginForTest(); + _resetSettingsForTest(); + clientRequestMock.mockReset(); +}); + +const LOGIN = { + loginId: "L1", + backend: "claude" as const, + verificationUrl: "https://claude.com/cai/oauth/authorize?code=true", + expiresAt: Date.now() + 600_000, + codeHint: "Paste the code the page shows.", +}; + +const SNAPSHOT = { + values: {}, + secrets: { CLAUDE_CODE_OAUTH_TOKEN: { set: true, source: "env-file" as const } }, + configPath: "/home/u/.codeoid/config.json", + envPath: "/home/u/.codeoid/.env", +}; + +function startResult() { + return { type: "backend.login.start.result", requestId: "r", login: LOGIN }; +} + +describe("backend login", () => { + it("holds the URL to show once the daemon has one", async () => { + clientRequestMock.mockResolvedValueOnce(startResult()); + await startBackendLogin("claude"); + expect(backendLoginState().phase).toBe("awaiting_code"); + expect(backendLoginState().login?.verificationUrl).toBe(LOGIN.verificationUrl); + expect(backendLoginState().error).toBeNull(); + }); + + it("a start that fails leaves the panel usable, with the reason", async () => { + clientRequestMock.mockRejectedValueOnce(new Error("claude is not installed")); + await startBackendLogin("claude"); + // Back to idle, not stuck in `starting` — the button must be pressable again. + expect(backendLoginState().phase).toBe("idle"); + expect(backendLoginState().error).toContain("not installed"); + }); + + it("a successful submit adopts the snapshot, so the credential shows as set", async () => { + clientRequestMock + .mockResolvedValueOnce(startResult()) + .mockResolvedValueOnce({ + type: "backend.login.submit.result", + requestId: "r", + ok: true, + snapshot: SNAPSHOT, + }); + + await startBackendLogin("claude"); + expect(await submitBackendLoginCode("the-code")).toBe(true); + + expect(backendLoginState().phase).toBe("done"); + expect(backendLoginState().login).toBeNull(); + // No second round trip: the snapshot rode back on the submit result. + expect(clientRequestMock).toHaveBeenCalledTimes(2); + expect(settingsState().snapshot?.secrets.CLAUDE_CODE_OAUTH_TOKEN?.set).toBe(true); + }); + + it("a rejected code ends the attempt and surfaces the daemon's reason", async () => { + clientRequestMock.mockResolvedValueOnce(startResult()).mockResolvedValueOnce({ + type: "backend.login.submit.result", + requestId: "r", + ok: false, + error: "OAuth error: status code 400. Start the sign-in again.", + snapshot: SNAPSHOT, + }); + + await startBackendLogin("claude"); + expect(await submitBackendLoginCode("wrong")).toBe(false); + + expect(backendLoginState().phase).toBe("idle"); + // The attempt is spent — the URL must go, or the user retries into a dead one. + expect(backendLoginState().login).toBeNull(); + expect(backendLoginState().error).toContain("OAuth error"); + }); + + it("submitting with nothing in flight is a no-op, not a request", async () => { + expect(await submitBackendLoginCode("code")).toBe(false); + expect(clientRequestMock).not.toHaveBeenCalled(); + }); + + it("cancel clears locally even when the daemon never answers", async () => { + clientRequestMock + .mockResolvedValueOnce(startResult()) + .mockRejectedValueOnce(new Error("socket closed")); + + await startBackendLogin("claude"); + await cancelBackendLogin(); + // A user who pressed cancel must not be left staring at a dead URL because + // the cancel message did not land; the daemon's TTL covers its side. + expect(backendLoginState().phase).toBe("idle"); + expect(backendLoginState().login).toBeNull(); + }); + + it("dismiss clears the panel without talking to the daemon", async () => { + clientRequestMock.mockResolvedValueOnce(startResult()).mockResolvedValueOnce({ + type: "backend.login.submit.result", + requestId: "r", + ok: true, + snapshot: SNAPSHOT, + }); + await startBackendLogin("claude"); + await submitBackendLoginCode("the-code"); + clientRequestMock.mockClear(); + + dismissBackendLogin(); + expect(backendLoginState().phase).toBe("idle"); + expect(clientRequestMock).not.toHaveBeenCalled(); + }); +}); diff --git a/web/src/state/backend-login.ts b/web/src/state/backend-login.ts new file mode 100644 index 00000000..80c4262f --- /dev/null +++ b/web/src/state/backend-login.ts @@ -0,0 +1,151 @@ +/** + * Interactive backend sign-in — client half. + * + * A two-step, out-of-band flow: ask the daemon to start the backend's own login + * command, show the user the URL it returns, take the code the vendor's page + * gives them, hand it back. At most one attempt is tracked at a time, matching + * the daemon, which holds at most one per backend. + * + * The code is passed straight through and never stored here — not in the state + * signal, not in a closure that outlives the call. What comes back is a + * settings snapshot, which reports the resulting credential as *set* and never + * carries its value. + */ + +import { createSignal } from "solid-js"; + +import { getClient, newRequestId } from "./connection"; +import { applySnapshot } from "./settings"; +import type { + BackendLoginCancelResultMsg, + BackendLoginStartResultMsg, + BackendLoginSubmitResultMsg, + LoginBackend, + PendingBackendLogin, +} from "../protocol/types"; + +/** + * `starting` and `submitting` are both slow (the daemon is waiting on a vendor + * command, not on us), which is exactly why they are distinct states rather + * than one `busy` flag — the two waits need different words on screen. + */ +export type LoginPhase = "idle" | "starting" | "awaiting_code" | "submitting" | "done"; + +interface State { + backend: LoginBackend | null; + phase: LoginPhase; + login: PendingBackendLogin | null; + error: string | null; +} + +const EMPTY: State = { backend: null, phase: "idle", login: null, error: null }; + +const [state, setState] = createSignal(EMPTY); + +export const backendLoginState = state; + +/** Test-only: reset the module singleton between cases. */ +export function _resetBackendLoginForTest(): void { + setState(EMPTY); +} + +/** Drop the panel back to its resting state without touching the daemon. */ +export function dismissBackendLogin(): void { + setState(EMPTY); +} + +/** + * Ask the daemon to start the backend's login command. + * + * Resolves only once there is a URL to show, so the panel never renders an + * "in progress" step the user cannot act on. Generous timeout: the daemon is + * waiting on a vendor binary's first output, not on a round trip. + */ +export async function startBackendLogin(backend: LoginBackend): Promise { + setState({ backend, phase: "starting", login: null, error: null }); + try { + const id = newRequestId(); + const res = await getClient().request( + { type: "backend.login.start", id, backend }, + { + waitForResult: (m) => + m.type === "backend.login.start.result" && m.requestId === id ? m : undefined, + timeoutMs: 60_000, + }, + ); + setState({ backend, phase: "awaiting_code", login: res.login, error: null }); + } catch (err) { + setState({ backend, phase: "idle", login: null, error: errText(err) }); + } +} + +/** + * Submit the vendor's code and finish the exchange. + * + * Terminal either way, matching the daemon: a rejected code ends the attempt + * and the user starts again, rather than retrying into a command whose state + * neither side can still describe. + */ +export async function submitBackendLoginCode(code: string): Promise { + const cur = state(); + if (!cur.login) return false; + const loginId = cur.login.loginId; + setState((s) => ({ ...s, phase: "submitting", error: null })); + try { + const id = newRequestId(); + const res = await getClient().request( + { type: "backend.login.submit", id, loginId, code }, + { + waitForResult: (m) => + m.type === "backend.login.submit.result" && m.requestId === id ? m : undefined, + timeoutMs: 120_000, + }, + ); + // The snapshot rides back on the result, so the drawer shows the new + // credential as set without a second round trip. + applySnapshot(res.snapshot); + if (res.ok) { + setState((s) => ({ ...s, phase: "done", login: null, error: null })); + return true; + } + setState((s) => ({ + ...s, + phase: "idle", + login: null, + error: res.error ?? "Sign-in failed. Start again.", + })); + return false; + } catch (err) { + setState((s) => ({ ...s, phase: "idle", login: null, error: errText(err) })); + return false; + } +} + +/** + * Abandon the attempt. Best-effort by design: the local state clears whatever + * the daemon says, because a user who pressed cancel should not be stuck + * looking at a dead URL if the message failed to land. The daemon expires the + * attempt on its own timer regardless. + */ +export async function cancelBackendLogin(): Promise { + const cur = state(); + setState(EMPTY); + if (!cur.login) return; + try { + const id = newRequestId(); + await getClient().request( + { type: "backend.login.cancel", id, loginId: cur.login.loginId }, + { + waitForResult: (m) => + m.type === "backend.login.cancel.result" && m.requestId === id ? m : undefined, + timeoutMs: 8_000, + }, + ); + } catch { + // Already gone, or the socket is down — the daemon's TTL covers both. + } +} + +function errText(err: unknown): string { + return err instanceof Error ? err.message : String(err); +} diff --git a/web/src/state/settings.ts b/web/src/state/settings.ts index 1e24f447..2ae4e8f4 100644 --- a/web/src/state/settings.ts +++ b/web/src/state/settings.ts @@ -109,6 +109,17 @@ export async function fetchSettings(force = false): Promise { } } +/** + * Adopt a snapshot that arrived on someone else's result message. + * + * A completed backend sign-in writes a secret, and its result already carries + * the settings AFTER that write — so the drawer shows the new credential as set + * without a redundant `settings.get`. Values only; the manifest is untouched. + */ +export function applySnapshot(snapshot: SettingsSnapshot): void { + setState((s) => ({ ...s, snapshot, fetchedAt: Date.now() })); +} + /** * Persist a batch of changes. Returns the result so the caller can clear its * dirty state on success / surface per-field errors on failure. From 08b2b2ab12fee8c390aaca394b18f74a0a0780b4 Mon Sep 17 00:00:00 2001 From: Yash Datta Date: Wed, 9 Sep 2026 16:11:25 +0200 Subject: [PATCH 2/2] fix: three defects found reviewing the backend-login broker MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A second verification pass over #331. Runtime side was clean — the pty tests pass five runs out of five, and the EPIPE crash I expected from writing to a dead child's stdin does not reproduce under bun (probed directly: no throw, no uncaught error). Reading the diff found three things worth fixing. 1. Cancelling mid-exchange blamed the command. `#dispose` wakes every waiter by marking the attempt exited and THEN drops the transcript, so an in-flight `submit` racing a `cancel` resumed against an empty window and reported "the sign-in command failed" for what the user had just done on purpose. It now notices the attempt is no longer its own and says it was cancelled. The same race exists for the 10-minute TTL, and the same check covers it. 2. The verification URL is now required to be https, in the broker. It becomes an `href` in every client, and today's patterns are anchored on `https://claude.com/…` so they cannot produce anything else — the point is that a future flow with a looser regex cannot turn scraped terminal output into a `javascript:` link. Enforced in one place rather than trusted to each flow's pattern. 3. `buildAgentEnv` now has a test for CLAUDE_CODE_OAUTH_TOKEN. It is the last link in the chain — sign-in writes the token, the settings store puts it in process.env live, and that allowlist decides whether the agent subprocess ever sees it. Tighten the prefixes and the sign-in keeps reporting success while authenticating as nothing, which is a failure with no error to follow. Also verified, not changed: bun.lock pins all eight `claude-agent-sdk-` packages including both musl variants, so the binary the broker resolves is present for every arch the workspace image builds for. 2494 pass, 0 fail. Typecheck and lint clean. --- src/daemon/auth/backend-login.ts | 17 ++++++++++++- src/tests/backend-login.test.ts | 40 +++++++++++++++++++++++++++++++ src/tests/provider-claude.test.ts | 14 +++++++++++ 3 files changed, 70 insertions(+), 1 deletion(-) diff --git a/src/daemon/auth/backend-login.ts b/src/daemon/auth/backend-login.ts index bfff30fd..7e0dc5ae 100644 --- a/src/daemon/auth/backend-login.ts +++ b/src/daemon/auth/backend-login.ts @@ -247,7 +247,13 @@ export class BackendLoginBroker { const absorb = (buf: Buffer | string) => { append(attempt, String(buf)); if (!attempt.verificationUrl) { - attempt.verificationUrl = longestMatch(attempt.transcript, flow.urlPattern); + const found = longestMatch(attempt.transcript, flow.urlPattern); + // Enforced here, not left to each flow's regex, because this string + // becomes an `href` in every client. Today's patterns are anchored on + // `https://claude.com/…` and cannot produce anything else; the point is + // that a future flow with a looser pattern cannot turn scraped terminal + // output into a `javascript:` link. One place, all flows, all clients. + attempt.verificationUrl = found?.startsWith("https://") ? found : null; } drain(attempt); }; @@ -354,6 +360,15 @@ export class BackendLoginBroker { return { ok: false, error: "The sign-in did not complete in time. Start again." }; } + // Cancelled (or expired) out from under us while we waited. `#dispose` + // wakes every waiter by marking the attempt exited and then drops the + // transcript, so falling through would judge the exchange against an empty + // window and report "the sign-in command failed" for what was actually a + // user pressing Cancel. Say what happened instead. + if (this.#byId.get(loginId) !== attempt) { + return { ok: false, error: "That sign-in attempt was cancelled." }; + } + const since = sinceMark(); const secret = mintedSecret() ?? undefined; const rejected = attempt.flow.failurePattern.test(since); diff --git a/src/tests/backend-login.test.ts b/src/tests/backend-login.test.ts index 4bf697b0..623d10c1 100644 --- a/src/tests/backend-login.test.ts +++ b/src/tests/backend-login.test.ts @@ -34,6 +34,8 @@ const TOKEN = "sk-ant-oat01-AAAABBBBCCCCDDDDEEEEFFFFGGGG"; let dir: string; let fakeCmd: string; +/** Prints a non-https "URL" and exits — for the href guard. */ +let evilCmd: string; let pidFile: string; beforeAll(() => { @@ -68,6 +70,10 @@ sleep 120 "utf8", ); chmodSync(fakeCmd, 0o755); + + evilCmd = join(dir, "evil-login.sh"); + writeFileSync(evilCmd, "#!/usr/bin/env bash\necho 'open javascript:alert(1) to sign in'\n", "utf8"); + chmodSync(evilCmd, 0o755); }); /** The real Claude matchers against a fake command — the matching is the point. */ @@ -110,6 +116,26 @@ describe("start", () => { expect(String(err.message)).toContain("not installed"); }); + test("a non-https match is not offered as a link", async () => { + // The URL becomes an `href` in every client, so the broker refuses anything + // that is not https rather than trusting each flow's regex to stay tight. + const pattern = /javascript:[A-Za-z0-9_.()]+/g; + // The pattern DOES match what the command prints — so the only reason this + // is not handed to a client is the https rule, not a failed match. + expect("javascript:alert(1)".match(pattern)).toBeTruthy(); + + const broker = newBroker({ + claude: { + ...CLAUDE_LOGIN_FLOW, + resolve: () => ({ file: evilCmd, args: [] }), + urlPattern: pattern, + }, + }); + const err = await broker.start("claude").catch((e) => e); + expect(err).toBeInstanceOf(BackendLoginError); + expect(String(err.message)).toContain("sign-in link"); + }); + test("an unknown backend is refused", async () => { const err = await newBroker() .start("nope" as LoginBackend) @@ -182,6 +208,20 @@ describe("submit", () => { expect(out.ok).toBe(false); }); + test("cancelling mid-exchange says so, rather than blaming the command", async () => { + // `#dispose` wakes waiters by marking the attempt exited and then drops the + // transcript, so a submit racing a cancel would otherwise judge the + // exchange against an empty window and report a command failure for what + // the user just did on purpose. + const broker = newBroker(); + const login = await broker.start("claude"); + const pending = broker.submit(login.loginId, "wrong-code"); + broker.cancel(login.loginId); + const out = await pending; + expect(out.ok).toBe(false); + expect(out.error).toContain("cancelled"); + }); + test.each([ ["empty", ""], ["whitespace", " "], diff --git a/src/tests/provider-claude.test.ts b/src/tests/provider-claude.test.ts index 1e343afd..a0e8e667 100644 --- a/src/tests/provider-claude.test.ts +++ b/src/tests/provider-claude.test.ts @@ -883,6 +883,20 @@ describe("withMcpToolTimeout", () => { // ── buildAgentEnv (GHSA-38vh vector 3) ──────────────────────────────────────── describe("buildAgentEnv", () => { + it("forwards the subscription token a backend sign-in writes", () => { + // The last link in the sign-in chain: "Sign in with Claude" stores + // CLAUDE_CODE_OAUTH_TOKEN, the settings store puts it in process.env live, + // and THIS allowlist decides whether the agent subprocess ever sees it. + // Tighten the prefixes and the sign-in keeps reporting success while + // authenticating as nothing — a failure with no error to follow. + const env = buildAgentEnv({ + PATH: "/usr/bin", + HOME: "/home/deploy", + CLAUDE_CODE_OAUTH_TOKEN: "sk-ant-oat01-AAAABBBBCCCCDDDDEEEEFFFF", + }); + expect(env.CLAUDE_CODE_OAUTH_TOKEN).toBe("sk-ant-oat01-AAAABBBBCCCCDDDDEEEEFFFF"); + }); + it("passes system + Anthropic/Claude vars through but drops daemon secrets", () => { const env = buildAgentEnv({ PATH: "/usr/bin",