carverauto/serviceradar

Fork 0

2145 clonedevicerecord shares empty non nil metadata map between original and clone #2587

Merged

mfreeman451 merged 1 commit from refs/pull/2587/head into staging

2025-12-17 00:18:46 +00:00

mfreeman451 commented

2025-12-17 00:12:11 +00:00

(Migrated from github.com)

Owner

Imported from GitHub pull request.

Original GitHub pull request: #2163
Original author: @mfreeman451
Original URL: https://github.com/carverauto/serviceradar/pull/2163
Original created: 2025-12-17T00:12:11Z
Original updated: 2025-12-17T00:18:55Z
Original head: carverauto/serviceradar:2145-clonedevicerecord-shares-empty-non-nil-metadata-map-between-original-and-clone
Original base: staging
Original merged: 2025-12-17T00:18:46Z 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

Bug fix, Enhancement, Tests

Description

Core Bug Fixes:

Fixed potential RWMutex deadlock in SNMP service Check method by removing recursive read locking
Fixed host result aliasing in sweep snapshots by implementing deep-copy semantics for PortResults, PortMap, and ICMPStatus
Fixed SNMP string type conversion error handling with explicit type validation for ObjectDescription and OctetString

Major Enhancements:

Implemented parameterized SRQL query support across MCP server and all query tools to prevent SQL injection
Added partition-scoped device identifier resolution in identity engine with cache key normalization
Refactored OTEL insert operations with generic callback pattern and extracted SQL templates as constants
Added batch execution helper function sendBatchExecAll for consistent error handling across database operations
Implemented Apache AGE Cypher graph query execution support with security checks
Added downsampling functionality for metric time-series queries with configurable aggregation
Added visualization metadata generation for query results with column type and semantic hints
Implemented mutual TLS (mTLS) client certificate authentication support in Rust database layer
Refactored all Rust query modules to support parameterized queries with bind parameter extraction

New Web-NG Foundation:

Established Phoenix LiveView-based web framework with authentication and routing
Integrated Tailwind CSS with daisyUI theme system and custom dark/light mode support
Added custom JavaScript hooks for interactive components (TimeseriesChart, topbar progress)
Implemented Heroicons SVG icon integration via Tailwind CSS plugin
Created Rust NIF module for SRQL query translation to Elixir integration

Docker & Infrastructure:

Enhanced certificate generation with support for extra CNPG certificate IPs and workstation certificates
Updated nginx entrypoint to use web-ng service on port 4000

Testing:

Added comprehensive deep-copy validation tests for host result snapshots
Added parameterized query binding tests for devices, logs, and events
Added SNMP type conversion error handling tests
Added batch operation error handling and execution tests
Added static analysis test to prevent RWMutex deadlock patterns
Added partition-scoped identifier resolution tests
Added poller status upsert validation tests

Diagram Walkthrough

flowchart LR
  A["SNMP Service<br/>RWMutex Fix"] --> B["Bug Fixes"]
  C["Host Result<br/>Deep-Copy"] --> B
  D["String Type<br/>Conversion"] --> B
  
  E["Parameterized<br/>SRQL Queries"] --> F["Enhancements"]
  G["Partition-Scoped<br/>Identifiers"] --> F
  H["OTEL Generic<br/>Callback Pattern"] --> F
  I["Batch Helper<br/>Function"] --> F
  J["Apache AGE<br/>Cypher Support"] --> F
  K["Metric<br/>Downsampling"] --> F
  L["Query Viz<br/>Metadata"] --> F
  M["mTLS Client<br/>Certificates"] --> F
  
  N["Phoenix LiveView<br/>Framework"] --> O["Web-NG Foundation"]
  P["Tailwind CSS<br/>+ daisyUI"] --> O
  Q["Custom JS<br/>Hooks"] --> O
  R["Rust NIF<br/>Integration"] --> O
  
  S["Deep-Copy<br/>Tests"] --> T["Comprehensive Tests"]
  U["Parameterized<br/>Query Tests"] --> T
  V["SNMP Error<br/>Tests"] --> T
  W["Batch Operation<br/>Tests"] --> T
  X["Deadlock Prevention<br/>Tests"] --> T
  
  B --> Y["Complete PR"]
  F --> Y
  O --> Y
  T --> Y

File Walkthrough

Relevant files

Enhancement

41 files

cnpg_observability.go `Refactor OTEL insert operations with generic callback pattern` pkg/db/cnpg_observability.go Refactored OTEL insert operations to use a generic `insertOTELRows` function with callback-based row handling Extracted SQL templates as module-level constants (`otelLogsInsertSQL`, `otelMetricsInsertSQL`, `otelTracesInsertSQL`) Created `otelRowInserter` interface and type-specific implementations (`otelLogInserter`, `otelMetricInserter`, `otelTraceInserter`) to encapsulate row data and queuing logic Added `unit` field to `otelMetricsInsertSQL` query	+188/-131
identity_engine.go `Add partition-scoped device identifier resolution` pkg/registry/identity_engine.go Updated cache key format to include partition: `"::"` instead of `":"` Added `strongIdentifierCacheKey` helper function to normalize partition and build cache keys Refactored `batchLookupByStrongIdentifiers` to group updates by partition and process each partition separately Added helper functions `groupUpdatesByPartition`, `collectStrongIdentifierSets`, and `batchLookupIdentifierType` for partition-scoped lookups Updated `BatchGetDeviceIDsByIdentifier` calls to include partition parameter	+124/-54
query_utils.go `Implement parameterized SRQL query support` pkg/mcp/query_utils.go Added `ParameterizedQueryExecutor` interface extending `QueryExecutor` with `ExecuteSRQLQueryWithParams` method Created `srqlBindBuilder` type for parameter binding with `Bind` method returning `$N` placeholders Modified query builders (`buildLogQuery`, `buildRecentLogsQuery`, `buildDevicesQuery`) to return tuples of `(string, []any)` with parameterized queries Added `executeSRQL` helper to route queries to parameterized or plain executor based on parameter presence Updated all query execution calls to use parameterized approach	+43/-20
builder.go `Add parameter binding to generic filter builder` pkg/mcp/builder.go Updated `FilterHandlerFunc` and `FilterBuilder.BuildFilters` signatures to accept `*srqlBindBuilder` parameter Modified `GenericFilterBuilder.BuildFilters` to use parameter binding instead of string concatenation Added sorted iteration over field mappings for deterministic filter ordering Updated `BuildGenericFilterTool` to create `srqlBindBuilder` and pass to filter handlers	+20/-7
server.go `Migrate MCP server to parameterized queries` pkg/mcp/server.go Updated `executeGetDevice` to use parameterized query with `$1` placeholder Modified `executeQueryEvents` to build parameterized queries with `srqlBindBuilder` Added `executeSRQLQueryWithParams` method to route queries to parameterized executor Updated `executeSRQLQuery` to delegate to `executeSRQLQueryWithParams` with empty params	+24/-7
sweep.go `Add HostResult deep-copy function` pkg/models/sweep.go Added `DeepCopyHostResult` function that creates a snapshot copy without aliasing pointer/slice/map fields Function handles `PortResults` slice, `PortMap` map, and `ICMPStatus` pointer with proper deep-copying Maintains referential integrity between `PortResults` and `PortMap` in the copy	+84/-1
tools_sweeps.go `Parameterize sweep tool queries` pkg/mcp/tools_sweeps.go Updated `registerGetRecentSweepsTool` to use parameterized query for `poller_id` filter Updated `registerGetSweepSummaryTool` to use parameterized query for `poller_id` filter Both tools now call `executeSRQLQueryWithParams` instead of `executeSRQLQuery`	+8/-4
cnpg_identity_engine.go `Add partition filtering to identifier lookup` pkg/db/cnpg_identity_engine.go Updated `batchGetDeviceIDsByIdentifierSQL` to include partition filter in WHERE clause Updated `BatchGetDeviceIDsByIdentifier` function signature to accept `partition` parameter Added partition normalization with default value fallback Updated query execution to pass partition as third parameter	+8/-3
tools_events.go `Parameterize alert tool queries` pkg/mcp/tools_events.go Updated `registerGetAlertsTool` to use parameterized query for `poller_id` filter Changed from string concatenation to `$1` placeholder with parameter binding Updated to call `executeSRQLQueryWithParams` instead of `executeSRQLQuery`	+4/-2
pgx_batch_helper.go `Add batch execution helper function` pkg/db/pgx_batch_helper.go Added `sendBatchExecAll` helper function for executing batch operations with error handling Function handles empty batches, iterates through batch commands, and ensures proper cleanup Returns formatted error messages with operation name and command index on failure	+29/-0
tools_devices.go `Parameterize device tools queries` pkg/mcp/tools_devices.go Updated device ID filter to use parameterized query with `$1` placeholder Changed from string concatenation to parameter binding Updated to call `executeSRQLQueryWithParams` instead of `executeSRQLQuery`	+3/-3
tools_logs.go `Parameterize log tool queries` pkg/mcp/tools_logs.go Updated `registerLogTools` to use parameterized query for `poller_id` filter Changed from string concatenation to `$1` placeholder with parameter binding Updated to call `executeSRQLQueryWithParams` instead of `executeSRQLQuery`	+4/-2
cnpg_unified_devices.go `Use batch helper for device deletion audit` pkg/db/cnpg_unified_devices.go Replaced manual batch result handling with `sendBatchExecAll` helper function Simplified error handling for audit log batch execution	+1/-2
auth.go `Use batch helper for user storage` pkg/db/auth.go Replaced manual batch result handling with `sendBatchExecAll` helper function Simplified error handling for user batch storage	+1/-2
viz.rs `Add visualization metadata generation for query results` rust/srql/src/query/viz.rs New file implementing visualization metadata generation for query results Defines `VizMeta`, `ColumnMeta`, `ColumnType`, `ColumnSemantic` structs for describing result columns Implements `meta_for_plan()` function that returns visualization suggestions and column metadata for different entity types Supports timeseries and table visualization kinds with semantic hints for columns	+605/-0
downsample.rs `Add downsampling support for metric time-series queries` rust/srql/src/query/downsample.rs New file implementing downsampling functionality for metric queries Provides `to_sql_and_params()` and `execute()` functions to build and run downsampled queries Supports time bucketing with configurable aggregation functions (avg, min, max, sum, count) Implements series grouping and filtering for various metric entity types	+521/-0
devices.rs `Refactor devices query to support stats and bind parameters` rust/srql/src/query/devices.rs Refactored `to_debug_sql()` to `to_sql_and_params()` returning SQL and bind parameters Added stats query support with `parse_stats_spec()` and `build_stats_query()` for count aggregations Implemented `collect_filter_params()` to extract bind parameters from device filters Added validation for stats expressions and parameter binding	+195/-7
services.rs `Refactor services query to support stats and bind parameters` rust/srql/src/query/services.rs Refactored `to_debug_sql()` to `to_sql_and_params()` returning SQL and bind parameters Added stats query support with `parse_stats_spec()` and `build_stats_query()` for count aggregations Implemented `collect_filter_params()` to extract bind parameters from service filters Added parameter binding validation and reconciliation	+172/-4
parser.rs `Add downsampling and graph cypher entity parsing support` rust/srql/src/parser.rs Added `GraphCypher` entity type for graph database queries Introduced `DownsampleSpec` and `DownsampleAgg` for downsampling configuration Implemented parsing for `bucket`, `agg`, and `series` downsampling parameters Enhanced stats parsing to support `as alias` syntax for result aliasing	+157/-2
graph_cypher.rs `Add Apache AGE Cypher graph query execution support` rust/srql/src/query/graph_cypher.rs New file implementing Apache AGE Cypher query execution for graph database Provides `execute()` and `to_sql_and_params()` functions for running read-only Cypher queries Implements security checks to prevent write operations and SQL injection Handles result transformation for nodes, edges, and generic row data	+208/-0
interfaces.rs `Refactor interfaces query to support bind parameters` rust/srql/src/query/interfaces.rs Refactored `to_debug_sql()` to `to_sql_and_params()` returning SQL and bind parameters Implemented `collect_filter_params()` to extract bind parameters from interface filters Added parameter binding validation and reconciliation with limit/offset Updated stats query handling to return formatted SQL with parameters	+112/-8
traces.rs `Refactor traces query to support bind parameters` rust/srql/src/query/traces.rs Refactored `to_debug_sql()` to `to_sql_and_params()` returning SQL and bind parameters Implemented `collect_filter_params()` and `collect_text_params()` for trace filter parameters Added support for integer list filtering on status_code and span_kind fields Added parameter binding validation	+109/-4
logs.rs `Refactor logs query to support bind parameters` rust/srql/src/query/logs.rs Refactored `to_debug_sql()` to `to_sql_and_params()` returning SQL and bind parameters Implemented `collect_filter_params()` for log filter parameter extraction Added support for severity_number integer filtering with list operations Enhanced stats query handling with parameter conversion	+103/-3
cpu_metrics.rs `Refactor CPU metrics query to support bind parameters` rust/srql/src/query/cpu_metrics.rs Refactored `to_debug_sql()` to `to_sql_and_params()` returning SQL and bind parameters Implemented `collect_filter_params()` for CPU metric filter parameter extraction Added support for numeric filtering on core_id, usage_percent, and frequency_hz Enhanced stats query handling with parameter conversion	+85/-5
pollers.rs `Refactor pollers query to support bind parameters` rust/srql/src/query/pollers.rs Refactored `to_debug_sql()` to `to_sql_and_params()` returning SQL and bind parameters Implemented `collect_filter_params()` for poller filter parameter extraction Added boolean filter support for `is_healthy` field Added parameter binding validation and reconciliation	+81/-14
otel_metrics.rs `Refactor OTEL metrics query to support bind parameters` rust/srql/src/query/otel_metrics.rs Refactored `to_debug_sql()` to `to_sql_and_params()` returning SQL and bind parameters Implemented `collect_filter_params()` for OTEL metric filter parameter extraction Added boolean filter support for `is_slow` field Enhanced stats query handling with parameter conversion	+85/-5
db.rs `Add mutual TLS (mTLS) client certificate authentication support` rust/srql/src/db.rs Added support for client certificate and key authentication in TLS connections Implemented `load_client_certs()` and `load_client_key()` functions for certificate loading Enhanced `build_client_config()` to support mutual TLS (mTLS) authentication Updated `PgConnectionManager::new()` to accept optional client cert and key paths	+77/-10
timeseries_metrics.rs `Refactor timeseries metrics query to support bind parameters` rust/srql/src/query/timeseries_metrics.rs Refactored `to_debug_sql()` to `to_sql_and_params()` returning SQL and bind parameters Implemented `collect_filter_params()` for timeseries metric filter parameter extraction Added support for numeric filtering on if_index and value fields Added parameter binding validation and reconciliation	+82/-4
process_metrics.rs `Refactor process metrics query to support bind parameters` rust/srql/src/query/process_metrics.rs Refactored `to_debug_sql()` to `to_sql_and_params()` returning SQL and bind parameters Implemented `collect_filter_params()` for process metric filter parameter extraction Added support for numeric filtering on pid, cpu_usage, and memory_usage fields Added parameter binding validation and reconciliation	+77/-4
trace_summaries.rs `Refactor trace summaries query to support bind parameters` rust/srql/src/query/trace_summaries.rs Refactored `to_debug_sql()` to `to_sql_and_params()` returning SQL and bind parameters Implemented `bind_param_from_query()` to convert SQL bind values to `BindParam` type Updated placeholder rewriting for parameter binding	+20/-3
models.rs `Add unit field to OTEL metric row model` rust/srql/src/models.rs Added `unit` field to `OtelMetricRow` struct for storing metric unit information Updated `into_json()` method to include the new `unit` field in JSON serialization	+2/-0
disk_metrics.rs `Refactor SQL generation to extract and return bind parameters` rust/srql/src/query/disk_metrics.rs Refactored `to_debug_sql` function to `to_sql_and_params` returning both SQL string and bind parameters Added `collect_text_params` and `collect_filter_params` helper functions to extract filter parameters Implemented bind parameter collection for time ranges, filters, and limit/offset clauses Added validation to ensure bind count matches between diesel query and collected parameters	+72/-4
memory_metrics.rs `Refactor SQL generation to extract and return bind parameters` rust/srql/src/query/memory_metrics.rs Refactored `to_debug_sql` function to `to_sql_and_params` returning both SQL string and bind parameters Added `collect_text_params` and `collect_filter_params` helper functions to extract filter parameters Implemented bind parameter collection for time ranges, filters, and limit/offset clauses Added validation to ensure bind count matches between diesel query and collected parameters	+73/-4
device_updates.rs `Refactor SQL generation to extract and return bind parameters` rust/srql/src/query/device_updates.rs Refactored `to_debug_sql` function to `to_sql_and_params` returning both SQL string and bind parameters Added `collect_text_params` with `allow_lists` parameter to control list filter support per field Implemented `collect_filter_params` to handle device-specific filter fields including boolean values Added validation to ensure bind count matches between diesel query and collected parameters	+78/-4
events.rs `Refactor SQL generation to extract and return bind parameters` rust/srql/src/query/events.rs Refactored `to_debug_sql` function to `to_sql_and_params` returning both SQL string and bind parameters Added `collect_text_params` and `collect_filter_params` helper functions to extract filter parameters Implemented bind parameter collection for time ranges, filters, and limit/offset clauses Added validation to ensure bind count matches between diesel query and collected parameters	+71/-4
lib.rs `Add Rust NIF for SRQL query translation to Elixir` web-ng/native/srql_nif/src/lib.rs Created new Rust NIF module for Elixir integration with SRQL query translation Implemented `translate` function as a dirty CPU NIF that converts SRQL queries to database queries Added error handling with atom-based responses for success and failure cases Integrated with `srql::query::translate_request` for query processing	+49/-0
lib.rs `Export query types and add embedded SRQL support` rust/srql/src/lib.rs Exported public types `QueryDirection`, `QueryEngine`, `QueryRequest`, `QueryResponse`, `TranslateRequest`, and `TranslateResponse` Added `EmbeddedSrql` struct to support embedded SRQL usage with connection pooling Implemented `EmbeddedSrql::new` constructor for initializing with `AppConfig`	+19/-0
schema.rs `Add unit field to events table schema` rust/srql/src/schema.rs Added `unit` field to the events table schema as nullable text column	+1/-0
generate-certs.sh `Add support for extra CNPG certificate IPs and workstation cert` docker/compose/generate-certs.sh Added logic to check if CNPG certificate is missing required SAN IPs and regenerate if needed Implemented support for `CNPG_CERT_EXTRA_IPS` environment variable to add additional IP addresses to CNPG certificate Added generation of new `workstation` certificate for developers connecting from outside Docker network Refactored certificate existence check to handle conditional regeneration	+28/-6
app.js `Add Phoenix LiveSocket setup and custom hooks` web-ng/assets/js/app.js Initialized Phoenix LiveSocket with custom hooks and colocated hooks Implemented `TimeseriesChart` hook for interactive chart tooltips and hover line visualization Added topbar progress bar configuration for page loading feedback Configured development mode features for server log streaming and editor integration	+142/-0
router.ex `Add Phoenix router with authentication and LiveView routes` web-ng/lib/serviceradar_web_ng_web/router.ex Defined browser and API pipelines with authentication and CSRF protection Added routes for home page, API query execution, and device management endpoints Configured LiveView sessions for authenticated users with multiple dashboard pages Implemented authentication routes for user registration, login, and logout	+103/-0

Tests

9 files

summary_snapshot_test.go `Add snapshot deep-copy verification tests` pkg/sweeper/summary_snapshot_test.go Added test `TestGetSummary_HostResultsDoNotAliasInternalState` verifying deep-copy semantics for host snapshots Added test `TestGetSummary_ConcurrentReadsDoNotPanic` ensuring concurrent access to snapshots doesn't panic Tests validate that `ICMPStatus` and `PortMap`/`PortResults` are properly deep-copied and not aliased	+157/-0
parameterized_queries_test.go `Add parameterized query binding tests` pkg/mcp/parameterized_queries_test.go Added test `TestDevicesGetDeviceBindsDeviceID` verifying device ID parameter binding Added test `TestLogsGetRecentLogsBindsPollerID` verifying poller ID parameter binding Added test `TestEventsGetEventsBindsMappedFilters` verifying multiple filter parameter binding Added test `TestStructuredToolsRequireParameterizedExecutor` ensuring non-parameterized executors are rejected Tests use `recordingParameterizedExecutor` mock to verify query and parameter handling	+123/-0
sweep_deepcopy_test.go `Add deep-copy validation tests for HostResult` pkg/models/sweep_deepcopy_test.go Added comprehensive test `TestDeepCopyHostResult_NoAliasing` validating deep-copy semantics Tests verify scalar fields match, pointer fields are copied, and mutations don't affect source Validates `PortResults` slice, `PortMap` map, and `ICMPStatus` are properly deep-copied	+78/-0
service_deadlock_test.go `Add deadlock prevention static analysis test` pkg/checker/snmp/service_deadlock_test.go Added static analysis test `TestCheckDoesNotRLockAndCallGetStatus` using AST parsing Test embeds `service.go` source and verifies `Check` method doesn't hold `s.mu.RLock` while calling `GetStatus` Prevents potential deadlock from recursive RWMutex read locking	+101/-0
pgx_batch_helper_test.go `Add batch execution helper tests` pkg/db/pgx_batch_helper_test.go Added test `TestSendBatchExecAll_EmptyBatchDoesNotSend` verifying empty batches skip execution Added test `TestSendBatchExecAll_ExecErrorIncludesCommandIndexAndCloses` verifying error reporting with command index Added test `TestSendBatchExecAll_CloseErrorReturnedWhenExecSucceeds` verifying close errors are surfaced Tests use `fakeBatchResults` mock to simulate batch execution scenarios	+99/-0
client_conversion_test.go `Add SNMP type conversion tests` pkg/checker/snmp/client_conversion_test.go Added test `TestConvertVariable_OctetStringBytes` verifying `[]byte` conversion to string Added test `TestConvertVariable_ObjectDescriptionBytes` verifying `[]byte` conversion for object descriptions Added test `TestConvertVariable_StringTypesUnexpectedValueDoNotPanic` verifying error handling for unexpected types Tests ensure conversions don't panic and return appropriate errors	+95/-0
pgx_batch_behavior_test.go `Add batch operation error handling tests` pkg/db/pgx_batch_behavior_test.go Added test `TestInsertEvents_SurfacesBatchInsertErrors` verifying batch insert error propagation Added test `TestStoreBatchUsers_SurfacesBatchInsertErrors` verifying user batch error handling Tests use `fakePgxExecutor` and `fakeBatchResults` mocks to simulate batch failures	+79/-0
identity_engine_partition_test.go `Add partition-scoped identifier resolution test` pkg/registry/identity_engine_partition_test.go Added test `TestBatchLookupByStrongIdentifiers_PartitionScoped` verifying partition-scoped identifier resolution Test validates that different partitions resolve to different device IDs for same MAC address Uses mock to verify `BatchGetDeviceIDsByIdentifier` is called with correct partition parameter	+51/-0
cnpg_registry_test.go `Add poller status upsert validation test` pkg/db/cnpg_registry_test.go Added test `TestUpsertPollerStatusSQL_PreservesRegistrationMetadata` verifying upsert query structure Test validates that registration metadata fields are not overwritten on conflict Ensures only `last_seen`, `is_healthy`, and `updated_at` are updated	+22/-0

