Skip to content

cargo: strip per-registry credential-provider from .cargo/config.toml #14356

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.

Sign up for GitHub

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

Closed
jeffwidman wants to merge 1 commit into from
+98 −2
Closed

cargo: strip per-registry credential-provider from .cargo/config.toml #14356

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
2 changes: 1 addition & 1 deletion cargo/lib/dependabot/cargo/file_updater/lockfile_updater.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -307,7 +307,7 @@ def write_temporary_dependency_files
return unless config

FileUtils.mkdir_p(File.dirname(T.must(config).name))
File.write(T.must(config).name, T.must(config).content)
File.write(T.must(config).name, Helpers.sanitize_cargo_config(T.must(T.must(config).content)))
Comment on lines 309 to +310
Copy link

Copilot AI Mar 3, 2026

Choose a reason for hiding this comment

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

This T.must(T.must(config).content) nesting makes the write path harder to read. Consider assigning config_file = T.must(config) and content = T.must(config_file.content) (or similar) before calling sanitize_cargo_config so the nil assertions are clearer.

See below for a potential fix:

          config_file = T.must(config)
          FileUtils.mkdir_p(File.dirname(config_file.name))
          config_content = T.must(config_file.content)
          sanitized_content = Helpers.sanitize_cargo_config(config_content)
          File.write(config_file.name, sanitized_content)

Copilot uses AI. Check for mistakes.
end

sig { void }
Expand Down
15 changes: 15 additions & 0 deletions cargo/lib/dependabot/cargo/helpers.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,21 @@ def self.bypass_cargo_credential_providers
# shell (along with the appropriate CARGO_REGISTRIES_{NAME}_TOKEN vars) for local development without the proxy.
ENV["CARGO_REGISTRY_GLOBAL_CREDENTIAL_PROVIDERS"] ||= ""
end

sig { params(config_content: String).returns(String) }
def self.sanitize_cargo_config(config_content)
# Remove per-registry `credential-provider` settings from .cargo/config.toml.
#
# Users may configure their repos with lines like:
# [registries.my-registry]
# credential-provider = "cargo:token"
#
# These per-registry settings override the global CARGO_REGISTRY_GLOBAL_CREDENTIAL_PROVIDERS env var,
# causing Cargo to look up tokens via CARGO_REGISTRIES_{NAME}_TOKEN env vars. Since the dependabot proxy
# handles authentication by intercepting HTTP requests, we need to strip these so Cargo makes plain
# requests that the proxy can decorate with credentials.
config_content.gsub(/^\s*credential-provider\s*=.*$/, "")
Copy link

Copilot AI Mar 3, 2026

Choose a reason for hiding this comment

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

The implementation strips any line starting with credential-provider, regardless of whether it’s in a [registries.*] section. Either tighten the sanitization to only affect per-registry config (as described in the comment) or update the comment/docs to reflect that this removes all credential-provider settings in the file.

Suggested change
config_content.gsub(/^\s*credential-provider\s*=.*$/, "")
current_section = nil
sanitized_lines = config_content.lines.each_with_object([]) do |line, kept|
if line =~ /^\s*\[(.+?)\]\s*$/
current_section = Regexp.last_match(1)
end
if current_section&.start_with?("registries.") && line =~ /^\s*credential-provider\s*=/
next
end
kept << line
end
sanitized_lines.join

Copilot uses AI. Check for mistakes.
end
end
end
end
2 changes: 1 addition & 1 deletion cargo/lib/dependabot/cargo/update_checker/version_resolver.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -218,7 +218,7 @@ def write_temporary_dependency_files(prepared: true)
return unless config

FileUtils.mkdir_p(File.dirname(T.must(config).name))
File.write(T.must(config).name, T.must(config).content)
File.write(T.must(config).name, Helpers.sanitize_cargo_config(T.must(T.must(config).content)))
Comment on lines 220 to +221
Copy link

Copilot AI Mar 3, 2026

Choose a reason for hiding this comment

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

This T.must(T.must(config).content) nesting makes the write path harder to read. Consider assigning config_file = T.must(config) and content = T.must(config_file.content) (or similar) before calling sanitize_cargo_config so the nil assertions are clearer.

See below for a potential fix:

          config_file = T.must(config)
          config_content = T.must(config_file.content)

          FileUtils.mkdir_p(File.dirname(config_file.name))
          File.write(config_file.name, Helpers.sanitize_cargo_config(config_content))

Copilot uses AI. Check for mistakes.
end

sig { void }
Expand Down
81 changes: 81 additions & 0 deletions cargo/spec/dependabot/cargo/helpers_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,4 +34,85 @@
end
end
end

describe ".sanitize_cargo_config" do
context "when config has no credential-provider lines" do
let(:config_content) do
<<~TOML
[registries.my-registry]
index = "sparse+https://example.com/index/"

[registries.another-registry]
index = "sparse+https://other.example.com/index/"
TOML
end

it "returns the content unchanged" do
result = described_class.sanitize_cargo_config(config_content)
expect(result).to eq(config_content)
end
end

context "when config has per-registry credential-provider lines" do
let(:config_content) do
<<~TOML
[registries.artifactory]
index = "sparse+https://example.com/api/cargo/cargo-local/index/"
credential-provider = "cargo:token"

[registries.artifactory-remote]
index = "sparse+https://example.com/api/cargo/cargo-crates-remote/index/"
credential-provider = "cargo:token"
TOML
end

it "strips the credential-provider lines" do
result = described_class.sanitize_cargo_config(config_content)
expect(result).not_to include("credential-provider")
expect(result).to include('index = "sparse+https://example.com/api/cargo/cargo-local/index/"')
expect(result).to include('index = "sparse+https://example.com/api/cargo/cargo-crates-remote/index/"')
end
end

context "when config has credential-provider with different spacing" do
let(:config_content) do
<<~TOML
[registries.my-registry]
index = "sparse+https://example.com/index/"
credential-provider = "cargo:token"
TOML
end

it "strips the credential-provider line regardless of whitespace" do
result = described_class.sanitize_cargo_config(config_content)
expect(result).not_to include("credential-provider")
expect(result).to include('index = "sparse+https://example.com/index/"')
end
end

context "when config has mixed registries with and without credential-provider" do
let(:config_content) do
<<~TOML
[registries.with-cred]
index = "sparse+https://example.com/index/"
credential-provider = "cargo:token"

[registries.without-cred]
index = "sparse+https://other.example.com/index/"

[source.crates-io]
replace-with = "with-cred"
TOML
end

it "strips only the credential-provider lines" do
result = described_class.sanitize_cargo_config(config_content)
expect(result).not_to include("credential-provider")
expect(result).to include('[registries.with-cred]')
expect(result).to include('[registries.without-cred]')
expect(result).to include('[source.crates-io]')
expect(result).to include('replace-with = "with-cred"')
end
end
end
end
Loading