Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Enable CorePack Installing Package Managers from Private Registries #11077

Merged
merged 26 commits into from
Dec 11, 2024
Merged
Show file tree
Hide file tree
Changes from 19 commits
Commits
Show all changes
26 commits
Select commit Hold shift + click to select a range
30d8dd2
add registry for corepack install package managers
kbukum1 Dec 7, 2024
07c385c
add registry helper into imports
kbukum1 Dec 7, 2024
8da6ed9
remove lockfile to find out registry for corepack install
kbukum1 Dec 7, 2024
b4031df
fix parameters in testing
kbukum1 Dec 9, 2024
e78b1ff
fix specs
kbukum1 Dec 9, 2024
05ed720
fixed spec
kbukum1 Dec 9, 2024
5c596bf
Merge branch 'main' into kamil/set_corepack_registry_for_installing_n…
kbukum1 Dec 9, 2024
234c4db
add dependabot yml credentials to check npm registry
kbukum1 Dec 9, 2024
9874d68
Merge branch 'main' into kamil/set_corepack_registry_for_installing_n…
kbukum1 Dec 9, 2024
e870538
move registry_helper into package manager helper
kbukum1 Dec 10, 2024
8b8b130
remove registry_helper
kbukum1 Dec 10, 2024
2e90495
fix reverse proxy check
kbukum1 Dec 10, 2024
56f8c7c
fix yarnrc issue
kbukum1 Dec 10, 2024
ec013e3
improve the parsing
kbukum1 Dec 10, 2024
def21df
moved registry helper as seperate class
kbukum1 Dec 10, 2024
0789da6
Merge branch 'main' into kamil/set_corepack_registry_for_installing_n…
kbukum1 Dec 10, 2024
7cd79a6
fix linting issue
kbukum1 Dec 10, 2024
16915d5
fix rubocop checks
kbukum1 Dec 10, 2024
6a0c2ec
Merge branch 'main' into kamil/set_corepack_registry_for_installing_n…
kbukum1 Dec 10, 2024
c97ef73
activate installed package manager
kbukum1 Dec 10, 2024
e745cde
add logging when activating the package manager
kbukum1 Dec 10, 2024
2878c4d
fix spec
kbukum1 Dec 10, 2024
02b9f2f
Merge branch 'main' into kamil/set_corepack_registry_for_installing_n…
kbukum1 Dec 10, 2024
1a3805e
fix logging
kbukum1 Dec 10, 2024
d18afbc
add feature for corepack registry
kbukum1 Dec 11, 2024
679cea2
add feature flag for enable registry for corepack
kbukum1 Dec 11, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 14 additions & 1 deletion npm_and_yarn/lib/dependabot/npm_and_yarn/file_fetcher.rb
Original file line number Diff line number Diff line change
Expand Up @@ -207,7 +207,9 @@ def package_manager_helper
@package_manager_helper ||= T.let(
PackageManagerHelper.new(
parsed_package_json,
lockfiles: lockfiles
lockfiles,
registry_config_files,
credentials
), T.nilable(PackageManagerHelper)
)
end
Expand All @@ -221,6 +223,17 @@ def lockfiles
}
end

# Returns the .npmrc, and .yarnrc files for the repository.
# @return [Hash{Symbol => Dependabot::DependencyFile}]
sig { returns(T::Hash[Symbol, T.nilable(Dependabot::DependencyFile)]) }
def registry_config_files
{
npmrc: npmrc,
yarnrc: yarnrc,
yarnrc_yml: yarnrc_yml
}
end

sig { returns(DependencyFile) }
def package_json
@package_json ||= T.let(fetch_file_from_host(MANIFEST_FILENAME), T.nilable(DependencyFile))
Expand Down
34 changes: 33 additions & 1 deletion npm_and_yarn/lib/dependabot/npm_and_yarn/file_parser.rb
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,9 @@ def package_manager_helper
@package_manager_helper ||= T.let(
PackageManagerHelper.new(
parsed_package_json,
lockfiles: lockfiles
lockfiles,
registry_config_files,
credentials
), T.nilable(PackageManagerHelper)
)
end
Expand All @@ -112,6 +114,15 @@ def lockfiles
}
end

sig { returns(T::Hash[Symbol, T.nilable(Dependabot::DependencyFile)]) }
def registry_config_files
{
npmrc: npmrc,
yarnrc: yarnrc,
yarnrc_yml: yarnrc_yml
}
end

sig { returns(T.untyped) }
def parsed_package_json
JSON.parse(T.must(package_json.content))
Expand Down Expand Up @@ -156,6 +167,27 @@ def pnpm_lock
end, T.nilable(Dependabot::DependencyFile))
end

sig { returns(T.nilable(Dependabot::DependencyFile)) }
def npmrc
@npmrc ||= T.let(dependency_files.find do |f|
f.name == NpmPackageManager::RC_FILENAME
end, T.nilable(Dependabot::DependencyFile))
end

sig { returns(T.nilable(Dependabot::DependencyFile)) }
def yarnrc
@yarnrc ||= T.let(dependency_files.find do |f|
f.name == YarnPackageManager::RC_FILENAME
end, T.nilable(Dependabot::DependencyFile))
end

sig { returns(T.nilable(DependencyFile)) }
def yarnrc_yml
@yarnrc_yml ||= T.let(dependency_files.find do |f|
f.name == YarnPackageManager::RC_YML_FILENAME
end, T.nilable(Dependabot::DependencyFile))
end

sig { returns(Dependabot::FileParsers::Base::DependencySet) }
def manifest_dependencies
dependency_set = DependencySet.new
Expand Down
27 changes: 21 additions & 6 deletions npm_and_yarn/lib/dependabot/npm_and_yarn/helpers.rb
Original file line number Diff line number Diff line change
Expand Up @@ -388,13 +388,20 @@ def self.run_single_yarn_command(command, fingerprint: nil)
end

# Install the package manager for specified version by using corepack
sig { params(name: String, version: String).returns(String) }
def self.install(name, version)
sig do
params(
name: String,
version: String,
env: T.nilable(T::Hash[String, String])
)
.returns(String)
end
def self.install(name, version, env: {})
Dependabot.logger.info("Installing \"#{name}@#{version}\"")

begin
# Try to install the specified version
output = package_manager_install(name, version)
output = package_manager_install(name, version, env: env)

# Confirm success based on the output
if output.match?(/Adding #{name}@.* to the cache/)
Expand Down Expand Up @@ -428,11 +435,19 @@ def self.fallback_to_local_version(name)
end

# Install the package manager for specified version by using corepack
sig { params(name: String, version: String).returns(String) }
def self.package_manager_install(name, version)
sig do
params(
name: String,
version: String,
env: T.nilable(T::Hash[String, String])
)
.returns(String)
end
def self.package_manager_install(name, version, env: {})
Dependabot::SharedHelpers.run_shell_command(
"corepack install #{name}@#{version} --global --cache-only",
fingerprint: "corepack install <name>@<version> --global --cache-only"
fingerprint: "corepack install <name>@<version> --global --cache-only",
env: env
).strip
end

Expand Down
24 changes: 17 additions & 7 deletions npm_and_yarn/lib/dependabot/npm_and_yarn/package_manager.rb
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
require "dependabot/ecosystem"
require "dependabot/npm_and_yarn/requirement"
require "dependabot/npm_and_yarn/version_selector"
require "dependabot/npm_and_yarn/registry_helper"

module Dependabot
module NpmAndYarn
Expand Down Expand Up @@ -311,17 +312,24 @@ class PackageManagerHelper
sig do
params(
package_json: T.nilable(T::Hash[String, T.untyped]),
lockfiles: T::Hash[Symbol, T.nilable(Dependabot::DependencyFile)]
lockfiles: T::Hash[Symbol, T.nilable(Dependabot::DependencyFile)],
registry_config_files: T::Hash[Symbol, T.nilable(Dependabot::DependencyFile)],
credentials: T.nilable(T::Array[Dependabot::Credential])
).void
end
def initialize(package_json, lockfiles:)
def initialize(package_json, lockfiles, registry_config_files, credentials)
@package_json = package_json
@lockfiles = lockfiles
@registry_helper = T.let(
RegistryHelper.new(registry_config_files, credentials),
Dependabot::NpmAndYarn::RegistryHelper
)
@package_manager_detector = T.let(PackageManagerDetector.new(lockfiles, package_json), PackageManagerDetector)
@manifest_package_manager = T.let(package_json&.fetch(MANIFEST_PACKAGE_MANAGER_KEY, nil), T.nilable(String))
@engines = T.let(package_json&.fetch(MANIFEST_ENGINES_KEY, nil), T.nilable(T::Hash[String, T.untyped]))

@installed_versions = T.let({}, T::Hash[String, String])
@registries = T.let({}, T::Hash[String, String])

@language = T.let(nil, T.nilable(Ecosystem::VersionManager))
@language_requirement = T.let(nil, T.nilable(Requirement))
Expand Down Expand Up @@ -379,8 +387,8 @@ def find_engine_constraints_as_requirement(name)
end

# rubocop:disable Metrics/CyclomaticComplexity
# rubocop:disable Metrics/PerceivedComplexity
# rubocop:disable Metrics/AbcSize
# rubocop:disable Metrics/PerceivedComplexity
sig { params(name: String).returns(T.nilable(T.any(Integer, String))) }
def setup(name)
# we prioritize version mentioned in "packageManager" instead of "engines"
Expand Down Expand Up @@ -438,6 +446,9 @@ def setup(name)
end
version
end
# rubocop:enable Metrics/CyclomaticComplexity
# rubocop:enable Metrics/AbcSize
# rubocop:enable Metrics/PerceivedComplexity

sig { params(name: T.nilable(String)).returns(Ecosystem::VersionManager) }
def package_manager_by_name(name)
Expand Down Expand Up @@ -468,9 +479,6 @@ def package_manager_by_name(name)
raise
end

# rubocop:enable Metrics/CyclomaticComplexity
# rubocop:enable Metrics/PerceivedComplexity
# rubocop:enable Metrics/AbcSize
# Retrieve the installed version of the package manager by executing
# the "corepack <name> -v" command and using the output.
# If the output does not match the expected version format (PACKAGE_MANAGER_VERSION_REGEX),
Expand Down Expand Up @@ -510,7 +518,9 @@ def raise_if_unsupported!(name, version)
sig { params(name: String, version: T.nilable(String)).void }
def install(name, version)
if Dependabot::Experiments.enabled?(:enable_corepack_for_npm_and_yarn)
return Helpers.install(name, version.to_s)
env = @registry_helper.find_corepack_env_variables
# Use the Helpers.install method to install the package manager
return Helpers.install(name, version.to_s, env: env)
end

Dependabot.logger.info("Installing \"#{name}@#{version}\"")
Expand Down
188 changes: 188 additions & 0 deletions npm_and_yarn/lib/dependabot/npm_and_yarn/registry_helper.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,188 @@
# typed: strict
# frozen_string_literal: true

require "yaml"
require "dependabot/dependency_file"
require "sorbet-runtime"

module Dependabot
module NpmAndYarn
class RegistryHelper
Copy link
Member

@jonabc jonabc Dec 10, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it's not entirely clear why this is its own class when it's calls are already fully encapsulated within PackageManagerHelper

If this is meant to be reused in other places, WDYT about providing a RegistryHelper instance to PackageManagerHelper.new instead of creating it in the constructor?

Otherwise if it's not reused elsewhere could this be collapsed into PackageManagerHelper? This could always be broken out into a separate class later if/when needed for reuse or portability.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can be moved into PackageManagerHelper

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is just moved.

extend T::Sig

# Keys for configurations
REGISTRY_KEY = "registry"
AUTH_KEY = "authToken"

# Yarn-specific keys
NPM_AUTH_TOKEN_KEY_FOR_YARN = "npmAuthToken"
NPM_SCOPE_KEY_FOR_YARN = "npmScopes"
NPM_REGISTER_KEY_FOR_YARN = "npmRegistryServer"

# Environment variable keys
COREPACK_NPM_REGISTRY_ENV = "COREPACK_NPM_REGISTRY"
COREPACK_NPM_TOKEN_ENV = "COREPACK_NPM_TOKEN"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suspect setting COREPACK_NPM_TOKEN_ENV is not necessary as the Proxy container will inject the needed credentials. But there may be folks out there still using the Ruby directly and handing it tokens. So fine to leave for now.


sig do
params(
registry_config_files: T::Hash[Symbol, T.nilable(Dependabot::DependencyFile)],
credentials: T.nilable(T::Array[Dependabot::Credential])
).void
end
def initialize(registry_config_files, credentials)
@registry_config_files = T.let(registry_config_files, T::Hash[Symbol, T.nilable(Dependabot::DependencyFile)])
@credentials = T.let(credentials, T.nilable(T::Array[Dependabot::Credential]))
end

sig { returns(T::Hash[String, String]) }
def find_corepack_env_variables
registry_info = find_registry_and_token

env_variables = {}
env_variables[COREPACK_NPM_REGISTRY_ENV] = registry_info[:registry] if registry_info[:registry]
env_variables[COREPACK_NPM_TOKEN_ENV] = registry_info[:auth_token] if registry_info[:auth_token]

env_variables
end

private

sig { returns(T::Hash[Symbol, T.nilable(String)]) }
def find_registry_and_token
# Step 1: Check dependabot.yml configuration
dependabot_config = config_npm_registry_and_token
return dependabot_config if dependabot_config[:registry]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not every NPM registry is also a reverse-proxy style registry, for instance npm.pkg.github.com which is usually scoped to a specific package. So if a customer had a private scoped package on a registry that wasn't a reverse-proxy, and they used dependabot.yml to provide credentials, corepack would try to download from there which would fail.

It might be better to look for replaces-base only which indicates it's a reverse-proxy style registry.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I’ve updated the code to ensure that the config_npm_registry_and_token method only returns the object if the registry is reverse-proxy-enabled (i.e., when replaces-base is true). Could you please confirm if this aligns with what you meant in your comment?


# Step 2: Check .npmrc
npmrc_config = @registry_config_files[:npmrc]
npmrc_result = parse_registry_from_npmrc_yarnrc(npmrc_config, "=", "npm")

return npmrc_result if npmrc_result[:registry]

# Step 3: Check .yarnrc
yarnrc_config = @registry_config_files[:yarnrc]
yarnrc_result = parse_registry_from_npmrc_yarnrc(yarnrc_config, " ", "npm")
return yarnrc_result if yarnrc_result[:registry]

# Step 4: Check yarnrc.yml
yarnrc_yml_config = @registry_config_files[:yarnrc_yml]
yarnrc_yml_result = parse_npm_from_yarnrc_yml(yarnrc_yml_config)
return yarnrc_yml_result if yarnrc_yml_result[:registry]

# Default values if no registry is found
{}
end

sig { returns(T::Hash[Symbol, T.nilable(String)]) }
def config_npm_registry_and_token
registries = {}

return registries unless @credentials&.any?

@credentials.each do |cred|
next unless cred["type"] == "npm_registry" # Skip if not an npm registry
next unless cred["replaces-base"] # Skip if not a reverse-proxy registry

# Set the registry if it's not already set
registries[:registry] ||= cred["registry"]

# Set the token if it's not already set
registries[:auth_token] ||= cred["token"]
end
registries
end

# Find registry and token in .npmrc or .yarnrc file
sig do
params(
file: T.nilable(Dependabot::DependencyFile),
separator: String
).returns(T::Hash[Symbol, T.nilable(String)])
end
def parse_npm_from_npm_or_yarn_rc(file, separator = "=")
parse_registry_from_npmrc_yarnrc(file, separator, NpmPackageManager::NAME)
end

# Find registry and token in .npmrc or .yarnrc file
sig do
params(
file: T.nilable(Dependabot::DependencyFile),
separator: String,
scope: T.nilable(String)
).returns(T::Hash[Symbol, T.nilable(String)])
end
def parse_registry_from_npmrc_yarnrc(file, separator = "=", scope = nil)
content = file&.content
return { registry: nil, auth_token: nil } unless content

global_registry = T.let(nil, T.nilable(String))
scoped_registry = T.let(nil, T.nilable(String))
auth_token = T.let(nil, T.nilable(String))

content.split("\n").each do |line|
# Split using the provided separator
key, value = line.strip.split(separator, 2)
next unless key && value

# Remove surrounding quotes from keys and values
cleaned_key = key.strip.gsub(/\A["']|["']\z/, "")
cleaned_value = value.strip.gsub(/\A["']|["']\z/, "")

case cleaned_key
when "registry"
# Case 1: Found a global registry
global_registry = cleaned_value
when "_authToken"
# Case 2: Found an auth token
auth_token = cleaned_value
else
# Handle scoped registry if a scope is provided
scoped_registry = cleaned_value if scope && cleaned_key == "@#{scope}:registry"
end
end

# Determine the registry to return (global first, fallback to scoped)
registry = global_registry || scoped_registry

{ registry: registry, auth_token: auth_token }
end

# rubocop:disable Metrics/PerceivedComplexity
sig { params(file: T.nilable(Dependabot::DependencyFile)).returns(T::Hash[Symbol, T.nilable(String)]) }
def parse_npm_from_yarnrc_yml(file)
content = file&.content
return { registry: nil, auth_token: nil } unless content

result = {}
yaml_data = safe_load_yaml(content)

# Step 1: Extract global registry and auth token
result[:registry] = yaml_data[NPM_REGISTER_KEY_FOR_YARN] if yaml_data.key?(NPM_REGISTER_KEY_FOR_YARN)
result[:auth_token] = yaml_data[NPM_AUTH_TOKEN_KEY_FOR_YARN] if yaml_data.key?(NPM_AUTH_TOKEN_KEY_FOR_YARN)

# Step 2: Fallback to any scoped registry and auth token if global is missing
if result[:registry].nil? && yaml_data.key?(NPM_SCOPE_KEY_FOR_YARN)
yaml_data[NPM_SCOPE_KEY_FOR_YARN].each do |_current_scope, config|
next unless config.is_a?(Hash)

result[:registry] ||= config[NPM_REGISTER_KEY_FOR_YARN]
result[:auth_token] ||= config[NPM_AUTH_TOKEN_KEY_FOR_YARN]
end
end

result
end
# rubocop:enable Metrics/PerceivedComplexity

# Safely loads the YAML content and logs any parsing errors
sig { params(content: String).returns(T::Hash[String, T.untyped]) }
def safe_load_yaml(content)
YAML.safe_load(content, permitted_classes: [Symbol, String]) || {}
rescue Psych::SyntaxError => e
# Log the error instead of raising it
Dependabot.logger.error("YAML parsing error: #{e.message}")
{}
end
end
end
end
Loading
Loading