diff --git a/crates/socket-patch-cli/CLI_CONTRACT.md b/crates/socket-patch-cli/CLI_CONTRACT.md index 30e0e1dd..c6c9a854 100644 --- a/crates/socket-patch-cli/CLI_CONTRACT.md +++ b/crates/socket-patch-cli/CLI_CONTRACT.md @@ -90,7 +90,7 @@ Beyond the globals above, each subcommand defines a small set of local arguments `scan` queries the patch API in `--batch-size` chunks. Authenticated runs POST `/v0/orgs/{slug}/patches/batch`; token-less runs POST `{proxy}/patch/batch` on the public proxy and degrade to per-package `GET /patch/by-package/:purl` requests in two cases: the deployed proxy predates the batch endpoint (legacy proxies answer the POST with their `400 "Unsupported endpoint"` catch-all), or the all-or-nothing batch validation rejects the chunk (e.g. a crawled PURL type the server doesn't recognize, such as `pkg:jsr/…` — the per-package path tolerates those individually, preserving the pre-batch scan semantics). Rate limits and over-capacity 503s surface instead of silently degrading. -**Lockfile supplement (v3.4)**: `scan` discovery is no longer limited to installed trees. The project's lockfiles (`package-lock.json`/`npm-shrinkwrap.json`, `pnpm-lock.yaml` v9, `yarn.lock` classic + berry, `bun.lock`, `Cargo.lock`, `go.sum`, `composer.lock`, `Gemfile.lock`, `uv.lock`/`poetry.lock`/pinned `requirements.txt`) are inventoried and dependencies with NO installed copy join discovery — counts, the API lookup, the table (flagged ` [NOT INSTALLED]`, plus a stderr note), and the prune "scanned" set (a wiped node_modules no longer prunes lockfile-listed entries). JSON gains a top-level `lockfileOnlyPackages` count and an additive `notInstalled: true` on matching `packages[]` entries. `--apply` partitions lockfile-only patches out BEFORE download (calm `skipped`/`package_not_installed` records — never an error exit, never a manifest write); `--vendor` passes them through to the vendor engine's auto-fetch. Vendored-ledger entries likewise stay discoverable on a fresh clone (the committed artifact is the dependency). Global scans (`--global`) get no supplement. **Rush monorepos** (no root lockfile, `rush.json` present): the npm-lock inventory falls back to the Rush source-of-truth locks — `common/config/rush/pnpm-lock.yaml` plus every `common/config/subspaces/*/pnpm-lock.yaml` (`read_dir`-sorted, repo-relative paths preserved) — so a Rush repo's dependencies still join discovery. +**Lockfile supplement (v3.4)**: `scan` discovery is no longer limited to installed trees. The project's lockfiles (`package-lock.json`/`npm-shrinkwrap.json`, `pnpm-lock.yaml` v9, `yarn.lock` classic + berry, `bun.lock`, `Cargo.lock`, `go.sum`, `composer.lock`, `Gemfile.lock`, `uv.lock`/`poetry.lock`/pinned `requirements.txt`) are inventoried and dependencies with NO installed copy join discovery — counts, the API lookup, the table (flagged ` [NOT INSTALLED]`, plus a stderr note), and the prune "scanned" set (a wiped node_modules no longer prunes lockfile-listed entries). JSON gains a top-level `lockfileOnlyPackages` count and an additive `notInstalled: true` on matching `packages[]` entries. `--apply` partitions lockfile-only patches out BEFORE download (calm `skipped`/`package_not_installed` records — never an error exit, never a manifest write); `--vendor` passes them through to the vendor engine's auto-fetch. Vendored-ledger entries likewise stay discoverable on a fresh clone (the committed artifact is the dependency). Global scans (`--global`) get no supplement. **Rush monorepos** (no root lockfile, `rush.json` present): the npm-lock inventory falls back to the Rush source-of-truth locks — `common/config/rush/pnpm-lock.yaml` plus every `common/config/subspaces/*/pnpm-lock.yaml` (`read_dir`-sorted, repo-relative paths preserved) — so a Rush repo's dependencies still join discovery. **Plug'n'Play layouts are an explicit refusal, not an empty inventory**: a `.pnp.*` loader means the npm packages are structurally unreachable in EVERY mode (under yarn PnP the installed-tree crawl is empty too — no `node_modules/`), so `scan` surfaces an additive top-level `warnings[]` array (`{code, detail}` objects, omitted when empty) carrying `yarn_pnp_unsupported` (same code as apply's refusal; remedy `yarn patch `) or `pnpm_pnp_unsupported` (pnpm's `node-linker=pnp` twin; pnpm remedies), plus a stderr `Warning (): …` line on the human path. Exit code and `status` are deliberately unchanged (exit 0 / `success` — the same posture as hosted refusals, which exit 0 with `redirected: 0`); the warning is the machine-readable signal that nothing was checked. Pinned by `tests/e2e_safety_yarn_pnp.rs`. **Vendor auto-fetch (v3.4)**: `vendor`/`scan --vendor` no longer fail on lockfile-resolved packages with no installed copy. Already-vendored purls stage from their committed artifact (sha256-verified against the vendor ledger; offline-safe). Otherwise the pristine artifact is fetched per the lockfile resolution and verified against the lock's recorded integrity FAIL-CLOSED before any write: npm SRI (or yarn classic's sha1 fragment), yarn berry's cache-zip checksum (rebuilt from the fetched tarball; cacheKey 10c0 only), Cargo.lock sha256 over the .crate, go.sum `h1:` dirhash over the module zip, composer `dist.shasum` (sha1), Gemfile.lock `CHECKSUMS` sha256, uv.lock wheel sha256 (pure `py3-none-any` wheels only). Entries the lock cannot verify are NEVER fetched (`vendor_fetch_unverifiable` warning + the calm `package_not_installed` skip). Registry bases honor `SOCKET_NPM_REGISTRY`, `SOCKET_CRATES_REGISTRY`, `SOCKET_GOPROXY` (else `GOPROXY`); npm/yarn/composer/gem/uv lock-recorded URLs are used verbatim. `--offline` refuses the fetch with the calm skip (the detail names the lockfile resolution). The fetch stages into a private tempdir — the project tree is never touched. @@ -1101,7 +1101,7 @@ socket-patch apply --json | jq ' Exit `0` when `status` is `success`, `noManifest`, or `notFound`-with-zero-failed. Exit `1` when `status` is `partialFailure` (any `events[*].action == "failed"`) or `error`. -`apply` with no manifest at all is a clean exit-0 no-op (`status: "noManifest"`), and an **empty** manifest (zero patches) is a plain `success` exit 0 — this is load-bearing for the install hooks, which run `apply` on every install. Pinned by `tests/in_process_edge_cases.rs` and `tests/cli_dry_run_paths_e2e.rs`. +`apply` with no manifest at all is a clean exit-0 no-op (`status: "noManifest"`), and an **empty** manifest (zero patches) is a plain `success` exit 0 — this is load-bearing for the install hooks, which run `apply` on every install. Pinned by `tests/in_process_edge_cases.rs` and `tests/cli_dry_run_paths_e2e.rs`. **One carve-out**: a yarn-berry Plug'n'Play layout (`.pnp.*` loader at `--cwd`) refuses with the loud `yarn_pnp_unsupported` error (exit 1) even when no manifest exists — `scan` cannot discover PnP packages (they live inside `.yarn/cache/*.zip`, no `node_modules/`) and therefore never writes a manifest, so without the carve-out the documented refusal was unreachable and a PnP project's only signal was the calm noManifest exit. Pinned by `tests/e2e_safety_yarn_pnp.rs`. ## Exit codes diff --git a/crates/socket-patch-cli/src/commands/apply.rs b/crates/socket-patch-cli/src/commands/apply.rs index ee3b0a2e..82d47c3f 100644 --- a/crates/socket-patch-cli/src/commands/apply.rs +++ b/crates/socket-patch-cli/src/commands/apply.rs @@ -589,6 +589,33 @@ pub(crate) fn result_to_event(result: &ApplyResult, dry_run: bool) -> PatchEvent PatchEvent::new(PatchAction::Applied, purl).with_files(files) } +/// Print the yarn-PnP refusal (JSON envelope or human stderr) and return +/// apply's refusal exit code. Shared by the pre-manifest gate and the +/// package-manager layout gate below: scan cannot discover PnP packages so +/// it never writes a manifest, which used to leave the calm `noManifest` +/// exit as the ONLY thing a PnP user ever saw — the documented loud +/// `yarn_pnp_unsupported` refusal was unreachable without a manifest. +fn refuse_yarn_pnp(args: &ApplyArgs) -> i32 { + if args.common.json { + let mut env = Envelope::new(Command::Apply); + env.dry_run = args.common.dry_run; + env.mark_error(EnvelopeError::new( + "yarn_pnp_unsupported", + "yarn-berry Plug'n'Play layout is not supported by socket-patch (packages live inside .yarn/cache zips). Use `yarn patch ` instead.", + )); + println!("{}", env.to_pretty_json()); + } else { + // Errors print even under --silent ("errors only", never + // "nothing"): exit 1 with no message would be undiagnosable. + eprintln!("Error: yarn-berry Plug'n'Play layout is not supported."); + eprintln!( + " Packages live inside .yarn/cache/*.zip — socket-patch cannot rewrite them in place." + ); + eprintln!(" Use `yarn patch ` instead."); + } + 1 +} + pub async fn run(args: ApplyArgs) -> i32 { apply_env_toggles(&args.common); let (telemetry_client, _) = @@ -600,6 +627,18 @@ pub async fn run(args: ApplyArgs) -> i32 { // Check if manifest exists - exit successfully if no .socket folder is set up if tokio::fs::metadata(&manifest_path).await.is_err() { + // A yarn-PnP layout refuses loudly even with no manifest: scan + // cannot discover PnP packages (they live inside .yarn/cache zips), + // so it never writes one — without this hoisted check the layout + // gate further down never fired and the ONLY signal a PnP project + // ever produced was this calm exit-0 noManifest, i.e. a silent + // no-op. Same envelope + exit semantics as the with-manifest gate. + if matches!( + detect_npm_pkg_manager(&args.common.cwd), + NpmPkgManager::YarnBerryPnP + ) { + return refuse_yarn_pnp(&args); + } if args.common.json { let mut env = Envelope::new(Command::Apply); env.status = Status::NoManifest; @@ -642,24 +681,7 @@ pub async fn run(args: ApplyArgs) -> i32 { // in `apply_file_patch` does the substantive safety work. match detect_npm_pkg_manager(&args.common.cwd) { NpmPkgManager::YarnBerryPnP => { - if args.common.json { - let mut env = Envelope::new(Command::Apply); - env.dry_run = args.common.dry_run; - env.mark_error(EnvelopeError::new( - "yarn_pnp_unsupported", - "yarn-berry Plug'n'Play layout is not supported by socket-patch (packages live inside .yarn/cache zips). Use `yarn patch ` instead.", - )); - println!("{}", env.to_pretty_json()); - } else { - // Errors print even under --silent ("errors only", never - // "nothing"): exit 1 with no message would be undiagnosable. - eprintln!("Error: yarn-berry Plug'n'Play layout is not supported."); - eprintln!( - " Packages live inside .yarn/cache/*.zip — socket-patch cannot rewrite them in place." - ); - eprintln!(" Use `yarn patch ` instead."); - } - return 1; + return refuse_yarn_pnp(&args); } NpmPkgManager::Pnpm => { if !args.common.json && !args.common.silent { diff --git a/crates/socket-patch-cli/src/commands/scan/discovery.rs b/crates/socket-patch-cli/src/commands/scan/discovery.rs index bcf5331c..93e6b6fd 100644 --- a/crates/socket-patch-cli/src/commands/scan/discovery.rs +++ b/crates/socket-patch-cli/src/commands/scan/discovery.rs @@ -27,6 +27,46 @@ pub(super) struct LockfileSupplement { pub(super) packages: Vec, /// Literal crawler-form purls, for fast membership tests. pub(super) purls: HashSet, + /// npm layouts the lockfile inventory REFUSED (Plug'n'Play loaders) — + /// packages structurally unreachable, as opposed to nothing-to-inventory. + /// Scan surfaces these as explicit refusal warnings: under yarn PnP the + /// installed-tree crawl is also empty (no `node_modules/`), so without + /// this channel a PnP project scans as a silent success-0 no-op in + /// every mode. + pub(super) unsupported: Vec, +} + +/// Map a core npm-layout refusal onto scan's warning channel as +/// `(code, detail)`. The yarn code matches apply's refusal errorCode +/// (`yarn_pnp_unsupported`) so consumers key on ONE name across commands; +/// the pnpm twin gets the parallel spelling. Details are scan-phrased (what +/// was NOT scanned + remedy) rather than the probe's vendor-phrased text. +pub(super) fn unsupported_layout_warnings(sup: &LockfileSupplement) -> Vec<(String, String)> { + sup.unsupported + .iter() + .map(|diag| match diag.code { + "vendor_yarn_berry_unsupported" => ( + "yarn_pnp_unsupported".to_string(), + "this project uses yarn Plug'n'Play (a `.pnp.*` loader is present): its npm \ + packages live inside `.yarn/cache/*.zip`, not `node_modules/`, so socket-patch \ + cannot discover or patch them in ANY mode (agent, hosted, or vendored) — npm \ + dependencies were NOT scanned. Use `yarn patch ` to patch them instead." + .to_string(), + ), + "vendor_pnpm_pnp_unsupported" => ( + "pnpm_pnp_unsupported".to_string(), + "this project uses pnpm's Plug'n'Play linker (`node-linker=pnp` in .npmrc): \ + lockfile discovery is skipped under this layout, so lockfile-only npm \ + dependencies were NOT scanned. Switch .npmrc to `node-linker=isolated`, run \ + `pnpm install`, and re-run — or use `socket-patch scan --mode hosted`, which \ + edits pnpm-lock.yaml in place." + .to_string(), + ), + // Forward-compat: a new refusal code surfaces verbatim rather + // than being swallowed back into silence. + other => (other.to_string(), diag.detail.clone()), + }) + .collect() } /// Inventory the project's lockfile(s) and fabricate crawl entries for @@ -45,7 +85,8 @@ pub(super) async fn lockfile_supplement( if common.global || common.global_prefix.is_some() { return out; } - let entries = lock_inventory::inventory_project(&common.cwd).await; + let (entries, unsupported) = lock_inventory::inventory_project_diagnosed(&common.cwd).await; + out.unsupported = unsupported; if entries.is_empty() { return out; } diff --git a/crates/socket-patch-cli/src/commands/scan/mod.rs b/crates/socket-patch-cli/src/commands/scan/mod.rs index d112ced1..be2a0fff 100644 --- a/crates/socket-patch-cli/src/commands/scan/mod.rs +++ b/crates/socket-patch-cli/src/commands/scan/mod.rs @@ -36,7 +36,8 @@ mod vendor_flow; use self::discovery::{ collect_vuln_ids, detect_updates, lockfile_supplement, merge_redirect_records_for_updates, - preverify_vendor_baselines, severity_order, vendored_ledger_supplement, + preverify_vendor_baselines, severity_order, unsupported_layout_warnings, + vendored_ledger_supplement, }; use self::gc::{gc_json, print_gc_vendored_line, run_apply_gc}; use self::hosted::run_redirect; @@ -957,6 +958,18 @@ pub(super) async fn note_vendor_supersedes_redirect( }); } +/// Top-level `warnings[]` JSON for scan's envelope from `(code, detail)` +/// pairs (see [`unsupported_layout_warnings`]). Same `{code, detail}` object +/// shape as the run-level `warnings[]` on the unified envelope. +fn layout_refusal_json(refusals: &[(String, String)]) -> serde_json::Value { + serde_json::Value::Array( + refusals + .iter() + .map(|(code, detail)| serde_json::json!({ "code": code, "detail": detail })) + .collect(), + ) +} + pub async fn run(mut args: ScanArgs) -> i32 { apply_env_toggles(&args.common); @@ -1085,6 +1098,16 @@ pub async fn run(mut args: ScanArgs) -> i32 { // discovery — counts, API lookup, table, the prune "scanned" set — and // are flagged "not yet installed" everywhere a user could act on them. let lockfile_only = lockfile_supplement(&args.common, &all_crawled).await; + // Explicit refusals for npm layouts whose packages are structurally + // unreachable (yarn PnP, pnpm node-linker=pnp). Under yarn PnP the + // crawler leg above is ALSO empty (no `node_modules/`), so without this + // channel every mode used to print a clean success with + // `scannedPackages: 0` — a silent no-op the user read as "protected". + // Surfaced as run-level `warnings[]` in the JSON envelope (omitted when + // empty) and a stderr line on the human path; exit code and `status` + // stay deliberately unchanged (same posture as hosted refusals, which + // exit 0 with `redirected: 0`). + let layout_refusals = unsupported_layout_warnings(&lockfile_only); if !lockfile_only.packages.is_empty() { for pkg in &lockfile_only.packages { if let Some(eco) = Ecosystem::from_purl(&pkg.purl) { @@ -1143,6 +1166,11 @@ pub async fn run(mut args: ScanArgs) -> i32 { if show_progress { eprintln!(); } + if !args.common.json && !args.common.silent { + for (code, detail) in &layout_refusals { + eprintln!("Warning ({code}): {detail}"); + } + } // Telemetry: empty-scan still counts as a successful scan. track_patch_scanned( 0, @@ -1177,6 +1205,14 @@ pub async fn run(mut args: ScanArgs) -> i32 { "packages": [], "updates": [], }); + // PnP layout refusals: additive top-level `warnings` (omitted + // when empty — run-level warnings precedent) so a JSON consumer + // can tell "structurally unscannable project" apart from a + // genuinely-empty one. This is the loud half of the fix for the + // yarn-PnP silent success-0 no-op. + if !layout_refusals.is_empty() { + result["warnings"] = layout_refusal_json(&layout_refusals); + } // Hosted mode: keep the `--json` envelope schema-consistent with // the ≥1-package path by including a (no-op) nested `redirect` // block — nothing was discovered, so nothing is redirected. The @@ -1257,6 +1293,12 @@ pub async fn run(mut args: ScanArgs) -> i32 { lockfile_only.purls.len(), ); } + // Polyglot PnP repos (e.g. a PnP frontend + a python venv) reach + // this non-empty path: the refusal still prints so the invisible + // npm half is never silently blessed by the other ecosystems' scan. + for (code, detail) in &layout_refusals { + eprintln!("Warning ({code}): {detail}"); + } } // Query API in batches @@ -1498,6 +1540,13 @@ pub async fn run(mut args: ScanArgs) -> i32 { "newUuid": u.new_uuid, })).collect::>(), }); + // PnP layout refusals ride the non-empty envelope too (polyglot + // repos: the OTHER ecosystems' discovery being non-empty must not + // silently bless the structurally-invisible npm half). Additive, + // omitted when empty. + if !layout_refusals.is_empty() { + result["warnings"] = layout_refusal_json(&layout_refusals); + } // Flag lockfile-only packages so JSON consumers can tell "patch // available but not installed" from the installed case. Additive // field; absent means installed. Matching bridges the API's diff --git a/crates/socket-patch-cli/tests/e2e_safety_yarn_pnp.rs b/crates/socket-patch-cli/tests/e2e_safety_yarn_pnp.rs index 0d37431c..9aa6a635 100644 --- a/crates/socket-patch-cli/tests/e2e_safety_yarn_pnp.rs +++ b/crates/socket-patch-cli/tests/e2e_safety_yarn_pnp.rs @@ -786,6 +786,300 @@ fn pnp_project_with_no_npm_patches_still_applies_its_other_patches() { ); } +// ── the silent no-op (P0) ──────────────────────────────────────────────────── +// +// Under PnP, `node_modules/` is absent, so the crawler leg of scan discovery +// is empty; the lockfile-supplement leg used to swallow the flavor probe's +// PnP diagnosis and return nothing. Scan then hit its `package_count == 0` +// early-return and printed `status: success` / `scannedPackages: 0` with NO +// warning — in ALL THREE modes — and, because scan wrote no manifest, +// apply's documented loud refusal above was unreachable (apply exited 0 with +// the calm `noManifest` status). The user believed they were protected; +// nothing was checked. Reproduced on yarn 2.4.3 / 3.8.7 / 4.6.0, plain PnP +// and zero-install. The tests below pin the fix: an explicit +// `yarn_pnp_unsupported` refusal warning in scan's JSON envelope (exit +// semantics deliberately unchanged: still exit 0 / status success), a stderr +// line in human mode, and a loud apply refusal even without a manifest. + +/// Recursively snapshot every file under `root` as relative path → git +/// sha256, for whole-tree no-mutation assertions. +fn snapshot_tree(root: &Path) -> std::collections::BTreeMap { + fn walk(root: &Path, dir: &Path, out: &mut std::collections::BTreeMap) { + for entry in + std::fs::read_dir(dir).unwrap_or_else(|e| panic!("read_dir {}: {e}", dir.display())) + { + let entry = entry.unwrap(); + let path = entry.path(); + if path.is_dir() { + walk(root, &path, out); + } else { + let rel = path + .strip_prefix(root) + .expect("walk stays under root") + .to_string_lossy() + .to_string(); + out.insert(rel, common::git_sha256_file(&path)); + } + } + } + let mut out = std::collections::BTreeMap::new(); + walk(root, root, &mut out); + out +} + +/// The scan-side fixture: [`make_yarn_berry_project`] plus the berry +/// lockfile and a committed cache zip, mirroring a real PnP (zero-install) +/// checkout. No `node_modules/` — under PnP there is none, which is exactly +/// why the crawler leg finds nothing. +fn make_yarn_berry_scan_fixture(cwd: &Path) { + make_yarn_berry_project(cwd); + std::fs::write( + cwd.join("yarn.lock"), + "# This file is generated by running \"yarn install\" inside your project.\n\ + # Manifest files (package.json) are also used.\n\n\ + __metadata:\n version: 8\n cacheKey: 10c0\n\n\ + \"dummy@npm:1.0.0\":\n version: 1.0.0\n resolution: \"dummy@npm:1.0.0\"\n checksum: 10c0/abc\n", + ) + .expect("write berry yarn.lock"); + std::fs::write( + cwd.join(".yarn") + .join("cache") + .join("dummy-npm-1.0.0-abc123-10c0.zip"), + b"PK\x05\x06 stub zip bytes", + ) + .expect("write cache zip"); +} + +/// Assert the scan envelope carries the explicit PnP refusal warning: +/// a run-level `warnings[]` entry with the stable code (same vocabulary as +/// apply's refusal) whose detail names the layout and the workaround. +fn assert_scan_pnp_refusal_warning(env: &serde_json::Value, code: &str, ctx: &str) { + let warnings = env + .get("warnings") + .and_then(|w| w.as_array()) + .unwrap_or_else(|| { + panic!("{ctx}: scan on a PnP project must carry a warnings[] refusal (the silent success-0 P0).\nenvelope: {env}") + }); + let w = warnings + .iter() + .find(|w| json_string(w, "code") == Some(code)) + .unwrap_or_else(|| panic!("{ctx}: warnings[] must contain code={code}.\nenvelope: {env}")); + let detail = json_string(w, "detail") + .unwrap_or_else(|| panic!("{ctx}: refusal warning must carry a detail.\nenvelope: {env}")); + assert!( + detail.contains("Plug'n'Play"), + "{ctx}: refusal detail should name the Plug'n'Play layout, got: {detail}" + ); + if code == "yarn_pnp_unsupported" { + assert!( + detail.contains("yarn patch"), + "{ctx}: refusal detail should point at `yarn patch`, got: {detail}" + ); + } +} + +/// One mode's scan leg: `scan --json --yes --mode ` on the PnP fixture +/// must keep its documented envelope (exit 0, status success — deliberately +/// unchanged) but surface the explicit refusal warning, and must not touch a +/// single file (no manifest, no lockfile edit, no cache rewrite). +fn scan_pnp_mode_case(mode: &str) { + let dir = tempfile::tempdir().unwrap(); + make_yarn_berry_scan_fixture(dir.path()); + let before = snapshot_tree(dir.path()); + + let (code, stdout, stderr) = run_with_env( + dir.path(), + &["scan", "--json", "--yes", "--mode", mode], + &[("SOCKET_TELEMETRY_DISABLED", "1")], + ); + assert_eq!( + code, 0, + "mode {mode}: scan exit semantics are deliberately unchanged (exit 0 + warning).\nstdout:\n{stdout}\nstderr:\n{stderr}" + ); + let env = parse_json_envelope(&stdout); + assert_eq!( + json_string(&env, "status"), + Some("success"), + "mode {mode}: status field deliberately unchanged.\nenvelope: {env}" + ); + assert_eq!( + env.get("scannedPackages").and_then(|v| v.as_u64()), + Some(0), + "mode {mode}: PnP packages are undiscoverable, count stays 0.\nenvelope: {env}" + ); + assert_scan_pnp_refusal_warning(&env, "yarn_pnp_unsupported", &format!("mode {mode}")); + if mode == "hosted" { + assert_eq!( + env.get("redirect") + .and_then(|r| r.get("redirected")) + .and_then(|v| v.as_u64()), + Some(0), + "mode {mode}: hosted envelope keeps its (empty) redirect block.\nenvelope: {env}" + ); + } + assert!( + !dir.path().join(".socket").exists(), + "mode {mode}: a refused scan must not create .socket/ state" + ); + assert_eq!( + snapshot_tree(dir.path()), + before, + "mode {mode}: scan on a PnP project must leave every file untouched" + ); +} + +#[test] +fn scan_agent_mode_on_pnp_project_surfaces_refusal_warning() { + scan_pnp_mode_case("agent"); +} + +#[test] +fn scan_hosted_mode_on_pnp_project_surfaces_refusal_warning() { + scan_pnp_mode_case("hosted"); +} + +#[test] +fn scan_vendored_mode_on_pnp_project_surfaces_refusal_warning() { + scan_pnp_mode_case("vendored"); +} + +/// Human (non-JSON) scan on the same fixture: the refusal must reach stderr +/// so an interactive user sees it, exit code unchanged (0). +#[test] +fn scan_human_mode_on_pnp_project_prints_refusal_to_stderr() { + let dir = tempfile::tempdir().unwrap(); + make_yarn_berry_scan_fixture(dir.path()); + + let (code, stdout, stderr) = + run_with_env(dir.path(), &["scan"], &[("SOCKET_TELEMETRY_DISABLED", "1")]); + assert_eq!( + code, 0, + "human scan stays exit 0.\nstdout:\n{stdout}\nstderr:\n{stderr}" + ); + assert!( + stderr.contains("yarn_pnp_unsupported"), + "human scan must print the stable refusal code to stderr, got:\n{stderr}" + ); + assert!( + stderr.contains("Plug'n'Play") && stderr.contains("yarn patch"), + "human scan stderr must name the layout and the workaround, got:\n{stderr}" + ); +} + +/// The pnpm twin: pnpm's own `node-linker=pnp` mode writes the same +/// `.pnp.cjs` loader. Its diagnosis must surface through the same warnings +/// channel with its own code (and a pnpm remedy, never `yarn patch`). +#[test] +fn scan_on_pnpm_pnp_project_surfaces_pnpm_refusal_warning() { + let dir = tempfile::tempdir().unwrap(); + std::fs::write( + dir.path().join("package.json"), + r#"{"name":"pnpm-pnp-fixture","version":"0.0.0","private":true}"#, + ) + .unwrap(); + std::fs::write(dir.path().join(".pnp.cjs"), b"// stub PnP loader\n").unwrap(); + std::fs::write( + dir.path().join("pnpm-lock.yaml"), + "lockfileVersion: '9.0'\n\nsettings:\n autoInstallPeers: true\n", + ) + .unwrap(); + // The installed pnpm store markers that reclassify the loader as pnpm's + // PnP mode (no crawlable package dirs, so discovery stays empty). + std::fs::create_dir_all(dir.path().join("node_modules").join(".pnpm")).unwrap(); + std::fs::write(dir.path().join("node_modules").join(".modules.yaml"), "").unwrap(); + + let (code, stdout, stderr) = run_with_env( + dir.path(), + &["scan", "--json", "--yes"], + &[("SOCKET_TELEMETRY_DISABLED", "1")], + ); + assert_eq!( + code, 0, + "pnpm-PnP scan stays exit 0.\nstdout:\n{stdout}\nstderr:\n{stderr}" + ); + let env = parse_json_envelope(&stdout); + assert_scan_pnp_refusal_warning(&env, "pnpm_pnp_unsupported", "pnpm-pnp"); + let warnings = env.get("warnings").and_then(|w| w.as_array()).unwrap(); + let detail = warnings + .iter() + .find(|w| json_string(w, "code") == Some("pnpm_pnp_unsupported")) + .and_then(|w| json_string(w, "detail")) + .unwrap(); + assert!( + detail.contains("node-linker=pnp"), + "pnpm-pnp refusal must diagnose the pnpm linker, got: {detail}" + ); + assert!( + !detail.contains("yarn patch"), + "pnpm-pnp refusal must not recommend a yarn command in a pnpm repo, got: {detail}" + ); +} + +/// The apply half of the P0: on a PnP checkout WITHOUT a manifest, apply +/// used to exit 0 with the calm `noManifest` status — the documented loud +/// `yarn_pnp_unsupported` refusal (pinned by the tests at the top of this +/// file) sat BELOW the noManifest early-return and was unreachable, because +/// scan never writes a manifest on PnP projects. The refusal must fire +/// first, matching the with-manifest envelope shape exactly. +#[test] +fn apply_without_manifest_on_pnp_project_refuses_loudly() { + let dir = tempfile::tempdir().unwrap(); + make_yarn_berry_project(dir.path()); + // Deliberately NO .socket/ directory: this is what a PnP project looks + // like after any number of scans (scan cannot discover its packages). + + let (code, stdout, stderr) = run(dir.path(), &["apply", "--json"]); + assert_eq!( + code, 1, + "apply on a PnP checkout must refuse loudly even without a manifest.\nstdout:\n{stdout}\nstderr:\n{stderr}" + ); + let env = parse_json_envelope(&stdout); + assert_ne!( + json_string(&env, "status"), + Some("noManifest"), + "the calm noManifest exit must not mask the PnP refusal.\nenvelope: {env}" + ); + assert_eq!( + envelope_error_code(&env), + Some("yarn_pnp_unsupported"), + "expected error.code=yarn_pnp_unsupported.\nenvelope: {env}" + ); + assert_eq!( + json_string(&env, "status"), + Some("error"), + "expected status=error.\nenvelope: {env}" + ); + let error_msg = envelope_error_message(&env) + .unwrap_or_else(|| panic!("error.message missing from envelope: {env}")); + assert!( + error_msg.contains("yarn patch") && error_msg.contains("Plug'n'Play"), + "error message should name `yarn patch` and the Plug'n'Play layout, got: {error_msg}" + ); +} + +/// Human-mode twin of the no-manifest refusal: exit 1 with the stderr +/// pointer, and the old calm "No .socket folder found" message must not be +/// what the user sees instead. +#[test] +fn apply_without_manifest_on_pnp_project_refuses_in_human_mode() { + let dir = tempfile::tempdir().unwrap(); + make_yarn_berry_project(dir.path()); + + let (code, stdout, stderr) = run(dir.path(), &["apply"]); + assert_eq!( + code, 1, + "expected exit 1.\nstdout:\n{stdout}\nstderr:\n{stderr}" + ); + assert!( + !stdout.contains("No .socket folder found"), + "the calm noManifest message must not mask the PnP refusal, got:\n{stdout}" + ); + assert!( + stderr.contains("Plug'n'Play") && stderr.contains("yarn patch"), + "stderr should name the layout and the workaround, got:\n{stderr}" + ); +} + /// Control for the two tests above: an in-scope npm patch in the SAME /// polyglot manifest still refuses. Without this, scoping the detector down /// to nothing at all would leave every positive test in this file passing diff --git a/crates/socket-patch-core/src/vendor/lock_inventory.rs b/crates/socket-patch-core/src/vendor/lock_inventory.rs index 91dc9da0..c03e20d9 100644 --- a/crates/socket-patch-core/src/vendor/lock_inventory.rs +++ b/crates/socket-patch-core/src/vendor/lock_inventory.rs @@ -96,21 +96,51 @@ impl LockfileEntry { } } +/// A project layout whose npm-family packages the inventory structurally +/// CANNOT serve — distinct from "no lockfile" (`Ok(None)`), which is a +/// normal, silent state. Today: the Plug'n'Play loaders (yarn berry's PnP, +/// and pnpm's own `node-linker=pnp` mode). Consumers surface this as an +/// explicit refusal instead of a silent empty inventory: under yarn PnP the +/// installed-tree crawl is ALSO structurally empty (no `node_modules/`), so +/// swallowing this diagnosis used to turn `scan` into a silent +/// success-0 no-op in every mode. +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct UnsupportedNpmLayout { + /// Stable diagnosis code from the flavor probe: + /// `vendor_yarn_berry_unsupported` or `vendor_pnpm_pnp_unsupported`. + pub code: &'static str, + /// Human-readable diagnosis with remedy. + pub detail: String, +} + /// Inventory the project's npm-family lockfile. Routes by -/// [`detect_npm_lock_flavor`] (PnP markers, bun.lockb, unsupported lock -/// versions, and a missing lockfile all yield `None`). +/// [`detect_npm_lock_flavor`]. `Ok(None)` means there is nothing to +/// inventory (missing lockfile, bun.lockb, unsupported lock versions); +/// `Err` propagates the probe's Plug'n'Play diagnosis — a layout whose +/// packages the inventory can NEVER serve, which callers must not conflate +/// with the calm no-lockfile case. pub(crate) async fn inventory_npm_lock( project_root: &Path, -) -> Option<(NpmLockFlavor, Vec)> { +) -> Result)>, UnsupportedNpmLayout> { // Rush monorepos have no root package.json/lock pair; their single // pnpm source-of-truth lives under common/config/rush/. The flavor // probe (root-relative) can't see it, so fall back explicitly when the // root lock is absent but rush.json is present. let (flavor, _warnings) = match detect_npm_lock_flavor(project_root).await { Ok(found) => found, - Err(_) => { + Err((code, detail)) => { + // The PnP loaders are a refusal, not an absence: propagate the + // diagnosis instead of discarding it. Every other probe error + // (missing lock, bun.lockb, unsupported versions) keeps the + // Rush fallback and the calm `Ok(None)`. + if matches!( + code, + "vendor_yarn_berry_unsupported" | "vendor_pnpm_pnp_unsupported" + ) { + return Err(UnsupportedNpmLayout { code, detail }); + } let rush = inventory_rush_pnpm_locks(project_root).await; - return (!rush.is_empty()).then(|| (NpmLockFlavor::Pnpm, finalize_npm(rush))); + return Ok((!rush.is_empty()).then(|| (NpmLockFlavor::Pnpm, finalize_npm(rush)))); } }; let raw = match flavor { @@ -119,8 +149,8 @@ pub(crate) async fn inventory_npm_lock( NpmLockFlavor::YarnClassic => inventory_yarn_classic(project_root).await, NpmLockFlavor::YarnBerry => inventory_yarn_berry(project_root).await, NpmLockFlavor::Bun => inventory_bun(project_root).await, - }?; - Some((flavor, finalize_npm(raw))) + }; + Ok(raw.map(|raw| (flavor, finalize_npm(raw)))) } /// Match a manifest/API purl (possibly percent-encoded, possibly carrying @@ -151,11 +181,26 @@ pub fn lookup<'a>(entries: &'a [LockfileEntry], purl: &str) -> Option<&'a Lockfi } /// Everything every recognized lockfile in the project resolves — the -/// union the scan supplement and the vendor auto-fetch consume. +/// union the scan supplement and the vendor auto-fetch consume. Drops the +/// npm-layout diagnosis; callers that must surface refusals (scan) use +/// [`inventory_project_diagnosed`]. pub async fn inventory_project(project_root: &Path) -> Vec { + inventory_project_diagnosed(project_root).await.0 +} + +/// [`inventory_project`] plus the npm-family layout refusals it hit: a +/// Plug'n'Play project yields no npm entries AND a diagnosis, so consumers +/// can tell "nothing to inventory" from "packages structurally unreachable" +/// and refuse explicitly instead of silently reporting an empty project. +pub async fn inventory_project_diagnosed( + project_root: &Path, +) -> (Vec, Vec) { let mut out: Vec = Vec::new(); - if let Some((_, entries)) = inventory_npm_lock(project_root).await { - out.extend(entries); + let mut unsupported: Vec = Vec::new(); + match inventory_npm_lock(project_root).await { + Ok(Some((_, entries))) => out.extend(entries), + Ok(None) => {} + Err(diag) => unsupported.push(diag), } if let Some(entries) = inventory_cargo_lock(project_root).await { out.extend(entries); @@ -172,7 +217,7 @@ pub async fn inventory_project(project_root: &Path) -> Vec { if let Some(entries) = inventory_pypi_locks(project_root).await { out.extend(entries); } - out + (out, unsupported) } /// Guard + dedup the raw npm entries: unsafe names/versions are dropped @@ -1569,7 +1614,7 @@ mod tests { let tmp = tempfile::tempdir().unwrap(); write(tmp.path(), "package-lock.json", PACKAGE_LOCK).await; - let (flavor, entries) = inventory_npm_lock(tmp.path()).await.unwrap(); + let (flavor, entries) = inventory_npm_lock(tmp.path()).await.unwrap().unwrap(); assert_eq!(flavor, NpmLockFlavor::PackageLock); let lp = entry(&entries, "left-pad"); @@ -1611,7 +1656,7 @@ mod tests { ) .await; - let (_, entries) = inventory_npm_lock(tmp.path()).await.unwrap(); + let (_, entries) = inventory_npm_lock(tmp.path()).await.unwrap().unwrap(); assert!(entries.iter().any(|e| e.name == "only-in-shrinkwrap")); assert!(!entries.iter().any(|e| e.name == "left-pad")); } @@ -1625,7 +1670,7 @@ mod tests { r#"{ "lockfileVersion": 1, "dependencies": { "left-pad": { "version": "1.3.0" } } }"#, ) .await; - assert!(inventory_npm_lock(tmp.path()).await.is_none()); + assert!(inventory_npm_lock(tmp.path()).await.unwrap().is_none()); } // ── pnpm ────────────────────────────────────────────────────────────── @@ -1670,7 +1715,7 @@ snapshots: let tmp = tempfile::tempdir().unwrap(); write(tmp.path(), "pnpm-lock.yaml", PNPM_LOCK).await; - let (flavor, entries) = inventory_npm_lock(tmp.path()).await.unwrap(); + let (flavor, entries) = inventory_npm_lock(tmp.path()).await.unwrap().unwrap(); assert_eq!(flavor, NpmLockFlavor::Pnpm); assert_eq!( @@ -1717,7 +1762,7 @@ packages: ) .await; - let (flavor, entries) = inventory_npm_lock(tmp.path()).await.unwrap(); + let (flavor, entries) = inventory_npm_lock(tmp.path()).await.unwrap().unwrap(); assert_eq!(flavor, NpmLockFlavor::Pnpm); // Union across the common lock and the subspace lock. assert_eq!(entry(&entries, "left-pad").version, "1.3.0"); @@ -1729,7 +1774,7 @@ packages: // rush.json but no common/subspace lock at all: nothing to inventory. let tmp = tempfile::tempdir().unwrap(); write(tmp.path(), "rush.json", r#"{"rushVersion":"5.0.0"}"#).await; - assert!(inventory_npm_lock(tmp.path()).await.is_none()); + assert!(inventory_npm_lock(tmp.path()).await.unwrap().is_none()); } #[tokio::test] @@ -1752,7 +1797,7 @@ packages: ) .await; - let (flavor, entries) = inventory_npm_lock(tmp.path()).await.unwrap(); + let (flavor, entries) = inventory_npm_lock(tmp.path()).await.unwrap().unwrap(); assert_eq!(flavor, NpmLockFlavor::Pnpm); assert!(entries.iter().any(|e| e.name == "left-pad")); assert!( @@ -1792,7 +1837,7 @@ aliased@npm:real-name@^3.0.0: let tmp = tempfile::tempdir().unwrap(); write(tmp.path(), "yarn.lock", YARN_CLASSIC).await; - let (flavor, entries) = inventory_npm_lock(tmp.path()).await.unwrap(); + let (flavor, entries) = inventory_npm_lock(tmp.path()).await.unwrap().unwrap(); assert_eq!(flavor, NpmLockFlavor::YarnClassic); let lp = entry(&entries, "left-pad"); @@ -1850,7 +1895,7 @@ __metadata: let tmp = tempfile::tempdir().unwrap(); write(tmp.path(), "yarn.lock", YARN_BERRY).await; - let (flavor, entries) = inventory_npm_lock(tmp.path()).await.unwrap(); + let (flavor, entries) = inventory_npm_lock(tmp.path()).await.unwrap().unwrap(); assert_eq!(flavor, NpmLockFlavor::YarnBerry); let lp = entry(&entries, "left-pad"); @@ -1885,7 +1930,7 @@ __metadata: let tmp = tempfile::tempdir().unwrap(); write(tmp.path(), "bun.lock", BUN_LOCK).await; - let (flavor, entries) = inventory_npm_lock(tmp.path()).await.unwrap(); + let (flavor, entries) = inventory_npm_lock(tmp.path()).await.unwrap().unwrap(); assert_eq!(flavor, NpmLockFlavor::Bun); assert_eq!( @@ -2292,21 +2337,54 @@ source = { editable = "." } } #[tokio::test] - async fn unsupported_flavors_yield_none() { - // PnP marker wins over any lockfile. + async fn pnp_layouts_propagate_the_diagnosis_instead_of_yielding_none() { + // PnP marker wins over any lockfile — and the diagnosis must + // PROPAGATE, not collapse into the calm no-lockfile `None`. Under + // yarn PnP the installed-tree crawl is also structurally empty, so + // swallowing this here made `scan` a silent success-0 no-op in + // every mode (the P0 this pins). let tmp = tempfile::tempdir().unwrap(); write(tmp.path(), ".pnp.cjs", "/* pnp */").await; write(tmp.path(), "package-lock.json", PACKAGE_LOCK).await; - assert!(inventory_npm_lock(tmp.path()).await.is_none()); + let diag = inventory_npm_lock(tmp.path()).await.unwrap_err(); + assert_eq!(diag.code, "vendor_yarn_berry_unsupported"); + assert!(diag.detail.contains("Plug'n'Play"), "{}", diag.detail); + assert!(diag.detail.contains("yarn patch"), "{}", diag.detail); + + // pnpm's own `node-linker=pnp` twin (same loader, pnpm store): + // same channel, pnpm diagnosis. + let tmp = tempfile::tempdir().unwrap(); + write(tmp.path(), ".pnp.cjs", "/* pnp */").await; + write(tmp.path(), "pnpm-lock.yaml", "lockfileVersion: '9.0'\n").await; + tokio::fs::create_dir_all(tmp.path().join("node_modules/.pnpm")) + .await + .unwrap(); + write(&tmp.path().join("node_modules"), ".modules.yaml", "").await; + let diag = inventory_npm_lock(tmp.path()).await.unwrap_err(); + assert_eq!(diag.code, "vendor_pnpm_pnp_unsupported"); + assert!(diag.detail.contains("node-linker=pnp"), "{}", diag.detail); + + // And the project-level union surfaces the same diagnosis while + // still serving the OTHER ecosystems' lockfiles. + let (entries, unsupported) = inventory_project_diagnosed(tmp.path()).await; + assert!(entries.is_empty(), "{entries:?}"); + assert_eq!(unsupported.len(), 1, "{unsupported:?}"); + assert_eq!(unsupported[0].code, "vendor_pnpm_pnp_unsupported"); + } - // pnpm v6. + #[tokio::test] + async fn unsupported_flavors_yield_none() { + // pnpm v6: no backend, but not a refusal — the calm None. let tmp = tempfile::tempdir().unwrap(); write(tmp.path(), "pnpm-lock.yaml", "lockfileVersion: '6.0'\n").await; - assert!(inventory_npm_lock(tmp.path()).await.is_none()); + assert!(inventory_npm_lock(tmp.path()).await.unwrap().is_none()); // No lockfile at all. let tmp = tempfile::tempdir().unwrap(); - assert!(inventory_npm_lock(tmp.path()).await.is_none()); + assert!(inventory_npm_lock(tmp.path()).await.unwrap().is_none()); + let (entries, unsupported) = inventory_project_diagnosed(tmp.path()).await; + assert!(entries.is_empty()); + assert!(unsupported.is_empty(), "{unsupported:?}"); } } diff --git a/crates/socket-patch-core/src/vendor/yarn_layering_tests.rs b/crates/socket-patch-core/src/vendor/yarn_layering_tests.rs index fb31cfe1..d9f580ad 100644 --- a/crates/socket-patch-core/src/vendor/yarn_layering_tests.rs +++ b/crates/socket-patch-core/src/vendor/yarn_layering_tests.rs @@ -327,6 +327,7 @@ async fn classic_vendor_rewrites_only_target_block_and_stays_parseable() { // from the registry inventory by design). let (flavor, entries) = inventory_npm_lock(fx.root()) .await + .expect("no layout refusal") .expect("rewritten lock must still be inventoriable"); assert_eq!(flavor, NpmLockFlavor::YarnClassic); let mut got: Vec<(String, String)> = entries @@ -817,7 +818,10 @@ async fn berry_vendor_leaves_builtin_patch_entries_byte_identical() { // The crate's own berry parser still reads the file; the untouched npm: // entries survive, patch:/file:/workspace resolutions are skipped by // design. - let (flavor, entries) = inventory_npm_lock(fx.root()).await.expect("parseable"); + let (flavor, entries) = inventory_npm_lock(fx.root()) + .await + .expect("no layout refusal") + .expect("parseable"); assert_eq!(flavor, NpmLockFlavor::YarnBerry); let mut got: Vec<(String, String)> = entries .iter()