fix: harden NAR hash validation and improve URL parsing [backport #842]

pkg/cache/cache_test.go

@@ -1050,21 +1050,21 @@ func TestDeadlock_NarInfo_Triggers_Nar_Refetch(t *testing.T) {
 	// NarInfoHash is 32 chars.
 	// narURL.Hash comes from URL.
 	// We want narURL.Hash == NarInfoHash.
-	collisionHash := "11111111111111111111111111111111"
+	collisionHash := "1111111111111111111111111111111111111111111111111111"
 
 	entry := testdata.Entry{
 		NarInfoHash:    collisionHash,
 		NarHash:        collisionHash,
 		NarCompression: "none",
-		NarInfoText: `StorePath: /nix/store/11111111111111111111111111111111-test-1.0
-URL: nar/11111111111111111111111111111111.nar
+		NarInfoText: `StorePath: /nix/store/1111111111111111111111111111111111111111111111111111-test-1.0
+URL: nar/1111111111111111111111111111111111111111111111111111.nar
 Compression: none
 FileHash: sha256:1111111111111111111111111111111111111111111111111111
 FileSize: 123
 NarHash: sha256:1111111111111111111111111111111111111111111111111111
 NarSize: 123
-References: aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa-dummy
-Deriver: dddddddddddddddddddddddddddddddd-test-1.0.drv
+References: aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa-dummy
+Deriver: dddddddddddddddddddddddddddddddddddddddddddddddddddd-test-1.0.drv
 Sig: cache.nixos.org-1:MadTCU1OSFCGUw4aqCKpLCZJpqBc7AbLvO7wgdlls0eq1DwaSnF/82SZE+wJGEiwlHbnZR+14daSaec0W3XoBQ==
 `,
 		NarText: "content-of-the-nar",

pkg/nar/filepath_test.go

@@ -19,8 +19,16 @@ func TestNarFilePath(t *testing.T) {
 		compression string
 		path        string
 	}{
-		{hash: "abc123", compression: "", path: filepath.Join("a", "ab", "abc123.nar")},
-		{hash: "def456", compression: "xz", path: filepath.Join("d", "de", "def456.nar.xz")},
+		{
+			hash:        "1mb5fxh7nzbx1b2q40bgzwjnjh8xqfap9mfnfqxlvvgvdyv8xwps",
+			compression: "",
+			path:        filepath.Join("1", "1m", "1mb5fxh7nzbx1b2q40bgzwjnjh8xqfap9mfnfqxlvvgvdyv8xwps.nar"),
+		},
+		{
+			hash:        "1mb5fxh7nzbx1b2q40bgzwjnjh8xqfap9mfnfqxlvvgvdyv8xwps",
+			compression: "xz",
+			path:        filepath.Join("1", "1m", "1mb5fxh7nzbx1b2q40bgzwjnjh8xqfap9mfnfqxlvvgvdyv8xwps.nar.xz"),
+		},
 	}
 
 	for _, test := range []string{"", "a", "ab"} {

pkg/nar/hash.go

@@ -3,17 +3,23 @@ package nar
 import (
 	"errors"
 	"regexp"
+
+	"github.com/kalbasit/ncps/pkg/narinfo"
 )
 
+// NormalizedHashPattern defines the valid characters for a Nix32 encoded hash.
+// Nix32 uses a 32-character alphabet excluding 'e', 'o', 'u', and 't'.
+// Valid characters: 0-9, a-d, f-n, p-s, v-z
+// Hashes must be exactly 52 characters long.
+const NormalizedHashPattern = `[0-9a-df-np-sv-z]{52}`
+
+const HashPattern = `(` + narinfo.HashPattern + `-)?` + NormalizedHashPattern
+
 var (
 	// ErrInvalidHash is returned if the hash is not valid.
 	ErrInvalidHash = errors.New("invalid nar hash")
 
-	// narHashPattern defines the valid characters for a nar hash.
-	//nolint:gochecknoglobals // This is used in other regexes to ensure they validate the same thing.
-	narHashPattern = `[a-z0-9_-]+`
-
-	narHashRegexp = regexp.MustCompile(`^(` + narHashPattern + `)$`)
+	narHashRegexp = regexp.MustCompile(`^(` + HashPattern + `)$`)
 )
 
 func ValidateHash(hash string) error {

pkg/nar/url.go

@@ -15,8 +15,8 @@ var (
 	// ErrInvalidURL is returned if the regexp did not match the given URL.
 	ErrInvalidURL = errors.New("invalid nar URL")
 
-	// https://regex101.com/r/yPwxpw/4
-	narRegexp = regexp.MustCompile(`^nar/(` + narHashPattern + `)\.nar(\.([a-z0-9]+))?(\?([a-z0-9=&]*))?$`)
+	// hashValidationRegexp validates that a string matches the HashPattern.
+	hashValidationRegexp = regexp.MustCompile(`^(` + HashPattern + `)$`)
 )
 
 // URL represents a nar URL.
@@ -27,29 +27,65 @@ type URL struct {
 }
 
 // ParseURL parses a nar URL (as present in narinfo) and returns its components.
+// It accepts URLs in the format: [path/]<hash>.nar[.<compression>][?query]
+// The hash must match HashPattern. This implementation is flexible about the
+// directory structure - only the filename matters, not the "nar/" prefix.
 func ParseURL(u string) (URL, error) {
-	if u == "" || !strings.HasPrefix(u, "nar/") {
+	if u == "" {
 		return URL{}, ErrInvalidURL
 	}
 
-	sm := narRegexp.FindStringSubmatch(u)
-	if len(sm) != 6 {
+	// Separate the query string from the path
+	pathPart, rawQuery, _ := strings.Cut(u, "?")
+
+	// Get the filename (last component of the path)
+	filename := filepath.Base(pathPart)
+	if filename == "" || filename == "." {
+		return URL{}, ErrInvalidURL
+	}
+
+	// The filename must contain ".nar" followed by optional compression extension
+	// Format: hash.nar[.compression]
+	// Everything before .nar is the hash, everything after is optional compression
+	hash, afterNar, found := strings.Cut(filename, ".nar")
+	if !found || hash == "" {
 		return URL{}, ErrInvalidURL
 	}
 
-	nu := URL{Hash: sm[1]}
+	// Validate that the hash matches HashPattern before processing further
+	if !hashValidationRegexp.MatchString(hash) {
+		return URL{}, ErrInvalidURL
+	}
 
-	var err error
+	// Extract compression extension (e.g., ".bz2" -> "bz2", "" -> "")
+	var compression string
 
-	if nu.Compression, err = CompressionTypeFromExtension(sm[3]); err != nil {
+	if afterNar != "" {
+		// afterNar should start with a dot
+		if !strings.HasPrefix(afterNar, ".") {
+			return URL{}, ErrInvalidURL
+		}
+
+		compression = afterNar[1:] // remove leading dot
+	}
+
+	// Determine compression type
+	ct, err := CompressionTypeFromExtension(compression)
+	if err != nil {
 		return URL{}, fmt.Errorf("error computing the compression type: %w", err)
 	}
 
-	if nu.Query, err = url.ParseQuery(sm[5]); err != nil {
+	// Parse the query string if present
+	query, err := url.ParseQuery(rawQuery)
+	if err != nil {
 		return URL{}, fmt.Errorf("error parsing the RawQuery as url.Values: %w", err)
 	}
 
-	return nu, nil
+	return URL{
+		Hash:        hash,
+		Compression: ct,
+		Query:       query,
+	}, nil
 }
 
 // NewLogger returns a new logger with the right fields.

pkg/nar/url_test.go

@@ -158,7 +158,7 @@ func TestParseURL(t *testing.T) {
 
 			narURL, err := nar.ParseURL(test.url)
 
-			if assert.ErrorIs(t, test.err, err) {
+			if assert.ErrorIs(t, err, test.err) {
 				assert.Equal(t, test.narURL, narURL)
 			}
 		})

pkg/storage/s3/s3.go

@@ -6,6 +6,7 @@ import (
 	"errors"
 	"fmt"
 	"io"
+	"net/http"
 	"path"
 	"strings"
 
@@ -66,6 +67,9 @@ type Config struct {
 	// Set to true for MinIO and other S3-compatible services
 	// Set to false for AWS S3 (default)
 	ForcePathStyle bool
+
+	// Transport is the HTTP transport to use for S3 requests.
+	Transport http.RoundTripper
 }
 
 // Store represents an S3 store and implements storage.Store.
@@ -96,6 +100,7 @@ func New(ctx context.Context, cfg Config) (*Store, error) {
 		Secure:       useSSL,
 		Region:       cfg.Region,
 		BucketLookup: bucketLookup,
+		Transport:    cfg.Transport,
 	})
 	if err != nil {
 		return nil, fmt.Errorf("error creating MinIO client: %w", err)