fix: enhance NAR URL normalization and fix potential path traversal

pkg/cache/cache.go

@@ -9,6 +9,7 @@ import (
 	"net/http"
 	"net/url"
 	"os"
+	"path/filepath"
 	"slices"
 	"strings"
 	"sync"
@@ -1100,7 +1101,7 @@ func (c *Cache) PutNar(ctx context.Context, narURL nar.URL, r io.ReadCloser) err
 }
 
 func (c *Cache) putNarWithCDC(ctx context.Context, narURL nar.URL, r io.Reader) error {
-	f, err := os.CreateTemp(c.tempDir, fmt.Sprintf("%s-*.nar", narURL.Hash))
+	f, err := os.CreateTemp(c.tempDir, fmt.Sprintf("%s-*.nar", filepath.Base(narURL.Hash)))
 	if err != nil {
 		return fmt.Errorf("failed to create temp file for CDC: %w", err)
 	}
@@ -1155,7 +1156,7 @@ func (c *Cache) DeleteNar(ctx context.Context, narURL nar.URL) error {
 // createTempNarFile creates a temporary file for storing the NAR during download.
 // It sets up cleanup to remove the file once all readers are done.
 func (c *Cache) createTempNarFile(ctx context.Context, narURL *nar.URL, ds *downloadState) (*os.File, error) {
-	pattern := narURL.Hash + "-*.nar"
+	pattern := filepath.Base(narURL.Hash) + "-*.nar"
 	if cext := narURL.Compression.String(); cext != "" {
 		pattern += "." + cext
 	}
@@ -2800,7 +2801,10 @@ func (c *Cache) storeInDatabase(
 
 		// Normalize the NAR URL to remove any narinfo hash prefix.
 		// This ensures nar_files.hash matches what's actually stored in the storage layer.
-		normalizedNarURL := narURL.Normalize()
+		normalizedNarURL, err := narURL.Normalize()
+		if err != nil {
+			return fmt.Errorf("error normalizing the nar URL: %w", err)
+		}
 
 		// Check if nar_file already exists (multiple narinfos can share the same nar_file)
 
@@ -3201,7 +3205,10 @@ func storeNarInfoInDatabase(ctx context.Context, db database.Querier, hash strin
 
 	// Normalize the NAR URL to remove any narinfo hash prefix.
 	// This ensures nar_files.hash matches what's actually stored in the storage layer.
-	normalizedNarURL := narURL.Normalize()
+	normalizedNarURL, err := narURL.Normalize()
+	if err != nil {
+		return fmt.Errorf("error normalizing the nar URL: %w", err)
+	}
 
 	// Create or get nar_file record
 	narFileID, err := createOrUpdateNarFile(ctx, qtx, normalizedNarURL, narInfo.FileSize)

pkg/nar/hash.go

@@ -13,13 +13,23 @@ import (
 // Hashes must be exactly 52 characters long.
 const NormalizedHashPattern = `[0-9a-df-np-sv-z]{52}`
 
-const HashPattern = `(` + narinfo.HashPattern + `-)?` + NormalizedHashPattern
+// HashPattern is the strict validation pattern for complete nar hashes.
+// It matches an optional prefix (narinfo hash + separator) followed by exactly
+// a 52-character normalized hash. Used with anchors (^...$) to validate the full input.
+// For extraction and lenient parsing, use HashPatternLenient instead.
+const HashPattern = `(?:(` + narinfo.HashPattern + `[-_]))?` + NormalizedHashPattern
+
+// HashPatternLenient is used for parsing/extraction. It matches optional prefix
+// followed by anything, allowing us to extract and validate parts separately.
+const HashPatternLenient = `(?:(` + narinfo.HashPattern + `[-_]))?(.+)`
 
 var (
 	// ErrInvalidHash is returned if the hash is not valid.
 	ErrInvalidHash = errors.New("invalid nar hash")
 
-	narHashRegexp = regexp.MustCompile(`^(` + HashPattern + `)$`)
+	narHashRegexp           = regexp.MustCompile(`^` + HashPattern + `$`)
+	narNormalizedHashRegexp = regexp.MustCompile(`^` + NormalizedHashPattern + `$`)
+	narHashLenientRegexp    = regexp.MustCompile(`^` + HashPatternLenient + `$`)
 )
 
 func ValidateHash(hash string) error {

pkg/nar/url.go

@@ -5,19 +5,13 @@ import (
 	"fmt"
 	"net/url"
 	"path/filepath"
-	"regexp"
 	"strings"
 
 	"github.com/rs/zerolog"
 )
 
-var (
-	// ErrInvalidURL is returned if the regexp did not match the given URL.
-	ErrInvalidURL = errors.New("invalid nar URL")
-
-	// hashValidationRegexp validates that a string matches the HashPattern.
-	hashValidationRegexp = regexp.MustCompile(`^(` + HashPattern + `)$`)
-)
+// ErrInvalidURL is returned if the regexp did not match the given URL.
+var ErrInvalidURL = errors.New("invalid nar URL")
 
 // URL represents a nar URL.
 type URL struct {
@@ -53,8 +47,8 @@ func ParseURL(u string) (URL, error) {
 	}
 
 	// Validate that the hash matches HashPattern before processing further
-	if !hashValidationRegexp.MatchString(hash) {
-		return URL{}, ErrInvalidURL
+	if err := ValidateHash(hash); err != nil {
+		return URL{}, err
 	}
 
 	// Extract compression extension (e.g., ".bz2" -> "bz2", "" -> "")
@@ -141,38 +135,28 @@ func (u URL) pathWithCompression() string {
 // Normalize returns a new URL with the narinfo hash prefix trimmed from the Hash.
 // nix-serve serves NAR URLs with the narinfo hash as a prefix (e.g., "narinfo-hash-actual-hash").
 // This method removes that prefix to standardize the hash for storage.
-func (u URL) Normalize() URL {
+func (u URL) Normalize() (URL, error) {
 	hash := u.Hash
 
-	// Find the first separator ('-' or '_').
-	idx := strings.IndexAny(hash, "-_")
-
-	// If a separator is found after the first character.
-	if idx > 0 {
-		prefix := hash[:idx]
-		suffix := hash[idx+1:]
-
-		// A narinfo hash prefix is typically 32 characters long. This is a strong signal.
-		// We check this and ensure the suffix is not empty.
-		if len(prefix) == 32 && len(suffix) > 0 {
-			hash = suffix
-		}
+	// First, try the lenient regex to extract prefix and potential hash part
+	sm := narHashLenientRegexp.FindStringSubmatch(hash)
+	if len(sm) < 3 {
+		return URL{}, fmt.Errorf("%w: %s", ErrInvalidHash, hash)
 	}
 
-	// Sanitize the hash to prevent path traversal.
-	// Even though ParseURL validates the hash, URL is a public struct
-	// and Normalize could be called on a manually constructed URL.
-	cleanedHash := filepath.Clean(hash)
-	if strings.Contains(cleanedHash, "..") || strings.HasPrefix(cleanedHash, "/") {
-		// If the cleaned hash is still invalid, we return the original URL
-		// to avoid potentially breaking something that might be valid in some context,
-		// but storage layers will still validate it using ToFilePath().
-		return u
+	// sm[0] is the entire match
+	// sm[1] is the optional prefix with separator (e.g., "abc-" or "abc_"), or empty if no prefix
+	// sm[2] is everything after the optional prefix (always the actual hash we want)
+	actualHash := sm[2]
+
+	// Strictly validate the extracted hash matches the normalized pattern
+	if !narNormalizedHashRegexp.MatchString(actualHash) {
+		return URL{}, fmt.Errorf("%w: %s", ErrInvalidHash, actualHash)
 	}
 
 	return URL{
-		Hash:        cleanedHash,
+		Hash:        actualHash,
 		Compression: u.Compression,
 		Query:       u.Query,
-	}
+	}, nil
 }

pkg/nar/url_test.go

@@ -138,6 +138,7 @@ func TestNormalize(t *testing.T) {
 	tests := []struct {
 		input  nar.URL
 		output nar.URL
+		errStr string
 	}{
 		{
 			input: nar.URL{
@@ -178,29 +179,27 @@ func TestNormalize(t *testing.T) {
 		{
 			// Valid hash with separator but no prefix
 			input: nar.URL{
-				Hash: "my-hash",
+				Hash: "1m9phnql68mxrnjc7ssxcvjrxxwcx0fzc849w025mkanwgsy1bpy",
 			},
 			output: nar.URL{
-				Hash: "my-hash",
+				Hash: "1m9phnql68mxrnjc7ssxcvjrxxwcx0fzc849w025mkanwgsy1bpy",
 			},
 		},
 		{
 			// Valid prefix and multiple separators in the suffix
 			input: nar.URL{
-				Hash: "c12lxpykv6sld7a0sakcnr3y0la70x8w-part1-part2",
+				Hash: "c12lxpykv6sld7a0sakcnr3y0la70x8w-1m9phnql68mxrnjc7ssxcvjrxxwcx0fzc849w025mkanwgsy1bpy",
 			},
 			output: nar.URL{
-				Hash: "part1-part2",
+				Hash: "1m9phnql68mxrnjc7ssxcvjrxxwcx0fzc849w025mkanwgsy1bpy",
 			},
 		},
 		{
 			// Potential path traversal attempt in the hash (should remain unchanged or be sanitized)
 			input: nar.URL{
 				Hash: "c12lxpykv6sld7a0sakcnr3y0la70x8w-../../etc/passwd",
 			},
-			output: nar.URL{
-				Hash: "c12lxpykv6sld7a0sakcnr3y0la70x8w-../../etc/passwd",
-			},
+			errStr: "invalid nar hash: ../../etc/passwd",
 		},
 	}
 
@@ -213,8 +212,13 @@ func TestNormalize(t *testing.T) {
 		t.Run(tname, func(t *testing.T) {
 			t.Parallel()
 
-			result := test.input.Normalize()
-			assert.Equal(t, test.output, result)
+			result, err := test.input.Normalize()
+			if test.errStr != "" {
+				assert.EqualError(t, err, test.errStr)
+			} else {
+				require.NoError(t, err)
+				assert.Equal(t, test.output, result)
+			}
 		})
 	}
 }

pkg/server/server.go

@@ -359,7 +359,19 @@ func (s *Server) getNarInfo(withBody bool) http.HandlerFunc {
 				return
 			}
 
-			narInfoCopy.URL = narURL.Normalize().String()
+			normalizedURL, err := narURL.Normalize()
+			if err != nil {
+				zerolog.Ctx(r.Context()).
+					Error().
+					Err(err).
+					Msg("error normalizing the NAR URL")
+
+				http.Error(w, err.Error(), http.StatusInternalServerError)
+
+				return
+			}
+
+			narInfoCopy.URL = normalizedURL.String()
 		}
 
 		narInfoBytes := []byte(narInfoCopy.String())

pkg/storage/local/local.go

@@ -330,7 +330,12 @@ func (s *Store) DeleteNarInfo(ctx context.Context, hash string) error {
 
 // HasNar returns true if the store has the nar.
 func (s *Store) HasNar(ctx context.Context, narURL nar.URL) bool {
-	tfp, err := narURL.Normalize().ToFilePath()
+	normalizedURL, err := narURL.Normalize()
+	if err != nil {
+		return false
+	}
+
+	tfp, err := normalizedURL.ToFilePath()
 	if err != nil {
 		return false
 	}
@@ -357,7 +362,10 @@ func (s *Store) HasNar(ctx context.Context, narURL nar.URL) bool {
 // NOTE: The caller must close the returned io.ReadCloser!
 func (s *Store) GetNar(ctx context.Context, narURL nar.URL) (int64, io.ReadCloser, error) {
 	// Normalize the NAR URL to handle URLs with embedded narinfo hash prefix
-	normalizedURL := narURL.Normalize()
+	normalizedURL, err := narURL.Normalize()
+	if err != nil {
+		return 0, nil, err
+	}
 
 	tfp, err := normalizedURL.ToFilePath()
 	if err != nil {
@@ -397,7 +405,10 @@ func (s *Store) GetNar(ctx context.Context, narURL nar.URL) (int64, io.ReadClose
 // PutNar puts the nar in the store.
 func (s *Store) PutNar(ctx context.Context, narURL nar.URL, body io.Reader) (int64, error) {
 	// Normalize the NAR URL to handle URLs with embedded narinfo hash prefix
-	normalizedURL := narURL.Normalize()
+	normalizedURL, err := narURL.Normalize()
+	if err != nil {
+		return 0, err
+	}
 
 	tfp, err := normalizedURL.ToFilePath()
 	if err != nil {
@@ -457,7 +468,10 @@ func (s *Store) PutNar(ctx context.Context, narURL nar.URL, body io.Reader) (int
 // DeleteNar deletes the nar from the store.
 func (s *Store) DeleteNar(ctx context.Context, narURL nar.URL) error {
 	// Normalize the NAR URL to handle URLs with embedded narinfo hash prefix
-	normalizedURL := narURL.Normalize()
+	normalizedURL, err := narURL.Normalize()
+	if err != nil {
+		return err
+	}
 
 	tfp, err := normalizedURL.ToFilePath()
 	if err != nil {

pkg/storage/s3/s3.go

@@ -566,7 +566,12 @@ func (s *Store) narInfoPath(hash string) (string, error) {
 }
 
 func (s *Store) narPath(narURL nar.URL) (string, error) {
-	tfp, err := narURL.Normalize().ToFilePath()
+	normalizedURL, err := narURL.Normalize()
+	if err != nil {
+		return "", err
+	}
+
+	tfp, err := normalizedURL.ToFilePath()
 	if err != nil {
 		return "", err
 	}

testdata/server.go

@@ -143,7 +143,15 @@ func (s *Server) handler() http.Handler {
 			}
 
 			// Support fetching by normalized hash (with prefix stripped)
-			normalizedHash := (&nar.URL{Hash: entry.NarHash}).Normalize().Hash
+			normalizedURL, err := (&nar.URL{Hash: entry.NarHash}).Normalize()
+			if err != nil {
+				http.Error(w, err.Error(), http.StatusBadRequest)
+
+				return
+			}
+
+			normalizedHash := normalizedURL.Hash
+
 			if normalizedHash != entry.NarHash {
 				if r.URL.Path == "/nar/"+normalizedHash+".nar" {
 					bs = []byte(entry.NarText)