Skip to content

Commit a001b53

Browse files
vincentkocvincentkoc
committed
refactor(auth): make external cli oauth runtime-only
1 parent 50e71da commit a001b53

5 files changed

Lines changed: 104 additions & 60 deletions

File tree

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

Lines changed: 14 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -5,45 +5,42 @@ import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
55
import { AUTH_STORE_VERSION } from "./auth-profiles/constants.js";
66
import type { AuthProfileStore } from "./auth-profiles/types.js";
77

8-
const mocks = vi.hoisted(() => ({
9-
syncExternalCliCredentials: vi.fn((store: AuthProfileStore) => {
10-
store.profiles["minimax-portal:default"] = {
11-
type: "oauth",
8+
const resolveExternalAuthProfilesWithPluginsMock = vi.fn(() => [
9+
{
10+
profileId: "minimax-portal:default",
11+
credential: {
12+
type: "oauth" as const,
1213
provider: "minimax-portal",
1314
access: "access-token",
1415
refresh: "refresh-token",
1516
expires: Date.now() + 60_000,
16-
};
17-
return true;
18-
}),
19-
}));
20-
21-
vi.mock("./auth-profiles/external-cli-sync.js", () => ({
22-
syncExternalCliCredentials: mocks.syncExternalCliCredentials,
23-
}));
17+
},
18+
persistence: "runtime-only" as const,
19+
},
20+
]);
2421

2522
vi.mock("../plugins/provider-runtime.js", () => ({
26-
resolveExternalAuthProfilesWithPlugins: () => [],
23+
resolveExternalAuthProfilesWithPlugins: resolveExternalAuthProfilesWithPluginsMock,
2724
}));
2825

2926
let clearRuntimeAuthProfileStoreSnapshots: typeof import("./auth-profiles.js").clearRuntimeAuthProfileStoreSnapshots;
3027
let loadAuthProfileStoreForRuntime: typeof import("./auth-profiles.js").loadAuthProfileStoreForRuntime;
3128

