Skip to content

fix: a closed console no longer ends a running setup | ERR trap fixes | check VM ID before creating - #219

Merged
MickLesk merged 5 commits into
mainfrom
fix/telemetry-findings-2026-11-08
Nov 8, 2026
Merged

MickLesk merged 5 commits into
mainfrom
fix/telemetry-findings-2026-11-08

Conversation

@MickLesk

@MickLesk MickLesk commented Nov 8, 2026

Copy link
Copy Markdown
Member

✍️ Description

  1. keep the caller's ERR trap in silent()
    -> silent() armed trap error_handler ERR after every call. A script that add-tailscale-lxc or add-netbird-lxc runs through pct exec has no error_handler, so the next failing apt call ended in "error_handler: command not found" and exit 127 instead of apt's error. silent() now restores the trap the caller had, and falls back to error_handler only where it exists.

  2. fix: a closed console no longer ends a running setup

  3. fix: fetch repository keys with retries and psql repo to apt.postzgres

  4. fix: check the VM ID and bridge before creating the VM

  5. fix: do not recommend unprivileged for apps that need privileged

🔗 Related Issue

Fixes #

✅ Prerequisites (X in brackets)

  • Self-review completed – Code follows project standards.
  • Tested thoroughly – Changes work as expected (Proxmox VE and/or Incus, as applicable).
  • No security risks – No hardcoded secrets, unnecessary privilege escalations, or permission issues.
  • API.txt regenerated – If a function signature or behavior changed, API.txt was updated to match.

🛠️ Type of Change (X in brackets)

  • 🐞 Bug fix – Resolves an issue without breaking functionality.
  • ✨ New feature – Adds new, non-breaking functionality.
  • 💥 Breaking change – Alters existing behavior in a way that may require updates in every downstream script.
  • 🔧 Refactoring / Code Cleanup – Improves readability or maintainability without changing functionality.
  • 📝 Documentation update – Changes to README, docs/, CONTRIBUTING.md, or other docs.

🤖 AI Assistance (X in brackets)

If you used an AI tool (GitHub Copilot, Claude, ChatGPT, etc.) to write or generate any code in this PR, you must confirm compliance below.
Select exactly one option.

  • No AI used – Code was written without AI assistance.
  • AI was used – The output has been reviewed and corrected, and I take responsibility for it matching this repo's conventions.

silent() armed trap error_handler ERR after every call. A script that add-tailscale-lxc or add-netbird-lxc runs through pct exec has no error_handler, so the next failing apt call ended in "error_handler: command not found" and exit 127 instead of apt's error. silent() now restores the trap the caller had, and falls back to error_handler only where it exists.
228 runs in three days logged "The terminal closed" and 145 of them ended as "Installation failed (exit code: 199)". apt kept the terminal as stdin and exited 100 when it went. on_hangup moved the output inside silent()'s own redirection, which bash undid on return, so error_handler and on_exit died writing to the dead terminal before the host got a marker. On the host the hangup killed pct status (17 runs reported a running container as stopped by itself) and the base package pct exec. Tools without catch_errors kept writing to the dead terminal, and a prompt that then read EOF counted as a failure.

Shielded commands now get /dev/null instead of a terminal as stdin, silent(), error_handler and on_exit move the output back to the log, the host shields its setup pct calls and asks pct status twice, on_hup_keepalive does what on_hangup does, and a read or select that hits EOF is an abort.
setup_deb822_repo downloaded the key with one curl and no timeout: 53 runs failed on it, 32 of them on www.postgresql.org while apt.postgresql.org answered. The PostgreSQL key now comes from apt.postgresql.org (same key), and every repo key gets four attempts with a connect timeout.
An ID typed with a trailing space reached qm create split and qm disk import quoted, and a mistyped bridge only failed at qm create.
8 UniFi OS Server runs switched to "Unprivileged (recommended)" in the advanced wizard, and the install refused. Where the app defaults to privileged, that choice is now marked as the app default.
@github-actions

github-actions Bot commented Nov 8, 2026

Copy link
Copy Markdown
Contributor

Try this branch

The engine and the scripts resolve independently, so a production script can
be run against the engine from this PR by setting one variable:

COMMUNITY_SCRIPTS_CORE_URL=https://raw.githubusercontent.com/community-scripts/core/fix/telemetry-findings-2026-11-08 \
bash -c "$(curl -fsSL https://raw.githubusercontent.com/community-scripts/ProxmoxVE/main/ct/debian.sh)"

Swap ct/debian.sh for whatever exercises the change.

This PR touches pve/, so it needs a Proxmox VE host.

Run a script from a fork as well
curl -fsSL https://raw.githubusercontent.com/community-scripts/core/fix/telemetry-findings-2026-11-08/tools/run.sh |
  bash -s -- https://raw.githubusercontent.com/YOU/DevScripts/your-branch ct/debian.sh \
             https://raw.githubusercontent.com/community-scripts/core/fix/telemetry-findings-2026-11-08

Note that run.sh is reached through a pipe, so the script it starts inherits
an exhausted stdin. Whiptail is fine — it opens /dev/tty — but a plain read
would see EOF. The single-variable form above does not have that problem.

Useful flags while testing

dev_mode=net logs every engine fetch with status and duration, which is the
quickest way to confirm the branch is really being used. dev_mode=keep stops a
failed build from deleting the container along with the evidence.

@MickLesk
MickLesk merged commit b3e2925 into main Nov 8, 2026
5 checks passed
@github-actions
github-actions Bot deleted the fix/telemetry-findings-2026-11-08 branch November 9, 2026 09:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant