Skip to content

Commit

Permalink
fix: explicitly set permissions for socket_vmnet dependencies (#363)
Browse files Browse the repository at this point in the history
Issue #, if available: #57

*Description of changes:*

Sets permissions explicitly for socket_vmnet dependencies, which will
override a user's `umask` to ensure that Finch can use the files as
needed.

*Testing done:*

```
$ umask 077
$ make
$ sudo rm -rf /opt/finch/bin/socket_vmnet*
$ sudo rm -rf /etc/sudoers.d/finch-lima
$ ./_output/bin/finch vm init
```

unit testing


- [x] I've reviewed the guidance in CONTRIBUTING.md


#### License Acceptance

By submitting this pull request, I confirm that my contribution is made
under the terms of the Apache 2.0 license.

Signed-off-by: Sam Berning <[email protected]>
  • Loading branch information
sam-berning authored Apr 17, 2023
1 parent 5e03a4d commit 0801b88
Show file tree
Hide file tree
Showing 4 changed files with 51 additions and 1 deletion.
8 changes: 8 additions & 0 deletions pkg/dependency/vmnet/binaries.go
Original file line number Diff line number Diff line change
Expand Up @@ -122,6 +122,14 @@ func (bin *binaries) Install() error {
return fmt.Errorf("error changing owner of files in directory %s, err: %w", installationPathBinDir, err)
}

// in most cases this should not be needed, as the permissions should be copied from the build artifacts,
// but the permissions can end up being wrong when Finch is built on a shell that has a restrictive umask.
chmodBinCmd := bin.cmdCreator.Create("sudo", "chmod", "755", bin.installationPathSocketVmnetExe())
_, err = chmodBinCmd.Output()
if err != nil {
return fmt.Errorf("error setting correct permissions for socket_vmnet binary, err: %w", err)
}

return nil
}

Expand Down
17 changes: 16 additions & 1 deletion pkg/dependency/vmnet/binaries_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -176,12 +176,13 @@ func TestBinaries_Install(t *testing.T) {
{
name: "happy path",
mockSvc: func(cmd *mocks.Command, creator *mocks.CommandCreator) {
cmd.EXPECT().Output().Times(4)
cmd.EXPECT().Output().Times(5)

creator.EXPECT().Create("sudo", "mkdir", "-p", "/opt/finch").Return(cmd)
creator.EXPECT().Create("sudo", "cp", "-rp", "mock_prefix/dependencies/lima-socket_vmnet/opt/finch", "/opt").Return(cmd)
creator.EXPECT().Create("sudo", "chown", "root:wheel", "/opt/finch").Return(cmd)
creator.EXPECT().Create("sudo", "chown", "-R", "root:wheel", "/opt/finch/bin").Return(cmd)
creator.EXPECT().Create("sudo", "chmod", "755", "/opt/finch/bin/socket_vmnet").Return(cmd)
},
want: nil,
},
Expand Down Expand Up @@ -230,6 +231,20 @@ func TestBinaries_Install(t *testing.T) {
},
want: fmt.Errorf("error changing owner of files in directory %s, err: %w", "/opt/finch/bin", errors.New("chown -R error")),
},
{
name: "sudo chmod of the binary throws an error",
mockSvc: func(cmd *mocks.Command, creator *mocks.CommandCreator) {
cmd.EXPECT().Output().Times(4)
cmd.EXPECT().Output().Return([]byte{}, errors.New("sudo chmod error"))

creator.EXPECT().Create("sudo", "mkdir", "-p", "/opt/finch").Return(cmd)
creator.EXPECT().Create("sudo", "cp", "-rp", "mock_prefix/dependencies/lima-socket_vmnet/opt/finch", "/opt").Return(cmd)
creator.EXPECT().Create("sudo", "chown", "root:wheel", "/opt/finch").Return(cmd)
creator.EXPECT().Create("sudo", "chown", "-R", "root:wheel", "/opt/finch/bin").Return(cmd)
creator.EXPECT().Create("sudo", "chmod", "755", "/opt/finch/bin/socket_vmnet").Return(cmd)
},
want: fmt.Errorf("error setting correct permissions for socket_vmnet binary, err: %w", errors.New("sudo chmod error")),
},
}

for _, tc := range testCases {
Expand Down
6 changes: 6 additions & 0 deletions pkg/dependency/vmnet/sudoers_file.go
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,12 @@ func (s *sudoersFile) Install() error {
return fmt.Errorf("failed to write to the sudoers file: %w", err)
}

// explicitly set permissions to ensure that the file is readable by the user
_, err = s.execCmdCreator.Create("sudo", "chmod", "644", s.path()).Output()
if err != nil {
return fmt.Errorf("failed to set correct permissions for sudoers file: %w", err)
}

return nil
}

Expand Down
21 changes: 21 additions & 0 deletions pkg/dependency/vmnet/sudoers_file_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -133,6 +133,8 @@ func TestSudoers_Install(t *testing.T) {
ec.EXPECT().Create("sudo", "tee", "/etc/sudoers.d/finch-lima").Return(cmd)
cmd.EXPECT().SetStdin(bytes.NewReader(mockSudoersOut))
cmd.EXPECT().Output().Return(mockSudoersOut, nil)
ec.EXPECT().Create("sudo", "chmod", "644", "/etc/sudoers.d/finch-lima").Return(cmd)
cmd.EXPECT().Output().Return([]byte{}, nil)
},
want: nil,
},
Expand Down Expand Up @@ -166,6 +168,25 @@ func TestSudoers_Install(t *testing.T) {
},
want: fmt.Errorf("failed to write to the sudoers file: %w", errors.New("sudo tee command error")),
},
{
name: "sudo chmod command throws err",
mockSvc: func(t *testing.T, cmd *mocks.Command, mFs afero.Fs, ec *mocks.CommandCreator, lc *mocks.LimaCmdCreator) {
sudoersData := []byte("test data")
mockSudoersOut := []byte("mock_sudoers_out")

err := afero.WriteFile(mFs, "/etc/sudoers.d/finch-lima", sudoersData, 0o666)
require.NoError(t, err)

lc.EXPECT().CreateWithoutStdio("sudoers").Return(cmd)
cmd.EXPECT().Output().Return(mockSudoersOut, nil)
ec.EXPECT().Create("sudo", "tee", "/etc/sudoers.d/finch-lima").Return(cmd)
cmd.EXPECT().SetStdin(bytes.NewReader(mockSudoersOut))
cmd.EXPECT().Output().Return(mockSudoersOut, nil)
ec.EXPECT().Create("sudo", "chmod", "644", "/etc/sudoers.d/finch-lima").Return(cmd)
cmd.EXPECT().Output().Return([]byte{}, errors.New("sudo chmod command error"))
},
want: fmt.Errorf("failed to set correct permissions for sudoers file: %w", errors.New("sudo chmod command error")),
},
}

for _, tc := range testCases {
Expand Down

0 comments on commit 0801b88

Please sign in to comment.