-
Notifications
You must be signed in to change notification settings - Fork 11
Python client: How to sanitise personalisation in send_email request #335
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -332,9 +332,62 @@ Hello ((name)) | |
| <small>Dear Anne Example, now <a href="https://malicious.link>click this evil link">click this evil link</a></small> | ||
| </pre> | ||
|
|
||
|
|
||
| #### Sanitise placeholder content | ||
| We recommend you sanitise all input from untrusted sources to prevent the injection of malicious content. | ||
| You can use a backslash to escape [individual characters](https://www.markdownguide.org/basic-syntax/#characters-you-can-escape). | ||
|
|
||
| To escape all Markdown characters, and remove any URLs you can: | ||
|
|
||
| ##### 1. Tell us which personalisation to sanitise | ||
| When sending emails via API, pass in the `sanitise_content_for` argument, and choose which personalisation you want to sanitise. | ||
|
|
||
| For all personalisation you tell us to sanitise, we will: | ||
|
|
||
| * escape all Markdown characters | ||
| * remove any URLs from the text | ||
| * tell you what content we’ve sanitised | ||
|
|
||
| For example, when you make the following API call: | ||
|
|
||
| ```python | ||
| response = notifications_client.send_email_notification( | ||
| email_address="amala@example.com", | ||
| template_id="9d751e0e-f929-4891-82a1-a3e1c3c18ee3", | ||
| "personalisation": { | ||
| "name": "user name from a form" | ||
| "sign_up_link": "link generated in your system" | ||
| }, | ||
| "sanitise_content_for": ["name"] | ||
| ) | ||
| ``` | ||
|
|
||
| Notify will sanitise the specificed placeholders, like this: | ||
|
|
||
| ```python | ||
| # malicious markdown: | ||
| name = "Anne Example, now [click this evil link](https://evil.link)" | ||
|
|
||
| # gets sanitised to: | ||
| "Anne Example, now \[click this evil link\]\(\)" | ||
|
|
||
| # email will appear as: | ||
| Anne Example, now [click this evil link]() | ||
|
Comment on lines
+372
to
+373
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This should be outside the code block (in a blockquote) because it’s not code
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. we had it like that at the beginning, it looked harder to read. Looks like you have quite a few comments regarding content as such - this PR is just one of several PRs implementing the content that was previously reviewed (three of those have already been merged). Feels like if we want to change the content, it would be good to do it in a separate PR in all docs where it's been implemented, so we maintain consistency?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. (although I already committed the more minor changes)
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do you have a link to that discussion/draft?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. yeah the draft is here - not sure if google docs show discussion history: https://docs.google.com/document/d/1kehXL5m3LTJeqAmnlxW3C8Z-D2VBuI4QgvQD5oJ7pOM/edit?usp=sharing To be fair the review happened back in the winter. But yeah - for bigger changes, I'd say lets have a chat there, and if we decide to pursue them, lets implement in all docs at once? And then this PR could be merged ahead of that discussion, so it could accompany the Python client changes. |
||
| ``` | ||
|
|
||
| The response will include the content that we’ve sanitised: | ||
|
|
||
| ```python | ||
| { | ||
| "sanitised_content": { | ||
| "name": { | ||
| "unsanitised": "Anne Example, now [click this evil link](https://evil.link)" | ||
| "sanitised": "Anne Example, now \[click this evil link\]\(\)\" | ||
| } | ||
| } | ||
| } | ||
| ``` | ||
|
|
||
| ##### 2. Use a backslash | ||
| Add this in front of [individual Markdown characters](https://www.markdownguide.org/basic-syntax/#characters-you-can-escape) to escape them. | ||
| The characters of most concern are those that could be used to add a URL link such as `[`, `]`, `(` or `)`. | ||
|
|
||
|
|
||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.