Skip to content

Commit 46f67b1

Browse files
committed
fix(cli): reject managed-global -g operations on the local CLI
The local CLI never had a vite-plus-managed package store. Routing `vp install/add/remove/update -g` and `vp pm list -g` through the shared `vite_pm_cli::dispatch` either silently ran the project flow (creating a fresh package.json on `vp install -g foo` in an empty directory), silently dropped flags like `--node` and `--dry-run`, or in the codex-flagged Update case mutated the user's local package.json / lockfile when they meant to update a global install. Add a one-shot rejection in the local CLI binding, scoped to exactly the cases the global CLI's `run_package_manager_command` specially handles (Install/Add/Remove/Update with `global=true`, plus `Pm::List -g`). Pass-through `-g` cases that already work the same on both CLIs (`outdated -g`, `why -g`, `pm config get -g`, etc.) are left alone — `is_managed_global()` returns `false` for them. - `crates/vite_pm_cli/src/cli.rs`: new `PackageManagerCommand::is_managed_global` helper. Shared crate stays semantically neutral; the binding decides rejection. - `packages/cli/binding/src/cli/mod.rs`: precheck in `execute_pm_command` returns a friendly error pointing the user at https://viteplus.dev/guide/ before calling `dispatch`. Exit 1 via the binding's existing JS error path. - `crates/vite_pm_cli/src/dispatch.rs`: drop the now-dead `--dry-run` rejection on Remove. Clap requires `--dry-run` alongside `-g`, the binding rejects `-g` first, and the global CLI intercepts `-g` before dispatch — so this arm only ever sees `dry_run: false`. - `crates/vite_pm_cli/src/handlers.rs`: revert the `if options.global` branches in `run_add` / `run_remove` for the same reason; they're unreachable now. - `packages/cli/snap-tests/command-pm-global-rejected/`: new local-only fixture that exercises every rejected case (install/add/remove/update -g, with --node and --dry-run combinations, plus pm ls -g). - `packages/cli/snap-tests/command-remove-pnpm10/snap.txt`: regenerated with the new full-rejection wording in place of the old dispatcher-side --dry-run-only message. snap-tests-global/ unchanged: the global CLI still hits its managed-install path before reaching the binding rejection. Verified end-to-end on the local CLI in an empty /tmp dir: - `vp install -g foo`, `vp add -g foo --node 20`, `vp remove -g foo`, `vp remove -g --dry-run foo`, `vp update -g [foo]`, `vp pm ls -g` all exit 1 with the new error and create no package.json. - `vp pm config get -g registry` (pass-through) still works on both CLIs. - `vp add lodash` (no -g) still creates a project and adds the dep. Closes the second codex adversarial review's high/medium findings on PR #1495.
1 parent 70a9b1a commit 46f67b1

8 files changed

Lines changed: 79 additions & 28 deletions

File tree

crates/vite_pm_cli/src/cli.rs

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -494,6 +494,22 @@ impl PackageManagerCommand {
494494
}
495495
}
496496

