Merge pull request #1514 from dcoric/feat/require-config-defaults

feat(config): require merged top-level defaults

Author: jescalada

src/config/index.ts

@@ -25,8 +25,56 @@ import { getConfigFile } from './file';
 import { validateConfig } from './validators';
 import { handleErrorAndLog, handleErrorAndThrow } from '../utils/errors';
 
+// Deprecated compatibility fields are still optional because the defaults do not set them.
+type OptionalTopLevelConfigKey = 'proxyUrl' | 'sslCertPemPath' | 'sslKeyPemPath';
+type RequiredTopLevelConfigKey = Exclude<keyof GitProxyConfig, OptionalTopLevelConfigKey>;
+
+export type FullGitProxyConfig = Required<Omit<GitProxyConfig, OptionalTopLevelConfigKey>> &
+  Pick<GitProxyConfig, OptionalTopLevelConfigKey>;
+
+const REQUIRED_TOP_LEVEL_CONFIG_KEYS = [
+  'api',
+  'apiAuthentication',
+  'attestationConfig',
+  'authentication',
+  'authorisedList',
+  'commitConfig',
+  'configurationSources',
+  'contactEmail',
+  'cookieSecret',
+  'csrfProtection',
+  'domains',
+  'plugins',
+  'privateOrganizations',
+  'rateLimit',
+  'sessionMaxAgeHours',
+  'sink',
+  'tempPassword',
+  'tls',
+  'uiRouteAuth',
+  'upstreamProxy',
+  'urlShortener',
+] as const satisfies readonly RequiredTopLevelConfigKey[];
+
+type MissingRequiredTopLevelConfigKeys = Exclude<
+  RequiredTopLevelConfigKey,
+  (typeof REQUIRED_TOP_LEVEL_CONFIG_KEYS)[number]
+>;
+type AssertNever<T extends never> = T;
+type _RequiredTopLevelConfigKeysAreExhaustive = AssertNever<MissingRequiredTopLevelConfigKeys>;
+
+export function assertHasRequiredTopLevelConfig(
+  config: GitProxyConfig,
+): asserts config is FullGitProxyConfig {
+  const missingKeys = REQUIRED_TOP_LEVEL_CONFIG_KEYS.filter((key) => config[key] === undefined);
+
+  if (missingKeys.length > 0) {
+    throw new Error(`Missing required top-level configuration values: ${missingKeys.join(', ')}`);
+  }
+}
+
 // Cache for current configuration
-let _currentConfig: GitProxyConfig | null = null;
+let _currentConfig: FullGitProxyConfig | null = null;
 let _configLoader: ConfigLoader | null = null;
 
 // Function to invalidate cache - useful for testing
