From 7c14a294479a78611d6df3fc8be90665874aad00 Mon Sep 17 00:00:00 2001 From: "heecheol.park" Date: Mon, 27 Apr 2026 18:04:08 +0900 Subject: [PATCH] fix(config): harden CLI option and config-file parsing against bad input MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Type-guard --token and --cookie validation so passing null or a non-string produces a clean validation error instead of a TypeError on .trim() - Surface a stderr warning when ~/.confluence-cli/config.json fails to parse, instead of silently returning null and falling through to "No configuration found!" — making corrupted-config debugging straightforward --- lib/config.js | 8 +++-- tests/config.test.js | 70 ++++++++++++++++++++++++++++++++++++++++++- tests/profile.test.js | 33 ++++++++++++++++++++ 3 files changed, 107 insertions(+), 4 deletions(-) diff --git a/lib/config.js b/lib/config.js index 9c8bc3c..587359e 100644 --- a/lib/config.js +++ b/lib/config.js @@ -235,7 +235,9 @@ function readConfigFile() { } return raw; - } catch { + } catch (error) { + console.error(chalk.yellow(`⚠ Failed to parse config file at ${CONFIG_FILE}: ${error.message}`)); + console.error(chalk.yellow(' Run "confluence init" to recreate it.')); return null; } } @@ -259,7 +261,7 @@ const validateCliOptions = (options) => { errors.push('--domain cannot be empty'); } - if (options.token !== undefined && !options.token.trim()) { + if (options.token !== undefined && (typeof options.token !== 'string' || !options.token.trim())) { errors.push('--token cannot be empty'); } @@ -304,7 +306,7 @@ const validateCliOptions = (options) => { } } - if (normAuthType === 'cookie' && options.cookie !== undefined && !options.cookie.trim()) { + if (normAuthType === 'cookie' && options.cookie !== undefined && (typeof options.cookie !== 'string' || !options.cookie.trim())) { errors.push('--cookie cannot be empty when using cookie authentication'); } diff --git a/tests/config.test.js b/tests/config.test.js index a31127c..b3193ac 100644 --- a/tests/config.test.js +++ b/tests/config.test.js @@ -1,4 +1,4 @@ -const { getConfig } = require('../lib/config'); +const { getConfig, initConfig } = require('../lib/config'); // Save and restore all relevant env vars around each test const ENV_KEYS = [ @@ -282,3 +282,71 @@ describe('getConfig env var aliases', () => { } }); }); + +describe('initConfig CLI option validation', () => { + let exitSpy; + let errorSpy; + let logSpy; + + beforeEach(() => { + exitSpy = jest.spyOn(process, 'exit').mockImplementation(() => { + throw new Error('process.exit called'); + }); + errorSpy = jest.spyOn(console, 'error').mockImplementation(() => {}); + logSpy = jest.spyOn(console, 'log').mockImplementation(() => {}); + }); + + afterEach(() => { + exitSpy.mockRestore(); + errorSpy.mockRestore(); + logSpy.mockRestore(); + }); + + test('null --token surfaces a validation error instead of crashing', async () => { + await expect(initConfig({ + domain: 'example.com', + token: null, + authType: 'bearer', + })).rejects.toThrow('process.exit called'); + + expect(errorSpy).toHaveBeenCalledWith( + expect.stringMatching(/--token cannot be empty/) + ); + }); + + test('non-string --token surfaces a validation error instead of crashing', async () => { + await expect(initConfig({ + domain: 'example.com', + token: 12345, + authType: 'bearer', + })).rejects.toThrow('process.exit called'); + + expect(errorSpy).toHaveBeenCalledWith( + expect.stringMatching(/--token cannot be empty/) + ); + }); + + test('null --cookie with cookie auth surfaces a validation error instead of crashing', async () => { + await expect(initConfig({ + domain: 'example.com', + authType: 'cookie', + cookie: null, + })).rejects.toThrow('process.exit called'); + + expect(errorSpy).toHaveBeenCalledWith( + expect.stringMatching(/--cookie cannot be empty/) + ); + }); + + test('whitespace-only --token still flagged as empty (regression guard)', async () => { + await expect(initConfig({ + domain: 'example.com', + token: ' ', + authType: 'bearer', + })).rejects.toThrow('process.exit called'); + + expect(errorSpy).toHaveBeenCalledWith( + expect.stringMatching(/--token cannot be empty/) + ); + }); +}); diff --git a/tests/profile.test.js b/tests/profile.test.js index 14b7f57..73814e7 100644 --- a/tests/profile.test.js +++ b/tests/profile.test.js @@ -486,4 +486,37 @@ describe('Profile management', () => { expect(fileChmod.mode).toBe(0o600); }); }); + + describe('readConfigFile error reporting', () => { + test('logs a warning when the config file contains invalid JSON', () => { + fs.existsSync.mockImplementation((filePath) => { + if (filePath === CONFIG_FILE) return true; + return jest.requireActual('fs').existsSync(filePath); + }); + fs.readFileSync.mockImplementation((filePath, encoding) => { + if (filePath === CONFIG_FILE) return '{ not valid json'; + return jest.requireActual('fs').readFileSync(filePath, encoding); + }); + + const errorSpy = jest.spyOn(console, 'error').mockImplementation(() => {}); + const logSpy = jest.spyOn(console, 'log').mockImplementation(() => {}); + const exitSpy = jest.spyOn(process, 'exit').mockImplementation(() => { + throw new Error('process.exit called'); + }); + + try { + expect(() => getConfig()).toThrow('process.exit called'); + expect(errorSpy).toHaveBeenCalledWith( + expect.stringMatching(/Failed to parse config file/) + ); + expect(errorSpy).toHaveBeenCalledWith( + expect.stringMatching(/Run "confluence init" to recreate it/) + ); + } finally { + errorSpy.mockRestore(); + logSpy.mockRestore(); + exitSpy.mockRestore(); + } + }); + }); });