497+
/// Whether this invocation hits the vite-plus-managed-global flow on the
498+
/// global CLI. The local CLI binding refuses these cases (it has no
499+
/// managed package store of its own); pass-through `-g` cases like
500+
/// `outdated -g`, `why -g`, and `pm config get -g` return `false` and
501+
/// keep working on both CLIs.
502+
pub fn is_managed_global(&self) -> bool {
503+
match self {
504+
Self::Install { global, .. }
505+
| Self::Add { global, .. }
506+
| Self::Remove { global, .. }
507+
| Self::Update { global, .. } => *global,
508+
Self::Pm(PmCommands::List { global, .. }) => *global,
509+
_ => false,
510+
}
511+
}
512+
497513
/// Determine the save dependency type from CLI flags shared by `Install` and `Add`.
498514
pub fn determine_save_dependency_type(
499515
save_dev: bool,

crates/vite_pm_cli/src/dispatch.rs

Lines changed: 5 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -141,21 +141,14 @@ pub async fn dispatch(
141141
workspace_root,
142142
recursive,
143143
global,
144-
dry_run,
144+
// `--dry-run` is clap-required to coexist with `-g`, and `-g` is
145+
// either intercepted by the global CLI's `run_package_manager_command`
146+
// (managed flow) or rejected by the local CLI binding's
147+
// `execute_pm_command`. Either way, this arm only sees `dry_run: false`.
148+
dry_run: _,
145149
packages,
146150
pass_through_args,
147151
} => {
148-
// `--dry-run` (which clap requires alongside `-g`) is honored by the
149-
// global CLI's vite-plus-managed uninstall flow. The shared
150-
// dispatcher reaches this arm only for non-managed paths
151-
// (project-scoped removes, or local-CLI global removes), and the
152-
// underlying `RemoveCommandOptions` has no preview field. Refuse
153-
// rather than silently performing a real uninstall.
154-
if dry_run {
155-
return Err(Error::UserMessage(
156-
"`--dry-run` is only supported by the globally-installed `vp` CLI for managed packages. Run the command without `--dry-run` to actually remove the package.".into(),
157-
));
158-
}
159152
let options = RemoveCommandOptions {
160153
packages: &packages,
161154
filters: filter.as_deref(),

crates/vite_pm_cli/src/handlers.rs

Lines changed: 3 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -50,14 +50,8 @@ pub async fn run_add(
5050
cwd: &AbsolutePath,
5151
options: &AddCommandOptions<'_>,
5252
) -> Result<ExitStatus, Error> {
53-
let pm = if options.global {
54-
// Global add doesn't need a project; fall back to npm if no package.json
55-
// is present so the underlying PM still has a binary to dispatch to.
56-
build_package_manager_or_npm_default(cwd).await?
57-
} else {
58-
ensure_package_json(cwd).await?;
59-
PackageManager::builder(cwd).build_with_default().await?
60-
};
53+
ensure_package_json(cwd).await?;
54+
let pm = PackageManager::builder(cwd).build_with_default().await?;
6155
Ok(pm.run_add_command(options, cwd).await?)
6256
}
6357

@@ -74,13 +68,7 @@ pub async fn run_remove(
7468
cwd: &AbsolutePath,
7569
options: &RemoveCommandOptions<'_>,
7670
) -> Result<ExitStatus, Error> {
77-
let pm = if options.global {
78-
// Global remove doesn't need a project; fall back to npm if no
79-
// package.json is present.
80-
build_package_manager_or_npm_default(cwd).await?
81-
} else {
82-
build_package_manager(cwd).await?
83-
};
71+
let pm = build_package_manager(cwd).await?;
8472
Ok(pm.run_remove_command(options, cwd).await?)
8573
}
8674

packages/cli/binding/src/cli/mod.rs

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -198,6 +198,15 @@ async fn execute_pm_command(
198198
command: vite_pm_cli::PackageManagerCommand,
199199
cwd: &AbsolutePath,
200200
) -> Result<ExitStatus, Error> {
201+
// `-g`/`--global` operations on install/add/remove/update/`pm list` map to
202+
// a vite-plus-managed package store on the global CLI; the local CLI has
203+
// no such store, so refuse rather than silently doing the wrong thing
204+
// (mutating the project, dropping `--node`, ignoring `--dry-run`, …).
205+
if command.is_managed_global() {
206+
return Err(Error::Anyhow(anyhow::anyhow!(
207+
"Global package operations (`-g`/`--global`) are only supported by the globally-installed `vp` CLI. See https://viteplus.dev/guide/ to install it, then run the same command via the global `vp` binary.",
208+
)));
209+
}
201210
let status = vite_pm_cli::dispatch(cwd, command)
202211
.await
203212
.map_err(|e| Error::Anyhow(anyhow::Error::new(e)))?;
Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
{
2+
"name": "command-pm-global-rejected",
3+
"version": "1.0.0",
4+
"private": true,
5+
"packageManager": "pnpm@10.15.1"
6+
}
Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
[1]> vp install -g testnpm2 # rejected: managed install
2+
error: Global package operations (`-g`/`--global`) are only supported by the globally-installed `vp` CLI. See https://viteplus.dev/guide/ to install it, then run the same command via the global `vp` binary.
3+
4+
[1]> vp install -g testnpm2 --node 20 # rejected: --node implicitly covered
5+
error: Global package operations (`-g`/`--global`) are only supported by the globally-installed `vp` CLI. See https://viteplus.dev/guide/ to install it, then run the same command via the global `vp` binary.
6+
7+
[1]> vp add -g testnpm2 # rejected: managed add
8+
error: Global package operations (`-g`/`--global`) are only supported by the globally-installed `vp` CLI. See https://viteplus.dev/guide/ to install it, then run the same command via the global `vp` binary.
9+
10+
[1]> vp add -g testnpm2 --node 20 # rejected: --node implicitly covered
11+
error: Global package operations (`-g`/`--global`) are only supported by the globally-installed `vp` CLI. See https://viteplus.dev/guide/ to install it, then run the same command via the global `vp` binary.
12+
13+
[1]> vp remove -g testnpm2 # rejected: managed uninstall
14+
error: Global package operations (`-g`/`--global`) are only supported by the globally-installed `vp` CLI. See https://viteplus.dev/guide/ to install it, then run the same command via the global `vp` binary.
15+
16+
[1]> vp remove -g --dry-run testnpm2 # rejected: --dry-run implicitly covered
17+
error: Global package operations (`-g`/`--global`) are only supported by the globally-installed `vp` CLI. See https://viteplus.dev/guide/ to install it, then run the same command via the global `vp` binary.
18+
19+
[1]> vp update -g # rejected: managed update
20+
error: Global package operations (`-g`/`--global`) are only supported by the globally-installed `vp` CLI. See https://viteplus.dev/guide/ to install it, then run the same command via the global `vp` binary.
21+
22+
[1]> vp update -g testnpm2 # rejected: managed update with package
23+
error: Global package operations (`-g`/`--global`) are only supported by the globally-installed `vp` CLI. See https://viteplus.dev/guide/ to install it, then run the same command via the global `vp` binary.
24+
25+
[1]> vp pm ls -g # rejected: managed packages listing
26+
error: Global package operations (`-g`/`--global`) are only supported by the globally-installed `vp` CLI. See https://viteplus.dev/guide/ to install it, then run the same command via the global `vp` binary.
Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
{
2+
"commands": [
3+
"vp install -g testnpm2 # rejected: managed install",
4+
"vp install -g testnpm2 --node 20 # rejected: --node implicitly covered",
5+
"vp add -g testnpm2 # rejected: managed add",
6+
"vp add -g testnpm2 --node 20 # rejected: --node implicitly covered",
7+
"vp remove -g testnpm2 # rejected: managed uninstall",
8+
"vp remove -g --dry-run testnpm2 # rejected: --dry-run implicitly covered",
9+
"vp update -g # rejected: managed update",
10+
"vp update -g testnpm2 # rejected: managed update with package",
11+
"vp pm ls -g # rejected: managed packages listing"
12+
]
13+
}

packages/cli/snap-tests/command-remove-pnpm10/snap.txt

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -98,7 +98,7 @@ Done in <variable>ms using pnpm v<semver>
9898
}
9999

100100
[1]> vp remove -g --dry-run testnpm2 && cat package.json # support remove global package with dry-run
101-
error: `--dry-run` is only supported by the globally-installed `vp` CLI for managed packages. Run the command without `--dry-run` to actually remove the package.
101+
error: Global package operations (`-g`/`--global`) are only supported by the globally-installed `vp` CLI. See https://viteplus.dev/guide/ to install it, then run the same command via the global `vp` binary.
102102

103103
[2]> vp rm --stream foo && should show tips to use pass through arguments when options are not supported
104104
error: Unexpected argument '--stream'

0 commit comments

Comments
 (0)