mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-10-23 17:33:51 +02:00
fix: serialize managed credential refresh with reads and secret edits
Co-Authored-By: Paperclip <noreply@paperclip.ing>
This commit is contained in:
1 parent
feba1f926f
commit
7cf8df553d
7 files changed
+109
-17
No files matched your search
@@ -463,13 +463,15 @@ describe("codex auth cache store", () => {
|
||||
releaseFirstCaller = resolve;
|
||||
});
|
||||
|
||||
const entered = deferredSignal();
|
||||
const firstCall = withAccountHomeSecretMutationLock(env, "company-shared", async () => {
|
||||
events.push("first-enter");
|
||||
entered.resolve();
|
||||
await firstCallerGate;
|
||||
events.push("first-exit");
|
||||
return "first";
|
||||
});
|
||||
await new Promise((resolve) => setTimeout(resolve, 20));
|
||||
await entered.promise;
|
||||
const secondCall = withAccountHomeSecretMutationLock(env, "company-shared", async () => {
|
||||
events.push("second-enter");
|
||||
events.push("second-exit");
|
||||
@@ -497,12 +499,14 @@ describe("codex auth cache store", () => {
|
||||
releaseCompanyA = resolve;
|
||||
});
|
||||
|
||||
const entered = deferredSignal();
|
||||
const companyACall = withAccountHomeSecretMutationLock(env, "company-a", async () => {
|
||||
events.push("a-enter");
|
||||
entered.resolve();
|
||||
await companyAGate;
|
||||
events.push("a-exit");
|
||||
});
|
||||
await new Promise((resolve) => setTimeout(resolve, 20));
|
||||
await entered.promise;
|
||||
const companyBCall = withAccountHomeSecretMutationLock(env, "company-b", async () => {
|
||||
events.push("b-enter");
|
||||
events.push("b-exit");
|
||||
@@ -536,6 +540,28 @@ describe("codex auth cache store", () => {
|
||||
|
||||
expect(events).toEqual(["promotion-enter", "mutation-enter", "mutation-exit", "promotion-exit"]);
|
||||
});
|
||||
it("allows awaited nested writes but expires inherited ownership after release", async () => {
|
||||
const env = envFor(await makeInstanceRoot());
|
||||
const detachedGate = deferredSignal();
|
||||
let detached!: Promise<void>;
|
||||
let enteredDetached = false;
|
||||
await withAccountHomeSecretMutationLock(env, "nested", async () => {
|
||||
await expect(withAccountHomeSecretMutationLock(env, "nested", async () => "nested write")).resolves.toBe("nested write");
|
||||
detached = detachedGate.promise.then(() => withAccountHomeSecretMutationLock(env, "nested", async () => { enteredDetached = true; }));
|
||||
});
|
||||
const holding = deferredSignal();
|
||||
const release = deferredSignal();
|
||||
const other = withAccountHomeSecretMutationLock(env, "nested", async () => { holding.resolve(); await release.promise; });
|
||||
await holding.promise;
|
||||
detachedGate.resolve();
|
||||
try {
|
||||
await new Promise(resolve => setTimeout(resolve, 30));
|
||||
expect(enteredDetached).toBe(false);
|
||||
} finally { release.resolve(); }
|
||||
await Promise.all([other, detached]);
|
||||
expect(enteredDetached).toBe(true);
|
||||
});
|
||||
|
||||
});
|
||||
|
||||
describe("Phase 8: account-home directory validity before a secret write commits", () => {
|
||||
@@ -581,3 +607,9 @@ describe("codex auth cache store", () => {
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
function deferredSignal() {
|
||||
let resolve!: () => void;
|
||||
const promise = new Promise<void>(done => { resolve = done; });
|
||||
return { promise, resolve };
|
||||
}
|
||||
@@ -1,4 +1,5 @@
|
||||
import { lstat, mkdir, open, readFile, rename, rm } from "node:fs/promises";
|
||||
import { AsyncLocalStorage } from "node:async_hooks";
|
||||
import { lstat, mkdir, open, readFile, realpath, rename, rm } from "node:fs/promises";
|
||||
import path from "node:path";
|
||||
import { randomUUID } from "node:crypto";
|
||||
import { resolvePaperclipInstanceRootForAdapter } from "@paperclipai/adapter-utils/server-utils";
|
||||
@@ -334,6 +335,11 @@ export async function withCodexAccountHomePromotionLock<T>(
|
||||
* create) can still call into a secrets-service write that takes this lock
|
||||
* without deadlocking on itself.
|
||||
*/
|
||||
// An outer credential transaction acquires this lock before database row locks.
|
||||
// Nested secret-service writes must share that ownership, not acquire it again.
|
||||
// Expire ownership on return so detached work cannot inherit a released lock.
|
||||
const secretMutationOwnership = new AsyncLocalStorage<Map<string, { active: boolean }>>();
|
||||
|
||||
export async function withAccountHomeSecretMutationLock<T>(
|
||||
env: NodeJS.ProcessEnv = process.env,
|
||||
companyId: string,
|
||||
@@ -341,7 +347,16 @@ export async function withAccountHomeSecretMutationLock<T>(
|
||||
): Promise<T> {
|
||||
const lockDir = resolveCodexAuthCacheNamedLockDir(env, companyId, "codex-account-home-secret-mutation-lock");
|
||||
await ensurePrivateDir(lockDir);
|
||||
return withDirectoryMergeLock(lockDir, fn, env);
|
||||
const key = await realpath(lockDir);
|
||||
const inherited = secretMutationOwnership.getStore();
|
||||
if (inherited?.get(key)?.active) return fn();
|
||||
return withDirectoryMergeLock(lockDir, async () => {
|
||||
const ownership = { active: true };
|
||||
const scope = new Map(inherited);
|
||||
scope.set(key, ownership);
|
||||
try { return await secretMutationOwnership.run(scope, fn); }
|
||||
finally { ownership.active = false; }
|
||||
}, env);
|
||||
}
|
||||
|
||||
/**
|
||||
|
||||
@@ -84,6 +84,37 @@ describe("managed AI connections", () => {
|
||||
} finally { fetchSpy.mockRestore(); }
|
||||
});
|
||||
|
||||
it("serializes quota exchange with a concurrent secret edit and runtime credential read", async () => {
|
||||
const owner = "quota-race-owner";
|
||||
await db.insert(companyMemberships).values({ companyId, principalId: owner, principalType: "user", status: "active", membershipRole: "member" });
|
||||
const original = JSON.stringify({ tokens: { access_token: "old-access", refresh_token: "single-use", id_token: "identity", account_id: "race-account" } });
|
||||
const account = await service.save(companyId, owner, { provider: "openai", method: "subscription", ownership: "personal", name: "Racing refresh", loginSessionId: "fixture", agentIds: [], allAgents: true }, original);
|
||||
const [row] = await service.quotaAccounts(companyId, owner);
|
||||
const secretId = row.grant.credentialSecretRefs.find(ref => ref.configPath === "ai.credential")!.secretId;
|
||||
const exchangeStarted = deferredSignal();
|
||||
const releaseExchange = deferredSignal();
|
||||
const request = vi.spyOn(globalThis, "fetch").mockImplementation(async () => {
|
||||
exchangeStarted.resolve(); await releaseExchange.promise;
|
||||
return Response.json({ access_token: "new-access", refresh_token: "new-refresh" });
|
||||
});
|
||||
let runtime: Awaited<ReturnType<typeof prepareManagedAiRuntime>> | undefined;
|
||||
const refreshing = service.refreshQuotaCredential(row, original, new AbortController().signal);
|
||||
try {
|
||||
await exchangeStarted.promise;
|
||||
const edit = secretService(db).update(secretId, { description: "Changed during refresh" });
|
||||
const preparing = prepareManagedAiRuntime(db, { companyId, agentId, responsibleUserId: owner, adapterType: "codex_local", binding: { provider: "openai", method: "subscription", mode: "responsible_user" }, config: {} });
|
||||
// Keep the provider exchange pending while both contenders reach their
|
||||
// lock/read paths. Neither may hold a conflicting row lock or vend old auth.
|
||||
await new Promise(resolve => setTimeout(resolve, 50));
|
||||
releaseExchange.resolve();
|
||||
const results = await Promise.all([refreshing, edit, preparing]);
|
||||
runtime = results[2];
|
||||
expect(JSON.parse(await readFile(path.join(String(runtime.config.env.CODEX_HOME), "auth.json"), "utf8"))).toMatchObject({ tokens: { access_token: "new-access", refresh_token: "new-refresh" } });
|
||||
expect(JSON.parse(await service.credential(row))).toMatchObject({ tokens: { access_token: "new-access", refresh_token: "new-refresh" } });
|
||||
expect((await secretService(db).getById(secretId))?.description).toBe("Changed during refresh");
|
||||
} finally { releaseExchange.resolve(); await runtime?.cleanup(); request.mockRestore(); }
|
||||
}, 10_000);
|
||||
|
||||
it("serializes quota credential refresh, persists rotation, and defers while the provider is active", async () => {
|
||||
const owner = "quota-refresh-owner";
|
||||
await db.insert(companyMemberships).values({ companyId, principalId: owner, principalType: "user", status: "active", membershipRole: "member" });
|
||||
@@ -1069,3 +1100,9 @@ describe("AI connection recovery delivery", () => {
|
||||
}, 30000,
|
||||
);
|
||||
});
|
||||
|
||||
function deferredSignal() {
|
||||
let resolve!: () => void;
|
||||
const promise = new Promise<void>(done => { resolve = done; });
|
||||
return { promise, resolve };
|
||||
}
|
||||
@@ -11,7 +11,7 @@ import {
|
||||
} from "@paperclipai/shared";
|
||||
import { aiConnectionService } from "./ai-connections.js";
|
||||
import { secretService } from "./secrets.js";
|
||||
import { decideCodexAuthMerge } from "@paperclipai/adapter-codex-local/server";
|
||||
import { decideCodexAuthMerge, withAccountHomeSecretMutationLock } from "@paperclipai/adapter-codex-local/server";
|
||||
import type { AdapterExecutionTarget } from "@paperclipai/adapter-utils/execution-target";
|
||||
import { runAdapterExecutionTargetProcess } from "@paperclipai/adapter-utils/execution-target";
|
||||
import { decideGrokAuthMerge } from "@paperclipai/adapter-grok-local/server";
|
||||
@@ -314,7 +314,7 @@ export async function prepareManagedAiRuntime(
|
||||
if (subscriptionFile) {
|
||||
const refreshed = await readFile(authFile, "utf8");
|
||||
if (refreshed !== value)
|
||||
await db.transaction(async (tx) => {
|
||||
await withAccountHomeSecretMutationLock(undefined, input.companyId, () => db.transaction(async (tx) => {
|
||||
const [grant] = await tx
|
||||
.select()
|
||||
.from(connectionGrants)
|
||||
@@ -374,7 +374,7 @@ export async function prepareManagedAiRuntime(
|
||||
.update(connectionGrants)
|
||||
.set({ updatedAt: new Date() })
|
||||
.where(eq(connectionGrants.id, grant.id));
|
||||
});
|
||||
}));
|
||||
}
|
||||
} finally {
|
||||
if (home) await rm(home, { recursive: true, force: true });
|
||||
|
||||
@@ -1,3 +1,4 @@
|
||||
import { withAccountHomeSecretMutationLock } from "@paperclipai/adapter-codex-local/server";
|
||||
import { syncConnectionCredentialBindings } from "./connection-credential-bindings.js";
|
||||
import { createHash, randomUUID } from "node:crypto";
|
||||
import { and, eq, inArray, or, sql } from "drizzle-orm";
|
||||
@@ -402,6 +403,9 @@ export function aiConnectionService(db: Db) {
|
||||
};
|
||||
}
|
||||
async function credential(row: Pick<Awaited<ReturnType<typeof select>>, "connection" | "grant">) {
|
||||
return withAccountHomeSecretMutationLock(undefined, row.connection.companyId, () => credentialUnlocked(row));
|
||||
}
|
||||
async function credentialUnlocked(row: Pick<Awaited<ReturnType<typeof select>>, "connection" | "grant">) {
|
||||
const ref = row.grant.credentialSecretRefs.find(
|
||||
(r) => r.configPath === "ai.credential",
|
||||
);
|
||||
@@ -498,7 +502,7 @@ export function aiConnectionService(db: Db) {
|
||||
);
|
||||
const id = reconnect?.connection.id ?? randomUUID();
|
||||
const grantId = reconnect?.grant.id ?? randomUUID();
|
||||
return db.transaction(async (tx) => {
|
||||
return withAccountHomeSecretMutationLock(undefined, companyId, () => db.transaction(async (tx) => {
|
||||
const secrets = secretService(tx);
|
||||
if (sessionId) {
|
||||
const [session] = await tx
|
||||
@@ -803,7 +807,7 @@ export function aiConnectionService(db: Db) {
|
||||
details: { provider: input.provider, method: input.method, grantId },
|
||||
});
|
||||
return { connectionId: id, grantId };
|
||||
});
|
||||
}));
|
||||
}
|
||||
/** A late failure must never invalidate credentials that were refreshed or reconnected meanwhile. */
|
||||
async function markAuthenticationFailed(input: {
|
||||
@@ -813,7 +817,7 @@ export function aiConnectionService(db: Db) {
|
||||
runStartedAt: Date;
|
||||
attribution: AiConnectionAttribution & { identity: string };
|
||||
}) {
|
||||
return db.transaction(async (tx) => {
|
||||
return withAccountHomeSecretMutationLock(undefined, input.companyId, () => db.transaction(async (tx) => {
|
||||
const { attribution } = input;
|
||||
const [grant] = await tx.select().from(connectionGrants).where(and(
|
||||
eq(connectionGrants.companyId, input.companyId),
|
||||
@@ -847,11 +851,11 @@ export function aiConnectionService(db: Db) {
|
||||
entityType: "tool_connection", entityId: connection.id,
|
||||
details: { provider: attribution.provider, grantId: grant.id },
|
||||
});
|
||||
});
|
||||
}));
|
||||
}
|
||||
/** Refresh only the selected vaulted identity, serializing rotation and reconnect. */
|
||||
async function refreshQuotaCredential(row: Parameters<typeof credential>[0], failedValue: string, signal: AbortSignal) {
|
||||
return db.transaction(async (tx) => {
|
||||
return withAccountHomeSecretMutationLock(undefined, row.connection.companyId, () => db.transaction(async (tx) => {
|
||||
const companyId = row.connection.companyId;
|
||||
const [grant] = await tx.select().from(connectionGrants).where(and(
|
||||
eq(connectionGrants.id, row.grant.id), eq(connectionGrants.companyId, companyId),
|
||||
@@ -904,7 +908,7 @@ export function aiConnectionService(db: Db) {
|
||||
action: "ai_connection.credential_refreshed", entityType: "tool_connection", entityId: row.connection.id,
|
||||
details: { provider: "openai", grantId: grant.id } });
|
||||
return refreshed;
|
||||
});
|
||||
}));
|
||||
}
|
||||
// A quota read uses the same credential audience as execution. Operator status
|
||||
// alone never grants access to another member's personal subscription.
|
||||
|
||||
@@ -15,9 +15,10 @@ describe("account quota panels", () => {
|
||||
});
|
||||
it("displays unknown utilization without inventing zero percent", () => {
|
||||
const html = renderToStaticMarkup(<AccountQuotaPanels accounts={[
|
||||
{ provider: "openai", accountKey: "a", ok: true, windows: [{ label: "Credits", usedPercent: null, resetsAt: null, valueLabel: "$5 remaining" }] },
|
||||
{ provider: "openai", accountKey: "a", ok: true, windows: [{ label: "Credits", usedPercent: null, resetsAt: "2026-10-05T00:00:00Z", valueLabel: "$5 remaining" }] },
|
||||
]} />);
|
||||
expect(html).toContain("$5 remaining");
|
||||
expect(html).toContain(`Resets ${new Date("2026-10-05T00:00:00Z").toLocaleString()}`);
|
||||
expect(html).not.toContain("0%");
|
||||
expect(html).not.toContain('role="progressbar"');
|
||||
});
|
||||
|
||||
@@ -27,9 +27,12 @@ export function AccountQuotaPanels({
|
||||
</p>
|
||||
)}
|
||||
{account.windows.map((window, index) => window.usedPercent == null ? (
|
||||
<div key={`${window.label}:${index}`} className="flex justify-between gap-3 text-sm">
|
||||
<span>{window.label}</span>
|
||||
<span className="text-muted-foreground">{window.valueLabel ?? "Usage not reported"}</span>
|
||||
<div key={`${window.label}:${index}`} className="space-y-1 text-sm">
|
||||
<div className="flex justify-between gap-3">
|
||||
<span>{window.label}</span>
|
||||
<span className="text-muted-foreground">{window.valueLabel ?? "Usage not reported"}</span>
|
||||
</div>
|
||||
{window.resetsAt && <p className="text-xs text-muted-foreground">Resets {new Date(window.resetsAt).toLocaleString()}</p>}
|
||||
</div>
|
||||
) : (
|
||||
<QuotaBar
|
||||
|
||||
Reference in new issue
Block a user