Skip to content

Commit b3f35e5

Browse files
vincentkocvincentkoc
committed
fix(auth): serialize auth login writes
1 parent 7593ba8 commit b3f35e5

11 files changed

Lines changed: 208 additions & 75 deletions

File tree

‎CHANGELOG.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ Docs: https://docs.openclaw.ai
2020
- Codex app-server: limit canonical OpenAI Codex app-server attribution rewrites to local transcript and trajectory records, leaving runtime/tool routing on the selected OpenAI model metadata so OpenAI API-key backup profiles keep their billing path.
2121
- Android/chat: make bare and markdown URLs in chat messages tappable by preserving Compose URL annotations in rendered markdown. Fixes #82187. (#82392) Thanks @neeravmakwana.
2222
- Plugins/doctor: migrate legacy top-level plugin `tools` declarations into `contracts.tools`, so `openclaw doctor --fix` repairs local plugins for the manifest tool contract. (#81112) Thanks @100yenadmin.
23+
- Auth/OpenAI Codex: serialize provider login writes through the auth-profile lock so a live Gateway cannot overwrite freshly refreshed OAuth credentials with an expired in-memory snapshot.
2324
- Codex app-server: release raw assistant completions when `turn/completed` is missing while keeping commentary/status items as progress, preventing completed Codex runs from hanging until timeout. Fixes #82343. (#82403) Thanks @IWhatsskill.
2425
- Agents/sessions: remove the transient `*.bak-<pid>-<ts>` backup written by `repairSessionFileIfNeeded` once the atomic replace succeeds, so a stuck session with a persistently malformed JSONL line no longer accumulates one snapshot per repair invocation. Fixes #80960. (#80969) Thanks @100yenadmin. Co-authored by @tynamite.
2526
- CLI/status: show plain empty-state messages instead of empty Channels and Sessions tables when no channels or sessions exist.

‎src/agents/auth-profiles/persisted.ts‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -460,7 +460,11 @@ function createFallbackOAuthProfileSecretKeyFile(): string | undefined {
460460
}
461461

462462
function shouldUseMacKeychainForOAuthProfileSecrets(): boolean {
463-
return process.platform === "darwin" && process.env.VITEST !== "true";
463+
return (
464+
process.platform === "darwin" &&
465+
process.env.VITEST !== "true" &&
466+
process.env.VITEST_WORKER_ID === undefined
467+
);
464468
}
465469

466470
function resolveOAuthProfileSecretKeySeed(options?: { create?: boolean }): string | undefined {

‎src/agents/auth-profiles/profiles.test.ts‎

Lines changed: 60 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ import { describe, expect, it, vi } from "vitest";
55
import { resolveOAuthDir } from "../../config/paths.js";
66
import { AUTH_STORE_VERSION } from "./constants.js";
77
import { resolveAuthStorePath } from "./paths.js";
8-
import { promoteAuthProfileInOrder } from "./profiles.js";
8+
import { promoteAuthProfileInOrder, upsertAuthProfileWithLock } from "./profiles.js";
99
import {
1010
clearRuntimeAuthProfileStoreSnapshots,
1111
findPersistedAuthProfileCredential,
@@ -138,6 +138,54 @@ function expectOpenClawCredentialsOAuthRef(
138138
}
139139

140140
describe("promoteAuthProfileInOrder", () => {
141+
it("normalizes copied secrets when using the locked upsert path", async () => {
142+
const stateDir = fs.mkdtempSync(path.join(os.tmpdir(), "openclaw-auth-profile-upsert-"));
143+
const agentDir = path.join(stateDir, "agents", "main", "agent");
144+
const previousStateDir = process.env.OPENCLAW_STATE_DIR;
145+
process.env.OPENCLAW_STATE_DIR = stateDir;
146+
try {
147+
fs.mkdirSync(agentDir, { recursive: true });
148+
149+
await upsertAuthProfileWithLock({
150+
profileId: "openai:manual",
151+
credential: {
152+
type: "token",
153+
provider: "openai",
154+
token: " bearer\r\n-token\u2502 ",
155+
},
156+
agentDir,
157+
});
158+
await upsertAuthProfileWithLock({
159+
profileId: "anthropic:key",
160+
credential: {
161+
type: "api_key",
162+
provider: "anthropic",
163+
key: " sk-\r\nant\u2502 ",
164+
},
165+
agentDir,
166+
});
167+
168+
const profiles = loadAuthProfileStoreWithoutExternalProfiles(agentDir).profiles;
169+
expect(profiles["openai:manual"]).toMatchObject({
170+
type: "token",
171+
provider: "openai",
172+
token: "bearer-token",
173+
});
174+
expect(profiles["anthropic:key"]).toMatchObject({
175+
type: "api_key",
176+
provider: "anthropic",
177+
key: "sk-ant",
178+
});
179+
} finally {
180+
if (previousStateDir === undefined) {
181+
delete process.env.OPENCLAW_STATE_DIR;
182+
} else {
183+
process.env.OPENCLAW_STATE_DIR = previousStateDir;
184+
}
185+
fs.rmSync(stateDir, { recursive: true, force: true });
186+
}
187+
});
188+
141189
it("omits inline openai-codex oauth secrets from persisted auth profile files", () => {
142190
const stateDir = fs.mkdtempSync(path.join(os.tmpdir(), "openclaw-auth-profile-metadata-"));
143191
const agentDir = path.join(stateDir, "agents", "main", "agent");
@@ -610,10 +658,18 @@ describe("promoteAuthProfileInOrder", () => {
610658
{ filterExternalAuthProfiles: false },
611659
);
612660

661+
const expectedKeyPath =
662+
process.platform === "darwin"
663+
? path.join(
664+
homeDir,
665+
"Library",
666+
"Application Support",
667+
"OpenClaw",
668+
"auth-profile-secret-key",
669+
)
670+
: path.join(homeDir, ".openclaw-auth-profile-secrets", "auth-profile-secret-key");
613671
const keyPaths = findFilesNamed(rootDir, "auth-profile-secret-key");
614-
expect(keyPaths).toEqual([
615-
path.join(homeDir, ".openclaw-auth-profile-secrets", "auth-profile-secret-key"),
616-
]);
672+
expect(keyPaths).toEqual([expectedKeyPath]);
617673
expect(keyPaths.every((keyPath) => !isPathInsideOrEqual(stateDir, keyPath))).toBe(true);
618674
const keyValues = keyPaths.map((keyPath) => fs.readFileSync(keyPath, "utf8").trim());
619675
const persistedStateTree = readPersistedTree(stateDir);

‎src/agents/auth-profiles/profiles.ts‎

Lines changed: 20 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -106,22 +106,25 @@ export async function promoteAuthProfileInOrder(params: {
106106
});
107107
}
108108

109+
function normalizeAuthProfileCredential(credential: AuthProfileCredential): AuthProfileCredential {
110+
if (credential.type === "api_key") {
111+
return {
112+
...credential,
113+
...(typeof credential.key === "string" ? { key: normalizeSecretInput(credential.key) } : {}),
114+
};
115+
}
116+
if (credential.type === "token") {
117+
return { ...credential, token: normalizeSecretInput(credential.token) };
118+
}
119+
return credential;
120+
}
121+
109122
export function upsertAuthProfile(params: {
110123
profileId: string;
111124
credential: AuthProfileCredential;
112125
agentDir?: string;
113126
}): void {
114-
const credential =
115-
params.credential.type === "api_key"
116-
? {
117-
...params.credential,
118-
...(typeof params.credential.key === "string"
119-
? { key: normalizeSecretInput(params.credential.key) }
120-
: {}),
121-
}
122-
: params.credential.type === "token"
123-
? { ...params.credential, token: normalizeSecretInput(params.credential.token) }
124-
: params.credential;
127+
const credential = normalizeAuthProfileCredential(params.credential);
125128
const store = ensureAuthProfileStoreForLocalUpdate(params.agentDir);
126129
store.profiles[params.profileId] = credential;
127130
saveAuthProfileStore(store, params.agentDir, {
@@ -135,10 +138,15 @@ export async function upsertAuthProfileWithLock(params: {
135138
credential: AuthProfileCredential;
136139
agentDir?: string;
137140
}): Promise<AuthProfileStore | null> {
141+
const credential = normalizeAuthProfileCredential(params.credential);
138142
return await updateAuthProfileStoreWithLock({
139143
agentDir: params.agentDir,
144+
saveOptions: {
145+
filterExternalAuthProfiles: false,
146+
syncExternalCli: false,
147+
},
140148
updater: (store) => {
141-
store.profiles[params.profileId] = params.credential;
149+
store.profiles[params.profileId] = credential;
142150
return true;
143151
},
144152
});

‎src/agents/auth-profiles/store.ts‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -456,6 +456,7 @@ function buildLocalAuthProfileStoreForSave(params: {
456456

457457
export async function updateAuthProfileStoreWithLock(params: {
458458
agentDir?: string;
459+
saveOptions?: SaveAuthProfileStoreOptions;
459460
updater: (store: AuthProfileStore) => boolean;
460461
}): Promise<AuthProfileStore | null> {
461462
const authPath = resolveAuthStorePath(params.agentDir);
@@ -469,7 +470,7 @@ export async function updateAuthProfileStoreWithLock(params: {
469470
const store = loadAuthProfileStoreForAgent(params.agentDir, { syncExternalCli: false });
470471
const shouldSave = params.updater(store);
471472
if (shouldSave) {
472-
saveAuthProfileStore(store, params.agentDir);
473+
saveAuthProfileStore(store, params.agentDir, params.saveOptions);
473474
}
474475
return store;
475476
});

‎src/agents/auth-profiles/upsert-with-lock.ts‎

Lines changed: 20 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,21 @@
1+
import { normalizeSecretInput } from "../../utils/normalize-secret-input.js";
12
import { ensureAuthStoreFile, resolveAuthStorePath } from "./paths.js";
23
import { updateAuthProfileStoreWithLock } from "./store.js";
34
import type { AuthProfileCredential, AuthProfileStore } from "./types.js";
45

6+
function normalizeAuthProfileCredential(credential: AuthProfileCredential): AuthProfileCredential {
7+
if (credential.type === "api_key") {
8+
return {
9+
...credential,
10+
...(typeof credential.key === "string" ? { key: normalizeSecretInput(credential.key) } : {}),
11+
};
12+
}
13+
if (credential.type === "token") {
14+
return { ...credential, token: normalizeSecretInput(credential.token) };
15+
}
16+
return credential;
17+
}
18+
519
export async function upsertAuthProfileWithLock(params: {
620
profileId: string;
721
credential: AuthProfileCredential;
@@ -11,10 +25,15 @@ export async function upsertAuthProfileWithLock(params: {
1125
ensureAuthStoreFile(authPath);
1226

1327
try {
28+
const credential = normalizeAuthProfileCredential(params.credential);
1429
return await updateAuthProfileStoreWithLock({
1530
agentDir: params.agentDir,
31+
saveOptions: {
32+
filterExternalAuthProfiles: false,
33+
syncExternalCli: false,
34+
},
1635
updater: (store) => {
17-
store.profiles[params.profileId] = params.credential;
36+
store.profiles[params.profileId] = credential;
1837
return true;
1938
},
2039
});

‎src/commands/models/auth.test.ts‎

Lines changed: 22 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,7 @@ const mocks = vi.hoisted(() => ({
4646
resolveAgentWorkspaceDir: vi.fn(),
4747
resolveDefaultAgentWorkspaceDir: vi.fn(),
4848
upsertAuthProfile: vi.fn(),
49+
upsertAuthProfileWithLock: vi.fn(),
4950
resolvePluginProviders: vi.fn(),
5051
createClackPrompter: vi.fn(),
5152
loadValidConfigOrThrow: vi.fn(),
@@ -65,6 +66,7 @@ vi.mock("../../agents/auth-profiles/profiles.js", () => ({
6566
listProfilesForProvider: mocks.listProfilesForProvider,
6667
promoteAuthProfileInOrder: mocks.promoteAuthProfileInOrder,
6768
upsertAuthProfile: mocks.upsertAuthProfile,
69+
upsertAuthProfileWithLock: mocks.upsertAuthProfileWithLock,
6870
}));
6971

7072
vi.mock("../../agents/auth-profiles/store.js", () => ({
@@ -322,7 +324,8 @@ describe("modelsAuthLoginCommand", () => {
322324
);
323325
mocks.clackSelect.mockReset();
324326
mocks.clackText.mockReset();
325-
mocks.upsertAuthProfile.mockReset();
327+
mocks.upsertAuthProfileWithLock.mockReset();
328+
mocks.upsertAuthProfileWithLock.mockResolvedValue({ version: 1, profiles: {} });
326329
mocks.promoteAuthProfileInOrder.mockReset();
327330

328331
mocks.resolveDefaultAgentId.mockReturnValue("main");
@@ -437,7 +440,7 @@ describe("modelsAuthLoginCommand", () => {
437440
runProviderAuth.mock.invocationCallOrder[0],
438441
);
439442
expect(runProviderAuth).toHaveBeenCalledOnce();
440-
const upsertCall = readMockCallArg(mocks.upsertAuthProfile) as UpsertAuthProfileCall;
443+
const upsertCall = readMockCallArg(mocks.upsertAuthProfileWithLock) as UpsertAuthProfileCall;
441444
expect(upsertCall.profileId).toBe("openai-codex:[email protected]");
442445
expect(upsertCall.credential?.type).toBe("oauth");
443446
expect(upsertCall.credential?.provider).toBe("openai-codex");
@@ -735,9 +738,9 @@ describe("modelsAuthLoginCommand", () => {
735738
const authRunCall = readMockCallArg(runProviderAuth) as AuthRunCall;
736739
expect(authRunCall.agentDir).toBe("/tmp/openclaw/agents/coder");
737740
expect(authRunCall.workspaceDir).toBe("/tmp/openclaw/workspaces/coder");
738-
expect((readMockCallArg(mocks.upsertAuthProfile) as UpsertAuthProfileCall).agentDir).toBe(
739-
"/tmp/openclaw/agents/coder",
740-
);
741+
expect(
742+
(readMockCallArg(mocks.upsertAuthProfileWithLock) as UpsertAuthProfileCall).agentDir,
743+
).toBe("/tmp/openclaw/agents/coder");
741744
});
742745

743746
it("loads the owning plugin for an explicit provider even in a clean config", async () => {
@@ -791,7 +794,7 @@ describe("modelsAuthLoginCommand", () => {
791794
expect(providerResolutionCall.providerRefs).toEqual(["anthropic"]);
792795
expect(providerResolutionCall.activate).toBe(true);
793796
expect(runClaudeCliMigration).toHaveBeenCalledOnce();
794-
expect(mocks.upsertAuthProfile).not.toHaveBeenCalled();
797+
expect(mocks.upsertAuthProfileWithLock).not.toHaveBeenCalled();
795798
expect(lastUpdatedConfig?.agents?.defaults?.model).toEqual({
796799
primary: "claude-cli/claude-sonnet-4-6",
797800
});
@@ -928,7 +931,7 @@ describe("modelsAuthLoginCommand", () => {
928931
(order) => order < runClaudeCliMigration.mock.invocationCallOrder[0],
929932
),
930933
).toBe(true);
931-
expect(mocks.upsertAuthProfile).not.toHaveBeenCalled();
934+
expect(mocks.upsertAuthProfileWithLock).not.toHaveBeenCalled();
932935
expect(lastUpdatedConfig?.agents?.defaults?.model).toEqual({
933936
primary: "claude-cli/claude-sonnet-4-6",
934937
fallbacks: ["claude-cli/claude-opus-4-6", "openai/gpt-5.2"],
@@ -1125,7 +1128,7 @@ describe("modelsAuthLoginCommand", () => {
11251128
"exit:0",
11261129
);
11271130

1128-
expect(mocks.upsertAuthProfile).not.toHaveBeenCalled();
1131+
expect(mocks.upsertAuthProfileWithLock).not.toHaveBeenCalled();
11291132
expect(mocks.updateConfig).not.toHaveBeenCalled();
11301133
expect(mocks.logConfigUpdated).not.toHaveBeenCalled();
11311134
} finally {
@@ -1139,7 +1142,7 @@ describe("modelsAuthLoginCommand", () => {
11391142

11401143
await modelsAuthPasteTokenCommand({ provider: "anthropic" }, runtime);
11411144

1142-
expect(mocks.upsertAuthProfile).toHaveBeenCalledWith({
1145+
expect(mocks.upsertAuthProfileWithLock).toHaveBeenCalledWith({
11431146
profileId: "anthropic:manual",
11441147
credential: {
11451148
type: "token",
@@ -1167,7 +1170,7 @@ describe("modelsAuthLoginCommand", () => {
11671170
await modelsAuthPasteTokenCommand({ provider: "openai", agent: "coder" }, runtime);
11681171

11691172
expect(mocks.resolveDefaultAgentId).not.toHaveBeenCalled();
1170-
expect(mocks.upsertAuthProfile).toHaveBeenCalledWith({
1173+
expect(mocks.upsertAuthProfileWithLock).toHaveBeenCalledWith({
11711174
profileId: "openai:manual",
11721175
credential: {
11731176
type: "token",
@@ -1189,7 +1192,7 @@ describe("modelsAuthLoginCommand", () => {
11891192
);
11901193

11911194
expect(mocks.clackText).not.toHaveBeenCalled();
1192-
expect(mocks.upsertAuthProfile).not.toHaveBeenCalled();
1195+
expect(mocks.upsertAuthProfileWithLock).not.toHaveBeenCalled();
11931196
expect(mocks.updateConfig).not.toHaveBeenCalled();
11941197
});
11951198

@@ -1225,7 +1228,7 @@ describe("modelsAuthLoginCommand", () => {
12251228
await modelsAuthSetupTokenCommand({ provider: "moonshot", yes: true }, runtime);
12261229

12271230
expect(runTokenAuth).toHaveBeenCalledOnce();
1228-
expect(mocks.upsertAuthProfile).toHaveBeenCalledWith({
1231+
expect(mocks.upsertAuthProfileWithLock).toHaveBeenCalledWith({
12291232
profileId: "moonshot:token",
12301233
credential: {
12311234
type: "token",
@@ -1272,9 +1275,9 @@ describe("modelsAuthLoginCommand", () => {
12721275
const tokenAuthCall = readMockCallArg(runTokenAuth) as AuthRunCall;
12731276
expect(tokenAuthCall.agentDir).toBe("/tmp/openclaw/agents/coder");
12741277
expect(tokenAuthCall.workspaceDir).toBe("/tmp/openclaw/workspaces/coder");
1275-
expect((readMockCallArg(mocks.upsertAuthProfile) as UpsertAuthProfileCall).agentDir).toBe(
1276-
"/tmp/openclaw/agents/coder",
1277-
);
1278+
expect(
1279+
(readMockCallArg(mocks.upsertAuthProfileWithLock) as UpsertAuthProfileCall).agentDir,
1280+
).toBe("/tmp/openclaw/agents/coder");
12781281
});
12791282

12801283
it("uses the requested agent store for interactive token auth add", async () => {
@@ -1314,9 +1317,9 @@ describe("modelsAuthLoginCommand", () => {
13141317
const tokenAuthCall = readMockCallArg(runTokenAuth) as AuthRunCall;
13151318
expect(tokenAuthCall.agentDir).toBe("/tmp/openclaw/agents/coder");
13161319
expect(tokenAuthCall.workspaceDir).toBe("/tmp/openclaw/workspaces/coder");
1317-
expect((readMockCallArg(mocks.upsertAuthProfile) as UpsertAuthProfileCall).agentDir).toBe(
1318-
"/tmp/openclaw/agents/coder",
1319-
);
1320+
expect(
1321+
(readMockCallArg(mocks.upsertAuthProfileWithLock) as UpsertAuthProfileCall).agentDir,
1322+
).toBe("/tmp/openclaw/agents/coder");
13201323
});
13211324

13221325
it("keeps the requested agent store when interactive auth add falls back to paste-token", async () => {
@@ -1333,7 +1336,7 @@ describe("modelsAuthLoginCommand", () => {
13331336
await modelsAuthAddCommand({ agent: "coder" }, runtime);
13341337

13351338
expect(mocks.resolveDefaultAgentId).not.toHaveBeenCalled();
1336-
expect(mocks.upsertAuthProfile).toHaveBeenCalledWith({
1339+
expect(mocks.upsertAuthProfileWithLock).toHaveBeenCalledWith({
13371340
profileId: "openai:manual",
13381341
credential: {
13391342
type: "token",

0 commit comments

Comments
 (0)