From f3fec1f2e0e628be79427c8139e816bf83ff16ad Mon Sep 17 00:00:00 2001 From: Jamie Magee Date: Thu, 2 Nov 2023 09:43:05 -0700 Subject: [PATCH] Type `SharedHelpers` more thoroughly (#8310) --- .../lib/dependabot/bundler/native_helpers.rb | 26 ++++- .../dependabot/bundler/native_helpers_spec.rb | 12 +- common/lib/dependabot/shared_helpers.rb | 107 +++++++++++++++--- common/lib/dependabot/workspace/base.rb | 10 +- .../file_updater/pipfile_file_updater.rb | 2 +- .../file_updater/poetry_file_updater.rb | 2 +- sorbet/rbi/shims/hash.rbi | 8 ++ 7 files changed, 141 insertions(+), 26 deletions(-) diff --git a/bundler/lib/dependabot/bundler/native_helpers.rb b/bundler/lib/dependabot/bundler/native_helpers.rb index 72dce5acce4..321de382138 100644 --- a/bundler/lib/dependabot/bundler/native_helpers.rb +++ b/bundler/lib/dependabot/bundler/native_helpers.rb @@ -1,32 +1,43 @@ -# typed: true +# typed: strict # frozen_string_literal: true require "bundler" +require "sorbet-runtime" require "dependabot/shared_helpers" module Dependabot module Bundler module NativeHelpers + extend T::Sig + extend T::Generic + class BundleCommand + extend T::Sig + MAX_SECONDS = 1800 MIN_SECONDS = 60 + sig { params(timeout_seconds: T.nilable(Integer)).void } def initialize(timeout_seconds) - @timeout_seconds = clamp(timeout_seconds) + @timeout_seconds = T.let(clamp(timeout_seconds), Integer) end + sig { params(script: String).returns(String) } def build(script) [timeout_command, :ruby, script].compact.join(" ") end private + sig { returns(Integer) } attr_reader :timeout_seconds + sig { returns(T.nilable(String)) } def timeout_command "timeout -s HUP #{timeout_seconds}" unless timeout_seconds.zero? end + sig { params(seconds: T.nilable(Integer)).returns(Integer) } def clamp(seconds) return 0 unless seconds @@ -34,6 +45,15 @@ def clamp(seconds) end end + sig do + params( + function: String, + args: T::Hash[Symbol, String], + bundler_version: String, + options: T::Hash[Symbol, T.untyped] + ) + .returns(T.untyped) + end def self.run_bundler_subprocess(function:, args:, bundler_version:, options: {}) # Run helper suprocess with all bundler-related ENV variables removed helpers_path = versioned_helper_path(bundler_version) @@ -60,10 +80,12 @@ def self.run_bundler_subprocess(function:, args:, bundler_version:, options: {}) end end + sig { params(bundler_major_version: String).returns(String) } def self.versioned_helper_path(bundler_major_version) File.join(native_helpers_root, "v#{bundler_major_version}") end + sig { returns(String) } def self.native_helpers_root helpers_root = ENV.fetch("DEPENDABOT_NATIVE_HELPERS_PATH", nil) return File.join(helpers_root, "bundler") unless helpers_root.nil? diff --git a/bundler/spec/dependabot/bundler/native_helpers_spec.rb b/bundler/spec/dependabot/bundler/native_helpers_spec.rb index c98a0fe549f..f0a1baca137 100644 --- a/bundler/spec/dependabot/bundler/native_helpers_spec.rb +++ b/bundler/spec/dependabot/bundler/native_helpers_spec.rb @@ -18,7 +18,7 @@ with_env("DEPENDABOT_NATIVE_HELPERS_PATH", native_helpers_path) do subject.run_bundler_subprocess( function: "noop", - args: [], + args: {}, bundler_version: "2", options: options ) @@ -34,7 +34,7 @@ .with( command: "timeout -s HUP 120 ruby /opt/bundler/v2/run.rb", function: "noop", - args: [], + args: {}, env: anything ) end @@ -54,7 +54,7 @@ .with( command: "timeout -s HUP 1800 ruby /opt/bundler/v2/run.rb", function: "noop", - args: [], + args: {}, env: anything ) end @@ -74,7 +74,7 @@ .with( command: "timeout -s HUP 60 ruby /opt/bundler/v2/run.rb", function: "noop", - args: [], + args: {}, env: anything ) end @@ -89,7 +89,7 @@ .with( command: "ruby /opt/bundler/v2/run.rb", function: "noop", - args: [], + args: {}, env: anything ) end @@ -104,7 +104,7 @@ .with( command: "ruby #{File.expand_path('../../../helpers/v2/run.rb', __dir__)}", function: "noop", - args: [], + args: {}, env: anything ) end diff --git a/common/lib/dependabot/shared_helpers.rb b/common/lib/dependabot/shared_helpers.rb index 081ac7fa5a0..e6e36bc9de1 100644 --- a/common/lib/dependabot/shared_helpers.rb +++ b/common/lib/dependabot/shared_helpers.rb @@ -1,4 +1,4 @@ -# typed: true +# typed: strict # frozen_string_literal: true require "digest" @@ -21,13 +21,25 @@ module Dependabot module SharedHelpers extend T::Sig - GIT_CONFIG_GLOBAL_PATH = File.expand_path(".gitconfig", Utils::BUMP_TMP_DIR_PATH) - USER_AGENT = "dependabot-core/#{Dependabot::VERSION} " \ - "#{Excon::USER_AGENT} ruby/#{RUBY_VERSION} " \ - "(#{RUBY_PLATFORM}) " \ - "(+https://github.com/dependabot/dependabot-core)".freeze + GIT_CONFIG_GLOBAL_PATH = T.let(File.expand_path(".gitconfig", Utils::BUMP_TMP_DIR_PATH), String) + USER_AGENT = T.let( + "dependabot-core/#{Dependabot::VERSION} " \ + "#{Excon::USER_AGENT} ruby/#{RUBY_VERSION} " \ + "(#{RUBY_PLATFORM}) " \ + "(+https://github.com/dependabot/dependabot-core)".freeze, + String + ) SIGKILL = 9 + sig do + type_parameters(:T) + .params( + directory: String, + repo_contents_path: T.nilable(String), + block: T.proc.params(arg0: T.any(Pathname, String)).returns(T.type_parameter(:T)) + ) + .returns(T.type_parameter(:T)) + end def self.in_a_temporary_repo_directory(directory = "/", repo_contents_path = nil, &block) if repo_contents_path # If a workspace has been defined to allow orcestration of the git repo @@ -49,9 +61,18 @@ def self.in_a_temporary_repo_directory(directory = "/", repo_contents_path = nil end end - def self.in_a_temporary_directory(directory = "/") + sig do + type_parameters(:T) + .params( + directory: String, + _block: T.proc.params(arg0: T.any(Pathname, String)).returns(T.type_parameter(:T)) + ) + .returns(T.type_parameter(:T)) + end + def self.in_a_temporary_directory(directory = "/", &_block) FileUtils.mkdir_p(Utils::BUMP_TMP_DIR_PATH) tmp_dir = Dir.mktmpdir(Utils::BUMP_TMP_FILE_PREFIX, Utils::BUMP_TMP_DIR_PATH) + path = Pathname.new(File.join(tmp_dir, directory)).expand_path begin path = Pathname.new(File.join(tmp_dir, directory)).expand_path @@ -63,22 +84,41 @@ def self.in_a_temporary_directory(directory = "/") end class HelperSubprocessFailed < Dependabot::DependabotError - attr_reader :error_class, :error_context, :trace + extend T::Sig + + sig { returns(String) } + attr_reader :error_class + + sig { returns(T::Hash[Symbol, String]) } + attr_reader :error_context + + sig { returns(T.nilable(T::Array[String])) } + attr_reader :trace + sig do + params( + message: String, + error_context: T::Hash[Symbol, String], + error_class: T.nilable(String), + trace: T.nilable(T::Array[String]) + ).void + end def initialize(message:, error_context:, error_class: nil, trace: nil) super(message) - @error_class = error_class || "HelperSubprocessFailed" + @error_class = T.let(error_class || "HelperSubprocessFailed", String) @error_context = error_context - @fingerprint = error_context[:fingerprint] || error_context[:command] + @fingerprint = T.let(error_context[:fingerprint] || error_context[:command], T.nilable(String)) @trace = trace end + sig { returns(T::Hash[Symbol, T.untyped]) } def raven_context { fingerprint: [@fingerprint], extra: @error_context.except(:stderr_output, :fingerprint) } end end # 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) @@ -86,6 +126,17 @@ def self.escape_command(command) # rubocop:disable Metrics/MethodLength # rubocop:disable Metrics/AbcSize + sig do + params( + command: String, + function: String, + args: T.any(T::Array[String], T::Hash[Symbol, String]), + env: T.nilable(T::Hash[String, String]), + stderr_to_stdout: T::Boolean, + allow_unsafe_shell_command: T::Boolean + ) + .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) @@ -150,6 +201,7 @@ def self.run_helper_subprocess(command:, function:, args:, env: nil, end # rubocop:enable Metrics/MethodLength + sig { params(stderr: T.nilable(String), error_context: T::Hash[Symbol, String]).void } def self.check_out_of_memory_error(stderr, error_context) return unless stderr&.include?("JavaScript heap out of memory") @@ -160,12 +212,14 @@ def self.check_out_of_memory_error(stderr, error_context) ) end + sig { returns(T::Array[T.class_of(Excon::Middleware::Base)]) } def self.excon_middleware - Excon.defaults[:middlewares] + + T.must(T.cast(Excon.defaults, T::Hash[Symbol, T::Array[T.class_of(Excon::Middleware::Base)]])[:middlewares]) + [Excon::Middleware::Decompress] + [Excon::Middleware::RedirectFollower] end + sig { params(headers: T.nilable(T::Hash[String, String])).returns(T::Hash[String, String]) } def self.excon_headers(headers = nil) headers ||= {} { @@ -173,9 +227,10 @@ def self.excon_headers(headers = nil) }.merge(headers) end + sig { params(options: T.nilable(T::Hash[Symbol, T.untyped])).returns(T::Hash[Symbol, T.untyped]) } def self.excon_defaults(options = nil) options ||= {} - headers = options.delete(:headers) + headers = T.cast(options.delete(:headers), T.nilable(T::Hash[String, String])) { instrumentor: Dependabot::SimpleInstrumentor, connect_timeout: 5, @@ -188,7 +243,15 @@ def self.excon_defaults(options = nil) }.merge(options) end - def self.with_git_configured(credentials:) + sig do + type_parameters(:T) + .params( + credentials: T::Array[T::Hash[String, String]], + _block: T.proc.returns(T.type_parameter(:T)) + ) + .returns(T.type_parameter(:T)) + end + def self.with_git_configured(credentials:, &_block) safe_directories = find_safe_directories FileUtils.mkdir_p(Utils::BUMP_TMP_DIR_PATH) @@ -209,17 +272,20 @@ def self.with_git_configured(credentials:) end # Handle SCP-style git URIs + sig { params(uri: String).returns(String) } def self.scp_to_standard(uri) return uri unless uri.start_with?("git@") - "https://#{uri.split('git@').last.sub(%r{:/?}, '/')}" + "https://#{T.must(uri.split('git@').last).sub(%r{:/?}, '/')}" end + sig { returns(String) } def self.credential_helper_path File.join(__dir__, "../../bin/git-credential-store-immutable") end # rubocop:disable Metrics/PerceivedComplexity + sig { params(credentials: T::Array[T::Hash[String, String]], safe_directories: T::Array[String]).void } def self.configure_git_to_use_https_with_credentials(credentials, safe_directories) File.open(GIT_CONFIG_GLOBAL_PATH, "w") do |file| file << "# Generated by dependabot/dependabot-core" @@ -279,6 +345,7 @@ def self.configure_git_to_use_https_with_credentials(credentials, safe_directori # rubocop:enable Metrics/AbcSize # rubocop:enable Metrics/PerceivedComplexity + sig { params(host: String).void } def self.configure_git_to_use_https(host) # NOTE: we use --global here (rather than --system) so that Dependabot # can be run without privileged access @@ -304,6 +371,7 @@ def self.configure_git_to_use_https(host) ) end + sig { params(path: String).void } def self.reset_git_repo(path) Dir.chdir(path) do run_shell_command("git reset HEAD --hard") @@ -311,6 +379,7 @@ def self.reset_git_repo(path) end end + sig { returns(T::Array[String]) } def self.find_safe_directories # to preserve safe directories from global .gitconfig output, process = Open3.capture2("git config --global --get-all safe.directory") @@ -319,6 +388,15 @@ def self.find_safe_directories safe_directories end + sig do + params( + command: String, + allow_unsafe_shell_command: T::Boolean, + env: T.nilable(T::Hash[String, String]), + fingerprint: T.nilable(String), + stderr_to_stdout: T::Boolean + ).returns(String) + end def self.run_shell_command(command, allow_unsafe_shell_command: false, env: {}, @@ -352,6 +430,7 @@ def self.run_shell_command(command, ) end + sig { params(command: String, stdin_data: String, env: T.nilable(T::Hash[String, String])).returns(String) } def self.helper_subprocess_bash_command(command:, stdin_data:, env:) escaped_stdin_data = stdin_data.gsub("\"", "\\\"") env_keys = env ? env.compact.map { |k, v| "#{k}=#{v}" }.join(" ") + " " : "" diff --git a/common/lib/dependabot/workspace/base.rb b/common/lib/dependabot/workspace/base.rb index 6082da976de..c6e12ce13d5 100644 --- a/common/lib/dependabot/workspace/base.rb +++ b/common/lib/dependabot/workspace/base.rb @@ -8,6 +8,8 @@ module Workspace class Base extend T::Sig extend T::Helpers + extend T::Generic + abstract! sig { returns(T::Array[Dependabot::Workspace::ChangeAttempt]) } @@ -38,8 +40,12 @@ def failed_change_attempts end sig do - params(memo: T.nilable(String), _blk: T.proc.params(arg0: T.any(Pathname, String)).returns(T.untyped)) - .returns(T.untyped) + type_parameters(:T) + .params( + memo: T.nilable(String), + _blk: T.proc.params(arg0: T.any(Pathname, String)).returns(T.type_parameter(:T)) + ) + .returns(T.type_parameter(:T)) end def change(memo = nil, &_blk) Dir.chdir(path) { yield(path) } diff --git a/python/lib/dependabot/python/file_updater/pipfile_file_updater.rb b/python/lib/dependabot/python/file_updater/pipfile_file_updater.rb index 7c9a9d9e3aa..7b576cea868 100644 --- a/python/lib/dependabot/python/file_updater/pipfile_file_updater.rb +++ b/python/lib/dependabot/python/file_updater/pipfile_file_updater.rb @@ -265,7 +265,7 @@ def pipfile_hash_for(pipfile_content) SharedHelpers.run_helper_subprocess( command: "pyenv exec python3 #{NativeHelpers.python_helper_path}", function: "get_pipfile_hash", - args: [dir] + args: [T.cast(dir, Pathname).to_s] ) end end diff --git a/python/lib/dependabot/python/file_updater/poetry_file_updater.rb b/python/lib/dependabot/python/file_updater/poetry_file_updater.rb index b299b4e5232..772b838945a 100644 --- a/python/lib/dependabot/python/file_updater/poetry_file_updater.rb +++ b/python/lib/dependabot/python/file_updater/poetry_file_updater.rb @@ -238,7 +238,7 @@ def pyproject_hash_for(pyproject_content) SharedHelpers.run_helper_subprocess( command: "pyenv exec python3 #{python_helper_path}", function: "get_pyproject_hash", - args: [dir] + args: [T.cast(dir, Pathname).to_s] ) end end diff --git a/sorbet/rbi/shims/hash.rbi b/sorbet/rbi/shims/hash.rbi index 2c8f3392b1b..53208ced047 100644 --- a/sorbet/rbi/shims/hash.rbi +++ b/sorbet/rbi/shims/hash.rbi @@ -2,6 +2,14 @@ # frozen_string_literal: true class Hash + extend T::Generic + sig { returns(String) } def to_json; end + + sig { returns(Integer) } + def hash; end + + sig { returns(T::Hash[K, V]) } + def compact; end end