Use shell-quote to sanitize CDS extractor args

Author: data-douser

extractors/cds/tools/dist/cds-extractor.bundle.js

@@ -2,51 +2,54 @@ import { statSync } from 'fs';
 
 import { build as esbuildFunc } from 'esbuild';
 
+const NODE_VERSION_TARGET = 'node18';
+
 const buildOptions = {
-  entryPoints: ['cds-extractor.ts'],
+  banner: {
+    js: '#!/usr/bin/env node',
+  },
   bundle: true,
-  platform: 'node',
-  target: 'node18',
-  outfile: 'dist/cds-extractor.bundle.js',
+  conditions: ['node'],
+  entryPoints: ['cds-extractor.ts'],
   external: [
     // Node.js built-in modules
-    'fs',
-    'path',
-    'os',
+    'assert',
+    'buffer',
     'child_process',
-    'util',
-    'events',
-    'stream',
-    'url',
     'crypto',
-    'process',
-    'buffer',
-    'assert',
-    'module',
-    'net',
-    'tls',
+    'events',
+    'fs',
     'http',
     'https',
-    'zlib',
+    'module',
+    'net',
+    'os',
+    'path',
+    'process',
     'readline',
+    'shell-quote',
+    'stream',
+    'tls',
+    'url',
+    'util',
     'worker_threads',
+    'zlib',
   ],
-  minify: true,
-  sourcemap: true,
   format: 'cjs',
-  banner: {
-    js: '#!/usr/bin/env node',
-  },
-  logLevel: 'info',
   // Handle TypeScript files
   loader: {
     '.ts': 'ts',
   },
-  // Resolve TypeScript paths
-  resolveExtensions: ['.ts', '.js'],
+  logLevel: 'info',
   // Ensure proper module resolution
   mainFields: ['main', 'module'],
-  conditions: ['node'],
+  minify: true,
+  outfile: 'dist/cds-extractor.bundle.js',
+  platform: 'node',
+  // Resolve TypeScript paths
+  resolveExtensions: ['.ts'],
+  sourcemap: true,
+  target: NODE_VERSION_TARGET,
 };
 
 async function build() {

extractors/cds/tools/dist/cds-extractor.bundle.js.map

@@ -1,5 +1,7 @@
 import { execSync } from 'child_process';
 
+import { quote } from 'shell-quote';
+
 import type { SemanticVersion } from './types';
 import { cdsExtractorLog } from '../logging';
 
@@ -140,7 +142,9 @@ export function getAvailableVersions(packageName: string): string[] {
   // Cache miss - fetch from npm
   cacheStats.misses++;
   try {
-    const output = execSync(`npm view ${packageName} versions --json`, {
+    // Use shell-quote to properly escape the package name to prevent injection
+    const escapedPackageName = quote([packageName]);
+    const output = execSync(`npm view ${escapedPackageName} versions --json`, {
       encoding: 'utf8',
       timeout: 30000, // 30 second timeout
     });

extractors/cds/tools/esbuild.config.mjs

@@ -1,9 +1,38 @@
+import { resolve } from 'path';
+
 const USAGE_MESSAGE = `\tUsage: node <script> <source-root>`;
 
+/**
+ * Resolves and validates a source root directory path.
+ *
+ * This function takes a source root path, validates it, normalizes it,
+ * and returns an absolute path to the directory.
+ *
+ * @param sourceRoot - The source root path to resolve
+ * @returns The normalized absolute path to the source root directory
+ * @throws {Error} If the source root is null, undefined, an empty string,
+ *                 or does not point to a valid directory
+ */
+function resolveSourceRoot(sourceRoot: string): string {
+  // Check for null, undefined, or empty string
+  if (!sourceRoot || typeof sourceRoot !== 'string') {
+    throw new Error('Source root must be a non-empty string');
+  }
+
+  // Normalize the path and resolve it to an absolute path.
+  const normalizedPath = resolve(sourceRoot);
+
+  // Check if the resolved path points to a valid, existing directory.
+  if (!normalizedPath || normalizedPath === '/') {
+    throw new Error('Source root must point to a valid directory');
+  }
+
+  return normalizedPath;
+}
+
 /**
  * Check if the script was invoked with the required arguments.
  * This function validates and sanitizes script arguments and returns them if valid.
- * The CDS extractor now runs in autobuild mode by default.
  *
  * Requirements:
  * - Only requires: <source-root>
@@ -28,7 +57,18 @@ export function validateArguments(args: string[]): {
   }
 
   // Get the source root from args (now the first parameter after script name)
-  const sourceRoot: string = args[2];
+  const rawSourceRoot: string = args[2];
+
+  // Validate and sanitize the source root path
+  let sourceRoot: string;
+  try {
+    sourceRoot = resolveSourceRoot(rawSourceRoot);
+  } catch (error) {
+    return {
+      isValid: false,
+      usageMessage: `Invalid source root: ${String(error)}`,
+    };
+  }
 
   // Return the validated arguments
   return {

extractors/cds/tools/src/packageManager/versionResolver.ts

@@ -1,5 +1,14 @@
+import { resolve } from 'path';
+
 import { validateArguments } from '../../src/utils';
 
+// Mock path module
+jest.mock('path', () => ({
+  resolve: jest.fn(),
+}));
+
+const mockedResolve = resolve as jest.MockedFunction<typeof resolve>;
+
 const EXTRACTOR_SCRIPT_NAME = 'cds-extractor.js';
 
 describe('utils', () => {
@@ -9,18 +18,23 @@ describe('utils', () => {
     beforeEach(() => {
       // Mock console.warn to avoid polluting test output
       console.warn = jest.fn();
+      // Set up default mock behavior
+      mockedResolve.mockImplementation((path: string) =>
+        path.startsWith('/') ? path : `/absolute/${path}`,
+      );
     });
 
     afterEach(() => {
       console.warn = originalConsoleWarn;
+      jest.clearAllMocks();
     });
 
     it('should validate when source root is provided', () => {
       const args = ['node', EXTRACTOR_SCRIPT_NAME, 'source-root'];
       const result = validateArguments(args);
       expect(result.isValid).toBe(true);
       expect(result.args).toEqual({
-        sourceRoot: 'source-root',
+        sourceRoot: '/absolute/source-root',
       });
     });
 
@@ -43,8 +57,57 @@ describe('utils', () => {
       const result = validateArguments(args);
       expect(result.isValid).toBe(true);
       expect(result.args).toEqual({
-        sourceRoot: 'source-root',
+        sourceRoot: '/absolute/source-root',
+      });
+    });
+
+    it('should handle source root with special characters', () => {
+      const specialInputs = [
+        'source-root;rm -rf /',
+        'source-root|cat /etc/passwd',
+        'source-root`whoami`',
+        'source-root$(whoami)',
+        'source-root{test}',
+        'source-root<test>',
+        'source-root*test',
+        'source-root?test',
+        'source-root~test',
+      ];
+
+      specialInputs.forEach(input => {
+        const args = ['node', EXTRACTOR_SCRIPT_NAME, input];
+        const result = validateArguments(args);
+        expect(result.isValid).toBe(true);
+        expect(result.args).toEqual({
+          sourceRoot: `/absolute/${input}`,
+        });
+      });
+    });
+
+    it('should handle source root with null bytes', () => {
+      const args = ['node', EXTRACTOR_SCRIPT_NAME, 'source-root\0test'];
+      const result = validateArguments(args);
+      expect(result.isValid).toBe(true);
+      expect(result.args).toEqual({
+        sourceRoot: '/absolute/source-root\0test',
       });
     });
+
+    it('should reject empty or non-string source root', () => {
+      const args = ['node', EXTRACTOR_SCRIPT_NAME, ''];
+      const result = validateArguments(args);
+      expect(result.isValid).toBe(false);
+      expect(result.usageMessage).toContain('non-empty string');
+    });
+
+    it('should reject source root that resolves to root directory', () => {
+      // Mock path.resolve to return '/' to test the edge case
+      mockedResolve.mockReturnValueOnce('/');
+
+      const args = ['node', EXTRACTOR_SCRIPT_NAME, 'some-path'];
+      const result = validateArguments(args);
+      expect(result.isValid).toBe(false);
+      expect(result.usageMessage).toContain('valid directory');
+    });
   });
 });

extractors/cds/tools/src/utils.ts

@@ -4,9 +4,12 @@ const fs = require('fs');
 const os = require('os');
 const path = require('path');
 
+const DEFAULT_TIMEOUT_MS = 5000; // Default timeout for execution in milliseconds
+const MAX_BUNDLE_SIZE_MB = 50; // Set a reasonable max bundle size
+
 const bundlePath = path.join(__dirname, 'dist', 'cds-extractor.bundle.js');
 
-console.log('🔍 Validating CDS extractor bundle...');
+console.log('🔍 Validating CDS extractor bundle at', bundlePath);
 
 // Check bundle exists
 if (!fs.existsSync(bundlePath)) {
@@ -21,7 +24,7 @@ const stats = fs.statSync(bundlePath);
 const sizeInMB = stats.size / (1024 * 1024);
 console.log(`📦 Bundle size: ${sizeInMB.toFixed(2)} MB`);
 
-if (sizeInMB > 50) {
+if (sizeInMB > MAX_BUNDLE_SIZE_MB) {
   console.warn('⚠️  Bundle size is quite large, consider optimizing');
 }
 
@@ -51,7 +54,7 @@ try {
     execSync(nodeCmd, {
       stdio: 'pipe',
       cwd: testDir,
-      timeout: 5000, // 5 second timeout
+      timeout: DEFAULT_TIMEOUT_MS,
       encoding: 'utf8',
     });
     console.log('✅ Bundle execution completed successfully');