fix: decode PURL percent-encoding in versions and package paths

internal/database/queries.go

@@ -443,11 +443,14 @@ func (db *DB) GetMostPopularPackages(limit int) ([]PopularPackage, error) {
 }
 
 type RecentPackage struct {
-	Ecosystem string    `db:"ecosystem"`
-	Name      string    `db:"name"`
-	Version   string    `db:"version"`
-	CachedAt  time.Time `db:"fetched_at"`
-	Size      int64     `db:"size"`
+	Ecosystem   string    `db:"ecosystem"`
+	Name        string    `db:"name"`
+	VersionPURL string    `db:"version_purl"`
+	CachedAt    time.Time `db:"fetched_at"`
+	Size        int64     `db:"size"`
+	// Version is derived from VersionPURL rather than selected, so that the
+	// PURL percent-encoding is decoded (e.g. "%2B" back to "+").
+	Version string `db:"-"`
 }
 
 func (db *DB) GetRecentlyCachedPackages(limit int) ([]RecentPackage, error) {
@@ -461,10 +464,10 @@ func (db *DB) GetRecentlyCachedPackages(limit int) ([]RecentPackage, error) {
 	}
 
 	var packages []RecentPackage
-	// We need to extract version from the purl since there's no separate version column
+	// There is no separate version column, so the full version PURL is selected
+	// and the version is decoded from it in Go.
 	query := db.Rebind(`
-		SELECT p.ecosystem, p.name,
-		       SUBSTR(v.purl, INSTR(v.purl, '@') + 1) as version,
+		SELECT p.ecosystem, p.name, v.purl as version_purl,
 		       a.fetched_at, COALESCE(a.size, 0) as size
 		FROM artifacts a
 		JOIN versions v ON v.purl = a.version_purl
@@ -474,25 +477,13 @@ func (db *DB) GetRecentlyCachedPackages(limit int) ([]RecentPackage, error) {
 		LIMIT ?
 	`)
 
-	// For postgres, use different string function
-	if db.dialect == DialectPostgres {
-		query = db.Rebind(`
-			SELECT p.ecosystem, p.name,
-			       SUBSTRING(v.purl FROM POSITION('@' IN v.purl) + 1) as version,
-			       a.fetched_at, COALESCE(a.size, 0) as size
-			FROM artifacts a
-			JOIN versions v ON v.purl = a.version_purl
-			JOIN packages p ON p.purl = v.package_purl
-			WHERE a.storage_path IS NOT NULL AND a.fetched_at IS NOT NULL
-			ORDER BY a.fetched_at DESC
-			LIMIT ?
-		`)
-	}
-
 	err = db.Select(&packages, query, limit)
 	if err != nil {
 		return nil, err
 	}
+	for i := range packages {
+		packages[i].Version = VersionFromPURL(packages[i].VersionPURL)
+	}
 	return packages, nil
 }
 

internal/database/types.go

@@ -2,6 +2,7 @@ package database
 
 import (
 	"database/sql"
+	"net/url"
 	"strings"
 	"time"
 )
