New notification agent: Zentik notifier agent #2490
Conversation
WalkthroughAdds a new Unraid Dynamix agent manifest ( Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: Repository UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
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: 2
🧹 Nitpick comments (2)
emhttp/plugins/dynamix/agents/ZentikNotifier.xml (2)
31-32: Consider expanding logging for better observability.The
LOGvariable is defined but only used for error messages (lines 74, 78, 102). For better troubleshooting, consider logging successful operations or key steps (e.g., "Sending notification to $ENDPOINT", "Notification sent successfully").
49-52: Optional: Simplify whitespace normalization.The sed pipeline for
TITLEandSUBTITLEworks correctly but could be slightly more concise:TITLE=$(echo -e "$TITLE" | tr '\n' ' ' | tr -s ' ' | sed 's/^ //; s/ $//') SUBTITLE=$(echo -e "$SUBTITLE" | tr '\n' ' ' | tr -s ' ' | sed 's/^ //; s/ $//')Or using
xargswhich naturally trims and normalizes whitespace:TITLE=$(echo -e "$TITLE" | xargs) SUBTITLE=$(echo -e "$SUBTITLE" | xargs)
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
emhttp/plugins/dynamix/icons/zentiknotifier.pngis excluded by!**/*.png
📒 Files selected for processing (1)
emhttp/plugins/dynamix/agents/ZentikNotifier.xml(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: ljm42
Repo: unraid/webgui PR: 2364
File: emhttp/plugins/dynamix/agents/Discord.xml:63-66
Timestamp: 2025-09-15T16:59:48.473Z
Learning: In Unraid systems, NGINX_DEFAULTURL configuration only produces servername or servername.localTLD formats, never IPv4/IPv6 addresses. The dot-checking logic in Discord notification agent is specifically designed to filter out bare hostnames that Discord API won't accept.
📚 Learning: 2025-09-05T16:33:12.970Z
Learnt from: Squidly271
Repo: unraid/webgui PR: 2353
File: emhttp/plugins/dynamix/DashStats.page:2257-2257
Timestamp: 2025-09-05T16:33:12.970Z
Learning: In the Unraid webGUI Nchan system, publisher scripts (like "update_1") publish to normalized endpoint names (like "update1"). The Nchan header lists the script names to start, while JavaScript subscribes to the published endpoints. For example: Nchan="update_1" starts the script which calls publish_noDupe('update1', data), and JavaScript subscribes to '/sub/update1'. This is the intended design, not a mismatch.
Applied to files:
emhttp/plugins/dynamix/agents/ZentikNotifier.xml
🔇 Additional comments (5)
emhttp/plugins/dynamix/agents/ZentikNotifier.xml (5)
1-12: LGTM! Well-structured agent manifest.The variable declarations follow Unraid agent conventions with appropriate help text, descriptions, and sensible defaults.
27-29: LGTM! Proper dependency validation.The upfront dependency check for
curlandjqprevents runtime failures with a clear error message.
54-65: LGTM! Sensible importance mapping.The mapping from Unraid importance levels to Zentik delivery types is logical, and the default case provides a safe fallback for unexpected values.
82-88: LGTM! Robust user IDs processing.The pipeline correctly handles comma-separated user IDs, trims whitespace, filters empty entries, and converts to a JSON array or null if empty.
90-104: LGTM! Proper JSON payload construction with error handling.Using
jqto build the payload ensures proper JSON escaping, and the exit status check catches any construction errors.
ServerUrl building reordered
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
emhttp/plugins/dynamix/agents/ZentikNotifier.xml (2)
51-53: Consider edge cases withecho -einterpretation.The
echo -ecommand interprets escape sequences in TITLE, SUBTITLE, and MESSAGE. While this may be intentional to support formatted input, it could cause unexpected behavior if notification fields contain literal backslashes or escape sequences like\n,\t, etc.If this interpretation is intentional, consider documenting it. Otherwise, use
echo "$VAR"without-eto avoid interpretation.
84-90: Consider documenting the complex USER_IDS parsing pipeline.The pipeline at line 88 correctly parses comma-separated user IDs into a JSON array, but it's fairly complex. Consider adding a brief comment explaining the transformation steps for future maintainability.
Example documentation
# userIds: optional comma-separated list USER_IDS_JSON="null" if [[ -n "$USER_IDS" && "$USER_IDS" != "none" ]]; then # Parse comma-separated list into JSON array: # 1. Split by comma, 2. Trim whitespace, 3. Drop empties, 4. Quote as JSON strings, 5. Collect into array USER_IDS_JSON=$(printf '%s\n' "$USER_IDS" | tr ',' '\n' | sed 's/^ *//; s/ *$//' | awk 'NF' | jq -R . | jq -s .) [[ "$USER_IDS_JSON" == "[]" ]] && USER_IDS_JSON="null" fi
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
emhttp/plugins/dynamix/agents/ZentikNotifier.xml(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-09-05T16:33:12.970Z
Learnt from: Squidly271
Repo: unraid/webgui PR: 2353
File: emhttp/plugins/dynamix/DashStats.page:2257-2257
Timestamp: 2025-09-05T16:33:12.970Z
Learning: In the Unraid webGUI Nchan system, publisher scripts (like "update_1") publish to normalized endpoint names (like "update1"). The Nchan header lists the script names to start, while JavaScript subscribes to the published endpoints. For example: Nchan="update_1" starts the script which calls publish_noDupe('update1', data), and JavaScript subscribes to '/sub/update1'. This is the intended design, not a mismatch.
Applied to files:
emhttp/plugins/dynamix/agents/ZentikNotifier.xml
📚 Learning: 2025-09-15T16:59:48.473Z
Learnt from: ljm42
Repo: unraid/webgui PR: 2364
File: emhttp/plugins/dynamix/agents/Discord.xml:63-66
Timestamp: 2025-09-15T16:59:48.473Z
Learning: In Unraid systems, NGINX_DEFAULTURL configuration only produces servername or servername.localTLD formats, never IPv4/IPv6 addresses. The dot-checking logic in Discord notification agent is specifically designed to filter out bare hostnames that Discord API won't accept.
Applied to files:
emhttp/plugins/dynamix/agents/ZentikNotifier.xml
🔇 Additional comments (9)
emhttp/plugins/dynamix/agents/ZentikNotifier.xml (9)
1-13: LGTM: Well-structured manifest with good security defaults.The XML manifest structure follows Dynamix agent conventions. Security defaults are appropriate with
ALLOW_INSECURE="no", and required credentials have no defaults, forcing explicit configuration.
32-48: LGTM: Good initialization with test-friendly defaults.The script properly initializes logging and provides sensible defaults for standalone testing. The fallback assignments for TITLE/SUBTITLE/MESSAGE handle edge cases gracefully.
56-66: LGTM: Clean importance-to-delivery-type mapping.The case statement properly maps Unraid importance levels to Zentik delivery types with a sensible default fallback.
71-78: LGTM: Proper validation order.Required variables are validated before use. This correctly addresses the previous review comment about validation ordering.
80-82: LGTM: Clean endpoint construction.The endpoint is constructed after validation, correctly stripping trailing slashes before appending the path.
92-106: LGTM: Secure JSON payload construction with proper error handling.The jq command properly uses
--argand--argjsonflags to safely construct the JSON payload, preventing injection issues. Error handling is appropriate.
118-118: Good: Conditional insecure SSL flag.The
-kflag is now properly gated behind theALLOW_INSECUREconfiguration variable, addressing the security concern from the previous review. This allows users to opt-in to insecure connections only when necessary.
120-127: Good: HTTP response validation added.The script now captures the HTTP response and validates the status code is in the 2xx range, properly addressing the previous review comment about response validation.
18-18: Verify the{0}placeholder usage.The
{0}placeholder appears to be a template variable. Ensure this is the correct placeholder format for Dynamix agent scripts and that the notification system properly substitutes common code here.#!/bin/bash # Description: Check other Dynamix agents for similar placeholder patterns # Search for {0} or similar placeholder patterns in other agent XML files fd -e xml . emhttp/plugins/dynamix/agents/ --exec grep -l '{[0-9]}' {}
| # POST | ||
| args=( | ||
| -s -X POST | ||
| -m 10 | ||
| -w "\n%{http_code}" | ||
| -H "Authorization: Bearer ${ACCESS_TOKEN}" | ||
| -H 'Content-Type: application/json' | ||
| --data-binary "$PAYLOAD" | ||
| "$ENDPOINT" | ||
| ) | ||
| [[ "${ALLOW_INSECURE}" == "yes" ]] && args+=( -k ) | ||
|
|
||
| RESPONSE=$(curl "${args[@]}" 2>&1) | ||
| HTTP_CODE=$(echo "$RESPONSE" | tail -n1) | ||
|
|
||
| # Check if HTTP status indicates success (2xx) | ||
| if [[ ! "$HTTP_CODE" =~ ^2[0-9]{2}$ ]]; then | ||
| echo "HTTP request failed with status $HTTP_CODE" >>"$LOG" | ||
| exit 1 | ||
| fi |
There was a problem hiding this comment.
Add curl exit code validation before parsing HTTP response.
The script captures the curl response but doesn't check curl's exit code. If curl fails due to network errors, DNS resolution failure, or timeout, the script won't properly detect it before attempting to parse the HTTP status code.
🔎 Proposed fix
[[ "${ALLOW_INSECURE}" == "yes" ]] && args+=( -k )
RESPONSE=$(curl "${args[@]}" 2>&1)
+CURL_EXIT=$?
+
+if [[ $CURL_EXIT -ne 0 ]]; then
+ echo "curl failed with exit code $CURL_EXIT" >>"$LOG"
+ exit 1
+fi
+
HTTP_CODE=$(echo "$RESPONSE" | tail -n1)
# Check if HTTP status indicates success (2xx)🤖 Prompt for AI Agents
In emhttp/plugins/dynamix/agents/ZentikNotifier.xml around lines 108 to 127, the
script runs curl and immediately parses the last line as HTTP status without
checking curl's exit code; capture curl's exit status (e.g., CURL_EXIT=$?)
immediately after the curl invocation and, if non-zero, write an error to the
log including the curl exit code and the raw curl output, then exit before
attempting to parse HTTP_CODE; only proceed to extract and validate HTTP_CODE
when curl succeeded.
Curl exit checked first
🔧 PR Test Plugin AvailableA test plugin has been generated for this PR that includes the modified files. Version: 📥 Installation Instructions:Install via Unraid Web UI:
Alternative: Direct Download
|
1 participant
This PR adds support to a new notification platform under development
https://notifier-docs.zentik.app/
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.