[vianmangal]

What changed and why

When the pnpm workspace root and a member declared the same vulnerable package, the direct fix target discarded the root declaration. The plan reported the finding as covered while printing only a member command.

Retain the root declaration alongside member paths. For shared lodash declarations, the plan emits pnpm add -w lodash@4.18.1 && pnpm --filter ./packages/member-0 add lodash@4.18.1, preserving -D for dev targets. Plan sections, per-finding output, JSON serialization, and --fix include both commands. The executor reports success only after both commands succeed.

workspaceRoot means the target needs a root install, whether or not members also declare the package. The grouping key keeps those targets separate from packages declared only by the same member, so a member-only package is not added to the root. Root-only targets emit and execute exactly one pnpm add -w command.

Closes #1592

Review follow-up and merge order

• Added the requested grouping and root-only cases for both production and dev dependencies, including the plan, per-finding command, sections, and actual executor argv.
• Removed the unnecessary Set from the grouping key and compute the group's workspaceRoot check once.
• fix(remediation): add -w to pnpm add for workspace-root dependencies #1587 must land first. It is still open. After it merges, this branch needs a rebase to reuse its isPnpmWorkspaceLockfile helper and fold its root-only argv shortcut into the unified root-install path here. The branch is currently rebased onto main at f6ec794.

Validation

• Ten lockfile-backed tests cover root plus one/two members, member-only and single-project controls, root-only installs, a shared target alongside member-only minimist, and a member-command failure after the root succeeds.
• Seven targeted mutations fail the tests: removing the root grouping marker, adding a duplicate bare command in the plan or executor, including the root in member filters, dropping root-only plan/per-finding routing, and removing the root marker. Restored the intended code afterward.
• Focused suites: 51 tests passed, one existing skip.
• Real pnpm 11.24.0 workspaces with two package patterns: ran the plan through the apply executor with lifecycle scripts disabled. Shared root-dev/member lodash advanced from 4.17.20 to 4.18.1; minimist advanced from 1.2.5 to 1.2.8 only in the member and stayed out of the root manifest. A root-only workspace ran exactly one command. Both rescans returned zero findings with exit code 0.
• Six existing offline CLI fixtures match main for findings, classifications, commands, and exit codes.
• npm ci, npm run lint:tests, and npm run build passed. Changed-test type checking against origin/main passed: the changed test file is clean, with no new errors and 31 existing baseline errors.
• Advisory sync completed with 230,862 records.
• Full suite: npm test -- --watchman=false --maxWorkers=2 --testTimeout=30000 passed all 228 suites, with 3,484 tests passed and three existing skips. The longer timeout accommodates network-dependent tests.

Implemented with AI assistance.

[coderabbitai]

No actionable comments were generated in the recent review.

Summary

Summary by CodeRabbit

• Bug Fixes
  • pnpm install fixes now correctly handle dependencies declared at the workspace root and in individual members, including root-only targets.
  • Root-level dependencies use a workspace-scoped install command, while member dependencies use an appropriately filtered command. Root-only targets no longer receive an unnecessary member-scoped command.
  • Install commands are applied with the correct workspace scope, preventing root and member dependency fixes from being applied to the wrong packages.

pnpm remediation plans now include commands for vulnerable packages declared at both a workspace root and its members. Install execution arguments include the root-level and member-scoped commands where applicable.

Changes: pnpm workspace remediation

Layer / File(s)	Summary
Detect and retain root targets src/remediation/fix-commands.ts	pnpm scans detect workspace mode from an adjacent pnpm-workspace.yaml. Direct targets retain root declarations, mark root targets, and preserve that target when merging.
Generate and apply root and member commands src/remediation/fix-commands.ts, src/utils/fix-runner.ts, tests/remediation/pnpm-root-and-member.test.ts	Command generation emits pnpm add -w for root declarations and member-scoped commands for member declarations. Install arguments and tests cover root-only, member-only, combined, and command-failure cases.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Change: Bug fix · Severity of issue fixed: Medium

Possibly related issues

• OWASP/cve-lite-cli#1582: The change emits pnpm add -w for workspace-root declarations.

Possibly related PRs

• OWASP/cve-lite-cli#1587: This change extends the workspaceRoot marker and root-command handling to targets that also have member declarations.
• OWASP/cve-lite-cli#1387: This change uses the structured argv path to emit multiple pnpm install commands.
• OWASP/cve-lite-cli#1532: This change preserves root workspace membership in remediation plans and install arguments.

Merge Risk: Minimal · up to f179c

The reported root-and-member remediation behavior has no established blocker in the supplied evidence. The change is ready for normal merge checks.

Pre-merge checks: 6 · 1

Failed checks: 1 inconclusive

Check name	Status	Explanation	Resolution
Closes Keyword Matches Scope		I found commands for the root and members, plus the requested root-and-one-member test. The plan also counts the tested finding as covered. Issue #1592 additionally requires the root marker to match t…	Provide the settled root-marker convention from #1532 and #1587, or their code, so I can compare it with workspaceRoot.

Passed checks: 6 passed

