carverauto/serviceradar

Fork 0

2222 feat integrate snmp checker into agent #2671

Merged

mfreeman451 merged 13 commits from refs/pull/2671/head into staging

2026-01-14 18:14:35 +00:00

mfreeman451 commented

2026-01-14 17:56:31 +00:00

(Migrated from github.com)

Owner

Imported from GitHub pull request.

Original GitHub pull request: #2300
Original author: @mfreeman451
Original URL: https://github.com/carverauto/serviceradar/pull/2300
Original created: 2026-01-14T17:56:31Z
Original updated: 2026-01-14T18:14:44Z
Original head: carverauto/serviceradar:2222-feat-integrate-snmp-checker-into-agent
Original base: staging
Original merged: 2026-01-14T18:14:35Z by @mfreeman451

User description

IMPORTANT: Please sign the Developer Certificate of Origin

Thank you for your contribution to ServiceRadar. Please note, when contributing, the developer must include
a DCO sign-off statement indicating the DCO acceptance in one commit message. Here
is an example DCO Signed-off-by line in a commit message:

Signed-off-by: J. Doe <j.doe@domain.com>

Describe your changes

Issue ticket number and link

Code checklist before requesting a review

I have signed the DCO?
The build completes without errors?
All tests are passing when running make test?

PR Type

Enhancement, Tests

Description

Integrates comprehensive SNMP monitoring capabilities into the ServiceRadar agent with full control plane support
Implements SNMP profile management system with support for SNMPv1, v2c, and v3 authentication protocols
Adds SRQL-based device targeting for dynamic profile assignment with priority-based resolution
Provides built-in OID templates organized by vendor (Standard, Cisco, Juniper, Arista) for common monitoring scenarios
Implements encrypted credential storage for SNMP community strings and SNMPv3 passwords using AES-256-GCM
Adds comprehensive LiveView UI for SNMP profile configuration with visual query builder and template browser
Implements agent-side SNMP service with support for local file, cache, and remote proto config sources
Includes extensive test coverage for profile management, config distribution, agent service, and integration scenarios
Extends protocol buffers with SNMP message types and enums for agent configuration delivery
Adds SNMP configuration routes and navigation menu items to web UI

Diagram Walkthrough

flowchart LR
  CP["Control Plane<br/>SNMP Profiles"]
  UI["Web UI<br/>Profile Management"]
  Compiler["SNMP Compiler<br/>Config Generation"]
  Proto["Protocol Buffers<br/>SNMP Messages"]
  Agent["Agent Service<br/>SNMP Monitoring"]
  Checker["SNMP Checker<br/>v1/v2c/v3"]
  
  CP -->|SRQL Targeting| Compiler
  UI -->|Create/Edit| CP
  Compiler -->|Serialize| Proto
  Proto -->|Deliver| Agent
  Agent -->|Execute| Checker
  Checker -->|Poll OIDs| Targets["Network Devices"]

File Walkthrough

Relevant files

Enhancement

21 files

index.ex `SNMP Profiles LiveView with Query Builder and Template Management` web-ng/lib/serviceradar_web_ng_web/live/settings/snmp_profiles_live/index.ex Implements comprehensive LiveView for SNMP profiles management with 2979 lines of new code Provides UI for managing SNMP profiles, targets, and OID templates with SRQL-based device targeting Includes visual query builder for interface targeting, SNMP connection testing, and OID template browser Supports SNMPv1, SNMPv2c, and SNMPv3 authentication with encrypted credential storage	+2979/-0
builtin_templates.ex `Built-in SNMP OID templates for vendor-specific monitoring` elixir/serviceradar_core/lib/serviceradar/snmp_profiles/builtin_templates.ex Introduces a new module providing 342 lines of built-in OID templates organized by vendor (Standard, Cisco, Juniper, Arista) Implements functions to retrieve all templates, filter by vendor, and seed templates into the database Provides template data for common SNMP monitoring scenarios (system info, interfaces, CPU, memory, environment sensors, BGP, etc.) Includes stable ID generation for UI references and error classification for seeding operations	+342/-0
snmp_profile.ex `SNMP profile resource with targeting and default management` elixir/serviceradar_core/lib/serviceradar/snmp_profiles/snmp_profile.ex Defines the `SNMPProfile` Ash resource for managing reusable SNMP monitoring configurations Implements profile attributes including polling interval, timeout, retries, and SRQL-based device targeting Provides actions for CRUD operations, setting default profiles, and listing targeting profiles ordered by priority Enforces policies restricting profile management to admins while allowing read access to all users	+262/-0
snmp_target.ex `SNMP target configuration with multi-version authentication` elixir/serviceradar_core/lib/serviceradar/snmp_profiles/snmp_target.ex Defines the `SNMPTarget` Ash resource representing individual network devices to poll via SNMP Supports SNMPv1, v2c, and v3 authentication with encrypted credential storage Implements credential encryption via custom change handler for community strings and v3 passwords Enforces unique target names per profile and admin-only modification policies	+241/-0
snmp_oid_config.ex `OID configuration resource with data type and scaling support` elixir/serviceradar_core/lib/serviceradar/snmp_profiles/snmp_oid_config.ex Defines the `SNMPOIDConfig` Ash resource for individual OID polling configurations Supports multiple data types (counter, gauge, boolean, bytes, string, float, timeticks) with scaling and delta calculation Implements unique OID per target constraint and bulk creation action Enforces admin-only modification with read access for all users	+184/-0
snmp_oid_template.ex `Reusable OID template definitions with vendor organization` elixir/serviceradar_core/lib/serviceradar/snmp_profiles/snmp_oid_template.ex Defines the `SNMPOIDTemplate` Ash resource for reusable OID template definitions Supports both built-in (read-only) and custom (user-created) templates organized by vendor and category Implements protection against modification of built-in templates via validation Provides filtering actions for templates by vendor and builtin status	+213/-0
srql_target_resolver.ex `SRQL-based profile targeting with device matching` elixir/serviceradar_core/lib/serviceradar/snmp_profiles/srql_target_resolver.ex Implements SRQL-based profile targeting resolution for SNMP profiles Validates device UIDs against UUID regex to prevent SRQL injection attacks Evaluates SRQL queries against device attributes and tags, supporting both device and interface targeting Returns the highest-priority matching profile or nil if no profiles match	+268/-0
snmp_compiler.ex `SNMP configuration compiler with profile resolution` elixir/serviceradar_core/lib/serviceradar/agent_config/compilers/snmp_compiler.ex Implements the `SNMPCompiler` module for transforming SNMP profiles into agent-consumable configuration Resolves profiles using SRQL targeting with fallback to default profile Compiles targets with OID configurations, handling credential decryption for v1/v2c and v3 authentication Formats protocol versions, security levels, and authentication protocols for agent consumption	+302/-0
validate_srql_query.ex `SRQL query validation for profile targeting` elixir/serviceradar_core/lib/serviceradar/snmp_profiles/changes/validate_srql_query.ex Implements validation change for SRQL `target_query` attributes in SNMP profiles Normalizes queries by adding `in:devices` prefix when needed, supporting both device and interface targeting Validates query syntax via SRQL NIF and adds error messages for invalid queries Treats empty strings as nil (no targeting)	+62/-0
set_as_default.ex `Default profile enforcement with automatic unset` elixir/serviceradar_core/lib/serviceradar/snmp_profiles/changes/set_as_default.ex Implements change handler to ensure only one default SNMP profile per tenant Automatically unsets existing default profiles before setting a new one Uses SystemActor for unset operations to maintain authorization consistency Extracts tenant ID from schema context for actor creation	+60/-0
compiler.ex `Compiler registry update for SNMP support` elixir/serviceradar_core/lib/serviceradar/agent_config/compiler.ex Adds `:snmp` to the `config_type` union type definition Registers `SNMPCompiler` in the compiler registry mapping	+3/-2
encrypt_credentials.ex `SNMP credential encryption change for Ash resources` elixir/serviceradar_core/lib/serviceradar/snmp_profiles/changes/encrypt_credentials.ex New Ash resource change module for encrypting SNMP credentials before storage Implements credential encryption for `community`, `auth_password`, and `priv_password` fields Uses Cloak/AES-256-GCM vault for encryption with write-only credential storage Handles empty values by clearing encrypted fields and errors with user-friendly messages	+51/-0
encrypt_passwords.ex `SNMPv3 password encryption change for Ash resources` elixir/serviceradar_core/lib/serviceradar/snmp_profiles/changes/encrypt_passwords.ex New Ash resource change module for encrypting SNMPv3 authentication and privacy passwords Intercepts plaintext `auth_password` and `priv_password` virtual attributes Encrypts passwords using Cloak and stores in `_encrypted` fields Provides error handling for encryption failures with field-level error messages	+47/-0
snmp_profiles.ex `SNMP profiles domain configuration module` elixir/serviceradar_core/lib/serviceradar/snmp_profiles.ex New Ash domain module for SNMP monitoring configuration management Registers four SNMP-related resources: `SNMPProfile`, `SNMPTarget`, `SNMPOIDConfig`, `SNMPOIDTemplate` Enables AshAdmin for admin interface and configures authorization settings Provides documentation on device targeting using SRQL queries	+45/-0
router.ex `SNMP profile configuration routes` web-ng/lib/serviceradar_web_ng_web/router.ex Added three new routes for SNMP profile configuration UI Routes handle index, new profile creation, and profile editing views Follows same pattern as existing sysmon profile routes	+5/-0
settings_components.ex `SNMP settings navigation menu item` web-ng/lib/serviceradar_web_ng_web/components/settings_components.ex Added SNMP navigation item to settings menu Links to `/settings/snmp` with active state detection Positioned alongside other configuration options like sysmon	+5/-0
monitoring.pb.go `SNMP protocol buffer definitions and message types` proto/monitoring.pb.go Added five new SNMP-related enums: `SNMPVersion`, `SNMPSecurityLevel`, `SNMPAuthProtocol`, `SNMPPrivProtocol`, `SNMPDataType` Added four new message types: `SNMPConfig`, `SNMPTargetConfig`, `SNMPv3Auth`, `SNMPOIDConfig` Extended `AgentConfigResponse` with `snmp_config` field for SNMP configuration delivery Updated enum and message type indices to accommodate new SNMP types	+821/-65
snmp_service.go `SNMP agent service implementation` pkg/agent/snmp_service.go New 784-line SNMP agent service implementation for embedded SNMP monitoring Manages SNMP service lifecycle with Start/Stop, config loading, and refresh loops Supports local file, cache, and remote proto config sources with change detection Implements concurrent-safe status reporting and proto config application with factory pattern	+784/-0
client.go `SNMPv3 client implementation and protocol conversion` pkg/checker/snmp/client.go Implemented SNMPv3 support in SNMP client initialization Added conversion functions for SNMPv3 security levels, auth protocols, and privacy protocols Maps internal types to gosnmp library types with proper error handling Validates SNMPv3 configuration requirements	+65/-1
server.go `SNMP service integration into agent server` pkg/agent/server.go Added SNMP service initialization in server startup Implemented `initSNMPService` method to create and start SNMP service Added `GetSNMPStatus` method for status reporting Integrated SNMP service shutdown in server Stop method	+47/-0
types.go `SNMPv3 type definitions and authentication structures` pkg/checker/snmp/types.go Added SNMPv3 type definitions: `SecurityLevel`, `AuthProtocol`, `PrivProtocol` enums Added `V3Auth` struct for SNMPv3 authentication parameters with sensitive field tags Extended `Target` struct with optional `V3Auth` field for SNMPv3 support	+55/-0

