Fix process spawn

docs/DevelopmentGuide.md

@@ -185,7 +185,7 @@ end
 3. Now let's generate that dependency Gemfile with `rake`. Simply run
 
 > [!IMPORTANT]
-> Ensure you are either using Ruby 3.3 as the current Ruby version (`ruby -v`) or running commands within a Docker container.
+> Ensure you are either using the repo's current Ruby version from `.ruby-version` (`ruby -v`) or running commands within a Docker container.
 
 ```console
 $ bundle exec rake dependency:generate

lib/datadog/core/configuration/components.rb

@@ -136,7 +136,7 @@ def initialize(settings)
           self.class::PATCH_ONLY_ONCE.run do
             Utils::AtForkMonkeyPatch.apply!
             Utils::SpawnMonkeyPatch.apply!(
-              lineage_envs_provider: Core::Environment::Identity.method(:runtime_propagation_envs),
+              env_provider: Core::Environment::Identity.method(:runtime_propagation_envs),
             )
 
             # Register callback that calls Components.after_fork

lib/datadog/core/utils/spawn_monkey_patch.rb

@@ -3,37 +3,54 @@
 module Datadog
   module Core
     module Utils
+      # Applies the Process.spawn wrapper used to merge additional environment variables
+      # into child processes.
       module SpawnMonkeyPatch
-        # @param lineage_envs_provider [#call] returns a Hash of env vars to merge into the child process
-        def self.apply!(lineage_envs_provider:)
-          @lineage_envs_provider = lineage_envs_provider
+        # @param env_provider [#call] returns a Hash of env vars to merge into the child process
+        def self.apply!(env_provider:)
+          @env_provider = env_provider
+
+          # Idempotent: tests, reloads, or repeated Components init must not stack prepends.
+          return if ::Process.singleton_class.ancestors.include?(ProcessSpawnPatch)
+
           ::Process.singleton_class.prepend(ProcessSpawnPatch)
-          true
         end
 
+        # Prepends `Process.spawn` to merge `env_provider` output into the child's environment hash.
         module ProcessSpawnPatch
-          def spawn(*args, **opts)
-            args.replace(SpawnMonkeyPatch.inject_lineage_envs(args))
-            super
+          def spawn(*args)
+            super(*SpawnMonkeyPatch.inject_envs(args))
           end
         end
 
-        # Process.spawn(env?, cmd, ...): env is optional first arg (Hash). When present, merge
-        # runtime_ids into it; when absent, prepend full ENV + runtime_ids so the child inherits both.
+        # Merge the env vars from `env_provider` with the optional env `Hash` from {Process.spawn}.
+        #
+        # `env` is the first argument when it is a {Hash}; see MRI `spawn([env, ] *args, options)`:
+        # https://docs.ruby-lang.org/en/master/Process.html#method-c-spawn
+        #
+        # When there is **no** leading env Hash, MRI inherits the parent's `ENV`; we prepend only the
+        # `env_provider` hash so spawned children see parent env plus injections.
+        #
+        # When callers pass `unsetenv_others: true`, MRI only forwards the explicitly passed env Hash;
+        # replacing a missing hash with DATADOG_ENV.to_h would wrongly carry over parent variables.
+        # Prepending only the provider hash preserves `unsetenv_others` semantics.
+        #
+        # See https://docs.ruby-lang.org/en/master/Process.html#module-Process-label-Environment+Variables+-28-3Aunsetenv_others-29
         #
         # NOTE: `::Hash` (not bare `Hash`) is required because this module is nested under
         # `Datadog::Core::Utils`, and `Datadog::Core::Utils::Hash` exists as a refinement module.
         # Bare `Hash` resolves to that module via Module.nesting, making `Hash === some_hash`
         # silently return `false`. See https://github.com/DataDog/dd-trace-rb/issues/5621.
-        def self.inject_lineage_envs(args)
-          runtime_ids = @lineage_envs_provider.call
-          env_provided = ::Hash === args.first
+        def self.inject_envs(args)
+          provided_env = @env_provider.call
 
-          base_env = env_provided ? args.first : DATADOG_ENV.to_h
-          env = base_env.merge(runtime_ids)
-          rest = env_provided ? args.drop(1) : args
+          if ::Hash === args.first
+            args[0] = args.first.merge(provided_env)
+          else
+            args.unshift(provided_env)
+          end
 
-          [env, *rest]
+          args
         end
       end
     end

sig/datadog/core/utils/spawn_monkey_patch.rbs

@@ -3,15 +3,15 @@ module Datadog
     module Utils
       module SpawnMonkeyPatch
         # Set in apply! before Process.spawn is intercepted; internal wiring only.
-        self.@lineage_envs_provider: ^() -> ::Hash[::String, ::String]
+        self.@env_provider: ^() -> ::Hash[::String, ::String]
 
-        def self.apply!: (lineage_envs_provider: ^() -> ::Hash[::String, ::String]) -> true
+        def self.apply!: (env_provider: ^() -> ::Hash[::String, ::String]) -> void
 
         module ProcessSpawnPatch
-          def spawn: (*untyped args, **untyped opts) -> untyped
+          def spawn: (*untyped args) -> untyped
         end
 
-        def self.inject_lineage_envs: (untyped args) -> ::Array[untyped]
+        def self.inject_envs: (untyped args) -> ::Array[untyped]
       end
     end
   end

spec/datadog/core/utils/spawn_monkey_patch_spec.rb

@@ -5,8 +5,31 @@
 require 'datadog/core/configuration/settings'
 
 RSpec.describe Datadog::Core::Utils::SpawnMonkeyPatch do
+  let(:envs) do
+    {
+      'ENV1' => 'val1',
+      'ENV2' => 'val2',
+    }
+  end
+
+  def process_spawn(*spawn_args)
+    IO.pipe do |read_io, write_io|
+      process_options = {in: File::NULL, out: write_io, err: write_io}
+      process_options.merge!(spawn_args.pop) if ::Hash === spawn_args.last
+
+      pid = Process.spawn(*spawn_args, process_options)
+      write_io.close
+      Process.wait(pid)
+
+      Datadog::Core::Utils::Array.filter_map(read_io.read.lines) do |line|
+        parts = line.chomp.split('=', 2)
+        [parts[0], parts[1]] if parts.size == 2
+      end.to_h
+    end
+  end
+
   describe '::apply!' do
-    subject(:apply!) { described_class.apply!(lineage_envs_provider: -> { {} }) }
+    subject(:apply!) { described_class.apply!(env_provider: -> { envs }) }
 
     context 'when Process.spawn is supported' do
       before do
@@ -15,23 +38,106 @@
       end
 
       it 'prepends the spawn monkey patch' do
-        expect_in_fork do
-          apply!
-          expect(Process.singleton_class.ancestors).to include(described_class::ProcessSpawnPatch)
-          expect(Process.method(:spawn).source_location.first).to match(/spawn_monkey_patch\.rb/)
-        end
+        apply!
+        expect(Process.singleton_class.ancestors).to include(described_class::ProcessSpawnPatch)
+        expect(Process.method(:spawn).source_location.first).to match(/spawn_monkey_patch\.rb/)
+      end
+
+      it 'does not patch twice' do
+        described_class.apply!(env_provider: -> { {'ENV1' => 'val1'} })
+        described_class.apply!(env_provider: -> { {'ENV1' => 'val2'} })
+
+        expect(Process.singleton_class.ancestors.count(described_class::ProcessSpawnPatch)).to eq(1)
+        expect(process_spawn('/usr/bin/env')).to include('ENV1' => 'val2')
+      end
+    end
+  end
+
+  describe 'on Process.spawn' do
+    subject(:apply!) { described_class.apply!(env_provider: -> { envs }) }
+
+    around do |example|
+      ClimateControl.modify('PARENT1' => 'parent_val') { example.run }
+    end
+
+    before do
+      skip 'Process.spawn not supported' unless Process.respond_to?(:spawn)
+      apply!
+    end
+
+    it 'merges env_provider, parent envs, and env argument' do
+      output = process_spawn({'ARG' => 'arg_val'}, '/usr/bin/env', pgroup: true)
+
+      expect(output).to include('PARENT1' => 'parent_val', 'ENV1' => 'val1', 'ENV2' => 'val2', 'ARG' => 'arg_val')
+    end
+
+    it 'merges env_provider and parent envs when no env argument is provided' do
+      output = process_spawn('/usr/bin/env', pgroup: true)
+
+      expect(output).to include('PARENT1' => 'parent_val', 'ENV1' => 'val1', 'ENV2' => 'val2')
+    end
+
+    it 'respects parent env removal through the value `nil`' do
+      output = process_spawn({'PARENT1' => nil}, '/usr/bin/env')
+
+      expect(output).not_to include('PARENT1')
+      expect(output).to include('ENV1' => 'val1', 'ENV2' => 'val2')
+    end
+
+    it 'respects unsetenv_others and does not inherit parent ENV aside from injections' do
+      output = process_spawn('/usr/bin/env', unsetenv_others: true)
+
+      expect(output).to include('ENV1' => 'val1', 'ENV2' => 'val2')
+      expect(output).not_to include('PARENT1')
+      expect(output.keys).not_to include('')
+    end
+
+    it 'respects array-form command variant' do
+      command = 'printf %s "$0:$ARG:$PARENT1:$ENV1:$ENV2"'
+
+      output = IO.pipe do |read_io, write_io|
+        pid = Process.spawn(
+          {'ARG' => 'arg_val'},
+          ['/bin/sh', 'cmd-name'],
+          '-c',
+          command,
+          in: File::NULL,
+          out: write_io,
+          err: write_io,
+        )
+        write_io.close
+        Process.wait(pid)
+
+        read_io.read
       end
+
+      expect(output).to eq('cmd-name:arg_val:parent_val:val1:val2')
+    end
+  end
+
+  describe '::inject_envs' do
+    subject(:inject_envs) { described_class.inject_envs(args.dup) }
+    let(:args) { [env_argument, '/bin/ls', '.', {pgroup: 0}] }
+    let(:env_argument) { {'TZ' => 'UTC'} }
+
+    before do
+      described_class.apply!(env_provider: -> { envs })
+    end
+
+    it 'does not mutate the provided env argument Hash' do
+      expect { inject_envs }.not_to change { env_argument }
     end
   end
 
   # Regression coverage for https://github.com/DataDog/dd-trace-rb/issues/5621.
   #
-  # The wrapper's env-detection check uses bare `Hash`, which resolves to
-  # `Datadog::Core::Utils::Hash` (a refinement module) via Module.nesting
-  # once that file is loaded — silently returning `false` for real Hashes.
-  # The function then takes the "no env provided" branch and prepends
-  # `DATADOG_ENV.to_h`, pushing the caller's env-Hash into the command slot
-  # and producing `TypeError: no implicit conversion of Hash into String`.
+  # When the env-detection check used bare `Hash`, it resolved to
+  # `Datadog::Core::Utils::Hash` (a refinement module) via Module.nesting,
+  # so `Hash === real_env_hash` silently returned false. `#inject_envs` then took
+  # the wrong branch and broke callers that pass an env `{Hash}` first
+  # (TypeError: no implicit conversion of Hash into String).
+  #
+  # Implementation uses `::Hash === args.first` and forwards with `super(*args)`.
   #
   # Affected callers (named in the issue): childprocess, terrapin, launchy,
   # selenium-webdriver, cuprite/ferrum, danger.
