Update engine hardening, wrapper-aware config edits, keygen secrets hygiene - #7
Merged
Conversation
…steps update-node.sh declares `set -uo pipefail` (no errexit -- the apply flows do manual error handling with health-verify + rollback), but sourcing lib/common.sh silently re-enabled -e. Any unguarded failure after the service was stopped (perl edit, daemon-reload, a restore cp) aborted the script mid-apply: node left down, no rollback offered, pending state stranded. - set +e right after the common.sh source, with a comment explaining why. - write_pending_state now returns non-zero on write failure and all four call sites check it (a silent failure previously became a false "pending update saved" under +e). - daemon-reload failures warn (main paths) or are explicit best-effort (restore paths) instead of relying on errexit. - Rollback restore-cp failures now hard-stop with manual recovery instructions instead of restarting on the NEW artifact and letting a passing re-verify misreport "rolled back". - perl edit stays unguarded by design: the existing hash + grep checks catch every failure mode; now commented as such. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Nothing prevented a UI-triggered `--json --apply` and a CLI run from updating the same node concurrently: double service stop, racing truncate-writes to .pending-update, and interleaved binary/image swaps. lib/common.sh v1.3.8 adds tn_acquire_update_lock(): - flock -n on /var/lock/telcoin-update.lock (fd 9 held for process lifetime, so the kernel releases the lock on any exit -- no stale locks possible). Lock file records the holder PID for diagnostics. - Atomic mkdir fallback with PID-staleness takeover where flock(1) or /var/lock is absent (stock macOS). - On contention, TN_UPDATE_LOCK_HOLDER carries the holder PID so JSON mode can report it. update-node.sh v1.1.59 takes the lock for every mutating mode -- interactive flow, --json prepare/apply/discard -- and exits with a clear "another update is running (PID N)" message when contended. Read-only --check/--json check stays lock-free so dashboard polls never block a real update. Fixture-tested (mkdir path, the one macOS uses): contention reports the holder PID, dead-PID locks are taken over, EXIT trap releases cleanly. flock path to be exercised on the staging node (Linux-only). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…s live verify_health_after_restart only checked unit-active + RPC "result", so a no-op update (cp to the wrong place, restart on a stale unit definition after a failed daemon-reload) passed verification and reported success while the node kept running the old version. update-node.sh v1.1.60: - Source applies: after restart, the installed binary must sha256-match BUILT_HASH from the pending state; mismatch = verify failure = the existing rollback path. If the interactive operator explicitly approves applying a binary that changed since prepare, the check follows what they approved. - Docker applies: prepare now records PULLED_IMAGE_ID (docker image inspect .Id) in the pending state; after restart the running container's .Image must match it. Container name comes from --name in the launch wrapper/unit, falling back to SERVICE_NAME. - Rollback verifies stay on the plain health probe -- the old artifact is exactly what should be running there. - Both checks skip with a warning when the pending state predates this version (no BUILT_HASH / PULLED_IMAGE_ID), so in-flight prepares still apply. Fixture-tested via PATH-shim harness: tamper -> rejected, match -> pass, old pending state -> skip, docker-inspect hiccup -> no false rollback. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Two exposure windows during validator keygen: - The docker keygen passed `-e TN_BLS_PASSPHRASE="$bls_passphrase"`, putting the passphrase in the docker client's argv -- world-readable via ps / /proc/<pid>/cmdline for the life of the keygen run. Switched to name-only pass-through (`-e TN_BLS_PASSPHRASE`), the same convention the runtime start wrapper already uses; the value now travels only via the environment (already exported for the binary branch). - The passphrase file was echo-written first and chmod 600 second, leaving it readable under the default umask (0644) for the gap between the two commands. Now created 0600 from the first byte via a umask-077 subshell (the lib/observability.sh token-file pattern), with the chmod kept for re-runs since umask cannot tighten a pre-existing file. echo -> printf '%s\n' while here, so a passphrase starting with "-e"/"-n" cannot be mangled. setup-node.sh v1.0.1. Verified with a docker PATH shim: argv carries no passphrase, the container env still receives it, and the file is 600 at creation under a 022 umask. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…y reads Config detection grepped the unit's ExecStart for a literal "docker run", but current installs launch via the start wrapper (/opt/telcoin/start-<svc>.sh) -- the unit's ExecStart is just the wrapper path. Docker installs were misdetected as binary, every flag showed "unknown", edits silently rewrote unit lines the service never reads, and edit_docker_image refused to run at all. Ports the update-node.sh 7b10e97 pattern (edit-config.sh v1.2.5): - TARGET_LAUNCH_FILE resolved via tn_node_launch_target (wrapper when present, unit for legacy inline installs; unit fallback if the wrapper is missing) in both the menu flow and --json set. - read_launch_line joins the wrapper's backslash-continued command so all flag/image inspection sees one logical line; is_docker_install and the show-config display now detect wrapper docker installs correctly. - Listener edits write wherever the service reads: the -e lines in the docker launch file (quote-preserving pattern), or the unit Environment= PLUS the wrapper export for binary/source (the export is what the process sees; the unit line stays in sync). Menu, p2p-ports, and --json set all share the one writer. - Metrics, verbosity, and image edits target TARGET_LAUNCH_FILE; backups and the --json rollback now cover both files when they differ. - set_verbosity rewritten: the old `sed s| -v\+ |` matched the first " -v " -- on any docker launch line that is the -v VOLUME flag (a latent corruption bug on legacy inline units too) and on wrapper files the line-start token never matched. Now replaces -v{1,5} only when followed by a continuation, another --flag, or end of line, so volume -v and `command -v` can never match. - edit_rpc refuses loudly on wrapper installs (its ExecStart rewrite is legacy-only) with the manual instruction, instead of appending junk args to the unit. edit_bls_passphrase is intentionally unchanged: only legacy inline units embed the passphrase, and its unit grep is the correct discriminator for exactly those. Fixture-tested against all three real layouts (docker wrapper incl. TPM lines, binary wrapper, legacy inline unit): detection, listener/metrics/ verbosity/image edits, quote integrity, wrapper still parses, and the volume--v regression case. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…-cut - update-scripts.sh v1.1.65 release marker so operators re-fetch the five changed files (update-node 1.1.60, lib/common 1.3.8, edit-config 1.2.5, setup-node 1.0.1, ui/server.py). - ui/server.py UI_VERSION 1.8.6 (no UI change): triggers install-ui.sh --update so the root-owned engine copies in /opt/telcoin-ui-update/ pick up the lock, identity-verify, and wrapper-aware edit fixes. - Regenerated the six affected .sha256 sidecars via tools/gen-checksums.sh. - README changelog: one entry per changed script version. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…estore Self-review caught two errexit landmines in the wrapper-aware port (edit-config.sh runs under set -e from common.sh, unlike update-node.sh): - edit_p2p_ports used `[[ -z x ]] && x=$(...)` for the launch-file fallback -- rc 1 whenever the unit already provided the value, which killed the script mid-menu. Now if/fi. - run_json_set's _json_restore_backups ended with the same && shape when no wrapper backup exists, killing the script before the terminal done-event reached the UI. Now if/fi with a guarded cp that reports a failed wrapper restore instead of dying silently. Sidecar refreshed; edit-config stays v1.2.5 (same unreleased cut). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
update-node.sh(v1.1.60): mutating operations now take a cross-invocation lock so a UI-triggered apply and a CLI apply can't run concurrently; post-apply verify now checks artifact identity (installed binary hash for source, running container image ID for docker) instead of health alone, so a no-op swap or stale unit no longer reports success; restored the script's intendedset -uo pipefail-only semantics (lib/common.shwas silently re-enablingset -e) and made every apply/rollback step check its own result, including failed pending-state writes and failed rollback restores.edit-config.sh(v1.2.5): detection and every edit path (listeners, p2p ports, metrics, verbosity, docker image, menu and--json set) now resolve the file the service actually reads, matchingupdate-node.sh's resolution. Docker installs launched through the start wrapper were previously misdetected as binary installs, so every field showed unknown and edits silently landed in a unit file the service never reads. Also fixes a verbosity-edit bug where the first-von a docker launch line (the volume flag) could be replaced instead of the verbosity flag. RPC editing on wrapper installs is refused with manual instructions rather than appending flags to the wrong file.setup-node.sh(v1.0.1): the docker keygen no longer passes the BLS passphrase as-e TN_BLS_PASSPHRASE="<value>", which was visible inps//proc/*/cmdlinefor the life of the keygen; it now passes the name only and lets docker read the value from the environment. The passphrase file is created0600from the first byte via a umask subshell instead of write-then-chmod.update-scripts.shandui/server.pyversion bumps to redeploy the updater and web UI copies in/opt/telcoin-ui-update/, plus refreshed.sha256sidecars and changelog entries for all of the above.