Skip registering credentials for anonymous registries for egress allowlist

internal/handlers/anonymous_credentials_test.go

@@ -0,0 +1,218 @@
+package handlers
+
+import (
+	"net/http/httptest"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+
+	"github.com/dependabot/proxy/internal/config"
+)
+
+// Anonymous registries are declared without any credential so that their host
+// lands in the per-job egress allowlist. The credential must therefore still be
+// present in the job config, which previously caused each handler to attach an
+// empty Authorization header ("Bearer " with no value, or Basic base64(":")).
+// Nexus and Artifactory reject that outright rather than serving the request
+// anonymously, so the request must instead be forwarded untouched.
+
+// TestCredentialIsAnonymous pins which credential shapes count as anonymous.
+// The two empty-secret shapes arrive by different routes: an org-level registry
+// is serialized with no token key at all, while a repo-level one is serialized
+// by dependabot-api as token ":" when neither username nor password is set.
+func TestCredentialIsAnonymous(t *testing.T) {
+	testCases := []struct {
+		name     string
+		cred     config.Credential
+		expected bool
+	}{
+		{
+			name:     "url only (org-level anonymous)",
+			cred:     config.Credential{"type": "npm_registry", "url": "https://nexus.example.net"},
+			expected: true,
+		},
+		{
+			name:     "colon token (repo-level anonymous)",
+			cred:     config.Credential{"type": "npm_registry", "url": "https://nexus.example.net", "token": ":"},
+			expected: true,
+		},
+		{
+			name:     "empty token",
+			cred:     config.Credential{"type": "npm_registry", "url": "https://nexus.example.net", "token": ""},
+			expected: true,
+		},
+		{
+			name:     "token",
+			cred:     config.Credential{"type": "npm_registry", "url": "https://nexus.example.net", "token": "secret"},
+			expected: false,
+		},
+		{
+			name:     "username and password",
+			cred:     config.Credential{"type": "npm_registry", "url": "https://nexus.example.net", "username": "u", "password": "p"},
+			expected: false,
+		},
+		{
+			// A username with no password is a misconfiguration, not an anonymous
+			// registry. Treating it as anonymous would silently drop an intended
+			// credential, so it stays registered and keeps its existing behaviour.
+			name:     "username only is not anonymous",
+			cred:     config.Credential{"type": "npm_registry", "url": "https://nexus.example.net", "username": "u"},
+			expected: false,
+		},
+		{
+			name:     "auth-key",
+			cred:     config.Credential{"type": "hex_repository", "url": "https://hex.example.net", "auth-key": "k"},
+			expected: false,
+		},
+		{
+			name:     "key",
+			cred:     config.Credential{"type": "hex_organization", "organization": "o", "key": "k"},
+			expected: false,
+		},
+	}
+
+	for _, tc := range testCases {
+		t.Run(tc.name, func(t *testing.T) {
+			assert.Equal(t, tc.expected, credentialIsAnonymous(tc.cred))
+		})
+	}
+}
+
+// TestAnonymousCredentialsAreNotAuthenticated covers every handler that did not
+// already skip credential-free entries. Each asserts no Authorization header is
+// added at all -- an empty "Bearer " or Basic base64(":") would fail this.
+func TestAnonymousCredentialsAreNotAuthenticated(t *testing.T) {
+	testCases := []struct {
+		name           string
+		cred           config.Credential
+		target         string
+		handlerFactory func(config.Credentials) oidcHandler
+	}{
+		{
+			name:   "npm_registry",
+			cred:   config.Credential{"type": "npm_registry", "registry": "https://nexus.example.net/repository/npm-all"},
+			target: "https://nexus.example.net/repository/npm-all/some-package",
+			handlerFactory: func(creds config.Credentials) oidcHandler {
+				return NewNPMRegistryHandler(creds, testOIDCClient)
+			},
+		},
+		{
+			name:   "composer_repository",
+			cred:   config.Credential{"type": "composer_repository", "registry": "https://nexus.example.net/repository/composer"},
+			target: "https://nexus.example.net/repository/composer/packages.json",
+			handlerFactory: func(creds config.Credentials) oidcHandler {
+				return NewComposerHandler(creds, testOIDCClient)
+			},
+		},
+		{
+			name:   "helm_registry",
+			cred:   config.Credential{"type": "helm_registry", "registry": "https://nexus.example.net/repository/helm"},
+			target: "https://nexus.example.net/repository/helm/index.yaml",
+			handlerFactory: func(creds config.Credentials) oidcHandler {
+				return NewHelmRegistryHandler(creds, testOIDCClient)
+			},
+		},
+		{
+			name:   "python_index",
+			cred:   config.Credential{"type": "python_index", "index-url": "https://nexus.example.net/repository/pypi/simple"},
+			target: "https://nexus.example.net/repository/pypi/simple/requests/",
+			handlerFactory: func(creds config.Credentials) oidcHandler {
+				return NewPythonIndexHandler(creds, testOIDCClient)
+			},
+		},
+		{
+			name:   "rubygems_server",
+			cred:   config.Credential{"type": "rubygems_server", "url": "https://nexus.example.net/repository/gems"},
+			target: "https://nexus.example.net/repository/gems/specs.4.8.gz",
+			handlerFactory: func(creds config.Credentials) oidcHandler {
+				return NewRubyGemsServerHandler(creds, testOIDCClient)
+			},
+		},
+		{
+			name:   "docker_registry",
+			cred:   config.Credential{"type": "docker_registry", "registry": "nexus.example.net"},
+			target: "https://nexus.example.net/v2/library/alpine/manifests/latest",
+			handlerFactory: func(creds config.Credentials) oidcHandler {
+				return NewDockerRegistryHandler(creds, testOIDCClient, nil)
+			},
+		},
+		{
+			name:   "nuget_feed",
+			cred:   config.Credential{"type": "nuget_feed", "url": "https://nexus.example.net/repository/nuget/index.json"},
+			target: "https://nexus.example.net/repository/nuget/index.json",
+			handlerFactory: func(creds config.Credentials) oidcHandler {
+				return NewNugetFeedHandler(creds, testOIDCClient)
+			},
+		},
+		{
+			name:   "cargo_registry",
+			cred:   config.Credential{"type": "cargo_registry", "url": "https://nexus.example.net/repository/cargo"},
+			target: "https://nexus.example.net/repository/cargo/api/v1/crates/serde",
+			handlerFactory: func(creds config.Credentials) oidcHandler {
+				return NewCargoRegistryHandler(creds, testOIDCClient)
+			},
+		},
+		{
+			name:   "terraform_registry",
+			cred:   config.Credential{"type": "terraform_registry", "url": "https://nexus.example.net/terraform"},
+			target: "https://nexus.example.net/terraform/v1/modules/foo/bar/aws/versions",
+			handlerFactory: func(creds config.Credentials) oidcHandler {
+				return NewTerraformRegistryHandler(creds, testOIDCClient)
+			},
+		},
+		{
+			name:   "pub_repository",
+			cred:   config.Credential{"type": "pub_repository", "url": "https://nexus.example.net/pub"},
+			target: "https://nexus.example.net/pub/api/packages/http",
+			handlerFactory: func(creds config.Credentials) oidcHandler {
+				return NewPubRepositoryHandler(creds, testOIDCClient)
+			},
+		},
+	}
+
+	for _, tc := range testCases {
+		t.Run(tc.name, func(t *testing.T) {
+			handler := tc.handlerFactory(config.Credentials{tc.cred})
+			req := httptest.NewRequestWithContext(t.Context(), "GET", tc.target, nil)
+			req = handleRequestAndClose(handler, req, nil)
+			assertUnauthenticated(t, req, "anonymous "+tc.name+" request")
+		})
+	}
+
+	// The repo-level anonymous shape arrives as token ":" rather than as a
+	// missing key. Several handlers guard only on an empty token, so this shape
+	// reaches the request path and becomes Basic base64(":"), "Bearer :", or a
+	// raw "Authorization: :" depending on the handler.
+	t.Run("colon token", func(t *testing.T) {
+		for _, tc := range testCases {
+			t.Run(tc.name, func(t *testing.T) {
+				cred := config.Credential{"token": ":"}
+				for k, v := range tc.cred {
+					cred[k] = v
+				}
+
+				handler := tc.handlerFactory(config.Credentials{cred})
+				req := httptest.NewRequestWithContext(t.Context(), "GET", tc.target, nil)
+				req = handleRequestAndClose(handler, req, nil)
+				assertUnauthenticated(t, req, "colon-token "+tc.name+" request")
+			})
+		}
+	})
+}
+
+// TestAnonymousCredentialsStillAuthenticateWhenSecretPresent guards against the
+// anonymous skip being too eager: the same handlers must keep authenticating a
+// credential that does carry a secret.
+func TestAnonymousCredentialsStillAuthenticateWhenSecretPresent(t *testing.T) {
+	handler := NewNPMRegistryHandler(config.Credentials{
+		config.Credential{
+			"type":     "npm_registry",
+			"registry": "https://nexus.example.net/repository/npm-all",
+			"token":    "s3cret",
+		},
+	}, testOIDCClient)
+
+	req := httptest.NewRequestWithContext(t.Context(), "GET", "https://nexus.example.net/repository/npm-all/some-package", nil)
+	req = handleRequestAndClose(handler, req, nil)
+	assertHasTokenAuth(t, req, "Bearer", "s3cret", "credentialed npm request")
+}

