[UPDATE PRIMITIVE] Fix path traversal check, add prompt fixture runner, fix comment mismatch (#155)

Author: Copilot

client/integration-tests/primitives/prompts/explain_codeql_query/invalid_query_path/README.md

@@ -15,7 +15,8 @@ when the user provides a `queryPath` that does not exist on disk.
 
 ## Parameters
 
-| Parameter   | Value                          | Notes              |
-| ----------- | ------------------------------ | ------------------ |
-| `queryPath` | `nonexistent/path/to/query.ql` | Does **not** exist |
-| `language`  | `javascript`                   | Valid language     |
+| Parameter      | Value                          | Notes              |
+| -------------- | ------------------------------ | ------------------ |
+| `databasePath` | `nonexistent/path/to/database` | Does **not** exist |
+| `queryPath`    | `nonexistent/path/to/query.ql` | Does **not** exist |
+| `language`     | `javascript`                   | Valid language     |

client/integration-tests/primitives/prompts/explain_codeql_query/invalid_query_path/before/monitoring-state.json

@@ -1,3 +1,8 @@
 {
-  "sessions": []
+  "sessions": [],
+  "parameters": {
+    "databasePath": "nonexistent/path/to/database",
+    "queryPath": "nonexistent/path/to/query.ql",
+    "language": "javascript"
+  }
 }

client/src/lib/integration-test-runner.js

@@ -229,6 +229,9 @@ export class IntegrationTestRunner {
       // Also run workflow integration tests
       await this.runWorkflowIntegrationTests(baseDir);
 
+      // Also run prompt integration tests
+      await this.runPromptIntegrationTests(baseDir);
+
       return totalIntegrationTests > 0;
     } catch (error) {
       this.logger.log(`Error running integration tests: ${error.message}`, "ERROR");
@@ -1095,6 +1098,141 @@ export class IntegrationTestRunner {
     return params;
   }
 
+  /**
+   * Run prompt-level integration tests.
+   * Discovers test fixtures under `integration-tests/primitives/prompts/`
+   * and calls `client.getPrompt()` for each, validating the response.
+   */
+  async runPromptIntegrationTests(baseDir) {
+    try {
+      this.logger.log("Discovering and running prompt integration tests...");
+
+      const promptTestsDir = path.join(baseDir, "..", "integration-tests", "primitives", "prompts");
+
+      if (!fs.existsSync(promptTestsDir)) {
+        this.logger.log("No prompt integration tests directory found", "INFO");
+        return true;
+      }
+
+      // Get list of available prompts from the server
+      const response = await this.client.listPrompts();
+      const prompts = response.prompts || [];
+      const promptNames = prompts.map((p) => p.name);
+
+      this.logger.log(`Found ${promptNames.length} prompts available on server`);
+
+      // Discover prompt test directories (each subdirectory = one prompt name)
+      const promptDirs = fs
+        .readdirSync(promptTestsDir, { withFileTypes: true })
+        .filter((dirent) => dirent.isDirectory())
+        .map((dirent) => dirent.name);
+
+      this.logger.log(
+        `Found ${promptDirs.length} prompt test directories: ${promptDirs.join(", ")}`
+      );
+
+      let totalPromptTests = 0;
+      for (const promptName of promptDirs) {
+        if (!promptNames.includes(promptName)) {
+          this.logger.log(`Skipping ${promptName} - prompt not found on server`, "WARN");
+          continue;
+        }
+
+        const promptDir = path.join(promptTestsDir, promptName);
+        const testCases = fs
+          .readdirSync(promptDir, { withFileTypes: true })
+          .filter((dirent) => dirent.isDirectory())
+          .map((dirent) => dirent.name);
+
+        this.logger.log(`Running ${testCases.length} test case(s) for prompt ${promptName}`);
+
+        for (const testCase of testCases) {
+          await this.runSinglePromptIntegrationTest(promptName, testCase, promptDir);
+          totalPromptTests++;
+        }
+      }
+
+      this.logger.log(`Total prompt integration tests executed: ${totalPromptTests}`);
+      return true;
+    } catch (error) {
+      this.logger.log(`Error running prompt integration tests: ${error.message}`, "ERROR");
+      return false;
+    }
+  }
+
+  /**
+   * Run a single prompt integration test.
+   *
+   * Reads parameters from `before/monitoring-state.json`, calls `getPrompt()`,
+   * and validates that the response contains messages (no protocol-level error).
+   */
+  async runSinglePromptIntegrationTest(promptName, testCase, promptDir) {
+    const testName = `prompt:${promptName}/${testCase}`;
+    this.logger.log(`\nRunning prompt integration test: ${testName}`);
+
+    try {
+      const testCaseDir = path.join(promptDir, testCase);
+      const beforeDir = path.join(testCaseDir, "before");
+      const afterDir = path.join(testCaseDir, "after");
+
+      // Validate test structure
+      if (!fs.existsSync(beforeDir)) {
+        this.logger.logTest(testName, false, "Missing before directory");
+        return;
+      }
+
+      if (!fs.existsSync(afterDir)) {
+        this.logger.logTest(testName, false, "Missing after directory");
+        return;
+      }
+
+      // Load parameters from before/monitoring-state.json
+      const monitoringStatePath = path.join(beforeDir, "monitoring-state.json");
+      if (!fs.existsSync(monitoringStatePath)) {
+        this.logger.logTest(testName, false, "Missing before/monitoring-state.json");
+        return;
+      }
+
+      const monitoringState = JSON.parse(fs.readFileSync(monitoringStatePath, "utf8"));
+      const params = monitoringState.parameters || {};
+      resolvePathPlaceholders(params, this.logger);
+
+      // Call the prompt
+      this.logger.log(`Calling prompt ${promptName} with params: ${JSON.stringify(params)}`);
+
+      const result = await this.client.getPrompt({
+        name: promptName,
+        arguments: params
+      });
+
+      // Validate that the response contains messages (no raw protocol error)
+      const hasMessages = result.messages && result.messages.length > 0;
+      if (!hasMessages) {
+        this.logger.logTest(testName, false, "Expected messages in prompt response");
+        return;
+      }
+
+      // If the after/monitoring-state.json has expected content checks, validate
+      const afterMonitoringPath = path.join(afterDir, "monitoring-state.json");
+      if (fs.existsSync(afterMonitoringPath)) {
+        const afterState = JSON.parse(fs.readFileSync(afterMonitoringPath, "utf8"));
+        if (afterState.expectedContentPatterns) {
+          const text = result.messages[0]?.content?.text || "";
+          for (const pattern of afterState.expectedContentPatterns) {
+            if (!text.includes(pattern)) {
+              this.logger.logTest(testName, false, `Expected response to contain "${pattern}"`);
+              return;
+            }
+          }
+        }
+      }
+
+      this.logger.logTest(testName, true, `Prompt returned ${result.messages.length} message(s)`);
+    } catch (error) {
+      this.logger.logTest(testName, false, `Error: ${error.message}`);
+    }
+  }
+
   /**
    * Run workflow-level integration tests
    * These tests validate complete workflows rather than individual tools

extensions/vscode/test/suite/mcp-prompt-e2e.integration.test.ts

@@ -300,7 +300,7 @@ suite('MCP Prompt Error Handling Integration Tests', () => {
   });
 
   // ─────────────────────────────────────────────────────────────────────
-  // sarif_rank_false_positives — optional sarifPath handling
+  // sarif_rank_false_positives — sarifPath validation
   // ─────────────────────────────────────────────────────────────────────
 
   test('sarif_rank_false_positives with nonexistent sarifPath should return warning', async function () {

server/dist/codeql-development-mcp-server.js

@@ -34972,7 +34972,7 @@ var require_send = __commonJS({
     var join19 = path4.join;
     var normalize2 = path4.normalize;
     var resolve15 = path4.resolve;
-    var sep2 = path4.sep;
+    var sep3 = path4.sep;
     var BYTES_RANGE_REGEXP = /^ *bytes=/;
     var MAX_MAXAGE = 60 * 60 * 24 * 365 * 1e3;
     var UP_PATH_REGEXP = /(?:^|[\\/])\.\.(?:[\\/]|$)/;
@@ -35133,22 +35133,22 @@ var require_send = __commonJS({
       var parts;
       if (root !== null) {
         if (path5) {
-          path5 = normalize2("." + sep2 + path5);
+          path5 = normalize2("." + sep3 + path5);
         }
         if (UP_PATH_REGEXP.test(path5)) {
           debug('malicious path "%s"', path5);
           this.error(403);
           return res;
         }
-        parts = path5.split(sep2);
+        parts = path5.split(sep3);
         path5 = normalize2(join19(root, path5));
       } else {
         if (UP_PATH_REGEXP.test(path5)) {
           debug('malicious path "%s"', path5);
           this.error(403);
           return res;
         }
-        parts = normalize2(path5).split(sep2);
+        parts = normalize2(path5).split(sep3);
         path5 = resolve15(path5);
       }
       if (containsDotFile(parts)) {
@@ -35242,7 +35242,7 @@ var require_send = __commonJS({
       var self = this;
       debug('stat "%s"', path5);
       fs3.stat(path5, function onstat(err, stat) {
-        var pathEndsWithSep = path5[path5.length - 1] === sep2;
+        var pathEndsWithSep = path5[path5.length - 1] === sep3;
         if (err && err.code === "ENOENT" && !extname3(path5) && !pathEndsWithSep) {
           return next(err);
         }
@@ -40549,8 +40549,8 @@ var require_adm_zip = __commonJS({
         return null;
       }
       function fixPath(zipPath) {
-        const { join: join19, normalize: normalize2, sep: sep2 } = pth.posix;
-        return join19(".", normalize2(sep2 + zipPath.split("\\").join(sep2) + sep2));
+        const { join: join19, normalize: normalize2, sep: sep3 } = pth.posix;
+        return join19(".", normalize2(sep3 + zipPath.split("\\").join(sep3) + sep3));
       }
       function filenameFilter(filterfn) {
         if (filterfn instanceof RegExp) {
@@ -64378,7 +64378,7 @@ function registerLanguageResources(server) {
 }
 
 // src/prompts/workflow-prompts.ts
-import { basename as basename7, isAbsolute as isAbsolute7, normalize, relative, resolve as resolve13 } from "path";
+import { basename as basename7, isAbsolute as isAbsolute7, normalize, relative, resolve as resolve13, sep as sep2 } from "path";
 import { existsSync as existsSync12 } from "fs";
 
 // src/prompts/check-for-duplicated-code.prompt.md
@@ -64478,7 +64478,7 @@ function resolvePromptFilePath(filePath, workspaceRoot) {
   const normalizedPath = normalize(filePath);
   const absolutePath = isAbsolute7(normalizedPath) ? normalizedPath : resolve13(effectiveRoot, normalizedPath);
   const rel = relative(effectiveRoot, absolutePath);
-  if (rel.startsWith("..") || isAbsolute7(rel)) {
+  if (rel === ".." || rel.startsWith(`..${sep2}`) || isAbsolute7(rel)) {
     return {
       resolvedPath: absolutePath,
       warning: `\u26A0 **File path** \`${filePath}\` **resolves outside the workspace root.** Resolved to: \`${absolutePath}\``

server/dist/codeql-development-mcp-server.js.map

@@ -7,7 +7,7 @@
 
 import { McpServer } from '@modelcontextprotocol/sdk/server/mcp.js';
 import { z } from 'zod';
-import { basename, isAbsolute, normalize, relative, resolve } from 'path';
+import { basename, isAbsolute, normalize, relative, resolve, sep } from 'path';
 import { existsSync } from 'fs';
 import { loadPromptTemplate, processPromptTemplate } from './prompt-loader';
 import { getUserWorkspaceDir } from '../utils/package-paths';
@@ -79,7 +79,7 @@ export function resolvePromptFilePath(
   // This catches path traversal (e.g. "../../etc/passwd") after full
   // resolution rather than relying on a fragile substring check.
   const rel = relative(effectiveRoot, absolutePath);
-  if (rel.startsWith('..') || isAbsolute(rel)) {
+  if (rel === '..' || rel.startsWith(`..${sep}`) || isAbsolute(rel)) {
     return {
       resolvedPath: absolutePath,
       warning: `⚠ **File path** \`${filePath}\` **resolves outside the workspace root.** Resolved to: \`${absolutePath}\``,

server/src/prompts/workflow-prompts.ts

@@ -1734,6 +1734,15 @@ describe('Workflow Prompts', () => {
       expect(result.warning).toContain('resolves outside the workspace root');
     });
 
+    it('should not flag filenames containing consecutive dots as traversal', () => {
+      const filePath = 'my..query.ql';
+      writeFileSync(join(testDir, filePath), 'select 1');
+
+      const result = resolvePromptFilePath(filePath, testDir);
+      expect(result.resolvedPath).toBe(join(testDir, filePath));
+      expect(result.warning).toBeUndefined();
+    });
+
     it('should return the original path as resolvedPath even for invalid paths', () => {
       const result = resolvePromptFilePath('nonexistent.ql', testDir);
       expect(result.resolvedPath).toBe(join(testDir, 'nonexistent.ql'));