fix(create-astro): make --add stricter

.changeset/public-lemons-mate.md

@@ -0,0 +1,5 @@
+---
+'create-astro': patch
+---
+
+Fixes an issue where `--add` could accept any kind of string, leading to different errors. Now `--add` accepts only values of valid integrations and adapters.

.changeset/quiet-cars-burn.md

@@ -0,0 +1,7 @@
+---
+'create-astro': patch
+'astro': patch
+---
+
+Fixes an issue where the `add` command could accept any arbitrary value, leading the possible command injections. Now `add` and `--add` accepts 
+values that are only acceptable npmjs.org names.

.changeset/tender-bats-tan.md

@@ -0,0 +1,5 @@
+---
+'@astrojs/internal-helpers': minor
+---
+
+Adds a new `/cli` specifier and the utility `NPM_PACKAGE_NAME_REGEX`.

packages/astro/src/cli/add/index.ts

@@ -2,6 +2,7 @@ import fsMod, { existsSync, promises as fs } from 'node:fs';
 import { createRequire } from 'node:module';
 import path from 'node:path';
 import { fileURLToPath, pathToFileURL } from 'node:url';
+import { assertValidPackageName } from '@astrojs/internal-helpers/cli';
 import boxen from 'boxen';
 import { diffWords } from 'diff';
 import { type ASTNode, builders, generateCode, loadFile, type ProxifiedModule } from 'magicast';
@@ -901,6 +902,11 @@ async function validateIntegrations(
 	flags: yargsParser.Arguments,
 	logger: Logger,
 ): Promise<IntegrationInfo[]> {
+	// First, validate all package names to prevent command injection
+	for (const integration of integrations) {
+		assertValidPackageName(integration);
+	}
+
 	const spinner = yoctoSpinner({ text: 'Resolving packages...' }).start();
 	try {
 		const integrationEntries = await Promise.all(

packages/create-astro/package.json

@@ -35,6 +35,7 @@
     "@bluwy/giget-core": "^0.1.6"
   },
   "devDependencies": {
+    "@astrojs/internal-helpers": "workspace:*",
     "arg": "^5.0.2",
     "astro-scripts": "workspace:*"
   },

packages/create-astro/src/actions/dependencies.ts

@@ -1,5 +1,6 @@
 import fs from 'node:fs';
 import path from 'node:path';
+import { assertValidPackageName } from '@astrojs/internal-helpers/cli';
 import { color } from '@astrojs/cli-kit';
 import { error, info, title } from '../messages.js';
 import { shell } from '../shell.js';
@@ -23,9 +24,15 @@ export async function dependencies(
 		}));
 		ctx.install = deps;
 	}
-
 	ctx.add = ctx.add?.reduce<string[]>((acc, item) => acc.concat(item.split(',')), []);
 
