diff --git a/src/agents/auth-profiles.ensureauthprofilestore.test.ts b/src/agents/auth-profiles.ensureauthprofilestore.test.ts index 79a2679f8d0..c3b498c2e99 100644 --- a/src/agents/auth-profiles.ensureauthprofilestore.test.ts +++ b/src/agents/auth-profiles.ensureauthprofilestore.test.ts @@ -4,6 +4,7 @@ import path from "node:path"; import { describe, expect, it, vi } from "vitest"; import { clearRuntimeAuthProfileStoreSnapshots, ensureAuthProfileStore } from "./auth-profiles.js"; import { AUTH_STORE_VERSION, log } from "./auth-profiles/constants.js"; +import type { AuthProfileCredential } from "./auth-profiles/types.js"; describe("ensureAuthProfileStore", () => { function withTempAgentDir(prefix: string, run: (agentDir: string) => T): T { @@ -15,6 +16,42 @@ describe("ensureAuthProfileStore", () => { } } + function writeAuthProfileStore(agentDir: string, profiles: Record): void { + fs.writeFileSync( + path.join(agentDir, "auth-profiles.json"), + `${JSON.stringify({ version: AUTH_STORE_VERSION, profiles }, null, 2)}\n`, + "utf8", + ); + } + + function loadAuthProfile(agentDir: string, profileId: string): AuthProfileCredential { + clearRuntimeAuthProfileStoreSnapshots(); + const store = ensureAuthProfileStore(agentDir); + const profile = store.profiles[profileId]; + expect(profile).toBeDefined(); + return profile; + } + + function expectApiKeyProfile( + profile: AuthProfileCredential, + ): Extract { + expect(profile.type).toBe("api_key"); + if (profile.type !== "api_key") { + throw new Error(`Expected api_key profile, got ${profile.type}`); + } + return profile; + } + + function expectTokenProfile( + profile: AuthProfileCredential, + ): Extract { + expect(profile.type).toBe("token"); + if (profile.type !== "token") { + throw new Error(`Expected token profile, got ${profile.type}`); + } + return profile; + } + it("migrates legacy auth.json and deletes it (PR #368)", () => { const agentDir = fs.mkdtempSync(path.join(os.tmpdir(), "openclaw-auth-profiles-")); try { @@ -348,252 +385,151 @@ describe("ensureAuthProfileStore", () => { } }); - it("migrates SecretRef object in `key` to `keyRef` and clears `key` (#58861)", () => { - withTempAgentDir("openclaw-nonstr-key-ref-", (agentDir) => { - clearRuntimeAuthProfileStoreSnapshots(); - const storeData = { - version: AUTH_STORE_VERSION, - profiles: { - "openai:default": { - type: "api_key", - provider: "openai", - key: { source: "env", provider: "default", id: "OPENAI_API_KEY" }, - }, - }, - }; - fs.writeFileSync( - path.join(agentDir, "auth-profiles.json"), - `${JSON.stringify(storeData, null, 2)}\n`, - "utf8", - ); - const store = ensureAuthProfileStore(agentDir); - const profile = store.profiles["openai:default"]; - expect(profile).toBeDefined(); - expect(profile.type).toBe("api_key"); - if (profile.type === "api_key") { - // key must not be an object — that would crash `.trim()` downstream - expect(profile.key).toBeUndefined(); - expect(profile.keyRef).toEqual({ + it.each([ + { + name: "migrates SecretRef object in `key` to `keyRef` and clears `key`", + prefix: "openclaw-nonstr-key-ref-", + profileId: "openai:default", + profile: { + type: "api_key", + provider: "openai", + key: { source: "env", provider: "default", id: "OPENAI_API_KEY" }, + }, + assert(profile: AuthProfileCredential) { + const apiKey = expectApiKeyProfile(profile); + expect(apiKey.key).toBeUndefined(); + expect(apiKey.keyRef).toEqual({ source: "env", provider: "default", id: "OPENAI_API_KEY", }); - } - }); - }); - - it("deletes non-string non-SecretRef `key` without setting keyRef (#58861)", () => { - withTempAgentDir("openclaw-nonstr-key-num-", (agentDir) => { - clearRuntimeAuthProfileStoreSnapshots(); - const storeData = { - version: AUTH_STORE_VERSION, - profiles: { - "openai:default": { - type: "api_key", - provider: "openai", - key: 12345, - }, - }, - }; - fs.writeFileSync( - path.join(agentDir, "auth-profiles.json"), - `${JSON.stringify(storeData, null, 2)}\n`, - "utf8", - ); - const store = ensureAuthProfileStore(agentDir); - const profile = store.profiles["openai:default"]; - expect(profile).toBeDefined(); - expect(profile.type).toBe("api_key"); - if (profile.type === "api_key") { - expect(profile.key).toBeUndefined(); - expect(profile.keyRef).toBeUndefined(); - } - }); - }); - - it("does not overwrite existing `keyRef` when `key` contains a SecretRef (#58861)", () => { - withTempAgentDir("openclaw-nonstr-key-dup-", (agentDir) => { - clearRuntimeAuthProfileStoreSnapshots(); - const storeData = { - version: AUTH_STORE_VERSION, - profiles: { - "openai:default": { - type: "api_key", - provider: "openai", - key: { source: "env", provider: "default", id: "WRONG_VAR" }, - keyRef: { source: "env", provider: "default", id: "CORRECT_VAR" }, - }, - }, - }; - fs.writeFileSync( - path.join(agentDir, "auth-profiles.json"), - `${JSON.stringify(storeData, null, 2)}\n`, - "utf8", - ); - const store = ensureAuthProfileStore(agentDir); - const profile = store.profiles["openai:default"]; - expect(profile).toBeDefined(); - expect(profile.type).toBe("api_key"); - if (profile.type === "api_key") { - expect(profile.key).toBeUndefined(); - expect(profile.keyRef).toEqual({ + }, + }, + { + name: "deletes non-string non-SecretRef `key` without setting keyRef", + prefix: "openclaw-nonstr-key-num-", + profileId: "openai:default", + profile: { + type: "api_key", + provider: "openai", + key: 12345, + }, + assert(profile: AuthProfileCredential) { + const apiKey = expectApiKeyProfile(profile); + expect(apiKey.key).toBeUndefined(); + expect(apiKey.keyRef).toBeUndefined(); + }, + }, + { + name: "does not overwrite existing `keyRef` when `key` contains a SecretRef", + prefix: "openclaw-nonstr-key-dup-", + profileId: "openai:default", + profile: { + type: "api_key", + provider: "openai", + key: { source: "env", provider: "default", id: "WRONG_VAR" }, + keyRef: { source: "env", provider: "default", id: "CORRECT_VAR" }, + }, + assert(profile: AuthProfileCredential) { + const apiKey = expectApiKeyProfile(profile); + expect(apiKey.key).toBeUndefined(); + expect(apiKey.keyRef).toEqual({ source: "env", provider: "default", id: "CORRECT_VAR", }); - } - }); - }); - - it("overwrites malformed `keyRef` with migrated ref from `key` (#58861)", () => { - withTempAgentDir("openclaw-nonstr-key-malformed-ref-", (agentDir) => { - clearRuntimeAuthProfileStoreSnapshots(); - const storeData = { - version: AUTH_STORE_VERSION, - profiles: { - "openai:default": { - type: "api_key", - provider: "openai", - key: { source: "env", provider: "default", id: "OPENAI_API_KEY" }, - keyRef: null, - }, - }, - }; - fs.writeFileSync( - path.join(agentDir, "auth-profiles.json"), - `${JSON.stringify(storeData, null, 2)}\n`, - "utf8", - ); - const store = ensureAuthProfileStore(agentDir); - const profile = store.profiles["openai:default"]; - expect(profile).toBeDefined(); - expect(profile.type).toBe("api_key"); - if (profile.type === "api_key") { - expect(profile.key).toBeUndefined(); - expect(profile.keyRef).toEqual({ + }, + }, + { + name: "overwrites malformed `keyRef` with migrated ref from `key`", + prefix: "openclaw-nonstr-key-malformed-ref-", + profileId: "openai:default", + profile: { + type: "api_key", + provider: "openai", + key: { source: "env", provider: "default", id: "OPENAI_API_KEY" }, + keyRef: null, + }, + assert(profile: AuthProfileCredential) { + const apiKey = expectApiKeyProfile(profile); + expect(apiKey.key).toBeUndefined(); + expect(apiKey.keyRef).toEqual({ source: "env", provider: "default", id: "OPENAI_API_KEY", }); - } - }); - }); - - it("preserves valid string `key` values unchanged (#58861)", () => { - withTempAgentDir("openclaw-str-key-", (agentDir) => { - clearRuntimeAuthProfileStoreSnapshots(); - const storeData = { - version: AUTH_STORE_VERSION, - profiles: { - "openai:default": { - type: "api_key", - provider: "openai", - key: "sk-valid-plaintext-key", - }, - }, - }; - fs.writeFileSync( - path.join(agentDir, "auth-profiles.json"), - `${JSON.stringify(storeData, null, 2)}\n`, - "utf8", - ); - const store = ensureAuthProfileStore(agentDir); - const profile = store.profiles["openai:default"]; - expect(profile).toBeDefined(); - expect(profile.type).toBe("api_key"); - if (profile.type === "api_key") { - expect(profile.key).toBe("sk-valid-plaintext-key"); - } - }); - }); - - it("migrates SecretRef object in `token` to `tokenRef` and clears `token` (#58861)", () => { - withTempAgentDir("openclaw-nonstr-token-ref-", (agentDir) => { - clearRuntimeAuthProfileStoreSnapshots(); - const storeData = { - version: AUTH_STORE_VERSION, - profiles: { - "anthropic:default": { - type: "token", - provider: "anthropic", - token: { source: "env", provider: "default", id: "ANTHROPIC_TOKEN" }, - }, - }, - }; - fs.writeFileSync( - path.join(agentDir, "auth-profiles.json"), - `${JSON.stringify(storeData, null, 2)}\n`, - "utf8", - ); - const store = ensureAuthProfileStore(agentDir); - const profile = store.profiles["anthropic:default"]; - expect(profile).toBeDefined(); - expect(profile.type).toBe("token"); - if (profile.type === "token") { - expect(profile.token).toBeUndefined(); - expect(profile.tokenRef).toEqual({ + }, + }, + { + name: "preserves valid string `key` values unchanged", + prefix: "openclaw-str-key-", + profileId: "openai:default", + profile: { + type: "api_key", + provider: "openai", + key: "sk-valid-plaintext-key", + }, + assert(profile: AuthProfileCredential) { + const apiKey = expectApiKeyProfile(profile); + expect(apiKey.key).toBe("sk-valid-plaintext-key"); + }, + }, + { + name: "migrates SecretRef object in `token` to `tokenRef` and clears `token`", + prefix: "openclaw-nonstr-token-ref-", + profileId: "anthropic:default", + profile: { + type: "token", + provider: "anthropic", + token: { source: "env", provider: "default", id: "ANTHROPIC_TOKEN" }, + }, + assert(profile: AuthProfileCredential) { + const token = expectTokenProfile(profile); + expect(token.token).toBeUndefined(); + expect(token.tokenRef).toEqual({ source: "env", provider: "default", id: "ANTHROPIC_TOKEN", }); - } - }); - }); - - it("deletes non-string non-SecretRef `token` without setting tokenRef (#58861)", () => { - withTempAgentDir("openclaw-nonstr-token-num-", (agentDir) => { - clearRuntimeAuthProfileStoreSnapshots(); - const storeData = { - version: AUTH_STORE_VERSION, - profiles: { - "anthropic:default": { - type: "token", - provider: "anthropic", - token: 99999, - }, - }, - }; - fs.writeFileSync( - path.join(agentDir, "auth-profiles.json"), - `${JSON.stringify(storeData, null, 2)}\n`, - "utf8", - ); - const store = ensureAuthProfileStore(agentDir); - const profile = store.profiles["anthropic:default"]; - expect(profile).toBeDefined(); - expect(profile.type).toBe("token"); - if (profile.type === "token") { - expect(profile.token).toBeUndefined(); - expect(profile.tokenRef).toBeUndefined(); - } - }); - }); - - it("preserves valid string `token` values unchanged (#58861)", () => { - withTempAgentDir("openclaw-str-token-", (agentDir) => { - clearRuntimeAuthProfileStoreSnapshots(); - const storeData = { - version: AUTH_STORE_VERSION, - profiles: { - "anthropic:default": { - type: "token", - provider: "anthropic", - token: "tok-valid-plaintext", - }, - }, - }; - fs.writeFileSync( - path.join(agentDir, "auth-profiles.json"), - `${JSON.stringify(storeData, null, 2)}\n`, - "utf8", - ); - const store = ensureAuthProfileStore(agentDir); - const profile = store.profiles["anthropic:default"]; - expect(profile).toBeDefined(); - expect(profile.type).toBe("token"); - if (profile.type === "token") { - expect(profile.token).toBe("tok-valid-plaintext"); - } - }); - }); + }, + }, + { + name: "deletes non-string non-SecretRef `token` without setting tokenRef", + prefix: "openclaw-nonstr-token-num-", + profileId: "anthropic:default", + profile: { + type: "token", + provider: "anthropic", + token: 99999, + }, + assert(profile: AuthProfileCredential) { + const token = expectTokenProfile(profile); + expect(token.token).toBeUndefined(); + expect(token.tokenRef).toBeUndefined(); + }, + }, + { + name: "preserves valid string `token` values unchanged", + prefix: "openclaw-str-token-", + profileId: "anthropic:default", + profile: { + type: "token", + provider: "anthropic", + token: "tok-valid-plaintext", + }, + assert(profile: AuthProfileCredential) { + const token = expectTokenProfile(profile); + expect(token.token).toBe("tok-valid-plaintext"); + }, + }, + ] as const)( + "normalizes secret-backed auth profile fields during store load: $name (#58861)", + (testCase) => { + withTempAgentDir(testCase.prefix, (agentDir) => { + writeAuthProfileStore(agentDir, { [testCase.profileId]: testCase.profile }); + const profile = loadAuthProfile(agentDir, testCase.profileId); + testCase.assert(profile); + }); + }, + ); }); diff --git a/src/agents/auth-profiles/store.ts b/src/agents/auth-profiles/store.ts index 67e948b8615..95f6728fcdc 100644 --- a/src/agents/auth-profiles/store.ts +++ b/src/agents/auth-profiles/store.ts @@ -126,6 +126,22 @@ function writeCachedAuthProfileStore( }); } +function normalizeSecretBackedField(params: { + entry: Record; + valueField: "key" | "token"; + refField: "keyRef" | "tokenRef"; +}): void { + const value = params.entry[params.valueField]; + if (value == null || typeof value === "string") { + return; + } + const ref = coerceSecretRef(value); + if (ref && !coerceSecretRef(params.entry[params.refField])) { + params.entry[params.refField] = ref; + } + delete params.entry[params.valueField]; +} + export async function updateAuthProfileStoreWithLock(params: { agentDir?: string; updater: (store: AuthProfileStore) => boolean; @@ -175,21 +191,9 @@ function normalizeRawCredentialEntry(raw: Record): Partial; }