Skip to content

Commit 90dfba1

Browse files
Sync dev to main (#171)
* test(orchestrator): maximize coverage for network_apply - 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. * ci: bump the actions-updates group across 1 directory with 2 updates (#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> * backup/pve: clarify datastore skip logs 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. * feat(webhook): optional Discord content fallback for embed-only messages 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. * Revert "feat(webhook): optional Discord content fallback for embed-only messages" This reverts commit fc0aed4. * Update go.mod * Harden restore decisions against untrusted backup metadata 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. * Enforce staged backup checksum verification in restore/decrypt flows 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. * fix(orchestrator): preserve staged network symlinks 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. * fix(orchestrator): harden restore path validation against broken symlink 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. * fix(backup): sanitize datastore and storage path segments 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. * fix(io): bound timed fs and input goroutine buildup 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. * fix(orchestrator): preserve operational restore errors during path validation 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. * Separate PBS override scan roots from real datastore identities. - 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 * fix(orchestrator): resolve symlinked base dirs before validating relative 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. * fix(safefs): harden timeout test cleanup and limiter capture 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. * fix(input): preserve completed inflight reads across retries 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. * fix(pbs): disambiguate datastore output keys across CLI and overrides 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. * test(input): assert inflight cleanup after completed retry consumption 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. * fix(safefs): return TimeoutError when deadline expires before run 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 * fix(restore): validate compatibility before full restore fallback 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. * fix(orchestrator): bound restore metadata reads 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. * fix(orchestrator): validate staged network symlink targets 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. * fix(orchestrator): validate symlink targets in firewall staged sync 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. * fix(orchestrator): recognize full Proxmox system names 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. * fix(orchestrator): validate raw backup checksums during discovery 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. * test: add PBS datastore status filename collision regression coverage 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. * fix(pbs): always apply datastore path overrides without CLI discovery 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. * Normalize safefs deadline handling 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. * Surface decompressor close errors during restore archive inspection 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. * Fix symlink target rewriting for resolved destination parents 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. * Revalidate created overlay symlinks within restore root 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. * Normalize trimmed age encryption mode during bundle preparation 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. * Use a fresh timeout for rclone checksum inspection 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. * fix(orchestrator): fail safety restore on operational symlink validation 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. * Avoid holding the input state mutex across blocking waits on ctx.Done() 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 * backup: skip relative PBS datastore overrides 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. * backup: prefer CLI datastore path when merging inventory 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. * fix(orchestrator): bound .sha256 reads during backup discovery 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 * fix(orchestrator): surface rclone checksum failures after early close 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. * fix(orchestrator): validate prepared archive paths before decrypt/copy 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. --------- Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
1 parent b7d9670 commit 90dfba1

66 files changed

Lines changed: 8342 additions & 712 deletions

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

.github/workflows/release.yml

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -70,7 +70,7 @@ jobs:
7070
# GORELEASER
7171
########################################
7272
- name: Run GoReleaser
73-
uses: goreleaser/goreleaser-action@v6
73+
uses: goreleaser/goreleaser-action@v7
7474
with:
7575
version: latest
7676
workdir: ${{ github.workspace }}
@@ -82,6 +82,6 @@ jobs:
8282
# ATTESTAZIONE PROVENIENZA BUILD
8383
########################################
8484
- name: Attest Build Provenance
85-
uses: actions/attest-build-provenance@v3
85+
uses: actions/attest-build-provenance@v4
8686
with:
8787
subject-path: build/proxsave_*

Makefile

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
11
.PHONY: build test clean run build-release test-coverage lint fmt deps help coverage coverage-check
22

33
COVERAGE_THRESHOLD ?= 50.0
4+
TOOLCHAIN_FROM_MOD := $(shell awk '/^toolchain /{print $$2}' go.mod 2>/dev/null)
5+
COVER_GOTOOLCHAIN := $(if $(TOOLCHAIN_FROM_MOD),$(TOOLCHAIN_FROM_MOD)+auto,auto)
46

57
# Build del progetto
68
build:
@@ -60,20 +62,20 @@ test:
6062

6163
# Test con coverage
6264
test-coverage:
63-
go test -coverprofile=coverage.out ./...
64-
go tool cover -html=coverage.out
65+
GOTOOLCHAIN=$(COVER_GOTOOLCHAIN) go test -coverprofile=coverage.out ./...
66+
GOTOOLCHAIN=$(COVER_GOTOOLCHAIN) go tool cover -html=coverage.out
6567

6668
# Full coverage report (all packages)
6769
coverage:
6870
@echo "Running coverage across all packages..."
69-
@go test -coverpkg=./... -coverprofile=coverage.out ./...
70-
@go tool cover -func=coverage.out | tail -n 1
71+
@GOTOOLCHAIN=$(COVER_GOTOOLCHAIN) go test -coverpkg=./... -coverprofile=coverage.out ./...
72+
@GOTOOLCHAIN=$(COVER_GOTOOLCHAIN) go tool cover -func=coverage.out | tail -n 1
7173

7274
# Enforce minimum coverage threshold
7375
coverage-check:
7476
@echo "Running coverage check (threshold $(COVERAGE_THRESHOLD)% )..."
75-
@go test -coverpkg=./... -coverprofile=coverage.out ./...
76-
@total=$$(go tool cover -func=coverage.out | grep total: | awk '{print $$3}' | sed 's/%//'); \
77+
@GOTOOLCHAIN=$(COVER_GOTOOLCHAIN) go test -coverpkg=./... -coverprofile=coverage.out ./...
78+
@total=$$(GOTOOLCHAIN=$(COVER_GOTOOLCHAIN) go tool cover -func=coverage.out | grep total: | awk '{print $$3}' | sed 's/%//'); \
7779
echo "Total coverage: $$total%"; \
7880
if awk -v total="$$total" -v threshold="$(COVERAGE_THRESHOLD)" 'BEGIN { exit !(total+0 >= threshold+0) }'; then \
7981
echo "Coverage check passed."; \

docs/CONFIGURATION.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1053,6 +1053,8 @@ VZDUMP_CONFIG_PATH=/etc/vzdump.conf
10531053

10541054
# PBS datastore paths (comma/space separated)
10551055
PBS_DATASTORE_PATH= # e.g., "/mnt/pbs1,/mnt/pbs2"
1056+
# Extra filesystem scan roots for datastore/PXAR discovery; these do not create
1057+
# real PBS datastore definitions and may use path-derived output keys.
10561058

10571059
# System root override (testing/chroot)
10581060
SYSTEM_ROOT_PREFIX= # Optional alternate root for system collection. Empty or "/" = real root.

docs/RESTORE_GUIDE.md

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -117,7 +117,7 @@ API apply is automatic for supported PBS staged categories; ProxSave may fall ba
117117
|----------|------|-------------|-------|
118118
| `pbs_config` | PBS Config Export | **Export-only** copy of /etc/proxmox-backup (never written to system) | `./etc/proxmox-backup/` |
119119
| `pbs_host` | PBS Host & Integrations | **Staged** node settings, ACME, proxy, metric servers and traffic control (API/file apply) | `./etc/proxmox-backup/node.cfg`<br>`./etc/proxmox-backup/proxy.cfg`<br>`./etc/proxmox-backup/acme/accounts.cfg`<br>`./etc/proxmox-backup/acme/plugins.cfg`<br>`./etc/proxmox-backup/metricserver.cfg`<br>`./etc/proxmox-backup/traffic-control.cfg`<br>`./var/lib/proxsave-info/commands/pbs/node_config.json`<br>`./var/lib/proxsave-info/commands/pbs/acme_accounts.json`<br>`./var/lib/proxsave-info/commands/pbs/acme_plugins.json`<br>`./var/lib/proxsave-info/commands/pbs/acme_account_*_info.json`<br>`./var/lib/proxsave-info/commands/pbs/acme_plugin_*_config.json`<br>`./var/lib/proxsave-info/commands/pbs/traffic_control.json` |
120-
| `datastore_pbs` | PBS Datastore Configuration | **Staged** datastore definitions (incl. S3 endpoints) (API/file apply) | `./etc/proxmox-backup/datastore.cfg`<br>`./etc/proxmox-backup/s3.cfg`<br>`./var/lib/proxsave-info/commands/pbs/datastore_list.json`<br>`./var/lib/proxsave-info/commands/pbs/datastore_*_status.json`<br>`./var/lib/proxsave-info/commands/pbs/s3_endpoints.json`<br>`./var/lib/proxsave-info/commands/pbs/s3_endpoint_*_buckets.json`<br>`./var/lib/proxsave-info/commands/pbs/pbs_datastore_inventory.json` |
120+
| `datastore_pbs` | PBS Datastore Configuration | **Staged** datastore definitions (incl. S3 endpoints) (API/file apply) | `./etc/proxmox-backup/datastore.cfg`<br>`./etc/proxmox-backup/s3.cfg`<br>`./var/lib/proxsave-info/commands/pbs/datastore_list.json`<br>`./var/lib/proxsave-info/commands/pbs/datastore_*_status.json`<br>`./var/lib/proxsave-info/commands/pbs/s3_endpoints.json`<br>`./var/lib/proxsave-info/commands/pbs/s3_endpoint_*_buckets.json`<br>`./var/lib/proxsave-info/commands/pbs/pbs_datastore_inventory.json`<br>Note: `PBS_DATASTORE_PATH` override scan roots are inventory context only and are not recreated as datastore definitions during restore. |
121121
| `maintenance_pbs` | PBS Maintenance | Maintenance settings | `./etc/proxmox-backup/maintenance.cfg` |
122122
| `pbs_jobs` | PBS Jobs | **Staged** sync/verify/prune jobs (API/file apply) | `./etc/proxmox-backup/sync.cfg`<br>`./etc/proxmox-backup/verification.cfg`<br>`./etc/proxmox-backup/prune.cfg`<br>`./var/lib/proxsave-info/commands/pbs/sync_jobs.json`<br>`./var/lib/proxsave-info/commands/pbs/verification_jobs.json`<br>`./var/lib/proxsave-info/commands/pbs/prune_jobs.json`<br>`./var/lib/proxsave-info/commands/pbs/gc_jobs.json` |
123123
| `pbs_remotes` | PBS Remotes | **Staged** remotes for sync/verify (may include credentials) (API/file apply) | `./etc/proxmox-backup/remote.cfg`<br>`./var/lib/proxsave-info/commands/pbs/remote_list.json` |
@@ -2384,6 +2384,7 @@ systemctl restart proxmox-backup proxmox-backup-proxy
23842384
**Restore behavior**:
23852385
- ProxSave detects this condition during staged apply.
23862386
- If `var/lib/proxsave-info/commands/pbs/pbs_datastore_inventory.json` is available in the backup, ProxSave will use its embedded snapshot of the original `datastore.cfg` to recover a valid configuration.
2387+
- Inventory entries that came only from `PBS_DATASTORE_PATH` scan roots are treated as diagnostic context and are excluded from regenerated `datastore.cfg`.
23872388
- If recovery is not possible, ProxSave will **leave the existing** `/etc/proxmox-backup/datastore.cfg` unchanged to avoid breaking PBS.
23882389

23892390
**Manual diagnosis**:

go.mod

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ module github.com/tis24dev/proxsave
22

33
go 1.25
44

5-
toolchain go1.25.7
5+
toolchain go1.25.8
66

77
require (
88
filippo.io/age v1.3.1

internal/backup/checksum.go

Lines changed: 33 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,30 @@ type Manifest struct {
3535
ClusterMode string `json:"cluster_mode,omitempty"`
3636
}
3737

38+
// NormalizeChecksum validates and normalizes a SHA256 checksum string.
39+
func NormalizeChecksum(value string) (string, error) {
40+
checksum := strings.ToLower(strings.TrimSpace(value))
41+
if checksum == "" {
42+
return "", fmt.Errorf("checksum is empty")
43+
}
44+
if len(checksum) != sha256.Size*2 {
45+
return "", fmt.Errorf("checksum must be %d hex characters, got %d", sha256.Size*2, len(checksum))
46+
}
47+
if _, err := hex.DecodeString(checksum); err != nil {
48+
return "", fmt.Errorf("checksum is not valid hex: %w", err)
49+
}
50+
return checksum, nil
51+
}
52+
53+
// ParseChecksumData extracts a SHA256 checksum from checksum file contents.
54+
func ParseChecksumData(data []byte) (string, error) {
55+
fields := strings.Fields(string(data))
56+
if len(fields) == 0 {
57+
return "", fmt.Errorf("checksum file is empty")
58+
}
59+
return NormalizeChecksum(fields[0])
60+
}
61+
3862
// GenerateChecksum calculates SHA256 checksum of a file
3963
func GenerateChecksum(ctx context.Context, logger *logging.Logger, filePath string) (string, error) {
4064
logger.Debug("Generating SHA256 checksum for: %s", filePath)
@@ -105,16 +129,21 @@ func CreateManifest(ctx context.Context, logger *logging.Logger, manifest *Manif
105129
func VerifyChecksum(ctx context.Context, logger *logging.Logger, filePath, expectedChecksum string) (bool, error) {
106130
logger.Debug("Verifying checksum for: %s", filePath)
107131

132+
normalizedExpected, err := NormalizeChecksum(expectedChecksum)
133+
if err != nil {
134+
return false, fmt.Errorf("invalid expected checksum: %w", err)
135+
}
136+
108137
actualChecksum, err := GenerateChecksum(ctx, logger, filePath)
109138
if err != nil {
110139
return false, fmt.Errorf("failed to generate checksum: %w", err)
111140
}
112141

113-
matches := actualChecksum == expectedChecksum
142+
matches := actualChecksum == normalizedExpected
114143
if matches {
115144
logger.Debug("Checksum verification passed")
116145
} else {
117-
logger.Warning("Checksum mismatch! Expected: %s, Got: %s", expectedChecksum, actualChecksum)
146+
logger.Warning("Checksum mismatch! Expected: %s, Got: %s", normalizedExpected, actualChecksum)
118147
}
119148

120149
return matches, nil
@@ -205,9 +234,8 @@ func parseLegacyMetadata(scanner *bufio.Scanner, legacy *Manifest) {
205234
func loadLegacyChecksum(archivePath string, legacy *Manifest) {
206235
// Attempt to load checksum from legacy .sha256 file
207236
if shaData, err := os.ReadFile(archivePath + ".sha256"); err == nil {
208-
fields := strings.Fields(string(shaData))
209-
if len(fields) > 0 {
210-
legacy.SHA256 = fields[0]
237+
if checksum, parseErr := ParseChecksumData(shaData); parseErr == nil {
238+
legacy.SHA256 = checksum
211239
}
212240
}
213241
}

internal/backup/checksum_legacy_test.go

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,8 @@ func TestLoadLegacyManifestWithShaAndFallbackEncryption(t *testing.T) {
2828
t.Fatalf("write metadata: %v", err)
2929
}
3030

31-
shaLine := "deadbeef " + filepath.Base(archive) + "\n"
31+
expectedSHA := strings.Repeat("a", 64)
32+
shaLine := expectedSHA + " " + filepath.Base(archive) + "\n"
3233
if err := os.WriteFile(archive+".sha256", []byte(shaLine), 0o640); err != nil {
3334
t.Fatalf("write sha256: %v", err)
3435
}
@@ -50,8 +51,8 @@ func TestLoadLegacyManifestWithShaAndFallbackEncryption(t *testing.T) {
5051
if m.EncryptionMode != "plain" {
5152
t.Fatalf("expected fallback encryption mode plain, got %s", m.EncryptionMode)
5253
}
53-
if m.SHA256 != "deadbeef" {
54-
t.Fatalf("expected sha256 deadbeef, got %s", m.SHA256)
54+
if m.SHA256 != expectedSHA {
55+
t.Fatalf("expected sha256 %s, got %s", expectedSHA, m.SHA256)
5556
}
5657
if time.Since(m.CreatedAt) > time.Minute {
5758
t.Fatalf("unexpected CreatedAt too old: %v", m.CreatedAt)

internal/backup/checksum_test.go

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -145,8 +145,9 @@ ENCRYPTION_MODE=age
145145
if err := os.WriteFile(metadataPath, []byte(metadata), 0644); err != nil {
146146
t.Fatalf("failed to write metadata: %v", err)
147147
}
148+
expectedSHA := "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"
148149
shaPath := archivePath + ".sha256"
149-
if err := os.WriteFile(shaPath, []byte("deadbeef "+filepath.Base(archivePath)), 0644); err != nil {
150+
if err := os.WriteFile(shaPath, []byte(expectedSHA+" "+filepath.Base(archivePath)), 0644); err != nil {
150151
t.Fatalf("failed to write sha file: %v", err)
151152
}
152153

@@ -163,7 +164,7 @@ ENCRYPTION_MODE=age
163164
if manifest.Hostname != "legacy-host" || manifest.ScriptVersion != "legacy-1.0" {
164165
t.Fatalf("legacy metadata not parsed correctly: %+v", manifest)
165166
}
166-
if manifest.SHA256 != "deadbeef" {
167+
if manifest.SHA256 != expectedSHA {
167168
t.Fatalf("expected SHA256 from sidecar, got %q", manifest.SHA256)
168169
}
169170
if manifest.EncryptionMode != "age" {

internal/backup/collector.go

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,8 @@ package backup
33
import (
44
"bytes"
55
"context"
6+
"crypto/sha256"
7+
"encoding/hex"
68
"errors"
79
"fmt"
810
"io"
@@ -1180,6 +1182,21 @@ func sanitizeFilename(name string) string {
11801182
return clean
11811183
}
11821184

1185+
func collectorPathKey(name string) string {
1186+
trimmed := strings.TrimSpace(name)
1187+
if trimmed == "" {
1188+
return "entry"
1189+
}
1190+
1191+
safe := sanitizeFilename(trimmed)
1192+
if safe == trimmed {
1193+
return safe
1194+
}
1195+
1196+
sum := sha256.Sum256([]byte(trimmed))
1197+
return fmt.Sprintf("%s_%s", safe, hex.EncodeToString(sum[:4]))
1198+
}
1199+
11831200
// GetStats returns current collection statistics
11841201
func (c *Collector) GetStats() *CollectionStats {
11851202
c.statsMu.Lock()

internal/backup/collector_helpers_extra_test.go

Lines changed: 26 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,9 @@
11
package backup
22

3-
import "testing"
3+
import (
4+
"strings"
5+
"testing"
6+
)
47

58
func TestSummarizeCommandOutputText(t *testing.T) {
69
if got := summarizeCommandOutputText(""); got != "(no stdout/stderr)" {
@@ -38,3 +41,25 @@ func TestSanitizeFilenameExtra(t *testing.T) {
3841
}
3942
}
4043
}
44+
45+
func TestCollectorPathKey(t *testing.T) {
46+
if got := collectorPathKey("store1"); got != "store1" {
47+
t.Fatalf("collectorPathKey(store1)=%q want %q", got, "store1")
48+
}
49+
50+
unsafe := "../evil"
51+
got := collectorPathKey(unsafe)
52+
if got == unsafe {
53+
t.Fatalf("collectorPathKey(%q) should not keep unsafe value", unsafe)
54+
}
55+
if got == sanitizeFilename(unsafe) {
56+
t.Fatalf("collectorPathKey(%q) should add a disambiguating suffix", unsafe)
57+
}
58+
if !strings.HasPrefix(got, "__evil") {
59+
t.Fatalf("collectorPathKey(%q)=%q should start with sanitized prefix", unsafe, got)
60+
}
61+
62+
if a, b := collectorPathKey("a/b"), collectorPathKey("a_b"); a == b {
63+
t.Fatalf("collectorPathKey should avoid collisions: %q == %q", a, b)
64+
}
65+
}

0 commit comments

Comments
 (0)