Bug/upgrade clobbering configs #2560

Merged
mfreeman451 merged 5 commits from refs/pull/2560/head into main 2025-12-14 22:34:38 +00:00
mfreeman451 commented 2025-12-14 21:35:26 +00:00 (Migrated from github.com)
Owner

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:

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

Describe your changes

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


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

flowchart LR
  A["Docker Compose Upgrade"] -->|explicit volume naming| B["Preserve Config State"]
  A -->|FORCE_REGENERATE_CONFIG flag| C["Non-Destructive Bootstrap"]
  D["KV Placeholder Repair"] -->|skip missing fields| E["Preserve Partial Overlays"]
  C -->|consistent sysmon wiring| F["Stable Endpoint Config"]
  B --> G["Safe Upgrade Contract"]
  E --> G
  F --> G

File Walkthrough

Relevant files
Bug fix
2 files
kv_repair.go
Skip missing fields in placeholder repair logic                   
+3/-1     
update-config.sh
Prevent config clobbering and stabilize sysmon endpoint wiring
+87/-15 
Tests
1 files
kv_repair_test.go
Update test expectations for missing field handling           
+2/-2     
Enhancement
1 files
Makefile
Add compose-up and compose-upgrade helper targets               
+9/-0     
Configuration changes
1 files
docker-compose.yml
Explicitly name stateful volumes with prefix variable       
+4/-0     
Documentation
4 files
docker-setup.md
Document safe upgrade procedure and volume naming               
+22/-10 
proposal.md
Add OpenSpec proposal for config clobbering fix                   
+39/-0   
spec.md
Add KV configuration spec requirements for non-destructive upgrades
+18/-0   
tasks.md
Add task checklist for config clobbering fix implementation
+14/-0   

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]( 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 ___ ### **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 ```mermaid flowchart LR A["Docker Compose Upgrade"] -->|explicit volume naming| B["Preserve Config State"] A -->|FORCE_REGENERATE_CONFIG flag| C["Non-Destructive Bootstrap"] D["KV Placeholder Repair"] -->|skip missing fields| E["Preserve Partial Overlays"] C -->|consistent sysmon wiring| F["Stable Endpoint Config"] B --> G["Safe Upgrade Contract"] E --> G F --> G ``` <details><summary><h3>File Walkthrough</h3></summary> <table><thead><tr><th></th><th align="left">Relevant files</th></tr></thead><tbody><tr><td><strong>Bug fix</strong></td><td><details><summary>2 files</summary><table> <tr> <td><strong>kv_repair.go</strong><dd><code>Skip missing fields in placeholder repair logic</code>&nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; </dd></td> <td><a href="https://github.com/carverauto/serviceradar/pull/2121/files#diff-cf55ade5ae708eee328e7cd48131c8cc5c4b83968511dfebc28ff28809e1d768">+3/-1</a>&nbsp; &nbsp; &nbsp; </td> </tr> <tr> <td><strong>update-config.sh</strong><dd><code>Prevent config clobbering and stabilize sysmon endpoint wiring</code></dd></td> <td><a href="https://github.com/carverauto/serviceradar/pull/2121/files#diff-9ae50be83a13010a038389c74407ba1bde8cabcea0944e238c4b3374133f78bf">+87/-15</a>&nbsp; </td> </tr> </table></details></td></tr><tr><td><strong>Tests</strong></td><td><details><summary>1 files</summary><table> <tr> <td><strong>kv_repair_test.go</strong><dd><code>Update test expectations for missing field handling</code>&nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; </dd></td> <td><a href="https://github.com/carverauto/serviceradar/pull/2121/files#diff-4d31f0198901fd3994040a41f1c88120720ca717d8c21ae48a3e225e36cfcf5e">+2/-2</a>&nbsp; &nbsp; &nbsp; </td> </tr> </table></details></td></tr><tr><td><strong>Enhancement</strong></td><td><details><summary>1 files</summary><table> <tr> <td><strong>Makefile</strong><dd><code>Add compose-up and compose-upgrade helper targets</code>&nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; </dd></td> <td><a href="https://github.com/carverauto/serviceradar/pull/2121/files#diff-76ed074a9305c04054cdebb9e9aad2d818052b07091de1f20cad0bbac34ffb52">+9/-0</a>&nbsp; &nbsp; &nbsp; </td> </tr> </table></details></td></tr><tr><td><strong>Configuration changes</strong></td><td><details><summary>1 files</summary><table> <tr> <td><strong>docker-compose.yml</strong><dd><code>Explicitly name stateful volumes with prefix variable</code>&nbsp; &nbsp; &nbsp; &nbsp; </dd></td> <td><a href="https://github.com/carverauto/serviceradar/pull/2121/files#diff-e45e45baeda1c1e73482975a664062aa56f20c03dd9d64a827aba57775bed0d3">+4/-0</a>&nbsp; &nbsp; &nbsp; </td> </tr> </table></details></td></tr><tr><td><strong>Documentation</strong></td><td><details><summary>4 files</summary><table> <tr> <td><strong>docker-setup.md</strong><dd><code>Document safe upgrade procedure and volume naming</code>&nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; </dd></td> <td><a href="https://github.com/carverauto/serviceradar/pull/2121/files#diff-8604269dffb3ce4133e48cab374ca8e97745d0efbdef67cad792aeb5945fe5ec">+22/-10</a>&nbsp; </td> </tr> <tr> <td><strong>proposal.md</strong><dd><code>Add OpenSpec proposal for config clobbering fix</code>&nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; </dd></td> <td><a href="https://github.com/carverauto/serviceradar/pull/2121/files#diff-d4f7fb7aaeb9384efaf2ec505c9e73e6a4405542d9e3c5f64dc736ba8e9e217d">+39/-0</a>&nbsp; &nbsp; </td> </tr> <tr> <td><strong>spec.md</strong><dd><code>Add KV configuration spec requirements for non-destructive upgrades</code></dd></td> <td><a href="https://github.com/carverauto/serviceradar/pull/2121/files#diff-b4ce1b71c288e8d4d2d9fce388795c84e0cfc51f0cd66e3e3f7f4767e7890a24">+18/-0</a>&nbsp; &nbsp; </td> </tr> <tr> <td><strong>tasks.md</strong><dd><code>Add task checklist for config clobbering fix implementation</code></dd></td> <td><a href="https://github.com/carverauto/serviceradar/pull/2121/files#diff-cf742869daef1d609cc7849d92f8cdba9eb4d3cd7eb3d403c86bd0a4e8b3ab3b">+14/-0</a>&nbsp; &nbsp; </td> </tr> </table></details></td></tr></tbody></table> </details> ___
qodo-code-review[bot] commented 2025-12-14 21:35:56 +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/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:

Security Compliance
🟢
No security concerns identified No security vulnerabilities detected by AI analysis. Human verification advised for critical code.
Ticket Compliance
🎫 No ticket provided
  • Create ticket/issue
Codebase Duplication Compliance
Codebase context is not defined

Follow the guide to enable codebase context checks.

Custom Compliance
🟢
Generic: Meaningful Naming and Self-Documenting Code

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

Status: Passed

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

Generic: Secure Error Handling

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 generic rules or creating your own custom rules

Generic: Security-First Input Validation and Data Handling

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

Status: Passed

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

Generic: Comprehensive Audit Trails

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

Status:
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
if [ ! -f "$CORE_CONFIG" ] || is_truthy "$FORCE_REGENERATE_CONFIG"; then
    if [ -f "$CORE_CONFIG" ]; then
        warn "FORCE_REGENERATE_CONFIG enabled; overwriting existing core.json from template"
    else
        echo "Seeding core.json from template for the first time..."
    fi
    cp /config/core.docker.json "$CORE_CONFIG"
    echo "✅ Created core.json from template"
else
    echo "core.json already exists; preserving existing auth keys and settings"
fi

# Generate JWT secret and API key if they don't exist
if [ ! -f "$CERT_DIR/jwt-secret" ]; then
    echo "Generating JWT secret..."
    random_hex 32 > "$CERT_DIR/jwt-secret"
    echo "✅ Generated JWT secret"

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

Generic: Robust Error Handling and Edge Case Management

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

Status:
Set-e exit only: The script relies on set -e and warnings but lacks explicit error handling/logging around
jq file edits and file moves, risking silent partial updates if jq fails.

Referred Code
# Only generate poller.json if it doesn't exist (preserves manual customizations unless forced)
if [ -f "$CONFIG_DIR/poller.json" ] && ! is_truthy "$FORCE_REGENERATE_CONFIG"; then
    echo "poller.json already exists; preserving existing configuration"
    EXISTING_SYSMON_DETAILS="$(jq -r '([.agents[]?.checks[]? | select(.service_name == "sysmon-osx") | .details] | .[0] // "")' "$CONFIG_DIR/poller.json" 2>/dev/null || echo "")"
    if [ -z "$SYSMON_OSX_ADDRESS" ]; then
        warn "sysmon endpoint is unset (SYSMON_OSX_ADDRESS/SYSMON_VM_ADDRESS); leaving sysmon-osx details as-is in poller.json"
    elif [ -z "$EXISTING_SYSMON_DETAILS" ]; then
        jq --arg addr "$SYSMON_OSX_ADDRESS" '
            (.agents[]?.checks[]? | select(.service_name == "sysmon-osx")).details
              |= (if (. // "") == "" then $addr else . end)
        ' "$CONFIG_DIR/poller.json" > "$CONFIG_DIR/poller.json.tmp"
        mv "$CONFIG_DIR/poller.json.tmp" "$CONFIG_DIR/poller.json"
        echo "✅ Repaired sysmon-osx details in poller.json to $SYSMON_OSX_ADDRESS"
    else
        echo "sysmon-osx details already set in poller.json (details=$EXISTING_SYSMON_DETAILS); leaving unchanged"
    fi

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

Generic: Secure Logging Practices

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

Status:
Logs 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
    }
  }
}
EOF
    if [ -n "$EXISTING_SYSMON_OSX_ADDRESS" ]; then
        echo "✅ Generated sysmon-osx checker config (mTLS) with address $SYSMON_OSX_ADDRESS"
    else
        echo "✅ Repaired sysmon-osx checker config (mTLS) with address $SYSMON_OSX_ADDRESS"
    fi

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

  • Update
