From 1970c94e6af60fc1ebe795f01b9af8cf6a23c242 Mon Sep 17 00:00:00 2001 From: sachin-sandhu Date: Wed, 17 Jul 2024 23:30:26 -0400 Subject: [PATCH 1/4] Adds filter for failed to replace env in config erros --- common/lib/dependabot/errors.rb | 21 +++++++ npm_and_yarn/lib/dependabot/npm_and_yarn.rb | 8 +++ .../npm_and_yarn/file_updater_spec.rb | 58 +++++++++++++++++++ .../.npmrc | 2 + .../package.json | 25 ++++++++ .../yarn.lock | 56 ++++++++++++++++++ 6 files changed, 170 insertions(+) create mode 100644 npm_and_yarn/spec/fixtures/projects/yarn/npm_global_registry_env_var_missing/.npmrc create mode 100644 npm_and_yarn/spec/fixtures/projects/yarn/npm_global_registry_env_var_missing/package.json create mode 100644 npm_and_yarn/spec/fixtures/projects/yarn/npm_global_registry_env_var_missing/yarn.lock diff --git a/common/lib/dependabot/errors.rb b/common/lib/dependabot/errors.rb index ce27666b38f..7aa19999404 100644 --- a/common/lib/dependabot/errors.rb +++ b/common/lib/dependabot/errors.rb @@ -242,6 +242,11 @@ def self.updater_error_details(error) "error-type": "git_token_auth_error", "error-detail": { message: error.message } } + when Dependabot::FailedToReplaceEnvInConfig + { + "error-type": "failed_to_replace_env", + "error-detail": { message: error.message } + } when *Octokit::RATE_LIMITED_ERRORS # If we get a rate-limited error we let dependabot-api handle the # retry by re-enqueing the update job after the reset @@ -340,6 +345,8 @@ class NotImplemented < DependabotError; end class InvalidGitAuthToken < DependabotError; end + class FailedToReplaceEnvInConfig < DependabotError; end + ##################### # Repo level errors # ##################### @@ -585,6 +592,20 @@ def initialize(source) end end + class FailedToReplaceEnvInConfig < DependabotError + extend T::Sig + + sig { returns(String) } + attr_reader :source + + sig { params(source: String).void } + def initialize(source) + @source = T.let(sanitize_source(source), String) + msg = @source.to_s + super(msg) + end + end + # Useful for JS file updaters, where the registry API sometimes returns # different results to the actual update process class InconsistentRegistryResponse < DependabotError; end diff --git a/npm_and_yarn/lib/dependabot/npm_and_yarn.rb b/npm_and_yarn/lib/dependabot/npm_and_yarn.rb index b7d5f52d806..eec42b9d2bc 100644 --- a/npm_and_yarn/lib/dependabot/npm_and_yarn.rb +++ b/npm_and_yarn/lib/dependabot/npm_and_yarn.rb @@ -74,6 +74,8 @@ module NpmAndYarn # Used to identify if error message is related to yarn workspaces DEPENDENCY_FILE_NOT_RESOLVABLE = "conflicts with direct dependency" + ENV_VAR_NOT_RESOLVABLE = /Failed to replace env in config: / + class Utils extend T::Sig @@ -191,6 +193,12 @@ def self.extract_node_versions(error_message) new_error: ->(_error, message) { DependencyFileNotResolvable.new(message) }, in_usage: false, matchfn: nil + }, + { + patterns: [ENV_VAR_NOT_RESOLVABLE], + new_error: ->(_error, message) { FailedToReplaceEnvInConfig.new(message) }, + in_usage: false, + matchfn: nil } ].freeze, T::Array[{ patterns: T::Array[T.any(String, Regexp)], diff --git a/npm_and_yarn/spec/dependabot/npm_and_yarn/file_updater_spec.rb b/npm_and_yarn/spec/dependabot/npm_and_yarn/file_updater_spec.rb index e853f820f81..78a5a1b3bfb 100644 --- a/npm_and_yarn/spec/dependabot/npm_and_yarn/file_updater_spec.rb +++ b/npm_and_yarn/spec/dependabot/npm_and_yarn/file_updater_spec.rb @@ -3281,6 +3281,28 @@ end end + context "when the npm registry access token var is missing its env var" do + let(:files) { project_dependency_files("yarn/npm_global_registry_env_var_missing") } + let(:credentials) do + [Dependabot::Credential.new({ + "type" => "npm_registry", + "registry" => "https://registry.npmjs.org", + "token" => "${NPM_TOKEN}" + })] + end + + let(:source) do + { type: "registry", url: "https://registry.npmjs.org" } + end + + it "keeps the preference for the npm registry" do + expect { updated_yarn_lock }.to raise_error do |error| + expect(error).to be_a(Dependabot::FailedToReplaceEnvInConfig) + expect(error.message).to include("Failed to replace env in config: ${NPM_TOKEN}") + end + end + end + context "when there's a duplicate indirect dependency" do let(:files) { project_dependency_files("yarn/duplicate_indirect_dependency") } @@ -3327,6 +3349,42 @@ end end + context "when the private registry access #1 token var is missing its env var" do + let(:files) { project_dependency_files("yarn/npm_global_registry_env_var_missing") } + let(:credentials) do + [Dependabot::Credential.new({ + "type" => "npm_registry", + "registry" => "https://packagecloud.io/", + "token" => "${PACKAGECLOUD_TOKEN}" + })] + end + + it "keeps the preference for the npm registry" do + expect { updated_yarn_lock }.to raise_error do |error| + expect(error).to be_a(Dependabot::FailedToReplaceEnvInConfig) + expect(error.message).to include("Failed to replace env in config: ${PACKAGECLOUD_TOKEN}") + end + end + end + + context "when the private registry access #2 token var is missing its env var" do + let(:files) { project_dependency_files("yarn/npm_global_registry_env_var_missing") } + let(:credentials) do + [Dependabot::Credential.new({ + "type" => "npm_registry", + "registry" => "https://npm.fontawesome.com/", + "token" => "${FONTAWESOME_NPM_AUTH_TOKEN}" + })] + end + + it "keeps the preference for the npm registry" do + expect { updated_yarn_lock }.to raise_error do |error| + expect(error).to be_a(Dependabot::FailedToReplaceEnvInConfig) + expect(error.message).to include("Failed to replace env in config: ${FONTAWESOME_NPM_AUTH_TOKEN}") + end + end + end + context "with resolutions" do let(:files) { project_dependency_files("yarn/resolution_specified") } diff --git a/npm_and_yarn/spec/fixtures/projects/yarn/npm_global_registry_env_var_missing/.npmrc b/npm_and_yarn/spec/fixtures/projects/yarn/npm_global_registry_env_var_missing/.npmrc new file mode 100644 index 00000000000..fb1c4e84b26 --- /dev/null +++ b/npm_and_yarn/spec/fixtures/projects/yarn/npm_global_registry_env_var_missing/.npmrc @@ -0,0 +1,2 @@ +registry "https://registry.npmjs.org" +"//registry.npmjs.org/:_authToken" ${NPM_TOKEN} \ No newline at end of file diff --git a/npm_and_yarn/spec/fixtures/projects/yarn/npm_global_registry_env_var_missing/package.json b/npm_and_yarn/spec/fixtures/projects/yarn/npm_global_registry_env_var_missing/package.json new file mode 100644 index 00000000000..08263e98a82 --- /dev/null +++ b/npm_and_yarn/spec/fixtures/projects/yarn/npm_global_registry_env_var_missing/package.json @@ -0,0 +1,25 @@ +{ + "name": "{{ name }}", + "version": "1.0.0", + "description": "", + "main": "index.js", + "scripts": { + "test": "echo \"Error: no\\ test\\ specified\" && exit 1", + "prettify": "prettier --write \"{{packages/*/src,examples,cypress,scripts}/**/,}*.{js,jsx,ts,tsx,css,md}\"" + }, + "repository": { + "type": "git", + "url": "git+https://github.com/waltfy/PROTO_TEST.git" + }, + "author": "", + "license": "ISC", + "bugs": { + "url": "https://github.com/waltfy/PROTO_TEST/issues" + }, + "homepage": "https://github.com/waltfy/PROTO_TEST#readme", + "dependencies": { + "fetch-factory": "^0.0.1" + }, + "devDependencies": { + "etag" : "^1.0.0" + }} diff --git a/npm_and_yarn/spec/fixtures/projects/yarn/npm_global_registry_env_var_missing/yarn.lock b/npm_and_yarn/spec/fixtures/projects/yarn/npm_global_registry_env_var_missing/yarn.lock new file mode 100644 index 00000000000..2b765761623 --- /dev/null +++ b/npm_and_yarn/spec/fixtures/projects/yarn/npm_global_registry_env_var_missing/yarn.lock @@ -0,0 +1,56 @@ +# THIS IS AN AUTOGENERATED FILE. DO NOT EDIT THIS FILE DIRECTLY. +# yarn lockfile v1 + + +encoding@^0.1.11: + version "0.1.12" + resolved "https://registry.npmjs.org/encoding/-/encoding-0.1.12.tgz#538b66f3ee62cd1ab51ec323829d1f9480c74beb" + dependencies: + iconv-lite "~0.4.13" + +es6-promise@^3.0.2: + version "3.3.1" + resolved "https://registry.npmjs.org/es6-promise/-/es6-promise-3.3.1.tgz#a08cdde84ccdbf34d027a1451bc91d4bcd28a613" + +etag@^1.0.0: + version "1.7.0" + resolved "https://registry.npmjs.org/etag/-/etag-1.7.0.tgz#03d30b5f67dd6e632d2945d30d6652731a34d5d8" + +fetch-factory@^0.0.1: + version "0.0.1" + resolved "https://registry.npmjs.org/fetch-factory/-/fetch-factory-0.0.1.tgz#e0076059bdb31e3147c75b3b8c04133ba8c7e071" + dependencies: + es6-promise "^3.0.2" + isomorphic-fetch "^2.1.1" + lodash "^3.10.1" + +iconv-lite@~0.4.13: + version "0.4.15" + resolved "https://registry.npmjs.org/iconv-lite/-/iconv-lite-0.4.15.tgz#fe265a218ac6a57cfe854927e9d04c19825eddeb" + +is-stream@^1.0.1: + version "1.1.0" + resolved "https://registry.npmjs.org/is-stream/-/is-stream-1.1.0.tgz#12d4a3dd4e68e0b79ceb8dbc84173ae80d91ca44" + +isomorphic-fetch@^2.1.1: + version "2.2.1" + resolved "https://registry.npmjs.org/isomorphic-fetch/-/isomorphic-fetch-2.2.1.tgz#611ae1acf14f5e81f729507472819fe9733558a9" + dependencies: + node-fetch "^1.0.1" + whatwg-fetch ">=0.10.0" + +lodash@^3.10.1: + version "3.10.1" + resolved "https://registry.npmjs.org/lodash/-/lodash-3.10.1.tgz#5bf45e8e49ba4189e17d482789dfd15bd140b7b6" + +node-fetch@^1.0.1: + version "1.6.3" + resolved "https://registry.npmjs.org/node-fetch/-/node-fetch-1.6.3.tgz#dc234edd6489982d58e8f0db4f695029abcd8c04" + dependencies: + encoding "^0.1.11" + is-stream "^1.0.1" + +whatwg-fetch@>=0.10.0: + version "2.0.2" + resolved "https://registry.npmjs.org/whatwg-fetch/-/whatwg-fetch-2.0.2.tgz#fe294d1d89e36c5be8b3195057f2e4bc74fc980e" + From 1429e1552a9e6ef13b8dee7fe7c3606c89648a2a Mon Sep 17 00:00:00 2001 From: sachin-sandhu Date: Thu, 18 Jul 2024 18:13:56 -0400 Subject: [PATCH 2/4] Changes as per comments --- common/lib/dependabot/errors.rb | 35 ++++++------------- npm_and_yarn/lib/dependabot/npm_and_yarn.rb | 15 ++++++-- .../npm_and_yarn/file_updater_spec.rb | 6 ++-- 3 files changed, 26 insertions(+), 30 deletions(-) diff --git a/common/lib/dependabot/errors.rb b/common/lib/dependabot/errors.rb index 7aa19999404..8ed2c4c8446 100644 --- a/common/lib/dependabot/errors.rb +++ b/common/lib/dependabot/errors.rb @@ -213,7 +213,8 @@ def self.updater_error_details(error) { "error-type": "missing_environment_variable", "error-detail": { - "environment-variable": error.environment_variable + "environment-variable": error.environment_variable, + "error-message": error.message } } when Dependabot::GoModulePathMismatch @@ -242,11 +243,6 @@ def self.updater_error_details(error) "error-type": "git_token_auth_error", "error-detail": { message: error.message } } - when Dependabot::FailedToReplaceEnvInConfig - { - "error-type": "failed_to_replace_env", - "error-detail": { message: error.message } - } when *Octokit::RATE_LIMITED_ERRORS # If we get a rate-limited error we let dependabot-api handle the # retry by re-enqueing the update job after the reset @@ -345,8 +341,6 @@ class NotImplemented < DependabotError; end class InvalidGitAuthToken < DependabotError; end - class FailedToReplaceEnvInConfig < DependabotError; end - ##################### # Repo level errors # ##################### @@ -557,10 +551,15 @@ class MissingEnvironmentVariable < DependabotError sig { returns(String) } attr_reader :environment_variable - sig { params(environment_variable: String).void } - def initialize(environment_variable) + sig { returns(String) } + attr_reader :message + + sig { params(environment_variable: String, message: T.nilable(String)).void } + def initialize(environment_variable, message = "") @environment_variable = environment_variable - super("Missing environment variable #{@environment_variable}") + @message = message + + super("Missing environment variable #{@environment_variable}. #{@message}") end end @@ -592,20 +591,6 @@ def initialize(source) end end - class FailedToReplaceEnvInConfig < DependabotError - extend T::Sig - - sig { returns(String) } - attr_reader :source - - sig { params(source: String).void } - def initialize(source) - @source = T.let(sanitize_source(source), String) - msg = @source.to_s - super(msg) - end - end - # Useful for JS file updaters, where the registry API sometimes returns # different results to the actual update process class InconsistentRegistryResponse < DependabotError; end diff --git a/npm_and_yarn/lib/dependabot/npm_and_yarn.rb b/npm_and_yarn/lib/dependabot/npm_and_yarn.rb index eec42b9d2bc..660e32a9f05 100644 --- a/npm_and_yarn/lib/dependabot/npm_and_yarn.rb +++ b/npm_and_yarn/lib/dependabot/npm_and_yarn.rb @@ -74,7 +74,7 @@ module NpmAndYarn # Used to identify if error message is related to yarn workspaces DEPENDENCY_FILE_NOT_RESOLVABLE = "conflicts with direct dependency" - ENV_VAR_NOT_RESOLVABLE = /Failed to replace env in config: / + ENV_VAR_NOT_RESOLVABLE = /Failed to replace env in config: \$\{(?.*)\}/ class Utils extend T::Sig @@ -89,6 +89,14 @@ def self.extract_node_versions(error_message) required_version: match_data[:required_version] } end + + sig { params(error_message: String).returns(String) } + def self.extract_var(error_message) + match_data = error_message.match(ENV_VAR_NOT_RESOLVABLE) + return nil unless match_data + + match_data[:var] + end end YARN_CODE_REGEX = /(YN\d{4})/ @@ -196,7 +204,10 @@ def self.extract_node_versions(error_message) }, { patterns: [ENV_VAR_NOT_RESOLVABLE], - new_error: ->(_error, message) { FailedToReplaceEnvInConfig.new(message) }, + new_error: lambda { |_error, message| + var = Utils.extract_var(message) + Dependabot::MissingEnvironmentVariable.new(var, message) + }, in_usage: false, matchfn: nil } diff --git a/npm_and_yarn/spec/dependabot/npm_and_yarn/file_updater_spec.rb b/npm_and_yarn/spec/dependabot/npm_and_yarn/file_updater_spec.rb index 78a5a1b3bfb..9bd142dae6d 100644 --- a/npm_and_yarn/spec/dependabot/npm_and_yarn/file_updater_spec.rb +++ b/npm_and_yarn/spec/dependabot/npm_and_yarn/file_updater_spec.rb @@ -3297,7 +3297,7 @@ it "keeps the preference for the npm registry" do expect { updated_yarn_lock }.to raise_error do |error| - expect(error).to be_a(Dependabot::FailedToReplaceEnvInConfig) + expect(error).to be_a(Dependabot::MissingEnvironmentVariable) expect(error.message).to include("Failed to replace env in config: ${NPM_TOKEN}") end end @@ -3361,7 +3361,7 @@ it "keeps the preference for the npm registry" do expect { updated_yarn_lock }.to raise_error do |error| - expect(error).to be_a(Dependabot::FailedToReplaceEnvInConfig) + expect(error).to be_a(Dependabot::MissingEnvironmentVariable) expect(error.message).to include("Failed to replace env in config: ${PACKAGECLOUD_TOKEN}") end end @@ -3379,7 +3379,7 @@ it "keeps the preference for the npm registry" do expect { updated_yarn_lock }.to raise_error do |error| - expect(error).to be_a(Dependabot::FailedToReplaceEnvInConfig) + expect(error).to be_a(Dependabot::MissingEnvironmentVariable) expect(error.message).to include("Failed to replace env in config: ${FONTAWESOME_NPM_AUTH_TOKEN}") end end From f89d42341940e1298a918f15be3d1ba8514f486e Mon Sep 17 00:00:00 2001 From: sachin-sandhu Date: Thu, 18 Jul 2024 18:30:11 -0400 Subject: [PATCH 3/4] fixes sorbet --- common/lib/dependabot/errors.rb | 2 +- npm_and_yarn/lib/dependabot/npm_and_yarn.rb | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/common/lib/dependabot/errors.rb b/common/lib/dependabot/errors.rb index 8ed2c4c8446..2f6d86fa9ce 100644 --- a/common/lib/dependabot/errors.rb +++ b/common/lib/dependabot/errors.rb @@ -554,7 +554,7 @@ class MissingEnvironmentVariable < DependabotError sig { returns(String) } attr_reader :message - sig { params(environment_variable: String, message: T.nilable(String)).void } + sig { params(environment_variable: String, message: String).void } def initialize(environment_variable, message = "") @environment_variable = environment_variable @message = message diff --git a/npm_and_yarn/lib/dependabot/npm_and_yarn.rb b/npm_and_yarn/lib/dependabot/npm_and_yarn.rb index 660e32a9f05..a5d84c80665 100644 --- a/npm_and_yarn/lib/dependabot/npm_and_yarn.rb +++ b/npm_and_yarn/lib/dependabot/npm_and_yarn.rb @@ -93,9 +93,9 @@ def self.extract_node_versions(error_message) sig { params(error_message: String).returns(String) } def self.extract_var(error_message) match_data = error_message.match(ENV_VAR_NOT_RESOLVABLE) - return nil unless match_data + return "" unless match_data - match_data[:var] + T.must(match_data[:var]) end end From 47f679116473064c49298054c45478eb950a3e1a Mon Sep 17 00:00:00 2001 From: sachin-sandhu Date: Thu, 18 Jul 2024 20:26:56 -0400 Subject: [PATCH 4/4] reg ex changes --- npm_and_yarn/lib/dependabot/npm_and_yarn.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/npm_and_yarn/lib/dependabot/npm_and_yarn.rb b/npm_and_yarn/lib/dependabot/npm_and_yarn.rb index a5d84c80665..31dba18c806 100644 --- a/npm_and_yarn/lib/dependabot/npm_and_yarn.rb +++ b/npm_and_yarn/lib/dependabot/npm_and_yarn.rb @@ -92,10 +92,10 @@ def self.extract_node_versions(error_message) sig { params(error_message: String).returns(String) } def self.extract_var(error_message) - match_data = error_message.match(ENV_VAR_NOT_RESOLVABLE) + match_data = T.must(error_message.match(ENV_VAR_NOT_RESOLVABLE)).named_captures["var"] return "" unless match_data - T.must(match_data[:var]) + match_data end end