Skip to content

Commit c7c1a31

Browse files
authored
fix(config): type-guard remaining CLI option validation against non-strings (#128)
Follow-up to #124, applying the same defensive pattern to the rest of validateCliOptions and the providedValues construction in initConfig: - domain, email, apiPath, protocol, authType: type-check before calling string methods (trim/startsWith/toLowerCase/includes), so non-string truthy inputs (e.g. numeric values from programmatic callers) surface a clean validation error instead of a TypeError - The early authType normalization in initConfig (`.trim().toLowerCase()` before validateCliOptions runs) is also guarded so the crash cannot occur before validation gets a chance to report it - Adds regression tests covering domain, apiPath, protocol, and authType
1 parent bb02928 commit c7c1a31

2 files changed

Lines changed: 61 additions & 7 deletions

File tree

lib/config.js

Lines changed: 11 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -257,20 +257,20 @@ function saveConfigFile(data) {
257257
const validateCliOptions = (options) => {
258258
const errors = [];
259259

260-
if (options.domain && !options.domain.trim()) {
260+
if (options.domain && (typeof options.domain !== 'string' || !options.domain.trim())) {
261261
errors.push('--domain cannot be empty');
262262
}
263263

264264
if (options.token !== undefined && (typeof options.token !== 'string' || !options.token.trim())) {
265265
errors.push('--token cannot be empty');
266266
}
267267

268-
if (options.email && !options.email.trim()) {
268+
if (options.email && (typeof options.email !== 'string' || !options.email.trim())) {
269269
errors.push('--email cannot be empty');
270270
}
271271

272272
if (options.apiPath) {
273-
if (!options.apiPath.startsWith('/')) {
273+
if (typeof options.apiPath !== 'string' || !options.apiPath.startsWith('/')) {
274274
errors.push('--api-path must start with "/"');
275275
} else {
276276
// Validate API path format
@@ -282,16 +282,18 @@ const validateCliOptions = (options) => {
282282
}
283283
}
284284

285-
if (options.protocol && !['http', 'https'].includes(options.protocol.toLowerCase())) {
285+
if (options.protocol && (typeof options.protocol !== 'string' || !['http', 'https'].includes(options.protocol.toLowerCase()))) {
286286
errors.push('--protocol must be "http" or "https"');
287287
}
288288

289-
if (options.authType && !AUTH_TYPES.includes(options.authType.toLowerCase())) {
289+
if (options.authType && (typeof options.authType !== 'string' || !AUTH_TYPES.includes(options.authType.toLowerCase()))) {
290290
errors.push('--auth-type must be "basic", "bearer", "mtls", or "cookie"');
291291
}
292292

293293
// Check if basic auth is provided with email
294-
const normAuthType = options.authType ? normalizeAuthType(options.authType, Boolean(options.email)) : null;
294+
const normAuthType = (typeof options.authType === 'string' && options.authType)
295+
? normalizeAuthType(options.authType, Boolean(options.email))
296+
: null;
295297
if (normAuthType === 'basic' && !options.email) {
296298
errors.push('--email is required when using basic authentication (use your username for on-premise)');
297299
}
@@ -512,7 +514,9 @@ async function initConfig(cliOptions = {}) {
512514
protocol: cliOptions.protocol,
513515
domain: cliOptions.domain,
514516
apiPath: cliOptions.apiPath,
515-
authType: cliOptions.authType ? cliOptions.authType.trim().toLowerCase() : undefined,
517+
authType: (typeof cliOptions.authType === 'string' && cliOptions.authType)
518+
? cliOptions.authType.trim().toLowerCase()
519+
: cliOptions.authType,
516520
email: cliOptions.email,
517521
token: cliOptions.token,
518522
cookie: cliOptions.cookie,

tests/config.test.js

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -349,4 +349,54 @@ describe('initConfig CLI option validation', () => {
349349
expect.stringMatching(/--token cannot be empty/)
350350
);
351351
});
352+
353+
test('non-string --domain surfaces a validation error instead of crashing', async () => {
354+
await expect(initConfig({
355+
domain: 12345,
356+
token: 'valid-token',
357+
authType: 'bearer',
358+
})).rejects.toThrow('process.exit called');
359+
360+
expect(errorSpy).toHaveBeenCalledWith(
361+
expect.stringMatching(/--domain cannot be empty/)
362+
);
363+
});
364+
365+
test('non-string --api-path surfaces a validation error instead of crashing', async () => {
366+
await expect(initConfig({
367+
domain: 'example.com',
368+
apiPath: 12345,
369+
token: 'valid-token',
370+
authType: 'bearer',
371+
})).rejects.toThrow('process.exit called');
372+
373+
expect(errorSpy).toHaveBeenCalledWith(
374+
expect.stringMatching(/--api-path must start with/)
375+
);
376+
});
377+
378+
test('non-string --protocol surfaces a validation error instead of crashing', async () => {
379+
await expect(initConfig({
380+
domain: 'example.com',
381+
protocol: 12345,
382+
token: 'valid-token',
383+
authType: 'bearer',
384+
})).rejects.toThrow('process.exit called');
385+
386+
expect(errorSpy).toHaveBeenCalledWith(
387+
expect.stringMatching(/--protocol must be/)
388+
);
389+
});
390+
391+
test('non-string --auth-type surfaces a validation error instead of crashing', async () => {
392+
await expect(initConfig({
393+
domain: 'example.com',
394+
token: 'valid-token',
395+
authType: 12345,
396+
})).rejects.toThrow('process.exit called');
397+
398+
expect(errorSpy).toHaveBeenCalledWith(
399+
expect.stringMatching(/--auth-type must be/)
400+
);
401+
});
352402
});

0 commit comments

Comments
 (0)