v1.1.1: reboot self-heal, caddy loop, secret persistence, root guard, qbit RAM#2
Conversation
…ard, qbit RAM Addresses the v1.1.0 issues reported on Discord (Loki/Ryder): - Reboot ordering: add a deunhealth sidecar that restarts qBittorrent when its healthcheck fails. depends_on ordering is ignored on host reboot, so a VPN-routed qbit can start before gluetun and lose its published WebUI port, and restart:unless-stopped can't fix it because qbit is up, just orphaned. Only qbit is labeled, so nothing else is ever auto-restarted. arrstack update injects the sidecar into existing VPN installs too. - Healthchecks: render the catalog health blocks (previously dead code) as container healthchecks using a portable curl-or-wget probe (arr/media images ship one or the other). Skip tcp/port-0 blocks (caddy, recyclarr). - Caddy crash-loop: emit a minimal :80 site in LAN-only mode with no local DNS instead of a 0-byte Caddyfile that makes caddy exit and restart-loop. - Secret persistence: reuse persisted api keys and a new secrets block (the Bazarr/translator AES key + Bazarr flask secret) across reconfigure/resume instead of rotating them out from under running containers. Stop the install with a fix message if admin.txt exists but is unreadable (root-owned) rather than silently generating a new password and locking the user out of qbit. - Root guard: warn in preflight and the wizard status strip, and clamp PUID/PGID away from 0 so LinuxServer.io containers do not run as root. - qBittorrent RAM: set enable_os_cache=false to bound the libtorrent-2.0 memory-mapped disk cache that can grow to many GB and OOM a small VM. - Tooling: add typescript to devDependencies so `bun run typecheck` works. 296 tests pass, typecheck clean.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 707e08e8da
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const url = `http://127.0.0.1:${h.port}${h.path}`; | ||
| return { | ||
| test: `["CMD-SHELL", "curl -fsS -o /dev/null ${url} || wget -q -O /dev/null ${url} || exit 1"]`, |
There was a problem hiding this comment.
For VPN-routed qBittorrent, this healthcheck runs inside the qBittorrent container and curls its own loopback address. In the reboot/orphaned-netns case described below, qBittorrent can still serve 127.0.0.1:8080 inside its stale namespace while gluetun no longer publishes that port to the host, so Docker keeps the container healthy and deunhealth never restarts it. The probe needs to exercise the gluetun/published path (or otherwise detect the stale namespace) for the new sidecar to fix the reboot failure.
Useful? React with 👍 / 👎.
| // 0. Not running as root (warning only, non-blocking) | ||
| results.push(checkNotRoot()); |
There was a problem hiding this comment.
Adding this non-blocking warning to every preflight result makes arrstack doctor fail whenever it is run as root: the doctor loop marks allOk = false for any !check.ok result without checking blocking (src/usecase/doctor.ts:69-76). That turns the new advisory into a failing diagnostic exit even though checkNotRoot() explicitly sets blocking: false; either doctor should treat non-blocking checks as warnings or this check should be skipped there.
Useful? React with 👍 / 👎.
| // non-sudo run then can't read it, would generate a NEW admin password, | ||
| // rewrite qBittorrent.conf, and lock the user out of the still-running qbit | ||
| // container. Stop early with a fix instead of silently rotating. | ||
| if (!opts.fresh && adminTxtUnreadable(installDir)) { |
There was a problem hiding this comment.
This protection only runs for arrstack install, but the same TUI can still be launched through bare arrstack (src/cli.ts:143-146) without calling adminTxtUnreadable. In that entrypoint, an unreadable root-owned admin.txt is still swallowed by readExistingAdminPassword, a new admin password is generated, and the qBittorrent lockout scenario described here can still happen when users reconfigure via the default command.
Useful? React with 👍 / 👎.
| const translatorEncryptionKey = | ||
| secrets.translator_encryption_key ?? randomBytes(32).toString("hex"); |
There was a problem hiding this comment.
For an install whose state.json was written before the new secrets field, the schema defaults state.secrets to {}, so the first reconfigure after upgrading still takes this fallback and mints a new translator ENCRYPTION_KEY (and Flask secret). The old key is already present in .env/Bazarr config but is ignored here, so this still orphans encrypted translator data for existing users; migrate from the existing files before falling back to random generation.
Useful? React with 👍 / 👎.
- P1: VPN-routed qBittorrent healthcheck now probes tunnel connectivity (cp.cloudflare.com/generate_204) instead of loopback 8080. A loopback probe passes even when qbit is orphaned from gluetun's netns after a reboot, so deunhealth would never restart it; the tunnel probe only succeeds in gluetun's live netns, which is exactly when the published WebUI port also routes. - P2: arrstack doctor no longer fails on a non-blocking preflight check. The root-user warning (blocking: false) prints as a warning, not a failing exit. - P2: apply the unreadable-admin.txt guard to bare `arrstack` (the default TUI entrypoint) too, not just `arrstack install`, via a shared helper. - P2: on upgrade, backfill the translator ENCRYPTION_KEY and Bazarr flask secret from the existing .env / Bazarr config before generating new ones, so the first reconfigure after upgrading doesn't rotate keys and orphan data encrypted at rest. 301 tests pass, typecheck clean.
|
Addressed all four Codex findings in 05e7552:
301 tests pass, typecheck clean. @codex review |
|
Codex Review: Didn't find any major issues. Can't wait for the next one! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
…secret regex, tests) A multi-agent review of the v1.1.1 branch surfaced these (all low/medium, no criticals): - VPN qbit healthcheck now probes gluetun's control server (127.0.0.1:8000, via the shared netns) instead of a third-party host. The old external probe could not tell an orphaned netns from a real tunnel/WAN outage, so deunhealth would restart-loop qbit during a genuine outage. The control server is up whenever the gluetun container is up, independent of the tunnel, so a real outage no longer churns qbit; -f is omitted so an auth 401 still counts as reachable. Verified reachable from the running qbit container. - arrstack update now persists deunhealth into state.services_enabled, not just the rendered compose, so doctor can verify the sidecar and later updates don't re-add/re-log it (state/compose drift). - Harden the Bazarr flask_secret migration regex: anchored, same-line value, strips optional quotes, so a re-serialized quoted/empty/commented key falls through to generation instead of capturing the wrong text. - Tests: cover the doctor warn-vs-fail path (classifyCheck), the admin.txt guard message (adminTxtGuardMessage), flask regex edge cases, and the update no-re-add path; reframe the tautological restart test into a real reboot-recovery-trio assertion. 310 tests pass, typecheck clean.
|
Addressed a follow-up multi-agent code review in 8822c64 (all findings were low/medium, no criticals):
310 tests pass, typecheck clean. @codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8822c64aad
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const GLUETUN_CONTROL_URL = "http://127.0.0.1:8000/v1/publicip/ip"; | ||
| function buildVpnHealthcheck(): Healthcheck { | ||
| return { | ||
| test: `["CMD-SHELL", "curl -sS -m 10 -o /dev/null ${GLUETUN_CONTROL_URL} || exit 1"]`, |
There was a problem hiding this comment.
When VPN routing is enabled this healthcheck is the signal that arms deunhealth, but it only verifies that gluetun's control server is reachable. If qBittorrent's WebUI/process is hung or no longer listening while gluetun is up, Docker will still report the qBittorrent container as healthy and deunhealth will never restart it; the VPN-specific probe should also check qBittorrent's local WebUI (in addition to the gluetun netns probe) so actual qbit failures are still self-healed.
Useful? React with 👍 / 👎.
The control-server-only probe proves qbit is in gluetun's live netns (catches the reboot orphaned-netns case) but would stay healthy if qbit's WebUI process hangs while gluetun is up, so deunhealth would never restart a hung qbit. Require BOTH legs: gluetun control server reachable AND qbit's WebUI responding. Verified both legs pass on a live VPN-routed qbit container. 310 tests pass, typecheck clean.
|
Addressed the re-review P2 in the latest commit: the VPN qbit healthcheck now requires BOTH legs, gluetun's control server reachable AND qbit's own WebUI responding, so a hung qbit (process up, UI dead) while gluetun is fine is now caught and self-healed too. Verified both legs pass on the live container. 310 tests pass, typecheck clean. @codex review |
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
1 participant
Fixes the v1.1.0 issues surfaced on Discord (Loki/Ryder), found via a multi-agent review of the reports.
Fixes
#1 Reboot ordering, qBittorrent loses gluetun's port after a restart (high)
depends_onis only honored ondocker compose up, NOT on host/daemon reboot. A VPN-routed qbit (network_mode: service:gluetun) can start before gluetun and end up orphaned from its netns with the WebUI port dead;restart: unless-stoppedcan't help because qbit is up, just broken. Added adeunhealthsidecar (ro docker.sock) that restarts qbit when its healthcheck goes unhealthy. Only qbit carries the restart label, so nothing else is ever auto-restarted.arrstack updateinjects the sidecar into existing VPN installs, so current users get the fix without a fresh install.#4 Per-service healthchecks (medium)
The catalog
health:blocks were dead code (only gluetun got a healthcheck). They now render as container healthchecks using a portablecurl || wgetprobe (verified against a live stack: some images ship only one).tcp/port: 0blocks (caddy, recyclarr) are skipped to avoid false-unhealthy.depends_onis intentionally left short-form to avoid first-install deadlocks.#2 Secret persistence on reconfigure/resume (high)
buildStateFromWizardregenerated api keys on every run, and the Bazarr/translator AES key + flask secret were minted fresh each install. Now persisted api keys and a newsecretsblock are reused. Also:arrstack installnow stops with an actionable message ifadmin.txtexists but is unreadable (root-owned from a priorsudorun) instead of silently rotating the password and locking the user out of the running qbit.#3 Caddy crash-loop in LAN-only + no-DNS (medium)
That combo rendered a 0-byte Caddyfile, so
caddy runexited (adapting config: EOF) and looped underrestart: unless-stopped. Now emits a minimal:80site.#5 Root-user guard (medium)
Preflight + wizard warnings when running as root, and PUID/PGID are clamped away from 0 (LinuxServer.io images break as root).
#6 qBittorrent RAM (medium)
enable_os_cache: falsebounds the libtorrent-2.0 memory-mapped disk cache that can grow to many GB and OOM a small VM.#7 Tooling
Added
typescriptto devDependencies (bun run typecheckpreviously failed withtsc: command not found).Deliberately out of scope
mem_limitfor qbit (OOM-kills it if set too low;enable_os_cachefixes the cause).depends_on: service_healthyflip (risks first-install deadlocks).Verification
bun test: 296 pass / 0 fail (+26 new tests)bun run typecheck: clean