Bug: Apply SELinux context after binary replacement (#1678)

- Move SELinux context handling to internal/ghupdate for reuse
- Make chcon a true fallback (only runs if semanage/restorecon unavailable)
- Handle existing semanage rules with -m (modify) after -a (add) fails
- Apply SELinux handling to both agent and hub updates
- Add tests with proper skip behavior for SELinux systems

---------

Co-authored-by: henrygd <hank@henrygd.me>

agent/update.go

@@ -1,12 +1,10 @@
 package agent
 
 import (
-	"fmt"
 	"log"
 	"os"
 	"os/exec"
 	"runtime"
-	"strings"
 
 	"github.com/henrygd/beszel/internal/ghupdate"
 )
@@ -108,12 +106,12 @@ func Update(useMirror bool) error {
 		}
 	}
 
-	// 6) Fix SELinux context if necessary
+	// Fix SELinux context if necessary
-	if err := handleSELinuxContext(exePath); err != nil {
+	if err := ghupdate.HandleSELinuxContext(exePath); err != nil {
 		ghupdate.ColorPrintf(ghupdate.ColorYellow, "Warning: SELinux context handling: %v", err)
 	}
 
-	// 7) Restart service if running under a recognised init system
+	// Restart service if running under a recognised init system
 	if r := detectRestarter(); r != nil {
 		if err := r.Restart(); err != nil {
 			ghupdate.ColorPrintf(ghupdate.ColorYellow, "Warning: failed to restart service: %v", err)
@@ -128,41 +126,3 @@ func Update(useMirror bool) error {
 	return nil
 }
 
-// handleSELinuxContext restores or applies the correct SELinux label to the binary.
-func handleSELinuxContext(path string) error {
-	out, err := exec.Command("getenforce").Output()
-	if err != nil {
-		// SELinux not enabled or getenforce not available
-		return nil
-	}
-	state := strings.TrimSpace(string(out))
-	if state == "Disabled" {
-		return nil
-	}
-
-	ghupdate.ColorPrint(ghupdate.ColorYellow, "SELinux is enabled; applying context…")
-	var errs []string
-
-	// Try persistent context via semanage+restorecon
-	if semanagePath, err := exec.LookPath("semanage"); err == nil {
-		if err := exec.Command(semanagePath, "fcontext", "-a", "-t", "bin_t", path).Run(); err != nil {
-			errs = append(errs, "semanage fcontext failed: "+err.Error())
-		} else if restoreconPath, err := exec.LookPath("restorecon"); err == nil {
-			if err := exec.Command(restoreconPath, "-v", path).Run(); err != nil {
-				errs = append(errs, "restorecon failed: "+err.Error())
-			}
-		}
-	}
-
-	// Fallback to temporary context via chcon
-	if chconPath, err := exec.LookPath("chcon"); err == nil {
-		if err := exec.Command(chconPath, "-t", "bin_t", path).Run(); err != nil {
-			errs = append(errs, "chcon failed: "+err.Error())
-		}
-	}
-
-	if len(errs) > 0 {
-		return fmt.Errorf("SELinux context errors: %s", strings.Join(errs, "; "))
-	}
-	return nil
-}

internal/ghupdate/selinux.go

@@ -0,0 +1,66 @@
+package ghupdate
+
+import (
+	"fmt"
+	"os/exec"
+	"strings"
+)
+
+// HandleSELinuxContext restores or applies the correct SELinux label to the binary.
+func HandleSELinuxContext(path string) error {
+	out, err := exec.Command("getenforce").Output()
+	if err != nil {
+		// SELinux not enabled or getenforce not available
+		return nil
+	}
+	state := strings.TrimSpace(string(out))
+	if state == "Disabled" {
+		return nil
+	}
+
+	ColorPrint(ColorYellow, "SELinux is enabled; applying context…")
+
+	// Try persistent context via semanage+restorecon
+	if success := trySemanageRestorecon(path); success {
+		return nil
+	}
+
+	// Fallback to temporary context via chcon
+	if chconPath, err := exec.LookPath("chcon"); err == nil {
+		if err := exec.Command(chconPath, "-t", "bin_t", path).Run(); err != nil {
+			return fmt.Errorf("chcon failed: %w", err)
+		}
+		return nil
+	}
+
+	return fmt.Errorf("no SELinux tools available (semanage/restorecon or chcon)")
+}
+
+// trySemanageRestorecon attempts to set persistent SELinux context using semanage and restorecon.
+// Returns true if successful, false otherwise.
+func trySemanageRestorecon(path string) bool {
+	semanagePath, err := exec.LookPath("semanage")
+	if err != nil {
+		return false
+	}
+
+	restoreconPath, err := exec.LookPath("restorecon")
+	if err != nil {
+		return false
+	}
+
+	// Try to add the fcontext rule; if it already exists, try to modify it
+	if err := exec.Command(semanagePath, "fcontext", "-a", "-t", "bin_t", path).Run(); err != nil {
+		// Rule may already exist, try modify instead
+		if err := exec.Command(semanagePath, "fcontext", "-m", "-t", "bin_t", path).Run(); err != nil {
+			return false
+		}
+	}
+
+	// Apply the context with restorecon
+	if err := exec.Command(restoreconPath, "-v", path).Run(); err != nil {
+		return false
+	}
+
+	return true
+}

internal/ghupdate/selinux_test.go

@@ -0,0 +1,53 @@
+package ghupdate
+
+import (
+	"os"
+	"os/exec"
+	"path/filepath"
+	"testing"
+)
+
+func TestHandleSELinuxContext_NoSELinux(t *testing.T) {
+	// Skip on SELinux systems - this test is for non-SELinux behavior
+	if _, err := exec.LookPath("getenforce"); err == nil {
+		t.Skip("skipping on SELinux-enabled system")
+	}
+
+	// On systems without SELinux, getenforce will fail and the function
+	// should return nil without error
+	tempFile := filepath.Join(t.TempDir(), "test-binary")
+	if err := os.WriteFile(tempFile, []byte("test"), 0755); err != nil {
+		t.Fatalf("failed to create temp file: %v", err)
+	}
+
+	err := HandleSELinuxContext(tempFile)
+	if err != nil {
+		t.Errorf("HandleSELinuxContext() on non-SELinux system returned error: %v", err)
+	}
+}
+
+func TestHandleSELinuxContext_InvalidPath(t *testing.T) {
+	// Skip on SELinux systems - this test is for non-SELinux behavior
+	if _, err := exec.LookPath("getenforce"); err == nil {
+		t.Skip("skipping on SELinux-enabled system")
+	}
+
+	// On non-SELinux systems, getenforce fails early so even invalid paths succeed
+	err := HandleSELinuxContext("/nonexistent/path/binary")
+	if err != nil {
+		t.Errorf("HandleSELinuxContext() with invalid path on non-SELinux system returned error: %v", err)
+	}
+}
+
+func TestTrySemanageRestorecon_NoTools(t *testing.T) {
+	// Skip if semanage is available (we don't want to modify system SELinux policy)
+	if _, err := exec.LookPath("semanage"); err == nil {
+		t.Skip("skipping on system with semanage available")
+	}
+
+	// Should return false when semanage is not available
+	result := trySemanageRestorecon("/some/path")
+	if result {
+		t.Error("trySemanageRestorecon() returned true when semanage is not available")
+	}
+}

internal/hub/update.go

@@ -45,6 +45,11 @@ func Update(cmd *cobra.Command, _ []string) {
 		fmt.Printf("Warning: failed to set executable permissions: %v\n", err)
 	}
 
+	// Fix SELinux context if necessary
+	if err := ghupdate.HandleSELinuxContext(exePath); err != nil {
+		ghupdate.ColorPrintf(ghupdate.ColorYellow, "Warning: SELinux context handling: %v", err)
+	}
+
 	// Try to restart the service if it's running
 	restartService()
 }