@@ -47,10 +48,66 @@ type Version struct {
 // Version extracts the version string from the PURL.
 // e.g., "pkg:npm/lodash@4.17.21" -> "4.17.21"
 func (v *Version) Version() string {
-	if idx := strings.LastIndex(v.PURL, "@"); idx >= 0 {
-		return v.PURL[idx+1:]
+	return VersionFromPURL(v.PURL)
+}
+
+// DisplayPURL returns the PURL with its path components percent-decoded, for
+// showing in the UI. The stored PURL keeps the canonical encoding (which is
+// what the API and all lookups use); this is only a readable rendering, so that
+// a version like "7.91+dfsg1-2ubuntu0.1" is not shown as "7.91%2Bdfsg1-2ubuntu0.1"
+// and an npm scope is shown as "@babel" rather than "%40babel". Qualifiers and
+// subpath keep their encoding, since decoding those would be ambiguous.
+func (v *Version) DisplayPURL() string {
+	base, suffix := v.PURL, ""
+	if i := strings.IndexAny(base, "?#"); i >= 0 {
+		base, suffix = base[:i], base[i:]
+	}
+
+	name, version := base, ""
+	if idx := strings.LastIndex(base, "@"); idx >= 0 {
+		name, version = base[:idx], "@"+decodePURLComponent(base[idx+1:])
+	}
+
+	parts := strings.Split(name, "/")
+	for i, part := range parts {
+		parts[i] = decodePURLComponent(part)
+	}
+	return strings.Join(parts, "/") + version + suffix
+}
+
+// VersionFromPURL extracts the decoded version string from a PURL.
+//
+// PURL percent-encodes characters that are not safe in a path component, so a
+// Debian version like "7.91+dfsg1-2ubuntu0.1" is stored as
+// "pkg:deb/nmap@7.91%2Bdfsg1-2ubuntu0.1". The raw substring after "@" is
+// therefore not the version: it must be percent-decoded before being displayed
+// or used to build a URL, otherwise "%2B" leaks into the UI and round-tripping
+// the value back into a PURL double-encodes it.
+//
+// e.g., "pkg:npm/lodash@4.17.21" -> "4.17.21"
+func VersionFromPURL(p string) string {
+	// Qualifiers ("?key=value") and subpath ("#path") follow the version.
+	if i := strings.IndexAny(p, "?#"); i >= 0 {
+		p = p[:i]
+	}
+	idx := strings.LastIndex(p, "@")
+	if idx < 0 {
+		return ""
+	}
+	return decodePURLComponent(p[idx+1:])
+}
+
+// decodePURLComponent percent-decodes a single PURL path component, returning
+// the input unchanged if it is not valid percent-encoding.
+func decodePURLComponent(s string) string {
+	if !strings.Contains(s, "%") {
+		return s
+	}
+	decoded, err := url.PathUnescape(s)
+	if err != nil {
+		return s
 	}
-	return ""
+	return decoded
 }
 
 // Artifact represents a cached artifact in the database.

internal/database/version_purl_test.go

@@ -0,0 +1,114 @@
+package database
+
+import (
+	"database/sql"
+	"testing"
+	"time"
+)
+
+func TestVersionFromPURL(t *testing.T) {
+	tests := []struct {
+		name string
+		purl string
+		want string
+	}{
+		{"simple", "pkg:npm/lodash@4.17.21", "4.17.21"},
+		{"namespaced", "pkg:composer/symfony/console@6.0.0", "6.0.0"},
+		// Debian/Ubuntu versions routinely contain "+", which PURL encodes.
+		{"encoded plus", "pkg:deb/nmap@7.91%2Bdfsg1%2Breally7.80%2Bdfsg1-2ubuntu0.1", "7.91+dfsg1+really7.80+dfsg1-2ubuntu0.1"},
+		{"encoded epoch", "pkg:deb/curl@1%3A7.81.0-1", "1:7.81.0-1"},
+		{"encoded plus with qualifier", "pkg:deb/nmap@7.91%2Bdfsg1?repository_url=http%3A%2F%2Fexample.com", "7.91+dfsg1"},
+		{"tilde is not encoded", "pkg:deb/foo@1.0~rc1", "1.0~rc1"},
+		{"no version", "pkg:npm/lodash", ""},
+		{"invalid escape passed through", "pkg:npm/lodash@1.0%zz", "1.0%zz"},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			if got := VersionFromPURL(tt.purl); got != tt.want {
+				t.Errorf("VersionFromPURL(%q) = %q, want %q", tt.purl, got, tt.want)
+			}
+			v := &Version{PURL: tt.purl}
+			if got := v.Version(); got != tt.want {
+				t.Errorf("Version.Version() for %q = %q, want %q", tt.purl, got, tt.want)
+			}
+		})
+	}
+}
+
+func TestVersionDisplayPURL(t *testing.T) {
+	tests := []struct {
+		name string
+		purl string
+		want string
+	}{
+		{"simple", "pkg:npm/lodash@4.17.21", "pkg:npm/lodash@4.17.21"},
+		{
+			"encoded plus",
+			"pkg:deb/nmap@7.91%2Bdfsg1%2Breally7.80%2Bdfsg1-2ubuntu0.1",
+			"pkg:deb/nmap@7.91+dfsg1+really7.80+dfsg1-2ubuntu0.1",
+		},
+		{
+			"qualifier preserved",
+			"pkg:deb/nmap@7.91%2Bdfsg1?repository_url=http%3A%2F%2Fexample.com",
+			"pkg:deb/nmap@7.91+dfsg1?repository_url=http%3A%2F%2Fexample.com",
+		},
+		// The namespace is encoded too: MakePURLString("npm", "@babel/core", …)
+		// produces "pkg:npm/%40babel/core@…".
+		{"encoded npm scope", "pkg:npm/%40babel/core@7.0.0", "pkg:npm/@babel/core@7.0.0"},
+		{"encoded scope without version", "pkg:npm/%40babel/core", "pkg:npm/@babel/core"},
+		{"no version", "pkg:npm/lodash", "pkg:npm/lodash"},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			v := &Version{PURL: tt.purl}
+			if got := v.DisplayPURL(); got != tt.want {
+				t.Errorf("DisplayPURL() for %q = %q, want %q", tt.purl, got, tt.want)
+			}
+		})
+	}
+}
+
+// TestGetRecentlyCachedPackagesDecodesVersion guards the dashboard's "recently
+// cached" list, which derives the version from the version PURL.
+func TestGetRecentlyCachedPackagesDecodesVersion(t *testing.T) {
+	runWithBothDatabases(t, func(t *testing.T, db *DB) {
+		const versionPURL = "pkg:deb/nmap@7.91%2Bdfsg1%2Breally7.80%2Bdfsg1-2ubuntu0.1"
+
+		if err := db.UpsertPackage(&Package{
+			PURL: "pkg:deb/nmap", Ecosystem: "deb", Name: "nmap",
+		}); err != nil {
+			t.Fatalf("UpsertPackage failed: %v", err)
+		}
+		if err := db.UpsertVersion(&Version{
+			PURL: versionPURL, PackagePURL: "pkg:deb/nmap",
+		}); err != nil {
+			t.Fatalf("UpsertVersion failed: %v", err)
+		}
+		if err := db.UpsertArtifact(&Artifact{
+			VersionPURL: versionPURL,
+			Filename:    "nmap_7.91+dfsg1+really7.80+dfsg1-2ubuntu0.1_amd64.deb",
+			UpstreamURL: "http://archive.ubuntu.com/ubuntu/pool/universe/n/nmap/nmap.deb",
+			StoragePath: sql.NullString{String: "/cache/nmap.deb", Valid: true},
+			FetchedAt:   sql.NullTime{Time: time.Now(), Valid: true},
+		}); err != nil {
+			t.Fatalf("UpsertArtifact failed: %v", err)
+		}
+
+		recent, err := db.GetRecentlyCachedPackages(10)
+		if err != nil {
+			t.Fatalf("GetRecentlyCachedPackages failed: %v", err)
+		}
+		if len(recent) != 1 {
+			t.Fatalf("expected 1 recent package, got %d", len(recent))
+		}
+		const want = "7.91+dfsg1+really7.80+dfsg1-2ubuntu0.1"
+		if recent[0].Version != want {
+			t.Errorf("Version = %q, want %q", recent[0].Version, want)
+		}
+		if recent[0].VersionPURL != versionPURL {
+			t.Errorf("VersionPURL = %q, want %q", recent[0].VersionPURL, versionPURL)
+		}
+	})
+}

