From e9e79b8fbe57c2921335f182e0e9443a2ec07f82 Mon Sep 17 00:00:00 2001 From: Juan Escalada Date: Sat, 17 Jan 2026 16:05:00 +0900 Subject: [PATCH 01/13] feat: add validation function for commitConfig regex --- src/config/index.ts | 53 +++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 53 insertions(+) diff --git a/src/config/index.ts b/src/config/index.ts index ca35c8b06..bfcbdf341 100644 --- a/src/config/index.ts +++ b/src/config/index.ts @@ -291,6 +291,59 @@ export const getRateLimit = () => { return config.rateLimit; }; +/** + * Validates that commit configuration uses valid regular expressions. + * @param config The commit configuration to validate + * @returns true if the commit configuration is valid, false otherwise + */ +export const validateCommitConfig = (config: CommitConfig): boolean => { + if (config.author?.email?.local?.block) { + try { + new RegExp(config.author?.email?.local?.block); + } catch (error: unknown) { + console.error( + `Invalid regular expression for author.email.local.block: ${config.author?.email?.local?.block}`, + ); + return false; + } + } + + if (config.author?.email?.domain?.allow) { + try { + new RegExp(config.author?.email?.domain?.allow); + } catch (error: unknown) { + console.error( + `Invalid regular expression for author.email.domain.allow: ${config.author?.email?.domain?.allow}`, + ); + return false; + } + } + + if (config.message?.block?.patterns) { + for (const pattern of config.message.block.patterns) { + try { + new RegExp(pattern); + } catch (error: unknown) { + console.error(`Invalid regular expression for message.block.patterns: ${pattern}`); + return false; + } + } + } + + if (config.diff?.block?.patterns) { + for (const pattern of config.diff.block.patterns) { + try { + new RegExp(pattern); + } catch (error: unknown) { + console.error(`Invalid regular expression for diff.block.patterns: ${pattern}`); + return false; + } + } + } + + return true; +}; + // Function to handle configuration updates const handleConfigUpdate = async (newConfig: Configuration) => { console.log('Configuration updated from external source'); From c57b480474c025b4108a34d0f40399ad57e99e52 Mon Sep 17 00:00:00 2001 From: Juan Escalada Date: Sat, 17 Jan 2026 16:05:26 +0900 Subject: [PATCH 02/13] fix: add custom validation for config load and reload --- index.ts | 2 +- src/config/ConfigLoader.ts | 6 ++++++ src/config/index.ts | 19 ++++++++++++++++++- 3 files changed, 25 insertions(+), 2 deletions(-) diff --git a/index.ts b/index.ts index 553d7a2c4..433e8cd0a 100755 --- a/index.ts +++ b/index.ts @@ -48,7 +48,7 @@ if (argv.v) { process.exit(0); } -console.log('validating config'); +console.log('Validating config'); validate(); console.log('Setting up the proxy and Service'); diff --git a/src/config/ConfigLoader.ts b/src/config/ConfigLoader.ts index 22dd6abfd..aded80a54 100644 --- a/src/config/ConfigLoader.ts +++ b/src/config/ConfigLoader.ts @@ -7,6 +7,7 @@ import { EventEmitter } from 'events'; import envPaths from 'env-paths'; import { GitProxyConfig, Convert } from './generated/config'; import { Configuration, ConfigurationSource, FileSource, HttpSource, GitSource } from './types'; +import { validateConfig } from '.'; const execFileAsync = promisify(execFile); @@ -189,6 +190,11 @@ export class ConfigLoader extends EventEmitter { } else { console.log('Configuration has not changed, no update needed'); } + + if (!validateConfig(newConfig)) { + console.error('Invalid configuration, skipping reload'); + return; + } } catch (error: any) { console.error('Error reloading configuration:', error); this.emit('configurationError', error); diff --git a/src/config/index.ts b/src/config/index.ts index bfcbdf341..5dda2d9b6 100644 --- a/src/config/index.ts +++ b/src/config/index.ts @@ -1,7 +1,7 @@ import { existsSync, readFileSync } from 'fs'; import defaultSettings from '../../proxy.config.json'; -import { GitProxyConfig, Convert } from './generated/config'; +import { GitProxyConfig, Convert, CommitConfig } from './generated/config'; import { ConfigLoader } from './ConfigLoader'; import { Configuration } from './types'; import { serverConfig } from './env'; @@ -69,9 +69,26 @@ function loadFullConfiguration(): GitProxyConfig { _currentConfig = mergeConfigurations(defaultConfig, userSettings); + if (!validateConfig(_currentConfig)) { + console.error( + 'Invalid configuration: Please check your configuration file and restart GitProxy.', + ); + process.exit(1); + } + return _currentConfig; } +/** + * Performs additional custom validations on the configuration + * + * @param config The configuration to validate + * @returns true if the configuration is valid, false otherwise + */ +export const validateConfig = (config: GitProxyConfig): boolean => { + return validateCommitConfig(config.commitConfig ?? {}); +}; + /** * Merge configurations with environment variable overrides * @param {GitProxyConfig} defaultConfig - The default configuration From 2bbd787a2389141374d54d331c20623c0cf8b80f Mon Sep 17 00:00:00 2001 From: Juan Escalada Date: Sun, 18 Jan 2026 14:03:53 +0900 Subject: [PATCH 03/13] fix: extract validators into own file and implement validation chain --- src/config/ConfigLoader.ts | 2 +- src/config/index.ts | 66 ++---------------------------------- src/config/validators.ts | 69 ++++++++++++++++++++++++++++++++++++++ 3 files changed, 72 insertions(+), 65 deletions(-) create mode 100644 src/config/validators.ts diff --git a/src/config/ConfigLoader.ts b/src/config/ConfigLoader.ts index aded80a54..cadfaa14a 100644 --- a/src/config/ConfigLoader.ts +++ b/src/config/ConfigLoader.ts @@ -7,7 +7,7 @@ import { EventEmitter } from 'events'; import envPaths from 'env-paths'; import { GitProxyConfig, Convert } from './generated/config'; import { Configuration, ConfigurationSource, FileSource, HttpSource, GitSource } from './types'; -import { validateConfig } from '.'; +import { validateConfig } from './validators'; const execFileAsync = promisify(execFile); diff --git a/src/config/index.ts b/src/config/index.ts index 5dda2d9b6..c94ffb2e2 100644 --- a/src/config/index.ts +++ b/src/config/index.ts @@ -1,11 +1,12 @@ import { existsSync, readFileSync } from 'fs'; import defaultSettings from '../../proxy.config.json'; -import { GitProxyConfig, Convert, CommitConfig } from './generated/config'; +import { GitProxyConfig, Convert } from './generated/config'; import { ConfigLoader } from './ConfigLoader'; import { Configuration } from './types'; import { serverConfig } from './env'; import { getConfigFile } from './file'; +import { validateConfig } from './validators'; // Cache for current configuration let _currentConfig: GitProxyConfig | null = null; @@ -79,16 +80,6 @@ function loadFullConfiguration(): GitProxyConfig { return _currentConfig; } -/** - * Performs additional custom validations on the configuration - * - * @param config The configuration to validate - * @returns true if the configuration is valid, false otherwise - */ -export const validateConfig = (config: GitProxyConfig): boolean => { - return validateCommitConfig(config.commitConfig ?? {}); -}; - /** * Merge configurations with environment variable overrides * @param {GitProxyConfig} defaultConfig - The default configuration @@ -308,59 +299,6 @@ export const getRateLimit = () => { return config.rateLimit; }; -/** - * Validates that commit configuration uses valid regular expressions. - * @param config The commit configuration to validate - * @returns true if the commit configuration is valid, false otherwise - */ -export const validateCommitConfig = (config: CommitConfig): boolean => { - if (config.author?.email?.local?.block) { - try { - new RegExp(config.author?.email?.local?.block); - } catch (error: unknown) { - console.error( - `Invalid regular expression for author.email.local.block: ${config.author?.email?.local?.block}`, - ); - return false; - } - } - - if (config.author?.email?.domain?.allow) { - try { - new RegExp(config.author?.email?.domain?.allow); - } catch (error: unknown) { - console.error( - `Invalid regular expression for author.email.domain.allow: ${config.author?.email?.domain?.allow}`, - ); - return false; - } - } - - if (config.message?.block?.patterns) { - for (const pattern of config.message.block.patterns) { - try { - new RegExp(pattern); - } catch (error: unknown) { - console.error(`Invalid regular expression for message.block.patterns: ${pattern}`); - return false; - } - } - } - - if (config.diff?.block?.patterns) { - for (const pattern of config.diff.block.patterns) { - try { - new RegExp(pattern); - } catch (error: unknown) { - console.error(`Invalid regular expression for diff.block.patterns: ${pattern}`); - return false; - } - } - } - - return true; -}; - // Function to handle configuration updates const handleConfigUpdate = async (newConfig: Configuration) => { console.log('Configuration updated from external source'); diff --git a/src/config/validators.ts b/src/config/validators.ts new file mode 100644 index 000000000..77bb644cc --- /dev/null +++ b/src/config/validators.ts @@ -0,0 +1,69 @@ +import { GitProxyConfig } from './generated/config'; + +const validationChain = [validateCommitConfig]; + +/** + * Executes all custom validators on the configuration + * @param config The configuration to validate + * @returns true if the configuration is valid, false otherwise + */ +export const validateConfig = (config: GitProxyConfig): boolean => { + return validationChain.every((validator) => validator(config)); +}; + +/** + * Validates that commit configuration uses valid regular expressions. + * @param config The commit configuration to validate + * @returns true if the commit configuration is valid, false otherwise + */ +function validateCommitConfig(config: GitProxyConfig): boolean { + if (config.commitConfig?.author?.email?.local?.block) { + try { + new RegExp(config.commitConfig.author.email.local.block); + } catch (error: unknown) { + console.error( + `Invalid regular expression for commitConfig.author.email.local.block: ${config.commitConfig.author.email.local.block}`, + ); + return false; + } + } + + if (config.commitConfig?.author?.email?.domain?.allow) { + try { + new RegExp(config.commitConfig.author.email.domain.allow); + } catch (error: unknown) { + console.error( + `Invalid regular expression for commitConfig.author.email.domain.allow: ${config.commitConfig.author.email.domain.allow}`, + ); + return false; + } + } + + if (config.commitConfig?.message?.block?.patterns) { + for (const pattern of config.commitConfig.message.block.patterns) { + try { + new RegExp(pattern); + } catch (error: unknown) { + console.error( + `Invalid regular expression for commitConfig.message.block.patterns: ${pattern}`, + ); + return false; + } + } + } + + if (config.commitConfig?.diff?.block?.patterns) { + for (const pattern of config.commitConfig.diff.block.patterns) { + try { + new RegExp(pattern); + } catch (error: unknown) { + console.error( + `Invalid regular expression for commitConfig.diff.block.patterns: ${pattern}`, + ); + return false; + } + } + } + + return true; +} From 50041fcbf33ae1e611d0513f427695f57ec81a54 Mon Sep 17 00:00:00 2001 From: Juan Escalada Date: Sun, 18 Jan 2026 14:14:59 +0900 Subject: [PATCH 04/13] test: add configValidators test file --- test/configValidators.test.ts | 358 ++++++++++++++++++++++++++++++++++ 1 file changed, 358 insertions(+) create mode 100644 test/configValidators.test.ts diff --git a/test/configValidators.test.ts b/test/configValidators.test.ts new file mode 100644 index 000000000..c2bb2cd1a --- /dev/null +++ b/test/configValidators.test.ts @@ -0,0 +1,358 @@ +import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; +import { validateConfig } from '../src/config/validators'; +import { GitProxyConfig } from '../src/config/generated/config'; + +describe('validators', () => { + let consoleErrorSpy: ReturnType; + + beforeEach(() => { + consoleErrorSpy = vi.spyOn(console, 'error').mockImplementation(() => {}); + }); + + afterEach(() => { + consoleErrorSpy.mockRestore(); + }); + + describe('validateConfig', () => { + it('should return true for empty config', () => { + const config: GitProxyConfig = {}; + expect(validateConfig(config)).toBe(true); + }); + + it('should return true for config without commitConfig', () => { + const config: GitProxyConfig = { + proxyUrl: 'https://test.com', + cookieSecret: 'secret', + }; + expect(validateConfig(config)).toBe(true); + }); + + it('should return true for valid commitConfig with all valid regex patterns', () => { + const config: GitProxyConfig = { + commitConfig: { + author: { + email: { + local: { + block: '^admin.*', + }, + domain: { + allow: '.*@example\\.com$', + }, + }, + }, + message: { + block: { + patterns: ['^WIP:', 'TODO', '[Tt]est'], + }, + }, + diff: { + block: { + patterns: ['password', 'secret.*key'], + }, + }, + }, + }; + expect(validateConfig(config)).toBe(true); + expect(consoleErrorSpy).not.toHaveBeenCalled(); + }); + + it('should return false when commitConfig.author.email.local.block has invalid regex', () => { + const config: GitProxyConfig = { + commitConfig: { + author: { + email: { + local: { + block: '[invalid(regex', + }, + }, + }, + }, + }; + expect(validateConfig(config)).toBe(false); + expect(consoleErrorSpy).toHaveBeenCalledWith( + 'Invalid regular expression for commitConfig.author.email.local.block: [invalid(regex', + ); + }); + + it('should return false when commitConfig.author.email.domain.allow has invalid regex', () => { + const config: GitProxyConfig = { + commitConfig: { + author: { + email: { + domain: { + allow: '(unclosed group', + }, + }, + }, + }, + }; + expect(validateConfig(config)).toBe(false); + expect(consoleErrorSpy).toHaveBeenCalledWith( + 'Invalid regular expression for commitConfig.author.email.domain.allow: (unclosed group', + ); + }); + + it('should return false when commitConfig.message.block.patterns contains invalid regex', () => { + const config: GitProxyConfig = { + commitConfig: { + message: { + block: { + patterns: ['valid-pattern', '[invalid[bracket', 'another-valid'], + }, + }, + }, + }; + expect(validateConfig(config)).toBe(false); + expect(consoleErrorSpy).toHaveBeenCalledWith( + 'Invalid regular expression for commitConfig.message.block.patterns: [invalid[bracket', + ); + }); + + it('should return false when commitConfig.diff.block.patterns contains invalid regex', () => { + const config: GitProxyConfig = { + commitConfig: { + diff: { + block: { + patterns: ['password', '*invalid-quantifier'], + }, + }, + }, + }; + expect(validateConfig(config)).toBe(false); + expect(consoleErrorSpy).toHaveBeenCalledWith( + 'Invalid regular expression for commitConfig.diff.block.patterns: *invalid-quantifier', + ); + }); + + it('should return false on first invalid pattern in message.block.patterns array', () => { + const config: GitProxyConfig = { + commitConfig: { + message: { + block: { + patterns: ['[invalid1', '[invalid2'], + }, + }, + }, + }; + expect(validateConfig(config)).toBe(false); + // Only logs the first invalid pattern + expect(consoleErrorSpy).toHaveBeenCalledTimes(1); + expect(consoleErrorSpy).toHaveBeenCalledWith( + 'Invalid regular expression for commitConfig.message.block.patterns: [invalid1', + ); + }); + + it('should validate all regex fields independently', () => { + const config: GitProxyConfig = { + commitConfig: { + author: { + email: { + local: { + block: '^valid.*', + }, + domain: { + allow: '.*@valid\\.com$', + }, + }, + }, + message: { + block: { + patterns: ['valid-message'], + }, + }, + diff: { + block: { + patterns: ['valid-diff'], + }, + }, + }, + }; + expect(validateConfig(config)).toBe(true); + expect(consoleErrorSpy).not.toHaveBeenCalled(); + }); + + it('should handle partial commitConfig with only author.email.local.block', () => { + const config: GitProxyConfig = { + commitConfig: { + author: { + email: { + local: { + block: '^admin', + }, + }, + }, + }, + }; + expect(validateConfig(config)).toBe(true); + expect(consoleErrorSpy).not.toHaveBeenCalled(); + }); + + it('should handle partial commitConfig with only author.email.domain.allow', () => { + const config: GitProxyConfig = { + commitConfig: { + author: { + email: { + domain: { + allow: 'example\\.com', + }, + }, + }, + }, + }; + expect(validateConfig(config)).toBe(true); + expect(consoleErrorSpy).not.toHaveBeenCalled(); + }); + + it('should handle partial commitConfig with only message.block.patterns', () => { + const config: GitProxyConfig = { + commitConfig: { + message: { + block: { + patterns: ['WIP', 'TODO'], + }, + }, + }, + }; + expect(validateConfig(config)).toBe(true); + expect(consoleErrorSpy).not.toHaveBeenCalled(); + }); + + it('should handle partial commitConfig with only diff.block.patterns', () => { + const config: GitProxyConfig = { + commitConfig: { + diff: { + block: { + patterns: ['password', 'secret'], + }, + }, + }, + }; + expect(validateConfig(config)).toBe(true); + expect(consoleErrorSpy).not.toHaveBeenCalled(); + }); + + it('should handle empty patterns array for message.block.patterns', () => { + const config: GitProxyConfig = { + commitConfig: { + message: { + block: { + patterns: [], + }, + }, + }, + }; + expect(validateConfig(config)).toBe(true); + expect(consoleErrorSpy).not.toHaveBeenCalled(); + }); + + it('should handle empty patterns array for diff.block.patterns', () => { + const config: GitProxyConfig = { + commitConfig: { + diff: { + block: { + patterns: [], + }, + }, + }, + }; + expect(validateConfig(config)).toBe(true); + expect(consoleErrorSpy).not.toHaveBeenCalled(); + }); + + it('should validate complex regex patterns correctly', () => { + const config: GitProxyConfig = { + commitConfig: { + author: { + email: { + local: { + block: '^(?!.*[._]{2})(?!^[._])(?!.*[._]$)[a-z0-9._]+$', + }, + domain: { + allow: '^([a-z0-9]+(-[a-z0-9]+)*\\.)+[a-z]{2,}$', + }, + }, + }, + message: { + block: { + patterns: [ + '\\b(password|secret|api[_-]?key)\\b', + '^(fixup!|squash!)', + '\\[skip[\\s-]ci\\]', + ], + }, + }, + diff: { + block: { + patterns: [ + '-----BEGIN (RSA|DSA|EC|OPENSSH) PRIVATE KEY-----', + '(AWS|aws)_?(SECRET|secret)_?(ACCESS|access)_?(KEY|key)', + ], + }, + }, + }, + }; + expect(validateConfig(config)).toBe(true); + expect(consoleErrorSpy).not.toHaveBeenCalled(); + }); + + it('should handle invalid regex with special characters', () => { + const config: GitProxyConfig = { + commitConfig: { + author: { + email: { + local: { + block: '???', + }, + }, + }, + }, + }; + expect(validateConfig(config)).toBe(false); + expect(consoleErrorSpy).toHaveBeenCalledWith( + 'Invalid regular expression for commitConfig.author.email.local.block: ???', + ); + }); + + it('should stop validation at first error and return false', () => { + const config: GitProxyConfig = { + commitConfig: { + author: { + email: { + local: { + block: '[invalid', + }, + domain: { + allow: '(also-invalid', + }, + }, + }, + message: { + block: { + patterns: ['*invalid-too'], + }, + }, + }, + }; + expect(validateConfig(config)).toBe(false); + // Stops at first error (local.block) + expect(consoleErrorSpy).toHaveBeenCalledTimes(1); + expect(consoleErrorSpy).toHaveBeenCalledWith( + 'Invalid regular expression for commitConfig.author.email.local.block: [invalid', + ); + }); + + it('should handle regex with unicode characters', () => { + const config: GitProxyConfig = { + commitConfig: { + message: { + block: { + patterns: ['[\\u4e00-\\u9fff]+', '\\p{Emoji}'], + }, + }, + }, + }; + expect(validateConfig(config)).toBe(true); + expect(consoleErrorSpy).not.toHaveBeenCalled(); + }); + }); +}); From 6315345a922719743f8b27abf21ea3ca1e95c116 Mon Sep 17 00:00:00 2001 From: Juan Escalada Date: Sun, 18 Jan 2026 14:31:45 +0900 Subject: [PATCH 05/13] test: add ConfigLoader tests for reloadConfiguration --- test/ConfigLoader.test.ts | 105 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 105 insertions(+) diff --git a/test/ConfigLoader.test.ts b/test/ConfigLoader.test.ts index 0121b775f..07138032e 100644 --- a/test/ConfigLoader.test.ts +++ b/test/ConfigLoader.test.ts @@ -192,6 +192,111 @@ describe('ConfigLoader', () => { expect(spy).not.toHaveBeenCalled(); }); + + it('should skip reload and log error when configuration is invalid', async () => { + const invalidConfig = { + proxyUrl: 'https://test.com', + commitConfig: { + author: { + email: { + local: { + block: '[invalid(regex', + }, + }, + }, + }, + }; + + fs.writeFileSync(tempConfigFile, JSON.stringify(invalidConfig)); + + const initialConfig: Configuration = { + configurationSources: { + enabled: true, + sources: [ + { + type: 'file', + enabled: true, + path: tempConfigFile, + }, + ], + reloadIntervalSeconds: 0, + }, + }; + + configLoader = new ConfigLoader(initialConfig); + + const changeEventSpy = vi.fn(); + const consoleErrorSpy = vi.spyOn(console, 'error').mockImplementation(() => {}); + + configLoader.on('configurationChanged', changeEventSpy); + + await configLoader.reloadConfiguration(); + + expect(changeEventSpy).toHaveBeenCalledOnce(); + + expect(consoleErrorSpy).toHaveBeenCalledWith( + 'Invalid regular expression for commitConfig.author.email.local.block: [invalid(regex', + ); + + expect(consoleErrorSpy).toHaveBeenCalledWith('Invalid configuration, skipping reload'); + + consoleErrorSpy.mockRestore(); + }); + + it('should successfully reload when configuration is valid', async () => { + const validConfig = { + proxyUrl: 'https://test.com', + commitConfig: { + author: { + email: { + local: { + block: '^admin.*', // Valid regex pattern + }, + }, + }, + message: { + block: { + patterns: ['WIP:', 'TODO'], + }, + }, + }, + }; + + fs.writeFileSync(tempConfigFile, JSON.stringify(validConfig)); + + const initialConfig: Configuration = { + configurationSources: { + enabled: true, + sources: [ + { + type: 'file', + enabled: true, + path: tempConfigFile, + }, + ], + reloadIntervalSeconds: 0, + }, + }; + + configLoader = new ConfigLoader(initialConfig); + + const changeEventSpy = vi.fn(); + const consoleErrorSpy = vi.spyOn(console, 'error').mockImplementation(() => {}); + + configLoader.on('configurationChanged', changeEventSpy); + + await configLoader.reloadConfiguration(); + + expect(changeEventSpy).toHaveBeenCalledOnce(); + expect(changeEventSpy.mock.calls[0][0]).toMatchObject(validConfig); + + expect(consoleErrorSpy).not.toHaveBeenCalledWith( + expect.stringContaining('Invalid regular expression'), + ); + expect(consoleErrorSpy).not.toHaveBeenCalledWith('Invalid configuration, skipping reload'); + + consoleErrorSpy.mockRestore(); + }); }); describe('initialize', () => { From a1984eeb7e5d1e98c539e8d02f96c705d5f4bebb Mon Sep 17 00:00:00 2001 From: Juan Escalada Date: Sun, 18 Jan 2026 15:16:36 +0900 Subject: [PATCH 06/13] test: add src/config/index tests, modify validation to throw error instead of exiting process --- src/config/index.ts | 4 +- test/testConfig.test.ts | 102 +++++++++++++++++++++++++++++++++++++++- 2 files changed, 104 insertions(+), 2 deletions(-) diff --git a/src/config/index.ts b/src/config/index.ts index c94ffb2e2..096b3420d 100644 --- a/src/config/index.ts +++ b/src/config/index.ts @@ -74,7 +74,9 @@ function loadFullConfiguration(): GitProxyConfig { console.error( 'Invalid configuration: Please check your configuration file and restart GitProxy.', ); - process.exit(1); + throw new Error( + 'Invalid configuration: Please check your configuration file and restart GitProxy.', + ); } return _currentConfig; diff --git a/test/testConfig.test.ts b/test/testConfig.test.ts index 862f7c90d..972f5e8cb 100644 --- a/test/testConfig.test.ts +++ b/test/testConfig.test.ts @@ -1,4 +1,4 @@ -import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest'; +import { describe, it, expect, beforeEach, afterEach, vi, MockInstance } from 'vitest'; import fs from 'fs'; import path from 'path'; import defaultSettings from '../proxy.config.json'; @@ -453,3 +453,103 @@ describe('Configuration Update Handling', () => { vi.resetModules(); }); }); + +describe('loadFullConfiguration', () => { + let tempDir: string; + let tempUserFile: string; + let oldEnv: NodeJS.ProcessEnv; + let consoleErrorSpy: MockInstance; + + beforeEach(async () => { + vi.resetModules(); + oldEnv = { ...process.env }; + tempDir = fs.mkdtempSync('gitproxy-test'); + tempUserFile = path.join(tempDir, 'test-settings.json'); + + consoleErrorSpy = vi.spyOn(console, 'error').mockImplementation(() => {}); + + const fileModule = await import('../src/config/file'); + fileModule.setConfigFile(tempUserFile); + }); + + afterEach(() => { + if (fs.existsSync(tempUserFile)) { + fs.rmSync(tempUserFile); + } + if (fs.existsSync(tempDir)) { + fs.rmdirSync(tempDir); + } + process.env = { ...oldEnv }; + consoleErrorSpy.mockRestore(); + vi.resetModules(); + }); + + describe('validation', () => { + it('should load successfully when user config contains valid regex patterns', async () => { + const validUser = { + commitConfig: { + author: { + email: { + local: { + block: '^admin.*', + }, + domain: { + allow: '.*@example\\.com$', + }, + }, + }, + message: { + block: { + patterns: ['^WIP:', 'TODO', '[Tt]est'], + }, + }, + diff: { + block: { + patterns: ['password', 'secret.*key'], + }, + }, + }, + }; + fs.writeFileSync(tempUserFile, JSON.stringify(validUser)); + + const config = await import('../src/config'); + config.invalidateCache(); + + expect(() => { + config.reloadConfiguration(); // Calls loadFullConfiguration + }).not.toThrow(); + + expect(consoleErrorSpy).not.toHaveBeenCalledWith( + expect.stringContaining('Invalid regular expression'), + ); + expect(consoleErrorSpy).not.toHaveBeenCalledWith( + 'Invalid configuration: Please check your configuration file and restart GitProxy.', + ); + }); + + it('should throw error when config file has invalid commitConfig entry', async () => { + const invalidConfig = { + commitConfig: { + author: { + email: { + local: { + block: '[invalid(regex', + }, + }, + }, + }, + }; + + fs.writeFileSync(tempUserFile, JSON.stringify(invalidConfig)); + + // Needed since loadFullConfiguration is executed on import too + await expect(import('../src/config')).rejects.toThrow( + 'Invalid configuration: Please check your configuration file and restart GitProxy.', + ); + + expect(consoleErrorSpy).toHaveBeenCalledWith( + 'Invalid regular expression for commitConfig.author.email.local.block: [invalid(regex', + ); + }); + }); +}); From 3fd5e381cf2fe94c45d8ff0cfa41df7fdbc74185 Mon Sep 17 00:00:00 2001 From: Juan Escalada Date: Tue, 10 Feb 2026 00:26:12 +0900 Subject: [PATCH 07/13] fix: emit config changed event only when valid --- src/config/ConfigLoader.ts | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/config/ConfigLoader.ts b/src/config/ConfigLoader.ts index cadfaa14a..e52adccd6 100644 --- a/src/config/ConfigLoader.ts +++ b/src/config/ConfigLoader.ts @@ -182,7 +182,12 @@ export class ConfigLoader extends EventEmitter { ) : { ...this.config, ...validConfigs[validConfigs.length - 1] }; // Use last config for override - // Emit change event if config changed + if (!validateConfig(newConfig)) { + console.error('Invalid configuration, skipping reload'); + return; + } + + // Emit change event if config changed and is valid if (JSON.stringify(newConfig) !== JSON.stringify(this.config)) { console.log('Configuration has changed, updating and emitting change event'); this.config = newConfig; @@ -190,11 +195,6 @@ export class ConfigLoader extends EventEmitter { } else { console.log('Configuration has not changed, no update needed'); } - - if (!validateConfig(newConfig)) { - console.error('Invalid configuration, skipping reload'); - return; - } } catch (error: any) { console.error('Error reloading configuration:', error); this.emit('configurationError', error); From 0bb6a29f5ba25dda000ab19b09e09af671b5b1c3 Mon Sep 17 00:00:00 2001 From: Juan Escalada Date: Tue, 10 Feb 2026 00:26:47 +0900 Subject: [PATCH 08/13] fix: incorrect test logic for skipping reload on invalid config --- test/ConfigLoader.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/ConfigLoader.test.ts b/test/ConfigLoader.test.ts index 07138032e..126fca7e5 100644 --- a/test/ConfigLoader.test.ts +++ b/test/ConfigLoader.test.ts @@ -232,7 +232,7 @@ describe('ConfigLoader', () => { await configLoader.reloadConfiguration(); - expect(changeEventSpy).toHaveBeenCalledOnce(); + expect(changeEventSpy).not.toHaveBeenCalled(); expect(consoleErrorSpy).toHaveBeenCalledWith( 'Invalid regular expression for commitConfig.author.email.local.block: [invalid(regex', From a27ce943639c8426f305f9e40b940f7df24a3b23 Mon Sep 17 00:00:00 2001 From: Juan Escalada Date: Wed, 11 Feb 2026 16:17:12 +0900 Subject: [PATCH 09/13] feat: add commitConfig.diff.block.providers regex check and tests --- src/config/validators.ts | 11 +++++++++ test/configValidators.test.ts | 44 +++++++++++++++++++++++++++++++++++ 2 files changed, 55 insertions(+) diff --git a/src/config/validators.ts b/src/config/validators.ts index 77bb644cc..e6af2f25c 100644 --- a/src/config/validators.ts +++ b/src/config/validators.ts @@ -65,5 +65,16 @@ function validateCommitConfig(config: GitProxyConfig): boolean { } } + if (config.commitConfig?.diff?.block?.providers) { + for (const [key, value] of Object.entries(config.commitConfig.diff.block.providers)) { + try { + new RegExp(value); + } catch (error: unknown) { + console.error(`Invalid regular expression for commitConfig.diff.block.providers: ${value}`); + return false; + } + } + } + return true; } diff --git a/test/configValidators.test.ts b/test/configValidators.test.ts index c2bb2cd1a..326196882 100644 --- a/test/configValidators.test.ts +++ b/test/configValidators.test.ts @@ -231,6 +231,20 @@ describe('validators', () => { expect(consoleErrorSpy).not.toHaveBeenCalled(); }); + it('should handle empty providers object for diff.block.providers', () => { + const config: GitProxyConfig = { + commitConfig: { + diff: { + block: { + providers: {}, + }, + }, + }, + }; + expect(validateConfig(config)).toBe(true); + expect(consoleErrorSpy).not.toHaveBeenCalled(); + }); + it('should handle empty patterns array for message.block.patterns', () => { const config: GitProxyConfig = { commitConfig: { @@ -354,5 +368,35 @@ describe('validators', () => { expect(validateConfig(config)).toBe(true); expect(consoleErrorSpy).not.toHaveBeenCalled(); }); + + it('should handle invalid regex with special characters in providers', () => { + const config: GitProxyConfig = { + commitConfig: { + diff: { + block: { + providers: { type: '???' }, + }, + }, + }, + }; + expect(validateConfig(config)).toBe(false); + expect(consoleErrorSpy).toHaveBeenCalledWith( + 'Invalid regular expression for commitConfig.diff.block.providers: ???', + ); + }); + + it('should work with valid regex in providers', () => { + const config: GitProxyConfig = { + commitConfig: { + diff: { + block: { + providers: { type: 'valid' }, + }, + }, + }, + }; + expect(validateConfig(config)).toBe(true); + expect(consoleErrorSpy).not.toHaveBeenCalled(); + }); }); }); From d7ea0841087f05947eabc3a3a814c7c31046bd2b Mon Sep 17 00:00:00 2001 From: Juan Escalada Date: Wed, 11 Feb 2026 16:31:22 +0900 Subject: [PATCH 10/13] fix: double loadFullConfiguration execution and unnecessary casting --- src/config/index.ts | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/config/index.ts b/src/config/index.ts index 096b3420d..133750dbb 100644 --- a/src/config/index.ts +++ b/src/config/index.ts @@ -335,7 +335,7 @@ const handleConfigUpdate = async (newConfig: Configuration) => { // Initialize config loader function initializeConfigLoader() { - const config = loadFullConfiguration() as Configuration; + const config = loadFullConfiguration(); _configLoader = new ConfigLoader(config); // Handle configuration updates @@ -362,7 +362,6 @@ export const reloadConfiguration = async () => { // Initialize configuration on module load try { - loadFullConfiguration(); initializeConfigLoader(); console.log('Configuration loaded successfully'); } catch (error) { From 60be1d43917cffa6b94894311be98b02c90d7405 Mon Sep 17 00:00:00 2001 From: Juan Escalada Date: Sat, 14 Feb 2026 23:04:43 +0900 Subject: [PATCH 11/13] feat: extract config loading and parsing logic into validators.ts helpers --- src/config/ConfigLoader.ts | 57 +++++++++++--------------------------- src/config/validators.ts | 20 ++++++++++++- 2 files changed, 35 insertions(+), 42 deletions(-) diff --git a/src/config/ConfigLoader.ts b/src/config/ConfigLoader.ts index e52adccd6..9c4d70625 100644 --- a/src/config/ConfigLoader.ts +++ b/src/config/ConfigLoader.ts @@ -5,9 +5,9 @@ import { execFile } from 'child_process'; import { promisify } from 'util'; import { EventEmitter } from 'events'; import envPaths from 'env-paths'; -import { GitProxyConfig, Convert } from './generated/config'; +import { GitProxyConfig } from './generated/config'; import { Configuration, ConfigurationSource, FileSource, HttpSource, GitSource } from './types'; -import { validateConfig } from './validators'; +import { loadConfig, validateConfig } from './validators'; const execFileAsync = promisify(execFile); @@ -149,7 +149,7 @@ export class ConfigLoader extends EventEmitter { ); console.log(`Found ${enabledSources.length} enabled configuration sources`); - const configs = await Promise.all( + const loadedConfigs = await Promise.all( enabledSources.map(async (source: ConfigurationSource) => { try { console.log(`Loading configuration from ${source.type} source`); @@ -162,10 +162,12 @@ export class ConfigLoader extends EventEmitter { ); // Filter out null results from failed loads - const validConfigs = configs.filter((config): config is GitProxyConfig => config !== null); + const nonNullConfigs = loadedConfigs.filter( + (config): config is GitProxyConfig => config !== null, + ); - if (validConfigs.length === 0) { - console.log('No valid configurations loaded from any source'); + if (nonNullConfigs.length === 0) { + console.log('All loaded configurations are empty, skipping reload'); return; } @@ -174,13 +176,13 @@ export class ConfigLoader extends EventEmitter { console.log(`Using ${shouldMerge ? 'merge' : 'override'} strategy for configuration`); const newConfig = shouldMerge - ? validConfigs.reduce( + ? nonNullConfigs.reduce( (acc, curr) => { return this.deepMerge(acc, curr) as Configuration; }, { ...this.config }, ) - : { ...this.config, ...validConfigs[validConfigs.length - 1] }; // Use last config for override + : { ...this.config, ...nonNullConfigs[nonNullConfigs.length - 1] }; // Use last config for override if (!validateConfig(newConfig)) { console.error('Invalid configuration, skipping reload'); @@ -224,16 +226,7 @@ export class ConfigLoader extends EventEmitter { throw new Error('Invalid configuration file path'); } console.log(`Loading configuration from file: ${configPath}`); - const content = await fs.promises.readFile(configPath, 'utf8'); - - // Use QuickType to validate and parse the configuration - try { - return Convert.toGitProxyConfig(content); - } catch (error) { - throw new Error( - `Invalid configuration file format: ${error instanceof Error ? error.message : 'Unknown error'}`, - ); - } + return loadConfig(`file: ${configPath}`, async () => fs.promises.readFile(configPath, 'utf8')); } async loadFromHttp(source: HttpSource): Promise { @@ -243,18 +236,10 @@ export class ConfigLoader extends EventEmitter { ...(source.auth?.type === 'bearer' ? { Authorization: `Bearer ${source.auth.token}` } : {}), }; - const response = await axios.get(source.url, { headers }); - - // Use QuickType to validate and parse the configuration from HTTP response - try { - const configJson = - typeof response.data === 'string' ? response.data : JSON.stringify(response.data); - return Convert.toGitProxyConfig(configJson); - } catch (error) { - throw new Error( - `Invalid configuration format from HTTP source: ${error instanceof Error ? error.message : 'Unknown error'}`, - ); - } + return loadConfig(`HTTP: ${source.url}`, async () => { + const response = await axios.get(source.url, { headers }); + return typeof response.data === 'string' ? response.data : JSON.stringify(response.data); + }); } async loadFromGit(source: GitSource): Promise { @@ -350,17 +335,7 @@ export class ConfigLoader extends EventEmitter { throw new Error(`Configuration file not found at ${configPath}`); } - try { - const content = await fs.promises.readFile(configPath, 'utf8'); - - // Use QuickType to validate and parse the configuration from Git - const config = Convert.toGitProxyConfig(content); - console.log('Configuration loaded successfully from Git'); - return config; - } catch (error: any) { - console.error('Failed to read or parse configuration file:', error.message); - throw new Error(`Failed to read or parse configuration file: ${error.message}`); - } + return loadConfig(`git: ${configPath}`, async () => fs.promises.readFile(configPath, 'utf8')); } deepMerge(target: Record, source: Record): Record { diff --git a/src/config/validators.ts b/src/config/validators.ts index e6af2f25c..5866ae9a9 100644 --- a/src/config/validators.ts +++ b/src/config/validators.ts @@ -1,4 +1,4 @@ -import { GitProxyConfig } from './generated/config'; +import { Convert, GitProxyConfig } from './generated/config'; const validationChain = [validateCommitConfig]; @@ -78,3 +78,21 @@ function validateCommitConfig(config: GitProxyConfig): boolean { return true; } + +export async function loadConfig( + context: string, + loader: () => Promise, +): Promise { + const raw = await loader(); + return parseGitProxyConfig(raw, context); +} + +function parseGitProxyConfig(raw: string, context: string): GitProxyConfig { + try { + return Convert.toGitProxyConfig(raw); + } catch (error) { + throw new Error( + `Invalid configuration format in ${context}: ${error instanceof Error ? error.message : 'Unknown error'}`, + ); + } +} From 0feedbd09a2f5a46fc6d6d5da146dd4f1386fbff Mon Sep 17 00:00:00 2001 From: Juan Escalada Date: Sat, 14 Feb 2026 23:05:09 +0900 Subject: [PATCH 12/13] test: update ConfigLoader tests to match new error messages --- test/ConfigLoader.test.ts | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/test/ConfigLoader.test.ts b/test/ConfigLoader.test.ts index 2f6e61df6..6fea3b27c 100644 --- a/test/ConfigLoader.test.ts +++ b/test/ConfigLoader.test.ts @@ -614,7 +614,7 @@ describe('ConfigLoader', () => { }; await expect(configLoader.loadFromSource(source)).rejects.toThrow( - /Failed to read or parse configuration file/, + /Invalid configuration format in git/, ); }, { timeout: 30000 }, @@ -812,7 +812,7 @@ describe('ConfigLoader Error Handling', () => { enabled: true, path: tempConfigFile, }), - ).rejects.toThrow(/Invalid configuration file format/); + ).rejects.toThrow(/Invalid configuration format in file/); }); it('should handle HTTP request errors', async () => { @@ -838,6 +838,7 @@ describe('ConfigLoader Error Handling', () => { enabled: true, url: 'http://config-service/config', }), - ).rejects.toThrow(/Invalid configuration format from HTTP source/); + // Check that the error message CONTAINS the following string: + ).rejects.toThrow(/Invalid configuration format in HTTP: http:\/\/config-service\/config/); }); }); From 3d977e7903f42e5f12fb03bb6102064180017f3b Mon Sep 17 00:00:00 2001 From: Juan Escalada Date: Sat, 14 Feb 2026 23:39:24 +0900 Subject: [PATCH 13/13] refactor: config regex validation logic into reusable functions --- src/config/validators.ts | 109 ++++++++++++++++++++++----------------- 1 file changed, 61 insertions(+), 48 deletions(-) diff --git a/src/config/validators.ts b/src/config/validators.ts index 5866ae9a9..17da0617b 100644 --- a/src/config/validators.ts +++ b/src/config/validators.ts @@ -17,68 +17,75 @@ export const validateConfig = (config: GitProxyConfig): boolean => { * @returns true if the commit configuration is valid, false otherwise */ function validateCommitConfig(config: GitProxyConfig): boolean { - if (config.commitConfig?.author?.email?.local?.block) { - try { - new RegExp(config.commitConfig.author.email.local.block); - } catch (error: unknown) { - console.error( - `Invalid regular expression for commitConfig.author.email.local.block: ${config.commitConfig.author.email.local.block}`, - ); - return false; - } - } + return ( + validateConfigRegex(config, 'commitConfig.author.email.local.block') && + validateConfigRegex(config, 'commitConfig.author.email.domain.allow') && + validateConfigRegex(config, 'commitConfig.message.block.patterns') && + validateConfigRegex(config, 'commitConfig.diff.block.patterns') && + validateConfigRegex(config, 'commitConfig.diff.block.providers') + ); +} - if (config.commitConfig?.author?.email?.domain?.allow) { - try { - new RegExp(config.commitConfig.author.email.domain.allow); - } catch (error: unknown) { - console.error( - `Invalid regular expression for commitConfig.author.email.domain.allow: ${config.commitConfig.author.email.domain.allow}`, - ); - return false; - } +/** + * Validates that a regular expression is valid. + * @param pattern The regular expression to validate + * @param context The context of the regular expression + * @returns true if the regular expression is valid, false otherwise + */ +function isValidRegex(pattern: string, context: string): boolean { + try { + new RegExp(pattern); + return true; + } catch { + console.error(`Invalid regular expression for ${context}: ${pattern}`); + return false; } +} - if (config.commitConfig?.message?.block?.patterns) { - for (const pattern of config.commitConfig.message.block.patterns) { - try { - new RegExp(pattern); - } catch (error: unknown) { - console.error( - `Invalid regular expression for commitConfig.message.block.patterns: ${pattern}`, - ); - return false; +/** + * Validates that a value in the configuration is a valid regular expression. + * @param config The configuration to validate + * @param path The path to the value to validate + * @returns true if the value is a valid regular expression, false otherwise + */ +function validateConfigRegex(config: GitProxyConfig, path: string): boolean { + const getValueAtPath = (obj: unknown, path: string): unknown => { + return path.split('.').reduce((current, key) => { + if (current == null || typeof current !== 'object') { + return undefined; } - } + return (current as Record)[key]; + }, obj); + }; + + const value = getValueAtPath(config, path); + + if (!value) return true; + + if (typeof value === 'string') { + return isValidRegex(value, path); } - if (config.commitConfig?.diff?.block?.patterns) { - for (const pattern of config.commitConfig.diff.block.patterns) { - try { - new RegExp(pattern); - } catch (error: unknown) { - console.error( - `Invalid regular expression for commitConfig.diff.block.patterns: ${pattern}`, - ); - return false; - } + if (Array.isArray(value)) { + for (const pattern of value) { + if (!isValidRegex(pattern, path)) return false; } + return true; } - if (config.commitConfig?.diff?.block?.providers) { - for (const [key, value] of Object.entries(config.commitConfig.diff.block.providers)) { - try { - new RegExp(value); - } catch (error: unknown) { - console.error(`Invalid regular expression for commitConfig.diff.block.providers: ${value}`); - return false; - } - } + if (typeof value === 'object') { + return Object.values(value).every((pattern) => isValidRegex(pattern as string, path)); } return true; } +/** + * Loads and parses a GitProxyConfig object from a given context and loading strategy. + * @param context The context of the configuration + * @param loader The loading strategy to use + * @returns The parsed GitProxyConfig object + */ export async function loadConfig( context: string, loader: () => Promise, @@ -87,6 +94,12 @@ export async function loadConfig( return parseGitProxyConfig(raw, context); } +/** + * Parses a raw string into a GitProxyConfig object. + * @param raw The raw string to parse + * @param context The context of the configuration + * @returns The parsed GitProxyConfig object + */ function parseGitProxyConfig(raw: string, context: string): GitProxyConfig { try { return Convert.toGitProxyConfig(raw);