Skip to content

Commit 17ceca8

Browse files
pgondhi987pgondhi987
authored
Redact persisted secret-shaped payloads [AI] (#79006)
* fix: redact persisted secret-shaped payloads * docs: add changelog entry for PR merge
1 parent be8bf35 commit 17ceca8

33 files changed

Lines changed: 2772 additions & 215 deletions

‎CHANGELOG.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,7 @@ Docs: https://docs.openclaw.ai
4848

4949
### Fixes
5050

51+
- Redact persisted secret-shaped payloads [AI]. (#79006) Thanks @pgondhi987.
5152
- OpenAI Codex: surface browser OAuth and device-code login failures instead of treating failed logins as empty successful auth results. Refs #80363.
5253
- CLI agents: carry runtime-only current-turn sender/reply context into CLI model prompts while keeping prompt-build hook input and transcript text clean.
5354
- fix(matrix): gate name-based allowlist resolution [AI]. (#79007) Thanks @pgondhi987.

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

Lines changed: 13 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -816,14 +816,23 @@ describe("ensureAuthProfileStore", () => {
816816
const persisted = JSON.parse(
817817
fs.readFileSync(path.join(agentDir, "auth-profiles.json"), "utf8"),
818818
) as {
819-
profiles: Record<string, unknown>;
819+
profiles: Record<string, Record<string, unknown>>;
820820
};
821-
expectRecordFields(persisted.profiles["openai-codex:default"], {
821+
const persistedProfile = persisted.profiles["openai-codex:default"];
822+
expect(persistedProfile).toMatchObject({
822823
type: "oauth",
823824
provider: "openai-codex",
824-
access: "access-token",
825-
refresh: "refresh-token",
825+
oauthRef: {
826+
source: "openclaw-credentials",
827+
provider: "openai-codex",
828+
id: expect.any(String),
829+
},
826830
});
831+
expect(persistedProfile).not.toHaveProperty("access");
832+
expect(persistedProfile).not.toHaveProperty("refresh");
833+
expect(persistedProfile).not.toHaveProperty("idToken");
834+
expect(JSON.stringify(persisted)).not.toContain("access-token");
835+
expect(JSON.stringify(persisted)).not.toContain("refresh-token");
827836
} finally {
828837
clearRuntimeAuthProfileStoreSnapshots();
829838
restoreEnvValue("OPENCLAW_STATE_DIR", previousStateDir);

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

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -145,6 +145,12 @@ function resolveExternalCliSyncProvider(params: {
145145
return provider;
146146
}
147147

148+
function hasInlineOAuthTokenMaterial(credential: OAuthCredential): boolean {
149+
return [credential.access, credential.refresh, credential.idToken].some(
150+
(value) => typeof value === "string" && value.trim().length > 0,
151+
);
152+
}
153+
148154
export function readExternalCliBootstrapCredential(params: {
149155
profileId: string;
150156
credential: OAuthCredential;
@@ -153,11 +159,7 @@ export function readExternalCliBootstrapCredential(params: {
153159
if (!provider) {
154160
return null;
155161
}
156-
// bootstrapOnly providers must not replace an existing local credential
157-
// during runtime refresh. The oauth-manager only calls this hook when a
158-
// local credential is already present, so returning null here keeps the
159-
// locally stored refresh token canonical.
160-
if (provider.bootstrapOnly) {
162+
if (provider.bootstrapOnly && hasInlineOAuthTokenMaterial(params.credential)) {
161163
return null;
162164
}
163165
return provider.readCredentials();
@@ -248,7 +250,11 @@ export function resolveExternalCliAuthProfiles(
248250
});
249251
continue;
250252
}
251-
if (providerConfig.bootstrapOnly && existingOAuth) {
253+
if (
254+
providerConfig.bootstrapOnly &&
255+
existingOAuth &&
256+
hasInlineOAuthTokenMaterial(existingOAuth)
257+
) {
252258
log.debug("kept local oauth over external cli bootstrap-only provider", {
253259
profileId: providerConfig.profileId,
254260
provider: providerConfig.provider,

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

Lines changed: 40 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import {
55
overlayExternalOAuthProfiles,
66
shouldPersistExternalOAuthProfile,
77
} from "./external-auth.js";
8+
import { readManagedExternalCliCredential } from "./external-cli-sync.js";
89
import type { AuthProfileStore, OAuthCredential } from "./types.js";
910

1011
const resolveExternalAuthProfilesWithPluginsMock = vi.fn<
@@ -158,7 +159,7 @@ describe("auth external oauth helpers", () => {
158159
expect(shouldPersist).toBe(true);
159160
});
160161

161-
it("does not use Codex CLI OAuth as a runtime overlay source", () => {
162+
it("keeps Codex CLI OAuth from replacing stored inline token material", () => {
162163
readCodexCliCredentialsCachedMock.mockReturnValue(
163164
createCredential({
164165
access: "fresh-cli-access-token",
@@ -185,6 +186,44 @@ describe("auth external oauth helpers", () => {
185186
expect(profile.accountId).toBe("acct-cli");
186187
});
187188

189+
it("uses Codex CLI OAuth when the stored Codex profile has no inline token material", () => {
190+
const cliCredential = createCredential({
191+
access: "fresh-cli-access-token",
192+
refresh: "fresh-cli-refresh-token",
193+
expires: createUsableOAuthExpiry(),
194+
accountId: "acct-cli",
195+
});
196+
const tokenlessCredential = {
197+
type: "oauth",
198+
provider: "openai-codex",
199+
expires: Date.now() - 60_000,
200+
accountId: "acct-cli",
201+
} as OAuthCredential;
202+
readCodexCliCredentialsCachedMock.mockReturnValue(cliCredential);
203+
204+
const overlaid = overlayExternalOAuthProfiles(
205+
createStore({
206+
"openai-codex:default": tokenlessCredential,
207+
}),
208+
);
209+
210+
expect(overlaid.profiles["openai-codex:default"]).toMatchObject({
211+
access: "fresh-cli-access-token",
212+
refresh: "fresh-cli-refresh-token",
213+
accountId: "acct-cli",
214+
});
215+
expect(
216+
readManagedExternalCliCredential({
217+
profileId: "openai-codex:default",
218+
credential: tokenlessCredential,
219+
}),
220+
).toMatchObject({
221+
access: "fresh-cli-access-token",
222+
refresh: "fresh-cli-refresh-token",
223+
accountId: "acct-cli",
224+
});
225+
});
226+
188227
it("keeps healthy local oauth even when external cli has a fresher token", () => {
189228
readCodexCliCredentialsCachedMock.mockReturnValue(
190229
createCredential({

‎src/agents/auth-profiles/oauth.adopt-identity.test.ts‎

Lines changed: 20 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -29,18 +29,18 @@ const {
2929
formatProviderAuthProfileApiKeyWithPluginMock,
3030
} = getOAuthProviderRuntimeMocks();
3131

32-
function expectOAuthProfileFields(
33-
store: AuthProfileStore,
34-
profileId: string,
35-
params: { access: string; accountId: string },
36-
) {
37-
const credential = store.profiles[profileId];
38-
expect(credential?.type).toBe("oauth");
39-
if (credential?.type !== "oauth") {
40-
throw new Error(`Expected OAuth credential for ${profileId}`);
41-
}
42-
expect(credential.access).toBe(params.access);
43-
expect(credential.accountId).toBe(params.accountId);
32+
function expectPersistedOpenAICodexProfileWithoutInlineTokens(
33+
credential: AuthProfileStore["profiles"][string],
34+
metadata: Record<string, unknown> = {},
35+
): void {
36+
expect(credential).toMatchObject({
37+
type: "oauth",
38+
provider: "openai-codex",
39+
...metadata,
40+
});
41+
expect(credential).not.toHaveProperty("access");
42+
expect(credential).not.toHaveProperty("refresh");
43+
expect(credential).not.toHaveProperty("idToken");
4444
}
4545

4646
// Cross-account-leak defense-in-depth: each adopt site in oauth.ts calls the
@@ -138,10 +138,11 @@ describe("OAuth credential adoption is identity-gated", () => {
138138
const subRaw = JSON.parse(
139139
await fs.readFile(path.join(subAgentDir, "auth-profiles.json"), "utf8"),
140140
) as AuthProfileStore;
141-
expectOAuthProfileFields(subRaw, profileId, {
142-
access: "sub-own-access",
141+
expectPersistedOpenAICodexProfileWithoutInlineTokens(subRaw.profiles[profileId], {
143142
accountId: "acct-sub",
143+
expires: subExpiry,
144144
});
145+
expect(JSON.stringify(subRaw)).not.toContain("sub-own-access");
145146
});
146147

147148
it("inside-the-lock main adoption refuses across accountId mismatch and proceeds to own refresh", async () => {
@@ -210,10 +211,11 @@ describe("OAuth credential adoption is identity-gated", () => {
210211
const mainRaw = JSON.parse(
211212
await fs.readFile(path.join(mainAgentDir, "auth-profiles.json"), "utf8"),
212213
) as AuthProfileStore;
213-
expectOAuthProfileFields(mainRaw, profileId, {
214-
access: "main-foreign-access",
214+
expectPersistedOpenAICodexProfileWithoutInlineTokens(mainRaw.profiles[profileId], {
215215
accountId: "acct-other",
216+
expires: freshExpiry,
216217
});
218+
expect(JSON.stringify(mainRaw)).not.toContain("main-foreign-access");
217219
});
218220

219221
it("catch-block main-inherit refuses across accountId mismatch and surfaces the original error", async () => {
@@ -286,9 +288,9 @@ describe("OAuth credential adoption is identity-gated", () => {
286288
const subRaw = JSON.parse(
287289
await fs.readFile(path.join(subAgentDir, "auth-profiles.json"), "utf8"),
288290
) as AuthProfileStore;
289-
expectOAuthProfileFields(subRaw, profileId, {
290-
access: "sub-stale",
291+
expectPersistedOpenAICodexProfileWithoutInlineTokens(subRaw.profiles[profileId], {
291292
accountId: "acct-sub",
292293
});
294+
expect(JSON.stringify(subRaw)).not.toContain("sub-stale");
293295
});
294296
});

‎src/agents/auth-profiles/oauth.mirror-refresh.test.ts‎

Lines changed: 45 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,20 @@ const {
2828
formatProviderAuthProfileApiKeyWithPluginMock,
2929
} = getOAuthProviderRuntimeMocks();
3030

31+
function expectPersistedOpenAICodexProfileWithoutInlineTokens(
32+
credential: AuthProfileStore["profiles"][string],
33+
metadata: Record<string, unknown> = {},
34+
): void {
35+
expect(credential).toMatchObject({
36+
type: "oauth",
37+
provider: "openai-codex",
38+
...metadata,
39+
});
40+
expect(credential).not.toHaveProperty("access");
41+
expect(credential).not.toHaveProperty("refresh");
42+
expect(credential).not.toHaveProperty("idToken");
43+
}
44+
3145
function requireOAuthCredential(store: AuthProfileStore, profileId: string): OAuthCredential {
3246
const profile = store.profiles[profileId];
3347
if (!profile || profile.type !== "oauth") {
@@ -36,7 +50,7 @@ function requireOAuthCredential(store: AuthProfileStore, profileId: string): OAu
3650
return profile;
3751
}
3852

39-
vi.mock("@earendil-works/pi-ai/oauth", () => ({
53+
vi.mock("@mariozechner/pi-ai/oauth", () => ({
4054
getOAuthProviders: () => [{ id: "anthropic" }, { id: "openai-codex" }],
4155
getOAuthApiKey: vi.fn(async (provider: string, credentials: Record<string, OAuthCredential>) => {
4256
const credential = credentials[provider];
@@ -85,7 +99,7 @@ describe("resolveApiKeyForProfile OAuth refresh mirror-to-main (#26322)", () =>
8599
await removeOAuthTestTempRoot(tempRoot);
86100
});
87101

88-
it("mirrors refreshed credentials into the main store so peers skip refresh", async () => {
102+
it("mirrors refreshed Codex OAuth metadata into the main store without inline tokens", async () => {
89103
const profileId = "openai-codex:default";
90104
const provider = "openai-codex";
91105
const accountId = "acct-shared";
@@ -116,15 +130,17 @@ describe("resolveApiKeyForProfile OAuth refresh mirror-to-main (#26322)", () =>
116130

117131
expect(result?.apiKey).toBe("sub-refreshed-access");
118132

119-
// Main store should now carry the refreshed credential, so a peer agent
120-
// starting fresh will adopt rather than race.
133+
// Main store should now carry refreshed metadata, so a peer agent
134+
// starting fresh can resolve the runtime credential without token races.
121135
const mainRaw = JSON.parse(
122136
await fs.readFile(path.join(mainAgentDir, "auth-profiles.json"), "utf8"),
123137
) as AuthProfileStore;
124-
const mainCredential = requireOAuthCredential(mainRaw, profileId);
125-
expect(mainCredential.access).toBe("sub-refreshed-access");
126-
expect(mainCredential.refresh).toBe("sub-refreshed-refresh");
127-
expect(mainCredential.expires).toBe(freshExpiry);
138+
expectPersistedOpenAICodexProfileWithoutInlineTokens(mainRaw.profiles[profileId], {
139+
expires: freshExpiry,
140+
accountId,
141+
});
142+
expect(JSON.stringify(mainRaw)).not.toContain("sub-refreshed-access");
143+
expect(JSON.stringify(mainRaw)).not.toContain("sub-refreshed-refresh");
128144
});
129145

130146
it("does not mirror when refresh was performed from the main agent itself", async () => {
@@ -161,10 +177,11 @@ describe("resolveApiKeyForProfile OAuth refresh mirror-to-main (#26322)", () =>
161177
const mainRaw = JSON.parse(
162178
await fs.readFile(path.join(mainAgentDir, "auth-profiles.json"), "utf8"),
163179
) as AuthProfileStore;
164-
const mainCredential = requireOAuthCredential(mainRaw, profileId);
165-
expect(mainCredential.access).toBe("main-refreshed-access");
166-
expect(mainCredential.refresh).toBe("main-refreshed-refresh");
167-
expect(mainCredential.expires).toBe(freshExpiry);
180+
expectPersistedOpenAICodexProfileWithoutInlineTokens(mainRaw.profiles[profileId], {
181+
expires: freshExpiry,
182+
});
183+
expect(JSON.stringify(mainRaw)).not.toContain("main-refreshed-access");
184+
expect(JSON.stringify(mainRaw)).not.toContain("main-refreshed-refresh");
168185
expect(refreshProviderOAuthCredentialWithPluginMock).toHaveBeenCalledTimes(1);
169186
});
170187

@@ -332,17 +349,22 @@ describe("resolveApiKeyForProfile OAuth refresh mirror-to-main (#26322)", () =>
332349
const subRaw = JSON.parse(
333350
await fs.readFile(path.join(subAgentDir, "auth-profiles.json"), "utf8"),
334351
) as AuthProfileStore;
335-
const subCredential = requireOAuthCredential(subRaw, profileId);
336-
expect(subCredential.access).toBe("local-stale-access");
337-
expect(subCredential.refresh).toBe("local-stale-refresh");
352+
expectPersistedOpenAICodexProfileWithoutInlineTokens(subRaw.profiles[profileId], {
353+
expires: now - 120_000,
354+
accountId,
355+
});
356+
expect(JSON.stringify(subRaw)).not.toContain("local-stale-access");
357+
expect(JSON.stringify(subRaw)).not.toContain("local-stale-refresh");
338358

339359
const mainRaw = JSON.parse(
340360
await fs.readFile(path.join(mainAgentDir, "auth-profiles.json"), "utf8"),
341361
) as AuthProfileStore;
342-
const mainCredential = requireOAuthCredential(mainRaw, profileId);
343-
expect(mainCredential.access).toBe("main-owner-refreshed-access");
344-
expect(mainCredential.refresh).toBe("main-owner-refreshed-refresh");
345-
expect(mainCredential.expires).toBe(freshExpiry);
362+
expectPersistedOpenAICodexProfileWithoutInlineTokens(mainRaw.profiles[profileId], {
363+
expires: freshExpiry,
364+
accountId,
365+
});
366+
expect(JSON.stringify(mainRaw)).not.toContain("main-owner-refreshed-access");
367+
expect(JSON.stringify(mainRaw)).not.toContain("main-owner-refreshed-refresh");
346368
});
347369

348370
it("inherits main-agent credentials via the catch-block fallback when refresh throws after main becomes fresh", async () => {
@@ -410,7 +432,10 @@ describe("resolveApiKeyForProfile OAuth refresh mirror-to-main (#26322)", () =>
410432
const subRaw = JSON.parse(
411433
await fs.readFile(path.join(subAgentDir, "auth-profiles.json"), "utf8"),
412434
) as AuthProfileStore;
413-
expect(requireOAuthCredential(subRaw, profileId).access).toBe("cached-access-token");
435+
expectPersistedOpenAICodexProfileWithoutInlineTokens(subRaw.profiles[profileId], {
436+
accountId: "acct-shared",
437+
});
438+
expect(JSON.stringify(subRaw)).not.toContain("cached-access-token");
414439
});
415440

416441
it("mirrors refreshed credentials produced by the plugin-refresh path", async () => {

0 commit comments

Comments
 (0)