@@ -64,7 +170,7 @@ def run_spawn(*spawn_args)
 
     it 'spawn(cmd_string) — no env, no options' do
       expect_in_fork do
-        described_class.apply!(lineage_envs_provider: -> { {lineage_var => lineage_val} })
+        described_class.apply!(env_provider: -> { {lineage_var => lineage_val} })
         ok, status, out = run_spawn(probe_cmd)
         expect(ok).to be(true)
         expect(status).to eq(0)
@@ -74,7 +180,7 @@ def run_spawn(*spawn_args)
 
     it 'spawn(cmd, kw: ...) — kwargs option syntax' do
       expect_in_fork do
-        described_class.apply!(lineage_envs_provider: -> { {lineage_var => lineage_val} })
+        described_class.apply!(env_provider: -> { {lineage_var => lineage_val} })
         ok, status, out = run_spawn(probe_cmd, pgroup: true)
         expect(ok).to be(true)
         expect(status).to eq(0)
@@ -84,7 +190,7 @@ def run_spawn(*spawn_args)
 
     it 'spawn(cmd, options_hash) — positional options hash variable' do
       expect_in_fork do
-        described_class.apply!(lineage_envs_provider: -> { {lineage_var => lineage_val} })
+        described_class.apply!(env_provider: -> { {lineage_var => lineage_val} })
         options = {pgroup: true}
         ok, status, out = run_spawn(probe_cmd, options)
         expect(ok).to be(true)
