From 1ba4d473ad2e0b4341aa90f82c17cb4a00b63e8b Mon Sep 17 00:00:00 2001 From: arturrez Date: Mon, 9 Dec 2024 18:21:19 -0800 Subject: [PATCH 01/13] sdk public archive --- sdk/constants/constants.go | 3 +- sdk/publicarchive/downloader.go | 174 ++++++++++++++++++++++++++++++++ sdk/publicarchive/utils.go | 43 ++++++++ 3 files changed, 219 insertions(+), 1 deletion(-) create mode 100644 sdk/publicarchive/downloader.go create mode 100644 sdk/publicarchive/utils.go diff --git a/sdk/constants/constants.go b/sdk/constants/constants.go index d985e24b1..ed0ddd382 100644 --- a/sdk/constants/constants.go +++ b/sdk/constants/constants.go @@ -10,5 +10,6 @@ const ( APIRequestLargeTimeout = 2 * time.Minute // node - WriteReadUserOnlyPerms = 0o600 + WriteReadUserOnlyPerms = 0o600 + WriteReadUserOnlyDirPerms = 0o700 ) diff --git a/sdk/publicarchive/downloader.go b/sdk/publicarchive/downloader.go new file mode 100644 index 000000000..94ffc4b0c --- /dev/null +++ b/sdk/publicarchive/downloader.go @@ -0,0 +1,174 @@ +// Copyright (C) 2024, Ava Labs, Inc. All rights reserved. +// See the file LICENSE for licensing terms. + +package publicarchive + +import ( + "archive/tar" + "fmt" + "io" + "os" + "path/filepath" + "sync" + "time" + + "github.com/ava-labs/avalanche-cli/sdk/network" + "github.com/ava-labs/avalanchego/utils/constants" + "github.com/ava-labs/avalanchego/utils/logging" + "github.com/cavaliergopher/grab/v3" + "go.uber.org/zap" + + sdkConstants "github.com/ava-labs/avalanche-cli/sdk/constants" +) + +const ( + updateInterval = 500 * time.Millisecond + // public archive + PChainArchiveFuji = "https://avalanchego-public-database.avax-test.network/p-chain/avalanchego/data-tar/latest.tar" +) + +type Getter struct { + client *grab.Client + request *grab.Request + size int64 + bytesComplete int64 +} + +type Downloader struct { + getter Getter + logger logging.Logger + mutex *sync.Mutex +} + +// newGetter returns a new Getter +func newGetter(endpoint string, target string) (Getter, error) { + if request, err := grab.NewRequest(target, endpoint); err != nil { + return Getter{}, err + } else { + return Getter{ + client: grab.NewClient(), + request: request, + size: 0, + bytesComplete: 0, + }, nil + } +} + +// NewDownloader returns a new Downloader +// network: the network to download from ( fuji or mainnet). todo: add mainnet support +// target: the path to download to +// logLevel: the log level +func NewDownloader( + network network.Network, + target string, + logLevel logging.Level, +) (Downloader, error) { + tmpFile, err := os.CreateTemp("", "avalanche-cli-public-archive-*") + if err != nil { + return Downloader{}, err + } + + switch network.ID { + case constants.FujiID: + if getter, err := newGetter(PChainArchiveFuji, tmpFile.Name()); err != nil { + return Downloader{}, err + } else { + return Downloader{ + getter: getter, + logger: logging.NewLogger("public-archive-downloader", logging.NewWrappedCore(logLevel, os.Stdout, logging.JSON.ConsoleEncoder())), + }, nil + } + default: + return Downloader{}, fmt.Errorf("unsupported network ID: %d", network.ID) + } +} + +func (d Downloader) Download() error { + d.mutex.Lock() + defer d.mutex.Unlock() + d.logger.Info("Download started from", zap.String("url", d.getter.request.URL().String())) + + resp := d.getter.client.Do(d.getter.request) + d.getter.size = resp.Size() + d.logger.Debug("Download response received", + zap.String("status", resp.HTTPResponse.Status)) + t := time.NewTicker(updateInterval) + defer t.Stop() + + var wg sync.WaitGroup + wg.Add(1) + go func() { + defer wg.Done() + for range t.C { + d.getter.bytesComplete = resp.BytesComplete() + d.logger.Info("Download progress", + zap.Int64("bytesComplete", d.getter.bytesComplete), + zap.Int64("size", d.getter.size), + ) + } + }() + <-resp.Done // Wait for the download to finish + t.Stop() // Stop the ticker + wg.Wait() + + // check for errors + if err := resp.Err(); err != nil { + d.logger.Error("Download failed", zap.Error(err)) + return err + } + + d.logger.Info("Download saved to", zap.String("path", d.getter.request.Filename)) + return nil +} + +func (d Downloader) UnpackTo(targetDir string) error { + // prepare destination path + if err := os.MkdirAll(targetDir, sdkConstants.WriteReadUserOnlyDirPerms); err != nil { + d.logger.Error("Failed to create target directory", zap.Error(err)) + return err + } + tarFile, err := os.Open(d.getter.request.Filename) + if err != nil { + d.logger.Error("Failed to open tar file", zap.Error(err)) + return fmt.Errorf("failed to open tar file: %w", err) + } + defer tarFile.Close() + + tarReader := tar.NewReader(tarFile) + for { + header, err := tarReader.Next() + if err == io.EOF { + d.logger.Debug("End of archive reached") + break // End of archive + } + if err != nil { + d.logger.Error("Failed to read tar archive", zap.Error(err)) + return fmt.Errorf("error reading tar archive: %w", err) + } + + targetPath := filepath.Join(targetDir, header.Name) + switch header.Typeflag { + case tar.TypeDir: + d.logger.Debug("Creating directory", zap.String("path", targetPath)) + if err := os.MkdirAll(targetPath, os.FileMode(header.Mode)); err != nil { + d.logger.Error("Failed to create directory", zap.Error(err)) + return fmt.Errorf("failed to create directory: %w", err) + } + case tar.TypeReg: + d.logger.Debug("Creating file", zap.String("path", targetPath)) + outFile, err := os.Create(targetPath) + if err != nil { + d.logger.Error("Failed to create file", zap.Error(err)) + return fmt.Errorf("failed to create file: %w", err) + } + defer outFile.Close() + if _, err := io.Copy(outFile, tarReader); err != nil { + d.logger.Error("Failed to write file", zap.Error(err)) + return fmt.Errorf("failed to write file: %w", err) + } + default: + d.logger.Debug("Skipping file", zap.String("path", targetPath)) + } + } + return nil +} diff --git a/sdk/publicarchive/utils.go b/sdk/publicarchive/utils.go new file mode 100644 index 000000000..a19cd6783 --- /dev/null +++ b/sdk/publicarchive/utils.go @@ -0,0 +1,43 @@ +package publicarchive + +import ( + "archive/tar" + "fmt" + "os" +) + +// IsEmpty returns true if the Downloader is empty and not initialized +func (d Downloader) IsEmpty() bool { + return d.getter.client == nil +} + +// GetDownloadSize returns the size of the download +func (d Downloader) GetDownloadSize() int64 { + return d.getter.size +} + +// GetCurrentProgress returns the current download progress +func (d Downloader) GetCurrentProgress() int64 { + return d.getter.bytesComplete +} + +func isValidTar(filePath string) (bool, error) { + // Open the file + file, err := os.Open(filePath) + if err != nil { + return false, fmt.Errorf("failed to open file: %w", err) + } + defer file.Close() + + // Create a tar reader + tarReader := tar.NewReader(file) + + // Try reading the first header to validate the archive + _, err = tarReader.Next() + if err != nil { + return false, nil // Not a valid tar file if it cannot read a header + } + + // If we successfully read a header, it's a valid tar file + return true, nil +} From d2618b3c392b554510a9ddbabc6e6d6bfef91f00 Mon Sep 17 00:00:00 2001 From: arturrez Date: Mon, 9 Dec 2024 19:09:52 -0800 Subject: [PATCH 02/13] lint additional checks --- sdk/publicarchive/downloader.go | 32 ++++++++++++++++++++++++++++---- sdk/publicarchive/utils.go | 27 --------------------------- 2 files changed, 28 insertions(+), 31 deletions(-) diff --git a/sdk/publicarchive/downloader.go b/sdk/publicarchive/downloader.go index 94ffc4b0c..5da75cbf1 100644 --- a/sdk/publicarchive/downloader.go +++ b/sdk/publicarchive/downloader.go @@ -9,6 +9,7 @@ import ( "io" "os" "path/filepath" + "strings" "sync" "time" @@ -23,6 +24,7 @@ import ( const ( updateInterval = 500 * time.Millisecond + maxFileSize = 10 * 1024 * 1024 * 1024 // 10GB per file // public archive PChainArchiveFuji = "https://avalanchego-public-database.avax-test.network/p-chain/avalanchego/data-tar/latest.tar" ) @@ -60,7 +62,6 @@ func newGetter(endpoint string, target string) (Getter, error) { // logLevel: the log level func NewDownloader( network network.Network, - target string, logLevel logging.Level, ) (Downloader, error) { tmpFile, err := os.CreateTemp("", "avalanche-cli-public-archive-*") @@ -134,7 +135,8 @@ func (d Downloader) UnpackTo(targetDir string) error { } defer tarFile.Close() - tarReader := tar.NewReader(tarFile) + tarReader := tar.NewReader(io.LimitReader(tarFile, maxFileSize)) + extractedSize := int64(0) for { header, err := tarReader.Next() if err == io.EOF { @@ -146,7 +148,23 @@ func (d Downloader) UnpackTo(targetDir string) error { return fmt.Errorf("error reading tar archive: %w", err) } - targetPath := filepath.Join(targetDir, header.Name) + targetPath := filepath.Join(targetDir, filepath.Clean(header.Name)) + + // security checks + if !strings.HasPrefix(targetPath, filepath.Clean(targetDir)+string(os.PathSeparator)) { + d.logger.Error("Invalid file path", zap.String("path", targetPath)) + return fmt.Errorf("invalid file path: %s", targetPath) + } + if extractedSize+header.Size > maxFileSize { + d.logger.Error("File too large", zap.String("path", header.Name), zap.Int64("size", header.Size)) + return fmt.Errorf("file too large: %s", header.Name) + } + if strings.Contains(header.Name, "..") { + d.logger.Error("Invalid file path", zap.String("path", header.Name)) + return fmt.Errorf("invalid file path: %s", header.Name) + } + // end of security checks + switch header.Typeflag { case tar.TypeDir: d.logger.Debug("Creating directory", zap.String("path", targetPath)) @@ -162,10 +180,16 @@ func (d Downloader) UnpackTo(targetDir string) error { return fmt.Errorf("failed to create file: %w", err) } defer outFile.Close() - if _, err := io.Copy(outFile, tarReader); err != nil { + copied, err := io.CopyN(outFile, tarReader, header.Size) + if err != nil { d.logger.Error("Failed to write file", zap.Error(err)) return fmt.Errorf("failed to write file: %w", err) } + if copied < header.Size { + d.logger.Error("Incomplete file write", zap.String("path", targetPath)) + return fmt.Errorf("incomplete file write for %s", targetPath) + } + extractedSize += header.Size default: d.logger.Debug("Skipping file", zap.String("path", targetPath)) } diff --git a/sdk/publicarchive/utils.go b/sdk/publicarchive/utils.go index a19cd6783..d8d6950b3 100644 --- a/sdk/publicarchive/utils.go +++ b/sdk/publicarchive/utils.go @@ -1,11 +1,5 @@ package publicarchive -import ( - "archive/tar" - "fmt" - "os" -) - // IsEmpty returns true if the Downloader is empty and not initialized func (d Downloader) IsEmpty() bool { return d.getter.client == nil @@ -20,24 +14,3 @@ func (d Downloader) GetDownloadSize() int64 { func (d Downloader) GetCurrentProgress() int64 { return d.getter.bytesComplete } - -func isValidTar(filePath string) (bool, error) { - // Open the file - file, err := os.Open(filePath) - if err != nil { - return false, fmt.Errorf("failed to open file: %w", err) - } - defer file.Close() - - // Create a tar reader - tarReader := tar.NewReader(file) - - // Try reading the first header to validate the archive - _, err = tarReader.Next() - if err != nil { - return false, nil // Not a valid tar file if it cannot read a header - } - - // If we successfully read a header, it's a valid tar file - return true, nil -} From eb1359dc057215115a7bc35a85e77315ce5a9cef Mon Sep 17 00:00:00 2001 From: arturrez Date: Mon, 9 Dec 2024 19:23:39 -0800 Subject: [PATCH 03/13] add unittests --- sdk/publicarchive/downloader.go | 28 +++-- sdk/publicarchive/downloader_test.go | 125 +++++++++++++++++++++ sdk/publicarchive/{utils.go => helpers.go} | 0 3 files changed, 143 insertions(+), 10 deletions(-) create mode 100644 sdk/publicarchive/downloader_test.go rename sdk/publicarchive/{utils.go => helpers.go} (100%) diff --git a/sdk/publicarchive/downloader.go b/sdk/publicarchive/downloader.go index 5da75cbf1..dce6a41a5 100644 --- a/sdk/publicarchive/downloader.go +++ b/sdk/publicarchive/downloader.go @@ -96,21 +96,24 @@ func (d Downloader) Download() error { t := time.NewTicker(updateInterval) defer t.Stop() - var wg sync.WaitGroup - wg.Add(1) + done := make(chan struct{}) go func() { - defer wg.Done() - for range t.C { - d.getter.bytesComplete = resp.BytesComplete() - d.logger.Info("Download progress", - zap.Int64("bytesComplete", d.getter.bytesComplete), - zap.Int64("size", d.getter.size), - ) + defer close(done) + for { + select { + case <-t.C: + d.getter.bytesComplete = resp.BytesComplete() + d.logger.Info("Download progress", + zap.Int64("bytesComplete", d.getter.bytesComplete), + zap.Int64("size", d.getter.size)) + case <-resp.Done: + return + } } }() <-resp.Done // Wait for the download to finish t.Stop() // Stop the ticker - wg.Wait() + <-done // check for errors if err := resp.Err(); err != nil { @@ -173,6 +176,11 @@ func (d Downloader) UnpackTo(targetDir string) error { return fmt.Errorf("failed to create directory: %w", err) } case tar.TypeReg: + d.logger.Debug("Ensure parent directory exists for ", zap.String("path", targetPath)) + if err := os.MkdirAll(filepath.Dir(targetPath), os.FileMode(0755)); err != nil { + d.logger.Error("Failed to create parent directory for file", zap.Error(err)) + return fmt.Errorf("failed to create parent directory for file: %w", err) + } d.logger.Debug("Creating file", zap.String("path", targetPath)) outFile, err := os.Create(targetPath) if err != nil { diff --git a/sdk/publicarchive/downloader_test.go b/sdk/publicarchive/downloader_test.go new file mode 100644 index 000000000..7e5883002 --- /dev/null +++ b/sdk/publicarchive/downloader_test.go @@ -0,0 +1,125 @@ +package publicarchive + +import ( + "archive/tar" + "bytes" + "fmt" + "net/http" + "net/http/httptest" + "os" + "path/filepath" + "sync" + "testing" + + "github.com/cavaliergopher/grab/v3" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/ava-labs/avalanche-cli/sdk/network" + "github.com/ava-labs/avalanchego/utils/constants" + "github.com/ava-labs/avalanchego/utils/logging" +) + +func TestNewGetter(t *testing.T) { + endpoint := "http://example.com/file.tar" + target := "/tmp/file.tar" + + getter, err := newGetter(endpoint, target) + assert.NoError(t, err, "newGetter should not return an error") + assert.NotNil(t, getter.client, "getter client should not be nil") + assert.NotNil(t, getter.request, "getter request should not be nil") + assert.Equal(t, endpoint, getter.request.URL().String(), "getter request URL should match the input endpoint") +} + +func TestNewDownloader(t *testing.T) { + logLevel := logging.Info + + downloader, err := NewDownloader(network.Network{ID: constants.FujiID}, logLevel) + assert.NoError(t, err, "NewDownloader should not return an error") + assert.NotNil(t, downloader.logger, "downloader logger should not be nil") + assert.NotNil(t, downloader.getter.client, "downloader getter client should not be nil") +} + +func TestDownloader_Download(t *testing.T) { + // Mock server to simulate file download + mockData := []byte("mock file content") + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Write(mockData) + })) + defer server.Close() + + tmpFile, err := os.CreateTemp("", "test-download-*") + require.NoError(t, err) + defer os.Remove(tmpFile.Name()) + + getter, err := newGetter(server.URL, tmpFile.Name()) + require.NoError(t, err) + + logger := logging.NoLog{} + downloader := Downloader{ + getter: getter, + logger: logger, + mutex: &sync.Mutex{}, + } + + err = downloader.Download() + assert.NoError(t, err, "Download should not return an error") + content, err := os.ReadFile(tmpFile.Name()) + assert.NoError(t, err, "Reading downloaded file should not return an error") + assert.Equal(t, mockData, content, "Downloaded file content should match the mock data") +} + +func TestDownloader_UnpackTo(t *testing.T) { + // Create a mock tar file + var buf bytes.Buffer + tarWriter := tar.NewWriter(&buf) + + files := []struct { + Name, Body string + }{ + {"file1.txt", "This is file1"}, + {"dir/file2.txt", "This is file2"}, + } + for _, file := range files { + header := &tar.Header{ + Name: file.Name, + Size: int64(len(file.Body)), + Mode: 0600, + } + require.NoError(t, tarWriter.WriteHeader(header)) + _, err := tarWriter.Write([]byte(file.Body)) + require.NoError(t, err) + } + require.NoError(t, tarWriter.Close()) + + // Write tar file to a temporary file + tmpTar, err := os.CreateTemp("", "test-tar-*") + require.NoError(t, err) + defer os.Remove(tmpTar.Name()) + _, err = tmpTar.Write(buf.Bytes()) + require.NoError(t, err) + require.NoError(t, tmpTar.Close()) + + targetDir := t.TempDir() + + logger := logging.NoLog{} + downloader := Downloader{ + getter: Getter{ + request: &grab.Request{ + Filename: tmpTar.Name(), + }, + }, + logger: logger, + } + + err = downloader.UnpackTo(targetDir) + assert.NoError(t, err, "UnpackTo should not return an error") + + // Verify unpacked files + for _, file := range files { + filePath := filepath.Join(targetDir, file.Name) + content, err := os.ReadFile(filePath) + assert.NoError(t, err, fmt.Sprintf("Reading file %s should not return an error", file.Name)) + assert.Equal(t, file.Body, string(content), fmt.Sprintf("File content for %s should match", file.Name)) + } +} diff --git a/sdk/publicarchive/utils.go b/sdk/publicarchive/helpers.go similarity index 100% rename from sdk/publicarchive/utils.go rename to sdk/publicarchive/helpers.go From fd5ec0c51ca6dab97c24749816a2971a236026fa Mon Sep 17 00:00:00 2001 From: arturrez Date: Tue, 10 Dec 2024 10:51:26 -0800 Subject: [PATCH 04/13] fixes --- sdk/publicarchive/downloader.go | 9 ++++---- sdk/publicarchive/downloader_test.go | 33 ++++++++++++++-------------- sdk/publicarchive/helpers.go | 18 ++++++++++++++- 3 files changed, 38 insertions(+), 22 deletions(-) diff --git a/sdk/publicarchive/downloader.go b/sdk/publicarchive/downloader.go index dce6a41a5..515f67b75 100644 --- a/sdk/publicarchive/downloader.go +++ b/sdk/publicarchive/downloader.go @@ -26,7 +26,7 @@ const ( updateInterval = 500 * time.Millisecond maxFileSize = 10 * 1024 * 1024 * 1024 // 10GB per file // public archive - PChainArchiveFuji = "https://avalanchego-public-database.avax-test.network/p-chain/avalanchego/data-tar/latest.tar" + PChainArchiveFuji = "https://avalanchego-public-database.avax-test.network/testnet/p-chain/avalanchego/data-tar/latest.tar" ) type Getter struct { @@ -34,6 +34,7 @@ type Getter struct { request *grab.Request size int64 bytesComplete int64 + mutex *sync.RWMutex } type Downloader struct { @@ -90,7 +91,7 @@ func (d Downloader) Download() error { d.logger.Info("Download started from", zap.String("url", d.getter.request.URL().String())) resp := d.getter.client.Do(d.getter.request) - d.getter.size = resp.Size() + d.setDownloadSize(resp.Size()) d.logger.Debug("Download response received", zap.String("status", resp.HTTPResponse.Status)) t := time.NewTicker(updateInterval) @@ -102,7 +103,7 @@ func (d Downloader) Download() error { for { select { case <-t.C: - d.getter.bytesComplete = resp.BytesComplete() + d.setBytesComplete(resp.BytesComplete()) d.logger.Info("Download progress", zap.Int64("bytesComplete", d.getter.bytesComplete), zap.Int64("size", d.getter.size)) @@ -177,7 +178,7 @@ func (d Downloader) UnpackTo(targetDir string) error { } case tar.TypeReg: d.logger.Debug("Ensure parent directory exists for ", zap.String("path", targetPath)) - if err := os.MkdirAll(filepath.Dir(targetPath), os.FileMode(0755)); err != nil { + if err := os.MkdirAll(filepath.Dir(targetPath), os.FileMode(0o755)); err != nil { d.logger.Error("Failed to create parent directory for file", zap.Error(err)) return fmt.Errorf("failed to create parent directory for file: %w", err) } diff --git a/sdk/publicarchive/downloader_test.go b/sdk/publicarchive/downloader_test.go index 7e5883002..e2aecff52 100644 --- a/sdk/publicarchive/downloader_test.go +++ b/sdk/publicarchive/downloader_test.go @@ -12,7 +12,6 @@ import ( "testing" "github.com/cavaliergopher/grab/v3" - "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "github.com/ava-labs/avalanche-cli/sdk/network" @@ -25,26 +24,26 @@ func TestNewGetter(t *testing.T) { target := "/tmp/file.tar" getter, err := newGetter(endpoint, target) - assert.NoError(t, err, "newGetter should not return an error") - assert.NotNil(t, getter.client, "getter client should not be nil") - assert.NotNil(t, getter.request, "getter request should not be nil") - assert.Equal(t, endpoint, getter.request.URL().String(), "getter request URL should match the input endpoint") + require.NoError(t, err, "newGetter should not return an error") + require.NotNil(t, getter.client, "getter client should not be nil") + require.NotNil(t, getter.request, "getter request should not be nil") + require.Equal(t, endpoint, getter.request.URL().String(), "getter request URL should match the input endpoint") } func TestNewDownloader(t *testing.T) { logLevel := logging.Info downloader, err := NewDownloader(network.Network{ID: constants.FujiID}, logLevel) - assert.NoError(t, err, "NewDownloader should not return an error") - assert.NotNil(t, downloader.logger, "downloader logger should not be nil") - assert.NotNil(t, downloader.getter.client, "downloader getter client should not be nil") + require.NoError(t, err, "NewDownloader should not return an error") + require.NotNil(t, downloader.logger, "downloader logger should not be nil") + require.NotNil(t, downloader.getter.client, "downloader getter client should not be nil") } func TestDownloader_Download(t *testing.T) { // Mock server to simulate file download mockData := []byte("mock file content") - server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - w.Write(mockData) + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + _, _ = w.Write(mockData) })) defer server.Close() @@ -63,10 +62,10 @@ func TestDownloader_Download(t *testing.T) { } err = downloader.Download() - assert.NoError(t, err, "Download should not return an error") + require.NoError(t, err, "Download should not return an error") content, err := os.ReadFile(tmpFile.Name()) - assert.NoError(t, err, "Reading downloaded file should not return an error") - assert.Equal(t, mockData, content, "Downloaded file content should match the mock data") + require.NoError(t, err, "Reading downloaded file should not return an error") + require.Equal(t, mockData, content, "Downloaded file content should match the mock data") } func TestDownloader_UnpackTo(t *testing.T) { @@ -84,7 +83,7 @@ func TestDownloader_UnpackTo(t *testing.T) { header := &tar.Header{ Name: file.Name, Size: int64(len(file.Body)), - Mode: 0600, + Mode: 0o600, } require.NoError(t, tarWriter.WriteHeader(header)) _, err := tarWriter.Write([]byte(file.Body)) @@ -113,13 +112,13 @@ func TestDownloader_UnpackTo(t *testing.T) { } err = downloader.UnpackTo(targetDir) - assert.NoError(t, err, "UnpackTo should not return an error") + require.NoError(t, err, "UnpackTo should not return an error") // Verify unpacked files for _, file := range files { filePath := filepath.Join(targetDir, file.Name) content, err := os.ReadFile(filePath) - assert.NoError(t, err, fmt.Sprintf("Reading file %s should not return an error", file.Name)) - assert.Equal(t, file.Body, string(content), fmt.Sprintf("File content for %s should match", file.Name)) + require.NoError(t, err, fmt.Sprintf("Reading file %s should not return an error", file.Name)) + require.Equal(t, file.Body, string(content), fmt.Sprintf("File content for %s should match", file.Name)) } } diff --git a/sdk/publicarchive/helpers.go b/sdk/publicarchive/helpers.go index d8d6950b3..9241f4158 100644 --- a/sdk/publicarchive/helpers.go +++ b/sdk/publicarchive/helpers.go @@ -7,10 +7,26 @@ func (d Downloader) IsEmpty() bool { // GetDownloadSize returns the size of the download func (d Downloader) GetDownloadSize() int64 { + d.getter.mutex.RLock() + defer d.getter.mutex.RUnlock() return d.getter.size } +func (d Downloader) setDownloadSize(size int64) { + d.getter.mutex.Lock() + defer d.getter.mutex.Unlock() + d.getter.size = size +} + // GetCurrentProgress returns the current download progress -func (d Downloader) GetCurrentProgress() int64 { +func (d Downloader) GetBytesComplete() int64 { + d.getter.mutex.RLock() + defer d.getter.mutex.RUnlock() return d.getter.bytesComplete } + +func (d Downloader) setBytesComplete(progress int64) { + d.getter.mutex.Lock() + defer d.getter.mutex.Unlock() + d.getter.bytesComplete = progress +} From 167a6233bf7dae3e4cb15be1dd599c65675a4f7b Mon Sep 17 00:00:00 2001 From: arturrez Date: Tue, 10 Dec 2024 12:46:44 -0800 Subject: [PATCH 05/13] add readme add e2e unittest. fixes --- sdk/publicarchive/README.md | 55 ++++++++++++++++++++++++ sdk/publicarchive/downloader.go | 22 +++++----- sdk/publicarchive/downloader_test.go | 62 +++++++++++++++++++++++++--- sdk/publicarchive/helpers.go | 6 +++ 4 files changed, 128 insertions(+), 17 deletions(-) create mode 100644 sdk/publicarchive/README.md diff --git a/sdk/publicarchive/README.md b/sdk/publicarchive/README.md new file mode 100644 index 000000000..84675ee04 --- /dev/null +++ b/sdk/publicarchive/README.md @@ -0,0 +1,55 @@ +# Public Archive Downloader SDK + +This Go package provides a utility to download and extract tar archives from public URLs. It's tailored for downloading Avalanche network archives but can be adapted for other use cases. + + +## Features + +* Downloads files from predefined URLs. +* Tracks download progress and logs status updates. +* Safely unpacks .tar archives to a target directory. +* Includes security checks to prevent path traversal and manage large files. + +## Usage example + +``` +// Copyright (C) 2024, Ava Labs, Inc. All rights reserved +// See the file LICENSE for licensing terms. + +``` +package main + +import ( + "fmt" + "os" + + "github.com/ava-labs/avalanche-cli/sdk/network" + "github.com/ava-labs/avalanchego/utils/constants" + "github.com/ava-labs/avalanchego/utils/logging" + "github.com/your-repo-name/publicarchive" +) + +func main() { + // Initialize the downloader + downloader, err := publicarchive.NewDownloader(network.FujiNetwork(), logging.Debug) + if err != nil { + fmt.Printf("Failed to create downloader: %v\n", err) + os.Exit(1) + } + + // Start downloading + if err := downloader.Download(); err != nil { + fmt.Printf("Download failed: %v\n", err) + os.Exit(1) + } + + // Specify the target directory for unpacking + targetDir := "./extracted_files" + if err := downloader.UnpackTo(targetDir); err != nil { + fmt.Printf("Failed to unpack archive: %v\n", err) + os.Exit(1) + } + + fmt.Printf("Files successfully unpacked to %s\n", targetDir) +} +``` diff --git a/sdk/publicarchive/downloader.go b/sdk/publicarchive/downloader.go index 515f67b75..9752db2d8 100644 --- a/sdk/publicarchive/downloader.go +++ b/sdk/publicarchive/downloader.go @@ -38,9 +38,9 @@ type Getter struct { } type Downloader struct { - getter Getter - logger logging.Logger - mutex *sync.Mutex + getter Getter + logger logging.Logger + currentOp *sync.Mutex } // newGetter returns a new Getter @@ -53,6 +53,7 @@ func newGetter(endpoint string, target string) (Getter, error) { request: request, size: 0, bytesComplete: 0, + mutex: &sync.RWMutex{}, }, nil } } @@ -76,8 +77,9 @@ func NewDownloader( return Downloader{}, err } else { return Downloader{ - getter: getter, - logger: logging.NewLogger("public-archive-downloader", logging.NewWrappedCore(logLevel, os.Stdout, logging.JSON.ConsoleEncoder())), + getter: getter, + logger: logging.NewLogger("public-archive-downloader", logging.NewWrappedCore(logLevel, os.Stdout, logging.JSON.ConsoleEncoder())), + currentOp: &sync.Mutex{}, }, nil } default: @@ -86,8 +88,8 @@ func NewDownloader( } func (d Downloader) Download() error { - d.mutex.Lock() - defer d.mutex.Unlock() + d.currentOp.Lock() + defer d.currentOp.Unlock() d.logger.Info("Download started from", zap.String("url", d.getter.request.URL().String())) resp := d.getter.client.Do(d.getter.request) @@ -127,6 +129,8 @@ func (d Downloader) Download() error { } func (d Downloader) UnpackTo(targetDir string) error { + d.currentOp.Lock() + defer d.currentOp.Unlock() // prepare destination path if err := os.MkdirAll(targetDir, sdkConstants.WriteReadUserOnlyDirPerms); err != nil { d.logger.Error("Failed to create target directory", zap.Error(err)) @@ -155,10 +159,6 @@ func (d Downloader) UnpackTo(targetDir string) error { targetPath := filepath.Join(targetDir, filepath.Clean(header.Name)) // security checks - if !strings.HasPrefix(targetPath, filepath.Clean(targetDir)+string(os.PathSeparator)) { - d.logger.Error("Invalid file path", zap.String("path", targetPath)) - return fmt.Errorf("invalid file path: %s", targetPath) - } if extractedSize+header.Size > maxFileSize { d.logger.Error("File too large", zap.String("path", header.Name), zap.Int64("size", header.Size)) return fmt.Errorf("file too large: %s", header.Name) diff --git a/sdk/publicarchive/downloader_test.go b/sdk/publicarchive/downloader_test.go index e2aecff52..a9f20b0d4 100644 --- a/sdk/publicarchive/downloader_test.go +++ b/sdk/publicarchive/downloader_test.go @@ -47,22 +47,31 @@ func TestDownloader_Download(t *testing.T) { })) defer server.Close() + // Create a temporary file for download target tmpFile, err := os.CreateTemp("", "test-download-*") - require.NoError(t, err) + require.NoError(t, err, "Temporary file creation failed") defer os.Remove(tmpFile.Name()) + // Ensure newGetter initializes properly getter, err := newGetter(server.URL, tmpFile.Name()) - require.NoError(t, err) + require.NoError(t, err, "Getter initialization failed") + // Ensure the getter has a valid request + require.NotNil(t, getter.request, "Getter request is nil") + + // Initialize a no-op logger to avoid output logger := logging.NoLog{} downloader := Downloader{ - getter: getter, - logger: logger, - mutex: &sync.Mutex{}, + getter: getter, + logger: logger, + currentOp: &sync.Mutex{}, } + // Test the Download functionality err = downloader.Download() require.NoError(t, err, "Download should not return an error") + + // Validate the downloaded content content, err := os.ReadFile(tmpFile.Name()) require.NoError(t, err, "Reading downloaded file should not return an error") require.Equal(t, mockData, content, "Downloaded file content should match the mock data") @@ -108,7 +117,8 @@ func TestDownloader_UnpackTo(t *testing.T) { Filename: tmpTar.Name(), }, }, - logger: logger, + logger: logger, + currentOp: &sync.Mutex{}, } err = downloader.UnpackTo(targetDir) @@ -122,3 +132,43 @@ func TestDownloader_UnpackTo(t *testing.T) { require.Equal(t, file.Body, string(content), fmt.Sprintf("File content for %s should match", file.Name)) } } + +func TestDownloader_EndToEnd(t *testing.T) { + // Set up a temporary directory for testing + tmpDir := t.TempDir() + targetDir := filepath.Join(tmpDir, "extracted_files") + + // Configure the test network (Fuji in this case) + net := network.Network{ID: constants.FujiID} + + // Initialize a logger + logLevel := logging.Debug + + // Step 1: Create the downloader + downloader, err := NewDownloader(net, logLevel) + require.NoError(t, err, "Failed to initialize downloader") + + // Step 2: Start the download + t.Log("Starting download...") + err = downloader.Download() + require.NoError(t, err, "Download failed") + + // Step 3: Unpack the downloaded archive + t.Log("Unpacking downloaded archive...") + err = downloader.UnpackTo(targetDir) + require.NoError(t, err, "Failed to unpack archive") + + // Step 4: Validate the extracted files + t.Log("Validating extracted files...") + fileInfo, err := os.Stat(targetDir) + require.NoError(t, err, "Extracted directory does not exist") + require.True(t, fileInfo.IsDir(), "Extracted path is not a directory") + + // Check that at least one file is extracted + extractedFiles, err := os.ReadDir(targetDir) + require.NoError(t, err, "Failed to read extracted directory contents") + require.NotEmpty(t, extractedFiles, "No files extracted from archive") + + // Step 5: Clean up (optional since TempDir handles this automatically) + t.Log("Test completed successfully!") +} diff --git a/sdk/publicarchive/helpers.go b/sdk/publicarchive/helpers.go index 9241f4158..7ad672961 100644 --- a/sdk/publicarchive/helpers.go +++ b/sdk/publicarchive/helpers.go @@ -1,5 +1,7 @@ package publicarchive +import "os" + // IsEmpty returns true if the Downloader is empty and not initialized func (d Downloader) IsEmpty() bool { return d.getter.client == nil @@ -30,3 +32,7 @@ func (d Downloader) setBytesComplete(progress int64) { defer d.getter.mutex.Unlock() d.getter.bytesComplete = progress } + +func (d Downloader) CleanUp() { + _ = os.Remove(d.getter.request.Filename) +} From 20907d51927ee341b7a240a672fd6782c002c4a5 Mon Sep 17 00:00:00 2001 From: arturrez Date: Tue, 10 Dec 2024 14:37:06 -0800 Subject: [PATCH 06/13] r4 testing --- pkg/node/helper.go | 68 +++++++++++++++++++++++++++++++++ pkg/node/local.go | 8 ++++ sdk/publicarchive/downloader.go | 7 ++-- sdk/publicarchive/helpers.go | 12 +++++- 4 files changed, 91 insertions(+), 4 deletions(-) diff --git a/pkg/node/helper.go b/pkg/node/helper.go index 80d6b3b4f..9625ebbd6 100644 --- a/pkg/node/helper.go +++ b/pkg/node/helper.go @@ -7,6 +7,8 @@ import ( "encoding/json" "errors" "fmt" + "os" + "path/filepath" "regexp" "sort" "strconv" @@ -24,8 +26,11 @@ import ( "github.com/ava-labs/avalanche-cli/pkg/utils" "github.com/ava-labs/avalanche-cli/pkg/ux" "github.com/ava-labs/avalanche-cli/pkg/vm" + "github.com/ava-labs/avalanche-cli/sdk/network" + "github.com/ava-labs/avalanche-cli/sdk/publicarchive" "github.com/ava-labs/avalanche-network-runner/rpcpb" "github.com/ava-labs/avalanchego/api/info" + "github.com/ava-labs/avalanchego/utils/logging" "github.com/ava-labs/avalanchego/utils/set" ) @@ -554,3 +559,66 @@ func GetNodeData(endpoint string) ( "0x" + hex.EncodeToString(proofOfPossession.ProofOfPossession[:]), nil } + +func SeedClusterData( + clusterNetwork models.Network, + rootDir string, + nodeNames []string, +) error { + // only fuji is supported for now + if clusterNetwork.Kind != models.Fuji { + return fmt.Errorf("unsupported network: %s", clusterNetwork.Name()) + } + network := network.FujiNetwork() + ux.Logger.Info("downloading public archive for network %s", clusterNetwork.Name()) + publicArcDownloader, err := publicarchive.NewDownloader(network, logging.Verbo) // off as we run inside of the spinner + if err != nil { + return fmt.Errorf("failed to create public archive downloader for network %s: %w", clusterNetwork.Name(), err) + } + + if err := publicArcDownloader.Download(); err != nil { + return fmt.Errorf("failed to download public archive: %w", err) + } + // defer publicArcDownloader.CleanUp() + if path, err := publicArcDownloader.GetDownloadedFilePath(); err != nil { + return fmt.Errorf("failed to get downloaded file path: %w", err) + } else { + ux.Logger.Info("public archive downloaded to %s", path) + } + + wg := sync.WaitGroup{} + errChan := make(chan error, len(nodeNames)) + + for _, nodeName := range nodeNames { + wg.Add(1) + go func(nodeName string) { + defer wg.Done() + target := filepath.Join(rootDir, nodeName, "db") + ux.Logger.Info("unpacking public archive to %s", target) + if err := publicArcDownloader.UnpackTo(target); err != nil { + errChan <- fmt.Errorf("failed to unpack public archive: %w", err) + } + }(nodeName) + return nil + } + + wg.Wait() + close(errChan) + + if len(errChan) > 0 { + _ = CleanUpClusterNodeData(rootDir, nodeNames) + } + for err := range errChan { + return err + } + return nil +} + +func CleanUpClusterNodeData(rootDir string, nodesNames []string) error { + for _, nodeName := range nodesNames { + if err := os.RemoveAll(filepath.Join(rootDir, nodeName)); err != nil { + return err + } + } + return nil +} diff --git a/pkg/node/local.go b/pkg/node/local.go index 565e84821..16daaa162 100644 --- a/pkg/node/local.go +++ b/pkg/node/local.go @@ -416,6 +416,14 @@ func StartLocalNode( ux.Logger.PrintToUser("Starting local avalanchego node using root: %s ...", rootDir) spinSession := ux.NewUserSpinner() spinner := spinSession.SpinToUser("Booting Network. Wait until healthy...") + // preseed nodes data from public archive. ignore errors + nodeNames := []string{} + for i := 1; i <= int(numNodes); i++ { + nodeNames = append(nodeNames, fmt.Sprintf("node%d", i)) + } + err := SeedClusterData(network, rootDir, nodeNames) + ux.Logger.Info("seeding public archive data finished with error: %v[ignored]", err) + if _, err := cli.Start(ctx, avalancheGoBinPath, anrOpts...); err != nil { ux.SpinFailWithError(spinner, "", err) _ = DestroyLocalNode(app, clusterName) diff --git a/sdk/publicarchive/downloader.go b/sdk/publicarchive/downloader.go index 9752db2d8..248eb5dc3 100644 --- a/sdk/publicarchive/downloader.go +++ b/sdk/publicarchive/downloader.go @@ -88,9 +88,10 @@ func NewDownloader( } func (d Downloader) Download() error { + d.logger.Info("Download started from", zap.String("url", d.getter.request.URL().String())) + d.currentOp.Lock() defer d.currentOp.Unlock() - d.logger.Info("Download started from", zap.String("url", d.getter.request.URL().String())) resp := d.getter.client.Do(d.getter.request) d.setDownloadSize(resp.Size()) @@ -107,8 +108,8 @@ func (d Downloader) Download() error { case <-t.C: d.setBytesComplete(resp.BytesComplete()) d.logger.Info("Download progress", - zap.Int64("bytesComplete", d.getter.bytesComplete), - zap.Int64("size", d.getter.size)) + zap.Int64("bytesComplete", d.GetBytesComplete()), + zap.Int64("size", d.GetDownloadSize())) case <-resp.Done: return } diff --git a/sdk/publicarchive/helpers.go b/sdk/publicarchive/helpers.go index 7ad672961..923e94291 100644 --- a/sdk/publicarchive/helpers.go +++ b/sdk/publicarchive/helpers.go @@ -1,12 +1,22 @@ package publicarchive -import "os" +import ( + "fmt" + "os" +) // IsEmpty returns true if the Downloader is empty and not initialized func (d Downloader) IsEmpty() bool { return d.getter.client == nil } +func (d Downloader) GetDownloadedFilePath() (string, error) { + if d.GetBytesComplete() != d.GetDownloadSize() { + return "", fmt.Errorf("download is not completed") + } + return d.getter.request.Filename, nil +} + // GetDownloadSize returns the size of the download func (d Downloader) GetDownloadSize() int64 { d.getter.mutex.RLock() From 9ae7c69642cbb0b1e92b7dcfdb3b4ee57bddb91e Mon Sep 17 00:00:00 2001 From: arturrez Date: Tue, 10 Dec 2024 16:02:51 -0800 Subject: [PATCH 07/13] r4r --- pkg/node/helper.go | 39 ++++++++++++++++++++------------- pkg/node/local.go | 9 +++++++- sdk/publicarchive/downloader.go | 2 ++ 3 files changed, 34 insertions(+), 16 deletions(-) diff --git a/pkg/node/helper.go b/pkg/node/helper.go index 9625ebbd6..7deadda12 100644 --- a/pkg/node/helper.go +++ b/pkg/node/helper.go @@ -571,7 +571,7 @@ func SeedClusterData( } network := network.FujiNetwork() ux.Logger.Info("downloading public archive for network %s", clusterNetwork.Name()) - publicArcDownloader, err := publicarchive.NewDownloader(network, logging.Verbo) // off as we run inside of the spinner + publicArcDownloader, err := publicarchive.NewDownloader(network, logging.Off) // off as we run inside of the spinner if err != nil { return fmt.Errorf("failed to create public archive downloader for network %s: %w", clusterNetwork.Name(), err) } @@ -587,30 +587,39 @@ func SeedClusterData( } wg := sync.WaitGroup{} - errChan := make(chan error, len(nodeNames)) + mu := sync.Mutex{} + var firstErr error for _, nodeName := range nodeNames { + target := filepath.Join(rootDir, nodeName, "db") + ux.Logger.Info("unpacking public archive to %s", target) + + // Skip if target already exists + if _, err := os.Stat(target); err == nil { + ux.Logger.Info("data folder already exists at %s. Skipping...", target) + continue + } wg.Add(1) - go func(nodeName string) { + go func(target string) { defer wg.Done() - target := filepath.Join(rootDir, nodeName, "db") - ux.Logger.Info("unpacking public archive to %s", target) + if err := publicArcDownloader.UnpackTo(target); err != nil { - errChan <- fmt.Errorf("failed to unpack public archive: %w", err) + // Capture the first error encountered + mu.Lock() + if firstErr == nil { + firstErr = fmt.Errorf("failed to unpack public archive: %w", err) + _ = CleanUpClusterNodeData(rootDir, nodeNames) + } + mu.Unlock() } - }(nodeName) - return nil + }(target) } - wg.Wait() - close(errChan) - if len(errChan) > 0 { - _ = CleanUpClusterNodeData(rootDir, nodeNames) - } - for err := range errChan { - return err + if firstErr != nil { + return firstErr } + ux.Logger.PrintToUser("public archive unpacked to %s", rootDir) return nil } diff --git a/pkg/node/local.go b/pkg/node/local.go index 16daaa162..7ef1f09cc 100644 --- a/pkg/node/local.go +++ b/pkg/node/local.go @@ -335,6 +335,13 @@ func StartLocalNode( } } if network.Kind == models.Fuji { + // disable indexing for fuji + nodeConfig[config.IndexEnabledKey] = false + nodeConfigBytes, err := json.Marshal(nodeConfig) + if err != nil { + return err + } + nodeConfigStr = string(nodeConfigBytes) ux.Logger.PrintToUser(logging.Yellow.Wrap("Warning: Fuji Bootstrapping can take several minutes")) } if err := preLocalChecks(anrSettings, avaGoVersionSetting, useEtnaDevnet, globalNetworkFlags); err != nil { @@ -426,7 +433,7 @@ func StartLocalNode( if _, err := cli.Start(ctx, avalancheGoBinPath, anrOpts...); err != nil { ux.SpinFailWithError(spinner, "", err) - _ = DestroyLocalNode(app, clusterName) + //_ = DestroyLocalNode(app, clusterName) return fmt.Errorf("failed to start local avalanchego: %w", err) } ux.SpinComplete(spinner) diff --git a/sdk/publicarchive/downloader.go b/sdk/publicarchive/downloader.go index 248eb5dc3..b3d6e2eaa 100644 --- a/sdk/publicarchive/downloader.go +++ b/sdk/publicarchive/downloader.go @@ -200,9 +200,11 @@ func (d Downloader) UnpackTo(targetDir string) error { return fmt.Errorf("incomplete file write for %s", targetPath) } extractedSize += header.Size + d.logger.Debug("Written bytes", zap.Int64("bytes", extractedSize)) default: d.logger.Debug("Skipping file", zap.String("path", targetPath)) } } + d.logger.Info("Download unpacked to", zap.String("path", targetDir)) return nil } From 710408c5428d4f309afdc5696ad48bb4abf46259 Mon Sep 17 00:00:00 2001 From: arturrez Date: Tue, 10 Dec 2024 16:12:35 -0800 Subject: [PATCH 08/13] nits --- pkg/node/helper.go | 2 +- pkg/node/local.go | 7 ++++++- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/pkg/node/helper.go b/pkg/node/helper.go index 7deadda12..9bcf44c38 100644 --- a/pkg/node/helper.go +++ b/pkg/node/helper.go @@ -619,7 +619,7 @@ func SeedClusterData( if firstErr != nil { return firstErr } - ux.Logger.PrintToUser("public archive unpacked to %s", rootDir) + ux.Logger.PrintToUser("Public archive unpacked to: %s", rootDir) return nil } diff --git a/pkg/node/local.go b/pkg/node/local.go index 7ef1f09cc..7373524e3 100644 --- a/pkg/node/local.go +++ b/pkg/node/local.go @@ -429,7 +429,7 @@ func StartLocalNode( nodeNames = append(nodeNames, fmt.Sprintf("node%d", i)) } err := SeedClusterData(network, rootDir, nodeNames) - ux.Logger.Info("seeding public archive data finished with error: %v[ignored]", err) + ux.Logger.Info("seeding public archive data finished with error: %v. Ignored if any", err) if _, err := cli.Start(ctx, avalancheGoBinPath, anrOpts...); err != nil { ux.SpinFailWithError(spinner, "", err) @@ -489,6 +489,9 @@ func UpsizeLocalNode( nodeConfig = map[string]interface{}{} } nodeConfig[config.NetworkAllowPrivateIPsKey] = true + if network.Kind == models.Fuji { + nodeConfig[config.IndexEnabledKey] = false // disable index for Fuji + } nodeConfigBytes, err := json.Marshal(nodeConfig) if err != nil { return "", err @@ -570,6 +573,8 @@ func UpsizeLocalNode( spinSession := ux.NewUserSpinner() spinner := spinSession.SpinToUser("Creating new node with name %s on local machine", newNodeName) + err = SeedClusterData(network, rootDir, []string{newNodeName}) + ux.Logger.Info("seeding public archive data finished with error: %v. Ignored if any", err) // add new local node if _, err := cli.AddNode(ctx, newNodeName, avalancheGoBinPath, anrOpts...); err != nil { ux.SpinFailWithError(spinner, "", err) From 6f9cee24ea0cb63a59f723800965f5e71e44daac Mon Sep 17 00:00:00 2001 From: arturrez Date: Tue, 10 Dec 2024 16:20:04 -0800 Subject: [PATCH 09/13] put destroy node on error back --- pkg/node/local.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/node/local.go b/pkg/node/local.go index 7373524e3..947a69dac 100644 --- a/pkg/node/local.go +++ b/pkg/node/local.go @@ -433,7 +433,7 @@ func StartLocalNode( if _, err := cli.Start(ctx, avalancheGoBinPath, anrOpts...); err != nil { ux.SpinFailWithError(spinner, "", err) - //_ = DestroyLocalNode(app, clusterName) + _ = DestroyLocalNode(app, clusterName) return fmt.Errorf("failed to start local avalanchego: %w", err) } ux.SpinComplete(spinner) From a531f625c36ae7fb2d49cc8b63473e04c6fe0574 Mon Sep 17 00:00:00 2001 From: arturrez Date: Wed, 11 Dec 2024 12:06:55 -0800 Subject: [PATCH 10/13] add another security check --- sdk/publicarchive/downloader.go | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/sdk/publicarchive/downloader.go b/sdk/publicarchive/downloader.go index b3d6e2eaa..51c82cf89 100644 --- a/sdk/publicarchive/downloader.go +++ b/sdk/publicarchive/downloader.go @@ -157,7 +157,12 @@ func (d Downloader) UnpackTo(targetDir string) error { return fmt.Errorf("error reading tar archive: %w", err) } - targetPath := filepath.Join(targetDir, filepath.Clean(header.Name)) + relPath, err := filepath.Rel(targetDir, filepath.Join(targetDir, header.Name)) + if err != nil || strings.HasPrefix(relPath, "..") { + d.logger.Error("Invalid file path", zap.String("path", header.Name)) + return fmt.Errorf("invalid file path: %s", header.Name) + } + targetPath := filepath.Join(targetDir, relPath) // security checks if extractedSize+header.Size > maxFileSize { From d4e03c9464abfc720ea034813ee240fb578c662c Mon Sep 17 00:00:00 2001 From: arturrez Date: Wed, 11 Dec 2024 12:10:35 -0800 Subject: [PATCH 11/13] lint --- sdk/publicarchive/downloader.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sdk/publicarchive/downloader.go b/sdk/publicarchive/downloader.go index 51c82cf89..d8e88f3cf 100644 --- a/sdk/publicarchive/downloader.go +++ b/sdk/publicarchive/downloader.go @@ -157,7 +157,7 @@ func (d Downloader) UnpackTo(targetDir string) error { return fmt.Errorf("error reading tar archive: %w", err) } - relPath, err := filepath.Rel(targetDir, filepath.Join(targetDir, header.Name)) + relPath, err := filepath.Rel(targetDir, filepath.Join(targetDir, filepath.Clean(header.Name))) if err != nil || strings.HasPrefix(relPath, "..") { d.logger.Error("Invalid file path", zap.String("path", header.Name)) return fmt.Errorf("invalid file path: %s", header.Name) From dcb27807127a543d0e69767d707d0de75fef808d Mon Sep 17 00:00:00 2001 From: arturrez Date: Thu, 12 Dec 2024 11:38:20 -0800 Subject: [PATCH 12/13] add missing license --- sdk/publicarchive/downloader_test.go | 3 +++ sdk/publicarchive/helpers.go | 3 +++ 2 files changed, 6 insertions(+) diff --git a/sdk/publicarchive/downloader_test.go b/sdk/publicarchive/downloader_test.go index a9f20b0d4..6301ee921 100644 --- a/sdk/publicarchive/downloader_test.go +++ b/sdk/publicarchive/downloader_test.go @@ -1,3 +1,6 @@ +// Copyright (C) 2024, Ava Labs, Inc. All rights reserved. +// See the file LICENSE for licensing terms. + package publicarchive import ( diff --git a/sdk/publicarchive/helpers.go b/sdk/publicarchive/helpers.go index 923e94291..2f46029a9 100644 --- a/sdk/publicarchive/helpers.go +++ b/sdk/publicarchive/helpers.go @@ -1,3 +1,6 @@ +// Copyright (C) 2024, Ava Labs, Inc. All rights reserved. +// See the file LICENSE for licensing terms. + package publicarchive import ( From 6066e3a094d17b813f6f98a3c5c828c14c505df1 Mon Sep 17 00:00:00 2001 From: arturrez Date: Tue, 7 Jan 2025 12:45:22 -0800 Subject: [PATCH 13/13] address feedback --- pkg/node/helper.go | 12 ++--- pkg/node/local.go | 4 +- sdk/constants/constants.go | 4 +- sdk/key/soft_key.go | 2 +- sdk/publicarchive/downloader.go | 65 ++++++++++++++++++++++++---- sdk/publicarchive/downloader_test.go | 9 +--- sdk/publicarchive/helpers.go | 51 ---------------------- 7 files changed, 69 insertions(+), 78 deletions(-) delete mode 100644 sdk/publicarchive/helpers.go diff --git a/pkg/node/helper.go b/pkg/node/helper.go index 61d03a94e..47b96819f 100644 --- a/pkg/node/helper.go +++ b/pkg/node/helper.go @@ -561,7 +561,7 @@ func GetNodeData(endpoint string) ( nil } -func SeedClusterData( +func DownloadPublicArchive( clusterNetwork models.Network, rootDir string, nodeNames []string, @@ -572,7 +572,7 @@ func SeedClusterData( } network := network.FujiNetwork() ux.Logger.Info("downloading public archive for network %s", clusterNetwork.Name()) - publicArcDownloader, err := publicarchive.NewDownloader(network, logging.Off) // off as we run inside of the spinner + publicArcDownloader, err := publicarchive.NewDownloader(network, logging.NewLogger("public-archive-downloader", logging.NewWrappedCore(logging.Off, os.Stdout, logging.JSON.ConsoleEncoder()))) // off as we run inside of the spinner if err != nil { return fmt.Errorf("failed to create public archive downloader for network %s: %w", clusterNetwork.Name(), err) } @@ -580,8 +580,8 @@ func SeedClusterData( if err := publicArcDownloader.Download(); err != nil { return fmt.Errorf("failed to download public archive: %w", err) } - // defer publicArcDownloader.CleanUp() - if path, err := publicArcDownloader.GetDownloadedFilePath(); err != nil { + defer publicArcDownloader.CleanUp() + if path, err := publicArcDownloader.GetFilePath(); err != nil { return fmt.Errorf("failed to get downloaded file path: %w", err) } else { ux.Logger.Info("public archive downloaded to %s", path) @@ -609,7 +609,7 @@ func SeedClusterData( mu.Lock() if firstErr == nil { firstErr = fmt.Errorf("failed to unpack public archive: %w", err) - _ = CleanUpClusterNodeData(rootDir, nodeNames) + _ = cleanUpClusterNodeData(rootDir, nodeNames) } mu.Unlock() } @@ -624,7 +624,7 @@ func SeedClusterData( return nil } -func CleanUpClusterNodeData(rootDir string, nodesNames []string) error { +func cleanUpClusterNodeData(rootDir string, nodesNames []string) error { for _, nodeName := range nodesNames { if err := os.RemoveAll(filepath.Join(rootDir, nodeName)); err != nil { return err diff --git a/pkg/node/local.go b/pkg/node/local.go index 1a2d0a8ce..f5489fcf6 100644 --- a/pkg/node/local.go +++ b/pkg/node/local.go @@ -432,7 +432,7 @@ func StartLocalNode( for i := 1; i <= int(numNodes); i++ { nodeNames = append(nodeNames, fmt.Sprintf("node%d", i)) } - err := SeedClusterData(network, rootDir, nodeNames) + err := DownloadPublicArchive(network, rootDir, nodeNames) ux.Logger.Info("seeding public archive data finished with error: %v. Ignored if any", err) if _, err := cli.Start(ctx, avalancheGoBinPath, anrOpts...); err != nil { @@ -597,7 +597,7 @@ func UpsizeLocalNode( spinSession := ux.NewUserSpinner() spinner := spinSession.SpinToUser("Creating new node with name %s on local machine", newNodeName) - err = SeedClusterData(network, rootDir, []string{newNodeName}) + err = DownloadPublicArchive(network, rootDir, []string{newNodeName}) ux.Logger.Info("seeding public archive data finished with error: %v. Ignored if any", err) // add new local node if _, err := cli.AddNode(ctx, newNodeName, avalancheGoBinPath, anrOpts...); err != nil { diff --git a/sdk/constants/constants.go b/sdk/constants/constants.go index ed0ddd382..571651361 100644 --- a/sdk/constants/constants.go +++ b/sdk/constants/constants.go @@ -10,6 +10,6 @@ const ( APIRequestLargeTimeout = 2 * time.Minute // node - WriteReadUserOnlyPerms = 0o600 - WriteReadUserOnlyDirPerms = 0o700 + UserOnlyWriteReadPerms = 0o600 + UserOnlyWriteReadExecPerms = 0o700 ) diff --git a/sdk/key/soft_key.go b/sdk/key/soft_key.go index 9729748be..588e66a9b 100644 --- a/sdk/key/soft_key.go +++ b/sdk/key/soft_key.go @@ -280,7 +280,7 @@ func (m *SoftKey) PrivKeyHex() string { // Saves the private key to disk with hex encoding. func (m *SoftKey) Save(p string) error { - return os.WriteFile(p, []byte(m.PrivKeyHex()), constants.WriteReadUserOnlyPerms) + return os.WriteFile(p, []byte(m.PrivKeyHex()), constants.UserOnlyWriteReadPerms) } func (m *SoftKey) P(networkHRP string) (string, error) { diff --git a/sdk/publicarchive/downloader.go b/sdk/publicarchive/downloader.go index d8e88f3cf..016738f04 100644 --- a/sdk/publicarchive/downloader.go +++ b/sdk/publicarchive/downloader.go @@ -13,13 +13,12 @@ import ( "sync" "time" + "github.com/ava-labs/avalanche-cli/sdk/constants" "github.com/ava-labs/avalanche-cli/sdk/network" - "github.com/ava-labs/avalanchego/utils/constants" + avagoConstants "github.com/ava-labs/avalanchego/utils/constants" "github.com/ava-labs/avalanchego/utils/logging" "github.com/cavaliergopher/grab/v3" "go.uber.org/zap" - - sdkConstants "github.com/ava-labs/avalanche-cli/sdk/constants" ) const ( @@ -59,12 +58,12 @@ func newGetter(endpoint string, target string) (Getter, error) { } // NewDownloader returns a new Downloader -// network: the network to download from ( fuji or mainnet). todo: add mainnet support +// network: the network to download from ( fuji only). // target: the path to download to // logLevel: the log level func NewDownloader( network network.Network, - logLevel logging.Level, + logger logging.Logger, ) (Downloader, error) { tmpFile, err := os.CreateTemp("", "avalanche-cli-public-archive-*") if err != nil { @@ -72,18 +71,18 @@ func NewDownloader( } switch network.ID { - case constants.FujiID: + case avagoConstants.FujiID: if getter, err := newGetter(PChainArchiveFuji, tmpFile.Name()); err != nil { return Downloader{}, err } else { return Downloader{ getter: getter, - logger: logging.NewLogger("public-archive-downloader", logging.NewWrappedCore(logLevel, os.Stdout, logging.JSON.ConsoleEncoder())), + logger: logger, currentOp: &sync.Mutex{}, }, nil } default: - return Downloader{}, fmt.Errorf("unsupported network ID: %d", network.ID) + return Downloader{}, fmt.Errorf("unsupported network ID: %d. Fuji only supported", network.ID) } } @@ -133,7 +132,7 @@ func (d Downloader) UnpackTo(targetDir string) error { d.currentOp.Lock() defer d.currentOp.Unlock() // prepare destination path - if err := os.MkdirAll(targetDir, sdkConstants.WriteReadUserOnlyDirPerms); err != nil { + if err := os.MkdirAll(targetDir, constants.UserOnlyWriteReadExecPerms); err != nil { d.logger.Error("Failed to create target directory", zap.Error(err)) return err } @@ -147,6 +146,7 @@ func (d Downloader) UnpackTo(targetDir string) error { tarReader := tar.NewReader(io.LimitReader(tarFile, maxFileSize)) extractedSize := int64(0) for { + // codeql [security] suppressed reason: This usage is safe in this context. header, err := tarReader.Next() if err == io.EOF { d.logger.Debug("End of archive reached") @@ -213,3 +213,50 @@ func (d Downloader) UnpackTo(targetDir string) error { d.logger.Info("Download unpacked to", zap.String("path", targetDir)) return nil } + +// IsEmpty returns true if the Downloader is empty and not initialized +func (d Downloader) IsEmpty() bool { + return d.getter.client == nil +} + +// IsComplete returns true if the download is complete +func (d Downloader) IsComplete() bool { + return d.GetBytesComplete() == d.GetDownloadSize() +} + +func (d Downloader) GetFilePath() (string, error) { + if !d.IsComplete() { + return "", fmt.Errorf("download is not completed") + } + return d.getter.request.Filename, nil +} + +// GetDownloadSize returns the size of the download +func (d Downloader) GetDownloadSize() int64 { + d.getter.mutex.RLock() + defer d.getter.mutex.RUnlock() + return d.getter.size +} + +func (d Downloader) setDownloadSize(size int64) { + d.getter.mutex.Lock() + defer d.getter.mutex.Unlock() + d.getter.size = size +} + +// GetCurrentProgress returns the current download progress +func (d Downloader) GetBytesComplete() int64 { + d.getter.mutex.RLock() + defer d.getter.mutex.RUnlock() + return d.getter.bytesComplete +} + +func (d Downloader) setBytesComplete(progress int64) { + d.getter.mutex.Lock() + defer d.getter.mutex.Unlock() + d.getter.bytesComplete = progress +} + +func (d Downloader) CleanUp() { + _ = os.Remove(d.getter.request.Filename) +} diff --git a/sdk/publicarchive/downloader_test.go b/sdk/publicarchive/downloader_test.go index 6301ee921..8cb55c86d 100644 --- a/sdk/publicarchive/downloader_test.go +++ b/sdk/publicarchive/downloader_test.go @@ -34,9 +34,7 @@ func TestNewGetter(t *testing.T) { } func TestNewDownloader(t *testing.T) { - logLevel := logging.Info - - downloader, err := NewDownloader(network.Network{ID: constants.FujiID}, logLevel) + downloader, err := NewDownloader(network.Network{ID: constants.FujiID}, logging.NewLogger("public-archive-downloader", logging.NewWrappedCore(logging.Info, os.Stdout, logging.JSON.ConsoleEncoder()))) require.NoError(t, err, "NewDownloader should not return an error") require.NotNil(t, downloader.logger, "downloader logger should not be nil") require.NotNil(t, downloader.getter.client, "downloader getter client should not be nil") @@ -144,11 +142,8 @@ func TestDownloader_EndToEnd(t *testing.T) { // Configure the test network (Fuji in this case) net := network.Network{ID: constants.FujiID} - // Initialize a logger - logLevel := logging.Debug - // Step 1: Create the downloader - downloader, err := NewDownloader(net, logLevel) + downloader, err := NewDownloader(net, logging.NewLogger("public-archive-downloader", logging.NewWrappedCore(logging.Debug, os.Stdout, logging.JSON.ConsoleEncoder()))) require.NoError(t, err, "Failed to initialize downloader") // Step 2: Start the download diff --git a/sdk/publicarchive/helpers.go b/sdk/publicarchive/helpers.go deleted file mode 100644 index 2f46029a9..000000000 --- a/sdk/publicarchive/helpers.go +++ /dev/null @@ -1,51 +0,0 @@ -// Copyright (C) 2024, Ava Labs, Inc. All rights reserved. -// See the file LICENSE for licensing terms. - -package publicarchive - -import ( - "fmt" - "os" -) - -// IsEmpty returns true if the Downloader is empty and not initialized -func (d Downloader) IsEmpty() bool { - return d.getter.client == nil -} - -func (d Downloader) GetDownloadedFilePath() (string, error) { - if d.GetBytesComplete() != d.GetDownloadSize() { - return "", fmt.Errorf("download is not completed") - } - return d.getter.request.Filename, nil -} - -// GetDownloadSize returns the size of the download -func (d Downloader) GetDownloadSize() int64 { - d.getter.mutex.RLock() - defer d.getter.mutex.RUnlock() - return d.getter.size -} - -func (d Downloader) setDownloadSize(size int64) { - d.getter.mutex.Lock() - defer d.getter.mutex.Unlock() - d.getter.size = size -} - -// GetCurrentProgress returns the current download progress -func (d Downloader) GetBytesComplete() int64 { - d.getter.mutex.RLock() - defer d.getter.mutex.RUnlock() - return d.getter.bytesComplete -} - -func (d Downloader) setBytesComplete(progress int64) { - d.getter.mutex.Lock() - defer d.getter.mutex.Unlock() - d.getter.bytesComplete = progress -} - -func (d Downloader) CleanUp() { - _ = os.Remove(d.getter.request.Filename) -}