Commit graph

1522 commits

Author SHA1 Message Date
rcourtman
185e7098bd refactor: Remove duplicate isLoopback function from hostagent
The isLoopback function in internal/hostagent/agent.go was unused in
production code - it was a duplicate of the same function in
internal/hostmetrics/collector.go which is actively used.

Removed the dead code along with its associated tests to reduce
maintenance burden and improve code clarity.
2025-12-02 16:52:55 +00:00
rcourtman
c108cba8bb security: Add request body size limits to remaining API handlers
Add http.MaxBytesReader to 8 additional handlers to complete API
hardening against memory exhaustion attacks:

- guest_metadata.go: HandleUpdateMetadata (16KB)
- notification_queue.go: RetryDLQItem, DeleteDLQItem (8KB each)
- temperature_proxy.go: HandleRegister (8KB)
- host_agents.go: HandleReport (256KB)
- updates.go: HandleApplyUpdate (8KB)
- docker_metadata.go: HandleUpdateMetadata (16KB)
- system_settings.go: UpdateSystemSettings (64KB)

All API handlers that decode JSON request bodies now have size limits.
2025-12-02 16:47:13 +00:00
rcourtman
7b55564b62 security: Add request body size limits to API handlers
Add http.MaxBytesReader to 16 additional handlers to prevent memory
exhaustion attacks via oversized request bodies:

- docker_agents.go: HandleReport (512KB), HandleCommandAck (8KB),
  HandleSetCustomDisplayName (8KB)
- alerts.go: UpdateAlertConfig (64KB), BulkAcknowledgeAlerts (32KB),
  BulkClearAlerts (32KB)
- config_handlers.go: HandleAddNode, HandleTestConnection,
  HandleUpdateNode, HandleTestNodeConfig (32KB each),
  HandleVerifyTemperatureSSH, HandleExportConfig, HandleDiscoverServers,
  HandleSetupScriptURL (8KB each), HandleImportConfig (1MB),
  HandleUpdateMockMode (16KB)
2025-12-02 16:43:13 +00:00
rcourtman
ccc26624e8 security: Add request body size limits to notification handlers
Add http.MaxBytesReader limits to prevent memory exhaustion attacks:
- UpdateEmailConfig: 32KB limit
- UpdateAppriseConfig: 64KB limit
- CreateWebhook: 64KB limit
- UpdateWebhook: 64KB limit

This follows the pattern already used in system_settings.go for
SSH config validation.
2025-12-02 16:37:30 +00:00
rcourtman
0bcaa41499 test: Fix unreachable code warning in panic recovery test
Refactor the test to avoid unreachable code after panic by
checking a flag set before the panic instead of after. This
resolves the go vet warning while maintaining test coverage.
2025-12-02 16:30:31 +00:00
rcourtman
2bcdeea537 refactor: Use zerolog instead of fmt.Printf in config export
Replace raw fmt.Printf calls with structured zerolog logging for
consistency with the rest of the codebase. This improves log
formatting and enables proper log level filtering.
2025-12-02 16:27:54 +00:00
rcourtman
12f133f6f8 test: Add tests for SyncGuestBackupTimes and UpdateStorageBackupsForInstance
Add tests for the remaining uncovered State methods:
- SyncGuestBackupTimes: Tests backup time sync from storage and PBS backups
- UpdateStorageBackupsForInstance: Tests storage backup updates with instance isolation

Improves internal/models coverage from 89.6% to 95.6%.
2025-12-02 16:22:38 +00:00
rcourtman
b5a5fddaa3 test: Add tests for Ceph, backup, replication, and snapshot methods
Add tests for previously uncovered State methods:
- UpdateCephClustersForInstance
- UpdateBackupTasksForInstance
- UpdateReplicationJobsForInstance
- UpdateGuestSnapshotsForInstance

