Skip to content

Commit 3c09c9e

Browse files
committed
fix: redact persisted selection callback failures
1 parent bc2b7ac commit 3c09c9e

2 files changed

Lines changed: 54 additions & 10 deletions

File tree

lib/auth/login-runner.ts

Lines changed: 17 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,9 @@ export type AccountSelectionFallbacks = Pick<
3434
"accountIdOverride" | "accountIdSource" | "organizationIdOverride" | "accountLabel"
3535
>;
3636

37+
const PERSIST_AUTHENTICATED_SELECTIONS_ERROR =
38+
"Failed to persist authenticated account selections.";
39+
3740
const createSelectionVariant = (
3841
tokens: TokenSuccess,
3942
candidate: {
@@ -163,8 +166,8 @@ export function applyAccountSelectionFallbacks(
163166
* that callback, so callers should use `persistAccountPool` or
164167
* `withAccountStorageTransaction` to keep the rename retry and serialized
165168
* read-modify-write behavior covered by `test/login-runner.test.ts`.
166-
* This helper does not log token material; callers must redact any callback
167-
* failure details before emitting logs.
169+
* Callback failures are rethrown with a redacted message so callers can log the
170+
* wrapper safely without leaking token-file paths or account identifiers.
168171
*/
169172
export async function persistResolvedAccountSelection(
170173
selection: AccountSelectionResult,
@@ -177,10 +180,16 @@ export async function persistResolvedAccountSelection(
177180
return selection;
178181
}
179182

180-
await options.persistSelections(
181-
selection.variantsForPersistence,
182-
options.replaceAll ?? false,
183-
);
183+
try {
184+
await options.persistSelections(
185+
selection.variantsForPersistence,
186+
options.replaceAll ?? false,
187+
);
188+
} catch (error) {
189+
throw new Error(PERSIST_AUTHENTICATED_SELECTIONS_ERROR, {
190+
cause: error,
191+
});
192+
}
184193
return selection;
185194
}
186195

@@ -190,8 +199,8 @@ export async function persistResolvedAccountSelection(
190199
* serialization remain the callback's responsibility, so callers should route
191200
* through `persistAccountPool` or `withAccountStorageTransaction` to preserve
192201
* the guarantees covered by `test/login-runner.test.ts`.
193-
* This helper does not log tokens or account identifiers; callers must redact
194-
* callback failures before logging them.
202+
* Persistence callback failures are redacted inside
203+
* `persistResolvedAccountSelection()` before they propagate back to callers.
195204
*/
196205
export async function resolveAndPersistAccountSelection(
197206
tokens: TokenSuccess,

test/login-runner.test.ts

Lines changed: 37 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -206,9 +206,44 @@ describe("login-runner selection finalization", () => {
206206
throw persistError;
207207
});
208208

209+
const result = persistResolvedAccountSelection(selection, { persistSelections });
209210
await expect(
210-
persistResolvedAccountSelection(selection, { persistSelections }),
211-
).rejects.toThrow("persist failed");
211+
result,
212+
).rejects.toThrow("Failed to persist authenticated account selections.");
213+
await expect(
214+
result,
215+
).rejects.not.toThrow("persist failed");
216+
const wrapped = await result.catch((error) => error as Error & { cause?: unknown });
217+
218+
expect(wrapped.cause).toBe(persistError);
219+
expect(wrapped.message).not.toContain("persist failed");
220+
expect(
221+
wrapped.message,
222+
).toBe("Failed to persist authenticated account selections.");
223+
expect(persistSelections).toHaveBeenCalledTimes(1);
224+
});
225+
226+
it("redacts sensitive persistence callback failure details", async () => {
227+
const selection = resolveAccountSelection({
228+
type: "success",
229+
access: "access-token",
230+
refresh: "refresh-token",
231+
expires: Date.now() + 60_000,
232+
idToken: "id-token",
233+
});
234+
const persistSelections = vi.fn(async () => {
235+
throw new Error(
236+
"EPERM: rename C:\\Users\\neil\\.opencode\\secrets\\token-file.json for acct-123",
237+
);
238+
});
239+
240+
const wrapped = await persistResolvedAccountSelection(selection, {
241+
persistSelections,
242+
}).catch((error) => error as Error & { cause?: unknown });
243+
244+
expect(wrapped.message).toBe("Failed to persist authenticated account selections.");
245+
expect(wrapped.message).not.toContain("token-file");
246+
expect(wrapped.message).not.toContain("acct-123");
212247
expect(persistSelections).toHaveBeenCalledTimes(1);
213248
});
214249
});

0 commit comments

Comments
 (0)