529039dc87
- splash no longer does the 5.5s slow-scroll flex - apt-get update only runs if lists are >6h old (-r/--refresh to force) - log() fork-free, debug spam gated behind ULTRABUNT_DEBUG=1 - category menu counts in one pass, no more dpkg fork storm per redraw - deleted refresh_cache_silent (600 background forks doing literally nothing) - rc-state packages no longer show as installed; flatpak exact-match - bulk ops no longer die after first package (((x++)) set -e trap) - bulk checklists no longer scramble themselves when sorting - ESC no longer nukes the whole script in buntage/log-viewer menus - swappiness menu: fixed regex crash + duplicate sysctl lines - wp hardening: domain validated (no more chown -R /var via ".."), options actually apply now - fun category (15 buntages) rescued from the void - --no-media/--no-web/--no-network/-minimal/-core now exclude real categories REVIEW.md has the full findings list incl. everything not fixed yet Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
125 lines
16 KiB
Markdown
125 lines
16 KiB
Markdown
# Ultrabunt Code Review — 2026-07-17
|
||
|
||
Full review of `ultrabunt.sh` (17k lines) and `ultrabunt-accessible.sh`.
|
||
Scope: real breakage and danger only — style ignored, silliness preserved.
|
||
|
||
Line numbers are approximate (the performance pass shifted things by a few
|
||
dozen lines in places) — search for the quoted function name if a number is off.
|
||
|
||
---
|
||
|
||
## Fixed in this pass (performance + crashers)
|
||
|
||
These are already applied to `ultrabunt.sh`:
|
||
|
||
| What | Why it was slow/broken |
|
||
|---|---|
|
||
| Splash screen printed instantly | 78 lines × `sleep 0.05` + `sleep 1.5` = ~5.5s of dead air at every launch |
|
||
| `apt-get update` skipped when lists < 6h old (`--refresh` to force) | Ran unconditionally at startup: 10–60s per launch on old hardware |
|
||
| `log()` rewritten fork-free (`printf %(...)T` + direct append) | Old version forked ~3 processes per line (`$(date)`, pipe, `tee`) and was called per package per menu redraw; also sprayed stderr underneath whiptail. Set `ULTRABUNT_DEBUG=1` to mirror logs to stderr |
|
||
| Removed per-check logging from `is_package_installed` | 3+ log lines (≈9 forks) per package, per menu redraw |
|
||
| Category counts computed in ONE pass over `PACKAGES` | Old loop re-walked all ~600 packages once per category (~15,000 iterations) on every main-menu redraw |
|
||
| `is_apt_installed` cache miss is now authoritative after startup scan | Old fallback forked `dpkg -l` for every NOT-installed package on every redraw — hundreds of forks per screen |
|
||
| `build_package_cache` always does one full dump per package manager | "Selective mode" forked one `dpkg-query` per package — strictly slower than dumping everything (which is 1 fork) |
|
||
| Deleted `refresh_cache_silent` + the 🔄 indicator | It forked ~600 background processes whose cache writes happened in a **child shell and were thrown away** — a subshell can't modify the parent's `INSTALLED_CACHE`. 100% wasted CPU on a machine that has none to spare |
|
||
| Removed `log "Menu items array contents: ..."` | Wrote the entire ~5KB menu array to the log on every redraw |
|
||
| dpkg checks filter on `Status-Status == installed` | `dpkg-query -W` also lists removed-but-not-purged (`rc`) packages, so uninstalled packages showed ✓ Installed |
|
||
| Flatpak checks use exact match (`grep -Fxq` on the app-id column) | Bare `grep -q "$pkg"` matched substrings |
|
||
| `((installed++))`-style counters → `x=$((x+1))` (6 sites in bulk ops) | `((x++))` returns exit 1 on the 0→1 increment; under `set -e` every bulk install/remove **killed the whole script after the first package** |
|
||
| Bulk-select checklists sort names first, then build triples | Old code joined tag/desc/OFF triples into lines and sorted ALL lines, scrambling the checklist alignment |
|
||
| `show_buntage_actions` menu guarded with `\|\| return`; install/reinstall success message now conditional on actual success | Cancel killed the script via `set -e`; failures reported "installed successfully!" |
|
||
| `view_log_file` menu guarded with `\|\| return` | ESC/Cancel in the log viewer killed the whole script |
|
||
| Swappiness regexes fixed (`vm\\.swappiness` → `vm\.swappiness`) + no-match grep guarded | The `\\.` inside single quotes is a literal backslash: "show entry" **crashed the script** and a duplicate `vm.swappiness` line was appended to sysctl.conf on every persist |
|
||
| `wordpress_security_hardening`: domain validated, wp-config.php required | Entering `..` made `site_dir=/var/www/..` = `/var`, then "Fix File Permissions" ran `chown -R www-data:www-data /var` — **system-wrecking** |
|
||
| Hardening checklist strips whiptail's literal quotes | No case arm ever matched `"file-permissions"` (with quotes), so every hardening option was silently skipped while reporting success |
|
||
| `fun` category added to `CATEGORIES` | 15 packages were tagged `fun` but the category wasn't in the menu — permanently invisible |
|
||
| `--no-media/--no-web/--no-network/-minimal/-core` use real category ids | They referenced ids that don't exist (`multimedia`, `music`, `web-browsers`, `network`, `education`…), so the exclusions silently did nothing |
|
||
| Package-list sort uses `mapfile` instead of unquoted `$(sort)` | Glob chars in descriptions (`(_*_)`) could expand against files in the cwd |
|
||
|
||
Expected result: startup goes from ~40–70s to a few seconds (splash + one dpkg/snap/flatpak dump), and menu redraws go from seconds of fork-storm to near-instant pure-bash.
|
||
|
||
---
|
||
|
||
## Outstanding — HIGH (dangerous or feature completely broken)
|
||
|
||
### WordPress / database
|
||
- **`finalize_wordpress_setup`**: custom setup's user-entered `site_dir` is ignored by the actual installers (they all install to `/var/www/$domain`) but the *finalizer still runs* `chown -R $(whoami):www-data "$site_dir"` + recursive chmod on the unvalidated path. Enter `/home` and it re-owns it. Validate/derive `site_dir` the same way the installers do.
|
||
- **SQL injection everywhere a password/name is interpolated into `mysql -e "... IDENTIFIED BY '$pass'"`** (`setup_wordpress_database*`, `create_database_user_interactive` ~L16381, DB repair `eval` ~L13870). A single `'` in a password breaks or injects as MariaDB root. The repair path pipes it through `eval` too. Fix: validate names as `[A-Za-z0-9_]+`, pass passwords via `--defaults-extra-file` or here-doc `SET PASSWORD` with proper quoting.
|
||
- **`setup_wordpress_database_custom` runs under command substitution but its whiptail dialogs aren't redirected to `/dev/tty`** — dialogs are captured into the result variable instead of being shown; flow appears hung and wp-config gets garbage credentials.
|
||
- **wp-config salts**: the awk insert anchors on `/** MySQL settings` which WordPress renamed in 5.8 — the sed first *deletes* all AUTH_KEY…NONCE_SALT lines, then the insert never fires → **modern WP sites end up with no security keys at all**, silently.
|
||
- **`setup_nginx_ssl`** writes the 443 block to `sites-available/$domain` while `.localhost` sites were created under the stripped base name → new file is never enabled; reports success, HTTPS never serves. Also appends a duplicate 443 block on every rerun (`tee -a`).
|
||
- **phpMyAdmin installer** writes the MariaDB root password to predictable `/tmp/mysql_defaults_phpmyadmin.cnf` (pre-creatable by any local user; 644 window before chmod), and the follow-up `mysql --defaults-file` runs as the wrong user so it silently no-ops. Root password also logged in plaintext to `/var/log/ultrabunt.log` (~L10883) and wp-config gets loosened to 0664 (group-writable by www-data, world-readable).
|
||
- **`delete_wordpress_site`** reads `/root/.mysql_root_password` *without sudo* → DB/user never actually dropped while reporting "completely removed including database"; also deletes the nginx vhost matching the *first label* of the domain (deleting `shop.example.com` can nuke `shop.localhost`'s config), and will DROP a database shared by other sites without cross-checking.
|
||
- **`grant_file_access_to_user`**: `chmod -R 2775 /var/www` makes every *file* group-writable+setgid and world-readable — www-data can rewrite all PHP (persistence vector) and wp-config passwords are world-readable. Use 664 files / 2775 dirs.
|
||
- **`show_individual_site_management`** uses `$site_dir` (never set locally — dynamically inherits the caller's *last* loop value) instead of the `site_root` it computes → wp-config view/edit/DB-info act on the **wrong site** whenever >1 site exists.
|
||
|
||
### Cloudflare / firewall
|
||
- **`cloudflare_create_rule` builds its JSON with raw printf and no escaping** — every filter expression contains quotes (some contain newlines) → malformed JSON → **every rule creation fails**, including "light". The whole feature can't work as written. Also uses the deprecated legacy `firewall/rules` endpoint, and the country code `UK` should be `GB` (would block UK visitors incl. you).
|
||
|
||
### Mirror management
|
||
- **The entire "fastest mirror" feature is dead**: it invokes `mirrorselect` (a Gentoo tool) which is never installed (`install_mirrorselect` actually installs `netselect-apt`). The auto mode uses netselect-apt's **Debian** mirror list and seds those hosts over `archive.ubuntu.com` → apt breaks. The "restore" path recomputes `$(date +%s)`-style backup names so it restores from a filename that doesn't exist and dies under `set -e` with sources.list already clobbered. `use_local_mirror`/`setup_local_mirror` write to `/etc/apt/` **without sudo** → instant permission-denied crash. On 24.04+ the real sources live in `/etc/apt/sources.list.d/ubuntu.sources` anyway. Recommend: rip this section out or rewrite around `netselect-apt`-for-Ubuntu with sudo and proper backup naming.
|
||
- `run_mirrorselect_custom` does `eval "sudo mirrorselect $custom_flags"` on free-typed input — arbitrary shell as root by design.
|
||
|
||
### Reverse-proxy add-ons
|
||
- **Umami**: publishes `3000:3000` on 0.0.0.0 (Docker's iptables bypass UFW) with hardcoded `APP_SECRET: replace-me-with-a-random-string` and default admin/umami login → internet-reachable with forgeable sessions on any VPS.
|
||
- **Mailcow**: mailcow's own nginx defaults to 80/443 (collides with host nginx) while the proxy snippet assumes `127.0.0.1:8080` which nothing configures — either compose fails or the proxy points at a dead port; completion message advertises `https://` URLs no config provides.
|
||
- Both add-on flows have zero error handling: any failed step under `set -e` kills the whole script with containers half-started and nginx confs already symlinked.
|
||
|
||
### Installers (selection — full list in agent notes)
|
||
- **`install_arduino-ide`**: `unzip --strip-components=1` — flag doesn't exist; wipes `/opt/arduino-ide` then always fails, and `set -e` kills the script.
|
||
- **`install_mailhog`**: `curl -Lo` without `-f` → 404 page installed as a root systemd-managed binary; also downloads into whatever cwd the script happens to be in (deno/zed installers `cd` into temp dirs and delete them, leaving a dead cwd).
|
||
- **Plex**: `curl | sudo apt-key add -` — `apt-key` is gone on newer Ubuntu, so it aborts *after* deleting the old sources; key would land in the global trusted keyring.
|
||
- **Fixed predictable `/tmp` paths + `sudo mv`/`dpkg -i`** (lazydocker, glow, cheat, broot, dog, discord, ums, gollama, koboldcpp, LM Studio…): a local user can pre-plant/race the file and get a root-installed binary. Use `mktemp -d`.
|
||
- **`dpkg -i || apt-get install -f -y`** pattern (6+ sites): non-dependency dpkg failures are masked and reported as success.
|
||
- **whisper.cpp** builds unpinned master and copies `main`/`quantize` — upstream renamed the binaries; always fails now, `make 2>/dev/null` hides why.
|
||
- Version-pinned URLs that 404 now or soon: ctop (latest/download + 0.7.7), kind v0.20.0, Insomnia 8.6.1, mergerfs jammy-only deb, LM Studio 0.3.5, UMS deb name, balena-etcher remove targets the old package name.
|
||
- `curl | bash` as root (nodesource, ollama, starship, ddev — ddev also points at the moved `drud/ddev` repo) — accepted risk for a personal tool, but at least pin/checksum the big ones.
|
||
- `gpg --dearmor` without `--yes` (caddy, ngrok, docker, warp): reruns abort mid-way after old keys were already deleted.
|
||
|
||
### Packages that can never install (verified against live registries)
|
||
- **apt, wrong/absent name**: `sqlitestudio` (26.04+ only), `chromium` (→ `chromium-browser`), `asciiquarium`, `batcat` (→ `bat`, and duplicates the existing `bat` entry), `mysql-workbench-community` (needs MySQL repo), `pgadmin4` (needs pgadmin repo), `bottom`, `google-drive-ocamlfuse` (PPA-only), `jellyfin` (needs jellyfin repo), `citra`, `dosbox-staging`, `python3-esptool` (→ `esptool`), `webcamize`; version-dependent: `exa` (gone in 24.04+, keep `eza`), `pastel`, `fastfetch` (25.04+), `qalculate` (→ `qalculate-gtk`), `bsnes`, `git-delta` (23.10+).
|
||
- **snap, 404**: `tableplus`, `mongodb-compass`, `nextcloud-server` (→ `nextcloud`), `heroic`, `dosbox`, `platformio`.
|
||
- **cargo/npm**: `qmasa` (→ `qmassa`), `dust` (→ `du-dust`), `adminmongo` (not on npm).
|
||
- **flatpak**: citra + yuzu delisted from Flathub (Nintendo takedown); `freetube`/`stremio` use bare names instead of app IDs.
|
||
|
||
---
|
||
|
||
## Outstanding — MEDIUM
|
||
|
||
- MariaDB/MySQL passwords passed as `-p"$pass"` on the command line **everywhere** (~20 sites) — visible in `/proc/*/cmdline` to any local user (matters most on the VPS). Standard fix: `--defaults-extra-file=<(...)` or `MYSQL_PWD` for a personal box.
|
||
- DB dumps from `backup_database_interactive` written world-readable (default umask) into `$BACKUP_DIR`; `grep -q "^${database}$"` treats the name as a regex.
|
||
- wp-config credential extraction via `grep | cut -d"'" -f4` matches commented lines, no `head -1`, breaks on `'` in passwords (DB test/repair flows).
|
||
- `restart_web_services` probes only php 8.1/8.0/7.4-fpm — never restarts the script's own PHP 8.3 FPM, so "optimize uploads" silently doesn't land. Same class: SQL-import optimizer derives paths from CLI `php -v` (can be a different version than FPM); `client_max_body_size` sed lowers larger values across *all* vhosts; `<IfModule mod_php8.c>` never matches; `mysql.connect_timeout` should be `mysqli.connect_timeout`.
|
||
- MySQL "import optimization" permanently sets `innodb_flush_log_at_trx_commit=2` and blind `buffer_pool=512M` with no revert or RAM check; PHP `memory_limit=8G` per worker can OOM a small machine.
|
||
- `modify_php_setting` interpolates unvalidated input into sed (a `/`, `&` or `\` corrupts php.ini).
|
||
- Bulk ops: `install_package ... | tee` runs in a pipeline subshell, so cache updates inside are discarded — status can go stale for the session (harmless-ish now that redraws are cheap; re-run `build_package_cache` after bulk ops to be exact).
|
||
- `ui_menu` returns the literal string `"back"` on ESC — any future package named `back` breaks navigation; bulk category picker can pass `"back"` into the match loop.
|
||
- Web-config-summary site pickers push 3 args per non-live site into a *menu* (that's checklist syntax) — misaligned rows; "No live sites" branch unreachable.
|
||
- Log viewer: Ctrl+C on `tail -f` SIGINTs the whole script (no trap); pager temp file at predictable `/tmp/ultrabunt_log_view_$$` copies sudo-read logs world-readable.
|
||
- `wordpress_cleanup_menu` find: `-name "*.localhost" -o -name "*.local" -print0` — `-print0` binds to one branch; `.localhost` sites never listed.
|
||
- certbot always adds `-d www.$domain` — fails for domains without a www record.
|
||
- Hotkeys: >24 categories get lowercase tags but the handler uppercases before lookup (opens the wrong category); >48 reuses letters and silently overwrites. The "cycling" code for shared hotkeys is dead (assignment overwrites, never appends). Currently 27 categories, so the first overflow row (`a`) is already live.
|
||
- `ultrabunt-accessible.sh`: launches `./ultrabunt.sh` relative to caller's cwd; instructs running the whole thing via `sudo`, which breaks `$USER`-based ownership logic in the main script and makes TTS silent (root has no Pulse session); `timeout 10s echo | festival` applies the timeout to `echo` (festival can hang forever); `sudo -u user cat > file` redirection still runs as root.
|
||
|
||
## Outstanding — LOW (grab-bag)
|
||
|
||
- `show_system_info` hardcodes a 50×140 dialog — breaks on a stock 80×24 SSH window.
|
||
- Export list writes to predictable `/tmp/ultrabunt-packages-<ts>.txt`.
|
||
- `get_status()` is dead code (never called; no arms for custom/npm/cargo methods).
|
||
- `silversearcher-ag` and `ag` are duplicate entries for the same package.
|
||
- `remove_phpmyadmin` only restarts services that are `is-enabled` — a running-but-disabled DB/web server stays down. `--force-yes` no longer exists in modern apt.
|
||
- `__site_is_live` needs `host` (not on minimal Ubuntu) → everything reports "not live".
|
||
- Keyboard config writes to `$HOME`/gsettings — configures root's session if run under sudo.
|
||
- bun installer greps live `$PATH` instead of rc files → duplicate export lines per rerun.
|
||
- README drift: claims pip is removed but `install_pip_package`/npm/cargo helpers and CUSTOM npm tools are still in the script; claims 25+ categories/560+ apps — counts don't match the code.
|
||
|
||
---
|
||
|
||
## Suggested priorities
|
||
|
||
1. **WordPress DB quoting + the `finalize_wordpress_setup` chown** — it's the feature you actually use, on machines you keep.
|
||
2. **Delete or rewrite mirror management** — it cannot work and it edits sources.list.
|
||
3. **Umami/Mailcow port + secret fixes** if you ever run them on the VPS.
|
||
4. **Purge the ~25 never-installable package entries** (list above) — they just waste menu space and end in failure dialogs.
|
||
5. Sweep the installers for `curl -f`, `mktemp -d`, and `gpg --dearmor --yes` — three mechanical patterns fix most of the installer findings.
|