ensure Orbit identifier file can be read by the outside world (#7608)

As stated by the title, this fixes a bug in token rotation caused by a file having existing read-only permissions. This affects Fleet Desktop after is upgraded if an existing Orbit created a file that's not accessible by non-root users.

I have also included 77a59ac to fix an URL format

orbit/cmd/orbit/orbit.go

@@ -615,12 +615,17 @@ func (d *desktopRunner) execute() error {
 	if err != nil {
 		return fmt.Errorf("invalid fleet-url: %w", err)
 	}
+	deviceURL, err := url.Parse(d.fleetURL)
+	if err != nil {
+		return fmt.Errorf("invalid fleet-url: %w", err)
+	}
+	deviceURL.Path = path.Join(url.Path, "device", d.trw.GetCached())
 	opts := []execuser.Option{
 		execuser.WithEnv("FLEET_DESKTOP_FLEET_URL", url.String()),
 		execuser.WithEnv("FLEET_DESKTOP_DEVICE_IDENTIFIER_PATH", d.identifierPath),
 		// TODO(roperzh): this env var is keept only for backwards compatibility,
 		// we should remove it once we think is safe
-		execuser.WithEnv("FLEET_DESKTOP_DEVICE_URL", path.Join(url.String(), "device", d.trw.GetCached())),
+		execuser.WithEnv("FLEET_DESKTOP_DEVICE_URL", deviceURL.String()),
 	}
 	if d.fleetRootCA != "" {
 		opts = append(opts, execuser.WithEnv("FLEET_DESKTOP_FLEET_ROOT_CA", d.fleetRootCA))

orbit/pkg/constant/constant.go

@@ -5,8 +5,11 @@ const (
 	DefaultDirMode = 0o755
 	// DefaultFileMode is the default file mode to apply to created files.
 	DefaultFileMode = 0o600
+	// DefaultWorldReadableFileMode is the default file mode to apply to files
+	// that can be read by other processes.
+	DefaultWorldReadableFileMode = 0o644
 	// DefaultSystemdUnitMode is the required file mode to systemd unit files.
-	DefaultSystemdUnitMode = 0o644
+	DefaultSystemdUnitMode = DefaultWorldReadableFileMode
 	// DesktopAppExecName is the name of Fleet's Desktop executable.
 	//
 	// We use fleet-desktop as name to properly identify the process when listing

orbit/pkg/token/readwriter.go

@@ -26,7 +26,11 @@ func (rw *ReadWriter) LoadOrGenerate() error {
 	_, err := rw.Read()
 	switch {
 	case err == nil:
-		// OK
+		// ensure the file is readable by other processes, old versions of Orbit
+		// used to chmod this file with 0o600
+		if err := rw.setChmod(); err != nil {
+			return fmt.Errorf("loading token file, chmod %q: %w", rw.Path, err)
+		}
 	case errors.Is(err, os.ErrNotExist):
 		if err := rw.Rotate(); err != nil {
 			return fmt.Errorf("rotating token on generation: %w", err)
@@ -44,15 +48,20 @@ func (rw *ReadWriter) Rotate() error {
 		return fmt.Errorf("generate identifier: %w", err)
 	}
 
-	err = os.WriteFile(rw.Path, []byte(id.String()), constant.DefaultSystemdUnitMode)
+	err = os.WriteFile(rw.Path, []byte(id.String()), constant.DefaultWorldReadableFileMode)
 	if err != nil {
 		return fmt.Errorf("write identifier file %q: %w", rw.Path, err)
 	}
 
+	// ensure the file is readable by other processes, os.WriteFile does not
+	// modify permissions if the file already exists
+	if err := rw.setChmod(); err != nil {
+		return fmt.Errorf("write identifier file, chmod %q: %w", rw.Path, err)
+	}
+
 	// ensure the `mtime` is updated, we have seen tests fail in some versions of
 	// Ubuntu because this value is not update when the file is written
-	err = os.Chtimes(rw.Path, time.Now(), time.Now())
-	if err != nil {
+	if err = os.Chtimes(rw.Path, time.Now(), time.Now()); err != nil {
 		return fmt.Errorf("set mtime of identifier file %q: %w", rw.Path, err)
 	}
 
@@ -63,3 +72,7 @@ func (rw *ReadWriter) Rotate() error {
 
 	return nil
 }
+
+func (rw *ReadWriter) setChmod() error {
+	return os.Chmod(rw.Path, constant.DefaultWorldReadableFileMode)
+}

orbit/pkg/token/readwriter_test.go

@@ -5,6 +5,7 @@ import (
 	"path/filepath"
 	"testing"
 
+	"github.com/fleetdm/fleet/v4/orbit/pkg/constant"
 	"github.com/stretchr/testify/require"
 )
 
@@ -19,6 +20,10 @@ func TestLoadOrGenerate(t *testing.T) {
 		token, err := rw.Read()
 		require.NoError(t, err)
 		require.NotEmpty(t, token)
+
+		stat, err := os.Stat(file)
+		require.NoError(t, err)
+		require.Equal(t, os.FileMode(constant.DefaultWorldReadableFileMode), stat.Mode())
 	})
 
 	t.Run("returns the file value if it exists", func(t *testing.T) {
@@ -28,11 +33,43 @@ func TestLoadOrGenerate(t *testing.T) {
 		require.NoError(t, err)
 		defer os.Remove(file.Name())
 
+		stat, err := file.Stat()
+		require.NoError(t, err)
+		oldMtime := stat.ModTime()
 		rw := NewReadWriter(file.Name())
 		rw.LoadOrGenerate()
 		token, err := rw.Read()
 		require.NoError(t, err)
 		require.Equal(t, "test", token)
+
+		stat, err = os.Stat(file.Name())
+		require.NoError(t, err)
+		require.Equal(t, os.FileMode(constant.DefaultWorldReadableFileMode), stat.Mode())
+		require.Equal(t, oldMtime, stat.ModTime())
+	})
+
+	t.Run("sets the file mode to DefaultWorldReadableFileMode if exists", func(t *testing.T) {
+		file, err := os.CreateTemp("", "identifier")
+		require.NoError(t, err)
+		_, err = file.WriteString("test")
+		require.NoError(t, err)
+		defer os.Remove(file.Name())
+
+		err = file.Chmod(constant.DefaultFileMode)
+		require.NoError(t, err)
+		stat, err := file.Stat()
+		require.NoError(t, err)
+		require.Equal(t, os.FileMode(constant.DefaultFileMode), stat.Mode())
+
+		rw := NewReadWriter(file.Name())
+		rw.LoadOrGenerate()
+		token, err := rw.Read()
+		require.NoError(t, err)
+		require.Equal(t, "test", token)
+
+		stat, err = file.Stat()
+		require.NoError(t, err)
+		require.Equal(t, os.FileMode(constant.DefaultWorldReadableFileMode), stat.Mode())
 	})
 
 	t.Run("errors for other reasons", func(t *testing.T) {
@@ -65,10 +102,16 @@ func TestRotate(t *testing.T) {
 	token, err = rw.Read()
 	require.NoError(t, err)
 	require.NotEmpty(t, token)
+	stat, err := file.Stat()
+	require.NoError(t, err)
+	require.Equal(t, os.FileMode(constant.DefaultWorldReadableFileMode), stat.Mode())
 
 	rw.Rotate()
 	newToken, err := rw.Read()
 	require.NoError(t, err)
 	require.NotEmpty(t, newToken)
 	require.NotEqual(t, token, newToken)
+	stat, err = file.Stat()
+	require.NoError(t, err)
+	require.Equal(t, os.FileMode(constant.DefaultWorldReadableFileMode), stat.Mode())
 }