@@ -95,7 +201,7 @@ def run_spawn(*spawn_args)
 
     it 'spawn(env_hash, cmd)' do
       expect_in_fork do
-        described_class.apply!(lineage_envs_provider: -> { {lineage_var => lineage_val} })
+        described_class.apply!(env_provider: -> { {lineage_var => lineage_val} })
         ok, status, out = run_spawn({'EXTRA' => '1'}, probe_cmd)
         expect(ok).to be(true)
         expect(status).to eq(0)
@@ -106,7 +212,7 @@ def run_spawn(*spawn_args)
     # Terrapin: `Process.spawn(env, command, options.merge(pipe.pipe_options))`
     it 'spawn(env_hash, cmd, options_hash) — terrapin shape' do
       expect_in_fork do
-        described_class.apply!(lineage_envs_provider: -> { {lineage_var => lineage_val} })
+        described_class.apply!(env_provider: -> { {lineage_var => lineage_val} })
         options = {pgroup: true}
         ok, status, out = run_spawn({'EXTRA' => '1'}, probe_cmd, options)
         expect(ok).to be(true)
@@ -118,7 +224,7 @@ def run_spawn(*spawn_args)
     # ChildProcess multi-arg: `::Process.spawn(environment, *args, options)`
     it 'spawn(env_hash, *args, options_hash) — childprocess multi-arg shape' do
       expect_in_fork do
