Release v0.39.0

.github/workflows/codecov.yml

@@ -39,7 +39,7 @@ jobs:
           go test $(go list ./... | grep -v -E '/cmd/|/pbs$|/bech32$|^github.com/tis24dev/proxsave$') -coverprofile=coverage.out
 
       - name: Upload coverage reports to Codecov
-        uses: codecov/codecov-action@fb8b3582c8e4def4969c97caa2f19720cb33a72f # v6
+        uses: codecov/codecov-action@303a32d7a59b442fa8d48b6a1cc6825c09c847a5 # v7.1.1
         with:
           token: ${{ secrets.CODECOV_TOKEN }}
           files: coverage.out

.github/workflows/codeql.yml

@@ -36,7 +36,7 @@ jobs:
           cache: false
 
       - name: Initialize CodeQL
-        uses: github/codeql-action/init@cdf488f595d80d6e07e03d4674febd5ab45fa938
+        uses: github/codeql-action/init@1c5b675653bb5c22dbe9b12b556ec555138e09fd
         with:
           languages: go
 
@@ -45,6 +45,6 @@ jobs:
         run: go build ./...
 
       - name: Perform CodeQL Analysis
-        uses: github/codeql-action/analyze@cdf488f595d80d6e07e03d4674febd5ab45fa938
+        uses: github/codeql-action/analyze@1c5b675653bb5c22dbe9b12b556ec555138e09fd
         with:
           category: "/language:go"

.github/workflows/security-ultimate.yml

@@ -94,7 +94,7 @@ jobs:
       # UPLOAD SARIF
       ########################################
       - name: Upload GoSec SARIF
-        uses: github/codeql-action/upload-sarif@cdf488f595d80d6e07e03d4674febd5ab45fa938
+        uses: github/codeql-action/upload-sarif@1c5b675653bb5c22dbe9b12b556ec555138e09fd
         with:
           sarif_file: gosec.sarif
 

cmd/proxsave/main_runtime.go

@@ -67,6 +67,51 @@ func logDetectionProvenance(bootstrap *logging.BootstrapLogger, info *environmen
 	for _, step := range info.Steps {
 		bootstrap.Debug("Detection probe: %s", step)
 	}
+	reportDetectionResidue(bootstrap, info)
+}
+
+// reportDetectionResidue reports a product whose files are present while no marker
+// proved it installed. The trace above says the same thing, but only on a debug run,
+// and the operator who needs this is the one looking at a type they did not expect:
+// they see PVE where a PBS directory exists, and nothing at a normal level tells them
+// that ProxSave looked at that directory and decided it does not count (issue #315).
+//
+// INFO, not warning, and the level is the point. A residue is recorded only when a
+// product was NOT proved installed, which leaves exactly two situations, and the
+// mount-shape matrix confirms there is no third:
+//
+//   - the type came out pve, pbs or dual: the product is genuinely absent, the backup
+//     is complete and correct, and the leftovers are untidy filesystem, not a fault.
+//     Warning here pinned such a host at exit 1 on every run, for something its
+//     operator often cannot remove (/var/lib/proxmox-backup belongs to the PVE
+//     file-restore stack), so a nightly monitor would alarm forever on a healthy node.
+//   - the type came out unknown: that IS a fault, and it already carries three
+//     warnings that decide the exit code between them - the detection error here,
+//     which now names the residue, the host-backup mount warning, and the collector
+//     saying it is collecting generic system info only. A fourth would be noise.
+//
+// The wording also stops short of declaring the package absent, which the residue does
+// not prove: dpkgPackageInstalled returns false both when the stanza says
+// not-installed and when the status file cannot be read, and under SYSTEM_ROOT_PREFIX
+// the second is the common case.
+func reportDetectionResidue(bootstrap *logging.BootstrapLogger, info *environment.EnvironmentInfo) {
+	if bootstrap == nil || info == nil {
+		return
+	}
+	for _, residue := range []struct {
+		product string
+		marker  string
+		pkg     string
+	}{
+		{"PVE", info.PVEResidual, "pve-manager"},
+		{"PBS", info.PBSResidual, "proxmox-backup-server"},
+	} {
+		if strings.TrimSpace(residue.marker) == "" {
+			continue
+		}
+		bootstrap.Info("%s files without a %s install - %s is present but nothing proved %s is installed, so this host is collected as %s",
+			residue.product, residue.product, residue.marker, residue.pkg, info.Type)
+	}
 }
 
 // detectionSourceLabel keeps the verdict line readable when a product was not

