Rely on global Go environment variables for all commands

This makes it easier to configure things correctly once, since the local
variables would otherwise override what is configured via go.env

Author: jurre

go_modules/lib/dependabot/go_modules/file_parser.rb

@@ -68,6 +68,10 @@ def set_go_environment_variables
           ENV["GOENV"] = go_env_path
         end
 
+        # Set GOPRIVATE from options if provided and not already set in go.env
+        goprivate = options.fetch(:goprivate, "*")
+        ENV["GOPRIVATE"] = goprivate if goprivate && (!go_env || !T.must(go_env).content&.include?("GOPRIVATE"))
+
         # We set the GOPROXY environment variable if there are any
         # goproxy_server credentials, from here the Go toolchain will
         # use the configured proxy to fetch dependencies.

go_modules/lib/dependabot/go_modules/file_updater.rb

@@ -29,7 +29,6 @@ class FileUpdater < Dependabot::FileUpdaters::Base
       def initialize(dependencies:, dependency_files:, credentials:, repo_contents_path: nil, options: {})
         super
 
-        @goprivate = T.let(options.fetch(:goprivate, "*"), String)
         use_repo_contents_stub if repo_contents_path.nil?
       end
 
@@ -149,7 +148,7 @@ def file_updater
             credentials: credentials,
             repo_contents_path: repo_contents_path,
             directory: T.must(directory),
-            options: { tidy: tidy?, vendor: vendor?, goprivate: @goprivate }
+            options: { tidy: tidy?, vendor: vendor? }
           ),
           T.nilable(Dependabot::GoModules::FileUpdater::GoModUpdater)
         )

go_modules/lib/dependabot/go_modules/file_updater/go_mod_updater.rb

