Merge pull request #286 from shivasurya/shiva/augment-security-review

feat: add directory listing tool and iterative file navigation for security analysis

Author: shivasurya

extension/secureflow/packages/secureflow-cli/lib/prompts/common/security-review-cli.txt

@@ -58,6 +58,9 @@ You are a highly experienced security engineer conducting a thorough code review
    - It may include README files
    - You may exclude other files
 
+6. ALWAYS use tools to navigate through the code to understand the complete context of the vulnerability.
+   - Even though the project structure is provided, use list files tool to navigate through the code to understand the complete context of the vulnerability.
+   - Use file request tool to request for files if needed for complete analysis iteratively.
 
 Strictly don't report below category of vulnerabilities from Analysis:
    - Theoretical vulnerabilities without practical impact
@@ -80,6 +83,17 @@ Strictly don't report below category of vulnerabilities from Analysis:
    - Client side DoS attacks
    - Assuming backend is malicious
 
+### How does a security engineer work?
+
+A security engineer works in an iterative manner. He first reviews the project information provided to him and 
+then requests for files to review. He then reviews the files and requests for additional files if needed for 
+complete analysis. If he finds a vulnerable pattern, he request for related files to review to understand the 
+complete context of the vulnerability. Mainly he validates the flow of the code to navigate through the code 
+across files and directories to understand the complete context of the vulnerability.
+
+Similar to a security engineer, you should also work in an iterative manner and you'll receive tools as request 
+to navigate through the code to understand the complete context of the vulnerability.
+
 ### Output
 
 Provide actionable, practical feedback that helps developers understand and fix real security issues while maintaining code quality and performance.

extension/secureflow/packages/secureflow-cli/lib/prompts/scanner/final-analysis.txt

@@ -1 +1 @@
-Please provide your final security analysis. Do not request any more files.
+Please provide your final security analysis. Do not request any more files or list of files.

extension/secureflow/packages/secureflow-cli/lib/prompts/scanner/iterative-analysis.txt

@@ -1,6 +1,9 @@
 Remember to be skeptical like a senior security engineer and make sure to read all source files. If the file 
-doesnt exist, strictly dont request it again and wait for the next iteration to request it.
+doesnt exist, strictly dont request it again and wait for the next iteration to request it. Be sure to list 
+all files under project directory without missing any path, because it helps to reduce blind spots in analysis.
 
 Based on the files analyzed so far, strictly follow either and don't ask any other questions:
 1. Request additional files if needed for complete analysis in format specified in the file request tag
-2. Provide your final security analysis if you have sufficient information
+2. Request additional list of files if needed for complete analysis in format specified in the list file request tag
+3. Provide your final security analysis if you have sufficient information
+4. If you haven't requested for list of files so far, please provide rational reason for not requesting it.

extension/secureflow/packages/secureflow-cli/lib/prompts/tools/file-request-instructions.txt

@@ -1,4 +1,5 @@
-Tool: read file
+------
+1. Tool: read file
 Description: This tool helps to read files for analysis
 Format:
 <file_request path="./relative/path/to/file.js" reason="short reason for requesting this file" />
@@ -16,3 +17,4 @@ Rules for file requests:
 10. If the file is not a source file, dont request it again
 11. You should request only files `Files in the project:` section. Files apart from this section are strictly prohibited.
 12. If you have already requested a file and read it, dont request it again
+------

extension/secureflow/packages/secureflow-cli/lib/prompts/tools/list-file-request-instructions.txt

@@ -1,4 +1,5 @@
-Tool: list files in directory
+------
+2. Tool: list files in directory (equivalent to 'ls' command)
 Description: This method helps to list files in a directory
 Format:
 <list_file_request path="./relative/path/to/directory" reason="short reason for requesting this directory" />
@@ -13,3 +14,4 @@ Rules for file requests:
 7. Skip directories that were not found previously
 8. Skip non-source directories
 9. Follow project structure strictly
+----

extension/secureflow/packages/secureflow-cli/scanner/ai-security-analyzer.js