cmd/proxsave/main_runtime_residue_test.go

@@ -0,0 +1,131 @@
+package main
+
+import (
+	"os"
+	"strings"
+	"testing"
+
+	"github.com/tis24dev/proxsave/internal/environment"
+	"github.com/tis24dev/proxsave/internal/logging"
+	"github.com/tis24dev/proxsave/internal/types"
+)
+
+// replayedWarnings returns what ReplayConsoleSince prints for entries recorded after
+// mark. That method replays warning and worse only, so an empty result is proof the
+// entries in between were below warning.
+func replayedWarnings(t *testing.T, bootstrap *logging.BootstrapLogger, mark int) string {
+	t.Helper()
+	old := os.Stderr
+	r, w, err := os.Pipe()
+	if err != nil {
+		t.Fatal(err)
+	}
+	os.Stderr = w
+	done := make(chan string, 1)
+	go func() {
+		var sb strings.Builder
+		buf := make([]byte, 4096)
+		for {
+			n, err := r.Read(buf)
+			sb.Write(buf[:n])
+			if err != nil {
+				break
+			}
+		}
+		done <- sb.String()
+	}()
+	bootstrap.ReplayConsoleSince(mark)
+	_ = w.Close()
+	os.Stderr = old
+	return <-done
+}
+
+// TestResidueIsReportedBelowWarning pins the level, not just the presence of the line.
+// A residue is recorded only when a product was NOT proved installed, and that leaves
+// two cases: a correct verdict, where the product is genuinely absent and the backup is
+// complete, or an unknown verdict, which already carries its own warnings. Reporting
+// this at warning pinned an otherwise healthy host at exit 1 on every single run, for
+// leftovers its operator often cannot delete.
+func TestResidueIsReportedBelowWarning(t *testing.T) {
+	bootstrap := logging.NewBootstrapLogger()
+	mark := bootstrap.EntryCount()
+
+	reportDetectionResidue(bootstrap, &environment.EnvironmentInfo{
+		Type:        types.ProxmoxVE,
+		PBSResidual: "directory (/etc/proxmox-backup)",
+	})
+
+	if got := bootstrap.EntryCount() - mark; got != 1 {
+		t.Fatalf("recorded %d entries, want 1", got)
+	}
+	if replayed := replayedWarnings(t, bootstrap, mark); replayed != "" {
+		t.Fatalf("the residue line replayed as warning-or-worse, so it still promotes a clean run off exit 0:\n%s", replayed)
+	}
+}
+
+// TestResidueStaysQuietWhenAProductWasFound: a product that was found records no
+// residue, so a healthy host of either kind must produce no line at all. A message that
+// appeared on every run would be ignored by the time it mattered.
+//
+// It is the RESIDUE FIELD being empty that makes it quiet, not the ladder stopping
+// early: since the versionless-command rung keeps walking, a host whose command
+// answered without a version does reach the residue rungs, and detectPVE/detectPBS
+// still return an empty residue because the product was proved installed. That is
+// covered on the detection side by TestAVersionlessCommandStillProvesTheInstall.
+func TestResidueStaysQuietWhenAProductWasFound(t *testing.T) {
+	for _, info := range []*environment.EnvironmentInfo{
+		{Type: types.ProxmoxVE},
+		{Type: types.ProxmoxBS},
+		{Type: types.ProxmoxDual},
+		{Type: types.ProxmoxVE, PVEResidual: "   "},
+	} {
+		bootstrap := logging.NewBootstrapLogger()
+		before := bootstrap.EntryCount()
+		reportDetectionResidue(bootstrap, info)
+		if got := bootstrap.EntryCount() - before; got != 0 {
+			t.Fatalf("type %s with residual %q logged %d entries, want silence", info.Type, info.PVEResidual, got)
+		}
+	}
+}
+
+// TestResidueIsReportedOncePerProduct: the host in issue #315 has PBS residue and no
+// PBS, and the operator looking at a type they did not expect never sees the trace,
+// which is debug only.
+func TestResidueIsReportedOncePerProduct(t *testing.T) {
+	for _, tc := range []struct {
+		name string
+		info *environment.EnvironmentInfo
+		want int
+	}{
+		{
+			name: "pbs residue on a pve host",
+			info: &environment.EnvironmentInfo{Type: types.ProxmoxVE, PBSResidual: "directory (/etc/proxmox-backup)"},
+			want: 1,
+		},
+		{
+			name: "both products left something behind",
+			info: &environment.EnvironmentInfo{
+				Type:        types.ProxmoxUnknown,
+				PVEResidual: "directory (/etc/pve)",
+				PBSResidual: "directory (/etc/proxmox-backup)",
+			},
+			want: 2,
+		},
+	} {
+		t.Run(tc.name, func(t *testing.T) {
+			bootstrap := logging.NewBootstrapLogger()
+			before := bootstrap.EntryCount()
+			reportDetectionResidue(bootstrap, tc.info)
+			if got := bootstrap.EntryCount() - before; got != tc.want {
+				t.Fatalf("logged %d entries, want %d", got, tc.want)
+			}
+		})
+	}
+}
+
+// TestResidueReportSurvivesNilArguments: it runs on the bootstrap path, before the
+// main logger exists, and must not be the thing that takes a run down.
+func TestResidueReportSurvivesNilArguments(t *testing.T) {
+	reportDetectionResidue(nil, &environment.EnvironmentInfo{PBSResidual: "directory (/etc/proxmox-backup)"})
+	reportDetectionResidue(logging.NewBootstrapLogger(), nil)
+}

docs/COLLECTOR_ARCHITECTURE.md

@@ -50,13 +50,12 @@ split across files:
 - `newPVERecipe()` (`collector_bricks_pve.go`)
 - `newPBSRecipe()` (`collector_bricks_pbs.go`)
 - `newSystemRecipe()` (`collector_bricks_system.go`)
-- `newDualRecipe()` (`collector_bricks.go`)
 
 ### Composition Rules
 
 - `newPVERecipe()` = PVE-only bricks
 - `newPBSRecipe()` = PBS-only bricks
-- `newDualRecipe()` = PVE bricks + PBS bricks
+- the dual branch runs `newPVERecipe()` and `newPBSRecipe()` as two recipes over one state
 - `newSystemRecipe()` = common/system bricks only
 
 `system/common` is executed once. It is not duplicated inside `dual`.
@@ -125,8 +124,15 @@ user-list.
 
 ## Dual Branch
 
-`CollectDualConfigs()` runs `newDualRecipe()` and collects both product roles in
-a single backup run.
+`CollectDualConfigs()` runs `newPVERecipe()` and `newPBSRecipe()` as two separate
+recipes over one shared collection state, and collects both product roles in a single
+backup run.
+
+They are separate on purpose. `runRecipe` is fail-fast, so as one concatenated recipe
+an abort anywhere in the PBS half discarded the PVE payload already collected, and the
+workspace holding it was deleted (issue #315). A half that fails is now recorded in
+`incomplete_targets` and reported at warning; the other half is kept. Both halves
+failing is still an error, because no role payload is left to keep.
 
 Important semantics:
 
@@ -219,7 +225,7 @@ real collector flow.
 ## Related Files
 
 - `internal/backup/collector.go`
-- `internal/backup/collector_bricks.go` (recipe machinery, brick IDs, `newDualRecipe()`)
+- `internal/backup/collector_bricks.go` (recipe machinery, brick IDs)
 - `internal/backup/collector_bricks_pve.go` (`newPVERecipe()`)
 - `internal/backup/collector_bricks_pbs.go` (`newPBSRecipe()`, `newPBSUserConfigRecipe()`)
 - `internal/backup/collector_bricks_system.go` (`newSystemRecipe()`)

docs/DEVELOPER_GUIDE.md

@@ -166,7 +166,6 @@ wrappers. It is built from explicit recipes and fine-grained bricks:
 
 - `newPVERecipe()`
 - `newPBSRecipe()`
-- `newDualRecipe()`
 - `newSystemRecipe()`
 
 Important invariants:

docs/RESTORE_TECHNICAL.md

@@ -552,34 +552,53 @@ if err := ValidateCompatibility(systemType, backupType); err != nil {
 **System Detection** (`compatibility.go`):
 ```go
 func DetectCurrentSystem() SystemType {
-    hasPVE := fileExists("/etc/pve") || fileExists("/usr/bin/qm") || fileExists("/usr/bin/pct")
-    hasPBS := fileExists("/etc/proxmox-backup") || fileExists("/usr/sbin/proxmox-backup-proxy")
-
-    switch {
-    case hasPVE && hasPBS:
-        return SystemTypeDual
-    case hasPVE:
-        return SystemTypePVE
-    case hasPBS:
-        return SystemTypePBS
-    default:
-        return SystemTypeUnknown
-    }
+    info, _ := detectEnvironment() // environment.Detect
+    // maps types.ProxmoxDual/VE/BS onto SystemTypeDual/PVE/PBS, unknown otherwise
 }
 ```
 
+Restore does not carry its own rule for what this host is: it reads the same detection
+ladder the backup side uses. The rule it replaced tested `/etc/proxmox-backup` OR
+`/usr/sbin/proxmox-backup-proxy`, and that second path exists on no PBS release (the
+proxy is a systemd unit, not a binary on PATH), so every PBS restore decision rested on
+one directory that no package owns and no removal deletes. That is the marker which
+turned a PVE-only host into a dual backup in issue #315.
+
 Restore compatibility is therefore **capability-based**, not exact-match only.
 
-**Backup Type Detection**:
+**Backup Type Detection** subtracts any role the archive was meant to carry and does
+not, recorded in the sidecar manifest as `incomplete_targets`. A dual run that lost its
+PBS half ships a PVE archive and is treated as one, so it reports partial compatibility
+against a dual host instead of clearing the check as though nothing were missing.
+
 ```go
 func DetectBackupType(manifest *backup.Manifest) SystemType {
-    if len(manifest.ProxmoxTargets) > 0 {
-        return parseSystemTargets(manifest.ProxmoxTargets)
+    if manifest == nil {
+        return SystemTypeUnknown
+    }
+    // Declared targets minus every role recorded in incomplete_targets.
+    if targets := completedTargets(manifest); len(targets) > 0 {
+        return parseSystemTargets(targets)
+    }
+    if len(manifest.ProxmoxTargets) > 0 && len(manifest.IncompleteTargets) > 0 {
+        // Every declared target failed: the archive carries no role payload at all.
+        return SystemTypeUnknown
     }
     if manifest.ProxmoxType != "" {
-        return parseSystemTypeString(manifest.ProxmoxType)
+        if backupType := parseSystemTypeString(manifest.ProxmoxType); backupType != SystemTypeUnknown {
+            return backupType
+        }
     }
     // Fallback: hostname heuristics
+    if manifest.Hostname != "" {
+        hostname := strings.ToLower(manifest.Hostname)
+        if strings.Contains(hostname, "pve") {
+            return SystemTypePVE
+        }
+        if strings.Contains(hostname, "pbs") {
+            return SystemTypePBS
+        }
+    }
     return SystemTypeUnknown
 }
 ```

go.mod

@@ -10,9 +10,9 @@ require (
 	github.com/Masterminds/semver/v3 v3.5.0
 	github.com/charmbracelet/colorprofile v0.4.3
 	github.com/charmbracelet/x/ansi v0.11.8
-	golang.org/x/crypto v0.56.0
-	golang.org/x/term v0.45.0
-	golang.org/x/text v0.41.0
+	golang.org/x/crypto v0.57.0
+	golang.org/x/term v0.46.0
+	golang.org/x/text v0.42.0
 )
 
 require (
@@ -30,6 +30,6 @@ require (
 	github.com/muesli/cancelreader v0.2.2 // indirect
 	github.com/rivo/uniseg v0.4.7 // indirect
 	github.com/xo/terminfo v0.0.0-20220910002029-abceb7e1c41e // indirect
-	golang.org/x/sync v0.22.0 // indirect
-	golang.org/x/sys v0.47.0 // indirect
+	golang.org/x/sync v0.23.0 // indirect
+	golang.org/x/sys v0.48.0 // indirect
 )

go.sum

@@ -46,15 +46,15 @@ github.com/rivo/uniseg v0.4.7 h1:WUdvkW8uEhrYfLC4ZzdpI2ztxP1I582+49Oc5Mq64VQ=
 github.com/rivo/uniseg v0.4.7/go.mod h1:FN3SvrM+Zdj16jyLfmOkMNblXMcoc8DfTHruCPUcx88=
 github.com/xo/terminfo v0.0.0-20220910002029-abceb7e1c41e h1:JVG44RsyaB9T2KIHavMF/ppJZNG9ZpyihvCd0w101no=
 github.com/xo/terminfo v0.0.0-20220910002029-abceb7e1c41e/go.mod h1:RbqR21r5mrJuqunuUZ/Dhy/avygyECGrLceyNeo4LiM=
-golang.org/x/crypto v0.56.0 h1:GUh5Ii4J5jtcseSMiRqr1jXCNHoxjeV9Fmekc2oLy6Y=
-golang.org/x/crypto v0.56.0/go.mod h1:OMW5y6CY9l38uPLmxU6l6pwcXp1obtLo3e6gT7gQR2I=
+golang.org/x/crypto v0.57.0 h1:3ZVCjf8Ggz7zneR/EHRVx68Ctf+2pmIMP2UFhh9cC6M=
+golang.org/x/crypto v0.57.0/go.mod h1:Fdz0i5U6CoizGwLda9DttjSk6qlZo25zYNtR+ycvuZA=
 golang.org/x/exp v0.0.0-20231006140011-7918f672742d h1:jtJma62tbqLibJ5sFQz8bKtEM8rJBtfilJ2qTU199MI=
 golang.org/x/exp v0.0.0-20231006140011-7918f672742d/go.mod h1:ldy0pHrwJyGW56pPQzzkH36rKxoZW1tw7ZJpeKx+hdo=
-golang.org/x/sync v0.22.0 h1:SZjpbeLmrCk4xhRSZFNZW5gFUeCeFgjekvI/+gfScek=
-golang.org/x/sync v0.22.0/go.mod h1:9xrNwdLfx4jkKbNva9FpL6vEN7evnE43NNNJQ2LF3+0=
-golang.org/x/sys v0.47.0 h1:o7XGOvZQCADBQQ4Y7VNq2dRWQR7JmOUW8Kxx4ZsNgWs=
-golang.org/x/sys v0.47.0/go.mod h1:4GL1E5IUh+htKOUEOaiffhrAeqysfVGipDYzABqnCmw=
-golang.org/x/term v0.45.0 h1:NwWyBmoJCbfTHpxrWoZ9C6/VxOf7ic219I8xZZFdrf0=
-golang.org/x/term v0.45.0/go.mod h1:9aqxs0blBcrm/n0L9QW0aRVD+ktan8ssZromtqJC43w=
-golang.org/x/text v0.41.0 h1:vz/seA0lnX87Othu2f/0L24RcgrXD9/YFTSuGjj3rH8=
-golang.org/x/text v0.41.0/go.mod h1:jvf1O8ajNzZqhSrQBPbutR/EB83Cc0CFrezNQIwbb5M=
+golang.org/x/sync v0.23.0 h1:KameEIfc1IkluZyXWLn39Wd4tURc6GbCiISGiZm2bQk=
+golang.org/x/sync v0.23.0/go.mod h1:sUUOizhqBxiL6pEWpqNLUiaJn1ShEbZ6BBqskPbjZm0=
+golang.org/x/sys v0.48.0 h1:bbX/i/6MgT9BVLM9RT1thmxL04yeTAhbEz4SyadbXoo=
+golang.org/x/sys v0.48.0/go.mod h1:hNLxWAXmnKAxqDtdwIYC4bM9oQPEecfsnNMuSxOs3og=
+golang.org/x/term v0.46.0 h1:3+OXuTbaKDgwk8jTi3aSLHRlmWqHEUDUtxnbFigO4YE=
+golang.org/x/term v0.46.0/go.mod h1:+K02xbkittuwc0Am4abfA3Fc+XRGXkvBXNO88NCXPoc=
+golang.org/x/text v0.42.0 h1:JbOZXgfeCPU9gacVtYliJqOhD+zhrEqK4LfdpmlUZqI=
+golang.org/x/text v0.42.0/go.mod h1:ojzP1Z+2QtioaF8DTtO8K5q7JWVVYwZKenzujK0Zd0E=