internal/handler/debian_test.go

@@ -12,6 +12,11 @@ func TestDebianHandler_parsePoolPath(t *testing.T) {
 		{"pool/main/libn/libncurses/libncurses6_6.2-1_amd64.deb", "libncurses6", "6.2-1", "amd64"},
 		{"pool/contrib/v/virtualbox/virtualbox_6.1.38-1_amd64.deb", "virtualbox", "6.1.38-1", "amd64"},
 		{"pool/main/g/git/git_2.39.2-1_arm64.deb", "git", "2.39.2-1", "arm64"},
+		{
+			"pool/universe/n/nmap/nmap_7.91+dfsg1+really7.80+dfsg1-2ubuntu0.1_amd64.deb",
+			"nmap", "7.91+dfsg1+really7.80+dfsg1-2ubuntu0.1", "amd64",
+		},
+		{"pool/main/o/openssl/openssl_3.0.2-0ubuntu1.15~build1_amd64.deb", "openssl", "3.0.2-0ubuntu1.15~build1", "amd64"},
 		{"invalid/path", "", "", ""},
 		{"pool/main/n/nginx/nginx.deb", "", "", ""},
 	})

internal/server/resolve.go

@@ -2,6 +2,7 @@ package server
 
 import (
 	"fmt"
+	"net/url"
 	"strings"
 	"unicode"
 
@@ -23,12 +24,25 @@ func validatePackagePath(path string) error {
 	if len(path) > maxPackagePathLen {
 		return fmt.Errorf("package path exceeds %d bytes", maxPackagePathLen)
 	}
-	for _, r := range path {
-		if r == 0 {
-			return fmt.Errorf("package path contains null byte")
+	// Validate the decoded segments: the handlers work with decoded values, so
+	// an escape such as "%00" or "%2E%2E" must not slip past these checks.
+	for _, seg := range splitWildcardPath(path) {
+		// A decoded segment can itself contain slashes (from "%2F"), and the
+		// segments are later rejoined into a package name that registries
+		// interpolate straight into an upstream URL. Check every path element,
+		// not just the segment as a whole, or "a%2F..%2F..%2Fb" traverses.
+		for _, elem := range strings.Split(seg, "/") {
+			if elem == ".." {
+				return fmt.Errorf("package path contains parent directory segment")
+			}
 		}
-		if unicode.IsControl(r) {
-			return fmt.Errorf("package path contains control character %#U", r)
+		for _, r := range seg {
+			if r == 0 {
+				return fmt.Errorf("package path contains null byte")
+			}
+			if unicode.IsControl(r) {
+				return fmt.Errorf("package path contains control character %#U", r)
+			}
 		}
 	}
 	return nil
@@ -60,10 +74,34 @@ func resolvePackageName(db *database.DB, ecosystem string, segments []string) (n
 
 // splitWildcardPath splits a chi wildcard path value into segments,
 // trimming any leading/trailing slashes.
+//
+// chi routes on the raw (still percent-encoded) path whenever the request URL
+// contains an escape, so each segment is decoded after splitting. Splitting
+// first keeps an encoded "%2F" inside a name from being mistaken for a
+// separator. Decoding matters for versions such as "1.0%2Bbuild1", which must
+// reach the handlers as "1.0+build1" so that rebuilding the PURL yields the
+// value that was stored rather than a double-encoded one.
 func splitWildcardPath(path string) []string {
 	path = strings.Trim(path, "/")
 	if path == "" {
 		return nil
 	}
-	return strings.Split(path, "/")
+	segments := strings.Split(path, "/")
+	for i, seg := range segments {
+		segments[i] = decodePathSegment(seg)
+	}
+	return segments
+}
+
+// decodePathSegment percent-decodes a single URL path segment, returning it
+// unchanged if it is not valid percent-encoding.
+func decodePathSegment(seg string) string {
+	if !strings.Contains(seg, "%") {
+		return seg
+	}
+	decoded, err := url.PathUnescape(seg)
+	if err != nil {
+		return seg
+	}
+	return decoded
 }

internal/server/resolve_test.go

@@ -104,6 +104,14 @@ func TestSplitWildcardPath(t *testing.T) {
 		{"symfony/console/6.0.0/browse", []string{"symfony", "console", "6.0.0", "browse"}},
 		{"", nil},
 		{"/", nil},
+		// chi routes on the raw path when the URL contains an escape, so
+		// segments arrive percent-encoded and must be decoded.
+		{"nmap/7.91%2Bdfsg1%2Breally7.80%2Bdfsg1-2ubuntu0.1", []string{"nmap", "7.91+dfsg1+really7.80+dfsg1-2ubuntu0.1"}},
+		{"%40babel/core/7.0.0", []string{"@babel", "core", "7.0.0"}},
+		// An encoded separator stays inside its segment rather than splitting.
+		{"vendor%2Fname/1.0.0", []string{"vendor/name", "1.0.0"}},
+		// Invalid escapes are passed through untouched.
+		{"lodash/1.0%zz", []string{"lodash", "1.0%zz"}},
 	}
 
 	for _, tt := range tests {
@@ -132,8 +140,19 @@ func TestValidatePackagePath(t *testing.T) {
 		{"composer namespaced", "symfony/console/6.0.0", false},
 		{"maven coordinates", "org.apache.commons/commons-lang3/3.12.0", false},
 		{"unicode", "café/1.0.0", false},
+		{"encoded plus in version", "nmap/7.91%2Bdfsg1-2ubuntu0.1", false},
 		{"empty", "", true},
 		{"null byte", "lodash\x00/4.17.21", true},
+		{"encoded null byte", "lodash/%00", true},
+		{"encoded newline", "lodash/1.0%0A", true},
+		{"parent segment", "lodash/../4.17.21", true},
+		{"encoded parent segment", "lodash/%2E%2E/4.17.21", true},
+		// A decoded segment can contain slashes, so traversal can hide inside
+		// one segment. Registries interpolate the resolved name straight into
+		// an upstream URL, and Go sends dot-segments verbatim.
+		{"traversal inside one segment", "pkg%2F..%2F..%2Fadmin", true},
+		{"traversal via encoded dots and slash", "pkg%2f%2e%2e%2fadmin", true},
+		{"encoded slash alone is allowed", "vendor%2Fname/1.0.0", false},
 		{"null byte suffix", "lodash\x00", true},
 		{"newline", "lodash\n4.17.21", true},
 		{"carriage return", "lodash\r", true},