diff --git a/src/agents/model-auth-label.test.ts b/src/agents/model-auth-label.test.ts new file mode 100644 index 00000000000..1423515c143 --- /dev/null +++ b/src/agents/model-auth-label.test.ts @@ -0,0 +1,50 @@ +import { beforeEach, describe, expect, it, vi } from "vitest"; + +const ensureAuthProfileStoreMock = vi.hoisted(() => vi.fn()); +const resolveAuthProfileOrderMock = vi.hoisted(() => vi.fn()); +const resolveAuthProfileDisplayLabelMock = vi.hoisted(() => vi.fn()); + +vi.mock("./auth-profiles.js", () => ({ + ensureAuthProfileStore: (...args: unknown[]) => ensureAuthProfileStoreMock(...args), + resolveAuthProfileOrder: (...args: unknown[]) => resolveAuthProfileOrderMock(...args), + resolveAuthProfileDisplayLabel: (...args: unknown[]) => + resolveAuthProfileDisplayLabelMock(...args), +})); + +vi.mock("./model-auth.js", () => ({ + getCustomProviderApiKey: () => undefined, + resolveEnvApiKey: () => null, +})); + +const { resolveModelAuthLabel } = await import("./model-auth-label.js"); + +describe("resolveModelAuthLabel", () => { + beforeEach(() => { + ensureAuthProfileStoreMock.mockReset(); + resolveAuthProfileOrderMock.mockReset(); + resolveAuthProfileDisplayLabelMock.mockReset(); + }); + + it("does not throw when token profile only has tokenRef", () => { + ensureAuthProfileStoreMock.mockReturnValue({ + version: 1, + profiles: { + "github-copilot:default": { + type: "token", + provider: "github-copilot", + tokenRef: { source: "env", id: "GITHUB_TOKEN" }, + }, + }, + } as never); + resolveAuthProfileOrderMock.mockReturnValue(["github-copilot:default"]); + resolveAuthProfileDisplayLabelMock.mockReturnValue("github-copilot:default"); + + const label = resolveModelAuthLabel({ + provider: "github-copilot", + cfg: {}, + sessionEntry: { authProfileOverride: "github-copilot:default" } as never, + }); + + expect(label).toContain("token ref(env:GITHUB_TOKEN)"); + }); +}); diff --git a/src/agents/model-auth-label.ts b/src/agents/model-auth-label.ts index 9781791574b..3237b33b3f4 100644 --- a/src/agents/model-auth-label.ts +++ b/src/agents/model-auth-label.ts @@ -19,6 +19,20 @@ function formatApiKeySnippet(apiKey: string): string { return `${head}…${tail}`; } +function formatCredentialSnippet(params: { + value: string | undefined; + ref: { source: string; id: string } | undefined; +}): string { + const value = typeof params.value === "string" ? params.value.trim() : ""; + if (value) { + return formatApiKeySnippet(value); + } + if (params.ref) { + return `ref(${params.ref.source}:${params.ref.id})`; + } + return "unknown"; +} + export function resolveModelAuthLabel(params: { provider?: string; cfg?: OpenClawConfig; @@ -57,9 +71,13 @@ export function resolveModelAuthLabel(params: { return `oauth${label ? ` (${label})` : ""}`; } if (profile.type === "token") { - return `token ${formatApiKeySnippet(profile.token)}${label ? ` (${label})` : ""}`; + return `token ${formatCredentialSnippet({ value: profile.token, ref: profile.tokenRef })}${ + label ? ` (${label})` : "" + }`; } - return `api-key ${formatApiKeySnippet(profile.key ?? "")}${label ? ` (${label})` : ""}`; + return `api-key ${formatCredentialSnippet({ value: profile.key, ref: profile.keyRef })}${ + label ? ` (${label})` : "" + }`; } const envKey = resolveEnvApiKey(providerKey); diff --git a/src/agents/pi-auth-credentials.ts b/src/agents/pi-auth-credentials.ts new file mode 100644 index 00000000000..bf35328843d --- /dev/null +++ b/src/agents/pi-auth-credentials.ts @@ -0,0 +1,88 @@ +import type { AuthProfileCredential, AuthProfileStore } from "./auth-profiles.js"; +import { normalizeProviderId } from "./model-selection.js"; + +export type PiApiKeyCredential = { type: "api_key"; key: string }; +export type PiOAuthCredential = { + type: "oauth"; + access: string; + refresh: string; + expires: number; +}; + +export type PiCredential = PiApiKeyCredential | PiOAuthCredential; +export type PiCredentialMap = Record; + +export function convertAuthProfileCredentialToPi(cred: AuthProfileCredential): PiCredential | null { + if (cred.type === "api_key") { + const key = typeof cred.key === "string" ? cred.key.trim() : ""; + if (!key) { + return null; + } + return { type: "api_key", key }; + } + + if (cred.type === "token") { + const token = typeof cred.token === "string" ? cred.token.trim() : ""; + if (!token) { + return null; + } + if ( + typeof cred.expires === "number" && + Number.isFinite(cred.expires) && + Date.now() >= cred.expires + ) { + return null; + } + return { type: "api_key", key: token }; + } + + if (cred.type === "oauth") { + const access = typeof cred.access === "string" ? cred.access.trim() : ""; + const refresh = typeof cred.refresh === "string" ? cred.refresh.trim() : ""; + if (!access || !refresh || !Number.isFinite(cred.expires) || cred.expires <= 0) { + return null; + } + return { + type: "oauth", + access, + refresh, + expires: cred.expires, + }; + } + + return null; +} + +export function resolvePiCredentialMapFromStore(store: AuthProfileStore): PiCredentialMap { + const credentials: PiCredentialMap = {}; + for (const credential of Object.values(store.profiles)) { + const provider = normalizeProviderId(String(credential.provider ?? "")).trim(); + if (!provider || credentials[provider]) { + continue; + } + const converted = convertAuthProfileCredentialToPi(credential); + if (converted) { + credentials[provider] = converted; + } + } + return credentials; +} + +export function piCredentialsEqual(a: PiCredential | undefined, b: PiCredential): boolean { + if (!a || typeof a !== "object") { + return false; + } + if (a.type !== b.type) { + return false; + } + + if (a.type === "api_key" && b.type === "api_key") { + return a.key === b.key; + } + + if (a.type === "oauth" && b.type === "oauth") { + return a.access === b.access && a.refresh === b.refresh && a.expires === b.expires; + } + + return false; +} diff --git a/src/agents/pi-auth-json.ts b/src/agents/pi-auth-json.ts index 33fb5e4f497..5b0b2519e8f 100644 --- a/src/agents/pi-auth-json.ts +++ b/src/agents/pi-auth-json.ts @@ -1,25 +1,17 @@ import fs from "node:fs/promises"; import path from "node:path"; import { ensureAuthProfileStore } from "./auth-profiles.js"; -import type { AuthProfileCredential } from "./auth-profiles/types.js"; -import { normalizeProviderId } from "./model-selection.js"; +import { + piCredentialsEqual, + resolvePiCredentialMapFromStore, + type PiCredential, +} from "./pi-auth-credentials.js"; /** * @deprecated Legacy bridge for older flows that still expect `agentDir/auth.json`. * Runtime auth resolution uses auth-profiles directly and should not depend on this module. */ -type AuthJsonCredential = - | { - type: "api_key"; - key: string; - } - | { - type: "oauth"; - access: string; - refresh: string; - expires: number; - [key: string]: unknown; - }; +type AuthJsonCredential = PiCredential; type AuthJsonShape = Record; @@ -36,75 +28,6 @@ async function readAuthJson(filePath: string): Promise { } } -/** - * Convert an OpenClaw auth-profiles credential to pi-coding-agent auth.json format. - * Returns null if the credential cannot be converted. - */ -function convertCredential(cred: AuthProfileCredential): AuthJsonCredential | null { - if (cred.type === "api_key") { - const key = typeof cred.key === "string" ? cred.key.trim() : ""; - if (!key) { - return null; - } - return { type: "api_key", key }; - } - - if (cred.type === "token") { - // pi-coding-agent treats static tokens as api_key type - const token = typeof cred.token === "string" ? cred.token.trim() : ""; - if (!token) { - return null; - } - const expires = - typeof (cred as { expires?: unknown }).expires === "number" - ? (cred as { expires: number }).expires - : Number.NaN; - if (Number.isFinite(expires) && expires > 0 && Date.now() >= expires) { - return null; - } - return { type: "api_key", key: token }; - } - - if (cred.type === "oauth") { - const accessRaw = (cred as { access?: unknown }).access; - const refreshRaw = (cred as { refresh?: unknown }).refresh; - const expiresRaw = (cred as { expires?: unknown }).expires; - - const access = typeof accessRaw === "string" ? accessRaw.trim() : ""; - const refresh = typeof refreshRaw === "string" ? refreshRaw.trim() : ""; - const expires = typeof expiresRaw === "number" ? expiresRaw : Number.NaN; - - if (!access || !refresh || !Number.isFinite(expires) || expires <= 0) { - return null; - } - return { type: "oauth", access, refresh, expires }; - } - - return null; -} - -/** - * Check if two auth.json credentials are equivalent. - */ -function credentialsEqual(a: AuthJsonCredential | undefined, b: AuthJsonCredential): boolean { - if (!a || typeof a !== "object") { - return false; - } - if (a.type !== b.type) { - return false; - } - - if (a.type === "api_key" && b.type === "api_key") { - return a.key === b.key; - } - - if (a.type === "oauth" && b.type === "oauth") { - return a.access === b.access && a.refresh === b.refresh && a.expires === b.expires; - } - - return false; -} - /** * pi-coding-agent's ModelRegistry/AuthStorage expects credentials in auth.json. * @@ -123,31 +46,16 @@ export async function ensurePiAuthJsonFromAuthProfiles(agentDir: string): Promis }> { const store = ensureAuthProfileStore(agentDir, { allowKeychainPrompt: false }); const authPath = path.join(agentDir, "auth.json"); - - // Group profiles by provider, taking the first valid profile for each - const providerCredentials = new Map(); - - for (const [, cred] of Object.entries(store.profiles)) { - const provider = normalizeProviderId(String(cred.provider ?? "")).trim(); - if (!provider || providerCredentials.has(provider)) { - continue; - } - - const converted = convertCredential(cred); - if (converted) { - providerCredentials.set(provider, converted); - } - } - - if (providerCredentials.size === 0) { + const providerCredentials = resolvePiCredentialMapFromStore(store); + if (Object.keys(providerCredentials).length === 0) { return { wrote: false, authPath }; } const existing = await readAuthJson(authPath); let changed = false; - for (const [provider, cred] of providerCredentials) { - if (!credentialsEqual(existing[provider], cred)) { + for (const [provider, cred] of Object.entries(providerCredentials)) { + if (!piCredentialsEqual(existing[provider], cred)) { existing[provider] = cred; changed = true; } diff --git a/src/agents/pi-model-discovery.ts b/src/agents/pi-model-discovery.ts index 9415cd9d83b..2f42b83fd6f 100644 --- a/src/agents/pi-model-discovery.ts +++ b/src/agents/pi-model-discovery.ts @@ -5,22 +5,10 @@ import { ModelRegistry, } from "@mariozechner/pi-coding-agent"; import { ensureAuthProfileStore } from "./auth-profiles.js"; -import type { AuthProfileCredential } from "./auth-profiles.js"; -import { normalizeProviderId } from "./model-selection.js"; +import { resolvePiCredentialMapFromStore, type PiCredentialMap } from "./pi-auth-credentials.js"; export { AuthStorage, ModelRegistry } from "@mariozechner/pi-coding-agent"; -type PiApiKeyCredential = { type: "api_key"; key: string }; -type PiOAuthCredential = { - type: "oauth"; - access: string; - refresh: string; - expires: number; -}; - -type PiCredential = PiApiKeyCredential | PiOAuthCredential; -type PiCredentialMap = Record; - function createAuthStorage(AuthStorageLike: unknown, path: string, creds: PiCredentialMap) { const withInMemory = AuthStorageLike as { inMemory?: (data?: unknown) => unknown }; if (typeof withInMemory.inMemory === "function") { @@ -59,61 +47,9 @@ function createAuthStorage(AuthStorageLike: unknown, path: string, creds: PiCred return withRuntimeOverride; } -function convertAuthProfileCredential(cred: AuthProfileCredential): PiCredential | null { - if (cred.type === "api_key") { - const key = typeof cred.key === "string" ? cred.key.trim() : ""; - if (!key) { - return null; - } - return { type: "api_key", key }; - } - - if (cred.type === "token") { - const token = typeof cred.token === "string" ? cred.token.trim() : ""; - if (!token) { - return null; - } - if ( - typeof cred.expires === "number" && - Number.isFinite(cred.expires) && - Date.now() >= cred.expires - ) { - return null; - } - return { type: "api_key", key: token }; - } - - if (cred.type === "oauth") { - const access = typeof cred.access === "string" ? cred.access.trim() : ""; - const refresh = typeof cred.refresh === "string" ? cred.refresh.trim() : ""; - if (!access || !refresh || !Number.isFinite(cred.expires) || cred.expires <= 0) { - return null; - } - return { - type: "oauth", - access, - refresh, - expires: cred.expires, - }; - } - - return null; -} - function resolvePiCredentials(agentDir: string): PiCredentialMap { const store = ensureAuthProfileStore(agentDir, { allowKeychainPrompt: false }); - const credentials: PiCredentialMap = {}; - for (const credential of Object.values(store.profiles)) { - const provider = normalizeProviderId(String(credential.provider ?? "")).trim(); - if (!provider || credentials[provider]) { - continue; - } - const converted = convertAuthProfileCredential(credential); - if (converted) { - credentials[provider] = converted; - } - } - return credentials; + return resolvePiCredentialMapFromStore(store); } // Compatibility helpers for pi-coding-agent 0.50+ (discover* helpers removed). diff --git a/src/commands/models/list.auth-overview.test.ts b/src/commands/models/list.auth-overview.test.ts new file mode 100644 index 00000000000..6bf4bf5fed4 --- /dev/null +++ b/src/commands/models/list.auth-overview.test.ts @@ -0,0 +1,24 @@ +import { describe, expect, it } from "vitest"; +import { resolveProviderAuthOverview } from "./list.auth-overview.js"; + +describe("resolveProviderAuthOverview", () => { + it("does not throw when token profile only has tokenRef", () => { + const overview = resolveProviderAuthOverview({ + provider: "github-copilot", + cfg: {}, + store: { + version: 1, + profiles: { + "github-copilot:default": { + type: "token", + provider: "github-copilot", + tokenRef: { source: "env", id: "GITHUB_TOKEN" }, + }, + }, + } as never, + modelsPath: "/tmp/models.json", + }); + + expect(overview.profiles.labels[0]).toContain("token:ref(env:GITHUB_TOKEN)"); + }); +}); diff --git a/src/commands/models/list.auth-overview.ts b/src/commands/models/list.auth-overview.ts index 49159e93af6..0fc2f9828c5 100644 --- a/src/commands/models/list.auth-overview.ts +++ b/src/commands/models/list.auth-overview.ts @@ -12,6 +12,22 @@ import { shortenHomePath } from "../../utils.js"; import { maskApiKey } from "./list.format.js"; import type { ProviderAuthOverview } from "./list.types.js"; +function formatProfileSecretLabel(params: { + value: string | undefined; + ref: { source: string; id: string } | undefined; + kind: "api-key" | "token"; +}): string { + const value = typeof params.value === "string" ? params.value.trim() : ""; + if (value) { + return params.kind === "token" ? `token:${maskApiKey(value)}` : maskApiKey(value); + } + if (params.ref) { + const refLabel = `ref(${params.ref.source}:${params.ref.id})`; + return params.kind === "token" ? `token:${refLabel}` : refLabel; + } + return params.kind === "token" ? "token:missing" : "missing"; +} + export function resolveProviderAuthOverview(params: { provider: string; cfg: OpenClawConfig; @@ -40,10 +56,24 @@ export function resolveProviderAuthOverview(params: { return `${profileId}=missing`; } if (profile.type === "api_key") { - return withUnusableSuffix(`${profileId}=${maskApiKey(profile.key ?? "")}`, profileId); + return withUnusableSuffix( + `${profileId}=${formatProfileSecretLabel({ + value: profile.key, + ref: profile.keyRef, + kind: "api-key", + })}`, + profileId, + ); } if (profile.type === "token") { - return withUnusableSuffix(`${profileId}=token:${maskApiKey(profile.token)}`, profileId); + return withUnusableSuffix( + `${profileId}=${formatProfileSecretLabel({ + value: profile.token, + ref: profile.tokenRef, + kind: "token", + })}`, + profileId, + ); } const display = resolveAuthProfileDisplayLabel({ cfg, store, profileId }); const suffix =