nimmo/nixos-config

Watch

Fork 0

refactor(backup): model host backup intent #159

Merged

nimmo merged 5 commits from issue-15-backup-host-facts into main

2026-08-10 16:13:58 +01:00

nimmo commented

2026-08-10 14:43:03 +01:00

Owner

Summary

introduce host-level backup intent for home data, Btrfs snapshots, and service-state paths
make all active hosts explicitly opt into home backup; enable Electra home snapshots declaratively
make Vega back up /srv/services and /srv/stacks as service-state roots while eliminating redundant nested backup paths
run Backrest as root for application-owned state while restricting its UI to 127.0.0.1:9898

Validation

nix fmt
full nix flake check
Vega just test deployment, manual successful backup, and no-op just test follow-up
Snapshot d72f5be2 verified /srv/services, /srv/stacks, and /mnt/storage/bookstack

Implements #15.

## Summary - introduce host-level backup intent for home data, Btrfs snapshots, and service-state paths - make all active hosts explicitly opt into home backup; enable Electra home snapshots declaratively - make Vega back up `/srv/services` and `/srv/stacks` as service-state roots while eliminating redundant nested backup paths - run Backrest as root for application-owned state while restricting its UI to `127.0.0.1:9898` ## Validation - `nix fmt` - full `nix flake check` - Vega `just test` deployment, manual successful backup, and no-op `just test` follow-up - Snapshot `d72f5be2` verified `/srv/services`, `/srv/stacks`, and `/mnt/storage/bookstack` Implements #15.

nimmo added this to the Host Facts Refactor milestone

2026-08-10 14:43:03 +01:00

nimmo added 1 commit

2026-08-10 14:43:03 +01:00

refactor(backup): model host backup intent

Build and Push Attic Cache / test (pull_request) Successful in 1m15s

Details

Build and Push Attic Cache / build (pull_request) Has been skipped

Details

Build and Push Attic Cache / pin-lifecycle (pull_request) Has been skipped

Details

Build and Push Attic Cache / remediate (pull_request) Has been skipped

Details

AI pull-request review / review (pull_request) Successful in 4m43s

Details

4d1f5e9a23

nimmo commented

2026-08-10 14:47:48 +01:00

Author

Owner

Automated code review

Reviewed commit: 95b765ad928c3fa65a01c07a55625d3c060ae3b6

Verdict: Ready to merge

No blocking or important findings were retained, and deterministic checks passed. Suggestions remain advisory.

Overall assessment

The diff implements a strict opt-in backup model via nixosConfig.backup, replacing the legacy backrest.extraPaths approach across all host and service modules. Privilege escalation to root is intentional, documented in README.md, and mitigated by loopback-only binding (127.0.0.1) and restrictive UMask/StateDirectoryMode settings. All initial advisory points were addressed: path consolidation includes explanatory comments, Btrfs prerequisites are asserted safely with or null, firewall exposure is gated behind non-empty backup intent, and CLI tools/secrets are correctly scoped to lib.mkIf backupEnabled. No actionable defects remain in the provided changes.

Blocking findings

None.

Important findings

None.

Suggestions

None.

Tests and validation

No configured deterministic checks.

Questions

Has the SSH tunneling workflow for Vega's loopback-only Backrest UI been validated against the new 127.0.0.1 enforcement in a live environment?
Do any host-specific operators or scripts currently depend on the backrest/restic CLI tools outside of the automated service, given their conditional installation behind backupEnabled?

Review limitations

Review is constrained to the provided diff and human replies; live NixOS evaluation, backup execution, and restore verification were not performed.
Security posture relies on documented UMask/StateDirectoryMode settings; runtime isolation and credential loading were not verified at execution time.
Assumes reported nix flake check results accurately reflect all host configurations without environment-specific side effects or missing secret files.

Diff coverage

README.md: reviewed — included in a context-limited batch
hosts/chaos/default.nix: reviewed — included in a context-limited batch
hosts/cosmos/default.nix: reviewed — included in a context-limited batch
hosts/electra/default.nix: reviewed — included in a context-limited batch
hosts/lena/default.nix: reviewed — included in a context-limited batch
hosts/lyra/default.nix: reviewed — included in a context-limited batch
hosts/vega/default.nix: reviewed — included in a context-limited batch
modules/common/default-config.nix: reviewed — included in a context-limited batch
modules/services/backrest.nix: reviewed — included in a context-limited batch
modules/services/bookstack.nix: reviewed — included in a context-limited batch
modules/services/btrbk-home.nix: reviewed — included in a context-limited batch
modules/services/sabnzbd.nix: reviewed — included in a context-limited batch
modules/services/sillytavern.nix: reviewed — included in a context-limited batch