internal/handlers/cargo_registry.go

@@ -103,6 +103,12 @@ func NewCargoRegistryHandler(credentials config.Credentials, client *http.Client
 			continue
 		}
 
+		// A ":" token passes the guard above but carries no credentials, and
+		// SetRawAuthorization would send it verbatim as "Authorization: :".
+		if credentialIsAnonymous(credential) {
+			continue
+		}
+
 		handler.credentials = append(handler.credentials, cargoCred)
 	}
 	return &handler

internal/handlers/composer.go

@@ -45,6 +45,12 @@ func NewComposerHandler(creds config.Credentials, client *http.Client) *Composer
 			continue
 		}
 
+		// An anonymous registry is allowlisted by its url/registry field but must
+		// not have an empty Authorization header attached to its requests.
+		if credentialIsAnonymous(cred) {
+			continue
+		}
+
 		composerCred := composerCredentials{
 			registry: registry,
 			url:      url,

internal/handlers/credentials.go

@@ -4,6 +4,8 @@ import (
 	"encoding/base64"
 	"net/http"
 	"strings"
+
+	"github.com/dependabot/proxy/internal/config"
 )
 
 const authorizationPlaceholder = "******"
@@ -37,3 +39,42 @@ func hasUsableAuthorization(req *http.Request) bool {
 func proxyOnlyCredentialRequestAllowed(req *http.Request) bool {
 	return req.URL.Scheme == "https" && (req.URL.Port() == "" || req.URL.Port() == "443")
 }
+
+// credentialSecretKeys are the credential fields that can carry secret material.
+// "username" is included because a username-only credential still represents an
+// intent to authenticate, and must not be mistaken for an anonymous one.
+var credentialSecretKeys = []string{
+	"token",
+	"password",
+	"auth-key",
+	"key",
+	"username",
+}
+
+// credentialIsAnonymous reports whether a credential names a registry but
+// carries nothing to authenticate with.
+//
+// An anonymous registry must still appear in the job's credentials, because the
+// egress allowlist derives its per-job hosts from exactly that list (see
+// credentialHosts in egress_dynamic_hosts.go). Its presence would otherwise
+// cause a handler to attach an empty Authorization header -- "Bearer " with no
+// value, or Basic base64(":") -- which Nexus and Artifactory reject outright
+// instead of falling back to anonymous access. Skipping registration lets the
+// request through unauthenticated, which is what such a registry expects, while
+// leaving the host allowlisted.
+//
+// A token of ":" is treated as absent: it is what dependabot-api emits for a
+// registry configured with neither a username nor a password, and it decodes to
+// an empty username and password.
+//
+// Callers must apply this only after OIDC registration has been attempted, so
+// that a credential which mints its token at request time is never mistaken for
+// an anonymous one.
+func credentialIsAnonymous(cred config.Credential) bool {
+	for _, key := range credentialSecretKeys {
+		if value := cred.GetString(key); value != "" && value != ":" {
+			return false
+		}
+	}
+	return true
+}

internal/handlers/docker_registry.go

@@ -76,6 +76,12 @@ func NewDockerRegistryHandler(creds config.Credentials, client *http.Client, get
 		if proxyOnly && cred.GetString("username") == "" && cred.GetString("password") == "" {
 			continue
 		}
+		// An anonymous registry is allowlisted by its registry field but must not
+		// have an empty Authorization header attached to its requests. ECR
+		// credentials carry an AWS key pair, so they are never anonymous here.
+		if credentialIsAnonymous(cred) {
+			continue
+		}
 
 		parsedRegistry, err := helpers.ParseURLLax(registryLocation)
 		if err != nil || parsedRegistry.Host == "" {

internal/handlers/egress_allowlist_test.go

@@ -1447,3 +1447,79 @@ func TestEgressAllowlist_ChangelogHostsThirdWaveAllowed(t *testing.T) {
 		}
 	}
 }
+
+// TestEgressAllowlist_AnonymousCredentialAllowlistsHost pins the behaviour the
+// org-level Anonymous registry option depends on: a credential carrying only a
+// registry location, with no secret of any kind, must still contribute its host
+// to the per-job allowlist. This is why an anonymous registry has to appear in
+// the job credentials at all, and therefore why the handlers must skip
+// authenticating it rather than the config omitting it.
+func TestEgressAllowlist_AnonymousCredentialAllowlistsHost(t *testing.T) {
+	for _, tc := range []struct {
+		name string
+		cred config.Credential
+		host string
+	}{
+		{
+			name: "npm_registry via registry",
+			cred: config.Credential{"type": "npm_registry", "registry": "nexus.example.net/repository/npm-all"},
+			host: "nexus.example.net",
+		},
+		{
+			name: "docker_registry via registry",
+			cred: config.Credential{"type": "docker_registry", "registry": "docker.example.net"},
+			host: "docker.example.net",
+		},
+		{
+			name: "python_index via index-url",
+			cred: config.Credential{"type": "python_index", "index-url": "https://pypi.example.net/simple"},
+			host: "pypi.example.net",
+		},
+		{
+			name: "maven_repository via url",
+			cred: config.Credential{"type": "maven_repository", "url": "https://maven.example.net/releases"},
+			host: "maven.example.net",
+		},
+		{
+			name: "nuget_feed via url",
+			cred: config.Credential{"type": "nuget_feed", "url": "https://nuget.example.net/v3/index.json"},
+			host: "nuget.example.net",
+		},
+		{
+			name: "colon token is still anonymous",
+			cred: config.Credential{"type": "npm_registry", "registry": "colon.example.net", "token": ":"},
+			host: "colon.example.net",
+		},
+	} {
+		t.Run(tc.name, func(t *testing.T) {
+			h := newEgressHandlerWithCreds(config.Credentials{tc.cred})
+			assert.Nil(t, egressResult(t, h, "https://"+tc.host+"/some/path"),
+				"anonymous credential must allowlist its host: "+tc.host)
+		})
+	}
+}
+
+// TestEgressAllowlist_AnonymousCredentialDoesNotWidenBeyondExactHost holds the
+// dynamic-host exact-match invariant for credential-free entries specifically.
+// Dynamic hosts are matched with AreHostnamesEqual only, so declaring an
+// anonymous registry must authorize that one host and nothing beneath or beside
+// it. These probes fail the moment dynamic matching is relaxed to a suffix.
+func TestEgressAllowlist_AnonymousCredentialDoesNotWidenBeyondExactHost(t *testing.T) {
+	h := newEgressHandlerWithCreds(config.Credentials{
+		config.Credential{"type": "npm_registry", "registry": "nexus.example.net/repository/npm-all"},
+	})
+
+	for _, blocked := range []string{
+		// Child probe: fails if the dynamic entry becomes a leading-dot suffix.
+		"https://evil.nexus.example.net/repository/npm-all",
+		// Sibling probe: fails if the entry is widened to its parent namespace.
+		"https://attacker.example.net/repository/npm-all",
+		// Suffix-appending lookalike.
+		"https://nexus.example.net.attacker.com/repository/npm-all",
+	} {
+		resp := egressResult(t, h, blocked)
+		if assert.NotNil(t, resp, "must stay blocked: "+blocked) {
+			assert.Equal(t, http.StatusForbidden, resp.StatusCode)
+		}
+	}
+}