feat(npm)!: npm remote foundation and walking skeleton, Steps 16-19 (S08 remote, batch 6/12)

Summary

Related to #47 (closed).

The npm band's foundation and its first remote row, per the S08 remote goal-run's annex §7 MR clusters ("npm foundation" plus "npm walking skeleton"). Lands Steps 16, 17, 18 and 19, plus six review-fix commits (§Review fixes below).

Step What it adds
16 The seven npm client gaps the remote rows need
17 The npm env's fixture-upstream client and accessors
18 The npm catalog's branch on repository kind, its protocol baseline, and the per-kind guard
19 npm.remote.preflight, and the remotefake npm adapter's seven seams

!260 (merged) must merge before this one. The plan gives Step 16 Depends on: Step 15, and Step 15 is !260 (merged)'s S06 HEAD amendment. This is a hard gate rather than a preference: 14 S06 AC #14 citations and four references to §HTTP surface's HEAD on the read routes live in this branch's production code and tests, and S06 on main has 13 acceptance criteria and no such subsection. Merging this first puts code on main citing a criterion that does not exist. !260 (merged) targets main independently, so git does not enforce the order and this note is the only thing that carries it. Nothing in this diff fails to compile without !260 (merged); what it lacks without it is the spec text its two HEAD methods are written against.

(!258 (merged) and !259 (merged) merged on 2026-09-02; this MR now targets main directly.)

Step 16: the client gaps

Seven additions to pkg/client/npm, each one a thing a remote row asserts and the hosted rows never needed:

  • HeadPackument and HeadTarball, returning a HeadResponse (status plus response headers, no body), per !260 (merged)'s S06 amendment.
  • HTTPError.Allow() and HTTPError.Code(), each reporting presence separately from value, because S08 AC #22 (closed) distinguishes a header that is absent from one that is present and empty, and a single string return cannot.
  • Publish returning a *PublishResponse rather than an error, so a caller can see the status of a write that succeeded. The seed-and-settle helper's means-stored verdict needs it: 201 and 202 are both non-errors and only one of them means the artifact is there. This is a breaking change to a public Go surface, which is why the MR title carries !; see §Public surface below.
  • PackageURL and DistTarballURL, the two URL builders. They spell a scoped name's path segment differently on purpose, and that is the subject of the correction below.
  • GetTarballAtVettedURL, which fetches an absolute URL rather than one the client computed. This is what S06 §Operation: Tarball download has always required and what the hosted tarball rows already do; the remote settle needs it too. It applies no SSRF allow-list, so the caller owes the confinement: see §Review fixes.
  • The three -rev write routes (PutPackumentRev, DeleteTarballRev, DeletePackageRev), which the remote write-refusal rows exercise in Step 23.
  • A post-redirect body cap and a scheme-downgrade refusal on the vetted-URL hop, so following an absolute URL cannot become an unbounded read or an HTTPS to HTTP downgrade.

S06 grows the client-surface documentation to match. That is the same MR shape as the hosted work: the spec's Go surface listing is normative, so a new exported method lands with its spec row.

Step 17: the env's upstream client

pkg/conformance/npm/env.go gains a second client addressed at --upstream-url, plus the accessors a remote row needs (UpstreamURL(), RegistryURL(), Client(), and the upstream client). A hosted run constructs neither, so the accessors are the seam that keeps a hosted row unable to reach an upstream that is not configured.

Step 18: the kind-aware catalog

module.go's descriptor list branches on Config.RepositoryKind: a hosted run sees exactly today's rows, a remote run sees the protocol baseline plus the remote rows. Three things guard it, and the third is the one worth reviewing:

  • The catalog-inventory test now reads both arms. It previously read only the hosted arm, which meant a remote-only slug could never have been covered by it. That is a pre-existing hole this step had to fix to be able to assert anything.
  • TestSlugAnchorsCoverModuleDescriptors likewise.
  • The per-kind guard asserts a hosted run's descriptor set is unchanged, not merely valid. A branch that leaks a remote row into a hosted run is the regression with the worst blast radius here, since every hosted consumer of this library would start failing on a repository that is not a proxy.

