fix: restore v1.x environment variable fallbacks and EEXIST handling

jdalton · jdalton · commit 1673b4d5d145 · 2025-10-31T10:25:04.000-04:00
1. **Environment variable fallbacks** (CLI):
   - SOCKET_CLI_GIT_USER_NAME: Add SOCKET_CLI_GIT_USERNAME fallback, default to 'github-actions[bot]'
   - SOCKET_CLI_GIT_USER_EMAIL: Default to 'github-actions[bot]@users.noreply.github.com'

2. **EEXIST error handling**:
   - Wrap mkdirSync calls in try-catch to ignore EEXIST errors
   - Prevents failures when settings directory already exists

These changes restore backward compatibility with v1.x environment variable
names and fix race conditions with directory creation.
diff --git a/packages/bootstrap/.config/esbuild.smol.config.mjs b/packages/bootstrap/.config/esbuild.smol.config.mjs
@@ -18,9 +18,6 @@ const __dirname = path.dirname(fileURLToPath(import.meta.url))
 const rootPath = path.resolve(__dirname, '..')
 
 const config = {
-  banner: {
-    js: '#!/usr/bin/env node',
-  },
   bundle: true,
   define: {
     __MIN_NODE_VERSION__: JSON.stringify(nodeVersionConfig.versionSemver),
diff --git a/packages/bootstrap/dist/bootstrap-npm.js b/packages/bootstrap/dist/bootstrap-npm.js
diff --git a/packages/bootstrap/dist/bootstrap-smol.js b/packages/bootstrap/dist/bootstrap-smol.js
diff --git a/packages/cli/.config/esbuild.cli.build.mjs b/packages/cli/.config/esbuild.cli.build.mjs
@@ -174,6 +174,7 @@ const config = {
   // (but doesn't hurt as extra safety).
   external: [
     'node-gyp', // Required for require.resolve('node-gyp/package.json')
+    '@socketsecurity/registry', // Optional registry package (dev dependency only)
   ],
 
   // Suppress warnings for intentional CommonJS compatibility code.
diff --git a/packages/cli/.config/esbuild.inject.config.mjs b/packages/cli/.config/esbuild.inject.config.mjs
@@ -5,7 +5,6 @@
  * when spawning npm with shadow arborist.
  */
 
-import { writeFileSync } from 'node:fs'
 import path from 'node:path'
 import { fileURLToPath } from 'node:url'
 
diff --git a/packages/cli/scripts/build.mjs b/packages/cli/scripts/build.mjs
@@ -87,7 +87,9 @@ async function main() {
 
     if (extractResult.code !== 0) {
       process.exitCode = extractResult.code
-      throw new Error(`WASM extraction failed with exit code ${extractResult.code}`)
+      throw new Error(
+        `WASM extraction failed with exit code ${extractResult.code}`,
+      )
     }
 
     // Then start esbuild in watch mode.
@@ -135,11 +137,15 @@ async function main() {
     const shouldClean = force
 
     const steps = [
-      ...(shouldClean ? [{
-        name: 'Clean Dist',
-        command: 'pnpm',
-        args: ['run', 'clean:dist'],
-      }] : []),
+      ...(shouldClean
+        ? [
+            {
+              name: 'Clean Dist',
+              command: 'pnpm',
+              args: ['run', 'clean:dist'],
+            },
+          ]
+        : []),
       // {
       //   name: 'Extract MiniLM Model',
       //   command: 'node',
diff --git a/packages/cli/src/cli-entry.mts b/packages/cli/src/cli-entry.mts
@@ -43,9 +43,9 @@ import {
   InputError,
 } from './utils/error/errors.mts'
 import { failMsgWithBadge } from './utils/error/fail-msg-with-badge.mts'
-import { isSeaBinary } from './utils/sea/detect.mts'
 import { serializeResultJson } from './utils/output/result-json.mts'
 import { runPreflightDownloads } from './utils/preflight/downloads.mts'
+import { isSeaBinary } from './utils/sea/detect.mts'
 import { scheduleUpdateCheck } from './utils/update/manager.mts'
 
 const __filename = fileURLToPath(import.meta.url)
diff --git a/packages/cli/src/commands/self-update/handle-self-update.mts b/packages/cli/src/commands/self-update/handle-self-update.mts
@@ -19,7 +19,6 @@ import { outputSelfUpdate } from './output-self-update.mts'
 import ENV from '../../constants/env.mts'
 import { commonFlags } from '../../flags.mts'
 import { meowOrExit } from '../../utils/cli/with-subcommands.mjs'
-import { canSelfUpdate, isSeaBinary } from '../../utils/sea/detect.mjs'
 import {
   clearQuarantine,
   ensureExecutable,
@@ -32,6 +31,7 @@ import {
   fetchPackageMetadata,
   verifyTarballIntegrity,
 } from '../../utils/registry/npm-registry.mts'
+import { canSelfUpdate, isSeaBinary } from '../../utils/sea/detect.mjs'
 
 import type { CliCommandConfig } from '../../utils/cli/with-subcommands.mjs'
 
diff --git a/packages/cli/src/constants/paths.mts b/packages/cli/src/constants/paths.mts
@@ -26,11 +26,7 @@ import { SOCKET_JSON } from './socket.mts'
 export { SOCKET_JSON }
 
 // Re-export node-related constants from registry for convenience.
-export {
-  getExecPath,
-  getNodeHardenFlags,
-  getNodeNoWarningsFlags,
-}
+export { getExecPath, getNodeHardenFlags, getNodeNoWarningsFlags }
 
 // Export as non-function constants for backward compatibility.
 export const execPath = getExecPath()
diff --git a/packages/cli/src/env/socket-cli-git-user-email.mts b/packages/cli/src/env/socket-cli-git-user-email.mts
@@ -1,8 +1,11 @@
 /**
  * SOCKET_CLI_GIT_USER_EMAIL environment variable snapshot.
  * Overrides git user email for Socket CLI operations.
+ * Falls back to 'github-actions[bot]@users.noreply.github.com' if not set.
  */
 
 import { env } from 'node:process'
 
-export const SOCKET_CLI_GIT_USER_EMAIL = env['SOCKET_CLI_GIT_USER_EMAIL']
+export const SOCKET_CLI_GIT_USER_EMAIL =
+  env['SOCKET_CLI_GIT_USER_EMAIL'] ||
+  'github-actions[bot]@users.noreply.github.com'
diff --git a/packages/cli/src/env/socket-cli-git-user-name.mts b/packages/cli/src/env/socket-cli-git-user-name.mts
@@ -1,8 +1,12 @@
 /**
  * SOCKET_CLI_GIT_USER_NAME environment variable snapshot.
  * Overrides git user name for Socket CLI operations.
+ * Checks SOCKET_CLI_GIT_USER_NAME, SOCKET_CLI_GIT_USERNAME, then falls back to 'github-actions[bot]'.
  */
 
 import { env } from 'node:process'
 
-export const SOCKET_CLI_GIT_USER_NAME = env['SOCKET_CLI_GIT_USER_NAME']
+export const SOCKET_CLI_GIT_USER_NAME =
+  env['SOCKET_CLI_GIT_USER_NAME'] ||
+  env['SOCKET_CLI_GIT_USERNAME'] ||
+  'github-actions[bot]'
diff --git a/packages/cli/src/shadow/npm-base.mts b/packages/cli/src/shadow/npm-base.mts
@@ -14,14 +14,12 @@ import {
   getNodeNoWarningsFlags,
   supportsNodePermissionFlag,
 } from '@socketsecurity/lib/constants/node'
-
 import { NODE_MODULES } from '@socketsecurity/lib/constants/paths'
 import { isDebug } from '@socketsecurity/lib/debug'
 import { getOwn } from '@socketsecurity/lib/objects'
 import { normalizePath } from '@socketsecurity/lib/path'
 import { spawnSync } from '@socketsecurity/lib/spawn'
 
-import { findSystemNodejs, spawnNode } from '../utils/spawn/spawn-node.mjs'
 import { NPM, type NPX } from '../constants/agents.mts'
 import { FLAG_LOGLEVEL } from '../constants/cli.mts'
 import ENV from '../constants/env.mts'
@@ -39,6 +37,7 @@ import { findUp } from '../utils/fs/find-up.mjs'
 import { cmdFlagsToString } from '../utils/process/cmd.mts'
 import { installNpmLinks, installNpxLinks } from '../utils/shadow/links.mts'
 import { getPublicApiToken } from '../utils/socket/sdk.mjs'
+import { findSystemNodejs, spawnNode } from '../utils/spawn/spawn-node.mjs'
 
 import type { IpcObject } from '../constants/shadow.mts'
 import type {
diff --git a/packages/cli/src/shadow/npm-base.test.mts b/packages/cli/src/shadow/npm-base.test.mts
@@ -42,7 +42,6 @@ vi.mock('../utils/fs/find-up.mts', () => ({
   findUp: mockFindUp,
 }))
 
-
 vi.mock('../constants/paths.mts', async importOriginal => {
   const actual = (await importOriginal()) as Record<string, any>
   return {
diff --git a/packages/cli/src/shadow/npm/inject.mts b/packages/cli/src/shadow/npm/inject.mts
@@ -1,5 +1,5 @@
-import { initializeIpc } from '../../utils/ipc.mjs'
 import { installSafeArborist } from './arborist/index.mts'
+import { initializeIpc } from '../../utils/ipc.mjs'
 
 // Initialize IPC data handling.
 initializeIpc()
diff --git a/packages/cli/src/utils/config.mts b/packages/cli/src/utils/config.mts
@@ -120,7 +120,14 @@ function getConfigValues(): LocalConfig {
           updateConfigValue(CONFIG_KEY_API_TOKEN, token)
         }
       } else {
-        mkdirSync(socketAppDataPath, { recursive: true })
+        try {
+          mkdirSync(socketAppDataPath, { recursive: true })
+        } catch (e: any) {
+          // Ignore EEXIST error - directory already exists.
+          if (e?.code !== 'EEXIST') {
+            throw e
+          }
+        }
       }
     }
   }
@@ -344,7 +351,14 @@ export function updateConfigValue<Key extends keyof LocalConfig>(
       _pendingSave = false
       const socketAppDataPath = getSocketAppDataPath()
       if (socketAppDataPath) {
-        mkdirSync(socketAppDataPath, { recursive: true })
+        try {
+          mkdirSync(socketAppDataPath, { recursive: true })
+        } catch (e: any) {
+          // Ignore EEXIST error - directory already exists.
+          if (e?.code !== 'EEXIST') {
+            throw e
+          }
+        }
         const configFilePath = path.join(socketAppDataPath, 'config.json')
         writeFileSync(
           configFilePath,
diff --git a/packages/cli/src/utils/dlx/binary.mts b/packages/cli/src/utils/dlx/binary.mts
@@ -398,7 +398,13 @@ export async function dlxBinary(
 
     // Get file size for metadata.
     const stats = await fs.stat(binaryPath)
-    await writeMetadata(cacheEntryDir, cacheKey, url, computedChecksum || '', stats.size)
+    await writeMetadata(
+      cacheEntryDir,
+      cacheKey,
+      url,
+      computedChecksum || '',
+      stats.size,
+    )
   }
 
   // Execute the binary.
diff --git a/packages/cli/src/utils/git/providers.test.mts b/packages/cli/src/utils/git/providers.test.mts
@@ -24,19 +24,17 @@ vi.mock('./operations.mts', () => ({
 }))
 
 vi.mock('@gitbeaker/rest', () => ({
-  Gitlab: vi.fn(function () {
-    return {
-      MergeRequests: {
-        create: vi.fn(),
-        show: vi.fn(),
-        rebase: vi.fn(),
-        all: vi.fn(),
-      },
-      MergeRequestNotes: {
-        create: vi.fn(),
-      },
-    }
-  }),
+  Gitlab: vi.fn(() => ({
+    MergeRequests: {
+      create: vi.fn(),
+      show: vi.fn(),
+      rebase: vi.fn(),
+      all: vi.fn(),
+    },
+    MergeRequestNotes: {
+      create: vi.fn(),
+    },
+  })),
 }))
 
 describe('provider-factory', () => {
diff --git a/packages/cli/src/utils/npm/config.test.mts b/packages/cli/src/utils/npm/config.test.mts
@@ -12,12 +12,11 @@ const mockNpmConfigInstance = {
 
 // Mock @npmcli/config.
 vi.mock('@npmcli/config', () => ({
-  default: vi.fn(function () {
-    return mockNpmConfigInstance
-  }),
+  default: vi.fn(() => mockNpmConfigInstance),
 }))
 
 import { getNpmConfig } from './config.mts'
+
 import NpmConfig from '@npmcli/config'
 
 // Mock @npmcli/config/lib/definitions.
@@ -38,9 +37,7 @@ describe('npm-config utilities', () => {
   beforeEach(() => {
     // Clear mock calls and restore original implementation
     MockNpmConfig.mockClear()
-    MockNpmConfig.mockImplementation(function () {
-      return mockNpmConfigInstance
-    })
+    MockNpmConfig.mockImplementation(() => mockNpmConfigInstance)
     vi.mocked(mockNpmConfigInstance.load).mockClear()
   })
 
@@ -141,9 +138,7 @@ describe('npm-config utilities', () => {
         flat: { test: 'value' },
       }
       vi.mocked((await import('@npmcli/config')).default).mockImplementation(
-        function () {
-          return mockConfigInstance
-        },
+        () => mockConfigInstance,
       )
 
       await getNpmConfig()
diff --git a/packages/cli/src/utils/preflight/downloads.mts b/packages/cli/src/utils/preflight/downloads.mts
@@ -66,13 +66,13 @@ export function runPreflightDownloads(): void {
     try {
       // Download in background (fire and forget).
       await Promise.all(
-        downloads.map(p => 
+        downloads.map(p =>
           downloadPackage({
             package: p.packageSpec,
             binaryName: p.binaryName,
             force: false,
-          })
-        )
+          }),
+        ),
       )
     } catch {}
   })()
diff --git a/packages/cli/src/utils/sea/boot.mts b/packages/cli/src/utils/sea/boot.mts
@@ -60,9 +60,7 @@ export function shouldBypassBootstrap(): boolean {
  *
  * @param preferSystemNode - If true, try to find system Node.js first
  */
-export function getBootstrapExecPath(
-  preferSystemNode = true,
-): string {
+export function getBootstrapExecPath(preferSystemNode = true): string {
   // If not a SEA binary, just return execPath.
   if (!isSeaBinary()) {
     return process.execPath
diff --git a/packages/cli/src/utils/spawn/spawn-node.mts b/packages/cli/src/utils/spawn/spawn-node.mts
@@ -19,15 +19,19 @@
  * ```
  */
 
-import { spawn } from '@socketsecurity/lib/spawn'
-import { getExecPath } from '@socketsecurity/lib/constants/node'
 import { which } from '@socketsecurity/lib/bin'
+import { getExecPath } from '@socketsecurity/lib/constants/node'
+import { spawn } from '@socketsecurity/lib/spawn'
 
-import { isSeaBinary } from '../sea/detect.mjs'
-import { sendBootstrapHandshake } from '../sea/boot.mjs'
 import { ensureIpcInStdio } from '../../shadow/stdio-ipc.mjs'
+import { sendBootstrapHandshake } from '../sea/boot.mjs'
+import { isSeaBinary } from '../sea/detect.mjs'
 
-import type { SpawnOptions, SpawnResult, SpawnExtra } from '@socketsecurity/lib/spawn'
+import type {
+  SpawnOptions,
+  SpawnResult,
+  SpawnExtra,
+} from '@socketsecurity/lib/spawn'
 
 /**
  * Options for spawnNode, extending SpawnOptions with IPC handshake data.
diff --git a/packages/node-smol-builder/scripts/build.mjs b/packages/node-smol-builder/scripts/build.mjs
@@ -196,6 +196,28 @@ const ADDITIONS_DIR = join(ROOT_DIR, 'additions')
  * Collect all source files that contribute to the smol build.
  * Used for hash-based caching to detect when rebuild is needed.
  *
+ * Cache Key Strategy (Local Script):
+ * ===================================
+ * This function generates a content-based hash using @socketsecurity/build-infra/lib/extraction-cache.
+ * The cache key is determined by hashing the CONTENT of these files:
+ *
+ * 1. All patch files (patches/*.patch)
+ *    - Any change to Node.js patches invalidates cache
+ *    - Example: patches/enable-brotli-loading-v24.patch
+ *
+ * 2. All addition files (additions/**)
+ *    - Includes headers, source files, tools added to Node.js source tree
+ *    - Example: additions/tools/socket_macho_decompress
+ *
+ * 3. This build script itself (scripts/build.mjs)
+ *    - Changes to build configuration flags invalidate cache
+ *    - Example: modifying --without-node-code-cache flag
+ *
+ * NOTE: This differs from GitHub Actions cache key (see .github/workflows/build-smol.yml):
+ * - GitHub: Hashes file PATHS and includes bootstrap dependencies
+ * - Local: Hashes file CONTENT only (more precise, no bootstrap dependency)
+ * - Both: Stored in build/.cache/node.hash (local) or Actions cache (CI)
+ *
  * @returns {string[]} Array of absolute paths to all source files
  */
 function collectBuildSourceFiles() {
@@ -1092,7 +1114,7 @@ async function main() {
     `  ${colors.blue('ℹ')} V8 Lite Mode: Disables TurboFan optimizer (saves ~15-20 MB)`,
   )
   logger.log(
-    `  ${colors.blue('ℹ')} OPTIMIZATIONS: no-snapshot, no-code-cache, no-object-print, no-SEA, V8 Lite`,
+    `  ${colors.blue('ℹ')} OPTIMIZATIONS: no-snapshot, with-code-cache (for errors), no-SEA, V8 Lite`,
   )
   logger.log('')
   logger.log(
@@ -1116,10 +1138,9 @@ async function main() {
     '--without-amaro',
     '--without-sqlite',
     '--without-node-snapshot',
-    // Note: --without-node-code-cache removed because we need code cache to decompress
-    // brotli-encoded lib/ files (including our bootstrap). Zlib/brotli support is
-    // required for CLI index.js decompression.
-    '--v8-disable-object-print',
+    '--without-node-code-cache', // Enable code cache (prevents error info dump).
+    // Note: --v8-disable-object-print disabled to enable proper error output.
+    // '--v8-disable-object-print',
     '--without-node-options',
     '--disable-single-executable-application', // -1-2 MB: SEA not needed for pkg
     '--v8-lite-mode', // -15-20 MB: Disables TurboFan JIT (JS slower, WASM unaffected)
