Skip to content

Implement message sanitization for Aliyun SMS#6605

Merged
CommanderStorm merged 3 commits intolouislam:masterfrom
IsayIsee:my_dev
Jan 6, 2026
Merged

Implement message sanitization for Aliyun SMS#6605
CommanderStorm merged 3 commits intolouislam:masterfrom
IsayIsee:my_dev

Conversation

@IsayIsee
Copy link
Copy Markdown
Contributor

@IsayIsee IsayIsee commented Jan 6, 2026

ℹ️ To keep reviews fast and effective, please make sure you’ve read our pull request guidelines

📝 Summary of changes done and why they are done

Added a method to remove IP addresses and domains from SMS messages to comply with Aliyun SMS restrictions.

📋 Related issues

📄 Checklist

Please follow this checklist to avoid unnecessary back and forth (click to expand)
  • ⚠️ If there are Breaking change (a fix or feature that alters existing functionality in a way that could cause issues) I have called them out
  • 🧠 I have disclosed any use of LLMs/AI in this contribution and reviewed all generated content.
    I understand that I am responsible for and able to explain every line of code I submit.
  • 🔍 My code adheres to the style guidelines of this project.
  • ⚠️ My changes generate no new warnings.
  • 🛠️ I have reviewed and tested my code.
  • 📝 I have commented my code, especially in hard-to-understand areas (e.g., using JSDoc for methods).
  • 🤖 I added or updated automated tests where appropriate.
  • 📄 Documentation updates are included (if applicable).
  • 🔒 I have considered potential security impacts and mitigated risks.
  • 🧰 Dependency updates are listed and explained.

IsayIsee and others added 3 commits January 6, 2026 17:21
Added a method to remove IP addresses and domains from SMS messages to comply with Aliyun SMS restrictions.
Copy link
Copy Markdown
Collaborator

@CommanderStorm CommanderStorm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wtf? Why do they do that??
Isn't the whole point of a notification to get the notification out??

I guess LGTM. I would recommend using a different notification provider then though.

@CommanderStorm CommanderStorm merged commit 0adf20f into louislam:master Jan 6, 2026
23 checks passed
@CommanderStorm CommanderStorm added this to the 2.1.0 milestone Jan 6, 2026
@IsayIsee
Copy link
Copy Markdown
Contributor Author

IsayIsee commented Jan 7, 2026

Wtf? Why do they do that?? Isn't the whole point of a notification to get the notification out??

I guess LGTM. I would recommend using a different notification provider then though.

To comply with the carrier's anti-fraud requirements.
I performed additional testing today. While replacing URLs and other sensitive information met Alibaba Cloud's compliance requirements, messages still undergo secondary keyword screening by the carriers. Their keyword filters are more stringent, causing message delivery failures and triggering carrier blacklist mechanisms. Repeated keyword violations may result in permanent blacklisting. Therefore, I propose making ${msg} an optional parameter, as carrier-level keyword screening is ultimately unavoidable.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants