Add Notification: Apprise #2151
Conversation
WalkthroughA new XML agent configuration named "Apprise" has been added. This configuration introduces six variables for server URL, stateful key, tags, stateless URLs, title, and message. It includes an embedded Bash script that processes these variables, formats a JSON payload for notification delivery, and sends it to a specified server endpoint using a POST request. The script also handles normalization of input values and maps importance levels to priority strings. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant AppriseAgent (Bash Script)
participant AppriseServer
User->>AppriseAgent: Provide notification variables (SERVER_URL, STATEFUL_KEY, etc.)
AppriseAgent->>AppriseAgent: Normalize and process variables
AppriseAgent->>AppriseAgent: Map IMPORTANCE to PRIORITY
AppriseAgent->>AppriseAgent: Build JSON payload
AppriseAgent->>AppriseServer: POST /notify[/{STATEFUL_KEY}] with JSON payload
AppriseServer-->>AppriseAgent: Response
Poem
Tip ⚡💬 Agentic Chat (Pro Plan, General Availability)
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![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 (1)
emhttp/plugins/dynamix/agents/Apprise.xml (1)
40-43: Add error handling for curl failures.Currently, the script does not check if the curl command succeeds or fails. For better reliability and troubleshooting, capture the exit code and output an error message if the notification fails.
Example addition:
-curl -X POST \ - -H "Content-Type: application/json" \ - -d "$JSON" \ - $SERVER_URL/notify${STATEFUL_KEY:+/$STATEFUL_KEY} 2>&1 +response=$(curl -s -w "%{http_code}" -o /tmp/apprise_response.txt -X POST \ + -H "Content-Type: application/json" \ + -d "$JSON" \ + "$SERVER_URL/notify${STATEFUL_KEY:+/$STATEFUL_KEY}") +if [ "$response" -ne 200 ]; then + echo "Apprise notification failed with HTTP status $response" + cat /tmp/apprise_response.txt +fiThis helps with debugging and ensures failures are visible.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
emhttp/plugins/dynamix/icons/apprise.pngis excluded by!**/*.png
📒 Files selected for processing (1)
emhttp/plugins/dynamix/agents/Apprise.xml(1 hunks)
🔇 Additional comments (1)
emhttp/plugins/dynamix/agents/Apprise.xml (1)
2-11: XML structure and variable definitions are clear and comprehensive.The agent definition and variable descriptions are well-written and provide clear guidance to users configuring Apprise notifications. Default values and help texts are appropriate.
| JSON="{\"urls\":\"$STATELESS_URLS\",\"tag\":\"$TAGS\",\"type\":\"$PRIORITY\",\"title\":\"$TITLE\",\"body\":\"$MESSAGE\"}" | ||
|
|
||
| curl -X POST \ | ||
| -H "Content-Type: application/json" \ | ||
| -d "$JSON" \ | ||
| $SERVER_URL/notify${STATEFUL_KEY:+/$STATEFUL_KEY} 2>&1 |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Potential JSON injection and quoting issues in payload construction.
- The JSON payload is constructed via string interpolation, which is unsafe if any variable contains quotes, backslashes, or newlines. This can break the JSON structure and potentially cause notification failures or security issues.
- Consider using a tool like
jq -norprintf '%s'with proper escaping to build the JSON payload safely.
Example fix using jq (if available in the environment):
-JSON="{\"urls\":\"$STATELESS_URLS\",\"tag\":\"$TAGS\",\"type\":\"$PRIORITY\",\"title\":\"$TITLE\",\"body\":\"$MESSAGE\"}"
+JSON=$(jq -n \
+ --arg urls "$STATELESS_URLS" \
+ --arg tag "$TAGS" \
+ --arg type "$PRIORITY" \
+ --arg title "$TITLE" \
+ --arg body "$MESSAGE" \
+ '{urls: $urls, tag: $tag, type: $type, title: $title, body: $body}')If jq is not available, at minimum, escape double quotes and backslashes in variables before constructing the JSON string.
- Additionally, consider capturing the exit code of
curland logging or handling errors for better robustness.
Would you like a ready-to-use Bash snippet for safe JSON construction without jq?
📝 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.
| JSON="{\"urls\":\"$STATELESS_URLS\",\"tag\":\"$TAGS\",\"type\":\"$PRIORITY\",\"title\":\"$TITLE\",\"body\":\"$MESSAGE\"}" | |
| curl -X POST \ | |
| -H "Content-Type: application/json" \ | |
| -d "$JSON" \ | |
| $SERVER_URL/notify${STATEFUL_KEY:+/$STATEFUL_KEY} 2>&1 | |
| JSON=$(jq -n \ | |
| --arg urls "$STATELESS_URLS" \ | |
| --arg tag "$TAGS" \ | |
| --arg type "$PRIORITY" \ | |
| --arg title "$TITLE" \ | |
| --arg body "$MESSAGE" \ | |
| '{urls: $urls, tag: $tag, type: $type, title: $title, body: $body}') | |
| curl -X POST \ | |
| -H "Content-Type: application/json" \ | |
| -d "$JSON" \ | |
| $SERVER_URL/notify${STATEFUL_KEY:+/$STATEFUL_KEY} 2>&1 |
| #!/bin/bash | ||
| ############ | ||
| {0} | ||
| ############ | ||
| MESSAGE=$(echo -e "$MESSAGE") | ||
| case "$IMPORTANCE" in | ||
| 'normal' ) | ||
| PRIORITY="info" | ||
| ;; | ||
| 'warning' ) | ||
| PRIORITY="warning" | ||
| ;; | ||
| 'alert' ) | ||
| PRIORITY="failure" | ||
| ;; | ||
| esac |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Bash script logic is clear, but input handling and quoting could be improved.
- The mapping of IMPORTANCE to PRIORITY is correct and clear.
- However, the script does not quote variable expansions (e.g.,
$STATEFUL_KEY,$STATELESS_URLS,$TAGS) in theifstatements, which can cause errors if variables contain spaces or are empty. - Consider quoting all variable expansions to prevent word splitting and unexpected behavior.
Example fix:
-if [ $STATEFUL_KEY == 'none' ]; then STATEFUL_KEY=; fi
-if [ $STATELESS_URLS == 'none' ]; then STATELESS_URLS=; fi
-if [ $TAGS == 'none' ]; then TAGS=; fi
+if [ "$STATEFUL_KEY" == 'none' ]; then STATEFUL_KEY=; fi
+if [ "$STATELESS_URLS" == 'none' ]; then STATELESS_URLS=; fi
+if [ "$TAGS" == 'none' ]; then TAGS=; fiCommittable suggestion skipped: line range outside the PR's diff.
So this agent has a dependency of the apprise container being installed? (or another server running a apprise?) Problem with the dependency, especially if its on the same server as a container would be that if something happens to the docker image / container, or if the array is stopped or docker system (or container) is stopped then notifications would be not be sent through which could prove to be troublesome. Not my decision, but why not instead have it installed outside of the OS via a plugin (see https://raw.githubusercontent.com/Squidly271/Wxwork-sample/main/wxwork-sample.plg for how to do this) along with an appropriate xml for CA to be able to include it. (FWIW, I love Apprise! Just wish that I could have managed to get it to do text messages without paying for it) |
|
Hi @Squidly271 - appreciate the comment, I'll go take a look at the plugin url you provided. Just trying to understand... As far as I can tell, this works the same as any of the other notification services, it just makes a curl request to a server with the message payload, doesn't necessarily have to be running on unraid. There's no real unraid os dependency, maybe I should have said apprise-api, since that's the rest api to apprise - [https://github.com/caronc/apprise-api]. For example, I'm running the gotify server on my unraid and using the gotify notification service which is already one of the options. I simply copied it's xml and tweaked it a bit. This just keeps the notification in the same place as all of the others and uses the same framework. I've seen a request for support for apprise in several threads on the forum and since I wanted it too just took a stab at it. Thanks! |
|
OK. Just as soon as I saw you mention the apprise container then I had concerns. But as you correctly noted (and reminded me of) is that other agents (not all of them) also have requirements for a container / executable to be operating in a container or on a separate server. I'm good either way. That being said, the plugin route to get the xml into the system has as an advantage that it works on OS v 7.0.0 / 7.0.1 (this PR will only take effect if merged for 7.1.0) and if for some reason the API being leveraged has a breaking change / additional options etc then via the plg route modifications can be easily made instead of the changes required in the xml only taking effect going forward. But, you should also consider the code modifications that coderabbit suggested here https://github.com/unraid/webgui/pull/2151/files#r2044656448 |
|
Hi @kelliott72 Code Rabbit made a few suggestions that I think would be worth implementing here |
3 participants
New Feature:
Add support for notifications using Apprise-api, which supports over 100 notification types and has an Unraid app docker.
Summary by CodeRabbit