Skip to content

Commit a47ab00

Browse files
ochafikclaude
andcommitted
fix: address security review comments
- Use proper URL parsing to check registry hostname (prevents spoofing via subdomains or paths containing "registry.npmjs.org") - Validate projectRoot path before using in shell command to prevent command injection via malicious path characters 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
1 parent 35ea69c commit a47ab00

File tree

1 file changed

+48
-8
lines changed

1 file changed

+48
-8
lines changed

scripts/preflight.mjs

Lines changed: 48 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -60,7 +60,7 @@ Examples:
6060
// Detect environment
6161
const isCI = Boolean(process.env.CI);
6262
const registryUrl = getRegistryUrl();
63-
const isInternalRegistry = !registryUrl.includes("registry.npmjs.org");
63+
const isInternalRegistry = !isPublicNpmRegistry(registryUrl);
6464

6565
function getRegistryUrl() {
6666
try {
@@ -70,6 +70,33 @@ function getRegistryUrl() {
7070
}
7171
}
7272

73+
function isPublicNpmRegistry(urlString) {
74+
try {
75+
const url = new URL(urlString);
76+
// Check exact hostname match to prevent subdomain/path spoofing
77+
return url.hostname === "registry.npmjs.org";
78+
} catch {
79+
return false;
80+
}
81+
}
82+
83+
function validatePathForShell(path) {
84+
// Reject paths with characters that could enable shell injection
85+
// Allow only alphanumeric, dash, underscore, dot, forward slash, and space
86+
const safePathPattern = /^[a-zA-Z0-9_\-./\s]+$/;
87+
if (!safePathPattern.test(path)) {
88+
throw new Error(
89+
`Unsafe characters in path: ${path}\n` +
90+
"Path contains characters that could be interpreted by the shell.",
91+
);
92+
}
93+
// Also reject paths that could escape the docker mount (e.g., containing ..)
94+
if (path.includes("..")) {
95+
throw new Error("Path cannot contain '..' segments");
96+
}
97+
return path;
98+
}
99+
73100
function log(msg) {
74101
console.log(msg);
75102
}
@@ -83,21 +110,30 @@ function verbose(msg) {
83110
// ============================================================================
84111

85112
if (FIX_DOCKER) {
86-
log("🐳 Regenerating package-lock.json using Docker (public npm registry)...\n");
113+
log(
114+
"🐳 Regenerating package-lock.json using Docker (public npm registry)...\n",
115+
);
87116

88117
if (!commandExists("docker")) {
89118
console.error("❌ Docker is not installed or not in PATH.");
90-
console.error(" Install Docker or use --local to regenerate with your current registry.");
119+
console.error(
120+
" Install Docker or use --local to regenerate with your current registry.",
121+
);
91122
process.exit(1);
92123
}
93124

94125
try {
126+
// Validate projectRoot before using in shell command to prevent injection
127+
const safePath = validatePathForShell(projectRoot);
128+
95129
// Read current prepare script to restore it later
96-
const pkgJson = JSON.parse(readFileSync(join(projectRoot, "package.json"), "utf-8"));
130+
const pkgJson = JSON.parse(
131+
readFileSync(join(projectRoot, "package.json"), "utf-8"),
132+
);
97133
const prepareScript = pkgJson.scripts?.prepare || "";
98134

99135
execSync(
100-
`docker run --rm -v "${projectRoot}:/app" -w /app node:20 bash -c '
136+
`docker run --rm -v "${safePath}:/app" -w /app node:20 bash -c '
101137
# Temporarily disable prepare script
102138
node -e "
103139
const fs = require(\\\"fs\\\");
@@ -117,7 +153,7 @@ if (FIX_DOCKER) {
117153
fs.writeFileSync(\\\"package.json\\\", JSON.stringify(pkg, null, 2));
118154
"
119155
'`,
120-
{ stdio: "inherit", cwd: projectRoot }
156+
{ stdio: "inherit", cwd: projectRoot },
121157
);
122158

123159
log("\n✅ Regenerated package-lock.json from public npm registry.");
@@ -200,7 +236,9 @@ log("\n" + "─".repeat(60));
200236

201237
if (isCI) {
202238
log("\n⚠️ CI Environment Detected");
203-
log(" The package-lock.json contains packages not available in the registry.");
239+
log(
240+
" The package-lock.json contains packages not available in the registry.",
241+
);
204242
log(" This PR should regenerate the lockfile using:");
205243
log(" node scripts/preflight.mjs --fix");
206244
process.exit(1);
@@ -212,7 +250,9 @@ if (isInternalRegistry) {
212250
log("\n Options:");
213251
log(" 1. Regenerate lockfile from your registry (versions may differ):");
214252
log(" node scripts/preflight.mjs --local");
215-
log("\n 2. Request the missing packages be synced to your internal registry.");
253+
log(
254+
"\n 2. Request the missing packages be synced to your internal registry.",
255+
);
216256
} else {
217257
log("\n💡 To fix, regenerate the lockfile from the public registry:");
218258
log(" node scripts/preflight.mjs --fix");

0 commit comments

Comments
 (0)