fix(sdk): avoid shell command injection in CLI exec calls (#21508)
## What Fixes the 5 open [CodeQL code-scanning alerts](https://github.com/twentyhq/twenty/security/code-scanning) of type `js/shell-command-constructed-from-input` (medium severity) in the `twenty-sdk` CLI. All flagged call sites built a shell command string by interpolating library inputs (`containerName`, `image`, `npmTag`) and ran it via `execSync`, which executes through a shell. A value containing shell metacharacters could break out of the intended command. ## Changes - `packages/twenty-sdk/src/cli/utilities/server/docker-container.ts` — every `docker` call switched from `execSync` with a template string to `execFileSync('docker', [...args])`. Arguments are passed as an array, so the binary runs directly without a shell and inputs are never reparsed. The single quotes that were shell-quoting the `-f` format strings are removed since there is no shell. - `packages/twenty-sdk/src/cli/operations/publish.ts` — `npm publish` switched to `execFileSync` with `--tag`/value as separate array elements. On Windows the binary resolves to `npm.cmd` (no shell to do the lookup). ## Verification - `npx nx typecheck twenty-sdk` passed - `npx nx lint twenty-sdk` passed <!-- This is an auto-generated description by cubic. --> <a href="https://cubic.dev/pr/twentyhq/twenty/pull/21508?utm_source=github" target="_blank" rel="noopener noreferrer" data-no-image-dialog="true"><picture><source media="(prefers-color-scheme: dark)" srcset="https://www.cubic.dev/buttons/review-in-cubic-dark.svg"><source media="(prefers-color-scheme: light)" srcset="https://www.cubic.dev/buttons/review-in-cubic-light.svg"><img alt="Review in cubic" src="https://www.cubic.dev/buttons/review-in-cubic-dark.svg"></picture></a> <!-- End of auto-generated description by cubic. -->
This commit is contained in:
@@ -1,4 +1,4 @@
|
||||
import { execSync } from 'child_process';
|
||||
import { execFileSync } from 'child_process';
|
||||
|
||||
import { runSafe } from '@/cli/utilities/run-safe';
|
||||
import { appBuild } from './build';
|
||||
@@ -30,12 +30,28 @@ const innerAppPublish = async (
|
||||
|
||||
onProgress?.('Publishing to npm...');
|
||||
|
||||
const tagArg = options.npmTag ? ` --tag ${options.npmTag}` : '';
|
||||
if (options.npmTag && !/^[a-zA-Z0-9][a-zA-Z0-9._-]*$/.test(options.npmTag)) {
|
||||
return {
|
||||
success: false,
|
||||
error: {
|
||||
code: APP_ERROR_CODES.PUBLISH_FAILED,
|
||||
message: `Invalid npm tag: ${options.npmTag}`,
|
||||
},
|
||||
};
|
||||
}
|
||||
|
||||
const publishArgs = [
|
||||
'publish',
|
||||
...(options.npmTag ? ['--tag', options.npmTag] : []),
|
||||
];
|
||||
|
||||
const isWindows = process.platform === 'win32';
|
||||
|
||||
try {
|
||||
execSync(`npm publish${tagArg}`, {
|
||||
execFileSync(isWindows ? 'npm.cmd' : 'npm', publishArgs, {
|
||||
cwd: buildResult.data.outputDir,
|
||||
stdio: 'inherit',
|
||||
shell: isWindows,
|
||||
});
|
||||
} catch {
|
||||
return {
|
||||
|
||||
@@ -1,4 +1,4 @@
|
||||
import { execSync } from 'node:child_process';
|
||||
import { execFileSync } from 'node:child_process';
|
||||
|
||||
export const CONTAINER_NAME = 'twenty-app-dev';
|
||||
export const TEST_CONTAINER_NAME = 'twenty-app-dev-test';
|
||||
@@ -8,8 +8,9 @@ export const DEFAULT_TEST_PORT = 2021;
|
||||
|
||||
export const isContainerRunning = (containerName = CONTAINER_NAME): boolean => {
|
||||
try {
|
||||
const result = execSync(
|
||||
`docker inspect -f '{{.State.Running}}' ${containerName}`,
|
||||
const result = execFileSync(
|
||||
'docker',
|
||||
['inspect', '-f', '{{.State.Running}}', containerName],
|
||||
{ encoding: 'utf-8', stdio: ['pipe', 'pipe', 'ignore'] },
|
||||
).trim();
|
||||
|
||||
@@ -24,8 +25,14 @@ export const getContainerPort = (containerName = CONTAINER_NAME): number => {
|
||||
containerName === TEST_CONTAINER_NAME ? DEFAULT_TEST_PORT : DEFAULT_PORT;
|
||||
|
||||
try {
|
||||
const result = execSync(
|
||||
`docker inspect -f '{{range .Config.Env}}{{println .}}{{end}}' ${containerName}`,
|
||||
const result = execFileSync(
|
||||
'docker',
|
||||
[
|
||||
'inspect',
|
||||
'-f',
|
||||
'{{range .Config.Env}}{{println .}}{{end}}',
|
||||
containerName,
|
||||
],
|
||||
{ encoding: 'utf-8', stdio: ['pipe', 'pipe', 'ignore'] },
|
||||
);
|
||||
|
||||
@@ -39,7 +46,7 @@ export const getContainerPort = (containerName = CONTAINER_NAME): number => {
|
||||
|
||||
export const containerExists = (containerName = CONTAINER_NAME): boolean => {
|
||||
try {
|
||||
execSync(`docker inspect ${containerName}`, {
|
||||
execFileSync('docker', ['inspect', containerName], {
|
||||
stdio: ['pipe', 'pipe', 'ignore'],
|
||||
});
|
||||
|
||||
@@ -56,10 +63,14 @@ export const getContainerDigest = (
|
||||
containerName = CONTAINER_NAME,
|
||||
): string | null => {
|
||||
try {
|
||||
return execSync(`docker inspect -f '{{.Image}}' ${containerName}`, {
|
||||
encoding: 'utf-8',
|
||||
stdio: ['pipe', 'pipe', 'ignore'],
|
||||
}).trim();
|
||||
return execFileSync(
|
||||
'docker',
|
||||
['inspect', '-f', '{{.Image}}', containerName],
|
||||
{
|
||||
encoding: 'utf-8',
|
||||
stdio: ['pipe', 'pipe', 'ignore'],
|
||||
},
|
||||
).trim();
|
||||
} catch {
|
||||
return null;
|
||||
}
|
||||
@@ -67,7 +78,7 @@ export const getContainerDigest = (
|
||||
|
||||
export const getImageDigest = (image: string): string | null => {
|
||||
try {
|
||||
return execSync(`docker inspect -f '{{.Id}}' ${image}`, {
|
||||
return execFileSync('docker', ['inspect', '-f', '{{.Id}}', image], {
|
||||
encoding: 'utf-8',
|
||||
stdio: ['pipe', 'pipe', 'ignore'],
|
||||
}).trim();
|
||||
@@ -81,8 +92,14 @@ export const getContainerEnvVar = (
|
||||
containerName = CONTAINER_NAME,
|
||||
): string | null => {
|
||||
try {
|
||||
const result = execSync(
|
||||
`docker inspect -f '{{range .Config.Env}}{{println .}}{{end}}' ${containerName}`,
|
||||
const result = execFileSync(
|
||||
'docker',
|
||||
[
|
||||
'inspect',
|
||||
'-f',
|
||||
'{{range .Config.Env}}{{println .}}{{end}}',
|
||||
containerName,
|
||||
],
|
||||
{ encoding: 'utf-8', stdio: ['pipe', 'pipe', 'ignore'] },
|
||||
);
|
||||
|
||||
@@ -96,7 +113,7 @@ export const getContainerEnvVar = (
|
||||
|
||||
export const checkDockerRunning = (): boolean => {
|
||||
try {
|
||||
execSync('docker info', { stdio: 'ignore' });
|
||||
execFileSync('docker', ['info'], { stdio: 'ignore' });
|
||||
|
||||
return true;
|
||||
} catch {
|
||||
|
||||
Reference in New Issue
Block a user