From d8e2446262cfb5b38fee65975bb0dcbf6d8137b8 Mon Sep 17 00:00:00 2001 From: unkin-agent Date: Fri, 16 Oct 2026 23:08:20 +1100 Subject: [PATCH 1/6] Add local npm registry publish and install --- README.md | 15 ++ e2e-docker/README.md | 4 +- e2e-docker/npm_local_test.go | 62 +++++ internal/api/v1/npm.go | 286 ++++++++++++++++++++++++ internal/api/v1/npm_test.go | 162 ++++++++++++++ internal/api/v1/proxy.go | 9 + ui/src/components/UsageInstructions.tsx | 21 ++ 7 files changed, 558 insertions(+), 1 deletion(-) create mode 100644 e2e-docker/npm_local_test.go create mode 100644 internal/api/v1/npm.go create mode 100644 internal/api/v1/npm_test.go diff --git a/README.md b/README.md index 660a658..1cecd76 100644 --- a/README.md +++ b/README.md @@ -279,6 +279,20 @@ content-addressable store (deduplicated by digest, reaped by GC once unreferenced); tags are mutable references and re-pushing a tag moves it. Blob uploads support both the monolithic and chunked (`POST`/`PATCH`/`PUT`) flows. +### Local npm registry + +A local `npm` repo is a real npm registry: `npm publish` (scoped and unscoped) +and `npm install` work against `/api/v1/local/{name}/`. npm requires an +`_authToken` for the registry before it will publish; any value works. + +```ini +# .npmrc +registry=https://artifactapi.k8s.syd1.au.unkin.net/api/v1/local/npm-internal/ +//artifactapi.k8s.syd1.au.unkin.net/api/v1/local/npm-internal/:_authToken=unused +``` + +Versions are immutable; dist-tags sent with a publish move to that version. + ## Access Control | Field | Default | Behaviour | @@ -297,6 +311,7 @@ No patterns + no blocklist = open proxy. Provider handles mutability classificat ``` GET /api/v1/remote/{name}/{path} Proxy/cache artifact GET /api/v1/virtual/{name}/{path} Virtual repo (merged index) +PUT /api/v1/local/{name}/{package} npm publish (local npm repos) GET /v2/{name}/{path} Docker Registry v2 ``` diff --git a/e2e-docker/README.md b/e2e-docker/README.md index 3125d23..3694888 100644 --- a/e2e-docker/README.md +++ b/e2e-docker/README.md @@ -27,7 +27,9 @@ already-running stack. mock upstream: first fetch `X-Artifact-Source: remote`, second `cache`, bytes verified against the origin fixture. - **Local uploads** — generic (upload/download), pypi (wheel + generated `simple/` - index), rpm (real package + **automatic repodata** generation). + index), rpm (real package + **automatic repodata** generation), npm (stock + `node:22` client: `npm publish` unscoped + scoped, `npm install` + `require()`, + second version, republish rejected). - **Virtual repositories** — pypi simple-index merge and helm `index.yaml` merge across two members. - **Mirrorlist** — an rpm remote with a `mirrorlist` of extra upstream mirrors diff --git a/e2e-docker/npm_local_test.go b/e2e-docker/npm_local_test.go new file mode 100644 index 0000000..9455611 --- /dev/null +++ b/e2e-docker/npm_local_test.go @@ -0,0 +1,62 @@ +//go:build dockere2e + +package e2edocker + +import ( + "fmt" + "os" + "os/exec" + "strings" + "testing" + "time" +) + +// TestLocalNPMPublishInstall drives a stock npm client against a local npm +// repo: publish unscoped and scoped packages, install and require() them in a +// fresh project, publish a second version, and reject a republish. +func TestLocalNPMPublishInstall(t *testing.T) { + network := os.Getenv("COMPOSE_NETWORK") + internal := os.Getenv("ARTIFACTAPI_INTERNAL") + if network == "" || internal == "" { + t.Skip("COMPOSE_NETWORK/ARTIFACTAPI_INTERNAL not set; run via scripts/docker-e2e.sh") + } + if _, err := exec.LookPath("docker"); err != nil { + t.Skip("docker not available on the test host") + } + + name := fmt.Sprintf("e2e-npm-local-%d", time.Now().UnixNano()) + createRepo(t, fmt.Sprintf(`{"name":%q,"package_type":"npm","repo_type":"local"}`, name)) + defer deleteRepo(t, name) + + reg := strings.TrimRight(internal, "/") + "/api/v1/local/" + name + "/" + script := `set -euo pipefail +printf 'registry=%s\n%s:_authToken=unused\n' "$REG" "${REG#http:}" > ~/.npmrc +mk() { d="/w/src/$1-$2"; mkdir -p "$d"; cd "$d" + printf '{"name":"%s","version":"%s","main":"index.js"}' "$1" "$2" > package.json + echo "module.exports = '$1@$2';" > index.js; } +mk e2e-npm-plain 1.0.0 && npm publish +mk @e2e/npm-scoped 1.0.0 && npm publish +mk e2e-npm-plain 1.0.0 +if npm publish 2>/dev/null; then echo "republish accepted"; exit 1; fi +mkdir -p /w/app && cd /w/app && npm init -y >/dev/null +npm install --no-audit --cache /tmp/fresh-cache e2e-npm-plain @e2e/npm-scoped +node -e "console.log('REQ', require('e2e-npm-plain'), require('@e2e/npm-scoped'))" +mk e2e-npm-plain 1.1.0 && npm publish +echo "VERSIONS $(npm view e2e-npm-plain versions --json | tr -d ' \n')" +echo "LATEST $(npm view e2e-npm-plain dist-tags.latest)"` + + out, err := exec.Command("docker", "run", "--rm", "--network", network, + "-e", "REG="+reg, "node:22", "bash", "-c", script).CombinedOutput() + if err != nil { + t.Fatalf("npm client run failed: %v\n%s", err, out) + } + for _, want := range []string{ + "REQ e2e-npm-plain@1.0.0 @e2e/npm-scoped@1.0.0", + `VERSIONS ["1.0.0","1.1.0"]`, + "LATEST 1.1.0", + } { + if !strings.Contains(string(out), want) { + t.Fatalf("missing %q in npm output:\n%s", want, out) + } + } +} diff --git a/internal/api/v1/npm.go b/internal/api/v1/npm.go new file mode 100644 index 0000000..253d245 --- /dev/null +++ b/internal/api/v1/npm.go @@ -0,0 +1,286 @@ +package v1 + +import ( + "bytes" + "context" + "crypto/sha1" + "crypto/sha512" + "encoding/base64" + "encoding/hex" + "encoding/json" + "errors" + "fmt" + "io" + "log/slog" + "net/http" + "net/url" + "regexp" + "strings" + + "github.com/go-chi/chi/v5" + + "git.unkin.net/unkin/artifactapi/internal/database" + "git.unkin.net/unkin/artifactapi/internal/storage" + "git.unkin.net/unkin/artifactapi/pkg/models" +) + +// Local npm repos speak the npm registry publish/read protocol. Per package: +// /-/-.tgz tarball (immutable) +// /-/-.json version manifest (immutable) +// /-/dist-tags/ version string (mutable) + +var ( + npmNameRe = regexp.MustCompile(`^(@[a-z0-9][a-z0-9._~-]*/)?[a-z0-9][a-z0-9._~-]*$`) + npmRefRe = regexp.MustCompile(`^[0-9A-Za-z][0-9A-Za-z.+_-]*$`) +) + +func (h *ProxyHandler) localNPMRemote(r *http.Request, name string) (*models.Remote, bool) { + remote, err := h.db.GetRemote(r.Context(), name) + if err != nil { + return nil, false + } + return remote, remote.RepoType == models.RepoTypeLocal && remote.PackageType == models.PackageNPM +} + +// npmPackageName returns the package a packument/publish path addresses, or "" +// when the path is something else (e.g. a tarball under /-/). +func npmPackageName(raw string) string { + p, err := url.PathUnescape(raw) + if err != nil { + return "" + } + p = strings.TrimSuffix(p, "/") + if !npmNameRe.MatchString(p) { + return "" + } + return p +} + +type npmRelease struct { + version string + manifest []byte + tarball []byte + tags []string +} + +func parseNPMPublish(body io.Reader, pkg string) (*npmRelease, error) { + var doc struct { + Name string `json:"name"` + DistTags map[string]string `json:"dist-tags"` + Versions map[string]json.RawMessage `json:"versions"` + Attachments map[string]struct { + Data string `json:"data"` + } `json:"_attachments"` + } + if err := json.NewDecoder(body).Decode(&doc); err != nil { + return nil, fmt.Errorf("invalid publish body: %w", err) + } + if doc.Name != pkg { + return nil, fmt.Errorf("body name %q does not match %q", doc.Name, pkg) + } + if len(doc.Versions) != 1 || len(doc.Attachments) != 1 { + return nil, errors.New("publish must carry exactly one version and one tarball") + } + + rel := &npmRelease{} + for v, m := range doc.Versions { + rel.version, rel.manifest = v, m + } + if !npmRefRe.MatchString(rel.version) { + return nil, fmt.Errorf("invalid version %q", rel.version) + } + var meta struct { + Name string `json:"name"` + Version string `json:"version"` + Dist struct { + Integrity string `json:"integrity"` + Shasum string `json:"shasum"` + } `json:"dist"` + } + if err := json.Unmarshal(rel.manifest, &meta); err != nil { + return nil, fmt.Errorf("invalid version manifest: %w", err) + } + if meta.Name != pkg || meta.Version != rel.version { + return nil, errors.New("version manifest name/version mismatch") + } + + for _, a := range doc.Attachments { + tgz, err := base64.StdEncoding.DecodeString(a.Data) + if err != nil { + return nil, fmt.Errorf("invalid tarball encoding: %w", err) + } + rel.tarball = tgz + } + if err := verifyNPMDist(rel.tarball, meta.Dist.Integrity, meta.Dist.Shasum); err != nil { + return nil, err + } + + for tag, v := range doc.DistTags { + if !npmRefRe.MatchString(tag) || v != rel.version { + return nil, fmt.Errorf("dist-tag %q must point at the published version", tag) + } + rel.tags = append(rel.tags, tag) + } + return rel, nil +} + +func verifyNPMDist(tgz []byte, integrity, shasum string) error { + if b64, ok := strings.CutPrefix(integrity, "sha512-"); ok { + sum := sha512.Sum512(tgz) + if base64.StdEncoding.EncodeToString(sum[:]) != b64 { + return errors.New("tarball does not match dist.integrity") + } + return nil + } + if shasum != "" { + sum := sha1.Sum(tgz) + if hex.EncodeToString(sum[:]) != shasum { + return errors.New("tarball does not match dist.shasum") + } + return nil + } + return errors.New("version manifest carries no sha512 integrity or shasum") +} + +func npmError(w http.ResponseWriter, status int, msg string) { + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(status) + json.NewEncoder(w).Encode(map[string]string{"error": msg}) +} + +func (h *ProxyHandler) handleLocalPut(w http.ResponseWriter, r *http.Request) { + remote, ok := h.localNPMRemote(r, chi.URLParam(r, "localName")) + if !ok { + npmError(w, http.StatusMethodNotAllowed, "publish is only supported on local npm repositories") + return + } + pkg := npmPackageName(chi.URLParam(r, "*")) + if pkg == "" { + npmError(w, http.StatusBadRequest, "invalid package name") + return + } + rel, err := parseNPMPublish(r.Body, pkg) + if err != nil { + npmError(w, http.StatusBadRequest, err.Error()) + return + } + + base := pkg[strings.LastIndex(pkg, "/")+1:] + stem := pkg + "/-/" + base + "-" + rel.version + // The tarball's immutable row is the version's uniqueness gate. + if err := h.npmStore(r.Context(), remote.Name, stem+".tgz", rel.tarball, "application/octet-stream", false); err != nil { + if errors.Is(err, database.ErrAlreadyExists) { + npmError(w, http.StatusConflict, fmt.Sprintf("%s@%s already exists; overwrites are not allowed", pkg, rel.version)) + return + } + npmError(w, http.StatusInternalServerError, err.Error()) + return + } + if err := h.npmStore(r.Context(), remote.Name, stem+".json", rel.manifest, "application/json", false); err != nil { + npmError(w, http.StatusInternalServerError, err.Error()) + return + } + for _, tag := range rel.tags { + if err := h.npmStore(r.Context(), remote.Name, pkg+"/-/dist-tags/"+tag, []byte(rel.version), "text/plain", true); err != nil { + npmError(w, http.StatusInternalServerError, err.Error()) + return + } + } + + slog.Info("local npm package published", "repo", remote.Name, "package", pkg, "version", rel.version) + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(http.StatusCreated) + json.NewEncoder(w).Encode(map[string]any{"ok": true, "id": pkg}) +} + +func (h *ProxyHandler) npmStore(ctx context.Context, repo, path string, data []byte, contentType string, mutable bool) error { + result, err := h.cas.Store(ctx, bytes.NewReader(data), contentType) + if err != nil { + return err + } + if err := h.db.UpsertBlob(ctx, result.ContentHash, result.S3Key, result.SizeBytes, contentType); err != nil { + return err + } + if mutable { + return h.db.UpsertLocalFile(ctx, repo, path, result.ContentHash) + } + return h.db.CreateLocalFile(ctx, repo, path, result.ContentHash) +} + +func (h *ProxyHandler) readLocalBlob(ctx context.Context, contentHash string) ([]byte, error) { + reader, _, err := h.store.Download(ctx, storage.BlobKey(strings.TrimPrefix(contentHash, "sha256:"))) + if err != nil { + return nil, err + } + defer reader.Close() + return io.ReadAll(reader) +} + +func (h *ProxyHandler) serveNPMPackument(w http.ResponseWriter, r *http.Request, remote *models.Remote, pkg string) { + files, err := h.db.ListLocalFilesByPrefix(r.Context(), remote.Name, pkg+"/-/") + if err != nil { + npmError(w, http.StatusInternalServerError, err.Error()) + return + } + read := func(hash string) ([]byte, error) { return h.readLocalBlob(r.Context(), hash) } + tarballBase := fmt.Sprintf("%s://%s/api/v1/local/%s/", scheme(r), r.Host, remote.Name) + doc, err := buildNPMPackument(pkg, files, read, tarballBase) + if err != nil { + npmError(w, http.StatusInternalServerError, err.Error()) + return + } + if doc == nil { + npmError(w, http.StatusNotFound, "not found") + return + } + w.Header().Set("Content-Type", "application/json") + w.Header().Set("X-Artifact-Source", "local") + w.WriteHeader(http.StatusOK) + json.NewEncoder(w).Encode(doc) +} + +// buildNPMPackument returns nil when pkg has no published versions. +func buildNPMPackument(pkg string, files []database.LocalFile, read func(hash string) ([]byte, error), tarballBase string) (map[string]any, error) { + prefix := pkg + "/-/" + versions := map[string]any{} + tags := map[string]string{} + for _, f := range files { + // LIKE treats '_' as a wildcard, so the prefix match can over-select. + rest, ok := strings.CutPrefix(f.FilePath, prefix) + if !ok { + continue + } + if tag, isTag := strings.CutPrefix(rest, "dist-tags/"); isTag { + v, err := read(f.ContentHash) + if err != nil { + return nil, err + } + tags[tag] = string(v) + continue + } + stem, isManifest := strings.CutSuffix(rest, ".json") + if !isManifest { + continue + } + raw, err := read(f.ContentHash) + if err != nil { + return nil, err + } + var m map[string]any + if err := json.Unmarshal(raw, &m); err != nil { + return nil, fmt.Errorf("manifest %s: %w", f.FilePath, err) + } + dist, _ := m["dist"].(map[string]any) + if dist == nil { + dist = map[string]any{} + m["dist"] = dist + } + dist["tarball"] = tarballBase + prefix + stem + ".tgz" + v, _ := m["version"].(string) + versions[v] = m + } + if len(versions) == 0 { + return nil, nil + } + return map[string]any{"_id": pkg, "name": pkg, "dist-tags": tags, "versions": versions}, nil +} diff --git a/internal/api/v1/npm_test.go b/internal/api/v1/npm_test.go new file mode 100644 index 0000000..d1af157 --- /dev/null +++ b/internal/api/v1/npm_test.go @@ -0,0 +1,162 @@ +package v1 + +import ( + "crypto/sha1" + "crypto/sha512" + "encoding/base64" + "encoding/hex" + "encoding/json" + "errors" + "strings" + "testing" + + "git.unkin.net/unkin/artifactapi/internal/database" +) + +func TestNPMPackageName(t *testing.T) { + tests := map[string]string{ + "left-pad": "left-pad", + "left-pad/": "left-pad", + "@acme%2fwidget": "@acme/widget", + "@acme%2Fwidget": "@acme/widget", + "@acme/widget": "@acme/widget", + "left-pad/-/left-pad.tgz": "", + "@acme/widget/-/w-1.tgz": "", + "a/b": "", + "../etc": "", + "Upper": "", + "": "", + } + for in, want := range tests { + if got := npmPackageName(in); got != want { + t.Errorf("npmPackageName(%q) = %q, want %q", in, got, want) + } + } +} + +type publishOpts struct { + bodyName, manifestName, version, integrity, shasum string + tags map[string]string + extraVersion bool +} + +func publishBody(t *testing.T, pkg string, tgz []byte, o publishOpts) string { + t.Helper() + sha := sha512.Sum512(tgz) + sh1 := sha1.Sum(tgz) + if o.bodyName == "" { + o.bodyName = pkg + } + if o.manifestName == "" { + o.manifestName = pkg + } + if o.version == "" { + o.version = "1.0.0" + } + if o.integrity == "" { + o.integrity = "sha512-" + base64.StdEncoding.EncodeToString(sha[:]) + } + if o.shasum == "" { + o.shasum = hex.EncodeToString(sh1[:]) + } + if o.tags == nil { + o.tags = map[string]string{"latest": o.version} + } + versions := map[string]any{o.version: map[string]any{ + "name": o.manifestName, "version": o.version, "main": "index.js", + "dist": map[string]any{"integrity": o.integrity, "shasum": o.shasum, "tarball": "http://elsewhere/x.tgz"}, + }} + if o.extraVersion { + versions["2.0.0"] = map[string]any{"name": pkg, "version": "2.0.0"} + } + b, _ := json.Marshal(map[string]any{ + "_id": pkg, "name": o.bodyName, "dist-tags": o.tags, "versions": versions, + "_attachments": map[string]any{pkg + "-" + o.version + ".tgz": map[string]any{ + "content_type": "application/octet-stream", "data": base64.StdEncoding.EncodeToString(tgz), "length": len(tgz), + }}, + }) + return string(b) +} + +func TestParseNPMPublish(t *testing.T) { + tgz := []byte("fake tarball bytes") + + rel, err := parseNPMPublish(strings.NewReader(publishBody(t, "@acme/widget", tgz, publishOpts{tags: map[string]string{"latest": "1.0.0", "beta": "1.0.0"}})), "@acme/widget") + if err != nil { + t.Fatalf("valid scoped publish rejected: %v", err) + } + if rel.version != "1.0.0" || string(rel.tarball) != string(tgz) || len(rel.tags) != 2 { + t.Fatalf("unexpected release: %+v", rel) + } + + if _, err := parseNPMPublish(strings.NewReader(publishBody(t, "left-pad", tgz, publishOpts{integrity: "none", shasum: hex.EncodeToString(make([]byte, 20))})), "left-pad"); err == nil { + t.Fatal("shasum fallback should reject a mismatched tarball") + } + if _, err := parseNPMPublish(strings.NewReader(publishBody(t, "left-pad", tgz, publishOpts{integrity: "none"})), "left-pad"); err != nil { + t.Fatalf("matching shasum fallback rejected: %v", err) + } + + bad := map[string]publishOpts{ + "body name mismatch": {bodyName: "other"}, + "manifest name mismatch": {manifestName: "other"}, + "integrity mismatch": {integrity: "sha512-AAAA"}, + "path-like version": {version: "../1.0.0"}, + "tag at another version": {tags: map[string]string{"latest": "9.9.9"}}, + "path-like tag": {tags: map[string]string{"../x": "1.0.0"}}, + "multiple versions at once": {extraVersion: true}, + } + for name, o := range bad { + if _, err := parseNPMPublish(strings.NewReader(publishBody(t, "left-pad", tgz, o)), "left-pad"); err == nil { + t.Errorf("%s: expected rejection", name) + } + } + if _, err := parseNPMPublish(strings.NewReader(`{"name":"left-pad","versions":{}}`), "left-pad"); err == nil { + t.Error("deprecate-style body without attachments should be rejected") + } +} + +func TestBuildNPMPackument(t *testing.T) { + blobs := map[string]string{ + "h1": `{"name":"my_pkg","version":"1.0.0","dist":{"shasum":"aa","tarball":"http://elsewhere/x.tgz"}}`, + "h2": `{"name":"my_pkg","version":"1.1.0"}`, + "h3": "1.1.0", + "hx": `{"name":"myXpkg","version":"6.6.6"}`, + } + read := func(h string) ([]byte, error) { + b, ok := blobs[h] + if !ok { + return nil, errors.New("missing blob") + } + return []byte(b), nil + } + files := []database.LocalFile{ + {FilePath: "my_pkg/-/dist-tags/latest", ContentHash: "h3"}, + {FilePath: "my_pkg/-/my_pkg-1.0.0.json", ContentHash: "h1"}, + {FilePath: "my_pkg/-/my_pkg-1.0.0.tgz", ContentHash: "nope"}, + {FilePath: "my_pkg/-/my_pkg-1.1.0.json", ContentHash: "h2"}, + {FilePath: "myXpkg/-/myXpkg-6.6.6.json", ContentHash: "hx"}, + } + + doc, err := buildNPMPackument("my_pkg", files, read, "http://reg/api/v1/local/npm/") + if err != nil { + t.Fatal(err) + } + if tags := doc["dist-tags"].(map[string]string); tags["latest"] != "1.1.0" { + t.Errorf("dist-tags = %v", tags) + } + versions := doc["versions"].(map[string]any) + if len(versions) != 2 { + t.Fatalf("versions = %v (LIKE over-match must be dropped)", versions) + } + v1 := versions["1.0.0"].(map[string]any)["dist"].(map[string]any) + if v1["tarball"] != "http://reg/api/v1/local/npm/my_pkg/-/my_pkg-1.0.0.tgz" || v1["shasum"] != "aa" { + t.Errorf("1.0.0 dist = %v", v1) + } + if versions["1.1.0"].(map[string]any)["dist"].(map[string]any)["tarball"] != "http://reg/api/v1/local/npm/my_pkg/-/my_pkg-1.1.0.tgz" { + t.Error("1.1.0 missing rewritten tarball") + } + + if doc, err := buildNPMPackument("absent", files, read, "x/"); err != nil || doc != nil { + t.Errorf("absent package: doc=%v err=%v", doc, err) + } +} diff --git a/internal/api/v1/proxy.go b/internal/api/v1/proxy.go index e77f43e..4378bd7 100644 --- a/internal/api/v1/proxy.go +++ b/internal/api/v1/proxy.go @@ -15,6 +15,7 @@ import ( "git.unkin.net/unkin/artifactapi/internal/proxy" "git.unkin.net/unkin/artifactapi/internal/storage" "git.unkin.net/unkin/artifactapi/internal/virtual" + "git.unkin.net/unkin/artifactapi/pkg/models" ) type ProxyHandler struct { @@ -41,6 +42,7 @@ func (h *ProxyHandler) Routes() chi.Router { r := chi.NewRouter() r.Get("/remote/{remoteName}/*", h.handleProxy) r.Get("/local/{localName}/*", h.handleLocal) + r.Put("/local/{localName}/*", h.handleLocalPut) r.Get("/virtual/{virtualName}/*", h.handleVirtual) return r } @@ -199,6 +201,13 @@ func (h *ProxyHandler) handleLocal(w http.ResponseWriter, r *http.Request) { return } + if remote.PackageType == models.PackageNPM { + if pkg := npmPackageName(path); pkg != "" { + h.serveNPMPackument(w, r, remote, pkg) + return + } + } + prov, _ := provider.Get(remote.PackageType) if indexer, ok := prov.(provider.LocalIndexer); ok { if indexer.ServeLocalIndex(w, r, h.db, remote.Name, path) { diff --git a/ui/src/components/UsageInstructions.tsx b/ui/src/components/UsageInstructions.tsx index 228a924..90c669a 100644 --- a/ui/src/components/UsageInstructions.tsx +++ b/ui/src/components/UsageInstructions.tsx @@ -101,6 +101,27 @@ pip install `, ]; case 'npm': + if (isLocal) { + const reg = `${url}/api/v1/local/${name}/`; + return [ + { + title: 'Configure .npmrc (registry + publish token)', + language: 'bash', + code: `cat >> .npmrc <<'EOF' +registry=${reg} +//${host}/api/v1/local/${name}/:_authToken=unused +EOF`, + note: 'npm refuses to publish without an _authToken for the registry; any value works. For a scope only, use @:registry=… instead of registry=.', + }, + { + title: 'Publish and install', + language: 'bash', + code: `npm publish +npm install `, + note: 'Versions are immutable: republishing an existing version is rejected. The dist-tags sent with the publish (latest, or --tag) move to the new version.', + }, + ]; + } return [ { title: 'Point npm at this registry', -- 2.47.3 From 4747693fa8baefcb838c3af87f44e21b1a5985f0 Mon Sep 17 00:00:00 2001 From: unkin-agent Date: Sat, 17 Oct 2026 12:34:18 +1100 Subject: [PATCH 2/6] Bound npm publish body and make interrupted publishes retryable --- internal/api/v1/npm.go | 65 ++++++--- internal/api/v1/npm_handler_test.go | 201 ++++++++++++++++++++++++++++ internal/api/v1/npm_test.go | 2 + 3 files changed, 248 insertions(+), 20 deletions(-) create mode 100644 internal/api/v1/npm_handler_test.go diff --git a/internal/api/v1/npm.go b/internal/api/v1/npm.go index 253d245..837b79b 100644 --- a/internal/api/v1/npm.go +++ b/internal/api/v1/npm.go @@ -30,8 +30,11 @@ import ( // /-/dist-tags/ version string (mutable) var ( - npmNameRe = regexp.MustCompile(`^(@[a-z0-9][a-z0-9._~-]*/)?[a-z0-9][a-z0-9._~-]*$`) - npmRefRe = regexp.MustCompile(`^[0-9A-Za-z][0-9A-Za-z.+_-]*$`) + npmNameRe = regexp.MustCompile(`^(@[a-z0-9][a-z0-9._~-]*/)?[a-z0-9][a-z0-9._~-]*$`) + npmRefRe = regexp.MustCompile(`^[0-9A-Za-z][0-9A-Za-z.+_-]*$`) + npmSemverRe = regexp.MustCompile(`^(0|[1-9]\d*)\.(0|[1-9]\d*)\.(0|[1-9]\d*)(?:-((?:0|[1-9]\d*|\d*[a-zA-Z-][0-9a-zA-Z-]*)(?:\.(?:0|[1-9]\d*|\d*[a-zA-Z-][0-9a-zA-Z-]*))*))?(?:\+([0-9a-zA-Z-]+(?:\.[0-9a-zA-Z-]+)*))?$`) + + npmMaxPublishBytes int64 = 256 << 20 ) func (h *ProxyHandler) localNPMRemote(r *http.Request, name string) (*models.Remote, bool) { @@ -86,7 +89,7 @@ func parseNPMPublish(body io.Reader, pkg string) (*npmRelease, error) { for v, m := range doc.Versions { rel.version, rel.manifest = v, m } - if !npmRefRe.MatchString(rel.version) { + if !npmSemverRe.MatchString(rel.version) { return nil, fmt.Errorf("invalid version %q", rel.version) } var meta struct { @@ -145,7 +148,7 @@ func verifyNPMDist(tgz []byte, integrity, shasum string) error { func npmError(w http.ResponseWriter, status int, msg string) { w.Header().Set("Content-Type", "application/json") w.WriteHeader(status) - json.NewEncoder(w).Encode(map[string]string{"error": msg}) + _ = json.NewEncoder(w).Encode(map[string]string{"error": msg}) } func (h *ProxyHandler) handleLocalPut(w http.ResponseWriter, r *http.Request) { @@ -159,52 +162,74 @@ func (h *ProxyHandler) handleLocalPut(w http.ResponseWriter, r *http.Request) { npmError(w, http.StatusBadRequest, "invalid package name") return } - rel, err := parseNPMPublish(r.Body, pkg) + rel, err := parseNPMPublish(http.MaxBytesReader(w, r.Body, npmMaxPublishBytes), pkg) if err != nil { + var tooLarge *http.MaxBytesError + if errors.As(err, &tooLarge) { + npmError(w, http.StatusRequestEntityTooLarge, fmt.Sprintf("publish body exceeds %d bytes", npmMaxPublishBytes)) + return + } npmError(w, http.StatusBadRequest, err.Error()) return } base := pkg[strings.LastIndex(pkg, "/")+1:] stem := pkg + "/-/" + base + "-" + rel.version - // The tarball's immutable row is the version's uniqueness gate. - if err := h.npmStore(r.Context(), remote.Name, stem+".tgz", rel.tarball, "application/octet-stream", false); err != nil { - if errors.Is(err, database.ErrAlreadyExists) { - npmError(w, http.StatusConflict, fmt.Sprintf("%s@%s already exists; overwrites are not allowed", pkg, rel.version)) + conflict := func() { + npmError(w, http.StatusConflict, fmt.Sprintf("%s@%s already exists; overwrites are not allowed", pkg, rel.version)) + } + // The manifest gates visibility, so it is written last; a tarball left by a + // failed publish is accepted again only with identical bytes. + hash, err := h.npmStore(r.Context(), remote.Name, stem+".tgz", rel.tarball, "application/octet-stream", false) + if errors.Is(err, database.ErrAlreadyExists) { + existing, getErr := h.db.GetLocalFile(r.Context(), remote.Name, stem+".tgz") + if getErr != nil || existing == nil { + npmError(w, http.StatusInternalServerError, fmt.Sprintf("read existing tarball: %v", getErr)) return } - npmError(w, http.StatusInternalServerError, err.Error()) - return + if existing.ContentHash != hash { + conflict() + return + } + err = nil } - if err := h.npmStore(r.Context(), remote.Name, stem+".json", rel.manifest, "application/json", false); err != nil { + if err != nil { npmError(w, http.StatusInternalServerError, err.Error()) return } for _, tag := range rel.tags { - if err := h.npmStore(r.Context(), remote.Name, pkg+"/-/dist-tags/"+tag, []byte(rel.version), "text/plain", true); err != nil { + if _, err := h.npmStore(r.Context(), remote.Name, pkg+"/-/dist-tags/"+tag, []byte(rel.version), "text/plain", true); err != nil { npmError(w, http.StatusInternalServerError, err.Error()) return } } + if _, err := h.npmStore(r.Context(), remote.Name, stem+".json", rel.manifest, "application/json", false); err != nil { + if errors.Is(err, database.ErrAlreadyExists) { + conflict() + return + } + npmError(w, http.StatusInternalServerError, err.Error()) + return + } slog.Info("local npm package published", "repo", remote.Name, "package", pkg, "version", rel.version) w.Header().Set("Content-Type", "application/json") w.WriteHeader(http.StatusCreated) - json.NewEncoder(w).Encode(map[string]any{"ok": true, "id": pkg}) + _ = json.NewEncoder(w).Encode(map[string]any{"ok": true, "id": pkg}) } -func (h *ProxyHandler) npmStore(ctx context.Context, repo, path string, data []byte, contentType string, mutable bool) error { +func (h *ProxyHandler) npmStore(ctx context.Context, repo, path string, data []byte, contentType string, mutable bool) (string, error) { result, err := h.cas.Store(ctx, bytes.NewReader(data), contentType) if err != nil { - return err + return "", err } if err := h.db.UpsertBlob(ctx, result.ContentHash, result.S3Key, result.SizeBytes, contentType); err != nil { - return err + return "", err } if mutable { - return h.db.UpsertLocalFile(ctx, repo, path, result.ContentHash) + return result.ContentHash, h.db.UpsertLocalFile(ctx, repo, path, result.ContentHash) } - return h.db.CreateLocalFile(ctx, repo, path, result.ContentHash) + return result.ContentHash, h.db.CreateLocalFile(ctx, repo, path, result.ContentHash) } func (h *ProxyHandler) readLocalBlob(ctx context.Context, contentHash string) ([]byte, error) { @@ -236,7 +261,7 @@ func (h *ProxyHandler) serveNPMPackument(w http.ResponseWriter, r *http.Request, w.Header().Set("Content-Type", "application/json") w.Header().Set("X-Artifact-Source", "local") w.WriteHeader(http.StatusOK) - json.NewEncoder(w).Encode(doc) + _ = json.NewEncoder(w).Encode(doc) } // buildNPMPackument returns nil when pkg has no published versions. diff --git a/internal/api/v1/npm_handler_test.go b/internal/api/v1/npm_handler_test.go new file mode 100644 index 0000000..95e91b8 --- /dev/null +++ b/internal/api/v1/npm_handler_test.go @@ -0,0 +1,201 @@ +package v1 + +import ( + "context" + "encoding/json" + "fmt" + "io" + "net/http" + "net/http/httptest" + "os" + "strings" + "sync" + "testing" + "time" + + "github.com/go-chi/chi/v5" + + "git.unkin.net/unkin/artifactapi/internal/database" + "git.unkin.net/unkin/artifactapi/internal/storage" + "git.unkin.net/unkin/artifactapi/internal/testsupport" + "git.unkin.net/unkin/artifactapi/pkg/models" +) + +var ( + testDSN string + testMinio testsupport.MinioConn +) + +func TestMain(m *testing.M) { + ctx := context.Background() + dsn, termPG, err := testsupport.StartPostgres(ctx) + if err != nil { + os.Exit(m.Run()) + } + conn, termMinio, err := testsupport.StartMinio(ctx) + if err != nil { + termPG() + os.Exit(m.Run()) + } + testDSN, testMinio = dsn, conn + code := m.Run() + termMinio() + termPG() + os.Exit(code) +} + +func npmTestServer(t *testing.T) (*ProxyHandler, *httptest.Server) { + t.Helper() + if testDSN == "" { + t.Skip("Docker unavailable") + } + db, err := database.New(testDSN) + if err != nil { + t.Fatal(err) + } + t.Cleanup(db.Close) + var store *storage.S3 + for i := 0; i < 20; i++ { + if store, err = storage.NewS3(testMinio.Endpoint, testMinio.AccessKey, testMinio.SecretKey, "npm", false, ""); err == nil { + break + } + time.Sleep(500 * time.Millisecond) + } + if err != nil { + t.Fatal(err) + } + h := NewProxyHandler(nil, nil, db, store, nil) + r := chi.NewRouter() + r.Mount("/api/v1", h.Routes()) + srv := httptest.NewServer(r) + t.Cleanup(srv.Close) + return h, srv +} + +func npmRepo(t *testing.T, h *ProxyHandler, pt models.PackageType) string { + t.Helper() + name := fmt.Sprintf("npm-%s-%d", pt, time.Now().UnixNano()) + if err := h.db.CreateRemote(context.Background(), &models.Remote{Name: name, PackageType: pt, RepoType: models.RepoTypeLocal}); err != nil { + t.Fatal(err) + } + return name +} + +func npmDo(t *testing.T, method, url, body string) (int, string) { + t.Helper() + req, _ := http.NewRequest(method, url, strings.NewReader(body)) + resp, err := http.DefaultClient.Do(req) + if err != nil { + t.Fatal(err) + } + defer func() { _ = resp.Body.Close() }() + b, _ := io.ReadAll(resp.Body) + return resp.StatusCode, string(b) +} + +func packumentVersions(t *testing.T, url string) map[string]any { + t.Helper() + code, body := npmDo(t, http.MethodGet, url, "") + if code != http.StatusOK { + t.Fatalf("GET %s = %d: %s", url, code, body) + } + var doc struct { + Versions map[string]any `json:"versions"` + } + if err := json.Unmarshal([]byte(body), &doc); err != nil { + t.Fatal(err) + } + return doc.Versions +} + +func TestNPMPublishHandler(t *testing.T) { + h, srv := npmTestServer(t) + repo := npmRepo(t, h, models.PackageNPM) + base := srv.URL + "/api/v1/local/" + repo + "/" + + t.Run("scoped name via %2f", func(t *testing.T) { + body := publishBody(t, "@acme/widget", []byte("scoped tgz"), publishOpts{}) + if code, b := npmDo(t, http.MethodPut, base+"@acme%2fwidget", body); code != http.StatusCreated { + t.Fatalf("publish = %d: %s", code, b) + } + v := packumentVersions(t, base+"@acme%2fwidget") + dist := v["1.0.0"].(map[string]any)["dist"].(map[string]any) + tarball := dist["tarball"].(string) + if !strings.HasPrefix(tarball, base+"@acme/widget/-/widget-1.0.0.tgz") { + t.Fatalf("tarball = %s", tarball) + } + if code, b := npmDo(t, http.MethodGet, tarball, ""); code != http.StatusOK || b != "scoped tgz" { + t.Fatalf("tarball fetch = %d %q", code, b) + } + }) + + t.Run("republish", func(t *testing.T) { + if code, b := npmDo(t, http.MethodPut, base+"repub", publishBody(t, "repub", []byte("v1"), publishOpts{})); code != http.StatusCreated { + t.Fatalf("first publish = %d: %s", code, b) + } + if code, _ := npmDo(t, http.MethodPut, base+"repub", publishBody(t, "repub", []byte("different"), publishOpts{})); code != http.StatusConflict { + t.Errorf("different bytes = %d, want 409", code) + } + if code, _ := npmDo(t, http.MethodPut, base+"repub", publishBody(t, "repub", []byte("v1"), publishOpts{})); code != http.StatusConflict { + t.Errorf("identical complete republish = %d, want 409", code) + } + }) + + t.Run("retry after tarball-only write", func(t *testing.T) { + tgz := []byte("half published") + if _, err := h.npmStore(context.Background(), repo, "retry/-/retry-1.0.0.tgz", tgz, "application/octet-stream", false); err != nil { + t.Fatal(err) + } + if code, _ := npmDo(t, http.MethodGet, base+"retry", ""); code != http.StatusNotFound { + t.Fatalf("tarball-only version visible: %d", code) + } + if code, _ := npmDo(t, http.MethodPut, base+"retry", publishBody(t, "retry", []byte("other bytes"), publishOpts{})); code != http.StatusConflict { + t.Errorf("retry with different bytes = %d, want 409", code) + } + if code, b := npmDo(t, http.MethodPut, base+"retry", publishBody(t, "retry", tgz, publishOpts{})); code != http.StatusCreated { + t.Fatalf("retry with identical bytes = %d: %s", code, b) + } + if _, ok := packumentVersions(t, base+"retry")["1.0.0"]; !ok { + t.Error("retried version missing from packument") + } + }) + + t.Run("concurrent versions", func(t *testing.T) { + var wg sync.WaitGroup + codes := make([]int, 2) + for i, v := range []string{"1.0.0", "2.0.0"} { + wg.Add(1) + go func() { + defer wg.Done() + codes[i], _ = npmDo(t, http.MethodPut, base+"conc", publishBody(t, "conc", []byte("tgz "+v), publishOpts{version: v})) + }() + } + wg.Wait() + if codes[0] != http.StatusCreated || codes[1] != http.StatusCreated { + t.Fatalf("codes = %v", codes) + } + v := packumentVersions(t, base+"conc") + if _, ok := v["1.0.0"]; !ok { + t.Error("1.0.0 missing") + } + if _, ok := v["2.0.0"]; !ok { + t.Error("2.0.0 missing") + } + }) + + t.Run("body too large", func(t *testing.T) { + old := npmMaxPublishBytes + npmMaxPublishBytes = 64 + defer func() { npmMaxPublishBytes = old }() + if code, _ := npmDo(t, http.MethodPut, base+"big", publishBody(t, "big", []byte("x"), publishOpts{})); code != http.StatusRequestEntityTooLarge { + t.Errorf("oversized body = %d, want 413", code) + } + }) + + t.Run("non-npm local", func(t *testing.T) { + generic := npmRepo(t, h, models.PackageGeneric) + if code, _ := npmDo(t, http.MethodPut, srv.URL+"/api/v1/local/"+generic+"/pkg", publishBody(t, "pkg", []byte("x"), publishOpts{})); code != http.StatusMethodNotAllowed { + t.Errorf("publish to generic local = %d, want 405", code) + } + }) +} diff --git a/internal/api/v1/npm_test.go b/internal/api/v1/npm_test.go index d1af157..bd025e5 100644 --- a/internal/api/v1/npm_test.go +++ b/internal/api/v1/npm_test.go @@ -101,6 +101,8 @@ func TestParseNPMPublish(t *testing.T) { "manifest name mismatch": {manifestName: "other"}, "integrity mismatch": {integrity: "sha512-AAAA"}, "path-like version": {version: "../1.0.0"}, + "non-semver version": {version: "1..2"}, + "leading-zero version": {version: "01.0.0"}, "tag at another version": {tags: map[string]string{"latest": "9.9.9"}}, "path-like tag": {tags: map[string]string{"../x": "1.0.0"}}, "multiple versions at once": {extraVersion: true}, -- 2.47.3 From cea1da59f658dbfe5a8d868d3149be9e4355a77a Mon Sep 17 00:00:00 2001 From: unkin-agent Date: Sat, 17 Oct 2026 12:44:39 +1100 Subject: [PATCH 3/6] Write npm dist-tags only after the manifest lands --- e2e-docker/npm_local_test.go | 2 +- internal/api/v1/npm.go | 57 ++++++++------- internal/api/v1/npm_handler_test.go | 107 ++++++++++++++++++++++++---- internal/api/v1/npm_test.go | 4 +- 4 files changed, 131 insertions(+), 39 deletions(-) diff --git a/e2e-docker/npm_local_test.go b/e2e-docker/npm_local_test.go index 9455611..30c873d 100644 --- a/e2e-docker/npm_local_test.go +++ b/e2e-docker/npm_local_test.go @@ -36,7 +36,7 @@ mk() { d="/w/src/$1-$2"; mkdir -p "$d"; cd "$d" echo "module.exports = '$1@$2';" > index.js; } mk e2e-npm-plain 1.0.0 && npm publish mk @e2e/npm-scoped 1.0.0 && npm publish -mk e2e-npm-plain 1.0.0 +mk e2e-npm-plain 1.0.0 && echo "// changed" >> index.js if npm publish 2>/dev/null; then echo "republish accepted"; exit 1; fi mkdir -p /w/app && cd /w/app && npm init -y >/dev/null npm install --no-audit --cache /tmp/fresh-cache e2e-npm-plain @e2e/npm-scoped diff --git a/internal/api/v1/npm.go b/internal/api/v1/npm.go index 837b79b..41cc5e1 100644 --- a/internal/api/v1/npm.go +++ b/internal/api/v1/npm.go @@ -175,27 +175,24 @@ func (h *ProxyHandler) handleLocalPut(w http.ResponseWriter, r *http.Request) { base := pkg[strings.LastIndex(pkg, "/")+1:] stem := pkg + "/-/" + base + "-" + rel.version - conflict := func() { - npmError(w, http.StatusConflict, fmt.Sprintf("%s@%s already exists; overwrites are not allowed", pkg, rel.version)) - } - // The manifest gates visibility, so it is written last; a tarball left by a - // failed publish is accepted again only with identical bytes. - hash, err := h.npmStore(r.Context(), remote.Name, stem+".tgz", rel.tarball, "application/octet-stream", false) - if errors.Is(err, database.ErrAlreadyExists) { - existing, getErr := h.db.GetLocalFile(r.Context(), remote.Name, stem+".tgz") - if getErr != nil || existing == nil { - npmError(w, http.StatusInternalServerError, fmt.Sprintf("read existing tarball: %v", getErr)) + // The manifest gates visibility and tags may only name visible versions, so + // the order is tarball, manifest, tags. Identical bytes make retries idempotent. + for _, f := range []struct { + path, ct string + data []byte + }{ + {stem + ".tgz", "application/octet-stream", rel.tarball}, + {stem + ".json", "application/json", rel.manifest}, + } { + ok, err := h.npmStoreImmutable(r.Context(), remote.Name, f.path, f.data, f.ct) + if err != nil { + npmError(w, http.StatusInternalServerError, err.Error()) return } - if existing.ContentHash != hash { - conflict() + if !ok { + npmError(w, http.StatusConflict, fmt.Sprintf("%s@%s already exists; overwrites are not allowed", pkg, rel.version)) return } - err = nil - } - if err != nil { - npmError(w, http.StatusInternalServerError, err.Error()) - return } for _, tag := range rel.tags { if _, err := h.npmStore(r.Context(), remote.Name, pkg+"/-/dist-tags/"+tag, []byte(rel.version), "text/plain", true); err != nil { @@ -203,14 +200,6 @@ func (h *ProxyHandler) handleLocalPut(w http.ResponseWriter, r *http.Request) { return } } - if _, err := h.npmStore(r.Context(), remote.Name, stem+".json", rel.manifest, "application/json", false); err != nil { - if errors.Is(err, database.ErrAlreadyExists) { - conflict() - return - } - npmError(w, http.StatusInternalServerError, err.Error()) - return - } slog.Info("local npm package published", "repo", remote.Name, "package", pkg, "version", rel.version) w.Header().Set("Content-Type", "application/json") @@ -218,6 +207,19 @@ func (h *ProxyHandler) handleLocalPut(w http.ResponseWriter, r *http.Request) { _ = json.NewEncoder(w).Encode(map[string]any{"ok": true, "id": pkg}) } +// npmStoreImmutable reports false when path already holds different bytes. +func (h *ProxyHandler) npmStoreImmutable(ctx context.Context, repo, path string, data []byte, contentType string) (bool, error) { + hash, err := h.npmStore(ctx, repo, path, data, contentType, false) + if !errors.Is(err, database.ErrAlreadyExists) { + return err == nil, err + } + existing, err := h.db.GetLocalFile(ctx, repo, path) + if err != nil || existing == nil { + return false, fmt.Errorf("read existing %s: %v", path, err) + } + return existing.ContentHash == hash, nil +} + func (h *ProxyHandler) npmStore(ctx context.Context, repo, path string, data []byte, contentType string, mutable bool) (string, error) { result, err := h.cas.Store(ctx, bytes.NewReader(data), contentType) if err != nil { @@ -307,5 +309,10 @@ func buildNPMPackument(pkg string, files []database.LocalFile, read func(hash st if len(versions) == 0 { return nil, nil } + for tag, v := range tags { + if _, ok := versions[v]; !ok { + delete(tags, tag) + } + } return map[string]any{"_id": pkg, "name": pkg, "dist-tags": tags, "versions": versions}, nil } diff --git a/internal/api/v1/npm_handler_test.go b/internal/api/v1/npm_handler_test.go index 95e91b8..70821d6 100644 --- a/internal/api/v1/npm_handler_test.go +++ b/internal/api/v1/npm_handler_test.go @@ -108,6 +108,33 @@ func packumentVersions(t *testing.T, url string) map[string]any { return doc.Versions } +func packumentTags(t *testing.T, url string) map[string]string { + t.Helper() + _, body := npmDo(t, http.MethodGet, url, "") + var doc struct { + Tags map[string]string `json:"dist-tags"` + } + if err := json.Unmarshal([]byte(body), &doc); err != nil { + t.Fatal(err) + } + return doc.Tags +} + +func concurrentPuts(t *testing.T, url string, bodies ...string) []int { + t.Helper() + var wg sync.WaitGroup + codes := make([]int, len(bodies)) + for i, b := range bodies { + wg.Add(1) + go func() { + defer wg.Done() + codes[i], _ = npmDo(t, http.MethodPut, url, b) + }() + } + wg.Wait() + return codes +} + func TestNPMPublishHandler(t *testing.T) { h, srv := npmTestServer(t) repo := npmRepo(t, h, models.PackageNPM) @@ -136,8 +163,71 @@ func TestNPMPublishHandler(t *testing.T) { if code, _ := npmDo(t, http.MethodPut, base+"repub", publishBody(t, "repub", []byte("different"), publishOpts{})); code != http.StatusConflict { t.Errorf("different bytes = %d, want 409", code) } - if code, _ := npmDo(t, http.MethodPut, base+"repub", publishBody(t, "repub", []byte("v1"), publishOpts{})); code != http.StatusConflict { - t.Errorf("identical complete republish = %d, want 409", code) + if code, _ := npmDo(t, http.MethodPut, base+"repub", publishBody(t, "repub", []byte("v1"), publishOpts{})); code != http.StatusCreated { + t.Errorf("identical republish = %d, want 201", code) + } + }) + + t.Run("rejected republish leaves tags", func(t *testing.T) { + for _, v := range []string{"1.0.0", "1.1.0"} { + if code, b := npmDo(t, http.MethodPut, base+"rej", publishBody(t, "rej", []byte("tgz "+v), publishOpts{version: v})); code != http.StatusCreated { + t.Fatalf("publish %s = %d: %s", v, code, b) + } + } + if code, _ := npmDo(t, http.MethodPut, base+"rej", publishBody(t, "rej", []byte("tgz 1.0.0"), publishOpts{shasum: "x"})); code != http.StatusConflict { + t.Fatalf("re-PUT 1.0.0 = %d, want 409", code) + } + if tags := packumentTags(t, base+"rej"); tags["latest"] != "1.1.0" { + t.Errorf("latest = %q, want 1.1.0", tags["latest"]) + } + }) + + t.Run("mid-publish window", func(t *testing.T) { + if code, b := npmDo(t, http.MethodPut, base+"mid", publishBody(t, "mid", []byte("v1"), publishOpts{})); code != http.StatusCreated { + t.Fatalf("publish = %d: %s", code, b) + } + ctx := context.Background() + if _, err := h.npmStore(ctx, repo, "mid/-/mid-1.0.1.tgz", []byte("v2"), "application/octet-stream", false); err != nil { + t.Fatal(err) + } + if tags := packumentTags(t, base+"mid"); tags["latest"] != "1.0.0" { + t.Errorf("tarball-only 1.0.1: latest = %q", tags["latest"]) + } + if _, err := h.npmStore(ctx, repo, "mid/-/dist-tags/latest", []byte("1.0.1"), "text/plain", true); err != nil { + t.Fatal(err) + } + if tags := packumentTags(t, base+"mid"); tags["latest"] == "1.0.1" { + t.Error("latest points at invisible 1.0.1") + } + }) + + t.Run("concurrent identical", func(t *testing.T) { + body := publishBody(t, "same", []byte("same tgz"), publishOpts{}) + codes := concurrentPuts(t, base+"same", body, body) + if codes[0] != http.StatusCreated || codes[1] != http.StatusCreated { + t.Fatalf("codes = %v", codes) + } + }) + + t.Run("concurrent different manifests", func(t *testing.T) { + tgz := []byte("race tgz") + codes := concurrentPuts(t, base+"race", + publishBody(t, "race", tgz, publishOpts{shasum: "a", tags: map[string]string{"a": "1.0.0"}}), + publishBody(t, "race", tgz, publishOpts{shasum: "b", tags: map[string]string{"b": "1.0.0"}})) + won := map[int]string{} + for i, c := range codes { + won[c] = []string{"a", "b"}[i] + } + if len(won) != 2 || won[http.StatusCreated] == "" || won[http.StatusConflict] == "" { + t.Fatalf("codes = %v, want one 201 and one 409", codes) + } + v := packumentVersions(t, base+"race")["1.0.0"].(map[string]any) + if got := v["dist"].(map[string]any)["shasum"]; got != won[http.StatusCreated] { + t.Errorf("manifest shasum = %v, winner %s", got, won[http.StatusCreated]) + } + tags := packumentTags(t, base+"race") + if len(tags) != 1 || tags[won[http.StatusCreated]] != "1.0.0" { + t.Errorf("tags = %v, winner %s", tags, won[http.StatusCreated]) } }) @@ -161,16 +251,9 @@ func TestNPMPublishHandler(t *testing.T) { }) t.Run("concurrent versions", func(t *testing.T) { - var wg sync.WaitGroup - codes := make([]int, 2) - for i, v := range []string{"1.0.0", "2.0.0"} { - wg.Add(1) - go func() { - defer wg.Done() - codes[i], _ = npmDo(t, http.MethodPut, base+"conc", publishBody(t, "conc", []byte("tgz "+v), publishOpts{version: v})) - }() - } - wg.Wait() + codes := concurrentPuts(t, base+"conc", + publishBody(t, "conc", []byte("tgz 1.0.0"), publishOpts{version: "1.0.0"}), + publishBody(t, "conc", []byte("tgz 2.0.0"), publishOpts{version: "2.0.0"})) if codes[0] != http.StatusCreated || codes[1] != http.StatusCreated { t.Fatalf("codes = %v", codes) } diff --git a/internal/api/v1/npm_test.go b/internal/api/v1/npm_test.go index bd025e5..82cf676 100644 --- a/internal/api/v1/npm_test.go +++ b/internal/api/v1/npm_test.go @@ -122,6 +122,7 @@ func TestBuildNPMPackument(t *testing.T) { "h1": `{"name":"my_pkg","version":"1.0.0","dist":{"shasum":"aa","tarball":"http://elsewhere/x.tgz"}}`, "h2": `{"name":"my_pkg","version":"1.1.0"}`, "h3": "1.1.0", + "h4": "2.0.0", "hx": `{"name":"myXpkg","version":"6.6.6"}`, } read := func(h string) ([]byte, error) { @@ -133,6 +134,7 @@ func TestBuildNPMPackument(t *testing.T) { } files := []database.LocalFile{ {FilePath: "my_pkg/-/dist-tags/latest", ContentHash: "h3"}, + {FilePath: "my_pkg/-/dist-tags/next", ContentHash: "h4"}, {FilePath: "my_pkg/-/my_pkg-1.0.0.json", ContentHash: "h1"}, {FilePath: "my_pkg/-/my_pkg-1.0.0.tgz", ContentHash: "nope"}, {FilePath: "my_pkg/-/my_pkg-1.1.0.json", ContentHash: "h2"}, @@ -143,7 +145,7 @@ func TestBuildNPMPackument(t *testing.T) { if err != nil { t.Fatal(err) } - if tags := doc["dist-tags"].(map[string]string); tags["latest"] != "1.1.0" { + if tags := doc["dist-tags"].(map[string]string); tags["latest"] != "1.1.0" || len(tags) != 1 { t.Errorf("dist-tags = %v", tags) } versions := doc["versions"].(map[string]any) -- 2.47.3 From 76d108d510bafc2ad7d4af629d9130f14161fc7f Mon Sep 17 00:00:00 2001 From: unkin-agent Date: Sat, 17 Oct 2026 12:51:54 +1100 Subject: [PATCH 4/6] Reject npm publish over an existing version --- e2e-docker/npm_local_test.go | 2 +- internal/api/v1/npm.go | 34 ++++++++++++++++------------- internal/api/v1/npm_handler_test.go | 17 +++++++++++---- 3 files changed, 33 insertions(+), 20 deletions(-) diff --git a/e2e-docker/npm_local_test.go b/e2e-docker/npm_local_test.go index 30c873d..9455611 100644 --- a/e2e-docker/npm_local_test.go +++ b/e2e-docker/npm_local_test.go @@ -36,7 +36,7 @@ mk() { d="/w/src/$1-$2"; mkdir -p "$d"; cd "$d" echo "module.exports = '$1@$2';" > index.js; } mk e2e-npm-plain 1.0.0 && npm publish mk @e2e/npm-scoped 1.0.0 && npm publish -mk e2e-npm-plain 1.0.0 && echo "// changed" >> index.js +mk e2e-npm-plain 1.0.0 if npm publish 2>/dev/null; then echo "republish accepted"; exit 1; fi mkdir -p /w/app && cd /w/app && npm init -y >/dev/null npm install --no-audit --cache /tmp/fresh-cache e2e-npm-plain @e2e/npm-scoped diff --git a/internal/api/v1/npm.go b/internal/api/v1/npm.go index 41cc5e1..18604cf 100644 --- a/internal/api/v1/npm.go +++ b/internal/api/v1/npm.go @@ -176,23 +176,27 @@ func (h *ProxyHandler) handleLocalPut(w http.ResponseWriter, r *http.Request) { base := pkg[strings.LastIndex(pkg, "/")+1:] stem := pkg + "/-/" + base + "-" + rel.version // The manifest gates visibility and tags may only name visible versions, so - // the order is tarball, manifest, tags. Identical bytes make retries idempotent. - for _, f := range []struct { - path, ct string - data []byte - }{ - {stem + ".tgz", "application/octet-stream", rel.tarball}, - {stem + ".json", "application/json", rel.manifest}, - } { - ok, err := h.npmStoreImmutable(r.Context(), remote.Name, f.path, f.data, f.ct) - if err != nil { - npmError(w, http.StatusInternalServerError, err.Error()) - return - } - if !ok { - npmError(w, http.StatusConflict, fmt.Sprintf("%s@%s already exists; overwrites are not allowed", pkg, rel.version)) + // the order is tarball, manifest, tags. A tarball left by a failed publish is + // accepted again only with identical bytes; an existing manifest always conflicts. + conflict := func() { + npmError(w, http.StatusConflict, fmt.Sprintf("%s@%s already exists; overwrites are not allowed", pkg, rel.version)) + } + ok, err = h.npmStoreImmutable(r.Context(), remote.Name, stem+".tgz", rel.tarball, "application/octet-stream") + if err != nil { + npmError(w, http.StatusInternalServerError, err.Error()) + return + } + if !ok { + conflict() + return + } + if _, err := h.npmStore(r.Context(), remote.Name, stem+".json", rel.manifest, "application/json", false); err != nil { + if errors.Is(err, database.ErrAlreadyExists) { + conflict() return } + npmError(w, http.StatusInternalServerError, err.Error()) + return } for _, tag := range rel.tags { if _, err := h.npmStore(r.Context(), remote.Name, pkg+"/-/dist-tags/"+tag, []byte(rel.version), "text/plain", true); err != nil { diff --git a/internal/api/v1/npm_handler_test.go b/internal/api/v1/npm_handler_test.go index 70821d6..8b21c09 100644 --- a/internal/api/v1/npm_handler_test.go +++ b/internal/api/v1/npm_handler_test.go @@ -163,8 +163,14 @@ func TestNPMPublishHandler(t *testing.T) { if code, _ := npmDo(t, http.MethodPut, base+"repub", publishBody(t, "repub", []byte("different"), publishOpts{})); code != http.StatusConflict { t.Errorf("different bytes = %d, want 409", code) } - if code, _ := npmDo(t, http.MethodPut, base+"repub", publishBody(t, "repub", []byte("v1"), publishOpts{})); code != http.StatusCreated { - t.Errorf("identical republish = %d, want 201", code) + if code, b := npmDo(t, http.MethodPut, base+"repub", publishBody(t, "repub", []byte("v2"), publishOpts{version: "1.1.0"})); code != http.StatusCreated { + t.Fatalf("publish 1.1.0 = %d: %s", code, b) + } + if code, _ := npmDo(t, http.MethodPut, base+"repub", publishBody(t, "repub", []byte("v1"), publishOpts{})); code != http.StatusConflict { + t.Errorf("identical republish = %d, want 409", code) + } + if tags := packumentTags(t, base+"repub"); tags["latest"] != "1.1.0" { + t.Errorf("latest = %q, want 1.1.0", tags["latest"]) } }) @@ -204,8 +210,11 @@ func TestNPMPublishHandler(t *testing.T) { t.Run("concurrent identical", func(t *testing.T) { body := publishBody(t, "same", []byte("same tgz"), publishOpts{}) codes := concurrentPuts(t, base+"same", body, body) - if codes[0] != http.StatusCreated || codes[1] != http.StatusCreated { - t.Fatalf("codes = %v", codes) + if codes[0]+codes[1] != http.StatusCreated+http.StatusConflict || codes[0] == codes[1] { + t.Fatalf("codes = %v, want one 201 and one 409", codes) + } + if tags := packumentTags(t, base+"same"); len(tags) != 1 || tags["latest"] != "1.0.0" { + t.Errorf("tags = %v", tags) } }) -- 2.47.3 From 2c200095994938136529f83efef29caf5285c8c0 Mon Sep 17 00:00:00 2001 From: unkin-agent Date: Sat, 17 Oct 2026 17:00:45 +1100 Subject: [PATCH 5/6] Add npm virtual repositories --- README.md | 2 +- docs/npm/virtual.md | 30 +++ e2e-docker/README.md | 3 +- e2e-docker/npm_virtual_test.go | 68 +++++++ internal/api/v1/npm_virtual.go | 26 +++ internal/api/v1/proxy.go | 10 +- internal/server/server.go | 1 + internal/virtual/engine.go | 20 +- internal/virtual/npm_merger.go | 267 +++++++++++++++++++++++++ internal/virtual/npm_merger_test.go | 289 ++++++++++++++++++++++++++++ 10 files changed, 709 insertions(+), 7 deletions(-) create mode 100644 docs/npm/virtual.md create mode 100644 e2e-docker/npm_virtual_test.go create mode 100644 internal/api/v1/npm_virtual.go create mode 100644 internal/virtual/npm_merger.go create mode 100644 internal/virtual/npm_merger_test.go diff --git a/README.md b/README.md index c2984e1..cefeede 100644 --- a/README.md +++ b/README.md @@ -33,7 +33,7 @@ Each type's paths, Terraform examples and local/virtual behaviour are in `docs/< - `generic`: [remote](docs/generic/remote.md) - `goproxy`: [remote](docs/goproxy/remote.md) - `helm`: [remote](docs/helm/remote.md), [local](docs/helm/local.md), [virtual](docs/helm/virtual.md) -- `npm`: [remote](docs/npm/remote.md), [local](docs/npm/local.md) +- `npm`: [remote](docs/npm/remote.md), [local](docs/npm/local.md), [virtual](docs/npm/virtual.md) - `nuget`: [remote](docs/nuget/remote.md) - `puppet`: [remote](docs/puppet/remote.md), [local](docs/puppet/local.md) - `pypi`: [remote](docs/pypi/remote.md) diff --git a/docs/npm/virtual.md b/docs/npm/virtual.md new file mode 100644 index 0000000..19deb9a --- /dev/null +++ b/docs/npm/virtual.md @@ -0,0 +1,30 @@ +# `npm` virtual + +An npm virtual serves one registry over local and remote npm members: +`npm install` and `npm view` work against `/api/v1/virtual//`. + +- Versions are the union across members; on a duplicate version the earlier + member wins. +- Each dist-tag comes from the first member defining it, and is dropped if it + names a version missing from the merged set. +- Tarball URLs point at the virtual (`/`), which 302s to the + owning member. +- Full and abbreviated (`application/vnd.npm.install-v1+json`) packuments are + both served. +- A member error fails the request with 502; a member without the package is + skipped. Merges are cached for 60s per package. + +```hcl +resource "artifactapi_virtual" "npm" { + name = "npm" + package_type = "npm" + members = [artifactapi_local_npm.internal.name, artifactapi_remote_npm.npmjs.name] +} +``` + +```ini +# .npmrc +registry=https://artifactapi.example.com/api/v1/virtual/npm/ +``` + +Publish to the local member directly; the virtual is read-only. diff --git a/e2e-docker/README.md b/e2e-docker/README.md index 85364c2..35816cc 100644 --- a/e2e-docker/README.md +++ b/e2e-docker/README.md @@ -31,7 +31,8 @@ already-running stack. `node:22` client: `npm publish` unscoped + scoped, `npm install` + `require()`, second version, republish rejected). - **Virtual repositories** — pypi simple-index merge and helm `index.yaml` merge - across two members. + across two members; npm packument merge over two locals via a stock `node:22` + client (merged versions, install through the member redirect). - **Mirrorlist** — an rpm remote with a `mirrorlist` of extra upstream mirrors (pool = `base_url` + `mirrorlist`): round-robin distribution across both mirrors (constant-body `mockupstreama` / `mockupstreamb`), failover past a dead primary, diff --git a/e2e-docker/npm_virtual_test.go b/e2e-docker/npm_virtual_test.go new file mode 100644 index 0000000..31f92b0 --- /dev/null +++ b/e2e-docker/npm_virtual_test.go @@ -0,0 +1,68 @@ +//go:build dockere2e + +package e2edocker + +import ( + "fmt" + "os" + "os/exec" + "strings" + "testing" + "time" +) + +// TestVirtualNPMInstall publishes to two local npm repos and drives a stock +// npm client through a virtual over both: merged versions and dist-tags, and +// an install whose tarballs resolve via the virtual's member redirect. +func TestVirtualNPMInstall(t *testing.T) { + network := os.Getenv("COMPOSE_NETWORK") + internal := os.Getenv("ARTIFACTAPI_INTERNAL") + if network == "" || internal == "" { + t.Skip("COMPOSE_NETWORK/ARTIFACTAPI_INTERNAL not set; run via scripts/docker-e2e.sh") + } + if _, err := exec.LookPath("docker"); err != nil { + t.Skip("docker not available on the test host") + } + + id := time.Now().UnixNano() + a, b, virt := fmt.Sprintf("e2e-npm-a-%d", id), fmt.Sprintf("e2e-npm-b-%d", id), fmt.Sprintf("e2e-npm-v-%d", id) + for _, name := range []string{a, b} { + createRepo(t, fmt.Sprintf(`{"name":%q,"package_type":"npm","repo_type":"local"}`, name)) + defer deleteRepo(t, name) + } + createVirtual(t, fmt.Sprintf(`{"name":%q,"package_type":"npm","members":[%q,%q]}`, virt, a, b)) + defer deleteVirtual(t, virt) + + base := strings.TrimRight(internal, "/") + script := `set -euo pipefail +pub() { d="/w/src/$2-$3-$1"; mkdir -p "$d"; cd "$d" + printf '{"name":"%s","version":"%s","main":"index.js"}' "$2" "$3" > package.json + echo "module.exports = '$1:$2@$3';" > index.js + printf 'registry=%s\n%s:_authToken=unused\n' "$BASE/api/v1/local/$1/" "${BASE#http:}/api/v1/local/$1/" > .npmrc + npm publish; } +pub "$A" @e2e/virt 1.0.0 +pub "$B" @e2e/virt 1.0.0 +pub "$B" @e2e/virt 1.1.0 +mkdir -p /w/app && cd /w/app && npm init -y >/dev/null +echo "registry=$BASE/api/v1/virtual/$V/" > .npmrc +echo "VERSIONS $(npm view @e2e/virt versions --json | tr -d ' \n')" +npm install --no-audit --cache /tmp/fresh-cache @e2e/virt@1.0.0 +node -e "console.log('REQ', require('@e2e/virt'))" +echo "RESOLVED $(node -p "require('./package-lock.json').packages['node_modules/@e2e/virt'].resolved")"` + + out, err := exec.Command("docker", "run", "--rm", "--network", network, + "-e", "BASE="+base, "-e", "A="+a, "-e", "B="+b, "-e", "V="+virt, + "node:22", "bash", "-c", script).CombinedOutput() + if err != nil { + t.Fatalf("npm client run failed: %v\n%s", err, out) + } + for _, want := range []string{ + `VERSIONS ["1.0.0","1.1.0"]`, + "REQ " + a + ":@e2e/virt@1.0.0", + "RESOLVED " + base + "/api/v1/virtual/" + virt + "/" + a + "/@e2e/virt/-/virt-1.0.0.tgz", + } { + if !strings.Contains(string(out), want) { + t.Fatalf("missing %q in npm output:\n%s", want, out) + } + } +} diff --git a/internal/api/v1/npm_virtual.go b/internal/api/v1/npm_virtual.go new file mode 100644 index 0000000..7ac006b --- /dev/null +++ b/internal/api/v1/npm_virtual.go @@ -0,0 +1,26 @@ +package v1 + +import ( + "context" + "encoding/json" + + "git.unkin.net/unkin/artifactapi/internal/virtual" +) + +// LocalNPMPackument serves a local npm repo's packument to npm virtuals, with +// tarball paths relative to the repo. +func (h *ProxyHandler) LocalNPMPackument(ctx context.Context, repo, pkg string) ([]byte, error) { + files, err := h.db.ListLocalFilesByPrefix(ctx, repo, pkg+"/-/") + if err != nil { + return nil, err + } + read := func(hash string) ([]byte, error) { return h.readLocalBlob(ctx, hash) } + doc, err := buildNPMPackument(pkg, files, read, "") + if err != nil { + return nil, err + } + if doc == nil { + return nil, virtual.ErrNotFound + } + return json.Marshal(doc) +} diff --git a/internal/api/v1/proxy.go b/internal/api/v1/proxy.go index 27395d0..2ff961b 100644 --- a/internal/api/v1/proxy.go +++ b/internal/api/v1/proxy.go @@ -7,6 +7,7 @@ import ( "io" "log/slog" "net/http" + "strings" "github.com/go-chi/chi/v5" @@ -186,7 +187,14 @@ func (h *ProxyHandler) handleVirtual(w http.ResponseWriter, r *http.Request) { return } - body, contentType, err := h.virtualEngine.Fetch(r.Context(), *virt, path, proxyBaseURL) + var body []byte + var contentType string + if virt.PackageType == models.PackageNPM { + abbreviated := strings.Contains(r.Header.Get("Accept"), virtual.NPMAbbreviatedType) + body, contentType, err = h.virtualEngine.FetchNPM(r.Context(), *virt, path, proxyBaseURL, abbreviated) + } else { + body, contentType, err = h.virtualEngine.Fetch(r.Context(), *virt, path, proxyBaseURL) + } if errors.Is(err, virtual.ErrNotFound) { http.Error(w, "not found", http.StatusNotFound) return diff --git a/internal/server/server.go b/internal/server/server.go index bb61795..b9d8a35 100644 --- a/internal/server/server.go +++ b/internal/server/server.go @@ -166,6 +166,7 @@ func (s *Server) routes() chi.Router { r.Mount(tfregistry.MountPath, s.tfRegistry.Routes()) proxyHandler := v1.NewProxyHandler(s.engine, s.virtEngine, s.db, s.store, s.localHandler) + s.virtEngine.NPMLocal = proxyHandler.LocalNPMPackument r.Mount("/api/v1", proxyHandler.Routes()) r.Mount("/v2", proxyHandler.DockerV2Routes()) diff --git a/internal/virtual/engine.go b/internal/virtual/engine.go index 1f8bf7c..0ac8687 100644 --- a/internal/virtual/engine.go +++ b/internal/virtual/engine.go @@ -26,6 +26,11 @@ type Engine struct { proxyEngine *proxy.Engine getRemote func(context.Context, string) (*models.Remote, error) rpmMember func(context.Context, string) (*RPMMember, error) + npmMember func(ctx context.Context, name, pkg string) (*NPMMember, error) + + // NPMLocal returns a local npm repo's packument with tarball paths relative + // to the repo, or ErrNotFound. + NPMLocal func(ctx context.Context, repo, pkg string) ([]byte, error) mergeTTL time.Duration @@ -55,6 +60,7 @@ const rpmMergeTTL = 60 * time.Second func NewEngine(db *database.DB, proxyEngine *proxy.Engine) *Engine { e := &Engine{db: db, proxyEngine: proxyEngine, getRemote: db.GetRemote, mergeTTL: rpmMergeTTL} e.rpmMember = e.fetchRPMMember + e.npmMember = e.fetchNPMMember return e } @@ -179,11 +185,17 @@ var ( ErrBadPath = errors.New("invalid path") ) -// MemberRedirect maps an rpm virtual package path (/, as written -// by MergeRPM) to the owning member's route. ok is false for any other path; -// ErrBadPath means the path is absolute or escapes the member. +// MemberRedirect maps an rpm or npm virtual package path (/, as +// written by MergeRPM/MergeNPM) to the owning member's route. ok is false for +// any other path; ErrBadPath means the path is absolute or escapes the member. func (e *Engine) MemberRedirect(ctx context.Context, virt models.Virtual, path, rawQuery, proxyBaseURL string) (string, bool, error) { - if virt.PackageType != models.PackageRPM || strings.HasPrefix(path, "repodata/") { + switch virt.PackageType { + case models.PackageRPM: + if strings.HasPrefix(path, "repodata/") { + return "", false, nil + } + case models.PackageNPM: + default: return "", false, nil } name, rest, found := strings.Cut(path, "/") diff --git a/internal/virtual/npm_merger.go b/internal/virtual/npm_merger.go new file mode 100644 index 0000000..3c19fc9 --- /dev/null +++ b/internal/virtual/npm_merger.go @@ -0,0 +1,267 @@ +package virtual + +import ( + "context" + "encoding/json" + "errors" + "fmt" + "net/http" + "net/url" + "regexp" + "slices" + "strings" + + "git.unkin.net/unkin/artifactapi/internal/proxy" + "git.unkin.net/unkin/artifactapi/pkg/models" +) + +const ( + NPMAbbreviatedType = "application/vnd.npm.install-v1+json" + npmAbbreviatedFile = "install-v1" + maxNPMPackuments = 1024 +) + +var npmVirtualNameRe = regexp.MustCompile(`^(@[A-Za-z0-9][A-Za-z0-9._~-]*/)?[A-Za-z0-9][A-Za-z0-9._~-]*$`) + +// npmAbbreviatedFields are the version fields kept in an install-v1 packument. +var npmAbbreviatedFields = []string{ + "name", "version", "deprecated", "dependencies", "optionalDependencies", + "devDependencies", "bundleDependencies", "peerDependencies", "peerDependenciesMeta", + "bin", "directories", "dist", "engines", "_hasShrinkwrap", "hasInstallScript", + "cpu", "os", "libc", +} + +// NPMMember is one member's full packument for a package; Doc is nil when the +// member does not have the package. Tarball URLs under one of Bases are +// rewritten to go via the virtual; with no Bases (a local) they are relative. +type NPMMember struct { + RemoteName string + Bases []string + Doc []byte +} + +// NPMPackument is a merged packument in full and install-v1 form. +type NPMPackument struct { + Full, Abbreviated []byte +} + +func (p *NPMPackument) index() []byte { return p.Full } +func (p *NPMPackument) files() map[string][]byte { + return map[string][]byte{npmAbbreviatedFile: p.Abbreviated} +} + +// MergeNPM unions versions across members (first member wins a duplicate), +// keeps the first member's value for each dist-tag, drops tags naming versions +// outside the merged set and points every owned tarball at +// tarballBase + /. ErrNotFound means no member has the package. +func MergeNPM(members []NPMMember, tarballBase string) (*NPMPackument, error) { + var base map[string]json.RawMessage + versions := map[string]map[string]json.RawMessage{} + tags := map[string]string{} + times := map[string]json.RawMessage{} + var modified string + for _, m := range members { + if m.Doc == nil { + continue + } + var doc map[string]json.RawMessage + var vs map[string]map[string]json.RawMessage + var dt map[string]string + var tm map[string]json.RawMessage + if err := json.Unmarshal(m.Doc, &doc); err != nil { + return nil, fmt.Errorf("member %q packument: %w", m.RemoteName, err) + } + for field, dst := range map[string]any{"versions": &vs, "dist-tags": &dt, "time": &tm} { + if raw, ok := doc[field]; ok { + if err := json.Unmarshal(raw, dst); err != nil { + return nil, fmt.Errorf("member %q %s: %w", m.RemoteName, field, err) + } + } + } + if base == nil { + base = doc + } + for v, man := range vs { + if _, dup := versions[v]; dup || man == nil { + continue + } + if err := rewriteNPMTarball(man, m, tarballBase); err != nil { + return nil, fmt.Errorf("member %q %s: %w", m.RemoteName, v, err) + } + versions[v] = man + } + for tag, v := range dt { + if _, ok := tags[tag]; !ok { + tags[tag] = v + } + } + for k, v := range tm { + if k == "modified" { + var s string + if json.Unmarshal(v, &s) == nil && s > modified { + modified = s + } + continue + } + if _, ok := times[k]; !ok { + times[k] = v + } + } + } + if base == nil { + return nil, ErrNotFound + } + for tag, v := range tags { + if _, ok := versions[v]; !ok { + delete(tags, tag) + } + } + if modified != "" { + times["modified"], _ = json.Marshal(modified) + } + + full := map[string]any{} + for k, v := range base { + full[k] = v + } + full["versions"], full["dist-tags"] = versions, tags + delete(full, "time") + if len(times) > 0 { + full["time"] = times + } + + abbrevVersions := make(map[string]map[string]json.RawMessage, len(versions)) + for v, man := range versions { + a := map[string]json.RawMessage{} + for _, f := range npmAbbreviatedFields { + if raw, ok := man[f]; ok { + a[f] = raw + } + } + abbrevVersions[v] = a + } + abbrev := map[string]any{"name": base["name"], "dist-tags": tags, "versions": abbrevVersions} + if modified != "" { + abbrev["modified"] = modified + } + + fullJSON, err := json.Marshal(full) + if err != nil { + return nil, err + } + abbrevJSON, err := json.Marshal(abbrev) + if err != nil { + return nil, err + } + return &NPMPackument{Full: fullJSON, Abbreviated: abbrevJSON}, nil +} + +// rewriteNPMTarball leaves a tarball outside the member's bases untouched. +func rewriteNPMTarball(man map[string]json.RawMessage, m NPMMember, tarballBase string) error { + raw, ok := man["dist"] + if !ok { + return nil + } + var dist map[string]json.RawMessage + if err := json.Unmarshal(raw, &dist); err != nil { + return fmt.Errorf("dist: %w", err) + } + var tarball string + if err := json.Unmarshal(dist["tarball"], &tarball); err != nil { + return nil + } + rel, ok := npmMemberPath(tarball, m.Bases) + if !ok { + return nil + } + dist["tarball"], _ = json.Marshal(tarballBase + url.PathEscape(m.RemoteName) + "/" + rel) + man["dist"], _ = json.Marshal(dist) + return nil +} + +func npmMemberPath(tarball string, bases []string) (string, bool) { + if len(bases) == 0 { + return tarball, tarball != "" && !strings.Contains(tarball, "://") && !strings.HasPrefix(tarball, "/") + } + for _, b := range bases { + if rel, ok := strings.CutPrefix(tarball, strings.TrimRight(b, "/")+"/"); ok && rel != "" { + return rel, true + } + } + return "", false +} + +// FetchNPM serves the merged packument for the package named by path, full or +// abbreviated (install-v1). +func (e *Engine) FetchNPM(ctx context.Context, virt models.Virtual, path, proxyBaseURL string, abbreviated bool) ([]byte, string, error) { + pkg, err := url.PathUnescape(strings.TrimSuffix(path, "/")) + if err != nil || !npmVirtualNameRe.MatchString(pkg) { + return nil, "", ErrNotFound + } + prefix := "npm\x00" + virt.Name + "\x00" + tarballBase := strings.TrimRight(proxyBaseURL, "/") + "/api/v1/virtual/" + url.PathEscape(virt.Name) + "/" + repo, err := e.merged(ctx, prefix+pkg+"\x00"+proxyBaseURL, func(ctx context.Context) (mergedRepo, error) { + members, err := fetchMembers(virt, func(name string) (*NPMMember, error) { return e.npmMember(ctx, name, pkg) }) + if err != nil { + return nil, err + } + p, err := MergeNPM(members, tarballBase) + if err != nil { + return nil, err + } + return p, nil + }) + if err != nil { + return nil, "", err + } + e.evictOldest(prefix, maxNPMPackuments) + if abbreviated { + return repo.files()[npmAbbreviatedFile], NPMAbbreviatedType, nil + } + return repo.index(), "application/json", nil +} + +// fetchNPMMember returns a member with a nil Doc when it lacks the package. +func (e *Engine) fetchNPMMember(ctx context.Context, name, pkg string) (*NPMMember, error) { + remote, err := e.getRemote(ctx, name) + if err != nil { + return nil, fmt.Errorf("remote %q: %w", name, err) + } + m := &NPMMember{RemoteName: name} + if remote.RepoType == models.RepoTypeLocal { + if e.NPMLocal == nil { + return nil, errors.New("local npm packuments are not wired") + } + m.Doc, err = e.NPMLocal(ctx, name, pkg) + } else { + m.Bases = remote.UpstreamPool() + m.Doc, err = e.fetchMemberPath(ctx, *remote, strings.Replace(pkg, "/", "%2f", 1)) + } + var pe *proxy.ProxyError + if errors.Is(err, ErrNotFound) || errors.As(err, &pe) && pe.Status == http.StatusNotFound { + return &NPMMember{RemoteName: name}, nil + } + if err != nil { + return nil, err + } + return m, nil +} + +// evictOldest drops the oldest merges under prefix beyond keep. +func (e *Engine) evictOldest(prefix string, keep int) { + e.mu.Lock() + defer e.mu.Unlock() + var keys []string + for k := range e.gens { + if strings.HasPrefix(k, prefix) { + keys = append(keys, k) + } + } + if len(keys) <= keep { + return + } + slices.SortFunc(keys, func(a, b string) int { return e.gens[b].at.Compare(e.gens[a].at) }) + for _, k := range keys[keep:] { + delete(e.gens, k) + } +} diff --git a/internal/virtual/npm_merger_test.go b/internal/virtual/npm_merger_test.go new file mode 100644 index 0000000..9536e0d --- /dev/null +++ b/internal/virtual/npm_merger_test.go @@ -0,0 +1,289 @@ +package virtual + +import ( + "context" + "encoding/json" + "errors" + "fmt" + "strings" + "sync" + "testing" + "time" + + "git.unkin.net/unkin/artifactapi/pkg/models" +) + +type npmDoc struct { + Name string `json:"name"` + Readme string `json:"readme,omitempty"` + Modified string `json:"modified,omitempty"` + DistTags map[string]string `json:"dist-tags"` + Versions map[string]map[string]any `json:"versions"` + Time map[string]string `json:"time,omitempty"` +} + +func npmVersion(name, v, tarball string) map[string]any { + return map[string]any{"name": name, "version": v, "readme": "long", "scripts": map[string]string{"test": "x"}, + "dependencies": map[string]string{"dep": "^1"}, "dist": map[string]any{"tarball": tarball, "shasum": "abc"}} +} + +func mustJSON(t *testing.T, v any) []byte { + t.Helper() + b, err := json.Marshal(v) + if err != nil { + t.Fatal(err) + } + return b +} + +func decodeNPM(t *testing.T, b []byte) npmDoc { + t.Helper() + var d npmDoc + if err := json.Unmarshal(b, &d); err != nil { + t.Fatalf("decode %s: %v", b, err) + } + return d +} + +func tarballOf(d npmDoc, v string) string { + dist, _ := d.Versions[v]["dist"].(map[string]any) + s, _ := dist["tarball"].(string) + return s +} + +func npmMembersFixture(t *testing.T) []NPMMember { + local := npmDoc{Name: "pkg", DistTags: map[string]string{"latest": "1.0.0", "beta": "9.9.9"}, + Versions: map[string]map[string]any{"1.0.0": npmVersion("pkg", "1.0.0", "pkg/-/pkg-1.0.0.tgz")}} + remote := npmDoc{Name: "pkg", Readme: "remote readme", + DistTags: map[string]string{"latest": "2.0.0", "next": "2.0.0", "beta": "1.0.0"}, + Versions: map[string]map[string]any{ + "1.0.0": npmVersion("pkg", "1.0.0", "https://registry.npmjs.org/pkg/-/pkg-1.0.0.tgz"), + "2.0.0": npmVersion("pkg", "2.0.0", "https://registry.npmjs.org/pkg/-/pkg-2.0.0.tgz"), + "3.0.0": npmVersion("pkg", "3.0.0", "https://cdn.example/pkg-3.0.0.tgz"), + }, + Time: map[string]string{"modified": "2024-01-01T00:00:00.000Z", "2.0.0": "2023-01-01T00:00:00.000Z"}} + return []NPMMember{ + {RemoteName: "internal", Doc: mustJSON(t, local)}, + {RemoteName: "absent"}, + {RemoteName: "npmjs", Bases: []string{"https://registry.npmjs.org/"}, Doc: mustJSON(t, remote)}, + } +} + +func TestMergeNPMUnionsVersionsMemberOrderWins(t *testing.T) { + p, err := MergeNPM(npmMembersFixture(t), "http://aa/api/v1/virtual/npm/") + if err != nil { + t.Fatal(err) + } + d := decodeNPM(t, p.Full) + if len(d.Versions) != 3 { + t.Fatalf("want 3 merged versions, got %v", d.Versions) + } + for v, want := range map[string]string{ + "1.0.0": "http://aa/api/v1/virtual/npm/internal/pkg/-/pkg-1.0.0.tgz", + "2.0.0": "http://aa/api/v1/virtual/npm/npmjs/pkg/-/pkg-2.0.0.tgz", + "3.0.0": "https://cdn.example/pkg-3.0.0.tgz", + } { + if got := tarballOf(d, v); got != want { + t.Errorf("%s tarball = %q, want %q", v, got, want) + } + } + if d.Readme != "" { + t.Errorf("top-level fields come from the first member with the package, got readme %q", d.Readme) + } + if d.Time["modified"] != "2024-01-01T00:00:00.000Z" || d.Time["2.0.0"] == "" { + t.Errorf("time not merged: %v", d.Time) + } +} + +func TestMergeNPMDistTags(t *testing.T) { + members := npmMembersFixture(t) + members[0].Doc = mustJSON(t, npmDoc{Name: "pkg", DistTags: map[string]string{"latest": "1.0.0", "gone": "0.0.1"}, + Versions: map[string]map[string]any{"1.0.0": npmVersion("pkg", "1.0.0", "pkg/-/pkg-1.0.0.tgz")}}) + p, err := MergeNPM(members, "http://aa/v/") + if err != nil { + t.Fatal(err) + } + got := decodeNPM(t, p.Full).DistTags + want := map[string]string{"latest": "1.0.0", "next": "2.0.0", "beta": "1.0.0"} + if fmt.Sprint(got) != fmt.Sprint(want) { + t.Fatalf("dist-tags = %v, want %v", got, want) + } +} + +func TestMergeNPMDropsFirstTagNamingMissingVersion(t *testing.T) { + // internal claims beta=9.9.9 first; it wins over npmjs's beta, and is then + // dropped because no member has 9.9.9. + p, err := MergeNPM(npmMembersFixture(t), "http://aa/v/") + if err != nil { + t.Fatal(err) + } + if tag, ok := decodeNPM(t, p.Full).DistTags["beta"]; ok { + t.Fatalf("beta should be dropped, got %q", tag) + } +} + +func TestMergeNPMAbbreviated(t *testing.T) { + p, err := MergeNPM(npmMembersFixture(t), "http://aa/v/") + if err != nil { + t.Fatal(err) + } + var a map[string]any + if err := json.Unmarshal(p.Abbreviated, &a); err != nil { + t.Fatal(err) + } + if a["modified"] != "2024-01-01T00:00:00.000Z" || a["name"] != "pkg" || a["readme"] != nil || a["time"] != nil { + t.Fatalf("bad abbreviated top level: %v", a) + } + v := a["versions"].(map[string]any)["2.0.0"].(map[string]any) + if v["readme"] != nil || v["scripts"] != nil { + t.Errorf("abbreviated version keeps non-install fields: %v", v) + } + if v["dependencies"] == nil || v["dist"].(map[string]any)["tarball"] != "http://aa/v/npmjs/pkg/-/pkg-2.0.0.tgz" { + t.Errorf("abbreviated version lost install fields: %v", v) + } +} + +func TestMergeNPMNoMemberHasPackage(t *testing.T) { + if _, err := MergeNPM([]NPMMember{{RemoteName: "a"}, {RemoteName: "b"}}, "http://aa/v/"); !errors.Is(err, ErrNotFound) { + t.Fatalf("want ErrNotFound, got %v", err) + } +} + +func TestNPMMemberPath(t *testing.T) { + for _, c := range []struct { + tarball string + bases []string + want string + ok bool + }{ + {"@s/n/-/n-1.0.0.tgz", nil, "@s/n/-/n-1.0.0.tgz", true}, + {"https://evil.example/x.tgz", nil, "", false}, + {"/abs/x.tgz", nil, "", false}, + {"https://r.example/@s/n/-/n-1.0.0.tgz", []string{"https://m.example", "https://r.example"}, "@s/n/-/n-1.0.0.tgz", true}, + {"https://r.example.evil/x.tgz", []string{"https://r.example"}, "", false}, + } { + got, ok := npmMemberPath(c.tarball, c.bases) + if ok != c.ok || ok && got != c.want { + t.Errorf("npmMemberPath(%q, %v) = %q,%v want %q,%v", c.tarball, c.bases, got, ok, c.want, c.ok) + } + } +} + +func npmEngine(fetch func(name, pkg string) (*NPMMember, error)) *Engine { + return &Engine{ + mergeTTL: time.Minute, + getRemote: func(_ context.Context, name string) (*models.Remote, error) { + return &models.Remote{Name: name, RepoType: models.RepoTypeRemote}, nil + }, + npmMember: func(_ context.Context, name, pkg string) (*NPMMember, error) { return fetch(name, pkg) }, + } +} + +func npmVirt(members ...string) models.Virtual { + return models.Virtual{Name: "npm", PackageType: models.PackageNPM, Members: members} +} + +func TestFetchNPMScopedAndAbbreviated(t *testing.T) { + var mu sync.Mutex + var asked []string + e := npmEngine(func(name, pkg string) (*NPMMember, error) { + mu.Lock() + asked = append(asked, pkg) + mu.Unlock() + if name == "b" { + return &NPMMember{RemoteName: name}, nil + } + return &NPMMember{RemoteName: name, Doc: mustJSON(t, npmDoc{Name: pkg, DistTags: map[string]string{"latest": "1.0.0"}, + Versions: map[string]map[string]any{"1.0.0": npmVersion(pkg, "1.0.0", pkg+"/-/n-1.0.0.tgz")}})}, nil + }) + body, ct, err := e.FetchNPM(context.Background(), npmVirt("a", "b"), "@s%2fn", "http://host", true) + if err != nil { + t.Fatal(err) + } + if ct != NPMAbbreviatedType || !strings.Contains(string(body), `"tarball":"http://host/api/v1/virtual/npm/a/@s/n/-/n-1.0.0.tgz"`) { + t.Fatalf("ct %q body %s", ct, body) + } + if asked[0] != "@s/n" { + t.Fatalf("member asked for %q, want decoded scoped name", asked[0]) + } + full, ct, err := e.FetchNPM(context.Background(), npmVirt("a", "b"), "@s%2fn", "http://host", false) + if err != nil || ct != "application/json" || string(full) == string(body) { + t.Fatalf("full packument: ct %q err %v", ct, err) + } + if len(asked) != 2 { + t.Fatalf("second fetch should be served from the merge cache, members asked %d times", len(asked)) + } +} + +func TestFetchNPMFailsClosed(t *testing.T) { + e := npmEngine(func(name, pkg string) (*NPMMember, error) { + if name == "b" { + return nil, errors.New("upstream down") + } + return &NPMMember{RemoteName: name, Doc: []byte(`{"name":"x","versions":{}}`)}, nil + }) + _, _, err := e.FetchNPM(context.Background(), npmVirt("a", "b"), "x", "http://host", false) + if err == nil || errors.Is(err, ErrNotFound) { + t.Fatalf("want upstream error with a member down, got %v", err) + } +} + +func TestFetchNPMNotFound(t *testing.T) { + e := npmEngine(func(name, _ string) (*NPMMember, error) { return &NPMMember{RemoteName: name}, nil }) + for _, path := range []string{"missing", "-/npm/v1/security/advisories/bulk", "a/../b", "@s"} { + if _, _, err := e.FetchNPM(context.Background(), npmVirt("a"), path, "http://host", false); !errors.Is(err, ErrNotFound) { + t.Errorf("%q: want ErrNotFound, got %v", path, err) + } + } +} + +func TestFetchNPMMemberLocalNotFound(t *testing.T) { + e := &Engine{getRemote: func(_ context.Context, name string) (*models.Remote, error) { + return &models.Remote{Name: name, RepoType: models.RepoTypeLocal}, nil + }} + e.NPMLocal = func(_ context.Context, _, pkg string) ([]byte, error) { + if pkg == "boom" { + return nil, errors.New("db down") + } + return nil, fmt.Errorf("wrapped: %w", ErrNotFound) + } + m, err := e.fetchNPMMember(context.Background(), "internal", "nope") + if err != nil || m.Doc != nil { + t.Fatalf("missing package must be an empty member, got %v %v", m, err) + } + if _, err := e.fetchNPMMember(context.Background(), "internal", "boom"); err == nil { + t.Fatal("local read errors must surface") + } +} + +func TestMemberRedirectNPM(t *testing.T) { + e := npmEngine(nil) + virt := npmVirt("internal", "npmjs") + loc, ok, err := e.MemberRedirect(context.Background(), virt, "npmjs/@s/n/-/n-1.0.0.tgz", "", "http://host") + if err != nil || !ok || loc != "http://host/api/v1/remote/npmjs/@s/n/-/n-1.0.0.tgz" { + t.Fatalf("got %q %v %v", loc, ok, err) + } + if _, ok, _ := e.MemberRedirect(context.Background(), virt, "@s/n", "", "http://host"); ok { + t.Fatal("scoped packument path must not redirect") + } + if _, _, err := e.MemberRedirect(context.Background(), virt, "npmjs/%2e%2e/x.tgz", "", "http://host"); !errors.Is(err, ErrBadPath) { + t.Fatalf("want ErrBadPath, got %v", err) + } +} + +func TestEvictOldestBoundsNPMMerges(t *testing.T) { + e := npmEngine(func(name, pkg string) (*NPMMember, error) { + return &NPMMember{RemoteName: name, Doc: mustJSON(t, npmDoc{Name: pkg, Versions: map[string]map[string]any{}})}, nil + }) + for i := range maxNPMPackuments + 5 { + if _, _, err := e.FetchNPM(context.Background(), npmVirt("a"), fmt.Sprintf("p%d", i), "http://host", false); err != nil { + t.Fatal(err) + } + } + if len(e.gens) != maxNPMPackuments { + t.Fatalf("cached merges = %d, want %d", len(e.gens), maxNPMPackuments) + } + if _, ok := e.gens["npm\x00npm\x00p0\x00http://host"]; ok { + t.Fatal("oldest merge should be evicted") + } +} -- 2.47.3 From 8dcbe55936919a385ccc94405f6a8c7a6ad5aef3 Mon Sep 17 00:00:00 2001 From: unkin-agent Date: Sat, 17 Oct 2026 21:46:17 +1100 Subject: [PATCH 6/6] Validate Host and mark npm packuments per-host uncacheable --- internal/api/v1/npm.go | 14 +++++++++- internal/api/v1/npm_handler_test.go | 40 +++++++++++++++++++++++++++++ internal/api/v1/proxy.go | 24 ++++++++++++++--- internal/virtual/npm_merger.go | 6 ++++- 4 files changed, 78 insertions(+), 6 deletions(-) diff --git a/internal/api/v1/npm.go b/internal/api/v1/npm.go index e5caf25..5927f7f 100644 --- a/internal/api/v1/npm.go +++ b/internal/api/v1/npm.go @@ -253,8 +253,12 @@ func (h *ProxyHandler) serveNPMPackument(w http.ResponseWriter, r *http.Request, npmError(w, http.StatusInternalServerError, err.Error()) return } + base, ok := requestBaseURL(w, r) + if !ok { + return + } read := func(hash string) ([]byte, error) { return h.readLocalBlob(r.Context(), hash) } - tarballBase := fmt.Sprintf("%s://%s/api/v1/local/%s/", Scheme(r), r.Host, remote.Name) + tarballBase := base + "/api/v1/local/" + remote.Name + "/" doc, err := buildNPMPackument(pkg, files, read, tarballBase) if err != nil { npmError(w, http.StatusInternalServerError, err.Error()) @@ -264,12 +268,20 @@ func (h *ProxyHandler) serveNPMPackument(w http.ResponseWriter, r *http.Request, npmError(w, http.StatusNotFound, "not found") return } + setNPMPerHostHeaders(w) w.Header().Set("Content-Type", "application/json") w.Header().Set("X-Artifact-Source", "local") w.WriteHeader(http.StatusOK) _ = json.NewEncoder(w).Encode(doc) } +// setNPMPerHostHeaders keeps shared caches off packuments whose tarball URLs +// are built from the request host and Accept (abbreviated vs full). +func setNPMPerHostHeaders(w http.ResponseWriter) { + w.Header().Set("Cache-Control", "no-store") + w.Header().Set("Vary", "Host, X-Forwarded-Proto, Accept") +} + // buildNPMPackument returns nil when pkg has no published versions. func buildNPMPackument(pkg string, files []database.LocalFile, read func(hash string) ([]byte, error), tarballBase string) (map[string]any, error) { prefix := pkg + "/-/" diff --git a/internal/api/v1/npm_handler_test.go b/internal/api/v1/npm_handler_test.go index 8b21c09..95c8f3b 100644 --- a/internal/api/v1/npm_handler_test.go +++ b/internal/api/v1/npm_handler_test.go @@ -18,6 +18,7 @@ import ( "git.unkin.net/unkin/artifactapi/internal/database" "git.unkin.net/unkin/artifactapi/internal/storage" "git.unkin.net/unkin/artifactapi/internal/testsupport" + "git.unkin.net/unkin/artifactapi/internal/virtual" "git.unkin.net/unkin/artifactapi/pkg/models" ) @@ -291,3 +292,42 @@ func TestNPMPublishHandler(t *testing.T) { } }) } + +func TestNPMPackumentPerHost(t *testing.T) { + h, srv := npmTestServer(t) + h.virtualEngine = virtual.NewEngine(h.db, nil) + h.virtualEngine.NPMLocal = h.LocalNPMPackument + repo := npmRepo(t, h, models.PackageNPM) + virt := "v" + repo + if err := h.db.CreateVirtual(context.Background(), &models.Virtual{Name: virt, PackageType: models.PackageNPM, Members: []string{repo}}); err != nil { + t.Fatal(err) + } + if code, b := npmDo(t, http.MethodPut, srv.URL+"/api/v1/local/"+repo+"/hostpkg", publishBody(t, "hostpkg", []byte("tgz"), publishOpts{})); code != http.StatusCreated { + t.Fatalf("publish = %d: %s", code, b) + } + routes := h.Routes() + get := func(path, host, proto string) *httptest.ResponseRecorder { + r := httptest.NewRequest(http.MethodGet, path, nil) + r.Host = host + if proto != "" { + r.Header.Set("X-Forwarded-Proto", proto) + } + w := httptest.NewRecorder() + routes.ServeHTTP(w, r) + return w + } + for _, path := range []string{"/local/" + repo + "/hostpkg", "/virtual/" + virt + "/hostpkg"} { + for _, host := range []string{"x@evil.com", "evil.example:"} { + if w := get(path, host, ""); w.Code != http.StatusBadRequest { + t.Errorf("%s Host %q = %d, want 400", path, host, w.Code) + } + } + w := get(path, "aa.example", "https") + if w.Code != http.StatusOK || !strings.Contains(w.Body.String(), `"tarball":"https://aa.example/api/v1/`) { + t.Fatalf("%s = %d: %s", path, w.Code, w.Body) + } + if cc, vary := w.Header().Get("Cache-Control"), w.Header().Get("Vary"); cc != "no-store" || vary != "Host, X-Forwarded-Proto, Accept" { + t.Errorf("%s Cache-Control %q Vary %q", path, cc, vary) + } + } +} diff --git a/internal/api/v1/proxy.go b/internal/api/v1/proxy.go index 2d6f3b7..3ee4212 100644 --- a/internal/api/v1/proxy.go +++ b/internal/api/v1/proxy.go @@ -196,7 +196,10 @@ func (h *ProxyHandler) handleVirtual(w http.ResponseWriter, r *http.Request) { return } - proxyBaseURL := fmt.Sprintf("%s://%s", Scheme(r), r.Host) + proxyBaseURL, ok := requestBaseURL(w, r) + if !ok { + return + } loc, ok, err := h.virtualEngine.MemberRedirect(r.Context(), *virt, path, r.URL.RawQuery, proxyBaseURL) if err != nil { @@ -226,6 +229,9 @@ func (h *ProxyHandler) handleVirtual(w http.ResponseWriter, r *http.Request) { return } + if virt.PackageType == models.PackageNPM { + setNPMPerHostHeaders(w) + } w.Header().Set("Content-Type", contentType) w.Header().Set("X-Artifact-Source", "virtual") w.WriteHeader(http.StatusOK) @@ -303,8 +309,8 @@ func Scheme(r *http.Request) string { // rewriteServed rewrites a cached body for the requesting host and marks the // response uncacheable by shared caches, since it varies per Host. func rewriteServed(w http.ResponseWriter, r *http.Request, sr provider.ServeRewriter, remote models.Remote, src io.Reader) ([]byte, bool) { - if !validHost(r.Host) { - http.Error(w, "invalid host header", http.StatusBadRequest) + base, ok := requestBaseURL(w, r) + if !ok { return nil, false } raw, err := io.ReadAll(src) @@ -312,13 +318,23 @@ func rewriteServed(w http.ResponseWriter, r *http.Request, sr provider.ServeRewr http.Error(w, "bad gateway", http.StatusBadGateway) return nil, false } - raw = sr.RewriteServed(raw, remote, fmt.Sprintf("%s://%s", Scheme(r), r.Host)) + raw = sr.RewriteServed(raw, remote, base) w.Header().Set("Cache-Control", "no-store") w.Header().Set("Vary", "Host, X-Forwarded-Proto") w.Header().Set("Content-Length", strconv.Itoa(len(raw))) return raw, true } +// requestBaseURL returns the client-facing scheme://host, answering 400 on an +// invalid Host. +func requestBaseURL(w http.ResponseWriter, r *http.Request) (string, bool) { + if !validHost(r.Host) { + http.Error(w, "invalid host header", http.StatusBadRequest) + return "", false + } + return Scheme(r) + "://" + r.Host, true +} + // validHost accepts host[:port] where host is a DNS name or IP literal. func validHost(hostport string) bool { host, port := hostport, "" diff --git a/internal/virtual/npm_merger.go b/internal/virtual/npm_merger.go index 3c19fc9..844059f 100644 --- a/internal/virtual/npm_merger.go +++ b/internal/virtual/npm_merger.go @@ -200,7 +200,9 @@ func (e *Engine) FetchNPM(ctx context.Context, virt models.Virtual, path, proxyB } prefix := "npm\x00" + virt.Name + "\x00" tarballBase := strings.TrimRight(proxyBaseURL, "/") + "/api/v1/virtual/" + url.PathEscape(virt.Name) + "/" + inserted := false repo, err := e.merged(ctx, prefix+pkg+"\x00"+proxyBaseURL, func(ctx context.Context) (mergedRepo, error) { + inserted = true members, err := fetchMembers(virt, func(name string) (*NPMMember, error) { return e.npmMember(ctx, name, pkg) }) if err != nil { return nil, err @@ -214,7 +216,9 @@ func (e *Engine) FetchNPM(ctx context.Context, virt models.Virtual, path, proxyB if err != nil { return nil, "", err } - e.evictOldest(prefix, maxNPMPackuments) + if inserted { + e.evictOldest(prefix, maxNPMPackuments) + } if abbreviated { return repo.files()[npmAbbreviatedFile], NPMAbbreviatedType, nil } -- 2.47.3