Tests

7 files

snmp_config_distribution_integration_test.exs `SNMP Config Distribution Integration Tests` elixir/serviceradar_core/test/serviceradar/edge/snmp_config_distribution_integration_test.exs Adds 438 lines of integration tests for SNMP configuration distribution from control plane to agent Tests SNMP profile compilation, SRQL-based device targeting, and agent config generation Validates SNMPv2c and SNMPv3 configuration handling with OID definitions Verifies disabled profiles and full agent config payload generation	+438/-0
snmp_profile_test.exs `SNMPProfile resource tests with CRUD validation` elixir/serviceradar_core/test/serviceradar/snmp_profiles/snmp_profile_test.exs Tests SNMPProfile resource structure, default values, and available actions Validates CRUD operations including profile creation with custom parameters Tests unique name constraint per tenant enforcement Tests `set_as_default` action ensuring only one default profile exists	+246/-0
srql_target_resolver_test.exs `SRQL target resolver tests with device matching` elixir/serviceradar_core/test/serviceradar/snmp_profiles/srql_target_resolver_test.exs Tests SRQL target resolver module structure and function exports Validates device UID format checking and error handling for invalid UUIDs Tests profile matching with hostname queries and priority-based resolution Tests filtering of disabled profiles and non-targeting profiles	+375/-0
snmp_compiler_test.exs `SNMPCompiler tests with profile and credential handling` elixir/serviceradar_core/test/serviceradar/snmp_profiles/snmp_compiler_test.exs Tests SNMPCompiler module structure, behavior implementation, and config type Validates disabled config structure and validation logic Tests compilation of profiles with targets and OIDs including v2c and v3 authentication Tests credential decryption and protocol/security level formatting	+335/-0
snmp_integration_test.go `SNMP agent service integration tests` pkg/agent/snmp_integration_test.go Comprehensive integration test suite for SNMP agent service (604 lines) Tests proto config application, enable/disable, idempotency, and updates Validates status reporting, SNMPv3 configuration, and config file reloading Includes concurrent operation tests for status calls and config updates	+604/-0
testing.go `SNMP testing utilities and mock factories` pkg/checker/snmp/testing.go New testing utilities for SNMP service with noop implementations Provides `noopCollector`, `noopAggregator`, and factory implementations Includes `NewMockServiceForTesting` helper for creating testable SNMP services	+103/-0
checker_integration_test.go `ICMP checker test updates for new API` pkg/agent/checker_integration_test.go Updated ICMP checker test to use new `NewICMPChecker` factory function Added logger dependency and proto request parameter to checker initialization Improved test assertions with string conversion for response validation	+14/-9

Configuration changes

4 files

config.exs `Register SNMPProfiles Domain in Ash Configuration` web-ng/config/config.exs Registers `ServiceRadar.SNMPProfiles` domain in both web-ng and serviceradar_core Ash configurations Adds SNMP profiles to the list of managed domains for proper resource initialization	+4/-2
20260114074955_add_snmp_profiles.exs `Database schema for SNMP and system monitoring profiles` elixir/serviceradar_core/priv/repo/tenant_migrations/20260114074955_add_snmp_profiles.exs Creates database tables for SNMP infrastructure: `snmp_profiles`, `snmp_targets`, `snmp_oid_configs`, `snmp_oid_templates` Adds `sysmon_profiles` table for system monitoring profiles with similar targeting capabilities Implements foreign key relationships with cascade delete and unique constraints per tenant Adds `config_source` column to `ocsf_agents` table for tracking configuration origin	+229/-0
config.exs `Configuration update for SNMP profiles domain` elixir/serviceradar_core/config/config.exs Adds `ServiceRadar.SNMPProfiles` to the list of domains for Ash resource discovery	+2/-1
test.exs `SNMP profiles domain test configuration` elixir/serviceradar_core/config/test.exs Added `ServiceRadar.SNMPProfiles` to the list of domains in test configuration Enables SNMP profile domain for testing	+1/-0

Formatting

3 files

index.ex `Whitespace formatting adjustment` web-ng/lib/serviceradar_web_ng_web/live/settings/networks_live/index.ex Minor whitespace adjustment in host status display text Changed spacing in "of hosts" text from single to double space	+1/-1
index.ex `Code formatting improvements` web-ng/lib/serviceradar_web_ng_web/live/log_live/index.ex Added blank lines for code readability in two functions Improves formatting around error handling and default return values	+2/-0
index.ex `Code formatting improvement` web-ng/lib/serviceradar_web_ng_web/live/analytics_live/index.ex Added blank line for code readability in analytics stats function Improves formatting around error handling	+1/-0

Additional files

18 files

snmp.md	+182/-0
20260114074956.json	+354/-0
20260114074956.json	+177/-0
20260114074956.json	+165/-0
20260114074956.json	+194/-0
20260114074956.json	+249/-0
20260114074956.json	+257/-0
design.md	+379/-0
proposal.md	+42/-0
spec.md	+120/-0
spec.md	+131/-0
tasks.md	+125/-0
snmp_service_test.go	+550/-0
sysmon_service.go	+2/-0
types.go	+1/-0
config.go	+62/-0
service.go	+16/-1
monitoring.proto	+112/-0