-        described_class.apply!(lineage_envs_provider: -> { {lineage_var => lineage_val} })
+        described_class.apply!(env_provider: -> { {lineage_var => lineage_val} })
         env = {}
         args = ['/bin/sh', '-c', probe_cmd]
         options = {pgroup: true}
@@ -133,7 +239,7 @@ def run_spawn(*spawn_args)
     # `Process.spawn(env, [cmd, argv0], options)`.
     it 'spawn(env_hash, [cmdname, argv0], options_hash) — childprocess single-arg shape' do
       expect_in_fork do
-        described_class.apply!(lineage_envs_provider: -> { {lineage_var => lineage_val} })
+        described_class.apply!(env_provider: -> { {lineage_var => lineage_val} })
         options = {pgroup: true}
         ok, status, out = run_spawn({}, ['/bin/sh', 'argv0-name'], '-c', probe_cmd, options)
         expect(ok).to be(true)
@@ -150,7 +256,7 @@ def run_spawn(*spawn_args)
 
     it 'spawn(env, cmd, **opts) — caller uses kwargs splat' do
       expect_in_fork do
-        described_class.apply!(lineage_envs_provider: -> { {lineage_var => lineage_val} })
+        described_class.apply!(env_provider: -> { {lineage_var => lineage_val} })
         opts = {pgroup: true}
         ok, status, out = run_spawn({'EXTRA' => '1'}, probe_cmd, **opts)
         expect(ok).to be(true)
@@ -161,7 +267,7 @@ def run_spawn(*spawn_args)
 
     it 'parent process ENV reaches the child when caller passes an env hash' do
       expect_in_fork do
-        described_class.apply!(lineage_envs_provider: -> { {lineage_var => lineage_val} })
+        described_class.apply!(env_provider: -> { {lineage_var => lineage_val} })
         ENV['PARENT_ONLY_VAR'] = 'parent-only-value'
         cmd = %(printf 'PARENT=%s\n' "$PARENT_ONLY_VAR")
         ok, status, out = run_spawn({'EXTRA' => '1'}, cmd)
@@ -173,7 +279,7 @@ def run_spawn(*spawn_args)
 
     it 'does not mutate the env Hash supplied by the caller' do
       expect_in_fork do
-        described_class.apply!(lineage_envs_provider: -> { {lineage_var => lineage_val} })
+        described_class.apply!(env_provider: -> { {lineage_var => lineage_val} })
         caller_env = {'CALLER_KEY' => 'caller-value'}
         before_keys = caller_env.keys.dup
         run_spawn(caller_env, probe_cmd)
@@ -192,16 +298,14 @@ def run_spawn(*spawn_args)
     end
 
     it 'applies both fork and spawn patches when Components is initialized' do
-      expect_in_fork do
-        Datadog::Core::Configuration::Components.new(Datadog::Core::Configuration::Settings.new)
+      Datadog::Core::Configuration::Components.new(Datadog::Core::Configuration::Settings.new)
 
-        expect(Process.singleton_class.ancestors).to include(
-          Datadog::Core::Utils::AtForkMonkeyPatch::ProcessMonkeyPatch,
-        )
-        expect(Process.singleton_class.ancestors).to include(
-          Datadog::Core::Utils::SpawnMonkeyPatch::ProcessSpawnPatch,
-        )
-      end
+      expect(Process.singleton_class.ancestors).to include(
+        Datadog::Core::Utils::AtForkMonkeyPatch::ProcessMonkeyPatch,
+      )
+      expect(Process.singleton_class.ancestors).to include(
+        Datadog::Core::Utils::SpawnMonkeyPatch::ProcessSpawnPatch,
+      )
     end
   end
 end

spec/support/synchronization_helpers.rb

@@ -1,6 +1,11 @@
 require 'English'
 
 module SynchronizationHelpers
+  # Runs the given block in a fork, allowing you to perform RSpec assertions in a fork
+  # and have them be reported in the parent process.
+  #
+  # You can alternatively use `execute_in_fork: true` {ForkableExample}
+  # if your whole example or example group should run in a forked process.
   def expect_in_fork(fork_expectations: nil, timeout_seconds: 10, trigger_stacktrace_on_kill: false, debug: false)
     fork_expectations ||= proc { |status:, stdout:, stderr:|
       expect(status && status.success?).to be(true),