Repository review — 13 September 2026

Reviewed revision: `6a461267a2121216a0ecb586b4e4b655085038ab`.

Second-pass scope clarification: camera isolation is excluded from the action list. The vendor's cloud app must continue working. `devices.toml` already leaves isolation disabled and documents why; the original description of the camera as currently internet-blocked was incorrect. Resource findings below identify work and contention worth measuring, not measured CPU, RAM or energy savings.

The architecture is worth keeping: one flake, host-specific hardware, mostly native services, a central builder, and a shared desktop. The main problems are incomplete failure boundaries and duplicated operational policy. More abstraction alone will not fix those. The largest likely resource savings are in scheduled work and storage maintenance, with smaller opportunities in the desktop shell.

This is a configuration and code review, not a live penetration test or performance measurement. Findings describe what this revision declares; manually configured service state and the active firewall may differ. I did not switch generations, restart services, update inputs, access runtime secrets, or modify configuration. Runtime wattage, RAM consumption and externally reachable ports still need measurement.

Validation: all four physical-host system derivations evaluated successfully offline (Gaming, Mediaserver, Macbook and Kayla). The existing router UI self-check passed, and the standalone shell scripts passed Bash syntax checking. Two additional in-memory validation probes confirmed the destination-validation issues below. `nix flake check --offline --no-write-lock-file --no-build` failed on `mediaserver-vm`, after checking the physical hosts. No complete system builds or GUI/VM boot tests were run. Mediaserver evaluation also warned that the custom Redlib package lacks `meta.mainProgram`.

**1. High priority: restore desktop firewall startup.**

