mirror of
https://github.com/getcompanion-ai/co-mono.git
synced 2026-04-15 21:03:19 +00:00
fix(coding-agent): handle compromised auth lock without crashing
closes #1322
This commit is contained in:
parent
dc22b0efd4
commit
98efcb30a9
2 changed files with 70 additions and 1 deletions
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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({
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue