From 0949d0adb91eb8be1ccd317ccd8888b710edc382 Mon Sep 17 00:00:00 2001 From: Peter Gundel Date: Mon, 4 Apr 2022 16:36:23 +0200 Subject: [PATCH 1/2] Change all file permissions to octal format This better represent permissions as Linux handles such information in octal format, meaning that the left-most 0 has an important meaning and is not to be ignored as normally integers would. See https://github.com/fluxcd/source-controller/issues/603 Signed-off-by: Peter Gundel --- controllers/helmchart_controller.go | 2 +- controllers/storage.go | 4 +- controllers/storage_test.go | 8 ++-- internal/fs/fs_test.go | 38 +++++++++---------- internal/helm/chart/builder_local_test.go | 4 +- internal/helm/chart/metadata_test.go | 4 +- .../helm/repository/chart_repository_test.go | 2 +- main.go | 2 +- pkg/gcp/gcp.go | 4 +- pkg/sourceignore/sourceignore_test.go | 4 +- tests/fuzz/gitrepository_fuzzer.go | 2 +- 11 files changed, 37 insertions(+), 37 deletions(-) diff --git a/controllers/helmchart_controller.go b/controllers/helmchart_controller.go index 7951836ac..1919c4e80 100644 --- a/controllers/helmchart_controller.go +++ b/controllers/helmchart_controller.go @@ -580,7 +580,7 @@ func (r *HelmChartReconciler) buildFromTarballArtifact(ctx context.Context, obj // Create directory to untar source into sourceDir := filepath.Join(tmpDir, "source") - if err := os.Mkdir(sourceDir, 0700); err != nil { + if err := os.Mkdir(sourceDir, 0o700); err != nil { e := &serror.Event{ Err: fmt.Errorf("failed to create directory to untar source into: %w", err), Reason: sourcev1.DirCreationFailedReason, diff --git a/controllers/storage.go b/controllers/storage.go index ff1408f33..bcd1b17e0 100644 --- a/controllers/storage.go +++ b/controllers/storage.go @@ -112,7 +112,7 @@ func (s Storage) SetHostname(URL string) string { // MkdirAll calls os.MkdirAll for the given v1beta1.Artifact base dir. func (s *Storage) MkdirAll(artifact sourcev1.Artifact) error { dir := filepath.Dir(s.LocalPath(artifact)) - return os.MkdirAll(dir, 0777) + return os.MkdirAll(dir, 0o777) } // RemoveAll calls os.RemoveAll for the given v1beta1.Artifact base dir. @@ -432,7 +432,7 @@ func (s *Storage) Archive(artifact *sourcev1.Artifact, dir string, filter Archiv return err } - if err := os.Chmod(tmpName, 0644); err != nil { + if err := os.Chmod(tmpName, 0o644); err != nil { return err } diff --git a/controllers/storage_test.go b/controllers/storage_test.go index 627317546..a699ff5e4 100644 --- a/controllers/storage_test.go +++ b/controllers/storage_test.go @@ -136,7 +136,7 @@ func TestStorage_Archive(t *testing.T) { } for name, b := range files { absPath := filepath.Join(dir, name) - if err = os.MkdirAll(filepath.Dir(absPath), 0755); err != nil { + if err = os.MkdirAll(filepath.Dir(absPath), 0o755); err != nil { return } f, err := os.Create(absPath) @@ -316,7 +316,7 @@ func TestStorageRemoveAllButCurrent(t *testing.T) { // Create artifact dir and artifacts. artifactDir := path.Join(dir, "foo", "bar") - g.Expect(os.MkdirAll(artifactDir, 0755)).NotTo(HaveOccurred()) + g.Expect(os.MkdirAll(artifactDir, 0o755)).NotTo(HaveOccurred()) current := []string{ path.Join(artifactDir, "artifact1.tar.gz"), } @@ -378,7 +378,7 @@ func TestStorageRemoveAll(t *testing.T) { } if tt.createArtifactPath { - g.Expect(os.MkdirAll(path.Join(dir, tt.artifactPath), 0755)).ToNot(HaveOccurred()) + g.Expect(os.MkdirAll(path.Join(dir, tt.artifactPath), 0o755)).ToNot(HaveOccurred()) } deleted, err := s.RemoveAll(artifact) @@ -412,7 +412,7 @@ func TestStorageCopyFromPath(t *testing.T) { } t.Cleanup(cleanupStoragePath(dir)) absPath = filepath.Join(dir, file.Name) - if err = os.MkdirAll(filepath.Dir(absPath), 0755); err != nil { + if err = os.MkdirAll(filepath.Dir(absPath), 0o755); err != nil { return } f, err := os.Create(absPath) diff --git a/internal/fs/fs_test.go b/internal/fs/fs_test.go index 250556bc2..4f3a4f20f 100644 --- a/internal/fs/fs_test.go +++ b/internal/fs/fs_test.go @@ -42,12 +42,12 @@ func TestRenameWithFallback(t *testing.T) { } srcpath = filepath.Join(dir, "a") - if err = os.MkdirAll(srcpath, 0777); err != nil { + if err = os.MkdirAll(srcpath, 0o777); err != nil { t.Fatal(err) } dstpath := filepath.Join(dir, "b") - if err = os.MkdirAll(dstpath, 0777); err != nil { + if err = os.MkdirAll(dstpath, 0o777); err != nil { t.Fatal(err) } @@ -64,7 +64,7 @@ func TestCopyDir(t *testing.T) { defer os.RemoveAll(dir) srcdir := filepath.Join(dir, "src") - if err := os.MkdirAll(srcdir, 0755); err != nil { + if err := os.MkdirAll(srcdir, 0o755); err != nil { t.Fatal(err) } @@ -81,7 +81,7 @@ func TestCopyDir(t *testing.T) { for i, file := range files { fn := filepath.Join(srcdir, file.path) dn := filepath.Dir(fn) - if err = os.MkdirAll(dn, 0755); err != nil { + if err = os.MkdirAll(dn, 0o755); err != nil { t.Fatal(err) } @@ -151,7 +151,7 @@ func TestCopyDirFail_SrcInaccessible(t *testing.T) { cleanup := setupInaccessibleDir(t, func(dir string) error { srcdir = filepath.Join(dir, "src") - return os.MkdirAll(srcdir, 0755) + return os.MkdirAll(srcdir, 0o755) }) defer cleanup() @@ -184,7 +184,7 @@ func TestCopyDirFail_DstInaccessible(t *testing.T) { defer os.RemoveAll(dir) srcdir = filepath.Join(dir, "src") - if err = os.MkdirAll(srcdir, 0755); err != nil { + if err = os.MkdirAll(srcdir, 0o755); err != nil { t.Fatal(err) } @@ -235,12 +235,12 @@ func TestCopyDirFail_DstExists(t *testing.T) { defer os.RemoveAll(dir) srcdir = filepath.Join(dir, "src") - if err = os.MkdirAll(srcdir, 0755); err != nil { + if err = os.MkdirAll(srcdir, 0o755); err != nil { t.Fatal(err) } dstdir = filepath.Join(dir, "dst") - if err = os.MkdirAll(dstdir, 0755); err != nil { + if err = os.MkdirAll(dstdir, 0o755); err != nil { t.Fatal(err) } @@ -256,7 +256,7 @@ func TestCopyDirFail_DstExists(t *testing.T) { func TestCopyDirFailOpen(t *testing.T) { if runtime.GOOS == "windows" { // XXX: setting permissions works differently in - // Microsoft Windows. os.Chmod(..., 0222) below is not + // Microsoft Windows. os.Chmod(..., 0o222) below is not // enough for the file to be readonly, and os.Chmod(..., // 0000) returns an invalid argument error. Skipping // this this until a compatible implementation is @@ -273,7 +273,7 @@ func TestCopyDirFailOpen(t *testing.T) { defer os.RemoveAll(dir) srcdir = filepath.Join(dir, "src") - if err = os.MkdirAll(srcdir, 0755); err != nil { + if err = os.MkdirAll(srcdir, 0o755); err != nil { t.Fatal(err) } @@ -285,7 +285,7 @@ func TestCopyDirFailOpen(t *testing.T) { srcf.Close() // setup source file so that it cannot be read - if err = os.Chmod(srcfn, 0222); err != nil { + if err = os.Chmod(srcfn, 0o222); err != nil { t.Fatal(err) } @@ -419,11 +419,11 @@ func TestCopyFileLongFilePath(t *testing.T) { } fullPath := filepath.Join(dir, dirName, string(os.PathSeparator)) - if err := os.MkdirAll(fullPath, 0755); err != nil && !os.IsExist(err) { + if err := os.MkdirAll(fullPath, 0o755); err != nil && !os.IsExist(err) { t.Fatalf("%+v", fmt.Errorf("unable to create temp directory: %s", fullPath)) } - err = os.WriteFile(fullPath+"src", []byte(nil), 0644) + err = os.WriteFile(fullPath+"src", []byte(nil), 0o644) if err != nil { t.Fatalf("%+v", err) } @@ -460,7 +460,7 @@ func TestCopyFileFail(t *testing.T) { cleanup := setupInaccessibleDir(t, func(dir string) error { dstdir = filepath.Join(dir, "dir") - return os.Mkdir(dstdir, 0777) + return os.Mkdir(dstdir, 0o777) }) defer cleanup() @@ -493,7 +493,7 @@ func setupInaccessibleDir(t *testing.T, op func(dir string) error) func() { subdir := filepath.Join(dir, "dir") cleanup := func() { - if err := os.Chmod(subdir, 0777); err != nil { + if err := os.Chmod(subdir, 0o777); err != nil { t.Error(err) } if err := os.RemoveAll(dir); err != nil { @@ -501,7 +501,7 @@ func setupInaccessibleDir(t *testing.T, op func(dir string) error) func() { } } - if err := os.Mkdir(subdir, 0777); err != nil { + if err := os.Mkdir(subdir, 0o777); err != nil { cleanup() t.Fatal(err) return nil @@ -513,7 +513,7 @@ func setupInaccessibleDir(t *testing.T, op func(dir string) error) func() { return nil } - if err := os.Chmod(subdir, 0666); err != nil { + if err := os.Chmod(subdir, 0o666); err != nil { cleanup() t.Fatal(err) return nil @@ -532,7 +532,7 @@ func TestIsDir(t *testing.T) { cleanup := setupInaccessibleDir(t, func(dir string) error { dn = filepath.Join(dir, "dir") - return os.Mkdir(dn, 0777) + return os.Mkdir(dn, 0o777) }) defer cleanup() @@ -575,7 +575,7 @@ func TestIsSymlink(t *testing.T) { defer os.RemoveAll(dir) dirPath := filepath.Join(dir, "directory") - if err = os.MkdirAll(dirPath, 0777); err != nil { + if err = os.MkdirAll(dirPath, 0o777); err != nil { t.Fatal(err) } diff --git a/internal/helm/chart/builder_local_test.go b/internal/helm/chart/builder_local_test.go index 10f681fd3..e63888fff 100644 --- a/internal/helm/chart/builder_local_test.go +++ b/internal/helm/chart/builder_local_test.go @@ -201,7 +201,7 @@ fullnameOverride: "full-foo-name-override"`), // Write value file in the base dir. for _, f := range tt.valuesFiles { vPath := filepath.Join(localRef.WorkDir, f.Name) - g.Expect(os.WriteFile(vPath, f.Data, 0644)).ToNot(HaveOccurred()) + g.Expect(os.WriteFile(vPath, f.Data, 0o644)).ToNot(HaveOccurred()) } // Write chart dependencies in the base dir. @@ -336,7 +336,7 @@ func Test_mergeFileValues(t *testing.T) { defer os.RemoveAll(baseDir) for _, f := range tt.files { - g.Expect(os.WriteFile(filepath.Join(baseDir, f.Name), f.Data, 0644)).To(Succeed()) + g.Expect(os.WriteFile(filepath.Join(baseDir, f.Name), f.Data, 0o644)).To(Succeed()) } got, err := mergeFileValues(baseDir, tt.paths) diff --git a/internal/helm/chart/metadata_test.go b/internal/helm/chart/metadata_test.go index 07449100a..b7cf4611b 100644 --- a/internal/helm/chart/metadata_test.go +++ b/internal/helm/chart/metadata_test.go @@ -140,7 +140,7 @@ func TestLoadChartMetadataFromDir(t *testing.T) { copy.Copy("../testdata/charts/helmchart", tmpDir) bigRequirementsFile := filepath.Join(tmpDir, "requirements.yaml") data := make([]byte, helm.MaxChartFileSize+10) - g.Expect(os.WriteFile(bigRequirementsFile, data, 0644)).ToNot(HaveOccurred()) + g.Expect(os.WriteFile(bigRequirementsFile, data, 0o644)).ToNot(HaveOccurred()) tests := []struct { name string @@ -205,7 +205,7 @@ func TestLoadChartMetadataFromArchive(t *testing.T) { defer os.RemoveAll(tmpDir) bigArchiveFile := filepath.Join(tmpDir, "chart.tgz") data := make([]byte, helm.MaxChartSize+10) - g.Expect(os.WriteFile(bigArchiveFile, data, 0644)).ToNot(HaveOccurred()) + g.Expect(os.WriteFile(bigArchiveFile, data, 0o644)).ToNot(HaveOccurred()) tests := []struct { name string diff --git a/internal/helm/repository/chart_repository_test.go b/internal/helm/repository/chart_repository_test.go index cc5ed452f..e1fceffda 100644 --- a/internal/helm/repository/chart_repository_test.go +++ b/internal/helm/repository/chart_repository_test.go @@ -363,7 +363,7 @@ func TestChartRepository_LoadIndexFromFile(t *testing.T) { defer os.RemoveAll(tmpDir) bigIndexFile := filepath.Join(tmpDir, "index.yaml") data := make([]byte, helm.MaxIndexSize+10) - g.Expect(os.WriteFile(bigIndexFile, data, 0644)).ToNot(HaveOccurred()) + g.Expect(os.WriteFile(bigIndexFile, data, 0o644)).ToNot(HaveOccurred()) tests := []struct { name string diff --git a/main.go b/main.go index 57fd5e3a2..0be197a92 100644 --- a/main.go +++ b/main.go @@ -299,7 +299,7 @@ func mustInitStorage(path string, storageAdvAddr string, artifactRetentionTTL ti if path == "" { p, _ := os.Getwd() path = filepath.Join(p, "bin") - os.MkdirAll(path, 0777) + os.MkdirAll(path, 0o777) } storage, err := controllers.NewStorage(path, storageAdvAddr, artifactRetentionTTL, artifactRetentionRecords) diff --git a/pkg/gcp/gcp.go b/pkg/gcp/gcp.go index 836ba3419..419885cbb 100644 --- a/pkg/gcp/gcp.go +++ b/pkg/gcp/gcp.go @@ -118,7 +118,7 @@ func (c *GCSClient) FGetObject(ctx context.Context, bucketName, objectName, loca objectDir, _ := filepath.Split(localPath) if objectDir != "" { // Create any missing top level directories. - if err := os.MkdirAll(objectDir, 0700); err != nil { + if err := os.MkdirAll(objectDir, 0o700); err != nil { return "", err } } @@ -130,7 +130,7 @@ func (c *GCSClient) FGetObject(ctx context.Context, bucketName, objectName, loca } // Prepare target file. - objectFile, err := os.OpenFile(localPath, os.O_CREATE|os.O_WRONLY, 0600) + objectFile, err := os.OpenFile(localPath, os.O_CREATE|os.O_WRONLY, 0o600) if err != nil { return "", err } diff --git a/pkg/sourceignore/sourceignore_test.go b/pkg/sourceignore/sourceignore_test.go index 786868ba1..3b0324e6b 100644 --- a/pkg/sourceignore/sourceignore_test.go +++ b/pkg/sourceignore/sourceignore_test.go @@ -209,10 +209,10 @@ func TestLoadExcludePatterns(t *testing.T) { "a/b/.sourceignore": "subdir.txt", } for n, c := range files { - if err = os.MkdirAll(filepath.Join(tmpDir, filepath.Dir(n)), 0755); err != nil { + if err = os.MkdirAll(filepath.Join(tmpDir, filepath.Dir(n)), 0o755); err != nil { t.Fatal(err) } - if err = os.WriteFile(filepath.Join(tmpDir, n), []byte(c), 0644); err != nil { + if err = os.WriteFile(filepath.Join(tmpDir, n), []byte(c), 0o644); err != nil { t.Fatal(err) } } diff --git a/tests/fuzz/gitrepository_fuzzer.go b/tests/fuzz/gitrepository_fuzzer.go index d96877d40..32581d8b9 100644 --- a/tests/fuzz/gitrepository_fuzzer.go +++ b/tests/fuzz/gitrepository_fuzzer.go @@ -494,7 +494,7 @@ func createRandomFiles(f *fuzz.ConsumeFuzzer, fs billy.Filesystem, wt *git.Workt return errors.New("Dir contains '..'") } - err = fs.MkdirAll(dirPath, 0777) + err = fs.MkdirAll(dirPath, 0o777) if err != nil { return errors.New("Could not create the subDir") } From 37551f19643ff4509fe55d522b9f251b210c578f Mon Sep 17 00:00:00 2001 From: Peter Gundel Date: Tue, 12 Apr 2022 21:15:01 +0200 Subject: [PATCH 2/2] Reduce file permission As suggested by @pjbgf Co-authored-by: Paulo Gomes Co-authored-by: Paulo Gomes Signed-off-by: Peter Gundel --- controllers/bucket_controller_test.go | 4 +-- controllers/gitrepository_controller_test.go | 4 +-- controllers/helmchart_controller_test.go | 4 +-- controllers/helmrepository_controller_test.go | 4 +-- controllers/storage.go | 4 +-- controllers/storage_test.go | 12 +++---- internal/fs/fs_test.go | 36 +++++++++---------- internal/helm/chart/builder_local_test.go | 4 +-- internal/helm/chart/metadata_test.go | 4 +-- .../helm/chart/secureloader/directory_test.go | 26 +++++++------- .../helm/chart/secureloader/loader_test.go | 4 +-- .../helm/repository/chart_repository_test.go | 2 +- main.go | 2 +- pkg/sourceignore/sourceignore_test.go | 4 +-- tests/fuzz/gitrepository_fuzzer.go | 6 ++-- 15 files changed, 60 insertions(+), 60 deletions(-) diff --git a/controllers/bucket_controller_test.go b/controllers/bucket_controller_test.go index 70983231f..c36559713 100644 --- a/controllers/bucket_controller_test.go +++ b/controllers/bucket_controller_test.go @@ -187,7 +187,7 @@ func TestBucketReconciler_reconcileStorage(t *testing.T) { if err := testStorage.MkdirAll(*obj.Status.Artifact); err != nil { return err } - if err := testStorage.AtomicWriteFile(obj.Status.Artifact, strings.NewReader(v), 0o644); err != nil { + if err := testStorage.AtomicWriteFile(obj.Status.Artifact, strings.NewReader(v), 0o640); err != nil { return err } if n != len(revisions)-1 { @@ -242,7 +242,7 @@ func TestBucketReconciler_reconcileStorage(t *testing.T) { if err := testStorage.MkdirAll(*obj.Status.Artifact); err != nil { return err } - if err := testStorage.AtomicWriteFile(obj.Status.Artifact, strings.NewReader("file"), 0o644); err != nil { + if err := testStorage.AtomicWriteFile(obj.Status.Artifact, strings.NewReader("file"), 0o640); err != nil { return err } return nil diff --git a/controllers/gitrepository_controller_test.go b/controllers/gitrepository_controller_test.go index 7d000b85d..3c5f82e58 100644 --- a/controllers/gitrepository_controller_test.go +++ b/controllers/gitrepository_controller_test.go @@ -1128,7 +1128,7 @@ func TestGitRepositoryReconciler_reconcileStorage(t *testing.T) { if err := testStorage.MkdirAll(*obj.Status.Artifact); err != nil { return err } - if err := testStorage.AtomicWriteFile(obj.Status.Artifact, strings.NewReader(v), 0o644); err != nil { + if err := testStorage.AtomicWriteFile(obj.Status.Artifact, strings.NewReader(v), 0o640); err != nil { return err } if n != len(revisions)-1 { @@ -1183,7 +1183,7 @@ func TestGitRepositoryReconciler_reconcileStorage(t *testing.T) { if err := testStorage.MkdirAll(*obj.Status.Artifact); err != nil { return err } - if err := testStorage.AtomicWriteFile(obj.Status.Artifact, strings.NewReader("file"), 0o644); err != nil { + if err := testStorage.AtomicWriteFile(obj.Status.Artifact, strings.NewReader("file"), 0o640); err != nil { return err } return nil diff --git a/controllers/helmchart_controller_test.go b/controllers/helmchart_controller_test.go index 8938f493f..64f44019a 100644 --- a/controllers/helmchart_controller_test.go +++ b/controllers/helmchart_controller_test.go @@ -187,7 +187,7 @@ func TestHelmChartReconciler_reconcileStorage(t *testing.T) { if err := testStorage.MkdirAll(*obj.Status.Artifact); err != nil { return err } - if err := testStorage.AtomicWriteFile(obj.Status.Artifact, strings.NewReader(v), 0o644); err != nil { + if err := testStorage.AtomicWriteFile(obj.Status.Artifact, strings.NewReader(v), 0o640); err != nil { return err } if n != len(revisions)-1 { @@ -242,7 +242,7 @@ func TestHelmChartReconciler_reconcileStorage(t *testing.T) { if err := testStorage.MkdirAll(*obj.Status.Artifact); err != nil { return err } - if err := testStorage.AtomicWriteFile(obj.Status.Artifact, strings.NewReader("file"), 0o644); err != nil { + if err := testStorage.AtomicWriteFile(obj.Status.Artifact, strings.NewReader("file"), 0o640); err != nil { return err } return nil diff --git a/controllers/helmrepository_controller_test.go b/controllers/helmrepository_controller_test.go index 3f48dc2e2..b25578925 100644 --- a/controllers/helmrepository_controller_test.go +++ b/controllers/helmrepository_controller_test.go @@ -158,7 +158,7 @@ func TestHelmRepositoryReconciler_reconcileStorage(t *testing.T) { if err := testStorage.MkdirAll(*obj.Status.Artifact); err != nil { return err } - if err := testStorage.AtomicWriteFile(obj.Status.Artifact, strings.NewReader(v), 0o644); err != nil { + if err := testStorage.AtomicWriteFile(obj.Status.Artifact, strings.NewReader(v), 0o640); err != nil { return err } if n != len(revisions)-1 { @@ -213,7 +213,7 @@ func TestHelmRepositoryReconciler_reconcileStorage(t *testing.T) { if err := testStorage.MkdirAll(*obj.Status.Artifact); err != nil { return err } - if err := testStorage.AtomicWriteFile(obj.Status.Artifact, strings.NewReader("file"), 0o644); err != nil { + if err := testStorage.AtomicWriteFile(obj.Status.Artifact, strings.NewReader("file"), 0o640); err != nil { return err } return nil diff --git a/controllers/storage.go b/controllers/storage.go index bcd1b17e0..13f1fa0aa 100644 --- a/controllers/storage.go +++ b/controllers/storage.go @@ -112,7 +112,7 @@ func (s Storage) SetHostname(URL string) string { // MkdirAll calls os.MkdirAll for the given v1beta1.Artifact base dir. func (s *Storage) MkdirAll(artifact sourcev1.Artifact) error { dir := filepath.Dir(s.LocalPath(artifact)) - return os.MkdirAll(dir, 0o777) + return os.MkdirAll(dir, 0o770) } // RemoveAll calls os.RemoveAll for the given v1beta1.Artifact base dir. @@ -432,7 +432,7 @@ func (s *Storage) Archive(artifact *sourcev1.Artifact, dir string, filter Archiv return err } - if err := os.Chmod(tmpName, 0o644); err != nil { + if err := os.Chmod(tmpName, 0o640); err != nil { return err } diff --git a/controllers/storage_test.go b/controllers/storage_test.go index a699ff5e4..e3550bd59 100644 --- a/controllers/storage_test.go +++ b/controllers/storage_test.go @@ -136,7 +136,7 @@ func TestStorage_Archive(t *testing.T) { } for name, b := range files { absPath := filepath.Join(dir, name) - if err = os.MkdirAll(filepath.Dir(absPath), 0o755); err != nil { + if err = os.MkdirAll(filepath.Dir(absPath), 0o750); err != nil { return } f, err := os.Create(absPath) @@ -316,7 +316,7 @@ func TestStorageRemoveAllButCurrent(t *testing.T) { // Create artifact dir and artifacts. artifactDir := path.Join(dir, "foo", "bar") - g.Expect(os.MkdirAll(artifactDir, 0o755)).NotTo(HaveOccurred()) + g.Expect(os.MkdirAll(artifactDir, 0o750)).NotTo(HaveOccurred()) current := []string{ path.Join(artifactDir, "artifact1.tar.gz"), } @@ -378,7 +378,7 @@ func TestStorageRemoveAll(t *testing.T) { } if tt.createArtifactPath { - g.Expect(os.MkdirAll(path.Join(dir, tt.artifactPath), 0o755)).ToNot(HaveOccurred()) + g.Expect(os.MkdirAll(path.Join(dir, tt.artifactPath), 0o750)).ToNot(HaveOccurred()) } deleted, err := s.RemoveAll(artifact) @@ -412,7 +412,7 @@ func TestStorageCopyFromPath(t *testing.T) { } t.Cleanup(cleanupStoragePath(dir)) absPath = filepath.Join(dir, file.Name) - if err = os.MkdirAll(filepath.Dir(absPath), 0o755); err != nil { + if err = os.MkdirAll(filepath.Dir(absPath), 0o750); err != nil { return } f, err := os.Create(absPath) @@ -591,7 +591,7 @@ func TestStorage_getGarbageFiles(t *testing.T) { artifact := sourcev1.Artifact{ Path: tt.artifactPaths[len(tt.artifactPaths)-1], } - g.Expect(os.MkdirAll(path.Join(dir, artifactFolder), 0o755)).ToNot(HaveOccurred()) + g.Expect(os.MkdirAll(path.Join(dir, artifactFolder), 0o750)).ToNot(HaveOccurred()) for _, artifactPath := range tt.artifactPaths { f, err := os.Create(path.Join(dir, artifactPath)) g.Expect(err).ToNot(HaveOccurred()) @@ -667,7 +667,7 @@ func TestStorage_GarbageCollect(t *testing.T) { artifact := sourcev1.Artifact{ Path: tt.artifactPaths[len(tt.artifactPaths)-1], } - g.Expect(os.MkdirAll(path.Join(dir, artifactFolder), 0o755)).ToNot(HaveOccurred()) + g.Expect(os.MkdirAll(path.Join(dir, artifactFolder), 0o750)).ToNot(HaveOccurred()) for i, artifactPath := range tt.artifactPaths { f, err := os.Create(path.Join(dir, artifactPath)) g.Expect(err).ToNot(HaveOccurred()) diff --git a/internal/fs/fs_test.go b/internal/fs/fs_test.go index 4f3a4f20f..fa51eae4a 100644 --- a/internal/fs/fs_test.go +++ b/internal/fs/fs_test.go @@ -42,12 +42,12 @@ func TestRenameWithFallback(t *testing.T) { } srcpath = filepath.Join(dir, "a") - if err = os.MkdirAll(srcpath, 0o777); err != nil { + if err = os.MkdirAll(srcpath, 0o770); err != nil { t.Fatal(err) } dstpath := filepath.Join(dir, "b") - if err = os.MkdirAll(dstpath, 0o777); err != nil { + if err = os.MkdirAll(dstpath, 0o770); err != nil { t.Fatal(err) } @@ -64,7 +64,7 @@ func TestCopyDir(t *testing.T) { defer os.RemoveAll(dir) srcdir := filepath.Join(dir, "src") - if err := os.MkdirAll(srcdir, 0o755); err != nil { + if err := os.MkdirAll(srcdir, 0o750); err != nil { t.Fatal(err) } @@ -81,7 +81,7 @@ func TestCopyDir(t *testing.T) { for i, file := range files { fn := filepath.Join(srcdir, file.path) dn := filepath.Dir(fn) - if err = os.MkdirAll(dn, 0o755); err != nil { + if err = os.MkdirAll(dn, 0o750); err != nil { t.Fatal(err) } @@ -151,7 +151,7 @@ func TestCopyDirFail_SrcInaccessible(t *testing.T) { cleanup := setupInaccessibleDir(t, func(dir string) error { srcdir = filepath.Join(dir, "src") - return os.MkdirAll(srcdir, 0o755) + return os.MkdirAll(srcdir, 0o750) }) defer cleanup() @@ -184,7 +184,7 @@ func TestCopyDirFail_DstInaccessible(t *testing.T) { defer os.RemoveAll(dir) srcdir = filepath.Join(dir, "src") - if err = os.MkdirAll(srcdir, 0o755); err != nil { + if err = os.MkdirAll(srcdir, 0o750); err != nil { t.Fatal(err) } @@ -235,12 +235,12 @@ func TestCopyDirFail_DstExists(t *testing.T) { defer os.RemoveAll(dir) srcdir = filepath.Join(dir, "src") - if err = os.MkdirAll(srcdir, 0o755); err != nil { + if err = os.MkdirAll(srcdir, 0o750); err != nil { t.Fatal(err) } dstdir = filepath.Join(dir, "dst") - if err = os.MkdirAll(dstdir, 0o755); err != nil { + if err = os.MkdirAll(dstdir, 0o750); err != nil { t.Fatal(err) } @@ -273,7 +273,7 @@ func TestCopyDirFailOpen(t *testing.T) { defer os.RemoveAll(dir) srcdir = filepath.Join(dir, "src") - if err = os.MkdirAll(srcdir, 0o755); err != nil { + if err = os.MkdirAll(srcdir, 0o750); err != nil { t.Fatal(err) } @@ -285,7 +285,7 @@ func TestCopyDirFailOpen(t *testing.T) { srcf.Close() // setup source file so that it cannot be read - if err = os.Chmod(srcfn, 0o222); err != nil { + if err = os.Chmod(srcfn, 0o220); err != nil { t.Fatal(err) } @@ -419,11 +419,11 @@ func TestCopyFileLongFilePath(t *testing.T) { } fullPath := filepath.Join(dir, dirName, string(os.PathSeparator)) - if err := os.MkdirAll(fullPath, 0o755); err != nil && !os.IsExist(err) { + if err := os.MkdirAll(fullPath, 0o750); err != nil && !os.IsExist(err) { t.Fatalf("%+v", fmt.Errorf("unable to create temp directory: %s", fullPath)) } - err = os.WriteFile(fullPath+"src", []byte(nil), 0o644) + err = os.WriteFile(fullPath+"src", []byte(nil), 0o640) if err != nil { t.Fatalf("%+v", err) } @@ -460,7 +460,7 @@ func TestCopyFileFail(t *testing.T) { cleanup := setupInaccessibleDir(t, func(dir string) error { dstdir = filepath.Join(dir, "dir") - return os.Mkdir(dstdir, 0o777) + return os.Mkdir(dstdir, 0o770) }) defer cleanup() @@ -493,7 +493,7 @@ func setupInaccessibleDir(t *testing.T, op func(dir string) error) func() { subdir := filepath.Join(dir, "dir") cleanup := func() { - if err := os.Chmod(subdir, 0o777); err != nil { + if err := os.Chmod(subdir, 0o770); err != nil { t.Error(err) } if err := os.RemoveAll(dir); err != nil { @@ -501,7 +501,7 @@ func setupInaccessibleDir(t *testing.T, op func(dir string) error) func() { } } - if err := os.Mkdir(subdir, 0o777); err != nil { + if err := os.Mkdir(subdir, 0o770); err != nil { cleanup() t.Fatal(err) return nil @@ -513,7 +513,7 @@ func setupInaccessibleDir(t *testing.T, op func(dir string) error) func() { return nil } - if err := os.Chmod(subdir, 0o666); err != nil { + if err := os.Chmod(subdir, 0o660); err != nil { cleanup() t.Fatal(err) return nil @@ -532,7 +532,7 @@ func TestIsDir(t *testing.T) { cleanup := setupInaccessibleDir(t, func(dir string) error { dn = filepath.Join(dir, "dir") - return os.Mkdir(dn, 0o777) + return os.Mkdir(dn, 0o770) }) defer cleanup() @@ -575,7 +575,7 @@ func TestIsSymlink(t *testing.T) { defer os.RemoveAll(dir) dirPath := filepath.Join(dir, "directory") - if err = os.MkdirAll(dirPath, 0o777); err != nil { + if err = os.MkdirAll(dirPath, 0o770); err != nil { t.Fatal(err) } diff --git a/internal/helm/chart/builder_local_test.go b/internal/helm/chart/builder_local_test.go index e63888fff..57f828f4c 100644 --- a/internal/helm/chart/builder_local_test.go +++ b/internal/helm/chart/builder_local_test.go @@ -201,7 +201,7 @@ fullnameOverride: "full-foo-name-override"`), // Write value file in the base dir. for _, f := range tt.valuesFiles { vPath := filepath.Join(localRef.WorkDir, f.Name) - g.Expect(os.WriteFile(vPath, f.Data, 0o644)).ToNot(HaveOccurred()) + g.Expect(os.WriteFile(vPath, f.Data, 0o640)).ToNot(HaveOccurred()) } // Write chart dependencies in the base dir. @@ -336,7 +336,7 @@ func Test_mergeFileValues(t *testing.T) { defer os.RemoveAll(baseDir) for _, f := range tt.files { - g.Expect(os.WriteFile(filepath.Join(baseDir, f.Name), f.Data, 0o644)).To(Succeed()) + g.Expect(os.WriteFile(filepath.Join(baseDir, f.Name), f.Data, 0o640)).To(Succeed()) } got, err := mergeFileValues(baseDir, tt.paths) diff --git a/internal/helm/chart/metadata_test.go b/internal/helm/chart/metadata_test.go index b7cf4611b..20c39bc7a 100644 --- a/internal/helm/chart/metadata_test.go +++ b/internal/helm/chart/metadata_test.go @@ -140,7 +140,7 @@ func TestLoadChartMetadataFromDir(t *testing.T) { copy.Copy("../testdata/charts/helmchart", tmpDir) bigRequirementsFile := filepath.Join(tmpDir, "requirements.yaml") data := make([]byte, helm.MaxChartFileSize+10) - g.Expect(os.WriteFile(bigRequirementsFile, data, 0o644)).ToNot(HaveOccurred()) + g.Expect(os.WriteFile(bigRequirementsFile, data, 0o640)).ToNot(HaveOccurred()) tests := []struct { name string @@ -205,7 +205,7 @@ func TestLoadChartMetadataFromArchive(t *testing.T) { defer os.RemoveAll(tmpDir) bigArchiveFile := filepath.Join(tmpDir, "chart.tgz") data := make([]byte, helm.MaxChartSize+10) - g.Expect(os.WriteFile(bigArchiveFile, data, 0o644)).ToNot(HaveOccurred()) + g.Expect(os.WriteFile(bigArchiveFile, data, 0o640)).ToNot(HaveOccurred()) tests := []struct { name string diff --git a/internal/helm/chart/secureloader/directory_test.go b/internal/helm/chart/secureloader/directory_test.go index a0594fd74..5dacfc7d8 100644 --- a/internal/helm/chart/secureloader/directory_test.go +++ b/internal/helm/chart/secureloader/directory_test.go @@ -49,7 +49,7 @@ func TestSecureDirLoader_Load(t *testing.T) { m := metadata b, err := yaml.Marshal(&m) g.Expect(err).ToNot(HaveOccurred()) - g.Expect(os.WriteFile(filepath.Join(tmpDir, "Chart.yaml"), b, 0o644)).To(Succeed()) + g.Expect(os.WriteFile(filepath.Join(tmpDir, "Chart.yaml"), b, 0o640)).To(Succeed()) got, err := (NewSecureDirLoader(tmpDir, "", helm.MaxChartFileSize)).Load() g.Expect(err).ToNot(HaveOccurred()) @@ -64,7 +64,7 @@ func TestSecureDirLoader_Load(t *testing.T) { m := metadata b, err := yaml.Marshal(&m) g.Expect(err).ToNot(HaveOccurred()) - g.Expect(os.WriteFile(filepath.Join(tmpDir, "Chart.yaml"), b, 0o644)).To(Succeed()) + g.Expect(os.WriteFile(filepath.Join(tmpDir, "Chart.yaml"), b, 0o640)).To(Succeed()) got, err := (NewSecureDirLoader(tmpDir, tmpDir, helm.MaxChartFileSize)).Load() g.Expect(err).ToNot(HaveOccurred()) @@ -80,7 +80,7 @@ func TestSecureDirLoader_Load(t *testing.T) { m := metadata b, err := yaml.Marshal(&m) g.Expect(err).ToNot(HaveOccurred()) - g.Expect(os.WriteFile(filepath.Join(tmpDir, "Chart.yaml"), b, 0o644)).To(Succeed()) + g.Expect(os.WriteFile(filepath.Join(tmpDir, "Chart.yaml"), b, 0o640)).To(Succeed()) root := filepath.Join(tmpDir, "root") g.Expect(os.Mkdir(root, 0o700)).To(Succeed()) @@ -103,9 +103,9 @@ func TestSecureDirLoader_Load(t *testing.T) { m := metadata b, err := yaml.Marshal(&m) g.Expect(err).ToNot(HaveOccurred()) - g.Expect(os.WriteFile(filepath.Join(tmpDir, "Chart.yaml"), b, 0o644)).To(Succeed()) - g.Expect(os.WriteFile(filepath.Join(tmpDir, ignore.HelmIgnore), []byte("file.txt"), 0o644)).To(Succeed()) - g.Expect(os.WriteFile(filepath.Join(tmpDir, "file.txt"), []byte("not included"), 0o644)).To(Succeed()) + g.Expect(os.WriteFile(filepath.Join(tmpDir, "Chart.yaml"), b, 0o640)).To(Succeed()) + g.Expect(os.WriteFile(filepath.Join(tmpDir, ignore.HelmIgnore), []byte("file.txt"), 0o640)).To(Succeed()) + g.Expect(os.WriteFile(filepath.Join(tmpDir, "file.txt"), []byte("not included"), 0o640)).To(Succeed()) got, err := (NewSecureDirLoader(tmpDir, "", helm.MaxChartFileSize)).Load() g.Expect(err).ToNot(HaveOccurred()) @@ -129,7 +129,7 @@ func Test_secureLoadIgnoreRules(t *testing.T) { g := NewWithT(t) tmpDir := t.TempDir() - g.Expect(os.WriteFile(filepath.Join(tmpDir, ignore.HelmIgnore), []byte("file.txt"), 0o644)).To(Succeed()) + g.Expect(os.WriteFile(filepath.Join(tmpDir, ignore.HelmIgnore), []byte("file.txt"), 0o640)).To(Succeed()) r, err := secureLoadIgnoreRules(tmpDir, "") g.Expect(err).ToNot(HaveOccurred()) @@ -144,7 +144,7 @@ func Test_secureLoadIgnoreRules(t *testing.T) { tmpDir := t.TempDir() chartPath := "./sub/chart" g.Expect(os.MkdirAll(filepath.Join(tmpDir, chartPath), 0o700)).To(Succeed()) - g.Expect(os.WriteFile(filepath.Join(tmpDir, chartPath, ignore.HelmIgnore), []byte("file.txt"), 0o644)).To(Succeed()) + g.Expect(os.WriteFile(filepath.Join(tmpDir, chartPath, ignore.HelmIgnore), []byte("file.txt"), 0o640)).To(Succeed()) r, err := secureLoadIgnoreRules(tmpDir, chartPath) g.Expect(err).ToNot(HaveOccurred()) @@ -157,7 +157,7 @@ func Test_secureLoadIgnoreRules(t *testing.T) { tmpDir := t.TempDir() chartPath := "sub/chart" g.Expect(os.MkdirAll(filepath.Join(tmpDir, chartPath), 0o700)).To(Succeed()) - g.Expect(os.WriteFile(filepath.Join(tmpDir, "symlink"), []byte("file.txt"), 0o644)).To(Succeed()) + g.Expect(os.WriteFile(filepath.Join(tmpDir, "symlink"), []byte("file.txt"), 0o640)).To(Succeed()) g.Expect(os.Symlink("../../symlink", filepath.Join(tmpDir, chartPath, ignore.HelmIgnore))) r, err := secureLoadIgnoreRules(tmpDir, chartPath) @@ -171,7 +171,7 @@ func Test_secureLoadIgnoreRules(t *testing.T) { tmpDir := t.TempDir() chartPath := "/sub/chart" g.Expect(os.MkdirAll(filepath.Join(tmpDir, chartPath), 0o700)).To(Succeed()) - g.Expect(os.WriteFile(filepath.Join(tmpDir, "symlink"), []byte("file.txt"), 0o644)).To(Succeed()) + g.Expect(os.WriteFile(filepath.Join(tmpDir, "symlink"), []byte("file.txt"), 0o640)).To(Succeed()) g.Expect(os.Symlink("../../symlink", filepath.Join(tmpDir, chartPath, ignore.HelmIgnore))) r, err := secureLoadIgnoreRules(filepath.Join(tmpDir, chartPath), "") @@ -184,7 +184,7 @@ func Test_secureLoadIgnoreRules(t *testing.T) { g := NewWithT(t) tmpDir := t.TempDir() - g.Expect(os.WriteFile(filepath.Join(tmpDir, ignore.HelmIgnore), []byte("**"), 0o644)).To(Succeed()) + g.Expect(os.WriteFile(filepath.Join(tmpDir, ignore.HelmIgnore), []byte("**"), 0o640)).To(Succeed()) _, err := secureLoadIgnoreRules(tmpDir, "") g.Expect(err).To(HaveOccurred()) @@ -319,7 +319,7 @@ func Test_secureFileWalker_walk(t *testing.T) { fileName := "append-file" fileData := []byte("append-file-data") absFilePath := filepath.Join(tmpDir, fileName) - g.Expect(os.WriteFile(absFilePath, fileData, 0o644)).To(Succeed()) + g.Expect(os.WriteFile(absFilePath, fileData, 0o640)).To(Succeed()) fileInfo, err := os.Lstat(absFilePath) g.Expect(err).ToNot(HaveOccurred()) @@ -338,7 +338,7 @@ func Test_secureFileWalker_walk(t *testing.T) { fileData := []byte("append-file-data") fileDataWithBom := append(utf8bom, fileData...) absFilePath := filepath.Join(tmpDir, fileName) - g.Expect(os.WriteFile(absFilePath, fileDataWithBom, 0o644)).To(Succeed()) + g.Expect(os.WriteFile(absFilePath, fileDataWithBom, 0o640)).To(Succeed()) fileInfo, err := os.Lstat(absFilePath) g.Expect(err).ToNot(HaveOccurred()) diff --git a/internal/helm/chart/secureloader/loader_test.go b/internal/helm/chart/secureloader/loader_test.go index 374948cdb..af7de550e 100644 --- a/internal/helm/chart/secureloader/loader_test.go +++ b/internal/helm/chart/secureloader/loader_test.go @@ -35,7 +35,7 @@ func TestLoader(t *testing.T) { tmpDir := t.TempDir() fakeChart := filepath.Join(tmpDir, "fake.tgz") - g.Expect(os.WriteFile(fakeChart, []byte(""), 0o644)).To(Succeed()) + g.Expect(os.WriteFile(fakeChart, []byte(""), 0o640)).To(Succeed()) t.Run("file loader", func(t *testing.T) { g := NewWithT(t) @@ -83,7 +83,7 @@ func TestLoad(t *testing.T) { } b, err := yaml.Marshal(&metadata) g.Expect(err).ToNot(HaveOccurred()) - g.Expect(os.WriteFile(filepath.Join(tmpDir, "Chart.yaml"), b, 0o644)).To(Succeed()) + g.Expect(os.WriteFile(filepath.Join(tmpDir, "Chart.yaml"), b, 0o640)).To(Succeed()) got, err := Load(tmpDir, "") g.Expect(err).ToNot(HaveOccurred()) diff --git a/internal/helm/repository/chart_repository_test.go b/internal/helm/repository/chart_repository_test.go index e1fceffda..ce33d8f0f 100644 --- a/internal/helm/repository/chart_repository_test.go +++ b/internal/helm/repository/chart_repository_test.go @@ -363,7 +363,7 @@ func TestChartRepository_LoadIndexFromFile(t *testing.T) { defer os.RemoveAll(tmpDir) bigIndexFile := filepath.Join(tmpDir, "index.yaml") data := make([]byte, helm.MaxIndexSize+10) - g.Expect(os.WriteFile(bigIndexFile, data, 0o644)).ToNot(HaveOccurred()) + g.Expect(os.WriteFile(bigIndexFile, data, 0o640)).ToNot(HaveOccurred()) tests := []struct { name string diff --git a/main.go b/main.go index 0be197a92..5b6af0e39 100644 --- a/main.go +++ b/main.go @@ -299,7 +299,7 @@ func mustInitStorage(path string, storageAdvAddr string, artifactRetentionTTL ti if path == "" { p, _ := os.Getwd() path = filepath.Join(p, "bin") - os.MkdirAll(path, 0o777) + os.MkdirAll(path, 0o770) } storage, err := controllers.NewStorage(path, storageAdvAddr, artifactRetentionTTL, artifactRetentionRecords) diff --git a/pkg/sourceignore/sourceignore_test.go b/pkg/sourceignore/sourceignore_test.go index 3b0324e6b..cbd73352d 100644 --- a/pkg/sourceignore/sourceignore_test.go +++ b/pkg/sourceignore/sourceignore_test.go @@ -209,10 +209,10 @@ func TestLoadExcludePatterns(t *testing.T) { "a/b/.sourceignore": "subdir.txt", } for n, c := range files { - if err = os.MkdirAll(filepath.Join(tmpDir, filepath.Dir(n)), 0o755); err != nil { + if err = os.MkdirAll(filepath.Join(tmpDir, filepath.Dir(n)), 0o750); err != nil { t.Fatal(err) } - if err = os.WriteFile(filepath.Join(tmpDir, n), []byte(c), 0o644); err != nil { + if err = os.WriteFile(filepath.Join(tmpDir, n), []byte(c), 0o640); err != nil { t.Fatal(err) } } diff --git a/tests/fuzz/gitrepository_fuzzer.go b/tests/fuzz/gitrepository_fuzzer.go index 32581d8b9..20b0e3a45 100644 --- a/tests/fuzz/gitrepository_fuzzer.go +++ b/tests/fuzz/gitrepository_fuzzer.go @@ -120,7 +120,7 @@ func ensureDependencies() error { // Output all embedded testdata files embedDirs := []string{"testdata/crd", "testdata/certs"} for _, dir := range embedDirs { - err := os.MkdirAll(dir, 0o755) + err := os.MkdirAll(dir, 0o750) if err != nil { return fmt.Errorf("mkdir %s: %v", dir, err) } @@ -139,7 +139,7 @@ func ensureDependencies() error { return fmt.Errorf("reading embedded file %s: %v", fileName, err) } - os.WriteFile(fileName, data, 0o644) + os.WriteFile(fileName, data, 0o640) if err != nil { return fmt.Errorf("writing %s: %v", fileName, err) } @@ -494,7 +494,7 @@ func createRandomFiles(f *fuzz.ConsumeFuzzer, fs billy.Filesystem, wt *git.Workt return errors.New("Dir contains '..'") } - err = fs.MkdirAll(dirPath, 0o777) + err = fs.MkdirAll(dirPath, 0o770) if err != nil { return errors.New("Could not create the subDir") }