diff --git a/.github/workflows/create-release.yml b/.github/workflows/create-release.yml index 1a315d4e9e..e3e227c4c0 100644 --- a/.github/workflows/create-release.yml +++ b/.github/workflows/create-release.yml @@ -2163,7 +2163,7 @@ jobs: dispatch="$( jq -n \ --arg tag "${TAG}" \ - '{ref: $tag, return_run_details: true, inputs: {tag: $tag}}' | \ + '{ref: "main", return_run_details: true, inputs: {tag: $tag}}' | \ gh api \ --method POST \ -H "Accept: application/vnd.github+json" \ diff --git a/.github/workflows/qualify-secure-runtime-release.yml b/.github/workflows/qualify-secure-runtime-release.yml index 92f9e3f5ad..d0fcc1a2dd 100644 --- a/.github/workflows/qualify-secure-runtime-release.yml +++ b/.github/workflows/qualify-secure-runtime-release.yml @@ -59,6 +59,20 @@ jobs: fetch-depth: 0 ref: ${{ steps.release.outputs.tag }} + - name: Bind canonical Pulse origin + run: | + set -euo pipefail + test "${GITHUB_REPOSITORY}" = "rcourtman/Pulse" + # checkout v7 records the canonical HTTPS URL without .git. The + # released attestation expects the equivalent .git spelling and + # independently checks the remote tag and branch identities. + origin_url="$(git remote get-url origin)" + case "${origin_url}" in + https://github.com/rcourtman/Pulse|https://github.com/rcourtman/Pulse.git) ;; + *) echo "::error::Release checkout origin is not canonical Pulse."; exit 1 ;; + esac + git remote set-url origin https://github.com/rcourtman/Pulse.git + - name: Verify detached release source id: source env: @@ -77,8 +91,11 @@ jobs: git fetch --no-tags origin \ "refs/heads/${required_branch}:refs/remotes/origin/${required_branch}" commit="$(git rev-parse "${TAG}^{commit}")" - test "${GITHUB_REF}" = "refs/tags/${TAG}" - test "${GITHUB_SHA}" = "${commit}" + # Run reviewed qualification control from protected main, while the + # checkout and the packet remain bound to the immutable release tag. + test "${GITHUB_REF}" = "refs/heads/main" + test "${GITHUB_WORKFLOW_REF}" = "${GITHUB_REPOSITORY}/.github/workflows/qualify-secure-runtime-release.yml@refs/heads/main" + test "${GITHUB_WORKFLOW_SHA}" = "${GITHUB_SHA}" test "$(git branch --show-current)" = "" test -z "$(git status --short --untracked-files=all)" git merge-base --is-ancestor "${commit}" "origin/${required_branch}" diff --git a/docs/release-control/v6/internal/subsystems/deployment-installability.md b/docs/release-control/v6/internal/subsystems/deployment-installability.md index 86624b192d..71d1d4f4c9 100644 --- a/docs/release-control/v6/internal/subsystems/deployment-installability.md +++ b/docs/release-control/v6/internal/subsystems/deployment-installability.md @@ -1219,7 +1219,16 @@ artifact-selection behaviour. Private signing material and publication credentials must never enter the compilation job. Post-publication secure-runtime qualification must authenticate before it - executes. `.github/workflows/qualify-secure-runtime-release.yml` may download + executes. The publication workflow dispatches the reviewed qualification + control from protected `main`, with the immutable RC tag as its sole source + input. The qualification workflow must prove its own repository, workflow + ref and workflow SHA are that `main` control, then check out the detached + release tag and verify its commit is on the governed release line. Its + checkout may report either canonical HTTPS origin spelling, with or without + `.git`; it must reject every other origin before normalising to the exact + `.git` spelling required by the packet attester. Normalisation must not + relax the separate remote tag, branch, release and artifact identity checks. + `.github/workflows/qualify-secure-runtime-release.yml` may download caller-owned release assets only into a non-executable holding directory. `scripts/release_control/secure_runtime_attestation_v7.py` must copy the six binaries, four collector signatures, checksum manifest, assembly and diff --git a/internal/hostagent/agent_sensors_test.go b/internal/hostagent/agent_sensors_test.go index 8f3fa0fc1d..a4b9919cee 100644 --- a/internal/hostagent/agent_sensors_test.go +++ b/internal/hostagent/agent_sensors_test.go @@ -57,6 +57,21 @@ func TestAgent_collectTemperatures_MapsKeys(t *testing.T) { } } +func TestAgent_collectTemperatures_ArmadaThermalFeedsMachineCPU(t *testing.T) { + mc := &mockCollector{ + goos: "linux", + sensorsLocalFn: func(context.Context) (string, error) { + return `{"armada_thermal-virtual-0":{"temp1":{"temp1_input":57}}}`, nil + }, + sensorsParseFn: sensors.Parse, + } + a := &Agent{logger: zerolog.Nop(), collector: mc} + got := a.collectTemperatures(context.Background()) + if got.TemperatureCelsius["cpu_package"] != 57 { + t.Fatalf("CPU package reading = %v, want 57", got.TemperatureCelsius) + } +} + func TestAgent_collectTemperatures_MergesNVIDIASMITemperatures(t *testing.T) { mc := &mockCollector{ goos: "linux", diff --git a/internal/models/issue2292_backup_age_test.go b/internal/models/issue2292_backup_age_test.go index ad4993e674..df3409c794 100644 --- a/internal/models/issue2292_backup_age_test.go +++ b/internal/models/issue2292_backup_age_test.go @@ -216,3 +216,94 @@ func TestSyncGuestBackupTimesNewestPBSBackupSameNodeCollision(t *testing.T) { }) } } + +// The reported notes template changed from "pulse" to "pdm21.21008.pulse". +// Root has no namespace placement, so a colliding CT ID needs the complete +// node/VMID/name comment to identify which PVE connection owns the newer row. +func TestSyncGuestBackupTimesNodeVMIDNameCommentForUniqueCT(t *testing.T) { + now := time.Now() + recent := now.Add(-10 * time.Hour) + state := NewState() + state.UpdateContainers([]Container{ + {VMID: 21008, Name: "pulse", Instance: "cluster-a", Node: "pdm21"}, + }) + state.mu.Lock() + state.PBSBackups = []PBSBackup{ + {ID: "old", VMID: "21008", BackupType: "ct", BackupTime: now.Add(-29 * 24 * time.Hour), + Instance: "pbs-main", Comment: "pulse"}, + {ID: "recent", VMID: "21008", BackupType: "ct", BackupTime: recent, + Instance: "pbs-main", Comment: "pdm21.21008.pulse"}, + } + state.mu.Unlock() + + state.SyncGuestBackupTimes() + if got := state.GetSnapshot().Containers[0].LastBackup; !got.Equal(recent) { + t.Errorf("unique CT LastBackup = %v, want newer Root snapshot %v", got, recent) + } +} + +func TestSyncGuestBackupTimesNodeVMIDNameCommentForCollidingCT(t *testing.T) { + now := time.Now() + old := now.Add(-29 * 24 * time.Hour) + recent := now.Add(-10 * time.Hour) + other := now.Add(-2 * time.Hour) + + state := NewState() + state.UpdateContainers([]Container{ + {VMID: 21008, Name: "pulse", Instance: "cluster-a", Node: "pdm21"}, + {VMID: 21008, Name: "other", Instance: "cluster-b", Node: "pdm22"}, + }) + state.mu.Lock() + state.PBSBackups = []PBSBackup{ + {ID: "old", VMID: "21008", BackupType: "ct", BackupTime: old, + Instance: "pbs-main", Comment: "pulse"}, + {ID: "recent", VMID: "21008", BackupType: "ct", BackupTime: recent, + Instance: "pbs-main", Comment: "pdm21.21008.pulse"}, + // An unmarked newer root snapshot is still unsafe to assign to either CT. + {ID: "unknown", VMID: "21008", BackupType: "ct", BackupTime: other, + Instance: "pbs-unattributed"}, + {ID: "running", VMID: "21008", BackupType: "ct", BackupTime: now, + Instance: "pbs-main", Comment: "pdm21.21008.pulse", InProgress: true}, + } + state.mu.Unlock() + + state.SyncGuestBackupTimes() + for _, guest := range state.GetSnapshot().Containers { + switch guest.Instance { + case "cluster-a": + if !guest.LastBackup.Equal(recent) || !guest.BackupInProgress { + t.Errorf("cluster-a LastBackup/running = %v/%v, want %v/true", guest.LastBackup, guest.BackupInProgress, recent) + } + case "cluster-b": + if !guest.LastBackup.IsZero() || guest.BackupInProgress { + t.Errorf("cluster-b inherited another CT's backup: %v/%v", guest.LastBackup, guest.BackupInProgress) + } + } + } +} + +// A matching node/VMID/name comment is still insufficient when two distinct +// connections host an indistinguishable CT. The #1639 collision guard wins. +func TestSyncGuestBackupTimesNodeVMIDNameCommentTieStaysUnattributed(t *testing.T) { + now := time.Now() + state := NewState() + state.UpdateContainers([]Container{ + {VMID: 21008, Name: "pulse", Instance: "cluster-a", Node: "pdm21"}, + {VMID: 21008, Name: "pulse", Instance: "cluster-b", Node: "pdm21"}, + }) + state.mu.Lock() + state.PBSBackups = []PBSBackup{ + {ID: "tied", VMID: "21008", BackupType: "ct", BackupTime: now.Add(-10 * time.Hour), + Instance: "pbs-main", Comment: "pdm21.21008.pulse"}, + {ID: "running-tied", VMID: "21008", BackupType: "ct", BackupTime: now, + Instance: "pbs-main", Comment: "pdm21.21008.pulse", InProgress: true}, + } + state.mu.Unlock() + + state.SyncGuestBackupTimes() + for _, guest := range state.GetSnapshot().Containers { + if !guest.LastBackup.IsZero() || guest.BackupInProgress { + t.Errorf("%s inherited tied CT backup: %v/%v", guest.Instance, guest.LastBackup, guest.BackupInProgress) + } + } +} diff --git a/internal/proxmoxidentity/backup_identity.go b/internal/proxmoxidentity/backup_identity.go index 113a11d1cc..68b38c5ada 100644 --- a/internal/proxmoxidentity/backup_identity.go +++ b/internal/proxmoxidentity/backup_identity.go @@ -81,9 +81,24 @@ func BackupCommentMatchesGuestName(comment, vmid, guestName string) bool { return subjectName != "" && guestName != "" && subjectName == guestName } +// backupCommentMatchesNodeVMIDName recognises a PVE notes template such as +// "pdm21.21008.pulse" only when all three fields match this guest. The node +// must match exactly: suffix or name-only matching could attribute a Root PBS +// snapshot to the wrong PVE connection when CT IDs collide (#2292, #1639). +func backupCommentMatchesNodeVMIDName(comment, vmid, guestName, nodeName string) bool { + vmid = strings.TrimSpace(vmid) + guestName = strings.TrimSpace(guestName) + nodeName = strings.TrimSpace(nodeName) + if vmid == "" || guestName == "" || nodeName == "" { + return false + } + return strings.EqualFold(strings.TrimSpace(comment), nodeName+"."+vmid+"."+guestName) +} + func BackupGuestMatchScore(namespace, comment, vmid, guestName, instanceName, nodeName string) int { score := NamespaceLocationScore(namespace, instanceName, nodeName) * 10 - if BackupCommentMatchesGuestName(comment, vmid, guestName) { + if BackupCommentMatchesGuestName(comment, vmid, guestName) || + backupCommentMatchesNodeVMIDName(comment, vmid, guestName, nodeName) { score += 5 } return score diff --git a/internal/proxmoxidentity/backup_identity_test.go b/internal/proxmoxidentity/backup_identity_test.go index 8311e0404c..d09185dc41 100644 --- a/internal/proxmoxidentity/backup_identity_test.go +++ b/internal/proxmoxidentity/backup_identity_test.go @@ -70,6 +70,30 @@ func TestBackupGuestMatchScoreRanksNodeMatchAboveClusterEntrypoint(t *testing.T) } } +func TestBackupGuestMatchScoreNodeVMIDNameComment(t *testing.T) { + const comment = "pdm21.21008.pulse" + if got := BackupGuestMatchScore("", comment, "21008", "pulse", "cluster-a", "pdm21"); got <= 0 { + t.Fatalf("matching node.VMID.name comment score = %d, want positive", got) + } + for _, tc := range []struct { + name, vmid, guestName, node string + }{ + {"other node", "21008", "pulse", "pdm22"}, + {"other VMID", "21009", "pulse", "pdm21"}, + {"other name", "21008", "other", "pdm21"}, + {"empty node", "21008", "pulse", ""}, + } { + t.Run(tc.name, func(t *testing.T) { + if got := BackupGuestMatchScore("", comment, tc.vmid, tc.guestName, "cluster-a", tc.node); got != 0 { + t.Fatalf("mismatched node.VMID.name comment score = %d, want zero", got) + } + }) + } + if got := BackupGuestMatchScore("", "other.pdm21.21008.pulse", "21008", "pulse", "cluster-a", "pdm21"); got != 0 { + t.Fatalf("prefixed comment score = %d, want zero", got) + } +} + func TestLocationLabelsEqualRequiresExactNormalizedMatch(t *testing.T) { if !LocationLabelsEqual("pve-nat", "PVE Nat") { t.Fatal("normalized identical labels should match") diff --git a/internal/sensors/collector.go b/internal/sensors/collector.go index 3471b85fb0..822c8ad4a4 100644 --- a/internal/sensors/collector.go +++ b/internal/sensors/collector.go @@ -2,10 +2,13 @@ package sensors import ( "context" + "encoding/json" "errors" "fmt" "io" + "os" "os/exec" + "path/filepath" "strconv" "strings" "time" @@ -20,77 +23,166 @@ const ( var ( errCommandOutputTooLarge = errors.New("command output exceeds size limit") - rpiThermalZonePath = "/sys/class/thermal/thermal_zone0/temp" + thermalZoneRoot = "/sys/class/thermal" + hwmonRoot = "/sys/class/hwmon" ) -var rpiThermalZoneTempPath = "/sys/class/thermal/thermal_zone0/temp" - -// CollectLocal reads sensor data from the local machine using lm-sensors. -// Returns the raw JSON output from `sensors -j` or an error if sensors is not available. +// CollectLocal reads lm-sensors JSON and supplements it with a recognised CPU +// thermal sysfs source when it contains no usable CPU temperature. Other +// lm-sensors readings are retained; sysfs is also used when lm-sensors is +// unavailable or empty. func CollectLocal(ctx context.Context) (string, error) { ctx = normalizeCollectionContext(ctx) - // Check if sensors command exists sensorsPath, err := exec.LookPath("sensors") - if err != nil { - return "", fmt.Errorf("lm-sensors not installed: %w", err) - } + if err == nil { + cmdCtx, cancel := context.WithTimeout(ctx, 5*time.Second) + defer cancel() - // Create context with timeout - cmdCtx, cancel := context.WithTimeout(ctx, 5*time.Second) - defer cancel() - - // Run sensors -j command with bounded output capture. - // sensors exits non-zero when optional subfeatures fail, so non-empty output is still accepted. - cmd := exec.CommandContext(cmdCtx, sensorsPath, "-j") - cmd.Stderr = io.Discard - output, err := runCommandOutputLimited(cmd, maxSensorsOutputSizeBytes) - if err != nil && errors.Is(err, errCommandOutputTooLarge) { - return "", fmt.Errorf("failed to execute sensors: %w", err) - } - - outputStr := strings.TrimSpace(string(output)) - if err != nil && outputStr == "" { - return "", fmt.Errorf("failed to execute sensors: %w", err) - } - - if outputStr == "" || outputStr == "{}" { - log.Debug(). - Str("component", "sensors_collector"). - Str("action", "collect_local_empty_output"). - Msg("lm-sensors returned empty output, attempting Raspberry Pi thermal fallback") - - // Try Raspberry Pi temperature method as fallback - cmd = exec.CommandContext(cmdCtx, "cat", rpiThermalZonePath) - rpiOutput, rpiErr := cmd.Output() - if rpiErr == nil { - rpiTemp := strings.TrimSpace(string(rpiOutput)) - if rpiTemp != "" { - parsed, parseErr := strconv.ParseFloat(rpiTemp, 64) - if parseErr != nil { - return "", fmt.Errorf("invalid thermal value %q: %w", rpiTemp, parseErr) - } - // Linux thermal_zone values are commonly millidegrees (e.g. 42000). - // Convert only when magnitude indicates millidegrees to keep degree inputs intact. - if parsed >= 1000 || parsed <= -1000 { - parsed = parsed / 1000.0 - } - rpiTemp = strconv.FormatFloat(parsed, 'f', 3, 64) - // Convert to pseudo-sensors format for compatibility - return fmt.Sprintf(`{"cpu_thermal-virtual-0":{"temp1":{"temp1_input":%s}}}`, rpiTemp), nil - } - } else { - log.Debug(). - Str("component", "sensors_collector"). - Str("action", "collect_local_rpi_fallback_failed"). - Str("thermal_path", "/sys/class/thermal/thermal_zone0/temp"). - Err(rpiErr). - Msg("Raspberry Pi thermal fallback failed") + // sensors can exit non-zero when optional subfeatures fail, so accept + // non-empty output even then. Never fall back after an output-limit hit. + cmd := exec.CommandContext(cmdCtx, sensorsPath, "-j") + cmd.Stderr = io.Discard + output, commandErr := runCommandOutputLimited(cmd, maxSensorsOutputSizeBytes) + if errors.Is(commandErr, errCommandOutputTooLarge) { + return "", fmt.Errorf("failed to execute sensors: %w", commandErr) } - return "", fmt.Errorf("sensors returned empty output") + outputStr := strings.TrimSpace(string(output)) + if outputStr != "" && outputStr != "{}" { + return addMissingSysfsCPU(ctx, outputStr), nil + } + if cmdCtx.Err() != nil { + return "", fmt.Errorf("failed to execute sensors: %w", cmdCtx.Err()) + } + log.Debug().Str("component", "sensors_collector"). + Str("action", "collect_local_empty_output"). + Msg("lm-sensors returned no data; trying recognised CPU thermal sysfs sources") } - return outputStr, nil + output, fallbackErr := collectSysfsCPUTemperature(ctx) + if fallbackErr == nil { + return output, nil + } + if err != nil { + return "", fmt.Errorf("lm-sensors unavailable and CPU thermal fallback failed: %w", fallbackErr) + } + return "", fmt.Errorf("sensors returned empty output and CPU thermal fallback failed: %w", fallbackErr) +} + +// addMissingSysfsCPU leaves existing output unchanged unless it is a JSON +// object without a usable CPU reading and an identified sysfs source is +// available. A failed optional lookup must not discard working sensors data. +func addMissingSysfsCPU(ctx context.Context, sensorsJSON string) string { + var chips map[string]json.RawMessage + if err := json.Unmarshal([]byte(sensorsJSON), &chips); err != nil || chips == nil { + return sensorsJSON + } + parsed, err := Parse(sensorsJSON) + if err != nil || parsed.CPUPackage > 0 { + return sensorsJSON + } + fallbackJSON, err := collectSysfsCPUTemperature(ctx) + if err != nil { + return sensorsJSON + } + var fallback map[string]json.RawMessage + if err := json.Unmarshal([]byte(fallbackJSON), &fallback); err != nil { + return sensorsJSON + } + for name, value := range fallback { + if _, exists := chips[name]; exists { + // A real lm-sensors chip can have the same name as the synthetic + // fallback. Keep its other readings rather than replacing the chip. + for suffix := 1; ; suffix++ { + alternate := fmt.Sprintf("%s-sysfs-%d", name, suffix) + if _, exists := chips[alternate]; !exists { + name = alternate + break + } + } + } + chips[name] = value + } + merged, err := json.Marshal(chips) + if err != nil || len(merged) > maxSensorsOutputSizeBytes { + return sensorsJSON + } + return string(merged) +} + +// collectSysfsCPUTemperature deliberately accepts only identified CPU/SoC +// sensors. thermal_zone0 is not necessarily a CPU on every Linux machine. +func collectSysfsCPUTemperature(ctx context.Context) (string, error) { + if err := ctx.Err(); err != nil { + return "", err + } + var lastErr error + for _, source := range []struct { + root, prefix, label, value string + }{ + {thermalZoneRoot, "thermal_zone", "type", "temp"}, + {hwmonRoot, "hwmon", "name", "temp1_input"}, + } { + entries, err := os.ReadDir(source.root) + if err != nil { + lastErr = err + continue + } + for _, entry := range entries { + if err := ctx.Err(); err != nil { + return "", err + } + if !strings.HasPrefix(entry.Name(), source.prefix) { + continue + } + dir := filepath.Join(source.root, entry.Name()) + name, err := readBoundedThermalFile(filepath.Join(dir, source.label)) + if err != nil || !isCPUSysfsThermalName(name) { + continue + } + raw, err := readBoundedThermalFile(filepath.Join(dir, source.value)) + if err != nil { + lastErr = err + continue + } + millidegrees, err := strconv.ParseInt(raw, 10, 64) + if err != nil || millidegrees < 1000 || millidegrees >= 150000 { + lastErr = fmt.Errorf("invalid CPU thermal millidegree value from %s", dir) + continue + } + celsius := float64(millidegrees) / 1000 + return fmt.Sprintf(`{"cpu_thermal-virtual-0":{"temp1":{"temp1_input":%.3f}}}`, celsius), nil + } + } + if lastErr != nil { + return "", fmt.Errorf("no valid CPU thermal sysfs reading: %w", lastErr) + } + return "", errors.New("no recognised CPU thermal sysfs source") +} + +func isCPUSysfsThermalName(name string) bool { + switch strings.ToLower(strings.TrimSpace(name)) { + case "armada_thermal", "cpu_thermal", "cpu-thermal", "soc_thermal", "soc-thermal", "x86_pkg_temp", "rpitemp": + return true + default: + return false + } +} + +func readBoundedThermalFile(path string) (string, error) { + f, err := os.Open(path) + if err != nil { + return "", err + } + defer f.Close() + contents, err := io.ReadAll(io.LimitReader(f, maxThermalFileReadBytes+1)) + if err != nil { + return "", err + } + if len(contents) > maxThermalFileReadBytes { + return "", fmt.Errorf("thermal sysfs value exceeds %d bytes", maxThermalFileReadBytes) + } + return strings.TrimSpace(string(contents)), nil } func runCommandOutputLimited(cmd *exec.Cmd, maxBytes int) ([]byte, error) { diff --git a/internal/sensors/collector_additional_test.go b/internal/sensors/collector_additional_test.go index d77ec07cbd..979fe22769 100644 --- a/internal/sensors/collector_additional_test.go +++ b/internal/sensors/collector_additional_test.go @@ -2,7 +2,9 @@ package sensors import ( "context" + "encoding/json" "os" + "path/filepath" "strings" "testing" ) @@ -12,6 +14,7 @@ func TestCollectLocal_EmptyFallbackReturnsError(t *testing.T) { writeScript(t, dir, "sensors", "#!/bin/sh\necho '{}'\n") writeScript(t, dir, "cat", "#!/bin/sh\necho ''\n") t.Setenv("PATH", dir+":"+os.Getenv("PATH")) + useTestSysfsRoots(t) _, err := CollectLocal(context.Background()) if err == nil { @@ -22,6 +25,176 @@ func TestCollectLocal_EmptyFallbackReturnsError(t *testing.T) { } } +func TestCollectLocal_MissingSensorsUsesIdentifiedArmadaThermalZone(t *testing.T) { + t.Setenv("PATH", t.TempDir()) + thermal, _ := useTestSysfsRoots(t) + writeSysfsSensor(t, thermal, "thermal_zone0", "type", "gpu-thermal", "70000") + writeSysfsSensor(t, thermal, "thermal_zone1", "type", "armada_thermal", "57000") + + out, err := CollectLocal(context.Background()) + if err != nil { + t.Fatal(err) + } + parsed, err := Parse(out) + if err != nil { + t.Fatal(err) + } + if !parsed.Available || parsed.CPUPackage != 57 || parsed.CPUMax != 57 { + t.Fatalf("armada sysfs reading not mapped to CPU: %+v", parsed) + } +} + +func TestCollectLocal_SysfsRejectsUnidentifiedAndBadReadings(t *testing.T) { + for _, tc := range []struct { + name, chip, value string + }{ + {"unidentified", "gpu-thermal", "57000"}, + {"empty", "armada_thermal", ""}, + {"degrees-not-millidegrees", "armada_thermal", "57"}, + {"negative", "armada_thermal", "-57000"}, + {"too-hot", "armada_thermal", "150000"}, + {"malformed", "armada_thermal", "57000C"}, + {"non-finite", "armada_thermal", "NaN"}, + {"oversized", "armada_thermal", strings.Repeat("5", maxThermalFileReadBytes+1)}, + } { + t.Run(tc.name, func(t *testing.T) { + t.Setenv("PATH", t.TempDir()) + thermal, _ := useTestSysfsRoots(t) + writeSysfsSensor(t, thermal, "thermal_zone0", "type", tc.chip, tc.value) + if out, err := CollectLocal(context.Background()); err == nil { + t.Fatalf("unexpected CPU temperature %s for %s", out, tc.name) + } + }) + } +} + +func TestCollectLocal_SysfsContinuesPastBadSource(t *testing.T) { + t.Setenv("PATH", t.TempDir()) + thermal, hwmon := useTestSysfsRoots(t) + writeSysfsSensor(t, thermal, "thermal_zone0", "type", "armada_thermal", "invalid") + writeSysfsSensor(t, hwmon, "hwmon3", "name", "armada_thermal", "57500") + + out, err := CollectLocal(context.Background()) + if err != nil { + t.Fatal(err) + } + if !strings.Contains(out, `57.500`) { + t.Fatalf("expected hwmon fallback, got %s", out) + } +} + +func TestCollectLocal_SensorsJSONGainsMissingSysfsCPU(t *testing.T) { + dir := t.TempDir() + writeScript(t, dir, "sensors", "#!/bin/sh\necho '{\"nvme-pci-0100\":{\"Composite\":{\"temp1_input\":39}}}'\n") + t.Setenv("PATH", dir+":"+os.Getenv("PATH")) + thermal, _ := useTestSysfsRoots(t) + writeSysfsSensor(t, thermal, "thermal_zone0", "type", "armada_thermal", "57000") + out, err := CollectLocal(context.Background()) + if err != nil { + t.Fatal(err) + } + parsed, err := Parse(out) + if err != nil { + t.Fatal(err) + } + if parsed.CPUPackage != 57 || parsed.NVMe["nvme0"] != 39 { + t.Fatalf("lm-sensors data and sysfs CPU were not combined: %+v", parsed) + } +} + +func TestCollectLocal_CPUChipWithoutTemperatureGainsSysfsCPU(t *testing.T) { + dir := t.TempDir() + const original = `{"armada_thermal-virtual-0":{"temp1":{"temp1_input":"bad"}},"nvme-pci-0100":{"Composite":{"temp1_input":39}}}` + writeScript(t, dir, "sensors", "#!/bin/sh\necho '"+original+"'\n") + t.Setenv("PATH", dir+":"+os.Getenv("PATH")) + thermal, _ := useTestSysfsRoots(t) + writeSysfsSensor(t, thermal, "thermal_zone0", "type", "armada_thermal", "57000") + + out, err := CollectLocal(context.Background()) + if err != nil { + t.Fatal(err) + } + parsed, err := Parse(out) + if err != nil { + t.Fatal(err) + } + if parsed.CPUPackage != 57 || parsed.NVMe["nvme0"] != 39 { + t.Fatalf("unusable CPU chip blocked sysfs or lost lm-sensors data: %+v", parsed) + } +} + +func TestCollectLocal_SysfsCPUDoesNotReplaceMatchingSensorChip(t *testing.T) { + dir := t.TempDir() + const original = `{"cpu_thermal-virtual-0":{"temp1":{"temp1_input":"bad"},"fan1":{"fan1_input":1200}}}` + writeScript(t, dir, "sensors", "#!/bin/sh\necho '"+original+"'\n") + t.Setenv("PATH", dir+":"+os.Getenv("PATH")) + thermal, _ := useTestSysfsRoots(t) + writeSysfsSensor(t, thermal, "thermal_zone0", "type", "armada_thermal", "57000") + + out, err := CollectLocal(context.Background()) + if err != nil { + t.Fatal(err) + } + parsed, err := Parse(out) + if err != nil { + t.Fatal(err) + } + if parsed.CPUPackage != 57 || parsed.Fans["cpu_thermal_fan1"] != 1200 { + t.Fatalf("sysfs CPU replaced existing chip readings: %+v", parsed) + } + var chips map[string]json.RawMessage + if err := json.Unmarshal([]byte(out), &chips); err != nil { + t.Fatal(err) + } + if _, ok := chips["cpu_thermal-virtual-0-sysfs-1"]; !ok { + t.Fatalf("expected a distinct sysfs CPU chip, got %s", out) + } +} + +func TestCollectLocal_ExistingCPUPrecedesSysfs(t *testing.T) { + dir := t.TempDir() + const original = `{"coretemp-isa-0000":{"Package id 0":{"temp1_input":42}}}` + writeScript(t, dir, "sensors", "#!/bin/sh\necho '"+original+"'\n") + t.Setenv("PATH", dir+":"+os.Getenv("PATH")) + thermal, _ := useTestSysfsRoots(t) + writeSysfsSensor(t, thermal, "thermal_zone0", "type", "armada_thermal", "57000") + out, err := CollectLocal(context.Background()) + if err != nil { + t.Fatal(err) + } + if out != original { + t.Fatalf("existing CPU reading was changed: %s", out) + } +} + +func TestCollectLocal_InvalidSysfsDoesNotDiscardOtherSensors(t *testing.T) { + dir := t.TempDir() + const original = `{"nvme-pci-0100":{"Composite":{"temp1_input":39}}}` + writeScript(t, dir, "sensors", "#!/bin/sh\necho '"+original+"'\n") + t.Setenv("PATH", dir+":"+os.Getenv("PATH")) + thermal, _ := useTestSysfsRoots(t) + writeSysfsSensor(t, thermal, "thermal_zone0", "type", "armada_thermal", "bad") + out, err := CollectLocal(context.Background()) + if err != nil { + t.Fatal(err) + } + if out != original { + t.Fatalf("valid lm-sensors data was discarded: %s", out) + } +} + +func TestCollectLocal_SysfsNameReadBounded(t *testing.T) { + t.Setenv("PATH", t.TempDir()) + thermal, _ := useTestSysfsRoots(t) + writeSysfsSensor(t, thermal, "thermal_zone0", "type", "armada_thermal", "57000") + if err := os.WriteFile(filepath.Join(thermal, "thermal_zone0", "type"), []byte(strings.Repeat("a", maxThermalFileReadBytes+1)), 0600); err != nil { + t.Fatal(err) + } + if out, err := CollectLocal(context.Background()); err == nil { + t.Fatalf("oversized type unexpectedly accepted: %s", out) + } +} + func TestCollectLocal_ContextCanceled(t *testing.T) { dir := t.TempDir() writeScript(t, dir, "sensors", "#!/bin/sh\necho '{\"chip\":{\"temp\":{\"temp1_input\":42}}}'\n") diff --git a/internal/sensors/collector_test.go b/internal/sensors/collector_test.go index 7dc6b157eb..f332ff88a9 100644 --- a/internal/sensors/collector_test.go +++ b/internal/sensors/collector_test.go @@ -16,9 +16,42 @@ func writeScript(t *testing.T, dir, name, content string) { } } +func useTestSysfsRoots(t *testing.T) (string, string) { + t.Helper() + root := t.TempDir() + thermal := filepath.Join(root, "thermal") + hwmon := filepath.Join(root, "hwmon") + for _, dir := range []string{thermal, hwmon} { + if err := os.Mkdir(dir, 0700); err != nil { + t.Fatal(err) + } + } + previousThermal, previousHwmon := thermalZoneRoot, hwmonRoot + thermalZoneRoot, hwmonRoot = thermal, hwmon + t.Cleanup(func() { thermalZoneRoot, hwmonRoot = previousThermal, previousHwmon }) + return thermal, hwmon +} + +func writeSysfsSensor(t *testing.T, root, dir, label, name, value string) { + t.Helper() + path := filepath.Join(root, dir) + if err := os.Mkdir(path, 0700); err != nil { + t.Fatal(err) + } + for file, content := range map[string]string{label: name, "temp": value} { + if label == "name" && file == "temp" { + file = "temp1_input" + } + if err := os.WriteFile(filepath.Join(path, file), []byte(content), 0600); err != nil { + t.Fatal(err) + } + } +} + func TestCollectLocalMissingSensors(t *testing.T) { dir := t.TempDir() t.Setenv("PATH", dir) + useTestSysfsRoots(t) if _, err := CollectLocal(context.Background()); err == nil { t.Fatal("expected error when sensors missing") @@ -57,15 +90,8 @@ func TestCollectLocalFallbackToPiTemp(t *testing.T) { dir := t.TempDir() writeScript(t, dir, "sensors", "#!/bin/sh\necho '{}'\n") t.Setenv("PATH", dir+":"+os.Getenv("PATH")) - thermalPath := filepath.Join(dir, "thermal_zone0_temp") - if err := os.WriteFile(thermalPath, []byte("42000\n"), 0600); err != nil { - t.Fatalf("write thermal file: %v", err) - } - originalThermalPath := rpiThermalZonePath - rpiThermalZonePath = thermalPath - t.Cleanup(func() { - rpiThermalZonePath = originalThermalPath - }) + thermal, _ := useTestSysfsRoots(t) + writeSysfsSensor(t, thermal, "thermal_zone0", "type", "cpu-thermal\n", "42000\n") out, err := CollectLocal(context.Background()) if err != nil { @@ -77,21 +103,12 @@ func TestCollectLocalFallbackToPiTemp(t *testing.T) { } } -func TestCollectLocalFallbackKeepsDegreeInput(t *testing.T) { +func TestCollectLocalFallbackToArmadaHwmon(t *testing.T) { dir := t.TempDir() writeScript(t, dir, "sensors", "#!/bin/sh\necho '{}'\n") - writeScript(t, dir, "cat", "#!/bin/sh\necho '42'\n") t.Setenv("PATH", dir+":"+os.Getenv("PATH")) - - thermalPath := filepath.Join(t.TempDir(), "thermal_zone0_temp") - if err := os.WriteFile(thermalPath, []byte("42000"), 0644); err != nil { - t.Fatalf("write thermal file: %v", err) - } - originalThermalPath := rpiThermalZoneTempPath - rpiThermalZoneTempPath = thermalPath - t.Cleanup(func() { - rpiThermalZoneTempPath = originalThermalPath - }) + _, hwmon := useTestSysfsRoots(t) + writeSysfsSensor(t, hwmon, "hwmon0", "name", "armada_thermal\n", "42000\n") out, err := CollectLocal(context.Background()) if err != nil { @@ -136,15 +153,8 @@ func TestCollectLocalFallbackRejectsInvalidThermalValue(t *testing.T) { dir := t.TempDir() writeScript(t, dir, "sensors", "#!/bin/sh\necho '{}'\n") t.Setenv("PATH", dir+":"+os.Getenv("PATH")) - thermalPath := filepath.Join(dir, "thermal_zone0_temp") - if err := os.WriteFile(thermalPath, []byte(`42"},"bad":{"temp1_input":1}`), 0600); err != nil { - t.Fatalf("write thermal file: %v", err) - } - originalThermalPath := rpiThermalZonePath - rpiThermalZonePath = thermalPath - t.Cleanup(func() { - rpiThermalZonePath = originalThermalPath - }) + thermal, _ := useTestSysfsRoots(t) + writeSysfsSensor(t, thermal, "thermal_zone0", "type", "cpu-thermal", `42"},"bad":{"temp1_input":1}`) if _, err := CollectLocal(context.Background()); err == nil { t.Fatal("expected error for invalid fallback thermal value") diff --git a/internal/sensors/parser.go b/internal/sensors/parser.go index 0acfa12dbc..d5f521cb5f 100644 --- a/internal/sensors/parser.go +++ b/internal/sensors/parser.go @@ -126,7 +126,7 @@ func isCPUChip(chipLower string) bool { "it87", "nct6687", "nct6775", "nct6776", "nct6779", "nct6791", "nct6792", "nct6793", "nct6795", "nct6796", "nct6797", "nct6798", "w83627", "f71882", - "cpu_thermal", "rp1_adc", "rpitemp", + "cpu_thermal", "rp1_adc", "rpitemp", "armada_thermal", } for _, chip := range cpuChips { @@ -166,7 +166,7 @@ func parseCPUTemps(chipMap map[string]interface{}, data *TemperatureData) { // Capture generic temp1 for chips like cpu_thermal (RPi, ARM SoCs) // that don't have labeled sensors if sensorNameLower == "temp1" { - if tempVal := extractTempInput(sensorMap); !math.IsNaN(tempVal) && tempVal > 0 { + if tempVal := extractTempInput(sensorMap); !math.IsNaN(tempVal) && tempVal > 0 && tempVal < 150 { genericTemp = tempVal } } diff --git a/internal/sensors/parser_test.go b/internal/sensors/parser_test.go index 6803ae9663..481ca485db 100644 --- a/internal/sensors/parser_test.go +++ b/internal/sensors/parser_test.go @@ -360,6 +360,7 @@ func TestIsCPUChip(t *testing.T) { {"cpu_thermal-virtual-0", true}, {"acpitz-acpi-0", true}, {"rp1_adc-isa-0000", true}, // Raspberry Pi RP1 ADC + {"armada_thermal-virtual-0", true}, {"nvme-pci-0100", false}, {"amdgpu-pci-0300", false}, {"unknown-chip", false}, @@ -375,6 +376,27 @@ func TestIsCPUChip(t *testing.T) { } } +func TestParseArmadaThermalCPUTemperatureAndOtherSensors(t *testing.T) { + input := `{"armada_thermal-virtual-0":{"temp1":{"temp1_input":57}},"nvme-pci-0100":{"Composite":{"temp1_input":39}}}` + data, err := Parse(input) + if err != nil { + t.Fatal(err) + } + if !data.Available || data.CPUPackage != 57 || data.CPUMax != 57 || data.NVMe["nvme0"] != 39 { + t.Fatalf("unexpected ARM and NVMe readings: %+v", data) + } + + for _, invalid := range []string{`null`, `"unavailable"`, `150`, `1000000`, `-3`} { + data, err := Parse(`{"armada_thermal-virtual-0":{"temp1":{"temp1_input":` + invalid + `}}}`) + if err != nil { + t.Fatal(err) + } + if data.CPUPackage != 0 { + t.Fatalf("invalid armada reading %s became CPU temperature %v", invalid, data.CPUPackage) + } + } +} + func TestExtractTempInput(t *testing.T) { tests := []struct { name string diff --git a/scripts/installtests/build_release_assets_test.go b/scripts/installtests/build_release_assets_test.go index f262cb0e61..a64578e473 100644 --- a/scripts/installtests/build_release_assets_test.go +++ b/scripts/installtests/build_release_assets_test.go @@ -2792,7 +2792,7 @@ func TestSecureRuntimeQualificationPacketIsHostedAndReleaseBound(t *testing.T) { "release/pulse-secure-runtime-collector-v1-linux-amd64", "release/pulse-secure-runtime-collector-v3-linux-amd64", "qualify-secure-runtime-release.yml/dispatches", - `{ref: $tag, return_run_details: true, inputs: {tag: $tag}}`, + `{ref: "main", return_run_details: true, inputs: {tag: $tag}}`, "Secure-runtime qualification dispatch did not return an exact workflow run.", "Immutable RC publication did not retain an exact secure-runtime qualification run identity.", } { @@ -2805,9 +2805,11 @@ func TestSecureRuntimeQualificationPacketIsHostedAndReleaseBound(t *testing.T) { if strings.Contains(qualificationWorkflow, "release:\n types: [published]") { t.Fatal("secure-runtime qualification must be explicitly dispatched after immutable publication, not rely on suppressed release events") } + originIndex := strings.Index(qualificationWorkflow, "Bind canonical Pulse origin") + sourceIndex := strings.Index(qualificationWorkflow, "Verify detached release source") preauthenticationIndex := strings.Index(qualificationWorkflow, "Pre-authenticate exact qualification packet") privilegedExecutionIndex := strings.Index(qualificationWorkflow, "docker run") - if preauthenticationIndex < 0 || privilegedExecutionIndex < 0 || preauthenticationIndex > privilegedExecutionIndex { + if originIndex < 0 || sourceIndex < originIndex || preauthenticationIndex < sourceIndex || privilegedExecutionIndex < preauthenticationIndex { t.Fatal("secure-runtime release packet must be authenticated before any privileged Docker execution") } if strings.Contains(qualificationWorkflow, `$RUNNER_TEMP/secure-runtime-downloads:/release:ro`) { @@ -2815,8 +2817,11 @@ func TestSecureRuntimeQualificationPacketIsHostedAndReleaseBound(t *testing.T) { } for _, required := range []string{ ".immutable == true", - `test "${GITHUB_REF}" = "refs/tags/${TAG}"`, - `test "${GITHUB_SHA}" = "${commit}"`, + `test "${GITHUB_REF}" = "refs/heads/main"`, + `test "${GITHUB_WORKFLOW_SHA}" = "${GITHUB_SHA}"`, + `test "${GITHUB_REPOSITORY}" = "rcourtman/Pulse"`, + `https://github.com/rcourtman/Pulse|https://github.com/rcourtman/Pulse.git)`, + `git remote set-url origin https://github.com/rcourtman/Pulse.git`, "ca-certificates curl dbus systemd systemd-sysv util-linux", `docker:27.5.1-dind@sha256:f649ef046008ca7f926a2571c32b0ac22e5c59eb61b959617f9acc2a4c638cf5`, `for command in curl docker dockerd id nsenter runuser systemctl`,