Refactor Terraform and Hex handlers to support multiple registries per organization

[AbhishekBhaskar]

Refactors terraform_registry.go and hex_organization.go from map-based to slice-based credential storage, enabling support for multiple private registries per ecosystem and aligning with other handler implementations.

Problem:
The Terraform handler used map[string]string keyed by host, which only allowed ONE credential per host. If two Terraform registries shared the same host (e.g., terraform.example.com/org1 and terraform.example.com/org2), the second credential would overwrite the first.

Changes:
terraform_registry.go

• Changed credentials map[string]string → credentials []terraformRegistryCredentials
• Added terraformRegistryCredentials struct with host, url, token fields
• Updated HandleRequest to iterate credentials with URL-first matching via helpers.UrlMatchesRequest(), then host-based matching via helpers.CheckHost()

hex_organization.go

• Changed orgTokens map[string]string → credentials []hexOrganizationCredentials
• Added hexOrganizationCredentials struct with organization, token fields
• Updated matching to iterate slice (consistency with other handlers)

[Copilot]

Pull request overview

Refactors the Terraform registry and Hex organization handlers to store credentials in slices rather than maps, enabling multiple credentials to coexist (e.g., multiple registries on the same hostname) and aligning matching behavior more closely with other request handlers in the proxy.

Changes:

• Terraform handler: replace host→token map with a slice of {host,url,token} entries and match requests by URL prefix first, then host.
• Hex handler: replace org→token map with a slice of {organization,token} entries and match by iterating credentials.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File	Description
internal/handlers/terraform_registry.go	Stores static credentials as a slice and introduces URL-prefix matching to support multiple registries on the same host.
internal/handlers/hex_organization.go	Stores org credentials as a slice and updates request matching to iterate credentials.

Comments suppressed due to low confidence (2)

internal/handlers/terraform_registry.go:54

• NewTerraformRegistryHandler appends static credentials even when token is empty (or when both host and url are empty). If a matching request occurs, this will set Authorization: Bearer and may overwrite an existing Authorization header with an invalid value. Consider filtering out incomplete/invalid static credentials during construction (e.g., require a non-empty token and at least one of url/host).

		terraformCred := terraformRegistryCredentials{
			host:  host,
			url:   credential.GetString("url"),
			token: credential.GetString("token"),
		}
		handler.credentials = append(handler.credentials, terraformCred)

internal/handlers/terraform_registry.go:84