[common.nix:155](common.nix#L155) forces `systemd.services.firewall.wantedBy = []` and orders it after `multi-user.target`. Evaluation confirms the empty dependency list on Gaming, Macbook and Kayla. The pinned iptables firewall module normally starts from `sysinit.target`, before networking. This override removes that normal boot activation; ordering a unit does not start it. It also conflicts with the intended early-network ordering if another dependency pulls it in.

Remove the common override. The server already explicitly disables the scripted firewall and supplies nftables. Keep that exception on the router. Verify the client firewall after a cold boot, especially on the Macbook away from home; a currently running firewall after a switch does not prove it starts on boot.

**2. High priority: Docker bypasses the intended WAN port inventory.**

[router.nix:225](services/router.nix#L225) accepts every WAN-forwarded packet with `ct status dnat`. That includes DNAT created by Docker, not only the rules generated from `ports.toml`. [pelican.nix:126](services/pelican.nix#L126) publishes `2022:2022` on all host addresses while claiming that its absence from `ports.toml` makes it LAN-only. The declared rules permit that Docker-published SFTP port from WAN, assuming the container is running and no independent upstream filter blocks it.

Bind private container ports explicitly and enforce the WAN allowlist on forwarded traffic too, accounting for original destination/port before translation. Test both an allowed port and an unlisted Docker-published port from a separate WAN-side test node. Merely removing entries from `ports.toml` is insufficient. This follows Docker's documented [port publishing behavior](https://docs.docker.com/engine/network/port-publishing/) and the repository's blanket DNAT acceptance.

**3. Withdrawn from the action list: camera isolation.**

[router.nix:40](services/router.nix#L40) describes an optional forward-chain MAC drop as providing VLAN-like isolation. That description is technically inaccurate: devices in the same `10.0.0.0/24` LAN can communicate directly through the switch/access point. However, [devices.toml](devices.toml) does not enable this rule and explicitly records its removal because it broke the vendor cloud tunnel. The earlier review incorrectly described the optional rule as active.

Preserve the working camera configuration. A separate subnet that permits required internet access while restricting local access is a possible future design, but compatibility with this app and the available network hardware has not been established. No camera networking change is recommended in this review. The loopback-only go2rtc configuration remains useful.

**4. High priority: router deployment is not reliably reversible.**

[router-ui.nix:50](services/router-ui.nix#L50) captures the old running system, then invokes `nixos-rebuild test`. Any nonzero result is described as a build failure with nothing activated. Activation can itself fail after partial changes, so this branch can leave a partially changed machine without rollback.

The subsequent health test checks WAN ping, dnsmasq process state, the LAN address and existence of a NAT table. It does not verify DNS resolution, actual LAN forwarding, nginx/Authelia access, or management reachability. Its rollback uses the mutable system profile rather than the captured `$old`. The pinned NixOS activation implementation already locks individual activations, but there is no lock covering the complete test → health-check → commit/rollback transaction. Another deployment can intervene between those steps and invalidate the assumed rollback target. A failed final switch is also not proof that the boot default is unchanged.

Build an exact revision first, capture and root the old closure, serialize all activation paths, and arm an independent rollback timer before activation. Activate the already-built closure and disarm rollback only after meaningful probes pass. Include a LAN-side test client where possible. Use the captured old closure for recovery and verify rollback success. The nightly server upgrade should use the same safeguards; currently it bypasses this workflow entirely.

**5. High priority: lockscreen startup relies on an unreliable session marker.**

[desktop.nix:8](settings/desktop.nix#L8) logs straight into fred's session; [quickshell.nix:754](settings/quickshell.nix#L754) locks only when creating `$XDG_RUNTIME_DIR/qs-boot-lock` succeeds. That runtime directory belongs to the user, not exclusively to one graphical session. Another login can keep it alive across compositor logout/restart, leaving the marker present and the new autologin session unlocked. A shell error before requesting the lock is another uncovered startup path.

[hyprland.nix:74](settings/hyprland.nix#L74) sends a lock request and sleeps for one second before suspend. It does not await compositor confirmation. Use a genuine greeter authentication boundary, or an independently supervised lock flow that verifies acquisition; separate it from the frequently edited bar. Quickshell exposes `WlSessionLock.secure` for compositor-confirmed coverage. An already-acquired lock failing is a different case: conformant compositors keep it locked, as the [Quickshell documentation](https://quickshell.org/docs/v0.2.1/types/Quickshell.Wayland/WlSessionLock/) explains. This finding concerns startup and acquisition, not a claim that killing an acquired lock exposes the desktop.

The suspend failure path is concrete: the generated script has no `set -e` or explicit IPC-error handling, so a failed `qs ipc` followed by successful `sleep 1` reports success. The unit is also only wanted by `sleep.target`; ordering alone is not a requirement that locking succeed. Correct failure propagation and require confirmed coverage before allowing suspend.

**6. High priority if this is the recovery method: backup coverage and failure handling lag behind the services.**

[backup-server.sh](backup-server.sh) backs up only seven application directories and secrets. It omits active state such as Pelican panel/worlds and `/etc/pelican`, Flatnotes, Shelfarr, Profilarr, SABnzbd and Seerr. Its `/mnt/disk4` destination is not declared in the hardware configuration and is not checked to be mounted. A missing backup disk can therefore turn the backup directory into a directory on the root filesystem.

If rsync fails under `set -e`, services remain stopped because restart is not in a trap. Conversely, stop failures are ignored, allowing an inconsistent live database copy. The script also starts services that might have been intentionally stopped beforehand.

Create an explicit state inventory, automate backups to a verified independent destination, and test restoration. Preserve prior service state and always restore it on failure, or use application-consistent database backups/snapshots. Include secret recovery. No scheduled backup or restore test is declared here; that does not establish whether you run one externally. Mergerfs pooling itself provides neither redundancy nor backup.

**7. Medium/high priority: storage failures should not disable the household router.**

[FredOS-Mediaserver hardware:28](hosts/hardware/FredOS-Mediaserver.nix#L28) makes the three media disks required boot mounts. A failed media disk can obstruct normal boot of the router. The mergerfs mount also lacks explicit dependencies on its three branch mounts: paths embedded in its device string are not a reliable substitute for mount dependencies.

Declare branch dependencies and require the pool for services that write media. Decide deliberately whether a missing branch should prevent the pool from mounting or permit degraded service. If disks become optional for boot, also prevent media services and tmpfiles rules from writing into bare mountpoint directories on the root disk. Make router/DNS startup independent of the media pool. Do not simply add `nofail` everywhere without those guards.

**8. Medium priority: turn nightly prebuilding into release publication.**

[prebuild.nix](services/prebuild.nix) and [common.nix:76](common.nix#L76) both build a moving remote branch. Clients do not actually read the nightly `/var/lib/prebuild/<host>` result: they run a fresh server-side `nix build --refresh`, with a separate output link in fred's home. It is a cache hit only if the current revision's closure is already available. An intervening push turns a supposedly copy-only update into another evaluation/build.

This does not mean every update recompiles all packages: unchanged derivations are reused, and an unchanged revision can be cheap. Publishing a fixed result improves the guarantee of fast, validated updates; it is not evidence that the existing system always does substantial duplicate compilation.

The schedules are also misaligned. The lock updater is scheduled for 04:00 UTC; the host's timers use Europe/London. During British Summer Time, the Macbook prebuild at 04:00 local precedes that day's lock update, while the Gaming job at 05:00 races it. The server upgrades at 05:15. One-hour staggering does not guarantee non-overlap, and persistent timers can bunch together after downtime.

Use one pipeline: update candidate inputs, evaluate/test, build the required host closures at the exact same revision, then atomically publish a manifest containing revision and per-host store paths. Clients copy their published path. Keep the previous successful manifest if any required build fails. For three machines, SSH plus a small manifest and GC roots is enough; a large deployment framework is optional. Remove `git commit --allow-empty` in the updater so unchanged inputs do not produce meaningless revisions. Separate candidate updates from promotion to the server's deployable branch/ref.

**9. Medium priority: resource budgets belong on the actual builders.**

Evaluated server settings are `max-jobs = "auto"` and `cores = 0`, with no explicit build CPU or memory budget in the repository. Many jobs and multithreaded compilers can contend with DNS, media and game servers. Prebuilds, the 05:15 upgrade, weekly subtitle syncing and the runner may overlap. Zram helps absorb pressure but does not reserve responsiveness for essential services.

Choose conservative concurrent-job and per-job core limits based on measured RAM and build peaks; prioritize router/DNS service cgroups and lower background build/transcode priorities. Apply limits to the daemon/build execution cgroups, not just the `nix build` client unit, because the daemon starts the compilers. Account separately for the runner's container-local Nix installation. Start with pressure and duration measurements rather than arbitrary hard memory caps that can kill valid builds.

The CI container installs Nix afresh just to update the lock, with no persistent Nix-store mount declared here. A maintained runner image containing Nix plus a suitable cache would remove repeated setup. A host runner is simpler but gives workflow code host-level authority; choose that tradeoff explicitly.

**10. Medium priority: remove routine recursive permission repair.**

The qBittorrent, SABnzbd, Sonarr, Radarr and server-permissions modules use recursive `Z` tmpfiles rules on whole library/download trees. They walk every descendant and enforce one ownership/mode policy, including regular files. This is recurring metadata I/O across mergerfs/HDDs, and the numeric modes can also add unnecessary executable bits to data files or broaden access to app-state files.

Keep directory creation, setgid directories, shared-group access and sane umasks. Perform recursive repairs explicitly as migrations, using separate directory/file modes where needed. Measure `systemd-tmpfiles` duration and disk activity before and after. This is a stronger initial optimization target than shaving a minute-scale UI timer.

**11. Medium priority: signed closures can replace globally disabled verification.**

[common.nix:257](common.nix#L257) disables signature requirements on every non-server host to accept copied results. That broadens trust for all such imports, not just your server. Sign server-built closures and configure its public signing key on clients while retaining normal signature requirements. SSH remains useful for transport. The [Nix configuration reference](https://nix.dev/manual/nix/2.33/command-ref/conf-file) documents signature checking and trusted keys.

Separate administrative SSH access from build/copy access where practical. Copying fred's private key into root's SSH directory couples credential rotation and gives the builder connection the authority of the personal account. Also, `max-jobs = 0` on Macbook means `update2` is not a complete server-down local-compilation fallback. Document a deliberate emergency override, and verify that root's builder SSH host verification/key setup actually works.

**12. Medium priority: container versions escape the flake lock.**

Pelican, Shelfarr, Profilarr, Flatnotes and bazarr-sync use mutable `latest` tags. The pinned NixOS OCI module defaults to pull policy `missing`, so existing images can stay old while a fresh machine fetches newer code from the same Nix revision. Neither a nightly Nix build nor reverting `flake.lock` captures those image contents or reverses database migrations.

Pin image digests and update them through a tested workflow. Back up state before migrations. Preserve explicit image retention alongside system generation retention. Running Docker for Pelican and Podman for subtitle jobs is not itself a major waste—Podman is daemonless—but using one engine would simplify network, image and cleanup policy if there is no isolation reason for both.

**13. Medium priority: router UI validation does not enforce its advertised constraints.**

[router-ui.py:378](scripts/router-ui.py#L378) validates port syntax but not destination addresses. Names are interpolated into nftables quoted comments without suitable restrictions/escaping; destinations are inserted as raw nftables text. It also considers SSH/HTTPS protected merely because their port numbers occur, even if redirected to another host. In-memory probes accepted both `not-an-ip` as a destination and moving TCP/22 to `10.0.0.99`.

Validate names, IP addresses, permitted destination ranges, duplicate/overlapping forwards and the actual management destination. Apply validation at the Nix/TOML boundary as well as in the UI, because direct git edits bypass Python. Add tests for these cases. For the HTTP API, validate request content type/origin and use explicit CSRF protection; its current handler accepts JSON bodies without those checks. Authentication at nginx is useful but should not be treated as input validation.

**14. Medium priority: separate public service convenience from admin authority.**

All Authelia access rules are `one_factor`, despite documentation describing an SSO/2FA gateway. Code-server runs as fred with its own auth disabled, and fred has unrestricted passwordless sudo; compromise of that web session therefore reaches full server/router administration. Prefer stronger authentication and tighter access for code, router and game administration. Restore meaningful backend authentication or restrict listeners wherever LAN access currently bypasses the proxy.

Owncast reuses the guest viewing password as the administrator password and stream key, while nginx exempts admin endpoints from its outer gate. Sharing viewing access therefore also shares administration. Give administration and ingest separate runtime secrets. The deliberate lack of chat-socket authentication should also be an explicit product decision, not protection by an obscure hostname. I have not reproduced credentials in this review.

**15. Medium priority: the transcoder can overwrite unrelated files and increase disk use.**

[transcode-hevc.sh:235](scripts/transcode-hevc.sh#L235) converts `name.mp4` to `name.mkv` and moves it over any existing file at that destination without a collision check. Successful output replaces the source even if it is larger. Duration-only validation does not verify retained streams, attachments or successful full decoding; the mapping excludes attachments, which can include subtitle fonts. Replacing the inode can change ownership/mode from the original file.

Use unique temporary files, refuse destination collisions, retain original metadata, require meaningful size savings, and verify streams before replacement. Lock concurrent runs: the shared `/tmp` size list and blanket cleanup of `.transcode-*` can conflict with another encoder. qBittorrent lookup failure currently disables seeded-file protection rather than stopping; decide whether that is acceptable before destructive processing. File size alone is only a heuristic for matching torrent data.

Software x265 can consume substantial CPU, and HEVC may make playback harder on the Haswell laptop than H.264. Measure real playback and total encoding energy before batch conversion; the presence of an NVIDIA transcoding card does not accelerate this script's `libx265` encoding.

**Quickshell: keep the design, reduce duplication and measure wakeups.**

The shell is not simply a collection of aggressive polling loops. It uses UPower/PipeWire/MPRIS integration, NetworkManager monitoring with a slow fallback, visible-only ANC polling, bounded notification history, and releases the unused wallpaper texture after transitions. Those are good decisions.

The clearest remaining targets are:

- Cava continues capturing during any media playback even with the calendar closed. Its output is parsed into JavaScript arrays before the visibility check, although publication is correctly gated to avoid hidden repainting. On Macbook, stop it while hidden or at least return before parsing hidden frames. Keeping it warm on Gaming can remain an explicit visual preference.
- Each screen instantiates a complete Bar, including weather retrieval, network monitoring and cava. Move shared data/processes into singleton services; keep per-screen rendering separate. Otherwise connecting an external monitor duplicates background work.
- Wallpaper and lockscreen images lack explicit decode-size caps. Generate sensible display-sized assets or bound `sourceSize` while preserving aspect ratio. Full-resolution source decoding and blur textures can matter on the laptop's shared memory.
- The full-screen bar shader makes frequent invalidation worth profiling. Compare idle, music with popup closed, popup open, and multiple displays. Do not assume the expensive hidden repaint issue remains: this revision already gates publication.
- `apps/zen.nix` checks the theme file every two seconds. This is small but continuous work for an infrequent event. Consider a supported file-change mechanism or a slower interval, and measure before making the integration more complex.

For maintainability, extract the approximately 7,300 lines of embedded QML into ordinary component files, leaving Nix to generate a small settings/command module. Separate lockscreen, launcher, bar components and shared service models. Move sizeable shell/Python helpers out of Nix strings so their syntax, formatting and tests are easier to run. This reduces change risk and improves tooling; it does not inherently reduce runtime memory.

**Other maintenance changes worth making.**

- Fix the VM output shape. `mediaserver-vm` currently fails ordinary `system.build.toplevel` evaluation because root filesystem/bootloader configuration exists only in the VM variant. Export runnable VM derivations under suitable package/check outputs, or make the configurations valid in their exposed form. The current failure is not proof that `build-vm` itself fails. The VM also disables the router rules, so it cannot test WAN firewall behavior; add a separate router test topology.
- Build tests around the actual failure boundaries: denied WAN Docker ports, invalid TOML, unsuccessful activation, pool unavailability and fresh media-service setup. Existing Python tests are useful but currently cover only a subset, and fixture counts are tied to today's personal device inventory.
- Use explicit roles/profiles instead of repeated hostname membership lists. For three hosts, simple imports such as base, desktop, media and router are sufficient. Move Docker enablement out of the nominally disabled game-server module. Keep hardware separate as it is now.
- The exported reusable gaming module disables distributed builds and removes SSH keys, but still inherits the `update` alias contacting your server and disabled signature checks. Parameterize deployment independently of the desktop profile.
- Replace `clean = nix-collect-garbage -d` as the routine cleanup path with a retention policy. It deletes old system generations and sacrifices rollback history. Keep a bounded number/age of known-good server and client release roots; boot-menu limits alone do not implement store retention.
- Scope the global ten-second stop timeout. Stateful services and game saves need enough shutdown time; inspect per-service overrides rather than assuming they all inherit ten seconds. A faster reboot is not worth interrupted persistence.
- Extend service monitoring to failed prebuilds, upgrades, backups, DHCP and reconciliation, and add application probes. A process being active is not proof that DNS or HTTP works. An external monitor is needed to report when the router itself cannot send ntfy notifications.
- Review subtitle scheduling: the hourly Radarr selection uses movie-added time rather than download/import time, so a movie downloaded long after being added is missed until the weekly sweep. The weekly and hourly jobs can overlap, and errors frequently become successful service exits. Use import events/history or durable incremental tracking, a common lock and meaningful failure status.
- Treat reconciliation failures as failures. `arr-interconnect` extracts keys before readiness checks, skips many unavailable components and can exit successfully with `RemainAfterExit`; its restart policy will not repair skipped work. It needs explicit convergence reporting and retries for incomplete first-run state. It also overwrites existing non-primary Seerr users' permissions wholesale; separate defaults from deliberate management of existing users.
- Refresh documentation: client deployment commands, service inventory, CrowdSec patch status, backup destinations, first-run secrets, and Kayla's placeholder hardware are not consistently described. Record local patches with pinned upstream versions, a reason and a condition for removal.

**Suggested order of work.**

First restore firewall startup, close the Docker allowlist gap, address lock acquisition and protect router activation/backup recovery. Then consolidate nightly work around exact published revisions, add resource budgets and replace recursive permission repair. Next pin containers and add targeted VM tests. Finally extract the QML/services and profile desktop optimizations.

Keeping routing on the server is workable, but it couples household connectivity to kernel updates, hardware faults, storage boot failures and development activity. A separate small router is the strongest way to remove that coupling if outages are disruptive. A router VM on the same server does not protect against host reboot or hardware failure. I would resolve the concrete software faults and measure idle power before deciding whether hardware separation is worth the cost.