@@ -96,7 +96,6 @@ def initialize(dependencies:, dependency_files:, credentials:, repo_contents_pat
           @directory = directory
           @tidy = T.let(options.fetch(:tidy, false), T::Boolean)
           @vendor = T.let(options.fetch(:vendor, false), T::Boolean)
-          @goprivate = T.let(options.fetch(:goprivate), T.nilable(String))
         end
 
         sig { returns(T.nilable(String)) }
@@ -188,7 +187,7 @@ def run_go_mod_tidy
           # continue with an info log here. `go mod tidy` shouldn't block
           # updating versions because there are some edge cases where it's OK to fail
           # (such as generated files not available yet to us).
-          _, stderr, status = Open3.capture3(environment, command)
+          _, stderr, status = Open3.capture3(command)
           if status.success?
             Dependabot.logger.info "`go mod tidy` succeeded"
           else
@@ -201,7 +200,7 @@ def run_go_vendor
           return unless vendor?
 
           command = "go mod vendor"
-          _, stderr, status = Open3.capture3(environment, command)
+          _, stderr, status = Open3.capture3(command)
           handle_subprocess_error(stderr) unless status.success?
         end
 
@@ -225,7 +224,7 @@ def run_go_get(dependencies = [])
           end
           command = SharedHelpers.escape_command(command)
 
-          _, stderr, status = Open3.capture3(environment, command)
+          _, stderr, status = Open3.capture3(command)
           handle_subprocess_error(stderr) unless status.success?
         ensure
           FileUtils.rm_f(T.must(tmp_go_file))
@@ -234,7 +233,7 @@ def run_go_get(dependencies = [])
         sig { returns(T::Hash[String, T.untyped]) }
         def parse_manifest
           command = "go mod edit -json"
-          stdout, stderr, status = Open3.capture3(environment, command)
+          stdout, stderr, status = Open3.capture3(command)
           handle_subprocess_error(stderr) unless status.success?
 
           JSON.parse(stdout) || {}
@@ -305,7 +304,7 @@ def handle_subprocess_error(stderr) # rubocop:disable Metrics/AbcSize
           end
 
           repo_error_regex = REPO_RESOLVABILITY_ERROR_REGEXES.find { |r| stderr =~ r }
-          ResolvabilityErrors.handle(stderr, goprivate: @goprivate) if repo_error_regex
+          ResolvabilityErrors.handle(stderr) if repo_error_regex
 
           path_regex = MODULE_PATH_MISMATCH_REGEXES.find { |r| stderr =~ r }
           if path_regex
@@ -366,11 +365,6 @@ def tidy?
         def vendor?
           !!@vendor
         end
-
-        sig { returns(T::Hash[String, T.untyped]) }
-        def environment
-          { "GOPRIVATE" => @goprivate }
-        end
       end
     end
   end

go_modules/lib/dependabot/go_modules/go_environment_helpers.rb

@@ -0,0 +1,22 @@
+# typed: strict
+# frozen_string_literal: true
+
+require "sorbet-runtime"
+
+module Dependabot
+  module GoModules
+    module GoEnvironmentHelpers
+      extend T::Sig
+
+      sig { returns(T::Hash[String, T.untyped]) }
+      def go_environment
+        @go_environment ||= T.let(begin
+          existing_goprivate = `go env GOPRIVATE`.strip
+          return {} unless existing_goprivate.empty?
+
+          { "GOPRIVATE" => @goprivate }
+        end, T.nilable(T::Hash[T.untyped, T.untyped]))
+      end
+    end
+  end
+end

go_modules/lib/dependabot/go_modules/package/package_details_fetcher.rb

@@ -41,15 +41,13 @@ class PackageDetailsFetcher
           params(
             dependency: Dependabot::Dependency,
             dependency_files: T::Array[Dependabot::DependencyFile],
-            credentials: T::Array[Dependabot::Credential],
-            goprivate: String
+            credentials: T::Array[Dependabot::Credential]
           ).void
         end
-        def initialize(dependency:, dependency_files:, credentials:, goprivate:)
+        def initialize(dependency:, dependency_files:, credentials:)
           @dependency = dependency
           @dependency_files = dependency_files
           @credentials = credentials
-          @goprivate = T.let(goprivate, String)
 
           @source_type = T.let(nil, T.nilable(String))
         end
@@ -63,9 +61,6 @@ def initialize(dependency:, dependency_files:, credentials:, goprivate:)
         sig { returns(T::Array[T.untyped]) }
         attr_reader :credentials
 
-        sig { returns(String) }
-        attr_reader :goprivate
-
         # rubocop:disable Metrics/AbcSize,Metrics/PerceivedComplexity
         sig { returns(T::Array[Dependabot::Package::PackageRelease]) }
         def fetch_available_versions
@@ -82,12 +77,9 @@ def fetch_available_versions
               end
 
               # Turn off the module proxy for private dependencies
-              env = { "GOPRIVATE" => @goprivate }
-
               versions_json = SharedHelpers.run_shell_command(
                 "go list -m -versions -json #{dependency.name}",
-                fingerprint: "go list -m -versions -json <dependency_name>",
-                env: env
+                fingerprint: "go list -m -versions -json <dependency_name>"
               )
               version_strings = JSON.parse(versions_json)["Versions"]
 
@@ -112,7 +104,7 @@ def fetch_available_versions
           retry_count += 1
           retry if transitory_failure?(e) && retry_count < 2
 
-          ResolvabilityErrors.handle(e.message, goprivate: @goprivate)
+          ResolvabilityErrors.handle(e.message)
           [package_release(version: T.must(dependency.version))]
         end
         # rubocop:enable Metrics/AbcSize,Metrics/PerceivedComplexity

go_modules/lib/dependabot/go_modules/resolvability_errors.rb

@@ -10,8 +10,8 @@ module ResolvabilityErrors
 
       GITHUB_REPO_REGEX = %r{github.com/[^:@]*}
 
-      sig { params(message: String, goprivate: T.untyped).void }
-      def self.handle(message, goprivate:)
+      sig { params(message: String).void }
+      def self.handle(message)
         mod_path = message.scan(GITHUB_REPO_REGEX).last
         unless mod_path && message.include?("If this is a private repository")
           raise Dependabot::DependencyFileNotResolvable, message
@@ -30,8 +30,7 @@ def self.handle(message, goprivate:)
                         mod_path
                       end
 
-          env = { "GOPRIVATE" => goprivate }
-          _, _, status = Open3.capture3(env, SharedHelpers.escape_command("go list -m -versions #{repo_path}"))
+          _, _, status = Open3.capture3(SharedHelpers.escape_command("go list -m -versions #{repo_path}"))
           raise Dependabot::DependencyFileNotResolvable, message if status.success?
 
           raise Dependabot::GitDependenciesNotReachable, [repo_path]

go_modules/lib/dependabot/go_modules/update_checker.rb

@@ -66,8 +66,7 @@ def latest_version_finder
             ignored_versions: ignored_versions,
             security_advisories: security_advisories,
             raise_on_ignored: raise_on_ignored,
-            cooldown_options: update_cooldown,
-            goprivate: options.fetch(:goprivate, "*")
+            cooldown_options: update_cooldown
           ),
           T.nilable(Dependabot::GoModules::UpdateChecker::LatestVersionFinder)
         )

go_modules/lib/dependabot/go_modules/update_checker/latest_version_finder.rb

@@ -46,7 +46,6 @@ class LatestVersionFinder < Dependabot::Package::PackageLatestVersionFinder
             credentials: T::Array[Dependabot::Credential],
             ignored_versions: T::Array[String],
             security_advisories: T::Array[Dependabot::SecurityAdvisory],
-            goprivate: String,
             raise_on_ignored: T::Boolean,
             cooldown_options: T.nilable(Dependabot::Package::ReleaseCooldownOptions)
           )
@@ -58,7 +57,6 @@ def initialize(
           credentials:,
           ignored_versions:,
           security_advisories:,
-          goprivate:,
           raise_on_ignored: false,
           cooldown_options: nil
         )
@@ -68,7 +66,6 @@ def initialize(
           @ignored_versions    = ignored_versions
           @security_advisories = security_advisories
           @raise_on_ignored    = raise_on_ignored
-          @goprivate           = goprivate
           @cooldown_options    = cooldown_options
           super(
             dependency: dependency,
@@ -122,9 +119,6 @@ def cooldown_enabled?
         sig { returns(T::Array[Dependabot::SecurityAdvisory]) }
         attr_reader :security_advisories
 
-        sig { returns(String) }
-        attr_reader :goprivate
-
         sig { returns(T.nilable(Dependabot::Package::ReleaseCooldownOptions)) }
         attr_reader :cooldown_options
 
@@ -133,8 +127,7 @@ def available_versions_details
           @available_versions_details ||= T.let(Package::PackageDetailsFetcher.new(
             dependency: dependency,
             dependency_files: dependency_files,
-            credentials: credentials,
-            goprivate: goprivate
+            credentials: credentials
           ).fetch_available_versions, T.nilable(T::Array[Dependabot::Package::PackageRelease]))
         end
 
@@ -197,13 +190,10 @@ def lazy_filter_cooldown_versions(releases, check_max: true)
         # rubocop:disable Metrics/AbcSize
         sig { params(release: Dependabot::Package::PackageRelease).returns(T::Boolean) }
         def in_cooldown_period?(release)
-          env = { "GOPRIVATE" => @goprivate }
-
           begin
             release_info = SharedHelpers.run_shell_command(
               "go list -m -json #{dependency.name}@#{release.details.[]('version_string')}",
-              fingerprint: "go list -m -json <dependency_name>",
-              env: env
+              fingerprint: "go list -m -json <dependency_name>"
             )
           rescue Dependabot::SharedHelpers::HelperSubprocessFailed => e
             Dependabot.logger.info("Error while fetching release date info: #{e.message}")

go_modules/spec/dependabot/go_modules/file_updater/go_mod_updater_spec.rb

@@ -5,16 +5,22 @@
 require "dependabot/dependency"
 require "dependabot/dependency_file"
 require "dependabot/go_modules/file_updater/go_mod_updater"
+require "dependabot/go_modules/file_parser"
 
 RSpec.describe Dependabot::GoModules::FileUpdater::GoModUpdater do
+  # Ensure GOPRIVATE is cleaned up after every test
+  after do
+    ENV.delete("GOPRIVATE")
+  end
+
   let(:updater) do
     described_class.new(
       dependencies: [dependency],
       dependency_files: dependency_files,
       credentials: credentials,
       repo_contents_path: repo_contents_path,
       directory: directory,
-      options: { tidy: tidy, vendor: false, goprivate: goprivate }
+      options: { tidy: tidy, vendor: false }
     )
   end
 
@@ -23,7 +29,6 @@
   let(:go_mod_content) { fixture("projects", project_name, "go.mod") }
   let(:tidy) { true }
   let(:directory) { "/" }
-  let(:goprivate) { "*" }
   let(:dependency_files) { [] }
 
   let(:credentials) { [] }
@@ -121,13 +126,13 @@
         end
 
         context "with an unrestricted goprivate" do
-          let(:goprivate) { "" }
+          before { ENV["GOPRIVATE"] = "" }
 
           it { is_expected.to include(%(rsc.io/quote v1.5.2\n)) }
         end
 
         context "with an org specific goprivate" do
-          let(:goprivate) { "rsc.io/*" }
+          before { ENV["GOPRIVATE"] = "rsc.io/*" }
 
           it { is_expected.to include(%(rsc.io/quote v1.5.2\n)) }
         end
@@ -409,8 +414,7 @@ module github.com/dependabot/vgotest
 
       before do
         allow(Open3).to receive(:capture3).and_call_original
-        allow(Open3).to receive(:capture3).with(anything,
-                                                "go get github.com/spf13/[email protected]").and_return(["", stderr,
+        allow(Open3).to receive(:capture3).with("go get github.com/spf13/[email protected]").and_return(["", stderr,
                                                                                                     exit_status])
       end
 
@@ -571,6 +575,8 @@ module github.com/dependabot/vgotest
     end
 
     context "when dealing with a invalid pseudo version" do
+      before { ENV["GOPRIVATE"] = "github.com/openshift/api" }
+      
       let(:project_name) { "invalid_pseudo_version" }
       let(:dependency_name) do
         "rsc.io/quote"
@@ -664,7 +670,9 @@ module github.com/dependabot/vgotest
       end
 
       context "with an unrestricted goprivate" do
-        let(:goprivate) { "" }
+        before do
+          ENV["GOPRIVATE"] = ""
+        end
 
         it "raises the correct error" do
           expect { updater.updated_go_sum_content }
@@ -673,7 +681,9 @@ module github.com/dependabot/vgotest
       end
 
       context "with an org specific goprivate" do
-        let(:goprivate) { "github.com/dependabot-fixtures/*" }
+        before do
+          ENV["GOPRIVATE"] = "github.com/dependabot-fixtures/*"
+        end
 
         it "raises the correct error" do
           expect { updater.updated_go_sum_content }

go_modules/spec/dependabot/go_modules/file_updater_spec.rb

@@ -145,7 +145,7 @@ module declares its path as: go.etcd.io/bbolt
       before do
         exit_status = double(success?: false)
         allow(Open3).to receive(:capture3).and_call_original
-        allow(Open3).to receive(:capture3).with(anything, "go get").and_return(["", stderr, exit_status])
+        allow(Open3).to receive(:capture3).with("go get").and_return(["", stderr, exit_status])
       end
 
       it "raises a helpful error" do
@@ -229,7 +229,7 @@ module declares its path as: go.etcd.io/bbolt
             credentials: anything,
             repo_contents_path: anything,
             directory: anything,
-            options: { tidy: false, vendor: false, goprivate: "*" }
+            options: { tidy: false, vendor: false }
           ).and_return(double)
 
         updater.updated_dependency_files