diff --git a/pkg/util/lock/lock.go b/pkg/util/lock/lock.go index fb242e618d..637dde54d8 100644 --- a/pkg/util/lock/lock.go +++ b/pkg/util/lock/lock.go @@ -22,6 +22,7 @@ import ( "fmt" "os" "path/filepath" + "runtime" "time" "errors" @@ -99,10 +100,28 @@ func PathMutexSpec(path string) Spec { return s } +func getUserSpecificDirName() string { + if runtime.GOOS == "windows" { + homeDir, err := os.UserHomeDir() + if err != nil { + // Fallback if home dir cannot be obtained + return "minikube-locks-windows-default" + } + // On Windows, os.Getuid() returns -1, so we cannot verify the user ID directly. + // Instead, we use the SHA1 hash of the user's home directory as a unique identifier. + // This ensures per-user isolation in shared temporary directories. + hash := sha1.Sum([]byte(homeDir)) + return fmt.Sprintf("minikube-locks-windows-%x", hash) + } + return fmt.Sprintf("minikube-locks-%d", os.Getuid()) +} + // Acquire acquires the lock specified by spec func Acquire(spec Spec) (Releaser, error) { tmpDir := os.TempDir() - lockDir := filepath.Join(tmpDir, "minikube-locks") + // minikube-locks- or minikube-locks-windows- ensures per-user isolation. + userDirName := getUserSpecificDirName() + lockDir := filepath.Join(tmpDir, userDirName) if err := os.MkdirAll(lockDir, 0755); err != nil { return nil, fmt.Errorf("creating lock dir: %w", err) } diff --git a/pkg/util/lock/lock_test.go b/pkg/util/lock/lock_test.go index 699571c552..2f4d02ab4c 100644 --- a/pkg/util/lock/lock_test.go +++ b/pkg/util/lock/lock_test.go @@ -17,11 +17,18 @@ limitations under the License. package lock import ( + "fmt" + "os" "path/filepath" + "runtime" + "strings" "testing" "time" ) +// TestUserMutexSpec verifies that the MutexSpec generation is deterministic and collision-resistant. +// It ensures that different paths generate unique 40-character SHA1 based names, which is crucial +// for avoiding lock collisions between different projects or resources. func TestUserMutexSpec(t *testing.T) { var tests = []struct { description string @@ -172,3 +179,46 @@ func TestMutexConcurrency(t *testing.T) { } r2.Release() } + +// TestLockDirectoryStructure verifies that the lock directory is created with correct permissions and ownership. +// It addresses security concerns by ensuring: +// 1. The lock directory is created in a user-specific path to avoid permission denial in multi-user environments. +// 2. The directory is writable by the current user, ensuring locks can be acquired. +// 3. On Unix systems, the directory name contains the user's UID for clear isolation (e.g. minikube-locks-1000). +func TestLockDirectoryStructure(t *testing.T) { + // 1. Acquire a lock + spec := PathMutexSpec("test-lock-structure") + r, err := Acquire(spec) + if err != nil { + t.Fatalf("Acquire failed: %v", err) + } + defer r.Release() + + // 2. Verify the directory was created with the correct UID suffix + expectedDir := getUserSpecificDirName() + lockDir := filepath.Join(os.TempDir(), expectedDir) + + info, err := os.Stat(lockDir) + if err != nil { + t.Fatalf("Expected lock directory %s does not exist", lockDir) + } + + if !info.IsDir() { + t.Errorf("Expected %s to be a directory", lockDir) + } + + // 3. Verify that the directory matches likely expectation for current user + if runtime.GOOS != "windows" { + uid := os.Getuid() + if !strings.Contains(expectedDir, fmt.Sprintf("%d", uid)) { + t.Errorf("Expected dir name %s to contain user UID %d", expectedDir, uid) + } + } + + // 4. Verify that the directory is writable by the current user + testFile := filepath.Join(lockDir, "write-test") + if err := os.WriteFile(testFile, []byte("test"), 0600); err != nil { + t.Errorf("Expected to be able to write to %s: %v", lockDir, err) + } + os.Remove(testFile) +}