Comprehensive review 2026-08-21: CI release detection broken (registries stale since 06-02) + cleanup/volume/docs findings

[libre-7]

Comprehensive Review — 2026-08-21 (main @ 4d46e05)

Full systematic review per the project checklist: pre-read scans → deep read of every file → cross-reference (compose ↔ template XML, both CI workflows ↔ each other, README ↔ start.sh ↔ upstream Gupax source) → live verification (CI run logs, upstream release API, Docker Hub API, upstream source audit).

Severity Summary

Severity	Count	Headline
🚨 Critical	1	CI release detection broke on upstream's non-semver tag — both registries frozen since 2026-06-02, missing upstream's June 15 Critical P2Pool Release
🔴 High	3	Crash path skips cleanup; gupax-state volume is dead weight with misleading docs; wallet-address instructions point at the wrong tab
🟡 Medium	3	Healthcheck can't detect dead VNC/Gupax; Unraid template loses cap_drop hardening; silent CI failures
🟢 Low	8	Casing, committed planning notes, stale CHANGELOG/TODO, hygiene

---

🚨 CRITICAL

C1. Release detection assumes semver tags; upstream broke the assumption — images stale for ~6 weeks

Evidence chain (all verified live):

• Upstream /releases/latest currently resolves to tag critical_update_p2pool (2026-06-15, "Critical P2Pool Release Update") — not a v* tag. Its Linux asset is named gupax-v2.0.1-linux-x64.tar.gz.
• CI feeds whatever tag it finds straight into the Dockerfile: run #282 (GHCR) and #171 (Docker Hub), both on push to main (2026-07-12), constructed gupax-critical_update_p2pool-linux-x64.tar.gz → curl: (22) … error: 404 → build failure.
• Last successful image on both registries: 2026-06-02 (86f845e). Docker Hub last_updated: 2026-06-02, :latest = 86f845e….

Impact: :latest on both registries predates upstream's critical P2Pool update by two weeks and misses every main-branch change since (including PR #65's fixes — merged but never shipped in an image). Worse: if upstream ever attaches a matching asset name to a non-semver tag, the build would succeed and push tags like :critical_update_p2pool / OCI label version=critical_update_p2pool-YYYYMMDD — garbage tags users can't parse.

Fix (both workflows, keep lockstep):

1. Validate detected tag against ^v[0-9]+\.[0-9]+\.[0-9]+$; on miss, fall back to latest matching v* release (filter /releases?per_page=30) — currently that yields v2.0.1, whose asset exists.
2. Better: resolve the actual asset filename from the chosen release's assets[] instead of constructing gupax-${VERSION}-linux-x64.tar.gz — decouples tag naming from asset naming entirely.
3. Add failure notification (the July 12 double-failure sat unnoticed for weeks) — see M3.

---

🔴 HIGH

H1. Gupax crash-exit skips cleanup() — set -e kills the script at wait

