mirror of https://github.com/openclaw/openclaw.git
refactor: dedupe auth profile store normalization
This commit is contained in:
Peter Steinberger
parent
ab3c646bb1
commit
33fbd9b770
|
|
@ -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<T>(prefix: string, run: (agentDir: string) => T): T {
|
||||
|
|
@ -15,6 +16,42 @@ describe("ensureAuthProfileStore", () => {
|
|||
}
|
||||
}
|
||||
|
||||
function writeAuthProfileStore(agentDir: string, profiles: Record<string, unknown>): 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<AuthProfileCredential, { type: "api_key" }> {
|
||||
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<AuthProfileCredential, { type: "token" }> {
|
||||
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);
|
||||
});
|
||||
},
|
||||
);
|
||||
});
|
||||
|
|
|
|||
|
|
@ -126,6 +126,22 @@ function writeCachedAuthProfileStore(
|
|||
});
|
||||
}
|
||||
|
||||
function normalizeSecretBackedField(params: {
|
||||
entry: Record<string, unknown>;
|
||||
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<string, unknown>): Partial<Auth
|
|||
// value is truthy (an object) but has no `.trim()` method. Migrate the
|
||||
// misplaced ref to `keyRef` so the secret-resolution pipeline can pick it
|
||||
// up, and clear the invalid `key` so callers never see a non-string value.
|
||||
if ("key" in entry && entry["key"] != null && typeof entry["key"] !== "string") {
|
||||
const ref = coerceSecretRef(entry["key"]);
|
||||
if (ref && !coerceSecretRef(entry["keyRef"])) {
|
||||
entry["keyRef"] = ref;
|
||||
}
|
||||
delete entry["key"];
|
||||
}
|
||||
normalizeSecretBackedField({ entry, valueField: "key", refField: "keyRef" });
|
||||
// Same treatment for `token` on TokenCredential entries.
|
||||
if ("token" in entry && entry["token"] != null && typeof entry["token"] !== "string") {
|
||||
const ref = coerceSecretRef(entry["token"]);
|
||||
if (ref && !coerceSecretRef(entry["tokenRef"])) {
|
||||
entry["tokenRef"] = ref;
|
||||
}
|
||||
delete entry["token"];
|
||||
}
|
||||
normalizeSecretBackedField({ entry, valueField: "token", refField: "tokenRef" });
|
||||
return entry as Partial<AuthProfileCredential>;
|
||||
}
|
||||
|
||||
|
|
|
|||
Loading…
Reference in New Issue