diff --git a/docs/TEMPERATURE_MONITORING.md b/docs/TEMPERATURE_MONITORING.md index bc1063438a..624fe83fad 100644 --- a/docs/TEMPERATURE_MONITORING.md +++ b/docs/TEMPERATURE_MONITORING.md @@ -1,6 +1,7 @@ # Temperature Monitoring -Pulse can collect host temperatures in two supported ways: +Pulse collects temperatures exposed by the host's existing drivers. For Linux +hosts, prefer the local agent; SSH is a legacy fallback, not a prerequisite: - Pulse agent on Proxmox hosts (recommended) - SSH-based collection from the Pulse server (fallback or for non-agent hosts) @@ -26,10 +27,60 @@ The Pulse agent token is not a Proxmox API token. Keep the secret out of command arguments and leave command execution disabled for monitoring. Notes: -- Install `lm-sensors` on each host (`apt install lm-sensors && sensors-detect --auto`). -- Temperatures appear automatically once the agent reports. +- On Linux, the agent reads `sensors -j` when available and supplements missing + CPU readings from recognised CPU/SoC thermal sysfs sources. `lm-sensors` is + not required for that fallback; empty output does not mean a temperature of + zero. Other sensor types need their own supported driver/provider. +- Check the active agent's version, last report and the affected host's reading + before changing its setup. A current server does not upgrade every agent. - When a Proxmox host has recent usable agent temperature data, Pulse treats the agent as the source of truth and does not also try SSH temperature collection for that host. +### Check existing Linux readings safely + +Use the monitored host's trusted local console, not the Pulse LXC or container. +Do not run `sensors-detect --auto`, bus scans, driver loading or a reboot merely +because Pulse shows no temperature: hardware probing can disrupt a running +host. Any necessary driver setup belongs in that host's normal maintenance +window, following its hardware documentation, not in a diagnostic retry loop. + +If `sensors` is already installed, this Bash check reads it once, limits the +command to five seconds (with a two-second forced-stop grace), and saves both +output streams privately. It installs nothing and prints only status and paths: + +```bash +( +set -eu +umask 077 +command -v sensors >/dev/null || { printf 'sensors is not installed; check the agent sysfs fallback.\n'; exit 1; } +command -v timeout >/dev/null || { printf 'GNU timeout is required for this bounded check.\n'; exit 1; } +result_dir=$(mktemp -d "${TMPDIR:-/tmp}/pulse-temperature.XXXXXX") || exit $? +check_exit=0 +timeout --signal=TERM --kill-after=2s 5s sensors -j \ + > "$result_dir/sensors.json" 2> "$result_dir/sensors.err" || check_exit=$? +printf 'Sensor command exit %s; private output in %s\n' "$check_exit" "$result_dir" +exit "$check_exit" +) +``` + +Inspect those files locally; non-zero exit can accompany partial sensor data, +and exit zero or `{}` does not establish a usable CPU reading. A timeout is a +failed check, not an empty reading or permission to repeat it. Do not post the +whole output: it can identify hardware and disks. + +Without usable CPU output, inspect the already exposed sysfs files locally: +pair `/sys/class/thermal/thermal_zone*/type` with its `temp`, or +`/sys/class/hwmon/hwmon*/name` with its `temp1_input`. Values are millidegrees +Celsius: `57000` is **57 °C**. `thermal_zone0` is not necessarily the CPU; +compare the sensor type/name, not just its number. The agent accepts recognised +CPU/SoC names, not every thermal zone as a CPU. Missing, unreadable, invalid or +unsupported readings remain unavailable; do not change permissions or mount +host device trees into a container just to make this check pass. + +Compare an existing local reading with the **same host and sensor**, current +Machines value and CPU-temperature History at the corresponding time. CPU/SoC +temperature is not physical-disk SMART temperature. A plausible value for one +sensor does not verify all sensors or historical continuity. + ## Windows temperatures On Windows hosts with an NVIDIA driver, the unified agent uses the driver's @@ -62,7 +113,7 @@ Pulse can also collect temperatures by SSHing into each host that does not have ### Requirements - SSH connectivity from the Pulse server to each host -- `lm-sensors` installed and `sensors -j` returning JSON on the host +- Existing usable sensor data on the host; normally `sensors -j` JSON - A restricted SSH key entry that only allows the Pulse sensor wrapper ### Setup @@ -84,30 +135,49 @@ If you use a non-standard SSH port, set `SSH_PORT` (system-wide) or configure it ### Containerized Pulse -SSH-based collection from inside a container is not recommended for production. Prefer the agent method or run Pulse on the host. For dev/test, you can allow SSH from the container with: - -```bash -PULSE_DEV_ALLOW_CONTAINER_SSH=true -``` +Prefer the local agent. A legacy configured key can still cause SSH fallback +from a container, with a production-security warning; the development override +changes that warning, not whether sensor data exists. Do not mount root SSH +keys into the Pulse container or enable a development override to repair a +missing reading. ### Verification -From the Pulse server, verify that SSH and sensors output work: +Only for an already configured SSH fallback, use its existing restricted key +and separately verified host-key trust from the Pulse server's account. Do not +accept a new or changed host key merely to obtain a temperature. This Bash +check requests the sensor wrapper once, with a fifteen-second overall bound +and two-second forced-stop grace, and keeps output private: ```bash -ssh -i /path/to/key root@node "sensors -j" +( +set -eu +umask 077 +command -v timeout >/dev/null || { printf 'GNU timeout is required for this bounded check.\n'; exit 1; } +result_dir=$(mktemp -d "${TMPDIR:-/tmp}/pulse-temperature-ssh.XXXXXX") || exit $? +check_exit=0 +timeout --signal=TERM --kill-after=2s 15s \ + ssh -T -i /path/to/pulse-key -o BatchMode=yes -o IdentitiesOnly=yes \ + -o StrictHostKeyChecking=yes -o ConnectTimeout=5 -o ConnectionAttempts=1 \ + -o ServerAliveInterval=5 -o ServerAliveCountMax=1 \ + root@node /usr/local/sbin/pulse-sensors \ + > "$result_dir/wrapper.json" 2> "$result_dir/ssh.err" || check_exit=$? +printf 'SSH command exit %s; private output in %s\n' "$check_exit" "$result_dir" +exit "$check_exit" +) ``` -For platforms that expose a thermal zone file: - -```bash -ssh -i /path/to/key root@node "cat /sys/class/thermal/thermal_zone0/temp" -``` +Replace only the key path and node with the configured ones. The forced key +command can ignore the requested command: current wrappers return `sensors` +and `smart` data, while older keys can return bare `sensors -j` JSON. Do not +remove the key restrictions to run another probe, or treat wrapper JSON as a +raw thermal-zone number. An SSH failure, partial output or timeout does not +prove missing hardware. Keep the wrapper output and error private; share only +the exit, relevant redacted error and affected sensor if help is needed. ### Troubleshooting -- If `sensors -j` returns empty output, run `sensors-detect --auto` and retry. -- If temperatures show as unavailable, confirm the host actually exposes sensor data. +- If temperatures are unavailable, use the [local reading check](#check-existing-linux-readings-safely) before considering setup changes. An empty reading is not a reason for automatic hardware detection. - If the unified agent is already reporting temperatures for a Proxmox host, SSH collection is not required for that host. - Ensure the SSH key entry is present and restricted to `/usr/local/sbin/pulse-sensors`. @@ -115,18 +185,44 @@ ssh -i /path/to/key root@node "cat /sys/class/thermal/thermal_zone0/temp" If you still have the old sensor proxy installed from prior releases, remove it from each **Proxmox host** (not the Pulse container) with the supported cleanup -helper. In that host's administrative shell, download it to a private directory: +helper. In that host's trusted administrative shell, download it privately. +This refuses an existing target, symlinked configuration directory or +failed/partial download; it does not overwrite an earlier helper or run a +response as a command: ```bash +( +set -eu umask 077 -mkdir -p "$HOME/.config/pulse" -chmod 700 "$HOME/.config/pulse" -curl --fail --silent --show-error --connect-timeout 10 --max-time 60 \ - --output "$HOME/.config/pulse/sensor-proxy-uninstall.sh" \ - https://raw.githubusercontent.com/rcourtman/Pulse/main/scripts/uninstall-sensor-proxy.sh +config_dir="$HOME/.config/pulse" +helper_file="$config_dir/sensor-proxy-uninstall.sh" +if [ -L "$HOME/.config" ] || [ -L "$config_dir" ]; then + printf 'Refusing a symlinked configuration directory.\n'; exit 1 +fi +mkdir -p "$config_dir" || exit $? +chmod 700 "$config_dir" || exit $? +if [ -e "$helper_file" ] || [ -L "$helper_file" ]; then + printf 'Helper path already exists; inspect it locally before choosing a replacement.\n'; exit 1 +fi +download_file=$(mktemp "$config_dir/sensor-cleanup-download.XXXXXX") || exit $? +curl_exit=0 +status=$(curl --disable --fail --silent --show-error --proto '=https' \ + --connect-timeout 5 --max-time 60 --output "$download_file" --write-out '%{http_code}' \ + https://raw.githubusercontent.com/rcourtman/Pulse/main/scripts/uninstall-sensor-proxy.sh) || curl_exit=$? +printf 'HTTP %s\n' "$status" +[ "$curl_exit" -eq 0 ] || exit "$curl_exit" +[ "$status" = 200 ] || exit 1 +mv -n "$download_file" "$helper_file" || exit $? +[ ! -e "$download_file" ] || exit 1 +printf 'Inspect the complete private helper at %s before running it.\n' "$helper_file" +) ``` -Stop if the download fails and inspect the saved script before running it. +Stop if preparation or download fails; a temporary response is not the helper. +Curl's local configuration is ignored, redirects are not followed and TLS +verification stays enabled. This is an HTTPS download from the repository's +current main, not a signed release asset. Inspect the complete saved script +and choose the cleanup scope before running it. Do not pipe a web response into a privileged shell: ```bash diff --git a/docs/TROUBLESHOOTING.md b/docs/TROUBLESHOOTING.md index f05100da0a..854694ec02 100644 --- a/docs/TROUBLESHOOTING.md +++ b/docs/TROUBLESHOOTING.md @@ -240,10 +240,16 @@ repair an older generated unit rather than adding a JSON-parsing wrapper. independent guest thaw confirmation before starting Pulse again. #### Temperature data missing -- Install `lm-sensors` on the host. -- Run `sensors-detect`. -- Install the unified agent on the Proxmox host with `--enable-proxmox`. -- See [Temperature Monitoring](TEMPERATURE_MONITORING.md). +- Compare the affected host's active agent version, last report, sensor and + observation time. A current server or another sensor's value is not evidence + that this reading is available. +- Use the [bounded local reading check](TEMPERATURE_MONITORING.md#check-existing-linux-readings-safely) + on the monitored host, not the Pulse container. Linux agents can use existing + recognised CPU/SoC sysfs readings without `lm-sensors`; unavailable is not zero. +- Do not run automatic hardware detection, load drivers, reboot or loosen SSH + restrictions merely to fill a temperature row. Retain output privately and + follow the [platform-specific guide](TEMPERATURE_MONITORING.md) for any needed + setup change during a maintenance window. #### Docker hosts appearing/disappearing diff --git a/frontend-modern/public/docs/TEMPERATURE_MONITORING.md b/frontend-modern/public/docs/TEMPERATURE_MONITORING.md index bc1063438a..624fe83fad 100644 --- a/frontend-modern/public/docs/TEMPERATURE_MONITORING.md +++ b/frontend-modern/public/docs/TEMPERATURE_MONITORING.md @@ -1,6 +1,7 @@ # Temperature Monitoring -Pulse can collect host temperatures in two supported ways: +Pulse collects temperatures exposed by the host's existing drivers. For Linux +hosts, prefer the local agent; SSH is a legacy fallback, not a prerequisite: - Pulse agent on Proxmox hosts (recommended) - SSH-based collection from the Pulse server (fallback or for non-agent hosts) @@ -26,10 +27,60 @@ The Pulse agent token is not a Proxmox API token. Keep the secret out of command arguments and leave command execution disabled for monitoring. Notes: -- Install `lm-sensors` on each host (`apt install lm-sensors && sensors-detect --auto`). -- Temperatures appear automatically once the agent reports. +- On Linux, the agent reads `sensors -j` when available and supplements missing + CPU readings from recognised CPU/SoC thermal sysfs sources. `lm-sensors` is + not required for that fallback; empty output does not mean a temperature of + zero. Other sensor types need their own supported driver/provider. +- Check the active agent's version, last report and the affected host's reading + before changing its setup. A current server does not upgrade every agent. - When a Proxmox host has recent usable agent temperature data, Pulse treats the agent as the source of truth and does not also try SSH temperature collection for that host. +### Check existing Linux readings safely + +Use the monitored host's trusted local console, not the Pulse LXC or container. +Do not run `sensors-detect --auto`, bus scans, driver loading or a reboot merely +because Pulse shows no temperature: hardware probing can disrupt a running +host. Any necessary driver setup belongs in that host's normal maintenance +window, following its hardware documentation, not in a diagnostic retry loop. + +If `sensors` is already installed, this Bash check reads it once, limits the +command to five seconds (with a two-second forced-stop grace), and saves both +output streams privately. It installs nothing and prints only status and paths: + +```bash +( +set -eu +umask 077 +command -v sensors >/dev/null || { printf 'sensors is not installed; check the agent sysfs fallback.\n'; exit 1; } +command -v timeout >/dev/null || { printf 'GNU timeout is required for this bounded check.\n'; exit 1; } +result_dir=$(mktemp -d "${TMPDIR:-/tmp}/pulse-temperature.XXXXXX") || exit $? +check_exit=0 +timeout --signal=TERM --kill-after=2s 5s sensors -j \ + > "$result_dir/sensors.json" 2> "$result_dir/sensors.err" || check_exit=$? +printf 'Sensor command exit %s; private output in %s\n' "$check_exit" "$result_dir" +exit "$check_exit" +) +``` + +Inspect those files locally; non-zero exit can accompany partial sensor data, +and exit zero or `{}` does not establish a usable CPU reading. A timeout is a +failed check, not an empty reading or permission to repeat it. Do not post the +whole output: it can identify hardware and disks. + +Without usable CPU output, inspect the already exposed sysfs files locally: +pair `/sys/class/thermal/thermal_zone*/type` with its `temp`, or +`/sys/class/hwmon/hwmon*/name` with its `temp1_input`. Values are millidegrees +Celsius: `57000` is **57 °C**. `thermal_zone0` is not necessarily the CPU; +compare the sensor type/name, not just its number. The agent accepts recognised +CPU/SoC names, not every thermal zone as a CPU. Missing, unreadable, invalid or +unsupported readings remain unavailable; do not change permissions or mount +host device trees into a container just to make this check pass. + +Compare an existing local reading with the **same host and sensor**, current +Machines value and CPU-temperature History at the corresponding time. CPU/SoC +temperature is not physical-disk SMART temperature. A plausible value for one +sensor does not verify all sensors or historical continuity. + ## Windows temperatures On Windows hosts with an NVIDIA driver, the unified agent uses the driver's @@ -62,7 +113,7 @@ Pulse can also collect temperatures by SSHing into each host that does not have ### Requirements - SSH connectivity from the Pulse server to each host -- `lm-sensors` installed and `sensors -j` returning JSON on the host +- Existing usable sensor data on the host; normally `sensors -j` JSON - A restricted SSH key entry that only allows the Pulse sensor wrapper ### Setup @@ -84,30 +135,49 @@ If you use a non-standard SSH port, set `SSH_PORT` (system-wide) or configure it ### Containerized Pulse -SSH-based collection from inside a container is not recommended for production. Prefer the agent method or run Pulse on the host. For dev/test, you can allow SSH from the container with: - -```bash -PULSE_DEV_ALLOW_CONTAINER_SSH=true -``` +Prefer the local agent. A legacy configured key can still cause SSH fallback +from a container, with a production-security warning; the development override +changes that warning, not whether sensor data exists. Do not mount root SSH +keys into the Pulse container or enable a development override to repair a +missing reading. ### Verification -From the Pulse server, verify that SSH and sensors output work: +Only for an already configured SSH fallback, use its existing restricted key +and separately verified host-key trust from the Pulse server's account. Do not +accept a new or changed host key merely to obtain a temperature. This Bash +check requests the sensor wrapper once, with a fifteen-second overall bound +and two-second forced-stop grace, and keeps output private: ```bash -ssh -i /path/to/key root@node "sensors -j" +( +set -eu +umask 077 +command -v timeout >/dev/null || { printf 'GNU timeout is required for this bounded check.\n'; exit 1; } +result_dir=$(mktemp -d "${TMPDIR:-/tmp}/pulse-temperature-ssh.XXXXXX") || exit $? +check_exit=0 +timeout --signal=TERM --kill-after=2s 15s \ + ssh -T -i /path/to/pulse-key -o BatchMode=yes -o IdentitiesOnly=yes \ + -o StrictHostKeyChecking=yes -o ConnectTimeout=5 -o ConnectionAttempts=1 \ + -o ServerAliveInterval=5 -o ServerAliveCountMax=1 \ + root@node /usr/local/sbin/pulse-sensors \ + > "$result_dir/wrapper.json" 2> "$result_dir/ssh.err" || check_exit=$? +printf 'SSH command exit %s; private output in %s\n' "$check_exit" "$result_dir" +exit "$check_exit" +) ``` -For platforms that expose a thermal zone file: - -```bash -ssh -i /path/to/key root@node "cat /sys/class/thermal/thermal_zone0/temp" -``` +Replace only the key path and node with the configured ones. The forced key +command can ignore the requested command: current wrappers return `sensors` +and `smart` data, while older keys can return bare `sensors -j` JSON. Do not +remove the key restrictions to run another probe, or treat wrapper JSON as a +raw thermal-zone number. An SSH failure, partial output or timeout does not +prove missing hardware. Keep the wrapper output and error private; share only +the exit, relevant redacted error and affected sensor if help is needed. ### Troubleshooting -- If `sensors -j` returns empty output, run `sensors-detect --auto` and retry. -- If temperatures show as unavailable, confirm the host actually exposes sensor data. +- If temperatures are unavailable, use the [local reading check](#check-existing-linux-readings-safely) before considering setup changes. An empty reading is not a reason for automatic hardware detection. - If the unified agent is already reporting temperatures for a Proxmox host, SSH collection is not required for that host. - Ensure the SSH key entry is present and restricted to `/usr/local/sbin/pulse-sensors`. @@ -115,18 +185,44 @@ ssh -i /path/to/key root@node "cat /sys/class/thermal/thermal_zone0/temp" If you still have the old sensor proxy installed from prior releases, remove it from each **Proxmox host** (not the Pulse container) with the supported cleanup -helper. In that host's administrative shell, download it to a private directory: +helper. In that host's trusted administrative shell, download it privately. +This refuses an existing target, symlinked configuration directory or +failed/partial download; it does not overwrite an earlier helper or run a +response as a command: ```bash +( +set -eu umask 077 -mkdir -p "$HOME/.config/pulse" -chmod 700 "$HOME/.config/pulse" -curl --fail --silent --show-error --connect-timeout 10 --max-time 60 \ - --output "$HOME/.config/pulse/sensor-proxy-uninstall.sh" \ - https://raw.githubusercontent.com/rcourtman/Pulse/main/scripts/uninstall-sensor-proxy.sh +config_dir="$HOME/.config/pulse" +helper_file="$config_dir/sensor-proxy-uninstall.sh" +if [ -L "$HOME/.config" ] || [ -L "$config_dir" ]; then + printf 'Refusing a symlinked configuration directory.\n'; exit 1 +fi +mkdir -p "$config_dir" || exit $? +chmod 700 "$config_dir" || exit $? +if [ -e "$helper_file" ] || [ -L "$helper_file" ]; then + printf 'Helper path already exists; inspect it locally before choosing a replacement.\n'; exit 1 +fi +download_file=$(mktemp "$config_dir/sensor-cleanup-download.XXXXXX") || exit $? +curl_exit=0 +status=$(curl --disable --fail --silent --show-error --proto '=https' \ + --connect-timeout 5 --max-time 60 --output "$download_file" --write-out '%{http_code}' \ + https://raw.githubusercontent.com/rcourtman/Pulse/main/scripts/uninstall-sensor-proxy.sh) || curl_exit=$? +printf 'HTTP %s\n' "$status" +[ "$curl_exit" -eq 0 ] || exit "$curl_exit" +[ "$status" = 200 ] || exit 1 +mv -n "$download_file" "$helper_file" || exit $? +[ ! -e "$download_file" ] || exit 1 +printf 'Inspect the complete private helper at %s before running it.\n' "$helper_file" +) ``` -Stop if the download fails and inspect the saved script before running it. +Stop if preparation or download fails; a temporary response is not the helper. +Curl's local configuration is ignored, redirects are not followed and TLS +verification stays enabled. This is an HTTPS download from the repository's +current main, not a signed release asset. Inspect the complete saved script +and choose the cleanup scope before running it. Do not pipe a web response into a privileged shell: ```bash diff --git a/frontend-modern/public/docs/TROUBLESHOOTING.md b/frontend-modern/public/docs/TROUBLESHOOTING.md index f05100da0a..854694ec02 100644 --- a/frontend-modern/public/docs/TROUBLESHOOTING.md +++ b/frontend-modern/public/docs/TROUBLESHOOTING.md @@ -240,10 +240,16 @@ repair an older generated unit rather than adding a JSON-parsing wrapper. independent guest thaw confirmation before starting Pulse again. #### Temperature data missing -- Install `lm-sensors` on the host. -- Run `sensors-detect`. -- Install the unified agent on the Proxmox host with `--enable-proxmox`. -- See [Temperature Monitoring](TEMPERATURE_MONITORING.md). +- Compare the affected host's active agent version, last report, sensor and + observation time. A current server or another sensor's value is not evidence + that this reading is available. +- Use the [bounded local reading check](TEMPERATURE_MONITORING.md#check-existing-linux-readings-safely) + on the monitored host, not the Pulse container. Linux agents can use existing + recognised CPU/SoC sysfs readings without `lm-sensors`; unavailable is not zero. +- Do not run automatic hardware detection, load drivers, reboot or loosen SSH + restrictions merely to fill a temperature row. Retain output privately and + follow the [platform-specific guide](TEMPERATURE_MONITORING.md) for any needed + setup change during a maintenance window. #### Docker hosts appearing/disappearing diff --git a/scripts/installtests/temperature_guidance_test.go b/scripts/installtests/temperature_guidance_test.go new file mode 100644 index 0000000000..297d1747a2 --- /dev/null +++ b/scripts/installtests/temperature_guidance_test.go @@ -0,0 +1,350 @@ +package installtests + +import ( + "encoding/pem" + "fmt" + "io" + "log" + "net/http" + "net/http/httptest" + "os" + "os/exec" + "path/filepath" + "regexp" + "strings" + "testing" +) + +// Exercise the actual copied recipes without hardware access, remote SSH or +// privileged cleanup. Network controls use only synthetic guest-local TLS. +func temperatureRecipes(t *testing.T) (local, ssh, download string) { + t.Helper() + for _, name := range []string{"TEMPERATURE_MONITORING.md", "TROUBLESHOOTING.md"} { + doc, err := os.ReadFile(repoFile("docs", name)) + if err != nil { + t.Fatal(err) + } + mirror, err := os.ReadFile(repoFile("frontend-modern", "public", "docs", name)) + if err != nil || string(doc) != string(mirror) { + t.Fatalf("%s shipped mirror differs from the tested guide", name) + } + if name == "TROUBLESHOOTING.md" { + section := strings.Split(strings.Split(string(doc), "#### Temperature data missing\n")[1], "\n#### ")[0] + if !strings.Contains(section, "TEMPERATURE_MONITORING.md#check-existing-linux-readings-safely") || + strings.Contains(section, "sensors-detect") { + t.Fatal("troubleshooting must lead to the passive check, not hardware detection") + } + continue + } + for _, match := range regexp.MustCompile("(?s)```bash\n(.*?)```").FindAllStringSubmatch(string(doc), -1) { + block := match[1] + if strings.Contains(block, "sensors-detect") || strings.Contains(block, "modprobe") { + t.Fatal("copied diagnostic recipe performs hardware detection or driver loading") + } + switch { + case strings.Contains(block, "5s sensors -j"): + local = block + case strings.Contains(block, "ssh -T "): + ssh = block + case strings.Contains(block, "curl "): + download = block + } + } + } + if local == "" || ssh == "" || download == "" { + t.Fatal("temperature guide lacks a bounded local, restricted SSH or private download recipe") + } + return +} + +func TestTemperatureGuidanceKeepsExistingSafetyAndSourceMeaning(t *testing.T) { + local, ssh, download := temperatureRecipes(t) + for _, recipe := range []string{local, ssh, download} { + for _, required := range []string{"set -eu", "umask 077"} { + if !strings.Contains(recipe, required) { + t.Fatalf("copied recipe lacks %q", required) + } + } + } + for _, recipe := range []string{local, ssh} { + for _, required := range []string{"timeout --signal=TERM --kill-after=2s", "mktemp -d", "|| check_exit=$?", `exit "$check_exit"`} { + if !strings.Contains(recipe, required) { + t.Fatalf("reading recipe lacks %q", required) + } + } + } + for _, required := range []string{"BatchMode=yes", "IdentitiesOnly=yes", "StrictHostKeyChecking=yes", "ConnectionAttempts=1", "/usr/local/sbin/pulse-sensors"} { + if !strings.Contains(ssh, required) { + t.Fatalf("SSH recipe lacks %q", required) + } + } + for _, forbidden := range []string{"StrictHostKeyChecking=no", "accept-new", "UserKnownHostsFile=/dev/null", "sensors-detect", "thermal_zone0/temp"} { + if strings.Contains(ssh, forbidden) { + t.Fatalf("SSH recipe weakens trust or bypasses the forced wrapper: %q", forbidden) + } + } + for _, required := range []string{"curl --disable --fail ", "--proto '=https'", "--connect-timeout 5 --max-time 60", `[ "$status" = 200 ]`, `mv -n "$download_file" "$helper_file"`, `[ ! -e "$download_file" ]`} { + if !strings.Contains(download, required) { + t.Fatalf("helper download lacks %q", required) + } + } + for _, forbidden := range []string{"--insecure", "--location", "--retry", " | ", "bash "} { + if strings.Contains(download, forbidden) { + t.Fatalf("download bypasses verification or runs its response: %q", forbidden) + } + } + doc, err := os.ReadFile(repoFile("docs", "TEMPERATURE_MONITORING.md")) + if err != nil { + t.Fatal(err) + } + for _, text := range []string{"thermal_zone0` is not necessarily the CPU", "millidegrees", "not physical-disk SMART", "--token-file", "--local-only", "--ssh-known-hosts", "--remove-proxmox-access", "not a signed release asset", "not an empty reading"} { + if !strings.Contains(string(doc), text) { + t.Fatalf("temperature guide loses existing scope or source distinction %q", text) + } + } +} + +func TestTemperatureGuidanceReadingCommandsArePrivateAndBounded(t *testing.T) { + local, ssh, _ := temperatureRecipes(t) + for index, recipe := range []string{local, ssh} { + for _, scenario := range []string{"success", "partial failure", "hung command", "temporary directory failure", "missing timeout"} { + t.Run(fmt.Sprintf("%d/%s", index, scenario), func(t *testing.T) { + home, bin, tmp := t.TempDir(), t.TempDir(), t.TempDir() + tool := "sensors" + outName, errName := "sensors.json", "sensors.err" + if index == 1 { + tool, outName, errName = "ssh", "wrapper.json", "ssh.err" + } + capture := filepath.Join(home, "argv") + const out = "synthetic-private-sensor-disk-identity" + const diagnostic = "synthetic-private-tool-error" + stub := "#!/bin/sh\nprintf 'call\\n' >> \"$TEMP_ARGV\"\nprintf '%s\\n' \"$@\" >> \"$TEMP_ARGV\"\nprintf '" + out + "'\nprintf '" + diagnostic + "' >&2\n" + wantExit := 0 + switch scenario { + case "partial failure": + wantExit = 2 + stub += "exit 2\n" + case "hung command": + // The real GNU timeout must enforce its TERM/KILL bound; the + // stub replaces itself, so no orphan child is manufactured. + stub += "trap '' TERM\nexec sleep 40\n" + wantExit = 137 + case "temporary directory failure": + wantExit = 42 + if err := os.WriteFile(filepath.Join(bin, "mktemp"), []byte("#!/bin/sh\nexit 42\n"), 0700); err != nil { + t.Fatal(err) + } + case "missing timeout": + wantExit = 1 + } + if err := os.WriteFile(filepath.Join(bin, tool), []byte(stub), 0700); err != nil { + t.Fatal(err) + } + env := []string{"TEMP_ARGV=" + capture, "TMPDIR=" + tmp} + if scenario == "missing timeout" { + env = append(env, "PATH="+bin) + } + output, exit := runPBSRecipe(t, recipe, home, bin, env...) + if exit != wantExit { + t.Fatalf("reading recipe exit %d, want %d: %s", exit, wantExit, output) + } + if strings.Contains(output, out) || strings.Contains(output, diagnostic) { + t.Fatal("reading recipe disclosed private output") + } + argv, err := os.ReadFile(capture) + if scenario == "missing timeout" || scenario == "temporary directory failure" { + if !os.IsNotExist(err) { + t.Fatal("failed preparation reached the reading tool") + } + return + } + if err != nil || strings.Count(string(argv), "call\n") != 1 { + t.Fatal("reading tool was not called exactly once") + } + if index == 0 && string(argv) != "call\n-j\n" { + t.Fatal("local check did not perform only the existing sensors JSON read") + } + if index == 1 { + want := "call\n-T\n-i\n/path/to/pulse-key\n-o\nBatchMode=yes\n-o\nIdentitiesOnly=yes\n-o\nStrictHostKeyChecking=yes\n-o\nConnectTimeout=5\n-o\nConnectionAttempts=1\n-o\nServerAliveInterval=5\n-o\nServerAliveCountMax=1\nroot@node\n/usr/local/sbin/pulse-sensors\n" + if string(argv) != want { + t.Fatal("SSH read changed the key, trust, bounded transport or forced wrapper arguments") + } + } + dirs, err := filepath.Glob(filepath.Join(tmp, "pulse-temperature*")) + if err != nil || len(dirs) != 1 { + t.Fatal("reading did not create one private result directory") + } + for name, want := range map[string]string{outName: out, errName: diagnostic} { + path := filepath.Join(dirs[0], name) + content, err := os.ReadFile(path) + info, statErr := os.Stat(path) + if err != nil || string(content) != want || statErr != nil || info.Mode().Perm() != 0600 { + t.Fatal("reading output was not retained completely and privately") + } + } + info, err := os.Stat(dirs[0]) + if err != nil || info.Mode().Perm() != 0700 { + t.Fatal("result directory is not owner-only") + } + }) + } + } +} + +func TestTemperatureGuidanceCleanupDownloadPreservesFilesAndTrust(t *testing.T) { + _, _, original := temperatureRecipes(t) + for _, scenario := range []string{"success", "401", "403", "redirect", "wrong success status", "untrusted HTTPS", "truncated response", "existing helper", "helper symlink", "dangling helper", "helper directory", "configuration symlink", "pulse directory symlink", "mkdir failure", "chmod failure"} { + t.Run(scenario, func(t *testing.T) { + realCurl, err := exec.LookPath("curl") + if err != nil { + t.Fatal("curl is required to verify copied download guidance") + } + home, bin := t.TempDir(), t.TempDir() + dir := filepath.Join(home, ".config", "pulse") + helper := filepath.Join(dir, "sensor-proxy-uninstall.sh") + if err := os.MkdirAll(dir, 0755); err != nil { + t.Fatal(err) + } + const preserved = "original-helper-or-link-target" + target := filepath.Join(home, "untouched") + if err := os.WriteFile(target, []byte(preserved), 0600); err != nil { + t.Fatal(err) + } + switch scenario { + case "existing helper": + if err := os.WriteFile(helper, []byte(preserved), 0600); err != nil { + t.Fatal(err) + } + case "helper symlink", "dangling helper": + linkTarget := target + if scenario == "dangling helper" { + linkTarget += "-missing" + } + if err := os.Symlink(linkTarget, helper); err != nil { + t.Fatal(err) + } + case "helper directory": + if err := os.Mkdir(helper, 0700); err != nil { + t.Fatal(err) + } + case "configuration symlink", "pulse directory symlink": + // Empty fixture directories only; never remove source or data. + if err := os.Remove(dir); err != nil { + t.Fatal(err) + } + link := dir + if scenario == "configuration symlink" { + link = filepath.Dir(dir) + if err := os.Remove(link); err != nil { + t.Fatal(err) + } + } + if err := os.Symlink(home, link); err != nil { + t.Fatal(err) + } + case "mkdir failure", "chmod failure": + tool := strings.Fields(scenario)[0] + if err := os.WriteFile(filepath.Join(bin, tool), []byte("#!/bin/sh\nexit 42\n"), 0700); err != nil { + t.Fatal(err) + } + } + trace, argv := filepath.Join(home, "must-not-trace"), filepath.Join(home, "argv") + if err := os.WriteFile(filepath.Join(home, ".curlrc"), []byte("insecure\nlocation\ntrace = "+trace+"\n"), 0600); err != nil { + t.Fatal(err) + } + stub := "#!/bin/sh\nprintf '%s\\0' \"$@\" > \"$TEMP_ARGV\"\nexec \"$TEMP_CURL\" \"$@\"\n" + if err := os.WriteFile(filepath.Join(bin, "curl"), []byte(stub), 0700); err != nil { + t.Fatal(err) + } + requests := make(chan string, 4) + const body = "synthetic-private-response-not-a-cleanup-command" + server := httptest.NewUnstartedServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + requests <- r.Method + " " + r.URL.Path + code := 200 + switch scenario { + case "401": + code = 401 + case "403": + code = 403 + case "redirect": + w.Header().Set("Location", "/must-not-follow") + code = 307 + case "wrong success status": + code = 206 + case "truncated response": + w.Header().Set("Content-Length", "1024") + } + w.WriteHeader(code) + fmt.Fprint(w, body) + })) + server.Config.ErrorLog = log.New(io.Discard, "", 0) + server.StartTLS() + t.Cleanup(server.Close) + recipe := strings.Replace(original, "https://raw.githubusercontent.com", server.URL, 1) + if scenario != "untrusted HTTPS" { + ca := pem.EncodeToMemory(&pem.Block{Type: "CERTIFICATE", Bytes: server.Certificate().Raw}) + if err := os.WriteFile(filepath.Join(home, "ca.pem"), ca, 0600); err != nil { + t.Fatal(err) + } + recipe = strings.Replace(recipe, "curl --disable ", `curl --disable --cacert "$HOME/ca.pem" `, 1) + } + output, exit := runPBSRecipe(t, recipe, home, bin, "TEMP_ARGV="+argv, "TEMP_CURL="+realCurl) + wantExit := 1 + switch scenario { + case "success": + wantExit = 0 + case "401", "403": + wantExit = 22 + case "untrusted HTTPS": + wantExit = 60 + case "truncated response": + wantExit = 18 + case "mkdir failure", "chmod failure": + wantExit = 42 + } + if exit != wantExit || strings.Contains(output, body) { + t.Fatalf("download exit %d, want %d, or response was disclosed: %s", exit, wantExit, output) + } + if _, err := os.Stat(trace); !os.IsNotExist(err) { + t.Fatal("ambient curl configuration enabled tracing") + } + content, err := os.ReadFile(target) + if err != nil || string(content) != preserved { + t.Fatal("preparation overwrote a link target") + } + args, argErr := os.ReadFile(argv) + localFailure := strings.Contains(scenario, "helper") || strings.Contains(scenario, "symlink") || strings.HasSuffix(scenario, "failure") + if localFailure { + if len(requests) != 0 || !os.IsNotExist(argErr) { + t.Fatal("unsafe preparation reached the downloader") + } + if scenario == "existing helper" { + content, err := os.ReadFile(helper) + if err != nil || string(content) != preserved { + t.Fatal("refused download overwrote an existing helper") + } + } + return + } + if argErr != nil || !strings.HasPrefix(string(args), "--disable\x00") { + t.Fatal("curl did not ignore configuration before other options") + } + if scenario == "untrusted HTTPS" { + if len(requests) != 0 { + t.Fatal("untrusted peer received a request") + } + } else if len(requests) != 1 || <-requests != "GET /rcourtman/Pulse/main/scripts/uninstall-sensor-proxy.sh" { + t.Fatal("download was missing, redirected, retried or used the wrong path") + } + content, err = os.ReadFile(helper) + if scenario == "success" { + info, statErr := os.Stat(helper) + if err != nil || string(content) != body || statErr != nil || info.Mode().Perm() != 0600 { + t.Fatal("complete helper was not promoted privately") + } + } else if !os.IsNotExist(err) { + t.Fatal("failed, redirected or partial download occupied the runnable helper path") + } + }) + } +}