diff --git a/packages/plugins/onepassword/src/react/OnePasswordSettings.tsx b/packages/plugins/onepassword/src/react/OnePasswordSettings.tsx index a036f2886..59e339aa7 100644 --- a/packages/plugins/onepassword/src/react/OnePasswordSettings.tsx +++ b/packages/plugins/onepassword/src/react/OnePasswordSettings.tsx @@ -41,6 +41,14 @@ import type { RedactedOnePasswordConfig } from "../sdk/types"; // Vault picker // --------------------------------------------------------------------------- +const VAULT_LIST_ERROR_FALLBACK = "Failed to list vaults"; + +const formatVaultListError = (error: Error): string => { + // oxlint-disable-next-line executor/no-unknown-error-message -- boundary: OnePasswordError carries a typed `message` + const message = error.message.trim(); + return message ? `${VAULT_LIST_ERROR_FALLBACK}: ${message}` : VAULT_LIST_ERROR_FALLBACK; +}; + function VaultPicker(props: { authKind: "desktop-app" | "service-account"; accountName: string; @@ -61,15 +69,15 @@ function VaultPicker(props: { isLoading: true, error: null, }), - onError: () => ({ + onError: (queryError) => ({ vaults: [] as { id: string; name: string }[], isLoading: false, - error: "Failed to list vaults", + error: formatVaultListError(queryError), }), onDefect: () => ({ vaults: [] as { id: string; name: string }[], isLoading: false, - error: "Failed to list vaults", + error: VAULT_LIST_ERROR_FALLBACK, }), onSuccess: ({ value }) => { const v = value.vaults; diff --git a/packages/plugins/onepassword/src/sdk/service.test.ts b/packages/plugins/onepassword/src/sdk/service.test.ts new file mode 100644 index 000000000..41c8cd487 --- /dev/null +++ b/packages/plugins/onepassword/src/sdk/service.test.ts @@ -0,0 +1,106 @@ +import { beforeEach, describe, expect, it } from "@effect/vitest"; +import { Effect } from "effect"; +// oxlint-disable-next-line executor/no-vitest-import -- boundary: vi.mock/vi.hoisted must come from vitest itself for mock hoisting to resolve +import { vi } from "vitest"; + +import { OnePasswordError } from "./errors"; +import { makeOnePasswordService } from "./service"; + +const opMocks = vi.hoisted(() => ({ + setGlobalFlags: vi.fn(), + setServiceAccount: vi.fn(), + vaultList: vi.fn(), + itemList: vi.fn(), + readParse: vi.fn(), +})); + +const sdkMocks = vi.hoisted(() => ({ + createClient: vi.fn(), + DesktopAuth: vi.fn((accountName: string) => ({ accountName })), +})); + +vi.mock("@1password/op-js", () => ({ + setGlobalFlags: opMocks.setGlobalFlags, + setServiceAccount: opMocks.setServiceAccount, + vault: { list: opMocks.vaultList }, + item: { list: opMocks.itemList }, + read: { parse: opMocks.readParse }, +})); + +vi.mock("@1password/sdk", () => ({ + createClient: sdkMocks.createClient, + DesktopAuth: sdkMocks.DesktopAuth, +})); + +describe("makeOnePasswordService", () => { + beforeEach(() => { + vi.clearAllMocks(); + opMocks.vaultList.mockReturnValue([]); + opMocks.itemList.mockReturnValue([]); + opMocks.readParse.mockReturnValue("secret"); + sdkMocks.createClient.mockResolvedValue({ + secrets: { resolve: vi.fn(async () => "secret") }, + vaults: { list: vi.fn(async () => []) }, + items: { list: vi.fn(async () => []) }, + }); + }); + + it.effect("falls back to the SDK when the CLI throws while listing vaults", () => + Effect.gen(function* () { + const sdkVaultsList = vi.fn(async () => [{ id: "sdk-vault", title: "SDK Vault" }]); + opMocks.vaultList.mockImplementation(() => { + // oxlint-disable-next-line executor/no-try-catch-or-throw, executor/no-error-constructor -- boundary: simulates the untyped op-js CLI wrapper throwing + throw new Error("spawn op ENOENT"); + }); + sdkMocks.createClient.mockResolvedValue({ + secrets: { resolve: vi.fn(async () => "secret") }, + vaults: { list: sdkVaultsList }, + items: { list: vi.fn(async () => []) }, + }); + + const service = yield* makeOnePasswordService( + { kind: "service-account", token: "ops_test_token" }, + { timeoutMs: 1_000 }, + ); + const vaults = yield* service.listVaults(); + + expect(vaults).toEqual([{ id: "sdk-vault", title: "SDK Vault" }]); + expect(sdkMocks.createClient).toHaveBeenCalledTimes(1); + expect(sdkVaultsList).toHaveBeenCalledTimes(1); + }), + ); + + it.effect("includes the backend cause when both vault listing backends fail", () => + Effect.gen(function* () { + opMocks.vaultList.mockImplementation(() => { + // oxlint-disable-next-line executor/no-try-catch-or-throw, executor/no-error-constructor -- boundary: simulates the untyped op-js CLI wrapper throwing + throw new Error("spawn op ENOENT"); + }); + sdkMocks.createClient.mockResolvedValue({ + secrets: { resolve: vi.fn(async () => "secret") }, + vaults: { + list: vi.fn(async () => { + // oxlint-disable-next-line executor/no-try-catch-or-throw, executor/no-error-constructor -- boundary: simulates the untyped 1Password SDK rejecting + throw new Error("desktop approval refused for account"); + }), + }, + items: { list: vi.fn(async () => []) }, + }); + + const error = yield* makeOnePasswordService( + { kind: "service-account", token: "ops_test_token" }, + { timeoutMs: 1_000 }, + ).pipe( + Effect.flatMap((service) => service.listVaults()), + Effect.flip, + ); + + expect(error).toBeInstanceOf(OnePasswordError); + // oxlint-disable executor/no-unknown-error-message -- boundary: OnePasswordError carries a typed message; asserting its contents + expect(error.message).toContain("1Password SDK vault listing failed:"); + expect(error.message).toContain("desktop approval refused for account"); + expect(error.message).not.toBe("1Password CLI vault listing failed"); + // oxlint-enable executor/no-unknown-error-message + }), + ); +}); diff --git a/packages/plugins/onepassword/src/sdk/service.ts b/packages/plugins/onepassword/src/sdk/service.ts index 3f38d5a42..1e93f6f68 100644 --- a/packages/plugins/onepassword/src/sdk/service.ts +++ b/packages/plugins/onepassword/src/sdk/service.ts @@ -48,15 +48,37 @@ export type ResolvedAuth = // --------------------------------------------------------------------------- const DEFAULT_TIMEOUT_MS = 15_000; +const MAX_ERROR_MESSAGE_LENGTH = 300; +const SERVICE_ACCOUNT_TOKEN_RE = /ops_[A-Za-z0-9_-]+/g; type OnePasswordSdkModule = typeof import("@1password/sdk"); +const formatCause = (cause: unknown): string => { + // oxlint-disable-next-line executor/no-unknown-error-message -- boundary: normalizing untyped op-js/SDK throwables into OnePasswordError.message + const maybeMessage = (cause as { readonly message?: unknown } | null | undefined)?.message; + const raw = + // oxlint-disable-next-line executor/no-unknown-error-message -- boundary: last-resort stringification of a non-Error throwable + typeof maybeMessage === "string" && maybeMessage.length > 0 ? maybeMessage : String(cause); + return raw + .replace(SERVICE_ACCOUNT_TOKEN_RE, "[redacted 1Password token]") + .replace(/\s+/g, " ") + .trim(); +}; + +const messageWithCause = (prefix: string, cause: unknown): string => { + const causeMessage = formatCause(cause); + const message = causeMessage ? `${prefix}: ${causeMessage}` : prefix; + return message.length > MAX_ERROR_MESSAGE_LENGTH + ? `${message.slice(0, MAX_ERROR_MESSAGE_LENGTH - 3)}...` + : message; +}; + const loadOnePasswordSdk = (): Effect.Effect => Effect.tryPromise({ try: () => import("@1password/sdk"), - catch: () => + catch: (cause) => new OnePasswordError({ operation: "sdk module load", - message: "Failed to load 1Password SDK", + message: messageWithCause("Failed to load 1Password SDK", cause), }), }); @@ -99,20 +121,20 @@ export const makeNativeSdkService = ( integrationName: "Executor", integrationVersion: "0.0.0", }), - catch: () => + catch: (cause) => new OnePasswordError({ operation: "client setup", - message: "Failed to set up 1Password client", + message: messageWithCause("Failed to set up 1Password client", cause), }), }).pipe(timeoutWithOnePasswordError("client setup", timeoutMs)); const wrap = (fn: () => Promise, operation: string): Effect.Effect => Effect.tryPromise({ try: fn, - catch: () => + catch: (cause) => new OnePasswordError({ operation, - message: `1Password SDK ${operation} failed`, + message: messageWithCause(`1Password SDK ${operation} failed`, cause), }), }).pipe( timeoutWithOnePasswordError(operation, timeoutMs), @@ -158,10 +180,10 @@ export const makeCliService = ( } return fn(); }, - catch: () => + catch: (cause) => new OnePasswordError({ operation, - message: `1Password CLI ${operation} failed`, + message: messageWithCause(`1Password CLI ${operation} failed`, cause), }), }), ) @@ -186,6 +208,24 @@ export const makeCliService = ( // Smart factory — tries CLI first (avoids IPC hang), falls back to SDK // --------------------------------------------------------------------------- +const isCliUnavailable = (error: OnePasswordError): boolean => { + // oxlint-disable-next-line executor/no-unknown-error-message -- boundary: OnePasswordError carries a typed `message` + const message = error.message.toLowerCase(); + return ( + message.includes("enoent") || + message.includes("not found") || + message.includes("command not found") || + message.includes("not installed") || + message.includes("no such file") || + message.includes("spawn op") + ); +}; + +const chooseFallbackError = ( + cliError: OnePasswordError, + sdkError: OnePasswordError, +): OnePasswordError => (isCliUnavailable(cliError) ? sdkError : cliError); + export const makeOnePasswordService = ( auth: ResolvedAuth, options?: { readonly preferSdk?: boolean; readonly timeoutMs?: number }, @@ -196,11 +236,33 @@ export const makeOnePasswordService = ( return makeNativeSdkService(auth, timeoutMs); } - // Default: prefer CLI to avoid the IPC hang bug - return makeCliService(auth).pipe( - Effect.catch((cliError: OnePasswordError) => - // CLI unavailable (e.g. `op` not installed) — fall back to SDK - makeNativeSdkService(auth, timeoutMs).pipe(Effect.mapError(() => cliError)), - ), - ); + return Effect.gen(function* () { + const cliService = yield* makeCliService(auth); + const sdkService = yield* Effect.cached(makeNativeSdkService(auth, timeoutMs)); + + const withSdkFallback = ( + cliEffect: Effect.Effect, + sdkEffect: (service: OnePasswordService) => Effect.Effect, + ): Effect.Effect => + cliEffect.pipe( + Effect.catch((cliError: OnePasswordError) => + sdkService.pipe( + Effect.flatMap(sdkEffect), + Effect.mapError((sdkError: OnePasswordError) => + chooseFallbackError(cliError, sdkError), + ), + ), + ), + ); + + return OnePasswordServiceTag.of({ + resolveSecret: (uri) => + withSdkFallback(cliService.resolveSecret(uri), (service) => service.resolveSecret(uri)), + + listVaults: () => withSdkFallback(cliService.listVaults(), (service) => service.listVaults()), + + listItems: (vaultId) => + withSdkFallback(cliService.listItems(vaultId), (service) => service.listItems(vaultId)), + }); + }).pipe(Effect.withSpan("onepassword.make_service")); };