.service")
+ logger.Info(" - zpool import -d /dev/disk/by-id")
+ logger.Info("")
+}
+
+func detectConfiguredZFSPools() []string {
+ pools := make(map[string]struct{})
+ addConfiguredZFSPoolsFromDirs(pools)
+ addConfiguredZFSPoolsFromGlobPatterns(pools)
+ return sortedPoolNames(pools)
+}
+
+func addConfiguredZFSPoolsFromDirs(pools map[string]struct{}) {
+ directories := []string{
+ "/etc/systemd/system/zfs-import.target.wants",
+ "/etc/systemd/system/multi-user.target.wants",
+ }
+
+ for _, dir := range directories {
+ entries, err := restoreFS.ReadDir(dir)
+ if err != nil {
+ continue
+ }
+
+ for _, entry := range entries {
+ if pool := parsePoolNameFromUnit(entry.Name()); pool != "" {
+ pools[pool] = struct{}{}
+ }
+ }
+ }
+}
+
+func addConfiguredZFSPoolsFromGlobPatterns(pools map[string]struct{}) {
+ globPatterns := []string{
+ "/etc/systemd/system/zfs-import@*.service",
+ "/etc/systemd/system/import@*.service",
+ }
+
+ for _, pattern := range globPatterns {
+ matches, err := restoreGlob(pattern)
+ if err != nil {
+ continue
+ }
+ for _, match := range matches {
+ if pool := parsePoolNameFromUnit(filepath.Base(match)); pool != "" {
+ pools[pool] = struct{}{}
+ }
+ }
+ }
+}
+
+func sortedPoolNames(pools map[string]struct{}) []string {
+ var poolNames []string
+ for pool := range pools {
+ poolNames = append(poolNames, pool)
+ }
+ sort.Strings(poolNames)
+ return poolNames
+}
+
+func parsePoolNameFromUnit(unitName string) string {
+ switch {
+ case strings.HasPrefix(unitName, "zfs-import@") && strings.HasSuffix(unitName, ".service"):
+ pool := strings.TrimPrefix(unitName, "zfs-import@")
+ return strings.TrimSuffix(pool, ".service")
+ case strings.HasPrefix(unitName, "import@") && strings.HasSuffix(unitName, ".service"):
+ pool := strings.TrimPrefix(unitName, "import@")
+ return strings.TrimSuffix(pool, ".service")
+ default:
+ return ""
+ }
+}
+
+func detectImportableZFSPools() ([]string, string, error) {
+ output, err := restoreCmd.Run(context.Background(), "zpool", "import")
+ poolNames := parseZpoolImportOutput(string(output))
+ if err != nil {
+ return poolNames, string(output), err
+ }
+ return poolNames, string(output), nil
+}
+
+func parseZpoolImportOutput(output string) []string {
+ var pools []string
+ scanner := bufio.NewScanner(strings.NewReader(output))
+ for scanner.Scan() {
+ line := strings.TrimSpace(scanner.Text())
+ if strings.HasPrefix(strings.ToLower(line), "pool:") {
+ pool := strings.TrimSpace(line[len("pool:"):])
+ if pool != "" {
+ pools = append(pools, pool)
+ }
+ }
+ }
+ return pools
+}
+
+func combinePoolNames(a, b []string) []string {
+ merged := make(map[string]struct{})
+ for _, pool := range a {
+ merged[pool] = struct{}{}
+ }
+ for _, pool := range b {
+ merged[pool] = struct{}{}
+ }
+
+ if len(merged) == 0 {
+ return nil
+ }
+
+ names := make([]string, 0, len(merged))
+ for pool := range merged {
+ names = append(names, pool)
+ }
+ sort.Strings(names)
+ return names
+}
From e1db44b9ff173a1a7d059c7bdc56c0836262e04b Mon Sep 17 00:00:00 2001
From: Damiano <71268257+tis24dev@users.noreply.github.com>
Date: Tue, 5 May 2026 13:14:18 +0200
Subject: [PATCH 13/35] Encode email bodies as quoted-printable
Use quoted-printable encoding for text and HTML parts to ensure 7-bit-safe emails and avoid 8bit transfer encoding. Add encodeQuotedPrintableBody helper and required imports, update Content-Transfer-Encoding headers in buildEmailMessage for both plain/text and html parts, and add a test (TestEmailNotifierBuildEmailMessage_EncodesUTF8BodiesAsSevenBitSafe) that verifies quoted-printable is used and no raw non-ASCII bytes remain in the message.
---
internal/notify/email.go | 26 +++++++++----
.../notify/email_delivery_methods_test.go | 37 +++++++++++++++++++
2 files changed, 55 insertions(+), 8 deletions(-)
diff --git a/internal/notify/email.go b/internal/notify/email.go
index ac7675ae..6a6ccdac 100644
--- a/internal/notify/email.go
+++ b/internal/notify/email.go
@@ -1,11 +1,13 @@
package notify
import (
+ "bytes"
"context"
"encoding/base64"
"encoding/json"
"errors"
"fmt"
+ "mime/quotedprintable"
"os"
"os/exec"
"path/filepath"
@@ -1110,6 +1112,14 @@ func summarizeSendmailTranscript(transcript string) (highlights []string, remote
return highlights, remoteID, localQueueID
}
+func encodeQuotedPrintableBody(body string) string {
+ var encoded bytes.Buffer
+ writer := quotedprintable.NewWriter(&encoded)
+ _, _ = writer.Write([]byte(body))
+ _ = writer.Close()
+ return encoded.String()
+}
+
func (e *EmailNotifier) buildEmailMessage(recipient, subject, htmlBody, textBody string, data *NotificationData) (emailMessage, toHeader string) {
e.logger.Debug("=== Building email message ===")
@@ -1152,17 +1162,17 @@ func (e *EmailNotifier) buildEmailMessage(recipient, subject, htmlBody, textBody
// Plain text part
email.WriteString(fmt.Sprintf("--%s\n", altBoundary))
email.WriteString("Content-Type: text/plain; charset=UTF-8\n")
- email.WriteString("Content-Transfer-Encoding: 8bit\n")
+ email.WriteString("Content-Transfer-Encoding: quoted-printable\n")
email.WriteString("\n")
- email.WriteString(textBody)
+ email.WriteString(encodeQuotedPrintableBody(textBody))
email.WriteString("\n\n")
// HTML part
email.WriteString(fmt.Sprintf("--%s\n", altBoundary))
email.WriteString("Content-Type: text/html; charset=UTF-8\n")
- email.WriteString("Content-Transfer-Encoding: 8bit\n")
+ email.WriteString("Content-Transfer-Encoding: quoted-printable\n")
email.WriteString("\n")
- email.WriteString(htmlBody)
+ email.WriteString(encodeQuotedPrintableBody(htmlBody))
email.WriteString("\n\n")
email.WriteString(fmt.Sprintf("--%s--\n", altBoundary))
@@ -1204,17 +1214,17 @@ func (e *EmailNotifier) buildEmailMessage(recipient, subject, htmlBody, textBody
// Plain text part
email.WriteString(fmt.Sprintf("--%s\n", altBoundary))
email.WriteString("Content-Type: text/plain; charset=UTF-8\n")
- email.WriteString("Content-Transfer-Encoding: 8bit\n")
+ email.WriteString("Content-Transfer-Encoding: quoted-printable\n")
email.WriteString("\n")
- email.WriteString(textBody)
+ email.WriteString(encodeQuotedPrintableBody(textBody))
email.WriteString("\n\n")
// HTML part
email.WriteString(fmt.Sprintf("--%s\n", altBoundary))
email.WriteString("Content-Type: text/html; charset=UTF-8\n")
- email.WriteString("Content-Transfer-Encoding: 8bit\n")
+ email.WriteString("Content-Transfer-Encoding: quoted-printable\n")
email.WriteString("\n")
- email.WriteString(htmlBody)
+ email.WriteString(encodeQuotedPrintableBody(htmlBody))
email.WriteString("\n\n")
email.WriteString(fmt.Sprintf("--%s--\n", altBoundary))
diff --git a/internal/notify/email_delivery_methods_test.go b/internal/notify/email_delivery_methods_test.go
index 41c42765..10a73fea 100644
--- a/internal/notify/email_delivery_methods_test.go
+++ b/internal/notify/email_delivery_methods_test.go
@@ -377,6 +377,43 @@ func TestEmailNotifierBuildEmailMessage_FallsBackWhenLogUnreadable(t *testing.T)
}
}
+func TestEmailNotifierBuildEmailMessage_EncodesUTF8BodiesAsSevenBitSafe(t *testing.T) {
+ logger := logging.New(types.LogLevelDebug, false)
+ logger.SetOutput(io.Discard)
+
+ notifier, err := NewEmailNotifier(EmailConfig{
+ Enabled: true,
+ DeliveryMethod: EmailDeliveryPMF,
+ From: "no-reply@proxmox.example.com",
+ }, types.ProxmoxBS, logger)
+ if err != nil {
+ t.Fatalf("NewEmailNotifier() error = %v", err)
+ }
+
+ emailMessage, _ := notifier.buildEmailMessage(
+ "admin@example.com",
+ "✅ PVE Backup à",
+ "Backup completato ✅ con avvisi: è pieno
",
+ "Backup completato ✅ con avvisi: è pieno",
+ createTestNotificationData(),
+ )
+
+ if strings.Contains(emailMessage, "Content-Transfer-Encoding: 8bit") {
+ t.Fatalf("email message must not use 8bit transfer encoding:\n%s", emailMessage)
+ }
+ if count := strings.Count(emailMessage, "Content-Transfer-Encoding: quoted-printable"); count != 2 {
+ t.Fatalf("expected two quoted-printable body parts, got %d:\n%s", count, emailMessage)
+ }
+ if strings.Contains(emailMessage, "✅") || strings.Contains(emailMessage, "è") || strings.Contains(emailMessage, "à") {
+ t.Fatalf("email message contains raw non-ASCII body/subject characters:\n%s", emailMessage)
+ }
+ for i, b := range []byte(emailMessage) {
+ if b > 0x7f {
+ t.Fatalf("email message contains non-ASCII byte 0x%x at offset %d", b, i)
+ }
+ }
+}
+
func TestEmailNotifierIsMTAServiceActive_SystemctlMissing(t *testing.T) {
logger := logging.New(types.LogLevelDebug, false)
logger.SetOutput(io.Discard)
From 743b24e6676ccb8edf09b92df95b23568394c084 Mon Sep 17 00:00:00 2001
From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com>
Date: Tue, 5 May 2026 13:40:14 +0200
Subject: [PATCH 14/35] deps(deps): bump github.com/gdamore/tcell/v2 from
2.13.8 to 2.13.9 in the security-patches group across 1 directory (#200)
deps(deps): bump github.com/gdamore/tcell/v2
Bumps the security-patches group with 1 update in the / directory: [github.com/gdamore/tcell/v2](https://github.com/gdamore/tcell).
Updates `github.com/gdamore/tcell/v2` from 2.13.8 to 2.13.9
- [Release notes](https://github.com/gdamore/tcell/releases)
- [Changelog](https://github.com/gdamore/tcell/blob/main/CHANGESv3.md)
- [Commits](https://github.com/gdamore/tcell/compare/v2.13.8...v2.13.9)
---
updated-dependencies:
- dependency-name: github.com/gdamore/tcell/v2
dependency-version: 2.13.9
dependency-type: direct:production
update-type: version-update:semver-patch
dependency-group: security-patches
...
Signed-off-by: dependabot[bot]
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
---
go.mod | 2 +-
go.sum | 4 ++--
2 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/go.mod b/go.mod
index 21b4cdd9..8bc70ffe 100644
--- a/go.mod
+++ b/go.mod
@@ -6,7 +6,7 @@ toolchain go1.25.9
require (
filippo.io/age v1.3.1
- github.com/gdamore/tcell/v2 v2.13.8
+ github.com/gdamore/tcell/v2 v2.13.9
github.com/rivo/tview v0.42.0
golang.org/x/crypto v0.50.0
golang.org/x/term v0.42.0
diff --git a/go.sum b/go.sum
index ac725be0..ca48a79f 100644
--- a/go.sum
+++ b/go.sum
@@ -8,8 +8,8 @@ filippo.io/hpke v0.4.0 h1:p575VVQ6ted4pL+it6M00V/f2qTZITO0zgmdKCkd5+A=
filippo.io/hpke v0.4.0/go.mod h1:EmAN849/P3qdeK+PCMkDpDm83vRHM5cDipBJ8xbQLVY=
github.com/gdamore/encoding v1.0.1 h1:YzKZckdBL6jVt2Gc+5p82qhrGiqMdG/eNs6Wy0u3Uhw=
github.com/gdamore/encoding v1.0.1/go.mod h1:0Z0cMFinngz9kS1QfMjCP8TY7em3bZYeeklsSDPivEo=
-github.com/gdamore/tcell/v2 v2.13.8 h1:Mys/Kl5wfC/GcC5Cx4C2BIQH9dbnhnkPgS9/wF3RlfU=
-github.com/gdamore/tcell/v2 v2.13.8/go.mod h1:+Wfe208WDdB7INEtCsNrAN6O2m+wsTPk1RAovjaILlo=
+github.com/gdamore/tcell/v2 v2.13.9 h1:uI5l3DYPcFvHINKlGft+en23evOKL+dwtD21QR8ejVA=
+github.com/gdamore/tcell/v2 v2.13.9/go.mod h1:+Wfe208WDdB7INEtCsNrAN6O2m+wsTPk1RAovjaILlo=
github.com/lucasb-eyer/go-colorful v1.3.0 h1:2/yBRLdWBZKrf7gB40FoiKfAWYQ0lqNcbuQwVHXptag=
github.com/lucasb-eyer/go-colorful v1.3.0/go.mod h1:R4dSotOR9KMtayYi1e77YzuveK+i7ruzyGqttikkLy0=
github.com/rivo/tview v0.42.0 h1:b/ftp+RxtDsHSaynXTbJb+/n/BxDEi+W3UfF5jILK6c=
From 323b45e4f8f7efe3dd6150aecb5a1982dd940d8b Mon Sep 17 00:00:00 2001
From: Damiano <71268257+tis24dev@users.noreply.github.com>
Date: Tue, 5 May 2026 13:54:26 +0200
Subject: [PATCH 15/35] Validate and resolve hardlink targets
Reject empty or absolute hardlink Linkname values and normalize them with filepath.FromSlash. Resolve the hardlink target via resolvePathWithinRootFS and use the resolved path for creation to prevent links escaping the extraction root. Add tests: recordingLinkFS to capture Link() args, TestExtractHardlink_UsesResolvedTargetPath to ensure the resolved target is used, and TestExtractHardlink_RejectsSymlinkEscapeTarget to verify symlink-escape targets are rejected.
---
.../orchestrator/restore_archive_entries.go | 12 ++-
internal/orchestrator/restore_test.go | 92 +++++++++++++++++++
2 files changed, 99 insertions(+), 5 deletions(-)
diff --git a/internal/orchestrator/restore_archive_entries.go b/internal/orchestrator/restore_archive_entries.go
index f77e4946..487dde72 100644
--- a/internal/orchestrator/restore_archive_entries.go
+++ b/internal/orchestrator/restore_archive_entries.go
@@ -230,20 +230,22 @@ func extractSymlink(target string, header *tar.Header, destRoot string, logger *
// extractHardlink creates a hard link
func extractHardlink(target string, header *tar.Header, destRoot string) error {
// Validate hard link target
- linkName := header.Linkname
+ linkName := filepath.FromSlash(header.Linkname)
+ if linkName == "" || filepath.Clean(linkName) == "." {
+ return fmt.Errorf("empty hardlink target not allowed")
+ }
// Reject absolute hard link targets immediately
if filepath.IsAbs(linkName) {
return fmt.Errorf("absolute hardlink target not allowed: %s", linkName)
}
- // Validate the hard link target stays within extraction root
- if _, err := resolvePathWithinRootFS(restoreFS, destRoot, linkName); err != nil {
+ // Resolve and validate the hard link target stays within extraction root.
+ linkTarget, err := resolvePathWithinRootFS(restoreFS, destRoot, linkName)
+ if err != nil {
return fmt.Errorf("hardlink target escapes root: %s -> %s: %w", header.Name, linkName, err)
}
- linkTarget := filepath.Join(destRoot, linkName)
-
// Remove existing file/link if it exists
_ = restoreFS.Remove(target)
diff --git a/internal/orchestrator/restore_test.go b/internal/orchestrator/restore_test.go
index 2a9c37a5..9dcdfc00 100644
--- a/internal/orchestrator/restore_test.go
+++ b/internal/orchestrator/restore_test.go
@@ -748,6 +748,18 @@ func TestExtractDirectory_Success(t *testing.T) {
// extractHardlink tests
// --------------------------------------------------------------------------
+type recordingLinkFS struct {
+ *FakeFS
+ oldname string
+ newname string
+}
+
+func (f *recordingLinkFS) Link(oldname, newname string) error {
+ f.oldname = oldname
+ f.newname = newname
+ return f.FakeFS.Link(oldname, newname)
+}
+
func TestExtractHardlink_AbsoluteTargetRejected(t *testing.T) {
header := &tar.Header{
Name: "link",
@@ -774,6 +786,86 @@ func TestExtractHardlink_EscapesRoot(t *testing.T) {
}
}
+func TestExtractHardlink_UsesResolvedTargetPath(t *testing.T) {
+ orig := restoreFS
+ fakeFS := NewFakeFS()
+ recordingFS := &recordingLinkFS{FakeFS: fakeFS}
+ restoreFS = recordingFS
+ t.Cleanup(func() {
+ restoreFS = orig
+ _ = fakeFS.Cleanup()
+ })
+
+ destRoot := fakeFS.Root
+ realDir := filepath.Join(destRoot, "real")
+ if err := fakeFS.MkdirAll(realDir, 0o755); err != nil {
+ t.Fatalf("mkdir real dir: %v", err)
+ }
+ realTarget := filepath.Join(realDir, "target.txt")
+ if err := fakeFS.WriteFile(realTarget, []byte("test"), 0o644); err != nil {
+ t.Fatalf("write real target: %v", err)
+ }
+ if err := os.Symlink("real", filepath.Join(destRoot, "alias")); err != nil {
+ t.Fatalf("create alias symlink: %v", err)
+ }
+
+ header := &tar.Header{
+ Name: "hardlink.txt",
+ Linkname: filepath.Join("alias", "target.txt"),
+ Typeflag: tar.TypeLink,
+ }
+ linkFile := filepath.Join(destRoot, header.Name)
+
+ if err := extractHardlink(linkFile, header, destRoot); err != nil {
+ t.Fatalf("extractHardlink failed: %v", err)
+ }
+ if recordingFS.oldname != realTarget {
+ t.Fatalf("hardlink source = %q, want resolved target %q", recordingFS.oldname, realTarget)
+ }
+ if recordingFS.newname != linkFile {
+ t.Fatalf("hardlink destination = %q, want %q", recordingFS.newname, linkFile)
+ }
+
+ realInfo, err := os.Stat(realTarget)
+ if err != nil {
+ t.Fatalf("stat real target: %v", err)
+ }
+ linkInfo, err := os.Stat(linkFile)
+ if err != nil {
+ t.Fatalf("stat hardlink: %v", err)
+ }
+ if !os.SameFile(realInfo, linkInfo) {
+ t.Fatalf("hardlink does not point to resolved target")
+ }
+}
+
+func TestExtractHardlink_RejectsSymlinkEscapeTarget(t *testing.T) {
+ orig := restoreFS
+ restoreFS = osFS{}
+ t.Cleanup(func() { restoreFS = orig })
+
+ destRoot := t.TempDir()
+ outside := t.TempDir()
+ if err := os.Symlink(outside, filepath.Join(destRoot, "escape-link")); err != nil {
+ t.Fatalf("create escape symlink: %v", err)
+ }
+
+ header := &tar.Header{
+ Name: "link.txt",
+ Linkname: filepath.Join("escape-link", "target.txt"),
+ Typeflag: tar.TypeLink,
+ }
+ linkFile := filepath.Join(destRoot, header.Name)
+
+ err := extractHardlink(linkFile, header, destRoot)
+ if err == nil || !strings.Contains(err.Error(), "escapes root") {
+ t.Fatalf("expected escapes root error, got: %v", err)
+ }
+ if _, err := os.Lstat(linkFile); !os.IsNotExist(err) {
+ t.Fatalf("hardlink should not be created, got err=%v", err)
+ }
+}
+
func TestExtractHardlink_Success(t *testing.T) {
orig := restoreFS
t.Cleanup(func() { restoreFS = orig })
From f89bdce5d438fcb39412adcdfd783761b086a599 Mon Sep 17 00:00:00 2001
From: Damiano <71268257+tis24dev@users.noreply.github.com>
Date: Tue, 5 May 2026 14:05:29 +0200
Subject: [PATCH 16/35] Add additional orchestrator tests
Add new unit tests for the orchestrator package. Tests cover estimatedBackupSizeGB scaling and minimum, BackupError wrapping and default messages for disk validation, backup metrics exit code logic, filling backup timing fields, and building backup collector config merging runtime excludes and blacklist. Also add restore tests for extractPlainArchive honoring skip functions and for runSafeClusterApplyWithUI ensuring storage/datacenter apply is skipped when storage_pve is staged.
---
.../backup_run_helpers_additional_test.go | 130 ++++++++++++++++++
.../restore_archive_additional_test.go | 40 ++++++
.../restore_cluster_apply_additional_test.go | 60 ++++++++
3 files changed, 230 insertions(+)
create mode 100644 internal/orchestrator/backup_run_helpers_additional_test.go
create mode 100644 internal/orchestrator/restore_archive_additional_test.go
create mode 100644 internal/orchestrator/restore_cluster_apply_additional_test.go
diff --git a/internal/orchestrator/backup_run_helpers_additional_test.go b/internal/orchestrator/backup_run_helpers_additional_test.go
new file mode 100644
index 00000000..49c33b93
--- /dev/null
+++ b/internal/orchestrator/backup_run_helpers_additional_test.go
@@ -0,0 +1,130 @@
+package orchestrator
+
+import (
+ "errors"
+ "math"
+ "strings"
+ "testing"
+ "time"
+
+ "github.com/tis24dev/proxsave/internal/config"
+ "github.com/tis24dev/proxsave/internal/types"
+)
+
+func TestEstimatedBackupSizeGBMinimumAndScaling(t *testing.T) {
+ tests := []struct {
+ name string
+ bytes int64
+ want float64
+ }{
+ {name: "zero uses minimum", bytes: 0, want: 0.001},
+ {name: "below minimum uses minimum", bytes: 512, want: 0.001},
+ {name: "one gibibyte", bytes: 1024 * 1024 * 1024, want: 1},
+ {name: "two and a half gibibytes", bytes: 5 * 1024 * 1024 * 1024 / 2, want: 2.5},
+ }
+
+ for _, tt := range tests {
+ t.Run(tt.name, func(t *testing.T) {
+ if got := estimatedBackupSizeGB(tt.bytes); math.Abs(got-tt.want) > 0.0000001 {
+ t.Fatalf("estimatedBackupSizeGB(%d)=%f want %f", tt.bytes, got, tt.want)
+ }
+ })
+ }
+}
+
+func TestBackupDiskValidationErrorWrapsDiskError(t *testing.T) {
+ diskErr := errors.New("need 3.5 GB free")
+ err := backupDiskValidationError("", diskErr)
+
+ var backupErr *BackupError
+ if !errors.As(err, &backupErr) {
+ t.Fatalf("expected BackupError, got %T", err)
+ }
+ if backupErr.Phase != "disk" || backupErr.Code != types.ExitDiskSpaceError {
+ t.Fatalf("unexpected BackupError fields: phase=%q code=%v", backupErr.Phase, backupErr.Code)
+ }
+ if !errors.Is(err, diskErr) {
+ t.Fatalf("expected disk error to be wrapped, got %v", err)
+ }
+}
+
+func TestBackupDiskValidationErrorUsesDefaultMessage(t *testing.T) {
+ err := backupDiskValidationError("", nil)
+
+ var backupErr *BackupError
+ if !errors.As(err, &backupErr) {
+ t.Fatalf("expected BackupError, got %T", err)
+ }
+ if !strings.Contains(err.Error(), "insufficient disk space") {
+ t.Fatalf("expected default disk space message, got %q", err.Error())
+ }
+}
+
+func TestBackupMetricsExitCode(t *testing.T) {
+ if got := backupMetricsExitCode(&BackupStats{}, nil); got != types.ExitSuccess.Int() {
+ t.Fatalf("success exit code=%d want %d", got, types.ExitSuccess.Int())
+ }
+ if got := backupMetricsExitCode(&BackupStats{ExitCode: 77}, nil); got != 77 {
+ t.Fatalf("stats exit code=%d want 77", got)
+ }
+
+ runErr := &BackupError{Phase: "disk", Err: errors.New("full"), Code: types.ExitDiskSpaceError}
+ if got := backupMetricsExitCode(&BackupStats{}, runErr); got != types.ExitDiskSpaceError.Int() {
+ t.Fatalf("backup error exit code=%d want %d", got, types.ExitDiskSpaceError.Int())
+ }
+ if got := backupMetricsExitCode(&BackupStats{}, errors.New("boom")); got != types.ExitGenericError.Int() {
+ t.Fatalf("generic error exit code=%d want %d", got, types.ExitGenericError.Int())
+ }
+}
+
+func TestEnsureBackupStatsTimingFillsEndAndDuration(t *testing.T) {
+ now := time.Date(2026, 5, 5, 10, 30, 0, 0, time.UTC)
+ orch := New(newTestLogger(), false)
+ orch.clock = &FakeTime{Current: now}
+
+ stats := &BackupStats{StartTime: now.Add(-90 * time.Second)}
+ orch.ensureBackupStatsTiming(stats)
+
+ if !stats.EndTime.Equal(now) {
+ t.Fatalf("EndTime=%v want %v", stats.EndTime, now)
+ }
+ if stats.Duration != 90*time.Second {
+ t.Fatalf("Duration=%v want %v", stats.Duration, 90*time.Second)
+ }
+}
+
+func TestBuildBackupCollectorConfigMergesRuntimeExcludesAndBlacklist(t *testing.T) {
+ orch := New(newTestLogger(), false)
+ orch.SetBackupConfig("/backup", "/logs", types.CompressionZstd, 3, 2, "fast", []string{"runtime/**"})
+ orch.SetConfig(&config.Config{
+ BackupBlacklist: []string{"/secret", "/tmp/cache"},
+ CustomBackupPaths: []string{"/srv/app"},
+ BaseDir: "/opt/proxsave",
+ ConfigPath: "/etc/proxsave/backup.env",
+ })
+
+ cfg := orch.buildBackupCollectorConfig()
+ for _, want := range []string{"runtime/**", "/secret", "/tmp/cache"} {
+ if !containsString(cfg.ExcludePatterns, want) {
+ t.Fatalf("ExcludePatterns missing %q: %#v", want, cfg.ExcludePatterns)
+ }
+ }
+ if len(cfg.BackupBlacklist) != 2 || cfg.BackupBlacklist[0] != "/secret" || cfg.BackupBlacklist[1] != "/tmp/cache" {
+ t.Fatalf("BackupBlacklist not copied: %#v", cfg.BackupBlacklist)
+ }
+ if len(cfg.CustomBackupPaths) != 1 || cfg.CustomBackupPaths[0] != "/srv/app" {
+ t.Fatalf("CustomBackupPaths not copied: %#v", cfg.CustomBackupPaths)
+ }
+ if cfg.ScriptRepositoryPath != "/opt/proxsave" || cfg.ConfigFilePath != "/etc/proxsave/backup.env" {
+ t.Fatalf("paths not copied: script=%q config=%q", cfg.ScriptRepositoryPath, cfg.ConfigFilePath)
+ }
+}
+
+func containsString(values []string, want string) bool {
+ for _, value := range values {
+ if value == want {
+ return true
+ }
+ }
+ return false
+}
diff --git a/internal/orchestrator/restore_archive_additional_test.go b/internal/orchestrator/restore_archive_additional_test.go
new file mode 100644
index 00000000..f8fda96d
--- /dev/null
+++ b/internal/orchestrator/restore_archive_additional_test.go
@@ -0,0 +1,40 @@
+package orchestrator
+
+import (
+ "context"
+ "os"
+ "path/filepath"
+ "testing"
+)
+
+func TestExtractPlainArchiveHonorsSkipFn(t *testing.T) {
+ origFS := restoreFS
+ t.Cleanup(func() { restoreFS = origFS })
+ restoreFS = osFS{}
+
+ tmpDir := t.TempDir()
+ archivePath := filepath.Join(tmpDir, "backup.tar")
+ if err := writeTarFile(archivePath, map[string]string{
+ "etc/fstab": "/dev/sda1 / ext4 defaults 0 1\n",
+ "etc/hosts": "127.0.0.1 localhost\n",
+ }); err != nil {
+ t.Fatalf("write archive: %v", err)
+ }
+
+ destRoot := filepath.Join(tmpDir, "restore")
+ if err := extractPlainArchive(context.Background(), archivePath, destRoot, newTestLogger(), fullRestoreSkipFn(true)); err != nil {
+ t.Fatalf("extractPlainArchive error: %v", err)
+ }
+
+ if _, err := os.Stat(filepath.Join(destRoot, "etc", "fstab")); !os.IsNotExist(err) {
+ t.Fatalf("expected skipped fstab to be absent, stat err=%v", err)
+ }
+
+ hosts, err := os.ReadFile(filepath.Join(destRoot, "etc", "hosts"))
+ if err != nil {
+ t.Fatalf("expected hosts to be extracted: %v", err)
+ }
+ if string(hosts) != "127.0.0.1 localhost\n" {
+ t.Fatalf("hosts content=%q", string(hosts))
+ }
+}
diff --git a/internal/orchestrator/restore_cluster_apply_additional_test.go b/internal/orchestrator/restore_cluster_apply_additional_test.go
new file mode 100644
index 00000000..06a61e68
--- /dev/null
+++ b/internal/orchestrator/restore_cluster_apply_additional_test.go
@@ -0,0 +1,60 @@
+package orchestrator
+
+import (
+ "context"
+ "os"
+ "path/filepath"
+ "strings"
+ "testing"
+)
+
+func TestRunSafeClusterApplyWithUI_SkipsStorageDatacenterWhenStoragePVEStaged(t *testing.T) {
+ origCmd := restoreCmd
+ origFS := restoreFS
+ t.Cleanup(func() {
+ restoreCmd = origCmd
+ restoreFS = origFS
+ })
+ restoreFS = osFS{}
+
+ pathDir := t.TempDir()
+ pveshPath := filepath.Join(pathDir, "pvesh")
+ if err := os.WriteFile(pveshPath, []byte("#!/bin/sh\nexit 0\n"), 0o755); err != nil {
+ t.Fatalf("write pvesh: %v", err)
+ }
+ t.Setenv("PATH", pathDir+string(os.PathListSeparator)+os.Getenv("PATH"))
+
+ runner := &recordingRunner{}
+ restoreCmd = runner
+
+ exportRoot := t.TempDir()
+ pveDir := filepath.Join(exportRoot, "etc", "pve")
+ if err := os.MkdirAll(pveDir, 0o755); err != nil {
+ t.Fatalf("mkdir pve dir: %v", err)
+ }
+ if err := os.WriteFile(filepath.Join(pveDir, "storage.cfg"), []byte("storage: local\n type dir\n"), 0o640); err != nil {
+ t.Fatalf("write storage.cfg: %v", err)
+ }
+ if err := os.WriteFile(filepath.Join(pveDir, "datacenter.cfg"), []byte("keyboard: it\n"), 0o640); err != nil {
+ t.Fatalf("write datacenter.cfg: %v", err)
+ }
+
+ plan := &RestorePlan{
+ SystemType: SystemTypePVE,
+ StagedCategories: []Category{{ID: "storage_pve", Type: CategoryTypePVE}},
+ }
+ ui := &fakeRestoreWorkflowUI{
+ applyStorageCfg: true,
+ applyDatacenterCfg: true,
+ }
+
+ if err := runSafeClusterApplyWithUI(context.Background(), ui, exportRoot, newTestLogger(), plan); err != nil {
+ t.Fatalf("runSafeClusterApplyWithUI error: %v", err)
+ }
+
+ for _, call := range runner.calls {
+ if strings.Contains(call, "/cluster/storage") || strings.Contains(call, "/cluster/config") {
+ t.Fatalf("storage/datacenter apply should be skipped for storage_pve staged restore; calls=%#v", runner.calls)
+ }
+ }
+}
From 481a2d63d08248accc438068d6a1eaf1c3b3cd64 Mon Sep 17 00:00:00 2001
From: Damiano <71268257+tis24dev@users.noreply.github.com>
Date: Tue, 5 May 2026 14:37:47 +0200
Subject: [PATCH 17/35] Improve PVE collection, mail path & restore abort
Return the first capture error from PVE collection loops (so collection continues but callers get notified) and surface errors from schedule captures. Add tests to assert capture-error behavior for backup history, replication status, and schedule collection. Introduce lookupAbsolutePath and use it to resolve systemctl/mailq paths (and use TrustedCommandContext/commandForMailTool) to make mail/service checks more robust. Clear stale restore abort info at start of runRestoreWorkflowWithUI and add a test ensuring abort info is cleared before validation.
---
internal/backup/collector_pve.go | 30 +++--
.../collector_pve_capture_errors_test.go | 106 ++++++++++++++++++
internal/notify/email.go | 36 ++++--
.../restore_workflow_abort_test.go | 12 ++
internal/orchestrator/restore_workflow_ui.go | 1 +
5 files changed, 163 insertions(+), 22 deletions(-)
create mode 100644 internal/backup/collector_pve_capture_errors_test.go
diff --git a/internal/backup/collector_pve.go b/internal/backup/collector_pve.go
index 815e88fb..60af6575 100644
--- a/internal/backup/collector_pve.go
+++ b/internal/backup/collector_pve.go
@@ -790,6 +790,7 @@ func (c *Collector) collectPVEBackupJobHistory(ctx context.Context, nodes []stri
}
seen := make(map[string]struct{})
+ var firstErr error
for _, node := range nodes {
if err := ctx.Err(); err != nil {
return err
@@ -803,13 +804,15 @@ func (c *Collector) collectPVEBackupJobHistory(ctx context.Context, nodes []stri
}
seen[node] = struct{}{}
outputPath := filepath.Join(jobsDir, fmt.Sprintf("%s_backup_history.json", node))
- c.captureCommandOutput(ctx,
+ if _, err := c.captureCommandOutput(ctx,
commandSpec("pvesh", "get", fmt.Sprintf("/nodes/%s/tasks", node), "--output-format=json", "--typefilter=vzdump"),
outputPath,
fmt.Sprintf("%s backup history", node),
- false)
+ false); err != nil && firstErr == nil {
+ firstErr = err
+ }
}
- return nil
+ return firstErr
}
func (c *Collector) collectPVEVZDumpCronSnapshot(ctx context.Context) error {
@@ -888,11 +891,13 @@ func (c *Collector) collectPVEScheduleCrontab(ctx context.Context) error {
return fmt.Errorf("failed to create schedules directory: %w", err)
}
- c.captureCommandOutput(ctx,
+ if _, err := c.captureCommandOutput(ctx,
commandSpec("crontab", "-l"),
filepath.Join(schedulesDir, "root_crontab.txt"),
"root crontab",
- false)
+ false); err != nil {
+ return fmt.Errorf("collectPVEScheduleCrontab: %w", err)
+ }
return nil
}
@@ -904,11 +909,13 @@ func (c *Collector) collectPVEScheduleTimers(ctx context.Context) error {
if err := c.ensureDir(schedulesDir); err != nil {
return fmt.Errorf("failed to create schedules directory: %w", err)
}
- c.captureCommandOutput(ctx,
+ if _, err := c.captureCommandOutput(ctx,
commandSpec("systemctl", "list-timers", "--all", "--no-pager"),
filepath.Join(schedulesDir, "systemd_timers.txt"),
"systemd timers",
- false)
+ false); err != nil {
+ return fmt.Errorf("collectPVEScheduleTimers: %w", err)
+ }
return nil
}
@@ -965,6 +972,7 @@ func (c *Collector) collectPVEReplicationStatus(ctx context.Context, nodes []str
}
seen := make(map[string]struct{})
+ var firstErr error
for _, node := range nodes {
if err := ctx.Err(); err != nil {
return err
@@ -978,13 +986,15 @@ func (c *Collector) collectPVEReplicationStatus(ctx context.Context, nodes []str
}
seen[node] = struct{}{}
outputPath := filepath.Join(repDir, fmt.Sprintf("%s_replication_status.json", node))
- c.captureCommandOutput(ctx,
+ if _, err := c.captureCommandOutput(ctx,
commandSpec("pvesh", "get", fmt.Sprintf("/nodes/%s/replication", node), "--output-format=json"),
outputPath,
fmt.Sprintf("%s replication status", node),
- false)
+ false); err != nil && firstErr == nil {
+ firstErr = err
+ }
}
- return nil
+ return firstErr
}
func (c *Collector) resolvePVEStorages(storages []pveStorageEntry) []pveStorageEntry {
diff --git a/internal/backup/collector_pve_capture_errors_test.go b/internal/backup/collector_pve_capture_errors_test.go
new file mode 100644
index 00000000..4626b4b1
--- /dev/null
+++ b/internal/backup/collector_pve_capture_errors_test.go
@@ -0,0 +1,106 @@
+package backup
+
+import (
+ "context"
+ "os"
+ "path/filepath"
+ "strings"
+ "testing"
+)
+
+func newPVECollectorWithSuccessfulCommands(t *testing.T, calls *[]string) *Collector {
+ t.Helper()
+ return newPVECollectorWithDeps(t, CollectorDeps{
+ LookPath: func(name string) (string, error) {
+ return "/bin/true", nil
+ },
+ RunCommand: func(ctx context.Context, name string, args ...string) ([]byte, error) {
+ *calls = append(*calls, commandSpec(name, args...).String())
+ return []byte("[]"), nil
+ },
+ })
+}
+
+func TestCollectPVEBackupJobHistoryReturnsFirstCaptureError(t *testing.T) {
+ var calls []string
+ collector := newPVECollectorWithSuccessfulCommands(t, &calls)
+
+ jobsDir := collector.pveJobsDir()
+ if err := os.MkdirAll(filepath.Join(jobsDir, "node-a_backup_history.json"), 0o755); err != nil {
+ t.Fatalf("create output collision: %v", err)
+ }
+
+ err := collector.collectPVEBackupJobHistory(context.Background(), []string{"node-a", "node-b"})
+ if err == nil {
+ t.Fatalf("expected capture error")
+ }
+ if !strings.Contains(err.Error(), "failed to write report") {
+ t.Fatalf("unexpected error: %v", err)
+ }
+ if len(calls) != 2 {
+ t.Fatalf("expected collection to continue after first capture error, calls=%#v", calls)
+ }
+}
+
+func TestCollectPVEReplicationStatusReturnsFirstCaptureError(t *testing.T) {
+ var calls []string
+ collector := newPVECollectorWithSuccessfulCommands(t, &calls)
+
+ repDir := collector.pveReplicationDir()
+ if err := os.MkdirAll(filepath.Join(repDir, "node-a_replication_status.json"), 0o755); err != nil {
+ t.Fatalf("create output collision: %v", err)
+ }
+
+ err := collector.collectPVEReplicationStatus(context.Background(), []string{"node-a", "node-b"})
+ if err == nil {
+ t.Fatalf("expected capture error")
+ }
+ if !strings.Contains(err.Error(), "failed to write report") {
+ t.Fatalf("unexpected error: %v", err)
+ }
+ if len(calls) != 2 {
+ t.Fatalf("expected collection to continue after first capture error, calls=%#v", calls)
+ }
+}
+
+func TestCollectPVEScheduleCrontabReturnsCaptureError(t *testing.T) {
+ var calls []string
+ collector := newPVECollectorWithSuccessfulCommands(t, &calls)
+
+ schedulesDir := collector.pveSchedulesDir()
+ if err := os.MkdirAll(filepath.Join(schedulesDir, "root_crontab.txt"), 0o755); err != nil {
+ t.Fatalf("create output collision: %v", err)
+ }
+
+ err := collector.collectPVEScheduleCrontab(context.Background())
+ if err == nil {
+ t.Fatalf("expected capture error")
+ }
+ if !strings.Contains(err.Error(), "collectPVEScheduleCrontab:") {
+ t.Fatalf("expected function context in error, got %v", err)
+ }
+ if len(calls) != 1 {
+ t.Fatalf("expected one command call, calls=%#v", calls)
+ }
+}
+
+func TestCollectPVEScheduleTimersReturnsCaptureError(t *testing.T) {
+ var calls []string
+ collector := newPVECollectorWithSuccessfulCommands(t, &calls)
+
+ schedulesDir := collector.pveSchedulesDir()
+ if err := os.MkdirAll(filepath.Join(schedulesDir, "systemd_timers.txt"), 0o755); err != nil {
+ t.Fatalf("create output collision: %v", err)
+ }
+
+ err := collector.collectPVEScheduleTimers(context.Background())
+ if err == nil {
+ t.Fatalf("expected capture error")
+ }
+ if !strings.Contains(err.Error(), "collectPVEScheduleTimers:") {
+ t.Fatalf("expected function context in error, got %v", err)
+ }
+ if len(calls) != 1 {
+ t.Fatalf("expected one command call, calls=%#v", calls)
+ }
+}
diff --git a/internal/notify/email.go b/internal/notify/email.go
index 6a6ccdac..cefe2380 100644
--- a/internal/notify/email.go
+++ b/internal/notify/email.go
@@ -684,6 +684,17 @@ func commandForMailTool(ctx context.Context, pathOrName string, args ...string)
return safeexec.CommandContext(ctx, pathOrName, args...)
}
+func lookupAbsolutePath(name string) (string, error) {
+ execPath, err := exec.LookPath(name)
+ if err != nil {
+ return "", err
+ }
+ if filepath.IsAbs(execPath) {
+ return execPath, nil
+ }
+ return filepath.Abs(execPath)
+}
+
// sendViaRelay sends email via cloud relay
func (e *EmailNotifier) sendViaRelay(ctx context.Context, recipient, subject, htmlBody, textBody string, data *NotificationData) error {
// Build payload
@@ -705,12 +716,13 @@ func (e *EmailNotifier) sendViaRelay(ctx context.Context, recipient, subject, ht
func (e *EmailNotifier) isMTAServiceActive(ctx context.Context) (bool, string) {
services := []string{"postfix", "sendmail", "exim4"}
- if _, err := exec.LookPath("systemctl"); err != nil {
+ systemctlPath, err := lookupAbsolutePath("systemctl")
+ if err != nil {
return false, "systemctl not available"
}
for _, service := range services {
- cmd, err := safeexec.CommandContext(ctx, "systemctl", "is-active", service)
+ cmd, err := safeexec.TrustedCommandContext(ctx, systemctlPath, "is-active", service)
if err != nil {
return false, err.Error()
}
@@ -775,13 +787,12 @@ func (e *EmailNotifier) checkRelayHostConfigured(ctx context.Context) (bool, str
// checkMailQueue checks the mail queue status
func (e *EmailNotifier) checkMailQueue(ctx context.Context) (int, error) {
// Try mailq command (works for both Postfix and Sendmail)
- mailqPath := "/usr/bin/mailq"
- if _, err := exec.LookPath("mailq"); err != nil {
- if _, err := exec.LookPath(mailqPath); err != nil {
+ mailqPath, err := lookupAbsolutePath("mailq")
+ if err != nil {
+ mailqPath, err = lookupAbsolutePath("/usr/bin/mailq")
+ if err != nil {
return 0, fmt.Errorf("mailq command not found")
}
- } else {
- mailqPath = "mailq"
}
cmd, err := commandForMailTool(ctx, mailqPath)
@@ -822,11 +833,12 @@ func (e *EmailNotifier) checkMailQueue(ctx context.Context) (int, error) {
// detectQueueEntry scans the mail queue for a recipient and returns the latest queue ID.
func (e *EmailNotifier) detectQueueEntry(ctx context.Context, recipient string) (string, string, error) {
- mailqPath := "/usr/bin/mailq"
- if _, err := exec.LookPath("mailq"); err == nil {
- mailqPath = "mailq"
- } else if _, err := exec.LookPath(mailqPath); err != nil {
- return "", "", fmt.Errorf("mailq command not found")
+ mailqPath, err := lookupAbsolutePath("mailq")
+ if err != nil {
+ mailqPath, err = lookupAbsolutePath("/usr/bin/mailq")
+ if err != nil {
+ return "", "", fmt.Errorf("mailq command not found")
+ }
}
cmd, err := commandForMailTool(ctx, mailqPath)
diff --git a/internal/orchestrator/restore_workflow_abort_test.go b/internal/orchestrator/restore_workflow_abort_test.go
index 032330a2..27215e07 100644
--- a/internal/orchestrator/restore_workflow_abort_test.go
+++ b/internal/orchestrator/restore_workflow_abort_test.go
@@ -14,6 +14,18 @@ import (
"github.com/tis24dev/proxsave/internal/types"
)
+func TestRunRestoreWorkflowWithUIClearsStaleAbortInfoBeforeValidation(t *testing.T) {
+ lastRestoreAbortInfo = &RestoreAbortInfo{NetworkRollbackArmed: true}
+ t.Cleanup(ClearRestoreAbortInfo)
+
+ if err := runRestoreWorkflowWithUI(context.Background(), nil, nil, "vtest", nil); err == nil {
+ t.Fatalf("expected configuration error")
+ }
+ if got := GetLastRestoreAbortInfo(); got != nil {
+ t.Fatalf("expected stale abort info to be cleared, got %#v", got)
+ }
+}
+
func TestRunRestoreWorkflow_FstabPromptInputAborted_AbortsWorkflow(t *testing.T) {
origRestoreFS := restoreFS
origRestoreCmd := restoreCmd
diff --git a/internal/orchestrator/restore_workflow_ui.go b/internal/orchestrator/restore_workflow_ui.go
index eb5a96f4..2ed294be 100644
--- a/internal/orchestrator/restore_workflow_ui.go
+++ b/internal/orchestrator/restore_workflow_ui.go
@@ -47,6 +47,7 @@ func prepareRestoreBundleWithUI(ctx context.Context, cfg *config.Config, logger
}
func runRestoreWorkflowWithUI(ctx context.Context, cfg *config.Config, logger *logging.Logger, version string, ui RestoreWorkflowUI) (err error) {
+ ClearRestoreAbortInfo()
if cfg == nil {
return fmt.Errorf("configuration not available")
}
From 28071ad6bb3ddb1151daa22d2bd580b00d0b07ba Mon Sep 17 00:00:00 2001
From: Damiano <71268257+tis24dev@users.noreply.github.com>
Date: Tue, 5 May 2026 15:05:02 +0200
Subject: [PATCH 18/35] Strip build metadata in version compare; doc fixes
Ignore build metadata when comparing semantic versions by trimming any '+' suffix in isNewerVersion, and add unit tests covering build-metadata cases. Also update related docs and metadata: fix Codacy instructions YAML frontmatter formatting, update the PayPal donate link in README, and rename a config key in EXAMPLES.md (BACKUP_PXAR_FILES -> PXAR_SCAN_ENABLE).
---
.github/instructions/codacy.instructions.md | 7 +++----
README.md | 2 +-
cmd/proxsave/main_update.go | 5 ++++-
cmd/proxsave/version_helpers_test.go | 2 ++
docs/EXAMPLES.md | 2 +-
5 files changed, 11 insertions(+), 7 deletions(-)
diff --git a/.github/instructions/codacy.instructions.md b/.github/instructions/codacy.instructions.md
index cb073c46..d42429a0 100644
--- a/.github/instructions/codacy.instructions.md
+++ b/.github/instructions/codacy.instructions.md
@@ -1,7 +1,6 @@
---
- description: Configuration for AI behavior when interacting with Codacy's MCP Server
- applyTo: '**'
----
+description: Configuration for AI behavior when interacting with Codacy's MCP Server
+applyTo: '**'
---
# Codacy Rules
Configuration for AI behavior when interacting with Codacy's MCP Server
@@ -69,4 +68,4 @@ Configuration for AI behavior when interacting with Codacy's MCP Server
- If the user accepts, run the `codacy_setup_repository` tool
- Do not ever try to run the `codacy_setup_repository` tool on your own
- After setup, immediately retry the action that failed (only retry once)
----
\ No newline at end of file
+---
diff --git a/README.md b/README.md
index dbc0e391..1823c9fd 100644
--- a/README.md
+++ b/README.md
@@ -14,7 +14,7 @@ Proxmox PBS & PVE System Files Backup
[](https://rclone.org/)
[](https://github.com/sponsors/tis24dev)
[](https://github.com/sponsors/tis24dev)
-[](https://www.paypal.com/donate/?hosted_button_id=&cmd=_donations&business=damigioanna%40gmail.com)
+[](https://www.paypal.com/cgi-bin/webscr?cmd=_donations&business=damigioanna%40gmail.com)
## About the Project
diff --git a/cmd/proxsave/main_update.go b/cmd/proxsave/main_update.go
index 759c6105..6ce50983 100644
--- a/cmd/proxsave/main_update.go
+++ b/cmd/proxsave/main_update.go
@@ -81,7 +81,7 @@ func checkForUpdates(ctx context.Context, logger *logging.Logger, currentVersion
}
// isNewerVersion returns true if latest is strictly newer than current,
-// comparing MAJOR.MINOR.PATCH (ignoring any leading 'v' and pre-release suffixes).
+// comparing MAJOR.MINOR.PATCH (ignoring any leading 'v', pre-release suffixes, and build metadata).
func isNewerVersion(current, latest string) bool {
parse := func(v string) (int, int, int) {
v = strings.TrimSpace(v)
@@ -89,6 +89,9 @@ func isNewerVersion(current, latest string) bool {
if i := strings.IndexByte(v, '-'); i >= 0 {
v = v[:i]
}
+ if i := strings.IndexByte(v, '+'); i >= 0 {
+ v = v[:i]
+ }
parts := strings.Split(v, ".")
toInt := func(s string) int {
diff --git a/cmd/proxsave/version_helpers_test.go b/cmd/proxsave/version_helpers_test.go
index 2b14c64e..bfa0890f 100644
--- a/cmd/proxsave/version_helpers_test.go
+++ b/cmd/proxsave/version_helpers_test.go
@@ -43,6 +43,8 @@ func TestIsNewerVersion(t *testing.T) {
{"major newer", "1.9.9", "2.0.0", true},
{"strip leading v", "v1.2.3", "1.2.4", true},
{"ignore prerelease", "1.2.3-rc1", "1.2.3", false},
+ {"ignore build metadata", "v1.2.3+current", "v1.2.4+latest", true},
+ {"build metadata does not zero patch", "v1.2.3+current", "v1.2.3+latest", false},
{"missing patch treated as 0", "1.2", "1.2.0", false},
}
diff --git a/docs/EXAMPLES.md b/docs/EXAMPLES.md
index 9aa85865..61b2bdae 100644
--- a/docs/EXAMPLES.md
+++ b/docs/EXAMPLES.md
@@ -940,7 +940,7 @@ BACKUP_PBS_NOTIFICATIONS=true
BACKUP_PBS_NODE_CONFIG=true
# Recommended for dual labs: keep diagnostics
-BACKUP_PXAR_FILES=true
+PXAR_SCAN_ENABLE=true
```
### Expected Behavior
From 140222ec460c76ea8dc561081c17e710fb8dad47 Mon Sep 17 00:00:00 2001
From: Damiano <71268257+tis24dev@users.noreply.github.com>
Date: Tue, 5 May 2026 15:17:27 +0200
Subject: [PATCH 19/35] Improve restore extraction summary and guards
Report failed files in restore summary and refine logging/guards. The restore summary now prints the number of failed files and includes them in the total archive count; summary log messages omit the "see detailed log" hint when no log path is configured. shouldSkipRestoreEntryTarget was tightened to match the /etc/pve boundary (exact match or trailing slash) to avoid false positives. Also changed a PBS user-list parse log from Debug to Warning and adjusted a couple of tests to account for whitespace-only config path handling. Added tests covering the restore summary and /etc/pve boundary behavior.
---
internal/backup/collector_bricks_pbs.go | 2 +-
.../backup/collector_pve_patterns_test.go | 2 +-
internal/backup/collector_pve_util_test.go | 2 +-
.../orchestrator/restore_archive_entries.go | 2 +-
.../orchestrator/restore_archive_extract.go | 15 ++++-
.../restore_archive_extract_summary_test.go | 65 +++++++++++++++++++
internal/orchestrator/restore_test.go | 23 +++++++
7 files changed, 104 insertions(+), 7 deletions(-)
create mode 100644 internal/orchestrator/restore_archive_extract_summary_test.go
diff --git a/internal/backup/collector_bricks_pbs.go b/internal/backup/collector_bricks_pbs.go
index 098f14bc..87a96d3f 100644
--- a/internal/backup/collector_bricks_pbs.go
+++ b/internal/backup/collector_bricks_pbs.go
@@ -115,7 +115,7 @@ func newPBSUserConfigRecipe() recipe {
state.pbs.userIDs = nil
return nil
}
- state.collector.logger.Debug("Failed to parse user list for token export: %v", err)
+ state.collector.logger.Warning("Failed to parse user list for token export: %v", err)
state.pbs.userIDs = nil
return nil
}
diff --git a/internal/backup/collector_pve_patterns_test.go b/internal/backup/collector_pve_patterns_test.go
index 9876f8f0..da7e6f84 100644
--- a/internal/backup/collector_pve_patterns_test.go
+++ b/internal/backup/collector_pve_patterns_test.go
@@ -208,7 +208,7 @@ func TestEffectivePVEClusterPath(t *testing.T) {
},
{
name: "whitespace only uses default",
- configPath: "",
+ configPath: " ",
expected: "/var/lib/pve-cluster",
},
{
diff --git a/internal/backup/collector_pve_util_test.go b/internal/backup/collector_pve_util_test.go
index 7dccf8ae..b6451873 100644
--- a/internal/backup/collector_pve_util_test.go
+++ b/internal/backup/collector_pve_util_test.go
@@ -1122,7 +1122,7 @@ func TestEffectivePVEConfigPathDetailed(t *testing.T) {
},
{
name: "whitespace only uses default",
- configPath: "",
+ configPath: " ",
expected: "/etc/pve",
},
{
diff --git a/internal/orchestrator/restore_archive_entries.go b/internal/orchestrator/restore_archive_entries.go
index 487dde72..0c9a9744 100644
--- a/internal/orchestrator/restore_archive_entries.go
+++ b/internal/orchestrator/restore_archive_entries.go
@@ -44,7 +44,7 @@ func shouldSkipRestoreEntryTarget(header *tar.Header, target, cleanDestRoot stri
return false, nil
}
// Hard guard: never write directly into /etc/pve when restoring to system root
- if strings.HasPrefix(target, "/etc/pve") {
+ if target == "/etc/pve" || strings.HasPrefix(target, "/etc/pve/") {
logger.Warning("Skipping restore to %s (writes to /etc/pve are prohibited)", target)
return true, nil
}
diff --git a/internal/orchestrator/restore_archive_extract.go b/internal/orchestrator/restore_archive_extract.go
index f8964cee..521073ca 100644
--- a/internal/orchestrator/restore_archive_extract.go
+++ b/internal/orchestrator/restore_archive_extract.go
@@ -206,7 +206,8 @@ func (log *restoreExtractionLog) writeSummary(stats restoreExtractionStats) {
fmt.Fprintf(log.logFile, "=== SUMMARY ===\n")
fmt.Fprintf(log.logFile, "Total files extracted: %d\n", stats.filesExtracted)
fmt.Fprintf(log.logFile, "Total files skipped: %d\n", stats.filesSkipped)
- fmt.Fprintf(log.logFile, "Total files in archive: %d\n", stats.filesExtracted+stats.filesSkipped)
+ fmt.Fprintf(log.logFile, "Total files failed: %d\n", stats.filesFailed)
+ fmt.Fprintf(log.logFile, "Total files in archive: %d\n", stats.filesExtracted+stats.filesSkipped+stats.filesFailed)
}
func (log *restoreExtractionLog) copyTempEntries(tempFile *os.File, label string) {
@@ -228,11 +229,19 @@ func logRestoreExtractionSummary(opts restoreArchiveOptions, stats restoreExtrac
opts.logger.Info("Successfully restored all %d files/directories", stats.filesExtracted)
}
} else {
- opts.logger.Warning("Restored %d files/directories; %d item(s) failed (see detailed log)", stats.filesExtracted, stats.filesFailed)
+ if opts.logFilePath != "" {
+ opts.logger.Warning("Restored %d files/directories; %d item(s) failed (see detailed log)", stats.filesExtracted, stats.filesFailed)
+ } else {
+ opts.logger.Warning("Restored %d files/directories; %d item(s) failed", stats.filesExtracted, stats.filesFailed)
+ }
}
if stats.filesSkipped > 0 {
- opts.logger.Info("%d additional archive entries (logs, diagnostics, system defaults) were left unchanged on this system; see detailed log for details", stats.filesSkipped)
+ if opts.logFilePath != "" {
+ opts.logger.Info("%d additional archive entries (logs, diagnostics, system defaults) were left unchanged on this system; see detailed log for details", stats.filesSkipped)
+ } else {
+ opts.logger.Info("%d additional archive entries (logs, diagnostics, system defaults) were left unchanged on this system", stats.filesSkipped)
+ }
}
if opts.logFilePath != "" {
diff --git a/internal/orchestrator/restore_archive_extract_summary_test.go b/internal/orchestrator/restore_archive_extract_summary_test.go
new file mode 100644
index 00000000..5c1caaf6
--- /dev/null
+++ b/internal/orchestrator/restore_archive_extract_summary_test.go
@@ -0,0 +1,65 @@
+package orchestrator
+
+import (
+ "bytes"
+ "os"
+ "path/filepath"
+ "strings"
+ "testing"
+
+ "github.com/tis24dev/proxsave/internal/logging"
+ "github.com/tis24dev/proxsave/internal/types"
+)
+
+func TestRestoreExtractionLogWriteSummaryIncludesFailedFiles(t *testing.T) {
+ logPath := filepath.Join(t.TempDir(), "restore.log")
+ logFile, err := os.Create(logPath)
+ if err != nil {
+ t.Fatalf("create log file: %v", err)
+ }
+
+ extractionLog := &restoreExtractionLog{
+ logger: newTestLogger(),
+ logFile: logFile,
+ }
+ extractionLog.writeSummary(restoreExtractionStats{
+ filesExtracted: 2,
+ filesSkipped: 3,
+ filesFailed: 4,
+ })
+ if err := logFile.Close(); err != nil {
+ t.Fatalf("close log file: %v", err)
+ }
+
+ content, err := os.ReadFile(logPath)
+ if err != nil {
+ t.Fatalf("read log file: %v", err)
+ }
+ text := string(content)
+ if !strings.Contains(text, "Total files failed: 4") {
+ t.Fatalf("summary missing failed count:\n%s", text)
+ }
+ if !strings.Contains(text, "Total files in archive: 9") {
+ t.Fatalf("summary total should include failed files:\n%s", text)
+ }
+}
+
+func TestLogRestoreExtractionSummaryOmitsDetailedLogHintWithoutLogPath(t *testing.T) {
+ var buf bytes.Buffer
+ logger := logging.New(types.LogLevelInfo, false)
+ logger.SetOutput(&buf)
+
+ logRestoreExtractionSummary(restoreArchiveOptions{logger: logger}, restoreExtractionStats{
+ filesExtracted: 2,
+ filesSkipped: 1,
+ filesFailed: 1,
+ })
+
+ output := buf.String()
+ if strings.Contains(output, "see detailed log") {
+ t.Fatalf("did not expect detailed log hint without log path:\n%s", output)
+ }
+ if !strings.Contains(output, "1 item(s) failed") {
+ t.Fatalf("expected failed count in summary:\n%s", output)
+ }
+}
diff --git a/internal/orchestrator/restore_test.go b/internal/orchestrator/restore_test.go
index 9dcdfc00..36d2387b 100644
--- a/internal/orchestrator/restore_test.go
+++ b/internal/orchestrator/restore_test.go
@@ -63,6 +63,29 @@ func TestExtractTarEntry_BlocksPathTraversal(t *testing.T) {
}
}
+func TestShouldSkipRestoreEntryTargetEtcPVEBoundary(t *testing.T) {
+ logger := logging.New(types.LogLevelDebug, false)
+ header := &tar.Header{Name: "etc/pveuser.conf"}
+
+ for _, target := range []string{"/etc/pve", "/etc/pve/local.cfg"} {
+ skip, err := shouldSkipRestoreEntryTarget(header, target, string(os.PathSeparator), logger)
+ if err != nil {
+ t.Fatalf("shouldSkipRestoreEntryTarget(%q) error: %v", target, err)
+ }
+ if !skip {
+ t.Fatalf("expected %q to be skipped", target)
+ }
+ }
+
+ skip, err := shouldSkipRestoreEntryTarget(header, "/etc/pveuser.conf", string(os.PathSeparator), logger)
+ if err != nil {
+ t.Fatalf("shouldSkipRestoreEntryTarget false-positive path error: %v", err)
+ }
+ if skip {
+ t.Fatalf("did not expect /etc/pveuser.conf to match /etc/pve guard")
+ }
+}
+
func TestExtractPlainArchive_WithFakeFS_RestoresFiles(t *testing.T) {
origRestoreFS := restoreFS
fakeFS := NewFakeFS()
From 2529ebd172abfec558c0f8ee0840606ddcfc1f76 Mon Sep 17 00:00:00 2001
From: Damiano <71268257+tis24dev@users.noreply.github.com>
Date: Tue, 5 May 2026 15:57:03 +0200
Subject: [PATCH 20/35] Add FS utimes/lchown and refactor command handling
Add Lchown and UtimesNano to the FS interface and implement them in osFS and test fakes; update restore code to use restoreFS.Lchown/UtimesNano instead of direct os/syscall calls. Simplify environment runCommand to use safeexec.TrustedCommandContext and update tests to call echo via LookPath. Refactor Proxmox notification header parsing: introduce parseSectionHeader with a regexp-based validation and keep parseProxmoxNotificationHeader as a wrapper; update callers. Move panic handling in main lifecycle into a deferred function and remove an extra success print in config apply. Also include minor docs/formatting tweaks and add a test simulating execCommand failure in namespaces tests.
---
.github/instructions/codacy.instructions.md | 40 +++++++++----------
cmd/proxsave/main_config_modes.go | 1 -
cmd/proxsave/main_lifecycle.go | 18 +++++----
docs/RESTORE_GUIDE.md | 2 +-
internal/environment/detect.go | 13 +-----
.../environment/detect_additional_test.go | 7 +++-
internal/orchestrator/deps.go | 7 ++++
internal/orchestrator/deps_test.go | 9 +++++
.../orchestrator/restore_archive_entries.go | 4 +-
.../orchestrator/restore_cluster_apply.go | 2 +-
internal/orchestrator/restore_errors_test.go | 7 ++++
.../orchestrator/restore_notifications.go | 20 +++++-----
internal/pbs/namespaces_test.go | 17 ++++++++
13 files changed, 92 insertions(+), 55 deletions(-)
diff --git a/.github/instructions/codacy.instructions.md b/.github/instructions/codacy.instructions.md
index d42429a0..9e27a080 100644
--- a/.github/instructions/codacy.instructions.md
+++ b/.github/instructions/codacy.instructions.md
@@ -7,9 +7,9 @@ Configuration for AI behavior when interacting with Codacy's MCP Server
## CRITICAL: After ANY successful `edit_file` or `reapply` operation
- YOU MUST IMMEDIATELY run the `codacy_cli_analyze` tool from Codacy's MCP Server for each file that was edited, with:
- - `rootPath`: set to the workspace path
- - `file`: set to the path of the edited file
- - `tool`: leave empty or unset
+ - `rootPath`: set to the workspace path
+ - `file`: set to the path of the edited file
+ - `tool`: leave empty or unset
- If any issues are found in the new edits, propose and apply fixes for them.
- > NOTE: Failure to follow this rule is considered a critical error.
@@ -24,8 +24,8 @@ Configuration for AI behavior when interacting with Codacy's MCP Server
## When there are no Codacy MCP Server tools available, or the MCP Server is not reachable
- Suggest the user the following troubleshooting steps:
- - Try to reset the MCP on the extension
- - If the user is using VSCode, suggest them to review their Copilot > MCP settings in Github, under their organization or personal account. Refer them to Settings > Copilot > Enable MCP servers in Copilot. Suggested URL (https://github.com/settings/copilot/features) or https://github.com/organizations/{organization-name}/settings/copilot/features (This can only be done by their organization admins / owners)
+ - Try to reset the MCP on the extension
+ - If the user is using VSCode, suggest them to review their Copilot > MCP settings in Github, under their organization or personal account. Refer them to Settings > Copilot > Enable MCP servers in Copilot. Suggested URL (https://github.com/settings/copilot/features) or https://github.com/organizations/{organization-name}/settings/copilot/features (This can only be done by their organization admins / owners)
- If none of the above steps work, suggest the user to contact Codacy support
## Trying to call a tool that needs a rootPath as a parameter
@@ -33,24 +33,24 @@ Configuration for AI behavior when interacting with Codacy's MCP Server
## CRITICAL: Dependencies and Security Checks
- IMMEDIATELY after ANY of these actions:
- - Running npm/yarn/pnpm install
- - Adding dependencies to package.json
- - Adding requirements to requirements.txt
- - Adding dependencies to pom.xml
- - Adding dependencies to build.gradle
- - Any other package manager operations
+ - Running npm/yarn/pnpm install
+ - Adding dependencies to package.json
+ - Adding requirements to requirements.txt
+ - Adding dependencies to pom.xml
+ - Adding dependencies to build.gradle
+ - Any other package manager operations
- You MUST run the `codacy_cli_analyze` tool with:
- - `rootPath`: set to the workspace path
- - `tool`: set to "trivy"
- - `file`: leave empty or unset
+ - `rootPath`: set to the workspace path
+ - `tool`: set to "trivy"
+ - `file`: leave empty or unset
- If any vulnerabilities are found because of the newly added packages:
- - Stop all other operations
- - Propose and apply fixes for the security issues
- - Only continue with the original task after security issues are resolved
+ - Stop all other operations
+ - Propose and apply fixes for the security issues
+ - Only continue with the original task after security issues are resolved
- EXAMPLE:
- - After: npm install react-markdown
- - Do: Run codacy_cli_analyze with trivy
- - Before: Continuing with any other tasks
+ - After: npm install react-markdown
+ - Do: Run codacy_cli_analyze with trivy
+ - Before: Continuing with any other tasks
## General
- Repeat the relevant steps for each modified file.
diff --git a/cmd/proxsave/main_config_modes.go b/cmd/proxsave/main_config_modes.go
index e47179ee..3ee7383c 100644
--- a/cmd/proxsave/main_config_modes.go
+++ b/cmd/proxsave/main_config_modes.go
@@ -153,7 +153,6 @@ func printConfigUpgradeDryRunResult(bootstrap *logging.BootstrapLogger, result *
}
func printConfigUpgradeApplyResult(bootstrap *logging.BootstrapLogger, result *config.UpgradeResult) {
- bootstrap.Println("Configuration upgraded successfully!")
if len(result.MissingKeys) > 0 {
bootstrap.Printf("- Added %d missing key(s): %s",
len(result.MissingKeys), strings.Join(result.MissingKeys, ", "))
diff --git a/cmd/proxsave/main_lifecycle.go b/cmd/proxsave/main_lifecycle.go
index 54d82fae..449d2b63 100644
--- a/cmd/proxsave/main_lifecycle.go
+++ b/cmd/proxsave/main_lifecycle.go
@@ -32,14 +32,16 @@ func startMainRun() runBootstrap {
}
func finishMainRun(run runBootstrap) {
- logging.DebugStepBootstrap(run.bootstrap, "main run", "exit_code=%d", run.state.finalExitCode)
- run.runDone(nil)
- if r := recover(); r != nil {
- stack := debug.Stack()
- run.bootstrap.Error("PANIC: %v", r)
- fmt.Fprintf(os.Stderr, "panic: %v\n%s\n", r, stack)
- os.Exit(types.ExitPanicError.Int())
- }
+ defer func() {
+ if r := recover(); r != nil {
+ stack := debug.Stack()
+ run.bootstrap.Error("PANIC: %v", r)
+ fmt.Fprintf(os.Stderr, "panic: %v\n%s\n", r, stack)
+ os.Exit(types.ExitPanicError.Int())
+ }
+ logging.DebugStepBootstrap(run.bootstrap, "main run", "exit_code=%d", run.state.finalExitCode)
+ run.runDone(nil)
+ }()
}
func preparePreRuntimeArgs(ctx context.Context, bootstrap *logging.BootstrapLogger, toolVersion string) (*cli.Args, int, bool) {
diff --git a/docs/RESTORE_GUIDE.md b/docs/RESTORE_GUIDE.md
index 6c11f31b..05bc7172 100644
--- a/docs/RESTORE_GUIDE.md
+++ b/docs/RESTORE_GUIDE.md
@@ -510,7 +510,7 @@ Backup system type: Proxmox Virtual Environment (PVE)
```
**Partial-compatibility warning**:
-```
+```text
⚠ WARNING: Partial compatibility detected
Current system: Proxmox Virtual Environment (PVE)
diff --git a/internal/environment/detect.go b/internal/environment/detect.go
index 49ef60b9..a6fd841d 100644
--- a/internal/environment/detect.go
+++ b/internal/environment/detect.go
@@ -47,8 +47,7 @@ var (
"/etc/apt/sources.list.d/proxmox.list",
}
- lookPathFunc = exec.LookPath
- commandContextFunc = safeexec.TrustedCommandContext
+ lookPathFunc = exec.LookPath
readFileFunc = os.ReadFile
statFunc = os.Stat
@@ -342,15 +341,7 @@ func runCommand(command string, args ...string) (string, error) {
ctx, cancel := context.WithTimeout(context.Background(), commandTimeout)
defer cancel()
- var (
- cmd *exec.Cmd
- cmdErr error
- )
- if filepath.IsAbs(command) {
- cmd, cmdErr = commandContextFunc(ctx, command, args...)
- } else {
- cmd, cmdErr = safeexec.CommandContext(ctx, command, args...)
- }
+ cmd, cmdErr := safeexec.TrustedCommandContext(ctx, command, args...)
if cmdErr != nil {
return "", cmdErr
}
diff --git a/internal/environment/detect_additional_test.go b/internal/environment/detect_additional_test.go
index 11e5c583..d8d347c9 100644
--- a/internal/environment/detect_additional_test.go
+++ b/internal/environment/detect_additional_test.go
@@ -3,6 +3,7 @@ package environment
import (
"context"
"os"
+ "os/exec"
"path/filepath"
"strings"
"testing"
@@ -151,7 +152,11 @@ func TestContainsAny(t *testing.T) {
// TestRunCommand tests command execution with timeout
func TestRunCommand(t *testing.T) {
// Test successful command
- output, err := runCommand("echo", "test")
+ echoPath, err := exec.LookPath("echo")
+ if err != nil {
+ t.Fatalf("LookPath(echo) failed: %v", err)
+ }
+ output, err := runCommand(echoPath, "test")
if err != nil {
t.Errorf("runCommand() error = %v", err)
}
diff --git a/internal/orchestrator/deps.go b/internal/orchestrator/deps.go
index 3e7d1f56..9a5099a4 100644
--- a/internal/orchestrator/deps.go
+++ b/internal/orchestrator/deps.go
@@ -7,6 +7,7 @@ import (
"io/fs"
"os"
"os/exec"
+ "syscall"
"time"
"github.com/tis24dev/proxsave/internal/config"
@@ -33,6 +34,8 @@ type FS interface {
CreateTemp(dir, pattern string) (*os.File, error)
MkdirTemp(dir, pattern string) (string, error)
Rename(oldpath, newpath string) error
+ Lchown(path string, uid, gid int) error
+ UtimesNano(path string, times []syscall.Timespec) error
}
// Prompter encapsulates interactive prompts.
@@ -94,6 +97,10 @@ func (osFS) CreateTemp(dir, pattern string) (*os.File, error) {
}
func (osFS) MkdirTemp(dir, pattern string) (string, error) { return os.MkdirTemp(dir, pattern) }
func (osFS) Rename(oldpath, newpath string) error { return os.Rename(oldpath, newpath) }
+func (osFS) Lchown(path string, uid, gid int) error { return os.Lchown(path, uid, gid) }
+func (osFS) UtimesNano(path string, times []syscall.Timespec) error {
+ return syscall.UtimesNano(path, times)
+}
type consolePrompter struct{}
diff --git a/internal/orchestrator/deps_test.go b/internal/orchestrator/deps_test.go
index 076220fe..dea9b1e3 100644
--- a/internal/orchestrator/deps_test.go
+++ b/internal/orchestrator/deps_test.go
@@ -7,6 +7,7 @@ import (
"os"
"path/filepath"
"strings"
+ "syscall"
"testing"
"time"
@@ -186,6 +187,14 @@ func (f *FakeFS) Rename(oldpath, newpath string) error {
return os.Rename(f.onDisk(oldpath), f.onDisk(newpath))
}
+func (f *FakeFS) Lchown(path string, uid, gid int) error {
+ return os.Lchown(f.onDisk(path), uid, gid)
+}
+
+func (f *FakeFS) UtimesNano(path string, times []syscall.Timespec) error {
+ return syscall.UtimesNano(f.onDisk(path), times)
+}
+
// FakeTime provides deterministic time.
type FakeTime struct {
Current time.Time
diff --git a/internal/orchestrator/restore_archive_entries.go b/internal/orchestrator/restore_archive_entries.go
index 0c9a9744..0411c84b 100644
--- a/internal/orchestrator/restore_archive_entries.go
+++ b/internal/orchestrator/restore_archive_entries.go
@@ -219,7 +219,7 @@ func extractSymlink(target string, header *tar.Header, destRoot string, logger *
}
// Set ownership (on the symlink itself, not the target)
- if err := os.Lchown(target, header.Uid, header.Gid); err != nil {
+ if err := restoreFS.Lchown(target, header.Uid, header.Gid); err != nil {
logger.Debug("Failed to lchown symlink %s: %v", target, err)
}
@@ -269,7 +269,7 @@ func setTimestamps(target string, header *tar.Header) error {
{Sec: mtime.Unix(), Nsec: int64(mtime.Nanosecond())},
}
- if err := syscall.UtimesNano(target, times); err != nil {
+ if err := restoreFS.UtimesNano(target, times); err != nil {
return fmt.Errorf("set atime/mtime: %w", err)
}
diff --git a/internal/orchestrator/restore_cluster_apply.go b/internal/orchestrator/restore_cluster_apply.go
index 3445ba09..227331b4 100644
--- a/internal/orchestrator/restore_cluster_apply.go
+++ b/internal/orchestrator/restore_cluster_apply.go
@@ -289,7 +289,7 @@ func parseStorageBlocks(cfgPath string) ([]storageBlock, error) {
// storage.cfg blocks use `: ` (e.g. `dir: local`, `nfs: backup`).
// Older exports may still use `storage: ` blocks.
- _, name, ok := parseProxmoxNotificationHeader(trimmed)
+ _, name, ok := parseSectionHeader(trimmed)
if ok {
flush()
current = &storageBlock{ID: name, data: []string{line}}
diff --git a/internal/orchestrator/restore_errors_test.go b/internal/orchestrator/restore_errors_test.go
index 2df7c3a7..c570ca30 100644
--- a/internal/orchestrator/restore_errors_test.go
+++ b/internal/orchestrator/restore_errors_test.go
@@ -11,6 +11,7 @@ import (
"os"
"path/filepath"
"strings"
+ "syscall"
"testing"
"time"
@@ -882,6 +883,12 @@ func (f *ErrorInjectingFS) MkdirTemp(dir, pattern string) (string, error) {
func (f *ErrorInjectingFS) Rename(oldpath, newpath string) error {
return f.base.Rename(oldpath, newpath)
}
+func (f *ErrorInjectingFS) Lchown(path string, uid, gid int) error {
+ return f.base.Lchown(path, uid, gid)
+}
+func (f *ErrorInjectingFS) UtimesNano(path string, times []syscall.Timespec) error {
+ return f.base.UtimesNano(path, times)
+}
func (f *ErrorInjectingFS) MkdirAll(path string, perm os.FileMode) error {
if f.mkdirAllErr != nil {
diff --git a/internal/orchestrator/restore_notifications.go b/internal/orchestrator/restore_notifications.go
index df841569..8c6305d7 100644
--- a/internal/orchestrator/restore_notifications.go
+++ b/internal/orchestrator/restore_notifications.go
@@ -6,6 +6,7 @@ import (
"fmt"
"os"
"path/filepath"
+ "regexp"
"strings"
"github.com/tis24dev/proxsave/internal/logging"
@@ -23,6 +24,8 @@ type proxmoxNotificationSection struct {
RedactFlags []string
}
+var sectionHeaderTypePattern = regexp.MustCompile(`^[A-Za-z0-9_-]+$`)
+
func maybeApplyNotificationsFromStage(ctx context.Context, logger *logging.Logger, plan *RestorePlan, stageRoot string, dryRun bool) (err error) {
if plan == nil {
return nil
@@ -401,7 +404,7 @@ func parseProxmoxNotificationSections(content string) ([]proxmoxNotificationSect
return out, nil
}
-func parseProxmoxNotificationHeader(line string) (typ, name string, ok bool) {
+func parseSectionHeader(line string) (typ, name string, ok bool) {
idx := strings.Index(line, ":")
if idx <= 0 {
return "", "", false
@@ -411,19 +414,16 @@ func parseProxmoxNotificationHeader(line string) (typ, name string, ok bool) {
if typ == "" || name == "" {
return "", "", false
}
- for _, r := range typ {
- switch {
- case r >= 'a' && r <= 'z':
- case r >= 'A' && r <= 'Z':
- case r >= '0' && r <= '9':
- case r == '-' || r == '_':
- default:
- return "", "", false
- }
+ if !sectionHeaderTypePattern.MatchString(typ) {
+ return "", "", false
}
return typ, name, true
}
+func parseProxmoxNotificationHeader(line string) (typ, name string, ok bool) {
+ return parseSectionHeader(line)
+}
+
func parseProxmoxNotificationKV(line string) (key, value string) {
fields := strings.Fields(line)
if len(fields) == 0 {
diff --git a/internal/pbs/namespaces_test.go b/internal/pbs/namespaces_test.go
index 2ff65c1a..22e7a473 100644
--- a/internal/pbs/namespaces_test.go
+++ b/internal/pbs/namespaces_test.go
@@ -3,6 +3,7 @@ package pbs
import (
"context"
"encoding/json"
+ "errors"
"fmt"
"os"
"os/exec"
@@ -192,6 +193,13 @@ func TestListNamespacesViaCLI_ErrorIncludesStderr(t *testing.T) {
}
}
+func TestListNamespacesViaCLI_ExecCommandError(t *testing.T) {
+ setExecCommandStub(t, "cmd-failure")
+ if _, err := listNamespacesViaCLI(context.Background(), "dummy"); err == nil || !strings.Contains(err.Error(), "simulated execCommand failure") {
+ t.Fatalf("expected execCommand error, got %v", err)
+ }
+}
+
func TestHelperProcess(t *testing.T) {
if os.Getenv("GO_WANT_HELPER_PROCESS") != "1" {
return
@@ -213,6 +221,15 @@ func TestHelperProcess(t *testing.T) {
func setExecCommandStub(t *testing.T, scenario string) {
t.Helper()
original := execCommand
+ if scenario == "cmd-failure" {
+ execCommand = func(context.Context, string, ...string) (*exec.Cmd, error) {
+ return nil, errors.New("simulated execCommand failure")
+ }
+ t.Cleanup(func() {
+ execCommand = original
+ })
+ return
+ }
execCommand = func(context.Context, string, ...string) (*exec.Cmd, error) {
cmd := exec.Command(os.Args[0], "-test.run=TestHelperProcess", "--")
cmd.Env = append(os.Environ(),
From 3048e3cf27265f5de9dcdda0801dcf9b085d79b1 Mon Sep 17 00:00:00 2001
From: Damiano <71268257+tis24dev@users.noreply.github.com>
Date: Tue, 5 May 2026 21:35:35 +0200
Subject: [PATCH 21/35] Refactor command execution and runtime handling
Centralize and harden command execution: introduce runAndClassifyCommand with commandRunOptions/result and classification enums; refactor safeCmdOutput and captureCommandOutput to reuse this logic, improving logging, output summarization, unprivileged-container handling, systemctl special cases, and dry-run/lookpath handling.
Runtime and signal updates: detect unprivileged container earlier in bootstrap (rt.unprivilegedInfo), remove duplicate detection in logging, stop signal notification on exit, and make stdin-close handling safe via a scoped sync.Once. Also fix heap profiling to defer file close.
Other changes: make pveContext methods use pointer receivers and nil-checks for safety; enforce Pushover webhook auth (require Token and User) and add tests; expand safeexec tests to validate the list of allowed commands.
---
cmd/proxsave/main_defers.go | 8 +-
cmd/proxsave/main_runtime.go | 1 +
cmd/proxsave/main_runtime_log.go | 2 -
cmd/proxsave/main_signals.go | 25 +-
internal/backup/collector.go | 270 +++++++++---------
.../backup/collector_bricks_pve_finalize.go | 12 +-
internal/notify/webhook.go | 10 +
internal/notify/webhook_test.go | 40 +++
internal/safeexec/safeexec_test.go | 19 +-
9 files changed, 228 insertions(+), 159 deletions(-)
diff --git a/cmd/proxsave/main_defers.go b/cmd/proxsave/main_defers.go
index 0ed07345..e138610a 100644
--- a/cmd/proxsave/main_defers.go
+++ b/cmd/proxsave/main_defers.go
@@ -14,6 +14,12 @@ import (
type runDeferredAction func()
func runDeferredActions(rt *appRuntime, state *appRunState) []runDeferredAction {
+ // runRuntime defers each returned action while iterating this slice, so these
+ // entries execute in reverse (LIFO) order. Keep the ordering intentional:
+ // dispatchDeferredEarlyErrorNotification must run before sendDeferredSupportEmail
+ // because it sets state.pendingSupportStat, which sendDeferredSupportEmail
+ // reads. Do not reorder these entries or change the defer pattern without
+ // preserving that dependency.
return []runDeferredAction{
func() {
if state.showSummary {
@@ -79,8 +85,8 @@ func closeRunProfiling(rt *appRuntime) {
logging.Warning("Failed to create heap profile file: %v", err)
return
}
+ defer f.Close()
if err := pprof.WriteHeapProfile(f); err != nil {
logging.Warning("Failed to write heap profile: %v", err)
}
- _ = f.Close()
}
diff --git a/cmd/proxsave/main_runtime.go b/cmd/proxsave/main_runtime.go
index a30818a4..90f4e648 100644
--- a/cmd/proxsave/main_runtime.go
+++ b/cmd/proxsave/main_runtime.go
@@ -78,6 +78,7 @@ func bootstrapRuntime(ctx context.Context, args *cli.Args, bootstrap *logging.Bo
rt.updateInfo = checkForUpdates(ctx, rt.logger, toolVersion)
applyRunPermissions(rt)
initializeRunProfiling(rt)
+ rt.unprivilegedInfo = environment.DetectUnprivilegedContainer()
return rt, types.ExitSuccess.Int(), true
}
diff --git a/cmd/proxsave/main_runtime_log.go b/cmd/proxsave/main_runtime_log.go
index df4513f1..2031a558 100644
--- a/cmd/proxsave/main_runtime_log.go
+++ b/cmd/proxsave/main_runtime_log.go
@@ -4,7 +4,6 @@ package main
import (
"strings"
- "github.com/tis24dev/proxsave/internal/environment"
"github.com/tis24dev/proxsave/internal/logging"
)
@@ -12,7 +11,6 @@ func logRunContext(rt *appRuntime) {
logRunDryRunStatus(rt)
baseDirSource := runBaseDirSource(rt)
logging.Info("Environment: %s %s", rt.envInfo.Type, rt.envInfo.Version)
- rt.unprivilegedInfo = environment.DetectUnprivilegedContainer()
logUserNamespaceContext(rt.logger, rt.unprivilegedInfo)
logging.Info("Backup enabled: %v", rt.cfg.BackupEnabled)
logging.Info("Debug level: %s", rt.logLevel.String())
diff --git a/cmd/proxsave/main_signals.go b/cmd/proxsave/main_signals.go
index d01d1532..1274ba91 100644
--- a/cmd/proxsave/main_signals.go
+++ b/cmd/proxsave/main_signals.go
@@ -12,24 +12,27 @@ import (
"github.com/tis24dev/proxsave/internal/tui"
)
-var closeStdinOnce sync.Once
-
func setupRunContext(bootstrap *logging.BootstrapLogger) (context.Context, context.CancelFunc) {
ctx, cancel := context.WithCancel(context.Background())
tui.SetAbortContext(ctx)
+ var closeStdinOnce sync.Once
sigChan := make(chan os.Signal, 1)
signal.Notify(sigChan, syscall.SIGINT, syscall.SIGTERM)
go func() {
- sig := <-sigChan
- logging.DebugStepBootstrap(bootstrap, "signal", "received=%v", sig)
- bootstrap.Info("\nReceived signal %v, initiating graceful shutdown...", sig)
- cancel()
- closeStdinOnce.Do(func() {
- if file := os.Stdin; file != nil {
- _ = file.Close()
- }
- })
+ defer signal.Stop(sigChan)
+ select {
+ case sig := <-sigChan:
+ logging.DebugStepBootstrap(bootstrap, "signal", "received=%v", sig)
+ bootstrap.Info("\nReceived signal %v, initiating graceful shutdown...", sig)
+ cancel()
+ closeStdinOnce.Do(func() {
+ if file := os.Stdin; file != nil {
+ _ = file.Close()
+ }
+ })
+ case <-ctx.Done():
+ }
}()
return ctx, cancel
diff --git a/internal/backup/collector.go b/internal/backup/collector.go
index 7bd627bc..da2d7faf 100644
--- a/internal/backup/collector.go
+++ b/internal/backup/collector.go
@@ -968,38 +968,68 @@ func (c *Collector) safeCopyDir(ctx context.Context, src, dest, description stri
return nil
}
-func (c *Collector) safeCmdOutput(ctx context.Context, spec CommandSpec, output, description string, critical bool) error {
+type commandRunClassification int
+
+const (
+ commandRunSucceeded commandRunClassification = iota
+ commandRunSkipped
+ commandRunNonCriticalFailure
+ commandRunDowngradedToSkip
+ commandRunCriticalFailure
+)
+
+type commandRunOptions struct {
+ output string
+ description string
+ caller string
+ critical bool
+ logCollection bool
+ handleSystemctlStatus bool
+}
+
+type commandRunResult struct {
+ output []byte
+ classification commandRunClassification
+ exitCode int
+ outputSummary string
+ contextInfo unprivilegedContainerContext
+}
+
+func (c *Collector) runAndClassifyCommand(ctx context.Context, spec CommandSpec, opts commandRunOptions) (commandRunResult, error) {
+ result := commandRunResult{classification: commandRunSkipped, exitCode: -1}
if err := ctx.Err(); err != nil {
- return err
+ return result, err
}
if err := spec.validate(); err != nil {
- return err
+ return result, err
}
- if output != "" && c.shouldExclude(output) {
- c.logger.Debug("Skipping %s: output %s excluded by pattern", description, output)
+ if opts.output != "" && c.shouldExclude(opts.output) {
+ c.logger.Debug("Skipping %s: output %s excluded by pattern", opts.description, opts.output)
c.incFilesSkipped()
- return nil
+ return result, nil
}
- c.logger.Debug("Collecting %s via command: %s > %s", description, spec.String(), output)
+ cmdString := spec.String()
+ if opts.logCollection {
+ c.logger.Debug("Collecting %s via command: %s > %s", opts.description, cmdString, opts.output)
+ }
- // Check if command exists
if _, err := c.depLookPath(spec.Name); err != nil {
- if critical {
+ if opts.critical {
c.incFilesFailed()
- return fmt.Errorf("critical command not available: %s", spec.Name)
+ result.classification = commandRunCriticalFailure
+ return result, fmt.Errorf("critical command not available: %s", spec.Name)
}
- c.logger.Debug("Command not available: %s (skipping %s)", spec.Name, description)
- return nil
+ c.logger.Debug("Command not available: %s (skipping %s)", spec.Name, opts.description)
+ return result, nil
}
if c.dryRun {
- c.logger.Debug("[DRY RUN] Would execute command: %s > %s", spec.String(), output)
- return nil
+ c.logger.Debug("[DRY RUN] Would execute command: %s > %s", cmdString, opts.output)
+ return result, nil
}
- cmdString := spec.String()
runCtx := ctx
var cancel context.CancelFunc
if spec.Name == "pvesh" && c.config != nil && c.config.PveshTimeoutSeconds > 0 {
@@ -1010,10 +1040,13 @@ func (c *Collector) safeCmdOutput(ctx context.Context, spec CommandSpec, output,
}
out, err := c.depRunCommand(runCtx, spec.Name, spec.Args...)
+ result.output = out
if err != nil {
- if critical {
+ result.outputSummary = summarizeCommandOutputText(string(out))
+ if opts.critical {
c.incFilesFailed()
- return fmt.Errorf("critical command `%s` failed for %s: %w (output: %s)", cmdString, description, err, summarizeCommandOutputText(string(out)))
+ result.classification = commandRunCriticalFailure
+ return result, fmt.Errorf("critical command `%s` failed for %s: %w (output: %s)", cmdString, opts.description, err, result.outputSummary)
}
exitCode := -1
var exitErr *exec.ExitError
@@ -1021,11 +1054,15 @@ func (c *Collector) safeCmdOutput(ctx context.Context, spec CommandSpec, output,
exitCode = exitErr.ExitCode()
}
outputText := strings.TrimSpace(string(out))
+ result.exitCode = exitCode
+ result.outputSummary = summarizeCommandOutputText(outputText)
+ result.classification = commandRunNonCriticalFailure
- c.logger.Debug("Non-critical command failed (safeCmdOutput): description=%q cmd=%q exitCode=%d err=%v", description, cmdString, exitCode, err)
- c.logger.Debug("Non-critical command output summary (safeCmdOutput): %s", summarizeCommandOutputText(outputText))
+ c.logger.Debug("Non-critical command failed (%s): description=%q cmd=%q exitCode=%d err=%v", opts.caller, opts.description, cmdString, exitCode, err)
+ c.logger.Debug("Non-critical command output summary (%s): %s", opts.caller, result.outputSummary)
ctxInfo := c.depDetectUnprivilegedContainer()
+ result.contextInfo = ctxInfo
c.logger.Debug("Privilege context evaluation: detected=%t details=%q", ctxInfo.Detected, strings.TrimSpace(ctxInfo.Details))
reason := ""
@@ -1039,32 +1076,83 @@ func (c *Collector) safeCmdOutput(ctx context.Context, spec CommandSpec, output,
}
if ctxInfo.Detected && reason != "" {
- c.logger.Debug("Downgrading WARNING->SKIP: description=%q cmd=%q exitCode=%d", description, cmdString, exitCode)
+ result.classification = commandRunDowngradedToSkip
+ c.logger.Debug("Downgrading WARNING->SKIP: description=%q cmd=%q exitCode=%d", opts.description, cmdString, exitCode)
- c.logger.Skip("Skipping %s: %s (Expected with limited privileges).", description, reason)
- c.logger.Debug("SKIP context (privilege-sensitive): description=%q cmd=%q exitCode=%d err=%v contextDetails=%q", description, cmdString, exitCode, err, strings.TrimSpace(ctxInfo.Details))
- c.logger.Debug("SKIP output summary for %s: %s", description, summarizeCommandOutputText(outputText))
- return nil
+ c.logger.Skip("Skipping %s: %s (Expected with limited privileges).", opts.description, reason)
+ c.logger.Debug("SKIP context (privilege-sensitive): description=%q cmd=%q exitCode=%d err=%v contextDetails=%q", opts.description, cmdString, exitCode, err, strings.TrimSpace(ctxInfo.Details))
+ c.logger.Debug("SKIP output summary for %s: %s", opts.description, result.outputSummary)
+ return result, nil
}
if ctxInfo.Detected {
- c.logger.Debug("No privilege-sensitive downgrade applied: command=%q did not match known patterns; emitting WARNING", spec.Name)
+ if opts.handleSystemctlStatus {
+ c.logger.Debug("No privilege-sensitive downgrade applied: command=%q did not match known patterns; continuing with standard handling", spec.Name)
+ } else {
+ c.logger.Debug("No privilege-sensitive downgrade applied: command=%q did not match known patterns; emitting WARNING", spec.Name)
+ }
}
- c.logger.Warning("Skipping %s: command `%s` failed (%v). Non-critical; backup continues. Ensure the required CLI is available and has proper permissions. Output: %s",
- description,
- cmdString,
- err,
- summarizeCommandOutputText(outputText),
- )
- return nil // Non-critical failure
+ if opts.handleSystemctlStatus && spec.Name == "systemctl" && len(spec.Args) >= 2 && spec.Args[0] == "status" {
+ unit := spec.Args[len(spec.Args)-1]
+ if exitCode == 4 || strings.Contains(outputText, "could not be found") {
+ c.logger.Warning("Skipping %s: %s.service not found (not installed?). Set BACKUP_FIREWALL_RULES=false to disable.",
+ opts.description,
+ unit,
+ )
+ return result, nil
+ }
+ if strings.Contains(outputText, "Failed to connect to system scope bus") || strings.Contains(outputText, "System has not been booted with systemd") {
+ c.logger.Warning("Skipping %s: systemd is not available/accessible in this environment. Non-critical; backup continues. Output: %s",
+ opts.description,
+ result.outputSummary,
+ )
+ return result, nil
+ }
+ }
+
+ if opts.handleSystemctlStatus {
+ c.logger.Warning("Skipping %s: command `%s` failed (%v). Non-critical; backup continues. Output: %s",
+ opts.description,
+ cmdString,
+ err,
+ result.outputSummary,
+ )
+ } else {
+ c.logger.Warning("Skipping %s: command `%s` failed (%v). Non-critical; backup continues. Ensure the required CLI is available and has proper permissions. Output: %s",
+ opts.description,
+ cmdString,
+ err,
+ result.outputSummary,
+ )
+ }
+ return result, nil
}
- if err := c.writeReportFile(output, out); err != nil {
+ result.classification = commandRunSucceeded
+ return result, nil
+}
+
+func (c *Collector) safeCmdOutput(ctx context.Context, spec CommandSpec, output, description string, critical bool) error {
+ result, err := c.runAndClassifyCommand(ctx, spec, commandRunOptions{
+ output: output,
+ description: description,
+ caller: "safeCmdOutput",
+ critical: critical,
+ logCollection: true,
+ })
+ if err != nil {
return err
}
+ if result.classification != commandRunSucceeded {
+ return nil
+ }
- c.logger.Debug("Successfully collected %s via command: %s", description, cmdString)
+ if err := c.writeReportFile(output, result.output); err != nil {
+ return err
+ }
+
+ c.logger.Debug("Successfully collected %s via command: %s", description, spec.String())
return nil
}
@@ -1331,117 +1419,25 @@ func (c *Collector) writeReportFile(path string, data []byte) error {
}
func (c *Collector) captureCommandOutput(ctx context.Context, spec CommandSpec, output, description string, critical bool) ([]byte, error) {
- if err := ctx.Err(); err != nil {
- return nil, err
- }
- if err := spec.validate(); err != nil {
+ result, err := c.runAndClassifyCommand(ctx, spec, commandRunOptions{
+ output: output,
+ description: description,
+ caller: "captureCommandOutput",
+ critical: critical,
+ handleSystemctlStatus: true,
+ })
+ if err != nil {
return nil, err
}
-
- if output != "" && c.shouldExclude(output) {
- c.logger.Debug("Skipping %s: output %s excluded by pattern", description, output)
- c.incFilesSkipped()
- return nil, nil
- }
-
- if _, err := c.depLookPath(spec.Name); err != nil {
- if critical {
- c.incFilesFailed()
- return nil, fmt.Errorf("critical command not available: %s", spec.Name)
- }
- c.logger.Debug("Command not available: %s (skipping %s)", spec.Name, description)
- return nil, nil
- }
-
- if c.dryRun {
- c.logger.Debug("[DRY RUN] Would execute command: %s > %s", spec.String(), output)
- return nil, nil
- }
-
- runCtx := ctx
- var cancel context.CancelFunc
- if spec.Name == "pvesh" && c.config != nil && c.config.PveshTimeoutSeconds > 0 {
- runCtx, cancel = context.WithTimeout(ctx, time.Duration(c.config.PveshTimeoutSeconds)*time.Second)
- }
- if cancel != nil {
- defer cancel()
- }
-
- out, err := c.depRunCommand(runCtx, spec.Name, spec.Args...)
- if err != nil {
- cmdString := spec.String()
- if critical {
- c.incFilesFailed()
- return nil, fmt.Errorf("critical command `%s` failed for %s: %w (output: %s)", cmdString, description, err, summarizeCommandOutputText(string(out)))
- }
- exitCode := -1
- var exitErr *exec.ExitError
- if errors.As(err, &exitErr) {
- exitCode = exitErr.ExitCode()
- }
- outputText := strings.TrimSpace(string(out))
-
- c.logger.Debug("Non-critical command failed (captureCommandOutput): description=%q cmd=%q exitCode=%d err=%v", description, cmdString, exitCode, err)
- c.logger.Debug("Non-critical command output summary (captureCommandOutput): %s", summarizeCommandOutputText(outputText))
-
- ctxInfo := c.depDetectUnprivilegedContainer()
- c.logger.Debug("Privilege context evaluation: detected=%t details=%q", ctxInfo.Detected, strings.TrimSpace(ctxInfo.Details))
-
- reason := ""
- if ctxInfo.Detected {
- c.logger.Debug("Privilege-sensitive allowlist: command=%q allowlisted=%t", spec.Name, isPrivilegeSensitiveCommand(spec.Name))
- match := privilegeSensitiveFailureMatch(spec.Name, exitCode, outputText)
- reason = match.Reason
- c.logger.Debug("Privilege-sensitive classification: command=%q matched=%t match=%q reason=%q", spec.Name, reason != "", match.Match, reason)
- } else {
- c.logger.Debug("Privilege-sensitive downgrade not considered: limited-privilege context not detected (command=%q)", spec.Name)
- }
-
- if ctxInfo.Detected && reason != "" {
- c.logger.Debug("Downgrading WARNING->SKIP: description=%q cmd=%q exitCode=%d", description, cmdString, exitCode)
-
- c.logger.Skip("Skipping %s: %s (Expected with limited privileges).", description, reason)
- c.logger.Debug("SKIP context (privilege-sensitive): description=%q cmd=%q exitCode=%d err=%v contextDetails=%q", description, cmdString, exitCode, err, strings.TrimSpace(ctxInfo.Details))
- c.logger.Debug("SKIP output summary for %s: %s", description, summarizeCommandOutputText(outputText))
- return nil, nil
- }
-
- if ctxInfo.Detected {
- c.logger.Debug("No privilege-sensitive downgrade applied: command=%q did not match known patterns; continuing with standard handling", spec.Name)
- }
-
- if spec.Name == "systemctl" && len(spec.Args) >= 2 && spec.Args[0] == "status" {
- unit := spec.Args[len(spec.Args)-1]
- if exitCode == 4 || strings.Contains(outputText, "could not be found") {
- c.logger.Warning("Skipping %s: %s.service not found (not installed?). Set BACKUP_FIREWALL_RULES=false to disable.",
- description,
- unit,
- )
- return nil, nil
- }
- if strings.Contains(outputText, "Failed to connect to system scope bus") || strings.Contains(outputText, "System has not been booted with systemd") {
- c.logger.Warning("Skipping %s: systemd is not available/accessible in this environment. Non-critical; backup continues. Output: %s",
- description,
- summarizeCommandOutputText(outputText),
- )
- return nil, nil
- }
- }
-
- c.logger.Warning("Skipping %s: command `%s` failed (%v). Non-critical; backup continues. Output: %s",
- description,
- cmdString,
- err,
- summarizeCommandOutputText(outputText),
- )
+ if result.classification != commandRunSucceeded {
return nil, nil
}
- if err := c.writeReportFile(output, out); err != nil {
+ if err := c.writeReportFile(output, result.output); err != nil {
return nil, err
}
- return out, nil
+ return result.output, nil
}
func (c *Collector) collectCommandMulti(ctx context.Context, spec CommandSpec, output, description string, critical bool, mirrors ...string) error {
diff --git a/internal/backup/collector_bricks_pve_finalize.go b/internal/backup/collector_bricks_pve_finalize.go
index 1585ff9b..9753a921 100644
--- a/internal/backup/collector_bricks_pve_finalize.go
+++ b/internal/backup/collector_bricks_pve_finalize.go
@@ -127,15 +127,15 @@ func newPVEManifestBricks() []collectionBrick {
}
}
-func (p pveContext) runtimeNodes() []string {
- if p.runtimeInfo == nil {
+func (p *pveContext) runtimeNodes() []string {
+ if p == nil || p.runtimeInfo == nil {
return nil
}
return p.runtimeInfo.Nodes
}
-func (p pveContext) runtimeStorages() []pveStorageEntry {
- if p.runtimeInfo == nil {
+func (p *pveContext) runtimeStorages() []pveStorageEntry {
+ if p == nil || p.runtimeInfo == nil {
return nil
}
return p.runtimeInfo.Storages
@@ -148,8 +148,8 @@ func (p *pveContext) ensureStorageScanResults() map[string]*pveStorageScanResult
return p.storageScanResults
}
-func (p pveContext) storageResult(storage pveStorageEntry) *pveStorageScanResult {
- if p.storageScanResults == nil {
+func (p *pveContext) storageResult(storage pveStorageEntry) *pveStorageScanResult {
+ if p == nil || p.storageScanResults == nil {
return nil
}
return p.storageScanResults[storage.pathKey()]
diff --git a/internal/notify/webhook.go b/internal/notify/webhook.go
index a6b35d79..08c5f629 100644
--- a/internal/notify/webhook.go
+++ b/internal/notify/webhook.go
@@ -81,6 +81,16 @@ func NewWebhookNotifier(webhookConfig *config.WebhookConfig, logger *logging.Log
format := resolveWebhookFormat(ep.Format, webhookConfig.DefaultFormat)
method := resolveWebhookMethod(ep.Method)
if strings.EqualFold(format, "pushover") {
+ missing := []string{}
+ if ep.Auth.Token == "" {
+ missing = append(missing, "token")
+ }
+ if ep.Auth.User == "" {
+ missing = append(missing, "user")
+ }
+ if len(missing) > 0 {
+ return nil, fmt.Errorf("webhook endpoint %q: Pushover requires Auth.Token and Auth.User; missing %s", ep.Name, strings.Join(missing, "/"))
+ }
if ep.Priority < -2 || ep.Priority > 1 {
return nil, fmt.Errorf("webhook endpoint %q: PRIORITY must be in range -2..1 (got %d); priority 2 (emergency) is not supported", ep.Name, ep.Priority)
}
diff --git a/internal/notify/webhook_test.go b/internal/notify/webhook_test.go
index ad639690..da918043 100644
--- a/internal/notify/webhook_test.go
+++ b/internal/notify/webhook_test.go
@@ -1318,3 +1318,43 @@ func TestNewWebhookNotifier_PushoverMethod(t *testing.T) {
})
}
}
+
+func TestNewWebhookNotifier_PushoverAuthRequired(t *testing.T) {
+ logger := logging.New(types.LogLevelDebug, false)
+
+ tests := []struct {
+ name string
+ token string
+ user string
+ missing string
+ }{
+ {name: "missing token", token: "", user: "user-key-xyz", missing: "missing token"},
+ {name: "missing user", token: "app-token-abc", user: "", missing: "missing user"},
+ {name: "missing both", token: "", user: "", missing: "missing token/user"},
+ }
+
+ for _, tt := range tests {
+ t.Run(tt.name, func(t *testing.T) {
+ ep := pushoverTestEndpoint(0)
+ ep.Auth.Token = tt.token
+ ep.Auth.User = tt.user
+
+ cfg := &config.WebhookConfig{
+ Enabled: true,
+ Timeout: 30,
+ Endpoints: []config.WebhookEndpoint{ep},
+ }
+
+ _, err := NewWebhookNotifier(cfg, logger)
+ if err == nil {
+ t.Fatal("expected error, got nil")
+ }
+ if !strings.Contains(err.Error(), "Pushover requires Auth.Token and Auth.User") {
+ t.Fatalf("error %q does not mention Pushover auth requirement", err.Error())
+ }
+ if !strings.Contains(err.Error(), tt.missing) {
+ t.Fatalf("error %q does not mention %q", err.Error(), tt.missing)
+ }
+ })
+ }
+}
diff --git a/internal/safeexec/safeexec_test.go b/internal/safeexec/safeexec_test.go
index 96bee49b..3144ad80 100644
--- a/internal/safeexec/safeexec_test.go
+++ b/internal/safeexec/safeexec_test.go
@@ -9,8 +9,23 @@ import (
)
func TestCommandContextAllowlist(t *testing.T) {
- if _, err := CommandContext(context.Background(), "rclone", "lsf", "remote:"); err != nil {
- t.Fatalf("CommandContext allowed command error: %v", err)
+ allowedCommands := []string{
+ "rclone",
+ "tar",
+ "xz",
+ "zstd",
+ "systemctl",
+ "mailq",
+ "tail",
+ "journalctl",
+ "pvesh",
+ "pveum",
+ "proxmox-backup-manager",
+ }
+ for _, command := range allowedCommands {
+ if _, err := CommandContext(context.Background(), command); err != nil {
+ t.Fatalf("CommandContext(%q) allowed command error: %v", command, err)
+ }
}
if _, err := CommandContext(context.Background(), "not-a-proxsave-command"); !errors.Is(err, ErrCommandNotAllowed) {
t.Fatalf("CommandContext unknown command error = %v, want ErrCommandNotAllowed", err)
From a165abace8488696a8979ebe3aa44ff1ce4bcff8 Mon Sep 17 00:00:00 2001
From: Damiano <71268257+tis24dev@users.noreply.github.com>
Date: Tue, 5 May 2026 21:41:32 +0200
Subject: [PATCH 22/35] Return cloud FS errors and collect mode errors
Propagate filesystem detection errors for cloud backends so callers get diagnostics (detectFilesystemInfo now returns an error for LocationCloud and backup init logs include the failure reason). Adjust backup storage init logging to include the detection reason and mark cloud disabled. Change validateModeCompatibility to accumulate and return all compatibility violations (with a new test added). Rename helper 'newer' to 'meetsMinimum' for clarity and normalize directory mode literal to 0o755. Tests updated/added to cover the new behaviors.
---
cmd/proxsave/backup_storage.go | 12 ++++++++----
cmd/proxsave/main_modes.go | 5 +++--
cmd/proxsave/main_modes_test.go | 11 +++++++++++
cmd/proxsave/main_runtime.go | 4 ++--
cmd/proxsave/runtime_helpers.go | 3 +++
cmd/proxsave/runtime_helpers_more_test.go | 14 ++++++++++++++
internal/orchestrator/restore_archive.go | 2 +-
7 files changed, 42 insertions(+), 9 deletions(-)
diff --git a/cmd/proxsave/backup_storage.go b/cmd/proxsave/backup_storage.go
index 02e4135d..5cfb621f 100644
--- a/cmd/proxsave/backup_storage.go
+++ b/cmd/proxsave/backup_storage.go
@@ -129,16 +129,20 @@ func initializeCloudStorage(opts backupModeOptions, orch *orchestrator.Orchestra
return nil
}
- cloudFS, _ := detectFilesystemInfo(opts.ctx, cloudBackend, cfg.CloudRemote, logger)
+ cloudFS, err := detectFilesystemInfo(opts.ctx, cloudBackend, cfg.CloudRemote, logger)
if cloudFS == nil {
- logging.DebugStep(logger, "storage init", "cloud unavailable, disabling")
+ reason := "filesystem detection unavailable"
+ if err != nil {
+ reason = fmt.Sprintf("filesystem detection failed: %v", err)
+ }
+ logging.DebugStep(logger, "storage init", "cloud unavailable, disabling: %s", reason)
cfg.CloudEnabled = false
cfg.CloudLogPath = ""
if checker != nil {
checker.DisableCloud()
}
- logStorageInitSummary(formatStorageInitSummary("Cloud storage", cfg, storage.LocationCloud, nil, nil))
- logging.Skip("Path Cloud: disabled")
+ logStorageInitSummary(fmt.Sprintf("%s; %s", formatStorageInitSummary("Cloud storage", cfg, storage.LocationCloud, nil, nil), reason))
+ logging.Skip("Path Cloud: disabled (%s)", reason)
return nil
}
diff --git a/cmd/proxsave/main_modes.go b/cmd/proxsave/main_modes.go
index c8d96912..ed2740a2 100644
--- a/cmd/proxsave/main_modes.go
+++ b/cmd/proxsave/main_modes.go
@@ -27,6 +27,7 @@ func validateModeCompatibility(args *cli.Args) []string {
return []string{"command-line arguments are required"}
}
+ var allMessages []string
for _, rule := range []modeCompatibilityRule{
validateCleanupGuardsCompatibility,
validateSupportCompatibility,
@@ -34,10 +35,10 @@ func validateModeCompatibility(args *cli.Args) []string {
validateUpgradeCompatibility,
} {
if messages := rule(args); len(messages) > 0 {
- return messages
+ allMessages = append(allMessages, messages...)
}
}
- return nil
+ return allMessages
}
func validateCleanupGuardsCompatibility(args *cli.Args) []string {
diff --git a/cmd/proxsave/main_modes_test.go b/cmd/proxsave/main_modes_test.go
index 3b9d5b59..1608eb49 100644
--- a/cmd/proxsave/main_modes_test.go
+++ b/cmd/proxsave/main_modes_test.go
@@ -52,6 +52,17 @@ func TestValidateModeCompatibility(t *testing.T) {
args: &cli.Args{Upgrade: true, Install: true},
want: []string{"Cannot use --upgrade together with --install or --new-install."},
},
+ {
+ name: "accumulates all compatibility violations",
+ args: &cli.Args{CleanupGuards: true, Support: true, Decrypt: true, Install: true, NewInstall: true, Upgrade: true},
+ want: []string{
+ "--cleanup-guards cannot be combined with: --support, --decrypt, --install, --new-install, --upgrade",
+ "Support mode cannot be combined with: --decrypt, --install, --new-install",
+ "--support is only available for the standard backup run or --restore.",
+ "Cannot use --install and --new-install together. Choose one installation mode.",
+ "Cannot use --upgrade together with --install or --new-install.",
+ },
+ },
}
for _, tt := range tests {
diff --git a/cmd/proxsave/main_runtime.go b/cmd/proxsave/main_runtime.go
index 90f4e648..f1a174d8 100644
--- a/cmd/proxsave/main_runtime.go
+++ b/cmd/proxsave/main_runtime.go
@@ -288,7 +288,7 @@ func checkGoRuntimeVersion(minimum string) error {
rtMaj, rtMin, rtPatch := parse(rt)
minMaj, minMin, minPatch := parse(minimum)
- newer := func(aMaj, aMin, aPatch, bMaj, bMin, bPatch int) bool {
+ meetsMinimum := func(aMaj, aMin, aPatch, bMaj, bMin, bPatch int) bool {
if aMaj != bMaj {
return aMaj > bMaj
}
@@ -298,7 +298,7 @@ func checkGoRuntimeVersion(minimum string) error {
return aPatch >= bPatch
}
- if !newer(rtMaj, rtMin, rtPatch, minMaj, minMin, minPatch) {
+ if !meetsMinimum(rtMaj, rtMin, rtPatch, minMaj, minMin, minPatch) {
return fmt.Errorf("go runtime version %s is below required %s — rebuild with go %s or set GOTOOLCHAIN=auto", rt, "go"+minimum, "go"+minimum)
}
return nil
diff --git a/cmd/proxsave/runtime_helpers.go b/cmd/proxsave/runtime_helpers.go
index 12fc9f9d..0a52b206 100644
--- a/cmd/proxsave/runtime_helpers.go
+++ b/cmd/proxsave/runtime_helpers.go
@@ -279,6 +279,9 @@ func detectFilesystemInfo(ctx context.Context, backend storage.Storage, path str
return nil, err
}
logger.Debug("WARNING: %s filesystem detection failed: %v", backend.Name(), err)
+ if backend.Location() == storage.LocationCloud {
+ return nil, err
+ }
return nil, nil
}
diff --git a/cmd/proxsave/runtime_helpers_more_test.go b/cmd/proxsave/runtime_helpers_more_test.go
index f71a813b..c7581281 100644
--- a/cmd/proxsave/runtime_helpers_more_test.go
+++ b/cmd/proxsave/runtime_helpers_more_test.go
@@ -161,6 +161,20 @@ func TestDetectFilesystemInfo(t *testing.T) {
}
})
+ t.Run("cloud error is returned for caller diagnostics", func(t *testing.T) {
+ backend := &fakeStorageBackend{
+ name: "cloud",
+ location: storage.LocationCloud,
+ enabled: true,
+ critical: false,
+ fsErr: errors.New("cloud detect failed"),
+ }
+ info, err := detectFilesystemInfo(ctx, backend, "remote:path", logger)
+ if err == nil || info != nil {
+ t.Fatalf("detectFilesystemInfo() = (%v,%v), want (nil,error)", info, err)
+ }
+ })
+
t.Run("critical error is returned", func(t *testing.T) {
backend := &fakeStorageBackend{
name: "primary",
diff --git a/internal/orchestrator/restore_archive.go b/internal/orchestrator/restore_archive.go
index e812ff29..363b7387 100644
--- a/internal/orchestrator/restore_archive.go
+++ b/internal/orchestrator/restore_archive.go
@@ -254,7 +254,7 @@ func extractSelectiveArchive(ctx context.Context, archivePath, destRoot string,
// Create detailed log directory
logDir := "/tmp/proxsave"
- if err := restoreFS.MkdirAll(logDir, 0755); err != nil {
+ if err := restoreFS.MkdirAll(logDir, 0o755); err != nil {
logger.Warning("Could not create log directory: %v", err)
}
From 5457ec55df28ae6920cc81cbf646e26a65c4e900 Mon Sep 17 00:00:00 2001
From: Damiano <71268257+tis24dev@users.noreply.github.com>
Date: Tue, 5 May 2026 21:49:44 +0200
Subject: [PATCH 23/35] Propagate context to ZFS restore checks
Pass context.Context through ZFS restore helper functions and tests so operations can be cancelled and use caller-provided contexts. checkZFSPoolsAfterRestore, detectImportableZFSPools and logNoImportableZFSPools now accept a context, check ctx.Err, and pass ctx to restoreCmd.Run. The UI caller was updated to forward the workflow context. Tests were updated to supply contexts and two new tests verify context propagation and behavior on canceled contexts. FakeCommandRunner was extended to record contexts for assertions.
---
internal/orchestrator/deps_test.go | 8 +--
.../restore_coverage_extra_test.go | 54 +++++++++++++++++--
internal/orchestrator/restore_workflow_ui.go | 2 +-
internal/orchestrator/restore_zfs.go | 36 +++++++++----
4 files changed, 81 insertions(+), 19 deletions(-)
diff --git a/internal/orchestrator/deps_test.go b/internal/orchestrator/deps_test.go
index dea9b1e3..fa7af74f 100644
--- a/internal/orchestrator/deps_test.go
+++ b/internal/orchestrator/deps_test.go
@@ -210,14 +210,16 @@ func (f *FakeTime) Advance(d time.Duration) {
// FakeCommandRunner records invocations and returns predefined outputs/errors.
type FakeCommandRunner struct {
- Outputs map[string][]byte
- Errors map[string]error
- Calls []string
+ Outputs map[string][]byte
+ Errors map[string]error
+ Calls []string
+ Contexts []context.Context
}
func (f *FakeCommandRunner) Run(ctx context.Context, name string, args ...string) ([]byte, error) {
key := commandKey(name, args)
f.Calls = append(f.Calls, key)
+ f.Contexts = append(f.Contexts, ctx)
var out []byte
if f.Outputs != nil {
out = f.Outputs[key]
diff --git a/internal/orchestrator/restore_coverage_extra_test.go b/internal/orchestrator/restore_coverage_extra_test.go
index 15bdd2b9..47d6c60b 100644
--- a/internal/orchestrator/restore_coverage_extra_test.go
+++ b/internal/orchestrator/restore_coverage_extra_test.go
@@ -21,6 +21,8 @@ func (runOnlyRunner) Run(ctx context.Context, name string, args ...string) ([]by
return nil, fmt.Errorf("unexpected command: %s", commandKey(name, args))
}
+type zfsContextTestKey struct{}
+
type recordingRunner struct {
calls []string
}
@@ -63,7 +65,7 @@ func TestDetectImportableZFSPools_ReturnsPoolsAndErrorWhenCommandFails(t *testin
}
restoreCmd = fake
- pools, output, err := detectImportableZFSPools()
+ pools, output, err := detectImportableZFSPools(context.Background())
if err == nil {
t.Fatalf("expected error")
}
@@ -86,7 +88,7 @@ func TestCheckZFSPoolsAfterRestore_ReturnsNilWhenZpoolMissing(t *testing.T) {
}
restoreCmd = fake
- if err := checkZFSPoolsAfterRestore(newTestLogger()); err != nil {
+ if err := checkZFSPoolsAfterRestore(context.Background(), newTestLogger()); err != nil {
t.Fatalf("expected nil error when zpool missing, got %v", err)
}
if len(fake.Calls) != 1 || fake.Calls[0] != "which zpool" {
@@ -94,6 +96,50 @@ func TestCheckZFSPoolsAfterRestore_ReturnsNilWhenZpoolMissing(t *testing.T) {
}
}
+func TestCheckZFSPoolsAfterRestore_UsesProvidedContext(t *testing.T) {
+ orig := restoreCmd
+ t.Cleanup(func() { restoreCmd = orig })
+
+ fake := &FakeCommandRunner{
+ Outputs: map[string][]byte{
+ "which zpool": []byte("/sbin/zpool\n"),
+ "zpool import": []byte(""),
+ },
+ }
+ restoreCmd = fake
+
+ ctx := context.WithValue(context.Background(), zfsContextTestKey{}, "restore")
+ if err := checkZFSPoolsAfterRestore(ctx, newTestLogger()); err != nil {
+ t.Fatalf("checkZFSPoolsAfterRestore error: %v", err)
+ }
+
+ if len(fake.Contexts) == 0 {
+ t.Fatalf("expected command contexts to be recorded")
+ }
+ for i, got := range fake.Contexts {
+ if got.Value(zfsContextTestKey{}) != "restore" {
+ t.Fatalf("command context %d did not use restore context", i)
+ }
+ }
+}
+
+func TestCheckZFSPoolsAfterRestore_ReturnsCanceledContext(t *testing.T) {
+ orig := restoreCmd
+ t.Cleanup(func() { restoreCmd = orig })
+
+ fake := &FakeCommandRunner{}
+ restoreCmd = fake
+
+ ctx, cancel := context.WithCancel(context.Background())
+ cancel()
+ if err := checkZFSPoolsAfterRestore(ctx, newTestLogger()); err != context.Canceled {
+ t.Fatalf("checkZFSPoolsAfterRestore error = %v, want context.Canceled", err)
+ }
+ if len(fake.Calls) != 0 {
+ t.Fatalf("expected no commands after canceled context, got %#v", fake.Calls)
+ }
+}
+
func TestCheckZFSPoolsAfterRestore_ConfiguredPools_NoImportables(t *testing.T) {
origCmd := restoreCmd
origFS := restoreFS
@@ -131,7 +177,7 @@ func TestCheckZFSPoolsAfterRestore_ConfiguredPools_NoImportables(t *testing.T) {
}
restoreCmd = fake
- if err := checkZFSPoolsAfterRestore(newTestLogger()); err != nil {
+ if err := checkZFSPoolsAfterRestore(context.Background(), newTestLogger()); err != nil {
t.Fatalf("checkZFSPoolsAfterRestore error: %v", err)
}
@@ -172,7 +218,7 @@ func TestCheckZFSPoolsAfterRestore_ReportsImportablePools(t *testing.T) {
}
restoreCmd = fake
- if err := checkZFSPoolsAfterRestore(newTestLogger()); err != nil {
+ if err := checkZFSPoolsAfterRestore(context.Background(), newTestLogger()); err != nil {
t.Fatalf("checkZFSPoolsAfterRestore error: %v", err)
}
diff --git a/internal/orchestrator/restore_workflow_ui.go b/internal/orchestrator/restore_workflow_ui.go
index 2ed294be..0fd91a27 100644
--- a/internal/orchestrator/restore_workflow_ui.go
+++ b/internal/orchestrator/restore_workflow_ui.go
@@ -855,7 +855,7 @@ func runRestoreWorkflowWithUI(ctx context.Context, cfg *config.Config, logger *l
if hasCategoryID(plan.NormalCategories, "zfs") {
logger.Info("")
- if err := checkZFSPoolsAfterRestore(logger); err != nil {
+ if err := checkZFSPoolsAfterRestore(ctx, logger); err != nil {
logger.Warning("ZFS pool check: %v", err)
}
} else {
diff --git a/internal/orchestrator/restore_zfs.go b/internal/orchestrator/restore_zfs.go
index ebf65dd2..e4bfbf12 100644
--- a/internal/orchestrator/restore_zfs.go
+++ b/internal/orchestrator/restore_zfs.go
@@ -13,9 +13,15 @@ import (
var restoreGlob = filepath.Glob
-// checkZFSPoolsAfterRestore checks if ZFS pools need to be imported after restore
-func checkZFSPoolsAfterRestore(logger *logging.Logger) error {
- if _, err := restoreCmd.Run(context.Background(), "which", "zpool"); err != nil {
+// checkZFSPoolsAfterRestore checks if ZFS pools need to be imported after restore.
+func checkZFSPoolsAfterRestore(ctx context.Context, logger *logging.Logger) error {
+ if err := ctx.Err(); err != nil {
+ return err
+ }
+ if _, err := restoreCmd.Run(ctx, "which", "zpool"); err != nil {
+ if ctxErr := ctx.Err(); ctxErr != nil {
+ return ctxErr
+ }
// zpool utility not available -> no ZFS tooling installed
return nil
}
@@ -23,14 +29,18 @@ func checkZFSPoolsAfterRestore(logger *logging.Logger) error {
logger.Info("Checking ZFS pool status...")
configuredPools := detectConfiguredZFSPools()
- importablePools, importOutput, importErr := detectImportableZFSPools()
+ importablePools, importOutput, importErr := detectImportableZFSPools(ctx)
+ if importErr != nil {
+ if ctxErr := ctx.Err(); ctxErr != nil {
+ return ctxErr
+ }
+ }
logConfiguredZFSPools(logger, configuredPools)
logImportableZFSPools(logger, importablePools, importOutput, importErr)
if len(importablePools) == 0 {
- logNoImportableZFSPools(logger, configuredPools)
- return nil
+ return logNoImportableZFSPools(ctx, logger, configuredPools)
}
logManualZFSImportInstructions(logger, importablePools)
@@ -65,19 +75,23 @@ func logImportableZFSPools(logger *logging.Logger, importablePools []string, imp
}
}
-func logNoImportableZFSPools(logger *logging.Logger, configuredPools []string) {
+func logNoImportableZFSPools(ctx context.Context, logger *logging.Logger, configuredPools []string) error {
logger.Info("`zpool import` did not report pools waiting for import.")
if len(configuredPools) == 0 {
- return
+ return nil
}
logger.Info("")
for _, pool := range configuredPools {
- if _, err := restoreCmd.Run(context.Background(), "zpool", "status", pool); err == nil {
+ if _, err := restoreCmd.Run(ctx, "zpool", "status", pool); err == nil {
logger.Info("Pool %s is already imported (no manual action needed)", pool)
} else {
+ if ctxErr := ctx.Err(); ctxErr != nil {
+ return ctxErr
+ }
logger.Warning("Systemd expects pool %s, but `zpool import` and `zpool status` did not report it. Check disk visibility and pool status.", pool)
}
}
+ return nil
}
func logManualZFSImportInstructions(logger *logging.Logger, importablePools []string) {
@@ -163,8 +177,8 @@ func parsePoolNameFromUnit(unitName string) string {
}
}
-func detectImportableZFSPools() ([]string, string, error) {
- output, err := restoreCmd.Run(context.Background(), "zpool", "import")
+func detectImportableZFSPools(ctx context.Context) ([]string, string, error) {
+ output, err := restoreCmd.Run(ctx, "zpool", "import")
poolNames := parseZpoolImportOutput(string(output))
if err != nil {
return poolNames, string(output), err
From 5ea720d328d0ba88dbdd524f50f2853b42871999 Mon Sep 17 00:00:00 2001
From: Damiano <71268257+tis24dev@users.noreply.github.com>
Date: Tue, 5 May 2026 22:00:36 +0200
Subject: [PATCH 24/35] Treat decompression readers as ReadClosers
Change decompression reader APIs to return io.ReadCloser and ensure readers are closed with errors propagated. Introduce closeDecompressionReader helper to defer Close() and convert close errors into the function's error return. Update callers to use the helper and simplify test cleanup; add a test (TestExtractArchiveNativeReturnsDecompressionCloseError) that verifies decompressor Close() errors surface. Also adjust imports and minor test fixes to read directly from readers.
---
.../orchestrator/decompress_reader_test.go | 72 +++++++++++++++++--
internal/orchestrator/nic_mapping.go | 6 +-
internal/orchestrator/resolv_conf_repair.go | 6 +-
.../orchestrator/restore_archive_extract.go | 6 +-
.../restore_coverage_extra_test.go | 8 +--
internal/orchestrator/restore_decision.go | 14 ++--
.../orchestrator/restore_decompression.go | 27 ++++---
internal/orchestrator/restore_errors_test.go | 2 +-
8 files changed, 98 insertions(+), 43 deletions(-)
diff --git a/internal/orchestrator/decompress_reader_test.go b/internal/orchestrator/decompress_reader_test.go
index 542c7bc8..ddf86fef 100644
--- a/internal/orchestrator/decompress_reader_test.go
+++ b/internal/orchestrator/decompress_reader_test.go
@@ -1,9 +1,12 @@
package orchestrator
import (
+ "bytes"
"context"
+ "errors"
"io"
"os"
+ "path/filepath"
"strings"
"testing"
)
@@ -36,6 +39,7 @@ func TestCreateDecompressionReaderTar(t *testing.T) {
if reader == nil {
t.Fatalf("reader should not be nil for tar")
}
+ _ = reader.Close()
}
type fakeStreamCommandRunner struct {
@@ -59,6 +63,28 @@ func (f *fakeStreamCommandRunner) RunStream(ctx context.Context, name string, st
return io.NopCloser(strings.NewReader("")), nil
}
+type extractionCloseErrorReadCloser struct {
+ *bytes.Reader
+ err error
+}
+
+func (r *extractionCloseErrorReadCloser) Close() error {
+ return r.err
+}
+
+type closeErrorStreamCommandRunner struct {
+ data []byte
+ closeErr error
+}
+
+func (f *closeErrorStreamCommandRunner) Run(context.Context, string, ...string) ([]byte, error) {
+ return nil, nil
+}
+
+func (f *closeErrorStreamCommandRunner) RunStream(context.Context, string, io.Reader, ...string) (io.ReadCloser, error) {
+ return &extractionCloseErrorReadCloser{Reader: bytes.NewReader(f.data), err: f.closeErr}, nil
+}
+
func TestCreateDecompressionReaderUsesStreamingRunnerForCompressedFormats(t *testing.T) {
orig := restoreCmd
t.Cleanup(func() { restoreCmd = orig })
@@ -98,14 +124,9 @@ func TestCreateDecompressionReaderUsesStreamingRunnerForCompressedFormats(t *tes
if err != nil {
t.Fatalf("createDecompressionReader(%s) error: %v", tt.ext, err)
}
+ defer reader.Close()
- rc, ok := reader.(io.ReadCloser)
- if !ok {
- t.Fatalf("expected io.ReadCloser, got %T", reader)
- }
- defer rc.Close()
-
- out, err := io.ReadAll(rc)
+ out, err := io.ReadAll(reader)
if err != nil {
t.Fatalf("ReadAll: %v", err)
}
@@ -118,3 +139,40 @@ func TestCreateDecompressionReaderUsesStreamingRunnerForCompressedFormats(t *tes
})
}
}
+
+func TestExtractArchiveNativeReturnsDecompressionCloseError(t *testing.T) {
+ origCmd := restoreCmd
+ origFS := restoreFS
+ t.Cleanup(func() {
+ restoreCmd = origCmd
+ restoreFS = origFS
+ })
+
+ dir := t.TempDir()
+ tarPath := filepath.Join(dir, "source.tar")
+ if err := writeTarFile(tarPath, map[string]string{"etc/example.conf": "ok\n"}); err != nil {
+ t.Fatalf("writeTarFile: %v", err)
+ }
+ tarData, err := os.ReadFile(tarPath)
+ if err != nil {
+ t.Fatalf("ReadFile: %v", err)
+ }
+
+ closeErr := errors.New("decompressor exited 2")
+ restoreCmd = &closeErrorStreamCommandRunner{data: tarData, closeErr: closeErr}
+ restoreFS = osFS{}
+
+ archivePath := filepath.Join(dir, "archive.tar.zst")
+ if err := os.WriteFile(archivePath, []byte("compressed"), 0o640); err != nil {
+ t.Fatalf("WriteFile: %v", err)
+ }
+
+ err = extractArchiveNative(context.Background(), restoreArchiveOptions{
+ archivePath: archivePath,
+ destRoot: filepath.Join(dir, "dest"),
+ logger: newTestLogger(),
+ })
+ if !errors.Is(err, closeErr) {
+ t.Fatalf("extractArchiveNative error = %v, want close error %v", err, closeErr)
+ }
+}
diff --git a/internal/orchestrator/nic_mapping.go b/internal/orchestrator/nic_mapping.go
index f2a0372c..b77dc273 100644
--- a/internal/orchestrator/nic_mapping.go
+++ b/internal/orchestrator/nic_mapping.go
@@ -312,7 +312,7 @@ func loadBackupNetworkInventoryFromArchive(ctx context.Context, archivePath stri
return &inv, used, nil
}
-func readArchiveEntry(ctx context.Context, archivePath string, candidates []string, maxBytes int64) ([]byte, string, error) {
+func readArchiveEntry(ctx context.Context, archivePath string, candidates []string, maxBytes int64) (data []byte, used string, err error) {
file, err := restoreFS.Open(archivePath)
if err != nil {
return nil, "", err
@@ -323,9 +323,7 @@ func readArchiveEntry(ctx context.Context, archivePath string, candidates []stri
if err != nil {
return nil, "", err
}
- if closer, ok := reader.(io.Closer); ok {
- defer closer.Close()
- }
+ defer closeDecompressionReader(reader, &err, "close decompression reader")
tr := tar.NewReader(reader)
diff --git a/internal/orchestrator/resolv_conf_repair.go b/internal/orchestrator/resolv_conf_repair.go
index 396e6415..bce82238 100644
--- a/internal/orchestrator/resolv_conf_repair.go
+++ b/internal/orchestrator/resolv_conf_repair.go
@@ -148,7 +148,7 @@ func repairResolvConfWithSystemdResolved(logger *logging.Logger) (bool, error) {
return false, nil
}
-func readTarEntry(ctx context.Context, archivePath, name string, maxBytes int64) ([]byte, error) {
+func readTarEntry(ctx context.Context, archivePath, name string, maxBytes int64) (data []byte, err error) {
file, err := restoreFS.Open(archivePath)
if err != nil {
return nil, fmt.Errorf("open archive: %w", err)
@@ -159,9 +159,7 @@ func readTarEntry(ctx context.Context, archivePath, name string, maxBytes int64)
if err != nil {
return nil, fmt.Errorf("create decompression reader: %w", err)
}
- if closer, ok := reader.(io.Closer); ok {
- defer closer.Close()
- }
+ defer closeDecompressionReader(reader, &err, "close decompression reader")
wantA := strings.TrimPrefix(strings.TrimSpace(name), "./")
wantB := "./" + wantA
diff --git a/internal/orchestrator/restore_archive_extract.go b/internal/orchestrator/restore_archive_extract.go
index 521073ca..ea16abb4 100644
--- a/internal/orchestrator/restore_archive_extract.go
+++ b/internal/orchestrator/restore_archive_extract.go
@@ -38,7 +38,7 @@ type restoreExtractionLog struct {
}
// extractArchiveNative extracts TAR archives natively in Go, preserving all timestamps.
-func extractArchiveNative(ctx context.Context, opts restoreArchiveOptions) error {
+func extractArchiveNative(ctx context.Context, opts restoreArchiveOptions) (err error) {
file, err := restoreFS.Open(opts.archivePath)
if err != nil {
return fmt.Errorf("open archive: %w", err)
@@ -49,9 +49,7 @@ func extractArchiveNative(ctx context.Context, opts restoreArchiveOptions) error
if err != nil {
return fmt.Errorf("create decompression reader: %w", err)
}
- if closer, ok := reader.(io.Closer); ok {
- defer closer.Close()
- }
+ defer closeDecompressionReader(reader, &err, "close decompression reader")
extractionLog := newRestoreExtractionLog(opts)
defer extractionLog.close()
diff --git a/internal/orchestrator/restore_coverage_extra_test.go b/internal/orchestrator/restore_coverage_extra_test.go
index 47d6c60b..8225f75a 100644
--- a/internal/orchestrator/restore_coverage_extra_test.go
+++ b/internal/orchestrator/restore_coverage_extra_test.go
@@ -658,13 +658,9 @@ func TestRunRestoreCommandStream_FallsBackToExecCommand(t *testing.T) {
if err != nil {
t.Fatalf("runRestoreCommandStream error: %v", err)
}
- rc, ok := reader.(io.ReadCloser)
- if !ok {
- t.Fatalf("expected io.ReadCloser, got %T", reader)
- }
- defer rc.Close()
+ defer reader.Close()
- out, err := io.ReadAll(rc)
+ out, err := io.ReadAll(reader)
if err != nil {
t.Fatalf("read: %v", err)
}
diff --git a/internal/orchestrator/restore_decision.go b/internal/orchestrator/restore_decision.go
index dafa20a6..903984f6 100644
--- a/internal/orchestrator/restore_decision.go
+++ b/internal/orchestrator/restore_decision.go
@@ -91,14 +91,12 @@ func inspectRestoreArchiveContents(archivePath string, logger *logging.Logger) (
if err != nil {
return nil, err
}
- if closer, ok := reader.(interface{ Close() error }); ok {
- defer func() {
- if closeErr := closer.Close(); closeErr != nil && err == nil {
- inspection = nil
- err = fmt.Errorf("inspect archive: %w", closeErr)
- }
- }()
- }
+ defer func() {
+ if closeErr := reader.Close(); closeErr != nil && err == nil {
+ inspection = nil
+ err = fmt.Errorf("inspect archive: %w", closeErr)
+ }
+ }()
tarReader := tar.NewReader(reader)
archivePaths, metadata, metadataErr, collectErr := collectRestoreArchiveFacts(tarReader)
diff --git a/internal/orchestrator/restore_decompression.go b/internal/orchestrator/restore_decompression.go
index c6fb1d0b..224fea52 100644
--- a/internal/orchestrator/restore_decompression.go
+++ b/internal/orchestrator/restore_decompression.go
@@ -15,11 +15,11 @@ import (
type restoreDecompressionFormat struct {
matches func(string) bool
- open func(context.Context, *os.File) (io.Reader, error)
+ open func(context.Context, *os.File) (io.ReadCloser, error)
}
// createDecompressionReader creates appropriate decompression reader based on file extension
-func createDecompressionReader(ctx context.Context, file *os.File, archivePath string) (io.Reader, error) {
+func createDecompressionReader(ctx context.Context, file *os.File, archivePath string) (io.ReadCloser, error) {
for _, format := range restoreDecompressionFormats() {
if format.matches(archivePath) {
return format.open(ctx, file)
@@ -28,11 +28,20 @@ func createDecompressionReader(ctx context.Context, file *os.File, archivePath s
return nil, fmt.Errorf("unsupported archive format: %s", filepath.Base(archivePath))
}
+func closeDecompressionReader(reader io.Closer, errp *error, operation string) {
+ if reader == nil || errp == nil {
+ return
+ }
+ if closeErr := reader.Close(); closeErr != nil && *errp == nil {
+ *errp = fmt.Errorf("%s: %w", operation, closeErr)
+ }
+}
+
func restoreDecompressionFormats() []restoreDecompressionFormat {
return []restoreDecompressionFormat{
{
matches: func(path string) bool { return strings.HasSuffix(path, ".tar.gz") || strings.HasSuffix(path, ".tgz") },
- open: func(_ context.Context, file *os.File) (io.Reader, error) { return gzip.NewReader(file) },
+ open: func(_ context.Context, file *os.File) (io.ReadCloser, error) { return gzip.NewReader(file) },
},
{matches: func(path string) bool { return strings.HasSuffix(path, ".tar.xz") }, open: createXZReader},
{
@@ -43,33 +52,33 @@ func restoreDecompressionFormats() []restoreDecompressionFormat {
},
{matches: func(path string) bool { return strings.HasSuffix(path, ".tar.bz2") }, open: createBzip2Reader},
{matches: func(path string) bool { return strings.HasSuffix(path, ".tar.lzma") }, open: createLzmaReader},
- {matches: func(path string) bool { return strings.HasSuffix(path, ".tar") }, open: func(_ context.Context, file *os.File) (io.Reader, error) { return file, nil }},
+ {matches: func(path string) bool { return strings.HasSuffix(path, ".tar") }, open: func(_ context.Context, file *os.File) (io.ReadCloser, error) { return file, nil }},
}
}
// createXZReader creates an XZ decompression reader using injectable command runner
-func createXZReader(ctx context.Context, file *os.File) (io.Reader, error) {
+func createXZReader(ctx context.Context, file *os.File) (io.ReadCloser, error) {
return runRestoreCommandStream(ctx, "xz", file, "-d", "-c")
}
// createZstdReader creates a Zstd decompression reader using injectable command runner
-func createZstdReader(ctx context.Context, file *os.File) (io.Reader, error) {
+func createZstdReader(ctx context.Context, file *os.File) (io.ReadCloser, error) {
return runRestoreCommandStream(ctx, "zstd", file, "-d", "-c")
}
// createBzip2Reader creates a Bzip2 decompression reader using injectable command runner
-func createBzip2Reader(ctx context.Context, file *os.File) (io.Reader, error) {
+func createBzip2Reader(ctx context.Context, file *os.File) (io.ReadCloser, error) {
return runRestoreCommandStream(ctx, "bzip2", file, "-d", "-c")
}
// createLzmaReader creates an LZMA decompression reader using injectable command runner
-func createLzmaReader(ctx context.Context, file *os.File) (io.Reader, error) {
+func createLzmaReader(ctx context.Context, file *os.File) (io.ReadCloser, error) {
return runRestoreCommandStream(ctx, "lzma", file, "-d", "-c")
}
// runRestoreCommandStream starts a command that reads from stdin and exposes stdout as a ReadCloser.
// It prefers an injectable streaming runner when available; otherwise falls back to safeexec.
-func runRestoreCommandStream(ctx context.Context, name string, stdin io.Reader, args ...string) (io.Reader, error) {
+func runRestoreCommandStream(ctx context.Context, name string, stdin io.Reader, args ...string) (io.ReadCloser, error) {
type streamingRunner interface {
RunStream(ctx context.Context, name string, stdin io.Reader, args ...string) (io.ReadCloser, error)
}
diff --git a/internal/orchestrator/restore_errors_test.go b/internal/orchestrator/restore_errors_test.go
index c570ca30..bcc34990 100644
--- a/internal/orchestrator/restore_errors_test.go
+++ b/internal/orchestrator/restore_errors_test.go
@@ -55,7 +55,7 @@ func TestRunRestoreCommandStream_UsesStreamingRunner(t *testing.T) {
if err != nil {
t.Fatalf("createXZReader: %v", err)
}
- defer reader.(io.Closer).Close()
+ defer reader.Close()
buf, err := io.ReadAll(reader)
if err != nil {
From 1e1610d0cb91cfdd2d408d513f0b149de5d240aa Mon Sep 17 00:00:00 2001
From: Damiano <71268257+tis24dev@users.noreply.github.com>
Date: Tue, 5 May 2026 22:15:54 +0200
Subject: [PATCH 25/35] Convert storage/VM config apply to pvesh flag args
Refactor how VM and storage configs are applied: storageBlock now stores Type and proxmoxNotificationEntry entries instead of raw data, and VM/storage config files are parsed into --key=value pvesh arguments rather than written to temp files and passed via --filename/-conf. Added parsing helpers (parseColonConfigLine, pveshArgsFromColonConfigFile/Lines, pveshArgsFromProxmoxEntries, storageBlockPveshArgs, storageEntryValue) and updated applyVMConfigs/applyStorageCfg to build proper pvesh calls (e.g. "pvesh create /storage --storage=... --type=... ..."). Tests were updated to reflect the new argument format and to assert that deprecated flags (--filename, -conf) are no longer used.
---
.../orchestrator/additional_helpers_test.go | 4 +-
.../orchestrator/restore_cluster_apply.go | 125 +++++++++++++++---
.../restore_cluster_apply_additional_test.go | 2 +-
.../restore_coverage_extra_test.go | 26 ++--
internal/orchestrator/restore_errors_test.go | 20 +++
internal/orchestrator/restore_test.go | 6 +
6 files changed, 151 insertions(+), 32 deletions(-)
diff --git a/internal/orchestrator/additional_helpers_test.go b/internal/orchestrator/additional_helpers_test.go
index 9e44f7f4..39286f81 100644
--- a/internal/orchestrator/additional_helpers_test.go
+++ b/internal/orchestrator/additional_helpers_test.go
@@ -828,8 +828,8 @@ storage: backup
if blocks[0].ID != "local" || blocks[1].ID != "backup" {
t.Fatalf("unexpected IDs: %+v", blocks)
}
- if len(blocks[0].data) == 0 || len(blocks[1].data) == 0 {
- t.Fatalf("expected data in blocks")
+ if len(blocks[0].entries) == 0 || len(blocks[1].entries) == 0 {
+ t.Fatalf("expected entries in blocks")
}
// Empty file -> zero blocks
diff --git a/internal/orchestrator/restore_cluster_apply.go b/internal/orchestrator/restore_cluster_apply.go
index 227331b4..bf9e3853 100644
--- a/internal/orchestrator/restore_cluster_apply.go
+++ b/internal/orchestrator/restore_cluster_apply.go
@@ -190,7 +190,13 @@ func applyVMConfigs(ctx context.Context, entries []vmEntry, logger *logging.Logg
return applied, failed
}
target := fmt.Sprintf("/nodes/%s/%s/%s/config", detectNodeForVM(), vm.Kind, vm.VMID)
- args := []string{"set", target, "--filename", vm.Path}
+ configArgs, err := pveshArgsFromColonConfigFile(vm.Path)
+ if err != nil {
+ logger.Warning("Failed to read %s (vmid=%s kind=%s): %v", vm.Path, vm.VMID, vm.Kind, err)
+ failed++
+ continue
+ }
+ args := append([]string{"set", target}, configArgs...)
if err := runPvesh(ctx, logger, args); err != nil {
logger.Warning("Failed to apply %s (vmid=%s kind=%s): %v", target, vm.VMID, vm.Kind, err)
failed++
@@ -216,8 +222,90 @@ func detectNodeForVM() string {
}
type storageBlock struct {
- ID string
- data []string
+ ID string
+ Type string
+ entries []proxmoxNotificationEntry
+}
+
+func pveshArgsFromColonConfigFile(path string) ([]string, error) {
+ data, err := restoreFS.ReadFile(path)
+ if err != nil {
+ return nil, err
+ }
+ return pveshArgsFromColonConfigLines(strings.Split(string(data), "\n")), nil
+}
+
+func pveshArgsFromColonConfigLines(lines []string) []string {
+ args := make([]string, 0, len(lines)*2)
+ for _, line := range lines {
+ key, value, ok := parseColonConfigLine(line)
+ if !ok {
+ continue
+ }
+ args = append(args, fmt.Sprintf("--%s=%s", key, value))
+ }
+ return args
+}
+
+func pveshArgsFromProxmoxEntries(entries []proxmoxNotificationEntry) []string {
+ args := make([]string, 0, len(entries)*2)
+ for _, entry := range entries {
+ key := strings.TrimSpace(entry.Key)
+ value := strings.TrimSpace(entry.Value)
+ if key == "" || value == "" {
+ continue
+ }
+ args = append(args, fmt.Sprintf("--%s=%s", key, value))
+ }
+ return args
+}
+
+func storageBlockPveshArgs(block storageBlock) ([]string, bool) {
+ storageType := strings.TrimSpace(block.Type)
+ if storageType == "" {
+ storageType = storageEntryValue(block.entries, "type")
+ }
+ if storageType == "" {
+ return nil, false
+ }
+
+ args := []string{
+ fmt.Sprintf("--storage=%s", block.ID),
+ fmt.Sprintf("--type=%s", storageType),
+ }
+ for _, entry := range block.entries {
+ if strings.EqualFold(strings.TrimSpace(entry.Key), "type") {
+ continue
+ }
+ args = append(args, pveshArgsFromProxmoxEntries([]proxmoxNotificationEntry{entry})...)
+ }
+ return args, true
+}
+
+func storageEntryValue(entries []proxmoxNotificationEntry, want string) string {
+ for _, entry := range entries {
+ if strings.EqualFold(strings.TrimSpace(entry.Key), want) {
+ return strings.TrimSpace(entry.Value)
+ }
+ }
+ return ""
+}
+
+func parseColonConfigLine(line string) (key, value string, ok bool) {
+ trimmed := strings.TrimSpace(line)
+ if trimmed == "" || strings.HasPrefix(trimmed, "#") {
+ return "", "", false
+ }
+ idx := strings.Index(trimmed, ":")
+ if idx <= 0 {
+ return "", "", false
+ }
+ key = strings.TrimSpace(trimmed[:idx])
+ value = strings.TrimSpace(trimmed[idx+1:])
+ if key == "" || value == "" {
+ return "", "", false
+ }
+ return key, value, true
}
func applyStorageCfg(ctx context.Context, cfgPath string, logger *logging.Logger) (applied, failed int, err error) {
@@ -231,21 +319,14 @@ func applyStorageCfg(ctx context.Context, cfgPath string, logger *logging.Logger
}
for _, blk := range blocks {
- tmp, tmpErr := restoreFS.CreateTemp("", fmt.Sprintf("pve-storage-%s-*.cfg", sanitizeID(blk.ID)))
- if tmpErr != nil {
+ createArgs, ok := storageBlockPveshArgs(blk)
+ if !ok {
+ logger.Warning("Skipping storage %s: storage type missing", blk.ID)
failed++
continue
}
- tmpName := tmp.Name()
- if _, werr := tmp.WriteString(strings.Join(blk.data, "\n") + "\n"); werr != nil {
- _ = tmp.Close()
- _ = restoreFS.Remove(tmpName)
- failed++
- continue
- }
- _ = tmp.Close()
+ args := append([]string{"create", "/storage"}, createArgs...)
- args := []string{"set", fmt.Sprintf("/cluster/storage/%s", blk.ID), "-conf", tmpName}
if runErr := runPvesh(ctx, logger, args); runErr != nil {
logger.Warning("Failed to apply storage %s: %v", blk.ID, runErr)
failed++
@@ -254,8 +335,6 @@ func applyStorageCfg(ctx context.Context, cfgPath string, logger *logging.Logger
applied++
}
- _ = restoreFS.Remove(tmpName)
-
if err := ctx.Err(); err != nil {
return applied, failed, err
}
@@ -289,14 +368,22 @@ func parseStorageBlocks(cfgPath string) ([]storageBlock, error) {
// storage.cfg blocks use `: ` (e.g. `dir: local`, `nfs: backup`).
// Older exports may still use `storage: ` blocks.
- _, name, ok := parseSectionHeader(trimmed)
+ typ, name, ok := parseSectionHeader(trimmed)
if ok {
flush()
- current = &storageBlock{ID: name, data: []string{line}}
+ storageType := ""
+ if !strings.EqualFold(typ, "storage") {
+ storageType = typ
+ }
+ current = &storageBlock{ID: name, Type: storageType}
continue
}
if current != nil {
- current.data = append(current.data, line)
+ key, value := parseProxmoxNotificationKV(trimmed)
+ if strings.TrimSpace(key) == "" {
+ continue
+ }
+ current.entries = append(current.entries, proxmoxNotificationEntry{Key: key, Value: value})
}
}
flush()
diff --git a/internal/orchestrator/restore_cluster_apply_additional_test.go b/internal/orchestrator/restore_cluster_apply_additional_test.go
index 06a61e68..d9d5ac62 100644
--- a/internal/orchestrator/restore_cluster_apply_additional_test.go
+++ b/internal/orchestrator/restore_cluster_apply_additional_test.go
@@ -53,7 +53,7 @@ func TestRunSafeClusterApplyWithUI_SkipsStorageDatacenterWhenStoragePVEStaged(t
}
for _, call := range runner.calls {
- if strings.Contains(call, "/cluster/storage") || strings.Contains(call, "/cluster/config") {
+ if strings.Contains(call, "pvesh create /storage") || strings.Contains(call, "pvesh set /storage") || strings.Contains(call, "/cluster/config") {
t.Fatalf("storage/datacenter apply should be skipped for storage_pve staged restore; calls=%#v", runner.calls)
}
}
diff --git a/internal/orchestrator/restore_coverage_extra_test.go b/internal/orchestrator/restore_coverage_extra_test.go
index 8225f75a..e21566cc 100644
--- a/internal/orchestrator/restore_coverage_extra_test.go
+++ b/internal/orchestrator/restore_coverage_extra_test.go
@@ -359,10 +359,10 @@ func TestRunSafeClusterApply_AppliesVMStorageAndDatacenterConfigs(t *testing.T)
}
wantPrefixes := []string{
- "pvesh set /nodes/" + node + "/qemu/100/config --filename ",
- "pvesh set /nodes/" + node + "/lxc/101/config --filename ",
- "pvesh set /cluster/storage/local -conf ",
- "pvesh set /cluster/storage/backup_ext -conf ",
+ "pvesh set /nodes/" + node + "/qemu/100/config --name=vm100",
+ "pvesh set /nodes/" + node + "/lxc/101/config --hostname=ct101",
+ "pvesh create /storage --storage=local --type=dir --path=/var/lib/vz",
+ "pvesh create /storage --storage=backup_ext --type=nfs --server=10.0.0.1",
"pvesh set /cluster/config -conf ",
}
for _, prefix := range wantPrefixes {
@@ -377,6 +377,14 @@ func TestRunSafeClusterApply_AppliesVMStorageAndDatacenterConfigs(t *testing.T)
t.Fatalf("expected a call with prefix %q; calls=%#v", prefix, runner.calls)
}
}
+ for _, call := range runner.calls {
+ if strings.Contains(call, "--filename") {
+ t.Fatalf("VM/CT apply must not use invalid --filename flag; calls=%#v", runner.calls)
+ }
+ if strings.Contains(call, "/cluster/storage/") || (strings.Contains(call, " -conf ") && strings.Contains(call, "storage")) {
+ t.Fatalf("storage apply must not use invalid cluster storage path or -conf flag; calls=%#v", runner.calls)
+ }
+ }
}
func TestRunSafeClusterApply_AppliesPoolsFromUserCfg(t *testing.T) {
@@ -531,11 +539,10 @@ func TestRunSafeClusterApply_UsesSingleExportedNodeWhenHostnameMismatch(t *testi
t.Fatalf("runSafeClusterApply error: %v", err)
}
- wantPrefix := "pvesh set /nodes/" + targetNode + "/qemu/100/config --filename "
- wantSourceSuffix := filepath.Join("etc", "pve", "nodes", sourceNode, "qemu-server", "100.conf")
+ wantPrefix := "pvesh set /nodes/" + targetNode + "/qemu/100/config --name=vm100"
found := false
for _, call := range runner.calls {
- if strings.HasPrefix(call, wantPrefix) && strings.Contains(call, wantSourceSuffix) {
+ if strings.HasPrefix(call, wantPrefix) {
found = true
break
}
@@ -593,11 +600,10 @@ func TestRunSafeClusterApply_PromptsForSourceNodeWhenMultipleExportNodes(t *test
t.Fatalf("runSafeClusterApply error: %v", err)
}
- wantPrefix := "pvesh set /nodes/" + targetNode + "/qemu/101/config --filename "
- wantSourceSuffix := filepath.Join("etc", "pve", "nodes", sourceNode2, "qemu-server", "101.conf")
+ wantPrefix := "pvesh set /nodes/" + targetNode + "/qemu/101/config --name=vm101"
found := false
for _, call := range runner.calls {
- if strings.HasPrefix(call, wantPrefix) && strings.Contains(call, wantSourceSuffix) {
+ if strings.HasPrefix(call, wantPrefix) {
found = true
break
}
diff --git a/internal/orchestrator/restore_errors_test.go b/internal/orchestrator/restore_errors_test.go
index bcc34990..31a2d9ce 100644
--- a/internal/orchestrator/restore_errors_test.go
+++ b/internal/orchestrator/restore_errors_test.go
@@ -551,9 +551,11 @@ func TestApplyStorageCfg_WithMultipleBlocks(t *testing.T) {
// Write storage config with multiple blocks
cfgPath := filepath.Join(t.TempDir(), "storage.cfg")
content := `storage: local
+ type dir
path /var/lib/vz
storage: backup
+ type nfs
path /mnt/backup
`
if err := os.WriteFile(cfgPath, []byte(content), 0o644); err != nil {
@@ -570,6 +572,16 @@ storage: backup
if applied != 2 {
t.Fatalf("expected 2 applied, got %d (failed=%d)", applied, failed)
}
+ calls := strings.Join(restoreCmd.(*FakeCommandRunner).CallsList(), "\n")
+ if strings.Contains(calls, " -conf ") {
+ t.Fatalf("storage apply must not use -conf; calls=%s", calls)
+ }
+ if !strings.Contains(calls, "pvesh create /storage --storage=local --type=dir --path=/var/lib/vz") {
+ t.Fatalf("missing local storage args; calls=%s", calls)
+ }
+ if !strings.Contains(calls, "pvesh create /storage --storage=backup --type=nfs --path=/mnt/backup") {
+ t.Fatalf("missing backup storage args; calls=%s", calls)
+ }
}
func TestApplyStorageCfg_PveshError(t *testing.T) {
@@ -583,6 +595,7 @@ func TestApplyStorageCfg_PveshError(t *testing.T) {
cfgPath := filepath.Join(t.TempDir(), "storage.cfg")
content := `storage: local
+ type dir
path /var/lib/vz
`
if err := os.WriteFile(cfgPath, []byte(content), 0o644); err != nil {
@@ -1711,6 +1724,13 @@ func TestApplyVMConfigs_SuccessfulApply(t *testing.T) {
if applied != 1 || failed != 0 {
t.Fatalf("expected (1,0), got (%d,%d)", applied, failed)
}
+ calls := strings.Join(fake.CallsList(), "\n")
+ if strings.Contains(calls, "--filename") {
+ t.Fatalf("VM apply must not use --filename; calls=%s", calls)
+ }
+ if !strings.Contains(calls, "pvesh set /nodes/") || !strings.Contains(calls, "/qemu/100/config --name=test-vm") {
+ t.Fatalf("missing VM config args; calls=%s", calls)
+ }
}
// --------------------------------------------------------------------------
diff --git a/internal/orchestrator/restore_test.go b/internal/orchestrator/restore_test.go
index 36d2387b..7ad785b3 100644
--- a/internal/orchestrator/restore_test.go
+++ b/internal/orchestrator/restore_test.go
@@ -1180,6 +1180,9 @@ nfs: nfs-backup
if blocks[0].ID != "local" || blocks[1].ID != "nfs-backup" {
t.Fatalf("unexpected block IDs: %v, %v", blocks[0].ID, blocks[1].ID)
}
+ if blocks[0].Type != "dir" || blocks[1].Type != "nfs" {
+ t.Fatalf("unexpected block types: %v, %v", blocks[0].Type, blocks[1].Type)
+ }
}
func TestParseStorageBlocks_LegacyStoragePrefix(t *testing.T) {
@@ -1207,6 +1210,9 @@ func TestParseStorageBlocks_LegacyStoragePrefix(t *testing.T) {
if blocks[0].ID != "local" {
t.Fatalf("unexpected block ID: %v", blocks[0].ID)
}
+ if blocks[0].Type != "" {
+ t.Fatalf("legacy storage block type = %q, want empty because type is in entries", blocks[0].Type)
+ }
}
// --------------------------------------------------------------------------
From 156b14e2519e105b76e3293eb1b3d9a8532207cf Mon Sep 17 00:00:00 2001
From: Damiano <71268257+tis24dev@users.noreply.github.com>
Date: Wed, 6 May 2026 00:18:09 +0200
Subject: [PATCH 26/35] Add ExitEncryptionError and classify encryption
failures
Add a new ExitEncryptionError exit code and string mapping to represent encryption setup/processing failures. Update createBackupArchive to report the "encryption" phase and use ExitEncryptionError when age recipient preparation fails. Add tests: backup_run_phases_test verifies age-recipient failures are classified as encryption, and exit_codes_test updated to include the new code. Also fix RunGoBackup to return the collected stats value when prepareBackupWorkspace fails so callers receive consistent results.
---
internal/orchestrator/backup_run_phases.go | 2 +-
.../orchestrator/backup_run_phases_test.go | 36 +++++++++++++++++++
internal/orchestrator/orchestrator.go | 2 +-
internal/types/exit_codes.go | 5 +++
internal/types/exit_codes_test.go | 2 ++
5 files changed, 45 insertions(+), 2 deletions(-)
create mode 100644 internal/orchestrator/backup_run_phases_test.go
diff --git a/internal/orchestrator/backup_run_phases.go b/internal/orchestrator/backup_run_phases.go
index d2e5d47e..959ad204 100644
--- a/internal/orchestrator/backup_run_phases.go
+++ b/internal/orchestrator/backup_run_phases.go
@@ -226,7 +226,7 @@ func (o *Orchestrator) createBackupArchive(run *backupRunContext, workspace *bac
ageRecipients, err := o.prepareAgeRecipients(run.ctx)
if err != nil {
- return nil, &BackupError{Phase: "config", Err: err, Code: types.ExitConfigError}
+ return nil, &BackupError{Phase: "encryption", Err: err, Code: types.ExitEncryptionError}
}
archiverConfig := o.buildBackupArchiverConfig(run, ageRecipients)
diff --git a/internal/orchestrator/backup_run_phases_test.go b/internal/orchestrator/backup_run_phases_test.go
new file mode 100644
index 00000000..581a408a
--- /dev/null
+++ b/internal/orchestrator/backup_run_phases_test.go
@@ -0,0 +1,36 @@
+package orchestrator
+
+import (
+ "context"
+ "errors"
+ "testing"
+
+ "github.com/tis24dev/proxsave/internal/config"
+ "github.com/tis24dev/proxsave/internal/types"
+)
+
+func TestCreateBackupArchiveClassifiesAgeRecipientFailureAsEncryption(t *testing.T) {
+ orch := New(newTestLogger(), false)
+ orch.SetConfig(&config.Config{
+ EncryptArchive: true,
+ BaseDir: t.TempDir(),
+ })
+ orch.SetBackupConfig(t.TempDir(), t.TempDir(), types.CompressionNone, 0, 0, "standard", nil)
+
+ run := orch.newBackupRunContext(context.Background(), nil, "test-host")
+ _, err := orch.createBackupArchive(run, &backupWorkspace{tempDir: t.TempDir()})
+ if err == nil {
+ t.Fatal("expected createBackupArchive error")
+ }
+
+ var backupErr *BackupError
+ if !errors.As(err, &backupErr) {
+ t.Fatalf("expected BackupError, got %T: %v", err, err)
+ }
+ if backupErr.Phase != "encryption" {
+ t.Fatalf("Phase=%q; want encryption", backupErr.Phase)
+ }
+ if backupErr.Code != types.ExitEncryptionError {
+ t.Fatalf("Code=%v; want %v", backupErr.Code, types.ExitEncryptionError)
+ }
+}
diff --git a/internal/orchestrator/orchestrator.go b/internal/orchestrator/orchestrator.go
index 985d1fdb..de9a9548 100644
--- a/internal/orchestrator/orchestrator.go
+++ b/internal/orchestrator/orchestrator.go
@@ -521,7 +521,7 @@ func (o *Orchestrator) RunGoBackup(ctx context.Context, envInfo *environment.Env
}()
if err := o.prepareBackupWorkspace(run, workspace); err != nil {
- return nil, err
+ return stats, err
}
defer func() {
o.cleanupBackupWorkspace(workspace)
diff --git a/internal/types/exit_codes.go b/internal/types/exit_codes.go
index 439c5f58..b91b84ac 100644
--- a/internal/types/exit_codes.go
+++ b/internal/types/exit_codes.go
@@ -49,6 +49,9 @@ const (
// ExitSecurityError - Errors detected by the security check.
ExitSecurityError ExitCode = 14
+
+ // ExitEncryptionError - Error during encryption setup or processing.
+ ExitEncryptionError ExitCode = 15
)
// String returns a human-readable description of the exit code.
@@ -84,6 +87,8 @@ func (e ExitCode) String() string {
return "panic error"
case ExitSecurityError:
return "security error"
+ case ExitEncryptionError:
+ return "encryption error"
default:
return "unknown error"
}
diff --git a/internal/types/exit_codes_test.go b/internal/types/exit_codes_test.go
index bda518c5..2cb8f7f6 100644
--- a/internal/types/exit_codes_test.go
+++ b/internal/types/exit_codes_test.go
@@ -17,6 +17,7 @@ func TestExitCodeString(t *testing.T) {
{"network error", ExitNetworkError, "network error"},
{"permission error", ExitPermissionError, "permission error"},
{"verification error", ExitVerificationError, "verification error"},
+ {"encryption error", ExitEncryptionError, "encryption error"},
{"unknown", ExitCode(99), "unknown error"},
}
@@ -45,6 +46,7 @@ func TestExitCodeInt(t *testing.T) {
{"network error", ExitNetworkError, 6},
{"permission error", ExitPermissionError, 7},
{"verification error", ExitVerificationError, 8},
+ {"encryption error", ExitEncryptionError, 15},
}
for _, tt := range tests {
From 2b70b8093002620a1366d6b257fe038a1da86d41 Mon Sep 17 00:00:00 2001
From: Damiano <71268257+tis24dev@users.noreply.github.com>
Date: Wed, 6 May 2026 00:26:35 +0200
Subject: [PATCH 27/35] Return error from checksum write; add tests
Change writeArchiveChecksum to return an error and propagate failures: writeArchiveChecksum now returns a wrapped error on write failure (uses 0o640 file mode) and verifyAndWriteBackupArtifacts returns a BackupError when checksum writing fails. Make server identity detection injectable for testing by introducing runtimeServerIdentityDetector and only run detection when ServerID is not configured; add unit tests for initializeServerIdentity. Add unit test ensuring checksum write errors are propagated. Also add Node.js 24 setup to the Dependabot automerge workflow so Dependabot metadata step runs under Node 24.
---
.github/workflows/dependabot-automerge.yml | 5 ++
cmd/proxsave/main_identity.go | 16 ++++--
cmd/proxsave/main_identity_test.go | 54 +++++++++++++++++++
internal/orchestrator/backup_run_helpers.go | 10 ++--
internal/orchestrator/backup_run_phases.go | 8 ++-
.../orchestrator/backup_run_phases_test.go | 24 +++++++++
6 files changed, 106 insertions(+), 11 deletions(-)
create mode 100644 cmd/proxsave/main_identity_test.go
diff --git a/.github/workflows/dependabot-automerge.yml b/.github/workflows/dependabot-automerge.yml
index 74f957de..c5264029 100644
--- a/.github/workflows/dependabot-automerge.yml
+++ b/.github/workflows/dependabot-automerge.yml
@@ -16,6 +16,11 @@ jobs:
if: github.actor == 'dependabot[bot]'
steps:
+ - name: Set up Node.js 24
+ uses: actions/setup-node@v4
+ with:
+ node-version: '24'
+
- name: Fetch Dependabot metadata
id: metadata
uses: dependabot/fetch-metadata@v3
diff --git a/cmd/proxsave/main_identity.go b/cmd/proxsave/main_identity.go
index 397f09bd..ca53fc12 100644
--- a/cmd/proxsave/main_identity.go
+++ b/cmd/proxsave/main_identity.go
@@ -10,15 +10,21 @@ import (
"github.com/tis24dev/proxsave/internal/notify"
)
+var runtimeServerIdentityDetector = detectServerIdentity
+
func initializeServerIdentity(rt *appRuntime) {
- identityInfo := detectServerIdentity(rt)
rt.serverIDValue = strings.TrimSpace(rt.cfg.ServerID)
rt.serverMACValue = ""
- if identityInfo != nil {
- applyDetectedIdentity(rt, identityInfo)
- }
- if rt.serverIDValue != "" && rt.cfg.ServerID == "" {
+ if rt.serverIDValue != "" {
rt.cfg.ServerID = rt.serverIDValue
+ } else {
+ identityInfo := runtimeServerIdentityDetector(rt)
+ if identityInfo != nil {
+ applyDetectedIdentity(rt, identityInfo)
+ }
+ if rt.serverIDValue != "" {
+ rt.cfg.ServerID = rt.serverIDValue
+ }
}
logServerIdentityValues(rt.serverIDValue, rt.serverMACValue)
diff --git a/cmd/proxsave/main_identity_test.go b/cmd/proxsave/main_identity_test.go
new file mode 100644
index 00000000..fd89a782
--- /dev/null
+++ b/cmd/proxsave/main_identity_test.go
@@ -0,0 +1,54 @@
+package main
+
+import (
+ "testing"
+
+ "github.com/tis24dev/proxsave/internal/config"
+ "github.com/tis24dev/proxsave/internal/identity"
+)
+
+func TestInitializeServerIdentityKeepsConfiguredServerID(t *testing.T) {
+ origDetector := runtimeServerIdentityDetector
+ t.Cleanup(func() { runtimeServerIdentityDetector = origDetector })
+
+ called := false
+ runtimeServerIdentityDetector = func(*appRuntime) *identity.Info {
+ called = true
+ return &identity.Info{ServerID: "detected", PrimaryMAC: "00:11:22:33:44:55"}
+ }
+
+ rt := &appRuntime{cfg: &config.Config{ServerID: " configured "}}
+ initializeServerIdentity(rt)
+
+ if called {
+ t.Fatal("detector should not run when ServerID is explicitly configured")
+ }
+ if rt.serverIDValue != "configured" {
+ t.Fatalf("serverIDValue=%q; want configured", rt.serverIDValue)
+ }
+ if rt.cfg.ServerID != "configured" {
+ t.Fatalf("cfg.ServerID=%q; want trimmed configured", rt.cfg.ServerID)
+ }
+}
+
+func TestInitializeServerIdentityStoresDetectedServerID(t *testing.T) {
+ origDetector := runtimeServerIdentityDetector
+ t.Cleanup(func() { runtimeServerIdentityDetector = origDetector })
+
+ runtimeServerIdentityDetector = func(*appRuntime) *identity.Info {
+ return &identity.Info{ServerID: "detected", PrimaryMAC: "00:11:22:33:44:55"}
+ }
+
+ rt := &appRuntime{cfg: &config.Config{}}
+ initializeServerIdentity(rt)
+
+ if rt.serverIDValue != "detected" {
+ t.Fatalf("serverIDValue=%q; want detected", rt.serverIDValue)
+ }
+ if rt.cfg.ServerID != "detected" {
+ t.Fatalf("cfg.ServerID=%q; want detected", rt.cfg.ServerID)
+ }
+ if rt.serverMACValue != "00:11:22:33:44:55" {
+ t.Fatalf("serverMACValue=%q; want detected MAC", rt.serverMACValue)
+ }
+}
diff --git a/internal/orchestrator/backup_run_helpers.go b/internal/orchestrator/backup_run_helpers.go
index 8591bd7a..32103742 100644
--- a/internal/orchestrator/backup_run_helpers.go
+++ b/internal/orchestrator/backup_run_helpers.go
@@ -259,13 +259,13 @@ func (o *Orchestrator) generateArchiveChecksum(ctx context.Context, archivePath
return checksum, nil
}
-func (o *Orchestrator) writeArchiveChecksum(workspace *backupWorkspace, artifacts *backupArtifacts, checksum string) {
+func (o *Orchestrator) writeArchiveChecksum(workspace *backupWorkspace, artifacts *backupArtifacts, checksum string) error {
checksumContent := fmt.Sprintf("%s %s\n", checksum, filepath.Base(artifacts.archivePath))
- if err := workspace.fs.WriteFile(artifacts.checksumPath, []byte(checksumContent), 0640); err != nil {
- o.logger.Warning("Failed to write checksum file %s: %v", artifacts.checksumPath, err)
- } else {
- o.logger.Debug("Checksum file written to %s", artifacts.checksumPath)
+ if err := workspace.fs.WriteFile(artifacts.checksumPath, []byte(checksumContent), 0o640); err != nil {
+ return fmt.Errorf("write checksum file %s: %w", artifacts.checksumPath, err)
}
+ o.logger.Debug("Checksum file written to %s", artifacts.checksumPath)
+ return nil
}
func (o *Orchestrator) writeArchiveManifest(run *backupRunContext, artifacts *backupArtifacts, checksum string) error {
diff --git a/internal/orchestrator/backup_run_phases.go b/internal/orchestrator/backup_run_phases.go
index 959ad204..0e681583 100644
--- a/internal/orchestrator/backup_run_phases.go
+++ b/internal/orchestrator/backup_run_phases.go
@@ -271,7 +271,13 @@ func (o *Orchestrator) verifyAndWriteBackupArtifacts(run *backupRunContext, work
}
stats.Checksum = checksum
- o.writeArchiveChecksum(workspace, artifacts, checksum)
+ if err := o.writeArchiveChecksum(workspace, artifacts, checksum); err != nil {
+ return &BackupError{
+ Phase: "verification",
+ Err: err,
+ Code: types.ExitVerificationError,
+ }
+ }
if err := o.writeArchiveManifest(run, artifacts, checksum); err != nil {
return err
}
diff --git a/internal/orchestrator/backup_run_phases_test.go b/internal/orchestrator/backup_run_phases_test.go
index 581a408a..dcfc270c 100644
--- a/internal/orchestrator/backup_run_phases_test.go
+++ b/internal/orchestrator/backup_run_phases_test.go
@@ -3,6 +3,7 @@ package orchestrator
import (
"context"
"errors"
+ "strings"
"testing"
"github.com/tis24dev/proxsave/internal/config"
@@ -34,3 +35,26 @@ func TestCreateBackupArchiveClassifiesAgeRecipientFailureAsEncryption(t *testing
t.Fatalf("Code=%v; want %v", backupErr.Code, types.ExitEncryptionError)
}
}
+
+func TestWriteArchiveChecksumPropagatesWriteError(t *testing.T) {
+ orch := New(newTestLogger(), false)
+ checksumPath := "/backups/test.tar.sha256"
+ writeErr := errors.New("disk full")
+ fakeFS := NewFakeFS()
+ t.Cleanup(func() { _ = fakeFS.Cleanup() })
+
+ err := orch.writeArchiveChecksum(
+ &backupWorkspace{fs: writeFileFailFS{FS: fakeFS, failPath: checksumPath, err: writeErr}},
+ &backupArtifacts{archivePath: "/backups/test.tar", checksumPath: checksumPath},
+ "abc123",
+ )
+ if err == nil {
+ t.Fatal("expected writeArchiveChecksum error")
+ }
+ if !errors.Is(err, writeErr) {
+ t.Fatalf("expected wrapped write error, got %v", err)
+ }
+ if !strings.Contains(err.Error(), checksumPath) {
+ t.Fatalf("expected checksum path in error, got %q", err.Error())
+ }
+}
From 532ebe49dfd45cb910053ac66a198dcaf0ab6020 Mon Sep 17 00:00:00 2001
From: Damiano <71268257+tis24dev@users.noreply.github.com>
Date: Wed, 6 May 2026 00:39:06 +0200
Subject: [PATCH 28/35] Propagate context cancellation in collectors
Add isContextCancellationError helper and use it across PVE collection bricks and storage/metadata steps so context cancellations are propagated (returning the ctx error) instead of being treated as regular failures. Add tests ensuring guest and storage bricks propagate cancellations. Include new standalone collection flags (BackupPBSNotificationsPriv, BackupRootHome, BackupScriptRepository, BackupUserHomes) in collectionOptionFlags and add tests validating they are accepted. Change validateCloudSettings to accept absolute cloud remote refs with a new isAbsoluteCloudRemoteRef helper and add tests for absolute refs and path validation. Remove unused setBackupResult from main_state.go and update related imports in tests.
---
cmd/proxsave/main_state.go | 6 ---
internal/backup/collector.go | 4 +-
internal/backup/collector_bricks.go | 15 +++++++
internal/backup/collector_bricks_pve.go | 9 ++++
.../backup/collector_bricks_pve_finalize.go | 18 ++++++++
internal/backup/collector_bricks_pve_jobs.go | 24 +++++++++++
.../backup/collector_bricks_pve_storage.go | 15 +++++++
internal/backup/collector_bricks_test.go | 43 +++++++++++++++++++
.../backup/collector_config_extra_test.go | 20 +++++++++
internal/backup/collector_pve.go | 12 ++++++
internal/config/config.go | 25 +++++++++--
internal/config/config_test.go | 24 +++++++++++
12 files changed, 205 insertions(+), 10 deletions(-)
diff --git a/cmd/proxsave/main_state.go b/cmd/proxsave/main_state.go
index b577e6e8..cef9c602 100644
--- a/cmd/proxsave/main_state.go
+++ b/cmd/proxsave/main_state.go
@@ -72,12 +72,6 @@ func (state *appRunState) finalize(code int) int {
return code
}
-func (state *appRunState) setBackupResult(result backupModeResult) {
- state.orch = result.orch
- state.earlyErrorState = result.earlyErrorState
- state.pendingSupportStat = result.supportStats
-}
-
func (state *appRunState) applyModeResult(result modeResult) {
state.orch = result.orch
state.earlyErrorState = result.earlyErrorState
diff --git a/internal/backup/collector.go b/internal/backup/collector.go
index da2d7faf..7bcbfad7 100644
--- a/internal/backup/collector.go
+++ b/internal/backup/collector.go
@@ -313,7 +313,8 @@ func (c *CollectorConfig) collectionOptionFlags() []bool {
c.BackupPVEBackupFiles, c.BackupCephConfig,
c.BackupDatastoreConfigs, c.BackupPBSS3Endpoints, c.BackupPBSNodeConfig,
c.BackupPBSAcmeAccounts, c.BackupPBSAcmePlugins, c.BackupPBSMetricServers,
- c.BackupPBSTrafficControl, c.BackupPBSNotifications, c.BackupUserConfigs, c.BackupRemoteConfigs,
+ c.BackupPBSTrafficControl, c.BackupPBSNotifications, c.BackupPBSNotificationsPriv,
+ c.BackupUserConfigs, c.BackupRemoteConfigs,
c.BackupSyncJobs, c.BackupVerificationJobs, c.BackupTapeConfigs,
c.BackupPBSNetworkConfig, c.BackupPruneSchedules, c.BackupPxarFiles,
c.BackupNetworkConfigs, c.BackupAptSources, c.BackupCronJobs,
@@ -321,6 +322,7 @@ func (c *CollectorConfig) collectionOptionFlags() []bool {
c.BackupKernelModules, c.BackupFirewallRules,
c.BackupInstalledPackages, c.BackupScriptDir, c.BackupCriticalFiles,
c.BackupSSHKeys, c.BackupZFSConfig, c.BackupConfigFile,
+ c.BackupRootHome, c.BackupScriptRepository, c.BackupUserHomes,
}
}
diff --git a/internal/backup/collector_bricks.go b/internal/backup/collector_bricks.go
index 7b2136af..a08c530c 100644
--- a/internal/backup/collector_bricks.go
+++ b/internal/backup/collector_bricks.go
@@ -3,6 +3,7 @@ package backup
import (
"context"
+ "errors"
"fmt"
)
@@ -306,6 +307,20 @@ func runRecipe(ctx context.Context, r recipe, state *collectionState) error {
return nil
}
+func isContextCancellationError(ctx context.Context, err error) bool {
+ if err == nil {
+ return false
+ }
+ if errors.Is(err, context.Canceled) || errors.Is(err, context.DeadlineExceeded) {
+ return true
+ }
+ if ctx == nil {
+ return false
+ }
+ ctxErr := ctx.Err()
+ return ctxErr != nil && errors.Is(err, ctxErr)
+}
+
func (s *collectionState) ensurePVECommandsDir() (string, error) {
if s.pve.commandsDir != "" {
return s.pve.commandsDir, nil
diff --git a/internal/backup/collector_bricks_pve.go b/internal/backup/collector_bricks_pve.go
index b2488546..4218cd0c 100644
--- a/internal/backup/collector_bricks_pve.go
+++ b/internal/backup/collector_bricks_pve.go
@@ -175,6 +175,9 @@ func newPVEGuestBricks() []collectionBrick {
}
c.logger.Info("Collecting VM and container configurations")
if err := c.collectPVEQEMUConfigs(ctx); err != nil {
+ if isContextCancellationError(ctx, err) {
+ return err
+ }
c.logger.Warning("Failed to collect QEMU VM configs: %v", err)
state.pve.guestCollectionAborted = true
}
@@ -190,6 +193,9 @@ func newPVEGuestBricks() []collectionBrick {
return nil
}
if err := c.collectPVELXCConfigs(ctx); err != nil {
+ if isContextCancellationError(ctx, err) {
+ return err
+ }
c.logger.Warning("Failed to collect LXC configs: %v", err)
state.pve.guestCollectionAborted = true
}
@@ -205,6 +211,9 @@ func newPVEGuestBricks() []collectionBrick {
return nil
}
if err := c.collectPVEGuestInventory(ctx); err != nil {
+ if isContextCancellationError(ctx, err) {
+ return err
+ }
c.logger.Warning("Failed to collect guest inventory: %v", err)
state.pve.guestCollectionAborted = true
}
diff --git a/internal/backup/collector_bricks_pve_finalize.go b/internal/backup/collector_bricks_pve_finalize.go
index 9753a921..0c647af4 100644
--- a/internal/backup/collector_bricks_pve_finalize.go
+++ b/internal/backup/collector_bricks_pve_finalize.go
@@ -15,6 +15,9 @@ func newPVECephBricks() []collectionBrick {
}
c.logger.Debug("Collecting Ceph configuration and status")
if err := c.collectPVECephConfigSnapshot(ctx); err != nil {
+ if isContextCancellationError(ctx, err) {
+ return err
+ }
c.logger.Warning("Failed to collect Ceph configuration snapshot: %v", err)
state.pve.cephCollectionAborted = true
}
@@ -30,6 +33,9 @@ func newPVECephBricks() []collectionBrick {
return nil
}
if err := c.collectPVECephRuntime(ctx); err != nil {
+ if isContextCancellationError(ctx, err) {
+ return err
+ }
c.logger.Warning("Failed to collect Ceph runtime information: %v", err)
state.pve.cephCollectionAborted = true
} else {
@@ -50,6 +56,9 @@ func newPVEAliasBricks() []collectionBrick {
c := state.collector
c.logger.Debug("Creating PVE info aliases under /var/lib/pve-cluster/info")
if err := c.createPVECoreAliases(ctx); err != nil {
+ if isContextCancellationError(ctx, err) {
+ return err
+ }
c.logger.Warning("Failed to create PVE core aliases: %v", err)
state.pve.finalizeCollectionAborted = true
}
@@ -70,6 +79,9 @@ func newPVEAggregateBricks() []collectionBrick {
return nil
}
if err := c.createPVEBackupHistoryAggregate(ctx); err != nil {
+ if isContextCancellationError(ctx, err) {
+ return err
+ }
c.logger.Warning("Failed to aggregate PVE backup history: %v", err)
state.pve.finalizeCollectionAborted = true
}
@@ -85,6 +97,9 @@ func newPVEAggregateBricks() []collectionBrick {
return nil
}
if err := c.createPVEReplicationAggregate(ctx); err != nil {
+ if isContextCancellationError(ctx, err) {
+ return err
+ }
c.logger.Warning("Failed to aggregate PVE replication status: %v", err)
state.pve.finalizeCollectionAborted = true
}
@@ -105,6 +120,9 @@ func newPVEVersionBricks() []collectionBrick {
return nil
}
if err := c.createPVEVersionInfo(ctx); err != nil {
+ if isContextCancellationError(ctx, err) {
+ return err
+ }
c.logger.Warning("Failed to write PVE version info: %v", err)
state.pve.finalizeCollectionAborted = true
}
diff --git a/internal/backup/collector_bricks_pve_jobs.go b/internal/backup/collector_bricks_pve_jobs.go
index 826b03dd..69e6cb26 100644
--- a/internal/backup/collector_bricks_pve_jobs.go
+++ b/internal/backup/collector_bricks_pve_jobs.go
@@ -15,6 +15,9 @@ func newPVEBackupJobBricks() []collectionBrick {
}
c.logger.Debug("Collecting PVE job definitions for nodes: %v", state.pve.runtimeNodes())
if err := c.collectPVEBackupJobDefinitions(ctx); err != nil {
+ if isContextCancellationError(ctx, err) {
+ return err
+ }
c.logger.Warning("Failed to collect PVE backup job definitions: %v", err)
state.pve.jobCollectionAborted = true
}
@@ -30,6 +33,9 @@ func newPVEBackupJobBricks() []collectionBrick {
return nil
}
if err := c.collectPVEBackupJobHistory(ctx, state.pve.runtimeNodes()); err != nil {
+ if isContextCancellationError(ctx, err) {
+ return err
+ }
c.logger.Warning("Failed to collect PVE backup history: %v", err)
state.pve.jobCollectionAborted = true
}
@@ -45,6 +51,9 @@ func newPVEBackupJobBricks() []collectionBrick {
return nil
}
if err := c.collectPVEVZDumpCronSnapshot(ctx); err != nil {
+ if isContextCancellationError(ctx, err) {
+ return err
+ }
c.logger.Warning("Failed to collect VZDump cron snapshot: %v", err)
state.pve.jobCollectionAborted = true
}
@@ -65,6 +74,9 @@ func newPVEScheduleBricks() []collectionBrick {
return nil
}
if err := c.collectPVEScheduleCrontab(ctx); err != nil {
+ if isContextCancellationError(ctx, err) {
+ return err
+ }
c.logger.Warning("Failed to collect PVE crontab schedules: %v", err)
state.pve.scheduleCollectionAborted = true
}
@@ -80,6 +92,9 @@ func newPVEScheduleBricks() []collectionBrick {
return nil
}
if err := c.collectPVEScheduleTimers(ctx); err != nil {
+ if isContextCancellationError(ctx, err) {
+ return err
+ }
c.logger.Warning("Failed to collect PVE timer schedules: %v", err)
state.pve.scheduleCollectionAborted = true
}
@@ -95,6 +110,9 @@ func newPVEScheduleBricks() []collectionBrick {
return nil
}
if err := c.collectPVEScheduleCronFiles(ctx); err != nil {
+ if isContextCancellationError(ctx, err) {
+ return err
+ }
c.logger.Warning("Failed to collect PVE cron schedule files: %v", err)
state.pve.scheduleCollectionAborted = true
}
@@ -116,6 +134,9 @@ func newPVEReplicationBricks() []collectionBrick {
}
c.logger.Debug("Collecting PVE replication settings for nodes: %v", state.pve.runtimeNodes())
if err := c.collectPVEReplicationDefinitions(ctx); err != nil {
+ if isContextCancellationError(ctx, err) {
+ return err
+ }
c.logger.Warning("Failed to collect PVE replication definitions: %v", err)
state.pve.replicationCollectionAborted = true
}
@@ -131,6 +152,9 @@ func newPVEReplicationBricks() []collectionBrick {
return nil
}
if err := c.collectPVEReplicationStatus(ctx, state.pve.runtimeNodes()); err != nil {
+ if isContextCancellationError(ctx, err) {
+ return err
+ }
c.logger.Warning("Failed to collect PVE replication status: %v", err)
state.pve.replicationCollectionAborted = true
}
diff --git a/internal/backup/collector_bricks_pve_storage.go b/internal/backup/collector_bricks_pve_storage.go
index 9299ce3f..8cd6c0c6 100644
--- a/internal/backup/collector_bricks_pve_storage.go
+++ b/internal/backup/collector_bricks_pve_storage.go
@@ -56,6 +56,9 @@ func newPVEStorageProbeBricks() []collectionBrick {
for _, storage := range state.pve.resolvedStorages {
result, err := c.preparePVEStorageScan(ctx, storage, baseDir, ioTimeout)
if err != nil {
+ if isContextCancellationError(ctx, err) {
+ return err
+ }
c.logger.Warning("Failed to probe PVE datastore %s: %v", storage.Name, err)
state.pve.storageCollectionAborted = true
return nil
@@ -89,6 +92,9 @@ func newPVEStorageMetadataJSONBricks() []collectionBrick {
continue
}
if err := c.collectPVEStorageMetadataJSONStep(ctx, result, ioTimeout); err != nil {
+ if isContextCancellationError(ctx, err) {
+ return err
+ }
c.logger.Warning("Failed to write PVE datastore JSON metadata for %s: %v", storage.Name, err)
state.pve.storageCollectionAborted = true
return nil
@@ -117,6 +123,9 @@ func newPVEStorageMetadataTextBricks() []collectionBrick {
continue
}
if err := c.collectPVEStorageMetadataTextStep(ctx, result, ioTimeout); err != nil {
+ if isContextCancellationError(ctx, err) {
+ return err
+ }
c.logger.Warning("Failed to write PVE datastore text metadata for %s: %v", storage.Name, err)
state.pve.storageCollectionAborted = true
return nil
@@ -145,6 +154,9 @@ func newPVEStorageAnalysisBricks() []collectionBrick {
continue
}
if err := c.collectPVEStorageBackupAnalysisStep(ctx, result, ioTimeout); err != nil {
+ if isContextCancellationError(ctx, err) {
+ return err
+ }
c.logger.Warning("Detailed backup analysis for %s failed: %v", storage.Name, err)
}
}
@@ -165,6 +177,9 @@ func newPVEStorageSummaryBricks() []collectionBrick {
return nil
}
if err := c.writePVEStorageSummary(ctx, state.pve.probedStorages); err != nil {
+ if isContextCancellationError(ctx, err) {
+ return err
+ }
c.logger.Warning("Failed to write PVE datastore summary: %v", err)
state.pve.storageCollectionAborted = true
return nil
diff --git a/internal/backup/collector_bricks_test.go b/internal/backup/collector_bricks_test.go
index 6270107c..0f559a40 100644
--- a/internal/backup/collector_bricks_test.go
+++ b/internal/backup/collector_bricks_test.go
@@ -10,6 +10,9 @@ import (
"sort"
"strings"
"testing"
+
+ "github.com/tis24dev/proxsave/internal/logging"
+ "github.com/tis24dev/proxsave/internal/types"
)
func TestRunRecipeRunsBricksInOrder(t *testing.T) {
@@ -120,6 +123,46 @@ func TestRunRecipePropagatesContextCancellation(t *testing.T) {
}
}
+func TestPVEGuestBrickPropagatesQEMUContextCancellation(t *testing.T) {
+ cfg := &CollectorConfig{
+ BackupVMConfigs: true,
+ PVEConfigPath: filepath.Join(t.TempDir(), "etc", "pve"),
+ }
+ if err := os.MkdirAll(filepath.Join(cfg.PVEConfigPath, "qemu-server"), 0o755); err != nil {
+ t.Fatalf("mkdir qemu-server: %v", err)
+ }
+
+ collector := NewCollector(logging.New(types.LogLevelError, false), cfg, t.TempDir(), types.ProxmoxVE, false)
+ state := newCollectionState(collector)
+ ctx, cancel := context.WithCancel(context.Background())
+ cancel()
+
+ err := newPVEGuestBricks()[0].Run(ctx, state)
+ if !errors.Is(err, context.Canceled) {
+ t.Fatalf("guest brick error = %v, want %v", err, context.Canceled)
+ }
+ if state.pve.guestCollectionAborted {
+ t.Fatalf("guest collection should not be marked aborted for context cancellation")
+ }
+}
+
+func TestPVEStorageProbeBrickPropagatesContextCancellation(t *testing.T) {
+ cfg := &CollectorConfig{BackupPVEBackupFiles: true}
+ collector := NewCollector(logging.New(types.LogLevelError, false), cfg, t.TempDir(), types.ProxmoxVE, false)
+ state := newCollectionState(collector)
+ state.pve.resolvedStorages = []pveStorageEntry{{Name: "local", Path: t.TempDir()}}
+ ctx, cancel := context.WithCancel(context.Background())
+ cancel()
+
+ err := newPVEStorageProbeBricks()[0].Run(ctx, state)
+ if !errors.Is(err, context.Canceled) {
+ t.Fatalf("storage probe brick error = %v, want %v", err, context.Canceled)
+ }
+ if state.pve.storageCollectionAborted {
+ t.Fatalf("storage collection should not be marked aborted for context cancellation")
+ }
+}
+
func recipeBrickIDs(r recipe) []BrickID {
ids := make([]BrickID, 0, len(r.Bricks))
for _, brick := range r.Bricks {
diff --git a/internal/backup/collector_config_extra_test.go b/internal/backup/collector_config_extra_test.go
index e13f6cdd..06e64f5b 100644
--- a/internal/backup/collector_config_extra_test.go
+++ b/internal/backup/collector_config_extra_test.go
@@ -33,6 +33,26 @@ func TestCollectorConfigValidateDefaultsAndErrors(t *testing.T) {
}
}
+func TestCollectorConfigValidateAcceptsNewStandaloneCollectionOptions(t *testing.T) {
+ tests := []struct {
+ name string
+ cfg *CollectorConfig
+ }{
+ {name: "pbs notification priv", cfg: &CollectorConfig{BackupPBSNotificationsPriv: true}},
+ {name: "root home", cfg: &CollectorConfig{BackupRootHome: true}},
+ {name: "script repository", cfg: &CollectorConfig{BackupScriptRepository: true}},
+ {name: "user homes", cfg: &CollectorConfig{BackupUserHomes: true}},
+ }
+
+ for _, tt := range tests {
+ t.Run(tt.name, func(t *testing.T) {
+ if err := tt.cfg.Validate(); err != nil {
+ t.Fatalf("Validate() error = %v", err)
+ }
+ })
+ }
+}
+
func TestGlobHelpers(t *testing.T) {
cases := []struct {
pattern string
diff --git a/internal/backup/collector_pve.go b/internal/backup/collector_pve.go
index 60af6575..18313254 100644
--- a/internal/backup/collector_pve.go
+++ b/internal/backup/collector_pve.go
@@ -1171,6 +1171,9 @@ func (c *Collector) collectPVEStorageMetadataJSONStep(ctx context.Context, resul
result.SkipRemaining = true
return nil
}
+ if isContextCancellationError(ctx, dirSampleErr) {
+ return dirSampleErr
+ }
if dirSampleErr != nil {
c.logger.Debug("Directory sample for datastore %s failed: %v", storage.Name, dirSampleErr)
}
@@ -1186,6 +1189,9 @@ func (c *Collector) collectPVEStorageMetadataJSONStep(ctx context.Context, resul
result.SkipRemaining = true
return nil
}
+ if isContextCancellationError(ctx, diskUsageErr) {
+ return diskUsageErr
+ }
if diskUsageErr != nil {
c.logger.Debug("Disk usage summary for %s failed: %v", storage.Name, diskUsageErr)
} else {
@@ -1210,6 +1216,9 @@ func (c *Collector) collectPVEStorageMetadataJSONStep(ctx context.Context, resul
result.SkipRemaining = true
return nil
}
+ if isContextCancellationError(ctx, sampleFileErr) {
+ return sampleFileErr
+ }
if sampleFileErr != nil {
c.logger.Debug("Backup file sample for %s failed: %v", storage.Name, sampleFileErr)
} else if len(fileSummaries) > 0 {
@@ -1243,6 +1252,9 @@ func (c *Collector) collectPVEStorageMetadataTextStep(ctx context.Context, resul
result.SkipRemaining = true
return nil
}
+ if isContextCancellationError(ctx, fileSampleErr) {
+ return fileSampleErr
+ }
if fileSampleErr != nil {
c.logger.Debug("General file sampling for %s failed: %v", storage.Name, fileSampleErr)
}
diff --git a/internal/config/config.go b/internal/config/config.go
index ae3c5e4c..07532c9d 100644
--- a/internal/config/config.go
+++ b/internal/config/config.go
@@ -405,9 +405,12 @@ func (c *Config) validateCloudSettings() error {
if !c.CloudEnabled {
return nil
}
- remoteName, basePath := splitCloudRemoteRef(strings.TrimSpace(c.CloudRemote))
- if err := safeexec.ValidateRcloneRemoteName(remoteName); err != nil {
- return fmt.Errorf("CLOUD_REMOTE invalid: %w", err)
+ cloudRemote := strings.TrimSpace(c.CloudRemote)
+ remoteName, basePath := splitCloudRemoteRef(cloudRemote)
+ if !isAbsoluteCloudRemoteRef(remoteName, basePath) {
+ if err := safeexec.ValidateRcloneRemoteName(remoteName); err != nil {
+ return fmt.Errorf("CLOUD_REMOTE invalid: %w", err)
+ }
}
if err := safeexec.ValidateRemoteRelativePath(strings.Trim(strings.TrimSpace(basePath), "/"), "CLOUD_REMOTE path"); err != nil {
return err
@@ -418,6 +421,22 @@ func (c *Config) validateCloudSettings() error {
return nil
}
+func isAbsoluteCloudRemoteRef(remoteName, basePath string) bool {
+ remoteName = strings.TrimSpace(remoteName)
+ basePath = strings.TrimSpace(basePath)
+ if filepath.IsAbs(remoteName) {
+ return true
+ }
+ if len(remoteName) != 1 {
+ return false
+ }
+ drive := remoteName[0]
+ if (drive < 'A' || drive > 'Z') && (drive < 'a' || drive > 'z') {
+ return false
+ }
+ return strings.HasPrefix(basePath, `\`) || strings.HasPrefix(basePath, "/")
+}
+
func splitCloudRemoteRef(ref string) (remoteName, relPath string) {
parts := strings.SplitN(ref, ":", 2)
if len(parts) < 2 {
diff --git a/internal/config/config_test.go b/internal/config/config_test.go
index 6db74d9a..e9e6bce0 100644
--- a/internal/config/config_test.go
+++ b/internal/config/config_test.go
@@ -370,6 +370,30 @@ SECONDARY_LOG_PATH=remote:/logs
}
}
+func TestValidateCloudSettingsAllowsAbsoluteCloudRemote(t *testing.T) {
+ tests := []string{
+ "/mnt/cloud",
+ `C:\cloud`,
+ "C:/cloud",
+ }
+
+ for _, remote := range tests {
+ t.Run(remote, func(t *testing.T) {
+ cfg := &Config{CloudEnabled: true, CloudRemote: remote}
+ if err := cfg.validateCloudSettings(); err != nil {
+ t.Fatalf("validateCloudSettings() error = %v", err)
+ }
+ })
+ }
+}
+
+func TestValidateCloudSettingsStillValidatesAbsoluteCloudRemotePath(t *testing.T) {
+ cfg := &Config{CloudEnabled: true, CloudRemote: "/mnt/cloud:../escape"}
+ if err := cfg.validateCloudSettings(); err == nil {
+ t.Fatal("expected validateCloudSettings to reject traversal in CLOUD_REMOTE path")
+ }
+}
+
func TestLoadConfigWithQuotes(t *testing.T) {
tmpDir := t.TempDir()
configPath := filepath.Join(tmpDir, "test_quotes.env")
From 16a7b694ede24e32cdbb643340a273165a5f586d Mon Sep 17 00:00:00 2001
From: Damiano <71268257+tis24dev@users.noreply.github.com>
Date: Wed, 6 May 2026 01:00:01 +0200
Subject: [PATCH 29/35] Add completion timeout to TUI simulation tests
Introduce simAppCompletionTimeout and group timing constants. Improve test timeout handling by reporting errors and stopping the app if the initial draw doesn't occur or the simulation doesn't finish within the completion timeout after injecting keys. Also properly stop and reset the timer to avoid races.
---
internal/orchestrator/tui_simulation_test.go | 21 +++++++++++++++++++-
1 file changed, 20 insertions(+), 1 deletion(-)
diff --git a/internal/orchestrator/tui_simulation_test.go b/internal/orchestrator/tui_simulation_test.go
index b846286d..82f9f247 100644
--- a/internal/orchestrator/tui_simulation_test.go
+++ b/internal/orchestrator/tui_simulation_test.go
@@ -13,7 +13,10 @@ import (
"github.com/tis24dev/proxsave/internal/tui"
)
-const simAppInitialDrawTimeout = 2 * time.Second
+const (
+ simAppInitialDrawTimeout = 2 * time.Second
+ simAppCompletionTimeout = 10 * time.Second
+)
type simKey struct {
Key tcell.Key
@@ -68,6 +71,8 @@ func withSimAppSequence(t *testing.T, keys []simKey) <-chan struct{} {
case <-done:
return
case <-timer.C:
+ t.Errorf("TUI simulation did not render its initial draw within %s", simAppInitialDrawTimeout)
+ app.Stop()
return
}
@@ -83,6 +88,20 @@ func withSimAppSequence(t *testing.T, keys []simKey) <-chan struct{} {
}
screen.InjectKey(k.Key, k.R, mod)
}
+
+ if !timer.Stop() {
+ select {
+ case <-timer.C:
+ default:
+ }
+ }
+ timer.Reset(simAppCompletionTimeout)
+ select {
+ case <-done:
+ case <-timer.C:
+ t.Errorf("TUI simulation did not finish within %s after injecting %d key(s)", simAppCompletionTimeout, len(keys))
+ app.Stop()
+ }
}()
})
return app
From daddefa6210e146c2e65b98310d3049cc7383c39 Mon Sep 17 00:00:00 2001
From: Damiano <71268257+tis24dev@users.noreply.github.com>
Date: Wed, 6 May 2026 12:17:12 +0200
Subject: [PATCH 30/35] Update decrypt_tui_e2e_helpers_test.go
---
.../decrypt_tui_e2e_helpers_test.go | 35 +++++++++++++++++--
1 file changed, 33 insertions(+), 2 deletions(-)
diff --git a/internal/orchestrator/decrypt_tui_e2e_helpers_test.go b/internal/orchestrator/decrypt_tui_e2e_helpers_test.go
index e15fe555..e82f7136 100644
--- a/internal/orchestrator/decrypt_tui_e2e_helpers_test.go
+++ b/internal/orchestrator/decrypt_tui_e2e_helpers_test.go
@@ -27,6 +27,11 @@ import (
var decryptTUIE2EMu sync.Mutex
+const (
+ timedSimScreenWaitTimeout = 5 * time.Second
+ timedSimCompletionTimeout = 10 * time.Second
+)
+
type notifyingSimulationScreen struct {
tcell.SimulationScreen
mu sync.Mutex
@@ -137,7 +142,20 @@ func withTimedSimAppSequence(t *testing.T, keys []timedSimKey) {
orig := newTUIApp
done := make(chan struct{})
var injectWG sync.WaitGroup
+ var appMu sync.RWMutex
+ var currentApp *tui.App
+
+ stopCurrentApp := func() {
+ appMu.RLock()
+ app := currentApp
+ appMu.RUnlock()
+ if app != nil {
+ app.Stop()
+ }
+ }
+
t.Cleanup(func() {
+ stopCurrentApp()
close(done)
injectWG.Wait()
newTUIApp = orig
@@ -156,8 +174,6 @@ func withTimedSimAppSequence(t *testing.T, keys []timedSimKey) {
}
screenStateCh := make(chan struct{}, 1)
- var appMu sync.RWMutex
- var currentApp *tui.App
screen := ¬ifyingSimulationScreen{
SimulationScreen: baseScreen,
notify: func() {
@@ -201,6 +217,8 @@ func withTimedSimAppSequence(t *testing.T, keys []timedSimKey) {
waitForScreenText := func(expected string) bool {
expected = strings.TrimSpace(expected)
+ timer := time.NewTimer(timedSimScreenWaitTimeout)
+ defer timer.Stop()
for {
current := currentScreenState()
if current.signature != "" {
@@ -214,6 +232,10 @@ func withTimedSimAppSequence(t *testing.T, keys []timedSimKey) {
case <-done:
return false
case <-screenStateCh:
+ case <-timer.C:
+ t.Errorf("TUI simulation did not render expected text %q within %s", expected, timedSimScreenWaitTimeout)
+ stopCurrentApp()
+ return false
}
}
}
@@ -237,6 +259,15 @@ func withTimedSimAppSequence(t *testing.T, keys []timedSimKey) {
screen.InjectKey(k.Key, k.R, mod)
lastInjectedState = current.signature
}
+
+ timer := time.NewTimer(timedSimCompletionTimeout)
+ defer timer.Stop()
+ select {
+ case <-done:
+ case <-timer.C:
+ t.Errorf("TUI simulation did not finish within %s after injecting %d key(s)", timedSimCompletionTimeout, len(keys))
+ stopCurrentApp()
+ }
}()
})
From da7a7e7a90add09c7e3b4c0a65f87318a42ef70c Mon Sep 17 00:00:00 2001
From: Damiano <71268257+tis24dev@users.noreply.github.com>
Date: Wed, 6 May 2026 13:24:22 +0200
Subject: [PATCH 31/35] tui: handle Stop-before-Run and test helpers
Ensure App.Stop is safe when called before Run by adding run state tracking, a mutex and a stopRequested flag; implement Run and markRunningAndStopIfRequested to defer stopping until the event loop starts. Add synchronized test helpers (setTimedSimAppStopper, stopTimedSimAppForTest) and protect simulated apps with mutexes so tests can reliably stop current TUI instances. Increase several simulation timeouts and update runDecryptWorkflowTUIForTest to use a cancellable context and trigger a timed shutdown path. Also add a test verifying pre-run Stop terminates RunWithContext as expected.
---
.../decrypt_tui_e2e_helpers_test.go | 52 +++++++++++++--
internal/orchestrator/tui_simulation_test.go | 19 +++++-
internal/tui/abort_context_test.go | 19 ++++++
internal/tui/app.go | 64 ++++++++++++++++++-
4 files changed, 145 insertions(+), 9 deletions(-)
diff --git a/internal/orchestrator/decrypt_tui_e2e_helpers_test.go b/internal/orchestrator/decrypt_tui_e2e_helpers_test.go
index e82f7136..5378b724 100644
--- a/internal/orchestrator/decrypt_tui_e2e_helpers_test.go
+++ b/internal/orchestrator/decrypt_tui_e2e_helpers_test.go
@@ -25,11 +25,15 @@ import (
"github.com/tis24dev/proxsave/internal/types"
)
-var decryptTUIE2EMu sync.Mutex
+var (
+ decryptTUIE2EMu sync.Mutex
+ timedSimAppStopperMu sync.Mutex
+ timedSimAppStopper func()
+)
const (
- timedSimScreenWaitTimeout = 5 * time.Second
- timedSimCompletionTimeout = 10 * time.Second
+ timedSimScreenWaitTimeout = 10 * time.Second
+ timedSimCompletionTimeout = 15 * time.Second
)
type notifyingSimulationScreen struct {
@@ -153,9 +157,11 @@ func withTimedSimAppSequence(t *testing.T, keys []timedSimKey) {
app.Stop()
}
}
+ restoreStopper := setTimedSimAppStopper(stopCurrentApp)
t.Cleanup(func() {
stopCurrentApp()
+ restoreStopper()
close(done)
injectWG.Wait()
newTUIApp = orig
@@ -275,6 +281,28 @@ func withTimedSimAppSequence(t *testing.T, keys []timedSimKey) {
}
}
+func setTimedSimAppStopper(stop func()) func() {
+ timedSimAppStopperMu.Lock()
+ previous := timedSimAppStopper
+ timedSimAppStopper = stop
+ timedSimAppStopperMu.Unlock()
+
+ return func() {
+ timedSimAppStopperMu.Lock()
+ timedSimAppStopper = previous
+ timedSimAppStopperMu.Unlock()
+ }
+}
+
+func stopTimedSimAppForTest() {
+ timedSimAppStopperMu.Lock()
+ stop := timedSimAppStopper
+ timedSimAppStopperMu.Unlock()
+ if stop != nil {
+ stop()
+ }
+}
+
func timedSimScreenStateSignature(snapshot timedSimScreenSnapshot, focus any) string {
if !snapshot.ready || snapshot.width <= 0 || snapshot.height <= 0 || len(snapshot.cells) < snapshot.width*snapshot.height {
return ""
@@ -442,12 +470,15 @@ func abortDecryptTUISequence() []timedSimKey {
func runDecryptWorkflowTUIForTest(t *testing.T, ctx context.Context, cfg *config.Config, configPath string) error {
t.Helper()
+ runCtx, cancel := context.WithCancel(ctx)
+ defer cancel()
+
logger := logging.New(types.LogLevelError, false)
logger.SetOutput(io.Discard)
errCh := make(chan error, 1)
go func() {
- errCh <- RunDecryptWorkflowTUI(ctx, cfg, logger, "1.0.0", configPath, "test-build")
+ errCh <- RunDecryptWorkflowTUI(runCtx, cfg, logger, "1.0.0", configPath, "test-build")
}()
waitTimeout := 30 * time.Second
@@ -464,7 +495,18 @@ func runDecryptWorkflowTUIForTest(t *testing.T, ctx context.Context, cfg *config
case err := <-errCh:
return err
case <-timer.C:
- if err := ctx.Err(); err != nil {
+ cancel()
+ stopTimedSimAppForTest()
+
+ shutdownTimer := time.NewTimer(2 * time.Second)
+ defer shutdownTimer.Stop()
+ select {
+ case err := <-errCh:
+ return err
+ case <-shutdownTimer.C:
+ }
+
+ if err := runCtx.Err(); err != nil {
t.Fatalf("RunDecryptWorkflowTUI did not return within %s (context state: %v)", waitTimeout, err)
return nil
}
diff --git a/internal/orchestrator/tui_simulation_test.go b/internal/orchestrator/tui_simulation_test.go
index 82f9f247..20d2cfdd 100644
--- a/internal/orchestrator/tui_simulation_test.go
+++ b/internal/orchestrator/tui_simulation_test.go
@@ -38,9 +38,23 @@ func withSimAppSequence(t *testing.T, keys []simKey) <-chan struct{} {
done := make(chan struct{})
var injectOnce sync.Once
var injectWG sync.WaitGroup
+ var appMu sync.RWMutex
+ var currentApp *tui.App
+
+ stopCurrentApp := func() {
+ appMu.RLock()
+ app := currentApp
+ appMu.RUnlock()
+ if app != nil {
+ app.Stop()
+ }
+ }
newTUIApp = func() *tui.App {
app := tui.NewApp()
+ appMu.Lock()
+ currentApp = app
+ appMu.Unlock()
app.SetScreen(screen)
readyCh := make(chan struct{})
var readyOnce sync.Once
@@ -72,7 +86,7 @@ func withSimAppSequence(t *testing.T, keys []simKey) <-chan struct{} {
return
case <-timer.C:
t.Errorf("TUI simulation did not render its initial draw within %s", simAppInitialDrawTimeout)
- app.Stop()
+ stopCurrentApp()
return
}
@@ -100,7 +114,7 @@ func withSimAppSequence(t *testing.T, keys []simKey) <-chan struct{} {
case <-done:
case <-timer.C:
t.Errorf("TUI simulation did not finish within %s after injecting %d key(s)", simAppCompletionTimeout, len(keys))
- app.Stop()
+ stopCurrentApp()
}
}()
})
@@ -108,6 +122,7 @@ func withSimAppSequence(t *testing.T, keys []simKey) <-chan struct{} {
}
t.Cleanup(func() {
+ stopCurrentApp()
close(done)
injectWG.Wait()
newTUIApp = orig
diff --git a/internal/tui/abort_context_test.go b/internal/tui/abort_context_test.go
index 93778c1c..2654353f 100644
--- a/internal/tui/abort_context_test.go
+++ b/internal/tui/abort_context_test.go
@@ -167,6 +167,25 @@ func TestAppRunWithContext_NilContextRunsUntilStopped(t *testing.T) {
}
}
+func TestAppRunWithContext_StopBeforeRunStopsWhenRunStarts(t *testing.T) {
+ app, _, _ := newSimulationApp(t)
+ app.Stop()
+
+ done := make(chan error, 1)
+ go func() {
+ done <- app.RunWithContext(context.Background())
+ }()
+
+ select {
+ case err := <-done:
+ if err != nil {
+ t.Fatalf("err=%v want nil", err)
+ }
+ case <-time.After(2 * time.Second):
+ t.Fatal("timed out waiting for pre-run Stop to end RunWithContext")
+ }
+}
+
func TestAppRunWithContext_ReturnsNilWhenStoppedWithoutCancellation(t *testing.T) {
app, _, started := newSimulationApp(t)
done := make(chan error, 1)
diff --git a/internal/tui/app.go b/internal/tui/app.go
index e190f67f..f1d480e9 100644
--- a/internal/tui/app.go
+++ b/internal/tui/app.go
@@ -2,16 +2,27 @@ package tui
import (
"context"
+ "sync"
"sync/atomic"
"github.com/gdamore/tcell/v2"
"github.com/rivo/tview"
)
+const (
+ appRunStateIdle = iota
+ appRunStateStarting
+ appRunStateRunning
+ appRunStateFinished
+)
+
// App wraps tview.Application with Proxmox-specific configuration
type App struct {
*tview.Application
- stopHook func()
+ stopHook func()
+ runMu sync.Mutex
+ runState int
+ stopRequested bool
}
// NewApp creates a new TUI application with Proxmox theme
@@ -49,8 +60,57 @@ func (a *App) Stop() {
return
}
if a.Application != nil {
- a.Application.Stop()
+ a.runMu.Lock()
+ switch a.runState {
+ case appRunStateIdle, appRunStateStarting:
+ // tview.Stop before Run clears the configured screen; apply it once
+ // the event loop can process the request instead.
+ a.stopRequested = true
+ a.runMu.Unlock()
+ return
+ case appRunStateRunning:
+ a.runMu.Unlock()
+ a.Application.Stop()
+ return
+ default:
+ a.runMu.Unlock()
+ }
+ }
+}
+
+func (a *App) Run() error {
+ if a == nil || a.Application == nil {
+ return nil
}
+
+ a.runMu.Lock()
+ a.runState = appRunStateStarting
+ a.runMu.Unlock()
+
+ go a.markRunningAndStopIfRequested()
+
+ err := a.Application.Run()
+
+ a.runMu.Lock()
+ a.runState = appRunStateFinished
+ a.stopRequested = false
+ a.runMu.Unlock()
+
+ return err
+}
+
+func (a *App) markRunningAndStopIfRequested() {
+ a.QueueUpdate(func() {
+ a.runMu.Lock()
+ a.runState = appRunStateRunning
+ stopRequested := a.stopRequested
+ a.stopRequested = false
+ a.runMu.Unlock()
+
+ if stopRequested {
+ a.Application.Stop()
+ }
+ })
}
func (a *App) RunWithContext(ctx context.Context) error {
From 2a44bbab813adb098ad1b7ee225cbb7cdc142a65 Mon Sep 17 00:00:00 2001
From: Damiano <71268257+tis24dev@users.noreply.github.com>
Date: Wed, 6 May 2026 13:50:39 +0200
Subject: [PATCH 32/35] Refactor TUI test harness and improve task start/stop
Replace ad-hoc timed simulation plumbing with a reusable timedSimHarness for decrypt TUI e2e tests. Introduce timedSimHarness, timedSimAppState, and richer timedSimKey fields (RequireNewApp, SettleAfterMatch) plus new timing constants to make key injection and app lifecycle more deterministic and to improve diagnostics on timeouts. Update helper functions and tests to return/use the harness and tighten shutdown logic (StopAll, better timeout/error messages).
Also change RunTask startup/shutdown in workflow_ui_tui_decrypt.go to trigger the background task after the first draw (SetAfterDrawFunc), add start synchronization (started, startOnce) and queueProgressUpdate to safely schedule UI updates and ensure proper waiting for task completion on exit. Minor import adjustments (sync) included.
---
.../decrypt_tui_e2e_helpers_test.go | 403 ++++++++++--------
internal/orchestrator/decrypt_tui_e2e_test.go | 8 +-
.../orchestrator/workflow_ui_tui_decrypt.go | 51 ++-
3 files changed, 276 insertions(+), 186 deletions(-)
diff --git a/internal/orchestrator/decrypt_tui_e2e_helpers_test.go b/internal/orchestrator/decrypt_tui_e2e_helpers_test.go
index 5378b724..538e7beb 100644
--- a/internal/orchestrator/decrypt_tui_e2e_helpers_test.go
+++ b/internal/orchestrator/decrypt_tui_e2e_helpers_test.go
@@ -25,15 +25,13 @@ import (
"github.com/tis24dev/proxsave/internal/types"
)
-var (
- decryptTUIE2EMu sync.Mutex
- timedSimAppStopperMu sync.Mutex
- timedSimAppStopper func()
-)
+var decryptTUIE2EMu sync.Mutex
const (
timedSimScreenWaitTimeout = 10 * time.Second
timedSimCompletionTimeout = 15 * time.Second
+ timedSimDefaultSettle = 15 * time.Millisecond
+ timedSimKeyDelay = 15 * time.Millisecond
)
type notifyingSimulationScreen struct {
@@ -118,11 +116,38 @@ func cloneSimCells(cells []tcell.SimCell) []tcell.SimCell {
}
type timedSimKey struct {
- Key tcell.Key
- R rune
- Mod tcell.ModMask
- Wait time.Duration
- WaitForText string
+ Key tcell.Key
+ R rune
+ Mod tcell.ModMask
+ WaitForText string
+ RequireNewApp bool
+ SettleAfterMatch time.Duration
+}
+
+type timedSimHarness struct {
+ t *testing.T
+ done chan struct{}
+ closeDoneOnce sync.Once
+ injectWG sync.WaitGroup
+ screenStateCh chan struct{}
+
+ appMu sync.RWMutex
+ apps []*tui.App
+ current *timedSimAppState
+}
+
+type timedSimAppState struct {
+ generation int
+ app *tui.App
+ screen *notifyingSimulationScreen
+}
+
+type timedSimScreenState struct {
+ generation int
+ text string
+ focusType string
+ ready bool
+ screen *notifyingSimulationScreen
}
type decryptTUIFixture struct {
@@ -139,182 +164,205 @@ type decryptTUIFixture struct {
ExpectedChecksum string
}
-func withTimedSimAppSequence(t *testing.T, keys []timedSimKey) {
+func withTimedSimAppSequence(t *testing.T, keys []timedSimKey) *timedSimHarness {
t.Helper()
decryptTUIE2EMu.Lock()
orig := newTUIApp
- done := make(chan struct{})
- var injectWG sync.WaitGroup
- var appMu sync.RWMutex
- var currentApp *tui.App
-
- stopCurrentApp := func() {
- appMu.RLock()
- app := currentApp
- appMu.RUnlock()
- if app != nil {
- app.Stop()
- }
+ h := &timedSimHarness{
+ t: t,
+ done: make(chan struct{}),
+ screenStateCh: make(chan struct{}, 1),
}
- restoreStopper := setTimedSimAppStopper(stopCurrentApp)
t.Cleanup(func() {
- stopCurrentApp()
- restoreStopper()
- close(done)
- injectWG.Wait()
+ h.stop()
newTUIApp = orig
decryptTUIE2EMu.Unlock()
})
- baseScreen := tcell.NewSimulationScreen("UTF-8")
- if err := baseScreen.Init(); err != nil {
- t.Fatalf("screen.Init: %v", err)
+ newTUIApp = func() *tui.App {
+ app := tui.NewApp()
+
+ baseScreen := tcell.NewSimulationScreen("UTF-8")
+ if err := baseScreen.Init(); err != nil {
+ t.Fatalf("screen.Init: %v", err)
+ }
+ baseScreen.SetSize(120, 40)
+
+ screen := ¬ifyingSimulationScreen{
+ SimulationScreen: baseScreen,
+ notify: h.notifyScreenStateChanged,
+ }
+
+ h.appMu.Lock()
+ state := &timedSimAppState{
+ generation: len(h.apps) + 1,
+ app: app,
+ screen: screen,
+ }
+ h.apps = append(h.apps, app)
+ h.current = state
+ h.appMu.Unlock()
+
+ app.SetScreen(screen)
+ h.notifyScreenStateChanged()
+ return app
}
- baseScreen.SetSize(120, 40)
- type timedSimScreenState struct {
- signature string
- text string
+ h.injectWG.Add(1)
+ go h.run(keys)
+
+ return h
+}
+
+func (h *timedSimHarness) notifyScreenStateChanged() {
+ select {
+ case h.screenStateCh <- struct{}{}:
+ default:
}
+}
- screenStateCh := make(chan struct{}, 1)
- screen := ¬ifyingSimulationScreen{
- SimulationScreen: baseScreen,
- notify: func() {
- select {
- case screenStateCh <- struct{}{}:
- default:
- }
- },
+func (h *timedSimHarness) stop() {
+ if h == nil {
+ return
}
+ h.closeDoneOnce.Do(func() {
+ close(h.done)
+ })
+ h.StopAll()
+ h.injectWG.Wait()
+}
- var once sync.Once
- newTUIApp = func() *tui.App {
- app := tui.NewApp()
- appMu.Lock()
- currentApp = app
- appMu.Unlock()
- app.SetScreen(screen)
+func (h *timedSimHarness) StopAll() {
+ if h == nil {
+ return
+ }
+ h.appMu.RLock()
+ apps := append([]*tui.App(nil), h.apps...)
+ h.appMu.RUnlock()
+ for i := len(apps) - 1; i >= 0; i-- {
+ apps[i].Stop()
+ }
+}
- once.Do(func() {
- injectWG.Add(1)
- go func() {
- defer injectWG.Done()
- var lastInjectedState string
-
- currentScreenState := func() timedSimScreenState {
- appMu.RLock()
- app := currentApp
- appMu.RUnlock()
-
- var focus any
- if app != nil {
- focus = app.GetFocus()
- }
- snapshot := screen.snapshotState()
-
- return timedSimScreenState{
- signature: timedSimScreenStateSignature(snapshot, focus),
- text: timedSimScreenText(snapshot),
- }
- }
-
- waitForScreenText := func(expected string) bool {
- expected = strings.TrimSpace(expected)
- timer := time.NewTimer(timedSimScreenWaitTimeout)
- defer timer.Stop()
- for {
- current := currentScreenState()
- if current.signature != "" {
- if (expected == "" || strings.Contains(current.text, expected)) &&
- (lastInjectedState == "" || current.signature != lastInjectedState) {
- return true
- }
- }
-
- select {
- case <-done:
- return false
- case <-screenStateCh:
- case <-timer.C:
- t.Errorf("TUI simulation did not render expected text %q within %s", expected, timedSimScreenWaitTimeout)
- stopCurrentApp()
- return false
- }
- }
- }
-
- for _, k := range keys {
- if k.Wait > 0 {
- if !waitForScreenText(k.WaitForText) {
- return
- }
- }
- current := currentScreenState()
- mod := k.Mod
- if mod == 0 {
- mod = tcell.ModNone
- }
- select {
- case <-done:
- return
- default:
- }
- screen.InjectKey(k.Key, k.R, mod)
- lastInjectedState = current.signature
- }
-
- timer := time.NewTimer(timedSimCompletionTimeout)
- defer timer.Stop()
- select {
- case <-done:
- case <-timer.C:
- t.Errorf("TUI simulation did not finish within %s after injecting %d key(s)", timedSimCompletionTimeout, len(keys))
- stopCurrentApp()
- }
- }()
- })
+func (h *timedSimHarness) run(keys []timedSimKey) {
+ defer h.injectWG.Done()
- return app
+ generation := 0
+ for idx, key := range keys {
+ minGeneration := generation
+ if minGeneration == 0 || key.RequireNewApp {
+ minGeneration++
+ }
+
+ state, ok := h.waitForScreenText(idx, key, minGeneration)
+ if !ok {
+ return
+ }
+ generation = state.generation
+
+ settle := key.SettleAfterMatch
+ if settle <= 0 {
+ settle = timedSimDefaultSettle
+ }
+ if !h.sleepOrDone(settle) {
+ return
+ }
+
+ mod := key.Mod
+ if mod == 0 {
+ mod = tcell.ModNone
+ }
+ state.screen.InjectKey(key.Key, key.R, mod)
+ if !h.sleepOrDone(timedSimKeyDelay) {
+ return
+ }
+ }
+
+ timer := time.NewTimer(timedSimCompletionTimeout)
+ defer timer.Stop()
+ select {
+ case <-h.done:
+ case <-timer.C:
+ h.t.Errorf("TUI simulation did not finish within %s after injecting %d key(s)\n%s", timedSimCompletionTimeout, len(keys), h.describeCurrentState())
+ h.StopAll()
}
}
-func setTimedSimAppStopper(stop func()) func() {
- timedSimAppStopperMu.Lock()
- previous := timedSimAppStopper
- timedSimAppStopper = stop
- timedSimAppStopperMu.Unlock()
+func (h *timedSimHarness) waitForScreenText(index int, key timedSimKey, minGeneration int) (timedSimScreenState, bool) {
+ expected := strings.TrimSpace(key.WaitForText)
+ timer := time.NewTimer(timedSimScreenWaitTimeout)
+ defer timer.Stop()
+
+ for {
+ state := h.currentScreenState()
+ if state.ready && state.generation >= minGeneration && (expected == "" || strings.Contains(state.text, expected)) {
+ return state, true
+ }
- return func() {
- timedSimAppStopperMu.Lock()
- timedSimAppStopper = previous
- timedSimAppStopperMu.Unlock()
+ select {
+ case <-h.done:
+ return timedSimScreenState{}, false
+ case <-h.screenStateCh:
+ case <-timer.C:
+ h.t.Errorf(
+ "TUI simulation timed out at action %d waiting for text %q within %s (min generation=%d, current generation=%d, focus=%s)\nCurrent screen:\n%s",
+ index,
+ expected,
+ timedSimScreenWaitTimeout,
+ minGeneration,
+ state.generation,
+ state.focusType,
+ state.text,
+ )
+ h.StopAll()
+ return state, false
+ }
}
}
-func stopTimedSimAppForTest() {
- timedSimAppStopperMu.Lock()
- stop := timedSimAppStopper
- timedSimAppStopperMu.Unlock()
- if stop != nil {
- stop()
+func (h *timedSimHarness) currentScreenState() timedSimScreenState {
+ h.appMu.RLock()
+ current := h.current
+ h.appMu.RUnlock()
+ if current == nil || current.screen == nil {
+ return timedSimScreenState{}
}
-}
-func timedSimScreenStateSignature(snapshot timedSimScreenSnapshot, focus any) string {
- if !snapshot.ready || snapshot.width <= 0 || snapshot.height <= 0 || len(snapshot.cells) < snapshot.width*snapshot.height {
- return ""
+ focusType := ""
+ if current.app != nil {
+ if focus := current.app.GetFocus(); focus != nil {
+ focusType = fmt.Sprintf("%T", focus)
+ }
}
+ snapshot := current.screen.snapshotState()
+ return timedSimScreenState{
+ generation: current.generation,
+ text: timedSimScreenText(snapshot),
+ focusType: focusType,
+ ready: snapshot.ready,
+ screen: current.screen,
+ }
+}
- sum := sha256.New()
- fmt.Fprintf(sum, "size:%d:%d cursor:%d:%d:%t focus:%T:%p\n", snapshot.width, snapshot.height, snapshot.cursorX, snapshot.cursorY, snapshot.cursorVisible, focus, focus)
- for _, cell := range snapshot.cells {
- fg, bg, attr := cell.Style.Decompose()
- fmt.Fprintf(sum, "%x/%d/%d/%d;", cell.Bytes, fg, bg, attr)
+func (h *timedSimHarness) describeCurrentState() string {
+ state := h.currentScreenState()
+ return fmt.Sprintf("current generation=%d focus=%s ready=%t\nCurrent screen:\n%s", state.generation, state.focusType, state.ready, state.text)
+}
+
+func (h *timedSimHarness) sleepOrDone(d time.Duration) bool {
+ if d <= 0 {
+ return true
+ }
+ timer := time.NewTimer(d)
+ defer timer.Stop()
+ select {
+ case <-h.done:
+ return false
+ case <-timer.C:
+ return true
}
- return hex.EncodeToString(sum.Sum(nil))
}
func timedSimScreenText(snapshot timedSimScreenSnapshot) string {
@@ -434,24 +482,25 @@ func createDecryptTUIEncryptedFixture(t *testing.T) *decryptTUIFixture {
func successDecryptTUISequence(secret string) []timedSimKey {
keys := []timedSimKey{
- {Key: tcell.KeyEnter, Wait: 1 * time.Second, WaitForText: "Select backup source"},
- {Key: tcell.KeyEnter, Wait: 750 * time.Millisecond, WaitForText: "Select backup"},
+ {Key: tcell.KeyEnter, WaitForText: "Select backup source", RequireNewApp: true},
+ {Key: tcell.KeyEnter, WaitForText: "Select backup", RequireNewApp: true},
}
- for _, r := range secret {
+ for idx, r := range secret {
keys = append(keys, timedSimKey{
- Key: tcell.KeyRune,
- R: r,
- Wait: 35 * time.Millisecond,
- WaitForText: "Decrypt key",
+ Key: tcell.KeyRune,
+ R: r,
+ WaitForText: "Decrypt key",
+ RequireNewApp: idx == 0,
+ SettleAfterMatch: 5 * time.Millisecond,
})
}
keys = append(keys,
- timedSimKey{Key: tcell.KeyTab, Wait: 150 * time.Millisecond, WaitForText: "Decrypt key"},
- timedSimKey{Key: tcell.KeyEnter, Wait: 100 * time.Millisecond, WaitForText: "Decrypt key"},
- timedSimKey{Key: tcell.KeyTab, Wait: 500 * time.Millisecond, WaitForText: "Destination directory"},
- timedSimKey{Key: tcell.KeyEnter, Wait: 100 * time.Millisecond, WaitForText: "Destination directory"},
+ timedSimKey{Key: tcell.KeyTab, WaitForText: "Decrypt key"},
+ timedSimKey{Key: tcell.KeyEnter, WaitForText: "Decrypt key"},
+ timedSimKey{Key: tcell.KeyTab, WaitForText: "Destination directory", RequireNewApp: true},
+ timedSimKey{Key: tcell.KeyEnter, WaitForText: "Destination directory"},
)
return keys
@@ -459,15 +508,15 @@ func successDecryptTUISequence(secret string) []timedSimKey {
func abortDecryptTUISequence() []timedSimKey {
return []timedSimKey{
- {Key: tcell.KeyEnter, Wait: 1 * time.Second, WaitForText: "Select backup source"},
- {Key: tcell.KeyEnter, Wait: 750 * time.Millisecond, WaitForText: "Select backup"},
- {Key: tcell.KeyRune, R: '0', Wait: 500 * time.Millisecond, WaitForText: "Decrypt key"},
- {Key: tcell.KeyTab, Wait: 150 * time.Millisecond, WaitForText: "Decrypt key"},
- {Key: tcell.KeyEnter, Wait: 100 * time.Millisecond, WaitForText: "Decrypt key"},
+ {Key: tcell.KeyEnter, WaitForText: "Select backup source", RequireNewApp: true},
+ {Key: tcell.KeyEnter, WaitForText: "Select backup", RequireNewApp: true},
+ {Key: tcell.KeyRune, R: '0', WaitForText: "Decrypt key", RequireNewApp: true},
+ {Key: tcell.KeyTab, WaitForText: "Decrypt key"},
+ {Key: tcell.KeyEnter, WaitForText: "Decrypt key"},
}
}
-func runDecryptWorkflowTUIForTest(t *testing.T, ctx context.Context, cfg *config.Config, configPath string) error {
+func runDecryptWorkflowTUIForTest(t *testing.T, sim *timedSimHarness, ctx context.Context, cfg *config.Config, configPath string) error {
t.Helper()
runCtx, cancel := context.WithCancel(ctx)
@@ -496,7 +545,9 @@ func runDecryptWorkflowTUIForTest(t *testing.T, ctx context.Context, cfg *config
return err
case <-timer.C:
cancel()
- stopTimedSimAppForTest()
+ if sim != nil {
+ sim.StopAll()
+ }
shutdownTimer := time.NewTimer(2 * time.Second)
defer shutdownTimer.Stop()
@@ -507,9 +558,15 @@ func runDecryptWorkflowTUIForTest(t *testing.T, ctx context.Context, cfg *config
}
if err := runCtx.Err(); err != nil {
+ if sim != nil {
+ t.Fatalf("RunDecryptWorkflowTUI did not return within %s (context state: %v)\n%s", waitTimeout, err, sim.describeCurrentState())
+ }
t.Fatalf("RunDecryptWorkflowTUI did not return within %s (context state: %v)", waitTimeout, err)
return nil
}
+ if sim != nil {
+ t.Fatalf("RunDecryptWorkflowTUI did not return within %s\n%s", waitTimeout, sim.describeCurrentState())
+ }
t.Fatalf("RunDecryptWorkflowTUI did not return within %s", waitTimeout)
return nil
}
diff --git a/internal/orchestrator/decrypt_tui_e2e_test.go b/internal/orchestrator/decrypt_tui_e2e_test.go
index 925b81d0..bea6a525 100644
--- a/internal/orchestrator/decrypt_tui_e2e_test.go
+++ b/internal/orchestrator/decrypt_tui_e2e_test.go
@@ -18,12 +18,12 @@ func TestRunDecryptWorkflowTUI_SuccessLocalEncrypted(t *testing.T) {
t.Cleanup(func() { restoreFS = origFS })
fixture := createDecryptTUIEncryptedFixture(t)
- withTimedSimAppSequence(t, successDecryptTUISequence(fixture.Secret))
+ sim := withTimedSimAppSequence(t, successDecryptTUISequence(fixture.Secret))
ctx, cancel := context.WithTimeout(context.Background(), 18*time.Second)
defer cancel()
- if err := runDecryptWorkflowTUIForTest(t, ctx, fixture.Config, fixture.ConfigPath); err != nil {
+ if err := runDecryptWorkflowTUIForTest(t, sim, ctx, fixture.Config, fixture.ConfigPath); err != nil {
t.Fatalf("RunDecryptWorkflowTUI error: %v", err)
}
@@ -79,12 +79,12 @@ func TestRunDecryptWorkflowTUI_AbortAtSecretPrompt(t *testing.T) {
t.Cleanup(func() { restoreFS = origFS })
fixture := createDecryptTUIEncryptedFixture(t)
- withTimedSimAppSequence(t, abortDecryptTUISequence())
+ sim := withTimedSimAppSequence(t, abortDecryptTUISequence())
ctx, cancel := context.WithTimeout(context.Background(), 18*time.Second)
defer cancel()
- err := runDecryptWorkflowTUIForTest(t, ctx, fixture.Config, fixture.ConfigPath)
+ err := runDecryptWorkflowTUIForTest(t, sim, ctx, fixture.Config, fixture.ConfigPath)
if !errors.Is(err, ErrDecryptAborted) {
t.Fatalf("RunDecryptWorkflowTUI error=%v; want %v", err, ErrDecryptAborted)
}
diff --git a/internal/orchestrator/workflow_ui_tui_decrypt.go b/internal/orchestrator/workflow_ui_tui_decrypt.go
index 531571d5..fa0483a5 100644
--- a/internal/orchestrator/workflow_ui_tui_decrypt.go
+++ b/internal/orchestrator/workflow_ui_tui_decrypt.go
@@ -5,6 +5,7 @@ import (
"fmt"
"path/filepath"
"strings"
+ "sync"
"github.com/gdamore/tcell/v2"
"github.com/rivo/tview"
@@ -86,35 +87,67 @@ func (u *tuiWorkflowUI) RunTask(ctx context.Context, title, initialMessage strin
form.SetParentView(page)
done := make(chan struct{})
+ started := make(chan struct{})
+ var startOnce sync.Once
var runErr error
+ queueProgressUpdate := func(update func()) {
+ select {
+ case <-taskCtx.Done():
+ return
+ default:
+ }
+ go func() {
+ select {
+ case <-taskCtx.Done():
+ return
+ default:
+ }
+ app.QueueUpdateDraw(update)
+ }()
+ }
+
report := func(message string) {
message = strings.TrimSpace(message)
if message == "" {
return
}
- app.QueueUpdateDraw(func() {
+ queueProgressUpdate(func() {
messageView.SetText(tview.Escape(message))
})
}
- go func() {
- runErr = run(taskCtx, report)
- close(done)
- app.QueueUpdateDraw(func() {
- app.Stop()
+ startTask := func() {
+ startOnce.Do(func() {
+ close(started)
+ go func() {
+ runErr = run(taskCtx, report)
+ close(done)
+ app.Stop()
+ }()
})
- }()
+ }
app.SetRoot(page, true).SetFocus(form.Form)
+ app.SetAfterDrawFunc(func(screen tcell.Screen) {
+ startTask()
+ })
if err := app.RunWithContext(taskCtx); err != nil {
cancel()
- <-done
+ select {
+ case <-started:
+ <-done
+ default:
+ }
return err
}
cancel()
- <-done
+ select {
+ case <-started:
+ <-done
+ default:
+ }
return runErr
}
From 5e36f471a78c323707d3a366c327165144303031 Mon Sep 17 00:00:00 2001
From: Damiano <71268257+tis24dev@users.noreply.github.com>
Date: Wed, 6 May 2026 15:05:32 +0200
Subject: [PATCH 33/35] Pin actions; fix update check, PBS & tests
Pin GitHub Actions steps to specific SHAs for reproducible runs. Add a nil-context guard in checkForUpdates to avoid panics when ctx is nil. Include PBS user config collection by running newPBSUserConfigRecipe(). Change lookupAbsolutePath to return an error if exec.LookPath returns a non-absolute path rather than attempting to make it absolute. Enhance TUI e2e test helpers by adding a Wait field to timedSimKey and using it to control timeouts/sleeps. Update test FakeFS to simulate ownership changes via an Ownership map and a FakeOwnership struct in Lchown instead of calling os.Lchown.
---
.github/workflows/dependabot-automerge.yml | 4 ++--
cmd/proxsave/main_update.go | 3 +++
internal/backup/collector_pbs.go | 3 +++
internal/notify/email.go | 2 +-
.../decrypt_tui_e2e_helpers_test.go | 14 ++++++++++++--
internal/orchestrator/deps_test.go | 17 ++++++++++++++++-
6 files changed, 37 insertions(+), 6 deletions(-)
diff --git a/.github/workflows/dependabot-automerge.yml b/.github/workflows/dependabot-automerge.yml
index c5264029..a4676472 100644
--- a/.github/workflows/dependabot-automerge.yml
+++ b/.github/workflows/dependabot-automerge.yml
@@ -17,13 +17,13 @@ jobs:
steps:
- name: Set up Node.js 24
- uses: actions/setup-node@v4
+ uses: actions/setup-node@49933ea5288caeca8642d1e84afbd3f7d6820020 # pinned from actions/setup-node@v4
with:
node-version: '24'
- name: Fetch Dependabot metadata
id: metadata
- uses: dependabot/fetch-metadata@v3
+ uses: dependabot/fetch-metadata@25dd0e34f4fe68f24cc83900b1fe3fe149efef98 # pinned from dependabot/fetch-metadata@v3
with:
github-token: "${{ secrets.GITHUB_TOKEN }}"
diff --git a/cmd/proxsave/main_update.go b/cmd/proxsave/main_update.go
index 6ce50983..f016fe48 100644
--- a/cmd/proxsave/main_update.go
+++ b/cmd/proxsave/main_update.go
@@ -34,6 +34,9 @@ func checkForUpdates(ctx context.Context, logger *logging.Logger, currentVersion
logger.Debug("Update check skipped: current version is empty")
return nil
}
+ if ctx == nil {
+ ctx = context.Background()
+ }
checkCtx, cancel := context.WithTimeout(ctx, 5*time.Second)
defer cancel()
diff --git a/internal/backup/collector_pbs.go b/internal/backup/collector_pbs.go
index 61c12077..44cd9553 100644
--- a/internal/backup/collector_pbs.go
+++ b/internal/backup/collector_pbs.go
@@ -80,6 +80,9 @@ func (c *Collector) CollectPBSConfigs(ctx context.Context) error {
if err := runRecipe(ctx, newPBSRecipe(), state); err != nil {
return err
}
+ if err := runRecipe(ctx, newPBSUserConfigRecipe(), state); err != nil {
+ return err
+ }
c.logger.Info("PBS configuration collection completed")
return nil
diff --git a/internal/notify/email.go b/internal/notify/email.go
index cefe2380..61fe6cb9 100644
--- a/internal/notify/email.go
+++ b/internal/notify/email.go
@@ -692,7 +692,7 @@ func lookupAbsolutePath(name string) (string, error) {
if filepath.IsAbs(execPath) {
return execPath, nil
}
- return filepath.Abs(execPath)
+ return "", fmt.Errorf("exec.LookPath returned non-absolute path %q", execPath)
}
// sendViaRelay sends email via cloud relay
diff --git a/internal/orchestrator/decrypt_tui_e2e_helpers_test.go b/internal/orchestrator/decrypt_tui_e2e_helpers_test.go
index 538e7beb..4278b2f2 100644
--- a/internal/orchestrator/decrypt_tui_e2e_helpers_test.go
+++ b/internal/orchestrator/decrypt_tui_e2e_helpers_test.go
@@ -120,6 +120,7 @@ type timedSimKey struct {
R rune
Mod tcell.ModMask
WaitForText string
+ Wait time.Duration
RequireNewApp bool
SettleAfterMatch time.Duration
}
@@ -261,6 +262,11 @@ func (h *timedSimHarness) run(keys []timedSimKey) {
return
}
generation = state.generation
+ if key.Wait > 0 && strings.TrimSpace(key.WaitForText) == "" {
+ if !h.sleepOrDone(key.Wait) {
+ return
+ }
+ }
settle := key.SettleAfterMatch
if settle <= 0 {
@@ -292,7 +298,11 @@ func (h *timedSimHarness) run(keys []timedSimKey) {
func (h *timedSimHarness) waitForScreenText(index int, key timedSimKey, minGeneration int) (timedSimScreenState, bool) {
expected := strings.TrimSpace(key.WaitForText)
- timer := time.NewTimer(timedSimScreenWaitTimeout)
+ timeout := timedSimScreenWaitTimeout
+ if key.Wait > 0 {
+ timeout = key.Wait
+ }
+ timer := time.NewTimer(timeout)
defer timer.Stop()
for {
@@ -310,7 +320,7 @@ func (h *timedSimHarness) waitForScreenText(index int, key timedSimKey, minGener
"TUI simulation timed out at action %d waiting for text %q within %s (min generation=%d, current generation=%d, focus=%s)\nCurrent screen:\n%s",
index,
expected,
- timedSimScreenWaitTimeout,
+ timeout,
minGeneration,
state.generation,
state.focusType,
diff --git a/internal/orchestrator/deps_test.go b/internal/orchestrator/deps_test.go
index fa7af74f..6154e075 100644
--- a/internal/orchestrator/deps_test.go
+++ b/internal/orchestrator/deps_test.go
@@ -24,6 +24,12 @@ type FakeFS struct {
MkdirAllErr error
MkdirTempErr error
OpenFileErr map[string]error
+ Ownership map[string]FakeOwnership
+}
+
+type FakeOwnership struct {
+ UID int
+ GID int
}
func NewFakeFS() *FakeFS {
@@ -33,6 +39,7 @@ func NewFakeFS() *FakeFS {
StatErr: make(map[string]error),
StatErrors: make(map[string]error),
OpenFileErr: make(map[string]error),
+ Ownership: make(map[string]FakeOwnership),
}
}
@@ -188,7 +195,15 @@ func (f *FakeFS) Rename(oldpath, newpath string) error {
}
func (f *FakeFS) Lchown(path string, uid, gid int) error {
- return os.Lchown(f.onDisk(path), uid, gid)
+ diskPath := f.onDisk(path)
+ if _, err := os.Lstat(diskPath); err != nil {
+ return err
+ }
+ if f.Ownership == nil {
+ f.Ownership = make(map[string]FakeOwnership)
+ }
+ f.Ownership[diskPath] = FakeOwnership{UID: uid, GID: gid}
+ return nil
}
func (f *FakeFS) UtimesNano(path string, times []syscall.Timespec) error {
From df70bb452a2bbb227dc2589ad5a5a3cfb5f06766 Mon Sep 17 00:00:00 2001
From: Damiano <71268257+tis24dev@users.noreply.github.com>
Date: Wed, 6 May 2026 15:38:09 +0200
Subject: [PATCH 34/35] Add PBS/restore helpers, improve panic handling & tests
Multiple fixes and feature additions across the codebase: update README PayPal link; improve finishMainRun panic handling and ensure runDone/logging are called; allow --install to choose CLI vs TUI; make isNewerVersion treat stable > prerelease and add tests; add timeout to hostname resolution. Backup collector: honor CustomBackupPaths, add pbsRepositoryWithDatastore helper and use it when building PBS_REPOSITORY, plus new tests. Email notifier: centralize mailq lookup (findMailqPath) and use it for queue checks. Orchestrator/restore: introduce localNodeName and helpers (pveshGuestExists, pveshCreateGuestArgs, pveshArgValue, isPveshNotFoundError), ensure missing guests are created before applying configs, stop parsing at section headers, and add tests for these behaviors. TUI tests: add runCompleted signaling to timedSimHarness and ensure RunDecryptWorkflowTUI signals completion. Misc: small test and brick-call refactors.
---
README.md | 2 +-
cmd/proxsave/main_lifecycle.go | 31 +++++--
cmd/proxsave/main_modes.go | 4 +-
cmd/proxsave/main_update.go | 24 ++++--
cmd/proxsave/runtime_helpers.go | 5 +-
cmd/proxsave/version_helpers_test.go | 3 +-
internal/backup/collector.go | 38 ++++++---
internal/backup/collector_bricks_test.go | 6 +-
.../backup/collector_config_extra_test.go | 10 +++
internal/backup/collector_pbs_auth_test.go | 24 ++++++
internal/notify/email.go | 27 +++---
.../decrypt_tui_e2e_helpers_test.go | 22 ++++-
.../orchestrator/restore_cluster_apply.go | 84 ++++++++++++++++++-
internal/orchestrator/restore_errors_test.go | 36 ++++++++
internal/orchestrator/restore_test.go | 36 +++++++-
15 files changed, 302 insertions(+), 50 deletions(-)
diff --git a/README.md b/README.md
index 1823c9fd..8c5d8329 100644
--- a/README.md
+++ b/README.md
@@ -14,7 +14,7 @@ Proxmox PBS & PVE System Files Backup
[](https://rclone.org/)
[](https://github.com/sponsors/tis24dev)
[](https://github.com/sponsors/tis24dev)
-[](https://www.paypal.com/cgi-bin/webscr?cmd=_donations&business=damigioanna%40gmail.com)
+[](https://paypal.me/DNoventa)
## About the Project
diff --git a/cmd/proxsave/main_lifecycle.go b/cmd/proxsave/main_lifecycle.go
index 449d2b63..b589a07a 100644
--- a/cmd/proxsave/main_lifecycle.go
+++ b/cmd/proxsave/main_lifecycle.go
@@ -32,16 +32,35 @@ func startMainRun() runBootstrap {
}
func finishMainRun(run runBootstrap) {
+ var panicErr error
+ exitAfterCleanup := false
defer func() {
- if r := recover(); r != nil {
- stack := debug.Stack()
- run.bootstrap.Error("PANIC: %v", r)
- fmt.Fprintf(os.Stderr, "panic: %v\n%s\n", r, stack)
+ if run.bootstrap != nil && run.state != nil {
+ logging.DebugStepBootstrap(run.bootstrap, "main run", "exit_code=%d", run.state.finalExitCode)
+ }
+ if run.runDone != nil {
+ run.runDone(panicErr)
+ }
+ if exitAfterCleanup {
os.Exit(types.ExitPanicError.Int())
}
- logging.DebugStepBootstrap(run.bootstrap, "main run", "exit_code=%d", run.state.finalExitCode)
- run.runDone(nil)
}()
+
+ r := recover()
+ if r == nil {
+ return
+ }
+
+ stack := debug.Stack()
+ panicErr = fmt.Errorf("panic: %v", r)
+ exitAfterCleanup = true
+ if run.state != nil {
+ run.state.finalExitCode = types.ExitPanicError.Int()
+ }
+ if run.bootstrap != nil {
+ run.bootstrap.Error("PANIC: %v\n%s", r, stack)
+ }
+ fmt.Fprintf(os.Stderr, "panic: %v\n%s\n", r, stack)
}
func preparePreRuntimeArgs(ctx context.Context, bootstrap *logging.BootstrapLogger, toolVersion string) (*cli.Args, int, bool) {
diff --git a/cmd/proxsave/main_modes.go b/cmd/proxsave/main_modes.go
index ed2740a2..18fd2a2c 100644
--- a/cmd/proxsave/main_modes.go
+++ b/cmd/proxsave/main_modes.go
@@ -224,9 +224,11 @@ func runInstallMode(ctx context.Context, args *cli.Args, bootstrap *logging.Boot
sessionLogger.Info("Starting --install (config=%s)", args.ConfigPath)
}
- err := runInstallTUI(ctx, args.ConfigPath, bootstrap)
+ var err error
if args.ForceCLI {
err = runInstall(ctx, args.ConfigPath, bootstrap)
+ } else {
+ err = runInstallTUI(ctx, args.ConfigPath, bootstrap)
}
if err != nil {
diff --git a/cmd/proxsave/main_update.go b/cmd/proxsave/main_update.go
index f016fe48..68596e46 100644
--- a/cmd/proxsave/main_update.go
+++ b/cmd/proxsave/main_update.go
@@ -83,16 +83,19 @@ func checkForUpdates(ctx context.Context, logger *logging.Logger, currentVersion
}
}
-// isNewerVersion returns true if latest is strictly newer than current,
-// comparing MAJOR.MINOR.PATCH (ignoring any leading 'v', pre-release suffixes, and build metadata).
+// isNewerVersion returns true if latest is strictly newer than current.
+// It compares MAJOR.MINOR.PATCH, ignores build metadata, and treats a stable
+// release as newer than a prerelease with the same numeric version.
func isNewerVersion(current, latest string) bool {
- parse := func(v string) (int, int, int) {
+ parse := func(v string) (int, int, int, bool) {
v = strings.TrimSpace(v)
v = strings.TrimPrefix(v, "v")
- if i := strings.IndexByte(v, '-'); i >= 0 {
+ if i := strings.IndexByte(v, '+'); i >= 0 {
v = v[:i]
}
- if i := strings.IndexByte(v, '+'); i >= 0 {
+ hasPrerelease := false
+ if i := strings.IndexByte(v, '-'); i >= 0 {
+ hasPrerelease = true
v = v[:i]
}
@@ -112,11 +115,11 @@ func isNewerVersion(current, latest string) bool {
if len(parts) > 2 {
patch = toInt(parts[2])
}
- return major, minor, patch
+ return major, minor, patch, hasPrerelease
}
- curMaj, curMin, curPatch := parse(current)
- latMaj, latMin, latPatch := parse(latest)
+ curMaj, curMin, curPatch, curPrerelease := parse(current)
+ latMaj, latMin, latPatch, latPrerelease := parse(latest)
if latMaj != curMaj {
return latMaj > curMaj
@@ -124,5 +127,8 @@ func isNewerVersion(current, latest string) bool {
if latMin != curMin {
return latMin > curMin
}
- return latPatch > curPatch
+ if latPatch != curPatch {
+ return latPatch > curPatch
+ }
+ return curPrerelease && !latPrerelease
}
diff --git a/cmd/proxsave/runtime_helpers.go b/cmd/proxsave/runtime_helpers.go
index 0a52b206..7a778d66 100644
--- a/cmd/proxsave/runtime_helpers.go
+++ b/cmd/proxsave/runtime_helpers.go
@@ -220,7 +220,10 @@ func logServerIdentityValues(serverID, mac string) {
func resolveHostname() string {
if path, err := exec.LookPath("hostname"); err == nil {
- cmd, cmdErr := safeexec.TrustedCommandContext(context.Background(), path, "-f")
+ cmdCtx, cancel := context.WithTimeout(context.Background(), 2*time.Second)
+ defer cancel()
+
+ cmd, cmdErr := safeexec.TrustedCommandContext(cmdCtx, path, "-f")
if cmdErr == nil {
if out, err := cmd.Output(); err == nil {
if fqdn := strings.TrimSpace(string(out)); fqdn != "" {
diff --git a/cmd/proxsave/version_helpers_test.go b/cmd/proxsave/version_helpers_test.go
index bfa0890f..39399a8a 100644
--- a/cmd/proxsave/version_helpers_test.go
+++ b/cmd/proxsave/version_helpers_test.go
@@ -42,7 +42,8 @@ func TestIsNewerVersion(t *testing.T) {
{"minor newer", "0.1.9", "0.2.0", true},
{"major newer", "1.9.9", "2.0.0", true},
{"strip leading v", "v1.2.3", "1.2.4", true},
- {"ignore prerelease", "1.2.3-rc1", "1.2.3", false},
+ {"stable newer than prerelease", "1.2.3-rc1", "1.2.3", true},
+ {"prerelease not newer than stable", "1.2.3", "1.2.3-rc1", false},
{"ignore build metadata", "v1.2.3+current", "v1.2.4+latest", true},
{"build metadata does not zero patch", "v1.2.3+current", "v1.2.3+latest", false},
{"missing patch treated as 0", "1.2", "1.2.0", false},
diff --git a/internal/backup/collector.go b/internal/backup/collector.go
index 7bcbfad7..b87366a3 100644
--- a/internal/backup/collector.go
+++ b/internal/backup/collector.go
@@ -297,6 +297,9 @@ func (c *CollectorConfig) validateExcludePatterns() error {
}
func (c *CollectorConfig) hasCollectionOptionEnabled() bool {
+ if len(c.CustomBackupPaths) > 0 {
+ return true
+ }
for _, enabled := range c.collectionOptionFlags() {
if enabled {
return true
@@ -1233,6 +1236,29 @@ func (c *Collector) safeCmdOutputWithPBSAuth(ctx context.Context, spec CommandSp
return nil
}
+func pbsRepositoryWithDatastore(repository, datastoreName string) string {
+ separator := -1
+ bracketDepth := 0
+ for i, r := range repository {
+ switch r {
+ case '[':
+ bracketDepth++
+ case ']':
+ if bracketDepth > 0 {
+ bracketDepth--
+ }
+ case ':':
+ if bracketDepth == 0 {
+ separator = i
+ }
+ }
+ }
+ if separator >= 0 {
+ return repository[:separator+1] + datastoreName
+ }
+ return repository + ":" + datastoreName
+}
+
// safeCmdOutputWithPBSAuthForDatastore executes a command with PBS authentication for a specific datastore
// This function appends the datastore name to the PBS_REPOSITORY environment variable
func (c *Collector) safeCmdOutputWithPBSAuthForDatastore(ctx context.Context, spec CommandSpec, output, description, datastoreName string, critical bool) error {
@@ -1274,17 +1300,7 @@ func (c *Collector) safeCmdOutputWithPBSAuthForDatastore(ctx context.Context, sp
// Build PBS_REPOSITORY with datastore
repoWithDatastore := ""
if c.config.PBSRepository != "" {
- // If repository already has a datastore (contains :), replace it
- // Otherwise append the datastore name
- repoWithDatastore = c.config.PBSRepository
- if strings.Contains(repoWithDatastore, ":") {
- // Replace existing datastore: "user@host:oldds" -> "user@host:newds"
- parts := strings.SplitN(repoWithDatastore, ":", 2)
- repoWithDatastore = fmt.Sprintf("%s:%s", parts[0], datastoreName)
- } else {
- // Append datastore: "user@host" -> "user@host:datastore"
- repoWithDatastore = fmt.Sprintf("%s:%s", repoWithDatastore, datastoreName)
- }
+ repoWithDatastore = pbsRepositoryWithDatastore(c.config.PBSRepository, datastoreName)
} else {
// No repository configured but we have password - use root@pam as default user
repoWithDatastore = fmt.Sprintf("root@pam@localhost:%s", datastoreName)
diff --git a/internal/backup/collector_bricks_test.go b/internal/backup/collector_bricks_test.go
index 0f559a40..778e233b 100644
--- a/internal/backup/collector_bricks_test.go
+++ b/internal/backup/collector_bricks_test.go
@@ -137,7 +137,8 @@ func TestPVEGuestBrickPropagatesQEMUContextCancellation(t *testing.T) {
ctx, cancel := context.WithCancel(context.Background())
cancel()
- err := newPVEGuestBricks()[0].Run(ctx, state)
+ brick := requireBrick(t, recipe{Name: "pve-guest", Bricks: newPVEGuestBricks()}, brickPVEVMQEMUConfigs)
+ err := brick.Run(ctx, state)
if !errors.Is(err, context.Canceled) {
t.Fatalf("guest brick error = %v, want %v", err, context.Canceled)
}
@@ -154,7 +155,8 @@ func TestPVEStorageProbeBrickPropagatesContextCancellation(t *testing.T) {
ctx, cancel := context.WithCancel(context.Background())
cancel()
- err := newPVEStorageProbeBricks()[0].Run(ctx, state)
+ brick := requireBrick(t, recipe{Name: "pve-storage-probe", Bricks: newPVEStorageProbeBricks()}, brickPVEStorageProbe)
+ err := brick.Run(ctx, state)
if !errors.Is(err, context.Canceled) {
t.Fatalf("storage probe brick error = %v, want %v", err, context.Canceled)
}
diff --git a/internal/backup/collector_config_extra_test.go b/internal/backup/collector_config_extra_test.go
index 06e64f5b..e88dcf54 100644
--- a/internal/backup/collector_config_extra_test.go
+++ b/internal/backup/collector_config_extra_test.go
@@ -49,10 +49,20 @@ func TestCollectorConfigValidateAcceptsNewStandaloneCollectionOptions(t *testing
if err := tt.cfg.Validate(); err != nil {
t.Fatalf("Validate() error = %v", err)
}
+ if tt.cfg.PxarDatastoreConcurrency != 3 {
+ t.Fatalf("PxarDatastoreConcurrency = %d, want 3", tt.cfg.PxarDatastoreConcurrency)
+ }
})
}
}
+func TestCollectorConfigValidateAcceptsCustomBackupPathsOnly(t *testing.T) {
+ cfg := &CollectorConfig{CustomBackupPaths: []string{"/opt/custom"}}
+ if err := cfg.Validate(); err != nil {
+ t.Fatalf("Validate() error = %v", err)
+ }
+}
+
func TestGlobHelpers(t *testing.T) {
cases := []struct {
pattern string
diff --git a/internal/backup/collector_pbs_auth_test.go b/internal/backup/collector_pbs_auth_test.go
index 42da4656..3c9f17ae 100644
--- a/internal/backup/collector_pbs_auth_test.go
+++ b/internal/backup/collector_pbs_auth_test.go
@@ -84,6 +84,30 @@ func TestSafeCmdOutputWithPBSAuthForDatastoreBuildsRepo(t *testing.T) {
}
}
+func TestPBSRepositoryWithDatastorePreservesHostPortAndIPv6(t *testing.T) {
+ tests := []struct {
+ name string
+ repo string
+ datastore string
+ want string
+ }{
+ {name: "host only", repo: "user@host", datastore: "newds", want: "user@host:newds"},
+ {name: "existing datastore", repo: "user@host:oldds", datastore: "newds", want: "user@host:newds"},
+ {name: "host port", repo: "user@host:8007:oldds", datastore: "newds", want: "user@host:8007:newds"},
+ {name: "bracketed ipv6", repo: "[2001:db8::1]:oldds", datastore: "newds", want: "[2001:db8::1]:newds"},
+ {name: "user bracketed ipv6", repo: "user@[2001:db8::1]:oldds", datastore: "newds", want: "user@[2001:db8::1]:newds"},
+ {name: "bracketed ipv6 without datastore", repo: "[2001:db8::1]", datastore: "newds", want: "[2001:db8::1]:newds"},
+ }
+
+ for _, tt := range tests {
+ t.Run(tt.name, func(t *testing.T) {
+ if got := pbsRepositoryWithDatastore(tt.repo, tt.datastore); got != tt.want {
+ t.Fatalf("pbsRepositoryWithDatastore(%q, %q) = %q, want %q", tt.repo, tt.datastore, got, tt.want)
+ }
+ })
+ }
+}
+
func TestSafeCmdOutputWithPBSAuthForDatastoreSkipsWhenNoCredentials(t *testing.T) {
origLookPath := execLookPath
origRun := runCommandWithEnv
diff --git a/internal/notify/email.go b/internal/notify/email.go
index 61fe6cb9..da44380f 100644
--- a/internal/notify/email.go
+++ b/internal/notify/email.go
@@ -695,6 +695,19 @@ func lookupAbsolutePath(name string) (string, error) {
return "", fmt.Errorf("exec.LookPath returned non-absolute path %q", execPath)
}
+func findMailqPath() (string, error) {
+ candidates := []string{"mailq", "/usr/bin/mailq"}
+ errs := make([]error, 0, len(candidates))
+ for _, candidate := range candidates {
+ path, err := lookupAbsolutePath(candidate)
+ if err == nil {
+ return path, nil
+ }
+ errs = append(errs, fmt.Errorf("%s: %w", candidate, err))
+ }
+ return "", fmt.Errorf("mailq command not found: %w", errors.Join(errs...))
+}
+
// sendViaRelay sends email via cloud relay
func (e *EmailNotifier) sendViaRelay(ctx context.Context, recipient, subject, htmlBody, textBody string, data *NotificationData) error {
// Build payload
@@ -787,12 +800,9 @@ func (e *EmailNotifier) checkRelayHostConfigured(ctx context.Context) (bool, str
// checkMailQueue checks the mail queue status
func (e *EmailNotifier) checkMailQueue(ctx context.Context) (int, error) {
// Try mailq command (works for both Postfix and Sendmail)
- mailqPath, err := lookupAbsolutePath("mailq")
+ mailqPath, err := findMailqPath()
if err != nil {
- mailqPath, err = lookupAbsolutePath("/usr/bin/mailq")
- if err != nil {
- return 0, fmt.Errorf("mailq command not found")
- }
+ return 0, err
}
cmd, err := commandForMailTool(ctx, mailqPath)
@@ -833,12 +843,9 @@ func (e *EmailNotifier) checkMailQueue(ctx context.Context) (int, error) {
// detectQueueEntry scans the mail queue for a recipient and returns the latest queue ID.
func (e *EmailNotifier) detectQueueEntry(ctx context.Context, recipient string) (string, string, error) {
- mailqPath, err := lookupAbsolutePath("mailq")
+ mailqPath, err := findMailqPath()
if err != nil {
- mailqPath, err = lookupAbsolutePath("/usr/bin/mailq")
- if err != nil {
- return "", "", fmt.Errorf("mailq command not found")
- }
+ return "", "", err
}
cmd, err := commandForMailTool(ctx, mailqPath)
diff --git a/internal/orchestrator/decrypt_tui_e2e_helpers_test.go b/internal/orchestrator/decrypt_tui_e2e_helpers_test.go
index 4278b2f2..9dd6dc32 100644
--- a/internal/orchestrator/decrypt_tui_e2e_helpers_test.go
+++ b/internal/orchestrator/decrypt_tui_e2e_helpers_test.go
@@ -131,6 +131,8 @@ type timedSimHarness struct {
closeDoneOnce sync.Once
injectWG sync.WaitGroup
screenStateCh chan struct{}
+ runCompleted chan struct{}
+ closeRunOnce sync.Once
appMu sync.RWMutex
apps []*tui.App
@@ -174,6 +176,7 @@ func withTimedSimAppSequence(t *testing.T, keys []timedSimKey) *timedSimHarness
t: t,
done: make(chan struct{}),
screenStateCh: make(chan struct{}, 1),
+ runCompleted: make(chan struct{}),
}
t.Cleanup(func() {
@@ -224,6 +227,18 @@ func (h *timedSimHarness) notifyScreenStateChanged() {
}
}
+func (h *timedSimHarness) markRunCompleted() {
+ if h == nil {
+ return
+ }
+ if h.runCompleted == nil {
+ return
+ }
+ h.closeRunOnce.Do(func() {
+ close(h.runCompleted)
+ })
+}
+
func (h *timedSimHarness) stop() {
if h == nil {
return
@@ -289,6 +304,7 @@ func (h *timedSimHarness) run(keys []timedSimKey) {
timer := time.NewTimer(timedSimCompletionTimeout)
defer timer.Stop()
select {
+ case <-h.runCompleted:
case <-h.done:
case <-timer.C:
h.t.Errorf("TUI simulation did not finish within %s after injecting %d key(s)\n%s", timedSimCompletionTimeout, len(keys), h.describeCurrentState())
@@ -537,7 +553,11 @@ func runDecryptWorkflowTUIForTest(t *testing.T, sim *timedSimHarness, ctx contex
errCh := make(chan error, 1)
go func() {
- errCh <- RunDecryptWorkflowTUI(runCtx, cfg, logger, "1.0.0", configPath, "test-build")
+ err := RunDecryptWorkflowTUI(runCtx, cfg, logger, "1.0.0", configPath, "test-build")
+ if sim != nil {
+ sim.markRunCompleted()
+ }
+ errCh <- err
}()
waitTimeout := 30 * time.Second
diff --git a/internal/orchestrator/restore_cluster_apply.go b/internal/orchestrator/restore_cluster_apply.go
index bf9e3853..a2fbac64 100644
--- a/internal/orchestrator/restore_cluster_apply.go
+++ b/internal/orchestrator/restore_cluster_apply.go
@@ -78,7 +78,7 @@ func listExportNodeDirs(exportRoot string) ([]string, error) {
nodesRoot := filepath.Join(exportRoot, "etc/pve/nodes")
entries, err := restoreFS.ReadDir(nodesRoot)
if err != nil {
- if errors.Is(err, os.ErrNotExist) || os.IsNotExist(err) {
+ if errors.Is(err, os.ErrNotExist) {
return nil, nil
}
return nil, err
@@ -184,18 +184,40 @@ func readVMName(confPath string) string {
}
func applyVMConfigs(ctx context.Context, entries []vmEntry, logger *logging.Logger) (applied, failed int) {
+ node := localNodeName()
for _, vm := range entries {
if err := ctx.Err(); err != nil {
logger.Warning("VM apply aborted: %v", err)
return applied, failed
}
- target := fmt.Sprintf("/nodes/%s/%s/%s/config", detectNodeForVM(), vm.Kind, vm.VMID)
+ target := fmt.Sprintf("/nodes/%s/%s/%s/config", node, vm.Kind, vm.VMID)
configArgs, err := pveshArgsFromColonConfigFile(vm.Path)
if err != nil {
logger.Warning("Failed to read %s (vmid=%s kind=%s): %v", vm.Path, vm.VMID, vm.Kind, err)
failed++
continue
}
+
+ exists, err := pveshGuestExists(ctx, logger, target)
+ if err != nil {
+ logger.Warning("Failed to check existing VM/CT config %s (vmid=%s kind=%s): %v", target, vm.VMID, vm.Kind, err)
+ failed++
+ continue
+ }
+ if !exists {
+ createArgs, err := pveshCreateGuestArgs(node, vm, configArgs)
+ if err != nil {
+ logger.Warning("Failed to prepare VM/CT create for %s (vmid=%s kind=%s): %v", target, vm.VMID, vm.Kind, err)
+ failed++
+ continue
+ }
+ if err := runPvesh(ctx, logger, createArgs); err != nil {
+ logger.Warning("Failed to create VM/CT config %s (vmid=%s kind=%s): %v", target, vm.VMID, vm.Kind, err)
+ failed++
+ continue
+ }
+ }
+
args := append([]string{"set", target}, configArgs...)
if err := runPvesh(ctx, logger, args); err != nil {
logger.Warning("Failed to apply %s (vmid=%s kind=%s): %v", target, vm.VMID, vm.Kind, err)
@@ -212,7 +234,7 @@ func applyVMConfigs(ctx context.Context, entries []vmEntry, logger *logging.Logg
return applied, failed
}
-func detectNodeForVM() string {
+func localNodeName() string {
host, _ := os.Hostname()
host = shortHost(host)
if host != "" {
@@ -221,6 +243,59 @@ func detectNodeForVM() string {
return "localhost"
}
+func pveshGuestExists(ctx context.Context, logger *logging.Logger, target string) (bool, error) {
+ if err := runPvesh(ctx, logger, []string{"get", target}); err != nil {
+ if isPveshNotFoundError(err) {
+ return false, nil
+ }
+ return false, err
+ }
+ return true, nil
+}
+
+func pveshCreateGuestArgs(node string, vm vmEntry, configArgs []string) ([]string, error) {
+ args := []string{
+ "create",
+ fmt.Sprintf("/nodes/%s/%s", node, vm.Kind),
+ fmt.Sprintf("--vmid=%s", vm.VMID),
+ }
+ switch vm.Kind {
+ case "qemu":
+ return args, nil
+ case "lxc":
+ ostemplate, ok := pveshArgValue(configArgs, "ostemplate")
+ if !ok {
+ return nil, fmt.Errorf("missing ostemplate in LXC config")
+ }
+ return append(args, fmt.Sprintf("--ostemplate=%s", ostemplate)), nil
+ default:
+ return nil, fmt.Errorf("unsupported guest kind %q", vm.Kind)
+ }
+}
+
+func pveshArgValue(args []string, key string) (string, bool) {
+ prefix := "--" + key + "="
+ for _, arg := range args {
+ if strings.HasPrefix(arg, prefix) {
+ return strings.TrimPrefix(arg, prefix), true
+ }
+ }
+ return "", false
+}
+
+func isPveshNotFoundError(err error) bool {
+ if err == nil {
+ return false
+ }
+ msg := strings.ToLower(err.Error())
+ for _, marker := range []string{"not found", "does not exist", "no such", "unable to find", "404"} {
+ if strings.Contains(msg, marker) {
+ return true
+ }
+ }
+ return false
+}
+
type storageBlock struct {
ID string
Type string
@@ -238,6 +313,9 @@ func pveshArgsFromColonConfigFile(path string) ([]string, error) {
func pveshArgsFromColonConfigLines(lines []string) []string {
args := make([]string, 0, len(lines)*2)
for _, line := range lines {
+ if strings.HasPrefix(strings.TrimSpace(line), "[") {
+ break
+ }
key, value, ok := parseColonConfigLine(line)
if !ok {
continue
diff --git a/internal/orchestrator/restore_errors_test.go b/internal/orchestrator/restore_errors_test.go
index 31a2d9ce..bfe6c39e 100644
--- a/internal/orchestrator/restore_errors_test.go
+++ b/internal/orchestrator/restore_errors_test.go
@@ -1733,6 +1733,42 @@ func TestApplyVMConfigs_SuccessfulApply(t *testing.T) {
}
}
+func TestApplyVMConfigs_CreatesMissingGuestBeforeSet(t *testing.T) {
+ orig := restoreCmd
+ t.Cleanup(func() { restoreCmd = orig })
+
+ node := localNodeName()
+ getCall := fmt.Sprintf("pvesh get /nodes/%s/qemu/100/config", node)
+ fake := &FakeCommandRunner{
+ Outputs: map[string][]byte{},
+ Errors: map[string]error{
+ getCall: fmt.Errorf("not found"),
+ },
+ }
+ restoreCmd = fake
+
+ dir := t.TempDir()
+ configPath := filepath.Join(dir, "100.conf")
+ if err := os.WriteFile(configPath, []byte("name: test-vm"), 0o644); err != nil {
+ t.Fatalf("write config: %v", err)
+ }
+
+ entries := []vmEntry{{VMID: "100", Kind: "qemu", Path: configPath}}
+ logger := logging.New(logging.GetDefaultLogger().GetLevel(), false)
+ applied, failed := applyVMConfigs(context.Background(), entries, logger)
+
+ if applied != 1 || failed != 0 {
+ t.Fatalf("expected (1,0), got (%d,%d)", applied, failed)
+ }
+ calls := strings.Join(fake.CallsList(), "\n")
+ if !strings.Contains(calls, fmt.Sprintf("pvesh create /nodes/%s/qemu --vmid=100", node)) {
+ t.Fatalf("missing create call; calls=%s", calls)
+ }
+ if !strings.Contains(calls, fmt.Sprintf("pvesh set /nodes/%s/qemu/100/config --name=test-vm", node)) {
+ t.Fatalf("missing set call; calls=%s", calls)
+ }
+}
+
// --------------------------------------------------------------------------
// extractDirectory success path test
// --------------------------------------------------------------------------
diff --git a/internal/orchestrator/restore_test.go b/internal/orchestrator/restore_test.go
index 7ad785b3..622057ca 100644
--- a/internal/orchestrator/restore_test.go
+++ b/internal/orchestrator/restore_test.go
@@ -1353,17 +1353,45 @@ func TestReadVMName_FileNotFound(t *testing.T) {
}
// --------------------------------------------------------------------------
-// detectNodeForVM tests
+// localNodeName tests
// --------------------------------------------------------------------------
-func TestDetectNodeForVM_ReturnsHostname(t *testing.T) {
- node := detectNodeForVM()
- // detectNodeForVM returns the current hostname, not the node from path
+func TestLocalNodeName_ReturnsHostname(t *testing.T) {
+ node := localNodeName()
if node == "" {
t.Fatalf("expected non-empty node from hostname")
}
}
+func TestPveshArgsFromColonConfigLinesStopsAtSectionHeader(t *testing.T) {
+ args := pveshArgsFromColonConfigLines([]string{
+ "name: vm100",
+ "memory: 2048",
+ "[snapshot]",
+ "parent: base",
+ "snaptime: 123",
+ })
+
+ got := strings.Join(args, " ")
+ if !strings.Contains(got, "--name=vm100") || !strings.Contains(got, "--memory=2048") {
+ t.Fatalf("expected pre-section args, got %v", args)
+ }
+ if strings.Contains(got, "parent") || strings.Contains(got, "snaptime") {
+ t.Fatalf("snapshot section args must be ignored, got %v", args)
+ }
+}
+
+func TestPveshCreateGuestArgsIncludesLXCOstemplate(t *testing.T) {
+ args, err := pveshCreateGuestArgs("node1", vmEntry{VMID: "101", Kind: "lxc"}, []string{"--hostname=ct101", "--ostemplate=local:vztmpl/debian.tar.zst"})
+ if err != nil {
+ t.Fatalf("pveshCreateGuestArgs error = %v", err)
+ }
+ got := strings.Join(args, " ")
+ if !strings.Contains(got, "create /nodes/node1/lxc --vmid=101") || !strings.Contains(got, "--ostemplate=local:vztmpl/debian.tar.zst") {
+ t.Fatalf("unexpected create args: %v", args)
+ }
+}
+
// --------------------------------------------------------------------------
// detectConfiguredZFSPools tests
// --------------------------------------------------------------------------
From 60de173146527a1cf9b52ed0d497227436d0c287 Mon Sep 17 00:00:00 2001
From: Damiano <71268257+tis24dev@users.noreply.github.com>
Date: Wed, 6 May 2026 15:56:00 +0200
Subject: [PATCH 35/35] Refactor webhook endpoint validation
Extract pushover-specific validation from NewWebhookNotifier into a new WebhookNotifier.validateEndpoint method and instantiate the notifier earlier. NewWebhookNotifier now assigns the HTTP client to the notifier and returns it, improving readability and isolating endpoint validation. Also move the nosemgrep annotation to the end of the return statement in TrustedCommandContext to satisfy linters.
---
internal/notify/webhook.go | 60 ++++++++++++++++++++---------------
internal/safeexec/safeexec.go | 3 +-
2 files changed, 36 insertions(+), 27 deletions(-)
diff --git a/internal/notify/webhook.go b/internal/notify/webhook.go
index 08c5f629..9a020bc3 100644
--- a/internal/notify/webhook.go
+++ b/internal/notify/webhook.go
@@ -63,6 +63,11 @@ func NewWebhookNotifier(webhookConfig *config.WebhookConfig, logger *logging.Log
return nil, fmt.Errorf("webhook notifications enabled but no endpoints configured")
}
+ notifier := &WebhookNotifier{
+ config: webhookConfig,
+ logger: logger,
+ }
+
// Log each endpoint configuration (with masked sensitive data)
for i, ep := range webhookConfig.Endpoints {
logger.Debug("Endpoint #%d configuration:", i+1)
@@ -77,26 +82,8 @@ func NewWebhookNotifier(webhookConfig *config.WebhookConfig, logger *logging.Log
logger.Debug(" Header: %s (value masked)", k)
}
}
-
- format := resolveWebhookFormat(ep.Format, webhookConfig.DefaultFormat)
- method := resolveWebhookMethod(ep.Method)
- if strings.EqualFold(format, "pushover") {
- missing := []string{}
- if ep.Auth.Token == "" {
- missing = append(missing, "token")
- }
- if ep.Auth.User == "" {
- missing = append(missing, "user")
- }
- if len(missing) > 0 {
- return nil, fmt.Errorf("webhook endpoint %q: Pushover requires Auth.Token and Auth.User; missing %s", ep.Name, strings.Join(missing, "/"))
- }
- if ep.Priority < -2 || ep.Priority > 1 {
- return nil, fmt.Errorf("webhook endpoint %q: PRIORITY must be in range -2..1 (got %d); priority 2 (emergency) is not supported", ep.Name, ep.Priority)
- }
- if method != http.MethodPost {
- return nil, fmt.Errorf("webhook endpoint %q: METHOD must be POST for pushover (got %s)", ep.Name, method)
- }
+ if err := notifier.validateEndpoint(ep); err != nil {
+ return nil, err
}
}
@@ -114,11 +101,34 @@ func NewWebhookNotifier(webhookConfig *config.WebhookConfig, logger *logging.Log
logger.Info("✅ WebhookNotifier initialized successfully with %d endpoint(s)", len(webhookConfig.Endpoints))
- return &WebhookNotifier{
- config: webhookConfig,
- logger: logger,
- client: client,
- }, nil
+ notifier.client = client
+ return notifier, nil
+}
+
+func (w *WebhookNotifier) validateEndpoint(ep config.WebhookEndpoint) error {
+ format := resolveWebhookFormat(ep.Format, w.config.DefaultFormat)
+ method := resolveWebhookMethod(ep.Method)
+ if !strings.EqualFold(format, "pushover") {
+ return nil
+ }
+
+ missing := []string{}
+ if ep.Auth.Token == "" {
+ missing = append(missing, "token")
+ }
+ if ep.Auth.User == "" {
+ missing = append(missing, "user")
+ }
+ if len(missing) > 0 {
+ return fmt.Errorf("webhook endpoint %q: Pushover requires Auth.Token and Auth.User; missing %s", ep.Name, strings.Join(missing, "/"))
+ }
+ if ep.Priority < -2 || ep.Priority > 1 {
+ return fmt.Errorf("webhook endpoint %q: PRIORITY must be in range -2..1 (got %d); priority 2 (emergency) is not supported", ep.Name, ep.Priority)
+ }
+ if method != http.MethodPost {
+ return fmt.Errorf("webhook endpoint %q: METHOD must be POST for pushover (got %s)", ep.Name, method)
+ }
+ return nil
}
// Name returns the notifier name
diff --git a/internal/safeexec/safeexec.go b/internal/safeexec/safeexec.go
index ed0e9dfd..b4960255 100644
--- a/internal/safeexec/safeexec.go
+++ b/internal/safeexec/safeexec.go
@@ -199,8 +199,7 @@ func TrustedCommandContext(ctx context.Context, execPath string, args ...string)
return nil, err
}
// #nosec G204 -- execPath is absolute, regular, executable, and not world-writable.
- // nosemgrep: go.lang.security.audit.dangerous-exec-command.dangerous-exec-command
- return exec.CommandContext(ctx, execPath, args...), nil
+ return exec.CommandContext(ctx, execPath, args...), nil // nosemgrep: go.lang.security.audit.dangerous-exec-command.dangerous-exec-command
}
func ValidateTrustedExecutablePath(execPath string) error {