Compliance status legend 🟢 - Fully Compliant
🟡 - Partial Compliant
🔴 - Not Compliant
- Requires Further Human Verification
🏷️ - Compliance label
Imported GitHub PR comment. Original author: @qodo-code-review[bot] Original URL: https://github.com/carverauto/serviceradar/pull/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](https://qodo-merge-docs.qodo.ai/installation/qodo_merge/#cloud-users)._ ## PR Compliance Guide 🔍 <!-- https://github.com/carverauto/serviceradar/commit/f78cbf193b1e11362e82689b82e2461bafdd9b64 --> Below is a summary of compliance checks for this PR:<br> <table><tbody><tr><td colspan='2'><strong>Security Compliance</strong></td></tr> <tr><td>🟢</td><td><details><summary><strong>No security concerns identified</strong></summary> No security vulnerabilities detected by AI analysis. Human verification advised for critical code. </details></td></tr> <tr><td colspan='2'><strong>Ticket Compliance</strong></td></tr> <tr><td>⚪</td><td><details><summary>🎫 <strong>No ticket provided </strong></summary> - [ ] Create ticket/issue <!-- /create_ticket --create_ticket=true --> </details></td></tr> <tr><td colspan='2'><strong>Codebase Duplication Compliance</strong></td></tr> <tr><td>⚪</td><td><details><summary><strong>Codebase context is not defined </strong></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'><strong>Custom Compliance</strong></td></tr> <tr><td rowspan=3>🟢</td><td> <details><summary><strong>Generic: Meaningful Naming and Self-Documenting Code</strong></summary><br> **Objective:** Ensure all identifiers clearly express their purpose and intent, making code <br>self-documenting<br> **Status:** Passed<br> > 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><strong>Generic: Secure Error Handling</strong></summary><br> **Objective:** To prevent the leakage of sensitive system information through error messages while <br>providing sufficient detail for internal debugging.<br> **Status:** Passed<br> > 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><strong>Generic: Security-First Input Validation and Data Handling</strong></summary><br> **Objective:** Ensure all data inputs are validated, sanitized, and handled securely to prevent <br>vulnerabilities<br> **Status:** Passed<br> > Learn more about managing compliance <a href='https://qodo-merge-docs.qodo.ai/tools/compliance/#configuration-options'>generic rules</a> or creating your own <a href='https://qodo-merge-docs.qodo.ai/tools/compliance/#custom-compliance'>custom rules</a> </details></td></tr> <tr><td rowspan=3>⚪</td> <td><details> <summary><strong>Generic: Comprehensive Audit Trails</strong></summary><br> **Objective:** To create a detailed and reliable record of critical system actions for security analysis <br>and compliance.<br> **Status:** <br><a href='https://github.com/carverauto/serviceradar/pull/2121/files#diff-9ae50be83a13010a038389c74407ba1bde8cabcea0944e238c4b3374133f78bfR60-R76'><strong>Missing auditing</strong></a>: The script performs critical config mutations (overwriting configs, regenerating files) <br>without emitting structured audit logs tying actions to a user or request, which may <br>hinder audit trails.<br> <details open><summary>Referred Code</summary> ```shell if [ ! -f "$CORE_CONFIG" ] || is_truthy "$FORCE_REGENERATE_CONFIG"; then if [ -f "$CORE_CONFIG" ]; then warn "FORCE_REGENERATE_CONFIG enabled; overwriting existing core.json from template" else echo "Seeding core.json from template for the first time..." fi cp /config/core.docker.json "$CORE_CONFIG" echo "✅ Created core.json from template" else echo "core.json already exists; preserving existing auth keys and settings" fi # Generate JWT secret and API key if they don't exist if [ ! -f "$CERT_DIR/jwt-secret" ]; then echo "Generating JWT secret..." random_hex 32 > "$CERT_DIR/jwt-secret" echo "✅ Generated JWT secret" ``` </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><strong>Generic: Robust Error Handling and Edge Case Management</strong></summary><br> **Objective:** Ensure comprehensive error handling that provides meaningful context and graceful <br>degradation<br> **Status:** <br><a href='https://github.com/carverauto/serviceradar/pull/2121/files#diff-9ae50be83a13010a038389c74407ba1bde8cabcea0944e238c4b3374133f78bfR456-R471'><strong>Set-e exit only</strong></a>: The script relies on <code>set -e</code> and warnings but lacks explicit error handling/logging around <br>jq file edits and file moves, risking silent partial updates if jq fails.<br> <details open><summary>Referred Code</summary> ```shell # Only generate poller.json if it doesn't exist (preserves manual customizations unless forced) if [ -f "$CONFIG_DIR/poller.json" ] && ! is_truthy "$FORCE_REGENERATE_CONFIG"; then echo "poller.json already exists; preserving existing configuration" EXISTING_SYSMON_DETAILS="$(jq -r '([.agents[]?.checks[]? | select(.service_name == "sysmon-osx") | .details] | .[0] // "")' "$CONFIG_DIR/poller.json" 2>/dev/null || echo "")" if [ -z "$SYSMON_OSX_ADDRESS" ]; then warn "sysmon endpoint is unset (SYSMON_OSX_ADDRESS/SYSMON_VM_ADDRESS); leaving sysmon-osx details as-is in poller.json" elif [ -z "$EXISTING_SYSMON_DETAILS" ]; then jq --arg addr "$SYSMON_OSX_ADDRESS" ' (.agents[]?.checks[]? | select(.service_name == "sysmon-osx")).details |= (if (. // "") == "" then $addr else . end) ' "$CONFIG_DIR/poller.json" > "$CONFIG_DIR/poller.json.tmp" mv "$CONFIG_DIR/poller.json.tmp" "$CONFIG_DIR/poller.json" echo "✅ Repaired sysmon-osx details in poller.json to $SYSMON_OSX_ADDRESS" else echo "sysmon-osx details already set in poller.json (details=$EXISTING_SYSMON_DETAILS); leaving unchanged" fi ``` </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><strong>Generic: Secure Logging Practices</strong></summary><br> **Objective:** To ensure logs are useful for debugging and auditing without exposing sensitive <br>information like PII, PHI, or cardholder data.<br> **Status:** <br><a href='https://github.com/carverauto/serviceradar/pull/2121/files#diff-9ae50be83a13010a038389c74407ba1bde8cabcea0944e238c4b3374133f78bfR393-R401'><strong>Logs may leak</strong></a>: The script echoes configuration values such as sysmon addresses in success messages, which <br>could expose environment-derived details in logs without structured redaction controls.<br> <details open><summary>Referred Code</summary> ```shell } } } EOF if [ -n "$EXISTING_SYSMON_OSX_ADDRESS" ]; then echo "✅ Generated sysmon-osx checker config (mTLS) with address $SYSMON_OSX_ADDRESS" else echo "✅ Repaired sysmon-osx checker config (mTLS) with address $SYSMON_OSX_ADDRESS" fi ``` </details> > Learn more about managing compliance <a href='https://qodo-merge-docs.qodo.ai/tools/compliance/#configuration-options'>generic rules</a> or creating your own <a href='https://qodo-merge-docs.qodo.ai/tools/compliance/#custom-compliance'>custom rules</a> </details></td></tr> <tr><td align="center" colspan="2"> - [ ] Update <!-- /compliance --update_compliance=true --> </td></tr></tbody></table> <details><summary>Compliance status legend</summary> 🟢 - Fully Compliant<br> 🟡 - Partial Compliant<br> 🔴 - Not Compliant<br> ⚪ - Requires Further Human Verification<br> 🏷️ - Compliance label<br> </details>
qodo-code-review[bot] commented 2025-12-14 21:36:57 +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/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:

