Skip to content

Fix installer set -e landmines, validate every prompt, verify registration#14

Merged
iamfarhad merged 1 commit into
mainfrom
feat/ui-redesign-production-docker
Jul 19, 2026
Merged

Fix installer set -e landmines, validate every prompt, verify registration#14
iamfarhad merged 1 commit into
mainfrom
feat/ui-redesign-production-docker

Conversation

@iamfarhad

Copy link
Copy Markdown
Owner

Summary

Audit of deploy/install.sh, deploy/install-node.sh, and deploy/node-entrypoint.sh found two silent-death set -e bugs, a firewall misconfiguration, and a set of unvalidated prompts whose bad values only surfaced much later with unrelated-looking errors. All fixed.

Critical (install.sh) — both reproduced before fixing

  • Self-node failure killed the script before the admin password printed. ( set +e; setup_self_node ); rc=$? still trips the parent's errexit on the subshell's non-zero exit, so the held-until-the-end summary — including the unrecoverable one-time admin password — never displayed. Now captured with ( ... ) || rc=$?.
  • A legacy .env missing a newer key killed the script with no output. Every VAR="$(grep '^KEY=' .env | cut ...)" dies under pipefail when the key is absent (e.g. NODE_AGENT_PORT on an env predating it — the same legacy-env scenario the PANEL_HTTP_PORT backfill exists for). All env reads now use an errexit-safe env_get helper with fallbacks.

Misconfigurations

  • setup_firewall hardcoded 80/tcp 443/tcp — custom PANEL_HTTP/HTTPS_PORT installs got the wrong ports opened. Now reads the configured ports.
  • SSH lockout: ufw allow OpenSSH || true fails silently without the app profile; ufw enable then default-denies a custom-port SSH session. Both installers now also allow the ports sshd actually listens on.
  • preflight_ports aborted re-runs over a live stack (docker-proxy holds NODE_AGENT_PORT). Now skips docker-proxy-held ports, and also checks the web ports — a taken 443 was the documented live incident.
  • Docker is enabled on boot even when preinstalled; apt runs noninteractive.

