Bug/upgrade clobbering configs #2560
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!2560
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "refs/pull/2560/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: #2121
Original author: @mfreeman451
Original URL: https://github.com/carverauto/serviceradar/pull/2121
Original created: 2025-12-14T21:35:26Z
Original updated: 2025-12-14T22:34:42Z
Original head: carverauto/serviceradar:bug/upgrade_clobbering_configs
Original base: main
Original merged: 2025-12-14T22:34:38Z 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
Bug fix, Enhancement
Description
Fix Docker Compose upgrades clobbering configuration by explicitly naming stateful volumes
Prevent bootstrap script from overwriting existing configs with empty values
Fix KV placeholder-repair logic to preserve partial config overlays during restarts
Add safe upgrade path with FORCE_REGENERATE_CONFIG flag and make compose-upgrade target
Stabilize sysmon endpoint wiring by deriving from single source of truth
Diagram Walkthrough
File Walkthrough
2 files
Skip missing fields in placeholder repair logicPrevent config clobbering and stabilize sysmon endpoint wiring1 files
Update test expectations for missing field handling1 files
Add compose-up and compose-upgrade helper targets1 files
Explicitly name stateful volumes with prefix variable4 files
Document safe upgrade procedure and volume namingAdd OpenSpec proposal for config clobbering fixAdd KV configuration spec requirements for non-destructive upgradesAdd task checklist for config clobbering fix implementationImported GitHub PR comment.
Original author: @qodo-code-review[bot]
Original URL: https://github.com/carverauto/serviceradar/pull/2121#issuecomment-3652219798
Original created: 2025-12-14T21:35:56Z
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:
No security concerns identified
No security vulnerabilities detected by AI analysis. Human verification advised for critical 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: Security-First Input Validation and Data Handling
Objective: Ensure all data inputs are validated, sanitized, and handled securely to prevent
vulnerabilities
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 auditing: The script performs critical config mutations (overwriting configs, regenerating files)
without emitting structured audit logs tying actions to a user or request, which may
hinder audit trails.
Referred Code
Generic: Robust Error Handling and Edge Case Management
Objective: Ensure comprehensive error handling that provides meaningful context and graceful
degradation
Status:
Set-e exit only: The script relies on
set -eand warnings but lacks explicit error handling/logging aroundjq file edits and file moves, risking silent partial updates if jq fails.
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:
Logs may leak: The script echoes configuration values such as sysmon addresses in success messages, which
could expose environment-derived details in logs without structured redaction controls.
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/2121#issuecomment-3652221490
Original created: 2025-12-14T21:36:57Z
You are nearing your monthly Qodo Merge usage quota. For more information, please visit here.
PR Code Suggestions ✨
Explore these optional code suggestions:
Use sponge for atomic file updates
To prevent potential race conditions when updating
poller.json, use thespongeutility for an atomic write operation instead of redirecting to a temporary file
and moving it.
docker/compose/update-config.sh [462-468]
Suggestion importance[1-10]: 3
__
Why: The suggestion correctly identifies a potential race condition with the
jq ... > tmp && mv ...pattern, but the risk is minimal in this context as the script runs during container startup, where concurrent access is highly unlikely. The proposed solution usingspongeintroduces an unstated dependency onmoreutils, which may not be available in the container image.