The SAML stack this branch builds on appends its boundary notes to
storage-recovery.md at the anchor main used for the linked-agent stale
flag note. Both sets of notes are kept, the SAML ones first.
Contract-Neutral: Merge of origin/main into the SAML stack: main's changes were governed when they landed, and the only resolution keeps both sets of storage-recovery boundary notes
With allowIdpInitiated off, the default, every SAML login failed:
ProcessResponse passed crewjam/saml no request IDs, and crewjam refuses
a Response whose InResponseTo is not among them. Only providers that
allowed IdP-initiated logins, which skips the check, could sign in.
MakeAuthRequest now records each AuthnRequest ID with the login's
returnTo and the hash of a random token that /login sets as the
HttpOnly SameSite=Lax cookie pulse_saml_login (__Host- over HTTPS),
reused across tabs. The IdP posts its Response cross-site, which
withholds Lax cookies, so the ACS answers a cookieless POST with a
page that re-posts the same fields once from Pulse's origin. The
Response must answer an unexpired request bound to the presenting
browser; it spends that request and returns to the recorded path.
Another browser's request is left for its owner.
The request ID is read from the signature covering the assertion
crewjam returns (the signed Response, else that assertion's own
SubjectConfirmationData), not from crewjam's parsed fields and not
from other signed assertions riding in the same unsigned Response.
encoding/xml fills an un-namespaced
attribute field from a namespace declaration with the same local name,
and exclusive canonicalization drops an unused one from the signed
form, so xmlns:InResponseTo could rebind a signed Response and
xmlns:IssueInstant / xmlns:NotOnOrAfter let a stale one pass crewjam's
age checks: an IdP-initiated login could be replayed indefinitely. The
signature verifier now refuses signed content declaring a prefix named
like an attribute crewjam reads, in both modes.
RelayState is now escaped into the redirect query, so a returnTo with
its own query string reaches the IdP whole.
The `/debug/pprof` handlers are gated on `RequireAdmin` plus a
`settings:read` scope that session logins skip. A `DEMO_MODE` instance
that signs visitors in as the configured admin therefore served every
visitor goroutine and heap dumps, the process command line, and CPU
profiles and execution traces that load the whole process. Demo mode
already hides diagnostics, logs, admin users and manual discovery
through the admin-operations policy; pprof was missing from it.
## Change
- `internal/api/demo_mode_operations.go`: through the admin-operations
policy, the demo guard answers `/debug/pprof` and every path below it
with `404` for every method, ahead of route authentication, so a write
probe such as the `POST` symbol lookup gets the same `404` instead of
the generic demo `403`. Instances outside demo mode keep the routes, and
`PULSE_PPROF_DISABLED` still removes them everywhere.
- Contract notes in api-contracts (owner), agent-lifecycle and
storage-recovery.
## Proof
- `TestContract_DemoModeHidesPprofFromDemoAdminSession` signs in as the
demo admin through the full router. The same session gets `200` from
`/debug/pprof/cmdline` below the demo guard, and `404` for eight pprof
paths × `GET`/`HEAD`/`OPTIONS`/`POST` through the served handler.
Against the previous policy (`go test -overlay`) `GET` and `HEAD`
returned pprof output (`200`, or the root's `301` into the index),
`OPTIONS` got the router's preflight `200` and `POST` the generic demo
`403`.
- `TestPublicDemoAdminOperationsPolicyHidesEveryDebugRoute` parses the
router files and fails if a literal route under `/debug/`, with or
without a method prefix, escapes the policy for any of seven methods, so
a new debug handler there cannot ship reachable on a demo by default. A
temporary `GET /debug/vars` registration made it fail as expected.
- Path-handling probe (not committed): 210 raw requests (35 request
targets × six methods), covering trailing and double slashes, dot
segments, `%2F`/`%2e` encodings, absolute-form targets and `CONNECT`,
run against a copy of the router's mux dispatch. None reached a pprof
handler; paths the mux would clean onto pprof only receive a redirect,
and the policy catches the follow-up request.
## Problem
Saving anything on Alerts > Thresholds turned flapping detection and
alert TTL cleanup off. The page's `PUT /api/alerts/config` body never
carries `flappingEnabled`, `maxAlertAgeDays`, `maxAcknowledgedAgeDays`,
`autoAcknowledgeAfterHours` or `customRules` (the page has no control
for any of them). `UpdateAlertConfig` decoded the body into a zero
`AlertConfig`, so every save wrote `false`/`0` for those fields, reset
the flapping tuning to defaults, dropped custom rules, and persisted
that. The `flapping`, `aggregation` and `ioNormalization` objects the
page sends are not `AlertConfig` fields and never enabled anything.
## Fix
- `PUT /api/alerts/config` now replaces only the top-level keys the body
carries. A key left out keeps its stored value. A sent key replaces its
setting whole, so `"overrides": {}` still clears overrides, and the
usual normalization still runs. A body that is not a JSON object
(including `null`) is a 400 and changes nothing.
- `alertconfig.ApplyAlertConfigUpdate` decodes the body on its own, so
sent keys get exact `encoding/json` semantics, including duplicates and
case-folded keys. It copies every unsent top-level field from a JSON
clone of the stored config.
- `Manager.ApplyConfigUpdate` reads, merges and applies under the
manager lock, so concurrent partial saves cannot revert each other. It
returns an owned snapshot, which the handler persists, so
`SaveAlertConfig`'s in-place normalization no longer writes into the
live config's maps. The handler runs one save at a time, so successful
saves reach `alerts.json` in the order they were applied.
- Stored configs are not rewritten; a value an earlier save zeroed stays
as it is.
## Verification
- Unit: `internal/alerts/config/update_test.go`; manager save path and
concurrent partial updates in
`internal/alerts/config_validation_test.go`; handler against a real
manager in `internal/api/alerting/alerts_test.go`. Each new regression
fails against main's code (overlay negative controls). The concurrency
test fails at round 0 against a non-atomic variant.
- `go test ./internal/alerts/... ./internal/api/alerting/` and the
`internal/api` alert endpoint, tenant and scope tests pass; the alert
tests pass under `-race`.
- Private stack (mock mode, Vite on main's frontend):
- On main, a node CPU change saved from the Thresholds page took
flapping and all three TTLs to false/0, in memory and in `alerts.json`.
- With this change, the same UI save kept them (true/7/1/24) and stored
the new threshold.
- The API repro (GET, drop the four keys, PUT) keeps them; explicit
`false`/`0` still applies; `null` and `[]` return 400.
- Five rounds of concurrent partial PUTs kept both changes in memory and
on disk.
## Follow-ups
- The thresholds page still sends the dead
`flapping`/`aggregation`/`ioNormalization` objects and hard-coded
`minimumDelta`/`suppressionWindow`/`hysteresisMargin`. A frontend
cleanup will follow once the open PRs touching
`alertsConfigurationModel.ts` land.
- `SaveAlertConfig` still normalizes caller-owned maps in place for
other callers (`ActivateAlerts`, startup paths), and the GET handler
encodes a shallow copy. That pre-existing race is tracked separately.
The session router dispatches `/api/ai/sessions/{id}/abort`,
`summarize`, `fork`, `undo` and `redo` by path alone, and none of those
handlers checked the HTTP method, so a GET or HEAD cancelled a running
turn, compacted or rewound a transcript, or persisted a fork.
- **Demo mode:** `DemoModeMiddleware` admits every GET/HEAD as a read,
so a public-demo session could mutate Assistant sessions despite the
read-only guard.
- **CSRF:** `CheckCSRF` skips GET/HEAD, and session cookies default to
SameSite=Lax, which browsers send on cross-site top-level GET
navigations. A link carrying a session ID could therefore trigger these
mutations for a signed-in user.
## Change
- `requireAssistantSessionMutationMethod` in
`internal/api/ai_handler.go`: abort, summarize, fork, undo, redo and
steer admit only POST and answer anything else with `405` + `Allow:
POST` before resolving the Assistant service. Steer already refused
non-POST but sent no `Allow` header; it now uses the same guard.
- The guard sits in the handlers, matching how chat, steer and the
approval approve/deny handlers already own their method contracts.
- `messages` (GET) and the unsupported `diff`/`revert`/`unrevert` routes
are unchanged. Session rename/delete and the collection route already
dispatch on the method.
- Callers: `frontend-modern/src/api/aiChat.ts` uses POST for all six;
pulse-mobile's abort call and its OTA compatibility probe use POST.
## Proof
- `TestAssistantSessionMutationsRequirePost`: GET/HEAD/PUT/DELETE get
405 + `Allow: POST` for every mutation, with no mutation call and no
`IsRunning` check reaching the service. POST still dispatches.
- `TestAssistantSessionReadSubresourcesKeepTheirMethods`: GET `messages`
returns 200 and GET `diff` returns 501, with no mutation either way.
- `TestDemoModeRouterRejectsAssistantSessionMutationsOverSafeMethods`:
drives `NewRouter(...).Handler()` with `DemoMode` on. GET/HEAD to every
mutation pass the demo guard and get 405, POST stays 403, nothing
mutates, and GET `messages` still reads.
- Negative controls via `go test -overlay`: on main's `ai_handler.go`,
every non-POST case fails and the demo test reports `GET abort status =
200`. With abort's guard moved after the `IsRunning` check, the ordering
assertion fails.
SAML providers configured with an SSO URL and a PEM signing certificate
(idpCertificate or idpCertFile) instead of IdP metadata never verified an
IdP signature. buildManualMetadata put the certificate into the IdP
descriptor's X509Certificate field as a PEM block, but crewjam/saml
base64-decodes that field as DER, so every ACS Response and LogoutResponse
failed with "cannot parse certificate: illegal base64 data at input byte 0"
before any key was compared. Providers configured from IdP metadata were
unaffected because metadata carries base64 DER.
Store base64 DER, as metadata does. The saved idpCertificate PEM and
idpCertFile path are read as before; only the descriptor rebuilt in memory
on each load changes, so no saved configuration needs migrating.
TestContract_SAMLManualIDPCertificateVerifiesIdPSignatures signs an ACS
Response and a LogoutResponse with one test IdP key and checks that
inline-certificate, certificate-file and metadata providers all accept
them, and that a Response signed by another key is refused. The test
allows IdP-initiated responses because ProcessResponse records no
AuthnRequest IDs, so at the default every ACS Response is refused on
InResponseTo. That is a separate open bug, not fixed here.
/api/saml/{id}/slo cleared the cookie-selected session for any
LogoutResponse the IdP had signed in the last 90 seconds. crewjam/saml
v0.5.1 checks a LogoutResponse's signature, Destination, IssueInstant,
Issuer and Status but never its InResponseTo, and the route must take
GET for the HTTP-Redirect binding, where the SameSite=Lax session cookie
rides a cross-site navigation and the CSRF check does not apply. A user
who logged out through Pulse could replay the IdP's answer to their own
logout through a link and log out anyone who followed it. Delivered as a
cross-site HTTP-POST form instead, the response arrived without the
victim's cookie, yet the handler's deletion cookies still logged their
browser out.
SP-initiated logout now records each LogoutRequest ID on the provider's
service for up to ten minutes, bound to the session-store hash of the
session it ends. The SLO handler spends the record the response's
InResponseTo names and refuses the response unless that record exists
and is unexpired and the browser carries either no session or the
session that logout ended. The genuine return carries no session,
because logout clears the cookie before redirecting to the IdP, so
requiring the cookie-selected session to match would refuse every real
SLO. Refusals answer 403 with a failed saml_slo_callback audit event,
and a misdelivered response still spends its record, so each
LogoutRequest is answered at most once.
The SLO handler no longer writes session state or cookies for an
accepted response: logout already invalidated the session and expired
its cookies, and the callback cannot see which session a cookieless POST
delivery's browser holds. InResponseTo is read only from the root
element's unprefixed attribute, because encoding/xml would also fill it
from a namespace declaration such as xmlns:InResponseTo, which exclusive
canonicalization can leave outside the signature when nothing uses it.
SP-initiated logout itself now answers 401 to a request without a
session cookie. The SAML routes are public and skip CSRF, so a
cookieless cross-site POST fell through to the plain logout and its
deletion cookies, a forced logout that a deployment with authentication
configured refuses on /api/logout as unauthenticated.
The /debug/pprof handlers are gated on RequireAdmin plus a settings:read
scope that session logins skip. The public demo signs every visitor in
as the configured admin, so any visitor could fetch goroutine and heap
dumps and the process command line, or start CPU profiles and execution
traces that load the whole process. DEMO_MODE already hides diagnostics,
logs, admin users and manual discovery through the admin-operations
policy; pprof was missing from it.
The policy now has the demo guard answer /debug/pprof and every path
below it with 404 for every method, ahead of route authentication.
Instances outside demo mode keep the routes, and PULSE_PPROF_DISABLED
still removes them everywhere.
TestContract_DemoModeHidesPprofFromDemoAdminSession signs in as the
demo admin through the full router, shows the same session still reads
/debug/pprof/cmdline below the demo guard, and gets 404 for eight pprof
paths through the served handler. On the previous policy GET and HEAD
returned pprof output (200, or the root's 301 into the index) and POST
got the generic demo 403. A policy test also fails if a literal route
the router files register under /debug/, with or without a method
prefix, escapes the policy for any common method.
Two routes registered without a mux method pattern changed state on any
method. The demo-mode read-only guard and the CSRF check both admit GET and
HEAD as reads, and the SameSite=Lax session cookie rides cross-site
top-level GET navigations, so each was writable by a safe method:
- /api/discovery/settings had no method check, so a GET or HEAD carrying a
JSON body changed the discovery staleness threshold, including on a
DEMO_MODE instance.
- SP-initiated SAML logout at /api/saml/{id}/logout cleared the session and
redirected to the IdP on GET. Nothing in the UI calls it, the /api/logout
fallback it delegates to is already POST-only, and the IdP's
LogoutResponse returns to /slo, not here.
Add requireRequestMethod (405 with Allow) as the shared handler-level
guard. Discovery settings keeps PUT, its documented method, and POST, which
it accepted before; SAML logout takes POST only. An audit of every route in
internal/api found no other caller-requested write reachable by GET or HEAD
beyond the Assistant session routes guarded in #2670. GET flows the
protocol requires (OIDC login and callback, SAML login initiation and SLO,
magic-link and cloud handoff, checkout bridges) and reads that run lazy
initialization or idempotent reconciliation are left as they are. One
known exception is deferred: single-resource report generation still runs
on GET and can call the configured AI narrator, which records provider
usage in the cost ledger; moving it behind POST changes the report API the
UI downloads from, so it is tracked separately.
The Thresholds page builds its PUT /api/alerts/config body from the
settings it edits. It has no control for flapping detection, alert TTL
cleanup or custom rules, so it never sends flappingEnabled,
maxAlertAgeDays, maxAcknowledgedAgeDays, autoAcknowledgeAfterHours or
customRules. The handler decoded the body into a zero AlertConfig, so
every thresholds save turned flapping detection and TTL cleanup off,
reset the flapping tuning, dropped custom rules, and persisted that.
The page's flapping, aggregation and ioNormalization objects are not
AlertConfig fields and never enabled anything.
The handler now hands the body to Manager.ApplyConfigUpdate, which
reads the stored config, merges and applies it under the manager lock,
so two concurrent partial saves cannot revert each other. The body is
decoded on its own, so the keys it carries mean exactly what
encoding/json makes of them; every top-level field it left out is
copied from a JSON round trip of the stored config, sharing no maps
with it. ApplyConfigUpdate returns a cloned snapshot, which the handler
persists, because SaveAlertConfig normalizes the config it is handed in
place and used to write into the live config's maps. Saves run one at
a time so alerts.json ends with the last applied config. A body that
is not a JSON object, null included, is a 400. Stored configs are not
rewritten, so a value an earlier save zeroed stays as it is.
On a private stack, a node CPU change saved from the Thresholds page
zeroed all four fields in memory and in alerts.json on main; with this
change the same save kept them and stored the new threshold.
`DemoModeMiddleware` let any request carrying `Upgrade: websocket`
through, whatever its method or path. On a `DEMO_MODE` instance a
`POST`/`PUT`/`PATCH`/`DELETE` with that header reached its handler, and
the read-only guard did nothing.
## How exposed the public demo was
- The public demo (`demo.pulserelay.pro`, v6.5.0) runs `DEMO_MODE=true`
with `PULSE_AUTH_USER=demo`, and its login page signs visitors in as
demo/demo. No proxy auth is configured. Caddy is a plain `reverse_proxy`
with no method filtering, and it forwards `Connection: Upgrade` and
`Upgrade: websocket` unchanged.
- That session is the configured admin.
`sessionUserHasSettingsAdminPrivileges` matches `cfg.AuthUser`, so
`RequireAdmin` accepts it, and session requests skip token scope checks.
Nothing in the handlers checks demo mode on its own, so this middleware
was the only write guard.
- Anyone using the public credentials could therefore make any admin
write by adding the two headers. That covers switching the demo out of
mock mode (`POST /api/system/mock-mode`), changing system settings,
creating organisations, importing config, changing the password and
adding notification targets. A CSRF token was needed, but the attacker
gets one from their own login.
- On the old code, the new contract test reproduces this end to end. A
demo/demo session's mock-mode POST with upgrade headers returned 200 and
turned mock mode off.
- I found no sign it was used. Over the last 30 days the demo journal
shows every "mock mode disabled" line paired with a process restart (new
PID). Config file mtimes match deploys. 5,782 ordinary writes were
blocked over the same period. Caddy keeps no access log for the demo, so
this is the absence of evidence, not proof.
## Change
The exemption is deleted rather than narrowed. RFC 6455 handshakes are
`GET` requests, gorilla's `Upgrader` refuses any other method, and `GET`
already passes the read-only branch above it. A narrowed `GET`-only
exemption would never run. `/ws` and `/api/agent/ws` keep connecting,
and the request method alone decides.
## Tests
- `TestContract_DemoModeUpgradeHeadersDoNotExemptWrites` (registered
api-contracts proof) runs against the full router. It signs in as the
configured demo admin and checks that the session passes `RequireAdmin`
for a read. A mock-mode POST carrying `Connection: Upgrade` and
`Upgrade: websocket` now gets the demo 403 and leaves mock mode on, and
a real `/ws` handshake from the same session still upgrades.
- `demo_middleware_test.go`: the table row that asserted the bypass now
expects 403. POST/PUT/PATCH/DELETE with handshake headers to mock-mode,
settings, orgs, `/ws` and `/api/agent/ws` all get 403. Real gorilla
handshakes to `/ws` and `/api/agent/ws` through the middleware still
upgrade and echo.
- Negative control: run against the old middleware via `go test
-overlay`, the new contract test and the upgrade-header test fail, and
the handshake test passes on both versions.
## Not changed here
- `auth.go` `isWebSocketUpgrade` and `configapi`
`explicitAPITokenFromRequest` still accept a `?token=` query credential
on any request with `Upgrade: websocket`. A valid, scoped token is still
required, so no access is gained, but the credential can travel in a
URL. The `ErrorHandler` skip in `middleware.go` only bypasses recovery
and metrics.
- Older and separate:
`/api/ai/sessions/{id}/{abort,summarize,fork,undo,redo}` change state on
`GET`, which the demo guard admits as reads. Tracked separately.
The session router dispatches /api/ai/sessions/{id}/abort, summarize,
fork, undo and redo by path alone, and none of those handlers checked
the method, so a GET or HEAD cancelled a running turn, compacted or
rewound a transcript, or persisted a fork. The demo-mode guard admits
every GET and HEAD as a read, so a public-demo visitor could mutate
sessions despite the read-only mode. The CSRF check also skips GET and
HEAD, and SameSite=Lax session cookies ride cross-site top-level GET
navigations, so a link carrying a session ID could do the same to a
signed-in user.
Each mutating handler now admits only POST and answers anything else
with 405 and Allow: POST before resolving the Assistant service. Steer
already refused non-POST but sent no Allow header; it now shares the
same guard. The checks sit in the handlers, where steer, chat and the
approval handlers already own their method contracts. messages (GET)
and the unsupported diff/revert/unrevert routes are unchanged. Every
caller (aiChat.ts, the mobile app and its OTA abort probe) already sends
POST.
DemoModeMiddleware admitted any request carrying "Upgrade: websocket",
whatever its method or path, so a POST/PUT/PATCH/DELETE with that header
reached its handler on a read-only demo instance. The public demo signs
visitors in as the configured admin user, which RequireAdmin accepts and
which skips token scope checks, so the middleware was the only write
guard: adding two headers let a demo session switch the instance out of
mock mode, change system settings or create organisations.
The exemption never served a real handshake. RFC 6455 handshakes are GET
requests, gorilla's Upgrader refuses any other method, and GET already
passes the read-only branch above it, so /ws and /api/agent/ws keep
connecting. Removing the branch leaves the method alone deciding.
TestContract_DemoModeUpgradeHeadersDoNotExemptWrites signs in as the demo
admin through the full router, proves the session passes RequireAdmin,
gets 403 for a mock-mode POST with upgrade headers and still opens /ws.
On the old middleware the same POST returned 200 and turned mock mode off.
Disk temperature alerts evaluate each SMART disk under the thresholds of
the host agent that reports it. An explicit Disk Temp override on that
host, or one inherited from its linked node or guest, replaces the
per-type trigger, and a host whose alerts are switched off raises none.
Since #2636 the Physical Disks verdict and Patrol judged heat by disk
type alone. A host with an 80C override still read Running Hot at the
70C NVMe trigger, and a host with its alerts off still read hot.
- alerts.Manager.HostDiskTemperatureThreshold resolves the policy for a
host exactly as CheckHost does, from the same helpers.
DiskTemperatureThreshold is its hostless case.
- Patrol and the AI chat context find each disk's host by walking its
parent to the agent machine (through the pool for Unraid disks) and
ask the threshold adapter for that host's limits.
- Physical Disks, the disk detail, the pool drawer's linked disks and
the guest drawer pass the reporting machine's override keys to the
thresholds resolver, in the order CheckHost reads them: agent ID,
canonical ID, then the merged node or guest. The resolver treats an
override or agent default that switches alerts off as no heat.
Two cases still fall back to the per-type trigger in the browser while
alerts and Patrol apply the override: on the Storage page, a disk whose
agent was manually linked into a VM, and an override an agent inherits
from a guest it is linked to without a merge.
Follows #2591. A node alert that moves to its linked Pulse agent closes
with a `moved_to_agent` resolution (#2580). Two readers still presented
that close as a recovery.
**Performance reports.** The node, VM and container enrichers copied
recently resolved alerts into `reporting.AlertInfo` without the
resolution, so a report generated in the five-minute window after a
handover read HEALTHY, "All systems operating normally", counted the
alert as "triggered and resolved", and sent the AI narrator `resolved:
true`.
- Report rows now carry the alert's resource and resolution
(`reportActiveAlertInfo` / `reportResolvedAlertInfo`).
- `pkg/reporting` has one rule (`unresolvedAlerts`): an alert that
closed without recovering stays live unless the report also shows its
successor's reading for that metric, meaning an open alert or a recovery
since the handover. That way the condition is counted once, and fleets
match handovers across all their resources.
- The single-resource PDF card reads WARNING (or CRITICAL at the alert's
level) with "Alert moved to <agent> - check the agent for the current
reading". The active list names the agent, the alerts table shows
"Moved" with the engine's summary as a note, and recommendations say to
check the agent.
- Fleets count the alert as active and name the agent. Their all-clear
pattern and "No fleet-wide action required" no longer appear while any
alert is active.
- The narrator payload carries `resolved: false`, the summary and
`successor_alert_listed`, with prompt rules for both.
**Resource history.** `alert_resolved` changes were labelled by kind
alone ("Alert resolved", green) with that label prefixed to the
headline. `getResourceChangePresentation` now reads the
`alert_resolution` metadata: the change reads "Alert moved" in the
neutral blue of the history badge, and its summary is the whole headline
in the drawer, `ResourceChangeSummary` and Patrol's assessment handoff.
**Verification**
- A private mock stack running this branch, with a local-only overlay
that closes a real pve1 memory alert through `clearAlertWithResolution`.
Within the window, `/api/admin/reports/generate?resourceType=node`
rendered CRITICAL with the handover message and a "Moved" row, while the
ordinary CPU recovery stayed green.
- In the browser, a full drawer (Availability checks, Fleet view) showed
the producer-made rows as "Alert moved" next to "Alert resolved" at 1280
and 375. A Proxmox node's own drawer uses the `table-row` presentation,
which fetches no change history (see below).
- Tests in registered proof files: `pkg/reporting/alert_state_test.go`
(verdict, successor timing, fleet pool, PDF text and width),
`internal/api/contract_test.go`, `internal/ai/report_narrator_test.go`,
`resourceChangePresentation.test.ts`,
`ResourceDetailDrawer.history.test.tsx` and
`patrolInvestigationContextModel.test.ts`. Each failed with the fix
reverted.
- Second opinion: two read-only review rounds. The temporal match,
fleet-wide pool, narrator dedupe signal, fleet all-clear fallbacks and
contract wording came from them.
**Not fixed here (separate changes)**
- Reports requested as `agent` carry no alert rows at all. That is how
the Reports page addresses every Proxmox node in v6, so such a report
reads HEALTHY even with active critical alerts. This change makes the
shared rule correct for when those reports attach the machine's alerts.
- Node and guest alert lifecycle changes are recorded under legacy alert
resource IDs, which canonical history queries never match. Facet kind
counts still group moved closes under "Alert resolved".
TestContract_ConnectionSystemMembersIgnoreOwnedPhysicalDisks proves a
disk's health never folds into its cluster node's liveness. Its warning
disk came from heat: an NVMe at 63C, which the flat 60C disk risk rule
flagged. Disk risk no longer judges temperature, and 63C is under the
NVMe alert trigger, so that disk is now online and the precondition
failed. Reallocated sectors give the disk the same warning status
through real SMART evidence, which keeps the test on its subject.
The PDF performance report's disk table coloured a temperature amber
from 50C and red from 60C for every disk type, the last fixed disk heat
line after disk heat moved to the alert disk temperature policy. An NVMe
at 63C, well under its 70C alert trigger, printed red. A user's raised
trigger changed nothing.
Each reported disk now carries the tenant's alert thresholds for its
type (the clear value and the trigger, from
alerts.Manager.DiskTemperatureThreshold). The PDF colours the reading
amber from the clear value and red from the trigger, as the Physical
Disks Temp column does, and leaves it plain when disk temperature
alerting is off.
Pulse decided whether a physical disk was too hot in three places with
three thresholds. Disk risk added temperature_high at a flat 60/70C for
every disk type and drove the disk status and the Physical Disks Health
verdict. Patrol flagged anything over 55C and the AI chat context listed
disks from 50C. Alerts and the Temp column used the per-type thresholds
users tune in Alerts (NVMe 70, SAS 65, SATA 55 by default). An NVMe at
63C read Running Hot beside a green Temp cell. A SATA disk at 56C read
Healthy beside a red one. A user who raised the NVMe trigger to 75 still
got a hot verdict at 70.
The alert disk temperature policy now decides disk heat.
alerts.Manager.DiskTemperatureThreshold exposes the per-type resolution
CheckHost already applies.
- Disk risk no longer judges temperature. Every registry that rebuilds
risk lacks the alert configuration, so no rule there could honour it.
A hot disk keeps an online status.
- The Health verdict reads Running Hot when a current reading reaches
the disk type's trigger. It uses the thresholds object that colours
the Temp cell, so the two cannot disagree. The amber band below the
trigger stays a cell colour, not a verdict.
- The TrueNAS storage table lost the heat it used to get from risk. It
now judges heat the same way: an Attention row with the reason, and a
warning dot for phones.
- Patrol triage, its disk issue filter, finding recovery and the chat
context's attention list judge the collected reading against the same
trigger, including scoped runs and a provider set before Patrol
starts.
- A switched-off agent Disk Temp default resolves to no thresholds,
which the displays that read them now treat as off.
Per-host Disk Temp overrides do not reach these surfaces yet. TrueNAS
disk alerts, the TrueNAS disk drawer and PDF reports keep their own
fixed temperatures for now.
Normalization keeps a disk's last-known temperature when the current
observation did not collect one (a disk in standby, a host agent past
its reporting lease) and marks it non-available. Several readers still
took PhysicalDisk.Temperature at face value. The AI chat context listed
a standby disk as needing attention for heat. Patrol raised health
issues and triage flags from the retained value. The AI query and
disk-health tools handed it to the model as the disk's temperature.
Performance reports and the reporting runtime snapshot tabulated it as
measured, and the disk temperature charts padded a series out to now
with it.
Each of them now goes through diskinventory.TemperatureCollected. AI
surfaces keep a retained value as context, labelled last known with
its reason. Reports and the charts leave it out. monitor.go's private
copy of the predicate is replaced by the shared helper.
Normalization may keep a disk temperature it did not collect (a disk in
standby, a host agent past its reporting lease) as long as its collection
state says so. Two consumers still presented that value as current.
The metrics-history handler answered an empty disk range with the disk's
temperature as a "live" point at now, regardless of its state. Since the
history writers stopped recording retained temperatures, a short-range
chart for a silent agent's disk or a sleeping disk showed the retained
value as a fresh reading. Mock mode also seeded synthetic series from it
and padded sparse stored series out to now. All of these now require a
temperature collected now (diskinventory.TemperatureCollected, the
shared form of the rule the history writers apply).
Unraid array-inventory rows carry no per-field provenance, so a
temperature taken from one looked collected after the agent went silent.
The adapter now derives its state from the host lease that
ExpireHostTelemetry already records, and from the disk's spin state:
unavailable once the host is offline or the disk is spun down. A SMART
row that borrows the inventory reading takes that state, including
available. The native row records only the withdrawing states, because
the registry merge picks temperature and state separately and an
available claim from it would mask the SMART row's own state. A
withdrawn inventory temperature no longer counts as heat in disk risk.
A node alert that moves to its linked Pulse agent closes with a
moved_to_agent resolution (#2580), and #2591 taught Patrol, alert
history and the assistant to say so. Two readers still showed the
close as a recovery.
Performance reports dropped the resolution when copying recently
resolved alerts into reporting.AlertInfo. A node report generated in
the five-minute window after a handover read HEALTHY, "All systems
operating normally", counted the alert as triggered and resolved, and
told the AI narrator resolved: true, although nothing had seen the
condition clear. Report rows now carry the resolution and the alert's
resource. pkg/reporting treats an alert that closed without recovering
as still live unless the report also shows its successor's reading for
that metric (an open alert, or a recovery since the handover), so a
handover never reads as healthy and one condition is counted once;
fleet reports match handovers across all their resources. The PDF
card, Quick Stats, active list and heuristic narrative say where the
alert went and to check the agent, the alerts table shows "Moved" with
the engine's summary, fleets count it as active and name the agent,
and the narrator gets resolved: false, the summary and whether the
successor's alert is listed, with prompt rules for both.
Resource change readers labelled every alert_resolved change "Alert
resolved" in green and prefixed the headline with it, so a moved close
read "Alert resolved: Alert moved to pve1 (Host Agent). This is not a
recovery...". getResourceChangePresentation now reads the
alert_resolution metadata: such a change is "Alert moved" in the
neutral blue of the history badge, and its summary is the whole
headline in the drawer, ResourceChangeSummary and Patrol's assessment
handoff.
Reports requested as agent, which is how the Reports page addresses
every Proxmox node, still carry no alert rows at all, and node alert
history is stored under legacy node IDs that the drawer never queries;
both are separate fixes.
The webhook editor sends back the ***REDACTED*** values the list returns
for every header, custom field and signing secret the user did not
retype. Three paths mishandled them:
- Update restored the whole saved header or custom-field map whenever any
value was masked, silently dropping a header or field added in the same
edit.
- Update echoed the restored saved custom fields, so a Pushover token and
user key came back in the response even though the list masks them, in
breach of the api-contracts webhook response clause. Create echoed the
request, and could store a masked value verbatim.
- The editor's Test button posts the form, so testing an edit after a
reload sent the literal mask as the token or header.
Resolve each masked value to the saved value of the same key on update and
on a form test that carries a saved webhook's id, keep values typed in the
same request, and drop masks with no saved value. Build list, create and
update responses from one masked shape. Normalization now drops blank
headers, so clearing a header removes it instead of sending an empty
Content-Type that generic receivers cannot parse.
A token binds the first agent ID its command channel presents and never
moves. The agent registered that channel before its first report, under
its configured --agent-id, or an ID Pulse had forked for a cloned
machine ID or kept from an earlier enrollment. Pulse meanwhile resolved
its reports to another host ID and acknowledged it, and the agent
persisted that ID. The config gate, which judges the resolved ID, then
turned the operator's commands off, and the next restart under the
persisted ID was refused for good.
Report acknowledgements now name commandAgentId: the resolved host ID,
only when the token's binding already holds it or a registration
presenting it would take the first bind, backfill, legacy migration,
hostname rebind or deploy repair it would get anyway. The credential
must also be one command admission takes, and the host must be held
under the reporting token. Naming grants nothing that registration
would not. An established binding to another ID is never moved from
report contents, because resolution can map a report onto another
host's continuity record by machine ID, so a forged report and a stale
one look the same. The config gate takes the same decision.
The agent keeps its channel staged until a report is acknowledged and
registers under the named identity, or under its own ID when none is
named, which is what an older Pulse or an already-bound token sees.
An unreadable ack keeps it waiting, and a 403 releases it under its own
ID because Pulse can never answer that token. A running client is never
rebuilt, since that would reopen its receipt store under in-flight
operations. A superseded client can no longer take the run slot.
Tokens already bound to a diverged ID stay as they are; re-running the
install command issues a fresh token that binds the resolved identity.
## Problem
The safe-profile migration's authority reduction (`POST
/api/agents/collector/reduce-authority`) needs the agent's ID on its
token: `bound_agent_id` must be set and equal the ID the installer
sends. Until now only a command-channel registration wrote that ID.
Deploy enrollment (since rcourtman/Pulse#2542), Proxmox
auto-registration and pre-v6.1.1 installs all leave a token bound to the
node's hostname with no agent ID. The ID is filled in only at the
agent's first command registration. An agent whose token lacks
`agent:exec`, or that never connects the command channel, never gets an
ID on its token. The reduction then refuses it with "Collector
credential binding mismatch" for as long as the agent runs. Before
#2542, deploy tokens instead carried an invented `agent-<hostname>`,
which failed the same check.
## Fix
The report path now records the agent's identity on such a token, under
the same rules as command binding:
- **When it binds.** After `/api/agents/agent/report` resolves a report,
a token with a `bound_hostname` and no `bound_agent_id` records the
server-resolved host ID. This happens only when the report presented
that same ID. In the same run that is the ID the agent's command
registration presents. It is also the `agentId` the response
acknowledges, which the agent persists and the installer sends to the
reduction.
- **Same decision as the command channel.**
`agentbinding.EvaluateReportedIdentity` records only where `Evaluate`
would first-bind, legacy-migrate or backfill that ID for the reported
hostname. It also requires:
- a token bound to at most one organization, whose command-session
organization matches the report's;
- a collector, legacy full-trust or empty runtime role;
- a token that has not expired.
- **Only a host Pulse actually holds.** The resolved host must be one
the monitor holds under that token (`host.TokenID`). Mock mode's
discarded, unresolved acknowledgements therefore never qualify, however
mock mode is toggled.
- **Judged on the report's own hostname.** That is the hostname the
agent's command registration presents, and it matters because a report
rejected by the ordering watermark is answered with the stored host.
- **Nothing else changes.** The path writes `bound_agent_id`, `bound_at`
and the binding version. The binding version makes the ID immutable to
every later registration. Scopes, runtime role, command policy and
`bound_hostname` are never touched. A bound ID never moves and a deploy
placeholder is never repaired. A recorded identity grants no execution
authority: command admission still needs `agent:exec` and the same ID.
- **Same store and lock as command admission.** The write goes to the
router's canonical token store and persistence, under the token lock
command admission holds, against the live record. A failed save restores
the previous metadata and the next report retries.
- **Cheap screen first.** The handler checks the request's token
snapshot before taking the exclusive token lock, so already-bound or
ineligible tokens cost nothing per report.
Contracts updated: agent-lifecycle, security-privacy, api-contracts and
storage-recovery.
## Behaviour change: the first recorded identity is final
As with a command registration, the first identity recorded is final.
Take an agent that reported under its machine ID while it had no command
channel, for example with commands off. If it is later restarted under a
different `--agent-id`, its command registration under the new ID is
refused on that token. A report replayed from an earlier run's buffer
has the same effect. This is exactly what happens today when that first
run's command channel did connect.
`TestReportedIdentityIsFinalForALaterReconfiguredAgent` pins the
behaviour. Command-channel identity convergence lifts it: that follow-up
to #2542 has the agent register under the identity Pulse names in the
report ack, and it builds on this decision.
## Tests
-
**`TestContractDeployEnrolledCollectorReducesItsAuthorityAfterItsFirstReport`**
(`internal/api/contract_test.go`) replays enrollment without commands,
the first report and the reduction, through the real router. It checks
that the token's scopes and runtime role are unchanged and that command
admission is still refused. It fails without the fix: the reduction
returns 403.
- **Boundary tests in `internal/api/security_regression_test.go`:**
- `TestReportedIdentityMatchesCommandRegistrationAndNeverMoves`: the
recorded ID matches the agent's own command registration; neither a
second identity's registration nor another machine's reports moves it;
an ID the command channel bound first survives a report that resolves
elsewhere.
-
`TestReportedIdentityRecordsNothingWhenTheResolvedIdentityIsNotThePresentedOne`:
an agent with a configured `--agent-id` records nothing, and its command
registration is still admitted.
- `TestReportedIdentityIsFinalForALaterReconfiguredAgent`: pins the
trade-off above.
- `TestReportedIdentityJudgesAReplayedReportByItsOwnHostname`: a report
the ordering watermark rejects is answered with the stored host, and is
judged on its own hostname rather than the stored one.
- `TestReportedIdentityRestoresTheTokenWhenTheSaveFails`,
`TestReportedIdentityRequiresTheTokensOwnOrganization` and
`TestReportedIdentityIgnoresAnExpiredToken`.
- `TestReportHandlerOffersOnlyQualifyingLiveReportsToTheRecorder`: the
handler's snapshot screen and mock mode.
- **`TestApplyHostReportStampsTheTokenOnlyOnHostsItHolds`**
(`internal/monitoring/monitor_host_agents_test.go`) pins the monitor
properties the handler relies on:
- an applied report carries its token's ID;
- a watermark-rejected report returns the stored host and hostname;
- mock mode's discard acknowledgement carries no token.
-
**`TestEvaluateReportedIdentityRecordsOnlyWhatCommandRegistrationWouldBind`**
(`internal/api/agentbinding/policy_test.go`) is the policy table: the
deploy, auto-registered, legacy and equivalent-hostname cases, plus
every refusal, including a versionless token already bound to an ID.
- **Mutation checks.** Each of these fails at least one named test:
- dropping the wiring;
- allowing presented ≠ resolved;
- letting a bound ID move;
- allowing multiple organizations;
- allowing any runtime role;
- skipping `Evaluate`;
- not stamping the binding version;
- not restoring on a failed save;
- dropping the organization or expiry check;
- dropping the held-host check;
- judging by the stored host's hostname;
- dropping the snapshot screen.
Reviewed adversarially in three passes:
1. The first pass found two issues: the overstated "same ID as command
registration" claim, now scoped to the same run and paired with the
final-identity trade-off; and a mock-mode toggle race, replaced by the
held-host check.
2. The second pass found that replayed reports were judged on the stored
hostname, now judged on the report's own.
3. The third pass found nothing further.
Governance:
- `canonical_completion_guard.py` passes in CI mode on this commit.
- `registry_audit.py --check`, `contract_audit.py --check` and
`status_audit.py --check` pass.
- All governance `*_test.py` suites from `canonical-governance.yml` and
`go test ./internal/repoctl` pass.
## Not in this change
- **Presented ≠ resolved.** When the agent presents an ID other than the
one Pulse resolves (a configured agent ID, a continuity fork), nothing
is recorded. Reconciling that disagreement belongs to the
command-channel identity convergence follow-up.
- **Tokens bound to no hostname.** Generic host install tokens without
`agent:exec` still get their binding only from command-channel first
use, so their reduction still has no ID to match.
- **Historical deploy placeholders.** Tokens still bound to
`agent-<hostname>` are repaired only by command registration, so a
placeholder token without `agent:exec` still cannot reduce.
- **Builds on #2542**, which has merged; this branch adds one commit on
top of it.
The non-required **Benchmarks** check has failed on every non-superseded
PR since about 15:00 UTC on 5 October. Examples are #2521, #2520, #2524,
#2525, #2527, #2530 and #2532, each ending with `Paired base and
candidate benchmarks (pull requests)` timed out after 25 minutes. The
paired step runs 10 base and 10 candidate samples, so each second added
per sample costs 20 seconds of step time.
### What made it slower
Paired step duration across PR runs, with per-package seconds per sample
taken from the `ok` lines in the job logs:
| Period | Paired step | `internal/unifiedresources` |
`internal/monitoring` |
|---|---|---|---|
| 24 Sep – 5 Oct 08:00 | 16–18 min | 7.9 s | 4.5 s |
| after #2486 (08:11) | 20.5–20.9 min | 16.2 s | 5.0 s |
| after #2518 (15:01) | killed at 25 min | 38.2 s | 9.9 s |
- **#2518** added `BenchmarkRegistryProjectionSnapshot`. Its `cold` mode
rebuilt the registry between `b.StopTimer` and `b.StartTimer` on every
iteration. Go picks `b.N` from timed work only, so the empty-registry
cases ran hundreds of thousands of untimed ingests. Per sample,
`/0/cold/point-targets` took 10.3 s and `/0/cold/bulk-targets` 5.9 s at
a 100 ms benchtime. That is the cause of the timeouts.
- **#2486** added 46 owner-index and metadata sub-benchmarks, about +8 s
per sample. That is legitimate coverage. #2518's
`BenchmarkBroadcastMetricsSnapshot` and #2527 added about +6 s more.
- `-count`, `-benchtime` and the package list did not change.
### Fix
1. **Root cause.** `cold` now invalidates the registry's derived state
(canonical metadata, typed views, source-target index) instead of
rebuilding the registry. The refresh never reads its own output, so this
repeats a fresh ingest's first read: at 1000 VMs, 40011 vs 40014
allocs/op and about 5.09 MB/op, the same as before. Locally the 24-case
family now runs in 4.9 s; before, its 6 cold cases alone took 33.8 s.
2. **Scope.** Paired PR runs now sample only packages whose in-module
dependency closure (`go list -deps -test`) holds a changed file. A
`_test.go` file selects only its own package. Changes to
`go.mod`/`go.sum`, dirty or non-Git trees, or an unreadable graph select
every package. Applied to today's failures: #2521, #2524 and #2525
sample nothing; #2530 samples `internal/api`; #2532 samples api and
monitoring; #2520 samples 5 packages; #2527 samples 7. The metadata
records `selection=` and `packages=`.
3. **Budget.** A module or shared-model change still samples nearly
everything, about 24 minutes including the ~2.7 min warm-up. The step
timeout is now a measured 30 minutes (job stays at 35).
`PULSE_BENCH_BUDGET_SECONDS=1680` makes the script project its total
after each round. Once the projection reaches 28 minutes it stops and
prints each package's mean seconds per sample, which replaces an
unexplained timeout. `TestPairedBenchmarksStopInsideTheirBudget` keeps
budget < step timeout < job timeout.
4. **Store-backed chart benchmarks** (second commit). Go calls a
benchmark function again for each `b.N` it tries.
`HandleWorkloadCharts_StoreBacked` and
`HandleWorkloadsSummaryCharts_StoreBacked` each seeded 84,000 points
into a fresh SQLite store (about 0.6 s) to time a request of 0.2 ms or
less, two or three times per sample. In CI that was 3.5 s and 3.6 s of
internal/api's 11.5 s per sample. The timed loop now runs in a
`range=4h` sub-benchmark, so the parent seeds once per run. Data,
request and bytes/allocs per op are unchanged, and locally the pair
falls from about 6 s to 1–1.5 s. The names gain `/range=4h`, so this
PR's comparison reports them unmatched.
Unpaired branch runs still sample every package. With the cold fix,
their 15-minute step (13–14.2 min today) regains about 3 minutes of
headroom.
### Verification
- `scripts/tests/test-ci-benchmarks.sh` covers no-op, own-test-only,
shared-dependency, unreadable-graph, dirty-tree, module-file,
budget-exceeded, in-budget and invalid-budget cases. Five mutations of
the selection and budget logic are each caught.
- On the real package graph, selection maps `internal/models` to 7
packages, `internal/monitoring` to api+monitoring, an `internal/api`
embed to api, and frontend or docs paths to none.
- End to end with real Go on two clean clones (`origin/main` vs this
change), only `./internal/unifiedresources/` was sampled. Its per-sample
time went from 53.9 s to 26.7 s on a loaded Mac.
- `go test ./scripts/installtests -run
'TestPairedBenchmarksStopInsideTheirBudget|TestBenchmarkQualificationRetainsProvenance'`,
plus the script-reference and doc-drift suites.
This PR's own Benchmarks run should sample only
`internal/unifiedresources` and `internal/api`, because both changes are
those packages' own test files.
Deploy enrollment, Proxmox auto-registration and pre-v6.1.1 installs
leave an agent's token bound to the node's hostname and to no agent ID
until the agent registers a command channel. An agent whose token lacks
agent:exec, or that never connects that channel, never gets an ID on its
token, and the safe-profile migration's authority reduction, which
matches that ID, refuses it for as long as the agent runs.
A report the monitor resolved under such a token now records the
server-resolved host ID on it when the report presented that same ID,
which in the same run is the ID the agent's command registration
presents. The shared binding policy must first-bind, migrate or backfill
that ID for the reported hostname, the token must run command sessions
in the report's organization, and the binding version makes the ID
immutable. Scopes, runtime role and command policy are untouched, so a
recorded identity grants nothing; command registration still needs
agent:exec and the same ID. When the presented and resolved IDs differ,
nothing is recorded and the command channel decides as before. As with a
command registration, the first identity recorded is final, so an agent
later restarted under a different --agent-id is refused under that ID on
the same token.
## Problem
Agents installed through the one-step cluster deploy lose their command
channel for good after their first restart.
`POST /api/agents/agent/enroll` bound the runtime token to an invented
`agent-<hostname>` and returned that as the agent's ID. The agent used
it until Pulse acknowledged its first report with the host's real,
machine-derived ID, which the agent persists. On the next start the
agent registered under that real ID. The token was by then a version-2
binding to `agent-<hostname>`, so admission refused it with "Agent token
mismatch: token is not bound to the registering agent", every few
minutes, indefinitely.
Seen live on a v6.5.0-rc.1 server with a deployed Proxmox node: reports
flowed, but the node could run no commands or actions.
## Fix
- **Enrollment leaves identity to the agent.** The runtime token is
bound to the node's hostname and the current binding version, marked
`deploy_identity_binding=agent`, and the response no longer carries
`agentId`. The first command registration backfills the agent's own ID
through the existing hostname-bound backfill path, and the config gate
takes the same decision.
- **One-time repair for tokens already issued.** A token enrollment
bound to `agent-<hostname>` (it has `deploy_job_id` and no identity
marker) may move once to the registering agent's ID:
- The hostname must match exactly (case-insensitive), not by short-name
equivalence.
- The move persists `deploy_identity_binding=repaired` in the same save,
and a failed save restores the previous metadata.
- It applies whatever the token's binding version.
- Any other identity or hostname is refused rather than handed to the
legacy hostname migration.
- A marked token never moves again, including after a hostname rebind
recreates the `agent-<hostname>` shape.
## Tests
-
**`TestHandleEnroll_RuntimeTokenBindsTheAgentsOwnIdentityOnFirstCommandRegistration`**
replays the field sequence: the identity enrollment handed out, then the
acknowledged identity after a restart. It fails on the old code at the
"registration after restart" step.
- **`TestDeployPlaceholderTokenMovesToTheRegisteringAgentOnce`** covers:
- the repair on the exact hostname only;
- the persisted marker after a reload;
- the repair, rebind to host b, repair-again attack;
- config-gate parity.
- **`TestVersionlessDeployPlaceholderCannotBeMovedTwice`** covers the
versionless variant of that attack.
- **`TestCurrentDeployTokenNeverMovesOnceBound`** checks that a token
issued by current enrollment never moves once bound.
- **Policy unit tests:**
`TestEvaluateRepairsDeployPlaceholderBindingOnce`,
`TestEvaluateVersionlessDeployPlaceholderOnlyMovesThroughTheRepair` and
`TestEvaluateCurrentDeployTokenBackfillsTheAgentIdentity`.
- **`TestContractDeployEnrollResponseCarriesNoAgentIdentity`** pins the
enroll response keys.
- **Mutation checks:** removing the marker check, loosening the hostname
match, not persisting the marker, or letting placeholders fall through
to the legacy branch each fails at least one of these tests.
## Not in this change
Both of these exist for every install path and are not made worse here:
- The first command registration binds whatever ID the agent presents.
If that differs from the identity Pulse later resolves for its reports
(a configured agent ID, or a host continuity fork), the next restart
still mismatches.
- Tokens that never connect the command channel keep an empty
`bound_agent_id`, so collector-authority reduction cannot match them.
Before this change they had a non-empty but wrong `agent-<hostname>`,
which failed the same way.
Go calls a benchmark function again for every b.N it tries. Each of the
two store-backed chart benchmarks seeded 84,000 points into a fresh
SQLite store, about 0.6 s, to time a request of 0.2 ms or less, and did
so two or three times per sample. In CI they took 3.5 s and 3.6 s of
internal/api's 11.5 s per sample, for about 0.1 s of timed work each.
The timed loop now runs in a range=4h sub-benchmark. A benchmark with
sub-benchmarks runs once, so the parent seeds its store once per run
while the data, request and sanity checks stay the same, and bytes and
allocations per op are unchanged. Locally the pair falls from about 6 s
to 1-1.5 s. The names gain /range=4h, so the first paired comparison
reports them unmatched.
An agent on a Proxmox cluster node reports SMART data for the node's
disks, and the registry stamps each disk with its node's Proxmox
identity so it stays discoverable in the Proxmox workspace. The
/api/connections member projection accepted any resource carrying that
identity, so each disk merged into its node's member row, adding its
model name to the aliases and its health to the node state.
A disk with a risk warning (for example 60C or more) therefore made an
actively reporting node show Stale. Since members are merged by
freshness, the row flipped every time the agent report and the Proxmox
poll swapped places, several times a minute on a customer's three-node
cluster running 6.4.5.
Only node resources now compose cluster members. The disk keeps its
ownership facet, risk and storage evidence unchanged.
Deploy enrollment bound the runtime token to an invented agent-<hostname>
and handed that ID to the agent. Once Pulse acknowledged the agent's reports
with its machine-derived ID, the next restart registered under that ID and
the command channel refused it for good.
Enrollment now binds the token to the node's hostname only and lets the first
command registration bind the agent's own ID. Tokens already issued with the
placeholder move once, on the exact same hostname, to the registering agent's
ID, and record that they were repaired so they never move again.
Preserve the exact documentation and regression-test candidate; native browser follow-through and the missing UI remain owned separately.
Change-source: pulse-maintainer
TestPatrolAutopilotStoreUnavailableDoesNotChangeModeOrFabricateEvidence
built its request through patrolAutopilotSessionRequest, which writes a
session into the process-global store, but never initialized one. Run
alone it panicked in GetSessionStore; in the package run it passed only
because an earlier test left a store behind.
The rest of the file got its store from newPatrolAutopilotTestHandler,
which called InitSessionStore on a t.TempDir() and never cleared it, so
these tests were also the ones leaving a store pointed at a deleted
directory for whatever ran next.
Both now go through initPatrolAutopilotSessionStore, which follows the
package's existing resetSessionStoreForTests/t.Cleanup pattern: start
from no store, initialize one on a temp dir, and clear it when the test
ends. Every test in the file passes when run alone.
Serve a harmless 404 for browser favicon requests and bind synthetic session requests to their recorded user agent. Retain the earlier browser setup failure.
Contract-Neutral: Synthetic proof-fixture preparation only; no runtime or rule-removal instructions changed.
Change-source: pulse-maintainer
Explain the missing v6.4.5 undo control and the existing scoped API. Verify exact selection, confirmation, browser session and CSRF, failure handling and readback without copying credentials or deleting unrelated rules.
Contract-Neutral: Documentation and regression-proof coverage of existing Patrol suppression APIs only; no runtime, route, authority or subsystem contract changes.
Change-source: pulse-maintainer
Adding the Pulse Agent to a Proxmox cluster member meant generating a
token in the UI and pasting it into an interactive prompt on the host,
even though Pulse already knows the cluster and has an agent on its
siblings. The server-side cluster deploy that avoids this was still
shipped, but its wizard was deleted as an orphan in May, and it could
not reach members discovered by hostname: targets were parsed from the
API URL, so a name like "delly2" hit the agent's literal-IP guard and
failed preflight.
Deploy targets now resolve to a literal IP in the member's API URL or
its cluster endpoint's effective IP, matched on the member's own
connection, and members with no known address come back undeployable
with a reason. The Settings > Infrastructure install actions open one
dialog that checks, installs, and confirms each node is reporting,
falling back to the scoped installer when no sibling agent is
connected or a node cannot be reached.
The deploy routes are keyed on the cluster name, so they now refuse a
name shared by two connections instead of merging two sites, and job
creation admits one install per cluster at a time under a lock, which
lets the dialog keep a run going after it is closed.
The agent-lifecycle, api-contracts, frontend-primitives, and
storage-recovery contracts record the new install path, its address
rule, and the persisted, time-bounded admission check.
Supersedes #2507, which split this change across two commits; the
per-commit canonical completion guard needs the contract updates in the
same commit.
Capture all endpoint limiters under their Router owner and seal late worker
admission. Concurrent shutdown joins each cleanup worker without resetting
request budgets or stopping another Router. Integration fixtures use the real
shutdown lifecycle before monitor teardown.
Contract-Neutral: Router worker ownership and concurrent shutdown only; HTTP payloads, limits and windows, auth, tenant selection, revocation and persistence contracts are unchanged.
Change-source: pulse-maintainer
Adding the Pulse Agent to a Proxmox cluster member meant generating a
token in the UI and pasting it into an interactive prompt on the host,
even though Pulse already knows the cluster and has an agent on its
siblings. The server-side cluster deploy that avoids this was still
shipped, but its wizard was deleted as an orphan in May, and it could
not reach members discovered by hostname: targets were parsed from the
API URL, so a name like "delly2" hit the agent's literal-IP guard and
failed preflight.
Deploy targets now resolve to a literal IP in the member's API URL or
its cluster endpoint's effective IP, matched on the member's own
connection, and members with no known address come back undeployable
with a reason. The Settings > Infrastructure install actions open one
dialog that checks, installs, and confirms each node is reporting,
falling back to the scoped installer when no sibling agent is
connected or a node cannot be reached.
The deploy routes are keyed on the cluster name, so they now refuse a
name shared by two connections instead of merging two sites, and job
creation admits one install per cluster at a time under a lock, which
lets the dialog keep a run going after it is closed.
The agent-lifecycle, api-contracts, frontend-primitives, and
storage-recovery contracts record the new install path, its address
rule, and the persisted, time-bounded admission check.
Admit detached metadata in order, merge identity fields without restoring stale
operator edits, and persist complete snapshots with one owned writer. Seal late
writes and return shutdown failures before organisation data or offboarding is
removed. Preserve existing drain budgets, file format, tenant boundaries and
atomic persistence. Add controlled burst, ordering, write-error, blocked-close
and real deletion controls.
Contract-Neutral: Behavioural repair of existing JSON persistence and organisation deletion; no route, field, permission, identity format, schema or provider authority change. Shutdown failure uses the existing HTTP error envelope. Internal ownership methods and explanatory contract text change, not the public contract shape.
Change-source: pulse-maintainer
Install the default monitor's snapshot callback after both platform pollers exist. Settings already observes their runtime owners; alert checks must not retain startup's nil pointers. Exercise actual saved connections, loopback failures, ledger, monitor confirmation ticks, disable and ordinary scheduled inventory recovery without changing policy or adding a surface.
Contract-Neutral: Restore existing connection-alert wiring; no route, payload, authentication, persistence or detector-policy changes.
Change-source: pulse-maintainer
Saved TrueNAS and VMware connections belong to validated tenant storage keys, not optional organisation metadata. A malformed org.json could prevent provider creation and block authoritative disable or removal in an unrelated tenant. Enumerate storage identities while retaining strict metadata/account readers, tenant isolation and config/credential failure handling.
Synthetic probe, polling and API controls reproduce the interval-only response family in #2382. They do not establish either reporter installation’s cause or native appliance acceptance.
Change-source: pulse-maintainer
Update the legacy contract assertion exposed by the full API suite. A successful system-info-only probe remains successful but must not invent a runtime attempt, success or observed inventory. The real poll and failure-preservation controls remain unchanged.
Change-source: pulse-maintainer
The connected proof passed both probe/poll evidence cases but found one older fixture still expecting a successful stored-key probe to manufacture poll success. Keep its stored-secret, HTTPS and missing-ID controls while requiring untouched runtime timestamps and observations. Retain the adverse proof separately.
Change-source: pulse-maintainer
Preserve reviewed tip 888d918ba1 and startup-fetched upstream 8e14b4a63e unchanged in history. Resolve the allocation-test conflict by retaining the upstream boolean entry point with the reviewed bounded child launcher, strict measurement witness, escaping root sink and background-allocation control. No runtime or threshold changes; no stopped refresh replay.
Contract-Neutral: Test-only additive reconciliation of allocation-isolation repairs; runtime contracts unchanged.
Change-source: pulse-maintainer
Merge exact upstream 99927467f5. Resolve only the agent-lifecycle contract append conflict by retaining both monitor recovery ownership and AI usage export clauses. Frontend auto-merges preserve reviewed final bytes; no runtime conflict correction or browser receipt rebinding.
Change-source: pulse-maintainer
A saved connection test reads system.info on a separate client. Keep both probe results out of runtime timestamps, failures, retry cadence and inventory readiness. Preserve stored-secret and tenant boundaries, and validate the existing API evidence through real local client/poller/handler controls. Update the matching API, monitoring, lifecycle and recovery contracts together. This repairs the diagnostic gap in #2382; its native polling/session cause remains unverified.
Change-source: pulse-maintainer
TestNormalizeRoute_RootFastPathDoesNotAllocate failed in the api-2 shard
of PR 2445 with normalizeRoute("/") reporting 419 allocations per call,
though the fast path only returns a constant. testing.AllocsPerRun diffs
the process-wide malloc counter, so it also counts allocations made by
parallel tests and by goroutines earlier tests leave running whenever the
scheduler lets one in during the measurement. Under CPU contention with a
leaked allocating goroutine, the old checks failed in 30 of 30 runs.
Both zero-allocation tests (normalizeRoute's root fast path and
normalizeSegment) now re-run themselves in a child of the same test
binary, limited to that one test, where nothing else allocates. The
assertion is unchanged: removing the fast path still fails with
normalizeRoute("/") allocated 1 times per call. The same contended
stress passes 30 of 30, and -count=200 passes locally and on pulse-dev,
with and without -race.
AllocsPerRun observes process-wide allocations even with GOMAXPROCS=1. Run both zero-allocation checks in the same instrumented test binary with only the selected measurement enabled, bound execution to its marker and retain strict failure propagation. A synchronised background-allocation control demonstrates the old attribution error while the child checks still demand zero. Root results now escape as real metric labels do. Production normalisation and thresholds are unchanged.
Contract-Neutral: Test-process isolation and connected allocation controls only; no runtime, API or performance contract delta.
Change-source: pulse-maintainer
The required API shard failed because its healthy filesystem fixture omitted VM config. Supply a verified unlocked config instead of bypassing the guard, and cover backup/snapshot locks, absent or malformed config, permission refusals and locks appearing in flight through the real diagnostic client.
Contract-Neutral: Diagnostic fixtures and connected safety assertions only; production guest admission, APIs and polling behavior are unchanged.
Change-source: pulse-maintainer
The containing-source API suite passed its auth assertions but failed temporary-directory cleanup in the tenant token test. It unnecessarily started a complete mock monitoring runtime for a request-context check. Use the existing cached-monitor fixture pattern instead; retain global fallback and add real tenant-token and invalid-token controls. This changes no production authentication or monitor lifecycle behaviour.
Change-source: pulse-maintainer
rcourtman