Step 19: npm.remote.preflight

One catalog row, ordered first among a remote run's descriptors. It seeds a package at a run-scoped coordinate through --upstream-url, settles by reading the artifact back from the upstream, then reads the packument through --registry-url and asserts the seeded version is in it.

Its whole reason for existing is diagnostic: when the two URLs name repositories that are not linked, every later row fails for the same reason, and this is the row whose message says so. The failure message names both base URLs, each rendered through redact.RedactURLUserinfo, and the read's own status and body ride on Detail.HTTP (plan Step 19).

The seeding goes through Step 5a's shared SeedAndSettle, so this row inherits S08 §Preflight's cause sort, its one resuming re-attempt, and AC #37's content identity without restating any of it. What is npm-specific is the three callbacks: build the publish body, decide from the write's status whether the artifact is stored, and read it back.

remotefake's npm adapter fills all seven Adapter seams. One ordering in Coordinate is load-bearing and is commented as such: the tarball--rev route must be matched before the bare tarball route, whose prefix test it also satisfies, or a write-only route turns readable and carries the wrong Allow.

Correction to the plan, found against the reference

The plan justified building the seeding probe with the dist-tarball builder by asserting that "the tarball route is not served at the @scope%2Ffoo coordinate". That is backwards for at least one conformant registry. The reference this run validated against serves the tarball only at the percent-encoded spelling, 404s the raw-slash form the builder produces, and advertises the encoded form in its own dist.tarball.

Neither spelling is safe to assume, which is exactly why S06 §Operation: Tarball download says the fetched URL is "the value of dist.tarball from the packument, not a computed path", and that the registry is free to return any absolute URL.

So the read-back computes no URL at all: it reads the upstream packument and fetches the dist.tarball it advertises, via GetTarballAtVettedURL. The existence probe still computes, because before the publish there is no packument to read an advertisement from.

What that costs is not nothing, and an earlier draft of this section said it was. Against a registry whose spelling differs, Preexisting never fires and an occupied coordinate reaches the publish. A duplicate refusal is an AC #37 resume only on the re-attempt: on the first attempt a 409 sorts through residueOrResume to a terminal row-attributable skip (AC #15 (closed)), and a 403 is claimed earlier by the 401-or-403 arm and ends the run naming a credential the upstream in fact accepts. It is accepted rather than fixed because the coordinate is run-ID-scoped, so it is occupied only on a re-run at a reused --run-id, which README.md §Usage already warns about and already names this exact outcome for npm.

The reference-run and mutation evidence behind this is in the goal run's validation/reference-quirks.md R10 and validation/decisions.md D15.

Review fixes, six commits on top of Steps 16-19

  • 178e28b Confines the advertised dist.tarball to --upstream-url's origin before fetching it. The settle followed a registry-chosen URL through GetTarballAtVettedURL, which applies no S06 §Tarball-URL SSRF allow-list and sends the run credential, against that method's own stated precondition and against the S06 text this MR lands. A hostile or compromised upstream could have named 169.254.169.254, a loopback port or any internal host and had the suite fetch it, authenticated, with every internal-address defense skipped. New conformance.SameOrigin confines scheme and host and leaves the path to the registry, which is what following the advertisement is for. It reuses normalizeHost, so it cannot disagree with S08 §Configuration's URL-equality rule about https://h against https://h:443. Also pins the read-back's two previously untestable "not readable yet" branches, states the stub's both-spellings fidelity limit, and moves S06's remote-row method block out of §npm-specific flag into the controlled surface list it claims to be part of.
  • c2f5a0f Attaches the relay read's response to Detail.HTTP, and skips rather than fails a cancelled relay read. Plan Step 19 required both. A StatusFail is the one verdict whose Detail a renderer shows, and an interrupted run was being recorded as a compliance verdict about the target. Neither cancellation point had a test, including the one already implemented.
  • ead56f3 Widens the non-text body filter to all Cc runes (it admitted DEL and the C1 range, including the control sequence introducer), names the wire length rather than the 4 KiB cap every large body saturated, points the operator at a channel that exists, and bounds the scrub window so a 64 MiB artifact is not regex-scanned twice to keep 4 KiB.
  • 876249b Pins the downgrade check's fail-closed arm.
  • f6d06b5 Corrects six comments the code stopped backing, including Coordinate's ordering note.
  • 184ced1 Corrects Step 19 in the plan and files three Band B spec-amendment candidates, which ship as one MR against docs/specs/ before Step 20 opens: S08 §Security Considerations overstating the redirect perimeter for --upstream-url, S08 classing the preflight Relay when its AC #13 (closed) byte comparison is unsatisfiable for a synthesized packument, and S04 §Entry points still saying three functions are exported from pkg/conformance.

