Feat: Investigate more license edge cases

[JosephMaynard]

Summary by CodeRabbit

• New Features

  • Dependency graphs now include resolved installation path information for packages and nested dependencies, improving visibility into where packages are installed.

• Tests

  • Added tests that assert path properties are populated in dependency trees and verify that lockfile path entries attempting to traverse outside the project are ignored.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between e7fc7c5 and 7e8bf92.

📒 Files selected for processing (1)

• src/runners/lockfileGraph.ts

🚧 Files skipped from review as they are similar to previous changes (1)

• src/runners/lockfileGraph.ts

---

📝 Walkthrough

Walkthrough

Adds logic to resolve installed filesystem paths from npm/pnpm lockfile package keys and attaches resolved paths to dependency nodes; includes tests asserting resolved top-level and nested dependency paths and ensuring paths that traverse outside the lock directory are not resolved.

Changes

Cohort / File(s)	Summary
Lockfile path resolution & node population src/runners/lockfileGraph.ts	Adds resolveNpmInstalledPath(packageKey, lockDir) (normalizes lockfile keys, validates segments, resolves against lockDir, returns absolute path only if contained within lockDir) and uses it in buildNpmNodeFromPackages to set out.path when resolution succeeds. A duplicate internal copy of the helper was also added later in the file.
Tests for path resolution and traversal safety src/runners/npmLs.test.ts	Adds assertions that top-level and nested dependency nodes include correct path values when resolvable, and adds tests ensuring lockfile path entries that would traverse outside the lock directory (e.g., ../evil) are not resolved (remain undefined).

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

Possibly related PRs

• Fix: Lockfile First Scan Issue #14: Modifies src/runners/lockfileGraph.ts to add npm install-state / path-resolution logic and handling of lockDir.
• Feat: Lockfile First Dependency Graph #12: Related changes to lockfile-based npm path resolution and ResolvedNode.path handling.

Poem

🐰
I hop through lockfiles, soft and neat,
Tracing each package to its rightful street,
No ../ mischief may find a way,
Every path I check, then safely stay,
Hooray — installed homes all in their seat.

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Title check	⚠️ Warning	The PR title mentions 'license edge cases' but the actual changes focus on resolving npm package paths and validating path traversal security, with no license-related modifications.	Update the title to reflect the actual changes, such as 'Feat: Resolve npm package installation paths with security validation' or similar to accurately describe the path resolution and traversal checks.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

• 📝 Generate docstrings (stacked PR)
• 📝 Generate docstrings (commit on current branch)

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch feat/investigate-more-license-edge-cases

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@src/runners/lockfileGraph.ts`:
- Around line 656-660: The resolveNpmInstalledPath function currently joins
normalizedKey segments into lockDir without guarding against path traversal;
validate the normalizedKey segments (from normalizeLockPackageKey) to reject any
segment equal to ".." or that is an absolute path, then build a safe path via
path.resolve(lockDir, ...segments) and verify the resulting resolved path is
contained within lockDir (e.g. ensure path.relative(lockDir, resolved) does not
start with '..'); if validation fails return undefined. Update
resolveNpmInstalledPath to use these checks and return undefined for any
traversal/absolute-segment case.

---

ℹ️ Review info

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between b643ae7 and c4ce4d2.

⛔ Files ignored due to path filters (2)

• dist/runners/lockfileGraph.js is excluded by !**/dist/**
• dist/utils.js is excluded by !**/dist/**

📒 Files selected for processing (2)

• src/runners/lockfileGraph.ts
• src/runners/npmLs.test.ts

[coderabbitai]

Note

Docstrings generation - SUCCESS
Generated docstrings and committed to branch feat/investigate-more-license-edge-cases (commit: 7e8bf92380c9863ed41de9617cd0efe3436d0cd5)