From 8e517cca1704e5af68769e9401e51ea139022f9c Mon Sep 17 00:00:00 2001 From: Carlos Alexandro Becker Date: Tue, 23 May 2023 22:45:48 -0300 Subject: [PATCH] Merge pull request from GHSA-w7jw-q4fg-qc4c * feat(security): adds the umask option closes https://github.com/goreleaser/nfpm/security/advisories/GHSA-w7jw-q4fg-qc4c Signed-off-by: Carlos Alexandro Becker * fix: correct bitwise op --------- Signed-off-by: Carlos Alexandro Becker --- files/files.go | 34 +++++++------ files/files_test.go | 51 +++++++++++-------- .../testdata/deep-paths/nested1/nested2/a.txt | 0 nfpm.go | 5 +- www/docs/configuration.md | 9 ++++ 5 files changed, 60 insertions(+), 39 deletions(-) mode change 100644 => 100755 files/testdata/deep-paths/nested1/nested2/a.txt diff --git a/files/files.go b/files/files.go index 7e1b8cc8..2705f508 100644 --- a/files/files.go +++ b/files/files.go @@ -104,7 +104,7 @@ func (c Contents) ContainsDestination(dst string) bool { return false } -func (c *Content) WithFileInfoDefaults() *Content { +func (c *Content) WithFileInfoDefaults(umask fs.FileMode) *Content { cc := &Content{ Source: c.Source, Destination: c.Destination, @@ -144,7 +144,8 @@ func (c *Content) WithFileInfoDefaults() *Content { cc.FileInfo.MTime = info.ModTime() } if cc.FileInfo.Mode == 0 { - cc.FileInfo.Mode = info.Mode() + fmt.Println("files.go:147", info.Mode().String(), (info.Mode() & ^umask).String()) + cc.FileInfo.Mode = info.Mode() &^ umask } cc.FileInfo.Size = info.Size() } @@ -226,13 +227,14 @@ func (c *Content) String() string { // - It expands globs (if enabled) and file trees // - It adds implicit directories (parent directories of files) // - It adds ownership and other file information if not specified directly +// - It applies the given umask if the file does not have a specific mode // - It normalizes content source paths to be unix style paths // - It normalizes content destination paths to be absolute paths with a trailing // slash if the entry is a directory // // If no packager is specified, only the files that are relevant for any // packager are considered. -func PrepareForPackager(rawContents Contents, packager string, disableGlobbing bool) (Contents, error) { +func PrepareForPackager(rawContents Contents, umask fs.FileMode, packager string, disableGlobbing bool) (Contents, error) { contentMap := make(map[string]*Content) for _, content := range rawContents { @@ -253,13 +255,13 @@ func PrepareForPackager(rawContents Contents, packager string, disableGlobbing b return nil, err } - cc := content.WithFileInfoDefaults() + cc := content.WithFileInfoDefaults(umask) cc.Source = ToNixPath(cc.Source) cc.Destination = NormalizeAbsoluteDirPath(cc.Destination) contentMap[cc.Destination] = cc case TypeImplicitDir: - // if theres an implicit directory, the contents probably already - // have been expanend so we can just ignore it, it will be created + // if there's an implicit directory, the contents probably already + // have been expanded so we can just ignore it, it will be created // by another content element again anyway case TypeRPMGhost, TypeSymlink, TypeRPMDoc, TypeRPMLicence, TypeRPMLicense, TypeRPMReadme, TypeDebChangelog: presentContent, destinationOccupied := contentMap[NormalizeAbsoluteFilePath(content.Destination)] @@ -272,12 +274,12 @@ func PrepareForPackager(rawContents Contents, packager string, disableGlobbing b return nil, err } - cc := content.WithFileInfoDefaults() + cc := content.WithFileInfoDefaults(umask) cc.Source = ToNixPath(cc.Source) cc.Destination = NormalizeAbsoluteFilePath(cc.Destination) contentMap[cc.Destination] = cc case TypeTree: - err := addTree(contentMap, content) + err := addTree(contentMap, content, umask) if err != nil { return nil, fmt.Errorf("add tree: %w", err) } @@ -287,7 +289,7 @@ func PrepareForPackager(rawContents Contents, packager string, disableGlobbing b return nil, err } - err = addGlobbedFiles(contentMap, globbed, content) + err = addGlobbedFiles(contentMap, globbed, content, umask) if err != nil { return nil, fmt.Errorf("add globbed files from %q: %w", content.Source, err) } @@ -384,7 +386,7 @@ func sortedParents(dst string) []string { return paths } -func addGlobbedFiles(all map[string]*Content, globbed map[string]string, origFile *Content) error { +func addGlobbedFiles(all map[string]*Content, globbed map[string]string, origFile *Content, umask fs.FileMode) error { for src, dst := range globbed { dst = NormalizeAbsoluteFilePath(dst) presentContent, destinationOccupied := all[dst] @@ -413,7 +415,7 @@ func addGlobbedFiles(all map[string]*Content, globbed map[string]string, origFil Type: origFile.Type, FileInfo: newFileInfo, Packager: origFile.Packager, - }).WithFileInfoDefaults() + }).WithFileInfoDefaults(umask) if dst, err := os.Readlink(src); err == nil { newFile.Source = dst newFile.Type = TypeSymlink @@ -425,7 +427,7 @@ func addGlobbedFiles(all map[string]*Content, globbed map[string]string, origFil return nil } -func addTree(all map[string]*Content, tree *Content) error { +func addTree(all map[string]*Content, tree *Content, umask os.FileMode) error { if tree.Destination != "/" && tree.Destination != "" { presentContent, destinationOccupied := all[NormalizeAbsoluteDirPath(tree.Destination)] if destinationOccupied && presentContent.Type != TypeImplicitDir { @@ -461,10 +463,11 @@ func addTree(all map[string]*Content, tree *Content) error { c.Type = TypeDir c.Destination = NormalizeAbsoluteDirPath(destination) + fmt.Println("files.go:446", info.Mode().String(), (info.Mode() &^ umask).String()) c.FileInfo = &ContentFileInfo{ Owner: "root", Group: "root", - Mode: info.Mode(), + Mode: info.Mode() &^ umask, MTime: info.ModTime(), } case d.Type()&os.ModeSymlink != 0: @@ -480,12 +483,13 @@ func addTree(all map[string]*Content, tree *Content) error { c.Source = path c.Destination = NormalizeAbsoluteFilePath(destination) c.Type = TypeFile + fmt.Println("files.go:486", d.Type().String(), (d.Type() &^ umask).String()) c.FileInfo = &ContentFileInfo{ - Mode: d.Type(), + Mode: d.Type() &^ umask, } } - all[c.Destination] = c.WithFileInfoDefaults() + all[c.Destination] = c.WithFileInfoDefaults(umask) return nil }) diff --git a/files/files_test.go b/files/files_test.go index 4e5ac1b5..e623e01e 100644 --- a/files/files_test.go +++ b/files/files_test.go @@ -43,7 +43,7 @@ contents: } } -func TestDeepPathsWithGlob(t *testing.T) { +func TestDeepPathsWithGlobAndUmask(t *testing.T) { var config testStruct dec := yaml.NewDecoder(strings.NewReader(`--- contents: @@ -52,19 +52,26 @@ contents: file_info: mode: 0644 mtime: 2008-01-02T15:04:05Z +- src: testdata/deep-paths/ + dst: /bla `)) dec.KnownFields(true) err := dec.Decode(&config) require.NoError(t, err) - require.Len(t, config.Contents, 1) - parsedContents, err := files.PrepareForPackager(config.Contents, "", false) + require.Len(t, config.Contents, 2) + parsedContents, err := files.PrepareForPackager(config.Contents, 0o113, "", false) require.NoError(t, err) for _, c := range parsedContents { switch c.Source { case "testdata/globtest/nested/b.txt": require.Equal(t, "/bla/nested/b.txt", c.Destination) + require.Equal(t, "-rw-r--r--", c.Mode().String()) case "testdata/globtest/multi-nested/subdir/c.txt": require.Equal(t, "/bla/multi-nested/subdir/c.txt", c.Destination) + require.Equal(t, "-rw-r--r--", c.Mode().String()) + case "testdata/deep-paths/nested1/nested2/a.txt": + require.Equal(t, "/bla/nested1/nested2/a.txt", c.Destination) + require.Equal(t, "-rw-rw-r--", c.Mode().String()) } } } @@ -80,7 +87,7 @@ contents: err := dec.Decode(&config) require.NoError(t, err) require.Len(t, config.Contents, 1) - parsedContents, err := files.PrepareForPackager(config.Contents, "", true) + parsedContents, err := files.PrepareForPackager(config.Contents, 0, "", true) require.NoError(t, err) present := false @@ -110,7 +117,7 @@ contents: err := dec.Decode(&config) require.NoError(t, err) - config.Contents, err = files.PrepareForPackager(config.Contents, "", true) + config.Contents, err = files.PrepareForPackager(config.Contents, 0, "", true) require.NoError(t, err) require.Len(t, config.Contents, 1) @@ -140,7 +147,7 @@ contents: err := dec.Decode(&config) require.NoError(t, err) - config.Contents, err = files.PrepareForPackager(config.Contents, "rpm", true) + config.Contents, err = files.PrepareForPackager(config.Contents, 0, "rpm", true) require.NoError(t, err) require.Len(t, config.Contents, 1) @@ -172,7 +179,7 @@ contents: err := dec.Decode(&config) require.NoError(t, err) - config.Contents, err = files.PrepareForPackager(config.Contents, "", true) + config.Contents, err = files.PrepareForPackager(config.Contents, 0, "", true) require.NoError(t, err) config.Contents = withoutFileInfo(config.Contents) @@ -232,7 +239,7 @@ contents: wg.Add(1) go func() { defer wg.Done() - _, err := files.PrepareForPackager(config.Contents, "", false) + _, err := files.PrepareForPackager(config.Contents, 0, "", false) require.NoError(t, err) }() } @@ -246,7 +253,7 @@ func TestCollision(t *testing.T) { {Source: "../testdata/whatever2.conf", Destination: "/samedestination"}, } - _, err := files.PrepareForPackager(configuredFiles, "", true) + _, err := files.PrepareForPackager(configuredFiles, 0, "", true) require.ErrorIs(t, err, files.ErrContentCollision) }) @@ -256,7 +263,7 @@ func TestCollision(t *testing.T) { {Source: "../testdata/whatever2.conf", Destination: "/samedestination", Packager: "bar"}, } - _, err := files.PrepareForPackager(configuredFiles, "foo", true) + _, err := files.PrepareForPackager(configuredFiles, 0, "foo", true) require.NoError(t, err) }) @@ -266,7 +273,7 @@ func TestCollision(t *testing.T) { {Source: "../testdata/whatever2.conf", Destination: "/samedestination", Packager: ""}, } - _, err := files.PrepareForPackager(configuredFiles, "foo", true) + _, err := files.PrepareForPackager(configuredFiles, 0, "foo", true) require.ErrorIs(t, err, files.ErrContentCollision) }) } @@ -297,7 +304,7 @@ func TestDisableGlobbing(t *testing.T) { content := testCase t.Run(strconv.Itoa(i), func(t *testing.T) { - result, err := files.PrepareForPackager(files.Contents{&content}, "", disableGlobbing) + result, err := files.PrepareForPackager(files.Contents{&content}, 0, "", disableGlobbing) if err != nil { t.Fatalf("expand content globs: %v", err) } @@ -341,7 +348,7 @@ func TestGlobbingWhenFilesHaveBrackets(t *testing.T) { Source: "./testdata/\\{test\\}/", Destination: ".", }, - }, "", false) + }, 0, "", false) if err != nil { t.Fatalf("expand content globs: %v", err) } @@ -382,7 +389,7 @@ func TestGlobbingFilesWithDifferentSizesWithFileInfo(t *testing.T) { Mode: 0o777, }, }, - }, "", false) + }, 0, "", false) if err != nil { t.Fatalf("expand content globs: %v", err) } @@ -404,7 +411,7 @@ func TestDestEndsWithSlash(t *testing.T) { Source: "./testdata/globtest/a.txt", Destination: "./foo/", }, - }, "", false) + }, 0, "", false) result = withoutImplicitDirs(result) require.NoError(t, err) require.Len(t, result, 1) @@ -421,7 +428,7 @@ contents: `)) dec.KnownFields(true) require.NoError(t, dec.Decode(&config)) - _, err := files.PrepareForPackager(config.Contents, "", false) + _, err := files.PrepareForPackager(config.Contents, 0, "", false) require.EqualError(t, err, "invalid content type: filr") } @@ -452,7 +459,7 @@ contents: `)) dec.KnownFields(true) require.NoError(t, dec.Decode(&config)) - _, err := files.PrepareForPackager(config.Contents, "", false) + _, err := files.PrepareForPackager(config.Contents, 0, "", false) require.NoError(t, err) } @@ -462,7 +469,7 @@ func TestImplicitDirectories(t *testing.T) { Source: "./testdata/globtest/a.txt", Destination: "./foo/bar/baz", }, - }, "", false) + }, 0, "", false) require.NoError(t, err) expected := files.Contents{ @@ -535,7 +542,7 @@ func TestRelevantFiles(t *testing.T) { } t.Run("deb", func(t *testing.T) { - results, err := files.PrepareForPackager(contents, "deb", false) + results, err := files.PrepareForPackager(contents, 0, "deb", false) require.NoError(t, err) require.Equal(t, files.Contents{ { @@ -558,7 +565,7 @@ func TestRelevantFiles(t *testing.T) { }) t.Run("rpm", func(t *testing.T) { - results, err := files.PrepareForPackager(contents, "rpm", false) + results, err := files.PrepareForPackager(contents, 0, "rpm", false) require.NoError(t, err) require.Equal(t, files.Contents{ { @@ -601,7 +608,7 @@ func TestRelevantFiles(t *testing.T) { }) t.Run("apk", func(t *testing.T) { - results, err := files.PrepareForPackager(contents, "apk", false) + results, err := files.PrepareForPackager(contents, 0, "apk", false) require.NoError(t, err) require.Equal(t, files.Contents{ { @@ -620,7 +627,7 @@ func TestTree(t *testing.T) { Destination: "/base", Type: files.TypeTree, }, - }, "", false) + }, 0, "", false) require.NoError(t, err) require.Equal(t, files.Contents{ diff --git a/files/testdata/deep-paths/nested1/nested2/a.txt b/files/testdata/deep-paths/nested1/nested2/a.txt old mode 100644 new mode 100755 diff --git a/nfpm.go b/nfpm.go index 702b7f31..a14c6b59 100644 --- a/nfpm.go +++ b/nfpm.go @@ -304,6 +304,7 @@ type Overridables struct { Suggests []string `yaml:"suggests,omitempty" json:"suggests,omitempty" jsonschema:"title=suggests directive,example=nfpm"` Conflicts []string `yaml:"conflicts,omitempty" json:"conflicts,omitempty" jsonschema:"title=conflicts directive,example=nfpm"` Contents files.Contents `yaml:"contents,omitempty" json:"contents,omitempty" jsonschema:"title=files to add to the package"` + Umask os.FileMode `yaml:"umask,omitempty" json:"umask,omitempty" jsonschema:"title=umask for file contents,example=002"` Scripts Scripts `yaml:"scripts,omitempty" json:"scripts,omitempty" jsonschema:"title=scripts to execute"` RPM RPM `yaml:"rpm,omitempty" json:"rpm,omitempty" jsonschema:"title=rpm-specific settings"` Deb Deb `yaml:"deb,omitempty" json:"deb,omitempty" jsonschema:"title=deb-specific settings"` @@ -441,7 +442,7 @@ func PrepareForPackager(info *Info, packager string) (err error) { return ErrFieldEmpty{"version"} } - info.Contents, err = files.PrepareForPackager(info.Contents, packager, info.DisableGlobbing) + info.Contents, err = files.PrepareForPackager(info.Contents, info.Umask, packager, info.DisableGlobbing) return err } @@ -460,7 +461,7 @@ func Validate(info *Info) (err error) { } for packager := range packagers { - _, err := files.PrepareForPackager(info.Contents, packager, info.DisableGlobbing) + _, err := files.PrepareForPackager(info.Contents, info.Umask, packager, info.DisableGlobbing) if err != nil { return err } diff --git a/www/docs/configuration.md b/www/docs/configuration.md index 9fb357a7..b298c559 100644 --- a/www/docs/configuration.md +++ b/www/docs/configuration.md @@ -195,6 +195,15 @@ contents: dst: /etc/bar.conf type: config|noreplace +# Umask to be used on files without explicit mode set. +# +# By default, nFPM will use the mode of the original file in the file system. +# This may lead to issues if these files are checkout out in Git, for example, +# as it won't keep all the permissions on fresh checkouts. +# +# 0o002 would remove the world-writable permission, for example. +umask: 0o002 + # These files are not actually present in the package, but the file names # are added to the package header. From the RPM directives documentation: #