Improves internal/models coverage from 80.4% to 89.6%.
2025-12-02 16:20:11 +00:00
rcourtman
cbbb790284 test: Add more State method tests for coverage
Add tests for previously uncovered State methods:
- UpdatePhysicalDisks
- UpdateStorageForInstance
- UpdatePBSInstances / UpdatePBSInstance
- UpdatePMGInstances / UpdatePMGInstance

Improves internal/models coverage from 71.9% to 80.4%.
2025-12-02 16:17:04 +00:00
rcourtman
5b7e7a3133 test: Add tests for state connection health and backup methods
Add tests for previously uncovered State methods:
- SetConnectionHealth / RemoveConnectionHealth
- UpdatePBSBackups
- UpdatePMGBackups

Improves internal/models coverage from 67.0% to 71.9%.
2025-12-02 16:14:40 +00:00
rcourtman
36740bffe3 docs: Add godoc comments to more exported functions
Add missing godoc comments to:
- BuildGuestKey in alerts/alerts.go
- GenerateMockData in mock/generator.go
- NewDockerUpdater, NewAURUpdater in updates/adapter_installsh.go
- NewMockUpdater in updates/mock_updater.go
2025-12-02 16:03:57 +00:00
rcourtman
c019e4cd0e docs: Add godoc comments to exported functions
Add missing godoc comments to:
- NewRateLimiter and Allow in ratelimit.go
- SnapshotSyncStatus in temperature_proxy.go
- NewClient and GetVersion in pkg/pmg/client.go
2025-12-02 15:58:59 +00:00
rcourtman
25294da5d5 perf: Cache hostname lowercase in temperature proxy lookups
Pre-compute strings.ToLower(hostname) before loops that search for
matching PVE instances. Avoids repeated lowercasing in two functions.
2025-12-02 15:43:41 +00:00
rcourtman
6931fa34cd perf: Cache err.Error() in cluster node validation
Cache err.Error() result in two locations in config_handlers.go:
- TLS mismatch detection (3x calls to 1)
- Standalone node detection (2x calls to 1)
2025-12-02 15:41:18 +00:00
rcourtman
6a29e20b5e perf: Cache err.Error() in storage timeout error handling
Cache err.Error() result in two locations:
- monitor.go: storage query retry logic (2x calls to 1)
- monitor_polling.go: storage timeout handling (2x calls to 1)
2025-12-02 15:39:37 +00:00
rcourtman
5d2eca3660 perf: Cache err.Error() in disk monitoring error handling
Compute err.Error() once and reuse errStr instead of calling Error()
four times when checking disk monitoring error types.
2025-12-02 15:37:48 +00:00
rcourtman
9fc567dc2e perf: Cache lowercase RAID state in alerts processing
Compute strings.ToLower(array.State) once and reuse stateLower instead
of calling ToLower three times when checking RAID array status.
2025-12-02 15:35:32 +00:00
rcourtman
128dccf270 chore: Remove debug logging that exposed config JSON
Removed two DEBUG log statements that were logging full nodes config
JSON at Info level. This was verbose and potentially exposed sensitive
configuration data (credentials, tokens) in logs.
2025-12-02 15:32:02 +00:00
rcourtman
8f5b258424 perf: Cache lowercase error string in guest agent error handling
Compute strings.ToLower(errStr) once and reuse errStrLower instead of
calling ToLower three times on the same string in permission check.
2025-12-02 15:26:40 +00:00
rcourtman
f383cf8ddf perf: Remove redundant fmt.Sprintf in tempproxy contains function
The fmt.Sprintf("%v", s) call was pointless since s is already a string.
Removing this unnecessary conversion.
2025-12-02 15:24:40 +00:00
rcourtman
b47a04aa97 perf: Use strconv.Itoa instead of fmt.Sprintf for int conversion
strconv.Itoa is faster than fmt.Sprintf("%d", ...) because it doesn't
need to parse a format string. Changed 4 occurrences in monitoring
package where integers are converted to strings.
2025-12-02 15:21:41 +00:00
rcourtman
a965285738 perf: Pre-compile regexes in updates/version package
Move regex compilation from function bodies to package-level variables
to avoid recompilation when parsing version strings.

