From 468e4214093a57cf6a85a1a745419783fe96fb17 Mon Sep 17 00:00:00 2001 From: "William T. Nelson" <35801+wtn@users.noreply.github.com> Date: Mon, 26 Oct 2026 19:19:59 -0500 Subject: [PATCH] fix(pluggable): enforce executable timeout and kill the helper on expiry PluggableAuthCredentials passed the configured timeout_millis value straight to Timeout.timeout, which takes seconds. The default 30000 ms timeout therefore allowed the helper executable to run for over eight hours before being interrupted, and the documented 5-120 second bounds were never enforced at run time. Correcting the units alone was not enough. Open3.capture3 joins the child in an ensure block, so Timeout::Error only surfaced once the helper exited on its own, and each expiry printed reader-thread backtraces to stderr. Run the helper with Open3.popen3 in its own process group and apply a single deadline to the process wait and to reading stdout and stderr, so a background child that inherits the pipes cannot extend the wait. Raise CredentialsError when the deadline passes, matching the documented contract of retrieve_subject_token!, and send SIGKILL to the group from an ensure whenever the block exits before the helper has been reaped. That covers the deadline and also an interrupted caller, which would otherwise wait on a helper that no longer shares its terminal's process group. On Windows only the direct process is killed. Specs cover the millisecond-to-second conversion, a real process that outlives a sub-second deadline, a background child holding stdout open after the executable exits, and an Interrupt raised in the caller before the deadline. Co-Authored-By: Claude Mythos 5.1 --- .../external_account/pluggable_credentials.rb | 42 +++++++++-- .../pluggable_credentials_spec.rb | 70 +++++++++++++++++-- 2 files changed, 103 insertions(+), 9 deletions(-) diff --git a/lib/googleauth/external_account/pluggable_credentials.rb b/lib/googleauth/external_account/pluggable_credentials.rb index 82db663a..17441e69 100644 --- a/lib/googleauth/external_account/pluggable_credentials.rb +++ b/lib/googleauth/external_account/pluggable_credentials.rb @@ -88,7 +88,7 @@ def retrieve_subject_token! # environment variable injection env = inject_environment_variables output = subprocess_with_timeout env, @credential_source_executable_command, - @credential_source_executable_timeout_millis + @credential_source_executable_timeout_millis / 1000.0 response = JSON.parse output, symbolize_names: true parse_subject_token response end @@ -157,15 +157,49 @@ def inject_environment_variables end def subprocess_with_timeout environment_vars, command, timeout_seconds - Timeout.timeout timeout_seconds do - output, error, status = Open3.capture3 environment_vars, command + Open3.popen3 environment_vars, command, **spawn_options do |stdin, stdout, stderr, wait_thr| + finished = false + stdin.close + deadline = Process.clock_gettime(Process::CLOCK_MONOTONIC) + timeout_seconds + out_reader = reader_thread stdout + err_reader = reader_thread stderr + unless [wait_thr, out_reader, err_reader].all? { |thread| join_until thread, deadline } + raise CredentialsError, "Executable timed out after #{timeout_seconds} seconds." + end + status = wait_thr.value + output = out_reader.value + finished = true unless status.success? raise CredentialsError, - "Executable exited with non-zero return code #{status.exitstatus}. Error: #{output}, #{error}" + "Executable exited with non-zero return code #{status.exitstatus}. " \ + "Error: #{output}, #{err_reader.value}" end output + ensure + kill_executable wait_thr.pid unless finished end end + + def join_until thread, deadline + remaining = deadline - Process.clock_gettime(Process::CLOCK_MONOTONIC) + thread.join [remaining, 0].max + end + + def reader_thread io + thread = Thread.new { io.read } + thread.report_on_exception = false + thread + end + + def spawn_options + Gem.win_platform? ? {} : { pgroup: true } + end + + def kill_executable pid + Process.kill "KILL", Gem.win_platform? ? pid : -pid + rescue Errno::ESRCH, Errno::EPERM + nil + end end end end diff --git a/spec/googleauth/external_account/pluggable_credentials_spec.rb b/spec/googleauth/external_account/pluggable_credentials_spec.rb index 3c242558..e25a94c1 100644 --- a/spec/googleauth/external_account/pluggable_credentials_spec.rb +++ b/spec/googleauth/external_account/pluggable_credentials_spec.rb @@ -42,6 +42,16 @@ describe Google::Auth::ExternalAccount::PluggableAuthCredentials do PluggableAuthCredentials = Google::Auth::ExternalAccount::PluggableAuthCredentials + + def stub_executable stdout, stderr, exit_status, command: anything + status = double("Process::Status", exitstatus: exit_status, success?: exit_status == 0) + wait_thr = double("wait_thr", pid: 12345, value: status) + allow(wait_thr).to receive(:join).and_return(wait_thr) + allow(Open3).to receive(:popen3).with(anything, command, any_args) do |*_args, &block| + block.call StringIO.new, StringIO.new(stdout), StringIO.new(stderr), wait_thr + end + end + CREDENTIAL_SOURCE_EXECUTABLE_COMMAND = "/fake/external/excutable --arg1=value1 --arg2=value2" CREDENTIAL_SOURCE_EXECUTABLE_OUTPUT_FILE = "fake_output_file" CREDENTIAL_SOURCE_EXECUTABLE = { @@ -287,7 +297,7 @@ } before :example do ENV[Google::Auth::ExternalAccount::PluggableAuthCredentials::ENABLE_PLUGGABLE_ENV] = "1" - allow(Open3).to receive(:capture3).and_raise(StandardError, "mock subprocess error") + allow(Open3).to receive(:popen3).and_raise(StandardError, "mock subprocess error") end it "executable subprocess error" do credentials = PluggableAuthCredentials.new options @@ -295,6 +305,58 @@ end end + describe "test retrieve subject token executable timeout" do + options = { + :audience => AUDIENCE, + :subject_token_type => SUBJECT_TOKEN_TYPE, + :token_url => TOKEN_URL, + :credential_source => { + :executable => { + :command => "sleep 30", + :timeout_millis => 5000, + }, + }, + } + before :example do + ENV[Google::Auth::ExternalAccount::PluggableAuthCredentials::ENABLE_PLUGGABLE_ENV] = "1" + end + it "converts timeout_millis to seconds" do + stub_executable JSON.generate(EXECUTABLE_SUCCESSFUL_OIDC_RESPONSE_ID_TOKEN), "", 0 + credentials = PluggableAuthCredentials.new options + expect(credentials).to receive(:subprocess_with_timeout).with(anything, "sleep 30", 5.0).and_call_original + expect(credentials.retrieve_subject_token!).to eql(EXECUTABLE_OIDC_TOKEN) + end + it "kills a real executable that exceeds the timeout" do + credentials = PluggableAuthCredentials.new options + cmd = %("#{RbConfig.ruby}" -e "sleep 5") + started = Process.clock_gettime Process::CLOCK_MONOTONIC + expect { credentials.send :subprocess_with_timeout, {}, cmd, 0.3 } + .to raise_error(Google::Auth::CredentialsError, /timed out after 0.3 seconds/) + expect(Process.clock_gettime(Process::CLOCK_MONOTONIC) - started).to be < 2 + end + it "kills the executable when the caller is interrupted before the deadline" do + credentials = PluggableAuthCredentials.new options + cmd = %("#{RbConfig.ruby}" -e "sleep 5") + caller_thread = Thread.current + Thread.new do + sleep 0.3 + caller_thread.raise Interrupt + end + started = Process.clock_gettime Process::CLOCK_MONOTONIC + expect { credentials.send :subprocess_with_timeout, {}, cmd, 30 }.to raise_error(Interrupt) + expect(Process.clock_gettime(Process::CLOCK_MONOTONIC) - started).to be < 2 + end + it "times out when a background child keeps stdout open after the executable exits" do + skip "relies on POSIX shell job control" if Gem.win_platform? + credentials = PluggableAuthCredentials.new options + cmd = %(sh -c '"#{RbConfig.ruby}" -e "sleep 5" & echo hi') + started = Process.clock_gettime Process::CLOCK_MONOTONIC + expect { credentials.send :subprocess_with_timeout, {}, cmd, 0.3 } + .to raise_error(Google::Auth::CredentialsError, /timed out after 0.3 seconds/) + expect(Process.clock_gettime(Process::CLOCK_MONOTONIC) - started).to be < 2 + end + end + describe "test retrieve subject token executable works but response error" do examples = [ { @@ -447,8 +509,7 @@ ENV[Google::Auth::ExternalAccount::PluggableAuthCredentials::ENABLE_PLUGGABLE_ENV] = "1" cmd = example[:options][:credential_source][:executable][:command] e = example[:executable_return] - exe_return = [e[:stdout], e[:stdout], double("Process::Status", exitstatus: e[:exit_status], success?: (e[:exit_status] == 0))] - allow(Open3).to receive(:capture3).with(anything, cmd).and_return(exe_return) + stub_executable e[:stdout], e[:stderr], e[:exit_status], command: cmd end it example[:name] do credentials = PluggableAuthCredentials.new example[:options] @@ -522,8 +583,7 @@ ENV[Google::Auth::ExternalAccount::PluggableAuthCredentials::ENABLE_PLUGGABLE_ENV] = "1" cmd = example[:options][:credential_source][:executable][:command] e = example[:executable_return] - exe_return = [e[:stdout], e[:stdout], double("Process::Status", exitstatus: e[:exit_status], success?: (e[:exit_status] == 0))] - allow(Open3).to receive(:capture3).with(anything, cmd).and_return(exe_return) + stub_executable e[:stdout], e[:stderr], e[:exit_status], command: cmd end it example[:name] do credentials = PluggableAuthCredentials.new example[:options]