-
-
Notifications
You must be signed in to change notification settings - Fork 68
Sync dev to main #159
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Sync dev to main #159
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -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,47 @@ 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) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| 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]) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
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]) | |
| } | |
| // 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 | |
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,89 @@ | ||
| 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. | ||
| const blkidReason = "block devices not accessible (restore hint: fstab remap may be limited)" | ||
| switch { | ||
| case exitCode == 2 && outputText == "": | ||
| return privilegeSensitiveMatch{ | ||
| Reason: blkidReason, | ||
| Match: "exit=2 and empty output", | ||
| } | ||
| case strings.Contains(lower, "permission denied"): | ||
| return privilegeSensitiveMatch{ | ||
| Reason: blkidReason, | ||
| Match: "stderr contains permission denied", | ||
| } | ||
| case strings.Contains(lower, "operation not permitted"): | ||
| return privilegeSensitiveMatch{ | ||
| Reason: blkidReason, | ||
| 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 | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
exitCodeis set to -1 whenerris not an*exec.ExitError(e.g., context cancellation/timeout). In that case,privilegeSensitiveFailureMatch()can still match dmidecode viaexitCode != 0 && outputText == "", causing timeouts/cancellations to be incorrectly downgraded toSKIP. Consider passing an additional flag indicating whether the exit code is known, or only applying theexit!=0 && empty outputheuristic whenerrors.As(err, *exec.ExitError)succeeds (or whenexitCode >= 0).