From e0e330b091a63e4183fecb79608a4b660f040b72 Mon Sep 17 00:00:00 2001 From: Aman Varshney Date: Fri, 14 Aug 2026 18:47:08 +0530 Subject: [PATCH 1/4] fix(auth): refresh credentials before delegated runs Signed-off-by: Aman Varshney --- .../engine/credential-manager-design.md | 11 ++ .../assets/engine/engine-interface-draft.ts | 14 +- .drive/projects/prisma-cli-v8/deferred.md | 18 ++- .drive/projects/prisma-cli-v8/plan.md | 2 +- .../cli-engine/src/active-access-token.ts | 83 ++++++++++++ packages/cli-engine/src/commands.ts | 7 +- packages/cli-engine/src/credential-manager.ts | 18 ++- .../src/environment-credential-manager.ts | 13 +- packages/cli-engine/src/execution/needs.ts | 58 +++++---- packages/cli-engine/src/exports/index.ts | 3 + .../src/in-memory-credential-manager.ts | 15 ++- packages/cli-engine/src/management-api.ts | 23 ++++ packages/cli-engine/src/testing.ts | 4 + packages/cli-engine/tests/spawn.test.ts | 122 ++++++++++++++++++ packages/cli/src/auth/credential-manager.ts | 84 +++++++++++- packages/cli/src/auth/refresh.ts | 62 +++++++++ packages/cli/src/runtime.ts | 5 +- packages/cli/tests/auth-refresh.test.ts | 93 +++++++++++++ packages/cli/tests/credential-manager.test.ts | 96 +++++++++++++- 19 files changed, 672 insertions(+), 59 deletions(-) create mode 100644 packages/cli-engine/src/active-access-token.ts create mode 100644 packages/cli/src/auth/refresh.ts create mode 100644 packages/cli/tests/auth-refresh.test.ts diff --git a/.drive/projects/prisma-cli-v8/assets/engine/credential-manager-design.md b/.drive/projects/prisma-cli-v8/assets/engine/credential-manager-design.md index 87c69a83..4f5af6fe 100644 --- a/.drive/projects/prisma-cli-v8/assets/engine/credential-manager-design.md +++ b/.drive/projects/prisma-cli-v8/assets/engine/credential-manager-design.md @@ -791,6 +791,17 @@ that same `ctx.api` through its `deps.client` seam, not by composing another client from env). For an environment-only manager the operation is a pass-through of the env token — no storage involved. +S3 amendment (2026-08-14): the child still receives only an access-token +snapshot, but a stored OAuth session is no longer rejected merely because +that snapshot is inside the five-minute window. Before the handler runs, the +engine asks `activeAccessToken(options)` to refresh the pair under the +manager's storage lock, persist the rotation, and return the new access token. +The shipped manager receives a host-side token-endpoint adapter at construction; +the manager remains the sole owner of storage reads and writes, and the +refresh token is never added to child env. Calling `activeAccessToken()` with +no options remains the fresh spawn-time read, so rotation by another process +between preflight and spawn is still observed. + ### 11.6 whoami `whoami` asks for the active credential's identity and renders it. It diff --git a/.drive/projects/prisma-cli-v8/assets/engine/engine-interface-draft.ts b/.drive/projects/prisma-cli-v8/assets/engine/engine-interface-draft.ts index 00bb1971..3b240716 100644 --- a/.drive/projects/prisma-cli-v8/assets/engine/engine-interface-draft.ts +++ b/.drive/projects/prisma-cli-v8/assets/engine/engine-interface-draft.ts @@ -585,6 +585,12 @@ export interface Session { readonly current: boolean } +export interface ActiveAccessTokenOptions { + readonly minimumValidityMs: number + readonly now: Date + readonly signal: AbortSignal +} + /** Manages sessions: six user-facing operations plus one * engine-facing accessor. Custody, not user interaction: never opens * a browser, never prompts. Env is a construction input. The manager @@ -628,10 +634,10 @@ export interface CredentialManager { * credential's ACCESS token, read fresh, for handing to a child * process that authenticates as this process does. Never the * refresh token — the child gets a snapshot it cannot refresh. - * Single consumer: ctx.spawn's credential injection. The read - * builds no second API client, so the one-client-per-process - * invariant holds (credential-manager-design.md §11.5). */ - activeAccessToken(): Promise + * With options, refreshes a near-expiry stored OAuth pair before + * returning its access token. With no options, this is ctx.spawn's + * fresh read. The refresh token never reaches the child. */ + activeAccessToken(options?: ActiveAccessTokenOptions): Promise } /** The SDK's typed client and token-storage contract, re-exported by diff --git a/.drive/projects/prisma-cli-v8/deferred.md b/.drive/projects/prisma-cli-v8/deferred.md index b4a922a5..778c7c28 100644 --- a/.drive/projects/prisma-cli-v8/deferred.md +++ b/.drive/projects/prisma-cli-v8/deferred.md @@ -147,17 +147,15 @@ CLI does not do, and each restarts as engine work if wanted: consumer (`packages/cli-engine/src/execution/spawn.ts`) moves with it. Recorded in `assets/engine/credential-manager-design.md`. - **Nothing bounds a child run to the token it was given.** A - `credentials: "child"` command hands the child a snapshot of the + `credentials: "child"` command still hands the child a snapshot of the access token and never the refresh token - (`packages/cli-engine/src/execution/spawn.ts`), and the only check is - the near-expiry refusal in `execution/needs.ts`: - `CREDENTIAL_NEAR_EXPIRY_MS` is 5 minutes, so the guarantee at spawn is - "more than five minutes left", not "enough for this run". A converge - that outlives the snapshot fails on an expired token, after the child - has already created resources. Two ways out, both unbuilt: hand the - child something that can refresh, or bound the child's run and refuse - when the remaining lifetime cannot cover it. Recorded as a release - limitation in `plan.md`'s coverage ledger. + (`packages/cli-engine/src/execution/spawn.ts`). The parent now refreshes a + stored OAuth pair before the handler when its access token is inside + `CREDENTIAL_NEAR_EXPIRY_MS`, so a refreshable session receives a fresh + snapshot instead of an unnecessary sign-in error. That does not bound the + child's total runtime: a converge that outlives even the refreshed snapshot + can still fail after creating resources. The remaining ways out are to hand + the child something that can refresh or to bound the child's run. - **A validated number flag**, if `--tail`'s old constraint is wanted back. `flag.number` accepts negatives and fractions, so "non-negative integer" is enforced nowhere. D4 took the other branch this item diff --git a/.drive/projects/prisma-cli-v8/plan.md b/.drive/projects/prisma-cli-v8/plan.md index fd3b2398..4621c674 100644 --- a/.drive/projects/prisma-cli-v8/plan.md +++ b/.drive/projects/prisma-cli-v8/plan.md @@ -203,7 +203,7 @@ Recorded so they are not lost between slices. | Prompts (defaults, consent, wizard) | S2 (init) | | Poll + status events; output streams | S2 (domain wait; `build logs` — `service logs` moved to S8) | | Auth via context | S2, S3 (deploy, destroy) | -| Refresh under long runs | **Still unproven** (corrected at S3 closure). S3 proves the STATIC-token handoff instead: the child is given a snapshot that never refreshes, and the refresh token is never injected, so a long converge runs on a token that can expire mid-run. The contract accepts that and refuses up front when the session is near expiry. **The bound that refusal buys is five minutes** (`CREDENTIAL_NEAR_EXPIRY_MS` in `execution/needs.ts`): a run starts only when more than five minutes remain, and nothing limits how long the child then runs, so a converge outliving the snapshot fails on an expired token after it has created resources. That is a release limitation, recorded in `deferred.md`, not a solved problem. The in-process leg uses the engine's refreshing client, but nothing in S3 runs long enough to make it refresh. | +| Refresh under long runs | **Still unproven.** The child receives an access-token snapshot and never the refresh token. As of the 2026-08-14 amendment, the parent proactively rotates a refreshable stored OAuth pair before the handler when the access token is inside `CREDENTIAL_NEAR_EXPIRY_MS`; this avoids rejecting a healthy login and gives the child a fresh snapshot. Nothing bounds the child runtime, so a converge can still outlive that refreshed snapshot and fail after it has created resources. That remaining limitation is recorded in `deferred.md`. | | Config sections, command families, validator absence | Two levels, and they are proven in different places. The section machinery — a total validator including absence, a validator's warning diagnostic, and the engine's unknown-section check — is proven by the engine's own suite (`packages/cli-engine/tests/config.test.ts`) against toy sections. What S3 adds is ONE real section end to end: composer's, a single optional string field, declared by one family, read from disk by the bin's real loader, accepted by composer's own validator, and arriving at composer's handler as the path it acts on (`v8-bin.test.ts`, "hands the composer section of prisma.config.ts to the composer family"). Only the accepting path is covered there: nothing in the bin shows composer's validator refusing a section, running on an absent one, or warning on an unknown key, because `log` against that one fixture is the only run a shipped composer command makes to config without credentials. The platform family declares no section, so two families contributing to one config file is unproven, and so is any section with required or structured fields. Both wait for S5. | | Session commands, signal lifetime | S3 (dev, log) | | Cross-repo/published consumption, pins, tandem releases | S3 | diff --git a/packages/cli-engine/src/active-access-token.ts b/packages/cli-engine/src/active-access-token.ts new file mode 100644 index 00000000..21e96d8c --- /dev/null +++ b/packages/cli-engine/src/active-access-token.ts @@ -0,0 +1,83 @@ +import { + authServiceError, + credentialsRequiredError, +} from "./credential-errors"; +import type { ActiveAccessTokenOptions } from "./credential-manager"; +import type { CredentialRefresher, TokenStorage } from "./management-api"; +import { CliStructuredError } from "./protocol"; +import { claimedExpiresAt } from "./token-claims"; + +type Tokens = NonNullable>>; + +/** Shared implementation used by every CredentialManager. */ +export async function readActiveAccessToken( + storage: TokenStorage, + refreshCredential: CredentialRefresher | undefined, + options?: ActiveAccessTokenOptions, +): Promise { + if (options === undefined) { + return (await storage.getTokens())?.accessToken ?? null; + } + const runLocked = storage.withRefreshLock ?? (async (fn) => fn()); + try { + return await runLocked(async () => { + // A waiter always re-reads inside the lock so a rotation that won the + // race is used without exchanging the old refresh token again. + const current = await storage.getTokens(); + if (current === null) return null; + if (!expiresSoon(current.accessToken, options)) { + return current.accessToken; + } + if (!current.refreshToken) { + throw credentialsRequiredError("expiring-soon"); + } + if (refreshCredential === undefined) { + throw new Error( + "@prisma/cli-engine: delegated OAuth refresh requires Runtime.refreshCredential", + ); + } + const refreshed = await refreshCredential({ + refreshToken: current.refreshToken, + signal: options.signal, + }); + if (refreshed.kind === "invalid") { + await clearCurrentTokens(storage, current); + throw credentialsRequiredError("expired"); + } + if (expiresSoon(refreshed.accessToken, options)) { + throw new Error("the OAuth endpoint returned a short-lived token"); + } + await storage.setTokens({ + workspaceId: current.workspaceId, + accessToken: refreshed.accessToken, + refreshToken: refreshed.refreshToken, + }); + return refreshed.accessToken; + }); + } catch (cause) { + if (CliStructuredError.is(cause) || options.signal.aborted) throw cause; + throw authServiceError(); + } +} + +function expiresSoon( + token: string, + options: ActiveAccessTokenOptions, +): boolean { + const expiresAt = claimedExpiresAt(token); + return ( + expiresAt !== undefined && + expiresAt.getTime() - options.now.getTime() <= options.minimumValidityMs + ); +} + +async function clearCurrentTokens( + storage: TokenStorage, + current: Tokens, +): Promise { + if (storage.clearTokensIfCurrent !== undefined) { + await storage.clearTokensIfCurrent(current); + return; + } + await storage.clearTokens(); +} diff --git a/packages/cli-engine/src/commands.ts b/packages/cli-engine/src/commands.ts index f656466f..22bcee66 100644 --- a/packages/cli-engine/src/commands.ts +++ b/packages/cli-engine/src/commands.ts @@ -65,9 +65,10 @@ export interface NeedsSpec { * Fail early with the sign-in error when unauthenticated. The * `"child"` form (S3) additionally makes the engine compose the * active credential into every child environment - * (PRISMA_SERVICE_TOKEN, PRISMA_WORKSPACE_ID) and refuse the run - * before the handler when that credential expires too soon to hand - * out — a child cannot refresh the snapshot it is given. It + * (PRISMA_SERVICE_TOKEN, PRISMA_WORKSPACE_ID). Before the handler it + * refreshes a stored OAuth session that expires too soon, or refuses + * an unrefreshable credential — a child cannot refresh the snapshot + * it is given. It * requires `maySpawn` (construction error otherwise) and entails * the plain credentials need. */ diff --git a/packages/cli-engine/src/credential-manager.ts b/packages/cli-engine/src/credential-manager.ts index 09951eec..a29b9f29 100644 --- a/packages/cli-engine/src/credential-manager.ts +++ b/packages/cli-engine/src/credential-manager.ts @@ -76,6 +76,13 @@ export interface ActiveCredential { readonly origin: CredentialOrigin; } +export interface ActiveAccessTokenOptions { + /** Refuse or refresh a token with no more than this lifetime left. */ + readonly minimumValidityMs: number; + readonly now: Date; + readonly signal: AbortSignal; +} + /** * Manages the credentials this machine holds: the stored per-workspace * sessions, which one is selected, and the credential this process @@ -142,11 +149,10 @@ export interface CredentialManager { /** * ENGINE-FACING. The active credential's ACCESS token, read fresh on * every call, for handing to a child process that authenticates as - * this process does. Never the refresh token: the child gets a - * snapshot it cannot refresh. Null when the material is gone (the - * session ended). Single consumer: ctx.spawn's credential injection - * (credential-manager-design.md §11.5) — the read builds no second - * API client, so the one-client-per-process invariant holds. + * this process does. With options, a near-expiry OAuth pair is refreshed + * under the storage lock before its access token is returned. Never the + * refresh token: the child gets a snapshot it cannot refresh. Null when + * the material is gone (the session ended). */ - activeAccessToken(): Promise; + activeAccessToken(options?: ActiveAccessTokenOptions): Promise; } diff --git a/packages/cli-engine/src/environment-credential-manager.ts b/packages/cli-engine/src/environment-credential-manager.ts index 7fc71fcb..9b3b79be 100644 --- a/packages/cli-engine/src/environment-credential-manager.ts +++ b/packages/cli-engine/src/environment-credential-manager.ts @@ -6,8 +6,10 @@ * refuses with a structured error. Env is a construction input; nothing * here reads process.env. */ +import { readActiveAccessToken } from "./active-access-token"; import { emptyServiceTokenError } from "./credential-errors"; import { + type ActiveAccessTokenOptions, type ActiveCredential, type Credential, type CredentialManager, @@ -100,8 +102,15 @@ export class EnvironmentCredentialManager implements CredentialManager { /** The spawn path's read: the env token passes through directly. It * is already a snapshot with no refresh token behind it. */ - async activeAccessToken(): Promise { - return this.#token() ?? null; + async activeAccessToken( + options?: ActiveAccessTokenOptions, + ): Promise { + if ((await this.activeCredential()) === null) return null; + return readActiveAccessToken( + await this.activeCredentialStorage(), + undefined, + options, + ); } #buildActiveStorage(): TokenStorage { diff --git a/packages/cli-engine/src/execution/needs.ts b/packages/cli-engine/src/execution/needs.ts index 22926e73..2e5b5fc4 100644 --- a/packages/cli-engine/src/execution/needs.ts +++ b/packages/cli-engine/src/execution/needs.ts @@ -18,10 +18,10 @@ import { withDocsUrl, writeDiagnostic } from "./rendering"; import { SEVERITY_RANK } from "./reporting"; /** - * D1 ruling (S3): a session expiring within this window is refused - * before the handler runs. The child receives a snapshot of the token - * and cannot refresh it, and the in-process work that precedes the - * spawn creates platform resources, so the refusal has to come first. + * A child receives an access-token snapshot it cannot refresh. Before + * the handler runs, a stored OAuth session inside this window is + * refreshed by the parent; a credential that cannot refresh is refused. + * The timing matters because pre-spawn work can create platform resources. */ export const CREDENTIAL_NEAR_EXPIRY_MS = 5 * 60_000; @@ -145,11 +145,11 @@ async function checkDependencies( * ctx.api raise identically. A host with no manager wired has no * credentials at all. * - * The `"child"` form resolves the same credential ONCE, additionally - * refuses a session about to expire — before the handler runs, not - * before the spawn: the work that precedes a spawn creates real - * platform resources, and the child cannot refresh the snapshot it is - * given — and carries the credential forward for the spawn path. + * The `"child"` form resolves the same credential ONCE and prepares an + * access-token snapshot before the handler runs. A refreshable stored + * OAuth session inside the near-expiry window is rotated and persisted; + * an unrefreshable credential is refused. This happens before the + * handler because work preceding a spawn may create platform resources. */ async function checkCredentials( needs: AnyCommand["needs"], @@ -180,25 +180,35 @@ async function checkCredentials( if (needs.credentials !== "child") { return {}; } - const expiry = nearExpiryFailure(credential, invocation); - return expiry === undefined - ? { spawnCredential: credential } - : { failure: expiry }; + try { + return { + spawnCredential: await prepareChildCredential(invocation), + }; + } catch (cause) { + if (CliStructuredError.is(cause)) { + return { failure: needsErrored(cause) }; + } + throw cause; + } } -function nearExpiryFailure( - credential: ActiveCredential, +async function prepareChildCredential( invocation: Invocation, -): NeedsOutcome | undefined { - if (credential.expiresAt === undefined) { - return undefined; - } - const remainingMs = - credential.expiresAt.getTime() - invocation.now().getTime(); - if (remainingMs > CREDENTIAL_NEAR_EXPIRY_MS) { - return undefined; +): Promise { + const manager = invocation.runtime.credentialManager; + if (manager === undefined) throw credentialsRequiredError(); + const accessToken = await manager.activeAccessToken({ + minimumValidityMs: CREDENTIAL_NEAR_EXPIRY_MS, + now: invocation.now(), + signal: invocation.signal, + }); + if (accessToken === null) throw credentialsRequiredError("session-ended"); + + const refreshedCredential = await manager.activeCredential(); + if (refreshedCredential === null) { + throw credentialsRequiredError("session-ended"); } - return needsErrored(credentialsRequiredError("expiring-soon")); + return refreshedCredential; } /** diff --git a/packages/cli-engine/src/exports/index.ts b/packages/cli-engine/src/exports/index.ts index cd42612e..fe78d648 100644 --- a/packages/cli-engine/src/exports/index.ts +++ b/packages/cli-engine/src/exports/index.ts @@ -68,6 +68,7 @@ export { noSessionForWorkspaceError, } from "../credential-errors"; export { + type ActiveAccessTokenOptions, type ActiveCredential, type Credential, type CredentialIdentity, @@ -85,6 +86,8 @@ export type { StreamMeta, } from "../events"; export type { + CredentialRefresher, + CredentialRefreshResult, ManagementApiClient, ManagementApiClientConfig, TokenStorage, diff --git a/packages/cli-engine/src/in-memory-credential-manager.ts b/packages/cli-engine/src/in-memory-credential-manager.ts index 0c183ea6..d7191477 100644 --- a/packages/cli-engine/src/in-memory-credential-manager.ts +++ b/packages/cli-engine/src/in-memory-credential-manager.ts @@ -14,19 +14,21 @@ * in the CLI's own tests. */ import { Buffer } from "node:buffer"; +import { readActiveAccessToken } from "./active-access-token"; import { credentialsRequiredError, credentialWorkspaceMismatchError, noSessionForWorkspaceError, } from "./credential-errors"; import type { + ActiveAccessTokenOptions, ActiveCredential, Credential, CredentialManager, Session, StoredSessions, } from "./credential-manager"; -import type { TokenStorage } from "./management-api"; +import type { CredentialRefresher, TokenStorage } from "./management-api"; import { claimedExpiresAt, claimedIdentity, @@ -61,6 +63,8 @@ export interface InMemoryCredentialManagerSeed { /** The credential PRISMA_SERVICE_TOKEN supplies. Its token may carry * no `workspace_id` claim, and it may carry a refresh token. */ readonly environmentCredential?: Credential; + /** OAuth exchange used when delegated credentials need rotation. */ + readonly refreshCredential?: CredentialRefresher; } /** The whole stored state, readable back after a run. */ @@ -158,6 +162,7 @@ export class InMemoryCredentialManager implements CredentialManager { private storedSessions: SessionRecord[]; private selection: string | undefined; private readonly environmentCredential: Credential | undefined; + private readonly refreshCredential: CredentialRefresher | undefined; private pin: Pin = { kind: "unresolved" }; private activeStorage: TokenStorage | undefined; @@ -165,6 +170,7 @@ export class InMemoryCredentialManager implements CredentialManager { this.storedSessions = [...(seed.sessions ?? [])]; this.selection = seed.selectedWorkspaceId; this.environmentCredential = seed.environmentCredential; + this.refreshCredential = seed.refreshCredential; if (seed.credential !== undefined) { const workspaceId = credentialWorkspaceId(seed.credential.token); if (workspaceId === undefined) { @@ -266,13 +272,14 @@ export class InMemoryCredentialManager implements CredentialManager { * fresh on every call, never the refresh token. Null when there is * no active credential to read — storage exists only once * activeCredential() has returned non-null. */ - async activeAccessToken(): Promise { + async activeAccessToken( + options?: ActiveAccessTokenOptions, + ): Promise { if ((await this.activeCredential()) === null) { return null; } const storage = await this.activeCredentialStorage(); - const tokens = await storage.getTokens(); - return tokens === null ? null : tokens.accessToken; + return readActiveAccessToken(storage, this.refreshCredential, options); } private buildActiveStorage(): TokenStorage { diff --git a/packages/cli-engine/src/management-api.ts b/packages/cli-engine/src/management-api.ts index fb96cbc6..4732c346 100644 --- a/packages/cli-engine/src/management-api.ts +++ b/packages/cli-engine/src/management-api.ts @@ -29,3 +29,26 @@ export interface ManagementApiClientConfig { readonly apiBaseUrl: string; readonly authBaseUrl: string; } + +/** + * The host-side OAuth exchange the engine may request before handing an + * access-token snapshot to a child. The refresh token crosses only this + * in-process seam; it is never added to the child's environment. + * + * `invalid` is the token endpoint's definitive `invalid_grant` verdict. + * Transport failures and every other endpoint failure are thrown so the + * engine can map them to CLI.AUTH_SERVICE_ERROR without exposing endpoint + * response text. + */ +export type CredentialRefreshResult = + | { + readonly kind: "success"; + readonly accessToken: string; + readonly refreshToken: string; + } + | { readonly kind: "invalid" }; + +export type CredentialRefresher = (request: { + readonly refreshToken: string; + readonly signal: AbortSignal; +}) => Promise; diff --git a/packages/cli-engine/src/testing.ts b/packages/cli-engine/src/testing.ts index 99053d6e..331e485d 100644 --- a/packages/cli-engine/src/testing.ts +++ b/packages/cli-engine/src/testing.ts @@ -8,6 +8,7 @@ import { type SessionRecord, } from "./in-memory-credential-manager"; import type { + CredentialRefresher, ManagementApiClient, ManagementApiClientConfig, } from "./management-api"; @@ -227,6 +228,8 @@ export function createTestCli(spec: { /** The SDK client construction config; defaults point every * endpoint at test.invalid hosts. */ readonly managementApiClientConfig?: ManagementApiClientConfig; + /** OAuth exchange behind delegated-credential preparation. */ + readonly refreshCredential?: CredentialRefresher; /** baseUrl defaults to "https://test.invalid"; when `client` is * supplied, ctx.api IS that object. */ readonly managementApi?: { @@ -279,6 +282,7 @@ export function createTestCli(spec: { selectedWorkspaceId: spec.selectedWorkspaceId, credential: spec.credential, environmentCredential: spec.environmentCredential, + refreshCredential: spec.refreshCredential, }); const managementApiBaseUrl = spec.managementApi?.baseUrl ?? "https://test.invalid"; diff --git a/packages/cli-engine/tests/spawn.test.ts b/packages/cli-engine/tests/spawn.test.ts index e25d5b4a..6767a8ff 100644 --- a/packages/cli-engine/tests/spawn.test.ts +++ b/packages/cli-engine/tests/spawn.test.ts @@ -786,6 +786,128 @@ describe("credential injection", () => { expect(result.spawns).toEqual([]); }); + test("a refreshable near-expiry session is rotated before the child starts", async () => { + let seen: Readonly> = {}; + const rotatedToken = jwtExpiringIn(3600, "ws_1"); + const refreshes: string[] = []; + const cli = createTestCli({ + commands: { converge: credentialedConverge }, + now: CLOCK, + credential: { + token: jwtExpiringIn(60, "ws_1"), + refreshToken: "refresh-1", + expiresAt: undefined, + }, + refreshCredential: async ({ refreshToken }) => { + refreshes.push(refreshToken); + return { + kind: "success", + accessToken: rotatedToken, + refreshToken: "refresh-2", + }; + }, + spawnScript: (request) => { + seen = request.env; + return { exitCode: 0, signal: null }; + }, + }); + + const result = await cli.run(["converge"]); + + expect(result.exitCode).toBe(0); + expect(refreshes).toEqual(["refresh-1"]); + expect(seen.PRISMA_SERVICE_TOKEN).toBe(rotatedToken); + expect(Object.values(seen)).not.toContain("refresh-1"); + expect(Object.values(seen)).not.toContain("refresh-2"); + expect(cli.credentialManager.state().sessions[0]?.credential).toEqual({ + token: rotatedToken, + refreshToken: "refresh-2", + expiresAt: new Date(NOW.getTime() + 3_600_000), + }); + }); + + test("concurrent delegated runs exchange one refresh token once", async () => { + const rotatedToken = jwtExpiringIn(3600, "ws_1"); + let refreshes = 0; + let releaseRefresh: (() => void) | undefined; + const held = new Promise((resolve) => { + releaseRefresh = resolve; + }); + const cli = createTestCli({ + commands: { converge: credentialedConverge }, + now: CLOCK, + credential: { + token: jwtExpiringIn(60, "ws_1"), + refreshToken: "refresh-1", + expiresAt: undefined, + }, + refreshCredential: async () => { + refreshes += 1; + await held; + return { + kind: "success", + accessToken: rotatedToken, + refreshToken: "refresh-2", + }; + }, + }); + + const first = cli.run(["converge"]); + const second = cli.run(["converge"]); + await new Promise((resolve) => setTimeout(resolve, 0)); + releaseRefresh?.(); + + expect( + (await Promise.all([first, second])).map((run) => run.exitCode), + ).toEqual([0, 0]); + expect(refreshes).toBe(1); + }); + + test("invalid_grant ends the stored session and reports it as expired", async () => { + const cli = createTestCli({ + commands: { converge: credentialedConverge }, + now: CLOCK, + credential: { + token: jwtExpiringIn(60, "ws_1"), + refreshToken: "refresh-1", + expiresAt: undefined, + }, + refreshCredential: async () => ({ kind: "invalid" }), + }); + + const result = await cli.run(["converge"]); + + expect(result.exitCode).toBe(2); + expect(result.stderr).toContain("Your session has expired"); + expect(result.spawns).toEqual([]); + expect(cli.credentialManager.state().sessions).toEqual([]); + }); + + test("a transient refresh failure preserves the session", async () => { + const token = jwtExpiringIn(60, "ws_1"); + const cli = createTestCli({ + commands: { converge: credentialedConverge }, + now: CLOCK, + credential: { + token, + refreshToken: "refresh-1", + expiresAt: undefined, + }, + refreshCredential: async () => { + throw new Error("endpoint response carrying SECRET material"); + }, + }); + + const result = await cli.run(["converge"]); + + expect(result.exitCode).toBe(2); + expect(result.stderr).toContain("CLI.AUTH_SERVICE_ERROR"); + expect(result.stderr).not.toContain("SECRET"); + expect(cli.credentialManager.state().sessions[0]?.credential.token).toBe( + token, + ); + }); + // The threshold is five minutes. Bracketing it at 60 s and 3600 s // would pass even with the wrong unit or comparison, so both sides of // the boundary are pinned to the second. diff --git a/packages/cli/src/auth/credential-manager.ts b/packages/cli/src/auth/credential-manager.ts index e3cac0d8..f0cce652 100644 --- a/packages/cli/src/auth/credential-manager.ts +++ b/packages/cli/src/auth/credential-manager.ts @@ -1,14 +1,17 @@ import fs from "node:fs/promises"; import path from "node:path"; import type { + ActiveAccessTokenOptions, ActiveCredential, Credential, CredentialManager, + CredentialRefresher, Session, StoredSessions, TokenStorage, } from "@prisma/cli-engine"; import { + authServiceError, claimedExpiresAt, claimedIdentity, credentialsRequiredError, @@ -16,6 +19,7 @@ import { credentialWorkspaceMismatchError, noSessionForWorkspaceError, } from "@prisma/cli-engine"; +import { CliStructuredError } from "@prisma/cli-engine/protocol"; import { environmentServiceToken } from "./service-token"; import { type CredentialState, @@ -49,6 +53,7 @@ export type FetchWorkspaceName = ( export interface FileCredentialManagerOptions { readonly env: Readonly>; readonly fetchWorkspaceName?: FetchWorkspaceName; + readonly refreshCredential?: CredentialRefresher; readonly debugWrite?: (text: string) => void; } @@ -92,6 +97,76 @@ function memoryBackedStorage( }; } +async function readActiveAccessToken( + storage: TokenStorage, + refreshCredential: CredentialRefresher | undefined, + options?: ActiveAccessTokenOptions, +): Promise { + if (options === undefined) { + return (await storage.getTokens())?.accessToken ?? null; + } + const runLocked = storage.withRefreshLock ?? (async (fn) => fn()); + try { + return await runLocked(async () => { + const current = await storage.getTokens(); + if (current === null) return null; + if (!expiresSoon(current.accessToken, options)) { + return current.accessToken; + } + if (!current.refreshToken) { + throw credentialsRequiredError("expiring-soon"); + } + if (refreshCredential === undefined) { + throw new Error( + "@prisma/cli: delegated OAuth refresh requires a credential refresher", + ); + } + const refreshed = await refreshCredential({ + refreshToken: current.refreshToken, + signal: options.signal, + }); + if (refreshed.kind === "invalid") { + await clearCurrentTokens(storage, current); + throw credentialsRequiredError("expired"); + } + if (expiresSoon(refreshed.accessToken, options)) { + throw new Error("the OAuth endpoint returned a short-lived token"); + } + await storage.setTokens({ + workspaceId: current.workspaceId, + accessToken: refreshed.accessToken, + refreshToken: refreshed.refreshToken, + }); + return refreshed.accessToken; + }); + } catch (cause) { + if (CliStructuredError.is(cause) || options.signal.aborted) throw cause; + throw authServiceError(); + } +} + +function expiresSoon( + token: string, + options: ActiveAccessTokenOptions, +): boolean { + const expiresAt = claimedExpiresAt(token); + return ( + expiresAt !== undefined && + expiresAt.getTime() - options.now.getTime() <= options.minimumValidityMs + ); +} + +async function clearCurrentTokens( + storage: TokenStorage, + current: Tokens, +): Promise { + if (storage.clearTokensIfCurrent !== undefined) { + await storage.clearTokensIfCurrent(current); + return; + } + await storage.clearTokens(); +} + /** * The credential manager over one state file. Sessions are keyed by * workspace id; which credential this process acts as is pinned once; @@ -103,6 +178,7 @@ export class FileCredentialManager implements CredentialManager { readonly #filePath: string; readonly #debug: DebugLog; readonly #fetchWorkspaceName: FetchWorkspaceName | undefined; + readonly #refreshCredential: CredentialRefresher | undefined; #pin: Pin = { kind: "unresolved" }; /** Built for one pinned credential. Every mutation that moves the * pin discards it, so a command that mutates and then reaches for @@ -116,6 +192,7 @@ export class FileCredentialManager implements CredentialManager { this.#filePath = resolveStateFilePath(options.env).filePath; this.#debug = makeDebugLog(options.env, options.debugWrite); this.#fetchWorkspaceName = options.fetchWorkspaceName; + this.#refreshCredential = options.refreshCredential; this.#debug(`state file ${this.#filePath}`); } @@ -269,13 +346,14 @@ export class FileCredentialManager implements CredentialManager { * fresh on every call, never the refresh token. Null when there is * no active credential to read — storage exists only once * activeCredential() has returned non-null. */ - async activeAccessToken(): Promise { + async activeAccessToken( + options?: ActiveAccessTokenOptions, + ): Promise { if ((await this.activeCredential()) === null) { return null; } const storage = await this.activeCredentialStorage(); - const tokens = await storage.getTokens(); - return tokens === null ? null : tokens.accessToken; + return readActiveAccessToken(storage, this.#refreshCredential, options); } /** §11.2: which storage is chosen once, when the pin resolves. Each diff --git a/packages/cli/src/auth/refresh.ts b/packages/cli/src/auth/refresh.ts new file mode 100644 index 00000000..714a0a98 --- /dev/null +++ b/packages/cli/src/auth/refresh.ts @@ -0,0 +1,62 @@ +import type { CredentialRefresher } from "@prisma/cli-engine"; + +import { CLIENT_ID } from "./client"; + +const TRAILING_SLASH = /\/$/; + +interface TokenEndpointBody { + readonly access_token?: unknown; + readonly refresh_token?: unknown; + readonly error?: unknown; +} + +/** The dumb HTTP adapter behind the engine's delegated-credential policy. */ +export function makeCredentialRefresher( + authBaseUrl: string, +): CredentialRefresher { + const endpoint = `${authBaseUrl.replace(TRAILING_SLASH, "")}/token`; + return async ({ refreshToken, signal }) => { + signal.throwIfAborted(); + const response = await fetch(endpoint, { + method: "POST", + headers: { "content-type": "application/x-www-form-urlencoded" }, + body: new URLSearchParams({ + grant_type: "refresh_token", + refresh_token: refreshToken, + client_id: CLIENT_ID, + }), + signal, + }); + const body = await readBody(response); + if ( + response.status >= 400 && + response.status < 500 && + body?.error === "invalid_grant" + ) { + return { kind: "invalid" }; + } + if ( + !response.ok || + typeof body?.access_token !== "string" || + typeof body.refresh_token !== "string" + ) { + throw new Error("OAuth token refresh failed"); + } + return { + kind: "success", + accessToken: body.access_token, + refreshToken: body.refresh_token, + }; + }; +} + +async function readBody(response: Response): Promise { + try { + const body: unknown = await response.json(); + return typeof body === "object" && body !== null + ? (body as TokenEndpointBody) + : null; + } catch { + return null; + } +} diff --git a/packages/cli/src/runtime.ts b/packages/cli/src/runtime.ts index 48156e7a..460d2c51 100644 --- a/packages/cli/src/runtime.ts +++ b/packages/cli/src/runtime.ts @@ -15,6 +15,7 @@ import { getAuthBaseUrl, } from "./auth/client"; import { FileCredentialManager } from "./auth/credential-manager"; +import { makeCredentialRefresher } from "./auth/refresh"; import { DEPRECATED_STATE_FILE_ENV_VAR, resolveStateFilePath, @@ -106,6 +107,7 @@ export async function assembleRuntime(proc: HostProcess): Promise { }; warnOnDeprecatedStateFileEnvVar(proc); const apiBaseUrl = getApiBaseUrl(proc.env); + const authBaseUrl = getAuthBaseUrl(proc.env); return { stdout: { write: (text) => { @@ -135,12 +137,13 @@ export async function assembleRuntime(proc: HostProcess): Promise { credentialManager: new FileCredentialManager({ env: proc.env, fetchWorkspaceName: fetchWorkspaceName(apiBaseUrl), + refreshCredential: makeCredentialRefresher(authBaseUrl), }), managementApiClientConfig: { clientId: CLIENT_ID, redirectUri: DEFAULT_REDIRECT_URI, apiBaseUrl, - authBaseUrl: getAuthBaseUrl(proc.env), + authBaseUrl, }, spawn: spawnChild, /** The engine has already decided and composed; the bin only forks diff --git a/packages/cli/tests/auth-refresh.test.ts b/packages/cli/tests/auth-refresh.test.ts new file mode 100644 index 00000000..00b6bd7b --- /dev/null +++ b/packages/cli/tests/auth-refresh.test.ts @@ -0,0 +1,93 @@ +import { afterEach, describe, expect, it, vi } from "vitest"; + +import { CLIENT_ID } from "../src/auth/client"; +import { makeCredentialRefresher } from "../src/auth/refresh"; + +afterEach(() => { + vi.unstubAllGlobals(); +}); + +describe("makeCredentialRefresher", () => { + it("exchanges a refresh token using the OAuth refresh grant", async () => { + const requestBodies: string[] = []; + vi.stubGlobal( + "fetch", + vi.fn(async (_url: string, init: RequestInit) => { + requestBodies.push(String(init.body)); + return new Response( + JSON.stringify({ + access_token: "access-2", + refresh_token: "refresh-2", + }), + { status: 200, headers: { "content-type": "application/json" } }, + ); + }), + ); + const refresher = makeCredentialRefresher("https://auth.example.test/"); + + const result = await refresher({ + refreshToken: "refresh-1", + signal: new AbortController().signal, + }); + + expect(fetch).toHaveBeenCalledWith( + "https://auth.example.test/token", + expect.objectContaining({ method: "POST" }), + ); + expect(new URLSearchParams(requestBodies[0])).toEqual( + new URLSearchParams({ + grant_type: "refresh_token", + refresh_token: "refresh-1", + client_id: CLIENT_ID, + }), + ); + expect(result).toEqual({ + kind: "success", + accessToken: "access-2", + refreshToken: "refresh-2", + }); + }); + + it("returns only the invalid_grant verdict from a rejected refresh", async () => { + vi.stubGlobal( + "fetch", + vi.fn( + async () => + new Response( + JSON.stringify({ + error: "invalid_grant", + error_description: "response text must not escape", + }), + { status: 400, headers: { "content-type": "application/json" } }, + ), + ), + ); + + await expect( + makeCredentialRefresher("https://auth.example.test")({ + refreshToken: "refresh-1", + signal: new AbortController().signal, + }), + ).resolves.toEqual({ kind: "invalid" }); + }); + + it("throws a fixed error for transient and malformed responses", async () => { + vi.stubGlobal( + "fetch", + vi.fn( + async () => + new Response("upstream SECRET response", { + status: 503, + headers: { "content-type": "text/plain" }, + }), + ), + ); + + await expect( + makeCredentialRefresher("https://auth.example.test")({ + refreshToken: "refresh-1", + signal: new AbortController().signal, + }), + ).rejects.toThrow("OAuth token refresh failed"); + }); +}); diff --git a/packages/cli/tests/credential-manager.test.ts b/packages/cli/tests/credential-manager.test.ts index 58afc165..011f0997 100644 --- a/packages/cli/tests/credential-manager.test.ts +++ b/packages/cli/tests/credential-manager.test.ts @@ -14,7 +14,7 @@ import fsPromises, { } from "node:fs/promises"; import os from "node:os"; import path from "node:path"; -import type { TokenStorage } from "@prisma/cli-engine"; +import type { CredentialRefresher, TokenStorage } from "@prisma/cli-engine"; import { mintTestJwt } from "@prisma/cli-engine/testing"; import { beforeEach, describe, expect, it, vi } from "vitest"; @@ -90,12 +90,14 @@ function makeManager( credential: { token: string }, workspaceId: string, ) => Promise; + refreshCredential?: CredentialRefresher; debugWrite?: (text: string) => void; } = {}, ) { return new FileCredentialManager({ env: { PRISMA_AUTH_FILE: stateFilePath, ...options.env }, fetchWorkspaceName: options.fetchWorkspaceName, + refreshCredential: options.refreshCredential, debugWrite: options.debugWrite, }); } @@ -1015,6 +1017,98 @@ describe("the file-backed TokenStorage", () => { }); }); +describe("delegated access-token preparation", () => { + const now = new Date("2030-01-01T00:00:00.000Z"); + const expiringToken = mintToken(WORKSPACE_A, { + exp: Math.floor(now.getTime() / 1000) + 60, + }); + const freshToken = mintToken(WORKSPACE_A, { + exp: Math.floor(now.getTime() / 1000) + 3600, + }); + const options = { + minimumValidityMs: 5 * 60_000, + now, + signal: new AbortController().signal, + }; + + it("persists a rotated pair and returns only its access token", async () => { + const refreshes: string[] = []; + const manager = makeManager({ + refreshCredential: async ({ refreshToken }) => { + refreshes.push(refreshToken); + return { + kind: "success", + accessToken: freshToken, + refreshToken: "refresh-2", + }; + }, + }); + await manager.createSession( + { + token: expiringToken, + refreshToken: "refresh-1", + expiresAt: undefined, + }, + WORKSPACE_A, + ); + + expect(await manager.activeAccessToken(options)).toBe(freshToken); + expect(refreshes).toEqual(["refresh-1"]); + expect( + (await readCredentialState(stateFilePath)).sessions[0], + ).toMatchObject({ + token: freshToken, + refreshToken: "refresh-2", + expiresAt: new Date(now.getTime() + 3_600_000).toISOString(), + }); + }); + + it("removes only the current pair after invalid_grant", async () => { + const manager = makeManager({ + refreshCredential: async () => ({ kind: "invalid" }), + }); + await manager.createSession( + { + token: expiringToken, + refreshToken: "refresh-1", + expiresAt: undefined, + }, + WORKSPACE_A, + ); + + await expect(manager.activeAccessToken(options)).rejects.toMatchObject({ + code: "CLI.CREDENTIALS_REQUIRED", + }); + expect((await readCredentialState(stateFilePath)).sessions).toEqual([]); + }); + + it("preserves the pair after a transient refresh failure", async () => { + const manager = makeManager({ + refreshCredential: async () => { + throw new Error("auth unavailable"); + }, + }); + await manager.createSession( + { + token: expiringToken, + refreshToken: "refresh-1", + expiresAt: undefined, + }, + WORKSPACE_A, + ); + + await expect(manager.activeAccessToken(options)).rejects.toMatchObject({ + code: "CLI.AUTH_SERVICE_ERROR", + }); + expect( + (await readCredentialState(stateFilePath)).sessions[0], + ).toMatchObject({ + token: expiringToken, + refreshToken: "refresh-1", + }); + }); +}); + describe("token material never leaks", () => { it("keeps the secret out of the debug lines of every write path and out of errors", async () => { const secret = "s3cret-refresh-token"; From edfed4fc41b6515b50c9f6b80a7a0dde77ac457c Mon Sep 17 00:00:00 2001 From: Aman Varshney Date: Fri, 14 Aug 2026 19:27:36 +0530 Subject: [PATCH 2/4] fix(auth): honor stored token expiry --- packages/cli-engine/src/active-access-token.ts | 6 ++++-- .../src/environment-credential-manager.ts | 4 +++- .../src/in-memory-credential-manager.ts | 10 ++++++++-- packages/cli-engine/tests/spawn.test.ts | 18 ++++++++++++++++++ packages/cli/src/auth/credential-manager.ts | 16 ++++++++++++---- packages/cli/tests/credential-manager.test.ts | 16 ++++++++++++++++ 6 files changed, 61 insertions(+), 9 deletions(-) diff --git a/packages/cli-engine/src/active-access-token.ts b/packages/cli-engine/src/active-access-token.ts index 21e96d8c..5bf0e828 100644 --- a/packages/cli-engine/src/active-access-token.ts +++ b/packages/cli-engine/src/active-access-token.ts @@ -14,6 +14,7 @@ export async function readActiveAccessToken( storage: TokenStorage, refreshCredential: CredentialRefresher | undefined, options?: ActiveAccessTokenOptions, + fallbackExpiresAt?: Date, ): Promise { if (options === undefined) { return (await storage.getTokens())?.accessToken ?? null; @@ -25,7 +26,7 @@ export async function readActiveAccessToken( // race is used without exchanging the old refresh token again. const current = await storage.getTokens(); if (current === null) return null; - if (!expiresSoon(current.accessToken, options)) { + if (!expiresSoon(current.accessToken, options, fallbackExpiresAt)) { return current.accessToken; } if (!current.refreshToken) { @@ -63,8 +64,9 @@ export async function readActiveAccessToken( function expiresSoon( token: string, options: ActiveAccessTokenOptions, + fallbackExpiresAt?: Date, ): boolean { - const expiresAt = claimedExpiresAt(token); + const expiresAt = claimedExpiresAt(token) ?? fallbackExpiresAt; return ( expiresAt !== undefined && expiresAt.getTime() - options.now.getTime() <= options.minimumValidityMs diff --git a/packages/cli-engine/src/environment-credential-manager.ts b/packages/cli-engine/src/environment-credential-manager.ts index 9b3b79be..2144550b 100644 --- a/packages/cli-engine/src/environment-credential-manager.ts +++ b/packages/cli-engine/src/environment-credential-manager.ts @@ -105,11 +105,13 @@ export class EnvironmentCredentialManager implements CredentialManager { async activeAccessToken( options?: ActiveAccessTokenOptions, ): Promise { - if ((await this.activeCredential()) === null) return null; + const credential = await this.activeCredential(); + if (credential === null) return null; return readActiveAccessToken( await this.activeCredentialStorage(), undefined, options, + credential.expiresAt, ); } diff --git a/packages/cli-engine/src/in-memory-credential-manager.ts b/packages/cli-engine/src/in-memory-credential-manager.ts index d7191477..c2fabddf 100644 --- a/packages/cli-engine/src/in-memory-credential-manager.ts +++ b/packages/cli-engine/src/in-memory-credential-manager.ts @@ -275,11 +275,17 @@ export class InMemoryCredentialManager implements CredentialManager { async activeAccessToken( options?: ActiveAccessTokenOptions, ): Promise { - if ((await this.activeCredential()) === null) { + const credential = await this.activeCredential(); + if (credential === null) { return null; } const storage = await this.activeCredentialStorage(); - return readActiveAccessToken(storage, this.refreshCredential, options); + return readActiveAccessToken( + storage, + this.refreshCredential, + options, + credential.expiresAt, + ); } private buildActiveStorage(): TokenStorage { diff --git a/packages/cli-engine/tests/spawn.test.ts b/packages/cli-engine/tests/spawn.test.ts index 6767a8ff..7e94ce72 100644 --- a/packages/cli-engine/tests/spawn.test.ts +++ b/packages/cli-engine/tests/spawn.test.ts @@ -786,6 +786,24 @@ describe("credential injection", () => { expect(result.spawns).toEqual([]); }); + test("a token without exp uses its explicit expiry for the threshold", async () => { + const cli = createTestCli({ + commands: { converge: credentialedConverge }, + now: CLOCK, + credential: { + token: mintTestJwt({ workspace_id: "ws_1" }), + refreshToken: undefined, + expiresAt: new Date(NOW.getTime() + 60_000), + }, + }); + + const result = await cli.run(["converge"]); + + expect(result.exitCode).toBe(2); + expect(result.stderr).toContain("expires too soon"); + expect(result.spawns).toEqual([]); + }); + test("a refreshable near-expiry session is rotated before the child starts", async () => { let seen: Readonly> = {}; const rotatedToken = jwtExpiringIn(3600, "ws_1"); diff --git a/packages/cli/src/auth/credential-manager.ts b/packages/cli/src/auth/credential-manager.ts index f0cce652..86176e69 100644 --- a/packages/cli/src/auth/credential-manager.ts +++ b/packages/cli/src/auth/credential-manager.ts @@ -101,6 +101,7 @@ async function readActiveAccessToken( storage: TokenStorage, refreshCredential: CredentialRefresher | undefined, options?: ActiveAccessTokenOptions, + fallbackExpiresAt?: Date, ): Promise { if (options === undefined) { return (await storage.getTokens())?.accessToken ?? null; @@ -110,7 +111,7 @@ async function readActiveAccessToken( return await runLocked(async () => { const current = await storage.getTokens(); if (current === null) return null; - if (!expiresSoon(current.accessToken, options)) { + if (!expiresSoon(current.accessToken, options, fallbackExpiresAt)) { return current.accessToken; } if (!current.refreshToken) { @@ -148,8 +149,9 @@ async function readActiveAccessToken( function expiresSoon( token: string, options: ActiveAccessTokenOptions, + fallbackExpiresAt?: Date, ): boolean { - const expiresAt = claimedExpiresAt(token); + const expiresAt = claimedExpiresAt(token) ?? fallbackExpiresAt; return ( expiresAt !== undefined && expiresAt.getTime() - options.now.getTime() <= options.minimumValidityMs @@ -349,11 +351,17 @@ export class FileCredentialManager implements CredentialManager { async activeAccessToken( options?: ActiveAccessTokenOptions, ): Promise { - if ((await this.activeCredential()) === null) { + const credential = await this.activeCredential(); + if (credential === null) { return null; } const storage = await this.activeCredentialStorage(); - return readActiveAccessToken(storage, this.#refreshCredential, options); + return readActiveAccessToken( + storage, + this.#refreshCredential, + options, + credential.expiresAt, + ); } /** §11.2: which storage is chosen once, when the pin resolves. Each diff --git a/packages/cli/tests/credential-manager.test.ts b/packages/cli/tests/credential-manager.test.ts index 011f0997..8458b58d 100644 --- a/packages/cli/tests/credential-manager.test.ts +++ b/packages/cli/tests/credential-manager.test.ts @@ -1031,6 +1031,22 @@ describe("delegated access-token preparation", () => { signal: new AbortController().signal, }; + it("uses the stored expiry when the access token has no exp claim", async () => { + const manager = makeManager(); + await manager.createSession( + { + token: mintToken(WORKSPACE_A), + refreshToken: undefined, + expiresAt: new Date(now.getTime() + 60_000), + }, + WORKSPACE_A, + ); + + await expect(manager.activeAccessToken(options)).rejects.toMatchObject({ + code: "CLI.CREDENTIALS_REQUIRED", + }); + }); + it("persists a rotated pair and returns only its access token", async () => { const refreshes: string[] = []; const manager = makeManager({ From 68dc19ae46b5da040a6e0877ecd1208427c8f1b7 Mon Sep 17 00:00:00 2001 From: Aman Varshney Date: Fri, 14 Aug 2026 19:50:40 +0530 Subject: [PATCH 3/4] fix(auth): harden delegated credential refresh Signed-off-by: Aman Varshney --- .../engine/credential-manager-design.md | 29 +++--- .../assets/engine/engine-interface-draft.ts | 17 +++- .../prisma-cli-v8/specs/s3-composer.md | 3 +- .../cli-engine/src/active-access-token.ts | 18 ++-- .../src/environment-credential-manager.ts | 11 ++- .../cli-engine/src/execution/api-client.ts | 2 +- packages/cli-engine/src/execution/spawn.ts | 13 ++- packages/cli-engine/src/exports/index.ts | 1 + .../src/in-memory-credential-manager.ts | 22 ++--- packages/cli-engine/src/management-api.ts | 22 ++++- packages/cli-engine/tests/engine.test.ts | 1 + packages/cli-engine/tests/spawn.test.ts | 55 ++++++++++- packages/cli/src/auth/credential-manager.ts | 99 +++---------------- packages/cli/src/auth/refresh.ts | 18 +++- packages/cli/tests/auth-refresh.test.ts | 64 +++++++++++- packages/cli/tests/credential-manager.test.ts | 78 ++++++++++++++- 16 files changed, 311 insertions(+), 142 deletions(-) diff --git a/.drive/projects/prisma-cli-v8/assets/engine/credential-manager-design.md b/.drive/projects/prisma-cli-v8/assets/engine/credential-manager-design.md index 4f5af6fe..4ce85305 100644 --- a/.drive/projects/prisma-cli-v8/assets/engine/credential-manager-design.md +++ b/.drive/projects/prisma-cli-v8/assets/engine/credential-manager-design.md @@ -309,8 +309,10 @@ current") with nextActions `auth workspace use` and login. TokenStorage view; the exchange itself runs OUTSIDE the file lock (§8) — only the resulting write takes it: - `setTokens` (the rotation write): updates IN PLACE only `token`, - `refreshToken`, `expiresAt` (re-derived from claims; the SDK's - pair carries no expiry) of its workspace's record. NEVER creates + `refreshToken`, `expiresAt` (the proactive token-endpoint adapter + supplies the explicit OAuth lifetime; an SDK-driven rotation falls + back to the access token's claim) of its workspace's record. NEVER + creates a record, NEVER moves the marker, NEVER touches the name. If the freshly-read state has no record for that workspace (ended by another process), refuse and throw — no resurrection. If the new @@ -758,9 +760,12 @@ endAllSessions(): Promise; * has returned non-null; the engine resolves that first. */ activeCredentialStorage(): Promise; /** ENGINE-FACING (S3). The active credential's ACCESS token, read - * fresh on every call, for handing to a child process. Never the - * refresh token. Single consumer: ctx.spawn's credential injection. */ -activeAccessToken(): Promise; + * fresh on every call, for handing to a child process. With options, + * refreshes or refuses a token that lacks the required remaining + * lifetime. Never the refresh token. */ +activeAccessToken( + options?: ActiveAccessTokenOptions, +): Promise; ``` All three mutations are workspace-id-keyed, symmetric with @@ -774,10 +779,10 @@ S3 amendment (2026-08-11, re-ruled after the PR-136 architect review): the engine forwards the storage `activeCredentialStorage()` returns into SDK client config and never calls its methods itself — no exceptions. What the spawn path needs is a manager OPERATION, not a -carve-out: the interface gains `activeAccessToken()`, whose single -consumer is `ctx.spawn`'s credential injection in the engine's spawn -module (`packages/cli-engine/src/execution/spawn.ts`, `spawnToken`). -It is read at spawn time and handed to the child as +carve-out: the interface gains `activeAccessToken()`, consumed by the +delegated-credential preflight and by `ctx.spawn`'s credential injection +in the engine's spawn module (`packages/cli-engine/src/execution/spawn.ts`, +`spawnToken`). It is read at spawn time and handed to the child as `PRISMA_SERVICE_TOKEN` (+ `PRISMA_WORKSPACE_ID` when the credential names a workspace; when it names none, an inherited `PRISMA_WORKSPACE_ID` is DELETED from the child environment — the two @@ -798,9 +803,9 @@ engine asks `activeAccessToken(options)` to refresh the pair under the manager's storage lock, persist the rotation, and return the new access token. The shipped manager receives a host-side token-endpoint adapter at construction; the manager remains the sole owner of storage reads and writes, and the -refresh token is never added to child env. Calling `activeAccessToken()` with -no options remains the fresh spawn-time read, so rotation by another process -between preflight and spawn is still observed. +refresh token is never added to child env. The spawn-time call is another +validated fresh read, so rotation by another process between preflight and +spawn is still observed without handing the child an unchecked replacement. ### 11.6 whoami diff --git a/.drive/projects/prisma-cli-v8/assets/engine/engine-interface-draft.ts b/.drive/projects/prisma-cli-v8/assets/engine/engine-interface-draft.ts index 3b240716..befb6ec4 100644 --- a/.drive/projects/prisma-cli-v8/assets/engine/engine-interface-draft.ts +++ b/.drive/projects/prisma-cli-v8/assets/engine/engine-interface-draft.ts @@ -43,8 +43,9 @@ * separate credentialsForSpawn declaration is gone, and the entailment * (child credentials imply the credentials need) is structural. The * manager gains the named engine-facing operation activeAccessToken() - * for the spawn path's read, so the "engine never calls storage - * methods" rule is absolute — no sanctioned exception. + * for delegated preflight and the spawn-time read, so the "engine + * never calls storage methods" rule is absolute — no sanctioned + * exception. * exitWithChildStatus(opts?) takes { nextActions? }, rendered * to stderr before the exit (R-S3-4's reproduce hint). * Amended 2026-08-11 (operator ruling) — SIGNAL SETTLEMENT IS THE @@ -635,8 +636,9 @@ export interface CredentialManager { * process that authenticates as this process does. Never the * refresh token — the child gets a snapshot it cannot refresh. * With options, refreshes a near-expiry stored OAuth pair before - * returning its access token. With no options, this is ctx.spawn's - * fresh read. The refresh token never reaches the child. */ + * returning its access token. Preflight and ctx.spawn both use the + * options form so the spawn-time fresh read is also validated. The + * refresh token never reaches the child. */ activeAccessToken(options?: ActiveAccessTokenOptions): Promise } @@ -649,7 +651,12 @@ import type { } from '@prisma/management-api-sdk' export type ManagementApiClient = SdkClient -export type TokenStorage = SdkTokenStorage +type SdkTokens = NonNullable>> +type StoredTokens = SdkTokens & { readonly expiresAt?: Date } +export type TokenStorage = Omit & { + getTokens(): Promise + setTokens(tokens: SdkTokens, expiresAt?: Date): Promise +} /** SDK client construction config, injected by the bin beside the * manager (§10). All four fields: the SDK's refreshing fetch diff --git a/.drive/projects/prisma-cli-v8/specs/s3-composer.md b/.drive/projects/prisma-cli-v8/specs/s3-composer.md index 7c9c69b6..3c6546d0 100644 --- a/.drive/projects/prisma-cli-v8/specs/s3-composer.md +++ b/.drive/projects/prisma-cli-v8/specs/s3-composer.md @@ -465,7 +465,8 @@ Acceptance verified against source and merged PRs: prisma-cli #136, are `packages/cli-engine/tests/spawn-real-child.test.ts` and `spawn.test.ts` (plus `environment-credential-manager.test.ts`); the SPI amendment is `credential-manager-design.md` §11.5 -(`activeAccessToken()`, single consumer `execution/spawn.ts`); the +(`activeAccessToken(options)`, consumed by delegated preflight and +`execution/spawn.ts`); the static-graph check is composer's `check:family-static-graph` and the sole-listener detector is composer's `cli/src/family/__tests__/signal-listeners.test.ts`; the tarball diff --git a/packages/cli-engine/src/active-access-token.ts b/packages/cli-engine/src/active-access-token.ts index 5bf0e828..dbaa9cd0 100644 --- a/packages/cli-engine/src/active-access-token.ts +++ b/packages/cli-engine/src/active-access-token.ts @@ -14,7 +14,6 @@ export async function readActiveAccessToken( storage: TokenStorage, refreshCredential: CredentialRefresher | undefined, options?: ActiveAccessTokenOptions, - fallbackExpiresAt?: Date, ): Promise { if (options === undefined) { return (await storage.getTokens())?.accessToken ?? null; @@ -26,7 +25,7 @@ export async function readActiveAccessToken( // race is used without exchanging the old refresh token again. const current = await storage.getTokens(); if (current === null) return null; - if (!expiresSoon(current.accessToken, options, fallbackExpiresAt)) { + if (!expiresSoon(current.accessToken, options, current.expiresAt)) { return current.accessToken; } if (!current.refreshToken) { @@ -45,14 +44,17 @@ export async function readActiveAccessToken( await clearCurrentTokens(storage, current); throw credentialsRequiredError("expired"); } - if (expiresSoon(refreshed.accessToken, options)) { + if (expiresSoon(refreshed.accessToken, options, refreshed.expiresAt)) { throw new Error("the OAuth endpoint returned a short-lived token"); } - await storage.setTokens({ - workspaceId: current.workspaceId, - accessToken: refreshed.accessToken, - refreshToken: refreshed.refreshToken, - }); + await storage.setTokens( + { + workspaceId: current.workspaceId, + accessToken: refreshed.accessToken, + refreshToken: refreshed.refreshToken, + }, + refreshed.expiresAt, + ); return refreshed.accessToken; }); } catch (cause) { diff --git a/packages/cli-engine/src/environment-credential-manager.ts b/packages/cli-engine/src/environment-credential-manager.ts index 2144550b..178db8fa 100644 --- a/packages/cli-engine/src/environment-credential-manager.ts +++ b/packages/cli-engine/src/environment-credential-manager.ts @@ -100,7 +100,7 @@ export class EnvironmentCredentialManager implements CredentialManager { return this.#activeStorage; } - /** The spawn path's read: the env token passes through directly. It + /** The delegated path's read: the env token passes through directly. It * is already a snapshot with no refresh token behind it. */ async activeAccessToken( options?: ActiveAccessTokenOptions, @@ -111,7 +111,6 @@ export class EnvironmentCredentialManager implements CredentialManager { await this.activeCredentialStorage(), undefined, options, - credential.expiresAt, ); } @@ -128,6 +127,7 @@ export class EnvironmentCredentialManager implements CredentialManager { workspaceId: this.#workspaceId(token) ?? NO_WORKSPACE_NAMED, accessToken: token, refreshToken: undefined, + expiresAt: claimedExpiresAt(token), }; const singleFlight = (fn: () => Promise): Promise => { const queued = this.#refreshLock.then(fn, fn); @@ -139,8 +139,11 @@ export class EnvironmentCredentialManager implements CredentialManager { }; return { getTokens: async () => tokens, - setTokens: async (rotated) => { - tokens = rotated; + setTokens: async (rotated, expiresAt) => { + tokens = { + ...rotated, + expiresAt: claimedExpiresAt(rotated.accessToken) ?? expiresAt, + }; }, clearTokens: async () => { tokens = null; diff --git a/packages/cli-engine/src/execution/api-client.ts b/packages/cli-engine/src/execution/api-client.ts index 2d6fdf18..4d732782 100644 --- a/packages/cli-engine/src/execution/api-client.ts +++ b/packages/cli-engine/src/execution/api-client.ts @@ -179,7 +179,7 @@ function observedTokenStorage( }; return { getTokens: () => storage.getTokens(), - setTokens: (tokens) => storage.setTokens(tokens), + setTokens: (tokens, expiresAt) => storage.setTokens(tokens, expiresAt), clearTokens: () => storage.clearTokens(), ...(storage.clearTokensIfCurrent === undefined ? {} diff --git a/packages/cli-engine/src/execution/spawn.ts b/packages/cli-engine/src/execution/spawn.ts index 3be412e5..2ccc9699 100644 --- a/packages/cli-engine/src/execution/spawn.ts +++ b/packages/cli-engine/src/execution/spawn.ts @@ -16,6 +16,7 @@ import type { import { constructionError } from "./command-tree"; import { makeDebugLog } from "./debug"; import type { Invocation } from "./engine"; +import { CREDENTIAL_NEAR_EXPIRY_MS } from "./needs"; import { firstLine } from "./rendering"; import { flushBufferedEvents } from "./reporting"; @@ -306,15 +307,21 @@ async function composeChildEnv( /** * The child's copy of the credential: the manager's activeAccessToken() - * operation, read at spawn time. The refresh token is never injected: - * the child runs on a snapshot it cannot refresh. + * operation, validated again at spawn time. This preserves the fresh re-read + * while ensuring a token changed after preflight still satisfies the child's + * minimum lifetime. The refresh token is never injected: the child runs on a + * snapshot it cannot refresh. */ async function spawnToken(invocation: Invocation): Promise { const manager = invocation.runtime.credentialManager; if (manager === undefined) { throw credentialsRequiredError(); } - const accessToken = await manager.activeAccessToken(); + const accessToken = await manager.activeAccessToken({ + minimumValidityMs: CREDENTIAL_NEAR_EXPIRY_MS, + now: invocation.now(), + signal: invocation.signal, + }); if (accessToken === null) { throw credentialsRequiredError("session-ended"); } diff --git a/packages/cli-engine/src/exports/index.ts b/packages/cli-engine/src/exports/index.ts index fe78d648..4c47e726 100644 --- a/packages/cli-engine/src/exports/index.ts +++ b/packages/cli-engine/src/exports/index.ts @@ -5,6 +5,7 @@ * The test harness lives on the ./testing subpath. */ +export { readActiveAccessToken } from "../active-access-token"; export { type Args, type ArgsSpec, diff --git a/packages/cli-engine/src/in-memory-credential-manager.ts b/packages/cli-engine/src/in-memory-credential-manager.ts index c2fabddf..811afcb0 100644 --- a/packages/cli-engine/src/in-memory-credential-manager.ts +++ b/packages/cli-engine/src/in-memory-credential-manager.ts @@ -125,11 +125,15 @@ function memoryBackedStorage( credentialWorkspaceId(credential.token) ?? NO_WORKSPACE_CLAIMED, accessToken: credential.token, refreshToken: credential.refreshToken, + expiresAt: claimedExpiresAt(credential.token) ?? credential.expiresAt, }; return { getTokens: async () => tokens, - setTokens: async (rotated) => { - tokens = rotated; + setTokens: async (rotated, expiresAt) => { + tokens = { + ...rotated, + expiresAt: claimedExpiresAt(rotated.accessToken) ?? expiresAt, + }; }, clearTokens: async () => { tokens = null; @@ -268,7 +272,7 @@ export class InMemoryCredentialManager implements CredentialManager { return this.activeStorage; } - /** The spawn path's read: the active credential's access token, + /** The delegated path's read: the active credential's access token, * fresh on every call, never the refresh token. Null when there is * no active credential to read — storage exists only once * activeCredential() has returned non-null. */ @@ -280,12 +284,7 @@ export class InMemoryCredentialManager implements CredentialManager { return null; } const storage = await this.activeCredentialStorage(); - return readActiveAccessToken( - storage, - this.refreshCredential, - options, - credential.expiresAt, - ); + return readActiveAccessToken(storage, this.refreshCredential, options); } private buildActiveStorage(): TokenStorage { @@ -318,9 +317,10 @@ export class InMemoryCredentialManager implements CredentialManager { workspaceId, accessToken: record.credential.token, refreshToken: record.credential.refreshToken, + expiresAt: record.credential.expiresAt, }; }, - setTokens: async (tokens) => { + setTokens: async (tokens, expiresAt) => { const record = pinnedRecord(); if (record === undefined) { // The same structured error the real manager raises, so a @@ -339,7 +339,7 @@ export class InMemoryCredentialManager implements CredentialManager { credential: { token: tokens.accessToken, refreshToken: tokens.refreshToken, - expiresAt: claimedExpiresAt(tokens.accessToken), + expiresAt: claimedExpiresAt(tokens.accessToken) ?? expiresAt, }, } : stored, diff --git a/packages/cli-engine/src/management-api.ts b/packages/cli-engine/src/management-api.ts index 4732c346..3bed7777 100644 --- a/packages/cli-engine/src/management-api.ts +++ b/packages/cli-engine/src/management-api.ts @@ -3,6 +3,13 @@ import type { TokenStorage as SdkTokenStorage, } from "@prisma/management-api-sdk"; +type SdkTokens = NonNullable>>; + +type StoredTokens = SdkTokens & { + /** The explicit OAuth lifetime when the access token has no exp claim. */ + readonly expiresAt?: Date; +}; + /** * The SDK's typed client, re-exported so consumers never import * @prisma/management-api-sdk directly. @@ -10,12 +17,15 @@ import type { export type ManagementApiClient = SdkClient; /** - * The SDK's token-storage contract, re-exported for the same reason. - * CredentialManager.activeCredentialStorage returns one; the engine - * forwards it into SDK client config and reads it only to tell a - * credential that could never be renewed from one that could. + * The SDK's token-storage contract plus the explicit OAuth expiry the + * credential manager persists for opaque access tokens. The extra data and + * optional setTokens argument are structurally compatible with the SDK, which + * ignores expiry and continues to call setTokens with one argument. */ -export type TokenStorage = SdkTokenStorage; +export type TokenStorage = Omit & { + getTokens(): Promise; + setTokens(tokens: SdkTokens, expiresAt?: Date): Promise; +}; /** * SDK client construction config, injected by the bin beside the @@ -45,6 +55,8 @@ export type CredentialRefreshResult = readonly kind: "success"; readonly accessToken: string; readonly refreshToken: string; + /** Absolute lifetime reported by the OAuth token endpoint. */ + readonly expiresAt: Date; } | { readonly kind: "invalid" }; diff --git a/packages/cli-engine/tests/engine.test.ts b/packages/cli-engine/tests/engine.test.ts index 83fcbc5f..e731ec92 100644 --- a/packages/cli-engine/tests/engine.test.ts +++ b/packages/cli-engine/tests/engine.test.ts @@ -40,6 +40,7 @@ describe("main export", () => { "loadConfig", "noSessionForWorkspaceError", "positional", + "readActiveAccessToken", "telemetryCommandGroup", ]); }); diff --git a/packages/cli-engine/tests/spawn.test.ts b/packages/cli-engine/tests/spawn.test.ts index 7e94ce72..f9daaf2d 100644 --- a/packages/cli-engine/tests/spawn.test.ts +++ b/packages/cli-engine/tests/spawn.test.ts @@ -742,6 +742,52 @@ describe("credential injection", () => { expect(Object.values(seen)).not.toContain("refresh-material"); }); + test("a token replaced after preflight is validated again at spawn time", async () => { + const reportThenSpawn = defineCommand({ + help: { summary: "Reports, then hands credentials to a child" }, + maySpawn: true, + needs: { credentials: "child" }, + handler: async (_args, ctx) => { + ctx.report({ kind: "status", subject: "run", status: "pre-spawn" }); + await ctx.spawn({ command: "alchemy" }); + return ok(exitWithChildStatus()); + }, + }); + const cli = createTestCli({ + commands: { converge: reportThenSpawn }, + now: CLOCK, + credential: { + token: jwtExpiringIn(3_600, "ws_1"), + refreshToken: undefined, + expiresAt: undefined, + }, + }); + + const result = await cli.run(["converge"], { + onEvent: (event) => { + if (event.kind === "status") { + cli.credentialManager.overwriteStoredState({ + sessions: [ + { + workspaceId: "ws_1", + workspaceName: undefined, + credential: { + token: jwtExpiringIn(60, "ws_1"), + refreshToken: undefined, + expiresAt: undefined, + }, + }, + ], + }); + } + }, + }); + + expect(result.exitCode).toBe(2); + expect(result.stderr).toContain("expires too soon"); + expect(result.spawns).toEqual([]); + }); + test("an environment credential's token is injected unchanged", async () => { let seen: Readonly> = {}; const token = jwtExpiringIn(3600, "ws_env"); @@ -822,6 +868,7 @@ describe("credential injection", () => { kind: "success", accessToken: rotatedToken, refreshToken: "refresh-2", + expiresAt: new Date(NOW.getTime() + 3_600_000), }; }, spawnScript: (request) => { @@ -848,9 +895,13 @@ describe("credential injection", () => { const rotatedToken = jwtExpiringIn(3600, "ws_1"); let refreshes = 0; let releaseRefresh: (() => void) | undefined; + let markRefreshStarted: (() => void) | undefined; const held = new Promise((resolve) => { releaseRefresh = resolve; }); + const refreshStarted = new Promise((resolve) => { + markRefreshStarted = resolve; + }); const cli = createTestCli({ commands: { converge: credentialedConverge }, now: CLOCK, @@ -861,18 +912,20 @@ describe("credential injection", () => { }, refreshCredential: async () => { refreshes += 1; + markRefreshStarted?.(); await held; return { kind: "success", accessToken: rotatedToken, refreshToken: "refresh-2", + expiresAt: new Date(NOW.getTime() + 3_600_000), }; }, }); const first = cli.run(["converge"]); const second = cli.run(["converge"]); - await new Promise((resolve) => setTimeout(resolve, 0)); + await refreshStarted; releaseRefresh?.(); expect( diff --git a/packages/cli/src/auth/credential-manager.ts b/packages/cli/src/auth/credential-manager.ts index 86176e69..7ab21a25 100644 --- a/packages/cli/src/auth/credential-manager.ts +++ b/packages/cli/src/auth/credential-manager.ts @@ -11,15 +11,14 @@ import type { TokenStorage, } from "@prisma/cli-engine"; import { - authServiceError, claimedExpiresAt, claimedIdentity, credentialsRequiredError, credentialWorkspaceId, credentialWorkspaceMismatchError, noSessionForWorkspaceError, + readActiveAccessToken, } from "@prisma/cli-engine"; -import { CliStructuredError } from "@prisma/cli-engine/protocol"; import { environmentServiceToken } from "./service-token"; import { type CredentialState, @@ -84,11 +83,15 @@ function memoryBackedStorage( credentialWorkspaceId(credential.token) ?? NO_WORKSPACE_CLAIMED, accessToken: credential.token, refreshToken: credential.refreshToken, + expiresAt: claimedExpiresAt(credential.token) ?? credential.expiresAt, }; return { getTokens: async () => tokens, - setTokens: async (rotated) => { - tokens = rotated; + setTokens: async (rotated, expiresAt) => { + tokens = { + ...rotated, + expiresAt: claimedExpiresAt(rotated.accessToken) ?? expiresAt, + }; }, clearTokens: async () => { tokens = null; @@ -97,78 +100,6 @@ function memoryBackedStorage( }; } -async function readActiveAccessToken( - storage: TokenStorage, - refreshCredential: CredentialRefresher | undefined, - options?: ActiveAccessTokenOptions, - fallbackExpiresAt?: Date, -): Promise { - if (options === undefined) { - return (await storage.getTokens())?.accessToken ?? null; - } - const runLocked = storage.withRefreshLock ?? (async (fn) => fn()); - try { - return await runLocked(async () => { - const current = await storage.getTokens(); - if (current === null) return null; - if (!expiresSoon(current.accessToken, options, fallbackExpiresAt)) { - return current.accessToken; - } - if (!current.refreshToken) { - throw credentialsRequiredError("expiring-soon"); - } - if (refreshCredential === undefined) { - throw new Error( - "@prisma/cli: delegated OAuth refresh requires a credential refresher", - ); - } - const refreshed = await refreshCredential({ - refreshToken: current.refreshToken, - signal: options.signal, - }); - if (refreshed.kind === "invalid") { - await clearCurrentTokens(storage, current); - throw credentialsRequiredError("expired"); - } - if (expiresSoon(refreshed.accessToken, options)) { - throw new Error("the OAuth endpoint returned a short-lived token"); - } - await storage.setTokens({ - workspaceId: current.workspaceId, - accessToken: refreshed.accessToken, - refreshToken: refreshed.refreshToken, - }); - return refreshed.accessToken; - }); - } catch (cause) { - if (CliStructuredError.is(cause) || options.signal.aborted) throw cause; - throw authServiceError(); - } -} - -function expiresSoon( - token: string, - options: ActiveAccessTokenOptions, - fallbackExpiresAt?: Date, -): boolean { - const expiresAt = claimedExpiresAt(token) ?? fallbackExpiresAt; - return ( - expiresAt !== undefined && - expiresAt.getTime() - options.now.getTime() <= options.minimumValidityMs - ); -} - -async function clearCurrentTokens( - storage: TokenStorage, - current: Tokens, -): Promise { - if (storage.clearTokensIfCurrent !== undefined) { - await storage.clearTokensIfCurrent(current); - return; - } - await storage.clearTokens(); -} - /** * The credential manager over one state file. Sessions are keyed by * workspace id; which credential this process acts as is pinned once; @@ -344,7 +275,7 @@ export class FileCredentialManager implements CredentialManager { return this.#activeStorage; } - /** The spawn path's read: the active credential's access token, + /** The delegated path's read: the active credential's access token, * fresh on every call, never the refresh token. Null when there is * no active credential to read — storage exists only once * activeCredential() has returned non-null. */ @@ -356,12 +287,7 @@ export class FileCredentialManager implements CredentialManager { return null; } const storage = await this.activeCredentialStorage(); - return readActiveAccessToken( - storage, - this.#refreshCredential, - options, - credential.expiresAt, - ); + return readActiveAccessToken(storage, this.#refreshCredential, options); } /** §11.2: which storage is chosen once, when the pin resolves. Each @@ -406,10 +332,13 @@ export class FileCredentialManager implements CredentialManager { ...(record.refreshToken === undefined ? {} : { refreshToken: record.refreshToken }), + ...(record.expiresAt === undefined + ? {} + : { expiresAt: new Date(record.expiresAt) }), }; }, - setTokens: async (tokens) => { + setTokens: async (tokens, expiresAt) => { this.#debug(`rotation write for session ${workspaceId}`); const claimed = credentialWorkspaceId(tokens.accessToken); if (claimed !== undefined && claimed !== workspaceId) { @@ -429,7 +358,7 @@ export class FileCredentialManager implements CredentialManager { ...(tokens.refreshToken === undefined ? {} : { refreshToken: tokens.refreshToken }), - ...expiresAtSlice(tokens.accessToken, undefined), + ...expiresAtSlice(tokens.accessToken, expiresAt), }; return { state: { diff --git a/packages/cli/src/auth/refresh.ts b/packages/cli/src/auth/refresh.ts index 714a0a98..235ef288 100644 --- a/packages/cli/src/auth/refresh.ts +++ b/packages/cli/src/auth/refresh.ts @@ -3,10 +3,12 @@ import type { CredentialRefresher } from "@prisma/cli-engine"; import { CLIENT_ID } from "./client"; const TRAILING_SLASH = /\/$/; +const CREDENTIAL_REFRESH_TIMEOUT_MS = 10_000; interface TokenEndpointBody { readonly access_token?: unknown; readonly refresh_token?: unknown; + readonly expires_in?: unknown; readonly error?: unknown; } @@ -17,6 +19,10 @@ export function makeCredentialRefresher( const endpoint = `${authBaseUrl.replace(TRAILING_SLASH, "")}/token`; return async ({ refreshToken, signal }) => { signal.throwIfAborted(); + const refreshSignal = AbortSignal.any([ + signal, + AbortSignal.timeout(CREDENTIAL_REFRESH_TIMEOUT_MS), + ]); const response = await fetch(endpoint, { method: "POST", headers: { "content-type": "application/x-www-form-urlencoded" }, @@ -25,7 +31,7 @@ export function makeCredentialRefresher( refresh_token: refreshToken, client_id: CLIENT_ID, }), - signal, + signal: refreshSignal, }); const body = await readBody(response); if ( @@ -38,14 +44,20 @@ export function makeCredentialRefresher( if ( !response.ok || typeof body?.access_token !== "string" || - typeof body.refresh_token !== "string" + typeof body.refresh_token !== "string" || + typeof body.expires_in !== "number" || + !Number.isFinite(body.expires_in) || + body.expires_in < 0 ) { - throw new Error("OAuth token refresh failed"); + throw new Error( + `OAuth token refresh failed (status ${String(response.status)})`, + ); } return { kind: "success", accessToken: body.access_token, refreshToken: body.refresh_token, + expiresAt: new Date(Date.now() + body.expires_in * 1_000), }; }; } diff --git a/packages/cli/tests/auth-refresh.test.ts b/packages/cli/tests/auth-refresh.test.ts index 00b6bd7b..6eb53d5f 100644 --- a/packages/cli/tests/auth-refresh.test.ts +++ b/packages/cli/tests/auth-refresh.test.ts @@ -5,10 +5,13 @@ import { makeCredentialRefresher } from "../src/auth/refresh"; afterEach(() => { vi.unstubAllGlobals(); + vi.restoreAllMocks(); }); describe("makeCredentialRefresher", () => { it("exchanges a refresh token using the OAuth refresh grant", async () => { + const now = new Date("2030-01-01T00:00:00.000Z"); + vi.spyOn(Date, "now").mockReturnValue(now.getTime()); const requestBodies: string[] = []; vi.stubGlobal( "fetch", @@ -18,6 +21,7 @@ describe("makeCredentialRefresher", () => { JSON.stringify({ access_token: "access-2", refresh_token: "refresh-2", + expires_in: 3_600, }), { status: 200, headers: { "content-type": "application/json" } }, ); @@ -45,6 +49,7 @@ describe("makeCredentialRefresher", () => { kind: "success", accessToken: "access-2", refreshToken: "refresh-2", + expiresAt: new Date(now.getTime() + 3_600_000), }); }); @@ -71,7 +76,7 @@ describe("makeCredentialRefresher", () => { ).resolves.toEqual({ kind: "invalid" }); }); - it("throws a fixed error for transient and malformed responses", async () => { + it("throws a fixed error for a transient response", async () => { vi.stubGlobal( "fetch", vi.fn( @@ -88,6 +93,61 @@ describe("makeCredentialRefresher", () => { refreshToken: "refresh-1", signal: new AbortController().signal, }), - ).rejects.toThrow("OAuth token refresh failed"); + ).rejects.toThrow("OAuth token refresh failed (status 503)"); + }); + + it("throws a fixed error for a malformed successful response", async () => { + vi.stubGlobal( + "fetch", + vi.fn( + async () => + new Response( + JSON.stringify({ + access_token: "access-2", + expires_in: 3_600, + }), + { status: 200, headers: { "content-type": "application/json" } }, + ), + ), + ); + + await expect( + makeCredentialRefresher("https://auth.example.test")({ + refreshToken: "refresh-1", + signal: new AbortController().signal, + }), + ).rejects.toThrow("OAuth token refresh failed (status 200)"); + }); + + it("aborts a stalled refresh after the fixed timeout", async () => { + const timeout = new AbortController(); + const timeoutSpy = vi + .spyOn(AbortSignal, "timeout") + .mockReturnValue(timeout.signal); + vi.stubGlobal( + "fetch", + vi.fn( + async (_url: string, init: RequestInit) => + await new Promise((_resolve, reject) => { + init.signal?.addEventListener( + "abort", + () => reject(init.signal?.reason), + { once: true }, + ); + }), + ), + ); + const refresh = makeCredentialRefresher("https://auth.example.test")({ + refreshToken: "refresh-1", + signal: new AbortController().signal, + }); + const rejection = expect(refresh).rejects.toMatchObject({ + name: "TimeoutError", + }); + + timeout.abort(new DOMException("Timed out", "TimeoutError")); + + await rejection; + expect(timeoutSpy).toHaveBeenCalledWith(10_000); }); }); diff --git a/packages/cli/tests/credential-manager.test.ts b/packages/cli/tests/credential-manager.test.ts index 8458b58d..17480b06 100644 --- a/packages/cli/tests/credential-manager.test.ts +++ b/packages/cli/tests/credential-manager.test.ts @@ -1056,6 +1056,7 @@ describe("delegated access-token preparation", () => { kind: "success", accessToken: freshToken, refreshToken: "refresh-2", + expiresAt: new Date(now.getTime() + 3_600_000), }; }, }); @@ -1079,10 +1080,80 @@ describe("delegated access-token preparation", () => { }); }); + it("persists the token-endpoint expiry for a rotated opaque token", async () => { + const opaqueToken = "opaque-access-token"; + const expiresAt = new Date(now.getTime() + 3_600_000); + let refreshes = 0; + const manager = makeManager({ + refreshCredential: async () => { + refreshes += 1; + return { + kind: "success", + accessToken: opaqueToken, + refreshToken: "refresh-2", + expiresAt, + }; + }, + }); + await manager.createSession( + { + token: expiringToken, + refreshToken: "refresh-1", + expiresAt: undefined, + }, + WORKSPACE_A, + ); + + await expect(manager.activeAccessToken(options)).resolves.toBe(opaqueToken); + await expect(manager.activeAccessToken(options)).resolves.toBe(opaqueToken); + + expect(refreshes).toBe(1); + expect( + (await readCredentialState(stateFilePath)).sessions[0], + ).toMatchObject({ + token: opaqueToken, + refreshToken: "refresh-2", + expiresAt: expiresAt.toISOString(), + }); + }); + + it("rejects and does not persist a rotated token that expires too soon", async () => { + const manager = makeManager({ + refreshCredential: async () => ({ + kind: "success", + accessToken: "short-lived-opaque-token", + refreshToken: "refresh-2", + expiresAt: new Date(now.getTime() + 60_000), + }), + }); + await manager.createSession( + { + token: expiringToken, + refreshToken: "refresh-1", + expiresAt: undefined, + }, + WORKSPACE_A, + ); + + await expect(manager.activeAccessToken(options)).rejects.toMatchObject({ + code: "CLI.AUTH_SERVICE_ERROR", + }); + expect( + (await readCredentialState(stateFilePath)).sessions[0], + ).toMatchObject({ + token: expiringToken, + refreshToken: "refresh-1", + }); + }); + it("removes only the current pair after invalid_grant", async () => { const manager = makeManager({ refreshCredential: async () => ({ kind: "invalid" }), }); + await manager.createSession( + credentialFor(WORKSPACE_B, "refresh-b"), + WORKSPACE_B, + ); await manager.createSession( { token: expiringToken, @@ -1095,7 +1166,12 @@ describe("delegated access-token preparation", () => { await expect(manager.activeAccessToken(options)).rejects.toMatchObject({ code: "CLI.CREDENTIALS_REQUIRED", }); - expect((await readCredentialState(stateFilePath)).sessions).toEqual([]); + expect((await readCredentialState(stateFilePath)).sessions).toEqual([ + expect.objectContaining({ + workspaceId: WORKSPACE_B, + refreshToken: "refresh-b", + }), + ]); }); it("preserves the pair after a transient refresh failure", async () => { From 09eed8c5e793c19671977cc174ea2e78400819fc Mon Sep 17 00:00:00 2001 From: willbot Date: Mon, 17 Aug 2026 11:37:00 +0200 Subject: [PATCH 4/4] fix(auth): harden the delegated refresh against review findings Apply the seven code-review findings on the delegated-credential refresh: - Hold a cross-process file lock (.refresh-lock, network-sized budgets) across the whole read/exchange/write, so two processes never spend the same refresh token; the loser re-reads the rotated pair inside the lock. - Persist a successfully rotated pair before refusing a short-lived access token, so a consumed refresh token is never left on disk. - Validate the spawn-time read with minimumValidityMs 0: only an already-expired token is refused or refreshed after the handler's pre-spawn work; the near-expiry policy runs at preflight only. - Settle a needs-phase throw via settleThrown, so Ctrl-C during the preflight exchange is a signal settlement, not CLI.INTERNAL_ERROR. - Preserve the stored expiresAt when a one-argument (SDK-driven) setTokens rotates to a token with no exp claim. - Make ActiveAccessTokenOptions required on activeAccessToken. - Inline prepareChildCredential into checkCredentials, dropping the dead manager guard and the redundant credential re-read. Co-Authored-By: Claude Fable 5 Signed-off-by: willbot Signed-off-by: Will Madden --- .../assets/engine/engine-interface-draft.ts | 9 +-- .../cli-engine/src/active-access-token.ts | 14 ++-- packages/cli-engine/src/credential-manager.ts | 11 +-- .../src/environment-credential-manager.ts | 7 +- packages/cli-engine/src/execution/engine.ts | 7 +- packages/cli-engine/src/execution/needs.ts | 33 +++------ packages/cli-engine/src/execution/spawn.ts | 13 ++-- .../src/in-memory-credential-manager.ts | 12 +++- .../tests/credential-manager.test.ts | 14 +++- .../environment-credential-manager.test.ts | 10 ++- packages/cli-engine/tests/spawn.test.ts | 58 ++++++++++++++- packages/cli/src/auth/credential-manager.ts | 27 +++++-- packages/cli/src/auth/state-file.ts | 71 ++++++++++++++++--- .../credential-manager-processes.test.ts | 18 +++-- packages/cli/tests/credential-manager.test.ts | 9 ++- 15 files changed, 229 insertions(+), 84 deletions(-) diff --git a/.drive/projects/prisma-cli-v8/assets/engine/engine-interface-draft.ts b/.drive/projects/prisma-cli-v8/assets/engine/engine-interface-draft.ts index befb6ec4..d8393f93 100644 --- a/.drive/projects/prisma-cli-v8/assets/engine/engine-interface-draft.ts +++ b/.drive/projects/prisma-cli-v8/assets/engine/engine-interface-draft.ts @@ -635,11 +635,12 @@ export interface CredentialManager { * credential's ACCESS token, read fresh, for handing to a child * process that authenticates as this process does. Never the * refresh token — the child gets a snapshot it cannot refresh. - * With options, refreshes a near-expiry stored OAuth pair before - * returning its access token. Preflight and ctx.spawn both use the - * options form so the spawn-time fresh read is also validated. The + * Refreshes a stored OAuth pair inside the caller's minimum + * validity before returning its access token. Preflight applies the + * near-expiry window; ctx.spawn's fresh read refuses only an + * already-expired token, so a run is never refused mid-handler. The * refresh token never reaches the child. */ - activeAccessToken(options?: ActiveAccessTokenOptions): Promise + activeAccessToken(options: ActiveAccessTokenOptions): Promise } /** The SDK's typed client and token-storage contract, re-exported by diff --git a/packages/cli-engine/src/active-access-token.ts b/packages/cli-engine/src/active-access-token.ts index dbaa9cd0..bb15564a 100644 --- a/packages/cli-engine/src/active-access-token.ts +++ b/packages/cli-engine/src/active-access-token.ts @@ -13,11 +13,8 @@ type Tokens = NonNullable>>; export async function readActiveAccessToken( storage: TokenStorage, refreshCredential: CredentialRefresher | undefined, - options?: ActiveAccessTokenOptions, + options: ActiveAccessTokenOptions, ): Promise { - if (options === undefined) { - return (await storage.getTokens())?.accessToken ?? null; - } const runLocked = storage.withRefreshLock ?? (async (fn) => fn()); try { return await runLocked(async () => { @@ -44,9 +41,9 @@ export async function readActiveAccessToken( await clearCurrentTokens(storage, current); throw credentialsRequiredError("expired"); } - if (expiresSoon(refreshed.accessToken, options, refreshed.expiresAt)) { - throw new Error("the OAuth endpoint returned a short-lived token"); - } + // Persist before judging the new pair: the server has already + // invalidated the old refresh token, so discarding the rotation + // would strand a dead refresh token in storage. await storage.setTokens( { workspaceId: current.workspaceId, @@ -55,6 +52,9 @@ export async function readActiveAccessToken( }, refreshed.expiresAt, ); + if (expiresSoon(refreshed.accessToken, options, refreshed.expiresAt)) { + throw new Error("the OAuth endpoint returned a short-lived token"); + } return refreshed.accessToken; }); } catch (cause) { diff --git a/packages/cli-engine/src/credential-manager.ts b/packages/cli-engine/src/credential-manager.ts index a29b9f29..1405b200 100644 --- a/packages/cli-engine/src/credential-manager.ts +++ b/packages/cli-engine/src/credential-manager.ts @@ -149,10 +149,11 @@ export interface CredentialManager { /** * ENGINE-FACING. The active credential's ACCESS token, read fresh on * every call, for handing to a child process that authenticates as - * this process does. With options, a near-expiry OAuth pair is refreshed - * under the storage lock before its access token is returned. Never the - * refresh token: the child gets a snapshot it cannot refresh. Null when - * the material is gone (the session ended). + * this process does. An OAuth pair inside the caller's minimum + * validity is refreshed under the refresh lock before its access + * token is returned. Never the refresh token: the child gets a + * snapshot it cannot refresh. Null when the material is gone (the + * session ended). */ - activeAccessToken(options?: ActiveAccessTokenOptions): Promise; + activeAccessToken(options: ActiveAccessTokenOptions): Promise; } diff --git a/packages/cli-engine/src/environment-credential-manager.ts b/packages/cli-engine/src/environment-credential-manager.ts index 178db8fa..dedd1db2 100644 --- a/packages/cli-engine/src/environment-credential-manager.ts +++ b/packages/cli-engine/src/environment-credential-manager.ts @@ -103,7 +103,7 @@ export class EnvironmentCredentialManager implements CredentialManager { /** The delegated path's read: the env token passes through directly. It * is already a snapshot with no refresh token behind it. */ async activeAccessToken( - options?: ActiveAccessTokenOptions, + options: ActiveAccessTokenOptions, ): Promise { const credential = await this.activeCredential(); if (credential === null) return null; @@ -142,7 +142,10 @@ export class EnvironmentCredentialManager implements CredentialManager { setTokens: async (rotated, expiresAt) => { tokens = { ...rotated, - expiresAt: claimedExpiresAt(rotated.accessToken) ?? expiresAt, + expiresAt: + claimedExpiresAt(rotated.accessToken) ?? + expiresAt ?? + tokens?.expiresAt, }; }, clearTokens: async () => { diff --git a/packages/cli-engine/src/execution/engine.ts b/packages/cli-engine/src/execution/engine.ts index fec829e5..ad4d11a9 100644 --- a/packages/cli-engine/src/execution/engine.ts +++ b/packages/cli-engine/src/execution/engine.ts @@ -637,7 +637,10 @@ export class EngineImpl implements Engine { } needsOutcome = await checkNeeds(entry.def, invocation); } catch (cause) { - settleBug(invocation, cause); + // The child preflight can be awaiting the token endpoint when the + // user interrupts; settleThrown keeps an abort a signal settlement + // rather than reporting it as an engine bug. + settleThrown(invocation, cause); return; } if (needsOutcome.kind === "errored") { @@ -719,7 +722,7 @@ export class EngineImpl implements Engine { try { needsOutcome = await checkNeeds(entry.def, invocation); } catch (cause) { - settleBug(invocation, cause); + settleThrown(invocation, cause); return; } if (needsOutcome.kind === "errored") { diff --git a/packages/cli-engine/src/execution/needs.ts b/packages/cli-engine/src/execution/needs.ts index 2e5b5fc4..8bfec8a3 100644 --- a/packages/cli-engine/src/execution/needs.ts +++ b/packages/cli-engine/src/execution/needs.ts @@ -181,9 +181,17 @@ async function checkCredentials( return {}; } try { - return { - spawnCredential: await prepareChildCredential(invocation), - }; + const accessToken = await manager.activeAccessToken({ + minimumValidityMs: CREDENTIAL_NEAR_EXPIRY_MS, + now: invocation.now(), + signal: invocation.signal, + }); + if (accessToken === null) { + return { + failure: needsErrored(credentialsRequiredError("session-ended")), + }; + } + return { spawnCredential: credential }; } catch (cause) { if (CliStructuredError.is(cause)) { return { failure: needsErrored(cause) }; @@ -192,25 +200,6 @@ async function checkCredentials( } } -async function prepareChildCredential( - invocation: Invocation, -): Promise { - const manager = invocation.runtime.credentialManager; - if (manager === undefined) throw credentialsRequiredError(); - const accessToken = await manager.activeAccessToken({ - minimumValidityMs: CREDENTIAL_NEAR_EXPIRY_MS, - now: invocation.now(), - signal: invocation.signal, - }); - if (accessToken === null) throw credentialsRequiredError("session-ended"); - - const refreshedCredential = await manager.activeCredential(); - if (refreshedCredential === null) { - throw credentialsRequiredError("session-ended"); - } - return refreshedCredential; -} - /** * A top-level key in the config file that is not one of the sections * the mounted commands and command families declare. The set is closed, diff --git a/packages/cli-engine/src/execution/spawn.ts b/packages/cli-engine/src/execution/spawn.ts index 2ccc9699..3244ebf6 100644 --- a/packages/cli-engine/src/execution/spawn.ts +++ b/packages/cli-engine/src/execution/spawn.ts @@ -16,7 +16,6 @@ import type { import { constructionError } from "./command-tree"; import { makeDebugLog } from "./debug"; import type { Invocation } from "./engine"; -import { CREDENTIAL_NEAR_EXPIRY_MS } from "./needs"; import { firstLine } from "./rendering"; import { flushBufferedEvents } from "./reporting"; @@ -307,10 +306,12 @@ async function composeChildEnv( /** * The child's copy of the credential: the manager's activeAccessToken() - * operation, validated again at spawn time. This preserves the fresh re-read - * while ensuring a token changed after preflight still satisfies the child's - * minimum lifetime. The refresh token is never injected: the child runs on a - * snapshot it cannot refresh. + * operation, read fresh at spawn time. The near-expiry policy already ran + * at preflight, before the handler; the spawn-time read refuses only a + * token that is already expired, so a still-valid credential can never be + * refused after the handler's pre-spawn work has created resources. The + * refresh token is never injected: the child runs on a snapshot it cannot + * refresh. */ async function spawnToken(invocation: Invocation): Promise { const manager = invocation.runtime.credentialManager; @@ -318,7 +319,7 @@ async function spawnToken(invocation: Invocation): Promise { throw credentialsRequiredError(); } const accessToken = await manager.activeAccessToken({ - minimumValidityMs: CREDENTIAL_NEAR_EXPIRY_MS, + minimumValidityMs: 0, now: invocation.now(), signal: invocation.signal, }); diff --git a/packages/cli-engine/src/in-memory-credential-manager.ts b/packages/cli-engine/src/in-memory-credential-manager.ts index 811afcb0..81b973c0 100644 --- a/packages/cli-engine/src/in-memory-credential-manager.ts +++ b/packages/cli-engine/src/in-memory-credential-manager.ts @@ -132,7 +132,10 @@ function memoryBackedStorage( setTokens: async (rotated, expiresAt) => { tokens = { ...rotated, - expiresAt: claimedExpiresAt(rotated.accessToken) ?? expiresAt, + expiresAt: + claimedExpiresAt(rotated.accessToken) ?? + expiresAt ?? + tokens?.expiresAt, }; }, clearTokens: async () => { @@ -277,7 +280,7 @@ export class InMemoryCredentialManager implements CredentialManager { * no active credential to read — storage exists only once * activeCredential() has returned non-null. */ async activeAccessToken( - options?: ActiveAccessTokenOptions, + options: ActiveAccessTokenOptions, ): Promise { const credential = await this.activeCredential(); if (credential === null) { @@ -339,7 +342,10 @@ export class InMemoryCredentialManager implements CredentialManager { credential: { token: tokens.accessToken, refreshToken: tokens.refreshToken, - expiresAt: claimedExpiresAt(tokens.accessToken) ?? expiresAt, + expiresAt: + claimedExpiresAt(tokens.accessToken) ?? + expiresAt ?? + stored.credential.expiresAt, }, } : stored, diff --git a/packages/cli-engine/tests/credential-manager.test.ts b/packages/cli-engine/tests/credential-manager.test.ts index 9d05c646..d7050a88 100644 --- a/packages/cli-engine/tests/credential-manager.test.ts +++ b/packages/cli-engine/tests/credential-manager.test.ts @@ -65,6 +65,12 @@ const environmentCredentialFor = (claims: { expiresAt: undefined, }); +const READ_OPTIONS = { + minimumValidityMs: 0, + now: new Date(0), + signal: new AbortController().signal, +}; + const credentialReader = () => { let seen: ActiveCredential | null | undefined; const command = defineCommand({ @@ -715,7 +721,7 @@ describe("activeAccessToken, the spawn path's read", () => { test("an unseeded manager has no token to give and returns null", async () => { const manager = new InMemoryCredentialManager({}); - expect(await manager.activeAccessToken()).toBeNull(); + expect(await manager.activeAccessToken(READ_OPTIONS)).toBeNull(); }); test("no token is available after the last session ends", async () => { @@ -725,7 +731,7 @@ describe("activeAccessToken, the spawn path's read", () => { }); await manager.endAllSessions(); - expect(await manager.activeAccessToken()).toBeNull(); + expect(await manager.activeAccessToken(READ_OPTIONS)).toBeNull(); }); test("a selected session's access token is what the child would get", async () => { @@ -735,7 +741,9 @@ describe("activeAccessToken, the spawn path's read", () => { selectedWorkspaceId: "workspace-1", }); - expect(await manager.activeAccessToken()).toBe(record.credential.token); + expect(await manager.activeAccessToken(READ_OPTIONS)).toBe( + record.credential.token, + ); }); }); diff --git a/packages/cli-engine/tests/environment-credential-manager.test.ts b/packages/cli-engine/tests/environment-credential-manager.test.ts index c0f70b11..29c4bbc6 100644 --- a/packages/cli-engine/tests/environment-credential-manager.test.ts +++ b/packages/cli-engine/tests/environment-credential-manager.test.ts @@ -21,6 +21,12 @@ const CLAIMED_TOKEN = mintTestJwt({ }); const CLAIMLESS_TOKEN = mintTestJwt({ sub: "user_1" }); +const READ_OPTIONS = { + minimumValidityMs: 0, + now: new Date(0), + signal: new AbortController().signal, +}; + describe("composition from the environment", () => { test("token and claimed workspace compose the active credential", async () => { const manager = new EnvironmentCredentialManager({ @@ -58,7 +64,7 @@ describe("composition from the environment", () => { const manager = new EnvironmentCredentialManager({ env: {} }); expect(await manager.activeCredential()).toBeNull(); - expect(await manager.activeAccessToken()).toBeNull(); + expect(await manager.activeAccessToken(READ_OPTIONS)).toBeNull(); expect(await manager.sessions()).toEqual({ sessions: [], selectedWorkspaceId: undefined, @@ -111,7 +117,7 @@ describe("the engine-facing reads", () => { env: { PRISMA_SERVICE_TOKEN: CLAIMED_TOKEN }, }); - expect(await manager.activeAccessToken()).toBe(CLAIMED_TOKEN); + expect(await manager.activeAccessToken(READ_OPTIONS)).toBe(CLAIMED_TOKEN); }); test("the storage is memory-backed and carries no refresh token", async () => { diff --git a/packages/cli-engine/tests/spawn.test.ts b/packages/cli-engine/tests/spawn.test.ts index f9daaf2d..95ed001a 100644 --- a/packages/cli-engine/tests/spawn.test.ts +++ b/packages/cli-engine/tests/spawn.test.ts @@ -742,7 +742,10 @@ describe("credential injection", () => { expect(Object.values(seen)).not.toContain("refresh-material"); }); - test("a token replaced after preflight is validated again at spawn time", async () => { + // The spawn-time read refuses only an already-expired token: the + // near-expiry policy ran at preflight, and a still-valid credential + // must never be refused after the handler's pre-spawn work. + test("a token expired by spawn time is refused", async () => { const reportThenSpawn = defineCommand({ help: { summary: "Reports, then hands credentials to a child" }, maySpawn: true, @@ -772,7 +775,7 @@ describe("credential injection", () => { workspaceId: "ws_1", workspaceName: undefined, credential: { - token: jwtExpiringIn(60, "ws_1"), + token: jwtExpiringIn(-60, "ws_1"), refreshToken: undefined, expiresAt: undefined, }, @@ -788,6 +791,57 @@ describe("credential injection", () => { expect(result.spawns).toEqual([]); }); + test("a token that drifts merely near expiry mid-handler still spawns", async () => { + let seen: Readonly> = {}; + const nearExpiryToken = jwtExpiringIn(60, "ws_1"); + const reportThenSpawn = defineCommand({ + help: { summary: "Reports, then hands credentials to a child" }, + maySpawn: true, + needs: { credentials: "child" }, + handler: async (_args, ctx) => { + ctx.report({ kind: "status", subject: "run", status: "pre-spawn" }); + await ctx.spawn({ command: "alchemy" }); + return ok(exitWithChildStatus()); + }, + }); + const cli = createTestCli({ + commands: { converge: reportThenSpawn }, + now: CLOCK, + credential: { + token: jwtExpiringIn(3_600, "ws_1"), + refreshToken: undefined, + expiresAt: undefined, + }, + spawnScript: (request) => { + seen = request.env; + return { exitCode: 0, signal: null }; + }, + }); + + const result = await cli.run(["converge"], { + onEvent: (event) => { + if (event.kind === "status") { + cli.credentialManager.overwriteStoredState({ + sessions: [ + { + workspaceId: "ws_1", + workspaceName: undefined, + credential: { + token: nearExpiryToken, + refreshToken: undefined, + expiresAt: undefined, + }, + }, + ], + }); + } + }, + }); + + expect(result.exitCode).toBe(0); + expect(seen.PRISMA_SERVICE_TOKEN).toBe(nearExpiryToken); + }); + test("an environment credential's token is injected unchanged", async () => { let seen: Readonly> = {}; const token = jwtExpiringIn(3600, "ws_env"); diff --git a/packages/cli/src/auth/credential-manager.ts b/packages/cli/src/auth/credential-manager.ts index 7ab21a25..647d09e8 100644 --- a/packages/cli/src/auth/credential-manager.ts +++ b/packages/cli/src/auth/credential-manager.ts @@ -28,6 +28,7 @@ import { readCredentialState, resolveStateFilePath, type StoredSession, + withRefreshFileLock, withStateLock, writeCredentialState, } from "./state-file"; @@ -90,7 +91,10 @@ function memoryBackedStorage( setTokens: async (rotated, expiresAt) => { tokens = { ...rotated, - expiresAt: claimedExpiresAt(rotated.accessToken) ?? expiresAt, + expiresAt: + claimedExpiresAt(rotated.accessToken) ?? + expiresAt ?? + tokens?.expiresAt, }; }, clearTokens: async () => { @@ -280,7 +284,7 @@ export class FileCredentialManager implements CredentialManager { * no active credential to read — storage exists only once * activeCredential() has returned non-null. */ async activeAccessToken( - options?: ActiveAccessTokenOptions, + options: ActiveAccessTokenOptions, ): Promise { const credential = await this.activeCredential(); if (credential === null) { @@ -358,7 +362,15 @@ export class FileCredentialManager implements CredentialManager { ...(tokens.refreshToken === undefined ? {} : { refreshToken: tokens.refreshToken }), - ...expiresAtSlice(tokens.accessToken, expiresAt), + // An SDK-driven rotation passes no expiry; keep the record's + // rather than erase the one the proactive refresher stored. + ...expiresAtSlice( + tokens.accessToken, + expiresAt ?? + (record.expiresAt === undefined + ? undefined + : new Date(record.expiresAt)), + ), }; return { state: { @@ -401,7 +413,14 @@ export class FileCredentialManager implements CredentialManager { }); }, - withRefreshLock: (fn) => this.#withRefreshLock(fn), + // The whole read → exchange → write sequence holds the + // cross-process refresh lock, so two processes never spend the + // same refresh token; the in-process chain serialises callers + // within this process first. + withRefreshLock: (fn) => + this.#withRefreshLock(() => + withRefreshFileLock(this.#filePath, this.#debug, fn), + ), }; } diff --git a/packages/cli/src/auth/state-file.ts b/packages/cli/src/auth/state-file.ts index 209a066f..b8fcd6d0 100644 --- a/packages/cli/src/auth/state-file.ts +++ b/packages/cli/src/auth/state-file.ts @@ -14,6 +14,29 @@ const FILE_MODE = 0o600; const LOCK_STALE_MS = 5_000; const LOCK_RETRY_MS = 10; const LOCK_WAIT_TIMEOUT_MS = 10_000; +// The refresh lock is held across the token-endpoint exchange (a +// network call bounded at 10s), so its budgets are network-sized. +const REFRESH_LOCK_STALE_MS = 30_000; +const REFRESH_LOCK_RETRY_MS = 100; +const REFRESH_LOCK_WAIT_TIMEOUT_MS = 30_000; + +interface LockTimings { + readonly staleMs: number; + readonly retryMs: number; + readonly waitTimeoutMs: number; +} + +const STATE_LOCK_TIMINGS: LockTimings = { + staleMs: LOCK_STALE_MS, + retryMs: LOCK_RETRY_MS, + waitTimeoutMs: LOCK_WAIT_TIMEOUT_MS, +}; + +const REFRESH_LOCK_TIMINGS: LockTimings = { + staleMs: REFRESH_LOCK_STALE_MS, + retryMs: REFRESH_LOCK_RETRY_MS, + waitTimeoutMs: REFRESH_LOCK_WAIT_TIMEOUT_MS, +}; export interface StoredSession { readonly workspaceId: string; @@ -193,12 +216,12 @@ export async function writeCredentialState( } class StateLockTimeoutError extends CliStructuredError { - constructor(lockPath: string) { + constructor(lockPath: string, waitTimeoutMs: number) { super( "CLI.CREDENTIALS_LOCKED", "Another prisma process is still updating your credentials.", { - why: `The credentials lock at ${lockPath} was held for longer than ${LOCK_WAIT_TIMEOUT_MS}ms.`, + why: `The credentials lock at ${lockPath} was held for longer than ${waitTimeoutMs}ms.`, nextActions: [ { kind: "user-choice", @@ -222,8 +245,36 @@ export async function withStateLock( debug: DebugLog, run: () => Promise, ): Promise { - const lockPath = `${filePath}.lock`; - const lockId = await acquireStateLock(lockPath, debug); + return withFileLock(`${filePath}.lock`, debug, STATE_LOCK_TIMINGS, run); +} + +/** + * The cross-process lock the delegated refresh holds for its whole + * read → exchange → write sequence, so two processes never spend the + * same refresh token. Distinct from the state lock: it IS held across + * network I/O, so its staleness and wait budgets are larger, and it + * uses its own lock path so short mutations are not queued behind it. + */ +export async function withRefreshFileLock( + filePath: string, + debug: DebugLog, + run: () => Promise, +): Promise { + return withFileLock( + `${filePath}.refresh-lock`, + debug, + REFRESH_LOCK_TIMINGS, + run, + ); +} + +async function withFileLock( + lockPath: string, + debug: DebugLog, + timings: LockTimings, + run: () => Promise, +): Promise { + const lockId = await acquireStateLock(lockPath, debug, timings); debug(`lock acquired ${lockPath}`); try { return await run(); @@ -236,6 +287,7 @@ export async function withStateLock( async function acquireStateLock( lockPath: string, debug: DebugLog, + timings: LockTimings, ): Promise { const lockId = randomUUID(); const startedAt = Date.now(); @@ -244,15 +296,15 @@ async function acquireStateLock( while (true) { if (await tryCreateStateLock(lockPath, lockId)) return lockId; - const tookOver = await takeOverStaleStateLock(lockPath, debug); + const tookOver = await takeOverStaleStateLock(lockPath, debug, timings); // The timeout is checked on every pass, including the ones that // took a lock over: a takeover that keeps appearing to succeed // must still end in a timeout rather than spinning. - if (Date.now() - startedAt >= LOCK_WAIT_TIMEOUT_MS) { - throw new StateLockTimeoutError(lockPath); + if (Date.now() - startedAt >= timings.waitTimeoutMs) { + throw new StateLockTimeoutError(lockPath, timings.waitTimeoutMs); } if (!tookOver) { - await new Promise((resolve) => setTimeout(resolve, LOCK_RETRY_MS)); + await new Promise((resolve) => setTimeout(resolve, timings.retryMs)); } } } @@ -287,10 +339,11 @@ async function tryCreateStateLock( async function takeOverStaleStateLock( lockPath: string, debug: DebugLog, + timings: LockTimings, ): Promise { const stale = await fs.stat(lockPath).catch(() => null); if (stale === null) return true; - if (Date.now() - stale.mtimeMs <= LOCK_STALE_MS) return false; + if (Date.now() - stale.mtimeMs <= timings.staleMs) return false; const takenPath = `${lockPath}.${randomUUID()}.stale`; try { diff --git a/packages/cli/tests/credential-manager-processes.test.ts b/packages/cli/tests/credential-manager-processes.test.ts index 49d9c061..61ca307a 100644 --- a/packages/cli/tests/credential-manager-processes.test.ts +++ b/packages/cli/tests/credential-manager-processes.test.ts @@ -204,23 +204,21 @@ describe("across processes", () => { ).toEqual([WORKSPACE_A, WORKSPACE_B, WORKSPACE_C]); }, 30_000); - it("leaves a valid pair in the file when two processes really refresh the same session", async () => { + // The refresh lock is a cross-process file lock, so the loser of the + // race re-reads the rotated pair inside the lock and never spends the + // seed refresh token a second time. + it("exchanges one refresh token once when two processes refresh the same session", async () => { const seedAccessToken = mintToken(WORKSPACE_A); const issued = [ { accessToken: mintToken(WORKSPACE_A, "rotated-1"), refreshToken: "refresh-1", }, - { - accessToken: mintToken(WORKSPACE_A, "rotated-2"), - refreshToken: "refresh-2", - }, ]; const endpoint = await startTokenEndpoint({ seedAccessToken, seedRefreshToken: "refresh-0", issued, - concurrentRefreshers: 2, }); await runWorker("create", WORKSPACE_A, seedAccessToken, "refresh-0"); @@ -233,13 +231,13 @@ describe("across processes", () => { { status: 200 }, { status: 200 }, ]); - expect(endpoint.exchanges()).toBe(2); + expect(endpoint.exchanges()).toBe(1); const state = await readCredentialState(stateFilePath); expect(state.sessions).toHaveLength(1); const record = state.sessions[0]; - expect( - issued.map((pair) => `${pair.accessToken}|${pair.refreshToken}`), - ).toContain(`${record.token}|${record.refreshToken}`); + expect(`${record.token}|${record.refreshToken}`).toBe( + `${issued[0].accessToken}|${issued[0].refreshToken}`, + ); }, 30_000); it("takes over a crashed holder's lock after the stale threshold", async () => { diff --git a/packages/cli/tests/credential-manager.test.ts b/packages/cli/tests/credential-manager.test.ts index 17480b06..13c7f292 100644 --- a/packages/cli/tests/credential-manager.test.ts +++ b/packages/cli/tests/credential-manager.test.ts @@ -1117,7 +1117,10 @@ describe("delegated access-token preparation", () => { }); }); - it("rejects and does not persist a rotated token that expires too soon", async () => { + // The exchange has already consumed refresh-1 server-side, so the + // rotated pair must be persisted even when its access token is + // refused: keeping the old pair would strand a dead refresh token. + it("persists a short-lived rotated pair before refusing it", async () => { const manager = makeManager({ refreshCredential: async () => ({ kind: "success", @@ -1141,8 +1144,8 @@ describe("delegated access-token preparation", () => { expect( (await readCredentialState(stateFilePath)).sessions[0], ).toMatchObject({ - token: expiringToken, - refreshToken: "refresh-1", + token: "short-lived-opaque-token", + refreshToken: "refresh-2", }); });