fix: correctness improvements across url/versions/specs/fs/process-lock

- url.urlSearchParamAsBoolean: accept envAsBoolean's vocabulary
  ('1'|'true'|'yes'|'on' case-insensitive) and honor `defaultValue`
  for empty string input
- versions.maxVersion/minVersion: pass `includePrerelease: true` so
  all-prerelease inputs resolve instead of returning undefined under
  semver's default which filters prereleases out against '*'. Also
  updates the vendored semver .d.ts to expose the third argument.
- packages.getRepoUrlDetails: anchor to the literal `github.com` host
  with a proper regex so non-GitHub URLs, lookalikes (`githubXcom`,
  `fake-github.com`, `github.com.attacker.tld`), and scp-style
  `git@github.com:…` return clean empty fields instead of garbage
- fs.findUp / findUpSync: traverse up to and INCLUDING the filesystem
  root so matches at `/.foo` are actually found; the loop used to
  exit before visiting the root
- process-lock.isStale: compare in milliseconds rather than truncating
  both sides to seconds. Sub-second `staleMs` values (e.g. 500) were
  silently rounded up to ~1s.
- process-lock.acquire: drop the redundant `existsSync` before
  `mkdirSync(lockPath)` — mkdir's EEXIST is atomic on its own, the
  pre-check only opened a TOCTOU window.

Tests updated where they pinned the old (incorrect) behavior.

Author: jdalton

src/external/semver.d.ts

@@ -22,8 +22,20 @@ export function gte(version1: string, version2: string): boolean
 export function lt(version1: string, version2: string): boolean
 export function lte(version1: string, version2: string): boolean
 export function valid(version: string): string | null