Bug fix

4 files

client.go `Fix SNMP string type conversion error handling` pkg/checker/snmp/client.go Removed `ObjectDescription` and `OctetString` from conversion map to handle them separately Updated `convertObjectDescription` and `convertOctetString` to return `(interface{}, error)` and validate `[]byte` type Added explicit type checks with error handling for string conversion types Removed unused conversion functions from map and added early-return checks before map lookup	+37/-21
memory_store.go `Use deep-copy for host result snapshots` pkg/sweeper/memory_store.go Updated `convertToSlice` to use `DeepCopyHostResult` instead of shallow copy Updated `buildSummary` to use `DeepCopyHostResult` for host snapshots Fixed struct field alignment in `InMemoryStore` and initialization for consistency	+15/-15
base_processor.go `Use deep-copy for processor host snapshots` pkg/sweeper/base_processor.go Updated `collectShardSummaries` to use `DeepCopyHostResult` for host snapshots Updated `processShardForSummary` to use `DeepCopyHostResult` when sending hosts to channel Fixed trailing whitespace in `processTCPResult` call	+3/-3
service.go `Fix potential RWMutex deadlock in Check method` pkg/checker/snmp/service.go Removed `s.mu.RLock()` and `defer s.mu.RUnlock()` from `Check` method Added comment explaining deadlock prevention: `GetStatus` performs its own locking Prevents recursive RWMutex read locking that can deadlock with write-preferring semantics	+3/-4

Miscellaneous

4 files

mock_db.go `Update mock for partition-scoped identifier lookup` pkg/db/mock_db.go Updated `BatchGetDeviceIDsByIdentifier` mock signature to include `partition` parameter Updated mock recorder method to accept partition argument Added newline formatting for consistency	+6/-5
registry_test.go `Update registry tests for partition parameter` pkg/registry/registry_test.go Updated all `BatchGetDeviceIDsByIdentifier` mock expectations to include partition parameter Changes applied to `allowCanonicalizationQueries`, `TestProcessBatchDeviceUpdates_MergesSweepIntoCanonicalDevice`, and other test functions	+5/-5
canon_simulation_test.go `Update DIRE simulation test for partition keys` pkg/registry/canon_simulation_test.go Updated `setupDIREMockDB` to use `strongIdentifierCacheKey` for cache key generation Updated mock `BatchGetDeviceIDsByIdentifier` to accept partition parameter Updated `UpsertDeviceIdentifiers` mock to use `strongIdentifierCacheKey`	+4/-4
interfaces.go `Update Service interface for partition parameter` pkg/db/interfaces.go Updated `BatchGetDeviceIDsByIdentifier` interface signature to include `partition` parameter	+1/-1

Formatting

1 files

server_test.go

Fix test file indentation

pkg/mcp/server_test.go

Fixed indentation from spaces to tabs for Go style consistency

+5/-5

Configuration changes

2 files

entrypoint-nginx.sh `Update nginx entrypoint to use web-ng service` docker/compose/entrypoint-nginx.sh Changed nginx upstream service from `web` on port 3000 to `web-ng` on port 4000 Removed wait logic for `core` service on port 8090 Simplified entrypoint to only wait for web-ng service	+4/-12
app.css `Configure Tailwind CSS with daisyUI and custom themes` web-ng/assets/css/app.css Configured Tailwind CSS with source scanning for CSS, JS, and template files Integrated daisyUI plugin with custom dark and light theme configurations Added custom Tailwind variants for LiveView loading states and dark mode Configured transparent display for LiveView wrapper divs	+105/-0

Dependencies

3 files

daisyui-theme.js `Add daisyUI theme plugin bundle for UI styling` web-ng/assets/vendor/daisyui-theme.js Added daisyUI theme plugin bundle with support for multiple color themes Implemented theme configuration system with customizable color schemes and design tokens Included MIT licensed daisyUI theme definitions for light and dark modes	+124/-0
topbar.js `Add topbar progress bar library` web-ng/assets/vendor/topbar.js Added topbar progress bar library for visual feedback during page navigation Implemented canvas-based progress bar with gradient colors and shadow effects Provided configuration options for bar thickness, colors, and animation behavior	+138/-0
heroicons.js `Add Heroicons Tailwind CSS plugin` web-ng/assets/vendor/heroicons.js Created Tailwind CSS plugin for Heroicons SVG icon integration Implemented dynamic icon loading from multiple icon sets (outline, solid, mini, micro) Added CSS mask-based icon rendering with customizable sizing	+43/-0

Additional files

101 files

.env.example	+18/-1
.tool-versions	+2/-0
AGENTS.md	+46/-0
docker-compose.yml	+27/-119
Dockerfile.web-ng	+71/-0
nginx.conf.template	+6/-138
AGENTS.md	+72/-0
design.md	+0/-29
proposal.md	+0/-25
spec.md	+0/-148
tasks.md	+0/-68
proposal.md	+0/-26
spec.md	+0/-34
tasks.md	+0/-13
proposal.md	+0/-68
proposal.md	+0/-42
tasks.md	+0/-20
design.md	+0/-26
proposal.md	+0/-16
spec.md	+0/-85
tasks.md	+0/-25
design.md	+114/-0
proposal.md	+48/-0
spec.md	+24/-0
spec.md	+22/-0
tasks.md	+83/-0
design.md	+0/-29
proposal.md	+0/-21
spec.md	+0/-23
tasks.md	+0/-19
design.md	+62/-0
proposal.md	+25/-0
spec.md	+20/-0
tasks.md	+28/-0
proposal.md	+0/-14
spec.md	+0/-28
spec.md	+0/-35
tasks.md	+0/-20
design.md	+208/-0
proposal.md	+54/-0
tasks.md	+220/-0
proposal.md	+0/-18
spec.md	+0/-61
tasks.md	+0/-13
proposal.md	+0/-45
spec.md	+0/-60
tasks.md	+0/-28
proposal.md	+0/-83
spec.md	+0/-112
tasks.md	+0/-84
proposal.md	+0/-44
spec.md	+0/-32
tasks.md	+0/-45
proposal.md	+0/-26
spec.md	+0/-46
tasks.md	+0/-24
proposal.md	+0/-19
spec.md	+0/-18
tasks.md	+0/-19
design.md	+32/-0
proposal.md	+22/-0
spec.md	+13/-0
tasks.md	+17/-0
design.md	+30/-0
proposal.md	+15/-0
spec.md	+40/-0
tasks.md	+18/-0
design.md	+39/-0
proposal.md	+19/-0
spec.md	+23/-0
tasks.md	+13/-0
design.md	+43/-0
proposal.md	+20/-0
spec.md	+27/-0
tasks.md	+10/-0
proposal.md	+0/-144
spec.md	+0/-81
tasks.md	+0/-43
proposal.md	+0/-14
spec.md	+0/-14
tasks.md	+0/-17
proposal.md	+16/-0
spec.md	+10/-0
tasks.md	+12/-0
proposal.md	+17/-0
spec.md	+20/-0
tasks.md	+13/-0
proposal.md	+0/-39
spec.md	+0/-70
tasks.md	+0/-33
proposal.md	+28/-0
spec.md	+21/-0
tasks.md	+17/-0
proposal.md	+0/-102
spec.md	+0/-46
tasks.md	+0/-53
proposal.md	+0/-125
proposal.md	+0/-129
spec.md	+0/-22
tasks.md	+0/-15
Additional files not shown