+	if (ctx.add) {
+		for (const addValue of ctx.add) {
+			// Validate package name to prevent command injection attacks
+			assertValidPackageName(addValue);
+		}
+	}
+
 	if (ctx.dryRun) {
 		await info(
 			'--dry-run',
@@ -86,11 +93,15 @@ async function astroAdd({
 	cwd: string;
 }) {
 	if (packageManager === 'yarn') await ensureYarnLock({ cwd });
-	return shell(
-		packageManager === 'npm' ? 'npx' : `${packageManager} dlx`,
-		['astro add', integrations.join(' '), '-y'],
-		{ cwd, timeout: 90_000, stdio: 'ignore' },
-	);
+
+	// Build command and args properly for spawn without shell interpretation
+	const command = packageManager === 'npm' ? 'npx' : packageManager;
+	const args =
+		packageManager === 'npm'
+			? ['astro', 'add', ...integrations, '-y']
+			: ['dlx', 'astro', 'add', ...integrations, '-y'];
+
+	return shell(command, args, { cwd, timeout: 90_000, stdio: 'ignore' });
 }
 
 async function install({ packageManager, cwd }: { packageManager: string; cwd: string }) {

packages/create-astro/src/shell.ts

@@ -27,7 +27,7 @@ export async function shell(
 	let stdout = '';
 	let stderr = '';
 	try {
-		child = spawn(`${command} ${flags.join(' ')}`, {
+		child = spawn(command, flags, {
 			cwd: opts.cwd,
 			shell: true,
 			stdio: opts.stdio,

packages/create-astro/test/dependencies.test.js

@@ -38,7 +38,6 @@ describe('dependencies', () => {
 	it('prompt no', async () => {
 		const context = {
 			cwd: '',
-			install: true,
 			packageManager: 'npm',
 			dryRun: true,
 			prompt: () => ({ deps: false }),
@@ -78,4 +77,41 @@ describe('dependencies', () => {
 		assert.ok(fixture.hasMessage('Skipping dependency installation'));
 		assert.equal(context.install, false);
 	});
+
+	describe('--add', async () => {
+		it('fails for non-supported integration', async () => {
+			let context = {
+				cwd: '',
+				add: ['foo '],
+				dryRun: true,
+				prompt: () => ({ deps: false }),
+			};
+
+			try {
+				await dependencies(context);
+				assert.fail('The function should throw an error');
+			} catch (error) {
+				assert.ok(
+					error.message.includes('Invalid package name "foo "'),
+					`Expected error about invalid package name, got: ${error.message}`,
+				);
+			}
+			context = {
+				cwd: '',
+				add: ['react', 'bar lorem'],
+				dryRun: true,
+				prompt: () => ({ deps: false }),
+			};
+
+			try {
+				await dependencies(context);
+				assert.fail('The function should throw an error');
+			} catch (error) {
+				assert.ok(
+					error.message.includes('Invalid package name "bar lorem"'),
+					`Expected error about invalid package name, got: ${error.message}`,
+				);
+			}
+		});
+	});
 });

packages/create-astro/test/integrations.test.js

@@ -62,4 +62,136 @@ describe('integrations', () => {
 		await dependencies(context);
 		assert.ok(fixture.hasMessage('--dry-run Skipping dependency installation'));
 	});
+
+	describe('Security: Command injection protection', () => {
+		it('blocks semicolon command injection', async () => {
+			const context = {
+				cwd: '',
+				yes: true,
+				packageManager: 'npm',
+				dryRun: true,
+				add: ['react;whoami'],
+			};
+
+			await assert.rejects(
+				() => dependencies(context),
+				/Invalid package name/,
+				'Should reject command injection with semicolon',
+			);
+		});
+
+		it('blocks command substitution with $()', async () => {
+			const context = {
+				cwd: '',
+				yes: true,
+				packageManager: 'npm',
+				dryRun: true,
+				add: ['react$(whoami)'],
+			};
+
+			await assert.rejects(
+				() => dependencies(context),
+				/Invalid package name/,
+				'Should reject command substitution with $()',
+			);
+		});
+
+		it('blocks command substitution with backticks', async () => {
+			const context = {
+				cwd: '',
+				yes: true,
+				packageManager: 'npm',
+				dryRun: true,
+				add: ['react`whoami`'],
+			};
+
+			await assert.rejects(
+				() => dependencies(context),
+				/Invalid package name/,
+				'Should reject command substitution with backticks',
+			);
+		});
+
+		it('blocks pipe operators', async () => {
+			const context = {
+				cwd: '',
+				yes: true,
+				packageManager: 'npm',
+				dryRun: true,
+				add: ['react|whoami'],
+			};
+
+			await assert.rejects(
+				() => dependencies(context),
+				/Invalid package name/,
+				'Should reject pipe operator injection',
+			);
+		});
+
+		it('blocks ampersand operators', async () => {
+			const context = {
+				cwd: '',
+				yes: true,
+				packageManager: 'npm',
+				dryRun: true,
+				add: ['react&&whoami'],
+			};
+
+			await assert.rejects(
+				() => dependencies(context),
+				/Invalid package name/,
+				'Should reject ampersand operator injection',
+			);
+		});
+
+		it('blocks redirect operators', async () => {
+			const context = {
+				cwd: '',
+				yes: true,
+				packageManager: 'npm',
+				dryRun: true,
+				add: ['react>file'],
+			};
+
+			await assert.rejects(
+				() => dependencies(context),
+				/Invalid package name/,
+				'Should reject redirect operator injection',
+			);
+		});
+
+		it('allows scoped packages', async () => {
+			const context = {
+				cwd: '',
+				yes: true,
+				packageManager: 'npm',
+				dryRun: true,
+				add: ['@astrojs/tailwind'],
+			};
+
+			await dependencies(context);
+			assert.ok(
+				fixture.hasMessage(
+					'--dry-run Skipping dependency installation and adding @astrojs/tailwind',
+				),
+			);
+		});
+
+		it('allows valid package names', async () => {
+			const context = {
+				cwd: '',
+				yes: true,
+				packageManager: 'npm',
+				dryRun: true,
+				add: ['my-package', 'package_2.0'],
+			};
+
+			await dependencies(context);
+			assert.ok(
+				fixture.hasMessage(
+					'--dry-run Skipping dependency installation and adding my-package, package_2.0',
+				),
+			);
+		});
+	});
 });