diff --git a/backend/internal/api/storage.go b/backend/internal/api/storage.go index 11e96d3..080a337 100644 --- a/backend/internal/api/storage.go +++ b/backend/internal/api/storage.go @@ -139,50 +139,35 @@ func buildStorageInfo() storageInfoResponse { } func normalizeStoragePoolsRequest(items []config.StoragePool) ([]config.StoragePool, error) { + return normalizeStoragePoolsRequestWithDisks(items, detectStorageDisks()) +} + +func normalizeStoragePoolsRequestWithDisks(items []config.StoragePool, disks []storageDiskInfo) ([]config.StoragePool, error) { if len(items) == 0 { return nil, fmt.Errorf("at least one mounted storage disk configuration must be retained") } result := make([]config.StoragePool, 0, len(items)) seen := map[string]bool{} defaultSeen := map[string]bool{} - disks := detectStorageDisks() for _, item := range items { - item.ID = strings.TrimSpace(item.ID) - item.Name = strings.TrimSpace(item.Name) - item.Path = filepath.Clean(strings.TrimSpace(item.Path)) - item.MountPoint = filepath.Clean(strings.TrimSpace(item.MountPoint)) - if item.MountPoint == "." { - item.MountPoint = "" + disk, managedPath, err := storageDiskForPoolRequest(item, disks) + if err != nil { + return nil, err } - if item.Name == "" { - return nil, fmt.Errorf("storage pool name is required") + id, name := storagePoolIdentity(disk) + if seen[id] { + return nil, fmt.Errorf("duplicate storage disk: %s", disk.MountPoint) } - if item.ID == "" { - item.ID = storageID(item.Name) - } - if seen[item.ID] { - return nil, fmt.Errorf("duplicate storage pool ID: %s", item.ID) - } - seen[item.ID] = true - if !filepath.IsAbs(item.Path) { - return nil, fmt.Errorf("%s path must be absolute", item.Name) - } - detectedMountPoint := bestMountPointForPath(item.Path, disks) - if detectedMountPoint == "" { - return nil, fmt.Errorf("%s path is not on an available mounted storage disk", item.Name) - } - if item.MountPoint != "" && filepath.Clean(item.MountPoint) != filepath.Clean(detectedMountPoint) { - return nil, fmt.Errorf("%s storage disk mount point has changed; refresh and try again", item.Name) - } - item.MountPoint = detectedMountPoint - item.ContentTypes = normalizeStorageContentTypes(item.ContentTypes) - item.DefaultContents = normalizeStorageContentTypes(item.DefaultContents) + seen[id] = true + + contentTypes := normalizeStorageContentTypes(item.ContentTypes) + defaultContents := normalizeStorageContentTypes(item.DefaultContents) allowed := map[string]bool{} - for _, content := range item.ContentTypes { + for _, content := range contentTypes { allowed[content] = true } - defaults := make([]string, 0, len(item.DefaultContents)) - for _, content := range item.DefaultContents { + defaults := make([]string, 0, len(defaultContents)) + for _, content := range defaultContents { if !allowed[content] { continue } @@ -192,25 +177,93 @@ func normalizeStoragePoolsRequest(items []config.StoragePool) ([]config.StorageP defaultSeen[content] = true defaults = append(defaults, content) } - item.DefaultContents = defaults - result = append(result, item) + result = append(result, config.StoragePool{ + ID: id, + Name: name, + Path: managedPath, + MountPoint: disk.MountPoint, + ContentTypes: contentTypes, + DefaultContents: defaults, + Enabled: item.Enabled, + }) } return result, nil } -func normalizeStorageContentTypes(values []string) []string { - valid := map[string]bool{ - config.StorageContentLXC: true, - config.StorageContentKVM: true, - config.StorageContentImages: true, - config.StorageContentSnapshots: true, - config.StorageContentBackups: true, +func storageDiskForPoolRequest(item config.StoragePool, disks []storageDiskInfo) (storageDiskInfo, string, error) { + requestedMount := filepath.Clean(strings.TrimSpace(item.MountPoint)) + if requestedMount == "." { + requestedMount = "" } + requestedPath := filepath.Clean(strings.TrimSpace(item.Path)) + if requestedPath == "." { + requestedPath = "" + } + for _, disk := range disks { + mountPoint := filepath.Clean(disk.MountPoint) + managedPath := managedStoragePath(mountPoint) + mountMatches := requestedMount != "" && requestedMount == mountPoint + pathMatches := requestedPath != "" && requestedPath == managedPath + if !mountMatches && !pathMatches { + continue + } + if requestedMount != "" && !mountMatches { + return storageDiskInfo{}, "", fmt.Errorf("storage disk mount point has changed; refresh and try again") + } + if requestedPath != "" && !pathMatches { + return storageDiskInfo{}, "", fmt.Errorf("custom storage paths are not allowed; refresh and try again") + } + return disk, managedPath, nil + } + return storageDiskInfo{}, "", fmt.Errorf("storage disk is not mounted or is no longer available") +} + +func storagePoolIdentity(disk storageDiskInfo) (string, string) { + mountPoint := filepath.Clean(disk.MountPoint) + if mountPoint == string(os.PathSeparator) { + return "disk-root", "system (/)" + } + baseName := filepath.Base(mountPoint) + if baseName == "" || baseName == "." || baseName == string(os.PathSeparator) { + baseName = strings.TrimSpace(disk.Name) + } + if baseName == "" { + baseName = "storage" + } + devicePath := strings.TrimSpace(disk.Path) + if devicePath == "" { + devicePath = strings.TrimSpace(disk.Name) + } + return "disk-" + storageID(baseName), fmt.Sprintf("%s (%s)", baseName, devicePath) +} + +func managedStoragePath(mountPoint string) string { + if filepath.Clean(mountPoint) == string(os.PathSeparator) { + return filepath.Join(string(os.PathSeparator), "var", "lib", "clicd") + } + return filepath.Join(filepath.Clean(mountPoint), "clicd") +} + +func normalizeStorageContentTypes(values []string) []string { seen := map[string]bool{} result := []string{} for _, value := range values { - next := strings.ToLower(strings.TrimSpace(value)) - if !valid[next] || seen[next] { + var next string + switch strings.ToLower(strings.TrimSpace(value)) { + case config.StorageContentLXC: + next = config.StorageContentLXC + case config.StorageContentKVM: + next = config.StorageContentKVM + case config.StorageContentImages: + next = config.StorageContentImages + case config.StorageContentSnapshots: + next = config.StorageContentSnapshots + case config.StorageContentBackups: + next = config.StorageContentBackups + default: + continue + } + if seen[next] { continue } seen[next] = true diff --git a/backend/internal/api/storage_test.go b/backend/internal/api/storage_test.go index 75eca59..d6350e8 100644 --- a/backend/internal/api/storage_test.go +++ b/backend/internal/api/storage_test.go @@ -5,6 +5,8 @@ import ( "path/filepath" "runtime" "testing" + + "clicd/internal/config" ) func TestIsUsableStorageMount(t *testing.T) { @@ -56,6 +58,46 @@ func TestBestMountPointForPath(t *testing.T) { } } +func TestNormalizeStoragePoolsUsesServerManagedPath(t *testing.T) { + disks := []storageDiskInfo{ + {Path: "/dev/sda2", MountPoint: "/"}, + {Path: "/dev/sdb1", MountPoint: "/mnt/data"}, + } + items := []config.StoragePool{{ + ID: "disk-data", + Name: "data", + Path: "/mnt/data/clicd", + MountPoint: "/mnt/data", + ContentTypes: []string{config.StorageContentLXC}, + DefaultContents: []string{config.StorageContentLXC}, + Enabled: true, + }} + pools, err := normalizeStoragePoolsRequestWithDisks(items, disks) + if err != nil { + t.Fatal(err) + } + wantPath := filepath.Join(filepath.Clean("/mnt/data"), "clicd") + if len(pools) != 1 || pools[0].ID != "disk-data" || pools[0].Name != "data (/dev/sdb1)" || pools[0].Path != wantPath || pools[0].MountPoint != "/mnt/data" { + t.Fatalf("unexpected normalized pools: %#v", pools) + } +} + +func TestNormalizeStoragePoolsRejectsUncontrolledPath(t *testing.T) { + disks := []storageDiskInfo{{Path: "/dev/sdb1", MountPoint: "/mnt/data"}} + for _, path := range []string{"/etc", "/mnt/data/clicd/../../etc", "/mnt/data/other"} { + _, err := normalizeStoragePoolsRequestWithDisks([]config.StoragePool{{ + ID: "disk-data", + Name: "data", + Path: path, + MountPoint: "/mnt/data", + Enabled: true, + }}, disks) + if err == nil { + t.Fatalf("path %q was accepted", path) + } + } +} + func TestDirSizeBytesUsesAllocatedBlocks(t *testing.T) { if runtime.GOOS != "linux" { t.Skip("allocated-block behavior is provided by the Linux du command") diff --git a/backend/internal/config/config.go b/backend/internal/config/config.go index 9a276d2..f972d99 100644 --- a/backend/internal/config/config.go +++ b/backend/internal/config/config.go @@ -223,6 +223,13 @@ func normalizeStoragePools() bool { if pool.MountPoint == "." { pool.MountPoint = "" } + if pool.MountPoint != "" { + managedPath := managedStoragePoolPath(pool.MountPoint) + if pool.Path != managedPath { + pool.Path = managedPath + changed = true + } + } if pool.ID == "" { pool.ID = storagePoolIDFromName(pool.Name, pool.Path) changed = true @@ -264,6 +271,14 @@ func normalizeStoragePools() bool { return changed } +func managedStoragePoolPath(mountPoint string) string { + mountPoint = filepath.Clean(strings.TrimSpace(mountPoint)) + if mountPoint == string(os.PathSeparator) { + return filepath.Join(string(os.PathSeparator), "var", "lib", "clicd") + } + return filepath.Join(mountPoint, "clicd") +} + func storagePoolIDFromName(name, path string) string { base := strings.ToLower(strings.TrimSpace(name)) if base == "" { diff --git a/backend/internal/config/storage_test.go b/backend/internal/config/storage_test.go index d9d35dc..3dcb74b 100644 --- a/backend/internal/config/storage_test.go +++ b/backend/internal/config/storage_test.go @@ -1,6 +1,30 @@ package config -import "testing" +import ( + "path/filepath" + "testing" +) + +func TestNormalizeStoragePoolsReplacesPersistedCustomPath(t *testing.T) { + previousConfig := AppConfig + t.Cleanup(func() { AppConfig = previousConfig }) + mountPoint := filepath.Join(t.TempDir(), "data") + + AppConfig = &ClicdConfig{StoragePools: []StoragePool{{ + ID: "data", + Name: "data", + Path: filepath.Join(t.TempDir(), "uncontrolled"), + MountPoint: mountPoint, + Enabled: true, + }}} + if !normalizeStoragePools() { + t.Fatal("expected custom path normalization to report a change") + } + want := managedStoragePoolPath(mountPoint) + if got := AppConfig.StoragePools[0].Path; got != want { + t.Fatalf("normalized path = %q, want %q", got, want) + } +} func TestSelectStoragePoolForContent(t *testing.T) { previousConfig := AppConfig diff --git a/backend/internal/kvm/kvm_test.go b/backend/internal/kvm/kvm_test.go index 665e36a..fdfa5ec 100644 --- a/backend/internal/kvm/kvm_test.go +++ b/backend/internal/kvm/kvm_test.go @@ -11,6 +11,17 @@ import ( "golang.org/x/crypto/ssh" ) +func TestLocalImageIDRejectsPathExpressions(t *testing.T) { + for _, id := range []string{"", ".", "..", "../../etc/passwd", `..\\..\\windows`, "/absolute"} { + if got := localImageID(id); got != "__invalid_image_id__" { + t.Fatalf("localImageID(%q) = %q", id, got) + } + } + if got := localImageID("debian-13-kvm"); got != "debian-13-kvm" { + t.Fatalf("localImageID(valid) = %q", got) + } +} + func TestChpasswdStdinPreservesShellMetacharacters(t *testing.T) { password := `pa'";$(touch /tmp/pwned); echo #\\word` got, err := chpasswdStdin("root", password) diff --git a/backend/internal/kvm/templates.go b/backend/internal/kvm/templates.go index 08296b3..d36777d 100644 --- a/backend/internal/kvm/templates.go +++ b/backend/internal/kvm/templates.go @@ -4,6 +4,7 @@ import ( "os" "path/filepath" "runtime" + "strings" "clicd/internal/config" ) @@ -195,7 +196,7 @@ func ImagePath(id string) string { if img != nil && img.Distro == "windows" { ext = ".iso" } - fileName := id + ext + fileName := localImageID(id) + ext for _, pool := range config.StoragePoolsForContent(config.StorageContentImages) { candidate := filepath.Join(pool.Path, "images", "kvm", fileName) if info, err := os.Stat(candidate); err == nil && !info.IsDir() { @@ -209,6 +210,15 @@ func ImagePath(id string) string { return filepath.Join(CacheDir(), fileName) } +func localImageID(id string) string { + trimmed := strings.TrimSpace(id) + local := filepath.Base(trimmed) + if trimmed == "" || local == "." || local == ".." || local != trimmed || strings.ContainsAny(trimmed, `/\\`) { + return "__invalid_image_id__" + } + return local +} + // IsWindowsImage returns true if the image distro is "windows". func IsWindowsImage(id string) bool { img := FindImage(id)