Validate project config srcDir within project root (#951)

* Validate project config `srcDir` within project root

Adds validation within `validateProjectConfig` to ensure the srcDir of a project points to either the project root, or a subdirectory.
This will help ensure a user doesn't attempt to upload a parent or root directory by mistake.

Adds a suite of jest tests to confirm our current validations, as well as the new validation logic.

To support the testing with mocks around `process.exit`, added `return` statements to all the spots in `validateProjectConfig` where the whole node process would be existing anyway.
Open to reverting those changes and using exceptions in the mocking of `process.exit` if that'd be cleaner / clearer.

* Add validateProjectConfig to project dev command

Also reworks the error message for srcOutsideProjectDir to be more informative and actionable

* Fix validateProjectConfig tests

* Tweak project validation error message based on DX feedback

Drops including the "project root" filesystem path, and just has the message say project root.

packages/cli/commands/project/dev.js

@@ -25,6 +25,7 @@ const {
   handleProjectUpload,
   pollProjectBuildAndDeploy,
   showPlatformVersionWarning,
+  validateProjectConfig,
 } = require('../../lib/projects');
 const { EXIT_CODES } = require('../../lib/enums/exitCodes');
 const {
@@ -87,6 +88,8 @@ exports.handler = async options => {
     process.exit(EXIT_CODES.ERROR);
   }
 
+  validateProjectConfig(projectConfig, projectDir);
+
   const accounts = getConfigAccounts();
   let targetAccountId = options.account ? accountId : null;
   let createNewSandbox = false;

packages/cli/lang/en.lyaml

@@ -917,6 +917,8 @@ en:
           startError: "Failed to start local dev server: {{ message }}"
           fileChangeError: "Failed to notify local dev server of file change: {{ message }}"
       projects:
+        config:
+          srcOutsideProjectDir: "Invalid value for 'srcDir' in {{ projectConfig }}: {{#bold}}srcDir: \"{{ srcDir }}\"{{/bold}}\n\t'srcDir' must be a relative path to a folder under the project root, such as \".\" or \"./src\""
         uploadProjectFiles:
           add: "Uploading {{#bold}}{{ projectName }}{{/bold}} project files to {{ accountIdentifier }}"
           fail: "Failed to upload {{#bold}}{{ projectName }}{{/bold}} project files to {{ accountIdentifier }}"

packages/cli/lib/__tests__/projects.test.js

@@ -0,0 +1,144 @@
+const fs = require('fs');
+const os = require('os');
+const path = require('path');
+const { EXIT_CODES } = require('../enums/exitCodes');
+const projects = require('../projects');
+
+describe('@hubspot/cli/lib/projects', () => {
+  describe('validateProjectConfig()', () => {
+    let realProcess;
+    let projectDir;
+    let exitMock;
+    let errorSpy;
+
+    beforeAll(() => {
+      projectDir = fs.mkdtempSync(path.join(os.tmpdir(), 'projects-'));
+      fs.mkdirSync(path.join(projectDir, 'src'));
+
+      realProcess = process;
+      errorSpy = jest.spyOn(console, 'error');
+    });
+
+    beforeEach(() => {
+      exitMock = jest.fn();
+      global.process = { ...realProcess, exit: exitMock };
+    });
+
+    afterEach(() => {
+      errorSpy.mockClear();
+    });
+
+    afterAll(() => {
+      global.process = realProcess;
+      errorSpy.mockRestore();
+    });
+
+    it('rejects undefined configuration', () => {
+      projects.validateProjectConfig(null, projectDir);
+
+      expect(exitMock).toHaveBeenCalledWith(EXIT_CODES.ERROR);
+      expect(errorSpy).toHaveBeenCalledWith(
+        expect.stringMatching(/.*config not found.*/)
+      );
+    });
+
+    it('rejects configuration with missing name', () => {
+      projects.validateProjectConfig({ srcDir: '.' }, projectDir);
+
+      expect(exitMock).toHaveBeenCalledWith(EXIT_CODES.ERROR);
+      expect(errorSpy).toHaveBeenCalledWith(
+        expect.stringMatching(/.*missing required fields*/)
+      );
+    });
+
+    it('rejects configuration with missing srcDir', () => {
+      projects.validateProjectConfig({ name: 'hello' }, projectDir);
+
+      expect(exitMock).toHaveBeenCalledWith(EXIT_CODES.ERROR);
+      expect(errorSpy).toHaveBeenCalledWith(
+        expect.stringMatching(/.*missing required fields.*/)
+      );
+    });
+
+    describe('rejects configuration with srcDir outside project directory', () => {
+      it('for parent directory', () => {
+        projects.validateProjectConfig(
+          { name: 'hello', srcDir: '..' },
+          projectDir
+        );
+
+        expect(exitMock).toHaveBeenCalledWith(EXIT_CODES.ERROR);
+        expect(errorSpy).toHaveBeenCalledWith(
+          expect.stringContaining('srcDir: ".."')
+        );
+      });
+
+      it('for root directory', () => {
+        projects.validateProjectConfig(
+          { name: 'hello', srcDir: '/' },
+          projectDir
+        );
+
+        expect(exitMock).toHaveBeenCalledWith(EXIT_CODES.ERROR);
+        expect(errorSpy).toHaveBeenCalledWith(
+          expect.stringContaining('srcDir: "/"')
+        );
+      });
+
+      it('for complicated directory', () => {
+        const srcDir = './src/././../src/../../src';
+
+        projects.validateProjectConfig({ name: 'hello', srcDir }, projectDir);
+
+        expect(exitMock).toHaveBeenCalledWith(EXIT_CODES.ERROR);
+        expect(errorSpy).toHaveBeenCalledWith(
+          expect.stringContaining(`srcDir: "${srcDir}"`)
+        );
+      });
+    });
+
+    it('rejects configuration with srcDir that does not exist', () => {
+      projects.validateProjectConfig(
+        { name: 'hello', srcDir: 'foo' },
+        projectDir
+      );
+
+      expect(exitMock).toHaveBeenCalledWith(EXIT_CODES.ERROR);
+      expect(errorSpy).toHaveBeenCalledWith(
+        expect.stringMatching(/.*could not be found in.*/)
+      );
+    });
+
+    describe('accepts configuration with valid srcDir', () => {
+      it('for current directory', () => {
+        projects.validateProjectConfig(
+          { name: 'hello', srcDir: '.' },
+          projectDir
+        );
+
+        expect(exitMock).not.toHaveBeenCalled();
+        expect(errorSpy).not.toHaveBeenCalled();
+      });
+
+      it('for relative directory', () => {
+        projects.validateProjectConfig(
+          { name: 'hello', srcDir: './src' },
+          projectDir
+        );
+
+        expect(exitMock).not.toHaveBeenCalled();
+        expect(errorSpy).not.toHaveBeenCalled();
+      });
+
+      it('for implied relative directory', () => {
+        projects.validateProjectConfig(
+          { name: 'hello', srcDir: 'src' },
+          projectDir
+        );
+
+        expect(exitMock).not.toHaveBeenCalled();
+        expect(errorSpy).not.toHaveBeenCalled();
+      });
+    });
+  });
+});

packages/cli/lib/projects.js

@@ -159,21 +159,36 @@ const validateProjectConfig = (projectConfig, projectDir) => {
     logger.error(
       `Project config not found. Try running 'hs project create' first.`
     );
-    process.exit(EXIT_CODES.ERROR);
+    return process.exit(EXIT_CODES.ERROR);
   }
 
   if (!projectConfig.name || !projectConfig.srcDir) {
     logger.error(
       'Project config is missing required fields. Try running `hs project create`.'
     );
-    process.exit(EXIT_CODES.ERROR);
+    return process.exit(EXIT_CODES.ERROR);
+  }
+
+  const resolvedPath = path.resolve(projectDir, projectConfig.srcDir);
+  if (!resolvedPath.startsWith(projectDir)) {
+    const projectConfigFile = path.relative(
+      '.',
+      path.join(projectDir, PROJECT_CONFIG_FILE)
+    );
+    logger.error(
+      i18n(`${i18nKey}.config.srcOutsideProjectDir`, {
+        srcDir: projectConfig.srcDir,
+        projectConfig: projectConfigFile,
+      })
+    );
+    return process.exit(EXIT_CODES.ERROR);
   }
 
-  if (!fs.existsSync(path.resolve(projectDir, projectConfig.srcDir))) {
+  if (!fs.existsSync(resolvedPath)) {
     logger.error(
       `Project source directory '${projectConfig.srcDir}' could not be found in ${projectDir}.`
     );
-    process.exit(EXIT_CODES.ERROR);
+    return process.exit(EXIT_CODES.ERROR);
   }
 };