fix: Error when unknown parameters are passed to the main executable

* bug: error when unknown parameters are passed to the main executable

* bug: error on unknown paramters and adapted to review

* fix: test wont pass in ci

* Update src/init/CliRunner.ts

Co-authored-by: Ruben Verborgh <ruben@verborgh.org>

* fix: adapted to review

* fix: made CliRunner.run async

Co-authored-by: Arne Vandoorslaer <arne@digita.ai>
Co-authored-by: Ruben Verborgh <ruben@verborgh.org>

src/init/CliRunner.ts

@@ -5,7 +5,7 @@ import type { IComponentsManagerBuilderOptions, LogLevel } from 'componentsjs';
 import { ComponentsManager } from 'componentsjs';
 import yargs from 'yargs';
 import { getLoggerFor } from '../logging/LogUtil';
-import { joinFilePath, ensureTrailingSlash, absoluteFilePath } from '../util/PathUtil';
+import { absoluteFilePath, ensureTrailingSlash, joinFilePath } from '../util/PathUtil';
 import type { Initializer } from './Initializer';
 
 export class CliRunner {
@@ -16,7 +16,7 @@ export class CliRunner {
    * @param args - Command line arguments.
    * @param stderr - Standard error stream.
    */
-  public run({
+  public async run({
     argv = process.argv,
     stderr = process.stderr,
   }: {
@@ -24,10 +24,30 @@ export class CliRunner {
     stdin?: ReadStream;
     stdout?: WriteStream;
     stderr?: WriteStream;
-  } = {}): void {
+  } = {}): Promise<void> {
     // Parse the command-line arguments
     const { argv: params } = yargs(argv.slice(2))
       .usage('node ./bin/server.js [args]')
+      .check((args, options): boolean => {
+        // Only take flags as arguments, not filenames
+        if (args._ && args._.length > 0) {
+          throw new Error(`Unsupported arguments: ${args._.join('", "')}`);
+        }
+        for (const key in args) {
+          // Skip filename arguments (_) and the script name ($0)
+          if (key !== '_' && key !== '$0') {
+            // Check if the argument occurs in the provided options list
+            if (!options[key]) {
+              throw new Error(`Unknown option: "${key}"`);
+            }
+            // Check if the argument actually has a value ('> ./bin/server.js -s' is not valid)
+            if (!args[key]) {
+              throw new Error(`Missing value for argument "${key}"`);
+            }
+          }
+        }
+        return true;
+      })
       .options({
         baseUrl: { type: 'string', alias: 'b' },
         config: { type: 'string', alias: 'c' },
@@ -51,7 +71,7 @@ export class CliRunner {
     const variables = this.createVariables(params);
 
     // Create and execute the server initializer
-    this.createInitializer(loaderProperties, configFile, variables)
+    await this.createInitializer(loaderProperties, configFile, variables)
       .then(
         async(initializer): Promise<void> => initializer.handleSafe(),
         (error: Error): void => {

test/unit/init/CliRunner.test.ts

@@ -31,15 +31,10 @@ describe('CliRunner', (): void => {
   });
 
   it('starts the server with default settings.', async(): Promise<void> => {
-    new CliRunner().run({
+    await new CliRunner().run({
       argv: [ 'node', 'script' ],
     });
 
-    // Wait until initializer has been called, because we can't await CliRunner.run.
-    await new Promise((resolve): void => {
-      setImmediate(resolve);
-    });
-
     expect(ComponentsManager.build).toHaveBeenCalledTimes(1);
     expect(ComponentsManager.build).toHaveBeenCalledWith({
       dumpErrorState: true,
@@ -69,7 +64,7 @@ describe('CliRunner', (): void => {
   });
 
   it('accepts abbreviated flags.', async(): Promise<void> => {
-    new CliRunner().run({
+    await new CliRunner().run({
       argv: [
         'node', 'script',
         '-b', 'http://pod.example/',
@@ -84,11 +79,6 @@ describe('CliRunner', (): void => {
       ],
     });
 
-    // Wait until initializer has been called, because we can't await CliRunner.run.
-    await new Promise((resolve): void => {
-      setImmediate(resolve);
-    });
-
     expect(ComponentsManager.build).toHaveBeenCalledTimes(1);
     expect(ComponentsManager.build).toHaveBeenCalledWith({
       dumpErrorState: true,
@@ -115,7 +105,7 @@ describe('CliRunner', (): void => {
   });
 
   it('accepts full flags.', async(): Promise<void> => {
-    new CliRunner().run({
+    await new CliRunner().run({
       argv: [
         'node', 'script',
         '--baseUrl', 'http://pod.example/',
@@ -130,11 +120,6 @@ describe('CliRunner', (): void => {
       ],
     });
 
-    // Wait until initializer has been called, because we can't await CliRunner.run.
-    await new Promise((resolve): void => {
-      setImmediate(resolve);
-    });
-
     expect(ComponentsManager.build).toHaveBeenCalledTimes(1);
     expect(ComponentsManager.build).toHaveBeenCalledWith({
       dumpErrorState: true,
@@ -162,10 +147,9 @@ describe('CliRunner', (): void => {
 
   it('exits with output to stderr when instantiation fails.', async(): Promise<void> => {
     manager.instantiate.mockRejectedValueOnce(new Error('Fatal'));
-    new CliRunner().run({
+    await new CliRunner().run({
       argv: [ 'node', 'script' ],
     });
-    await new Promise((resolve): any => setImmediate(resolve));
 
     expect(write).toHaveBeenCalledTimes(2);
     expect(write).toHaveBeenNthCalledWith(1,
@@ -179,11 +163,42 @@ describe('CliRunner', (): void => {
 
   it('exits without output to stderr when initialization fails.', async(): Promise<void> => {
     initializer.handleSafe.mockRejectedValueOnce(new Error('Fatal'));
-    new CliRunner().run();
+    await new CliRunner().run();
-    await new Promise((resolve): any => setImmediate(resolve));
 
     expect(write).toHaveBeenCalledTimes(0);
 
+    expect(exit).toHaveBeenCalledWith(1);
+  });
+
+  it('exits when unknown options are passed to the main executable.', async(): Promise<void> => {
+    await new CliRunner().run({
+      argv: [
+        'node', 'script', '--foo',
+      ],
+    });
+
+    expect(exit).toHaveBeenCalledTimes(1);
+    expect(exit).toHaveBeenCalledWith(1);
+  });
+
+  it('exits when no value is passed to the main executable for an argument.', async(): Promise<void> => {
+    await new CliRunner().run({
+      argv: [
+        'node', 'script', '-s',
+      ],
+    });
+
+    expect(exit).toHaveBeenCalledTimes(1);
+    expect(exit).toHaveBeenCalledWith(1);
+  });
+
+  it('exits when unknown parameters are passed to the main executable.', async(): Promise<void> => {
+    await new CliRunner().run({
+      argv: [
+        'node', 'script', 'foo', 'bar', 'foo.txt', 'bar.txt',
+      ],
+    });
+
     expect(exit).toHaveBeenCalledTimes(1);
     expect(exit).toHaveBeenCalledWith(1);
   });