Affected regexes:
- semverRe: Matches semantic version format (X.Y.Z-prerelease+build)
- rcNumRe: Extracts RC number from prerelease strings

These are called multiple times during version comparison and update checks.
2025-12-02 15:14:15 +00:00
rcourtman
fd3c57377c perf: Pre-compile regexes in mdadm package
Move regex compilation from function bodies to package-level variables
to avoid recompilation on each RAID array check during polling cycles.

Affected regexes:
- mdDeviceRe: Matches md device names in /proc/mdstat
- slotRe: Matches device slot lines in mdadm --detail output
- speedRe: Extracts rebuild speed from /proc/mdstat
2025-12-02 15:11:04 +00:00
rcourtman
ac4e48fa4a fix: Properly show disabled storage status from Proxmox
Storage that is disabled in Proxmox (Datacenter > Storage > enabled=no)
was incorrectly showing as "available" in Pulse. The issue was that
Enabled/Active fields defaulted to true and were never set to false
from the per-node API response.

Now the model correctly initializes Enabled/Active from the Proxmox
per-node storage API response, and the status determination prioritizes
checking the disabled state first.

Related to #796
2025-12-02 15:03:40 +00:00
rcourtman
be23624592 refactor: Remove unreachable dead code branches
- parseDetail: Remove len(parts) != 2 check after SplitN(line, ":", 2)
  when we already verified line contains ":" (SplitN always returns 2)

Function improved from 96.4% to 98.1% coverage.
2025-12-02 15:00:26 +00:00
rcourtman
a0a473ada8 refactor: Remove unreachable dead code branches
- checkOrigin: Remove redundant same-origin check at line 155 that was
  already handled at line 116 (origin == requestOrigin)

Function improved from 95.1% to 97.4% coverage.
2025-12-02 14:56:35 +00:00
rcourtman
a647ffb219 refactor: Remove unreachable dead code branches
- Host.ToFrontend: Remove redundant h.DisplayName check after struct
  initialization already set host.DisplayName = h.DisplayName

Function now at 100% coverage.
2025-12-02 14:53:07 +00:00
rcourtman
c1d98e6aa8 refactor: Remove unreachable dead code branches
- errors.isRetryable: Convert final case to default (all ErrorType values covered)
- scheduler.SelectInterval: Remove bounds checks after mathematical computation
  that guarantees target ∈ [min, max]

Both functions now at 100% coverage.
2025-12-02 14:48:57 +00:00
rcourtman
2453198f62 refactor: Remove unreachable dead code branches
- firstForwardedValue: strings.Split always returns at least one element
- shouldRunBackupPoll: remaining is always >= 1 by math
- convertContainerDiskInfo: lowerLabel is never empty for non-rootfs

