fix(pnpm): preserve underscores in package names when stripping peer suffix

Fixes SMO-632. `stripPnpmPeerSuffix` scanned the whole dep path for the
first `(` or `_`, which truncated `http_ece@1.0.0` to `http` and caused
`socket pnpm install` to produce a blocking false-positive malware alert
(`http@0.0.1-security: Known malware (critical; blocked)`) on any project
that (transitively) depends on `http_ece` — e.g. `web-push@3.6.7`.

Customer Rio Transfer was blocked by this; reproducible with:

    mkdir /tmp/repro && cd /tmp/repro
    echo '{"name":"t","private":true,"dependencies":{"web-push":"3.6.7"}}' > package.json
    pnpm install
    socket pnpm install --frozen-lockfile

## Fix

`(` and `_` peer separators only appear *after* the version-separating
`@` — semver forbids both characters inside a version, prerelease tag,
or build metadata. Narrow the search to the version-and-beyond portion
of the dep path so package-name underscores are preserved.

Walk-through for the regression case:
  `http_ece@1.0.0_web-push@3.6.7`
  * scopeOffset=0 (no leading `@`)
  * versionAt = indexOf('@', 0) = 8
  * tail = `@1.0.0_web-push@3.6.7`, underscore at tail[6]
  * slice at 8+6 = 14 → `http_ece@1.0.0` ✓

And the scoped variant:
  `@foo/bar_baz@1.0.0_peer@2.0.0`
  * scopeOffset=1 (skip leading `@`)
  * versionAt = indexOf('@', 1) = 12
  * tail = `@1.0.0_peer@2.0.0`, underscore at tail[6]
  * slice at 12+6 = 18 → `@foo/bar_baz@1.0.0` ✓

## Tests

Added two `it` blocks with six assertions:
  * `preserves underscores inside package names (SMO-632 regression)` —
    `http_ece` with no suffix / underscore peer / parenthesized peer.
  * `handles scoped packages with underscore names` — `@foo/bar_baz`
    variants, proving the scope leader `@` doesn't confuse the
    version-separator search.

Also rewrote the "prefers parentheses over underscore" case as
"prefers the earlier separator when both appear" — the new algorithm
picks the earlier separator rather than categorically preferring one,
and the only real-world shape where both appear is `(peer)_legacy`
where `(` is earlier, so the observable outcome is unchanged.

Author: jdalton

packages/cli/src/utils/pnpm/lockfile.mts

