fix(coding-agent): handle compromised auth lock without crashing

closes #1322
This commit is contained in:
Mario Zechner 2026-02-06 19:01:42 +01:00
parent 2f1ab3641f
commit ddd5a65c7e
2 changed files with 70 additions and 1 deletions

View file

@ -196,6 +196,13 @@ export class AuthStorage {
}
let release: (() => Promise<void>) | undefined;
let lockCompromised = false;
let lockCompromisedError: Error | undefined;
const throwIfLockCompromised = () => {
if (lockCompromised) {
throw lockCompromisedError ?? new Error("OAuth refresh lock was compromised");
}
};
try {
// Acquire exclusive lock with retry and timeout
@ -209,8 +216,14 @@ export class AuthStorage {
randomize: true,
},
stale: 30000, // Consider lock stale after 30 seconds
onCompromised: (err) => {
lockCompromised = true;
lockCompromisedError = err;
},
});
throwIfLockCompromised();
// Re-read file after acquiring lock - another instance may have refreshed
this.reload();
@ -223,6 +236,7 @@ export class AuthStorage {
// (another instance may have already refreshed it)
if (Date.now() < cred.expires) {
// Token is now valid - another instance refreshed it
throwIfLockCompromised();
const apiKey = provider.getApiKey(cred);
return { apiKey, newCredentials: cred };
}
@ -237,11 +251,14 @@ export class AuthStorage {
const result = await getOAuthApiKey(providerId, oauthCreds);
if (result) {
throwIfLockCompromised();
this.data[providerId] = { type: "oauth", ...result.newCredentials };
this.save();
throwIfLockCompromised();
return result;
}
throwIfLockCompromised();
return null;
} finally {
// Always release the lock

View file

@ -1,7 +1,9 @@
import { existsSync, mkdirSync, readFileSync, rmSync, writeFileSync } from "node:fs";
import { tmpdir } from "node:os";
import { join } from "node:path";
import { afterEach, beforeEach, describe, expect, test } from "vitest";
import { registerOAuthProvider } from "@mariozechner/pi-ai";
import lockfile from "proper-lockfile";
import { afterEach, beforeEach, describe, expect, test, vi } from "vitest";
import { AuthStorage } from "../src/core/auth-storage.js";
import { clearConfigValueCache } from "../src/core/resolve-config-value.js";
@ -21,6 +23,7 @@ describe("AuthStorage", () => {
rmSync(tempDir, { recursive: true });
}
clearConfigValueCache();
vi.restoreAllMocks();
});
function writeAuthJson(data: Record<string, unknown>) {
@ -287,6 +290,55 @@ describe("AuthStorage", () => {
});
});
describe("oauth lock compromise handling", () => {
test("returns undefined on compromised lock and allows a later retry", async () => {
const providerId = `test-oauth-provider-${Date.now()}-${Math.random().toString(36).slice(2)}`;
registerOAuthProvider({
id: providerId,
name: "Test OAuth Provider",
async login() {
throw new Error("Not used in this test");
},
async refreshToken(credentials) {
return {
...credentials,
access: "refreshed-access-token",
expires: Date.now() + 60_000,
};
},
getApiKey(credentials) {
return `Bearer ${credentials.access}`;
},
});
writeAuthJson({
[providerId]: {
type: "oauth",
refresh: "refresh-token",
access: "expired-access-token",
expires: Date.now() - 10_000,
},
});
authStorage = new AuthStorage(authJsonPath);
const realLock = lockfile.lock.bind(lockfile);
const lockSpy = vi.spyOn(lockfile, "lock");
lockSpy.mockImplementationOnce(async (file, options) => {
options?.onCompromised?.(new Error("Unable to update lock within the stale threshold"));
return realLock(file, options);
});
const firstTry = await authStorage.getApiKey(providerId);
expect(firstTry).toBeUndefined();
lockSpy.mockRestore();
const secondTry = await authStorage.getApiKey(providerId);
expect(secondTry).toBe("Bearer refreshed-access-token");
});
});
describe("runtime overrides", () => {
test("runtime override takes priority over auth.json", async () => {
writeAuthJson({