diff --git a/common/lib/dependabot/command_helpers.rb b/common/lib/dependabot/command_helpers.rb new file mode 100644 index 0000000000..f10aeff67b --- /dev/null +++ b/common/lib/dependabot/command_helpers.rb @@ -0,0 +1,226 @@ +# typed: strict +# frozen_string_literal: true + +require "open3" +require "timeout" +require "sorbet-runtime" +require "shellwords" + +module Dependabot + module CommandHelpers + extend T::Sig + + module TIMEOUTS + NO_TIME_OUT = -1 + LOCAL = 30 + NETWORK = 120 + LONG_RUNNING = 300 + DEFAULT = NETWORK + end + + class ProcessStatus + extend T::Sig + + sig { params(process_status: Process::Status, custom_exitstatus: T.nilable(Integer)).void } + def initialize(process_status, custom_exitstatus = nil) + @process_status = process_status + @custom_exitstatus = custom_exitstatus + end + + # Return the exit status, either from the process status or the custom one + sig { returns(Integer) } + def exitstatus + @custom_exitstatus || @process_status.exitstatus || 0 + end + + # Determine if the process was successful + sig { returns(T::Boolean) } + def success? + @custom_exitstatus.nil? ? @process_status.success? || false : @custom_exitstatus.zero? + end + + # Return the PID of the process (if available) + sig { returns(T.nilable(Integer)) } + def pid + @process_status.pid + end + + sig { returns(T.nilable(Integer)) } + def termsig + @process_status.termsig + end + + # String representation of the status + sig { returns(String) } + def to_s + if @custom_exitstatus + "pid #{pid || 'unknown'}: exit #{@custom_exitstatus} (custom status)" + else + @process_status.to_s + end + end + end + + # rubocop:disable Metrics/AbcSize + # rubocop:disable Metrics/MethodLength + # rubocop:disable Metrics/PerceivedComplexity + # rubocop:disable Metrics/CyclomaticComplexity + sig do + params( + env_cmd: T::Array[T.any(T::Hash[String, String], String)], + stdin_data: T.nilable(String), + stderr_to_stdout: T::Boolean, + timeout: Integer + ).returns([T.nilable(String), T.nilable(String), T.nilable(ProcessStatus), Float]) + end + def self.capture3_with_timeout( + env_cmd, + stdin_data: nil, + stderr_to_stdout: false, + timeout: TIMEOUTS::DEFAULT + ) + + stdout = T.let("", String) + stderr = T.let("", String) + status = T.let(nil, T.nilable(ProcessStatus)) + pid = T.let(nil, T.untyped) + start_time = Time.now + + begin + T.unsafe(Open3).popen3(*env_cmd) do |stdin, stdout_io, stderr_io, wait_thr| # rubocop:disable Metrics/BlockLength + pid = wait_thr.pid + Dependabot.logger.info("Started process PID: #{pid} with command: #{env_cmd.join(' ')}") + + # Write to stdin if input data is provided + stdin&.write(stdin_data) if stdin_data + stdin&.close + + stdout_io.sync = true + stderr_io.sync = true + + # Array to monitor both stdout and stderr + ios = [stdout_io, stderr_io] + + last_output_time = Time.now # Track the last time output was received + + until ios.empty? + if timeout.positive? + # Calculate remaining timeout dynamically + remaining_timeout = timeout - (Time.now - last_output_time) + + # Raise an error if timeout is exceeded + if remaining_timeout <= 0 + Dependabot.logger.warn("Process PID: #{pid} timed out after #{timeout}s. Terminating...") + terminate_process(pid) + status = ProcessStatus.new(wait_thr.value, 124) + raise Timeout::Error, "Timed out due to inactivity after #{timeout} seconds" + end + end + + # Use IO.select with a dynamically calculated short timeout + ready_ios = IO.select(ios, nil, nil, 0) + + # Process ready IO streams + ready_ios&.first&.each do |io| + # 1. Read data from the stream + io.set_encoding("BINARY") + data = io.read_nonblock(1024) + + # 2. Force encoding to UTF-8 (for proper conversion) + data.force_encoding("UTF-8") + + # 3. Convert to UTF-8 safely, handling invalid/undefined bytes + data = data.encode("UTF-8", invalid: :replace, undef: :replace, replace: "?") + + # Reset the timeout if data is received + last_output_time = Time.now unless data.empty? + + # 4. Append data to the appropriate stream + if io == stdout_io + stdout += data + else + stderr += data unless stderr_to_stdout + stdout += data if stderr_to_stdout + end + rescue EOFError + # Remove the stream when EOF is reached + ios.delete(io) + rescue IO::WaitReadable + # Continue when IO is not ready yet + next + end + end + + status = ProcessStatus.new(wait_thr.value) + Dependabot.logger.info("Process PID: #{pid} completed with status: #{status}") + end + rescue Timeout::Error => e + Dependabot.logger.error("Process PID: #{pid} failed due to timeout: #{e.message}") + terminate_process(pid) + + # Append timeout message only to stderr without interfering with stdout + stderr += "\n#{e.message}" unless stderr_to_stdout + stdout += "\n#{e.message}" if stderr_to_stdout + rescue Errno::ENOENT => e + Dependabot.logger.error("Command failed: #{e.message}") + stderr += e.message unless stderr_to_stdout + stdout += e.message if stderr_to_stdout + end + + elapsed_time = Time.now - start_time + Dependabot.logger.info("Total execution time: #{elapsed_time.round(2)} seconds") + [stdout, stderr, status, elapsed_time] + end + # rubocop:enable Metrics/AbcSize + # rubocop:enable Metrics/MethodLength + # rubocop:enable Metrics/PerceivedComplexity + # rubocop:enable Metrics/CyclomaticComplexity + + # Terminate a process by PID + sig { params(pid: T.nilable(Integer)).void } + def self.terminate_process(pid) + return unless pid + + begin + if process_alive?(pid) + Process.kill("TERM", pid) # Attempt graceful termination + sleep(0.5) # Allow process to terminate + end + if process_alive?(pid) + Process.kill("KILL", pid) # Forcefully kill if still running + end + rescue Errno::EPERM + Dependabot.logger.error("Insufficient permissions to terminate process: #{pid}") + ensure + begin + Process.waitpid(pid) + rescue Errno::ESRCH, Errno::ECHILD + # Process has already exited + end + end + end + + # Check if the process is still alive + sig { params(pid: T.nilable(Integer)).returns(T::Boolean) } + def self.process_alive?(pid) + return false if pid.nil? + + begin + Process.kill(0, pid) # Check if the process exists + true + rescue Errno::ESRCH + false + rescue Errno::EPERM + Dependabot.logger.error("Insufficient permissions to check process: #{pid}") + false + end + end + + # Escape shell commands to ensure safe execution + sig { params(command: String).returns(String) } + def self.escape_command(command) + command_parts = command.split.map(&:strip).reject(&:empty?) + Shellwords.join(command_parts) + end + end +end diff --git a/common/lib/dependabot/shared_helpers.rb b/common/lib/dependabot/shared_helpers.rb index 4ef7a3c201..650c913ae8 100644 --- a/common/lib/dependabot/shared_helpers.rb +++ b/common/lib/dependabot/shared_helpers.rb @@ -7,7 +7,6 @@ require "fileutils" require "json" require "open3" -require "shellwords" require "sorbet-runtime" require "tmpdir" @@ -17,9 +16,10 @@ require "dependabot/errors" require "dependabot/workspace" require "dependabot" +require "dependabot/command_helpers" module Dependabot - module SharedHelpers + module SharedHelpers # rubocop:disable Metrics/ModuleLength extend T::Sig GIT_CONFIG_GLOBAL_PATH = T.let(File.expand_path(".gitconfig", Utils::BUMP_TMP_DIR_PATH), String) @@ -121,8 +121,7 @@ def sentry_context # Escapes all special characters, e.g. = & | <> sig { params(command: String).returns(String) } def self.escape_command(command) - command_parts = command.split.map(&:strip).reject(&:empty?) - Shellwords.join(command_parts) + CommandHelpers.escape_command(command) end # rubocop:disable Metrics/MethodLength @@ -135,14 +134,16 @@ def self.escape_command(command) env: T.nilable(T::Hash[String, String]), stderr_to_stdout: T::Boolean, allow_unsafe_shell_command: T::Boolean, - error_class: T.class_of(HelperSubprocessFailed) + error_class: T.class_of(HelperSubprocessFailed), + timeout: Integer ) .returns(T.nilable(T.any(String, T::Hash[String, T.untyped], T::Array[T::Hash[String, T.untyped]]))) end def self.run_helper_subprocess(command:, function:, args:, env: nil, stderr_to_stdout: false, allow_unsafe_shell_command: false, - error_class: HelperSubprocessFailed) + error_class: HelperSubprocessFailed, + timeout: CommandHelpers::TIMEOUTS::DEFAULT) start = Time.now stdin_data = JSON.dump(function: function, args: args) cmd = allow_unsafe_shell_command ? command : escape_command(command) @@ -157,7 +158,15 @@ def self.run_helper_subprocess(command:, function:, args:, env: nil, end env_cmd = [env, cmd].compact - stdout, stderr, process = T.unsafe(Open3).capture3(*env_cmd, stdin_data: stdin_data) + if Experiments.enabled?(:enable_shared_helpers_command_timeout) + stdout, stderr, process = CommandHelpers.capture3_with_timeout( + env_cmd, + stdin_data: stdin_data, + timeout: timeout + ) + else + stdout, stderr, process = T.unsafe(Open3).capture3(*env_cmd, stdin_data: stdin_data) + end time_taken = Time.now - start if ENV["DEBUG_HELPERS"] == "true" @@ -177,16 +186,16 @@ def self.run_helper_subprocess(command:, function:, args:, env: nil, function: function, args: args, time_taken: time_taken, - stderr_output: stderr ? stderr[0..50_000] : "", # Truncate to ~100kb + stderr_output: stderr[0..50_000], # Truncate to ~100kb process_exit_value: process.to_s, - process_termsig: process.termsig + process_termsig: process&.termsig } check_out_of_memory_error(stderr, error_context, error_class) begin response = JSON.parse(stdout) - return response["result"] if process.success? + return response["result"] if process&.success? raise error_class.new( message: response["error"], @@ -415,6 +424,7 @@ def self.find_safe_directories safe_directories end + # rubocop:disable Metrics/PerceivedComplexity sig do params( command: String, @@ -422,7 +432,8 @@ def self.find_safe_directories cwd: T.nilable(String), env: T.nilable(T::Hash[String, String]), fingerprint: T.nilable(String), - stderr_to_stdout: T::Boolean + stderr_to_stdout: T::Boolean, + timeout: Integer ).returns(String) end def self.run_shell_command(command, @@ -430,7 +441,8 @@ def self.run_shell_command(command, cwd: nil, env: {}, fingerprint: nil, - stderr_to_stdout: true) + stderr_to_stdout: true, + timeout: CommandHelpers::TIMEOUTS::DEFAULT) start = Time.now cmd = allow_unsafe_shell_command ? command : escape_command(command) @@ -439,7 +451,14 @@ def self.run_shell_command(command, opts = {} opts[:chdir] = cwd if cwd - if stderr_to_stdout + env_cmd = [env || {}, cmd, opts].compact + if Experiments.enabled?(:enable_shared_helpers_command_timeout) + stdout, stderr, process, _elapsed_time = CommandHelpers.capture3_with_timeout( + env_cmd, + stderr_to_stdout: stderr_to_stdout, + timeout: timeout + ) + elsif stderr_to_stdout stdout, process = Open3.capture2e(env || {}, cmd, opts) else stdout, stderr, process = Open3.capture3(env || {}, cmd, opts) @@ -449,7 +468,7 @@ def self.run_shell_command(command, # Raise an error with the output from the shell session if the # command returns a non-zero status - return stdout if process.success? + return stdout || "" if process&.success? error_context = { command: cmd, @@ -461,10 +480,11 @@ def self.run_shell_command(command, check_out_of_disk_memory_error(stderr, error_context) raise SharedHelpers::HelperSubprocessFailed.new( - message: stderr_to_stdout ? stdout : "#{stderr}\n#{stdout}", + message: stderr_to_stdout ? (stdout || "") : "#{stderr}\n#{stdout}", error_context: error_context ) end + # rubocop:enable Metrics/PerceivedComplexity sig { params(stderr: T.nilable(String), error_context: T::Hash[Symbol, String]).void } def self.check_out_of_disk_memory_error(stderr, error_context) diff --git a/common/spec/dependabot/command_helpers_spec.rb b/common/spec/dependabot/command_helpers_spec.rb new file mode 100644 index 0000000000..8130838abf --- /dev/null +++ b/common/spec/dependabot/command_helpers_spec.rb @@ -0,0 +1,102 @@ +# typed: false +# frozen_string_literal: true + +require "spec_helper" +require "dependabot/command_helpers" + +RSpec.describe Dependabot::CommandHelpers do + describe ".capture3_with_timeout" do + let(:success_cmd) { command_fixture("success.sh") } + let(:error_cmd) { command_fixture("error.sh") } + let(:output_hang_cmd) { command_fixture("output_hang.sh") } + let(:error_hang_cmd) { command_fixture("error_hang.sh") } + let(:invalid_cmd) { "non_existent_command" } + let(:no_timeout_cmd) { command_fixture("no_timeout.sh") } + let(:timeout) { 2 } # Timeout for hanging commands + + context "when the command runs successfully" do + it "captures stdout and exits successfully" do + stdout, stderr, status, elapsed_time = described_class.capture3_with_timeout( + [success_cmd], + timeout: timeout + ) + + expect(stdout).to eq("This is a successful command.\n") + expect(stderr).to eq("") + expect(status.exitstatus).to eq(0) + expect(elapsed_time).to be > 0 + end + end + + context "when the command runs with an error" do + it "captures stderr and returns an error status" do + stdout, stderr, status, elapsed_time = described_class.capture3_with_timeout( + [error_cmd], + timeout: timeout + ) + + expect(stdout).to eq("") + expect(stderr).to eq("This is an error message.\n") + expect(status.exitstatus).to eq(1) + expect(elapsed_time).to be > 0 + end + end + + context "when the command runs with output but hangs" do + it "times out and appends a timeout message to stderr" do + stdout, stderr, status, elapsed_time = described_class.capture3_with_timeout( + [output_hang_cmd], + timeout: timeout + ) + + expect(stdout).to eq("This is a hanging command.\n") + expect(stderr).to include("Timed out due to inactivity after #{timeout} seconds") + expect(status.exitstatus).to eq(124) # Timeout-specific status code + expect(elapsed_time).to be_within(1).of(timeout) + end + end + + context "when the command runs with an error but hangs" do + it "times out and appends a timeout message to stderr" do + stdout, stderr, status, elapsed_time = described_class.capture3_with_timeout( + [error_hang_cmd], + timeout: timeout + ) + + expect(stdout).to eq("This is a output for command error command.\n") + expect(stderr).to include("This is an error message.") + expect(stderr).to include("Timed out due to inactivity after #{timeout} seconds") + expect(status.exitstatus).to eq(124) + expect(elapsed_time).to be_within(1).of(timeout) + end + end + + context "when the command is invalid" do + it "raises an error and captures stderr" do + stdout, stderr, status, elapsed_time = described_class.capture3_with_timeout( + [invalid_cmd], + timeout: timeout + ) + + expect(stdout).to eq("") + expect(stderr).to include("No such file or directory - non_existent_command") if stderr + expect(status).to be_nil + expect(elapsed_time).to be > 0 + end + end + + context "when timeout is zero or negative" do + it "waiting commands to be done" do + stdout, stderr, status, elapsed_time = described_class.capture3_with_timeout( + [no_timeout_cmd], + timeout: -1 + ) + + expect(stdout).to eq("This is a command result.\n") + expect(stderr).to eql("") + expect(status.exitstatus).to eq(0) + expect(elapsed_time).to be_within(0.5).of(3) + end + end + end +end diff --git a/common/spec/dependabot/shared_helpers_spec.rb b/common/spec/dependabot/shared_helpers_spec.rb index c45fc3fba6..132b8db88d 100644 --- a/common/spec/dependabot/shared_helpers_spec.rb +++ b/common/spec/dependabot/shared_helpers_spec.rb @@ -16,6 +16,18 @@ class EcoSystemHelperSubprocessFailed < Dependabot::SharedHelpers::HelperSubproc let(:spec_root) { File.join(File.dirname(__FILE__), "..") } let(:tmp) { Dependabot::Utils::BUMP_TMP_DIR_PATH } + let(:enable_shared_helpers_command_timeout) { false } + + before do + allow(Dependabot::Experiments).to receive(:enabled?) + .with(:enable_shared_helpers_command_timeout) + .and_return(enable_shared_helpers_command_timeout) + end + + after do + Dependabot::Experiments.reset! + end + describe ".in_a_temporary_directory" do def existing_tmp_folders Dir.glob(File.join(tmp, "*")) @@ -149,84 +161,173 @@ def existing_tmp_folders let(:stderr_to_stdout) { false } let(:error_class) { Dependabot::SharedHelpers::HelperSubprocessFailed } - context "when the subprocess is successful" do - it "returns the result" do - expect(run_subprocess).to eq("function" => function, "args" => args) + context "when enable_shared_helpers_command_timeout is disabled" do + let(:enable_shared_helpers_command_timeout) { false } + + context "when the subprocess is successful" do + it "returns the result" do + expect(run_subprocess).to eq("function" => function, "args" => args) + end + + context "with an env" do + let(:env) { { "MIX_EXS" => "something" } } + + it "runs the function passed, as expected" do + expect(run_subprocess).to eq("function" => function, "args" => args) + end + end + + context "when sending stderr to stdout" do + let(:stderr_to_stdout) { true } + let(:function) { "useful_error" } + + it "raises a HelperSubprocessFailed error with stderr output" do + expect { run_subprocess } + .to raise_error( + Dependabot::SharedHelpers::HelperSubprocessFailed + ) do |error| + expect(error.message) + .to include("Some useful error") + end + end + end end - context "with an env" do - let(:env) { { "MIX_EXS" => "something" } } + context "when the subprocess fails gracefully" do + let(:function) { "error" } - it "runs the function passed, as expected" do - expect(run_subprocess).to eq("function" => function, "args" => args) + it "raises a HelperSubprocessFailed error" do + expect { run_subprocess } + .to raise_error(Dependabot::SharedHelpers::HelperSubprocessFailed) end end - context "when sending stderr to stdout" do - let(:stderr_to_stdout) { true } - let(:function) { "useful_error" } + context "when the subprocess fails gracefully with sensitive data" do + let(:function) { "sensitive_error" } - it "raises a HelperSubprocessFailed error with stderr output" do + it "raises a HelperSubprocessFailed error" do expect { run_subprocess } - .to raise_error( - Dependabot::SharedHelpers::HelperSubprocessFailed - ) do |error| - expect(error.message) - .to include("Some useful error") + .to raise_error(Dependabot::SharedHelpers::HelperSubprocessFailed) do |error| + expect(error.message).to include("Something went wrong: https://www.example.com") end end end - end - context "when the subprocess fails gracefully" do - let(:function) { "error" } + context "when the subprocess fails ungracefully" do + let(:function) { "hard_error" } + + it "raises a HelperSubprocessFailed error" do + expect { run_subprocess } + .to raise_error(Dependabot::SharedHelpers::HelperSubprocessFailed) + end + end + + context "when the subprocess is killed" do + let(:function) { "killed" } + + it "raises a HelperSubprocessFailed error" do + expect { run_subprocess } + .to(raise_error do |error| + expect(error) + .to be_a(Dependabot::SharedHelpers::HelperSubprocessFailed) + expect(error.error_context[:process_termsig]).to eq(9) + end) + end + end + + context "when a custom error class is passed" do + let(:error_class) { EcoSystemHelperSubprocessFailed } + let(:function) { "hard_error" } - it "raises a HelperSubprocessFailed error" do - expect { run_subprocess } - .to raise_error(Dependabot::SharedHelpers::HelperSubprocessFailed) + it "raises the custom error class" do + expect { run_subprocess } + .to raise_error(EcoSystemHelperSubprocessFailed) + end end end - context "when the subprocess fails gracefully with sensitive data" do - let(:function) { "sensitive_error" } + context "when enable_shared_helpers_command_timeout is enabled" do + let(:enable_shared_helpers_command_timeout) { true } + + context "when the subprocess is successful" do + it "returns the result" do + expect(run_subprocess).to eq("function" => function, "args" => args) + end + + context "with an env" do + let(:env) { { "MIX_EXS" => "something" } } + + it "runs the function passed, as expected" do + expect(run_subprocess).to eq("function" => function, "args" => args) + end + end - it "raises a HelperSubprocessFailed error" do - expect { run_subprocess } - .to raise_error(Dependabot::SharedHelpers::HelperSubprocessFailed) do |error| - expect(error.message).to include("Something went wrong: https://www.example.com") + context "when sending stderr to stdout" do + let(:stderr_to_stdout) { true } + let(:function) { "useful_error" } + + it "raises a HelperSubprocessFailed error with stderr output" do + expect { run_subprocess } + .to raise_error( + Dependabot::SharedHelpers::HelperSubprocessFailed + ) do |error| + expect(error.message) + .to include("Some useful error") + end end + end end - end - context "when the subprocess fails ungracefully" do - let(:function) { "hard_error" } + context "when the subprocess fails gracefully" do + let(:function) { "error" } - it "raises a HelperSubprocessFailed error" do - expect { run_subprocess } - .to raise_error(Dependabot::SharedHelpers::HelperSubprocessFailed) + it "raises a HelperSubprocessFailed error" do + expect { run_subprocess } + .to raise_error(Dependabot::SharedHelpers::HelperSubprocessFailed) + end end - end - context "when the subprocess is killed" do - let(:function) { "killed" } + context "when the subprocess fails gracefully with sensitive data" do + let(:function) { "sensitive_error" } - it "raises a HelperSubprocessFailed error" do - expect { run_subprocess } - .to(raise_error do |error| - expect(error) - .to be_a(Dependabot::SharedHelpers::HelperSubprocessFailed) - expect(error.error_context[:process_termsig]).to eq(9) - end) + it "raises a HelperSubprocessFailed error" do + expect { run_subprocess } + .to raise_error(Dependabot::SharedHelpers::HelperSubprocessFailed) do |error| + expect(error.message).to include("Something went wrong: https://www.example.com") + end + end end - end - context "when a custom error class is passed" do - let(:error_class) { EcoSystemHelperSubprocessFailed } - let(:function) { "hard_error" } + context "when the subprocess fails ungracefully" do + let(:function) { "hard_error" } - it "raises the custom error class" do - expect { run_subprocess } - .to raise_error(EcoSystemHelperSubprocessFailed) + it "raises a HelperSubprocessFailed error" do + expect { run_subprocess } + .to raise_error(Dependabot::SharedHelpers::HelperSubprocessFailed) + end + end + + context "when the subprocess is killed" do + let(:function) { "killed" } + + it "raises a HelperSubprocessFailed error" do + expect { run_subprocess } + .to(raise_error do |error| + expect(error) + .to be_a(Dependabot::SharedHelpers::HelperSubprocessFailed) + expect(error.error_context[:process_termsig]).to eq(9) + end) + end + end + + context "when a custom error class is passed" do + let(:error_class) { EcoSystemHelperSubprocessFailed } + let(:function) { "hard_error" } + + it "raises the custom error class" do + expect { run_subprocess } + .to raise_error(EcoSystemHelperSubprocessFailed) + end end end end @@ -239,71 +340,147 @@ def existing_tmp_folders let(:command) { File.join(spec_root, "helpers/test/run_bash") + " output" } let(:env) { nil } - context "when the subprocess is successful" do - it "returns the result" do - expect(run_shell_command).to eq("output\n") - end - end + context "when enable_shared_helpers_command_timeout is disabled" do + let(:enable_shared_helpers_command_timeout) { false } - context "with bash command as argument" do - let(:command) do - File.join(spec_root, "helpers/test/run_bash") + " $(ps)" + context "when the subprocess is successful" do + it "returns the result" do + expect(run_shell_command).to eq("output\n") + end end - it "returns the argument" do - expect(run_shell_command).to eq("$(ps)\n") + context "with bash command as argument" do + let(:command) do + File.join(spec_root, "helpers/test/run_bash") + " $(ps)" + end + + it "returns the argument" do + expect(run_shell_command).to eq("$(ps)\n") + end + + context "when allowing unsafe shell command" do + subject(:run_shell_command) do + described_class + .run_shell_command(command, allow_unsafe_shell_command: true) + end + + it "returns the command output" do + output = run_shell_command + expect(output).not_to eq("$(ps)\n") + expect(output).to include("PID") + end + end end - context "when allowing unsafe shell command" do - subject(:run_shell_command) do - described_class - .run_shell_command(command, allow_unsafe_shell_command: true) + context "with an environment variable" do + let(:env) { { "TEST_ENV" => "prefix:" } } + + it "is available to the command" do + expect(run_shell_command).to eq("prefix:output\n") end + end - it "returns the command output" do - output = run_shell_command - expect(output).not_to eq("$(ps)\n") - expect(output).to include("PID") + context "when the subprocess exits" do + let(:command) { File.join(spec_root, "helpers/test/error_bash") } + + it "raises a HelperSubprocessFailed error" do + expect { run_shell_command } + .to raise_error(Dependabot::SharedHelpers::HelperSubprocessFailed) end end - end - context "with an environment variable" do - let(:env) { { "TEST_ENV" => "prefix:" } } + context "when the subprocess exits with out of disk error" do + let(:command) { File.join(spec_root, "helpers/test/error_bash disk") } - it "is available to the command" do - expect(run_shell_command).to eq("prefix:output\n") + it "raises a HelperSubprocessFailed out of disk error" do + expect { run_shell_command } + .to raise_error(Dependabot::SharedHelpers::HelperSubprocessFailed) do |error| + expect(error.message).to include("No space left on device") + end + end + + context "when the subprocess exits with out of memory error" do + let(:command) { File.join(spec_root, "helpers/test/error_bash memory") } + + it "raises a HelperSubprocessFailed out of memory error" do + expect { run_shell_command } + .to raise_error(Dependabot::SharedHelpers::HelperSubprocessFailed) do |error| + expect(error.message).to include("MemoryError") + end + end + end end end - context "when the subprocess exits" do - let(:command) { File.join(spec_root, "helpers/test/error_bash") } + context "when enable_shared_helpers_command_timeout is enabled" do + let(:enable_shared_helpers_command_timeout) { true } - it "raises a HelperSubprocessFailed error" do - expect { run_shell_command } - .to raise_error(Dependabot::SharedHelpers::HelperSubprocessFailed) + context "when the subprocess is successful" do + it "returns the result" do + expect(run_shell_command).to eq("output\n") + end end - end - context "when the subprocess exits with out of disk error" do - let(:command) { File.join(spec_root, "helpers/test/error_bash disk") } + context "with bash command as argument" do + let(:command) do + File.join(spec_root, "helpers/test/run_bash") + " $(ps)" + end + + it "returns the argument" do + expect(run_shell_command).to eq("$(ps)\n") + end - it "raises a HelperSubprocessFailed out of disk error" do - expect { run_shell_command } - .to raise_error(Dependabot::SharedHelpers::HelperSubprocessFailed) do |error| - expect(error.message).to include("No space left on device") + context "when allowing unsafe shell command" do + subject(:run_shell_command) do + described_class + .run_shell_command(command, allow_unsafe_shell_command: true) end + + it "returns the command output" do + output = run_shell_command + expect(output).not_to eq("$(ps)\n") + expect(output).to include("PID") + end + end end - context "when the subprocess exits with out of memory error" do - let(:command) { File.join(spec_root, "helpers/test/error_bash memory") } + context "with an environment variable" do + let(:env) { { "TEST_ENV" => "prefix:" } } - it "raises a HelperSubprocessFailed out of memory error" do + it "is available to the command" do + expect(run_shell_command).to eq("prefix:output\n") + end + end + + context "when the subprocess exits" do + let(:command) { File.join(spec_root, "helpers/test/error_bash") } + + it "raises a HelperSubprocessFailed error" do + expect { run_shell_command } + .to raise_error(Dependabot::SharedHelpers::HelperSubprocessFailed) + end + end + + context "when the subprocess exits with out of disk error" do + let(:command) { File.join(spec_root, "helpers/test/error_bash disk") } + + it "raises a HelperSubprocessFailed out of disk error" do expect { run_shell_command } .to raise_error(Dependabot::SharedHelpers::HelperSubprocessFailed) do |error| - expect(error.message).to include("MemoryError") + expect(error.message).to include("No space left on device") end end + + context "when the subprocess exits with out of memory error" do + let(:command) { File.join(spec_root, "helpers/test/error_bash memory") } + + it "raises a HelperSubprocessFailed out of memory error" do + expect { run_shell_command } + .to raise_error(Dependabot::SharedHelpers::HelperSubprocessFailed) do |error| + expect(error.message).to include("MemoryError") + end + end + end end end end diff --git a/common/spec/fixtures/commands/error.sh b/common/spec/fixtures/commands/error.sh new file mode 100755 index 0000000000..0985371495 --- /dev/null +++ b/common/spec/fixtures/commands/error.sh @@ -0,0 +1,3 @@ +#!/bin/bash +echo "This is an error message." >&2 +exit 1 diff --git a/common/spec/fixtures/commands/error_hang.sh b/common/spec/fixtures/commands/error_hang.sh new file mode 100755 index 0000000000..ff59cb756a --- /dev/null +++ b/common/spec/fixtures/commands/error_hang.sh @@ -0,0 +1,8 @@ +#!/bin/bash +echo "This is a output for command error command." + +# Simulate an error output to stderr +echo "This is an error message." >&2 + +# Simulate a hang +sleep 30 \ No newline at end of file diff --git a/common/spec/fixtures/commands/no_timeout.sh b/common/spec/fixtures/commands/no_timeout.sh new file mode 100755 index 0000000000..8e52912090 --- /dev/null +++ b/common/spec/fixtures/commands/no_timeout.sh @@ -0,0 +1,4 @@ +#!/bin/bash +echo "This is a command result." +sleep 3 +exit 0 diff --git a/common/spec/fixtures/commands/output_hang.sh b/common/spec/fixtures/commands/output_hang.sh new file mode 100755 index 0000000000..e73b57a804 --- /dev/null +++ b/common/spec/fixtures/commands/output_hang.sh @@ -0,0 +1,3 @@ +#!/bin/bash +echo "This is a hanging command." +sleep 30 diff --git a/common/spec/fixtures/commands/success.sh b/common/spec/fixtures/commands/success.sh new file mode 100755 index 0000000000..45241e8517 --- /dev/null +++ b/common/spec/fixtures/commands/success.sh @@ -0,0 +1,3 @@ +#!/bin/bash +echo "This is a successful command." +exit 0 diff --git a/common/spec/spec_helper.rb b/common/spec/spec_helper.rb index 0a2084bf0f..f376821476 100644 --- a/common/spec/spec_helper.rb +++ b/common/spec/spec_helper.rb @@ -194,3 +194,11 @@ def github_credentials }] end end + +# Load a command from the fixtures/commands directory +def command_fixture(name) + path = File.join("spec", "fixtures", "commands", name) + raise "Command fixture '#{name}' does not exist" unless File.exist?(path) + + File.expand_path(path) +end diff --git a/npm_and_yarn/spec/dependabot/npm_and_yarn/file_parser_spec.rb b/npm_and_yarn/spec/dependabot/npm_and_yarn/file_parser_spec.rb index 96aab6748c..cfdd667837 100644 --- a/npm_and_yarn/spec/dependabot/npm_and_yarn/file_parser_spec.rb +++ b/npm_and_yarn/spec/dependabot/npm_and_yarn/file_parser_spec.rb @@ -42,6 +42,8 @@ .with(:npm_fallback_version_above_v6).and_return(npm_fallback_version_above_v6_enabled) allow(Dependabot::Experiments).to receive(:enabled?) .with(:enable_corepack_for_npm_and_yarn).and_return(enable_corepack_for_npm_and_yarn) + allow(Dependabot::Experiments).to receive(:enabled?) + .with(:enable_shared_helpers_command_timeout).and_return(true) end after do diff --git a/npm_and_yarn/spec/dependabot/npm_and_yarn/file_updater/npm_lockfile_updater_spec.rb b/npm_and_yarn/spec/dependabot/npm_and_yarn/file_updater/npm_lockfile_updater_spec.rb index c0f8f842d1..c5fe8dbaee 100644 --- a/npm_and_yarn/spec/dependabot/npm_and_yarn/file_updater/npm_lockfile_updater_spec.rb +++ b/npm_and_yarn/spec/dependabot/npm_and_yarn/file_updater/npm_lockfile_updater_spec.rb @@ -72,6 +72,8 @@ .with(:npm_fallback_version_above_v6).and_return(npm_fallback_version_above_v6_enabled) allow(Dependabot::Experiments).to receive(:enabled?) .with(:enable_corepack_for_npm_and_yarn).and_return(enable_corepack_for_npm_and_yarn) + allow(Dependabot::Experiments).to receive(:enabled?) + .with(:enable_shared_helpers_command_timeout).and_return(true) end after do diff --git a/npm_and_yarn/spec/dependabot/npm_and_yarn/file_updater/pnpm_lockfile_updater_spec.rb b/npm_and_yarn/spec/dependabot/npm_and_yarn/file_updater/pnpm_lockfile_updater_spec.rb index 4f86ad0ec8..811d04a513 100644 --- a/npm_and_yarn/spec/dependabot/npm_and_yarn/file_updater/pnpm_lockfile_updater_spec.rb +++ b/npm_and_yarn/spec/dependabot/npm_and_yarn/file_updater/pnpm_lockfile_updater_spec.rb @@ -70,6 +70,8 @@ FileUtils.mkdir_p(tmp_path) allow(Dependabot::Experiments).to receive(:enabled?) .with(:enable_corepack_for_npm_and_yarn).and_return(enable_corepack_for_npm_and_yarn) + allow(Dependabot::Experiments).to receive(:enabled?) + .with(:enable_shared_helpers_command_timeout).and_return(true) end after do diff --git a/npm_and_yarn/spec/dependabot/npm_and_yarn/file_updater/yarn_lockfile_updater_spec.rb b/npm_and_yarn/spec/dependabot/npm_and_yarn/file_updater/yarn_lockfile_updater_spec.rb index aa4c55324e..af955adf0b 100644 --- a/npm_and_yarn/spec/dependabot/npm_and_yarn/file_updater/yarn_lockfile_updater_spec.rb +++ b/npm_and_yarn/spec/dependabot/npm_and_yarn/file_updater/yarn_lockfile_updater_spec.rb @@ -65,6 +65,8 @@ FileUtils.mkdir_p(tmp_path) allow(Dependabot::Experiments).to receive(:enabled?) .with(:enable_corepack_for_npm_and_yarn).and_return(enable_corepack_for_npm_and_yarn) + allow(Dependabot::Experiments).to receive(:enabled?) + .with(:enable_shared_helpers_command_timeout).and_return(true) end after do diff --git a/npm_and_yarn/spec/dependabot/npm_and_yarn/file_updater_spec.rb b/npm_and_yarn/spec/dependabot/npm_and_yarn/file_updater_spec.rb index ab362f490f..3514c191af 100644 --- a/npm_and_yarn/spec/dependabot/npm_and_yarn/file_updater_spec.rb +++ b/npm_and_yarn/spec/dependabot/npm_and_yarn/file_updater_spec.rb @@ -68,6 +68,8 @@ .with(:npm_fallback_version_above_v6).and_return(npm_fallback_version_above_v6_enabled) allow(Dependabot::Experiments).to receive(:enabled?) .with(:enable_corepack_for_npm_and_yarn).and_return(enable_corepack_for_npm_and_yarn) + allow(Dependabot::Experiments).to receive(:enabled?) + .with(:enable_shared_helpers_command_timeout).and_return(true) end after do diff --git a/npm_and_yarn/spec/dependabot/npm_and_yarn/update_checker/subdependency_version_resolver_spec.rb b/npm_and_yarn/spec/dependabot/npm_and_yarn/update_checker/subdependency_version_resolver_spec.rb index 51f3bd5de3..10247cb171 100644 --- a/npm_and_yarn/spec/dependabot/npm_and_yarn/update_checker/subdependency_version_resolver_spec.rb +++ b/npm_and_yarn/spec/dependabot/npm_and_yarn/update_checker/subdependency_version_resolver_spec.rb @@ -41,6 +41,8 @@ .with(:npm_fallback_version_above_v6).and_return(npm_fallback_version_above_v6_enabled) allow(Dependabot::Experiments).to receive(:enabled?) .with(:enable_corepack_for_npm_and_yarn).and_return(enable_corepack_for_npm_and_yarn) + allow(Dependabot::Experiments).to receive(:enabled?) + .with(:enable_shared_helpers_command_timeout).and_return(true) end after do diff --git a/npm_and_yarn/spec/dependabot/npm_and_yarn/update_checker/version_resolver_spec.rb b/npm_and_yarn/spec/dependabot/npm_and_yarn/update_checker/version_resolver_spec.rb index 0387075973..05a001c0a4 100644 --- a/npm_and_yarn/spec/dependabot/npm_and_yarn/update_checker/version_resolver_spec.rb +++ b/npm_and_yarn/spec/dependabot/npm_and_yarn/update_checker/version_resolver_spec.rb @@ -83,6 +83,8 @@ .with(:npm_fallback_version_above_v6).and_return(npm_fallback_version_above_v6_enabled) allow(Dependabot::Experiments).to receive(:enabled?) .with(:enable_corepack_for_npm_and_yarn).and_return(enable_corepack_for_npm_and_yarn) + allow(Dependabot::Experiments).to receive(:enabled?) + .with(:enable_shared_helpers_command_timeout).and_return(true) end after do diff --git a/npm_and_yarn/spec/dependabot/npm_and_yarn/update_checker_spec.rb b/npm_and_yarn/spec/dependabot/npm_and_yarn/update_checker_spec.rb index 93208bf669..1af401cf5a 100644 --- a/npm_and_yarn/spec/dependabot/npm_and_yarn/update_checker_spec.rb +++ b/npm_and_yarn/spec/dependabot/npm_and_yarn/update_checker_spec.rb @@ -71,6 +71,8 @@ .with(:enable_corepack_for_npm_and_yarn).and_return(enable_corepack_for_npm_and_yarn) allow(Dependabot::Experiments).to receive(:enabled?) .with(:npm_fallback_version_above_v6).and_return(npm_fallback_version_above_v6_enabled) + allow(Dependabot::Experiments).to receive(:enabled?) + .with(:enable_shared_helpers_command_timeout).and_return(true) end after do diff --git a/updater/spec/dependabot/dependency_snapshot_spec.rb b/updater/spec/dependabot/dependency_snapshot_spec.rb index fc63e23a94..e309f117c8 100644 --- a/updater/spec/dependabot/dependency_snapshot_spec.rb +++ b/updater/spec/dependabot/dependency_snapshot_spec.rb @@ -108,6 +108,9 @@ allow(Dependabot::Experiments).to receive(:enabled?) .with(:add_deprecation_warn_to_pr_message) .and_return(true) + allow(Dependabot::Experiments).to receive(:enabled?) + .with(:enable_shared_helpers_command_timeout) + .and_return(true) end after do diff --git a/updater/spec/dependabot/updater/operations/refresh_security_update_pull_request_spec.rb b/updater/spec/dependabot/updater/operations/refresh_security_update_pull_request_spec.rb index e69e456adf..e093a1f590 100644 --- a/updater/spec/dependabot/updater/operations/refresh_security_update_pull_request_spec.rb +++ b/updater/spec/dependabot/updater/operations/refresh_security_update_pull_request_spec.rb @@ -158,6 +158,10 @@ .and_return(stub_dependency_change) allow(mock_service).to receive(:close_pull_request) + + allow(Dependabot::Experiments).to receive(:enabled?) + .with(:enable_shared_helpers_command_timeout) + .and_return(true) end after do diff --git a/updater/spec/dependabot/updater/operations/refresh_version_update_pull_request_spec.rb b/updater/spec/dependabot/updater/operations/refresh_version_update_pull_request_spec.rb index dbeeed37b0..b8fb3ce989 100644 --- a/updater/spec/dependabot/updater/operations/refresh_version_update_pull_request_spec.rb +++ b/updater/spec/dependabot/updater/operations/refresh_version_update_pull_request_spec.rb @@ -140,6 +140,9 @@ before do allow(Dependabot::Experiments).to receive(:enabled?).with(:lead_security_dependency).and_return(false) + allow(Dependabot::Experiments).to receive(:enabled?) + .with(:enable_shared_helpers_command_timeout) + .and_return(true) allow(Dependabot::UpdateCheckers).to receive(:for_package_manager).and_return(stub_update_checker_class) allow(Dependabot::DependencyChangeBuilder)