start.sh:390-391: wait $GUPAX_PID followed by EXIT_CODE=$?. Under set -e, if Gupax exits non-zero (a crash — exactly when you'd want teardown), bash aborts at wait and never reaches cleanup or exit $EXIT_CODE. Verified empirically: killing the child produced script exit 137 with no [cleanup ran] output. Xvfb/x11vnc/websockify/Tor are torn down only by PID-namespace death instead of graceful shutdown; exit-code propagation only works for exit 0.

# start.sh:390 — replace:
wait $GUPAX_PID
EXIT_CODE=$?
# with:
set +e
wait $GUPAX_PID
EXIT_CODE=$?
set -e

H2. gupax-state volume is dead weight, and the template tells users their wallet lives there

Verified against upstream source (src/disk/mod.rs): the only persistence root is dirs::data_dir() → ~/.local/share/gupax (state.toml, node.toml, pool.toml). Upstream never uses ~/.local/state/gupax. Therefore:

• Compose's gupax-state:/home/miner/.local/state/gupax mount receives nothing — an empty volume forever.
• templates/gupax-docker.xml:50 maps Unraid config/ → .local/state/gupax described as "Gupax configuration and state (wallet, settings persist here)". A user who backs up /mnt/user/appdata/gupax/config/ before recreating the container loses their actual config, which lives in the share mapping. Data-loss footgun, not cosmetics.

Fix: drop the gupax-state service/volume from compose + template Config line + volume tables everywhere — or keep the mount but re-label accurately and move the "config" description to share/.

H3. Wallet-address instructions point at the Node tab; upstream puts the field on the P2Pool tab

README §Setting Your Wallet Address (line ~247) and the template <Description> say to enter the Monero wallet address in the Node tab. Upstream disagrees: address belongs to pub struct P2pool (src/disk/state.rs:499), passed to p2pool args (helper/p2pool.rs:508); the Node struct has no address field. New users land on the Node tab hunting for a field that isn't there. Fix both locations to say P2Pool tab.

---

🟡 MEDIUM

M1. HEALTHCHECK cannot detect a dead VNC stack or dead Gupax

healthcheck.sh probes urlopen('http://localhost:6080/') — that's websockify's static file server, which happily serves index.html after x11vnc dies. Container shows healthy with a black/dead GUI indefinitely (README troubleshooting describes exactly this symptom class). Fix: add a real VNC probe — nc -z 127.0.0.1 5900 plus optionally pgrep -x x11vnc / pgrep -f gupax.

M2. Unraid template silently loses the capability hardening

Compose enforces cap_drop: [ALL] + five adds; the template has no equivalent and <ExtraParams> is empty — so the CA/manual-install path (the primary install route per README) runs with Docker's full default capability set. Resource limits were matched (4 GB/2 CPU); capabilities weren't. Fix: populate <ExtraParams> with --cap-drop=ALL --cap-add=SETUID --cap-add=SETGID --cap-add=DAC_OVERRIDE --cap-add=FOWNER --cap-add=CHOWN (verify Unraid honors these on deploy).

M3. CI failures are completely silent

Both registry workflows have no failure notification; the 2026-07-12 double-failure went unnoticed for weeks. Add an if: failure() step that opens/comments on a tracking issue so red registries surface without watching Actions.

---

🟢 LOW / HYGIENE

#	Finding	Where
L1	Badge + clone URLs use Gupax-docker casing; repo is lowercase gupax-docker (verified via API). Prior reviews fixed this class (#22, #52).	README.md:3-5,45
L2	Internal agent planning notes committed to public repo — contain stale VNC_PASSWORD naming (pre-rename!), internal VM IP, duplicated TODOs. Remove from git; add .hermes/ to .gitignore.	.hermes/plans/*.md
L3	CHANGELOG newest entry is v2.0.1-20260518; two months of landed work (template v2 schema, SECURITY/CONTRIBUTING, PR #65, checkout@v7) unrecorded.	CHANGELOG.md
L4	TODO.md claims [x] Multi-arch builds … linux/arm64 — both workflows build linux/amd64 only. Stale checkbox contradicts the arm64 research conclusion.	TODO.md:16 vs workflows
L5	Docker Hub full_description drifted from repo README (old badge set, missing DH-build badge). Re-sync via API.	Docker Hub
L6	eval $(dbus-launch --sh-syntax) trips SC2046 (intentional splitting; below CI's severity: error gate). Add explicit disable comment.	start.sh:335
L7	Actions pinned by floating major tags (@v7, @v4) not commit SHAs; Dependabot mitigates but SHA-pinning is stronger for a repo publishing images from main.	all three workflows
L8	Minor: PR health-gate accepts starting; docker-publish.yml:46 bare curl -s vs Hub's -fsSL (asymmetric); SCREEN_RESOLUTION regex admits 0x0x0.	workflows, start.sh:116

---

✅ Positives (verified, not assumed)

• Supply-chain discipline: base image digest-pinned; Gupax tarball verified against upstream SHA256SUMS at build time (fails closed); provenance mode=max + SBOM on both registries.
• Runtime hardening: cap_drop ALL with per-capability rationale; VNC password via -passwdfile (never in /proc/cmdline), temp file deleted post-start and again in cleanup; sudoers scoped to %gupax + one binary.
• Correctness culture: pipefail in build SHELL, entrypoint, healthcheck; timeout=5 on healthcheck urlopen; FUSE fallback honestly disclosed.
• Registry lockstep restored: both workflows share triggers, tagging logic, build-args pattern, provenance/SBOM — fix: CI build-args broken multiline scalar + remove asymmetric paths filter #48/fix: align Docker Hub version tags with GHCR (strip v prefix) #53/fix: dynamic build date, OCI labels, and stale reference cleanup #65 drift fully resolved.
• Tor documentation is genuinely excellent — tx-only bandwidth ethics, --tx-proxy comma-format pitfall, --rpc-login incompatibility warning.
• shellcheck/yamllint/trailing-newline: clean apart from L6.

---

Recommended action plan

1. Now: C1 — restore image builds (semver validation + asset-name resolution + failure alerting), then trigger both workflows manually so :latest picks up the June P2Pool fix + PR fix: dynamic build date, OCI labels, and stale reference cleanup #65.
2. Same branch: H1 (two-line fix), H2/H3 (volume + tab corrections across compose/template/READMEs).
3. Follow-up: M1–M3, then hygiene batch.

Per project convention: findings presented first, no code changed yet. Fixes will go to a branch with individual VM verification before any Unraid test.

---

Review performed by Hermes Agent — systematic methodology: pre-read scans, full-file deep read, cross-referencing, live CI/API/upstream-source verification.