Dead-code sweep. Functions flagged unreachable by golang.org/x/tools/cmd/deadcode
and confirmed unused across pulse, pulse-enterprise, pulse-pro and pulse-mobile by
adversarial cross-repo verification. Cross-module reachability was checked
explicitly (only pkg/ exported symbols are importable by other modules; internal/
packages and _test.go files are not). go build, go vet and test-compile all pass.
Root fix for the recurring class behind issue #1470. The served /install.sh and
/install.ps1 endpoints existed to hand out the unified AGENT installer, but their
GitHub fallback fetched the top-level install.sh release asset, which since
49412357a is the SERVER installer. Prior commits made that fallback unreachable
in normal deployments (deploy the sidecars; serve the local script even when
unsigned), but the endpoint was still structurally capable of serving the wrong
script in the no-local-bundle case.
The agent installer is a per-build artifact bundled into every release tarball and
Docker image, not a release asset, so the endpoint has no business proxying a
release asset at all. Remove proxyInstallScriptFromGitHub and its
installScriptReleaseAssetURL wrapper. handleDownloadInstallScriptCommon now serves
the locally bundled agent installer (signed when sidecars are present, unsigned
otherwise) or fails closed with 503 when no bundled script exists. Serving the
SERVER installer at this endpoint is now structurally impossible, not merely
unreachable.
The shared version-pinning (releaseAssetTag/releaseAssetURL) and installScriptClient
remain for the agent-BINARY download proxy, which legitimately fetches published
release assets; its version-pinning stays covered by the agentBinaryReleaseAssetURL
contract tests.
Replace the obsolete install-script proxy tests with fail-closed assertions
(including a guard that the endpoint makes no outbound call), drop the four
installScriptReleaseAssetURL contract tests, and revise the four subsystem
contracts that pinned the install-script fallback transport (api-contracts items
8 and 27, agent-lifecycle item 14, storage-recovery item 14, plus the
deployment-installability note) to state that install scripts are served local or
fail closed with no GitHub fallback.
The "Install on Linux/Windows" wizard does `curl -fsSL <server>/install.sh |
bash -s -- --url ...` and never verifies the response signature headers (curl|bash
discards them). But for published releases handleDownloadInstallScriptCommon
proxied the top-level GitHub install.sh asset whenever the local agent installer
lacked its .sig/.sshsig sidecars, and since 49412357a that asset is the SERVER
installer, which rejects --url. Every install missing the sidecars served the
wrong script. The companion deploy_agent_scripts fix deploys the sidecars for new
installs, but existing boxes stay broken until they redeploy.
Serve the locally bundled agent installer when its signatures are absent instead
of proxying. An unsigned-but-correct local script beats a signed-but-wrong proxied
one when nothing verifies the headers, and this retroactively fixes already-deployed
boxes the moment they get the new binary. The proxy now runs only when no local
installer is bundled at all, so the endpoint can no longer hand the agent wizard a
server installer in any reachable deployment state. New installs still ship the
sidecars and are served signed.
Revise the install-script signature/fallback contract this changes, across the
three subsystems that pin it (api-contracts item 8, agent-lifecycle item 14,
storage-recovery item 14) plus the deployment-installability note, to state that
the served endpoint serves the agent installer with correctness outranking
signature presence. Add a handler guard asserting a published-release server with
a present-but-unsigned local installer serves it locally and does not proxy.
Previously an alert that triggered Patrol ran a broad health check that
explicitly ignored the threshold breach. Now an alert carries its real
payload (metric type, value, threshold, identifier, level, message) into
the patrol scope, and the alert_fired run is framed around root-causing
that specific breach instead of a general assessment.
Three coordinated changes:
- Carry the alert payload into PatrolScope.AlertContext through the alert
bridge (PatrolTriggerEvent), so the patrol prompt sees the breach
specifics rather than just an alert-type string.
- Frame alert_fired patrol runs around the breach: replace the
"ignore threshold breaches" instruction with a root-cause directive
targeting the alert's metric and threshold.
- Add per-rule control via AIConfig.AlertTriggersInvestigation: a master
enable, a minimum-severity floor (patrol_alert_trigger_min_severity,
default critical-only), and an optional alert-type allowlist
(patrol_alert_trigger_types). The router's bridge callback consults the
policy and drops non-qualifying alert_fired events before queuing a
scoped patrol. A config-panel selector persists the severity floor.
Adds config, handler, and frontend proof tests, and updates the affected
subsystem contracts.
Forward-port of the release/5.1 fix (2c46c6c2d).
UpdateAlertConfig used to log SaveAlertConfig failures and still tell
the client "saved successfully", leaving the in-memory state with the
new override but the on-disk file untouched. On the next config reload
or process restart, the override silently vanished and the user saw
their threshold "revert" with no surfaced error. Return HTTP 500 with
the persistence error so the frontend can show a real save-failed
toast instead of false confidence.
Forward-port of the release/5.1 fix (9ac7df976). buildAlertsDiagnostic
previously emitted only cooldown/grouping flags, so triaging support
cases like #1341 where a user suspects an override key mismatch
required asking them to paste alerts config from inside their
container. Add an Overrides slice that names each persisted key with
its thresholds and disabled flags. Sanitize mode in the frontend
redacts the keys to override-N while keeping thresholds visible.
The four ReadDeadline(time.Now().Add(2 * time.Second)) calls in
router_integration_test.go (lines 1496, 1543, 1573, 1682) were
producing 'read tcp: i/o timeout' failures in CI under -race while
passing locally. The 2-second window is enough to read the welcome
+ initialState messages on a quiet dev workstation but too tight
once the runner is loaded with cumulative test work and the race
detector overhead. rc.5 cleared the same tests in CI but recent
fixture-size growth (k8s clusters 1->3 in 7938f28de plus the SMART
disk-temperature mock data added in 23ea4e487) pushed the
end-to-end server-start-to-welcome-message latency past the 2s
budget. Bumping to 15s gives CI breathing room without affecting
local test duration (the deadline only takes effect when the read
is genuinely stuck).
handleWorkloadCharts: add a 3s TTL response cache + per-key singleflight
so repeated sparkline polls from dashboards share work. TestSLO_WorkloadCharts
p95 drops from 188ms (over the 90ms target) to 838µs because most polls
hit cache or coalesce. Cold-call cost is unchanged; the win is dedup of
the steady-state polling pattern.
Cache + singleflight state live on the Router struct, not as globals, so
tenants don't cross-contaminate and tests don't pollute each other. Same
treatment for stateComputeGroup added in the previous commit.
monitor_metrics.mergeMetricHistory: drop the defensive cloneMetricPointMap
of base and cloneMetricSeries of candidate inside the merge. Every caller
already passes a freshly-owned map (from filterChartMetricMap,
GetAllGuestMetrics, GetAllStorageMetrics, queryStore*), so the internal
clones were pure waste. Caller-side cloneMetricPointMap is removed in
the three sites where the input is owned. ~6% allocation reduction on
BenchmarkHandleWorkloadCharts_StoreBacked.
Contract-neutral: no endpoint, response body, header, or wire-format
change.
Under load, 5x concurrent /api/state degraded from 276ms (single) to ~4s
each (linear), because every caller serialized on the monitor lock to
rebuild and JSON-encode the full 1.6MB state. /api/diagnostics had the
same dogpile shape on cache miss, even though its 45s TTL cache was
working as designed.
Wrap both handlers in a per-tenant singleflight.Group so concurrent
callers share the work: 20x concurrent /api/state now completes in 358ms
wall (~14-50x improvement). Diagnostics warm-cache responses are now
sub-3ms; cold compute coalesces.
Also drop websocket Upgrader buffers from 4MB read/write to 64KB. gorilla
streams larger payloads across the buffer transparently, so the 4MB
allocation per connection was overhead that scaled badly with concurrent
clients (100 clients * 8MB = 800MB just in buffers).
Contract-neutral: no endpoint, response body, header, or wire-format
change.
Closes the only API-coverage gap from the Docker / Kubernetes IA
maturity review: Roles, ClusterRoles, RoleBindings, and
ClusterRoleBindings now flow from the Kubernetes agent through the
canonical resource registry into the Kubernetes platform-page
Configuration tab.
Agent: pkg/agents/kubernetes/report.go gains four new report struct
types that carry summary counts plus subject-kind sets; individual
subject names and full PolicyRule contents are deliberately omitted
so Pulse stays a "what permissions exist where" surface, not an RBAC
enumeration tool. internal/kubernetesagent/agent.go gains four
collectors that call rbacv1.RoleList/ClusterRoleList/etc. through the
existing runKubernetesCallWithRetry wrapper, matching the
ServiceAccount collector's RBAC-forbidden retry pattern.
Canonical: internal/models mirrors with NormalizeCollections coverage;
convert* funcs in internal/monitoring/kubernetes_agents.go translate
agent report -> model; ResourceTypeK8sRole / K8sClusterRole /
K8sRoleBinding / K8sClusterRoleBinding join the canonical type set;
registry ingest* + adapter resourceFrom* functions emit one Resource
per RBAC object with ruleCount / roleKind / roleName / subjectCount /
subjectKinds / aggregationLabels on the K8s meta; search mapping in
internal/api/resources.go and the privacy allow-list in
internal/api/org_handlers.go pick up the four new type tokens; the
K8s privacy category in unifiedresources/policy_metadata.go classifies
them like the rest of K8s.
Frontend: ResourceType union + ResourceKubernetesMeta carry the new
kinds and RBAC summary fields; KubernetesPageSurface query asks for
them; the page model buckets them into the Configuration group;
KubernetesConfigTable renders Role / ClusterRole rule counts and the
aggregated flag, plus RoleBinding / ClusterRoleBinding role refs and
"N subjects · Kind1, Kind2 +overflow" subject summaries.
Curated demo seeds per-namespace Roles + RoleBindings plus an
aggregated ClusterRole + ClusterRoleBinding for pulse-demo-monitoring
in each cluster so the Configuration tab renders 18 RBAC rows across
the three demo clusters.
Contracts updated for the canonical-shape guard: monitoring,
api-contracts, unified-resources, frontend-primitives,
organization-settings (canonical) plus agent-lifecycle and
storage-recovery (dependent via Extension Points). Verification
proofs extended: kubernetes_registry_test.go, kubernetes_agents_test.go,
agent_inventory_test.go (new TestCollectRBACInventoryReportsSummaryCountsOnly
that pins the subject-name-omission contract), demo_scenarios_test.go,
adapter_coverage_test.go, contract_test.go, org_handlers_test.go,
resourceIdentity.test.ts, reportingResourceTypes.test.ts,
KubernetesConfigTable.test.tsx, and the
subsystem_lookup_test.py line-anchor bumps that the contract edits
shifted (api-contracts 246 -> 253, organization-settings 92 -> 93).
Verified:
- go build ./internal/... ./cmd/... clean
- go test ./internal/unifiedresources/..., ./internal/mock/...,
./internal/kubernetesagent/..., ./internal/api/...,
the K8s subset of ./internal/monitoring/... all clean (three
pre-existing unrelated monitoring failures noted earlier remain
unchanged by this commit)
- npm run type-check, lint:eslint, lint:theme,
lint:canonical-platforms clean
- vitest: 70 K8s frontend tests pass including the new RBAC render
coverage in KubernetesConfigTable.test.tsx
- browser proof on /kubernetes/configuration: 36 config rows
including 18 RBAC rows across three clusters; ClusterRole
"pulse-demo-monitoring" shows "12 rules · Aggregated";
ClusterRoleBinding shows "3 subjects · Group, ServiceAccount +1"
Collect native Kubernetes config, policy, and autoscaling objects.
Project the new resource types through API filters, unified resources, mock fixtures, and Kubernetes tabs.
Keep Secret inventory metadata-only and route k8s-secret policy as restricted local-only.
Project vCenter network inventory through canonical resources and add the vSphere Networks table backed by vCenter network topology. Align resource presentation coalescing so state and resource APIs share the same host contract.
Add a global resource timeline endpoint for provider activity and wire vSphere Activity to VMware timeline changes. Seed mock VMware activity through the same supplemental-change path and keep the relevant resource contract tests current.
clearSession invalidated the session-store record and zeroed both
session/CSRF cookies on the client side, but never deleted the
matching entry from the CSRF store. The orphaned record stuck around
for up to 4 hours (CSRF TTL) after every logout. Not exploitable —
the session itself was gone, so the stale CSRF entry could not be
used — but the asymmetry was real: InvalidateUserSessions (password
change) and InvalidateOldSessionFromRequest (re-login) both already
called DeleteCSRFToken, and logout was the missing path.
Add the same call in clearSession alongside InvalidateSession.
Regression test TestClearSession_DeletesServerSideCSRFToken sets up a
real session + CSRF token pair, calls clearSession with a request
carrying the session cookie, and asserts the CSRF token no longer
validates against the session ID afterward.
RequirePermission was reading the authenticated username from
w.Header().Get("X-Authenticated-User"). The header is set by checkAuth
upstream, but response headers are mutable across the handler chain:
any middleware sitting between checkAuth and RequirePermission that
wrote that header would substitute an arbitrary identity, and the
RBAC authorizer would make its access decision against the
substituted value. Identity belongs on the request context, not in a
mutable response surface.
Read internalauth.GetUser(r.Context()) first — checkAuth already
calls attachUserContext on every auth path that produces a real user,
so the context is the authoritative source. Keep a defensive,
warn-logged fallback to the response header for the one upstream path
that sets the header but skips attachUserContext (the anonymous-user
flow), so the existing test contract is preserved while the warning
flags the source path to plug.
Regression test TestRequirePermissionUsesContextUsername constructs
the attack: a request whose context says "real-authenticated-user"
and whose response header says "evil-spoofed-user". The RBAC
authorizer must observe the context value.
handleSAMLSLO was an unauthenticated force-logout DoS: any cross-origin
GET or POST to /api/saml/{id}/slo cleared the user's session and
returned a 302 to /?logout=success. No SAML payload was required, no
signature validation, no IdP-identity check.
Add ValidateLogoutResponse to SAMLService wrapping crewjam/saml's
ServiceProvider.ValidateLogoutResponseRequest, which validates the
XML-DSig against the configured IdP certificate and the standard
LogoutResponse temporal and target invariants.
In handleSAMLSLO:
- extract and validate the provider ID
- require either a SAMLResponse (response to our SLO request) or a
SAMLRequest (IdP-initiated SLO) parameter — reject bare requests
with 400 and do not clear session
- reject IdP-initiated LogoutRequest with 400: we don't track
in-flight request IDs to bind one to yet, and silently treating it
as a force-logout signal is the same bug we are closing
- on SAMLResponse, call service.ValidateLogoutResponse. On failure
return 403 and do not clear session; audit-log the failure
- only when validation passes do we clear the session and redirect
Regression tests:
- TestHandleSAMLSLO_RejectsEmptyPayload (400, no redirect, no session
state mutation)
- TestHandleSAMLSLO_RejectsInvalidLogoutResponse (403 on garbage
SAMLResponse value)
- TestHandleSAMLSLO_RejectsIDPInitiatedLogoutRequest (400 on
SAMLRequest-only payload)
The prior TestHandleSAMLSLO_Redirects test, which asserted the unsafe
"clear session and 302 on any GET" behaviour, is replaced by the
three above.
getCookieSettings auto-set SameSite=None on the session and CSRF
cookies whenever a request arrived via a trusted proxy with
X-Forwarded-Proto: https. The original intent was "be more permissive
for proxied deployments," but in practice that disabled the
browser-side CSRF defence for every Pulse deployment behind a reverse
proxy — which is nearly all of them.
SameSite=None tells the browser to attach the cookie on arbitrary
cross-site requests. That is only required for cross-origin iframe
embedding scenarios Pulse does not document or support. The
top-level-navigation cases that actually matter (OIDC/SAML callback
landing, Cloudflare-tunnel access from a bookmark) all work under Lax,
which still attaches cookies on top-level navigations and only blocks
cross-site sub-resource requests.
Always return SameSiteLaxMode. The Secure flag still tracks the
actual connection state via isConnectionSecure(r). Together with the
earlier CSRF bypass fix (require token regardless of Authorization
header), the browser-side and server-side defences are now both in
place.
Test expectations updated: three cases that previously asserted
SameSiteNoneMode for the proxied-HTTPS / Cloudflare-tunnel / direct-
TLS-with-Forwarded paths now assert SameSiteLaxMode, with comments
flagging them as regression coverage.
isHostedSubscriptionValid was treating any tenant with a non-nil
TrialEndsAt as a valid bounded trial, never comparing the timestamp to
the current time. A tenant whose trial ended in the past but whose
stored subscription state was still "trial" (e.g. lease refresh failed
or never ran) retained full hosted Cloud access indefinitely.
The validity rule now requires both that a trial end timestamp exists
AND that time.Now() is before it. Refactored the switch to make the
trial branch read explicitly: nil evaluator -> deny, nil end -> deny,
past end -> deny, otherwise allow.
Added TestTenantMiddleware_HostedMode_ExpiredTrial_Blocked as the
regression test (mirrors the existing BoundedTrial_Allowed +
UnboundedTrial_Blocked pair with a past-dated trial end).
CheckCSRF was skipping the CSRF check whenever the request carried
Authorization: Bearer, Authorization: Basic, or X-API-Token, without
validating the credential. An attacker on a cross-origin page could
fetch() any state-changing endpoint with credentials: 'include' plus an
arbitrary Authorization header — the browser would auto-attach the
victim's pulse_session cookie, the server would skip CSRF, and the
request would execute as the logged-in user. Full CSRF bypass for every
session-authenticated user.
CSRF protection exists because the browser auto-attaches the session
cookie. That cookie is the only auto-attached credential we issue, so
it is the only correct signal for whether CSRF applies. Header-based
auth is set explicitly per request and is not CSRF-vulnerable, but its
presence does not make a session-cookie-bearing request safe. Skip CSRF
only when no session cookie is present; otherwise require the token
regardless of any Authorization or X-API-Token header.
Tests: the three "header bypasses CSRF" cases in security_test.go were
passing only because they sent no session cookie (so the no-cookie path
returned true). Renamed those to make the no-cookie precondition
explicit. Added TestCheckCSRF_HeaderDoesNotBypassWhenSessionCookiePresent
in security_regression_test.go covering X-API-Token, Authorization:
Basic, Authorization: Bearer, and mixed-case Bearer with a session
cookie present — each must require a valid CSRF token.
Scope TrueNAS child source IDs under the system source key so duplicate pool, dataset, app, VM, share, and disk names from different appliances do not merge.
Use the resulting hierarchy for per-system TrueNAS overview counts and scoped mock metric targets.
