Security fixes for TOCTOU & OS tmp files

[data-douser]

Summary of Changes

This pull request introduces several improvements focused on security (mitigating TOCTOU race conditions), more robust file/directory handling, and a shift from using global /tmp directories to project-local temporary directories throughout the codebase. The changes also enhance atomic file creation and simplify directory initialization logic.

Outline of Changes

Key improvements include:

TOCTOU Race Condition Mitigations & Atomic File Operations:

• Replaced checks like existsSync followed by file creation with atomic file creation using the 'wx' flag in writeFileSync, ensuring files are only created if they do not already exist and preventing race conditions. This is applied in query-scaffolding.ts and session-data-manager.ts. [1] [2]
• Updated output validation logic in the integration test runner to read files directly and handle errors (such as non-existent files or directories) robustly, avoiding TOCTOU issues. [1] [2] [3] [4]

Project-local Temporary Directory Standardization:

• Replaced direct usage of /tmp and related Node.js temp directory utilities with project-local temporary directories via helpers like getProjectTmpDir and createProjectTempDir across the language server, CLI tool registry, log directory manager, and CodeQL tools. This ensures all temporary files and logs are stored in a consistent, project-scoped location. [1] [2] [3] [4] [5] [6] [7]

Simplified and Safer Directory Creation:

• Updated directory creation logic to use mkdirSync with { recursive: true } unconditionally, since it is a no-op if the directory already exists, further reducing race conditions and simplifying code. [1] [2] [3]

API and Documentation Updates:

• Updated tool option documentation and default values to reflect the new project-local temporary directory usage instead of /tmp. [1] [2]

These changes collectively improve the safety, reliability, and maintainability of file and directory operations across the codebase.

---

TOCTOU Race Condition Mitigations & Atomic File Operations

• Replaced existsSync checks with atomic file creation using the 'wx' flag in writeFileSync for query scaffolding and session data manager config files, eliminating TOCTOU vulnerabilities. [1] [2]
• Updated integration test runner output validation to read files directly and handle errors for missing files or directories, ensuring robust and race-free validation logic. [1] [2] [3] [4]

Project-local Temporary Directory Standardization

• Replaced /tmp and OS temp directory usage with project-local temp directories via getProjectTmpDir and createProjectTempDir in the language server, CLI tool registry, log directory manager, and CodeQL tools. [1] [2] [3] [4] [5] [6] [7]

Simplified and Safer Directory Creation

• Used mkdirSync with { recursive: true } unconditionally for all directory creation, simplifying logic and avoiding race conditions. [1] [2] [3]

API and Documentation Updates

• Updated descriptions and defaults for log directories and output paths in tool schemas to reference project-local .tmp directories instead of /tmp. [1] [2]

[Copilot]

Pull request overview

This PR hardens file-system interactions by mitigating TOCTOU patterns (via atomic writes / simplified directory creation) and standardizing temporary/log output locations to a project-local .tmp/ directory across the server, client integration tests, and server test suite.

Changes:

• Introduces project-local temp directory helpers and updates tools/tests to stop using OS temp locations like /tmp / os.tmpdir().
• Replaces existsSync + create/write sequences with more atomic patterns (e.g., writeFileSync(..., { flag: 'wx' })) and simplifies directory initialization with mkdirSync(..., { recursive: true }).
• Updates integration test output validation to avoid TOCTOU by reading files directly and handling missing paths/dirs robustly.

Reviewed changes

Copilot reviewed 23 out of 26 changed files in this pull request and generated 8 comments.

Show a summary per file

File	Description
server/test/utils/temp-dir.ts	New test temp-dir helper intended to create temp dirs under <repoRoot>/.tmp/test-data.
server/test/src/utils/temp-dir.test.ts	Adds unit tests for the server temp-dir utilities.
server/test/src/tools/codeql/register-database.test.ts	Updates tests to use project-local test temp directories.
server/test/src/tools/codeql/quick-evaluate.test.ts	Updates temp file creation to use project-local test temp directories.
server/test/src/tools/codeql/profile-codeql-query.test.ts	Updates temp directory usage to project-local test temp directories.
server/test/src/tools/codeql/find-query-files.test.ts	Updates temp directory usage/cleanup to new helpers.
server/test/src/tools/codeql/find-predicate-position.test.ts	Updates temp file creation to use project-local test temp directories.
server/test/src/tools/codeql/find-class-position.test.ts	Updates temp file creation to use project-local test temp directories.
server/test/src/lib/validation.test.ts	Updates temp directory usage/cleanup to new helpers.
server/test/src/lib/query-results-evaluator.test.ts	Updates temp directory usage/cleanup to new helpers.
server/test/src/lib/query-file-finder.test.ts	Updates temp directory usage/cleanup to new helpers.
server/test/src/lib/log-directory-manager.test.ts	Updates temp directory usage and adjusts default path expectations.
server/src/utils/temp-dir.ts	New server-side project-local temp-dir utilities under <repoRoot>/.tmp.
server/src/tools/codeql/test-run.ts	Updates schema docs to reflect .tmp defaults for logs.
server/src/tools/codeql/quick-evaluate.ts	Changes default output behavior to use .tmp/quickeval when not specified.
server/src/tools/codeql/query-run.ts	Updates schema docs to reflect .tmp defaults for logs.
server/src/tools/codeql/language-server-eval.ts	Uses project-local temp location for evaluation URIs.
server/src/lib/session-data-manager.ts	Simplifies directory creation and uses atomic config creation (wx).
server/src/lib/query-scaffolding.ts	Simplifies directory creation and uses atomic file creation (wx).
server/src/lib/log-directory-manager.ts	Defaults log base to project-local .tmp/query-logs.
server/src/lib/language-server.ts	Switches default eval document URI away from /tmp to project-local .tmp.
server/src/lib/cli-tool-registry.ts	Uses project-local temp dirs for intermediate CSV files.
server/dist/ql-mcp-server.js	Updates bundled output to reflect source changes.
client/src/lib/integration-test-runner.js	Reworks output validation logic to avoid TOCTOU patterns.
.gitignore	Adds .tmp/ to ignored paths.