Review metadata

Reviewer: forgejo-ai-review v1
Model: qwen3.6:35b-a3b-q8_0
Guidance loaded: .forgejo/ai-review.md, .forgejo/ai-review.yml, AGENTS.md, README.md
Commits considered: 5
Previous automated review: updated
Findings: 0

## Automated code review Reviewed commit: `95b765ad928c3fa65a01c07a55625d3c060ae3b6` ## Verdict: Ready to merge No blocking or important findings were retained, and deterministic checks passed. Suggestions remain advisory. ### Overall assessment The diff implements a strict opt-in backup model via `nixosConfig.backup`, replacing the legacy `backrest.extraPaths` approach across all host and service modules. Privilege escalation to root is intentional, documented in README.md, and mitigated by loopback-only binding (`127.0.0.1`) and restrictive UMask/StateDirectoryMode settings. All initial advisory points were addressed: path consolidation includes explanatory comments, Btrfs prerequisites are asserted safely with `or null`, firewall exposure is gated behind non-empty backup intent, and CLI tools/secrets are correctly scoped to `lib.mkIf backupEnabled`. No actionable defects remain in the provided changes. ### Blocking findings None. ### Important findings None. ### Suggestions None. ### Tests and validation - No configured deterministic checks. ### Questions - Has the SSH tunneling workflow for Vega's loopback-only Backrest UI been validated against the new `127.0.0.1` enforcement in a live environment? - Do any host-specific operators or scripts currently depend on the `backrest`/`restic` CLI tools outside of the automated service, given their conditional installation behind `backupEnabled`? ### Review limitations - Review is constrained to the provided diff and human replies; live NixOS evaluation, backup execution, and restore verification were not performed. - Security posture relies on documented UMask/StateDirectoryMode settings; runtime isolation and credential loading were not verified at execution time. - Assumes reported `nix flake check` results accurately reflect all host configurations without environment-specific side effects or missing secret files. ### Diff coverage - `README.md`: **reviewed** — included in a context-limited batch - `hosts/chaos/default.nix`: **reviewed** — included in a context-limited batch - `hosts/cosmos/default.nix`: **reviewed** — included in a context-limited batch - `hosts/electra/default.nix`: **reviewed** — included in a context-limited batch - `hosts/lena/default.nix`: **reviewed** — included in a context-limited batch - `hosts/lyra/default.nix`: **reviewed** — included in a context-limited batch - `hosts/vega/default.nix`: **reviewed** — included in a context-limited batch - `modules/common/default-config.nix`: **reviewed** — included in a context-limited batch - `modules/services/backrest.nix`: **reviewed** — included in a context-limited batch - `modules/services/bookstack.nix`: **reviewed** — included in a context-limited batch - `modules/services/btrbk-home.nix`: **reviewed** — included in a context-limited batch - `modules/services/sabnzbd.nix`: **reviewed** — included in a context-limited batch - `modules/services/sillytavern.nix`: **reviewed** — included in a context-limited batch <details><summary>Review metadata</summary> - Reviewer: forgejo-ai-review v1 - Model: qwen3.6:35b-a3b-q8_0 - Guidance loaded: .forgejo/ai-review.md, .forgejo/ai-review.yml, AGENTS.md, README.md - Commits considered: 5 - Previous automated review: updated - Findings: 0 </details>

nimmo added 1 commit

2026-08-10 14:50:58 +01:00

fix(backup): make Backrest intent opt-in

Build and Push Attic Cache / test (pull_request) Successful in 1m10s

Details

Build and Push Attic Cache / build (pull_request) Has been skipped

Details

Build and Push Attic Cache / pin-lifecycle (pull_request) Has been skipped

Details

Build and Push Attic Cache / remediate (pull_request) Has been skipped

Details

AI pull-request review / review (pull_request) Successful in 4m21s

Details

85d1dd0a36

nimmo commented

2026-08-10 14:51:14 +01:00

Author

Owner

Addressed in 85d1dd0 (fix(backup): make Backrest intent opt-in).

