diff --git a/apps/cli/src/legacy/auth/legacy-credentials.layer.ts b/apps/cli/src/legacy/auth/legacy-credentials.layer.ts index 2d78ea2ec8..ffddf8b017 100644 --- a/apps/cli/src/legacy/auth/legacy-credentials.layer.ts +++ b/apps/cli/src/legacy/auth/legacy-credentials.layer.ts @@ -1,4 +1,4 @@ -import { Effect, FileSystem, Layer, Option, Path, Redacted } from "effect"; +import { Effect, FileSystem, Layer, Option, Path, Redacted, Result } from "effect"; import { RuntimeInfo } from "../../shared/runtime/runtime-info.service.ts"; import { normalizeKeyringToken } from "../../shared/auth/keyring-token.ts"; @@ -92,7 +92,7 @@ const tryKeyringDelete = ( deleted = true; } - if (platform === "win32" && readGoWindowsTarget(module, account)) { + if (platform === "win32" && probeWindowsTarget(module, account) !== "absent") { deleted = deleteGoWindowsTarget(module, account) || deleted; } @@ -123,6 +123,45 @@ function readGoWindowsTarget(module: KeyringModule, account: string): string | n } } +// `Entry.withTarget` is avoided as a probe — its constructor writes an empty +// placeholder. A `findCredentials` throw is ambiguous (an undecodable Go blob or +// a real enumeration failure), so reported as `"unknown"`, never assumed present. +type WindowsTargetProbe = "present" | "absent" | "unknown"; + +function probeWindowsTarget(module: KeyringModule, account: string): WindowsTargetProbe { + try { + const credentials = module.findCredentials(KEYRING_SERVICE, goWindowsCredentialTarget(account)); + // An empty password is an orphaned placeholder, not a real credential. + const credential = credentials.find( + (item) => item.account === account && item.password.length > 0, + ); + return credential ? "present" : "absent"; + } catch { + return "unknown"; + } +} + +// Constructing `withTarget` always leaves something at that target — the real +// credential, or the placeholder — so a delete that follows is never a +// legitimate "nothing to delete": any non-success is surfaced. +const deleteProbedWindowsTarget = ( + module: KeyringModule, + account: string, + onFailure: (cause: unknown) => E, +): Effect.Effect => + Effect.gen(function* () { + const result = yield* Effect.try(() => + module.Entry.withTarget( + goWindowsCredentialTarget(account), + KEYRING_SERVICE, + account, + ).deleteCredential(), + ).pipe(Effect.result); + if (Result.isSuccess(result) && result.success) return true; + const cause = Result.isFailure(result) ? result.failure : "credential was not removed"; + return yield* Effect.fail(onFailure(cause)); + }); + function normalizeGoWindowsPassword(value: string): string { const direct = normalizeKeyringToken(value); if (LEGACY_ACCESS_TOKEN_PATTERN.test(direct)) return direct; @@ -172,11 +211,10 @@ function deleteGoWindowsTarget(module: KeyringModule, account: string): boolean // (backend unavailable) and only surfaces other errors (`unlink.go:36-40`). // // The plain `Entry(service, projectRef)` is the macOS/Linux form and the Windows -// default. On Windows, Go also writes a separate target-shaped credential; it is -// detected via `findCredentials` (a plain `getPassword` does not read the Go -// target reliably) and deleted through the `withTarget` entry. The `withTarget` -// entry is only constructed on Windows — on macOS its first argument is an -// invalid keychain domain and throws. +// default. On Windows, Go also writes a separate target-shaped credential, +// deleted through the `withTarget` entry. The `withTarget` entry is only +// constructed on Windows — on macOS its first argument is an invalid keychain +// domain and throws. // // Each entry is probed before `deleteCredential()`: on macOS deleting an absent // entry blocks on a Keychain authorization prompt, and an absent read means @@ -204,22 +242,16 @@ const deleteKeyringEntryStrict = ( deleted = true; } - if (platform === "win32" && readGoWindowsTarget(module, account)) { - const target = module.Entry.withTarget( - goWindowsCredentialTarget(account), - KEYRING_SERVICE, + if (platform === "win32" && probeWindowsTarget(module, account) !== "absent") { + const removed = yield* deleteProbedWindowsTarget( + module, account, - ); - yield* Effect.try({ - try: () => { - target.deleteCredential(); - }, - catch: (cause) => + (cause) => new LegacyCredentialDeleteError({ message: `failed to delete project credential: ${String(cause)}`, }), - }); - deleted = true; + ); + deleted ||= removed; } return deleted; @@ -256,43 +288,48 @@ const deleteProfileKeyringEntry = ( found = true; } - if (platform === "win32" && readGoWindowsTarget(module, account)) { - const target = module.Entry.withTarget( - goWindowsCredentialTarget(account), - KEYRING_SERVICE, + if (platform === "win32" && probeWindowsTarget(module, account) !== "absent") { + const removed = yield* deleteProbedWindowsTarget( + module, account, - ); - yield* Effect.try({ - try: () => { - target.deleteCredential(); - }, - catch: (cause) => + (cause) => new LegacyDeleteTokenError({ message: `failed to delete access token from keyring: ${String(cause)}`, }), - }); - found = true; + ); + found ||= removed; } return found ? "deleted" : "notFound"; }); -// Best-effort wipe of every entry in the `"Supabase CLI"` keyring namespace — -// the project database-password credentials `link` writes. Mirrors Go's -// `keyring.DeleteAll(namespace)` (`store.go:71`). Never fails: per-entry delete -// errors are swallowed so a single stuck credential can't abort logout. +// Best-effort wipe of the `"Supabase CLI"` keyring namespace, mirroring Go's +// `keyring.DeleteAll` (`store.go:71`); errors are swallowed so one stuck +// credential can't abort logout, matching Go's own handling (`logout.go:35`). // -// On Windows, Go stores credentials under the target-shaped name -// `Supabase CLI:` rather than the plain `Entry(service, account)` form -// (see `writeGoWindowsTarget`). So each discovered account is deleted in BOTH -// forms — the plain entry and, on win32, the Go target entry — mirroring the -// individual deletes in `deleteProfileKeyringEntry`. Without this, a Go-written -// project credential would survive `logout` on Windows. +// Go writes Windows credentials under the target-shaped form +// (`Supabase CLI:`), invisible to the plain enumeration below, so +// it's swept separately. `findCredentials` decodes every matched blob and +// aborts entirely on one undecodable entry, unlike Go's raw-byte `wincred.List`. const deleteAllKeyringEntries = ( module: KeyringModule, platform: RuntimePlatform, ): Effect.Effect => Effect.sync(() => { + if (platform === "win32") { + try { + const entries = module.findCredentials( + KEYRING_SERVICE, + `${goWindowsCredentialTarget("")}*`, + ); + for (const { account } of entries) { + deleteGoWindowsTarget(module, account); + } + } catch { + // best-effort + } + } + let entries: ReadonlyArray<{ account: string }>; try { entries = module.findCredentials(KEYRING_SERVICE); @@ -305,9 +342,6 @@ const deleteAllKeyringEntries = ( } catch { // best-effort per entry } - if (platform === "win32" && readGoWindowsTarget(module, account)) { - deleteGoWindowsTarget(module, account); - } } }); diff --git a/apps/cli/src/legacy/auth/legacy-credentials.layer.unit.test.ts b/apps/cli/src/legacy/auth/legacy-credentials.layer.unit.test.ts index 06dad317de..f5320dba28 100644 --- a/apps/cli/src/legacy/auth/legacy-credentials.layer.unit.test.ts +++ b/apps/cli/src/legacy/auth/legacy-credentials.layer.unit.test.ts @@ -32,24 +32,29 @@ let throwOnSetPassword = false; let throwOnSetSecret = false; const throwOnGetPasswordAccounts = new Set(); const throwOnDeleteAccounts = new Set(); +const failDeleteAccounts = new Set(); const withTargetCalls: string[] = []; +const opaqueAccounts = new Set(); +let throwOnFindCredentials = false; vi.mock("@napi-rs/keyring", () => ({ - findCredentials: (service: string, target?: string) => - Array.from(passwords.entries()) - .filter(([key]) => - // No target → model the Windows `CredEnumerate("*")` sweep, - // which matches both the plain (`/…`) and the Go target-shaped - // (`:/…`) entries by the leading segment. With a - // target → narrow to that specific Go target (used by readGoWindowsTarget). - target === undefined - ? key.split("/")[0]!.startsWith(service) - : key.startsWith(`${target}/`), - ) - .map(([key, password]) => ({ - account: key.split("/").at(-1)!, - password, - })), + findCredentials: (service: string, target?: string) => { + if (throwOnFindCredentials) throw new Error("CredEnumerate failed"); + const targetName = (key: string) => key.split("/")[0]!; + const matchesFilter = (key: string): boolean => { + if (target === undefined) return targetName(key) === service; + if (target.endsWith("*")) return targetName(key).startsWith(target.slice(0, -1)); + return targetName(key) === target; + }; + const matches = Array.from(passwords.entries()).filter(([key]) => matchesFilter(key)); + if (matches.some(([key]) => opaqueAccounts.has(key))) { + throw new Error("BadEncoding: invalid UTF-8 data in secure storage"); + } + return matches.map(([key, password]) => ({ + account: key.split("/").at(-1)!, + password, + })); + }, Entry: class Entry { service: string; account: string; @@ -61,7 +66,9 @@ vi.mock("@napi-rs/keyring", () => ({ } static withTarget(target: string, service: string, account: string) { withTargetCalls.push(`${target}/${service}/${account}`); - return new this(service, account, target); + const entry = new this(service, account, target); + if (!passwords.has(entry.key())) passwords.set(entry.key(), ""); + return entry; } key(): string { return this.target === undefined @@ -86,6 +93,7 @@ vi.mock("@napi-rs/keyring", () => ({ deleteCredential(): boolean { const key = this.key(); if (throwOnDeleteAccounts.has(key)) throw new Error("Keyring delete failed"); + if (failDeleteAccounts.has(key)) return false; if (!passwords.has(key)) throw new Error("not found"); passwords.delete(key); return true; @@ -140,6 +148,9 @@ beforeEach(() => { throwOnGetPasswordAccounts.clear(); throwOnDeleteAccounts.clear(); withTargetCalls.length = 0; + opaqueAccounts.clear(); + failDeleteAccounts.clear(); + throwOnFindCredentials = false; tempHome = mkdtempSync(join(tmpdir(), "supabase-legacy-creds-")); }); @@ -337,7 +348,7 @@ describe("legacyCredentialsLayer.saveAccessToken", () => { return Effect.gen(function* () { const { saveAccessToken } = yield* LegacyCredentials; yield* saveAccessToken(VALID_TOKEN); - expect(passwords.has(goWindowsKey("supabase"))).toBe(false); + expect(passwords.get(goWindowsKey("supabase")) ?? "").toBe(""); expect(passwords.has("Supabase CLI/supabase")).toBe(false); const content = readFileSync(join(tempHome, ".supabase", "access-token"), "utf-8"); expect(content).toBe(VALID_TOKEN); @@ -423,6 +434,31 @@ describe("legacyCredentialsLayer.deleteAccessToken", () => { }, ); + it.effect("win32: no Go target credential → LegacyNotLoggedInError, nothing fabricated", () => { + return Effect.gen(function* () { + const { deleteAccessToken } = yield* LegacyCredentials; + const exit = yield* Effect.exit(deleteAccessToken); + expect(exit._tag).toBe("Failure"); + if (exit._tag === "Failure") { + expect(JSON.stringify(exit.cause)).toContain("LegacyNotLoggedInError"); + } + expect(passwords.has(goWindowsKey("supabase"))).toBe(false); + expect(withTargetCalls).toEqual([]); + }).pipe(Effect.provide(makeLayer({ platform: "win32" }))); + }); + + it.effect("win32: an empty placeholder Go target → LegacyNotLoggedInError", () => { + passwords.set(goWindowsKey("supabase"), ""); + return Effect.gen(function* () { + const { deleteAccessToken } = yield* LegacyCredentials; + const exit = yield* Effect.exit(deleteAccessToken); + expect(exit._tag).toBe("Failure"); + if (exit._tag === "Failure") { + expect(JSON.stringify(exit.cause)).toContain("LegacyNotLoggedInError"); + } + }).pipe(Effect.provide(makeLayer({ platform: "win32" }))); + }); + it.effect( "keyring unavailable (SUPABASE_NO_KEYRING) with token in file → removes file, still NotLoggedIn", () => { @@ -521,6 +557,61 @@ describe("legacyCredentialsLayer.deleteAccessToken", () => { }).pipe(Effect.provide(makeLayer({ platform: "win32" }))); }); + it.effect( + "win32: deletes a Go target whose blob getPassword/findCredentials can't decode", + () => { + passwords.set(goWindowsKey("supabase"), VALID_TOKEN); + opaqueAccounts.add(goWindowsKey("supabase")); + return Effect.gen(function* () { + const { deleteAccessToken } = yield* LegacyCredentials; + yield* deleteAccessToken; + expect(passwords.has(goWindowsKey("supabase"))).toBe(false); + }).pipe(Effect.provide(makeLayer({ platform: "win32" }))); + }, + ); + + it.effect( + "win32: a confirmed Go target whose delete returns false → LegacyDeleteTokenError", + () => { + passwords.set(goWindowsKey("supabase"), VALID_TOKEN); + failDeleteAccounts.add(goWindowsKey("supabase")); + return Effect.gen(function* () { + const { deleteAccessToken } = yield* LegacyCredentials; + const exit = yield* Effect.exit(deleteAccessToken); + expect(exit._tag).toBe("Failure"); + if (exit._tag === "Failure") { + expect(JSON.stringify(exit.cause)).toContain("LegacyDeleteTokenError"); + } + expect(passwords.has(goWindowsKey("supabase"))).toBe(true); + }).pipe(Effect.provide(makeLayer({ platform: "win32" }))); + }, + ); + + it.effect("win32: an enumeration hiccup alone does not block logout", () => { + throwOnFindCredentials = true; + return Effect.gen(function* () { + const { deleteAccessToken } = yield* LegacyCredentials; + const exit = yield* Effect.exit(deleteAccessToken); + expect(exit._tag).toBe("Success"); + }).pipe(Effect.provide(makeLayer({ platform: "win32" }))); + }); + + it.effect( + "win32: an enumeration failure whose target also fails to delete → LegacyDeleteTokenError", + () => { + throwOnFindCredentials = true; + failDeleteAccounts.add(goWindowsKey("supabase")); + return Effect.gen(function* () { + const { deleteAccessToken } = yield* LegacyCredentials; + const exit = yield* Effect.exit(deleteAccessToken); + expect(exit._tag).toBe("Failure"); + if (exit._tag === "Failure") { + expect(JSON.stringify(exit.cause)).toContain("LegacyDeleteTokenError"); + } + }).pipe(Effect.provide(makeLayer({ platform: "win32" }))); + }, + ); + it.effect("legacy-keyring delete error is swallowed and does not change the outcome", () => { passwords.set("Supabase CLI/supabase", VALID_TOKEN); passwords.set("Supabase CLI/access-token", VALID_OAUTH_TOKEN); @@ -554,26 +645,71 @@ describe("legacyCredentialsLayer.deleteAllProjectCredentials", () => { }).pipe(Effect.provide(makeLayer({ env: { SUPABASE_NO_KEYRING: "1" } }))); }); + it.effect("never fails even when an individual delete throws", () => { + passwords.set("Supabase CLI/abcdefghijklmnopqrs1", "secret-1"); + throwOnDeleteAccounts.add("Supabase CLI/abcdefghijklmnopqrs1"); + return Effect.gen(function* () { + const { deleteAllProjectCredentials } = yield* LegacyCredentials; + const exit = yield* Effect.exit(deleteAllProjectCredentials); + expect(exit._tag).toBe("Success"); + }).pipe(Effect.provide(makeLayer())); + }); + it.effect("win32: deletes Go target-shaped project credentials", () => { - // Go stores Windows project credentials under `Supabase CLI:`, not the - // plain `Supabase CLI/` form. The sweep must delete the target form too. passwords.set(goWindowsKey("abcdefghijklmnopqrs1"), "secret-1"); + passwords.set(goWindowsKey("abcdefghijklmnopqrs2"), "secret-2"); return Effect.gen(function* () { const { deleteAllProjectCredentials } = yield* LegacyCredentials; yield* deleteAllProjectCredentials; expect(passwords.has(goWindowsKey("abcdefghijklmnopqrs1"))).toBe(false); + expect(passwords.has(goWindowsKey("abcdefghijklmnopqrs2"))).toBe(false); }).pipe(Effect.provide(makeLayer({ platform: "win32" }))); }); - it.effect("never fails even when an individual delete throws", () => { - passwords.set("Supabase CLI/abcdefghijklmnopqrs1", "secret-1"); - throwOnDeleteAccounts.add("Supabase CLI/abcdefghijklmnopqrs1"); + it.effect( + "win32: a single undecodable entry aborts the Go target sweep without failing logout", + () => { + passwords.set(goWindowsKey("abcdefghijklmnopqrs1"), "secret-1"); + passwords.set(goWindowsKey("abcdefghijklmnopqrs2"), "secret-2"); + opaqueAccounts.add(goWindowsKey("abcdefghijklmnopqrs1")); + return Effect.gen(function* () { + const { deleteAllProjectCredentials } = yield* LegacyCredentials; + const exit = yield* Effect.exit(deleteAllProjectCredentials); + expect(exit._tag).toBe("Success"); + // One undecodable entry aborts the whole findCredentials call. + expect(passwords.has(goWindowsKey("abcdefghijklmnopqrs1"))).toBe(true); + expect(passwords.has(goWindowsKey("abcdefghijklmnopqrs2"))).toBe(true); + }).pipe(Effect.provide(makeLayer({ platform: "win32" }))); + }, + ); +}); + +describe("legacyCredentialsLayer.deleteProjectCredential", () => { + it.effect("win32: deletes a Go target-shaped project credential", () => { + passwords.set(goWindowsKey("abcdefghijklmnopqrs1"), "db-password"); return Effect.gen(function* () { - const { deleteAllProjectCredentials } = yield* LegacyCredentials; - const exit = yield* Effect.exit(deleteAllProjectCredentials); - expect(exit._tag).toBe("Success"); - }).pipe(Effect.provide(makeLayer())); + const { deleteProjectCredential } = yield* LegacyCredentials; + const deleted = yield* deleteProjectCredential("abcdefghijklmnopqrs1"); + expect(deleted).toBe(true); + expect(passwords.has(goWindowsKey("abcdefghijklmnopqrs1"))).toBe(false); + }).pipe(Effect.provide(makeLayer({ platform: "win32" }))); }); + + it.effect( + "win32: a confirmed project credential whose delete fails → LegacyCredentialDeleteError", + () => { + passwords.set(goWindowsKey("abcdefghijklmnopqrs1"), "db-password"); + throwOnDeleteAccounts.add(goWindowsKey("abcdefghijklmnopqrs1")); + return Effect.gen(function* () { + const { deleteProjectCredential } = yield* LegacyCredentials; + const exit = yield* Effect.exit(deleteProjectCredential("abcdefghijklmnopqrs1")); + expect(exit._tag).toBe("Failure"); + if (exit._tag === "Failure") { + expect(JSON.stringify(exit.cause)).toContain("LegacyCredentialDeleteError"); + } + }).pipe(Effect.provide(makeLayer({ platform: "win32" }))); + }, + ); }); // Suppress unused-import nag — referenced in JSDoc / used in assertions above.