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:
HeadPackumentandHeadTarball, returning aHeadResponse(status plus response headers, no body), per !260 (merged)'s S06 amendment.HTTPError.Allow()andHTTPError.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 singlestringreturn cannot.Publishreturning a*PublishResponserather than anerror, so a caller can see the status of a write that succeeded. The seed-and-settle helper's means-stored verdict needs it:201and202are 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.PackageURLandDistTarballURL, 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
-revwrite 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.
TestSlugAnchorsCoverModuleDescriptorslikewise.- 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
178e28bConfines the advertiseddist.tarballto--upstream-url's origin before fetching it. The settle followed a registry-chosen URL throughGetTarballAtVettedURL, 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 named169.254.169.254, a loopback port or any internal host and had the suite fetch it, authenticated, with every internal-address defense skipped. Newconformance.SameOriginconfines scheme and host and leaves the path to the registry, which is what following the advertisement is for. It reusesnormalizeHost, so it cannot disagree with S08 §Configuration's URL-equality rule abouthttps://hagainsthttps://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.c2f5a0fAttaches the relay read's response toDetail.HTTP, and skips rather than fails a cancelled relay read. Plan Step 19 required both. AStatusFailis the one verdict whoseDetaila 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.ead56f3Widens the non-text body filter to allCcrunes (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.876249bPins the downgrade check's fail-closed arm.f6d06b5Corrects six comments the code stopped backing, includingCoordinate's ordering note.184ced1Corrects Step 19 in the plan and files three Band B spec-amendment candidates, which ship as one MR againstdocs/specs/before Step 20 opens: S08 §Security Considerations overstating the redirect perimeter for--upstream-url, S08 classing the preflightRelaywhen its AC #13 (closed) byte comparison is unsatisfiable for a synthesized packument, and S04 §Entry points still saying three functions are exported frompkg/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.preflightpasses 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 versionThe upstream read-back never succeeds the settle establishes the fixture SKIP, exit2The publish reports 202rather than201means-stored, reissue, duplicate-resume PASS, by design: four spec rules in one runThe upstream serves bytes that are not this run's fixture AC #37 content identity SKIP, exit2, classified as a collisionA 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/httpdecodes%2Fout ofURL.Pathbefore 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 rawRequestURI. 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.