security(spawn): strip dynamic-linker env vars from subprocess env

src/utils/__tests__/env.test.ts

@@ -9,6 +9,7 @@ import * as config from '../../config';
 import {
   checkEnvForPrerequisite,
   sanitizeDynamicLinkerEnv,
+  sanitizeSpawnEnv,
   warnIfCraftEnvFileExists,
 } from '../env';
 import { ConfigurationError } from '../errors';
@@ -336,4 +337,88 @@ describe('env utils functions', () => {
       expect(logger.warn).toHaveBeenCalledTimes(1);
     });
   }); // end describe('sanitizeDynamicLinkerEnv')
+
+  describe('sanitizeSpawnEnv', () => {
+    beforeEach(() => {
+      delete process.env.CRAFT_ALLOW_DYNAMIC_LINKER_ENV;
+    });
+
+    test('returns undefined for undefined input', () => {
+      expect(sanitizeSpawnEnv(undefined)).toBeUndefined();
+    });
+
+    test('returns a shallow copy without dynamic-linker keys', () => {
+      const input = {
+        PATH: '/usr/bin',
+        HOME: '/home/runner',
+        LD_PRELOAD: '/tmp/evil.so',
+        LD_LIBRARY_PATH: '/opt/lib',
+        GITHUB_TOKEN: 'gho_xxx',
+      };
+
+      const result = sanitizeSpawnEnv(input);
+
+      expect(result).toEqual({
+        PATH: '/usr/bin',
+        HOME: '/home/runner',
+        GITHUB_TOKEN: 'gho_xxx',
+      });
+      // Input must not be mutated — callers may reuse it.
+      expect(input.LD_PRELOAD).toBe('/tmp/evil.so');
+      expect(input.LD_LIBRARY_PATH).toBe('/opt/lib');
+    });
+
+    test('strips all DYLD_* variants on macOS layouts', () => {
+      const input = {
+        PATH: '/usr/bin',
+        DYLD_INSERT_LIBRARIES: '/tmp/a.dylib',
+        DYLD_LIBRARY_PATH: '/opt/a',
+        DYLD_FRAMEWORK_PATH: '/opt/b',
+        DYLD_FALLBACK_LIBRARY_PATH: '/opt/c',
+        DYLD_FALLBACK_FRAMEWORK_PATH: '/opt/d',
+      };
+
+      const result = sanitizeSpawnEnv(input);
+
+      expect(result).toEqual({ PATH: '/usr/bin' });
+    });
+
+    test('warns once when any dynamic-linker key is stripped', () => {
+      sanitizeSpawnEnv({ PATH: '/u', LD_PRELOAD: '/x', LD_LIBRARY_PATH: '/y' });
+
+      expect(logger.warn).toHaveBeenCalledTimes(1);
+      const msg = (logger.warn as any).mock.calls[0].join(' ');
+      expect(msg).toContain('subprocess');
+      // Value must never appear in the log.
+      expect(msg).not.toContain('/x');
+      expect(msg).not.toContain('/y');
+    });
+
+    test('does not warn when no dynamic-linker keys are present', () => {
+      sanitizeSpawnEnv({ PATH: '/u', HOME: '/h' });
+      expect(logger.warn).not.toHaveBeenCalled();
+    });
+
+    test('honours CRAFT_ALLOW_DYNAMIC_LINKER_ENV=1 opt-out', () => {
+      process.env.CRAFT_ALLOW_DYNAMIC_LINKER_ENV = '1';
+      const input = { PATH: '/u', LD_PRELOAD: '/tmp/legit.so' };
+
+      const result = sanitizeSpawnEnv(input);
+
+      expect(result).toEqual(input);
+      expect(result).toBe(input); // same reference — explicit no-op
+      expect(logger.warn).not.toHaveBeenCalled();
+    });
+
+    test('opt-out only applies when value is exactly "1"', () => {
+      process.env.CRAFT_ALLOW_DYNAMIC_LINKER_ENV = 'yes';
+      const result = sanitizeSpawnEnv({
+        PATH: '/u',
+        LD_PRELOAD: '/tmp/x.so',
+      });
+
+      expect(result).toEqual({ PATH: '/u' });
+      expect(logger.warn).toHaveBeenCalledTimes(1);
+    });
+  }); // end describe('sanitizeSpawnEnv')
 }); // end describe('env utils functions')

