From df9d9b1a245640b40f1cd6a0006854a0cb541b11 Mon Sep 17 00:00:00 2001 From: Narek Hovhannisyan Date: Tue, 3 Dec 2024 19:43:26 +0400 Subject: [PATCH 1/5] feat(.github): init codeql-config --- .github/codeql/codeql-config.yml | 7 +++++++ 1 file changed, 7 insertions(+) create mode 100644 .github/codeql/codeql-config.yml diff --git a/.github/codeql/codeql-config.yml b/.github/codeql/codeql-config.yml new file mode 100644 index 00000000..b1d2347f --- /dev/null +++ b/.github/codeql/codeql-config.yml @@ -0,0 +1,7 @@ +paths: + - "src" +paths-ignore: + - "src/__tests__/**/*.js" + - "src/__tests__/**/*.ts" + - "src/__tests__/**/*.jsx" + - "src/__tests__/**/*.tsx" From a6712ab18117aabd2e8235ae0183ab77b10cec3c Mon Sep 17 00:00:00 2001 From: Narek Hovhannisyan Date: Wed, 4 Dec 2024 23:46:59 +0400 Subject: [PATCH 2/5] fix(util): tune npm to escape dangerous shell commands --- src/if-check/util/npm.ts | 45 +++++++++++++++++++++++----------------- 1 file changed, 26 insertions(+), 19 deletions(-) diff --git a/src/if-check/util/npm.ts b/src/if-check/util/npm.ts index a6137a9d..a3846e49 100644 --- a/src/if-check/util/npm.ts +++ b/src/if-check/util/npm.ts @@ -1,43 +1,50 @@ import * as path from 'node:path'; - import {execPromise} from '../../common/util/helpers'; import {getFileName, removeFileIfExists} from '../../common/util/fs'; - import {STRINGS} from '../config'; const {IF_CHECK_VERIFIED} = STRINGS; +/** + * Escapes shell characters that could be dangerous in a command. + */ +const escapeShellArg = (str: string) => str.replace(/([`$\\&;|*?<>])/g, '\\$1'); + /** * Executes a series of npm commands based on the provided manifest file. */ export const executeCommands = async (manifest: string, cwd: boolean) => { - // TODO: After release remove isGlobal and appropriate checks const isGlobal = !!process.env.npm_config_global; const manifestDirPath = path.dirname(manifest); const manifestFileName = getFileName(manifest); const executedManifest = path.join(manifestDirPath, `re-${manifestFileName}`); + const prefixFlag = process.env.CURRENT_DIR && process.env.CURRENT_DIR !== process.cwd() ? `--prefix=${path.relative(process.env.CURRENT_DIR!, process.cwd())}` : ''; - const ifEnv = `${ - isGlobal ? `if-env ${prefixFlag}` : `npm run if-env ${prefixFlag} --` - } -m ${manifest}`; - const ifEnvCommand = cwd ? `${ifEnv} -c` : ifEnv; + + const sanitizedManifest = escapeShellArg(manifest); + const sanitizedExecutedManifest = escapeShellArg(executedManifest); + + const ifEnvCommand = `${ + isGlobal ? 'if-env' : 'npm run if-env' + } ${prefixFlag} -- -m ${sanitizedManifest}`; const ifRunCommand = `${ - isGlobal ? `if-run ${prefixFlag}` : `npm run if-run ${prefixFlag} --` - } -m ${manifest} -o ${executedManifest}`; + isGlobal ? 'if-run' : 'npm run if-run' + } ${prefixFlag} -- -m ${sanitizedManifest} -o ${sanitizedExecutedManifest}`; + const ttyCommand = "node -p 'Boolean(process.stdout.isTTY)'"; const ifDiffCommand = `${ - isGlobal ? `if-diff ${prefixFlag}` : `npm run if-diff ${prefixFlag} --` - } -s ${executedManifest}.yaml -t ${manifest}`; - const ttyCommand = " node -p 'Boolean(process.stdout.isTTY)'"; - - await execPromise( - `${ifEnvCommand} && ${ifRunCommand} && ${ttyCommand} | ${ifDiffCommand}`, - { - cwd: process.env.CURRENT_DIR || process.cwd(), - } - ); + isGlobal ? 'if-diff' : 'npm run if-diff' + } ${prefixFlag} -- -s ${sanitizedExecutedManifest}.yaml -t ${sanitizedManifest}`; + + const fullCommand = + [ifEnvCommand, ifRunCommand, ttyCommand].join(' && ') + + ` | ${ifDiffCommand}`; + + await execPromise(fullCommand, { + cwd: process.env.CURRENT_DIR || process.cwd(), + }); if (!cwd) { await removeFileIfExists(`${manifestDirPath}/package.json`); From ace8e45c7eaf7d6c27711179d419395bbc23b231 Mon Sep 17 00:00:00 2001 From: Narek Hovhannisyan Date: Wed, 4 Dec 2024 23:47:45 +0400 Subject: [PATCH 3/5] test(util): fix assertion in npm units --- src/__tests__/if-check/util/npm.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/__tests__/if-check/util/npm.test.ts b/src/__tests__/if-check/util/npm.test.ts index 4bdffc10..8a516f67 100644 --- a/src/__tests__/if-check/util/npm.test.ts +++ b/src/__tests__/if-check/util/npm.test.ts @@ -25,7 +25,7 @@ jest.mock('../../../common/util/helpers', () => { break; case 'if-check': expect(param).toEqual( - "npm run if-env -- -m ./src/__mocks__/mock-manifest.yaml && npm run if-run -- -m ./src/__mocks__/mock-manifest.yaml -o src/__mocks__/re-mock-manifest && node -p 'Boolean(process.stdout.isTTY)' | npm run if-diff -- -s src/__mocks__/re-mock-manifest.yaml -t ./src/__mocks__/mock-manifest.yaml" + "npm run if-env -- -m ./src/__mocks__/mock-manifest.yaml && npm run if-run -- -m ./src/__mocks__/mock-manifest.yaml -o src/__mocks__/re-mock-manifest && node -p 'Boolean(process.stdout.isTTY)' | npm run if-diff -- -s src/__mocks__/re-mock-manifest.yaml -t ./src/__mocks__/mock-manifest.yaml" ); break; } From ced3b21b10ed69d43cb1343f10bfadbc187fe76f Mon Sep 17 00:00:00 2001 From: Narek Hovhannisyan Date: Thu, 5 Dec 2024 12:09:48 +0400 Subject: [PATCH 4/5] fix(util): tune npm to escape injection on shell commands --- src/if-check/util/npm.ts | 53 +++++++++++++++++++++++++++++----------- 1 file changed, 39 insertions(+), 14 deletions(-) diff --git a/src/if-check/util/npm.ts b/src/if-check/util/npm.ts index a3846e49..9717819f 100644 --- a/src/if-check/util/npm.ts +++ b/src/if-check/util/npm.ts @@ -27,21 +27,46 @@ export const executeCommands = async (manifest: string, cwd: boolean) => { const sanitizedManifest = escapeShellArg(manifest); const sanitizedExecutedManifest = escapeShellArg(executedManifest); - const ifEnvCommand = `${ - isGlobal ? 'if-env' : 'npm run if-env' - } ${prefixFlag} -- -m ${sanitizedManifest}`; - const ifRunCommand = `${ - isGlobal ? 'if-run' : 'npm run if-run' - } ${prefixFlag} -- -m ${sanitizedManifest} -o ${sanitizedExecutedManifest}`; - const ttyCommand = "node -p 'Boolean(process.stdout.isTTY)'"; - const ifDiffCommand = `${ - isGlobal ? 'if-diff' : 'npm run if-diff' - } ${prefixFlag} -- -s ${sanitizedExecutedManifest}.yaml -t ${sanitizedManifest}`; - - const fullCommand = - [ifEnvCommand, ifRunCommand, ttyCommand].join(' && ') + - ` | ${ifDiffCommand}`; + const ifEnvCommand = [ + isGlobal ? 'if-env' : 'npm run if-env', + '--', + ...(prefixFlag === '' ? [] : prefixFlag), + '-m', + sanitizedManifest, + ]; + const ifRunCommand = [ + isGlobal ? 'if-run' : 'npm run if-run', + '--', + ...(prefixFlag === '' ? [] : prefixFlag), + '-m', + sanitizedManifest, + '-o', + sanitizedExecutedManifest, + ]; + + const ttyCommand = ['node', '-p', "'Boolean(process.stdout.isTTY)'"]; + const ifDiffCommand = [ + isGlobal ? 'if-diff' : 'npm run if-diff', + '--', + ...(prefixFlag === '' ? [] : prefixFlag), + '-s', + `${sanitizedExecutedManifest}.yaml`, + '-t', + sanitizedManifest, + ]; + + const fullCommand = [ + ...ifEnvCommand, + '&&', + ...ifRunCommand, + '&&', + ...ttyCommand, + '|', + ...ifDiffCommand, + ].join(' '); + + // Execute the full command await execPromise(fullCommand, { cwd: process.env.CURRENT_DIR || process.cwd(), }); From 5070fbbdca30a212d7ef1ac471fea4ae43604bbe Mon Sep 17 00:00:00 2001 From: Narek Hovhannisyan Date: Thu, 5 Dec 2024 12:10:37 +0400 Subject: [PATCH 5/5] test(util): remove redundant spaces from assertion --- src/__tests__/if-check/util/npm.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/__tests__/if-check/util/npm.test.ts b/src/__tests__/if-check/util/npm.test.ts index 8a516f67..cad8b9bd 100644 --- a/src/__tests__/if-check/util/npm.test.ts +++ b/src/__tests__/if-check/util/npm.test.ts @@ -25,7 +25,7 @@ jest.mock('../../../common/util/helpers', () => { break; case 'if-check': expect(param).toEqual( - "npm run if-env -- -m ./src/__mocks__/mock-manifest.yaml && npm run if-run -- -m ./src/__mocks__/mock-manifest.yaml -o src/__mocks__/re-mock-manifest && node -p 'Boolean(process.stdout.isTTY)' | npm run if-diff -- -s src/__mocks__/re-mock-manifest.yaml -t ./src/__mocks__/mock-manifest.yaml" + "npm run if-env -- -m ./src/__mocks__/mock-manifest.yaml && npm run if-run -- -m ./src/__mocks__/mock-manifest.yaml -o src/__mocks__/re-mock-manifest && node -p 'Boolean(process.stdout.isTTY)' | npm run if-diff -- -s src/__mocks__/re-mock-manifest.yaml -t ./src/__mocks__/mock-manifest.yaml" ); break; }