Skip to content

Commit

Permalink
Prevent symlinks from generating 'set permissions' logs. (#103)
Browse files Browse the repository at this point in the history
  • Loading branch information
mjkw31 authored Nov 5, 2024
1 parent 4a71c14 commit dbbfe91
Show file tree
Hide file tree
Showing 3 changed files with 49 additions and 27 deletions.
20 changes: 18 additions & 2 deletions basedirs/basedirs_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -836,8 +836,7 @@ func TestBaseDirs(t *testing.T) {
mainTable[0].DateNoSpace = time.Time{}
mainTable[0].DateNoFiles = time.Time{}

So(len(mainTable), ShouldEqual, 7)
So(mainTable, ShouldResemble, []*Usage{
mainTableExpectation := []*Usage{
{
Name: "group1", GID: 1, UIDs: []uint32{101}, Owner: "Alan", BaseDir: projectA,
UsageSize: twoGig + halfGig*2, QuotaSize: fiveGig,
Expand Down Expand Up @@ -873,8 +872,25 @@ func TestBaseDirs(t *testing.T) {
UsageSize: 60, QuotaSize: 500, UsageInodes: 1,
QuotaInodes: 50, Mtime: expectedMtime,
},
}

sort.Slice(mainTable, func(i, j int) bool {
return bytes.Compare(
idToByteSlice(mainTable[i].GID),
idToByteSlice(mainTable[j].GID),
) != -1
})

sort.Slice(mainTableExpectation, func(i, j int) bool {
return bytes.Compare(
idToByteSlice(mainTableExpectation[i].GID),
idToByteSlice(mainTableExpectation[j].GID),
) != -1
})

So(len(mainTable), ShouldEqual, 7)
So(mainTable, ShouldResemble, mainTableExpectation)

history, err := bdr.History(1, projectA)
fixHistoryTimes(history)

Expand Down
16 changes: 5 additions & 11 deletions ch/ch.go
Original file line number Diff line number Diff line change
Expand Up @@ -191,6 +191,10 @@ func groupName(gid int) (string, error) {
}

func (c *Ch) chmod(rule *Rule, path string, info fs.FileInfo) error {
if info.Mode()&fs.ModeSymlink == fs.ModeSymlink {
return nil
}

currentPerms := info.Mode()

var desiredPerms fs.FileMode
Expand All @@ -205,21 +209,11 @@ func (c *Ch) chmod(rule *Rule, path string, info fs.FileInfo) error {
return nil
}

if err := chmod(info, path, desiredPerms); err != nil {
if err := os.Chmod(path, desiredPerms); err != nil {
return err
}

c.logger.Info("set permissions", "path", path, "old", currentPerms, "new", desiredPerms)

return nil
}

// chmod is like os.Chmod, but checks the given info to do nothing if this is a
// symlink.
func chmod(info fs.FileInfo, path string, mode fs.FileMode) error {
if info.Mode()&os.ModeSymlink == os.ModeSymlink {
return nil
}

return os.Chmod(path, mode)
}
40 changes: 26 additions & 14 deletions ch/ch_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -179,6 +179,32 @@ func TestCh(t *testing.T) {
So(err.Error(), ShouldContainSubstring, "2 errors occurred")
})

Convey("Do on a symlink doesn't try to change permissions", func() {
mySymlink := filepath.Join(dir, "mySymlink")

err := os.Symlink("non-existent-target", mySymlink)
So(err, ShouldBeNil)

info, err := os.Lstat(mySymlink)
So(err, ShouldBeNil)

err = ch.Do(mySymlink, info)
So(err, ShouldBeNil)
So(buff.String(), ShouldNotContainSubstring, `lvl=info msg="set permissions" path=`)

createTestFile(t, filepath.Join(dir, "a"), otherGID, 0755)

err = os.Remove(mySymlink)
So(err, ShouldBeNil)

err = os.Symlink("a", mySymlink)
So(err, ShouldBeNil)

err = ch.Do(mySymlink, info)
So(err, ShouldBeNil)
So(buff.String(), ShouldNotContainSubstring, `lvl=info msg="set permissions" path=`)
})

Convey("chownGroup applies to symlinks themselves, not their targets", func() {
path := filepath.Join(dir, "e")
slink := filepath.Join(dir, "f")
Expand All @@ -198,20 +224,6 @@ func TestCh(t *testing.T) {

_, gid := getIDsFromFileInfo(info)
So(gid, ShouldEqual, otherGID)

Convey("chmod ignores symlinks but works on real files", func() {
err = chmod(info, slink, 0670)
So(err, ShouldBeNil)

info, err = os.Lstat(path)
So(info.Mode().Perm(), ShouldEqual, fs.FileMode(0660))

err = chmod(info, path, 0670)
So(err, ShouldBeNil)

info, err = os.Lstat(path)
So(info.Mode().Perm(), ShouldEqual, fs.FileMode(0670))
})
})
})
})
Expand Down

0 comments on commit dbbfe91

Please sign in to comment.