diff --git a/src/__tests__/config-migration.test.ts b/src/__tests__/config-migration.test.ts new file mode 100644 index 0000000..6c121c0 --- /dev/null +++ b/src/__tests__/config-migration.test.ts @@ -0,0 +1,67 @@ +import { describe, expect, it } from 'vitest'; +import { mergeLegacyConfig, notificationFromLegacy } from '../config/migration'; + +describe('config migration', () => { + it('returns undefined for empty legacy input', () => { + expect(notificationFromLegacy({})).toBeUndefined(); + }); + + it('converts legacy notification keys into the new notification shape', () => { + const notifications = notificationFromLegacy({ + notification_service: 'email', + discord_webhook: 'https://discord.com/api/webhooks/123/token', + email_host: 'smtp.test.com', + email_port: 587, + email_user: 'user@test.com', + email_to: 'to@test.com', + email_password: 'secret', + alert_cooldown: 120, + }); + + expect(notifications).toEqual({ + service: 'email', + discordWebhook: 'https://discord.com/api/webhooks/123/token', + emailHost: 'smtp.test.com', + emailPort: 587, + emailUser: 'user@test.com', + emailTo: 'to@test.com', + emailPassword: 'secret', + alertCooldown: 120, + }); + }); + + it('prefers explicit new notification config over migrated legacy values', () => { + const config = mergeLegacyConfig({ + notification_service: 'discord', + discord_webhook: 'legacy-webhook', + notifications: { + service: 'email', + discordWebhook: 'new-webhook', + }, + }); + + expect(config.notifications).toMatchObject({ + service: 'email', + discordWebhook: 'new-webhook', + }); + }); + + it('ignores malformed legacy notification values', () => { + const notifications = notificationFromLegacy({ + notification_service: { invalid: 'object' } as never, + email_port: 'not-a-number' as never, + alert_cooldown: 'not-a-number' as never, + }); + + expect(notifications).toEqual({ + service: 'none', + emailPort: undefined, + alertCooldown: undefined, + discordWebhook: undefined, + emailHost: undefined, + emailUser: undefined, + emailTo: undefined, + emailPassword: undefined, + }); + }); +}); diff --git a/src/__tests__/config-store.test.ts b/src/__tests__/config-store.test.ts new file mode 100644 index 0000000..c46388e --- /dev/null +++ b/src/__tests__/config-store.test.ts @@ -0,0 +1,122 @@ +import { beforeEach, describe, expect, it, vi } from 'vitest'; + +vi.mock('conf', () => { + const mockConfigStore = new Map(); + const mockConfInstance = { + get store() { + return Object.fromEntries(mockConfigStore.entries()); + }, + set: vi.fn((key, val) => { + mockConfigStore.set(key, val); + }), + get: vi.fn((key) => mockConfigStore.get(key)), + delete: vi.fn((key) => { + mockConfigStore.delete(key); + }), + clear: vi.fn(() => { + mockConfigStore.clear(); + }), + }; + + (globalThis as any).mockConfigStore = mockConfigStore; + + return { + default: class MockConf { + constructor() { + return mockConfInstance; + } + }, + }; +}); +import { + clearConfig, + getActiveFilters, + getCacheConfig, + getConfig, + getLegacyConfig, + getLegacyValue, + getOutputConfig, + setActiveFilters, + setAIConfig, + setCacheConfig, + setConfigValue, + setKubernetesConfig, + setLegacyValue, + setOutputConfig, +} from '../config/store'; + +describe('config store', () => { + beforeEach(() => { + clearConfig(); + vi.clearAllMocks(); + }); + + it('returns defaults for new configuration sections', () => { + expect(getActiveFilters()).toEqual([]); + expect(getCacheConfig()).toEqual({ type: 'file', enabled: true }); + expect(getOutputConfig()).toEqual({ format: 'text', language: 'english' }); + }); + + it('stores and reads new typed configuration sections', () => { + setAIConfig({ + providers: [{ name: 'openai', model: 'gpt-4o' }], + defaultProvider: 'openai', + }); + setActiveFilters(['Pod', 'Deployment']); + setKubernetesConfig({ namespace: 'default', kubecontext: 'minikube' }); + setCacheConfig({ type: 'file', enabled: false, path: '/tmp/kdm-cache' }); + setOutputConfig({ format: 'json', language: 'english' }); + + expect(getConfig()).toMatchObject({ + ai: { + providers: [{ name: 'openai', model: 'gpt-4o' }], + defaultProvider: 'openai', + }, + activeFilters: ['Pod', 'Deployment'], + kubernetes: { namespace: 'default', kubecontext: 'minikube' }, + cache: { type: 'file', enabled: false, path: '/tmp/kdm-cache' }, + output: { format: 'json', language: 'english' }, + }); + }); + + it('keeps legacy notification reads compatible', () => { + setLegacyValue('notification_service', 'discord'); + setLegacyValue('discord_webhook', 'https://discord.com/api/webhooks/123/token'); + setLegacyValue('alert_cooldown', 300); + + expect(getLegacyConfig()).toEqual({ + notification_service: 'discord', + discord_webhook: 'https://discord.com/api/webhooks/123/token', + alert_cooldown: 300, + email_host: undefined, + email_port: undefined, + email_user: undefined, + email_to: undefined, + email_password: undefined, + }); + }); + + it('keeps legacy value reads compatible with migrated notification config', () => { + setConfigValue('notifications', { + service: 'email', + emailHost: 'smtp.test.com', + emailPort: 587, + emailUser: 'user@test.com', + emailTo: 'to@test.com', + }); + + expect(getLegacyValue('notification_service')).toBe('email'); + expect(getLegacyValue('email_host')).toBe('smtp.test.com'); + expect(getLegacyValue('email_port')).toBe(587); + }); + + it('returns safe legacy defaults for malformed notification values', () => { + setLegacyValue('notification_service', { invalid: 'object' } as never); + setLegacyValue('alert_cooldown', 'not-a-number' as never); + + expect(getLegacyConfig()).toMatchObject({ + notification_service: 'none', + alert_cooldown: undefined, + }); + }); +}); diff --git a/src/__tests__/config-utils.test.ts b/src/__tests__/config-utils.test.ts index e69f510..b39517e 100644 --- a/src/__tests__/config-utils.test.ts +++ b/src/__tests__/config-utils.test.ts @@ -3,7 +3,9 @@ import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; vi.mock('conf', () => { const mockConfigStore = new Map(); const mockConfInstance = { - store: {}, + get store() { + return Object.fromEntries(mockConfigStore.entries()); + }, set: vi.fn((key, val) => { mockConfigStore.set(key, val); }), @@ -30,7 +32,7 @@ vi.mock('conf', () => { }; }); -import { getSMTPSettings, clearNotificationCredentials } from '../utils/config'; +import { getSMTPSettings, clearNotificationCredentials, setConfig } from '../utils/config'; describe('config utils', () => { const originalEnv = process.env; @@ -89,4 +91,9 @@ describe('config utils', () => { settings = getSMTPSettings(); expect(settings.auth.pass).toBe(''); }); + + it('should reject storing SMTP passwords in config', () => { + expect(() => setConfig('email_password', 'secret')).toThrow(/KDM_SMTP_PASSWORD/); + expect(mockConfInstance.set).not.toHaveBeenCalledWith('email_password', 'secret'); + }); }); diff --git a/src/__tests__/config.test.ts b/src/__tests__/config.test.ts index b7bf32e..aa4acde 100644 --- a/src/__tests__/config.test.ts +++ b/src/__tests__/config.test.ts @@ -95,8 +95,7 @@ describe('config command', () => { .mockResolvedValueOnce('smtp.gmail.com') // host .mockResolvedValueOnce('587') // port .mockResolvedValueOnce('user@test.com') // user - .mockResolvedValueOnce('to@test.com') // to - .mockResolvedValueOnce(''); // password (empty) + .mockResolvedValueOnce('to@test.com'); // to await program.parseAsync(['node', 'test', 'config', 'setup']); @@ -113,37 +112,33 @@ describe('config command', () => { expect(guideOrder).toBeLessThan(firstInputOrder()); }); - it('should save email_password if provided during email setup', async () => { + it('should not ask for or persist SMTP password during email setup', async () => { vi.mocked(tui.select).mockResolvedValue('email'); vi.mocked(tui.input) .mockResolvedValueOnce('smtp.gmail.com') // host .mockResolvedValueOnce('587') // port .mockResolvedValueOnce('user@test.com') // user - .mockResolvedValueOnce('to@test.com') // to - .mockResolvedValueOnce('pass123'); // password + .mockResolvedValueOnce('to@test.com'); // to await program.parseAsync(['node', 'test', 'config', 'setup']); - expect(configUtils.setConfig).toHaveBeenCalledWith('email_password', 'pass123'); + expect(tui.input).toHaveBeenCalledTimes(4); + expect(configUtils.setConfig).not.toHaveBeenCalledWith('email_password', expect.any(String)); + expect(consoleLogSpy).toHaveBeenCalledWith(expect.stringContaining('KDM_SMTP_PASSWORD')); }); - it('should require an SMTP host during email setup and validate optional SMTP password', async () => { + it('should require an SMTP host during email setup', async () => { vi.mocked(tui.select).mockResolvedValue('email'); vi.mocked(tui.input) .mockResolvedValueOnce('smtp.gmail.com') .mockResolvedValueOnce('587') .mockResolvedValueOnce('user@test.com') - .mockResolvedValueOnce('to@test.com') - .mockResolvedValueOnce(''); + .mockResolvedValueOnce('to@test.com'); await program.parseAsync(['node', 'test', 'config', 'setup']); const smtpHostPrompt = vi.mocked(tui.input).mock.calls[0][0]; expect(smtpHostPrompt.validate('')).toBe('Host is required'); - - const smtpPasswordPrompt = vi.mocked(tui.input).mock.calls[4][0]; - expect(smtpPasswordPrompt.validate('')).toBe(true); - expect(smtpPasswordPrompt.validate('anything')).toBe(true); }); it('should call setConfig on config set', async () => { diff --git a/src/commands/config.ts b/src/commands/config.ts index 1d42758..9d5c957 100644 --- a/src/commands/config.ts +++ b/src/commands/config.ts @@ -68,22 +68,16 @@ export const registerConfigCommand = (program: Command) => { message: 'Alert Recipient Email:', validate: (v) => /^[^\s@]+@[^\s@]+\.[^\s@]+$/.test(v) || 'Must be a valid email address', }); - const password = await input({ - message: 'SMTP Password (optional, press Enter to skip):', - validate: () => true, - }); clearNotificationCredentials(); setConfig('email_host', host); setConfig('email_port', parseInt(portStr, 10)); setConfig('email_user', user); setConfig('email_to', to); - if (password) { - setConfig('email_password', password); - } setConfig('notification_service', 'email'); console.log(chalk.green('\n✓ Email SMTP configured.')); + console.log(chalk.dim(' Set KDM_SMTP_PASSWORD in your environment to provide the SMTP password.')); } console.log(chalk.green(`✓ Notification service set to: ${chalk.bold(choice.toUpperCase())}`)); @@ -127,7 +121,7 @@ export const registerConfigCommand = (program: Command) => { } console.log(chalk.gray('──────────────────────────────────────────────────')); - console.log(chalk.dim('\n Note: SMTP password can be set either in config or via the KDM_SMTP_PASSWORD environment variable, which takes precedence if both are set.\n')); + console.log(chalk.dim('\n Note: SMTP password is not stored in config. Set it with the KDM_SMTP_PASSWORD environment variable.\n')); }); config @@ -156,7 +150,7 @@ const printEmailSmtpGuide = () => { console.log(chalk.white(' 1. Find your provider SMTP settings before continuing.')); console.log(chalk.white(' 2. Common hosts: smtp.gmail.com for Gmail, smtp.office365.com for Outlook.')); console.log(chalk.white(' 3. Use port 587 for STARTTLS unless your provider says otherwise.')); - console.log(chalk.white(' 4. Provide the SMTP password during setup or via the KDM_SMTP_PASSWORD environment variable.')); + console.log(chalk.white(' 4. Provide the SMTP password via the KDM_SMTP_PASSWORD environment variable.')); console.log(chalk.dim(' Gmail accounts with 2FA usually require an App Password.')); console.log(chalk.gray('──────────────────────────────────────────────────\n')); }; diff --git a/src/config/migration.ts b/src/config/migration.ts new file mode 100644 index 0000000..991ea44 --- /dev/null +++ b/src/config/migration.ts @@ -0,0 +1,51 @@ +import type { KDMConfig, LegacyNotificationConfig, NotificationConfig, StoredKDMConfig } from './schema'; + +const isNotificationService = (value: unknown): value is NotificationConfig['service'] => + value === 'discord' || value === 'email' || value === 'none'; + +const stringOrUndefined = (value: unknown) => (typeof value === 'string' ? value : undefined); +const numberOrUndefined = (value: unknown) => (typeof value === 'number' ? value : undefined); + +const hasLegacyNotificationConfig = (config: LegacyNotificationConfig) => + config.notification_service !== undefined || + config.discord_webhook !== undefined || + config.email_host !== undefined || + config.email_port !== undefined || + config.email_user !== undefined || + config.email_to !== undefined || + config.email_password !== undefined || + config.alert_cooldown !== undefined; + +export const notificationFromLegacy = ( + legacy: LegacyNotificationConfig, +): NotificationConfig | undefined => { + if (!hasLegacyNotificationConfig(legacy)) { + return undefined; + } + + return { + service: isNotificationService(legacy.notification_service) ? legacy.notification_service : 'none', + discordWebhook: stringOrUndefined(legacy.discord_webhook), + emailHost: stringOrUndefined(legacy.email_host), + emailPort: numberOrUndefined(legacy.email_port), + emailUser: stringOrUndefined(legacy.email_user), + emailTo: stringOrUndefined(legacy.email_to), + emailPassword: stringOrUndefined(legacy.email_password), + alertCooldown: numberOrUndefined(legacy.alert_cooldown), + }; +}; +export const mergeLegacyConfig = (stored: StoredKDMConfig): KDMConfig => { + const legacyNotifications = notificationFromLegacy(stored); + + return { + ai: stored.ai, + activeFilters: stored.activeFilters, + kubernetes: stored.kubernetes, + cache: stored.cache, + output: stored.output, + notifications: { + ...legacyNotifications, + ...stored.notifications, + }, + }; +}; diff --git a/src/config/schema.ts b/src/config/schema.ts new file mode 100644 index 0000000..b00b938 --- /dev/null +++ b/src/config/schema.ts @@ -0,0 +1,79 @@ +export interface KDMConfig { + ai?: AIConfig; + activeFilters?: string[]; + kubernetes?: KubernetesConfig; + cache?: CacheConfig; + output?: OutputConfig; + notifications?: NotificationConfig; +} + +export interface AIConfig { + providers: AIProviderConfig[]; + defaultProvider?: string; +} + +export interface AIProviderConfig { + name: string; + model: string; + password?: string; + baseUrl?: string; + temperature?: number; + topP?: number; + topK?: number; + maxTokens?: number; + customHeaders?: Record; +} + +export interface KubernetesConfig { + kubeconfig?: string; + kubecontext?: string; + namespace?: string; +} + +export interface CacheConfig { + type: 'file' | 'memory' | 's3' | 'gcs' | 'azure'; + enabled: boolean; + path?: string; + bucket?: string; + region?: string; +} + +export interface OutputConfig { + format: 'text' | 'json'; + language: string; +} + +export interface NotificationConfig { + service: 'discord' | 'email' | 'none'; + discordWebhook?: string; + emailHost?: string; + emailPort?: number; + emailUser?: string; + emailTo?: string; + emailPassword?: string; + alertCooldown?: number; +} + +export type LegacyNotificationConfig = { + notification_service?: 'discord' | 'email' | 'none'; + discord_webhook?: string; + email_host?: string; + email_port?: number; + email_user?: string; + email_to?: string; + email_password?: string; + alert_cooldown?: number; +}; +export type StoredKDMConfig = KDMConfig & LegacyNotificationConfig; + +export const defaultKDMConfig: Required> = { + activeFilters: [], + cache: { + type: 'file', + enabled: true, + }, + output: { + format: 'text', + language: 'english', + }, +}; diff --git a/src/config/store.ts b/src/config/store.ts new file mode 100644 index 0000000..32ba40f --- /dev/null +++ b/src/config/store.ts @@ -0,0 +1,103 @@ +import Conf from 'conf'; +import { + defaultKDMConfig, + type AIConfig, + type CacheConfig, + type KDMConfig, + type KubernetesConfig, + type LegacyNotificationConfig, + type NotificationConfig, + type OutputConfig, + type StoredKDMConfig, +} from './schema'; +import { mergeLegacyConfig } from './migration'; + +const config = new Conf({ + projectName: 'kdm-cli', +}); + +const withDefaults = (stored: StoredKDMConfig): KDMConfig => ({ + ...defaultKDMConfig, + ...mergeLegacyConfig(stored), + cache: { + ...defaultKDMConfig.cache, + ...stored.cache, + }, + output: { + ...defaultKDMConfig.output, + ...stored.output, + }, + activeFilters: stored.activeFilters ?? defaultKDMConfig.activeFilters, +}); + +export const getConfig = (): KDMConfig => withDefaults(config.store); + +export const updateConfig = (nextConfig: KDMConfig) => { + Object.entries(nextConfig).forEach(([key, value]) => { + config.set(key as keyof StoredKDMConfig, value as never); + }); +}; + +export const setConfigValue = (key: Key, value: KDMConfig[Key]) => { + config.set(key as keyof StoredKDMConfig, value as never); +}; + +export const clearConfig = () => config.clear(); + +export const getAIConfig = (): AIConfig => getConfig().ai ?? { providers: [] }; +export const setAIConfig = (ai: AIConfig) => setConfigValue('ai', ai); + +export const getActiveFilters = (): string[] => getConfig().activeFilters ?? []; +export const setActiveFilters = (activeFilters: string[]) => setConfigValue('activeFilters', activeFilters); + +export const getKubernetesConfig = (): KubernetesConfig => getConfig().kubernetes ?? {}; +export const setKubernetesConfig = (kubernetes: KubernetesConfig) => + setConfigValue('kubernetes', kubernetes); + +export const getCacheConfig = (): CacheConfig => getConfig().cache ?? defaultKDMConfig.cache; +export const setCacheConfig = (cache: CacheConfig) => setConfigValue('cache', cache); + +export const getOutputConfig = (): OutputConfig => getConfig().output ?? defaultKDMConfig.output; +export const setOutputConfig = (output: OutputConfig) => setConfigValue('output', output); + +export const getNotificationConfig = (): NotificationConfig => ({ + service: 'none', + ...getConfig().notifications, +}); + +export const setNotificationConfig = (notifications: NotificationConfig) => + setConfigValue('notifications', notifications); + +export const getLegacyConfig = (): LegacyNotificationConfig => { + const notifications = getNotificationConfig(); + return { + notification_service: notifications.service, + discord_webhook: notifications.discordWebhook, + email_host: notifications.emailHost, + email_port: notifications.emailPort, + email_user: notifications.emailUser, + email_to: notifications.emailTo, + email_password: notifications.emailPassword, + alert_cooldown: notifications.alertCooldown, + }; +}; + +export const getLegacyValue = ( + key: Key, +): LegacyNotificationConfig[Key] => { + const legacy = getLegacyConfig(); + return legacy[key]; +}; + +export const setLegacyValue = ( + key: Key, + value: LegacyNotificationConfig[Key], +) => { + config.set(key, value as never); +}; + +export const deleteLegacyValue = (key: keyof LegacyNotificationConfig) => { + config.delete(key); +}; + +export const rawConfigStore = config; diff --git a/src/utils/config.ts b/src/utils/config.ts index f18cb42..85178ca 100644 --- a/src/utils/config.ts +++ b/src/utils/config.ts @@ -1,46 +1,50 @@ -import Conf from 'conf'; +import { + clearConfig as clearStoredConfig, + deleteLegacyValue, + getLegacyConfig, + getLegacyValue, + setLegacyValue, +} from '../config/store'; +import type { LegacyNotificationConfig } from '../config/schema'; -interface KDMConfig { - notification_service?: 'discord' | 'email' | 'none'; - discord_webhook?: string; - email_host?: string; - email_port?: number; - email_user?: string; - email_to?: string; - email_password?: string; - alert_cooldown?: number; // in seconds -} +export const getConfig = () => getLegacyConfig(); -const config = new Conf({ - projectName: 'kdm-cli', -}); +const sensitiveLegacyKeys = new Set(['email_password']); -export const getConfig = () => config.store; -export const setConfig = (key: keyof KDMConfig, value: any) => config.set(key, value); -export const deleteConfig = (key: keyof KDMConfig) => config.delete(key); -export const clearConfig = () => config.clear(); +export const setConfig = ( + key: Key, + value: LegacyNotificationConfig[Key], +) => { + if (sensitiveLegacyKeys.has(key)) { + throw new Error(`${key} must not be stored in config. Use the KDM_SMTP_PASSWORD environment variable instead.`); + } + setLegacyValue(key, value); +}; + +export const deleteConfig = (key: keyof LegacyNotificationConfig) => deleteLegacyValue(key); +export const clearConfig = () => clearStoredConfig(); export const clearNotificationCredentials = () => { - config.delete('discord_webhook'); - config.delete('email_host'); - config.delete('email_port'); - config.delete('email_user'); - config.delete('email_to'); - config.delete('email_password'); + deleteLegacyValue('discord_webhook'); + deleteLegacyValue('email_host'); + deleteLegacyValue('email_port'); + deleteLegacyValue('email_user'); + deleteLegacyValue('email_to'); + deleteLegacyValue('email_password'); }; // Helper for sensitive data - always use environment variables export const getSMTPSettings = () => { return { - host: config.get('email_host'), - port: config.get('email_port') || 587, + host: getLegacyValue('email_host'), + port: getLegacyValue('email_port') || 587, auth: { - user: config.get('email_user'), + user: getLegacyValue('email_user'), pass: process.env.KDM_SMTP_PASSWORD !== undefined ? process.env.KDM_SMTP_PASSWORD - : config.get('email_password'), + : getLegacyValue('email_password'), }, - to: config.get('email_to'), + to: getLegacyValue('email_to'), }; };