@@ -57,17 +105,17 @@ function cleanUndefinedValues(obj: any): any {
 
 /**
  * Load and merge default + user configuration with QuickType validation
- * @return {GitProxyConfig} The merged and validated configuration
+ * @return {FullGitProxyConfig} The merged and validated configuration
  */
-function loadFullConfiguration(): GitProxyConfig {
+function loadFullConfiguration(): FullGitProxyConfig {
   if (_currentConfig) {
     return _currentConfig;
   }
 
   const rawDefaultConfig = Convert.toGitProxyConfig(JSON.stringify(defaultSettings));
 
   // Clean undefined values from defaultConfig
-  const defaultConfig = cleanUndefinedValues(rawDefaultConfig);
+  const defaultConfig = cleanUndefinedValues(rawDefaultConfig) as GitProxyConfig;
 
   let userSettings: Partial<GitProxyConfig> = {};
   const userConfigFile = process.env.CONFIG_FILE || getConfigFile();
@@ -102,12 +150,12 @@ function loadFullConfiguration(): GitProxyConfig {
  * Merge configurations with environment variable overrides
  * @param {GitProxyConfig} defaultConfig - The default configuration
  * @param {Partial<GitProxyConfig>} userSettings - User-provided configuration overrides
- * @return {GitProxyConfig} The merged configuration
+ * @return {FullGitProxyConfig} The merged configuration
  */
 function mergeConfigurations(
   defaultConfig: GitProxyConfig,
   userSettings: Partial<GitProxyConfig>,
-): GitProxyConfig {
+): FullGitProxyConfig {
   // Special handling for TLS configuration when legacy fields are used
   let tlsConfig = userSettings.tls || defaultConfig.tls;
 
@@ -121,7 +169,7 @@ function mergeConfigurations(
     };
   }
 
-  return {
+  const config = {
     ...defaultConfig,
     ...userSettings,
     // Deep merge for specific objects
@@ -141,6 +189,9 @@ function mergeConfigurations(
       userSettings.cookieSecret ||
       defaultConfig.cookieSecret,
   };
+
+  assertHasRequiredTopLevelConfig(config);
+  return config;
 }
 
 // Get configured proxy URL
@@ -169,7 +220,7 @@ export const getUpstreamProxyConfig = () => {
 // Gets a list of authorised repositories
 export const getAuthorisedList = () => {
   const config = loadFullConfiguration();
-  return config.authorisedList || [];
+  return config.authorisedList;
 };
 
 // Gets a list of authorised repositories
@@ -181,7 +232,7 @@ export const getTempPasswordConfig = () => {
 // Gets the configured data sink, defaults to filesystem
 export const getDatabase = () => {
   const config = loadFullConfiguration();
-  const databases = config.sink || [];
+  const databases = config.sink;
 
   for (const db of databases) {
     if (db.enabled) {
@@ -204,7 +255,7 @@ export const getDatabase = () => {
  */
 export const getAuthMethods = () => {
   const config = loadFullConfiguration();
-  const authSources = config.authentication || [];
+  const authSources = config.authentication;
 
   const enabledAuthMethods = authSources.filter((auth) => auth.enabled);
 
@@ -223,7 +274,7 @@ export const getAuthMethods = () => {
  */
 export const getAPIAuthMethods = () => {
   const config = loadFullConfiguration();
-  const apiAuthSources = config.apiAuthentication || [];
+  const apiAuthSources = config.apiAuthentication;
 
   return apiAuthSources.filter((auth: { enabled: any }) => auth.enabled);
 };
@@ -244,7 +295,7 @@ export const logConfiguration = () => {
 
 export const getAPIs = () => {
   const config = loadFullConfiguration();
-  return config.api || {};
+  return config.api;
 };
 
 export const getCookieSecret = (): string => {
@@ -259,25 +310,25 @@ export const getCookieSecret = (): string => {
 
 export const getSessionMaxAgeHours = (): number => {
   const config = loadFullConfiguration();
-  return config.sessionMaxAgeHours || 24;
+  return config.sessionMaxAgeHours;
 };
 
 // Get commit related configuration
 export const getCommitConfig = () => {
   const config = loadFullConfiguration();
-  return config.commitConfig || {};
+  return config.commitConfig;
 };
 
 // Get attestation related configuration
 export const getAttestationConfig = () => {
   const config = loadFullConfiguration();
-  return config.attestationConfig || {};
+  return config.attestationConfig;
 };
 
 // Get private organizations related configuration
 export const getPrivateOrganizations = () => {
   const config = loadFullConfiguration();
-  return config.privateOrganizations || [];
+  return config.privateOrganizations;
 };
 
 // Get URL shortener
@@ -301,7 +352,7 @@ export const getCSRFProtection = (): boolean | undefined => {
 // Get loadable push plugins
 export const getPlugins = () => {
   const config = loadFullConfiguration();
-  return config.plugins || [];
+  return config.plugins;
 };
 
 export const getTLSKeyPemPath = (): string | undefined => {
@@ -321,12 +372,12 @@ export const getTLSEnabled = (): boolean => {
 
 export const getDomains = () => {
   const config = loadFullConfiguration();
-  return config.domains || {};
+  return config.domains;
 };
 
 export const getUIRouteAuth = () => {
   const config = loadFullConfiguration();
-  return config.uiRouteAuth || {};
+  return config.uiRouteAuth;
 };
 
 export const getRateLimit = () => {
@@ -342,12 +393,13 @@ const handleConfigUpdate = async (newConfig: Configuration) => {
     const validatedConfig = Convert.toGitProxyConfig(JSON.stringify(newConfig));
 
     // 2. Get proxy module dynamically to avoid circular dependency
-    const proxy = require('../proxy');
+    const proxy = (await import('../proxy')) as any;
 
     // 3. Stop existing services
     await proxy.stop();
 
     // 4. Update config
+    assertHasRequiredTopLevelConfig(validatedConfig);
     _currentConfig = validatedConfig;
 
     // 5. Restart services with new config
@@ -358,7 +410,7 @@ const handleConfigUpdate = async (newConfig: Configuration) => {
     handleErrorAndLog(error, 'Failed to apply new configuration');
     // Attempt to restart with previous config
     try {
-      const proxy = require('../proxy');
+      const proxy = (await import('../proxy')) as any;
       await proxy.start();
     } catch (startError: unknown) {
       handleErrorAndLog(startError, 'Failed to restart services');

src/service/index.ts

@@ -35,8 +35,6 @@ const limiter = rateLimit(config.getRateLimit());
 
 const { GIT_PROXY_UI_PORT: uiPort, GIT_PROXY_HTTPS_UI_PORT: uiHttpsPort } = serverConfig;
 
-const DEFAULT_SESSION_MAX_AGE_HOURS = 12;
-
 const app: Express = express();
 let _httpServer: http.Server | null = null;
 let _httpsServer: https.Server | null = null;
@@ -157,7 +155,7 @@ async function createApp(proxy: Proxy): Promise<Express> {
       cookie: {
         secure: 'auto',
         httpOnly: true,
-        maxAge: (config.getSessionMaxAgeHours() || DEFAULT_SESSION_MAX_AGE_HOURS) * 60 * 60 * 1000,
+        maxAge: config.getSessionMaxAgeHours() * 60 * 60 * 1000,
       },
     }),
   );

test/proxy.test.ts

@@ -20,7 +20,7 @@ import { describe, it, beforeEach, afterEach, expect, vi, Mock } from 'vitest';
 import fs from 'fs';
 import { GitProxyConfig } from '../src/config/generated/config';
 import { Proxy } from '../src/proxy';
-import { handleAndLogError } from '../src/utils/errors';
+import { handleErrorAndLog } from '../src/utils/errors';
 
 describe('Proxy Module TLS Certificate Loading', () => {
   let proxyModule: Proxy;
@@ -123,7 +123,7 @@ describe('Proxy Module TLS Certificate Loading', () => {
     try {
       await proxyModule.stop();
     } catch (error: unknown) {
-      handleAndLogError(error, 'Error occurred when stopping the proxy');
+      handleErrorAndLog(error, 'Error occurred when stopping the proxy');
     }
     vi.restoreAllMocks();
   });