drop force flag #2287

Merged
mfreeman451 merged 2 commits from refs/pull/2287/head into main 2025-10-06 00:36:10 +00:00
mfreeman451 commented 2025-10-06 00:30:32 +00:00 (Migrated from github.com)
Owner

Imported from GitHub pull request.

Original GitHub pull request: #1715
Original author: @mfreeman451
Original URL: https://github.com/carverauto/serviceradar/pull/1715
Original created: 2025-10-06T00:30:32Z
Original updated: 2025-10-06T00:36:13Z
Original head: carverauto/serviceradar:core/auth_issue
Original base: main
Original merged: 2025-10-06T00:36:10Z by @mfreeman451

PR Type

Enhancement


Description

  • Remove --force flag from JWT key generation command

Diagram Walkthrough

flowchart LR
  A["serviceradar-cli command"] --> B["generate-jwt-keys without --force flag"]

File Walkthrough

Relevant files
Configuration changes
serviceradar-core.yaml
Remove force flag from JWT generation                                       

k8s/demo/base/serviceradar-core.yaml

  • Removed --force flag from serviceradar-cli generate-jwt-keys command
    in init container
+1/-1     

Imported from GitHub pull request. Original GitHub pull request: #1715 Original author: @mfreeman451 Original URL: https://github.com/carverauto/serviceradar/pull/1715 Original created: 2025-10-06T00:30:32Z Original updated: 2025-10-06T00:36:13Z Original head: carverauto/serviceradar:core/auth_issue Original base: main Original merged: 2025-10-06T00:36:10Z by @mfreeman451 --- ### **PR Type** Enhancement ___ ### **Description** - Remove `--force` flag from JWT key generation command ___ ### Diagram Walkthrough ```mermaid flowchart LR A["serviceradar-cli command"] --> B["generate-jwt-keys without --force flag"] ``` <details> <summary><h3> File Walkthrough</h3></summary> <table><thead><tr><th></th><th align="left">Relevant files</th></tr></thead><tbody><tr><td><strong>Configuration changes</strong></td><td><table> <tr> <td> <details> <summary><strong>serviceradar-core.yaml</strong><dd><code>Remove force flag from JWT generation</code>&nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; </dd></summary> <hr> k8s/demo/base/serviceradar-core.yaml <ul><li>Removed <code>--force</code> flag from <code>serviceradar-cli generate-jwt-keys</code> command <br>in init container</ul> </details> </td> <td><a href="https://github.com/carverauto/serviceradar/pull/1715/files#diff-2f484d8fe3bae65aace437568f6dd660c92f57b452f7bd1608083a8fe3716ba3">+1/-1</a>&nbsp; &nbsp; &nbsp; </td> </tr> </table></td></tr></tr></tbody></table> </details> ___
qodo-code-review[bot] commented 2025-10-06 00:30:47 +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/1715#issuecomment-3369566824
Original created: 2025-10-06T00:30:47Z

PR Compliance Guide 🔍

Below is a summary of compliance checks for this PR:

Security Compliance
Sensitive key storage

Description: Generating and storing JWT keys at /var/lib/serviceradar within an init container may
persist sensitive key material to a writable path whose security and lifecycle are
unclear, risking exposure if the volume is not adequately protected or rotated.
serviceradar-core.yaml [31-31]

Referred Code
  serviceradar-cli generate-jwt-keys --file /var/lib/serviceradar/core.json --bits 2048
volumeMounts:
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
No custom compliance provided

