This commit addresses Phase 1 (CRITICAL - Data Loss Prevention) issues
from the comprehensive bugs and inconsistencies analysis.
CRITICAL FIXES:
1. Protobuf Schema Inconsistencies
- Added missing job_id field (field 5) to SnmpResult message
- Fixed AgentError message structure (job_id, message, timestamp)
- Removed obsolete transport field from SnmpDevice struct
- Regenerated protobuf code with protoc-gen-elixir
- Prevents crashes when agent sends SNMP results or errors
2. Interface/Sensor Discovery Timeout Data Loss
- Changed timeout behavior to return error instead of empty list
- Prevents deletion of ALL existing interfaces/sensors on slow networks
- Discovery fails gracefully instead of destroying historical data
- Addresses commit 7a57f7c sensor discovery pipeline data loss issue
3. Polling Offset Schedule Mismatch
- Fixed schedule_next_poll() to use offset only (not interval + offset)
- Maintains consistent polling intervals across all executions
- Prevents load bunching that offset was designed to prevent
- Example: 300s interval with offset=94s now consistently polls every 5m
4. Always Reschedule Race Condition
- Added should_continue_polling?() and should_continue_monitoring?()
- Only reschedule if device is still enabled and should be polled by Phoenix
- Prevents zombie jobs from continuing to poll disabled/deleted/reassigned devices
- Stops race condition where stop_polling() is bypassed by in-flight jobs
Files Changed:
- priv/proto/agent.proto
- lib/towerops/proto/agent.pb.ex (regenerated)
- lib/towerops_web/channels/agent_channel.ex
- lib/towerops/snmp/discovery.ex
- lib/towerops/workers/device_poller_worker.ex
- lib/towerops/workers/device_monitor_worker.ex
- test/towerops/workers/device_poller_worker_test.exs
Test Results: All passing
- device_poller_worker_test.exs: 8 tests, 0 failures
- device_monitor_worker_test.exs: 3 tests, 0 failures
- discovery_test.exs: 13 tests, 0 failures
Remaining Phases:
- Phase 2 (HIGH): 8 data integrity issues
- Phase 3 (MEDIUM): 10 reliability issues
- Phase 4 (LOW): 3 cleanup issues
Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Added integration tests for real-time updates and metrics display.
Also fixed dialyzer warnings in worker event broadcasting by removing
unreachable error handling branches.
Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Add LiveView for real-time job monitoring dashboard:
- Subscribe to job:lifecycle PubSub topic for live updates
- Display health metrics (completed, failed, avg duration, active jobs)
- Show active operations with real-time updates
- Display problems section (stuck/failed job counts)
- Add router entry under /admin/monitoring
Tests verify PubSub subscription and basic rendering.
Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Create JobMonitoring.Metrics module with calculate_all/0 to provide
comprehensive metrics including execution counts, success rates,
queue depths, and average execution times.
Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Add list_failed_jobs/0 to return retryable and cancelled jobs
and list_recent_completions/1 to return completed, cancelled,
or discarded jobs with configurable limit.
Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Fixes discovery failures on enterprise network devices with many
interfaces and routing table entries.
Issue:
Discovery was failing with "too_many_oids - OID values map exceeds
1000 entries" on SNMPv3 devices. Enterprise routers/switches during
discovery can easily return 10k-50k OIDs from:
- 100+ interfaces with 20-30 OIDs each
- System info, routing tables, ARP tables, neighbor tables
- Various MIB objects
The original limit of 1,000 was too restrictive for real-world devices
while still being intended as DoS protection.
Changes:
- Increase @max_oid_values from 1,000 to 50,000
- Update test to generate 50,100 OIDs (exceeds new limit)
- Add comment explaining why limit is set to 50k
This maintains DoS protection (prevents multi-million OID attacks)
while accommodating realistic device discovery scenarios.
All tests passing (5578 tests, 0 failures).
Continues the pattern of using custom Ecto types to create rich domain objects
and avoid primitive obsession. Adds two new custom types following the
established pattern from IPAddress.
## MacAddress Custom Type
Provides structured MAC address handling with:
- Multiple input format support (colon, hyphen, dot-separated, compact)
- Normalization to colon-separated lowercase (canonical format)
- SNMP binary format conversion (6-byte binary)
- Format conversion helpers (:colon, :hyphen, :dot, :compact)
- Comprehensive validation and error handling
### Supported Formats
- Colon: `00:11:22:33:44:55`
- Hyphen: `00-11-22-33-44-55`
- Dot (Cisco): `0011.2233.4455`
- Compact: `001122334455`
- SNMP binary: `<<0, 17, 34, 51, 68, 85>>`
All formats normalize to lowercase colon-separated format for consistency.
## SnmpOid Custom Type
Provides structured SNMP OID handling with:
- OID format validation (numeric and named)
- Normalization to dotted-decimal with leading dot
- OID resolution via SnmpKit (named → numeric)
- Helper functions (parent, child_of?, append)
- Integer list representation support
### Supported Formats
- Dotted-decimal: `.1.3.6.1.2.1.1.1.0`
- Numeric (no leading dot): `1.3.6.1.2.1.1.1.0`
- Named OID: `sysDescr.0` (with automatic resolution)
- Integer list: `[1, 3, 6, 1, 2, 1, 1, 1, 0]`
## Benefits
- **Type Safety**: Structured data with validation at cast time
- **Normalization**: Consistent storage format regardless of input
- **Rich API**: Helper functions for common operations
- **Zero Migration Cost**: No database changes needed
- **Better Errors**: Clear validation errors instead of DB constraint violations
- **Domain Modeling**: Code works with %MacAddress{} and %SnmpOid{} instead of strings
## Database Storage
Both types store as VARCHAR in the database:
- MacAddress: VARCHAR(17) - colon-separated format
- SnmpOid: VARCHAR(255) - dotted-decimal format with leading dot
## Test Coverage
Added comprehensive test suites:
- MacAddress: 47 tests covering all formats, roundtrips, edge cases
- SnmpOid: 49 tests covering OID operations, validation, helpers
- All 96 tests passing ✅
## Usage Example
```elixir
# Before (primitive obsession)
field :mac_address, :string
field :sensor_oid, :string
# After (rich domain types)
field :mac_address, Towerops.EctoTypes.MacAddress
field :sensor_oid, Towerops.EctoTypes.SnmpOid
# Usage
changeset
|> cast(attrs, [:mac_address, :sensor_oid])
# Validation happens automatically in cast/1
```
Files:
- lib/towerops/ecto_types/mac_address.ex (330 lines)
- lib/towerops/ecto_types/snmp_oid.ex (340 lines)
- test/towerops/ecto_types/mac_address_test.exs (380 lines)
- test/towerops/ecto_types/snmp_oid_test.exs (390 lines)
Implements comprehensive validation layer on top of protobuf schema to
prevent malformed data, DoS attacks, and business logic violations.
Changes:
- Created Towerops.Agent.Validator module with strict validation for all
agent message types (AgentHeartbeat, MetricBatch, SnmpResult, AgentError,
CredentialTestResult, MikrotikResult)
- Added validation limits to prevent memory exhaustion and DoS attacks
(max string lengths, collection sizes, numeric ranges)
- Added format validation for UUIDs, IP addresses, versions, hostnames
- Added enum validation for status and protocol fields
- Modified AgentChannel handlers to validate all incoming messages before
processing
- Added comprehensive typespecs to AgentChannel for better static analysis
- Created safe_base64_decode/1 helper with error handling
- Added 32 comprehensive tests covering valid cases, invalid inputs, and
edge cases
Security improvements:
- Prevents memory exhaustion via oversized strings and collections
- Validates all numeric fields are within expected ranges
- Checks UUID format for all ID fields
- Validates IP addresses and hostnames against RFC standards
- Rejects future timestamps and excessive values
Backward compatibility:
- Empty version and hostname strings allowed for older agents
Improve coverage from 37.97% to 40.64% with tests for:
- PDU types in SNMPv3 messages (get_next, get_response, set, getbulk)
- Context engine ID and context name (empty, non-empty, both)
- Message max size variations (minimum 484, custom values)
- Multiple varbinds in v3 messages
- Error responses with various error_status values (0-5)
- Reportable flag variations (true/false)
- USM security model
- All SNMP value types (counter64, gauge32, ip_address, opaque, oid)
- Exception values (no_such_object, no_such_instance, end_of_mib_view)
- Message ID edge cases (0, max 2147483647)
- Discovery message consistency
Tests verify message encoding completes without crashes
even when V3 security processing is involved.
Coverage: 1,391 → 1,419 tests, all passing
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Added complete test coverage for MIB.Error module (16.13% -> target 100%):
**Error Creation:**
- All supported error types (syntax, semantic, import, type, constraint, etc.)
- Error creation with line and column information
- Error creation with context maps
- Suggestion generation for common mistakes
**Error Types Tested:**
- unexpected_token with various combinations (expected/actual/value)
- unexpected_eof with expected values
- unterminated_string
- invalid_number and invalid_identifier
- file_not_found with path
- import_error with symbol and module
- duplicate_definition with name
- Generic error types
**Suggestion Generation:**
- MAX-ACCESS vs ACCESS typo
- deprecated 'mandatory' vs 'current'
- OBJECT-TYPE vs OBJECT-IDENTITY confusion
- Unterminated string hints
- File not found with RFC/MIB name detection
- Import error dependency hints
**Error Formatting:**
- Format with/without line and column
- Format with/without suggestions
- Multiple suggestion formatting
- Edge cases (nil values, empty strings, non-standard types)
**Edge Cases:**
- All nil fields
- Large line numbers
- Zero/negative line/column values
- Various context data types
- Mixed token value types
- Empty options handling
53 new tests added
Test count: 1,211 -> 1,264 (all passing)
Added comprehensive test coverage for previously untested functionality:
- IP address parsing (valid IPs, invalid inputs, IPv6 rejection)
- Zero-filling behavior for incomplete IPs (Erlang :inet standard)
- Type inference for all SNMP types (integers, strings, lists, booleans, nulls)
- Value decoding for all SNMP types (strings, integers, IPs, OIDs, exceptions)
- Value encoding with type inference and validation controls
- Unsigned32 validation (valid ranges, overflow, underflow)
- Coercion edge cases (IP addresses, unsigned32, octet_string, OIDs, booleans)
Fixes:
- Corrected type inference expectation for large integers (returns :unsigned32)
- Fixed boolean assertion to use equality check instead of pattern match
- Documented :inet.parse_address zero-fill behavior for incomplete IPs
Total: 97 tests, all passing
Fixed 2 failing tests in HostParser that were using unreliable hostname resolution. Changed tests to use malformed IP addresses that will always fail validation consistently.
Added tests for SnmpKit.SNMP bang methods (get_bulk!, bulk_walk!) which have actual implementation logic beyond delegation.
All 954 SNMPkit tests now pass (0 failures).
- Remove unused Ecto.Query import from device_poller_worker_test
- Update route paths from /orgs/:slug to /dashboard after navigation refactor
- Fix HostParser tests to match actual hostname resolution behavior
- Update UserAuth redirect assertions to expect /dashboard instead of /devices
- Fix DashboardLiveTest missing organization context
- Fix OrgLive.NewTest to verify organization creation correctly
All 4850 tests now passing with zero failures and zero warnings.
Changes:
- Fix traffic graph to handle nil interface octets gracefully
- Add comprehensive tests for nil octets scenarios
- Fix agent polling query to support devices assigned directly to org (left join on sites)
- Fix test SNMP connection to inherit credentials from site/org
- Refactor agent_channel process_polling_result to reduce nesting (Credo)
- Add alias for JobCleanupTask in application.ex (Credo)
This fixes crashes when interface stats have nil octets and ensures
devices without a site assignment can be polled by agents.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Extract duplicate access control logic from multiple LiveView files into
a reusable AccessControl helper module.
**Changes:**
- NEW: lib/towerops_web/live/helpers/access_control.ex
- NEW: test/towerops_web/live/helpers/access_control_test.exs
- Refactor device_live/index.ex to use AccessControl.verify_site_access/2
and verify_device_access/2
- Refactor device_live/form.ex to use AccessControl.verify_device_access/2
- Refactor alert_live/index.ex to use AccessControl.verify_alert_access/2
- Refactor device_live/show.ex to use AccessControl.verify_device_access/2
- Remove unused Repo aliases from refactored files
**Benefits:**
- Reduces code duplication across 7+ locations
- Centralizes security-critical access checks
- Improves testability (helper module has >90% coverage)
- Consistent error handling across all LiveViews
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Fixed production bug where visiting the edit page for a device with no
SNMP discovery data would crash with "expected boolean on left-side of
'and', got: nil" error.
The mikrotik_device?/1 function was using && operator which returns nil
when device.snmp_device is nil, instead of returning false. The template
conditional on line 455 requires a proper boolean value.
Changed from:
device.snmp_device && (...)
To:
not is_nil(device.snmp_device) and (...)
This ensures the function always returns a boolean (true/false) instead
of potentially returning nil.
Added regression test to verify the page renders correctly when a device
has no SNMP discovery data yet.
Fixes: https://app.honeybadger.io/projects/136860/faults/127120431
