Conversation
- Add network_apply_additional_test.go with extensive tests for rollback (arm/disarm), NIC repair CLI (overrides/conflicts), snapshot IP parsing, command selection, and error paths. - Use FakeFS/FakeCommandRunner plus PATH stubs to deterministically exercise both success and failure branches. - Bring network_apply.go coverage to ~99% with no production code changes.
…164) Bumps the actions-updates group with 2 updates in the / directory: [goreleaser/goreleaser-action](https://github.com/goreleaser/goreleaser-action) and [actions/attest-build-provenance](https://github.com/actions/attest-build-provenance). Updates `goreleaser/goreleaser-action` from 6 to 7 - [Release notes](https://github.com/goreleaser/goreleaser-action/releases) - [Commits](goreleaser/goreleaser-action@v6...v7) Updates `actions/attest-build-provenance` from 3 to 4 - [Release notes](https://github.com/actions/attest-build-provenance/releases) - [Changelog](https://github.com/actions/attest-build-provenance/blob/main/RELEASE.md) - [Commits](actions/attest-build-provenance@v3...v4) --- updated-dependencies: - dependency-name: goreleaser/goreleaser-action dependency-version: '7' dependency-type: direct:production update-type: version-update:semver-major dependency-group: actions-updates - dependency-name: actions/attest-build-provenance dependency-version: '4' dependency-type: direct:production update-type: version-update:semver-major dependency-group: actions-updates ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Make PVE datastore skip messages user-friendly: log disabled storages as SKIP, offline storages as actionable WARNING, and always emit DEBUG skip details with active/enabled/status and an explicit reason. Fix detected datastores report header to match output column order. Extend unit tests to cover runtime flag parsing and the new skip logging, and make Makefile coverage targets honor the go.mod toolchain via GOTOOLCHAIN.
Adds per-endpoint opt-in Discord content text (WEBHOOK_<ENDPOINT>_DISCORD_CONTENT_ENABLED/..._DISCORD_CONTENT) to avoid “empty” messages when embeds aren’t rendered. Default remains unchanged (embed-only). Includes 2000-char truncation, docs/template updates, and tests.
…ly messages" This reverts commit fc0aed4.
Derive restore compatibility, cluster gating, and hostname warnings from archive-backed facts instead of candidate manifest metadata. Add restore archive analysis for internal backup metadata and category-based fallback, update restore planning and compatibility checks to consume trusted decision inputs, and add regression tests covering manifest spoofing for backup type and cluster mode.
Require checksum verification for staged backup archives before any restore or decrypt operation proceeds. Add strict SHA256 parsing and normalization, enforce checksum source agreement between sidecar and manifest metadata, reject raw backup candidates with no verifiable integrity source, centralize shared prepare-path verification for bundle and raw inputs, preserve source artifact checksum separately from the prepared plain archive checksum, and update regression tests to cover missing, mismatched, and legacy checksum scenarios.
Use Lstat in network staged apply so symlink entries are handled as symlinks instead of being dereferenced and copied as regular file contents. Reject symlinked staged network directories, extend the FS abstraction with Lstat, and add regression tests covering preserved symlinks and invalid symlinked stage roots.
…ink escapes Replace the restore path safety checks with a shared FS-aware resolver that walks paths component by component using Lstat and Readlink, correctly handling missing tails and rejecting intermediate symlink escapes. Apply the new validation to regular restore, safety restore, symlink and hardlink handling, and add regression tests for broken escaping symlinks, symlink loops, and symlinked restore roots.
Introduce a shared path-safe key for PBS datastore names and PVE storage names when building collector output paths, preventing path traversal and filename collisions from raw config values. Keep raw names unchanged in metadata and command invocations, update all affected PBS/PVE collector call sites, and add regression tests covering unsafe names while preserving the existing layout for already-safe names.
Prevent unbounded goroutine accumulation in timeout/cancel wrappers by limiting in-flight safefs operations and reusing a single in-flight read per reader/file descriptor in the input package. Keep public APIs unchanged, preserve current timeout semantics, and add regression coverage for limiter saturation, repeated timeouts, and race-safe cleanup of timed operations.
…lidation Teach the restore path resolver to distinguish security violations from local operational failures, so only real root-escape conditions are reported as illegal paths. Keep broken-symlink and traversal hardening intact, allow in-root permission and ENOTDIR failures to surface through the normal restore flow, and add regression tests covering the new security vs operational error classification.
- add explicit PBS datastore metadata for source, CLI identity and output key - derive stable path-based output keys for PBS_DATASTORE_PATH overrides - keep existing output names for real CLI-discovered PBS datastores - skip datastore show/status CLI calls for override-only entries - use filesystem-only namespace discovery for override paths - keep PXAR outputs isolated for colliding override basenames - preserve distinct override entries in PBS datastore inventory - exclude override-only inventory entries from datastore.cfg fallback restore - add regression coverage for basename collisions and restore safety - update PBS_DATASTORE_PATH documentation to reflect scan-root semantics
…tive links resolvePathRelativeToBaseWithinRootFS now canonicalizes baseDir by resolving existing symlinks before validating relative link targets. This closes the bypass where an archive could create an apparently harmless parent symlink and then install a relative symlink that escapes destRoot once materialized by the kernel.
Make safefs timeout tests failure-safe by draining blocked workers from cleanup and separating worker cleanup from global state restore. Also capture limiter and fs operation hooks locally in runLimited/Stat/ReadDir/Statfs to avoid races with global resets while async work is still in flight.
Keep completed in-flight line and password reads attached to their state until a caller consumes the buffered result, instead of clearing the inflight state from the producer goroutine. This fixes the race where a read could complete after a timeout, be cleaned up before the next retry started, and leave the completed input unreachable. Add deterministic tests for both retry-while-pending and completion-before-retry cases to lock the behavior down.
Add a PBS-specific output-key resolver to guarantee stable, unique datastore filenames and directories across auto-detected datastores and PBS_DATASTORE_PATH overrides. This fixes real collisions between CLI datastore names and path-derived override keys, makes pbsDatastore.pathKey() source-aware when OutputKey is unset, and keeps inventory output_key values aligned with the actual files written by PBS collectors. Includes regression tests for CLI-vs-override collisions, override fallback behavior, and inventory consistency.
The new tests verify that line and password inflight reads remain attached after timeout until the completed result is reused, and that state.inflight is cleared immediately after that consumer receives the result.
update runLimited to re-check ctx.Err() after effectiveTimeout() avoid calling run() when effectiveTimeout() returns 0 because the context deadline already elapsed preserve the no-timeout path only for genuinely disabled timeouts add a regression test for the expired-deadline edge case in internal/safefs
When restore archive analysis fails in the UI workflow, do not jump immediately to full restore. Build a best-effort RestoreDecisionInfo from the manifest, run the existing compatibility check first, and only then fall back to runFullRestoreWithUI. Also add a regression test covering the analysis-error path to ensure the compatibility warning is still shown before the full restore fallback executes.
Reject oversized backup_metadata.txt entries during restore archive inspection by reading through a fixed limit before parsing. This prevents unbounded memory allocation from crafted archives and ensures truncated metadata caused by limit overflow is treated as invalid rather than partially trusted. Add regression tests covering oversized metadata rejection and the public restore analysis path.
Validate symlink targets during staged network apply before recreating them under the destination tree. Reuse the existing path-security resolver to ensure targets stay within the allowed destination root, rewrite safe absolute targets to relative links, and reject absolute or relative targets that escape the subtree. Add regression tests for safe rewrites and escaping symlink targets.
Validate symlink targets in syncDirExact before recreating them under the destination subtree. Reuse the existing path-security resolver to ensure firewall and SDN staged symlinks stay within the allowed destination root, rewrite safe absolute targets to relative links, and reject absolute or relative targets that escape the subtree. Add regression tests for safe rewrites and escaping symlink targets.
Verify and fix parseSystemTypeString in compatibility.go. The parser now recognizes space-separated and full-name variants such as "Proxmox VE", "Proxmox Backup", and "Proxmox Backup Server", in addition to the existing acronym and hyphenated forms. Add regression tests to ensure these values map correctly to SystemTypePVE and SystemTypePBS.
Validate and normalize manifest and sidecar SHA256 values before accepting local or rclone raw backup candidates. Reject candidates when the manifest checksum is malformed, the .sha256 sidecar cannot be parsed, or both checksum sources disagree. Persist the normalized checksum and its source on the candidate so downstream staged integrity verification only works with pre-validated values. Add coverage for local and rclone discovery, malformed and conflicting checksums, and reuse of the candidate integrity expectation during staged verification.
Adds a regression test for collectPBSCommands() covering colliding PBS datastore output keys. The test verifies that assignUniquePBSDatastoreOutputKeys() disambiguates colliding datastore status filenames and that two distinct datastore_*_status.json files are written with the correct datastore-specific content.
Make PBS datastore enumeration best-effort in getDatastoreList so configured PBSDatastorePaths overrides are still appended when proxmox-backup-manager is missing, the datastore list command fails, or its JSON output is invalid. Preserve context cancellation as a fatal error, keep unique output key assignment, and add regression tests for missing CLI, command failure, and parse failure fallback cases.
Add focused safefs tests for expired-deadline paths and make deadline-driven context cancellations consistently map to ErrTimeout/TimeoutError in runLimited and operationLimiter.acquire. This fixes inconsistent timeout classification where some context deadline expirations returned context.DeadlineExceeded instead of the safefs timeout sentinel, while preserving context.Canceled behavior.
Make restore archive inspection fail when the decompression reader reports an error on Close, instead of silently accepting the archive as valid. This updates inspectRestoreArchiveContents to preserve existing inspect errors while surfacing deferred decompressor failures, and adds targeted tests covering both close-error propagation and error-precedence behavior.
Update overlay symlink validation to rewrite absolute targets relative to the resolved destination parent instead of the lexical parent path. This fixes incorrect rewritten targets when the destination's parent directory is itself a symlink, and adds a regression test covering the symlinked-parent case.
Add post-creation symlink verification for network overlay and firewall sync paths, mirroring the existing restore symlink safety model. The new check reads back each created symlink, ensures its effective target still resolves within the destination root, removes the link on validation failure, and adds regression tests covering readback failures and post-create escape scenarios.
Trim manifest EncryptionMode before lowercasing in preparePlainBundleCommon so values like " age " still trigger decryption. This aligns decryption behavior with statusFromManifest and adds a regression test proving whitespace-padded age mode is decrypted instead of being misclassified as plain.
Stop reusing the raw-item metadata context for checksum probing in discoverRcloneBackups. The checksum fetch now gets its own per-command timeout budget, matching the documented RCLONE_TIMEOUT_CONNECTION behavior, and a regression test covers the case where a slow metadata read previously left too little time for the checksum fetch.
…ion errors Distinguish path-security errors from operational failures during safety-backup symlink restore. Keep the existing warning-and-skip behavior for symlink targets that escape the restore root, but propagate non-security errors from resolvePathRelativeToBaseWithinRootFS so restore failures are reported instead of silently dropping symlinks. Add tests covering operational failures before and after symlink creation.
…() and inflight completion. - lock only to create/read the current inflight operation - release the mutex before waiting on cancellation or read completion - re-acquire the mutex only to clear state.inflight if it still matches the captured inflight - store completed line/password results on the inflight state and use done as a broadcast completion signal for concurrent waiters - add regression tests for concurrent cancelled callers and preserve existing retry/inflight reuse behavior
Reject relative PBS_DATASTORE_PATH values when building the datastore list. normalizePBSDatastorePath only cleans input, so relative paths could previously be accepted as overrides. Keep the existing override naming, output key, and deduplication logic for valid absolute paths, and log a warning when skipping an invalid relative override. Add a regression test covering relative PBS_DATASTORE_PATH entries.
Fix PBS datastore inventory merge so the runtime CLI path overrides datastore.cfg for the same datastore identity. Add a regression test covering config-vs-CLI path precedence during merge.
Replace unbounded checksum sidecar reads in backup discovery with bounded streaming reads for both local files and rclone sources. - cap checksum input to 4 KiB before parsing - remove full-file ReadFile / CombinedOutput usage on .sha256 paths - return clear errors for oversized or empty checksum files - preserve compatibility with valid bounded checksum files without trailing newline - add coverage for local and rclone oversized / bounded-input cases
Only ignore `rclone cat` wait errors when stdout was closed early after reading a valid first checksum line and stderr is empty. This prevents genuine rclone failures from being masked when the checksum is read successfully but the command still exits non-zero with an error message. Add a regression test for a fake rclone that prints a valid checksum line and then exits with code 1.
Validate the staged archive path against the manifest encryption mode in preparePlainBundleCommon before deriving the plain archive output path. This rejects inconsistent inputs where `EncryptionMode=age` does not have a `.age` archive suffix, or where a non-age archive still ends with `.age`. It also ensures decryption never runs with identical input and output paths by generating a unique output name when needed. Add regression coverage for both mode/suffix mismatch cases and the unique-path fallback for age archives.
Dependency ReviewThe following issues were found:
License Issues.github/workflows/release.yml
OpenSSF Scorecard
Scanned Files
|
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (66)
📝 WalkthroughWalkthroughThis pull request introduces comprehensive enhancements for backup collection, restoration, and integrity verification. Key additions include PBS datastore override support with deterministic output keying, path security validation for restore operations, archive integrity verification, restore decision logic based on archive analysis, improved checksum normalization, concurrency improvements for input handling and filesystem operations, and network configuration enhancements. The changes span collection, restoration, path safety, and orchestration layers. Changes
Sequence Diagram(s)sequenceDiagram
participant App as Application
participant Analyzer as Archive Analyzer
participant Tar as Tar Reader
participant Decision as Restore Decision
App->>Analyzer: AnalyzeRestoreArchive(archivePath)
Analyzer->>Tar: Open & decompress archive
Tar->>Tar: Read tar entries & collect paths
Tar->>Tar: Parse restore decision metadata
Analyzer->>Decision: buildRestoreDecisionInfo(metadata, categories)
Decision->>Decision: detectBackupTypeFromCategories()
Analyzer-->>App: RestoreDecisionInfo{BackupType, ClusterPayload, Hostname}
sequenceDiagram
participant UI as Restore UI
participant Decision as Decision Logic
participant Archive as Archive Analysis
participant Compat as Compatibility Check
UI->>Archive: AnalyzeRestoreArchive(archivePath)
Archive-->>Decision: availableCategories, decisionInfo
alt Analysis succeeds
Decision->>Compat: ValidateCompatibility(currentSystem, decisionInfo.BackupType)
else Analysis fails
Decision->>Decision: fallbackRestoreDecisionInfoFromManifest(manifest)
end
Compat-->>UI: Result (compatible/incompatible)
UI->>UI: Plan restore using decisionInfo.ClusterPayload
sequenceDiagram
participant Client as Restore Client
participant PathResolver as Path Resolver
participant FS as Filesystem
participant Validator as Validator
Client->>PathResolver: resolvePathWithinRootFS(destRoot, candidate)
loop Component traversal
PathResolver->>FS: Lstat(component)
alt Symlink detected
FS-->>PathResolver: FileInfo(symlink)
PathResolver->>FS: Readlink(component)
FS-->>PathResolver: target
Validator->>Validator: validateTarget within root
else Regular file/dir
FS-->>PathResolver: FileInfo(regular)
end
end
Validator-->>PathResolver: Validated absolute path
PathResolver-->>Client: Safe path or error
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Poem
✨ Finishing Touches
🧪 Generate unit tests (beta)
Comment |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit
New Features
Improvements
Documentation