Skip to content

Commit cf12f8d

Browse files
dcoricdcoric
committed
feat(config): require merged top-level defaults
1 parent 6981427 commit cf12f8d

3 files changed

Lines changed: 95 additions & 22 deletions

File tree

‎src/config/index.ts‎

Lines changed: 70 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -25,8 +25,55 @@ import { getConfigFile } from './file';
2525
import { validateConfig } from './validators';
2626
import { handleErrorAndLog, handleErrorAndThrow } from '../utils/errors';
2727

28+
// Deprecated compatibility fields are still optional because the defaults do not set them.
29+
type OptionalTopLevelConfigKey = 'proxyUrl' | 'sslCertPemPath' | 'sslKeyPemPath';
30+
type RequiredTopLevelConfigKey = Exclude<keyof GitProxyConfig, OptionalTopLevelConfigKey>;
31+
32+
export type FullGitProxyConfig = Required<Omit<GitProxyConfig, OptionalTopLevelConfigKey>> &
33+
Pick<GitProxyConfig, OptionalTopLevelConfigKey>;
34+
35+
const REQUIRED_TOP_LEVEL_CONFIG_KEYS = [
36+
'api',
37+
'apiAuthentication',
38+
'attestationConfig',
39+
'authentication',
40+
'authorisedList',
41+
'commitConfig',
42+
'configurationSources',
43+
'contactEmail',
44+
'cookieSecret',
45+
'csrfProtection',
46+
'domains',
47+
'plugins',
48+
'privateOrganizations',
49+
'rateLimit',
50+
'sessionMaxAgeHours',
51+
'sink',
52+
'tempPassword',
53+
'tls',
54+
'uiRouteAuth',
55+
'urlShortener',
56+
] as const satisfies readonly RequiredTopLevelConfigKey[];
57+
58+
type MissingRequiredTopLevelConfigKeys = Exclude<
59+
RequiredTopLevelConfigKey,
60+
(typeof REQUIRED_TOP_LEVEL_CONFIG_KEYS)[number]
61+
>;
62+
type AssertNever<T extends never> = T;
63+
type _RequiredTopLevelConfigKeysAreExhaustive = AssertNever<MissingRequiredTopLevelConfigKeys>;
64+
65+
function assertHasRequiredTopLevelConfig(
66+
config: GitProxyConfig,
67+
): asserts config is FullGitProxyConfig {
68+
const missingKeys = REQUIRED_TOP_LEVEL_CONFIG_KEYS.filter((key) => config[key] === undefined);
69+
70+
if (missingKeys.length > 0) {
71+
throw new Error(`Missing required top-level configuration values: ${missingKeys.join(', ')}`);
72+
}
73+
}
74+
2875
// Cache for current configuration
29-
let _currentConfig: GitProxyConfig | null = null;
76+
let _currentConfig: FullGitProxyConfig | null = null;
3077
let _configLoader: ConfigLoader | null = null;
3178

