diff --git a/src/config/index.ts b/src/config/index.ts index 0d4591300..75960c6c0 100644 --- a/src/config/index.ts +++ b/src/config/index.ts @@ -25,8 +25,55 @@ 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; + +export type FullGitProxyConfig = Required> & + Pick; + +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', + 'urlShortener', +] as const satisfies readonly RequiredTopLevelConfigKey[]; + +type MissingRequiredTopLevelConfigKeys = Exclude< + RequiredTopLevelConfigKey, + (typeof REQUIRED_TOP_LEVEL_CONFIG_KEYS)[number] +>; +type AssertNever = T; +type _RequiredTopLevelConfigKeysAreExhaustive = AssertNever; + +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,9 +104,9 @@ 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; } @@ -67,7 +114,7 @@ function loadFullConfiguration(): GitProxyConfig { 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 = {}; const userConfigFile = process.env.CONFIG_FILE || getConfigFile(); @@ -102,12 +149,12 @@ function loadFullConfiguration(): GitProxyConfig { * Merge configurations with environment variable overrides * @param {GitProxyConfig} defaultConfig - The default configuration * @param {Partial} userSettings - User-provided configuration overrides - * @return {GitProxyConfig} The merged configuration + * @return {FullGitProxyConfig} The merged configuration */ function mergeConfigurations( defaultConfig: GitProxyConfig, userSettings: Partial, -): GitProxyConfig { +): FullGitProxyConfig { // Special handling for TLS configuration when legacy fields are used let tlsConfig = userSettings.tls || defaultConfig.tls; @@ -121,7 +168,7 @@ function mergeConfigurations( }; } - return { + const config = { ...defaultConfig, ...userSettings, // Deep merge for specific objects @@ -141,6 +188,9 @@ function mergeConfigurations( userSettings.cookieSecret || defaultConfig.cookieSecret, }; + + assertHasRequiredTopLevelConfig(config); + return config; } // Get configured proxy URL @@ -152,7 +202,7 @@ export const getProxyUrl = (): string | undefined => { // Gets a list of authorised repositories export const getAuthorisedList = () => { const config = loadFullConfiguration(); - return config.authorisedList || []; + return config.authorisedList; }; // Gets a list of authorised repositories @@ -164,7 +214,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) { @@ -187,7 +237,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); @@ -206,7 +256,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); }; @@ -227,7 +277,7 @@ export const logConfiguration = () => { export const getAPIs = () => { const config = loadFullConfiguration(); - return config.api || {}; + return config.api; }; export const getCookieSecret = (): string => { @@ -242,25 +292,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 @@ -284,7 +334,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 => { @@ -304,12 +354,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 = () => { @@ -325,12 +375,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 @@ -341,7 +392,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'); diff --git a/src/service/index.ts b/src/service/index.ts index b8ee756b8..d2762be67 100644 --- a/src/service/index.ts +++ b/src/service/index.ts @@ -33,8 +33,6 @@ const limiter = rateLimit(config.getRateLimit()); const { GIT_PROXY_UI_PORT: uiPort } = serverConfig; -const DEFAULT_SESSION_MAX_AGE_HOURS = 12; - const app: Express = express(); let _httpServer: http.Server | null = null; @@ -143,7 +141,7 @@ async function createApp(proxy: Proxy): Promise { cookie: { secure: 'auto', httpOnly: true, - maxAge: (config.getSessionMaxAgeHours() || DEFAULT_SESSION_MAX_AGE_HOURS) * 60 * 60 * 1000, + maxAge: config.getSessionMaxAgeHours() * 60 * 60 * 1000, }, }), ); diff --git a/test/testConfig.test.ts b/test/testConfig.test.ts index a886183fd..688e9d322 100644 --- a/test/testConfig.test.ts +++ b/test/testConfig.test.ts @@ -36,6 +36,7 @@ describe('default configuration', () => { expect(config.getAuthMethods()).toEqual(enabledMethods); expect(config.getDatabase()).toEqual(defaultSettings.sink[0]); expect(config.getTempPasswordConfig()).toEqual(defaultSettings.tempPassword); + expect(config.getProxyUrl()).toBeUndefined(); expect(config.getAuthorisedList()).toEqual(defaultSettings.authorisedList); expect(config.getRateLimit()).toEqual(defaultSettings.rateLimit); expect(config.getTLSKeyPemPath()).toEqual(defaultSettings.tls.key); @@ -46,8 +47,15 @@ describe('default configuration', () => { expect(config.getContactEmail()).toEqual(defaultSettings.contactEmail); expect(config.getPlugins()).toEqual(defaultSettings.plugins); expect(config.getCSRFProtection()).toEqual(defaultSettings.csrfProtection); + expect(config.getSessionMaxAgeHours()).toEqual(defaultSettings.sessionMaxAgeHours); + expect(config.getCommitConfig()).toEqual(defaultSettings.commitConfig); expect(config.getAttestationConfig()).toEqual(defaultSettings.attestationConfig); expect(config.getAPIs()).toEqual(defaultSettings.api); + expect(config.getAPIAuthMethods()).toEqual( + defaultSettings.apiAuthentication.filter((method) => method.enabled), + ); + expect(config.getPrivateOrganizations()).toEqual(defaultSettings.privateOrganizations); + expect(config.getUIRouteAuth()).toEqual(defaultSettings.uiRouteAuth); }); }); @@ -261,6 +269,22 @@ describe('user configuration', () => { expect(config.getAPIs()).toEqual(user.api); }); + it('should keep default top-level config values when user config only overrides one entry', async () => { + const user = { + sessionMaxAgeHours: 6, + }; + fs.writeFileSync(tempUserFile, JSON.stringify(user)); + + const config = await import('../src/config'); + config.invalidateCache(); + + expect(config.getSessionMaxAgeHours()).toBe(user.sessionMaxAgeHours); + expect(config.getCommitConfig()).toEqual(defaultSettings.commitConfig); + expect(config.getAttestationConfig()).toEqual(defaultSettings.attestationConfig); + expect(config.getPrivateOrganizations()).toEqual(defaultSettings.privateOrganizations); + expect(config.getUIRouteAuth()).toEqual(defaultSettings.uiRouteAuth); + }); + it('should override default settings for cookieSecret if env var is used', async () => { fs.writeFileSync(tempUserFile, '{}'); process.env.GIT_PROXY_COOKIE_SECRET = 'test-cookie-secret'; @@ -402,24 +426,29 @@ describe('Configuration Update Handling', () => { let tempUserFile: string; let oldEnv: NodeJS.ProcessEnv; + const waitForMockCall = async (mock: MockInstance, callCount = 1) => { + for (let i = 0; i < 20; i += 1) { + if (mock.mock.calls.length >= callCount) { + return; + } + await new Promise((resolve) => setTimeout(resolve, 10)); + } + }; + beforeEach(() => { oldEnv = { ...process.env }; tempDir = fs.mkdtempSync('gitproxy-test'); tempUserFile = path.join(tempDir, 'test-settings.json'); + process.env.CONFIG_FILE = tempUserFile; configFile.setConfigFile(tempUserFile); }); it('should test ConfigLoader initialization', async () => { const configWithSources = { + ...defaultSettings, configurationSources: { enabled: true, - sources: [ - { - type: 'file', - enabled: true, - path: tempUserFile, - }, - ], + sources: [], }, }; @@ -457,15 +486,98 @@ describe('Configuration Update Handling', () => { consoleErrorSpy.mockRestore(); }); - afterEach(() => { - if (fs.existsSync(tempUserFile)) { - fs.rmSync(tempUserFile, { force: true }); + it('should apply valid configuration updates from external sources', async () => { + const updatedConfigFile = path.join(tempDir, 'updated-settings.json'); + const proxyStop = vi.fn().mockResolvedValue(undefined); + const proxyStart = vi.fn().mockResolvedValue(undefined); + vi.doMock('../src/proxy', () => ({ + stop: proxyStop, + start: proxyStart, + })); + + const configWithSources = { + configurationSources: { + enabled: true, + sources: [ + { + type: 'file', + enabled: true, + path: updatedConfigFile, + }, + ], + }, + }; + const updatedConfig = { + ...defaultSettings, + configurationSources: configWithSources.configurationSources, + sessionMaxAgeHours: 8, + }; + + fs.writeFileSync(tempUserFile, JSON.stringify(configWithSources)); + fs.writeFileSync(updatedConfigFile, JSON.stringify(updatedConfig)); + + const config = await import('../src/config'); + + await waitForMockCall(proxyStart); + + expect(proxyStop).toHaveBeenCalledTimes(1); + expect(proxyStart).toHaveBeenCalledTimes(1); + expect(config.getSessionMaxAgeHours()).toBe(updatedConfig.sessionMaxAgeHours); + }); + + it('should restart the proxy with the previous config when updates fail', async () => { + const updatedConfigFile = path.join(tempDir, 'updated-settings.json'); + const proxyStop = vi.fn().mockRejectedValue(new Error('stop failed')); + const proxyStart = vi.fn().mockResolvedValue(undefined); + vi.doMock('../src/proxy', () => ({ + stop: proxyStop, + start: proxyStart, + })); + const consoleErrorSpy = vi.spyOn(console, 'error').mockImplementation(() => {}); + + const configWithSources = { + configurationSources: { + enabled: true, + sources: [ + { + type: 'file', + enabled: true, + path: updatedConfigFile, + }, + ], + }, + }; + const updatedConfig = { + ...defaultSettings, + configurationSources: configWithSources.configurationSources, + sessionMaxAgeHours: 8, + }; + + fs.writeFileSync(tempUserFile, JSON.stringify(configWithSources)); + fs.writeFileSync(updatedConfigFile, JSON.stringify(updatedConfig)); + + try { + await import('../src/config'); + + await waitForMockCall(proxyStart); + + expect(proxyStop).toHaveBeenCalledTimes(1); + expect(proxyStart).toHaveBeenCalledTimes(1); + expect(consoleErrorSpy).toHaveBeenCalledWith( + 'Failed to apply new configuration: stop failed', + ); + } finally { + consoleErrorSpy.mockRestore(); } + }); + + afterEach(() => { if (fs.existsSync(tempDir)) { - fs.rmdirSync(tempDir); + fs.rmSync(tempDir, { recursive: true, force: true }); } process.env = oldEnv; + vi.doUnmock('../src/proxy'); vi.resetModules(); }); }); @@ -567,5 +679,14 @@ describe('loadFullConfiguration', () => { 'Invalid regular expression for commitConfig.author.email.local.block: [invalid(regex', ); }); + + it('should throw error when merged defaults miss required top-level values', async () => { + const config = await import('../src/config'); + const settingsWithoutSink = { ...defaultSettings, sink: undefined }; + + expect(() => config.assertHasRequiredTopLevelConfig(settingsWithoutSink)).toThrow( + 'Missing required top-level configuration values: sink', + ); + }); }); });