From f62f3d600132a3e4e95138164b9ac98531997810 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Sun, 22 Feb 2026 18:51:14 +0100 Subject: [PATCH 1/4] Bump filippo.io/edwards25519 from 1.1.0 to 1.1.1 (#160) Bumps [filippo.io/edwards25519](https://github.com/FiloSottile/edwards25519) from 1.1.0 to 1.1.1. - [Commits](https://github.com/FiloSottile/edwards25519/compare/v1.1.0...v1.1.1) --- updated-dependencies: - dependency-name: filippo.io/edwards25519 dependency-version: 1.1.1 dependency-type: indirect ... 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 717ff99..4a36bb8 100644 --- a/go.mod +++ b/go.mod @@ -14,7 +14,7 @@ require ( ) require ( - filippo.io/edwards25519 v1.1.0 // indirect + filippo.io/edwards25519 v1.1.1 // indirect filippo.io/hpke v0.4.0 // indirect github.com/gdamore/encoding v1.0.1 // indirect github.com/lucasb-eyer/go-colorful v1.3.0 // indirect diff --git a/go.sum b/go.sum index ab99f6d..3ce3e36 100644 --- a/go.sum +++ b/go.sum @@ -2,8 +2,8 @@ c2sp.org/CCTV/age v0.0.0-20251208015420-e9274a7bdbfd h1:ZLsPO6WdZ5zatV4UfVpr7oAw c2sp.org/CCTV/age v0.0.0-20251208015420-e9274a7bdbfd/go.mod h1:SrHC2C7r5GkDk8R+NFVzYy/sdj0Ypg9htaPXQq5Cqeo= filippo.io/age v1.3.1 h1:hbzdQOJkuaMEpRCLSN1/C5DX74RPcNCk6oqhKMXmZi0= filippo.io/age v1.3.1/go.mod h1:EZorDTYUxt836i3zdori5IJX/v2Lj6kWFU0cfh6C0D4= -filippo.io/edwards25519 v1.1.0 h1:FNf4tywRC1HmFuKW5xopWpigGjJKiJSV0Cqo0cJWDaA= -filippo.io/edwards25519 v1.1.0/go.mod h1:BxyFTGdWcka3PhytdK4V28tE5sGfRvvvRV7EaN4VDT4= +filippo.io/edwards25519 v1.1.1 h1:YpjwWWlNmGIDyXOn8zLzqiD+9TyIlPhGFG96P39uBpw= +filippo.io/edwards25519 v1.1.1/go.mod h1:BxyFTGdWcka3PhytdK4V28tE5sGfRvvvRV7EaN4VDT4= 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= From f0fe6e99abc42a963ecc3482841cff1a707dbe7b Mon Sep 17 00:00:00 2001 From: Damiano <71268257+tis24dev@users.noreply.github.com> Date: Mon, 23 Feb 2026 00:01:50 +0100 Subject: [PATCH 2/4] Remove ENABLE_GO_BACKUP flag and legacy wrappers Remove legacy Go-pipeline compatibility and related dead code. Deleted the prefilter-manual command and removed references to ENABLE_GO_BACKUP from configs and docs. Dropped Config.EnableGoBackup and its tests, cleaned up proxsave logging that referenced the flag. Consolidated bundle creation by removing the package-level createBundle wrapper and updating callers to use Orchestrator.createBundle; removed several legacy/compat helper functions in identity and orchestrator and adjusted unit tests to call the new helpers (encodeProtectedServerIDWithMACs, collectMACCandidates, etc.). Miscellaneous test cleanup: removed obsolete fake FS/test helpers no longer needed. These changes simplify code paths and eliminate obsolete compatibility layers. --- cmd/prefilter-manual/main.go | 59 -------------------- cmd/proxsave/main.go | 7 --- docs/CONFIGURATION.md | 5 -- docs/EXAMPLES.md | 1 - internal/config/config.go | 2 - internal/config/config_test.go | 27 --------- internal/config/templates/backup.env | 1 - internal/identity/identity.go | 19 ------- internal/identity/identity_test.go | 42 ++++++-------- internal/orchestrator/bundle_test.go | 35 ------------ internal/orchestrator/decrypt_test.go | 37 ------------ internal/orchestrator/decrypt_workflow_ui.go | 3 +- internal/orchestrator/orchestrator.go | 6 -- 13 files changed, 20 insertions(+), 224 deletions(-) delete mode 100644 cmd/prefilter-manual/main.go diff --git a/cmd/prefilter-manual/main.go b/cmd/prefilter-manual/main.go deleted file mode 100644 index 6f0d1a8..0000000 --- a/cmd/prefilter-manual/main.go +++ /dev/null @@ -1,59 +0,0 @@ -package main - -import ( - "context" - "flag" - "os" - "path/filepath" - "strings" - - "github.com/tis24dev/proxsave/internal/backup" - "github.com/tis24dev/proxsave/internal/logging" - "github.com/tis24dev/proxsave/internal/types" -) - -func parseLogLevel(raw string) types.LogLevel { - switch strings.ToLower(strings.TrimSpace(raw)) { - case "debug": - return types.LogLevelDebug - case "info", "": - return types.LogLevelInfo - case "warning", "warn": - return types.LogLevelWarning - case "error": - return types.LogLevelError - default: - return types.LogLevelInfo - } -} - -func main() { - var ( - root string - maxSize int64 - levelLabel string - ) - - flag.StringVar(&root, "root", "/tmp/test_prefilter", "Root directory to run prefilter on") - flag.Int64Var(&maxSize, "max-size", 8*1024*1024, "Max file size (bytes) to prefilter") - flag.StringVar(&levelLabel, "log-level", "info", "Log level: debug|info|warn|error") - flag.Parse() - - root = filepath.Clean(strings.TrimSpace(root)) - if root == "" || root == "." { - root = string(os.PathSeparator) - } - - logger := logging.New(parseLogLevel(levelLabel), false) - logger.SetOutput(os.Stdout) - - cfg := backup.OptimizationConfig{ - EnablePrefilter: true, - PrefilterMaxFileSizeBytes: maxSize, - } - - if err := backup.ApplyOptimizations(context.Background(), logger, root, cfg); err != nil { - logger.Error("Prefilter failed: %v", err) - os.Exit(1) - } -} diff --git a/cmd/proxsave/main.go b/cmd/proxsave/main.go index 1d51c69..185e7b6 100644 --- a/cmd/proxsave/main.go +++ b/cmd/proxsave/main.go @@ -1302,13 +1302,6 @@ func run() int { fmt.Println() - if !cfg.EnableGoBackup && !args.Support { - logging.Warning("ENABLE_GO_BACKUP=false is ignored; the Go backup pipeline is always used.") - } else { - logging.Debug("Go backup pipeline enabled") - } - fmt.Println() - // Storage info logging.Info("Storage configuration:") logging.Info(" Primary: %s", formatStorageLabel(cfg.BackupPath, localFS)) diff --git a/docs/CONFIGURATION.md b/docs/CONFIGURATION.md index fddead4..87e655b 100644 --- a/docs/CONFIGURATION.md +++ b/docs/CONFIGURATION.md @@ -46,9 +46,6 @@ Complete reference for all 200+ configuration variables in `configs/backup.env`. # Enable/disable backup system BACKUP_ENABLED=true # true | false -# Enable Go pipeline (vs legacy Bash) -ENABLE_GO_BACKUP=true # true | false - # Colored output in terminal USE_COLOR=true # true | false @@ -922,8 +919,6 @@ METRICS_ENABLED=false # true | false METRICS_PATH=${BASE_DIR}/metrics # Empty = /var/lib/prometheus/node-exporter ``` -> ℹ️ Metrics export is available only for the Go pipeline (`ENABLE_GO_BACKUP=true`). - **Output**: Creates `proxmox_backup.prom` in `METRICS_PATH` with: - Backup duration and start/end timestamps - Archive size and raw bytes collected diff --git a/docs/EXAMPLES.md b/docs/EXAMPLES.md index 0a53278..3f8a6eb 100644 --- a/docs/EXAMPLES.md +++ b/docs/EXAMPLES.md @@ -866,7 +866,6 @@ CLOUD_LOG_PATH= # configs/backup.env SYSTEM_ROOT_PREFIX=/mnt/snapshot-root # points to the alternate root BACKUP_ENABLED=true -ENABLE_GO_BACKUP=true # /etc, /var, /root, /home are resolved under the prefix ``` diff --git a/internal/config/config.go b/internal/config/config.go index 1bea4f2..bb35388 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -33,7 +33,6 @@ type Config struct { DebugLevel types.LogLevel UseColor bool ColorizeStepLogs bool - EnableGoBackup bool ProfilingEnabled bool BaseDir string DryRun bool @@ -423,7 +422,6 @@ func (c *Config) parseOptimizationSettings() { } func (c *Config) parseSecuritySettings() { - c.EnableGoBackup = c.getBoolWithFallback([]string{"ENABLE_GO_BACKUP", "ENABLE_GO_PIPELINE"}, true) c.DisableNetworkPreflight = c.getBool("DISABLE_NETWORK_PREFLIGHT", false) // Base directory diff --git a/internal/config/config_test.go b/internal/config/config_test.go index eb26f81..3fb7ab0 100644 --- a/internal/config/config_test.go +++ b/internal/config/config_test.go @@ -427,38 +427,11 @@ func TestConfigDefaults(t *testing.T) { t.Errorf("Default LocalRetentionDays = %d; want 7", cfg.LocalRetentionDays) } - if !cfg.EnableGoBackup { - t.Error("Expected default EnableGoBackup to be true") - } - if cfg.BaseDir != "/defaults/base" { t.Errorf("Default BaseDir = %q; want %q", cfg.BaseDir, "/defaults/base") } } -func TestEnableGoBackupFlag(t *testing.T) { - tmpDir := t.TempDir() - configPath := filepath.Join(tmpDir, "go_pipeline.env") - - content := `ENABLE_GO_BACKUP=false -` - if err := os.WriteFile(configPath, []byte(content), 0644); err != nil { - t.Fatalf("Failed to create test config: %v", err) - } - - cleanup := setBaseDirEnv(t, "/flag/base") - defer cleanup() - - cfg, err := LoadConfig(configPath) - if err != nil { - t.Fatalf("LoadConfig() error = %v", err) - } - - if cfg.EnableGoBackup { - t.Error("Expected EnableGoBackup to be false when explicitly disabled") - } -} - func TestLoadConfigBaseDirFromConfig(t *testing.T) { tmpDir := t.TempDir() configPath := filepath.Join(tmpDir, "base_dir.env") diff --git a/internal/config/templates/backup.env b/internal/config/templates/backup.env index 9c3ec39..2dbf810 100644 --- a/internal/config/templates/backup.env +++ b/internal/config/templates/backup.env @@ -7,7 +7,6 @@ # General settings # ---------------------------------------------------------------------- BACKUP_ENABLED=true -ENABLE_GO_BACKUP=true PROFILING_ENABLED=true # Enable CPU/heap profiling (pprof) for Go pipeline USE_COLOR=true COLORIZE_STEP_LOGS=true # Highlight "Step N/8" lines (requires USE_COLOR=true) diff --git a/internal/identity/identity.go b/internal/identity/identity.go index 0e43740..87ae582 100644 --- a/internal/identity/identity.go +++ b/internal/identity/identity.go @@ -170,11 +170,6 @@ func collectMACCandidates(logger *logging.Logger) ([]macCandidate, []string) { return candidates, macs } -func collectMACAddresses() []string { - _, macs := collectMACCandidates(nil) - return macs -} - func selectPreferredMAC(candidates []macCandidate) (string, string) { var best *macCandidate for i := range candidates { @@ -469,10 +464,6 @@ func buildSystemData(macs []string, logger *logging.Logger) string { return builder.String() } -func encodeProtectedServerID(serverID, primaryMAC string, logger *logging.Logger) (string, error) { - return encodeProtectedServerIDWithMACs(serverID, []string{primaryMAC}, primaryMAC, logger) -} - func encodeProtectedServerIDWithMACs(serverID string, macs []string, primaryMAC string, logger *logging.Logger) (string, error) { logDebug(logger, "Identity: encodeProtectedServerID: start (serverID=%s primaryMAC=%s)", serverID, primaryMAC) keyField := buildIdentityKeyField(macs, primaryMAC, logger) @@ -574,16 +565,6 @@ func decodeProtectedServerID(fileContent, primaryMAC string, logger *logging.Log return serverID, matchedByMAC, nil } -func generateSystemKey(primaryMAC string, logger *logging.Logger) string { - machineID := readMachineID(logger) - hostnamePart := readHostnamePart(logger) - - macPart := strings.ReplaceAll(primaryMAC, ":", "") - key := computeSystemKey(machineID, hostnamePart, macPart) - logDebug(logger, "Identity: generateSystemKey: systemKey=%s", key) - return key -} - func buildIdentityKeyField(macs []string, primaryMAC string, logger *logging.Logger) string { machineID := readMachineID(logger) hostnamePart := readHostnamePart(logger) diff --git a/internal/identity/identity_test.go b/internal/identity/identity_test.go index d7a6354..8271696 100644 --- a/internal/identity/identity_test.go +++ b/internal/identity/identity_test.go @@ -15,11 +15,15 @@ import ( "github.com/tis24dev/proxsave/internal/types" ) +func encodeProtectedServerIDForTest(serverID, primaryMAC string, logger *logging.Logger) (string, error) { + return encodeProtectedServerIDWithMACs(serverID, []string{primaryMAC}, primaryMAC, logger) +} + func TestEncodeDecodeProtectedServerIDRoundTrip(t *testing.T) { const serverID = "1234567890123456" const mac = "aa:bb:cc:dd:ee:ff" - content, err := encodeProtectedServerID(serverID, mac, nil) + content, err := encodeProtectedServerIDForTest(serverID, mac, nil) if err != nil { t.Fatalf("encodeProtectedServerID() error = %v", err) } @@ -57,7 +61,7 @@ func TestDecodeProtectedServerIDAcceptsDifferentMACOnSameHost(t *testing.T) { } const serverID = "1111222233334444" - content, err := encodeProtectedServerID(serverID, "aa:bb:cc:dd:ee:ff", nil) + content, err := encodeProtectedServerIDForTest(serverID, "aa:bb:cc:dd:ee:ff", nil) if err != nil { t.Fatalf("encodeProtectedServerID() error = %v", err) } @@ -95,7 +99,7 @@ func TestDecodeProtectedServerIDRejectsDifferentHost(t *testing.T) { } const serverID = "1111222233334444" - content, err := encodeProtectedServerID(serverID, "aa:bb:cc:dd:ee:ff", nil) + content, err := encodeProtectedServerIDForTest(serverID, "aa:bb:cc:dd:ee:ff", nil) if err != nil { t.Fatalf("encodeProtectedServerID() error = %v", err) } @@ -147,7 +151,7 @@ func TestDecodeProtectedServerIDDetectsCorruptedData(t *testing.T) { const serverID = "5555666677778888" const mac = "aa:aa:aa:aa:aa:aa" - content, err := encodeProtectedServerID(serverID, mac, nil) + content, err := encodeProtectedServerIDForTest(serverID, mac, nil) if err != nil { t.Fatalf("encodeProtectedServerID() error = %v", err) } @@ -204,14 +208,14 @@ func TestDetectUsesExistingIdentityFile(t *testing.T) { } identityPath := filepath.Join(identityDir, identityFileName) - macs := collectMACAddresses() + _, macs := collectMACCandidates(nil) if len(macs) == 0 { t.Skip("no non-loopback MACs available on this system") } primary := macs[0] const serverID = "1234567890123456" - content, err := encodeProtectedServerID(serverID, primary, nil) + content, err := encodeProtectedServerIDForTest(serverID, primary, nil) if err != nil { t.Fatalf("encodeProtectedServerID() error = %v", err) } @@ -354,21 +358,8 @@ func TestFallbackServerIDFormat(t *testing.T) { } } -func TestGenerateSystemKeyStableAndLength(t *testing.T) { - const mac = "aa:bb:cc:dd:ee:ff" - k1 := generateSystemKey(mac, nil) - k2 := generateSystemKey(mac, nil) - - if len(k1) != 16 { - t.Fatalf("generateSystemKey length = %d, want 16", len(k1)) - } - if k1 != k2 { - t.Fatalf("generateSystemKey should be stable, got %q and %q", k1, k2) - } -} - func TestCollectMACAddressesSortedAndUnique(t *testing.T) { - macs := collectMACAddresses() + _, macs := collectMACCandidates(nil) for i := 0; i < len(macs); i++ { if macs[i] == "" { t.Fatalf("unexpected empty MAC at index %d", i) @@ -413,7 +404,7 @@ func TestDecodeProtectedServerIDInvalidPayloadFormat(t *testing.T) { func TestDecodeProtectedServerIDInvalidServerIDFormat(t *testing.T) { const mac = "aa:bb:cc:dd:ee:ff" - content, err := encodeProtectedServerID("AAAAAAAAAAAAAAAA", mac, nil) + content, err := encodeProtectedServerIDForTest("AAAAAAAAAAAAAAAA", mac, nil) if err != nil { t.Fatalf("encodeProtectedServerID() error = %v", err) } @@ -446,7 +437,7 @@ func TestLoadServerIDWithEmptyMACSlice(t *testing.T) { path := filepath.Join(dir, "identity.conf") const serverID = "1234567890123456" - content, err := encodeProtectedServerID(serverID, "", nil) + content, err := encodeProtectedServerIDForTest(serverID, "", nil) if err != nil { t.Fatalf("encodeProtectedServerID() error = %v", err) } @@ -487,7 +478,10 @@ func TestLoadServerIDFailsAllMACs(t *testing.T) { } func encodeProtectedServerIDLegacy(serverID, primaryMAC string) (string, error) { - systemKey := generateSystemKey(primaryMAC, nil) + machineID := readMachineID(nil) + hostnamePart := readHostnamePart(nil) + macPart := strings.ReplaceAll(primaryMAC, ":", "") + systemKey := computeSystemKey(machineID, hostnamePart, macPart) timestamp := time.Unix(1700000000, 0).Unix() data := fmt.Sprintf("%s:%d:%s", serverID, timestamp, systemKey[:systemKeyPrefixLength]) checksum := sha256.Sum256([]byte(data)) @@ -1588,7 +1582,7 @@ func TestDecodeProtectedServerIDWithEmptyMAC(t *testing.T) { } const serverID = "1234567890123456" - content, err := encodeProtectedServerID(serverID, "aa:bb:cc:dd:ee:ff", nil) + content, err := encodeProtectedServerIDForTest(serverID, "aa:bb:cc:dd:ee:ff", nil) if err != nil { t.Fatalf("encodeProtectedServerID() error = %v", err) } diff --git a/internal/orchestrator/bundle_test.go b/internal/orchestrator/bundle_test.go index 9462b4b..280060e 100644 --- a/internal/orchestrator/bundle_test.go +++ b/internal/orchestrator/bundle_test.go @@ -121,41 +121,6 @@ func TestCreateBundle_CreatesValidTarArchive(t *testing.T) { } } -func TestLegacyCreateBundleWrapper_DelegatesToMethod(t *testing.T) { - logger := logging.New(logging.GetDefaultLogger().GetLevel(), false) - tempDir := t.TempDir() - archive := filepath.Join(tempDir, "backup.tar") - - // Minimal associated files required by createBundle - required := map[string]string{ - "": "archive-content", - ".sha256": "checksum", - ".metadata": "metadata-json", - } - for suffix, content := range required { - if err := os.WriteFile(archive+suffix, []byte(content), 0o640); err != nil { - t.Fatalf("write %s: %v", suffix, err) - } - } - - ctx := context.Background() - - // Call legacy wrapper - bundlePath, err := createBundle(ctx, logger, archive) - if err != nil { - t.Fatalf("legacy createBundle returned error: %v", err) - } - - expectedPath := archive + ".bundle.tar" - if bundlePath != expectedPath { - t.Fatalf("bundle path = %s, want %s", bundlePath, expectedPath) - } - - if _, err := os.Stat(bundlePath); err != nil { - t.Fatalf("expected bundle file to exist, got %v", err) - } -} - func TestRemoveAssociatedFiles_RemovesAll(t *testing.T) { logger := logging.New(logging.GetDefaultLogger().GetLevel(), false) tempDir := t.TempDir() diff --git a/internal/orchestrator/decrypt_test.go b/internal/orchestrator/decrypt_test.go index ae17bd2..84fcee1 100644 --- a/internal/orchestrator/decrypt_test.go +++ b/internal/orchestrator/decrypt_test.go @@ -4250,22 +4250,6 @@ exit 0 os.Setenv("PATH", tmp+":"+origPath) defer os.Setenv("PATH", origPath) - // Create a filesystem wrapper that allows download but fails MkdirAll for tempRoot - type fakeMkdirAllFailOnTempRoot struct { - osFS - } - fake := &struct { - osFS - mkdirCalls int - }{} - - // Use osFS with a hook to fail on the second MkdirAll (tempRoot creation) - type osFSWithMkdirHook struct { - osFS - mkdirCalls int - } - hookFS := &osFSWithMkdirHook{} - orig := restoreFS // Use regular osFS - the download will work, then MkdirAll for /tmp/proxsave should succeed // but we can trigger error by making /tmp/proxsave unwritable after download @@ -4289,27 +4273,6 @@ exit 0 } // If download succeeds and extraction succeeds, that's fine - we've tested the path _ = err - _ = fake - _ = hookFS -} - -// fakeChecksumFailFS wraps osFS to make the plain archive unreadable after extraction -// This triggers GenerateChecksum error (lines 670-673) -type fakeChecksumFailFS struct { - osFS - extractDone bool -} - -func (f *fakeChecksumFailFS) OpenFile(path string, flag int, perm os.FileMode) (*os.File, error) { - file, err := os.OpenFile(path, flag, perm) - if err != nil { - return nil, err - } - // After extracting, make the archive unreadable for checksum - if f.extractDone && strings.Contains(path, "proxmox-decrypt") && strings.HasSuffix(path, ".tar.xz") { - os.Chmod(path, 0o000) - } - return file, nil } // fakeStatThenRemoveFS removes the file after stat succeeds diff --git a/internal/orchestrator/decrypt_workflow_ui.go b/internal/orchestrator/decrypt_workflow_ui.go index a57d45a..2ae37d7 100644 --- a/internal/orchestrator/decrypt_workflow_ui.go +++ b/internal/orchestrator/decrypt_workflow_ui.go @@ -361,7 +361,8 @@ func runDecryptWorkflowWithUI(ctx context.Context, cfg *config.Config, logger *l } logger.Info("Creating decrypted bundle...") - bundlePath, err := createBundle(ctx, logger, tempArchivePath) + o := &Orchestrator{logger: logger, fs: osFS{}} + bundlePath, err := o.createBundle(ctx, tempArchivePath) if err != nil { return err } diff --git a/internal/orchestrator/orchestrator.go b/internal/orchestrator/orchestrator.go index c629435..4eb71f7 100644 --- a/internal/orchestrator/orchestrator.go +++ b/internal/orchestrator/orchestrator.go @@ -1192,12 +1192,6 @@ func (o *Orchestrator) removeAssociatedFiles(archivePath string) error { return nil } -// Legacy compatibility wrapper for callers that used the package-level createBundle function. -func createBundle(ctx context.Context, logger *logging.Logger, archivePath string) (string, error) { - o := &Orchestrator{logger: logger, fs: osFS{}, clock: realTimeProvider{}} - return o.createBundle(ctx, archivePath) -} - // encryptArchive was replaced by streaming encryption inside the archiver. // SaveStatsReport writes a JSON report with backup statistics to the log directory. From 7caa1823e3f466cd5af7430f7657d0fbdd832c90 Mon Sep 17 00:00:00 2001 From: Damiano <71268257+tis24dev@users.noreply.github.com> Date: Mon, 23 Feb 2026 01:34:46 +0100 Subject: [PATCH 3/4] Make mount guard functions mockable, add tests Extract direct OS/syscall/fstab calls in mount guard into package-level function variables (e.g. mountGuardGeteuid, mountGuardReadFile, mountGuardMkdirAll, mountGuardSysMount, mountGuardSysUnmount, mountGuardFstabMountpointsSet, mountGuardIsPathOnRootFilesystem, mountGuardParsePBSDatastoreCfg) and update usages to call those variables. This makes the mount-guard logic easily mockable for unit tests. Add extensive tests in internal/orchestrator/mount_guard_more_test.go covering guardDirForTarget, isMounted (mountinfo/proc mounts fallback and error combinations), guardMountPoint behaviors (mkdir, bind, remount, unmount, context handling), and many flows for maybeApplyPBSDatastoreMountGuards including parsing, fstab fallback, mount attempts and timeout handling. Also adjust an existing test case in pbs_mount_guard_test.go to include a /run/media root scenario and remove a redundant check in pbsMountGuardRootForDatastorePath. These changes improve test coverage and reliability without changing runtime behavior. --- internal/orchestrator/mount_guard.go | 43 +- .../orchestrator/mount_guard_more_test.go | 896 ++++++++++++++++++ internal/orchestrator/pbs_mount_guard_test.go | 1 + 3 files changed, 923 insertions(+), 17 deletions(-) create mode 100644 internal/orchestrator/mount_guard_more_test.go diff --git a/internal/orchestrator/mount_guard.go b/internal/orchestrator/mount_guard.go index 037811d..f4ee262 100644 --- a/internal/orchestrator/mount_guard.go +++ b/internal/orchestrator/mount_guard.go @@ -18,6 +18,18 @@ import ( const mountGuardBaseDir = "/var/lib/proxsave/guards" const mountGuardMountAttemptTimeout = 10 * time.Second +var ( + mountGuardGeteuid = os.Geteuid + mountGuardReadFile = os.ReadFile + mountGuardMkdirAll = os.MkdirAll + mountGuardReadDir = os.ReadDir + mountGuardSysMount = syscall.Mount + mountGuardSysUnmount = syscall.Unmount + mountGuardFstabMountpointsSet = fstabMountpointsSet + mountGuardIsPathOnRootFilesystem = isPathOnRootFilesystem + mountGuardParsePBSDatastoreCfg = parsePBSDatastoreCfgBlocks +) + func maybeApplyPBSDatastoreMountGuards(ctx context.Context, logger *logging.Logger, plan *RestorePlan, stageRoot, destRoot string, dryRun bool) error { if plan == nil || plan.SystemType != SystemTypePBS || !plan.HasCategoryID("datastore_pbs") { return nil @@ -44,7 +56,7 @@ func maybeApplyPBSDatastoreMountGuards(ctx context.Context, logger *logging.Logg } return nil } - if os.Geteuid() != 0 { + if mountGuardGeteuid() != 0 { if logger != nil { logger.Warning("Skipping PBS mount guards: requires root privileges") } @@ -64,7 +76,7 @@ func maybeApplyPBSDatastoreMountGuards(ctx context.Context, logger *logging.Logg } normalized, _ := normalizePBSDatastoreCfgContent(string(data)) - blocks, err := parsePBSDatastoreCfgBlocks(normalized) + blocks, err := mountGuardParsePBSDatastoreCfg(normalized) if err != nil { return err } @@ -75,7 +87,7 @@ func maybeApplyPBSDatastoreMountGuards(ctx context.Context, logger *logging.Logg var fstabMounts map[string]struct{} var mountpointCandidates []string currentFstab := filepath.Join(destRoot, "etc", "fstab") - if mounts, err := fstabMountpointsSet(currentFstab); err != nil { + if mounts, err := mountGuardFstabMountpointsSet(currentFstab); err != nil { if logger != nil { logger.Warning("PBS mount guard: unable to parse current fstab %s: %v (continuing without fstab cross-check)", currentFstab, err) } @@ -123,14 +135,14 @@ func maybeApplyPBSDatastoreMountGuards(ctx context.Context, logger *logging.Logg } } - if err := os.MkdirAll(guardTarget, 0o755); err != nil { + if err := mountGuardMkdirAll(guardTarget, 0o755); err != nil { if logger != nil { logger.Warning("PBS mount guard: unable to create mountpoint directory %s: %v", guardTarget, err) } continue } - onRootFS, _, devErr := isPathOnRootFilesystem(guardTarget) + onRootFS, _, devErr := mountGuardIsPathOnRootFilesystem(guardTarget) if devErr != nil { if logger != nil { logger.Warning("PBS mount guard: unable to determine filesystem device for %s: %v", guardTarget, devErr) @@ -158,7 +170,7 @@ func maybeApplyPBSDatastoreMountGuards(ctx context.Context, logger *logging.Logg out, attemptErr := restoreCmd.Run(mountCtx, "mount", guardTarget) cancel() if attemptErr == nil { - onRootFSNow, _, devErrNow := isPathOnRootFilesystem(guardTarget) + onRootFSNow, _, devErrNow := mountGuardIsPathOnRootFilesystem(guardTarget) if devErrNow == nil && !onRootFSNow { if logger != nil { logger.Info("PBS mount guard: mountpoint %s is now mounted (mount attempt succeeded)", guardTarget) @@ -209,7 +221,7 @@ func maybeApplyPBSDatastoreMountGuards(ctx context.Context, logger *logging.Logg protected[guardTarget] = struct{}{} if logger != nil { - if entries, err := os.ReadDir(guardTarget); err == nil && len(entries) > 0 { + if entries, err := mountGuardReadDir(guardTarget); err == nil && len(entries) > 0 { logger.Warning("PBS mount guard: guard mount point %s is not empty (entries=%d)", guardTarget, len(entries)) } logger.Warning("PBS mount guard: %s resolves to root filesystem (mount missing?) — bind-mounted a read-only guard to prevent writes until storage is available", guardTarget) @@ -241,22 +253,22 @@ func guardMountPoint(ctx context.Context, guardTarget string) error { } guardDir := guardDirForTarget(target) - if err := os.MkdirAll(guardDir, 0o755); err != nil { + if err := mountGuardMkdirAll(guardDir, 0o755); err != nil { return fmt.Errorf("mkdir guard dir: %w", err) } - if err := os.MkdirAll(target, 0o755); err != nil { + if err := mountGuardMkdirAll(target, 0o755); err != nil { return fmt.Errorf("mkdir target: %w", err) } // Bind mount guard directory over the mountpoint to avoid writes to the underlying rootfs path. - if err := syscall.Mount(guardDir, target, "", syscall.MS_BIND, ""); err != nil { + if err := mountGuardSysMount(guardDir, target, "", syscall.MS_BIND, ""); err != nil { return fmt.Errorf("bind mount guard: %w", err) } // Make the bind mount read-only to ensure PBS cannot write backup data to the guard directory. remountFlags := uintptr(syscall.MS_BIND | syscall.MS_REMOUNT | syscall.MS_RDONLY | syscall.MS_NODEV | syscall.MS_NOSUID | syscall.MS_NOEXEC) - if err := syscall.Mount("", target, "", remountFlags, ""); err != nil { - _ = syscall.Unmount(target, 0) + if err := mountGuardSysMount("", target, "", remountFlags, ""); err != nil { + _ = mountGuardSysUnmount(target, 0) return fmt.Errorf("remount guard read-only: %w", err) } @@ -274,7 +286,7 @@ func guardDirForTarget(target string) string { } func isMounted(path string) (bool, error) { - data, err := os.ReadFile("/proc/self/mountinfo") + data, err := mountGuardReadFile("/proc/self/mountinfo") if err == nil { return isMountedFromMountinfo(string(data), path), nil } @@ -315,7 +327,7 @@ func isMountedFromMountinfo(mountinfo, path string) bool { } func isMountedFromProcMounts(path string) (bool, error) { - data, err := os.ReadFile("/proc/mounts") + data, err := mountGuardReadFile("/proc/mounts") if err != nil { return false, err } @@ -408,9 +420,6 @@ func pbsMountGuardRootForDatastorePath(path string) string { case strings.HasPrefix(p, "/run/media/"): rest := strings.TrimPrefix(p, "/run/media/") parts := splitPath(rest) - if len(parts) == 0 { - return "" - } if len(parts) == 1 { return filepath.Join("/run/media", parts[0]) } diff --git a/internal/orchestrator/mount_guard_more_test.go b/internal/orchestrator/mount_guard_more_test.go new file mode 100644 index 0000000..4110908 --- /dev/null +++ b/internal/orchestrator/mount_guard_more_test.go @@ -0,0 +1,896 @@ +package orchestrator + +import ( + "context" + "crypto/sha256" + "errors" + "fmt" + "os" + "path/filepath" + "strings" + "syscall" + "testing" + "time" +) + +func TestGuardDirForTarget(t *testing.T) { + t.Parallel() + + target := "/mnt/datastore" + sum := sha256.Sum256([]byte(target)) + id := fmt.Sprintf("%x", sum[:8]) + want := filepath.Join(mountGuardBaseDir, fmt.Sprintf("%s-%s", filepath.Base(target), id)) + if got := guardDirForTarget(target); got != want { + t.Fatalf("guardDirForTarget(%q)=%q want %q", target, got, want) + } + + rootTarget := "/" + sum = sha256.Sum256([]byte(rootTarget)) + id = fmt.Sprintf("%x", sum[:8]) + want = filepath.Join(mountGuardBaseDir, fmt.Sprintf("%s-%s", "guard", id)) + if got := guardDirForTarget(rootTarget); got != want { + t.Fatalf("guardDirForTarget(%q)=%q want %q", rootTarget, got, want) + } +} + +func TestIsMountedFromMountinfo(t *testing.T) { + t.Parallel() + + mountinfo := strings.Join([]string{ + "36 25 0:32 / / rw,relatime - ext4 /dev/sda1 rw", + `37 36 0:33 / /mnt/pbs\040datastore rw,relatime - ext4 /dev/sdb1 rw`, + "bad line", + "", + }, "\n") + + if got := isMountedFromMountinfo(mountinfo, "/"); !got { + t.Fatalf("expected / to be mounted") + } + if got := isMountedFromMountinfo(mountinfo, "/mnt/pbs datastore"); !got { + t.Fatalf("expected escaped mountpoint to match") + } + if got := isMountedFromMountinfo(mountinfo, "/not-mounted"); got { + t.Fatalf("expected /not-mounted to be unmounted") + } + if got := isMountedFromMountinfo(mountinfo, ""); got { + t.Fatalf("expected empty path to be unmounted") + } +} + +func TestFstabMountpointsSet(t *testing.T) { + tmp := filepath.Join(t.TempDir(), "fstab") + content := strings.Join([]string{ + "# comment", + "UUID=abc / ext4 defaults 0 1", + "/dev/sdb1 /mnt/data/ ext4 defaults 0 2", + "/dev/sdc1 /mnt/data2 ext4 defaults 0 2 # inline comment", + "/dev/sdd1 . ext4 defaults 0 0", + "invalidline", + "", + }, "\n") + if err := os.WriteFile(tmp, []byte(content), 0o600); err != nil { + t.Fatalf("write temp fstab: %v", err) + } + + mps, err := fstabMountpointsSet(tmp) + if err != nil { + t.Fatalf("fstabMountpointsSet error: %v", err) + } + + for _, mp := range []string{"/", "/mnt/data", "/mnt/data2"} { + if _, ok := mps[mp]; !ok { + t.Fatalf("expected mountpoint %s to be present", mp) + } + } + if _, ok := mps["."]; ok { + t.Fatalf("expected dot mountpoint to be skipped") + } +} + +func TestFstabMountpointsSet_Error(t *testing.T) { + origFS := restoreFS + t.Cleanup(func() { restoreFS = origFS }) + restoreFS = NewFakeFS() + + if _, err := fstabMountpointsSet("/does-not-exist"); err == nil { + t.Fatalf("expected error") + } +} + +func TestSplitPathAndMountRootWithPrefix(t *testing.T) { + t.Parallel() + + if got := splitPath("a//b/ /c/"); strings.Join(got, ",") != "a,b,c" { + t.Fatalf("splitPath unexpected: %#v", got) + } + if got := mountRootWithPrefix("/mnt/datastore/Data1", "/mnt/"); got != "/mnt/datastore" { + t.Fatalf("mountRootWithPrefix got %q want %q", got, "/mnt/datastore") + } + if got := mountRootWithPrefix("/mnt/", "/mnt/"); got != "" { + t.Fatalf("mountRootWithPrefix(/mnt/)=%q want empty", got) + } +} + +func TestSortByLengthDesc(t *testing.T) { + t.Parallel() + + items := []string{"a", "abc", "ab"} + sortByLengthDesc(items) + if len(items) != 3 { + t.Fatalf("unexpected len: %d", len(items)) + } + if !(len(items[0]) >= len(items[1]) && len(items[1]) >= len(items[2])) { + t.Fatalf("expected non-increasing lengths, got %#v", items) + } +} + +func TestFirstFstabMountpointMatch(t *testing.T) { + t.Parallel() + + mountpoints := []string{"/mnt/storage/pbs", "/mnt/storage", "/"} + if got := firstFstabMountpointMatch("/mnt/storage/pbs/ds1/data", mountpoints); got != "/mnt/storage/pbs" { + t.Fatalf("firstFstabMountpointMatch got %q want %q", got, "/mnt/storage/pbs") + } + if got := firstFstabMountpointMatch(" ", mountpoints); got != "" { + t.Fatalf("firstFstabMountpointMatch empty got %q want empty", got) + } +} + +func TestIsMounted_Variants(t *testing.T) { + origReadFile := mountGuardReadFile + t.Cleanup(func() { mountGuardReadFile = origReadFile }) + + t.Run("prefers mountinfo", func(t *testing.T) { + mountGuardReadFile = func(path string) ([]byte, error) { + if path != "/proc/self/mountinfo" { + t.Fatalf("unexpected read path: %s", path) + } + return []byte("1 2 3:4 / /mnt/target rw - ext4 /dev/sda1 rw\n"), nil + } + mounted, err := isMounted("/mnt/target") + if err != nil { + t.Fatalf("isMounted error: %v", err) + } + if !mounted { + t.Fatalf("expected mounted") + } + }) + + t.Run("falls back to proc mounts", func(t *testing.T) { + mountGuardReadFile = func(path string) ([]byte, error) { + switch path { + case "/proc/self/mountinfo": + return nil, os.ErrNotExist + case "/proc/mounts": + return []byte("/dev/sda1 /mnt/target ext4 rw 0 0\n"), nil + default: + t.Fatalf("unexpected read path: %s", path) + return nil, nil + } + } + mounted, err := isMounted("/mnt/target") + if err != nil { + t.Fatalf("isMounted error: %v", err) + } + if !mounted { + t.Fatalf("expected mounted") + } + }) + + t.Run("reports mounts error when mountinfo missing", func(t *testing.T) { + wantErr := errors.New("mounts read failed") + mountGuardReadFile = func(path string) ([]byte, error) { + switch path { + case "/proc/self/mountinfo": + return nil, os.ErrNotExist + case "/proc/mounts": + return nil, wantErr + default: + t.Fatalf("unexpected read path: %s", path) + return nil, nil + } + } + _, err := isMounted("/mnt/target") + if !errors.Is(err, wantErr) { + t.Fatalf("expected mounts error, got %v", err) + } + }) + + t.Run("includes both errors when mountinfo read fails", func(t *testing.T) { + mountErr := errors.New("mountinfo boom") + mountsErr := errors.New("mounts boom") + mountGuardReadFile = func(path string) ([]byte, error) { + switch path { + case "/proc/self/mountinfo": + return nil, mountErr + case "/proc/mounts": + return nil, mountsErr + default: + t.Fatalf("unexpected read path: %s", path) + return nil, nil + } + } + _, err := isMounted("/mnt/target") + if err == nil || !strings.Contains(err.Error(), "mountinfo boom") || !strings.Contains(err.Error(), "mounts boom") { + t.Fatalf("expected combined error, got %v", err) + } + }) +} + +func TestIsMountedFromProcMounts_Parsing(t *testing.T) { + origReadFile := mountGuardReadFile + t.Cleanup(func() { mountGuardReadFile = origReadFile }) + + mountGuardReadFile = func(path string) ([]byte, error) { + if path != "/proc/mounts" { + t.Fatalf("unexpected read path: %s", path) + } + return []byte(strings.Join([]string{ + "", + "invalid", + "/dev/sda1 /mnt/other ext4 rw 0 0", + "", + }, "\n")), nil + } + + mounted, err := isMountedFromProcMounts("/mnt/target") + if err != nil { + t.Fatalf("isMountedFromProcMounts error: %v", err) + } + if mounted { + t.Fatalf("expected unmounted") + } + + mounted, err = isMountedFromProcMounts(" ") + if err != nil { + t.Fatalf("isMountedFromProcMounts empty target error: %v", err) + } + if mounted { + t.Fatalf("expected empty target to be unmounted") + } +} + +func TestGuardMountPoint(t *testing.T) { + origReadFile := mountGuardReadFile + origMkdirAll := mountGuardMkdirAll + origMount := mountGuardSysMount + origUnmount := mountGuardSysUnmount + t.Cleanup(func() { + mountGuardReadFile = origReadFile + mountGuardMkdirAll = origMkdirAll + mountGuardSysMount = origMount + mountGuardSysUnmount = origUnmount + }) + + t.Run("rejects invalid target", func(t *testing.T) { + if err := guardMountPoint(context.Background(), "/"); err == nil { + t.Fatalf("expected error") + } + }) + + t.Run("nil context uses background", func(t *testing.T) { + mountGuardReadFile = func(string) ([]byte, error) { + return []byte("1 2 3:4 / / rw - ext4 /dev/sda1 rw\n"), nil + } + mountGuardMkdirAll = func(string, os.FileMode) error { return nil } + mountGuardSysMount = func(string, string, string, uintptr, string) error { return nil } + mountGuardSysUnmount = func(string, int) error { + t.Fatalf("unexpected unmount call") + return nil + } + + if err := guardMountPoint(nil, "/mnt/nilctx"); err != nil { + t.Fatalf("unexpected error: %v", err) + } + }) + + t.Run("mount status check error", func(t *testing.T) { + mountGuardReadFile = func(string) ([]byte, error) { return nil, errors.New("read failed") } + if err := guardMountPoint(context.Background(), "/mnt/statuserr"); err == nil || !strings.Contains(err.Error(), "check mount status") { + t.Fatalf("expected status check error, got %v", err) + } + }) + + t.Run("mkdir guard dir failure", func(t *testing.T) { + target := "/mnt/mkdir-guard-dir-fail" + guardDir := guardDirForTarget(target) + + mountGuardReadFile = func(string) ([]byte, error) { + return []byte("1 2 3:4 / / rw - ext4 /dev/sda1 rw\n"), nil + } + mountGuardMkdirAll = func(path string, _ os.FileMode) error { + if filepath.Clean(path) == filepath.Clean(guardDir) { + return errors.New("mkdir guard dir failed") + } + return nil + } + mountGuardSysMount = func(string, string, string, uintptr, string) error { + t.Fatalf("unexpected mount call") + return nil + } + + if err := guardMountPoint(context.Background(), target); err == nil || !strings.Contains(err.Error(), "mkdir guard dir") { + t.Fatalf("expected mkdir guard dir error, got %v", err) + } + }) + + t.Run("mkdir target failure", func(t *testing.T) { + target := "/mnt/mkdir-target-fail" + guardDir := guardDirForTarget(target) + + mountGuardReadFile = func(string) ([]byte, error) { + return []byte("1 2 3:4 / / rw - ext4 /dev/sda1 rw\n"), nil + } + mountGuardMkdirAll = func(path string, _ os.FileMode) error { + switch filepath.Clean(path) { + case filepath.Clean(guardDir): + return nil + case filepath.Clean(target): + return errors.New("mkdir target failed") + default: + return nil + } + } + mountGuardSysMount = func(string, string, string, uintptr, string) error { + t.Fatalf("unexpected mount call") + return nil + } + + if err := guardMountPoint(context.Background(), target); err == nil || !strings.Contains(err.Error(), "mkdir target") { + t.Fatalf("expected mkdir target error, got %v", err) + } + }) + + t.Run("returns nil when already mounted", func(t *testing.T) { + mountGuardReadFile = func(path string) ([]byte, error) { + if path != "/proc/self/mountinfo" { + return nil, os.ErrNotExist + } + return []byte("1 2 3:4 / /mnt/already rw - ext4 /dev/sda1 rw\n"), nil + } + mountGuardMkdirAll = func(string, os.FileMode) error { + t.Fatalf("unexpected mkdir call") + return nil + } + mountGuardSysMount = func(string, string, string, uintptr, string) error { + t.Fatalf("unexpected mount call") + return nil + } + + if err := guardMountPoint(context.Background(), "/mnt/already"); err != nil { + t.Fatalf("unexpected error: %v", err) + } + }) + + t.Run("bind mount failure", func(t *testing.T) { + mountGuardReadFile = func(path string) ([]byte, error) { + return []byte("1 2 3:4 / / rw - ext4 /dev/sda1 rw\n"), nil + } + mountGuardMkdirAll = func(string, os.FileMode) error { return nil } + mountGuardSysMount = func(_, _, _ string, flags uintptr, _ string) error { + if flags == syscall.MS_BIND { + return syscall.EPERM + } + return nil + } + mountGuardSysUnmount = func(string, int) error { + t.Fatalf("unexpected unmount call") + return nil + } + + if err := guardMountPoint(context.Background(), "/mnt/failbind"); err == nil || !strings.Contains(err.Error(), "bind mount guard") { + t.Fatalf("expected bind mount error, got %v", err) + } + }) + + t.Run("remount failure unmounts", func(t *testing.T) { + mountGuardReadFile = func(path string) ([]byte, error) { + return []byte("1 2 3:4 / / rw - ext4 /dev/sda1 rw\n"), nil + } + mountGuardMkdirAll = func(string, os.FileMode) error { return nil } + + mountCalls := 0 + mountGuardSysMount = func(_, _, _ string, _ uintptr, _ string) error { + mountCalls++ + if mountCalls == 2 { + return syscall.EPERM + } + return nil + } + + unmountCalls := 0 + mountGuardSysUnmount = func(target string, flags int) error { + unmountCalls++ + if target != "/mnt/failremount" || flags != 0 { + t.Fatalf("unexpected unmount args: target=%s flags=%d", target, flags) + } + return nil + } + + if err := guardMountPoint(context.Background(), "/mnt/failremount"); err == nil || !strings.Contains(err.Error(), "remount guard read-only") { + t.Fatalf("expected remount error, got %v", err) + } + if unmountCalls != 1 { + t.Fatalf("expected 1 unmount call, got %d", unmountCalls) + } + }) + + t.Run("context canceled", func(t *testing.T) { + ctx, cancel := context.WithCancel(context.Background()) + cancel() + if err := guardMountPoint(ctx, "/mnt/ctx"); !errors.Is(err, context.Canceled) { + t.Fatalf("expected context canceled, got %v", err) + } + }) +} + +type mountGuardCommandCall struct { + name string + args []string +} + +type mountGuardCommandRunner struct { + run func(ctx context.Context, name string, args ...string) ([]byte, error) + calls []mountGuardCommandCall +} + +func (f *mountGuardCommandRunner) Run(ctx context.Context, name string, args ...string) ([]byte, error) { + f.calls = append(f.calls, mountGuardCommandCall{name: name, args: append([]string{}, args...)}) + if f.run != nil { + return f.run(ctx, name, args...) + } + return nil, nil +} + +func TestMaybeApplyPBSDatastoreMountGuards_EarlyReturns(t *testing.T) { + logger := newTestLogger() + ctx := context.Background() + plan := &RestorePlan{SystemType: SystemTypePBS, NormalCategories: []Category{{ID: "datastore_pbs"}}} + + if err := maybeApplyPBSDatastoreMountGuards(ctx, logger, nil, "/stage", "/", false); err != nil { + t.Fatalf("nil plan error: %v", err) + } + if err := maybeApplyPBSDatastoreMountGuards(ctx, logger, &RestorePlan{SystemType: SystemTypePVE}, "/stage", "/", false); err != nil { + t.Fatalf("wrong system type error: %v", err) + } + if err := maybeApplyPBSDatastoreMountGuards(ctx, logger, &RestorePlan{SystemType: SystemTypePBS}, "/stage", "/", false); err != nil { + t.Fatalf("missing category error: %v", err) + } + if err := maybeApplyPBSDatastoreMountGuards(ctx, logger, plan, "", "/", false); err != nil { + t.Fatalf("empty stageRoot error: %v", err) + } + if err := maybeApplyPBSDatastoreMountGuards(ctx, logger, plan, "/stage", "/not-root", false); err != nil { + t.Fatalf("destRoot not root error: %v", err) + } + if err := maybeApplyPBSDatastoreMountGuards(ctx, logger, plan, "/stage", "/", true); err != nil { + t.Fatalf("dryRun error: %v", err) + } + + origFS := restoreFS + t.Cleanup(func() { restoreFS = origFS }) + restoreFS = NewFakeFS() + if err := maybeApplyPBSDatastoreMountGuards(ctx, logger, plan, "/stage", "/", false); err != nil { + t.Fatalf("non-real restoreFS error: %v", err) + } + + origGeteuid := mountGuardGeteuid + t.Cleanup(func() { mountGuardGeteuid = origGeteuid }) + mountGuardGeteuid = func() int { return 1 } + restoreFS = origFS + if err := maybeApplyPBSDatastoreMountGuards(ctx, logger, plan, "/stage", "/", false); err != nil { + t.Fatalf("non-root user error: %v", err) + } +} + +func TestMaybeApplyPBSDatastoreMountGuards_StagedDatastoreCfgHandling(t *testing.T) { + logger := newTestLogger() + ctx := context.Background() + plan := &RestorePlan{SystemType: SystemTypePBS, NormalCategories: []Category{{ID: "datastore_pbs"}}} + + origGeteuid := mountGuardGeteuid + t.Cleanup(func() { mountGuardGeteuid = origGeteuid }) + mountGuardGeteuid = func() int { return 0 } + + stageRoot := t.TempDir() + + // Missing file => no-op. + if err := maybeApplyPBSDatastoreMountGuards(ctx, logger, plan, stageRoot, "/", false); err != nil { + t.Fatalf("missing staged file error: %v", err) + } + + // Non-file error should propagate. + stagePath := filepath.Join(stageRoot, "etc/proxmox-backup/datastore.cfg") + if err := os.MkdirAll(stagePath, 0o755); err != nil { + t.Fatalf("mkdir staged path: %v", err) + } + if err := maybeApplyPBSDatastoreMountGuards(ctx, logger, plan, stageRoot, "/", false); err == nil || !strings.Contains(err.Error(), "read staged datastore.cfg") { + t.Fatalf("expected read staged error, got %v", err) + } + + // Empty file => no-op. + if err := os.RemoveAll(filepath.Dir(stagePath)); err != nil { + t.Fatalf("cleanup staged dir: %v", err) + } + if err := os.MkdirAll(filepath.Dir(stagePath), 0o755); err != nil { + t.Fatalf("mkdir staged dir: %v", err) + } + if err := os.WriteFile(stagePath, []byte(" \n\t"), 0o600); err != nil { + t.Fatalf("write staged file: %v", err) + } + if err := maybeApplyPBSDatastoreMountGuards(ctx, logger, plan, stageRoot, "/", false); err != nil { + t.Fatalf("empty staged content error: %v", err) + } +} + +func TestMaybeApplyPBSDatastoreMountGuards_NoBlocks(t *testing.T) { + logger := newTestLogger() + ctx := context.Background() + plan := &RestorePlan{SystemType: SystemTypePBS, NormalCategories: []Category{{ID: "datastore_pbs"}}} + + origGeteuid := mountGuardGeteuid + t.Cleanup(func() { mountGuardGeteuid = origGeteuid }) + mountGuardGeteuid = func() int { return 0 } + + stageRoot := t.TempDir() + stagePath := filepath.Join(stageRoot, "etc/proxmox-backup/datastore.cfg") + if err := os.MkdirAll(filepath.Dir(stagePath), 0o755); err != nil { + t.Fatalf("mkdir staged dir: %v", err) + } + if err := os.WriteFile(stagePath, []byte("# comment only\n"), 0o600); err != nil { + t.Fatalf("write datastore.cfg: %v", err) + } + + if err := maybeApplyPBSDatastoreMountGuards(ctx, logger, plan, stageRoot, "/", false); err != nil { + t.Fatalf("maybeApplyPBSDatastoreMountGuards error: %v", err) + } +} + +func TestMaybeApplyPBSDatastoreMountGuards_FstabParseErrorContinues(t *testing.T) { + logger := newTestLogger() + ctx := context.Background() + plan := &RestorePlan{SystemType: SystemTypePBS, NormalCategories: []Category{{ID: "datastore_pbs"}}} + + origGeteuid := mountGuardGeteuid + origFstab := mountGuardFstabMountpointsSet + origMkdirAll := mountGuardMkdirAll + origRootFS := mountGuardIsPathOnRootFilesystem + t.Cleanup(func() { + mountGuardGeteuid = origGeteuid + mountGuardFstabMountpointsSet = origFstab + mountGuardMkdirAll = origMkdirAll + mountGuardIsPathOnRootFilesystem = origRootFS + }) + + mountGuardGeteuid = func() int { return 0 } + mountGuardFstabMountpointsSet = func(string) (map[string]struct{}, error) { + return nil, errors.New("fstab parse failed") + } + mountGuardMkdirAll = func(string, os.FileMode) error { return nil } + mountGuardIsPathOnRootFilesystem = func(path string) (bool, string, error) { + return false, filepath.Clean(path), nil + } + + stageRoot := t.TempDir() + stagePath := filepath.Join(stageRoot, "etc/proxmox-backup/datastore.cfg") + if err := os.MkdirAll(filepath.Dir(stagePath), 0o755); err != nil { + t.Fatalf("mkdir staged dir: %v", err) + } + if err := os.WriteFile(stagePath, []byte("datastore: ds\n path /mnt/test/store\n"), 0o600); err != nil { + t.Fatalf("write datastore.cfg: %v", err) + } + + if err := maybeApplyPBSDatastoreMountGuards(ctx, logger, plan, stageRoot, "/", false); err != nil { + t.Fatalf("maybeApplyPBSDatastoreMountGuards error: %v", err) + } +} + +func TestMaybeApplyPBSDatastoreMountGuards_ParseBlocksError(t *testing.T) { + logger := newTestLogger() + ctx := context.Background() + plan := &RestorePlan{SystemType: SystemTypePBS, NormalCategories: []Category{{ID: "datastore_pbs"}}} + + origGeteuid := mountGuardGeteuid + origParse := mountGuardParsePBSDatastoreCfg + t.Cleanup(func() { + mountGuardGeteuid = origGeteuid + mountGuardParsePBSDatastoreCfg = origParse + }) + + mountGuardGeteuid = func() int { return 0 } + wantErr := errors.New("parse blocks failed") + mountGuardParsePBSDatastoreCfg = func(string) ([]pbsDatastoreBlock, error) { + return nil, wantErr + } + + stageRoot := t.TempDir() + stagePath := filepath.Join(stageRoot, "etc/proxmox-backup/datastore.cfg") + if err := os.MkdirAll(filepath.Dir(stagePath), 0o755); err != nil { + t.Fatalf("mkdir staged dir: %v", err) + } + if err := os.WriteFile(stagePath, []byte("datastore: ds\n path /mnt/test/store\n"), 0o600); err != nil { + t.Fatalf("write datastore.cfg: %v", err) + } + + if err := maybeApplyPBSDatastoreMountGuards(ctx, logger, plan, stageRoot, "/", false); !errors.Is(err, wantErr) { + t.Fatalf("expected parse error %v, got %v", wantErr, err) + } +} + +func TestMaybeApplyPBSDatastoreMountGuards_FullFlow(t *testing.T) { + logger := newTestLogger() + ctx := context.Background() + plan := &RestorePlan{SystemType: SystemTypePBS, NormalCategories: []Category{{ID: "datastore_pbs"}}} + + origGeteuid := mountGuardGeteuid + origReadFile := mountGuardReadFile + origMkdirAll := mountGuardMkdirAll + origReadDir := mountGuardReadDir + origMount := mountGuardSysMount + origUnmount := mountGuardSysUnmount + origFstab := mountGuardFstabMountpointsSet + origRootFS := mountGuardIsPathOnRootFilesystem + origCmd := restoreCmd + t.Cleanup(func() { + mountGuardGeteuid = origGeteuid + mountGuardReadFile = origReadFile + mountGuardMkdirAll = origMkdirAll + mountGuardReadDir = origReadDir + mountGuardSysMount = origMount + mountGuardSysUnmount = origUnmount + mountGuardFstabMountpointsSet = origFstab + mountGuardIsPathOnRootFilesystem = origRootFS + restoreCmd = origCmd + }) + + mountGuardGeteuid = func() int { return 0 } + + stageRoot := t.TempDir() + stagePath := filepath.Join(stageRoot, "etc/proxmox-backup/datastore.cfg") + if err := os.MkdirAll(filepath.Dir(stagePath), 0o755); err != nil { + t.Fatalf("mkdir staged dir: %v", err) + } + cfg := strings.Join([]string{ + "datastore: ds-chattrsuccess", + " path /mnt/chattrsuccess/store", + "datastore: ds-invalid", + " path /", + "datastore: ds-nomountstyle", + " path /srv/pbs", + "datastore: ds-storage", + " path /mnt/storage/pbs/ds1/data", + "datastore: ds-media-skip-fstab", + " path /media/USB/PBS", + "datastore: ds-mkdirerr", + " path /mnt/mkdirerr/store", + "datastore: ds-deverr", + " path /mnt/deverr/store", + "datastore: ds-notroot", + " path /mnt/notroot/store", + "datastore: ds-mounted", + " path /mnt/mounted/store", + "datastore: ds-mountok", + " path /mnt/mountok/store", + "datastore: ds-mountok2", + " path /mnt/mountok2/store", + "datastore: ds-chattrfail", + " path /mnt/chattrfail/store", + "datastore: ds-guardok", + " path /mnt/guardok/store", + "datastore: ds-guarddup", + " path /mnt/guardok/other", + "", + }, "\n") + if err := os.WriteFile(stagePath, []byte(cfg), 0o600); err != nil { + t.Fatalf("write datastore.cfg: %v", err) + } + + mountGuardFstabMountpointsSet = func(string) (map[string]struct{}, error) { + return map[string]struct{}{ + "/": {}, + "/srv": {}, + "/mnt/storage": {}, + "/mnt/storage/pbs": {}, + "/mnt/mkdirerr": {}, + "/mnt/deverr": {}, + "/mnt/notroot": {}, + "/mnt/mounted": {}, + "/mnt/mountok": {}, + "/mnt/mountok2": {}, + "/mnt/chattrfail": {}, + "/mnt/chattrsuccess": {}, + "/mnt/guardok": {}, + }, nil + } + + mountGuardMkdirAll = func(path string, _ os.FileMode) error { + if filepath.Clean(path) == "/mnt/mkdirerr" { + return errors.New("mkdir denied") + } + return nil + } + + rootCalls := make(map[string]int) + mountGuardIsPathOnRootFilesystem = func(path string) (bool, string, error) { + path = filepath.Clean(path) + rootCalls[path]++ + switch path { + case "/mnt/deverr": + return false, path, errors.New("stat failed") + case "/mnt/notroot": + return false, path, nil + case "/mnt/mountok": + if rootCalls[path] == 1 { + return true, path, nil + } + return false, path, nil + default: + return true, path, nil + } + } + + mountedTargets := map[string]struct{}{ + "/mnt/mounted": {}, + } + mountinfoReads := 0 + mountsReads := 0 + buildMountinfo := func() string { + var b strings.Builder + for mp := range mountedTargets { + b.WriteString(fmt.Sprintf("1 2 3:4 / %s rw - ext4 /dev/sda1 rw\n", mp)) + } + return b.String() + } + buildProcMounts := func() string { + var b strings.Builder + for mp := range mountedTargets { + b.WriteString(fmt.Sprintf("/dev/sda1 %s ext4 rw 0 0\n", mp)) + } + return b.String() + } + mountGuardReadFile = func(path string) ([]byte, error) { + switch path { + case "/proc/self/mountinfo": + mountinfoReads++ + if mountinfoReads == 1 { + return nil, errors.New("mountinfo read failed") + } + return []byte(buildMountinfo()), nil + case "/proc/mounts": + mountsReads++ + if mountsReads == 1 { + return nil, errors.New("mounts read failed") + } + return []byte(buildProcMounts()), nil + default: + return nil, fmt.Errorf("unexpected read: %s", path) + } + } + + mountGuardReadDir = func(path string) ([]os.DirEntry, error) { + if filepath.Clean(path) == "/mnt/guardok" { + return []os.DirEntry{&fakeDirEntry{name: "nonempty"}}, nil + } + return nil, os.ErrNotExist + } + + mountGuardSysMount = func(_, target, _ string, _ uintptr, _ string) error { + switch filepath.Clean(target) { + case "/mnt/chattrfail", "/mnt/chattrsuccess": + return syscall.EPERM + default: + return nil + } + } + mountGuardSysUnmount = func(string, int) error { return nil } + + cmd := &mountGuardCommandRunner{} + cmd.run = func(_ context.Context, name string, args ...string) ([]byte, error) { + switch name { + case "mount": + if len(args) != 1 { + return nil, fmt.Errorf("unexpected mount args: %v", args) + } + target := filepath.Clean(args[0]) + switch target { + case "/mnt/mountok", "/mnt/mountok2": + if target == "/mnt/mountok2" { + mountedTargets[target] = struct{}{} + } + return nil, nil + case "/mnt/chattrfail": + return []byte(" \n\t"), errors.New("mount failed") + default: + return []byte("mount: failed"), errors.New("mount failed") + } + case "chattr": + if len(args) != 2 || args[0] != "+i" { + return nil, fmt.Errorf("unexpected chattr args: %v", args) + } + target := filepath.Clean(args[1]) + if target == "/mnt/chattrfail" { + return nil, errors.New("chattr failed") + } + return nil, nil + default: + return nil, fmt.Errorf("unexpected command: %s", name) + } + } + restoreCmd = cmd + + if err := maybeApplyPBSDatastoreMountGuards(ctx, logger, plan, stageRoot, "/", false); err != nil { + t.Fatalf("maybeApplyPBSDatastoreMountGuards error: %v", err) + } + + // Ensure the longest fstab mountpoint match wins (/mnt/storage/pbs instead of /mnt/storage). + foundStoragePBS := false + for _, c := range cmd.calls { + if c.name == "mount" && len(c.args) == 1 && filepath.Clean(c.args[0]) == "/mnt/storage/pbs" { + foundStoragePBS = true + break + } + } + if !foundStoragePBS { + t.Fatalf("expected mount attempt for /mnt/storage/pbs, calls=%#v", cmd.calls) + } +} + +func TestMaybeApplyPBSDatastoreMountGuards_MountAttemptTimeout(t *testing.T) { + logger := newTestLogger() + baseCtx, cancel := context.WithDeadline(context.Background(), time.Now().Add(-1*time.Second)) + t.Cleanup(cancel) + plan := &RestorePlan{SystemType: SystemTypePBS, NormalCategories: []Category{{ID: "datastore_pbs"}}} + + origGeteuid := mountGuardGeteuid + origReadFile := mountGuardReadFile + origMkdirAll := mountGuardMkdirAll + origMount := mountGuardSysMount + origUnmount := mountGuardSysUnmount + origFstab := mountGuardFstabMountpointsSet + origRootFS := mountGuardIsPathOnRootFilesystem + origCmd := restoreCmd + t.Cleanup(func() { + mountGuardGeteuid = origGeteuid + mountGuardReadFile = origReadFile + mountGuardMkdirAll = origMkdirAll + mountGuardSysMount = origMount + mountGuardSysUnmount = origUnmount + mountGuardFstabMountpointsSet = origFstab + mountGuardIsPathOnRootFilesystem = origRootFS + restoreCmd = origCmd + }) + + mountGuardGeteuid = func() int { return 0 } + mountGuardReadFile = func(string) ([]byte, error) { return []byte(""), nil } + mountGuardMkdirAll = func(string, os.FileMode) error { return nil } + mountGuardSysMount = func(string, string, string, uintptr, string) error { return nil } + mountGuardSysUnmount = func(string, int) error { return nil } + mountGuardIsPathOnRootFilesystem = func(path string) (bool, string, error) { return true, filepath.Clean(path), nil } + mountGuardFstabMountpointsSet = func(string) (map[string]struct{}, error) { + return map[string]struct{}{"/mnt/timeout": {}}, nil + } + + stageRoot := t.TempDir() + stagePath := filepath.Join(stageRoot, "etc/proxmox-backup/datastore.cfg") + if err := os.MkdirAll(filepath.Dir(stagePath), 0o755); err != nil { + t.Fatalf("mkdir staged dir: %v", err) + } + if err := os.WriteFile(stagePath, []byte("datastore: ds\n path /mnt/timeout/store\n"), 0o600); err != nil { + t.Fatalf("write datastore.cfg: %v", err) + } + + restoreCmd = &mountGuardCommandRunner{ + run: func(ctx context.Context, name string, args ...string) ([]byte, error) { + if name == "mount" { + return nil, ctx.Err() + } + if name == "chattr" { + return nil, ctx.Err() + } + return nil, fmt.Errorf("unexpected command: %s", name) + }, + } + + if err := maybeApplyPBSDatastoreMountGuards(baseCtx, logger, plan, stageRoot, "/", false); err != nil { + t.Fatalf("maybeApplyPBSDatastoreMountGuards error: %v", err) + } +} diff --git a/internal/orchestrator/pbs_mount_guard_test.go b/internal/orchestrator/pbs_mount_guard_test.go index f456170..a9efbc1 100644 --- a/internal/orchestrator/pbs_mount_guard_test.go +++ b/internal/orchestrator/pbs_mount_guard_test.go @@ -13,6 +13,7 @@ func TestPBSMountGuardRootForDatastorePath(t *testing.T) { {name: "mnt nested", in: "/mnt/datastore/Data1", want: "/mnt/datastore"}, {name: "mnt deep", in: "/mnt/Synology_NFS/PBS_Backup", want: "/mnt/Synology_NFS"}, {name: "media", in: "/media/USB/PBS", want: "/media/USB"}, + {name: "run media root", in: "/run/media/root", want: "/run/media/root"}, {name: "run media", in: "/run/media/root/USB/PBS", want: "/run/media/root/USB"}, {name: "not mount style", in: "/srv/pbs", want: ""}, {name: "empty", in: "", want: ""}, From efa57ece8546f5fbfd76a5e9d479c259ee0ca59a Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Mon, 23 Feb 2026 03:13:31 +0000 Subject: [PATCH 4/4] ci: bump goreleaser/goreleaser-action in the actions-updates group Bumps the actions-updates group with 1 update: [goreleaser/goreleaser-action](https://github.com/goreleaser/goreleaser-action). Updates `goreleaser/goreleaser-action` from 6 to 7 - [Release notes](https://github.com/goreleaser/goreleaser-action/releases) - [Commits](https://github.com/goreleaser/goreleaser-action/compare/v6...v7) --- updated-dependencies: - dependency-name: goreleaser/goreleaser-action dependency-version: '7' dependency-type: direct:production update-type: version-update:semver-major dependency-group: actions-updates ... Signed-off-by: dependabot[bot] --- .github/workflows/release.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 73f72f2..8da4761 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -70,7 +70,7 @@ jobs: # GORELEASER ######################################## - name: Run GoReleaser - uses: goreleaser/goreleaser-action@v6 + uses: goreleaser/goreleaser-action@v7 with: version: latest workdir: ${{ github.workspace }}