Comments suppressed due to low confidence (4)

server/test/utils/temp-dir.ts:52

• cleanupTestTempDir checks existsSync(dir) before rmSync(..., { force: true }), but force: true already handles non-existent paths. Dropping the existence check avoids a TOCTOU race and simplifies cleanup logic.

export function cleanupTestTempDir(dir: string): void {
  if (existsSync(dir)) {
    rmSync(dir, { recursive: true, force: true });
  }

server/test/src/tools/codeql/find-class-position.test.ts:22

• withTempFile creates a unique temp directory but only removes the temp file via safeUnlink; the directory itself is never removed. Add directory cleanup in the finally block to avoid leaking empty directories under .tmp.

  const tempDir = createTestTempDir('find-class-pos');
  const tempFile = join(tempDir, `${testName}.ql`);
  try {
    await fs.writeFile(tempFile, content);
    return await fn(tempFile);
  } finally {
    await safeUnlink(tempFile);
  }

server/test/src/tools/codeql/quick-evaluate.test.ts:36

• withTempFile creates a unique temp directory but only unlinks the file; the directory itself is never removed. Over time (or when tests are interrupted) this can accumulate many empty directories under .tmp. Add directory cleanup in the finally block (e.g., fs.rm(tempDir, { recursive: true, force: true }) or cleanupTestTempDir(tempDir)).

  const tempDir = createTestTempDir('quick-eval');
  const tempFile = join(tempDir, `${testName}.ql`);
  try {
    await fs.writeFile(tempFile, content);
    return await fn(tempFile);
  } finally {
    try {
      await fs.unlink(tempFile);
    } catch {
      // Ignore cleanup errors
    }
  }

server/test/src/tools/codeql/find-predicate-position.test.ts:22

• withTempFile creates a unique temp directory but only removes the temp file via safeUnlink; the directory itself is never removed. Add directory cleanup in the finally block to avoid leaking empty directories under .tmp.

  const tempDir = createTestTempDir('find-pred-pos');
  const tempFile = join(tempDir, `${testName}.ql`);
  try {
    await fs.writeFile(tempFile, content);
    return await fn(tempFile);
  } finally {
    await safeUnlink(tempFile);
  }

[Copilot]

Pull request overview

Copilot reviewed 23 out of 26 changed files in this pull request and generated 1 comment.

Comments suppressed due to low confidence (4)

server/src/lib/log-directory-manager.ts:35

• ensurePathWithinBase uses absTarget.startsWith(absBase + '/'), which is not path-separator agnostic and will fail on Windows (resolved paths use \). Consider using path.relative/isAbsolute semantics (e.g., relative(base, target) not starting with ..), or normalizing separators consistently before the prefix check.

  const baseLogDir = process.env.CODEQL_QUERY_LOG_DIR || getProjectTmpDir('query-logs');

  // If logDir is explicitly provided, validate it is within baseLogDir
  if (logDir) {
    const absLogDir = ensurePathWithinBase(baseLogDir, logDir);

server/test/src/tools/codeql/quick-evaluate.test.ts:31

• withTempFile creates a new temp directory for each call but only unlinks the file in finally, leaving the directory behind under .tmp/test-data. Use cleanupTestTempDir(tempDir) (or fs.rm(tempDir, { recursive: true, force: true })) in the finally block to avoid accumulating temp directories across test runs.

async function withTempFile<T>(content: string, testName: string, fn: (filePath: string) => Promise<T>): Promise<T> {
  const tempDir = createTestTempDir('quick-eval');
  const tempFile = join(tempDir, `${testName}.ql`);
  try {
    await fs.writeFile(tempFile, content);
    return await fn(tempFile);
  } finally {
    try {

server/test/src/tools/codeql/find-predicate-position.test.ts:20

• withTempFile creates a new temp directory for each call but only removes the file, leaving the directory behind under .tmp/test-data. Consider calling cleanupTestTempDir(tempDir) in the finally block to avoid accumulating temp directories over time.

async function withTempFile<T>(content: string, testName: string, fn: (filePath: string) => Promise<T>): Promise<T> {
  const tempDir = createTestTempDir('find-pred-pos');
  const tempFile = join(tempDir, `${testName}.ql`);
  try {
    await fs.writeFile(tempFile, content);
    return await fn(tempFile);
  } finally {

server/test/src/tools/codeql/find-class-position.test.ts:20

• withTempFile creates a new temp directory for each call but only removes the file, leaving the directory behind under .tmp/test-data. Consider calling cleanupTestTempDir(tempDir) in the finally block to avoid accumulating temp directories over time.

async function withTempFile<T>(content: string, testName: string, fn: (filePath: string) => Promise<T>): Promise<T> {
  const tempDir = createTestTempDir('find-class-pos');
  const tempFile = join(tempDir, `${testName}.ql`);
  try {
    await fs.writeFile(tempFile, content);
    return await fn(tempFile);
  } finally {