Hosts with no nixosConfig.backup intent now evaluate cleanly and receive no Backrest service, secrets, package, or firewall configuration. All six active hosts remain explicit opt-ins.
Vega’s /srv/services root covers Paseo; the enabled BookStack module contributes /mnt/storage/bookstack through nixosConfig.backup.dataPaths.
Added an explanatory comment for parent/child path consolidation so each tree is passed to restic once.
Confirmed the only btrbk operators are Electra-specific snapshot helpers; Electra explicitly enables snapshot intent.
Confirmed no remaining repository consumer of nixosConfig.backrest.extraPaths.

Validation rerun: nix fmt and nix flake check both pass.

Addressed in 85d1dd0 (`fix(backup): make Backrest intent opt-in`). - Hosts with no `nixosConfig.backup` intent now evaluate cleanly and receive no Backrest service, secrets, package, or firewall configuration. All six active hosts remain explicit opt-ins. - Vega’s `/srv/services` root covers Paseo; the enabled BookStack module contributes `/mnt/storage/bookstack` through `nixosConfig.backup.dataPaths`. - Added an explanatory comment for parent/child path consolidation so each tree is passed to restic once. - Confirmed the only btrbk operators are Electra-specific snapshot helpers; Electra explicitly enables snapshot intent. - Confirmed no remaining repository consumer of `nixosConfig.backrest.extraPaths`. Validation rerun: `nix fmt` and `nix flake check` both pass.

nimmo added 1 commit

2026-08-10 15:02:51 +01:00

fix(backup): validate backup service prerequisites

Build and Push Attic Cache / test (pull_request) Successful in 1m11s

Details

Build and Push Attic Cache / build (pull_request) Has been skipped

Details

Build and Push Attic Cache / pin-lifecycle (pull_request) Has been skipped

Details

Build and Push Attic Cache / remediate (pull_request) Has been skipped

Details

AI pull-request review / review (pull_request) Successful in 6m10s

Details

adce2d3c4b

nimmo commented

2026-08-10 15:02:59 +01:00

Author

Owner

Followed up on the two advisory items in adce2d3 (fix(backup): validate backup service prerequisites).

The Backrest firewall role assertion now evaluates regardless of whether the host declares backup paths.
Enabling nixosConfig.backup.snapshots.home now asserts that /home is Btrfs; Electra's generated Disko filesystem configuration satisfies it.
No other module references Backrest's SOPS secret paths.

Validation rerun: nix fmt and full nix flake check pass.

Followed up on the two advisory items in adce2d3 (`fix(backup): validate backup service prerequisites`). - The Backrest firewall role assertion now evaluates regardless of whether the host declares backup paths. - Enabling `nixosConfig.backup.snapshots.home` now asserts that `/home` is Btrfs; Electra's generated Disko filesystem configuration satisfies it. - No other module references Backrest's SOPS secret paths. Validation rerun: `nix fmt` and full `nix flake check` pass.

nimmo added 1 commit

2026-08-10 15:38:09 +01:00

fix(backup): reject incomplete firewall intent

Build and Push Attic Cache / test (pull_request) Successful in 1m11s

Details

Build and Push Attic Cache / build (pull_request) Has been skipped

Details

Build and Push Attic Cache / pin-lifecycle (pull_request) Has been skipped

Details

Build and Push Attic Cache / remediate (pull_request) Has been skipped

Details

AI pull-request review / review (pull_request) Successful in 4m28s

Details

c227ed0432

nimmo commented

2026-08-10 15:38:17 +01:00

Author

Owner

Final robustness follow-up in c227ed0 (fix(backup): reject incomplete firewall intent).

nixosConfig.backrest.openFirewall = true now requires non-empty backup intent, so a port can never be silently requested without an enabled Backrest service.
The Btrfs assertion now treats an undefined /home filesystem declaration as a clean assertion failure instead of an attribute error.
Full nix flake check passes across all active hosts, confirming Electra's snapshot-enabled /home is explicitly Btrfs and that service mkAfter path contributions evaluate correctly.

No further code changes are needed from the automated feedback; the remaining gate is the documented live canary before merge.

