diff --git a/.changeset/defer-irreversible-cleanup.md b/.changeset/defer-irreversible-cleanup.md index 67ff239831..f501750bd4 100644 --- a/.changeset/defer-irreversible-cleanup.md +++ b/.changeset/defer-irreversible-cleanup.md @@ -1,5 +1,5 @@ --- -"executor": minor +"executor": patch --- **Irreversible cleanup now waits for the transaction to commit, and plugins can do the same** diff --git a/.changeset/onepassword-multiple-vaults.md b/.changeset/onepassword-multiple-vaults.md new file mode 100644 index 0000000000..24b96a2813 --- /dev/null +++ b/.changeset/onepassword-multiple-vaults.md @@ -0,0 +1,11 @@ +--- +"executor": patch +--- + +**The 1Password provider can now be scoped to several vaults, with explicit per-vault addressing** + +The provider previously bound exactly one vault. The configuration now holds a set of vaults selected with checkboxes, and every reference is explicit about which vault it means: the item picker is a searchable list that shows each item's vault and stores a vault-qualified `op://` reference, so identically-titled items in different vaults can never collide. A bare item name is accepted only when it matches exactly one item across the selected vaults — a name that exists in more than one place fails with an error naming the matching vaults instead of silently picking one. + +Reopening the vault or item pickers no longer flashes a loading state: listings are retained and re-validated in the background, so the last-known list renders instantly. + +Configurations saved before this change keep working: the stored single-vault shape is read as a one-vault list and upgrades to the new shape the next time it is saved. The `status` tool reports `vaultNames` for all configured vaults and flags any configured vault the account can no longer see. Provider entries also gained an optional `group` label, which pickers use to show where an item lives. diff --git a/packages/core/api/src/handlers/providers.ts b/packages/core/api/src/handlers/providers.ts index 615d5d6ccd..092b56df91 100644 --- a/packages/core/api/src/handlers/providers.ts +++ b/packages/core/api/src/handlers/providers.ts @@ -21,7 +21,11 @@ export const ProvidersHandlers = HttpApiBuilder.group(ExecutorApi, "providers", Effect.gen(function* () { const executor = yield* ExecutorService; const entries = yield* executor.providers.items(path.key); - return entries.map((entry) => ({ id: entry.id, name: entry.name })); + return entries.map((entry) => ({ + id: entry.id, + name: entry.name, + ...(entry.group !== undefined ? { group: entry.group } : {}), + })); }), ), ), diff --git a/packages/core/api/src/providers/api.ts b/packages/core/api/src/providers/api.ts index 72b209db6a..efd8b635a7 100644 --- a/packages/core/api/src/providers/api.ts +++ b/packages/core/api/src/providers/api.ts @@ -26,6 +26,7 @@ const ProviderParams = { key: ProviderKey }; const ProviderEntryResponse = Schema.Struct({ id: ProviderItemId, name: Schema.String, + group: Schema.optional(Schema.String), }); // --------------------------------------------------------------------------- diff --git a/packages/core/sdk/src/provider.ts b/packages/core/sdk/src/provider.ts index 42a3defa4a..d2a895a771 100644 --- a/packages/core/sdk/src/provider.ts +++ b/packages/core/sdk/src/provider.ts @@ -16,6 +16,9 @@ export interface ProviderEntry { * a connection can reference it without core knowing its internal shape. */ readonly id: ProviderItemId; readonly name: string; + /** Optional provenance label for pickers when a provider spans several + * containers (a 1Password vault name). Purely presentational. */ + readonly group?: string; } export interface CredentialProvider { diff --git a/packages/plugins/onepassword/src/react/OnePasswordSettings.tsx b/packages/plugins/onepassword/src/react/OnePasswordSettings.tsx index 59e339aa75..220b2ddbc3 100644 --- a/packages/plugins/onepassword/src/react/OnePasswordSettings.tsx +++ b/packages/plugins/onepassword/src/react/OnePasswordSettings.tsx @@ -1,8 +1,9 @@ -import { useState } from "react"; -import { useAtomSet, useAtomValue } from "@effect/atom-react"; +import { useEffect, useRef, useState } from "react"; +import { useAtomRefresh, useAtomSet, useAtomValue } from "@effect/atom-react"; import * as Exit from "effect/Exit"; import * as AsyncResult from "effect/unstable/reactivity/AsyncResult"; import { Button } from "@executor-js/react/components/button"; +import { Checkbox } from "@executor-js/react/components/checkbox"; import { Input } from "@executor-js/react/components/input"; import { Label } from "@executor-js/react/components/label"; import { @@ -35,10 +36,10 @@ import { removeOnePasswordConfig, onepasswordWriteKeys, } from "./atoms"; -import type { RedactedOnePasswordConfig } from "../sdk/types"; +import type { RedactedOnePasswordConfig, Vault } from "../sdk/types"; // --------------------------------------------------------------------------- -// Vault picker +// Vault picker — multi-select // --------------------------------------------------------------------------- const VAULT_LIST_ERROR_FALLBACK = "Failed to list vaults"; @@ -52,11 +53,24 @@ const formatVaultListError = (error: Error): string => { function VaultPicker(props: { authKind: "desktop-app" | "service-account"; accountName: string; - vaultId: string; - onVaultSelect: (id: string, name: string) => void; + selected: ReadonlyArray; + onSelectedChange: (vaults: ReadonlyArray) => void; }) { const account = props.accountName.trim(); - const vaultsResult = useAtomValue(onepasswordVaultsAtom(props.authKind, account)); + const vaultsAtom = onepasswordVaultsAtom(props.authKind, account); + const vaultsResult = useAtomValue(vaultsAtom); + const refreshVaults = useAtomRefresh(vaultsAtom); + + // Stale-while-revalidate: with a retained value the vault list renders + // instantly and one background refresh per atom key picks up changes + // (refreshing keeps the previous value, so nothing flashes). A cold key is + // already fetching — refreshing it would only restart the request. The ref + // carries the latest cached-ness into the effect without re-running it. + const isCachedRef = useRef(false); + isCachedRef.current = AsyncResult.isSuccess(vaultsResult); + useEffect(() => { + if (isCachedRef.current) refreshVaults(); + }, [refreshVaults]); const { vaults, isLoading, error } = AsyncResult.matchWithError( vaultsResult as AsyncResult.AsyncResult< @@ -81,12 +95,9 @@ function VaultPicker(props: { }), onSuccess: ({ value }) => { const v = value.vaults; - const defaultVault = v[0]; - if ( - defaultVault && - (!props.vaultId || (v.length === 1 && props.vaultId !== defaultVault.id)) - ) { - queueMicrotask(() => props.onVaultSelect(defaultVault.id, defaultVault.name)); + const onlyVault = v.length === 1 ? v[0] : undefined; + if (onlyVault && props.selected.length === 0) { + queueMicrotask(() => props.onSelectedChange([onlyVault])); } return { vaults: [...v], isLoading: false, error: null }; }, @@ -101,34 +112,51 @@ function VaultPicker(props: { ); } - const singleVault = vaults.length === 1 ? vaults[0] : null; + // Selected vaults missing from the loaded list (renamed, revoked, or the + // list failed to load while editing) stay visible so they can be unchecked. + const loadedIds = new Set(vaults.map((v) => v.id)); + const stale = props.selected.filter((v) => !loadedIds.has(v.id)); + const rows = [...vaults, ...stale]; + + const toggle = (vault: Vault, checked: boolean) => { + if (checked) { + if (!props.selected.some((v) => v.id === vault.id)) { + props.onSelectedChange([...props.selected, vault]); + } + return; + } + props.onSelectedChange(props.selected.filter((v) => v.id !== vault.id)); + }; return (
- {singleVault ? ( -
- {singleVault.name} -
+ {isLoading ? ( +

Loading vaults…

+ ) : rows.length === 0 ? ( +

No vaults found.

) : ( - +
+ {rows.map((vault) => { + const checked = props.selected.some((v) => v.id === vault.id); + return ( + + ); + })} +
)} {error && (
@@ -151,7 +179,7 @@ function ConfigDialog(props: { initial?: { authKind: string; accountName: string; - vaultId: string; + vaults: ReadonlyArray; name: string; }; }) { @@ -160,8 +188,10 @@ function ConfigDialog(props: { (props.initial?.authKind as "desktop-app" | "service-account") ?? "desktop-app", ); const [accountName, setAccountName] = useState(props.initial?.accountName ?? "my.1password.com"); - const [vaultId, setVaultId] = useState(props.initial?.vaultId ?? ""); - const [vaultName, setVaultName] = useState(props.initial?.name ?? ""); + const [selectedVaults, setSelectedVaults] = useState>( + props.initial?.vaults ?? [], + ); + const [displayName, setDisplayName] = useState(props.initial?.name ?? ""); const [saving, setSaving] = useState(false); const [error, setError] = useState(null); @@ -171,15 +201,16 @@ function ConfigDialog(props: { if (!isEdit) { setAuthKind("desktop-app"); setAccountName("my.1password.com"); - setVaultId(""); - setVaultName(""); + setSelectedVaults([]); + setDisplayName(""); } setError(null); setSaving(false); }; const handleSave = async () => { - if (!accountName.trim() || !vaultId.trim()) return; + const [firstVault, ...restVaults] = selectedVaults; + if (!accountName.trim() || firstVault === undefined) return; setSaving(true); setError(null); @@ -191,8 +222,8 @@ function ConfigDialog(props: { const exit = await doConfigure({ payload: { auth, - vaultId: vaultId.trim(), - name: vaultName.trim() || "1Password", + vaults: [firstVault, ...restVaults], + name: displayName.trim() || "1Password", }, reactivityKeys: onepasswordWriteKeys, }); @@ -220,7 +251,8 @@ function ConfigDialog(props: { {isEdit ? "Edit 1Password" : "Connect 1Password"} - Link a vault to resolve secrets via the 1Password desktop app or a service account. + Link one or more vaults to resolve secrets via the 1Password desktop app or a service + account. @@ -262,19 +294,16 @@ function ConfigDialog(props: {

- {/* Vault */} + {/* Vaults */}
{ - setVaultId(id); - setVaultName(name); - }} + selected={selectedVaults} + onSelectedChange={setSelectedVaults} />
@@ -285,8 +314,8 @@ function ConfigDialog(props: { setVaultName((e.target as HTMLInputElement).value)} + value={displayName} + onChange={(e) => setDisplayName((e.target as HTMLInputElement).value)} className="text-[13px] h-9" />
@@ -307,7 +336,7 @@ function ConfigDialog(props: { @@ -371,14 +400,18 @@ export default function OnePasswordSettings() { {config.auth.kind === "desktop-app" ? config.auth.accountName : "service-account"} - Vault + + {config.vaults.length === 1 ? "Vault" : "Vaults"} +
- {config.name} + + {config.vaults.map((vault) => vault.name).join(", ")} +
) : ( - Resolve secrets from your 1Password vault. + Resolve secrets from your 1Password vaults. )} @@ -429,7 +462,7 @@ export default function OnePasswordSettings() { // Service-account tokens are never surfaced (redacted); the // user re-enters the token when editing that auth method. accountName: config.auth.kind === "desktop-app" ? config.auth.accountName : "", - vaultId: config.vaultId, + vaults: config.vaults, name: config.name, } : undefined diff --git a/packages/plugins/onepassword/src/react/atoms.ts b/packages/plugins/onepassword/src/react/atoms.ts index af237aac3a..d6cec111bb 100644 --- a/packages/plugins/onepassword/src/react/atoms.ts +++ b/packages/plugins/onepassword/src/react/atoms.ts @@ -33,7 +33,10 @@ export const onepasswordVaultsAtom = ( ) => OnePasswordClient.query("onepassword", "listVaults", { query: { authKind, account }, - timeToLive: "30 seconds", + // Long retention on purpose: vault listing goes through the op CLI/SDK and + // is slow, so a reopened dialog renders the last-known vaults instantly + // and revalidates in the background instead of flashing a loading state. + timeToLive: "10 minutes", reactivityKeys: [ReactivityKey.providers], }); diff --git a/packages/plugins/onepassword/src/sdk/index.ts b/packages/plugins/onepassword/src/sdk/index.ts index f3dd9624e5..751108ebda 100644 --- a/packages/plugins/onepassword/src/sdk/index.ts +++ b/packages/plugins/onepassword/src/sdk/index.ts @@ -1,12 +1,18 @@ export { onepasswordPlugin, makeOnePasswordStore, + resolveConfiguredRef, + ambiguityMessage, + type RefResolution, type OnePasswordExtension, type OnePasswordPluginOptions, type OnePasswordStore, } from "./plugin"; export { OnePasswordConfig, + LegacyOnePasswordConfig, + StoredOnePasswordConfig, + normalizeStoredConfig, RedactedOnePasswordConfig, RedactedOnePasswordAuth, redactConfig, diff --git a/packages/plugins/onepassword/src/sdk/plugin.test.ts b/packages/plugins/onepassword/src/sdk/plugin.test.ts index 5f04b394aa..4d9f1ed916 100644 --- a/packages/plugins/onepassword/src/sdk/plugin.test.ts +++ b/packages/plugins/onepassword/src/sdk/plugin.test.ts @@ -2,9 +2,12 @@ import { describe, it, expect } from "@effect/vitest"; import { Effect } from "effect"; import { ProviderKey, ToolAddress, createExecutor } from "@executor-js/sdk"; +import { makeInMemoryBlobStore, pluginBlobStore } from "@executor-js/sdk/core"; import { makeTestConfig } from "@executor-js/sdk/testing"; -import { onepasswordPlugin } from "./plugin"; +import { makeOnePasswordStore, onepasswordPlugin, resolveConfiguredRef } from "./plugin"; +import type { OnePasswordService } from "./service"; +import { OnePasswordError } from "./errors"; import { OnePasswordConfig, DesktopAppAuth } from "./types"; // removed: v1 routed configure/removeConfig through an explicit `ScopeId` @@ -16,6 +19,18 @@ import { OnePasswordConfig, DesktopAppAuth } from "./types"; const ONEPASSWORD = ProviderKey.make("onepassword"); +const twoVaultConfig = OnePasswordConfig.make({ + auth: DesktopAppAuth.make({ + kind: "desktop-app", + accountName: "my.1password.com", + }), + vaults: [ + { id: "vault-123", name: "Personal" }, + { id: "vault-456", name: "Work" }, + ], + name: "1Password", +}); + describe("onepassword plugin", () => { it.effect("registers onepassword as a credential provider", () => Effect.gen(function* () { @@ -36,20 +51,14 @@ describe("onepassword plugin", () => { const initial = yield* executor.onepassword.getConfig(); expect(initial).toBeNull(); - const config = OnePasswordConfig.make({ - auth: DesktopAppAuth.make({ - kind: "desktop-app", - accountName: "my.1password.com", - }), - vaultId: "vault-123", - name: "Personal", - }); - - yield* executor.onepassword.configure(config); + yield* executor.onepassword.configure(twoVaultConfig); const loaded = yield* executor.onepassword.getConfig(); - expect(loaded?.vaultId).toBe("vault-123"); - expect(loaded?.name).toBe("Personal"); + expect(loaded?.vaults).toEqual([ + { id: "vault-123", name: "Personal" }, + { id: "vault-456", name: "Work" }, + ]); + expect(loaded?.name).toBe("1Password"); expect(loaded?.auth.kind).toBe("desktop-app"); yield* executor.onepassword.removeConfig(); @@ -67,7 +76,7 @@ describe("onepassword plugin", () => { yield* executor.onepassword.configure( OnePasswordConfig.make({ auth: { kind: "service-account", token: "super-secret-token" }, - vaultId: "vault-123", + vaults: [{ id: "vault-123", name: "CI" }], name: "CI", }), ); @@ -89,8 +98,11 @@ describe("onepassword plugin", () => { ToolAddress.make("executor.onepassword.configure"), { auth: { kind: "desktop-app", accountName: "my.1password.com" }, - vaultId: "vault-123", - name: "Personal", + vaults: [ + { id: "vault-123", name: "Personal" }, + { id: "vault-456", name: "Work" }, + ], + name: "1Password", }, { onElicitation: "accept-all" }, ); @@ -100,7 +112,15 @@ describe("onepassword plugin", () => { yield* executor.execute(ToolAddress.make("executor.onepassword.getConfig"), {}), ).toMatchObject({ ok: true, - data: { config: { vaultId: "vault-123", name: "Personal" } }, + data: { + config: { + vaults: [ + { id: "vault-123", name: "Personal" }, + { id: "vault-456", name: "Work" }, + ], + name: "1Password", + }, + }, }); const removed = yield* executor.execute( @@ -125,3 +145,180 @@ describe("onepassword plugin", () => { }), ); }); + +// --------------------------------------------------------------------------- +// Stored-config compatibility — blobs written before multi-vault support hold +// `{ auth, vaultId, name }`. Reads normalize that to a one-element vaults +// array; the next save writes the current shape. +// --------------------------------------------------------------------------- + +describe("onepassword store", () => { + const makeStore = () => { + const blobs = pluginBlobStore( + makeInMemoryBlobStore(), + { org: "org_test", user: null }, + "onepassword", + ); + return { blobs, store: makeOnePasswordStore(blobs) }; + }; + + it.effect("upgrades a legacy single-vault blob on read", () => + Effect.gen(function* () { + const { blobs, store } = makeStore(); + yield* blobs.put( + "config", + JSON.stringify({ + auth: { kind: "desktop-app", accountName: "my.1password.com" }, + vaultId: "vault-123", + name: "Personal", + }), + { owner: "org" }, + ); + + const config = yield* store.getConfig(); + expect(config).toEqual({ + auth: { kind: "desktop-app", accountName: "my.1password.com" }, + vaults: [{ id: "vault-123", name: "Personal" }], + name: "Personal", + }); + }), + ); + + it.effect("persists and reads back the multi-vault shape", () => + Effect.gen(function* () { + const { store } = makeStore(); + yield* store.saveConfig(twoVaultConfig, "org"); + const config = yield* store.getConfig(); + expect(config).toEqual(twoVaultConfig); + }), + ); +}); + +// --------------------------------------------------------------------------- +// Explicit ref resolution — vault-qualified refs resolve directly; bare refs +// must locate exactly one item, and a multi-vault match is an explicit +// ambiguity, never a precedence pick. +// --------------------------------------------------------------------------- + +const fakeService = ( + itemsByVault: Readonly>, + onResolve?: (uri: string) => void, +): OnePasswordService => ({ + resolveSecret: (uri) => { + onResolve?.(uri); + return Effect.succeed(`secret:${uri}`); + }, + listVaults: () => Effect.succeed(Object.keys(itemsByVault).map((id) => ({ id, title: id }))), + listItems: (vaultId) => { + const items = itemsByVault[vaultId]; + return items === undefined + ? Effect.fail(new OnePasswordError({ operation: "item listing", message: "no such vault" })) + : Effect.succeed(items); + }, +}); + +describe("resolveConfiguredRef", () => { + it.effect("resolves a fully-qualified op:// URI in a configured vault as-is", () => + Effect.gen(function* () { + const svc = fakeService({}); + const result = yield* resolveConfiguredRef( + svc, + twoVaultConfig, + "op://vault-456/item-abc/password", + ); + expect(result).toEqual({ + kind: "resolved", + value: "secret:op://vault-456/item-abc/password", + }); + }), + ); + + it.effect("appends the credential field to a picker-shaped op://vault/item ref", () => + Effect.gen(function* () { + const svc = fakeService({}); + const result = yield* resolveConfiguredRef(svc, twoVaultConfig, "op://vault-123/item-abc"); + expect(result).toEqual({ + kind: "resolved", + value: "secret:op://vault-123/item-abc/credential", + }); + }), + ); + + it.effect("accepts an op:// URI addressed by vault name", () => + Effect.gen(function* () { + const svc = fakeService({}); + const result = yield* resolveConfiguredRef(svc, twoVaultConfig, "op://Work/item/password"); + expect(result).toEqual({ kind: "resolved", value: "secret:op://Work/item/password" }); + }), + ); + + it.effect("reports an op:// URI outside the configured vaults", () => + Effect.gen(function* () { + const svc = fakeService({}); + const result = yield* resolveConfiguredRef( + svc, + twoVaultConfig, + "op://vault-999/item/password", + ); + expect(result).toEqual({ kind: "outside-vaults" }); + }), + ); + + it.effect("resolves a bare ref that matches exactly one item across vaults", () => + Effect.gen(function* () { + const svc = fakeService({ + "vault-123": [{ id: "item-1", title: "GitHub Token" }], + "vault-456": [{ id: "item-2", title: "Stripe Key" }], + }); + const result = yield* resolveConfiguredRef(svc, twoVaultConfig, "GitHub Token"); + expect(result).toEqual({ + kind: "resolved", + value: "secret:op://vault-123/item-1/credential", + }); + }), + ); + + it.effect("fails a bare ref that matches in two vaults with the vaults named", () => + Effect.gen(function* () { + const svc = fakeService({ + "vault-123": [{ id: "item-1", title: "GitHub Token" }], + "vault-456": [{ id: "item-2", title: "GitHub Token" }], + }); + const result = yield* resolveConfiguredRef(svc, twoVaultConfig, "GitHub Token"); + expect(result).toEqual({ + kind: "ambiguous", + matches: [ + { + vaultId: "vault-123", + vaultName: "Personal", + itemId: "item-1", + itemTitle: "GitHub Token", + }, + { vaultId: "vault-456", vaultName: "Work", itemId: "item-2", itemTitle: "GitHub Token" }, + ], + }); + }), + ); + + it.effect("treats duplicate titles inside one vault as ambiguous too", () => + Effect.gen(function* () { + const svc = fakeService({ + "vault-123": [ + { id: "item-1", title: "GitHub Token" }, + { id: "item-9", title: "GitHub Token" }, + ], + "vault-456": [], + }); + const result = yield* resolveConfiguredRef(svc, twoVaultConfig, "GitHub Token"); + expect(result.kind).toBe("ambiguous"); + }), + ); + + it.effect("reports not-found for a bare ref matching nothing", () => + Effect.gen(function* () { + const svc = fakeService({ "vault-123": [], "vault-456": [] }); + const result = yield* resolveConfiguredRef(svc, twoVaultConfig, "missing"); + expect(result).toEqual({ kind: "not-found" }); + }), + ); +}); diff --git a/packages/plugins/onepassword/src/sdk/plugin.ts b/packages/plugins/onepassword/src/sdk/plugin.ts index acc997965e..fa7ad5f52a 100644 --- a/packages/plugins/onepassword/src/sdk/plugin.ts +++ b/packages/plugins/onepassword/src/sdk/plugin.ts @@ -20,8 +20,10 @@ import { OnePasswordAuth, OnePasswordConfig, RedactedOnePasswordConfig, + StoredOnePasswordConfig, Vault, ConnectionStatus, + normalizeStoredConfig, redactConfig, } from "./types"; import { OnePasswordError } from "./errors"; @@ -42,11 +44,7 @@ const schemaToStaticToolSchema = (schema: Schema.Decoder): StaticToo I >; -const OnePasswordConfigureInput = Schema.Struct({ - auth: OnePasswordAuth, - vaultId: Schema.String, - name: Schema.String, -}); +const OnePasswordConfigureInput = OnePasswordConfig; const OnePasswordConfigureOutput = Schema.Struct({ configured: Schema.Boolean, @@ -98,10 +96,12 @@ export type OnePasswordExtensionFailure = OnePasswordError | StorageFailure; // --------------------------------------------------------------------------- // Typed config store — single blob, JSON encoded, owner-partitioned. The // stored config carries the auth credential (desktop account name, or -// service-account token) plus the selected vault. v1 keyed this by executor +// service-account token) plus the selected vaults. v1 keyed this by executor // scope; v2 partitions by `owner` — the plugin-owned config row owns the -// partition, mirroring the connection model. Blob I/O failures surface as -// `StorageError`; decode failures stay `OnePasswordError`. +// partition, mirroring the connection model. Reads also accept the legacy +// single-`vaultId` shape and normalize it to the vaults array; saves always +// write the current shape. Blob I/O failures surface as `StorageError`; +// decode failures stay `OnePasswordError`. // --------------------------------------------------------------------------- export interface OnePasswordStore { @@ -116,7 +116,7 @@ export interface OnePasswordStore { readonly deleteConfig: (owner: Owner) => Effect.Effect; } -const decodeConfig = Schema.decodeUnknownEffect(Schema.fromJsonString(OnePasswordConfig)); +const decodeConfig = Schema.decodeUnknownEffect(Schema.fromJsonString(StoredOnePasswordConfig)); const blobStorageError = (operation: string) => @@ -133,6 +133,7 @@ export const makeOnePasswordStore = (blobs: PluginBlobStore): OnePasswordStore = Effect.flatMap((raw) => { if (raw === null) return Effect.succeed(null); return decodeConfig(raw).pipe( + Effect.map(normalizeStoredConfig), Effect.mapError( () => new OnePasswordError({ @@ -150,7 +151,7 @@ export const makeOnePasswordStore = (blobs: PluginBlobStore): OnePasswordStore = CONFIG_KEY, JSON.stringify({ auth: config.auth, - vaultId: config.vaultId, + vaults: config.vaults, name: config.name, }), { owner }, @@ -177,22 +178,105 @@ const getServiceFromConfig = ( ): Effect.Effect => makeOnePasswordService(resolveAuth(config.auth), { timeoutMs, preferSdk }); -const configuredVaultUri = (config: OnePasswordConfig, itemId: string): string | null => { - if (!itemId.startsWith("op://")) { - return `op://${config.vaultId}/${itemId}/${CREDENTIAL_FIELD}`; +// --------------------------------------------------------------------------- +// Explicit ref resolution. +// +// A ref is one of: +// - `op://vault/item/field...` — fully qualified, resolved as-is. +// - `op://vault/item` — picker-shaped; the default credential field +// is appended. This is the id shape `list()` +// hands out, so every picked item permanently +// records which vault it came from. +// - a bare item id or title — located by listing the configured vaults. +// Exactly one match resolves; several matches +// are an explicit ambiguity failure naming the +// vaults — never a silent precedence pick. +// --------------------------------------------------------------------------- + +export type RefResolution = + | { readonly kind: "resolved"; readonly value: string } + | { readonly kind: "not-found" } + | { readonly kind: "outside-vaults" } + | { + readonly kind: "ambiguous"; + readonly matches: readonly { + readonly vaultId: string; + readonly vaultName: string; + readonly itemId: string; + readonly itemTitle: string; + }[]; + }; + +export const ambiguityMessage = ( + ref: string, + matches: Extract["matches"], +): string => + [ + `1Password ref "${ref}" is ambiguous: it matches`, + matches.map((m) => `"${m.itemTitle}" in vault "${m.vaultName}"`).join(", "), + `. Use op:/// to pick one.`, + ].join(" "); + +const isConfiguredVaultSegment = (config: OnePasswordConfig, segment: string): boolean => + config.vaults.some((vault) => vault.id === segment || vault.name === segment); + +/** Resolve a ref against the configured vaults. Backend failures stay on the + * error channel; every addressing outcome is an explicit `RefResolution`. */ +export const resolveConfiguredRef = ( + svc: OnePasswordService, + config: OnePasswordConfig, + ref: string, +): Effect.Effect => { + if (ref.startsWith("op://")) { + const segments = ref.slice("op://".length).split("/"); + const vaultSegment = segments[0]; + if (segments.length < 2 || vaultSegment === undefined || segments.includes("")) { + return Effect.succeed({ kind: "not-found" }); + } + if (!isConfiguredVaultSegment(config, vaultSegment)) { + return Effect.succeed({ kind: "outside-vaults" }); + } + const uri = segments.length === 2 ? `${ref}/${CREDENTIAL_FIELD}` : ref; + return svc + .resolveSecret(uri) + .pipe(Effect.map((value): RefResolution => ({ kind: "resolved", value }))); } - const match = itemId.match(/^op:\/\/([^/]+)\/.+/); - if (!match || match[1] !== config.vaultId) return null; - return itemId; + + return Effect.gen(function* () { + const matches = (yield* Effect.forEach(config.vaults, (vault) => + svc.listItems(vault.id).pipe( + Effect.map((items) => + items + .filter((item) => item.id === ref || item.title === ref) + .map((item) => ({ + vaultId: vault.id, + vaultName: vault.name, + itemId: item.id, + itemTitle: item.title, + })), + ), + ), + )).flat(); + + const [only, ...extra] = matches; + if (only === undefined) return { kind: "not-found" } as const; + if (extra.length > 0) return { kind: "ambiguous", matches } as const; + + const value = yield* svc.resolveSecret( + `op://${only.vaultId}/${only.itemId}/${CREDENTIAL_FIELD}`, + ); + return { kind: "resolved", value } as const; + }); }; // --------------------------------------------------------------------------- -// CredentialProvider — read-only, resolves op:// URIs or vaultId-based lookups. +// CredentialProvider — read-only, resolves op:// URIs or vault-scoped lookups. // // v2: `get(id)` receives only an opaque `ProviderItemId` — no scope. The id is -// either a fully-qualified `op://vault/item/field` URI or a bare item id that -// the stored config's vault scopes. The plugin's stored config supplies the -// auth + vault binding; the provider never writes (writable: false). +// a vault-qualified `op://` ref (what `list()` hands out) or a bare item +// id/title that must locate exactly one item across the configured vaults. +// The plugin's stored config supplies the auth + vault bindings; the provider +// never writes (writable: false). // --------------------------------------------------------------------------- const makeProvider = ( @@ -205,19 +289,32 @@ const makeProvider = ( get: (id: ProviderItemId): Effect.Effect => ctx.storage.getConfig().pipe( + // An undecodable stored config reads as "not configured" here; the + // settings surface reports the decode problem. + Effect.catchTag("OnePasswordError", () => Effect.succeed(null)), Effect.flatMap((config) => { if (!config) return Effect.succeed(null as string | null); - const uri = configuredVaultUri(config, id); - if (uri === null) return Effect.succeed(null as string | null); - return getServiceFromConfig(config, timeoutMs, preferSdk).pipe( - Effect.flatMap((svc) => svc.resolveSecret(uri)), - Effect.map((v): string | null => v), - Effect.orElseSucceed(() => null), + Effect.flatMap((svc) => resolveConfiguredRef(svc, config, id)), + // Backend unreachability degrades to "no value", matching the other + // providers. Ambiguity does NOT: silently picking a vault (or + // silently failing) hides a real conflict, so it surfaces as a + // typed failure with the full explanation. + Effect.catch(() => Effect.succeed({ kind: "not-found" } as RefResolution)), + Effect.flatMap( + (resolution): Effect.Effect => + resolution.kind === "ambiguous" + ? Effect.fail( + new StorageError({ + message: ambiguityMessage(id, resolution.matches), + cause: undefined, + }), + ) + : Effect.succeed(resolution.kind === "resolved" ? resolution.value : null), + ), ); }), - Effect.catch(() => Effect.succeed(null as string | null)), ), list: (): Effect.Effect => @@ -225,10 +322,25 @@ const makeProvider = ( Effect.flatMap((config) => { if (!config) return Effect.succeed([] as readonly ProviderEntry[]); return getServiceFromConfig(config, timeoutMs, preferSdk).pipe( - Effect.flatMap((svc) => svc.listItems(config.vaultId)), - Effect.map((items): readonly ProviderEntry[] => - items.map((item) => ({ id: ProviderItemId.make(item.id), name: item.title })), + Effect.flatMap((svc) => + Effect.forEach(config.vaults, (vault) => + svc.listItems(vault.id).pipe( + Effect.map((items) => + items.map( + // Vault-qualified ids: picking an entry permanently + // records which vault it came from, so identically-titled + // items in different vaults can never collide. + (item): ProviderEntry => ({ + id: ProviderItemId.make(`op://${vault.id}/${item.id}`), + name: item.title, + group: vault.name, + }), + ), + ), + ), + ), ), + Effect.map((groups): readonly ProviderEntry[] => groups.flat()), ); }), Effect.catch(() => Effect.succeed([] as readonly ProviderEntry[])), @@ -270,11 +382,19 @@ const makeOnePasswordExtension = ( }); } const svc = yield* getServiceFromConfig(config, timeoutMs, preferSdk); - const vaults = yield* svc.listVaults(); - const vault = vaults.find((v) => v.id === config.vaultId); + const live = yield* svc.listVaults(); + const liveById = new Map(live.map((v) => [v.id, v.title])); + const missing = config.vaults.filter((vault) => !liveById.has(vault.id)); return ConnectionStatus.make({ connected: true, - vaultName: vault?.title, + vaultNames: config.vaults.map((vault) => liveById.get(vault.id) ?? vault.name), + ...(missing.length > 0 + ? { + error: `Configured vaults not found: ${missing + .map((vault) => vault.name) + .join(", ")}`, + } + : {}), }); }), @@ -299,15 +419,25 @@ const makeOnePasswordExtension = ( message: "1Password is not configured", }); } - const scopedUri = configuredVaultUri(config, uri); - if (scopedUri === null) { + const svc = yield* getServiceFromConfig(config, timeoutMs, preferSdk); + const resolution = yield* resolveConfiguredRef(svc, config, uri); + if (resolution.kind === "resolved") return resolution.value; + if (resolution.kind === "outside-vaults") { return yield* new OnePasswordError({ operation: "resolve", - message: "1Password secret URI is outside the configured vault", + message: "1Password secret URI is outside the configured vaults", }); } - const svc = yield* getServiceFromConfig(config, timeoutMs, preferSdk); - return yield* svc.resolveSecret(scopedUri); + if (resolution.kind === "ambiguous") { + return yield* new OnePasswordError({ + operation: "resolve", + message: ambiguityMessage(uri, resolution.matches), + }); + } + return yield* new OnePasswordError({ + operation: "resolve", + message: `1Password item "${uri}" was not found in the configured vaults`, + }); }), }; }; @@ -345,7 +475,7 @@ export const onepasswordPlugin = definePlugin((options?: OnePasswordPluginOption tool({ name: "status", description: - "Check whether the 1Password credential provider is configured and can reach its selected vault. This returns status only, never secret values.", + "Check whether the 1Password credential provider is configured and can reach its selected vaults. This returns status only, never secret values.", outputSchema: OnePasswordStatusOutputStd, execute: () => Effect.map(self.status(), ToolResult.ok), }), @@ -368,7 +498,7 @@ export const onepasswordPlugin = definePlugin((options?: OnePasswordPluginOption tool({ name: "configure", description: - "Configure the 1Password credential provider for the acting owner. Use desktop-app auth for local biometric access, or service-account auth with the token. The token is stored in the plugin's owner-partitioned config and never surfaced again.", + "Configure the 1Password credential provider for the acting owner with one or more vaults. Use desktop-app auth for local biometric access, or service-account auth with the token. The token is stored in the plugin's owner-partitioned config and never surfaced again.", annotations: { requiresApproval: true, approvalDescription: "Configure the 1Password credential provider", @@ -377,7 +507,7 @@ export const onepasswordPlugin = definePlugin((options?: OnePasswordPluginOption outputSchema: OnePasswordConfigureOutputStd, execute: (input) => Effect.as( - self.configure({ auth: input.auth, vaultId: input.vaultId, name: input.name }), + self.configure({ auth: input.auth, vaults: input.vaults, name: input.name }), ToolResult.ok({ configured: true }), ), }), diff --git a/packages/plugins/onepassword/src/sdk/types.ts b/packages/plugins/onepassword/src/sdk/types.ts index 6c4e903451..82f70269ed 100644 --- a/packages/plugins/onepassword/src/sdk/types.ts +++ b/packages/plugins/onepassword/src/sdk/types.ts @@ -24,19 +24,53 @@ export type ServiceAccountAuth = typeof ServiceAccountAuth.Type; export const OnePasswordAuth = Schema.Union([DesktopAppAuth, ServiceAccountAuth]); export type OnePasswordAuth = typeof OnePasswordAuth.Type; +// --------------------------------------------------------------------------- +// Vault +// --------------------------------------------------------------------------- + +export const Vault = Schema.Struct({ + id: Schema.String, + name: Schema.String, +}); +export type Vault = typeof Vault.Type; + // --------------------------------------------------------------------------- // Stored config — persisted via KV // --------------------------------------------------------------------------- export const OnePasswordConfig = Schema.Struct({ auth: OnePasswordAuth, - /** Vault to scope operations to */ - vaultId: Schema.String, - /** Human label */ + /** Vaults to scope operations to. Order is presentational only: refs are + * vault-qualified, and a bare ref that matches in more than one vault is an + * explicit ambiguity failure, never a precedence pick. */ + vaults: Schema.NonEmptyArray(Vault), + /** Human label for the whole connection */ name: Schema.String, }); export type OnePasswordConfig = typeof OnePasswordConfig.Type; +/** Pre-multi-vault stored shape: a single vault id whose display name doubled + * as the connection label. Still accepted on read; every save writes the + * current shape, so a config row upgrades the first time it is re-saved. */ +export const LegacyOnePasswordConfig = Schema.Struct({ + auth: OnePasswordAuth, + vaultId: Schema.String, + name: Schema.String, +}); +export type LegacyOnePasswordConfig = typeof LegacyOnePasswordConfig.Type; + +export const StoredOnePasswordConfig = Schema.Union([OnePasswordConfig, LegacyOnePasswordConfig]); +export type StoredOnePasswordConfig = typeof StoredOnePasswordConfig.Type; + +export const normalizeStoredConfig = (stored: StoredOnePasswordConfig): OnePasswordConfig => + "vaultId" in stored + ? { + auth: stored.auth, + vaults: [{ id: stored.vaultId, name: stored.name }], + name: stored.name, + } + : stored; + // --------------------------------------------------------------------------- // Redacted config — what `getConfig` returns to agents / the UI. The // service-account token is stripped; only the auth kind + account metadata is @@ -56,7 +90,7 @@ export const RedactedOnePasswordAuth = Schema.Union([ export const RedactedOnePasswordConfig = Schema.Struct({ auth: RedactedOnePasswordAuth, - vaultId: Schema.String, + vaults: Schema.NonEmptyArray(Vault), name: Schema.String, }); export type RedactedOnePasswordConfig = typeof RedactedOnePasswordConfig.Type; @@ -67,27 +101,17 @@ export const redactConfig = (config: OnePasswordConfig): RedactedOnePasswordConf config.auth.kind === "desktop-app" ? { kind: "desktop-app", accountName: config.auth.accountName } : { kind: "service-account" }, - vaultId: config.vaultId, + vaults: config.vaults, name: config.name, }); -// --------------------------------------------------------------------------- -// Vault -// --------------------------------------------------------------------------- - -export const Vault = Schema.Struct({ - id: Schema.String, - name: Schema.String, -}); -export type Vault = typeof Vault.Type; - // --------------------------------------------------------------------------- // Connection status // --------------------------------------------------------------------------- export const ConnectionStatus = Schema.Struct({ connected: Schema.Boolean, - vaultName: Schema.optional(Schema.String), + vaultNames: Schema.optional(Schema.Array(Schema.String)), error: Schema.optional(Schema.String), }); export type ConnectionStatus = typeof ConnectionStatus.Type; diff --git a/packages/react/src/api/atoms.tsx b/packages/react/src/api/atoms.tsx index c20018bd9d..2b76c1d979 100644 --- a/packages/react/src/api/atoms.tsx +++ b/packages/react/src/api/atoms.tsx @@ -132,7 +132,10 @@ export const providersAtom = ExecutorApiClient.query("providers", "list", { export const providerItemsAtom = (key: ProviderKey) => ExecutorApiClient.query("providers", "items", { params: { key }, - timeToLive: "30 seconds", + // Long retention on purpose: external-provider listings (1Password) are + // slow, so pickers render the last-known list instantly and revalidate in + // the background on mount instead of flashing a loading state each open. + timeToLive: "10 minutes", reactivityKeys: [ReactivityKey.providers], }); diff --git a/packages/react/src/components/add-account-modal.tsx b/packages/react/src/components/add-account-modal.tsx index 43f72ef5e6..40dfb78f7b 100644 --- a/packages/react/src/components/add-account-modal.tsx +++ b/packages/react/src/components/add-account-modal.tsx @@ -1,5 +1,5 @@ import { useCallback, useEffect, useMemo, useRef, useState } from "react"; -import { useAtomSet, useAtomValue } from "@effect/atom-react"; +import { useAtomRefresh, useAtomSet, useAtomValue } from "@effect/atom-react"; import * as Exit from "effect/Exit"; import * as AsyncResult from "effect/unstable/reactivity/AsyncResult"; import { @@ -106,7 +106,14 @@ import { import { Input } from "./input"; import { Label } from "./label"; import { RadioGroup, RadioGroupItem } from "./radio-group"; -import { Select, SelectContent, SelectItem, SelectTrigger, SelectValue } from "./select"; +import { + Combobox, + ComboboxContent, + ComboboxEmpty, + ComboboxInput, + ComboboxItem, + ComboboxList, +} from "./combobox"; import { Tabs, TabsContent, TabsList, TabsTrigger } from "./tabs"; // --------------------------------------------------------------------------- @@ -335,38 +342,45 @@ function PasteCredentialInputs(props: { ); } +type OnePasswordItem = { + readonly id: ProviderItemId; + readonly name: string; + readonly group?: string; +}; + function OnePasswordItemSelect(props: { readonly value: string; readonly onChange: (value: string) => void; }) { - const itemsResult = useAtomValue(providerItemsAtom(ONEPASSWORD_PROVIDER)); + const itemsAtom = providerItemsAtom(ONEPASSWORD_PROVIDER); + const itemsResult = useAtomValue(itemsAtom); + const refreshItems = useAtomRefresh(itemsAtom); + + // Stale-while-revalidate: with a retained value the list renders instantly + // and one background refresh per mount picks up vault changes (refreshing + // keeps the previous value, so nothing flashes). A cold mount is already + // fetching — refreshing it would only restart the request. The ref carries + // the latest cached-ness into the effect without re-running it. + const isCachedRef = useRef(false); + isCachedRef.current = AsyncResult.isSuccess(itemsResult); + useEffect(() => { + if (isCachedRef.current) refreshItems(); + }, [refreshItems]); const state = AsyncResult.matchWithError( - itemsResult as AsyncResult.AsyncResult< - readonly { readonly id: ProviderItemId; readonly name: string }[], - Error - >, + itemsResult as AsyncResult.AsyncResult, { onInitial: () => ({ - items: [] as readonly { - readonly id: ProviderItemId; - readonly name: string; - }[], + items: [] as readonly OnePasswordItem[], loading: true, error: null as string | null, }), onError: () => ({ - items: [] as readonly { - readonly id: ProviderItemId; - readonly name: string; - }[], + items: [] as readonly OnePasswordItem[], loading: false, error: "Failed to load 1Password items", }), onDefect: () => ({ - items: [] as readonly { - readonly id: ProviderItemId; - readonly name: string; - }[], + items: [] as readonly OnePasswordItem[], loading: false, error: "Failed to load 1Password items", }), @@ -374,6 +388,33 @@ function OnePasswordItemSelect(props: { }, ); + const stateItems = state.items; + const sorted = useMemo( + () => + [...stateItems].sort( + (a, b) => a.name.localeCompare(b.name) || (a.group ?? "").localeCompare(b.group ?? ""), + ), + [stateItems], + ); + const byId = useMemo(() => { + const map = new Map(); + for (const item of sorted) map.set(String(item.id), item); + return map; + }, [sorted]); + const ids = useMemo(() => sorted.map((item) => String(item.id)), [sorted]); + + const filter = useCallback( + (id: string, query: string) => { + const needle = query.trim().toLowerCase(); + if (needle === "") return true; + const item = byId.get(id); + return [item?.name ?? "", item?.group ?? ""].some((part) => + part.toLowerCase().includes(needle), + ); + }, + [byId], + ); + if (state.loading) { return

Loading 1Password items…

; } @@ -386,18 +427,36 @@ function OnePasswordItemSelect(props: { return (
- + 0 ? props.value : null} + filter={filter} + limit={100} + itemToStringLabel={(id: string) => byId.get(id)?.name ?? id} + onValueChange={(id) => { + if (id !== null) props.onChange(id); + }} + > + + + No items match. + + {(id: string) => { + const item = byId.get(id); + return ( + + {item?.name ?? id} + {item?.group && ( + + {item.group} + + )} + + ); + }} + + +
); }