feat(remote): shared upstream URL gate and npm remote details read (S15 Step 7, part 1/3)

David Fernandezrequested to merge
10io/s15-step-7-part1of3 into main

📚 Stacked MRs

This change is split into 3 stacked MRs to keep each within the review size limit (≈800 reviewable LoC). Each part targets the previous one (part 1 targets main); review and merge proceed bottom-up.

📦 What this MR does

Part 1 of 3 of npm remote plan — Step 7: repo service layer — credentials + cache-validity:

  • Extracts the fail-closed upstream base-URL canonicalization gate to internal/remote as remote.ParseUpstreamBase, the single definition of "canonical" the remote slices persist, compare (the credential-clearing URL-change comparison), and pin against.

  • Rewires Maven's parseUpstreamBase to a thin delegate (maven.ErrUpstreamBaseURL aliases the shared sentinel, errors.Is-compatible both ways). The gate hardens three spellings the Maven copy accepted — a zero-padded port (:0443), an out-of-range port (:99999), and a trailing-dot host — each refused at configuration time; all three are pinned in Maven's own rejection tables as well as the gate's suite.

  • Adds the npm remote-repository redacted details read (FindRemoteDetailsByRepositoryID): the service-layer GET beneath the future management CRUD. The returned struct has no token field and has_credentials is computed in SQL, so the plaintext token never leaves the database on this path. The query carries the read paths' full parent gate (npm format, remote kind, soft-delete) and namespace isolation on both join sides.

  • Converges the npm request-URL builder onto the shared gate too, so one stored URL is not gated by one predicate on write and a weaker one on read. npmremote.NewUpstreamURLBuilder delegates to remote.ParseUpstreamBase exactly as Maven's does; its three local sentinels and their redaction helper are deleted. The builder inherits the refusals it lacked (scheme allowlist, empty/zero-padded/out-of-range port, trailing-dot host, non-canonically escaped and unclean paths), and a base url.Parse rejects no longer carries the net/url cause, which for a malformed escape inside userinfo is credential bytes. It has no production caller yet, so the effect is on tests.

  • Folds the last two duplicated definitions in the same area onto the shared package: the dialable-scheme pair (remote.SchemeHTTP/SchemeHTTPS, with Maven's redirect check now reading them) and maven.ErrUpstreamBaseURL, which aliased the shared sentinel under a name nothing outside the package referenced.

✅ Spec coverage

Requirement Tests
S15 Credentials and health AC 2 — a service-layer read surfaces only a presence flag, never the token TestNpmRemoteRepositoryStore_FindRemoteDetailsByRepositoryID (integration), TestNpmRemoteRepositoryStore_FindRemoteDetailsByRepositoryID_ArgumentGuards (unit)
Gate refusal set + canonical form + idempotence TestParseUpstreamBase (each row pins the verdict naming the arm that refused it), TestParseUpstreamBase_IsIdempotent (over every accepted form), TestParseUpstreamBase_DropsParseCause, FuzzParseUpstreamBase (totality, idempotence, and no interpolation of the input in any verdict)
Maven inherits the hardened refusals new rows in TestNormalizeUpstreamURL_RejectsUnusableURL, TestNewUpstreamURLBuilder_RejectsUnusableBase, and TestResolveCacheValidityHours (the trailing-dot host, whose refusal replaces a silent unpin of a Central upstream)
npm inherits them through the same gate TestNewUpstreamURLBuilder_RejectsUnusableBase and TestNewUpstreamURLBuilder_DropsTheParseCause (npmremote)

🔍 Notes for reviewers

  • The error paths of the gate never interpolate the raw URL: userinfo, query, or fragment components can carry credentials, so each verdict names the component instead (pinned by tests with SECRET markers).
  • No e2e scenario catalog update: this is the datastore service layer beneath the not-yet-shipped management CRUD; no HTTP surface changes.

Related to #287

:bricks: Stacked MRs (review/merge bottom-up)

Edited by David Fernandez

Merge request reports