Files

T

daemonhornandClaude Sonnet 5 a5e683546f Add JetKVM SSH deploy hook (#7254 )

* Add JetKVM SSH deploy hook

Adds deploy/jetkvm.sh to deploy a certificate to a JetKVM
(https://jetkvm.com) KVM-over-IP device over plain SSH, writing the
cert/key via a small POSIX shell script piped to the remote "sh" (JetKVM
has no scp/SFTP server), staged under temp names and atomically renamed
into place so a dropped connection can't leave the device with a
mismatched cert/key pair for its own HTTPS listener. Defaults target
JetKVM's confirmed "Custom" TLS storage path/filenames and default the
post-upload command to "reboot", since JetKVM has no hot-reload for a new
certificate.

This factors out the SSH upload logic originally proposed in
opnsense/plugins#5621 (an OPNsense ACME Client plugin automation) per
maintainer feedback there, so it can be reused as a small config instead
of plugin-specific code: https://github.com/opnsense/plugins/pull/5621#issuecomment-5570647257

* Fix restart-command exit-code handling in jetkvm.sh

The default restart command ("reboot") tears down the very SSH
connection running it, which real hardware testing shows makes ssh's
own exit code unreliable: it can come back as either a clean 0 or a
connection-reset 255 for the exact same successful reboot depending on
timing. The hook previously trusted that single exit code directly, so
a fully successful, unattended cron renewal could be reported as a
failed deploy.

Split into two SSH calls: the first uploads and stages the cert/key and
its exit code is trusted as-is (no reboot risk there). The second runs
the restart command and is judged by whether a marker line printed
*before* that command shows up in the captured output -- if the marker
is missing, the call never really ran (real failure); if it's present,
only a clean exit or 255 (connection dropped, expected) counts as
success, while any other exit code is treated as the restart command's
own genuine failure (e.g. 127 = command not found).

Also: a blank DEPLOY_JETKVM_RESTART_CMD now falls back to "reboot"
rather than silently skipping the restart -- previously there was no
way to actually configure "no restart command", since the hook coerced
any blank value (including one the user deliberately set) back to
"reboot" on every run. Skipping it now requires the explicit sentinel
DEPLOY_JETKVM_RESTART_CMD="none".

* Verify JetKVM HTTPS Mode is "Custom" before uploading

Uploading a certificate that the device's active HTTPS Mode won't even
serve was previously a silent no-op -- the write would succeed but never
take effect until a human noticed and fixed the mode themselves.

JetKVM's own JSON-RPC getTLSState/setTLSState calls require an
authenticated WebRTC session (see jetkvm/kvm#1240 and the still-open
jetkvm/kvm#1515), so there's no documented/headless way to query this.
Its firmware (web_tls.go / config.go in jetkvm/kvm) does persist the
mode as a plain JSON field, "tls_mode" (values "", "self-signed", or
"custom"), in /userdata/kvm_config.json -- confirmed against a real
device, including that its busybox grep handles the -E/[[:space:]]
regex used here.

The check runs as the first step of the existing upload SSH call (no
extra round trip), exits a dedicated code (3) if "tls_mode" isn't
"custom", and the hook surfaces that as a specific, actionable error
pointing at the device's web UI setting, distinct from a generic upload
failure. Since the underlying config file/field is just as undocumented
as everything else this hook depends on, DEPLOY_JETKVM_REQUIRE_CUSTOM_MODE=no
opts out entirely in case a future firmware version changes the format.

Also tightens two things noticed while adding this: the "Uploading
certificate..." info log no longer prints before a call that might
immediately fail the mode check, and the remote script's own error
echo (redundant with the local hook's more detailed _err message) is
dropped.

Confirmed end-to-end against a real JetKVM device: the regex correctly
matched the device's actual tls_mode=custom, and a full run of the
updated hook (mode check included) succeeded.

* Address maintainer review: hardcode firmware constants, drop local temp files, fix POSIX portability

Per @neilpang's review (acmesh-official/acme.sh#7254):

1. Drop [[:space:]]/-E from the tls_mode grep -- not portable (Solaris
sed/grep read it as a literal bracket set); the compact and indented
JSON forms are both covered by a plain space with '*'.
2. Use the core _time() wrapper instead of `date +%s || echo 0` -- the
fallback was dead code (a date binary that doesn't understand %s
still exits 0), and _time() is the idiom every other hook/dnsapi
script already uses for this.
3. Drop the local temp files entirely for both the upload and restart
SSH calls. The upload script is now built in a variable and piped
directly into `ssh ... sh` (same pattern as deploy/windows_rdp.sh);
$? after the pipeline is still ssh's own exit code. This keeps the
private key off local disk and removes the _mktemp/chmod/rm dance.
4. Refuse to deploy when the key or fullchain file is empty (e.g. a
--signcsr-only run) instead of uploading an empty key file and
rebooting the device.
5. Use `printf '%s\n'`, not `echo`, for every generated script line --
dash's echo interprets backslash escapes, so the remote script's
content would otherwise depend on which /bin/sh happens to run
acme.sh.
6. Save DEPLOY_JETKVM_SSH_CMD and DEPLOY_JETKVM_RESTART_CMD with
_savedeployconf's "base64" flag (as deploy/docker.sh does for its
own reload command), since a value containing a single quote would
otherwise break the saved domain.conf line.
7. Distinguish "config file missing/unreadable" from "HTTPS Mode isn't
Custom" with separate exit codes -- grep's own exit 2 for a missing
file was previously funneled into the same "not custom" error,
misdiagnosing the actual problem. Also stopped suggesting
REQUIRE_CUSTOM_MODE=no in that error message: following it silently
turns every future deploy into a no-op once persisted to domain.conf.
8. Hardcode the remote path, filenames, chmod values and config file
path as constants instead of DEPLOY_JETKVM_* variables. They're
firmware facts on a single-purpose, single-root appliance, not user
configuration -- and since _savedeployconf pins whatever value is
first used into domain.conf, a firmware-side correction to one of
these later would never reach anyone who'd already deployed once.
Only USER/HOST/PORT/SSH_CMD/RESTART_CMD/REQUIRE_CUSTOM_MODE remain.
9. Run the restart command detached (nohup sh -c 'sleep N; $CMD' &) so
the ssh call returns as soon as it's launched, before the device
actually reboots, instead of racing the connection teardown. This
also removes the marker/case-based "0 or 255" exit-code logic
entirely, along with the bug it had: a connection dropping after the
marker printed but before the restart command actually ran was
previously reported as a successful deploy. The tradeoff (also
called out inline and in the PR description): a restart command that
fails after being launched can no longer be detected, only a failure
to launch it at all.
10. Use fixed temp filenames for the staged cert/key (not one new name
per run) plus a `trap ... EXIT` in the generated script, so any
abort (the mode check, a write failure under `set -e`) cleans up
instead of leaving another stray key-bearing file on the device.
11. Trimmed the header: removed the marker/0|255 rationale (obsoleted by
#9), corrected the "typically overnight" claim about when the
restart actually runs, and added a wiki reference.

Not yet done: a deployhooks wiki entry (acmesh-official/acme.sh#7254's
point 12) -- flagged in the PR thread since only a repo collaborator can
edit that wiki.

Verified: shellcheck (no exclusions needed anymore) and shfmt -i 2
clean; a local smoke-test harness (stubbed acme.sh core, a fake ssh
that redirects the hardcoded device paths into a scratch directory)
covering a clean deploy with byte-exact content/permissions and no
leftover staged files, RESTART_CMD=none, HTTPS Mode not "custom",
the config file missing entirely (now a distinct error),
REQUIRE_CUSTOM_MODE=no, an empty key file (--signcsr case), and a
failed restart-command launch.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01M7rTpUF3btoBXSZ95Psjh7

* Fix restart-command quoting and correct the failure-detection comment

Per @neilpang's second review round:

1. DEPLOY_JETKVM_RESTART_CMD was interpolated unescaped inside the
detached command's own single-quoted "sh -c '...'" wrapper. A value
containing a single quote (e.g. "sh -c 'sync; reboot'") broke that
quoting, splitting the string so only part of the intended command
ran, un-detached. Escape embedded single quotes (the standard
'\'' substitution) before nesting the value, matching how a value
with no quotes at all still behaves identically. Verified against
sh and dash directly, and with a new local smoke-test case that
actually executes the generated detached command and confirms both
halves of a quoted restart command run intact.

2. The comment claiming "only a failure to launch it at all is caught
below" was wrong: since the restart command runs as an unwaited
background job (nohup ... &), the remote sh returns 0 as soon as
that job is launched, regardless of whether nohup, sh, or the
restart command itself actually exist or succeed -- measured 0 in
both cases. Reworded so the comment describes what's actually
caught (an outright SSH connection failure) instead of implying a
guarantee the code doesn't provide. No behavior change from this
half of the fix, comment-only.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01M7rTpUF3btoBXSZ95Psjh7

---------

Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>