Merge pull request finos#1355 from jescalada/1336-fix-invalid-regex-commitConfig

fix: add invalid regex validation for `commitConfig`

Author: jescalada

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');

src/config/ConfigLoader.ts

@@ -5,8 +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 { loadConfig, validateConfig } from './validators';
 
 const execFileAsync = promisify(execFile);
 
@@ -148,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`);
@@ -161,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;
       }
 
@@ -173,15 +176,20 @@ 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');
+        return;
+      }
 
-      // Emit change event if config changed
+      // 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;
@@ -218,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<GitProxyConfig> {
@@ -237,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<GitProxyConfig> {
@@ -344,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<string, any>, source: Record<string, any>): Record<string, any> {

src/config/index.ts

@@ -6,6 +6,7 @@ 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;
@@ -69,6 +70,15 @@ function loadFullConfiguration(): GitProxyConfig {
 
   _currentConfig = mergeConfigurations(defaultConfig, userSettings);
 
+  if (!validateConfig(_currentConfig)) {
+    console.error(
+      'Invalid configuration: Please check your configuration file and restart GitProxy.',
+    );
+    throw new Error(
+      'Invalid configuration: Please check your configuration file and restart GitProxy.',
+    );
+  }
+
   return _currentConfig;
 }
 
@@ -325,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
@@ -352,7 +362,6 @@ export const reloadConfiguration = async () => {
 
 // Initialize configuration on module load
 try {
-  loadFullConfiguration();
   initializeConfigLoader();
   console.log('Configuration loaded successfully');
 } catch (error) {

src/config/validators.ts

@@ -0,0 +1,111 @@
+import { Convert, 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 {
+  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')
+  );
+}
+
+/**
+ * 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;
+  }
+}
+
+/**
+ * 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<string, unknown>)[key];
+    }, obj);
+  };
+
+  const value = getValueAtPath(config, path);
+
+  if (!value) return true;
+
+  if (typeof value === 'string') {
+    return isValidRegex(value, path);
+  }
+
+  if (Array.isArray(value)) {
+    for (const pattern of value) {
+      if (!isValidRegex(pattern, path)) return false;
+    }
+    return true;
+  }
+
+  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<string>,
+): Promise<GitProxyConfig> {
+  const raw = await loader();
+  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);
+  } catch (error) {
+    throw new Error(
+      `Invalid configuration format in ${context}: ${error instanceof Error ? error.message : 'Unknown error'}`,
+    );
+  }
+}