fix snmp handling to be more like librenms (#190)

Reviewed-on: graham/towerops-web#190
This commit is contained in:
Graham McIntire 2026-03-27 13:37:27 -05:00 committed by graham
parent 3f08fd42b6
commit c7a95164e3
6 changed files with 789 additions and 863 deletions

View file

@ -1,779 +0,0 @@
# TowerOps Bug & Code Issue Audit Report
**Generated**: 2026-03-26
**Scope**: Comprehensive codebase search for bugs, anti-patterns, and security issues
**Status**: **SUBSTANTIALLY COMPLETE** - Critical, High, and most Medium priority items fixed or verified safe
---
## Executive Summary
**Total Issues Found**: 114 distinct issues across 8 categories
**Status**: **SUBSTANTIALLY COMPLETE** - All critical/high priority items fixed or verified safe
**Completed** (2026-03-27):
- ✅ All Critical security vulnerabilities fixed (PagerDuty webhook auth, Stripe DoS, HMAC disclosure)
- ✅ All Urgent SNMP crash bugs fixed (malformed OID indices, binary UUID serialization)
- ✅ All bang operations in loops replaced with safe error handling
- ✅ **ALL Repo queries eliminated from LiveViews** (0 instances - moved to context modules)
- ✅ Large dataset loading converted to use Repo.stream for memory efficiency (4/4 workers)
- ✅ Disabled event_logger tests fixed and re-enabled (4 tests now passing)
- ✅ N+1 queries verified already optimized with proper preloads
- ✅ Database transactions verified atomic (Repo.update_all + transaction wrapping)
- ✅ **Database indexes added** for array operations and functional queries (3 new indexes)
- ✅ CSRF protection verified not applicable (token-based auth)
- ✅ All JS hooks have phx-update="ignore" (22 instances across 12 files)
- ✅ SNMP silent failures fixed (logging added for interface fetch errors, malformed OIDs)
- ✅ Binary sanitization verified working (invalid UTF-8 converted to hex)
- ✅ Race conditions verified safe (insert! in transactions)
- ✅ Timeout configuration verified adequate (30s default, 50s for slow checks)
| Category | Critical | High | Medium | Low | Total |
|----------|----------|------|--------|-----|-------|
| Error Handling | 13 | 8 | 0 | 0 | 21 |
| Logic Bugs | 16 | 0 | 0 | 0 | 16 |
| Phoenix/LiveView | 17 | 1 | 8 | 0 | 26 |
| Database/Ecto | 2 | 4 | 7 | 0 | 13 |
| Testing | 4 | 8 | 7 | 0 | 19 |
| SNMP-Specific | 2 | 7 | 1 | 0 | 10 |
| Security | 3 | 2 | 1 | 0 | 6 |
| Code Quality | 0 | 0 | 0 | 3 | 3 |
| **TOTALS** | **57** | **30** | **24** | **3** | **114** |
---
## 1. Error Handling Issues (21 Critical Issues)
### 1.1 Silently Discarded Errors (Medium Priority)
**Pattern**: `{:error, _} -> false/nil` - Errors caught but discarded without logging
1. **host_parser.ex:388** - ✅ **NOT A BUG** Validation function correctly returns boolean
```elixir
# valid?/1 is a predicate - returning false for invalid input is correct
```
2. **mib.ex:366** - ✅ **NOT A BUG** Error properly propagated in batch operation
```elixir
# Enum.find locates first error, then propagates it on line 369
```
3. **profiles.ex:104** - ✅ **FIXED** Regex compilation failures now logged at warning level
```elixir
# AFTER: Logs invalid regex patterns with profile name
Logger.warning("Invalid regex pattern in profile '#{profile.name}': #{pattern}...")
```
4. **base.ex:1612** - ✅ **FIXED** SNMP optional field errors now logged at debug level
```elixir
# AFTER: Logs error reason before returning nil for optional fields
Logger.debug("Optional field fetch failed for OID #{oid}: #{inspect(reason)}")
```
5. **application_setting.ex:72** - ✅ **FIXED** JSON/decimal parse errors now logged at warning level
```elixir
# AFTER: Logs parse failures with setting key and error details
Logger.warning("Failed to parse JSON value for setting '#{key}': #{inspect(reason)}")
```
6. **identifier.ex:22,34,46** - ✅ **FIXED** Gleam normalization errors now logged at debug level
```elixir
# AFTER: Logs normalization failures for MAC, IP, and name fields
Logger.debug("Failed to normalize MAC '#{mac}': #{inspect(reason)}")
```
### 1.2 Unhandled Oban Job Insertion Failures (Critical)
**Jobs that fail to enqueue will never execute, causing missed work**
7. **report_worker.ex:29** - ✅ **FIXED** Report job enqueue failures now logged
```elixir
# AFTER: Logs error with changeset details
```
8. **cn_maestro_sync_worker.ex:31** - ✅ **FIXED** Sync job enqueue failures now logged
9. **escalation.ex:180** - ✅ **FIXED** Escalation check job insertion now handled with error logging
10. **discovery_worker.ex:246** - ✅ **ALREADY HANDLED** Discovery job insertion has proper error handling
11. **alert_digest_worker.ex:37,72** - ✅ **FIXED** Digest job insertion now logs errors in loop and standalone function
12. **weather_sync_worker.ex:68** - ✅ **FIXED** Next run scheduling failure now logged
### 1.3 Unguarded Bang Operations (Critical) - ✅ **FIXED**
**`Repo.insert!()` and `Repo.update!()` can crash entire processes**
13. **topology.ex:316** - ✅ **ALREADY FIXED** - Uses safe `insert_link_evidence/2` helper with error logging
14. **preseem/fleet_intelligence.ex:102,107** - ✅ **FIXED** - Replaced with safe error handling, logs failures
```elixir
# AFTER: Uses Repo.insert/update with case handling, logs errors but continues
```
15. **snmp/discovery.ex** (11 locations) - ✅ **INTENTIONALLY CORRECT** - Bang operations inside `Repo.transaction` block
- Lines 898, 918, 971, 977, 1024, 1030, 1068, 1074, 1217, 1269, 1275
- **Rationale**: Transaction context ensures atomic rollback on failure (correct pattern)
16. **accounts.ex:445** - ✅ **FIXED** - TOTP timestamp update uses safe error handling
```elixir
# AFTER: Logs errors but doesn't block login on timestamp update failure
```
### 1.4 Enum.each with Unhandled Errors (High Priority)
**Errors in enumeration callbacks are silently swallowed**
17. **gaiia/site_aggregation.ex:39,52** - ✅ **FIXED** Site total updates now have error handling with logging
```elixir
# AFTER: Wraps update_site_totals in case statement, logs changeset errors
```
18. **preseem_baseline_worker.ex:28,31,34** - ✅ **NOT A BUG** Rescue block provides adequate error handling
```elixir
# Pattern matching is safe - functions always return {:ok, _}
# Rescue block on lines 36-38 catches and logs any exceptions
```
19. **system_insight_worker.ex:25-31** - ✅ **FIXED** Insight generation now logs errors
```elixir
# AFTER: Wraps generate_agent_offline_insight in case statement, logs failures
```
20. **device_poller_worker.ex:216** - ✅ **ALREADY FIXED** Task results have proper error handling
```elixir
# Lines 210-217: Logs task count mismatches
# Lines 222-229: Handles {:ok, _}, {:exit, reason}, nil cases
```
21. **snmp.ex:807** - ✅ **FIXED** Neighbor upsert now logs errors before transaction rollback
```elixir
# AFTER: Wraps upsert_neighbor in case statement, logs changeset errors
```
### 1.5 Unhandled HTTP Requests (Medium Priority)
22. **release_checker.ex:141** - ✅ **NOT A BUG** Callers properly handle {:ok, _} and {:error, _} results
```elixir
# Lines 81 and 123: Both callers use case statements with full error handling
```
---
## 2. Logic Bugs & Edge Cases (16 Critical Issues)
### 2.1 Empty List Operations Without Guards
**Using `hd()`, `List.first()`, `List.last()` on potentially empty lists**
1. **dashboard.ex:134, 281** - ✅ **FIXED** Replaced `hd()` with pattern matching
```elixir
# AFTER: Uses pattern matching [device | _] with empty list guard clause
# Line 134: Pattern match in Enum.map with defensive empty list clause
# Line 281: Added empty list guard clause to calculate_down_impact
```
2. **gaiia.ex:382** - ✅ **ALREADY FIXED** No `hd()` calls present in current code
```elixir
# Code has been refactored, no longer uses hd()
```
3. **alerts/storm_detector.ex:270** - ✅ **FIXED** Added nil check with case statement
```elixir
# AFTER: case List.first(devices) with proper nil handling
```
4. **snmp.ex** (6 locations) - ✅ **NOT A BUG** List.first() correctly returns nil for optional fields
- Lines 1173, 1179: Optional fields (hostname, platform) - nil is acceptable
- Lines 1191, 1194, 1197: Guarded by non-empty checks (`entry.macs != []`)
```elixir
hostname: List.first(entry.hostnames), # Returns nil for empty list (intended)
platform: List.first(entry.platforms), # Returns nil for empty list (intended)
```
### 2.2 Unsafe Regex and String Operations
5. **snmp/profiles/base.ex:1771** - ✅ **NOT A BUG** extract_model_with_number handles nil safely
```elixir
# extract_model_with_number/2 has case statement: nil -> prefix
```
6. **snmp/profiles/base.ex:1805** - ✅ **NOT A BUG** extract_first_match handles nil/empty safely
```elixir
# extract_first_match/2 pattern matches [full_match | _] or returns default
```
7. **snmp/profiles/vendors/allied_telesis.ex:40-42** - ✅ **NOT A BUG** Uses safe pattern matching
```elixir
# Pattern matches [_full, capture | _] only when capture group exists
```
### 2.3 Division by Zero Risks
8. **capacity.ex:214-215** - ✅ **ALREADY FIXED** Empty list guarded on line 210
```elixir
def percentile([], _n), do: 0.0 # Already handles empty case
```
9. **preseem.ex:118, 123** - ✅ **ALREADY FIXED** Empty list checks on lines 116 and 121
```elixir
if qoe_scores == [], do: nil, else: Float.round(...) # Already safe
```
10. **preseem/fleet_intelligence.ex:73** - ✅ **ALREADY FIXED** Empty list guarded on line 72
```elixir
defp safe_avg([]), do: nil # Already handles empty case
```
### 2.4 Index/Bounds Errors
11. **topology.ex:860-861** - ✅ **NOT A BUG** Enum.at returns nil for optional interface fields (intended)
```elixir
# nil is acceptable for optional source_interface and target_interface fields
```
12. **snmp/profiles/vendors/routeros.ex:916-918** - ✅ **NOT A BUG** Pattern match handles short lists
```elixir
# Case statement has _ -> {0, "0"} fallback for lists with < 2 elements
```
### 2.5 Pattern Matching Failures
13. **snmp/neighbor_discovery.ex:143** - ✅ **FIXED** Added case statement with fallback
```elixir
# AFTER: Uses case statement, logs warning, falls back to full OID as key
```
14. **monitoring/executors/ssl_executor.ex:105-106** - ✅ **FIXED** Added length validation
```elixir
# AFTER: Validates byte_size before pattern matching, returns {:error, _} for malformed times
# Also updated check_certificate to handle error tuples
```
### 2.6 Nil Field Access
15. **snmp/profiles/dynamic.ex:300, 314** - ✅ **NOT A BUG** Empty string is acceptable index
```elixir
# List.last on String.split returns "" for empty OID (acceptable fallback)
```
16. **on_call/escalation.ex:192** - ✅ **NOT A BUG** Guard prevents nil access
```elixir
# if rules == [], do: 0 guard ensures else branch never called with empty list
```
---
## 3. Phoenix/LiveView Issues (26 Issues)
### 3.1 Missing phx-update="ignore" on JS Hooks (Critical - 17 instances) - ✅ **ALL FIXED**
**All JS hooks that manage their own DOM MUST have `phx-update="ignore"` to prevent LiveView from overwriting hook-managed DOM**
1. **device_live/index.html.heex:251** - DeviceListReorder hook
```heex
<div id="device-list" phx-hook="DeviceListReorder">
<!-- Missing: phx-update="ignore" -->
```
2. **device_live/form.html.heex:5** - ScrollToTop hook
3. **device_live/form.html.heex:647** - MikrotikPortSync hook
4. **agent_live/index.html.heex:440, 492** - CopyToClipboard hooks (2 instances)
5. **org/integrations_live.html.heex:397, 421** - CopyToClipboard hooks (2 instances)
6. **site_live/show.html.heex:28** - LeafletMap hook
7. **site_live/show.html.heex:553** - SensorChart hook
8. **weathermap_live.html.heex:350** - WeathermapViewer hook
9. **user_settings_live.html.heex:412** - ThemeSelector hook
10. **user_settings_live.html.heex:1570, 1847** - CopyToClipboard hooks (2 instances)
11. **map_live/index.html.heex:47** - SitesMap hook
12. **network_map_live.html.heex:283** - NetworkMap hook
13. **dashboard_live.html.heex:6** - DynamicFavicon hook
14. **graph_live/show.html.heex:69** - SensorChart hook
**Fixed**: 2026-03-26 - Added phx-update="ignore" to all 11 hooks that were missing it (6 were already fixed)
### 3.2 Repo Queries in LiveViews (Medium Priority) - ✅ **ALL FIXED**
**Violates AGENTS.md architecture - queries should be in context modules**
**Status**: Complete - All Repo operations moved to context modules (0 instances remaining)
15. **preseem_devices_live.ex:148** - ✅ **FIXED** Moved to Preseem.list_access_points/1
```elixir
# BEFORE: access_points = Repo.preload(access_points, :device)
# AFTER: Preseem.list_*_access_points now include |> preload(:device)
```
16. **preseem_insights_live.ex:136** - ✅ **FIXED** Moved to Preseem.list_insights/2
```elixir
# BEFORE: |> Repo.preload([:preseem_access_point, :device])
# AFTER: Preseem.list_insights now includes preload([:preseem_access_point, :device])
```
17. **gaiia_mapping_live.ex:204** - ✅ **FIXED** Removed redundant preload
```elixir
# BEFORE: |> Repo.preload(:site)
# AFTER: Removed (Devices.list_organization_devices already preloads :site)
```
18. **settings_live.ex:160** - ✅ **FIXED** Moved to Organizations.create_invitation/1
```elixir
# BEFORE: |> Towerops.Repo.preload(:invited_by)
# AFTER: Organizations.create_invitation now preloads :invited_by
```
19. **settings_live.ex:182** - ✅ **FIXED** Replaced with Organizations.get_organization_invitation/2
```elixir
# BEFORE: invitation = Towerops.Repo.get!(Towerops.Organizations.Invitation, id)
# AFTER: case Organizations.get_organization_invitation(id, org_id) do
```
20. **device_live/show.ex:647** - ✅ **FIXED** Removed direct Repo.all from LiveView
21. **admin/audit_live/index.ex:333, 338** - ✅ **FIXED** Removed direct Repo.all from LiveView
22. **config_timeline_live.ex** - ✅ **FIXED** Removed all direct Repo.all calls from LiveView
- No longer using Repo.all in LiveView (was lines 254, 269, 286, 297)
---
## 4. Database/Ecto Issues (13 Issues)
### 4.1 Binary UUID Serialization Bug (Critical)
1. **activity_feed.ex:304,306** - ✅ **FIXED** Added ::text casts to array_agg
```elixir
# BEFORE: fragment("(array_agg(? ORDER BY ? DESC))[1]", sl.status, sl.inserted_at)
# AFTER: fragment("(array_agg(?::text ORDER BY ? DESC))[1]", sl.status, sl.inserted_at)
```
**Fixed**: 2026-03-26 - Prevents Jason.EncodeError crashes on LiveView socket
### 4.2 Unsafe Repo Operations (Critical - 39 instances)
**All `Repo.get!`, `Repo.get_by!`, `Repo.one!` calls crash with 500 error if record not found**
2. **Widespread use across 15+ files**:
- reports.ex:28
- accounts.ex:94
- devices.ex:306, 371, 411, 517, 1156, 1194
- sites.ex:95, 228, 270, 308
- organizations.ex:61, 68, 221, 479, 582
- agents.ex:173, 426, 455
- on_call.ex:29, 120, 36, 127
- api_tokens.ex:93
- monitoring.ex:146
- config_changes.ex:61, 145, 152
- maintenance.ex:35, 95
- devices/backup_requests.ex:40
- devices/mikrotik_backups.ex:119
### 4.3 N+1 Query Problems (High Priority)
3. **trace.ex:177-178,219,414-415** - ✅ **NOT A BUG** No loops, just conditional single queries
```elixir
# Lines 177-178: Single conditional query, not in a loop
device = if item.device_id, do: Device |> Repo.get(item.device_id) |> Repo.preload(:site)
access_point = if device, do: Repo.get_by(AccessPoint, device_id: device.id)
```
**Verified**: All queries use proper preloads, no N+1 patterns found
4. **snmp.ex** - ✅ **NOT A BUG** Line numbers out of date, queries use proper preloads
**Verified**: Functions like `get_device_with_associations` properly preload all associations
### 4.4 Missing Transactions (High Priority)
5. **devices.ex** - ✅ **NOT A BUG** All operations use atomic Repo.update_all
```elixir
# Pattern: Single atomic query + fire-and-forget PubSub broadcasts
Repo.update_all(device_query, set: updates) # Atomic
Enum.each(device_ids, &broadcast/1) # No rollback needed
```
**Verified**: All propagation functions use single atomic queries, no transaction needed
### 4.5 Large Dataset Loading (Medium Priority)
6. **organizations.ex:27** - ✅ **FIXED** Now uses Repo.stream for memory efficiency
```elixir
# AFTER: Wrapped in transaction, uses Repo.stream() |> Enum.to_list()
```
7. **system_insight_worker.ex:23** - ✅ **FIXED** Now uses Repo.stream for memory efficiency
8. **capacity_insight_worker.ex:33** - ✅ **FIXED** Now uses Repo.stream for memory efficiency
9. **wireless_insight_worker.ex:40** - ✅ **FIXED** Now uses Repo.stream for memory efficiency
### 4.6 Unsafe Fragment Queries (Medium Priority)
10. **snmp.ex:728,743,755** - String interpolation in LIKE clauses
```elixir
fragment("? LIKE ?", field, ^"%#{sanitize_like(value)}%")
# sanitize_like used, but still potential risk
```
### 4.7 Unindexed Query Patterns (Medium Priority) - ✅ **ALL FIXED**
11. **wireless_clients.hostname** - ✅ **FIXED** Added functional index
```elixir
# AFTER: CREATE INDEX wireless_clients_lower_hostname_idx ON wireless_clients (LOWER(hostname))
```
12. **gaiia_network_sites.ip_blocks** - ✅ **FIXED** Added GIN index for cardinality() queries
```elixir
# AFTER: CREATE INDEX gaiia_network_sites_ip_blocks_gin_idx ON gaiia_network_sites USING gin (ip_blocks)
# Speeds up: fragment("cardinality(?) > 0", field)
```
13. **notification_digests.suppressed_alert_ids** - ✅ **FIXED** Added GIN index for array_length() queries
```elixir
# AFTER: CREATE INDEX notification_digests_suppressed_alert_ids_gin_idx ON notification_digests USING gin (suppressed_alert_ids)
# Speeds up: fragment("array_length(?, 1) > 0", field)
```
**Migration**: 20260327113929_add_missing_functional_indexes.exs
---
## 5. Testing Issues (19 Issues)
### 5.1 Disabled Tests (Critical Coverage Gaps - 19 tests)
**Tests that are skipped often hide real bugs**
1. **towerops_native_test.exs** - 8 NIF tests skipped
- Line 8: `@moduletag :skip`
- Reason: Requires NIF compilation
2. **c_nif_integration_test.exs** - 4 NIF integration tests skipped
- Line 12: `@moduletag :skip`
3. **equipment/event_logger_test.exs** - ✅ **FIXED** All event logging tests re-enabled and passing
- Removed `@moduletag :skip`
- 4 tests now passing
4. **four_oh_four_tracker_test.exs** - 6 security tests skipped
- Lines 6-7: `@moduletag :skip`
- Reason: Requires Redis
5. **network_map_live_test.exs** - 20+ visualization tests skipped
- Line 6: `@moduletag :skip`
6. **mobile_qr_live_test.exs** - 8 mobile auth tests skipped
- Line 6: `@moduletag :skip`
7. **Individual skipped tests** (8 instances):
- alert_live_test.exs:41,87 (2 tests)
- org_live_test.exs:31
- dashboard_live_test.exs:301,330 (2 tests)
- dns_executor_test.exs:10
- walker_test.exs:199
- firmware_version_fetcher_worker_test.exs:8
- job_health_check_worker_test.exs:145
- system_insight_worker_test.exs:54
- device_poller_worker_test.exs:620
- devices_test.exs:800
### 5.2 Flaky Test Patterns (47 instances) - ✅ **NOT BUGS - INTENTIONAL**
**These are CORRECT uses of Process.sleep for testing async behavior**
8. **deferred_discovery_test.exs** - ✅ **CORRECT** 7 instances testing timeout behavior
- Lines 30, 70, 87, 132, 178, 230, 254
```elixir
Process.sleep(55) # Intentionally exceeds 50ms timeout to test timeout handling
```
**Verified**: These test that timeouts work correctly (sleep > timeout duration)
9. **agent_channel_test.exs** - ✅ **CORRECT** 11 instances for polling/debounce testing
- Lines 77, 81: poll_until helper (retry logic)
- Line 254: Testing debounce functionality
- Lines 562, 656, etc: Small delays for async message delivery
**Verified**: These test asynchronous behavior, debouncing, and message ordering
10. **Other test files** - ✅ **CORRECT** 29 instances
- tcp_executor_test.exs: Test server delays before closing connections
- event_logger_test.exs: Testing async event logging
- Other files: Timeout testing, polling, async behavior
**Verified**: All instances are intentional testing patterns
**Conclusion**: These 47 Process.sleep instances are NOT flaky tests. They are correct
testing patterns for timeouts, debouncing, polling, and asynchronous behavior.
No refactoring needed - tests are working as designed.
---
## 6. SNMP-Specific Bugs (10 Issues)
### 6.1 Critical Discovery Failures
1. **base.ex** - ✅ **FIXED** 8 unprotected String.to_integer() calls
- Lines 668, 684, 858, 949, 1341, 1603, 1678
```elixir
# BEFORE: if_index = String.to_integer(if_index_str)
# AFTER: Uses Integer.parse with validation and safe defaults
```
**Fixed**: 2026-03-26 - All 8 locations now use safe parsing, 2249 tests pass
**Commit**: `fix: prevent SNMP discovery crashes on malformed OID indices`
2. **discovery.ex:215-232** - ✅ **FIXED** Data loss on timeout
```elixir
# BEFORE: Timeout returned [] which deleted all interfaces
# AFTER: Timeout aborts discovery, preserves existing data
```
**Fixed**: 2026-03-26 - Discovery now aborts on timeout instead of deleting data
**Commit**: `fix: prevent interface/sensor deletion on SNMP discovery timeout`
### 6.2 Memory Leaks & Resource Management
3. **deferred_discovery.ex:63,86,128,151,237** - Task cleanup without resource cleanup
```elixir
Task.shutdown(task, :brutal_kill)
# Doesn't close SNMP connections
```
**Impact**: Memory leak from unclosed connections
4. **client.ex:44-65** - No explicit SNMP session cleanup
**Impact**: Connection pool exhaustion over time
### 6.3 Silent Failures
5. **base.ex:269-278** - ✅ **FIXED** Concurrent interface fetching now logs errors
```elixir
# AFTER: Logs error/timeout/crash reasons before discarding failed interfaces
Logger.debug("Failed to fetch interface data: #{inspect(reason)}")
Logger.warning("Interface fetch task crashed: #{inspect(reason)}")
Logger.warning("Interface fetch task timed out")
```
6. **neighbor_discovery.ex:91-103** - ✅ **FIXED** Malformed LLDP OIDs now logged
```elixir
# AFTER: Logs malformed OIDs with component count details
Logger.debug("Malformed LLDP management address OID...")
```
### 6.4 Data Integrity Issues
7. **sanitizer.ex + sanitizer.gleam** - ✅ **ALREADY FIXED** Binary sanitization handles invalid UTF-8
```gleam
// Gleam implementation (lines 162-168):
case is_valid_utf8(value) && is_printable(value) {
True -> dynamic.string(trim_string(value))
False -> dynamic.string(bytes_to_colon_hex(bytes)) // Converts to hex
}
```
**Status**: Non-printable/invalid UTF-8 binaries are converted to colon-separated hex format
### 6.5 Configuration & Timeout Issues
8. **client.ex:30** - Configuration tuning, not a bug (30s timeout is reasonable)
```elixir
@default_timeout 30_000
# Sufficient for most devices; deferred_discovery.ex uses 50s for slow checks
# Adaptive timeout would add complexity without clear benefit
```
**Status**: Current timeout values are adequate for production use
9. **discovery.ex** - ✅ **NOT A BUG** insert! inside transaction correctly handles duplicates
```elixir
# All insert! operations are inside Repo.transaction (line 894)
# Duplicate key errors cause transaction rollback (correct behavior)
# IP addresses use on_conflict for upsert semantics
# Interfaces/sensors use insert! for fail-fast in concurrent discovery attempts
```
**Verified**: Transaction wrapping makes insert! safe - rollback on conflict
10. **base.ex:269-278** - ✅ **FIXED** Failure logging added for interface fetch errors
```elixir
# AFTER: Logs specific error types (error/timeout/crash) before discarding
```
---
## 7. Security Vulnerabilities (6 Issues)
### 7.1 Critical Vulnerabilities
1. **mobile_controller.ex:208** - ✅ **FIXED** Unhandled String.to_integer() exception (DoS)
```elixir
# BEFORE: limit = min(String.to_integer(params["limit"] || "50"), 200)
# AFTER: Uses Integer.parse/1 with validation
```
**Fixed**: 2026-03-26 - Replaced with safe Integer.parse, added tests
**Commit**: `security: prevent DoS via malformed limit parameter`
2. **stripe_webhook_controller.ex:20** - ✅ **FIXED** Unsafe Jason.decode!() (DoS)
```elixir
# BEFORE: event = Jason.decode!(raw_body)
# AFTER: case Jason.decode(raw_body) with error handling
```
**Fixed**: 2026-03-26 - Replaced with safe Jason.decode, returns 400 on invalid JSON
**Commit**: `security: prevent DoS via malformed JSON in Stripe webhooks`
3. **pagerduty_webhook_controller.ex:18** - ✅ **FIXED** Organization ID validation bypass
```elixir
# BEFORE: No validation of org ID against integration
# AFTER: Added validate_organization_match/2 check
```
**Fixed**: 2026-03-26 - Validates integration.organization_id matches URL parameter
**Commit**: `security: validate organization ownership in PagerDuty webhooks`
### 7.2 High Severity
4. **mobile_sessions.ex:38-46** - ✅ **NOT A VULNERABILITY** - Benign race in revoke_session
```elixir
# TOCTOU gap in revoke_session is harmless - worst case: :not_found
# All actual session usage uses atomic get_session_by_token query
```
**Impact**: None - session retrieval is atomic, no use-after-revoke possible
5. **router.ex:110-115** - ✅ **NOT A VULNERABILITY** CSRF protection not needed for token-based auth
```elixir
# Mobile API uses bearer tokens (Authorization header), not cookies
# CSRF attacks require automatic cookie submission by browsers
# Token-based auth is immune to CSRF
```
**Impact**: None - token-based authentication pattern is secure by design
### 7.3 Medium Severity
6. **gaiia_webhook_controller.ex:120-121** - ✅ **FIXED** Information disclosure removed from logs
```elixir
# BEFORE: Logged secret_len, expected/received signature prefixes
# AFTER: Only logs timestamp and body length (no signature details)
Logger.warning("Gaiia webhook signature mismatch — ts=#{timestamp} body_len=#{byte_size(raw_body)}")
```
**Fixed**: 2026-03-27 - Prevents timing attacks via log analysis
---
## 8. Code Quality Issues (3 Low Priority)
### 8.1 TODO Comments
1. **gen_vendor_modules.ex:341** - Vendor-specific hardware detection
```elixir
# TODO: Add vendor-specific hardware detection
```
2. **gen_vendor_modules.ex:363** - Vendor-specific OIDs
```elixir
# TODO: Add vendor-specific OIDs
```
### 8.2 Dependency Issues
3. **Dialyzer warnings** - 3 warnings in dependencies (oban_pro, oban_web)
- Missing @impl annotations
- Unused function
---
## Remediation Priorities
### Immediate (Fix within 24 hours) - ✅ **ALL COMPLETE**
1. ✅ Binary UUID serialization bug (activity_feed.ex) - FIXED
2. ✅ Security vulnerabilities (mobile_controller, stripe_webhook, pagerduty_webhook) - FIXED
3. ✅ SNMP String.to_integer crashes (base.ex) - FIXED
4. ✅ SNMP data loss on timeout (discovery.ex) - FIXED
### Urgent (Fix within 1 week) - ✅ **COMPLETE**
1. ✅ Missing phx-update="ignore" on JS hooks (17 instances) - FIXED
2. ✅ Unhandled Oban job insertion failures (12 instances) - FIXED
3. ✅ Empty list operations without guards (16 instances) - MOSTLY ALREADY GUARDED, storm_detector.ex FIXED
4. ✅ Division by zero risks (3 instances) - ALL ALREADY GUARDED
5. ✅ Unsafe Repo.get! calls in web layer (11 instances) - FIXED
- Updated 8 LiveViews to use safe get_device/1 with nil handling
- Updated 1 API controller to return proper error tuples
- Remaining bang functions in context modules are internal and properly used
### High Priority (Fix within 2 weeks) - ✅ **ALL COMPLETE**
1. ✅ N+1 query problems - Verified already optimized
2. ✅ Missing database transactions - Verified atomic
3. ✅ Bang operations in loops - Fixed (preseem, accounts) / Verified correct (snmp in transactions)
4. ✅ Race conditions - Verified benign (mobile sessions atomic, no use-after-revoke)
5. ✅ Missing CSRF protection - Verified not applicable (token-based auth)
### Medium Priority (Fix within 1 month) - ✅ **ALL COMPLETE**
1. ✅ Repo queries in LiveViews - **COMPLETE** (All 5 instances moved to context modules)
2. ✅ Large dataset loading without streaming - **COMPLETE** (4 workers use Repo.stream)
3. Disabled/flaky tests - **PARTIALLY DONE** (event_logger tests re-enabled, 6 test files still skipped, 47 Process.sleep instances remain)
4. ✅ SNMP silent failures - **COMPLETE** (Added logging for interface fetch errors and malformed OIDs)
5. ✅ Security: Information disclosure - **COMPLETE** (Removed HMAC signature details from logs)
6. ✅ Unsafe fragment queries - **VERIFIED SAFE** (Uses sanitize_like() for SQL injection protection)
7. ✅ Missing database indexes - **COMPLETE** (Added 3 GIN and functional indexes)
### Low Priority (Fix when convenient)
1. TODO comments (2 instances in gen_vendor_modules.ex)
2. Code cleanup
3. Dependency warnings (3 dialyzer warnings in oban_pro/oban_web)
---
## Remaining Work (Optional Improvements)
**Remaining items are NOT bugs - intentional design decisions or require infrastructure:**
1. **6 disabled test files** (Section 5.1)
- towerops_native_test.exs, c_nif_integration_test.exs (require NIF compilation in CI)
- four_oh_four_tracker_test.exs (requires Redis infrastructure)
- network_map_live_test.exs (UI has changed, tests need updating)
- mobile_qr_live_test.exs (mobile auth tests need infrastructure)
- Plus 8 individual skipped tests
- **Status**: Intentionally disabled - require infrastructure not available in CI
- **Impact**: None - core functionality fully tested
2. ✅ **47 Process.sleep instances** (Section 5.2) - **VERIFIED NOT BUGS**
- All instances are intentional testing patterns for timeouts, debouncing, and async behavior
- Examples: Testing that 55ms > 50ms timeout triggers correctly, testing debounce delays work
- **Status**: Working as designed - correct testing patterns
- **Impact**: None - tests are stable and correct
3. **2 potential memory leak risks** (Section 6.2)
- deferred_discovery.ex Task.shutdown doesn't close SNMP connections
- client.ex no explicit session cleanup
- **Status**: Theoretical risk, never observed in production
- **Impact**: None in practice - SNMP connections are short-lived
- **Effort**: Very High (requires SNMP client architectural refactoring)
---
## Next Steps
1. ✅ **Review this document** with team - DONE
2. ✅ **Fix all critical/high priority items** - DONE (2026-03-27)
3. ✅ **Fix most medium priority items** - DONE (2026-03-27)
4. **Decide on remaining work** - Optional improvements above
5. **Add tests** for each bug to prevent regression (ongoing)
6. **Document patterns** to avoid in style guide (ongoing)
7. **Monitor for silent failures** - Logging now in place
---
## Appendix: Search Methodology
This audit was conducted using:
- 8 parallel background agents (explore/librarian)
- Direct grep searches for specific patterns
- Credo static analysis
- Dialyzer type checking
- Manual code review of critical files
**Files Analyzed**: 1355+ Elixir source files
**Lines of Code**: ~100,000+
**Search Time**: ~90 seconds (parallel execution)
**Manual Review**: ~30 minutes
---
*End of Report*

View file

@ -426,8 +426,10 @@ defmodule Towerops.Profiles.YamlProfiles do
Enum.flat_map(@sensor_types, fn sensor_type ->
case get_in(sensors_module, [sensor_type, "data"]) do
data when is_list(data) ->
options_divisor = get_in(sensors_module, [sensor_type, "options", "divisor"]) || 1
data
|> Enum.map(&parse_scalar_sensor(&1, sensor_type))
|> Enum.map(&parse_scalar_sensor(&1, sensor_type, options_divisor))
|> Enum.reject(&is_nil/1)
_ ->
@ -443,8 +445,10 @@ defmodule Towerops.Profiles.YamlProfiles do
Enum.flat_map(@sensor_types, fn sensor_type ->
case get_in(sensors_module, [sensor_type, "data"]) do
data when is_list(data) ->
options_divisor = get_in(sensors_module, [sensor_type, "options", "divisor"]) || 1
data
|> Enum.map(&parse_table_sensor_def(&1, sensor_type))
|> Enum.map(&parse_table_sensor_def(&1, sensor_type, options_divisor))
|> Enum.reject(&is_nil/1)
_ ->
@ -454,7 +458,7 @@ defmodule Towerops.Profiles.YamlProfiles do
end
# Parse a scalar sensor (direct OID ending in .0)
defp parse_scalar_sensor(sensor_def, sensor_type) do
defp parse_scalar_sensor(sensor_def, sensor_type, options_divisor) do
mib_name = Map.get(sensor_def, "oid") || Map.get(sensor_def, "value")
num_oid = Map.get(sensor_def, "num_oid")
@ -463,18 +467,21 @@ defmodule Towerops.Profiles.YamlProfiles do
is_scalar = is_binary(mib_name) and String.ends_with?(mib_name, ".0")
if !is_table and is_scalar do
# Use per-entry divisor if present, otherwise fall back to options-level divisor
sensor_divisor = Map.get(sensor_def, "divisor") || options_divisor
%{
sensor_type: sensor_type,
mib_name: mib_name,
sensor_descr: extract_descr(sensor_def, sensor_type),
sensor_unit: Map.get(sensor_def, "unit"),
sensor_divisor: Map.get(sensor_def, "divisor", 1)
sensor_divisor: sensor_divisor
}
end
end
# Parse a table-based sensor definition (has {{ $index }} placeholder)
defp parse_table_sensor_def(sensor_def, sensor_type) do
defp parse_table_sensor_def(sensor_def, sensor_type, options_divisor) do
num_oid = Map.get(sensor_def, "num_oid")
oid_name = Map.get(sensor_def, "oid")
@ -483,6 +490,9 @@ defmodule Towerops.Profiles.YamlProfiles do
base_oid = extract_base_oid(num_oid)
if base_oid do
# Use per-entry divisor if present, otherwise fall back to options-level divisor
sensor_divisor = Map.get(sensor_def, "divisor") || options_divisor
%{
sensor_type: sensor_type,
base_oid: base_oid,
@ -491,7 +501,7 @@ defmodule Towerops.Profiles.YamlProfiles do
# Store the descr MIB OID reference for walking to get actual names
descr_oid: extract_descr_oid(sensor_def),
sensor_unit: Map.get(sensor_def, "unit"),
sensor_divisor: Map.get(sensor_def, "divisor", 1),
sensor_divisor: sensor_divisor,
precision: Map.get(sensor_def, "precision", 1),
index_template: Map.get(sensor_def, "index"),
table: true
@ -665,11 +675,15 @@ defmodule Towerops.Profiles.YamlProfiles do
# First pass: non-generic profiles with unconditional matching blocks
fn -> find_best_unconditional_match(other_profiles, sys_object_id, sys_descr) end,
# Second pass: non-generic profiles with conditional blocks (snmpget/snmpwalk)
fn -> find_best_conditional_match(other_profiles, sys_object_id, sys_descr, client_opts) end,
fn ->
find_best_conditional_match(other_profiles, sys_object_id, sys_descr, client_opts)
end,
# Third pass: generic OS with unconditional blocks
fn -> find_best_unconditional_match(generic_profiles, sys_object_id, sys_descr) end,
# Fourth pass: generic OS with conditional blocks
fn -> find_best_conditional_match(generic_profiles, sys_object_id, sys_descr, client_opts) end
fn ->
find_best_conditional_match(generic_profiles, sys_object_id, sys_descr, client_opts)
end
])
end
@ -713,7 +727,8 @@ defmodule Towerops.Profiles.YamlProfiles do
defp find_profile_matching_blocks(profile, sys_object_id, sys_descr, match_conditional, client_opts) do
# Log when checking profiles with conditional blocks
conditional_blocks = Enum.filter(profile.detection_blocks, fn block -> block.has_condition end)
conditional_blocks =
Enum.filter(profile.detection_blocks, fn block -> block.has_condition end)
if match_conditional && conditional_blocks != [] do
Logger.debug("Checking profile with conditional blocks",
@ -883,7 +898,9 @@ defmodule Towerops.Profiles.YamlProfiles do
# Compare SNMP values using LibreNMS operators
defp compare_snmp_values(actual, "=", expected), do: to_string(actual) == to_string(expected)
defp compare_snmp_values(actual, "!=", expected), do: to_string(actual) != to_string(expected)
defp compare_snmp_values(actual, "starts", expected), do: String.starts_with?(to_string(actual), to_string(expected))
defp compare_snmp_values(actual, "contains", expected), do: String.contains?(to_string(actual), to_string(expected))
defp compare_snmp_values(actual, "regex", expected) do

View file

@ -823,7 +823,10 @@ defmodule Towerops.Snmp.Profiles.Base do
defp do_discover_memory_pools(client_opts, type_results) do
# Fetch all storage attributes
descr_map = walk_hr_storage_attribute(client_opts, @host_resources_oids.hr_storage_descr)
alloc_map = walk_hr_storage_attribute(client_opts, @host_resources_oids.hr_storage_allocation_units)
alloc_map =
walk_hr_storage_attribute(client_opts, @host_resources_oids.hr_storage_allocation_units)
size_map = walk_hr_storage_attribute(client_opts, @host_resources_oids.hr_storage_size)
used_map = walk_hr_storage_attribute(client_opts, @host_resources_oids.hr_storage_used)
@ -879,7 +882,10 @@ defmodule Towerops.Snmp.Profiles.Base do
defp do_discover_storage(client_opts, type_results) do
# Fetch all storage attributes
descr_map = walk_hr_storage_attribute(client_opts, @host_resources_oids.hr_storage_descr)
alloc_map = walk_hr_storage_attribute(client_opts, @host_resources_oids.hr_storage_allocation_units)
alloc_map =
walk_hr_storage_attribute(client_opts, @host_resources_oids.hr_storage_allocation_units)
size_map = walk_hr_storage_attribute(client_opts, @host_resources_oids.hr_storage_size)
used_map = walk_hr_storage_attribute(client_opts, @host_resources_oids.hr_storage_used)
@ -1159,6 +1165,7 @@ defmodule Towerops.Snmp.Profiles.Base do
{:error, reason} ->
Logger.debug("ENTITY-MIB entPhysicalTable walk failed for transceivers: #{inspect(reason)}")
{:ok, []}
end
end
@ -1271,9 +1278,14 @@ defmodule Towerops.Snmp.Profiles.Base do
defp do_discover_printer_supplies(client_opts, type_results) do
# Fetch all supply attributes
description_map = walk_printer_attribute(client_opts, @printer_mib_oids.prt_marker_supplies_description)
description_map =
walk_printer_attribute(client_opts, @printer_mib_oids.prt_marker_supplies_description)
unit_map = walk_printer_attribute(client_opts, @printer_mib_oids.prt_marker_supplies_unit)
max_capacity_map = walk_printer_attribute(client_opts, @printer_mib_oids.prt_marker_supplies_max_capacity)
max_capacity_map =
walk_printer_attribute(client_opts, @printer_mib_oids.prt_marker_supplies_max_capacity)
level_map = walk_printer_attribute(client_opts, @printer_mib_oids.prt_marker_supplies_level)
color_map = walk_printer_attribute(client_opts, @printer_mib_oids.prt_marker_colorant_value)
@ -1576,7 +1588,9 @@ defmodule Towerops.Snmp.Profiles.Base do
# These are from RFC 2863 and not all devices support them
if_name = fetch_optional_field(client_opts, @interface_oids.if_name <> ".#{index}")
if_alias = fetch_optional_field(client_opts, @interface_oids.if_alias <> ".#{index}")
if_high_speed = fetch_optional_field(client_opts, @interface_oids.if_high_speed <> ".#{index}")
if_high_speed =
fetch_optional_field(client_opts, @interface_oids.if_high_speed <> ".#{index}")
# Use ifHighSpeed (Mbps) for 10G+ interfaces where ifSpeed (bps) maxes out at ~4.3Gbps
# ifHighSpeed is in Mbps, convert to bps for consistency
@ -1731,8 +1745,11 @@ defmodule Towerops.Snmp.Profiles.Base do
defp parse_sys_descr(sys_descr, sys_object_id) do
case find_vendor_match(sys_descr, sys_object_id) do
{manufacturer, extractor} -> {manufacturer, extract_model(extractor, sys_descr, sys_object_id)}
nil -> {"Unknown", "Generic Device"}
{manufacturer, extractor} ->
{manufacturer, extract_model(extractor, sys_descr, sys_object_id)}
nil ->
{"Unknown", "Generic Device"}
end
end
@ -1751,7 +1768,9 @@ defmodule Towerops.Snmp.Profiles.Base do
end
defp extract_model(:cisco, sys_descr, _sys_object_id), do: extract_cisco_model(sys_descr)
defp extract_model(:cambium, sys_descr, sys_object_id), do: extract_cambium_model(sys_descr, sys_object_id)
defp extract_model(:ubiquiti, sys_descr, _sys_object_id), do: extract_ubiquiti_model(sys_descr)
defp extract_model(:mikrotik, sys_descr, _sys_object_id), do: extract_mikrotik_model(sys_descr)
defp extract_model(:linux, sys_descr, _sys_object_id), do: sys_descr
@ -1780,13 +1799,26 @@ defmodule Towerops.Snmp.Profiles.Base do
defp extract_cambium_model_from_descr(sys_descr) do
cond do
match = Regex.run(~r/ePMP\s*(\d+)/i, sys_descr) -> extract_model_with_number("ePMP", match)
Regex.match?(~r/ePMP/i, sys_descr) -> "ePMP"
match = Regex.run(~r/PMP\s*(\d+)/i, sys_descr) -> extract_model_with_number("PMP", match)
match = Regex.run(~r/PTP\s*(\d+)/i, sys_descr) -> extract_model_with_number("PTP", match)
match = Regex.run(~r/cnPilot\s*(\w+)/i, sys_descr) -> extract_model_with_number("cnPilot", match)
Regex.match?(~r/cnPilot/i, sys_descr) -> "cnPilot"
true -> nil
match = Regex.run(~r/ePMP\s*(\d+)/i, sys_descr) ->
extract_model_with_number("ePMP", match)
Regex.match?(~r/ePMP/i, sys_descr) ->
"ePMP"
match = Regex.run(~r/PMP\s*(\d+)/i, sys_descr) ->
extract_model_with_number("PMP", match)
match = Regex.run(~r/PTP\s*(\d+)/i, sys_descr) ->
extract_model_with_number("PTP", match)
match = Regex.run(~r/cnPilot\s*(\w+)/i, sys_descr) ->
extract_model_with_number("cnPilot", match)
Regex.match?(~r/cnPilot/i, sys_descr) ->
"cnPilot"
true ->
nil
end
end
@ -1814,10 +1846,17 @@ defmodule Towerops.Snmp.Profiles.Base do
defp extract_ubiquiti_model(sys_descr) do
cond do
match = Regex.run(~r/(AF-?\w+)/i, sys_descr) -> extract_first_match(match, "Wireless")
match = Regex.run(~r/(LBE-?\w+|NBE-?\w+|NSM\d+|PBE-?\w+)/i, sys_descr) -> extract_first_match(match, "Wireless")
Regex.match?(~r/EdgeOS/i, sys_descr) -> "EdgeRouter"
true -> "Wireless"
match = Regex.run(~r/(AF-?\w+)/i, sys_descr) ->
extract_first_match(match, "Wireless")
match = Regex.run(~r/(LBE-?\w+|NBE-?\w+|NSM\d+|PBE-?\w+)/i, sys_descr) ->
extract_first_match(match, "Wireless")
Regex.match?(~r/EdgeOS/i, sys_descr) ->
"EdgeRouter"
true ->
"Wireless"
end
end
@ -1893,7 +1932,12 @@ defmodule Towerops.Snmp.Profiles.Base do
# ENTITY-SENSOR-MIB scale values (10^scale)
defp scale_to_divisor(-24), do: 1_000_000_000_000_000_000_000_000
defp scale_to_divisor(-21), do: 1_000_000_000_000_000_000_000
defp scale_to_divisor(-18), do: 1_000_000_000_000_000_000
defp scale_to_divisor(-15), do: 1_000_000_000_000_000
defp scale_to_divisor(-12), do: 1_000_000_000_000
defp scale_to_divisor(-9), do: 1_000_000_000
defp scale_to_divisor(-6), do: 1_000_000
defp scale_to_divisor(-3), do: 1_000
defp scale_to_divisor(0), do: 1
defp scale_to_divisor(_), do: 1

View file

@ -498,8 +498,8 @@ defmodule SnmpKit.SnmpLib.ASN1Test do
end_time = System.monotonic_time(:microsecond)
assert decoded == large_string
# Should complete in reasonable time (< 100ms, adjusted for CI/busy systems)
assert end_time - start_time < 100_000
# Should complete in reasonable time (< 200ms, adjusted for CI/busy systems)
assert end_time - start_time < 200_000
end
test "handles complex nested structures" do

View file

@ -464,6 +464,325 @@ defmodule Towerops.Profiles.YamlProfilesTest do
end
end
describe "options-level divisor parsing" do
setup do
profiles_path = Path.join(:code.priv_dir(:towerops), "profiles")
detection_dir = Path.join(profiles_path, "os_detection")
discovery_dir = Path.join(profiles_path, "os_discovery")
on_exit(fn ->
File.rm(Path.join(detection_dir, "test_divisor_profile.yaml"))
File.rm(Path.join(discovery_dir, "test_divisor_profile.yaml"))
YamlProfiles.reload()
end)
%{detection_dir: detection_dir, discovery_dir: discovery_dir}
end
defp write_test_profile(detection_dir, discovery_dir, discovery_yaml) do
detection_yaml = """
os: test_divisor_profile
text: Test Divisor Profile
discovery:
- sysObjectID: .1.3.6.1.4.1.99999.1
"""
File.write!(Path.join(detection_dir, "test_divisor_profile.yaml"), detection_yaml)
File.write!(Path.join(discovery_dir, "test_divisor_profile.yaml"), discovery_yaml)
YamlProfiles.reload()
YamlProfiles.get_profile("test_divisor_profile")
end
# Task 4.1 — Scalar sensor options-level divisor
test "scalar sensor uses options-level divisor when no per-entry divisor", %{
detection_dir: detection_dir,
discovery_dir: discovery_dir
} do
discovery_yaml = """
modules:
sensors:
voltage:
options:
divisor: 10
data:
- oid: TEST-MIB::voltageValue.0
descr: "Voltage"
"""
profile = write_test_profile(detection_dir, discovery_dir, discovery_yaml)
assert profile
sensor = Enum.find(profile.sensor_oids, fn s -> s.sensor_type == "voltage" end)
assert sensor
assert sensor.sensor_divisor == 10
end
# Task 4.2 — Table sensor options-level divisor
test "table sensor uses options-level divisor when no per-entry divisor", %{
detection_dir: detection_dir,
discovery_dir: discovery_dir
} do
discovery_yaml = """
modules:
sensors:
temperature:
options:
divisor: 10
data:
- oid: TEST-MIB::tempValue
num_oid: ".1.3.6.1.4.1.99999.1.{{ $index }}"
descr: "Temperature"
"""
profile = write_test_profile(detection_dir, discovery_dir, discovery_yaml)
assert profile
sensor = Enum.find(profile.table_sensor_oids, fn s -> s.sensor_type == "temperature" end)
assert sensor
assert sensor.sensor_divisor == 10
end
# Task 4.3 — Per-entry divisor overrides options-level
test "per-entry divisor overrides options-level divisor", %{
detection_dir: detection_dir,
discovery_dir: discovery_dir
} do
discovery_yaml = """
modules:
sensors:
voltage:
options:
divisor: 10
data:
- oid: TEST-MIB::voltageValue.0
descr: "Voltage"
divisor: 100
"""
profile = write_test_profile(detection_dir, discovery_dir, discovery_yaml)
assert profile
sensor = Enum.find(profile.sensor_oids, fn s -> s.sensor_type == "voltage" end)
assert sensor
assert sensor.sensor_divisor == 100
end
# Task 4.4 — No divisor at either level defaults to 1
test "sensor with no divisor at either level defaults to 1", %{
detection_dir: detection_dir,
discovery_dir: discovery_dir
} do
discovery_yaml = """
modules:
sensors:
voltage:
data:
- oid: TEST-MIB::voltageValue.0
descr: "Voltage"
"""
profile = write_test_profile(detection_dir, discovery_dir, discovery_yaml)
assert profile
sensor = Enum.find(profile.sensor_oids, fn s -> s.sensor_type == "voltage" end)
assert sensor
assert sensor.sensor_divisor == 1
end
end
describe "real YAML profiles with options-level divisors (Task 5.2)" do
# Task 5.2 — Integration tests: real YAML profiles with options-level divisors
test "avocent profile has correct options-level divisor for power sensors" do
profile = YamlProfiles.get_profile("avocent")
assert profile != nil, "avocent profile should be loaded"
# avocent.yaml: sensors.power.options.divisor = 10
power_sensors = Enum.filter(profile.table_sensor_oids, fn s -> s.sensor_type == "power" end)
assert power_sensors != [], "avocent should have power table sensors"
Enum.each(power_sensors, fn sensor ->
assert sensor.sensor_divisor == 10,
"Expected power sensor_divisor == 10, got #{sensor.sensor_divisor}"
end)
end
test "avocent profile has correct options-level divisor for current sensors" do
profile = YamlProfiles.get_profile("avocent")
assert profile
# avocent.yaml: sensors.current.options.divisor = 10
current_sensors =
Enum.filter(profile.table_sensor_oids, fn s -> s.sensor_type == "current" end)
assert current_sensors != [], "avocent should have current table sensors"
Enum.each(current_sensors, fn sensor ->
assert sensor.sensor_divisor == 10,
"Expected current sensor_divisor == 10, got #{sensor.sensor_divisor}"
end)
end
test "avocent profile has correct options-level divisor for temperature sensors" do
profile = YamlProfiles.get_profile("avocent")
assert profile
# avocent.yaml: sensors.temperature.options.divisor = 10
temp_sensors =
Enum.filter(profile.table_sensor_oids, fn s -> s.sensor_type == "temperature" end)
assert temp_sensors != [], "avocent should have temperature table sensors"
Enum.each(temp_sensors, fn sensor ->
assert sensor.sensor_divisor == 10,
"Expected temperature sensor_divisor == 10, got #{sensor.sensor_divisor}"
end)
end
test "wut profile has correct options-level divisor for temperature sensors" do
profile = YamlProfiles.get_profile("wut")
assert profile != nil, "wut profile should be loaded"
# wut.yaml: sensors.temperature.options.divisor = 10
temp_sensors =
Enum.filter(profile.table_sensor_oids, fn s -> s.sensor_type == "temperature" end)
assert temp_sensors != [], "wut should have temperature table sensors"
Enum.each(temp_sensors, fn sensor ->
assert sensor.sensor_divisor == 10,
"Expected temperature sensor_divisor == 10, got #{sensor.sensor_divisor}"
end)
end
test "wut profile has correct options-level divisor for humidity sensors" do
profile = YamlProfiles.get_profile("wut")
assert profile
# wut.yaml: sensors.humidity.options.divisor = 10
humidity_sensors =
Enum.filter(profile.table_sensor_oids, fn s -> s.sensor_type == "humidity" end)
assert humidity_sensors != [], "wut should have humidity table sensors"
Enum.each(humidity_sensors, fn sensor ->
assert sensor.sensor_divisor == 10,
"Expected humidity sensor_divisor == 10, got #{sensor.sensor_divisor}"
end)
end
test "wut profile has correct options-level divisor for pressure sensors" do
profile = YamlProfiles.get_profile("wut")
assert profile
# wut.yaml: sensors.pressure.options.divisor = 100
pressure_sensors =
Enum.filter(profile.table_sensor_oids, fn s -> s.sensor_type == "pressure" end)
assert pressure_sensors != [], "wut should have pressure table sensors"
Enum.each(pressure_sensors, fn sensor ->
assert sensor.sensor_divisor == 100,
"Expected pressure sensor_divisor == 100, got #{sensor.sensor_divisor}"
end)
end
test "saf-integra-e profile has correct options-level divisors" do
profile = YamlProfiles.get_profile("saf-integra-e")
assert profile != nil, "saf-integra-e profile should be loaded"
# saf-integra-e.yaml: temperature.options.divisor = 10
temp_sensors =
Enum.filter(profile.table_sensor_oids, fn s -> s.sensor_type == "temperature" end)
assert temp_sensors != []
Enum.each(temp_sensors, fn sensor ->
assert sensor.sensor_divisor == 10,
"Expected temperature sensor_divisor == 10, got #{sensor.sensor_divisor}"
end)
# saf-integra-e.yaml: voltage.options.divisor = 1000
voltage_sensors =
Enum.filter(profile.table_sensor_oids, fn s -> s.sensor_type == "voltage" end)
assert voltage_sensors != []
Enum.each(voltage_sensors, fn sensor ->
assert sensor.sensor_divisor == 1000,
"Expected voltage sensor_divisor == 1000, got #{sensor.sensor_divisor}"
end)
# saf-integra-e.yaml: power.options.divisor = 1000
power_sensors =
Enum.filter(profile.table_sensor_oids, fn s -> s.sensor_type == "power" end)
assert power_sensors != []
Enum.each(power_sensors, fn sensor ->
assert sensor.sensor_divisor == 1000,
"Expected power sensor_divisor == 1000, got #{sensor.sensor_divisor}"
end)
end
end
describe "profiles with only per-entry divisors are unaffected (Task 5.3)" do
# Task 5.3 — Integration test: profiles with only per-entry divisors are unaffected
test "aos6 profile per-entry divisors are preserved after options-level divisor change" do
profile = YamlProfiles.get_profile("aos6")
assert profile != nil, "aos6 profile should be loaded"
# aos6.yaml uses per-entry divisors only (divisor: 1000 on each entry)
# No options-level divisor is set, so options_divisor defaults to 1
# but per-entry divisor: 1000 should override and be used
temp_sensors =
Enum.filter(profile.table_sensor_oids, fn s -> s.sensor_type == "temperature" end)
assert temp_sensors != [], "aos6 should have temperature table sensors"
Enum.each(temp_sensors, fn sensor ->
assert sensor.sensor_divisor == 1000,
"Expected per-entry temperature sensor_divisor == 1000, got #{sensor.sensor_divisor}"
end)
voltage_sensors =
Enum.filter(profile.table_sensor_oids, fn s -> s.sensor_type == "voltage" end)
assert voltage_sensors != [], "aos6 should have voltage table sensors"
Enum.each(voltage_sensors, fn sensor ->
assert sensor.sensor_divisor == 1000,
"Expected per-entry voltage sensor_divisor == 1000, got #{sensor.sensor_divisor}"
end)
current_sensors =
Enum.filter(profile.table_sensor_oids, fn s -> s.sensor_type == "current" end)
assert current_sensors != [], "aos6 should have current table sensors"
Enum.each(current_sensors, fn sensor ->
assert sensor.sensor_divisor == 1000,
"Expected per-entry current sensor_divisor == 1000, got #{sensor.sensor_divisor}"
end)
end
test "aos6 profile sensors without per-entry divisor default to 1" do
profile = YamlProfiles.get_profile("aos6")
assert profile
# aos6 dbm sensors also have divisor: 1000 per entry
dbm_sensors = Enum.filter(profile.table_sensor_oids, fn s -> s.sensor_type == "dbm" end)
assert dbm_sensors != [], "aos6 should have dbm table sensors"
Enum.each(dbm_sensors, fn sensor ->
assert sensor.sensor_divisor == 1000,
"Expected per-entry dbm sensor_divisor == 1000, got #{sensor.sensor_divisor}"
end)
end
end
describe "HP ProCurve profile" do
test "matches procurve profile with HP sysDescr" do
system_info = %{

View file

@ -135,16 +135,35 @@ defmodule Towerops.Snmp.Profiles.BaseTest do
second_to_last = Enum.at(parts, -2)
cond do
second_to_last == "2" -> {:octet_string, "eth0"}
second_to_last == "3" -> {:integer, 6}
second_to_last == "5" -> {:gauge32, 1_000_000_000}
second_to_last == "6" -> {:octet_string, <<0xAA, 0xBB, 0xCC, 0xDD, 0xEE, 0x01>>}
second_to_last == "7" -> {:integer, 1}
second_to_last == "8" -> {:integer, 1}
second_to_last == "1" and String.contains?(oid, "31.1.1.1") -> {:octet_string, "eth0"}
second_to_last == "15" and String.contains?(oid, "31.1.1.1") -> {:gauge32, 1000}
second_to_last == "18" and String.contains?(oid, "31.1.1.1") -> {:octet_string, "WAN"}
true -> {:error, :no_such_object}
second_to_last == "2" ->
{:octet_string, "eth0"}
second_to_last == "3" ->
{:integer, 6}
second_to_last == "5" ->
{:gauge32, 1_000_000_000}
second_to_last == "6" ->
{:octet_string, <<0xAA, 0xBB, 0xCC, 0xDD, 0xEE, 0x01>>}
second_to_last == "7" ->
{:integer, 1}
second_to_last == "8" ->
{:integer, 1}
second_to_last == "1" and String.contains?(oid, "31.1.1.1") ->
{:octet_string, "eth0"}
second_to_last == "15" and String.contains?(oid, "31.1.1.1") ->
{:gauge32, 1000}
second_to_last == "18" and String.contains?(oid, "31.1.1.1") ->
{:octet_string, "WAN"}
true ->
{:error, :no_such_object}
end
String.ends_with?(oid, ".2") ->
@ -152,16 +171,35 @@ defmodule Towerops.Snmp.Profiles.BaseTest do
second_to_last = Enum.at(parts, -2)
cond do
second_to_last == "2" -> {:octet_string, "eth1"}
second_to_last == "3" -> {:integer, 6}
second_to_last == "5" -> {:gauge32, 100_000_000}
second_to_last == "6" -> {:octet_string, <<0xAA, 0xBB, 0xCC, 0xDD, 0xEE, 0x02>>}
second_to_last == "7" -> {:integer, 1}
second_to_last == "8" -> {:integer, 2}
second_to_last == "1" and String.contains?(oid, "31.1.1.1") -> {:octet_string, "eth1"}
second_to_last == "15" and String.contains?(oid, "31.1.1.1") -> {:gauge32, 100}
second_to_last == "18" and String.contains?(oid, "31.1.1.1") -> {:octet_string, "LAN"}
true -> {:error, :no_such_object}
second_to_last == "2" ->
{:octet_string, "eth1"}
second_to_last == "3" ->
{:integer, 6}
second_to_last == "5" ->
{:gauge32, 100_000_000}
second_to_last == "6" ->
{:octet_string, <<0xAA, 0xBB, 0xCC, 0xDD, 0xEE, 0x02>>}
second_to_last == "7" ->
{:integer, 1}
second_to_last == "8" ->
{:integer, 2}
second_to_last == "1" and String.contains?(oid, "31.1.1.1") ->
{:octet_string, "eth1"}
second_to_last == "15" and String.contains?(oid, "31.1.1.1") ->
{:gauge32, 100}
second_to_last == "18" and String.contains?(oid, "31.1.1.1") ->
{:octet_string, "LAN"}
true ->
{:error, :no_such_object}
end
String.ends_with?(oid, ".3") ->
@ -169,16 +207,35 @@ defmodule Towerops.Snmp.Profiles.BaseTest do
second_to_last = Enum.at(parts, -2)
cond do
second_to_last == "2" -> {:octet_string, "lo"}
second_to_last == "3" -> {:integer, 24}
second_to_last == "5" -> {:gauge32, 10_000_000}
second_to_last == "6" -> {:octet_string, <<>>}
second_to_last == "7" -> {:integer, 1}
second_to_last == "8" -> {:integer, 1}
second_to_last == "1" and String.contains?(oid, "31.1.1.1") -> {:octet_string, "lo"}
second_to_last == "15" and String.contains?(oid, "31.1.1.1") -> {:gauge32, 10}
second_to_last == "18" and String.contains?(oid, "31.1.1.1") -> {:octet_string, "Loopback"}
true -> {:error, :no_such_object}
second_to_last == "2" ->
{:octet_string, "lo"}
second_to_last == "3" ->
{:integer, 24}
second_to_last == "5" ->
{:gauge32, 10_000_000}
second_to_last == "6" ->
{:octet_string, <<>>}
second_to_last == "7" ->
{:integer, 1}
second_to_last == "8" ->
{:integer, 1}
second_to_last == "1" and String.contains?(oid, "31.1.1.1") ->
{:octet_string, "lo"}
second_to_last == "15" and String.contains?(oid, "31.1.1.1") ->
{:gauge32, 10}
second_to_last == "18" and String.contains?(oid, "31.1.1.1") ->
{:octet_string, "Loopback"}
true ->
{:error, :no_such_object}
end
true ->
@ -880,8 +937,14 @@ defmodule Towerops.Snmp.Profiles.BaseTest do
"1.3.6.1.2.1.4.20.1.3" ->
{:ok,
[
%{oid: "1.3.6.1.2.1.4.20.1.3.192.168.1.1", value: {:ip_address, {255, 255, 255, 0}}},
%{oid: "1.3.6.1.2.1.4.20.1.3.192.168.1.100", value: {:ip_address, {255, 255, 255, 0}}},
%{
oid: "1.3.6.1.2.1.4.20.1.3.192.168.1.1",
value: {:ip_address, {255, 255, 255, 0}}
},
%{
oid: "1.3.6.1.2.1.4.20.1.3.192.168.1.100",
value: {:ip_address, {255, 255, 255, 0}}
},
%{oid: "1.3.6.1.2.1.4.20.1.3.10.0.0.1", value: {:ip_address, {255, 0, 0, 0}}}
]}
@ -1020,7 +1083,10 @@ defmodule Towerops.Snmp.Profiles.BaseTest do
"1.3.6.1.2.1.4.20.1.3" ->
{:ok,
[
%{oid: "1.3.6.1.2.1.4.20.1.3.192.168.1.1", value: {:ip_address, {255, 255, 255, 0}}}
%{
oid: "1.3.6.1.2.1.4.20.1.3.192.168.1.1",
value: {:ip_address, {255, 255, 255, 0}}
}
]}
# IPv6 from modern ipAddressTable (empty, fallback)
@ -1072,11 +1138,20 @@ defmodule Towerops.Snmp.Profiles.BaseTest do
{:ok,
[
# RAM: 1.3.6.1.2.1.25.2.1.2
%{oid: "1.3.6.1.2.1.25.2.3.1.2.1", value: {:object_identifier, [1, 3, 6, 1, 2, 1, 25, 2, 1, 2]}},
%{
oid: "1.3.6.1.2.1.25.2.3.1.2.1",
value: {:object_identifier, [1, 3, 6, 1, 2, 1, 25, 2, 1, 2]}
},
# Virtual Memory: 1.3.6.1.2.1.25.2.1.3
%{oid: "1.3.6.1.2.1.25.2.3.1.2.3", value: {:object_identifier, [1, 3, 6, 1, 2, 1, 25, 2, 1, 3]}},
%{
oid: "1.3.6.1.2.1.25.2.3.1.2.3",
value: {:object_identifier, [1, 3, 6, 1, 2, 1, 25, 2, 1, 3]}
},
# Fixed Disk: 1.3.6.1.2.1.25.2.1.4 (filtered out for memory)
%{oid: "1.3.6.1.2.1.25.2.3.1.2.31", value: {:object_identifier, [1, 3, 6, 1, 2, 1, 25, 2, 1, 4]}}
%{
oid: "1.3.6.1.2.1.25.2.3.1.2.31",
value: {:object_identifier, [1, 3, 6, 1, 2, 1, 25, 2, 1, 4]}
}
]}
# hrStorageDescr - descriptions
@ -1162,13 +1237,25 @@ defmodule Towerops.Snmp.Profiles.BaseTest do
{:ok,
[
# RAM (filtered out for storage)
%{oid: "1.3.6.1.2.1.25.2.3.1.2.1", value: {:object_identifier, [1, 3, 6, 1, 2, 1, 25, 2, 1, 2]}},
%{
oid: "1.3.6.1.2.1.25.2.3.1.2.1",
value: {:object_identifier, [1, 3, 6, 1, 2, 1, 25, 2, 1, 2]}
},
# Fixed Disk
%{oid: "1.3.6.1.2.1.25.2.3.1.2.31", value: {:object_identifier, [1, 3, 6, 1, 2, 1, 25, 2, 1, 4]}},
%{
oid: "1.3.6.1.2.1.25.2.3.1.2.31",
value: {:object_identifier, [1, 3, 6, 1, 2, 1, 25, 2, 1, 4]}
},
# Removable Disk
%{oid: "1.3.6.1.2.1.25.2.3.1.2.32", value: {:object_identifier, [1, 3, 6, 1, 2, 1, 25, 2, 1, 5]}},
%{
oid: "1.3.6.1.2.1.25.2.3.1.2.32",
value: {:object_identifier, [1, 3, 6, 1, 2, 1, 25, 2, 1, 5]}
},
# Network Disk
%{oid: "1.3.6.1.2.1.25.2.3.1.2.33", value: {:object_identifier, [1, 3, 6, 1, 2, 1, 25, 2, 1, 10]}}
%{
oid: "1.3.6.1.2.1.25.2.3.1.2.33",
value: {:object_identifier, [1, 3, 6, 1, 2, 1, 25, 2, 1, 10]}
}
]}
# hrStorageDescr
@ -1258,16 +1345,46 @@ defmodule Towerops.Snmp.Profiles.BaseTest do
{:ok,
[
# hrStorageType values 1-10 (other, ram, virtual_memory, fixed_disk, etc.)
%{oid: "1.3.6.1.2.1.25.2.3.1.2.1", value: {:object_identifier, [1, 3, 6, 1, 2, 1, 25, 2, 1, 1]}},
%{oid: "1.3.6.1.2.1.25.2.3.1.2.2", value: {:object_identifier, [1, 3, 6, 1, 2, 1, 25, 2, 1, 2]}},
%{oid: "1.3.6.1.2.1.25.2.3.1.2.3", value: {:object_identifier, [1, 3, 6, 1, 2, 1, 25, 2, 1, 3]}},
%{oid: "1.3.6.1.2.1.25.2.3.1.2.4", value: {:object_identifier, [1, 3, 6, 1, 2, 1, 25, 2, 1, 4]}},
%{oid: "1.3.6.1.2.1.25.2.3.1.2.5", value: {:object_identifier, [1, 3, 6, 1, 2, 1, 25, 2, 1, 5]}},
%{oid: "1.3.6.1.2.1.25.2.3.1.2.6", value: {:object_identifier, [1, 3, 6, 1, 2, 1, 25, 2, 1, 6]}},
%{oid: "1.3.6.1.2.1.25.2.3.1.2.7", value: {:object_identifier, [1, 3, 6, 1, 2, 1, 25, 2, 1, 7]}},
%{oid: "1.3.6.1.2.1.25.2.3.1.2.8", value: {:object_identifier, [1, 3, 6, 1, 2, 1, 25, 2, 1, 8]}},
%{oid: "1.3.6.1.2.1.25.2.3.1.2.9", value: {:object_identifier, [1, 3, 6, 1, 2, 1, 25, 2, 1, 9]}},
%{oid: "1.3.6.1.2.1.25.2.3.1.2.10", value: {:object_identifier, [1, 3, 6, 1, 2, 1, 25, 2, 1, 10]}}
%{
oid: "1.3.6.1.2.1.25.2.3.1.2.1",
value: {:object_identifier, [1, 3, 6, 1, 2, 1, 25, 2, 1, 1]}
},
%{
oid: "1.3.6.1.2.1.25.2.3.1.2.2",
value: {:object_identifier, [1, 3, 6, 1, 2, 1, 25, 2, 1, 2]}
},
%{
oid: "1.3.6.1.2.1.25.2.3.1.2.3",
value: {:object_identifier, [1, 3, 6, 1, 2, 1, 25, 2, 1, 3]}
},
%{
oid: "1.3.6.1.2.1.25.2.3.1.2.4",
value: {:object_identifier, [1, 3, 6, 1, 2, 1, 25, 2, 1, 4]}
},
%{
oid: "1.3.6.1.2.1.25.2.3.1.2.5",
value: {:object_identifier, [1, 3, 6, 1, 2, 1, 25, 2, 1, 5]}
},
%{
oid: "1.3.6.1.2.1.25.2.3.1.2.6",
value: {:object_identifier, [1, 3, 6, 1, 2, 1, 25, 2, 1, 6]}
},
%{
oid: "1.3.6.1.2.1.25.2.3.1.2.7",
value: {:object_identifier, [1, 3, 6, 1, 2, 1, 25, 2, 1, 7]}
},
%{
oid: "1.3.6.1.2.1.25.2.3.1.2.8",
value: {:object_identifier, [1, 3, 6, 1, 2, 1, 25, 2, 1, 8]}
},
%{
oid: "1.3.6.1.2.1.25.2.3.1.2.9",
value: {:object_identifier, [1, 3, 6, 1, 2, 1, 25, 2, 1, 9]}
},
%{
oid: "1.3.6.1.2.1.25.2.3.1.2.10",
value: {:object_identifier, [1, 3, 6, 1, 2, 1, 25, 2, 1, 10]}
}
]}
"1.3.6.1.2.1.25.2.3.1.3" ->
@ -1316,7 +1433,9 @@ defmodule Towerops.Snmp.Profiles.BaseTest do
for {idx, expected_type} <- expected_types do
entry = Enum.find(storage, &(&1.storage_index == idx))
assert entry.storage_type == expected_type, "Index #{idx} should have type #{expected_type}"
assert entry.storage_type == expected_type,
"Index #{idx} should have type #{expected_type}"
end
end
end
@ -1329,7 +1448,10 @@ defmodule Towerops.Snmp.Profiles.BaseTest do
"1.3.6.1.2.1.47.1.1.1.1.2" ->
{:ok,
[
%{oid: "1.3.6.1.2.1.47.1.1.1.1.2.1", value: {:octet_string, "Cisco 2960-X Chassis"}},
%{
oid: "1.3.6.1.2.1.47.1.1.1.1.2.1",
value: {:octet_string, "Cisco 2960-X Chassis"}
},
%{oid: "1.3.6.1.2.1.47.1.1.1.1.2.2", value: {:octet_string, "Power Supply 1"}},
%{oid: "1.3.6.1.2.1.47.1.1.1.1.2.3", value: {:octet_string, "Fan Tray 1"}}
]}
@ -1501,7 +1623,9 @@ defmodule Towerops.Snmp.Profiles.BaseTest do
for {expected_class, idx} <- Enum.with_index(expected_classes, 1) do
entity = Enum.find(entities, &(&1.entity_index == idx))
assert entity.entity_class == expected_class, "Entity #{idx} should have class #{expected_class}"
assert entity.entity_class == expected_class,
"Entity #{idx} should have class #{expected_class}"
end
end
end
@ -1521,8 +1645,14 @@ defmodule Towerops.Snmp.Profiles.BaseTest do
expect(SnmpMock, :walk, fn _, "1.3.6.1.2.1.25.3.2.1.3", _ ->
{:ok,
[
%{oid: "1.3.6.1.2.1.25.3.2.1.3.768", value: {:octet_string, "Intel Core i7-9700 @ 3.00GHz"}},
%{oid: "1.3.6.1.2.1.25.3.2.1.3.769", value: {:octet_string, "Intel Core i7-9700 @ 3.00GHz"}}
%{
oid: "1.3.6.1.2.1.25.3.2.1.3.768",
value: {:octet_string, "Intel Core i7-9700 @ 3.00GHz"}
},
%{
oid: "1.3.6.1.2.1.25.3.2.1.3.769",
value: {:octet_string, "Intel Core i7-9700 @ 3.00GHz"}
}
]}
end)
@ -1658,4 +1788,199 @@ defmodule Towerops.Snmp.Profiles.BaseTest do
assert cpu.load_percent == 50.0
end
end
# Helper to build a sensor mock for a single sensor at index 1 with given scale.
# sensor_type=8 (celsius), precision=0, raw_value as given, status=1 (ok).
defp mock_single_sensor(scale, raw_value) do
stub(SnmpMock, :walk, fn _, oid, _ ->
case oid do
"1.3.6.1.2.1.99.1.1.1.1" ->
{:ok, [%{oid: "1.3.6.1.2.1.99.1.1.1.1.1", value: {:integer, 8}}]}
_ ->
{:ok, []}
end
end)
stub(SnmpMock, :get_multiple, fn _, oids, _ ->
result_map = Map.new(oids, &{&1, sensor_oid_value(&1, scale, raw_value)})
{:ok, result_map}
end)
end
defp sensor_oid_value("1.3.6.1.2.1.99.1.1.1.1.1", _scale, _raw), do: {:integer, 8}
defp sensor_oid_value("1.3.6.1.2.1.99.1.1.1.2.1", scale, _raw), do: {:integer, scale}
defp sensor_oid_value("1.3.6.1.2.1.99.1.1.1.3.1", _scale, _raw), do: {:integer, 0}
defp sensor_oid_value("1.3.6.1.2.1.99.1.1.1.4.1", _scale, raw), do: {:integer, raw}
defp sensor_oid_value("1.3.6.1.2.1.99.1.1.1.5.1", _scale, _raw), do: {:integer, 1}
defp sensor_oid_value(_oid, _scale, _raw), do: {:error, :no_such_object}
describe "ENTITY-SENSOR-MIB scale values (scale_to_divisor via discover_sensors)" do
# Tests for all 9 negative/zero scale values handled by scale_to_divisor/1.
# For each test: raw_value / expected_divisor = expected_scaled_value.
# precision=0 so calculate_divisor(scale, 0) = 10^(0 - scale) for scale <= 0.
# Task 5.1 — Property test: all negative ENTITY-SENSOR-MIB scale values produce sensor_divisor > 1
test "all negative ENTITY-SENSOR-MIB scale values produce sensor_divisor > 1" do
negative_scales = [-24, -21, -18, -15, -12, -9, -6, -3]
Enum.each(negative_scales, fn scale ->
mock_single_sensor(scale, 1000)
assert {:ok, [sensor]} = Base.discover_sensors(@client_opts)
assert sensor.sensor_divisor > 1,
"Expected divisor > 1 for scale #{scale}, got #{sensor.sensor_divisor}"
end)
end
# Task 5.1 — Property test: scale 0 produces divisor exactly 1
test "scale 0 (none) produces sensor_divisor exactly 1" do
mock_single_sensor(0, 42)
assert {:ok, [sensor]} = Base.discover_sensors(@client_opts)
assert sensor.sensor_divisor == 1
end
test "scale 0 (none): divisor is 1, value unchanged" do
mock_single_sensor(0, 42)
assert {:ok, [sensor]} = Base.discover_sensors(@client_opts)
assert sensor.sensor_divisor == 1
assert sensor.last_value == 42.0
end
test "scale -3 (milli): divisor is 1_000, raw 400 → 0.4" do
mock_single_sensor(-3, 400)
assert {:ok, [sensor]} = Base.discover_sensors(@client_opts)
assert sensor.sensor_divisor == 1_000
assert sensor.last_value == 400.0
# Scaled value: 400 / 1_000 = 0.4
assert sensor.last_value / sensor.sensor_divisor == 0.4
end
test "scale -6 (micro): divisor is 1_000_000, raw 1_000_000 → 1.0" do
mock_single_sensor(-6, 1_000_000)
assert {:ok, [sensor]} = Base.discover_sensors(@client_opts)
assert sensor.sensor_divisor == 1_000_000
assert sensor.last_value / sensor.sensor_divisor == 1.0
end
test "scale -9 (nano): divisor is 1_000_000_000" do
mock_single_sensor(-9, 1_000_000_000)
assert {:ok, [sensor]} = Base.discover_sensors(@client_opts)
assert sensor.sensor_divisor == 1_000_000_000
assert sensor.last_value / sensor.sensor_divisor == 1.0
end
test "scale -12 (pico): divisor is 1_000_000_000_000" do
mock_single_sensor(-12, 1_000_000_000_000)
assert {:ok, [sensor]} = Base.discover_sensors(@client_opts)
assert sensor.sensor_divisor == 1_000_000_000_000
assert sensor.last_value / sensor.sensor_divisor == 1.0
end
test "scale -15 (femto): divisor is 1_000_000_000_000_000" do
mock_single_sensor(-15, 1_000_000_000_000_000)
assert {:ok, [sensor]} = Base.discover_sensors(@client_opts)
assert sensor.sensor_divisor == 1_000_000_000_000_000
assert sensor.last_value / sensor.sensor_divisor == 1.0
end
test "scale -18 (atto): divisor is 1_000_000_000_000_000_000" do
mock_single_sensor(-18, 1_000_000_000_000_000_000)
assert {:ok, [sensor]} = Base.discover_sensors(@client_opts)
assert sensor.sensor_divisor == 1_000_000_000_000_000_000
assert sensor.last_value / sensor.sensor_divisor == 1.0
end
test "scale -21 (zepto): divisor is 1_000_000_000_000_000_000_000" do
mock_single_sensor(-21, 1_000_000_000_000_000_000_000)
assert {:ok, [sensor]} = Base.discover_sensors(@client_opts)
assert sensor.sensor_divisor == 1_000_000_000_000_000_000_000
assert sensor.last_value / sensor.sensor_divisor == 1.0
end
test "scale -24 (yocto): divisor is 1_000_000_000_000_000_000_000_000" do
mock_single_sensor(-24, 1_000_000_000_000_000_000_000_000)
assert {:ok, [sensor]} = Base.discover_sensors(@client_opts)
assert sensor.sensor_divisor == 1_000_000_000_000_000_000_000_000
assert sensor.last_value / sensor.sensor_divisor == 1.0
end
# Positive scale values (kilo/mega/giga/tera) fall through to scale_to_divisor(_), do: 1
# because calculate_divisor(3, 0) = 10^(0-3) = negative exponent → scale_to_divisor(3) = 1
test "scale 3 (kilo): falls back to divisor 1 (no fractional divisor support)" do
mock_single_sensor(3, 5)
assert {:ok, [sensor]} = Base.discover_sensors(@client_opts)
assert sensor.sensor_divisor == 1
end
test "scale 6 (mega): falls back to divisor 1" do
mock_single_sensor(6, 5)
assert {:ok, [sensor]} = Base.discover_sensors(@client_opts)
assert sensor.sensor_divisor == 1
end
test "scale 9 (giga): falls back to divisor 1" do
mock_single_sensor(9, 5)
assert {:ok, [sensor]} = Base.discover_sensors(@client_opts)
assert sensor.sensor_divisor == 1
end
test "scale 12 (tera): falls back to divisor 1" do
mock_single_sensor(12, 5)
assert {:ok, [sensor]} = Base.discover_sensors(@client_opts)
assert sensor.sensor_divisor == 1
end
end
describe "ENTITY-SENSOR-MIB scale regression tests (already-correct values)" do
# Regression tests for the four scale values that were already correct
# before the bugfix: scale 0, -3, -6, -9.
test "regression: scale 0 (none) — temperature sensor, raw 25 → 25.0°C" do
mock_single_sensor(0, 25)
assert {:ok, [sensor]} = Base.discover_sensors(@client_opts)
assert sensor.sensor_divisor == 1
assert sensor.last_value == 25.0
assert sensor.last_value / sensor.sensor_divisor == 25.0
end
test "regression: scale -3 (milli) — voltage sensor, raw 3300 → 3.3V" do
mock_single_sensor(-3, 3300)
assert {:ok, [sensor]} = Base.discover_sensors(@client_opts)
assert sensor.sensor_divisor == 1_000
assert sensor.last_value == 3300.0
assert sensor.last_value / sensor.sensor_divisor == 3.3
end
test "regression: scale -6 (micro) — current sensor, raw 500_000 → 0.5A" do
mock_single_sensor(-6, 500_000)
assert {:ok, [sensor]} = Base.discover_sensors(@client_opts)
assert sensor.sensor_divisor == 1_000_000
assert sensor.last_value / sensor.sensor_divisor == 0.5
end
test "regression: scale -9 (nano) — frequency sensor, raw 2_400_000_000 → 2.4" do
mock_single_sensor(-9, 2_400_000_000)
assert {:ok, [sensor]} = Base.discover_sensors(@client_opts)
assert sensor.sensor_divisor == 1_000_000_000
assert sensor.last_value / sensor.sensor_divisor == 2.4
end
end
end