Final robustness follow-up in c227ed0 (`fix(backup): reject incomplete firewall intent`). - `nixosConfig.backrest.openFirewall = true` now requires non-empty backup intent, so a port can never be silently requested without an enabled Backrest service. - The Btrfs assertion now treats an undefined `/home` filesystem declaration as a clean assertion failure instead of an attribute error. - Full `nix flake check` passes across all active hosts, confirming Electra's snapshot-enabled `/home` is explicitly Btrfs and that service `mkAfter` path contributions evaluate correctly. No further code changes are needed from the automated feedback; the remaining gate is the documented live canary before merge.

nimmo added 1 commit

2026-08-10 15:58:09 +01:00

fix(backup): restrict root Backrest access

Build and Push Attic Cache / test (pull_request) Successful in 1m11s

Details

Build and Push Attic Cache / build (pull_request) Has been skipped

Details

Build and Push Attic Cache / pin-lifecycle (pull_request) Has been skipped

Details

Build and Push Attic Cache / remediate (pull_request) Has been skipped

Details

AI pull-request review / review (pull_request) Successful in 3m42s

Details

95b765ad92

nimmo commented

2026-08-10 15:58:17 +01:00

Author

Owner

Live canary follow-up in 95b765a (fix(backup): restrict root Backrest access).

The initial Vega run exposed permission-denied reads for application-owned service state, so its snapshot was not valid coverage evidence. Backrest now runs as root to read the declared paths, with UMask=0077; its UI is hard-bound to 127.0.0.1:9898 and 9898 is no longer opened in Vega's firewall. The README documents SSH forwarding for remote administration.

Validation: nix fmt, full nix flake check, and evaluated Vega unit/firewall checks (User=root, UMask=0077, no 9898 port) all pass.

A fresh just test and successful manual backup with path listing are required before merge.

Live canary follow-up in 95b765a (`fix(backup): restrict root Backrest access`). The initial Vega run exposed permission-denied reads for application-owned service state, so its snapshot was not valid coverage evidence. Backrest now runs as `root` to read the declared paths, with `UMask=0077`; its UI is hard-bound to `127.0.0.1:9898` and 9898 is no longer opened in Vega's firewall. The README documents SSH forwarding for remote administration. Validation: `nix fmt`, full `nix flake check`, and evaluated Vega unit/firewall checks (`User=root`, `UMask=0077`, no 9898 port) all pass. A fresh `just test` and successful manual backup with path listing are required before merge.

nimmo commented

2026-08-10 16:12:19 +01:00

Author

Owner

Live canary succeeded after just test.

Backrest restarted as root and bound its UI to 127.0.0.1:9898.
Manual plan run created snapshot d72f5be2 at 2026-08-10 16:10:27 BST, owned by root@vega.
Snapshot paths are /home/nimmo, /mnt/storage/bookstack, /srv/services, /srv/stacks, and /var/lib/SillyTavern.
restic ls verified /srv/services (including Paseo and SABnzbd), /srv/stacks, and BookStack content.
The successful 16:10 run logged no permission-denied warnings.

One no-op just test follow-up remains before merge; then the PR can be merged without closing #15 until its operational handoff is complete.

Live canary succeeded after `just test`. - Backrest restarted as `root` and bound its UI to `127.0.0.1:9898`. - Manual plan run created snapshot `d72f5be2` at 2026-08-10 16:10:27 BST, owned by `root@vega`. - Snapshot paths are `/home/nimmo`, `/mnt/storage/bookstack`, `/srv/services`, `/srv/stacks`, and `/var/lib/SillyTavern`. - `restic ls` verified `/srv/services` (including Paseo and SABnzbd), `/srv/stacks`, and BookStack content. - The successful 16:10 run logged no permission-denied warnings. One no-op `just test` follow-up remains before merge; then the PR can be merged without closing #15 until its operational handoff is complete.

nimmo commented

2026-08-10 16:13:13 +01:00

Author

Owner

No-op deployment follow-up completed on Vega at 16:12 BST. just test activated successfully without restarting Backrest; backrest.service remained active on the validated root, loopback-only instance (PID 672670). This completes the required genuine deployment plus no-op follow-up evidence.

No-op deployment follow-up completed on Vega at 16:12 BST. `just test` activated successfully without restarting Backrest; `backrest.service` remained active on the validated root, loopback-only instance (PID 672670). This completes the required genuine deployment plus no-op follow-up evidence.

nimmo merged commit 08de60ccc6 into main

2026-08-10 16:13:58 +01:00