Skip to content

Commit 2827f61

Browse files
pgondhi987pgondhi987
committed
addressing review-skill
1 parent 520f279 commit 2827f61

3 files changed

Lines changed: 142 additions & 2 deletions

File tree

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

Lines changed: 48 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,10 @@ type OAuthProfileSecretPayload = {
5050
idToken?: string;
5151
};
5252

53+
type LoadPersistedAuthProfileStoreOptions = {
54+
rewriteInlineOAuthSecrets?: boolean;
55+
};
56+
5357
function normalizeSecretBackedField(params: {
5458
entry: Record<string, unknown>;
5559
valueField: "key" | "token";
@@ -185,6 +189,10 @@ function omitInlineOAuthSecrets(params: {
185189
return sanitized as AuthProfileCredential;
186190
}
187191

192+
function hasInlinePersistableOAuthSecrets(credential: AuthProfileCredential): boolean {
193+
return shouldPersistOAuthWithoutInlineSecrets(credential) && hasInlineOAuthSecrets(credential);
194+
}
195+
188196
function parseCredentialEntry(
189197
raw: unknown,
190198
fallbackProvider?: string,
@@ -787,7 +795,39 @@ function resolvePersistedOAuthProfileSecrets(store: AuthProfileStore): AuthProfi
787795
};
788796
}
789797

790-
export function loadPersistedAuthProfileStore(agentDir?: string): AuthProfileStore | null {
798+
function buildPersistedAuthProfileFilePayload(params: {
799+
store: AuthProfileStore;
800+
raw: unknown;
801+
agentDir?: string;
802+
}): AuthProfileSecretsStore & Partial<AuthProfileStore> {
803+
const payload = buildPersistedAuthProfileSecretsStore(params.store, undefined, {
804+
agentDir: params.agentDir,
805+
}) as AuthProfileSecretsStore & Partial<AuthProfileStore>;
806+
const state = coerceAuthProfileState(params.raw);
807+
return {
808+
...payload,
809+
...(state.order ? { order: state.order } : {}),
810+
...(state.lastGood ? { lastGood: state.lastGood } : {}),
811+
...(state.usageStats ? { usageStats: state.usageStats } : {}),
812+
};
813+
}
814+
815+
function rewritePersistedInlineOAuthSecrets(params: {
816+
authPath: string;
817+
raw: unknown;
818+
store: AuthProfileStore;
819+
agentDir?: string;
820+
}): void {
821+
if (!Object.values(params.store.profiles).some(hasInlinePersistableOAuthSecrets)) {
822+
return;
823+
}
824+
saveJsonFile(params.authPath, buildPersistedAuthProfileFilePayload(params));
825+
}
826+
827+
export function loadPersistedAuthProfileStore(
828+
agentDir?: string,
829+
options?: LoadPersistedAuthProfileStoreOptions,
830+
): AuthProfileStore | null {
791831
const authPath = resolveAuthStorePath(agentDir);
792832
const raw = loadJsonFile(authPath);
793833
const store = coercePersistedAuthProfileStore(raw);
@@ -798,6 +838,13 @@ export function loadPersistedAuthProfileStore(agentDir?: string): AuthProfileSto
798838
...store,
799839
...mergeAuthProfileState(coerceAuthProfileState(raw), loadPersistedAuthProfileState(agentDir)),
800840
};
841+
if (options?.rewriteInlineOAuthSecrets !== false) {
842+
try {
843+
rewritePersistedInlineOAuthSecrets({ authPath, raw, store: merged, agentDir });
844+
} catch (err) {
845+
log.warn("failed to rewrite inline oauth auth profile secrets", { err, authPath });
846+
}
847+
}
801848
return resolvePersistedOAuthProfileSecrets(merged);
802849
}
803850

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

Lines changed: 91 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -110,6 +110,97 @@ describe("promoteAuthProfileInOrder", () => {
110110
}
111111
});
112112

113+
it("rewrites existing inline openai-codex oauth secrets during runtime load", () => {
114+
const stateDir = fs.mkdtempSync(path.join(os.tmpdir(), "openclaw-auth-profile-rewrite-"));
115+
const agentDir = path.join(stateDir, "agents", "main", "agent");
116+
const previousStateDir = process.env.OPENCLAW_STATE_DIR;
117+
process.env.OPENCLAW_STATE_DIR = stateDir;
118+
try {
119+
fs.mkdirSync(agentDir, { recursive: true });
120+
const profileId = "openai-codex:default";
121+
const expires = Date.now() + 60 * 60 * 1000;
122+
fs.writeFileSync(
123+
resolveAuthStorePath(agentDir),
124+
`${JSON.stringify(
125+
{
126+
version: AUTH_STORE_VERSION,
127+
profiles: {
128+
[profileId]: {
129+
type: "oauth",
130+
provider: "openai-codex",
131+
access: "existing-access-token",
132+
refresh: "existing-refresh-token",
133+
idToken: "existing-id-token",
134+
expires,
135+
accountId: "acct-existing",
136+
},
137+
},
138+
order: {
139+
"openai-codex": [profileId],
140+
},
141+
},
142+
null,
143+
2,
144+
)}\n`,
145+
);
146+
147+
expect(
148+
loadAuthProfileStoreForRuntime(agentDir, { externalCli: { mode: "none" } }).profiles[
149+
profileId
150+
],
151+
).toMatchObject({
152+
type: "oauth",
153+
provider: "openai-codex",
154+
access: "existing-access-token",
155+
refresh: "existing-refresh-token",
156+
idToken: "existing-id-token",
157+
});
158+
159+
const persisted = JSON.parse(fs.readFileSync(resolveAuthStorePath(agentDir), "utf8")) as {
160+
profiles: Record<string, Record<string, unknown>>;
161+
order?: Record<string, string[]>;
162+
};
163+
const credential = persisted.profiles[profileId];
164+
expect(credential).toMatchObject({
165+
type: "oauth",
166+
provider: "openai-codex",
167+
expires,
168+
accountId: "acct-existing",
169+
oauthRef: {
170+
source: "openclaw-credentials",
171+
provider: "openai-codex",
172+
id: expect.any(String),
173+
},
174+
});
175+
expect(persisted.order?.["openai-codex"]).toEqual([profileId]);
176+
expect(credential).not.toHaveProperty("access");
177+
expect(credential).not.toHaveProperty("refresh");
178+
expect(credential).not.toHaveProperty("idToken");
179+
const persistedAgentTree = readPersistedAgentTree(agentDir);
180+
expect(persistedAgentTree).not.toContain("existing-access-token");
181+
expect(persistedAgentTree).not.toContain("existing-refresh-token");
182+
expect(persistedAgentTree).not.toContain("existing-id-token");
183+
184+
clearRuntimeAuthProfileStoreSnapshots();
185+
expect(
186+
loadAuthProfileStoreWithoutExternalProfiles(agentDir).profiles[profileId],
187+
).toMatchObject({
188+
type: "oauth",
189+
provider: "openai-codex",
190+
access: "existing-access-token",
191+
refresh: "existing-refresh-token",
192+
idToken: "existing-id-token",
193+
});
194+
} finally {
195+
if (previousStateDir === undefined) {
196+
delete process.env.OPENCLAW_STATE_DIR;
197+
} else {
198+
process.env.OPENCLAW_STATE_DIR = previousStateDir;
199+
}
200+
fs.rmSync(stateDir, { recursive: true, force: true });
201+
}
202+
});
203+
113204
it("moves a relogin profile to the front of an existing per-agent provider order", async () => {
114205
const stateDir = fs.mkdtempSync(path.join(os.tmpdir(), "openclaw-auth-order-promote-"));
115206
const agentDir = path.join(stateDir, "agents", "main", "agent");

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

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -371,7 +371,9 @@ function loadAuthProfileStoreForAgent(
371371
return cached;
372372
}
373373
}
374-
const asStore = loadPersistedAuthProfileStore(agentDir);
374+
const asStore = loadPersistedAuthProfileStore(agentDir, {
375+
rewriteInlineOAuthSecrets: !readOnly && process.env.OPENCLAW_AUTH_STORE_READONLY !== "1",
376+
});
375377
if (asStore) {
376378
if (!readOnly) {
377379
writeCachedAuthProfileStore({

0 commit comments

Comments
 (0)