Skip to content

Comments

Sync dev to main#159

Merged
tis24dev merged 2 commits intomainfrom
dev
Feb 22, 2026
Merged

Sync dev to main#159
tis24dev merged 2 commits intomainfrom
dev

Conversation

@tis24dev
Copy link
Owner

  • Skip privilege-sensitive failures in rootless env
  • Simplify blkid unprivileged messages and logs

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.
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.
Copilot AI review requested due to automatic review settings February 22, 2026 16:41
@tis24dev tis24dev merged commit 99d227c into main Feb 22, 2026
11 checks passed
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds unprivileged/rootless environment detection and uses it to downgrade known “missing privileges” failures (e.g., dmidecode, blkid) from WARNING to SKIP, with supporting tests and user-facing documentation.

Changes:

  • Introduce unprivileged container detection via /proc/self/{uid_map,gid_map} and surface details for logging.
  • Add privilege-sensitive failure classification (allowlist + output/exit-code heuristics) and apply it in command collection paths.
  • Document the new SKIP ... Expected in unprivileged containers behavior and restore impact when blkid inventory is missing.

Reviewed changes

Copilot reviewed 11 out of 11 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
internal/environment/unprivileged.go Implements best-effort unprivileged/rootless detection via uid/gid maps and optional container hint.
internal/environment/unprivileged_test.go Adds unit tests for parsing id maps and detection behavior.
internal/backup/collector_deps.go Adds DI hook for unprivileged detection to support deterministic tests.
internal/backup/collector_unprivileged.go Adds cached (sync.Once) unprivileged detection integration for Collector.
internal/backup/collector_privilege_sensitive.go Adds allowlisted privilege-sensitive command failure matching.
internal/backup/collector_privilege_sensitive_test.go Adds tests ensuring certain failures are downgraded to SKIP in unprivileged envs.
internal/backup/collector.go Applies downgrade logic to safeCmdOutput and captureCommandOutput when in unprivileged context.
docs/TROUBLESHOOTING.md Documents the new SKIP behavior in unprivileged containers and how to verify uid/gid maps.
docs/RESTORE_TECHNICAL.md Notes restore limitations when block-device inventory is missing from unprivileged backups.
docs/RESTORE_GUIDE.md Adds a user-facing note about remap limitations for unprivileged/rootless backups.
docs/CLI_REFERENCE.md Documents log labels including SKIP and provides examples.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +936 to +938
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)
Copy link

Copilot AI Feb 22, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

exitCode is set to -1 when err is not an *exec.ExitError (e.g., context cancellation/timeout). In that case, privilegeSensitiveFailureMatch() can still match dmidecode via exitCode != 0 && outputText == "", causing timeouts/cancellations to be incorrectly downgraded to SKIP. Consider passing an additional flag indicating whether the exit code is known, or only applying the exit!=0 && empty output heuristic when errors.As(err, *exec.ExitError) succeeds (or when exitCode >= 0).

Suggested change
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)
if exitCode >= 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 classification skipped: unknown or unavailable exit code (command=%q exitCode=%d)", cmdParts[0], exitCode)
}

Copilot uses AI. Check for mistakes.
Comment on lines +1269 to +1290
}
exitCode := -1
var exitErr *exec.ExitError
if errors.As(err, &exitErr) {
exitCode = exitErr.ExitCode()
}
outputText := strings.TrimSpace(string(out))

c.logger.Debug("Non-critical command failed (captureCommandOutput): description=%q cmd=%q exitCode=%d err=%v", description, cmdString, exitCode, err)
c.logger.Debug("Non-critical command output summary (captureCommandOutput): %s", summarizeCommandOutputText(outputText))

ctxInfo := c.depDetectUnprivilegedContainer()
c.logger.Debug("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])
Copy link

Copilot AI Feb 22, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same downgrade issue as in safeCmdOutput: when err isn't an *exec.ExitError, exitCode remains -1 and dmidecode can match the exit!=0 && empty output fallback, incorrectly downgrading context deadline/cancel (and other non-exit errors) to SKIP. Gate the exit-code-based heuristics on having a known exit code (or on *exec.ExitError).

Copilot uses AI. Check for mistakes.
Comment on lines +930 to +955
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)

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
}

if ctxInfo.Detected {
c.logger.Debug("No privilege-sensitive downgrade applied: command=%q did not match known patterns; emitting WARNING", cmdParts[0])
}

Copy link

Copilot AI Feb 22, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The privilege-sensitive downgrade/logging block is duplicated in both safeCmdOutput and captureCommandOutput (context detection, matching, SKIP logging). This duplication increases the risk of the two paths drifting (e.g., new patterns or fixes applied to only one). Consider extracting a small helper that takes (command, exitCode, outputText, description, err) and returns whether to downgrade + the reason/match.

Suggested change
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)
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
}
if ctxInfo.Detected {
c.logger.Debug("No privilege-sensitive downgrade applied: command=%q did not match known patterns; emitting WARNING", cmdParts[0])
}
// Centralized privilege-sensitive downgrade evaluation.
shouldDowngrade, reason, unprivDetails := c.evaluatePrivilegeSensitiveDowngrade(
cmdParts[0],
exitCode,
outputText,
description,
err,
"safeCmdOutput",
)
if shouldDowngrade {
c.logger.Debug("Downgrading WARNING->SKIP: description=%q cmd=%q exitCode=%d", description, cmdString, exitCode)
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, unprivDetails)
c.logger.Debug("SKIP output summary for %s: %s", description, summarizeCommandOutputText(outputText))
return nil
}

Copilot uses AI. Check for mistakes.
@codecov
Copy link

codecov bot commented Feb 22, 2026

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant