added further restrictions on arguments

Author: TheSlimvReal

src/app.controller.spec.ts

@@ -1,20 +1,30 @@
 import { AppController } from './app.controller';
 import { Test, TestingModule } from '@nestjs/testing';
-import { of, Subject, Subscription, throwError } from 'rxjs';
+import {
+  firstValueFrom,
+  lastValueFrom,
+  of,
+  Subject,
+  Subscription,
+  throwError,
+} from 'rxjs';
 import { HttpService } from '@nestjs/axios';
 import { BadRequestException, UnauthorizedException } from '@nestjs/common';
 import { DeploymentInfo } from './deployment-info.dto';
 import { ConfigModule } from '@nestjs/config';
 import * as fs from 'fs';
 
 let onSubscription: Subscription;
-const lines = new Subject<string>();
+let lines: Subject<string>;
 jest.mock('tail', () => {
   // Mock Tail constructor
   return {
     Tail: jest.fn().mockReturnValue({
       on: (event, callback) => (onSubscription = lines.subscribe(callback)),
-      unwatch: () => onSubscription.unsubscribe(),
+      unwatch: () => {
+        console.log('subscription', onSubscription);
+        onSubscription.unsubscribe();
+      },
     }),
   };
 });
@@ -37,6 +47,7 @@ describe('AppController', () => {
   };
 
   beforeEach(async () => {
+    lines = new Subject();
     mockWs = { write: jest.fn(), close: jest.fn() };
     jest.spyOn(fs, 'createWriteStream').mockReturnValue(mockWs as any);
     mockHttp = {
@@ -67,45 +78,107 @@ describe('AppController', () => {
     });
   });
 
-  it('should throw bad request exception if data has wrong format', (done) => {
-    const invalidData = { ...deploymentData, name: 'with space' };
-    // TODO: add an extensive list of invalid formats including attempts someone could pass to try and inject code?
+  it('should throw bad request exception if name has wrong format', async () => {
+    passDeployment();
+    function testName(name: string) {
+      return lastValueFrom(controller.deployApp({ ...deploymentData, name }));
+    }
+    await expect(testName('with space')).rejects.toBeInstanceOf(
+      BadRequestException,
+    );
+    await expect(testName('withCapital')).resolves.toBeTruthy();
+    await expect(testName('withSymbol?')).rejects.toBeInstanceOf(
+      BadRequestException,
+    );
+    await expect(testName('withNumber123')).resolves.toBeTruthy();
+    await expect(testName('with-dash')).resolves.toBeTruthy();
+    await expect(testName('with_underscore')).rejects.toBeInstanceOf(
+      BadRequestException,
+    );
+  });
 
-    controller.deployApp(invalidData).subscribe({
-      error: (err) => {
-        expect(err).toBeInstanceOf(BadRequestException);
-        done();
-      },
+  function passDeployment() {
+    // automatically finish deployment
+    jest.spyOn(lines, 'subscribe').mockImplementation((fn) => {
+      setTimeout(() => fn('DONE'));
+      return { unsubscribe: () => undefined } as any;
     });
+  }
+
+  it('should throw bad request exception if username has wrong format', async () => {
+    passDeployment();
+    function testName(username: string) {
+      return lastValueFrom(
+        controller.deployApp({ ...deploymentData, username }),
+      );
+    }
+
+    await expect(testName('with space')).rejects.toBeInstanceOf(
+      BadRequestException,
+    );
+    await expect(testName('withCapital')).resolves.toBeTruthy();
+    await expect(testName('withSymbol?')).rejects.toBeInstanceOf(
+      BadRequestException,
+    );
+    await expect(testName('withNumber123')).resolves.toBeTruthy();
+    await expect(testName('with-dash')).resolves.toBeTruthy();
+    await expect(testName('with_underscore')).rejects.toBeInstanceOf(
+      BadRequestException,
+    );
   });
 
-  it('should throw error if ERROR is written to log', (done) => {
-    controller.deployApp(deploymentData).subscribe({
-      error: (err: BadRequestException) => {
-        expect(err).toBeInstanceOf(BadRequestException);
-        expect(err.message).toBe('my custom error');
-        // Ensure tail is properly "unwatched"
-        expect(onSubscription.closed).toBeTruthy();
-        done();
-      },
-    });
+  it('should throw bad request exception if email has wrong format', async () => {
+    passDeployment();
+    function testMail(email: string) {
+      return lastValueFrom(controller.deployApp({ ...deploymentData, email }));
+    }
+
+    await expect(testMail('testmail')).rejects.toBeInstanceOf(
+      BadRequestException,
+    );
+    await expect(testMail('test@mail')).rejects.toBeInstanceOf(
+      BadRequestException,
+    );
+    await expect(testMail('Test@[email protected]')).rejects.toBeInstanceOf(
+      BadRequestException,
+    );
+    await expect(testMail('[email protected]')).resolves.toBeTruthy();
+    await expect(testMail('[email protected]')).resolves.toBeTruthy();
+    await expect(testMail('[email protected]')).resolves.toBeTruthy();
+    await expect(testMail('[email protected]')).resolves.toBeTruthy();
+    await expect(testMail('[email protected]')).resolves.toBeTruthy();
+  });
 
+  it('should throw error if ERROR is written to log', async () => {
+    const res = firstValueFrom(controller.deployApp(deploymentData));
     lines.next('some logs');
     lines.next('ERROR my custom error');
-  });
 
-  it('should write arguments to file', (done) => {
-    controller.deployApp(deploymentData).subscribe(() => {
-      expect(mockWs.write).toHaveBeenCalledWith(
-        'test-name de [email protected] test-username test-base y n',
-      );
-      expect(mockWs.close).toHaveBeenCalled();
+    try {
+      await res;
+    } catch (err) {
+      expect(err).toBeInstanceOf(BadRequestException);
+      expect(err.message).toBe('my custom error');
       // Ensure tail is properly "unwatched"
       expect(onSubscription.closed).toBeTruthy();
-      done();
-    });
+      return;
+    }
+
+    throw new Error('No error thrown');
+  });
+
+  it('should write arguments to file', () => {
+    const res = firstValueFrom(controller.deployApp(deploymentData));
 
     lines.next('DONE');
+
+    expect(res).resolves.toBeTruthy();
+    expect(mockWs.write).toHaveBeenCalledWith(
+      'test-name de [email protected] test-username test-base y n',
+    );
+    expect(mockWs.close).toHaveBeenCalled();
+    // Ensure tail is properly "unwatched"
+    expect(onSubscription.closed).toBeTruthy();
   });
 
   it('should use the default locale if empty', (done) => {

src/app.controller.ts

@@ -51,6 +51,24 @@ export class AppController {
       throw new BadRequestException('No spaces allowed in arguments');
     }
 
+    if (!deploymentInfo.name.match(/^[a-zA-Z0-9\-]*$/)) {
+      throw new BadRequestException(
+        'Only letters, numbers and dashes are allowed in name',
+      );
+    }
+
+    if (!deploymentInfo.username.match(/^[a-zA-Z0-9\-]*$/)) {
+      throw new BadRequestException(
+        'Only letters, numbers and dashes are allowed in username',
+      );
+    }
+
+    // See https://regex101.com/r/lHs2R3/1
+    const emailRegex = /^[\w\-.]+@([\w-]+\.)+[\w-]{2,}$/;
+    if (!deploymentInfo.email.match(emailRegex)) {
+      throw new BadRequestException('Not a valid email');
+    }
+
     const args = `${deploymentInfo.name} ${deploymentInfo.locale || 'en'} ${
       deploymentInfo.email
     } ${deploymentInfo.username} ${deploymentInfo.base} ${
@@ -80,7 +98,6 @@ export class AppController {
         result.complete();
       }
     });
-
     return result.asObservable();
   }
 }