Merge pull request #56 from OpenForgeProject/fix-code-scanning

Fix code scanning issues

Author: Morgy93

.github/workflows/ci.yml

@@ -6,6 +6,10 @@ on:
   pull_request:
     branches: [ main, develop ]
 
+permissions:
+  contents: read
+  pull-requests: read
+
 jobs:
   test:
     strategy:

.github/workflows/code-quality.yml

@@ -6,6 +6,10 @@ on:
   pull_request:
     branches: [ main, develop ]
 
+permissions:
+  contents: read
+  pull-requests: read
+
 jobs:
   lint:
     runs-on: ubuntu-latest

.github/workflows/dependabot.yml

@@ -4,6 +4,12 @@ on:
   pull_request:
     types: [opened, synchronize]
 
+permissions:
+  pull-requests: write
+  contents: write
+  checks: read
+  statuses: read
+
 jobs:
   auto-approve:
     runs-on: ubuntu-latest

package.json

@@ -7,7 +7,7 @@
   "license": "GPL-3.0",
   "icon": "assets/icon.png",
   "engines": {
-    "vscode": "^1.100.0"
+    "vscode": "^1.108.0"
   },
   "categories": [
     "Linters",

src/services/phpstan-service.ts

@@ -55,32 +55,35 @@ export class PhpstanService extends BasePhpToolService {
     /**
      * Build PHPStan command with configuration
      */
-    protected buildToolCommand(filePath: string): string {
-        let command = 'phpstan analyze --error-format=json --no-progress';
+    protected buildToolCommand(filePath: string): string[] {
+        const command = ['phpstan', 'analyze', '--error-format=json', '--no-progress'];
 
         // Use config file if specified, otherwise use individual settings
         if (this.config.configPath) {
             // Use specified config file
-            command += ` -c "${this.config.configPath}"`;
+            command.push('-c', this.config.configPath);
         } else {
             // Try to auto-detect common PHPStan config files
             const autoConfigPath = this.detectPhpstanConfig();
             if (autoConfigPath) {
                 console.log(`PHPStan: Auto-detected config file: ${autoConfigPath}`);
-                command += ` -c "${autoConfigPath}"`;
+                command.push('-c', autoConfigPath);
             } else {
                 // Use individual settings when no config file is found
-                command += ` --level=${this.config.level}`;
+                command.push('--level', String(this.config.level));
 
                 // Add exclude paths
                 if (this.config.excludePaths && this.config.excludePaths.length > 0) {
                     for (const excludePath of this.config.excludePaths) {
-                        command += ` --exclude="${excludePath}"`;
+                        command.push('--exclude', excludePath);
                     }
                 }
             }
         }
 
+        // Add the file to analyze
+        command.push(filePath);
+
         return command;
     }
 

src/shared/services/base-php-tool-service.ts

@@ -65,7 +65,7 @@ export abstract class BasePhpToolService {
     /**
      * Build the command to execute the tool
      */
-    protected abstract buildToolCommand(relativePath: string): string;
+    protected abstract buildToolCommand(relativePath: string): string[];
 
     /**
      * Process the tool output and convert to diagnostics
@@ -193,7 +193,7 @@ export abstract class BasePhpToolService {
         try {
             // PHPStan returns exit code 1 when errors are found, which is normal
             const allowedExitCodes = this.toolName === 'phpstan' ? [0, 1] : [0];
-            console.log(`Executing ${this.displayName} command: ${toolCommand}`);
+            console.log(`Executing ${this.displayName} command: ${toolCommand.join(' ')}`);
 
             const output = DdevUtils.execDdev(toolCommand, workspaceFolder.uri.fsPath, allowedExitCodes);
             console.log(`${this.displayName} output length: ${output.length} characters`);

src/shared/utils/ddev-utils.ts

@@ -19,7 +19,17 @@
  */
 
 import * as vscode from 'vscode';
-import { execSync } from 'child_process';
+import * as cp from 'child_process';
+import * as fs from 'fs';
+import * as path from 'path';
+
+/**
+ * System calls wrapper for testability
+ */
+export const sys = {
+    spawnSync: cp.spawnSync,
+    existsSync: fs.existsSync
+};
 
 /**
  * DDEV project validation result
@@ -46,11 +56,8 @@ export class DdevUtils {
      */
     public static hasDdevProject(workspacePath: string): boolean {
         try {
-            const ddevConfig = execSync('test -f .ddev/config.yaml && echo "exists"', {
-                cwd: workspacePath,
-                encoding: 'utf-8'
-            });
-            return ddevConfig.includes('exists');
+            const configPath = path.join(workspacePath, '.ddev', 'config.yaml');
+            return sys.existsSync(configPath);
         } catch (error) {
             return false;
         }
@@ -64,11 +71,23 @@ export class DdevUtils {
      */
     public static isDdevRunning(workspacePath: string): boolean {
         try {
-            execSync('ddev exec echo "test"', {
+            const result = sys.spawnSync('ddev', ['exec', 'echo', 'test'], {
                 cwd: workspacePath,
-                stdio: 'ignore'
+                encoding: 'utf-8'
             });
-            return true;
+            if (result.error) {
+                vscode.window.showErrorMessage(
+                    'Failed to execute "ddev". Please ensure DDEV is installed and available in your PATH.',
+                );
+                return false;
+            }
+            if (result.status === null) {
+                vscode.window.showErrorMessage(
+                    'The "ddev" command did not exit normally. Please check your DDEV installation.',
+                );
+                return false;
+            }
+            return result.status === 0;
         } catch (error) {
             return false;
         }
@@ -83,7 +102,7 @@ export class DdevUtils {
      */
     public static isToolInstalled(toolName: string, workspacePath: string): boolean {
         try {
-            this.execDdev(`${toolName} --version`, workspacePath);
+            this.execDdev([toolName, '--version'], workspacePath);
             return true;
         } catch (error) {
             return false;
@@ -109,7 +128,7 @@ export class DdevUtils {
 
         // Try to run the tool
         try {
-            this.execDdev(`${toolName} --version`, workspacePath);
+            this.execDdev([toolName, '--version'], workspacePath);
 
             return {
                 isValid: true
@@ -174,37 +193,60 @@ export class DdevUtils {
     /**
      * Execute a command in the DDEV container
      *
-     * @param command Command to execute
+     * @param command Command to execute (as array of strings)
      * @param workspacePath Path to the workspace
      * @param allowedExitCodes Array of exit codes that should not throw (default: [0])
      * @returns Output of the command
      * @throws Error if the command fails with disallowed exit code
      */
-    public static execDdev(command: string, workspacePath: string, allowedExitCodes: number[] = [0]): string {
+    public static execDdev(command: string[], workspacePath: string, allowedExitCodes: number[] = [0]): string {
         try {
-            // Wrap command in bash -c to allow setting environment variables (specifically disabling Xdebug)
-            // This fixes issues where Xdebug causes the command to hang or run slowly
-            // We use single quotes for the bash command and escape any single quotes in the original command
-            const escapedCommand = command.replace(/'/g, "'\\''");
-            return execSync(`ddev exec bash -c 'XDEBUG_MODE=off ${escapedCommand}'`, {
+            // Use spawnSync to avoid shell injection and safely pass arguments
+            // We use 'env' to set environment variables inside the container
+            const args = ['exec', 'env', 'XDEBUG_MODE=off', ...command];
+
+            const result = sys.spawnSync('ddev', args, {
                 cwd: workspacePath,
                 encoding: 'utf-8'
             });
-        } catch (error: any) {
-            // Check if this is an acceptable exit code (e.g., PHPStan returns 1 when errors are found)
-            if (error.status && allowedExitCodes.includes(error.status)) {
+
+            if (result.error) {
+                throw result.error;
+            }
+
+            // Check if this is an acceptable exit code
+            if (result.status !== null && allowedExitCodes.includes(result.status)) {
                 // Return stdout even if exit code is non-zero but allowed
-                console.log(`Command exited with allowed code ${error.status}: ${command}`);
-                return error.stdout || '';
+                if (result.status !== 0) {
+                    console.log(`Command exited with allowed code ${result.status}: ${command.join(' ')}`);
+                }
+                return result.stdout || '';
+            }
+
+            if (result.status !== 0) {
+                // Enhance error message with more details
+                const enhancedError = new Error(result.stderr || 'Command execution failed');
+                enhancedError.name = 'CommandError';
+                (enhancedError as any).status = result.status;
+                (enhancedError as any).stderr = result.stderr;
+                (enhancedError as any).stdout = result.stdout;
+                (enhancedError as any).command = `ddev exec ${command.join(' ')}`;
+                (enhancedError as any).workspacePath = workspacePath;
+                throw enhancedError;
+            }
+        } catch (error: any) {
+            // If error was already thrown above, rethrow it
+            if (error.name === 'CommandError') {
+                throw error;
             }
 
-            // Enhance error message with more details
+            // Handle unexpected errors
             const enhancedError = new Error(error.message || 'Command execution failed');
             enhancedError.name = error.name || 'CommandError';
             (enhancedError as any).status = error.status;
             (enhancedError as any).stderr = error.stderr;
             (enhancedError as any).stdout = error.stdout;
-            (enhancedError as any).command = `ddev exec ${command}`;
+            (enhancedError as any).command = `ddev exec ${command.join(' ')}`;
             (enhancedError as any).workspacePath = workspacePath;
 
             throw enhancedError;