Skip to content

Fix path traversal vulnerability in setup script dependency removal #28

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
toolate28 merged 4 commits into from
Jan 17, 2026
Merged

Fix path traversal vulnerability in setup script dependency removal #28

Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
e706f6b
Initial plan
Copilot Jan 16, 2026
4a958eb
Fix security vulnerability in setup.ts path validation
Copilot Jan 16, 2026
ee399d2
Simplify path validation logic to handle edge cases
Copilot Jan 16, 2026
4733bf2
Merge branch 'copilot/sub-pr-8-again' into copilot/sub-pr-24-yet-again
toolate28 Jan 17, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 12 additions & 3 deletions scripts/setup.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
import { $ } from "bun";
import { existsSync, rmSync } from "fs";
import { mkdir, writeFile } from "fs/promises";
import { join } from "path";
import { join, resolve } from "path";

const ROOT_DIR = join(import.meta.dir, "..");
const ATOM_TRAIL_DIR = join(ROOT_DIR, ".atom-trail");
Expand Down Expand Up @@ -101,8 +101,17 @@ async function installDependencies(force: boolean): Promise<boolean> {
const nodeModulesPath = join(ROOT_DIR, "node_modules");
if (force) {
log("step", "Force reinstalling dependencies...");
// Validate path before removal for safety
if (existsSync(nodeModulesPath) && nodeModulesPath.endsWith("node_modules")) {
// Validate path before removal for security
// Ensure the resolved path is within the project directory
Copy link

Copilot AI Jan 17, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The comment states "Ensure the resolved path is within the project directory" but the validation actually checks for exact path equality, not containment. Consider updating the comment to be more accurate:

"Ensure the resolved path exactly matches the expected node_modules location"

This better reflects that the code validates exact equality rather than directory containment.

Suggested change
// Ensure the resolved path is within the project directory
// Ensure the resolved path exactly matches the project node_modules directory

Copilot uses AI. Check for mistakes.
const resolvedNodeModules = resolve(nodeModulesPath);
const resolvedRoot = resolve(ROOT_DIR);
const expectedPath = resolve(resolvedRoot, "node_modules");

// Check that the resolved path exactly matches the expected node_modules path
// This prevents path traversal attacks (e.g., /etc/node_modules)
const isValidPath = resolvedNodeModules === expectedPath;

Comment on lines +107 to +113
Copy link

Copilot AI Jan 17, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The variable resolvedRoot is unnecessary since ROOT_DIR is already a stable path computed at module level. The code can be simplified by directly using resolve(ROOT_DIR, "node_modules") for the expected path comparison.

Consider simplifying to:

const resolvedNodeModules = resolve(nodeModulesPath);
const expectedPath = resolve(ROOT_DIR, "node_modules");
const isValidPath = resolvedNodeModules === expectedPath;

This reduces redundancy while maintaining the same security guarantees.

Suggested change
const resolvedRoot = resolve(ROOT_DIR);
const expectedPath = resolve(resolvedRoot, "node_modules");
// Check that the resolved path exactly matches the expected node_modules path
// This prevents path traversal attacks (e.g., /etc/node_modules)
const isValidPath = resolvedNodeModules === expectedPath;
const expectedPath = resolve(ROOT_DIR, "node_modules");
// Check that the resolved path exactly matches the expected node_modules path
// This prevents path traversal attacks (e.g., /etc/node_modules)
const isValidPath = resolvedNodeModules === expectedPath;

Copilot uses AI. Check for mistakes.
if (existsSync(nodeModulesPath) && isValidPath) {
rmSync(nodeModulesPath, { recursive: true, force: true });
}
Comment on lines +104 to 116
Copy link

Copilot AI Jan 17, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This security-critical path validation logic lacks test coverage. Given that this fix addresses a path traversal vulnerability, it should include tests that verify:

  1. The validation correctly accepts valid paths (e.g., ROOT_DIR/node_modules)
  2. The validation correctly rejects malicious paths (e.g., /etc/node_modules, ../../../etc/node_modules)
  3. Edge cases are handled (symlinks, Windows paths, relative paths)

The repository has comprehensive test coverage for other packages (e.g., quantum-ethics), so adding tests for this security fix would be consistent with the project's testing practices.

Copilot uses AI. Check for mistakes.
await $`cd ${ROOT_DIR} && bun install`.quiet();
Expand Down
Loading