• This new URL-first matching logic is intended to support multiple Terraform registries that share a hostname but differ by path/prefix, but there’s no coverage here for the ambiguous/overlap case (e.g., two credentials for terraform.example.com/org1 and terraform.example.com/org2 and ensuring the correct token is chosen for each path). Adding a test for multiple credentials on the same host with different URL prefixes would help prevent regressions.

	for _, cred := range h.credentials {
		// Match by URL first (more specific), then by host
		if cred.url != "" {
			if helpers.UrlMatchesRequest(request, cred.url, true) {
				logging.RequestLogf(context, "* authenticating terraform registry request (url: %s)", cred.url)
				request.Header.Set("Authorization", "Bearer "+cred.token)
				return request, nil
			}
		} else if cred.host != "" {
			if helpers.CheckHost(request, cred.host) {
				logging.RequestLogf(context, "* authenticating terraform registry request (host: %s)", cred.host)
				request.Header.Set("Authorization", "Bearer "+cred.token)
				return request, nil
			}
		}

[kbukum1]

1. Terraform — No input validation on credentials construction (terraform_registry.go, NewTerraformRegistryHandler): Credentials with empty token or both empty host+url are appended without being filtered out. An empty token would set Authorization: Bearer (invalid). Add validation like Cargo/Pub handlers do.

2. Terraform — Nested if/else matching doesn't follow codebase convention (terraform_registry.go, HandleRequest loop): Use the flat !UrlMatchesRequest && !CheckHost → continue guard pattern like Maven/Python instead of nested if/else if branches.

3. Terraform — Missing test for the core use case (terraform_registry_test.go): No test for multiple credentials on the same host with different URL paths — which is the primary scenario this PR enables.

4. Hex Organization — "token" renamed to "key" is a potential breaking change (hex_organization.go, NewHexOrganizationHandler): cred.GetString("token") changed to cred.GetString("key") reads a different config field. Existing configs using "token" will silently fail.

5. Hex Organization — Missing OIDC support unlike all other handlers (hex_organization.go): Every other handler has oidcCredentials, mutex, and calls oidc.TryAuthOIDCRequestWithPrefix. This handler has none, diverging from the stated alignment goal. (Could be a follow-up.) 🔍

[AbhishekBhaskar]

@kbukum1 I've addressed the above comment with the following changes:

#	Issue	Fix
1	No input validation	Added validation: skip if token == "" or both host and url empty
2	Nested if/else pattern	Changed to flat guard pattern. Key fix: only set host when url is empty, so the || logic works correctly
3	Missing multi-credential test	Added test for multiple credentials on same host with different URL paths
4	Breaking change token → key	Added backwards compatibility: checks key first, falls back to token
5	Missing OIDC support (hex)	Pre-existing issue, noted as follow-up

[kbukum1]

I'm approving the changes. @jakecoffman, @JamieMagee — it would be great to get your confirmation that these structural changes look good.

During our investigation, we noticed Terraform and Hex had a different data structure compared to the other ecosystems. @AbhishekBhaskar aligned them with the existing structure, but we'd love to know if there was a specific reason they differed. Please let us know if you see any issues.

[jakecoffman]

I don't know the history of why these endpoints are this way. You might check the private repo's history to see if there was a reason. The Terraform change is probably ok, but the hex change seems like it wouldn't change behavior.

[JamieMagee]

One real issue: UrlMatchesRequest uses strings.HasPrefix without a path boundary check, so /org matches /org1/.... Not new, but matters more now. See inline comment.

[kbukum1]

@AbhishekBhaskar , we may need to discuss the approach. I am currently creating an approach and I am planing to apply to all. Not sure but we may want to either ships yours and create another to match the sollution I am creating or fix it in one PR.

You can check followings to see what I am trying to do

• Add generalized OIDCRegistry for collision-free credential storage #78
• Migrate cargo handler to OIDCRegistry #79

[Copilot]

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

[Copilot]

Pull request overview

Refactors Terraform registry and Hex organization handlers to store credentials in slices (instead of maps), enabling multiple credentials per host/organization and more consistent matching behavior with other request handlers in the proxy.

Changes:

• Terraform: replace host→token map with a credential slice, add URL-prefix matching with path-boundary checks, and sort credentials for specificity.
• Terraform: add tests covering multiple URL-path credentials on the same host and path-boundary behavior.
• Hex: replace org→token map with a credential slice, support key with legacy token fallback, and add backwards-compatibility tests.

Show a summary per file

File	Description
internal/handlers/terraform_registry.go	Switches to slice-based credential storage; adds URL boundary matching and sorts credentials for more specific matches.
internal/handlers/terraform_registry_test.go	Adds coverage for multiple registry URLs on the same host and path-boundary correctness.
internal/handlers/hex_organization.go	Switches to slice-based storage; supports key and legacy token; updates matching to iterate credentials.
internal/handlers/hex_organization_test.go	Updates tests to use key and adds backwards compatibility coverage for legacy token.

Copilot's findings

• Files reviewed: 4/4 changed files
• Comments generated: 3

[Copilot]

Behavior change vs the previous map implementation: with a slice, the first matching credential wins. Previously, the last configured credential for an organization would overwrite earlier ones in the map. To preserve backwards-compatible precedence, consider iterating credentials in reverse order during matching or de-duplicating by organization during handler construction (keeping the last).

[Copilot]

PR description mentions a token field, but the refactor introduces a key field and also accepts config key/legacy token. Either update the PR description to match the implementation or rename fields for consistency to avoid confusion for reviewers and future maintainers.

[Copilot]

The comment says "URL path length" but the code sorts by len(cred.url) (full URL string length), and sort.Slice is not stable when lengths are equal. This can lead to surprising/unstable credential precedence (e.g., two equivalent URLs differing only by trailing /, or multiple host-only creds where url == "" for all entries). Consider normalizing stored URLs/paths (e.g., trimming trailing /) and sorting by normalized parsed path length with a deterministic tie-breaker (or using sort.SliceStable).