Each new guard is mutation-checked: disabling SameOrigin, either of the two cancellation re-checks, or FailWithHTTPDetail each makes the corresponding new test fail. The post-seed re-check was the exception until review caught it: both re-checks produce a message containing "canceled", so the substring assertion passed with the guard replaced by if false, the cancellation falling through to the relay-read re-check one leg later. Both assertions are now exact on the reason, and the claim above holds for all four guards. The SSRF test asserts that no request reached the off-origin server, not merely that an error came back, because an implementation that refused after dialling would already have sent the credential.

Public surface

(*npm.Client).Publish goes from error to (*PublishResponse, error). docs/dev/architecture.md names pkg/client/* as importable by external consumers, notably the Artifact Registry's own integration tests, and the plan records that "a two-value Publish is a breaking change". The MR title therefore carries ! so semantic-release cuts a major rather than the minor a plain feat would produce. All 26 in-tree callers are updated.

Everything else on both surfaces is additive: PackageURL, DistTarballURL, HeadResponse, PublishResponse, HeadPackument, HeadTarball, GetTarballAtVettedURL, the three -rev routes, HTTPError.Allow / Code / ErrorShapeReason, and conformance.SameOrigin. No CLI flag is added, renamed or re-semanticised; no error is re-mapped to a different exit code; no exported error type is renamed or removed. HTTPError gains two unexported fields, header and envelope, and that breaks two things for an external caller. An unkeyed cross-package composite literal, though every in-tree literal is keyed. And comparability: header is an http.Header, so HTTPError values are no longer comparable, and a == b, use as a map key, and satisfying a comparable constraint all stop compiling. Every in-tree use is pointer-based, so nothing here breaks. The semver verdict does not change, since the MR is already breaking.

Spec coverage

Specs: S06 (with the HEAD amendment !260 (merged) lands), S08, S04. The per-step tables below are the merged form of the ones in the f5c4f8f and 8e139ab commit bodies, which carry the full reasoning for each cell.

Step 16, the client gaps

# Criterion Tests
S06 AC-14 HEAD resolves as the GET does with the body omitted, same status, headers and Accept negotiation TestHeadPackument_HonoursAcceptNegotiation, TestHeadPackument_CarriesTheGetsHeaders, TestHeadPackument_UnscopedNamePassesThrough; tarball half via assertVettedSuccess's HEAD branch in every TestVettedPair_* success row and TestHeadTarball_ReportsDeclaredLengthAboveTheBodyCap. "Those two routes only" is a negative and is not asserted
S08 AC-22 Target-specific assertions carried by a catalog row whose Notes name the requiring spec Client half only: TestHTTPError_Code, TestUnpublishRoutes_RefusalSurfacesTheAllowAndTheCode. The row and its Notes are Step 24's
S08 §Write refusal per format 405 with an Allow listing exactly the route's read verbs; a write-only path carries a present-and-empty Allow TestUnpublishRoutes_RefusalSurfacesTheAllowAndTheCode, TestHTTPError_Allow (present, present-and-empty, absent), TestHTTPError_AllowOnATransportError
S08 §Preflight, means-stored Any 2xx other than 202, and only on a terminal request using the write verb TestPublish_ReportsTheTerminalStatus, TestPublish_ReportsTheTerminalMethodAfterARedirect (301/302/303 report GET; 307/308 report PUT), TestPublish_ReportsNoResponseOnFailure
S06 §Tarball-URL SSRF The allow-list polices the host a registry document chose TestGetTarball_RefusesEveryVettedHost against TestVettedPair_ReachesHostsTheAllowListRefuses, run off one table so neither can drift. Step 19's SameOrigin confines the pair's caller on the initial advertised URL only; the hop resolved after it is gated by scheme and drops the credential when the hop leaves the initial host, but its target host is still unconfined and confining it is filed as #68
S06 §Operation: Tarball download One hop resolved manually; malformed Location is a StatusCode 0 rejection; a second 3xx is a wire-layer error TestVettedPair_ResolvesOneHopPreservingMethod, TestVettedPair_MalformedRedirectSurfacesStatusZero, TestVettedPair_SecondRedirectSurfacesHTTPError, TestVettedPair_NonSuccessSurfacesHTTPError
S06 §Request shape The scoped-name slash is encoded exactly once, idempotently TestPackageURL_TakesThePackumentShape, plus the wire-path assertions in TestHeadPackument_HonoursAcceptNegotiation and TestUnpublishRoutes_RefusalSurfacesTheAllowAndTheCode
S06 §Operation: Publish dist.tarball keeps the scoped name in the path and drops the scope from the filename TestDistTarballURL_TakesTheTarballShape, TestDistTarballURL_MatchesThePublishedDistTarball, TestURLBuilders_AreNotInterchangeable
S04 §HTTP client policy HTTPS-to-HTTP is rejected on any hop, same-host included TestVettedPair_RefusesHTTPSToHTTPRedirectHop, TestVettedPair_RefusesAHopWhoseOriginSchemeCannotBeEstablished (the fail-closed arm), with TestVettedPair_FollowsHopsThatDoNotDowngrade as the control
S04 AC-32, AC-33 *HTTPError round-trips through errors.As; ResponseBody truncated to 4 KiB Pre-existing, plus TestHTTPError_CodeOnATruncatedBody for the new accessor

Error cases: not-found baseline (TestHeadPackument_NonSuccessSurfacesHTTPError, TestVettedPair_NonSuccessSurfacesHTTPError); duplicate publish (TestPublish_ReportsNoResponseOnFailure); over-cap body (TestGetTarballAtVettedURL_OverCapBodySurfacesHTTPError, TestHeadTarball_ReportsDeclaredLengthAboveTheBodyCap); write-only 405 (TestUnpublishRoutes_RefusalSurfacesTheAllowAndTheCode, kept honest by TestUnpublishRoutes_SuccessReportsNoError). A pair-specific transport-failure row is deliberately absent: it shares doWith with GetTarball, already covered.

Security: the credential rides a same-host hop and is dropped on a cross-host one (TestVettedPair_ForwardsCredentialOnASameHostHop, TestVettedPair_DropsCredentialOnACrossHostHop), which leaves the same-host hop as the exposure the downgrade refusal covers; path-injection escaping (TestUnpublishRoutes_EscapeTheirVariableSegments); and the allow-list split above.

Step 19, the row and its callbacks

# Criterion Tests
AC-1 A remote run executes ## Remote rows and no ## Local or ## Errors row This row's half: TestRemotePreflight_IsFirstInTheRemoteCatalogAndNeedsUpstream. Section-level assertions are Step 18's
AC-13 Two-channel half only. Seed through --upstream-url, read through --registry-url TestRemotePreflight_SeedsUpstreamSettlesOnTheOriginAndRelaysThroughTheRemote (asserted over the observation log's Side, not the verdict), TestRemoteSeedReadBack_ReadsTheTarballAndReturnsTheServedBytes. The byte-identity half is not satisfiable by this row and is Steps 20-21's; filed as a Band B spec-amendment candidate
AC-15 A refused seeding write skips with a reason naming --upstream-url and the status, carrying the write's own response as Detail TestRemotePreflight_A409OnTheFirstAttemptSkipsNamingUpstreamURLAndTheStatus, TestSeedAndSettle_A409OnTheFirstAttemptIsTheRowsOwn
AC-24 Ordered first; on failure StatusFail naming both URLs and the observed detail, later rows still run; both URLs through RedactURLUserinfo TestRemotePreflight_IsFirstInTheRemoteCatalogAndNeedsUpstream, TestRemotePreflight_FailsWhenTheRelayedPackumentOmitsTheSeededVersion, TestRemotePreflight_ARelayReadFailureCarriesTheRefusalStatusInDetail, TestRemotePreflight_MessageRendersUserinfoInEitherBaseURLAsTheSentinel. The conditional body half is target-dependent and is asserted at its own site by TestSeedAndSettle_ScrubsTheDetailWithBothPasses
AC-26 A NeedsUpstream row skips with the runner's reason when the Env exposes no upstream TestRemotePreflight_SkipsUnderUpstreamFreeOnly, driven through RunModule
AC-29 Each remote row passes when run alone under a --filter selecting only it TestRemotePreflight_RunsAloneUnderAFilterSelectingOnlyIt
AC-33 A row that cannot seed ends the run with exit 2 carrying that failure's detail; the re-attempt resumes rather than restarts TestSeedAndSettle_A403OnTheFirstAttemptEndsTheRun, TestSeedAndSettle_AWriteThatLandsAndNeverSettlesCarriesTheSecondAttemptsDetail, TestRemotePreflight_AWriteThatLandsAndNeverSettlesEndsTheRun, TestRemoteSeedWrite_ReportsNPMsMeansStoredAndDuplicateRefusalVerdicts, TestSeedAndSettle_Reissues202AndReissuesNeither200Nor201
AC-35 Fixture-needing ## Remote rows carry NeedsUpstream: true The row's own half: TestRemotePreflight_IsFirstInTheRemoteCatalogAndNeedsUpstream. The partition assertion is Step 25's
AC-37 A duplicate refusal on the re-attempt is the first write's success only when the resumed settle is byte-identical Both triggers, both directions: TestSeedAndSettle_ADuplicateRefusalOnTheReattemptResumesAndChecksIdentity (four rows), TestSeedAndSettle_APreexistingCoordinateChecksContentIdentity, TestRemoteSeedWrite_ReportsPreexistingWithoutPublishing, TestNPMPair_ResumedSettleFaultServesMatchingOrDifferingBytes
S04 AC-29 A cancelled run is not a compliance verdict TestRemotePreflight_ACancelledSeedingSkipsRatherThanFailing, TestRemotePreflight_ACancelledRelayReadSkipsRatherThanFailing
S08 §Security Considerations The advertised dist.tarball is confined to --upstream-url's origin before an authenticated fetch TestSameOrigin (19 rows, both directions), TestRemoteSeedReadBack_RefusesAnAdvertisementOffTheUpstreamOrigin (asserts no request reached the off-origin server)

Error cases: --upstream-free-only (TestRemotePreflight_SkipsUnderUpstreamFreeOnly, including that no request reaches either handler); residue skip (E-3 above); 401/403 on the first attempt (TestSeedAndSettle_A403OnTheFirstAttemptEndsTheRun); transient then exit 2 (TestSeedAndSettle_AWriteThatLandsAndNeverSettlesCarriesTheSecondAttemptsDetail); the two URLs not linked (TestRemotePreflight_FailsWhenTheRelayedPackumentOmitsTheSeededVersion); a relay read that fails outright (TestRemotePreflight_ARelayReadFailureCarriesTheRefusalStatusInDetail); the packument advertising nothing yet (TestRemoteSeedReadBack_ReportsNotReadableWhenThePackumentOmitsTheVersion, ...WhenTheAdvertisedTarballIsEmpty).

Test plan

  • go test -race ./... green.

  • golangci-lint run ./...: 0 issues.

  • pre-commit run --all-files: every hook passes.

  • Run against a live reference registry (a locally booted Artifact Registry with a remote npm repository over a real upstream), not only against remotefake. npm.remote.preflight passes cold and warm.

  • Negative controls, through a mutating proxy sitting between the tool and the reference, one per assertion class the step introduces. Each mutation makes the row fail for its own reason:

    Mutation Assertion class Result
    The relayed packument loses the seeded version the relay assertion FAIL, message names both URLs and the missing version
    The upstream read-back never succeeds the settle establishes the fixture SKIP, exit 2
    The publish reports 202 rather than 201 means-stored, reissue, duplicate-resume PASS, by design: four spec rules in one run
    The upstream serves bytes that are not this run's fixture AC #37 content identity SKIP, exit 2, classified as a collision

    A control run with no mutation passes first, since a matrix whose baseline does not pass proves nothing about the rows below it.

  • remotefake's own model-fidelity limit is stated in its tests rather than left implicit: net/http decodes %2F out of URL.Path before any handler runs, so the double routes both scoped-name spellings identically and cannot catch a wrong tarball-URL builder. A separate recording upstream asserts the raw RequestURI. The purpose-built stub shares that limit, which is now stated where its routing is.

The live-reference run and the negative-control matrix predate the six review-fix commits. They remain the evidence for Steps 16-19 as originally authored. They are not evidence for the origin confinement, the new Detail.HTTP, or the two cancellation guards, which are covered by unit tests with a mutation check on each new guard. The post-seed cancellation guard's check was not real until review caught it; see §Review fixes.

Size

9,035 insertions and 330 deletions: 6,894 test, 277 docs, about 1,864 lines of production Go across four steps and six review fixes. Batching is annex §7's shape for this run and is recorded as a deviation in the goal run's validation/deviations.md; splitting the four steps into four MRs would have put a client-surface MR, an env MR and a catalog-branch MR in review with nothing exercising any of them. Step 16 is the one that was separable on its own merits, noted for the next batch.

One defect this MR fixes in Step 5a's code

35b62a4 changes pkg/conformance/seed.go, which landed in !259 (merged). A seed failure appends the last response body to the operator's message, by design, since a registry's error body is usually the most useful thing to show. Nothing guaranteed those bytes were text.

The body must now be valid UTF-8 carrying no control character (Unicode category Cc, which is C0 plus DEL plus C1) other than tab, newline and carriage return, or it is described by its wire length instead. SeedError.Result keeps the bytes either way.

Found by the mutation above that swaps a tarball read's 200 to a 404, which leaves the gzip tarball as the "error body". It is the second path of the same class as the fix in 580a1ce; that one covered the branch where the body is the artifact, this one covers the branch that appends by design.

ead56f3 corrects two things about this fix that review caught. The filter was r < 0x20, which admitted DEL and the whole C1 range including the control sequence introducer a terminal still acts on. And the stated reason was wrong: all three surfaces the message reaches strip control bytes already (the JUnit path through stripControlLines, stdout through writeIndented, the exit-2 stderr line through internal/cli's stripControl). The real hazard is that a pasted tarball fills the operator's one line with kilobytes of ? and buries the cause in front of it. What the stderr line alone applies nothing of is redaction, which is a separate and correct claim.

Process

This MR came from a goal run whose annex §7 defines its scope. Judgement calls are recorded in validation/decisions.md, divergences between the in-tree double and the live reference in validation/reference-quirks.md, rather than resolved silently.

Edited by Sylvia Shen

Merge request reports

Loading
Loading