Fix yarn berry exec:/git copies escaping VEX (#1630)

crates/socket-patch-cli/tests/hosted_memory_engine.rs

@@ -2085,3 +2085,70 @@ async fn issue_1598_berry_invalid_manifest_name_or_version_is_named_on_disk() {
         );
     }
 }
+
+/// #1630: a berry `exec:` script, or a git remote that isn't http(s),
+/// under another dependency name installs `node_modules/lp2` from a
+/// package the lock doesn't name, at its lock version. It may be the
+/// pinned left-pad@1.3.0, so the hosted scan names it as possibly
+/// unpatched (`redirect_copy_unpatched`); it used to report a clean
+/// success while yarn installed unpatched left-pad as `node_modules/lp2`.
+/// Control: the same copy at another version is no copy of 1.3.0.
+#[tokio::test]
+async fn issue_1630_berry_exec_and_git_copies_are_named() {
+    const UUID: &str = "16301630-1630-4630-8630-163016301630";
+    let ws = "app%40workspace%3A.";
+    for (spec, key, resolution) in [
+        (
+            "exec:./gen.js",
+            format!("lp2@exec:./gen.js::locator={ws}"),
+            format!("lp2@exec:./gen.js#./gen.js::hash=db115a&locator={ws}"),
+        ),
+        (
+            "git+ssh://git@github.com/stevemao/left-pad.git#v1.3.0",
+            "lp2@git+ssh://git@github.com/stevemao/left-pad.git#v1.3.0".to_string(),
+            "lp2@git+ssh://git@github.com/stevemao/left-pad.git#commit=5d3c4f2".to_string(),
+        ),
+    ] {
+        for (version, named) in [("1.3.0", true), ("2.0.0", false)] {
+            let server = MockServer::start().await;
+            mount_left_pad_grant(&server, UUID).await;
+            let mut files = berry_linked_copy_files(spec);
+            let lock = String::from_utf8(files["yarn.lock"].clone()).unwrap();
+            let linked = format!(
+                "\"lp2@{spec}::locator={ws}\":\n  version: 0.0.0-use.local\n  \
+                 resolution: \"lp2@{spec}::locator={ws}\"\n  \
+                 languageName: node\n  linkType: soft\n"
+            );
+            assert!(lock.contains(&linked), "{lock}");
+            let lock = lock.replace(
+                &linked,
+                &format!(
+                    "\"{key}\":\n  version: {version}\n  resolution: \"{resolution}\"\n  \
+                     languageName: node\n  linkType: hard\n"
+                ),
+            );
+            files.insert("yarn.lock".to_string(), lock.into_bytes());
+            let output = run_engine(&server, build_input(&files, &[], options(false))).await;
+            let project = &output.projects[0];
+            assert!(project.error.is_none(), "{spec}: {:?}", project.error);
+            assert_eq!(
+                project.redirected.len(),
+                1,
+                "{spec}: {:?} {:#}",
+                project.skipped,
+                project.redirect
+            );
+            let details = copy_unpatched_details(project);
+            if !named {
+                assert!(details.is_empty(), "{spec}@{version}: {details:?}");
+                continue;
+            }
+            assert_eq!(details.len(), 1, "{spec}@{version}: {details:?}");
+            assert!(
+                details[0].contains(&key) && details[0].contains("UNPATCHED"),
+                "{spec}: {}",
+                details[0]
+            );
+        }
+    }
+}

crates/socket-patch-core/src/formats/yarn/copy.rs

@@ -2,8 +2,8 @@
 //! dependency name the entry is keyed by (#939, #1236).
 //!
 //! Yarn (classic and berry alike) keys a `file:` tarball or directory, a
-//! url tarball and a berry `portal:` / `link:` dependency by the name the
-//! depender gave it: `"lp2": "file:./left-pad-1.3.0.tgz"` locks as
+//! url tarball and a berry `portal:` / `link:` / `exec:` / git dependency
+//! by the name the depender gave it: `"lp2": "file:./left-pad-1.3.0.tgz"` locks as
 //! `lp2@file:…`, while `node_modules/lp2` IS left-pad@1.3.0. No pin of
 //! `left-pad` reaches that copy, so every reader that asks "does this lock
 //! also install an unpatched copy of the patched `name@version`?" (lockfile
