From 05a5d5e350c57b2a047a2d5c1db5d1a8c90eef82 Mon Sep 17 00:00:00 2001
From: Vladimir Stoilov <vladimir@safing.io>
Date: Fri, 6 Dec 2024 13:28:24 +0200
Subject: [PATCH] [service] Fix unit tests

---
 base/database/storage/fstree/fstree.go |  1 -
 base/updater/registry.go               |  5 -----
 base/updater/registry_test.go          |  2 +-
 base/utils/fs.go                       | 11 ++++++-----
 base/utils/permissions_windows.go      |  3 +++
 base/utils/structure.go                |  6 +++---
 base/utils/structure_test.go           | 26 +++++++++++++-------------
 cmds/notifier/main.go                  |  4 ++--
 cmds/updatemgr/main.go                 |  2 +-
 cmds/updatemgr/release.go              |  3 ++-
 service/updates/upgrader.go            |  2 +-
 11 files changed, 32 insertions(+), 33 deletions(-)

diff --git a/base/database/storage/fstree/fstree.go b/base/database/storage/fstree/fstree.go
index 0b4f175c..4b04f41d 100644
--- a/base/database/storage/fstree/fstree.go
+++ b/base/database/storage/fstree/fstree.go
@@ -15,7 +15,6 @@ import (
 	"strings"
 	"time"
 
-	"github.com/hectane/go-acl"
 	"github.com/safing/portmaster/base/database/iterator"
 	"github.com/safing/portmaster/base/database/query"
 	"github.com/safing/portmaster/base/database/record"
diff --git a/base/updater/registry.go b/base/updater/registry.go
index 4450fe98..a2bf5fcd 100644
--- a/base/updater/registry.go
+++ b/base/updater/registry.go
@@ -5,7 +5,6 @@ import (
 	"fmt"
 	"os"
 	"path/filepath"
-	"runtime"
 	"strings"
 	"sync"
 
@@ -13,10 +12,6 @@ import (
 	"github.com/safing/portmaster/base/utils"
 )
 
-const (
-	onWindows = runtime.GOOS == "windows"
-)
-
 // ResourceRegistry is a registry for managing update resources.
 type ResourceRegistry struct {
 	sync.RWMutex
diff --git a/base/updater/registry_test.go b/base/updater/registry_test.go
index a8978f68..1b409b25 100644
--- a/base/updater/registry_test.go
+++ b/base/updater/registry_test.go
@@ -20,7 +20,7 @@ func TestMain(m *testing.M) {
 		DevMode:        true,
 		Online:         true,
 	}
-	err = registry.Initialize(utils.NewDirStructure(tmpDir, 0o0777))
+	err = registry.Initialize(utils.NewDirStructure(tmpDir, utils.PublicWritePermission))
 	if err != nil {
 		panic(err)
 	}
diff --git a/base/utils/fs.go b/base/utils/fs.go
index 6ede989d..5eb456b1 100644
--- a/base/utils/fs.go
+++ b/base/utils/fs.go
@@ -21,7 +21,7 @@ func EnsureDirectory(path string, perm FSPermission) error {
 			// directory exists, check permissions
 			if isWindows {
 				// Ignore windows permission error. For none admin users it will always fail.
-				SetDirPermission(path, perm)
+				_ = SetDirPermission(path, perm)
 				return nil
 			} else if f.Mode().Perm() != perm.AsUnixDirExecPermission() {
 				return SetDirPermission(path, perm)
@@ -39,10 +39,11 @@ func EnsureDirectory(path string, perm FSPermission) error {
 		if err != nil {
 			return fmt.Errorf("could not create dir %s: %w", path, err)
 		}
-		// Set windows permissions. Linux permission where already set with creation.
-		if isWindows {
-			// Ignore windows permission error. For none admin users it will always fail.
-			SetDirPermission(path, perm)
+		// Set permissions.
+		err = SetDirPermission(path, perm)
+		// Ignore windows permission error. For none admin users it will always fail.
+		if !isWindows {
+			return err
 		}
 		return nil
 	}
diff --git a/base/utils/permissions_windows.go b/base/utils/permissions_windows.go
index ac48c21a..5f36ebb9 100644
--- a/base/utils/permissions_windows.go
+++ b/base/utils/permissions_windows.go
@@ -32,4 +32,7 @@ func setWindowsFilePermissions(path string, perm FSPermission) {
 		acl.Apply(path, true, false, acl.GrantName(windows.GENERIC_ALL|windows.STANDARD_RIGHTS_ALL, "Administrators"))
 		acl.Apply(path, false, false, acl.GrantName(windows.GENERIC_ALL|windows.STANDARD_RIGHTS_ALL, "Users"))
 	}
+
+	// For completeness
+	acl.Apply(path, false, false, acl.GrantName(windows.GENERIC_ALL|windows.STANDARD_RIGHTS_ALL, "SYSTEM"))
 }
diff --git a/base/utils/structure.go b/base/utils/structure.go
index e33a920f..8e1f0786 100644
--- a/base/utils/structure.go
+++ b/base/utils/structure.go
@@ -16,7 +16,7 @@ const (
 	PublicWritePermission
 )
 
-// AsUnixDirPermission return the corresponding unix permission for a directory or executable.
+// AsUnixDirExecPermission return the corresponding unix permission for a directory or executable.
 func (perm FSPermission) AsUnixDirExecPermission() fs.FileMode {
 	switch perm {
 	case AdminOnlyPermission:
@@ -30,13 +30,13 @@ func (perm FSPermission) AsUnixDirExecPermission() fs.FileMode {
 	return 0
 }
 
-// AsUnixDirPermission return the corresponding unix permission for a regular file.
+// AsUnixFilePermission return the corresponding unix permission for a regular file.
 func (perm FSPermission) AsUnixFilePermission() fs.FileMode {
 	switch perm {
 	case AdminOnlyPermission:
 		return 0o600
 	case PublicReadPermission:
-		return 0o655
+		return 0o644
 	case PublicWritePermission:
 		return 0o666
 	}
diff --git a/base/utils/structure_test.go b/base/utils/structure_test.go
index 2acfebd2..d3debcf7 100644
--- a/base/utils/structure_test.go
+++ b/base/utils/structure_test.go
@@ -13,13 +13,13 @@ func ExampleDirStructure() {
 	// output:
 	// / [755]
 	// /repo [777]
-	// /repo/b [707]
-	// /repo/b/c [750]
-	// /repo/b/d [707]
-	// /repo/b/d/e [707]
-	// /repo/b/d/f [707]
-	// /repo/b/d/f/g [707]
-	// /repo/b/d/f/g/h [707]
+	// /repo/b [755]
+	// /repo/b/c [777]
+	// /repo/b/d [755]
+	// /repo/b/d/e [755]
+	// /repo/b/d/f [755]
+	// /repo/b/d/f/g [755]
+	// /repo/b/d/f/g/h [755]
 	// /secret [700]
 
 	basePath, err := os.MkdirTemp("", "")
@@ -28,12 +28,12 @@ func ExampleDirStructure() {
 		return
 	}
 
-	ds := NewDirStructure(basePath, 0o0755)
-	secret := ds.ChildDir("secret", 0o0700)
-	repo := ds.ChildDir("repo", 0o0777)
-	_ = repo.ChildDir("a", 0o0700)
-	b := repo.ChildDir("b", 0o0707)
-	c := b.ChildDir("c", 0o0750)
+	ds := NewDirStructure(basePath, PublicReadPermission)
+	secret := ds.ChildDir("secret", AdminOnlyPermission)
+	repo := ds.ChildDir("repo", PublicWritePermission)
+	_ = repo.ChildDir("a", AdminOnlyPermission)
+	b := repo.ChildDir("b", PublicReadPermission)
+	c := b.ChildDir("c", PublicWritePermission)
 
 	err = ds.Ensure()
 	if err != nil {
diff --git a/cmds/notifier/main.go b/cmds/notifier/main.go
index e40487bb..a11bea2a 100644
--- a/cmds/notifier/main.go
+++ b/cmds/notifier/main.go
@@ -225,14 +225,14 @@ func configureRegistry(mustLoadIndex bool) error {
 	// Remove left over quotes.
 	dataDir = strings.Trim(dataDir, `\"`)
 	// Initialize data root.
-	err := dataroot.Initialize(dataDir, 0o0755)
+	err := dataroot.Initialize(dataDir, utils.PublicReadPermission)
 	if err != nil {
 		return fmt.Errorf("failed to initialize data root: %w", err)
 	}
 	dataRoot = dataroot.Root()
 
 	// Initialize registry.
-	err = registry.Initialize(dataRoot.ChildDir("updates", 0o0755))
+	err = registry.Initialize(dataRoot.ChildDir("updates", utils.PublicReadPermission))
 	if err != nil {
 		return err
 	}
diff --git a/cmds/updatemgr/main.go b/cmds/updatemgr/main.go
index acd9a0d4..4a999f49 100644
--- a/cmds/updatemgr/main.go
+++ b/cmds/updatemgr/main.go
@@ -31,7 +31,7 @@ var rootCmd = &cobra.Command{
 		}
 
 		registry = &updater.ResourceRegistry{}
-		err = registry.Initialize(utils.NewDirStructure(absDistPath, 0o0755))
+		err = registry.Initialize(utils.NewDirStructure(absDistPath, utils.PublicReadPermission))
 		if err != nil {
 			return err
 		}
diff --git a/cmds/updatemgr/release.go b/cmds/updatemgr/release.go
index 0f5d596e..e6642a63 100644
--- a/cmds/updatemgr/release.go
+++ b/cmds/updatemgr/release.go
@@ -11,6 +11,7 @@ import (
 	"github.com/spf13/cobra"
 
 	"github.com/safing/portmaster/base/updater"
+	"github.com/safing/portmaster/base/utils"
 )
 
 var (
@@ -63,7 +64,7 @@ func release(cmd *cobra.Command, args []string) error {
 			fmt.Println("aborted...")
 			return nil
 		}
-		symlinksDir := registry.StorageDir().ChildDir("latest", 0o755)
+		symlinksDir := registry.StorageDir().ChildDir("latest", utils.PublicReadPermission)
 		err = registry.CreateSymlinks(symlinksDir)
 		if err != nil {
 			return err
diff --git a/service/updates/upgrader.go b/service/updates/upgrader.go
index f1d3c297..b07d649e 100644
--- a/service/updates/upgrader.go
+++ b/service/updates/upgrader.go
@@ -351,7 +351,7 @@ func upgradeBinary(fileToUpgrade string, file *updater.File) error {
 
 	// check permissions
 	if onWindows {
-		utils.SetExecPermission(fileToUpgrade, utils.PublicReadPermission)
+		_ = utils.SetExecPermission(fileToUpgrade, utils.PublicReadPermission)
 	} else {
 		perm := utils.PublicReadPermission
 		info, err := os.Stat(fileToUpgrade)