feat(apply): safety hardening — atomicity, locking, pnpm CoW, sidecars, Maven gate

.github/workflows/ci.yml

@@ -411,6 +411,30 @@ jobs:
             suite: e2e_scan
           - os: macos-latest
             suite: e2e_scan
+          # Safety-hardening e2e suites. The fast non-ignored ones
+          # (e2e_safety_lock, e2e_safety_yarn_pnp) run via the
+          # standard `test` job above on all three platforms, so no
+          # matrix entry is needed for them. The two below need real
+          # toolchains and are #[ignore]-gated.
+          - os: ubuntu-latest
+            suite: e2e_safety_cargo_build
+          - os: macos-latest
+            suite: e2e_safety_cargo_build
+          - os: windows-latest
+            suite: e2e_safety_cargo_build
+          - os: ubuntu-latest
+            suite: e2e_safety_pnpm
+          - os: macos-latest
+            suite: e2e_safety_pnpm
+          # pnpm-on-Windows uses junctions for symlinks and copies
+          # (not hardlinks) by default, so the CoW invariant holds
+          # vacuously. Test still runs to verify apply doesn't error
+          # on Windows — semantic Windows nlink coverage is a
+          # follow-up (`std::fs::Metadata` doesn't expose nlink on
+          # Windows; needs `GetFileInformationByHandle` via
+          # `windows-sys`).
+          - os: windows-latest
+            suite: e2e_safety_pnpm
     runs-on: ${{ matrix.os }}
     steps:
       - name: Checkout
@@ -436,11 +460,20 @@ jobs:
           restore-keys: ${{ matrix.os }}-cargo-e2e-
 
       - name: Setup Node.js
-        if: matrix.suite == 'e2e_npm' || matrix.suite == 'e2e_scan'
+        if: matrix.suite == 'e2e_npm' || matrix.suite == 'e2e_scan' || matrix.suite == 'e2e_safety_pnpm'
         uses: actions/setup-node@49933ea5288caeca8642d1e84afbd3f7d6820020 # v4
         with:
           node-version: '20.20.2'
 
+      - name: Setup pnpm
+        if: matrix.suite == 'e2e_safety_pnpm'
+        # Pin the major version so the store layout the test
+        # asserts on stays stable. `npm install -g` is the simplest
+        # cross-platform install path (works on ubuntu, macos,
+        # windows-runners — they all ship a usable npm via
+        # actions/setup-node).
+        run: npm install -g pnpm@10
+
       - name: Setup Python
         if: matrix.suite == 'e2e_pypi'
         uses: actions/setup-python@a26af69be951a213d495a4c3e4e4022e16d87065 # v5

Cargo.toml

@@ -28,6 +28,7 @@ once_cell = "=1.21.3"
 qbsdiff = "=1.4.4"
 tar = "=0.4.45"
 flate2 = "=1.1.9"
+fs2 = "=0.4.3"
 wiremock = "=0.6.5"
 portable-pty = "=0.9.0"
 testcontainers = "=0.27.3"

crates/socket-patch-cli/Cargo.toml

@@ -49,3 +49,8 @@ base64 = { workspace = true }
 reqwest = { workspace = true }
 tempfile = { workspace = true }
 serial_test = { workspace = true }
+# Used by `tests/e2e_safety_lock.rs` to externally hold the same
+# `.socket/apply.lock` the binary takes, then spawn the binary and
+# assert the lock_held exit-code contract. Same crate the binary
+# uses internally (`socket-patch-core::patch::apply_lock`).
+fs2 = { workspace = true }

crates/socket-patch-cli/src/commands/apply.rs

@@ -4,11 +4,15 @@ use socket_patch_core::api::blob_fetcher::{
     get_missing_blobs, DownloadMode,
 };
 use socket_patch_core::api::client::get_api_client_with_overrides;
-use socket_patch_core::crawlers::{CrawlerOptions, Ecosystem};
+use socket_patch_core::crawlers::{
+    detect_npm_pkg_manager, CrawlerOptions, Ecosystem, NpmPkgManager,
+};
 use socket_patch_core::manifest::operations::read_manifest;
 use socket_patch_core::patch::apply::{
     apply_package_patch, verify_file_patch, ApplyResult, PatchSources, VerifyStatus,
 };
+
+use crate::commands::lock_cli::acquire_or_emit;
 use socket_patch_core::utils::purl::strip_purl_qualifiers;
 use socket_patch_core::utils::telemetry::{track_patch_applied, track_patch_apply_failed};
 use std::collections::{HashMap, HashSet};
