Skip to content
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

WAF whitelist XSS_BODY for logo upload #6210

Merged
merged 3 commits into from
Jan 29, 2024

Conversation

akashkj
Copy link
Contributor

@akashkj akashkj commented Jan 23, 2024

Add WAF rule for project basic settings in which a custom logo is uploaded.

Ticket: https://dimagi-dev.atlassian.net/browse/USH-4025
Commcare-HQ PR: dimagi/commcare-hq#34017

Environments Affected

Production, India and Staging

@@ -55,5 +56,5 @@
""".strip().split('\n')

COMMCAREHQ_XML_QUERYSTRING_URLS_REGEX = """
Copy link
Member

Choose a reason for hiding this comment

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

I would suggest fixing the lint error not by escaping the \ within the string, but by making this an r-string (starting with r""") as described here https://docs.python.org/3/library/re.html:

The solution is to use Python’s raw string notation for regular expression patterns; backslashes are not handled in any special way in a string literal prefixed with 'r'. So r"\n" is a two-character string containing '' and 'n', while "\n" is a one-character string containing a newline. Usually patterns will be expressed in Python code using this raw string notation.

That's also how the block of regular expressions above is defined.

@dannyroberts
Copy link
Member

dannyroberts commented Jan 23, 2024

@akashkj can you update the PR description to include a ticket, list the environments it affects ("saas aws environments"), and remove the section about announcing a new release (since there's no announcement needed with this one because it doesn't affect third parties)?

Make sure also to link it to the related PR in commcare-hq.

@dannyroberts
Copy link
Member

Besides my two comments, this looks good to me. Ping me when it's ready for re-review

@akashkj akashkj requested a review from dannyroberts January 24, 2024 16:26
@akashkj
Copy link
Contributor Author

akashkj commented Jan 29, 2024

@dannyroberts Other(hq) branch is merged.

@dannyroberts dannyroberts merged commit edcc33e into master Jan 29, 2024
3 checks passed
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