Validation & verification

  • All prompts validated: panel domain (scheme/path stripped — the agent prepends https:// itself, so a pasted URL previously produced https://https://…), email, username, ports 1–65535, IFNAMSIZ interface names, real CIDR octet/prefix ranges, numeric node capacity (previously interpolated raw into the bootstrap JSON body).
  • The wrong-port HTML probe buffers curl output so grep -q's early-exit SIGPIPE can't swallow a positive match under pipefail.
  • verify() (both installers) waits for the agent's "registered"/"fatal" JSON log markers instead of declaring a crash-looping container a success after 3 seconds, and prints the actual failure.
  • node-entrypoint.sh syncs ListenPort/Address on an existing persisted wg conf, so a re-install with a changed port no longer publishes one port while the interface listens on another (keys preserved).

Test plan

  • bash -n on all three scripts
  • Both set -e bugs reproduced in isolation; fixes verified to survive
  • env_get tested against present / missing / =-containing keys
  • All validators unit-tested under set -euo pipefail (CIDR incl. leading-zero octal trap, ports, domain/email/username sanitization, iface regex, sshd-port extraction, conf-sync sed)
  • Fresh install + re-run on a Debian/Ubuntu VPS (recommended before release)

🤖 Generated with Claude Code

…ation

install.sh had two errexit bugs that killed the script silently:
`( set +e; setup_self_node ); rc=$?` aborts the parent on subshell failure
before the final summary (and the one-time admin password) prints, and
every `VAR="$(grep '^KEY=' .env | cut ...)"` dies under pipefail when a
legacy .env lacks the key. Self-node rc is now captured via `|| rc=$?`
and all env reads go through an errexit-safe env_get helper.

Also across both installers:
- setup_firewall opens the configured PANEL_HTTP/HTTPS ports instead of
  hardcoded 80/443, and allows the ports sshd actually listens on so
  `ufw enable` can't lock out a session on a custom SSH port
- preflight_ports also checks the web ports, but skips ports held by
  docker-proxy so re-runs over a live stack pass their own preflight
- every prompt is validated (domain stripped of scheme/path, email,
  username, ports 1-65535, IFNAMSIZ iface names, real CIDR octets,
  numeric node capacity - previously interpolated raw into JSON)
- PANEL_ADDR strips a pasted https:// scheme the agent would double up
- the wrong-port HTML probe buffers curl output so pipefail can't
  swallow a positive match via grep -q's early-exit SIGPIPE
- verify() waits for the agent's "registered"/"fatal" log markers
  instead of calling a crash-looping container a success after 3s
- docker is enabled on boot even when preinstalled; apt runs
  noninteractive
- node-entrypoint syncs ListenPort/Address on an existing wg conf so a
  re-install with a new port actually applies it (keys preserved)

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request significantly hardens the installation and configuration scripts by adding robust input validation for domains, subnets, usernames, and ports. It also improves firewall setup to prevent SSH lockouts, ensures Docker starts on boot, and updates the node registration verification to monitor actual container log markers rather than just container status. The review feedback suggests optimizing the log-monitoring loops by using --tail 100 to limit log retrieval size, and adding fallback defaults in preflight_ports to handle legacy .env files where port variables might be missing.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Comment thread deploy/install-node.sh
local logs deadline=$((SECONDS + 45))
while (( SECONDS < deadline )); do
sleep 3
logs="$(cd "$NODE_DIR" && docker compose logs --no-color 2>/dev/null || true)"

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Fetching the entire log history of the container on every iteration of the loop can become slow and resource-intensive, especially if the container has been running for a while or is crash-looping rapidly.

Limiting the log retrieval to the most recent entries (e.g., using --tail 100) improves efficiency and reduces memory usage.

Suggested change
logs="$(cd "$NODE_DIR" && docker compose logs --no-color 2>/dev/null || true)"
logs="$(cd "$NODE_DIR" && docker compose logs --no-color --tail 100 2>/dev/null || true)"

Comment thread deploy/install.sh
log "Waiting for the node to register with the control plane..."
while (( SECONDS < deadline )); do
sleep 3
logs="$(cd "$NODE_DIR" && docker compose logs --no-color 2>/dev/null || true)"

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Fetching the entire log history of the container on every iteration of the loop can become slow and resource-intensive, especially if the container has been running for a while or is crash-looping rapidly.

Limiting the log retrieval to the most recent entries (e.g., using --tail 100) improves efficiency and reduces memory usage.

Suggested change
logs="$(cd "$NODE_DIR" && docker compose logs --no-color 2>/dev/null || true)"
logs="$(cd "$NODE_DIR" && docker compose logs --no-color --tail 100 2>/dev/null || true)"

Comment thread deploy/install.sh
Comment on lines 316 to +318
local port pid
port="$(grep '^NODE_AGENT_PORT=' "$ENV_FILE" | cut -d= -f2)"
[[ -n "$port" ]] || return 0
if command -v ss >/dev/null 2>&1 && ss -tlnH "sport = :${port}" 2>/dev/null | grep -q .; then
command -v ss >/dev/null 2>&1 || return 0
for port in "$(env_get NODE_AGENT_PORT)" "$(env_get PANEL_HTTP_PORT)" "$(env_get PANEL_HTTPS_PORT)"; do

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

If NODE_AGENT_PORT, PANEL_HTTP_PORT, or PANEL_HTTPS_PORT are not explicitly defined in a legacy .env file, env_get will return an empty string, causing preflight_ports to skip checking those ports.

Providing fallback defaults (matching those used in setup_firewall) ensures that the preflight check always validates the ports the stack will attempt to bind to.

Suggested change
local port pid
port="$(grep '^NODE_AGENT_PORT=' "$ENV_FILE" | cut -d= -f2)"
[[ -n "$port" ]] || return 0
if command -v ss >/dev/null 2>&1 && ss -tlnH "sport = :${port}" 2>/dev/null | grep -q .; then
command -v ss >/dev/null 2>&1 || return 0
for port in "$(env_get NODE_AGENT_PORT)" "$(env_get PANEL_HTTP_PORT)" "$(env_get PANEL_HTTPS_PORT)"; do
local port pid agent_port http_port https_port
command -v ss >/dev/null 2>&1 || return 0
agent_port="$(env_get NODE_AGENT_PORT)"
http_port="$(env_get PANEL_HTTP_PORT)"
https_port="$(env_get PANEL_HTTPS_PORT)"
for port in "${agent_port:-48443}" "${http_port:-80}" "${https_port:-443}"; do

@iamfarhad
iamfarhad merged commit ec27fdd into main Jul 19, 2026
2 checks passed
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