From 90d7b69c539cf68004c5790055578b164f355600 Mon Sep 17 00:00:00 2001 From: Damiano <71268257+tis24dev@users.noreply.github.com> Date: Sun, 22 Feb 2026 17:04:10 +0100 Subject: [PATCH 1/2] Skip privilege-sensitive failures in rootless env Detect unprivileged/shifted user-namespace environments (rootless/LXC) and treat certain privilege-sensitive command failures as non-critical SKIPs instead of warnings. Adds environment detection (reading /proc/self/{uid_map,gid_map} and /run/systemd/container), a deps hook to override detection, and Collector logic to downgrade failures for dmidecode, blkid, sensors and smartctl when patterns match. Updates logging to include SKIP lines and debug details, introduces unit tests for detection and matching, and documents the behavior and restore implications (limited /etc/fstab remap) in the docs. --- docs/CLI_REFERENCE.md | 16 ++ docs/RESTORE_GUIDE.md | 2 +- docs/RESTORE_TECHNICAL.md | 1 + docs/TROUBLESHOOTING.md | 29 +++ internal/backup/collector.go | 103 +++++++- internal/backup/collector_deps.go | 9 +- .../backup/collector_privilege_sensitive.go | 88 +++++++ .../collector_privilege_sensitive_test.go | 222 ++++++++++++++++++ internal/backup/collector_unprivileged.go | 39 +++ internal/environment/unprivileged.go | 93 ++++++++ internal/environment/unprivileged_test.go | 103 ++++++++ 11 files changed, 691 insertions(+), 14 deletions(-) create mode 100644 internal/backup/collector_privilege_sensitive.go create mode 100644 internal/backup/collector_privilege_sensitive_test.go create mode 100644 internal/backup/collector_unprivileged.go create mode 100644 internal/environment/unprivileged.go create mode 100644 internal/environment/unprivileged_test.go diff --git a/docs/CLI_REFERENCE.md b/docs/CLI_REFERENCE.md index eae8335..d32a00d 100644 --- a/docs/CLI_REFERENCE.md +++ b/docs/CLI_REFERENCE.md @@ -496,6 +496,22 @@ If you want to remove those guards manually (optional): - `--log-level` (CLI flag): Controls logging verbosity - `DEBUG_LEVEL` (config): Controls operation detail level (`standard`/`advanced`/`extreme`) +### Log Labels (PHASE/STEP/SKIP) + +Some log lines use a label to make the output easier to scan: + +| Label | Level | Meaning | +|-------|-------|---------| +| `PHASE` | `info` | High-level workflow phase marker | +| `STEP` | `info` | A notable step within a phase | +| `SKIP` | `info` | Optional item intentionally skipped or not applicable | + +**Common `SKIP` examples**: +- A feature is disabled by configuration. +- A non-critical CLI tool is not installed. +- Running in an **unprivileged container/rootless** environment where low-level inventory commands are expected to fail (for example `dmidecode` or `blkid`). In this case, ProxSave still attempts the collection, but logs a `SKIP` (not a `WARNING`) when the failure matches known “missing privileges” patterns. + - For `blkid`, the skip reason also includes a restore hint: automated `/etc/fstab` device remap (UUID/PARTUUID/LABEL) may be limited. + ### Flag Reference | Flag | Short | Description | diff --git a/docs/RESTORE_GUIDE.md b/docs/RESTORE_GUIDE.md index bac43c2..4c04c4c 100644 --- a/docs/RESTORE_GUIDE.md +++ b/docs/RESTORE_GUIDE.md @@ -1922,7 +1922,7 @@ If the restore includes filesystem configuration (notably `/etc/fstab`), ProxSav - Compares the current `/etc/fstab` with the backup copy. - Keeps existing critical entries (for example, root and swap) when they already match the running system. - Detects **safe mount candidates** from the backup (for example, additional NFS mounts) and offers to add them. -- If ProxSave inventory data is present in the backup, ProxSave can remap **unstable** `/dev/*` devices from the backup (for example `/dev/sdb1`) to stable `UUID=`/`PARTUUID=`/`LABEL=` references **on the restore host** (only when the stable reference exists on the system). +- If ProxSave inventory data is present in the backup, ProxSave can remap **unstable** `/dev/*` devices from the backup (for example `/dev/sdb1`) to stable `UUID=`/`PARTUUID=`/`LABEL=` references **on the restore host** (only when the stable reference exists on the system). Note: backups taken from an **unprivileged container/rootless** environment may not include usable block-device inventory, so automated remap can be limited/unavailable. - Normalizes restored entries by adding `nofail` (and `_netdev` for network mounts) so offline storage does not block boot/restore. **Safety behavior**: diff --git a/docs/RESTORE_TECHNICAL.md b/docs/RESTORE_TECHNICAL.md index 2bd5c02..b862e37 100644 --- a/docs/RESTORE_TECHNICAL.md +++ b/docs/RESTORE_TECHNICAL.md @@ -1510,6 +1510,7 @@ When restoring to the real system root (`/`), ProxSave avoids blindly overwritin - If the backup contains ProxSave inventory (`var/lib/proxsave-info/commands/system/{blkid.txt,lsblk_json.json,lsblk.txt}` or PBS datastore inventory), ProxSave can remap unstable device paths from the backup (e.g. `/dev/sdb1`) to stable references (`UUID=`/`PARTUUID=`/`LABEL=`) **when the stable reference exists on the restore host**. - This reduces the risk of mounting the wrong disk after a reinstall where `/dev/sdX` ordering changes. +- Note: backups taken from an **unprivileged container/rootless** environment may not include usable block-device inventory (for example `blkid` output can be empty/skipped). In that case, automated device remap is limited/unavailable and `/etc/fstab` entries may require manual review during restore. **Normalization**: - Entries written by the merge are normalized to include `nofail` (and `_netdev` for network mounts) to prevent offline storage from blocking boot/restore. diff --git a/docs/TROUBLESHOOTING.md b/docs/TROUBLESHOOTING.md index 04c7e84..49ac807 100644 --- a/docs/TROUBLESHOOTING.md +++ b/docs/TROUBLESHOOTING.md @@ -168,6 +168,35 @@ COMPRESSION_TYPE=xz # Valid: xz, zstd, gzip, bzip2, lz4 --- +#### Notice: `SKIP ... Expected in unprivileged containers` (LXC/rootless) + +**Symptoms**: +- Running ProxSave inside an **unprivileged** LXC container (or a rootless container) produces log lines like: + - `SKIP Skipping Hardware DMI information: command \`dmidecode\` failed (...). Expected in unprivileged containers (...) (DMI tables not accessible). Non-critical; backup continues.` + - `SKIP Skipping Block device identifiers (blkid): command \`blkid\` failed (...). Expected in unprivileged containers (...) (block devices not accessible; restore hint: automated fstab device remap (UUID/PARTUUID/LABEL) may be limited). Non-critical; backup continues.` + +**Cause**: In unprivileged containers, access to low-level system interfaces is intentionally restricted (for example `/dev/mem` and most block devices). Some inventory commands can fail even though the backup itself is working correctly. + +**Behavior**: +- ProxSave still attempts the collection. +- Only a small allowlist of **privilege-sensitive** commands is downgraded from `WARNING` to `SKIP` when failure is expected in this environment (`dmidecode`, `blkid`, `sensors`, `smartctl`). +- Other failures are **not** downgraded and still appear as warnings/errors. + +**Impact**: +- Hardware inventory output may be missing/empty. +- If `blkid` is skipped, ProxSave restore may have **limited** ability to automatically remap `/etc/fstab` devices (UUID/PARTUUID/LABEL). You may need to review mounts manually during restore. + +**How to verify** (shifted user namespace mapping): +```bash +cat /proc/self/uid_map +cat /proc/self/gid_map +# If the second column is non-zero (e.g. "0 100000 65536"), you're in a shifted/unprivileged mapping. +``` + +**Optional**: If you want to hide `SKIP` lines on the console, run with `--log-level warning` (this also hides normal info logs). + +--- + ### 3. Cloud Storage Issues #### Error: `rclone not found in PATH` diff --git a/internal/backup/collector.go b/internal/backup/collector.go index 925d298..b9c0c73 100644 --- a/internal/backup/collector.go +++ b/internal/backup/collector.go @@ -40,14 +40,16 @@ type FileSummary struct { // Collector handles backup data collection type Collector struct { - logger *logging.Logger - config *CollectorConfig - stats *CollectionStats - statsMu sync.Mutex - tempDir string - proxType types.ProxmoxType - dryRun bool - deps CollectorDeps + logger *logging.Logger + config *CollectorConfig + stats *CollectionStats + statsMu sync.Mutex + tempDir string + proxType types.ProxmoxType + dryRun bool + deps CollectorDeps + unprivilegedOnce sync.Once + unprivilegedCtx unprivilegedContainerContext // clusteredPVE records whether cluster mode was detected during PVE collection. clusteredPVE bool @@ -915,11 +917,56 @@ func (c *Collector) safeCmdOutput(ctx context.Context, cmd, output, description c.incFilesFailed() return 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 (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)) + + ctxInfo := c.depDetectUnprivilegedContainer() + c.logger.Debug("Unprivileged 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", cmdParts[0], isPrivilegeSensitiveCommand(cmdParts[0])) + match := privilegeSensitiveFailureMatch(cmdParts[0], exitCode, outputText) + reason = match.Reason + c.logger.Debug("Privilege-sensitive classification: command=%q matched=%t match=%q reason=%q", cmdParts[0], reason != "", match.Match, reason) + } else { + c.logger.Debug("Privilege-sensitive downgrade not considered: unprivileged context not detected (command=%q)", cmdParts[0]) + } + + if ctxInfo.Detected && reason != "" { + c.logger.Debug("Downgrading WARNING->SKIP: description=%q cmd=%q exitCode=%d", description, cmdString, exitCode) + + details := strings.TrimSpace(ctxInfo.Details) + if details != "" { + details = " (" + details + ")" + } + c.logger.Skip("Skipping %s: command `%s` failed (%v). Expected in unprivileged containers%s (%s). Non-critical; backup continues.", + description, + cmdString, + err, + details, + reason, + ) + c.logger.Debug("Skip details for %s: output: %s", description, summarizeCommandOutputText(outputText)) + return nil + } + + if ctxInfo.Detected { + c.logger.Debug("No privilege-sensitive downgrade applied: command=%q did not match known patterns; emitting WARNING", cmdParts[0]) + } + 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(string(out)), + summarizeCommandOutputText(outputText), ) return nil // Non-critical failure } @@ -1236,6 +1283,44 @@ func (c *Collector) captureCommandOutput(ctx context.Context, cmd, output, descr } 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("Unprivileged 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", parts[0], isPrivilegeSensitiveCommand(parts[0])) + match := privilegeSensitiveFailureMatch(parts[0], exitCode, outputText) + reason = match.Reason + c.logger.Debug("Privilege-sensitive classification: command=%q matched=%t match=%q reason=%q", parts[0], reason != "", match.Match, reason) + } else { + c.logger.Debug("Privilege-sensitive downgrade not considered: unprivileged context not detected (command=%q)", parts[0]) + } + + if ctxInfo.Detected && reason != "" { + c.logger.Debug("Downgrading WARNING->SKIP: description=%q cmd=%q exitCode=%d", description, cmdString, exitCode) + + details := strings.TrimSpace(ctxInfo.Details) + if details != "" { + details = " (" + details + ")" + } + c.logger.Skip("Skipping %s: command `%s` failed (%v). Expected in unprivileged containers%s (%s). Non-critical; backup continues.", + description, + cmdString, + err, + details, + reason, + ) + c.logger.Debug("Skip details for %s: output: %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", parts[0]) + } + if parts[0] == "systemctl" && len(parts) >= 2 && parts[1] == "status" { unit := parts[len(parts)-1] if exitCode == 4 || strings.Contains(outputText, "could not be found") { diff --git a/internal/backup/collector_deps.go b/internal/backup/collector_deps.go index cbb9aff..6cb980a 100644 --- a/internal/backup/collector_deps.go +++ b/internal/backup/collector_deps.go @@ -26,10 +26,11 @@ var ( // CollectorDeps allows injecting external dependencies for the Collector. type CollectorDeps struct { - LookPath func(string) (string, error) - RunCommandWithEnv func(context.Context, []string, string, ...string) ([]byte, error) - RunCommand func(context.Context, string, ...string) ([]byte, error) - Stat func(string) (os.FileInfo, error) + LookPath func(string) (string, error) + RunCommandWithEnv func(context.Context, []string, string, ...string) ([]byte, error) + RunCommand func(context.Context, string, ...string) ([]byte, error) + Stat func(string) (os.FileInfo, error) + DetectUnprivilegedContainer func() (bool, string) } func defaultCollectorDeps() CollectorDeps { diff --git a/internal/backup/collector_privilege_sensitive.go b/internal/backup/collector_privilege_sensitive.go new file mode 100644 index 0000000..0ce39b5 --- /dev/null +++ b/internal/backup/collector_privilege_sensitive.go @@ -0,0 +1,88 @@ +package backup + +import "strings" + +type privilegeSensitiveMatch struct { + Reason string + Match string +} + +func isPrivilegeSensitiveCommand(command string) bool { + switch strings.TrimSpace(command) { + case "dmidecode", "blkid", "sensors", "smartctl": + return true + default: + return false + } +} + +func privilegeSensitiveFailureMatch(command string, exitCode int, outputText string) privilegeSensitiveMatch { + command = strings.TrimSpace(command) + if command == "" { + return privilegeSensitiveMatch{} + } + + outputText = strings.TrimSpace(outputText) + lower := strings.ToLower(outputText) + + switch command { + case "dmidecode": + // dmidecode typically fails due to restricted access to DMI tables (/sys/firmware/dmi or /dev/mem). + switch { + case strings.Contains(lower, "/dev/mem"): + return privilegeSensitiveMatch{Reason: "DMI tables not accessible", Match: "stderr contains /dev/mem"} + case strings.Contains(lower, "permission denied"): + return privilegeSensitiveMatch{Reason: "DMI tables not accessible", Match: "stderr contains permission denied"} + case strings.Contains(lower, "operation not permitted"): + return privilegeSensitiveMatch{Reason: "DMI tables not accessible", Match: "stderr contains operation not permitted"} + case exitCode != 0 && outputText == "": + return privilegeSensitiveMatch{Reason: "DMI tables not accessible", Match: "exit!=0 and empty output"} + } + return privilegeSensitiveMatch{} + case "blkid": + // In unprivileged LXC, blkid often exits 2 with empty output when block devices are not accessible. + switch { + case exitCode == 2 && outputText == "": + return privilegeSensitiveMatch{ + Reason: "block devices not accessible; restore hint: automated fstab device remap (UUID/PARTUUID/LABEL) may be limited", + Match: "exit=2 and empty output", + } + case strings.Contains(lower, "permission denied"): + return privilegeSensitiveMatch{ + Reason: "block devices not accessible; restore hint: automated fstab device remap (UUID/PARTUUID/LABEL) may be limited", + Match: "stderr contains permission denied", + } + case strings.Contains(lower, "operation not permitted"): + return privilegeSensitiveMatch{ + Reason: "block devices not accessible; restore hint: automated fstab device remap (UUID/PARTUUID/LABEL) may be limited", + Match: "stderr contains operation not permitted", + } + } + return privilegeSensitiveMatch{} + case "sensors": + // In containers, sensors may not be available or may report no sensors found. + switch { + case strings.Contains(lower, "permission denied"): + return privilegeSensitiveMatch{Reason: "hardware sensors not accessible", Match: "stderr contains permission denied"} + case strings.Contains(lower, "operation not permitted"): + return privilegeSensitiveMatch{Reason: "hardware sensors not accessible", Match: "stderr contains operation not permitted"} + case strings.Contains(lower, "no sensors found"): + return privilegeSensitiveMatch{Reason: "hardware sensors not accessible", Match: "stderr contains no sensors found"} + } + return privilegeSensitiveMatch{} + case "smartctl": + switch { + case strings.Contains(lower, "permission denied"): + return privilegeSensitiveMatch{Reason: "SMART devices not accessible", Match: "stderr contains permission denied"} + case strings.Contains(lower, "operation not permitted"): + return privilegeSensitiveMatch{Reason: "SMART devices not accessible", Match: "stderr contains operation not permitted"} + } + return privilegeSensitiveMatch{} + default: + return privilegeSensitiveMatch{} + } +} + +func privilegeSensitiveFailureReason(command string, exitCode int, outputText string) string { + return privilegeSensitiveFailureMatch(command, exitCode, outputText).Reason +} diff --git a/internal/backup/collector_privilege_sensitive_test.go b/internal/backup/collector_privilege_sensitive_test.go new file mode 100644 index 0000000..13b846e --- /dev/null +++ b/internal/backup/collector_privilege_sensitive_test.go @@ -0,0 +1,222 @@ +package backup + +import ( + "bytes" + "context" + "os" + "os/exec" + "path/filepath" + "strings" + "testing" + + "github.com/tis24dev/proxsave/internal/logging" + "github.com/tis24dev/proxsave/internal/types" +) + +func TestSafeCmdOutput_Unprivileged_DowngradesDmidecodeToSkip(t *testing.T) { + buf := &bytes.Buffer{} + logger := logging.New(types.LogLevelDebug, false) + logger.SetOutput(buf) + + deps := CollectorDeps{ + LookPath: func(string) (string, error) { return "/usr/sbin/dmidecode", nil }, + RunCommand: func(ctx context.Context, name string, args ...string) ([]byte, error) { + cmd := exec.CommandContext(ctx, "sh", "-c", "echo '/dev/mem: Permission denied'; exit 1") + return cmd.CombinedOutput() + }, + DetectUnprivilegedContainer: func() (bool, string) { + return true, "uid_map=0->100000(len=65536), container=lxc" + }, + } + + tmp := t.TempDir() + c := NewCollectorWithDeps(logger, GetDefaultCollectorConfig(), tmp, types.ProxmoxUnknown, false, deps) + outPath := filepath.Join(tmp, "dmidecode.txt") + + if err := c.safeCmdOutput(context.Background(), "dmidecode", outPath, "Hardware DMI information", false); err != nil { + t.Fatalf("safeCmdOutput returned error: %v", err) + } + + if _, err := os.Stat(outPath); !os.IsNotExist(err) { + t.Fatalf("expected no output file to be created, stat err=%v", err) + } + + logText := buf.String() + if !strings.Contains(logText, "] SKIP") { + t.Fatalf("expected SKIP log line, got: %s", logText) + } + if !strings.Contains(logText, "Expected in unprivileged containers") { + t.Fatalf("expected unprivileged hint in logs, got: %s", logText) + } + if !strings.Contains(logText, "DMI tables not accessible") { + t.Fatalf("expected reason in logs, got: %s", logText) + } +} + +func TestCaptureCommandOutput_Unprivileged_DowngradesBlkidToSkipWithRestoreHint(t *testing.T) { + buf := &bytes.Buffer{} + logger := logging.New(types.LogLevelDebug, false) + logger.SetOutput(buf) + + deps := CollectorDeps{ + LookPath: func(string) (string, error) { return "/sbin/blkid", nil }, + RunCommand: func(ctx context.Context, name string, args ...string) ([]byte, error) { + cmd := exec.CommandContext(ctx, "sh", "-c", "exit 2") + return cmd.CombinedOutput() + }, + DetectUnprivilegedContainer: func() (bool, string) { + return true, "uid_map=0->100000(len=65536), container=lxc" + }, + } + + tmp := t.TempDir() + c := NewCollectorWithDeps(logger, GetDefaultCollectorConfig(), tmp, types.ProxmoxUnknown, false, deps) + outPath := filepath.Join(tmp, "blkid.txt") + + data, err := c.captureCommandOutput(context.Background(), "blkid", outPath, "Block device identifiers (blkid)", false) + if err != nil { + t.Fatalf("captureCommandOutput returned error: %v", err) + } + if data != nil { + t.Fatalf("expected nil data when command is skipped, got %q", string(data)) + } + if _, err := os.Stat(outPath); !os.IsNotExist(err) { + t.Fatalf("expected no output file to be created, stat err=%v", err) + } + + logText := buf.String() + if !strings.Contains(logText, "] SKIP") { + t.Fatalf("expected SKIP log line, got: %s", logText) + } + if !strings.Contains(logText, "Expected in unprivileged containers") { + t.Fatalf("expected unprivileged hint in logs, got: %s", logText) + } + if !strings.Contains(logText, "restore hint: automated fstab device remap") { + t.Fatalf("expected restore hint in logs, got: %s", logText) + } +} + +func TestSafeCmdOutput_Unprivileged_DowngradesSensorsToSkip(t *testing.T) { + buf := &bytes.Buffer{} + logger := logging.New(types.LogLevelDebug, false) + logger.SetOutput(buf) + + deps := CollectorDeps{ + LookPath: func(string) (string, error) { return "/usr/bin/sensors", nil }, + RunCommand: func(ctx context.Context, name string, args ...string) ([]byte, error) { + cmd := exec.CommandContext(ctx, "sh", "-c", "echo 'No sensors found'; exit 1") + return cmd.CombinedOutput() + }, + DetectUnprivilegedContainer: func() (bool, string) { + return true, "uid_map=0->100000(len=65536), container=lxc" + }, + } + + tmp := t.TempDir() + c := NewCollectorWithDeps(logger, GetDefaultCollectorConfig(), tmp, types.ProxmoxUnknown, false, deps) + outPath := filepath.Join(tmp, "sensors.txt") + + if err := c.safeCmdOutput(context.Background(), "sensors", outPath, "Hardware sensors", false); err != nil { + t.Fatalf("safeCmdOutput returned error: %v", err) + } + + if _, err := os.Stat(outPath); !os.IsNotExist(err) { + t.Fatalf("expected no output file to be created, stat err=%v", err) + } + + logText := buf.String() + if !strings.Contains(logText, "] SKIP") { + t.Fatalf("expected SKIP log line, got: %s", logText) + } + if !strings.Contains(logText, "Expected in unprivileged containers") { + t.Fatalf("expected unprivileged hint in logs, got: %s", logText) + } + if !strings.Contains(logText, "hardware sensors not accessible") { + t.Fatalf("expected reason in logs, got: %s", logText) + } +} + +func TestSafeCmdOutput_Unprivileged_DowngradesSmartctlToSkip(t *testing.T) { + buf := &bytes.Buffer{} + logger := logging.New(types.LogLevelDebug, false) + logger.SetOutput(buf) + + deps := CollectorDeps{ + LookPath: func(string) (string, error) { return "/usr/sbin/smartctl", nil }, + RunCommand: func(ctx context.Context, name string, args ...string) ([]byte, error) { + cmd := exec.CommandContext(ctx, "sh", "-c", "echo 'Permission denied'; exit 1") + return cmd.CombinedOutput() + }, + DetectUnprivilegedContainer: func() (bool, string) { + return true, "uid_map=0->100000(len=65536), container=lxc" + }, + } + + tmp := t.TempDir() + c := NewCollectorWithDeps(logger, GetDefaultCollectorConfig(), tmp, types.ProxmoxUnknown, false, deps) + outPath := filepath.Join(tmp, "smartctl_scan.txt") + + if err := c.safeCmdOutput(context.Background(), "smartctl --scan", outPath, "SMART scan", false); err != nil { + t.Fatalf("safeCmdOutput returned error: %v", err) + } + + if _, err := os.Stat(outPath); !os.IsNotExist(err) { + t.Fatalf("expected no output file to be created, stat err=%v", err) + } + + logText := buf.String() + if !strings.Contains(logText, "] SKIP") { + t.Fatalf("expected SKIP log line, got: %s", logText) + } + if !strings.Contains(logText, "Expected in unprivileged containers") { + t.Fatalf("expected unprivileged hint in logs, got: %s", logText) + } + if !strings.Contains(logText, "SMART devices not accessible") { + t.Fatalf("expected reason in logs, got: %s", logText) + } +} + +func TestPrivilegeSensitiveFailureReason(t *testing.T) { + const blkidReason = "block devices not accessible; restore hint: automated fstab device remap (UUID/PARTUUID/LABEL) may be limited" + + tests := []struct { + name string + command string + exitCode int + output string + want string + }{ + {"dmidecode permission denied", "dmidecode", 1, "Permission denied", "DMI tables not accessible"}, + {"dmidecode /dev/mem", "dmidecode", 1, "/dev/mem: Operation not permitted", "DMI tables not accessible"}, + {"dmidecode operation not permitted", "dmidecode", 1, "Operation not permitted", "DMI tables not accessible"}, + {"dmidecode exit non-zero empty output", "dmidecode", 1, "", "DMI tables not accessible"}, + {"dmidecode success", "dmidecode", 0, "some output", ""}, + + {"blkid exit2 empty", "blkid", 2, "", blkidReason}, + {"blkid exit2 with output", "blkid", 2, "/dev/sda1: UUID=\"...\"", ""}, + {"blkid permission denied", "blkid", 1, "Permission denied", blkidReason}, + {"blkid exit0 empty", "blkid", 0, "", ""}, + + {"sensors no sensors found", "sensors", 1, "No sensors found", "hardware sensors not accessible"}, + {"sensors permission denied", "sensors", 1, "Permission denied", "hardware sensors not accessible"}, + {"sensors success", "sensors", 0, "coretemp-isa-0000\nAdapter: ISA adapter\n", ""}, + + {"smartctl permission denied", "smartctl", 1, "Permission denied", "SMART devices not accessible"}, + {"smartctl operation not permitted", "smartctl", 1, "Operation not permitted", "SMART devices not accessible"}, + {"smartctl no such device", "smartctl", 1, "No such device", ""}, + {"smartctl with spaces", " smartctl ", 1, "Permission denied", "SMART devices not accessible"}, + + {"unknown command no match", "lspci", 1, "permission denied", ""}, + {"empty command", "", 1, "permission denied", ""}, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + got := privilegeSensitiveFailureReason(tc.command, tc.exitCode, tc.output) + if got != tc.want { + t.Fatalf("privilegeSensitiveFailureReason(%q, %d, %q) = %q, want %q", + tc.command, tc.exitCode, tc.output, got, tc.want) + } + }) + } +} diff --git a/internal/backup/collector_unprivileged.go b/internal/backup/collector_unprivileged.go new file mode 100644 index 0000000..7eb67f4 --- /dev/null +++ b/internal/backup/collector_unprivileged.go @@ -0,0 +1,39 @@ +package backup + +import "github.com/tis24dev/proxsave/internal/environment" + +type unprivilegedContainerContext struct { + Detected bool + Details string +} + +func (c *Collector) depDetectUnprivilegedContainer() unprivilegedContainerContext { + if c == nil { + return unprivilegedContainerContext{} + } + + c.unprivilegedOnce.Do(func() { + if c.deps.DetectUnprivilegedContainer != nil { + detected, details := c.deps.DetectUnprivilegedContainer() + c.unprivilegedCtx = unprivilegedContainerContext{ + Detected: detected, + Details: details, + } + if c.logger != nil { + c.logger.Debug("Unprivileged container detection (deps override): detected=%t details=%q", c.unprivilegedCtx.Detected, c.unprivilegedCtx.Details) + } + return + } + + info := environment.DetectUnprivilegedContainer() + c.unprivilegedCtx = unprivilegedContainerContext{ + Detected: info.Detected, + Details: info.Details, + } + if c.logger != nil { + c.logger.Debug("Unprivileged container detection: detected=%t details=%q", c.unprivilegedCtx.Detected, c.unprivilegedCtx.Details) + } + }) + + return c.unprivilegedCtx +} diff --git a/internal/environment/unprivileged.go b/internal/environment/unprivileged.go new file mode 100644 index 0000000..0e96dcd --- /dev/null +++ b/internal/environment/unprivileged.go @@ -0,0 +1,93 @@ +package environment + +import ( + "fmt" + "strconv" + "strings" +) + +const ( + selfUIDMapPath = "/proc/self/uid_map" + selfGIDMapPath = "/proc/self/gid_map" + systemdContainerPath = "/run/systemd/container" +) + +// UnprivilegedContainerInfo describes whether the current process appears to be +// running in an unprivileged (shifted user namespace) environment. +// +// This is commonly true for unprivileged LXC containers and rootless containers. +type UnprivilegedContainerInfo struct { + Detected bool + Details string +} + +// DetectUnprivilegedContainer detects whether the current process appears to be +// running in an unprivileged (shifted user namespace) environment. +// +// Detection is based on /proc/self/{uid_map,gid_map}. When the mapping for +// inside-ID 0 maps to a non-zero outside-ID, the process is in a shifted user +// namespace and likely lacks low-level hardware/block-device privileges. +// +// The return value is intentionally best-effort and never returns an error. +func DetectUnprivilegedContainer() UnprivilegedContainerInfo { + uidOutside0, uidLen, uidOK := readIDMapOutsideZero(selfUIDMapPath) + gidOutside0, gidLen, gidOK := readIDMapOutsideZero(selfGIDMapPath) + + detected := (uidOK && uidOutside0 != 0) || (gidOK && gidOutside0 != 0) + details := make([]string, 0, 3) + + if uidOK { + details = append(details, fmt.Sprintf("uid_map=0->%d(len=%d)", uidOutside0, uidLen)) + } else { + details = append(details, "uid_map=unavailable") + } + if gidOK { + details = append(details, fmt.Sprintf("gid_map=0->%d(len=%d)", gidOutside0, gidLen)) + } else { + details = append(details, "gid_map=unavailable") + } + + if container := strings.TrimSpace(readAndTrim(systemdContainerPath)); container != "" { + details = append(details, fmt.Sprintf("container=%s", container)) + } + + out := UnprivilegedContainerInfo{Detected: detected} + if len(details) > 0 { + out.Details = strings.Join(details, ", ") + } + return out +} + +func readIDMapOutsideZero(path string) (outside0, length int64, ok bool) { + data, err := readFileFunc(path) + if err != nil { + return 0, 0, false + } + return parseIDMapOutsideZero(string(data)) +} + +func parseIDMapOutsideZero(content string) (outside0, length int64, ok bool) { + for _, line := range strings.Split(content, "\n") { + line = strings.TrimSpace(line) + if line == "" { + continue + } + fields := strings.Fields(line) + if len(fields) < 3 { + continue + } + + insideStart, err1 := strconv.ParseInt(fields[0], 10, 64) + outsideStart, err2 := strconv.ParseInt(fields[1], 10, 64) + lengthVal, err3 := strconv.ParseInt(fields[2], 10, 64) + if err1 != nil || err2 != nil || err3 != nil { + continue + } + + // We only care about the mapping for inside ID 0. + if insideStart == 0 && lengthVal > 0 { + return outsideStart, lengthVal, true + } + } + return 0, 0, false +} diff --git a/internal/environment/unprivileged_test.go b/internal/environment/unprivileged_test.go new file mode 100644 index 0000000..2cb55c1 --- /dev/null +++ b/internal/environment/unprivileged_test.go @@ -0,0 +1,103 @@ +package environment + +import ( + "errors" + "strings" + "testing" +) + +func TestParseIDMapOutsideZero(t *testing.T) { + t.Run("privileged mapping", func(t *testing.T) { + outside0, length, ok := parseIDMapOutsideZero("0 0 4294967295\n") + if !ok { + t.Fatal("expected ok=true") + } + if outside0 != 0 { + t.Fatalf("outside0=%d, want 0", outside0) + } + if length != 4294967295 { + t.Fatalf("length=%d, want 4294967295", length) + } + }) + + t.Run("unprivileged mapping", func(t *testing.T) { + outside0, length, ok := parseIDMapOutsideZero("0 100000 65536\n") + if !ok { + t.Fatal("expected ok=true") + } + if outside0 != 100000 { + t.Fatalf("outside0=%d, want 100000", outside0) + } + if length != 65536 { + t.Fatalf("length=%d, want 65536", length) + } + }) + + t.Run("multiple lines", func(t *testing.T) { + outside0, length, ok := parseIDMapOutsideZero("0 100000 65536\n1000 0 1\n") + if !ok { + t.Fatal("expected ok=true") + } + if outside0 != 100000 || length != 65536 { + t.Fatalf("got outside0=%d length=%d, want outside0=100000 length=65536", outside0, length) + } + }) + + t.Run("invalid", func(t *testing.T) { + _, _, ok := parseIDMapOutsideZero("not a map\n") + if ok { + t.Fatal("expected ok=false") + } + }) +} + +func TestDetectUnprivilegedContainer(t *testing.T) { + t.Run("shifted uid/gid maps", func(t *testing.T) { + setValue(t, &readFileFunc, func(path string) ([]byte, error) { + switch path { + case selfUIDMapPath: + return []byte("0 100000 65536\n"), nil + case selfGIDMapPath: + return []byte("0 100000 65536\n"), nil + case systemdContainerPath: + return []byte("lxc\n"), nil + default: + return nil, errors.New("not found") + } + }) + + info := DetectUnprivilegedContainer() + if !info.Detected { + t.Fatalf("Detected=false, want true (details=%q)", info.Details) + } + if !strings.Contains(info.Details, "uid_map=0->100000") { + t.Fatalf("expected uid_map details, got %q", info.Details) + } + if !strings.Contains(info.Details, "gid_map=0->100000") { + t.Fatalf("expected gid_map details, got %q", info.Details) + } + if !strings.Contains(info.Details, "container=lxc") { + t.Fatalf("expected container details, got %q", info.Details) + } + }) + + t.Run("privileged mapping", func(t *testing.T) { + setValue(t, &readFileFunc, func(path string) ([]byte, error) { + switch path { + case selfUIDMapPath: + return []byte("0 0 4294967295\n"), nil + case selfGIDMapPath: + return []byte("0 0 4294967295\n"), nil + case systemdContainerPath: + return []byte("lxc\n"), nil + default: + return nil, errors.New("not found") + } + }) + + info := DetectUnprivilegedContainer() + if info.Detected { + t.Fatalf("Detected=true, want false (details=%q)", info.Details) + } + }) +} From 94762a908dfaffdd9161376f7dd9df72733e0b5c Mon Sep 17 00:00:00 2001 From: Damiano <71268257+tis24dev@users.noreply.github.com> Date: Sun, 22 Feb 2026 17:40:19 +0100 Subject: [PATCH 2/2] Simplify blkid unprivileged messages and logs Shorten and standardize wording for blkid/unprivileged skip messages across docs and code. Replace verbose "automated /etc/fstab device remap (UUID/PARTUUID/LABEL)" wording with a concise "fstab remap may be limited" hint, centralize the blkid reason text in collector_privilege_sensitive.go, and simplify Skip logging in collector.go while adding clearer debug entries. Update corresponding tests and documentation to match the new message. --- docs/CLI_REFERENCE.md | 2 +- docs/TROUBLESHOOTING.md | 4 +-- internal/backup/collector.go | 30 ++++--------------- .../backup/collector_privilege_sensitive.go | 7 +++-- .../collector_privilege_sensitive_test.go | 4 +-- 5 files changed, 15 insertions(+), 32 deletions(-) diff --git a/docs/CLI_REFERENCE.md b/docs/CLI_REFERENCE.md index d32a00d..d69cdc1 100644 --- a/docs/CLI_REFERENCE.md +++ b/docs/CLI_REFERENCE.md @@ -510,7 +510,7 @@ Some log lines use a label to make the output easier to scan: - A feature is disabled by configuration. - A non-critical CLI tool is not installed. - Running in an **unprivileged container/rootless** environment where low-level inventory commands are expected to fail (for example `dmidecode` or `blkid`). In this case, ProxSave still attempts the collection, but logs a `SKIP` (not a `WARNING`) when the failure matches known “missing privileges” patterns. - - For `blkid`, the skip reason also includes a restore hint: automated `/etc/fstab` device remap (UUID/PARTUUID/LABEL) may be limited. + - For `blkid`, the skip reason also includes a restore hint: `/etc/fstab` remap may be limited. ### Flag Reference diff --git a/docs/TROUBLESHOOTING.md b/docs/TROUBLESHOOTING.md index 49ac807..4158f24 100644 --- a/docs/TROUBLESHOOTING.md +++ b/docs/TROUBLESHOOTING.md @@ -172,8 +172,8 @@ COMPRESSION_TYPE=xz # Valid: xz, zstd, gzip, bzip2, lz4 **Symptoms**: - Running ProxSave inside an **unprivileged** LXC container (or a rootless container) produces log lines like: - - `SKIP Skipping Hardware DMI information: command \`dmidecode\` failed (...). Expected in unprivileged containers (...) (DMI tables not accessible). Non-critical; backup continues.` - - `SKIP Skipping Block device identifiers (blkid): command \`blkid\` failed (...). Expected in unprivileged containers (...) (block devices not accessible; restore hint: automated fstab device remap (UUID/PARTUUID/LABEL) may be limited). Non-critical; backup continues.` + - `SKIP Skipping Hardware DMI information: DMI tables not accessible (Expected in unprivileged containers).` + - `SKIP Skipping Block device identifiers (blkid): block devices not accessible (restore hint: fstab remap may be limited) (Expected in unprivileged containers).` **Cause**: In unprivileged containers, access to low-level system interfaces is intentionally restricted (for example `/dev/mem` and most block devices). Some inventory commands can fail even though the backup itself is working correctly. diff --git a/internal/backup/collector.go b/internal/backup/collector.go index b9c0c73..c34f02b 100644 --- a/internal/backup/collector.go +++ b/internal/backup/collector.go @@ -943,18 +943,9 @@ func (c *Collector) safeCmdOutput(ctx context.Context, cmd, output, description if ctxInfo.Detected && reason != "" { c.logger.Debug("Downgrading WARNING->SKIP: description=%q cmd=%q exitCode=%d", description, cmdString, exitCode) - details := strings.TrimSpace(ctxInfo.Details) - if details != "" { - details = " (" + details + ")" - } - c.logger.Skip("Skipping %s: command `%s` failed (%v). Expected in unprivileged containers%s (%s). Non-critical; backup continues.", - description, - cmdString, - err, - details, - reason, - ) - c.logger.Debug("Skip details for %s: output: %s", description, summarizeCommandOutputText(outputText)) + c.logger.Skip("Skipping %s: %s (Expected in unprivileged containers).", description, reason) + c.logger.Debug("SKIP context (privilege-sensitive): description=%q cmd=%q exitCode=%d err=%v unprivilegedDetails=%q", description, cmdString, exitCode, err, strings.TrimSpace(ctxInfo.Details)) + c.logger.Debug("SKIP output summary for %s: %s", description, summarizeCommandOutputText(outputText)) return nil } @@ -1302,18 +1293,9 @@ func (c *Collector) captureCommandOutput(ctx context.Context, cmd, output, descr if ctxInfo.Detected && reason != "" { c.logger.Debug("Downgrading WARNING->SKIP: description=%q cmd=%q exitCode=%d", description, cmdString, exitCode) - details := strings.TrimSpace(ctxInfo.Details) - if details != "" { - details = " (" + details + ")" - } - c.logger.Skip("Skipping %s: command `%s` failed (%v). Expected in unprivileged containers%s (%s). Non-critical; backup continues.", - description, - cmdString, - err, - details, - reason, - ) - c.logger.Debug("Skip details for %s: output: %s", description, summarizeCommandOutputText(outputText)) + c.logger.Skip("Skipping %s: %s (Expected in unprivileged containers).", description, reason) + c.logger.Debug("SKIP context (privilege-sensitive): description=%q cmd=%q exitCode=%d err=%v unprivilegedDetails=%q", description, cmdString, exitCode, err, strings.TrimSpace(ctxInfo.Details)) + c.logger.Debug("SKIP output summary for %s: %s", description, summarizeCommandOutputText(outputText)) return nil, nil } diff --git a/internal/backup/collector_privilege_sensitive.go b/internal/backup/collector_privilege_sensitive.go index 0ce39b5..1cb48be 100644 --- a/internal/backup/collector_privilege_sensitive.go +++ b/internal/backup/collector_privilege_sensitive.go @@ -41,20 +41,21 @@ func privilegeSensitiveFailureMatch(command string, exitCode int, outputText str return privilegeSensitiveMatch{} case "blkid": // In unprivileged LXC, blkid often exits 2 with empty output when block devices are not accessible. + const blkidReason = "block devices not accessible (restore hint: fstab remap may be limited)" switch { case exitCode == 2 && outputText == "": return privilegeSensitiveMatch{ - Reason: "block devices not accessible; restore hint: automated fstab device remap (UUID/PARTUUID/LABEL) may be limited", + Reason: blkidReason, Match: "exit=2 and empty output", } case strings.Contains(lower, "permission denied"): return privilegeSensitiveMatch{ - Reason: "block devices not accessible; restore hint: automated fstab device remap (UUID/PARTUUID/LABEL) may be limited", + Reason: blkidReason, Match: "stderr contains permission denied", } case strings.Contains(lower, "operation not permitted"): return privilegeSensitiveMatch{ - Reason: "block devices not accessible; restore hint: automated fstab device remap (UUID/PARTUUID/LABEL) may be limited", + Reason: blkidReason, Match: "stderr contains operation not permitted", } } diff --git a/internal/backup/collector_privilege_sensitive_test.go b/internal/backup/collector_privilege_sensitive_test.go index 13b846e..6e65e26 100644 --- a/internal/backup/collector_privilege_sensitive_test.go +++ b/internal/backup/collector_privilege_sensitive_test.go @@ -91,7 +91,7 @@ func TestCaptureCommandOutput_Unprivileged_DowngradesBlkidToSkipWithRestoreHint( if !strings.Contains(logText, "Expected in unprivileged containers") { t.Fatalf("expected unprivileged hint in logs, got: %s", logText) } - if !strings.Contains(logText, "restore hint: automated fstab device remap") { + if !strings.Contains(logText, "restore hint: fstab remap may be limited") { t.Fatalf("expected restore hint in logs, got: %s", logText) } } @@ -177,7 +177,7 @@ func TestSafeCmdOutput_Unprivileged_DowngradesSmartctlToSkip(t *testing.T) { } func TestPrivilegeSensitiveFailureReason(t *testing.T) { - const blkidReason = "block devices not accessible; restore hint: automated fstab device remap (UUID/PARTUUID/LABEL) may be limited" + const blkidReason = "block devices not accessible (restore hint: fstab remap may be limited)" tests := []struct { name string