Follow the guide to enable custom compliance check.

  • 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/1715#issuecomment-3369566824 Original created: 2025-10-06T00:30:47Z --- ## PR Compliance Guide 🔍 <!-- https://github.com/carverauto/serviceradar/commit/1eff66a55b6f551d8fbfe21e16f310042fd898a4 --> 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 rowspan=1>⚪</td> <td><details><summary><strong>Sensitive key storage </strong></summary><br> <b>Description:</b> Generating and storing JWT keys at /var/lib/serviceradar within an init container may <br>persist sensitive key material to a writable path whose security and lifecycle are <br>unclear, risking exposure if the volume is not adequately protected or rotated.<br> <strong><a href='https://github.com/carverauto/serviceradar/pull/1715/files#diff-2f484d8fe3bae65aace437568f6dd660c92f57b452f7bd1608083a8fe3716ba3R31-R31'>serviceradar-core.yaml [31-31]</a></strong><br> <details open><summary>Referred Code</summary> ```yaml serviceradar-cli generate-jwt-keys --file /var/lib/serviceradar/core.json --bits 2048 volumeMounts: ``` </details></details></td></tr> <tr><td colspan='2'><strong>Ticket Compliance</strong></td></tr> <tr><td>⚪</td><td><details><summary>🎫 <strong>No ticket provided </summary></strong> - [ ] 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>⚪</td><td><details><summary><strong>No custom compliance provided</strong></summary> Follow the <a href='https://qodo-merge-docs.qodo.ai/tools/compliance/'>guide</a> to enable custom compliance check. </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-10-06 00:31:35 +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/1715#issuecomment-3369567907
Original created: 2025-10-06T00:31:35Z

PR Code Suggestions

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
Possible issue
Prevent JWT key regeneration on restarts

Make the JWT key generation process idempotent by checking for the existence of
the configuration file before initializing it. This will prevent key
regeneration on every pod restart, which currently invalidates all existing
tokens.

k8s/demo/base/serviceradar-core.yaml [28-31]

 set -e
 mkdir -p /var/lib/serviceradar
-cp /etc/serviceradar/core.json /var/lib/serviceradar/core.json
-serviceradar-cli generate-jwt-keys --file /var/lib/serviceradar/core.json --bits 2048
+CONFIG_FILE="/var/lib/serviceradar/core.json"
+if [ ! -f "$CONFIG_FILE" ]; then
+  echo "Configuration not found, initializing..."
+  cp /etc/serviceradar/core.json "$CONFIG_FILE"
+  serviceradar-cli generate-jwt-keys --file "$CONFIG_FILE" --bits 2048
+else
+  echo "Configuration found, skipping initialization."
+fi
  • Apply / Chat
Suggestion importance[1-10]: 10

__

Why: The suggestion correctly identifies a critical flaw in the init container's logic where JWT keys are regenerated on every pod restart, which would invalidate all user sessions and API tokens. The proposed fix to make the initialization idempotent is the correct approach to ensure key stability.

High
  • Update
Imported GitHub PR comment. Original author: @qodo-code-review[bot] Original URL: https://github.com/carverauto/serviceradar/pull/1715#issuecomment-3369567907 Original created: 2025-10-06T00:31:35Z --- ## PR Code Suggestions ✨ <!-- 1eff66a --> 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>Prevent JWT key regeneration on restarts</summary> ___ **Make the JWT key generation process idempotent by checking for the existence of <br>the configuration file before initializing it. This will prevent key <br>regeneration on every pod restart, which currently invalidates all existing <br>tokens.** [k8s/demo/base/serviceradar-core.yaml [28-31]](https://github.com/carverauto/serviceradar/pull/1715/files#diff-2f484d8fe3bae65aace437568f6dd660c92f57b452f7bd1608083a8fe3716ba3R28-R31) ```diff set -e mkdir -p /var/lib/serviceradar -cp /etc/serviceradar/core.json /var/lib/serviceradar/core.json -serviceradar-cli generate-jwt-keys --file /var/lib/serviceradar/core.json --bits 2048 +CONFIG_FILE="/var/lib/serviceradar/core.json" +if [ ! -f "$CONFIG_FILE" ]; then + echo "Configuration not found, initializing..." + cp /etc/serviceradar/core.json "$CONFIG_FILE" + serviceradar-cli generate-jwt-keys --file "$CONFIG_FILE" --bits 2048 +else + echo "Configuration found, skipping initialization." +fi ``` - [ ] **Apply / Chat** <!-- /improve --apply_suggestion=0 --> <details><summary>Suggestion importance[1-10]: 10</summary> __ Why: The suggestion correctly identifies a critical flaw in the init container's logic where JWT keys are regenerated on every pod restart, which would invalidate all user sessions and API tokens. The proposed fix to make the initialization idempotent is the correct approach to ensure key stability. </details></details></td><td align=center>High </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!2287
No description provided.