Skip to content

Commit b59f203

Browse files
committed
Limit npm scans to shipped production dependency trees
Tighten npm scanning so it follows the production dependency tree that ships with each dep instead of drifting into nested manifests and dev-only trees. Changes: - scan only dep-root package.json files under deps/ - skip nested package.json files by default - use npm install/audit with --omit=dev - prefer existing node_modules or package-lock.json when available - fall back to install + audit when package-lock-only audit reports ENOLOCK - improve diagnostics for npm install/audit failures Add regression coverage for the new package discovery and lockfile/install behaviour.
1 parent bcb8e6f commit b59f203

2 files changed

Lines changed: 282 additions & 53 deletions

File tree

dep_checker/npm_audit.py

Lines changed: 66 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,9 @@
11
"""NPM package vulnerability checker using npm audit
22
33
This module handles npm package vulnerability scanning by:
4-
1. Finding all package.json files in the repository
5-
2. Running npm install --production in each directory
6-
3. Running npm audit --json to get vulnerability data
4+
1. Finding dep-root package.json files in the repository
5+
2. Reusing package-lock.json when present, otherwise installing non-dev dependencies
6+
3. Running npm audit against the prod dependency tree
77
4. Parsing results into Vulnerability objects
88
"""
99

@@ -55,7 +55,7 @@ def __init__(self, repo_path: Path, timeout: int = 300):
5555
self.failed_packages: List[str] = []
5656

5757
def find_package_json_files(self) -> List[Path]:
58-
"""Find all package.json files in the deps/ folder only, excluding specified folders"""
58+
"""Find dep-root package.json files in deps/, excluding nested package manifests."""
5959
package_json_files = []
6060
excluded_count = 0
6161

@@ -86,8 +86,17 @@ def find_package_json_files(self) -> List[Path]:
8686
logger.debug(f"Excluding {package_json} (matches exclusion: {exclude_path})")
8787
break
8888

89-
if not is_excluded:
90-
package_json_files.append(package_json)
89+
if is_excluded:
90+
continue
91+
92+
relative_to_deps = package_json.relative_to(deps_path)
93+
dep_root = deps_path / relative_to_deps.parts[0] / "package.json"
94+
if package_json != dep_root:
95+
# ponytail: only audit dep roots; add a nested allowlist if a shipped package ever lives deeper.
96+
logger.debug(f"Skipping nested package.json {package_json}")
97+
continue
98+
99+
package_json_files.append(package_json)
91100

92101
logger.info(f"Found {len(package_json_files)} package.json files in deps/ folder")
93102
if excluded_count > 0:
@@ -98,8 +107,21 @@ def find_package_json_files(self) -> List[Path]:
98107
logger.error(f"Error finding package.json files in deps/ folder: {e}")
99108
return []
100109

110+
def has_package_lock(self, package_dir: Path) -> bool:
111+
return (package_dir / "package-lock.json").exists()
112+
113+
def has_node_modules(self, package_dir: Path) -> bool:
114+
return (package_dir / "node_modules").is_dir()
115+
116+
def is_missing_lockfile_error(self, audit_data: Optional[Dict]) -> bool:
117+
if not isinstance(audit_data, dict):
118+
return False
119+
error = audit_data.get("error")
120+
return isinstance(error, dict) and error.get("code") == "ENOLOCK"
121+
122+
101123
def run_npm_install(self, package_dir: Path) -> bool:
102-
"""Install production dependencies needed for an audit without lifecycle scripts."""
124+
"""Install non-dev dependencies needed for an audit when no lockfile is present."""
103125
try:
104126
logger.info(f"Running npm install in {package_dir}")
105127
# Check if npm is available
@@ -119,8 +141,8 @@ def run_npm_install(self, package_dir: Path) -> bool:
119141
# construct npm's dependency tree for an audit.
120142
result = subprocess.run(
121143
[
122-
"npm", "install", "--production", "--ignore-scripts",
123-
"--no-audit", "--no-fund", "--silent",
144+
"npm", "install", "--omit=dev", "--ignore-scripts",
145+
"--no-audit", "--no-fund",
124146
],
125147
cwd=package_dir,
126148
capture_output=True,
@@ -145,12 +167,18 @@ def run_npm_install(self, package_dir: Path) -> bool:
145167
logger.error(f"Error running npm install in {package_dir}: {e}")
146168
return False
147169

148-
def run_npm_audit(self, package_dir: Path) -> Optional[Dict]:
149-
"""Run npm audit --json in the given directory"""
170+
def run_npm_audit(
171+
self, package_dir: Path, package_lock_only: bool = False
172+
) -> Optional[Dict]:
173+
"""Run npm audit against the production dependency tree in the given directory."""
150174
try:
151175
logger.info(f"Running npm audit in {package_dir}")
176+
command = ["npm", "audit", "--omit=dev"]
177+
if package_lock_only:
178+
command.append("--package-lock-only")
179+
command.append("--json")
152180
result = subprocess.run(
153-
["npm", "audit", "--json"],
181+
command,
154182
cwd=package_dir,
155183
capture_output=True,
156184
text=True,
@@ -331,6 +359,16 @@ def parse_audit_results(
331359

332360
except Exception as e:
333361
logger.error(f"Error parsing audit results from {package_dir}: {e}")
362+
if isinstance(audit_data, dict):
363+
keys = sorted(audit_data.keys())
364+
logger.error(f"npm audit JSON top-level keys for {package_dir}: {keys}")
365+
if "error" in audit_data:
366+
logger.error(f"npm audit JSON error payload for {package_dir}: {audit_data['error']}")
367+
metadata = audit_data.get("metadata")
368+
if metadata is not None:
369+
logger.error(f"npm audit JSON metadata for {package_dir}: {metadata}")
370+
else:
371+
logger.error(f"npm audit JSON type for {package_dir}: {type(audit_data).__name__}")
334372
return None
335373

336374
def check_npm_vulnerabilities(self, vulnerability_class) -> List:
@@ -347,14 +385,25 @@ def check_npm_vulnerabilities(self, vulnerability_class) -> List:
347385
logger.info(f"Processing {package_json}")
348386

349387
try:
350-
# Run npm install
351-
if not self.run_npm_install(package_dir):
388+
has_package_lock = self.has_package_lock(package_dir)
389+
has_node_modules = self.has_node_modules(package_dir)
390+
package_lock_only = has_package_lock and not has_node_modules
391+
392+
if not has_node_modules and not has_package_lock and not self.run_npm_install(package_dir):
352393
logger.warning(f"Skipping npm audit for {package_dir} due to install failure")
353394
self.failed_packages.append(f"{package_dir}: npm install failed")
354395
continue
355-
356-
# Run npm audit
357-
audit_data = self.run_npm_audit(package_dir)
396+
397+
audit_data = self.run_npm_audit(package_dir, package_lock_only=package_lock_only)
398+
if package_lock_only and self.is_missing_lockfile_error(audit_data):
399+
logger.warning(
400+
f"npm audit reported ENOLOCK for {package_dir}; falling back to npm install + npm audit"
401+
)
402+
if not self.run_npm_install(package_dir):
403+
logger.warning(f"Skipping npm audit for {package_dir} due to install failure")
404+
self.failed_packages.append(f"{package_dir}: npm install failed")
405+
continue
406+
audit_data = self.run_npm_audit(package_dir, package_lock_only=False)
358407
if audit_data is None:
359408
logger.warning(f"Skipping vulnerability parsing for {package_dir} due to audit failure")
360409
self.failed_packages.append(f"{package_dir}: npm audit failed")

0 commit comments

Comments
 (0)