fix(cli): validate user input before running commands

Author: danielroe

cli/package.json

@@ -29,10 +29,12 @@
     "h3": "^1.15.5",
     "listhen": "^1.9.0",
     "ofetch": "^1.5.1",
-    "picocolors": "^1.1.1"
+    "picocolors": "^1.1.1",
+    "validate-npm-package-name": "^7.0.2"
   },
   "devDependencies": {
     "@types/node": "^24.10.9",
+    "@types/validate-npm-package-name": "^4.0.2",
     "typescript": "^5.9.3",
     "unbuild": "^3.6.1"
   }

cli/src/npm-client.ts

@@ -1,9 +1,68 @@
 import process from 'node:process'
-import { exec } from 'node:child_process'
+import { execFile } from 'node:child_process'
 import { promisify } from 'node:util'
+import validateNpmPackageName from 'validate-npm-package-name'
 import { logCommand, logSuccess, logError } from './logger.ts'
 
-const execAsync = promisify(exec)
+const execFileAsync = promisify(execFile)
+
+// Validation pattern for npm usernames/org names
+// These follow similar rules: lowercase alphanumeric with hyphens, can't start/end with hyphen
+const NPM_USERNAME_RE = /^[a-z0-9]([a-z0-9-]*[a-z0-9])?$/i
+
+/**
+ * Validates an npm package name using the official npm validation package
+ * @throws Error if the name is invalid
+ */
+export function validatePackageName(name: string): void {
+  const result = validateNpmPackageName(name)
+  if (!result.validForNewPackages && !result.validForOldPackages) {
+    const errors = result.errors || result.warnings || ['Invalid package name']
+    throw new Error(`Invalid package name "${name}": ${errors.join(', ')}`)
+  }
+}
+
+/**
+ * Validates an npm username
+ * @throws Error if the username is invalid
+ */
+export function validateUsername(name: string): void {
+  if (!name || name.length > 50 || !NPM_USERNAME_RE.test(name)) {
+    throw new Error(`Invalid username: ${name}`)
+  }
+}
+
+/**
+ * Validates an npm org name (without the @ prefix)
+ * @throws Error if the org name is invalid
+ */
+export function validateOrgName(name: string): void {
+  if (!name || name.length > 50 || !NPM_USERNAME_RE.test(name)) {
+    throw new Error(`Invalid org name: ${name}`)
+  }
+}
+
+/**
+ * Validates a scope:team format (e.g., @myorg:developers)
+ * @throws Error if the scope:team is invalid
+ */
+export function validateScopeTeam(scopeTeam: string): void {
+  if (!scopeTeam || scopeTeam.length > 100) {
+    throw new Error(`Invalid scope:team: ${scopeTeam}`)
+  }
+  // Format: @scope:team
+  const match = scopeTeam.match(/^@([^:]+):(.+)$/)
+  if (!match) {
+    throw new Error(`Invalid scope:team format: ${scopeTeam}`)
+  }
+  const [, scope, team] = match
+  if (!scope || !NPM_USERNAME_RE.test(scope)) {
+    throw new Error(`Invalid scope in scope:team: ${scopeTeam}`)
+  }
+  if (!team || !NPM_USERNAME_RE.test(team)) {
+    throw new Error(`Invalid team name in scope:team: ${scopeTeam}`)
+  }
+}
 
 export interface NpmExecResult {
   stdout: string
@@ -56,20 +115,21 @@ export async function execNpm(
   args: string[],
   options: { otp?: string; silent?: boolean } = {},
 ): Promise<NpmExecResult> {
-  const cmd = ['npm', ...args]
-
-  if (options.otp) {
-    cmd.push('--otp', options.otp)
-  }
+  // Build the full args array including OTP if provided
+  const npmArgs = options.otp ? [...args, '--otp', options.otp] : args
 
   // Log the command being run (hide OTP value for security)
   if (!options.silent) {
-    const displayCmd = options.otp ? ['npm', ...args, '--otp', '******'].join(' ') : cmd.join(' ')
+    const displayCmd = options.otp
+      ? ['npm', ...args, '--otp', '******'].join(' ')
+      : ['npm', ...args].join(' ')
     logCommand(displayCmd)
   }
 
   try {
-    const { stdout, stderr } = await execAsync(cmd.join(' '), {
+    // Use execFile instead of exec to avoid shell injection vulnerabilities
+    // execFile does not spawn a shell, so metacharacters are passed literally
+    const { stdout, stderr } = await execFileAsync('npm', npmArgs, {
       timeout: 60000,
       env: { ...process.env, FORCE_COLOR: '0' },
     })
@@ -127,6 +187,8 @@ export async function orgAddUser(
   role: 'developer' | 'admin' | 'owner',
   otp?: string,
 ): Promise<NpmExecResult> {
+  validateOrgName(org)
+  validateUsername(user)
   return execNpm(['org', 'set', org, user, role], { otp })
 }
 
@@ -135,14 +197,18 @@ export async function orgRemoveUser(
   user: string,
   otp?: string,
 ): Promise<NpmExecResult> {
+  validateOrgName(org)
+  validateUsername(user)
   return execNpm(['org', 'rm', org, user], { otp })
 }
 
 export async function teamCreate(scopeTeam: string, otp?: string): Promise<NpmExecResult> {
+  validateScopeTeam(scopeTeam)
   return execNpm(['team', 'create', scopeTeam], { otp })
 }
 
 export async function teamDestroy(scopeTeam: string, otp?: string): Promise<NpmExecResult> {
+  validateScopeTeam(scopeTeam)
   return execNpm(['team', 'destroy', scopeTeam], { otp })
 }
 
@@ -151,6 +217,8 @@ export async function teamAddUser(
   user: string,
   otp?: string,
 ): Promise<NpmExecResult> {
+  validateScopeTeam(scopeTeam)
+  validateUsername(user)
   return execNpm(['team', 'add', scopeTeam, user], { otp })
 }
 
@@ -159,6 +227,8 @@ export async function teamRemoveUser(
   user: string,
   otp?: string,
 ): Promise<NpmExecResult> {
+  validateScopeTeam(scopeTeam)
+  validateUsername(user)
   return execNpm(['team', 'rm', scopeTeam, user], { otp })
 }
 
@@ -168,6 +238,8 @@ export async function accessGrant(
   pkg: string,
   otp?: string,
 ): Promise<NpmExecResult> {
+  validateScopeTeam(scopeTeam)
+  validatePackageName(pkg)
   return execNpm(['access', 'grant', permission, scopeTeam, pkg], { otp })
 }
 
@@ -176,31 +248,41 @@ export async function accessRevoke(
   pkg: string,
   otp?: string,
 ): Promise<NpmExecResult> {
+  validateScopeTeam(scopeTeam)
+  validatePackageName(pkg)
   return execNpm(['access', 'revoke', scopeTeam, pkg], { otp })
 }
 
 export async function ownerAdd(user: string, pkg: string, otp?: string): Promise<NpmExecResult> {
+  validateUsername(user)
+  validatePackageName(pkg)
   return execNpm(['owner', 'add', user, pkg], { otp })
 }
 
 export async function ownerRemove(user: string, pkg: string, otp?: string): Promise<NpmExecResult> {
+  validateUsername(user)
+  validatePackageName(pkg)
   return execNpm(['owner', 'rm', user, pkg], { otp })
 }
 
 // List functions (for reading data) - silent since they're not user-triggered operations
 
 export async function orgListUsers(org: string): Promise<NpmExecResult> {
+  validateOrgName(org)
   return execNpm(['org', 'ls', org, '--json'], { silent: true })
 }
 
 export async function teamListTeams(org: string): Promise<NpmExecResult> {
+  validateOrgName(org)
   return execNpm(['team', 'ls', org, '--json'], { silent: true })
 }
 
 export async function teamListUsers(scopeTeam: string): Promise<NpmExecResult> {
+  validateScopeTeam(scopeTeam)
   return execNpm(['team', 'ls', scopeTeam, '--json'], { silent: true })
 }
 
 export async function accessListCollaborators(pkg: string): Promise<NpmExecResult> {
+  validatePackageName(pkg)
   return execNpm(['access', 'list', 'collaborators', pkg, '--json'], { silent: true })
 }

pnpm-lock.yaml

@@ -0,0 +1,132 @@
+import { describe, expect, it } from 'vitest'
+import {
+  validateUsername,
+  validateOrgName,
+  validateScopeTeam,
+  validatePackageName,
+} from '../../cli/src/npm-client.ts'
+
+describe('validateUsername', () => {
+  it('accepts valid usernames', () => {
+    expect(() => validateUsername('alice')).not.toThrow()
+    expect(() => validateUsername('bob123')).not.toThrow()
+    expect(() => validateUsername('my-user')).not.toThrow()
+    expect(() => validateUsername('user-name-123')).not.toThrow()
+    expect(() => validateUsername('a')).not.toThrow()
+    expect(() => validateUsername('A1')).not.toThrow()
+  })
+
+  it('rejects empty or missing usernames', () => {
+    expect(() => validateUsername('')).toThrow('Invalid username')
+    expect(() => validateUsername(null as unknown as string)).toThrow('Invalid username')
+    expect(() => validateUsername(undefined as unknown as string)).toThrow('Invalid username')
+  })
+
+  it('rejects usernames that are too long', () => {
+    const longName = 'a'.repeat(51)
+    expect(() => validateUsername(longName)).toThrow('Invalid username')
+  })
+
+  it('rejects usernames with invalid characters', () => {
+    expect(() => validateUsername('user;rm -rf')).toThrow('Invalid username')
+    expect(() => validateUsername('user && evil')).toThrow('Invalid username')
+    expect(() => validateUsername('$(whoami)')).toThrow('Invalid username')
+    expect(() => validateUsername('user`id`')).toThrow('Invalid username')
+    expect(() => validateUsername('user|cat')).toThrow('Invalid username')
+    expect(() => validateUsername('user name')).toThrow('Invalid username')
+    expect(() => validateUsername('user.name')).toThrow('Invalid username')
+    expect(() => validateUsername('user_name')).toThrow('Invalid username')
+    expect(() => validateUsername('user@name')).toThrow('Invalid username')
+  })
+
+  it('rejects usernames starting or ending with hyphen', () => {
+    expect(() => validateUsername('-username')).toThrow('Invalid username')
+    expect(() => validateUsername('username-')).toThrow('Invalid username')
+    expect(() => validateUsername('-')).toThrow('Invalid username')
+  })
+})
+
+describe('validateOrgName', () => {
+  it('accepts valid org names', () => {
+    expect(() => validateOrgName('nuxt')).not.toThrow()
+    expect(() => validateOrgName('my-org')).not.toThrow()
+    expect(() => validateOrgName('org123')).not.toThrow()
+  })
+
+  it('rejects empty or missing org names', () => {
+    expect(() => validateOrgName('')).toThrow('Invalid org name')
+  })
+
+  it('rejects org names that are too long', () => {
+    const longName = 'a'.repeat(51)
+    expect(() => validateOrgName(longName)).toThrow('Invalid org name')
+  })
+
+  it('rejects org names with shell injection characters', () => {
+    expect(() => validateOrgName('org;rm -rf /')).toThrow('Invalid org name')
+    expect(() => validateOrgName('org && evil')).toThrow('Invalid org name')
+    expect(() => validateOrgName('$(whoami)')).toThrow('Invalid org name')
+  })
+})
+
+describe('validateScopeTeam', () => {
+  it('accepts valid scope:team format', () => {
+    expect(() => validateScopeTeam('@nuxt:developers')).not.toThrow()
+    expect(() => validateScopeTeam('@my-org:my-team')).not.toThrow()
+    expect(() => validateScopeTeam('@org123:team456')).not.toThrow()
+    expect(() => validateScopeTeam('@a:b')).not.toThrow()
+  })
+
+  it('rejects empty or missing scope:team', () => {
+    expect(() => validateScopeTeam('')).toThrow('Invalid scope:team')
+    expect(() => validateScopeTeam(null as unknown as string)).toThrow('Invalid scope:team')
+  })
+
+  it('rejects scope:team that is too long', () => {
+    const longScopeTeam = '@' + 'a'.repeat(50) + ':' + 'b'.repeat(50)
+    expect(() => validateScopeTeam(longScopeTeam)).toThrow('Invalid scope:team')
+  })
+
+  it('rejects invalid scope:team format', () => {
+    expect(() => validateScopeTeam('nuxt:developers')).toThrow('Invalid scope:team format')
+    expect(() => validateScopeTeam('@nuxt')).toThrow('Invalid scope:team format')
+    expect(() => validateScopeTeam('developers')).toThrow('Invalid scope:team format')
+    expect(() => validateScopeTeam('@:team')).toThrow('Invalid scope:team format')
+    expect(() => validateScopeTeam('@org:')).toThrow('Invalid scope:team format')
+  })
+
+  it('rejects scope:team with shell injection in scope', () => {
+    expect(() => validateScopeTeam('@org;rm:team')).toThrow('Invalid scope in scope:team')
+    expect(() => validateScopeTeam('@$(whoami):team')).toThrow('Invalid scope in scope:team')
+  })
+
+  it('rejects scope:team with shell injection in team', () => {
+    expect(() => validateScopeTeam('@org:team;rm')).toThrow('Invalid team name in scope:team')
+    expect(() => validateScopeTeam('@org:$(whoami)')).toThrow('Invalid team name in scope:team')
+  })
+
+  it('rejects scope or team starting/ending with hyphen', () => {
+    expect(() => validateScopeTeam('@-org:team')).toThrow('Invalid scope in scope:team')
+    expect(() => validateScopeTeam('@org-:team')).toThrow('Invalid scope in scope:team')
+    expect(() => validateScopeTeam('@org:-team')).toThrow('Invalid team name in scope:team')
+    expect(() => validateScopeTeam('@org:team-')).toThrow('Invalid team name in scope:team')
+  })
+})
+
+describe('validatePackageName', () => {
+  it('accepts valid package names', () => {
+    expect(() => validatePackageName('my-package')).not.toThrow()
+    expect(() => validatePackageName('@scope/package')).not.toThrow()
+    expect(() => validatePackageName('package123')).not.toThrow()
+  })
+
+  it('rejects package names with shell injection', () => {
+    expect(() => validatePackageName('pkg;rm -rf /')).toThrow('Invalid package name')
+    expect(() => validatePackageName('pkg && evil')).toThrow('Invalid package name')
+    expect(() => validatePackageName('$(whoami)')).toThrow('Invalid package name')
+  })
+
+  it('rejects empty package names', () => {
+    expect(() => validatePackageName('')).toThrow('Invalid package name')
+  })
+})