From 51a0845adba702ac02437405988b24b3b2c9fb45 Mon Sep 17 00:00:00 2001
From: James O'Doherty <james@theodohertyfamily.com>
Date: Wed, 3 Jun 2026 23:45:45 -0400
Subject: fix: resolve resource leaks and improve namespace lifecycle
 management

- Fix DNS resolver leaks by creating temporary resolv.conf files within the profile's runtime directory and ensuring robust cleanup.
- Fix isolation block directory leaks by explicitly removing the block directory during namespace unpinning.
- Improve namespace lifecycle management:
    - Register processes before joining an active namespace to prevent race conditions in reference counting.
    - Update `IsLastProcess` and corresponding tests to reflect the unregister-then-check cleanup flow.
- Improve test reliability and correctness:
    - Convert `TestAppRun_ProfileDirInjection` to use separate binary execution, preventing process replacement and ensuring `t.TempDir()` cleanup.
    - Replace hardcoded test configuration paths with `t.TempDir()` in `mount_leak_test.go`.
    - Implement `SetEnvOverrides` helper for cleaner environment variable management in E2E tests.
    - Improve E2E lifecycle tests with better environment handling and output redirection.
---
 internal/namespace/lifecycle.go      |  2 +-
 internal/namespace/lifecycle_test.go | 27 +++++++++++++++++++++++++--
 internal/namespace/pinning.go        | 29 ++++++++++++++++++++++++++++-
 3 files changed, 54 insertions(+), 4 deletions(-)

(limited to 'internal/namespace')

diff --git a/internal/namespace/lifecycle.go b/internal/namespace/lifecycle.go
index 3bd1753..5f729d3 100644
--- a/internal/namespace/lifecycle.go
+++ b/internal/namespace/lifecycle.go
@@ -168,7 +168,7 @@ func IsLastProcess(pm *paths.PathManager, profile string) (bool, error) {
 		}
 	}
 
-	return activeCount <= 1, nil
+	return activeCount == 0, nil
 }
 
 // SetControllerPid records the current process as the owner of the namespace.
diff --git a/internal/namespace/lifecycle_test.go b/internal/namespace/lifecycle_test.go
index 1fb0a13..9962e14 100644
--- a/internal/namespace/lifecycle_test.go
+++ b/internal/namespace/lifecycle_test.go
@@ -87,17 +87,40 @@ func TestLifecycleReferenceCounting(t *testing.T) {
 		if err := RegisterProcess(pm, profile); err != nil {
 			t.Fatal(err)
 		}
+		
+		// Simulate the application flow: Unregister before checking if we are the last one
+		if err := UnregisterProcess(pm, profile); err != nil {
+			t.Fatal(err)
+		}
+
 		isLast, err = IsLastProcess(pm, profile)
 		if err != nil || !isLast {
-			t.Errorf("Expected IsLastProcess to be true for single process, got %v, err: %v", isLast, err)
+			t.Errorf("Expected IsLastProcess to be true after unregistering the only process, got %v, err: %v", isLast, err)
 		}
 
+		// Add a "stale" process to ensure it's pruned and doesn't count as active
 		if err := os.WriteFile(filepath.Join(pidsDir, "1234567"), []byte(""), 0644); err != nil {
 			t.Fatal(err)
 		}
+		
+		// Register a real process so that pruning has something to do
+		if err := RegisterProcess(pm, profile); err != nil {
+			t.Fatal(err)
+		}
+		
+		// Prune the stale one
+		if err := PruneStalePids(pm, profile); err != nil {
+			t.Fatal(err)
+		}
+
+		// Unregister the real one
+		if err := UnregisterProcess(pm, profile); err != nil {
+			t.Fatal(err)
+		}
+
 		isLast, err = IsLastProcess(pm, profile)
 		if err != nil || !isLast {
-			t.Errorf("Expected IsLastProcess to be true because 1234567 is dead, got %v, err: %v", isLast, err)
+			t.Errorf("Expected IsLastProcess to be true after pruning stale and unregistering current, got %v, err: %v", isLast, err)
 		}
 	})
 }
diff --git a/internal/namespace/pinning.go b/internal/namespace/pinning.go
index 07f15f8..9bf4fee 100644
--- a/internal/namespace/pinning.go
+++ b/internal/namespace/pinning.go
@@ -12,6 +12,24 @@ import (
 	"golang.org/x/sys/unix"
 )
 
+// blockPaths defines the host services that are bind-mounted over to block access
+// from within the isolated namespace.
+var blockPaths = []string{
+	"/run/dbus/system_bus_socket",
+	"/run/systemd/resolve/io.systemd.Resolve",
+	"/run/systemd/resolve/io.systemd.Resolve.Monitor",
+	"/run/nscd/socket",
+	"/var/run/dbus/system_bus_socket",
+	"/var/run/systemd/resolve/io.systemd.Resolve",
+	"/var/run/systemd/resolve/io.systemd.Resolve.Monitor",
+	"/var/run/nscd/socket",
+}
+
+// GetBlockPaths returns the list of paths blocked for namespace isolation.
+func GetBlockPaths() []string {
+	return blockPaths
+}
+
 // PinNamespace binds the current network namespace to the profile's namespace path.
 // This prevents the kernel from destroying the namespace when all processes exit.
 func PinNamespace(pm *paths.PathManager, profile string) error {
@@ -44,7 +62,6 @@ func UnpinNamespace(pm *paths.PathManager, profile string) error {
 	}
 
 	// 1. Unmount the namespace first.
-	// If this is the last reference to the namespace, the kernel will destroy it.
 	if err := unix.Unmount(nsPath, 0); err != nil {
 		return fmt.Errorf("failed to unmount namespace %s: %w", nsPath, err)
 	}
@@ -54,6 +71,16 @@ func UnpinNamespace(pm *paths.PathManager, profile string) error {
 		return fmt.Errorf("failed to remove pin file %s: %w", nsPath, err)
 	}
 
+	// 3. Unmount and clean up blocking services.
+	// Since the block files are located within the profile directory, 
+	// we must unmount them before we can remove the directory.
+	for _, p := range GetBlockPaths() {
+		_ = unix.Unmount(p, unix.MNT_DETACH)
+	}
+
+	blockDir := filepath.Join(pm.RuntimeBaseDir(), "profiles", profile, "block")
+	_ = os.RemoveAll(blockDir)
+
 	pidsDir := GetPidsDirPath(pm, profile)
 
 	// Try to remove pids directory and empty parent directories
-- 
cgit v1.2.3