From e72c2cdf1927cdddd9ffcb128574a9b5c70013ec Mon Sep 17 00:00:00 2001 From: juangaitanv Date: Fri, 12 Jun 2026 14:20:49 +0200 Subject: [PATCH 1/3] Phase 3 lane 1: uv gate + yarn/pnpm named-only wrappers MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Harvested from the install-vuln-gate spike (dfac68e); still public-mode only, no --json. - corgea uv: `uv pip install` / `uv add` / `uv pip sync` gate through `uv pip compile` (--only-binary :all:, temp .in file) so the full resolved set is verdicted; `uv sync` is gated from the nearest-ancestor uv.lock (local/editable stanzas skipped, unparsable lock refuses, --force escape); `uv lock` and everything else passes through; top-level `uv install` gets a did-you-mean - corgea yarn|pnpm: named targets verified (with the loud named-only warning — no safe dry-run); bare installs exec unchecked behind an honest stderr note; bare `yarn` routes through `yarn install` - wrong-package-manager guard: npm/yarn/pnpm cross-checked against lockfiles + the packageManager field (ambiguous indicators stand down, fresh projects don't inherit ancestor lockfiles); pip↔uv cross-checked against uv.lock / requirements files; all suggestions name the corgea-wrapped command; --force bypasses - SKILL.md updated for the new managers and limitations --- PRD.md | 178 ++++++++++++++++++++++++ skills/corgea/SKILL.md | 34 +++-- src/main.rs | 15 ++ src/precheck/detect.rs | 232 +++++++++++++++++++++++++++++++ src/precheck/mod.rs | 183 +++++++++++++++++------- src/precheck/parse.rs | 283 ++++++++++++++++++++++++++++++++++++-- src/precheck/render.rs | 20 ++- src/precheck/tree.rs | 135 +++++++++++++++++- src/precheck/uv.rs | 175 +++++++++++++++++++++++ tests/cli_bare_install.rs | 88 +++++++++++- tests/cli_install.rs | 246 ++++++++++++++++++++++++++++++++- tests/cli_tree.rs | 45 ++++++ tests/cli_uv_sync.rs | 207 ++++++++++++++++++++++++++++ tests/common/mod.rs | 6 + 14 files changed, 1761 insertions(+), 86 deletions(-) create mode 100644 PRD.md create mode 100644 src/precheck/detect.rs create mode 100644 src/precheck/uv.rs create mode 100644 tests/cli_uv_sync.rs diff --git a/PRD.md b/PRD.md new file mode 100644 index 0000000..6006b01 --- /dev/null +++ b/PRD.md @@ -0,0 +1,178 @@ +--- +date: 2026-06-12 +researcher: claude +git_commit: dfac68e +branch: install-vuln-gate +repository: cli +status: draft +tags: [jtbd, spec, install-gate, supply-chain, agents] +last_updated: 2026-06-12 +last_updated_by: claude +--- + +# Gate Package Installs + +**Date**: 2026-06-12 +**Researcher**: claude +**Git Commit**: dfac68e +**Branch**: install-vuln-gate + +## Job Statement + +**Full JTBD**: When a coding agent or developer is about to install a package, +I want the install to pass through a gate that blocks vulnerable, malicious, +or suspiciously new versions and steers to a safe one, so I can trust that +development — especially autonomous, agent-driven development — never +introduces a supply-chain compromise. + +**Trigger Context**: Agents install packages autonomously with nobody +watching. Developers add dependencies with no check at the decision moment. +Malware campaigns spread through registries in hours, faster than any CI +scan. Security teams have no control point at install time. + +**Success Indicators**: +- An agent hits a block, reads the refusal, and installs the safe version — + no human in the loop. +- Gated installs catch real vulnerable/malicious packages; `--force` stays rare. +- Teams and agent sessions gate installs weekly — adoption, not a demo. +- End state: with a token, nothing unverifiable gets through (fail-closed). + +## Why restart + +The previous attempt (branch `install-vuln-gate`, 60+ commits, ~10.7k lines) +built the right thing but is unreviewable as one PR. Three things ate it: + +1. **Scope creep to the full tree.** Named-target gating grew into resolving + the entire would-install set per manager — dry-runs, lockfiles, bare + installs — each with its own quirks. +2. **Edge-case hardening.** Fail-open/fail-closed modes, retries, yanked + releases, PEP 668, constraint files, JSON stdout purity — correctness + tails with no phase boundary to stop at. +3. **Test infra weight.** The vuln-api stub, integration harness, and + fixtures became a project inside the project, built incidentally. + +The restart keeps the learnings and the proven modules, and lands the work as +one PR per phase, each with explicit exit criteria. + +**Process rule**: one phase = one PR. A phase that can't land as a reviewable +PR is two phases. + +## Phases + +### Phase 0: Foundation — vuln-api contract + test harness + +**Scope**: The vuln-api client and its versioned contract (clean / vulnerable +/ malware / unknown verdicts, remediation data). In-process test stub, gated +out of release builds. Shared integration-test scaffold. Deterministic +staging targets documented (`axios@0.21.0`, `minimist@0.0.8`, +`node-fetch@2.6.0`, `mezzanine==6.0.0`). + +**Not Included**: Any user-facing command. Auth/token handling. Retries. + +**Entry Criteria**: Staging worker (`cve-worker-staging`) serving +deterministic verdicts. + +**Exit Criteria**: Contract tests green against both the stub and staging. +Another phase can write an integration test in under 20 lines of setup. + +**Harvest**: `src/vuln_api/mod.rs`, `src/vuln_api_stub/mod.rs`, +`tests/common/mod.rs`, `tests/fixtures/vuln_api/`. + +### Phase 1: Core gate (SHIP) — `corgea pip|npm install ` + +**Scope**: Install-verb detection behind global flags. Named-spec parsing +(exact pins and ranges). Registry resolution (PyPI, npm). Two independent +blocks: recency (`-t`, default `2d`) and vuln verdict. Refusal output built +for agent self-correction: per-advisory `fixed in ` lines and a +`→ safe version: @` steer. `--force` (override everything), +`--no-fail` (demote recency only). Git/URL/path specs pass through with a +note, never blocked. Non-install subcommands pass straight through. Public +mode only: no token, lookup outages warn and continue (fail-open). +`skills/corgea/SKILL.md` section, including the limitations doc (wrapper, not +an enforcement boundary). + +**Not Included**: Transitive/tree resolution. Bare installs, lockfiles, +`npm ci`. `-r requirements.txt` parsing (noted, not gated). `--json`. Token +auth and fail-closed mode. yarn/pnpm/uv. Retry logic. + +**Entry Criteria**: Phase 0 merged. + +**Exit Criteria**: Dogfood pass — a real agent session with the skill +installed hits a staging-target block and self-corrects to the safe version +unprompted. All deterministic staging targets block with exit 1. + +**Harvest**: `src/precheck/parse.rs`, `detect.rs`, `verdict.rs`, `render.rs` +(trimmed to named-target paths), `src/verify_deps/registry.rs`. + +### Phase 2: Depth (SHIP) — the full would-install set + +**Scope**: pip tree via `pip install --dry-run`; npm tree via isolated +`npm install --package-lock-only` in a temp dir. Bare `npm install` gated +from the nearest `package.json`; `npm ci` gated from the lockfile. +Transitive findings labeled by provenance. Honest named-only fallback with a +printed warning when a dry-run fails or `--prefix`/`-g` redirects the root. +`-r requirements.txt` fallback parsing. Bounded verdict pool (fixed at 8). + +**Not Included**: uv/yarn/pnpm. `--json`. Auth. + +**Entry Criteria**: Phase 1 shipped and dogfooding. + +**Exit Criteria**: A vulnerable transitive dep blocks the install. A +vulnerable lockfile blocks `npm ci`. Fallback warnings fire when and only +when the tree pass didn't run. + +**Harvest**: `src/precheck/tree.rs`, `tests/cli_tree.rs`, +`tests/cli_bare_install.rs`, `tests/cli_npm_ci.rs`. + +### Phase 3: Breadth + guarantee + +**Scope**: Three independent lanes, each its own PR: +1. **uv** — `uv pip install`/`uv add`/`uv pip sync` via `uv pip compile`; + `uv sync` from `uv.lock`. yarn/pnpm named-only with honest ungated notes. +2. **Machine output** — `--json` (stdout purity, `verdict_mode`, `tree` + object, `remediation` field). +3. **Org guarantee** — authenticated fail-closed mode, custom-URL token + opt-in, transient-failure retries, PEP 668 refusal, yanked-release + handling. + +**Not Included**: Org policy config, telemetry, registry allow-listing — +future work, separate PRD. + +**Entry Criteria**: Phase 2 shipped. + +**Exit Criteria**: Per lane. Lane 3's bar: with a token, an unverifiable +package or a vuln-api outage blocks the install. + +**Harvest**: `src/precheck/uv.rs`, `tests/cli_uv_sync.rs`, +`tests/cli_verdict.rs` (auth modes), JSON paths in `render.rs`. + +## Known dead ends — do not rebuild + +Built and deliberately removed in the previous attempt: +- npm audit warn-only second opinion (`ccceb7a`) +- Steer re-verification pass (`e62399c`) +- `--concurrency` flag — fixed pool of 8 instead (`bfc8cf1`) +- Persisted `vuln_api_url` config — env var only (`204fb47`) +- Standalone vuln-api-stub binary — in-process stub instead (`b6c2e83`) + +## Open Questions + +- Recency default: `2d` carried over from the spike — validate against real + release cadence data before P1 ships. +- Does `--json` pull forward into P1 if agent dogfooding wants structured + output over refusal text? +- Staging worker is the current default vuln-api endpoint — when does the + production worker take over, and who owns its seed data? +- Telemetry (catch rates, `--force` usage) — needed to measure success + indicators, but where does it report? Separate PRD. + +## References + +- Previous attempt: branch `install-vuln-gate` (head `dfac68e`) — harvest + source and design reference. +- Agent contract: `skills/corgea/SKILL.md` (limitations section at + `dfac68e`). +- Staging verdicts: `https://cve-worker-staging.corgea.workers.dev` + (source: `/Users/juan/Code/corgea/vuln-api`). +- Flag validation pattern to mirror: `src/main.rs` (blast-only flag + rejection). diff --git a/skills/corgea/SKILL.md b/skills/corgea/SKILL.md index 7c9d2f4..c010563 100644 --- a/skills/corgea/SKILL.md +++ b/skills/corgea/SKILL.md @@ -131,7 +131,7 @@ Agent environments default to compact TSV; force output with `--format human|age Notes: `deps scan --out-format table|json|sarif` is the report/export selector; do not combine it with `deps scan --format`. -### Install Wrappers — `corgea pip|npm ` +### Install Wrappers — `corgea pip|npm|yarn|pnpm|uv ` Run a package manager through Corgea's install gate. Install commands with named targets are resolved against the public registry first, then gated @@ -148,19 +148,27 @@ finds it — nearest ancestor) is gated too: the full lockfile-resolved tree is verdicted, so a vulnerable lockfile blocks. `npm ci` (and aliases) is gated from the project lockfile directly. -The vuln check covers the **full would-install set**, not just the named -targets: `pip` and `npm` resolve the complete tree (named + transitive) via a -safe dry-run (`pip install --dry-run …`; an isolated -`npm install --package-lock-only` in a temp dir, never touching your -lockfile); every resolved package is verdicted, so a flagged **transitive** -dependency blocks the install too, labeled by provenance (`(transitive)`, -`(from requirements)`, `(already in package.json)`, `(locked)`). Whenever a -dry-run fails or an npm flag redirects the project root (`--prefix`, `-g`), -the gate falls back to named-only and prints +The vuln check covers the **full would-install set** where the manager has a +safe resolver, not just the named targets: `pip` and `npm` resolve the +complete tree (named + transitive) via a safe dry-run +(`pip install --dry-run …`; an isolated `npm install --package-lock-only` in +a temp dir, never touching your lockfile), and `uv pip install` / `uv add` / +`uv pip sync` resolve theirs via `uv pip compile`; every resolved package is +verdicted, so a flagged **transitive** dependency blocks the install too, +labeled by provenance (`(transitive)`, `(from requirements)`, +`(already in package.json)`, `(locked)`). `uv sync` is gated from `uv.lock` +(found like uv finds it — nearest ancestor). `yarn` and `pnpm` have no safe +dry-run, so they verify the named targets only; bare `yarn`/`pnpm` installs +run unchecked after a stderr note +(`note: bare ' ' is not gated …`). Whenever a dry-run fails or an +npm flag redirects the project root (`--prefix`, `-g`), the gate falls back +to named-only and prints `warning: transitive dependencies not checked (…); only named packages were verified.` -— for pip, entries of `-r requirements.txt` files are still parsed and +— for pip/uv, entries of `-r requirements.txt` files are still parsed and verified in that fallback. Verdict requests run in a bounded pool -(8 parallel). +(8 parallel). Running the wrong manager for a project (npm in a pnpm +project, pip in a uv project, …) is refused with a +`Did you mean `corgea …`?` suggestion; `--force` bypasses that guard too. Wrapper flags (`--force`, `--no-fail`, `-t`) are read between the manager name and the install verb (`corgea npm --force install x`); flags after the @@ -201,6 +209,8 @@ The gate is a wrapper, not an enforcement boundary. By design it cannot catch: - **Named-only fallback** — when a dry-run fails (old pip, broken resolution) or `--prefix`/`-g` redirects npm's root, transitive dependencies install unchecked behind the printed warning. +- **Ungated managers** — bare `yarn`/`pnpm` installs run unchecked (see the + bare-install note above); only their named targets are verified. Hard enforcement needs org-level controls — lockfile review, registry allow-listing — alongside the wrapper. diff --git a/src/main.rs b/src/main.rs index ea4ab1c..d7c0fcd 100644 --- a/src/main.rs +++ b/src/main.rs @@ -201,8 +201,14 @@ enum Commands { }, /// Wrap `npm` commands: gate install targets on recency + vuln verdicts, then run npm. Npm(InstallWrapArgs), + /// Wrap `yarn` commands: gate install targets on recency + vuln verdicts, then run yarn. + Yarn(InstallWrapArgs), + /// Wrap `pnpm` commands: gate install targets on recency + vuln verdicts, then run pnpm. + Pnpm(InstallWrapArgs), /// Wrap `pip` commands: gate install targets on recency + vuln verdicts, then run pip. Pip(InstallWrapArgs), + /// Wrap `uv` commands: gate install targets on recency + vuln verdicts, then run uv. + Uv(InstallWrapArgs), } /// Shared flags for the install-wrapper subcommands (`corgea npm|pip`). @@ -560,9 +566,18 @@ fn main() { Some(Commands::Npm(args)) => { run_install_wrap_command(corgea::precheck::PackageManager::Npm, args) } + Some(Commands::Yarn(args)) => { + run_install_wrap_command(corgea::precheck::PackageManager::Yarn, args) + } + Some(Commands::Pnpm(args)) => { + run_install_wrap_command(corgea::precheck::PackageManager::Pnpm, args) + } Some(Commands::Pip(args)) => { run_install_wrap_command(corgea::precheck::PackageManager::Pip, args) } + Some(Commands::Uv(args)) => { + run_install_wrap_command(corgea::precheck::PackageManager::Uv, args) + } None => { if let Some(message) = corgea::precheck::pip3_alias_message(&cli.args) { eprintln!("{message}"); diff --git a/src/precheck/detect.rs b/src/precheck/detect.rs new file mode 100644 index 0000000..a058358 --- /dev/null +++ b/src/precheck/detect.rs @@ -0,0 +1,232 @@ +//! Package-manager/project detection: wrong-manager guidance messages. + +use std::path::Path; + +use super::{parse, PackageManager}; + +pub(super) fn wrong_package_manager_message( + manager: PackageManager, + rest: &[String], + parsed: &parse::ParsedInstall, +) -> Option { + let cwd = &std::env::current_dir().ok()?; + let expected = match manager { + PackageManager::Npm | PackageManager::Yarn | PackageManager::Pnpm => { + let expected = detect_node_manager_from(cwd)?; + (expected != manager).then_some(expected)? + } + PackageManager::Pip if detect_uv_project_from(cwd) => PackageManager::Uv, + PackageManager::Uv if detect_pip_project_from(cwd) => PackageManager::Pip, + _ => return None, + }; + + let suggestion = suggested_install_command(expected, rest, parsed); + Some(format!( + "error: this project appears to use {}, but you ran {}.\nDid you mean `{suggestion}`?", + expected.binary_name(), + manager.binary_name() + )) +} + +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +enum ProjectManagerDetection { + None, + Ambiguous, + Found(PackageManager), +} + +fn detect_node_manager_from(start: &Path) -> Option { + for dir in start.ancestors() { + match detect_node_manager_in_dir(dir) { + ProjectManagerDetection::Found(manager) => return Some(manager), + ProjectManagerDetection::Ambiguous => return None, + ProjectManagerDetection::None => {} + } + // A `package.json` marks the project root (npm/yarn/pnpm scope + // their own discovery the same way). A project with no manager + // indicators of its own must not inherit a stray ancestor lockfile + // — that would hard-refuse installs in every fresh project under it. + if dir.join("package.json").is_file() { + return None; + } + } + None +} + +fn detect_node_manager_in_dir(dir: &Path) -> ProjectManagerDetection { + match package_json_manager(dir) { + ProjectManagerDetection::None => {} + found => return found, + } + + let mut found = Vec::new(); + if dir.join("pnpm-lock.yaml").is_file() { + found.push(PackageManager::Pnpm); + } + if dir.join("yarn.lock").is_file() { + found.push(PackageManager::Yarn); + } + if dir.join("package-lock.json").is_file() || dir.join("npm-shrinkwrap.json").is_file() { + found.push(PackageManager::Npm); + } + + match found.as_slice() { + [] => ProjectManagerDetection::None, + [manager] => ProjectManagerDetection::Found(*manager), + _ => ProjectManagerDetection::Ambiguous, + } +} + +/// `packageManager`-field detection. Missing/unparsable `package.json` and a +/// missing field both fall through to lockfile detection (`None`). +fn package_json_manager(dir: &Path) -> ProjectManagerDetection { + let json: Option = std::fs::read_to_string(dir.join("package.json")) + .ok() + .and_then(|raw| serde_json::from_str(&raw).ok()); + let Some(package_manager) = json + .as_ref() + .and_then(|j| j.get("packageManager")) + .and_then(|v| v.as_str()) + else { + return ProjectManagerDetection::None; + }; + parse_node_package_manager(package_manager) + .map(ProjectManagerDetection::Found) + .unwrap_or(ProjectManagerDetection::Ambiguous) +} + +fn parse_node_package_manager(raw: &str) -> Option { + let name = raw.trim().split('@').next().unwrap_or("").trim(); + match name { + "npm" => Some(PackageManager::Npm), + "yarn" => Some(PackageManager::Yarn), + "pnpm" => Some(PackageManager::Pnpm), + _ => None, + } +} + +/// Walk up looking for `uv.lock`, but stop at the nearest Python project +/// boundary (a `pyproject.toml` or requirements file without a `uv.lock` +/// beside it) — symmetric with [`detect_pip_project_from`], so a stray +/// `~/uv.lock` can't condemn every pip project beneath it. +fn detect_uv_project_from(start: &Path) -> bool { + for dir in start.ancestors() { + if dir.join("uv.lock").is_file() { + return true; + } + if dir.join("pyproject.toml").is_file() || has_requirements_file(dir) { + return false; + } + } + false +} + +fn detect_pip_project_from(start: &Path) -> bool { + start + .ancestors() + .take_while(|dir| !dir.join("pyproject.toml").is_file() && !dir.join("uv.lock").is_file()) + .any(has_requirements_file) +} + +fn has_requirements_file(dir: &Path) -> bool { + let Ok(entries) = std::fs::read_dir(dir) else { + return false; + }; + entries.filter_map(Result::ok).any(|entry| { + let name = entry.file_name(); + let name = name.to_string_lossy(); + entry.path().is_file() + && ((name.starts_with("requirements") + && (name.ends_with(".txt") || name.ends_with(".in"))) + || name.ends_with("-requirements.txt")) + }) +} + +fn suggested_install_command( + expected: PackageManager, + rest: &[String], + parsed: &parse::ParsedInstall, +) -> String { + let mut parts = vec!["corgea".to_string(), expected.binary_name().to_string()]; + match expected { + PackageManager::Npm => parts.push("install".to_string()), + PackageManager::Yarn | PackageManager::Pnpm => { + if parsed.targets.is_empty() && parsed.requirements_files.is_empty() { + parts.push("install".to_string()); + } else { + parts.push("add".to_string()); + } + } + PackageManager::Uv => { + if is_plain_pip_target_install(rest, parsed) { + parts.push("add".to_string()); + parts.extend(parsed.targets.iter().map(|target| target.display.clone())); + return parts.join(" "); + } + parts.push("pip".to_string()); + parts.push("install".to_string()); + } + PackageManager::Pip => parts.push("install".to_string()), + } + parts.extend(rest.iter().cloned()); + parts.join(" ") +} + +fn is_plain_pip_target_install(rest: &[String], parsed: &parse::ParsedInstall) -> bool { + !parsed.targets.is_empty() + && parsed.requirements_files.is_empty() + && rest.len() == parsed.targets.len() + && rest + .iter() + .zip(&parsed.targets) + .all(|(arg, target)| arg == &target.display) +} + +#[cfg(test)] +mod tests { + use super::*; + + fn touch(path: &Path) { + std::fs::write(path, "").expect("write marker file"); + } + + #[test] + fn node_walk_stops_at_the_project_boundary() { + // A stray ancestor lockfile must not condemn a fresh project that + // has its own package.json but no manager indicators yet. + let root = tempfile::tempdir().expect("tempdir"); + touch(&root.path().join("package-lock.json")); + let project = root.path().join("newapp"); + std::fs::create_dir(&project).expect("mkdir"); + std::fs::write(project.join("package.json"), "{}").expect("write manifest"); + + assert_eq!(detect_node_manager_from(&project), None); + + // Without its own package.json the walk still reaches the ancestor. + let bare = root.path().join("scratch"); + std::fs::create_dir(&bare).expect("mkdir"); + assert_eq!(detect_node_manager_from(&bare), Some(PackageManager::Npm)); + } + + #[test] + fn uv_walk_stops_at_a_nearer_python_project() { + // A pip project (requirements/pyproject, no uv.lock) must not be + // blamed for a stray uv.lock further up. + let root = tempfile::tempdir().expect("tempdir"); + touch(&root.path().join("uv.lock")); + let pip_project = root.path().join("legacy"); + std::fs::create_dir(&pip_project).expect("mkdir"); + touch(&pip_project.join("requirements.txt")); + + assert!(!detect_uv_project_from(&pip_project)); + + // The uv root itself (uv.lock beside pyproject.toml) still detects. + touch(&root.path().join("pyproject.toml")); + assert!(detect_uv_project_from(root.path())); + + // And a plain subdirectory of the uv project still walks up to it. + let sub = root.path().join("src"); + std::fs::create_dir(&sub).expect("mkdir"); + assert!(detect_uv_project_from(&sub)); + } +} diff --git a/src/precheck/mod.rs b/src/precheck/mod.rs index b26c708..211a0d1 100644 --- a/src/precheck/mod.rs +++ b/src/precheck/mod.rs @@ -1,4 +1,5 @@ -//! Install wrappers: `corgea npm`, `corgea pip`. +//! Install wrappers: `corgea npm`, `corgea yarn`, `corgea pnpm`, +//! `corgea pip`, `corgea uv`. //! //! Wraps an install command from a supported package manager, resolves what //! the package manager *would* install against the public registry, and @@ -13,10 +14,12 @@ //! Verdict lookups are public and fail open: a vuln-api outage warns and the //! install continues. +mod detect; mod exec; mod parse; mod render; mod tree; +mod uv; mod verdict; #[cfg(test)] @@ -31,14 +34,20 @@ use chrono::Utc; #[derive(Debug, Clone, Copy, PartialEq, Eq)] pub enum PackageManager { Npm, + Yarn, + Pnpm, Pip, + Uv, } impl PackageManager { pub fn binary_name(self) -> &'static str { match self { PackageManager::Npm => "npm", + PackageManager::Yarn => "yarn", + PackageManager::Pnpm => "pnpm", PackageManager::Pip => "pip", + PackageManager::Uv => "uv", } } @@ -47,15 +56,20 @@ impl PackageManager { pub fn is_install_subcommand(self, sub: &str) -> bool { match self { PackageManager::Npm => matches!(sub, "install" | "i" | "add"), + PackageManager::Yarn => matches!(sub, "add" | "install"), + PackageManager::Pnpm => matches!(sub, "add" | "install" | "i"), PackageManager::Pip => matches!(sub, "install"), + PackageManager::Uv => false, } } /// vuln-api ecosystem for this manager's registry. pub fn ecosystem(self) -> crate::vuln_api::Ecosystem { match self { - PackageManager::Npm => crate::vuln_api::Ecosystem::Npm, - PackageManager::Pip => crate::vuln_api::Ecosystem::Pypi, + PackageManager::Npm | PackageManager::Yarn | PackageManager::Pnpm => { + crate::vuln_api::Ecosystem::Npm + } + PackageManager::Pip | PackageManager::Uv => crate::vuln_api::Ecosystem::Pypi, } } @@ -304,7 +318,17 @@ impl PrecheckReport { /// `["install", "axios@^1.0.0", "--save-dev"]`. An empty `cmd` execs the /// package manager with no arguments. pub fn run_install(manager: PackageManager, cmd: &[String], opts: PrecheckOptions) -> i32 { + if manager == PackageManager::Uv { + return uv::run_uv(cmd, opts); + } + if cmd.is_empty() { + // Bare `yarn` IS `yarn install` — route it through the install + // path so the bare-install note prints instead of a silent exec. + if manager == PackageManager::Yarn { + let install = ["install".to_string()]; + return run_install(manager, &install, opts); + } return exec::exec_command(manager.binary_name(), &[]); } @@ -353,6 +377,16 @@ pub fn run_install(manager: PackageManager, cmd: &[String], opts: PrecheckOption warn_registry_override(manager, rest); + // Project guard. `--force` (documented as overriding every block) is + // the escape hatch — a stray ancestor lockfile must not leave the + // command permanently refused. + if !opts.force { + if let Some(message) = detect::wrong_package_manager_message(manager, rest, &parsed) { + eprintln!("{message}"); + return 1; + } + } + run_parsed_install( manager, subcommand, @@ -418,8 +452,10 @@ fn unsupported_pip_add_message(rest: &[String]) -> String { /// allow-listing is future work, separate PRD. fn warn_registry_override(manager: PackageManager, rest: &[String]) { let flags: &[&str] = match manager { - PackageManager::Npm => &["--registry"], - PackageManager::Pip => &["-i", "--index-url", "--extra-index-url"], + PackageManager::Npm | PackageManager::Yarn | PackageManager::Pnpm => &["--registry"], + PackageManager::Pip | PackageManager::Uv => { + &["-i", "--index-url", "--extra-index-url", "--default-index"] + } }; if let Some(flag) = rest.iter().find(|a| { flags @@ -478,8 +514,11 @@ fn run_parsed_install( } if parsed.targets.is_empty() && !tree_eligible { - // A `-r requirements.txt` install with verdicts disabled is only - // noted; a truly bare install has nothing to note at all. + // Only a truly bare install gets the bare note. A `-r requirements.txt` + // install is covered by `requirements_note`. + if bare_install { + render::bare_install_note(manager, subcommand_label); + } render::requirements_note(&parsed); return exec(); } @@ -535,48 +574,29 @@ fn run_parsed_install( report_and_exec(&report, &opts, exec) } -/// `npm ci` (and aliases): installs the project lockfile exactly as -/// written, so the gate verdicts the lockfile-pinned set directly — no -/// dry-run needed. Recency isn't checked — locked versions aren't newly -/// chosen by this command; the verdict pass is the gate. Without a project -/// or lockfile npm errors on its own; the gate just execs. -fn run_npm_ci(subcommand: &str, rest: &[String], opts: PrecheckOptions) -> i32 { - let exec = || exec::exec_install_with_args(PackageManager::Npm, subcommand, rest); - +/// Gate a lockfile-pinned install (`npm ci`, `uv sync`): verdict every +/// locked package. Recency isn't checked — locked versions aren't newly +/// chosen by this command; the verdict pass is the gate. +fn run_locked_install( + manager: PackageManager, + subcommand: &str, + original_args: Vec, + lock: Result, String>, + opts: &PrecheckOptions, + exec: impl FnOnce() -> i32, +) -> i32 { let Some(cfg) = &opts.verdict else { + // Direct callers may still disable verdicts completely. return exec(); }; - // A root-redirect flag (`--prefix ../other`, `-C ../other`) makes npm ci - // install a DIFFERENT project's lockfile than the CWD one we'd verdict, so - // verifying the CWD lockfile would pass on the wrong project. Fail closed - // unless `--force`. - if !opts.force { - if let Some(flag) = tree::npm_root_redirect_flag(rest) { - eprintln!( - "error: cannot verify 'npm {subcommand}' with '{flag}': it installs a redirected project's lockfile, not this one (pass --force to proceed unchecked)" - ); - return 1; - } - } - let Some(root) = tree::npm_project_root() else { - return exec(); - }; - // npm-shrinkwrap.json takes precedence over package-lock.json. - let Some(lock_path) = ["npm-shrinkwrap.json", "package-lock.json"] - .iter() - .map(|n| root.join(n)) - .find(|p| p.is_file()) - else { - return exec(); - }; - - let lock = std::fs::read_to_string(&lock_path) - .map_err(|e| format!("read {}: {e}", lock_path.display())) - .and_then(|content| tree::parse_npm_lockfile(&content)); let jobs = match lock { Ok(jobs) => jobs, Err(e) if opts.force => { - eprintln!("warning: cannot verify 'npm {subcommand}' ({e}); proceeding under --force"); + eprintln!( + "warning: cannot verify '{} {}' ({e}); proceeding under --force", + manager.binary_name(), + subcommand + ); return exec(); } Err(e) => { @@ -585,14 +605,16 @@ fn run_npm_ci(subcommand: &str, rest: &[String], opts: PrecheckOptions) -> i32 { // means there is no report to feed the predicate, so the gate // refuses directly (--force above is the only escape). eprintln!( - "error: cannot verify 'npm {subcommand}': {e} (pass --force to proceed unchecked)" + "error: cannot verify '{} {}': {e} (pass --force to proceed unchecked)", + manager.binary_name(), + subcommand ); return 1; } }; let resolved_count = jobs.len(); - let results = verdict::verdict_pool(jobs, cfg, PackageManager::Npm); + let results = verdict::verdict_pool(jobs, cfg, manager); let transitive = results .into_iter() .map(|(pkg, verdict)| TreeOutcome { @@ -603,9 +625,9 @@ fn run_npm_ci(subcommand: &str, rest: &[String], opts: PrecheckOptions) -> i32 { }) .collect(); let report = PrecheckReport { - manager: PackageManager::Npm, + manager, subcommand: subcommand.to_string(), - original_args: rest.to_vec(), + original_args, outcomes: Vec::new(), threshold: opts.threshold, tree: Some(TreeReport::Full { @@ -615,7 +637,54 @@ fn run_npm_ci(subcommand: &str, rest: &[String], opts: PrecheckOptions) -> i32 { bare_install: true, }; - report_and_exec(&report, &opts, exec) + report_and_exec(&report, opts, exec) +} + +/// `npm ci` (and aliases): installs the project lockfile exactly as +/// written, so the gate verdicts the lockfile-pinned set directly — no +/// dry-run needed. Without a project or lockfile npm errors on its own; +/// the gate just execs. +fn run_npm_ci(subcommand: &str, rest: &[String], opts: PrecheckOptions) -> i32 { + let exec = || exec::exec_install_with_args(PackageManager::Npm, subcommand, rest); + + if opts.verdict.is_none() { + return exec(); + } + // A root-redirect flag (`--prefix ../other`, `-C ../other`) makes npm ci + // install a DIFFERENT project's lockfile than the CWD one we'd verdict, so + // verifying the CWD lockfile would pass on the wrong project. Fail closed + // unless `--force`. + if !opts.force { + if let Some(flag) = tree::npm_root_redirect_flag(rest) { + eprintln!( + "error: cannot verify 'npm {subcommand}' with '{flag}': it installs a redirected project's lockfile, not this one (pass --force to proceed unchecked)" + ); + return 1; + } + } + let Some(root) = tree::npm_project_root() else { + return exec(); + }; + // npm-shrinkwrap.json takes precedence over package-lock.json. + let Some(lock_path) = ["npm-shrinkwrap.json", "package-lock.json"] + .iter() + .map(|n| root.join(n)) + .find(|p| p.is_file()) + else { + return exec(); + }; + + let lock = std::fs::read_to_string(&lock_path) + .map_err(|e| format!("read {}: {e}", lock_path.display())) + .and_then(|content| tree::parse_npm_lockfile(&content)); + run_locked_install( + PackageManager::Npm, + subcommand, + rest.to_vec(), + lock, + &opts, + exec, + ) } /// One verdict job (`requested: true`) per named resolved target, in @@ -697,7 +766,9 @@ fn requirements_fallback_outcomes( opts: &PrecheckOptions, now: &chrono::DateTime, ) -> Vec { - if manager != PackageManager::Pip || parsed.requirements_files.is_empty() { + if !matches!(manager, PackageManager::Pip | PackageManager::Uv) + || parsed.requirements_files.is_empty() + { return Vec::new(); } @@ -770,6 +841,13 @@ mod tests { assert!(PackageManager::Npm.is_install_subcommand("add")); assert!(!PackageManager::Npm.is_install_subcommand("update")); + assert!(PackageManager::Yarn.is_install_subcommand("add")); + assert!(PackageManager::Yarn.is_install_subcommand("install")); + + assert!(PackageManager::Pnpm.is_install_subcommand("add")); + assert!(PackageManager::Pnpm.is_install_subcommand("install")); + assert!(PackageManager::Pnpm.is_install_subcommand("i")); + assert!(PackageManager::Pip.is_install_subcommand("install")); assert!(!PackageManager::Pip.is_install_subcommand("freeze")); } @@ -827,7 +905,10 @@ mod tests { fn ecosystem_mapping() { use crate::vuln_api::Ecosystem; assert_eq!(PackageManager::Pip.ecosystem(), Ecosystem::Pypi); + assert_eq!(PackageManager::Uv.ecosystem(), Ecosystem::Pypi); assert_eq!(PackageManager::Npm.ecosystem(), Ecosystem::Npm); + assert_eq!(PackageManager::Yarn.ecosystem(), Ecosystem::Npm); + assert_eq!(PackageManager::Pnpm.ecosystem(), Ecosystem::Npm); } #[test] @@ -837,6 +918,10 @@ mod tests { PackageManager::Pip.normalize_name("Flask_Cors"), "flask-cors" ); + assert_eq!( + PackageManager::Uv.normalize_name("zope.interface"), + "zope-interface" + ); assert_eq!(PackageManager::Pip.normalize_name("a__b"), "a-b"); // npm names are case-sensitive and pass through verbatim. assert_eq!(PackageManager::Npm.normalize_name("Left_Pad"), "Left_Pad"); diff --git a/src/precheck/parse.rs b/src/precheck/parse.rs index 68e0b2e..802cadd 100644 --- a/src/precheck/parse.rs +++ b/src/precheck/parse.rs @@ -22,6 +22,23 @@ pub struct ParsedInstall { pub allow_prerelease: bool, } +/// `pip install` / `uv pip install` argument list (everything after the +/// verb). Carries the `--pre` flag through so prerelease resolution applies +/// to both pip and uv pip install. +pub fn parse_pip_install_args(args: &[String]) -> Result { + let mut parsed = build_parsed_install(extract_pip_positionals(args)?, parse_pypi_spec); + parsed.allow_prerelease = pip_allows_prerelease(args); + Ok(parsed) +} + +/// `uv add` argument list (everything after `add`). +pub fn parse_pypi_positionals_args(args: &[String]) -> ParsedInstall { + build_parsed_install( + extract_node_positionals(PackageManager::Uv, args), + parse_pypi_spec, + ) +} + fn build_parsed_install( positionals: PositionalSplit, parse_spec: impl Fn(&str) -> InstallTarget, @@ -78,18 +95,66 @@ pub fn parse_install_args( args: &[String], ) -> Result { match manager { - PackageManager::Pip => { - let mut parsed = build_parsed_install(extract_pip_positionals(args)?, parse_pypi_spec); - parsed.allow_prerelease = pip_allows_prerelease(args); - Ok(parsed) - } - PackageManager::Npm => { + PackageManager::Pip => parse_pip_install_args(args), + PackageManager::Npm | PackageManager::Yarn | PackageManager::Pnpm => { let default_tag = npm_default_tag(args); Ok(build_parsed_install( extract_node_positionals(manager, args), |raw| parse_npm_spec(raw, default_tag.as_deref()), )) } + PackageManager::Uv => unreachable!("uv uses classify_uv_command"), + } +} + +/// Install-shaped `uv` invocations we know how to verify. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum UvCommand<'a> { + Passthrough, + PipInstall { + install_args: &'a [String], + }, + /// `uv pip sync reqs.txt` — installs exactly the given requirements + /// set; gated like `uv pip install -r reqs.txt`. + PipSync { + sync_args: &'a [String], + }, + Add { + add_args: &'a [String], + }, + /// `uv sync` — installs the locked project environment; gated from + /// `uv.lock`. (`uv lock` stays passthrough: it installs nothing.) + Sync, +} + +pub fn classify_uv_command(cmd: &[String]) -> UvCommand<'_> { + match cmd.first().map(String::as_str) { + Some("pip") if matches!(cmd.get(1).map(String::as_str), Some("install" | "i")) => { + UvCommand::PipInstall { + install_args: &cmd[2..], + } + } + Some("pip") if cmd.get(1).map(String::as_str) == Some("sync") => UvCommand::PipSync { + sync_args: &cmd[2..], + }, + Some("add") => UvCommand::Add { + add_args: &cmd[1..], + }, + Some("sync") => UvCommand::Sync, + _ => UvCommand::Passthrough, + } +} + +/// `uv pip sync` argument list: positionals are requirements files, not +/// package specs. +pub fn parse_pip_sync_args(args: &[String]) -> ParsedInstall { + let split = extract_node_positionals(PackageManager::Uv, args); + let mut requirements_files = split.requirements_files; + requirements_files.extend(split.specs.iter().map(PathBuf::from)); + ParsedInstall { + targets: Vec::new(), + requirements_files, + allow_prerelease: false, } } @@ -298,7 +363,9 @@ struct PositionalSplit { /// The fallback heuristic in [`skip_unknown_flag`] only skips URL/path-like /// values, so a bare-word value (`-w my-workspace`) would otherwise parse — /// and get verified or blocked — as a package spec. Not exhaustive; the -/// heuristic still backstops anything unlisted. +/// heuristic still backstops anything unlisted. The same letter can differ +/// by manager: npm's `-w ` takes a value, while pnpm's `-w` +/// (workspace-root) and yarn's `-W` are boolean. pub(super) fn takes_value(manager: PackageManager, flag: &str) -> bool { match manager { PackageManager::Npm => matches!( @@ -323,6 +390,60 @@ pub(super) fn takes_value(manager: PackageManager, flag: &str) -> bool { | "--globalconfig" | "--depth" ), + PackageManager::Pnpm => matches!( + flag, + "-C" | "--dir" + | "--filter" + | "--registry" + | "--reporter" + | "--loglevel" + | "--store-dir" + | "--virtual-store-dir" + | "--modules-dir" + | "--lockfile-dir" + ), + PackageManager::Yarn => matches!( + flag, + "--registry" + | "--modules-folder" + | "--cache-folder" + | "--mutex" + | "--network-timeout" + | "--network-concurrency" + | "--global-folder" + | "--link-folder" + | "--preferred-cache-folder" + ), + PackageManager::Uv => matches!( + flag, + "--group" + | "--extra" + | "--index" + | "--default-index" + | "--index-url" + | "--extra-index-url" + | "-f" + | "--find-links" + | "--index-strategy" + | "--keyring-provider" + | "--tag" + | "--branch" + | "--rev" + | "--package" + | "-c" + | "--constraints" + | "--constraint" + | "--overrides" + | "-p" + | "--python" + | "--resolution" + | "--prerelease" + | "--exclude-newer" + | "--directory" + | "--project" + | "--config-setting" + | "--link-mode" + ), PackageManager::Pip => matches!( flag, "-i" | "--index-url" @@ -364,8 +485,8 @@ pub(super) fn takes_value(manager: PackageManager, flag: &str) -> bool { } } -/// Strip flags from an npm install argument list, returning only the -/// positional package specs. +/// Strip flags from a npm/yarn/pnpm (or `uv add`) install argument list, +/// returning only the positional package specs. /// /// We treat anything starting with `-` as a flag. Boolean flags (`-D`, /// `--save-dev`, `--no-save`, ...) are dropped on their own. Flags @@ -386,6 +507,25 @@ fn extract_node_positionals(manager: PackageManager, args: &[String]) -> Positio break; } if a.starts_with('-') { + // `uv add -r reqs.txt` adds the file's entries as dependencies — + // track the file like pip's `-r` so the gate covers its contents. + if manager == PackageManager::Uv { + if matches!(a.as_str(), "-r" | "--requirements" | "--requirement") { + if let Some(path) = args.get(i + 1) { + out.requirements_files.push(PathBuf::from(path)); + } + i += 2; + continue; + } + if let Some(rest) = a + .strip_prefix("--requirements=") + .or_else(|| a.strip_prefix("--requirement=")) + { + out.requirements_files.push(PathBuf::from(rest)); + i += 1; + continue; + } + } if !a.contains('=') && takes_value(manager, a) { i += 2; continue; @@ -771,6 +911,14 @@ fn parse_pypi_spec(raw: &str) -> InstallTarget { } } +/// Bare PyPI name from a requirement line: stop at extras, operators, +/// markers, or whitespace. Callers normalize when they need a comparison key. +pub(super) fn pypi_name_part(spec: &str) -> &str { + let stop = |c: char| matches!(c, '[' | '<' | '>' | '=' | '!' | '~' | ';' | ' '); + let cut = spec.find(stop).unwrap_or(spec.len()); + spec[..cut].trim() +} + #[cfg(test)] mod tests { use super::*; @@ -817,6 +965,123 @@ mod tests { assert_eq!(p.specs, vec!["lodash".to_string()]); } + #[test] + fn pnpm_and_yarn_boolean_workspace_flags_keep_the_spec() { + // pnpm's `-w` (--workspace-root) and yarn's `-W` are boolean — + // the next token is the package being installed. + let args = vec!["-w".to_string(), "lodash".to_string()]; + let p = extract_node_positionals(PackageManager::Pnpm, &args); + assert_eq!(p.specs, vec!["lodash".to_string()]); + + let args = vec!["-W".to_string(), "lodash".to_string()]; + let p = extract_node_positionals(PackageManager::Yarn, &args); + assert_eq!(p.specs, vec!["lodash".to_string()]); + + // pnpm's `--filter ` does take a value. + let args = vec![ + "--filter".to_string(), + "my-app".to_string(), + "lodash".to_string(), + ]; + let p = extract_node_positionals(PackageManager::Pnpm, &args); + assert_eq!(p.specs, vec!["lodash".to_string()]); + } + + #[test] + fn uv_add_group_flag_value_is_not_a_spec() { + let args = vec![ + "--group".to_string(), + "dev".to_string(), + "requests".to_string(), + ]; + let p = extract_node_positionals(PackageManager::Uv, &args); + assert_eq!(p.specs, vec!["requests".to_string()]); + } + + #[test] + fn classify_uv_command_recognizes_install_shapes() { + assert!(matches!( + classify_uv_command(&[ + "pip".to_string(), + "install".to_string(), + "requests".to_string(), + ]), + UvCommand::PipInstall { .. } + )); + assert!(matches!( + classify_uv_command(&["pip".to_string(), "i".to_string()]), + UvCommand::PipInstall { .. } + )); + assert!(matches!( + classify_uv_command(&["add".to_string(), "django".to_string()]), + UvCommand::Add { .. } + )); + assert_eq!( + classify_uv_command(&["sync".to_string(), "--extra".to_string(), "dev".to_string()]), + UvCommand::Sync + ); + assert_eq!( + classify_uv_command(&["run".to_string(), "pytest".to_string()]), + UvCommand::Passthrough + ); + assert_eq!( + classify_uv_command(&["lock".to_string()]), + UvCommand::Passthrough + ); + } + + #[test] + fn uv_add_positionals_parse_as_pypi_specs() { + let parsed = parse_pypi_positionals_args(&["requests==2.31.0".into()]); + assert_eq!(parsed.targets.len(), 1); + assert!( + matches!( + &parsed.targets[0].kind, + TargetKind::Pypi(PypiSpec::Exact(v)) if v == "2.31.0" + ), + "uv add targets must parse as PyPI specs, got {:?}", + parsed.targets[0].kind + ); + } + + #[test] + fn uv_add_requirements_flag_tracks_the_file() { + for args in [ + vec!["-r".to_string(), "reqs.txt".to_string()], + vec!["--requirements".to_string(), "reqs.txt".to_string()], + vec!["--requirements=reqs.txt".to_string()], + ] { + let p = extract_node_positionals(PackageManager::Uv, &args); + assert_eq!( + p.requirements_files, + vec![PathBuf::from("reqs.txt")], + "args {args:?}" + ); + assert!(p.specs.is_empty(), "args {args:?}"); + } + // `-c constraints.txt` doesn't add packages — value skipped. + let p = extract_node_positionals( + PackageManager::Uv, + &[ + "-c".to_string(), + "cons.txt".to_string(), + "flask".to_string(), + ], + ); + assert_eq!(p.specs, vec!["flask".to_string()]); + assert!(p.requirements_files.is_empty()); + } + + #[test] + fn pypi_name_part_strips_extras_markers_and_operators() { + assert_eq!(pypi_name_part("requests"), "requests"); + assert_eq!(pypi_name_part("requests[security]==2.31.0"), "requests"); + assert_eq!(pypi_name_part("Flask_Cors>=4.0"), "Flask_Cors"); + assert_eq!(pypi_name_part("pkg; python_version >= \"3.7\""), "pkg"); + assert_eq!(pypi_name_part("pkg ==1.0"), "pkg"); + assert_eq!(pypi_name_part(""), ""); + } + #[test] fn extracts_npm_positionals_after_double_dash() { let args = vec![ diff --git a/src/precheck/render.rs b/src/precheck/render.rs index 4a942c2..3be2e09 100644 --- a/src/precheck/render.rs +++ b/src/precheck/render.rs @@ -3,9 +3,27 @@ use crate::verify_deps; use super::{ - parse, PrecheckOptions, PrecheckReport, TargetOutcome, TreeOrigin, TreeReport, VerdictStatus, + parse, PackageManager, PrecheckOptions, PrecheckReport, TargetOutcome, TreeOrigin, TreeReport, + VerdictStatus, }; +/// One honest stderr line when a zero-spec install can't be gated: +/// yarn/pnpm/uv have no safe dry-run, so a bare install pulls its whole +/// dependency set unchecked. No-op for other managers (bare npm is gated +/// via the tree pass; bare pip installs nothing). +pub(super) fn bare_install_note(manager: PackageManager, subcommand_label: &str) { + if matches!( + manager, + PackageManager::Yarn | PackageManager::Pnpm | PackageManager::Uv + ) { + eprintln!( + "note: bare '{} {}' is not gated (no safe dry-run) — dependencies install unchecked", + manager.binary_name(), + subcommand_label + ); + } +} + /// The refusal line on stderr. Messaging only; the block decision and the /// choice of escape hatch live in `verdict::block_reason`. pub(super) fn print_refusal(reason: super::verdict::BlockReason) { diff --git a/src/precheck/tree.rs b/src/precheck/tree.rs index 0c34c74..d48ea97 100644 --- a/src/precheck/tree.rs +++ b/src/precheck/tree.rs @@ -23,13 +23,15 @@ pub struct TreePackage { } /// Whether this manager's resolver has anything to resolve for the parsed -/// install. pip's dry-run also reads `-r` requirements files, so those make -/// an install eligible even with no named targets. npm's lockfile resolution -/// reads `package.json`, so a bare `npm install` is eligible whenever the -/// project (found like npm finds it — nearest ancestor manifest) has one. +/// install. pip's dry-run and uv's compile also read `-r` requirements +/// files, so those make an install eligible even with no named targets. +/// npm's lockfile resolution reads `package.json`, so a bare `npm install` +/// is eligible whenever the project (found like npm finds it — nearest +/// ancestor manifest) has one. pub fn covers_input(manager: PackageManager, parsed: &super::parse::ParsedInstall) -> bool { !parsed.targets.is_empty() - || (manager == PackageManager::Pip && !parsed.requirements_files.is_empty()) + || (matches!(manager, PackageManager::Pip | PackageManager::Uv) + && !parsed.requirements_files.is_empty()) || (manager == PackageManager::Npm && npm_project_root().is_some()) } @@ -63,8 +65,8 @@ pub(super) fn npm_root_redirect_flag(args: &[String]) -> Option { .cloned() } -/// `Err(reason)`: the dry-run failed — the caller falls back to named-only -/// and its warning carries `reason`. +/// `Err(reason)`: no safe dry-run for this manager, or the dry-run failed — +/// the caller falls back to named-only and its warning carries `reason`. pub fn resolve_tree( manager: PackageManager, install_args: &[String], @@ -73,6 +75,10 @@ pub fn resolve_tree( match manager { PackageManager::Pip => resolve_pip_tree(manager.binary_name(), install_args, parsed), PackageManager::Npm => resolve_npm_tree(manager.binary_name(), install_args), + PackageManager::Uv => resolve_uv_tree(parsed), + PackageManager::Yarn | PackageManager::Pnpm => { + Err(format!("{} has no safe dry-run", manager.binary_name())) + } } } @@ -128,6 +134,121 @@ fn resolve_pip_tree( parse_pip_report(&String::from_utf8_lossy(&output.stdout)) } +/// Resolve uv's would-install set with `uv pip compile` — uv's own +/// resolver, run without executing package code (`--only-binary :all:` +/// blocks sdist builds, mirroring the pip dry-run guard). Compile takes +/// requirements files rather than bare specs, so named registry specs and +/// absolutized `-r` includes are written to a temp `.in` file. +/// Unverifiable targets (URL / git / editable / path) are excluded — they +/// are already surfaced as skipped warnings. Index selection comes from +/// uv's env/config; index flags on the wrapped command don't carry over. +fn resolve_uv_tree(parsed: &super::parse::ParsedInstall) -> Result, String> { + let uv = super::exec::resolve_binary("uv")?; + let mut input = String::new(); + for t in &parsed.targets { + if !matches!(t.kind, super::TargetKind::Unverifiable { .. }) { + input.push_str(&t.display); + input.push('\n'); + } + } + for f in &parsed.requirements_files { + let abs = std::fs::canonicalize(f).map_err(|e| format!("read {}: {e}", f.display()))?; + input.push_str(&format!("-r {}\n", abs.display())); + } + if input.is_empty() { + return Err("nothing uv pip compile can resolve (all targets are URL/path refs)".into()); + } + + let work = tempfile::tempdir().map_err(|e| format!("create temp dir: {e}"))?; + let in_file = work.path().join("corgea-gate.in"); + std::fs::write(&in_file, &input).map_err(|e| format!("write compile input: {e}"))?; + let output = Command::new(&uv) + .args([ + "pip", + "compile", + "--only-binary", + ":all:", + "--no-header", + "--no-annotate", + "--quiet", + ]) + .arg(&in_file) + .output() + .map_err(|e| format!("run uv pip compile: {e}"))?; + if !output.status.success() { + return Err(format!("uv pip compile failed: {}", stderr_tail(&output))); + } + parse_compiled_requirements( + &String::from_utf8_lossy(&output.stdout), + &requested_names(parsed), + ) +} + +/// PEP 503-normalized names the user asked for — named CLI targets plus +/// entries of `-r` files — so tree findings label "(from requirements)" like +/// pip's `requested` report flag. Best-effort line parse; anything unparsed +/// just labels "(transitive)". +fn requested_names(parsed: &super::parse::ParsedInstall) -> std::collections::HashSet { + let norm = |n: &str| crate::vuln_api::Ecosystem::Pypi.normalize_name(n); + let mut out: std::collections::HashSet = parsed + .targets + .iter() + .filter(|t| !matches!(t.kind, super::TargetKind::Unverifiable { .. })) + .map(|t| norm(&t.name)) + .collect(); + for f in &parsed.requirements_files { + let Ok(content) = std::fs::read_to_string(f) else { + continue; + }; + for line in content.lines() { + let line = line.trim(); + if line.is_empty() || line.starts_with(['#', '-']) || line.contains("://") { + continue; + } + let name = super::parse::pypi_name_part(line); + if !name.is_empty() { + out.insert(norm(name)); + } + } + } + out +} + +/// Parse `uv pip compile` stdout (requirements.txt-format `name==version` +/// pins) into the would-install set. Any line that isn't a pin is an error — +/// silently skipping could hide part of the tree. +fn parse_compiled_requirements( + out: &str, + requested: &std::collections::HashSet, +) -> Result, String> { + let mut pkgs = Vec::new(); + for line in out.lines() { + let line = line.trim(); + if line.is_empty() || line.starts_with(['#', '-']) { + continue; + } + // Strip env markers and trailing comments: `pkg==1.0 ; marker # via`. + let line = line.split(';').next().unwrap_or(line).trim(); + let line = line.split(" #").next().unwrap_or(line).trim(); + let Some((name, version)) = line.split_once("==") else { + return Err(format!( + "unexpected line in uv pip compile output: '{line}'" + )); + }; + // Strip extras: `celery[redis]==5.3.4`. + let name = super::parse::pypi_name_part(name).to_string(); + pkgs.push(TreePackage { + requested: requested.contains(&crate::vuln_api::Ecosystem::Pypi.normalize_name(&name)), + name, + version: version.trim().to_string(), + }); + } + if pkgs.is_empty() { + return Err("uv pip compile produced no packages".to_string()); + } + Ok(pkgs) +} + fn parse_pip_report(json: &str) -> Result, String> { let report: serde_json::Value = serde_json::from_str(json).map_err(|e| format!("parse pip report: {e}"))?; diff --git a/src/precheck/uv.rs b/src/precheck/uv.rs new file mode 100644 index 0000000..d5fdbdd --- /dev/null +++ b/src/precheck/uv.rs @@ -0,0 +1,175 @@ +//! `corgea uv` routing: `uv pip install` / `uv add` / `uv pip sync` reuse +//! the parsed-install gate; `uv sync` is gated from `uv.lock`. + +use super::{corgea_cmd, detect, exec, parse, tree, PackageManager, PrecheckOptions}; + +pub(super) fn run_uv(cmd: &[String], opts: PrecheckOptions) -> i32 { + let exec = move || exec::exec_command("uv", cmd); + + if matches!(cmd.first().map(String::as_str), Some("install" | "i")) { + eprintln!("{}", unsupported_uv_install_message(&cmd[1..])); + return 1; + } + + match parse::classify_uv_command(cmd) { + // Passthrough is a transparent shim: no report, untouched stdio. + parse::UvCommand::Passthrough => exec::exec_command("uv", cmd), + parse::UvCommand::PipInstall { install_args } => { + let parsed = match parse::parse_pip_install_args(install_args) { + Ok(p) => p, + Err(e) => { + eprintln!("failed to parse install args: {}", e); + return 2; + } + }; + super::run_parsed_install( + PackageManager::Uv, + "pip install", + install_args, + parsed, + exec, + opts, + ) + } + parse::UvCommand::PipSync { sync_args } => { + // `uv pip sync reqs.txt` installs exactly the given requirements + // set — gate it like `uv pip install -r reqs.txt`. + let parsed = parse::parse_pip_sync_args(sync_args); + if parsed.requirements_files.is_empty() { + // No files named: uv errors on its own. + return exec::exec_command("uv", cmd); + } + super::run_parsed_install( + PackageManager::Uv, + "pip sync", + sync_args, + parsed, + exec, + opts, + ) + } + parse::UvCommand::Add { add_args } => { + let parsed = parse::parse_pypi_positionals_args(add_args); + if !opts.force { + if let Some(message) = + detect::wrong_package_manager_message(PackageManager::Uv, add_args, &parsed) + { + eprintln!("{message}"); + return 1; + } + } + super::run_parsed_install(PackageManager::Uv, "add", add_args, parsed, exec, opts) + } + parse::UvCommand::Sync => run_uv_sync(cmd, opts, exec), + } +} + +fn unsupported_uv_install_message(rest: &[String]) -> String { + format!( + "error: uv does not support top-level `install`.\nDid you mean `{}`?", + corgea_cmd(&["uv", "pip", "install"], rest) + ) +} + +/// Gate `uv sync` from the project's `uv.lock`. The lockfile is the full +/// locked universe (all groups/extras) — a superset of what sync installs, +/// conservative in the blocking direction; a stale lock that sync would +/// re-resolve is gated as written. Recency isn't checked (locked versions +/// aren't newly chosen by this command); the verdict pass is the gate. We +/// never run `uv lock` ourselves — locking can build sdists, which would +/// execute package code before any verdict. +fn run_uv_sync(cmd: &[String], opts: PrecheckOptions, exec: impl FnOnce() -> i32) -> i32 { + if opts.verdict.is_none() { + // Direct callers may still disable verdicts completely. + return exec(); + } + // uv discovers the project by walking up from the CWD — find `uv.lock` + // the same way, so a sync run from a project subdirectory stays gated. + let Some(lock_path) = tree::find_up("uv.lock") else { + eprintln!( + "note: no uv.lock here — 'uv sync' is not gated; dependencies install unchecked (run 'uv lock' first to enable the gate)" + ); + return exec(); + }; + let lock = std::fs::read_to_string(&lock_path) + .map_err(|e| format!("read {}: {e}", lock_path.display())) + .and_then(|content| parse_uv_lock(&content)); + super::run_locked_install( + PackageManager::Uv, + "sync", + cmd[1..].to_vec(), + lock, + &opts, + exec, + ) +} + +/// Packages from `uv.lock` that `uv sync` installs from an index. Local +/// stanzas (the project itself and path deps: editable / virtual / +/// directory / path sources) carry no registry identity and are skipped. +fn parse_uv_lock(content: &str) -> Result, String> { + #[derive(serde::Deserialize)] + struct Lock { + #[serde(default)] + package: Vec, + } + #[derive(serde::Deserialize)] + struct Pkg { + name: String, + version: Option, + #[serde(default)] + source: std::collections::BTreeMap, + } + const LOCAL_SOURCES: [&str; 4] = ["editable", "virtual", "directory", "path"]; + + let lock: Lock = toml::from_str(content).map_err(|e| format!("parse uv.lock: {e}"))?; + Ok(lock + .package + .into_iter() + .filter(|p| !LOCAL_SOURCES.iter().any(|k| p.source.contains_key(*k))) + .filter_map(|p| { + Some(tree::TreePackage { + name: p.name, + version: p.version?, + requested: false, + }) + }) + .collect()) +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn parse_uv_lock_keeps_index_packages_and_skips_local_sources() { + let lock = r#" +version = 1 + +[[package]] +name = "proj" +version = "0.1.0" +source = { editable = "." } + +[[package]] +name = "evildep" +version = "0.4.2" +source = { registry = "https://pypi.org/simple" } + +[[package]] +name = "gitdep" +version = "1.2.3" +source = { git = "https://example.com/repo?rev=abc#abc" } +"#; + let pkgs = parse_uv_lock(lock).expect("parse uv.lock"); + let names: Vec<&str> = pkgs.iter().map(|p| p.name.as_str()).collect(); + assert_eq!(names, vec!["evildep", "gitdep"]); + assert_eq!(pkgs[0].version, "0.4.2"); + } + + #[test] + fn parse_uv_lock_rejects_invalid_toml() { + let err = parse_uv_lock("not = [valid").expect_err("invalid toml"); + assert!(err.contains("parse uv.lock"), "got: {err}"); + } +} diff --git a/tests/cli_bare_install.rs b/tests/cli_bare_install.rs index 755ee50..1eb9be6 100644 --- a/tests/cli_bare_install.rs +++ b/tests/cli_bare_install.rs @@ -3,11 +3,13 @@ //! With a `package.json`, bare `npm install` is gated like any other //! install: the tree pass resolves the full lockfile set and verdicts //! every package, so a vulnerable lockfile blocks (exit 1, `--force` -//! escape). +//! escape). Bare yarn/pnpm/uv installs have no safe dry-run — they exec +//! unchecked behind one honest stderr note. //! -//! Harness mirrors `cli_tree.rs`: tree-aware fake npm on a private PATH + -//! local registry stub + in-crate vuln-api stub. `oldpkg` is published in -//! 2020 so recency never blocks here. +//! Harness mirrors `cli_tree.rs`: fake package manager on a private PATH +//! (tree-aware for npm, plain argv recorder for yarn/pnpm/uv) + local +//! registry stub + in-crate vuln-api stub. `oldpkg` is published in 2020 so +//! recency never blocks here. #![cfg(unix)] @@ -180,6 +182,84 @@ fn bare_npm_install_root_redirect_refuses_without_force() { ); } +#[test] +fn bare_yarn_with_no_args_prints_note_and_execs() { + // `corgea yarn` with zero args IS `yarn install` — it must get the same + // honest ungated note instead of a silent exec. + let mut h = GateHarness::new() + .fake_recorder("yarn", 0) + .oldpkg_registry() + .vuln_checks(HashMap::new()) + .in_project_dir() + .build(); + let out = h.cmd.args(["yarn"]).output().expect("run corgea"); + assert_eq!(out.status.code(), Some(0)); + assert_eq!(h.recorded_argv().as_deref(), Some("install")); + assert!( + String::from_utf8_lossy(&out.stderr).contains("bare 'yarn install' is not gated"), + "stderr: {}", + String::from_utf8_lossy(&out.stderr) + ); +} + +#[test] +fn bare_ungated_managers_print_note_and_exec() { + // yarn's nonzero exit also proves the manager's own exit code propagates. + let cases = [ + ("yarn", &["yarn", "install"][..], "install", 7), + ("pnpm", &["pnpm", "install"][..], "install", 0), + ("uv", &["uv", "add"][..], "add", 0), + ("uv", &["uv", "pip", "install"][..], "pip install", 0), + ]; + for (binary, args, forwarded_argv, exit_code) in cases { + let mut h = GateHarness::new() + .fake_recorder(binary, exit_code) + .oldpkg_registry() + .vuln_checks(HashMap::new()) + .in_project_dir() + .build(); + let out = h.cmd.args(args).output().expect("run corgea"); + assert_eq!(out.status.code(), Some(exit_code), "{args:?}"); + assert_eq!(h.recorded_argv().as_deref(), Some(forwarded_argv)); + let note = format!( + "note: bare '{}' is not gated (no safe dry-run) — dependencies install unchecked", + args.join(" ") + ); + assert!( + String::from_utf8_lossy(&out.stderr).contains(¬e), + "{args:?} stderr: {}", + String::from_utf8_lossy(&out.stderr) + ); + } +} + +#[test] +fn yarn_named_target_does_not_print_bare_note() { + // A named target takes the gated path: named-only warning, no bare note. + let mut h = GateHarness::new() + .fake_recorder("yarn", 0) + .oldpkg_registry() + .vuln_checks(HashMap::new()) + .in_project_dir() + .build(); + let out = h + .cmd + .args(["yarn", "add", "oldpkg@1.0.0"]) + .output() + .expect("run corgea"); + assert_eq!(out.status.code(), Some(0), "clean named target proceeds"); + assert_eq!(h.recorded_argv().as_deref(), Some("add oldpkg@1.0.0")); + let stderr = String::from_utf8_lossy(&out.stderr); + assert!( + !stderr.contains("not gated"), + "named install must not print the bare note: {stderr}" + ); + assert!( + stderr.contains("transitive dependencies not checked"), + "named-only warning still applies to yarn: {stderr}" + ); +} + #[test] fn bare_npm_install_from_subdirectory_is_gated() { // npm walks ancestors to find the project; the gate must too, or a diff --git a/tests/cli_install.rs b/tests/cli_install.rs index 3888c95..ed88d53 100644 --- a/tests/cli_install.rs +++ b/tests/cli_install.rs @@ -20,6 +20,7 @@ use common::{ use std::collections::HashMap; use std::sync::atomic::{AtomicUsize, Ordering}; use std::sync::Arc; +use tempfile::TempDir; /// Spawn a registry stub serving both the PyPI and npm routes the /// resolver hits. Returns the base URL and a counter of accepted @@ -62,10 +63,13 @@ fn wrapper_with_hits( let (base_url, registry_hits) = spawn_registry_stub(); // RESOLUTION_FAILS: the tree dry-run exits non-zero without touching // the argv marker, so `recorded_argv()` reflects only the real install. - let h = GateHarness::new() - .fake_tree_pm(binary, RESOLUTION_FAILS, pm_exit_code) - .registry_env(registry_env, &base_url) - .build(); + // yarn/pnpm/uv have no tree invocation to intercept — plain recorders. + let h = GateHarness::new(); + let h = match binary { + "npm" | "pip" => h.fake_tree_pm(binary, RESOLUTION_FAILS, pm_exit_code), + _ => h.fake_recorder(binary, pm_exit_code), + }; + let h = h.registry_env(registry_env, &base_url).build(); (h, registry_hits) } @@ -261,6 +265,240 @@ fn npm_old_pin_runs_install_with_forwarded_args() { assert_eq!(h.recorded_argv().as_deref(), Some("install oldpkg@1.0.0")); } +#[test] +fn node_wrong_manager_lockfiles_block_with_suggestions() { + struct Case { + run_manager: &'static str, + lockfile: &'static str, + lock_contents: &'static str, + args: &'static [&'static str], + expected_manager: &'static str, + expected_suggestion: &'static str, + } + + let cases = [ + Case { + run_manager: "npm", + lockfile: "pnpm-lock.yaml", + lock_contents: "lockfileVersion: '9.0'\n", + args: &["npm", "i", "oldpkg"], + expected_manager: "pnpm", + expected_suggestion: "corgea pnpm add oldpkg", + }, + Case { + run_manager: "pnpm", + lockfile: "package-lock.json", + lock_contents: "{}", + args: &["pnpm", "install"], + expected_manager: "npm", + expected_suggestion: "corgea npm install", + }, + Case { + run_manager: "npm", + lockfile: "yarn.lock", + lock_contents: "# yarn lockfile v1\n", + args: &["npm", "i", "oldpkg"], + expected_manager: "yarn", + expected_suggestion: "corgea yarn add oldpkg", + }, + ]; + + for case in cases { + let (mut h, registry_hits) = wrapper_with_hits(case.run_manager, "CORGEA_NPM_REGISTRY", 0); + let project = TempDir::new().expect("project dir"); + std::fs::write(project.path().join(case.lockfile), case.lock_contents) + .expect("write lockfile"); + let out = h + .cmd + .current_dir(project.path()) + .args(case.args) + .output() + .expect("run corgea"); + assert_eq!(out.status.code(), Some(1), "{}", case.lockfile); + assert_eq!( + h.recorded_argv(), + None, + "{} must not run in a {} project", + case.run_manager, + case.expected_manager + ); + assert_eq!( + registry_hits.load(Ordering::SeqCst), + 0, + "wrong-manager refusal must not touch the registry" + ); + let stderr = String::from_utf8_lossy(&out.stderr); + assert!( + stderr.contains(&format!( + "this project appears to use {}", + case.expected_manager + )), + "{} stderr: {stderr}", + case.lockfile + ); + assert!( + stderr.contains(&format!("Did you mean `{}`?", case.expected_suggestion)), + "{} stderr: {stderr}", + case.lockfile + ); + } +} + +#[test] +fn force_overrides_the_wrong_manager_guard() { + let mut h = wrapper("npm", "CORGEA_NPM_REGISTRY", 0); + let project = TempDir::new().expect("project dir"); + std::fs::write( + project.path().join("pnpm-lock.yaml"), + "lockfileVersion: '9.0'\n", + ) + .expect("write lockfile"); + let out = h + .cmd + .current_dir(project.path()) + .args(["npm", "--force", "install", "oldpkg@1.0.0"]) + .output() + .expect("run corgea"); + assert_eq!( + out.status.code(), + Some(0), + "--force must bypass the guard: {}", + String::from_utf8_lossy(&out.stderr) + ); + assert_eq!(h.recorded_argv().as_deref(), Some("install oldpkg@1.0.0")); +} + +#[test] +fn fresh_project_is_not_blamed_for_ancestor_lockfiles() { + // A project with its own package.json but no manager indicators must + // not inherit a stray ancestor pnpm-lock.yaml. + let mut h = wrapper("npm", "CORGEA_NPM_REGISTRY", 0); + let root = TempDir::new().expect("root dir"); + std::fs::write( + root.path().join("pnpm-lock.yaml"), + "lockfileVersion: '9.0'\n", + ) + .expect("write ancestor lockfile"); + let project = root.path().join("newapp"); + std::fs::create_dir(&project).expect("mkdir"); + std::fs::write(project.join("package.json"), "{}").expect("write manifest"); + let out = h + .cmd + .current_dir(&project) + .args(["npm", "install", "oldpkg@1.0.0"]) + .output() + .expect("run corgea"); + assert_eq!( + out.status.code(), + Some(0), + "fresh project must not be refused: {}", + String::from_utf8_lossy(&out.stderr) + ); + assert_eq!(h.recorded_argv().as_deref(), Some("install oldpkg@1.0.0")); +} + +#[test] +fn package_manager_field_beats_missing_lockfile_for_node_guard() { + // `packageManager: "pnpm@9"` marks a pnpm project even before the + // first install writes a lockfile. + let mut h = wrapper("npm", "CORGEA_NPM_REGISTRY", 0); + let project = TempDir::new().expect("project dir"); + std::fs::write( + project.path().join("package.json"), + r#"{"name":"proj","packageManager":"pnpm@9.0.0"}"#, + ) + .expect("write manifest"); + let out = h + .cmd + .current_dir(project.path()) + .args(["npm", "i", "oldpkg"]) + .output() + .expect("run corgea"); + assert_eq!(out.status.code(), Some(1)); + let stderr = String::from_utf8_lossy(&out.stderr); + assert!( + stderr.contains("this project appears to use pnpm"), + "stderr: {stderr}" + ); +} + +#[test] +fn conflicting_node_lockfiles_do_not_block_as_wrong_manager() { + // Two lockfiles → ambiguous; the guard must stand down rather than + // guess. + let mut h = wrapper("npm", "CORGEA_NPM_REGISTRY", 0); + let project = TempDir::new().expect("project dir"); + std::fs::write( + project.path().join("pnpm-lock.yaml"), + "lockfileVersion: '9.0'\n", + ) + .expect("write pnpm lockfile"); + std::fs::write(project.path().join("yarn.lock"), "# yarn lockfile v1\n") + .expect("write yarn lockfile"); + let out = h + .cmd + .current_dir(project.path()) + .args(["npm", "install", "oldpkg@1.0.0"]) + .output() + .expect("run corgea"); + assert_eq!( + out.status.code(), + Some(0), + "ambiguous indicators must not refuse: {}", + String::from_utf8_lossy(&out.stderr) + ); + assert_eq!(h.recorded_argv().as_deref(), Some("install oldpkg@1.0.0")); +} + +#[test] +fn pip_in_uv_lock_project_blocks_with_uv_add_suggestion() { + let mut h = wrapper("pip", "CORGEA_PYPI_REGISTRY", 0); + let project = TempDir::new().expect("project dir"); + std::fs::write(project.path().join("uv.lock"), "version = 1\n").expect("write uv.lock"); + let out = h + .cmd + .current_dir(project.path()) + .args(["pip", "install", "oldpkg==1.0.0"]) + .output() + .expect("run corgea"); + assert_eq!(out.status.code(), Some(1)); + assert_eq!(h.recorded_argv(), None, "pip must not run"); + let stderr = String::from_utf8_lossy(&out.stderr); + assert!( + stderr.contains("this project appears to use uv"), + "stderr: {stderr}" + ); + assert!( + stderr.contains("Did you mean `corgea uv add oldpkg==1.0.0`?"), + "stderr: {stderr}" + ); +} + +#[test] +fn uv_add_in_requirements_project_blocks_with_pip_install_suggestion() { + let mut h = wrapper("uv", "CORGEA_PYPI_REGISTRY", 0); + let project = TempDir::new().expect("project dir"); + std::fs::write(project.path().join("requirements.txt"), "oldpkg==1.0.0\n") + .expect("write requirements.txt"); + let out = h + .cmd + .current_dir(project.path()) + .args(["uv", "add", "oldpkg==1.0.0"]) + .output() + .expect("run corgea"); + assert_eq!(out.status.code(), Some(1)); + assert_eq!(h.recorded_argv(), None, "uv must not run"); + let stderr = String::from_utf8_lossy(&out.stderr); + assert!( + stderr.contains("this project appears to use pip"), + "stderr: {stderr}" + ); + assert!( + stderr.contains("Did you mean `corgea pip install oldpkg==1.0.0`?"), + "stderr: {stderr}" + ); +} + #[test] fn npm_install_verb_behind_global_flags_is_still_gated() { // SKILL.md promises `npm --loglevel silent install x` is still gated: diff --git a/tests/cli_tree.rs b/tests/cli_tree.rs index a2268a4..79f874b 100644 --- a/tests/cli_tree.rs +++ b/tests/cli_tree.rs @@ -14,6 +14,7 @@ mod common; use common::{ key, tree_harness, vulnerable_body, GateHarness, NPM_LOCK, RESOLUTION_FAILS, TREE_REPORT, + UV_COMPILED, }; use std::collections::HashMap; use tempfile::TempDir; @@ -129,6 +130,12 @@ fn transitive_vulnerable_blocks_install() { NPM_LOCK, &["npm", "install", "oldpkg@1.0.0"][..], ), + ( + "uv", + "pypi", + UV_COMPILED, + &["uv", "pip", "install", "oldpkg==1.0.0"][..], + ), ]; for (binary, eco, payload, args) in cases { let mut checks = HashMap::new(); @@ -152,6 +159,39 @@ fn transitive_vulnerable_blocks_install() { } } +#[test] +fn uv_requirements_file_install_is_tree_gated() { + // `uv pip install -r requirements.txt` names no targets — the gate must + // still resolve the full set via `uv pip compile` and block on the + // vulnerable pin instead of exec'ing unchecked. + let cwd = TempDir::new().expect("temp cwd"); + std::fs::write(cwd.path().join("requirements.txt"), "oldpkg==1.0.0\n") + .expect("write requirements.txt"); + let mut checks = HashMap::new(); + checks.insert( + key("pypi", "evildep", "0.4.2"), + vulnerable_evildep_body("pypi"), + ); + let mut h = tree_harness("uv", checks, HashMap::new(), UV_COMPILED); + let out = h + .cmd + .current_dir(cwd.path()) + .args(["uv", "pip", "install", "-r", "requirements.txt"]) + .output() + .expect("run corgea"); + assert_eq!(out.status.code(), Some(1), "transitive vuln must block"); + assert_eq!(h.recorded_argv(), None, "uv must not run when blocked"); + let stdout = String::from_utf8_lossy(&out.stdout); + for needle in ["evildep", "MAL-2024-0002", "(transitive)"] { + assert!(stdout.contains(needle), "stdout: {stdout}"); + } + let stderr = String::from_utf8_lossy(&out.stderr); + assert!( + !stderr.contains("not gated"), + "gated uv requirements install must not print the bare note: {stderr}" + ); +} + #[test] fn tree_pass_runs_via_pip3_when_pip_is_absent() { // Only `pip3` exists on PATH (common Linux/macOS). The tree pass must @@ -196,6 +236,11 @@ fn resolution_failure_falls_back_with_loud_warning() { &["npm", "install", "oldpkg@1.0.0"][..], "install oldpkg@1.0.0", ), + ( + "uv", + &["uv", "pip", "install", "oldpkg==1.0.0"][..], + "pip install oldpkg==1.0.0", + ), ]; for (binary, args, forwarded_argv) in cases { let mut h = tree_harness(binary, HashMap::new(), HashMap::new(), RESOLUTION_FAILS); diff --git a/tests/cli_uv_sync.rs b/tests/cli_uv_sync.rs new file mode 100644 index 0000000..1a90e0e --- /dev/null +++ b/tests/cli_uv_sync.rs @@ -0,0 +1,207 @@ +//! Hermetic e2e tests for the `corgea uv sync` gate. +//! +//! `uv sync` is gated from the project's `uv.lock`: every index-sourced pin +//! is verdicted against the vuln-api stub before uv runs. Without a lockfile +//! it execs behind an honest note. Harness: fake `uv` argv recorder on a +//! private PATH + in-crate vuln-api stub + throwaway project dir as cwd. No +//! registry stub — the sync gate does no recency resolution. + +#![cfg(unix)] + +mod common; + +use common::{key, vulnerable_body, GateHarness}; +use corgea::vuln_api_stub::PackageKey; +use std::collections::HashMap; + +/// `proj` is the project itself (editable — skipped); `evildep` is the one +/// index-sourced pin the gate must verdict. +const UV_LOCK: &str = r#" +version = 1 + +[[package]] +name = "proj" +version = "0.1.0" +source = { editable = "." } + +[[package]] +name = "evildep" +version = "0.4.2" +source = { registry = "https://pypi.org/simple" } +"#; + +fn vulnerable_evildep_checks() -> HashMap { + let mut checks = HashMap::new(); + checks.insert( + key("pypi", "evildep", "0.4.2"), + vulnerable_body("pypi", "evildep", "0.4.2", "MAL-2024-0002", None), + ); + checks +} + +#[test] +fn uv_sync_from_subdirectory_is_gated() { + // uv discovers the project by walking up; the gate must find uv.lock + // the same way or a sync from /src runs silently ungated. + let mut h = GateHarness::new() + .fake_recorder("uv", 0) + .vuln_checks(vulnerable_evildep_checks()) + .with_project_file("uv.lock", UV_LOCK) + .in_subdir("src") + .build(); + let out = h.cmd.args(["uv", "sync"]).output().expect("run corgea"); + assert_eq!( + out.status.code(), + Some(1), + "vulnerable ancestor lock must block: {}", + String::from_utf8_lossy(&out.stderr) + ); + assert_eq!(h.recorded_argv(), None, "uv must not run"); +} + +#[test] +fn uv_sync_vulnerable_lockfile_blocks() { + let mut h = GateHarness::new() + .fake_recorder("uv", 0) + .vuln_checks(vulnerable_evildep_checks()) + .with_project_file("uv.lock", UV_LOCK) + .build(); + let out = h.cmd.args(["uv", "sync"]).output().expect("run corgea"); + assert_eq!(out.status.code(), Some(1), "vulnerable lock must block"); + assert_eq!( + h.recorded_argv(), + None, + "uv must not run on a vulnerable verdict" + ); + let stdout = String::from_utf8_lossy(&out.stdout); + for needle in ["evildep", "MAL-2024-0002", "(locked)"] { + assert!(stdout.contains(needle), "stdout: {stdout}"); + } + // Nothing was named by this command — the refusal blames the lock, not + // the user's input. + assert!( + String::from_utf8_lossy(&out.stderr) + .contains("your existing dependency tree has known-vulnerable packages"), + "stderr: {}", + String::from_utf8_lossy(&out.stderr) + ); +} + +#[test] +fn uv_sync_clean_lockfile_proceeds() { + let mut h = GateHarness::new() + .fake_recorder("uv", 0) + .vuln_checks(HashMap::new()) + .with_project_file("uv.lock", UV_LOCK) + .build(); + let out = h + .cmd + .args(["uv", "sync", "--frozen"]) + .output() + .expect("run corgea"); + assert_eq!(out.status.code(), Some(0), "clean lock must proceed"); + assert_eq!( + h.recorded_argv().as_deref(), + Some("sync --frozen"), + "uv's own args must be forwarded untouched" + ); + let stdout = String::from_utf8_lossy(&out.stdout); + assert!( + stdout.contains("tree: 1 packages resolved"), + "the project's own editable stanza must be skipped: {stdout}" + ); +} + +#[test] +fn uv_sync_force_overrides_block() { + let mut h = GateHarness::new() + .fake_recorder("uv", 0) + .vuln_checks(vulnerable_evildep_checks()) + .with_project_file("uv.lock", UV_LOCK) + .build(); + let out = h + .cmd + .args(["uv", "--force", "sync"]) + .output() + .expect("run corgea"); + assert_eq!(out.status.code(), Some(0), "--force must run the sync"); + assert_eq!(h.recorded_argv().as_deref(), Some("sync")); + assert!( + String::from_utf8_lossy(&out.stdout).contains("evildep"), + "findings still printed under --force" + ); +} + +#[test] +fn uv_sync_without_lockfile_execs_with_note() { + let mut h = GateHarness::new() + .fake_recorder("uv", 0) + .vuln_checks(HashMap::new()) + .in_project_dir() + .build(); + let out = h.cmd.args(["uv", "sync"]).output().expect("run corgea"); + assert_eq!(out.status.code(), Some(0)); + assert_eq!(h.recorded_argv().as_deref(), Some("sync")); + assert!( + String::from_utf8_lossy(&out.stderr).contains("'uv sync' is not gated"), + "stderr must carry the explicit ungated note: {}", + String::from_utf8_lossy(&out.stderr) + ); +} + +#[test] +fn uv_sync_malformed_lockfile_fails_closed() { + let mut h = GateHarness::new() + .fake_recorder("uv", 0) + .vuln_checks(HashMap::new()) + .with_project_file("uv.lock", "not = [valid") + .build(); + let out = h.cmd.args(["uv", "sync"]).output().expect("run corgea"); + assert_eq!(out.status.code(), Some(1), "unparseable lock must block"); + assert_eq!(h.recorded_argv(), None); + let stderr = String::from_utf8_lossy(&out.stderr); + assert!( + stderr.contains("cannot verify 'uv sync'"), + "stderr: {stderr}" + ); + assert!(stderr.contains("--force"), "stderr: {stderr}"); +} + +#[test] +fn uv_lock_stays_passthrough() { + // `uv lock` installs nothing; the gate applies to the sync that follows. + let mut h = GateHarness::new() + .fake_recorder("uv", 0) + .vuln_checks(vulnerable_evildep_checks()) + .with_project_file("uv.lock", UV_LOCK) + .build(); + let out = h.cmd.args(["uv", "lock"]).output().expect("run corgea"); + assert_eq!(out.status.code(), Some(0)); + assert_eq!(h.recorded_argv().as_deref(), Some("lock")); + assert!(!String::from_utf8_lossy(&out.stdout).contains("Pre-checking")); +} + +#[test] +fn uv_top_level_install_blocks_with_suggestion() { + let mut h = GateHarness::new() + .fake_recorder("uv", 0) + .vuln_checks(HashMap::new()) + .in_project_dir() + .build(); + let out = h + .cmd + .args(["uv", "install", "requests"]) + .output() + .expect("run corgea"); + assert_eq!(out.status.code(), Some(1)); + assert_eq!(h.recorded_argv(), None, "uv must not run"); + let stderr = String::from_utf8_lossy(&out.stderr); + assert!( + stderr.contains("uv does not support top-level `install`"), + "stderr: {stderr}" + ); + assert!( + stderr.contains("Did you mean `corgea uv pip install requests`?"), + "stderr: {stderr}" + ); +} diff --git a/tests/common/mod.rs b/tests/common/mod.rs index 46c5ace..5251da8 100644 --- a/tests/common/mod.rs +++ b/tests/common/mod.rs @@ -83,6 +83,11 @@ pub const NPM_LOCK: &str = r#"{"name":"proj","lockfileVersion":3,"packages":{ "node_modules/oldpkg":{"version":"1.0.0"}, "node_modules/evildep":{"version":"0.4.2"}}}"#; +/// `uv pip compile` stdout: `oldpkg` + transitive `evildep`, same shape as +/// `TREE_REPORT` / `NPM_LOCK`. +#[allow(dead_code)] +pub const UV_COMPILED: &str = "oldpkg==1.0.0\nevildep==0.4.2\n"; + /// Spawn a one-response-per-connection HTTP stub on an ephemeral 127.0.0.1 /// port; `route` maps a request path to `(status line, body)`. Returns the /// base URL. @@ -207,6 +212,7 @@ pub fn write_fake_tree_pm( let (tree_flag, redirect, fail_exit) = match binary { "pip" | "pip3" => ("--dry-run", "", 2), "npm" => ("--package-lock-only", " > package-lock.json", 1), + "uv" => ("compile", "", 1), other => panic!("unsupported fake manager {other}"), }; let tree_branch = if payload == RESOLUTION_FAILS { From aea2ef45084245b02773fe5f995696bdfb807e37 Mon Sep 17 00:00:00 2001 From: juangaitanv Date: Fri, 12 Jun 2026 16:36:37 +0200 Subject: [PATCH 2/3] Phase 3.1 review fixes: uv flag-skip, uv.lock sources, workspace-root guard MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Addresses Cursor review on #113. - uv commands are now classified after skipping leading global flags, so `uv --project ./app sync` / `uv --quiet add x` are gated instead of falling through to ungated passthrough. - the custom-index warning (from Phase 1) now fires for uv install/add/sync too, listing uv's index flags (--index, --default-index, --find-links, …). - the pip↔uv wrong-manager guard is applied consistently: it stays on `uv add` (project management, writes pyproject) but NOT `uv pip install` / `uv pip sync` — those are uv's pip-compatible interface, correct to use in a requirements project, and already fully gated by the tree pass. (Partial decline of the review's ask, with the reasoning above.) - parse_uv_lock now verdicts ONLY registry-sourced packages: git/url direct artifacts are skipped (their name@version is not a PyPI identity), and a registry package missing a version fails closed instead of being silently dropped. - the Node wrong-manager guard walks past a workspace MEMBER's leaf package.json to the workspace root (root `workspaces` field or pnpm-workspace.yaml), so a member install is checked against the root's manager. Standalone fresh projects still stop at their own leaf. --- src/precheck/detect.rs | 54 ++++++++++++++++++++++++- src/precheck/mod.rs | 12 ++++-- src/precheck/parse.rs | 23 +++++++++++ src/precheck/uv.rs | 90 ++++++++++++++++++++++++++++++++---------- tests/cli_uv_sync.rs | 52 ++++++++++++++++++++++++ 5 files changed, 206 insertions(+), 25 deletions(-) diff --git a/src/precheck/detect.rs b/src/precheck/detect.rs index a058358..27d8eb9 100644 --- a/src/precheck/detect.rs +++ b/src/precheck/detect.rs @@ -46,13 +46,33 @@ fn detect_node_manager_from(start: &Path) -> Option { // their own discovery the same way). A project with no manager // indicators of its own must not inherit a stray ancestor lockfile // — that would hard-refuse installs in every fresh project under it. - if dir.join("package.json").is_file() { + // + // EXCEPT a workspace member: its package manager is controlled by the + // workspace ROOT (root `workspaces` field or `pnpm-workspace.yaml`), + // so keep walking up to find it instead of stopping at the leaf. + if dir.join("package.json").is_file() && !is_inside_npm_workspace(dir) { return None; } } None } +/// Whether `dir` is a member of an npm/yarn/pnpm workspace rooted at some +/// ancestor — i.e. a strict ancestor carries a `pnpm-workspace.yaml` or a +/// `package.json` with a `workspaces` field. Used to keep the wrong-manager +/// walk going past a leaf member up to the workspace root. +fn is_inside_npm_workspace(dir: &Path) -> bool { + dir.ancestors().skip(1).any(|ancestor| { + if ancestor.join("pnpm-workspace.yaml").is_file() { + return true; + } + std::fs::read_to_string(ancestor.join("package.json")) + .ok() + .and_then(|raw| serde_json::from_str::(&raw).ok()) + .is_some_and(|j| j.get("workspaces").is_some()) + }) +} + fn detect_node_manager_in_dir(dir: &Path) -> ProjectManagerDetection { match package_json_manager(dir) { ProjectManagerDetection::None => {} @@ -208,6 +228,38 @@ mod tests { assert_eq!(detect_node_manager_from(&bare), Some(PackageManager::Npm)); } + #[test] + fn node_walk_reaches_workspace_root_from_a_member() { + // A workspace member's own package.json has no manager indicators, + // but the workspace ROOT controls the manager. The walk must reach + // the root (here: pnpm via pnpm-lock.yaml + `workspaces`) instead of + // hard-stopping at the leaf. + let root = tempfile::tempdir().expect("tempdir"); + std::fs::write( + root.path().join("package.json"), + r#"{"name":"monorepo","workspaces":["packages/*"]}"#, + ) + .expect("write root manifest"); + touch(&root.path().join("pnpm-lock.yaml")); + let member = root.path().join("packages").join("a"); + std::fs::create_dir_all(&member).expect("mkdir"); + std::fs::write(member.join("package.json"), r#"{"name":"a"}"#).expect("write member"); + + assert_eq!( + detect_node_manager_from(&member), + Some(PackageManager::Pnpm), + "a workspace member must resolve to the root's manager" + ); + + // A standalone project (no workspace ancestor) still stops at its leaf. + let standalone = tempfile::tempdir().expect("tempdir"); + touch(&standalone.path().join("yarn.lock")); + let fresh = standalone.path().join("fresh"); + std::fs::create_dir(&fresh).expect("mkdir"); + std::fs::write(fresh.join("package.json"), "{}").expect("write manifest"); + assert_eq!(detect_node_manager_from(&fresh), None); + } + #[test] fn uv_walk_stops_at_a_nearer_python_project() { // A pip project (requirements/pyproject, no uv.lock) must not be diff --git a/src/precheck/mod.rs b/src/precheck/mod.rs index 211a0d1..2b6b30e 100644 --- a/src/precheck/mod.rs +++ b/src/precheck/mod.rs @@ -453,9 +453,15 @@ fn unsupported_pip_add_message(rest: &[String]) -> String { fn warn_registry_override(manager: PackageManager, rest: &[String]) { let flags: &[&str] = match manager { PackageManager::Npm | PackageManager::Yarn | PackageManager::Pnpm => &["--registry"], - PackageManager::Pip | PackageManager::Uv => { - &["-i", "--index-url", "--extra-index-url", "--default-index"] - } + PackageManager::Pip | PackageManager::Uv => &[ + "-i", + "--index-url", + "--extra-index-url", + "--index", + "--default-index", + "-f", + "--find-links", + ], }; if let Some(flag) = rest.iter().find(|a| { flags diff --git a/src/precheck/parse.rs b/src/precheck/parse.rs index 802cadd..7a61d92 100644 --- a/src/precheck/parse.rs +++ b/src/precheck/parse.rs @@ -127,6 +127,29 @@ pub enum UvCommand<'a> { Sync, } +/// Skip leading uv global flags (and their values) to where the uv +/// subcommand starts, so install-shaped commands behind global flags +/// (`uv --project ./app sync`, `uv --quiet add x`) classify as the +/// subcommand instead of falling through to `Passthrough` ungated. +pub fn uv_subcommand(cmd: &[String]) -> &[String] { + let mut i = 0; + while i < cmd.len() { + let a = &cmd[i]; + if a == "--" { + return &cmd[(i + 1).min(cmd.len())..]; + } + if !a.starts_with('-') { + return &cmd[i..]; + } + i += if !a.contains('=') && takes_value(PackageManager::Uv, a) { + 2 + } else { + 1 + }; + } + &[] +} + pub fn classify_uv_command(cmd: &[String]) -> UvCommand<'_> { match cmd.first().map(String::as_str) { Some("pip") if matches!(cmd.get(1).map(String::as_str), Some("install" | "i")) => { diff --git a/src/precheck/uv.rs b/src/precheck/uv.rs index d5fdbdd..f97ca18 100644 --- a/src/precheck/uv.rs +++ b/src/precheck/uv.rs @@ -6,12 +6,17 @@ use super::{corgea_cmd, detect, exec, parse, tree, PackageManager, PrecheckOptio pub(super) fn run_uv(cmd: &[String], opts: PrecheckOptions) -> i32 { let exec = move || exec::exec_command("uv", cmd); - if matches!(cmd.first().map(String::as_str), Some("install" | "i")) { - eprintln!("{}", unsupported_uv_install_message(&cmd[1..])); + // Classify on the subcommand, skipping any leading uv global flags + // (`uv --project ./app sync` is still a gated sync). The exec path always + // forwards the original `cmd`, global flags included. + let sub = parse::uv_subcommand(cmd); + + if matches!(sub.first().map(String::as_str), Some("install" | "i")) { + eprintln!("{}", unsupported_uv_install_message(&sub[1..])); return 1; } - match parse::classify_uv_command(cmd) { + match parse::classify_uv_command(sub) { // Passthrough is a transparent shim: no report, untouched stdio. parse::UvCommand::Passthrough => exec::exec_command("uv", cmd), parse::UvCommand::PipInstall { install_args } => { @@ -22,6 +27,11 @@ pub(super) fn run_uv(cmd: &[String], opts: PrecheckOptions) -> i32 { return 2; } }; + // No wrong-manager guard here: `uv pip install` IS uv's + // pip-compatible interface, so using it in a requirements/pip + // project is correct, not a wrong-manager mistake — and it is + // fully gated by the tree pass below regardless. + super::warn_registry_override(PackageManager::Uv, install_args); super::run_parsed_install( PackageManager::Uv, "pip install", @@ -39,6 +49,7 @@ pub(super) fn run_uv(cmd: &[String], opts: PrecheckOptions) -> i32 { // No files named: uv errors on its own. return exec::exec_command("uv", cmd); } + super::warn_registry_override(PackageManager::Uv, sync_args); super::run_parsed_install( PackageManager::Uv, "pip sync", @@ -49,7 +60,10 @@ pub(super) fn run_uv(cmd: &[String], opts: PrecheckOptions) -> i32 { ) } parse::UvCommand::Add { add_args } => { + // `uv add` is project management (writes pyproject); using it in a + // pip/requirements project IS a wrong-manager mistake. let parsed = parse::parse_pypi_positionals_args(add_args); + super::warn_registry_override(PackageManager::Uv, add_args); if !opts.force { if let Some(message) = detect::wrong_package_manager_message(PackageManager::Uv, add_args, &parsed) @@ -104,9 +118,14 @@ fn run_uv_sync(cmd: &[String], opts: PrecheckOptions, exec: impl FnOnce() -> i32 ) } -/// Packages from `uv.lock` that `uv sync` installs from an index. Local -/// stanzas (the project itself and path deps: editable / virtual / -/// directory / path sources) carry no registry identity and are skipped. +/// Packages from `uv.lock` that `uv sync` installs from an index. Only +/// registry-sourced packages carry a name@version the public vuln-api can +/// verify, so: +/// * non-registry sources (editable / virtual / directory / path — local; +/// git / url — direct artifacts) are skipped: sending their name@version +/// to PyPI would verdict an unrelated package; +/// * a registry package missing a `version` is a parse error (fail closed) +/// rather than a silent omission that installs unchecked. fn parse_uv_lock(content: &str) -> Result, String> { #[derive(serde::Deserialize)] struct Lock { @@ -120,21 +139,33 @@ fn parse_uv_lock(content: &str) -> Result, String> { #[serde(default)] source: std::collections::BTreeMap, } - const LOCAL_SOURCES: [&str; 4] = ["editable", "virtual", "directory", "path"]; + // Sources that are NOT a registry/index: skip them entirely. + const NON_REGISTRY_SOURCES: [&str; 6] = + ["editable", "virtual", "directory", "path", "git", "url"]; let lock: Lock = toml::from_str(content).map_err(|e| format!("parse uv.lock: {e}"))?; - Ok(lock - .package - .into_iter() - .filter(|p| !LOCAL_SOURCES.iter().any(|k| p.source.contains_key(*k))) - .filter_map(|p| { - Some(tree::TreePackage { - name: p.name, - version: p.version?, - requested: false, - }) - }) - .collect()) + let mut out = Vec::new(); + for p in lock.package { + if NON_REGISTRY_SOURCES + .iter() + .any(|k| p.source.contains_key(*k)) + { + continue; + } + // Registry (or default-index) package — it must carry a version. + let version = p.version.ok_or_else(|| { + format!( + "uv.lock registry package '{}' has no version; cannot verify", + p.name + ) + })?; + out.push(tree::TreePackage { + name: p.name, + version, + requested: false, + }); + } + Ok(out) } #[cfg(test)] @@ -142,7 +173,11 @@ mod tests { use super::*; #[test] - fn parse_uv_lock_keeps_index_packages_and_skips_local_sources() { + fn parse_uv_lock_keeps_only_registry_packages() { + // Only the registry-sourced `evildep` is verifiable; the local + // (editable) project and the git source carry no PyPI identity and + // must be skipped — sending the git package's name@version to the + // public vuln-api would verdict an unrelated package. let lock = r#" version = 1 @@ -163,10 +198,23 @@ source = { git = "https://example.com/repo?rev=abc#abc" } "#; let pkgs = parse_uv_lock(lock).expect("parse uv.lock"); let names: Vec<&str> = pkgs.iter().map(|p| p.name.as_str()).collect(); - assert_eq!(names, vec!["evildep", "gitdep"]); + assert_eq!(names, vec!["evildep"]); assert_eq!(pkgs[0].version, "0.4.2"); } + #[test] + fn parse_uv_lock_registry_package_missing_version_fails_closed() { + let lock = r#" +version = 1 + +[[package]] +name = "mystery" +source = { registry = "https://pypi.org/simple" } +"#; + let err = parse_uv_lock(lock).expect_err("missing version must fail"); + assert!(err.contains("has no version"), "got: {err}"); + } + #[test] fn parse_uv_lock_rejects_invalid_toml() { let err = parse_uv_lock("not = [valid").expect_err("invalid toml"); diff --git a/tests/cli_uv_sync.rs b/tests/cli_uv_sync.rs index 1a90e0e..fbdcfa8 100644 --- a/tests/cli_uv_sync.rs +++ b/tests/cli_uv_sync.rs @@ -181,6 +181,58 @@ fn uv_lock_stays_passthrough() { assert!(!String::from_utf8_lossy(&out.stdout).contains("Pre-checking")); } +#[test] +fn uv_global_flags_before_subcommand_still_gate() { + // `uv --quiet sync` (global flag before the subcommand) must classify as + // a gated sync, not fall through to ungated passthrough. + let mut h = GateHarness::new() + .fake_recorder("uv", 0) + .vuln_checks(vulnerable_evildep_checks()) + .with_project_file("uv.lock", UV_LOCK) + .build(); + let out = h + .cmd + .args(["uv", "--quiet", "sync"]) + .output() + .expect("run corgea"); + assert_eq!( + out.status.code(), + Some(1), + "flags before the subcommand must not skip the gate: {}", + String::from_utf8_lossy(&out.stderr) + ); + assert_eq!(h.recorded_argv(), None, "uv must not run when blocked"); +} + +#[test] +fn uv_pip_install_in_requirements_project_is_not_wrong_manager() { + // `uv pip install` is uv's pip-compatible interface — using it in a + // requirements/pip project is correct, NOT a wrong-manager mistake. It is + // still fully gated (the tree pass blocks a vulnerable pin), but it must + // not be refused with a "did you mean pip" suggestion the way `uv add` is. + let mut h = GateHarness::new() + .fake_recorder("uv", 0) + .vuln_checks(HashMap::new()) + .with_project_file("requirements.txt", "flask\n") + .build(); + let out = h + .cmd + .args(["uv", "pip", "install", "git+https://example.com/x.git"]) + .output() + .expect("run corgea"); + // The only target is an unverifiable VCS spec → clean gate → uv runs. + assert_eq!( + out.status.code(), + Some(0), + "uv pip install must not be wrong-manager refused: {}", + String::from_utf8_lossy(&out.stderr) + ); + assert!( + !String::from_utf8_lossy(&out.stderr).contains("this project appears to use pip"), + "uv pip install is pip-compatible, not wrong-manager" + ); +} + #[test] fn uv_top_level_install_blocks_with_suggestion() { let mut h = GateHarness::new() From f8426b603619f1f2bf12a1b3888654b7d072638e Mon Sep 17 00:00:00 2001 From: juangaitanv Date: Fri, 12 Jun 2026 20:14:29 +0200 Subject: [PATCH 3/3] Phase 3.1 second-round review fixes: valued-flag gate bypass, uv pip flag-skip, ungated disclosures MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - SECURITY: valued global flags missing from takes_value made their VALUE classify as the subcommand → silent ungated passthrough. Added uv's --color/--config-file/--cache-dir/--allow-insecure-host and yarn's --cwd ('corgea uv --color always add x' and 'corgea yarn --cwd dir add x' installed unchecked). Unit + e2e regression tests with valued flags. - classify_uv_command skips flags between 'pip' and its verb, so 'uv pip --quiet install x' gates instead of passing through. - uv add's --optional/--bounds/--script values no longer parse as specs. - 'uv run' / 'uv tool install|run|upgrade' print an ungated-install note (and SKILL.md lists them as limitations) instead of passing silently; same for 'yarn global add'. - Workspace-member walk now checks dir membership against the declared globs (package.json workspaces / pnpm-workspace.yaml packages), so a standalone project nested under an unrelated monorepo keeps its leaf boundary instead of being wrong-manager-refused. - uv.lock parsing discloses skipped non-registry pin counts and warns on non-default registry sources; run_uv_sync echoes the command correctly behind global flags. --- skills/corgea/SKILL.md | 4 ++ src/precheck/detect.rs | 136 +++++++++++++++++++++++++++++++++++--- src/precheck/mod.rs | 10 ++- src/precheck/parse.rs | 111 +++++++++++++++++++++++++++++-- src/precheck/uv.rs | 134 ++++++++++++++++++++++++++++++++----- tests/cli_bare_install.rs | 31 +++++++++ tests/cli_uv_sync.rs | 48 ++++++++++++++ 7 files changed, 441 insertions(+), 33 deletions(-) diff --git a/skills/corgea/SKILL.md b/skills/corgea/SKILL.md index c010563..d4392ce 100644 --- a/skills/corgea/SKILL.md +++ b/skills/corgea/SKILL.md @@ -211,6 +211,10 @@ The gate is a wrapper, not an enforcement boundary. By design it cannot catch: unchecked behind the printed warning. - **Ungated managers** — bare `yarn`/`pnpm` installs run unchecked (see the bare-install note above); only their named targets are verified. +- **Ungated uv/yarn subcommands** — `uv run` (project sync on first run, + `--with` packages), `uv tool install`/`uv tool run`, and + `yarn global add` install packages without a gate; each prints an + ungated note instead of passing silently. Hard enforcement needs org-level controls — lockfile review, registry allow-listing — alongside the wrapper. diff --git a/src/precheck/detect.rs b/src/precheck/detect.rs index 27d8eb9..154bf9b 100644 --- a/src/precheck/detect.rs +++ b/src/precheck/detect.rs @@ -58,21 +58,87 @@ fn detect_node_manager_from(start: &Path) -> Option { } /// Whether `dir` is a member of an npm/yarn/pnpm workspace rooted at some -/// ancestor — i.e. a strict ancestor carries a `pnpm-workspace.yaml` or a -/// `package.json` with a `workspaces` field. Used to keep the wrong-manager -/// walk going past a leaf member up to the workspace root. +/// ancestor — a strict ancestor declares workspaces (`pnpm-workspace.yaml` +/// or a `package.json` `workspaces` field) AND `dir` matches the declared +/// member globs. The membership check matters: a standalone project nested +/// under an unrelated monorepo checkout (vendored repo, `examples/`) must +/// keep its own leaf boundary, not get wrong-manager-refused against the +/// stranger's lockfile. Used to keep the wrong-manager walk going past a +/// leaf member up to the workspace root. fn is_inside_npm_workspace(dir: &Path) -> bool { dir.ancestors().skip(1).any(|ancestor| { - if ancestor.join("pnpm-workspace.yaml").is_file() { - return true; - } - std::fs::read_to_string(ancestor.join("package.json")) - .ok() - .and_then(|raw| serde_json::from_str::(&raw).ok()) - .is_some_and(|j| j.get("workspaces").is_some()) + workspace_member_patterns(ancestor).is_some_and(|patterns| { + dir.strip_prefix(ancestor) + .is_ok_and(|rel| workspace_globs_match(&patterns, rel)) + }) }) } +/// Workspace member globs declared at `root`: `pnpm-workspace.yaml`'s +/// `packages:` list, or `package.json`'s `workspaces` (array form or the +/// `{packages: [...]}` object form). `None` when `root` declares no +/// workspace at all. +fn workspace_member_patterns(root: &Path) -> Option> { + if let Ok(yaml) = std::fs::read_to_string(root.join("pnpm-workspace.yaml")) { + return Some(pnpm_workspace_packages(&yaml)); + } + let raw = std::fs::read_to_string(root.join("package.json")).ok()?; + let json: serde_json::Value = serde_json::from_str(&raw).ok()?; + let ws = json.get("workspaces")?; + let arr = ws + .as_array() + .or_else(|| ws.get("packages").and_then(|p| p.as_array()))?; + Some( + arr.iter() + .filter_map(|v| v.as_str().map(str::to_string)) + .collect(), + ) +} + +/// Minimal extraction of the `packages:` list items from a +/// `pnpm-workspace.yaml`. Handles the standard file shape (a flat list of +/// quoted/unquoted globs); anything more exotic yields fewer patterns, +/// which is conservative — an unmatched member keeps its leaf boundary. +fn pnpm_workspace_packages(yaml: &str) -> Vec { + let mut in_packages = false; + let mut out = Vec::new(); + for line in yaml.lines() { + let trimmed = line.trim(); + if trimmed.starts_with("packages:") { + in_packages = true; + continue; + } + if in_packages { + if let Some(item) = trimmed.strip_prefix('-') { + out.push( + item.trim() + .trim_matches(|c| c == '"' || c == '\'') + .to_string(), + ); + } else if !trimmed.is_empty() && !trimmed.starts_with('#') { + break; + } + } + } + out +} + +/// Whether `rel` (a member dir relative to the workspace root) matches any +/// declared glob. Negations (`!excluded`) are skipped — conservative toward +/// NOT treating the dir as a member. +fn workspace_globs_match(patterns: &[String], rel: &Path) -> bool { + let mut builder = globset::GlobSetBuilder::new(); + for p in patterns { + if p.starts_with('!') { + continue; + } + if let Ok(glob) = globset::Glob::new(p) { + builder.add(glob); + } + } + builder.build().is_ok_and(|set| set.is_match(rel)) +} + fn detect_node_manager_in_dir(dir: &Path) -> ProjectManagerDetection { match package_json_manager(dir) { ProjectManagerDetection::None => {} @@ -260,6 +326,56 @@ mod tests { assert_eq!(detect_node_manager_from(&fresh), None); } + #[test] + fn node_walk_keeps_leaf_boundary_for_non_member_under_workspace() { + // An ancestor declaring workspaces does NOT make every nested + // project a member: a standalone checkout under `vendor/` (outside + // the declared globs) must keep its own leaf boundary instead of + // being wrong-manager-refused against the monorepo's lockfile. + let root = tempfile::tempdir().expect("tempdir"); + std::fs::write( + root.path().join("package.json"), + r#"{"name":"monorepo","workspaces":["packages/*"]}"#, + ) + .expect("write root manifest"); + touch(&root.path().join("pnpm-lock.yaml")); + let outsider = root.path().join("vendor").join("thing"); + std::fs::create_dir_all(&outsider).expect("mkdir"); + std::fs::write(outsider.join("package.json"), "{}").expect("write manifest"); + + assert_eq!( + detect_node_manager_from(&outsider), + None, + "a non-member nested project must not inherit the workspace root's manager" + ); + } + + #[test] + fn node_walk_reaches_pnpm_workspace_root_via_yaml_globs() { + // pnpm-workspace.yaml form: membership comes from its `packages:` + // globs, matched — not just the file's presence. + let root = tempfile::tempdir().expect("tempdir"); + std::fs::write( + root.path().join("pnpm-workspace.yaml"), + "packages:\n - \"apps/*\"\n", + ) + .expect("write workspace yaml"); + touch(&root.path().join("pnpm-lock.yaml")); + let member = root.path().join("apps").join("web"); + std::fs::create_dir_all(&member).expect("mkdir"); + std::fs::write(member.join("package.json"), r#"{"name":"web"}"#).expect("write member"); + assert_eq!( + detect_node_manager_from(&member), + Some(PackageManager::Pnpm) + ); + + // Outside the yaml's globs: leaf boundary holds. + let outsider = root.path().join("tools").join("x"); + std::fs::create_dir_all(&outsider).expect("mkdir"); + std::fs::write(outsider.join("package.json"), "{}").expect("write manifest"); + assert_eq!(detect_node_manager_from(&outsider), None); + } + #[test] fn uv_walk_stops_at_a_nearer_python_project() { // A pip project (requirements/pyproject, no uv.lock) must not be diff --git a/src/precheck/mod.rs b/src/precheck/mod.rs index 2b6b30e..2d974b8 100644 --- a/src/precheck/mod.rs +++ b/src/precheck/mod.rs @@ -363,7 +363,15 @@ pub fn run_install(manager: PackageManager, cmd: &[String], opts: PrecheckOption } if !manager.is_install_subcommand(subcommand) { - // Non-install subcommand: transparent passthrough, args untouched. + // Non-install subcommand: transparent passthrough, args untouched — + // but `yarn global add` installs from the registry, so disclose + // that it isn't gated rather than pass silently. + if manager == PackageManager::Yarn + && subcommand == "global" + && cmd.get(verb_idx + 1).map(String::as_str) == Some("add") + { + eprintln!("note: 'yarn global add' is not gated; packages install unchecked"); + } return exec::exec_command(manager.binary_name(), cmd); } diff --git a/src/precheck/parse.rs b/src/precheck/parse.rs index 7a61d92..a261341 100644 --- a/src/precheck/parse.rs +++ b/src/precheck/parse.rs @@ -152,14 +152,22 @@ pub fn uv_subcommand(cmd: &[String]) -> &[String] { pub fn classify_uv_command(cmd: &[String]) -> UvCommand<'_> { match cmd.first().map(String::as_str) { - Some("pip") if matches!(cmd.get(1).map(String::as_str), Some("install" | "i")) => { - UvCommand::PipInstall { - install_args: &cmd[2..], + Some("pip") => { + // uv accepts flags between `pip` and its verb + // (`uv pip --quiet install x` is valid) — skip them like + // `uv_subcommand` does, or the verb lands in Passthrough + // ungated. + let rest = uv_subcommand(&cmd[1..]); + match rest.first().map(String::as_str) { + Some("install" | "i") => UvCommand::PipInstall { + install_args: &rest[1..], + }, + Some("sync") => UvCommand::PipSync { + sync_args: &rest[1..], + }, + _ => UvCommand::Passthrough, } } - Some("pip") if cmd.get(1).map(String::as_str) == Some("sync") => UvCommand::PipSync { - sync_args: &cmd[2..], - }, Some("add") => UvCommand::Add { add_args: &cmd[1..], }, @@ -436,6 +444,10 @@ pub(super) fn takes_value(manager: PackageManager, flag: &str) -> bool { | "--global-folder" | "--link-folder" | "--preferred-cache-folder" + // yarn-classic's monorepo dir redirect: missing it makes + // `yarn --cwd packages/app add x` classify the VALUE as the + // verb → ungated passthrough. + | "--cwd" ), PackageManager::Uv => matches!( flag, @@ -466,6 +478,18 @@ pub(super) fn takes_value(manager: PackageManager, flag: &str) -> bool { | "--project" | "--config-setting" | "--link-mode" + // uv global options: a valued flag missing here makes its + // VALUE classify as the subcommand → ungated passthrough + // (`uv --color always add x` must still gate the add). + | "--color" + | "--config-file" + | "--cache-dir" + | "--allow-insecure-host" + // `uv add` value flags whose bare-word values would + // otherwise parse as package specs. + | "--optional" + | "--bounds" + | "--script" ), PackageManager::Pip => matches!( flag, @@ -1053,6 +1077,81 @@ mod tests { ); } + #[test] + fn uv_valued_global_flags_do_not_swallow_the_subcommand() { + // SECURITY: a valued global flag missing from `takes_value` makes + // its VALUE classify as the subcommand → silent ungated + // passthrough. `uv --color always add x` must still gate the add. + for (flag, value) in [ + ("--color", "always"), + ("--config-file", "uv.toml"), + ("--cache-dir", "/tmp/cache"), + ("--allow-insecure-host", "example.com"), + ] { + let cmd = vec![ + flag.to_string(), + value.to_string(), + "add".to_string(), + "requests".to_string(), + ]; + let sub = uv_subcommand(&cmd); + assert_eq!( + sub.first().map(String::as_str), + Some("add"), + "{flag} must skip its value, not surface it as the verb" + ); + } + } + + #[test] + fn uv_pip_flags_between_pip_and_verb_still_classify() { + // `uv pip --quiet install x` is a valid uv invocation; the flag + // between `pip` and the verb must not push it to Passthrough. + assert!(matches!( + classify_uv_command(&[ + "pip".to_string(), + "--quiet".to_string(), + "install".to_string(), + "requests".to_string(), + ]), + UvCommand::PipInstall { .. } + )); + assert!(matches!( + classify_uv_command(&[ + "pip".to_string(), + "--python".to_string(), + "3.12".to_string(), + "sync".to_string(), + "reqs.txt".to_string(), + ]), + UvCommand::PipSync { .. } + )); + } + + #[test] + fn uv_add_value_flags_do_not_parse_as_specs() { + // `uv add --optional cli rich` adds `rich` to the `cli` extra — + // `cli` is a flag value, not a package (it's a real PyPI name, so + // misparsing it would false-block or noise the report). + let p = extract_node_positionals( + PackageManager::Uv, + &[ + "--optional".to_string(), + "cli".to_string(), + "rich".to_string(), + ], + ); + assert_eq!(p.specs, vec!["rich".to_string()]); + } + + #[test] + fn yarn_cwd_value_does_not_swallow_the_verb() { + // SECURITY: yarn-classic's `--cwd ` takes a value; without it + // in `takes_value` the DIRECTORY becomes the verb and + // `yarn --cwd packages/app add lodash` execs ungated. + assert!(takes_value(PackageManager::Yarn, "--cwd")); + } + #[test] fn uv_add_positionals_parse_as_pypi_specs() { let parsed = parse_pypi_positionals_args(&["requests==2.31.0".into()]); diff --git a/src/precheck/uv.rs b/src/precheck/uv.rs index f97ca18..26bd722 100644 --- a/src/precheck/uv.rs +++ b/src/precheck/uv.rs @@ -17,8 +17,13 @@ pub(super) fn run_uv(cmd: &[String], opts: PrecheckOptions) -> i32 { } match parse::classify_uv_command(sub) { - // Passthrough is a transparent shim: no report, untouched stdio. - parse::UvCommand::Passthrough => exec::exec_command("uv", cmd), + // Passthrough is a transparent shim: no report, untouched stdio — + // but uv subcommands that install packages get an honesty note + // first, mirroring the bare yarn/pnpm disclosure. + parse::UvCommand::Passthrough => { + uv_ungated_install_note(sub); + exec::exec_command("uv", cmd) + } parse::UvCommand::PipInstall { install_args } => { let parsed = match parse::parse_pip_install_args(install_args) { Ok(p) => p, @@ -74,7 +79,7 @@ pub(super) fn run_uv(cmd: &[String], opts: PrecheckOptions) -> i32 { } super::run_parsed_install(PackageManager::Uv, "add", add_args, parsed, exec, opts) } - parse::UvCommand::Sync => run_uv_sync(cmd, opts, exec), + parse::UvCommand::Sync => run_uv_sync(sub, opts, exec), } } @@ -85,6 +90,29 @@ fn unsupported_uv_install_message(rest: &[String]) -> String { ) } +/// Honesty note for passthrough uv subcommands that install packages: +/// `uv run` syncs the project environment on first run (and `--with` +/// installs arbitrary packages); `uv tool install` / `uv tool run` install +/// from the index. None are gated — say so instead of passing silently, +/// matching the bare yarn/pnpm note. +fn uv_ungated_install_note(sub: &[String]) { + let label = match sub.first().map(String::as_str) { + Some("run") => "uv run", + Some("tool") + if matches!( + sub.get(1).map(String::as_str), + Some("install" | "run" | "upgrade") + ) => + { + "uv tool" + } + _ => return, + }; + eprintln!( + "note: '{label}' may install packages (project sync / --with / tool installs); these are not gated" + ); +} + /// Gate `uv sync` from the project's `uv.lock`. The lockfile is the full /// locked universe (all groups/extras) — a superset of what sync installs, /// conservative in the blocking direction; a stale lock that sync would @@ -92,7 +120,7 @@ fn unsupported_uv_install_message(rest: &[String]) -> String { /// aren't newly chosen by this command); the verdict pass is the gate. We /// never run `uv lock` ourselves — locking can build sdists, which would /// execute package code before any verdict. -fn run_uv_sync(cmd: &[String], opts: PrecheckOptions, exec: impl FnOnce() -> i32) -> i32 { +fn run_uv_sync(sub: &[String], opts: PrecheckOptions, exec: impl FnOnce() -> i32) -> i32 { if opts.verdict.is_none() { // Direct callers may still disable verdicts completely. return exec(); @@ -107,26 +135,61 @@ fn run_uv_sync(cmd: &[String], opts: PrecheckOptions, exec: impl FnOnce() -> i32 }; let lock = std::fs::read_to_string(&lock_path) .map_err(|e| format!("read {}: {e}", lock_path.display())) - .and_then(|content| parse_uv_lock(&content)); + .and_then(|content| parse_uv_lock(&content)) + .map(|set| { + set.print_notes(); + set.packages + }); + // `sub` starts at the sync verb (global flags already skipped), so the + // echoed command renders `uv sync ` — not the verb twice. super::run_locked_install( PackageManager::Uv, "sync", - cmd[1..].to_vec(), + sub[1..].to_vec(), lock, &opts, exec, ) } +/// Parsed `uv.lock` verdict set plus the honesty notes the lock carried: +/// how many locked packages have no registry identity (skipped), and any +/// non-default registries the pins resolve from. +#[derive(Debug)] +struct UvLockSet { + packages: Vec, + skipped_non_registry: usize, + non_default_registries: std::collections::BTreeSet, +} + +impl UvLockSet { + fn print_notes(&self) { + if self.skipped_non_registry > 0 { + eprintln!( + "note: {} non-registry locked package(s) (git/url/path/workspace) are not verified", + self.skipped_non_registry + ); + } + for registry in &self.non_default_registries { + eprintln!( + "warning: uv.lock pins resolve from {registry} — the gate verdicts name@version against the public PyPI data and cannot vouch that registry's artifacts match" + ); + } + } +} + /// Packages from `uv.lock` that `uv sync` installs from an index. Only /// registry-sourced packages carry a name@version the public vuln-api can /// verify, so: /// * non-registry sources (editable / virtual / directory / path — local; -/// git / url — direct artifacts) are skipped: sending their name@version -/// to PyPI would verdict an unrelated package; +/// git / url — direct artifacts) are skipped, with a count so the skip +/// is disclosed (the named-target path warns for the same shapes); /// * a registry package missing a `version` is a parse error (fail closed) -/// rather than a silent omission that installs unchecked. -fn parse_uv_lock(content: &str) -> Result, String> { +/// rather than a silent omission that installs unchecked; +/// * non-default registry URLs are collected for a warning — the gate +/// verdicts against public PyPI data and can't vouch a private index's +/// artifact matches. +fn parse_uv_lock(content: &str) -> Result { #[derive(serde::Deserialize)] struct Lock { #[serde(default)] @@ -142,16 +205,30 @@ fn parse_uv_lock(content: &str) -> Result, String> { // Sources that are NOT a registry/index: skip them entirely. const NON_REGISTRY_SOURCES: [&str; 6] = ["editable", "virtual", "directory", "path", "git", "url"]; + const DEFAULT_REGISTRY: &str = "https://pypi.org/simple"; let lock: Lock = toml::from_str(content).map_err(|e| format!("parse uv.lock: {e}"))?; - let mut out = Vec::new(); + let mut set = UvLockSet { + packages: Vec::new(), + skipped_non_registry: 0, + non_default_registries: std::collections::BTreeSet::new(), + }; for p in lock.package { if NON_REGISTRY_SOURCES .iter() .any(|k| p.source.contains_key(*k)) { + set.skipped_non_registry += 1; continue; } + if let Some(registry) = p.source.get("registry").and_then(|v| v.as_str()) { + if !registry + .trim_end_matches('/') + .eq_ignore_ascii_case(DEFAULT_REGISTRY) + { + set.non_default_registries.insert(registry.to_string()); + } + } // Registry (or default-index) package — it must carry a version. let version = p.version.ok_or_else(|| { format!( @@ -159,13 +236,13 @@ fn parse_uv_lock(content: &str) -> Result, String> { p.name ) })?; - out.push(tree::TreePackage { + set.packages.push(tree::TreePackage { name: p.name, version, requested: false, }); } - Ok(out) + Ok(set) } #[cfg(test)] @@ -196,10 +273,35 @@ name = "gitdep" version = "1.2.3" source = { git = "https://example.com/repo?rev=abc#abc" } "#; - let pkgs = parse_uv_lock(lock).expect("parse uv.lock"); - let names: Vec<&str> = pkgs.iter().map(|p| p.name.as_str()).collect(); + let set = parse_uv_lock(lock).expect("parse uv.lock"); + let names: Vec<&str> = set.packages.iter().map(|p| p.name.as_str()).collect(); assert_eq!(names, vec!["evildep"]); - assert_eq!(pkgs[0].version, "0.4.2"); + assert_eq!(set.packages[0].version, "0.4.2"); + // The two skipped sources are counted so the gate can disclose them. + assert_eq!(set.skipped_non_registry, 2); + // pypi.org/simple is the default index — no registry warning. + assert!(set.non_default_registries.is_empty()); + } + + #[test] + fn parse_uv_lock_collects_non_default_registries() { + // A pin resolving from a private index gets verdicted by + // name@version against public PyPI data — the mismatch risk must + // surface as a warning, so the registry URL is collected. + let lock = r#" +version = 1 + +[[package]] +name = "innerpkg" +version = "2.0.0" +source = { registry = "https://private.example/simple" } +"#; + let set = parse_uv_lock(lock).expect("parse uv.lock"); + assert_eq!(set.packages.len(), 1); + assert_eq!( + set.non_default_registries.iter().collect::>(), + vec!["https://private.example/simple"] + ); } #[test] diff --git a/tests/cli_bare_install.rs b/tests/cli_bare_install.rs index 1eb9be6..646525e 100644 --- a/tests/cli_bare_install.rs +++ b/tests/cli_bare_install.rs @@ -233,6 +233,37 @@ fn bare_ungated_managers_print_note_and_exec() { } } +#[test] +fn yarn_cwd_value_does_not_bypass_the_gate() { + // SECURITY: yarn-classic's `--cwd ` takes a value; if the + // directory is mistaken for the verb, `yarn --cwd packages/app add x` + // execs as ungated passthrough. The vulnerable named target must + // still block. + let mut checks = HashMap::new(); + checks.insert( + key("npm", "oldpkg", "1.0.0"), + vulnerable_body("npm", "oldpkg", "1.0.0", "MAL-2024-0007", None), + ); + let mut h = GateHarness::new() + .fake_recorder("yarn", 0) + .oldpkg_registry() + .vuln_checks(checks) + .in_project_dir() + .build(); + let out = h + .cmd + .args(["yarn", "--cwd", "packages/app", "add", "oldpkg@1.0.0"]) + .output() + .expect("run corgea"); + assert_eq!( + out.status.code(), + Some(1), + "--cwd's value must not swallow the verb: {}", + String::from_utf8_lossy(&out.stderr) + ); + assert_eq!(h.recorded_argv(), None, "yarn must not run when blocked"); +} + #[test] fn yarn_named_target_does_not_print_bare_note() { // A named target takes the gated path: named-only warning, no bare note. diff --git a/tests/cli_uv_sync.rs b/tests/cli_uv_sync.rs index fbdcfa8..33d698d 100644 --- a/tests/cli_uv_sync.rs +++ b/tests/cli_uv_sync.rs @@ -204,6 +204,54 @@ fn uv_global_flags_before_subcommand_still_gate() { assert_eq!(h.recorded_argv(), None, "uv must not run when blocked"); } +#[test] +fn uv_valued_global_flag_before_subcommand_still_gates() { + // SECURITY: `--color always` takes a value; if the value is mistaken + // for the subcommand, `uv --color always sync` (and `… add x`) execs + // as ungated passthrough. The boolean-flag test above can't catch + // this class. + let mut h = GateHarness::new() + .fake_recorder("uv", 0) + .vuln_checks(vulnerable_evildep_checks()) + .with_project_file("uv.lock", UV_LOCK) + .build(); + let out = h + .cmd + .args(["uv", "--color", "always", "sync"]) + .output() + .expect("run corgea"); + assert_eq!( + out.status.code(), + Some(1), + "a valued global flag must not swallow the subcommand: {}", + String::from_utf8_lossy(&out.stderr) + ); + assert_eq!(h.recorded_argv(), None, "uv must not run when blocked"); +} + +#[test] +fn uv_run_prints_ungated_note() { + // `uv run` syncs the project environment on first run and `--with` + // installs packages — none of it gated. The passthrough must say so + // instead of staying silent (SKILL.md lists it as a limitation). + let mut h = GateHarness::new() + .fake_recorder("uv", 0) + .vuln_checks(HashMap::new()) + .build(); + let out = h + .cmd + .args(["uv", "run", "pytest"]) + .output() + .expect("run corgea"); + assert_eq!(out.status.code(), Some(0), "passthrough forwards to uv"); + assert_eq!(h.recorded_argv().as_deref(), Some("run pytest")); + let stderr = String::from_utf8_lossy(&out.stderr); + assert!( + stderr.contains("'uv run'") && stderr.contains("not gated"), + "uv run must disclose it is ungated: {stderr}" + ); +} + #[test] fn uv_pip_install_in_requirements_project_is_not_wrong_manager() { // `uv pip install` is uv's pip-compatible interface — using it in a