fix(auth-profiles ): ensure credential key and token are strings to prevent crash

Fixes #58861
This commit is contained in:
openperf 2026-04-01 17:46:55 +08:00 committed by Peter Steinberger
parent cd4b03c568
commit ac68321d4d
2 changed files with 271 additions and 1 deletions

View File

@ -326,7 +326,6 @@ describe("ensureAuthProfileStore", () => {
`${JSON.stringify(invalidStore, null, 2)}\n`,
"utf8",
);
const store = ensureAuthProfileStore(agentDir);
expect(store.profiles).toEqual({});
expect(warnSpy).toHaveBeenCalledTimes(1);
@ -348,4 +347,253 @@ describe("ensureAuthProfileStore", () => {
warnSpy.mockRestore();
}
});
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({
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({
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({
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({
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");
}
});
});
});

View File

@ -1,5 +1,6 @@
import fs from "node:fs";
import { resolveOAuthPath } from "../../config/paths.js";
import { coerceSecretRef } from "../../config/types.secrets.js";
import { withFileLock } from "../../infra/file-lock.js";
import { loadJsonFile, saveJsonFile } from "../../infra/json-file.js";
import {
@ -168,6 +169,27 @@ function normalizeRawCredentialEntry(raw: Record<string, unknown>): Partial<Auth
if (!("key" in entry) && typeof entry["apiKey"] === "string") {
entry["key"] = entry["apiKey"];
}
// Ensure `key` is a string. Users sometimes write a SecretRef object into
// the `key` field instead of `keyRef`. When that happens every downstream
// consumer that calls `cred.key?.trim()` throws a TypeError because the
// 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"];
}
// 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"];
}
return entry as Partial<AuthProfileCredential>;
}