Check name	Status	Explanation
Title check		The title follows the required type: short description format, uses the valid fix type, stays lowercase after the colon, and accurately describes the pnpm root-and-member fix.
Linked Issues check		Users with a pnpm root and member declaring the same vulnerable package now receive commands for both locations. I found the workspaceRoot marker retained with member paths, root and member command …
Out of Scope Changes check		I found no unrelated changes. The target metadata, pnpm command generation, apply-runner behavior, and tests all support the #1592 fix or its controls.
Linked Issue		The pull request body references the issue with Closes #1592.
No New Runtime Dependency		Users get no new runtime dependencies from this change. I checked the PR diff and found no changes to package.json.
Description check		The description explains what changed and why, identifies the fix behavior and test coverage, and includes the linked issue. It follows the required template.

Full details: Linked Issues check

Explanation

Users with a pnpm root and member declaring the same vulnerable package now receive commands for both locations. I found the workspaceRoot marker retained with member paths, root and member command generation in buildWorkspaceInstallCommands, and both commands in the apply runner. Coverage and output paths use the resulting command. Tests cover root plus one or more members, dev and production targets, and a failed member command. The root marker uses the convention named in #1592.

Full details: Closes Keyword Matches Scope

Explanation

I found commands for the root and members, plus the requested root-and-one-member test. The plan also counts the tested finding as covered. Issue #1592 additionally requires the root marker to match the convention settled by #1532 and #1587. The PR uses workspaceRoot, but the available evidence only says that #1587 proposed it. I cannot verify the settled convention from the available refs or issue context.

Resolution

Provide the settled root-marker convention from #1532 and #1587, or their code, so I can compare it with workspaceRoot.

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

Share

• X
• Mastodon
• Reddit
• LinkedIn

Recent review info

Automatic approval policy

Commit: f179c38f9ce6.

Setting	Effective value	Source
Automatic approvals	Enabled	Global configuration
Merge risk	Moderate	Global configuration
Security risk	Not set	Global configuration

No overrides matched. The global limits apply.

Not set means no limit was configured after inheritance. Use @coderabbitai configuration to inspect the resolved settings and their sources.

Run configuration

• Source mode: Auto
• Repository: OWASP/cve-lite-cli
• Branch: main
• File: .coderabbit.yaml
• Commit: f6ec794
• Layers: Repository YAML
• Review profile: CHILL
• Plan: Advanced
• Run ID: 666899a9-1edf-4987-bd14-04f7ce908249

Commits

Reviewing files that changed from the base of the PR and between 4da09be and f179c38.

Files selected for processing: 2

• src/remediation/fix-commands.ts
• tests/remediation/pnpm-root-and-member.test.ts

Included review availability: This review used your included allowance. Your plan provides up to 4 included reviews per hour; 2 remain after this review.

[sonukapoor]

Thanks @vianmangal, the fix works. On a real pnpm 11 workspace with two package patterns, root dev plus member lodash@4.17.20 used to print only the member command and leave the root vulnerable (rescan: 1 finding). With your branch it prints pnpm add -w lodash@4.18.1 && pnpm --filter ./packages/m add lodash@4.18.1, both run, and the rescan is clean. Member-only targets are unchanged, a member-only package in the same member stays out of the root install, and --fix runs the same pairs. Merged with current main the suite passes and it builds.

Two things before it can go in:

1. Order with #1587. #1587 is older and covers the root-only -w case, including chain-resolution and parent-update targets, and the two conflict in fix-commands.ts. It goes first; then could you rebase onto it and reuse its isPnpmWorkspaceLockfile instead of the inline check at line 111? After both land, #1587's workspaces.length === 0 && target.workspaceRoot line in fix-runner.ts becomes dead, so please fold the two root paths into one. Let workspaceRoot mean "this target needs a root install", whether or not members also declare the package, so it has one meaning rather than two.
2. Tests for the grouping and root-only paths. Seven of the changes can be reverted without a test failing, and they're all on these two paths. Two cases would pin them:
   • A root+member target and a member-only target in the same member: assert the member-only package isn't installed into the root. Dropping "." from the grouping key at line 1073 passes the whole suite today, but would put minimist into the root.
   • Root-only in a workspace: assert exactly one pnpm add -w and no bare pnpm add, in both the plan and the --fix argv (lines 1110 and fix-runner.ts:216).

Small ones: targets.some(target => target.workspaceRoot) is computed twice around lines 1107-1110, and the new Set at 1074 isn't needed since "." is already filtered out.

[sonukapoor]

Thanks @vianmangal, this covers everything I asked for. The two new cases pin the grouping and the root-only path well: minimist stays out of the root install, and a root-only workspace runs exactly one pnpm add -w in both the plan and --fix. Reverting the source changes one at a time fails these tests in nearly every case. The duplicate check and the Set are gone too. Merged with current main the suite passes and it builds.

The one thing left is the order with #1587, which you've already called out. It's still open, so this waits for it. Once it lands, a rebase onto it that reuses isPnpmWorkspaceLockfile and folds its root-only shortcut into your root path is all that's needed, and I'll take another quick look then.