-
-
Notifications
You must be signed in to change notification settings - Fork 97
feat: detect phantom dependencies #753
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.
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -59,7 +59,7 @@ import { renderOverrideFindings } from "./output/override-findings-terminal.js"; | |
| import { installSkill } from "./skills/install.js"; | ||
| import { readConfig, validateCaCertFile } from "./cli/config.js"; | ||
| import { runConfigCommand } from "./cli/config-command.js"; | ||
| import { readDirectDependencyNames, hasOverrideEntries } from "./utils/package-json.js"; | ||
| import { readDirectDependencyNames, hasOverrideEntries, readOverridesAndResolutions } from "./utils/package-json.js"; | ||
| import { | ||
| applyFixesIfRequested, | ||
| FixExecutionResult, | ||
|
|
@@ -656,6 +656,20 @@ if (parsedArgs) { | |
| } | ||
| if (showOverrideHint) printOverrideHint(); | ||
| } | ||
|
|
||
| if (options.usage) { | ||
| if (scanState.pd001 && scanState.pd001.length > 0) { | ||
| console.log(chalk.red("\n[PD001] Override-only Phantom Dependencies Detected (Build Breakers)")); | ||
| scanState.pd001.forEach(pkg => console.log(` - ${pkg}`)); | ||
| } | ||
| if (scanState.pd002 && scanState.pd002.length > 0) { | ||
| console.log(chalk.yellow("\n[PD002] Transitive-only Phantom Dependencies Detected")); | ||
| scanState.pd002.forEach(pkg => console.log(` - ${pkg}`)); | ||
| } | ||
| if ((scanState.pd001 && scanState.pd001.length > 0) || (scanState.pd002 && scanState.pd002.length > 0)) { | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Phantom findings aren't wired into |
||
| console.log(chalk.gray("\nAction: Declare these dependencies explicitly in your package.json.\n")); | ||
| } | ||
| } | ||
| } | ||
| } | ||
|
|
||
|
|
@@ -708,7 +722,9 @@ if (parsedArgs) { | |
| const reachesFailOn = (f: { severity: SeverityLabel }) => | ||
| severityOrder[f.severity] >= severityOrder[failLevel]; | ||
| const shouldFail = | ||
| scanState.sorted.some(reachesFailOn) || overrideFindings.some(reachesFailOn); | ||
| scanState.sorted.some(reachesFailOn) || | ||
| overrideFindings.some(reachesFailOn) || | ||
| (options.usage && ((scanState.pd001 && scanState.pd001.length > 0) || (scanState.pd002 && scanState.pd002.length > 0))); | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Any phantom finding triggers exit code 1 here regardless of what |
||
| const exitCode = shouldFail ? 1 : 0; | ||
|
|
||
| // Emit scan.finished event and close audit-log | ||
|
|
@@ -784,11 +800,32 @@ async function scanProject(params: { | |
| scanFilePath: params.scanInput.filePath, | ||
| }, params.debugLog); | ||
|
|
||
| let pd001: string[] = []; | ||
| let pd002: string[] = []; | ||
|
|
||
| if (params.options.usage) { | ||
| logInfo(`Scanning project source for usage hints...`, params.options); | ||
| logInfo(`Scanning project source for usage hints and phantom dependencies...`, params.options); | ||
| const usageStartedAt = Date.now(); | ||
| const pkgNames = new Set(findings.map(f => f.pkg.name)); | ||
| const usageData = scanProjectForPackageUsage(params.projectPath, pkgNames); | ||
|
|
||
| // Pass undefined to get ALL imported packages for phantom dependency detection | ||
| const usageData = scanProjectForPackageUsage(params.projectPath); | ||
|
|
||
| const allLockfilePackages = new Set(params.scanInput.packages.map(p => p.name)); | ||
| const overridesAndResolutions = readOverridesAndResolutions(params.projectPath); | ||
| const directDeps = directDependencyNames || new Set<string>(); | ||
|
|
||
| for (const importedPkg of Object.keys(usageData)) { | ||
| if (usageData[importedPkg].length === 0) continue; | ||
|
|
||
| if (!directDeps.has(importedPkg)) { | ||
| if (overridesAndResolutions.has(importedPkg)) { | ||
| pd001.push(importedPkg); | ||
| } else if (allLockfilePackages.has(importedPkg)) { | ||
| pd002.push(importedPkg); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| let matchedPackages = 0; | ||
| for (const finding of findings) { | ||
| const files = usageData[finding.pkg.name]; | ||
|
|
@@ -805,8 +842,10 @@ async function scanProject(params: { | |
| if (params.options.debug) { | ||
| params.debugLog("Usage scan", { | ||
| durationMs: Date.now() - usageStartedAt, | ||
| packagesChecked: pkgNames.size, | ||
| packagesChecked: Object.keys(usageData).length, | ||
| matchedPackages, | ||
| pd001: pd001.length, | ||
| pd002: pd002.length, | ||
| }); | ||
| } | ||
| } | ||
|
|
@@ -839,6 +878,8 @@ async function scanProject(params: { | |
| tableFindings, | ||
| suggestedFixCommands, | ||
| allPackages: params.scanInput.packages, | ||
| pd001, | ||
| pd002, | ||
| }; | ||
| } | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -157,3 +157,31 @@ export function hasOverrideEntries(dir: string): boolean { | |
| return false; | ||
| } | ||
| } | ||
|
|
||
| export function readOverridesAndResolutions(projectPath: string): Set<string> { | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The existing |
||
| const packageJsonPath = path.join(projectPath, "package.json"); | ||
| const result = new Set<string>(); | ||
|
|
||
| if (!fs.existsSync(packageJsonPath)) { | ||
| return result; | ||
| } | ||
|
|
||
| const manifest = readPackageJsonObject(packageJsonPath); | ||
| if (!manifest) { | ||
| return result; | ||
| } | ||
|
|
||
| const sections: Array<Record<string, unknown> | undefined> = [ | ||
| isRecord(manifest.overrides) ? manifest.overrides : undefined, | ||
| isRecord(manifest.resolutions) ? manifest.resolutions : undefined, | ||
| ]; | ||
|
|
||
| for (const section of sections) { | ||
| if (!section) continue; | ||
| for (const key of Object.keys(section)) { | ||
| result.add(key); | ||
| } | ||
| } | ||
|
|
||
| return result; | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Phantom detection is gated on
options.usage, but--usageis a separate feature for import-reachability filtering with its own semantics. Users who don't pass--usageget no phantom detection at all, and users who do pass--usageget phantom detection they didn't ask for. These should be decoupled - either run phantom detection unconditionally or add a dedicated--check-phantomsflag.