Skip to content

Commit 4496a4c

Browse files
committed
Cleanup OS tmp dir client integration tests
1 parent 1eec148 commit 4496a4c

File tree

9 files changed

+61
-15
lines changed

9 files changed

+61
-15
lines changed

.github/agents/ql-mcp-tool-tester.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ My `ql-mcp-tool-tester` agent:
2525
- NEVER makes anything up about CodeQL CLI behavior or MCP protocol.
2626
- NEVER modifies the MCP server or client code; focuses solely on testing and validating the tools/primitives.
2727
- NEVER "pipes" or redirects `npm test` or `npm run test*` command outputs in any way. Just observe the raw output and use exit codes to determine success/failure.
28+
- **NEVER uses `os.tmpdir()`, `/tmp`, or any OS-level temporary directory** in test code, fixtures, or tool invocations. The OS temp directory is world-readable and triggers CWE-377/CWE-378 vulnerabilities. All temporary files MUST use the project-local `<repoRoot>/.tmp/` directory. In integration test fixtures the `{{tmpdir}}` placeholder resolves to this project-local directory at runtime — it does NOT resolve to the OS temp directory.
2829

2930
## Related Skills
3031

.github/instructions/server_test_ts.instructions.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,3 +35,4 @@ This file contains instructions for working with TypeScript test files in the `s
3535
- NEVER write tests that depend on external resources or network calls without proper mocking.
3636
- NEVER write overly complex tests that test multiple concerns in a single test case.
3737
- NEVER skip writing tests for new functionality or bug fixes.
38+
- **NEVER use `os.tmpdir()`, `/tmp`, or any OS-level temporary directory** in test code or test fixtures. The OS temp directory is world-readable and triggers CWE-377/CWE-378 vulnerabilities. Instead, ALWAYS use the project-local `.tmp/` directory via `getProjectTmpDir()`, `createProjectTempDir()`, or `getProjectTmpBase()` from `server/src/utils/temp-dir.ts`. For integration test fixtures, use the `{{tmpdir}}` placeholder which resolves at runtime to `<repoRoot>/.tmp/`.

client/integration-tests/README.md

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -38,15 +38,15 @@ Common mistakes to avoid:
3838
-**DO NOT** commit files like `evaluator-log.json`, `query-results.bqrs`, `*.bqrs` files to the repository root
3939
-**DO NOT** commit temporary files created during integration test development to the repository root
4040
-**DO** place all test output files in the appropriate `client/integration-tests/primitives/tools/<tool_name>/<test_name>/after/` directory
41-
-**DO** use `/tmp/` paths for temporary files during development and testing
41+
-**DO** use `{{tmpdir}}` as a placeholder for the project-local temporary directory in test fixture paths (resolved at runtime to `<repoRoot>/.tmp/`)
4242

4343
**File generation best practices:**
4444

4545
1. **Generate test files correctly**: When creating integration tests that involve file generation (e.g., BQRS files, evaluator logs):
4646
- Run the actual tool command to generate authentic files
4747
- Copy the generated files from their temporary location to the correct `after/` directory
4848
- Never fabricate or "make up" binary file contents
49-
2. **Use proper paths**: Always use absolute paths or `/tmp/` paths when running commands that generate files during development
49+
2. **Use proper paths**: Always use `{{tmpdir}}/` as a placeholder for the project-local temp directory in test fixture JSON files (e.g., `"output": "{{tmpdir}}/results.sarif"`). This resolves at runtime to `<repoRoot>/.tmp/`, **not** the OS temp directory, to avoid CWE-377/CWE-378 (world-readable temp files).
5050
3. **Verify placement**: Before committing, verify that generated files are in the correct `after/` directory, not in the repository root
5151

5252
The `.gitignore` file has been updated to help prevent accidental commits of common integration test output files in the root directory.

client/integration-tests/primitives/tools/codeql_database_analyze/analyze_javascript_database/before/monitoring-state.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,6 @@
44
"database": "server/ql/javascript/examples/test/ExampleQuery1/ExampleQuery1.testproj",
55
"queries": "server/ql/javascript/examples/src/ExampleQuery1/ExampleQuery1.ql",
66
"format": "sarif-latest",
7-
"output": "/tmp/integration-test-analyze-results.sarif"
7+
"output": "{{tmpdir}}/integration-test-analyze-results.sarif"
88
}
99
}

client/integration-tests/primitives/tools/codeql_database_create/create_javascript_database/before/monitoring-state.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
{
22
"sessions": [],
33
"parameters": {
4-
"database": "/tmp/codeql-integration-test-db",
4+
"database": "{{tmpdir}}/codeql-integration-test-db",
55
"language": "javascript",
66
"source-root": "server/ql/javascript/examples/test/ExampleQuery1",
77
"overwrite": true

client/integration-tests/primitives/tools/codeql_query_run/evaluator_logging_with_tuple_counting/after/monitoring-state.json

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -10,17 +10,17 @@
1010
"arguments": {
1111
"query": "client/integration-tests/static/javascript/src/ExampleQuery1/ExampleQuery1.ql",
1212
"database": "client/integration-tests/static/javascript/test/ExampleQuery1/ExampleQuery1.testproj",
13-
"output": "/tmp/test-query-results.bqrs",
14-
"evaluator-log": "/tmp/test-evaluator-log.json",
13+
"output": "{{tmpdir}}/test-query-results.bqrs",
14+
"evaluator-log": "{{tmpdir}}/test-evaluator-log.json",
1515
"evaluator-log-level": 5,
1616
"tuple-counting": true
1717
},
1818
"response": {
1919
"stdout": "Evaluation completed successfully",
2020
"stderr": "WARNING: Ignoring local version because local version support is off.",
2121
"filesGenerated": [
22-
"/tmp/test-query-results.bqrs",
23-
"/tmp/test-evaluator-log.json"
22+
"{{tmpdir}}/test-query-results.bqrs",
23+
"{{tmpdir}}/test-evaluator-log.json"
2424
]
2525
}
2626
}

client/integration-tests/primitives/tools/codeql_query_run/evaluator_logging_with_tuple_counting/test-config.json

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,8 +3,8 @@
33
"arguments": {
44
"query": "client/integration-tests/static/javascript/src/ExampleQuery1/ExampleQuery1.ql",
55
"database": "client/integration-tests/static/javascript/test/ExampleQuery1/ExampleQuery1.testproj",
6-
"output": "/tmp/test-query-results.bqrs",
7-
"evaluator-log": "/tmp/test-evaluator-log.json",
6+
"output": "{{tmpdir}}/test-query-results.bqrs",
7+
"evaluator-log": "{{tmpdir}}/test-evaluator-log.json",
88
"evaluator-log-level": 5,
99
"tuple-counting": true
1010
}

client/integration-tests/primitives/tools/create_codeql_query/create_javascript_query/before/monitoring-state.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
{
22
"sessions": [],
33
"parameters": {
4-
"basePath": "/tmp/codeql-test-query",
4+
"basePath": "{{tmpdir}}/codeql-test-query",
55
"queryName": "TestQuery",
66
"language": "javascript",
77
"description": "Integration test query"

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

Lines changed: 48 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,6 @@
33
*/
44

55
import fs from "fs";
6-
import os from "os";
76
import path from "path";
87
import { fileURLToPath } from "url";
98
import {
@@ -13,6 +12,44 @@ import {
1312
removeDirectory
1413
} from "./file-utils.js";
1514

15+
/**
16+
* Repository root, calculated once at module load.
17+
* Mirrors `server/src/utils/temp-dir.ts`.
18+
*/
19+
const __filename = fileURLToPath(import.meta.url);
20+
const __dirname = path.dirname(__filename);
21+
const repoRoot = path.resolve(__dirname, "..", "..", "..");
22+
23+
/**
24+
* Project-local temporary directory (`<repoRoot>/.tmp`).
25+
* All temporary files are kept here instead of the OS temp directory
26+
* to avoid CWE-377/CWE-378 (world-readable temp files).
27+
*/
28+
const PROJECT_TMP_BASE = path.join(repoRoot, ".tmp");
29+
30+
/**
31+
* Resolve `{{tmpdir}}` placeholders in string values of a parameters object.
32+
* Test fixtures use `{{tmpdir}}` as a cross-platform placeholder for the
33+
* project-local temporary directory (`<repoRoot>/.tmp`), which avoids
34+
* writing to the world-readable OS temp directory (CWE-377 / CWE-378).
35+
*
36+
* @param {Record<string, unknown>} params - Tool parameters object (mutated in place)
37+
* @param {object} [logger] - Optional logger for diagnostics
38+
* @returns {Record<string, unknown>} The same object, with placeholders resolved
39+
*/
40+
export function resolvePathPlaceholders(params, logger) {
41+
fs.mkdirSync(PROJECT_TMP_BASE, { recursive: true });
42+
for (const [key, value] of Object.entries(params)) {
43+
if (typeof value === "string" && value.includes("{{tmpdir}}")) {
44+
params[key] = value.replace(/\{\{tmpdir\}\}/g, PROJECT_TMP_BASE);
45+
if (logger) {
46+
logger.log(` Resolved ${key}: {{tmpdir}} → ${params[key]}`);
47+
}
48+
}
49+
}
50+
return params;
51+
}
52+
1653
/**
1754
* Integration test runner class
1855
*/
@@ -208,8 +245,11 @@ export class IntegrationTestRunner {
208245
return;
209246
}
210247

211-
// Create temp directory for test execution
212-
const tempDir = fs.mkdtempSync(path.join(os.tmpdir(), `mcp-test-${toolName}-${testCase}-`));
248+
// Create temp directory for test execution under project .tmp/
249+
fs.mkdirSync(PROJECT_TMP_BASE, { recursive: true });
250+
const tempDir = fs.mkdtempSync(
251+
path.join(PROJECT_TMP_BASE, `mcp-test-${toolName}-${testCase}-`)
252+
);
213253

214254
try {
215255
// Copy before files to temp directory
@@ -414,7 +454,10 @@ export class IntegrationTestRunner {
414454
* Run a file-based test with custom configuration
415455
*/
416456
async runFileBasedConfigurableTest(toolName, testCase, testConfig, beforeDir, afterDir) {
417-
const tempDir = fs.mkdtempSync(path.join(os.tmpdir(), `mcp-test-${toolName}-${testCase}-`));
457+
fs.mkdirSync(PROJECT_TMP_BASE, { recursive: true });
458+
const tempDir = fs.mkdtempSync(
459+
path.join(PROJECT_TMP_BASE, `mcp-test-${toolName}-${testCase}-`)
460+
);
418461

419462
try {
420463
// Copy before files to temp directory
@@ -630,6 +673,7 @@ export class IntegrationTestRunner {
630673
if (monitoringState.parameters) {
631674
params = monitoringState.parameters;
632675
this.logger.log(`Using parameters from monitoring-state.json`);
676+
resolvePathPlaceholders(params, this.logger);
633677

634678
// Helper function to ensure database is extracted
635679
const ensureDatabaseExtracted = async (dbPath) => {

0 commit comments

Comments
 (0)