FE: Notifications sorting fix #1713#1714
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThe notification timestamp renderer now preserves raw values for sorting, while the Telegram publisher sends JSON requests through an argument-based curl subprocess with updated truncation, timeout, output, and error handling. ChangesNotification timestamp rendering
Telegram message publishing
Sequence Diagram(s)sequenceDiagram
participant Publisher as Telegram publisher
participant Curl as curl subprocess
participant Telegram as Telegram API
Publisher->>Publisher: Serialize truncated message as JSON
Publisher->>Curl: Execute curl with argv request
Curl->>Telegram: Send JSON request
Telegram-->>Curl: Return response
Curl-->>Publisher: Return stdout
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@front/userNotifications.php`:
- Around line 91-93: Update the conditional in the DataTables rendering callback
around the sort/type handling to return the raw value whenever type is not
"display". Replace the limited sort/type check with a type !== "display" check
so filter uses the unlocalized value as well.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 319f419c-5a35-4fb3-b8a9-6da928d0ff44
📒 Files selected for processing (1)
front/userNotifications.php
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@front/plugins/_publisher_telegram/tg.py`:
- Line 109: Update the debug logging around the Telegram command in the
publisher flow to avoid including the bot token embedded in the API URL. Log
only the operation name or a URL with the token redacted, while preserving the
existing command execution behavior.
- Around line 88-91: Update the truncation logic in the payload construction
block to ensure the final payload, including " (text was truncated)", never
exceeds TELEGRAM_SIZE. Reduce the text slice by the marker length before
appending it, while preserving the existing unmodified text path when no
truncation is needed.
- Around line 99-118: Add curl connection and total request timeouts to the
command assembled in the Telegram publisher flow before subprocess.run, using
values both below RUN_TIMEOUT. Update the cmd construction associated with the
sendMessage request while preserving the existing subprocess behavior and
logging.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 5cb31b82-0f48-4585-80fd-a7ad5baeb671
📒 Files selected for processing (1)
front/plugins/_publisher_telegram/tg.py
| if len(text) > limit: | ||
| payloadData = text[:limit] + " (text was truncated)" | ||
| payload_data = text[:limit] + " (text was truncated)" | ||
| else: | ||
| payloadData = text | ||
| payload_data = text |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Keep the truncated payload within TELEGRAM_SIZE.
Slicing to limit and then appending the marker produces a message longer than the configured limit, potentially causing Telegram to reject it.
Proposed fix
+ marker = " (text was truncated)"
- payload_data = text[:limit] + " (text was truncated)"
+ payload_data = (text[:max(0, limit - len(marker))] + marker)[:limit]📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if len(text) > limit: | |
| payloadData = text[:limit] + " (text was truncated)" | |
| payload_data = text[:limit] + " (text was truncated)" | |
| else: | |
| payloadData = text | |
| payload_data = text | |
| if len(text) > limit: | |
| marker = " (text was truncated)" | |
| payload_data = (text[:max(0, limit - len(marker))] + marker)[:limit] | |
| else: | |
| payload_data = text |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@front/plugins/_publisher_telegram/tg.py` around lines 88 - 91, Update the
truncation logic in the payload construction block to ensure the final payload,
including " (text was truncated)", never exceeds TELEGRAM_SIZE. Reduce the text
slice by the marker length before appending it, while preserving the existing
unmodified text path when no truncation is needed.
| cmd = [ | ||
| "curl", | ||
| "--location", | ||
| f"https://api.telegram.org/bot{get_setting_value('TELEGRAM_URL')}/sendMessage", | ||
| "--header", | ||
| "Content-Type: application/json", | ||
| "--data", | ||
| payload, | ||
| ] | ||
|
|
||
| mylog("debug", ["Executing:", " ".join(cmd[:-1]), "--data <json>"]) | ||
|
|
||
| try: | ||
| # try runnning a subprocess | ||
|
|
||
| req = """curl --location 'https://api.telegram.org/bot%s/sendMessage' \\ | ||
| --header 'Content-Type: application/json' \\ | ||
| --data '{ | ||
| "chat_id": "%s", | ||
| "text": "%s", | ||
| "disable_notification": false | ||
| }'""" % (get_setting_value('TELEGRAM_URL'), get_setting_value('TELEGRAM_HOST'), payloadData) | ||
|
|
||
| mylog('debug', [req]) | ||
|
|
||
| p = subprocess.Popen(req, stdout=subprocess.PIPE, stderr=subprocess.STDOUT, shell=True) | ||
| stdout, stderr = p.communicate() | ||
|
|
||
| # write stdout and stderr into .log files for debugging if needed | ||
| # Log the stdout and stderr | ||
| mylog('debug', [stdout, stderr]) | ||
|
|
||
| # log result | ||
| result = stdout | ||
| proc = subprocess.run( | ||
| cmd, | ||
| stdout=subprocess.PIPE, | ||
| stderr=subprocess.STDOUT, | ||
| text=True, | ||
| check=False, | ||
| ) |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the target file around the cited lines.
sed -n '1,180p' front/plugins/_publisher_telegram/tg.py
# Find timeout-related settings and usages in this plugin.
rg -n "RUN_TIMEOUT|connect-timeout|max-time|timeout|curl" front/plugins/_publisher_telegram/tg.py front/plugins/_publisher_telegram -S
# Look for similar curl invocations elsewhere for comparison.
rg -n "subprocess\.run\(.*curl|--connect-timeout|--max-time" front -SRepository: netalertx/NetAlertX
Length of output: 4453
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect RUN_TIMEOUT definition and how it is used in this plugin/config.
rg -n "RUN_TIMEOUT" front/plugins/_publisher_telegram front/plugins -S
# Read the relevant config section for the Telegram plugin.
sed -n '330,390p' front/plugins/_publisher_telegram/config.json
# Search for run-time enforcement helpers in the repo.
rg -n "run timeout|RUN_TIMEOUT|timeout" front server -S | head -n 200Repository: netalertx/NetAlertX
Length of output: 50376
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Show the Telegram plugin's RUN_TIMEOUT config block.
sed -n '340,375p' front/plugins/_publisher_telegram/config.json
# Show only the send() function and surrounding lines from tg.py.
sed -n '78,130p' front/plugins/_publisher_telegram/tg.py
# Check whether subprocess.run in this file uses timeout= anywhere.
rg -n "subprocess\.run\(|timeout=" front/plugins/_publisher_telegram/tg.py -n -SRepository: netalertx/NetAlertX
Length of output: 2204
Add curl connection and request timeouts at front/plugins/_publisher_telegram/tg.py:99-118.
A stalled Telegram request can block this publisher loop and delay later notifications. Set --connect-timeout and --max-time to values below RUN_TIMEOUT.
🧰 Tools
🪛 ast-grep (0.44.1)
[error] 111-117: Use of unsanitized data to create processes
Context: subprocess.run(
cmd,
stdout=subprocess.PIPE,
stderr=subprocess.STDOUT,
text=True,
check=False,
)
Note: [CWE-78] Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection').
(os-system-unsanitized-data)
[error] 111-117: Command coming from incoming request
Context: subprocess.run(
cmd,
stdout=subprocess.PIPE,
stderr=subprocess.STDOUT,
text=True,
check=False,
)
Note: [CWE-78] Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection').
(subprocess-from-request)
🪛 Ruff (0.15.21)
[error] 112-112: subprocess call: check for execution of untrusted input
(S603)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@front/plugins/_publisher_telegram/tg.py` around lines 99 - 118, Add curl
connection and total request timeouts to the command assembled in the Telegram
publisher flow before subprocess.run, using values both below RUN_TIMEOUT.
Update the cmd construction associated with the sendMessage request while
preserving the existing subprocess behavior and logging.
Source: Learnings
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@front/plugins/_publisher_telegram/tg.py`:
- Around line 116-118: Remove the standalone stray period immediately before the
mylog("debug", ["Executing: Telegram sendMessage", "--data <json>"]) call,
leaving the surrounding list or statement structure valid so tg.py parses and
runs successfully.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 552e935e-1710-42ec-b6ea-cda26f2375f8
📒 Files selected for processing (1)
front/plugins/_publisher_telegram/tg.py
1 participant
Summary by CodeRabbit
Bug Fixes
Improvements