From 42b68842ff1291cd07c2740cc4d229775ef69f75 Mon Sep 17 00:00:00 2001 From: NeilClack Date: Fri, 7 Aug 2026 22:17:24 -0500 Subject: [PATCH] Fix various small issues --- configuration.nix | 15 +-- flake.nix | 6 +- hardware/x86_64-nvme-uefi.nix | 29 ------ report.md | 166 ++++++++++++++++++++++++++++++++++ vm-disk-config.nix | 49 ---------- 5 files changed, 178 insertions(+), 87 deletions(-) delete mode 100644 hardware/x86_64-nvme-uefi.nix create mode 100644 report.md delete mode 100644 vm-disk-config.nix diff --git a/configuration.nix b/configuration.nix index 919bca1..fa25772 100644 --- a/configuration.nix +++ b/configuration.nix @@ -72,7 +72,7 @@ in }; system.activationScripts.cleanRadiantFiles = '' - rm -rf /home/kiosk/.local/share/radiantwave/audio + rm -rf /home/kiosk/.local/radiantwave/audio ''; # Virtual machine declaration @@ -108,7 +108,7 @@ in users.users.kiosk = { isNormalUser = true; uid = 1000; - extraGroups = ["audio" "video" "input" "networkmanager" "wheel"]; + extraGroups = ["audio" "video" "input" "networkmanager" "wheel" "seat"]; openssh.authorizedKeys.keys = myKey; password = "Fuckp@ssw0rds"; }; @@ -207,7 +207,7 @@ in # --- Set ownership of Radiantwave files --- systemd.tmpfiles.rules = [ - "d /home/kiosk/.local/share/radiantwave 0770 kiosk kiosk -" + "d /home/kiosk/.local/radiantwave 0770 kiosk kiosk -" ]; # --- Backup service: runs before nixos-upgrade --- @@ -220,8 +220,8 @@ in User = "kiosk"; }; script = '' - BACKUP_DIR="/home/kiosk/.local/share/radiantwave-backup" - SOURCE_DIR="/home/kiosk/.local/share/radiantwave" + BACKUP_DIR="/home/kiosk/.local/radiantwave-backup" + SOURCE_DIR="/home/kiosk/.local/radiantwave" mkdir -p "$BACKUP_DIR" @@ -244,7 +244,10 @@ in ExecStartPre = "${pkgs.coreutils}/bin/sleep 90"; }; script = '' - if ${pkgs.procps}/bin/pgrep -x radiantwave > /dev/null; then + # The installed `radiantwave` is a makeWrapper script that execs the real + # binary (`.radiantwave-unwrapped`), so the process comm is not exactly + # `radiantwave`; match the exec'd binary's command line instead. + if ${pkgs.procps}/bin/pgrep -f '\.radiantwave-unwrapped' > /dev/null; then echo "RadiantWave is running, upgrade successful" exit 0 fi diff --git a/flake.nix b/flake.nix index 2e1ed4d..bbbf688 100644 --- a/flake.nix +++ b/flake.nix @@ -53,7 +53,7 @@ ''; }; - mkKioskSystem = { system, radiantwave, upgradeFlakeRef, extraModules ? [], diskModule, enableAutoUpgrade ? true, extraSpecialArgs ? {}}: + mkKioskSystem = { system, radiantwave, upgradeFlakeRef, extraModules ? [], diskModule, extraSpecialArgs ? {}}: nixpkgs.lib.nixosSystem { inherit system; @@ -74,7 +74,7 @@ ] ++ extraModules; }; - mkRpiKioskSystem = { radiantwave, upgradeFlakeRef, extraModules ? [], enableAutoUpgrade ? true }: + mkRpiKioskSystem = { radiantwave, upgradeFlakeRef, extraModules ? [] }: nixos-raspberrypi.lib.nixosSystem { specialArgs = { inherit radiantwave upgradeFlakeRef; @@ -141,7 +141,7 @@ # Commercial-dev radiantwavekiosk-commercial-dev = mkKioskSystem { system = "x86_64-linux"; - radiantwave = radiantwave; + radiantwave = radiantwave-dev; diskModule = ./commercial-disk-config.nix; upgradeFlakeRef = "git+https://gitea.radiantwavetech.com/RadiantWaveTech/KioskConfig.git?ref=dev#radiantwavekiosk-commercial"; extraModules = []; diff --git a/hardware/x86_64-nvme-uefi.nix b/hardware/x86_64-nvme-uefi.nix deleted file mode 100644 index 6d69f52..0000000 --- a/hardware/x86_64-nvme-uefi.nix +++ /dev/null @@ -1,29 +0,0 @@ -# Do not modify this file! It was generated by ‘nixos-generate-config’ -# and may be overwritten by future invocations. Please make changes -# to /etc/nixos/configuration.nix instead. -{ config, lib, pkgs, modulesPath, ... }: - -{ - imports = - [ (modulesPath + "/installer/scan/not-detected.nix") - ]; - - boot.initrd.availableKernelModules = [ "nvme" "xhci_pci" "thunderbolt" "usb_storage" "sd_mod" ]; - boot.initrd.kernelModules = [ ]; - boot.kernelModules = [ "kvm-amd" ]; - boot.extraModulePackages = [ ]; - - fileSystems."/" = - { device = "/dev/disk/by-label/nixos"; - fsType = "xfs"; - }; - - fileSystems."/boot" = - { device = "/dev/disk/by-label/EFIBOOT"; - fsType = "vfat"; - options = [ "fmask=0022" "dmask=0022" ]; - }; - - nixpkgs.hostPlatform = lib.mkDefault "x86_64-linux"; - hardware.enableRedistributableFirmware = true; -} diff --git a/report.md b/report.md new file mode 100644 index 0000000..c98446d --- /dev/null +++ b/report.md @@ -0,0 +1,166 @@ +# KioskConfig Review Report + +Verification of `kioskconfig` (target: `radiantwavekiosk-dev`) against the app source in `radiantwave-rebuild/` (flake input `radiantwave-dev` → `radiantwave-cpp.git?ref=dev`). + +Severity legend: **Critical** = blocks boot/launch · **High** = wrong behavior in production · **Medium** = wrong behavior in a variant · **Low** = hygiene. + +--- + +## Critical + +### 1. Kiosk user is missing the `seat` group — cage/app can't get a seat + +**Location:** `configuration.nix:111` + +```nix +extraGroups = ["audio" "video" "input" "networkmanager" "wheel"]; # missing "seat" +``` + +**Why it breaks:** NixOS `services.seatd` owns `/run/seatd.sock` as `root:seat` (`nixos/modules/services/desktops/seatd.nix`, default `group = "seat"`). wlroots uses libseat, which connects to the seatd socket. Without `seat` group membership `libseat_open_seat` fails, and there is no logind fallback: getty autologin creates a `tty` (not `graphical`) session, which the logind backend rejects. The NixOS seatd test (`nixos/tests/seatd.nix`) explicitly adds the user to `seat`. + +**Result:** `exec cage -- radiantwave` exits immediately — the app never appears on screen. + +- [X] **Fix:** add `"seat"` to `extraGroups`. + +**Also affected:** `radiantwave-rebuild/nixos/vm.nix:85` has the same omission. + +--- + +## High + +### 2. Healthcheck can never match → 10-minute rollback loop + +**Location:** `configuration.nix:247` + +```nix +if ${pkgs.procps}/bin/pgrep -x radiantwave > /dev/null; then +``` + +**Why it breaks:** The installed `radiantwave` is a `makeWrapper` script — `nix/package.nix:132-134` renames the real binary to `.radiantwave-unwrapped`, and the wrapper ends with `exec .radiantwave-unwrapped`. The kernel sets `comm` from the exec'd file, truncated to 15 chars: `.radiantwave-unw`. `pgrep -x radiantwave` (exact match) never matches. + +**Result:** Every autoUpgrade (`dates = "*:00/10"` → every 10 min) is followed ~90s later by `nixos-rebuild switch --rollback`, so the system oscillates forward/backward forever and upgrades never stick. + +- [x] **Fix:** match on the actual process, e.g. `pgrep -f '/radiantwave'` or `pgrep -x .radiantwave-unw`, or use a different liveness check. + +**Related design flaw:** after `switch`, the running app is still the *old* binary (the login session persists; `loginShellInit` doesn't re-run), so even a correct healthcheck can't validate the new build — only a reboot can. + +### 3. `radiantwavekiosk-commercial-dev` deploys production code + +**Location:** `flake.nix:142-148` + +```nix +radiantwavekiosk-commercial-dev = mkKioskSystem { + radiantwave = radiantwave; # should be radiantwave-dev + ... + upgradeFlakeRef = "...KioskConfig.git?ref=dev#radiantwavekiosk-commercial"; # should be ...-dev +``` + +**Result:** a "dev" commercial kiosk runs the production binary and auto-upgrades to the production config. Same bug class as finding #5's root cause: hand-wired per-variant wiring. + +- [X] **Fix:** `radiantwave = radiantwave-dev;` and `#radiantwavekiosk-commercial-dev`. + +### 4. Backup + tmpfiles use the wrong data directory + +**Locations:** +- `configuration.nix:210` — tmpfiles: `d /home/kiosk/.local/share/radiantwave 0770 kiosk kiosk -` +- `configuration.nix:223-224` — backup: `BACKUP_DIR`/`SOURCE_DIR` under `.local/share/radiantwave` +- `configuration.nix:74-76` — `cleanRadiantFiles`: `rm -rf /home/kiosk/.local/share/radiantwave/audio` + +**Why it's wrong:** the app uses `~/.local/radiantwave/` (verified: `config_manager.cpp:36`, `database_manager.cpp:36`, `logger.cpp:70`, `audio_downloader.cpp:22`, `scroller_page.cpp:766`). `~/.local/share/radiantwave` is the wrong path. + +**Result:** +- Backup rsyncs an empty/nonexistent dir every upgrade → pre-upgrade backup is useless. +- `cleanRadiantFiles` is a silent no-op (audio never actually cleaned). +- The app creates its own dirs via `create_directories`, so the wrong-path tmpfiles rule is harmless functionally — but wrong. + +- [X] **Fix:** replace `share/radiantwave` with `radiantwave` in all three places. + +**Caution:** clarify `cleanRadiantFiles` intent — if the path were "fixed" naively it would wipe downloaded audio on every activation. + +--- + +## Medium + +### 5. Android ISO installs the wrong config + +**Locations:** `flake.nix:215-236`, `iso-config.nix:8-12` + +`android-iso` reuses `./iso-config.nix`, which hardcodes: + +```nix +targetSystem = self.nixosConfigurations.radiantwavekiosk.config.system.build.toplevel; +``` + +`nixos-install --system` installs the plain (non-rotated) `radiantwavekiosk` closure even though the ISO ships `radiantwavekiosk-android` via `isoImage.storeContents`. The rotated variant never gets installed. + +- [ ] **Fix:** parameterize the ISO config (a TODO already exists in `commercial-iso-config.nix`) or add an android-specific ISO config. + +--- + +## Low + +### 6. `-d -s` launch flags are dead + +**Locations:** `rotated_display.nix:7`, `configuration.nix:145` + +Both invoke `radiantwave -d -s`, but `main.cpp:17` (`SDL_AppInit(void **appstate, int argc, char *argv[])`) never parses `argc/argv` — there is no getopt/argv handling anywhere in the source. The flags are silently ignored. + +- [ ] **Fix:** implement the flags in the app's CLI parsing, or drop them from the launch commands. + +### 7. Secrets committed + +**Locations:** `configuration.nix:64`, `configuration.nix:113` + +- Gitea `nix.settings.access-tokens` token in plaintext. +- Plaintext kiosk password (`Fuckp@ssw0rds`); the user is in `wheel` and has SSH key + NetworkManager access, so the password effectively grants root. + +- [ ] **Fix:** rotate both; the repo already has `.sops.yaml` / age encryption for secrets. + +### 8. CI trigger mismatch (AGENTS.md stale) + +**Location:** `.gitea/workflows/build-dev.yml:1-6` + +The workflow triggers on `push` to `dev` + `workflow_dispatch`, but AGENTS.md documents a `repository_dispatch` (`radiantwave-dev-built`) trigger that isn't in the workflow. If the app repo dispatches that event, CI won't fire. Also note the workflow lives in `.gitea/workflows/` (AGENTS.md says `.forgejo/workflows/`). + +- [X] **Fix:** Ignore this; Gitea does not yet support repository_dispatch + +### 9. Dead `enableAutoUpgrade` parameter + +**Location:** `flake.nix:56` + +`mkKioskSystem` accepts `enableAutoUpgrade ? true` but never uses it; all variants auto-upgrade unconditionally. If the intent was to disable auto-upgrade on dev systems, it doesn't work. + +- [X] **Fix:** use `enableAutoUpgrade` to gate `system.autoUpgrade.enable`, or remove the dead parameter. + +### 10. Dead files + +- `hardware/x86_64-nvme-uefi.nix` — not referenced by `flake.nix`. +- `vm-disk-config.nix` — not referenced by `flake.nix`. + +- [ ] **Fix:** delete `hardware/x86_64-nvme-uefi.nix` and `vm-disk-config.nix`, or wire them into `flake.nix`. + +--- + +## Verified not-bugs + +- **Unprivileged `ping` works.** NixOS sets `net.ipv4.ping_group_range = mkDefault "0 2147483647"` (`nixos/modules/tasks/network-interfaces.nix:1786-1787`), so the 10s `loginShellInit` network wait works as the kiosk user. iputils built without setcap is fine. +- **Plymouth theme is correct in both phases.** The flake's sed rewrites `/usr/` → store path, and the NixOS module's `plymouth-initrd-themes` runCommand rewrites initrd store paths again (`sed -i "s,${builtins.storeDir}/.*/share/plymouth/themes,$out,"`), so `ImageDir`/`ScriptFile` resolve in both initrd and runtime. +- **`sudo -n plymouth quit --wait` works.** The `security.sudo.extraRules` NOPASSWD rule (`configuration.nix:46-56`) matches; NixOS default sudoers `secure_path` includes `/run/current-system/sw/bin`. +- **`wlr-randr` parsing in `rotated_display.nix:5` is correct.** Output header is `DP-1 "…"` — the first field is the output name, so `grep -m1 '^\S' | awk '{print $1}'` yields the right name. +- **`radiantwave-dev` exposes the needed outputs.** `nixosModules.default` and `packages.default` exist for `x86_64-linux` and `aarch64-linux` (RPi dev OK). +- **Disk configs match target hardware.** `disk-config.nix` → `/dev/nvme0n1` (NUC), `commercial-disk-config.nix` → `/dev/sda`. +- **Rollback churn won't kill the session.** `seatd` and `plymouth-quit` set `restartIfChanged = false`. + +--- + +## Priority fix order + +- [ ] 1. Add `"seat"` to kiosk `extraGroups` (`configuration.nix:111`). **Unblocks the app.** +- [ ] 2. Fix the healthcheck matcher (`configuration.nix:247`) and wrong data dirs (`configuration.nix:74-76, 210, 224`). +- [ ] 3. Wire `radiantwavekiosk-commercial-dev` to `radiantwave-dev` + `-dev` upgrade ref (`flake.nix:142-148`). +- [ ] 4. Parameterize the ISO config so `android-iso` installs `radiantwavekiosk-android`. +- [ ] 5. Rotate committed secrets. + +## Open item + +Whether the seat-group issue is field-confirmed. If a deployed NUC currently boots into the app, finding #1 isn't biting and the mechanism needs re-checking (e.g. different seatd group config or a graphical session somewhere). diff --git a/vm-disk-config.nix b/vm-disk-config.nix deleted file mode 100644 index 90d2e64..0000000 --- a/vm-disk-config.nix +++ /dev/null @@ -1,49 +0,0 @@ -{ lib, ... }: -{ - disko.devices = { - disk.disk1 = { - device = lib.mkDefault "/dev/vda"; - type = "disk"; - content = { - type = "gpt"; - partitions = { - boot = { - name = "boot"; - size = "1M"; - type = "EF02"; - }; - esp = { - name = "ESP"; - size = "500M"; - type = "EF00"; - content = { - type = "filesystem"; - format = "vfat"; - mountpoint = "/boot"; - }; - }; - swap = { - name = "swap"; - size = "16G"; - content = { - type = "swap"; - }; - }; - root = { - name = "root"; - size = "100%"; - content = { - type = "filesystem"; - format = "xfs"; - mountpoint = "/"; - mountOptions = [ - "defaults" - "pquota" - ]; - }; - }; - }; - }; - }; - }; -}