3279
// Function to invalidate cache - useful for testing
@@ -57,17 +104,17 @@ function cleanUndefinedValues(obj: any): any {
57104

58105
/**
59106
* Load and merge default + user configuration with QuickType validation
60-
* @return {GitProxyConfig} The merged and validated configuration
107+
* @return {FullGitProxyConfig} The merged and validated configuration
61108
*/
62-
function loadFullConfiguration(): GitProxyConfig {
109+
function loadFullConfiguration(): FullGitProxyConfig {
63110
if (_currentConfig) {
64111
return _currentConfig;
65112
}
66113

67114
const rawDefaultConfig = Convert.toGitProxyConfig(JSON.stringify(defaultSettings));
68115

69116
// Clean undefined values from defaultConfig
70-
const defaultConfig = cleanUndefinedValues(rawDefaultConfig);
117+
const defaultConfig = cleanUndefinedValues(rawDefaultConfig) as GitProxyConfig;
71118

72119
let userSettings: Partial<GitProxyConfig> = {};
73120
const userConfigFile = process.env.CONFIG_FILE || getConfigFile();
@@ -102,12 +149,12 @@ function loadFullConfiguration(): GitProxyConfig {
102149
* Merge configurations with environment variable overrides
103150
* @param {GitProxyConfig} defaultConfig - The default configuration
104151
* @param {Partial<GitProxyConfig>} userSettings - User-provided configuration overrides
105-
* @return {GitProxyConfig} The merged configuration
152+
* @return {FullGitProxyConfig} The merged configuration
106153
*/
107154
function mergeConfigurations(
108155
defaultConfig: GitProxyConfig,
109156
userSettings: Partial<GitProxyConfig>,
110-
): GitProxyConfig {
157+
): FullGitProxyConfig {
111158
// Special handling for TLS configuration when legacy fields are used
112159
let tlsConfig = userSettings.tls || defaultConfig.tls;
113160

@@ -121,7 +168,7 @@ function mergeConfigurations(
121168
};
122169
}
123170

124-
return {
171+
const config = {
125172
...defaultConfig,
126173
...userSettings,
127174
// Deep merge for specific objects
@@ -141,6 +188,9 @@ function mergeConfigurations(
141188
userSettings.cookieSecret ||
142189
defaultConfig.cookieSecret,
143190
};
191+
192+
assertHasRequiredTopLevelConfig(config);
193+
return config;
144194
}
145195

146196
// Get configured proxy URL
@@ -152,7 +202,7 @@ export const getProxyUrl = (): string | undefined => {
152202
// Gets a list of authorised repositories
153203
export const getAuthorisedList = () => {
154204
const config = loadFullConfiguration();
155-
return config.authorisedList || [];
205+
return config.authorisedList;
156206
};
157207

158208
// Gets a list of authorised repositories
@@ -164,7 +214,7 @@ export const getTempPasswordConfig = () => {
164214
// Gets the configured data sink, defaults to filesystem
165215
export const getDatabase = () => {
166216
const config = loadFullConfiguration();
167-
const databases = config.sink || [];
217+
const databases = config.sink;
168218

169219
for (const db of databases) {
170220
if (db.enabled) {
@@ -187,7 +237,7 @@ export const getDatabase = () => {
187237
*/
188238
export const getAuthMethods = () => {
189239
const config = loadFullConfiguration();
190-
const authSources = config.authentication || [];
240+
const authSources = config.authentication;
191241

192242
const enabledAuthMethods = authSources.filter((auth) => auth.enabled);
193243

@@ -206,7 +256,7 @@ export const getAuthMethods = () => {
206256
*/
207257
export const getAPIAuthMethods = () => {
208258
const config = loadFullConfiguration();
209-
const apiAuthSources = config.apiAuthentication || [];
259+
const apiAuthSources = config.apiAuthentication;
210260

211261
return apiAuthSources.filter((auth: { enabled: any }) => auth.enabled);
212262
};
@@ -227,7 +277,7 @@ export const logConfiguration = () => {
227277

228278
export const getAPIs = () => {
229279
const config = loadFullConfiguration();
230-
return config.api || {};
280+
return config.api;
231281
};
232282

233283
export const getCookieSecret = (): string => {
@@ -242,25 +292,25 @@ export const getCookieSecret = (): string => {
242292

243293
export const getSessionMaxAgeHours = (): number => {
244294
const config = loadFullConfiguration();
245-
return config.sessionMaxAgeHours || 24;
295+
return config.sessionMaxAgeHours;
246296
};
247297

248298
// Get commit related configuration
249299
export const getCommitConfig = () => {
250300
const config = loadFullConfiguration();
251-
return config.commitConfig || {};
301+
return config.commitConfig;
252302
};
253303

254304
// Get attestation related configuration
255305
export const getAttestationConfig = () => {
256306
const config = loadFullConfiguration();
257-
return config.attestationConfig || {};
307+
return config.attestationConfig;
258308
};
259309

260310
// Get private organizations related configuration
261311
export const getPrivateOrganizations = () => {
262312
const config = loadFullConfiguration();
263-
return config.privateOrganizations || [];
313+
return config.privateOrganizations;
264314
};
265315

266316
// Get URL shortener
@@ -284,7 +334,7 @@ export const getCSRFProtection = (): boolean | undefined => {
284334
// Get loadable push plugins
285335
export const getPlugins = () => {
286336
const config = loadFullConfiguration();
287-
return config.plugins || [];
337+
return config.plugins;
288338
};
289339

290340
export const getTLSKeyPemPath = (): string | undefined => {
@@ -304,12 +354,12 @@ export const getTLSEnabled = (): boolean => {
304354

305355
export const getDomains = () => {
306356
const config = loadFullConfiguration();
307-
return config.domains || {};
357+
return config.domains;
308358
};
309359

310360
export const getUIRouteAuth = () => {
311361
const config = loadFullConfiguration();
312-
return config.uiRouteAuth || {};
362+
return config.uiRouteAuth;
313363
};
314364

315365
export const getRateLimit = () => {
@@ -331,6 +381,7 @@ const handleConfigUpdate = async (newConfig: Configuration) => {
331381
await proxy.stop();
332382

333383
// 4. Update config
384+
assertHasRequiredTopLevelConfig(validatedConfig);
334385
_currentConfig = validatedConfig;
335386

336387
// 5. Restart services with new config

‎src/service/index.ts‎

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -33,8 +33,6 @@ const limiter = rateLimit(config.getRateLimit());
3333

3434
const { GIT_PROXY_UI_PORT: uiPort } = serverConfig;
3535

36-
const DEFAULT_SESSION_MAX_AGE_HOURS = 12;
37-
3836
const app: Express = express();
3937
let _httpServer: http.Server | null = null;
4038

@@ -143,7 +141,7 @@ async function createApp(proxy: Proxy): Promise<Express> {
143141
cookie: {
144142
secure: 'auto',
145143
httpOnly: true,
146-
maxAge: (config.getSessionMaxAgeHours() || DEFAULT_SESSION_MAX_AGE_HOURS) * 60 * 60 * 1000,
144+
maxAge: config.getSessionMaxAgeHours() * 60 * 60 * 1000,
147145
},
148146
}),
149147
);

‎test/testConfig.test.ts‎

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@ describe('default configuration', () => {
3636
expect(config.getAuthMethods()).toEqual(enabledMethods);
3737
expect(config.getDatabase()).toEqual(defaultSettings.sink[0]);
3838
expect(config.getTempPasswordConfig()).toEqual(defaultSettings.tempPassword);
39+
expect(config.getProxyUrl()).toBeUndefined();
3940
expect(config.getAuthorisedList()).toEqual(defaultSettings.authorisedList);
4041
expect(config.getRateLimit()).toEqual(defaultSettings.rateLimit);
4142
expect(config.getTLSKeyPemPath()).toEqual(defaultSettings.tls.key);
@@ -46,8 +47,15 @@ describe('default configuration', () => {
4647
expect(config.getContactEmail()).toEqual(defaultSettings.contactEmail);
4748
expect(config.getPlugins()).toEqual(defaultSettings.plugins);
4849
expect(config.getCSRFProtection()).toEqual(defaultSettings.csrfProtection);
50+
expect(config.getSessionMaxAgeHours()).toEqual(defaultSettings.sessionMaxAgeHours);
51+
expect(config.getCommitConfig()).toEqual(defaultSettings.commitConfig);
4952
expect(config.getAttestationConfig()).toEqual(defaultSettings.attestationConfig);
5053
expect(config.getAPIs()).toEqual(defaultSettings.api);
54+
expect(config.getAPIAuthMethods()).toEqual(
55+
defaultSettings.apiAuthentication.filter((method) => method.enabled),
56+
);
57+
expect(config.getPrivateOrganizations()).toEqual(defaultSettings.privateOrganizations);
58+
expect(config.getUIRouteAuth()).toEqual(defaultSettings.uiRouteAuth);
5159
});
5260
});
5361

@@ -261,6 +269,22 @@ describe('user configuration', () => {
261269
expect(config.getAPIs()).toEqual(user.api);
262270
});
263271

272+
it('should keep default top-level config values when user config only overrides one entry', async () => {
273+
const user = {
274+
sessionMaxAgeHours: 6,
275+
};
276+
fs.writeFileSync(tempUserFile, JSON.stringify(user));
277+
278+
const config = await import('../src/config');
279+
config.invalidateCache();
280+
281+
expect(config.getSessionMaxAgeHours()).toBe(user.sessionMaxAgeHours);
282+
expect(config.getCommitConfig()).toEqual(defaultSettings.commitConfig);
283+
expect(config.getAttestationConfig()).toEqual(defaultSettings.attestationConfig);
284+
expect(config.getPrivateOrganizations()).toEqual(defaultSettings.privateOrganizations);
285+
expect(config.getUIRouteAuth()).toEqual(defaultSettings.uiRouteAuth);
286+
});
287+
264288
it('should override default settings for cookieSecret if env var is used', async () => {
265289
fs.writeFileSync(tempUserFile, '{}');
266290
process.env.GIT_PROXY_COOKIE_SECRET = 'test-cookie-secret';

0 commit comments

Comments
 (0)
⚡