diff --git a/Manifest.txt b/Manifest.txt index f2ddb0fb2653..e20b0809a3d9 100644 --- a/Manifest.txt +++ b/Manifest.txt @@ -363,6 +363,10 @@ lib/rubygems/core_ext/kernel_gem.rb lib/rubygems/core_ext/kernel_require.rb lib/rubygems/core_ext/kernel_warn.rb lib/rubygems/core_ext/tcpsocket_init.rb +lib/rubygems/credential_store.rb +lib/rubygems/credential_store/native/linux.rb +lib/rubygems/credential_store/native/macos.rb +lib/rubygems/credential_store/native/windows.rb lib/rubygems/defaults.rb lib/rubygems/dependency.rb lib/rubygems/dependency_installer.rb diff --git a/bundler.gemspec b/bundler.gemspec index efeb096e61a0..dd4585333f78 100644 --- a/bundler.gemspec +++ b/bundler.gemspec @@ -37,14 +37,15 @@ Gem::Specification.new do |s| s.files = Dir.glob("lib/bundler{.rb,/**/*}", File::FNM_DOTMATCH).reject {|f| File.directory?(f) } # Bundler reuses RubyGems' vendored URI, SecureRandom and PubGrub, its - # pure-Ruby YAML serializer and its compact index client. Ship a copy under - # lib/rubygems so Bundler stays self-contained on RubyGems versions that - # predate them. + # pure-Ruby YAML serializer, its compact index client and its credential + # store. Ship a copy under lib/rubygems so Bundler stays self-contained on + # RubyGems versions that predate them. s.files += Dir.glob("lib/rubygems/vendor/uri/**/*", File::FNM_DOTMATCH).reject {|f| File.directory?(f) } s.files += Dir.glob("lib/rubygems/vendor/securerandom/**/*", File::FNM_DOTMATCH).reject {|f| File.directory?(f) } s.files += Dir.glob("lib/rubygems/vendor/pub_grub/**/*", File::FNM_DOTMATCH).reject {|f| File.directory?(f) } s.files += Dir.glob("lib/rubygems/yaml_serializer.rb") s.files += Dir.glob("lib/rubygems/compact_index_client{.rb,/**/*}", File::FNM_DOTMATCH).reject {|f| File.directory?(f) } + s.files += Dir.glob("lib/rubygems/credential_store{.rb,/**/*}", File::FNM_DOTMATCH).reject {|f| File.directory?(f) } # include the gemspec itself because warbler breaks w/o it s.files += %w[bundler.gemspec] diff --git a/lib/bundler/cli/config.rb b/lib/bundler/cli/config.rb index 976cda748466..0ae5aafa11b3 100644 --- a/lib/bundler/cli/config.rb +++ b/lib/bundler/cli/config.rb @@ -97,7 +97,9 @@ def run confirm(name) end - if current_value.nil? + # A credential in the store has no value here, but it is set: the + # exit status has to say so even though the secret is not printed. + if current_value.nil? && !Bundler.settings.credential_stored?(name) exit 1 else return @@ -111,6 +113,11 @@ def run def confirm_all if @options[:parseable] thor.with_padding do + # --parseable output is meant to be fed back to `bundle config + # set`, so a credential that lives in the store is skipped rather + # than printed with a placeholder value. A placeholder would be + # read back as the credential itself, and writing it would drop + # the real secret from the store. Bundler.settings.all.each do |setting| val = Bundler.settings[setting] Bundler.ui.info "#{setting}=#{val}" @@ -118,7 +125,7 @@ def confirm_all end else Bundler.ui.confirm "Settings are listed in order of priority. The top value will be used.\n" - Bundler.settings.all.each do |setting| + Bundler.settings.all_including_stored_credentials.each do |setting| Bundler.ui.confirm setting show_pretty_values_for(setting) Bundler.ui.confirm "" diff --git a/lib/bundler/env.rb b/lib/bundler/env.rb index 2b2970506098..99df2e490524 100644 --- a/lib/bundler/env.rb +++ b/lib/bundler/env.rb @@ -17,9 +17,11 @@ def self.report(options = {}) append_formatted_table("Environment", environment, out) append_formatted_table("Bundler Build Metadata", BuildMetadata.to_h, out) - unless Bundler.settings.all.empty? + settings = Bundler.settings.all_including_stored_credentials + + unless settings.empty? out << "\n## Bundler settings\n\n```\n" - Bundler.settings.all.each do |setting| + settings.each do |setting| out << setting << "\n" Bundler.settings.pretty_values_for(setting).each do |line| out << " " << line << "\n" diff --git a/lib/bundler/man/bundle-config.1 b/lib/bundler/man/bundle-config.1 index 0ae7c8b4510a..03ebb38d7218 100644 --- a/lib/bundler/man/bundle-config.1 +++ b/lib/bundler/man/bundle-config.1 @@ -105,6 +105,16 @@ Cooldown filtering depends on the gem server providing a per\-version \fBcreated .IP A \fBcreated_at\fR timestamp is read as UTC when it carries no time zone offset\. \fBrubygems\.org\fR always sends one, but a third\-party server that omits it would otherwise shift the cooldown window by the offset of whatever machine runs bundler\. .IP "\(bu" 4 +\fBcredential_store\fR (\fBBUNDLE_CREDENTIAL_STORE\fR): Experimental: store and read host credentials (the values otherwise set via \fBbundle config set \fR) in a credential store instead of the plain text config file\. Set it to \fBtrue\fR to use the operating system's native store (macOS Keychain, Linux Secret Service, Windows Credential Manager) when one is available on this platform, or to the name of a backend provided by a third\-party gem, such as \fB1password\fR\. Falls back to the config file when the store is unavailable or fails, warning that the credential was written in plain text\. Defaults to false\. Credentials already written to the config file are not migrated automatically; re\-run \fBbundle config set \fR with the setting enabled to move each one into the store\. Being experimental, the name and behavior of this setting may change in a future release\. +.IP +A credential kept in the store is never printed back\. \fBbundle config get \fR and \fBbundle config list\fR name the key and say that its value lives in the credential store\. With \fB\-\-parseable\fR, such a key is left out entirely, since that output is meant to be read back by \fBbundle config set\fR\. A third\-party backend is not required to enumerate what it holds, so a credential kept in one may not be listed at all\. +.IP +The store belongs to the machine's user, not to a project, so \fB\-\-local\fR and \fB\-\-global\fR make no difference to where a credential is kept\. Setting a host's credential in one project changes it for every project on the machine, and unsetting it there removes it everywhere\. Protecting the store itself is the operating system's job, or that of whichever backend you selected\. +.IP +A credential given in the environment, such as \fBBUNDLE_GEMS__EXAMPLE__COM\fR, takes precedence over the stored one, so a CI run can pass its own credentials without the store getting in the way\. The store takes precedence over the config file\. +.IP +The store can also be selected per host with \fBcredential_store\.\fR (\fBBUNDLE_CREDENTIAL_STORE__\fR), for example \fBbundle config set credential_store\.gems\.example\.com code_artifact\fR\. That host then uses only the named backend, with no chaining to the global one, while every other host keeps following the global setting\. Setting a host's value to \fBfalse\fR keeps that one host on the config file even when a global store is enabled\. +.IP "\(bu" 4 \fBdefault_cli_command\fR (\fBBUNDLE_DEFAULT_CLI_COMMAND\fR): The command that running \fBbundle\fR without arguments should run\. Defaults to \fBcli_help\fR since Bundler 4, but can also be \fBinstall\fR which was the previous default\. .IP "\(bu" 4 \fBdeployment\fR (\fBBUNDLE_DEPLOYMENT\fR): Equivalent to setting \fBfrozen\fR to \fBtrue\fR and \fBpath\fR to \fBvendor/bundle\fR\. diff --git a/lib/bundler/man/bundle-config.1.ronn b/lib/bundler/man/bundle-config.1.ronn index f01f43d709c2..d612bdba9a85 100644 --- a/lib/bundler/man/bundle-config.1.ronn +++ b/lib/bundler/man/bundle-config.1.ronn @@ -179,6 +179,46 @@ learn more about their operation in [bundle install(1)](bundle-install.1.html). offset. `rubygems.org` always sends one, but a third-party server that omits it would otherwise shift the cooldown window by the offset of whatever machine runs bundler. +* `credential_store` (`BUNDLE_CREDENTIAL_STORE`): + Experimental: store and read host credentials (the values otherwise set + via `bundle config set `) in a credential store instead + of the plain text config file. Set it to `true` to use the operating + system's native store (macOS Keychain, Linux Secret Service, Windows + Credential Manager) when one is available on this platform, or to the name + of a backend provided by a third-party gem, such as `1password`. Falls + back to the config file when the store is unavailable or fails, warning + that the credential was written in plain text. Defaults to false. + Credentials already written to the config file are not migrated + automatically; re-run `bundle config set ` with the + setting enabled to move each one into the store. Being experimental, the + name and behavior of this setting may change in a future release. + + A credential kept in the store is never printed back. `bundle config get + ` and `bundle config list` name the key and say that its value lives + in the credential store. With `--parseable`, such a key is left out + entirely, since that output is meant to be read back by `bundle config + set`. A third-party backend is not required to enumerate what it holds, so + a credential kept in one may not be listed at all. + + The store belongs to the machine's user, not to a project, so + `--local` and `--global` make no difference to where a credential is + kept. Setting a host's credential in one project changes it for every + project on the machine, and unsetting it there removes it everywhere. + Protecting the store itself is the operating system's job, or that of + whichever backend you selected. + + A credential given in the environment, such as + `BUNDLE_GEMS__EXAMPLE__COM`, takes precedence over the stored one, so a + CI run can pass its own credentials without the store getting in the + way. The store takes precedence over the config file. + + The store can also be selected per host with `credential_store.` + (`BUNDLE_CREDENTIAL_STORE__`), for example `bundle config set + credential_store.gems.example.com code_artifact`. That host then uses + only the named backend, with no chaining to the global one, while every + other host keeps following the global setting. Setting a host's value + to `false` keeps that one host on the config file even when a global + store is enabled. * `default_cli_command` (`BUNDLE_DEFAULT_CLI_COMMAND`): The command that running `bundle` without arguments should run. Defaults to `cli_help` since Bundler 4, but can also be `install` which was the previous diff --git a/lib/bundler/settings.rb b/lib/bundler/settings.rb index c1f8ecf824e1..247db1eac43a 100644 --- a/lib/bundler/settings.rb +++ b/lib/bundler/settings.rb @@ -62,6 +62,7 @@ class Settings bin cache_path console + credential_store default_cli_command gem.ci gem.github_username @@ -167,6 +168,29 @@ def all keys end + ## + # #all plus the keys whose credential lives in the credential store. Kept + # apart from #all because that one is on the hot path (it is read per gem + # source and per download, and its keys are advertised in the User-Agent), + # while this one is for the commands that display settings. + + def all_including_stored_credentials + keys = stored_credential_keys.map do |key| + key = key.delete_prefix("BUNDLE_") + key.gsub!("___", "-") + key.gsub!("__", ".") + key.downcase! + key + end + + # The listing comes from the globally selected store, but a host can + # name its own. Keep only the keys the per-host lookup agrees are set, + # or the display would name a key and then report it as unconfigured. + keys.select! {|key| credential_stored?(key) } + + all.union(keys).sort + end + def local_overrides repos = {} all.each do |k| @@ -185,6 +209,9 @@ def mirror_for(uri) end def credentials_for(uri) + stored = credentials_from_store(uri) + return credentials_from_env(uri) || stored if stored + self[uri.to_s] || self[uri.host] end @@ -221,6 +248,14 @@ def pretty_values_for(exposed_key) locations << "Set via #{key}: #{printable_value(value, exposed_key).inspect}" end + if credential_stored?(exposed_key) + # The heading calls this a priority order, but a stored credential + # does not sit in that order: it is used ahead of every config file, + # whichever line it happens to be printed on. Say so here rather than + # leave the position implying otherwise. + locations << "Set in the credential store, which is used ahead of the config files" + end + if value = @global_config[key] locations << "Set for the current user (#{global_config_file}): #{printable_value(value, exposed_key).inspect}" end @@ -229,6 +264,31 @@ def pretty_values_for(exposed_key) locations end + ## + # True when +name+'s credential lives in the credential store. The secret + # itself is never returned: callers only need to know the setting exists, + # since Settings#[] cannot see past the config files. + + def credential_stored?(name) + raw_key = self.class.key_to_s(name) + return false unless credential_store_key?(raw_key) + return false unless store = active_credential_store(credential_host(raw_key)) + + !store.get(credential_account(raw_key)).nil? + end + + ## + # The keys credentials are stored under, in the same encoding the config + # hashes use, so #all can fold them in. Empty when no store is enabled or + # when the backend cannot enumerate its entries, which is why + # bundle-config(1) warns that a third-party backend may not list. + + def stored_credential_keys + return [] unless store = active_credential_store + + Array(store.list) + end + def processor_count require "etc" Etc.nprocessors @@ -355,7 +415,7 @@ def is_bool(name) def is_string(name) name = self.class.key_to_s(name) - STRING_KEYS.include?(name) || name.start_with?("local.") || name.start_with?("mirror.") || name.start_with?("build.") + STRING_KEYS.include?(name) || name.start_with?("local.") || name.start_with?("mirror.") || name.start_with?("build.") || name.start_with?("credential_store.") end def to_bool(value) @@ -385,6 +445,179 @@ def is_userinfo(value) value.include?(":") end + ## + # The Gem::CredentialStore instance to use, or nil when the + # `credential_store` setting is off. The value is `true`/`"true"` for this + # platform's native backend or a backend name such as `"1password"`. + # Guarded by a cheap lookup so reading and writing settings costs nothing + # extra when the setting is disabled. + + # The account namespace Bundler uses in the shared native store, kept + # separate from RubyGems so that gem signout does not remove Bundler's + # host credentials. + CREDENTIAL_STORE_SERVICE = "bundler" + + def active_credential_store(host = nil) + spec = credential_store_spec(host) + return nil unless spec + + store_class = credential_store_class + return nil unless store_class + + store_class.for(spec, service: CREDENTIAL_STORE_SERVICE) + end + + # A `credential_store.` setting overrides the global + # `credential_store` for that host only, so one source can use a + # dedicated backend (say, a CodeArtifact token issuer) while every other + # host keeps the global choice. There is no chain between backends: an + # explicit `false` for a host keeps that host on the config file even + # when a global store is enabled. + def credential_store_spec(host = nil) + value = self["credential_store.#{host}"] if host + value = self[:credential_store] if value.nil? + + # Anything that reads as a boolean is one, matching what to_bool accepts + # for the settings that are declared boolean. Only a value that reads as + # neither names a backend. Compared against ASCII either way, and an + # environment variable can carry bytes that String#downcase would reject. + case value.to_s.b.downcase + when "", "false", "0", "no", "off", "f", "n" then nil + when "true", "1", "yes", "on", "t", "y" then true + else value.to_s + end + end + + # The Gem::CredentialStore class, or nil when the paired RubyGems is too + # old to ship one. In that case the setting is honored as a no-op with a + # one-time warning so a bundle keeps using the config file instead of + # raising. Bundler can run on an older RubyGems than it was released with. + def credential_store_class + return @credential_store_class if defined?(@credential_store_class) + + @credential_store_class = + begin + require "rubygems/credential_store" + Gem::CredentialStore if Gem::CredentialStore.respond_to?(:for) + rescue LoadError + nil + end + + if @credential_store_class.nil? + Bundler.ui.warn "The `credential_store` setting is set but this RubyGems does not provide a credential store. Falling back to the Bundler config file." + elsif @credential_store_class.respond_to?(:warn_handler=) + # Bundler replaces Gem.ui with a Gem::SilentUI subclass, which drops + # alert_warning, so without this every credential store warning is + # lost for the whole command. Resolved late because Bundler.ui is + # replaced during startup. + @credential_store_class.warn_handler = ->(message) { Bundler.ui.warn(message) } + end + + @credential_store_class + end + + # A credential key names a host, either bare ("gems.example.com") or as a + # full URL ("https://gems.example.com/"). + CREDENTIAL_URL_KEY = %r{\Ahttps?://}i + CREDENTIAL_HOST_KEY = /\A[a-z0-9-]+(\.[a-z0-9-]+)+(:\d+)?\z/i + + ## + # True for keys that name a host and can therefore hold a credential, + # like the ones set via `bundle config set gems.example.com user:pass`. + # Deliberately a positive test: a key this version does not recognize + # stays in the config file, where Settings#[] can read it back. Matching + # everything not on the known-settings lists would send values such as + # `ssl_client_cert` to the credential store, and they would then read + # back as nil because only #credentials_for consults the store. + + def credential_store_key?(raw_key) + return false if is_bool(raw_key) || is_num(raw_key) || is_array(raw_key) || is_string(raw_key) || is_credential(raw_key) + + CREDENTIAL_URL_KEY.match?(raw_key) || CREDENTIAL_HOST_KEY.match?(raw_key) + end + + # Removes +key+ from +store+, reporting success when the store cannot be + # reached at all. A store with no usable backend never held the + # credential, so there is nothing there to fail at removing. + def remove_from_store(store, key) + unless store.available? + # Nothing can be removed from a store that cannot be reached, and + # whatever it holds stays there. Reporting a clean removal would be a + # lie, and reporting a failure would be one too, so say which it is. + Bundler.ui.warn "The credential store is enabled but unavailable, so any credential it holds was left in place." + return true + end + + store.delete(key) + end + + # A write clears the plaintext only from the config file it targets, so a + # copy in the other scope survives the move into the store and would come + # back into use the moment the setting is turned off. Name it rather than + # leave it to be discovered. + def warn_plaintext_in_other_scope(raw_key, key, hash) + other, other_file = + if hash.equal?(@local_config) + [@global_config, global_config_file] + else + [@local_config, @local_root.join("config")] + end + + return unless other.key?(key) + + # Deliberately no `bundle config unset` here. That command clears the + # store as well, which would throw away the credential this write has + # moved into it. Only the leftover file entry should go. + safe_key = self.class.remove_userinfo(raw_key) + Bundler.ui.warn "The credential for #{safe_key} moved into the credential store, but a plain text copy" \ + " remains in #{other_file}. Delete the #{key_for(safe_key)} entry from that file to finish the move." + end + + # Reported without the userinfo a URL-shaped key can carry, so a password + # does not end up in a warning that lands in a CI log or a pasted report. + def warn_unremoved_credential(raw_key) + Bundler.ui.warn "Could not remove the credential for #{self.class.remove_userinfo(raw_key)} from the credential store." \ + " It is still there. Remove it with your platform's credential manager." + end + + # The account a credential is stored under. Userinfo is dropped because + # the account reaches the backend as a command argument, where every + # other user on the machine can read it, and a source URL written as + # "https://user:pass@host" would carry the password straight into it. + # The config file path has no such exposure, so its keys are unchanged. + def credential_account(raw_key) + key_for(self.class.remove_userinfo(raw_key)) + end + + # Per-host store selection wants the host whichever form the key took. + def credential_host(raw_key) + return raw_key unless CREDENTIAL_URL_KEY.match?(raw_key) + + require_relative "vendored_uri" + Gem::URI(raw_key).host || raw_key + rescue Gem::URI::Error + raw_key + end + + # A credential given in the environment wins over a stored one. The store + # stands in for the config file, so it must not override the layer that + # already overrides the config file. Otherwise a stale entry on a machine + # would silently beat the credential a CI run passes in. This is only + # consulted when the store answered, so the layer order for everyone who + # has no store stays exactly as it was. + def credentials_from_env(uri) + @env_config[key_for(uri.to_s)] || @env_config[key_for(uri.host)] + end + + def credentials_from_store(uri) + return nil unless store = active_credential_store(uri.host) + + # Normalize with credential_account so a value stored under, say, + # "https://host/" is found when the source URI is "https://host", + # exactly as the config-file path already matches via self[]. + store.get(credential_account(uri.to_s)) || store.get(credential_account(uri.host)) + end + def to_array(value) return [] unless value value.tr(" ", ":").split(":").map(&:to_sym) @@ -398,9 +631,38 @@ def array_to_s(array) def set_key(raw_key, value, hash, file) raw_key = self.class.key_to_s(raw_key) - value = array_to_s(value) if is_array(raw_key) - key = key_for(raw_key) + account = credential_account(raw_key) + + # Only a write that persists to a config file may touch the credential + # store. #temporary passes a nil file, and storing its value would + # outlive the block while its restore pass deleted the real entry. + if file && credential_store_key?(raw_key) && (store = active_credential_store(credential_host(raw_key))) + if value.nil? + warn_unremoved_credential(raw_key) unless remove_from_store(store, account) + elsif value.is_a?(String) && is_userinfo(value) + if store.set(account, value) + # Stored in the credential store, so drop any plaintext copy left + # in this config file by falling through with a nil value. + value = nil + warn_plaintext_in_other_scope(raw_key, key, hash) + else + # Same reason as the branch below: the value is about to go to the + # config file, and #credentials_for reads the store first, so a + # stored value left here would keep winning over it. + warn_unremoved_credential(raw_key) if store.available? && !store.delete(account) + Bundler.ui.warn "Could not write the credential for #{self.class.remove_userinfo(raw_key)} to the credential store," \ + " so it was written to #{file} in plain text." + end + else + # This value goes to the config file, so a stored value for the same + # key must not stay behind. #credentials_for reads the store first, + # so the old secret would keep winning over the new setting. + warn_unremoved_credential(raw_key) unless remove_from_store(store, account) + end + end + + value = array_to_s(value) if is_array(raw_key) return if hash[key] == value @@ -531,6 +793,23 @@ def self.key_for(key) key.gsub(/\A([ #]*)/, '\1BUNDLE_') end + # Drops the userinfo from a URL-shaped key, leaving every other form + # untouched. A key that cannot be parsed is returned as it came, since + # the caller only needs a stable account name. + def self.remove_userinfo(key) + return key unless CREDENTIAL_URL_KEY.match?(key) + + require_relative "vendored_uri" + uri = Gem::URI(key) + return key unless uri.userinfo + + uri = uri.dup + uri.user = uri.password = nil + uri.to_s + rescue Gem::URI::Error + key + end + # TODO: duplicates Rubygems#normalize_uri # TODO: is this the correct place to validate mirror URIs? def self.normalize_uri(uri) diff --git a/lib/rubygems/commands/push_command.rb b/lib/rubygems/commands/push_command.rb index 78fb844eb963..494525d661af 100644 --- a/lib/rubygems/commands/push_command.rb +++ b/lib/rubygems/commands/push_command.rb @@ -17,7 +17,9 @@ def description # :nodoc: The gem can be removed from the index and deleted from the server using the yank command. For further discussion see the help for the yank command. -The push command will use ~/.gem/credentials to authenticate to a server, but you can use the RubyGems environment variable GEM_HOST_API_KEY to set the api key to authenticate. +The push command will use ~/.gem/credentials to authenticate to a server, but you can use the RubyGems environment variable GEM_HOST_API_KEY to set the api key to authenticate. If the :credential_store: gemrc option (or RUBYGEMS_CREDENTIAL_STORE environment variable) is set, the API key is stored in and read from the credential store it selects instead of ~/.gem/credentials. + +The API key to send is resolved in this order: the GEM_HOST_API_KEY environment variable, the --key option, the host's own key in the credential store (when :credential_store: is set), the host's own key in ~/.gem/credentials, then the default RubyGems.org key from either place. The first one found is used. EOF end diff --git a/lib/rubygems/commands/signin_command.rb b/lib/rubygems/commands/signin_command.rb index 0f77908c5bfb..033884474747 100644 --- a/lib/rubygems/commands/signin_command.rb +++ b/lib/rubygems/commands/signin_command.rb @@ -21,7 +21,9 @@ def description # :nodoc: "The signin command executes host sign in for a push server (the default is"\ " https://rubygems.org). The host can be provided with the host flag or can"\ " be inferred from the provided gem. Host resolution matches the resolution"\ - " strategy for the push command." + " strategy for the push command. If the :credential_store: gemrc option (or"\ + " RUBYGEMS_CREDENTIAL_STORE environment variable) is set, the resulting API key is"\ + " stored in the credential store it selects instead of ~/.gem/credentials." end def usage # :nodoc: diff --git a/lib/rubygems/commands/signout_command.rb b/lib/rubygems/commands/signout_command.rb index bdd01e4393f5..529d262d33be 100644 --- a/lib/rubygems/commands/signout_command.rb +++ b/lib/rubygems/commands/signout_command.rb @@ -9,7 +9,10 @@ def initialize def description # :nodoc: "The `signout` command is used to sign out from all current sessions,"\ - " allowing you to sign in using a different set of credentials." + " allowing you to sign in using a different set of credentials. It removes"\ + " the ~/.gem/credentials file. If the :credential_store: gemrc option is"\ + " set, it also removes every RubyGems key from the credential store,"\ + " including keys saved for other hosts with `gem signin --host`." end def usage # :nodoc: @@ -18,14 +21,34 @@ def usage # :nodoc: def execute credentials_path = Gem.configuration.credentials_path + credentials_file_exists = File.exist?(credentials_path) - if !File.exist?(credentials_path) + if !credentials_file_exists && !Gem.configuration.credential_store alert_error "You are not currently signed in." - elsif !File.writable?(credentials_path) - alert_error "File '#{Gem.configuration.credentials_path}' is read-only."\ - " Please make sure it is writable." + return + end + + # Clearing the store does not depend on the credentials file, so an + # unwritable file must not leave the stored keys behind. Each half + # reports its own outcome. + store_cleared, file_removed = Gem.configuration.unset_api_key! + + unremoved = [] + unremoved << "the credential store" unless store_cleared + unremoved << "'#{credentials_path}'" if credentials_file_exists && !file_removed + + unless unremoved.empty? + alert_error "Could not remove the credentials from #{unremoved.join(" and ")}." \ + " They are still there. Check that the file and its directory are writable," \ + " or remove them yourself to finish signing out." + terminate_interaction 1 + end + + # The wording only widens when the store is in play, so anyone who never + # turned it on keeps reading what they always read. + if Gem.configuration.credential_store + say "You have successfully signed out of every registry, including RubyGems.org." else - Gem.configuration.unset_api_key! say "You have successfully signed out from all sessions." end end diff --git a/lib/rubygems/config_file.rb b/lib/rubygems/config_file.rb index 101e9f89ecb0..da5fec597c70 100644 --- a/lib/rubygems/config_file.rb +++ b/lib/rubygems/config_file.rb @@ -65,6 +65,14 @@ class Gem::ConfigFile DEFAULT_INSTALL_EXTENSION_IN_LIB = true DEFAULT_GLOBAL_GEM_CACHE = false DEFAULT_USE_PSYCH = false + DEFAULT_CREDENTIAL_STORE = false + + ## + # The account name under which the default RubyGems.org API key is + # stored in the credential store, mirroring the +:rubygems_api_key+ + # symbol used by the plain text credentials file. + + CREDENTIAL_STORE_DEFAULT_ACCOUNT = "rubygems_api_key" ## # For Ruby packagers to set configuration defaults. Set in @@ -196,6 +204,17 @@ class Gem::ConfigFile attr_reader :ssl_client_cert + ## + # == Experimental == + # Store and read push/authentication credentials in a credential store + # instead of the plain text credentials file. +true+ selects the operating + # system's native store (macOS Keychain, Linux Secret Service, Windows + # Credential Manager) when one is available on this platform. A string + # selects a named backend registered by a third-party gem, such as + # +"1password"+. +false+ (the default) keeps using the credentials file. + + attr_accessor :credential_store + ## # Create the config file object. +args+ is the list of arguments # from the command line. @@ -231,6 +250,7 @@ def initialize(args) @ipv4_fallback_enabled = ENV["IPV4_FALLBACK_ENABLED"] == "true" || DEFAULT_IPV4_FALLBACK_ENABLED @global_gem_cache = ENV["RUBYGEMS_GLOBAL_GEM_CACHE"] == "true" || DEFAULT_GLOBAL_GEM_CACHE @use_psych = ENV["RUBYGEMS_USE_PSYCH"] == "true" || DEFAULT_USE_PSYCH + @credential_store = normalize_credential_store(ENV["RUBYGEMS_CREDENTIAL_STORE"], DEFAULT_CREDENTIAL_STORE) operating_system_config = Marshal.load Marshal.dump(OPERATING_SYSTEM_DEFAULTS) platform_config = Marshal.load Marshal.dump(PLATFORM_DEFAULTS) @@ -253,7 +273,7 @@ def initialize(args) # gemhome and gempath are not working with symbol keys if %w[backtrace bulk_threshold cooldown verbose update_sources cert_expiration_length_days concurrent_downloads install_extension_in_lib ipv4_fallback_enabled - global_gem_cache use_psych sources + global_gem_cache use_psych credential_store sources disable_default_gem_server ssl_verify_mode ssl_ca_cert ssl_client_cert].include?(k) k.to_sym else @@ -273,6 +293,7 @@ def initialize(args) @ipv4_fallback_enabled = @hash[:ipv4_fallback_enabled] if @hash.key? :ipv4_fallback_enabled @global_gem_cache = @hash[:global_gem_cache] if @hash.key? :global_gem_cache @use_psych = @hash[:use_psych] if @hash.key? :use_psych + @credential_store = normalize_credential_store(@hash[:credential_store], @credential_store) if @hash.key? :credential_store @home = @hash[:gemhome] if @hash.key? :gemhome @path = @hash[:gempath] if @hash.key? :gempath @@ -289,7 +310,11 @@ def initialize(args) end ## - # Hash of RubyGems.org and alternate API keys + # Hash of RubyGems.org and alternate API keys, as they appear in the + # credentials file. Keys held in the credential store are not included, so + # this is not the full set of keys a command can authenticate with. Use + # #credential_store_api_key_for or #credential_store_default_api_key to + # reach those. def api_keys load_api_keys unless @api_keys @@ -364,45 +389,168 @@ def load_api_keys def rubygems_api_key load_api_keys unless @rubygems_api_key - @rubygems_api_key + # The key may have moved into the credential store, which #load_api_keys + # does not read: it only sees the credentials file, and storing the key + # removes the plain text copy from it. Without this the accessor would + # return nil in every process after the one that stored the key. The + # store wins, since a key left in the file after the move is stale. + credential_store_default_api_key || @rubygems_api_key end ## # Sets the RubyGems.org API key to +api_key+ def rubygems_api_key=(api_key) + if credential_store + store = active_credential_store + + if api_key.to_s.empty? + # Clearing the key has to reach the store too. #rubygems_api_key reads + # the store first, so a stored key left behind would outrank the empty + # value being set here. + warn_unremoved_credential(CREDENTIAL_STORE_DEFAULT_ACCOUNT) if store&.available? && !store.delete(CREDENTIAL_STORE_DEFAULT_ACCOUNT) + elsif store&.set(CREDENTIAL_STORE_DEFAULT_ACCOUNT, api_key) + remove_api_key_from_file(:rubygems_api_key) + @rubygems_api_key = api_key + return + else + # The key is about to be written to the file, and the reader consults + # the store first, so a stored key left behind here would outrank it + # and the new key would never be used. + warn_unremoved_credential(CREDENTIAL_STORE_DEFAULT_ACCOUNT) if store&.available? && !store.delete(CREDENTIAL_STORE_DEFAULT_ACCOUNT) + warn_credential_store_fallback + end + end + set_api_key :rubygems_api_key, api_key @rubygems_api_key = api_key end + ## + # Looks up +host+'s own API key from the credential store, when the + # #credential_store setting is on. Only the host-specific account is + # consulted: falling back to the default account here would send the + # RubyGems.org key to whatever host was asked for, ahead of that host's own + # key in the credentials file. #credential_store_default_api_key covers the + # default account, at the precedence the credentials file uses for it. + + def credential_store_api_key_for(host) + return nil if host.nil? || host.to_s.empty? + return nil unless credential_store + return nil unless store = active_credential_store + + store.get(self.class.credential_store_account(host)) + end + + ## + # True when a read for +host+ failed rather than finding nothing. Whether + # the store holds a key for it is unknowable once the read fails, which is + # the point: a caller that would otherwise fall through to a key belonging + # to a different host has to treat "unknown" differently from "absent". + + def credential_store_read_failed_for?(host) + return false unless credential_store + return false unless store = active_credential_store + + # The default account holds the RubyGems.org key, and #rubygems_api_key + # reads it on the way to answering. A failure there counts as much as one + # under the host's own name, and for the default host it is the only one + # that can happen, since nothing is ever written under that host name. + return true if store.read_failed?(CREDENTIAL_STORE_DEFAULT_ACCOUNT) + return false if host.nil? || host.to_s.empty? + + store.read_failed?(self.class.credential_store_account(host)) + end + + ## + # The default RubyGems.org API key from the credential store, or +nil+. + # This is the stored counterpart of #rubygems_api_key, and belongs at the + # same point in the lookup order. + + def credential_store_default_api_key + return nil unless credential_store + return nil unless store = active_credential_store + + store.get(CREDENTIAL_STORE_DEFAULT_ACCOUNT) + end + ## # Set a specific host's API key to +api_key+ def set_api_key(host, api_key) + if credential_store && host != :rubygems_api_key + store = active_credential_store + + if api_key.to_s.empty? + # Clearing the key has to reach the store too, since the store is read + # ahead of the file and a leftover entry would outrank the empty value. + delete_stored_key(store, host) + elsif store&.set(self.class.credential_store_account(host), api_key) + remove_api_key_from_file(host) + return + else + # Same reason as in #rubygems_api_key=: the store is read ahead of the + # file, so a stale entry would win over the key written below. + delete_stored_key(store, host) + warn_credential_store_fallback + end + end + check_credentials_permissions - config = load_file(credentials_path).merge(host => api_key) + # Normalized on the way in as well, so a second write lands on the same + # key the first one created instead of adding a near-duplicate entry. + config = load_file(credentials_path).merge(self.class.normalize_credentials_key(host) => api_key) - dirname = File.dirname credentials_path - require "fileutils" - FileUtils.mkdir_p(dirname) - - permissions = 0o600 & ~File.umask - File.open(credentials_path, "w", permissions) do |f| - f.write self.class.dump_with_rubygems_yaml(config) - end + write_credentials(config) load_api_keys # reload end ## - # Remove the +~/.gem/credentials+ file to clear all the current sessions. + # Remove the +~/.gem/credentials+ file to clear all the current sessions, + # and every RubyGems key from the credential store when the + # #credential_store setting is on, including keys saved for other hosts + # with gem signin --host. def unset_api_key! - return false unless File.exist?(credentials_path) + store = active_credential_store + store_cleared = + if store.nil? + true + elsif store.available? + store.delete_all + else + # The setting is on but the backend could not be resolved, so whatever + # it holds cannot be reached, let alone removed. Failing here would + # make signout exit 1 on every platform without a native store, and + # reporting plain success would claim to have removed keys nobody + # looked at. Say what happened and let the sign-out finish. + Gem::CredentialStore.warn_once "The credential store is enabled but unavailable, so any key it holds was left in place." + true + end + + file_removed = + if File.exist?(credentials_path) + # A read-only file deletes without protest on POSIX as long as the + # directory is writable, so the marking has to be honored explicitly. + # Someone who made the file unwritable asked for it to be left alone. + if File.writable?(credentials_path) + begin + File.delete(credentials_path) + true + rescue SystemCallError + false + end + else + false + end + else + false + end - File.delete(credentials_path) + [store_cleared, file_removed] end def load_file(filename) @@ -607,6 +755,38 @@ def self.load_with_rubygems_config_hash(yaml) private + # The account a host is stored under. It starts from the same normalized + # form the credentials file uses, so the two never disagree about which host + # a spelling refers to. Userinfo is then dropped, because the account reaches + # the backend as a command argument where any other user on the machine can + # read it, and a host written as https://user:pass@example would carry the + # password into it. Bundler derives its own accounts the same way. + def self.credential_store_account(host) + host = normalize_credentials_key(host).to_s + return host unless host.match?(%r{\Ahttps?://}i) + + require_relative "vendor/uri/lib/uri" + uri = Gem::URI(host) + return host unless uri.userinfo + + uri = uri.dup + uri.user = uri.password = nil + uri.to_s + rescue Gem::URI::Error + host + end + + # The form +host+ takes once the credentials file has been through key + # normalization, so a lookup finds what #load_file actually returns. A host + # written with a trailing slash, or with the underscore pair that stands in + # for a dot, comes back rewritten, and comparing the raw host against those + # keys silently misses. Symbols already arrive in their final form. + def self.normalize_credentials_key(host) + return host unless host.is_a?(String) + + deep_transform_config_keys!(host => nil).keys.first + end + def self.deep_transform_config_keys!(config) config.transform_keys! do |k| if k.match?(/\A:(.*)\Z/) @@ -647,6 +827,91 @@ def self.deep_transform_config_keys!(config) config end + def active_credential_store + return nil unless credential_store + + require_relative "credential_store" + Gem::CredentialStore.for(credential_store) + end + + # Writes +config+ (a host => key hash) to the credentials file with 0600 + # permissions, creating the directory if needed. + def write_credentials(config) + dirname = File.dirname credentials_path + require "fileutils" + FileUtils.mkdir_p(dirname) + + permissions = 0o600 & ~File.umask + File.open(credentials_path, "w", permissions) do |f| + f.write self.class.dump_with_rubygems_yaml(config) + end + end + + # Drops +host+'s plaintext key from the credentials file once it has moved + # into the credential store, so the secret does not linger on disk. Warns + # rather than failing when the file cannot be rewritten: the key is safely + # in the store either way, but the user still has a plaintext copy to + # delete. Nothing to warn about when the file is absent or has no such key. + def remove_api_key_from_file(host) + return unless File.exist?(credentials_path) + + unless File.writable?(credentials_path) + alert_warning "The API key moved to the credential store but the plain text copy " \ + "in #{credentials_path} could not be removed. Delete it yourself." + return + end + + key = self.class.normalize_credentials_key(host) + config = load_file(credentials_path) + return unless config.key?(key) + + config.delete(key) + write_credentials(config) + load_api_keys + end + + def delete_stored_key(store, host) + account = self.class.credential_store_account(host) + warn_unremoved_credential(account) if store&.available? && !store.delete(account) + end + + # The reader consults the store ahead of the file, so a key that could not + # be removed keeps outranking whatever is written in its place. Say so. + def warn_unremoved_credential(account) + alert_warning "Could not remove the API key for #{account} from the credential store. " \ + "It is still there and will be used instead of the one just set. " \ + "Remove it with your platform's credential manager." + end + + def warn_credential_store_fallback + alert_warning "Could not write the API key to the credential store, so it was written to #{credentials_path} in plain text." + end + + # Interprets a +credential_store+ value from the environment: +"true"+ + # selects the native backend, +"false"+/blank selects +default+, and any + # other value is a backend name passed through as-is. + # Anything that reads as a boolean is one. Only a value that reads as + # neither names a backend, so `RUBYGEMS_CREDENTIAL_STORE=0` turns the store + # off rather than looking for a backend gem called "0". + CREDENTIAL_STORE_OFF = %w[false 0 no off f n].freeze + CREDENTIAL_STORE_ON = %w[true 1 yes on t y].freeze + + def normalize_credential_store(value, default) + # Compared against ASCII either way, and an environment variable can carry + # bytes that String#downcase would reject outright. + normalized = value.to_s.b.downcase + + if normalized.empty? + default + elsif CREDENTIAL_STORE_OFF.include?(normalized) + false + elsif CREDENTIAL_STORE_ON.include?(normalized) + true + else + value + end + end + def set_config_file_name(args) @config_file_name = ENV["GEMRC"] need_config_file_name = false diff --git a/lib/rubygems/credential_store.rb b/lib/rubygems/credential_store.rb new file mode 100644 index 000000000000..e1b0e9fcdeb5 --- /dev/null +++ b/lib/rubygems/credential_store.rb @@ -0,0 +1,342 @@ +# frozen_string_literal: true + +# Skip reloading when an identical copy (e.g. the one shipped inside the Bundler +# gem) was already required from a different path, to avoid redefinition warnings. +return if defined?(Gem::CredentialStore::SERVICE_NAME) + +## +# Gem::CredentialStore is opt-in storage for authentication secrets (API +# keys, host credentials) in the operating system's native secret store +# instead of a plain text file: +# +# * macOS: Keychain, via the +security+ command line tool. +# * Linux: the Secret Service API (GNOME Keyring, KWallet, ...), via +# +secret-tool+. +# * Windows: Credential Manager, via the +Windows.Security.Credentials.PasswordVault+ +# API from PowerShell. +# +# A third party can add another backend (1Password, pass, HashiCorp Vault, +# ...) by shipping a gem that provides +# rubygems/credential_store/backends/ and calls +# .register_backend from it. Users then select it by name instead of +true+ +# (see .resolve_backend). +# +# Every public method traps all errors and returns +nil+/+false+ instead of +# raising, so that callers can transparently fall back to their existing +# file-based storage when the native store is unavailable or fails (a +# locked keychain over SSH, a headless Linux session without a keyring +# daemon, ...). + +class Gem::CredentialStore + SERVICE_NAME = "rubygems" + + ## + # Returns the store to use for +spec+, or +nil+ when the credential store + # is off. +spec+ is either +true+ (use this platform's native backend) or + # the name of a registered backend such as "1password". +service+ names + # the account namespace within the backend, so RubyGems and Bundler keep + # separate credentials in one native store. The store is memoized per + # +spec+ and +service+ for the life of the process, so the read cache and + # any expensive backend startup are shared across callers. A test may + # install a stand-in via #instance= that is returned here for any enabled + # +spec+, or inject a shared backend via #backend=. + + def self.for(spec, service: SERVICE_NAME) + return nil unless spec + return @override if defined?(@override) && @override + + backend = defined?(@override_backend) && @override_backend ? @override_backend : backend_for(spec) + (@instances ||= {})[[spec, service]] ||= new(backend: backend, service: service) + end + + ## + # The default-backed store for this platform, i.e. for(true). + # Kept for callers and tests that only care about the native backend. + + def self.instance + self.for(true) + end + + ## + # Installs a stand-in store that .for returns for any enabled setting. + # Intended for tests that inject a fake backend. + + def self.instance=(store) + @override = store + end + + ## + # Installs a shared backend that .for wraps for every spec and service. + # Intended for tests that need RubyGems and Bundler credentials to land in + # one backend under their own service names. + + def self.backend=(backend) + @override_backend = backend + end + + ## + # Clears the memoized stores, the injected overrides, and the warned + # messages. Intended for tests only. + + def self.reset! + @override = nil + @override_backend = nil + @instances = nil + @warned = nil + @warn_handler = nil + end + + ## + # Warns once per distinct message. A single flag for every message would + # let an early warning about, say, a misspelled backend name suppress the + # later warning that a secret was written in plain text. + + def self.warn_once(message) + @warned ||= {} + return if @warned.key?(message) + + @warned[message] = true + + if defined?(@warn_handler) && @warn_handler + @warn_handler.call(message) + else + Gem.ui.alert_warning message + end + end + + ## + # Sends warnings to +handler+ (anything responding to #call) instead of + # Gem.ui. Bundler sets this because it replaces Gem.ui with a subclass of + # Gem::SilentUI, which discards alert_warning entirely, so a credential + # store failure would otherwise be silent for the whole bundle command. + + def self.warn_handler=(handler) + @warn_handler = handler + end + + ## + # Registers +backend+ under +name+ so it can be selected with + # credential_store = . A third-party backend gem calls this + # from the file RubyGems loads for that name (see .resolve_backend). + + def self.register_backend(name, backend) + (@backends ||= {})[name.to_s] = backend + end + + BACKEND_NAME = /\A[a-z0-9_-]+\z/ + + ## + # Resolves a registered backend by +name+, requiring + # rubygems/credential_store/backends/ on first use so a + # backend shipped as its own gem loads only when actually selected. + # Returns +nil+ (warning once) when the name is malformed or no gem + # provides it, which makes callers fall back to file storage. The fixed + # require prefix and the restricted name charset keep the setting a piece + # of data, never a path or a command. + + def self.resolve_backend(name) + # Compared against ASCII either way, and the setting can carry bytes that + # Regexp#match? would reject outright. A name that gets past the match is + # ASCII only, so interpolating it into the require path stays sound. + name = name.to_s.b + unless BACKEND_NAME.match?(name) + warn_once "Ignoring invalid credential store backend name #{name.inspect}." + return nil + end + + return @backends[name] if @backends&.key?(name) + + begin + require "rubygems/credential_store/backends/#{name}" + rescue LoadError + warn_once "Credential store backend #{name.inspect} is not installed. " \ + "Install a gem that provides rubygems/credential_store/backends/#{name}, " \ + "or unset the credential_store setting. Falling back to file storage." + return nil + end + + @backends && @backends[name] + end + + def self.backend_for(spec) + spec == true ? default_backend : resolve_backend(spec) + end + private_class_method :backend_for + + def self.default_backend + if Gem.win_platform? + require_relative "credential_store/native/windows" + WindowsBackend + elsif RUBY_PLATFORM.include?("darwin") + require_relative "credential_store/native/macos" + MacOSBackend + elsif RUBY_PLATFORM.include?("linux") + require_relative "credential_store/native/linux" + LinuxBackend if LinuxBackend.available? + end + end + + ## + # +backend+ is only used by tests to inject a fake backend regardless of + # the platform the test suite happens to run on. +service+ is the account + # namespace this store reads and writes under. + + def initialize(backend: self.class.default_backend, service: SERVICE_NAME) + @backend = backend + @service = service + @cache = {} + end + + ## + # True if a native credential backend is usable on this platform. + + def available? + !@backend.nil? + end + + ## + # Returns the secret stored for +account+, or +nil+ if there is none or + # the backend is unavailable/fails. + + def get(account) + return nil unless @backend + return @cache[account] if @cache.key?(account) + + @cache[account] = @backend.get(@service, account) + rescue StandardError => e + warn_failure(:read, e) + # Remember the failure too. Retrying the same account means another + # subprocess and, on some platforms, another authorization prompt for + # every lookup. #read_failed? keeps this distinguishable from an account + # the store simply does not hold. + (@failed ||= {})[account] = true + @cache[account] = nil + end + + ## + # True when #get returned +nil+ for +account+ because the backend could not + # answer, rather than because nothing is stored under it. Callers that would + # otherwise fall back to a different credential need the difference: a + # missing entry means "use something else", an unreadable one does not. + + def read_failed?(account) + return false unless defined?(@failed) && @failed + + @failed.key?(account) + end + + ## + # Stores +secret+ for +account+. Returns +true+ on success. + + def set(account, secret) + return false unless @backend + + validate_credential(account, secret) + + if @backend.set(@service, account, secret) + @cache[account] = secret + @failed&.delete(account) + invalidate_list + true + else + false + end + rescue StandardError => e + warn_failure(:write, e) + false + end + + ## + # Removes the secret stored for +account+. Returns +true+ if the entry is + # gone, whether or not it existed beforehand. + + def delete(account) + return false unless @backend + + result = @backend.delete(@service, account) + @cache.delete(account) + @failed&.delete(account) + invalidate_list + result + rescue StandardError => e + warn_failure(:remove, e) + false + end + + ## + # The accounts this store holds, or +nil+ when the backend cannot + # enumerate them. Listing is optional in the backend protocol: the native + # backends implement it, but a third-party backend that only resolves + # credentials on demand has nothing to enumerate. Callers must treat +nil+ + # as "unknown", not as "empty". Secrets are never returned. + + def list + return nil unless @backend.respond_to?(:list) + return @list if defined?(@list) + + @list = @backend.list(@service) + rescue StandardError => e + warn_failure(:list, e) + # Remembered for the same reason #get remembers a failed read. + @list = nil + end + + ## + # Removes every entry this store owns (all accounts under its service). + # Returns +true+ when the store is now clear. Used by +gem signout+ to end + # every session at once, mirroring deletion of the whole credentials file. + + def delete_all + return false unless @backend + + result = @backend.delete_all(@service) + @cache.clear + @failed = nil + invalidate_list + result + rescue StandardError => e + warn_failure(:remove, e) + false + end + + private + + # The listing is memoized for the life of the store, so any write has to + # drop it or the next list would report the state from before the write. + def invalidate_list + remove_instance_variable(:@list) if defined?(@list) + end + + # What happens after a failure depends on the operation, so the warning has + # to say the right thing for each. A failed write really does end up in the + # config file, but a failed read does not come back with the credential the + # file no longer holds, and a failed removal leaves the secret where it was. + # A secret has to survive the round trip through whichever backend is in + # use, and the macOS keychain hands non-printable bytes back as hex through + # the only read-back its CLI offers. Refusing them here rather than in that + # one backend means the same value is stored, or refused for the same + # reason, on every platform. The newline rules go with it: a newline in an + # account would start a second command in the macOS batch input. + PRINTABLE_ASCII = /\A[\x20-\x7e]*\z/ + + OUTCOMES = { + read: "any copy left in the config file will be used instead", + write: "falling back to file storage", + remove: "the credential is still in the store", + list: "stored credentials will not be listed", + }.freeze + + # Raises when the pair cannot be stored faithfully. #set turns that back + # into a warning and a false, which is the same answer a backend gives when + # it refuses the write, so the caller falls back to the file either way. + def validate_credential(account, secret) + raise ArgumentError, "credential secret must be printable ASCII" unless secret.to_s.b.match?(PRINTABLE_ASCII) + raise ArgumentError, "credential account must not contain a newline" if account.to_s.include?("\n") + raise ArgumentError, "credential service must not contain a newline" if @service.to_s.include?("\n") + end + + def warn_failure(operation, error) + self.class.warn_once "Credential store #{operation} failed for #{@service}" \ + " (#{error.class}: #{error.message}); #{OUTCOMES[operation]}." + end +end diff --git a/lib/rubygems/credential_store/native/linux.rb b/lib/rubygems/credential_store/native/linux.rb new file mode 100644 index 000000000000..afff97352a78 --- /dev/null +++ b/lib/rubygems/credential_store/native/linux.rb @@ -0,0 +1,106 @@ +# frozen_string_literal: true + +require "open3" + +class Gem::CredentialStore; end unless defined?(Gem::CredentialStore) + +## +# Stores credentials in the Secret Service API (GNOME Keyring, KWallet, +# ...) via the +secret-tool+ command line tool from libsecret. + +class Gem::CredentialStore::LinuxBackend + # secret-tool prints an item's attributes to stderr, one per line. + ACCOUNT_ATTRIBUTE = /^attribute\.account = (.*)$/ + def self.available? + return @available if defined?(@available) + + @available = ENV["PATH"].to_s.split(File::PATH_SEPARATOR).any? do |dir| + File.executable?(File.join(dir, "secret-tool")) + end + end + + ## + # Clears the memoized #available? result. Intended for tests only. + + def self.reset! + remove_instance_variable(:@available) if defined?(@available) + end + + def self.get(service, account) + out, err, status = Open3.capture3( + "secret-tool", "lookup", "service", service, "account", account + ) + # secret-tool exits 1 with nothing on stderr when the entry is simply + # absent. Anything else is a real failure, so raise and let the wrapper + # report it rather than authenticating with no credential at all. + unless status.success? + return nil if status.exitstatus == 1 && err.to_s.strip.empty? + + raise "secret-tool exited with #{status.exitstatus}: #{err.strip}" + end + + secret = out.chomp + secret.empty? ? nil : secret + end + + def self.set(service, account, secret) + _out, status = Open3.capture2( + "secret-tool", "store", "--label=RubyGems", "service", service, "account", account, + stdin_data: secret + ) + status.success? + end + + # The accounts stored under +service+. secret-tool writes the attributes to + # stderr and the secrets themselves to stdout, so reading the accounts means + # reading stderr. stdout is discarded, which also keeps a secret containing + # a newline from being mistaken for an attribute line. + def self.list(service) + _out, err, status = Open3.capture3( + "secret-tool", "search", "--all", "service", service + ) + return [] unless status.success? + + err.scan(ACCOUNT_ATTRIBUTE).flatten.uniq + end + + def self.delete(service, account) + _out, err, status = Open3.capture3( + "secret-tool", "clear", "service", service, "account", account + ) + return cleared?(service, account) if status.success? + + # secret-tool clear exits 1 with no stderr when nothing matched. + status.exitstatus == 1 && err.to_s.strip.empty? + end + + # secret-tool clear removes every item matching the attributes, so a clear + # keyed on the service alone empties just that service. + def self.delete_all(service) + _out, err, status = Open3.capture3( + "secret-tool", "clear", "service", service + ) + return cleared?(service) if status.success? + + # Exits 1 with no stderr when there was nothing to clear. + status.exitstatus == 1 && err.to_s.strip.empty? + end + + # libsecret clears only unlocked items and reports no error for the ones it + # skipped, so a locked keyring answers a clear with success while keeping + # every secret. Ask what is left rather than trust that answer, otherwise + # gem signout tells the user it removed keys that are still there. search + # exits zero whether or not it matched anything, so the accounts it reports + # are the answer, and a search that fails outright leaves the question open + # and is reported as not cleared. + def self.cleared?(service, account = nil) + _out, err, status = Open3.capture3( + "secret-tool", "search", "--all", "service", service + ) + return false unless status.success? + + remaining = err.scan(ACCOUNT_ATTRIBUTE).flatten + account ? !remaining.include?(account) : remaining.empty? + end + private_class_method :cleared? +end diff --git a/lib/rubygems/credential_store/native/macos.rb b/lib/rubygems/credential_store/native/macos.rb new file mode 100644 index 000000000000..4071dc573ea8 --- /dev/null +++ b/lib/rubygems/credential_store/native/macos.rb @@ -0,0 +1,93 @@ +# frozen_string_literal: true + +require "open3" + +class Gem::CredentialStore; end unless defined?(Gem::CredentialStore) + +## +# Stores credentials in the macOS Keychain via the +security+ command line +# tool. +security+ has no way to read a password from stdin as raw bytes +# for +add-generic-password+, so #set uses +security -i+ (batch/interactive +# mode, one tokenized command per stdin line) to keep the secret off argv +# and out of +ps+ output. +# +# A newline would start a second command in the +security -i+ batch, and +# +security find-generic-password -w+ prints any non-printable byte back as a +# hex string rather than the original value, so a non-ASCII secret would +# round-trip corrupted. Those are the limits Gem::CredentialStore#set enforces +# for every backend, so the caller falls back to file storage rather than +# storing something that cannot be read back. + +class Gem::CredentialStore::MacOSBackend + NOT_FOUND_STATUS = 44 + + def self.get(service, account) + out, err, status = Open3.capture3( + "security", "find-generic-password", "-a", account, "-s", service, "-w" + ) + # A locked keychain and an absent entry both yield no secret, but only the + # second one is ordinary. Raise on the rest so the wrapper can say why the + # credential could not be read instead of silently authenticating without + # one. + unless status.success? + return nil if status.exitstatus == NOT_FOUND_STATUS + + raise "security exited with #{status.exitstatus}: #{err.strip}" + end + + secret = out.chomp + secret.empty? ? nil : secret + end + + def self.set(service, account, secret) + command = "add-generic-password -U -a #{quote(account)} -s #{quote(service)} -w #{quote(secret)}\n" + _out, err, status = Open3.capture3("security", "-i", stdin_data: command) + return true if status.success? + + # Raise rather than return false so the reason reaches the user: the + # wrapper turns this back into false after reporting it, and a bare + # false would leave "could not write" with no explanation. + raise "security exited with #{status.exitstatus}: #{err.strip}" + end + + # The accounts stored under +service+. security has no "list by service" + # subcommand, so this reads the dump, which reports attributes but never + # the secrets themselves, and keeps the entries whose service matches. + def self.list(service) + out, status = Open3.capture2("security", "dump-keychain", err: File::NULL) + return [] unless status.success? + + out.split(/^keychain: /).filter_map do |entry| + next unless entry[/^\s*"svce"="(.*)"$/, 1] == service + + entry[/^\s*"acct"="(.*)"$/, 1] + end.uniq + end + + def self.delete(service, account) + _out, status = Open3.capture2( + "security", "delete-generic-password", "-a", account, "-s", service, + err: File::NULL + ) + status.success? || status.exitstatus == NOT_FOUND_STATUS + end + + # security deletes one entry per call, so keep deleting the given service + # until it reports there is nothing left (exit 44). This only touches the + # given service and leaves entries for other services intact. + def self.delete_all(service) + loop do + _out, status = Open3.capture2( + "security", "delete-generic-password", "-s", service, + err: File::NULL + ) + return true if status.exitstatus == NOT_FOUND_STATUS + return false unless status.success? + end + end + + def self.quote(value) + %("#{value.gsub("\\", "\\\\\\\\").gsub('"', '\\"')}") + end + private_class_method :quote +end diff --git a/lib/rubygems/credential_store/native/windows.rb b/lib/rubygems/credential_store/native/windows.rb new file mode 100644 index 000000000000..9480a8c36585 --- /dev/null +++ b/lib/rubygems/credential_store/native/windows.rb @@ -0,0 +1,137 @@ +# frozen_string_literal: true + +require "open3" + +class Gem::CredentialStore; end unless defined?(Gem::CredentialStore) + +## +# Stores credentials in the Windows Credential Manager via the +# +Windows.Security.Credentials.PasswordVault+ WinRT API, driven from +# PowerShell. Account/service/secret values are passed as environment +# variables rather than interpolated into the script text, so no quoting +# scheme is needed and values cannot break out of the script. +# +# Windows PowerShell is used rather than PowerShell 7 (+pwsh+) because the +# WinRT projection used here is not reliably available under pwsh. It is +# spawned as +powershell+ rather than +powershell.exe+, the way this codebase +# spawns +git+, so PATHEXT resolves it. That also lets the tests put a shim +# ahead of it on Windows, where a file with a shebang is not executable. + +class Gem::CredentialStore::WindowsBackend + LOAD_VAULT_TYPE = <<~POWERSHELL + $ErrorActionPreference = 'Stop' + [void][Windows.Security.Credentials.PasswordVault,Windows.Security.Credentials,ContentType=WindowsRuntime] + POWERSHELL + private_constant :LOAD_VAULT_TYPE + + def self.get(service, account) + script = <<~POWERSHELL + #{LOAD_VAULT_TYPE} + $vault = New-Object Windows.Security.Credentials.PasswordVault + $credential = $vault.Retrieve($env:RUBYGEMS_CRED_SERVICE, $env:RUBYGEMS_CRED_ACCOUNT) + # Write straight to the console stream. Emitting the string into the + # pipeline would send it through PowerShell's formatter, which wraps + # long lines at the host width and would corrupt a long secret. + [Console]::Out.Write($credential.Password) + POWERSHELL + + out, err, status = run(script, service, account) + # An absent credential is ordinary. Any other failure is not, so raise and + # let the wrapper report why rather than authenticating without one. + unless status.success? + return nil if missing_credential?(err) + + raise "powershell exited with #{status.exitstatus}: #{err.strip}" + end + + secret = out.chomp + secret.empty? ? nil : secret + end + + def self.set(service, account, secret) + script = <<~POWERSHELL + #{LOAD_VAULT_TYPE} + $vault = New-Object Windows.Security.Credentials.PasswordVault + try { + $existing = $vault.Retrieve($env:RUBYGEMS_CRED_SERVICE, $env:RUBYGEMS_CRED_ACCOUNT) + $vault.Remove($existing) + } catch {} + $credential = New-Object Windows.Security.Credentials.PasswordCredential($env:RUBYGEMS_CRED_SERVICE, $env:RUBYGEMS_CRED_ACCOUNT, $env:RUBYGEMS_CRED_SECRET) + $vault.Add($credential) + POWERSHELL + + _out, err, status = run(script, service, account, secret) + return true if status.success? + + # Raise rather than return false so the reason reaches the user: the + # wrapper turns this back into false after reporting it, and a bare + # false would leave "could not write" with no explanation. + raise "powershell exited with #{status.exitstatus}: #{err.strip}" + end + + def self.delete(service, account) + script = <<~POWERSHELL + #{LOAD_VAULT_TYPE} + $vault = New-Object Windows.Security.Credentials.PasswordVault + $credential = $vault.Retrieve($env:RUBYGEMS_CRED_SERVICE, $env:RUBYGEMS_CRED_ACCOUNT) + $vault.Remove($credential) + POWERSHELL + + _out, err, status = run(script, service, account) + status.success? || missing_credential?(err) + end + + # The accounts stored under +service+. FindAllByResource raises when the + # resource has no entries, which is an empty list rather than an error. + # Only the user names are read; the passwords stay in the vault. + def self.list(service) + script = <<~POWERSHELL + #{LOAD_VAULT_TYPE} + $vault = New-Object Windows.Security.Credentials.PasswordVault + try { + $vault.FindAllByResource($env:RUBYGEMS_CRED_SERVICE) | ForEach-Object { + [Console]::Out.WriteLine($_.UserName) + } + } catch { + if (-not ($_.Exception.Message -match 'not found|0x80070490')) { throw } + } + POWERSHELL + + out, _err, status = run(script, service, nil) + return [] unless status.success? + + out.split("\n").map(&:chomp).reject(&:empty?).uniq + end + + # Removes every credential stored under the given resource (service), + # leaving other resources untouched. FindAllByResource raises when the + # resource has no entries, which is treated as an empty, successful clear. + def self.delete_all(service) + script = <<~POWERSHELL + #{LOAD_VAULT_TYPE} + $vault = New-Object Windows.Security.Credentials.PasswordVault + try { + $vault.FindAllByResource($env:RUBYGEMS_CRED_SERVICE) | ForEach-Object { $vault.Remove($_) } + } catch { + if (-not ($_.Exception.Message -match 'not found|0x80070490')) { throw } + } + POWERSHELL + + _out, err, status = run(script, service, nil) + status.success? || missing_credential?(err) + end + + def self.run(script, service, account, secret = nil) + env = { "RUBYGEMS_CRED_SERVICE" => service, "RUBYGEMS_CRED_ACCOUNT" => account } + env["RUBYGEMS_CRED_SECRET"] = secret if secret + + Open3.capture3(env, "powershell", "-NoProfile", "-NonInteractive", "-Command", "-", stdin_data: script) + end + private_class_method :run + + def self.missing_credential?(message) + text = message.to_s.downcase + text.include?("element not found") || text.include?("0x80070490") || text.include?("could not be found") + end + private_class_method :missing_credential? +end diff --git a/lib/rubygems/gemcutter_utilities.rb b/lib/rubygems/gemcutter_utilities.rb index 9c22c14fad5b..f14f2a96bcab 100644 --- a/lib/rubygems/gemcutter_utilities.rb +++ b/lib/rubygems/gemcutter_utilities.rb @@ -48,10 +48,28 @@ def api_key ENV["GEM_HOST_API_KEY"] elsif options[:key] verify_api_key options[:key] + elsif credential_store_key = Gem.configuration.credential_store_api_key_for(host) + credential_store_key elsif Gem.configuration.api_keys.key?(host) Gem.configuration.api_keys[host] else - Gem.configuration.rubygems_api_key + key = Gem.configuration.rubygems_api_key + + # Two ways this last resort goes wrong once the store has refused to + # answer. Handing the RubyGems.org key to another host is one, since + # that host may well have a key of its own that simply could not be + # read. Coming away with nothing is the other: the caller reads that as + # "not signed in" and asks for a password, so a locked store quietly + # downgrades key authentication to the account password. A key found + # here despite the failure came from the credentials file, and for the + # default host that is exactly the key to use. + if !@recognizing_session && Gem.configuration.credential_store_read_failed_for?(host) && (key.nil? || !default_host?) + alert_error "The credential store could not be read, so no API key for #{host} could be found. " \ + "Make the store readable and run the command again." + terminate_interaction ERROR_CODE + end + + key end end @@ -155,7 +173,19 @@ def update_scope(scope) def sign_in(sign_in_host = nil, scope: nil) sign_in_host ||= host pretty_host = pretty_host(sign_in_host) - if api_key + # Asking here is only how we recognize an existing session. Stopping + # because the store cannot be read would close the one command that can + # re-authenticate, so this lookup gets to come back empty instead. The + # flag rather than an argument keeps #api_key callable with no arguments, + # which is how command plugins that override it define it. + @recognizing_session = true + signed_in = begin + api_key + ensure + @recognizing_session = false + end + + if signed_in say "You are already signed in on #{pretty_host}." return end @@ -196,12 +226,31 @@ def sign_in(sign_in_host = nil, scope: nil) def verify_api_key(key) if Gem.configuration.api_keys.key? key Gem.configuration.api_keys[key] + elsif stored_key = stored_api_key_named(key) + stored_key else alert_error "No such API key. Please add it to your configuration (done automatically on initial `gem push`)." terminate_interaction(ERROR_CODE) end end + ## + # The default key, when +name+ is the name the credentials file knows it by. + # That file renames :rubygems_api_key to :rubygems on the way in, so the name + # survives only there, and moving the key into the store would otherwise put + # it out of reach of --key. + # + # Only that one name. The store is keyed by host, and --key names a key, so + # looking any other name up there would let --key reach a host's key and send + # it somewhere else. The credentials file keeps the two apart by type, since + # --key arrives as a Symbol and host entries are strings. + + def stored_api_key_named(name) + return nil unless name.to_s == "rubygems" + + Gem.configuration.credential_store_default_api_key + end + ## # If +response+ is an HTTP Success (2XX) response, yields the response if a # block was given or shows the response body to the user. diff --git a/spec/bundler/settings_spec.rb b/spec/bundler/settings_spec.rb index 9ac3603caf11..cb689fe167d3 100644 --- a/spec/bundler/settings_spec.rb +++ b/spec/bundler/settings_spec.rb @@ -1,6 +1,8 @@ # frozen_string_literal: true require "bundler/settings" +require "rubygems/credential_store" +require_relative "../support/fake_credential_backend" RSpec.describe Bundler::Settings do subject(:settings) { described_class.new(bundled_app) } @@ -276,6 +278,409 @@ expect(settings.credentials_for(uri)).to eq(credentials) end end + + context "with credential_store enabled" do + let(:fake_store) { Gem::CredentialStore.new(backend: FakeCredentialBackend.new) } + + before do + settings.set_local "credential_store", "true" + Gem::CredentialStore.instance = fake_store + end + + after { Gem::CredentialStore.reset! } + + it "returns nil when nothing is configured anywhere" do + expect(settings.credentials_for(uri)).to be_nil + end + + it "round-trips credentials set under the full URL" do + settings.set_local "https://gemserver.example.org/", credentials + + expect(settings.credentials_for(uri)).to eq(credentials) + end + + it "round-trips credentials set under the hostname" do + settings.set_local "gemserver.example.org", credentials + + expect(settings.credentials_for(uri)).to eq(credentials) + end + + it "keeps a password in the source URL out of the store account" do + # The account reaches the backend as a command argument, where any + # other user on the machine can read it. + recorder = Class.new(FakeCredentialBackend) do + def accounts_seen + @accounts_seen ||= [] + end + + def get(service, account) + accounts_seen << account + super + end + end.new + Gem::CredentialStore.instance = Gem::CredentialStore.new(backend: recorder) + settings.set_local "gemserver.example.org", credentials + + with_auth = Gem::URI("https://someone:s3cr3t@gemserver.example.org") + + expect(settings.credentials_for(with_auth)).to eq(credentials) + # key_for upcases, so compare without regard to case. + expect(recorder.accounts_seen).not_to be_empty + expect(recorder.accounts_seen.join.downcase).not_to include("s3cr3t") + end + + it "matches a URL key regardless of a trailing slash, like the config file does" do + settings.set_local "https://gemserver.example.org", credentials + + expect(settings.credentials_for(Gem::URI("https://gemserver.example.org/"))).to eq(credentials) + end + + it "does not write the secret to the local config file" do + settings.set_local "gemserver.example.org", credentials + + expect(settings.locations("gemserver.example.org")[:local]).to be_nil + end + + it "prefers a credential given in the environment over the credential_store" do + settings.set_local "gemserver.example.org", credentials + + ENV["BUNDLE_GEMSERVER__EXAMPLE__ORG"] = "ci:token" + env_settings = Bundler::Settings.new(bundled_app) + + expect(env_settings.credentials_for(uri)).to eq("ci:token") + end + + it "leaves the layer order alone for a host the store does not hold" do + # Written straight to the config file so the store never holds it. + # Resolution must then be exactly what it was before the store + # existed, with local beating env. + allow(settings).to receive(:active_credential_store).and_return(nil) + settings.set_local "other.example.org", "local:pass" + allow(settings).to receive(:active_credential_store).and_call_original + + ENV["BUNDLE_OTHER__EXAMPLE__ORG"] = "env:pass" + env_settings = Bundler::Settings.new(bundled_app) + + expect(env_settings.credentials_for(Gem::URI("https://other.example.org/"))).to eq("local:pass") + end + + it "falls back to the credential_store when the environment has no entry" do + settings.set_local "gemserver.example.org", credentials + + ENV["BUNDLE_OTHER__EXAMPLE__ORG"] = "ci:token" + env_settings = Bundler::Settings.new(bundled_app) + + expect(env_settings.credentials_for(uri)).to eq(credentials) + end + + it "prefers the credential_store over a stale local config value" do + # A plain-text credential left in the config file before the store + # was enabled, simulated by routing this one write to the file. + allow(settings).to receive(:active_credential_store).and_return(nil) + settings.set_local "gemserver.example.org", "stale:value" + allow(settings).to receive(:active_credential_store).and_call_original + + settings.set_local "gemserver.example.org", credentials + + expect(settings.credentials_for(uri)).to eq(credentials) + end + end + + context "with a named credential_store backend" do + let(:fake_store) { Gem::CredentialStore.new(backend: FakeCredentialBackend.new) } + + before do + settings.set_local "credential_store", "1password" + Gem::CredentialStore.instance = fake_store + end + + after { Gem::CredentialStore.reset! } + + it "round-trips credentials through the selected backend" do + settings.set_local "gemserver.example.org", credentials + + expect(settings.credentials_for(uri)).to eq(credentials) + end + end + + context "with a per-host credential_store backend" do + let(:host_backend) { FakeCredentialBackend.new } + let(:global_backend) { FakeCredentialBackend.new } + let(:service) { Bundler::Settings::CREDENTIAL_STORE_SERVICE } + + before do + Gem::CredentialStore.register_backend("fake-host", host_backend) + Gem::CredentialStore.register_backend("fake-global", global_backend) + settings.set_local "credential_store", "fake-global" + settings.set_local "credential_store.gemserver.example.org", "fake-host" + end + + after { Gem::CredentialStore.reset! } + + it "reads the host's credentials from the backend selected for that host" do + host_backend.set(service, Bundler::Settings.key_for("gemserver.example.org"), credentials) + + expect(settings.credentials_for(uri)).to eq(credentials) + end + + it "does not chain to the global backend when the host's backend misses" do + global_backend.set(service, Bundler::Settings.key_for("gemserver.example.org"), credentials) + + expect(settings.credentials_for(uri)).to be_nil + end + + it "keeps other hosts on the globally selected backend" do + global_backend.set(service, Bundler::Settings.key_for("other.example.org"), credentials) + + expect(settings.credentials_for(Gem::URI("https://other.example.org/"))).to eq(credentials) + end + + it "writes a host credential to the backend selected for that host" do + settings.set_local "gemserver.example.org", credentials + + expect(host_backend.get(service, Bundler::Settings.key_for("gemserver.example.org"))).to eq(credentials) + expect(global_backend.get(service, Bundler::Settings.key_for("gemserver.example.org"))).to be_nil + end + + it "writes a URL-keyed credential to the backend selected for its host" do + settings.set_local "https://gemserver.example.org/", credentials + + expect(host_backend.get(service, Bundler::Settings.key_for("https://gemserver.example.org/"))).to eq(credentials) + end + + it "keeps a host on the config file when its store is set to false" do + settings.set_local "credential_store.gemserver.example.org", "false" + + settings.set_local "gemserver.example.org", credentials + + expect(settings.locations("gemserver.example.org")[:local]).to eq(credentials) + expect(settings.credentials_for(uri)).to eq(credentials) + end + end + + context "with credential_store set to false" do + before { settings.set_local "credential_store", "false" } + + it "does not consult a credential store" do + expect(Gem::CredentialStore).not_to receive(:for) + expect(settings.credentials_for(uri)).to be_nil + end + end + + context "when the paired RubyGems has no credential store" do + before do + settings.set_local "credential_store", "true" + allow(settings).to receive(:require).and_call_original + allow(settings).to receive(:require).with("rubygems/credential_store").and_raise(LoadError) + end + + it "warns once and falls back to the config file without raising" do + allow(Bundler.ui).to receive(:warn) + + expect { settings.set_local "gemserver.example.org", "username:password" }.not_to raise_error + expect(settings.credentials_for(uri)).to eq("username:password") + + expect(Bundler.ui).to have_received(:warn).once + end + end + end + + describe "credential storage with credential_store enabled" do + let(:fake_store) { Gem::CredentialStore.new(backend: FakeCredentialBackend.new) } + + before do + settings.set_local "credential_store", "true" + Gem::CredentialStore.instance = fake_store + end + + after { Gem::CredentialStore.reset! } + + it "writes a host credential to the credential_store instead of the local config file" do + settings.set_local "gemserver.example.org", "username:password" + + expect(fake_store.get(Bundler::Settings.key_for("gemserver.example.org"))).to eq("username:password") + expect(settings.locations("gemserver.example.org")[:local]).to be_nil + end + + it "leaves the credential_store alone for temporary settings" do + settings.set_local "gemserver.example.org", "username:password" + stored = Bundler::Settings.key_for("gemserver.example.org") + + settings.temporary("gemserver.example.org" => "temp:value") do + # The temporary value must not be persisted to the OS store... + expect(fake_store.get(stored)).to eq("username:password") + end + + # ...and restoring it must not delete the real credential either. + expect(fake_store.get(stored)).to eq("username:password") + end + + it "names the host and the file it fell back to when the store write fails" do + Gem::CredentialStore.instance = Gem::CredentialStore.new(backend: nil) + allow(Bundler.ui).to receive(:warn) + + settings.set_local "gemserver.example.org", "username:password" + + expect(Bundler.ui).to have_received(:warn). + with(%r{credential for gemserver\.example\.org .* #{Regexp.escape(bundled_app.to_s)}/config in plain text}m) + end + + it "warns when the credential cannot be removed from the credential_store" do + # A usable backend that refuses to delete. A missing backend never held + # the credential, so that case must stay silent. + refusing = Class.new(FakeCredentialBackend) do + def delete(_service, _account) + false + end + end.new + Gem::CredentialStore.instance = Gem::CredentialStore.new(backend: refusing) + allow(Bundler.ui).to receive(:warn) + + settings.set_local "gemserver.example.org", nil + + expect(Bundler.ui).to have_received(:warn).with(/Could not remove the credential for gemserver\.example\.org/) + end + + it "says the store was unreachable rather than claiming a removal" do + # Nothing can be removed from a store that cannot be reached. Reporting + # a clean removal would be a lie, and reporting a failure would be one + # too, so the warning says which it is. + Gem::CredentialStore.instance = Gem::CredentialStore.new(backend: nil) + allow(Bundler.ui).to receive(:warn) + + settings.set_local "gemserver.example.org", nil + + expect(Bundler.ui).to have_received(:warn).with(/enabled but unavailable/) + expect(Bundler.ui).not_to have_received(:warn).with(/Could not remove/) + end + + it "removes the stored credential when the same host is set to a value it cannot store" do + settings.set_local "gemserver.example.org", "username:password" + expect(fake_store.get(Bundler::Settings.key_for("gemserver.example.org"))).to eq("username:password") + + # A bare token has no colon, so it goes to the config file. The stored + # user:pass must not stay behind and keep winning in #credentials_for. + settings.set_local "gemserver.example.org", "baretoken" + + expect(fake_store.get(Bundler::Settings.key_for("gemserver.example.org"))).to be_nil + expect(settings.credentials_for(Gem::URI("https://gemserver.example.org/"))).to eq("baretoken") + end + + it "routes credential store warnings to Bundler.ui" do + # Bundler replaces Gem.ui with a Gem::SilentUI subclass, so a warning + # left on Gem.ui would never reach the user during a bundle command. + allow(Bundler.ui).to receive(:warn) + settings.set_local "gemserver.example.org", "username:password" + + Gem::CredentialStore.warn_once "store trouble" + + expect(Bundler.ui).to have_received(:warn).with("store trouble") + end + + it "lists stored credentials by key" do + settings.set_local "gemserver.example.org", "username:password" + + expect(settings.all_including_stored_credentials).to include("gemserver.example.org") + end + + it "keeps stored credentials out of the hot-path key list" do + settings.set_local "gemserver.example.org", "username:password" + + # #all is read per gem source and per download, and its keys go into + # the User-Agent, so the store must not be consulted there. + expect(settings.all).not_to include("gemserver.example.org") + end + + it "omits stored credentials when the backend cannot enumerate them" do + # A resolver-style third-party backend has nothing to list. + no_list = Class.new(FakeCredentialBackend) do + undef_method :list + end.new + Gem::CredentialStore.instance = Gem::CredentialStore.new(backend: no_list) + + settings.set_local "gemserver.example.org", "username:password" + + expect(settings.all_including_stored_credentials).not_to include("gemserver.example.org") + expect(settings.credential_stored?("gemserver.example.org")).to be true + end + + it "reports a stored credential without revealing it" do + settings.set_local "gemserver.example.org", "username:password" + + values = settings.pretty_values_for("gemserver.example.org") + + expect(values).to include(/Set in the credential store, which is used ahead of the config files/) + expect(values.join).not_to include("password") + expect(settings.credential_stored?("gemserver.example.org")).to be true + end + + it "does not claim a credential is stored when it is not" do + expect(settings.credential_stored?("gemserver.example.org")).to be false + expect(settings.credential_stored?("jobs")).to be false + end + + it "leaves settings that are not host names in the config file" do + # ssl_client_cert is not on any known-settings list and a Windows path + # contains a colon, so a default-allow rule would move it into the + # store, and Settings#[] would then read it back as nil. + settings.set_local "ssl_client_cert", 'C:\certs\client.pem' + settings.set_local "user_agent", "MyCorp/1.0 (build: 123)" + + expect(settings["ssl_client_cert"]).to eq('C:\certs\client.pem') + expect(settings["user_agent"]).to eq("MyCorp/1.0 (build: 123)") + expect(fake_store.get(Bundler::Settings.key_for("ssl_client_cert"))).to be_nil + end + + it "stores a credential keyed by a host with a port" do + settings.set_local "my-registry.example.com:8080", "username:password" + + expect(fake_store.get(Bundler::Settings.key_for("my-registry.example.com:8080"))).to eq("username:password") + end + + it "does not route non-credential-shaped values to the credential_store" do + settings.set_local "jobs", "4" + + expect(settings["jobs"]).to eq(4) + end + + it "does not route the gem.push_key signing key path to the credential_store" do + settings.set_local "gem.push_key", "/path/to/key.pem" + + expect(settings["gem.push_key"]).to eq("/path/to/key.pem") + end + + it "falls back to the local config file and warns when the credential_store write fails" do + Gem::CredentialStore.instance = Gem::CredentialStore.new(backend: nil) + allow(Bundler.ui).to receive(:warn) + + settings.set_local "gemserver.example.org", "username:password" + + expect(settings["gemserver.example.org"]).to eq("username:password") + expect(Bundler.ui).to have_received(:warn).once + end + + it "removes a stale plaintext credential from the config file once it moves to the store" do + # written to the config file before the store took over + allow(settings).to receive(:active_credential_store).and_return(nil) + settings.set_local "gemserver.example.org", "old:secret" + expect(settings.locations("gemserver.example.org")[:local]).to eq("old:secret") + allow(settings).to receive(:active_credential_store).and_call_original + + settings.set_local "gemserver.example.org", "new:secret" + + expect(fake_store.get(Bundler::Settings.key_for("gemserver.example.org"))).to eq("new:secret") + expect(settings.locations("gemserver.example.org")[:local]).to be_nil + end + + it "removes a credential_store-stored credential on unset" do + account = Bundler::Settings.key_for("gemserver.example.org") + settings.set_local "gemserver.example.org", "username:password" + expect(fake_store.get(account)).to eq("username:password") + + settings.set_local "gemserver.example.org", nil + + expect(fake_store.get(account)).to be_nil + end end describe "URI normalization" do diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index cc49ce8c1c29..f4030e70a6fa 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -138,6 +138,11 @@ def self.ruby=(ruby) ENV["XDG_CONFIG_HOME"] = nil ENV["XDG_CACHE_HOME"] = nil ENV["GEMRC"] = nil + # Left set, these point the suite at the real OS credential store, where + # specs that configure a host credential would write into the developer's + # own keychain. + ENV["BUNDLE_CREDENTIAL_STORE"] = nil + ENV["RUBYGEMS_CREDENTIAL_STORE"] = nil # Prevent tests from modifying the user's global git config. # GIT_CONFIG_GLOBAL and GIT_CONFIG_NOSYSTEM are available since Git 2.32. diff --git a/spec/support/fake_credential_backend.rb b/spec/support/fake_credential_backend.rb new file mode 100644 index 000000000000..662dfb1dfa16 --- /dev/null +++ b/spec/support/fake_credential_backend.rb @@ -0,0 +1,32 @@ +# frozen_string_literal: true + +# An in-memory Gem::CredentialStore backend for specs that exercise +# credential-store-enabled code paths without touching a real OS credential store. +class FakeCredentialBackend + def initialize + @data = {} + end + + def get(service, account) + @data[[service, account]] + end + + def set(service, account, secret) + @data[[service, account]] = secret + true + end + + def delete(service, account) + @data.delete([service, account]) + true + end + + def list(service) + @data.keys.select {|entry_service, _account| entry_service == service }.map(&:last) + end + + def delete_all(service) + @data.reject! {|(entry_service, _account), _secret| entry_service == service } + true + end +end diff --git a/spec/support/path.rb b/spec/support/path.rb index 68baec93778f..18e4a55a2166 100644 --- a/spec/support/path.rb +++ b/spec/support/path.rb @@ -359,7 +359,7 @@ def git_ls_files(glob) end def tracked_files_glob - ruby_core? ? "libexec/bundle* lib/bundler lib/bundler.rb lib/rubygems/vendor/uri lib/rubygems/vendor/securerandom lib/rubygems/vendor/pub_grub lib/rubygems/yaml_serializer.rb lib/rubygems/compact_index_client* spec/bundler man/bundle*" : "exe/bundle exe/bundler lib/bundler lib/bundler.rb lib/rubygems/vendor/uri lib/rubygems/vendor/securerandom lib/rubygems/vendor/pub_grub lib/rubygems/yaml_serializer.rb lib/rubygems/compact_index_client* bundler.gemspec CHANGELOG-bundler.md LICENSE-bundler.md README-bundler.md" + ruby_core? ? "libexec/bundle* lib/bundler lib/bundler.rb lib/rubygems/vendor/uri lib/rubygems/vendor/securerandom lib/rubygems/vendor/pub_grub lib/rubygems/yaml_serializer.rb lib/rubygems/compact_index_client* lib/rubygems/credential_store* spec/bundler man/bundle*" : "exe/bundle exe/bundler lib/bundler lib/bundler.rb lib/rubygems/vendor/uri lib/rubygems/vendor/securerandom lib/rubygems/vendor/pub_grub lib/rubygems/yaml_serializer.rb lib/rubygems/compact_index_client* lib/rubygems/credential_store* bundler.gemspec CHANGELOG-bundler.md LICENSE-bundler.md README-bundler.md" end def lib_tracked_files_glob diff --git a/test/rubygems/fake_credential_backend.rb b/test/rubygems/fake_credential_backend.rb new file mode 100644 index 000000000000..3a692d4566f7 --- /dev/null +++ b/test/rubygems/fake_credential_backend.rb @@ -0,0 +1,35 @@ +# frozen_string_literal: true + +## +# An in-memory Gem::CredentialStore backend for tests that need to exercise +# credential-store-enabled code paths without touching a real OS credential store. +# Inject it via Gem::CredentialStore.instance = Gem::CredentialStore.new(backend: Gem::FakeCredentialBackend.new). + +class Gem::FakeCredentialBackend + def initialize + @data = {} + end + + def get(service, account) + @data[[service, account]] + end + + def set(service, account, secret) + @data[[service, account]] = secret + true + end + + def delete(service, account) + @data.delete([service, account]) + true + end + + def list(service) + @data.keys.select {|entry_service, _account| entry_service == service }.map(&:last) + end + + def delete_all(service) + @data.reject! {|(entry_service, _account), _secret| entry_service == service } + true + end +end diff --git a/test/rubygems/helper.rb b/test/rubygems/helper.rb index 73360ced5c84..73e109796a7b 100644 --- a/test/rubygems/helper.rb +++ b/test/rubygems/helper.rb @@ -47,6 +47,7 @@ require "zlib" require_relative "mock_gem_ui" require_relative "pem_utilities" +require_relative "fake_credential_backend" # JRuby on Windows raises TypeError inside File.symlink (the wincode helper # trips on a nil path), so any test that exercises Gem::Installer's symlink @@ -382,6 +383,9 @@ def setup ENV["GEM_VENDOR"] = nil ENV["GEMRC"] = nil + # Left set, this points the suite at the real OS credential store, where + # tests that clear an API key would delete the developer's own. + ENV["RUBYGEMS_CREDENTIAL_STORE"] = nil ENV["XDG_CACHE_HOME"] = nil ENV["XDG_CONFIG_HOME"] = nil ENV["XDG_DATA_HOME"] = nil @@ -594,6 +598,19 @@ def credential_teardown FileUtils.rm_rf @temp_cred end + ## + # Runs the block with Gem::CredentialStore.instance backed by an + # in-memory Gem::FakeCredentialBackend, so credential_store-enabled code paths + # can be exercised without touching a real OS credential store. + + def with_fake_credential_store + require "rubygems/credential_store" + Gem::CredentialStore.instance = Gem::CredentialStore.new(backend: Gem::FakeCredentialBackend.new) + yield Gem::CredentialStore.instance + ensure + Gem::CredentialStore.reset! + end + def common_installer_setup common_installer_teardown diff --git a/test/rubygems/test_gem_commands_signin_command.rb b/test/rubygems/test_gem_commands_signin_command.rb index e612288faf91..74e5fba1f8aa 100644 --- a/test/rubygems/test_gem_commands_signin_command.rb +++ b/test/rubygems/test_gem_commands_signin_command.rb @@ -22,6 +22,13 @@ def teardown super end + def test_sign_in_calls_api_key_without_arguments + # Command plugins include Gem::GemcutterUtilities and override #api_key + # with no parameters, so sign_in has to keep calling it that way. + assert_equal 0, Gem::GemcutterUtilities.instance_method(:api_key).arity + assert_empty Gem::GemcutterUtilities.instance_method(:api_key).parameters + end + def test_execute_when_not_already_signed_in sign_in_ui = util_capture { @cmd.execute } assert_match(/Signed in./, sign_in_ui.output) diff --git a/test/rubygems/test_gem_commands_signout_command.rb b/test/rubygems/test_gem_commands_signout_command.rb index 999a14080f20..d64558f1bb8c 100644 --- a/test/rubygems/test_gem_commands_signout_command.rb +++ b/test/rubygems/test_gem_commands_signout_command.rb @@ -2,6 +2,7 @@ require_relative "helper" require "rubygems/commands/signout_command" +require "rubygems/credential_store" require "rubygems/installer" class TestGemCommandsSignoutCommand < Gem::TestCase @@ -21,10 +22,138 @@ def test_execute_when_user_is_signed_in assert_equal false, File.exist?(Gem.configuration.credentials_path) end + def test_execute_keeps_the_original_wording_without_the_credential_store + # Anyone who never turned the store on should read what they always read, + # since scripts match on this line. + FileUtils.mkdir_p File.dirname(Gem.configuration.credentials_path) + FileUtils.touch Gem.configuration.credentials_path + + @sign_out_ui = Gem::MockGemUi.new + use_ui(@sign_out_ui) { @cmd.execute } + + assert_match(/You have successfully signed out from all sessions\./, @sign_out_ui.output) + refute_match(/every registry/, @sign_out_ui.output) + end + + def test_execute_refuses_to_delete_a_read_only_credentials_file + pend "chmod not supported" if Gem.win_platform? + pend "running as root bypasses the write permission check" if Process.uid.zero? + + FileUtils.mkdir_p File.dirname(Gem.configuration.credentials_path) + FileUtils.touch Gem.configuration.credentials_path + File.chmod 0o400, Gem.configuration.credentials_path + + @sign_out_ui = Gem::MockGemUi.new + assert_raise Gem::MockGemUi::TermError do + use_ui(@sign_out_ui) { @cmd.execute } + end + + assert File.exist?(Gem.configuration.credentials_path) + assert_match(/Could not remove the credentials/, @sign_out_ui.error) + refute_match(/successfully signed out/, @sign_out_ui.output) + ensure + if File.exist?(Gem.configuration.credentials_path) + File.chmod 0o600, Gem.configuration.credentials_path + end + end + def test_execute_when_not_signed_in # i.e. no credential file created @sign_out_ui = Gem::MockGemUi.new use_ui(@sign_out_ui) { @cmd.execute } assert_match(/You are not currently signed in/, @sign_out_ui.error) end + + def test_execute_signs_out_of_every_registry_via_credential_store # no credentials file + Gem.configuration.credential_store = true + + with_fake_credential_store do |store| + store.set(Gem::ConfigFile::CREDENTIAL_STORE_DEFAULT_ACCOUNT, "rubygems-key") + store.set("https://other.example", "other-key") + + @sign_out_ui = Gem::MockGemUi.new + use_ui(@sign_out_ui) { @cmd.execute } + + assert_match(/signed out of every registry, including RubyGems\.org/, @sign_out_ui.output) + assert_nil store.get(Gem::ConfigFile::CREDENTIAL_STORE_DEFAULT_ACCOUNT) + assert_nil store.get("https://other.example") + end + ensure + Gem.configuration.credential_store = false + end + + def test_execute_clears_the_credential_store_even_when_the_file_is_unremovable + pend "chmod not supported" if Gem.win_platform? + pend "running as root bypasses the write permission check" if Process.uid.zero? + + Gem.configuration.credential_store = true + + FileUtils.mkdir_p File.dirname(Gem.configuration.credentials_path) + FileUtils.touch Gem.configuration.credentials_path + File.chmod 0o400, Gem.configuration.credentials_path + + with_fake_credential_store do |store| + store.set(Gem::ConfigFile::CREDENTIAL_STORE_DEFAULT_ACCOUNT, "rubygems-key") + + # An unremovable credentials file is a separate problem; it must not + # keep the stored keys alive. + @sign_out_ui = Gem::MockGemUi.new + assert_raise Gem::MockGemUi::TermError do + use_ui(@sign_out_ui) { @cmd.execute } + end + + assert_nil store.get(Gem::ConfigFile::CREDENTIAL_STORE_DEFAULT_ACCOUNT) + assert File.exist?(Gem.configuration.credentials_path) + assert_match(/Could not remove the credentials from '/, @sign_out_ui.error) + refute_match(/credential store/, @sign_out_ui.error) + end + ensure + Gem.configuration.credential_store = false + if File.exist?(Gem.configuration.credentials_path) + File.chmod 0o600, Gem.configuration.credentials_path + end + end + + def test_execute_reports_a_credential_store_that_could_not_be_cleared + Gem.configuration.credential_store = true + + # A usable backend that refuses to clear. A missing backend is a + # different case: it never held anything, so there is nothing to fail at. + refusing = Class.new(Gem::FakeCredentialBackend) do + def delete_all(_service) + false + end + end.new + Gem::CredentialStore.instance = Gem::CredentialStore.new(backend: refusing) + + @sign_out_ui = Gem::MockGemUi.new + assert_raise Gem::MockGemUi::TermError do + use_ui(@sign_out_ui) { @cmd.execute } + end + + assert_match(/Could not remove the credentials from the credential store/, @sign_out_ui.error) + refute_match(/successfully signed out/, @sign_out_ui.output) + ensure + Gem::CredentialStore.reset! + Gem.configuration.credential_store = false + end + + def test_execute_succeeds_when_the_platform_has_no_credential_store + Gem.configuration.credential_store = true + + FileUtils.mkdir_p File.dirname(Gem.configuration.credentials_path) + FileUtils.touch Gem.configuration.credentials_path + + # No native backend on this platform. Nothing was ever stored, so signout + # must not report a removal failure. + Gem::CredentialStore.instance = Gem::CredentialStore.new(backend: nil) + + @sign_out_ui = Gem::MockGemUi.new + use_ui(@sign_out_ui) { @cmd.execute } + + assert_match(/successfully signed out/, @sign_out_ui.output) + ensure + Gem::CredentialStore.reset! + Gem.configuration.credential_store = false + end end diff --git a/test/rubygems/test_gem_config_file.rb b/test/rubygems/test_gem_config_file.rb index 0ca05e7203ae..23a0be79a1cf 100644 --- a/test/rubygems/test_gem_config_file.rb +++ b/test/rubygems/test_gem_config_file.rb @@ -2,6 +2,7 @@ require_relative "helper" require "rubygems/config_file" +require "rubygems/credential_store" class TestGemConfigFile < Gem::TestCase def setup @@ -460,6 +461,351 @@ def test_rubygems_api_key_equals_bad_permission assert_equal 0o644, stat.mode & 0o644 end + def test_credential_store_defaults_to_false + refute @cfg.credential_store + end + + def test_credential_store_from_gemrc + File.open @temp_conf, "w" do |fp| + fp.puts ":credential_store: true" + end + + util_config_file %W[--config-file=#{@temp_conf}] + + assert @cfg.credential_store + end + + def test_credential_store_from_environment_variable + with_env(ENV.to_h.merge("RUBYGEMS_CREDENTIAL_STORE" => "true")) do + util_config_file + end + + assert @cfg.credential_store + end + + def test_credential_store_reads_every_boolean_spelling_from_either_source + %w[0 no off f n].each do |off| + ENV["RUBYGEMS_CREDENTIAL_STORE"] = off + assert_equal false, Gem::ConfigFile.new([]).credential_store, "#{off.inspect} from the environment" + + File.open(@temp_conf, "w") {|fp| fp.puts ":credential_store: #{off}" } + assert_equal false, Gem::ConfigFile.new(["--config-file", @temp_conf]).credential_store, "#{off.inspect} from gemrc" + end + + %w[1 yes on t y].each do |on| + ENV["RUBYGEMS_CREDENTIAL_STORE"] = on + assert_equal true, Gem::ConfigFile.new([]).credential_store, "#{on.inspect} from the environment" + end + ensure + ENV["RUBYGEMS_CREDENTIAL_STORE"] = nil + end + + def test_credential_store_survives_an_undecodable_environment_variable + # Whatever locale the environment carries, the setting is compared against + # ASCII, and String#downcase would refuse these bytes outright. Windows + # rewrites an undecodable byte on its way through the environment, so what + # the setting has to carry through is whatever comes back out of it. + ENV["RUBYGEMS_CREDENTIAL_STORE"] = "\xff".dup.force_encoding("UTF-8") + + assert_equal ENV["RUBYGEMS_CREDENTIAL_STORE"], Gem::ConfigFile.new([]).credential_store + ensure + ENV["RUBYGEMS_CREDENTIAL_STORE"] = nil + end + + def test_credential_store_backend_name_from_gemrc + File.open @temp_conf, "w" do |fp| + fp.puts ":credential_store: 1password" + end + + util_config_file %W[--config-file=#{@temp_conf}] + + assert_equal "1password", @cfg.credential_store + end + + def test_credential_store_backend_name_from_environment_variable + with_env(ENV.to_h.merge("RUBYGEMS_CREDENTIAL_STORE" => "1password")) do + util_config_file + end + + assert_equal "1password", @cfg.credential_store + end + + def test_credential_store_false_environment_variable_keeps_default + with_env(ENV.to_h.merge("RUBYGEMS_CREDENTIAL_STORE" => "false")) do + util_config_file + end + + refute @cfg.credential_store + end + + def test_rubygems_api_key_equals_with_credential_store_writes_to_store_and_clears_file + @cfg.credential_store = true + + with_fake_credential_store do |store| + @cfg.rubygems_api_key = "x" + + assert_equal "x", @cfg.rubygems_api_key + assert_equal "x", store.get(Gem::ConfigFile::CREDENTIAL_STORE_DEFAULT_ACCOUNT) + # The plaintext key from credential_setup is removed once it is stored. + refute_includes load_yaml_file(@cfg.credentials_path).keys, :rubygems_api_key + end + end + + def test_set_api_key_with_credential_store_writes_to_store_and_removes_plaintext + # A plaintext host key written before the store was enabled. + @cfg.set_api_key "https://example.org", "old" + assert_equal "old", load_yaml_file(@cfg.credentials_path)["https://example.org"] + + @cfg.credential_store = true + + with_fake_credential_store do |store| + @cfg.set_api_key "https://example.org", "new" + + assert_equal "new", store.get("https://example.org") + refute_includes load_yaml_file(@cfg.credentials_path).keys, "https://example.org" + end + end + + def test_rubygems_api_key_equals_warns_and_uses_file_when_store_write_fails + @cfg.credential_store = true + Gem::CredentialStore.instance = Gem::CredentialStore.new(backend: nil) + + use_ui @ui do + @cfg.rubygems_api_key = "x" + end + + assert_match(/plain text/, @ui.error) + assert_equal "x", load_yaml_file(@cfg.credentials_path)[:rubygems_api_key] + ensure + Gem::CredentialStore.reset! + end + + def test_named_backend_routes_reads_and_writes_to_the_store + @cfg.credential_store = "1password" + + with_fake_credential_store do |store| + @cfg.rubygems_api_key = "x" + + assert_equal "x", store.get(Gem::ConfigFile::CREDENTIAL_STORE_DEFAULT_ACCOUNT) + assert_equal "x", @cfg.rubygems_api_key + end + end + + def test_credential_store_api_key_for_only_checks_the_host_account + @cfg.credential_store = true + + with_fake_credential_store do |store| + assert_nil @cfg.credential_store_api_key_for("https://example.org") + + # The default account must not answer for another host, or a push to + # that host would send the RubyGems.org key. + store.set(Gem::ConfigFile::CREDENTIAL_STORE_DEFAULT_ACCOUNT, "default-key") + assert_nil @cfg.credential_store_api_key_for("https://example.org") + + store.set("https://example.org", "host-key") + assert_equal "host-key", @cfg.credential_store_api_key_for("https://example.org") + end + end + + def test_credential_store_api_key_for_returns_nil_without_a_host + @cfg.credential_store = true + + with_fake_credential_store do |store| + store.set("", "empty-account-key") + + assert_nil @cfg.credential_store_api_key_for(nil) + assert_nil @cfg.credential_store_api_key_for("") + end + end + + def test_clearing_the_api_key_removes_it_from_the_store + @cfg.credential_store = true + + with_fake_credential_store do |store| + @cfg.rubygems_api_key = "stored-key" + @cfg.rubygems_api_key = nil + + assert_nil store.get(Gem::ConfigFile::CREDENTIAL_STORE_DEFAULT_ACCOUNT) + assert_nil Gem::ConfigFile.new([]).tap {|c| c.credential_store = true }.rubygems_api_key + end + ensure + @cfg.credential_store = false + end + + def test_clearing_a_host_api_key_removes_it_from_the_store + @cfg.credential_store = true + + with_fake_credential_store do |store| + @cfg.set_api_key "https://other.example", "host-key" + @cfg.set_api_key "https://other.example", "" + + assert_nil store.get("https://other.example") + end + ensure + @cfg.credential_store = false + end + + def test_rubygems_api_key_reads_the_store_in_a_later_process + @cfg.credential_store = true + + with_fake_credential_store do + @cfg.rubygems_api_key = "stored-key" + + # A fresh ConfigFile stands in for the next process: the plain text + # copy is gone from the credentials file, so only the store has it. + fresh = Gem::ConfigFile.new([]) + fresh.credential_store = true + + assert_equal "stored-key", fresh.rubygems_api_key + end + ensure + @cfg.credential_store = false + end + + def test_credential_store_default_api_key_reads_the_default_account + @cfg.credential_store = true + + with_fake_credential_store do |store| + assert_nil @cfg.credential_store_default_api_key + + store.set(Gem::ConfigFile::CREDENTIAL_STORE_DEFAULT_ACCOUNT, "default-key") + assert_equal "default-key", @cfg.credential_store_default_api_key + end + end + + def test_credential_store_api_key_for_returns_nil_when_credential_store_disabled + with_fake_credential_store do |store| + store.set(Gem::ConfigFile::CREDENTIAL_STORE_DEFAULT_ACCOUNT, "default-key") + + assert_nil @cfg.credential_store_api_key_for("https://example.org") + end + end + + def test_storing_an_api_key_warns_when_the_plain_text_copy_cannot_be_removed + pend "chmod is not enforced for the owner on Windows" if Gem.win_platform? + pend "running as root bypasses the write permission check" if Process.uid.zero? + + @cfg.credential_store = true + + File.write @cfg.credentials_path, @cfg.class.dump_with_rubygems_yaml(rubygems_api_key: "old") + File.chmod 0o400, @cfg.credentials_path + + with_fake_credential_store do |store| + use_ui @ui do + @cfg.rubygems_api_key = "new" + end + + assert_equal "new", store.get(Gem::ConfigFile::CREDENTIAL_STORE_DEFAULT_ACCOUNT) + assert_match(/plain text copy .* could not be removed/, @ui.error) + end + ensure + File.chmod 0o600, @cfg.credentials_path if File.exist?(@cfg.credentials_path) + end + + def test_falling_back_to_the_file_clears_the_stored_key + # The reader consults the store first, so a stored key left behind would + # outrank the one just written to the file and never be replaced. + @cfg.credential_store = true + + refusing = Class.new(Gem::FakeCredentialBackend) do + def refuse_writes! + @refusing = true + end + + def set(service, account, secret) + return false if @refusing + + super + end + end.new + refusing.set("rubygems", Gem::ConfigFile::CREDENTIAL_STORE_DEFAULT_ACCOUNT, "stale") + refusing.refuse_writes! + Gem::CredentialStore.instance = Gem::CredentialStore.new(backend: refusing) + + use_ui(@ui) { @cfg.rubygems_api_key = "fresh" } + + assert_nil refusing.get("rubygems", Gem::ConfigFile::CREDENTIAL_STORE_DEFAULT_ACCOUNT) + assert_equal "fresh", @cfg.rubygems_api_key + ensure + Gem::CredentialStore.reset! + @cfg.credential_store = false + end + + def test_unset_api_key_bang_removes_from_credential_store + @cfg.credential_store = true + + with_fake_credential_store do |store| + @cfg.rubygems_api_key = "x" + assert_equal "x", store.get(Gem::ConfigFile::CREDENTIAL_STORE_DEFAULT_ACCOUNT) + + @cfg.unset_api_key! + + assert_nil store.get(Gem::ConfigFile::CREDENTIAL_STORE_DEFAULT_ACCOUNT) + end + end + + def test_credential_store_account_agrees_with_the_credentials_file_key + # The two have to name the same host, or a key written under one spelling + # cannot be found under another. Loading only this file also proves the + # account derivation brings its own URI support along. + %w[https://gems.example.com/ https://gems.example.com gems__example__com].each do |spelling| + account = Gem::ConfigFile.credential_store_account(spelling) + key = Gem::ConfigFile.normalize_credentials_key(spelling) + + assert_equal key, account, spelling + end + + assert_equal "https://gems.example.com", + Gem::ConfigFile.credential_store_account("https://user:secret@gems.example.com/") + end + + def test_unset_api_key_bang_leaves_a_read_only_credentials_file_alone + pend "chmod not supported" if Gem.win_platform? + pend "running as root bypasses the write permission check" if Process.uid.zero? + + # POSIX deletes a read-only file without protest when the directory is + # writable, so the refusal has to come from the code, as it always did. + FileUtils.mkdir_p File.dirname(@cfg.credentials_path) + FileUtils.touch @cfg.credentials_path + File.chmod 0o400, @cfg.credentials_path + + _store_cleared, file_removed = @cfg.unset_api_key! + + assert_equal false, file_removed + assert File.exist?(@cfg.credentials_path) + ensure + File.chmod 0o600, @cfg.credentials_path if File.exist?(@cfg.credentials_path) + end + + def test_unset_api_key_bang_removes_every_host_from_credential_store + @cfg.credential_store = true + + with_fake_credential_store do |store| + @cfg.rubygems_api_key = "x" + @cfg.set_api_key "https://other.example", "y" + assert_equal "x", store.get(Gem::ConfigFile::CREDENTIAL_STORE_DEFAULT_ACCOUNT) + assert_equal "y", store.get("https://other.example") + + @cfg.unset_api_key! + + assert_nil store.get(Gem::ConfigFile::CREDENTIAL_STORE_DEFAULT_ACCOUNT) + assert_nil store.get("https://other.example") + end + end + + def test_rubygems_api_key_equals_falls_back_to_file_when_credential_store_unavailable + @cfg.credential_store = true + + Gem::CredentialStore.instance = Gem::CredentialStore.new(backend: nil) + + @cfg.rubygems_api_key = "x" + + assert_equal "x", @cfg.rubygems_api_key + assert_equal({ rubygems_api_key: "x" }, load_yaml_file(@cfg.credentials_path)) + ensure + Gem::CredentialStore.reset! + end + def test_write @cfg.backtrace = false @cfg.update_sources = false diff --git a/test/rubygems/test_gem_credential_store.rb b/test/rubygems/test_gem_credential_store.rb new file mode 100644 index 000000000000..c31feaf1e290 --- /dev/null +++ b/test/rubygems/test_gem_credential_store.rb @@ -0,0 +1,392 @@ +# frozen_string_literal: true + +require_relative "helper" +require "rubygems/credential_store" + +class TestGemCredentialStore < Gem::TestCase + class FakeBackend + attr_reader :calls + + def initialize + @calls = [] + @data = {} + @get_calls = 0 + end + + def get(service, account) + @calls << [:get, service, account] + @get_calls += 1 + @data[[service, account]] + end + + def set(service, account, secret) + @calls << [:set, service, account, secret] + @data[[service, account]] = secret + true + end + + def delete(service, account) + @calls << [:delete, service, account] + @data.delete([service, account]) + true + end + + def delete_all(service) + @calls << [:delete_all, service] + @data.reject! {|(entry_service, _account), _secret| entry_service == service } + true + end + + def get_call_count + @get_calls + end + end + + # Fails until #raising is turned off, so a test can show the store noticing + # that a backend started answering again. + class SometimesRaisingBackend < Gem::FakeCredentialBackend + attr_accessor :raising + + def initialize + super + @raising = true + end + + def get(service, account) + raise Errno::ENOENT, "security" if @raising + + super + end + end + + class RaisingBackend + def get(_service, _account) + raise Errno::ENOENT, "security" + end + + def set(_service, _account, _secret) + raise Errno::ENOENT, "security" + end + + def delete(_service, _account) + raise Errno::ENOENT, "security" + end + + def delete_all(_service) + raise Errno::ENOENT, "security" + end + end + + def setup + super + Gem::CredentialStore.reset! + end + + def teardown + Gem::CredentialStore.reset! + super + end + + def test_available_without_backend + store = Gem::CredentialStore.new(backend: nil) + refute store.available? + end + + def test_available_with_backend + store = Gem::CredentialStore.new(backend: FakeBackend.new) + assert store.available? + end + + def test_get_set_delete_roundtrip + store = Gem::CredentialStore.new(backend: FakeBackend.new) + + assert_nil store.get("example.org") + assert store.set("example.org", "s3cr3t") + assert_equal "s3cr3t", store.get("example.org") + assert store.delete("example.org") + end + + def test_set_uses_service_name + backend = FakeBackend.new + store = Gem::CredentialStore.new(backend: backend) + + store.set("example.org", "s3cr3t") + + assert_includes backend.calls, [:set, Gem::CredentialStore::SERVICE_NAME, "example.org", "s3cr3t"] + end + + def test_get_is_memoized_per_account + backend = FakeBackend.new + backend.set(Gem::CredentialStore::SERVICE_NAME, "example.org", "s3cr3t") + store = Gem::CredentialStore.new(backend: backend) + + 3.times { store.get("example.org") } + + assert_equal 1, backend.get_call_count + end + + def test_set_updates_cache_without_extra_get + backend = FakeBackend.new + store = Gem::CredentialStore.new(backend: backend) + + store.set("example.org", "s3cr3t") + assert_equal "s3cr3t", store.get("example.org") + + assert_equal 0, backend.get_call_count + end + + def test_delete_clears_cache + backend = FakeBackend.new + backend.set(Gem::CredentialStore::SERVICE_NAME, "example.org", "s3cr3t") + store = Gem::CredentialStore.new(backend: backend) + store.get("example.org") + + store.delete("example.org") + store.get("example.org") + + assert_equal 2, backend.get_call_count + end + + def test_operations_without_backend_are_safe_noops + store = Gem::CredentialStore.new(backend: nil) + + assert_nil store.get("example.org") + refute store.set("example.org", "s3cr3t") + refute store.delete("example.org") + end + + def test_get_swallows_backend_errors_and_returns_nil + store = Gem::CredentialStore.new(backend: RaisingBackend.new) + + assert_nil store.get("example.org") + end + + def test_set_refuses_a_value_no_backend_can_round_trip + # The macOS keychain returns non-printable bytes as hex through the only + # read-back its CLI offers, so the rule applies to every backend and the + # caller falls back to the file rather than storing something unreadable. + backend = FakeBackend.new + store = Gem::CredentialStore.new(backend: backend) + + use_ui(@ui) do + assert_equal false, store.set("example.org", "p\u00e9\u3042") + assert_equal false, store.set("example.org", "line1\nline2") + assert_equal false, store.set("acct\nadd-generic-password", "secret") + end + + assert_nil backend.get("rubygems", "example.org") + assert_match(/Credential store write failed/, @ui.errs.string) + end + + def test_set_swallows_backend_errors_and_returns_false + store = Gem::CredentialStore.new(backend: RaisingBackend.new) + + refute store.set("example.org", "s3cr3t") + end + + def test_delete_swallows_backend_errors_and_returns_false + store = Gem::CredentialStore.new(backend: RaisingBackend.new) + + refute store.delete("example.org") + end + + def test_read_failed_distinguishes_an_unreadable_account_from_an_absent_one + store = Gem::CredentialStore.new(backend: Gem::FakeCredentialBackend.new) + + use_ui(@ui) { assert_nil store.get("absent.example") } + refute store.read_failed?("absent.example") + + failing = Gem::CredentialStore.new(backend: RaisingBackend.new) + use_ui(@ui) { assert_nil failing.get("unreadable.example") } + + assert failing.read_failed?("unreadable.example") + end + + def test_a_successful_write_clears_the_read_failure + backend = SometimesRaisingBackend.new + store = Gem::CredentialStore.new(backend: backend) + + use_ui(@ui) { store.get("example.org") } + assert store.read_failed?("example.org") + + backend.raising = false + store.set("example.org", "s3cr3t") + + refute store.read_failed?("example.org") + end + + def test_repeated_failures_of_one_operation_warn_only_once + store = Gem::CredentialStore.new(backend: RaisingBackend.new) + + use_ui(@ui) do + store.get("a") + store.get("b") + end + + assert_equal 1, @ui.errs.string.scan(/WARNING:/).length + end + + def test_each_operation_reports_its_own_outcome + store = Gem::CredentialStore.new(backend: RaisingBackend.new) + + use_ui(@ui) do + store.get("a") + store.set("c", "x") + end + + # A failed write lands in the config file, a failed read does not come + # back with anything, so one warning cannot stand for both. + assert_match(/read failed .* any copy left in the config file/, @ui.errs.string) + assert_match(/write failed .* falling back to file storage/, @ui.errs.string) + end + + def test_list_returns_nil_when_the_backend_cannot_enumerate + backend = Class.new(Gem::FakeCredentialBackend) { undef_method :list }.new + store = Gem::CredentialStore.new(backend: backend) + store.set("example.org", "s3cr3t") + + # nil means "unknown", which callers must not confuse with "empty". + assert_nil store.list + end + + def test_list_returns_the_accounts_when_the_backend_can_enumerate + store = Gem::CredentialStore.new(backend: Gem::FakeCredentialBackend.new) + store.set("example.org", "s3cr3t") + store.set("other.example", "other") + + assert_equal ["example.org", "other.example"], store.list + end + + def test_warn_handler_receives_the_message_instead_of_gem_ui + received = [] + Gem::CredentialStore.warn_handler = ->(message) { received << message } + + use_ui(@ui) do + Gem::CredentialStore.warn_once "routed elsewhere" + end + + assert_equal ["routed elsewhere"], received + refute_match(/routed elsewhere/, @ui.errs.string) + end + + def test_different_warnings_are_each_emitted + use_ui(@ui) do + Gem::CredentialStore.warn_once "first problem" + Gem::CredentialStore.warn_once "first problem" + # A warning about one problem must not suppress a different one, or a + # plain text fallback would go unreported after any earlier warning. + Gem::CredentialStore.warn_once "second problem" + end + + assert_equal 2, @ui.errs.string.scan(/WARNING:/).length + end + + def test_instance_returns_the_same_object + assert_same Gem::CredentialStore.instance, Gem::CredentialStore.instance + end + + def test_delete_all_clears_the_service + backend = FakeBackend.new + backend.set(Gem::CredentialStore::SERVICE_NAME, "acct", "s") + store = Gem::CredentialStore.new(backend: backend) + + assert store.delete_all + assert_nil backend.get(Gem::CredentialStore::SERVICE_NAME, "acct") + end + + def test_delete_all_without_backend_is_safe + store = Gem::CredentialStore.new(backend: nil) + refute store.delete_all + end + + def test_delete_all_swallows_backend_errors + store = Gem::CredentialStore.new(backend: RaisingBackend.new) + refute store.delete_all + end + + def test_delete_all_only_removes_its_own_service + backend = FakeBackend.new + Gem::CredentialStore.backend = backend + gem_store = Gem::CredentialStore.for(true, service: "rubygems") + bundler_store = Gem::CredentialStore.for(true, service: "bundler") + gem_store.set("acct", "gem-key") + bundler_store.set("gems.example.com", "user:pass") + + gem_store.delete_all + + assert_nil gem_store.get("acct") + assert_equal "user:pass", bundler_store.get("gems.example.com") + end + + def test_for_returns_nil_when_disabled + assert_nil Gem::CredentialStore.for(false) + assert_nil Gem::CredentialStore.for(nil) + end + + def test_for_memoizes_per_spec + Gem::CredentialStore.register_backend("faux", FakeBackend.new) + + assert_same Gem::CredentialStore.for("faux"), Gem::CredentialStore.for("faux") + end + + def test_register_and_resolve_backend_roundtrip + backend = FakeBackend.new + Gem::CredentialStore.register_backend("faux", backend) + + assert_same backend, Gem::CredentialStore.resolve_backend("faux") + + store = Gem::CredentialStore.for("faux") + assert store.set("example.org", "s3cr3t") + assert_equal "s3cr3t", store.get("example.org") + assert_includes backend.calls, [:set, Gem::CredentialStore::SERVICE_NAME, "example.org", "s3cr3t"] + end + + def test_resolve_backend_rejects_invalid_name + use_ui(@ui) do + assert_nil Gem::CredentialStore.resolve_backend("../evil") + assert_nil Gem::CredentialStore.resolve_backend("Foo Bar") + end + + assert_match(/invalid credential store backend name/, @ui.errs.string) + end + + def test_resolve_backend_rejects_an_undecodable_name_without_raising + # The setting can carry any bytes the environment hands over, and every + # public method on this class promises to warn rather than raise. + use_ui(@ui) do + assert_nil Gem::CredentialStore.resolve_backend("\xff".dup.force_encoding("UTF-8")) + end + + assert_match(/invalid credential store backend name/, @ui.errs.string) + end + + def test_resolve_backend_requires_convention_path_and_registers + backends_dir = File.join(@tempdir, "rubygems", "credential_store", "backends") + FileUtils.mkdir_p(backends_dir) + File.write(File.join(backends_dir, "faux_ext.rb"), <<~RUBY) + Gem::CredentialStore.register_backend("faux_ext", Object.new) + RUBY + + $LOAD_PATH.unshift(@tempdir) + + refute_nil Gem::CredentialStore.resolve_backend("faux_ext") + ensure + $LOAD_PATH.delete(@tempdir) + end + + def test_resolve_backend_unknown_name_returns_nil_and_warns + use_ui(@ui) do + assert_nil Gem::CredentialStore.resolve_backend("definitely_not_installed_xyz") + end + + assert_match(/is not installed/, @ui.errs.string) + end + + def test_instance_override_wins_for_any_enabled_spec + fake = Gem::CredentialStore.new(backend: FakeBackend.new) + Gem::CredentialStore.instance = fake + + assert_same fake, Gem::CredentialStore.for(true) + assert_same fake, Gem::CredentialStore.for("1password") + end +end diff --git a/test/rubygems/test_gem_credential_store_linux_backend.rb b/test/rubygems/test_gem_credential_store_linux_backend.rb new file mode 100644 index 000000000000..f00c54535f91 --- /dev/null +++ b/test/rubygems/test_gem_credential_store_linux_backend.rb @@ -0,0 +1,223 @@ +# frozen_string_literal: true + +require_relative "helper" +require "rubygems/credential_store/native/linux" +require "json" + +class TestGemCredentialStoreLinuxBackend < Gem::TestCase + FAKE_COMMAND = <<~'RUBY' + #!/usr/bin/env ruby + require "json" + stdin_content = $stdin.read + if record_path = ENV["RUBYGEMS_FAKE_CMD_RECORD"] + calls = File.exist?(record_path) ? JSON.parse(File.read(record_path)) : [] + calls << {"argv" => ARGV, "stdin" => stdin_content} + File.write(record_path, calls.to_json) + end + # A clear is followed by a search confirming nothing is left. secret-tool + # exits zero from a search whether or not it matched, and reports what it + # found on stderr, so the fake answers that call separately. + if ARGV.first == "search" && ENV.key?("RUBYGEMS_FAKE_CMD_REMAINING") + $stderr.write(ENV["RUBYGEMS_FAKE_CMD_REMAINING"].to_s) + exit(0) + end + $stdout.write(ENV["RUBYGEMS_FAKE_CMD_STDOUT"].to_s) + $stderr.write(ENV["RUBYGEMS_FAKE_CMD_STDERR"].to_s) + exit(ENV["RUBYGEMS_FAKE_CMD_EXIT"].to_i) + RUBY + + def setup + super + pend "fake shebang executables aren't supported on native Windows" if Gem.win_platform? + + @fake_bin_dir = File.join(@tempdir, "fake-bin") + FileUtils.mkdir_p(@fake_bin_dir) + fake_path = File.join(@fake_bin_dir, "secret-tool") + File.write(fake_path, FAKE_COMMAND) + File.chmod(0o755, fake_path) + + @record_path = File.join(@tempdir, "record.json") + Gem::CredentialStore::LinuxBackend.reset! + end + + def teardown + Gem::CredentialStore::LinuxBackend.reset! + super + end + + def test_available_is_true_when_secret_tool_is_on_path + with_env(ENV.to_h.merge("PATH" => [@fake_bin_dir, ENV["PATH"]].join(File::PATH_SEPARATOR))) do + Gem::CredentialStore::LinuxBackend.reset! + assert Gem::CredentialStore::LinuxBackend.available? + end + end + + def test_available_is_false_when_secret_tool_is_missing + empty_dir = File.join(@tempdir, "empty-bin") + FileUtils.mkdir_p(empty_dir) + + with_env(ENV.to_h.merge("PATH" => empty_dir)) do + Gem::CredentialStore::LinuxBackend.reset! + refute Gem::CredentialStore::LinuxBackend.available? + end + end + + def test_get_returns_stripped_secret_on_success + with_fake_env(stdout: "s3cr3t\n", exit: 0) do + assert_equal "s3cr3t", Gem::CredentialStore::LinuxBackend.get("rubygems", "example.org") + end + end + + def test_get_returns_nil_when_not_found + with_fake_env(stdout: "", exit: 1) do + assert_nil Gem::CredentialStore::LinuxBackend.get("rubygems", "example.org") + end + end + + def test_get_raises_when_the_keyring_reports_an_error + # An absent entry exits 1 with nothing on stderr. Anything else is a real + # failure and must not read as "no credential stored". + with_fake_env(stderr: "unexpected D-Bus error", exit: 1) do + error = assert_raise(RuntimeError) do + Gem::CredentialStore::LinuxBackend.get("rubygems", "example.org") + end + + assert_match(/D-Bus/, error.message) + end + end + + def test_get_uses_expected_argv + with_fake_env(stdout: "s3cr3t\n", exit: 0) do + Gem::CredentialStore::LinuxBackend.get("rubygems", "example.org") + end + + record = read_record + assert_equal %w[lookup service rubygems account example.org], record["argv"] + end + + def test_set_returns_true_on_success + with_fake_env(exit: 0) do + assert Gem::CredentialStore::LinuxBackend.set("rubygems", "example.org", "s3cr3t") + end + end + + def test_set_passes_secret_via_stdin_not_argv + with_fake_env(exit: 0) do + Gem::CredentialStore::LinuxBackend.set("rubygems", "example.org", "s3cr3t") + end + + record = read_record + refute_includes record["argv"], "s3cr3t" + assert_equal "s3cr3t", record["stdin"] + end + + def test_list_reads_the_account_attribute_from_stderr + # secret-tool prints attributes to stderr and the secrets to stdout. + attributes = <<~ERR + attribute.service = bundler + attribute.account = gems.example.com + attribute.service = bundler + attribute.account = other.example.org + ERR + + with_fake_env(stdout: "username:password", stderr: attributes, exit: 0) do + assert_equal ["gems.example.com", "other.example.org"], + Gem::CredentialStore::LinuxBackend.list("bundler") + end + + assert_equal %w[search --all service bundler], read_record["argv"] + end + + def test_list_ignores_attribute_lines_planted_inside_a_secret + # A secret is free-form and lands on stdout, so a newline inside it must + # not be able to add an account to the listing. + planted = "u:p\nattribute.account = injected\n" + + with_fake_env(stdout: planted, stderr: "attribute.account = real.example.com\n", exit: 0) do + assert_equal ["real.example.com"], Gem::CredentialStore::LinuxBackend.list("bundler") + end + end + + def test_list_returns_empty_on_failure + with_fake_env(exit: 1) do + assert_empty Gem::CredentialStore::LinuxBackend.list("bundler") + end + end + + def test_delete_returns_true_on_success + with_fake_env(exit: 0, remaining: "") do + assert Gem::CredentialStore::LinuxBackend.delete("rubygems", "example.org") + end + end + + def test_delete_returns_true_when_nothing_matched + with_fake_env(stderr: "", exit: 1) do + assert Gem::CredentialStore::LinuxBackend.delete("rubygems", "example.org") + end + end + + def test_delete_returns_false_on_other_failure + with_fake_env(stderr: "unexpected D-Bus error", exit: 1) do + refute Gem::CredentialStore::LinuxBackend.delete("rubygems", "example.org") + end + end + + def test_delete_all_clears_by_service_only + with_fake_env(exit: 0, remaining: "") do + assert Gem::CredentialStore::LinuxBackend.delete_all("rubygems") + end + + record = read_record + assert_equal %w[clear service rubygems], record["argv"] + end + + def test_delete_all_reports_failure_when_the_entries_survive + # libsecret skips locked items and still reports success, so a locked + # keyring would otherwise let signout claim it removed keys it kept. + survivor = "attribute.account = example.org\n" + + with_fake_env(exit: 0, remaining: survivor) do + refute Gem::CredentialStore::LinuxBackend.delete_all("rubygems") + refute Gem::CredentialStore::LinuxBackend.delete("rubygems", "example.org") + end + end + + def test_delete_reports_success_when_only_other_accounts_remain + with_fake_env(exit: 0, remaining: "attribute.account = other.example.org\n") do + assert Gem::CredentialStore::LinuxBackend.delete("rubygems", "example.org") + refute Gem::CredentialStore::LinuxBackend.delete_all("rubygems") + end + end + + def test_delete_all_returns_true_when_nothing_matched + with_fake_env(stderr: "", exit: 1) do + assert Gem::CredentialStore::LinuxBackend.delete_all("rubygems") + end + end + + def test_delete_all_returns_false_on_other_failure + with_fake_env(stderr: "unexpected D-Bus error", exit: 1) do + refute Gem::CredentialStore::LinuxBackend.delete_all("rubygems") + end + end + + private + + def with_fake_env(stdout: "", stderr: "", exit: 0, remaining: nil) + overrides = ENV.to_h.merge( + "PATH" => [@fake_bin_dir, ENV["PATH"]].join(File::PATH_SEPARATOR), + "RUBYGEMS_FAKE_CMD_STDOUT" => stdout, + "RUBYGEMS_FAKE_CMD_STDERR" => stderr, + "RUBYGEMS_FAKE_CMD_EXIT" => exit.to_s, + "RUBYGEMS_FAKE_CMD_REMAINING" => remaining, + "RUBYGEMS_FAKE_CMD_RECORD" => @record_path + ) + with_env(overrides) { yield } + end + + # The first call, so a delete still reports the clear it issued rather than + # the search that confirms the clear took effect. + def read_record + JSON.parse(File.read(@record_path)).first + end +end diff --git a/test/rubygems/test_gem_credential_store_macos_backend.rb b/test/rubygems/test_gem_credential_store_macos_backend.rb new file mode 100644 index 000000000000..560f4c82be55 --- /dev/null +++ b/test/rubygems/test_gem_credential_store_macos_backend.rb @@ -0,0 +1,221 @@ +# frozen_string_literal: true + +require_relative "helper" +require "rubygems/credential_store" +require "rubygems/credential_store/native/macos" +require "json" + +class TestGemCredentialStoreMacosBackend < Gem::TestCase + FAKE_COMMAND = <<~'RUBY' + #!/usr/bin/env ruby + require "json" + stdin_content = $stdin.read + if record_path = ENV["RUBYGEMS_FAKE_CMD_RECORD"] + File.write(record_path, {"argv" => ARGV, "stdin" => stdin_content}.to_json) + end + $stdout.write(ENV["RUBYGEMS_FAKE_CMD_STDOUT"].to_s) + $stderr.write(ENV["RUBYGEMS_FAKE_CMD_STDERR"].to_s) + exit(ENV["RUBYGEMS_FAKE_CMD_EXIT"].to_i) + RUBY + + def setup + super + pend "fake shebang executables aren't supported on native Windows" if Gem.win_platform? + + @fake_bin_dir = File.join(@tempdir, "fake-bin") + FileUtils.mkdir_p(@fake_bin_dir) + fake_path = File.join(@fake_bin_dir, "security") + File.write(fake_path, FAKE_COMMAND) + File.chmod(0o755, fake_path) + + @record_path = File.join(@tempdir, "record.json") + end + + def test_get_returns_stripped_secret_on_success + with_fake_env(stdout: "s3cr3t\n", exit: 0) do + assert_equal "s3cr3t", Gem::CredentialStore::MacOSBackend.get("rubygems", "example.org") + end + end + + def test_get_returns_nil_when_not_found + with_fake_env(stdout: "", exit: 44) do + assert_nil Gem::CredentialStore::MacOSBackend.get("rubygems", "example.org") + end + end + + def test_get_raises_when_the_keychain_refuses + # A locked keychain is not an absent entry. Raising lets the wrapper say + # why the credential could not be read. + with_fake_env(stderr: "User interaction is not allowed.", exit: 51) do + error = assert_raise(RuntimeError) do + Gem::CredentialStore::MacOSBackend.get("rubygems", "example.org") + end + + assert_match(/User interaction is not allowed/, error.message) + end + end + + def test_get_uses_expected_argv + with_fake_env(stdout: "s3cr3t\n", exit: 0) do + Gem::CredentialStore::MacOSBackend.get("rubygems", "example.org") + end + + record = read_record + assert_equal %w[find-generic-password -a example.org -s rubygems -w], record["argv"] + end + + def test_set_returns_true_on_success + with_fake_env(exit: 0) do + assert Gem::CredentialStore::MacOSBackend.set("rubygems", "example.org", "s3cr3t") + end + end + + def test_set_reports_why_it_failed + # The wrapper turns this into false; raising is how the reason reaches + # the user instead of an unexplained "could not write" message. + with_fake_env(stderr: "keychain is locked", exit: 1) do + error = assert_raise RuntimeError do + Gem::CredentialStore::MacOSBackend.set("rubygems", "example.org", "s3cr3t") + end + + assert_match(/keychain is locked/, error.message) + end + end + + def test_set_failure_becomes_false_through_the_store + with_fake_env(stderr: "keychain is locked", exit: 1) do + store = Gem::CredentialStore.new(backend: Gem::CredentialStore::MacOSBackend) + + refute store.set("example.org", "s3cr3t") + end + end + + def test_set_passes_secret_via_stdin_not_argv + with_fake_env(exit: 0) do + Gem::CredentialStore::MacOSBackend.set("rubygems", "example.org", "s3cr3t") + end + + record = read_record + refute_includes record["argv"], "s3cr3t" + assert_includes record["stdin"], "s3cr3t" + end + + def test_set_escapes_quotes_and_backslashes_in_the_stdin_command + with_fake_env(exit: 0) do + Gem::CredentialStore::MacOSBackend.set("rubygems", "example.org", %(pa"ss\\word)) + end + + record = read_record + assert_equal %(add-generic-password -U -a "example.org" -s "rubygems" -w "pa\\"ss\\\\word"\n), record["stdin"] + end + + def test_list_returns_accounts_for_the_service_only + dump = <<~DUMP + keychain: "/Users/x/Library/Keychains/login.keychain-db" + attributes: + "acct"="gems.example.com" + "svce"="bundler" + keychain: "/Users/x/Library/Keychains/login.keychain-db" + attributes: + "acct"="other.example.org" + "svce"="bundler" + keychain: "/Users/x/Library/Keychains/login.keychain-db" + attributes: + "acct"="unrelated" + "svce"="something-else" + DUMP + + with_fake_env(stdout: dump, exit: 0) do + assert_equal ["gems.example.com", "other.example.org"], + Gem::CredentialStore::MacOSBackend.list("bundler") + end + end + + def test_list_returns_empty_on_failure + with_fake_env(exit: 1) do + assert_empty Gem::CredentialStore::MacOSBackend.list("bundler") + end + end + + def test_delete_returns_true_on_success + with_fake_env(exit: 0) do + assert Gem::CredentialStore::MacOSBackend.delete("rubygems", "example.org") + end + end + + def test_delete_returns_true_when_not_found + with_fake_env(exit: 44) do + assert Gem::CredentialStore::MacOSBackend.delete("rubygems", "example.org") + end + end + + def test_delete_returns_false_on_other_failure + with_fake_env(exit: 1) do + refute Gem::CredentialStore::MacOSBackend.delete("rubygems", "example.org") + end + end + + def test_delete_all_loops_until_not_found + # security deletes one entry per call; stub exits 0 twice, then 44. + counter = File.join(@tempdir, "counter") + File.write(counter, "0") + script = <<~RUBY + #!/usr/bin/env ruby + c = File.read(#{counter.inspect}).to_i + File.write(#{counter.inspect}, (c + 1).to_s) + exit(c < 2 ? 0 : 44) + RUBY + File.write(File.join(@fake_bin_dir, "security"), script) + File.chmod(0o755, File.join(@fake_bin_dir, "security")) + + with_env(ENV.to_h.merge("PATH" => [@fake_bin_dir, ENV["PATH"]].join(File::PATH_SEPARATOR))) do + assert Gem::CredentialStore::MacOSBackend.delete_all("rubygems") + end + + assert_equal 3, File.read(counter).to_i + end + + def test_delete_all_returns_false_on_error + with_fake_env(exit: 1) do + refute Gem::CredentialStore::MacOSBackend.delete_all("rubygems") + end + end + + def test_get_returns_no_credential_when_command_missing + empty_dir = File.join(@tempdir, "empty-bin") + FileUtils.mkdir_p(empty_dir) + + # A missing security binary must not yield a credential. MRI raises + # Errno::ENOENT from Open3; other implementations (JRuby) report a + # failure status instead of raising, so accept either and assert only + # that nothing is returned. Gem::CredentialStore#get traps the error + # class either way. + with_env(ENV.to_h.merge("PATH" => empty_dir)) do + result = + begin + Gem::CredentialStore::MacOSBackend.get("rubygems", "example.org") + rescue StandardError + nil + end + + assert_nil result + end + end + + private + + def with_fake_env(stdout: "", stderr: "", exit: 0) + overrides = ENV.to_h.merge( + "PATH" => [@fake_bin_dir, ENV["PATH"]].join(File::PATH_SEPARATOR), + "RUBYGEMS_FAKE_CMD_STDOUT" => stdout, + "RUBYGEMS_FAKE_CMD_STDERR" => stderr, + "RUBYGEMS_FAKE_CMD_EXIT" => exit.to_s, + "RUBYGEMS_FAKE_CMD_RECORD" => @record_path + ) + with_env(overrides) { yield } + end + + def read_record + JSON.parse(File.read(@record_path)) + end +end diff --git a/test/rubygems/test_gem_credential_store_windows_backend.rb b/test/rubygems/test_gem_credential_store_windows_backend.rb new file mode 100644 index 000000000000..0a8e75c33216 --- /dev/null +++ b/test/rubygems/test_gem_credential_store_windows_backend.rb @@ -0,0 +1,186 @@ +# frozen_string_literal: true + +require_relative "helper" +require "rubygems/credential_store/native/windows" +require "json" + +class TestGemCredentialStoreWindowsBackend < Gem::TestCase + FAKE_COMMAND = <<~'RUBY' + #!/usr/bin/env ruby + require "json" + stdin_content = $stdin.read + if record_path = ENV["RUBYGEMS_FAKE_CMD_RECORD"] + record = { + "argv" => ARGV, + "stdin" => stdin_content, + "service_env" => ENV["RUBYGEMS_CRED_SERVICE"], + "account_env" => ENV["RUBYGEMS_CRED_ACCOUNT"], + "secret_env" => ENV["RUBYGEMS_CRED_SECRET"], + } + File.write(record_path, record.to_json) + end + $stdout.write(ENV["RUBYGEMS_FAKE_CMD_STDOUT"].to_s) + $stderr.write(ENV["RUBYGEMS_FAKE_CMD_STDERR"].to_s) + exit(ENV["RUBYGEMS_FAKE_CMD_EXIT"].to_i) + RUBY + + def setup + super + + # The stand-in below is reached through MRI's own PATH search, which walks + # every extension inside one directory before moving to the next and knows + # to run a batch file through a command line. JRuby spawns through Java, + # whose search appends only .exe, so it would reach the real powershell. + pend "the powershell stand-in relies on how MRI resolves a program name" if Gem.java_platform? + + @fake_bin_dir = File.join(@tempdir, "fake-bin") + FileUtils.mkdir_p(@fake_bin_dir) + fake_path = File.join(@fake_bin_dir, "powershell") + File.write(fake_path, FAKE_COMMAND) + File.chmod(0o755, fake_path) + + # A shebang does not make a file executable on Windows. PATHEXT finds the + # batch file for the extensionless name the backend spawns, and the batch + # file hands the script next to it to the running ruby. + if Gem.win_platform? + File.write("#{fake_path}.bat", <<~BATCH) + @ECHO OFF + @"#{Gem.ruby.tr("/", File::ALT_SEPARATOR || "/")}" "%~dpn0" %* + BATCH + end + + @record_path = File.join(@tempdir, "record.json") + end + + def test_get_returns_stripped_secret_on_success + with_fake_env(stdout: "s3cr3t\n", exit: 0) do + assert_equal "s3cr3t", Gem::CredentialStore::WindowsBackend.get("rubygems", "example.org") + end + end + + def test_get_returns_nil_when_credential_missing + with_fake_env(stderr: "Element not found. (Exception from HRESULT: 0x80070490)", exit: 1) do + assert_nil Gem::CredentialStore::WindowsBackend.get("rubygems", "example.org") + end + end + + def test_get_raises_on_any_other_failure + # A vault that refuses to answer is not the same as one holding nothing. + # Raising lets the wrapper report why rather than authenticating with no + # credential at all. + with_fake_env(stderr: "Access is denied.", exit: 1) do + error = assert_raise(RuntimeError) do + Gem::CredentialStore::WindowsBackend.get("rubygems", "example.org") + end + + assert_match(/Access is denied/, error.message) + end + end + + def test_get_passes_service_and_account_via_environment_not_script + with_fake_env(stdout: "s3cr3t\n", exit: 0) do + Gem::CredentialStore::WindowsBackend.get("rubygems", "example.org") + end + + record = read_record + assert_equal "rubygems", record["service_env"] + assert_equal "example.org", record["account_env"] + end + + def test_set_passes_secret_via_environment_not_argv_or_script_text + with_fake_env(exit: 0) do + Gem::CredentialStore::WindowsBackend.set("rubygems", "example.org", "s3cr3t") + end + + record = read_record + assert_equal "s3cr3t", record["secret_env"] + refute_includes record["argv"].to_s, "s3cr3t" + end + + def test_set_returns_true_on_success + with_fake_env(exit: 0) do + assert Gem::CredentialStore::WindowsBackend.set("rubygems", "example.org", "s3cr3t") + end + end + + def test_list_returns_the_user_names + with_fake_env(stdout: "gems.example.com\nother.example.org\n", exit: 0) do + assert_equal ["gems.example.com", "other.example.org"], + Gem::CredentialStore::WindowsBackend.list("bundler") + end + + assert_equal "bundler", read_record["service_env"] + end + + def test_list_returns_empty_on_failure + with_fake_env(stderr: "Access is denied.", exit: 1) do + assert_empty Gem::CredentialStore::WindowsBackend.list("bundler") + end + end + + def test_delete_returns_true_on_success + with_fake_env(exit: 0) do + assert Gem::CredentialStore::WindowsBackend.delete("rubygems", "example.org") + end + end + + def test_delete_returns_true_when_credential_missing + with_fake_env(stderr: "Element not found. (Exception from HRESULT: 0x80070490)", exit: 1) do + assert Gem::CredentialStore::WindowsBackend.delete("rubygems", "example.org") + end + end + + def test_delete_returns_false_on_other_failure + with_fake_env(stderr: "Access is denied.", exit: 1) do + refute Gem::CredentialStore::WindowsBackend.delete("rubygems", "example.org") + end + end + + def test_delete_all_passes_service_and_succeeds + with_fake_env(exit: 0) do + assert Gem::CredentialStore::WindowsBackend.delete_all("rubygems") + end + + assert_equal "rubygems", read_record["service_env"] + end + + def test_delete_all_returns_true_when_resource_missing + with_fake_env(stderr: "Element not found. (Exception from HRESULT: 0x80070490)", exit: 1) do + assert Gem::CredentialStore::WindowsBackend.delete_all("rubygems") + end + end + + def test_delete_all_returns_false_on_other_failure + with_fake_env(stderr: "Access is denied.", exit: 1) do + refute Gem::CredentialStore::WindowsBackend.delete_all("rubygems") + end + end + + def test_uses_powershell_exe_not_pwsh + with_fake_env(stdout: "s3cr3t\n", exit: 0) do + Gem::CredentialStore::WindowsBackend.get("rubygems", "example.org") + end + + # No assertion beyond "this succeeded": the fake binary is only + # discoverable under the literal name powershell.exe, so a pass here + # proves the backend invokes that name specifically. + assert File.exist?(@record_path) + end + + private + + def with_fake_env(stdout: "", stderr: "", exit: 0) + overrides = ENV.to_h.merge( + "PATH" => [@fake_bin_dir, ENV["PATH"]].join(File::PATH_SEPARATOR), + "RUBYGEMS_FAKE_CMD_STDOUT" => stdout, + "RUBYGEMS_FAKE_CMD_STDERR" => stderr, + "RUBYGEMS_FAKE_CMD_EXIT" => exit.to_s, + "RUBYGEMS_FAKE_CMD_RECORD" => @record_path + ) + with_env(overrides) { yield } + end + + def read_record + JSON.parse(File.read(@record_path)) + end +end diff --git a/test/rubygems/test_gem_gemcutter_utilities.rb b/test/rubygems/test_gem_gemcutter_utilities.rb index ca34c8d03dc7..98a697e2b09b 100644 --- a/test/rubygems/test_gem_gemcutter_utilities.rb +++ b/test/rubygems/test_gem_gemcutter_utilities.rb @@ -6,6 +6,7 @@ require "rubygems/command" require "rubygems/gemcutter_utilities" require "rubygems/config_file" +require "rubygems/credential_store" class TestGemGemcutterUtilities < Gem::TestCase def setup @@ -35,6 +36,31 @@ def teardown super end + def test_key_option_does_not_reach_a_hosts_stored_key + # --key names a key, the store is keyed by host. Letting one resolve the + # other would hand a host's key to whatever host is being pushed to. + Gem.configuration.credential_store = true + + with_fake_credential_store do + Gem.configuration.set_api_key "https://internal.example.com", "internal-key" + + @cmd = Gem::Command.new "dummy", "dummy" + @cmd.extend Gem::GemcutterUtilities + @cmd.options[:key] = :"https://internal.example.com" + @cmd.host = "https://public.example.com" + + use_ui @ui do + assert_raise Gem::MockGemUi::TermError do + @cmd.api_key + end + end + + assert_match(/No such API key/, @ui.error) + end + ensure + Gem.configuration.credential_store = false + end + def test_alternate_key_alternate_host keys = { :rubygems_api_key => "KEY", @@ -52,6 +78,199 @@ def test_alternate_key_alternate_host assert_equal "EYKEY", @cmd.api_key end + def test_api_key_from_credential_store_takes_precedence_over_file + Gem.configuration.credential_store = true + + with_fake_credential_store do |store| + keys = { rubygems_api_key: "FILE-KEY" } + + File.open Gem.configuration.credentials_path, "w" do |f| + f.write Gem::ConfigFile.dump_with_rubygems_yaml(keys) + end + + Gem.configuration.load_api_keys + store.set(Gem::ConfigFile::CREDENTIAL_STORE_DEFAULT_ACCOUNT, "CREDENTIAL_STORE-KEY") + + assert_equal "CREDENTIAL_STORE-KEY", @cmd.api_key + end + ensure + Gem.configuration.credential_store = false + end + + def unreadable_credential_store + backend = Class.new(Gem::FakeCredentialBackend) do + def get(_service, _account) + raise Errno::ENOENT, "security" + end + end.new + + Gem::CredentialStore.new(backend: backend) + end + + def test_api_key_refuses_to_fall_back_when_the_hosts_key_cannot_be_read + Gem.configuration.credential_store = true + + # Nothing here belongs to the third-party host, and the store will not say + # whether it holds anything. Falling through to the RubyGems.org key would + # hand it to that host. + Gem::CredentialStore.instance = unreadable_credential_store + + File.open Gem.configuration.credentials_path, "w" do |f| + f.write Gem::ConfigFile.dump_with_rubygems_yaml({ rubygems_api_key: "RUBYGEMS-ORG-KEY" }) + end + Gem.configuration.load_api_keys + + ENV["RUBYGEMS_HOST"] = "http://rubygems.engineyard.com" + + assert_raise Gem::MockGemUi::TermError do + use_ui(@ui) { @cmd.api_key } + end + + assert_match(%r{no API key for http://rubygems\.engineyard\.com could be found}, @ui.error) + refute_match(/RUBYGEMS-ORG-KEY/, @ui.error) + ensure + Gem::CredentialStore.reset! + Gem.configuration.credential_store = false + end + + def test_api_key_uses_the_hosts_file_key_when_the_store_cannot_be_read + Gem.configuration.credential_store = true + + # The store is unreadable but this host has its own key on disk. That key + # cannot leak anywhere, so there is nothing to stop for. + Gem::CredentialStore.instance = unreadable_credential_store + + keys = { + :rubygems_api_key => "RUBYGEMS-ORG-KEY", + "http://rubygems.engineyard.com" => "EYKEY", + } + File.open Gem.configuration.credentials_path, "w" do |f| + f.write Gem::ConfigFile.dump_with_rubygems_yaml(keys) + end + Gem.configuration.load_api_keys + + ENV["RUBYGEMS_HOST"] = "http://rubygems.engineyard.com" + + use_ui(@ui) { assert_equal "EYKEY", @cmd.api_key } + ensure + Gem::CredentialStore.reset! + Gem.configuration.credential_store = false + end + + def test_api_key_uses_the_default_hosts_file_key_when_the_store_cannot_be_read + Gem.configuration.credential_store = true + + # The key is still in the credentials file, so the unreadable store cost + # nothing. Stopping here would make every plain `gem push` fail. + Gem::CredentialStore.instance = unreadable_credential_store + + File.open Gem.configuration.credentials_path, "w" do |f| + f.write Gem::ConfigFile.dump_with_rubygems_yaml({ rubygems_api_key: "RUBYGEMS-ORG-KEY" }) + end + Gem.configuration.load_api_keys + + use_ui(@ui) { assert_equal "RUBYGEMS-ORG-KEY", @cmd.api_key } + ensure + Gem::CredentialStore.reset! + Gem.configuration.credential_store = false + end + + def test_api_key_notices_a_failure_under_the_default_account_alone + Gem.configuration.credential_store = true + + # Nothing is ever written under the default host's own name, so a read + # there finds nothing and records no failure. The key lives under the + # default account, and a refusal to read that one is the only signal + # there will be. + selective = Class.new(Gem::FakeCredentialBackend) do + def get(service, account) + raise Errno::ENOENT, "security" if account == Gem::ConfigFile::CREDENTIAL_STORE_DEFAULT_ACCOUNT + + super + end + end.new + Gem::CredentialStore.instance = Gem::CredentialStore.new(backend: selective) + + File.open Gem.configuration.credentials_path, "w" do |f| + f.write Gem::ConfigFile.dump_with_rubygems_yaml({}) + end + Gem.configuration.load_api_keys + + assert_raise Gem::MockGemUi::TermError do + use_ui(@ui) { @cmd.api_key } + end + ensure + Gem::CredentialStore.reset! + Gem.configuration.credential_store = false + end + + def test_api_key_stops_rather_than_asking_for_a_password_when_the_store_is_all_there_is + Gem.configuration.credential_store = true + + # The migrated case: the key lives only in the store, which will not + # answer. Returning nil here reads as "not signed in" and sends the user + # to a password prompt, downgrading key authentication to the account + # password over a locked keychain. + Gem::CredentialStore.instance = unreadable_credential_store + + File.open Gem.configuration.credentials_path, "w" do |f| + f.write Gem::ConfigFile.dump_with_rubygems_yaml({}) + end + Gem.configuration.load_api_keys + + assert_raise Gem::MockGemUi::TermError do + use_ui(@ui) { @cmd.api_key } + end + + assert_match(/credential store could not be read/, @ui.error) + ensure + Gem::CredentialStore.reset! + Gem.configuration.credential_store = false + end + + def test_api_key_prefers_the_hosts_file_key_over_the_stored_default_key + Gem.configuration.credential_store = true + + with_fake_credential_store do |store| + keys = { + :rubygems_api_key => "FILE-DEFAULT-KEY", + "http://rubygems.engineyard.com" => "EYKEY", + } + + File.open Gem.configuration.credentials_path, "w" do |f| + f.write Gem::ConfigFile.dump_with_rubygems_yaml(keys) + end + + ENV["RUBYGEMS_HOST"] = "http://rubygems.engineyard.com" + Gem.configuration.load_api_keys + store.set(Gem::ConfigFile::CREDENTIAL_STORE_DEFAULT_ACCOUNT, "RUBYGEMS-ORG-KEY") + + # Sending RUBYGEMS-ORG-KEY here would hand the RubyGems.org key to a + # third-party host that has a key of its own. + assert_equal "EYKEY", @cmd.api_key + end + ensure + Gem.configuration.credential_store = false + end + + def test_api_key_falls_back_to_file_when_no_credential_store_entry + Gem.configuration.credential_store = true + + with_fake_credential_store do + keys = { rubygems_api_key: "FILE-KEY" } + + File.open Gem.configuration.credentials_path, "w" do |f| + f.write Gem::ConfigFile.dump_with_rubygems_yaml(keys) + end + + Gem.configuration.load_api_keys + + assert_equal "FILE-KEY", @cmd.api_key + end + ensure + Gem.configuration.credential_store = false + end + def test_api_key keys = { rubygems_api_key: "KEY" } diff --git a/tool/quality_check.rb b/tool/quality_check.rb index 1218f3022b1c..614018ff36f0 100644 --- a/tool/quality_check.rb +++ b/tool/quality_check.rb @@ -260,7 +260,7 @@ def check_for_specific_pronouns(filename) end def tracked_files - @tracked_files ||= git_ls_files("exe/bundle exe/bundler lib/bundler lib/bundler.rb lib/rubygems/vendor/uri lib/rubygems/vendor/securerandom lib/rubygems/vendor/pub_grub lib/rubygems/yaml_serializer.rb lib/rubygems/compact_index_client* bundler.gemspec CHANGELOG-bundler.md LICENSE-bundler.md README-bundler.md") + @tracked_files ||= git_ls_files("exe/bundle exe/bundler lib/bundler lib/bundler.rb lib/rubygems/vendor/uri lib/rubygems/vendor/securerandom lib/rubygems/vendor/pub_grub lib/rubygems/yaml_serializer.rb lib/rubygems/compact_index_client* lib/rubygems/credential_store* bundler.gemspec CHANGELOG-bundler.md LICENSE-bundler.md README-bundler.md") end def lib_tracked_files