@@ -25,6 +25,7 @@ class AISecurityAnalyzer {
     let iteration = 0;
     let currentContext = await this._buildInitialContext(profileInfo, projectSummary, reviewPrompt);
     let allFileContents = [];
+    let fileRequestsHistory = [];
     let finalAnalysis = null;
     let messages = [{ role: 'user', content: currentContext }];
 
@@ -34,7 +35,6 @@ class AISecurityAnalyzer {
 
       // Send context to AI and get response
       const aiResponse = await this._sendToAI(null, iteration, messages);
-
       messages.push({ role: 'assistant', content: aiResponse });
 
       this.analysisLog.push({
@@ -46,10 +46,25 @@ class AISecurityAnalyzer {
 
       // Check if AI wants to request files
       const fileRequests = this._extractFileRequests(aiResponse);
+      const newFileRequests = fileRequests.filter(request => !fileRequestsHistory.some(f => f.path === request.path));
+      fileRequestsHistory.push(...newFileRequests);
+
+      // Check if AI wants to list files
+      const listFileRequests = this._extractListFileRequests(aiResponse);
 
-      if (fileRequests.length > 0) {
-        // Process file requests
-        const fileResults = await this.fileHandler.processFileRequests(aiResponse);
+      if (fileRequests.length > 0 || listFileRequests.length > 0) {
+        let fileResults = [];
+        let listResults = [];
+        
+        // Process file requests first
+        if (fileRequests.length > 0) {
+          fileResults = await this.fileHandler.processFileRequests(aiResponse);
+        }
+        
+        // Process list file requests separately
+        if (listFileRequests.length > 0) {
+          listResults = await this.fileHandler.processListFileRequests(aiResponse);
+        }
 
         // Add successful file contents to context, avoiding duplicates
         const newFileContents = fileResults
@@ -63,17 +78,19 @@ class AISecurityAnalyzer {
 
         allFileContents.push(...newFileContents);
 
-        // Build context for next iteration
+        // Build context for next iteration with both file contents and directory listings
         currentContext = await this._buildIterativeContext(
           newFileContents,
           fileResults,
-          iteration
+          listResults,
+          iteration,
+          fileRequestsHistory,
+          projectSummary
         );
         messages.push({ role: 'user', content: currentContext });
 
       } else {
-        // No more file requests, this should be the final analysis
-        console.log(green('✅ AI completed analysis without additional file requests'));
+        // No more file or list requests, this should be the final analysis
         finalAnalysis = aiResponse;
         break;
       }
@@ -116,7 +133,7 @@ class AISecurityAnalyzer {
     context += `Project Path: ${projectSummary.projectPath}\n`;
     context += `Total Files: ${projectSummary.totalFiles}\n\n`;
 
-    context += `Files in the project:\n${projectSummary.directoryStructure}\n`;
+    // context += `Files in the project:\n${projectSummary.directoryStructure}\n`;
 
     context += `Files by Extension:\n`;
     Object.entries(projectSummary.filesByExtension).forEach(([ext, files]) => {
@@ -139,31 +156,74 @@ class AISecurityAnalyzer {
   /**
    * Build context for iterative requests
    */
-  async _buildIterativeContext(fileContents, fileResults, iteration) {
+  async _buildIterativeContext(fileContents, fileResults, listResults, iteration, fileRequestsHistory, projectSummary) {
     let context = "\n";
 
     // Add file contents from previous requests
-    context += `[ANALYZED FILES - Iteration ${iteration}]\n`;
-    fileContents.forEach(file => {
-      context += `\n=== FILE: ${file.path} ===\n`;
-      context += `Reason: ${file.reason}\n`;
-      if (file.reason === 'duplicate') {
-        context += `[File was already analyzed in previous conversation]\n`;
-      } else {
-        context += `${file.content}\n`;
-      }
-      context += `=== END FILE: ${file.path} ===\n`;
-    });
+    if (fileContents.length > 0) {
+      context += `[ANALYZED FILES - Iteration ${iteration}]\n`;
+      fileContents.forEach(file => {
+        context += `\n=== FILE: ${file.path} ===\n`;
+        context += `Reason: ${file.reason}\n`;
+        if (file.reason === 'duplicate') {
+          context += `[File was already analyzed in previous conversation]\n`;
+        } else {
+          context += `${file.content}\n`;
+        }
+        context += `=== END FILE: ${file.path} ===\n`;
+      });
+    }
+
+    // Add directory listings from previous requests
+    if (listResults && listResults.length > 0) {
+      context += `\n[DIRECTORY LISTINGS - Iteration ${iteration}]\n`;
+      listResults.forEach(result => {
+        if (result.status === 'success') {
+          context += `\n=== DIRECTORY: ${result.path} ===\n`;
+          context += `Reason: ${result.reason}\n`;
+          context += `Files and directories found (${result.fileCount} items):\n`;
+          result.files.forEach(file => {
+            const icon = file.type === 'directory' ? '📁' : '📄';
+            context += `${icon} ${file.name} (${file.type})\n`;
+          });
+          context += `=== END DIRECTORY: ${result.path} ===\n`;
+        } else {
+          context += `❌ Directory listing failed for ${result.path}: ${result.reason}\n`;
+        }
+      });
+    }
 
     // Add file request results summary
-    context += `\n[FILE REQUEST RESULTS]\n`;
-    fileResults.forEach(result => {
-      if (result.status === 'success') {
-        context += `✅ ${result.path}: Successfully read (${result.lines} lines)\n`;
-      } else {
-        context += `❌ This file doesn't exist or is not a source file ${result.path}: ${result.reason}. Don't request it again\n`;
-      }
-    });
+    if (fileResults.length > 0) {
+      context += `\n[FILE REQUEST RESULTS]\n`;
+      fileResults.forEach(result => {
+        if (result.status === 'success') {
+          context += `✅ ${result.path}: Successfully read (${result.lines} lines)\n`;
+        } else {
+          context += `❌ This file doesn't exist or is not a source file ${result.path}: ${result.reason}. Don't request it again\n`;
+        }
+      });
+    }
+
+    // Add list request results summary
+    if (listResults && listResults.length > 0) {
+      context += `\n[DIRECTORY LISTING RESULTS]\n`;
+      listResults.forEach(result => {
+        if (result.status === 'success') {
+          context += `✅ ${result.path}: Successfully listed (${result.fileCount} items)\n`;
+        } else {
+          context += `❌ Directory listing failed for ${result.path}: ${result.reason}. Don't request it again\n`;
+        }
+      });
+    }
+
+    // Add file request history
+    if (fileRequestsHistory.length > 0) {
+      context += `\n[FILE REQUEST HISTORY]\n`;
+      context += `So far, you have requested ${fileRequestsHistory.length} files\n`;
+      context += `Project has ${projectSummary.totalFiles} files\n`;
+      context += `\n`;
+    }
 
     // Add instructions for next step
     if (iteration < this.maxIterations) {
@@ -198,17 +258,35 @@ class AISecurityAnalyzer {
     return requests;
   }
 
+  /**
+   * Extract list file requests from AI response
+   */
+  _extractListFileRequests(response) {
+    const listFileRequestRegex = /<list_file_request\s+path="([^"]+)"(?:\s+reason="([^"]*)")?\s*\/>/g;
+    const requests = [];
+    
+    let match;
+    while ((match = listFileRequestRegex.exec(response)) !== null) {
+      requests.push({
+        path: match[1],
+        reason: match[2] || 'No reason provided'
+      });
+    }
+
+    return requests;
+  }
+
   /**
    * Send context to AI and get response
    */
   async _sendToAI(context, iteration, messages) {
     //console.log(dim(`📤 Sending context to AI (${context.length} characters)`));
 
     // Display session state before the call
-    if (this.tokenTracker) {
-      const preCallData = this.tokenTracker.getPreCallUsageData(iteration);
-      TokenDisplay.displayPreCallUsage(preCallData);
-    }
+    // if (this.tokenTracker) {
+    //   const preCallData = this.tokenTracker.getPreCallUsageData(iteration);
+    //   TokenDisplay.displayPreCallUsage(preCallData);
+    // }
 
     try {
       const response = await this.aiClient.analyze(context, messages);

extension/secureflow/packages/secureflow-cli/scanner/cli-full-scan-command.js

@@ -339,7 +339,6 @@ class CLIFullScanCommand {
    * Output DefectDojo summary for user convenience
    */
   _outputDefectDojoSummary(scanResult, defectDojoFindings) {    
-    console.log(`🤖 Model: ${scanResult.model}`);
     console.log(`📊 Files: ${scanResult.filesAnalyzed}/${scanResult.totalFiles} analyzed`);
 
     const summaryCounts = {