Skip to content

Commit 39d5bbb

Browse files
hiddecohiddeco
committed
helm: attach loader to helm.MaxChartFileSize
Signed-off-by: Hidde Beydals <[email protected]>
1 parent 95bc619 commit 39d5bbb

File tree

4 files changed

+35
-32
lines changed

4 files changed

+35
-32
lines changed

‎internal/helm/chart/secureloader/directory.go‎

Lines changed: 9 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@ import (
3434
"strings"
3535

3636
securejoin "github.com/cyphar/filepath-securejoin"
37+
"github.com/fluxcd/source-controller/internal/helm"
3738
"helm.sh/helm/v3/pkg/chart"
3839
"helm.sh/helm/v3/pkg/chart/loader"
3940

@@ -42,10 +43,6 @@ import (
4243
)
4344

4445
var (
45-
// DefaultMaxFileSize is the default maximum file size of any chart file
46-
// loaded.
47-
DefaultMaxFileSize = 16 << 20 // 16MiB
48-
4946
utf8bom = []byte{0xEF, 0xBB, 0xBF}
5047
)
5148

@@ -54,16 +51,16 @@ var (
5451
type SecureDirLoader struct {
5552
root string
5653
path string
57-
maxSize int
54+
maxSize int64
5855
}
5956

6057
// NewSecureDirLoader returns a new SecureDirLoader, configured to the scope of the
6158
// root and provided dir. Max size configures the maximum size a file must not
62-
// exceed to be loaded. If 0 it defaults to DefaultMaxFileSize, it can be
59+
// exceed to be loaded. If 0 it defaults to helm.MaxChartFileSize, it can be
6360
// disabled using a negative integer.
64-
func NewSecureDirLoader(root string, path string, maxSize int) SecureDirLoader {
61+
func NewSecureDirLoader(root string, path string, maxSize int64) SecureDirLoader {
6562
if maxSize == 0 {
66-
maxSize = DefaultMaxFileSize
63+
maxSize = helm.MaxChartFileSize
6764
}
6865
return SecureDirLoader{
6966
root: root,
@@ -80,7 +77,7 @@ func (l SecureDirLoader) Load() (*chart.Chart, error) {
8077
// SecureLoadDir securely loads a chart from the path relative to root, without
8178
// traversing outside root. When maxSize >= 0, files are not allowed to exceed
8279
// this size, or an error is returned.
83-
func SecureLoadDir(root, path string, maxSize int) (*chart.Chart, error) {
80+
func SecureLoadDir(root, path string, maxSize int64) (*chart.Chart, error) {
8481
root, err := filepath.Abs(root)
8582
if err != nil {
8683
return nil, err
@@ -152,12 +149,12 @@ func secureLoadIgnoreRules(root, chartPath string) (*ignore.Rules, error) {
152149
type secureFileWalker struct {
153150
root string
154151
absChartPath string
155-
maxSize int
152+
maxSize int64
156153
rules *ignore.Rules
157154
files []*loader.BufferedFile
158155
}
159156

160-
func newSecureFileWalker(root, absChartPath string, maxSize int, rules *ignore.Rules) *secureFileWalker {
157+
func newSecureFileWalker(root, absChartPath string, maxSize int64, rules *ignore.Rules) *secureFileWalker {
161158
absChartPath = filepath.Clean(absChartPath) + string(filepath.Separator)
162159
return &secureFileWalker{
163160
root: root,
@@ -216,7 +213,7 @@ func (w *secureFileWalker) walk(name, absName string, fi os.FileInfo, err error)
216213
}
217214

218215
// Confirm size it not outside boundaries
219-
if fileSize := fi.Size(); w.maxSize > 0 && fileSize > int64(w.maxSize) {
216+
if fileSize := fi.Size(); w.maxSize > 0 && fileSize > w.maxSize {
220217
return fmt.Errorf("cannot load file %s as file size (%d) exceeds limit (%d)", n, fileSize, w.maxSize)
221218
}
222219

‎internal/helm/chart/secureloader/directory_test.go‎

Lines changed: 20 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -26,10 +26,12 @@ import (
2626
"testing"
2727
"testing/fstest"
2828

29-
"github.com/fluxcd/source-controller/internal/helm/chart/secureloader/ignore"
3029
. "github.com/onsi/gomega"
3130
"helm.sh/helm/v3/pkg/chart"
3231
"sigs.k8s.io/yaml"
32+
33+
"github.com/fluxcd/source-controller/internal/helm"
34+
"github.com/fluxcd/source-controller/internal/helm/chart/secureloader/ignore"
3335
)
3436

3537
func TestSecureDirLoader_Load(t *testing.T) {
@@ -49,7 +51,7 @@ func TestSecureDirLoader_Load(t *testing.T) {
4951
g.Expect(err).ToNot(HaveOccurred())
5052
g.Expect(os.WriteFile(filepath.Join(tmpDir, "Chart.yaml"), b, 0o644)).To(Succeed())
5153

52-
got, err := (NewSecureDirLoader(tmpDir, "", DefaultMaxFileSize)).Load()
54+
got, err := (NewSecureDirLoader(tmpDir, "", helm.MaxChartFileSize)).Load()
5355
g.Expect(err).ToNot(HaveOccurred())
5456
g.Expect(got).ToNot(BeNil())
5557
g.Expect(got.Name()).To(Equal(m.Name))
@@ -64,7 +66,7 @@ func TestSecureDirLoader_Load(t *testing.T) {
6466
g.Expect(err).ToNot(HaveOccurred())
6567
g.Expect(os.WriteFile(filepath.Join(tmpDir, "Chart.yaml"), b, 0o644)).To(Succeed())
6668

67-
got, err := (NewSecureDirLoader(tmpDir, tmpDir, DefaultMaxFileSize)).Load()
69+
got, err := (NewSecureDirLoader(tmpDir, tmpDir, helm.MaxChartFileSize)).Load()
6870
g.Expect(err).ToNot(HaveOccurred())
6971
g.Expect(got).ToNot(BeNil())
7072
g.Expect(got.Name()).To(Equal(m.Name))
@@ -83,12 +85,12 @@ func TestSecureDirLoader_Load(t *testing.T) {
8385
root := filepath.Join(tmpDir, "root")
8486
g.Expect(os.Mkdir(root, 0o700)).To(Succeed())
8587

86-
got, err := (NewSecureDirLoader(root, "../", DefaultMaxFileSize)).Load()
88+
got, err := (NewSecureDirLoader(root, "../", helm.MaxChartFileSize)).Load()
8789
g.Expect(err).To(HaveOccurred())
8890
g.Expect(err.Error()).To(ContainSubstring("failed to load chart from /: Chart.yaml file is missing"))
8991
g.Expect(got).To(BeNil())
9092

91-
got, err = (NewSecureDirLoader(root, tmpDir, DefaultMaxFileSize)).Load()
93+
got, err = (NewSecureDirLoader(root, tmpDir, helm.MaxChartFileSize)).Load()
9294
g.Expect(err).To(HaveOccurred())
9395
g.Expect(err.Error()).To(ContainSubstring("failed to load chart from /: Chart.yaml file is missing"))
9496
g.Expect(got).To(BeNil())
@@ -105,7 +107,7 @@ func TestSecureDirLoader_Load(t *testing.T) {
105107
g.Expect(os.WriteFile(filepath.Join(tmpDir, ignore.HelmIgnore), []byte("file.txt"), 0o644)).To(Succeed())
106108
g.Expect(os.WriteFile(filepath.Join(tmpDir, "file.txt"), []byte("not included"), 0o644)).To(Succeed())
107109

108-
got, err := (NewSecureDirLoader(tmpDir, "", DefaultMaxFileSize)).Load()
110+
got, err := (NewSecureDirLoader(tmpDir, "", helm.MaxChartFileSize)).Load()
109111
g.Expect(err).ToNot(HaveOccurred())
110112
g.Expect(got).ToNot(BeNil())
111113
g.Expect(got.Name()).To(Equal(m.Name))
@@ -218,7 +220,7 @@ func Test_secureFileWalker_walk(t *testing.T) {
218220
t.Run("given name equals top dir", func(t *testing.T) {
219221
g := NewWithT(t)
220222

221-
w := newSecureFileWalker(root, chartPath, DefaultMaxFileSize, ignore.Empty())
223+
w := newSecureFileWalker(root, chartPath, helm.MaxChartFileSize, ignore.Empty())
222224
g.Expect(w.walk(chartPath+"/", chartPath, nil, nil)).To(BeNil())
223225
})
224226

@@ -237,7 +239,7 @@ func Test_secureFileWalker_walk(t *testing.T) {
237239
rules, err := ignore.Parse(strings.NewReader(fakeDirName + "/"))
238240
g.Expect(err).ToNot(HaveOccurred())
239241

240-
w := newSecureFileWalker(root, chartPath, DefaultMaxFileSize, rules)
242+
w := newSecureFileWalker(root, chartPath, helm.MaxChartFileSize, rules)
241243
g.Expect(w.walk(filepath.Join(w.absChartPath, fakeDirName), filepath.Join(w.absChartPath, fakeDirName), fakeDirInfo, nil)).To(Equal(fs.SkipDir))
242244
})
243245

@@ -247,21 +249,21 @@ func Test_secureFileWalker_walk(t *testing.T) {
247249
rules, err := ignore.Parse(strings.NewReader(fakeDirName + "/"))
248250
g.Expect(err).ToNot(HaveOccurred())
249251

250-
w := newSecureFileWalker(root, chartPath, DefaultMaxFileSize, rules)
252+
w := newSecureFileWalker(root, chartPath, helm.MaxChartFileSize, rules)
251253
g.Expect(w.walk(filepath.Join(w.absChartPath, "symlink"), filepath.Join(w.absChartPath, fakeDirName), fakeDirInfo, nil)).To(BeNil())
252254
})
253255

254256
t.Run("ignore rule not applicable to dir", func(t *testing.T) {
255257
g := NewWithT(t)
256258

257-
w := newSecureFileWalker(root, chartPath, DefaultMaxFileSize, ignore.Empty())
259+
w := newSecureFileWalker(root, chartPath, helm.MaxChartFileSize, ignore.Empty())
258260
g.Expect(w.walk(filepath.Join(w.absChartPath, fakeDirName), filepath.Join(w.absChartPath, fakeDirName), fakeDirInfo, nil)).To(BeNil())
259261
})
260262

261263
t.Run("absolute path outside root", func(t *testing.T) {
262264
g := NewWithT(t)
263265

264-
w := newSecureFileWalker(root, chartPath, DefaultMaxFileSize, ignore.Empty())
266+
w := newSecureFileWalker(root, chartPath, helm.MaxChartFileSize, ignore.Empty())
265267
err := w.walk(filepath.Join(w.absChartPath, fakeDirName), filepath.Join("/fake/another/root/", fakeDirName), fakeDirInfo, nil)
266268
g.Expect(err).To(HaveOccurred())
267269
g.Expect(err.Error()).To(ContainSubstring("cannot load 'fake-dir' directory: absolute path traverses outside root boundary"))
@@ -273,7 +275,7 @@ func Test_secureFileWalker_walk(t *testing.T) {
273275
rules, err := ignore.Parse(strings.NewReader(fakeDirName + "/"))
274276
g.Expect(err).ToNot(HaveOccurred())
275277

276-
w := newSecureFileWalker(root, chartPath, DefaultMaxFileSize, rules)
278+
w := newSecureFileWalker(root, chartPath, helm.MaxChartFileSize, rules)
277279
g.Expect(w.walk(filepath.Join(w.absChartPath, fakeDirName), filepath.Join("/fake/another/root/", fakeDirName), fakeDirInfo, nil)).To(Equal(fs.SkipDir))
278280
})
279281

@@ -283,21 +285,21 @@ func Test_secureFileWalker_walk(t *testing.T) {
283285
rules, err := ignore.Parse(strings.NewReader(fakeFileName))
284286
g.Expect(err).ToNot(HaveOccurred())
285287

286-
w := newSecureFileWalker(root, chartPath, DefaultMaxFileSize, rules)
288+
w := newSecureFileWalker(root, chartPath, helm.MaxChartFileSize, rules)
287289
g.Expect(w.walk(filepath.Join(w.absChartPath, fakeFileName), filepath.Join(w.absChartPath, fakeFileName), fakeFileInfo, nil)).To(BeNil())
288290
})
289291

290292
t.Run("file path outside root", func(t *testing.T) {
291293
g := NewWithT(t)
292294

293-
w := newSecureFileWalker(root, chartPath, DefaultMaxFileSize, ignore.Empty())
295+
w := newSecureFileWalker(root, chartPath, helm.MaxChartFileSize, ignore.Empty())
294296
err := w.walk(filepath.Join(w.absChartPath, fakeFileName), filepath.Join("/fake/another/root/", fakeFileName), fakeFileInfo, nil)
295297
g.Expect(err).To(HaveOccurred())
296298
g.Expect(err.Error()).To(ContainSubstring("cannot load 'fake-file' file: absolute path traverses outside root boundary"))
297299
})
298300

299301
t.Run("irregular file", func(t *testing.T) {
300-
w := newSecureFileWalker(root, chartPath, DefaultMaxFileSize, ignore.Empty())
302+
w := newSecureFileWalker(root, chartPath, helm.MaxChartFileSize, ignore.Empty())
301303
err := w.walk(fakeDeviceFileName, filepath.Join(w.absChartPath), fakeDeviceInfo, nil)
302304
g.Expect(err).To(HaveOccurred())
303305
g.Expect(err.Error()).To(ContainSubstring("cannot load irregular file fake-device as it has file mode type bits set"))
@@ -321,7 +323,7 @@ func Test_secureFileWalker_walk(t *testing.T) {
321323
fileInfo, err := os.Lstat(absFilePath)
322324
g.Expect(err).ToNot(HaveOccurred())
323325

324-
w := newSecureFileWalker(tmpDir, tmpDir, DefaultMaxFileSize, ignore.Empty())
326+
w := newSecureFileWalker(tmpDir, tmpDir, helm.MaxChartFileSize, ignore.Empty())
325327
g.Expect(w.walk(fileName, absFilePath, fileInfo, nil)).To(Succeed())
326328
g.Expect(w.files).To(HaveLen(1))
327329
g.Expect(w.files[0].Name).To(Equal(fileName))
@@ -340,7 +342,7 @@ func Test_secureFileWalker_walk(t *testing.T) {
340342
fileInfo, err := os.Lstat(absFilePath)
341343
g.Expect(err).ToNot(HaveOccurred())
342344

343-
w := newSecureFileWalker(tmpDir, tmpDir, DefaultMaxFileSize, ignore.Empty())
345+
w := newSecureFileWalker(tmpDir, tmpDir, helm.MaxChartFileSize, ignore.Empty())
344346
g.Expect(w.walk(fileName, absFilePath, fileInfo, nil)).To(Succeed())
345347
g.Expect(w.files).To(HaveLen(1))
346348
g.Expect(w.files[0].Name).To(Equal(fileName))
@@ -351,7 +353,7 @@ func Test_secureFileWalker_walk(t *testing.T) {
351353
g := NewWithT(t)
352354
tmpDir := t.TempDir()
353355

354-
w := newSecureFileWalker(tmpDir, tmpDir, DefaultMaxFileSize, ignore.Empty())
356+
w := newSecureFileWalker(tmpDir, tmpDir, helm.MaxChartFileSize, ignore.Empty())
355357
err := w.walk(filepath.Join(w.absChartPath, "invalid"), filepath.Join(w.absChartPath, "invalid"), fakeFileInfo, nil)
356358
g.Expect(err).To(HaveOccurred())
357359
g.Expect(errors.Is(err, fs.ErrNotExist)).To(BeTrue())

‎internal/helm/chart/secureloader/loader.go‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,8 @@ import (
2727
securejoin "github.com/cyphar/filepath-securejoin"
2828
"helm.sh/helm/v3/pkg/chart"
2929
"helm.sh/helm/v3/pkg/chart/loader"
30+
31+
"github.com/fluxcd/source-controller/internal/helm"
3032
)
3133

3234
// Loader returns a new loader.ChartLoader appropriate for the given chart
@@ -61,7 +63,7 @@ func Loader(root, name string) (loader.ChartLoader, error) {
6163
}
6264

6365
if fi.IsDir() {
64-
return NewSecureDirLoader(root, relName, DefaultMaxFileSize), nil
66+
return NewSecureDirLoader(root, relName, helm.MaxChartFileSize), nil
6567
}
6668
return FileLoader(secureName), nil
6769
}

‎internal/helm/chart/secureloader/loader_test.go‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,8 @@ import (
2626
"helm.sh/helm/v3/pkg/chart"
2727
"helm.sh/helm/v3/pkg/chart/loader"
2828
"sigs.k8s.io/yaml"
29+
30+
"github.com/fluxcd/source-controller/internal/helm"
2931
)
3032

3133
func TestLoader(t *testing.T) {
@@ -51,7 +53,7 @@ func TestLoader(t *testing.T) {
5153

5254
got, err := Loader(tmpDir, "fake")
5355
g.Expect(err).ToNot(HaveOccurred())
54-
g.Expect(got).To(Equal(SecureDirLoader{root: tmpDir, path: "fake", maxSize: DefaultMaxFileSize}))
56+
g.Expect(got).To(Equal(SecureDirLoader{root: tmpDir, path: "fake", maxSize: helm.MaxChartFileSize}))
5557
})
5658

5759
t.Run("illegal path", func(t *testing.T) {

0 commit comments

Comments
 (0)