diff --git a/cli/src/auth/hosts.test.ts b/cli/src/auth/hosts.test.ts index e4083cf0014..fc1c2756914 100644 --- a/cli/src/auth/hosts.test.ts +++ b/cli/src/auth/hosts.test.ts @@ -1,10 +1,7 @@ import type { AccountContext } from './hosts' -import type { Key, Store } from '@/store/store' -import { mkdtemp, rm } from 'node:fs/promises' -import { tmpdir } from 'node:os' -import { join } from 'node:path' -import { afterEach, beforeEach, describe, expect, it } from 'vitest' -import { ENV_CONFIG_DIR } from '@/store/dir' +import { useTempConfigDir } from '@test/fixtures/config-dir' +import { MemStore } from '@test/fixtures/mem-store' +import { describe, expect, it } from 'vitest' import { AccountContextSchema, notLoggedInError, Registry, RegistrySchema } from './hosts' describe('RegistrySchema', () => { @@ -53,6 +50,20 @@ describe('RegistrySchema', () => { }) expect(ctx.external_subject?.issuer).toBe('https://issuer') }) + + it('strips a stale available_workspaces field from legacy contexts', () => { + const raw = { + account: { id: 'acct-1', email: 'bob@corp.com', name: 'Bob' }, + workspace: { id: 'ws-1', name: 'Space', role: 'owner' }, + available_workspaces: [ + { id: 'ws-1', name: 'Space', role: 'owner' }, + { id: '00000000-0000-0000-0000-000000000002', name: 'Other', role: 'normal' }, + ], + } as unknown as Record + const ctx = AccountContextSchema.parse(raw) + expect((ctx as Record).available_workspaces).toBeUndefined() + expect(ctx.workspace?.id).toBe('ws-1') + }) }) describe('notLoggedInError', () => { @@ -126,19 +137,7 @@ describe('Registry (pure)', () => { }) describe('Registry.load / Registry.save', () => { - let dir: string - let prev: string | undefined - beforeEach(async () => { - dir = await mkdtemp(join(tmpdir(), 'difyctl-reg-')) - prev = process.env[ENV_CONFIG_DIR] - process.env[ENV_CONFIG_DIR] = dir - }) - afterEach(async () => { - if (prev === undefined) - delete process.env[ENV_CONFIG_DIR] - else process.env[ENV_CONFIG_DIR] = prev - await rm(dir, { recursive: true, force: true }) - }) + useTempConfigDir('difyctl-reg-') it('returns an empty registry when nothing saved', () => { const reg = Registry.load() @@ -158,27 +157,8 @@ describe('Registry.load / Registry.save', () => { }) }) -class MemStore implements Store { - readonly entries = new Map() - get(key: Key): T { return (this.entries.get(key.key) as T | undefined) ?? key.default } - set(key: Key, value: T): void { this.entries.set(key.key, value) } - unset(key: Key): void { this.entries.delete(key.key) } -} - describe('Registry.forget', () => { - let dir: string - let prev: string | undefined - beforeEach(async () => { - dir = await mkdtemp(join(tmpdir(), 'difyctl-forget-')) - prev = process.env[ENV_CONFIG_DIR] - process.env[ENV_CONFIG_DIR] = dir - }) - afterEach(async () => { - if (prev === undefined) - delete process.env[ENV_CONFIG_DIR] - else process.env[ENV_CONFIG_DIR] = prev - await rm(dir, { recursive: true, force: true }) - }) + useTempConfigDir('difyctl-forget-') it('drops token + active context, keeps siblings, unsets pointers', () => { const store = new MemStore() @@ -188,12 +168,12 @@ describe('Registry.forget', () => { reg.setHost('h1') reg.setAccount('a@x') reg.save() - store.set({ key: 'tokens.h1.a@x', default: '' }, 'dfoa_a') + store.write('h1', 'a@x', 'dfoa_a') const active = reg.resolveActive()! reg.forget(active, store) - expect(store.get({ key: 'tokens.h1.a@x', default: '' })).toBe('') + expect(store.read('h1', 'a@x')).toBe('') const after = Registry.load() expect(after?.hosts.h1?.accounts['a@x']).toBeUndefined() expect(after?.hosts.h1?.accounts['b@x']).toBeDefined() diff --git a/cli/src/auth/hosts.ts b/cli/src/auth/hosts.ts index 34c8c4d782e..69066131d4b 100644 --- a/cli/src/auth/hosts.ts +++ b/cli/src/auth/hosts.ts @@ -1,11 +1,14 @@ -import type { Store } from '@/store/store' +import type { StorageMode } from '@/store/store' +import type { TokenStore } from '@/store/token-store' import { z } from 'zod' import { BaseError } from '@/errors/base' import { ErrorCode } from '@/errors/codes' -import { getHostStore, tokenKey } from '@/store/manager' +import { getHostStore } from '@/store/manager' +import { STORAGE_MODES } from '@/store/store' -const StorageModeSchema = z.enum(['keychain', 'file']) -export type StorageMode = z.infer +const StorageModeSchema = z.enum(STORAGE_MODES) + +export type { StorageMode } from '@/store/store' export const AccountSchema = z.object({ id: z.string().optional(), @@ -30,7 +33,6 @@ export type ExternalSubject = z.infer export const AccountContextSchema = z.object({ account: AccountSchema, workspace: WorkspaceSchema.optional(), - available_workspaces: z.array(WorkspaceSchema).optional(), token_id: z.string().optional(), token_expires_at: z.string().optional(), external_subject: ExternalSubjectSchema.optional(), @@ -163,9 +165,9 @@ export class Registry { // Teardown for "this credential is gone": drop the token, drop the context // (unsets pointers when active), persist. Logout + self-revoke share it. - forget(active: ActiveContext, store: Store): void { + forget(active: ActiveContext, store: TokenStore): void { try { - store.unset(tokenKey(active.host, active.email)) + store.remove(active.host, active.email) } catch { /* best-effort */ } this.remove(active.host, active.email) diff --git a/cli/src/commands/_shared/authed-command.ts b/cli/src/commands/_shared/authed-command.ts index 68bd2de018b..3f57887af43 100644 --- a/cli/src/commands/_shared/authed-command.ts +++ b/cli/src/commands/_shared/authed-command.ts @@ -2,7 +2,7 @@ import type { ActiveContext } from '@/auth/hosts' import type { AppInfoCache } from '@/cache/app-info' import type { Command } from '@/framework/command' import type { HttpClient } from '@/http/types' -import type { Store } from '@/store/store' +import type { TokenStore } from '@/store/token-store' import type { IOStreams } from '@/sys/io/streams' import { META_PROBE_TIMEOUT_MS, MetaClient } from '@/api/meta' import { notLoggedInError, Registry } from '@/auth/hosts' @@ -11,7 +11,7 @@ import { loadNudgeStore } from '@/cache/nudge-store' import { getEnv } from '@/env/registry' import { formatErrorForCli } from '@/errors/format' import { createHttpClient } from '@/http/client' -import { getTokenStore, tokenKey } from '@/store/manager' +import { getTokenStore } from '@/store/manager' import { realStreams } from '@/sys/io/streams' import { hostWithScheme, openAPIBase } from '@/util/host' import { versionInfo } from '@/version/info' @@ -21,7 +21,7 @@ import { resolveRetryAttempts } from './global-flags.js' export type AuthedContext = { readonly reg: Registry readonly active: ActiveContext - readonly store: Store + readonly store: TokenStore readonly http: HttpClient readonly host: string readonly io: IOStreams @@ -44,8 +44,8 @@ export async function buildAuthedContext( if (active === undefined) fail(cmd, opts, io) - const { store } = getTokenStore() - const bearer = store.get(tokenKey(active.host, active.email)) + const store = getTokenStore(reg.token_storage) + const bearer = store.read(active.host, active.email) if (bearer === '') fail(cmd, opts, io) diff --git a/cli/src/commands/auth/devices/_shared/devices.test.ts b/cli/src/commands/auth/devices/_shared/devices.test.ts index 28a49010229..6636e2d27dc 100644 --- a/cli/src/commands/auth/devices/_shared/devices.test.ts +++ b/cli/src/commands/auth/devices/_shared/devices.test.ts @@ -2,43 +2,20 @@ import type { SessionListResponse, SessionRow } from '@dify/contracts/api/openap import type { DifyMock } from '@test/fixtures/dify-mock/server' import type { AccountSessionsClient } from '@/api/account-sessions' import type { ActiveContext } from '@/auth/hosts' -import type { Key, Store } from '@/store/store' -import { mkdtemp, rm } from 'node:fs/promises' -import { tmpdir } from 'node:os' -import { join } from 'node:path' +import { useTempConfigDir } from '@test/fixtures/config-dir' import { startMock } from '@test/fixtures/dify-mock/server' import { testHttpClient } from '@test/fixtures/http-client' +import { MemStore } from '@test/fixtures/mem-store' import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' import { Registry } from '@/auth/hosts' -import { ENV_CONFIG_DIR } from '@/store/dir' -import { tokenKey } from '@/store/manager' import { bufferStreams } from '@/sys/io/streams' import { listAllSessions, runDevicesList, runDevicesRevoke } from './devices.js' -class MemStore implements Store { - readonly entries = new Map() - get(key: Key): T { - return (this.entries.get(key.key) as T | undefined) ?? key.default - } - - set(key: Key, value: T): void { - this.entries.set(key.key, value) - } - - unset(key: Key): void { - this.entries.delete(key.key) - } -} - function buildRegistry(host: string, email: string, tokenId: string): { reg: Registry, active: ActiveContext } { const reg = Registry.empty('file') reg.upsert(host, email, { account: { id: 'acct-1', email, name: 'Test Tester' }, workspace: { id: 'ws-1', name: 'Default', role: 'owner' }, - available_workspaces: [ - { id: 'ws-1', name: 'Default', role: 'owner' }, - { id: 'ws-2', name: 'Other', role: 'normal' }, - ], token_id: tokenId, }) reg.setHost(host) @@ -82,28 +59,19 @@ describe('runDevicesList', () => { describe('runDevicesRevoke', () => { let mock: DifyMock - let configDir: string - let prevConfigDir: string | undefined + useTempConfigDir('difyctl-devrevoke-') beforeEach(async () => { mock = await startMock({ scenario: 'happy' }) - configDir = await mkdtemp(join(tmpdir(), 'difyctl-devrevoke-')) - prevConfigDir = process.env[ENV_CONFIG_DIR] - process.env[ENV_CONFIG_DIR] = configDir }) afterEach(async () => { - if (prevConfigDir === undefined) - delete process.env[ENV_CONFIG_DIR] - else - process.env[ENV_CONFIG_DIR] = prevConfigDir await mock.stop() - await rm(configDir, { recursive: true, force: true }) }) it('exact device_label: revokes one + leaves local creds', async () => { const io = bufferStreams() const store = new MemStore() const { reg, active } = buildRegistry(mock.url, 'tester@dify.ai', 'tok-1') - store.set(tokenKey(mock.url, 'tester@dify.ai'), 'dfoa_test') + store.write(mock.url, 'tester@dify.ai', 'dfoa_test') reg.save() const http = testHttpClient(mock.url, 'dfoa_test') @@ -168,7 +136,7 @@ describe('runDevicesRevoke', () => { const io = bufferStreams() const store = new MemStore() const { reg, active } = buildRegistry(mock.url, 'tester@dify.ai', 'tok-1') - store.set(tokenKey(mock.url, 'tester@dify.ai'), 'dfoa_test') + store.write(mock.url, 'tester@dify.ai', 'dfoa_test') reg.save() const http = testHttpClient(mock.url, 'dfoa_test') diff --git a/cli/src/commands/auth/devices/_shared/devices.ts b/cli/src/commands/auth/devices/_shared/devices.ts index 83d41d8fd1b..78d191e1924 100644 --- a/cli/src/commands/auth/devices/_shared/devices.ts +++ b/cli/src/commands/auth/devices/_shared/devices.ts @@ -1,7 +1,7 @@ import type { SessionRow } from '@dify/contracts/api/openapi/types.gen' import type { ActiveContext, Registry } from '@/auth/hosts' import type { HttpClient } from '@/http/types' -import type { Store } from '@/store/store' +import type { TokenStore } from '@/store/token-store' import type { IOStreams } from '@/sys/io/streams' import { AccountSessionsClient } from '@/api/account-sessions' import { BaseError } from '@/errors/base' @@ -71,7 +71,7 @@ export type DevicesRevokeOptions = { readonly io: IOStreams readonly reg: Registry readonly active: ActiveContext - readonly store: Store + readonly store: TokenStore readonly http: HttpClient readonly target?: string readonly all: boolean diff --git a/cli/src/commands/auth/login/login.test.ts b/cli/src/commands/auth/login/login.test.ts index bd8b7ef6e1f..6397f6d7414 100644 --- a/cli/src/commands/auth/login/login.test.ts +++ b/cli/src/commands/auth/login/login.test.ts @@ -1,16 +1,14 @@ import type { DifyMock } from '@test/fixtures/dify-mock/server' import type { Clock } from './device-flow.js' -import type { Key, Store } from '@/store/store' -import { mkdtemp, readFile, rm } from 'node:fs/promises' -import { tmpdir } from 'node:os' +import { readFile } from 'node:fs/promises' import { join } from 'node:path' +import { useTempConfigDir } from '@test/fixtures/config-dir' import { startMock } from '@test/fixtures/dify-mock/server' import { testHttpClient } from '@test/fixtures/http-client' +import { MemStore } from '@test/fixtures/mem-store' import { afterEach, beforeEach, describe, expect, it } from 'vitest' import { DeviceFlowApi } from '@/api/oauth-device' import { createHttpClient } from '@/http/client' -import { ENV_CONFIG_DIR } from '@/store/dir' -import { tokenKey } from '@/store/manager' import { bufferStreams } from '@/sys/io/streams' import { openAPIBase } from '@/util/host' import { runLogin } from './login.js' @@ -22,40 +20,16 @@ const noopClock: Clock = { const noopBrowser = async (): Promise => { /* skip OS open */ } -class MemStore implements Store { - readonly entries = new Map() - get(key: Key): T { - return (this.entries.get(key.key) as T | undefined) ?? key.default - } - - set(key: Key, value: T): void { - this.entries.set(key.key, value) - } - - unset(key: Key): void { - this.entries.delete(key.key) - } -} - describe('runLogin', () => { let mock: DifyMock - let configDir: string - let prevConfigDir: string | undefined + const configDir = useTempConfigDir('difyctl-login-') beforeEach(async () => { mock = await startMock({ scenario: 'happy' }) - configDir = await mkdtemp(join(tmpdir(), 'difyctl-login-')) - prevConfigDir = process.env[ENV_CONFIG_DIR] - process.env[ENV_CONFIG_DIR] = configDir }) afterEach(async () => { - if (prevConfigDir === undefined) - delete process.env[ENV_CONFIG_DIR] - else - process.env[ENV_CONFIG_DIR] = prevConfigDir await mock.stop() - await rm(configDir, { recursive: true, force: true }) }) it('happy: stores bearer + writes hosts.yml + greets account user', async () => { @@ -75,10 +49,9 @@ describe('runLogin', () => { const active = reg.resolveActive() expect(active?.ctx.account.email).toBe('tester@dify.ai') expect(active?.ctx.workspace?.id).toBe('550e8400-e29b-41d4-a716-446655440000') - expect(active?.ctx.available_workspaces).toHaveLength(2) - expect(store.get(tokenKey(active!.host, 'tester@dify.ai'))).toBe('dfoa_test') + expect(store.read(active!.host, 'tester@dify.ai')).toBe('dfoa_test') - const hostsRaw = await readFile(join(configDir, 'hosts.yml'), 'utf8') + const hostsRaw = await readFile(join(configDir(), 'hosts.yml'), 'utf8') expect(hostsRaw).toContain('current_host:') expect(hostsRaw).toContain('tester@dify.ai') expect(hostsRaw).not.toContain('dfoa_test') @@ -109,7 +82,7 @@ describe('runLogin', () => { expect(active?.ctx.external_subject?.email).toBe('sso@dify.ai') expect(active?.ctx.external_subject?.issuer).toBe('https://issuer.example') expect(active?.ctx.account.email).toBe('') - expect(store.get(tokenKey(active!.host, 'sso@dify.ai'))).toBe('dfoe_test') + expect(store.read(active!.host, 'sso@dify.ai')).toBe('dfoe_test') expect(io.outBuf()).toContain('external SSO') expect(io.outBuf()).toContain('sso@dify.ai') }) @@ -130,7 +103,7 @@ describe('runLogin', () => { browserOpener: noopBrowser, })).rejects.toThrow(/denied/) expect(store.entries.size).toBe(0) - await expect(readFile(join(configDir, 'hosts.yml'), 'utf8')).rejects.toThrow(/ENOENT/) + await expect(readFile(join(configDir(), 'hosts.yml'), 'utf8')).rejects.toThrow(/ENOENT/) }) it('expired: throws DeviceFlowError', async () => { diff --git a/cli/src/commands/auth/login/login.ts b/cli/src/commands/auth/login/login.ts index fc9562e065a..2dfb5dc0a94 100644 --- a/cli/src/commands/auth/login/login.ts +++ b/cli/src/commands/auth/login/login.ts @@ -1,7 +1,8 @@ import type { Clock } from './device-flow.js' import type { CodeResponse, PollSuccess } from '@/api/oauth-device' -import type { AccountContext, Workspace } from '@/auth/hosts' -import type { StorageMode, Store } from '@/store/store' +import type { AccountContext } from '@/auth/hosts' +import type { StorageMode } from '@/store/store' +import type { TokenStore } from '@/store/token-store' import type { ParseResult } from '@/sys/io/prompt' import type { IOStreams } from '@/sys/io/streams' import type { BrowserEnv, BrowserOpener } from '@/util/browser' @@ -11,7 +12,7 @@ import { Registry } from '@/auth/hosts' import { BaseError, isBaseError } from '@/errors/base' import { ErrorCode } from '@/errors/codes' import { createHttpClient } from '@/http/client' -import { getTokenStore, tokenKey } from '@/store/manager' +import { detectTokenStore } from '@/store/manager' import { colorEnabled, colorScheme } from '@/sys/io/color' import { promptText } from '@/sys/io/prompt' import { startSpinner } from '@/sys/io/spinner' @@ -25,7 +26,7 @@ export type LoginOptions = { readonly noBrowser?: boolean readonly insecure?: boolean readonly deviceLabel?: string - readonly store?: { readonly store: Store, readonly mode: StorageMode } + readonly store?: { readonly store: TokenStore, readonly mode: StorageMode } readonly api?: DeviceFlowApi readonly browserEnv?: BrowserEnv readonly browserOpener?: BrowserOpener @@ -69,12 +70,12 @@ export async function runLogin(opts: LoginOptions): Promise { spinner.stop() } - const storeBundle = opts.store ?? getTokenStore() + const storeBundle = opts.store ?? detectTokenStore() const display = bareHost(host) const email = accountEmail(success) const ctx = contextFromSuccess(success) - storeBundle.store.set(tokenKey(display, email), success.token) + storeBundle.store.write(display, email, success.token) const reg = Registry.load() reg.token_storage = storeBundle.mode @@ -187,9 +188,6 @@ function contextFromSuccess(s: PollSuccess): AccountContext { const def = findDefaultWorkspace(s) if (def !== undefined) ctx.workspace = def - if (s.workspaces !== undefined && s.workspaces.length > 0) { - ctx.available_workspaces = s.workspaces.map(w => ({ id: w.id, name: w.name, role: w.role })) - } return ctx } diff --git a/cli/src/commands/auth/logout/index.ts b/cli/src/commands/auth/logout/index.ts index 5da93932f24..80dc1142ccf 100644 --- a/cli/src/commands/auth/logout/index.ts +++ b/cli/src/commands/auth/logout/index.ts @@ -3,7 +3,7 @@ import type { HttpClient } from '@/http/types' import { Registry } from '@/auth/hosts' import { DifyCommand } from '@/commands/_shared/dify-command' import { createHttpClient } from '@/http/client' -import { getTokenStore, tokenKey } from '@/store/manager' +import { getTokenStore } from '@/store/manager' import { runWithSpinner } from '@/sys/io/spinner' import { realStreams } from '@/sys/io/streams' import { hostWithScheme, openAPIBase } from '@/util/host' @@ -26,7 +26,11 @@ export default class Logout extends DifyCommand { let http: HttpClient | undefined if (active !== undefined) { - const bearer = getTokenStore().store.get(tokenKey(active.host, active.email)) + let bearer = '' + try { + bearer = getTokenStore(reg.token_storage).read(active.host, active.email) + } + catch { /* keyring locked — skip remote revocation, local cleanup still runs */ } if (bearer !== '') { http = createHttpClient({ baseURL: openAPIBase(hostWithScheme(active.host, active.scheme)), bearer, retryAttempts: 0 }) } diff --git a/cli/src/commands/auth/logout/logout.test.ts b/cli/src/commands/auth/logout/logout.test.ts index 9144c2ef6cf..e1f7d84d0b4 100644 --- a/cli/src/commands/auth/logout/logout.test.ts +++ b/cli/src/commands/auth/logout/logout.test.ts @@ -1,34 +1,14 @@ -import type { Key, Store } from '@/store/store' -import { mkdtemp, readFile, rm } from 'node:fs/promises' -import { tmpdir } from 'node:os' +import { readFile } from 'node:fs/promises' import { join } from 'node:path' -import { afterEach, beforeEach, describe, expect, it } from 'vitest' +import { useTempConfigDir } from '@test/fixtures/config-dir' +import { MemStore } from '@test/fixtures/mem-store' +import { describe, expect, it } from 'vitest' import { Registry } from '@/auth/hosts' -import { ENV_CONFIG_DIR } from '@/store/dir' import { bufferStreams } from '@/sys/io/streams' import { runLogout } from './logout.js' -class MemStore implements Store { - readonly entries = new Map() - get(key: Key): T { return (this.entries.get(key.key) as T | undefined) ?? key.default } - set(key: Key, value: T): void { this.entries.set(key.key, value) } - unset(key: Key): void { this.entries.delete(key.key) } -} - describe('runLogout', () => { - let dir: string - let prev: string | undefined - beforeEach(async () => { - dir = await mkdtemp(join(tmpdir(), 'difyctl-logout-')) - prev = process.env[ENV_CONFIG_DIR] - process.env[ENV_CONFIG_DIR] = dir - }) - afterEach(async () => { - if (prev === undefined) - delete process.env[ENV_CONFIG_DIR] - else process.env[ENV_CONFIG_DIR] = prev - await rm(dir, { recursive: true, force: true }) - }) + const dir = useTempConfigDir('difyctl-logout-') function seed(store: MemStore) { const reg = Registry.empty('file') @@ -37,8 +17,8 @@ describe('runLogout', () => { reg.setHost('h1') reg.setAccount('a@x') reg.save() - store.set({ key: 'tokens.h1.a@x', default: '' }, 'dfoa_a') - store.set({ key: 'tokens.h1.b@x', default: '' }, 'dfoa_b') + store.write('h1', 'a@x', 'dfoa_a') + store.write('h1', 'b@x', 'dfoa_b') } it('removes only the active context, keeps others, unsets pointers, file survives', async () => { @@ -49,12 +29,23 @@ describe('runLogout', () => { expect(after?.hosts.h1?.accounts['a@x']).toBeUndefined() expect(after?.hosts.h1?.accounts['b@x']).toBeDefined() expect(after?.current_host).toBeUndefined() - expect(store.get({ key: 'tokens.h1.a@x', default: '' })).toBe('') - expect(store.get({ key: 'tokens.h1.b@x', default: '' })).toBe('dfoa_b') - const raw = await readFile(join(dir, 'hosts.yml'), 'utf8') + expect(store.read('h1', 'a@x')).toBe('') + expect(store.read('h1', 'b@x')).toBe('dfoa_b') + const raw = await readFile(join(dir(), 'hosts.yml'), 'utf8') expect(raw).toContain('b@x') }) + it('clears local credentials even when the store.read throws (e.g. keyring locked)', async () => { + const store = new MemStore() + seed(store) + store.read = () => { + throw new Error('keyring locked') + } + await runLogout({ io: bufferStreams(), reg: Registry.load(), store }) + const after = Registry.load() + expect(after?.hosts.h1?.accounts['a@x']).toBeUndefined() + }) + it('throws NotLoggedIn when no active context', async () => { Registry.empty('file').save() await expect(runLogout({ io: bufferStreams(), reg: Registry.load(), store: new MemStore() })) diff --git a/cli/src/commands/auth/logout/logout.ts b/cli/src/commands/auth/logout/logout.ts index 7ca414f786a..584c528f97e 100644 --- a/cli/src/commands/auth/logout/logout.ts +++ b/cli/src/commands/auth/logout/logout.ts @@ -1,9 +1,9 @@ import type { Registry } from '@/auth/hosts' import type { HttpClient } from '@/http/types' -import type { Store } from '@/store/store' +import type { TokenStore } from '@/store/token-store' import type { IOStreams } from '@/sys/io/streams' import { AccountSessionsClient } from '@/api/account-sessions' -import { getTokenStore, tokenKey } from '@/store/manager' +import { getTokenStore } from '@/store/manager' import { colorEnabled, colorScheme } from '@/sys/io/color' export type LogoutOptions = { @@ -11,7 +11,7 @@ export type LogoutOptions = { readonly reg: Registry readonly http?: HttpClient /** Optional override for tests; production resolves via `getTokenStore`. */ - readonly store?: Store + readonly store?: TokenStore } const REVOCABLE_PREFIXES = ['dfoa_', 'dfoe_'] as const @@ -21,8 +21,12 @@ export async function runLogout(opts: LogoutOptions): Promise { const reg = opts.reg const active = reg.requireActive() - const store = opts.store ?? getTokenStore().store - const bearer = store.get(tokenKey(active.host, active.email)) + const store = opts.store ?? getTokenStore(reg.token_storage) + let bearer = '' + try { + bearer = store.read(active.host, active.email) + } + catch { /* keyring locked — skip remote revocation, local cleanup still runs */ } let revokeWarning = '' if (bearer !== '' && revokeAllowed(bearer) && opts.http !== undefined) { diff --git a/cli/src/commands/config/get/run.test.ts b/cli/src/commands/config/get/run.test.ts index 2246b111afe..6199706cb2f 100644 --- a/cli/src/commands/config/get/run.test.ts +++ b/cli/src/commands/config/get/run.test.ts @@ -1,30 +1,12 @@ -import { mkdtemp, rm } from 'node:fs/promises' -import { tmpdir } from 'node:os' -import { join } from 'node:path' -import { afterEach, beforeEach, describe, expect, it } from 'vitest' +import { useTempConfigDir } from '@test/fixtures/config-dir' +import { describe, expect, it } from 'vitest' import { isBaseError } from '@/errors/base' import { ErrorCode } from '@/errors/codes' -import { ENV_CONFIG_DIR } from '@/store/dir' import { getConfigurationStore } from '@/store/manager' import { runConfigGet } from './run' describe('runConfigGet', () => { - let dir: string - let prevConfigDir: string | undefined - - beforeEach(async () => { - dir = await mkdtemp(join(tmpdir(), 'difyctl-get-')) - prevConfigDir = process.env[ENV_CONFIG_DIR] - process.env[ENV_CONFIG_DIR] = dir - }) - - afterEach(async () => { - if (prevConfigDir === undefined) - delete process.env[ENV_CONFIG_DIR] - else - process.env[ENV_CONFIG_DIR] = prevConfigDir - await rm(dir, { recursive: true, force: true }) - }) + useTempConfigDir('difyctl-get-') it('returns set value with trailing newline', () => { getConfigurationStore().setTyped({ diff --git a/cli/src/commands/config/set/run.test.ts b/cli/src/commands/config/set/run.test.ts index 6185dd87801..404218d156d 100644 --- a/cli/src/commands/config/set/run.test.ts +++ b/cli/src/commands/config/set/run.test.ts @@ -1,31 +1,13 @@ -import { mkdtemp, rm } from 'node:fs/promises' -import { tmpdir } from 'node:os' -import { join } from 'node:path' -import { afterEach, beforeEach, describe, expect, it } from 'vitest' +import { useTempConfigDir } from '@test/fixtures/config-dir' +import { describe, expect, it } from 'vitest' import { loadConfig } from '@/config/config-loader' import { isBaseError } from '@/errors/base' import { ErrorCode, ExitCode } from '@/errors/codes' -import { ENV_CONFIG_DIR } from '@/store/dir' import { getConfigurationStore } from '@/store/manager' import { runConfigSet } from './run' describe('runConfigSet', () => { - let dir: string - let prevConfigDir: string | undefined - - beforeEach(async () => { - dir = await mkdtemp(join(tmpdir(), 'difyctl-set-')) - prevConfigDir = process.env[ENV_CONFIG_DIR] - process.env[ENV_CONFIG_DIR] = dir - }) - - afterEach(async () => { - if (prevConfigDir === undefined) - delete process.env[ENV_CONFIG_DIR] - else - process.env[ENV_CONFIG_DIR] = prevConfigDir - await rm(dir, { recursive: true, force: true }) - }) + useTempConfigDir('difyctl-set-') it('persists the value and returns "set k = v\\n"', () => { const out = runConfigSet({ store: getConfigurationStore(), key: 'defaults.format', value: 'json' }) diff --git a/cli/src/commands/config/unset/run.test.ts b/cli/src/commands/config/unset/run.test.ts index 60444578413..22bb736c92d 100644 --- a/cli/src/commands/config/unset/run.test.ts +++ b/cli/src/commands/config/unset/run.test.ts @@ -1,31 +1,13 @@ -import { mkdtemp, rm } from 'node:fs/promises' -import { tmpdir } from 'node:os' -import { join } from 'node:path' -import { afterEach, beforeEach, describe, expect, it } from 'vitest' +import { useTempConfigDir } from '@test/fixtures/config-dir' +import { describe, expect, it } from 'vitest' import { loadConfig } from '@/config/config-loader' import { isBaseError } from '@/errors/base' import { ErrorCode } from '@/errors/codes' -import { ENV_CONFIG_DIR } from '@/store/dir' import { getConfigurationStore } from '@/store/manager' import { runConfigUnset } from './run' describe('runConfigUnset', () => { - let dir: string - let prevConfigDir: string | undefined - - beforeEach(async () => { - dir = await mkdtemp(join(tmpdir(), 'difyctl-unset-')) - prevConfigDir = process.env[ENV_CONFIG_DIR] - process.env[ENV_CONFIG_DIR] = dir - }) - - afterEach(async () => { - if (prevConfigDir === undefined) - delete process.env[ENV_CONFIG_DIR] - else - process.env[ENV_CONFIG_DIR] = prevConfigDir - await rm(dir, { recursive: true, force: true }) - }) + useTempConfigDir('difyctl-unset-') it('clears the requested key, leaves others intact', () => { getConfigurationStore().setTyped({ diff --git a/cli/src/commands/config/view/run.test.ts b/cli/src/commands/config/view/run.test.ts index 45e3aff8ca3..017b0376ef4 100644 --- a/cli/src/commands/config/view/run.test.ts +++ b/cli/src/commands/config/view/run.test.ts @@ -1,28 +1,10 @@ -import { mkdtemp, rm } from 'node:fs/promises' -import { tmpdir } from 'node:os' -import { join } from 'node:path' -import { afterEach, beforeEach, describe, expect, it } from 'vitest' -import { ENV_CONFIG_DIR } from '@/store/dir' +import { useTempConfigDir } from '@test/fixtures/config-dir' +import { describe, expect, it } from 'vitest' import { getConfigurationStore } from '@/store/manager' import { runConfigView } from './run' describe('runConfigView', () => { - let dir: string - let prevConfigDir: string | undefined - - beforeEach(async () => { - dir = await mkdtemp(join(tmpdir(), 'difyctl-view-')) - prevConfigDir = process.env[ENV_CONFIG_DIR] - process.env[ENV_CONFIG_DIR] = dir - }) - - afterEach(async () => { - if (prevConfigDir === undefined) - delete process.env[ENV_CONFIG_DIR] - else - process.env[ENV_CONFIG_DIR] = prevConfigDir - await rm(dir, { recursive: true, force: true }) - }) + useTempConfigDir('difyctl-view-') it('text format: empty config returns empty string', () => { const out = runConfigView({ store: getConfigurationStore() }) diff --git a/cli/src/commands/create/member/run.test.ts b/cli/src/commands/create/member/run.test.ts index f3fc75a84b5..6c363691dc6 100644 --- a/cli/src/commands/create/member/run.test.ts +++ b/cli/src/commands/create/member/run.test.ts @@ -11,7 +11,6 @@ function active(): ActiveContext { ctx: { account: { id: 'acct-1', email: 'inviter@example.com', name: 'Inviter' }, workspace: { id: '550e8400-e29b-41d4-a716-446655440000', name: 'Default', role: 'owner' }, - available_workspaces: [{ id: '550e8400-e29b-41d4-a716-446655440000', name: 'Default', role: 'owner' }], }, } } diff --git a/cli/src/commands/delete/member/run.test.ts b/cli/src/commands/delete/member/run.test.ts index 3476c2bb6c7..bbc58ff2c3f 100644 --- a/cli/src/commands/delete/member/run.test.ts +++ b/cli/src/commands/delete/member/run.test.ts @@ -11,7 +11,6 @@ function active(): ActiveContext { ctx: { account: { id: 'acct-1', email: 'me@example.com', name: 'Me' }, workspace: { id: '550e8400-e29b-41d4-a716-446655440000', name: 'Default', role: 'owner' }, - available_workspaces: [{ id: '550e8400-e29b-41d4-a716-446655440000', name: 'Default', role: 'owner' }], }, } } diff --git a/cli/src/commands/describe/app/run.test.ts b/cli/src/commands/describe/app/run.test.ts index d99519bbe34..4ed7cedc7a5 100644 --- a/cli/src/commands/describe/app/run.test.ts +++ b/cli/src/commands/describe/app/run.test.ts @@ -19,10 +19,6 @@ function active(): ActiveContext { ctx: { account: { id: 'acct-1', email: 't@d.ai', name: 'T' }, workspace: { id: 'ws-1', name: 'Default', role: 'owner' }, - available_workspaces: [ - { id: 'ws-1', name: 'Default', role: 'owner' }, - { id: 'ws-2', name: 'Other', role: 'normal' }, - ], }, } } @@ -71,7 +67,7 @@ describe('runDescribeApp', () => { }) it('text: agent app shows Agent: true', async () => { - const out = await render({ appId: 'app-4', workspace: 'ws-2' }) + const out = await render({ appId: 'app-4', workspace: '00000000-0000-0000-0000-000000000002' }) expect(out).toContain('Agent:') expect(out).toContain('true') }) diff --git a/cli/src/commands/export/app/run.test.ts b/cli/src/commands/export/app/run.test.ts index a0c2cd80232..d3c1a9a83d9 100644 --- a/cli/src/commands/export/app/run.test.ts +++ b/cli/src/commands/export/app/run.test.ts @@ -9,13 +9,14 @@ import { afterEach, beforeEach, describe, expect, it } from 'vitest' import { bufferStreams } from '@/sys/io/streams' import { runExportApp } from './run.js' +const WS_ID = 'aaaaaaaa-0000-0000-0000-000000000001' + const baseActive: ActiveContext = { host: '127.0.0.1', email: 'tester@dify.ai', ctx: { account: { id: 'acct-1', email: 'tester@dify.ai', name: 'Test Tester' }, - workspace: { id: 'ws-1', name: 'Default', role: 'owner' }, - available_workspaces: [{ id: 'ws-1', name: 'Default', role: 'owner' }], + workspace: { id: WS_ID, name: 'Default', role: 'owner' }, }, scheme: 'http', } diff --git a/cli/src/commands/get/app/run.test.ts b/cli/src/commands/get/app/run.test.ts index 9f11e30c488..7c0cc76c009 100644 --- a/cli/src/commands/get/app/run.test.ts +++ b/cli/src/commands/get/app/run.test.ts @@ -13,10 +13,6 @@ const baseActive: ActiveContext = { ctx: { account: { id: 'acct-1', email: 'tester@dify.ai', name: 'Test Tester' }, workspace: { id: '550e8400-e29b-41d4-a716-446655440000', name: 'Default', role: 'owner' }, - available_workspaces: [ - { id: '550e8400-e29b-41d4-a716-446655440000', name: 'Default', role: 'owner' }, - { id: '550e8400-e29b-41d4-a716-446655440001', name: 'Other', role: 'normal' }, - ], }, scheme: 'http', } diff --git a/cli/src/commands/get/app/run.ts b/cli/src/commands/get/app/run.ts index 940f0ac44e3..5c061d2548f 100644 --- a/cli/src/commands/get/app/run.ts +++ b/cli/src/commands/get/app/run.ts @@ -114,14 +114,7 @@ function describeToEnvelope(desc: AppDescribeResponse, wsId: string, wsName: str function workspaceNameForId(active: ActiveContext, id: string): string { if (id === '') return '' - const ctx = active.ctx - if (ctx.workspace?.id === id) - return ctx.workspace.name - for (const w of ctx.available_workspaces ?? []) { - if (w.id === id) - return w.name - } - return '' + return active.ctx.workspace?.id === id ? active.ctx.workspace.name : '' } async function runAllWorkspaces( diff --git a/cli/src/commands/get/member/run.test.ts b/cli/src/commands/get/member/run.test.ts index 1dc2da2a364..6993e6905e1 100644 --- a/cli/src/commands/get/member/run.test.ts +++ b/cli/src/commands/get/member/run.test.ts @@ -12,7 +12,6 @@ function active(): ActiveContext { ctx: { account: { id: 'acct-1', email: 'me@example.com', name: 'Me' }, workspace: { id: '550e8400-e29b-41d4-a716-446655440000', name: 'Default', role: 'owner' }, - available_workspaces: [{ id: '550e8400-e29b-41d4-a716-446655440000', name: 'Default', role: 'owner' }], }, } } diff --git a/cli/src/commands/get/workspace/handlers.test.ts b/cli/src/commands/get/workspace/handlers.test.ts index 8d963632575..bfab5321d30 100644 --- a/cli/src/commands/get/workspace/handlers.test.ts +++ b/cli/src/commands/get/workspace/handlers.test.ts @@ -6,7 +6,7 @@ function env(): WorkspaceListResponse { return { workspaces: [ { id: 'ws-1', name: 'Default', role: 'owner', status: 'normal', current: true }, - { id: 'ws-2', name: 'Other', role: 'normal', status: 'normal', current: false }, + { id: '00000000-0000-0000-0000-000000000002', name: 'Other', role: 'normal', status: 'normal', current: false }, ], } } diff --git a/cli/src/commands/get/workspace/run.test.ts b/cli/src/commands/get/workspace/run.test.ts index 7dd36e935aa..cf5238af3a4 100644 --- a/cli/src/commands/get/workspace/run.test.ts +++ b/cli/src/commands/get/workspace/run.test.ts @@ -13,10 +13,6 @@ const baseActive: ActiveContext = { ctx: { account: { id: 'acct-1', email: 'tester@dify.ai', name: 'Test Tester' }, workspace: { id: '550e8400-e29b-41d4-a716-446655440000', name: 'Default', role: 'owner' }, - available_workspaces: [ - { id: '550e8400-e29b-41d4-a716-446655440000', name: 'Default', role: 'owner' }, - { id: '550e8400-e29b-41d4-a716-446655440001', name: 'Other', role: 'normal' }, - ], }, scheme: 'http', } diff --git a/cli/src/commands/import/app/run.test.ts b/cli/src/commands/import/app/run.test.ts index 562f2a207da..c37e00c5376 100644 --- a/cli/src/commands/import/app/run.test.ts +++ b/cli/src/commands/import/app/run.test.ts @@ -13,13 +13,14 @@ import { bufferStreams } from '@/sys/io/streams' import { ZERO } from '@/util/uuid.js' import { pluginDependencyLabel, runImportApp } from './run.js' +const WS_ID = 'aaaaaaaa-0000-0000-0000-000000000001' + const baseActive: ActiveContext = { host: '127.0.0.1', email: 'tester@dify.ai', ctx: { account: { id: 'acct-1', email: 'tester@dify.ai', name: 'Test Tester' }, - workspace: { id: 'ws-1', name: 'Default', role: 'owner' }, - available_workspaces: [{ id: 'ws-1', name: 'Default', role: 'owner' }], + workspace: { id: WS_ID, name: 'Default', role: 'owner' }, }, scheme: 'http', } diff --git a/cli/src/commands/run/app/run.test.ts b/cli/src/commands/run/app/run.test.ts index 92184590d2d..4fff2d02873 100644 --- a/cli/src/commands/run/app/run.test.ts +++ b/cli/src/commands/run/app/run.test.ts @@ -20,10 +20,6 @@ function active(): ActiveContext { ctx: { account: { id: 'acct-1', email: 't@d.ai', name: 'T' }, workspace: { id: 'ws-1', name: 'Default', role: 'owner' }, - available_workspaces: [ - { id: 'ws-1', name: 'Default', role: 'owner' }, - { id: 'ws-2', name: 'Other', role: 'normal' }, - ], }, } } @@ -139,7 +135,7 @@ describe('runApp', () => { const io = bufferStreams() const cache = await loadAppInfoCache({ store: getCache(CACHE_APP_INFO) }) await runApp( - { appId: 'app-4', workspace: 'ws-2', message: 'do research' }, + { appId: 'app-4', workspace: '00000000-0000-0000-0000-000000000002', message: 'do research' }, { active: active(), http: testHttpClient(mock.url, 'dfoa_test'), host: mock.url, io, cache }, ) expect(io.outBuf()).toContain('do research') @@ -150,7 +146,7 @@ describe('runApp', () => { const io = bufferStreams() const cache = await loadAppInfoCache({ store: getCache(CACHE_APP_INFO) }) await runApp( - { appId: 'app-4', workspace: 'ws-2', message: 'go', stream: true }, + { appId: 'app-4', workspace: '00000000-0000-0000-0000-000000000002', message: 'go', stream: true }, { active: active(), http: testHttpClient(mock.url, 'dfoa_test'), host: mock.url, io, cache }, ) expect(io.outBuf()).toContain('go') diff --git a/cli/src/commands/set/member/run.test.ts b/cli/src/commands/set/member/run.test.ts index 1afac76c6c7..2e86a2afe09 100644 --- a/cli/src/commands/set/member/run.test.ts +++ b/cli/src/commands/set/member/run.test.ts @@ -11,7 +11,6 @@ function active(): ActiveContext { ctx: { account: { id: 'acct-1', email: 'me@example.com', name: 'Me' }, workspace: { id: '550e8400-e29b-41d4-a716-446655440000', name: 'Default', role: 'owner' }, - available_workspaces: [{ id: '550e8400-e29b-41d4-a716-446655440000', name: 'Default', role: 'owner' }], }, } } diff --git a/cli/src/commands/use/account/use-account.test.ts b/cli/src/commands/use/account/use-account.test.ts index 9fa6d506d1e..2af64e1727c 100644 --- a/cli/src/commands/use/account/use-account.test.ts +++ b/cli/src/commands/use/account/use-account.test.ts @@ -1,29 +1,13 @@ -import type { Key, Store } from '@/store/store' -import { mkdtemp, rm } from 'node:fs/promises' -import { tmpdir } from 'node:os' -import { join } from 'node:path' -import { afterEach, beforeEach, describe, expect, it } from 'vitest' +import { useTempConfigDir } from '@test/fixtures/config-dir' +import { MemStore } from '@test/fixtures/mem-store' +import { beforeEach, describe, expect, it } from 'vitest' import { Registry } from '@/auth/hosts' -import { ENV_CONFIG_DIR } from '@/store/dir' import { bufferStreams } from '@/sys/io/streams' import { runUseAccount } from './use-account' -function memStore(seed: Record): Store { - const m = new Map(Object.entries(seed)) - return { - get(k: Key): T { return (m.get(k.key) as T | undefined) ?? k.default }, - set(k: Key, v: T): void { m.set(k.key, v) }, - unset(k: Key): void { m.delete(k.key) }, - } -} - describe('runUseAccount', () => { - let dir: string - let prev: string | undefined - beforeEach(async () => { - dir = await mkdtemp(join(tmpdir(), 'difyctl-useacct-')) - prev = process.env[ENV_CONFIG_DIR] - process.env[ENV_CONFIG_DIR] = dir + useTempConfigDir('difyctl-useacct-') + beforeEach(() => { const reg = Registry.empty('file') reg.upsert('h1', 'a@x', { account: { id: '1', email: 'a@x', name: 'A' } }) reg.upsert('h1', 'b@x', { account: { id: '2', email: 'b@x', name: 'B' } }) @@ -31,26 +15,20 @@ describe('runUseAccount', () => { reg.setAccount('a@x') reg.save() }) - afterEach(async () => { - if (prev === undefined) - delete process.env[ENV_CONFIG_DIR] - else process.env[ENV_CONFIG_DIR] = prev - await rm(dir, { recursive: true, force: true }) - }) it('switches current_account when email valid + token present', async () => { - await runUseAccount({ io: bufferStreams(), email: 'b@x', store: memStore({ 'tokens.h1.b@x': 'dfoa_b' }) }) + await runUseAccount({ io: bufferStreams(), email: 'b@x', store: new MemStore({ 'h1 b@x': 'dfoa_b' }) }) expect(Registry.load().hosts.h1?.current_account).toBe('b@x') }) it('errors when the account has no stored token', async () => { - await expect(runUseAccount({ io: bufferStreams(), email: 'b@x', store: memStore({}) })) + await expect(runUseAccount({ io: bufferStreams(), email: 'b@x', store: new MemStore({}) })) .rejects .toThrow(/log in|no credential/i) }) it('errors when the email is unknown on the current host', async () => { - await expect(runUseAccount({ io: bufferStreams(), email: 'z@x', store: memStore({ 'tokens.h1.z@x': 'x' }) })) + await expect(runUseAccount({ io: bufferStreams(), email: 'z@x', store: new MemStore({ 'h1 z@x': 'x' }) })) .rejects .toThrow(/unknown account|no account/i) }) @@ -58,6 +36,6 @@ describe('runUseAccount', () => { it('errors in non-TTY when email omitted', async () => { const io = bufferStreams() ;(io as { isErrTTY: boolean }).isErrTTY = false - await expect(runUseAccount({ io, email: undefined, store: memStore({}) })).rejects.toThrow(/--email/i) + await expect(runUseAccount({ io, email: undefined, store: new MemStore({}) })).rejects.toThrow(/--email/i) }) }) diff --git a/cli/src/commands/use/account/use-account.ts b/cli/src/commands/use/account/use-account.ts index 3fdb8d455bd..63af4c20dff 100644 --- a/cli/src/commands/use/account/use-account.ts +++ b/cli/src/commands/use/account/use-account.ts @@ -1,10 +1,10 @@ import type { HostEntry } from '@/auth/hosts' -import type { Store } from '@/store/store' +import type { TokenStore } from '@/store/token-store' import type { IOStreams } from '@/sys/io/streams' import { notLoggedInError, Registry } from '@/auth/hosts' import { BaseError } from '@/errors/base' import { ErrorCode } from '@/errors/codes' -import { getTokenStore, tokenKey } from '@/store/manager' +import { getTokenStore } from '@/store/manager' import { colorEnabled, colorScheme } from '@/sys/io/color' import { selectFromList } from '@/sys/io/select' @@ -12,7 +12,7 @@ export type UseAccountOptions = { readonly io: IOStreams readonly email: string | undefined /** Optional override for tests; production resolves via `getTokenStore`. */ - readonly store?: Store + readonly store?: TokenStore } type AccountChoice = { email: string, name: string, sso: boolean, active: boolean } @@ -38,8 +38,8 @@ export async function runUseAccount(opts: UseAccountOptions): Promise { }) } - const store = opts.store ?? getTokenStore().store - if (store.get(tokenKey(host, target)) === '') { + const store = opts.store ?? getTokenStore(reg.token_storage) + if (store.read(host, target) === '') { throw new BaseError({ code: ErrorCode.NotLoggedIn, message: `no credential stored for ${target} on ${host}`, diff --git a/cli/src/commands/use/host/use-host.test.ts b/cli/src/commands/use/host/use-host.test.ts index 5796d5b4a00..e40a34e811b 100644 --- a/cli/src/commands/use/host/use-host.test.ts +++ b/cli/src/commands/use/host/use-host.test.ts @@ -1,19 +1,12 @@ -import { mkdtemp, rm } from 'node:fs/promises' -import { tmpdir } from 'node:os' -import { join } from 'node:path' -import { afterEach, beforeEach, describe, expect, it } from 'vitest' +import { useTempConfigDir } from '@test/fixtures/config-dir' +import { beforeEach, describe, expect, it } from 'vitest' import { Registry } from '@/auth/hosts' -import { ENV_CONFIG_DIR } from '@/store/dir' import { bufferStreams } from '@/sys/io/streams' import { runUseHost } from './use-host' describe('runUseHost', () => { - let dir: string - let prev: string | undefined - beforeEach(async () => { - dir = await mkdtemp(join(tmpdir(), 'difyctl-usehost-')) - prev = process.env[ENV_CONFIG_DIR] - process.env[ENV_CONFIG_DIR] = dir + useTempConfigDir('difyctl-usehost-') + beforeEach(() => { const reg = Registry.empty('file') reg.upsert('h1', 'a@x', { account: { id: '1', email: 'a@x', name: 'A' } }) reg.upsert('h2', 'b@x', { account: { id: '2', email: 'b@x', name: 'B' } }) @@ -21,12 +14,6 @@ describe('runUseHost', () => { reg.setAccount('a@x') reg.save() }) - afterEach(async () => { - if (prev === undefined) - delete process.env[ENV_CONFIG_DIR] - else process.env[ENV_CONFIG_DIR] = prev - await rm(dir, { recursive: true, force: true }) - }) it('switches current_host when host is valid', async () => { await runUseHost({ io: bufferStreams(), host: 'h2' }) diff --git a/cli/src/commands/use/workspace/index.ts b/cli/src/commands/use/workspace/index.ts index 805d7654235..9df7c5ffd76 100644 --- a/cli/src/commands/use/workspace/index.ts +++ b/cli/src/commands/use/workspace/index.ts @@ -5,16 +5,17 @@ import { Args } from '@/framework/flags' import { runUseWorkspace } from './use' export default class UseWorkspace extends DifyCommand { - static override description = 'Switch the active workspace on the server and refresh hosts.yml' + static override description = 'Switch the active workspace on the server (omit the id to pick interactively)' static override effect: CommandEffect = 'write' static override examples = [ '<%= config.bin %> use workspace ws-abc123', + '<%= config.bin %> use workspace', ] static override args = { - workspaceId: Args.string({ description: 'workspace id to switch to', required: true }), + workspaceId: Args.string({ description: 'workspace id to switch to (omit to pick interactively)', required: false }), } static override flags = { diff --git a/cli/src/commands/use/workspace/use.test.ts b/cli/src/commands/use/workspace/use.test.ts index 33c629d83d1..2e2ad6b8a83 100644 --- a/cli/src/commands/use/workspace/use.test.ts +++ b/cli/src/commands/use/workspace/use.test.ts @@ -4,24 +4,24 @@ import type { } from '@dify/contracts/api/openapi/types.gen' import type { ActiveContext } from '@/auth/hosts' import type { HttpClient } from '@/http/types' -import { mkdtemp, rm } from 'node:fs/promises' -import { tmpdir } from 'node:os' -import { join } from 'node:path' -import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import { useTempConfigDir } from '@test/fixtures/config-dir' +import { beforeEach, describe, expect, it, vi } from 'vitest' import { Registry } from '@/auth/hosts' -import { ENV_CONFIG_DIR } from '@/store/dir' +import { selectFromList } from '@/sys/io/select' import { bufferStreams } from '@/sys/io/streams' import { runUseWorkspace } from './use.js' +vi.mock('@/sys/io/select', () => ({ + selectFromList: vi.fn(), +})) + +const selectFromListMock = vi.mocked(selectFromList) + function makeRegistry(): Registry { const reg = Registry.empty('file') reg.upsert('cloud.dify.ai', 'tester@dify.ai', { account: { id: 'acct-1', email: 'tester@dify.ai', name: 'Tester' }, workspace: { id: 'ws-1', name: 'Default', role: 'owner' }, - available_workspaces: [ - { id: 'ws-1', name: 'Default', role: 'owner' }, - { id: 'ws-2', name: 'Stale Name', role: 'normal' }, - ], }) reg.setHost('cloud.dify.ai') reg.setAccount('tester@dify.ai') @@ -35,46 +35,41 @@ function makeActive(reg: Registry): ActiveContext { return active } +function makeDetail(over: Partial = {}): WorkspaceDetailResponse { + return { + id: '00000000-0000-0000-0000-000000000002', + name: 'Two', + role: 'owner', + status: 'normal', + current: true, + created_at: '2026-05-18T00:00:00Z', + ...over, + } +} + function fakeClient(opts: { switch?: () => Promise list?: () => Promise }) { return { - switch: vi.fn(opts.switch ?? (() => Promise.resolve({ - id: 'ws-2', - name: 'Switched', - role: 'normal', - status: 'normal', - current: true, - created_at: '2026-05-18T00:00:00Z', - }))), + switch: vi.fn(opts.switch ?? (() => Promise.resolve(makeDetail()))), list: vi.fn(opts.list ?? (() => Promise.resolve({ workspaces: [ - { id: 'ws-1', name: 'Default', role: 'owner', status: 'normal', current: false }, - { id: 'ws-2', name: 'Switched', role: 'normal', status: 'normal', current: true }, + { id: 'ws-1', name: 'Default', role: 'owner', status: 'normal', current: true }, + { id: '00000000-0000-0000-0000-000000000002', name: 'Two', role: 'owner', status: 'normal', current: false }, ], }))), } } describe('runUseWorkspace', () => { - let configDir: string + useTempConfigDir('difyctl-use-workspace-') - let prevConfigDir: string | undefined - beforeEach(async () => { - configDir = await mkdtemp(join(tmpdir(), 'difyctl-use-workspace-')) - prevConfigDir = process.env[ENV_CONFIG_DIR] - process.env[ENV_CONFIG_DIR] = configDir - }) - afterEach(async () => { - if (prevConfigDir === undefined) - delete process.env[ENV_CONFIG_DIR] - else - process.env[ENV_CONFIG_DIR] = prevConfigDir - await rm(configDir, { recursive: true, force: true }) + beforeEach(() => { + selectFromListMock.mockReset() }) - it('happy path: POST /switch → GET /workspaces → write hosts.yml', async () => { + it('arg path: switches directly without listing and persists only the active workspace', async () => { const io = bufferStreams() const reg = makeRegistry() reg.save() @@ -82,7 +77,7 @@ describe('runUseWorkspace', () => { const client = fakeClient({}) const next = await runUseWorkspace( - { workspaceId: 'ws-2' }, + { workspaceId: '00000000-0000-0000-0000-000000000002' }, { reg, active, @@ -92,62 +87,42 @@ describe('runUseWorkspace', () => { }, ) - expect(client.switch).toHaveBeenCalledExactlyOnceWith('ws-2') - expect(client.list).toHaveBeenCalledOnce() + expect(client.switch).toHaveBeenCalledExactlyOnceWith('00000000-0000-0000-0000-000000000002') + expect(client.list).not.toHaveBeenCalled() const activeCtx = next.resolveActive() - expect(activeCtx?.ctx.workspace).toEqual({ id: 'ws-2', name: 'Switched', role: 'normal' }) - expect(activeCtx?.ctx.available_workspaces).toEqual([ - { id: 'ws-1', name: 'Default', role: 'owner' }, - { id: 'ws-2', name: 'Switched', role: 'normal' }, - ]) + expect(activeCtx?.ctx.workspace).toEqual({ id: '00000000-0000-0000-0000-000000000002', name: 'Two', role: 'owner' }) + expect((activeCtx?.ctx as Record | undefined)?.available_workspaces).toBeUndefined() const reloaded = Registry.load() const reloadedActive = reloaded?.resolveActive() - expect(reloadedActive?.ctx.workspace?.id).toBe('ws-2') - expect(reloadedActive?.ctx.workspace?.name).toBe('Switched') + expect(reloadedActive?.ctx.workspace?.id).toBe('00000000-0000-0000-0000-000000000002') + expect(reloadedActive?.ctx.workspace?.name).toBe('Two') + expect((reloadedActive?.ctx as Record | undefined)?.available_workspaces).toBeUndefined() - expect(io.outBuf()).toMatch(/Switched to Switched \(ws-2\)/) + expect(io.outBuf()).toMatch(/Switched to Two \(00000000-0000-0000-0000-000000000002\)/) }) - it('hosts.yml contains no bearer after switch', async () => { + it('no-arg + no-TTY: rejects with usage_missing_arg and never switches', async () => { const io = bufferStreams() + io.isErrTTY = false const reg = makeRegistry() reg.save() const active = makeActive(reg) const client = fakeClient({}) - await runUseWorkspace( - { workspaceId: 'ws-2' }, - { reg, active, http: {} as HttpClient, io, workspacesFactory: () => client as never }, - ) + await expect( + runUseWorkspace( + { workspaceId: undefined }, + { reg, active, http: {} as HttpClient, io, workspacesFactory: () => client as never }, + ), + ).rejects.toMatchObject({ code: 'usage_missing_arg' }) - const reloaded = Registry.load() - const raw = JSON.stringify(reloaded) - expect(raw).not.toMatch(/bearer/) + expect(client.switch).not.toHaveBeenCalled() + expect(client.list).not.toHaveBeenCalled() }) - it('refreshes stale workspace name from server', async () => { - // registry has ws-2 named "Stale Name"; server returns "Switched". - // We expect saveRegistry to record the fresh name from the server. - const io = bufferStreams() - const reg = makeRegistry() - reg.save() - const active = makeActive(reg) - const client = fakeClient({}) - - await runUseWorkspace( - { workspaceId: 'ws-2' }, - { reg, active, http: {} as HttpClient, io, workspacesFactory: () => client as never }, - ) - - const reloaded = Registry.load() - const reloadedActive = reloaded?.resolveActive() - expect(reloadedActive?.ctx.workspace?.name).toBe('Switched') - expect(reloadedActive?.ctx.available_workspaces?.find(w => w.id === 'ws-2')?.name).toBe('Switched') - }) - - it('does NOT mutate hosts.yml when POST /switch fails', async () => { + it('switch failure: rejects and leaves the active workspace untouched', async () => { const io = bufferStreams() const reg = makeRegistry() reg.save() @@ -160,85 +135,41 @@ describe('runUseWorkspace', () => { await expect( runUseWorkspace( - { workspaceId: 'ws-2' }, - { - reg, - active, - http: {} as HttpClient, - io, - workspacesFactory: () => client as never, - }, + { workspaceId: '00000000-0000-0000-0000-000000000002' }, + { reg, active, http: {} as HttpClient, io, workspacesFactory: () => client as never }, ), ).rejects.toThrow(/forbidden/) - expect(client.list).not.toHaveBeenCalled() const after = Registry.load() expect(after).toEqual(before) - const afterActive = after?.resolveActive() - expect(afterActive?.ctx.workspace?.id).toBe('ws-1') + expect(after?.resolveActive()?.ctx.workspace?.id).toBe('ws-1') }) - it('does NOT mutate hosts.yml when GET /workspaces fails after switch', async () => { + it('picker path (TTY): lists live workspaces and switches to the selected one', async () => { const io = bufferStreams() + io.isErrTTY = true const reg = makeRegistry() reg.save() const active = makeActive(reg) - const before = Registry.load() + const client = fakeClient({}) - const client = fakeClient({ - list: () => Promise.reject(new Error('transient list failure')), - }) + selectFromListMock.mockResolvedValue({ id: '00000000-0000-0000-0000-000000000002', name: 'Two', role: 'owner' }) - await expect( - runUseWorkspace( - { workspaceId: 'ws-2' }, - { - reg, - active, - http: {} as HttpClient, - io, - workspacesFactory: () => client as never, - }, - ), - ).rejects.toThrow(/transient list failure/) + await runUseWorkspace( + { workspaceId: undefined }, + { reg, active, http: {} as HttpClient, io, workspacesFactory: () => client as never }, + ) - const after = Registry.load() - expect(after).toEqual(before) - }) + expect(client.list).toHaveBeenCalledOnce() + expect(selectFromListMock).toHaveBeenCalledOnce() + const passed = selectFromListMock.mock.calls[0]![0] + expect(passed.items).toEqual([ + { id: 'ws-1', name: 'Default', role: 'owner' }, + { id: '00000000-0000-0000-0000-000000000002', name: 'Two', role: 'owner' }, + ]) + expect(client.switch).toHaveBeenCalledExactlyOnceWith('00000000-0000-0000-0000-000000000002') - it('throws when server returns switch= but id is missing from /workspaces list', async () => { - const io = bufferStreams() - const reg = makeRegistry() - reg.save() - const active = makeActive(reg) - - const client = fakeClient({ - switch: () => Promise.resolve({ - id: 'ws-7', - name: 'Ghost', - role: 'normal', - status: 'normal', - current: true, - created_at: null as unknown as string, - }), - list: () => Promise.resolve({ - workspaces: [ - { id: 'ws-1', name: 'Default', role: 'owner', status: 'normal', current: false }, - ], - }), - }) - - await expect( - runUseWorkspace( - { workspaceId: 'ws-7' }, - { - reg, - active, - http: {} as HttpClient, - io, - workspacesFactory: () => client as never, - }, - ), - ).rejects.toThrow(/not visible in \/workspaces/) + const reloadedActive = Registry.load()?.resolveActive() + expect(reloadedActive?.ctx.workspace?.id).toBe('00000000-0000-0000-0000-000000000002') }) }) diff --git a/cli/src/commands/use/workspace/use.ts b/cli/src/commands/use/workspace/use.ts index daf0bb7e291..5371ace8eb3 100644 --- a/cli/src/commands/use/workspace/use.ts +++ b/cli/src/commands/use/workspace/use.ts @@ -5,10 +5,11 @@ import { WorkspacesClient } from '@/api/workspaces' import { BaseError } from '@/errors/base' import { ErrorCode } from '@/errors/codes' import { colorEnabled, colorScheme } from '@/sys/io/color' +import { selectFromList } from '@/sys/io/select' import { runWithSpinner } from '@/sys/io/spinner' export type UseWorkspaceOptions = { - readonly workspaceId: string + readonly workspaceId?: string } export type UseWorkspaceDeps = { @@ -22,16 +23,12 @@ export type UseWorkspaceDeps = { /** * Switch the caller's active workspace. * - * Strict ordering: - * 1. POST /workspaces//switch — if this fails (403/404/etc.) we abort - * with no `hosts.yml` mutation, so local state never diverges from the - * server. Any fallback to a pure-local update is explicitly disallowed - * (see workspace-plan.md decision D4). - * 2. GET /workspaces — refresh the membership list so `available_workspaces` - * stays in sync. Failure here also aborts; the server-side current has - * already moved, but the local file is left untouched. A follow-up - * `difyctl get workspace` will reconcile. - * 3. Persist `workspace` + `available_workspaces` atomically via `saveRegistry`. + * With an explicit id we switch directly; with no id we fetch the live + * workspace list and let the caller pick one interactively (TTY only). + * + * The server-side switch is the source of truth: if POST + * `/workspaces//switch` fails we abort before touching `hosts.yml`, so + * local state never diverges from the server. */ export async function runUseWorkspace( opts: UseWorkspaceOptions, @@ -41,32 +38,51 @@ export async function runUseWorkspace( const factory = deps.workspacesFactory ?? ((h: HttpClient) => new WorkspacesClient(h)) const client = factory(deps.http) + const argId = opts.workspaceId?.trim() ?? '' + const id = argId !== '' ? argId : await pickWorkspaceId(client, deps) + const detail = await runWithSpinner( - { io: deps.io, label: `Switching to ${opts.workspaceId}` }, - () => client.switch(opts.workspaceId), + { io: deps.io, label: `Switching to ${id}` }, + () => client.switch(id), ) - const list = await runWithSpinner( - { io: deps.io, label: 'Refreshing workspaces' }, - () => client.list(), - ) - - const matched = list.workspaces.find(w => w.id === detail.id) - if (matched === undefined) { - throw new BaseError({ - code: ErrorCode.Unknown, - message: `server returned switch=${detail.id} but it is not visible in /workspaces`, - hint: 'try again or contact your workspace admin', - }) - } - const nextCtx = { ...deps.active.ctx, - workspace: { id: matched.id, name: matched.name, role: matched.role }, - available_workspaces: list.workspaces.map(w => ({ id: w.id, name: w.name, role: w.role })), + workspace: { id: detail.id, name: detail.name, role: detail.role }, } deps.reg.upsert(deps.active.host, deps.active.email, nextCtx) deps.reg.save() - deps.io.out.write(`${cs.successIcon()} Switched to ${matched.name} (${matched.id})\n`) + deps.io.out.write(`${cs.successIcon()} Switched to ${detail.name} (${detail.id})\n`) return deps.reg } + +async function pickWorkspaceId(client: WorkspacesClient, deps: UseWorkspaceDeps): Promise { + if (!deps.io.isErrTTY) { + throw new BaseError({ + code: ErrorCode.UsageMissingArg, + message: 'a workspace id is required (no TTY)', + hint: 'pass the id: \'difyctl use workspace \'', + }) + } + + const list = await runWithSpinner( + { io: deps.io, label: 'Loading workspaces' }, + () => client.list(), + ) + const items = list.workspaces.map(w => ({ id: w.id, name: w.name, role: w.role })) + if (items.length === 0) { + throw new BaseError({ + code: ErrorCode.AccessDenied, + message: 'no workspaces available to switch to', + }) + } + + const activeId = deps.active.ctx.workspace?.id + const picked = await selectFromList({ + io: deps.io, + items, + header: 'Select a workspace', + render: w => `${w.id === activeId ? '* ' : ' '}${w.name} (${w.role})`, + }) + return picked.id +} diff --git a/cli/src/config/config-loader.test.ts b/cli/src/config/config-loader.test.ts index 501010cabd0..e3e5ae0053e 100644 --- a/cli/src/config/config-loader.test.ts +++ b/cli/src/config/config-loader.test.ts @@ -1,31 +1,13 @@ import type { YamlStore } from '@/store/store' -import { mkdtemp, rm } from 'node:fs/promises' -import { tmpdir } from 'node:os' -import { join } from 'node:path' -import { afterEach, beforeEach, describe, expect, it } from 'vitest' +import { useTempConfigDir } from '@test/fixtures/config-dir' +import { describe, expect, it } from 'vitest' import { isBaseError } from '@/errors/base' import { ErrorCode } from '@/errors/codes' -import { ENV_CONFIG_DIR } from '@/store/dir' import { getConfigurationStore } from '@/store/manager' import { loadConfig } from './config-loader' describe('loadConfig', () => { - let dir: string - let prevConfigDir: string | undefined - - beforeEach(async () => { - dir = await mkdtemp(join(tmpdir(), 'difyctl-cfg-')) - prevConfigDir = process.env[ENV_CONFIG_DIR] - process.env[ENV_CONFIG_DIR] = dir - }) - - afterEach(async () => { - if (prevConfigDir === undefined) - delete process.env[ENV_CONFIG_DIR] - else - process.env[ENV_CONFIG_DIR] = prevConfigDir - await rm(dir, { recursive: true, force: true }) - }) + useTempConfigDir('difyctl-cfg-') it('returns found:false when config is missing', () => { const r = loadConfig(getConfigurationStore()) diff --git a/cli/src/errors/codes.ts b/cli/src/errors/codes.ts index e2e69e5cd20..cc0d09b745e 100644 --- a/cli/src/errors/codes.ts +++ b/cli/src/errors/codes.ts @@ -17,6 +17,7 @@ export const ErrorCode = { ClientError: 'client_error', Unknown: 'unknown', IllegalArgumentError: 'illegal_argument', + KeyringUnavailable: 'keyring_unavailable', } as const export type ErrorCodeValue = (typeof ErrorCode)[keyof typeof ErrorCode] @@ -50,6 +51,7 @@ const CODE_TO_EXIT: Readonly> = { client_error: ExitCode.Generic, unknown: ExitCode.Generic, illegal_argument: ExitCode.Usage, + keyring_unavailable: ExitCode.Generic, } export function exitFor(code: string): ExitCodeValue { diff --git a/cli/src/store/config-writer.test.ts b/cli/src/store/config-writer.test.ts index 69a3300b281..23e9b350c7e 100644 --- a/cli/src/store/config-writer.test.ts +++ b/cli/src/store/config-writer.test.ts @@ -1,30 +1,12 @@ -import { mkdtemp, rm } from 'node:fs/promises' -import { tmpdir } from 'node:os' -import { join } from 'node:path' -import { afterEach, beforeEach, describe, expect, it } from 'vitest' +import { useTempConfigDir } from '@test/fixtures/config-dir' +import { describe, expect, it } from 'vitest' import { loadConfig } from '@/config/config-loader' import { emptyConfig } from '@/config/schema' import { saveConfig } from './config-writer' -import { ENV_CONFIG_DIR } from './dir' import { getConfigurationStore } from './manager' describe('saveConfig', () => { - let dir: string - let prevConfigDir: string | undefined - - beforeEach(async () => { - dir = await mkdtemp(join(tmpdir(), 'difyctl-w-')) - prevConfigDir = process.env[ENV_CONFIG_DIR] - process.env[ENV_CONFIG_DIR] = dir - }) - - afterEach(async () => { - if (prevConfigDir === undefined) - delete process.env[ENV_CONFIG_DIR] - else - process.env[ENV_CONFIG_DIR] = prevConfigDir - await rm(dir, { recursive: true, force: true }) - }) + useTempConfigDir('difyctl-w-') it('stamps schema_version=1 even if caller passed 0', () => { saveConfig(getConfigurationStore(), { ...emptyConfig() }) diff --git a/cli/src/store/keychain-token-store.test.ts b/cli/src/store/keychain-token-store.test.ts new file mode 100644 index 00000000000..66eca13257d --- /dev/null +++ b/cli/src/store/keychain-token-store.test.ts @@ -0,0 +1,138 @@ +import { beforeEach, describe, expect, it, vi } from 'vitest' +import { BaseError } from '@/errors/base' +import { ErrorCode } from '@/errors/codes' + +type EntryArgs = { service: string, username: string } + +const passwords = new Map() +const constructed: EntryArgs[] = [] +let getPasswordError: Error | null = null +let setPasswordError: Error | null = null + +class FakeEntry { + private readonly key: string + constructor(service: string, username: string) { + constructed.push({ service, username }) + this.key = `${service}::${username}` + } + + setPassword(value: string): void { + if (setPasswordError !== null) + throw setPasswordError + passwords.set(this.key, value) + } + + getPassword(): string | null { + if (getPasswordError !== null) + throw getPasswordError + return passwords.get(this.key) ?? null + } + + deletePassword(): boolean { + if (!passwords.has(this.key)) + return false + passwords.delete(this.key) + return true + } +} + +vi.mock('@napi-rs/keyring', () => ({ + Entry: FakeEntry, +})) + +const { KeychainTokenStore } = await import('./token-store') + +const SERVICE = 'difyctl-test' + +beforeEach(() => { + passwords.clear() + constructed.length = 0 + getPasswordError = null + setPasswordError = null +}) + +describe('KeychainTokenStore', () => { + it('round-trips a bearer through write/read', () => { + const store = new KeychainTokenStore(SERVICE) + store.write('https://cloud.dify.ai', 'a@x.com', 'dfoa_secret') + expect(store.read('https://cloud.dify.ai', 'a@x.com')).toBe('dfoa_secret') + }) + + it('returns empty string for an absent credential', () => { + const store = new KeychainTokenStore(SERVICE) + expect(store.read('https://cloud.dify.ai', 'missing@x.com')).toBe('') + }) + + it('removes a credential, after which read returns empty string', () => { + const store = new KeychainTokenStore(SERVICE) + store.write('h', 'e', 'dfoa_secret') + store.remove('h', 'e') + expect(store.read('h', 'e')).toBe('') + }) + + it('treats remove of an absent credential as a no-op', () => { + const store = new KeychainTokenStore(SERVICE) + expect(() => store.remove('h', 'absent')).not.toThrow() + }) + + it('uses the legacy entry name tokens.. (back-compat)', () => { + const store = new KeychainTokenStore(SERVICE) + store.write('https://cloud.dify.ai', 'a@x.com', 'dfoa_secret') + expect(constructed).toContainEqual({ + service: SERVICE, + username: 'tokens.https://cloud.dify.ai.a@x.com', + }) + }) + + it('keeps host and email literal — dots, colons, and @ are never split', () => { + const store = new KeychainTokenStore(SERVICE) + const host = 'https://my.dify.example.com:8443' + const email = 'first.last@sub.example.com' + store.write(host, email, 'dfoa_literal') + expect(store.read(host, email)).toBe('dfoa_literal') + expect(constructed).toContainEqual({ + service: SERVICE, + username: `tokens.${host}.${email}`, + }) + }) + + it('returns empty string when the stored value decodes to a non-string', () => { + const store = new KeychainTokenStore(SERVICE) + passwords.set(`${SERVICE}::tokens.h.e`, '123') + expect(store.read('h', 'e')).toBe('') + }) + + it('returns empty string when the stored value is not valid JSON', () => { + const store = new KeychainTokenStore(SERVICE) + passwords.set(`${SERVICE}::tokens.h.e`, 'not-json') + expect(store.read('h', 'e')).toBe('') + }) + + it('throws KeyringUnavailable (not empty string) when keyring access fails on read', () => { + getPasswordError = new Error('keyring locked') + const store = new KeychainTokenStore(SERVICE) + let caught: unknown + try { + store.read('h', 'e') + } + catch (err) { + caught = err + } + expect(caught).toBeInstanceOf(BaseError) + expect((caught as BaseError).code).toBe(ErrorCode.KeyringUnavailable) + }) + + it('throws KeyringUnavailable when keyring access fails on write', () => { + setPasswordError = new Error('keyring locked') + const store = new KeychainTokenStore(SERVICE) + let caught: unknown + try { + store.write('h', 'e', 'dfoa_secret') + } + catch (err) { + caught = err + } + expect(caught).toBeInstanceOf(BaseError) + expect((caught as BaseError).code).toBe(ErrorCode.KeyringUnavailable) + }) +}) diff --git a/cli/src/store/manager.test.ts b/cli/src/store/manager.test.ts index 1e83c026f24..0a499793e25 100644 --- a/cli/src/store/manager.test.ts +++ b/cli/src/store/manager.test.ts @@ -1,28 +1,29 @@ -import type { Key, Store } from './store' +import type { TokenStore } from './token-store' import { describe, expect, it, vi } from 'vitest' -import { getTokenStore } from './manager' +import { detectTokenStore, getTokenStore } from './manager' -function memStore(label: string): Store & { _label: string } { - const map = new Map() +function memStore(label: string): TokenStore & { _label: string } { + const map = new Map() + const k = (h: string, e: string): string => `${h} ${e}` return { _label: label, - get(key: Key): T { - return (map.get(key.key) as T | undefined) ?? key.default + read(host: string, email: string): string { + return map.get(k(host, email)) ?? '' }, - set(key: Key, value: T): void { - map.set(key.key, value) + write(host: string, email: string, bearer: string): void { + map.set(k(host, email), bearer) }, - unset(key: Key): void { - map.delete(key.key) + remove(host: string, email: string): void { + map.delete(k(host, email)) }, } } -describe('getTokenStore', () => { +describe('detectTokenStore', () => { it('returns keychain store when probe succeeds', () => { const k = memStore('keyring') const f = memStore('file') - const result = getTokenStore({ + const result = detectTokenStore({ factory: { keyring: () => k, file: () => f }, }) expect(result.mode).toBe('keychain') @@ -32,12 +33,10 @@ describe('getTokenStore', () => { it('falls back to file when keyring set throws', () => { const k = memStore('keyring') const f = memStore('file') - k.set = vi.fn( - () => { - throw new Error('locked') - }, - ) - const result = getTokenStore({ + k.write = vi.fn(() => { + throw new Error('locked') + }) + const result = detectTokenStore({ factory: { keyring: () => k, file: () => f }, }) expect(result.mode).toBe('file') @@ -47,8 +46,8 @@ describe('getTokenStore', () => { it('falls back to file when probe round-trip mismatches', () => { const k = memStore('keyring') const f = memStore('file') - k.get = vi.fn(() => 'something-else') as Store['get'] - const result = getTokenStore({ + k.read = vi.fn(() => 'something-else') as TokenStore['read'] + const result = detectTokenStore({ factory: { keyring: () => k, file: () => f }, }) expect(result.mode).toBe('file') @@ -57,7 +56,7 @@ describe('getTokenStore', () => { it('falls back to file when keyring constructor throws', () => { const f = memStore('file') - const result = getTokenStore({ + const result = detectTokenStore({ factory: { keyring: () => { throw new Error('no backend') }, file: () => f, @@ -70,9 +69,48 @@ describe('getTokenStore', () => { it('cleans up probe entry after successful probe', () => { const k = memStore('keyring') const f = memStore('file') - getTokenStore({ + detectTokenStore({ factory: { keyring: () => k, file: () => f }, }) - expect(k.get({ key: '__difyctl_probe__', default: '' })).toBe('') + expect(k.read('__difyctl_probe__', '__difyctl_probe__')).toBe('') + }) + + it('removes the probe entry even when the probe read throws', () => { + const k = memStore('keyring') + const f = memStore('file') + const removeSpy = vi.spyOn(k, 'remove') + k.read = vi.fn(() => { + throw new Error('read boom') + }) as TokenStore['read'] + const result = detectTokenStore({ + factory: { keyring: () => k, file: () => f }, + }) + expect(removeSpy).toHaveBeenCalledWith('__difyctl_probe__', '__difyctl_probe__') + expect(result.mode).toBe('file') + expect(result.store).toBe(f) + }) +}) + +describe('getTokenStore', () => { + it('constructs the keychain backend without probing when mode is keychain', () => { + const k = memStore('keyring') + const f = memStore('file') + k.write = vi.fn(() => { + throw new Error('probe must never run on the read path') + }) + const store = getTokenStore('keychain', { + factory: { keyring: () => k, file: () => f }, + }) + expect(store).toBe(k) + }) + + it('constructs the file backend when mode is file, never touching the keyring', () => { + const keyringFactory = vi.fn(() => memStore('keyring')) + const f = memStore('file') + const store = getTokenStore('file', { + factory: { keyring: keyringFactory, file: () => f }, + }) + expect(store).toBe(f) + expect(keyringFactory).not.toHaveBeenCalled() }) }) diff --git a/cli/src/store/manager.ts b/cli/src/store/manager.ts index dfd5288e11e..4b9cea376e2 100644 --- a/cli/src/store/manager.ts +++ b/cli/src/store/manager.ts @@ -1,7 +1,9 @@ -import type { Key, StorageMode, Store } from './store' +import type { StorageMode, Store } from './store' +import type { TokenStore } from './token-store' import { join } from 'node:path' import { resolveCacheDir, resolveConfigDir } from './dir' -import { KeyringBasedStore, YamlStore } from './store' +import { YamlStore } from './store' +import { FileTokenStore, KeychainTokenStore } from './token-store' export const CACHE_APP_INFO = 'app-info' export const CACHE_NUDGE = 'nudge' @@ -31,51 +33,52 @@ export function getHostStore(): YamlStore { return getStore(join(resolveConfigDir(), HOSTS_FILE)) } -const PROBE_KEY: Key = { key: '__difyctl_probe__', default: '' } +const PROBE_HOST = '__difyctl_probe__' +const PROBE_EMAIL = '__difyctl_probe__' const PROBE_VALUE = 'probe-v1' export type GetTokenStoreOptions = { readonly factory?: { - readonly keyring?: () => Store - readonly file?: () => Store + readonly keyring?: () => TokenStore + readonly file?: () => TokenStore } } +const TOKEN_STORE_OPENERS: Record TokenStore> = { + file: opts => opts.factory?.file?.() ?? new FileTokenStore(join(resolveConfigDir(), TOKENS_FILE)), + keychain: opts => opts.factory?.keyring?.() ?? new KeychainTokenStore(KEYRING_SERVICE), +} + /** - * Single entry point for the credential store. Probes the OS keyring; if it - * round-trips a value, returns the keychain-backed store. Otherwise falls - * back to the YAML file at `/tokens.yml`. Both implementations - * satisfy the `Store` interface, so callers interact uniformly. - * - * Business logic should always obtain the token store through this factory - * rather than constructing one directly. + * Decide which credential backend to use by probing the OS keyring with a + * write/read/remove round-trip. The probe MUTATES the keyring, so call this + * only where a credential is about to be written anyway (login). */ -export function getTokenStore(opts: GetTokenStoreOptions = {}): { store: Store, mode: StorageMode } { - const fileFactory = opts.factory?.file ?? (() => getStore(join(resolveConfigDir(), TOKENS_FILE))) - const keyringFactory = opts.factory?.keyring ?? (() => new KeyringBasedStore(KEYRING_SERVICE)) +export function detectTokenStore(opts: GetTokenStoreOptions = {}): { store: TokenStore, mode: StorageMode } { // DIFY_E2E_NO_KEYRING=1 forces file-based storage in E2E tests to avoid // macOS keychain UI prompts blocking child processes spawned by vitest. if (process.env.DIFY_E2E_NO_KEYRING === '1') - return { store: fileFactory(), mode: 'file' } + return { store: TOKEN_STORE_OPENERS.file(opts), mode: 'file' } try { - const k = keyringFactory() - k.set(PROBE_KEY, PROBE_VALUE) - const got = k.get(PROBE_KEY) - k.unset(PROBE_KEY) - if (got !== PROBE_VALUE) - throw new Error('keyring round-trip mismatch') - return { store: k, mode: 'keychain' } - } - catch { - return { store: fileFactory(), mode: 'file' } + const k = TOKEN_STORE_OPENERS.keychain(opts) + k.write(PROBE_HOST, PROBE_EMAIL, PROBE_VALUE) + let got = '' + try { + got = k.read(PROBE_HOST, PROBE_EMAIL) + } + finally { + k.remove(PROBE_HOST, PROBE_EMAIL) + } + if (got === PROBE_VALUE) + return { store: k, mode: 'keychain' } } + catch { /* keyring unavailable → fall through to file */ } + return { store: TOKEN_STORE_OPENERS.file(opts), mode: 'file' } } /** - * Maps an auth identity (host + accountId) to a `Store` key. All token store - * reads/writes in business logic go through this helper so the on-disk / - * keyring layout stays consistent. + * Construct the credential backend the registry already recorded at login. */ -export function tokenKey(host: string, accountId: string): Key { - return { key: `tokens.${host}.${accountId}`, default: '' } +export function getTokenStore(mode: StorageMode, opts: GetTokenStoreOptions = {}): TokenStore { + return TOKEN_STORE_OPENERS[mode](opts) } diff --git a/cli/src/store/store.ts b/cli/src/store/store.ts index 85b0cd7391f..97052da0efd 100644 --- a/cli/src/store/store.ts +++ b/cli/src/store/store.ts @@ -21,7 +21,8 @@ export type Store = { unset: (key: Key) => void } -export type StorageMode = 'keychain' | 'file' +export const STORAGE_MODES = ['keychain', 'file'] as const +export type StorageMode = typeof STORAGE_MODES[number] abstract class FileBasedStore implements Store { filePath: string diff --git a/cli/src/store/token-store.test.ts b/cli/src/store/token-store.test.ts new file mode 100644 index 00000000000..b076267234c --- /dev/null +++ b/cli/src/store/token-store.test.ts @@ -0,0 +1,81 @@ +import { mkdtempSync, readFileSync, rmSync, writeFileSync } from 'node:fs' +import { tmpdir } from 'node:os' +import { join } from 'node:path' +import { afterEach, beforeEach, describe, expect, it } from 'vitest' +import { FileTokenStore } from './token-store' + +describe('FileTokenStore', () => { + let dir: string + let file: string + + beforeEach(() => { + dir = mkdtempSync(join(tmpdir(), 'difyctl-tok-')) + file = join(dir, 'tokens.yml') + }) + afterEach(() => rmSync(dir, { recursive: true, force: true })) + + it('returns empty string for a missing credential', () => { + const s = new FileTokenStore(file) + expect(s.read('https://cloud.dify.ai', 'a@x.com')).toBe('') + }) + + it('round-trips a bearer with dots and @ kept literal', () => { + const s = new FileTokenStore(file) + s.write('https://cloud.dify.ai', 'a.b@x.com', 'dfoa_secret') + expect(s.read('https://cloud.dify.ai', 'a.b@x.com')).toBe('dfoa_secret') + }) + + it('keeps multiple accounts under one host and isolates hosts', () => { + const s = new FileTokenStore(file) + s.write('https://cloud.dify.ai', 'a@x.com', 'A') + s.write('https://cloud.dify.ai', 'b@x.com', 'B') + s.write('https://self.example.com', 'a@x.com', 'C') + expect(s.read('https://cloud.dify.ai', 'a@x.com')).toBe('A') + expect(s.read('https://cloud.dify.ai', 'b@x.com')).toBe('B') + expect(s.read('https://self.example.com', 'a@x.com')).toBe('C') + }) + + it('persists the versioned nested shape on disk', () => { + const s = new FileTokenStore(file) + s.write('https://cloud.dify.ai', 'a@x.com', 'A') + const raw = readFileSync(file, 'utf8') + expect(raw).toContain('version: 1') + expect(raw).toContain('https://cloud.dify.ai') + expect(raw).toContain('a@x.com') + }) + + it('reads empty when the document version is an unknown future version', () => { + writeFileSync(file, 'version: 999\ntokens:\n "h":\n "e": "x"\n') + const s = new FileTokenStore(file) + expect(s.read('h', 'e')).toBe('') + }) + + it('reads tokens from legacy format (no version field) for transparent migration', () => { + writeFileSync(file, 'tokens:\n "h":\n "e": "dfoa_legacy"\n') + const s = new FileTokenStore(file) + expect(s.read('h', 'e')).toBe('dfoa_legacy') + }) + + it('preserves existing tokens and stamps version when writing to a legacy file', () => { + writeFileSync(file, 'tokens:\n "h":\n "existing@x": "dfoa_existing"\n') + const s = new FileTokenStore(file) + s.write('h', 'new@x', 'dfoa_new') + expect(s.read('h', 'existing@x')).toBe('dfoa_existing') + expect(s.read('h', 'new@x')).toBe('dfoa_new') + expect(readFileSync(file, 'utf8')).toContain('version: 1') + }) + + it('remove deletes the credential and prunes the empty host map', () => { + const s = new FileTokenStore(file) + s.write('https://cloud.dify.ai', 'a@x.com', 'A') + s.remove('https://cloud.dify.ai', 'a@x.com') + expect(s.read('https://cloud.dify.ai', 'a@x.com')).toBe('') + const raw = readFileSync(file, 'utf8') + expect(raw).not.toContain('cloud.dify.ai') + }) + + it('remove is a no-op for an absent credential', () => { + const s = new FileTokenStore(file) + expect(() => s.remove('h', 'e')).not.toThrow() + }) +}) diff --git a/cli/src/store/token-store.ts b/cli/src/store/token-store.ts new file mode 100644 index 00000000000..e6d6f29599a --- /dev/null +++ b/cli/src/store/token-store.ts @@ -0,0 +1,130 @@ +import { Entry } from '@napi-rs/keyring' +import { BaseError } from '@/errors/base' +import { ErrorCode } from '@/errors/codes' +import { YamlStore } from './store' + +/** + * Credential store keyed by an opaque (host, email) pair. + */ +export type TokenStore = { + read: (host: string, email: string) => string + write: (host: string, email: string, bearer: string) => void + remove: (host: string, email: string) => void +} + +const DOC_VERSION = 1 + +type TokenDoc = { + version?: number + tokens?: Record> +} + +export class FileTokenStore implements TokenStore { + private readonly store: YamlStore + + constructor(filePath: string) { + this.store = new YamlStore(filePath) + } + + read(host: string, email: string): string { + const doc = this.store.getTyped() + if (doc === null) + return '' + // missing version = legacy pre-v1 format (same data shape); future unknown versions are rejected + if (doc.version !== undefined && doc.version !== DOC_VERSION) + return '' + return doc.tokens?.[host]?.[email] ?? '' + } + + write(host: string, email: string, bearer: string): void { + const doc = this.load() + const hostMap = doc.tokens[host] ?? {} + hostMap[email] = bearer + doc.tokens[host] = hostMap + this.store.setTyped(doc) + } + + remove(host: string, email: string): void { + const doc = this.store.getTyped() + if (doc === null) + return + if (doc.version !== undefined && doc.version !== DOC_VERSION) + return + const tokens = doc.tokens ?? {} + const hostMap = tokens[host] + if (hostMap === undefined || !(email in hostMap)) + return + delete hostMap[email] + if (Object.keys(hostMap).length === 0) + delete tokens[host] + this.store.setTyped({ version: DOC_VERSION, tokens }) + } + + private load(): { version: number, tokens: Record> } { + const doc = this.store.getTyped() + if (doc === null) + return { version: DOC_VERSION, tokens: {} } + if (doc.version !== undefined && doc.version !== DOC_VERSION) + return { version: DOC_VERSION, tokens: {} } + return { version: DOC_VERSION, tokens: (doc.tokens ?? {}) as Record> } + } +} + +/** + * One OS-keyring entry per (host, email). + */ +export class KeychainTokenStore implements TokenStore { + private readonly service: string + + constructor(service: string) { + this.service = service + } + + read(host: string, email: string): string { + let raw: string | null + try { + raw = new Entry(this.service, entryName(host, email)).getPassword() + } + catch (err) { + throw keyringUnavailableError(err) + } + if (raw === null || raw === undefined || raw === '') + return '' + try { + const parsed: unknown = JSON.parse(raw) + return typeof parsed === 'string' ? parsed : '' + } + catch { + return '' + } + } + + write(host: string, email: string, bearer: string): void { + try { + new Entry(this.service, entryName(host, email)).setPassword(JSON.stringify(bearer)) + } + catch (err) { + throw keyringUnavailableError(err) + } + } + + remove(host: string, email: string): void { + try { + new Entry(this.service, entryName(host, email)).deletePassword() + } + catch { /* missing entry is fine */ } + } +} + +function entryName(host: string, email: string): string { + return `tokens.${host}.${email}` +} + +function keyringUnavailableError(cause: unknown): BaseError { + return new BaseError({ + code: ErrorCode.KeyringUnavailable, + message: 'OS keychain is unreachable', + hint: 'credentials are stored in the system keychain but it could not be accessed; unlock the keychain (or the login session) and retry', + cause, + }) +} diff --git a/cli/src/workspace/resolver.test.ts b/cli/src/workspace/resolver.test.ts new file mode 100644 index 00000000000..daac192b82e --- /dev/null +++ b/cli/src/workspace/resolver.test.ts @@ -0,0 +1,29 @@ +import type { ActiveContext } from '@/auth/hosts' +import { describe, expect, it } from 'vitest' +import { resolveWorkspaceId } from './resolver' + +function active(workspaceId?: string): ActiveContext { + return { host: 'h', email: 'e', ctx: { account: { id: '', email: 'e', name: '' }, workspace: workspaceId ? { id: workspaceId, name: 'W', role: 'owner' } : undefined } } +} + +const UUID_FLAG = 'aaaaaaaa-0000-0000-0000-000000000001' +const UUID_ENV = 'aaaaaaaa-0000-0000-0000-000000000002' +const UUID_CTX = 'aaaaaaaa-0000-0000-0000-000000000003' + +describe('resolveWorkspaceId', () => { + it('prefers the flag', () => { + expect(resolveWorkspaceId({ flag: UUID_FLAG, env: UUID_ENV, active: active(UUID_CTX) })).toBe(UUID_FLAG) + }) + it('falls back to env over active workspace', () => { + expect(resolveWorkspaceId({ env: UUID_ENV, active: active(UUID_CTX) })).toBe(UUID_ENV) + }) + it('falls back to active workspace when no flag or env', () => { + expect(resolveWorkspaceId({ active: active(UUID_CTX) })).toBe(UUID_CTX) + }) + it('throws when active workspace ID is not a valid UUID', () => { + expect(() => resolveWorkspaceId({ active: active('ws-ctx') })).toThrow(/stored workspace ID/) + }) + it('throws when no workspace is selected (no implicit default)', () => { + expect(() => resolveWorkspaceId({ active: active(undefined) })).toThrow(/no workspace selected/) + }) +}) diff --git a/cli/src/workspace/resolver.ts b/cli/src/workspace/resolver.ts index 40af50b38bb..a5b03d7c613 100644 --- a/cli/src/workspace/resolver.ts +++ b/cli/src/workspace/resolver.ts @@ -25,14 +25,16 @@ export function resolveWorkspaceId(inputs: WorkspaceResolveInputs): string { throw new BaseError({ code: ErrorCode.UsageInvalidFlag, message: `DIFY_WORKSPACE_ID value ${JSON.stringify(inputs.env)} is not a valid UUID` }) return inputs.env } - const ctx = inputs.active?.ctx - if (ctx !== undefined) { - if (truthy(ctx.workspace?.id)) - return ctx.workspace.id - if (ctx.available_workspaces !== undefined && ctx.available_workspaces.length > 0 - && truthy(ctx.available_workspaces[0]?.id)) { - return ctx.available_workspaces[0].id + const wsId = inputs.active?.ctx.workspace?.id + if (truthy(wsId)) { + if (!isValidUuid(wsId)) { + throw new BaseError({ + code: ErrorCode.UsageInvalidFlag, + message: `stored workspace ID ${JSON.stringify(wsId)} is not a valid UUID`, + hint: 'run \'difyctl use workspace\' to update your active workspace', + }) } + return wsId } throw new BaseError({ code: ErrorCode.UsageMissingArg, diff --git a/cli/test/fixtures/config-dir.ts b/cli/test/fixtures/config-dir.ts new file mode 100644 index 00000000000..fa32563236d --- /dev/null +++ b/cli/test/fixtures/config-dir.ts @@ -0,0 +1,24 @@ +import { mkdtemp, rm } from 'node:fs/promises' +import { tmpdir } from 'node:os' +import { join } from 'node:path' +import { afterEach, beforeEach } from 'vitest' +import { ENV_CONFIG_DIR } from '../../src/store/dir.js' + +// Points ENV_CONFIG_DIR at a fresh temp dir per test and restores it after. +// Call inside a describe block; returns a getter because the dir changes per test. +export function useTempConfigDir(prefix: string): () => string { + let dir = '' + let prev: string | undefined + beforeEach(async () => { + dir = await mkdtemp(join(tmpdir(), prefix)) + prev = process.env[ENV_CONFIG_DIR] + process.env[ENV_CONFIG_DIR] = dir + }) + afterEach(async () => { + if (prev === undefined) + delete process.env[ENV_CONFIG_DIR] + else process.env[ENV_CONFIG_DIR] = prev + await rm(dir, { recursive: true, force: true }) + }) + return () => dir +} diff --git a/cli/test/fixtures/mem-store.ts b/cli/test/fixtures/mem-store.ts new file mode 100644 index 00000000000..23ab779f839 --- /dev/null +++ b/cli/test/fixtures/mem-store.ts @@ -0,0 +1,27 @@ +import type { TokenStore } from '../../src/store/token-store.js' + +// In-memory TokenStore for tests; mirrors keyring semantics (empty string = missing). +export class MemStore implements TokenStore { + readonly entries: Map + + // Seed keys use the composite " " form, e.g. { 'h1 a@x': 'dfoa_a' }. + constructor(seed: Record = {}) { + this.entries = new Map(Object.entries(seed)) + } + + private k(host: string, email: string): string { + return `${host} ${email}` + } + + read(host: string, email: string): string { + return this.entries.get(this.k(host, email)) ?? '' + } + + write(host: string, email: string, bearer: string): void { + this.entries.set(this.k(host, email), bearer) + } + + remove(host: string, email: string): void { + this.entries.delete(this.k(host, email)) + } +}