-export function maxSatisfying(versions: string[], range: string): string | null
-export function minSatisfying(versions: string[], range: string): string | null
+export interface RangeOptions {
+  includePrerelease?: boolean
+  loose?: boolean
+}
+export function maxSatisfying(
+  versions: string[],
+  range: string,
+  options?: RangeOptions | boolean,
+): string | null
+export function minSatisfying(
+  versions: string[],
+  range: string,
+  options?: RangeOptions | boolean,
+): string | null
 export function sort(versions: string[]): string[]
 export function rsort(versions: string[]): string[]
 export function diff(

src/fs.ts

@@ -517,7 +517,10 @@ export async function findUp(
   let dir = path.resolve(cwd)
   const { root } = path.parse(dir)
   const names = isArray(name) ? name : [name as string]
-  while (dir && dir !== root) {
+  // Traverse up to and INCLUDING the filesystem root. Previously the
+  // loop exited when `dir === root`, so a match at e.g. `/.foo` was
+  // never visited.
+  while (dir) {
     for (const n of names) {
       if (signal?.aborted) {
         return undefined
@@ -534,6 +537,9 @@ export async function findUp(
         }
       } catch {}
     }
+    if (dir === root) {
+      break
+    }
     dir = path.dirname(dir)
   }
   return undefined
@@ -588,7 +594,10 @@ export function findUpSync(
   const { root } = path.parse(dir)
   const stopDir = stopAt ? path.resolve(stopAt) : undefined
   const names = isArray(name) ? name : [name as string]
-  while (dir && dir !== root) {
+  // Traverse up to and INCLUDING the filesystem root (or stopAt). The
+  // old `while (dir && dir !== root)` exited before visiting `root`
+  // itself, so a match at `/.foo` was never found.
+  while (dir) {
     // Check if we should stop at this directory.
     if (stopDir && dir === stopDir) {
       // Check current directory but don't go up.
@@ -618,6 +627,9 @@ export function findUpSync(
         }
       } catch {}
     }
+    if (dir === root) {
+      break
+    }
     dir = path.dirname(dir)
   }
   return undefined

src/packages/specs.ts

@@ -21,14 +21,23 @@ export function getRepoUrlDetails(repoUrl: string = ''): {
   user: string
   project: string
 } {
-  const userAndRepo = repoUrl.replace(/^.+github.com\//, '').split('/')
+  // Anchor the host to exactly `github.com` (optionally preceded by
+  // userinfo like `user@`). Escaping the `.` blocks lookalikes like
+  // `githubXcom`; pinning the host to the full final label blocks
+  // `fake-github.com` or `github.com.attacker.tld` shenanigans. Callers
+  // passing scp-style git@github.com:… need to normalize upstream; we
+  // require `://` here so the host component is unambiguous.
+  const match =
+    /^(?:[a-z]+:\/\/)(?:[^/@]+@)?github\.com\/([^?#]+)(?:[?#]|$)/i.exec(repoUrl)
+  if (!match || !match[1]) {
+    return { user: '', project: '' }
+  }
+  const userAndRepo = match[1].split('/')
   const user = userAndRepo[0] || ''
-  const project =
-    userAndRepo.length > 1
-      ? (userAndRepo[1]?.endsWith('.git')
-          ? userAndRepo[1].slice(0, -4)
-          : userAndRepo[1]) || ''
-      : ''
+  const rawProject = userAndRepo[1] ?? ''
+  const project = rawProject.endsWith('.git')
+    ? rawProject.slice(0, -4)
+    : rawProject
   return { user, project }
 }
 

src/process-lock.ts

@@ -227,10 +227,14 @@ class ProcessLockManager {
       if (!stats) {
         return false
       }
-      // Use second-level granularity to avoid APFS issues.
-      const ageSeconds = Math.floor((Date.now() - stats.mtime.getTime()) / 1000)
-      const staleSeconds = Math.floor(staleMs / 1000)
-      return ageSeconds > staleSeconds
+      // Compare in milliseconds. APFS's mtime precision is second-level
+      // but truncating the age to seconds was silently rounding sub-
+      // second `staleMs` values up to ~1s (staleMs=500 → staleSeconds=0,
+      // so the lock had to be 1s+ past mtime before it was considered
+      // stale). Floor only the mtime side to absorb APFS quirks, keep
+      // the threshold at full precision.
+      const ageMs = Date.now() - Math.floor(stats.mtime.getTime())
+      return ageMs > staleMs
     } catch {
       return false
     }
@@ -288,24 +292,21 @@ class ProcessLockManager {
             }
           }
 
-          // Check if lock already exists before creating.
-          const fs = getFs()
-          if (fs.existsSync(lockPath)) {
-            throw new Error(`Lock already exists: ${lockPath}`)
-          }
-
           // Ensure parent directory exists. Use path.dirname() so that both
           // POSIX and Windows separators (and mixed-separator inputs) are
           // handled correctly — the previous Math.max(lastIndexOf('/'), '\\')
           // approach failed on relative paths and mixed-separator inputs.
+          const fs = getFs()
           const parent = getPath().dirname(lockPath)
           if (parent && parent !== '.' && parent !== lockPath) {
             fs.mkdirSync(parent, { recursive: true })
           }
 
           // Atomic lock acquisition via mkdir (without recursive).
           // Without recursive, mkdirSync throws EEXIST if another process
-          // created the directory between the existsSync check and here.
+          // already owns the directory. No pre-check: an existsSync +
+          // mkdirSync pair opens a TOCTOU window without adding safety,
+          // because mkdirSync is atomic on its own.
           fs.mkdirSync(lockPath)
 
           // Track lock for cleanup.

src/url.ts

@@ -137,7 +137,21 @@ export function urlSearchParamAsBoolean(
   } as UrlSearchParamAsBooleanOptions
   if (typeof value === 'string') {
     const trimmed = value.trim()
-    return trimmed === '1' || trimmed.toLowerCase() === 'true'
+    // Empty string → use defaultValue, same as null/undefined. Previously
+    // fell through to the final truthy check and returned false, silently
+    // bypassing `defaultValue: true`.
+    if (trimmed === '') {
+      return !!defaultValue
+    }
+    const lowered = trimmed.toLowerCase()
+    // Accept the same truthy vocabulary as `envAsBoolean` so query-string
+    // flags behave predictably cross-context: '1', 'true', 'yes', 'on'.
+    return (
+      lowered === '1' ||
+      lowered === 'true' ||
+      lowered === 'yes' ||
+      lowered === 'on'
+    )
   }
   if (value === null || value === undefined) {
     return !!defaultValue

src/versions.ts

@@ -242,7 +242,14 @@ export function isValidVersion(version: string): boolean {
 export function maxVersion(versions: string[]): string | undefined {
   /* c8 ignore next - External semver call */
   const semver = getSemver()
-  return semver.maxSatisfying(versions, '*') || undefined
+  // includePrerelease: true so an all-prerelease input like
+  // ['1.0.0-alpha', '1.0.0-beta'] resolves to the latest prerelease
+  // instead of returning undefined under semver's default (which filters
+  // prereleases out against '*').
+  return (
+    semver.maxSatisfying(versions, '*', { includePrerelease: true }) ||
+    undefined
+  )
 }
 
 /**
@@ -256,7 +263,10 @@ export function maxVersion(versions: string[]): string | undefined {
 export function minVersion(versions: string[]): string | undefined {
   /* c8 ignore next - External semver call */
   const semver = getSemver()
-  return semver.minSatisfying(versions, '*') || undefined
+  return (
+    semver.minSatisfying(versions, '*', { includePrerelease: true }) ||
+    undefined
+  )
 }
 
 /**

test/unit/packages/specs.test.mts

@@ -36,11 +36,14 @@ describe('packages/specs', () => {
       expect(result.project).toBe('node')
     })
 
-    it('should handle git@ protocol URLs', () => {
+    it('rejects git@ protocol URLs (scp-style is not supported)', () => {
+      // scp-style `git@github.com:npm/cli.git` has no `://` separator;
+      // the matcher requires one so the host is unambiguously `github.com`
+      // rather than the whole string before `:`. Callers that need scp
+      // parsing should normalize to https upstream.
       const result = getRepoUrlDetails('git@github.com:npm/cli.git')
-      // Note: the function doesn't handle git@ URLs with : separator correctly
-      expect(result.user).toBe('git@github.com:npm')
-      expect(result.project).toBe('cli')
+      expect(result.user).toBe('')
+      expect(result.project).toBe('')
     })
 
     it('should handle git:// protocol URLs', () => {
@@ -49,12 +52,28 @@ describe('packages/specs', () => {
       expect(result.project).toBe('berry')
     })
 
-    it('should return empty strings for invalid URL', () => {
+    it('returns empty strings for invalid URL', () => {
+      // Previously the loose `/^.+github.com\//` replace left garbage in
+      // user/project for non-GitHub or malformed inputs. Now returns a
+      // clean empty result so callers can branch on it.
       const result = getRepoUrlDetails('not-a-valid-url')
-      expect(result.user).toBe('not-a-valid-url')
+      expect(result.user).toBe('')
       expect(result.project).toBe('')
     })
 
+    it('rejects github.com lookalike hosts', () => {
+      // Previously `/^.+github.com\//` matched any host that happened to
+      // end with `github.com` literally because `.` was unescaped.
+      expect(getRepoUrlDetails('https://githubXcom/a/b')).toEqual({
+        user: '',
+        project: '',
+      })
+      expect(getRepoUrlDetails('https://fake-github.com.attacker.tld/a/b')).toEqual({
+        user: '',
+        project: '',
+      })
+    })
+
     it('should handle empty string', () => {
       const result = getRepoUrlDetails('')
       expect(result.user).toBe('')

test/unit/url.test.mts

@@ -221,9 +221,13 @@ describe('url', () => {
       expect(urlSearchParamAsBoolean('0')).toBe(false)
     })
 
-    it('should return false for other strings', () => {
+    it('accepts the same truthy vocabulary as envAsBoolean', () => {
+      // Expanded set so callers get consistent behavior across env vars
+      // and query strings: '1' | 'true' | 'yes' | 'on' (case-insensitive).
+      expect(urlSearchParamAsBoolean('yes')).toBe(true)
+      expect(urlSearchParamAsBoolean('Yes')).toBe(true)
+      expect(urlSearchParamAsBoolean('on')).toBe(true)
       expect(urlSearchParamAsBoolean('hello')).toBe(false)
-      expect(urlSearchParamAsBoolean('yes')).toBe(false)
       expect(urlSearchParamAsBoolean('no')).toBe(false)
     })