Imported from GitHub pull request. Original GitHub pull request: #2163 Original author: @mfreeman451 Original URL: https://github.com/carverauto/serviceradar/pull/2163 Original created: 2025-12-17T00:12:11Z Original updated: 2025-12-17T00:18:55Z Original head: carverauto/serviceradar:2145-clonedevicerecord-shares-empty-non-nil-metadata-map-between-original-and-clone Original base: staging Original merged: 2025-12-17T00:18:46Z 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** Bug fix, Enhancement, Tests ___ ### **Description** **Core Bug Fixes:** - Fixed potential RWMutex deadlock in SNMP service `Check` method by removing recursive read locking - Fixed host result aliasing in sweep snapshots by implementing deep-copy semantics for `PortResults`, `PortMap`, and `ICMPStatus` - Fixed SNMP string type conversion error handling with explicit type validation for `ObjectDescription` and `OctetString` **Major Enhancements:** - Implemented parameterized SRQL query support across MCP server and all query tools to prevent SQL injection - Added partition-scoped device identifier resolution in identity engine with cache key normalization - Refactored OTEL insert operations with generic callback pattern and extracted SQL templates as constants - Added batch execution helper function `sendBatchExecAll` for consistent error handling across database operations - Implemented Apache AGE Cypher graph query execution support with security checks - Added downsampling functionality for metric time-series queries with configurable aggregation - Added visualization metadata generation for query results with column type and semantic hints - Implemented mutual TLS (mTLS) client certificate authentication support in Rust database layer - Refactored all Rust query modules to support parameterized queries with bind parameter extraction **New Web-NG Foundation:** - Established Phoenix LiveView-based web framework with authentication and routing - Integrated Tailwind CSS with daisyUI theme system and custom dark/light mode support - Added custom JavaScript hooks for interactive components (TimeseriesChart, topbar progress) - Implemented Heroicons SVG icon integration via Tailwind CSS plugin - Created Rust NIF module for SRQL query translation to Elixir integration **Docker & Infrastructure:** - Enhanced certificate generation with support for extra CNPG certificate IPs and workstation certificates - Updated nginx entrypoint to use web-ng service on port 4000 **Testing:** - Added comprehensive deep-copy validation tests for host result snapshots - Added parameterized query binding tests for devices, logs, and events - Added SNMP type conversion error handling tests - Added batch operation error handling and execution tests - Added static analysis test to prevent RWMutex deadlock patterns - Added partition-scoped identifier resolution tests - Added poller status upsert validation tests ___ ### Diagram Walkthrough ```mermaid flowchart LR A["SNMP Service RWMutex Fix"] --> B["Bug Fixes"] C["Host Result Deep-Copy"] --> B D["String Type Conversion"] --> B E["Parameterized SRQL Queries"] --> F["Enhancements"] G["Partition-Scoped Identifiers"] --> F H["OTEL Generic Callback Pattern"] --> F I["Batch Helper Function"] --> F J["Apache AGE Cypher Support"] --> F K["Metric Downsampling"] --> F L["Query Viz Metadata"] --> F M["mTLS Client Certificates"] --> F N["Phoenix LiveView Framework"] --> O["Web-NG Foundation"] P["Tailwind CSS + daisyUI"] --> O Q["Custom JS Hooks"] --> O R["Rust NIF Integration"] --> O S["Deep-Copy Tests"] --> T["Comprehensive Tests"] U["Parameterized Query Tests"] --> T V["SNMP Error Tests"] --> T W["Batch Operation Tests"] --> T X["Deadlock Prevention Tests"] --> T B --> Y["Complete PR"] F --> Y O --> Y T --> Y ``` <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>41 files</summary><table> <tr> <td> <details> <summary>cnpg_observability.go<dd><code>Refactor OTEL insert operations with generic callback pattern</code></dd></summary> <hr> pkg/db/cnpg_observability.go <ul><li>Refactored OTEL insert operations to use a generic <code>insertOTELRows</code> function with callback-based row handling <li> Extracted SQL templates as module-level constants (<code>otelLogsInsertSQL</code>, <code>otelMetricsInsertSQL</code>, <code>otelTracesInsertSQL</code>) <li> Created <code>otelRowInserter</code> interface and type-specific implementations (<code>otelLogInserter</code>, <code>otelMetricInserter</code>, <code>otelTraceInserter</code>) to encapsulate row data and queuing logic <li> Added <code>unit</code> field to <code>otelMetricsInsertSQL</code> query</ul> </details> </td> <td><a href="https://github.com/carverauto/serviceradar/pull/2163/files#diff-abaae95adb6fa6d0241553f6d1b39ecff3dd6159db72babbe39dfb3cd231cd3d">+188/-131</a></td> </tr> <tr> <td> <details> <summary>identity_engine.go<dd><code>Add partition-scoped device identifier resolution</code>                </dd></summary> <hr> pkg/registry/identity_engine.go <ul><li>Updated cache key format to include partition: <code>"<partition>:<type>:<value>"</code> instead of <code>"<type>:<value>"</code> <li> Added <code>strongIdentifierCacheKey</code> helper function to normalize partition and build cache keys <li> Refactored <code>batchLookupByStrongIdentifiers</code> to group updates by partition and process each partition separately <li> Added helper functions <code>groupUpdatesByPartition</code>, <code>collectStrongIdentifierSets</code>, and <code>batchLookupIdentifierType</code> for partition-scoped lookups <li> Updated <code>BatchGetDeviceIDsByIdentifier</code> calls to include partition parameter</ul> </details> </td> <td><a href="https://github.com/carverauto/serviceradar/pull/2163/files#diff-496f24b3784656e1d6bb97cafe5c528bbecea5a0b74b4be578d3b83e858038c7">+124/-54</a></td> </tr> <tr> <td> <details> <summary>query_utils.go<dd><code>Implement parameterized SRQL query support</code>                              </dd></summary> <hr> pkg/mcp/query_utils.go <ul><li>Added <code>ParameterizedQueryExecutor</code> interface extending <code>QueryExecutor</code> with <code>ExecuteSRQLQueryWithParams</code> method <li> Created <code>srqlBindBuilder</code> type for parameter binding with <code>Bind</code> method returning <code>$N</code> placeholders <li> Modified query builders (<code>buildLogQuery</code>, <code>buildRecentLogsQuery</code>, <code>buildDevicesQuery</code>) to return tuples of <code>(string, []any)</code> with parameterized queries <li> Added <code>executeSRQL</code> helper to route queries to parameterized or plain executor based on parameter presence <li> Updated all query execution calls to use parameterized approach</ul> </details> </td> <td><a href="https://github.com/carverauto/serviceradar/pull/2163/files#diff-6a33491056954507b65b8252854786d3d9c65534c7c88700fcc2592f808b192b">+43/-20</a>  </td> </tr> <tr> <td> <details> <summary>builder.go<dd><code>Add parameter binding to generic filter builder</code>                    </dd></summary> <hr> pkg/mcp/builder.go <ul><li>Updated <code>FilterHandlerFunc</code> and <code>FilterBuilder.BuildFilters</code> signatures to accept <code>*srqlBindBuilder</code> parameter <li> Modified <code>GenericFilterBuilder.BuildFilters</code> to use parameter binding instead of string concatenation <li> Added sorted iteration over field mappings for deterministic filter ordering <li> Updated <code>BuildGenericFilterTool</code> to create <code>srqlBindBuilder</code> and pass to filter handlers</ul> </details> </td> <td><a href="https://github.com/carverauto/serviceradar/pull/2163/files#diff-ee3c16d7d8882a3a65d28bfedf7f0a5e59698b00ae482bc5565b3314503d5db2">+20/-7</a>    </td> </tr> <tr> <td> <details> <summary>server.go<dd><code>Migrate MCP server to parameterized queries</code>                            </dd></summary> <hr> pkg/mcp/server.go <ul><li>Updated <code>executeGetDevice</code> to use parameterized query with <code>$1</code> placeholder <li> Modified <code>executeQueryEvents</code> to build parameterized queries with <code>srqlBindBuilder</code> <li> Added <code>executeSRQLQueryWithParams</code> method to route queries to parameterized executor <li> Updated <code>executeSRQLQuery</code> to delegate to <code>executeSRQLQueryWithParams</code> with empty params</ul> </details> </td> <td><a href="https://github.com/carverauto/serviceradar/pull/2163/files#diff-bbb88ac338d81b0a37cd59fc53d68171d2de4780653253a2fd257f18fbf81bd0">+24/-7</a>    </td> </tr> <tr> <td> <details> <summary>sweep.go<dd><code>Add HostResult deep-copy function</code>                                                </dd></summary> <hr> pkg/models/sweep.go <ul><li>Added <code>DeepCopyHostResult</code> function that creates a snapshot copy without aliasing pointer/slice/map fields <li> Function handles <code>PortResults</code> slice, <code>PortMap</code> map, and <code>ICMPStatus</code> pointer with proper deep-copying <li> Maintains referential integrity between <code>PortResults</code> and <code>PortMap</code> in the copy</ul> </details> </td> <td><a href="https://github.com/carverauto/serviceradar/pull/2163/files#diff-3cc7dd0e748c9f77be9e384fed2703ab77375716524b70860153b6a1abae27ca">+84/-1</a>    </td> </tr> <tr> <td> <details> <summary>tools_sweeps.go<dd><code>Parameterize sweep tool queries</code>                                                    </dd></summary> <hr> pkg/mcp/tools_sweeps.go <ul><li>Updated <code>registerGetRecentSweepsTool</code> to use parameterized query for <code>poller_id</code> filter <li> Updated <code>registerGetSweepSummaryTool</code> to use parameterized query for <code>poller_id</code> filter <li> Both tools now call <code>executeSRQLQueryWithParams</code> instead of <code>executeSRQLQuery</code></ul> </details> </td> <td><a href="https://github.com/carverauto/serviceradar/pull/2163/files#diff-c2ddcfeb6736358e171e60164cac1cb48b8af464bebe8445fd17fe580caaaa2f">+8/-4</a>      </td> </tr> <tr> <td> <details> <summary>cnpg_identity_engine.go<dd><code>Add partition filtering to identifier lookup</code>                          </dd></summary> <hr> pkg/db/cnpg_identity_engine.go <ul><li>Updated <code>batchGetDeviceIDsByIdentifierSQL</code> to include partition filter in WHERE clause <li> Updated <code>BatchGetDeviceIDsByIdentifier</code> function signature to accept <code>partition</code> parameter <li> Added partition normalization with default value fallback <li> Updated query execution to pass partition as third parameter</ul> </details> </td> <td><a href="https://github.com/carverauto/serviceradar/pull/2163/files#diff-756702fbe82153fa86ae9b6bd6e5b109901d5fb8ee27cb9bc19f51277d3632f0">+8/-3</a>      </td> </tr> <tr> <td> <details> <summary>tools_events.go<dd><code>Parameterize alert tool queries</code>                                                    </dd></summary> <hr> pkg/mcp/tools_events.go <ul><li>Updated <code>registerGetAlertsTool</code> to use parameterized query for <code>poller_id</code> filter <li> Changed from string concatenation to <code>$1</code> placeholder with parameter binding <li> Updated to call <code>executeSRQLQueryWithParams</code> instead of <code>executeSRQLQuery</code></ul> </details> </td> <td><a href="https://github.com/carverauto/serviceradar/pull/2163/files#diff-3adaae4a85720d41db6c831160f8fd28273584047ac4a4540f2f9bcc5c6bcd90">+4/-2</a>      </td> </tr> <tr> <td> <details> <summary>pgx_batch_helper.go<dd><code>Add batch execution helper function</code>                                            </dd></summary> <hr> pkg/db/pgx_batch_helper.go <ul><li>Added <code>sendBatchExecAll</code> helper function for executing batch operations with error handling <li> Function handles empty batches, iterates through batch commands, and ensures proper cleanup <li> Returns formatted error messages with operation name and command index on failure</ul> </details> </td> <td><a href="https://github.com/carverauto/serviceradar/pull/2163/files#diff-9a38f78952feb40cd7f517c2d9a6c4a76a8ffcfa3989ced955298ad0ee6786b8">+29/-0</a>    </td> </tr> <tr> <td> <details> <summary>tools_devices.go<dd><code>Parameterize device tools queries</code>                                                </dd></summary> <hr> pkg/mcp/tools_devices.go <ul><li>Updated device ID filter to use parameterized query with <code>$1</code> placeholder <li> Changed from string concatenation to parameter binding <li> Updated to call <code>executeSRQLQueryWithParams</code> instead of <code>executeSRQLQuery</code></ul> </details> </td> <td><a href="https://github.com/carverauto/serviceradar/pull/2163/files#diff-9305d32d7f3683b79015dd7448623cf5393bf7f0631548313d810a3ed08d65dd">+3/-3</a>      </td> </tr> <tr> <td> <details> <summary>tools_logs.go<dd><code>Parameterize log tool queries</code>                                                        </dd></summary> <hr> pkg/mcp/tools_logs.go <ul><li>Updated <code>registerLogTools</code> to use parameterized query for <code>poller_id</code> filter <li> Changed from string concatenation to <code>$1</code> placeholder with parameter binding <li> Updated to call <code>executeSRQLQueryWithParams</code> instead of <code>executeSRQLQuery</code></ul> </details> </td> <td><a href="https://github.com/carverauto/serviceradar/pull/2163/files#diff-8f85b80d8ce508452e002b324e459f01c581bbd97ad961c7e30a054505d8f29a">+4/-2</a>      </td> </tr> <tr> <td> <details> <summary>cnpg_unified_devices.go<dd><code>Use batch helper for device deletion audit</code>                              </dd></summary> <hr> pkg/db/cnpg_unified_devices.go <ul><li>Replaced manual batch result handling with <code>sendBatchExecAll</code> helper function <li> Simplified error handling for audit log batch execution</ul> </details> </td> <td><a href="https://github.com/carverauto/serviceradar/pull/2163/files#diff-ab3bf557cf9bb1b281a315a73abee38748de1654941c2471542e5e9bfc1716d8">+1/-2</a>      </td> </tr> <tr> <td> <details> <summary>auth.go<dd><code>Use batch helper for user storage</code>                                                </dd></summary> <hr> pkg/db/auth.go <ul><li>Replaced manual batch result handling with <code>sendBatchExecAll</code> helper function <li> Simplified error handling for user batch storage</ul> </details> </td> <td><a href="https://github.com/carverauto/serviceradar/pull/2163/files#diff-4d4e15c7e925bdd07a11ca5d2779a6f2acef74c2bebe286c4bbc9a7593cefdb7">+1/-2</a>      </td> </tr> <tr> <td> <details> <summary>viz.rs<dd><code>Add visualization metadata generation for query results</code>    </dd></summary> <hr> rust/srql/src/query/viz.rs <ul><li>New file implementing visualization metadata generation for query results <li> Defines <code>VizMeta</code>, <code>ColumnMeta</code>, <code>ColumnType</code>, <code>ColumnSemantic</code> structs for describing result columns <li> Implements <code>meta_for_plan()</code> function that returns visualization suggestions and column metadata for different entity types <li> Supports timeseries and table visualization kinds with semantic hints for columns</ul> </details> </td> <td><a href="https://github.com/carverauto/serviceradar/pull/2163/files#diff-70c860470ae125afe6d7e4a2f9cf9eb36f8967ae773c3a47804329bbf56a2d8b">+605/-0</a>  </td> </tr> <tr> <td> <details> <summary>downsample.rs<dd><code>Add downsampling support for metric time-series queries</code>    </dd></summary> <hr> rust/srql/src/query/downsample.rs <ul><li>New file implementing downsampling functionality for metric queries <li> Provides <code>to_sql_and_params()</code> and <code>execute()</code> functions to build and run downsampled queries <li> Supports time bucketing with configurable aggregation functions (avg, min, max, sum, count) <li> Implements series grouping and filtering for various metric entity types</ul> </details> </td> <td><a href="https://github.com/carverauto/serviceradar/pull/2163/files#diff-94f68b4684578afa112ab05ff903667b6cd902ad276e17c12af350078b300a6a">+521/-0</a>  </td> </tr> <tr> <td> <details> <summary>devices.rs<dd><code>Refactor devices query to support stats and bind parameters</code></dd></summary> <hr> rust/srql/src/query/devices.rs <ul><li>Refactored <code>to_debug_sql()</code> to <code>to_sql_and_params()</code> returning SQL and bind parameters <li> Added stats query support with <code>parse_stats_spec()</code> and <code>build_stats_query()</code> for count aggregations <li> Implemented <code>collect_filter_params()</code> to extract bind parameters from device filters <li> Added validation for stats expressions and parameter binding</ul> </details> </td> <td><a href="https://github.com/carverauto/serviceradar/pull/2163/files#diff-3202f22fff6863ed7848a129c49e2323322462b379d896d3fca2e59aa6f7b4c5">+195/-7</a>  </td> </tr> <tr> <td> <details> <summary>services.rs<dd><code>Refactor services query to support stats and bind parameters</code></dd></summary> <hr> rust/srql/src/query/services.rs <ul><li>Refactored <code>to_debug_sql()</code> to <code>to_sql_and_params()</code> returning SQL and bind parameters <li> Added stats query support with <code>parse_stats_spec()</code> and <code>build_stats_query()</code> for count aggregations <li> Implemented <code>collect_filter_params()</code> to extract bind parameters from service filters <li> Added parameter binding validation and reconciliation</ul> </details> </td> <td><a href="https://github.com/carverauto/serviceradar/pull/2163/files#diff-ad5e7e1a352406ed473a765de4856625f1bede590e7e2c724163bcd1e7b313e9">+172/-4</a>  </td> </tr> <tr> <td> <details> <summary>parser.rs<dd><code>Add downsampling and graph cypher entity parsing support</code>  </dd></summary> <hr> rust/srql/src/parser.rs <ul><li>Added <code>GraphCypher</code> entity type for graph database queries <li> Introduced <code>DownsampleSpec</code> and <code>DownsampleAgg</code> for downsampling configuration <li> Implemented parsing for <code>bucket</code>, <code>agg</code>, and <code>series</code> downsampling parameters <li> Enhanced stats parsing to support <code>as alias</code> syntax for result aliasing</ul> </details> </td> <td><a href="https://github.com/carverauto/serviceradar/pull/2163/files#diff-b2edf55d1721185349ecddb2f4eacc42e0dfcae19b6c2bc638602f187da67e66">+157/-2</a>  </td> </tr> <tr> <td> <details> <summary>graph_cypher.rs<dd><code>Add Apache AGE Cypher graph query execution support</code>            </dd></summary> <hr> rust/srql/src/query/graph_cypher.rs <ul><li>New file implementing Apache AGE Cypher query execution for graph database <li> Provides <code>execute()</code> and <code>to_sql_and_params()</code> functions for running read-only Cypher queries <li> Implements security checks to prevent write operations and SQL injection <li> Handles result transformation for nodes, edges, and generic row data</ul> </details> </td> <td><a href="https://github.com/carverauto/serviceradar/pull/2163/files#diff-378b0ec80e1b1221c347160a0dac36c82d082fa06766d47d18cf069220166309">+208/-0</a>  </td> </tr> <tr> <td> <details> <summary>interfaces.rs<dd><code>Refactor interfaces query to support bind parameters</code>          </dd></summary> <hr> rust/srql/src/query/interfaces.rs <ul><li>Refactored <code>to_debug_sql()</code> to <code>to_sql_and_params()</code> returning SQL and bind parameters <li> Implemented <code>collect_filter_params()</code> to extract bind parameters from interface filters <li> Added parameter binding validation and reconciliation with limit/offset <li> Updated stats query handling to return formatted SQL with parameters</ul> </details> </td> <td><a href="https://github.com/carverauto/serviceradar/pull/2163/files#diff-1ec833d4525fb2806523888b8c57b76ca8dd2ab70539368ccecc4d262769c8c5">+112/-8</a>  </td> </tr> <tr> <td> <details> <summary>traces.rs<dd><code>Refactor traces query to support bind parameters</code>                  </dd></summary> <hr> rust/srql/src/query/traces.rs <ul><li>Refactored <code>to_debug_sql()</code> to <code>to_sql_and_params()</code> returning SQL and bind parameters <li> Implemented <code>collect_filter_params()</code> and <code>collect_text_params()</code> for trace filter parameters <li> Added support for integer list filtering on status_code and span_kind fields <li> Added parameter binding validation</ul> </details> </td> <td><a href="https://github.com/carverauto/serviceradar/pull/2163/files#diff-f390b0aef82baa9c3438719276dca70be826318d7446074a83b4527558528e19">+109/-4</a>  </td> </tr> <tr> <td> <details> <summary>logs.rs<dd><code>Refactor logs query to support bind parameters</code>                      </dd></summary> <hr> rust/srql/src/query/logs.rs <ul><li>Refactored <code>to_debug_sql()</code> to <code>to_sql_and_params()</code> returning SQL and bind parameters <li> Implemented <code>collect_filter_params()</code> for log filter parameter extraction <li> Added support for severity_number integer filtering with list operations <li> Enhanced stats query handling with parameter conversion</ul> </details> </td> <td><a href="https://github.com/carverauto/serviceradar/pull/2163/files#diff-f4d3b33667f2e79a6ebe4cfff931f93c728d9a81c305ac13586e623850a504db">+103/-3</a>  </td> </tr> <tr> <td> <details> <summary>cpu_metrics.rs<dd><code>Refactor CPU metrics query to support bind parameters</code>        </dd></summary> <hr> rust/srql/src/query/cpu_metrics.rs <ul><li>Refactored <code>to_debug_sql()</code> to <code>to_sql_and_params()</code> returning SQL and bind parameters <li> Implemented <code>collect_filter_params()</code> for CPU metric filter parameter extraction <li> Added support for numeric filtering on core_id, usage_percent, and frequency_hz <li> Enhanced stats query handling with parameter conversion</ul> </details> </td> <td><a href="https://github.com/carverauto/serviceradar/pull/2163/files#diff-400c860c805cd3e1a5e7f7c42cece63cce3e62c1f3fd8b49a5803e26b40fe31e">+85/-5</a>    </td> </tr> <tr> <td> <details> <summary>pollers.rs<dd><code>Refactor pollers query to support bind parameters</code>                </dd></summary> <hr> rust/srql/src/query/pollers.rs <ul><li>Refactored <code>to_debug_sql()</code> to <code>to_sql_and_params()</code> returning SQL and bind parameters <li> Implemented <code>collect_filter_params()</code> for poller filter parameter extraction <li> Added boolean filter support for <code>is_healthy</code> field <li> Added parameter binding validation and reconciliation</ul> </details> </td> <td><a href="https://github.com/carverauto/serviceradar/pull/2163/files#diff-e2192a3ba3041ed995e0abf9973489b7dff550949ff75da41d31bf514e765cc4">+81/-14</a>  </td> </tr> <tr> <td> <details> <summary>otel_metrics.rs<dd><code>Refactor OTEL metrics query to support bind parameters</code>      </dd></summary> <hr> rust/srql/src/query/otel_metrics.rs <ul><li>Refactored <code>to_debug_sql()</code> to <code>to_sql_and_params()</code> returning SQL and bind parameters <li> Implemented <code>collect_filter_params()</code> for OTEL metric filter parameter extraction <li> Added boolean filter support for <code>is_slow</code> field <li> Enhanced stats query handling with parameter conversion</ul> </details> </td> <td><a href="https://github.com/carverauto/serviceradar/pull/2163/files#diff-8106bfa2099b8a6d945b338532f8b1108467e2f0592ddbd64d546fcbce3e3613">+85/-5</a>    </td> </tr> <tr> <td> <details> <summary>db.rs<dd><code>Add mutual TLS (mTLS) client certificate authentication support</code></dd></summary> <hr> rust/srql/src/db.rs <ul><li>Added support for client certificate and key authentication in TLS connections <li> Implemented <code>load_client_certs()</code> and <code>load_client_key()</code> functions for certificate loading <li> Enhanced <code>build_client_config()</code> to support mutual TLS (mTLS) authentication <li> Updated <code>PgConnectionManager::new()</code> to accept optional client cert and key paths</ul> </details> </td> <td><a href="https://github.com/carverauto/serviceradar/pull/2163/files#diff-8d5a0c48024aa12a3e06de065ca71e940d2b5007fbc34ef87471b90d7937891c">+77/-10</a>  </td> </tr> <tr> <td> <details> <summary>timeseries_metrics.rs<dd><code>Refactor timeseries metrics query to support bind parameters</code></dd></summary> <hr> rust/srql/src/query/timeseries_metrics.rs <ul><li>Refactored <code>to_debug_sql()</code> to <code>to_sql_and_params()</code> returning SQL and bind parameters <li> Implemented <code>collect_filter_params()</code> for timeseries metric filter parameter extraction <li> Added support for numeric filtering on if_index and value fields <li> Added parameter binding validation and reconciliation</ul> </details> </td> <td><a href="https://github.com/carverauto/serviceradar/pull/2163/files#diff-8450bff3aef4c1271bbf9b96809b4a5ac7b218e3f98c8b60387e35dbf4845bb0">+82/-4</a>    </td> </tr> <tr> <td> <details> <summary>process_metrics.rs<dd><code>Refactor process metrics query to support bind parameters</code></dd></summary> <hr> rust/srql/src/query/process_metrics.rs <ul><li>Refactored <code>to_debug_sql()</code> to <code>to_sql_and_params()</code> returning SQL and bind parameters <li> Implemented <code>collect_filter_params()</code> for process metric filter parameter extraction <li> Added support for numeric filtering on pid, cpu_usage, and memory_usage fields <li> Added parameter binding validation and reconciliation</ul> </details> </td> <td><a href="https://github.com/carverauto/serviceradar/pull/2163/files#diff-c70bb600f5fba7199d80930fdc735cfc38371ef2d4ff3c821af55100e4cb0bac">+77/-4</a>    </td> </tr> <tr> <td> <details> <summary>trace_summaries.rs<dd><code>Refactor trace summaries query to support bind parameters</code></dd></summary> <hr> rust/srql/src/query/trace_summaries.rs <ul><li>Refactored <code>to_debug_sql()</code> to <code>to_sql_and_params()</code> returning SQL and bind parameters <li> Implemented <code>bind_param_from_query()</code> to convert SQL bind values to <code>BindParam</code> type <li> Updated placeholder rewriting for parameter binding</ul> </details> </td> <td><a href="https://github.com/carverauto/serviceradar/pull/2163/files#diff-ff98619eb5dc4d75205d1797477e9e2880f42081e8d0c61d6fd1d6e5926b72fa">+20/-3</a>    </td> </tr> <tr> <td> <details> <summary>models.rs<dd><code>Add unit field to OTEL metric row model</code>                                    </dd></summary> <hr> rust/srql/src/models.rs <ul><li>Added <code>unit</code> field to <code>OtelMetricRow</code> struct for storing metric unit information <li> Updated <code>into_json()</code> method to include the new <code>unit</code> field in JSON serialization</ul> </details> </td> <td><a href="https://github.com/carverauto/serviceradar/pull/2163/files#diff-57c82b01f92e9bd40063d0b0178c12d452771ac133f2121fb0ac008b167da367">+2/-0</a>      </td> </tr> <tr> <td> <details> <summary>disk_metrics.rs<dd><code>Refactor SQL generation to extract and return bind parameters</code></dd></summary> <hr> rust/srql/src/query/disk_metrics.rs <ul><li>Refactored <code>to_debug_sql</code> function to <code>to_sql_and_params</code> returning both SQL string and bind parameters <li> Added <code>collect_text_params</code> and <code>collect_filter_params</code> helper functions to extract filter parameters <li> Implemented bind parameter collection for time ranges, filters, and limit/offset clauses <li> Added validation to ensure bind count matches between diesel query and collected parameters</ul> </details> </td> <td><a href="https://github.com/carverauto/serviceradar/pull/2163/files#diff-b5a536baa37176f971e96c789991202b483314e2a85d4556489b4568a5675162">+72/-4</a>    </td> </tr> <tr> <td> <details> <summary>memory_metrics.rs<dd><code>Refactor SQL generation to extract and return bind parameters</code></dd></summary> <hr> rust/srql/src/query/memory_metrics.rs <ul><li>Refactored <code>to_debug_sql</code> function to <code>to_sql_and_params</code> returning both SQL string and bind parameters <li> Added <code>collect_text_params</code> and <code>collect_filter_params</code> helper functions to extract filter parameters <li> Implemented bind parameter collection for time ranges, filters, and limit/offset clauses <li> Added validation to ensure bind count matches between diesel query and collected parameters</ul> </details> </td> <td><a href="https://github.com/carverauto/serviceradar/pull/2163/files#diff-a12a227c4b5c224359799853f5fd7bbf626ec14b1e7eae40e4c65b1db6699417">+73/-4</a>    </td> </tr> <tr> <td> <details> <summary>device_updates.rs<dd><code>Refactor SQL generation to extract and return bind parameters</code></dd></summary> <hr> rust/srql/src/query/device_updates.rs <ul><li>Refactored <code>to_debug_sql</code> function to <code>to_sql_and_params</code> returning both SQL string and bind parameters <li> Added <code>collect_text_params</code> with <code>allow_lists</code> parameter to control list filter support per field <li> Implemented <code>collect_filter_params</code> to handle device-specific filter fields including boolean values <li> Added validation to ensure bind count matches between diesel query and collected parameters</ul> </details> </td> <td><a href="https://github.com/carverauto/serviceradar/pull/2163/files#diff-77b99f0ef409a6a3e4ecc65800ae841231a82b27a64e8dc1d4667db6698a539d">+78/-4</a>    </td> </tr> <tr> <td> <details> <summary>events.rs<dd><code>Refactor SQL generation to extract and return bind parameters</code></dd></summary> <hr> rust/srql/src/query/events.rs <ul><li>Refactored <code>to_debug_sql</code> function to <code>to_sql_and_params</code> returning both SQL string and bind parameters <li> Added <code>collect_text_params</code> and <code>collect_filter_params</code> helper functions to extract filter parameters <li> Implemented bind parameter collection for time ranges, filters, and limit/offset clauses <li> Added validation to ensure bind count matches between diesel query and collected parameters</ul> </details> </td> <td><a href="https://github.com/carverauto/serviceradar/pull/2163/files#diff-ad603163a35223c63c68279e53a1a4c9219b9096042a168d8a87443abaa29a58">+71/-4</a>    </td> </tr> <tr> <td> <details> <summary>lib.rs<dd><code>Add Rust NIF for SRQL query translation to Elixir</code>                </dd></summary> <hr> web-ng/native/srql_nif/src/lib.rs <ul><li>Created new Rust NIF module for Elixir integration with SRQL query translation <li> Implemented <code>translate</code> function as a dirty CPU NIF that converts SRQL queries to database queries <li> Added error handling with atom-based responses for success and failure cases <li> Integrated with <code>srql::query::translate_request</code> for query processing</ul> </details> </td> <td><a href="https://github.com/carverauto/serviceradar/pull/2163/files#diff-74a225cdf31e64db49fb42c0419c524068150b07528e8d722a75e3ec8034c297">+49/-0</a>    </td> </tr> <tr> <td> <details> <summary>lib.rs<dd><code>Export query types and add embedded SRQL support</code>                  </dd></summary> <hr> rust/srql/src/lib.rs <ul><li>Exported public types <code>QueryDirection</code>, <code>QueryEngine</code>, <code>QueryRequest</code>, <code>QueryResponse</code>, <code>TranslateRequest</code>, and <code>TranslateResponse</code> <li> Added <code>EmbeddedSrql</code> struct to support embedded SRQL usage with connection pooling <li> Implemented <code>EmbeddedSrql::new</code> constructor for initializing with <code>AppConfig</code></ul> </details> </td> <td><a href="https://github.com/carverauto/serviceradar/pull/2163/files#diff-8c0401c0d0bb9761ed66ff5328703755132a87d625f77878f53037e2329644b8">+19/-0</a>    </td> </tr> <tr> <td> <details> <summary>schema.rs<dd><code>Add unit field to events table schema</code>                                        </dd></summary> <hr> rust/srql/src/schema.rs <ul><li>Added <code>unit</code> field to the events table schema as nullable text column</ul> </details> </td> <td><a href="https://github.com/carverauto/serviceradar/pull/2163/files#diff-2fe4906f42b52f96b7bc2d3431885b996b6701cfb88416905ae130b472d536ea">+1/-0</a>      </td> </tr> <tr> <td> <details> <summary>generate-certs.sh<dd><code>Add support for extra CNPG certificate IPs and workstation cert</code></dd></summary> <hr> docker/compose/generate-certs.sh <ul><li>Added logic to check if CNPG certificate is missing required SAN IPs and regenerate if needed <li> Implemented support for <code>CNPG_CERT_EXTRA_IPS</code> environment variable to add additional IP addresses to CNPG certificate <li> Added generation of new <code>workstation</code> certificate for developers connecting from outside Docker network <li> Refactored certificate existence check to handle conditional regeneration</ul> </details> </td> <td><a href="https://github.com/carverauto/serviceradar/pull/2163/files#diff-8298241543b4744a6ac7780c760ac5b5a0a87ba62de19c8612ebe1aba0996ebd">+28/-6</a>    </td> </tr> <tr> <td> <details> <summary>app.js<dd><code>Add Phoenix LiveSocket setup and custom hooks</code>                        </dd></summary> <hr> web-ng/assets/js/app.js <ul><li>Initialized Phoenix LiveSocket with custom hooks and colocated hooks <li> Implemented <code>TimeseriesChart</code> hook for interactive chart tooltips and hover line visualization <li> Added topbar progress bar configuration for page loading feedback <li> Configured development mode features for server log streaming and editor integration</ul> </details> </td> <td><a href="https://github.com/carverauto/serviceradar/pull/2163/files#diff-16bc6f19f34cfdd66ac36b4ef2dd66d34e3470c4eb324ebc3e4bb8e58a91360c">+142/-0</a>  </td> </tr> <tr> <td> <details> <summary>router.ex<dd><code>Add Phoenix router with authentication and LiveView routes</code></dd></summary> <hr> web-ng/lib/serviceradar_web_ng_web/router.ex <ul><li>Defined browser and API pipelines with authentication and CSRF protection <li> Added routes for home page, API query execution, and device management endpoints <li> Configured LiveView sessions for authenticated users with multiple dashboard pages <li> Implemented authentication routes for user registration, login, and logout</ul> </details> </td> <td><a href="https://github.com/carverauto/serviceradar/pull/2163/files#diff-df516cd33165cd85914c1ccb3ff6511d3fe688d4a66498b99807958998c5d07c">+103/-0</a>  </td> </tr> </table></details></td></tr><tr><td>Tests</td><td><details><summary>9 files</summary><table> <tr> <td> <details> <summary>summary_snapshot_test.go<dd><code>Add snapshot deep-copy verification tests</code>                                </dd></summary> <hr> pkg/sweeper/summary_snapshot_test.go <ul><li>Added test <code>TestGetSummary_HostResultsDoNotAliasInternalState</code> verifying deep-copy semantics for host snapshots <li> Added test <code>TestGetSummary_ConcurrentReadsDoNotPanic</code> ensuring concurrent access to snapshots doesn't panic <li> Tests validate that <code>ICMPStatus</code> and <code>PortMap</code>/<code>PortResults</code> are properly deep-copied and not aliased</ul> </details> </td> <td><a href="https://github.com/carverauto/serviceradar/pull/2163/files#diff-eb3740146940283b92c377e3c550ebe123ef3e010feaa2974a3029ef9260b08d">+157/-0</a>  </td> </tr> <tr> <td> <details> <summary>parameterized_queries_test.go<dd><code>Add parameterized query binding tests</code>                                        </dd></summary> <hr> pkg/mcp/parameterized_queries_test.go <ul><li>Added test <code>TestDevicesGetDeviceBindsDeviceID</code> verifying device ID parameter binding <li> Added test <code>TestLogsGetRecentLogsBindsPollerID</code> verifying poller ID parameter binding <li> Added test <code>TestEventsGetEventsBindsMappedFilters</code> verifying multiple filter parameter binding <li> Added test <code>TestStructuredToolsRequireParameterizedExecutor</code> ensuring non-parameterized executors are rejected <li> Tests use <code>recordingParameterizedExecutor</code> mock to verify query and parameter handling</ul> </details> </td> <td><a href="https://github.com/carverauto/serviceradar/pull/2163/files#diff-330a1eb194c06c458ba999e3ce7346c4dc33dea154e15a8132dc6fc13bf3c296">+123/-0</a>  </td> </tr> <tr> <td> <details> <summary>sweep_deepcopy_test.go<dd><code>Add deep-copy validation tests for HostResult</code>                        </dd></summary> <hr> pkg/models/sweep_deepcopy_test.go <ul><li>Added comprehensive test <code>TestDeepCopyHostResult_NoAliasing</code> validating deep-copy semantics <li> Tests verify scalar fields match, pointer fields are copied, and mutations don't affect source <li> Validates <code>PortResults</code> slice, <code>PortMap</code> map, and <code>ICMPStatus</code> are properly deep-copied</ul> </details> </td> <td><a href="https://github.com/carverauto/serviceradar/pull/2163/files#diff-45bfc89e5b79e84e249bab30c33c776bd8465af54f17693e0bbc97c4eed0f003">+78/-0</a>    </td> </tr> <tr> <td> <details> <summary>service_deadlock_test.go<dd><code>Add deadlock prevention static analysis test</code>                          </dd></summary> <hr> pkg/checker/snmp/service_deadlock_test.go <ul><li>Added static analysis test <code>TestCheckDoesNotRLockAndCallGetStatus</code> using AST parsing <li> Test embeds <code>service.go</code> source and verifies <code>Check</code> method doesn't hold <code>s.mu.RLock</code> while calling <code>GetStatus</code> <li> Prevents potential deadlock from recursive RWMutex read locking</ul> </details> </td> <td><a href="https://github.com/carverauto/serviceradar/pull/2163/files#diff-6a31b13924de8ba2b712fc5deb881056c50f7448bb61eb84efe82850eb08325e">+101/-0</a>  </td> </tr> <tr> <td> <details> <summary>pgx_batch_helper_test.go<dd><code>Add batch execution helper tests</code>                                                  </dd></summary> <hr> pkg/db/pgx_batch_helper_test.go <ul><li>Added test <code>TestSendBatchExecAll_EmptyBatchDoesNotSend</code> verifying empty batches skip execution <li> Added test <code>TestSendBatchExecAll_ExecErrorIncludesCommandIndexAndCloses</code> verifying error reporting with command index <li> Added test <code>TestSendBatchExecAll_CloseErrorReturnedWhenExecSucceeds</code> verifying close errors are surfaced <li> Tests use <code>fakeBatchResults</code> mock to simulate batch execution scenarios</ul> </details> </td> <td><a href="https://github.com/carverauto/serviceradar/pull/2163/files#diff-67dd6237893aea0b294efd6b76f9aa138f4e8cd98424a3683f65ab9a7ce0372e">+99/-0</a>    </td> </tr> <tr> <td> <details> <summary>client_conversion_test.go<dd><code>Add SNMP type conversion tests</code>                                                      </dd></summary> <hr> pkg/checker/snmp/client_conversion_test.go <ul><li>Added test <code>TestConvertVariable_OctetStringBytes</code> verifying <code>[]byte</code> conversion to string <li> Added test <code>TestConvertVariable_ObjectDescriptionBytes</code> verifying <code>[]byte</code> conversion for object descriptions <li> Added test <code>TestConvertVariable_StringTypesUnexpectedValueDoNotPanic</code> verifying error handling for unexpected types <li> Tests ensure conversions don't panic and return appropriate errors</ul> </details> </td> <td><a href="https://github.com/carverauto/serviceradar/pull/2163/files#diff-e382a2bd9f2612005f0a6239ae6a30272af91d68a70e25b7ac3c0ed8da3211cc">+95/-0</a>    </td> </tr> <tr> <td> <details> <summary>pgx_batch_behavior_test.go<dd><code>Add batch operation error handling tests</code>                                  </dd></summary> <hr> pkg/db/pgx_batch_behavior_test.go <ul><li>Added test <code>TestInsertEvents_SurfacesBatchInsertErrors</code> verifying batch insert error propagation <li> Added test <code>TestStoreBatchUsers_SurfacesBatchInsertErrors</code> verifying user batch error handling <li> Tests use <code>fakePgxExecutor</code> and <code>fakeBatchResults</code> mocks to simulate batch failures</ul> </details> </td> <td><a href="https://github.com/carverauto/serviceradar/pull/2163/files#diff-0c98da9a5d20f19ce43861711a001e0aebe953c4f004cb2bb38a245020f4e206">+79/-0</a>    </td> </tr> <tr> <td> <details> <summary>identity_engine_partition_test.go<dd><code>Add partition-scoped identifier resolution test</code>                    </dd></summary> <hr> pkg/registry/identity_engine_partition_test.go <ul><li>Added test <code>TestBatchLookupByStrongIdentifiers_PartitionScoped</code> verifying partition-scoped identifier resolution <li> Test validates that different partitions resolve to different device IDs for same MAC address <li> Uses mock to verify <code>BatchGetDeviceIDsByIdentifier</code> is called with correct partition parameter</ul> </details> </td> <td><a href="https://github.com/carverauto/serviceradar/pull/2163/files#diff-77aee244beb622dec723a0a62d2d11785b5b76a9fe5df912ba559227d3f4ba70">+51/-0</a>    </td> </tr> <tr> <td> <details> <summary>cnpg_registry_test.go<dd><code>Add poller status upsert validation test</code>                                  </dd></summary> <hr> pkg/db/cnpg_registry_test.go <ul><li>Added test <code>TestUpsertPollerStatusSQL_PreservesRegistrationMetadata</code> verifying upsert query structure <li> Test validates that registration metadata fields are not overwritten on conflict <li> Ensures only <code>last_seen</code>, <code>is_healthy</code>, and <code>updated_at</code> are updated</ul> </details> </td> <td><a href="https://github.com/carverauto/serviceradar/pull/2163/files#diff-e38716ee5b3614089e00e851d22d2c61e9934ebcc2b44b7aa4b79623251af869">+22/-0</a>    </td> </tr> </table></details></td></tr><tr><td>Bug fix</td><td><details><summary>4 files</summary><table> <tr> <td> <details> <summary>client.go<dd><code>Fix SNMP string type conversion error handling</code>                      </dd></summary> <hr> pkg/checker/snmp/client.go <ul><li>Removed <code>ObjectDescription</code> and <code>OctetString</code> from conversion map to handle them separately <li> Updated <code>convertObjectDescription</code> and <code>convertOctetString</code> to return <code>(interface{}, error)</code> and validate <code>[]byte</code> type <li> Added explicit type checks with error handling for string conversion types <li> Removed unused conversion functions from map and added early-return checks before map lookup</ul> </details> </td> <td><a href="https://github.com/carverauto/serviceradar/pull/2163/files#diff-b5e92490b688495a040a2f6f5227dc83c46fc5e7ea59885f8285a3d6c868bd87">+37/-21</a>  </td> </tr> <tr> <td> <details> <summary>memory_store.go<dd><code>Use deep-copy for host result snapshots</code>                                    </dd></summary> <hr> pkg/sweeper/memory_store.go <ul><li>Updated <code>convertToSlice</code> to use <code>DeepCopyHostResult</code> instead of shallow copy <li> Updated <code>buildSummary</code> to use <code>DeepCopyHostResult</code> for host snapshots <li> Fixed struct field alignment in <code>InMemoryStore</code> and initialization for consistency</ul> </details> </td> <td><a href="https://github.com/carverauto/serviceradar/pull/2163/files#diff-54c761e2155455ac0e86e2cf1405610fe315d966ab17d4f008c3dd7227a05389">+15/-15</a>  </td> </tr> <tr> <td> <details> <summary>base_processor.go<dd><code>Use deep-copy for processor host snapshots</code>                              </dd></summary> <hr> pkg/sweeper/base_processor.go <ul><li>Updated <code>collectShardSummaries</code> to use <code>DeepCopyHostResult</code> for host snapshots <li> Updated <code>processShardForSummary</code> to use <code>DeepCopyHostResult</code> when sending hosts to channel <li> Fixed trailing whitespace in <code>processTCPResult</code> call</ul> </details> </td> <td><a href="https://github.com/carverauto/serviceradar/pull/2163/files#diff-40a89f977fa936cab19d62a659fb835665f646012674fbf3a8b09a8987917fbb">+3/-3</a>      </td> </tr> <tr> <td> <details> <summary>service.go<dd><code>Fix potential RWMutex deadlock in Check method</code>                      </dd></summary> <hr> pkg/checker/snmp/service.go <ul><li>Removed <code>s.mu.RLock()</code> and <code>defer s.mu.RUnlock()</code> from <code>Check</code> method <li> Added comment explaining deadlock prevention: <code>GetStatus</code> performs its own locking <li> Prevents recursive RWMutex read locking that can deadlock with write-preferring semantics</ul> </details> </td> <td><a href="https://github.com/carverauto/serviceradar/pull/2163/files#diff-c592649b582c62a85d1dd416ac7fb609e55531cb43c8199a15f4432ce8ae05d8">+3/-4</a>      </td> </tr> </table></details></td></tr><tr><td>Miscellaneous</td><td><details><summary>4 files</summary><table> <tr> <td> <details> <summary>mock_db.go<dd><code>Update mock for partition-scoped identifier lookup</code>              </dd></summary> <hr> pkg/db/mock_db.go <ul><li>Updated <code>BatchGetDeviceIDsByIdentifier</code> mock signature to include <code>partition</code> parameter <li> Updated mock recorder method to accept partition argument <li> Added newline formatting for consistency</ul> </details> </td> <td><a href="https://github.com/carverauto/serviceradar/pull/2163/files#diff-30e38f888d4849fc40d7ebb1559c2a84c43aa8cd13b3b89fd7ec6cf873b243c7">+6/-5</a>      </td> </tr> <tr> <td> <details> <summary>registry_test.go<dd><code>Update registry tests for partition parameter</code>                        </dd></summary> <hr> pkg/registry/registry_test.go <ul><li>Updated all <code>BatchGetDeviceIDsByIdentifier</code> mock expectations to include partition parameter <li> Changes applied to <code>allowCanonicalizationQueries</code>, <code>TestProcessBatchDeviceUpdates_MergesSweepIntoCanonicalDevice</code>, and other test functions</ul> </details> </td> <td><a href="https://github.com/carverauto/serviceradar/pull/2163/files#diff-f010972d104404be52d2a8e6e784cb56e31194f90795a69571a12696bcbdc075">+5/-5</a>      </td> </tr> <tr> <td> <details> <summary>canon_simulation_test.go<dd><code>Update DIRE simulation test for partition keys</code>                      </dd></summary> <hr> pkg/registry/canon_simulation_test.go <ul><li>Updated <code>setupDIREMockDB</code> to use <code>strongIdentifierCacheKey</code> for cache key generation <li> Updated mock <code>BatchGetDeviceIDsByIdentifier</code> to accept partition parameter <li> Updated <code>UpsertDeviceIdentifiers</code> mock to use <code>strongIdentifierCacheKey</code></ul> </details> </td> <td><a href="https://github.com/carverauto/serviceradar/pull/2163/files#diff-73b73409b3941af4ac860561c87772763feca734fd8ab597f36e5e4047c6bf3c">+4/-4</a>      </td> </tr> <tr> <td> <details> <summary>interfaces.go<dd><code>Update Service interface for partition parameter</code>                  </dd></summary> <hr> pkg/db/interfaces.go <ul><li>Updated <code>BatchGetDeviceIDsByIdentifier</code> interface signature to include <code>partition</code> parameter</ul> </details> </td> <td><a href="https://github.com/carverauto/serviceradar/pull/2163/files#diff-c230fe0c315251837357bfde4ae7f7b34080398d8e48af6bf78badb2124271f3">+1/-1</a>      </td> </tr> </table></details></td></tr><tr><td>Formatting</td><td><details><summary>1 files</summary><table> <tr> <td> <details> <summary>server_test.go<dd><code>Fix test file indentation</code>                                                                </dd></summary> <hr> pkg/mcp/server_test.go - Fixed indentation from spaces to tabs for Go style consistency </details> </td> <td><a href="https://github.com/carverauto/serviceradar/pull/2163/files#diff-0b91a426e8631fe9cc9109206e3e6d922ff1de7dfdee7b4c6518fbdd94e86c7a">+5/-5</a>      </td> </tr> </table></details></td></tr><tr><td>Configuration changes</td><td><details><summary>2 files</summary><table> <tr> <td> <details> <summary>entrypoint-nginx.sh<dd><code>Update nginx entrypoint to use web-ng service</code>                        </dd></summary> <hr> docker/compose/entrypoint-nginx.sh <ul><li>Changed nginx upstream service from <code>web</code> on port 3000 to <code>web-ng</code> on port 4000 <li> Removed wait logic for <code>core</code> service on port 8090 <li> Simplified entrypoint to only wait for web-ng service</ul> </details> </td> <td><a href="https://github.com/carverauto/serviceradar/pull/2163/files#diff-0372d61e48b7bdb23bed91a7f72fc774fe36afca5ecafc97b170ed35be4e2e59">+4/-12</a>    </td> </tr> <tr> <td> <details> <summary>app.css<dd><code>Configure Tailwind CSS with daisyUI and custom themes</code>        </dd></summary> <hr> web-ng/assets/css/app.css <ul><li>Configured Tailwind CSS with source scanning for CSS, JS, and template files <li> Integrated daisyUI plugin with custom dark and light theme configurations <li> Added custom Tailwind variants for LiveView loading states and dark mode <li> Configured transparent display for LiveView wrapper divs</ul> </details> </td> <td><a href="https://github.com/carverauto/serviceradar/pull/2163/files#diff-e2f1698b818ca68e34f2f330886b5dc3621fb6033b4414869c7e3d677aa4c4d3">+105/-0</a>  </td> </tr> </table></details></td></tr><tr><td>Dependencies</td><td><details><summary>3 files</summary><table> <tr> <td> <details> <summary>daisyui-theme.js<dd><code>Add daisyUI theme plugin bundle for UI styling</code>                      </dd></summary> <hr> web-ng/assets/vendor/daisyui-theme.js <ul><li>Added daisyUI theme plugin bundle with support for multiple color themes <li> Implemented theme configuration system with customizable color schemes and design tokens <li> Included MIT licensed daisyUI theme definitions for light and dark modes</ul> </details> </td> <td><a href="https://github.com/carverauto/serviceradar/pull/2163/files#diff-1f651678246fddd353d5b865da740f0fd7f0a16d5e252a378d791ae43136c803">+124/-0</a>  </td> </tr> <tr> <td> <details> <summary>topbar.js<dd><code>Add topbar progress bar library</code>                                                    </dd></summary> <hr> web-ng/assets/vendor/topbar.js <ul><li>Added topbar progress bar library for visual feedback during page navigation <li> Implemented canvas-based progress bar with gradient colors and shadow effects <li> Provided configuration options for bar thickness, colors, and animation behavior</ul> </details> </td> <td><a href="https://github.com/carverauto/serviceradar/pull/2163/files#diff-b33ec2270b5ebb791a0c96c3ff024f82d66d346be6d542374e5885273f7bc004">+138/-0</a>  </td> </tr> <tr> <td> <details> <summary>heroicons.js<dd><code>Add Heroicons Tailwind CSS plugin</code>                                                </dd></summary> <hr> web-ng/assets/vendor/heroicons.js <ul><li>Created Tailwind CSS plugin for Heroicons SVG icon integration <li> Implemented dynamic icon loading from multiple icon sets (outline, solid, mini, micro) <li> Added CSS mask-based icon rendering with customizable sizing</ul> </details> </td> <td><a href="https://github.com/carverauto/serviceradar/pull/2163/files#diff-435329705234b4577a06b84302017b73aaf264e7af376e6bfd5abbf71611d9bd">+43/-0</a>    </td> </tr> </table></details></td></tr><tr><td>Additional files</td><td><details><summary>101 files</summary><table> <tr> <td>.env.example</td> <td><a href="https://github.com/carverauto/serviceradar/pull/2163/files#diff-a3046da0d15a27e89f2afe639b25748a7ad4d9290af3e7b1b6c1a5533c8f0a8c">+18/-1</a>    </td> </tr> <tr> <td>.tool-versions</td> <td><a href="https://github.com/carverauto/serviceradar/pull/2163/files#diff-751af1a340658c7b8176fe32d7db9fadbe15d1d075daba1919a91df04155bc70">+2/-0</a>      </td> </tr> <tr> <td>AGENTS.md</td> <td><a href="https://github.com/carverauto/serviceradar/pull/2163/files#diff-a54ff182c7e8acf56acfd6e4b9c3ff41e2c41a31c9b211b2deb9df75d9a478f9">+46/-0</a>    </td> </tr> <tr> <td>docker-compose.yml</td> <td><a href="https://github.com/carverauto/serviceradar/pull/2163/files#diff-e45e45baeda1c1e73482975a664062aa56f20c03dd9d64a827aba57775bed0d3">+27/-119</a></td> </tr> <tr> <td>Dockerfile.web-ng</td> <td><a href="https://github.com/carverauto/serviceradar/pull/2163/files#diff-92d43af1965575d56c3380ecc8a81024aac2ff36f039ec2d3839e9fc7852bc10">+71/-0</a>    </td> </tr> <tr> <td>nginx.conf.template</td> <td><a href="https://github.com/carverauto/serviceradar/pull/2163/files#diff-62ef305390094ce81632d493253e470ca46e7daa76da1079c131e41502975d07">+6/-138</a>  </td> </tr> <tr> <td>AGENTS.md</td> <td><a href="https://github.com/carverauto/serviceradar/pull/2163/files#diff-593e5b1ccdb5f528a4b0f5d7dd56b10c4422aeb11cf7292bf4c21a93a7756340">+72/-0</a>    </td> </tr> <tr> <td>design.md</td> <td><a href="https://github.com/carverauto/serviceradar/pull/2163/files#diff-5e5a5da59cb9f0b4f7a8fc3c492f1be70a9292fc0533fe4fc103a85f27f3c105">+0/-29</a>    </td> </tr> <tr> <td>proposal.md</td> <td><a href="https://github.com/carverauto/serviceradar/pull/2163/files#diff-3fa8ff1eb6c2b79b5937f3b7e97d0eaa429475802e0e136d70e61f3ce8b996e1">+0/-25</a>    </td> </tr> <tr> <td>spec.md</td> <td><a href="https://github.com/carverauto/serviceradar/pull/2163/files#diff-68425fc01e3047a9fd103cfcb917fa84bdc3f2781539b29a5f0f983753d57627">+0/-148</a>  </td> </tr> <tr> <td>tasks.md</td> <td><a href="https://github.com/carverauto/serviceradar/pull/2163/files#diff-8bd336677c2048834707d9f86bcb964404304815695a2ebd7273203c57cb7c92">+0/-68</a>    </td> </tr> <tr> <td>proposal.md</td> <td><a href="https://github.com/carverauto/serviceradar/pull/2163/files#diff-6f0c5aff0b965fcdd9db0c5694e0687fa72e170a89c33bda18ec8f82f426ad33">+0/-26</a>    </td> </tr> <tr> <td>spec.md</td> <td><a href="https://github.com/carverauto/serviceradar/pull/2163/files#diff-2860cadf010e37686782686da542b1669a8e860231acf8b00bbadbbde3f54719">+0/-34</a>    </td> </tr> <tr> <td>tasks.md</td> <td><a href="https://github.com/carverauto/serviceradar/pull/2163/files#diff-bc5b4da0f06380ea7f2f89c15b94cdbfc7c6637d3db0b6703950e2609f7933ff">+0/-13</a>    </td> </tr> <tr> <td>proposal.md</td> <td><a href="https://github.com/carverauto/serviceradar/pull/2163/files#diff-bf83f1fb2e0bb30309a84bc3343bc28a06fef20726e1c8583b406a93697dfc87">+0/-68</a>    </td> </tr> <tr> <td>proposal.md</td> <td><a href="https://github.com/carverauto/serviceradar/pull/2163/files#diff-ad29ce951260d3acc52d2ff591319fda8dbf485bb420f3968f8a9dfd1e91c856">+0/-42</a>    </td> </tr> <tr> <td>tasks.md</td> <td><a href="https://github.com/carverauto/serviceradar/pull/2163/files#diff-557ef0882398cb52535288f1dc8edd81197b0a34dc62ca68b69ecf1754d3c479">+0/-20</a>    </td> </tr> <tr> <td>design.md</td> <td><a href="https://github.com/carverauto/serviceradar/pull/2163/files#diff-888d2efd1371a95f95ba77bf4506d180c1fd65434603a56cf2e8a3ad495386ce">+0/-26</a>    </td> </tr> <tr> <td>proposal.md</td> <td><a href="https://github.com/carverauto/serviceradar/pull/2163/files#diff-f3c3c8d9463a1c6eb3934bb6c516985accb7f58b402a0d5e414ff65c1c732bc3">+0/-16</a>    </td> </tr> <tr> <td>spec.md</td> <td><a href="https://github.com/carverauto/serviceradar/pull/2163/files#diff-6e956f86e942a3106c94ea3f1a1bd46210c035e9240be297a488b639449d3e90">+0/-85</a>    </td> </tr> <tr> <td>tasks.md</td> <td><a href="https://github.com/carverauto/serviceradar/pull/2163/files#diff-7d7c4d6b4718fd5255ec47aa62dee71b5da2c83ff2160164603454379b618181">+0/-25</a>    </td> </tr> <tr> <td>design.md</td> <td><a href="https://github.com/carverauto/serviceradar/pull/2163/files#diff-799ac314609701ea23f8321cd5e4464c24b9c4634ee8b863d104dc2955aa6eda">+114/-0</a>  </td> </tr> <tr> <td>proposal.md</td> <td><a href="https://github.com/carverauto/serviceradar/pull/2163/files#diff-bfc316cf42c3c49e04d2363e9ee566c1d78cd3d8830932590ddc8abe2e777a24">+48/-0</a>    </td> </tr> <tr> <td>spec.md</td> <td><a href="https://github.com/carverauto/serviceradar/pull/2163/files#diff-b3d0f1681371e322ccc253767df5d32e59d23aaddfb979da0746932ef1f97139">+24/-0</a>    </td> </tr> <tr> <td>spec.md</td> <td><a href="https://github.com/carverauto/serviceradar/pull/2163/files#diff-5482ba561260689cb4331860e556a69f4e0c312d8b6a0636c4e354e80bfff88a">+22/-0</a>    </td> </tr> <tr> <td>tasks.md</td> <td><a href="https://github.com/carverauto/serviceradar/pull/2163/files#diff-08fc870484787c82a4ca64544593d4fbeb34b7d2d09c0bd6f1ae1f1354ae1368">+83/-0</a>    </td> </tr> <tr> <td>design.md</td> <td><a href="https://github.com/carverauto/serviceradar/pull/2163/files#diff-4aa7fc4af23187c7e80727f72cd233de9676cb832a0850271ea8c180dd50c140">+0/-29</a>    </td> </tr> <tr> <td>proposal.md</td> <td><a href="https://github.com/carverauto/serviceradar/pull/2163/files#diff-cea28b6af37c67606e313f2233bd1ae7187f9a4e1289bef0823678c8346e6b7c">+0/-21</a>    </td> </tr> <tr> <td>spec.md</td> <td><a href="https://github.com/carverauto/serviceradar/pull/2163/files#diff-dbf6e0bdd671feb4089a7d8897979220c1a3f7442d99cd6badf2c02cd80b4d03">+0/-23</a>    </td> </tr> <tr> <td>tasks.md</td> <td><a href="https://github.com/carverauto/serviceradar/pull/2163/files#diff-16748c17e91c4e6e2e591bec79735cf7c956cfd005e0aa7787d0893c04cf08d0">+0/-19</a>    </td> </tr> <tr> <td>design.md</td> <td><a href="https://github.com/carverauto/serviceradar/pull/2163/files#diff-fcc4aa820ee62670cf4965e87b2e0206086d7bb9f83d4146f68a3d763ed66fd6">+62/-0</a>    </td> </tr> <tr> <td>proposal.md</td> <td><a href="https://github.com/carverauto/serviceradar/pull/2163/files#diff-5a6bb224d3c0031de4edb02183b529c4e54b7d0c4f2972babcc848411436718f">+25/-0</a>    </td> </tr> <tr> <td>spec.md</td> <td><a href="https://github.com/carverauto/serviceradar/pull/2163/files#diff-1e72eebd22cd4f5b5250f5b168b102c3b6fcc5d1ef2418d71acba7a0e3efd340">+20/-0</a>    </td> </tr> <tr> <td>tasks.md</td> <td><a href="https://github.com/carverauto/serviceradar/pull/2163/files#diff-eeff025d43ddd0d8d735cd97a9aa573b0d831f44190263c5cdcddce34d1a9b01">+28/-0</a>    </td> </tr> <tr> <td>proposal.md</td> <td><a href="https://github.com/carverauto/serviceradar/pull/2163/files#diff-81a990985d497493238589e703ea366222940cc604a571ce21bfeca5bfc442e5">+0/-14</a>    </td> </tr> <tr> <td>spec.md</td> <td><a href="https://github.com/carverauto/serviceradar/pull/2163/files#diff-f5daa5f8f49623f6cec383c2b7197ae8431b51bb217c391dd3f674a380bba00d">+0/-28</a>    </td> </tr> <tr> <td>spec.md</td> <td><a href="https://github.com/carverauto/serviceradar/pull/2163/files#diff-21ba4baae6c7dbf1e091af2f2cfde05086826dcd839802d6e26eaa407eab584c">+0/-35</a>    </td> </tr> <tr> <td>tasks.md</td> <td><a href="https://github.com/carverauto/serviceradar/pull/2163/files#diff-c00044e9668389895df2d518bf418b693b3a12ad3488e2926abbdd71b98644ba">+0/-20</a>    </td> </tr> <tr> <td>design.md</td> <td><a href="https://github.com/carverauto/serviceradar/pull/2163/files#diff-cb59e5a2e863f7206ac3be889b3cc23b7eef933fbc33909b0d4f19235eeacbd6">+208/-0</a>  </td> </tr> <tr> <td>proposal.md</td> <td><a href="https://github.com/carverauto/serviceradar/pull/2163/files#diff-381b711be98b0c138b5513beec105376f4b63e46d82bfd75e7d22e4b0eb3e6ff">+54/-0</a>    </td> </tr> <tr> <td>tasks.md</td> <td><a href="https://github.com/carverauto/serviceradar/pull/2163/files#diff-52b58e4010c2df75f7534dc76212ed1c275e462f5f4be90fa0b947a5bfd407ce">+220/-0</a>  </td> </tr> <tr> <td>proposal.md</td> <td><a href="https://github.com/carverauto/serviceradar/pull/2163/files#diff-8478335a646a61fa4a74ea2f9c784cd441632ec92dba1b3f8d5b5fed52c9db3d">+0/-18</a>    </td> </tr> <tr> <td>spec.md</td> <td><a href="https://github.com/carverauto/serviceradar/pull/2163/files#diff-e14559d7bc9392db00b7d08e31b875dd4fbe0eb15538a04ec4e9dbd8b342ad8c">+0/-61</a>    </td> </tr> <tr> <td>tasks.md</td> <td><a href="https://github.com/carverauto/serviceradar/pull/2163/files#diff-da75158b3b75bf8129cbec0f47cd86dcee1b97f8cccb1b1864c628e11fb83beb">+0/-13</a>    </td> </tr> <tr> <td>proposal.md</td> <td><a href="https://github.com/carverauto/serviceradar/pull/2163/files#diff-7af7b9e5ec186b60f6082c416f1cf7fa777e26c7b67aa7a2fe5f582f3c558813">+0/-45</a>    </td> </tr> <tr> <td>spec.md</td> <td><a href="https://github.com/carverauto/serviceradar/pull/2163/files#diff-860d8d8d95eb220b8d6e5590df8982fec404e52dea8565587343d431998e54bb">+0/-60</a>    </td> </tr> <tr> <td>tasks.md</td> <td><a href="https://github.com/carverauto/serviceradar/pull/2163/files#diff-9031f16c5cff8b7150870d905750fef62ad51f9ec20faec52aa62a74efcb79a9">+0/-28</a>    </td> </tr> <tr> <td>proposal.md</td> <td><a href="https://github.com/carverauto/serviceradar/pull/2163/files#diff-ed59ef5f7b9f94cea669c81eccf7ae83d579c5ea60b4f139da3eb68a02e368ff">+0/-83</a>    </td> </tr> <tr> <td>spec.md</td> <td><a href="https://github.com/carverauto/serviceradar/pull/2163/files#diff-796f2e05e4b1942bcfd740f51f5f5a63cace288520ceee7a439936da6196521e">+0/-112</a>  </td> </tr> <tr> <td>tasks.md</td> <td><a href="https://github.com/carverauto/serviceradar/pull/2163/files#diff-7306d84aa629841d30563d0ddbcc2178e690cdd564451cbf009ebdbfdcbf3e95">+0/-84</a>    </td> </tr> <tr> <td>proposal.md</td> <td><a href="https://github.com/carverauto/serviceradar/pull/2163/files#diff-804cde784a8519f9f453950e7f5faf7eebe67a771a7c29a501f53acdd5ffac61">+0/-44</a>    </td> </tr> <tr> <td>spec.md</td> <td><a href="https://github.com/carverauto/serviceradar/pull/2163/files#diff-74ae971093987c8e2e0fae413c88feb5b5e95218869956d88808827005df0029">+0/-32</a>    </td> </tr> <tr> <td>tasks.md</td> <td><a href="https://github.com/carverauto/serviceradar/pull/2163/files#diff-9ef6c02134c8900f69fbd4326e8f1facbf6ac0cced8f09714c0776270840f24f">+0/-45</a>    </td> </tr> <tr> <td>proposal.md</td> <td><a href="https://github.com/carverauto/serviceradar/pull/2163/files#diff-81ac6baf30e8d593daac5904fe8ee44954ea59b60cfebb932d6167039401ec70">+0/-26</a>    </td> </tr> <tr> <td>spec.md</td> <td><a href="https://github.com/carverauto/serviceradar/pull/2163/files#diff-b476498724d1ccfae69ca3db18d65fc2a18e629803fe277531948545ca36b944">+0/-46</a>    </td> </tr> <tr> <td>tasks.md</td> <td><a href="https://github.com/carverauto/serviceradar/pull/2163/files#diff-e7a2e1f30c6f53270296c7e92cf5828c71a4a21dc4ea2a6c1b4d74da304e87ac">+0/-24</a>    </td> </tr> <tr> <td>proposal.md</td> <td><a href="https://github.com/carverauto/serviceradar/pull/2163/files#diff-ca1b0821c2554f35858d70b8a41cd51bac6611ac04e7e16dfad75f38ed2ce930">+0/-19</a>    </td> </tr> <tr> <td>spec.md</td> <td><a href="https://github.com/carverauto/serviceradar/pull/2163/files#diff-69f23ca388bf44599fe256a2dedafc0e8af9fb346bb917efe6f2c95b84e971d7">+0/-18</a>    </td> </tr> <tr> <td>tasks.md</td> <td><a href="https://github.com/carverauto/serviceradar/pull/2163/files#diff-0d3a5a254abeadabd0c30d04b56b43c8f4b43dcdb6c8f300d1f953fa602247d7">+0/-19</a>    </td> </tr> <tr> <td>design.md</td> <td><a href="https://github.com/carverauto/serviceradar/pull/2163/files#diff-7120eaa6a5f458aa2e1649559057f77ecd7355c3eb9079a3cbe53fdfac97b93b">+32/-0</a>    </td> </tr> <tr> <td>proposal.md</td> <td><a href="https://github.com/carverauto/serviceradar/pull/2163/files#diff-cd52b3fd553ef7792c277ff50c4e9a9e1f3499b62def55e845f1311f5da7eb0d">+22/-0</a>    </td> </tr> <tr> <td>spec.md</td> <td><a href="https://github.com/carverauto/serviceradar/pull/2163/files#diff-a212262a53d3ee2d68753329b05835cd20f2aa7671ffa686b4a6083f1d330256">+13/-0</a>    </td> </tr> <tr> <td>tasks.md</td> <td><a href="https://github.com/carverauto/serviceradar/pull/2163/files#diff-0b26ad240d4dcd68f67721a1352e54bcf78ee404d2ee167feca6675008466f61">+17/-0</a>    </td> </tr> <tr> <td>design.md</td> <td><a href="https://github.com/carverauto/serviceradar/pull/2163/files#diff-241d7df520e53d9bafd95027f32a00ffe31495c6bfaec479494a5904c4215d63">+30/-0</a>    </td> </tr> <tr> <td>proposal.md</td> <td><a href="https://github.com/carverauto/serviceradar/pull/2163/files#diff-9b1ddcbf90157cb6d07c63b0c435a434272e3279594c5510f2a3d297223fd672">+15/-0</a>    </td> </tr> <tr> <td>spec.md</td> <td><a href="https://github.com/carverauto/serviceradar/pull/2163/files#diff-2d6fc29f0ddfb9b6213d83339d0d93ef5196a7ec7fc0e6b07c587c5908894c95">+40/-0</a>    </td> </tr> <tr> <td>tasks.md</td> <td><a href="https://github.com/carverauto/serviceradar/pull/2163/files#diff-9024bbd8abac8c96b2d4e0437250ee344cf5f12560785561285afe82e21d4457">+18/-0</a>    </td> </tr> <tr> <td>design.md</td> <td><a href="https://github.com/carverauto/serviceradar/pull/2163/files#diff-551f58cf66926ed96cb4df08f8de6bb718951933afbbcbb86831aa837b41d79f">+39/-0</a>    </td> </tr> <tr> <td>proposal.md</td> <td><a href="https://github.com/carverauto/serviceradar/pull/2163/files#diff-2f311be222e7803de3df6b7b3361776eca39ecaa96c09f5699f260450154ec3d">+19/-0</a>    </td> </tr> <tr> <td>spec.md</td> <td><a href="https://github.com/carverauto/serviceradar/pull/2163/files#diff-113e7f6dcd42a481e54c270e3233214e97232f2c1788722655352a1e6d49590e">+23/-0</a>    </td> </tr> <tr> <td>tasks.md</td> <td><a href="https://github.com/carverauto/serviceradar/pull/2163/files#diff-3c4807d2b2ab7fb4b40fc23f5b929a2d0ffc4915da557ec96742b82b6d7b3fb8">+13/-0</a>    </td> </tr> <tr> <td>design.md</td> <td><a href="https://github.com/carverauto/serviceradar/pull/2163/files#diff-fd19193e9fe30b9b73e8c397a9a274eafa39efe8cef21fd07190038fc867112c">+43/-0</a>    </td> </tr> <tr> <td>proposal.md</td> <td><a href="https://github.com/carverauto/serviceradar/pull/2163/files#diff-511a066de4507065281b37a979c77119b90e46e97e206f2e879f8af73d13c854">+20/-0</a>    </td> </tr> <tr> <td>spec.md</td> <td><a href="https://github.com/carverauto/serviceradar/pull/2163/files#diff-e8ea81126ba5d49a821706cd6479d176ac1ee40a496dbb38938d830ea94d3e0a">+27/-0</a>    </td> </tr> <tr> <td>tasks.md</td> <td><a href="https://github.com/carverauto/serviceradar/pull/2163/files#diff-6aa430ae5e044c6f55358d45cd7b2368b1168a735aa4f84739c6102fa7769720">+10/-0</a>    </td> </tr> <tr> <td>proposal.md</td> <td><a href="https://github.com/carverauto/serviceradar/pull/2163/files#diff-bc74372977e86a0e3f64c1af49c0610fe4d80798113604c17298cba508433003">+0/-144</a>  </td> </tr> <tr> <td>spec.md</td> <td><a href="https://github.com/carverauto/serviceradar/pull/2163/files#diff-57a85c7ff6659407d8665b61c6d9d7b09c71a213d1aca1e5fd993c027c14cfc0">+0/-81</a>    </td> </tr> <tr> <td>tasks.md</td> <td><a href="https://github.com/carverauto/serviceradar/pull/2163/files#diff-1ba2529c7b515076581e20414759e23c0d25c327e312d456620c98c2c0700f6a">+0/-43</a>    </td> </tr> <tr> <td>proposal.md</td> <td><a href="https://github.com/carverauto/serviceradar/pull/2163/files#diff-4bcd11bed34206ce4102519db0e148b7b197fab33423d03712aa23475df761bc">+0/-14</a>    </td> </tr> <tr> <td>spec.md</td> <td><a href="https://github.com/carverauto/serviceradar/pull/2163/files#diff-9f15500fd74ac3845aec9f5a6a2b2024a019b2f9438ce92f47fe18a6d945ab09">+0/-14</a>    </td> </tr> <tr> <td>tasks.md</td> <td><a href="https://github.com/carverauto/serviceradar/pull/2163/files#diff-bd097cdff5fea1b8137a6f3fffb2dda7686b999f1e6e3889b4372481c1c37a66">+0/-17</a>    </td> </tr> <tr> <td>proposal.md</td> <td><a href="https://github.com/carverauto/serviceradar/pull/2163/files#diff-0534d758c6b8e27938b14175fe5a6ff9bf0ee3a352da208c3f651cab9a460736">+16/-0</a>    </td> </tr> <tr> <td>spec.md</td> <td><a href="https://github.com/carverauto/serviceradar/pull/2163/files#diff-5845d29d43f16fd852471b164a176e879de47eb15be4d4bcc32e27130a6095ec">+10/-0</a>    </td> </tr> <tr> <td>tasks.md</td> <td><a href="https://github.com/carverauto/serviceradar/pull/2163/files#diff-f7b9a49b7e16fb0ce430f87e5fe38160d9a072efe92259ca776fe6ee0449a7c9">+12/-0</a>    </td> </tr> <tr> <td>proposal.md</td> <td><a href="https://github.com/carverauto/serviceradar/pull/2163/files#diff-8bad22e013f99a913f7db91e78970c89634e94cd39262d339a06e72734531ee4">+17/-0</a>    </td> </tr> <tr> <td>spec.md</td> <td><a href="https://github.com/carverauto/serviceradar/pull/2163/files#diff-fa51756b168cdca03f99da7568ffe261ea59b0b33f5c7ac1de4781af378f55fc">+20/-0</a>    </td> </tr> <tr> <td>tasks.md</td> <td><a href="https://github.com/carverauto/serviceradar/pull/2163/files#diff-b18156d1b4a0904f964884f6642aae45fd88d0ddcab58b0c699d236e34be3cee">+13/-0</a>    </td> </tr> <tr> <td>proposal.md</td> <td><a href="https://github.com/carverauto/serviceradar/pull/2163/files#diff-a913209a981d4c6b3e8339293fcb062e02065c6485c215304d34a66dfca38997">+0/-39</a>    </td> </tr> <tr> <td>spec.md</td> <td><a href="https://github.com/carverauto/serviceradar/pull/2163/files#diff-8ea36913f5d5236389f519283c919ffd1c3d1124de5684b954873958051c8dbb">+0/-70</a>    </td> </tr> <tr> <td>tasks.md</td> <td><a href="https://github.com/carverauto/serviceradar/pull/2163/files#diff-138a6873eaaab55e8974878fd16911bae1de7b8c708e006b996744428c2ecd5b">+0/-33</a>    </td> </tr> <tr> <td>proposal.md</td> <td><a href="https://github.com/carverauto/serviceradar/pull/2163/files#diff-6303af01f1a835c5774bb184afb2e37b9707b7b7c4283e3ee537b2b221ba614b">+28/-0</a>    </td> </tr> <tr> <td>spec.md</td> <td><a href="https://github.com/carverauto/serviceradar/pull/2163/files#diff-34df7c0ca139956ca9981cd0accef64a55838bf3057b72fc9dfcfa77ff19bd82">+21/-0</a>    </td> </tr> <tr> <td>tasks.md</td> <td><a href="https://github.com/carverauto/serviceradar/pull/2163/files#diff-116bbee895b6dbf4fa6aaa32bd8fa799e2d2181a66071fb5e3129a46cc9154d7">+17/-0</a>    </td> </tr> <tr> <td>proposal.md</td> <td><a href="https://github.com/carverauto/serviceradar/pull/2163/files#diff-c47d90ba805c8ea005726fc211e6179821ffa7e92b78054a338f6d5f6b81ab31">+0/-102</a>  </td> </tr> <tr> <td>spec.md</td> <td><a href="https://github.com/carverauto/serviceradar/pull/2163/files#diff-4a5a9adb68cec10a1c6b76d4788fe9b774c2f0034d1aa780814fb144fc458ad0">+0/-46</a>    </td> </tr> <tr> <td>tasks.md</td> <td><a href="https://github.com/carverauto/serviceradar/pull/2163/files#diff-f34fa0a85f29520c314cde60d360176dff077bc0aae86fa99d96764c523ccac3">+0/-53</a>    </td> </tr> <tr> <td>proposal.md</td> <td><a href="https://github.com/carverauto/serviceradar/pull/2163/files#diff-23212a3c01b653c3006efbbf6ca9782f767e74ce7aa85d7b71805dbb36d562ae">+0/-125</a>  </td> </tr> <tr> <td>proposal.md</td> <td><a href="https://github.com/carverauto/serviceradar/pull/2163/files#diff-ad3bf3baec592ea2000de51abcd43628efd97c1524956d0580562e54b56515ad">+0/-129</a>  </td> </tr> <tr> <td>spec.md</td> <td><a href="https://github.com/carverauto/serviceradar/pull/2163/files#diff-239ceb7ed8d7185674ebc80a40aab82ea6955a159999df3ec7736870a416fac3">+0/-22</a>    </td> </tr> <tr> <td>tasks.md</td> <td><a href="https://github.com/carverauto/serviceradar/pull/2163/files#diff-355450ac691dae6d2c3167233558588ea0e0ab969b5266044d4226b00e61ddab">+0/-15</a>    </td> </tr> <tr> <td>Additional files not shown</td> <td><a href="https://github.com/carverauto/serviceradar/pull/2163/files#diff-2f328e4cd8dbe3ad193e49d92bcf045f47a6b72b1e9487d366f6b8288589b4ca"></a></td> </tr> </table></details></td></tr></tbody></table> </details> ___

