Skip to content

Commit

Permalink
Stop exposing real account tokens in plaintext
Browse files Browse the repository at this point in the history
After I rotated the Gemfury/Hex.PM tokens, these unit tests started
failing, which made us realize we've been exposing the account tokens
in the clear.

Historically this was fine because:
1. These private registries were used solely for test accounts, it
   didn't matter if they were compromised.
1. Dependabot was a startup manned by a small team, so everyone knew
   these registries were publicly exposed and shouldn't be re-used for
   anything private.

However, 2锔忊儯 is no longer true... now that :dependabot: is part of
GitHub, the team has grown so there's a risk that someone could upload
something that should stay private to these orgs.

Unfortunately, neither Gemfury nor Hex.PM support limiting the scope of
a token to particular package(s). So while they're read-only, they still
expose everything on the account.

After discussing further, we decided that we no longer want to expose
these tokens, so flipping them to use Env Vars.

Furthermore, these tests exercise private registry auth code in
`dependabot-core`, but we don't use that in production at GitHub.
Instead, we use a [private credential
proxy](https://github.com/dependabot/dependabot-core#private-registry-credential-management).

Long term, we've batted around the idea of open sourcing that, in which
case we'd remove all the private registry auth code from
`dependabot-core`.

So this PR removes the plain text tokens (and I will also delete them
from the registry side to ensure they aren't usable). And wires up env
var support so that if we _do_ need to run these tests, we can still do
that. But it does _not_ go to the trouble of wiring up setting the env
var using GitHub actions because there is a reasonable chance this code
will all be deleted in the not-too-distant future. And until then, it's
not even used by GitHub production, only by folks running
`dependabot-core` standalone. So the impact if this does break is fairly
small and in that case we'd happily accept a contribution to fix things,
but we're unlikely to invest the engineering efforts ourselves to fix
it.
  • Loading branch information
jeffwidman committed Jul 7, 2023
1 parent df6e81c commit b522d30
Show file tree
Hide file tree
Showing 3 changed files with 15 additions and 5 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -455,6 +455,7 @@
end

context "with good credentials" do
let(:gemfury_deploy_token) { ENV.fetch("GEMFURY_DEPLOY_TOKEN", nil) }
let(:credentials) do
[{
"type" => "git_source",
Expand All @@ -464,12 +465,13 @@
}, {
"type" => "composer_repository",
"registry" => "php.fury.io",
"username" => "yFu9PBmw1HxNjFB818TW", # Throwaway account
"username" => gemfury_deploy_token,
"password" => ""
}]
end

it "has details of the updated item" do
skip("skipped because env var GEMFURY_DEPLOY_TOKEN is not set") if gemfury_deploy_token.nil?
expect(updated_lockfile_content).to include("\"version\":\"2.2.0\"")
end
end
Expand Down
8 changes: 6 additions & 2 deletions composer/spec/dependabot/composer/update_checker_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -330,6 +330,7 @@
end

context "with good credentials" do
let(:gemfury_deploy_token) { ENV.fetch("GEMFURY_DEPLOY_TOKEN", nil) }
let(:credentials) do
[{
"type" => "git_source",
Expand All @@ -339,12 +340,15 @@
}, {
"type" => "composer_repository",
"registry" => "php.fury.io",
"username" => "yFu9PBmw1HxNjFB818TW", # Throwaway account
"username" => gemfury_deploy_token,
"password" => ""
}]
end

it { is_expected.to be >= Gem::Version.new("2.2.0") }
it "returns the expected version" do
skip("skipped because env var GEMFURY_DEPLOY_TOKEN is not set") if gemfury_deploy_token.nil?
is_expected.to be >= Gem::Version.new("2.2.0")
end
end

context "with bad credentials" do
Expand Down
8 changes: 6 additions & 2 deletions hex/spec/dependabot/hex/update_checker_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -275,6 +275,7 @@
end

context "with good credentials" do
let(:hex_pm_org_token) { ENV.fetch("HEX_PM_ORGANIZATION_TOKEN", nil) }
let(:credentials) do
[{
"type" => "git_source",
Expand All @@ -284,11 +285,14 @@
}, {
"type" => "hex_organization",
"organization" => "dependabot",
"token" => "b6294cd1e1cf158e9f65ea6b02a9a1ec"
"token" => hex_pm_org_token
}]
end

it { is_expected.to eq(Gem::Version.new("1.1.0")) }
it "returns the expected version" do
skip("skipped because env var HEX_PM_ORGANIZATION_TOKEN is not set") if hex_pm_org_token.nil?
is_expected.to eq(Gem::Version.new("1.1.0"))
end
end

context "with bad credentials" do
Expand Down

0 comments on commit b522d30

Please sign in to comment.