Imported from GitHub pull request. Original GitHub pull request: #2300 Original author: @mfreeman451 Original URL: https://github.com/carverauto/serviceradar/pull/2300 Original created: 2026-01-14T17:56:31Z Original updated: 2026-01-14T18:14:44Z Original head: carverauto/serviceradar:2222-feat-integrate-snmp-checker-into-agent Original base: staging Original merged: 2026-01-14T18:14:35Z by @mfreeman451 --- ### **User description** ## IMPORTANT: Please sign the Developer Certificate of Origin Thank you for your contribution to ServiceRadar. Please note, when contributing, the developer must include a [DCO sign-off statement]( https://developercertificate.org/) indicating the DCO acceptance in one commit message. Here is an example DCO Signed-off-by line in a commit message: ``` Signed-off-by: J. Doe <j.doe@domain.com> ``` ## Describe your changes ## Issue ticket number and link ## Code checklist before requesting a review - [ ] I have signed the DCO? - [ ] The build completes without errors? - [ ] All tests are passing when running make test? ___ ### **PR Type** Enhancement, Tests ___ ### **Description** - Integrates comprehensive SNMP monitoring capabilities into the ServiceRadar agent with full control plane support - Implements SNMP profile management system with support for SNMPv1, v2c, and v3 authentication protocols - Adds SRQL-based device targeting for dynamic profile assignment with priority-based resolution - Provides built-in OID templates organized by vendor (Standard, Cisco, Juniper, Arista) for common monitoring scenarios - Implements encrypted credential storage for SNMP community strings and SNMPv3 passwords using AES-256-GCM - Adds comprehensive LiveView UI for SNMP profile configuration with visual query builder and template browser - Implements agent-side SNMP service with support for local file, cache, and remote proto config sources - Includes extensive test coverage for profile management, config distribution, agent service, and integration scenarios - Extends protocol buffers with SNMP message types and enums for agent configuration delivery - Adds SNMP configuration routes and navigation menu items to web UI ___ ### Diagram Walkthrough ```mermaid flowchart LR CP["Control Plane SNMP Profiles"] UI["Web UI Profile Management"] Compiler["SNMP Compiler Config Generation"] Proto["Protocol Buffers SNMP Messages"] Agent["Agent Service SNMP Monitoring"] Checker["SNMP Checker v1/v2c/v3"] CP -->|SRQL Targeting| Compiler UI -->|Create/Edit| CP Compiler -->|Serialize| Proto Proto -->|Deliver| Agent Agent -->|Execute| Checker Checker -->|Poll OIDs| Targets["Network Devices"] ``` <details><summary><h3>File Walkthrough</h3></summary> <table><thead><tr><th></th><th align="left">Relevant files</th></tr></thead><tbody><tr><td>Enhancement</td><td><details><summary>21 files</summary><table> <tr> <td> <details> <summary>index.ex<dd><code>SNMP Profiles LiveView with Query Builder and Template Management</code></dd></summary> <hr> web-ng/lib/serviceradar_web_ng_web/live/settings/snmp_profiles_live/index.ex <ul><li>Implements comprehensive LiveView for SNMP profiles management with 2979 lines of new code <li> Provides UI for managing SNMP profiles, targets, and OID templates with SRQL-based device targeting <li> Includes visual query builder for interface targeting, SNMP connection testing, and OID template browser <li> Supports SNMPv1, SNMPv2c, and SNMPv3 authentication with encrypted credential storage</ul> </details> </td> <td><a href="https://github.com/carverauto/serviceradar/pull/2300/files#diff-1493b4100054d62c0d3fdc343e3fb7ce5f21735f752fad7d0fc978ae1ba114e4">+2979/-0</a></td> </tr> <tr> <td> <details> <summary>builtin_templates.ex<dd><code>Built-in SNMP OID templates for vendor-specific monitoring</code></dd></summary> <hr> elixir/serviceradar_core/lib/serviceradar/snmp_profiles/builtin_templates.ex <ul><li>Introduces a new module providing 342 lines of built-in OID templates organized by vendor (Standard, Cisco, Juniper, Arista) <li> Implements functions to retrieve all templates, filter by vendor, and seed templates into the database <li> Provides template data for common SNMP monitoring scenarios (system info, interfaces, CPU, memory, environment sensors, BGP, etc.) <li> Includes stable ID generation for UI references and error classification for seeding operations</ul> </details> </td> <td><a href="https://github.com/carverauto/serviceradar/pull/2300/files#diff-dfb09a552c8a813238fd1455326888586384c927450dedf18a30344548e3c683">+342/-0</a>  </td> </tr> <tr> <td> <details> <summary>snmp_profile.ex<dd><code>SNMP profile resource with targeting and default management</code></dd></summary> <hr> elixir/serviceradar_core/lib/serviceradar/snmp_profiles/snmp_profile.ex <ul><li>Defines the <code>SNMPProfile</code> Ash resource for managing reusable SNMP monitoring configurations <li> Implements profile attributes including polling interval, timeout, retries, and SRQL-based device targeting <li> Provides actions for CRUD operations, setting default profiles, and listing targeting profiles ordered by priority <li> Enforces policies restricting profile management to admins while allowing read access to all users</ul> </details> </td> <td><a href="https://github.com/carverauto/serviceradar/pull/2300/files#diff-d829077b86a0e73a28a3b0ba483be2e13e79771c1c1c9661ddc33552441b476c">+262/-0</a>  </td> </tr> <tr> <td> <details> <summary>snmp_target.ex<dd><code>SNMP target configuration with multi-version authentication</code></dd></summary> <hr> elixir/serviceradar_core/lib/serviceradar/snmp_profiles/snmp_target.ex <ul><li>Defines the <code>SNMPTarget</code> Ash resource representing individual network devices to poll via SNMP <li> Supports SNMPv1, v2c, and v3 authentication with encrypted credential storage <li> Implements credential encryption via custom change handler for community strings and v3 passwords <li> Enforces unique target names per profile and admin-only modification policies</ul> </details> </td> <td><a href="https://github.com/carverauto/serviceradar/pull/2300/files#diff-a7472ec491caa80d337ec0334013e0a936533d65cd3c84260ce5f6131d159d6d">+241/-0</a>  </td> </tr> <tr> <td> <details> <summary>snmp_oid_config.ex<dd><code>OID configuration resource with data type and scaling support</code></dd></summary> <hr> elixir/serviceradar_core/lib/serviceradar/snmp_profiles/snmp_oid_config.ex <ul><li>Defines the <code>SNMPOIDConfig</code> Ash resource for individual OID polling configurations <li> Supports multiple data types (counter, gauge, boolean, bytes, string, float, timeticks) with scaling and delta calculation <li> Implements unique OID per target constraint and bulk creation action <li> Enforces admin-only modification with read access for all users</ul> </details> </td> <td><a href="https://github.com/carverauto/serviceradar/pull/2300/files#diff-6aa0fe00d2c5f5ef6ec080fd9319ae4a3565d41709967832695930a133e23563">+184/-0</a>  </td> </tr> <tr> <td> <details> <summary>snmp_oid_template.ex<dd><code>Reusable OID template definitions with vendor organization</code></dd></summary> <hr> elixir/serviceradar_core/lib/serviceradar/snmp_profiles/snmp_oid_template.ex <ul><li>Defines the <code>SNMPOIDTemplate</code> Ash resource for reusable OID template definitions <li> Supports both built-in (read-only) and custom (user-created) templates organized by vendor and category <li> Implements protection against modification of built-in templates via validation <li> Provides filtering actions for templates by vendor and builtin status</ul> </details> </td> <td><a href="https://github.com/carverauto/serviceradar/pull/2300/files#diff-265f44bb3b4d3c7da5baa3a8ddd402c87858edf8adcc831ac84468982a320d03">+213/-0</a>  </td> </tr> <tr> <td> <details> <summary>srql_target_resolver.ex<dd><code>SRQL-based profile targeting with device matching</code>                </dd></summary> <hr> elixir/serviceradar_core/lib/serviceradar/snmp_profiles/srql_target_resolver.ex <ul><li>Implements SRQL-based profile targeting resolution for SNMP profiles <li> Validates device UIDs against UUID regex to prevent SRQL injection attacks <li> Evaluates SRQL queries against device attributes and tags, supporting both device and interface targeting <li> Returns the highest-priority matching profile or nil if no profiles match</ul> </details> </td> <td><a href="https://github.com/carverauto/serviceradar/pull/2300/files#diff-2007e921214ed2cefc6170d60d4c2af9402cc499be1cd15737ec20344b2308ac">+268/-0</a>  </td> </tr> <tr> <td> <details> <summary>snmp_compiler.ex<dd><code>SNMP configuration compiler with profile resolution</code>            </dd></summary> <hr> elixir/serviceradar_core/lib/serviceradar/agent_config/compilers/snmp_compiler.ex <ul><li>Implements the <code>SNMPCompiler</code> module for transforming SNMP profiles into agent-consumable configuration <li> Resolves profiles using SRQL targeting with fallback to default profile <li> Compiles targets with OID configurations, handling credential decryption for v1/v2c and v3 authentication <li> Formats protocol versions, security levels, and authentication protocols for agent consumption</ul> </details> </td> <td><a href="https://github.com/carverauto/serviceradar/pull/2300/files#diff-3f9551d39c5e77c379f701d5dee9c0a3d95e2a7861ba7515c402e10b585c61e0">+302/-0</a>  </td> </tr> <tr> <td> <details> <summary>validate_srql_query.ex<dd><code>SRQL query validation for profile targeting</code>                            </dd></summary> <hr> elixir/serviceradar_core/lib/serviceradar/snmp_profiles/changes/validate_srql_query.ex <ul><li>Implements validation change for SRQL <code>target_query</code> attributes in SNMP profiles <li> Normalizes queries by adding <code>in:devices</code> prefix when needed, supporting both device and interface targeting <li> Validates query syntax via SRQL NIF and adds error messages for invalid queries <li> Treats empty strings as nil (no targeting)</ul> </details> </td> <td><a href="https://github.com/carverauto/serviceradar/pull/2300/files#diff-8eceb8f3bdc936bf2949b55d3f02daab69cbeb78ac93b94d982bd3840cab592c">+62/-0</a>    </td> </tr> <tr> <td> <details> <summary>set_as_default.ex<dd><code>Default profile enforcement with automatic unset</code>                  </dd></summary> <hr> elixir/serviceradar_core/lib/serviceradar/snmp_profiles/changes/set_as_default.ex <ul><li>Implements change handler to ensure only one default SNMP profile per tenant <li> Automatically unsets existing default profiles before setting a new one <li> Uses SystemActor for unset operations to maintain authorization consistency <li> Extracts tenant ID from schema context for actor creation</ul> </details> </td> <td><a href="https://github.com/carverauto/serviceradar/pull/2300/files#diff-24900bdf51eca0e0319e2891a5ed38b0db74f9ff7dfcc316d8d0929d8306d95c">+60/-0</a>    </td> </tr> <tr> <td> <details> <summary>compiler.ex<dd><code>Compiler registry update for SNMP support</code>                                </dd></summary> <hr> elixir/serviceradar_core/lib/serviceradar/agent_config/compiler.ex <ul><li>Adds <code>:snmp</code> to the <code>config_type</code> union type definition <li> Registers <code>SNMPCompiler</code> in the compiler registry mapping</ul> </details> </td> <td><a href="https://github.com/carverauto/serviceradar/pull/2300/files#diff-c853b548702e07ffa78e0a371869cd58f00b8ec13836220866d34298e047cbcd">+3/-2</a>      </td> </tr> <tr> <td> <details> <summary>encrypt_credentials.ex<dd><code>SNMP credential encryption change for Ash resources</code>            </dd></summary> <hr> elixir/serviceradar_core/lib/serviceradar/snmp_profiles/changes/encrypt_credentials.ex <ul><li>New Ash resource change module for encrypting SNMP credentials before storage <li> Implements credential encryption for <code>community</code>, <code>auth_password</code>, and <code>priv_password</code> fields <li> Uses Cloak/AES-256-GCM vault for encryption with write-only credential storage <li> Handles empty values by clearing encrypted fields and errors with user-friendly messages</ul> </details> </td> <td><a href="https://github.com/carverauto/serviceradar/pull/2300/files#diff-bce52d49583a0ad577d67ade030aa382ced6f779a82f0f7fd489bdc8b3d5e6b3">+51/-0</a>    </td> </tr> <tr> <td> <details> <summary>encrypt_passwords.ex<dd><code>SNMPv3 password encryption change for Ash resources</code>            </dd></summary> <hr> elixir/serviceradar_core/lib/serviceradar/snmp_profiles/changes/encrypt_passwords.ex <ul><li>New Ash resource change module for encrypting SNMPv3 authentication and privacy passwords <li> Intercepts plaintext <code>auth_password</code> and <code>priv_password</code> virtual attributes <li> Encrypts passwords using Cloak and stores in <code>_encrypted</code> fields <li> Provides error handling for encryption failures with field-level error messages</ul> </details> </td> <td><a href="https://github.com/carverauto/serviceradar/pull/2300/files#diff-27d973f56e16c631b2556361857a11587a62d8c92a3e01b35c4590a1b524f4a8">+47/-0</a>    </td> </tr> <tr> <td> <details> <summary>snmp_profiles.ex<dd><code>SNMP profiles domain configuration module</code>                                </dd></summary> <hr> elixir/serviceradar_core/lib/serviceradar/snmp_profiles.ex <ul><li>New Ash domain module for SNMP monitoring configuration management <li> Registers four SNMP-related resources: <code>SNMPProfile</code>, <code>SNMPTarget</code>, <code>SNMPOIDConfig</code>, <code>SNMPOIDTemplate</code> <li> Enables AshAdmin for admin interface and configures authorization settings <li> Provides documentation on device targeting using SRQL queries</ul> </details> </td> <td><a href="https://github.com/carverauto/serviceradar/pull/2300/files#diff-4abdb1bd929c42aab673ed4ca35faf8e5099df3b645d7b535121caf7a3a98fe3">+45/-0</a>    </td> </tr> <tr> <td> <details> <summary>router.ex<dd><code>SNMP profile configuration routes</code>                                                </dd></summary> <hr> web-ng/lib/serviceradar_web_ng_web/router.ex <ul><li>Added three new routes for SNMP profile configuration UI <li> Routes handle index, new profile creation, and profile editing views <li> Follows same pattern as existing sysmon profile routes</ul> </details> </td> <td><a href="https://github.com/carverauto/serviceradar/pull/2300/files#diff-df516cd33165cd85914c1ccb3ff6511d3fe688d4a66498b99807958998c5d07c">+5/-0</a>      </td> </tr> <tr> <td> <details> <summary>settings_components.ex<dd><code>SNMP settings navigation menu item</code>                                              </dd></summary> <hr> web-ng/lib/serviceradar_web_ng_web/components/settings_components.ex <ul><li>Added SNMP navigation item to settings menu <li> Links to <code>/settings/snmp</code> with active state detection <li> Positioned alongside other configuration options like sysmon</ul> </details> </td> <td><a href="https://github.com/carverauto/serviceradar/pull/2300/files#diff-540f72cd405a3221e6f350afd04e644db83f0a504938a8907e94c417d070601e">+5/-0</a>      </td> </tr> <tr> <td> <details> <summary>monitoring.pb.go<dd><code>SNMP protocol buffer definitions and message types</code>              </dd></summary> <hr> proto/monitoring.pb.go <ul><li>Added five new SNMP-related enums: <code>SNMPVersion</code>, <code>SNMPSecurityLevel</code>, <code>SNMPAuthProtocol</code>, <code>SNMPPrivProtocol</code>, <code>SNMPDataType</code> <li> Added four new message types: <code>SNMPConfig</code>, <code>SNMPTargetConfig</code>, <code>SNMPv3Auth</code>, <code>SNMPOIDConfig</code> <li> Extended <code>AgentConfigResponse</code> with <code>snmp_config</code> field for SNMP configuration delivery <li> Updated enum and message type indices to accommodate new SNMP types</ul> </details> </td> <td><a href="https://github.com/carverauto/serviceradar/pull/2300/files#diff-4f7e955b42854cc9cf3fb063b95e58a04f36271e6a0c1cb42ea6d7953dd96cc4">+821/-65</a></td> </tr> <tr> <td> <details> <summary>snmp_service.go<dd><code>SNMP agent service implementation</code>                                                </dd></summary> <hr> pkg/agent/snmp_service.go <ul><li>New 784-line SNMP agent service implementation for embedded SNMP monitoring <li> Manages SNMP service lifecycle with Start/Stop, config loading, and refresh loops <li> Supports local file, cache, and remote proto config sources with change detection <li> Implements concurrent-safe status reporting and proto config application with factory pattern</ul> </details> </td> <td><a href="https://github.com/carverauto/serviceradar/pull/2300/files#diff-703d839c384ade98beb93a20adb32586e2035374ce27ca4861f7cdee656e1699">+784/-0</a>  </td> </tr> <tr> <td> <details> <summary>client.go<dd><code>SNMPv3 client implementation and protocol conversion</code>          </dd></summary> <hr> pkg/checker/snmp/client.go <ul><li>Implemented SNMPv3 support in SNMP client initialization <li> Added conversion functions for SNMPv3 security levels, auth protocols, and privacy protocols <li> Maps internal types to gosnmp library types with proper error handling <li> Validates SNMPv3 configuration requirements</ul> </details> </td> <td><a href="https://github.com/carverauto/serviceradar/pull/2300/files#diff-b5e92490b688495a040a2f6f5227dc83c46fc5e7ea59885f8285a3d6c868bd87">+65/-1</a>    </td> </tr> <tr> <td> <details> <summary>server.go<dd><code>SNMP service integration into agent server</code>                              </dd></summary> <hr> pkg/agent/server.go <ul><li>Added SNMP service initialization in server startup <li> Implemented <code>initSNMPService</code> method to create and start SNMP service <li> Added <code>GetSNMPStatus</code> method for status reporting <li> Integrated SNMP service shutdown in server Stop method</ul> </details> </td> <td><a href="https://github.com/carverauto/serviceradar/pull/2300/files#diff-3c51e5356a25859a3300ab62ed2494462feb2567ef69e6db3aa2bbc1032c1c5d">+47/-0</a>    </td> </tr> <tr> <td> <details> <summary>types.go<dd><code>SNMPv3 type definitions and authentication structures</code>        </dd></summary> <hr> pkg/checker/snmp/types.go <ul><li>Added SNMPv3 type definitions: <code>SecurityLevel</code>, <code>AuthProtocol</code>, <code>PrivProtocol</code> enums <li> Added <code>V3Auth</code> struct for SNMPv3 authentication parameters with sensitive field tags <li> Extended <code>Target</code> struct with optional <code>V3Auth</code> field for SNMPv3 support</ul> </details> </td> <td><a href="https://github.com/carverauto/serviceradar/pull/2300/files#diff-d084fb749025dce23d720e215c6e66f52fe28432c912037984099c7570572a8b">+55/-0</a>    </td> </tr> </table></details></td></tr><tr><td>Tests</td><td><details><summary>7 files</summary><table> <tr> <td> <details> <summary>snmp_config_distribution_integration_test.exs<dd><code>SNMP Config Distribution Integration Tests</code>                              </dd></summary> <hr> elixir/serviceradar_core/test/serviceradar/edge/snmp_config_distribution_integration_test.exs <ul><li>Adds 438 lines of integration tests for SNMP configuration distribution from control plane to agent <li> Tests SNMP profile compilation, SRQL-based device targeting, and agent config generation <li> Validates SNMPv2c and SNMPv3 configuration handling with OID definitions <li> Verifies disabled profiles and full agent config payload generation</ul> </details> </td> <td><a href="https://github.com/carverauto/serviceradar/pull/2300/files#diff-8fd152b39318fa1d1ea5d6e4e00cc87e9daee20c0b8af46c77d2b743d827961f">+438/-0</a>  </td> </tr> <tr> <td> <details> <summary>snmp_profile_test.exs<dd><code>SNMPProfile resource tests with CRUD validation</code>                    </dd></summary> <hr> elixir/serviceradar_core/test/serviceradar/snmp_profiles/snmp_profile_test.exs <ul><li>Tests SNMPProfile resource structure, default values, and available actions <li> Validates CRUD operations including profile creation with custom parameters <li> Tests unique name constraint per tenant enforcement <li> Tests <code>set_as_default</code> action ensuring only one default profile exists</ul> </details> </td> <td><a href="https://github.com/carverauto/serviceradar/pull/2300/files#diff-c1bc8c4b709ff8068058ec0cdacdb8c68443fc15d93bc8336c01aeca2f3d1664">+246/-0</a>  </td> </tr> <tr> <td> <details> <summary>srql_target_resolver_test.exs<dd><code>SRQL target resolver tests with device matching</code>                    </dd></summary> <hr> elixir/serviceradar_core/test/serviceradar/snmp_profiles/srql_target_resolver_test.exs <ul><li>Tests SRQL target resolver module structure and function exports <li> Validates device UID format checking and error handling for invalid UUIDs <li> Tests profile matching with hostname queries and priority-based resolution <li> Tests filtering of disabled profiles and non-targeting profiles</ul> </details> </td> <td><a href="https://github.com/carverauto/serviceradar/pull/2300/files#diff-c2329efa90cd0fe64e211b4942fb3b43d549908829e53dd67c00303c355026e5">+375/-0</a>  </td> </tr> <tr> <td> <details> <summary>snmp_compiler_test.exs<dd><code>SNMPCompiler tests with profile and credential handling</code>    </dd></summary> <hr> elixir/serviceradar_core/test/serviceradar/snmp_profiles/snmp_compiler_test.exs <ul><li>Tests SNMPCompiler module structure, behavior implementation, and config type <li> Validates disabled config structure and validation logic <li> Tests compilation of profiles with targets and OIDs including v2c and v3 authentication <li> Tests credential decryption and protocol/security level formatting</ul> </details> </td> <td><a href="https://github.com/carverauto/serviceradar/pull/2300/files#diff-00ebe1280516fdb784057290bda325a48925b8fd56219583641135addc9e6e69">+335/-0</a>  </td> </tr> <tr> <td> <details> <summary>snmp_integration_test.go<dd><code>SNMP agent service integration tests</code>                                          </dd></summary> <hr> pkg/agent/snmp_integration_test.go <ul><li>Comprehensive integration test suite for SNMP agent service (604 lines) <li> Tests proto config application, enable/disable, idempotency, and updates <li> Validates status reporting, SNMPv3 configuration, and config file reloading <li> Includes concurrent operation tests for status calls and config updates</ul> </details> </td> <td><a href="https://github.com/carverauto/serviceradar/pull/2300/files#diff-f82d866873a075c9a17baa4d8e88054d4eb50a6f3a0e5eb1dd4a08e31cf35317">+604/-0</a>  </td> </tr> <tr> <td> <details> <summary>testing.go<dd><code>SNMP testing utilities and mock factories</code>                                </dd></summary> <hr> pkg/checker/snmp/testing.go <ul><li>New testing utilities for SNMP service with noop implementations <li> Provides <code>noopCollector</code>, <code>noopAggregator</code>, and factory implementations <li> Includes <code>NewMockServiceForTesting</code> helper for creating testable SNMP services</ul> </details> </td> <td><a href="https://github.com/carverauto/serviceradar/pull/2300/files#diff-39f3d28a82022f03da84f91457d34822e658c61a7275ab1d040a4da5c341f315">+103/-0</a>  </td> </tr> <tr> <td> <details> <summary>checker_integration_test.go<dd><code>ICMP checker test updates for new API</code>                                        </dd></summary> <hr> pkg/agent/checker_integration_test.go <ul><li>Updated ICMP checker test to use new <code>NewICMPChecker</code> factory function <li> Added logger dependency and proto request parameter to checker initialization <li> Improved test assertions with string conversion for response validation</ul> </details> </td> <td><a href="https://github.com/carverauto/serviceradar/pull/2300/files#diff-ca90f1fec436a1e7e20474bc8c4f65d9217c83e94626cfefc2cb2f21151b3b8e">+14/-9</a>    </td> </tr> </table></details></td></tr><tr><td>Configuration changes</td><td><details><summary>4 files</summary><table> <tr> <td> <details> <summary>config.exs<dd><code>Register SNMPProfiles Domain in Ash Configuration</code>                </dd></summary> <hr> web-ng/config/config.exs <ul><li>Registers <code>ServiceRadar.SNMPProfiles</code> domain in both web-ng and serviceradar_core Ash configurations <li> Adds SNMP profiles to the list of managed domains for proper resource initialization</ul> </details> </td> <td><a href="https://github.com/carverauto/serviceradar/pull/2300/files#diff-da0b524f083d350207155c247526098fdd68866d64e7e4eae3d96fdae31bcfb6">+4/-2</a>      </td> </tr> <tr> <td> <details> <summary>20260114074955_add_snmp_profiles.exs<dd><code>Database schema for SNMP and system monitoring profiles</code>    </dd></summary> <hr> elixir/serviceradar_core/priv/repo/tenant_migrations/20260114074955_add_snmp_profiles.exs <ul><li>Creates database tables for SNMP infrastructure: <code>snmp_profiles</code>, <code>snmp_targets</code>, <code>snmp_oid_configs</code>, <code>snmp_oid_templates</code> <li> Adds <code>sysmon_profiles</code> table for system monitoring profiles with similar targeting capabilities <li> Implements foreign key relationships with cascade delete and unique constraints per tenant <li> Adds <code>config_source</code> column to <code>ocsf_agents</code> table for tracking configuration origin</ul> </details> </td> <td><a href="https://github.com/carverauto/serviceradar/pull/2300/files#diff-4f3619b4ea9997d925d720374e4c4853c17ef9eacaa250690652550db711abd6">+229/-0</a>  </td> </tr> <tr> <td> <details> <summary>config.exs<dd><code>Configuration update for SNMP profiles domain</code>                        </dd></summary> <hr> elixir/serviceradar_core/config/config.exs <ul><li>Adds <code>ServiceRadar.SNMPProfiles</code> to the list of domains for Ash resource discovery</ul> </details> </td> <td><a href="https://github.com/carverauto/serviceradar/pull/2300/files#diff-42b3888cc53d9dcf4ebc45ec7fffb2c672b129bffe763b6c76de58e4678a13a8">+2/-1</a>      </td> </tr> <tr> <td> <details> <summary>test.exs<dd><code>SNMP profiles domain test configuration</code>                                    </dd></summary> <hr> elixir/serviceradar_core/config/test.exs <ul><li>Added <code>ServiceRadar.SNMPProfiles</code> to the list of domains in test configuration <li> Enables SNMP profile domain for testing</ul> </details> </td> <td><a href="https://github.com/carverauto/serviceradar/pull/2300/files#diff-d8fffec56a9bc7305c02bbdd424e21031ecbbb2c2e6ba07ba626e6043540e360">+1/-0</a>      </td> </tr> </table></details></td></tr><tr><td>Formatting</td><td><details><summary>3 files</summary><table> <tr> <td> <details> <summary>index.ex<dd><code>Whitespace formatting adjustment</code>                                                  </dd></summary> <hr> web-ng/lib/serviceradar_web_ng_web/live/settings/networks_live/index.ex <ul><li>Minor whitespace adjustment in host status display text <li> Changed spacing in "of hosts" text from single to double space</ul> </details> </td> <td><a href="https://github.com/carverauto/serviceradar/pull/2300/files#diff-b2127e71582033bc6dfd2d7397f56bf43c1f7c613defffc504b3d8ee1e7406c4">+1/-1</a>      </td> </tr> <tr> <td> <details> <summary>index.ex<dd><code>Code formatting improvements</code>                                                          </dd></summary> <hr> web-ng/lib/serviceradar_web_ng_web/live/log_live/index.ex <ul><li>Added blank lines for code readability in two functions <li> Improves formatting around error handling and default return values</ul> </details> </td> <td><a href="https://github.com/carverauto/serviceradar/pull/2300/files#diff-5c469994d989d04955c9b8a31536ed518a182b3f46819ecee9c3c22f651a4321">+2/-0</a>      </td> </tr> <tr> <td> <details> <summary>index.ex<dd><code>Code formatting improvement</code>                                                            </dd></summary> <hr> web-ng/lib/serviceradar_web_ng_web/live/analytics_live/index.ex <ul><li>Added blank line for code readability in analytics stats function <li> Improves formatting around error handling</ul> </details> </td> <td><a href="https://github.com/carverauto/serviceradar/pull/2300/files#diff-35fb1715e171ac9f3240e9c02d2170b636acbb369952d58d352e1f75c91f82e3">+1/-0</a>      </td> </tr> </table></details></td></tr><tr><td>Additional files</td><td><details><summary>18 files</summary><table> <tr> <td>snmp.md</td> <td><a href="https://github.com/carverauto/serviceradar/pull/2300/files#diff-5d2a72888517a4c731287280253a29cedbcbe595f1ce57c2fa14aef37c62595a">+182/-0</a>  </td> </tr> <tr> <td>20260114074956.json</td> <td><a href="https://github.com/carverauto/serviceradar/pull/2300/files#diff-99b8acbc728a9a2e1fe4b7edd09b84052641f6159014142fbd55d85b9f03a7d8">+354/-0</a>  </td> </tr> <tr> <td>20260114074956.json</td> <td><a href="https://github.com/carverauto/serviceradar/pull/2300/files#diff-1c4f2389aded310f1ef3e57418269c50ea1f1e0e6a721b3646a0849a9a3ceddb">+177/-0</a>  </td> </tr> <tr> <td>20260114074956.json</td> <td><a href="https://github.com/carverauto/serviceradar/pull/2300/files#diff-7b96ff1dd5c08ade5b77b5cc473a54cc7615e998ebe905346a43dddf43389247">+165/-0</a>  </td> </tr> <tr> <td>20260114074956.json</td> <td><a href="https://github.com/carverauto/serviceradar/pull/2300/files#diff-533b7fce7ce1884234942bce26abaa886b0f18bee4f772f898539537232ca005">+194/-0</a>  </td> </tr> <tr> <td>20260114074956.json</td> <td><a href="https://github.com/carverauto/serviceradar/pull/2300/files#diff-16060141fd7139ea1c35574771d06bc31705d099ebd182e4e16ab86cebc16169">+249/-0</a>  </td> </tr> <tr> <td>20260114074956.json</td> <td><a href="https://github.com/carverauto/serviceradar/pull/2300/files#diff-179201ca3c8ff5fb9320b62893b5671f06a619f189bda00cc19aba8dcc252e48">+257/-0</a>  </td> </tr> <tr> <td>design.md</td> <td><a href="https://github.com/carverauto/serviceradar/pull/2300/files#diff-d5a94e78fcb56f7a87e5c6301357809709c351cbe0ee9b106d4faefd0347ecc7">+379/-0</a>  </td> </tr> <tr> <td>proposal.md</td> <td><a href="https://github.com/carverauto/serviceradar/pull/2300/files#diff-351f179fc846c318e3d803791ed050d95df43ebd5951bc2c38c9427eabb4ad15">+42/-0</a>    </td> </tr> <tr> <td>spec.md</td> <td><a href="https://github.com/carverauto/serviceradar/pull/2300/files#diff-4ae7d708fae44fe393b1908da2ffe7ebf1732536c6b21d6d39e01cc250679dbb">+120/-0</a>  </td> </tr> <tr> <td>spec.md</td> <td><a href="https://github.com/carverauto/serviceradar/pull/2300/files#diff-edc22549e1dd22326bd2c33d8865a117fd4d67c2a300ef36b9ee7d26a450ad04">+131/-0</a>  </td> </tr> <tr> <td>tasks.md</td> <td><a href="https://github.com/carverauto/serviceradar/pull/2300/files#diff-7bc05c95bc50ed240fcc70081ef9cca96b03ed5f74a4d1519173c82a59418171">+125/-0</a>  </td> </tr> <tr> <td>snmp_service_test.go</td> <td><a href="https://github.com/carverauto/serviceradar/pull/2300/files#diff-5286b0ba755cf551f524540ecf2f4926517f6f66748c25fc98ffe7363b917fe0">+550/-0</a>  </td> </tr> <tr> <td>sysmon_service.go</td> <td><a href="https://github.com/carverauto/serviceradar/pull/2300/files#diff-8c00578b299aa029ef81f77ebe8959d20b7122299ee24e525c345f7c91c28d48">+2/-0</a>      </td> </tr> <tr> <td>types.go</td> <td><a href="https://github.com/carverauto/serviceradar/pull/2300/files#diff-e3a3767c816cdb568a387a32243f7348046f1f3445549cc06368870c914496cd">+1/-0</a>      </td> </tr> <tr> <td>config.go</td> <td><a href="https://github.com/carverauto/serviceradar/pull/2300/files#diff-1e0bd78d2e0cae2b44ebfcbf0d0a4bedacd55d145b32e63150491b38a8da34cc">+62/-0</a>    </td> </tr> <tr> <td>service.go</td> <td><a href="https://github.com/carverauto/serviceradar/pull/2300/files#diff-c592649b582c62a85d1dd416ac7fb609e55531cb43c8199a15f4432ce8ae05d8">+16/-1</a>    </td> </tr> <tr> <td>monitoring.proto</td> <td><a href="https://github.com/carverauto/serviceradar/pull/2300/files#diff-c4fc4e9693e4d2cf45697113ccca65b8c5ff18d2037e31ade411473533b36c2b">+112/-0</a>  </td> </tr> </table></details></td></tr></tbody></table> </details> ___

