fix(proxmox): enforce cloud-image checksum verification

docs/homelab-verification.md

@@ -117,3 +117,27 @@ string `user` field generated by Proxmox 9.1.1, even with no bootstrap errors.
 Its JSON status distinguishes `status: done`, an empty `errors` list, and
 `recoverable_errors.DEPRECATED` warnings. Retain and report those warnings;
 accept a completed bootstrap only after checking its own readiness results.
+
+## Cloud-image checksum verification
+
+`cloudImage.checksum` accepts `algorithm:hex-digest`: md5, sha1, sha224, sha256,
+sha384, or sha512, with the full digest length required by that algorithm.
+Prefer SHA-256 or SHA-512 and obtain the expected digest from a trusted publisher.
+Omitting the field leaves downloads unverified. Malformed and unsupported
+checksums are rejected before template lookup.
+
+openctl passes the digest and algorithm to Proxmox's `download-url` API and
+requires a successful task `exitstatus` before creating or importing the template.
+A stopped task is not necessarily successful. Successful imports carry an
+`openctl-image-<algorithm>-<digest>` provenance tag. A pinned request can reuse a
+cached template only when that tag matches. Legacy unverified templates or a
+different digest fail closed: choose an unused `cloudImage.templateName` to build
+a newly verified template. Do not manually add a provenance tag to bypass this
+check.
+
+The native handler was exercised on Proxmox 9.1.1 with a disposable 1 MiB raw
+disk, never booted: a wrong SHA-256 failed before any VM/template creation, the
+correct digest imported and cloned successfully, a second VM reused the verified
+template without another download, and a changed digest could not use that
+cache. Both sandbox VMs, their template, and the downloaded image were removed;
+the existing development VM remained running.

internal/schema/schemas/proxmox/vm.cue

@@ -59,7 +59,9 @@ import "openctl.io/schemas/base"
 		url: string
 		// Proxmox storage that will host the downloaded image and template VM disk.
 		storage: string
-		// Optional checksum to verify the download (e.g. "sha256:abc123").
+		// Optional algorithm:hex-digest verified by Proxmox before image import.
+		// Supports md5, sha1, sha224, sha256 (64 hex characters), sha384, and sha512.
+		// Cached templates must carry the same verified digest.
 		checksum?: string
 		// Override the auto-generated template name (otherwise derived from URL).
 		templateName?: string

pkg/proxmox/client/client.go

@@ -805,16 +805,20 @@ func (c *Client) GetStorageInfo(ctx context.Context, storage string) (*StorageIn
 	return &result.Data, nil
 }
 
-// DownloadToStorage downloads a file from URL to Proxmox storage
-// Returns the task UPID for tracking progress
-func (c *Client) DownloadToStorage(ctx context.Context, node, storage, url, filename, contentType string) (string, error) {
+// DownloadToStorage starts a storage download, optionally verified by Proxmox.
+// It requires a task UPID so callers can check completion before using the file.
+func (c *Client) DownloadToStorage(ctx context.Context, node, storage, url, filename, contentType string, checksum ImageChecksum) (string, error) {
 	path := fmt.Sprintf("/api2/json/nodes/%s/storage/%s/download-url", node, storage)
 
 	params := map[string]any{
 		"url":      url,
 		"filename": filename,
 		"content":  contentType,
 	}
+	if checksum.algorithm != "" {
+		params["checksum-algorithm"] = checksum.algorithm
+		params["checksum"] = checksum.digest
+	}
 
 	debugf("DownloadToStorage: downloading %s to %s:%s/%s", url, storage, contentType, filename)
 
@@ -827,7 +831,10 @@ func (c *Client) DownloadToStorage(ctx context.Context, node, storage, url, file
 		Data string `json:"data"`
 	}
 	if err := json.Unmarshal(resp, &result); err != nil {
-		return "", nil
+		return "", fmt.Errorf("failed to parse download task: %w", err)
+	}
+	if result.Data == "" {
+		return "", fmt.Errorf("download response did not contain a task UPID")
 	}
 
 	return result.Data, nil
@@ -909,12 +916,16 @@ func (c *Client) getTaskStatus(ctx context.Context, node, upid string) (string,
 
 	var result struct {
 		Data struct {
-			Status string `json:"status"`
+			Status     string `json:"status"`
+			ExitStatus string `json:"exitstatus"`
 		} `json:"data"`
 	}
 	if err := json.Unmarshal(resp, &result); err != nil {
 		return "", err
 	}
+	if result.Data.Status == "stopped" && result.Data.ExitStatus != "OK" {
+		return "", fmt.Errorf("task %s failed: exitstatus %q", upid, result.Data.ExitStatus)
+	}
 
 	return result.Data.Status, nil
 }

pkg/proxmox/client/client_test.go

@@ -605,7 +605,7 @@ func TestDeleteVM_WaitsForDestroyTask(t *testing.T) {
 			json.NewEncoder(w).Encode(map[string]any{"data": "UPID:pve1:0000:qmdestroy:100:root@pam:"})
 		case strings.HasPrefix(r.URL.Path, "/api2/json/nodes/pve1/tasks/") && strings.HasSuffix(r.URL.Path, "/status"):
 			taskChecked = true
-			json.NewEncoder(w).Encode(map[string]any{"data": map[string]any{"status": "stopped"}})
+			json.NewEncoder(w).Encode(map[string]any{"data": map[string]any{"status": "stopped", "exitstatus": "OK"}})
 		default:
 			http.Error(w, "unexpected "+r.Method+" "+r.URL.Path, http.StatusNotFound)
 		}
@@ -874,46 +874,6 @@ func TestAddCloudInitDrive(t *testing.T) {
 	}
 }
 
-func TestDownloadToStorage(t *testing.T) {
-	var receivedParams map[string]string
-	server := mockServer(t, func(w http.ResponseWriter, r *http.Request) {
-		if r.URL.Path == "/api2/json/nodes/pve1/storage/local/download-url" && r.Method == "POST" {
-			r.ParseForm()
-			receivedParams = make(map[string]string)
-			for k, v := range r.Form {
-				if len(v) > 0 {
-					receivedParams[k] = v[0]
-				}
-			}
-			json.NewEncoder(w).Encode(map[string]any{
-				"data": "UPID:pve1:00001234:12345678:download:local:root@pam:",
-			})
-		}
-	})
-	defer server.Close()
-
-	c := New(server.URL, "test", "test")
-	c.httpClient = server.Client()
-
-	upid, err := c.DownloadToStorage(context.Background(), "pve1", "local", "https://example.com/image.img", "image.img", "iso")
-	if err != nil {
-		t.Fatalf("DownloadToStorage failed: %v", err)
-	}
-
-	if upid == "" {
-		t.Error("expected non-empty UPID")
-	}
-	if receivedParams["url"] != "https://example.com/image.img" {
-		t.Errorf("expected url parameter, got %s", receivedParams["url"])
-	}
-	if receivedParams["filename"] != "image.img" {
-		t.Errorf("expected filename parameter, got %s", receivedParams["filename"])
-	}
-	if receivedParams["content"] != "iso" {
-		t.Errorf("expected content=iso, got %s", receivedParams["content"])
-	}
-}
-
 func TestTruncate(t *testing.T) {
 	tests := []struct {
 		input    string

pkg/proxmox/client/image_checksum.go

@@ -0,0 +1,61 @@
+package client
+
+import (
+	"fmt"
+	"strings"
+)
+
+// ImageChecksum is a validated download digest. Its zero value disables checksum
+// verification; nonzero values must be constructed with ParseImageChecksum.
+type ImageChecksum struct {
+	algorithm string
+	digest    string
+}
+
+// ParseImageChecksum accepts algorithm:hex-digest for Proxmox's supported
+// algorithms. It validates the complete digest and canonicalizes hexadecimal case.
+func ParseImageChecksum(value string) (ImageChecksum, error) {
+	if value == "" {
+		return ImageChecksum{}, nil
+	}
+	algorithm, digest, ok := strings.Cut(value, ":")
+	if !ok {
+		return ImageChecksum{}, fmt.Errorf("checksum must use algorithm:hex-digest format")
+	}
+	var length int
+	switch algorithm {
+	case "md5":
+		length = 32
+	case "sha1":
+		length = 40
+	case "sha224":
+		length = 56
+	case "sha256":
+		length = 64
+	case "sha384":
+		length = 96
+	case "sha512":
+		length = 128
+	default:
+		return ImageChecksum{}, fmt.Errorf("unsupported checksum algorithm %q", algorithm)
+	}
+	if len(digest) != length {
+		return ImageChecksum{}, fmt.Errorf("%s checksum must contain %d hexadecimal characters", algorithm, length)
+	}
+	for i := range len(digest) {
+		b := digest[i]
+		if (b < '0' || b > '9') && (b < 'a' || b > 'f') && (b < 'A' || b > 'F') {
+			return ImageChecksum{}, fmt.Errorf("%s checksum contains a non-hexadecimal character", algorithm)
+		}
+	}
+	return ImageChecksum{algorithm: algorithm, digest: strings.ToLower(digest)}, nil
+}
+
+// TemplateTag records the digest verified before importing a cloud-image template.
+// Empty checksums deliberately have no provenance tag.
+func (c ImageChecksum) TemplateTag() string {
+	if c.algorithm == "" {
+		return ""
+	}
+	return "openctl-image-" + c.algorithm + "-" + c.digest
+}

pkg/proxmox/client/image_checksum_test.go

@@ -0,0 +1,63 @@
+package client
+
+import (
+	"context"
+	"encoding/json"
+	"net/http"
+	"strings"
+	"testing"
+	"time"
+)
+
+func TestParseImageChecksum(t *testing.T) {
+	for algorithm, size := range map[string]int{"md5": 32, "sha1": 40, "sha224": 56, "sha256": 64, "sha384": 96, "sha512": 128} {
+		t.Run(algorithm, func(t *testing.T) {
+			checksum, err := ParseImageChecksum(algorithm + ":" + strings.Repeat("Ab", size/2))
+			if err != nil || checksum.algorithm != algorithm || checksum.digest != strings.Repeat("ab", size/2) {
+				t.Fatalf("valid digest was not normalized: checksum=%+v err=%v", checksum, err)
+			}
+		})
+	}
+	for _, value := range []string{"sha256", "sha256:", "sha256:abc123", "sha256:" + strings.Repeat("g", 64), "sha256:" + strings.Repeat("0", 65), "sha256:" + strings.Repeat("0", 63), "blake3:" + strings.Repeat("0", 64), "sha256:" + strings.Repeat("0", 64) + ":extra", " sha256:" + strings.Repeat("0", 64)} {
+		if _, err := ParseImageChecksum(value); err == nil {
+			t.Errorf("invalid checksum accepted: %q", value)
+		}
+	}
+	checksum, err := ParseImageChecksum("")
+	if err != nil || checksum != (ImageChecksum{}) || checksum.TemplateTag() != "" {
+		t.Fatalf("omitted checksum must disable verification: %+v, %v", checksum, err)
+	}
+}
+
+func TestWaitForTaskRequiresSuccessfulExit(t *testing.T) {
+	for _, exitStatus := range []string{"OK", "checksum mismatch", ""} {
+		t.Run("exit="+exitStatus, func(t *testing.T) {
+			server := mockServer(t, func(w http.ResponseWriter, r *http.Request) {
+				json.NewEncoder(w).Encode(map[string]any{"data": map[string]string{"status": "stopped", "exitstatus": exitStatus}})
+			})
+			defer server.Close()
+			c := New(server.URL, "test", "test")
+			c.httpClient = server.Client()
+			err := c.WaitForTask(context.Background(), "pve1", "UPID:download", time.Second)
+			if (err == nil) != (exitStatus == "OK") {
+				t.Fatalf("only a successful task may proceed: exitstatus=%q err=%v", exitStatus, err)
+			}
+		})
+	}
+}
+
+func TestDownloadRequiresTrackableTask(t *testing.T) {
+	for _, response := range []string{`{"data":""}`, `{"data":null}`, `not json`} {
+		t.Run(response, func(t *testing.T) {
+			server := mockServer(t, func(w http.ResponseWriter, r *http.Request) {
+				_, _ = w.Write([]byte(response))
+			})
+			defer server.Close()
+			c := New(server.URL, "test", "test")
+			c.httpClient = server.Client()
+			if _, err := c.DownloadToStorage(context.Background(), "pve1", "local", "https://example.invalid/image.img", "image.qcow2", "import", ImageChecksum{}); err == nil {
+				t.Fatal("untrackable download would bypass completion and checksum verification")
+			}
+		})
+	}
+}