Slice 6/6: tests + docs + upgrade tooling + dependabot bumps#263
Merged
Conversation
… prompt The guide walked operators into the destructive cluster-restore behavior without ever naming the mandatory safety prompt that governs it. Rewritten against the current restore engine, verified by two adversarial review rounds. - New section "Cluster restore modes: SAFE vs RECOVERY": the mandatory prompt shown whenever a PVE restore includes pve_cluster and the backup carries cluster data (FULL and STORAGE always reach it, CUSTOM only if selected, SYSTEM BASE never). SAFE applies exported configs to the running cluster via pvesh/pveum and never writes config.db, stops services, or unmounts /etc/pve; RECOVERY stops the four services, unmounts /etc/pve, restores config.db, and is only for an offline/isolated node. - Corrected the old "STORAGE only -> stop services / unmount /etc/pve / restart" story: that sequence is RECOVERY only, not a result of the STORAGE scope. Fixed Scenarios 1/2/4/5 to choose the right mode (RECOVERY when the node is offline, SAFE when it is up) instead of implying STORAGE restores config.db by itself. - Documented: automatic VM/CT source-node selection on a hostname mismatch, the offline-storage mount guards + `proxsave --cleanup-guards`, the armed 180s network/firewall/HA/access-control auto-rollback, the preserved safety backup and its rm hint, the real STORAGE category set, and that extraction preserves owner/mode from the archive. - During RECOVERY, /etc/pve is not written directly: the staged config categories skip their apply (config.db owns them) with a shadow-guard as a backstop; contents return from config.db on remount. - Dropped the unnecessary `cd /opt/proxsave` steps, fixed the fabricated per-service log transcript, and removed style tells.
…ainst the real pipeline The release trigger, workflow names, and provenance asset names had all drifted. Rewritten against the live workflows and .goreleaser.yml, verified by an adversarial review pass. RELEASE-PROCESS.md: - Real flow: push an unprotected pr-vX.Y.Z tag on dev HEAD -> release-intake.yml opens a "Release vX.Y.Z" dev->main PR with a release-tag body marker -> release-guard.yml validates the PR -> squash-merge -> post-merge-release.yml force-moves dev (--force-with-lease) and creates the authoritative vX.Y.Z tag once on the squash commit -> release.yml gates on reachable-from-main, validates the signing key before publishing, runs GoReleaser, signs SHA256SUMS, and attests provenance. - Removed the wrong "git tag vX.Y.Z on main" trigger and the "do not tag on dev" rule (the pr-v* trigger is pushed on dev); removed the nonexistent sync-dev.yml and autotag.yml references; kept the legacy-escape-hatch note. PROVENANCE_VERIFICATION.md: - State that only linux/amd64 is published and use the real underscore asset names (proxsave_X.Y.Z_linux_amd64); fixed every gh attestation example accordingly. - Deleted the fabricated "Migration from GPG" section and the GPG comparison table (no GPG scheme, SHA256SUMS.asc, or that key ever shipped); removed the macOS and Windows binary examples; refreshed stale version examples to a vX.Y.Z placeholder. - Kept the accurate ECDSA P-256 / pinned-key SHA256SUMS.sig section; noted the attestation subject-path build/proxsave_*.
…bsystem The stub framed the daemon as reporting "two independent checks" and omitted roughly half of what it does. Rewritten from a code map of the daemon + internal/health subsystem, verified by an adversarial review pass. - The four monitored checks with exact ping semantics: proxsave-alive (heartbeat), proxsave-backup (/start then the exit code, or /fail on a hang), proxsave-updates (/0 up to date, /1 available), and one proxsave-notify-<channel> per channel the run attempted. Corrected the old "/1 warning" claim (the backup finish ping is the exit code, not a fixed /1). - BACKUP_ENABLED=false skips the scheduled run while the heartbeat continues. - The SIGUSR1 standalone-backup handoff (the daemon is the sole pinger; a manual or dashboard run hands off its exit code, 15-minute stale drop). - --daemon-status output fields and exit-code semantics (0 only when running and aligned); the conditional version/alignment lines. - Binary alignment after an in-place upgrade (hash-free /proc/<pid>/exe) and the restart-and-verify that waits for an in-progress backup. - The systemd unit (Type=simple, --config pin, Restart=always, RestartSec=10), the five on-disk state files under identity/, centralized vs self URL resolution, and the full config-key set (adds the updates and eight notify keys the stub omitted; notes HEALTHCHECK_ENABLED is forced true by setup).
…d-vs-template defaults CONFIGURATION.md documented correct key names but omitted two config areas and presented several template values as if they were the compiled fallbacks. Verified against config.go by an adversarial review pass. - Add a Scheduler engine section (SCHEDULER_MODE/TIME, MAX_RUN_DURATION, DAEMON_OPT_OUT) and a Healthchecks connector section (the full HEALTHCHECK_* set including the updates and eight per-channel notify keys), both pointing to DAEMON.md. Note HEALTHCHECK_ENABLED is forced true when enabling the daemon, and a fresh install writes SCHEDULER_MODE=daemon. - Fix the config-file default: configs/backup.env resolves under the install directory (BASE_DIR), not the current directory; absolute --config is used as-is. - Distinguish compiled fallbacks from the shipped template values: compression 6/standard (template 9/ultra), RCLONE_TRANSFERS 4 (template 16), RCLONE_BANDWIDTH_LIMIT empty (template 10M), MIN_DISK_SPACE 10 GB with <=0 coerced and SECONDARY/CLOUD falling back to PRIMARY, MAX_*_BACKUPS 7/14/30 (template 15), GFS tiers all 0 (off until enabled, DAILY forced >=1). - Add the Telegram delivery-confirmation keys; use the canonical compression values (gz/bz2/zst) with the long names as aliases; note advanced and extreme both resolve to the debug log level. Remove curly quotes, em dashes, and a stray Unicode ellipsis.
…steps The install wizard now chooses a scheduler engine and a healthchecks mode, and --upgrade migrates cron installs to the daemon, but INSTALL.md still framed scheduling as a cron-only step. Verified against the install code. - Wizard: add the Scheduler engine step (ProxSave local daemon vs system cron; fresh installs and Overwrite default to the daemon, editing keeps the stored engine) and the Healthchecks step (daemon only: Off / ProxSave HC Server (centralized, default) / Your own server (self, with a ping-URL screen and a verification screen); dimmed and forced off with cron). Reframe the schedule as "Run at (HH:MM)", the daily time used by either engine. - Upgrade: drop the wrong "does NOT touch your cron schedule"; --upgrade migrates a cron install to the daemon unless opted out with --daemon-remove, and a daemon install stays on the daemon. Check the scheduler with proxsave --daemon-status. - Note install.sh accepts --cli for the text-mode installer; fix the config-path framing (from --config, resolved under BASE_DIR, not asked in the wizard); add a "Scheduling and the daemon" cross-link to DAEMON.md and the --daemon-* flags. - Remove decorative TOC emoji, a stray em dash, and the check/cross emoji list.
…lized bot relay New HIGH-create doc mapped against the notification code: the non-critical invariant (IsCritical false, adapter swallows Send errors, exit code frozen before dispatch), the two-tier delivery/monitoring split, dispatch order with Healthchecks last, the centralized Telegram relay (bot token stays on the host), TOFU relay-secret provisioning, the two-response delivery model (acceptance vs polled delivery), the portal magic-link sanitize boundary, Email/Gotify/Webhook channels, the serverbot transport contract, and the log-redaction rules. Indexed in docs/README.md.
RESTORE_TECHNICAL.md described superseded, less-safe mechanisms. Rewritten against internal/orchestrator: the atomic sibling-temp + rename extraction model (fchown/fchmod on the FD, .proxsave-tmp-* pattern) replacing the old os.Create+io.Copy-on-target snippets; the three-tier category split (normal/export/staged) replacing the two-pass framing; the real staged-apply order (PBS, PVE, PVE SDN, access-control, accounts, notifications) with the anti-lockout account merge and PBS raw-config skips; dedup symlink materialization (issue #70); the secure temp-root guard (#54) and registry orphan cleanup (#55) added to Safety Mechanisms; isRealRestoreFS on the root check; ZFS check corrected (category-gated, not PBS-only); restore_<ts>_<seq>.log log name. Style pass: removed em dashes, curly quotes, and table emoji.
DEVELOPER_GUIDE.md navigated by a stale map. Regenerated the internal/ package tree from the real layout (internal/tui -> internal/ui; added health, serverbot, safefs, safeexec, cron, installer, support, uitest, and others; removed the non-existent configs/ dir); noted the embedded config template and the Charm UI stack. Replaced the cobra dependency examples with real deps (filippo.io/age, stdlib flag); fixed the benchmark path (internal/backup, not internal/compression); completed the compression list; added make coverage / coverage-check and a section on the two structural lint guards (check-no-tview, check-serverbot-leaf); pointed lint at make lint. Trimmed decorative emoji. Indexed via existing links.
SECURITY.md was accurate but far too thin (58 lines, only the threat model and gosec bookkeeping). Expanded from the runtime security code with new sections: execution model (safeexec argv-only allowlist, no shell, self-exec validation, rclone sanitizers); the security preflight (executable integrity with symlink refusal + SameFile TOCTOU + <exe>.md5, permission/ownership enforcement that refuses to touch symlinks, private-key scan, network/process checks, abort semantics, and the real env-var defaults); secret redaction; terminal-escape scrubbing of untrusted display data; immutable identity secrets; bounded filesystem I/O (safefs); read-only bind guards. Softened the G304 note (a few legacy #nosec sites remain). Cross-linked NOTIFICATIONS/DAEMON/RESTORE_TECHNICAL.
… drift RESTORE_GUIDE.md and RESTORE_DIAGRAMS.md were substantively accurate on the engine but drifted on presentation. --restore/--decrypt default to a Charm TUI now (classic text prompts only under --cli), so added a framing note and relabeled the transcripts as the --cli experience. Fixed the category counts (11 PVE + 9 PBS + 12 Common = 32; PVE host 23, PBS host 21) and documented the missing 'accounts' Common category (passwd/group/shadow/gshadow/sudoers, safe merge preserving the current host root/system accounts). Replaced the nonexistent decryption sub-menu with the single key-or-passphrase field, documented the two-stage restore confirmation, and noted that STORAGE/BASE strip export-only categories. In DIAGRAMS: renamed Two-Pass to Three-Tier Extraction (added the staged-apply lane), dropped the passphrase/identity branch, and added accounts to CommonFull. Style pass: removed em dashes, curly quotes, and ellipses.
… bundle layout CLOUD_STORAGE.md was largely accurate but omitted several behaviors. Added an RCLONE_FLAGS row (whitespace-split, injected verbatim into every rclone command) and a BUNDLE_ASSOCIATED_FILES row (default true). Corrected CLOUD_REMOTE from 'required' to empty-by-default (cloud is silently disabled when the remote is blank, even with CLOUD_ENABLED=true). Documented RCLONE_TIMEOUT_OPERATION=0 as unbounded uploads while management/query ops use that value when > 0 else a 300s floor. Added a 'Cloud layout (bundle vs raw)' section (single .bundle.tar by default vs raw archive + sidecars, with .manifest.json as the authoritative raw metadata) and a 'How ProxSave invokes rclone' section (fixed subcommand allowlist, copyto not copy). Noted the legacy env-var aliases and that bundling is the default in the DR steps. Style pass: removed curly quotes.
… ownership ENCRYPTION.md was largely accurate but omitted a few enforced behaviors. Documented the passphrase-strength policy (min 12 chars, at least 3 of 4 character classes, a small weak-passphrase blocklist), the AGE_RECIPIENTS plural fallback alias (used only when AGE_RECIPIENT is empty), and that AGE_RECIPIENT_FILE is empty by default and resolved to <BASE_DIR>/identity/age/recipient.txt at runtime. Added the root:root ownership requirement (auto-fixed with AUTO_FIX_PERMISSIONS, else warned) to the permission notes, and expanded the salt section to the decrypt fallback chain (per-install v2 manifest salt first, then the proxsave and pre-rebrand proxmox-backup-go v1 namespaces). Style pass: removed an em dash and an ellipsis.
…th, support email; add exit codes Two HIGH plus reference gaps. Replaced the nonexistent lz4 compression value with the real set (gzip/bzip2/xz/lzma/zstd, also pigz; none). Removed the fabricated [DRY-RUN] output block and described what dry-run actually does. Corrected the lock file path (.backup.lock under LOCK_PATH, default <BASE_DIR>/lock/.backup.lock) and the support-mode email (the maintainer address is baked into the build, injected at build time, not the hardcoded github-support@tis24.it). Added an Exit Codes table (0-15 from types/exit_codes.go), including that a warnings-only run is promoted from 0 to exit 1 and errors to 4. Style pass: removed curly quotes and ellipses.
…itest TEST_STRATEGY.md was the only Italian doc while docs/README links it as English. Translated it to English. Updated the stale legacy-file count (347 legacy + 30 audited, was 238) and added a section on internal/uitest (test-only Charm driver helpers; uitest.Deadline scales render-poll timeouts by raceScale 8 under -race, 1 otherwise) plus the serial/no-CPU-saturation constraint for UI tests. Corrected the context reference: the pre-test audit file is diagnostics/coverage-audit-2026-06-09.md (dev @ 3222a30, the tests-audit-baseline commit); dropped the fabricated precoverage-bughunt filename and the unverifiable '26 bugs' count.
…fest vs metadata The three audit LOW imprecisions, verified against internal/backup/collector*.go. Only newDualRecipe() lives in collector_bricks.go; the per-role builders are in collector_bricks_pve.go/_pbs.go/_system.go. Documented that the PBS branch runs two recipes (newPBSRecipe then newPBSUserConfigRecipe, the per-user API-token pass seeded by the user_list.json snapshot). Split the manifest/metadata fields: the collector manifest.json carries only ProxmoxType/ProxmoxTargets and is an ExportOnly diagnostic not read by restore, while PVE_VERSION/PBS_VERSION are written by the orchestrator into the metadata sidecar that restore actually reads. Expanded Related Files with the split builder files.
…dd a daemon example Example 4 depicted the --newkey and --decrypt flows as numbered text menus; they are a Charm TUI by default (--cli forces text prompts). Reworded both: --newkey derives a recipient from a passphrase (min 12 chars, at least 3 character classes), and --decrypt selects a backup, takes the key/passphrase in a single field, and prompts for a destination directory (defaults to decrypt/ under BASE_DIR), writing <name>.decrypted.bundle.tar there. Added Example 11 for the resident daemon and healthchecks monitoring (--daemon-setup/--daemon-status/--daemon-remove, the systemd service, and the four monitored checks). Style pass: removed an ellipsis.
…rouping comment Resolves the two deferred dashboard-consistency items. Issue 3 (Status keyword casing): the shared Status: colored keyword was fed in three casings across sibling screens (ALL-CAPS, lowercase, Title). Standardized on ALL-CAPS at the caller sites: restartVerifyStatus, the daemon admin results, the cleanup-guards and update-config check screens, and the post-install audit. daemonStatusStyle is shared with the plain-text --daemon-status CLI line, so its dashboard screen is uppercased at the graphical consumer only, leaving the CLI/log readout in its natural case; installVerifyVerdict and summarizeRestartVerify (log/install-outcome lines) are likewise left natural. RenderStatusLevel is untouched (it must not blindly upcase embedded version tokens). Tests updated to the new casing. Issue 8 (menu enum comment): clarified that the Action enum's second group is a DISPATCH grouping (routed through runDashboardDiagnostic), not the menu layout, so ActionCheckUpgrade rendering under Maintenance is expected, not a mismatch. Build, vet, and the cmd/proxsave + install + menu tests are green.
Both are upgrade finalize-block bricks that had zero direct test coverage. Pin their observable behavior before extracting an upgradeFinalizePhase seam: - maybeAutoMigrateDaemon: the three gates that short-circuit before the systemd migration (already-daemon, opted-out, unreadable config). The real migration branch shells to systemctl and is left to integration coverage; absence of the pre-migration stdout marker proves the gate held. - installSupportDocs: every embedded doc written under the base dir at its Name (nested dirs included), byte-for-byte at mode 0600, plus the directory-creation error path.
Split runUpgrade into two phases so the local finalize can be driven on a binary that is already on disk: - upgradeAcquireBinary: download+install the latest release (default), or with --localfile return the on-disk binary and skip the release check + download entirely (a fetch would resolve the latest STABLE release and could pull a beta tester off their build). - upgradeFinalizePhase: the shared post-binary work (upgrade backup.env, refresh docs/symlinks, migrate/restart the daemon, fix permissions, footer). --localfile is a modifier of --upgrade (rejected on its own) and is the seam upgrade-beta.sh will use after it swaps in a verified beta binary. Behavior of plain --upgrade is unchanged. Covered by the maybeAutoMigrateDaemon and installSupportDocs characterization tests plus new tests for the localfile branch and the validation guard.
Twin of install.sh for beta/rc testers. The built-in --upgrade and install.sh
only see /releases/latest, which excludes prereleases, so betas are invisible
to them.
Resolution looks at the newest release OVERALL (/releases is newest-first, so
the first non-draft entry is it) and branches on its prerelease flag:
- newest is a beta -> show installed vs available, confirm, install it,
- newest is stable -> point the user at 'proxsave --upgrade' and stop, so a
month-old beta is never installed over a newer stable.
An explicit tag argument forces that tag (escape hatch) and skips the guards.
It downloads the archive + SHA256SUMS + .sig, verifies the signature (same
pinned ECDSA key) and checksum identically to install.sh, backs the current
binary up to .prev, swaps in the beta, sanity-checks it runs (rolling back on
failure), and finalizes with 'proxsave --upgrade --localfile' so the beta binary
does the local upgrade (backup.env keys, docs, daemon, permissions) without a
re-download. Confirmation is interactive (/dev/tty) unless -y is passed.
Hardening (adversarial review):
- on finalize failure, print the rollback path explicitly (set -e would
otherwise abort before the footer that documents .prev),
- guard the installed-version probe against set -e when a corrupt binary's
--version exits non-zero (the 'unknown' fallback was unreachable),
- refuse a publish-order downgrade in the auto path (sort -V): a back-ported
beta published after a newer stable is no longer offered.
The auto-detect downgrade guard referenced ${BETA_TAG#v} before BETA_TAG was
assigned (it was only set after the guard), so under 'set -euo pipefail' the
auto path aborted with 'BETA_TAG: unbound variable' before any download,
breaking 'sh upgrade-beta.sh' whenever a beta was available. Move the
BETA_TAG=${LATEST_TAG} assignment ahead of the guard.
…urity-patches group across 1 directory (#249) deps(deps): bump charm.land/bubbles/v2 Bumps the security-patches group with 1 update in the / directory: [charm.land/bubbles/v2](https://github.com/charmbracelet/bubbles). Updates `charm.land/bubbles/v2` from 2.1.0 to 2.1.1 - [Release notes](https://github.com/charmbracelet/bubbles/releases) - [Commits](charmbracelet/bubbles@v2.1.0...v2.1.1) --- updated-dependencies: - dependency-name: charm.land/bubbles/v2 dependency-version: 2.1.1 dependency-type: direct:production update-type: version-update:semver-patch dependency-group: security-patches ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
…250) Bumps the actions-updates group with 6 updates in the / directory: | Package | From | To | | --- | --- | --- | | [actions/setup-go](https://github.com/actions/setup-go) | `6.4.0` | `6.5.0` | | [github/codeql-action/init](https://github.com/github/codeql-action) | `4.36.2` | `4.37.0` | | [github/codeql-action/analyze](https://github.com/github/codeql-action) | `4.36.2` | `4.37.0` | | [goreleaser/goreleaser-action](https://github.com/goreleaser/goreleaser-action) | `7.2.2` | `7.2.3` | | [actions/attest-build-provenance](https://github.com/actions/attest-build-provenance) | `4.1.0` | `4.1.1` | | [github/codeql-action/upload-sarif](https://github.com/github/codeql-action) | `4.36.2` | `4.37.0` | Updates `actions/setup-go` from 6.4.0 to 6.5.0 - [Release notes](https://github.com/actions/setup-go/releases) - [Commits](actions/setup-go@4a36011...924ae3a) Updates `github/codeql-action/init` from 4.36.2 to 4.37.0 - [Release notes](https://github.com/github/codeql-action/releases) - [Changelog](https://github.com/github/codeql-action/blob/main/CHANGELOG.md) - [Commits](github/codeql-action@8aad20d...99df26d) Updates `github/codeql-action/analyze` from 4.36.2 to 4.37.0 - [Release notes](https://github.com/github/codeql-action/releases) - [Changelog](https://github.com/github/codeql-action/blob/main/CHANGELOG.md) - [Commits](github/codeql-action@8aad20d...99df26d) Updates `goreleaser/goreleaser-action` from 7.2.2 to 7.2.3 - [Release notes](https://github.com/goreleaser/goreleaser-action/releases) - [Commits](goreleaser/goreleaser-action@5daf1e9...f06c13b) Updates `actions/attest-build-provenance` from 4.1.0 to 4.1.1 - [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@a2bbfa2...0f67c3f) Updates `github/codeql-action/upload-sarif` from 4.36.2 to 4.37.0 - [Release notes](https://github.com/github/codeql-action/releases) - [Changelog](https://github.com/github/codeql-action/blob/main/CHANGELOG.md) - [Commits](github/codeql-action@8aad20d...99df26d) --- updated-dependencies: - dependency-name: actions/attest-build-provenance dependency-version: 4.1.1 dependency-type: direct:production update-type: version-update:semver-patch dependency-group: actions-updates - dependency-name: actions/setup-go dependency-version: 6.5.0 dependency-type: direct:production update-type: version-update:semver-minor dependency-group: actions-updates - dependency-name: github/codeql-action/analyze dependency-version: 4.36.3 dependency-type: direct:production update-type: version-update:semver-patch dependency-group: actions-updates - dependency-name: github/codeql-action/init dependency-version: 4.36.3 dependency-type: direct:production update-type: version-update:semver-patch dependency-group: actions-updates - dependency-name: github/codeql-action/upload-sarif dependency-version: 4.36.3 dependency-type: direct:production update-type: version-update:semver-patch dependency-group: actions-updates - dependency-name: goreleaser/goreleaser-action dependency-version: 7.2.3 dependency-type: direct:production update-type: version-update:semver-patch dependency-group: actions-updates ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Qodo reviews are paused for this user.Troubleshooting steps vary by plan Learn more → On a Teams plan? Using GitHub Enterprise Server, GitLab Self-Managed, or Bitbucket Data Center? |
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Sorry @tis24dev, your pull request is larger than the review limit of 150000 diff characters
📝 WalkthroughWalkthroughThis pull request adds a local-file upgrade path and prerelease upgrade script, standardizes dashboard status rendering, introduces Go linting, refreshes pinned CI actions and dependencies, and expands documentation for daemon operation, restore workflows, security, notifications, configuration, and release verification. ChangesUpgrade flow
Status UI
CI and repository tooling
Documentation
Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Dependency ReviewThe following issues were found:
License Issuesgo.mod
OpenSSF Scorecard
Scanned Files
|
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
Owner
Author
|
@coderabbitai review |
✅ Action performedReview finished.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 10
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
docs/CLOUD_STORAGE.md (1)
794-799: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winDownload all raw-layout sidecars during recovery.
When
BUNDLE_ASSOCIATED_FILES=false, the preceding step still selects and copies only oneLATESTobject, but the archive, manifest, metadata, and checksum files are separate. Skipping extraction therefore leaves required files unavailable for restore discovery or verification. Update Steps 4–6 to derive the archive base name and download all associated objects.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/CLOUD_STORAGE.md` around lines 794 - 799, Update Steps 4–6 in the recovery instructions for BUNDLE_ASSOCIATED_FILES=false to derive the selected LATEST archive’s base name and download its separate archive, manifest, metadata, and checksum sidecars. Ensure the raw-layout path provides every required object for restore discovery and verification, while preserving the existing bundled-archive extraction flow.
🧹 Nitpick comments (1)
docs/CLUSTER_RECOVERY.md (1)
137-137: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd language identifiers to the flagged fenced blocks.
This removes the MD040 warnings and keeps the documentation lint-clean:
docs/CLUSTER_RECOVERY.md#L137-L137: mark the prompt block astext.docs/CLUSTER_RECOVERY.md#L197-L197: mark the output block astext.docs/CLUSTER_RECOVERY.md#L393-L393: mark the prompt block astext.docs/CLUSTER_RECOVERY.md#L403-L403: mark the restore-plan block astext.docs/CLUSTER_RECOVERY.md#L419-L419: mark the log block astext.docs/RESTORE_GUIDE.md#L520-L520: mark the prompt block astext.docs/RESTORE_GUIDE.md#L644-L644: mark the confirmation block astext.docs/RESTORE_TECHNICAL.md#L1911-L1911: mark the tier summary block astext.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/CLUSTER_RECOVERY.md` at line 137, Add the text language identifier to each flagged fenced block: docs/CLUSTER_RECOVERY.md lines 137, 197, 393, 403, and 419; docs/RESTORE_GUIDE.md lines 520 and 644; and docs/RESTORE_TECHNICAL.md line 1911. Mark each prompt, output, restore-plan, log, confirmation, or tier-summary fence as text without changing its contents.Source: Linters/SAST tools
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/race.yml:
- Around line 40-43: Disable Go caching in the setup-go steps for both
.github/workflows/race.yml lines 40-43 and .github/workflows/release.yml lines
86-88 by removing or disabling the cache setting, ensuring pull request
workflows cannot populate caches reused by release workflows.
In `@docs/CLUSTER_RECOVERY.md`:
- Around line 1477-1480: Update the permissions statement in the “Permissions
wrong” section to describe restored ownership as a best-effort chown attempt,
acknowledging that owners may differ if it fails; retain the assertion that file
modes are preserved as long as chmod failures remain fatal.
In `@docs/CONFIGURATION.md`:
- Around line 828-829: The configuration documentation’s GFS fallback statement
conflates raw fallback values with the post-activation daily minimum. Update the
paragraph near the “Compiled fallbacks” text to state that all raw fallbacks are
0, then clarify that enabling RETENTION_POLICY=gfs promotes RETENTION_DAILY=0 to
1 while other tiers remain unset until configured.
In `@docs/DAEMON.md`:
- Around line 66-74: Add language identifiers to every affected fenced block:
use text for docs/DAEMON.md lines 66-74 and docs/NOTIFICATIONS.md lines 55-57
and 197-200, and bash for docs/DAEMON.md lines 129-163, preserving all block
contents.
In `@docs/EXAMPLES.md`:
- Around line 971-991: Update the daemon documentation near the description of
the four checks to state that healthchecks reports three fixed checks—alive,
backup, and updates—plus one check for each enabled notification channel; remove
the hard-coded total.
In `@docs/PROVENANCE_VERIFICATION.md`:
- Around line 121-127: Update the verification commands in the provenance
instructions to invoke gh attestation verify separately for the ${ASSET}
artifact and the ${ASSET}.tar.gz artifact, avoiding the multi-file glob while
preserving the existing repository and download context.
- Around line 222-233: Update the attestation verification examples in
PROVENANCE_VERIFICATION.md to enforce the claims listed in the verification
table by adding --signer-workflow for .github/workflows/release.yml and
--source-ref for the release tag or ref alongside --repo. Apply this
consistently to each relevant gh attestation verify example.
- Around line 175-185: Update the offline verification example in the provenance
documentation to use the digest-based filename produced by gh attestation
download instead of attestation.jsonl, and add --custom-trusted-root
trusted_root.jsonl to the gh attestation verify command while preserving the
existing repository and artifact arguments.
In `@docs/RELEASE-PROCESS.md`:
- Around line 9-16: Update the workflow diagram code fence in the release
process documentation to specify a language immediately after the opening fence,
using text unless converting the diagram to valid mermaid is necessary; preserve
the existing diagram content.
In `@upgrade-beta.sh`:
- Around line 318-338: Handle failure of the binary swap in the installation
block around `mv proxsave "${TARGET_BIN}"` instead of relying on `set -e`. If
the move fails, restore `${TARGET_BIN}.prev` to `${TARGET_BIN}` when possible,
ensure the restored binary has mode 755, print clear recovery guidance
consistent with the finalize-failure path, and exit nonzero; preserve the
existing post-swap sanity-check rollback for binaries that install but fail to
run.
---
Outside diff comments:
In `@docs/CLOUD_STORAGE.md`:
- Around line 794-799: Update Steps 4–6 in the recovery instructions for
BUNDLE_ASSOCIATED_FILES=false to derive the selected LATEST archive’s base name
and download its separate archive, manifest, metadata, and checksum sidecars.
Ensure the raw-layout path provides every required object for restore discovery
and verification, while preserving the existing bundled-archive extraction flow.
---
Nitpick comments:
In `@docs/CLUSTER_RECOVERY.md`:
- Line 137: Add the text language identifier to each flagged fenced block:
docs/CLUSTER_RECOVERY.md lines 137, 197, 393, 403, and 419;
docs/RESTORE_GUIDE.md lines 520 and 644; and docs/RESTORE_TECHNICAL.md line
1911. Mark each prompt, output, restore-plan, log, confirmation, or tier-summary
fence as text without changing its contents.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: fc37e11b-f4e5-4b5f-8cd6-a29d520f1d53
⛔ Files ignored due to path filters (1)
go.sumis excluded by!**/*.sum
📒 Files selected for processing (58)
.github/workflows/codecov.yml.github/workflows/codeql.yml.github/workflows/lint.yml.github/workflows/race.yml.github/workflows/release.yml.github/workflows/security-ultimate.yml.gitignore.golangci.ymlREADME.mdcmd/proxsave/daemon_auto_migrate_characterization_test.gocmd/proxsave/daemon_restart_verify_test.gocmd/proxsave/daemon_setup.gocmd/proxsave/dashboard.gocmd/proxsave/dashboard_check_apply.gocmd/proxsave/dashboard_cleanup_guards.gocmd/proxsave/dashboard_update_config.gocmd/proxsave/dashboard_upgrade.gocmd/proxsave/dashboard_upgrade_test.gocmd/proxsave/install_docs_characterization_test.gocmd/proxsave/main_modes.gocmd/proxsave/main_modes_test.gocmd/proxsave/upgrade.gocmd/proxsave/upgrade_localfile_test.godocs/CLOUD_STORAGE.mddocs/CLUSTER_RECOVERY.mddocs/COLLECTOR_ARCHITECTURE.mddocs/CONFIGURATION.mddocs/DAEMON.mddocs/DEVELOPER_GUIDE.mddocs/ENCRYPTION.mddocs/EXAMPLES.mddocs/INSTALL.mddocs/NOTIFICATIONS.mddocs/PROVENANCE_VERIFICATION.mddocs/README.mddocs/RELEASE-PROCESS.mddocs/RESTORE_DIAGRAMS.mddocs/RESTORE_GUIDE.mddocs/RESTORE_TECHNICAL.mddocs/SECURITY.mddocs/TEST_STRATEGY.mddocs/TROUBLESHOOTING.mdgo.modinternal/cli/args.gointernal/health/daemon_state_test.gointernal/logging/bootstrap.gointernal/orchestrator/healthcheck_setup_test.gointernal/orchestrator/pve_staged_apply_additional_test.gointernal/orchestrator/restore_dedup_materialize_test.gointernal/orchestrator/workflow_ui_charm.gointernal/orchestrator/workflow_ui_status.gointernal/orchestrator/workflow_ui_status_test.gointernal/ui/components/formgrid_test.gointernal/ui/components/sanitize_test.gointernal/ui/flows/install/audit.gointernal/ui/flows/install/install_test.gointernal/ui/flows/menu/menu.goupgrade-beta.sh
Comment on lines
+40
to
43
| uses: actions/setup-go@924ae3a1cded613372ab5595356fb5720e22ba16 | ||
| with: | ||
| go-version-file: go.mod | ||
| cache: true |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
rg -n 'pull_request|pull_request_target|workflow_call|permissions:|cache:' \
.github/workflows/race.yml .github/workflows/release.ymlRepository: tis24dev/proxsave
Length of output: 515
🏁 Script executed:
#!/bin/bash
sed -n '1,120p' .github/workflows/race.yml
printf '\n----\n'
sed -n '1,140p' .github/workflows/release.ymlRepository: tis24dev/proxsave
Length of output: 7340
🌐 Web query:
actions/setup-go cache input default false README cache-dependency-path go-version-file
💡 Result:
In the actions/setup-go GitHub Action, the default value for the cache input is true, meaning caching is enabled by default [1][2][3]. Regarding the other inputs: cache-dependency-path: This input is used to specify the path to dependency files (e.g., go.sum) [1][2][3]. By default, the action searches for go.sum in the repository root and uses its hash as part of the cache key [2][4][5]. You should use this input if you have multiple dependency files or if they are located in different subdirectories; it supports glob patterns [2][3][5]. go-version-file: This input specifies the path to a go.mod, go.work,.go-version, or.tool-versions file to determine which Go version to use [2][3][5]. If both the go-version and go-version-file inputs are provided, the go-version input takes precedence [2][3][5].
Citations:
- 1: https://github.com/actions/setup-go/blob/fdc0d672a12b8fc05ef4c549c3fd5b0f03ebeb4c/action.yml
- 2: https://github.com/actions/setup-go/blob/main/README.md
- 3: https://github.com/actions/setup-go?tab=readme-ov-file
- 4: https://github.com/actions/setup-go/tree/v6.1.0
- 5: https://github.com/actions/setup-go/blob/d35c59ab/README.md
Disable shared Go caching across the PR and release workflows. .github/workflows/race.yml#L40-L43 runs on pull_request with cache: true, and .github/workflows/release.yml#L86-L88 also enables the default cache. That lets an untrusted PR populate the cache later restored by release runs; disable caching or split the cache scope by trust boundary.
📍 Affects 2 files
.github/workflows/race.yml#L40-L43(this comment).github/workflows/release.yml#L86-L88
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.github/workflows/race.yml around lines 40 - 43, Disable Go caching in the
setup-go steps for both .github/workflows/race.yml lines 40-43 and
.github/workflows/release.yml lines 86-88 by removing or disabling the cache
setting, ensuring pull request workflows cannot populate caches reused by
release workflows.
Source: Linters/SAST tools
Comment on lines
1477
to
+1480
| **3. Permissions wrong**: | ||
|
|
||
| ProxSave preserves each restored file's owner and mode from the backup archive, so it does not hardcode these. The following is manual Proxmox advice for the case where the ownership on `/var/lib/pve-cluster` is wrong for some other reason: | ||
|
|
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Qualify the ownership-preservation claim.
Extraction applies ownership with best-effort chown; therefore restored owners may differ when the operation fails. Document this as an attempt/best-effort behavior rather than guaranteeing preservation, while keeping the mode-preservation claim if chmod remains fatal.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/CLUSTER_RECOVERY.md` around lines 1477 - 1480, Update the permissions
statement in the “Permissions wrong” section to describe restored ownership as a
best-effort chown attempt, acknowledging that owners may differ if it fails;
retain the assertion that file modes are preserved as long as chmod failures
remain fatal.
Comment on lines
+828
to
+829
| **Compiled fallbacks** are all `0`: GFS keeps nothing until you set `RETENTION_POLICY=gfs` and at least one tier. The `7/4/12/3` above are example/template values, not the defaults. `RETENTION_DAILY` is forced to at least `1` (0 is treated as 1). | ||
|
|
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
sed -n '810,845p' docs/CONFIGURATION.md
printf '\n--- SEARCH ---\n'
rg -n "RETENTION_DAILY|Compiled fallbacks|RETENTION_POLICY|gfs" -S .Repository: tis24dev/proxsave
Length of output: 16927
🏁 Script executed:
sed -n '730,760p' internal/config/config.go
printf '\n--- NORMALIZE ---\n'
sed -n '1,120p' internal/storage/retention_normalize.go
printf '\n--- TEST ---\n'
sed -n '880,910p' internal/config/config_test.goRepository: tis24dev/proxsave
Length of output: 3332
🏁 Script executed:
sed -n '1,220p' internal/storage/retention.go
printf '\n--- GFS TESTS ---\n'
sed -n '1,220p' internal/storage/retention_test.go
printf '\n--- LOCAL GFS TEST ---\n'
sed -n '1,140p' internal/storage/local_gfs_retention_test.goRepository: tis24dev/proxsave
Length of output: 14047
🏁 Script executed:
rg -n "NormalizeGFSRetentionConfig|EffectiveGFSRetentionConfig|applyGFSRetention|ClassifyBackupsGFS" internal/storage internal/orchestrator cmd/proxsave -S
printf '\n--- APPLY PATH ---\n'
sed -n '440,560p' internal/storage/local.goRepository: tis24dev/proxsave
Length of output: 13890
Clarify the GFS fallback semantics at docs/CONFIGURATION.md:828-829. RETENTION_DAILY=0 is promoted to 1 once RETENTION_POLICY=gfs is active, so “GFS keeps nothing until you set RETENTION_POLICY=gfs and at least one tier” is misleading. Spell out that 0 is the raw fallback and the daily minimum is applied after GFS is enabled.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/CONFIGURATION.md` around lines 828 - 829, The configuration
documentation’s GFS fallback statement conflates raw fallback values with the
post-activation daily minimum. Update the paragraph near the “Compiled
fallbacks” text to state that all raw fallbacks are 0, then clarify that
enabling RETENTION_POLICY=gfs promotes RETENTION_DAILY=0 to 1 while other tiers
remain unset until configured.
Comment on lines
+66
to
+74
| ``` | ||
| Daemon status: <keyword> | ||
| Scheduler mode: <cron|daemon> | ||
| Daemon service (proxsave-daemon.service): installed | not installed | ||
| Service state (systemctl is-active): <active|inactive|...> | ||
| Opted out of auto-migration (--daemon-remove): yes | no | ||
| Running version: <version> (<commit>) | ||
| Binary alignment: aligned | BEHIND (restart needed) | unknown | ||
| ``` |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Make all fenced blocks pass markdownlint.
docs/DAEMON.md#L66-L74: add a language such astext.docs/DAEMON.md#L129-L163: addbash.docs/NOTIFICATIONS.md#L55-L57: addtext.docs/NOTIFICATIONS.md#L197-L200: addtext.
🧰 Tools
🪛 markdownlint-cli2 (0.23.0)
[warning] 66-66: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
📍 Affects 2 files
docs/DAEMON.md#L66-L74(this comment)docs/DAEMON.md#L129-L163docs/NOTIFICATIONS.md#L55-L57docs/NOTIFICATIONS.md#L197-L200
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/DAEMON.md` around lines 66 - 74, Add language identifiers to every
affected fenced block: use text for docs/DAEMON.md lines 66-74 and
docs/NOTIFICATIONS.md lines 55-57 and 197-200, and bash for docs/DAEMON.md lines
129-163, preserving all block contents.
Source: Linters/SAST tools
Comment on lines
+971
to
+991
| ## Example 11: Resident daemon and healthchecks monitoring | ||
|
|
||
| **Scenario**: You want the backup scheduled and supervised by a resident service with a | ||
| hang watchdog and an external dead-man switch, instead of a bare cron entry. Fresh | ||
| installs default to this engine; use these commands to switch an existing install or to | ||
| inspect it. | ||
|
|
||
| ### Switch to the daemon | ||
|
|
||
| ```bash | ||
| # Install the systemd service, remove the cron entry, turn on centralized healthchecks | ||
| proxsave --daemon-setup | ||
|
|
||
| # Check status (exit 0 only when running, beating, and binary-aligned) | ||
| proxsave --daemon-status | ||
| ``` | ||
|
|
||
| `--daemon-setup` writes `SCHEDULER_MODE=daemon` and `HEALTHCHECK_ENABLED=true` and starts | ||
| `proxsave-daemon.service`. The daemon runs the backup daily at `SCHEDULER_TIME`, under a | ||
| `MAX_RUN_DURATION` watchdog, and reports four checks (alive, backup, updates, and one per | ||
| notification channel) to an external healthchecks monitor. |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Do not hard-code the daemon check count.
“Four checks” conflicts with “one per notification channel,” because the total varies. Document this as three fixed checks plus one check per enabled notification channel.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/EXAMPLES.md` around lines 971 - 991, Update the daemon documentation
near the description of the four checks to state that healthchecks reports three
fixed checks—alive, backup, and updates—plus one check for each enabled
notification channel; remove the hard-coded total.
Comment on lines
121
to
127
| ```bash | ||
| # Download all the binaries you need | ||
| cd ~/downloads | ||
| wget https://github.com/tis24dev/proxsave/releases/download/v0.9.0/proxsave-linux-amd64 | ||
| wget https://github.com/tis24dev/proxsave/releases/download/v0.9.0/proxsave-darwin-amd64 | ||
| wget "https://github.com/tis24dev/proxsave/releases/download/${TAG}/${ASSET}" | ||
| wget "https://github.com/tis24dev/proxsave/releases/download/${TAG}/${ASSET}.tar.gz" | ||
|
|
||
| # Verify all together | ||
| gh attestation verify proxsave-* --repo tis24dev/proxsave | ||
| gh attestation verify proxsave_${VER}_linux_amd64* --repo tis24dev/proxsave | ||
| ``` |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
sed -n '100,150p' docs/PROVENANCE_VERIFICATION.mdRepository: tis24dev/proxsave
Length of output: 1648
🏁 Script executed:
git diff -- docs/PROVENANCE_VERIFICATION.mdRepository: tis24dev/proxsave
Length of output: 155
🌐 Web query:
gh attestation verify accepts multiple artifacts glob positional arguments manual
💡 Result:
The gh attestation verify command does not support multiple artifacts, glob patterns, or positional argument lists for simultaneous verification [1][2][3]. The command is designed to accept a single file path or a single OCI image URI as a positional argument [3][4]. Users cannot pass multiple files or glob patterns directly to the command line to verify them in a single execution [1]. If you need to verify multiple artifacts, you must iterate over them using a shell script or similar mechanism, running the gh attestation verify command once for each artifact. Note that while the actions/attest GitHub Action (used to create attestations) supports glob patterns and lists for generating attestations [5], this functionality is not mirrored in the verification command provided by the GitHub CLI [1][2].
Citations:
- 1:
gh attestation verifycan't operate on file globs and is unclear on this in the docs cli/cli#9215 - 2: https://cli.github.com/manual/gh%5Fattestation%5Fverify
- 3: https://manpages.opensuse.org/Leap-16.0/gh/gh-attestation-verify.1.en.html
- 4: https://cli.github.com/manual/gh_attestation_verify
- 5: https://github.com/actions/attest?tab=readme-ov-file
🌐 Web query:
gh attestation verify manual single artifact path glob
💡 Result:
The GitHub CLI command gh attestation verify does not support glob patterns for specifying artifact paths [1]. The command explicitly requires either a single, specific file path to an artifact on disk or a container image URI (e.g., oci://) [2][3][4]. Because the command does not natively support globbing, you must provide the path to each artifact individually. If you need to verify multiple artifacts, you must iterate over the files in your shell environment and invoke the command for each one [5]. For example, in a bash-like environment, you could use a loop: for artifact in path/to/artifacts/*; do gh attestation verify "$artifact" -R owner/repo done Note that while the GitHub Actions action actions/attest supports glob patterns when generating attestations [6], this functionality does not extend to the verification command provided by the GitHub CLI [1].
Citations:
- 1:
gh attestation verifycan't operate on file globs and is unclear on this in the docs cli/cli#9215 - 2: https://cli.github.com/manual/gh_attestation_verify
- 3: https://man.archlinux.org/man/gh-attestation-verify.1.en
- 4: https://manpages.opensuse.org/Leap-16.0/gh/gh-attestation-verify.1.en.html
- 5: https://github.blog/security/supply-chain-security/configure-github-artifact-attestations-for-secure-cloud-native-delivery/
- 6: https://github.com/actions/attest?tab=readme-ov-file
🌐 Web query:
gh attestation verify accepts multiple artifact paths glob positional arguments
💡 Result:
The gh attestation verify command in the GitHub CLI does not natively support glob patterns or multiple positional arguments for artifact paths [1]. The command is designed to accept a single artifact path or OCI image URI as a positional argument [2][3][4]. Because the command does not support batch processing via globs or multiple arguments, users must iterate over their artifacts using a shell loop or a similar mechanism to perform verification on multiple files [5]. Example of how to verify multiple artifacts: for artifact in path/to/artifacts/*.zip; do gh attestation verify "$artifact" --repo OWNER/REPO done Official documentation and existing issues confirm that the command expects a single target per execution [6][1][7].
Citations:
- 1:
gh attestation verifycan't operate on file globs and is unclear on this in the docs cli/cli#9215 - 2: https://manpages.opensuse.org/Leap-16.0/gh/gh-attestation-verify.1.en.html
- 3: https://man.archlinux.org/man/gh-attestation-verify.1.en
- 4: https://man.archlinux.org/man/extra/github-cli/gh-attestation-verify.1.en
- 5: https://github.com/integrations/terraform-provider-github/blob/main/VERIFY_ATTESTATIONS.md
- 6: https://cli.github.com/manual/gh_attestation_verify
- 7: https://github.com/github/docs/blob/main/content/actions/how-tos/secure-your-work/use-artifact-attestations/use-artifact-attestations.md
Verify each artifact separately. gh attestation verify only accepts one artifact path per run, so the glob here expands to multiple paths and will fail; run it once for ${ASSET} and once for ${ASSET}.tar.gz.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/PROVENANCE_VERIFICATION.md` around lines 121 - 127, Update the
verification commands in the provenance instructions to invoke gh attestation
verify separately for the ${ASSET} artifact and the ${ASSET}.tar.gz artifact,
avoiding the multi-file glob while preserving the existing repository and
download context.
Source: MCP tools
Comment on lines
175
to
185
| ```bash | ||
| # Download the attestation as a bundle | ||
| gh attestation download proxsave-linux-amd64 \ | ||
| gh attestation download "${ASSET}" \ | ||
| --repo tis24dev/proxsave \ | ||
| --output attestation.jsonl | ||
|
|
||
| # Verify offline using the bundle | ||
| gh attestation verify proxsave-linux-amd64 \ | ||
| gh attestation verify "${ASSET}" \ | ||
| --bundle attestation.jsonl \ | ||
| --repo tis24dev/proxsave | ||
| ``` |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the referenced section in the docs file.
sed -n '160,200p' docs/PROVENANCE_VERIFICATION.md
# Search for other attestation examples in the repo for context.
rg -n "gh attestation (download|verify)|custom-trusted-root|--output" docs -SRepository: tis24dev/proxsave
Length of output: 2106
🌐 Web query:
gh attestation download --output custom-trusted-root offline verification docs
💡 Result:
To perform offline verification of artifact attestations using the GitHub CLI (gh), you must follow a three-step process involving downloading the necessary files on an internet-connected machine and performing the verification in your offline environment [1]. 1. Download the attestation bundle Use the gh attestation download command to retrieve the attestation bundle for your artifact [1]. gh attestation download PATH/TO/YOUR/ARTIFACT -R OWNER/REPO 2. Download the trusted roots On the online machine, generate a trusted_root.jsonl file, which is required for offline verification [1][2]. gh attestation trusted-root > trusted_root.jsonl 3. Perform offline verification Transfer the GitHub CLI, your artifact, the bundle file (from Step 1), and the trusted root file (from Step 2) to your offline environment [1]. Then, use the gh attestation verify command with the --bundle and --custom-trusted-root flags [1][3]: gh attestation verify PATH/TO/YOUR/ARTIFACT -R OWNER/REPO --bundle PATH/TO/BUNDLE.jsonl --custom-trusted-root trusted_root.jsonl Key considerations for offline verification: - It is best practice to generate a fresh trusted_root.jsonl file whenever importing new signed material [1]. - The trusted_root.jsonl file does not contain an expiration date, but offline verification will not account for any key revocations that occur after the file is generated [1]. - For more detailed guidance, refer to the official GitHub documentation on verifying attestations offline [1][4].
Citations:
- 1: https://docs.github.com/en/actions/how-tos/secure-your-work/use-artifact-attestations/verify-attestations-offline
- 2: https://cli.github.com/manual/gh_attestation_trusted-root
- 3: https://cli.github.com/manual/gh_attestation_verify
- 4: https://docs.github.com/actions/security-for-github-actions/using-artifact-attestations/verifying-attestations-offline
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
import urllib.request, ssl
urls = [
"https://cli.github.com/manual/gh_attestation_download",
"https://cli.github.com/manual/gh_attestation_verify",
"https://cli.github.com/manual/gh_attestation_trusted-root",
]
for url in urls:
print(f"\n### {url}")
try:
with urllib.request.urlopen(url, context=ssl.create_default_context(), timeout=20) as r:
text = r.read().decode("utf-8", "replace")
# print a compact excerpt around flag names
for needle in ["--output", "--bundle", "--custom-trusted-root", "usage", "USAGE"]:
idx = text.find(needle)
if idx != -1:
start = max(0, idx - 400)
end = min(len(text), idx + 1000)
print(text[start:end].replace("\n", " "))
break
else:
print(text[:1200].replace("\n", " "))
except Exception as e:
print("ERROR:", e)
PYRepository: tis24dev/proxsave
Length of output: 757
🌐 Web query:
site:cli.github.com/manual/gh_attestation_download --output gh attestation download
💡 Result:
The gh attestation download command in the GitHub CLI is used to download attestations associated with a specific artifact for offline use [1]. To use this command, you must provide either a file path to the local artifact or a container image URI [1]. The command is structured as follows: gh attestation download [file-path | oci-image-uri] [flags] This command is part of the broader gh attestation toolset provided by the GitHub CLI [1].
Citations:
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
import ssl, urllib.request, re
urls = {
"download": "https://cli.github.com/manual/gh_attestation_download",
"verify": "https://cli.github.com/manual/gh_attestation_verify",
"trusted-root": "https://cli.github.com/manual/gh_attestation_trusted-root",
}
ctx = ssl._create_unverified_context()
for name, url in urls.items():
print(f"\n### {name} ###")
with urllib.request.urlopen(url, context=ctx, timeout=30) as r:
text = r.read().decode("utf-8", "replace")
# Extract relevant flag/usage-ish lines from the HTML page text.
for needle in ["gh attestation download", "gh attestation verify", "--output", "--bundle", "--custom-trusted-root", "trusted-root"]:
idx = text.find(needle)
if idx != -1:
start = max(0, idx - 700)
end = min(len(text), idx + 1800)
snippet = text[start:end]
# compact whitespace a bit for readability
snippet = re.sub(r"\s+", " ", snippet)
print(snippet)
print("---")
break
PYRepository: tis24dev/proxsave
Length of output: 6959
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
import ssl, urllib.request, re
urls = {
"download": "https://cli.github.com/manual/gh_attestation_download",
"verify": "https://cli.github.com/manual/gh_attestation_verify",
"trusted-root": "https://cli.github.com/manual/gh_attestation_trusted-root",
}
ctx = ssl._create_unverified_context()
for name, url in urls.items():
print(f"\n### {name} ###")
with urllib.request.urlopen(url, context=ctx, timeout=30) as r:
text = r.read().decode("utf-8", "replace")
for needle in ["--output", "--bundle", "--custom-trusted-root", "gh attestation download", "gh attestation verify", "gh attestation trusted-root"]:
idx = text.find(needle)
if idx != -1:
start = max(0, idx - 900)
end = min(len(text), idx + 2200)
snippet = text[start:end]
snippet = re.sub(r"\s+", " ", snippet)
print(snippet)
print("---")
print()
PYRepository: tis24dev/proxsave
Length of output: 20074
Fix the offline verification example.
gh attestation download writes the bundle to a file named after the artifact digest, and gh attestation verify needs --custom-trusted-root trusted_root.jsonl for offline use.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/PROVENANCE_VERIFICATION.md` around lines 175 - 185, Update the offline
verification example in the provenance documentation to use the digest-based
filename produced by gh attestation download instead of attestation.jsonl, and
add --custom-trusted-root trusted_root.jsonl to the gh attestation verify
command while preserving the existing repository and artifact arguments.
Source: MCP tools
Comment on lines
222
to
+233
| When you run `gh attestation verify`, the following checks are performed: | ||
|
|
||
| | Verification | Description | | ||
| |--------------|-------------| | ||
| | ✓ **Source repository** | Confirms that the build comes from `github.com/tis24dev/proxsave` | | ||
| | ✓ **Commit SHA** | Verifies the exact commit used for the build | | ||
| | ✓ **Workflow** | Checks that `.github/workflows/release.yml` was used | | ||
| | ✓ **Build environment** | Confirms that the build occurred on GitHub-hosted runner | | ||
| | ✓ **SHA256 integrity** | Calculates the file hash and compares it with the attested one | | ||
| | ✓ **Cryptographic signature** | Verifies the OIDC/Sigstore signature on the attestation | | ||
| | ✓ **Rekor timestamp** | Checks the immutable record in the transparency log | | ||
| | ✓ **SLSA compliance** | Verifies that the attestation complies with the SLSA v1 standard | | ||
| | Source repository | Confirms the build comes from `github.com/tis24dev/proxsave` | | ||
| | Commit SHA | Verifies the exact commit used for the build | | ||
| | Workflow | Checks that `.github/workflows/release.yml` was used | | ||
| | Build environment | Confirms the build occurred on a GitHub-hosted runner | | ||
| | SHA256 integrity | Recomputes the file hash and compares it with the attested one | | ||
| | Cryptographic signature | Verifies the OIDC/Sigstore signature on the attestation | | ||
| | Rekor timestamp | Checks the immutable record in the transparency log | | ||
| | SLSA compliance | Verifies the attestation complies with the SLSA v1 standard | |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Map the document structure, then inspect the referenced range.
wc -l docs/PROVENANCE_VERIFICATION.md
ast-grep outline docs/PROVENANCE_VERIFICATION.md || true
sed -n '200,260p' docs/PROVENANCE_VERIFICATION.md | cat -n
# Search for gh attestation verify usage and any signer-workflow/source-ref flags.
rg -n "gh attestation verify|signer-workflow|source-ref|release\.yml|source-ref" docs/PROVENANCE_VERIFICATION.mdRepository: tis24dev/proxsave
Length of output: 3293
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the document for any other verification examples that constrain signer workflow or source ref.
rg -n -C 3 "gh attestation verify|signer-workflow|source-ref|release\.yml" docs/PROVENANCE_VERIFICATION.mdRepository: tis24dev/proxsave
Length of output: 2603
🌐 Web query:
gh attestation verify signer-workflow source-ref source-tag source-branch cli.github.com manual
💡 Result:
The gh attestation verify command is used in the GitHub CLI to verify the integrity and provenance of an artifact using cryptographically signed attestations [1][2]. When verifying attestations, you can use several optional flags to enforce specific requirements regarding the identity of the signer and the source of the build [3][2]. Key flags related to your query include: --signer-workflow: Enforces that the Actions workflow that signed the attestation matches a specified value. This is particularly useful when using reusable workflows, as it ensures the attestation was generated by the expected workflow file (format: [host/]owner/repo/.github/workflows/workflow.yml) [3][2]. --source-ref: Enforces that the Git reference (e.g., a branch name or tag) associated with the source repository matches the provided value [4][2]. --source-tag: Note that there is no specific --source-tag flag in the current gh attestation verify command. Users should typically use --source-ref to enforce a specific branch or tag [4][2]. Other related flags include: --signer-repo: Enforces that the repository containing the workflow that signed the attestation matches the provided value [3][2]. --signer-digest: Enforces that the digest associated with the signer workflow matches the provided value [4][2]. --source-digest: Enforces that the digest associated with the source repository matches the provided value [4][2]. These flags help ensure that the verified artifact originated from the expected source and was produced by the authorized workflow [3][2].
Citations:
- 1: https://man.archlinux.org/man/gh-attestation-verify.1.en
- 2: https://www.mankier.com/1/gh-attestation-verify
- 3: https://github.com/github/docs/blob/main/content/actions/how-tos/secure-your-work/use-artifact-attestations/increase-security-rating.md
- 4: Add signer-digest, source-ref, and source-digest options for
gh attestation verifycli/cli#10308
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Show the exact section around the table and any verifier examples adjacent to it.
sed -n '210,255p' docs/PROVENANCE_VERIFICATION.md | cat -n
# Show all occurrences of verifier commands in the doc.
rg -n -C 2 "gh attestation verify" docs/PROVENANCE_VERIFICATION.mdRepository: tis24dev/proxsave
Length of output: 3238
Bind attestation verification to the published workflow and tag/ref. The examples only use --repo, so they don’t enforce .github/workflows/release.yml or the release ref the table claims. Add --signer-workflow and --source-ref, or soften the table.
🧰 Tools
🪛 LanguageTool
[uncategorized] ~228-~228: The official name of this software platform is spelled with a capital “H”.
Context: ...or the build | | Workflow | Checks that .github/workflows/release.yml was used | | Bui...
(GITHUB)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/PROVENANCE_VERIFICATION.md` around lines 222 - 233, Update the
attestation verification examples in PROVENANCE_VERIFICATION.md to enforce the
claims listed in the verification table by adding --signer-workflow for
.github/workflows/release.yml and --source-ref for the release tag or ref
alongside --repo. Apply this consistently to each relevant gh attestation verify
example.
Source: MCP tools
Comment on lines
9
to
16
| ``` | ||
| dev → Pull Request → main → tag vX.Y.Z → GitHub Actions (GoReleaser) → GitHub Release | ||
| push pr-vX.Y.Z on dev | ||
| -> release-intake.yml opens a dev -> main PR (marker: <!-- release-tag: vX.Y.Z -->) | ||
| -> release-guard.yml validates the PR | ||
| -> squash-merge the PR into main | ||
| -> post-merge-release.yml force-moves dev and creates the vX.Y.Z tag on the squash commit | ||
| -> release.yml (GoReleaser) builds, signs, and publishes the GitHub Release | ||
| ``` |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Specify the language for the workflow diagram fence.
Add text (or convert the block to valid mermaid) after the opening fence so Markdown linting passes.
🧰 Tools
🪛 markdownlint-cli2 (0.23.0)
[warning] 9-9: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/RELEASE-PROCESS.md` around lines 9 - 16, Update the workflow diagram
code fence in the release process documentation to specify a language
immediately after the opening fence, using text unless converting the diagram to
valid mermaid is necessary; preserve the existing diagram content.
Source: Linters/SAST tools
Comment on lines
+318
to
+338
| ############################################### | ||
| # 15) Swap in the beta binary (keep a rollback copy) | ||
| ############################################### | ||
| echo "[+] Backing up current binary -> ${TARGET_BIN}.prev" | ||
| cp -f "${TARGET_BIN}" "${TARGET_BIN}.prev" | ||
|
|
||
| echo "[+] Installing beta binary -> ${TARGET_BIN}" | ||
| # Conventional executable mode 0755 (owner rwx, group/other r-x). The binary runs | ||
| # as root and only root can replace it; the security check verifies it is | ||
| # root-owned and not group/other-writable, not an exact mode. | ||
| mv proxsave "${TARGET_BIN}" | ||
| chmod 755 "${TARGET_BIN}" | ||
|
|
||
| # Sanity-check the swapped binary actually runs before finalizing. A corrupt or | ||
| # incompatible binary is rolled back to the previous one immediately. | ||
| if ! "${TARGET_BIN}" --version >/dev/null 2>&1; then | ||
| echo "❌ The installed beta binary failed to run; rolling back to the previous binary." | ||
| mv -f "${TARGET_BIN}.prev" "${TARGET_BIN}" | ||
| chmod 755 "${TARGET_BIN}" | ||
| exit 1 | ||
| fi |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
No auto-rollback if mv itself fails during the binary swap.
The post-swap sanity check (Line 333) rolls back to .prev if the new binary fails to run, but a failure of the mv on Line 328 itself (likely a copy+unlink if TMP_DIR and ${TARGET_DIR} are on different filesystems, e.g. tmpfs vs /opt) aborts the script via set -e with no rollback and no printed guidance -- unlike the finalize-failure path (Lines 350-359), which does tell the user how to recover. A partial/failed cross-device copy can leave ${TARGET_BIN} corrupted with the process's only cue being a bare error.
🔒️ Suggested fix: rollback + guidance on swap failure
echo "[+] Installing beta binary -> ${TARGET_BIN}"
-mv proxsave "${TARGET_BIN}"
-chmod 755 "${TARGET_BIN}"
+if ! mv proxsave "${TARGET_BIN}"; then
+ echo "❌ Failed to install the new binary; restoring the previous binary."
+ mv -f "${TARGET_BIN}.prev" "${TARGET_BIN}" 2>/dev/null || true
+ chmod 755 "${TARGET_BIN}" 2>/dev/null || true
+ exit 1
+fi
+chmod 755 "${TARGET_BIN}"📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| ############################################### | |
| # 15) Swap in the beta binary (keep a rollback copy) | |
| ############################################### | |
| echo "[+] Backing up current binary -> ${TARGET_BIN}.prev" | |
| cp -f "${TARGET_BIN}" "${TARGET_BIN}.prev" | |
| echo "[+] Installing beta binary -> ${TARGET_BIN}" | |
| # Conventional executable mode 0755 (owner rwx, group/other r-x). The binary runs | |
| # as root and only root can replace it; the security check verifies it is | |
| # root-owned and not group/other-writable, not an exact mode. | |
| mv proxsave "${TARGET_BIN}" | |
| chmod 755 "${TARGET_BIN}" | |
| # Sanity-check the swapped binary actually runs before finalizing. A corrupt or | |
| # incompatible binary is rolled back to the previous one immediately. | |
| if ! "${TARGET_BIN}" --version >/dev/null 2>&1; then | |
| echo "❌ The installed beta binary failed to run; rolling back to the previous binary." | |
| mv -f "${TARGET_BIN}.prev" "${TARGET_BIN}" | |
| chmod 755 "${TARGET_BIN}" | |
| exit 1 | |
| fi | |
| ############################################### | |
| # 15) Swap in the beta binary (keep a rollback copy) | |
| ############################################### | |
| echo "[+] Backing up current binary -> ${TARGET_BIN}.prev" | |
| cp -f "${TARGET_BIN}" "${TARGET_BIN}.prev" | |
| echo "[+] Installing beta binary -> ${TARGET_BIN}" | |
| # Conventional executable mode 0755 (owner rwx, group/other r-x). The binary runs | |
| # as root and only root can replace it; the security check verifies it is | |
| # root-owned and not group/other-writable, not an exact mode. | |
| if ! mv proxsave "${TARGET_BIN}"; then | |
| echo "❌ Failed to install the new binary; restoring the previous binary." | |
| mv -f "${TARGET_BIN}.prev" "${TARGET_BIN}" 2>/dev/null || true | |
| chmod 755 "${TARGET_BIN}" 2>/dev/null || true | |
| exit 1 | |
| fi | |
| chmod 755 "${TARGET_BIN}" | |
| # Sanity-check the swapped binary actually runs before finalizing. A corrupt or | |
| # incompatible binary is rolled back to the previous one immediately. | |
| if ! "${TARGET_BIN}" --version >/dev/null 2>&1; then | |
| echo "❌ The installed beta binary failed to run; rolling back to the previous binary." | |
| mv -f "${TARGET_BIN}.prev" "${TARGET_BIN}" | |
| chmod 755 "${TARGET_BIN}" | |
| exit 1 | |
| fi |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@upgrade-beta.sh` around lines 318 - 338, Handle failure of the binary swap in
the installation block around `mv proxsave "${TARGET_BIN}"` instead of relying
on `set -e`. If the move fails, restore `${TARGET_BIN}.prev` to `${TARGET_BIN}`
when possible, ensure the restored binary has mode 755, print clear recovery
guidance consistent with the finalize-failure path, and exit nonzero; preserve
the existing post-swap sanity-check rollback for binaries that install but fail
to run.
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.
Slice 6 of 6 (final) — completes the progressive rebuild of dev (base=dev), cherry-picked checkpoint range, tree matches
b1c847f,go build ./...verified. ~59 files.Content: characterization tests, docs rewrites (DAEMON, CONFIGURATION, RELEASE-PROCESS, CLUSTER_RECOVERY, ENCRYPTION), upgrade-beta.sh +
--upgrade --localfile, golangci-lint fixes, dependabot bumps (bubbles v2.1.1, actions group).After merge, dev == dev-temp (full work reconstructed in 6 reviewed PRs). Trigger review with
@coderabbitai review.Greptile Summary
This PR completes the final rebuild slice. The main changes are:
--upgrade --localfilefinalization.Confidence Score: 5/5
This looks safe to merge.
Important Files Changed
--localfileto upgrade mode.Flowchart
%%{init: {'theme': 'neutral'}}%% flowchart TD A[Start upgrade] --> B{Local file mode?} B -- No --> C[Fetch and install latest release] B -- Yes --> D[Use installed binary] C --> E[Shared finalization] D --> E E --> F[Upgrade configuration] F --> G[Refresh docs and symlinks] G --> H[Migrate or restart daemon] H --> I[Normalize permissions] J[upgrade-beta.sh] --> K[Download beta assets] K --> L[Verify signature and checksum] L --> M[Back up and replace binary] M --> N{Version check passes?} N -- No --> O[Restore previous binary] N -- Yes --> P[Run local-file finalization] P --> E%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%% flowchart TD A[Start upgrade] --> B{Local file mode?} B -- No --> C[Fetch and install latest release] B -- Yes --> D[Use installed binary] C --> E[Shared finalization] D --> E E --> F[Upgrade configuration] F --> G[Refresh docs and symlinks] G --> H[Migrate or restart daemon] H --> I[Normalize permissions] J[upgrade-beta.sh] --> K[Download beta assets] K --> L[Verify signature and checksum] L --> M[Back up and replace binary] M --> N{Version check passes?} N -- No --> O[Restore previous binary] N -- Yes --> P[Run local-file finalization] P --> EReviews (1): Last reviewed commit: "ci: bump the actions-updates group acros..." | Re-trigger Greptile
Summary by CodeRabbit
--localfileupgrade support to finalize upgrades without downloading another binary.