@@ -129,6 +133,11 @@ pub(crate) fn result_to_event(result: &ApplyResult, dry_run: bool) -> PatchEvent
                 .map(AppliedVia::from_core),
         })
         .collect();
+    // Sidecar data is NOT attached here — it's surfaced at the
+    // envelope level under `Envelope.sidecars[]` by the run loop.
+    // See `Envelope::record_sidecar`. Keeping events clean of
+    // sidecar info means each event describes only the apply
+    // action; sidecar reporting is a separate, JOIN-able list.
     PatchEvent::new(PatchAction::Applied, purl).with_files(files)
 }
 
@@ -154,6 +163,60 @@ pub async fn run(args: ApplyArgs) -> i32 {
         return 0;
     }
 
+    // Serialize against concurrent socket-patch runs targeting the same
+    // `.socket/` directory. The guard releases on function return; see
+    // `socket_patch_core::patch::apply_lock`.
+    let socket_dir = manifest_path.parent().unwrap_or(Path::new("."));
+    let _lock = match acquire_or_emit(
+        socket_dir,
+        Command::Apply,
+        args.common.json,
+        args.common.silent,
+        args.common.dry_run,
+    ) {
+        Ok(guard) => guard,
+        Err(code) => return code,
+    };
+
+    // Package-manager layout detection. yarn-berry PnP keeps packages
+    // inside `.yarn/cache/*.zip` and resolves them via `.pnp.cjs` —
+    // the npm crawler can't reach them and rewriting zips is a
+    // different operation entirely. Refuse with a clear pointer to
+    // `yarn patch`. pnpm gets an informational event; the CoW guard
+    // in `apply_file_patch` does the substantive safety work.
+    let pkg_manager = detect_npm_pkg_manager(&args.common.cwd);
+    match pkg_manager {
+        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 <pkg>` instead.",
+                ));
+                println!("{}", env.to_pretty_json());
+            } else if !args.common.silent {
+                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 <pkg>` instead.");
+            }
+            return 1;
+        }
+        NpmPkgManager::Pnpm => {
+            if !args.common.json && !args.common.silent {
+                eprintln!(
+                    "Note: pnpm layout detected. Copy-on-write will keep the global store untouched."
+                );
+            }
+            // Non-fatal — CoW handles the safety. JSON consumers see
+            // the layout-detected info in the apply envelope's
+            // existing events (no separate event added here yet).
+        }
+        _ => {}
+    }
+
     match apply_patches_inner(&args, &manifest_path).await {
         Ok((success, results, unmatched)) => {
             let patched_count = results
@@ -166,6 +229,13 @@ pub async fn run(args: ApplyArgs) -> i32 {
                 env.dry_run = args.common.dry_run;
                 for result in &results {
                     env.record(result_to_event(result, args.common.dry_run));
+                    // Sidecar records live on the envelope, not on
+                    // individual events. Consumers iterate
+                    // `envelope.sidecars[]` and JOIN against
+                    // `events[]` by `purl` for per-package context.
+                    if let Some(ref sidecar) = result.sidecar {
+                        env.record_sidecar(sidecar.clone());
+                    }
                 }
                 // Manifest entries that targeted in-scope ecosystems but
                 // had no installed package on disk — emit one Skipped
@@ -705,6 +775,7 @@ mod tests {
             files_patched: vec!["package/index.js".to_string()],
             applied_via,
             error: None,
+            sidecar: None,
         }
     }
 
@@ -779,6 +850,7 @@ mod tests {
             ],
             applied_via,
             error: None,
+            sidecar: None,
         };
 
         let event = result_to_event(&result, false);

crates/socket-patch-cli/src/commands/lock_cli.rs

@@ -0,0 +1,117 @@
+//! Envelope-aware wrapper around the
+//! `socket_patch_core::patch::apply_lock` advisory lock.
+//!
+//! Mutating subcommands (`apply`, `rollback`, `repair`, `remove`) all
+//! need the same shape: acquire the lock at the top of `run`, on
+//! contention emit a JSON envelope with `errorCode: "lock_held"` (or
+//! stderr in human mode) and exit 1. This module centralises that
+//! emission so the four call sites stay one line each.
+//!
+//! The lock itself is in `socket-patch-core` (cross-crate, also used
+//! by tests). This module is the CLI-side glue that knows how to
+//! render the failure through the shared [`crate::json_envelope`].
+
+use std::path::Path;
+use std::time::Duration;
+
+use socket_patch_core::patch::apply_lock::{acquire, LockError, LockGuard};
+
+use crate::json_envelope::{Command, Envelope, EnvelopeError};
+
+/// Try to acquire `<socket_dir>/apply.lock` and return the guard, or
+/// emit a failure envelope and a non-zero exit code.
+///
+/// `command` selects the envelope's `command` field so downstream
+/// consumers see `apply` / `rollback` / `repair` / `remove` rather
+/// than a generic "lock failed". `dry_run` is plumbed through to the
+/// envelope's `dry_run` field for the (rare) case where lock
+/// contention happens during a dry-run apply.
+pub fn acquire_or_emit(
+    socket_dir: &Path,
+    command: Command,
+    json: bool,
+    silent: bool,
+    dry_run: bool,
+) -> Result<LockGuard, i32> {
+    match acquire(socket_dir, Duration::ZERO) {
+        Ok(guard) => Ok(guard),
+        Err(LockError::Held) => {
+            emit(
+                command,
+                json,
+                silent,
+                dry_run,
+                "lock_held",
+                "another socket-patch process is operating in this directory",
+                Some(socket_dir),
+            );
+            Err(1)
+        }
+        Err(LockError::Io { path, source }) => {
+            let msg = format!("failed to open lock file at {}: {}", path.display(), source);
+            emit(command, json, silent, dry_run, "lock_io", &msg, None);
+            Err(1)
+        }
+    }
+}
+
+fn emit(
+    command: Command,
+    json: bool,
+    silent: bool,
+    dry_run: bool,
+    code: &str,
+    message: &str,
+    hint_dir: Option<&Path>,
+) {
+    if json {
+        let mut env = Envelope::new(command);
+        env.dry_run = dry_run;
+        env.mark_error(EnvelopeError::new(code, message));
+        println!("{}", env.to_pretty_json());
+    } else if !silent {
+        eprintln!("Error: {message}.");
+        if let Some(dir) = hint_dir {
+            eprintln!(
+                "  If you are sure no other process is running, remove {}/apply.lock and retry.",
+                dir.display()
+            );
+        }
+    }
+}
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    #[test]
+    fn acquire_or_emit_succeeds_on_fresh_dir() {
+        let dir = tempfile::tempdir().unwrap();
+        let guard = acquire_or_emit(dir.path(), Command::Apply, false, true, false).unwrap();
+        drop(guard);
+    }
+
+    #[test]
+    fn acquire_or_emit_returns_one_on_contention() {
+        let dir = tempfile::tempdir().unwrap();
+        let _first =
+            acquire_or_emit(dir.path(), Command::Apply, false, true, false).unwrap();
+        let code =
+            acquire_or_emit(dir.path(), Command::Apply, false, true, false).unwrap_err();
+        assert_eq!(code, 1);
+    }
+
+    #[test]
+    fn acquire_or_emit_returns_one_when_socket_dir_missing() {
+        let dir = tempfile::tempdir().unwrap();
+        let code = acquire_or_emit(
+            &dir.path().join("nope"),
+            Command::Apply,
+            false,
+            true,
+            false,
+        )
+        .unwrap_err();
+        assert_eq!(code, 1);
+    }
+}

crates/socket-patch-cli/src/commands/mod.rs

@@ -1,6 +1,7 @@
 pub mod apply;
 pub mod get;
 pub mod list;
+pub mod lock_cli;
 pub mod remove;
 pub mod repair;
 pub mod rollback;

crates/socket-patch-cli/src/commands/remove.rs

@@ -8,6 +8,7 @@ use std::path::Path;
 
 use super::rollback::rollback_patches;
 use crate::args::{apply_env_toggles, GlobalArgs};
+use crate::commands::lock_cli::acquire_or_emit;
 use crate::json_envelope::{
     Command, Envelope, EnvelopeError, PatchAction, PatchEvent, Status,
 };
@@ -56,6 +57,23 @@ pub async fn run(args: RemoveArgs) -> i32 {
         return 1;
     }
 
+    // Serialize against concurrent socket-patch runs targeting the
+    // same `.socket/` directory. Note: `rollback_patches` (which
+    // `remove` calls into) does NOT acquire the lock — that would
+    // self-deadlock — so the outer remove invocation holds it for
+    // both the rollback and the manifest mutation.
+    let socket_dir = manifest_path.parent().unwrap_or(Path::new("."));
+    let _lock = match acquire_or_emit(
+        socket_dir,
+        Command::Remove,
+        args.common.json,
+        false, // remove has no --silent on its own; use false
+        false, // remove has no --dry-run
+    ) {
+        Ok(guard) => guard,
+        Err(code) => return code,
+    };
+
     // Read manifest to show what will be removed and confirm
     let manifest = match read_manifest(&manifest_path).await {
         Ok(Some(m)) => m,