qodo-code-review[bot] commented

2025-12-17 00:13:41 +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/2163#issuecomment-3662998200
Original created: 2025-12-17T00:13:41Z

You are nearing your monthly Qodo Merge usage quota. For more information, please visit here.

PR Compliance Guide 🔍

Below is a summary of compliance checks for this PR:

Security Compliance

⚪

SRQL injection risk

Description: User-controlled params.Filter is appended directly into the SRQL WHERE conditions without
binding/sanitization, allowing crafted input (e.g., "x' OR '1'='1") to alter query
semantics if Filter originates from external requests. query_utils.go [83-90]

Referred Code

func buildLogQuery(params LogQueryParams) (string, []any) {
	query := showLogsQuery
	conditions := []string{}
	binds := &srqlBindBuilder{}

	if params.Filter != "" {
		conditions = append(conditions, params.Filter)
	}

Ticket Compliance

🟡

🎫 #2145

🔴	Fix `cloneDeviceRecord` in `pkg/registry/device_store.go` to deep-copy `Metadata` even when it is an empty-but-non-nil map, so clones do not share the underlying map with the source record.
🔴	Ensure modifying the cloned record’s `Metadata` does not mutate the original stored record’s `Metadata` (defensive copy / snapshot isolation).
⚪	Add/ensure tests cover the empty-but-non-nil `Metadata` map cloning scenario (and that they pass).

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

Generic: Secure Error Handling

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

Status: Passed

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: Passed

🔴

Generic: Robust Error Handling and Edge Case Management

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

Status:
Silent lookup failure: Batch identifier lookup errors are silently swallowed by returning an empty match map
without logging or propagating context, making failures hard to detect and debug.

Referred Code

results, err := e.db.BatchGetDeviceIDsByIdentifier(ctx, identifierType, values, partition)
if err != nil {
	return matches
}

⚪

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 new generic OTEL insert path centralizes write operations but does not visibly add or
ensure audit-context logging (e.g., actor/user ID and outcome), which may be required for
critical actions depending on how these inserts are triggered.

Referred Code

func (db *DB) insertOTELRows(
	ctx context.Context,
	table string,
	defaultTable string,
	kind string,
	rowCount int,
	buildQuery func(sanitizedTable string) string,
	timestampAt func(rowIndex int) time.Time,
	queueRow func(batch *pgx.Batch, query string, rowIndex int, timestamp time.Time),
) error {
	if rowCount == 0 {
		return nil
	}

	if !db.cnpgConfigured() {
		return ErrCNPGUnavailable
	}

	sanitizedTable, canonicalTable := sanitizeObservabilityTable(table, defaultTable)
	query := buildQuery(sanitizedTable)



 ... (clipped 14 lines)

Generic: Security-First Input Validation and Data Handling

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

Status:
Mixed param enforcement: The new parameter-binding support improves injection safety, but because some paths can
still execute non-parameterized SRQL when params is empty, a human review is needed to
confirm all externally influenced filters are consistently bound (especially in
unprocessed files and other tools).

Referred Code

// ParameterizedQueryExecutor extends QueryExecutor with parameter binding support.
// Implementations MUST treat params as bound values (not text concatenated into query).
type ParameterizedQueryExecutor interface {
	QueryExecutor
	ExecuteSRQLQueryWithParams(ctx context.Context, query string, params []any, limit int) ([]map[string]interface{}, error)
}

func executeSRQL(ctx context.Context, executor QueryExecutor, query string, params []any, limit int) ([]map[string]interface{}, error) {
	if len(params) == 0 {
		return executor.ExecuteSRQLQuery(ctx, query, limit)
	}

	parameterized, ok := executor.(ParameterizedQueryExecutor)
	if !ok {
		return nil, fmt.Errorf("%w", errParameterizedSRQLNotSupported)
	}

	return parameterized.ExecuteSRQLQueryWithParams(ctx, query, params, limit)
}

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/2163#issuecomment-3662998200 Original created: 2025-12-17T00:13:41Z --- _You are nearing your monthly Qodo Merge usage quota. For more information, please visit [here](https://qodo-merge-docs.qodo.ai/installation/qodo_merge/#cloud-users)._ ## 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>SRQL injection risk </summary> Description: User-controlled <code>params.Filter</code> is appended directly into the SRQL <code>WHERE</code> conditions without binding/sanitization, allowing crafted input (e.g., <code>"x' OR '1'='1"</code>) to alter query semantics if <code>Filter</code> originates from external requests. <a href='https://github.com/carverauto/serviceradar/pull/2163/files#diff-6a33491056954507b65b8252854786d3d9c65534c7c88700fcc2592f808b192bR83-R90'>query_utils.go [83-90]</a> <details open><summary>Referred Code</summary> ```go func buildLogQuery(params LogQueryParams) (string, []any) { query := showLogsQuery conditions := []string{} binds := &srqlBindBuilder{} if params.Filter != "" { conditions = append(conditions, params.Filter) } ``` </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/2145>#2145</a></summary> <table width='100%'><tbody> <tr><td rowspan=2>🔴</td> <td>Fix <code>cloneDeviceRecord</code> in <code>pkg/registry/device_store.go</code> to deep-copy <code>Metadata</code> even when it is an empty-but-non-nil map, so clones do not share the underlying map with the source record. </td></tr> <tr><td>Ensure modifying the cloned record’s <code>Metadata</code> does not mutate the original stored record’s <code>Metadata</code> (defensive copy / snapshot isolation). </td></tr> <tr><td rowspan=1>⚪</td> <td>Add/ensure tests cover the empty-but-non-nil <code>Metadata</code> map cloning scenario (and that they pass). </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=3>🟢</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> <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:** 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> <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:** 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=1>🔴</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/2163/files#diff-496f24b3784656e1d6bb97cafe5c528bbecea5a0b74b4be578d3b83e858038c7R630-R633'>Silent lookup failure</a>: Batch identifier lookup errors are silently swallowed by returning an empty match map without logging or propagating context, making failures hard to detect and debug. <details open><summary>Referred Code</summary> ```go results, err := e.db.BatchGetDeviceIDsByIdentifier(ctx, identifierType, values, partition) if err != nil { return matches } ``` </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/2163/files#diff-abaae95adb6fa6d0241553f6d1b39ecff3dd6159db72babbe39dfb3cd231cd3dR213-R247'>Audit logging unclear</a>: The new generic OTEL insert path centralizes write operations but does not visibly add or ensure audit-context logging (e.g., actor/user ID and outcome), which may be required for critical actions depending on how these inserts are triggered. <details open><summary>Referred Code</summary> ```go func (db *DB) insertOTELRows( ctx context.Context, table string, defaultTable string, kind string, rowCount int, buildQuery func(sanitizedTable string) string, timestampAt func(rowIndex int) time.Time, queueRow func(batch *pgx.Batch, query string, rowIndex int, timestamp time.Time), ) error { if rowCount == 0 { return nil } if !db.cnpgConfigured() { return ErrCNPGUnavailable } sanitizedTable, canonicalTable := sanitizeObservabilityTable(table, defaultTable) query := buildQuery(sanitizedTable) ... (clipped 14 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: 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/2163/files#diff-6a33491056954507b65b8252854786d3d9c65534c7c88700fcc2592f808b192bR63-R81'>Mixed param enforcement</a>: The new parameter-binding support improves injection safety, but because some paths can still execute non-parameterized SRQL when <code>params</code> is empty, a human review is needed to confirm all externally influenced filters are consistently bound (especially in unprocessed files and other tools). <details open><summary>Referred Code</summary> ```go // ParameterizedQueryExecutor extends QueryExecutor with parameter binding support. // Implementations MUST treat params as bound values (not text concatenated into query). type ParameterizedQueryExecutor interface { QueryExecutor ExecuteSRQLQueryWithParams(ctx context.Context, query string, params []any, limit int) ([]map[string]interface{}, error) } func executeSRQL(ctx context.Context, executor QueryExecutor, query string, params []any, limit int) ([]map[string]interface{}, error) { if len(params) == 0 { return executor.ExecuteSRQLQuery(ctx, query, limit) } parameterized, ok := executor.(ParameterizedQueryExecutor) if !ok { return nil, fmt.Errorf("%w", errParameterizedSRQLNotSupported) } return parameterized.ExecuteSRQLQueryWithParams(ctx, query, params, limit) } ``` </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">   </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

2025-12-17 00:14:43 +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/2163#issuecomment-3663000459
Original created: 2025-12-17T00:14:43Z

You are nearing your monthly Qodo Merge usage quota. For more information, please visit here.

PR Compliance Guide 🔍

Below is a summary of compliance checks for this PR:

Security Compliance

⚪

SRQL injection

Description: buildLogQuery appends params.Filter directly into the SRQL WHERE clause without parameter
binding or validation, allowing crafted filter text (e.g., ... OR 1=1 ...) to alter query
semantics despite other parameters being bound. query_utils.go [83-100]

Referred Code

func buildLogQuery(params LogQueryParams) (string, []any) {
	query := showLogsQuery
	conditions := []string{}
	binds := &srqlBindBuilder{}

	if params.Filter != "" {
		conditions = append(conditions, params.Filter)
	}

	if params.StartTime != "" {
		conditions = append(conditions, fmt.Sprintf("_tp_time >= %s", binds.Bind(params.StartTime)))
	}

	if params.EndTime != "" {
		conditions = append(conditions, fmt.Sprintf("_tp_time <= %s", binds.Bind(params.EndTime)))
	}

	if len(conditions) > 0 {

Ticket Compliance

🟡

🎫 #2145

⚪	Fix `cloneDeviceRecord` in `pkg/registry/device_store.go` so that when `src.Metadata` is an empty but non-nil map, the clone gets its own independent (deep-copied) map rather than sharing the original map reference.
	Ensure callers modifying the cloned record's `Metadata` cannot affect the original stored `DeviceRecord` (defensive copy semantics).
	Add/ensure tests covering the empty-but-non-nil `Metadata` map case to prevent regressions.

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

🔴

Generic: Security-First Input Validation and Data Handling

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

Status:
Unvalidated filter injection: buildLogQuery directly appends params.Filter into the SRQL WHERE clause without validation
or parameter binding, reintroducing injection risk despite other parameterization changes.

Referred Code

func buildLogQuery(params LogQueryParams) (string, []any) {
	query := showLogsQuery
	conditions := []string{}
	binds := &srqlBindBuilder{}

	if params.Filter != "" {
		conditions = append(conditions, params.Filter)
	}

	if params.StartTime != "" {
		conditions = append(conditions, fmt.Sprintf("_tp_time >= %s", binds.Bind(params.StartTime)))
	}

	if params.EndTime != "" {
		conditions = append(conditions, fmt.Sprintf("_tp_time <= %s", binds.Bind(params.EndTime)))
	}

	if len(conditions) > 0 {

⚪

Generic: Comprehensive Audit Trails

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

Status:
Audit scope unclear: The PR adds new query execution paths (including parameterized execution) but the diff
does not show whether these critical query actions are audited with user identity,
timestamp, and outcome.

Referred Code

// executeSRQLQuery executes an SRQL query directly via the query executor
// It handles transformations for unsupported entity types like sweep_results
func (m *MCPServer) executeSRQLQuery(ctx context.Context, query string, limit int) ([]map[string]interface{}, error) {
	return m.executeSRQLQueryWithParams(ctx, query, nil, limit)
}

// executeSRQLQueryWithParams executes an SRQL query with bound parameters via a parameter-capable query executor.
// If params is empty, this falls back to plain ExecuteSRQLQuery.
func (m *MCPServer) executeSRQLQueryWithParams(ctx context.Context, query string, params []any, limit int) ([]map[string]interface{}, error) {
	// Transform sweep_results queries to devices queries with sweep discovery source filter
	transformedQuery := m.transformSweepResultsQuery(query)

	if len(params) == 0 {
		return m.queryExecutor.ExecuteSRQLQuery(ctx, transformedQuery, limit)
	}

	executor, ok := m.queryExecutor.(ParameterizedQueryExecutor)
	if !ok {
		return nil, fmt.Errorf("%w", errParameterizedSRQLNotSupported)
	}




 ... (clipped 2 lines)

Generic: Robust Error Handling and Edge Case Management

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

Status:
Error silently dropped: Database lookup errors in batchLookupIdentifierType are swallowed by returning an empty
map without logging or surfacing context, which can hide partial failures and complicate
debugging.

Referred Code

func (e *IdentityEngine) batchLookupIdentifierType(
	ctx context.Context,
	identifierType string,
	identifierValues map[string]struct{},
	partition string,
) map[string]string {
	matches := make(map[string]string)
	if e == nil || e.db == nil || identifierType == "" || len(identifierValues) == 0 {
		return matches
	}

	values := make([]string, 0, len(identifierValues))
	for v := range identifierValues {
		values = append(values, v)
	}

	results, err := e.db.BatchGetDeviceIDsByIdentifier(ctx, identifierType, values, partition)
	if err != nil {
		return matches
	}

Generic: Secure Error Handling

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

Status:
Potential user-facing detail: The handler returns raw execution errors (wrapped) which may expose backend implementation
details depending on how MCP tool errors are presented to end users.

Referred Code

Name:        name,
Description: description,
Handler: func(ctx context.Context, args json.RawMessage) (interface{}, error) {
	binds := &srqlBindBuilder{}

	// Extract common filter parameters
	var commonParams FilterQueryParams
	if err := json.Unmarshal(args, &commonParams); err != nil {
		return nil, fmt.Errorf("invalid filter arguments: %w", err)
	}

	// Get entity-specific filters and response filters
	additionalFilters, responseFilters, err := filterHandler(args, binds)
	if err != nil {
		return nil, err
	}



 ... (clipped 6 lines)

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 include identifier: The tool logs user-supplied device_id which could be sensitive in some deployments and
should be assessed against logging policy/redaction requirements.

Referred Code

// Build SRQL query for specific device (parameterized)
filter := "device_id = $1"
query := BuildSRQL("devices", filter, "", 1, false)

m.logger.Debug().Str("device_id", deviceIDArgs.DeviceID).Str("query", query).Msg("Retrieving device")

// Execute SRQL query via API
results, err := m.executeSRQLQueryWithParams(ctx, query, []any{deviceIDArgs.DeviceID}, 1)
if err != nil {

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/2163#issuecomment-3663000459 Original created: 2025-12-17T00:14:43Z --- _You are nearing your monthly Qodo Merge usage quota. For more information, please visit [here](https://qodo-merge-docs.qodo.ai/installation/qodo_merge/#cloud-users)._ ## 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>SRQL injection </summary> Description: <code>buildLogQuery</code> appends <code>params.Filter</code> directly into the SRQL <code>WHERE</code> clause without parameter binding or validation, allowing crafted filter text (e.g., <code>... OR 1=1 ...</code>) to alter query semantics despite other parameters being bound. <a href='https://github.com/carverauto/serviceradar/pull/2163/files#diff-6a33491056954507b65b8252854786d3d9c65534c7c88700fcc2592f808b192bR83-R100'>query_utils.go [83-100]</a> <details open><summary>Referred Code</summary> ```go func buildLogQuery(params LogQueryParams) (string, []any) { query := showLogsQuery conditions := []string{} binds := &srqlBindBuilder{} if params.Filter != "" { conditions = append(conditions, params.Filter) } if params.StartTime != "" { conditions = append(conditions, fmt.Sprintf("_tp_time >= %s", binds.Bind(params.StartTime))) } if params.EndTime != "" { conditions = append(conditions, fmt.Sprintf("_tp_time <= %s", binds.Bind(params.EndTime))) } if len(conditions) > 0 { ``` </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/2145>#2145</a></summary> <table width='100%'><tbody> <tr><td rowspan=3>⚪</td> <td>Fix <code>cloneDeviceRecord</code> in <code>pkg/registry/device_store.go</code> so that when <code>src.Metadata</code> is an empty but non-nil map, the clone gets its own independent (deep-copied) map rather than sharing the original map reference. </td></tr> <tr><td>Ensure callers modifying the cloned record's <code>Metadata</code> cannot affect the original stored <code>DeviceRecord</code> (defensive copy semantics). </td></tr> <tr><td>Add/ensure tests covering the empty-but-non-nil <code>Metadata</code> map case to prevent regressions. </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=1>🔴</td> <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/2163/files#diff-6a33491056954507b65b8252854786d3d9c65534c7c88700fcc2592f808b192bR83-R100'>Unvalidated filter injection</a>: <code>buildLogQuery</code> directly appends <code>params.Filter</code> into the SRQL WHERE clause without validation or parameter binding, reintroducing injection risk despite other parameterization changes. <details open><summary>Referred Code</summary> ```go func buildLogQuery(params LogQueryParams) (string, []any) { query := showLogsQuery conditions := []string{} binds := &srqlBindBuilder{} if params.Filter != "" { conditions = append(conditions, params.Filter) } if params.StartTime != "" { conditions = append(conditions, fmt.Sprintf("_tp_time >= %s", binds.Bind(params.StartTime))) } if params.EndTime != "" { conditions = append(conditions, fmt.Sprintf("_tp_time <= %s", binds.Bind(params.EndTime))) } if len(conditions) > 0 { ``` </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=4>⚪</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/2163/files#diff-bbb88ac338d81b0a37cd59fc53d68171d2de4780653253a2fd257f18fbf81bd0R801-R823'>Audit scope unclear</a>: The PR adds new query execution paths (including parameterized execution) but the diff does not show whether these critical query actions are audited with user identity, timestamp, and outcome. <details open><summary>Referred Code</summary> ```go // executeSRQLQuery executes an SRQL query directly via the query executor // It handles transformations for unsupported entity types like sweep_results func (m *MCPServer) executeSRQLQuery(ctx context.Context, query string, limit int) ([]map[string]interface{}, error) { return m.executeSRQLQueryWithParams(ctx, query, nil, limit) } // executeSRQLQueryWithParams executes an SRQL query with bound parameters via a parameter-capable query executor. // If params is empty, this falls back to plain ExecuteSRQLQuery. func (m *MCPServer) executeSRQLQueryWithParams(ctx context.Context, query string, params []any, limit int) ([]map[string]interface{}, error) { // Transform sweep_results queries to devices queries with sweep discovery source filter transformedQuery := m.transformSweepResultsQuery(query) if len(params) == 0 { return m.queryExecutor.ExecuteSRQLQuery(ctx, transformedQuery, limit) } executor, ok := m.queryExecutor.(ParameterizedQueryExecutor) if !ok { return nil, fmt.Errorf("%w", errParameterizedSRQLNotSupported) } ... (clipped 2 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: 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/2163/files#diff-496f24b3784656e1d6bb97cafe5c528bbecea5a0b74b4be578d3b83e858038c7R614-R633'>Error silently dropped</a>: Database lookup errors in <code>batchLookupIdentifierType</code> are swallowed by returning an empty map without logging or surfacing context, which can hide partial failures and complicate debugging. <details open><summary>Referred Code</summary> ```go func (e *IdentityEngine) batchLookupIdentifierType( ctx context.Context, identifierType string, identifierValues map[string]struct{}, partition string, ) map[string]string { matches := make(map[string]string) if e == nil || e.db == nil || identifierType == "" || len(identifierValues) == 0 { return matches } values := make([]string, 0, len(identifierValues)) for v := range identifierValues { values = append(values, v) } results, err := e.db.BatchGetDeviceIDsByIdentifier(ctx, identifierType, values, partition) if err != nil { return matches } ``` </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/2163/files#diff-ee3c16d7d8882a3a65d28bfedf7f0a5e59698b00ae482bc5565b3314503d5db2R198-R224'>Potential user-facing detail</a>: The handler returns raw execution errors (wrapped) which may expose backend implementation details depending on how MCP tool errors are presented to end users. <details open><summary>Referred Code</summary> ```go Name: name, Description: description, Handler: func(ctx context.Context, args json.RawMessage) (interface{}, error) { binds := &srqlBindBuilder{} // Extract common filter parameters var commonParams FilterQueryParams if err := json.Unmarshal(args, &commonParams); err != nil { return nil, fmt.Errorf("invalid filter arguments: %w", err) } // Get entity-specific filters and response filters additionalFilters, responseFilters, err := filterHandler(args, binds) if err != nil { return nil, err } ... (clipped 6 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 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/2163/files#diff-9305d32d7f3683b79015dd7448623cf5393bf7f0631548313d810a3ed08d65ddR83-R91'>Logs include identifier</a>: The tool logs user-supplied <code>device_id</code> which could be sensitive in some deployments and should be assessed against logging policy/redaction requirements. <details open><summary>Referred Code</summary> ```go // Build SRQL query for specific device (parameterized) filter := "device_id = $1" query := BuildSRQL("devices", filter, "", 1, false) m.logger.Debug().Str("device_id", deviceIDArgs.DeviceID).Str("query", query).Msg("Retrieving device") // Execute SRQL query via API results, err := m.executeSRQLQueryWithParams(ctx, query, []any{deviceIDArgs.DeviceID}, 1) if err != nil { ``` </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">   </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

2025-12-17 00:15:27 +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/2163#issuecomment-3663002544
Original created: 2025-12-17T00:15:27Z

PR Code Suggestions ✨

Latest suggestions up to 19d46a5

Category	Suggestion	Impact
Possible issue	Strengthen slice aliasing regression test Strengthen the slice aliasing test by adding assertions to verify that appending to a cloned slice does not affect the original record or other clones. pkg/registry/device_store_test.go [108-136] func TestCloneDeviceRecord_EmptySlicesWithCapacityDoNotAlias(t testing.T) { original := &DeviceRecord{ DeviceID: testDeviceID1, IP: "10.0.0.1", DiscoverySources: make([]string, 0, 1), Capabilities: make([]string, 0, 1), } clone1 := cloneDeviceRecord(original) if clone1 == nil { t.Fatalf("expected clone to be non-nil") } + clone1.DiscoverySources = append(clone1.DiscoverySources, "a") clone1.Capabilities = append(clone1.Capabilities, "icmp") + + // Appending to a clone must not mutate the original. + if len(original.DiscoverySources) != 0 { + t.Fatalf("expected original discovery sources to remain empty, got %#v", original.DiscoverySources) + } + if len(original.Capabilities) != 0 { + t.Fatalf("expected original capabilities to remain empty, got %#v", original.Capabilities) + } clone2 := cloneDeviceRecord(original) if clone2 == nil { t.Fatalf("expected second clone to be non-nil") } + clone2.DiscoverySources = append(clone2.DiscoverySources, "b") clone2.Capabilities = append(clone2.Capabilities, "snmp") - if got := clone1.DiscoverySources[0]; got != "a" { - t.Fatalf("expected clone1 discovery source to remain %q, got %q", "a", got) + // Mutating clone2 must not affect clone1. + if got := clone1.DiscoverySources; len(got) != 1 \|\| got[0] != "a" { + t.Fatalf("expected clone1 discovery sources to remain [\"a\"], got %#v", got) } - if got := clone1.Capabilities[0]; got != "icmp" { - t.Fatalf("expected clone1 capability to remain %q, got %q", "icmp", got) + if got := clone1.Capabilities; len(got) != 1 \|\| got[0] != "icmp" { + t.Fatalf("expected clone1 capabilities to remain [\"icmp\"], got %#v", got) + } + + // Sanity-check clone2 values. + if got := clone2.DiscoverySources; len(got) != 1 \|\| got[0] != "b" { + t.Fatalf("expected clone2 discovery sources to be [\"b\"], got %#v", got) + } + if got := clone2.Capabilities; len(got) != 1 \|\| got[0] != "snmp" { + t.Fatalf("expected clone2 capabilities to be [\"snmp\"], got %#v", got) } } Apply / Chat* Suggestion importance[1-10]: 7 __ Why: The suggestion correctly points out a significant weakness in the new test case, as it doesn't verify the most critical aspects of the fix: that the original object is not mutated and that clones are isolated from each other. Applying this suggestion makes the regression test substantially more robust.	Medium
Possible issue	Make metadata cloning test stricter Make the metadata cloning test stricter by adding assertions to ensure cloned maps are not nil and to verify that mutations on one clone do not affect another. pkg/registry/device_store_test.go [81-106] func TestCloneDeviceRecord_EmptyMetadataMapDoesNotAlias(t testing.T) { original := &DeviceRecord{ DeviceID: testDeviceID1, IP: "10.0.0.1", Metadata: map[string]string{}, } clone1 := cloneDeviceRecord(original) if clone1 == nil { t.Fatalf("expected clone to be non-nil") + } + if clone1.Metadata == nil { + t.Fatalf("expected clone metadata to be non-nil") } clone1.Metadata["k"] = "v1" if len(original.Metadata) != 0 { t.Fatalf("expected original metadata to remain empty, got %#v", original.Metadata) } clone2 := cloneDeviceRecord(original) if clone2 == nil { t.Fatalf("expected second clone to be non-nil") } + if clone2.Metadata == nil { + t.Fatalf("expected second clone metadata to be non-nil") + } + if _, ok := clone2.Metadata["k"]; ok { t.Fatalf("expected second clone metadata to not include prior clone mutation, got %#v", clone2.Metadata) } + + clone2.Metadata["k2"] = "v2" + if _, ok := clone1.Metadata["k2"]; ok { + t.Fatalf("expected clone1 metadata to not include clone2 mutation, got %#v", clone1.Metadata) + } } Apply / Chat* Suggestion importance[1-10]: 6 __ Why: The suggestion improves the test's robustness by adding a check for non-nil maps to prevent potential panics and by adding a symmetric check for clone-to-clone isolation, making the test more thorough.	Low
More

Previous suggestions

Suggestions up to commit 990a438

Category	Suggestion	Impact
High-level	PR contains many unrelated changes The PR is a large, monolithic submission containing many unrelated features and does not include the specific bug fix for `cloneDeviceRecord` that is described in the linked ticket. Solution Walkthrough: Before: // The PR is expected to fix a bug in `cloneDeviceRecord` // as described in the ticket. func cloneDeviceRecord(src DeviceRecord) DeviceRecord { // ... // BUG: This condition is false for empty, non-nil maps. if len(src.Metadata) > 0 { meta := make(map[string]string, len(src.Metadata)) for k, v := range src.Metadata { meta[k] = v } dst.Metadata = meta } return &dst } After: `// The PR should have contained this fix, but does not. func cloneDeviceRecord(src DeviceRecord) DeviceRecord { // ... // FIX: Check if the map is nil, not if its length is > 0. if src.Metadata != nil { meta := make(map[string]string, len(src.Metadata)) for k, v := range src.Metadata { meta[k] = v } dst.Metadata = meta } return &dst }` Suggestion importance[1-10]: 10 __ Why: This suggestion is critically important as it correctly identifies that the PR is a massive, multi-feature submission that completely fails to address the specific bug described in the linked ticket.	High
Possible issue	Fix parameter counting logic bug Fix a logic flaw in `count_debug_binds_list` that causes it to miscount parameters by removing an incorrect reset of the `in_item` flag. rust/srql/src/query/mod.rs [363-457] #[cfg(any(test, debug_assertions))] fn count_debug_binds_list(binds: &str) -> Option<usize> { let bytes = binds.as_bytes(); let mut i = 0usize; while i < bytes.len() && bytes[i].is_ascii_whitespace() { i += 1; } if i >= bytes.len() \|\| bytes[i] != b'[' { return None; } let mut bracket_depth = 0i32; let mut paren_depth = 0i32; let mut brace_depth = 0i32; let mut in_string = false; let mut escape = false; let mut in_item = false; let mut count = 0usize; for &b in bytes[i..].iter() { if in_string { if escape { escape = false; continue; } if b == b'\\' { escape = true; continue; } if b == b'"' { in_string = false; } continue; } match b { b'"' => { if bracket_depth == 1 && !in_item && brace_depth == 0 && paren_depth == 0 { in_item = true; } in_string = true; } b'[' => { if bracket_depth == 1 && !in_item && brace_depth == 0 && paren_depth == 0 { in_item = true; } bracket_depth += 1; - if bracket_depth == 1 { - in_item = false; - } } b']' => { if bracket_depth == 1 && in_item { count += 1; in_item = false; } bracket_depth -= 1; if bracket_depth <= 0 { break; } } b'{' => { if bracket_depth == 1 && !in_item && brace_depth == 0 && paren_depth == 0 { in_item = true; } brace_depth += 1 } b'}' => brace_depth -= 1, b'(' => { if bracket_depth == 1 && !in_item && brace_depth == 0 && paren_depth == 0 { in_item = true; } paren_depth += 1 } b')' => paren_depth -= 1, b',' => { if bracket_depth == 1 && brace_depth == 0 && paren_depth == 0 && in_item { count += 1; in_item = false; } } b if b.is_ascii_whitespace() => {} _ => { if bracket_depth == 1 && !in_item { in_item = true; } } } } Some(count) } Suggestion importance[1-10]: 8 __ Why: This suggestion correctly identifies a subtle but critical bug in a new debug utility function that would cause incorrect parameter counting and lead to failing tests, and it provides the correct one-line fix.	Medium
	Fix deep copy logic bug In `DeepCopyHostResult`, prevent appending to `dst.PortResults` while iterating `src.PortMap` to ensure the `PortResults` slice is a true deep copy of the source and not altered by the map's contents. pkg/models/sweep.go [203-233] if src.PortMap != nil { dst.PortMap = make(map[int]*PortResult, len(src.PortMap)) for port, pr := range src.PortMap { if pr == nil { dst.PortMap[port] = nil continue } + // Prefer the copy that came from the PortResults slice to maintain referential integrity. if prCopy, ok := copiedPortResults[pr]; ok { dst.PortMap[port] = prCopy continue } - if prCopy, ok := copiedByPort[port]; ok { - dst.PortMap[port] = prCopy - continue - } - + // If the port result was only in the map, it still needs to be copied. + // However, we should not append it to dst.PortResults, as that would + // alter the slice which is meant to be a copy of src.PortResults. prCopy := &PortResult{ Port: pr.Port, Available: pr.Available, RespTime: pr.RespTime, Service: pr.Service, } - - copiedPortResults[pr] = prCopy - copiedByPort[port] = prCopy dst.PortMap[port] = prCopy - dst.PortResults = append(dst.PortResults, prCopy) } } Suggestion importance[1-10]: 7 __ Why: The suggestion correctly identifies a bug where `dst.PortResults` is mutated while iterating `src.PortMap`, which is incorrect for a deep copy function. The fix prevents this mutation, ensuring the copied `PortResults` slice is a faithful snapshot of the source.	Medium
	Fix error message formatting Correct the formatting string in the `anyhow!` macro to properly display the `expected` and `current` bind counts in the error message. rust/srql/src/query/mod.rs [302-304] `return Err(ServiceError::Internal(anyhow::anyhow!( - "sql expects {expected} binds but {current} were collected" + "sql expects {} binds but {} were collected", + expected, + current )));` Suggestion importance[1-10]: 5 __ Why: The suggestion correctly fixes a formatting error in a debug assertion message, which improves debuggability by ensuring the error message is rendered correctly.	Low
	Fix bind count mismatch formatting Correct the formatting string in the `anyhow!` macro to properly display `bind_count` and `params.len()` in the error message. rust/srql/src/query/devices.rs [76-80] `if bind_count != params.len() { return Err(ServiceError::Internal(anyhow::anyhow!( - "bind count mismatch (diesel {bind_count} vs params {})", + "bind count mismatch (diesel {} vs params {})", + bind_count, params.len() ))); }` Suggestion importance[1-10]: 5 __ Why: The suggestion correctly fixes a formatting error in a debug assertion message, which improves debuggability by ensuring the error message is rendered correctly.	Low
	Correct error message formatting Correct the formatting string in the `anyhow!` macro to properly display `bind_count` and `params.len()` in the error message. rust/srql/src/query/services.rs [71-75] `if bind_count != params.len() { return Err(ServiceError::Internal(anyhow::anyhow!( - "bind count mismatch (diesel {bind_count} vs params {})", + "bind count mismatch (diesel {} vs params {})", + bind_count, params.len() ))); }` Suggestion importance[1-10]: 5 __ Why: The suggestion correctly fixes a formatting error in a debug assertion message, which improves debuggability by ensuring the error message is rendered correctly.	Low
General	Improve inconsistent boolean value parsing Expand boolean parsing for `available` and `is_available` to also accept "yes" as a true value, improving consistency with other modules. rust/srql/src/query/device_updates.rs [192-197] `"available" \| "is_available" => { let value = filter.value.as_scalar()?.to_lowercase(); - let bool_val = value == "true" \|\| value == "1"; + let bool_val = match value.as_str() { + "true" \| "1" \| "yes" => true, + _ => false, + }; params.push(BindParam::Bool(bool_val)); Ok(()) }` Suggestion importance[1-10]: 4 __ Why: The suggestion correctly identifies an inconsistency in boolean string parsing compared to other parts of the codebase. Applying this change improves consistency and user experience, though a better change would be to use the shared `parse_bool` function.	Low

Imported GitHub PR comment. Original author: @qodo-code-review[bot] Original URL: https://github.com/carverauto/serviceradar/pull/2163#issuecomment-3663002544 Original created: 2025-12-17T00:15:27Z --- ## PR Code Suggestions ✨  Latest suggestions up to 19d46a5 <table><thead><tr><td>Category</td><td align=left>Suggestion                                                                                                                                    </td><td align=center>Impact</td></tr><tbody><tr><td rowspan=2>Possible issue</td> <td> <details><summary>Strengthen slice aliasing regression test</summary> ___ **Strengthen the slice aliasing test by adding assertions to verify that appending to a cloned slice does not affect the original record or other clones.** [pkg/registry/device_store_test.go [108-136]](https://github.com/carverauto/serviceradar/pull/2163/files#diff-8768ba941333f15376041e9abc2080bc00a813c32cb2cef78dac197e346644e3R108-R136) ```diff func TestCloneDeviceRecord_EmptySlicesWithCapacityDoNotAlias(t *testing.T) { original := &DeviceRecord{ DeviceID: testDeviceID1, IP: "10.0.0.1", DiscoverySources: make([]string, 0, 1), Capabilities: make([]string, 0, 1), } clone1 := cloneDeviceRecord(original) if clone1 == nil { t.Fatalf("expected clone to be non-nil") } + clone1.DiscoverySources = append(clone1.DiscoverySources, "a") clone1.Capabilities = append(clone1.Capabilities, "icmp") + + // Appending to a clone must not mutate the original. + if len(original.DiscoverySources) != 0 { + t.Fatalf("expected original discovery sources to remain empty, got %#v", original.DiscoverySources) + } + if len(original.Capabilities) != 0 { + t.Fatalf("expected original capabilities to remain empty, got %#v", original.Capabilities) + } clone2 := cloneDeviceRecord(original) if clone2 == nil { t.Fatalf("expected second clone to be non-nil") } + clone2.DiscoverySources = append(clone2.DiscoverySources, "b") clone2.Capabilities = append(clone2.Capabilities, "snmp") - if got := clone1.DiscoverySources[0]; got != "a" { - t.Fatalf("expected clone1 discovery source to remain %q, got %q", "a", got) + // Mutating clone2 must not affect clone1. + if got := clone1.DiscoverySources; len(got) != 1 || got[0] != "a" { + t.Fatalf("expected clone1 discovery sources to remain [\"a\"], got %#v", got) } - if got := clone1.Capabilities[0]; got != "icmp" { - t.Fatalf("expected clone1 capability to remain %q, got %q", "icmp", got) + if got := clone1.Capabilities; len(got) != 1 || got[0] != "icmp" { + t.Fatalf("expected clone1 capabilities to remain [\"icmp\"], got %#v", got) + } + + // Sanity-check clone2 values. + if got := clone2.DiscoverySources; len(got) != 1 || got[0] != "b" { + t.Fatalf("expected clone2 discovery sources to be [\"b\"], got %#v", got) + } + if got := clone2.Capabilities; len(got) != 1 || got[0] != "snmp" { + t.Fatalf("expected clone2 capabilities to be [\"snmp\"], got %#v", got) } } ``` - [ ] **Apply / Chat**  <details><summary>Suggestion importance[1-10]: 7</summary> __ Why: The suggestion correctly points out a significant weakness in the new test case, as it doesn't verify the most critical aspects of the fix: that the original object is not mutated and that clones are isolated from each other. Applying this suggestion makes the regression test substantially more robust. </details></details></td><td align=center>Medium </td></tr><tr><td> <details><summary>Make metadata cloning test stricter</summary> ___ **Make the metadata cloning test stricter by adding assertions to ensure cloned maps are not nil and to verify that mutations on one clone do not affect another.** [pkg/registry/device_store_test.go [81-106]](https://github.com/carverauto/serviceradar/pull/2163/files#diff-8768ba941333f15376041e9abc2080bc00a813c32cb2cef78dac197e346644e3R81-R106) ```diff func TestCloneDeviceRecord_EmptyMetadataMapDoesNotAlias(t *testing.T) { original := &DeviceRecord{ DeviceID: testDeviceID1, IP: "10.0.0.1", Metadata: map[string]string{}, } clone1 := cloneDeviceRecord(original) if clone1 == nil { t.Fatalf("expected clone to be non-nil") + } + if clone1.Metadata == nil { + t.Fatalf("expected clone metadata to be non-nil") } clone1.Metadata["k"] = "v1" if len(original.Metadata) != 0 { t.Fatalf("expected original metadata to remain empty, got %#v", original.Metadata) } clone2 := cloneDeviceRecord(original) if clone2 == nil { t.Fatalf("expected second clone to be non-nil") } + if clone2.Metadata == nil { + t.Fatalf("expected second clone metadata to be non-nil") + } + if _, ok := clone2.Metadata["k"]; ok { t.Fatalf("expected second clone metadata to not include prior clone mutation, got %#v", clone2.Metadata) } + + clone2.Metadata["k2"] = "v2" + if _, ok := clone1.Metadata["k2"]; ok { + t.Fatalf("expected clone1 metadata to not include clone2 mutation, got %#v", clone1.Metadata) + } } ``` - [ ] **Apply / Chat**  <details><summary>Suggestion importance[1-10]: 6</summary> __ Why: The suggestion improves the test's robustness by adding a check for non-nil maps to prevent potential panics and by adding a symmetric check for clone-to-clone isolation, making the test more thorough. </details></details></td><td align=center>Low </td></tr> <tr><td align="center" colspan="2"> - [ ] More  </td><td></td></tr></tbody></table> ___ #### Previous suggestions <details><summary>Suggestions up to commit 990a438</summary> <table><thead><tr><td>Category</td><td align=left>Suggestion                                                                                                                                    </td><td align=center>Impact</td></tr><tbody><tr><td rowspan=1>High-level</td> <td> <details><summary>PR contains many unrelated changes</summary> ___ **The PR is a large, monolithic submission containing many unrelated features and does not include the specific bug fix for <code>cloneDeviceRecord</code> that is described in the linked ticket.** ### Solution Walkthrough: #### Before: ```pseudocode // The PR is expected to fix a bug in `cloneDeviceRecord` // as described in the ticket. func cloneDeviceRecord(src *DeviceRecord) *DeviceRecord { // ... // BUG: This condition is false for empty, non-nil maps. if len(src.Metadata) > 0 { meta := make(map[string]string, len(src.Metadata)) for k, v := range src.Metadata { meta[k] = v } dst.Metadata = meta } return &dst } ``` #### After: ```pseudocode // The PR should have contained this fix, but does not. func cloneDeviceRecord(src *DeviceRecord) *DeviceRecord { // ... // FIX: Check if the map is nil, not if its length is > 0. if src.Metadata != nil { meta := make(map[string]string, len(src.Metadata)) for k, v := range src.Metadata { meta[k] = v } dst.Metadata = meta } return &dst } ``` <details><summary>Suggestion importance[1-10]: 10</summary> __ Why: This suggestion is critically important as it correctly identifies that the PR is a massive, multi-feature submission that completely fails to address the specific bug described in the linked ticket. </details></details></td><td align=center>High </td></tr><tr><td rowspan=5>Possible issue</td> <td> <details><summary>Fix parameter counting logic bug</summary> ___ **Fix a logic flaw in <code>count_debug_binds_list</code> that causes it to miscount parameters by removing an incorrect reset of the <code>in_item</code> flag.** [rust/srql/src/query/mod.rs [363-457]](https://github.com/carverauto/serviceradar/pull/2163/files#diff-393e3fa3d0e41741834cd7cd398a06111ab7b141ae6caca7a5dcc0e036172491R363-R457) ```diff #[cfg(any(test, debug_assertions))] fn count_debug_binds_list(binds: &str) -> Option<usize> { let bytes = binds.as_bytes(); let mut i = 0usize; while i < bytes.len() && bytes[i].is_ascii_whitespace() { i += 1; } if i >= bytes.len() || bytes[i] != b'[' { return None; } let mut bracket_depth = 0i32; let mut paren_depth = 0i32; let mut brace_depth = 0i32; let mut in_string = false; let mut escape = false; let mut in_item = false; let mut count = 0usize; for &b in bytes[i..].iter() { if in_string { if escape { escape = false; continue; } if b == b'\\' { escape = true; continue; } if b == b'"' { in_string = false; } continue; } match b { b'"' => { if bracket_depth == 1 && !in_item && brace_depth == 0 && paren_depth == 0 { in_item = true; } in_string = true; } b'[' => { if bracket_depth == 1 && !in_item && brace_depth == 0 && paren_depth == 0 { in_item = true; } bracket_depth += 1; - if bracket_depth == 1 { - in_item = false; - } } b']' => { if bracket_depth == 1 && in_item { count += 1; in_item = false; } bracket_depth -= 1; if bracket_depth <= 0 { break; } } b'{' => { if bracket_depth == 1 && !in_item && brace_depth == 0 && paren_depth == 0 { in_item = true; } brace_depth += 1 } b'}' => brace_depth -= 1, b'(' => { if bracket_depth == 1 && !in_item && brace_depth == 0 && paren_depth == 0 { in_item = true; } paren_depth += 1 } b')' => paren_depth -= 1, b',' => { if bracket_depth == 1 && brace_depth == 0 && paren_depth == 0 && in_item { count += 1; in_item = false; } } b if b.is_ascii_whitespace() => {} _ => { if bracket_depth == 1 && !in_item { in_item = true; } } } } Some(count) } ``` <details><summary>Suggestion importance[1-10]: 8</summary> __ Why: This suggestion correctly identifies a subtle but critical bug in a new debug utility function that would cause incorrect parameter counting and lead to failing tests, and it provides the correct one-line fix. </details></details></td><td align=center>Medium </td></tr><tr><td> <details><summary>Fix deep copy logic bug</summary> ___ **In <code>DeepCopyHostResult</code>, prevent appending to <code>dst.PortResults</code> while iterating <code>src.PortMap</code> to ensure the <code>PortResults</code> slice is a true deep copy of the source and not altered by the map's contents.** [pkg/models/sweep.go [203-233]](https://github.com/carverauto/serviceradar/pull/2163/files#diff-3cc7dd0e748c9f77be9e384fed2703ab77375716524b70860153b6a1abae27caR203-R233) ```diff if src.PortMap != nil { dst.PortMap = make(map[int]*PortResult, len(src.PortMap)) for port, pr := range src.PortMap { if pr == nil { dst.PortMap[port] = nil continue } + // Prefer the copy that came from the PortResults slice to maintain referential integrity. if prCopy, ok := copiedPortResults[pr]; ok { dst.PortMap[port] = prCopy continue } - if prCopy, ok := copiedByPort[port]; ok { - dst.PortMap[port] = prCopy - continue - } - + // If the port result was only in the map, it still needs to be copied. + // However, we should not append it to dst.PortResults, as that would + // alter the slice which is meant to be a copy of src.PortResults. prCopy := &PortResult{ Port: pr.Port, Available: pr.Available, RespTime: pr.RespTime, Service: pr.Service, } - - copiedPortResults[pr] = prCopy - copiedByPort[port] = prCopy dst.PortMap[port] = prCopy - dst.PortResults = append(dst.PortResults, prCopy) } } ``` <details><summary>Suggestion importance[1-10]: 7</summary> __ Why: The suggestion correctly identifies a bug where `dst.PortResults` is mutated while iterating `src.PortMap`, which is incorrect for a deep copy function. The fix prevents this mutation, ensuring the copied `PortResults` slice is a faithful snapshot of the source. </details></details></td><td align=center>Medium </td></tr><tr><td> <details><summary>Fix error message formatting</summary> ___ **Correct the formatting string in the <code>anyhow!</code> macro to properly display the <code>expected</code> and <code>current</code> bind counts in the error message.** [rust/srql/src/query/mod.rs [302-304]](https://github.com/carverauto/serviceradar/pull/2163/files#diff-393e3fa3d0e41741834cd7cd398a06111ab7b141ae6caca7a5dcc0e036172491R302-R304) ```diff return Err(ServiceError::Internal(anyhow::anyhow!( - "sql expects {expected} binds but {current} were collected" + "sql expects {} binds but {} were collected", + expected, + current ))); ``` <details><summary>Suggestion importance[1-10]: 5</summary> __ Why: The suggestion correctly fixes a formatting error in a debug assertion message, which improves debuggability by ensuring the error message is rendered correctly. </details></details></td><td align=center>Low </td></tr><tr><td> <details><summary>Fix bind count mismatch formatting</summary> ___ **Correct the formatting string in the <code>anyhow!</code> macro to properly display <code>bind_count</code> and <code>params.len()</code> in the error message.** [rust/srql/src/query/devices.rs [76-80]](https://github.com/carverauto/serviceradar/pull/2163/files#diff-3202f22fff6863ed7848a129c49e2323322462b379d896d3fca2e59aa6f7b4c5R76-R80) ```diff if bind_count != params.len() { return Err(ServiceError::Internal(anyhow::anyhow!( - "bind count mismatch (diesel {bind_count} vs params {})", + "bind count mismatch (diesel {} vs params {})", + bind_count, params.len() ))); } ``` <details><summary>Suggestion importance[1-10]: 5</summary> __ Why: The suggestion correctly fixes a formatting error in a debug assertion message, which improves debuggability by ensuring the error message is rendered correctly. </details></details></td><td align=center>Low </td></tr><tr><td> <details><summary>Correct error message formatting</summary> ___ **Correct the formatting string in the <code>anyhow!</code> macro to properly display <code>bind_count</code> and <code>params.len()</code> in the error message.** [rust/srql/src/query/services.rs [71-75]](https://github.com/carverauto/serviceradar/pull/2163/files#diff-ad5e7e1a352406ed473a765de4856625f1bede590e7e2c724163bcd1e7b313e9R71-R75) ```diff if bind_count != params.len() { return Err(ServiceError::Internal(anyhow::anyhow!( - "bind count mismatch (diesel {bind_count} vs params {})", + "bind count mismatch (diesel {} vs params {})", + bind_count, params.len() ))); } ``` <details><summary>Suggestion importance[1-10]: 5</summary> __ Why: The suggestion correctly fixes a formatting error in a debug assertion message, which improves debuggability by ensuring the error message is rendered correctly. </details></details></td><td align=center>Low </td></tr><tr><td rowspan=1>General</td> <td> <details><summary>Improve inconsistent boolean value parsing</summary> ___ **Expand boolean parsing for <code>available</code> and <code>is_available</code> to also accept "yes" as a true value, improving consistency with other modules.** [rust/srql/src/query/device_updates.rs [192-197]](https://github.com/carverauto/serviceradar/pull/2163/files#diff-77b99f0ef409a6a3e4ecc65800ae841231a82b27a64e8dc1d4667db6698a539dR192-R197) ```diff "available" | "is_available" => { let value = filter.value.as_scalar()?.to_lowercase(); - let bool_val = value == "true" || value == "1"; + let bool_val = match value.as_str() { + "true" | "1" | "yes" => true, + _ => false, + }; params.push(BindParam::Bool(bool_val)); Ok(()) } ``` <details><summary>Suggestion importance[1-10]: 4</summary> __ Why: The suggestion correctly identifies an inconsistency in boolean string parsing compared to other parts of the codebase. Applying this change improves consistency and user experience, though a better change would be to use the shared `parse_bool` function. </details></details></td><td align=center>Low </td></tr> <tr><td align="center" colspan="2">  </td><td></td></tr></tbody></table> </details>

qodo-code-review[bot] commented

2025-12-17 00:16:18 +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/2163#issuecomment-3663004427
Original created: 2025-12-17T00:16:18Z

You are nearing your monthly Qodo Merge usage quota. For more information, please visit here.

PR Code Suggestions ✨

Explore these optional code suggestions:

Category	Suggestion	Impact
Security	Improve Cypher query security check Refactor the `ensure_read_only` function to correctly parse Cypher queries, ignoring keywords within string literals and comments to prevent false positives. rust/srql/src/query/graph_cypher.rs [104-126] fn ensure_read_only(raw: &str) -> Result<()> { let lower = raw.to_lowercase(); if lower.contains(';') { return Err(ServiceError::InvalidRequest( "cypher queries must not contain ';'".into(), )); } - for keyword in [ - "create", "merge", "set", "delete", "detach", "remove", "drop", "call", - ] { - if lower - .split(\|c: char\| !c.is_ascii_alphanumeric() && c != '_') - .any(\|token\| token == keyword) - { - return Err(ServiceError::InvalidRequest(format!( - "cypher queries must be read-only (found '{keyword}')" - ))); + let mut in_single_quote = false; + let mut in_double_quote = false; + let mut in_line_comment = false; + let mut in_block_comment = false; + let mut escape = false; + + let mut word_buffer = String::new(); + + let mut chars = lower.chars().peekable(); + + while let Some(c) = chars.next() { + if in_line_comment { + if c == '\n' { + in_line_comment = false; + } + continue; + } + + if in_block_comment { + if c == '' && chars.peek() == Some(&'/') { + chars.next(); // consume '/' + in_block_comment = false; + } + continue; + } + + if escape { + escape = false; + } else if c == '\\' { + escape = true; + } else if c == '\'' && !in_double_quote { + in_single_quote = !in_single_quote; + } else if c == '"' && !in_single_quote { + in_double_quote = !in_double_quote; + } + + if in_single_quote \|\| in_double_quote { + word_buffer.clear(); + continue; + } + + if c == '/' && chars.peek() == Some(&'/') { + chars.next(); // consume '/' + in_line_comment = true; + word_buffer.clear(); + continue; + } + + if c == '/' && chars.peek() == Some(&'') { + chars.next(); // consume '*' + in_block_comment = true; + word_buffer.clear(); + continue; + } + + if c.is_ascii_alphanumeric() { + word_buffer.push(c); + } else { + if !word_buffer.is_empty() { + match word_buffer.as_str() { + "create" \| "merge" \| "set" \| "delete" \| "detach" \| "remove" \| "drop" \| "call" => { + return Err(ServiceError::InvalidRequest(format!( + "cypher queries must be read-only (found '{}')", + word_buffer + ))); + } + _ => {} + } + word_buffer.clear(); + } + } + } + + if !word_buffer.is_empty() { + match word_buffer.as_str() { + "create" \| "merge" \| "set" \| "delete" \| "detach" \| "remove" \| "drop" \| "call" => { + return Err(ServiceError::InvalidRequest(format!( + "cypher queries must be read-only (found '{}')", + word_buffer + ))); + } + _ => {} } } Ok(()) } `[To ensure code accuracy, apply this suggestion manually]` Suggestion importance[1-10]: 8 __ Why: The suggestion correctly identifies a bug in the `ensure_read_only` function that would reject valid queries, and provides a more robust implementation that correctly handles string literals and comments.	Medium
Possible issue	Avoid placeholder replacement within strings Update `rewrite_placeholders` to avoid replacing `?` characters that appear inside string literals, preventing the generation of incorrect SQL. rust/srql/src/query/downsample.rs [508-521] `fn rewrite_placeholders(sql: &str) -> String { let mut result = String::with_capacity(sql.len()); let mut index = 1; - for ch in sql.chars() { - if ch == '?' { + let mut in_string = false; + let mut chars = sql.chars().peekable(); + + while let Some(ch) = chars.next() { + if ch == '\'' { + in_string = !in_string; + result.push(ch); + continue; + } + + if ch == '?' && !in_string { result.push('$'); result.push_str(&index.to_string()); index += 1; } else { result.push(ch); } } result }` `[To ensure code accuracy, apply this suggestion manually]` Suggestion importance[1-10]: 7 __ Why: The suggestion correctly identifies a bug where `?` inside string literals would be incorrectly replaced, and provides a fix that makes the function aware of string context, preventing invalid SQL generation.	Medium
	Fix bug in debug bind counter Fix a bug in the `count_debug_binds_list` function to correctly count all items in the debug output list. rust/srql/src/query/mod.rs [363-457] #[cfg(any(test, debug_assertions))] fn count_debug_binds_list(binds: &str) -> Option<usize> { - let bytes = binds.as_bytes(); - let mut i = 0usize; - while i < bytes.len() && bytes[i].is_ascii_whitespace() { - i += 1; + let mut bytes = binds.as_bytes().iter().peekable(); + + // Skip whitespace + while let Some(&&b) = bytes.peek() { + if b.is_ascii_whitespace() { + bytes.next(); + } else { + break; + } } - if i >= bytes.len() \|\| bytes[i] != b'[' { + if bytes.next() != Some(&b'[') { return None; } - let mut bracket_depth = 0i32; - let mut paren_depth = 0i32; - let mut brace_depth = 0i32; + let mut bracket_depth = 1i32; let mut in_string = false; let mut escape = false; - let mut in_item = false; let mut count = 0usize; + let mut found_item = false; - for &b in bytes[i..].iter() { + for &b in bytes { + if escape { + escape = false; + continue; + } + + if b == b'\\' { + escape = true; + continue; + } + + if b == b'"' { + in_string = !in_string; + } + if in_string { - if escape { - escape = false; - continue; - } - - if b == b'\\' { - escape = true; - continue; - } - - if b == b'"' { - in_string = false; - } - continue; } match b { - b'"' => { - if bracket_depth == 1 && !in_item && brace_depth == 0 && paren_depth == 0 { - in_item = true; - } - in_string = true; - } - b'[' => { - if bracket_depth == 1 && !in_item && brace_depth == 0 && paren_depth == 0 { - in_item = true; - } - bracket_depth += 1; - if bracket_depth == 1 { - in_item = false; + b'[' => bracket_depth += 1, + b']' => { + bracket_depth -= 1; + if bracket_depth == 0 { + if found_item { + count += 1; + } + return Some(count); } } - b']' => { - if bracket_depth == 1 && in_item { + b',' if bracket_depth == 1 => { + if found_item { count += 1; - in_item = false; - } - bracket_depth -= 1; - if bracket_depth <= 0 { - break; + found_item = false; } } - b'{' => { - if bracket_depth == 1 && !in_item && brace_depth == 0 && paren_depth == 0 { - in_item = true; - } - brace_depth += 1 + b if !b.is_ascii_whitespace() && bracket_depth == 1 => { + found_item = true; } - b'}' => brace_depth -= 1, - b'(' => { - if bracket_depth == 1 && !in_item && brace_depth == 0 && paren_depth == 0 { - in_item = true; - } - paren_depth += 1 - } - b')' => paren_depth -= 1, - b',' => { - if bracket_depth == 1 && brace_depth == 0 && paren_depth == 0 && in_item { - count += 1; - in_item = false; - } - } - b if b.is_ascii_whitespace() => {} - _ => { - if bracket_depth == 1 && !in_item { - in_item = true; - } - } + _ => {} } } - Some(count) + None // Unmatched bracket } `[To ensure code accuracy, apply this suggestion manually]` Suggestion importance[1-10]: 7 __ Why: The suggestion correctly identifies a bug in the debug-only helper function `count_debug_binds_list` and provides a corrected implementation, improving the reliability of debug assertions.	Medium
	Handle non-UTF-8 SNMP strings safely Validate that the SNMP `ObjectDescription` is a printable ASCII string before converting; otherwise, return a hex representation to prevent data corruption from invalid UTF-8 sequences. pkg/checker/snmp/client.go [273-280] `func convertObjectDescription(variable gosnmp.SnmpPDU) (interface{}, error) { bytes, ok := variable.Value.([]byte) if !ok { return nil, fmt.Errorf("%w: ObjectDescription expected []byte, got %T", ErrSNMPConvert, variable.Value) } + for _, b := range bytes { + if b < 32 \|\| b > 126 { + // Not a printable ASCII string, return as hex + return fmt.Sprintf("0x%x", bytes), nil + } + } + return string(bytes), nil }` `[To ensure code accuracy, apply this suggestion manually]` Suggestion importance[1-10]: 6 __ Why: The suggestion correctly identifies that SNMP strings may not be valid UTF-8 and proposes a robust fallback to hex encoding, which prevents potential data corruption.	Low
General	Populate returned filter map Populate the `responseFilters` map with the filters that have been applied to the query. pkg/mcp/builder.go [178-182] `if value, exists := rawArgs[jsonField]; exists && value != nil { if strValue, ok := value.(string); ok && strValue != "" { additionalFilters = append(additionalFilters, fmt.Sprintf("%s = %s", sqlField, binds.Bind(strValue))) + responseFilters[jsonField] = strValue } }` `[To ensure code accuracy, apply this suggestion manually]` Suggestion importance[1-10]: 7 __ Why: The suggestion correctly identifies that the `responseFilters` map is not being populated, which is a bug that prevents applied filters from being reflected in the API response.	Medium
	Check context cancellation Add a check for context cancellation inside the row processing loop to allow for early termination. pkg/db/cnpg_observability.go [237-244] `for rowIndex := 0; rowIndex < rowCount; rowIndex++ { + select { + case <-ctx.Done(): + return ctx.Err() + default: + } ts := timestampAt(rowIndex) if ts.IsZero() { ts = now } queueRow(batch, query, rowIndex, ts.UTC()) }` `[To ensure code accuracy, apply this suggestion manually]` Suggestion importance[1-10]: 7 __ Why: The suggestion correctly points out the lack of context cancellation handling in a potentially long-running loop, improving the responsiveness and resource management of the application.	Medium
	Use standardized boolean parsing Replace custom boolean parsing logic with a standardized `parse_bool` helper to ensure consistent behavior and proper error handling for invalid inputs. rust/srql/src/query/device_updates.rs [192-197] `"available" \| "is_available" => { - let value = filter.value.as_scalar()?.to_lowercase(); - let bool_val = value == "true" \|\| value == "1"; + let bool_val = parse_bool(filter.value.as_scalar()?)?; params.push(BindParam::Bool(bool_val)); Ok(()) }` `[To ensure code accuracy, apply this suggestion manually]` Suggestion importance[1-10]: 5 __ Why: The suggestion correctly identifies inconsistent and overly permissive boolean parsing logic and proposes using a stricter, more robust approach, which improves code quality and consistency.	Low
More

Imported GitHub PR comment. Original author: @qodo-code-review[bot] Original URL: https://github.com/carverauto/serviceradar/pull/2163#issuecomment-3663004427 Original created: 2025-12-17T00:16:18Z --- _You are nearing your monthly Qodo Merge usage quota. For more information, please visit [here](https://qodo-merge-docs.qodo.ai/installation/qodo_merge/#cloud-users)._ ## 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=1>Security</td> <td> <details><summary>Improve Cypher query security check</summary> ___ **Refactor the <code>ensure_read_only</code> function to correctly parse Cypher queries, ignoring keywords within string literals and comments to prevent false positives.** [rust/srql/src/query/graph_cypher.rs [104-126]](https://github.com/carverauto/serviceradar/pull/2163/files#diff-378b0ec80e1b1221c347160a0dac36c82d082fa06766d47d18cf069220166309R104-R126) ```diff fn ensure_read_only(raw: &str) -> Result<()> { let lower = raw.to_lowercase(); if lower.contains(';') { return Err(ServiceError::InvalidRequest( "cypher queries must not contain ';'".into(), )); } - for keyword in [ - "create", "merge", "set", "delete", "detach", "remove", "drop", "call", - ] { - if lower - .split(|c: char| !c.is_ascii_alphanumeric() && c != '_') - .any(|token| token == keyword) - { - return Err(ServiceError::InvalidRequest(format!( - "cypher queries must be read-only (found '{keyword}')" - ))); + let mut in_single_quote = false; + let mut in_double_quote = false; + let mut in_line_comment = false; + let mut in_block_comment = false; + let mut escape = false; + + let mut word_buffer = String::new(); + + let mut chars = lower.chars().peekable(); + + while let Some(c) = chars.next() { + if in_line_comment { + if c == '\n' { + in_line_comment = false; + } + continue; + } + + if in_block_comment { + if c == '*' && chars.peek() == Some(&'/') { + chars.next(); // consume '/' + in_block_comment = false; + } + continue; + } + + if escape { + escape = false; + } else if c == '\\' { + escape = true; + } else if c == '\'' && !in_double_quote { + in_single_quote = !in_single_quote; + } else if c == '"' && !in_single_quote { + in_double_quote = !in_double_quote; + } + + if in_single_quote || in_double_quote { + word_buffer.clear(); + continue; + } + + if c == '/' && chars.peek() == Some(&'/') { + chars.next(); // consume '/' + in_line_comment = true; + word_buffer.clear(); + continue; + } + + if c == '/' && chars.peek() == Some(&'*') { + chars.next(); // consume '*' + in_block_comment = true; + word_buffer.clear(); + continue; + } + + if c.is_ascii_alphanumeric() { + word_buffer.push(c); + } else { + if !word_buffer.is_empty() { + match word_buffer.as_str() { + "create" | "merge" | "set" | "delete" | "detach" | "remove" | "drop" | "call" => { + return Err(ServiceError::InvalidRequest(format!( + "cypher queries must be read-only (found '{}')", + word_buffer + ))); + } + _ => {} + } + word_buffer.clear(); + } + } + } + + if !word_buffer.is_empty() { + match word_buffer.as_str() { + "create" | "merge" | "set" | "delete" | "detach" | "remove" | "drop" | "call" => { + return Err(ServiceError::InvalidRequest(format!( + "cypher queries must be read-only (found '{}')", + word_buffer + ))); + } + _ => {} } } Ok(()) } ``` `[To ensure code accuracy, apply this suggestion manually]` <details><summary>Suggestion importance[1-10]: 8</summary> __ Why: The suggestion correctly identifies a bug in the `ensure_read_only` function that would reject valid queries, and provides a more robust implementation that correctly handles string literals and comments. </details></details></td><td align=center>Medium </td></tr><tr><td rowspan=3>Possible issue</td> <td> <details><summary>Avoid placeholder replacement within strings</summary> ___ **Update <code>rewrite_placeholders</code> to avoid replacing <code>?</code> characters that appear inside string literals, preventing the generation of incorrect SQL.** [rust/srql/src/query/downsample.rs [508-521]](https://github.com/carverauto/serviceradar/pull/2163/files#diff-94f68b4684578afa112ab05ff903667b6cd902ad276e17c12af350078b300a6aR508-R521) ```diff fn rewrite_placeholders(sql: &str) -> String { let mut result = String::with_capacity(sql.len()); let mut index = 1; - for ch in sql.chars() { - if ch == '?' { + let mut in_string = false; + let mut chars = sql.chars().peekable(); + + while let Some(ch) = chars.next() { + if ch == '\'' { + in_string = !in_string; + result.push(ch); + continue; + } + + if ch == '?' && !in_string { result.push('$'); result.push_str(&index.to_string()); index += 1; } else { result.push(ch); } } result } ``` `[To ensure code accuracy, apply this suggestion manually]` <details><summary>Suggestion importance[1-10]: 7</summary> __ Why: The suggestion correctly identifies a bug where `?` inside string literals would be incorrectly replaced, and provides a fix that makes the function aware of string context, preventing invalid SQL generation. </details></details></td><td align=center>Medium </td></tr><tr><td> <details><summary>Fix bug in debug bind counter</summary> ___ **Fix a bug in the <code>count_debug_binds_list</code> function to correctly count all items in the debug output list.** [rust/srql/src/query/mod.rs [363-457]](https://github.com/carverauto/serviceradar/pull/2163/files#diff-393e3fa3d0e41741834cd7cd398a06111ab7b141ae6caca7a5dcc0e036172491R363-R457) ```diff #[cfg(any(test, debug_assertions))] fn count_debug_binds_list(binds: &str) -> Option<usize> { - let bytes = binds.as_bytes(); - let mut i = 0usize; - while i < bytes.len() && bytes[i].is_ascii_whitespace() { - i += 1; + let mut bytes = binds.as_bytes().iter().peekable(); + + // Skip whitespace + while let Some(&&b) = bytes.peek() { + if b.is_ascii_whitespace() { + bytes.next(); + } else { + break; + } } - if i >= bytes.len() || bytes[i] != b'[' { + if bytes.next() != Some(&b'[') { return None; } - let mut bracket_depth = 0i32; - let mut paren_depth = 0i32; - let mut brace_depth = 0i32; + let mut bracket_depth = 1i32; let mut in_string = false; let mut escape = false; - let mut in_item = false; let mut count = 0usize; + let mut found_item = false; - for &b in bytes[i..].iter() { + for &b in bytes { + if escape { + escape = false; + continue; + } + + if b == b'\\' { + escape = true; + continue; + } + + if b == b'"' { + in_string = !in_string; + } + if in_string { - if escape { - escape = false; - continue; - } - - if b == b'\\' { - escape = true; - continue; - } - - if b == b'"' { - in_string = false; - } - continue; } match b { - b'"' => { - if bracket_depth == 1 && !in_item && brace_depth == 0 && paren_depth == 0 { - in_item = true; - } - in_string = true; - } - b'[' => { - if bracket_depth == 1 && !in_item && brace_depth == 0 && paren_depth == 0 { - in_item = true; - } - bracket_depth += 1; - if bracket_depth == 1 { - in_item = false; + b'[' => bracket_depth += 1, + b']' => { + bracket_depth -= 1; + if bracket_depth == 0 { + if found_item { + count += 1; + } + return Some(count); } } - b']' => { - if bracket_depth == 1 && in_item { + b',' if bracket_depth == 1 => { + if found_item { count += 1; - in_item = false; - } - bracket_depth -= 1; - if bracket_depth <= 0 { - break; + found_item = false; } } - b'{' => { - if bracket_depth == 1 && !in_item && brace_depth == 0 && paren_depth == 0 { - in_item = true; - } - brace_depth += 1 + b if !b.is_ascii_whitespace() && bracket_depth == 1 => { + found_item = true; } - b'}' => brace_depth -= 1, - b'(' => { - if bracket_depth == 1 && !in_item && brace_depth == 0 && paren_depth == 0 { - in_item = true; - } - paren_depth += 1 - } - b')' => paren_depth -= 1, - b',' => { - if bracket_depth == 1 && brace_depth == 0 && paren_depth == 0 && in_item { - count += 1; - in_item = false; - } - } - b if b.is_ascii_whitespace() => {} - _ => { - if bracket_depth == 1 && !in_item { - in_item = true; - } - } + _ => {} } } - Some(count) + None // Unmatched bracket } ``` `[To ensure code accuracy, apply this suggestion manually]` <details><summary>Suggestion importance[1-10]: 7</summary> __ Why: The suggestion correctly identifies a bug in the debug-only helper function `count_debug_binds_list` and provides a corrected implementation, improving the reliability of debug assertions. </details></details></td><td align=center>Medium </td></tr><tr><td> <details><summary>Handle non-UTF-8 SNMP strings safely</summary> ___ **Validate that the SNMP <code>ObjectDescription</code> is a printable ASCII string before converting; otherwise, return a hex representation to prevent data corruption from invalid UTF-8 sequences.** [pkg/checker/snmp/client.go [273-280]](https://github.com/carverauto/serviceradar/pull/2163/files#diff-b5e92490b688495a040a2f6f5227dc83c46fc5e7ea59885f8285a3d6c868bd87R273-R280) ```diff func convertObjectDescription(variable gosnmp.SnmpPDU) (interface{}, error) { bytes, ok := variable.Value.([]byte) if !ok { return nil, fmt.Errorf("%w: ObjectDescription expected []byte, got %T", ErrSNMPConvert, variable.Value) } + for _, b := range bytes { + if b < 32 || b > 126 { + // Not a printable ASCII string, return as hex + return fmt.Sprintf("0x%x", bytes), nil + } + } + return string(bytes), nil } ``` `[To ensure code accuracy, apply this suggestion manually]` <details><summary>Suggestion importance[1-10]: 6</summary> __ Why: The suggestion correctly identifies that SNMP strings may not be valid UTF-8 and proposes a robust fallback to hex encoding, which prevents potential data corruption. </details></details></td><td align=center>Low </td></tr><tr><td rowspan=3>General</td> <td> <details><summary>Populate returned filter map</summary> ___ **Populate the <code>responseFilters</code> map with the filters that have been applied to the query.** [pkg/mcp/builder.go [178-182]](https://github.com/carverauto/serviceradar/pull/2163/files#diff-ee3c16d7d8882a3a65d28bfedf7f0a5e59698b00ae482bc5565b3314503d5db2R178-R182) ```diff if value, exists := rawArgs[jsonField]; exists && value != nil { if strValue, ok := value.(string); ok && strValue != "" { additionalFilters = append(additionalFilters, fmt.Sprintf("%s = %s", sqlField, binds.Bind(strValue))) + responseFilters[jsonField] = strValue } } ``` `[To ensure code accuracy, apply this suggestion manually]` <details><summary>Suggestion importance[1-10]: 7</summary> __ Why: The suggestion correctly identifies that the `responseFilters` map is not being populated, which is a bug that prevents applied filters from being reflected in the API response. </details></details></td><td align=center>Medium </td></tr><tr><td> <details><summary>Check context cancellation</summary> ___ **Add a check for context cancellation inside the row processing loop to allow for early termination.** [pkg/db/cnpg_observability.go [237-244]](https://github.com/carverauto/serviceradar/pull/2163/files#diff-abaae95adb6fa6d0241553f6d1b39ecff3dd6159db72babbe39dfb3cd231cd3dR237-R244) ```diff for rowIndex := 0; rowIndex < rowCount; rowIndex++ { + select { + case <-ctx.Done(): + return ctx.Err() + default: + } ts := timestampAt(rowIndex) if ts.IsZero() { ts = now } queueRow(batch, query, rowIndex, ts.UTC()) } ``` `[To ensure code accuracy, apply this suggestion manually]` <details><summary>Suggestion importance[1-10]: 7</summary> __ Why: The suggestion correctly points out the lack of context cancellation handling in a potentially long-running loop, improving the responsiveness and resource management of the application. </details></details></td><td align=center>Medium </td></tr><tr><td> <details><summary>Use standardized boolean parsing</summary> ___ **Replace custom boolean parsing logic with a standardized <code>parse_bool</code> helper to ensure consistent behavior and proper error handling for invalid inputs.** [rust/srql/src/query/device_updates.rs [192-197]](https://github.com/carverauto/serviceradar/pull/2163/files#diff-77b99f0ef409a6a3e4ecc65800ae841231a82b27a64e8dc1d4667db6698a539dR192-R197) ```diff "available" | "is_available" => { - let value = filter.value.as_scalar()?.to_lowercase(); - let bool_val = value == "true" || value == "1"; + let bool_val = parse_bool(filter.value.as_scalar()?)?; params.push(BindParam::Bool(bool_val)); Ok(()) } ``` `[To ensure code accuracy, apply this suggestion manually]` <details><summary>Suggestion importance[1-10]: 5</summary> __ Why: The suggestion correctly identifies inconsistent and overly permissive boolean parsing logic and proposes using a stricter, more robust approach, which improves code quality and consistency. </details></details></td><td align=center>Low </td></tr> <tr><td align="center" colspan="2"> - [ ] More  </td><td></td></tr></tbody></table>

qodo-code-review[bot] commented

2025-12-17 00:17:17 +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/2163#issuecomment-3663006861
Original created: 2025-12-17T00:17:17Z

Persistent suggestions updated to latest commit 19d46a5

Imported GitHub PR comment. Original author: @qodo-code-review[bot] Original URL: https://github.com/carverauto/serviceradar/pull/2163#issuecomment-3663006861 Original created: 2025-12-17T00:17:17Z --- **[Persistent suggestions](https://github.com/carverauto/serviceradar/pull/2163#issuecomment-3663002544)** updated to latest commit 19d46a5