@@ -30,6 +30,15 @@ pub(crate) enum CopyRef {
     /// A url tarball (`#` fragment dropped): the package is the registry
     /// package its `/<name>/-/<leaf>-<version>.tgz` path names.
     Url(String),
+    /// A root-relative berry `exec:` script that builds the package at
+    /// install time: the lock names only its version, so the package is
+    /// never known (#1630).
+    Exec(String),
+    /// A berry git remote that isn't http(s) (`git+ssh:`, `ssh:`, `git:`;
+    /// `#` fragment dropped): the lock names only its version, so the
+    /// package is never known (#1630). An http(s) git remote is a
+    /// [`CopyRef::Url`], whose path names no registry package either.
+    Git(String),
 }
 
 /// Which berry protocol a [`CopyRef::Linked`] copy came from. yarn installs
@@ -44,7 +53,8 @@ pub(crate) enum LinkKind {
 impl CopyRef {
     /// The root-relative file to read for [`CopyRef::identity`]: the tarball
     /// itself, or the directory's `package.json`. `None` for a url, which
-    /// names its package in its path.
+    /// names its package in its path, and for an `exec:` script or a git
+    /// remote, which hold no package until yarn builds or clones them.
     ///
     /// `None` too for an absolute path: a lock is untrusted input, and an
     /// absolute spelling can name any file on the machine, or (a Windows
@@ -63,14 +73,14 @@ impl CopyRef {
             } else {
                 format!("{dir}/package.json")
             }),
-            CopyRef::Url(_) => None,
+            CopyRef::Url(_) | CopyRef::Exec(_) | CopyRef::Git(_) => None,
         }
     }
 
     /// The `(name, version)` this copy installs, given the lock entry's
     /// `version` and the bytes read at [`CopyRef::read_path`]. `None` when
     /// the copy does not say (an unreadable or nameless manifest, a
-    /// non-registry url).
+    /// non-registry url, an `exec:` script, a git remote).
     pub(crate) fn identity(
         &self,
         lock_version: &str,
@@ -84,6 +94,7 @@ impl CopyRef {
                 return Some((package_name(&manifest)?, package_version(&manifest)?));
             }
             CopyRef::Url(url) => registry_tarball_name(url, lock_version)?,
+            CopyRef::Exec(_) | CopyRef::Git(_) => return None,
         };
         Some((name, lock_version.to_string()))
     }
@@ -116,6 +127,11 @@ impl CopyRef {
                 "installs it from {:?}",
                 crate::utils::redact::redact_url(url)
             ),
+            CopyRef::Exec(rel) => format!("builds it with the user's exec: script {rel}"),
+            CopyRef::Git(url) => format!(
+                "installs it from git {:?}",
+                crate::utils::redact::redact_url(url)
+            ),
         }
     }
 }
@@ -150,22 +166,34 @@ pub(crate) fn classic_copy_ref(patterns: &[String], resolved: Option<&str>) -> O
 }
 
 /// The copy a berry locator `reference` installs from: a `file:` tarball or
-/// directory, a `portal:` / `link:` directory (each relative to the
-/// workspace its `locator=` binding names), or an http(s) url. `None` for
-/// any other protocol (`npm:`, `workspace:`, `patch:`, `exec:`, git). A
-/// path that leaves the root is still read there ([`copy_path`]).
+/// directory, a `portal:` / `link:` directory, an `exec:` script (each
+/// relative to the workspace its `locator=` binding names), an http(s)
+/// url, or another git remote. `None` for any other protocol (`npm:`,
+/// `workspace:`, `patch:`). A path that leaves the root is still read
+/// there ([`copy_path`]).
 pub(crate) fn berry_copy_ref(reference: &str) -> Option<CopyRef> {
+    let url = || reference.split('#').next().unwrap_or_default().to_string();
     if reference.starts_with("http") {
-        return Some(CopyRef::Url(
-            reference.split('#').next().unwrap_or_default().to_string(),
-        ));
+        return Some(CopyRef::Url(url()));
     }
-    let (path, linked) = if let Some(path) = reference.strip_prefix("file:") {
+    // yarn's git resolver takes these besides http(s) (#1630); `git@host:`
+    // is the scp-like ssh spelling.
+    if ["git:", "git+", "ssh:", "git@"]
+        .iter()
+        .any(|p| reference.starts_with(p))
+    {
+        return Some(CopyRef::Git(url()));
+    }
+    // `None` for `file:`, which is a tarball or a directory by its path.
+    type Kind = Option<fn(String) -> CopyRef>;
+    let (path, kind): (_, Kind) = if let Some(path) = reference.strip_prefix("file:") {
         (path, None)
     } else if let Some(path) = reference.strip_prefix("portal:") {
-        (path, Some(LinkKind::Portal))
+        (path, Some(|rel| CopyRef::Linked(rel, LinkKind::Portal)))
     } else if let Some(path) = reference.strip_prefix("link:") {
-        (path, Some(LinkKind::Link))
+        (path, Some(|rel| CopyRef::Linked(rel, LinkKind::Link)))
+    } else if let Some(path) = reference.strip_prefix("exec:") {
+        (path, Some(CopyRef::Exec))
     } else {
         return None;
     };
@@ -174,17 +202,17 @@ pub(crate) fn berry_copy_ref(reference: &str) -> Option<CopyRef> {
     let path = path.split("::").next().unwrap_or_default();
     let path = path.split('#').next().unwrap_or_default();
     let rel = berry_copy_path(reference, path)?;
-    Some(if let Some(kind) = linked {
-        CopyRef::Linked(rel, kind)
+    Some(if let Some(kind) = kind {
+        kind(rel)
     } else if is_tarball_path(path) {
         CopyRef::Tarball(rel)
     } else {
         CopyRef::Directory(rel)
     })
 }
 
-/// The root-relative path a berry `file:` / `portal:` / `link:` reference
-/// names: relative to the workspace in its `locator=` binding
+/// The root-relative path a berry `file:` / `portal:` / `link:` / `exec:`
+/// reference names: relative to the workspace in its `locator=` binding
 /// (`b@workspace:packages/b`), or to the root when it has none
 /// ([`copy_path`]).
 fn berry_copy_path(reference: &str, path: &str) -> Option<String> {
@@ -367,6 +395,43 @@ mod tests {
         assert_eq!(berry_copy_ref("workspace:packages/b"), None);
     }
 
+    /// #1630: an `exec:` generator, or a git remote that isn't http(s),
+    /// installs a copy under the dependency name whose package the lock
+    /// doesn't say. It is a copy all the same, never read, so it may be
+    /// any package at its lock version.
+    #[test]
+    fn issue_1630_exec_and_git_refs_are_unread_copies() {
+        let ws = "::locator=b%40workspace%3Apackages%2Fb";
+        let exec = berry_copy_ref(&format!("exec:./gen.js#./gen.js::hash=db115a&{}", &ws[2..]));
+        assert_eq!(exec, Some(CopyRef::Exec("packages/b/gen.js".into())));
+        for git in [
+            "git+ssh://git@github.com/stevemao/left-pad.git#commit=5d3c4f2",
+            "ssh://git@github.com/stevemao/left-pad.git#commit=5d3c4f2",
+            "git://github.com/stevemao/left-pad.git#commit=5d3c4f2",
+            "git@github.com:stevemao/left-pad.git#commit=5d3c4f2",
+        ] {
+            let source = berry_copy_ref(git);
+            assert_eq!(
+                source,
+                Some(CopyRef::Git(git.split('#').next().unwrap().into())),
+                "{git}"
+            );
+        }
+        for source in [exec.unwrap(), CopyRef::Git("ssh://git@h/x.git".into())] {
+            assert_eq!(source.read_path(), None, "{source:?}");
+            assert_eq!(source.identity("1.3.0", None), None, "{source:?}");
+            assert_eq!(source.linked_name(None), None, "{source:?}");
+        }
+        assert_eq!(
+            CopyRef::Git("git+ssh://tok:secret@h/x.git".into()).how(),
+            "installs it from git \"git+ssh://<redacted>@h/x.git\""
+        );
+        assert_eq!(
+            CopyRef::Exec("gen.js".into()).how(),
+            "builds it with the user's exec: script gen.js"
+        );
+    }
+
     /// #1540: an absolute `portal:` / `link:` path keeps its whole spelling,
     /// a Windows drive's `:` included, and is never read.
     #[test]

crates/socket-patch-core/src/vendor/yarn_berry_lock.rs

@@ -237,10 +237,11 @@ impl NpmLockBackend for YarnBerryBackend {
                 ),
             ));
         }
-        // A `file:` / url / `portal:` / `link:` copy of the package under
-        // ANOTHER dependency name (`lp2@file:…`, #939): the name-keyed
-        // resolutions entry never moves it, so it keeps installing the
-        // unpatched bytes. Named, like the alias entries above.
+        // A `file:` / url / `portal:` / `link:` / `exec:` / git copy of the
+        // package under ANOTHER dependency name (`lp2@file:…`, #939,
+        // #1630): the name-keyed resolutions entry never moves it, so it
+        // keeps installing the unpatched bytes. Named, like the alias
+        // entries above.
         let other_name =
             super::yarn_copies::other_name_copies(project_root, &lock_text, name, version).await;
         // The package IS installed, only under other names, and `yarn
@@ -2431,7 +2432,8 @@ __metadata:
     /// (`lp2@…`), so a copy of left-pad@1.3.0 under another name escapes the
     /// name-keyed resolutions entry and installs unpatched. Vendoring still
     /// wires the registry entry, names that copy (`vendor_copy_unpatched`),
-    /// and leaves it byte-untouched. Control: a `file:` / `portal:` copy
+    /// and leaves it byte-untouched. So does an `exec:` script or a
+    /// non-http git remote (#1630). Control: a `file:` / `portal:` copy
     /// holding another package is not named.
     #[tokio::test]
     async fn issue_939_other_name_copies_are_named() {
@@ -2470,10 +2472,27 @@ __metadata:
                 "0.0.0-use.local",
                 format!("lp2@link:./forks/left-pad::locator={ws}"),
             ),
+            // #1630: the lock doesn't say which package these build or
+            // clone, so they are named whatever is on disk.
+            (
+                "exec: script",
+                format!("lp2@exec:./gen.js::locator={ws}"),
+                "1.3.0",
+                format!("lp2@exec:./gen.js#./gen.js::hash=db115a&locator={ws}"),
+            ),
+            (
+                "git+ssh: remote",
+                "lp2@git+ssh://git@github.com/stevemao/left-pad.git#v1.3.0".to_string(),
+                "1.3.0",
+                "lp2@git+ssh://git@github.com/stevemao/left-pad.git#commit=5d3c4f2".to_string(),
+            ),
         ];
         for (case, key, version, resolution) in &cases {
             for fork in ["left-pad", "other-pkg"] {
-                let named = fork == "left-pad" || case.contains("url");
+                let named = fork == "left-pad"
+                    || case.contains("url")
+                    || case.contains("exec")
+                    || case.contains("git");
                 let block = format!(
                     "\n\"{key}\":\n  version: {version}\n  resolution: \"{resolution}\"\n  \
                      languageName: node\n  linkType: hard\n"

crates/socket-patch-core/src/vendor/yarn_copies.rs

@@ -3,7 +3,8 @@
 //! #1236), shared by the classic and berry backends.
 //!
 //! Yarn keys a `file:` tarball or directory, a url tarball and a berry
-//! `portal:` / `link:` dependency by the name the depender gave it
+//! `portal:` / `link:` / `exec:` / git dependency by the name the depender
+//! gave it
 //! (`lp2@file:./left-pad-1.3.0.tgz`), so a re-lock or `resolutions` entry
 //! for `left-pad` never moves it, and `node_modules/lp2` keeps the
 //! unpatched left-pad bytes. Which package such a copy is comes from the
@@ -31,8 +32,9 @@ pub(crate) struct OtherNameCopy {
     pub(crate) source: CopyRef,
     /// The copy itself says it is `name@version`. `false` for a copy of
     /// that version whose package cannot be read (a missing directory, a
-    /// tarball with no readable `package.json`, a non-registry url, or an
-    /// unread `portal:` / `link:` directory of any version): it may be
+    /// tarball with no readable `package.json`, a non-registry url, an
+    /// `exec:` script, a git remote, or an unread `portal:` / `link:`
+    /// directory of any version): it may be
     /// `name`, so it is named, but never taken as proof that `name` is
     /// installed (#1516, #1540).
     pub(crate) identified: bool,
@@ -189,5 +191,7 @@ pub(crate) fn copy_origin(source: &CopyRef) -> &'static str {
         CopyRef::Directory(_) => "a file: directory, which yarn copies into node_modules",
         CopyRef::Linked(..) => "a portal:/link: directory, which yarn links into node_modules",
         CopyRef::Url(_) => "a URL tarball",
+        CopyRef::Exec(_) => "an exec: script, which yarn runs to build it",
+        CopyRef::Git(_) => "a git repository",
     }
 }

crates/socket-patch-core/src/vex/discover/mod.rs

@@ -978,7 +978,8 @@ impl Discovery {
 
     /// A lock entry of `file` that installs a copy of some package at
     /// `version` whose package can't be read (a non-registry url, a missing
-    /// directory, a tarball with no readable `package.json`). It may be any
+    /// directory, a tarball with no readable `package.json`, a berry
+    /// `exec:` script or git remote). It may be any
     /// package at that version, so [`Discovery::contest_within_locks`]
     /// takes it as an unpatched copy of every package `file` wires at
     /// `version` (at any version when `version` is `None`, #1540; of