Skip to content

Commit

Permalink
Merge pull request from GHSA-w7jw-q4fg-qc4c
Browse files Browse the repository at this point in the history 
* feat(security): adds the umask option

closes GHSA-w7jw-q4fg-qc4c

Signed-off-by: Carlos Alexandro Becker <[email protected]>

* fix: correct bitwise op

---------

Signed-off-by: Carlos Alexandro Becker <[email protected]>
  • Loading branch information
@caarlos0
caarlos0 authored May 24, 2023
1 parent 9ac3288 commit 8e517cc
Show file tree
Hide file tree
Showing 5 changed files with 60 additions and 39 deletions.
34 changes: 19 additions & 15 deletions files/files.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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()
}
Expand Down Expand Up @@ -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 {
Expand All @@ -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)]
Expand All @@ -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)
}
Expand All @@ -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)
}
Expand Down Expand Up @@ -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]
Expand Down Expand Up @@ -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
Expand All @@ -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 {
Expand Down Expand Up @@ -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:
Expand All @@ -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
})
Expand Down
51 changes: 29 additions & 22 deletions files/files_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ contents:
}
}

func TestDeepPathsWithGlob(t *testing.T) {
func TestDeepPathsWithGlobAndUmask(t *testing.T) {
var config testStruct
dec := yaml.NewDecoder(strings.NewReader(`---
contents:
Expand All @@ -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())
}
}
}
Expand All @@ -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

Expand Down Expand Up @@ -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)

Expand Down Expand Up @@ -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)

Expand Down Expand Up @@ -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)

Expand Down Expand Up @@ -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)
}()
}
Expand All @@ -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)
})

Expand All @@ -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)
})

Expand All @@ -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)
})
}
Expand Down Expand Up @@ -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)
}
Expand Down Expand Up @@ -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)
}
Expand Down Expand Up @@ -382,7 +389,7 @@ func TestGlobbingFilesWithDifferentSizesWithFileInfo(t *testing.T) {
Mode: 0o777,
},
},
}, "", false)
}, 0, "", false)
if err != nil {
t.Fatalf("expand content globs: %v", err)
}
Expand All @@ -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)
Expand All @@ -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")
}

Expand Down Expand Up @@ -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)
}

Expand All @@ -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{
Expand Down Expand Up @@ -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{
{
Expand All @@ -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{
{
Expand Down Expand Up @@ -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{
{
Expand All @@ -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{
Expand Down
Empty file modified files/testdata/deep-paths/nested1/nested2/a.txt
100644 → 100755
View file
Empty file.
5 changes: 3 additions & 2 deletions nfpm.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"`
Expand Down Expand Up @@ -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
}
Expand All @@ -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
}
Expand Down
9 changes: 9 additions & 0 deletions www/docs/configuration.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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:
#
Expand Down

0 comments on commit 8e517cc

Please sign in to comment.