mirror of
https://github.com/fleetdm/fleet
synced 2026-04-21 21:47:20 +00:00
4fcbb57d23
## Summary - `checkPermFile` in `pkg/secure/secure.go` now self-heals incorrect file permissions via `os.Chmod` instead of returning a fatal error - Fixes orbit crash-looping indefinitely when `/opt/orbit/updates-metadata.json` has mode 755 instead of the expected 600 ## Problem Orbit refuses to start when `updates-metadata.json` has wrong permissions (e.g. 755 instead of 600), entering an infinite restart loop (`systemd` restart counter observed at 3447+). The manual workaround is `chmod 600 /opt/orbit/updates-metadata.json`, but the root cause — an external process changing file permissions — is intermittent and hard to track. The `checkPermFile` function in `pkg/secure/secure.go` was designed as a security check, but its behavior of fatally erroring on any permission mismatch causes a denial-of-service on the legitimate user. For comparison, `checkPermPath` (the directory equivalent) already tolerates permissions that are less permissive than expected. ## Fix When `checkPermFile` detects a permission mismatch, it now attempts `os.Chmod` to correct the permissions before proceeding. It only returns an error if the chmod itself fails (e.g. insufficient privileges). This preserves the security intent — files end up with correct permissions — while making orbit resilient to external permission drift. ## Test plan - [ ] `go test ./pkg/secure/ -v -run TestOpenFile` — verifies self-healing behavior - [ ] `go test ./pkg/secure/ -v -run TestMkdirAll` — unchanged, verifies directory checks still work - [ ] Manual: create `/opt/orbit/updates-metadata.json` with mode 755, start orbit, confirm it self-heals and starts normally --------- Co-authored-by: Bash Bandicoot <bash-bandicoot@users.noreply.github.com>
59 lines
1.7 KiB
Go
59 lines
1.7 KiB
Go
package secure
|
|
|
|
import (
|
|
"fmt"
|
|
"os"
|
|
"path/filepath"
|
|
"testing"
|
|
|
|
"github.com/stretchr/testify/require"
|
|
)
|
|
|
|
func TestMkdirAll(t *testing.T) {
|
|
tmpDir := t.TempDir()
|
|
basePath := filepath.Join(tmpDir, "test")
|
|
require.NoError(t, os.MkdirAll(basePath, 0700))
|
|
err := MkdirAll(basePath, 0677)
|
|
require.Error(t, err)
|
|
expectedErr := fmt.Sprintf(
|
|
"Path %s already exists with mode 20000000700 instead of the expected %o", basePath, 0677^os.ModeDir)
|
|
require.Equal(t, expectedErr, err.Error())
|
|
|
|
err = MkdirAll(filepath.Join(basePath, "test2", "test3"), 0677)
|
|
require.Error(t, err)
|
|
require.Equal(t, expectedErr, err.Error())
|
|
|
|
err = MkdirAll(filepath.Join(basePath, "test2", "test3"), 0700)
|
|
require.NoError(t, err)
|
|
}
|
|
|
|
func TestOpenFile(t *testing.T) {
|
|
tmpDir := t.TempDir()
|
|
basePath := filepath.Join(tmpDir, "test")
|
|
require.NoError(t, os.MkdirAll(basePath, 0755))
|
|
|
|
filePath := filepath.Join(basePath, "file1")
|
|
_, err := OpenFile(filePath, os.O_CREATE|os.O_WRONLY, 0677)
|
|
require.Error(t, err)
|
|
expectedErr := fmt.Sprintf(
|
|
"Path %s already exists with mode 20000000755 instead of the expected %o", basePath, 0677^os.ModeDir)
|
|
require.Equal(t, expectedErr, err.Error())
|
|
|
|
fd, err := OpenFile(filePath, os.O_CREATE|os.O_WRONLY, 0755)
|
|
require.NoError(t, err)
|
|
require.NotNil(t, fd)
|
|
require.NoError(t, fd.Close())
|
|
|
|
// Opening with a different perm should self-heal via chmod rather than error.
|
|
fd, err = OpenFile(filePath, os.O_CREATE|os.O_WRONLY, 0600)
|
|
require.NoError(t, err)
|
|
fd.Close()
|
|
info, err := os.Stat(filePath)
|
|
require.NoError(t, err)
|
|
require.Equal(t, os.FileMode(0600), info.Mode())
|
|
|
|
// Re-open with the now-correct mode still works.
|
|
fd, err = OpenFile(filePath, os.O_CREATE|os.O_WRONLY, 0600)
|
|
require.NoError(t, err)
|
|
fd.Close()
|
|
}
|