From 77ae49cc036c2eeaa331a468d9bc52246e51a534 Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Mon, 13 Jul 2026 23:51:45 +0000 Subject: [PATCH] fix: fail fast on empty api key and profile inputs instead of falling back Empty --api-key/--profile flags and RESEND_API_KEY/RESEND_PROFILE env vars were treated as missing by truthiness checks, so commands silently fell back to other credentials and could target the wrong account. Explicitly provided empty values now raise a clear validation error at the point they would be consulted, and logout with an empty --profile no longer removes all profiles. Co-authored-by: Bu Kinoshita --- src/commands/auth/logout.ts | 2 +- src/commands/whoami.ts | 22 +++--- src/lib/client.ts | 4 +- src/lib/config.ts | 46 +++++------ src/utils/require-non-empty.ts | 14 ++++ tests/commands/auth/logout.test.ts | 35 +++++++- tests/commands/whoami.test.ts | 40 ++++++++++ tests/lib/client.test.ts | 110 ++++++++++++++++++++++++++ tests/lib/config-async.test.ts | 72 +++++++++++++++++ tests/lib/config.test.ts | 48 +++++++++++ tests/utils/require-non-empty.test.ts | 24 ++++++ 11 files changed, 378 insertions(+), 39 deletions(-) create mode 100644 src/utils/require-non-empty.ts create mode 100644 tests/utils/require-non-empty.test.ts diff --git a/src/commands/auth/logout.ts b/src/commands/auth/logout.ts index 1b79f856..1c978700 100644 --- a/src/commands/auth/logout.ts +++ b/src/commands/auth/logout.ts @@ -54,7 +54,7 @@ If no credentials file exists, exits cleanly with no error.`, } const profileFlag = globalOpts.profile; - const logoutAll = !profileFlag; + const logoutAll = profileFlag === undefined; // For logoutAll we don't need a specific profile; for single-profile // removal, the user-supplied flag is the source of truth. We deliberately // avoid resolveProfileName() so a corrupted credentials file doesn't diff --git a/src/commands/whoami.ts b/src/commands/whoami.ts index c46512c1..10d41c95 100644 --- a/src/commands/whoami.ts +++ b/src/commands/whoami.ts @@ -53,21 +53,19 @@ Shows which profile is active and where the active credential comes from.`, ); if (!resolved) { - const requestedProfile = profileFlag - ? profileFlag - : resolveProfileName(profileFlag); + const requestedProfile = profileFlag ?? resolveProfileName(profileFlag); const profiles = listProfiles(); const profileExists = profiles.some((p) => p.name === requestedProfile); - const explicitProfile = profileFlag || process.env.RESEND_PROFILE; + const explicitProfile = profileFlag ?? process.env.RESEND_PROFILE; + const explicitProfileNotFound = + explicitProfile !== undefined && !profileExists; - const message = - explicitProfile && !profileExists - ? `Profile "${requestedProfile}" not found.\nAvailable profiles: ${profiles.map((p) => p.name).join(', ') || '(none)'}` - : 'Not authenticated.\nRun `resend login` to get started.'; - const code = - explicitProfile && !profileExists - ? 'profile_not_found' - : 'not_authenticated'; + const message = explicitProfileNotFound + ? `Profile "${requestedProfile}" not found.\nAvailable profiles: ${profiles.map((p) => p.name).join(', ') || '(none)'}` + : 'Not authenticated.\nRun `resend login` to get started.'; + const code = explicitProfileNotFound + ? 'profile_not_found' + : 'not_authenticated'; if (globalOpts.json || !isInteractive()) { outputError({ message, code }, { json: globalOpts.json }); diff --git a/src/lib/client.ts b/src/lib/client.ts index af0064f6..b6587de8 100644 --- a/src/lib/client.ts +++ b/src/lib/client.ts @@ -37,7 +37,7 @@ export async function createClient( ): Promise { const resolved = await resolveAuthentication(flagValue, profileName); if (!resolved) { - if (profileName) { + if (profileName !== undefined) { const profiles = listProfiles(); const exists = profiles.some((p) => p.name === profileName); if (!exists) { @@ -64,7 +64,7 @@ export async function requireClient( try { const resolved = await resolveAuthentication(opts.apiKey, profileName); if (!resolved) { - if (profileName) { + if (profileName !== undefined) { const profiles = listProfiles(); const exists = profiles.some((p) => p.name === profileName); if (!exists) { diff --git a/src/lib/config.ts b/src/lib/config.ts index a6a1d84d..1c188bc8 100644 --- a/src/lib/config.ts +++ b/src/lib/config.ts @@ -7,6 +7,7 @@ import { } from 'node:fs'; import { homedir } from 'node:os'; import { join } from 'node:path'; +import { requireNonEmpty } from '../utils/require-non-empty'; import { CorruptedCredentialsError } from './corrupted-credentials-error'; import { type CredentialBackend, @@ -206,12 +207,16 @@ export function writeCredentials(creds: CredentialsFile): string { } export function resolveProfileName(flagValue?: string): string { - if (flagValue) { - return flagValue; + const flagProfile = requireNonEmpty(flagValue, '--profile'); + if (flagProfile !== undefined) { + return flagProfile; } - const envProfile = process.env.RESEND_PROFILE; - if (envProfile) { + const envProfile = requireNonEmpty( + process.env.RESEND_PROFILE, + 'RESEND_PROFILE', + ); + if (envProfile !== undefined) { return envProfile; } @@ -227,12 +232,13 @@ export function resolveApiKey( flagValue?: string, profileName?: string, ): ResolvedApiKey | null { - if (flagValue) { - return { type: 'api_key', key: flagValue, source: 'flag' }; + const flagKey = requireNonEmpty(flagValue, '--api-key'); + if (flagKey !== undefined) { + return { type: 'api_key', key: flagKey, source: 'flag' }; } - const envKey = process.env.RESEND_API_KEY; - if (envKey) { + const envKey = requireNonEmpty(process.env.RESEND_API_KEY, 'RESEND_API_KEY'); + if (envKey !== undefined) { return { type: 'api_key', key: envKey, source: 'env' }; } @@ -307,7 +313,8 @@ export function removeApiKey(profileName?: string): string { throw new Error('No credentials file found.'); } - const profile = profileName || resolveProfileName(); + const profile = + requireNonEmpty(profileName, '--profile') ?? resolveProfileName(); if (!creds.profiles[profile]) { throw new Error( `Profile "${profile}" not found. Available profiles: ${Object.keys(creds.profiles).join(', ')}`, @@ -459,21 +466,18 @@ export async function resolveAuthentication( profileName?: string, options?: { refresh?: boolean }, ): Promise { - if (flagValue) { - return { type: 'api_key', key: flagValue, source: 'flag' }; + const flagKey = requireNonEmpty(flagValue, '--api-key'); + if (flagKey !== undefined) { + return { type: 'api_key', key: flagKey, source: 'flag' }; } - const envKey = process.env.RESEND_API_KEY; - if (envKey) { + const envKey = requireNonEmpty(process.env.RESEND_API_KEY, 'RESEND_API_KEY'); + if (envKey !== undefined) { return { type: 'api_key', key: envKey, source: 'env' }; } const creds = readCredentials(); - const profile = - profileName || - process.env.RESEND_PROFILE || - creds?.active_profile || - 'default'; + const profile = resolveProfileName(profileName); if (creds?.storage === 'secure_storage' && creds.profiles[profile]) { const credential = creds.profiles[profile]; @@ -642,11 +646,7 @@ export async function storeOAuthGrant( export async function removeApiKeyAsync(profileName?: string): Promise { return withFileLock(getCredentialsLockPath(), async () => { const creds = readCredentials(); - const profile = - profileName || - process.env.RESEND_PROFILE || - creds?.active_profile || - 'default'; + const profile = resolveProfileName(profileName); if (!creds?.profiles[profile]) { throw new Error( diff --git a/src/utils/require-non-empty.ts b/src/utils/require-non-empty.ts new file mode 100644 index 00000000..ee7bbe07 --- /dev/null +++ b/src/utils/require-non-empty.ts @@ -0,0 +1,14 @@ +export const requireNonEmpty = ( + value: string | undefined, + source: string, +): string | undefined => { + if (value === undefined) { + return undefined; + } + if (value.trim().length === 0) { + throw new Error( + `${source} is set but empty. Provide a non-empty value or remove it.`, + ); + } + return value; +}; diff --git a/tests/commands/auth/logout.test.ts b/tests/commands/auth/logout.test.ts index adf5f157..9f4841f7 100644 --- a/tests/commands/auth/logout.test.ts +++ b/tests/commands/auth/logout.test.ts @@ -1,4 +1,10 @@ -import { existsSync, mkdirSync, rmSync, writeFileSync } from 'node:fs'; +import { + existsSync, + mkdirSync, + readFileSync, + rmSync, + writeFileSync, +} from 'node:fs'; import { tmpdir } from 'node:os'; import { join } from 'node:path'; import { @@ -133,6 +139,33 @@ describe('logout command', () => { expect(output.profile).toBe('staging'); }); + it('does not remove all profiles when --profile is empty', async () => { + spies = setupOutputSpies(); + errorSpy = vi.spyOn(console, 'error').mockImplementation(() => {}); + exitSpy = mockExitThrow(); + writeCredentials({ staging: 're_staging_key', production: 're_prod_key' }); + + const { Command } = await import('@commander-js/extra-typings'); + const { logoutCommand } = await import('../../../src/commands/auth/logout'); + const program = new Command() + .option('--profile ') + .option('--json') + .addCommand(logoutCommand); + + await expectExit1(() => + program.parseAsync(['logout', '--profile', ''], { from: 'user' }), + ); + + const configPath = join(tmpDir, 'resend', 'credentials.json'); + expect(existsSync(configPath)).toBe(true); + const remaining = JSON.parse(readFileSync(configPath, 'utf-8')); + expect(remaining.profiles.staging).toBeDefined(); + expect(remaining.profiles.production).toBeDefined(); + + const output = JSON.parse(errorSpy?.mock.calls[0][0] as string); + expect(output.error.code).toBe('remove_failed'); + }); + it('exits with error when file removal fails', async () => { spies = setupOutputSpies(); errorSpy = vi.spyOn(console, 'error').mockImplementation(() => {}); diff --git a/tests/commands/whoami.test.ts b/tests/commands/whoami.test.ts index 68581790..84cd350d 100644 --- a/tests/commands/whoami.test.ts +++ b/tests/commands/whoami.test.ts @@ -76,6 +76,46 @@ describe('whoami command', () => { expect(parsed.error.message).toContain('not found'); }); + it('errors on empty RESEND_PROFILE instead of falling back to the active profile', async () => { + const configDir = join(tmpDir, 'resend'); + mkdirSync(configDir, { recursive: true }); + writeFileSync( + join(configDir, 'credentials.json'), + JSON.stringify({ + active_profile: 'production', + profiles: { production: { api_key: 're_test_key_abcd' } }, + }), + ); + process.env.RESEND_PROFILE = ''; + + spies = setupOutputSpies(); + + const { whoamiCommand } = await import('../../src/commands/whoami'); + await expect( + whoamiCommand.parseAsync([], { from: 'user' }), + ).rejects.toThrow('RESEND_PROFILE is set but empty'); + }); + + it('errors on empty RESEND_API_KEY instead of falling back to stored credentials', async () => { + const configDir = join(tmpDir, 'resend'); + mkdirSync(configDir, { recursive: true }); + writeFileSync( + join(configDir, 'credentials.json'), + JSON.stringify({ + active_profile: 'production', + profiles: { production: { api_key: 're_test_key_abcd' } }, + }), + ); + process.env.RESEND_API_KEY = ''; + + spies = setupOutputSpies(); + + const { whoamiCommand } = await import('../../src/commands/whoami'); + await expect( + whoamiCommand.parseAsync([], { from: 'user' }), + ).rejects.toThrow('RESEND_API_KEY is set but empty'); + }); + it('shows authenticated JSON when key exists in config', async () => { const configDir = join(tmpDir, 'resend'); mkdirSync(configDir, { recursive: true }); diff --git a/tests/lib/client.test.ts b/tests/lib/client.test.ts index 6ade9676..550a9b5c 100644 --- a/tests/lib/client.test.ts +++ b/tests/lib/client.test.ts @@ -87,6 +87,116 @@ describe('createClient', () => { rmSync(tmpDir, { recursive: true, force: true }); }); + + it('rejects an empty api key flag instead of falling back to env', async () => { + process.env.RESEND_API_KEY = 're_env_key'; + process.env.RESEND_CREDENTIAL_STORE = 'file'; + const { createClient } = await import('../../src/lib/client'); + await expect(createClient('')).rejects.toThrow( + '--api-key is set but empty', + ); + }); + + it('rejects an empty profile name instead of using the active profile', async () => { + delete process.env.RESEND_API_KEY; + process.env.RESEND_CREDENTIAL_STORE = 'file'; + const tmpDir = join( + tmpdir(), + `resend-test-${Date.now()}-${Math.random().toString(36).slice(2)}`, + ); + mkdirSync(tmpDir, { recursive: true }); + process.env.XDG_CONFIG_HOME = tmpDir; + + const configDir = join(tmpDir, 'resend'); + mkdirSync(configDir, { recursive: true }); + writeFileSync( + join(configDir, 'credentials.json'), + JSON.stringify({ + active_profile: 'default', + profiles: { default: { api_key: 're_default_key' } }, + }), + ); + + const { createClient } = await import('../../src/lib/client'); + await expect(createClient(undefined, '')).rejects.toThrow( + '--profile is set but empty', + ); + + rmSync(tmpDir, { recursive: true, force: true }); + }); +}); + +describe('requireClient input validation', () => { + const restoreEnv = captureTestEnv(); + let tmpDir: string; + let exitSpy: MockInstance | undefined; + + beforeEach(() => { + tmpDir = join( + tmpdir(), + `resend-test-${Date.now()}-${Math.random().toString(36).slice(2)}`, + ); + mkdirSync(tmpDir, { recursive: true }); + process.env.XDG_CONFIG_HOME = tmpDir; + process.env.RESEND_CREDENTIAL_STORE = 'file'; + delete process.env.RESEND_API_KEY; + delete process.env.RESEND_PROFILE; + }); + + afterEach(() => { + restoreEnv(); + exitSpy?.mockRestore(); + exitSpy = undefined; + rmSync(tmpDir, { recursive: true, force: true }); + }); + + it('exits with auth_error when --profile is empty instead of using the active profile', async () => { + const configDir = join(tmpDir, 'resend'); + mkdirSync(configDir, { recursive: true }); + writeFileSync( + join(configDir, 'credentials.json'), + JSON.stringify({ + active_profile: 'default', + profiles: { default: { api_key: 're_default_key' } }, + }), + ); + + setupOutputSpies(); + const errSpy = vi.spyOn(console, 'error').mockImplementation(() => {}); + exitSpy = mockExitThrow(); + + const { requireClient } = await import('../../src/lib/client'); + await expectExit1(() => requireClient({ json: true, profile: '' })); + + const output = errSpy.mock.calls[0][0] as string; + expect(output).toContain('auth_error'); + expect(output).toContain('--profile is set but empty'); + errSpy.mockRestore(); + }); + + it('exits with auth_error when --api-key is empty instead of falling back', async () => { + const configDir = join(tmpDir, 'resend'); + mkdirSync(configDir, { recursive: true }); + writeFileSync( + join(configDir, 'credentials.json'), + JSON.stringify({ + active_profile: 'default', + profiles: { default: { api_key: 're_default_key' } }, + }), + ); + + setupOutputSpies(); + const errSpy = vi.spyOn(console, 'error').mockImplementation(() => {}); + exitSpy = mockExitThrow(); + + const { requireClient } = await import('../../src/lib/client'); + await expectExit1(() => requireClient({ json: true, apiKey: '' })); + + const output = errSpy.mock.calls[0][0] as string; + expect(output).toContain('auth_error'); + expect(output).toContain('--api-key is set but empty'); + errSpy.mockRestore(); + }); }); describe('requireClient permission check', () => { diff --git a/tests/lib/config-async.test.ts b/tests/lib/config-async.test.ts index dfd15626..ee3bc4ce 100644 --- a/tests/lib/config-async.test.ts +++ b/tests/lib/config-async.test.ts @@ -211,6 +211,78 @@ describe('resolveAuthentication', () => { expect(result).toBeNull(); expect(mockBackend.get).not.toHaveBeenCalled(); }); + + it('throws on empty --api-key flag instead of falling back to env', async () => { + process.env.RESEND_API_KEY = 're_env_key'; + const { resolveAuthentication } = await import('../../src/lib/config'); + await expect(resolveAuthentication('')).rejects.toThrow( + '--api-key is set but empty', + ); + }); + + it('throws on empty RESEND_API_KEY instead of falling back to stored credentials', async () => { + const configDir = join(tmpDir, 'resend'); + mkdirSync(configDir, { recursive: true }); + writeFileSync( + join(configDir, 'credentials.json'), + JSON.stringify({ + active_profile: 'default', + profiles: { default: { api_key: 're_file_key' } }, + }), + ); + process.env.RESEND_API_KEY = ''; + + const { resolveAuthentication } = await import('../../src/lib/config'); + await expect(resolveAuthentication()).rejects.toThrow( + 'RESEND_API_KEY is set but empty', + ); + }); + + it('throws on empty profile name instead of resolving the active profile', async () => { + const configDir = join(tmpDir, 'resend'); + mkdirSync(configDir, { recursive: true }); + writeFileSync( + join(configDir, 'credentials.json'), + JSON.stringify({ + active_profile: 'default', + profiles: { default: { api_key: 're_file_key' } }, + }), + ); + + const { resolveAuthentication } = await import('../../src/lib/config'); + await expect(resolveAuthentication(undefined, '')).rejects.toThrow( + '--profile is set but empty', + ); + }); + + it('throws on empty RESEND_PROFILE instead of resolving the active profile', async () => { + const configDir = join(tmpDir, 'resend'); + mkdirSync(configDir, { recursive: true }); + writeFileSync( + join(configDir, 'credentials.json'), + JSON.stringify({ + active_profile: 'default', + profiles: { default: { api_key: 're_file_key' } }, + }), + ); + process.env.RESEND_PROFILE = ''; + + const { resolveAuthentication } = await import('../../src/lib/config'); + await expect(resolveAuthentication()).rejects.toThrow( + 'RESEND_PROFILE is set but empty', + ); + }); + + it('uses flag key without consulting an empty RESEND_API_KEY', async () => { + process.env.RESEND_API_KEY = ''; + const { resolveAuthentication } = await import('../../src/lib/config'); + const result = await resolveAuthentication('re_flag_key'); + expect(result).toEqual({ + type: 'api_key', + key: 're_flag_key', + source: 'flag', + }); + }); }); describe('storeApiKeyAsync', () => { diff --git a/tests/lib/config.test.ts b/tests/lib/config.test.ts index 0273148c..037210c7 100644 --- a/tests/lib/config.test.ts +++ b/tests/lib/config.test.ts @@ -175,6 +175,37 @@ describe('resolveApiKey', () => { const result = resolveApiKey(undefined, 'nonexistent'); expect(result).toBeNull(); }); + + it('throws on empty flag value instead of falling back to env', () => { + process.env.RESEND_API_KEY = 're_env_key'; + expect(() => resolveApiKey('')).toThrow('--api-key is set but empty'); + }); + + it('throws on empty RESEND_API_KEY instead of falling back to config', () => { + process.env.RESEND_API_KEY = ''; + process.env.XDG_CONFIG_HOME = tmpDir; + const configDir = join(tmpDir, 'resend'); + mkdirSync(configDir, { recursive: true }); + writeFileSync( + join(configDir, 'credentials.json'), + JSON.stringify({ + active_profile: 'default', + profiles: { default: { api_key: 're_config_key' } }, + }), + ); + + expect(() => resolveApiKey()).toThrow('RESEND_API_KEY is set but empty'); + }); + + it('uses flag value without consulting an empty RESEND_API_KEY', () => { + process.env.RESEND_API_KEY = ''; + const result = resolveApiKey('re_flag_key'); + expect(result).toEqual({ + type: 'api_key', + key: 're_flag_key', + source: 'flag', + }); + }); }); describe('resolveProfileName', () => { @@ -224,6 +255,23 @@ describe('resolveProfileName', () => { delete process.env.RESEND_PROFILE; expect(resolveProfileName()).toBe('default'); }); + + it('throws on empty flag value instead of falling back', () => { + process.env.RESEND_PROFILE = 'env_profile'; + expect(() => resolveProfileName('')).toThrow('--profile is set but empty'); + }); + + it('throws on empty RESEND_PROFILE instead of falling back', () => { + process.env.RESEND_PROFILE = ''; + expect(() => resolveProfileName()).toThrow( + 'RESEND_PROFILE is set but empty', + ); + }); + + it('uses flag value without consulting an empty RESEND_PROFILE', () => { + process.env.RESEND_PROFILE = ''; + expect(resolveProfileName('flag_profile')).toBe('flag_profile'); + }); }); describe('storeApiKey', () => { diff --git a/tests/utils/require-non-empty.test.ts b/tests/utils/require-non-empty.test.ts new file mode 100644 index 00000000..db661672 --- /dev/null +++ b/tests/utils/require-non-empty.test.ts @@ -0,0 +1,24 @@ +import { describe, expect, it } from 'vitest'; +import { requireNonEmpty } from '../../src/utils/require-non-empty'; + +describe('requireNonEmpty', () => { + it('returns undefined when the value is undefined', () => { + expect(requireNonEmpty(undefined, '--api-key')).toBeUndefined(); + }); + + it('returns the value when it is non-empty', () => { + expect(requireNonEmpty('re_key', '--api-key')).toBe('re_key'); + }); + + it('throws when the value is an empty string', () => { + expect(() => requireNonEmpty('', '--api-key')).toThrow( + '--api-key is set but empty. Provide a non-empty value or remove it.', + ); + }); + + it('throws when the value is whitespace only', () => { + expect(() => requireNonEmpty(' ', 'RESEND_PROFILE')).toThrow( + 'RESEND_PROFILE is set but empty. Provide a non-empty value or remove it.', + ); + }); +});