diff --git a/pkg/cache/cache.go b/pkg/cache/cache.go index abf85969..999424cd 100644 --- a/pkg/cache/cache.go +++ b/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) diff --git a/pkg/nar/hash.go b/pkg/nar/hash.go index 8f9c7564..00f86237 100644 --- a/pkg/nar/hash.go +++ b/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 { diff --git a/pkg/nar/url.go b/pkg/nar/url.go index 5f6619d5..a9a5a16b 100644 --- a/pkg/nar/url.go +++ b/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 } diff --git a/pkg/nar/url_test.go b/pkg/nar/url_test.go index 6bfde518..db401a21 100644 --- a/pkg/nar/url_test.go +++ b/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,19 +179,19 @@ 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", }, }, { @@ -198,9 +199,7 @@ func TestNormalize(t *testing.T) { 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) + } }) } } diff --git a/pkg/server/server.go b/pkg/server/server.go index ad29766f..5edb9c54 100644 --- a/pkg/server/server.go +++ b/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()) diff --git a/pkg/storage/local/local.go b/pkg/storage/local/local.go index c395cf30..791cb472 100644 --- a/pkg/storage/local/local.go +++ b/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 { diff --git a/pkg/storage/s3/s3.go b/pkg/storage/s3/s3.go index 2be6e885..909e9a66 100644 --- a/pkg/storage/s3/s3.go +++ b/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 } diff --git a/testdata/server.go b/testdata/server.go index 360eb899..18decef6 100644 --- a/testdata/server.go +++ b/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)