From a1bd247648aacd68a87fba35311ec77dd71dd395 Mon Sep 17 00:00:00 2001 From: Wael Nasreddine Date: Wed, 11 Feb 2026 18:39:48 -0800 Subject: [PATCH] fix: harden NAR hash validation and improve URL parsing [backport #842] This change strengthens the validation of NAR hashes by enforcing a strict Nix32 format (52 characters) and optional narinfo hash prefix. The ParseURL function has been rewritten to be more robust, separating the hash and compression components more reliably and validating the hash before further processing. Key improvements: - Defined NormalizedHashPattern and HashPattern for precise hash validation. - Enhanced ParseURL to correctly handle various NAR URL formats including those with query parameters. - Improved the Normalize method in the URL struct to better handle narinfo hash prefixes and sanitize the hash against path traversal. - Updated all relevant tests to use valid Nix32 hashes and removed redundant test cases. - Ensured consistent URL construction in JoinURL by correctly handling paths and query parameters. This hardening prevents potential security issues related to invalid or malicious NAR URLs and ensures consistent behavior across the application. (cherry picked from commit 2bb8649e9ede4161b25b801fcffe5b795728aa2d) --- pkg/cache/cache_test.go | 10 +- pkg/nar/filepath_test.go | 12 +- pkg/nar/hash.go | 16 +- pkg/nar/url.go | 56 ++- pkg/nar/url_test.go | 2 +- pkg/storage/s3/s3.go | 5 + pkg/storage/s3/s3_test.go | 749 ++++++++++++++++++++++++++++++++++++++ 7 files changed, 827 insertions(+), 23 deletions(-) diff --git a/pkg/cache/cache_test.go b/pkg/cache/cache_test.go index 46040c4c..130bfe98 100644 --- a/pkg/cache/cache_test.go +++ b/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", diff --git a/pkg/nar/filepath_test.go b/pkg/nar/filepath_test.go index c5721769..232e8912 100644 --- a/pkg/nar/filepath_test.go +++ b/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"} { diff --git a/pkg/nar/hash.go b/pkg/nar/hash.go index 38fe8a2e..8f9c7564 100644 --- a/pkg/nar/hash.go +++ b/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 { diff --git a/pkg/nar/url.go b/pkg/nar/url.go index 57cb2b2a..5f6619d5 100644 --- a/pkg/nar/url.go +++ b/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/].nar[.][?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. diff --git a/pkg/nar/url_test.go b/pkg/nar/url_test.go index 74c91ab5..a9141d06 100644 --- a/pkg/nar/url_test.go +++ b/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) } }) diff --git a/pkg/storage/s3/s3.go b/pkg/storage/s3/s3.go index 59d59a48..22e722e2 100644 --- a/pkg/storage/s3/s3.go +++ b/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) diff --git a/pkg/storage/s3/s3_test.go b/pkg/storage/s3/s3_test.go index 18739ea8..7691ba5d 100644 --- a/pkg/storage/s3/s3_test.go +++ b/pkg/storage/s3/s3_test.go @@ -2,7 +2,10 @@ package s3_test import ( "context" + "errors" + "fmt" "io" + "net/http" "os" "strings" "testing" @@ -20,6 +23,23 @@ import ( "github.com/kalbasit/ncps/testhelper" ) +//nolint:gochecknoglobals +var ( + errConnectionFailed = errors.New("connection failed") + errListObjectsFailed = errors.New("list objects failed") + errCallbackFailed = errors.New("callback failed") + errDeleteFailed = errors.New("delete failed") + errGetFailed = errors.New("get failed") + errPutFailed = errors.New("put failed") + + s3LocationResponseString = `` + + `us-west-2` + s3ListObjectsResponse = `` + + `` + + `test-bucketstore/narinfo/valid.narinfo` + s3NoSuchKey = "NoSuchKey" +) + const cacheName = "cache.example.com" func TestValidateConfig(t *testing.T) { @@ -233,6 +253,735 @@ func TestNew(t *testing.T) { }) } +type roundTripperFunc func(*http.Request) (*http.Response, error) + +func (f roundTripperFunc) RoundTrip(req *http.Request) (*http.Response, error) { + return f(req) +} + +func s3NotFoundResponse(code, message string) (*http.Response, error) { + body := fmt.Sprintf(` + + %s + %s + 4442587FB7D0A2F9 + nmIPG6bRoc0OcSR89Our983D5z77Fv9A= +`, code, message) + + header := make(http.Header) + header.Set("Content-Type", "application/xml") + header.Set("Last-Modified", "Mon, 02 Jan 2006 15:04:05 GMT") + + return &http.Response{ + StatusCode: http.StatusNotFound, + Header: header, + Body: io.NopCloser(strings.NewReader(body)), + }, nil +} + +func s3OKResponse(body string) (*http.Response, error) { + header := make(http.Header) + header.Set("Last-Modified", "Mon, 02 Jan 2006 15:04:05 GMT") + + return &http.Response{ + StatusCode: http.StatusOK, + Header: header, + Body: io.NopCloser(strings.NewReader(body)), + }, nil +} + +func TestBucketAccess_ErrorPaths(t *testing.T) { + t.Parallel() + + ctx := newContext() + cfg := s3.Config{ + Bucket: "test-bucket", + Endpoint: "http://localhost:9000", + AccessKeyID: "minioadmin", + SecretAccessKey: "minioadmin", + } + + t.Run("BucketExists returns error", func(t *testing.T) { + t.Parallel() + + expectedErr := errConnectionFailed + cfgWithMock := cfg + cfgWithMock.Transport = roundTripperFunc(func(_ *http.Request) (*http.Response, error) { + return nil, expectedErr + }) + + _, err := s3.New(ctx, cfgWithMock) + require.Error(t, err) + assert.Contains(t, err.Error(), "connection failed") + }) + + t.Run("bucket does not exist", func(t *testing.T) { + t.Parallel() + + cfgWithMock := cfg + cfgWithMock.Transport = roundTripperFunc(func(req *http.Request) (*http.Response, error) { + // BucketExists/GetBucketLocation check + if req.URL.Query().Has("location") && strings.Contains(req.URL.Path, "test-bucket") { + return s3NotFoundResponse("NoSuchBucket", "The specified bucket does not exist.") + } + // Fallback for HEAD + if req.Method == http.MethodHead && strings.HasSuffix(req.URL.Path, "test-bucket") { + return s3NotFoundResponse("NoSuchBucket", "The specified bucket does not exist.") + } + + return s3OKResponse("") + }) + + _, err := s3.New(ctx, cfgWithMock) + assert.ErrorIs(t, err, s3.ErrBucketNotFound) + }) +} + +func TestWalkNarInfos_ErrorPaths(t *testing.T) { + t.Parallel() + + ctx := newContext() + cfg := s3.Config{ + Bucket: "test-bucket", + Endpoint: "http://localhost:9000", + AccessKeyID: "minioadmin", + SecretAccessKey: "minioadmin", + } + + t.Run("ListObjects returns error", func(t *testing.T) { + t.Parallel() + + expectedErr := errListObjectsFailed + cfgWithMock := cfg + cfgWithMock.Transport = roundTripperFunc(func(req *http.Request) (*http.Response, error) { + // BucketExists check in New + if req.URL.Query().Has("location") && strings.Contains(req.URL.Path, "test-bucket") { + return s3OKResponse(s3LocationResponseString) + } + + // Then it lists objects + if req.Method == http.MethodGet && strings.Contains(req.URL.Path, "test-bucket") { + return nil, expectedErr + } + + return s3OKResponse("") + }) + + store, err := s3.New(ctx, cfgWithMock) + require.NoError(t, err) + + err = store.WalkNarInfos(ctx, func(_ string) error { + return nil + }) + require.Error(t, err) + assert.Contains(t, err.Error(), "list objects failed") + }) +} + +func TestWalkNarInfos_Structure(t *testing.T) { + t.Parallel() + + ctx := newContext() + cfg := s3.Config{ + Bucket: "test-bucket", + Endpoint: "http://localhost:9000", + AccessKeyID: "minioadmin", + SecretAccessKey: "minioadmin", + } + + t.Run("WalkNarInfos handles valid and invalid keys and callback errors", func(t *testing.T) { + t.Parallel() + + cfgWithMock := cfg + cfgWithMock.Transport = roundTripperFunc(func(req *http.Request) (*http.Response, error) { + // BucketExists check in New + if req.URL.Query().Has("location") && strings.Contains(req.URL.Path, "test-bucket") { + return s3OKResponse(s3LocationResponseString) + } + + // ListObjects + if req.Method == http.MethodGet && strings.Contains(req.URL.Path, "test-bucket") { + return s3OKResponse(s3ListObjectsResponse) + } + + return s3OKResponse("") + }) + + store, err := s3.New(ctx, cfgWithMock) + require.NoError(t, err) + + t.Run("Filters non-narinfo keys", func(t *testing.T) { + t.Parallel() + + var visited []string + + err := store.WalkNarInfos(ctx, func(hash string) error { + visited = append(visited, hash) + + return nil + }) + require.NoError(t, err) + require.Len(t, visited, 1) + assert.Equal(t, "valid", visited[0]) + }) + + t.Run("Propagates callback error", func(t *testing.T) { + t.Parallel() + + err := store.WalkNarInfos(ctx, func(_ string) error { + return errCallbackFailed + }) + require.ErrorIs(t, err, errCallbackFailed) + }) + }) +} + +func TestHasNarInfo_ErrorPaths(t *testing.T) { + t.Parallel() + + ctx := newContext() + cfg := s3.Config{ + Bucket: "test-bucket", + Endpoint: "http://localhost:9000", + AccessKeyID: "minioadmin", + SecretAccessKey: "minioadmin", + } + + t.Run("HasNarInfo StatObject error", func(t *testing.T) { + t.Parallel() + + expectedErr := errConnectionFailed + cfgWithMock := cfg + cfgWithMock.Transport = roundTripperFunc(func(req *http.Request) (*http.Response, error) { + if req.URL.Query().Has("location") && strings.Contains(req.URL.Path, "test-bucket") { + return s3OKResponse(s3LocationResponseString) + } + + // StatObject (HEAD) fails with unexpected error + if req.Method == http.MethodHead && strings.Contains(req.URL.Path, ".narinfo") { + return nil, expectedErr + } + + return s3OKResponse("") + }) + + store, err := s3.New(ctx, cfgWithMock) + require.NoError(t, err) + + exists := store.HasNarInfo(ctx, "00ji9synj1r6h6sjw27wwv8fw98myxsg") + assert.False(t, exists) + }) + + t.Run("HasNarInfo invalid hash returns false", func(t *testing.T) { + t.Parallel() + + // No transport needed as checking happens before network call + // But New requires it for BucketExists + cfgWithMock := cfg + cfgWithMock.Transport = roundTripperFunc(func(req *http.Request) (*http.Response, error) { + if req.URL.Query().Has("location") && strings.Contains(req.URL.Path, "test-bucket") { + return s3OKResponse(s3LocationResponseString) + } + + return s3OKResponse("") + }) + + store, err := s3.New(ctx, cfgWithMock) + require.NoError(t, err) + + exists := store.HasNarInfo(ctx, "in") // invalid hash (too short) + assert.False(t, exists) + }) +} + +func TestDeleteNarInfo_ErrorPaths(t *testing.T) { + t.Parallel() + + ctx := newContext() + cfg := s3.Config{ + Bucket: "test-bucket", + Endpoint: "http://localhost:9000", + AccessKeyID: "minioadmin", + SecretAccessKey: "minioadmin", + } + + t.Run("DeleteNarInfo StatObject error", func(t *testing.T) { + t.Parallel() + + expectedErr := errConnectionFailed + cfgWithMock := cfg + cfgWithMock.Transport = roundTripperFunc(func(req *http.Request) (*http.Response, error) { + if req.URL.Query().Has("location") && strings.Contains(req.URL.Path, "test-bucket") { + return s3OKResponse(s3LocationResponseString) + } + + // StatObject (HEAD) fails with unexpected error + if req.Method == http.MethodHead && strings.Contains(req.URL.Path, ".narinfo") { + return nil, expectedErr + } + + return s3OKResponse("") + }) + + store, err := s3.New(ctx, cfgWithMock) + require.NoError(t, err) + + err = store.DeleteNarInfo(ctx, "00ji9synj1r6h6sjw27wwv8fw98myxsg") + require.Error(t, err) + assert.Contains(t, err.Error(), "error checking if narinfo exists") + }) + + t.Run("DeleteNarInfo RemoveObject error", func(t *testing.T) { + t.Parallel() + + expectedErr := errDeleteFailed + cfgWithMock := cfg + cfgWithMock.Transport = roundTripperFunc(func(req *http.Request) (*http.Response, error) { + if req.URL.Query().Has("location") && strings.Contains(req.URL.Path, "test-bucket") { + return s3OKResponse(s3LocationResponseString) + } + + // StatObject (HEAD) succeeds + if req.Method == http.MethodHead && strings.Contains(req.URL.Path, ".narinfo") { + return s3OKResponse("") + } + + // RemoveObject fails + if req.Method == http.MethodDelete && strings.Contains(req.URL.Path, ".narinfo") { + return nil, expectedErr + } + + return s3OKResponse("") + }) + + store, err := s3.New(ctx, cfgWithMock) + require.NoError(t, err) + + err = store.DeleteNarInfo(ctx, "00ji9synj1r6h6sjw27wwv8fw98myxsg") + require.Error(t, err) + assert.Contains(t, err.Error(), "error deleting narinfo from S3") + }) + + t.Run("DeleteNarInfo invalid hash returns error", func(t *testing.T) { + t.Parallel() + + cfgWithMock := cfg + cfgWithMock.Transport = roundTripperFunc(func(req *http.Request) (*http.Response, error) { + if req.URL.Query().Has("location") && strings.Contains(req.URL.Path, "test-bucket") { + return s3OKResponse(s3LocationResponseString) + } + + return s3OKResponse("") + }) + + store, err := s3.New(ctx, cfgWithMock) + require.NoError(t, err) + + err = store.DeleteNarInfo(ctx, "in") // invalid hash + require.Error(t, err) + }) +} + +func TestNarInfo_ErrorPaths(t *testing.T) { + t.Parallel() + + ctx := newContext() + cfg := s3.Config{ + Bucket: "test-bucket", + Endpoint: "http://localhost:9000", + AccessKeyID: "minioadmin", + SecretAccessKey: "minioadmin", + } + + t.Run("GetNarInfo results in 404", func(t *testing.T) { + t.Parallel() + + cfgWithMock := cfg + cfgWithMock.Transport = roundTripperFunc(func(req *http.Request) (*http.Response, error) { + if req.URL.Query().Has("location") && strings.Contains(req.URL.Path, "test-bucket") { + return s3OKResponse(s3LocationResponseString) + } + + if req.Method == http.MethodGet && strings.Contains(req.URL.Path, ".narinfo") { + return s3NotFoundResponse("NoSuchKey", "The specified key does not exist.") + } + + if req.Method == http.MethodHead && strings.Contains(req.URL.Path, ".narinfo") { + return s3NotFoundResponse("NoSuchKey", "The specified key does not exist.") + } + + return s3OKResponse("") + }) + + store, err := s3.New(ctx, cfgWithMock) + require.NoError(t, err) + + _, err = store.GetNarInfo(ctx, "00ji9synj1r6h6sjw27wwv8fw98myxsg") + assert.ErrorIs(t, err, storage.ErrNotFound) + }) + + t.Run("PutNarInfo returns error", func(t *testing.T) { + t.Parallel() + + expectedErr := errPutFailed + cfgWithMock := cfg + cfgWithMock.Transport = roundTripperFunc(func(req *http.Request) (*http.Response, error) { + if req.URL.Query().Has("location") && strings.Contains(req.URL.Path, "test-bucket") { + return s3OKResponse(s3LocationResponseString) + } + + if req.Method == http.MethodPut && strings.Contains(req.URL.Path, ".narinfo") { + return nil, expectedErr + } + + // StatObject (to check if it exists) + if req.Method == http.MethodHead && strings.Contains(req.URL.Path, ".narinfo") { + return s3NotFoundResponse("NoSuchKey", "The specified key does not exist.") + } + + return s3OKResponse("") + }) + + store, err := s3.New(ctx, cfgWithMock) + require.NoError(t, err) + + ni, err := narinfo.Parse(strings.NewReader(testdata.Nar1.NarInfoText)) + require.NoError(t, err) + + err = store.PutNarInfo(ctx, testdata.Nar1.NarInfoHash, ni) + require.Error(t, err) + assert.Contains(t, err.Error(), "put failed") + }) + + t.Run("GetNarInfo with invalid hash returns error", func(t *testing.T) { + t.Parallel() + + cfgWithMock := cfg + cfgWithMock.Transport = roundTripperFunc(func(req *http.Request) (*http.Response, error) { + if req.URL.Query().Has("location") && strings.Contains(req.URL.Path, "test-bucket") { + return s3OKResponse(s3LocationResponseString) + } + + return s3OKResponse("") + }) + + store, err := s3.New(ctx, cfgWithMock) + require.NoError(t, err) + + _, err = store.GetNarInfo(ctx, "in") // invalid hash + require.Error(t, err) + }) + + t.Run("PutNarInfo with invalid hash returns error", func(t *testing.T) { + t.Parallel() + + cfgWithMock := cfg + cfgWithMock.Transport = roundTripperFunc(func(req *http.Request) (*http.Response, error) { + if req.URL.Query().Has("location") && strings.Contains(req.URL.Path, "test-bucket") { + return s3OKResponse(s3LocationResponseString) + } + + return s3OKResponse("") + }) + + store, err := s3.New(ctx, cfgWithMock) + require.NoError(t, err) + + ni := &narinfo.NarInfo{} + err = store.PutNarInfo(ctx, "in", ni) // invalid hash + require.Error(t, err) + }) +} + +func TestHasNar_ErrorPaths(t *testing.T) { + t.Parallel() + + ctx := newContext() + cfg := s3.Config{ + Bucket: "test-bucket", + Endpoint: "http://localhost:9000", + AccessKeyID: "minioadmin", + SecretAccessKey: "minioadmin", + } + + t.Run("HasNar StatObject error", func(t *testing.T) { + t.Parallel() + + expectedErr := errConnectionFailed + cfgWithMock := cfg + cfgWithMock.Transport = roundTripperFunc(func(req *http.Request) (*http.Response, error) { + if req.URL.Query().Has("location") && strings.Contains(req.URL.Path, "test-bucket") { + return s3OKResponse(s3LocationResponseString) + } + + // StatObject (HEAD) fails with unexpected error + if req.Method == http.MethodHead && strings.Contains(req.URL.Path, "store/nar") { + return nil, expectedErr + } + + return s3OKResponse("") + }) + + store, err := s3.New(ctx, cfgWithMock) + require.NoError(t, err) + + narURL := nar.URL{Hash: "00ji9synj1r6h6sjw27wwv8fw98myxsg92q5ma1pvrbmh451kc27", Compression: "none"} + exists := store.HasNar(ctx, narURL) + assert.False(t, exists) + }) + + t.Run("HasNar invalid URL returns false", func(t *testing.T) { + t.Parallel() + + cfgWithMock := cfg + cfgWithMock.Transport = roundTripperFunc(func(req *http.Request) (*http.Response, error) { + if req.URL.Query().Has("location") && strings.Contains(req.URL.Path, "test-bucket") { + return s3OKResponse(s3LocationResponseString) + } + + return s3OKResponse("") + }) + + store, err := s3.New(ctx, cfgWithMock) + require.NoError(t, err) + + narURL := nar.URL{Hash: "in", Compression: "none"} // invalid hash + exists := store.HasNar(ctx, narURL) + assert.False(t, exists) + }) +} + +func TestPutNar_ErrorPaths(t *testing.T) { + t.Parallel() + + ctx := newContext() + cfg := s3.Config{ + Bucket: "test-bucket", + Endpoint: "http://localhost:9000", + AccessKeyID: "minioadmin", + SecretAccessKey: "minioadmin", + } + + t.Run("PutNar StatObject error", func(t *testing.T) { + t.Parallel() + + expectedErr := errConnectionFailed + cfgWithMock := cfg + cfgWithMock.Transport = roundTripperFunc(func(req *http.Request) (*http.Response, error) { + if req.URL.Query().Has("location") && strings.Contains(req.URL.Path, "test-bucket") { + return s3OKResponse(s3LocationResponseString) + } + + // StatObject (HEAD) fails with unexpected error + if req.Method == http.MethodHead && strings.Contains(req.URL.Path, "store/nar") { + return nil, expectedErr + } + + return s3OKResponse("") + }) + + store, err := s3.New(ctx, cfgWithMock) + require.NoError(t, err) + + narURL := nar.URL{Hash: "00ji9synj1r6h6sjw27wwv8fw98myxsg92q5ma1pvrbmh451kc27", Compression: "none"} + _, err = store.PutNar(ctx, narURL, strings.NewReader("content")) + require.Error(t, err) + assert.Contains(t, err.Error(), "error checking if nar exists") + }) + + t.Run("PutNar PutObject error", func(t *testing.T) { + t.Parallel() + + expectedErr := errPutFailed + cfgWithMock := cfg + cfgWithMock.Transport = roundTripperFunc(func(req *http.Request) (*http.Response, error) { + if req.URL.Query().Has("location") && strings.Contains(req.URL.Path, "test-bucket") { + return s3OKResponse(s3LocationResponseString) + } + + // StatObject (HEAD) returns 404 (Not Found), allowing Put to proceed + if req.Method == http.MethodHead && strings.Contains(req.URL.Path, "store/nar") { + return s3NotFoundResponse(s3NoSuchKey, "The specified key does not exist.") + } + + // PutObject fails + if req.Method == http.MethodPut && strings.Contains(req.URL.Path, "store/nar") { + return nil, expectedErr + } + + return s3OKResponse("") + }) + + store, err := s3.New(ctx, cfgWithMock) + require.NoError(t, err) + + narURL := nar.URL{Hash: "00ji9synj1r6h6sjw27wwv8fw98myxsg92q5ma1pvrbmh451kc27", Compression: "none"} + _, err = store.PutNar(ctx, narURL, strings.NewReader("content")) + require.Error(t, err) + assert.Contains(t, err.Error(), "error putting nar to S3") + }) + + t.Run("PutNar with invalid URL returns error", func(t *testing.T) { + t.Parallel() + + cfgWithMock := cfg + cfgWithMock.Transport = roundTripperFunc(func(req *http.Request) (*http.Response, error) { + if req.URL.Query().Has("location") && strings.Contains(req.URL.Path, "test-bucket") { + return s3OKResponse(s3LocationResponseString) + } + + return s3OKResponse("") + }) + + store, err := s3.New(ctx, cfgWithMock) + require.NoError(t, err) + + narURL := nar.URL{Hash: "in", Compression: "none"} // invalid hash + _, err = store.PutNar(ctx, narURL, strings.NewReader("content")) + require.Error(t, err) + }) +} + +func TestNar_ErrorPaths(t *testing.T) { + t.Parallel() + + ctx := newContext() + cfg := s3.Config{ + Bucket: "test-bucket", + Endpoint: "http://localhost:9000", + AccessKeyID: "minioadmin", + SecretAccessKey: "minioadmin", + } + + t.Run("GetNar returns error", func(t *testing.T) { + t.Parallel() + + expectedErr := errGetFailed + cfgWithMock := cfg + cfgWithMock.Transport = roundTripperFunc(func(req *http.Request) (*http.Response, error) { + if req.URL.Query().Has("location") && strings.Contains(req.URL.Path, "test-bucket") { + return s3OKResponse(s3LocationResponseString) + } + + if req.Method == http.MethodGet && strings.Contains(req.URL.Path, "store/nar") { + return nil, expectedErr + } + + if req.Method == http.MethodHead && strings.Contains(req.URL.Path, "store/nar") { + return nil, expectedErr + } + + return s3OKResponse("") + }) + + store, err := s3.New(ctx, cfgWithMock) + require.NoError(t, err) + + narURL := nar.URL{Hash: "00ji9synj1r6h6sjw27wwv8fw98myxsg92q5ma1pvrbmh451kc27", Compression: "none"} + _, _, err = store.GetNar(ctx, narURL) + require.Error(t, err) + assert.Contains(t, err.Error(), "get failed") + }) + + t.Run("DeleteNar StatObject returns error", func(t *testing.T) { + t.Parallel() + + expectedErr := errConnectionFailed + cfgWithMock := cfg + cfgWithMock.Transport = roundTripperFunc(func(req *http.Request) (*http.Response, error) { + if req.URL.Query().Has("location") && strings.Contains(req.URL.Path, "test-bucket") { + return s3OKResponse(s3LocationResponseString) + } + + // StatObject (HEAD) fails with unexpected error + if req.Method == http.MethodHead && strings.Contains(req.URL.Path, "store/nar") { + return nil, expectedErr + } + + return s3OKResponse("") + }) + + store, err := s3.New(ctx, cfgWithMock) + require.NoError(t, err) + + narURL := nar.URL{Hash: "00ji9synj1r6h6sjw27wwv8fw98myxsg92q5ma1pvrbmh451kc27", Compression: "none"} + err = store.DeleteNar(ctx, narURL) + require.Error(t, err) + assert.Contains(t, err.Error(), "error checking if nar exists") + }) + + t.Run("DeleteNar returns error", func(t *testing.T) { + t.Parallel() + + expectedErr := errDeleteFailed + cfgWithMock := cfg + cfgWithMock.Transport = roundTripperFunc(func(req *http.Request) (*http.Response, error) { + if req.URL.Query().Has("location") && strings.Contains(req.URL.Path, "test-bucket") { + return s3OKResponse(s3LocationResponseString) + } + + if req.Method == http.MethodDelete && strings.Contains(req.URL.Path, "store/nar") { + return nil, expectedErr + } + + // StatObject (to check if it exists) + if req.Method == http.MethodHead && strings.Contains(req.URL.Path, "store/nar") { + return s3OKResponse("") + } + + return s3OKResponse("") + }) + + store, err := s3.New(ctx, cfgWithMock) + require.NoError(t, err) + + narURL := nar.URL{Hash: "00ji9synj1r6h6sjw27wwv8fw98myxsg92q5ma1pvrbmh451kc27", Compression: "none"} + err = store.DeleteNar(ctx, narURL) + require.Error(t, err) + assert.Contains(t, err.Error(), "delete failed") + }) + + t.Run("GetNar with invalid URL returns error", func(t *testing.T) { + t.Parallel() + + cfgWithMock := cfg + cfgWithMock.Transport = roundTripperFunc(func(req *http.Request) (*http.Response, error) { + if req.URL.Query().Has("location") && strings.Contains(req.URL.Path, "test-bucket") { + return s3OKResponse(s3LocationResponseString) + } + + return s3OKResponse("") + }) + + store, err := s3.New(ctx, cfgWithMock) + require.NoError(t, err) + + narURL := nar.URL{Hash: "in", Compression: "none"} // invalid hash + _, _, err = store.GetNar(ctx, narURL) + require.Error(t, err) + }) + + t.Run("DeleteNar with invalid URL returns error", func(t *testing.T) { + t.Parallel() + + cfgWithMock := cfg + cfgWithMock.Transport = roundTripperFunc(func(req *http.Request) (*http.Response, error) { + if req.URL.Query().Has("location") && strings.Contains(req.URL.Path, "test-bucket") { + return s3OKResponse(s3LocationResponseString) + } + + return s3OKResponse("") + }) + + store, err := s3.New(ctx, cfgWithMock) + require.NoError(t, err) + + narURL := nar.URL{Hash: "in", Compression: "none"} // invalid hash + err = store.DeleteNar(ctx, narURL) + require.Error(t, err) + }) +} + // Integration tests - require running MinIO instance //nolint:paralleltest