qodo-code-review[bot] commented

2026-01-14 17:57:45 +00:00

(Migrated from github.com)

Author

Owner

Imported GitHub PR comment.

Original author: @qodo-code-review[bot]
Original URL: https://github.com/carverauto/serviceradar/pull/2300#issuecomment-3750886011
Original created: 2026-01-14T17:57:45Z

PR Compliance Guide 🔍

Below is a summary of compliance checks for this PR:

Security Compliance

⚪

Sensitive data exposure

Description: The compiler decrypts SNMP community strings and SNMPv3 auth/priv passwords and embeds
them into the generated agent configuration payload (e.g., community,
v3_auth.auth_password, v3_auth.priv_password), which could expose credentials if the
config distribution/storage/logging/transport path is not strictly protected end-to-end. snmp_compiler.ex [186-209]

Referred Code

  # Add authentication based on version
  case target.version do
    :v1 ->
      Map.put(base_target, "community", decrypt_credential(target.community_encrypted))

    :v2c ->
      Map.put(base_target, "community", decrypt_credential(target.community_encrypted))

    :v3 ->
      Map.put(base_target, "v3_auth", compile_v3_auth(target))
  end
end

# Compile SNMPv3 authentication parameters
defp compile_v3_auth(target) do
  %{
    "username" => target.username,
    "security_level" => format_security_level(target.security_level),
    "auth_protocol" => format_auth_protocol(target.auth_protocol),
    "auth_password" => decrypt_credential(target.auth_password_encrypted),
    "priv_protocol" => format_priv_protocol(target.priv_protocol),


 ... (clipped 3 lines)

Ticket Compliance

🟡

🎫 #2222

⚪	Merge/integrate the standalone `snmp-checker` functionality into `serviceradar-agent` so the agent no longer needs to talk to an external SNMP checker over gRPC, reducing the number of components users must install.

Codebase Duplication Compliance

⚪

Codebase context is not defined

Follow the guide to enable codebase context checks.

Custom Compliance

🟢

Generic: Meaningful Naming and Self-Documenting Code

Objective: Ensure all identifiers clearly express their purpose and intent, making code
self-documenting

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

🔴

Generic: Robust Error Handling and Edge Case Management

Objective: Ensure comprehensive error handling that provides meaningful context and graceful
degradation

Status:
Silent decryption failure: Credential decryption errors are silently swallowed by returning nil (and query read
errors return empty lists), which can mask production failures without actionable context
or logs.

Referred Code

# Decrypt an encrypted credential, returning nil if not set
defp decrypt_credential(nil), do: nil

defp decrypt_credential(encrypted) do
  case Vault.decrypt(encrypted) do
    {:ok, decrypted} -> decrypted
    {:error, _} -> nil
  end
end

# Format version atom to string
defp format_version(:v1), do: "v1"
defp format_version(:v2c), do: "v2c"
defp format_version(:v3), do: "v3"
defp format_version(_), do: "v2c"

# Format security level atom to string
defp format_security_level(:no_auth_no_priv), do: "noAuthNoPriv"
defp format_security_level(:auth_no_priv), do: "authNoPriv"
defp format_security_level(:auth_priv), do: "authPriv"
defp format_security_level(_), do: "noAuthNoPriv"


 ... (clipped 44 lines)

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Secure Error Handling

Objective: To prevent the leakage of sensitive system information through error messages while
providing sufficient detail for internal debugging.

Status:
Raw exception returned: The compiler rescues and returns the raw exception term in {:error, {:compilation_error,
e}}, which risks exposing internal implementation details to callers if propagated to
user-facing layers.

Referred Code

rescue
  e ->
    Logger.error("SNMPCompiler: error compiling config - #{inspect(e)}")
    {:error, {:compilation_error, e}}
end

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Secure Logging Practices

Objective: To ensure logs are useful for debugging and auditing without exposing sensitive
information like PII, PHI, or cardholder data.

Status:
Logs full exception: The error log writes inspect(e) for compilation failures, which can inadvertently include
sensitive data embedded in exception structs (e.g., config/changeset contents).

Referred Code

rescue
  e ->
    Logger.error("SNMPCompiler: error compiling config - #{inspect(e)}")
    {:error, {:compilation_error, e}}

Learn more about managing compliance generic rules or creating your own custom rules

⚪

Generic: Comprehensive Audit Trails

Objective: To create a detailed and reliable record of critical system actions for security analysis
and compliance.

Status:
Audit logging unclear: The PR adds sensitive configuration creation/seed operations (e.g., seed!/2) but no
explicit audit logging is visible in the diff, so it’s unclear whether critical actions
are recorded with actor context and outcome.

Referred Code

Seeds all built-in templates into the database for a tenant.
Skips templates that already exist.
"""
@spec seed!(String.t(), map()) :: {:ok, integer()} | {:error, term()}
def seed!(tenant_schema, actor) do
  templates = all()

  results =
    Enum.map(templates, fn template ->
      attrs = Map.put(template, :is_builtin, true)

      SNMPOIDTemplate
      |> Ash.Changeset.for_create(:create, attrs, actor: actor, tenant: tenant_schema)
      |> Ash.create(actor: actor)
      |> classify_seed_result()
    end)

  created = Enum.count(results, &(&1 == :created))
  {:ok, created}
end

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Security-First Input Validation and Data Handling

Objective: Ensure all data inputs are validated, sanitized, and handled securely to prevent
vulnerabilities

Status:
SRQL handling review: Although device_uid is UUID-validated to reduce SRQL injection risk, the overall SRQL
parsing/execution and tag filtering via SQL fragments requires a full security review
beyond the visible diff to confirm sanitization and authorization guarantees end-to-end.

Referred Code

@spec resolve_for_device(String.t(), String.t(), map()) ::
        {:ok, SNMPProfile.t() | nil} | {:error, term()}
def resolve_for_device(tenant_schema, device_uid, actor) when is_binary(device_uid) do
  # Validate device_uid is a proper UUID to prevent SRQL injection
  if Regex.match?(@uuid_regex, device_uid) do
    # Load all targeting profiles ordered by priority
    case load_targeting_profiles(tenant_schema, actor) do
      {:ok, []} ->
        {:ok, nil}

      {:ok, profiles} ->
        # Try each profile in order until one matches
        find_matching_profile(profiles, tenant_schema, device_uid, actor)

      {:error, reason} ->
        {:error, reason}
    end
  else
    Logger.warning("SNMPSrqlTargetResolver: invalid device_uid format: #{inspect(device_uid)}")
    {:error, :invalid_device_uid}
  end


 ... (clipped 188 lines)

Learn more about managing compliance generic rules or creating your own custom rules

Update

Compliance status legend

🟢 - Fully Compliant
🟡 - Partial Compliant
🔴 - Not Compliant
⚪ - Requires Further Human Verification
🏷️ - Compliance label

Imported GitHub PR comment. Original author: @qodo-code-review[bot] Original URL: https://github.com/carverauto/serviceradar/pull/2300#issuecomment-3750886011 Original created: 2026-01-14T17:57:45Z --- ## PR Compliance Guide 🔍  Below is a summary of compliance checks for this PR: <table><tbody><tr><td colspan='2'>Security Compliance</td></tr> <tr><td rowspan=1>⚪</td> <td><details><summary>Sensitive data exposure </summary> Description: The compiler decrypts SNMP community strings and SNMPv3 auth/priv passwords and embeds them into the generated agent configuration payload (e.g., <code>community</code>, <code>v3_auth.auth_password</code>, <code>v3_auth.priv_password</code>), which could expose credentials if the config distribution/storage/logging/transport path is not strictly protected end-to-end. <a href='https://github.com/carverauto/serviceradar/pull/2300/files#diff-3f9551d39c5e77c379f701d5dee9c0a3d95e2a7861ba7515c402e10b585c61e0R186-R209'>snmp_compiler.ex [186-209]</a> <details open><summary>Referred Code</summary> ```elixir # Add authentication based on version case target.version do :v1 -> Map.put(base_target, "community", decrypt_credential(target.community_encrypted)) :v2c -> Map.put(base_target, "community", decrypt_credential(target.community_encrypted)) :v3 -> Map.put(base_target, "v3_auth", compile_v3_auth(target)) end end # Compile SNMPv3 authentication parameters defp compile_v3_auth(target) do %{ "username" => target.username, "security_level" => format_security_level(target.security_level), "auth_protocol" => format_auth_protocol(target.auth_protocol), "auth_password" => decrypt_credential(target.auth_password_encrypted), "priv_protocol" => format_priv_protocol(target.priv_protocol), ... (clipped 3 lines) ``` </details></details></td></tr> <tr><td colspan='2'>Ticket Compliance</td></tr> <tr><td>🟡</td> <td> <details> <summary>🎫 <a href=https://github.com/carverauto/serviceradar/issues/2222>#2222</a></summary> <table width='100%'><tbody> <tr><td rowspan=1>⚪</td> <td>Merge/integrate the standalone <code>snmp-checker</code> functionality into <code>serviceradar-agent</code> so the agent no longer needs to talk to an external SNMP checker over gRPC, reducing the number of components users must install. </td></tr> </tbody></table> </details> </td></tr> <tr><td colspan='2'>Codebase Duplication Compliance</td></tr> <tr><td>⚪</td><td><details><summary>Codebase context is not defined </summary> Follow the <a href='https://qodo-merge-docs.qodo.ai/core-abilities/rag_context_enrichment/'>guide</a> to enable codebase context checks. </details></td></tr> <tr><td colspan='2'>Custom Compliance</td></tr> <tr><td rowspan=1>🟢</td><td> <details><summary>Generic: Meaningful Naming and Self-Documenting Code</summary> **Objective:** Ensure all identifiers clearly express their purpose and intent, making code self-documenting **Status:** Passed > Learn more about managing compliance <a href='https://qodo-merge-docs.qodo.ai/tools/compliance/#configuration-options'>generic rules</a> or creating your own <a href='https://qodo-merge-docs.qodo.ai/tools/compliance/#custom-compliance'>custom rules</a> </details></td></tr> <tr><td rowspan=3>🔴</td> <td><details> <summary>Generic: Robust Error Handling and Edge Case Management</summary> **Objective:** Ensure comprehensive error handling that provides meaningful context and graceful degradation **Status:** <a href='https://github.com/carverauto/serviceradar/pull/2300/files#diff-3f9551d39c5e77c379f701d5dee9c0a3d95e2a7861ba7515c402e10b585c61e0R211-R275'>Silent decryption failure</a>: Credential decryption errors are silently swallowed by returning <code>nil</code> (and query read errors return empty lists), which can mask production failures without actionable context or logs. <details open><summary>Referred Code</summary> ```elixir # Decrypt an encrypted credential, returning nil if not set defp decrypt_credential(nil), do: nil defp decrypt_credential(encrypted) do case Vault.decrypt(encrypted) do {:ok, decrypted} -> decrypted {:error, _} -> nil end end # Format version atom to string defp format_version(:v1), do: "v1" defp format_version(:v2c), do: "v2c" defp format_version(:v3), do: "v3" defp format_version(_), do: "v2c" # Format security level atom to string defp format_security_level(:no_auth_no_priv), do: "noAuthNoPriv" defp format_security_level(:auth_no_priv), do: "authNoPriv" defp format_security_level(:auth_priv), do: "authPriv" defp format_security_level(_), do: "noAuthNoPriv" ... (clipped 44 lines) ``` </details> > Learn more about managing compliance <a href='https://qodo-merge-docs.qodo.ai/tools/compliance/#configuration-options'>generic rules</a> or creating your own <a href='https://qodo-merge-docs.qodo.ai/tools/compliance/#custom-compliance'>custom rules</a> </details></td></tr> <tr><td><details> <summary>Generic: Secure Error Handling</summary> **Objective:** To prevent the leakage of sensitive system information through error messages while providing sufficient detail for internal debugging. **Status:** <a href='https://github.com/carverauto/serviceradar/pull/2300/files#diff-3f9551d39c5e77c379f701d5dee9c0a3d95e2a7861ba7515c402e10b585c61e0R88-R92'>Raw exception returned</a>: The compiler rescues and returns the raw exception term in <code>{:error, {:compilation_error, </code> <code>e}}</code>, which risks exposing internal implementation details to callers if propagated to user-facing layers. <details open><summary>Referred Code</summary> ```elixir rescue e -> Logger.error("SNMPCompiler: error compiling config - #{inspect(e)}") {:error, {:compilation_error, e}} end ``` </details> > Learn more about managing compliance <a href='https://qodo-merge-docs.qodo.ai/tools/compliance/#configuration-options'>generic rules</a> or creating your own <a href='https://qodo-merge-docs.qodo.ai/tools/compliance/#custom-compliance'>custom rules</a> </details></td></tr> <tr><td><details> <summary>Generic: Secure Logging Practices</summary> **Objective:** To ensure logs are useful for debugging and auditing without exposing sensitive information like PII, PHI, or cardholder data. **Status:** <a href='https://github.com/carverauto/serviceradar/pull/2300/files#diff-3f9551d39c5e77c379f701d5dee9c0a3d95e2a7861ba7515c402e10b585c61e0R88-R91'>Logs full exception</a>: The error log writes <code>inspect(e)</code> for compilation failures, which can inadvertently include sensitive data embedded in exception structs (e.g., config/changeset contents). <details open><summary>Referred Code</summary> ```elixir rescue e -> Logger.error("SNMPCompiler: error compiling config - #{inspect(e)}") {:error, {:compilation_error, e}} ``` </details> > Learn more about managing compliance <a href='https://qodo-merge-docs.qodo.ai/tools/compliance/#configuration-options'>generic rules</a> or creating your own <a href='https://qodo-merge-docs.qodo.ai/tools/compliance/#custom-compliance'>custom rules</a> </details></td></tr> <tr><td rowspan=2>⚪</td> <td><details> <summary>Generic: Comprehensive Audit Trails</summary> **Objective:** To create a detailed and reliable record of critical system actions for security analysis and compliance. **Status:** <a href='https://github.com/carverauto/serviceradar/pull/2300/files#diff-dfb09a552c8a813238fd1455326888586384c927450dedf18a30344548e3c683R83-R102'>Audit logging unclear</a>: The PR adds sensitive configuration creation/seed operations (e.g., <code>seed!/2</code>) but no explicit audit logging is visible in the diff, so it’s unclear whether critical actions are recorded with actor context and outcome. <details open><summary>Referred Code</summary> ```elixir Seeds all built-in templates into the database for a tenant. Skips templates that already exist. """ @spec seed!(String.t(), map()) :: {:ok, integer()} | {:error, term()} def seed!(tenant_schema, actor) do templates = all() results = Enum.map(templates, fn template -> attrs = Map.put(template, :is_builtin, true) SNMPOIDTemplate |> Ash.Changeset.for_create(:create, attrs, actor: actor, tenant: tenant_schema) |> Ash.create(actor: actor) |> classify_seed_result() end) created = Enum.count(results, &(&1 == :created)) {:ok, created} end ``` </details> > Learn more about managing compliance <a href='https://qodo-merge-docs.qodo.ai/tools/compliance/#configuration-options'>generic rules</a> or creating your own <a href='https://qodo-merge-docs.qodo.ai/tools/compliance/#custom-compliance'>custom rules</a> </details></td></tr> <tr><td><details> <summary>Generic: Security-First Input Validation and Data Handling</summary> **Objective:** Ensure all data inputs are validated, sanitized, and handled securely to prevent vulnerabilities **Status:** <a href='https://github.com/carverauto/serviceradar/pull/2300/files#diff-2007e921214ed2cefc6170d60d4c2af9402cc499be1cd15737ec20344b2308acR59-R267'>SRQL handling review</a>: Although <code>device_uid</code> is UUID-validated to reduce SRQL injection risk, the overall SRQL parsing/execution and tag filtering via SQL fragments requires a full security review beyond the visible diff to confirm sanitization and authorization guarantees end-to-end. <details open><summary>Referred Code</summary> ```elixir @spec resolve_for_device(String.t(), String.t(), map()) :: {:ok, SNMPProfile.t() | nil} | {:error, term()} def resolve_for_device(tenant_schema, device_uid, actor) when is_binary(device_uid) do # Validate device_uid is a proper UUID to prevent SRQL injection if Regex.match?(@uuid_regex, device_uid) do # Load all targeting profiles ordered by priority case load_targeting_profiles(tenant_schema, actor) do {:ok, []} -> {:ok, nil} {:ok, profiles} -> # Try each profile in order until one matches find_matching_profile(profiles, tenant_schema, device_uid, actor) {:error, reason} -> {:error, reason} end else Logger.warning("SNMPSrqlTargetResolver: invalid device_uid format: #{inspect(device_uid)}") {:error, :invalid_device_uid} end ... (clipped 188 lines) ``` </details> > Learn more about managing compliance <a href='https://qodo-merge-docs.qodo.ai/tools/compliance/#configuration-options'>generic rules</a> or creating your own <a href='https://qodo-merge-docs.qodo.ai/tools/compliance/#custom-compliance'>custom rules</a> </details></td></tr> <tr><td align="center" colspan="2"> - [ ] Update  </td></tr></tbody></table> <details><summary>Compliance status legend</summary> 🟢 - Fully Compliant 🟡 - Partial Compliant 🔴 - Not Compliant ⚪ - Requires Further Human Verification 🏷️ - Compliance label </details>

qodo-code-review[bot] commented

2026-01-14 17:59:00 +00:00

(Migrated from github.com)

Author

Owner

Imported GitHub PR comment.

Original author: @qodo-code-review[bot]
Original URL: https://github.com/carverauto/serviceradar/pull/2300#issuecomment-3750891616
Original created: 2026-01-14T17:59:00Z

PR Code Suggestions ✨

Explore these optional code suggestions:

Category	Suggestion	Impact
Possible issue	Use correct relationship management type In the `create` action, change the `manage_relationship` type from `:append` to `:replace` to correctly handle the `belongs_to` association. elixir/serviceradar_core/lib/serviceradar/snmp_profiles/snmp_oid_config.ex [66-79] `create :create do accept [ :oid, :name, :data_type, :scale, :delta ] argument :snmp_target_id, :uuid, allow_nil?: false - change manage_relationship(:snmp_target_id, :snmp_target, type: :append) + change manage_relationship(:snmp_target_id, :snmp_target, type: :replace) change ServiceRadar.Changes.AssignTenantId end` Apply / Chat Suggestion importance[1-10]: 9 __ Why: The suggestion correctly identifies a bug where an incorrect relationship management type (`:append`) is used for a `belongs_to` relationship, which would cause creation to fail.	High
	Replace undefined min usage Replace the call to the undefined `min` function with an explicit length check to prevent a compilation error when slicing a hash string. pkg/agent/snmp_service.go [209-213] `+// Shorten hash to up to 8 characters +hashShort := s.configHash +if len(hashShort) > 8 { + hashShort = hashShort[:8] +} s.logger.Info(). Str("source", source). - Str("config_hash", s.configHash[:min(8, len(s.configHash))]). + Str("config_hash", hashShort). Int("target_count", len(config.Targets)). Msg("SNMP agent service started")` Apply / Chat Suggestion importance[1-10]: 9 __ Why: The suggestion correctly points out the use of an undefined `min` function, which would cause a compilation error, and provides a valid fix.	High
	Fix race condition in stop logic Fix a race condition in the `Stop` function by adjusting the mutex locking to ensure safe concurrent access to shared state. pkg/agent/snmp_service.go [221-261] // Stop halts the SNMP service and config refresh loop. // //nolint:dupl // Intentional parallel structure with SysmonService.Stop func (s SNMPAgentService) Stop(ctx context.Context) error { s.mu.Lock() + defer s.mu.Unlock() if !s.started { - s.mu.Unlock() return nil } // Mark as stopping to prevent concurrent Stop() calls s.started = false // Stop the config refresh loop if s.stopRefresh != nil { close(s.stopRefresh) + + // Create a temporary channel to wait on, as s.refreshDone might be nil + done := s.refreshDone + s.mu.Unlock() // Unlock while waiting + + // Wait for refresh loop to finish (with timeout from context) + if done != nil { + select { + case <-done: + // Refresh loop stopped + case <-ctx.Done(): + s.logger.Warn().Msg("Timeout waiting for SNMP config refresh loop to stop") + case <-time.After(5 time.Second): + s.logger.Warn().Msg("Timeout waiting for SNMP config refresh loop to stop") + } + } + s.mu.Lock() // Re-lock before continuing } - s.mu.Unlock() - - // Wait for refresh loop to finish (with timeout from context) - if s.refreshDone != nil { - select { - case <-s.refreshDone: - // Refresh loop stopped - case <-ctx.Done(): - s.logger.Warn().Msg("Timeout waiting for SNMP config refresh loop to stop") - case <-time.After(5 * time.Second): - s.logger.Warn().Msg("Timeout waiting for SNMP config refresh loop to stop") - } - } - - s.mu.Lock() - defer s.mu.Unlock() if s.service != nil { if err := s.service.Stop(); err != nil { return fmt.Errorf("failed to stop SNMP service: %w", err) } } s.logger.Info().Msg("SNMP agent service stopped") return nil } `[To ensure code accuracy, apply this suggestion manually]` Suggestion importance[1-10]: 8 __ Why: The suggestion correctly identifies a race condition in the `Stop` function where the mutex is unlocked prematurely, potentially leading to concurrent access issues.	Medium
	Use specific error for uniqueness check Refine error classification by matching on the specific `Ash.Error.Changes.Unique` error for uniqueness violations, instead of the generic `Ash.Error.Changes.InvalidChanges`. elixir/serviceradar_core/lib/serviceradar/snmp_profiles/builtin_templates.ex [108-111] `defp classify_ash_error(errors) do # Check if it's a uniqueness error (template already exists) - if Enum.any?(errors, &match?(%Ash.Error.Changes.InvalidChanges{}, &1)), do: :exists, else: :error + if Enum.any?(errors, &match?(%Ash.Error.Changes.Unique{}, &1)), do: :exists, else: :error end` Apply / Chat Suggestion importance[1-10]: 7 __ Why: This suggestion correctly identifies a bug where any invalid change error would be misclassified as a uniqueness violation, improving the accuracy of error handling during template seeding.	Medium
	Log error on credential decryption failure Log an error when credential decryption fails to improve debuggability, as it currently fails silently. elixir/serviceradar_core/lib/serviceradar/agent_config/compilers/snmp_compiler.ex [214-219] `defp decrypt_credential(encrypted) do case Vault.decrypt(encrypted) do {:ok, decrypted} -> decrypted - {:error, _} -> nil + {:error, reason} -> + Logger.error("SNMPCompiler: failed to decrypt credential: #{inspect(reason)}") + nil end end` Apply / Chat Suggestion importance[1-10]: 6 __ Why: The suggestion correctly points out that silent credential decryption failures are problematic for debugging. Adding logging significantly improves observability without changing the function's core behavior.	Low
General	Use dynamic community string Refactor `build_snmp_get_request` to accept a dynamic community string from the form. This involves computing packet lengths dynamically to ensure the SNMP connection test uses the user-provided credentials. web-ng/lib/serviceradar_web_ng_web/live/settings/snmp_profiles_live/index.ex [1049-1103] -defp build_snmp_get_request do - # SNMPv1 GET request structure (ASN.1 BER encoded) - # Request for .1.3.6.1.2.1.1.1.0 (sysDescr.0) with community "public" +defp build_snmp_get_request(community) do + community_bin = to_charlist(community) + community_len = byte_size(community) + # compute PDU length dynamically: version(3) + community tag(2+len) + PDU header(2) + rest(…) + # Here we assume the rest of the PDU after community is fixed 26 bytes + total_len = 1 + 1 + // version + 1 + 1 + community_len + + 1 + 1 + 26 << - # SEQUENCE (total length 0x27 = 39 bytes) - 0x30, - 0x27, - # INTEGER - version (0 = SNMPv1) - 0x02, - 0x01, - 0x00, - # OCTET STRING - community "public" - 0x04, - 0x06, - "public", - # GetRequest-PDU (length 0x1A = 26 bytes) - 0xA0, - 0x1A, - # INTEGER - request-id - 0x02, - 0x04, - 0x00, - 0x00, - 0x00, - 0x01, - # INTEGER - error-status - 0x02, - 0x01, - 0x00, - # INTEGER - error-index - 0x02, - 0x01, - 0x00, - # SEQUENCE - variable-bindings (length 0x0C = 12 bytes) - 0x30, - 0x0C, - # SEQUENCE - single binding (length 0x0A = 10 bytes) - 0x30, - 0x0A, - # OID - .1.3.6.1.2.1.1.1.0 (sysDescr.0) - length 8 - 0x06, - 0x08, - 0x2B, - 0x06, - 0x01, - 0x02, - 0x01, - 0x01, - 0x01, - 0x00, - # NULL value - 0x05, - 0x00 + 0x30, total_len, + # version + 0x02, 0x01, 0x00, + # community + 0x04, community_len, community_bin, + # GetRequest-PDU header + 0xA0, 26, + # rest of the fixed PDU + 0x02, 0x04, 0x00, 0x00, 0x00, 0x01, + 0x02, 0x01, 0x00, + 0x02, 0x01, 0x00, + 0x30, 0x0C, + 0x30, 0x0A, + 0x06, 0x08, 0x2B, 0x06, 0x01, 0x02, 0x01, 0x01, 0x01, 0x00, + 0x05, 0x00 >> end `[To ensure code accuracy, apply this suggestion manually]` Suggestion importance[1-10]: 8 __ Why: The suggestion correctly identifies that the "Test Connection" feature is flawed because it uses a hard-coded "public" community string instead of the one provided by the user, making the test less useful. Implementing this would significantly improve the feature's correctness and utility.	Medium
	Use dynamic module name in logs Replace the hard-coded module name in the log message with the `MODULE` macro for better maintainability. elixir/serviceradar_core/lib/serviceradar/snmp_profiles/srql_target_resolver.ex [77] `if Regex.match?(@uuid_regex, device_uid) do ... else - Logger.warning("SNMPSrqlTargetResolver: invalid device_uid format: #{inspect(device_uid)}") + Logger.warning("#{__MODULE__}: invalid device_uid format: #{inspect(device_uid)}") {:error, :invalid_device_uid} end` `[To ensure code accuracy, apply this suggestion manually]` Suggestion importance[1-10]: 3 __ Why: This is a good code style and maintainability suggestion, as using `__MODULE__` makes the log message more robust to future refactoring of the module name.	Low
	Improve UUID validation to be more flexible Update the UUID validation regex to optionally match hyphens, allowing for both hyphenated and non-hyphenated UUID formats. elixir/serviceradar_core/lib/serviceradar/snmp_profiles/srql_target_resolver.ex [38-39] `# UUID regex for validation - prevents SRQL injection via crafted device UIDs -@uuid_regex ~r/^[0-9a-fA-F]{8}-[0-9a-fA-F]{4}-[0-9a-fA-F]{4}-[0-9a-fA-F]{4}-[0-9a-fA-F]{12}$/ +@uuid_regex ~r/^[0-9a-fA-F]{8}-?[0-9a-fA-F]{4}-?[0-9a-fA-F]{4}-?[0-9a-fA-F]{4}-?[0-9a-fA-F]{12}$/` Apply / Chat Suggestion importance[1-10]: 2 __ Why: While technically correct that UUIDs can be represented without hyphens, the `device_uid` is consistently generated with hyphens throughout the codebase, making this change a minor improvement for a currently non-existent edge case.	Low
Update

Imported GitHub PR comment. Original author: @qodo-code-review[bot] Original URL: https://github.com/carverauto/serviceradar/pull/2300#issuecomment-3750891616 Original created: 2026-01-14T17:59:00Z --- ## PR Code Suggestions ✨  Explore these optional code suggestions: <table><thead><tr><td>Category</td><td align=left>Suggestion                                                                                                                                    </td><td align=center>Impact</td></tr><tbody><tr><td rowspan=5>Possible issue</td> <td> <details><summary>Use correct relationship management type</summary> ___ **In the <code>create</code> action, change the <code>manage_relationship</code> type from <code>:append</code> to <code>:replace</code> to correctly handle the <code>belongs_to</code> association.** [elixir/serviceradar_core/lib/serviceradar/snmp_profiles/snmp_oid_config.ex [66-79]](https://github.com/carverauto/serviceradar/pull/2300/files#diff-6aa0fe00d2c5f5ef6ec080fd9319ae4a3565d41709967832695930a133e23563R66-R79) ```diff create :create do accept [ :oid, :name, :data_type, :scale, :delta ] argument :snmp_target_id, :uuid, allow_nil?: false - change manage_relationship(:snmp_target_id, :snmp_target, type: :append) + change manage_relationship(:snmp_target_id, :snmp_target, type: :replace) change ServiceRadar.Changes.AssignTenantId end ``` - [ ] **Apply / Chat**  <details><summary>Suggestion importance[1-10]: 9</summary> __ Why: The suggestion correctly identifies a bug where an incorrect relationship management type (`:append`) is used for a `belongs_to` relationship, which would cause creation to fail. </details></details></td><td align=center>High </td></tr><tr><td> <details><summary>Replace undefined min usage</summary> ___ **Replace the call to the undefined <code>min</code> function with an explicit length check to prevent a compilation error when slicing a hash string.** [pkg/agent/snmp_service.go [209-213]](https://github.com/carverauto/serviceradar/pull/2300/files#diff-703d839c384ade98beb93a20adb32586e2035374ce27ca4861f7cdee656e1699R209-R213) ```diff +// Shorten hash to up to 8 characters +hashShort := s.configHash +if len(hashShort) > 8 { + hashShort = hashShort[:8] +} s.logger.Info(). Str("source", source). - Str("config_hash", s.configHash[:min(8, len(s.configHash))]). + Str("config_hash", hashShort). Int("target_count", len(config.Targets)). Msg("SNMP agent service started") ``` - [ ] **Apply / Chat**  <details><summary>Suggestion importance[1-10]: 9</summary> __ Why: The suggestion correctly points out the use of an undefined `min` function, which would cause a compilation error, and provides a valid fix. </details></details></td><td align=center>High </td></tr><tr><td> <details><summary>Fix race condition in stop logic</summary> ___ **Fix a race condition in the <code>Stop</code> function by adjusting the mutex locking to ensure safe concurrent access to shared state.** [pkg/agent/snmp_service.go [221-261]](https://github.com/carverauto/serviceradar/pull/2300/files#diff-703d839c384ade98beb93a20adb32586e2035374ce27ca4861f7cdee656e1699R221-R261) ```diff // Stop halts the SNMP service and config refresh loop. // //nolint:dupl // Intentional parallel structure with SysmonService.Stop func (s *SNMPAgentService) Stop(ctx context.Context) error { s.mu.Lock() + defer s.mu.Unlock() if !s.started { - s.mu.Unlock() return nil } // Mark as stopping to prevent concurrent Stop() calls s.started = false // Stop the config refresh loop if s.stopRefresh != nil { close(s.stopRefresh) + + // Create a temporary channel to wait on, as s.refreshDone might be nil + done := s.refreshDone + s.mu.Unlock() // Unlock while waiting + + // Wait for refresh loop to finish (with timeout from context) + if done != nil { + select { + case <-done: + // Refresh loop stopped + case <-ctx.Done(): + s.logger.Warn().Msg("Timeout waiting for SNMP config refresh loop to stop") + case <-time.After(5 * time.Second): + s.logger.Warn().Msg("Timeout waiting for SNMP config refresh loop to stop") + } + } + s.mu.Lock() // Re-lock before continuing } - s.mu.Unlock() - - // Wait for refresh loop to finish (with timeout from context) - if s.refreshDone != nil { - select { - case <-s.refreshDone: - // Refresh loop stopped - case <-ctx.Done(): - s.logger.Warn().Msg("Timeout waiting for SNMP config refresh loop to stop") - case <-time.After(5 * time.Second): - s.logger.Warn().Msg("Timeout waiting for SNMP config refresh loop to stop") - } - } - - s.mu.Lock() - defer s.mu.Unlock() if s.service != nil { if err := s.service.Stop(); err != nil { return fmt.Errorf("failed to stop SNMP service: %w", err) } } s.logger.Info().Msg("SNMP agent service stopped") return nil } ``` `[To ensure code accuracy, apply this suggestion manually]` <details><summary>Suggestion importance[1-10]: 8</summary> __ Why: The suggestion correctly identifies a race condition in the `Stop` function where the mutex is unlocked prematurely, potentially leading to concurrent access issues. </details></details></td><td align=center>Medium </td></tr><tr><td> <details><summary>Use specific error for uniqueness check</summary> ___ **Refine error classification by matching on the specific <code>Ash.Error.Changes.Unique</code> error for uniqueness violations, instead of the generic <code>Ash.Error.Changes.InvalidChanges</code>.** [elixir/serviceradar_core/lib/serviceradar/snmp_profiles/builtin_templates.ex [108-111]](https://github.com/carverauto/serviceradar/pull/2300/files#diff-dfb09a552c8a813238fd1455326888586384c927450dedf18a30344548e3c683R108-R111) ```diff defp classify_ash_error(errors) do # Check if it's a uniqueness error (template already exists) - if Enum.any?(errors, &match?(%Ash.Error.Changes.InvalidChanges{}, &1)), do: :exists, else: :error + if Enum.any?(errors, &match?(%Ash.Error.Changes.Unique{}, &1)), do: :exists, else: :error end ``` - [ ] **Apply / Chat**  <details><summary>Suggestion importance[1-10]: 7</summary> __ Why: This suggestion correctly identifies a bug where any invalid change error would be misclassified as a uniqueness violation, improving the accuracy of error handling during template seeding. </details></details></td><td align=center>Medium </td></tr><tr><td> <details><summary>Log error on credential decryption failure</summary> ___ **Log an error when credential decryption fails to improve debuggability, as it currently fails silently.** [elixir/serviceradar_core/lib/serviceradar/agent_config/compilers/snmp_compiler.ex [214-219]](https://github.com/carverauto/serviceradar/pull/2300/files#diff-3f9551d39c5e77c379f701d5dee9c0a3d95e2a7861ba7515c402e10b585c61e0R214-R219) ```diff defp decrypt_credential(encrypted) do case Vault.decrypt(encrypted) do {:ok, decrypted} -> decrypted - {:error, _} -> nil + {:error, reason} -> + Logger.error("SNMPCompiler: failed to decrypt credential: #{inspect(reason)}") + nil end end ``` - [ ] **Apply / Chat**  <details><summary>Suggestion importance[1-10]: 6</summary> __ Why: The suggestion correctly points out that silent credential decryption failures are problematic for debugging. Adding logging significantly improves observability without changing the function's core behavior. </details></details></td><td align=center>Low </td></tr><tr><td rowspan=3>General</td> <td> <details><summary>Use dynamic community string</summary> ___ **Refactor <code>build_snmp_get_request</code> to accept a dynamic community string from the form. This involves computing packet lengths dynamically to ensure the SNMP connection test uses the user-provided credentials.** [web-ng/lib/serviceradar_web_ng_web/live/settings/snmp_profiles_live/index.ex [1049-1103]](https://github.com/carverauto/serviceradar/pull/2300/files#diff-1493b4100054d62c0d3fdc343e3fb7ce5f21735f752fad7d0fc978ae1ba114e4R1049-R1103) ```diff -defp build_snmp_get_request do - # SNMPv1 GET request structure (ASN.1 BER encoded) - # Request for .1.3.6.1.2.1.1.1.0 (sysDescr.0) with community "public" +defp build_snmp_get_request(community) do + community_bin = to_charlist(community) + community_len = byte_size(community) + # compute PDU length dynamically: version(3) + community tag(2+len) + PDU header(2) + rest(…) + # Here we assume the rest of the PDU after community is fixed 26 bytes + total_len = 1 + 1 + // version + 1 + 1 + community_len + + 1 + 1 + 26 << - # SEQUENCE (total length 0x27 = 39 bytes) - 0x30, - 0x27, - # INTEGER - version (0 = SNMPv1) - 0x02, - 0x01, - 0x00, - # OCTET STRING - community "public" - 0x04, - 0x06, - "public", - # GetRequest-PDU (length 0x1A = 26 bytes) - 0xA0, - 0x1A, - # INTEGER - request-id - 0x02, - 0x04, - 0x00, - 0x00, - 0x00, - 0x01, - # INTEGER - error-status - 0x02, - 0x01, - 0x00, - # INTEGER - error-index - 0x02, - 0x01, - 0x00, - # SEQUENCE - variable-bindings (length 0x0C = 12 bytes) - 0x30, - 0x0C, - # SEQUENCE - single binding (length 0x0A = 10 bytes) - 0x30, - 0x0A, - # OID - .1.3.6.1.2.1.1.1.0 (sysDescr.0) - length 8 - 0x06, - 0x08, - 0x2B, - 0x06, - 0x01, - 0x02, - 0x01, - 0x01, - 0x01, - 0x00, - # NULL value - 0x05, - 0x00 + 0x30, total_len, + # version + 0x02, 0x01, 0x00, + # community + 0x04, community_len, community_bin, + # GetRequest-PDU header + 0xA0, 26, + # rest of the fixed PDU + 0x02, 0x04, 0x00, 0x00, 0x00, 0x01, + 0x02, 0x01, 0x00, + 0x02, 0x01, 0x00, + 0x30, 0x0C, + 0x30, 0x0A, + 0x06, 0x08, 0x2B, 0x06, 0x01, 0x02, 0x01, 0x01, 0x01, 0x00, + 0x05, 0x00 >> end ``` `[To ensure code accuracy, apply this suggestion manually]` <details><summary>Suggestion importance[1-10]: 8</summary> __ Why: The suggestion correctly identifies that the "Test Connection" feature is flawed because it uses a hard-coded "public" community string instead of the one provided by the user, making the test less useful. Implementing this would significantly improve the feature's correctness and utility. </details></details></td><td align=center>Medium </td></tr><tr><td> <details><summary>Use dynamic module name in logs</summary> ___ **Replace the hard-coded module name in the log message with the <code>__MODULE__</code> macro for better maintainability.** [elixir/serviceradar_core/lib/serviceradar/snmp_profiles/srql_target_resolver.ex [77]](https://github.com/carverauto/serviceradar/pull/2300/files#diff-2007e921214ed2cefc6170d60d4c2af9402cc499be1cd15737ec20344b2308acR77-R77) ```diff if Regex.match?(@uuid_regex, device_uid) do ... else - Logger.warning("SNMPSrqlTargetResolver: invalid device_uid format: #{inspect(device_uid)}") + Logger.warning("#{__MODULE__}: invalid device_uid format: #{inspect(device_uid)}") {:error, :invalid_device_uid} end ``` `[To ensure code accuracy, apply this suggestion manually]` <details><summary>Suggestion importance[1-10]: 3</summary> __ Why: This is a good code style and maintainability suggestion, as using `__MODULE__` makes the log message more robust to future refactoring of the module name. </details></details></td><td align=center>Low </td></tr><tr><td> <details><summary>Improve UUID validation to be more flexible</summary> ___ **Update the UUID validation regex to optionally match hyphens, allowing for both hyphenated and non-hyphenated UUID formats.** [elixir/serviceradar_core/lib/serviceradar/snmp_profiles/srql_target_resolver.ex [38-39]](https://github.com/carverauto/serviceradar/pull/2300/files#diff-2007e921214ed2cefc6170d60d4c2af9402cc499be1cd15737ec20344b2308acR38-R39) ```diff # UUID regex for validation - prevents SRQL injection via crafted device UIDs -@uuid_regex ~r/^[0-9a-fA-F]{8}-[0-9a-fA-F]{4}-[0-9a-fA-F]{4}-[0-9a-fA-F]{4}-[0-9a-fA-F]{12}$/ +@uuid_regex ~r/^[0-9a-fA-F]{8}-?[0-9a-fA-F]{4}-?[0-9a-fA-F]{4}-?[0-9a-fA-F]{4}-?[0-9a-fA-F]{12}$/ ``` - [ ] **Apply / Chat**  <details><summary>Suggestion importance[1-10]: 2</summary> __ Why: While technically correct that UUIDs can be represented without hyphens, the `device_uid` is consistently generated with hyphens throughout the codebase, making this change a minor improvement for a currently non-existent edge case. </details></details></td><td align=center>Low </td></tr> <tr><td align="center" colspan="2"> - [ ] Update  </td><td></td></tr></tbody></table>