All three functions now at 100% coverage.
2025-12-02 14:41:53 +00:00
rcourtman
3d9c4f6da1 test: Add UpdateStatus not found test for notifications package 2025-12-02 14:26:17 +00:00
rcourtman
3f8455dfa9 test: Add DispatchDue enqueue error test for monitoring package 2025-12-02 14:21:25 +00:00
rcourtman
a5f263cf79 test: Add NewNotificationQueue invalid path test for notifications package 2025-12-02 14:17:05 +00:00
rcourtman
eb302e6d4e test: Add SessionStore.load legacy format tests for API package 2025-12-02 14:12:32 +00:00
rcourtman
e7fa996e03 test: Add CSRFTokenStore.load format tests for API package
Cover legacy JSON format migration and current format with nil/expired
entries. Improves load function coverage from 67.9% to 100%.
2025-12-02 14:07:00 +00:00
rcourtman
daafb51493 test: Add CheckAuth tests for API package
Add tests for sliding expiration session validation and no-auth
configured scenarios. These test explicit paths for better coverage
documentation even though they were already exercised indirectly.
2025-12-02 13:57:23 +00:00
rcourtman
2f203bf2d9 test: Add CheckCSRF valid token test for 100% coverage
Test the success path where a valid CSRF token is provided with a
matching session. This covers the final branch in CheckCSRF.
2025-12-02 13:51:27 +00:00
rcourtman
e1ff07fd9f test: Add singleAlertTemplate type coverage tests
Cover io type (formats as "I/O") and custom type (uses titleCase)
branches that were previously untested in the email template.
2025-12-02 13:45:49 +00:00
rcourtman
4797e8d76a test: Add ValidateWebhookURL edge case tests for notifications package
Cover empty URL, invalid scheme, missing hostname, cloud metadata
endpoints, loopback variants, and IPv6 link-local addresses.
2025-12-02 13:41:34 +00:00
rcourtman
50769e12ea test: Add HandleSetupScript PBS path tests for API package
Cover the PBS script generation branch that was previously untested.
Verifies PBS-specific content, auth token handling, and placeholder host.
2025-12-02 13:36:23 +00:00
rcourtman
28533975f6 test: Add CheckAuth tests for API package
Cover proxy auth headers, OIDC session validation, and session cookie
paths that were previously untested.
2025-12-02 13:32:21 +00:00
rcourtman
d058b2ab96 test: Add CheckProxyAuth tests for API package
Add comprehensive direct tests for the CheckProxyAuth function covering:
- Not configured (returns false)
- Invalid secret (returns false)
- Missing secret header (returns false)
- Valid secret without user header configured (returns true, admin)
- Missing user header when configured (returns false)
- Valid auth with username (returns true with username)
- Role checking with empty roles header (defaults to admin)
- Role checking with admin role present (returns admin=true)
- Role checking without admin role (returns admin=false)
- Custom role separator (comma instead of pipe)
- Role with whitespace (trimmed correctly)

Coverage: CheckProxyAuth 89.3% → 100%
2025-12-02 13:28:02 +00:00
rcourtman
d219d9965d test: Add ValidateSession tests for API package
Add comprehensive tests for the ValidateSession wrapper function covering:
- Non-existent token (returns false)
- Empty token (returns false)
- Valid token (returns true)
- Expired token (returns false)

The ValidateSession function is a simple wrapper around the SessionStore's
ValidateSession method, but having direct tests ensures the wrapper is
exercised and documents its expected behavior.

Coverage: ValidateSession 0% → 100%
2025-12-02 13:22:45 +00:00
rcourtman
ef6a370144 test: Add ensureScope tests for API package
Add comprehensive tests for the ensureScope function covering:
- Empty scope parameter (always allows access)
- No token in context (session-based request, allows access)
- Token with matching scope (allows access)
- Token with multiple scopes including required one (allows access)
- Token missing required scope (rejects with 403)
- Token with empty scopes (defaults to wildcard, allows access)
- Rejection returns proper JSON response format

Coverage: ensureScope 0% → 100%
Coverage: API package 32.1% → 32.2%
2025-12-02 13:19:11 +00:00
rcourtman
301338d800 test: Add sendResolvedEmail tests for notifications package
Add comprehensive tests for the sendResolvedEmail function covering:
- Empty alert list (returns error)
- Nil alert list (returns error)
- All nil alerts (returns error from content builder)
- Single alert (exercises email sending path)
- Multiple alerts (tests grouped notification)
- Mixed nil and valid alerts (filters correctly)
- Zero resolved time (handles gracefully)

Also improves buildResolvedNotificationContent coverage as a collateral
benefit since sendResolvedEmail calls it internally.

