From 4d0376e43769df74fd672ccecda16d59bb03fbb0 Mon Sep 17 00:00:00 2001 From: "open-design-crew[bot]" <299007234+open-design-crew[bot]@users.noreply.github.com> Date: Mon, 3 Aug 2026 09:40:31 +0000 Subject: [PATCH] fix(byok): restore local runtime configuration (#6356) * fix(byok): restore local runtime configuration Return BYOK settings to the browser-local configuration path and pass credentials only to the active OpenCode child run. Remove the unpublished secure-profile migration, credential backends, API, CLI, contracts, and MCP exposure while retaining raw-secret rejection on external MCP calls. Validated with focused web and daemon suites, full web tests, workspace typecheck, guard, and diff checks. * test(e2e): align BYOK settings checks with local storage Update the settings UI contract after rolling BYOK credentials back to local app config storage. Assert the entered key is persisted, keep incomplete BYOK drafts from activating, and seed the Local CLI isolation case with a usable BYOK key. Validated with all seven P0 cases in settings-api-protocol.test.ts and the e2e TypeScript check. * fix(daemon): validate plugin request ids explicitly Use a field-specific canonical UUID/ULID validator for attributed request IDs and document that format in the MCP schema and server instructions. Invalid calls now identify requestId instead of incorrectly blaming pluginWorkflowId, while Codex can generate a valid ID on its first attempt. Validated with a red/green MCP observability regression test, daemon typecheck, repository guard, and a real Local Codex plugin smoke that completed in one start_run attempt. * test(pack): follow active PTY consumer in prebundle coverage Point the macOS prebundle boundary test at the Terminal runtime after the retired BYOK credential service was removed. This keeps native node-pty externalization covered without restoring legacy credential code. Validated with the focused mac-prebundle suite, the complete tools-pack suite, tools-pack typechecks, repository guard, and deleted-file reference scans. --------- Co-authored-by: Cheems <94773058+itscheems@users.noreply.github.com> --- apps/daemon/src/byok/credential-service.ts | 596 ------------------ apps/daemon/src/cli.ts | 11 - apps/daemon/src/mcp-brief.ts | 2 +- apps/daemon/src/mcp-observability.ts | 15 +- apps/daemon/src/mcp.ts | 107 +--- apps/daemon/src/routes/byok-credentials.ts | 162 ----- apps/daemon/src/routes/runs.ts | 101 +-- apps/daemon/src/runtimes/byok-opencode.ts | 2 +- apps/daemon/src/server-context.ts | 1 - apps/daemon/src/server.ts | 54 +- apps/daemon/src/services/node-pty.ts | 4 +- apps/daemon/src/tools-byok-cli.ts | 387 ------------ .../tests/byok/credential-routes.test.ts | 196 ------ ...edential-service.node-pty-boundary.test.ts | 66 -- .../tests/byok/credential-service.test.ts | 292 --------- .../tests/byok/run-input-boundary.test.ts | 66 +- apps/daemon/tests/chat-route.test.ts | 64 +- apps/daemon/tests/headless-runs.test.ts | 10 +- apps/daemon/tests/mcp-brief-app.test.ts | 2 +- apps/daemon/tests/mcp-observability.test.ts | 29 + apps/daemon/tests/mcp-runs.test.ts | 76 +-- apps/daemon/tests/tools-byok-cli.test.ts | 206 ------ apps/web/src/App.tsx | 66 +- apps/web/src/components/EntryShell.tsx | 92 +-- apps/web/src/components/EntryView.tsx | 12 +- apps/web/src/components/ProjectView.tsx | 76 ++- apps/web/src/components/SettingsDialog.tsx | 167 +---- apps/web/src/components/byok/preflight.ts | 11 +- apps/web/src/providers/connection-test.ts | 30 - apps/web/src/providers/daemon.ts | 8 +- apps/web/src/state/config.ts | 563 +---------------- apps/web/src/types.ts | 4 - .../components/EntryShell.onboarding.test.tsx | 24 +- .../ProjectView.api-empty-response.test.tsx | 11 +- .../ProjectView.pendingPrompt.test.tsx | 4 +- .../ProjectView.run-isolation.test.tsx | 29 +- .../SettingsDialog.execution.test.tsx | 71 --- .../tests/providers/connection-test.test.ts | 22 - apps/web/tests/providers/sse.test.ts | 18 +- apps/web/tests/state/config.test.ts | 413 +----------- e2e/ui/amr-onboarding.test.ts | 46 +- e2e/ui/api-empty-response.test.ts | 25 +- e2e/ui/project-management-flows.test.ts | 38 +- e2e/ui/settings-api-protocol.test.ts | 39 +- .../contracts/src/api/byok-credentials.ts | 40 -- packages/contracts/src/api/chat.ts | 7 +- packages/contracts/src/index.ts | 1 - tools/pack/tests/mac-prebundle.test.ts | 2 +- 48 files changed, 364 insertions(+), 3904 deletions(-) delete mode 100644 apps/daemon/src/byok/credential-service.ts delete mode 100644 apps/daemon/src/routes/byok-credentials.ts delete mode 100644 apps/daemon/src/tools-byok-cli.ts delete mode 100644 apps/daemon/tests/byok/credential-routes.test.ts delete mode 100644 apps/daemon/tests/byok/credential-service.node-pty-boundary.test.ts delete mode 100644 apps/daemon/tests/byok/credential-service.test.ts delete mode 100644 apps/daemon/tests/tools-byok-cli.test.ts delete mode 100644 packages/contracts/src/api/byok-credentials.ts diff --git a/apps/daemon/src/byok/credential-service.ts b/apps/daemon/src/byok/credential-service.ts deleted file mode 100644 index 4146c35465..0000000000 --- a/apps/daemon/src/byok/credential-service.ts +++ /dev/null @@ -1,596 +0,0 @@ -import { spawn } from 'node:child_process'; -import { randomUUID } from 'node:crypto'; -import { access, mkdir, readFile, rename, unlink, writeFile } from 'node:fs/promises'; -import path from 'node:path'; -import type { - ByokChatProviderConfig, - ByokCredentialProfile, - UpsertByokCredentialProfileRequest, -} from '@open-design/contracts'; -import { - isNodePtyUnavailableError, - loadNodePty, -} from '../services/node-pty.js'; - -const PROFILE_ID_PATTERN = /^byok-[a-z0-9][a-z0-9._-]{2,95}$/u; -const KEYCHAIN_SERVICE = 'dev.opendesign.byok'; -const MAX_SECRET_OUTPUT_BYTES = 64 * 1024; -const INTERACTIVE_SECRET_TIMEOUT_MS = 10_000; - -type StoredProfile = Omit; -type StoredDocument = { - version: 1; - profiles: StoredProfile[]; -}; - -export interface ByokSecretBackend { - readonly kind: string; - available(): Promise; - set(profileId: string, secret: string): Promise; - get(profileId: string): Promise; - delete(profileId: string): Promise; -} - -export interface ResolvedByokCredentialProfile { - profile: ByokCredentialProfile; - provider: ByokChatProviderConfig; - apiKey: string; -} - -export interface ByokCredentialServiceOptions { - backend?: ByokSecretBackend; - dataDir: string; - persistMetadata?: ( - metadataPath: string, - document: { version: 1; profiles: readonly unknown[] }, - ) => Promise; -} - -export class ByokCredentialService { - readonly backend: ByokSecretBackend; - readonly metadataPath: string; - private mutationQueue: Promise = Promise.resolve(); - private readonly persistMetadata: NonNullable< - ByokCredentialServiceOptions['persistMetadata'] - >; - - constructor(options: ByokCredentialServiceOptions) { - this.backend = options.backend ?? createPlatformByokSecretBackend( - process.platform, - options.dataDir, - ); - this.metadataPath = path.join(options.dataDir, 'byok', 'profiles.json'); - this.persistMetadata = options.persistMetadata ?? writeMetadataDocument; - } - - async status(): Promise<{ available: boolean; backend: string }> { - return { - available: await this.backend.available(), - backend: this.backend.kind, - }; - } - - async list(): Promise { - const document = await this.readDocument(); - return Promise.all(document.profiles.map((profile) => this.toPublicProfile(profile))); - } - - async get(profileId: string): Promise { - assertProfileId(profileId); - const stored = (await this.readDocument()).profiles.find((profile) => profile.id === profileId); - return stored ? this.toPublicProfile(stored) : null; - } - - /** - * Checks profile metadata without reading the OS credential store. Run - * admission uses this so the secret is not materialized until the runtime is - * actually being started. - */ - async has(profileId: string): Promise { - assertProfileId(profileId); - return (await this.readDocument()).profiles.some((profile) => profile.id === profileId); - } - - async upsert(input: UpsertByokCredentialProfileRequest): Promise { - return this.serializeMutation(() => this.upsertUnlocked(input)); - } - - private async upsertUnlocked( - input: UpsertByokCredentialProfileRequest, - ): Promise { - const available = await this.backend.available(); - if (!available) { - throw new Error('Secure credential storage is unavailable on this system.'); - } - const normalized = normalizeProfileInput(input); - const document = await this.readDocument(); - const existingIndex = input.id - ? document.profiles.findIndex((profile) => profile.id === input.id) - : -1; - const now = Date.now(); - const id = input.id ?? createProfileId(); - assertProfileId(id); - const existing = existingIndex >= 0 ? document.profiles[existingIndex] : undefined; - const apiKey = typeof input.apiKey === 'string' ? input.apiKey.trim() : ''; - if (normalized.requiresApiKey && !apiKey && !existing) { - throw new Error('An API key is required when creating this BYOK profile.'); - } - const previousSecret = apiKey ? await this.backend.get(id) : null; - if (apiKey) { - await this.backend.set(id, apiKey); - } else if (normalized.requiresApiKey && !(await this.backend.get(id))) { - throw new Error('The BYOK profile has no credential in secure storage.'); - } - const stored: StoredProfile = { - id, - ...normalized, - createdAt: existing?.createdAt ?? now, - updatedAt: now, - }; - if (existingIndex >= 0) document.profiles[existingIndex] = stored; - else document.profiles.push(stored); - try { - await this.writeDocument(document); - } catch (error) { - if (apiKey) { - await this.restoreSecretAfterMetadataFailure( - id, - previousSecret, - error, - ); - } - throw error; - } - return this.toPublicProfile(stored); - } - - async resolve(profileId: string): Promise { - assertProfileId(profileId); - const stored = (await this.readDocument()).profiles.find((profile) => profile.id === profileId); - if (!stored) return null; - const apiKey = stored.requiresApiKey ? (await this.backend.get(profileId))?.trim() ?? '' : ''; - if (stored.requiresApiKey && !apiKey) return null; - const profile = await this.toPublicProfile(stored, apiKey); - return { - profile, - apiKey, - provider: { - protocol: stored.protocol, - apiKey, - baseUrl: stored.baseUrl, - model: stored.model, - requiresApiKey: stored.requiresApiKey, - ...(stored.apiVersion ? { apiVersion: stored.apiVersion } : {}), - }, - }; - } - - async delete(profileId: string): Promise { - return this.serializeMutation(() => this.deleteUnlocked(profileId)); - } - - private async deleteUnlocked(profileId: string): Promise { - assertProfileId(profileId); - const document = await this.readDocument(); - const next = document.profiles.filter((profile) => profile.id !== profileId); - const existed = next.length !== document.profiles.length; - const previousSecret = await this.backend.get(profileId); - const secretDeleted = await this.backend.delete(profileId); - if (existed) { - document.profiles = next; - try { - await this.writeDocument(document); - } catch (error) { - if (secretDeleted && previousSecret !== null) { - await this.restoreSecretAfterMetadataFailure( - profileId, - previousSecret, - error, - ); - } - throw error; - } - } - return existed || secretDeleted; - } - - private serializeMutation(operation: () => Promise): Promise { - const result = this.mutationQueue.then(operation, operation); - this.mutationQueue = result.then( - () => undefined, - () => undefined, - ); - return result; - } - - private async toPublicProfile( - profile: StoredProfile, - knownSecret?: string, - ): Promise { - const secret = profile.requiresApiKey - ? knownSecret ?? await this.backend.get(profile.id) ?? '' - : ''; - return { - ...profile, - configured: !profile.requiresApiKey || Boolean(secret), - ...(secret ? { keyTail: secret.slice(-4) } : {}), - }; - } - - private async readDocument(): Promise { - try { - const parsed = JSON.parse(await readFile(this.metadataPath, 'utf8')) as Partial; - if (parsed.version !== 1 || !Array.isArray(parsed.profiles)) { - return { version: 1, profiles: [] }; - } - return { - version: 1, - profiles: parsed.profiles.filter(isStoredProfile), - }; - } catch (error) { - if ((error as NodeJS.ErrnoException).code === 'ENOENT') { - return { version: 1, profiles: [] }; - } - throw error; - } - } - - private async writeDocument(document: StoredDocument): Promise { - await this.persistMetadata(this.metadataPath, document); - } - - private async restoreSecretAfterMetadataFailure( - profileId: string, - previousSecret: string | null, - metadataError: unknown, - ): Promise { - try { - if (previousSecret === null) { - await this.backend.delete(profileId); - } else { - await this.backend.set(profileId, previousSecret); - } - } catch (rollbackError) { - throw new AggregateError( - [metadataError, rollbackError], - 'BYOK metadata persistence failed and the secure credential rollback also failed.', - ); - } - } -} - -async function writeMetadataDocument( - metadataPath: string, - document: { version: 1; profiles: readonly unknown[] }, -): Promise { - await mkdir(path.dirname(metadataPath), { recursive: true, mode: 0o700 }); - const temporaryPath = `${metadataPath}.${process.pid}.${randomUUID()}.tmp`; - await writeFile(temporaryPath, `${JSON.stringify(document, null, 2)}\n`, { - encoding: 'utf8', - mode: 0o600, - }); - await rename(temporaryPath, metadataPath); -} - -function normalizeProfileInput( - input: UpsertByokCredentialProfileRequest, -): Omit { - const label = requiredBoundedString(input.label, 'label', 120); - const baseUrl = requiredBoundedString(input.baseUrl, 'baseUrl', 2_048).replace(/\/+$/u, ''); - const model = requiredBoundedString(input.model, 'model', 256); - const protocol = input.protocol; - if (!['anthropic', 'openai', 'azure', 'google', 'ollama', 'senseaudio', 'aihubmix'].includes(protocol)) { - throw new Error('Unsupported BYOK protocol.'); - } - try { - const parsed = new URL(baseUrl); - if ( - !['http:', 'https:'].includes(parsed.protocol) - || parsed.username.length > 0 - || parsed.password.length > 0 - || parsed.search.length > 0 - || parsed.hash.length > 0 - ) { - throw new Error(); - } - } catch { - throw new Error( - 'BYOK baseUrl must be an absolute HTTP(S) URL without credentials, query, or fragment.', - ); - } - const apiVersion = typeof input.apiVersion === 'string' ? input.apiVersion.trim() : ''; - return { - label, - protocol, - baseUrl, - model, - requiresApiKey: input.requiresApiKey !== false, - ...(apiVersion ? { apiVersion: apiVersion.slice(0, 128) } : {}), - }; -} - -function requiredBoundedString(value: unknown, field: string, max: number): string { - const trimmed = typeof value === 'string' ? value.trim() : ''; - if (!trimmed || trimmed.length > max) { - throw new Error(`${field} is required and must be at most ${max} characters.`); - } - return trimmed; -} - -function createProfileId(): string { - return `byok-${randomUUID()}`; -} - -function assertProfileId(profileId: string): void { - if (!PROFILE_ID_PATTERN.test(profileId)) throw new Error('Invalid BYOK profile id.'); -} - -function isStoredProfile(value: unknown): value is StoredProfile { - if (!value || typeof value !== 'object' || Array.isArray(value)) return false; - const profile = value as Partial; - return typeof profile.id === 'string' - && PROFILE_ID_PATTERN.test(profile.id) - && typeof profile.label === 'string' - && typeof profile.protocol === 'string' - && typeof profile.baseUrl === 'string' - && typeof profile.model === 'string' - && typeof profile.requiresApiKey === 'boolean' - && typeof profile.createdAt === 'number' - && typeof profile.updatedAt === 'number'; -} - -export function createPlatformByokSecretBackend( - platform: NodeJS.Platform = process.platform, - dataDir?: string, -): ByokSecretBackend { - if (platform === 'darwin') return new MacOsKeychainBackend(); - if (platform === 'linux') return new LinuxSecretServiceBackend(); - if (platform === 'win32' && dataDir) { - return new RetiredWindowsSecretCleanupBackend( - path.join(dataDir, 'byok', 'secrets'), - ); - } - return new UnavailableSecretBackend(platform); -} - -class MacOsKeychainBackend implements ByokSecretBackend { - readonly kind = 'macos-keychain'; - private writable: boolean | null = null; - - async available() { - return this.writable !== false && await commandAvailable('/usr/bin/security'); - } - - async set(profileId: string, secret: string) { - try { - await runInteractiveMacOsSecretCommand( - '/usr/bin/security', - ['add-generic-password', '-a', profileId, '-s', KEYCHAIN_SERVICE, '-U', '-w'], - secret, - ); - this.writable = true; - } catch (error) { - this.writable = false; - if (isNodePtyUnavailableError(error)) { - throw new Error( - 'Secure credential storage is unavailable because its native PTY helper could not be loaded.', - { cause: error }, - ); - } - throw new Error('Secure credential backend command failed.'); - } - } - - async get(profileId: string) { - const result = await runSecretCommand( - '/usr/bin/security', - ['find-generic-password', '-a', profileId, '-s', KEYCHAIN_SERVICE, '-w'], - undefined, - true, - ); - return result === null ? null : result.trimEnd(); - } - - async delete(profileId: string) { - return (await runSecretCommand( - '/usr/bin/security', - ['delete-generic-password', '-a', profileId, '-s', KEYCHAIN_SERVICE], - undefined, - true, - )) !== null; - } -} - -class LinuxSecretServiceBackend implements ByokSecretBackend { - readonly kind = 'linux-secret-service'; - private readonly command = 'secret-tool'; - - async available() { - return commandAvailable(this.command); - } - - async set(profileId: string, secret: string) { - await runSecretCommand( - this.command, - ['store', '--label=Open Design BYOK', 'service', KEYCHAIN_SERVICE, 'account', profileId], - secret, - ); - } - - async get(profileId: string) { - const result = await runSecretCommand( - this.command, - ['lookup', 'service', KEYCHAIN_SERVICE, 'account', profileId], - undefined, - true, - ); - return result === null ? null : result.trimEnd(); - } - - async delete(profileId: string) { - return (await runSecretCommand( - this.command, - ['clear', 'service', KEYCHAIN_SERVICE, 'account', profileId], - undefined, - true, - )) !== null; - } -} - -class UnavailableSecretBackend implements ByokSecretBackend { - readonly kind: string; - - constructor(platform: NodeJS.Platform) { - this.kind = `unavailable-${platform}`; - } - - async available() { return false; } - async set(_profileId: string, _secret: string) { - throw new Error('Secure credential storage is unavailable on this system.'); - } - async get(_profileId: string) { return null; } - async delete(_profileId: string) { return false; } -} - -class RetiredWindowsSecretCleanupBackend extends UnavailableSecretBackend { - constructor(private readonly secretsDir: string) { - super('win32'); - } - - override async delete(profileId: string) { - assertProfileId(profileId); - try { - await unlink(path.join(this.secretsDir, `${profileId}.bin`)); - return true; - } catch (error) { - if ((error as NodeJS.ErrnoException).code === 'ENOENT') return false; - throw error; - } - } -} - -async function commandAvailable(command: string): Promise { - if (path.isAbsolute(command)) { - try { - await access(command); - return true; - } catch { - return false; - } - } - const pathEntries = (process.env.PATH ?? '').split(path.delimiter); - return Promise.any(pathEntries.map(async (entry) => { - const candidate = path.join(entry, command); - await access(candidate); - return true; - })).catch(() => false); -} - -async function runSecretCommand( - command: string, - args: string[], - secretInput?: string, - allowNotFound = false, -): Promise { - return new Promise((resolve, reject) => { - const child = spawn(command, args, { - stdio: ['pipe', 'pipe', 'pipe'], - windowsHide: true, - }); - const stdout: Buffer[] = []; - let stdoutBytes = 0; - child.stdout.on('data', (chunk: Buffer) => { - stdoutBytes += chunk.length; - if (stdoutBytes <= MAX_SECRET_OUTPUT_BYTES) stdout.push(chunk); - }); - child.stderr.resume(); - child.on('error', () => reject(new Error('Secure credential backend command failed.'))); - child.on('close', (code) => { - if (code === 0) return resolve(Buffer.concat(stdout).toString('utf8')); - if (allowNotFound && code === 44) return resolve(null); - if (allowNotFound && code === 1) return resolve(null); - return reject(new Error('Secure credential backend command failed.')); - }); - if (secretInput !== undefined) child.stdin.end(`${secretInput}\n`); - else child.stdin.end(); - }); -} - -/** - * `security add-generic-password -w` intentionally prompts twice when the - * password argument is omitted. A regular stdin pipe is not accepted by the - * macOS tool, while putting the key after `-w` exposes it in the process list. - * Drive only those bounded prompts through a pseudo-terminal and keep all - * output private. - */ -async function runInteractiveMacOsSecretCommand( - command: string, - args: string[], - secret: string, -): Promise { - const { spawn: spawnPty } = await loadNodePty(); - return new Promise((resolve, reject) => { - let settled = false; - let promptResponses = 0; - let promptScan = ''; - let dataDisposable: { dispose(): void } | null = null; - let exitDisposable: { dispose(): void } | null = null; - // `security` does not reliably disable terminal echo before its password - // prompts. Use a fixed shell program (no interpolation) to disable echo, - // then exec the command and its validated argv verbatim. - const child = spawnPty('/bin/sh', [ - '-c', - 'stty -echo; exec "$@"', - 'open-design-keychain', - command, - ...args, - ], { - name: 'xterm-256color', - cols: 80, - rows: 24, - cwd: process.cwd(), - env: { ...process.env }, - }); - const finish = (error?: Error) => { - if (settled) return; - settled = true; - clearTimeout(timer); - dataDisposable?.dispose(); - exitDisposable?.dispose(); - if (error) reject(error); - else resolve(); - }; - const timer = setTimeout(() => { - try { - child.kill(); - } catch { - // Best-effort termination; the generic failure below stays secret-free. - } - finish(new Error('Secure credential backend command failed.')); - }, INTERACTIVE_SECRET_TIMEOUT_MS); - timer.unref?.(); - dataDisposable = child.onData((chunk) => { - // Security prompts should disable terminal echo. Redact defensively - // before retaining the bounded prompt tail anyway. - const safeChunk = secret - ? chunk.split(secret).join('[redacted]') - : chunk; - promptScan = `${promptScan}${safeChunk}`.slice(-512); - const promptCount = ( - promptScan.match(/(?:retype[^\r\n:]*|password[^\r\n:]*)\s*:/giu) - ?? [] - ).length; - while (promptResponses < Math.min(promptCount, 2)) { - child.write(`${secret}\r`); - promptResponses += 1; - } - }); - exitDisposable = child.onExit(({ exitCode }) => { - finish( - exitCode === 0 - ? undefined - : new Error('Secure credential backend command failed.'), - ); - }); - }); -} diff --git a/apps/daemon/src/cli.ts b/apps/daemon/src/cli.ts index 04bb87728f..3ce93619e5 100644 --- a/apps/daemon/src/cli.ts +++ b/apps/daemon/src/cli.ts @@ -12,7 +12,6 @@ import { DESIGN_SYSTEMS_USAGE, isDesignSystemsHelpArg } from './cli-help/index.j import { BRAND_USAGE, isBrandHelpArg } from './cli-help/index.js'; import { parseDesignSystemRenameArgs } from './design-systems/rename-args.js'; import { runLiveArtifactsToolCli } from './tools-live-artifacts-cli.js'; -import { runByokToolCli } from './tools-byok-cli.js'; import { splitResearchSubcommand } from './research/cli-args.js'; import { resolveDaemonUrl } from './daemon-url.js'; import { requestJsonIpc } from '@open-design/sidecar'; @@ -334,7 +333,6 @@ const SUBCOMMAND_MAP = { artifacts: runArtifacts, media: runMedia, mcp: runMcp, - byok: runByok, amr: runAmr, 'message-center': runMessageCenter, research: runResearch, @@ -1472,15 +1470,6 @@ artifact / message body. The daemon writes the bytes into the project's files folder so the FileViewer can preview them immediately.`); } -// --------------------------------------------------------------------------- -// Subcommand: od byok -// --------------------------------------------------------------------------- - -async function runByok(args) { - const result = await runByokToolCli(args); - if (result.exitCode !== 0) process.exit(result.exitCode); -} - // --------------------------------------------------------------------------- // Subcommand: od mcp // --------------------------------------------------------------------------- diff --git a/apps/daemon/src/mcp-brief.ts b/apps/daemon/src/mcp-brief.ts index f07e51b708..ae9484e01b 100644 --- a/apps/daemon/src/mcp-brief.ts +++ b/apps/daemon/src/mcp-brief.ts @@ -51,7 +51,7 @@ const BRIEF_COPY: Record = { artifactNames: ENGLISH_ARTIFACT_NAMES, title: (artifactName) => `Choose the ${artifactName} direction`, description: - 'Choose one option for each decision. The same readable brief can be used with Open Design Cloud, Local Codex, or Secure BYOK.', + 'Choose one option for each decision. The same readable brief can be used with Open Design Cloud or Local Codex.', submitLabel: 'Confirm brief', completeCard: 'Complete the rendered Open Design brief card. The confirmation returns a readable summary; internal correlation values must remain hidden.', diff --git a/apps/daemon/src/mcp-observability.ts b/apps/daemon/src/mcp-observability.ts index d0d9822977..53b02926d7 100644 --- a/apps/daemon/src/mcp-observability.ts +++ b/apps/daemon/src/mcp-observability.ts @@ -147,19 +147,30 @@ export function validateExternalPluginContext( }; } -export function validatePluginWorkflowId(value: unknown): string { +function validateCanonicalPluginCorrelationId( + value: unknown, + fieldName: 'pluginWorkflowId' | 'requestId', +): string { if ( typeof value !== 'string' || value.length > 64 || !WORKFLOW_ID_PATTERN.test(value) ) { throw pluginContractError( - 'pluginWorkflowId must be a canonical UUID or ULID', + `${fieldName} must be a canonical UUID or ULID`, ); } return value; } +export function validatePluginWorkflowId(value: unknown): string { + return validateCanonicalPluginCorrelationId(value, 'pluginWorkflowId'); +} + +export function validatePluginRequestId(value: unknown): string { + return validateCanonicalPluginCorrelationId(value, 'requestId'); +} + export function mapMcpHostProduct( clientInfo: { name?: unknown; version?: unknown } | null | undefined, ): AnalyticsHostProduct { diff --git a/apps/daemon/src/mcp.ts b/apps/daemon/src/mcp.ts index 6a00c8ce58..ba0ed9128b 100644 --- a/apps/daemon/src/mcp.ts +++ b/apps/daemon/src/mcp.ts @@ -57,6 +57,7 @@ import { pluginContractError, resolvePluginGenerationSloWindowMs, validateExternalPluginContext, + validatePluginRequestId, validatePluginWorkflowId, } from './mcp-observability.js'; @@ -67,7 +68,7 @@ const OPEN_DESIGN_BRIEF_APP_RESOURCE = 'ui://open-design/artifact-card-v6.html'; export const MCP_SERVER_INSTRUCTIONS = [ - 'Use only these product names in user-facing replies: Open Design Cloud, Local Codex, and Secure BYOK.', + 'Use only these product names in user-facing replies: Open Design Cloud and Local Codex.', 'Tool names, runtime ids, endpoints, and correlation values are machine protocol. Never repeat them as product copy.', ].join('\n'); @@ -84,7 +85,7 @@ interface ProjectPayload { project?: ProjectSummary; id?: string; name?: string; interface ActiveContext { active?: boolean; projectId?: string; projectName?: string | null; fileName?: string | null; ageMs?: number | null } type ResolvedProject = { id: string; name: string; source: 'uuid' | 'id' | 'exact' | 'slug' | 'substring' }; interface ProjectListCache { baseUrl: string; t: number; list: ProjectSummary[] } -interface McpArgs extends JsonObject { project?: unknown; entry?: unknown; include?: unknown; maxBytes?: unknown; path?: unknown; offset?: unknown; limit?: unknown; since?: unknown; query?: unknown; pattern?: unknown; max?: unknown; name?: unknown; content?: unknown; encoding?: unknown; artifactManifest?: unknown; confirm?: unknown; prompt?: unknown; plugin?: unknown; inputs?: unknown; agent?: unknown; model?: unknown; serviceTier?: unknown; byokProfile?: unknown; apiKey?: unknown; requestId?: unknown; resume?: unknown; runId?: unknown; id?: unknown; designSystem?: unknown; skill?: unknown; includeUnavailable?: unknown; artifactType?: unknown; projectTitle?: unknown; locale?: unknown; knownAnswers?: unknown; skip?: unknown; briefDraftId?: unknown; nonce?: unknown; answers?: unknown; externalPluginContext?: unknown; pluginWorkflowId?: unknown } +interface McpArgs extends JsonObject { project?: unknown; entry?: unknown; include?: unknown; maxBytes?: unknown; path?: unknown; offset?: unknown; limit?: unknown; since?: unknown; query?: unknown; pattern?: unknown; max?: unknown; name?: unknown; content?: unknown; encoding?: unknown; artifactManifest?: unknown; confirm?: unknown; prompt?: unknown; plugin?: unknown; inputs?: unknown; agent?: unknown; model?: unknown; serviceTier?: unknown; apiKey?: unknown; requestId?: unknown; resume?: unknown; runId?: unknown; id?: unknown; designSystem?: unknown; skill?: unknown; includeUnavailable?: unknown; artifactType?: unknown; projectTitle?: unknown; locale?: unknown; knownAnswers?: unknown; skip?: unknown; briefDraftId?: unknown; nonce?: unknown; answers?: unknown; externalPluginContext?: unknown; pluginWorkflowId?: unknown } interface ProjectFileBundleEntry { name: string; mime: string; size: number | null; content: string | null; binary: boolean } interface BundleInput { project: ProjectPayload | ProjectSummary; entry: string; files: ProjectFileBundleEntry[]; truncated: boolean; skippedFileCount?: number; active: ActiveContext | null; resolved?: ResolvedProject | null } interface ErrorWithCode { message?: string; code?: string; cause?: { code?: string } } @@ -616,17 +617,6 @@ export const TOOL_DEFS = [ }, annotations: { ...READ_ANNOTATIONS, title: 'List Open Design plugins' }, }, - { - name: 'list_byok_profiles', - description: - 'List secure local BYOK profile references available to start_run.byokProfile. Returns only non-secret metadata; API keys never cross MCP.', - inputSchema: { - type: 'object', - properties: { pluginWorkflowId: PLUGIN_WORKFLOW_ID_ARG }, - additionalProperties: false, - }, - annotations: { ...READ_ANNOTATIONS, title: 'List secure BYOK profiles' }, - }, { name: 'start_vela_login', description: @@ -695,15 +685,10 @@ export const TOOL_DEFS = [ type: 'string', description: "Service tier override for the selected model, e.g. 'priority' for Codex Fast. Optional.", }, - byokProfile: { - type: 'string', - description: - 'Secure profile id from list_byok_profiles. Selects the local BYOK OpenCode runtime; raw API keys are never accepted by MCP.', - }, requestId: { type: 'string', description: - 'Stable id for this confirmed generation action. Generate it once before calling start_run and reuse it verbatim if the tool response is lost or retried; a different payload with the same id is rejected.', + 'Stable canonical UUID or ULID for this confirmed generation action. Generate it once before calling start_run and reuse it verbatim if the tool response is lost or retried; a different payload with the same id is rejected.', }, resume: { type: 'boolean', @@ -849,7 +834,7 @@ export function localMcpResourceDefinitions() { name: 'Open Design brief', title: 'Choose the artifact direction', description: - 'Interactive local Open Design brief card shared by Open Design Cloud, Local Codex, and Secure BYOK modes.', + 'Interactive local Open Design brief card shared by Open Design Cloud and Local Codex modes.', mimeType: 'text/html;profile=mcp-app', _meta: { ui: { @@ -1271,7 +1256,7 @@ function mcpFailureFacts( const failureStage = name === 'collect_brief' || name === 'confirm_brief' ? 'brief' - : name.includes('vela_login') || name === 'list_byok_profiles' + : name.includes('vela_login') ? 'auth' : name.includes('project') ? 'project' @@ -1622,12 +1607,9 @@ export async function runMcpStdio({ daemonUrl }: RunMcpOptions): Promise { ' - collect_brief first for a new artifact unless the user explicitly', ' asks to skip questions. Let the user complete the rendered card;', ' confirm_brief returns the readable brief to reuse with Open Design', - ' Cloud, Local Codex, or Secure BYOK. Never print or ask the user to copy', + ' Cloud or Local Codex. Never print or ask the user to copy', ' briefDraftId, nonce, or any other internal correlation value.', ' - list_skills / list_plugins to see what you can ask OD to make.', - ' - list_byok_profiles returns secure local credential references when', - ' the user explicitly chooses Secure BYOK. Never request or pass a', - ' raw API key through MCP; pass only start_run.byokProfile.', ' - for Open Design Cloud, call the Cloud login-status tool first.', ' If signed out, call the Cloud sign-in tool once, show its activation', ' URL/code when present, and poll login status until loggedIn:true.', @@ -1639,8 +1621,9 @@ export async function runMcpStdio({ daemonUrl }: RunMcpOptions): Promise { ' generate into; start_run requires an existing project.', ' - start_run(prompt, requestId, [skill], [plugin], [inputs]) kicks off', ' generation in the active or named project and returns a runId.', - ' Generate requestId once per confirmed user action and reuse the', - ' exact same value after a timeout/lost response. Do not call', + ' Generate a canonical UUID or ULID requestId once per confirmed', + ' user action and reuse the exact same value after a timeout/lost', + ' response. Do not call', ' start_run again while get_run reports the original run in flight.', ' If get_run returns failureAction:"recharge", show rechargeUrl;', ' after the user confirms top-up, call the exact original start_run', @@ -2037,8 +2020,6 @@ async function handleMcpToolCall( return ok(await getJson(`${baseUrl}/api/skills`)); case 'list_plugins': return ok(await listPlugins(baseUrl)); - case 'list_byok_profiles': - return ok(await listByokProfiles(baseUrl)); case 'list_agents': return ok(await listAgents(baseUrl, args.includeUnavailable === true)); case 'start_vela_login': { @@ -2263,58 +2244,6 @@ async function listAgents(baseUrl: string, includeUnavailable: boolean): Promise return { agents }; } -async function listByokProfiles(baseUrl: string): Promise { - const payload = await getJson(`${baseUrl}/api/byok/profiles`); - const rawProfiles = Array.isArray(payload.profiles) ? payload.profiles : []; - const profiles = rawProfiles.flatMap((value) => { - if (!value || typeof value !== 'object' || Array.isArray(value)) return []; - const profile = value as JsonObject; - if ( - typeof profile.id !== 'string' - || typeof profile.label !== 'string' - || typeof profile.protocol !== 'string' - || typeof profile.baseUrl !== 'string' - || typeof profile.model !== 'string' - ) { - return []; - } - try { - const parsed = new URL(profile.baseUrl); - if ( - !['http:', 'https:'].includes(parsed.protocol) - || parsed.username.length > 0 - || parsed.password.length > 0 - || parsed.search.length > 0 - || parsed.hash.length > 0 - ) { - return []; - } - } catch { - return []; - } - return [{ - id: profile.id, - label: profile.label, - protocol: profile.protocol, - baseUrl: profile.baseUrl, - model: profile.model, - ...(typeof profile.apiVersion === 'string' - ? { apiVersion: profile.apiVersion } - : {}), - requiresApiKey: profile.requiresApiKey === true, - configured: profile.configured === true, - ...(typeof profile.keyTail === 'string' ? { keyTail: profile.keyTail } : {}), - ...(typeof profile.createdAt === 'number' ? { createdAt: profile.createdAt } : {}), - ...(typeof profile.updatedAt === 'number' ? { updatedAt: profile.updatedAt } : {}), - }]; - }); - return { - available: payload.available === true, - backend: typeof payload.backend === 'string' ? payload.backend : 'unknown', - profiles, - }; -} - // Derive a valid project id ([A-Za-z0-9._-], <=128) from a display name, // with a short random suffix so repeated creates with the same name // don't collide on the daemon's primary key. @@ -2341,7 +2270,7 @@ async function startRun( || containsMcpCredentialField(args.inputs) ) { throw new Error( - 'raw API keys are not accepted by Open Design MCP. Save the key through the Open Design UI or `od byok save --api-key-stdin`, then pass only byokProfile.', + 'raw API keys are not accepted by Open Design MCP. Configure Local BYOK in the Open Design UI and start that run from the local product instead.', ); } const { id, resolved, active } = await resolveProjectArg(baseUrl, args.project); @@ -2360,7 +2289,7 @@ async function startRun( : randomUUID(); const body: JsonObject = { projectId: id, clientRequestId: requestId }; if (options.pluginAttribution) { - validatePluginWorkflowId(requestId); + validatePluginRequestId(requestId); const logical = logicalPluginRequestDigest(requestId); body.analyticsHints = { entrySurface: 'external_mcp', @@ -2397,18 +2326,6 @@ async function startRun( if (typeof args.serviceTier === 'string' && args.serviceTier.length > 0) { body.serviceTier = args.serviceTier; } - if (args.byokProfile !== undefined) { - requireString(args.byokProfile, 'byokProfile'); - if ( - typeof args.agent === 'string' - && args.agent.length > 0 - && args.agent !== 'byok-opencode' - ) { - throw new Error('byokProfile can only be used with the byok-opencode agent.'); - } - body.agentId = 'byok-opencode'; - body.byokProfileId = args.byokProfile; - } if (args.inputs !== undefined) { if (args.inputs === null || typeof args.inputs !== 'object' || Array.isArray(args.inputs)) { throw new Error('inputs must be an object'); diff --git a/apps/daemon/src/routes/byok-credentials.ts b/apps/daemon/src/routes/byok-credentials.ts deleted file mode 100644 index 85ed0fba6e..0000000000 --- a/apps/daemon/src/routes/byok-credentials.ts +++ /dev/null @@ -1,162 +0,0 @@ -import type { Express, RequestHandler, Response } from 'express'; -import type { - ByokChatProtocol, - ConnectionTestResponse, - UpsertByokCredentialProfileRequest, -} from '@open-design/contracts'; -import type { - ByokCredentialService, - ResolvedByokCredentialProfile, -} from '../byok/credential-service.js'; - -export interface RegisterByokCredentialRoutesDeps { - http: { - requireLocalDaemonRequest: RequestHandler; - sendApiError: ( - res: Response, - status: number, - code: string, - message: string, - ) => unknown; - }; - byokCredentials: Pick< - ByokCredentialService, - 'delete' | 'list' | 'resolve' | 'status' | 'upsert' - >; - connectionTest: ( - input: { - protocol: ByokChatProtocol; - baseUrl: string; - apiKey: string; - model: string; - apiVersion?: string; - }, - ) => Promise; -} - -export function registerByokCredentialRoutes( - app: Express, - deps: RegisterByokCredentialRoutesDeps, -): void { - const { requireLocalDaemonRequest, sendApiError } = deps.http; - - app.get('/api/byok/profiles', requireLocalDaemonRequest, async (_req, res) => { - try { - const [status, profiles] = await Promise.all([ - deps.byokCredentials.status(), - deps.byokCredentials.list(), - ]); - res.setHeader('Cache-Control', 'no-store'); - return res.json({ ...status, profiles }); - } catch { - return sendApiError( - res, - 500, - 'INTERNAL', - 'Failed to read BYOK credential profiles', - ); - } - }); - - app.post('/api/byok/profiles', requireLocalDaemonRequest, async (req, res) => { - try { - const profile = await deps.byokCredentials.upsert( - requestBody(req.body), - ); - res.setHeader('Cache-Control', 'no-store'); - return res.status(201).json({ profile }); - } catch (error) { - const message = safeCredentialErrorMessage(error); - const unavailable = /secure credential storage is unavailable/iu.test(message); - return sendApiError( - res, - unavailable ? 503 : 400, - unavailable ? 'UPSTREAM_UNAVAILABLE' : 'VALIDATION_FAILED', - message, - ); - } - }); - - app.delete('/api/byok/profiles/:id', requireLocalDaemonRequest, async (req, res) => { - try { - const deleted = await deps.byokCredentials.delete(routeProfileId(req.params.id)); - if (!deleted) { - return sendApiError(res, 404, 'NOT_FOUND', 'BYOK credential profile not found'); - } - return res.status(204).end(); - } catch (error) { - return sendApiError( - res, - 400, - 'VALIDATION_FAILED', - safeCredentialErrorMessage(error), - ); - } - }); - - app.post('/api/byok/profiles/:id/test', requireLocalDaemonRequest, async (req, res) => { - let resolved: ResolvedByokCredentialProfile | null; - try { - resolved = await deps.byokCredentials.resolve(routeProfileId(req.params.id)); - } catch (error) { - return sendApiError( - res, - 400, - 'VALIDATION_FAILED', - safeCredentialErrorMessage(error), - ); - } - if (!resolved) { - return sendApiError( - res, - 404, - 'NOT_FOUND', - 'BYOK credential profile is missing or has no stored credential', - ); - } - try { - // The key exists only in this local variable and in the outbound - // authorization header built by the provider tester. Its result shape is - // already redacted and never contains the request credential. - const result = await deps.connectionTest({ - protocol: resolved.profile.protocol, - baseUrl: resolved.profile.baseUrl, - apiKey: resolved.apiKey, - model: resolved.profile.model, - ...(resolved.profile.apiVersion - ? { apiVersion: resolved.profile.apiVersion } - : {}), - }); - res.setHeader('Cache-Control', 'no-store'); - return res.json(result); - } catch { - return sendApiError(res, 500, 'INTERNAL', 'Connection test failed'); - } - }); -} - -function requestBody(value: unknown): UpsertByokCredentialProfileRequest { - if (!value || typeof value !== 'object' || Array.isArray(value)) { - throw new Error('A BYOK credential profile body is required.'); - } - return value as UpsertByokCredentialProfileRequest; -} - -function routeProfileId(value: string | string[] | undefined): string { - if (typeof value !== 'string' || !value) { - throw new Error('Invalid BYOK profile id.'); - } - return value; -} - -function safeCredentialErrorMessage(error: unknown): string { - if (!(error instanceof Error)) return 'Invalid BYOK credential profile.'; - const message = error.message.trim(); - if ( - /^(?:secure credential storage|an API key|the BYOK profile|invalid BYOK profile id|unsupported BYOK protocol|BYOK baseUrl|label|baseUrl|model)/iu - .test(message) - ) { - return message; - } - return 'Invalid BYOK credential profile.'; -} diff --git a/apps/daemon/src/routes/runs.ts b/apps/daemon/src/routes/runs.ts index c1571c4071..b4218ff082 100644 --- a/apps/daemon/src/routes/runs.ts +++ b/apps/daemon/src/routes/runs.ts @@ -8,6 +8,7 @@ import { RUN_RESULT_PACKAGE_SCHEMA, type AppliedPluginSnapshot, type ArtifactManifest, + type ByokChatProviderConfig, type ChatRunStatus, type ChatRunStatusResponse, type ProjectMetadata as ContractProjectMetadata, @@ -30,7 +31,6 @@ import { codexSessionIdFromRunEvents, readCodexRolloutFirstCall, } from '../codex-rollout-usage.js'; -import type { ByokCredentialService } from '../byok/credential-service.js'; import type { ConnectorService } from '../connectors/service.js'; import { conversationTurnIndexForRun, @@ -99,6 +99,7 @@ import { } from '../run-tool-bundle.js'; import type { DetectedAgent, RuntimeAgentDef } from '../runtimes/types.js'; import { + buildOpenCodeByokProviderConfig, BYOK_OPENCODE_AGENT_ID, BYOK_OPENCODE_PROVIDER_REQUIRED_MESSAGE, } from '../runtimes/byok-opencode.js'; @@ -465,7 +466,6 @@ export interface RegisterRunRoutesDeps { chat: { startChatRun: (meta: RunCreateMeta, run: ChatRun) => Promise; }; - byokCredentials: Pick; lifecycle: { isDaemonShuttingDown: () => boolean; }; @@ -690,6 +690,7 @@ function routeParamId(req: ApiRequest): string | null { function withoutSensitiveRunInput(body: JsonRecord): JsonRecord { const sanitized = { ...body }; delete sanitized.byokProvider; + delete sanitized.byokProfileId; delete sanitized.apiKey; delete sanitized.rechargeResumeCapability; return sanitized; @@ -777,49 +778,12 @@ function externalPluginAttributionMismatch( ); } -const CREDENTIAL_FIELD_PATTERN = - /^(?:api[_-]?key|authorization|access[_-]?token|refresh[_-]?token|secret|password)$/iu; - -function containsCredentialShapedField(value: unknown, depth = 0): boolean { - // Over-complex structured input is rejected rather than allowed to outrun - // the secret scan. This keeps the credential boundary fail-closed. - if (depth > 20) return true; - if (value === null || typeof value !== 'object') return false; - if (Array.isArray(value)) { - return value.some((item) => containsCredentialShapedField(item, depth + 1)); - } - return Object.entries(value as JsonRecord).some(([key, nested]) => - CREDENTIAL_FIELD_PATTERN.test(key) - || containsCredentialShapedField(nested, depth + 1), - ); -} - -async function byokRunInputError( - meta: JsonRecord, - credentials: Pick, -): Promise { - const isByokRequest = - meta.agentId === BYOK_OPENCODE_AGENT_ID - || typeof meta.byokProfileId === 'string'; - if ( - Object.prototype.hasOwnProperty.call(meta, 'byokProvider') - || Object.prototype.hasOwnProperty.call(meta, 'apiKey') - || (isByokRequest && containsCredentialShapedField(meta)) - ) { - return 'Raw BYOK credentials are not accepted by run APIs; save a secure credential profile and pass byokProfileId.'; - } - if (meta.agentId !== BYOK_OPENCODE_AGENT_ID) return null; - const profileId = typeof meta.byokProfileId === 'string' - ? meta.byokProfileId.trim() - : ''; - if (!profileId) return BYOK_OPENCODE_PROVIDER_REQUIRED_MESSAGE; - try { - return await credentials.has(profileId) - ? null - : BYOK_OPENCODE_PROVIDER_REQUIRED_MESSAGE; - } catch { - return BYOK_OPENCODE_PROVIDER_REQUIRED_MESSAGE; - } +function hasCompleteByokOpenCodeConfig(meta: JsonRecord): boolean { + if (meta.agentId !== BYOK_OPENCODE_AGENT_ID) return true; + return buildOpenCodeByokProviderConfig( + meta.byokProvider as ByokChatProviderConfig | null | undefined, + typeof meta.model === 'string' ? meta.model : null, + ) !== null; } function toOdNativeEvent(record: RunEventRecord): OdNativeEvent | null { @@ -882,16 +846,12 @@ export function registerRunRoutes(app: Express, ctx: RegisterRunRoutesDeps) { if (!toolBundle.ok) { return sendApiError(res, 400, 'BAD_REQUEST', toolBundle.message); } - const initialByokInputError = await byokRunInputError( - requestBody, - ctx.byokCredentials, - ); - if (initialByokInputError) { + if (!hasCompleteByokOpenCodeConfig(requestBody)) { return sendApiError( res, 400, 'VALIDATION_FAILED', - initialByokInputError, + BYOK_OPENCODE_PROVIDER_REQUIRED_MESSAGE, ); } // Reject a client-supplied conversationId that is missing a projectId or @@ -1007,16 +967,17 @@ export function registerRunRoutes(app: Express, ctx: RegisterRunRoutesDeps) { console.warn('[runs] agent id fallback failed', err); } } - const resolvedByokInputError = await byokRunInputError( - meta, - ctx.byokCredentials, - ); - if (resolvedByokInputError) { + if (!hasCompleteByokOpenCodeConfig({ + ...meta, + ...(requestBody.byokProvider !== undefined + ? { byokProvider: requestBody.byokProvider } + : {}), + })) { return sendApiError( res, 400, 'VALIDATION_FAILED', - resolvedByokInputError, + BYOK_OPENCODE_PROVIDER_REQUIRED_MESSAGE, ); } const toolBundleSupport = validateRunToolBundleForAgent( @@ -1355,7 +1316,13 @@ export function registerRunRoutes(app: Express, ctx: RegisterRunRoutesDeps) { console.warn('[plugins] skill candidate hook setup failed', err); } } - design.runs.start(run, () => startChatRun(meta, run)); + const executionMeta: RunCreateMeta = { + ...meta, + ...(requestBody.byokProvider !== undefined + ? { byokProvider: requestBody.byokProvider } + : {}), + }; + design.runs.start(run, () => startChatRun(executionMeta, run)); const reqBody = requestBody; const analyticsHints = @@ -2349,16 +2316,12 @@ export function registerRunRoutes(app: Express, ctx: RegisterRunRoutesDeps) { return sendApiError(res, 404, 'CONVERSATION_NOT_FOUND', 'conversation not found for project'); } } - const byokInputError = await byokRunInputError( - requestBody, - ctx.byokCredentials, - ); - if (byokInputError) { + if (!hasCompleteByokOpenCodeConfig(requestBody)) { return sendApiError( res, 400, 'VALIDATION_FAILED', - byokInputError, + BYOK_OPENCODE_PROVIDER_REQUIRED_MESSAGE, ); } const meta: RunCreateMeta = { @@ -2389,9 +2352,15 @@ export function registerRunRoutes(app: Express, ctx: RegisterRunRoutesDeps) { } design.runs.stream(run, req, res); reconcileAssistantMessageOnRunEnd(db, design.runs, run); - design.runs.start(run, () => startChatRun(meta, run)); + const executionMeta: RunCreateMeta = { + ...meta, + ...(requestBody.byokProvider !== undefined + ? { byokProvider: requestBody.byokProvider } + : {}), + }; + design.runs.start(run, () => startChatRun(executionMeta, run)); }); } -export const __forTestByokRunInputError = byokRunInputError; +export const __forTestHasCompleteByokOpenCodeConfig = hasCompleteByokOpenCodeConfig; export const __forTestWithoutSensitiveRunInput = withoutSensitiveRunInput; diff --git a/apps/daemon/src/runtimes/byok-opencode.ts b/apps/daemon/src/runtimes/byok-opencode.ts index af8c6b4712..d6f3df6dc1 100644 --- a/apps/daemon/src/runtimes/byok-opencode.ts +++ b/apps/daemon/src/runtimes/byok-opencode.ts @@ -4,7 +4,7 @@ export const BYOK_OPENCODE_AGENT_ID = 'byok-opencode'; export const BYOK_OPENCODE_PROVIDER_ID = 'open-design-byok'; export const BYOK_OPENCODE_API_KEY_ENV = 'OPEN_DESIGN_BYOK_API_KEY'; export const BYOK_OPENCODE_PROVIDER_REQUIRED_MESSAGE = - 'BYOK OpenCode requires a configured secure credential profile for this run.'; + 'BYOK OpenCode requires a complete provider configuration for this run.'; const DEFAULT_CONTEXT_TOKEN_LIMIT = 128_000; const DEFAULT_OUTPUT_TOKEN_LIMIT = 16_384; diff --git a/apps/daemon/src/server-context.ts b/apps/daemon/src/server-context.ts index 2a39ddb108..9e20409a61 100644 --- a/apps/daemon/src/server-context.ts +++ b/apps/daemon/src/server-context.ts @@ -134,7 +134,6 @@ export interface ServerContext { finalize: any; handoff: any; chat: any; - byokCredentials: any; messages: any; agents: any; critique: any; diff --git a/apps/daemon/src/server.ts b/apps/daemon/src/server.ts index b52cbed144..935f82f4e2 100644 --- a/apps/daemon/src/server.ts +++ b/apps/daemon/src/server.ts @@ -632,7 +632,6 @@ import { EmptyTranscriptError, synthesizeHandoffPrompt } from './design/index.js import { TranscriptExportLockedError } from './transcript-export.js'; import { registerChatRoutes } from './routes/chat.js'; import { registerRunRoutes } from './routes/runs.js'; -import { registerByokCredentialRoutes } from './routes/byok-credentials.js'; import { registerTerminalRoutes } from './routes/terminal.js'; import { createTerminalService } from './terminals.js'; import { registerSocialShareRoutes } from './routes/social-share.js'; @@ -698,7 +697,6 @@ import { apiTokenFromEnv, isApiAuthDisabled, isApiTokenMiddlewareEnabled } from import { createOpenDesignPublicMetadataService } from './services/open-design-public-metadata.js'; import { createWhatsNewService } from './services/whats-new.js'; import { execCommandViaLoginShell } from './services/login-shell.js'; -import { ByokCredentialService } from './byok/credential-service.js'; import { OFFICIAL_MARKETPLACE_ID, createMarketplaceSeedHelpers, @@ -859,9 +857,6 @@ const SANDBOX_MODE_ENABLED = isSandboxModeEnabled(process.env); const RUNTIME_DATA_DIR = resolveDataDir(process.env.OD_DATA_DIR, PROJECT_ROOT, { requireExplicit: SANDBOX_MODE_ENABLED, }); -const defaultByokCredentialService = new ByokCredentialService({ - dataDir: RUNTIME_DATA_DIR, -}); const SANDBOX_RUNTIME = resolveSandboxRuntimeConfig(SANDBOX_MODE_ENABLED, RUNTIME_DATA_DIR); ensureSandboxRuntimeDirs(SANDBOX_RUNTIME); const PLUGIN_LOCKFILE_PATH = path.join(RUNTIME_DATA_DIR, 'od-plugin-lock.json'); @@ -2046,7 +2041,6 @@ export interface DaemonRuntimeContext { } export interface StartServerOptions { - byokCredentialService?: ByokCredentialService; desktopArtifactExporter?: DesktopArtifactExporter | null; desktopPdfExporter?: DesktopPdfExporter | null; desktopSlideRenderer?: DesktopSlideRenderer | null; @@ -2064,7 +2058,6 @@ export interface StartServerResult { } export async function startServer({ - byokCredentialService = defaultByokCredentialService, port = 7456, host = normalizeDaemonBindHost(process.env.OD_BIND_HOST), returnServer = false, @@ -2994,11 +2987,6 @@ export async function startServer({ http: httpDeps, paths: pathDeps, }); - registerByokCredentialRoutes(app, { - http: { requireLocalDaemonRequest, sendApiError }, - byokCredentials: byokCredentialService, - connectionTest: testProviderConnection, - }); // Project workspace registerActiveContextRoutes(app, { db, @@ -4334,7 +4322,7 @@ export async function startServer({ research, context, titleGeneration, - byokProfileId, + byokProvider, byokMediaDefaults, } = chatBody; lifecycle.mark('prompt_build_start'); @@ -4380,22 +4368,10 @@ export async function startServer({ ); if (!def.bin) return design.runs.fail(run, 'AGENT_UNAVAILABLE', 'agent has no binary'); - let resolvedByokCredential = null; - if (def.id === 'byok-opencode') { - try { - resolvedByokCredential = - typeof byokProfileId === 'string' && byokProfileId - ? await byokCredentialService.resolve(byokProfileId) - : null; - } catch { - resolvedByokCredential = null; - } - } const byokOpenCodeProvider = def.id === 'byok-opencode' ? buildOpenCodeByokProviderConfig( - resolvedByokCredential?.provider, - resolvedByokCredential?.profile.model - ?? (typeof model === 'string' ? model : null), + byokProvider, + typeof model === 'string' ? model : null, ) : null; if (def.id === 'byok-opencode' && !byokOpenCodeProvider) { @@ -4406,7 +4382,7 @@ export async function startServer({ ); } const requestedRuntimeModel = def.id === 'byok-opencode' - ? resolvedByokCredential?.profile.model ?? null + ? byokOpenCodeProvider?.modelId ?? null : model; // Validate the checked-in `inactivityTimeoutMs` hint immediately // after the runtime def is selected and before any side-effectful @@ -6913,9 +6889,9 @@ export async function startServer({ // mini extraction in the background just because the user has // an OpenAI key parked in media-config. // - // Normalize the spawn-resolved BYOK profile shape for the memory - // extractor. The raw secret never entered the persisted run body; it is - // held only by this run closure while the child is alive. + // Normalize the run-scoped BYOK provider shape for the memory extractor. + // The raw secret never enters the persisted run body; it is held only by + // this run closure while the child is alive. const memoryChatProvider: { provider?: string; apiKey?: string; @@ -6923,14 +6899,14 @@ export async function startServer({ apiVersion?: string; model?: string; requiresApiKey?: boolean; - } | null = resolvedByokCredential + } | null = byokProvider ? { - provider: resolvedByokCredential.profile.protocol, - apiKey: resolvedByokCredential.apiKey, - baseUrl: resolvedByokCredential.profile.baseUrl, - apiVersion: resolvedByokCredential.profile.apiVersion, - model: resolvedByokCredential.profile.model, - requiresApiKey: resolvedByokCredential.profile.requiresApiKey, + provider: (byokProvider as { protocol?: string }).protocol ?? undefined, + apiKey: (byokProvider as { apiKey?: string }).apiKey, + baseUrl: (byokProvider as { baseUrl?: string }).baseUrl, + apiVersion: (byokProvider as { apiVersion?: string }).apiVersion, + model: typeof safeModel === 'string' ? safeModel : undefined, + requiresApiKey: (byokProvider as { requiresApiKey?: boolean }).requiresApiKey, } : null; const memoryOptions = { @@ -8648,7 +8624,6 @@ export async function startServer({ paths: { PROJECTS_DIR, RUNTIME_DATA_DIR }, agents: { detectAgents, getAgentDef }, chat: { startChatRun }, - byokCredentials: byokCredentialService, lifecycle: { isDaemonShuttingDown: () => daemonShuttingDown }, plugins: { connectorService, @@ -9067,7 +9042,6 @@ export async function startServer({ finalize: finalizeDeps, handoff: handoffDeps, chat: { startChatRun }, - byokCredentials: byokCredentialService, messages: { pinAssistantMessageOnRunCreate, reconcileAssistantMessageOnRunEnd, diff --git a/apps/daemon/src/services/node-pty.ts b/apps/daemon/src/services/node-pty.ts index 8819f6a0ee..0154f8f880 100644 --- a/apps/daemon/src/services/node-pty.ts +++ b/apps/daemon/src/services/node-pty.ts @@ -26,8 +26,8 @@ export function isNodePtyUnavailableError(error: unknown): error is NodePtyUnava /** * Resolve node-pty's POSIX spawn-helper candidates without importing its - * native addon. The shared lookup keeps Terminal and Keychain PTY users on the - * same package-relative runtime boundary. + * native addon. The lookup keeps Terminal users on the package-relative + * runtime boundary used by the packaged app. */ export function spawnHelperCandidatePaths(): string[] { if (process.platform === 'win32') return []; diff --git a/apps/daemon/src/tools-byok-cli.ts b/apps/daemon/src/tools-byok-cli.ts deleted file mode 100644 index 9ba2812413..0000000000 --- a/apps/daemon/src/tools-byok-cli.ts +++ /dev/null @@ -1,387 +0,0 @@ -import type { - ByokCredentialProfile, - ByokCredentialProfileResponse, - ByokCredentialProfilesResponse, - ConnectionTestResponse, - UpsertByokCredentialProfileRequest, -} from '@open-design/contracts'; - -import { resolveDaemonUrl } from './daemon-url.js'; - -interface ByokCliDependencies { - fetchImpl?: typeof fetch; - readStdin?: () => Promise; - resolveDaemonUrlImpl?: typeof resolveDaemonUrl; - stdout?: (text: string) => void; - stderr?: (text: string) => void; -} - -interface ParsedByokArgs { - command: 'list' | 'save' | 'test' | 'delete' | 'help'; - positionals: string[]; - values: Record; - booleans: Set; -} - -export interface ByokCliResult { - exitCode: number; -} - -const USAGE = `Usage: - od byok list [--json] [--daemon-url ] - od byok save --label --protocol --base-url \\ - --model (--api-key-stdin | --no-api-key) [--id ] \\ - [--api-version ] [--json] - od byok test [--json] [--daemon-url ] - od byok delete [--json] [--daemon-url ] - -Security: - API keys are accepted only from stdin via --api-key-stdin. Never put a key - in command arguments, chat, environment variables, manifests, or files. - Use --no-api-key only for local/self-hosted endpoints that need no credential. -`; - -const VALUE_FLAGS = new Set([ - 'daemon-url', - 'id', - 'label', - 'protocol', - 'base-url', - 'model', - 'api-version', -]); -const BOOLEAN_FLAGS = new Set(['api-key-stdin', 'no-api-key', 'json', 'help', 'h']); -const BYOK_PROTOCOLS = new Set([ - 'anthropic', - 'openai', - 'azure', - 'google', - 'ollama', - 'senseaudio', - 'aihubmix', -]); - -function parseArgs(args: string[]): ParsedByokArgs | { error: string } { - const normalized = args[0] === 'profiles' ? args.slice(1) : args; - const first = normalized[0]; - const command = - first === undefined || first === 'help' || first === '--help' || first === '-h' - ? 'help' - : first; - if ( - command !== 'list' - && command !== 'save' - && command !== 'test' - && command !== 'delete' - && command !== 'help' - ) { - return { error: `unknown BYOK command: ${command}` }; - } - - const values: Record = {}; - const booleans = new Set(); - const positionals: string[] = []; - for ( - let index = command === 'help' && first?.startsWith('-') ? 0 : 1; - index < normalized.length; - index += 1 - ) { - const arg = normalized[index]; - if (!arg) continue; - if (!arg.startsWith('--')) { - positionals.push(arg); - continue; - } - const separator = arg.indexOf('='); - const key = separator >= 0 ? arg.slice(2, separator) : arg.slice(2); - if (key === 'api-key') { - return { - error: - '--api-key is forbidden because command arguments may be exposed. Use --api-key-stdin.', - }; - } - if (BOOLEAN_FLAGS.has(key)) { - if (separator >= 0) return { error: `--${key} does not accept a value` }; - booleans.add(key); - continue; - } - if (!VALUE_FLAGS.has(key)) return { error: `unknown option: --${key}` }; - const value = separator >= 0 ? arg.slice(separator + 1) : normalized[++index]; - if (!value || value.startsWith('--')) return { error: `--${key} requires a value` }; - values[key] = value; - } - if (booleans.has('help') || booleans.has('h')) { - return { command: 'help', positionals: [], values, booleans }; - } - return { command, positionals, values, booleans }; -} - -async function defaultReadStdin(): Promise { - process.stdin.setEncoding('utf8'); - let value = ''; - for await (const chunk of process.stdin) { - value += chunk; - if (value.length > 65_536) { - throw new Error('credential input exceeds the 64 KiB safety limit'); - } - } - return value; -} - -function safeHttpBaseUrl(value: string): string | null { - try { - const url = new URL(value); - if ( - !['http:', 'https:'].includes(url.protocol) - || url.username.length > 0 - || url.password.length > 0 - || url.search.length > 0 - || url.hash.length > 0 - ) { - return null; - } - return value; - } catch { - return null; - } -} - -function safeLoopbackDaemonBaseUrl(value: string): string | null { - try { - const url = new URL(value); - if ( - !['http:', 'https:'].includes(url.protocol) - || !new Set(['127.0.0.1', '::1', 'localhost']).has(url.hostname.toLowerCase()) - || url.username.length > 0 - || url.password.length > 0 - || url.search.length > 0 - || url.hash.length > 0 - ) { - return null; - } - return value.replace(/\/+$/u, ''); - } catch { - return null; - } -} - -function safeProfile(value: unknown): ByokCredentialProfile | null { - if (!value || typeof value !== 'object' || Array.isArray(value)) return null; - const profile = value as Partial; - if ( - typeof profile.id !== 'string' - || typeof profile.label !== 'string' - || typeof profile.protocol !== 'string' - || typeof profile.baseUrl !== 'string' - || typeof profile.model !== 'string' - ) { - return null; - } - const baseUrl = safeHttpBaseUrl(profile.baseUrl); - if (!baseUrl) return null; - return { - id: profile.id, - label: profile.label, - protocol: profile.protocol, - baseUrl, - model: profile.model, - ...(typeof profile.apiVersion === 'string' ? { apiVersion: profile.apiVersion } : {}), - requiresApiKey: profile.requiresApiKey === true, - configured: profile.configured === true, - ...(typeof profile.keyTail === 'string' ? { keyTail: profile.keyTail } : {}), - createdAt: typeof profile.createdAt === 'number' ? profile.createdAt : 0, - updatedAt: typeof profile.updatedAt === 'number' ? profile.updatedAt : 0, - }; -} - -async function requestJson( - fetchImpl: typeof fetch, - url: string, - init?: RequestInit, -): Promise { - let response: Response; - try { - response = await fetchImpl(url, init); - } catch { - throw new Error('Open Design daemon is not reachable'); - } - if (!response.ok) { - throw new Error(`Open Design daemon rejected the request (HTTP ${response.status})`); - } - if (response.status === 204) return {}; - try { - return await response.json(); - } catch { - throw new Error('Open Design daemon returned invalid JSON'); - } -} - -function printProfile(profile: ByokCredentialProfile): string { - const credential = profile.configured - ? `configured${profile.keyTail ? ` (…${profile.keyTail})` : ''}` - : 'not configured'; - return `${profile.id}\t${profile.label}\t${profile.protocol}\t${profile.model}\t${credential}`; -} - -function safeConnectionTest(value: unknown): Pick< - ConnectionTestResponse, - 'ok' | 'kind' | 'latencyMs' | 'model' | 'status' -> | null { - if (!value || typeof value !== 'object' || Array.isArray(value)) return null; - const result = value as Partial; - if ( - typeof result.ok !== 'boolean' - || typeof result.kind !== 'string' - || typeof result.latencyMs !== 'number' - ) { - return null; - } - return { - ok: result.ok, - kind: result.kind, - latencyMs: result.latencyMs, - ...(typeof result.model === 'string' ? { model: result.model } : {}), - ...(typeof result.status === 'number' ? { status: result.status } : {}), - }; -} - -export async function runByokToolCli( - args: string[], - dependencies: ByokCliDependencies = {}, -): Promise { - const parsed = parseArgs(args); - const stdout = dependencies.stdout ?? ((text: string) => process.stdout.write(text)); - const stderr = dependencies.stderr ?? ((text: string) => process.stderr.write(text)); - const fail = (message: string): ByokCliResult => { - stderr(`${message}\n`); - return { exitCode: 1 }; - }; - - if ('error' in parsed) return fail(parsed.error); - if (parsed.command === 'help') { - stdout(USAGE); - return { exitCode: 0 }; - } - - const resolveUrl = dependencies.resolveDaemonUrlImpl ?? resolveDaemonUrl; - const daemonBaseUrl = safeLoopbackDaemonBaseUrl( - await resolveUrl( - parsed.values['daemon-url'] - ? { flagUrl: parsed.values['daemon-url'] } - : {}, - ), - ); - if (!daemonBaseUrl) { - return fail('BYOK credentials may only be sent to a loopback Open Design daemon URL'); - } - const fetchImpl = dependencies.fetchImpl ?? fetch; - const useJson = parsed.booleans.has('json'); - - try { - if (parsed.command === 'list') { - if (parsed.positionals.length > 0) return fail('list does not accept positional arguments'); - const payload = await requestJson( - fetchImpl, - `${daemonBaseUrl}/api/byok/profiles`, - ) as Partial; - const profiles = Array.isArray(payload.profiles) - ? payload.profiles - .map(safeProfile) - .filter((profile): profile is ByokCredentialProfile => profile !== null) - : []; - if (useJson) { - stdout(`${JSON.stringify({ - available: payload.available === true, - backend: typeof payload.backend === 'string' ? payload.backend : 'unknown', - profiles, - })}\n`); - } else if (profiles.length === 0) { - stdout('No secure BYOK profiles configured.\n'); - } else { - stdout(`${profiles.map(printProfile).join('\n')}\n`); - } - return { exitCode: 0 }; - } - - if (parsed.command === 'save') { - if (parsed.positionals.length > 0) return fail('save does not accept positional arguments'); - for (const required of ['label', 'protocol', 'base-url', 'model']) { - if (!parsed.values[required]) return fail(`--${required} is required`); - } - if (!BYOK_PROTOCOLS.has(parsed.values.protocol!)) { - return fail(`--protocol must be one of: ${[...BYOK_PROTOCOLS].join(', ')}`); - } - const providerBaseUrl = safeHttpBaseUrl(parsed.values['base-url']!); - if (!providerBaseUrl) { - return fail('--base-url must be an HTTP(S) URL without credentials, query, or fragment'); - } - const readsApiKey = parsed.booleans.has('api-key-stdin'); - const isKeyless = parsed.booleans.has('no-api-key'); - if (readsApiKey === isKeyless) { - return fail('choose exactly one of --api-key-stdin or --no-api-key'); - } - let apiKey = ''; - if (readsApiKey) { - const readStdin = dependencies.readStdin ?? defaultReadStdin; - apiKey = (await readStdin()).replace(/[\r\n]+$/u, ''); - if (!apiKey) return fail('stdin did not contain an API key'); - } - const request: UpsertByokCredentialProfileRequest = { - ...(parsed.values.id ? { id: parsed.values.id } : {}), - label: parsed.values.label!, - protocol: parsed.values.protocol! as UpsertByokCredentialProfileRequest['protocol'], - baseUrl: providerBaseUrl, - model: parsed.values.model!, - ...(parsed.values['api-version'] - ? { apiVersion: parsed.values['api-version'] } - : {}), - requiresApiKey: readsApiKey, - ...(readsApiKey ? { apiKey } : {}), - }; - const payload = await requestJson( - fetchImpl, - `${daemonBaseUrl}/api/byok/profiles`, - { - method: 'POST', - headers: { Accept: 'application/json', 'Content-Type': 'application/json' }, - body: JSON.stringify(request), - }, - ) as Partial; - const profile = safeProfile(payload.profile); - if (!profile) throw new Error('Open Design daemon returned an invalid profile'); - stdout(useJson - ? `${JSON.stringify({ profile })}\n` - : `Saved ${printProfile(profile)}\n`); - return { exitCode: 0 }; - } - - const id = parsed.positionals[0]; - if (!id || parsed.positionals.length !== 1) { - return fail(`${parsed.command} requires exactly one profile id`); - } - if (parsed.command === 'test') { - const payload = await requestJson( - fetchImpl, - `${daemonBaseUrl}/api/byok/profiles/${encodeURIComponent(id)}/test`, - { method: 'POST', headers: { Accept: 'application/json' } }, - ); - const result = safeConnectionTest(payload); - if (!result) throw new Error('Open Design daemon returned an invalid connection test'); - stdout(useJson - ? `${JSON.stringify(result)}\n` - : `${result.ok ? 'Connection succeeded' : 'Connection failed'} (${result.kind}, ${result.latencyMs} ms)${result.model ? ` using ${result.model}` : ''}.\n`); - return { exitCode: result.ok ? 0 : 1 }; - } - await requestJson( - fetchImpl, - `${daemonBaseUrl}/api/byok/profiles/${encodeURIComponent(id)}`, - { method: 'DELETE', headers: { Accept: 'application/json' } }, - ); - stdout(useJson - ? `${JSON.stringify({ deleted: true, id })}\n` - : `Deleted BYOK profile ${id}.\n`); - return { exitCode: 0 }; - } catch (error) { - return fail(error instanceof Error ? error.message : 'BYOK command failed'); - } -} diff --git a/apps/daemon/tests/byok/credential-routes.test.ts b/apps/daemon/tests/byok/credential-routes.test.ts deleted file mode 100644 index 07bdc8f3b1..0000000000 --- a/apps/daemon/tests/byok/credential-routes.test.ts +++ /dev/null @@ -1,196 +0,0 @@ -import type { Express, RequestHandler, Response } from 'express'; -import { mkdtemp, rm } from 'node:fs/promises'; -import { tmpdir } from 'node:os'; -import path from 'node:path'; -import { afterEach, describe, expect, it, vi } from 'vitest'; - -import { - ByokCredentialService, - type ByokSecretBackend, -} from '../../src/byok/credential-service.js'; -import { registerByokCredentialRoutes } from '../../src/routes/byok-credentials.js'; - -class MemorySecretBackend implements ByokSecretBackend { - readonly kind = 'test-memory'; - readonly secrets = new Map(); - - async available() { return true; } - async set(profileId: string, secret: string) { this.secrets.set(profileId, secret); } - async get(profileId: string) { return this.secrets.get(profileId) ?? null; } - async delete(profileId: string) { return this.secrets.delete(profileId); } -} - -describe('BYOK credential routes', () => { - const roots: string[] = []; - - afterEach(async () => { - await Promise.all(roots.splice(0).map((root) => - rm(root, { recursive: true, force: true }), - )); - }); - - it('keeps raw credentials out of profile responses and tests with the stored secret', async () => { - const dataDir = await mkdtemp(path.join(tmpdir(), 'od-byok-routes-')); - roots.push(dataDir); - const backend = new MemorySecretBackend(); - const credentials = new ByokCredentialService({ dataDir, backend }); - const connectionTest = vi.fn(async (input: { - apiKey: string; - model: string; - }) => ({ - ok: true, - kind: 'success' as const, - latencyMs: 12, - model: input.model, - sample: 'ok', - })); - const routes = createRoutes(credentials, connectionTest); - const secret = 'route-test-secret-must-not-escape'; - - const createResponse = await routes.call('POST', '/api/byok/profiles', { - body: { - label: 'OpenRouter', - protocol: 'openai', - baseUrl: 'https://openrouter.ai/api/v1', - model: 'openrouter/free', - apiKey: secret, - }, - }); - expect(createResponse.statusCode).toBe(201); - const createdText = JSON.stringify(createResponse.body); - expect(createdText).not.toContain(secret); - const created = JSON.parse(createdText) as { profile: { id: string; keyTail: string } }; - expect(created.profile.keyTail).toBe('cape'); - - const listResponse = await routes.call('GET', '/api/byok/profiles'); - expect(listResponse.statusCode).toBe(200); - expect(JSON.stringify(listResponse.body)).not.toContain(secret); - - const testResponse = await routes.call('POST', '/api/byok/profiles/:id/test', { - params: { id: created.profile.id }, - }); - expect(testResponse.statusCode).toBe(200); - const testText = JSON.stringify(testResponse.body); - expect(testText).not.toContain(secret); - expect(JSON.parse(testText)).toMatchObject({ - ok: true, - kind: 'success', - model: 'openrouter/free', - }); - expect(connectionTest).toHaveBeenCalledWith(expect.objectContaining({ - apiKey: secret, - baseUrl: 'https://openrouter.ai/api/v1', - model: 'openrouter/free', - protocol: 'openai', - })); - }); - - it('guards every credential route as loopback-only', async () => { - const dataDir = await mkdtemp(path.join(tmpdir(), 'od-byok-routes-')); - roots.push(dataDir); - const credentials = new ByokCredentialService({ - dataDir, - backend: new MemorySecretBackend(), - }); - const guard: RequestHandler = (_req, res) => { - res.status(403).json({ error: { code: 'FORBIDDEN', message: 'local only' } }); - }; - const routes = createRoutes( - credentials, - vi.fn(), - guard, - ); - - for (const request of [ - { path: '/api/byok/profiles', method: 'GET' as const }, - { path: '/api/byok/profiles', method: 'POST' as const }, - { path: '/api/byok/profiles/:id', method: 'DELETE' as const }, - { path: '/api/byok/profiles/:id/test', method: 'POST' as const }, - ]) { - const response = await routes.call(request.method, request.path, { - params: { id: 'byok-test-profile' }, - body: {}, - }); - expect(response.statusCode, `${request.method} ${request.path}`).toBe(403); - } - }); -}); - -function createRoutes( - credentials: ByokCredentialService, - connectionTest: (...args: any[]) => Promise, - requireLocalDaemonRequest: RequestHandler = (_req, _res, next) => next(), -): { - call( - method: 'GET' | 'POST' | 'DELETE', - path: string, - request?: { body?: unknown; params?: Record }, - ): Promise<{ statusCode: number; body: unknown }>; -} { - const registered = new Map(); - const app = { - get(path: string, ...handlers: RequestHandler[]) { - registered.set(`GET ${path}`, handlers); - }, - post(path: string, ...handlers: RequestHandler[]) { - registered.set(`POST ${path}`, handlers); - }, - delete(path: string, ...handlers: RequestHandler[]) { - registered.set(`DELETE ${path}`, handlers); - }, - } as unknown as Express; - registerByokCredentialRoutes(app, { - byokCredentials: credentials, - connectionTest, - http: { - requireLocalDaemonRequest, - sendApiError( - res: Response, - status: number, - code: string, - message: string, - ) { - return res.status(status).json({ error: { code, message } }); - }, - }, - }); - return { - async call(method, routePath, request = {}) { - const handlers = registered.get(`${method} ${routePath}`); - if (!handlers) throw new Error(`route not registered: ${method} ${routePath}`); - let statusCode = 200; - let body: unknown; - const response = { - status(code: number) { - statusCode = code; - return response; - }, - json(value: unknown) { - body = value; - return response; - }, - end() { - return response; - }, - setHeader() { - return response; - }, - } as unknown as Response; - const req = { - body: request.body, - params: request.params ?? {}, - }; - const dispatch = async (index: number): Promise => { - const handler = handlers[index]; - if (!handler) return; - let nextPromise: Promise | null = null; - await handler(req as never, response, () => { - nextPromise = dispatch(index + 1); - }); - if (nextPromise) await nextPromise; - }; - await dispatch(0); - return { statusCode, body }; - }, - }; -} diff --git a/apps/daemon/tests/byok/credential-service.node-pty-boundary.test.ts b/apps/daemon/tests/byok/credential-service.node-pty-boundary.test.ts deleted file mode 100644 index 41350c279f..0000000000 --- a/apps/daemon/tests/byok/credential-service.node-pty-boundary.test.ts +++ /dev/null @@ -1,66 +0,0 @@ -import { mkdtemp, rm } from 'node:fs/promises'; -import { tmpdir } from 'node:os'; -import path from 'node:path'; - -import { afterEach, describe, expect, it, vi } from 'vitest'; - -describe('BYOK credential node-pty boundary', () => { - const roots: string[] = []; - - afterEach(async () => { - vi.doUnmock('node-pty'); - vi.resetModules(); - await Promise.all(roots.splice(0).map((root) => rm(root, { recursive: true, force: true }))); - }); - - it('keeps non-Keychain credential paths usable when node-pty cannot load', async () => { - vi.resetModules(); - vi.doMock('node-pty', () => { - throw new Error('simulated missing node-pty native addon'); - }); - - const { ByokCredentialService } = await import('../../src/byok/credential-service.js'); - const dataDir = await mkdtemp(path.join(tmpdir(), 'od-byok-node-pty-boundary-')); - roots.push(dataDir); - const secrets = new Map(); - const service = new ByokCredentialService({ - dataDir, - backend: { - kind: 'test-memory', - available: async () => true, - set: async (profileId: string, secret: string) => { - secrets.set(profileId, secret); - }, - get: async (profileId: string) => secrets.get(profileId) ?? null, - delete: async (profileId: string) => secrets.delete(profileId), - }, - }); - - await expect(service.status()).resolves.toEqual({ available: true, backend: 'test-memory' }); - await expect(service.upsert({ - id: 'byok-node-pty-boundary', - label: 'Boundary test', - protocol: 'openai', - baseUrl: 'https://example.test/v1', - model: 'test-model', - apiKey: 'test-secret', - })).resolves.toMatchObject({ - configured: true, - id: 'byok-node-pty-boundary', - }); - }); - - it('fails only the macOS Keychain write with an attributable storage error', async () => { - vi.resetModules(); - vi.doMock('node-pty', () => { - throw new Error('simulated missing node-pty native addon'); - }); - - const { createPlatformByokSecretBackend } = await import('../../src/byok/credential-service.js'); - const backend = createPlatformByokSecretBackend('darwin'); - - await expect(backend.set('byok-node-pty-boundary', 'test-secret')).rejects.toThrow( - /secure credential storage is unavailable.*native PTY helper/iu, - ); - }); -}); diff --git a/apps/daemon/tests/byok/credential-service.test.ts b/apps/daemon/tests/byok/credential-service.test.ts deleted file mode 100644 index 2cd0674ca7..0000000000 --- a/apps/daemon/tests/byok/credential-service.test.ts +++ /dev/null @@ -1,292 +0,0 @@ -import { mkdir, mkdtemp, readFile, rm, writeFile } from 'node:fs/promises'; -import { tmpdir } from 'node:os'; -import path from 'node:path'; -import { afterEach, describe, expect, it } from 'vitest'; - -import { - ByokCredentialService, - createPlatformByokSecretBackend, - type ByokSecretBackend, -} from '../../src/byok/credential-service.js'; - -class MemorySecretBackend implements ByokSecretBackend { - readonly kind = 'test-memory'; - readonly secrets = new Map(); - - async available() { - return true; - } - - async set(profileId: string, secret: string) { - this.secrets.set(profileId, secret); - } - - async get(profileId: string) { - return this.secrets.get(profileId) ?? null; - } - - async delete(profileId: string) { - return this.secrets.delete(profileId); - } -} - -describe('BYOK credential service', () => { - const roots: string[] = []; - - afterEach(async () => { - await Promise.all(roots.splice(0).map((root) => rm(root, { recursive: true, force: true }))); - }); - - it('keeps API keys in the secret backend and only non-secret profile metadata under OD_DATA_DIR', async () => { - const dataDir = await mkdtemp(path.join(tmpdir(), 'od-byok-credentials-')); - roots.push(dataDir); - const backend = new MemorySecretBackend(); - const service = new ByokCredentialService({ dataDir, backend }); - const apiKey = 'test-openrouter-secret-not-for-disk'; - - const profile = await service.upsert({ - label: 'OpenRouter', - protocol: 'openai', - baseUrl: 'https://openrouter.ai/api/v1', - model: 'openrouter/free', - apiKey, - }); - - expect(profile).toMatchObject({ - label: 'OpenRouter', - protocol: 'openai', - configured: true, - keyTail: 'disk', - }); - expect(JSON.stringify(profile)).not.toContain(apiKey); - const metadata = await readFile(path.join(dataDir, 'byok', 'profiles.json'), 'utf8'); - expect(metadata).not.toContain(apiKey); - expect(metadata).not.toContain('keyTail'); - expect(backend.secrets.get(profile.id)).toBe(apiKey); - - const resolved = await service.resolve(profile.id); - expect(resolved?.apiKey).toBe(apiKey); - expect(JSON.stringify(await service.list())).not.toContain(apiKey); - - expect(await service.delete(profile.id)).toBe(true); - expect(await service.resolve(profile.id)).toBeNull(); - }); - - it('fails closed when no secure backend is available', async () => { - const dataDir = await mkdtemp(path.join(tmpdir(), 'od-byok-credentials-')); - roots.push(dataDir); - const backend = new MemorySecretBackend(); - backend.available = async () => false; - const service = new ByokCredentialService({ dataDir, backend }); - - await expect(service.upsert({ - label: 'OpenRouter', - protocol: 'openai', - baseUrl: 'https://openrouter.ai/api/v1', - model: 'openrouter/free', - apiKey: 'test-secret', - })).rejects.toThrow(/secure credential storage is unavailable/i); - }); - - it('fails closed when Windows has no supported secure credential backend', async () => { - const dataDir = await mkdtemp(path.join(tmpdir(), 'od-byok-credentials-')); - roots.push(dataDir); - const backend = createPlatformByokSecretBackend('win32', dataDir); - - expect(backend.kind).toBe('unavailable-win32'); - await expect(backend.available()).resolves.toBe(false); - }); - - it('deletes a retired Windows DPAPI blob with its profile metadata', async () => { - const dataDir = await mkdtemp(path.join(tmpdir(), 'od-byok-credentials-')); - roots.push(dataDir); - const profileId = 'byok-retired-windows'; - const byokDir = path.join(dataDir, 'byok'); - const secretsDir = path.join(byokDir, 'secrets'); - const secretPath = path.join(secretsDir, `${profileId}.bin`); - await mkdir(secretsDir, { recursive: true }); - await writeFile(path.join(byokDir, 'profiles.json'), JSON.stringify({ - version: 1, - profiles: [{ - id: profileId, - label: 'Retired Windows profile', - protocol: 'openai', - baseUrl: 'https://example.test/v1', - model: 'model', - requiresApiKey: true, - createdAt: 1, - updatedAt: 1, - }], - })); - await writeFile(secretPath, 'retired-dpapi-blob'); - const service = new ByokCredentialService({ - dataDir, - backend: createPlatformByokSecretBackend('win32', dataDir), - }); - - await expect(service.delete(profileId)).resolves.toBe(true); - await expect(service.get(profileId)).resolves.toBeNull(); - await expect(readFile(secretPath)).rejects.toMatchObject({ code: 'ENOENT' }); - }); - - it('serializes concurrent metadata mutations so profiles cannot overwrite each other', async () => { - const dataDir = await mkdtemp(path.join(tmpdir(), 'od-byok-credentials-')); - roots.push(dataDir); - const service = new ByokCredentialService({ - dataDir, - backend: new MemorySecretBackend(), - }); - - await Promise.all([ - service.upsert({ - id: 'byok-provider-one', - label: 'Provider One', - protocol: 'openai', - baseUrl: 'https://one.example/v1', - model: 'model-one', - apiKey: 'test-secret-one', - }), - service.upsert({ - id: 'byok-provider-two', - label: 'Provider Two', - protocol: 'openai', - baseUrl: 'https://two.example/v1', - model: 'model-two', - apiKey: 'test-secret-two', - }), - ]); - - expect((await service.list()).map((profile) => profile.id).sort()).toEqual([ - 'byok-provider-one', - 'byok-provider-two', - ]); - }); - - it('removes a newly created secret when metadata persistence fails', async () => { - const dataDir = await mkdtemp(path.join(tmpdir(), 'od-byok-credentials-')); - roots.push(dataDir); - const backend = new MemorySecretBackend(); - const service = new ByokCredentialService({ - dataDir, - backend, - persistMetadata: async () => { - throw new Error('metadata unavailable'); - }, - }); - - await expect(service.upsert({ - id: 'byok-create-failure', - label: 'Create failure', - protocol: 'openai', - baseUrl: 'https://example.test/v1', - model: 'model', - apiKey: 'new-secret', - })).rejects.toThrow('metadata unavailable'); - - expect(backend.secrets.has('byok-create-failure')).toBe(false); - }); - - it('restores the previous secret when profile metadata update fails', async () => { - const dataDir = await mkdtemp(path.join(tmpdir(), 'od-byok-credentials-')); - roots.push(dataDir); - const backend = new MemorySecretBackend(); - let rejectMetadata = false; - const service = new ByokCredentialService({ - dataDir, - backend, - persistMetadata: async (metadataPath, document) => { - if (rejectMetadata) throw new Error('metadata unavailable'); - await writeFile( - metadataPath, - `${JSON.stringify(document)}\n`, - 'utf8', - ); - }, - }); - await mkdir(path.join(dataDir, 'byok'), { recursive: true }); - await service.upsert({ - id: 'byok-update-failure', - label: 'Before', - protocol: 'openai', - baseUrl: 'https://example.test/v1', - model: 'model-before', - apiKey: 'old-secret', - }); - rejectMetadata = true; - - await expect(service.upsert({ - id: 'byok-update-failure', - label: 'After', - protocol: 'openai', - baseUrl: 'https://example.test/v1', - model: 'model-after', - apiKey: 'new-secret', - })).rejects.toThrow('metadata unavailable'); - - expect(backend.secrets.get('byok-update-failure')).toBe('old-secret'); - expect(await service.get('byok-update-failure')).toMatchObject({ - label: 'Before', - model: 'model-before', - }); - }); - - it('restores a deleted secret when profile metadata deletion fails', async () => { - const dataDir = await mkdtemp(path.join(tmpdir(), 'od-byok-credentials-')); - roots.push(dataDir); - const backend = new MemorySecretBackend(); - let rejectMetadata = false; - const service = new ByokCredentialService({ - dataDir, - backend, - persistMetadata: async (metadataPath, document) => { - if (rejectMetadata) throw new Error('metadata unavailable'); - await writeFile( - metadataPath, - `${JSON.stringify(document)}\n`, - 'utf8', - ); - }, - }); - await mkdir(path.join(dataDir, 'byok'), { recursive: true }); - await service.upsert({ - id: 'byok-delete-failure', - label: 'Delete failure', - protocol: 'openai', - baseUrl: 'https://example.test/v1', - model: 'model', - apiKey: 'keep-secret', - }); - rejectMetadata = true; - - await expect(service.delete('byok-delete-failure')).rejects.toThrow( - 'metadata unavailable', - ); - - expect(backend.secrets.get('byok-delete-failure')).toBe('keep-secret'); - expect(await service.get('byok-delete-failure')).toMatchObject({ - id: 'byok-delete-failure', - configured: true, - }); - }); - - it.each([ - 'https://user:password@example.test/v1', - 'https://example.test/v1?api_key=plaintext', - 'https://example.test/v1#plaintext', - ])('rejects credential-bearing provider metadata before persistence: %s', async (baseUrl) => { - const dataDir = await mkdtemp(path.join(tmpdir(), 'od-byok-credentials-')); - roots.push(dataDir); - const service = new ByokCredentialService({ - dataDir, - backend: new MemorySecretBackend(), - }); - - await expect(service.upsert({ - label: 'Unsafe', - protocol: 'openai', - baseUrl, - model: 'model', - apiKey: 'test-secret', - })).rejects.toThrow(/without credentials, query, or fragment/i); - }); -}); diff --git a/apps/daemon/tests/byok/run-input-boundary.test.ts b/apps/daemon/tests/byok/run-input-boundary.test.ts index 980af18197..bf5070e9a3 100644 --- a/apps/daemon/tests/byok/run-input-boundary.test.ts +++ b/apps/daemon/tests/byok/run-input-boundary.test.ts @@ -1,66 +1,39 @@ -import { describe, expect, it, vi } from 'vitest'; +import { describe, expect, it } from 'vitest'; import { - __forTestByokRunInputError, + __forTestHasCompleteByokOpenCodeConfig, __forTestWithoutSensitiveRunInput, } from '../../src/routes/runs.js'; describe('BYOK run input boundary', () => { - it('rejects raw credentials before consulting profile metadata', async () => { - const has = vi.fn(async () => true); - const error = await __forTestByokRunInputError({ + it('accepts a complete run-scoped Local BYOK provider', () => { + expect(__forTestHasCompleteByokOpenCodeConfig({ agentId: 'byok-opencode', - byokProfileId: 'byok-openrouter', + model: 'gpt-5.4-mini', byokProvider: { protocol: 'openai', - apiKey: 'raw-secret', + apiKey: 'local-only-secret', + baseUrl: 'https://api.openai.com/v1', }, - }, { has }); - - expect(error).toMatch(/raw BYOK credentials are not accepted/iu); - expect(has).not.toHaveBeenCalled(); + })).toBe(true); }); - it('accepts only an existing non-secret profile reference', async () => { - const has = vi.fn(async (profileId: string) => profileId === 'byok-openrouter'); - - await expect(__forTestByokRunInputError({ + it('rejects a BYOK run without a run-scoped provider', () => { + expect(__forTestHasCompleteByokOpenCodeConfig({ agentId: 'byok-opencode', - byokProfileId: 'byok-openrouter', - }, { has })).resolves.toBeNull(); - await expect(__forTestByokRunInputError({ - agentId: 'byok-opencode', - byokProfileId: 'byok-missing', - }, { has })).resolves.toMatch(/secure credential profile/iu); + })).toBe(false); }); - it('rejects credential-shaped fields nested in BYOK plugin inputs', async () => { - const has = vi.fn(async () => true); - const error = await __forTestByokRunInputError({ + it('accepts a keyless run-scoped provider when the protocol permits it', () => { + expect(__forTestHasCompleteByokOpenCodeConfig({ agentId: 'byok-opencode', - byokProfileId: 'byok-openrouter', - pluginInputs: { - nested: [{ apiKey: 'raw-secret' }], + model: 'local-model', + byokProvider: { + protocol: 'openai', + baseUrl: 'http://127.0.0.1:1234/v1', + requiresApiKey: false, }, - }, { has }); - - expect(error).toMatch(/raw BYOK credentials are not accepted/iu); - expect(has).not.toHaveBeenCalled(); - }); - - it('fails closed when BYOK structured input exceeds the scan depth', async () => { - const has = vi.fn(async () => true); - let nested: Record = { apiKey: 'raw-secret' }; - for (let depth = 0; depth < 24; depth += 1) nested = { nested }; - - const error = await __forTestByokRunInputError({ - agentId: 'byok-opencode', - byokProfileId: 'byok-openrouter', - pluginInputs: nested, - }, { has }); - - expect(error).toMatch(/raw BYOK credentials are not accepted/iu); - expect(has).not.toHaveBeenCalled(); + })).toBe(true); }); it('removes every credential-bearing compatibility field before persistence', () => { @@ -75,7 +48,6 @@ describe('BYOK run input boundary', () => { expect(sanitized).toEqual({ agentId: 'byok-opencode', - byokProfileId: 'byok-openrouter', message: 'Create a site', }); expect(JSON.stringify(sanitized)).not.toContain('secret'); diff --git a/apps/daemon/tests/chat-route.test.ts b/apps/daemon/tests/chat-route.test.ts index 9ba3d06a15..3f7f9ec0e5 100644 --- a/apps/daemon/tests/chat-route.test.ts +++ b/apps/daemon/tests/chat-route.test.ts @@ -36,10 +36,6 @@ import { readAppConfig, writeAppConfig } from '../src/app-config.js'; import { readMemoryConfig, writeMemoryConfig } from '../src/memory.js'; import { upsertMessage } from '../src/db.js'; import { renderCodexImagegenOverride } from '../src/prompts/system.js'; -import { - ByokCredentialService, - type ByokSecretBackend, -} from '../src/byok/credential-service.js'; const FAKE_VELA_FIXTURE = resolve(process.cwd(), 'tests', 'fixtures', 'fake-vela.mjs'); @@ -144,41 +140,9 @@ describe('/api/chat', () => { extraction: null, }); } - const byokDataDir = mkdtempSync(join(tmpdir(), 'od-chat-route-byok-')); - tempDirs.push(byokDataDir); - const byokSecrets = new Map(); - const byokBackend: ByokSecretBackend = { - kind: 'test-memory', - async available() { return true; }, - async set(profileId, secret) { byokSecrets.set(profileId, secret); }, - async get(profileId) { return byokSecrets.get(profileId) ?? null; }, - async delete(profileId) { return byokSecrets.delete(profileId); }, - }; - const byokCredentialService = new ByokCredentialService({ - dataDir: byokDataDir, - backend: byokBackend, - }); - await byokCredentialService.upsert({ - id: 'byok-chat-route-keyful', - label: 'Chat route keyful fixture', - protocol: 'senseaudio', - apiKey: 'sk-test-byok', - baseUrl: 'https://api.senseaudio.cn', - model: 'deepseek-v4-flash', - requiresApiKey: true, - }); - await byokCredentialService.upsert({ - id: 'byok-chat-route-keyless', - label: 'Chat route keyless fixture', - protocol: 'openai', - baseUrl: 'http://127.0.0.1:8000/v1', - model: 'model', - requiresApiKey: false, - }); const started = await startServer({ port: 0, returnServer: true, - byokCredentialService, }) as { url: string; server: http.Server; @@ -493,7 +457,13 @@ process.stdin.on('end', () => { projectId, message: 'hello', model: 'deepseek-v4-flash', - byokProfileId: 'byok-chat-route-keyful', + byokProvider: { + protocol: 'senseaudio', + apiKey: 'sk-test-byok', + baseUrl: 'https://api.senseaudio.cn', + model: 'deepseek-v4-flash', + requiresApiKey: true, + }, }), }); const body = await response.text(); @@ -580,7 +550,13 @@ process.stdin.on('end', () => { projectId, message: 'hello', model: 'model', - byokProfileId: 'byok-chat-route-keyless', + byokProvider: { + protocol: 'openai', + apiKey: '', + baseUrl: 'http://127.0.0.1:8000/v1', + model: 'model', + requiresApiKey: false, + }, }), }); const body = await response.text(); @@ -619,7 +595,7 @@ process.stdin.on('end', () => { ); }); - it('rejects forged BYOK provider config for other local runtimes', async () => { + it('does not pass BYOK provider config to other local runtimes', async () => { if (!process.env.OD_DATA_DIR) { throw new Error('OD_DATA_DIR is required for BYOK OpenCode config tests'); } @@ -669,10 +645,12 @@ process.stdin.on('end', () => { }); const body = await response.text(); - expect(response.status).toBe(400); - expect(body).toContain('Raw BYOK credentials are not accepted'); - expect(existsSync(keyFile)).toBe(false); - expect(existsSync(envFile)).toBe(false); + expect(response.ok).toBe(true); + expect(body).toContain('opencode-ok'); + expect(await fsp.readFile(keyFile, 'utf8')).toBe(''); + const rawConfig = await fsp.readFile(envFile, 'utf8'); + expect(rawConfig).not.toContain('open-design-byok'); + expect(rawConfig).not.toContain('sk-test-byok'); }, ); }); diff --git a/apps/daemon/tests/headless-runs.test.ts b/apps/daemon/tests/headless-runs.test.ts index a0d7b7836b..b2228690b9 100644 --- a/apps/daemon/tests/headless-runs.test.ts +++ b/apps/daemon/tests/headless-runs.test.ts @@ -30,6 +30,14 @@ describe('POST /api/runs headless fallbacks', () => { else process.env.OD_AGENT_HOME = oldAgentHome; }); + it('does not expose the retired secure BYOK profile API', async () => { + started = await startTestServer(); + + const response = await fetch(`${started.url}/api/byok/profiles`); + + expect(response.status).toBe(404); + }); + it('rejects incomplete BYOK OpenCode config before creating a run', async () => { started = await startTestServer(); const incompleteConfigs = [ @@ -81,7 +89,7 @@ describe('POST /api/runs headless fallbacks', () => { error: { code: 'VALIDATION_FAILED', message: expect.stringMatching( - /secure credential profile|raw BYOK credentials are not accepted/iu, + /complete provider configuration/iu, ), }, }); diff --git a/apps/daemon/tests/mcp-brief-app.test.ts b/apps/daemon/tests/mcp-brief-app.test.ts index eed67fb325..85dd2084de 100644 --- a/apps/daemon/tests/mcp-brief-app.test.ts +++ b/apps/daemon/tests/mcp-brief-app.test.ts @@ -707,7 +707,7 @@ describe('local Open Design MCP brief app', () => { expect(userFacingCopy).toContain('Open Design Cloud'); expect(userFacingCopy).toContain('Local Codex'); - expect(userFacingCopy).toContain('Secure BYOK'); + expect(userFacingCopy).not.toContain('Secure BYOK'); expect(userFacingCopy).not.toMatch(/\b(?:Vela|AMR)\b/u); expect(userFacingCopy).not.toMatch(/agent\s*:\s*["']?[a-z]/iu); }); diff --git a/apps/daemon/tests/mcp-observability.test.ts b/apps/daemon/tests/mcp-observability.test.ts index a90e2f9472..6cee9f894f 100644 --- a/apps/daemon/tests/mcp-observability.test.ts +++ b/apps/daemon/tests/mcp-observability.test.ts @@ -447,6 +447,35 @@ describe('local MCP plugin observability contract', () => { ), ).toBe(false); + const invalidRequest = await handleMcpToolCall( + 'http://127.0.0.1:17456', + 'start_run', + { + project: 'Demo', + prompt: 'Create a launch page', + requestId: 'od-mscwn4y2-tlnx02dig7', + pluginWorkflowId: '018f6f2e-4444-7444-8444-444444444444', + }, + { + pluginAttribution: { + pluginWorkflowId: '018f6f2e-4444-7444-8444-444444444444', + context: pluginContext, + }, + }, + ); + expect(invalidRequest).toMatchObject({ isError: true }); + expect(invalidRequest.content[0]?.text).toContain( + 'requestId must be a canonical UUID or ULID', + ); + expect(invalidRequest.content[0]?.text).not.toContain( + 'pluginWorkflowId must be a canonical UUID or ULID', + ); + expect( + fetchMock.mock.calls.some( + ([url]) => String(url).endsWith('/api/runs'), + ), + ).toBe(false); + const requestId = '018f6f2e-5555-7555-8555-555555555555'; const result = await handleMcpToolCall( 'http://127.0.0.1:17456', diff --git a/apps/daemon/tests/mcp-runs.test.ts b/apps/daemon/tests/mcp-runs.test.ts index 26d7570f9e..7e99be7a4f 100644 --- a/apps/daemon/tests/mcp-runs.test.ts +++ b/apps/daemon/tests/mcp-runs.test.ts @@ -1,7 +1,7 @@ import { afterEach, describe, expect, it, vi } from 'vitest'; import { buildProjectRawFileUrl } from '@open-design/contracts'; -import { handleMcpToolCall } from '../src/mcp.js'; +import { handleMcpToolCall, localMcpToolDefinitions } from '../src/mcp.js'; const originalFetch = globalThis.fetch; @@ -32,74 +32,11 @@ describe('public MCP discovery + generation tools', () => { expect(JSON.parse(firstText(result))).toEqual({ skills: [{ id: 'deck', name: 'Deck' }] }); }); - it('list_byok_profiles returns daemon-owned non-secret profile metadata', async () => { - const fetchMock = vi.fn(async (url: string) => { - expect(url).toBe('http://127.0.0.1:17456/api/byok/profiles'); - return new Response(JSON.stringify({ - available: true, - backend: 'macos-keychain', - profiles: [{ - id: 'byok-openrouter-1', - label: 'OpenRouter', - protocol: 'openai', - baseUrl: 'https://openrouter.ai/api/v1', - model: 'openai/gpt-5.4-mini', - requiresApiKey: true, - configured: true, - keyTail: '1234', - apiKey: 'must-never-cross-mcp', - }], - }), { status: 200 }); - }); - vi.stubGlobal('fetch', fetchMock); - - const result = await handleMcpToolCall( - 'http://127.0.0.1:17456', - 'list_byok_profiles', - {}, - ); - const payload = JSON.parse(firstText(result)); - expect(payload.profiles[0]).toMatchObject({ - id: 'byok-openrouter-1', - configured: true, - keyTail: '1234', - }); - expect(JSON.stringify(payload)).not.toContain('apiKey'); - expect(JSON.stringify(payload)).not.toContain('must-never-cross-mcp'); - }); - - it('start_run selects secure Local BYOK through a profile reference', async () => { - const fetchMock = vi.fn(async (url: string, init?: RequestInit) => { - if (url.endsWith('/api/projects')) { - return new Response(JSON.stringify({ - projects: [{ id: 'project-1', name: 'Demo' }], - }), { status: 200 }); - } - return new Response(JSON.stringify({ runId: 'run-byok' }), { status: 200 }); - }); - vi.stubGlobal('fetch', fetchMock); - - const result = await handleMcpToolCall( - 'http://127.0.0.1:17456', - 'start_run', - { - project: '11111111-1111-4111-8111-111111111111', - prompt: 'Create a launch page', - byokProfile: 'byok-openrouter-1', - }, - ); - - const runsCall = fetchMock.mock.calls.find( - ([url, init]) => - String(url).endsWith('/api/runs') - && (init as RequestInit)?.method === 'POST', - ); - expect(JSON.parse(String(runsCall?.[1]?.body))).toMatchObject({ - projectId: '11111111-1111-4111-8111-111111111111', - agentId: 'byok-opencode', - byokProfileId: 'byok-openrouter-1', - }); - expect(JSON.parse(firstText(result))).toMatchObject({ runId: 'run-byok' }); + it('does not expose secure BYOK profiles or a BYOK start_run parameter', () => { + const definitions = localMcpToolDefinitions(); + expect(definitions.some((tool) => tool.name === 'list_byok_profiles')).toBe(false); + const startRun = definitions.find((tool) => tool.name === 'start_run'); + expect(startRun?.inputSchema.properties).not.toHaveProperty('byokProfile'); }); it('start_run rejects raw credential fields at any nesting depth', async () => { @@ -113,7 +50,6 @@ describe('public MCP discovery + generation tools', () => { 'start_run', { prompt: 'Create a launch page', - byokProfile: 'byok-openrouter-1', inputs: { provider: { api_key: 'must-never-cross-mcp' } }, }, ); diff --git a/apps/daemon/tests/tools-byok-cli.test.ts b/apps/daemon/tests/tools-byok-cli.test.ts deleted file mode 100644 index c5d622c12b..0000000000 --- a/apps/daemon/tests/tools-byok-cli.test.ts +++ /dev/null @@ -1,206 +0,0 @@ -import { describe, expect, it, vi } from 'vitest'; - -import { runByokToolCli } from '../src/tools-byok-cli.js'; - -const BASE_URL = 'http://127.0.0.1:17456'; -const PROFILE = { - id: 'byok-openrouter-1', - label: 'OpenRouter', - protocol: 'openai', - baseUrl: 'https://openrouter.ai/api/v1', - model: 'openai/gpt-5.4-mini', - requiresApiKey: true, - configured: true, - keyTail: 'tkey', - createdAt: 1, - updatedAt: 2, -}; - -function dependencies(fetchImpl: typeof fetch, stdin = '') { - const stdout: string[] = []; - const stderr: string[] = []; - return { - deps: { - fetchImpl, - readStdin: vi.fn(async () => stdin), - resolveDaemonUrlImpl: vi.fn(async () => BASE_URL), - stdout: (text: string) => stdout.push(text), - stderr: (text: string) => stderr.push(text), - }, - stdout, - stderr, - }; -} - -describe('secure BYOK CLI', () => { - it('rejects API keys in argv before making any daemon request', async () => { - const fetchMock = vi.fn(); - const { deps, stderr } = dependencies(fetchMock, 'not-used'); - - const result = await runByokToolCli([ - 'save', - '--label', 'OpenRouter', - '--protocol', 'openai', - '--base-url', 'https://openrouter.ai/api/v1', - '--model', 'openai/gpt-5.4-mini', - '--api-key', 'argv-secret', - ], deps); - - expect(result.exitCode).toBe(1); - expect(fetchMock).not.toHaveBeenCalled(); - expect(stderr.join('')).toContain('--api-key-stdin'); - expect(stderr.join('')).not.toContain('argv-secret'); - }); - - it('rejects a non-loopback daemon before reading stdin', async () => { - const fetchMock = vi.fn(); - const { deps, stderr } = dependencies(fetchMock, 'must-not-be-read'); - deps.resolveDaemonUrlImpl = vi.fn(async () => 'https://attacker.example'); - - const result = await runByokToolCli([ - 'save', - '--label', 'OpenRouter', - '--protocol', 'openai', - '--base-url', 'https://openrouter.ai/api/v1', - '--model', 'openai/gpt-5.4-mini', - '--api-key-stdin', - ], deps); - - expect(result.exitCode).toBe(1); - expect(deps.readStdin).not.toHaveBeenCalled(); - expect(fetchMock).not.toHaveBeenCalled(); - expect(stderr.join('')).toContain('loopback'); - }); - - it('reads the key from stdin and prints only the safe profile', async () => { - const secret = 'test-only-secret'; - const fetchMock = vi.fn(async (url, init) => { - expect(url).toBe(`${BASE_URL}/api/byok/profiles`); - expect(JSON.parse(String(init?.body)).apiKey).toBe(secret); - return new Response(JSON.stringify({ profile: PROFILE }), { status: 201 }); - }); - const { deps, stdout, stderr } = dependencies(fetchMock, `${secret}\n`); - - const result = await runByokToolCli([ - 'save', - '--label', 'OpenRouter', - '--protocol', 'openai', - '--base-url', 'https://openrouter.ai/api/v1', - '--model', 'openai/gpt-5.4-mini', - '--api-key-stdin', - '--json', - ], deps); - - expect(result.exitCode).toBe(0); - expect(JSON.parse(stdout.join(''))).toEqual({ profile: PROFILE }); - expect(`${stdout.join('')}${stderr.join('')}`).not.toContain(secret); - }); - - it('saves an explicit keyless profile without reading stdin', async () => { - const keylessProfile = { - ...PROFILE, - id: 'byok-ollama-local', - label: 'Local Ollama', - baseUrl: 'http://127.0.0.1:11434', - model: 'llama3.2', - protocol: 'ollama', - requiresApiKey: false, - configured: true, - keyTail: undefined, - }; - const fetchMock = vi.fn(async (_url, init) => { - expect(JSON.parse(String(init?.body))).toEqual({ - label: 'Local Ollama', - protocol: 'ollama', - baseUrl: 'http://127.0.0.1:11434', - model: 'llama3.2', - requiresApiKey: false, - }); - return new Response(JSON.stringify({ profile: keylessProfile }), { - status: 201, - }); - }); - const { deps, stdout } = dependencies(fetchMock, 'must-not-be-read'); - - const result = await runByokToolCli([ - 'save', - '--label', 'Local Ollama', - '--protocol', 'ollama', - '--base-url', 'http://127.0.0.1:11434', - '--model', 'llama3.2', - '--no-api-key', - '--json', - ], deps); - - expect(result.exitCode).toBe(0); - expect(deps.readStdin).not.toHaveBeenCalled(); - expect(JSON.parse(stdout.join(''))).toEqual({ profile: keylessProfile }); - }); - - it('rejects conflicting or missing credential mode flags', async () => { - const fetchMock = vi.fn(); - const shared = [ - 'save', - '--label', 'Local Ollama', - '--protocol', 'ollama', - '--base-url', 'http://127.0.0.1:11434', - '--model', 'llama3.2', - ]; - - const conflicting = dependencies(fetchMock, 'must-not-be-read'); - expect((await runByokToolCli([ - ...shared, - '--api-key-stdin', - '--no-api-key', - ], conflicting.deps)).exitCode).toBe(1); - expect(conflicting.stderr.join('')).toContain('exactly one'); - expect(conflicting.deps.readStdin).not.toHaveBeenCalled(); - - const missing = dependencies(fetchMock, 'must-not-be-read'); - expect((await runByokToolCli(shared, missing.deps)).exitCode).toBe(1); - expect(missing.stderr.join('')).toContain('exactly one'); - expect(missing.deps.readStdin).not.toHaveBeenCalled(); - expect(fetchMock).not.toHaveBeenCalled(); - }); - - it('lists, tests, and deletes a profile without exposing extra fields', async () => { - const fetchMock = vi.fn(async (url, init) => { - if (String(url).endsWith('/test')) { - return new Response(JSON.stringify({ - ok: true, - kind: 'success', - latencyMs: 42, - model: PROFILE.model, - sample: 'must-be-dropped', - }), { status: 200 }); - } - if (init?.method === 'DELETE') return new Response(null, { status: 204 }); - return new Response(JSON.stringify({ - available: true, - backend: 'macos-keychain', - profiles: [{ ...PROFILE, apiKey: 'must-be-dropped' }], - }), { status: 200 }); - }); - - const listIo = dependencies(fetchMock); - expect((await runByokToolCli(['list', '--json'], listIo.deps)).exitCode).toBe(0); - expect(JSON.parse(listIo.stdout.join('')).profiles).toEqual([PROFILE]); - expect(listIo.stdout.join('')).not.toContain('must-be-dropped'); - - const testIo = dependencies(fetchMock); - expect((await runByokToolCli(['test', PROFILE.id, '--json'], testIo.deps)).exitCode).toBe(0); - expect(JSON.parse(testIo.stdout.join(''))).toEqual({ - ok: true, - kind: 'success', - latencyMs: 42, - model: PROFILE.model, - }); - - const deleteIo = dependencies(fetchMock); - expect((await runByokToolCli(['delete', PROFILE.id, '--json'], deleteIo.deps)).exitCode).toBe(0); - expect(JSON.parse(deleteIo.stdout.join(''))).toEqual({ - deleted: true, - id: PROFILE.id, - }); - }); -}); diff --git a/apps/web/src/App.tsx b/apps/web/src/App.tsx index 0e16dc21a3..b5b7c00078 100644 --- a/apps/web/src/App.tsx +++ b/apps/web/src/App.tsx @@ -77,19 +77,14 @@ import { AMR_LOGIN_STATUS_EVENT } from './components/amrLoginPolling'; import { navigate, useRoute } from './router'; import { fetchDaemonConfig, - fetchByokCredentialProfilesFromDaemon, DEFAULT_PET, fetchMediaProvidersFromDaemon, hasAnyConfiguredProvider, fetchComposioConfigFromDaemon, - legacyByokMigrationErrorPresentation, loadConfig, - migrateLegacyByokCredentialsToDaemon, mergeDaemonConfig, - mergeByokCredentialProfiles, mergeDaemonMediaProviders, saveConfig, - persistByokCredentialProfileToDaemon, shouldSyncLocalMediaProvidersToDaemon, syncComposioConfigToDaemon, syncConfigToDaemon, @@ -454,8 +449,6 @@ function AppInner() { // effect while the project actually stayed in the managed root. const [workingDirError, setWorkingDirError] = useState(null); const [projectOpenError, setProjectOpenError] = useState(null); - const [legacyByokMigrationError, setLegacyByokMigrationError] = - useState(null); const [settingsWelcome, setSettingsWelcome] = useState(false); const [settingsInitialSection, setSettingsInitialSection] = useState('execution'); const [settingsHighlight, setSettingsHighlight] = useState(null); @@ -1007,21 +1000,6 @@ function AppInner() { setAppVersionInfo(info); }); - const legacyByokMigration = - await migrateLegacyByokCredentialsToDaemon( - latestPersistedConfigRef.current, - ); - if (cancelled) return; - setLegacyByokMigrationError( - legacyByokMigration.status === 'failed' - ? legacyByokMigration.error - : null, - ); - const migrationBaseConfig = legacyByokMigration.config; - if (legacyByokMigration.status === 'migrated') { - latestPersistedConfigRef.current = migrationBaseConfig; - } - // Daemon-persisted config + composio config + media provider config land // together so the welcome-modal decision and daemon-backed settings // apply in one merge, avoiding a flash where local-only state is shown @@ -1030,14 +1008,10 @@ function AppInner() { fetchDaemonConfig(), fetchComposioConfigFromDaemon(), fetchMediaProvidersFromDaemon(), - migrationBaseConfig.byokProfileId - ? fetchByokCredentialProfilesFromDaemon() - : Promise.resolve(null), ]).then(([ daemonConfig, daemonComposioConfig, daemonMediaProvidersResult, - byokCredentialProfiles, ]) => { if (cancelled) return; const daemonMediaProvidersLoaded = @@ -1059,23 +1033,18 @@ function AppInner() { baseConfig.mediaProviders, daemonMediaProvidersLoaded, ); - const next = mergeByokCredentialProfiles( - mergeDaemonMediaProviders( - clearStaleAmrModelChoiceOnProfileChange( - baseConfig, - mergeDaemonConfig(baseConfig, daemonConfig), - ), - daemonMediaProvidersLoaded, + const next = mergeDaemonMediaProviders( + clearStaleAmrModelChoiceOnProfileChange( + baseConfig, + mergeDaemonConfig(baseConfig, daemonConfig), ), - byokCredentialProfiles, + daemonMediaProvidersLoaded, ); const hasLocalComposioKey = Boolean(next.composio?.apiKey?.trim()); if (!hasLocalComposioKey && daemonComposioConfig) { next.composio = daemonComposioConfig; } - if (legacyByokMigration.status !== 'failed') { - saveConfig(next); - } + saveConfig(next); if ( daemonMediaProvidersResult.status === 'ok' && migratedLocalMediaProviders @@ -2562,7 +2531,6 @@ function AppInner() { onApiProtocolChange={handleApiProtocolChange} onApiModelChange={handleApiModelChange} onConfigPersist={handleConfigPersist} - onPersistByokCredential={persistByokCredentialProfileToDaemon} daemonAppConfigReady={daemonAppConfigReady} onSilentUpdatePreferenceChange={handleSilentUpdatePreferenceChange} onSkillsRefresh={refreshSkills} @@ -2630,12 +2598,6 @@ function AppInner() { /> ); } - const legacyByokMigrationErrorView = legacyByokMigrationError - ? legacyByokMigrationErrorPresentation( - legacyByokMigrationError, - t('settings.autosaveError'), - ) - : null; return ( <>
{ // Closing the dialog is the canonical "I'm done" gesture // now that there is no global Save button. We mark @@ -2746,21 +2707,6 @@ function AppInner() { onDismiss={() => setProjectOpenError(null)} /> ) : null} - {legacyByokMigrationErrorView ? ( - { - setLegacyByokMigrationError(null); - openSettings('execution'); - }} - role="alert" - tone="error" - ttlMs={0} - onDismiss={() => setLegacyByokMigrationError(null)} - /> - ) : null} {/* First-run privacy consent banner. It waits for daemon config hydration because privacyDecisionAt is daemon-owned and stripped from localStorage. It waits for `onboardingCompleted` so first-run diff --git a/apps/web/src/components/EntryShell.tsx b/apps/web/src/components/EntryShell.tsx index 6d928fda4f..0c585501fb 100644 --- a/apps/web/src/components/EntryShell.tsx +++ b/apps/web/src/components/EntryShell.tsx @@ -24,12 +24,10 @@ import { defaultScenarioPluginIdForProjectMetadata, PROFILE_MEMORY_ID, type AmrWalletSnapshot, - type ByokCredentialProfile, type ChatSessionMode, type ConnectorDetail, type InstalledPluginRecord, type RunContextSelection, - type UpsertByokCredentialProfileRequest, type UpsertMemoryRequest, } from '@open-design/contracts'; import type { OpenDesignHostProjectImportSuccess } from '@open-design/host'; @@ -159,11 +157,7 @@ import { API_PROTOCOL_TABS, SUGGESTED_MODELS_BY_PROTOCOL, } from '../state/apiProtocols'; -import { - applySavedByokCredentialProfile, - defaultKnownProviderModel, - KNOWN_PROVIDERS, -} from '../state/config'; +import { defaultKnownProviderModel, KNOWN_PROVIDERS } from '../state/config'; import type { KnownProvider } from '../state/config'; import { saveOnboardingProfile } from '../state/onboarding-profile'; import { testAgent, testApiProvider } from '../providers/connection-test'; @@ -438,9 +432,6 @@ interface Props { onOpenDesignSystem?: (id: string) => void; onDesignSystemsRefresh?: () => Promise | void; onPersistComposioKey: (composio: AppConfig['composio']) => Promise | void; - onPersistByokCredential?: ( - input: UpsertByokCredentialProfileRequest, - ) => Promise; onOpenSettings: (section?: EntrySettingsSection) => void; onCompleteOnboarding: () => void; artifactUpgradeSlot?: ReactNode; @@ -544,7 +535,6 @@ export function EntryShell({ onOpenDesignSystem, onDesignSystemsRefresh, onPersistComposioKey, - onPersistByokCredential, onOpenSettings, onCompleteOnboarding, artifactUpgradeSlot, @@ -940,7 +930,6 @@ export function EntryShell({ onApiProtocolChange={onApiProtocolChange} onApiModelChange={onApiModelChange} onConfigPersist={onConfigPersist} - {...(onPersistByokCredential ? { onPersistByokCredential } : {})} onRefreshAgents={onRefreshAgents} onFinish={finishOnboarding} onThemeChange={onThemeChange} @@ -1310,7 +1299,6 @@ function OnboardingView({ onApiProtocolChange, onApiModelChange, onConfigPersist, - onPersistByokCredential, onRefreshAgents, onFinish, onThemeChange, @@ -1331,9 +1319,6 @@ function OnboardingView({ onApiProtocolChange: (protocol: ApiProtocol) => void; onApiModelChange: (model: string) => void; onConfigPersist: (cfg: AppConfig) => Promise | void; - onPersistByokCredential?: ( - input: UpsertByokCredentialProfileRequest, - ) => Promise; onRefreshAgents: () => Promise | AgentInfo[]; // `survey` is passed on the About-you completion paths (not on skip) so the // shell can build a personalized Home recommendation. @@ -1351,7 +1336,6 @@ function OnboardingView({ // straight from the landing's primary button. const [connectExpanded, setConnectExpanded] = useState<'local' | 'byok' | null>(null); const [apiKeyVisible, setApiKeyVisible] = useState(false); - const [byokPersistPending, setByokPersistPending] = useState(false); const [cliScanStatus, setCliScanStatus] = useState<'idle' | 'scanning' | 'done'>('idle'); const [amrStatus, setAmrStatus] = useState(null); // Initial login status fetch has settled, whether signed in or not. The @@ -2074,7 +2058,7 @@ function OnboardingView({ setStep((current) => current - 1); } async function handlePrimaryAction() { - if (newsletterSubmitting || byokPersistPending) return; + if (newsletterSubmitting) return; // Connect gate: the button is `aria-disabled` (not natively disabled, so it // can still surface its tooltip on hover), so guard the click here — a // blocked Continue must not advance past the Connect step. @@ -2092,72 +2076,6 @@ function OnboardingView({ void handleAmrSignInToContinue(attribution); return; } - if (step === 0 && runtime === 'byok') { - if (!byokConnectionVerified) return; - if (apiProtocol === 'bedrock') { - setProviderTestState({ - status: 'done', - inputKey: providerTestInputKey, - result: { - ok: false, - kind: 'unknown', - latencyMs: 0, - model: config.model, - detail: 'Secure BYOK profiles do not support the Bedrock protocol', - }, - }); - return; - } - if (!onPersistByokCredential) { - setProviderTestState({ - status: 'done', - inputKey: providerTestInputKey, - result: { - ok: false, - kind: 'unknown', - latencyMs: 0, - model: config.model, - detail: 'Secure BYOK credential storage is unavailable', - }, - }); - return; - } - setByokPersistPending(true); - try { - const profile = await onPersistByokCredential({ - ...(config.byokProfileId ? { id: config.byokProfileId } : {}), - label: selectedProvider?.label ?? apiProtocol, - protocol: apiProtocol, - baseUrl: config.baseUrl.trim(), - model: config.model.trim(), - ...(apiProtocol === 'azure' && config.apiVersion?.trim() - ? { apiVersion: config.apiVersion.trim() } - : {}), - requiresApiKey: true, - apiKey: config.apiKey.trim(), - }); - await onConfigPersist(applySavedByokCredentialProfile(config, profile)); - emitOnboardingClick('continue', 'continue'); - setStep((current) => current + 1); - } catch (error) { - setProviderTestState({ - status: 'done', - inputKey: providerTestInputKey, - result: { - ok: false, - kind: 'unknown', - latencyMs: 0, - model: config.model, - detail: error instanceof Error - ? error.message - : 'Secure BYOK credential storage is unavailable', - }, - }); - } finally { - setByokPersistPending(false); - } - return; - } if (isLastStep) { await runOnboardingCompletion('completed_without_design_system'); onFinish({ @@ -2759,10 +2677,8 @@ function OnboardingView({ step, ]); - const onboardingNavigationLocked = newsletterSubmitting || byokPersistPending; - const primaryActionLabel = byokPersistPending - ? t('common.loading') - : isLastStep && newsletterSubmitting + const onboardingNavigationLocked = newsletterSubmitting; + const primaryActionLabel = isLastStep && newsletterSubmitting ? t('common.loading') : step === 0 && amrLoginPending ? t('settings.amrSigningIn') diff --git a/apps/web/src/components/EntryView.tsx b/apps/web/src/components/EntryView.tsx index 1f3e9b4ae0..298a7ef2ed 100644 --- a/apps/web/src/components/EntryView.tsx +++ b/apps/web/src/components/EntryView.tsx @@ -6,12 +6,7 @@ import { type ReactNode, type SetStateAction, } from 'react'; -import type { - ByokCredentialProfile, - ChatSessionMode, - ConnectorDetail, - UpsertByokCredentialProfileRequest, -} from '@open-design/contracts'; +import type { ChatSessionMode, ConnectorDetail } from '@open-design/contracts'; import type { OpenDesignHostProjectImportSuccess } from '@open-design/host'; import { DEFAULT_AUDIO_MODEL, @@ -101,9 +96,6 @@ interface Props { onApiProtocolChange: (protocol: ApiProtocol) => void; onApiModelChange: (model: string) => void; onConfigPersist: (cfg: AppConfig) => Promise | void; - onPersistByokCredential?: ( - input: UpsertByokCredentialProfileRequest, - ) => Promise; /** True only when GET /api/app-config returned a real config object. */ daemonAppConfigReady?: boolean; /** Non-optimistic daemon write for the silent-update preference. */ @@ -267,7 +259,6 @@ export function EntryView({ onApiProtocolChange, onApiModelChange, onConfigPersist, - onPersistByokCredential, daemonAppConfigReady = false, onSilentUpdatePreferenceChange, onSkillsRefresh, @@ -392,7 +383,6 @@ export function EntryView({ onApiProtocolChange={onApiProtocolChange} onApiModelChange={onApiModelChange} onConfigPersist={onConfigPersist} - {...(onPersistByokCredential ? { onPersistByokCredential } : {})} daemonAppConfigReady={daemonAppConfigReady} onSilentUpdatePreferenceChange={onSilentUpdatePreferenceChange} onSkillsRefresh={onSkillsRefresh} diff --git a/apps/web/src/components/ProjectView.tsx b/apps/web/src/components/ProjectView.tsx index 0b4d9a5a83..1e22d10d8f 100644 --- a/apps/web/src/components/ProjectView.tsx +++ b/apps/web/src/components/ProjectView.tsx @@ -52,6 +52,7 @@ import { useCoalescedCallback } from '../hooks/useCoalescedCallback'; import { requestAmrArtifactUpgrade } from '../runtime/amr-artifact-upgrade'; import { type AmrWalletSnapshot, + type ByokChatProviderConfig, type ByokMediaDefaults, type ByokChatProtocol, type ResearchOptions, @@ -101,7 +102,7 @@ import { } from '../utils/apiProtocol'; import { playSound, showCompletionNotification } from '../utils/notifications'; import { randomUUID } from '../utils/uuid'; -import { DEFAULT_NOTIFICATIONS } from '../state/config'; +import { DEFAULT_NOTIFICATIONS, KNOWN_PROVIDERS } from '../state/config'; import type { TodoItem } from '../runtime/todos'; import { appendErrorStatusEvent, @@ -270,6 +271,7 @@ import { effectiveMaxTokens } from '../state/maxTokens'; import { effectiveAgentModelChoice } from './agentModelSelection'; import { mediaExecutionPolicyForProjectMetadata } from '../media/execution-policy'; import { mediaModelProviderId } from '../media/models'; +import { byokProviderRequiresApiKey } from '../utils/byokProvider'; import { useByokImageModelOptions, useByokVideoModelOptions, @@ -1236,13 +1238,48 @@ function byokMediaDefaultsForRun(input: { }; } -function byokOpenCodeProfileIdFromConfig( +function byokOpenCodeProviderFromConfig( config: AppConfig, -): string | undefined { +): ByokChatProviderConfig | undefined { if (!isOpenCodeByokChatProtocol(config.apiProtocol)) return undefined; - if (byokPreflightBlockReason(config) !== null) return undefined; - if (!config.byokCredentialConfigured) return undefined; - return config.byokProfileId?.trim() || undefined; + const selectedProvider = selectedKnownProviderForConfig(config); + const model = config.model.trim(); + if ( + (byokProviderRequiresApiKey(config.apiProtocol, selectedProvider, config.baseUrl) + && !config.apiKey.trim()) + || !model + || model.toLowerCase() === 'default' + || (config.apiProtocol === 'azure' && !config.baseUrl.trim()) + ) { + return undefined; + } + return { + protocol: config.apiProtocol, + apiKey: config.apiKey.trim(), + baseUrl: config.baseUrl.trim(), + model, + ...(config.apiProtocol === 'azure' && config.apiVersion?.trim() + ? { apiVersion: config.apiVersion.trim() } + : {}), + requiresApiKey: byokProviderRequiresApiKey( + config.apiProtocol, + selectedProvider, + config.baseUrl, + ), + }; +} + +function selectedKnownProviderForConfig(config: AppConfig) { + if (!config.apiProtocol) return undefined; + return KNOWN_PROVIDERS.find( + (provider) => + provider.protocol === config.apiProtocol + && provider.baseUrl === config.baseUrl + && ( + config.apiProviderBaseUrl == null + || provider.baseUrl === config.apiProviderBaseUrl + ), + ); } function isOpenCodeByokChatProtocol( @@ -4805,11 +4842,11 @@ export function ProjectView({ chatAttachmentsFromPreviewCommentImages(attachment.imageAttachments), ), ); - const byokProfileId = byokOpenCodeProfileIdFromConfig(config); + const byokOpenCodeProvider = byokOpenCodeProviderFromConfig(config); const requiresByokPreflight = (config.mode === 'api' && config.apiProtocol !== 'bedrock') || (config.mode === 'daemon' && config.agentId === 'byok-opencode'); - if (requiresByokPreflight && !byokProfileId) { + if (requiresByokPreflight && !byokOpenCodeProvider) { trackByokPreflightBlocked(analytics.track, { source: 'run', reason: byokPreflightBlockReason(config) ?? 'config_invalid', @@ -5899,8 +5936,8 @@ export function ProjectView({ model: daemonByokOpenCode ? config.model : choice?.model ?? null, reasoning: daemonByokOpenCode ? null : choice?.reasoning ?? null, serviceTier: daemonByokOpenCode ? null : choice?.serviceTier ?? null, - ...(daemonByokOpenCode && byokProfileId - ? { byokProfileId } + ...(daemonByokOpenCode && byokOpenCodeProvider + ? { byokProvider: byokOpenCodeProvider } : {}), ...(daemonByokOpenCode ? { @@ -5981,9 +6018,18 @@ export function ProjectView({ // BYOK users even though the UI saves model + index + entries // for that mode. const userText = (userMsg.content ?? '').trim(); - // Pass only the non-secret profile reference so "Same as chat" - // memory extraction resolves the same daemon-owned credential as the - // run. Raw provider keys never cross this browser call boundary. + // Forward the per-call BYOK provider snapshot so "Same as chat" + // memory extraction uses the same vendor, endpoint, key and model as + // the run. The daemon consumes it for this request only. + const byokChatProvider = byokOpenCodeProvider + ? { + provider: byokOpenCodeProvider.protocol, + apiKey: byokOpenCodeProvider.apiKey, + baseUrl: byokOpenCodeProvider.baseUrl, + apiVersion: byokOpenCodeProvider.apiVersion, + model: byokOpenCodeProvider.model, + } + : undefined; if (userText.length > 0) { try { await fetch('/api/memory/extract', { @@ -5993,7 +6039,7 @@ export function ProjectView({ userMessage: userText, projectId: project.id, conversationId: runConversationId, - byokProfileId, + byokChatProvider, }), }); } catch { @@ -6045,7 +6091,7 @@ export function ProjectView({ model: config.model, reasoning: null, serviceTier: null, - ...(byokProfileId ? { byokProfileId } : {}), + ...(byokOpenCodeProvider ? { byokProvider: byokOpenCodeProvider } : {}), byokMediaDefaults: byokMediaDefaultsForRun({ imageModelOverride: byokImageModelOverride, videoModelOverride: byokVideoModelOverride, diff --git a/apps/web/src/components/SettingsDialog.tsx b/apps/web/src/components/SettingsDialog.tsx index 8208c0692d..44e3a42e93 100644 --- a/apps/web/src/components/SettingsDialog.tsx +++ b/apps/web/src/components/SettingsDialog.tsx @@ -1,11 +1,7 @@ import { useCallback, useEffect, useLayoutEffect, useMemo, useRef, useState } from 'react'; import type { CSSProperties, Dispatch, SetStateAction } from 'react'; import { Button, VisuallyHidden } from '@open-design/components'; -import type { - AmrWalletSnapshot, - ByokCredentialProfile, - UpsertByokCredentialProfileRequest, -} from '@open-design/contracts'; +import type { AmrWalletSnapshot } from '@open-design/contracts'; import { validateBaseUrl } from '@open-design/contracts/api/connectionTest'; import { agentIdToTracking, @@ -74,10 +70,8 @@ import { } from './modelOptions'; import { BYOK_PROVIDER_PRESETS, - classifyByokCredentialProfileFailure, DEFAULT_NOTIFICATIONS, DEFAULT_ORBIT, - applySavedByokCredentialProfile, defaultKnownProviderModel, isStoredMediaProviderEntryEmpty, isStoredMediaProviderEntryPresent, @@ -133,7 +127,6 @@ import type { import { testAgent, testApiProvider, - testSavedByokProfile, } from '../providers/connection-test'; import { fetchProviderModels } from '../providers/provider-models'; import { @@ -431,14 +424,6 @@ interface Props { * "Save key" button rather than the autosave channel. */ onPersistComposioKey: (composio: AppConfig['composio']) => Promise | void; - /** - * Explicitly moves the current BYOK key draft into the daemon's OS-backed - * credential store. The returned profile is non-secret and becomes the only - * credential reference retained by the UI. - */ - onPersistByokCredential?: ( - input: UpsertByokCredentialProfileRequest, - ) => Promise; /** * True while the daemon-backed Composio config is still hydrating on * first paint after a dev-server / app restart. The Connectors section @@ -1371,11 +1356,7 @@ export function shouldEnableSettingsSave( cfg.agentId && agents.find((a) => a.id === cfg.agentId)?.available, ); } - return Boolean( - (cfg.apiKey.trim() || (cfg.byokProfileId && cfg.byokCredentialConfigured)) - && cfg.model.trim() - && isBaseUrlValid, - ); + return Boolean(cfg.apiKey.trim() && cfg.model.trim() && isBaseUrlValid); } /** @@ -1410,9 +1391,6 @@ export function sanitizeSettingsSavePayload( ...cfg, mode: initial.mode, apiKey: initial.apiKey, - byokProfileId: initial.byokProfileId, - byokCredentialConfigured: initial.byokCredentialConfigured, - byokCredentialTail: initial.byokCredentialTail, apiProtocol: initial.apiProtocol, apiVersion: initial.apiVersion, apiProtocolConfigs: initial.apiProtocolConfigs, @@ -1443,7 +1421,7 @@ export function switchApiProtocolConfig( }, protocol, ); - const switched = applyApiProtocolConfig( + return applyApiProtocolConfig( { ...config, mode: 'api', @@ -1452,14 +1430,6 @@ export function switchApiProtocolConfig( protocol, nextApiConfig, ); - return currentProtocol === protocol - ? switched - : { - ...switched, - byokProfileId: undefined, - byokCredentialConfigured: false, - byokCredentialTail: undefined, - }; } export function SettingsDialog({ @@ -1474,7 +1444,6 @@ export function SettingsDialog({ onPersist, onSilentUpdatePreferenceChange, onPersistComposioKey, - onPersistByokCredential, composioConfigLoading = false, onClose, onRefreshAgents, @@ -2179,14 +2148,6 @@ export function SettingsDialog({ ? (current.apiProviderBaseUrl ?? null) !== null : currentProtocol !== provider.protocol || (current.apiProviderBaseUrl ?? null) !== nextProviderBaseUrl; - const finalizeProviderSwitch = (next: AppConfig): AppConfig => providerChanged - ? { - ...next, - byokProfileId: undefined, - byokCredentialConfigured: false, - byokCredentialTail: undefined, - } - : next; const switched = switchApiProtocolConfig(current, provider.protocol); const fallbackApiConfig = currentApiProtocolConfig(switched); const customDraftKey = provider.custom @@ -2217,7 +2178,7 @@ export function SettingsDialog({ }; if (savedDraft) { applyDraftUiState(savedDraft); - return finalizeProviderSwitch(applyApiProtocolConfig( + return applyApiProtocolConfig( persistByokProviderConfigDraft( { ...switched, @@ -2228,11 +2189,11 @@ export function SettingsDialog({ ), provider.protocol, savedDraft.apiConfig, - )); + ); } if (persistedDraft) { applyDraftUiState(undefined); - return finalizeProviderSwitch(applyApiProtocolConfig( + return applyApiProtocolConfig( persistByokProviderConfigDraft( { ...switched, @@ -2243,7 +2204,7 @@ export function SettingsDialog({ ), provider.protocol, persistedDraft.apiConfig, - )); + ); } const switchedWithCurrentDraft = persistByokProviderConfigDraft( switched, @@ -2252,42 +2213,22 @@ export function SettingsDialog({ ); if (provider.custom) { applyDraftUiState(undefined); - return finalizeProviderSwitch(updateCurrentApiProtocolConfig(switchedWithCurrentDraft, { + return updateCurrentApiProtocolConfig(switchedWithCurrentDraft, { apiProviderBaseUrl: null, ...(providerChanged ? { model: '' } : {}), - })); + }); } applyDraftUiState(undefined); - return finalizeProviderSwitch(updateCurrentApiProtocolConfig(switchedWithCurrentDraft, { + return updateCurrentApiProtocolConfig(switchedWithCurrentDraft, { ...(providerChanged ? { apiKey: '' } : {}), baseUrl: provider.baseUrl, model: provider.preferredModels[0] ?? '', apiProviderBaseUrl: provider.baseUrl, - })); + }); }); }; const updateApiConfig = (patch: Partial) => - setCfg((c) => { - const next = updateCurrentApiProtocolConfig(c, patch); - const invalidatesProfile = ( - (patch.apiKey !== undefined && Boolean(patch.apiKey.trim())) - || (patch.baseUrl !== undefined && patch.baseUrl !== c.baseUrl) - || (patch.model !== undefined && patch.model !== c.model) - || (patch.apiVersion !== undefined && patch.apiVersion !== c.apiVersion) - ); - return invalidatesProfile - ? { - ...next, - // Keep the id so a confirmed replacement updates the existing - // secure-store entry instead of leaking orphaned keychain items. - // The configured marker is cleared so runs remain blocked until - // the edited draft is tested and saved again. - byokProfileId: c.byokProfileId, - byokCredentialConfigured: false, - byokCredentialTail: undefined, - } - : next; - }); + setCfg((c) => updateCurrentApiProtocolConfig(c, patch)); const updateMaxTokensInput = (raw: string) => { setMaxTokensInput(raw); const trimmed = raw.trim(); @@ -2581,57 +2522,24 @@ export function SettingsDialog({ } }; try { - const testingSavedProfile = Boolean( - cfg.byokProfileId - && cfg.byokCredentialConfigured - && !cfg.apiKey.trim(), + const result = await testApiProvider( + { + protocol: apiProtocol, + baseUrl: cfg.baseUrl, + apiKey: cleanByokApiKey(cfg.apiKey), + model: cfg.model, + apiVersion: + apiProtocol === 'azure' + ? cfg.apiVersion?.trim() || undefined + : undefined, + }, + controller.signal, ); - const result = testingSavedProfile && cfg.byokProfileId - ? await testSavedByokProfile(cfg.byokProfileId, controller.signal) - : await testApiProvider( - { - protocol: apiProtocol, - baseUrl: cfg.baseUrl, - apiKey: cleanByokApiKey(cfg.apiKey), - model: cfg.model, - apiVersion: - apiProtocol === 'azure' - ? cfg.apiVersion?.trim() || undefined - : undefined, - }, - controller.signal, - ); if (controller.signal.aborted) return; if (providerTestRevisionRef.current !== revision) { clearIfStale(); return; } - if (result.ok && apiProtocol !== 'bedrock' && !testingSavedProfile) { - if (!onPersistByokCredential) { - throw new Error('Secure BYOK credential storage is unavailable'); - } - const profile = await onPersistByokCredential({ - ...(cfg.byokProfileId ? { id: cfg.byokProfileId } : {}), - label: selectedProvider?.label ?? API_PROTOCOL_LABELS[apiProtocol], - protocol: apiProtocol, - baseUrl: cfg.baseUrl.trim(), - model: cfg.model.trim(), - ...(apiProtocol === 'azure' && cfg.apiVersion?.trim() - ? { apiVersion: cfg.apiVersion.trim() } - : {}), - requiresApiKey: byokRequiresApiKey, - ...(cfg.apiKey.trim() - ? { apiKey: cleanByokApiKey(cfg.apiKey) } - : {}), - }); - if (controller.signal.aborted) return; - if (providerTestRevisionRef.current !== revision) { - clearIfStale(); - return; - } - providerTestSkipNextResetRef.current = true; - setCfg((current) => applySavedByokCredentialProfile(current, profile)); - } setProviderTestState({ status: 'done', result }); if (!result.ok && result.kind === 'not_found_model') { focusByokRequiredField('model'); @@ -2670,14 +2578,13 @@ export function SettingsDialog({ }); const byokProviderId = byokProtocolToTracking(apiProtocol); if (byokProviderId) { - const failure = classifyByokCredentialProfileFailure(err); trackSettingsByokTestResult(analytics.track, { page_name: 'settings', area: 'execution_model', provider_id: byokProviderId, result: 'failed', - error_code: failure.errorCode, - error_kind: failure.errorKind, + error_code: err instanceof Error ? err.name : 'UNKNOWN', + error_kind: err instanceof Error ? err.name : 'UNKNOWN', field_missing: 'none', config_key_changed: configKeyChanged, success_after_action: false, @@ -3394,13 +3301,6 @@ export function SettingsDialog({ cfg.baseUrl, ); const byokProviderConfigured = (provider: ByokProviderPreset): boolean => { - if ( - selectedByokProvider?.id === provider.id - && cfg.byokProfileId - && cfg.byokCredentialConfigured - ) { - return true; - } if (provider.custom) { return canRunProviderConnectionTest(currentApiProtocolConfig(cfg), { requiresApiKey: byokRequiresApiKey, @@ -3451,9 +3351,6 @@ export function SettingsDialog({ }, { requiresApiKey: byokRequiresApiKey, - credentialConfigured: Boolean( - cfg.byokProfileId && cfg.byokCredentialConfigured, - ), keyValidationBaseUrl: byokKeyValidationBaseUrl, }, ), @@ -3463,8 +3360,6 @@ export function SettingsDialog({ byokRequiresApiKey, cfg.apiKey, cfg.baseUrl, - cfg.byokCredentialConfigured, - cfg.byokProfileId, cfg.model, ], ); @@ -3479,8 +3374,6 @@ export function SettingsDialog({ cfg.apiProtocol, cfg.apiProviderBaseUrl, cfg.baseUrl, - cfg.byokCredentialConfigured, - cfg.byokProfileId, cfg.model, ], ); @@ -3497,9 +3390,6 @@ export function SettingsDialog({ }, { requiresApiKey: byokRequiresApiKey, - credentialConfigured: Boolean( - cfg.byokProfileId && cfg.byokCredentialConfigured, - ), requireModel: false, keyValidationBaseUrl: byokKeyValidationBaseUrl, }, @@ -3510,8 +3400,6 @@ export function SettingsDialog({ byokRequiresApiKey, cfg.apiKey, cfg.baseUrl, - cfg.byokCredentialConfigured, - cfg.byokProfileId, cfg.model, ], ); @@ -3803,8 +3691,7 @@ export function SettingsDialog({ focusByokRequiredFieldAfterProtocolSwitchRef.current = false; focusByokRequiredField( missingByokConnectionFields(cfg, { - requiresApiKey: byokRequiresApiKey - && !(cfg.byokProfileId && cfg.byokCredentialConfigured), + requiresApiKey: byokRequiresApiKey, })[0], ); }, [apiModelCustomActive, cfg, apiProtocol, byokRequiresApiKey]); diff --git a/apps/web/src/components/byok/preflight.ts b/apps/web/src/components/byok/preflight.ts index 6c09d03f64..0a11b0d783 100644 --- a/apps/web/src/components/byok/preflight.ts +++ b/apps/web/src/components/byok/preflight.ts @@ -6,13 +6,7 @@ import { blockingByokDraftIssues, validateByokDraft } from './validation'; type ByokPreflightConfig = Pick< AppConfig, - | 'apiKey' - | 'apiProtocol' - | 'apiProviderBaseUrl' - | 'baseUrl' - | 'model' - | 'byokProfileId' - | 'byokCredentialConfigured' + 'apiKey' | 'apiProtocol' | 'apiProviderBaseUrl' | 'baseUrl' | 'model' >; export function byokPreflightBlockReason( @@ -38,9 +32,6 @@ export function byokPreflightBlockReason( selectedProvider, config.baseUrl, ), - credentialConfigured: Boolean( - config.byokProfileId && config.byokCredentialConfigured, - ), }, ); const missingReasons = new Set(); diff --git a/apps/web/src/providers/connection-test.ts b/apps/web/src/providers/connection-test.ts index c4bc74da6b..7569792adf 100644 --- a/apps/web/src/providers/connection-test.ts +++ b/apps/web/src/providers/connection-test.ts @@ -68,36 +68,6 @@ export function testApiProvider( return postTest({ mode: 'provider', ...input }, signal); } -export async function testSavedByokProfile( - profileId: string, - signal?: AbortSignal, -): Promise { - const start = Date.now(); - try { - const response = await fetch( - `/api/byok/profiles/${encodeURIComponent(profileId)}/test`, - { method: 'POST', signal }, - ); - if (!response.ok) { - return { - ok: false, - kind: 'unknown', - latencyMs: Date.now() - start, - detail: `Daemon responded with ${response.status}`, - }; - } - return await response.json() as ConnectionTestResponse; - } catch (err) { - if (err instanceof DOMException && err.name === 'AbortError') throw err; - return { - ok: false, - kind: 'unknown', - latencyMs: Date.now() - start, - detail: err instanceof Error ? err.message : 'Network request failed', - }; - } -} - export function testAgent( input: AgentTestRequest, signal?: AbortSignal, diff --git a/apps/web/src/providers/daemon.ts b/apps/web/src/providers/daemon.ts index 0f5ffbe3a1..6002a8681f 100644 --- a/apps/web/src/providers/daemon.ts +++ b/apps/web/src/providers/daemon.ts @@ -31,6 +31,7 @@ import type { DaemonAgentPayload, AmrModelsResponse, AmrWalletSnapshot, + ByokChatProviderConfig, MediaExecutionPolicy, ResearchOptions, RunContextSelection, @@ -311,8 +312,7 @@ export interface DaemonStreamOptions { model?: string | null; reasoning?: string | null; serviceTier?: string | null; - /** Non-secret reference resolved by the daemon from the OS credential store. */ - byokProfileId?: string; + byokProvider?: ByokChatProviderConfig; byokMediaDefaults?: ChatRequest['byokMediaDefaults']; research?: ResearchOptions; context?: RunContextSelection; @@ -652,7 +652,7 @@ export async function streamViaDaemon({ model, reasoning, serviceTier, - byokProfileId, + byokProvider, byokMediaDefaults, research, context, @@ -691,7 +691,7 @@ export async function streamViaDaemon({ model: model ?? null, reasoning: reasoning ?? null, serviceTier: serviceTier ?? null, - ...(byokProfileId ? { byokProfileId } : {}), + ...(byokProvider ? { byokProvider } : {}), ...(byokMediaDefaults ? { byokMediaDefaults } : {}), locale, ...(appliedPluginSnapshotId ? { appliedPluginSnapshotId } : {}), diff --git a/apps/web/src/state/config.ts b/apps/web/src/state/config.ts index 77ec4c3837..adc84d1eb4 100644 --- a/apps/web/src/state/config.ts +++ b/apps/web/src/state/config.ts @@ -1,9 +1,4 @@ -import type { - AppConfigPrefs, - ByokCredentialProfile, - ByokCredentialProfilesResponse, - UpsertByokCredentialProfileRequest, -} from '@open-design/contracts'; +import type { AppConfigPrefs } from '@open-design/contracts'; import { MEDIA_PROVIDERS } from '../media/models'; import { isOpenAICompatible } from '../providers/openai-compatible'; import type { @@ -28,6 +23,11 @@ import { randomUUID } from '../utils/uuid'; const STORAGE_KEY = 'open-design:config'; const CONFIG_MIGRATION_VERSION = 2; +const RETIRED_SECURE_BYOK_KEYS = [ + 'byokProfileId', + 'byokCredentialConfigured', + 'byokCredentialTail', +] as const; // Hatched out of the box, but tucked away — the user has to go through // either the entry-view "adopt a pet" callout or Settings → Pets to @@ -625,18 +625,6 @@ function inferApiProtocol(model: string, baseUrl: string): ApiProtocol { } } -function hasLegacyByokSecret(config: AppConfig): boolean { - return Boolean( - config.apiKey?.trim() - || Object.values(config.apiProtocolConfigs ?? {}).some( - (entry) => Boolean(entry?.apiKey?.trim()), - ) - || Object.values(config.byokProviderConfigDrafts ?? {}).some( - (draft) => Boolean(draft?.apiConfig.apiKey?.trim()), - ), - ); -} - function migrateRetiredKnownProviderModel( protocol: ApiProtocol, config: Pick< @@ -677,6 +665,9 @@ export function loadConfig(): AppConfig { for (const key of DAEMON_OWNED_KEYS) { delete (parsed as Record)[key]; } + for (const key of RETIRED_SECURE_BYOK_KEYS) { + delete (parsed as Record)[key]; + } const parsedHasApiProtocol = Object.prototype.hasOwnProperty.call( parsed, 'apiProtocol', @@ -695,42 +686,7 @@ export function loadConfig(): AppConfig { notifications: normalizeNotifications(parsed.notifications), orbit: normalizeOrbit(parsed.orbit), }; - // Browser storage is allowed to remember which non-secret profile the - // user selected, but never to assert that the credential still exists. - // Startup hydration re-enables it only after the daemon confirms the - // secure-store entry. - if (merged.byokProfileId) { - merged.byokCredentialConfigured = false; - merged.byokCredentialTail = undefined; - } - let migratedConfig = false; - // Older builds stored BYOK credentials in browser storage. Keep those - // values intact until migrateLegacyByokCredentialsToDaemon has confirmed - // every secure-profile write; deleting them synchronously here would lose - // the user's only credential when the daemon or OS store is unavailable. - const hadLegacyByokSecret = hasLegacyByokSecret(merged); - if (hadLegacyByokSecret) { - // Keep the browser record untouched until the async secure-store - // migration succeeds, but do not hydrate plaintext credentials into the - // long-lived application state. - merged.apiKey = ''; - merged.apiProtocolConfigs = Object.fromEntries( - Object.entries(merged.apiProtocolConfigs ?? {}).map(([protocol, entry]) => [ - protocol, - entry ? { ...entry, apiKey: '' } : entry, - ]), - ) as AppConfig['apiProtocolConfigs']; - merged.byokProviderConfigDrafts = Object.fromEntries( - Object.entries(merged.byokProviderConfigDrafts ?? {}).map(([key, draft]) => [ - key, - { - ...draft, - apiConfig: { ...draft.apiConfig, apiKey: '' }, - }, - ]), - ); - } const parsedMigrationVersion = typeof parsed.configMigrationVersion === 'number' ? parsed.configMigrationVersion @@ -805,10 +761,7 @@ export function loadConfig(): AppConfig { merged.baseUrl = resolveFixedOriginBaseUrl(merged.apiProtocol, merged.baseUrl); } - if ( - !hadLegacyByokSecret - && (migratedConfig || downgradedUnsupportedChatProtocol) - ) { + if (migratedConfig || downgradedUnsupportedChatProtocol) { // Best-effort re-persist of the migrated / downgraded config. A localStorage // write failure here (quota exceeded, private-mode storage disabled) must not // fall through to the outer catch and discard the valid config we just @@ -836,422 +789,6 @@ interface PublicComposioConfigResponse { apiKeyTail?: string; } -export async function fetchByokCredentialProfilesFromDaemon(): Promise< - ByokCredentialProfilesResponse | null -> { - try { - const response = await fetch('/api/byok/profiles'); - if (!response.ok) return null; - return await response.json() as ByokCredentialProfilesResponse; - } catch { - return null; - } -} - -export class ByokCredentialProfileHttpError extends Error { - readonly status: number; - readonly code?: string; - - constructor(status: number, message: string, code?: string) { - super(message); - this.name = 'ByokCredentialProfileHttpError'; - this.status = status; - this.code = code; - } -} - -export class ByokCredentialProfileNetworkError extends Error { - constructor(message: string, cause: unknown) { - super(message, { cause }); - this.name = 'ByokCredentialProfileNetworkError'; - } -} - -export function classifyByokCredentialProfileFailure(error: unknown): { - errorCode: string; - errorKind: string; -} { - if (error instanceof ByokCredentialProfileHttpError) { - return { - errorCode: error.code || `HTTP_${error.status}`, - errorKind: 'unknown', - }; - } - if (error instanceof ByokCredentialProfileNetworkError) { - return { - errorCode: 'DAEMON_UNREACHABLE', - errorKind: 'unknown', - }; - } - const fallback = error instanceof Error ? error.name : 'UNKNOWN'; - return { errorCode: fallback, errorKind: fallback }; -} - -export function legacyByokMigrationErrorPresentation( - error: Error, - daemonUnavailableMessage: string, -): { message: string; details?: string } { - if (error instanceof ByokCredentialProfileNetworkError) { - return { - message: daemonUnavailableMessage, - details: error.message, - }; - } - return { message: error.message }; -} - -async function byokCredentialProfileHttpError( - response: Response, -): Promise { - const fallback = `Failed to save BYOK credential (${response.status})`; - try { - const payload = await response.json() as unknown; - if ( - payload - && typeof payload === 'object' - && 'error' in payload - && payload.error - && typeof payload.error === 'object' - ) { - const message = 'message' in payload.error - && typeof payload.error.message === 'string' - ? payload.error.message.trim() - : ''; - const code = 'code' in payload.error - && typeof payload.error.code === 'string' - ? payload.error.code.trim() - : ''; - if (message) { - return new ByokCredentialProfileHttpError( - response.status, - message, - code || undefined, - ); - } - } - } catch { - // Keep the status-bearing fallback when the daemon/proxy body is invalid. - } - return new ByokCredentialProfileHttpError(response.status, fallback); -} - -export async function persistByokCredentialProfileToDaemon( - input: UpsertByokCredentialProfileRequest, -): Promise { - let response: Response; - try { - response = await fetch('/api/byok/profiles', { - method: 'POST', - headers: { 'content-type': 'application/json' }, - body: JSON.stringify(input), - }); - } catch (error) { - const message = error instanceof Error && error.message.trim() - ? error.message - : 'Failed to reach the local daemon while saving the BYOK credential.'; - throw new ByokCredentialProfileNetworkError(message, error); - } - if (!response.ok) { - throw await byokCredentialProfileHttpError(response); - } - const payload = await response.json() as { profile?: ByokCredentialProfile }; - if (!payload.profile?.id || !payload.profile.configured) { - throw new Error('Daemon did not confirm the BYOK credential profile'); - } - return payload.profile; -} - -export type LegacyByokCredentialMigrationResult = - | { status: 'not-needed'; config: AppConfig } - | { status: 'migrated'; config: AppConfig } - | { status: 'failed'; config: AppConfig; error: Error }; - -interface LegacyByokCredentialCandidate { - input: UpsertByokCredentialProfileRequest & { id: string }; - signature: string; -} - -const BYOK_PROTOCOLS = new Set([ - 'anthropic', - 'openai', - 'azure', - 'google', - 'ollama', - 'senseaudio', - 'aihubmix', -]); - -function legacyByokProfileId(source: string): string { - let hash = 0x811c9dc5; - for (let index = 0; index < source.length; index += 1) { - hash ^= source.charCodeAt(index); - hash = Math.imul(hash, 0x01000193); - } - return `byok-legacy-${(hash >>> 0).toString(36)}`; -} - -function legacyByokCandidate( - source: string, - protocol: ApiProtocol | undefined, - apiConfig: Pick, -): LegacyByokCredentialCandidate | null { - const apiKey = apiConfig.apiKey?.trim(); - if (!apiKey || !protocol || !BYOK_PROTOCOLS.has(protocol)) return null; - const knownProvider = KNOWN_PROVIDERS.find( - (candidate) => - candidate.protocol === protocol - && candidate.baseUrl === apiConfig.baseUrl, - ); - const signature = [ - protocol, - apiConfig.baseUrl, - apiConfig.model, - apiConfig.apiVersion ?? '', - apiKey, - ].join('\u0000'); - return { - signature, - input: { - id: legacyByokProfileId(source), - label: knownProvider?.label ?? `Imported ${protocol} profile`, - protocol: protocol as UpsertByokCredentialProfileRequest['protocol'], - baseUrl: apiConfig.baseUrl, - model: apiConfig.model, - apiKey, - ...(apiConfig.apiVersion - ? { apiVersion: apiConfig.apiVersion } - : {}), - }, - }; -} - -function legacyByokCredentialCandidates( - config: AppConfig, -): LegacyByokCredentialCandidate[] { - const activeProtocol = isBedrockRuntimeBaseUrl(config.baseUrl) - ? 'bedrock' - : config.apiProtocol ?? inferApiProtocol(config.model, config.baseUrl); - const candidates: Array = [ - legacyByokCandidate('active', activeProtocol, config), - ...Object.entries(config.apiProtocolConfigs ?? {}).map( - ([protocol, apiConfig]) => - apiConfig - ? legacyByokCandidate( - `protocol:${protocol}`, - protocol as ApiProtocol, - apiConfig, - ) - : null, - ), - ...Object.entries(config.byokProviderConfigDrafts ?? {}).map( - ([draftKey, draft]) => { - const separator = draftKey.indexOf(':'); - const protocol = ( - separator > 0 ? draftKey.slice(0, separator) : activeProtocol - ) as ApiProtocol; - return legacyByokCandidate( - `draft:${draftKey}`, - protocol, - draft.apiConfig, - ); - }, - ), - ]; - const unique = new Map(); - for (const candidate of candidates) { - if (candidate && !unique.has(candidate.signature)) { - unique.set(candidate.signature, candidate); - } - } - return [...unique.values()]; -} - -function hasUnsupportedLegacyByokSecret(config: AppConfig): boolean { - const activeProtocol = isBedrockRuntimeBaseUrl(config.baseUrl) - ? 'bedrock' - : config.apiProtocol ?? inferApiProtocol(config.model, config.baseUrl); - if ( - config.apiKey?.trim() - && !BYOK_PROTOCOLS.has(activeProtocol) - ) { - return true; - } - if ( - Object.entries(config.apiProtocolConfigs ?? {}).some( - ([protocol, entry]) => - Boolean(entry?.apiKey?.trim()) - && !BYOK_PROTOCOLS.has(protocol as ApiProtocol), - ) - ) { - return true; - } - return Object.entries(config.byokProviderConfigDrafts ?? {}).some( - ([draftKey, draft]) => { - if (!draft.apiConfig.apiKey?.trim()) return false; - const separator = draftKey.indexOf(':'); - const protocol = ( - separator > 0 ? draftKey.slice(0, separator) : activeProtocol - ) as ApiProtocol; - return !BYOK_PROTOCOLS.has(protocol); - }, - ); -} - -function legacyByokMigrationSource(config: AppConfig): AppConfig { - try { - const raw = localStorage.getItem(STORAGE_KEY); - if (!raw) return config; - const parsed = JSON.parse(raw) as Partial; - return { - ...config, - ...parsed, - apiProtocol: parsed.apiProtocol ?? config.apiProtocol, - apiProtocolConfigs: { - ...(config.apiProtocolConfigs ?? {}), - ...(parsed.apiProtocolConfigs ?? {}), - }, - byokProviderConfigDrafts: { - ...(config.byokProviderConfigDrafts ?? {}), - ...(parsed.byokProviderConfigDrafts ?? {}), - }, - }; - } catch { - return config; - } -} - -export function applySavedByokCredentialProfile( - config: AppConfig, - selectedProfile: ByokCredentialProfile, -): AppConfig { - return { - ...config, - apiKey: '', - byokProfileId: selectedProfile.id, - byokCredentialConfigured: selectedProfile.configured, - byokCredentialTail: selectedProfile.keyTail, - apiProtocolConfigs: Object.fromEntries( - Object.entries(config.apiProtocolConfigs ?? {}).map(([protocol, entry]) => [ - protocol, - entry ? { ...entry, apiKey: '' } : entry, - ]), - ) as AppConfig['apiProtocolConfigs'], - byokProviderConfigDrafts: Object.fromEntries( - Object.entries(config.byokProviderConfigDrafts ?? {}).map( - ([key, draft]) => [ - key, - { - ...draft, - apiConfig: { ...draft.apiConfig, apiKey: '' }, - }, - ], - ), - ), - }; -} - -export async function migrateLegacyByokCredentialsToDaemon( - config: AppConfig, - persistProfile: ( - input: UpsertByokCredentialProfileRequest, - ) => Promise = - persistByokCredentialProfileToDaemon, -): Promise { - const migrationSource = legacyByokMigrationSource(config); - const hasLegacySecret = hasLegacyByokSecret(migrationSource); - if (!hasLegacySecret) return { status: 'not-needed', config }; - - if (hasUnsupportedLegacyByokSecret(migrationSource)) { - return { - status: 'failed', - config, - error: new Error( - 'The saved BYOK credential uses an unsupported legacy provider. Re-enter it in Settings.', - ), - }; - } - const candidates = legacyByokCredentialCandidates(migrationSource); - if (candidates.length === 0) { - return { - status: 'failed', - config, - error: new Error( - 'The saved BYOK credential uses an unsupported legacy provider. Re-enter it in Settings.', - ), - }; - } - - try { - const profiles: ByokCredentialProfile[] = []; - for (const candidate of candidates) { - profiles.push(await persistProfile(candidate.input)); - } - const selectedProfile = profiles[0]; - if (!selectedProfile?.configured) { - throw new Error('Secure credential migration did not produce a usable profile.'); - } - const migrated = applySavedByokCredentialProfile(config, selectedProfile); - saveConfig(migrated); - return { status: 'migrated', config: migrated }; - } catch (error) { - return { - status: 'failed', - config, - error: error instanceof Error ? error : new Error(String(error)), - }; - } -} - -/** - * Reconciles a locally selected non-secret profile reference with the daemon. - * No automatic "first profile" selection is made: changing execution - * credentials must remain an explicit user choice. - */ -export function mergeByokCredentialProfiles( - config: AppConfig, - response: ByokCredentialProfilesResponse | null, -): AppConfig { - if (!config.byokProfileId || !response) return config; - const profile = response.profiles.find((candidate) => candidate.id === config.byokProfileId); - if (!response.available || !profile?.configured) { - return { - ...config, - byokProfileId: undefined, - byokCredentialConfigured: false, - byokCredentialTail: undefined, - }; - } - const protocol = profile.protocol as ApiProtocol; - const knownProvider = KNOWN_PROVIDERS.find( - (candidate) => - candidate.protocol === protocol - && candidate.baseUrl === profile.baseUrl, - ); - const previousProtocolConfig = config.apiProtocolConfigs?.[protocol]; - return { - ...config, - apiKey: '', - apiProtocol: protocol, - apiProviderBaseUrl: knownProvider?.baseUrl ?? null, - apiProtocolConfigs: { - ...(config.apiProtocolConfigs ?? {}), - [protocol]: { - ...(previousProtocolConfig ?? {}), - apiKey: '', - apiProviderBaseUrl: knownProvider?.baseUrl ?? null, - apiVersion: profile.apiVersion ?? '', - baseUrl: profile.baseUrl, - model: profile.model, - }, - }, - apiVersion: profile.apiVersion ?? '', - baseUrl: profile.baseUrl, - byokCredentialConfigured: true, - byokCredentialTail: profile.keyTail, - model: profile.model, - }; -} - interface PublicMediaProviderConfigEntry { configured?: boolean; source?: string; @@ -1463,90 +1000,16 @@ function sanitizeAgentCliEnv(agentCliEnv: AppConfig['agentCliEnv']): AppConfig[' return sanitized; } -function preserveLegacyByokSecrets( - config: AppConfig, - stored: AppConfig, -): AppConfig { - const apiProtocolConfigs = { - ...(config.apiProtocolConfigs ?? {}), - }; - for (const [protocol, storedEntry] of Object.entries(stored.apiProtocolConfigs ?? {})) { - if (!storedEntry?.apiKey?.trim()) continue; - apiProtocolConfigs[protocol as ApiProtocol] = { - ...storedEntry, - ...(apiProtocolConfigs[protocol as ApiProtocol] ?? {}), - apiKey: storedEntry.apiKey, - }; - } - - const byokProviderConfigDrafts = { - ...(config.byokProviderConfigDrafts ?? {}), - }; - for (const [key, storedDraft] of Object.entries(stored.byokProviderConfigDrafts ?? {})) { - if (!storedDraft.apiConfig.apiKey?.trim()) continue; - const currentDraft = byokProviderConfigDrafts[key]; - byokProviderConfigDrafts[key] = { - ...storedDraft, - ...currentDraft, - apiConfig: { - ...storedDraft.apiConfig, - ...(currentDraft?.apiConfig ?? {}), - apiKey: storedDraft.apiConfig.apiKey, - }, - }; - } - - return { - ...config, - ...(stored.apiKey?.trim() ? { apiKey: stored.apiKey } : {}), - apiProtocolConfigs, - byokProviderConfigDrafts, - }; -} - export function saveConfig(config: AppConfig): void { - let storedConfig: AppConfig | null = null; - try { - storedConfig = JSON.parse( - localStorage.getItem(STORAGE_KEY) ?? '{}', - ) as AppConfig; - } catch { - // A malformed old record cannot be preserved as a usable credential. - } - const apiProtocolConfigs = config.apiProtocolConfigs - ? Object.fromEntries(Object.entries(config.apiProtocolConfigs).map(([protocol, entry]) => [ - protocol, - entry ? { ...entry, apiKey: '' } : entry, - ])) as AppConfig['apiProtocolConfigs'] - : config.apiProtocolConfigs; - const byokProviderConfigDrafts = config.byokProviderConfigDrafts - ? Object.fromEntries(Object.entries(config.byokProviderConfigDrafts).map(([key, draft]) => [ - key, - { - ...draft, - apiConfig: { ...draft.apiConfig, apiKey: '' }, - }, - ])) - : config.byokProviderConfigDrafts; - let sanitized: AppConfig = { + const sanitized: AppConfig = { ...config, - apiKey: '', - apiProtocolConfigs, - byokProviderConfigDrafts, agentCliEnv: sanitizeAgentCliEnv(config.agentCliEnv), }; for (const key of DAEMON_OWNED_KEYS) { delete (sanitized as unknown as Record)[key]; } - if ( - storedConfig - && hasLegacyByokSecret(storedConfig) - && !(config.byokProfileId && config.byokCredentialConfigured) - ) { - // A legacy plaintext key may be the user's only credential. Keep only - // those secret projections until secure migration succeeds, while still - // allowing unrelated settings (mode, model, theme, etc.) to persist. - sanitized = preserveLegacyByokSecrets(sanitized, storedConfig); + for (const key of RETIRED_SECURE_BYOK_KEYS) { + delete (sanitized as unknown as Record)[key]; } localStorage.setItem(STORAGE_KEY, JSON.stringify(sanitized)); } diff --git a/apps/web/src/types.ts b/apps/web/src/types.ts index 3ae5b92a1f..31f846a6dc 100644 --- a/apps/web/src/types.ts +++ b/apps/web/src/types.ts @@ -381,10 +381,6 @@ export interface PetConfig { export interface AppConfig { mode: ExecMode; apiKey: string; - /** Non-secret daemon-owned credential profile selected for BYOK runs. */ - byokProfileId?: string; - byokCredentialConfigured?: boolean; - byokCredentialTail?: string; baseUrl: string; model: string; apiProtocol?: ApiProtocol; diff --git a/apps/web/tests/components/EntryShell.onboarding.test.tsx b/apps/web/tests/components/EntryShell.onboarding.test.tsx index 3efaae105f..6ce61df0f0 100644 --- a/apps/web/tests/components/EntryShell.onboarding.test.tsx +++ b/apps/web/tests/components/EntryShell.onboarding.test.tsx @@ -120,19 +120,6 @@ function renderOnboarding( onRenameProject: vi.fn(), onChangeDefaultDesignSystem: vi.fn(), onPersistComposioKey: vi.fn(), - onPersistByokCredential: vi.fn(async (input) => ({ - id: input.id ?? 'byok-onboarding-test', - label: input.label, - protocol: input.protocol, - baseUrl: input.baseUrl, - model: input.model, - apiVersion: input.apiVersion, - requiresApiKey: input.requiresApiKey ?? true, - configured: true, - keyTail: input.apiKey?.slice(-4), - createdAt: 1, - updatedAt: 1, - })), onOpenSettings: vi.fn(), onCompleteOnboarding: vi.fn(), ...overrides, @@ -1735,12 +1722,6 @@ describe('EntryShell onboarding Open Design AMR runtime', () => { expect(props.onModeChange).toHaveBeenCalledWith('api'); expect(props.onApiModelChange).toHaveBeenCalledWith('claude-opus-4-8'); - expect(props.onPersistByokCredential).toHaveBeenCalledWith(expect.objectContaining({ - protocol: 'anthropic', - apiKey: 'test-api-key', - baseUrl: 'https://api.anthropic.com', - model: 'claude-opus-4-8', - })); expect(props.onConfigPersist).toHaveBeenCalled(); await waitFor(() => { expect(props.onCompleteOnboarding).toHaveBeenCalledTimes(1); @@ -1748,13 +1729,10 @@ describe('EntryShell onboarding Open Design AMR runtime', () => { expect((props.onConfigPersist as ReturnType).mock.calls.at(-1)?.[0]).toMatchObject({ mode: 'api', apiProtocol: 'anthropic', - apiKey: '', + apiKey: 'test-api-key', baseUrl: 'https://api.anthropic.com', model: 'claude-opus-4-8', apiProviderBaseUrl: null, - byokProfileId: 'byok-onboarding-test', - byokCredentialConfigured: true, - byokCredentialTail: '-key', }); }); diff --git a/apps/web/tests/components/ProjectView.api-empty-response.test.tsx b/apps/web/tests/components/ProjectView.api-empty-response.test.tsx index 5413ef66d0..b4dc827a42 100644 --- a/apps/web/tests/components/ProjectView.api-empty-response.test.tsx +++ b/apps/web/tests/components/ProjectView.api-empty-response.test.tsx @@ -203,9 +203,7 @@ const mockedPlaySound = vi.mocked(playSound); const config: AppConfig = { mode: 'api', apiProtocol: 'openai', - apiKey: '', - byokProfileId: 'byok-test-profile', - byokCredentialConfigured: true, + apiKey: 'byok-test-key', baseUrl: 'https://api.deepseek.com', model: 'deepseek-chat', agentId: null, @@ -750,7 +748,12 @@ describe('ProjectView API empty response handling', () => { await waitFor(() => expect(capturedOptions.current).not.toBeNull()); expect(capturedOptions.current).toEqual(expect.objectContaining({ agentId: 'byok-opencode', - byokProfileId: 'byok-test-profile', + byokProvider: expect.objectContaining({ + protocol: 'openai', + apiKey: 'byok-test-key', + baseUrl: 'https://api.deepseek.com', + model: 'deepseek-chat', + }), byokMediaDefaults: { imageModel: 'gpt-image-2', speechModel: 'gpt-4o-mini-tts', diff --git a/apps/web/tests/components/ProjectView.pendingPrompt.test.tsx b/apps/web/tests/components/ProjectView.pendingPrompt.test.tsx index e72f104ad4..4b532d0e7a 100644 --- a/apps/web/tests/components/ProjectView.pendingPrompt.test.tsx +++ b/apps/web/tests/components/ProjectView.pendingPrompt.test.tsx @@ -217,9 +217,7 @@ const mockedSaveMessage = vi.mocked(saveMessage); const config: AppConfig = { mode: 'api', apiProtocol: 'openai', - apiKey: '', - byokProfileId: 'byok-test-profile', - byokCredentialConfigured: true, + apiKey: 'byok-test-key', baseUrl: 'https://api.openai.com/v1', model: 'api-model', agentId: null, diff --git a/apps/web/tests/components/ProjectView.run-isolation.test.tsx b/apps/web/tests/components/ProjectView.run-isolation.test.tsx index daf945cf02..b2ccb37268 100644 --- a/apps/web/tests/components/ProjectView.run-isolation.test.tsx +++ b/apps/web/tests/components/ProjectView.run-isolation.test.tsx @@ -469,8 +469,6 @@ vi.mock('../../src/components/ChatPane', () => ({ const config: AppConfig = { mode: 'daemon', apiKey: '', - byokProfileId: 'byok-test-profile', - byokCredentialConfigured: true, baseUrl: '', model: '', agentId: 'agent-1', @@ -1832,9 +1830,7 @@ describe('ProjectView conversation run isolation', () => { ...config, mode: 'api', apiProtocol: 'openai', - apiKey: '', - byokProfileId: 'byok-test-profile', - byokCredentialConfigured: true, + apiKey: 'byok-test-key', baseUrl: 'https://api.openai.com/v1', model: 'api-model', }); @@ -1847,7 +1843,12 @@ describe('ProjectView conversation run isolation', () => { await waitFor(() => expect(streamViaDaemon).toHaveBeenCalledTimes(1)); expect(streamViaDaemon).toHaveBeenCalledWith(expect.objectContaining({ agentId: 'byok-opencode', - byokProfileId: 'byok-test-profile', + byokProvider: expect.objectContaining({ + protocol: 'openai', + apiKey: 'byok-test-key', + baseUrl: 'https://api.openai.com/v1', + model: 'api-model', + }), model: 'api-model', })); await waitFor(() => expect(playSound).toHaveBeenCalledWith('success-sound')); @@ -1891,8 +1892,6 @@ describe('ProjectView conversation run isolation', () => { agentId, apiProtocol: 'openai', apiKey, - byokProfileId: undefined, - byokCredentialConfigured: false, baseUrl: 'https://api.openai.com/v1', model, }, @@ -1947,7 +1946,12 @@ describe('ProjectView conversation run isolation', () => { await waitFor(() => expect(streamViaDaemon).toHaveBeenCalledTimes(1)); expect(streamViaDaemon).toHaveBeenCalledWith(expect.objectContaining({ agentId: 'byok-opencode', - byokProfileId: 'byok-test-profile', + byokProvider: expect.objectContaining({ + protocol: 'ollama', + baseUrl: 'http://localhost:11434', + model: 'llama3.2', + requiresApiKey: false, + }), model: 'llama3.2', })); }); @@ -1974,7 +1978,12 @@ describe('ProjectView conversation run isolation', () => { await waitFor(() => expect(streamViaDaemon).toHaveBeenCalledTimes(1)); expect(streamViaDaemon).toHaveBeenCalledWith(expect.objectContaining({ agentId: 'byok-opencode', - byokProfileId: 'byok-test-profile', + byokProvider: expect.objectContaining({ + protocol: 'openai', + baseUrl: 'http://127.0.0.1:8000/v1', + model: 'model', + requiresApiKey: false, + }), model: 'model', })); }); diff --git a/apps/web/tests/components/SettingsDialog.execution.test.tsx b/apps/web/tests/components/SettingsDialog.execution.test.tsx index ff3cc62669..8708ea5c92 100644 --- a/apps/web/tests/components/SettingsDialog.execution.test.tsx +++ b/apps/web/tests/components/SettingsDialog.execution.test.tsx @@ -4,7 +4,6 @@ import { act, cleanup, fireEvent, render, screen, waitFor, within } from '@testi import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; import type { OpenDesignHostUpdaterStatusSnapshot } from '@open-design/host'; import { installMockOpenDesignHost } from '@open-design/host/testing'; -import type { UpsertByokCredentialProfileRequest } from '@open-design/contracts'; import { en } from '../../src/i18n/locales/en'; function optionNames(container: HTMLElement): string[] { @@ -112,7 +111,6 @@ import { reconcileAmrProfileEnv } from '../../src/components/SettingsDialog'; import { providerModelsCacheKey } from '../../src/components/providerModelsCache'; import { I18nProvider } from '../../src/i18n'; import { LOCALES } from '../../src/i18n/types'; -import { ByokCredentialProfileHttpError } from '../../src/state/config'; import { MAX_MAX_TOKENS, MIN_MAX_TOKENS } from '../../src/state/maxTokens'; import type { AgentInfo, @@ -286,19 +284,6 @@ function renderSettingsDialog( ) { const onPersist = vi.fn(); const onPersistComposioKey = vi.fn(); - const onPersistByokCredential = vi.fn(async (input: UpsertByokCredentialProfileRequest) => ({ - id: input.id ?? 'byok-test-profile', - label: input.label, - protocol: input.protocol, - baseUrl: input.baseUrl, - model: input.model, - apiVersion: input.apiVersion, - requiresApiKey: input.requiresApiKey ?? true, - configured: true, - keyTail: input.apiKey?.slice(-4), - createdAt: 1, - updatedAt: 1, - })); const onSilentUpdatePreferenceChange: (allowSilentUpdates: boolean) => Promise = options.onSilentUpdatePreferenceChange ?? (async () => undefined); @@ -317,7 +302,6 @@ function renderSettingsDialog( onPersist={onPersist} onSilentUpdatePreferenceChange={onSilentUpdatePreferenceChange} onPersistComposioKey={onPersistComposioKey} - onPersistByokCredential={onPersistByokCredential} onClose={onClose} onRefreshAgents={onRefreshAgents} />, @@ -327,7 +311,6 @@ function renderSettingsDialog( onPersist, onSilentUpdatePreferenceChange, onPersistComposioKey, - onPersistByokCredential, onClose, onRefreshAgents, ...view, @@ -2395,60 +2378,6 @@ describe('SettingsDialog execution settings BYOK interactions', () => { ); }); - it('reports secure profile persistence failures with stable BYOK telemetry', async () => { - const fetchMock = vi.fn(async (input: RequestInfo | URL) => { - const url = input.toString(); - if (url === '/api/memory') { - return new Response( - JSON.stringify({ enabled: true, memories: [], extraction: null }), - { status: 200, headers: { 'content-type': 'application/json' } }, - ); - } - expect(url).toBe('/api/test/connection'); - return new Response( - JSON.stringify({ - ok: true, - kind: 'ok', - latencyMs: 20, - model: 'claude-sonnet-4-5', - sample: 'pong', - }), - { status: 200, headers: { 'content-type': 'application/json' } }, - ); - }); - vi.stubGlobal('fetch', fetchMock); - - const { onPersistByokCredential } = renderSettingsDialog({ - apiKey: 'sk-ant-test-provider', - }); - onPersistByokCredential.mockRejectedValueOnce( - new ByokCredentialProfileHttpError( - 400, - 'Invalid secure profile', - 'VALIDATION_FAILED', - ), - ); - - fireEvent.click(screen.getByRole('button', { name: 'Test' })); - - await waitFor(() => { - expect(analyticsTrackMock).toHaveBeenCalledWith( - 'settings_byok_test_result', - expect.objectContaining({ - page_name: 'settings', - area: 'execution_model', - provider_id: 'anthropic', - result: 'failed', - error_code: 'VALIDATION_FAILED', - error_kind: 'unknown', - field_missing: 'none', - success_after_action: false, - }), - undefined, - ); - }); - }); - it('renders invalid Base URL test failures on the Base URL field', async () => { const fetchMock = vi.fn(async (input: RequestInfo | URL) => { const url = input.toString(); diff --git a/apps/web/tests/providers/connection-test.test.ts b/apps/web/tests/providers/connection-test.test.ts index 4f6fdf1fe0..55e9af8107 100644 --- a/apps/web/tests/providers/connection-test.test.ts +++ b/apps/web/tests/providers/connection-test.test.ts @@ -6,7 +6,6 @@ import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; import { testAgent, testApiProvider, - testSavedByokProfile, } from '../../src/providers/connection-test'; import type { ConnectionTestResponse } from '../../src/types'; @@ -143,24 +142,3 @@ describe('testAgent', () => { }); }); }); - -describe('testSavedByokProfile', () => { - it('tests the encoded profile reference without sending a raw key', async () => { - const fetchMock = vi.fn().mockResolvedValueOnce(jsonResponse({ - ok: true, - kind: 'success', - latencyMs: 12, - model: 'openrouter/free', - })); - globalThis.fetch = fetchMock as typeof fetch; - - const result = await testSavedByokProfile('byok-openrouter/profile'); - - expect(result.ok).toBe(true); - expect(fetchMock).toHaveBeenCalledWith( - '/api/byok/profiles/byok-openrouter%2Fprofile/test', - { method: 'POST', signal: undefined }, - ); - expect(JSON.stringify(fetchMock.mock.calls)).not.toContain('apiKey'); - }); -}); diff --git a/apps/web/tests/providers/sse.test.ts b/apps/web/tests/providers/sse.test.ts index cd35c633ca..f69340b6cc 100644 --- a/apps/web/tests/providers/sse.test.ts +++ b/apps/web/tests/providers/sse.test.ts @@ -70,7 +70,7 @@ describe('streamViaDaemon', () => { expect(body.currentPrompt).toBe('post-consent revision'); }); - it('sends BYOK profile references without a raw provider payload', async () => { + it('sends the selected Local BYOK provider only to the local run endpoint', async () => { const handlers = createDaemonHandlers(); const fetchMock = vi.fn(async (input: RequestInfo | URL) => { const url = String(input); @@ -84,7 +84,12 @@ describe('streamViaDaemon', () => { await streamViaDaemon({ agentId: 'byok-opencode', - byokProfileId: 'byok-openrouter', + byokProvider: { + protocol: 'openai', + apiKey: 'local-test-key', + baseUrl: 'https://api.openai.com/v1', + model: 'gpt-5.4-mini', + }, handlers, history: [{ id: '1', role: 'user', content: 'Create a site' }], signal: new AbortController().signal, @@ -95,8 +100,13 @@ describe('streamViaDaemon', () => { RequestInit, ]; const body = JSON.parse(String(createRunInit.body)); - expect(body.byokProfileId).toBe('byok-openrouter'); - expect(body).not.toHaveProperty('byokProvider'); + expect(body).not.toHaveProperty('byokProfileId'); + expect(body.byokProvider).toEqual({ + protocol: 'openai', + apiKey: 'local-test-key', + baseUrl: 'https://api.openai.com/v1', + model: 'gpt-5.4-mini', + }); }); it('publishes an authoritative successful run with an artifact to the app gate', async () => { diff --git a/apps/web/tests/state/config.test.ts b/apps/web/tests/state/config.test.ts index 663b449df0..38db36b7b0 100644 --- a/apps/web/tests/state/config.test.ts +++ b/apps/web/tests/state/config.test.ts @@ -1,24 +1,16 @@ import { afterEach, describe, expect, it, vi } from 'vitest'; import { - ByokCredentialProfileHttpError, - ByokCredentialProfileNetworkError, buildMediaProvidersForDaemonSave, BYOK_PROVIDER_PRESETS, - classifyByokCredentialProfileFailure, DEFAULT_CONFIG, defaultKnownProviderModel, - fetchByokCredentialProfilesFromDaemon, fetchMediaProvidersFromDaemon, isStoredMediaProviderEntryEmpty, isStoredMediaProviderEntryPresent, KNOWN_PROVIDERS, - legacyByokMigrationErrorPresentation, loadConfig, - migrateLegacyByokCredentialsToDaemon, mergeDaemonConfig, - mergeByokCredentialProfiles, mergeDaemonMediaProviders, - persistByokCredentialProfileToDaemon, saveConfig, shouldSyncLocalMediaProvidersToDaemon, syncComposioConfigToDaemon, @@ -1109,7 +1101,7 @@ describe('loadConfig', () => { const config = loadConfig(); expect(config.apiProtocol).toBe('openai'); - expect(config.apiKey).toBe(''); + expect(config.apiKey).toBe('sk-proxy'); expect(config.apiVersion).toBe('2024-01-01'); expect(config.baseUrl).toBe('https://proxy.example.com/bedrock-runtime/v1'); expect(config.model).toBe('gpt-4o'); @@ -1196,7 +1188,7 @@ describe('loadConfig', () => { expect(config.apiProviderBaseUrl).toBe(DEFAULT_CONFIG.apiProviderBaseUrl); expect(config.apiProtocolConfigs?.bedrock).toBeUndefined(); expect(config.apiProtocolConfigs?.openai).toEqual({ - apiKey: '', + apiKey: 'sk-openai', baseUrl: 'https://api.openai.com/v1', model: 'gpt-4o', }); @@ -1204,15 +1196,11 @@ describe('loadConfig', () => { const persisted = JSON.parse( store.get('open-design:config') ?? '{}', ) as Partial; - expect(persisted.apiProtocol).toBe('bedrock'); - expect(persisted.apiKey).toBe('bedrock-secret'); - expect(persisted.apiVersion).toBe('bedrock-2023-05-31'); - expect(persisted.baseUrl).toBe( - 'https://bedrock-runtime.us-east-1.amazonaws.com', - ); - expect(persisted.apiProtocolConfigs?.bedrock?.apiKey).toBe( - 'nested-bedrock-secret', - ); + expect(persisted.apiProtocol).toBe('anthropic'); + expect(persisted.apiKey).toBe(''); + expect(persisted.apiVersion).toBe(''); + expect(persisted.baseUrl).toBe(DEFAULT_CONFIG.baseUrl); + expect(persisted.apiProtocolConfigs?.bedrock).toBeUndefined(); expect(persisted.apiProtocolConfigs?.openai).toEqual({ apiKey: 'sk-openai', baseUrl: 'https://api.openai.com/v1', @@ -1331,7 +1319,7 @@ describe('loadConfig', () => { const config = loadConfig(); expect(config.mode).toBe('api'); - expect(config.apiKey).toBe(''); + expect(config.apiKey).toBe('sk-test'); expect(config.baseUrl).toBe('https://[broken-ipv6'); expect(config.model).toBe('custom-model'); expect(config.apiProtocol).toBe('anthropic'); @@ -1386,8 +1374,8 @@ describe('loadConfig', () => { }); describe('saveConfig', () => { - it('keeps every BYOK API-key projection out of localStorage while preserving a secure profile reference', () => { - saveConfig({ + it('persists Local BYOK API keys while removing unpublished secure-profile metadata', () => { + store.set('open-design:config', JSON.stringify({ ...DEFAULT_CONFIG, mode: 'api', apiKey: 'top-level-secret', @@ -1409,18 +1397,24 @@ describe('saveConfig', () => { }, }, }, - }); + })); + + const loaded = loadConfig(); + expect(loaded.apiKey).toBe('top-level-secret'); + expect(loaded.apiProtocolConfigs?.openai?.apiKey).toBe('protocol-secret'); + expect(loaded.byokProviderConfigDrafts?.openrouter?.apiConfig.apiKey).toBe('draft-secret'); + saveConfig(loaded); const raw = store.get('open-design:config') ?? ''; const saved = JSON.parse(raw); - expect(raw).not.toContain('top-level-secret'); - expect(raw).not.toContain('protocol-secret'); - expect(raw).not.toContain('draft-secret'); - expect(saved.apiKey).toBe(''); - expect(saved.apiProtocolConfigs.openai.apiKey).toBe(''); - expect(saved.byokProviderConfigDrafts.openrouter.apiConfig.apiKey).toBe(''); - expect(saved.byokProfileId).toBe('byok-openrouter-1'); - expect(saved.byokCredentialConfigured).toBe(true); + expect(raw).toContain('top-level-secret'); + expect(raw).toContain('protocol-secret'); + expect(raw).toContain('draft-secret'); + expect(saved.apiKey).toBe('top-level-secret'); + expect(saved.apiProtocolConfigs.openai.apiKey).toBe('protocol-secret'); + expect(saved.byokProviderConfigDrafts.openrouter.apiConfig.apiKey).toBe('draft-secret'); + expect(saved.byokProfileId).toBeUndefined(); + expect(saved.byokCredentialConfigured).toBeUndefined(); }); it('keeps daemon-owned privacy fields out of localStorage', () => { @@ -1477,360 +1471,3 @@ describe('saveConfig', () => { }); }); }); - -describe('secure BYOK profiles', () => { - afterEach(() => { - vi.stubGlobal('fetch', originalFetch); - }); - - it('persists a key only through the daemon profile endpoint and returns non-secret metadata', async () => { - const fetchMock = vi.fn(async (_input: RequestInfo | URL, init?: RequestInit) => { - const request = JSON.parse(String(init?.body)); - expect(request.apiKey).toBe('draft-secret'); - return Response.json({ - profile: { - id: 'byok-openrouter-1', - label: request.label, - protocol: request.protocol, - baseUrl: request.baseUrl, - model: request.model, - requiresApiKey: true, - configured: true, - keyTail: 'cret', - createdAt: 1, - updatedAt: 1, - }, - }); - }); - vi.stubGlobal('fetch', fetchMock); - - const profile = await persistByokCredentialProfileToDaemon({ - label: 'OpenRouter', - protocol: 'openai', - baseUrl: 'https://openrouter.ai/api/v1', - model: 'openrouter/free', - apiKey: 'draft-secret', - }); - - expect(fetchMock).toHaveBeenCalledWith( - '/api/byok/profiles', - expect.objectContaining({ method: 'POST' }), - ); - expect(profile).not.toHaveProperty('apiKey'); - expect(profile.id).toBe('byok-openrouter-1'); - }); - - it('removes legacy browser credentials only after secure profile migration succeeds', async () => { - const persisted: Partial = { - ...DEFAULT_CONFIG, - mode: 'api', - apiKey: 'legacy-secret', - apiProtocol: 'openai', - baseUrl: 'https://openrouter.ai/api/v1', - model: 'openai/gpt-5.4', - apiProtocolConfigs: { - openai: { - apiKey: 'legacy-secret', - baseUrl: 'https://openrouter.ai/api/v1', - model: 'openai/gpt-5.4', - }, - }, - }; - store.set('open-design:config', JSON.stringify(persisted)); - const loaded = loadConfig(); - const persistProfile = vi.fn(async (input) => ({ - id: input.id ?? 'missing', - label: input.label, - protocol: input.protocol, - baseUrl: input.baseUrl, - model: input.model, - apiVersion: input.apiVersion, - requiresApiKey: true, - configured: true, - keyTail: 'cret', - createdAt: 1, - updatedAt: 1, - })); - - expect(loaded.apiKey).toBe(''); - expect(store.get('open-design:config')).toContain('legacy-secret'); - - const result = await migrateLegacyByokCredentialsToDaemon( - loaded, - persistProfile, - ); - - expect(result.status).toBe('migrated'); - expect(persistProfile).toHaveBeenCalledTimes(1); - expect(persistProfile).toHaveBeenCalledWith(expect.objectContaining({ - id: expect.stringMatching(/^byok-legacy-/), - apiKey: 'legacy-secret', - protocol: 'openai', - })); - expect(result.config.apiKey).toBe(''); - expect(result.config.byokProfileId).toMatch(/^byok-legacy-/); - expect(store.get('open-design:config')).not.toContain('legacy-secret'); - }); - - it('preserves legacy browser credentials when secure profile migration fails', async () => { - const persisted: Partial = { - ...DEFAULT_CONFIG, - mode: 'api', - apiKey: 'legacy-secret', - apiProtocol: 'openai', - baseUrl: 'https://openrouter.ai/api/v1', - model: 'openai/gpt-5.4', - }; - store.set('open-design:config', JSON.stringify(persisted)); - const loaded = loadConfig(); - - const result = await migrateLegacyByokCredentialsToDaemon( - loaded, - async () => { - throw new Error('secure backend unavailable'); - }, - ); - - expect(result).toMatchObject({ - status: 'failed', - error: { message: 'secure backend unavailable' }, - }); - expect(result.config.apiKey).toBe(''); - expect(result.config.byokProfileId).toBeUndefined(); - expect(store.get('open-design:config')).toContain('legacy-secret'); - - saveConfig({ - ...result.config, - theme: 'dark', - mode: 'daemon', - agentId: 'codex', - }); - const saved = JSON.parse(store.get('open-design:config') ?? '{}'); - expect(saved.apiKey).toBe('legacy-secret'); - expect(saved.theme).toBe('dark'); - expect(saved.mode).toBe('daemon'); - expect(saved.agentId).toBe('codex'); - }); - - it('surfaces the daemon validation reason and preserves legacy credentials when migration gets a 400', async () => { - const validationMessage = - 'BYOK baseUrl must be an absolute HTTP(S) URL without credentials, query, or fragment.'; - const persisted: Partial = { - ...DEFAULT_CONFIG, - mode: 'api', - apiKey: 'legacy-secret', - apiProtocol: 'openai', - baseUrl: 'not-an-absolute-url', - model: 'openai/gpt-5.4', - }; - store.set('open-design:config', JSON.stringify(persisted)); - const loaded = loadConfig(); - const fetchMock = vi.fn(async (_input: RequestInfo | URL, init?: RequestInit) => { - expect(JSON.parse(String(init?.body))).toMatchObject({ - apiKey: 'legacy-secret', - baseUrl: 'not-an-absolute-url', - }); - return Response.json({ - error: { - code: 'VALIDATION_FAILED', - message: validationMessage, - }, - }, { status: 400 }); - }); - vi.stubGlobal('fetch', fetchMock); - - const result = await migrateLegacyByokCredentialsToDaemon(loaded); - - expect(result).toMatchObject({ - status: 'failed', - error: { - name: 'ByokCredentialProfileHttpError', - status: 400, - code: 'VALIDATION_FAILED', - message: validationMessage, - }, - }); - if (result.status !== 'failed') throw new Error('expected migration failure'); - expect(result.error).toBeInstanceOf(ByokCredentialProfileHttpError); - expect(legacyByokMigrationErrorPresentation( - result.error, - 'Local daemon may be offline.', - )).toEqual({ message: validationMessage }); - expect(fetchMock).toHaveBeenCalledWith( - '/api/byok/profiles', - expect.objectContaining({ method: 'POST' }), - ); - expect(store.get('open-design:config')).toContain('legacy-secret'); - }); - - it('keeps the HTTP status fallback when a daemon error body is not structured JSON', async () => { - vi.stubGlobal('fetch', vi.fn(async () => new Response( - 'bad gateway', - { status: 502, headers: { 'content-type': 'text/html' } }, - ))); - - const promise = persistByokCredentialProfileToDaemon({ - label: 'OpenRouter', - protocol: 'openai', - baseUrl: 'https://openrouter.ai/api/v1', - model: 'openrouter/free', - apiKey: 'draft-secret', - }); - - await expect(promise).rejects.toMatchObject({ - name: 'ByokCredentialProfileHttpError', - status: 502, - message: 'Failed to save BYOK credential (502)', - }); - }); - - it('classifies fetch rejection separately from daemon HTTP responses', async () => { - vi.stubGlobal('fetch', vi.fn(async () => { - throw new TypeError('Failed to fetch'); - })); - - const promise = persistByokCredentialProfileToDaemon({ - label: 'OpenRouter', - protocol: 'openai', - baseUrl: 'https://openrouter.ai/api/v1', - model: 'openrouter/free', - apiKey: 'draft-secret', - }); - - const error = await promise.catch((cause: unknown) => cause); - expect(error).toBeInstanceOf(ByokCredentialProfileNetworkError); - expect(error).toMatchObject({ - name: 'ByokCredentialProfileNetworkError', - message: 'Failed to fetch', - }); - if (!(error instanceof Error)) throw new Error('expected network error'); - expect(legacyByokMigrationErrorPresentation( - error, - 'Local daemon may be offline.', - )).toEqual({ - message: 'Local daemon may be offline.', - details: 'Failed to fetch', - }); - }); - - it('classifies secure profile persistence failures with stable telemetry values', () => { - expect(classifyByokCredentialProfileFailure( - new ByokCredentialProfileHttpError( - 400, - 'Invalid secure profile', - 'VALIDATION_FAILED', - ), - )).toEqual({ - errorCode: 'VALIDATION_FAILED', - errorKind: 'unknown', - }); - expect(classifyByokCredentialProfileFailure( - new ByokCredentialProfileHttpError(502, 'Bad gateway'), - )).toEqual({ - errorCode: 'HTTP_502', - errorKind: 'unknown', - }); - expect(classifyByokCredentialProfileFailure( - new ByokCredentialProfileNetworkError( - 'Failed to fetch', - new TypeError('Failed to fetch'), - ), - )).toEqual({ - errorCode: 'DAEMON_UNREACHABLE', - errorKind: 'unknown', - }); - expect(classifyByokCredentialProfileFailure( - new TypeError('Test request failed'), - )).toEqual({ - errorCode: 'TypeError', - errorKind: 'TypeError', - }); - }); - - it('hydrates only an explicitly selected secure profile reference', async () => { - vi.stubGlobal('fetch', vi.fn(async () => Response.json({ - available: true, - backend: 'macos-keychain', - profiles: [{ - id: 'byok-openrouter-1', - label: 'OpenRouter', - protocol: 'openai', - baseUrl: 'https://openrouter.ai/api/v1', - model: 'openrouter/free', - requiresApiKey: true, - configured: true, - keyTail: '1234', - createdAt: 1, - updatedAt: 1, - }], - }))); - - const response = await fetchByokCredentialProfilesFromDaemon(); - const merged = mergeByokCredentialProfiles({ - ...DEFAULT_CONFIG, - byokProfileId: 'byok-openrouter-1', - }, response); - - expect(merged).toMatchObject({ - byokProfileId: 'byok-openrouter-1', - byokCredentialConfigured: true, - byokCredentialTail: '1234', - }); - expect(merged.apiKey).toBe(''); - }); - - it('uses daemon profile metadata as authoritative after a CLI update', () => { - const merged = mergeByokCredentialProfiles({ - ...DEFAULT_CONFIG, - apiKey: '', - apiProtocol: 'anthropic', - apiVersion: 'stale-version', - baseUrl: 'https://stale.example/v1', - model: 'stale-model', - byokProfileId: 'byok-openrouter-1', - apiProtocolConfigs: { - openai: { - apiKey: '', - apiVersion: 'stale-version', - baseUrl: 'https://stale.example/v1', - model: 'stale-model', - }, - }, - }, { - available: true, - backend: 'macos-keychain', - profiles: [{ - id: 'byok-openrouter-1', - label: 'OpenRouter', - protocol: 'openai', - baseUrl: 'https://openrouter.ai/api/v1', - model: 'openai/gpt-5.4', - apiVersion: '2026-07-01', - requiresApiKey: true, - configured: true, - keyTail: '1234', - createdAt: 1, - updatedAt: 2, - }], - }); - - expect(merged).toMatchObject({ - apiProtocol: 'openai', - apiProviderBaseUrl: 'https://openrouter.ai/api/v1', - apiVersion: '2026-07-01', - baseUrl: 'https://openrouter.ai/api/v1', - model: 'openai/gpt-5.4', - byokCredentialConfigured: true, - byokCredentialTail: '1234', - apiProtocolConfigs: { - openai: { - apiKey: '', - apiProviderBaseUrl: 'https://openrouter.ai/api/v1', - apiVersion: '2026-07-01', - baseUrl: 'https://openrouter.ai/api/v1', - model: 'openai/gpt-5.4', - }, - }, - }); - }); -}); diff --git a/e2e/ui/amr-onboarding.test.ts b/e2e/ui/amr-onboarding.test.ts index a3cbc0e99a..2ab90499c3 100644 --- a/e2e/ui/amr-onboarding.test.ts +++ b/e2e/ui/amr-onboarding.test.ts @@ -17,9 +17,6 @@ type OnboardingConfig = { apiProtocol?: string; baseUrl: string; model: string; - byokProfileId?: string; - byokCredentialConfigured?: boolean; - byokCredentialTail?: string; agentId: string | null; skillId: null; designSystemId: null; @@ -624,12 +621,9 @@ test('[P0] @critical onboarding BYOK path can fetch models, test the provider, a await expectOnboardingFinished(page); await pollStoredConfig(page).toMatchObject({ mode: 'api', - apiKey: '', + apiKey: 'test-api-key', baseUrl: 'https://api.anthropic.com', model: 'claude-opus-4-8', - byokProfileId: 'byok-onboarding-1', - byokCredentialConfigured: true, - byokCredentialTail: '-key', onboardingCompleted: true, }); }); @@ -753,12 +747,9 @@ test('[P0] onboarding BYOK path supports Anthropic model selection and API key v await pollStoredConfig(page).toMatchObject({ mode: 'api', apiProtocol: 'anthropic', - apiKey: '', + apiKey: 'anthropic-test-key', baseUrl: 'https://api.anthropic.com', model: 'claude-sonnet-4-5', - byokProfileId: 'byok-onboarding-1', - byokCredentialConfigured: true, - byokCredentialTail: '-key', onboardingCompleted: true, }); }); @@ -919,7 +910,6 @@ async function wireOnboardingMocks( let loginCalls = 0; let cancelCalls = 0; let authAttemptId: string | null = null; - let byokProfile: Record | null = null; await page.route('**/api/health', async (route) => { await route.fulfill({ status: 200, contentType: 'application/json', body: '{"ok":true}' }); @@ -946,38 +936,6 @@ async function wireOnboardingMocks( await route.continue(); }); - await page.route('**/api/byok/profiles', async (route) => { - if (route.request().method() === 'GET') { - await route.fulfill({ - json: { - available: true, - backend: 'test', - profiles: byokProfile ? [byokProfile] : [], - }, - }); - return; - } - if (route.request().method() === 'POST') { - const request = route.request().postDataJSON() as Record; - const apiKey = String(request.apiKey ?? ''); - byokProfile = { - id: String(request.id ?? 'byok-onboarding-1'), - label: String(request.label ?? 'Anthropic'), - protocol: String(request.protocol ?? 'anthropic'), - baseUrl: String(request.baseUrl ?? ''), - model: String(request.model ?? ''), - requiresApiKey: request.requiresApiKey !== false, - configured: true, - keyTail: apiKey.slice(-4), - createdAt: 1, - updatedAt: 1, - }; - await route.fulfill({ json: { profile: byokProfile } }); - return; - } - await route.continue(); - }); - const localAgents = options.localAgents ?? [{ id: 'codex', name: 'Codex CLI', diff --git a/e2e/ui/api-empty-response.test.ts b/e2e/ui/api-empty-response.test.ts index fe5a84d46d..bca4e43045 100644 --- a/e2e/ui/api-empty-response.test.ts +++ b/e2e/ui/api-empty-response.test.ts @@ -15,12 +15,9 @@ test.beforeEach(async ({ page }) => { JSON.stringify({ mode: 'api', apiProtocol: 'openai', - apiKey: '', + apiKey: 'test-byok-key', baseUrl: 'https://api.deepseek.com', model: 'deepseek-v4-flash', - byokProfileId: 'byok-api-empty-response', - byokCredentialConfigured: true, - byokCredentialTail: 'test', agentId: null, skillId: null, designSystemId: null, @@ -63,26 +60,6 @@ test.beforeEach(async ({ page }) => { }, ]); }); - await page.route('**/api/byok/profiles', async (route) => { - await route.fulfill({ - json: { - available: true, - backend: 'test', - profiles: [{ - id: 'byok-api-empty-response', - label: 'DeepSeek', - protocol: 'openai', - baseUrl: 'https://api.deepseek.com', - model: 'deepseek-v4-flash', - requiresApiKey: true, - configured: true, - keyTail: 'test', - createdAt: 1, - updatedAt: 1, - }], - }, - }); - }); }); test('[P0] @critical API empty stream shows No output instead of Done', async ({ page }) => { diff --git a/e2e/ui/project-management-flows.test.ts b/e2e/ui/project-management-flows.test.ts index ce94e9ac1d..ff457d77b4 100644 --- a/e2e/ui/project-management-flows.test.ts +++ b/e2e/ui/project-management-flows.test.ts @@ -23,32 +23,6 @@ async function stubCatalogsEmpty(page: Page): Promise { const STORAGE_KEY = 'open-design:config'; const ACTIVE_ARTIFACT_PREVIEW_SELECTOR = '[data-testid="artifact-preview-frame"]:visible, [data-testid="artifact-preview-frame-url-load"]:visible, [data-testid="artifact-preview-frame-srcdoc"]:visible, [data-testid="live-artifact-preview-frame"]:visible'; -async function routeByokProfile( - page: Page, - config: Record, -): Promise { - await page.route('**/api/byok/profiles', async (route) => { - await route.fulfill({ - json: { - available: true, - backend: 'test', - profiles: [{ - id: config.byokProfileId, - label: 'OpenAI', - protocol: config.apiProtocol, - baseUrl: config.baseUrl, - model: config.model, - requiresApiKey: true, - configured: true, - keyTail: config.byokCredentialTail, - createdAt: 1, - updatedAt: 1, - }], - }, - }); - }); -} - const AGENTS = [ { id: 'codex', @@ -1382,15 +1356,12 @@ test('[P0] @critical project detail composer BYOK model switch persists from the test.setTimeout(60_000); const config = { mode: 'daemon', - apiKey: '', + apiKey: 'test-byok-key', apiProtocol: 'openai', apiVersion: '', baseUrl: 'https://api.openai.com/v1', model: 'gpt-4o-2024-05-13', apiProviderBaseUrl: 'https://api.openai.com/v1', - byokProfileId: 'byok-project-model-switch', - byokCredentialConfigured: true, - byokCredentialTail: 'test', agentId: 'codex', skillId: null, designSystemId: null, @@ -1424,7 +1395,6 @@ test('[P0] @critical project detail composer BYOK model switch persists from the body: JSON.stringify({ config }), }); }); - await routeByokProfile(page, config); await page.goto('/'); await createProject(page, 'Composer BYOK model switch'); @@ -1456,15 +1426,12 @@ test('[P0] @critical project detail composer keeps Local CLI and BYOK model choi test.setTimeout(60_000); const config = { mode: 'daemon', - apiKey: '', + apiKey: 'test-byok-key', apiProtocol: 'openai', apiVersion: '', baseUrl: 'https://api.openai.com/v1', model: 'gpt-4o-2024-05-13', apiProviderBaseUrl: 'https://api.openai.com/v1', - byokProfileId: 'byok-project-model-isolation', - byokCredentialConfigured: true, - byokCredentialTail: 'test', agentId: 'codex', skillId: null, designSystemId: null, @@ -1498,7 +1465,6 @@ test('[P0] @critical project detail composer keeps Local CLI and BYOK model choi body: JSON.stringify({ config }), }); }); - await routeByokProfile(page, config); await page.goto('/'); await createProject(page, 'Composer model mode isolation'); diff --git a/e2e/ui/settings-api-protocol.test.ts b/e2e/ui/settings-api-protocol.test.ts index d985c130a7..f5ec9380e2 100644 --- a/e2e/ui/settings-api-protocol.test.ts +++ b/e2e/ui/settings-api-protocol.test.ts @@ -135,28 +135,6 @@ async function openExecutionSettingsWithAgents( await page.route('**/api/health', async (route) => { await route.fulfill({ status: 200, contentType: 'application/json', body: '{"ok":true}' }); }); - if (typeof config.byokProfileId === 'string') { - await page.route('**/api/byok/profiles', async (route) => { - await route.fulfill({ - json: { - available: true, - backend: 'test', - profiles: [{ - id: config.byokProfileId, - label: 'OpenAI', - protocol: config.apiProtocol ?? 'openai', - baseUrl: config.baseUrl ?? '', - model: config.model ?? '', - requiresApiKey: true, - configured: true, - keyTail: config.byokCredentialTail ?? 'test', - createdAt: 1, - updatedAt: 1, - }], - }, - }); - }); - } await routeAgents(page, agents); await gotoEntryHome(page); @@ -275,7 +253,7 @@ test('[P0] @critical BYOK quick fill provider updates fields and saved settings .toMatchObject({ mode: 'api', apiProtocol: 'openai', - apiKey: '', + apiKey: 'sk-openai-test', baseUrl: 'https://api.deepseek.com', model: 'deepseek-v4-flash', apiProviderBaseUrl: 'https://api.deepseek.com', @@ -288,7 +266,7 @@ test('[P0] @critical BYOK quick fill provider updates fields and saved settings expect(savedConfig).toMatchObject({ mode: 'api', apiProtocol: 'openai', - apiKey: '', + apiKey: 'sk-openai-test', baseUrl: 'https://api.deepseek.com', model: 'deepseek-v4-flash', apiProviderBaseUrl: 'https://api.deepseek.com', @@ -473,7 +451,7 @@ test('[P1] BYOK connection test surfaces NVIDIA degraded provider detail', async ); }); -test('[P0] BYOK save stays disabled until required fields are valid', async ({ page }) => { +test('[P0] BYOK autosave waits until required fields are valid', async ({ page }) => { await openExecutionSettings(page, { mode: 'api', apiKey: '', @@ -496,7 +474,7 @@ test('[P0] BYOK save stays disabled until required fields are valid', async ({ p await expect(closeButton).toBeEnabled(); await dialog.getByLabel('API key').fill('sk-openai-test'); - await expect.poll(async () => readSavedConfig(page)).toMatchObject({ apiKey: '' }); + await expect.poll(async () => readSavedConfig(page)).toMatchObject({ apiKey: 'sk-openai-test' }); const baseUrlInput = dialog.getByLabel('Base URL'); // A non-http scheme is still rejected client-side. (An internal-IP URL is no @@ -507,7 +485,7 @@ test('[P0] BYOK save stays disabled until required fields are valid', async ({ p await baseUrlInput.fill('http://localhost:11434/v1'); await expect.poll(async () => readSavedConfig(page)).toMatchObject({ - apiKey: '', + apiKey: 'sk-openai-test', baseUrl: 'http://localhost:11434/v1', }); }); @@ -681,7 +659,7 @@ test('[P0] @critical BYOK clearing the API key restores the suggested OpenAI mod await apiKeyInput.blur(); await expect.poll(() => providerModelRequests.length).toBe(1); await expect.poll(async () => readSavedConfig(page)).toMatchObject({ - apiKey: '', + apiKey: 'sk-openai-test', }); await modelSelect.click(); @@ -1034,15 +1012,12 @@ test('[P0] @critical Settings keeps Local CLI and BYOK model choices isolated af page, { mode: 'api', - apiKey: '', + apiKey: 'sk-openai-test', apiProtocol: 'openai', apiVersion: '', baseUrl: 'https://api.openai.com/v1', model: 'gpt-4o-mini', apiProviderBaseUrl: 'https://api.openai.com/v1', - byokProfileId: 'byok-settings-model-isolation', - byokCredentialConfigured: true, - byokCredentialTail: 'test', agentId: null, skillId: null, designSystemId: null, diff --git a/packages/contracts/src/api/byok-credentials.ts b/packages/contracts/src/api/byok-credentials.ts deleted file mode 100644 index 4e8f7909fc..0000000000 --- a/packages/contracts/src/api/byok-credentials.ts +++ /dev/null @@ -1,40 +0,0 @@ -import type { ByokChatProtocol } from './chat.js'; - -export interface ByokCredentialProfile { - id: string; - label: string; - protocol: ByokChatProtocol; - baseUrl: string; - model: string; - apiVersion?: string; - requiresApiKey: boolean; - configured: boolean; - keyTail?: string; - createdAt: number; - updatedAt: number; -} - -export interface UpsertByokCredentialProfileRequest { - id?: string; - label: string; - protocol: ByokChatProtocol; - baseUrl: string; - model: string; - apiVersion?: string; - requiresApiKey?: boolean; - /** - * Accepted only by the loopback daemon write endpoint. Never returned, - * persisted in profile metadata, accepted by run APIs, or placed in a URL. - */ - apiKey?: string; -} - -export interface ByokCredentialProfilesResponse { - available: boolean; - backend: string; - profiles: ByokCredentialProfile[]; -} - -export interface ByokCredentialProfileResponse { - profile: ByokCredentialProfile; -} diff --git a/packages/contracts/src/api/chat.ts b/packages/contracts/src/api/chat.ts index dff49bf2c6..e48bb7aaa0 100644 --- a/packages/contracts/src/api/chat.ts +++ b/packages/contracts/src/api/chat.ts @@ -95,10 +95,11 @@ export interface ChatRequest { reasoning?: string | null; serviceTier?: string | null; /** - * Non-secret reference to a daemon-owned BYOK profile. Runtime credentials - * are resolved from OS secure storage immediately before child spawn. + * Run-scoped BYOK provider credentials for the daemon-backed OpenCode + * adapter. The daemon must not persist this object; it is translated into + * child env + OPENCODE_CONFIG_CONTENT for the current run only. */ - byokProfileId?: string; + byokProvider?: ByokChatProviderConfig; /** * Run-scoped BYOK media defaults selected in the chat UI. The daemon uses * these to guide OpenCode-backed `od media generate` calls for this run only. diff --git a/packages/contracts/src/index.ts b/packages/contracts/src/index.ts index d48253a585..ddf3b35108 100644 --- a/packages/contracts/src/index.ts +++ b/packages/contracts/src/index.ts @@ -9,7 +9,6 @@ export * from './api/automations.js'; export * from './api/artifacts.js'; export * from './api/brands.js'; export * from './api/brief.js'; -export * from './api/byok-credentials.js'; export * from './api/chat.js'; export * from './api/community.js'; export * from './api/context.js'; diff --git a/tools/pack/tests/mac-prebundle.test.ts b/tools/pack/tests/mac-prebundle.test.ts index 50dc0c2578..bdf494ecfb 100644 --- a/tools/pack/tests/mac-prebundle.test.ts +++ b/tools/pack/tests/mac-prebundle.test.ts @@ -143,7 +143,7 @@ describe("mac standalone prebundle policy", () => { logLevel: "silent", metafile: true, platform: "node", - entryPoints: [join(workspaceRoot, "apps", "daemon", "src", "byok", "credential-service.ts")], + entryPoints: [join(workspaceRoot, "apps", "daemon", "src", "terminals.ts")], target: MAC_PREBUNDLE_ESBUILD_TARGET, write: false, });