From 918333faad113a6487d972c9f9397ae0a5e7088f Mon Sep 17 00:00:00 2001 From: Jeff Widman Date: Thu, 10 Aug 2023 12:40:06 -0700 Subject: [PATCH] Add `name` key to `sources` in `Pipfile`s (#7744) Newer versions of `pipenv` require any specified `sources` to be explicitly `name`'d. More context here: https://github.com/pypa/pipenv/discussions/5370#discussioncomment-3701061 This has been true since at least Sept 2022. Our version of `pipenv` is from April of 2022, so it doesn't complain. But it will as soon as we upgrade. Also, for sources that :dependabot: dynamically injects into the `Pipfile`, they need a name. These sources are stripped from the final `Pipfile` / `Pipfile.lock` during the `FileUpdater#post_process_lockfile` method, so all we need is a placeholder `name` to placate `pipenv`. Long term, we may want to add custom error handling to flag this missing key as a `Dependabot::DependencyFileNotResolvable` error. But I decided that was out of scope for now as this PR does not generate the error... that will not happen until we upgrade to newer `pipenv`. And even at that point, our first priority will be upgrading, and then from there handling any new errors that start popping up. And even then, most of the active users of `pipenv` are unlikely to see this error because they're likely running a newer version of `pipenv`. --- .../python/file_updater/pipfile_preparer.rb | 9 ++++-- .../file_updater/pipfile_file_updater_spec.rb | 28 +++++++++++++++++++ .../file_updater/pipfile_preparer_spec.rb | 16 +++++++---- .../fixtures/pipfile_files/arbitrary_equality | 1 + .../pipfile_files/conflict_at_current | 1 + .../fixtures/pipfile_files/conflict_at_latest | 1 + .../pipfile_files/environment_variable_source | 1 + .../spec/fixtures/pipfile_files/exact_version | 1 + .../pipfile_files/extra_subdependency | 2 +- python/spec/fixtures/pipfile_files/git_source | 1 + .../fixtures/pipfile_files/git_source_bad_ref | 1 + .../fixtures/pipfile_files/git_source_no_ref | 1 + .../pipfile_files/git_source_unreachable | 1 + python/spec/fixtures/pipfile_files/hard_names | 1 + .../fixtures/pipfile_files/not_in_lockfile | 1 + python/spec/fixtures/pipfile_files/only_dev | 1 + .../fixtures/pipfile_files/path_dependency | 1 + .../pipfile_files/path_dependency_not_self | 1 + .../fixtures/pipfile_files/private_source | 1 + .../pipfile_files/private_source_auth | 2 +- .../spec/fixtures/pipfile_files/prod_and_dev | 1 + .../pipfile_files/prod_and_dev_different | 1 + .../fixtures/pipfile_files/required_python | 1 + .../pipfile_files/required_python_implicit | 1 + .../pipfile_files/required_python_invalid | 1 + .../pipfile_files/required_python_unsupported | 1 + .../spec/fixtures/pipfile_files/unparseable | 3 +- .../fixtures/pipfile_files/unsupported_dep | 2 +- .../spec/fixtures/pipfile_files/version_hash | 1 + .../pipfile_files/version_not_specified | 1 + .../pipfile_files/version_not_specified.lock | 3 +- .../spec/fixtures/pipfile_files/version_table | 1 + python/spec/fixtures/pipfile_files/wildcard | 1 + .../spec/fixtures/pipfile_files/with_quotes | 1 + python/spec/fixtures/pipfile_files/yanked | 1 + 35 files changed, 78 insertions(+), 14 deletions(-) diff --git a/python/lib/dependabot/python/file_updater/pipfile_preparer.rb b/python/lib/dependabot/python/file_updater/pipfile_preparer.rb index cb3285439f7..bb77e5b9a69 100644 --- a/python/lib/dependabot/python/file_updater/pipfile_preparer.rb +++ b/python/lib/dependabot/python/file_updater/pipfile_preparer.rb @@ -130,9 +130,12 @@ def sub_auth_url(source, credentials) def config_variable_sources(credentials) @config_variable_sources ||= - credentials. - select { |cred| cred["type"] == "python_index" }. - map { |c| { "url" => AuthedUrlBuilder.authed_url(credential: c) } } + credentials.select { |cred| cred["type"] == "python_index" }.map.with_index do |c, i| + { + "name" => "dependabot-inserted-index-#{i}", + "url" => AuthedUrlBuilder.authed_url(credential: c) + } + end end end end diff --git a/python/spec/dependabot/python/file_updater/pipfile_file_updater_spec.rb b/python/spec/dependabot/python/file_updater/pipfile_file_updater_spec.rb index 136d0558864..f40378f7509 100644 --- a/python/spec/dependabot/python/file_updater/pipfile_file_updater_spec.rb +++ b/python/spec/dependabot/python/file_updater/pipfile_file_updater_spec.rb @@ -179,6 +179,34 @@ end end + context "with a source not included in the original Pipfile" do + let(:credentials) do + [ + { + "type" => "git_source", + "host" => "github.com", + "username" => "x-access-token", + "password" => "token" + }, + { + "type" => "python_index", + "index-url" => "https://pypi.posrip.com/pypi/" + } + ] + end + + it "the source is not included in the final updated files" do + expect(updated_files.map(&:name)).to eq(%w(Pipfile.lock)) # because Pipfile shouldn't have changed + + updated_lockfile = updated_files.find { |f| f.name == "Pipfile.lock" } + expect(updated_lockfile.content).not_to include("dependabot-inserted-index") + expect(updated_lockfile.content).not_to include("https://pypi.posrip.com/pypi/") + + json_lockfile = JSON.parse(updated_lockfile.content) + expect(json_lockfile["_meta"]["sources"]).to eq(JSON.parse(lockfile.content)["_meta"]["sources"]) + end + end + context "when the Pipfile included an environment variable source" do let(:pipfile_fixture_name) { "environment_variable_source" } let(:lockfile_fixture_name) { "environment_variable_source.lock" } diff --git a/python/spec/dependabot/python/file_updater/pipfile_preparer_spec.rb b/python/spec/dependabot/python/file_updater/pipfile_preparer_spec.rb index b8cd68fff96..710707d4458 100644 --- a/python/spec/dependabot/python/file_updater/pipfile_preparer_spec.rb +++ b/python/spec/dependabot/python/file_updater/pipfile_preparer_spec.rb @@ -135,8 +135,11 @@ let(:lockfile_fixture_name) { "version_not_specified.lock" } it "adds the source" do - expect(updated_content). - to include("https://username:password@pypi.posrip.com/pypi/") + expect(updated_content).to include( + "[[source]]\n" \ + "name = \"dependabot-inserted-index-0\"\n" \ + "url = \"https://username:password@pypi.posrip.com/pypi/\"\n" + ) end context "with auth details provided as a token" do @@ -154,8 +157,11 @@ end it "adds the source" do - expect(updated_content). - to include("https://username:password@pypi.posrip.com/pypi/") + expect(updated_content).to include( + "[[source]]\n" \ + "name = \"dependabot-inserted-index-0\"\n" \ + "url = \"https://username:password@pypi.posrip.com/pypi/\"\n" + ) end end @@ -178,7 +184,7 @@ it "keeps source config" do expect(updated_content).to include( "[[source]]\n" \ - "name = \"pypi\"\n" \ + "name = \"internal-pypi\"\n" \ "url = \"https://username:password@pypi.posrip.com/pypi/\"\n" \ "verify_ssl = true\n" ) diff --git a/python/spec/fixtures/pipfile_files/arbitrary_equality b/python/spec/fixtures/pipfile_files/arbitrary_equality index 76f9360fcc3..684fe70d565 100644 --- a/python/spec/fixtures/pipfile_files/arbitrary_equality +++ b/python/spec/fixtures/pipfile_files/arbitrary_equality @@ -1,4 +1,5 @@ [[source]] +name = "pypi" url = "https://pypi.org/simple" verify_ssl = true diff --git a/python/spec/fixtures/pipfile_files/conflict_at_current b/python/spec/fixtures/pipfile_files/conflict_at_current index e97cb128a09..8a19ad69169 100644 --- a/python/spec/fixtures/pipfile_files/conflict_at_current +++ b/python/spec/fixtures/pipfile_files/conflict_at_current @@ -1,4 +1,5 @@ [[source]] +name = "pypi" url = "https://pypi.org/simple" verify_ssl = true diff --git a/python/spec/fixtures/pipfile_files/conflict_at_latest b/python/spec/fixtures/pipfile_files/conflict_at_latest index 7cced80f368..be6abd85be3 100644 --- a/python/spec/fixtures/pipfile_files/conflict_at_latest +++ b/python/spec/fixtures/pipfile_files/conflict_at_latest @@ -1,4 +1,5 @@ [[source]] +name = "pypi" url = "https://pypi.org/simple" verify_ssl = true diff --git a/python/spec/fixtures/pipfile_files/environment_variable_source b/python/spec/fixtures/pipfile_files/environment_variable_source index 71101787074..44b76979eab 100644 --- a/python/spec/fixtures/pipfile_files/environment_variable_source +++ b/python/spec/fixtures/pipfile_files/environment_variable_source @@ -1,4 +1,5 @@ [[source]] +name = "pypi" url = "https://pypi.org/${ENV_VAR}" verify_ssl = true diff --git a/python/spec/fixtures/pipfile_files/exact_version b/python/spec/fixtures/pipfile_files/exact_version index 6bef68998e7..aaa0096585f 100644 --- a/python/spec/fixtures/pipfile_files/exact_version +++ b/python/spec/fixtures/pipfile_files/exact_version @@ -1,4 +1,5 @@ [[source]] +name = "pypi" url = "https://pypi.org/simple" verify_ssl = true diff --git a/python/spec/fixtures/pipfile_files/extra_subdependency b/python/spec/fixtures/pipfile_files/extra_subdependency index d3b28dead39..58ee60c2682 100644 --- a/python/spec/fixtures/pipfile_files/extra_subdependency +++ b/python/spec/fixtures/pipfile_files/extra_subdependency @@ -1,7 +1,7 @@ [[source]] +name = "pypi" url = "https://pypi.org/simple" verify_ssl = true -name = "pypi" [packages] flask = "==1.0.*" diff --git a/python/spec/fixtures/pipfile_files/git_source b/python/spec/fixtures/pipfile_files/git_source index a2edea4a305..827aaedbbcb 100644 --- a/python/spec/fixtures/pipfile_files/git_source +++ b/python/spec/fixtures/pipfile_files/git_source @@ -1,4 +1,5 @@ [[source]] +name = "pypi" url = "https://pypi.org/simple" verify_ssl = true diff --git a/python/spec/fixtures/pipfile_files/git_source_bad_ref b/python/spec/fixtures/pipfile_files/git_source_bad_ref index 8310b7aee0b..b3ca7d30023 100644 --- a/python/spec/fixtures/pipfile_files/git_source_bad_ref +++ b/python/spec/fixtures/pipfile_files/git_source_bad_ref @@ -1,4 +1,5 @@ [[source]] +name = "pypi" url = "https://pypi.org/simple" verify_ssl = true diff --git a/python/spec/fixtures/pipfile_files/git_source_no_ref b/python/spec/fixtures/pipfile_files/git_source_no_ref index d3abd5a1203..24966cbc35d 100644 --- a/python/spec/fixtures/pipfile_files/git_source_no_ref +++ b/python/spec/fixtures/pipfile_files/git_source_no_ref @@ -1,4 +1,5 @@ [[source]] +name = "pypi" url = "https://pypi.org/simple" verify_ssl = true diff --git a/python/spec/fixtures/pipfile_files/git_source_unreachable b/python/spec/fixtures/pipfile_files/git_source_unreachable index 20b0c74db8f..4df40330391 100644 --- a/python/spec/fixtures/pipfile_files/git_source_unreachable +++ b/python/spec/fixtures/pipfile_files/git_source_unreachable @@ -1,4 +1,5 @@ [[source]] +name = "pypi" url = "https://pypi.org/simple" verify_ssl = true diff --git a/python/spec/fixtures/pipfile_files/hard_names b/python/spec/fixtures/pipfile_files/hard_names index 936a548a32b..840fb65bd6a 100644 --- a/python/spec/fixtures/pipfile_files/hard_names +++ b/python/spec/fixtures/pipfile_files/hard_names @@ -1,4 +1,5 @@ [[source]] +name = "pypi" url = "https://pypi.org/simple" verify_ssl = true diff --git a/python/spec/fixtures/pipfile_files/not_in_lockfile b/python/spec/fixtures/pipfile_files/not_in_lockfile index ed900ef1a2a..b67e707ff83 100644 --- a/python/spec/fixtures/pipfile_files/not_in_lockfile +++ b/python/spec/fixtures/pipfile_files/not_in_lockfile @@ -1,4 +1,5 @@ [[source]] +name = "pypi" url = "https://pypi.org/simple" verify_ssl = true diff --git a/python/spec/fixtures/pipfile_files/only_dev b/python/spec/fixtures/pipfile_files/only_dev index 3637b1a9c37..651b5b24d97 100644 --- a/python/spec/fixtures/pipfile_files/only_dev +++ b/python/spec/fixtures/pipfile_files/only_dev @@ -1,4 +1,5 @@ [[source]] +name = "pypi" url = "https://pypi.org/simple" verify_ssl = true diff --git a/python/spec/fixtures/pipfile_files/path_dependency b/python/spec/fixtures/pipfile_files/path_dependency index 4f654b7ffd4..7ffa1519c75 100644 --- a/python/spec/fixtures/pipfile_files/path_dependency +++ b/python/spec/fixtures/pipfile_files/path_dependency @@ -1,4 +1,5 @@ [[source]] +name = "pypi" url = "https://pypi.org/simple" verify_ssl = true diff --git a/python/spec/fixtures/pipfile_files/path_dependency_not_self b/python/spec/fixtures/pipfile_files/path_dependency_not_self index 35d5aeb6c0f..93840f836bc 100644 --- a/python/spec/fixtures/pipfile_files/path_dependency_not_self +++ b/python/spec/fixtures/pipfile_files/path_dependency_not_self @@ -1,4 +1,5 @@ [[source]] +name = "pypi" url = "https://pypi.org/simple" verify_ssl = true diff --git a/python/spec/fixtures/pipfile_files/private_source b/python/spec/fixtures/pipfile_files/private_source index 36ab600974d..bd95a2d8ff3 100644 --- a/python/spec/fixtures/pipfile_files/private_source +++ b/python/spec/fixtures/pipfile_files/private_source @@ -1,4 +1,5 @@ [[source]] +name = "internal-pypi" url = "https://some.internal.registry.com/pypi/" verify_ssl = true diff --git a/python/spec/fixtures/pipfile_files/private_source_auth b/python/spec/fixtures/pipfile_files/private_source_auth index 07c39638343..ae36a45e301 100644 --- a/python/spec/fixtures/pipfile_files/private_source_auth +++ b/python/spec/fixtures/pipfile_files/private_source_auth @@ -1,7 +1,7 @@ [[source]] +name = "internal-pypi" url = "https://${ENV_USER}:${ENV_PASSWORD}@pypi.posrip.com/pypi/" verify_ssl = true -name = "pypi" [dev-packages] pytest = "==3.4.0" diff --git a/python/spec/fixtures/pipfile_files/prod_and_dev b/python/spec/fixtures/pipfile_files/prod_and_dev index 83c1f46916d..651703d5086 100644 --- a/python/spec/fixtures/pipfile_files/prod_and_dev +++ b/python/spec/fixtures/pipfile_files/prod_and_dev @@ -1,4 +1,5 @@ [[source]] +name = "pypi" url = "https://pypi.org/simple" verify_ssl = true diff --git a/python/spec/fixtures/pipfile_files/prod_and_dev_different b/python/spec/fixtures/pipfile_files/prod_and_dev_different index 34eb4afc28d..893ef34fa65 100644 --- a/python/spec/fixtures/pipfile_files/prod_and_dev_different +++ b/python/spec/fixtures/pipfile_files/prod_and_dev_different @@ -1,4 +1,5 @@ [[source]] +name = "pypi" url = "https://pypi.org/simple" verify_ssl = true diff --git a/python/spec/fixtures/pipfile_files/required_python b/python/spec/fixtures/pipfile_files/required_python index 10c96ff9263..80fdf3f1ebe 100644 --- a/python/spec/fixtures/pipfile_files/required_python +++ b/python/spec/fixtures/pipfile_files/required_python @@ -1,4 +1,5 @@ [[source]] +name = "pypi" url = "https://pypi.org/simple" verify_ssl = true diff --git a/python/spec/fixtures/pipfile_files/required_python_implicit b/python/spec/fixtures/pipfile_files/required_python_implicit index 3626f5d6cce..e02788e7517 100644 --- a/python/spec/fixtures/pipfile_files/required_python_implicit +++ b/python/spec/fixtures/pipfile_files/required_python_implicit @@ -1,4 +1,5 @@ [[source]] +name = "pypi" url = "https://pypi.org/simple" verify_ssl = true diff --git a/python/spec/fixtures/pipfile_files/required_python_invalid b/python/spec/fixtures/pipfile_files/required_python_invalid index 6158e841667..44c43aba04f 100644 --- a/python/spec/fixtures/pipfile_files/required_python_invalid +++ b/python/spec/fixtures/pipfile_files/required_python_invalid @@ -1,4 +1,5 @@ [[source]] +name = "pypi" url = "https://pypi.org/simple" verify_ssl = true diff --git a/python/spec/fixtures/pipfile_files/required_python_unsupported b/python/spec/fixtures/pipfile_files/required_python_unsupported index 481f7a99f15..f957d73a448 100644 --- a/python/spec/fixtures/pipfile_files/required_python_unsupported +++ b/python/spec/fixtures/pipfile_files/required_python_unsupported @@ -1,4 +1,5 @@ [[source]] +name = "pypi" url = "https://pypi.org/simple" verify_ssl = true diff --git a/python/spec/fixtures/pipfile_files/unparseable b/python/spec/fixtures/pipfile_files/unparseable index 67ba5f6e7b4..cd5b790b1ad 100644 --- a/python/spec/fixtures/pipfile_files/unparseable +++ b/python/spec/fixtures/pipfile_files/unparseable @@ -1,8 +1,7 @@ [[source]] - +name = "pypi" url = "https://pypi.org/simple" verify_ssl = true -name = "pypi" [dev-packages] diff --git a/python/spec/fixtures/pipfile_files/unsupported_dep b/python/spec/fixtures/pipfile_files/unsupported_dep index d28e51afc38..d09870896b0 100644 --- a/python/spec/fixtures/pipfile_files/unsupported_dep +++ b/python/spec/fixtures/pipfile_files/unsupported_dep @@ -1,7 +1,7 @@ [[source]] +name = "pypi" url = "https://pypi.org/simple" verify_ssl = true -name = "pypi" [packages] requests = "==2.18.0" diff --git a/python/spec/fixtures/pipfile_files/version_hash b/python/spec/fixtures/pipfile_files/version_hash index 3dfd68752f7..6514e4a2e9f 100644 --- a/python/spec/fixtures/pipfile_files/version_hash +++ b/python/spec/fixtures/pipfile_files/version_hash @@ -1,4 +1,5 @@ [[source]] +name = "pypi" url = "https://pypi.org/simple" verify_ssl = true diff --git a/python/spec/fixtures/pipfile_files/version_not_specified b/python/spec/fixtures/pipfile_files/version_not_specified index 4488c424425..6cb20258d45 100644 --- a/python/spec/fixtures/pipfile_files/version_not_specified +++ b/python/spec/fixtures/pipfile_files/version_not_specified @@ -1,4 +1,5 @@ [[source]] +name = "pypi" url = "https://pypi.org/simple" verify_ssl = true diff --git a/python/spec/fixtures/pipfile_files/version_not_specified.lock b/python/spec/fixtures/pipfile_files/version_not_specified.lock index b82814317e2..35c73b8e759 100644 --- a/python/spec/fixtures/pipfile_files/version_not_specified.lock +++ b/python/spec/fixtures/pipfile_files/version_not_specified.lock @@ -1,12 +1,13 @@ { "_meta": { "hash": { - "sha256": "c402ea48092e9d467af51a483bb8dd8ad0620e11c94f009dcd433f97a99d45db" + "sha256": "e76ae491d793d659f05c7f7eab261fd6167dc062efcba08f17be68e73eb87665" }, "pipfile-spec": 6, "requires": {}, "sources": [ { + "name": "pypi", "url": "https://pypi.org/simple", "verify_ssl": true } diff --git a/python/spec/fixtures/pipfile_files/version_table b/python/spec/fixtures/pipfile_files/version_table index ea9df9dd61c..f72cea8958c 100644 --- a/python/spec/fixtures/pipfile_files/version_table +++ b/python/spec/fixtures/pipfile_files/version_table @@ -1,4 +1,5 @@ [[source]] +name = "pypi" url = "https://pypi.org/simple" verify_ssl = true diff --git a/python/spec/fixtures/pipfile_files/wildcard b/python/spec/fixtures/pipfile_files/wildcard index 6dd6d650864..2cde9a7cdfe 100644 --- a/python/spec/fixtures/pipfile_files/wildcard +++ b/python/spec/fixtures/pipfile_files/wildcard @@ -1,4 +1,5 @@ [[source]] +name = "pypi" url = "https://pypi.org/simple" verify_ssl = true diff --git a/python/spec/fixtures/pipfile_files/with_quotes b/python/spec/fixtures/pipfile_files/with_quotes index ffce80a7e84..89ea1a02240 100644 --- a/python/spec/fixtures/pipfile_files/with_quotes +++ b/python/spec/fixtures/pipfile_files/with_quotes @@ -1,4 +1,5 @@ [[source]] +name = "pypi" url = "https://pypi.org/simple" verify_ssl = true diff --git a/python/spec/fixtures/pipfile_files/yanked b/python/spec/fixtures/pipfile_files/yanked index c8c41005edf..fcd8009680a 100644 --- a/python/spec/fixtures/pipfile_files/yanked +++ b/python/spec/fixtures/pipfile_files/yanked @@ -1,4 +1,5 @@ [[source]] +name = "pypi" url = "https://pypi.org/simple" verify_ssl = true