Updates/systemactor module #2656
No reviewers
Labels
No labels
1week
2weeks
Failed compliance check
IP cameras
NATS
Possible security concern
Review effort 1/5
Review effort 2/5
Review effort 3/5
Review effort 4/5
Review effort 5/5
UI
aardvark
accessibility
amd64
api
arm64
auth
back-end
bgp
blog
bug
build
checkers
ci-cd
cleanup
cnpg
codex
core
dependencies
device-management
documentation
duplicate
dusk
ebpf
enhancement
eta 1d
eta 1hr
eta 3d
eta 3hr
feature
fieldsurvey
github_actions
go
good first issue
help wanted
invalid
javascript
k8s
log-collector
mapper
mtr
needs-triage
netflow
network-sweep
observability
oracle
otel
plug-in
proton
python
question
reddit
redhat
research
rperf
rperf-checker
rust
sdk
security
serviceradar-agent
serviceradar-agent-gateway
serviceradar-web
serviceradar-web-ng
siem
snmp
sysmon
topology
ubiquiti
wasm
wontfix
zen-engine
No milestone
No project
No assignees
1 participant
Notifications
Due date
No due date set.
Dependencies
No dependencies set.
Reference
carverauto/serviceradar!2656
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "refs/pull/2656/head"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Imported from GitHub pull request.
Original GitHub pull request: #2273
Original author: @mfreeman451
Original URL: https://github.com/carverauto/serviceradar/pull/2273
Original created: 2026-01-12T18:00:23Z
Original updated: 2026-01-12T18:21:53Z
Original head: carverauto/serviceradar:updates/systemactor-module
Original base: staging
Original merged: 2026-01-12T18:21:51Z 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:
Describe your changes
Issue ticket number and link
Code checklist before requesting a review
PR Type
Enhancement, Refactoring, Tests
Description
Introduced
SystemActormodule for background operations withfor_tenant/2andplatform/1functions to replace unsafeauthorize?: falsepatterns throughout the codebaseReplaced
authorize?: falsewith actor-based authorization across 30+ modules including event processors, workers, seeders, and infrastructure componentsAdded Credo configuration and custom check to detect and warn about
authorize?: falseusage, guiding developers to useSystemActorinsteadComprehensive code refactoring across event processors, workers, and core modules with extracted helper functions for improved readability and maintainability
Refactored complex functions into smaller, focused helper functions in modules like
stateful_alert_engine,stats_aggregator,sweep_results_ingestor, and many event processorsImproved code formatting with underscore separators in numeric literals and consistent alias ordering
Added comprehensive test suite for the new
SystemActormodule covering tenant-scoped and platform-wide operationsSimplified conditional logic by replacing
condwithif-else, usingEnum.empty?()instead oflength()checks, and extracting pattern matching into separate function clausesDiagram Walkthrough
File Walkthrough
48 files
stateful_alert_engine.ex
SystemActor integration and code refactoring for authorizationelixir/serviceradar_core/lib/serviceradar/observability/stateful_alert_engine.ex
authorize?: falsewithactor: actorusingSystemActor.for_tenant()for authorizationEnum.eachloops into dedicated helper functions(
process_log_rules,process_event_rules,maybe_process_log_rule,maybe_process_event_rule)(
log_matches?,event_matches?,fetch_attr)helper functions for record processing
stats_aggregator.ex
Code refactoring for improved readability and maintainabilityelixir/serviceradar_core/lib/serviceradar/core/stats_aggregator.ex
(
build_snapshot_from_devices,update_device_stats,device_active?)record_key,canonical_record?,upsert_canonical_record,upsert_fallback_record)(
maybe_increment_collectors,increment_capability,has_collector?)should_log_snapshot?,stats_changed?,meta_changed?,maybe_log_non_canonical)sweep_results_ingestor.ex
Code refactoring for better function decompositionelixir/serviceradar_core/lib/serviceradar/sweep_jobs/sweep_results_ingestor.ex
build_unknown_device_recordsandinsert_unknown_device_recordsfunctions(
result_available?,host_status,device_id_for_ip,build_host_record,response_time_ms,open_ports,parse_integer,valid_port?)(
result_ips_for_status,device_uids_for_ips,update_device_statuses,maybe_add_sweep_source)smaller, focused functions
alias_events.ex
Code refactoring for improved function decompositionelixir/serviceradar_core/lib/serviceradar/identity/alias_events.ex
from_metadatato use helper functions (build_alias_record,build_alias_maps,seed_service_aliases,seed_ip_aliases,update_alias_maps)update_service_aliasandupdate_ip_aliasfunctionsmaybe_putandmaybe_put_maphelperfunctions
build_device_alias_eventsand
alias_valuesfunctionssweep.ex
SystemActor integration and code refactoringelixir/serviceradar_core/lib/serviceradar/event_writer/processors/sweep.ex
SystemActoralias and integrated it for authorization instead ofauthorize?: falsebuild_rowsandinsert_sweep_rowsfunctions
process_execution_resultsfunction
update_availability,update_available_devices,update_unavailable_devicesfunctionsstatus_from_icmp,protocol_from_icmp)log_promotion.ex
SystemActor integration and code refactoringelixir/serviceradar_core/lib/serviceradar/observability/log_promotion.ex
authorize?: falsewithactor: actorusingSystemActor.for_tenant()for authorization@severity_text_mapmodule attribute for severity mappingevent_message,event_status_id,event_status,event_actor,event_log_name,event_log_provider,event_log_level,event_log_time,event_uids)update_alert_countsandmaybe_emit_alert_metricsfunctionsprovision_collector_worker.ex
SystemActor integration and code refactoringelixir/serviceradar_core/lib/serviceradar/edge/workers/provision_collector_worker.ex
authorize?: falsewithactor: actorusingSystemActor.for_tenant()andSystemActor.platform()for authorizationdecrypt_private_key,build_ca_data,generate_component_certfunctionsactorparameter instead ofauthorize?: falsedevice_lookup.ex
Code refactoring for improved function decompositionelixir/serviceradar_core/lib/serviceradar/identity/device_lookup.ex
fetch_cache_hitsandcache_db_resultsfunctionsnormalize_key_entryandmaybe_add_ip_hintfunctionscached_record_for_key,handle_lookup_miss,cache_lookup_resultfunctionspartition_matches?helper functionmaybe_record_for_ipfunctiononboarding_packages.ex
SystemActor integration and code formattingelixir/serviceradar_core/lib/serviceradar/edge/onboarding_packages.ex
SystemActoralias for authorizationauthorize?: falsewithactor: actorusingSystemActor.for_tenant()andSystemActor.platform()actor-based authorization
86400to86_400)template_seeder.ex
Replace authorize false with SystemActor and refactor seedingelixir/serviceradar_core/lib/serviceradar/observability/template_seeder.ex
SystemActor.platform/1for cross-tenant seeding operationsinstead of
authorize?: falseSystemActor.for_tenant/2for tenant-scoped seeding insteadof manual actor maps
seed_template_if_missing/5andcreate_template/4functionsseed_zen_template_if_missing/4and
create_zen_template/4functionsmaybe_rename_legacy_template/3,rename_template_if_missing/5, anddo_rename_template/6functionssync_ingestor_queue.ex
Replace authorize false with SystemActor in sync queueelixir/serviceradar_core/lib/serviceradar/inventory/sync_ingestor_queue.ex
SystemActor.for_tenant/2to replace manual actor mapconstruction
available_slots/2functionapply_ingestion_result/1function
with_sync_service/4andupdate_sync_source/7helper functionsauthorize?: falsewith proper actor-based authorizationsweep_compiler.ex
Replace authorize false with SystemActor in sweep compilerelixir/serviceradar_core/lib/serviceradar/agent_config/compilers/sweep_compiler.ex
SystemActor.for_tenant/2to replace manual actor mapconstruction
build_system_actor/1function in favor ofSystemActormoduleauthorize?: falsewith proper actor-based authorization inAsh queries
tenant.ex
Replace authorize false with SystemActor in tenant moduleelixir/serviceradar_core/lib/serviceradar/identity/tenant.ex
SystemActor.platform/1for platform-level operations (CAgeneration, tenant registration)
authorize?: falsewith actor-based authorization throughoutregistration, and bootstrap operations
functions
system_actor.ex
New SystemActor module for background operationselixir/serviceradar_core/lib/serviceradar/actors/system_actor.ex
for_tenant/2function for tenant-scoped systemactors
platform/1function for platform-wide systemactors
system_actor?/1predicate function to identify system actorsbenefits
zen_rule_seeder.ex
Replace authorize false with SystemActor in zen rule seederelixir/serviceradar_core/lib/serviceradar/observability/zen_rule_seeder.ex
SystemActor.platform/1for cross-tenant seeding operationsSystemActor.for_tenant/2for tenant-scoped seedingseed_rule_if_missing/4andcreate_rule/3functionsrename_legacy_rule/3,rename_rule_if_missing/4, anddo_rename_rule/6functionsauthorize?: falsewith proper actor-based authorizationagent_gateway_sync.ex
Replace authorize false with SystemActor in gateway syncelixir/serviceradar_core/lib/serviceradar/edge/agent_gateway_sync.ex
SystemActor.for_tenant/2to replace manual actor mapconstruction
system_actor/1function in favor ofSystemActormoduleauthorize?: falsewith proper actor-based authorization inAsh operations
sweep_monitor_worker.ex
Replace authorize false with SystemActor in sweep monitorelixir/serviceradar_core/lib/serviceradar/sweep_jobs/sweep_monitor_worker.ex
SystemActor.for_tenant/2for sweep monitoring operationscheck_group_schedule/4functionwith pattern matching
parse_interval_to_seconds/1intoparse_interval_string/1,unit_seconds/1, andparse_interval_fallback/1helpers86_400)operator_bootstrap.ex
Replace authorize false with SystemActor in operator bootstrapelixir/serviceradar_core/lib/serviceradar/nats/operator_bootstrap.ex
SystemActor.platform/1for platform-level NATS operationsauthorize?: falsewith proper actor-based authorizationhandle_tenants_needing_accounts/1andenqueue_tenant_account/1functions
oban_running?/0by removing try-rescue wrappercreate_account_worker.ex
Replace authorize false with SystemActor in account workerelixir/serviceradar_core/lib/serviceradar/nats/workers/create_account_worker.ex
SystemActor.platform/1to replace manual actor mapconstruction
authorize?: falsewith proper actor-based authorizationthroughout
oban_running?/0by removing try-rescue wrapperzen_rule_sync.ex
Replace authorize false with SystemActor in zen rule syncelixir/serviceradar_core/lib/serviceradar/observability/zen_rule_sync.ex
SystemActor.for_tenant/2for zen rule sync operationsactorfield to module struct for GenServer statesync_rule_with_logging/2function
sync_rule_with_actor/2functionauthorize?: falsewith proper actor-based authorizationnats_leaf_server.ex
Replace authorize false with SystemActor patternelixir/serviceradar_core/lib/serviceradar/edge/nats_leaf_server.ex
SystemActoralias and replacedauthorize?: falsewithactor-based authorization
ProvisionLeafWorkeralias to simplify module referencesparse_cert_expiry/1to use rescue clause instead oftry-catch block
:systemrole to perform alloperations
tenant_resolver.ex
Refactor to use SystemActor and extract helper functionselixir/serviceradar_core/lib/serviceradar/edge/tenant_resolver.ex
SystemActorandTenantCAGeneratoraliases for cleaner codeauthorize?: falsewith platform actor in tenant lookup and CAoperations
cowboy_cert/1andbandit_cert/1helper functionsidentity_reconciler.ex
Refactor device reconciliation with helper functionselixir/serviceradar_core/lib/serviceradar/inventory/identity_reconciler.ex
withstatement for betterreadability
resolve_fallback_device_id/3helper functionlookup_by_strong_identifiers/2conditional logiclookup_identifier/3helper to reduce nesting in identifierlookup loop
lookup_by_ip/2condition logic for claritylength(seeds) > 0withnot Enum.empty?(seeds)for idiomaticElixir
provision_leaf_worker.ex
Replace authorize false with SystemActor patternelixir/serviceradar_core/lib/serviceradar/edge/workers/provision_leaf_worker.ex
SystemActoralias for authorizationauthorize?: falsewith platform/tenant-specific actors inresource loading
get_tenant_ca/2to use tenant-scoped actor instead ofbypassing authorization
update_leaf_server_status/5to use actor-based authorizationhealth_tracker.ex
Replace authorize false with SystemActor patternelixir/serviceradar_core/lib/serviceradar/infrastructure/health_tracker.ex
SystemActoralias for authorizationauthorize?: falsewith tenant-scoped actors in all Ashoperations
build_summary/1helper function to reduce nesting insummary/1sync_ingestor.ex
Refactor with Enum.empty and helper functionselixir/serviceradar_core/lib/serviceradar/inventory/sync_ingestor.ex
length(list) > 0checks withEnum.empty?()for idiomaticElixir
bulk_lookup_identifiers/2andbulk_lookup_by_ip/2cached_device_id/2andexisting_device_id/3helper functionsauthorize_false_usage.ex
Add Credo check for authorize false usageelixir/serviceradar_core/lib/serviceradar/credo/check/warning/authorize_false_usage.ex
authorize?: falseusage inproduction code
SystemActor.for_tenant/2orSystemActor.platform/1insteadauthorize?: falserule_seeder.ex
Replace authorize false with SystemActor patternelixir/serviceradar_core/lib/serviceradar/observability/rule_seeder.ex
SystemActoralias for authorizationauthorize?: falsewith platform/tenant-scoped actorsseed_rule_if_missing/5andcreate_rule/4helper functionsclient.ex
Extract configuration helper functionselixir/serviceradar_core/lib/serviceradar/data_service/client.ex
config_value/5,config_value_int/5, andconfig_value_bool/5helper functions
||chains
tenant_worker.ex
Refactor tenant worker with helper functionselixir/serviceradar_core/lib/serviceradar/oban/tenant_worker.ex
execute_with_tenant/3to extract result handling logichandle_perform_result/4,maybe_on_success/3, andmaybe_on_failure/4helperstenant_registry.ex
Extract helper functions for registry operationselixir/serviceradar_core/lib/serviceradar/cluster/tenant_registry.ex
find_parent_supervisor/2,resolve_ancestor_pid/1, andmaybe_child_pid/2helperstenant_registries/1helper function for cleaner listcomprehension
event_publisher.ex
Refactor event messages and add SystemActorelixir/serviceradar_core/lib/serviceradar/infrastructure/event_publisher.ex
SystemActoralias for authorizationmessage_for_event/4into separate clauses for each eventtype
authorize?: falsewith platform actor in tenant lookuptenant_queues.ex
Replace authorize false and extract queue helperselixir/serviceradar_core/lib/serviceradar/oban/tenant_queues.ex
SystemActoralias for authorizationauthorize?: falsewith platform actor in tenant provisioningfetch_queue_stats/2,queue_stats/2,queue_counts/2,normalize_queue_counts/1, andempty_tenant_stats/1helpersassign_tenant_id.ex
Add aliases and extract tenant ID helperselixir/serviceradar_core/lib/serviceradar/changes/assign_tenant_id.ex
Ash.ChangesetandAsh.Resource.Infofor cleaner codeactor_tenant_id/1andchangeset_tenant_id/1helper functionsplatform_service_certificates.ex
Replace authorize false with SystemActor patternelixir/serviceradar_core/lib/serviceradar/edge/platform_service_certificates.ex
SystemActoralias for authorizationauthorize?: falsewith tenant-scoped actors throughout"system"to properSystemActor.for_tenant/2cluster_status.ex
Refactor cluster status with helper functionselixir/serviceradar_core/lib/serviceradar/cluster/cluster_status.ex
coordinator_health/0to use if-else instead of condfind_remote_coordinator/0andcoordinator_node?/1helperfunctions
get_local_health/0by removing try-catch wrapperconfig_server.ex
Replace authorize false with SystemActor patternelixir/serviceradar_core/lib/serviceradar/agent_config/config_server.ex
SystemActoralias for authorizationauthorize?: falsewith tenant-scoped actor in databaseloading
compile_cache_miss/6helper function for cleaner cachehandling
compile_zen_rule.ex
Extract zen rule compilation helperselixir/serviceradar_core/lib/serviceradar/observability/changes/compile_zen_rule.ex
apply_compiled_jdm/2,jdm_definition_present?/1, andatomic_payload/3helperswithstatementgenerator.ex
Replace authorize false and format numberselixir/serviceradar_core/lib/serviceradar/edge/tenant_ca/generator.ex
SystemActoralias for authorizationauthorize?: falsewith platform actor in tenant loading65537to65_537)reserved_tenant_slug.ex
Extract slug and platform flag helperselixir/serviceradar_core/lib/serviceradar/identity/validations/reserved_tenant_slug.ex
changeset_slug/1,data_slug/1,normalize_slug/1helpersplatform_flag_from_data/1helper functionalert.ex
Add module aliases and system actor policyelixir/serviceradar_core/lib/serviceradar/monitoring/alert.ex
:systemrole to perform alloperations
is_nil(resolved_at)instead of negation
collector_package.ex
Replace authorize false and add SystemActorelixir/serviceradar_core/lib/serviceradar/edge/collector_package.ex
SystemActorandPubSubaliasesauthorize?: falsewith tenant-scoped actors in credentialoperations
:systemrole to perform alloperations
health_check_registrar.ex
Extract service configuration helperselixir/serviceradar_core/lib/serviceradar/infrastructure/health_check_registrar.ex
map_service_config/3andbasic_service_config/4helperfunctions
construction
nats_credential.ex
Add PubSub alias and system actor policyelixir/serviceradar_core/lib/serviceradar/edge/nats_credential.ex
PubSubalias for cleaner code:systemrole to perform alloperations
agent_config_generator.ex
Reorder aliases and simplify error handlingelixir/serviceradar_core/lib/serviceradar/edge/agent_config_generator.ex
load_agent_checks/2functiondevice.ex
Replace authorize false with SystemActor patternelixir/serviceradar_core/lib/serviceradar/actors/device.ex
SystemActoralias for authorizationauthorize?: falsewith tenant-scoped actors in deviceoperations
length(state.events) > 0toEnum.empty?(state.events)foridiomatic Elixir
pipeline.ex
Refactor batcher determination with ruleselixir/serviceradar_core/lib/serviceradar/event_writer/pipeline.ex
determine_batcher/1to useEnum.find_value/3with rule listbatcher_rules/0helper function for cleaner subject matchingspiffe.ex
Refactor SPIFFE verification with helperselixir/serviceradar_core/lib/serviceradar/spiffe.ex
verify_peer_callback/3into separate function clauses foreach event type
verify_spiffe_id/2helper function6 files
agent_registry_test.exs
Test code formatting and idiomatic improvementselixir/serviceradar_core/test/serviceradar/registry/agent_registry_test.exs
50051to50_051) for readabilitylength(list) >= 1checks withrefute Enum.empty?(list)forbetter idiomatic Elixir
length(list) >= 2with pattern matching[_first, _second | _]for clarity
agent_health_test.exs
Format numeric literals with underscoreselixir/serviceradar_core/test/serviceradar/infrastructure/agent_health_test.exs
(e.g.,
50_051instead of50051)test fixtures
otel_traces_test.exs
Format numeric literals with underscoreselixir/serviceradar_core/test/serviceradar/event_writer/processors/otel_traces_test.exs
(e.g.,
1_705_315_800_000_000_000)test data
tenant_schemas.ex
Add module aliases for SQL and error handlingelixir/serviceradar_core/lib/serviceradar/cluster/tenant_schemas.ex
Ecto.Adapters.SQLandPostgrex.Errorfor cleanercode
Ecto.Adapters.SQLcalls withSQLaliasPostgrex.ErrorwithPostgrexErroralias throughoutagent_test.exs
Format port numbers with underscore separatorselixir/serviceradar_core/test/serviceradar/infrastructure/agent_test.exs
50051to50_051)cross_tenant_access_test.exs
Reorder aliases and format port numberselixir/serviceradar_core/test/serviceradar/security/cross_tenant_access_test.exs
50051to50_051)7 files
events.ex
Refactor event processing with extracted helper functionselixir/serviceradar_core/lib/serviceradar/event_writer/processors/events.ex
functions
build_rows/1andinsert_event_rows/2parse_message/1to usewithpattern matching instead ofnested
if/casestatementsrequired_event_fields/1function
jsonb_or_empty_map/1,jsonb_or_empty_list/1,and
parse_string_or/2to reduce duplicationnetflow.ex
Refactor netflow processing with helper functionselixir/serviceradar_core/lib/serviceradar/event_writer/processors/netflow.ex
@direction_mapmodule attribute to consolidate direction stringmappings
build_rows/1andinsert_netflow_rows/2functionsparse_message/1to usewithpattern matching for cleanererror handling
flow_value/4helper function to reduce duplication in fieldextraction logic
parse_direction/1to use map lookup instead of casestatement
result_processor.ex
Refactor result processor with extracted helper functionselixir/serviceradar_core/lib/serviceradar/core/result_processor.ex
length(list) > 0checks withEnum.empty?/1for consistencyapply_canonical_device_id/2,apply_snapshot_mac/2,apply_snapshot_hostname/2, andapply_snapshot_attributes/2copy_attribute_if_empty/3function
has_strong_identity?/1to use helper functionshas_attribute?/2andhas_value?/1poll_orchestrator.ex
Refactor gateway finding with pattern matchingelixir/serviceradar_core/lib/serviceradar/monitoring/poll_orchestrator.ex
find_gateway/1into separate clauses for each assignmentmode (
:any,:partition,:domain,:specific)resolve_gateway_pid/2andresolve_gateway_pid_from_registry/2functionsresolve_gateway_id/2functionlogs.ex
Refactor log processing with extracted helper functionselixir/serviceradar_core/lib/serviceradar/event_writer/processors/logs.ex
build_rows/1andinsert_log_rows/2functionsparse_message/1to usewithpattern matching for cleanererror handling
parse_log_payload/4functionbuild_ingest_metadata/1andmerge_ingest_metadata/2functionsmerge_ingest_value/2helper for conditional map mergingalert_generator.ex
Refactor alert generator with extracted helper functionselixir/serviceradar_core/lib/serviceradar/monitoring/alert_generator.ex
skip_stats_alert?/4functionmaybe_send_stats_alert/4functionbuild_stats_alert/4andbuild_stats_alert_details/3functionsseverity_from_event/1to use helper functionseverity_for_id/1with pattern matchingcluster_health.ex
Simplify cluster health check logicelixir/serviceradar_core/lib/serviceradar/cluster/cluster_health.ex
condwithif-elsestatementfirst
1 files
.credo.exs
Add Credo configuration for code qualityelixir/serviceradar_core/.credo.exs
authorize?: falseusage detectionsuggestions
1 files
system_actor_test.exs
Add SystemActor module testselixir/serviceradar_core/test/serviceradar/actors/system_actor_test.exs
SystemActormodulefor_tenant/2function with various component namesplatform/1functionsystem_actor?/1predicate with multiple actor types98 files
Imported GitHub PR comment.
Original author: @qodo-code-review[bot]
Original URL: https://github.com/carverauto/serviceradar/pull/2273#issuecomment-3739799002
Original created: 2026-01-12T18:01:47Z
PR Compliance Guide 🔍
Below is a summary of compliance checks for this PR:
Dynamic schema injection
Description: Dynamic Ecto query sources are built via string concatenation (
{tenant_schema <>".ocsf_devices", Device}) without validating/sanitizingtenant_schema, so ifTenantSchemas.schema_for_tenant/1can be influenced or returns unexpected values thiscould enable cross-tenant writes or SQL injection via crafted schema/table identifiers.
sweep.ex [203-226]
Referred Code
🎫 No ticket provided
Codebase context is not defined
Follow the guide to enable codebase context checks.
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: Comprehensive Audit Trails
Objective: To create a detailed and reliable record of critical system actions for security analysis
and compliance.
Status:
Missing audit logging: The new actor-based background operations (reads/updates/creates via
Ash.*calls) do notshow explicit audit-trail logging of critical actions (actor, action, outcome), which may
be required but cannot be verified from this diff alone.
Referred Code
Generic: Robust Error Handling and Edge Case Management
Objective: Ensure comprehensive error handling that provides meaningful context and graceful
degradation
Status:
Update results unchecked: New
Repo.update_all/2calls for availability updates do not check or log the updateresult, making failures (or unexpectedly low update counts) harder to detect and diagnose
in production.
Referred Code
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:
Sensitive error logging: New warning logs interpolate
inspect(reason)(and similar inspected error terms) which mayinclude sensitive internal details depending on upstream error contents and should be
reviewed/sanitized.
Referred Code
Generic: Security-First Input Validation and Data Handling
Objective: Ensure all data inputs are validated, sanitized, and handled securely to prevent
vulnerabilities
Status:
Dynamic schema usage: The new code builds dynamic table references using
tenant_schema <>".ocsf_devices"without an in-function validation/allowlist check, which is safeonly if
tenant_schemais guaranteed trusted by upstream code.Referred Code
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/2273#issuecomment-3739813553
Original created: 2026-01-12T18:05:54Z
PR Code Suggestions ✨
Explore these optional code suggestions:
Strengthen system actor identification check
Strengthen the
system_actor?/1check by adding pattern matching for theidprefix and the presence of
tenant_idto prevent potential authorizationbypasses.
elixir/serviceradar_core/lib/serviceradar/actors/system_actor.ex [157-159]
Suggestion importance[1-10]: 9
__
Why: The suggestion correctly identifies a security vulnerability in the newly added
system_actor?/1function, where a check is too broad and could be bypassed, and proposes a stricter pattern match to fix it.Prevent GenServer crash on update
Use
Ash.update(raise?: false)inresolve_alert_record/6to prevent theStatefulAlertEngineGenServer from crashing on update failures.elixir/serviceradar_core/lib/serviceradar/observability/stateful_alert_engine.ex [458-482]
[To ensure code accuracy, apply this suggestion manually]Suggestion importance[1-10]: 7
__
Why: The suggestion correctly identifies that
Ash.update()can crash theStatefulAlertEngineGenServer and proposes usingAsh.update(raise?: false)for graceful error handling, which is a valid and important improvement for process stability.Return nil on invalid events
Explicitly return
nilin the{:error, reason}case ofparse_event/4to ensureinvalid events are correctly filtered out before database insertion.
elixir/serviceradar_core/lib/serviceradar/event_writer/processors/events.ex [126-129]
Suggestion importance[1-10]: 7
__
Why: The suggestion correctly points out that the function should explicitly return
nilon error to ensure invalid events are filtered out, which is a valid and important fix for data integrity.Handle zero-second interval parsing correctly
Modify
parse_interval_fallback/1to explicitly handle an interval of "0" asinvalid, preventing it from incorrectly defaulting to 3600 seconds.
elixir/serviceradar_core/lib/serviceradar/sweep_jobs/sweep_monitor_worker.ex [284-296]
[To ensure code accuracy, apply this suggestion manually]Suggestion importance[1-10]: 6
__
Why: The suggestion correctly identifies an edge case where an interval of "0" is handled incorrectly and proposes a fix to explicitly handle it, improving the function's robustness.
Simplify check by removing helper
Refactor the
empty?/1function by removing the redundantblank?/1helper andusing built-in functions like
is_nil/1,String.trim/1, andEnum.empty?/1directly.
elixir/serviceradar_core/lib/serviceradar/identity/alias_events.ex [204-215]
[To ensure code accuracy, apply this suggestion manually]Suggestion importance[1-10]: 4
__
Why: The suggestion correctly identifies that the
blank?/1helper function can be inlined for better readability and to reduce a minor abstraction. The proposed change is a valid refactoring that improves code style.Imported GitHub PR comment.
Original author: @qodo-code-review[bot]
Original URL: https://github.com/carverauto/serviceradar/pull/2273#issuecomment-3739875836
Original created: 2026-01-12T18:21:04Z
CI Feedback 🧐
A test triggered by this PR failed. Here is an AI-generated analysis of the failure:
Action: build
Failed stage: Configure SRQL fixture database for tests [❌]
Failed test name: ""
Failure summary:
The action failed because the workflow requires the secret
SRQL_TEST_DATABASE_CA_CERTto be set, butit is empty/missing (
SRQL_TEST_DATABASE_CA_CERT:shows no value in the environment).The job
explicitly aborts with the message
SRQL_TEST_DATABASE_CA_CERT secret must be configured to verifySRQL fixture TLS.and exits with code 1 (log lines 636-637).Relevant error logs: