From 0ecf9f5435473bd038cd17faab63dad5403185b1 Mon Sep 17 00:00:00 2001 From: John Mendelewski Date: Fri, 3 Nov 2023 18:46:38 -0400 Subject: [PATCH] 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. --- packages/cli/lang/en.lyaml | 2 + packages/cli/lib/__tests__/projects.test.js | 150 ++++++++++++++++++++ packages/cli/lib/projects.js | 19 ++- 3 files changed, 167 insertions(+), 4 deletions(-) create mode 100644 packages/cli/lib/__tests__/projects.test.js diff --git a/packages/cli/lang/en.lyaml b/packages/cli/lang/en.lyaml index a911ece8e..e8d7f3262 100644 --- a/packages/cli/lang/en.lyaml +++ b/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: "Project source directory '{{ srcDir }}' should be contained within '{{ projectDir }}'" uploadProjectFiles: add: "Uploading {{#bold}}{{ projectName }}{{/bold}} project files to {{ accountIdentifier }}" fail: "Failed to upload {{#bold}}{{ projectName }}{{/bold}} project files to {{ accountIdentifier }}" diff --git a/packages/cli/lib/__tests__/projects.test.js b/packages/cli/lib/__tests__/projects.test.js new file mode 100644 index 000000000..f4cf51b8a --- /dev/null +++ b/packages/cli/lib/__tests__/projects.test.js @@ -0,0 +1,150 @@ +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.stringMatching( + new RegExp(`.*'..'.*contained within.*'${projectDir}'.*`) + ) + ); + }); + + it('for root directory', () => { + projects.validateProjectConfig( + { name: 'hello', srcDir: '/' }, + projectDir + ); + + expect(exitMock).toHaveBeenCalledWith(EXIT_CODES.ERROR); + expect(errorSpy).toHaveBeenCalledWith( + expect.stringMatching( + new RegExp(`.*'/'.*contained within.*'${projectDir}'.*`) + ) + ); + }); + + 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.stringMatching( + new RegExp(`.*'${srcDir}'.*contained within.*'${projectDir}'.*`) + ) + ); + }); + }); + + 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(); + }); + }); + }); +}); diff --git a/packages/cli/lib/projects.js b/packages/cli/lib/projects.js index fd9c816e0..a850c5c88 100644 --- a/packages/cli/lib/projects.js +++ b/packages/cli/lib/projects.js @@ -159,21 +159,32 @@ 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); } - if (!fs.existsSync(path.resolve(projectDir, projectConfig.srcDir))) { + const resolvedPath = path.resolve(projectDir, projectConfig.srcDir); + if (!resolvedPath.startsWith(projectDir)) { + logger.error( + i18n(`${i18nKey}.config.srcOutsideProjectDir`, { + srcDir: projectConfig.srcDir, + projectDir, + }) + ); + return process.exit(EXIT_CODES.ERROR); + } + + 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); } };