32-
describe("auth profiles read-only external CLI sync", () => {
29+
describe("auth profiles read-only external auth overlay", () => {
3330
beforeEach(async () => {
3431
vi.resetModules();
3532
({ clearRuntimeAuthProfileStoreSnapshots, loadAuthProfileStoreForRuntime } =
3633
await import("./auth-profiles.js"));
3734
clearRuntimeAuthProfileStoreSnapshots();
38-
mocks.syncExternalCliCredentials.mockClear();
35+
resolveExternalAuthProfilesWithPluginsMock.mockClear();
3936
});
4037

4138
afterEach(() => {
4239
clearRuntimeAuthProfileStoreSnapshots();
4340
vi.clearAllMocks();
4441
});
4542

46-
it("syncs external CLI credentials in-memory without writing auth-profiles.json in read-only mode", () => {
43+
it("overlays runtime-only external auth without writing auth-profiles.json in read-only mode", () => {
4744
const agentDir = fs.mkdtempSync(path.join(os.tmpdir(), "openclaw-auth-readonly-sync-"));
4845
try {
4946
const authPath = path.join(agentDir, "auth-profiles.json");
@@ -61,10 +58,7 @@ describe("auth profiles read-only external CLI sync", () => {
6158

6259
const loaded = loadAuthProfileStoreForRuntime(agentDir, { readOnly: true });
6360

64-
expect(mocks.syncExternalCliCredentials).toHaveBeenCalledWith(
65-
expect.any(Object),
66-
expect.objectContaining({ log: false }),
67-
);
61+
expect(resolveExternalAuthProfilesWithPluginsMock).toHaveBeenCalled();
6862
expect(loaded.profiles["minimax-portal:default"]).toMatchObject({
6963
type: "oauth",
7064
provider: "minimax-portal",

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

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import type { ProviderExternalAuthProfile } from "../../plugins/provider-external-auth.types.js";
22
import { resolveExternalAuthProfilesWithPlugins } from "../../plugins/provider-runtime.js";
3+
import { resolveExternalCliAuthProfiles } from "./external-cli-sync.js";
34
import type { AuthProfileStore, OAuthCredential } from "./types.js";
45

56
type ExternalAuthProfileMap = Map<string, ProviderExternalAuthProfile>;
@@ -48,6 +49,13 @@ function resolveExternalAuthProfileMap(params: {
4849
});
4950

5051
const resolved: ExternalAuthProfileMap = new Map();
52+
for (const profile of resolveExternalCliAuthProfiles(params.store)) {
53+
resolved.set(profile.profileId, {
54+
profileId: profile.profileId,
55+
credential: profile.credential,
56+
persistence: "runtime-only",
57+
});
58+
}
5159
for (const rawProfile of profiles) {
5260
const profile = normalizeExternalAuthProfile(rawProfile);
5361
if (!profile) {

‎src/agents/auth-profiles/external-cli-sync.ts‎

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,11 @@ type ExternalCliSyncOptions = {
1414
log?: boolean;
1515
};
1616

17+
export type ExternalCliResolvedProfile = {
18+
profileId: string;
19+
credential: OAuthCredential;
20+
};
21+
1722
type ExternalCliSyncProvider = {
1823
profileId: string;
1924
provider: string;
@@ -94,6 +99,11 @@ function withExternalCliManager(
9499
};
95100
}
96101

102+
function stripExternalCliManager(creds: OAuthCredential): OAuthCredential {
103+
const { managedBy: _managedBy, ...runtimeCredential } = creds;
104+
return runtimeCredential;
105+
}
106+
97107
function resolveExternalCliSyncProvider(params: {
98108
profileId?: string;
99109
credential?: OAuthCredential;
@@ -133,6 +143,34 @@ export function readManagedExternalCliCredential(params: {
133143
return withExternalCliManager(creds, provider.managedBy);
134144
}
135145

146+
export function resolveExternalCliAuthProfiles(
147+
store: AuthProfileStore,
148+
): ExternalCliResolvedProfile[] {
149+
const profiles: ExternalCliResolvedProfile[] = [];
150+
for (const providerConfig of EXTERNAL_CLI_SYNC_PROVIDERS) {
151+
const creds = providerConfig.readCredentials();
152+
if (!creds) {
153+
continue;
154+
}
155+
const runtimeCredential = stripExternalCliManager(
156+
withExternalCliManager(creds, providerConfig.managedBy),
157+
);
158+
const existing = store.profiles[providerConfig.profileId];
159+
const existingOAuth = existing?.type === "oauth" ? existing : undefined;
160+
if (
161+
!shouldReplaceStoredOAuthCredential(existingOAuth, runtimeCredential) &&
162+
!areOAuthCredentialsEquivalent(existingOAuth, runtimeCredential)
163+
) {
164+
continue;
165+
}
166+
profiles.push({
167+
profileId: providerConfig.profileId,
168+
credential: runtimeCredential,
169+
});
170+
}
171+
return profiles;
172+
}
173+
136174
/** Sync external CLI credentials into the store for a given provider. */
137175
function syncExternalCliCredentialsForProvider(
138176
store: AuthProfileStore,

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

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,14 @@ import type { AuthProfileStore, OAuthCredential } from "./types.js";
1010
const resolveExternalAuthProfilesWithPluginsMock = vi.fn<
1111
(params: unknown) => ProviderExternalAuthProfile[]
1212
>(() => []);
13+
const { readCodexCliCredentialsCachedMock } = vi.hoisted(() => ({
14+
readCodexCliCredentialsCachedMock: vi.fn<() => OAuthCredential | null>(() => null),
15+
}));
16+
17+
vi.mock("../cli-credentials.js", () => ({
18+
readCodexCliCredentialsCached: readCodexCliCredentialsCachedMock,
19+
readMiniMaxCliCredentialsCached: () => null,
20+
}));
1321

1422
function createStore(profiles: AuthProfileStore["profiles"] = {}): AuthProfileStore {
1523
return { version: 1, profiles };
@@ -30,6 +38,8 @@ describe("auth external oauth helpers", () => {
3038
beforeEach(() => {
3139
resolveExternalAuthProfilesWithPluginsMock.mockReset();
3240
resolveExternalAuthProfilesWithPluginsMock.mockReturnValue([]);
41+
readCodexCliCredentialsCachedMock.mockReset();
42+
readCodexCliCredentialsCachedMock.mockReturnValue(null);
3343
__testing.setResolveExternalAuthProfilesForTest(resolveExternalAuthProfilesWithPluginsMock);
3444
});
3545

@@ -108,4 +118,38 @@ describe("auth external oauth helpers", () => {
108118

109119
expect(shouldPersist).toBe(true);
110120
});
121+
122+
it("overlays fresher external CLI OAuth credentials without treating them as persisted store state", () => {
123+
readCodexCliCredentialsCachedMock.mockReturnValue(
124+
createCredential({
125+
access: "fresh-cli-access-token",
126+
refresh: "fresh-cli-refresh-token",
127+
expires: 456,
128+
}),
129+
);
130+
131+
const overlaid = overlayExternalOAuthProfiles(
132+
createStore({
133+
"openai-codex:default": createCredential({
134+
access: "stale-store-access-token",
135+
refresh: "stale-store-refresh-token",
136+
expires: 123,
137+
}),
138+
}),
139+
);
140+
141+
expect(overlaid.profiles["openai-codex:default"]).toMatchObject({
142+
access: "fresh-cli-access-token",
143+
refresh: "fresh-cli-refresh-token",
144+
expires: 456,
145+
});
146+
147+
const shouldPersist = shouldPersistExternalOAuthProfile({
148+
store: overlaid,
149+
profileId: "openai-codex:default",
150+
credential: overlaid.profiles["openai-codex:default"] as OAuthCredential,
151+
});
152+
153+
expect(shouldPersist).toBe(false);
154+
});
111155
});

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

Lines changed: 0 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,6 @@ import {
88
log,
99
} from "./constants.js";
1010
import { overlayExternalAuthProfiles, shouldPersistExternalAuthProfile } from "./external-auth.js";
11-
import { syncExternalCliCredentials } from "./external-cli-sync.js";
1211
import {
1312
ensureAuthStoreFile,
1413
resolveAuthStatePath,
@@ -149,34 +148,9 @@ export async function updateAuthProfileStoreWithLock(params: {
149148
}
150149
}
151150

152-
function shouldLogAuthStoreTiming(): boolean {
153-
return process.env.OPENCLAW_DEBUG_INGRESS_TIMING === "1";
154-
}
155-
156-
function syncExternalCliCredentialsTimed(
157-
store: AuthProfileStore,
158-
options?: Parameters<typeof syncExternalCliCredentials>[1],
159-
): boolean {
160-
if (!shouldLogAuthStoreTiming()) {
161-
return syncExternalCliCredentials(store, options);
162-
}
163-
const startMs = Date.now();
164-
const mutated = syncExternalCliCredentials(store, options);
165-
log.info(
166-
`auth-store stage=external-cli-sync elapsedMs=${Date.now() - startMs} mutated=${mutated}`,
167-
);
168-
return mutated;
169-
}
170-
171-
function shouldSyncExternalCliCredentials(options?: { syncExternalCli?: boolean }): boolean {
172-
return options?.syncExternalCli !== false;
173-
}
174-
175151
export function loadAuthProfileStore(): AuthProfileStore {
176152
const asStore = loadPersistedAuthProfileStore();
177153
if (asStore) {
178-
// Sync from external CLI tools on every load.
179-
syncExternalCliCredentialsTimed(asStore);
180154
return overlayExternalAuthProfiles(asStore);
181155
}
182156
const legacy = loadLegacyAuthProfileStore();
@@ -186,12 +160,10 @@ export function loadAuthProfileStore(): AuthProfileStore {
186160
profiles: {},
187161
};
188162
applyLegacyAuthStore(store, legacy);
189-
syncExternalCliCredentialsTimed(store);
190163
return overlayExternalAuthProfiles(store);
191164
}
192165

193166
const store: AuthProfileStore = { version: AUTH_STORE_VERSION, profiles: {} };
194-
syncExternalCliCredentialsTimed(store);
195167
return overlayExternalAuthProfiles(store);
196168
}
197169

@@ -216,11 +188,6 @@ function loadAuthProfileStoreForAgent(
216188
}
217189
const asStore = loadPersistedAuthProfileStore(agentDir);
218190
if (asStore) {
219-
// Runtime secret activation must remain read-only:
220-
// sync external CLI credentials in-memory, but never persist while readOnly.
221-
if (shouldSyncExternalCliCredentials(options)) {
222-
syncExternalCliCredentialsTimed(asStore, { log: !readOnly });
223-
}
224191
if (!readOnly) {
225192
writeCachedAuthProfileStore({
226193
authPath,
@@ -260,10 +227,6 @@ function loadAuthProfileStoreForAgent(
260227
}
261228

262229
const mergedOAuth = mergeOAuthFileIntoStore(store);
263-
// Keep external CLI credentials visible in runtime even during read-only loads.
264-
if (shouldSyncExternalCliCredentials(options)) {
265-
syncExternalCliCredentialsTimed(store, { log: !readOnly });
266-
}
267230
const forceReadOnly = process.env.OPENCLAW_AUTH_STORE_READONLY === "1";
268231
const shouldWrite = !readOnly && !forceReadOnly && (legacy !== null || mergedOAuth);
269232
if (shouldWrite) {
@@ -394,9 +357,6 @@ export function saveAuthProfileStore(
394357
saveJsonFile(authPath, payload);
395358
savePersistedAuthProfileState(store, agentDir);
396359
const runtimeStore = cloneAuthProfileStore(store);
397-
if (shouldSyncExternalCliCredentials(options)) {
398-
syncExternalCliCredentialsTimed(runtimeStore, { log: false });
399-
}
400360
writeCachedAuthProfileStore({
401361
authPath,
402362
authMtimeMs: readAuthStoreMtimeMs(authPath),

0 commit comments

Comments
 (0)