fix(cache,dlx,github): fix clock skew, TOCTOU races, and parsing bugs

Quality scan findings addressed:

Critical fixes:
- Add try-catch to JSON.parse in github.ts to prevent crashes on malformed responses

High-severity fixes:
- Fix unconditional .git truncation in specs.ts (now checks if URL ends with .git)
- Fix scoped package parsing in dlx/package.ts (check atIndex === 0 instead of startsWith('@'))
- Add clock skew protection to cache validation in dlx/binary.ts (reject future timestamps)
- Add clock skew detection to TTL cache in cache-with-ttl.ts (treat far-future expiresAt as expired)

Medium-severity fixes:
- Fix future timestamp handling in dlx cache cleanup (treat as expired)
- Use atomic write-then-rename for metadata writes in dlx/binary.ts
- Add TOCTOU race protection to version file check in releases/github.ts
- Replace process.exit() with process.exitCode in scripts/build/js.mjs

Test updates:
- Update specs.test.mts to expect correct behavior after .git truncation fix

Author: jdalton

scripts/build/js.mjs

@@ -101,7 +101,8 @@ async function watchJS() {
         logger.log('\nStopping watch mode...')
       }
       await ctx.dispose()
-      process.exit(0)
+      process.exitCode = 0
+      throw new Error('Watch mode interrupted')
     })
 
     // Wait indefinitely
@@ -119,7 +120,7 @@ async function watchJS() {
 if (isWatch) {
   watchJS().catch(error => {
     logger.error(error)
-    process.exit(1)
+    process.exitCode = 1
   })
 } else {
   buildJS()

src/cache-with-ttl.ts

@@ -190,9 +190,16 @@ export function createTtlCache(options?: TtlCacheOptions): TtlCache {
 
   /**
    * Check if entry is expired.
+   * Also detects clock skew by treating suspiciously far-future expiresAt as expired.
    */
   function isExpired(entry: TtlCacheEntry<any>): boolean {
-    return Date.now() > entry.expiresAt
+    const now = Date.now()
+    // Detect future expiresAt (clock skew or corruption).
+    // If expiresAt is more than 2x TTL in the future, treat as expired.
+    if (entry.expiresAt > now + ttl * 2) {
+      return true
+    }
+    return now > entry.expiresAt
   }
 
   /**

src/dlx/binary.ts

@@ -225,7 +225,8 @@ export async function cleanDlxCache(
           ? now - timestamp
           : Number.POSITIVE_INFINITY
 
-      if (age > maxAge) {
+      // Treat future timestamps (clock skew) as expired
+      if (age < 0 || age > maxAge) {
         // Remove entire cache entry directory.
         // eslint-disable-next-line no-await-in-loop
         await safeDelete(entryPath, { force: true, recursive: true })
@@ -645,7 +646,10 @@ export async function isCacheValid(
       return false
     }
     const age = now - timestamp
-
+    // Reject future timestamps (clock skew or corruption)
+    if (age < 0) {
+      return false
+    }
     return age < cacheTtl
   } catch {
     return false
@@ -752,5 +756,8 @@ export async function writeMetadata(
     },
   }
   const fs = getFs()
-  await fs.promises.writeFile(metaPath, JSON.stringify(metadata, null, 2))
+  // Use atomic write-then-rename pattern to prevent corruption on crash
+  const tmpPath = `${metaPath}.tmp.${process.pid}`
+  await fs.promises.writeFile(tmpPath, JSON.stringify(metadata, null, 2))
+  await fs.promises.rename(tmpPath, metaPath)
 }

src/dlx/package.ts

@@ -618,8 +618,8 @@ export function parsePackageSpec(spec: string): {
   } catch {
     // Fallback to simple parsing if npm-package-arg fails.
     const atIndex = spec.lastIndexOf('@')
-    if (atIndex === -1 || spec.startsWith('@')) {
-      // No version or scoped package without version.
+    if (atIndex === -1 || atIndex === 0) {
+      // No version or scoped package without version (@ only at position 0).
       return { name: spec, version: undefined }
     }
     return {

src/github.ts

@@ -214,7 +214,16 @@ export async function fetchGitHub<T = unknown>(
     )
   }
 
-  return JSON.parse(response.body.toString('utf8')) as T
+  try {
+    return JSON.parse(response.body.toString('utf8')) as T
+  } catch (error) {
+    throw new Error(
+      `Failed to parse GitHub API response: ${error instanceof Error ? error.message : String(error)}\n` +
+        `URL: ${url}\n` +
+        'Response may be malformed or incomplete.',
+      { cause: error },
+    )
+  }
 }
 
 /**

src/packages/specs.ts

@@ -18,7 +18,11 @@ export function getRepoUrlDetails(repoUrl: string = ''): {
   const userAndRepo = repoUrl.replace(/^.+github.com\//, '').split('/')
   const user = userAndRepo[0] || ''
   const project =
-    userAndRepo.length > 1 ? userAndRepo[1]?.slice(0, -'.git'.length) || '' : ''
+    userAndRepo.length > 1
+      ? (userAndRepo[1]?.endsWith('.git')
+          ? userAndRepo[1].slice(0, -4)
+          : userAndRepo[1]) || ''
+      : ''
   return { user, project }
 }
 

src/releases/github.ts

@@ -217,7 +217,8 @@ export async function downloadGitHubRelease(
     const cachedVersion = (
       await fs.promises.readFile(versionPath, 'utf8')
     ).trim()
-    if (cachedVersion === tag) {
+    // Re-check binary exists after reading version (prevent TOCTOU race)
+    if (cachedVersion === tag && fs.existsSync(binaryPath)) {
       if (!quiet) {
         logger.info(`Using cached ${toolName} (${platformArch}): ${binaryPath}`)
       }

test/unit/packages/specs.test.mts

@@ -32,8 +32,8 @@ describe('packages/specs', () => {
     it('should handle URL without .git extension', () => {
       const result = getRepoUrlDetails('https://github.com/nodejs/node')
       expect(result.user).toBe('nodejs')
-      // Note: function slices off 4 chars (.git) even when not present
-      expect(result.project).toBe('')
+      // Fixed: now correctly returns project name without .git extension
+      expect(result.project).toBe('node')
     })
 
     it('should handle git@ protocol URLs', () => {
@@ -72,8 +72,8 @@ describe('packages/specs', () => {
         'https://github.com/facebook/react/tree/main',
       )
       expect(result.user).toBe('facebook')
-      // Note: function slices off 4 chars (.git) from "react/tree/main"
-      expect(result.project).toBe('r')
+      // Fixed: now correctly returns repo name without incorrect .git truncation
+      expect(result.project).toBe('react')
     })
   })