src/utils/__tests__/system.test.ts

@@ -96,6 +96,70 @@ describe('spawnProcess', () => {
     expect(mockedLogInfo).toHaveBeenCalledTimes(1);
     expect(mockedLogInfo.mock.calls[0][0]).toMatch(/test-string/);
   });
+
+  describe('env sanitisation (defence-in-depth)', () => {
+    const savedEnv = { ...process.env };
+
+    afterEach(() => {
+      process.env = { ...savedEnv };
+    });
+
+    test('strips LD_PRELOAD from an explicit options.env', async () => {
+      const stdout =
+        (await spawnProcess(
+          process.execPath,
+          [
+            '-e',
+            'process.stdout.write(JSON.stringify({ ld: process.env.LD_PRELOAD, marker: process.env.CHILD_MARKER }))',
+          ],
+          {
+            env: {
+              PATH: process.env.PATH,
+              LD_PRELOAD: '/tmp/evil.so',
+              CHILD_MARKER: 'reached',
+            },
+          },
+        )) || '';
+
+      const parsed = JSON.parse(stdout.toString());
+      expect(parsed.ld).toBeUndefined();
+      // Other env vars still propagate.
+      expect(parsed.marker).toBe('reached');
+    });
+
+    test('strips LD_PRELOAD set on process.env after startup', async () => {
+      // Simulate a post-startup mutation of process.env (hostile or
+      // accidental). startup-level sanitizeDynamicLinkerEnv() cannot
+      // catch this; the spawn-level sanitiser must.
+      process.env.LD_PRELOAD = '/tmp/later-evil.so';
+
+      const stdout =
+        (await spawnProcess(process.execPath, [
+          '-e',
+          'process.stdout.write(process.env.LD_PRELOAD || "undef")',
+        ])) || '';
+
+      expect(stdout.toString()).toBe('undef');
+    });
+
+    test('honours CRAFT_ALLOW_DYNAMIC_LINKER_ENV=1 opt-out', async () => {
+      process.env.CRAFT_ALLOW_DYNAMIC_LINKER_ENV = '1';
+
+      const stdout =
+        (await spawnProcess(
+          process.execPath,
+          ['-e', 'process.stdout.write(process.env.LD_PRELOAD || "undef")'],
+          {
+            env: {
+              PATH: process.env.PATH,
+              LD_PRELOAD: '/tmp/allowed.so',
+            },
+          },
+        )) || '';
+
+      expect(stdout.toString()).toBe('/tmp/allowed.so');
+    });
+  });
 });
 
 describe('replaceEnvVariable', () => {

src/utils/dynamicLinkerEnv.ts

@@ -0,0 +1,123 @@
+/**
+ * Helpers for stripping dynamic-linker environment variables
+ * (`LD_PRELOAD`, `LD_LIBRARY_PATH`, `DYLD_*`, etc.) from Craft's own
+ * environment and from environments passed to subprocess spawns.
+ *
+ * These variables are a well-known supply-chain attack vector: an
+ * attacker who can influence the environment of a process can load an
+ * arbitrary shared library into every subprocess it spawns, gaining
+ * code execution with access to every credential those subprocesses
+ * touch (GitHub tokens, npm tokens, GPG keys, ...).
+ *
+ * This module has intentionally minimal imports (just the logger) so
+ * it can be safely imported from low-level utilities like
+ * `src/utils/system.ts` without creating a circular dependency with
+ * `src/config.ts` / the artifact providers.
+ */
+
+import { logger } from '../logger';
+
+/**
+ * Environment variable names Craft refuses to propagate.
+ *
+ * Any legitimate use (e.g. an instrumented build toolchain that relies
+ * on `LD_LIBRARY_PATH`) can be re-enabled for a single Craft
+ * invocation via {@link ALLOW_DYNAMIC_LINKER_ENV_VAR}.
+ */
+export const DYNAMIC_LINKER_ENV_VARS = [
+  // Linux / glibc / musl
+  'LD_PRELOAD',
+  'LD_LIBRARY_PATH',
+  'LD_AUDIT',
+  // macOS dyld
+  'DYLD_INSERT_LIBRARIES',
+  'DYLD_LIBRARY_PATH',
+  'DYLD_FRAMEWORK_PATH',
+  'DYLD_FALLBACK_LIBRARY_PATH',
+  'DYLD_FALLBACK_FRAMEWORK_PATH',
+] as const;
+
+/**
+ * Opt-out environment variable. When set to exactly `"1"` in
+ * `process.env`, {@link sanitizeDynamicLinkerEnv} and
+ * {@link sanitizeSpawnEnv} become no-ops.
+ *
+ * Noisy by design: both helpers log whenever the opt-out is in effect,
+ * so the escape hatch is visible in CI logs.
+ */
+export const ALLOW_DYNAMIC_LINKER_ENV_VAR = 'CRAFT_ALLOW_DYNAMIC_LINKER_ENV';
+
+/**
+ * Strips dynamic-linker environment variables from `process.env` at
+ * startup, logging a warning per stripped key. Values are never logged.
+ */
+export function sanitizeDynamicLinkerEnv(): void {
+  const allowOverride = process.env[ALLOW_DYNAMIC_LINKER_ENV_VAR] === '1';
+  const presentKeys = DYNAMIC_LINKER_ENV_VARS.filter(
+    key => process.env[key] !== undefined,
+  );
+
+  if (presentKeys.length === 0) {
+    return;
+  }
+
+  if (allowOverride) {
+    logger.info(
+      `${ALLOW_DYNAMIC_LINKER_ENV_VAR}=1 set; preserving dynamic-linker environment variables: ${presentKeys.join(
+        ', ',
+      )}. This is not recommended.`,
+    );
+    return;
+  }
+
+  for (const key of presentKeys) {
+    logger.warn(
+      `Stripping dynamic-linker environment variable "${key}" for security reasons. ` +
+        `Set ${ALLOW_DYNAMIC_LINKER_ENV_VAR}=1 to override (not recommended).`,
+    );
+    delete process.env[key];
+  }
+}
+
+/**
+ * Returns a copy of `env` with dynamic-linker environment variables
+ * removed. If the global opt-out `CRAFT_ALLOW_DYNAMIC_LINKER_ENV=1` is
+ * set on `process.env`, the input is returned unchanged.
+ *
+ * Defence-in-depth for subprocess spawns: even if an earlier code path
+ * somehow restores one of these variables to `process.env` after
+ * startup, or a caller explicitly constructs an `env` object that
+ * contains one (e.g. via `{ ...process.env, ...custom }`), the spawned
+ * child still won't inherit it. Values are never logged.
+ *
+ * @param env Environment variables bag to sanitise. The input is
+ *   never mutated.
+ * @returns A shallow copy with dynamic-linker keys removed; or the
+ *   original reference when input is `undefined` or when the opt-out
+ *   is in effect.
+ */
+export function sanitizeSpawnEnv(
+  env: NodeJS.ProcessEnv | undefined,
+): NodeJS.ProcessEnv | undefined {
+  if (env === undefined) {
+    return env;
+  }
+  if (process.env[ALLOW_DYNAMIC_LINKER_ENV_VAR] === '1') {
+    return env;
+  }
+  const sanitized: NodeJS.ProcessEnv = { ...env };
+  let strippedAny = false;
+  for (const key of DYNAMIC_LINKER_ENV_VARS) {
+    if (sanitized[key] !== undefined) {
+      delete sanitized[key];
+      strippedAny = true;
+    }
+  }
+  if (strippedAny) {
+    logger.warn(
+      `Stripped dynamic-linker environment variable(s) from a subprocess env. ` +
+        `Set ${ALLOW_DYNAMIC_LINKER_ENV_VAR}=1 to override (not recommended).`,
+    );
+  }
+  return sanitized;
+}

src/utils/env.ts

@@ -72,70 +72,17 @@ function envHasVar(envVar: RequiredConfigVar): boolean {
   return true;
 }
 
-/**
- * Dynamic-linker environment variables that Craft refuses to propagate.
- *
- * Setting these allows arbitrary code to be loaded into every subprocess
- * spawned by Craft (`LD_PRELOAD` on Linux, `DYLD_*` on macOS). They are a
- * well-known supply-chain attack vector: an attacker who can influence
- * Craft's environment (e.g. via a dotfile, a previous build step, or a
- * misconfigured CI secret) can silently execute code with access to every
- * release credential Craft touches. We strip these at startup as
- * defence-in-depth — legitimate uses are extremely rare and can be
- * re-enabled per-invocation via `CRAFT_ALLOW_DYNAMIC_LINKER_ENV=1`.
- */
-const DYNAMIC_LINKER_ENV_VARS = [
-  // Linux / glibc / musl
-  'LD_PRELOAD',
-  'LD_LIBRARY_PATH',
-  'LD_AUDIT',
-  // macOS dyld
-  'DYLD_INSERT_LIBRARIES',
-  'DYLD_LIBRARY_PATH',
-  'DYLD_FRAMEWORK_PATH',
-  'DYLD_FALLBACK_LIBRARY_PATH',
-  'DYLD_FALLBACK_FRAMEWORK_PATH',
-] as const;
-
-/** Opt-out env var for {@link sanitizeDynamicLinkerEnv}. */
-const ALLOW_DYNAMIC_LINKER_ENV_VAR = 'CRAFT_ALLOW_DYNAMIC_LINKER_ENV';
-
-/**
- * Strips dynamic-linker environment variables (`LD_PRELOAD`, `LD_LIBRARY_PATH`,
- * `DYLD_*`, etc.) from `process.env` at startup, logging a warning for each
- * stripped key. Values are never logged.
- *
- * Users who legitimately require these variables (e.g. for an instrumented
- * build toolchain) can set `CRAFT_ALLOW_DYNAMIC_LINKER_ENV=1` to opt out;
- * this is noisy by design to make the escape hatch visible in CI logs.
- */
-export function sanitizeDynamicLinkerEnv(): void {
-  const allowOverride = process.env[ALLOW_DYNAMIC_LINKER_ENV_VAR] === '1';
-  const presentKeys = DYNAMIC_LINKER_ENV_VARS.filter(
-    key => process.env[key] !== undefined,
-  );
-
-  if (presentKeys.length === 0) {
-    return;
-  }
-
-  if (allowOverride) {
-    logger.info(
-      `${ALLOW_DYNAMIC_LINKER_ENV_VAR}=1 set; preserving dynamic-linker environment variables: ${presentKeys.join(
-        ', ',
-      )}. This is not recommended.`,
-    );
-    return;
-  }
-
-  for (const key of presentKeys) {
-    logger.warn(
-      `Stripping dynamic-linker environment variable "${key}" for security reasons. ` +
-        `Set ${ALLOW_DYNAMIC_LINKER_ENV_VAR}=1 to override (not recommended).`,
-    );
-    delete process.env[key];
-  }
-}
+// Re-exported from `./dynamicLinkerEnv` so that existing callers
+// (notably `src/index.ts`) keep working without edits. The logic now
+// lives in a leaf module with minimal imports to avoid circular deps
+// with `src/utils/system.ts` (see the file header in
+// `./dynamicLinkerEnv`).
+export {
+  ALLOW_DYNAMIC_LINKER_ENV_VAR,
+  DYNAMIC_LINKER_ENV_VARS,
+  sanitizeDynamicLinkerEnv,
+  sanitizeSpawnEnv,
+} from './dynamicLinkerEnv';
 
 /**
  * Warns the user if a legacy `.craft.env` file is present in the home

src/utils/system.ts

@@ -11,6 +11,7 @@ import { logger } from '../logger';
 import { reportError } from './errors';
 import { isDryRun } from './helpers';
 import { isInWorktreeMode } from './dryRun';
+import { sanitizeSpawnEnv } from './dynamicLinkerEnv';
 
 /**
  * Types of supported hashing algorithms
@@ -167,6 +168,15 @@ export async function spawnProcess(
         replaceEnvVariable(arg, { ...process.env, ...options.env }),
       );
 
+      // Defence-in-depth: strip dynamic-linker env vars (LD_PRELOAD,
+      // DYLD_*, etc.) from whatever env the child process will see.
+      // Startup already sanitises process.env, so the only way one of
+      // these keys reaches this point is via a later mutation (hostile
+      // or accidental) or via an explicit `options.env` that spreads
+      // in attacker-controlled values. Either way, we refuse to
+      // propagate them. See sanitizeSpawnEnv() in ../utils/env.ts.
+      options.env = sanitizeSpawnEnv(options.env ?? process.env);
+
       // Allow child to accept input (use 'pipe' for stdin if we need to write to it)
       options.stdio = [
         spawnProcessOptions.stdin !== undefined ? 'pipe' : 'inherit',