CategorySuggestion                                                                                                                                    Impact
Possible issue
Use sponge for atomic file updates

To prevent potential race conditions when updating poller.json, use the sponge
utility for an atomic write operation instead of redirecting to a temporary file
and moving it.

docker/compose/update-config.sh [462-468]

 elif [ -z "$EXISTING_SYSMON_DETAILS" ]; then
     jq --arg addr "$SYSMON_OSX_ADDRESS" '
         (.agents[]?.checks[]? | select(.service_name == "sysmon-osx")).details
           |= (if (. // "") == "" then $addr else . end)
-    ' "$CONFIG_DIR/poller.json" > "$CONFIG_DIR/poller.json.tmp"
-    mv "$CONFIG_DIR/poller.json.tmp" "$CONFIG_DIR/poller.json"
+    ' "$CONFIG_DIR/poller.json" | sponge "$CONFIG_DIR/poller.json"
     echo "✅ Repaired sysmon-osx details in poller.json to $SYSMON_OSX_ADDRESS"
  • Apply / Chat
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 using sponge introduces an unstated dependency on moreutils, which may not be available in the container image.

Low
  • Update
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](https://qodo-merge-docs.qodo.ai/installation/qodo_merge/#cloud-users)._ ## PR Code Suggestions ✨ <!-- f78cbf1 --> Explore these optional code suggestions: <table><thead><tr><td><strong>Category</strong></td><td align=left><strong>Suggestion&nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; </strong></td><td align=center><strong>Impact</strong></td></tr><tbody><tr><td rowspan=1>Possible issue</td> <td> <details><summary>Use sponge for atomic file updates</summary> ___ **To prevent potential race conditions when updating <code>poller.json</code>, use the <code>sponge</code> <br>utility for an atomic write operation instead of redirecting to a temporary file <br>and moving it.** [docker/compose/update-config.sh [462-468]](https://github.com/carverauto/serviceradar/pull/2121/files#diff-9ae50be83a13010a038389c74407ba1bde8cabcea0944e238c4b3374133f78bfR462-R468) ```diff elif [ -z "$EXISTING_SYSMON_DETAILS" ]; then jq --arg addr "$SYSMON_OSX_ADDRESS" ' (.agents[]?.checks[]? | select(.service_name == "sysmon-osx")).details |= (if (. // "") == "" then $addr else . end) - ' "$CONFIG_DIR/poller.json" > "$CONFIG_DIR/poller.json.tmp" - mv "$CONFIG_DIR/poller.json.tmp" "$CONFIG_DIR/poller.json" + ' "$CONFIG_DIR/poller.json" | sponge "$CONFIG_DIR/poller.json" echo "✅ Repaired sysmon-osx details in poller.json to $SYSMON_OSX_ADDRESS" ``` - [ ] **Apply / Chat** <!-- /improve --apply_suggestion=0 --> <details><summary>Suggestion importance[1-10]: 3</summary> __ 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 using `sponge` introduces an unstated dependency on `moreutils`, which may not be available in the container image. </details></details></td><td align=center>Low </td></tr> <tr><td align="center" colspan="2"> - [ ] Update <!-- /improve_multi --more_suggestions=true --> </td><td></td></tr></tbody></table>
Sign in to join this conversation.
No reviewers
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
carverauto/serviceradar!2560
No description provided.