@@ -98,28 +98,51 @@ export function stripLeadingPnpmDepPathSlash(depPath: string): string {
 }
 
 /**
- * Strip pnpm peer dependency suffix from dep path.
+ * Strip pnpm peer dependency suffix from a dep path.
  *
- * PNPM appends peer dependency info in format:
- * - `package@1.0.0(peer@2.0.0)` - parentheses for peer deps.
- * - `package@1.0.0_peer@2.0.0` - underscore for peer deps.
+ * pnpm appends peer dependency info in one of two formats:
+ * - `package@1.0.0(peer@2.0.0)` — parentheses (pnpm v7+).
+ * - `package@1.0.0_peer@2.0.0`  — underscore (pnpm v6 / older lockfiles).
  *
- * Edge cases to consider:
- * - Package names with `(` or `_` in legitimate names (rare but valid).
- * - Scoped packages: `@scope/package` should work correctly.
- * - Empty or malformed dep paths should return unchanged.
+ * Any `(` or `_` that signals a peer suffix lives *after* the
+ * version-separating `@`. Package names themselves may legitimately
+ * contain underscores (e.g. `http_ece`) but semver forbids `_` / `(`
+ * in a version string, so the first separator we see after the
+ * version-`@` is always the peer suffix.
  *
- * Current implementation: Find first `(` or `_` and strip everything after.
- * Limitation: May incorrectly strip if package name contains these chars.
- * Mitigation: Most package names don't contain `(` or `_`, pnpm format is predictable.
+ * This previously scanned the whole string for `_` / `(`, which
+ * truncated `http_ece@1.0.0` to `http` and caused a false-positive
+ * malware alert for the unrelated `http@0.0.1-security` placeholder.
  */
 export function stripPnpmPeerSuffix(depPath: string): string {
-  // Defensive: Handle empty or invalid inputs.
+  // Defensive: handle empty or invalid inputs.
   if (!depPath || typeof depPath !== 'string') {
     return depPath
   }
 
-  const parenIndex = depPath.indexOf('(')
-  const index = parenIndex === -1 ? depPath.indexOf('_') : parenIndex
-  return index === -1 ? depPath : depPath.slice(0, index)
+  // Locate the version-separating `@`. Skip a leading `@` on scoped
+  // packages so we don't stop at the scope delimiter.
+  const scopeOffset = depPath.startsWith('@') ? 1 : 0
+  const versionAt = depPath.indexOf('@', scopeOffset)
+  if (versionAt === -1) {
+    return depPath
+  }
+
+  // Search for `(` or `_` in the version-and-beyond portion only.
+  const tail = depPath.slice(versionAt)
+  const parenInTail = tail.indexOf('(')
+  const underInTail = tail.indexOf('_')
+
+  let cutInTail: number
+  if (parenInTail === -1) {
+    cutInTail = underInTail
+  } else if (underInTail === -1) {
+    cutInTail = parenInTail
+  } else {
+    // Both present — take whichever comes first (handles the pnpm
+    // v7 `(peer)_legacyPeer` shape observed in mixed-format lockfiles).
+    cutInTail = Math.min(parenInTail, underInTail)
+  }
+
+  return cutInTail === -1 ? depPath : depPath.slice(0, versionAt + cutInTail)
 }

packages/cli/test/unit/utils/pnpm/lockfile.test.mts

@@ -226,14 +226,43 @@ packages: {}`
       )
     })
 
-    it('prefers parentheses over underscore', () => {
+    it('prefers the earlier separator when both appear', () => {
+      // pnpm v7's `(peer)` wins because it comes before the legacy `_peer`.
       expect(stripPnpmPeerSuffix('pkg@1.0.0(peer)_other')).toBe('pkg@1.0.0')
     })
 
     it('returns unchanged for paths without suffixes', () => {
       expect(stripPnpmPeerSuffix('lodash@4.17.21')).toBe('lodash@4.17.21')
       expect(stripPnpmPeerSuffix('@babel/core@7.0.0')).toBe('@babel/core@7.0.0')
     })
+
+    it('preserves underscores inside package names (SMO-632 regression)', () => {
+      // `http_ece` is a legitimate package (encryption lib used by web-push).
+      // Stripping at the first `_` truncated it to `http`, which resolved to
+      // npm's `http@0.0.1-security` malware placeholder and produced a
+      // blocking false-positive alert.
+      expect(stripPnpmPeerSuffix('http_ece@1.0.0')).toBe('http_ece@1.0.0')
+      expect(stripPnpmPeerSuffix('http_ece@1.2.0_web-push@3.6.7')).toBe(
+        'http_ece@1.2.0',
+      )
+      expect(stripPnpmPeerSuffix('http_ece@1.2.0(web-push@3.6.7)')).toBe(
+        'http_ece@1.2.0',
+      )
+    })
+
+    it('handles scoped packages with underscore names', () => {
+      // Scoped equivalents of the SMO-632 pattern — the scope leader `@`
+      // must not be mistaken for the version-separating `@`.
+      expect(stripPnpmPeerSuffix('@foo/bar_baz@1.0.0')).toBe(
+        '@foo/bar_baz@1.0.0',
+      )
+      expect(stripPnpmPeerSuffix('@foo/bar_baz@1.0.0_peer@2.0.0')).toBe(
+        '@foo/bar_baz@1.0.0',
+      )
+      expect(stripPnpmPeerSuffix('@foo/bar_baz@1.0.0(peer@2.0.0)')).toBe(
+        '@foo/bar_baz@1.0.0',
+      )
+    })
   })
 
   describe('extractPurlsFromPnpmLockfile', () => {