Coverage: sendResolvedEmail 0% → 100%
Coverage: buildResolvedNotificationContent → 100%
Coverage: notifications package 58.3% → 58.6%
2025-12-02 13:14:22 +00:00
rcourtman
d970055532 test: Add RequireAuth tests for API package
Add comprehensive tests for the RequireAuth middleware covering:
- No auth configured (allows access by design)
- API-only mode (rejects requests without token)
- API-only mode (accepts valid X-API-Token)
- Basic auth with invalid credentials
- Basic auth JSON vs plain text error responses
- Valid basic auth (allowed)
- Proxy auth (allowed)
- Proxy auth with invalid secret (rejected)
- Bearer token with basic auth configured (allowed)
- Invalid Bearer token (rejected)

Coverage: RequireAuth 7.1% → 78.6%
Coverage: CheckAuth 66.9% → 69.1%
Coverage: API package 31.9% → 32.1%
2025-12-02 13:09:48 +00:00
rcourtman
1073c511b3 test: Add RequireAdmin tests for API package
Add comprehensive tests for the RequireAdmin middleware covering:
- No auth configured (allows access by design)
- API-only mode (rejects requests without token)
- Basic auth with invalid credentials
- Proxy auth with admin role (allowed)
- Proxy auth with non-admin role (forbidden)
- Proxy auth with invalid secret (unauthorized)
- Proxy auth without role header (defaults to admin)
- Proxy auth with custom role separator
- Proxy auth with spaces in roles (trimmed)
- Basic auth authenticated users (allowed as admin)
- JSON vs plain text error responses based on path/Accept header

Also improves CheckProxyAuth coverage as a side effect.

Coverage: RequireAdmin 20.8% → 87.5%
Coverage: CheckProxyAuth 0.0% → 89.3%
Coverage: API package 30.9% → 31.9%
2025-12-02 13:06:06 +00:00
rcourtman
71e15886ed test: Add isRequestAuthenticated tests for API package
Add comprehensive tests for the isRequestAuthenticated function covering:
- Nil inputs (config, request, both)
- Basic auth (valid, invalid password, invalid username, malformed base64, missing colon)
- API token via X-API-Token header
- API token via Bearer authorization header (case insensitive)
- Invalid/empty/whitespace API tokens
- No auth configured scenarios
- Empty session cookie handling

Coverage: isRequestAuthenticated 26.1% → 82.6%
Coverage: API package 30.7% → 30.9%
2025-12-02 12:59:18 +00:00
rcourtman
cf6149e884 test: Add CheckCSRF tests for API package
Add comprehensive tests for the CheckCSRF function covering:
- Safe methods (GET, HEAD, OPTIONS) bypass
- API token authentication bypass
- Basic auth bypass
- No session cookie handling
- Missing CSRF token rejection with new token issuance
- Invalid CSRF token rejection with new token issuance
- CSRF token from FormValue
- Unsafe methods (POST, PUT, DELETE, PATCH) enforcement

Coverage: CheckCSRF 32.0% → 96.0%
Coverage: API package 30.5% → 30.7%
2025-12-02 12:53:32 +00:00
rcourtman
5deca850d4 test: Add saveHistoryWithRetry tests for alerts package
Add comprehensive tests for the saveHistoryWithRetry function covering:
- Backup file creation from existing history
- Empty history serialization
- Single retry success
- Read-only directory failure with retries
- Concurrent saves with serialization via saveMu
- Snapshot isolation during save

Coverage: saveHistoryWithRetry 58.6% → 86.2%
Coverage: alerts package 87.4% → 87.8%
2025-12-02 12:49:27 +00:00
rcourtman
cd33c577f8 test: Add resolvePublicURL tests for API package
Add comprehensive tests for the resolvePublicURL function covering:
- Configured PublicURL (simple, trailing slashes, ports, whitespace)
- Request-derived URL (HTTP, HTTPS via TLS, X-Forwarded-Proto)
- No host fallback (with/without frontend port)
- Nil request handling

Coverage: resolvePublicURL 12.5% → 100%
Coverage: API package 30.3% → 30.5%
2025-12-02 12:45:04 +00:00