Skip to content

fix: authenticate python index requests on same host regardless of path #114

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

Draft
thavaahariharangit wants to merge 1 commit into
base: main
Choose a base branch
from
Draft

fix: authenticate python index requests on same host regardless of path #114

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
8 changes: 7 additions & 1 deletion internal/handlers/python_index.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,13 @@ func (h *PythonIndexHandler) HandleRequest(req *http.Request, ctx *goproxy.Proxy
// Fall back to static credentials
for _, cred := range h.credentials {
indexURL := simpleSuffixRe.ReplaceAllString(cred.indexURL, "/")
if !helpers.UrlMatchesRequest(req, indexURL, true) && !helpers.CheckHost(req, cred.host) {
// Apply credentials if:
// 1. URL matches with path (e.g., /pypi/...), OR
// 2. Host:port matches (regardless of path), OR
// 3. Explicit host field matches
if !helpers.UrlMatchesRequest(req, indexURL, true) &&
!helpers.UrlMatchesRequest(req, indexURL, false) &&
Comment on lines +100 to +103
Copy link

Copilot AI Apr 28, 2026

Choose a reason for hiding this comment

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

helpers.UrlMatchesRequest(req, indexURL, false) makes index-url credentials apply to any request on the same host:port (regardless of path). This is a behavioral change from URL+path prefix scoping and will cause previously-path-mismatched requests (e.g. /foo when configured for /pyreg/) to be authenticated, potentially leaking credentials to unrelated endpoints on the same host. Consider keeping path scoping for index-url and only allowing host-only matching when an explicit host field is configured (as other handlers do), or introduce an explicit opt-in for host-only matching rather than inferring it from index-url.

Suggested change
// 2. Host:port matches (regardless of path), OR
// 3. Explicit host field matches
if !helpers.UrlMatchesRequest(req, indexURL, true) &&
!helpers.UrlMatchesRequest(req, indexURL, false) &&
// 2. Explicit host field matches
if !helpers.UrlMatchesRequest(req, indexURL, true) &&

Copilot uses AI. Check for mistakes.
Comment on lines +102 to +103
Copy link

Copilot AI Apr 28, 2026

Choose a reason for hiding this comment

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

This multiline if condition has trailing whitespace after &&, which will fail gofmt/linters in many setups. Please run gofmt (it will also reformat the line breaks/indentation for this condition).

Suggested change
if !helpers.UrlMatchesRequest(req, indexURL, true) &&
!helpers.UrlMatchesRequest(req, indexURL, false) &&
if !helpers.UrlMatchesRequest(req, indexURL, true) &&
!helpers.UrlMatchesRequest(req, indexURL, false) &&

Copilot uses AI. Check for mistakes.
!helpers.CheckHost(req, cred.host) {
Comment on lines 95 to +104
Copy link

Copilot AI Apr 28, 2026

Choose a reason for hiding this comment

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

The new host-only matching is only applied to static credentials in the fallback loop. When a python_index credential is OIDC-configured, NewPythonIndexHandler registers it in oidcRegistry and does not add it to h.credentials, so requests on the same host but different paths will still not be authenticated via OIDC (since OIDCRegistry.TryAuth is path-prefix based). If the reported issue affects OIDC-based python index auth too, this change won't address it; consider whether OIDC registrations for python indexes should also support host-only matching (or an explicit opt-in).

Copilot uses AI. Check for mistakes.
continue
}

Expand Down
7 changes: 7 additions & 0 deletions internal/handlers/python_index_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -104,4 +104,11 @@ func TestPythonIndexHandler(t *testing.T) {
req = httptest.NewRequest("GET", "https://PKGS.dev.azure.com/somepkg", nil)
req = handleRequestAndClose(handler, req, nil)
assertHasBasicAuth(t, req, deltaForceUser, deltaForcePassword, "azure devops case insensitive registry request")

// Package download on completely different path on same host
// Simulates: config pypi.cyco.fun/pypi, but request to pypi.cyco.fun/packages/...
// Using corp.deltaforce.com which has / as the index path
req = httptest.NewRequest("GET", "https://corp.deltaforce.com/packages/somepkg/1.0/wheel.whl", nil)
req = handleRequestAndClose(handler, req, nil)
assertHasBasicAuth(t, req, deltaForceUser, deltaForcePassword, "cert registry with package download on different path")
Comment on lines +109 to +113
Copy link

Copilot AI Apr 28, 2026

Choose a reason for hiding this comment

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

This new test case doesn't actually validate the new host-only matching behavior: the configured index-url for corp.deltaforce.com is /, and the existing prefix-matching logic already authenticates every path on that host (because the trimmed path prefix becomes empty). To cover the intended behavior, add a case where the configured index-url has a non-root path (e.g. /pypi/ or /dependabot/) and the request goes to a different top-level path (e.g. /packages/...), and assert the expected auth behavior.

Copilot generated this review using guidance from repository custom instructions.
}
Loading