diff --git a/internal/chart/v3/loader/directory.go b/internal/chart/v3/loader/directory.go index dfe3af3b2..3f7e8e77d 100644 --- a/internal/chart/v3/loader/directory.go +++ b/internal/chart/v3/loader/directory.go @@ -43,6 +43,10 @@ func (l DirLoader) Load() (*chart.Chart, error) { // // This loads charts only from directories. func LoadDir(dir string) (*chart.Chart, error) { + return loadDir(dir, archive.MaxDecompressedChartSize) +} + +func loadDir(dir string, budget int64) (*chart.Chart, error) { topdir, err := filepath.Abs(dir) if err != nil { return nil, err @@ -64,6 +68,7 @@ func LoadDir(dir string) (*chart.Chart, error) { files := []*archive.BufferedFile{} topdir += string(filepath.Separator) + budgetReader := archive.NewBudgetedReader(budget) walk := func(name string, fi os.FileInfo, err error) error { n := strings.TrimPrefix(name, topdir) @@ -100,11 +105,7 @@ func LoadDir(dir string) (*chart.Chart, error) { return fmt.Errorf("cannot load irregular file %s as it has file mode type bits set", name) } - if fi.Size() > archive.MaxDecompressedFileSize { - return fmt.Errorf("chart file %q is larger than the maximum file size %d", fi.Name(), archive.MaxDecompressedFileSize) - } - - data, err := os.ReadFile(name) + data, err := budgetReader.ReadFileWithBudget(name, fi.Size()) if err != nil { return fmt.Errorf("error reading %s: %w", n, err) } diff --git a/internal/chart/v3/loader/load_test.go b/internal/chart/v3/loader/load_test.go index c4c252407..be5b3701d 100644 --- a/internal/chart/v3/loader/load_test.go +++ b/internal/chart/v3/loader/load_test.go @@ -51,6 +51,16 @@ func TestLoadDir(t *testing.T) { verifyDependenciesLock(t, c) } +func TestLoadDirExceedsBudget(t *testing.T) { + _, err := loadDir("testdata/frobnitz", 1) + if err == nil { + t.Fatal("expected error when chart directory exceeds budget") + } + if !strings.Contains(err.Error(), "chart exceeds maximum decompressed size") { + t.Fatalf("unexpected error: %v", err) + } +} + func TestLoadDirWithDevNull(t *testing.T) { if runtime.GOOS == "windows" { t.Skip("test only works on unix systems with /dev/null present") diff --git a/pkg/chart/loader/archive/archive.go b/pkg/chart/loader/archive/archive.go index a35c0152d..633871bcc 100644 --- a/pkg/chart/loader/archive/archive.go +++ b/pkg/chart/loader/archive/archive.go @@ -37,8 +37,10 @@ import ( // The default value is 100 MiB. var MaxDecompressedChartSize int64 = 100 * 1024 * 1024 // Default 100 MiB -// MaxDecompressedFileSize is the size of the largest file that Helm will attempt to load. -// The size of the file is the decompressed version of it when it is stored in an archive. +// MaxDecompressedFileSize was the per-file size limit enforced during chart loading. +// It is no longer used internally; aggregate chart size is enforced via MaxDecompressedChartSize. +// +// Deprecated: Retained for backward compatibility with external callers. Will be removed in Helm v5. var MaxDecompressedFileSize int64 = 5 * 1024 * 1024 // Default 5 MiB var drivePathPattern = regexp.MustCompile(`^[a-zA-Z]:/`) @@ -128,10 +130,6 @@ func LoadArchiveFiles(in io.Reader) ([]*BufferedFile, error) { return nil, fmt.Errorf("decompressed chart is larger than the maximum size %d", MaxDecompressedChartSize) } - if hd.Size > MaxDecompressedFileSize { - return nil, fmt.Errorf("decompressed chart file %q is larger than the maximum file size %d", hd.Name, MaxDecompressedFileSize) - } - limitedReader := io.LimitReader(tr, remainingSize) bytesWritten, err := io.Copy(b, limitedReader) diff --git a/pkg/chart/loader/archive/budget.go b/pkg/chart/loader/archive/budget.go new file mode 100644 index 000000000..3f9b90e4a --- /dev/null +++ b/pkg/chart/loader/archive/budget.go @@ -0,0 +1,74 @@ +/* +Copyright The Helm Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package archive + +import ( + "fmt" + "io" + "math" + "os" +) + +// BudgetedReader tracks cumulative file reads against a size limit. +type BudgetedReader struct { + max int64 + remaining int64 +} + +// NewBudgetedReader creates a BudgetedReader with the given maximum total size. +// The remaining budget is initialized to the maximum. +func NewBudgetedReader(limit int64) *BudgetedReader { + return &BudgetedReader{ + max: limit, + remaining: limit, + } +} + +// ReadFileWithBudget reads a file and decrements the remaining budget by the bytes read. +// It returns an error if the total would exceed the configured maximum. +// The read is capped via io.LimitReader so a file that grows between stat +// and read cannot cause unbounded memory allocation. +func (r *BudgetedReader) ReadFileWithBudget(path string, size int64) ([]byte, error) { + if size > r.remaining { + return nil, fmt.Errorf("chart exceeds maximum decompressed size of %d bytes", r.max) + } + + f, err := os.Open(path) + if err != nil { + return nil, err + } + defer f.Close() + + // Read at most r.remaining+1 bytes so we can detect over-budget without + // allocating unbounded memory if the file grew since stat. + // Clamp to avoid int64 overflow when r.remaining is near math.MaxInt64. + limit := r.remaining + if limit < math.MaxInt64 { + limit++ + } + data, err := io.ReadAll(io.LimitReader(f, limit)) + if err != nil { + return nil, err + } + + if int64(len(data)) > r.remaining { + return nil, fmt.Errorf("chart exceeds maximum decompressed size of %d bytes", r.max) + } + + r.remaining -= int64(len(data)) + return data, nil +} diff --git a/pkg/chart/loader/archive/budget_test.go b/pkg/chart/loader/archive/budget_test.go new file mode 100644 index 000000000..f37f78687 --- /dev/null +++ b/pkg/chart/loader/archive/budget_test.go @@ -0,0 +1,128 @@ +/* +Copyright The Helm Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package archive + +import ( + "fmt" + "os" + "path/filepath" + "testing" +) + +func TestReadFileWithBudget(t *testing.T) { + dir := t.TempDir() + + writeFile := func(t *testing.T, name string, size int) string { + t.Helper() + p := filepath.Join(dir, name) + if err := os.WriteFile(p, make([]byte, size), 0644); err != nil { + t.Fatal(err) + } + return p + } + + tcs := []struct { + name string + check func(t *testing.T) + }{ + { + name: "reads file and decrements budget", + check: func(t *testing.T) { + t.Helper() + p := writeFile(t, "small.txt", 100) + fi, err := os.Stat(p) + if err != nil { + t.Fatalf("failed to stat %s: %v", p, err) + } + limit := int64(1000) + + br := NewBudgetedReader(limit) + data, err := br.ReadFileWithBudget(p, fi.Size()) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if len(data) != 100 { + t.Fatalf("expected 100 bytes, got %d", len(data)) + } + if br.remaining != 900 { + t.Fatalf("expected remaining=900, got %d", br.remaining) + } + }, + }, + { + name: "rejects file exceeding budget", + check: func(t *testing.T) { + t.Helper() + p := writeFile(t, "big.txt", 500) + fi, err := os.Stat(p) + if err != nil { + t.Fatalf("failed to stat %s: %v", p, err) + } + limit := int64(100) + + br := NewBudgetedReader(limit) + _, err = br.ReadFileWithBudget(p, fi.Size()) + if err == nil { + t.Fatal("expected error for file exceeding budget") + } + expectedErr := fmt.Sprintf("chart exceeds maximum decompressed size of %d bytes", limit) + if err.Error() != expectedErr { + t.Fatalf("expected %q, got %q", expectedErr, err.Error()) + } + if br.remaining != 100 { + t.Fatalf("budget should not change on rejection, got %d", br.remaining) + } + }, + }, + { + name: "tracks budget across multiple reads", + check: func(t *testing.T) { + t.Helper() + remaining := int64(250) + + br := NewBudgetedReader(remaining) + for i := range 3 { + p := writeFile(t, fmt.Sprintf("f%d.txt", i), 80) + fi, err := os.Stat(p) + if err != nil { + t.Fatalf("failed to stat %s: %v", p, err) + } + if _, err := br.ReadFileWithBudget(p, fi.Size()); err != nil { + t.Fatalf("read %d: unexpected error: %v", i, err) + } + } + if br.remaining != 10 { + t.Fatalf("expected remaining=10, got %d", br.remaining) + } + + p := writeFile(t, "over.txt", 20) + fi, err := os.Stat(p) + if err != nil { + t.Fatalf("failed to stat %s: %v", p, err) + } + _, err = br.ReadFileWithBudget(p, fi.Size()) + if err == nil { + t.Fatal("expected error when cumulative reads exceed budget") + } + }, + }, + } + + for _, tc := range tcs { + t.Run(tc.name, tc.check) + } +} diff --git a/pkg/chart/v2/loader/directory.go b/pkg/chart/v2/loader/directory.go index 82578d924..d54fbd3ed 100644 --- a/pkg/chart/v2/loader/directory.go +++ b/pkg/chart/v2/loader/directory.go @@ -43,6 +43,10 @@ func (l DirLoader) Load() (*chart.Chart, error) { // // This loads charts only from directories. func LoadDir(dir string) (*chart.Chart, error) { + return loadDir(dir, archive.MaxDecompressedChartSize) +} + +func loadDir(dir string, budget int64) (*chart.Chart, error) { topdir, err := filepath.Abs(dir) if err != nil { return nil, err @@ -64,6 +68,7 @@ func LoadDir(dir string) (*chart.Chart, error) { files := []*archive.BufferedFile{} topdir += string(filepath.Separator) + budgetReader := archive.NewBudgetedReader(budget) walk := func(name string, fi os.FileInfo, err error) error { n := strings.TrimPrefix(name, topdir) @@ -100,11 +105,7 @@ func LoadDir(dir string) (*chart.Chart, error) { return fmt.Errorf("cannot load irregular file %s as it has file mode type bits set", name) } - if fi.Size() > archive.MaxDecompressedFileSize { - return fmt.Errorf("chart file %q is larger than the maximum file size %d", fi.Name(), archive.MaxDecompressedFileSize) - } - - data, err := os.ReadFile(name) + data, err := budgetReader.ReadFileWithBudget(name, fi.Size()) if err != nil { return fmt.Errorf("error reading %s: %w", n, err) } diff --git a/pkg/chart/v2/loader/load_test.go b/pkg/chart/v2/loader/load_test.go index aed071b2f..4d3321913 100644 --- a/pkg/chart/v2/loader/load_test.go +++ b/pkg/chart/v2/loader/load_test.go @@ -51,6 +51,16 @@ func TestLoadDir(t *testing.T) { verifyDependenciesLock(t, c) } +func TestLoadDirExceedsBudget(t *testing.T) { + _, err := loadDir("testdata/frobnitz", 1) + if err == nil { + t.Fatal("expected error when chart directory exceeds budget") + } + if !strings.Contains(err.Error(), "chart exceeds maximum decompressed size") { + t.Fatalf("unexpected error: %v", err) + } +} + func TestLoadDirWithDevNull(t *testing.T) { if runtime.GOOS == "windows" { t.Skip("test only works on unix systems with /dev/null present")