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

fix(email): don't escape strings in plaintext emails #76476

Draft
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

keeakita
Copy link
Member

Fixes #76475

Copy link

codecov bot commented Aug 21, 2024

❌ 5 Tests Failed:

Tests completed Failed Passed Skipped
21655 5 21650 202
View the top 3 failed tests by shortest run time
tests.sentry.users.models.test_lostpasswordhash.LostPasswordTest test_send_relocation_mail
Stack Traces | 3.58s run time
#x1B[1m#x1B[.../users/models/test_lostpasswordhash.py#x1B[0m:43: in test_send_relocation_mail
    assert msg.body.startswith(
#x1B[1m#x1B[31mE   AssertionError: assert False#x1B[0m
#x1B[1m#x1B[31mE    +  where False = <built-in method startswith of SafeString object at 0x7f0865e3a7a0>('The following Sentry organizations that you are a member of have been migrated onto sentry.io:\n\n* testorg\n\n\nTo continue with using these accounts at their new location, please claim your account with sentry.io.\n\nClaim Account')#x1B[0m
#x1B[1m#x1B[31mE    +    where <built-in method startswith of SafeString object at 0x7f0865e3a7a0> = '\nThe following Sentry organizations that you are a member of have been migrated onto sentry.io:\n\n* testorg\n\n\nTo...h sentry.io.\n\nClaim Account (http:.../confirm/856/MIoq2oHvz6zFTvKJ5XVdQHCRgbbbRvc4/)\n\n'.startswith#x1B[0m
#x1B[1m#x1B[31mE    +      where '\nThe following Sentry organizations that you are a member of have been migrated onto sentry.io:\n\n* testorg\n\n\nTo...h sentry.io.\n\nClaim Account (http:.../confirm/856/MIoq2oHvz6zFTvKJ5XVdQHCRgbbbRvc4/)\n\n' = <django.core.mail.message.EmailMultiAlternatives object at 0x7f08657aa210>.body#x1B[0m
tests.sentry.tasks.test_weekly_reports.WeeklyReportsTest test_integration
Stack Traces | 4.8s run time
#x1B[1m#x1B[31m.venv/lib/python3.11.../django/template/base.py#x1B[0m:502: in parse
    compile_func = self.tags[command]
#x1B[1m#x1B[31mE   KeyError: 'endautoescape'#x1B[0m

#x1B[33mDuring handling of the above exception, another exception occurred:#x1B[0m
#x1B[1m#x1B[.../sentry/tasks/base.py#x1B[0m:165: in wrapped
    return func(*args, **kwargs)
#x1B[1m#x1B[.../tasks/summaries/weekly_reports.py#x1B[0m:242: in prepare_organization_report
    batch.deliver_reports()
#x1B[1m#x1B[.../tasks/summaries/weekly_reports.py#x1B[0m:289: in deliver_reports
    self._send_to_user(user_template)
#x1B[1m#x1B[.../tasks/summaries/weekly_reports.py#x1B[0m:297: in _send_to_user
    self.send_email(template_ctx=template_context, user_id=user_id)
#x1B[1m#x1B[.../tasks/summaries/weekly_reports.py#x1B[0m:335: in send_email
    message.send_async()
#x1B[1m#x1B[.../utils/email/message_builder.py#x1B[0m:241: in send_async
    messages = self.get_built_messages(to, cc=cc, bcc=bcc)
#x1B[1m#x1B[.../utils/email/message_builder.py#x1B[0m:207: in get_built_messages
    results = [
#x1B[1m#x1B[.../utils/email/message_builder.py#x1B[0m:208: in <listcomp>
    self.build(to=email, reply_to=send_to, cc=cc, bcc=bcc) for email in send_to if email
#x1B[1m#x1B[.../utils/email/message_builder.py#x1B[0m:185: in build
    body=self.__render_text_body(),
#x1B[1m#x1B[.../utils/email/message_builder.py#x1B[0m:136: in __render_text_body
    body: str = render_to_string(self.template, self.context)
#x1B[1m#x1B[.../sentry/web/helpers.py#x1B[0m:29: in render_to_string
    rendered = loader.render_to_string(template, context=context, request=request)
#x1B[1m#x1B[31m.venv/lib/python3.11.../django/template/loader.py#x1B[0m:61: in render_to_string
    template = get_template(template_name, using=using)
#x1B[1m#x1B[31m.venv/lib/python3.11.../django/template/loader.py#x1B[0m:15: in get_template
    return engine.get_template(template_name)
#x1B[1m#x1B[31m.venv/lib/python3.11.../template/backends/django.py#x1B[0m:33: in get_template
    return Template(self.engine.get_template(template_name), self)
#x1B[1m#x1B[31m.venv/lib/python3.11.../django/template/engine.py#x1B[0m:177: in get_template
    template, origin = self.find_template(template_name)
#x1B[1m#x1B[31m.venv/lib/python3.11.../django/template/engine.py#x1B[0m:159: in find_template
    template = loader.get_template(name, skip=skip)
#x1B[1m#x1B[31m.venv/lib/python3.11.../template/loaders/cached.py#x1B[0m:57: in get_template
    template = super().get_template(template_name, skip)
#x1B[1m#x1B[31m.venv/lib/python3.11.../template/loaders/base.py#x1B[0m:28: in get_template
    return Template(
#x1B[1m#x1B[31m.venv/lib/python3.11.../django/template/base.py#x1B[0m:154: in __init__
    self.nodelist = self.compile_nodelist()
#x1B[1m#x1B[31m.venv/lib/python3.11.../django/template/base.py#x1B[0m:196: in compile_nodelist
    return parser.parse()
#x1B[1m#x1B[31m.venv/lib/python3.11.../django/template/base.py#x1B[0m:504: in parse
    self.invalid_block_tag(token, command, parse_until)
#x1B[1m#x1B[31m.venv/lib/python3.11.../django/template/base.py#x1B[0m:565: in invalid_block_tag
    raise self.error(
#x1B[1m#x1B[31mE   django.template.exceptions.TemplateSyntaxError: Invalid block tag on line 1: 'endautoescape'. Did you forget to register or load this tag?#x1B[0m

#x1B[33mDuring handling of the above exception, another exception occurred:#x1B[0m
#x1B[1m#x1B[.../sentry/tasks/test_weekly_reports.py#x1B[0m:102: in test_integration
    schedule_organizations(timestamp=self.now.timestamp())
#x1B[1m#x1B[.../sentry/silo/base.py#x1B[0m:148: in override
    return original_method(*args, **kwargs)
#x1B[1m#x1B[31m.venv/lib/python3.11.../site-packages/celery/local.py#x1B[0m:182: in __call__
    return self._get_current_object()(*a, **kw)
#x1B[1m#x1B[31m.venv/lib/python3.11.../celery/app/task.py#x1B[0m:411: in __call__
    return self.run(*args, **kwargs)
#x1B[1m#x1B[.../sentry/silo/base.py#x1B[0m:148: in override
    return original_method(*args, **kwargs)
#x1B[1m#x1B[.../sentry/tasks/base.py#x1B[0m:128: in _wrapped
    result = func(*args, **kwargs)
#x1B[1m#x1B[.../sentry/tasks/base.py#x1B[0m:172: in wrapped
    current_task.retry(exc=exc)
#x1B[1m#x1B[.../sentry/silo/base.py#x1B[0m:148: in override
    return original_method(*args, **kwargs)
#x1B[1m#x1B[31m.venv/lib/python3.11.../celery/app/task.py#x1B[0m:720: in retry
    raise_with_context(exc or Retry('Task can be retried', None))
#x1B[1m#x1B[.../sentry/tasks/base.py#x1B[0m:165: in wrapped
    return func(*args, **kwargs)
#x1B[1m#x1B[.../tasks/summaries/weekly_reports.py#x1B[0m:88: in schedule_organizations
    prepare_organization_report.delay(
#x1B[1m#x1B[.../sentry/silo/base.py#x1B[0m:148: in override
    return original_method(*args, **kwargs)
#x1B[1m#x1B[31msrc/sentry/celery.py#x1B[0m:101: in delay
    return super().delay(*args, **kwargs)
#x1B[1m#x1B[31m.venv/lib/python3.11.../celery/app/task.py#x1B[0m:444: in delay
    return self.apply_async(args, kwargs)
#x1B[1m#x1B[.../sentry/silo/base.py#x1B[0m:148: in override
    return original_method(*args, **kwargs)
#x1B[1m#x1B[31msrc/sentry/celery.py#x1B[0m:114: in apply_async
    return Task.apply_async(self, *args, **kwargs)
#x1B[1m#x1B[.../testutils/pytest/stale_database_reads.py#x1B[0m:157: in apply_async
    return old_apply_async(self, args, kwargs, **options)
#x1B[1m#x1B[31m.venv/lib/python3.11........./site-packages/sentry_sdk/utils.py#x1B[0m:1718: in runner
    return original_function(*args, **kwargs)
#x1B[1m#x1B[31m.venv/lib/python3.11.../celery/app/task.py#x1B[0m:591: in apply_async
    return self.apply(args, kwargs, task_id=task_id or uuid(),
#x1B[1m#x1B[.../sentry/silo/base.py#x1B[0m:148: in override
    return original_method(*args, **kwargs)
#x1B[1m#x1B[31m.venv/lib/python3.11.../celery/app/task.py#x1B[0m:819: in apply
    ret = tracer(task_id, args, kwargs, request)
#x1B[1m#x1B[31m.venv/lib/python3.11........./site-packages/sentry_sdk/utils.py#x1B[0m:1718: in runner
    return original_function(*args, **kwargs)
#x1B[1m#x1B[31m.venv/lib/python3.11.../celery/app/trace.py#x1B[0m:490: in trace_task
    I, R, state, retval = on_error(
#x1B[1m#x1B[31m.venv/lib/python3.11.../celery/app/trace.py#x1B[0m:477: in trace_task
    R = retval = fun(*args, **kwargs)
#x1B[1m#x1B[31m.venv/lib/python3.11........./site-packages/sentry_sdk/utils.py#x1B[0m:1718: in runner
    return original_function(*args, **kwargs)
#x1B[1m#x1B[.../sentry/silo/base.py#x1B[0m:148: in override
    return original_method(*args, **kwargs)
#x1B[1m#x1B[.../sentry/tasks/base.py#x1B[0m:128: in _wrapped
    result = func(*args, **kwargs)
#x1B[1m#x1B[.../sentry/tasks/base.py#x1B[0m:172: in wrapped
    current_task.retry(exc=exc)
#x1B[1m#x1B[.../sentry/silo/base.py#x1B[0m:148: in override
    return original_method(*args, **kwargs)
#x1B[1m#x1B[31m.venv/lib/python3.11.../celery/app/task.py#x1B[0m:749: in retry
    raise ret
#x1B[1m#x1B[31mE   celery.exceptions.Retry: Retry in 180s: TemplateSyntaxError("Invalid block tag on line 1: 'endautoescape'. Did you forget to register or load this tag?")#x1B[0m
tests.sentry.tasks.test_weekly_reports.WeeklyReportsTest test_message_links_customer_domains
Stack Traces | 4.8s run time
#x1B[1m#x1B[31m.venv/lib/python3.11.../django/template/base.py#x1B[0m:502: in parse
    compile_func = self.tags[command]
#x1B[1m#x1B[31mE   KeyError: 'endautoescape'#x1B[0m

#x1B[33mDuring handling of the above exception, another exception occurred:#x1B[0m
#x1B[1m#x1B[.../sentry/tasks/base.py#x1B[0m:165: in wrapped
    return func(*args, **kwargs)
#x1B[1m#x1B[.../tasks/summaries/weekly_reports.py#x1B[0m:242: in prepare_organization_report
    batch.deliver_reports()
#x1B[1m#x1B[.../tasks/summaries/weekly_reports.py#x1B[0m:289: in deliver_reports
    self._send_to_user(user_template)
#x1B[1m#x1B[.../tasks/summaries/weekly_reports.py#x1B[0m:297: in _send_to_user
    self.send_email(template_ctx=template_context, user_id=user_id)
#x1B[1m#x1B[.../tasks/summaries/weekly_reports.py#x1B[0m:335: in send_email
    message.send_async()
#x1B[1m#x1B[.../utils/email/message_builder.py#x1B[0m:241: in send_async
    messages = self.get_built_messages(to, cc=cc, bcc=bcc)
#x1B[1m#x1B[.../utils/email/message_builder.py#x1B[0m:207: in get_built_messages
    results = [
#x1B[1m#x1B[.../utils/email/message_builder.py#x1B[0m:208: in <listcomp>
    self.build(to=email, reply_to=send_to, cc=cc, bcc=bcc) for email in send_to if email
#x1B[1m#x1B[.../utils/email/message_builder.py#x1B[0m:185: in build
    body=self.__render_text_body(),
#x1B[1m#x1B[.../utils/email/message_builder.py#x1B[0m:136: in __render_text_body
    body: str = render_to_string(self.template, self.context)
#x1B[1m#x1B[.../sentry/web/helpers.py#x1B[0m:29: in render_to_string
    rendered = loader.render_to_string(template, context=context, request=request)
#x1B[1m#x1B[31m.venv/lib/python3.11.../django/template/loader.py#x1B[0m:61: in render_to_string
    template = get_template(template_name, using=using)
#x1B[1m#x1B[31m.venv/lib/python3.11.../django/template/loader.py#x1B[0m:15: in get_template
    return engine.get_template(template_name)
#x1B[1m#x1B[31m.venv/lib/python3.11.../template/backends/django.py#x1B[0m:33: in get_template
    return Template(self.engine.get_template(template_name), self)
#x1B[1m#x1B[31m.venv/lib/python3.11.../django/template/engine.py#x1B[0m:177: in get_template
    template, origin = self.find_template(template_name)
#x1B[1m#x1B[31m.venv/lib/python3.11.../django/template/engine.py#x1B[0m:159: in find_template
    template = loader.get_template(name, skip=skip)
#x1B[1m#x1B[31m.venv/lib/python3.11.../template/loaders/cached.py#x1B[0m:57: in get_template
    template = super().get_template(template_name, skip)
#x1B[1m#x1B[31m.venv/lib/python3.11.../template/loaders/base.py#x1B[0m:28: in get_template
    return Template(
#x1B[1m#x1B[31m.venv/lib/python3.11.../django/template/base.py#x1B[0m:154: in __init__
    self.nodelist = self.compile_nodelist()
#x1B[1m#x1B[31m.venv/lib/python3.11.../django/template/base.py#x1B[0m:196: in compile_nodelist
    return parser.parse()
#x1B[1m#x1B[31m.venv/lib/python3.11.../django/template/base.py#x1B[0m:504: in parse
    self.invalid_block_tag(token, command, parse_until)
#x1B[1m#x1B[31m.venv/lib/python3.11.../django/template/base.py#x1B[0m:565: in invalid_block_tag
    raise self.error(
#x1B[1m#x1B[31mE   django.template.exceptions.TemplateSyntaxError: Invalid block tag on line 1: 'endautoescape'. Did you forget to register or load this tag?#x1B[0m

#x1B[33mDuring handling of the above exception, another exception occurred:#x1B[0m
#x1B[1m#x1B[.../sentry/tasks/test_weekly_reports.py#x1B[0m:152: in test_message_links_customer_domains
    schedule_organizations(timestamp=self.now.timestamp())
#x1B[1m#x1B[.../sentry/silo/base.py#x1B[0m:148: in override
    return original_method(*args, **kwargs)
#x1B[1m#x1B[31m.venv/lib/python3.11.../site-packages/celery/local.py#x1B[0m:182: in __call__
    return self._get_current_object()(*a, **kw)
#x1B[1m#x1B[31m.venv/lib/python3.11.../celery/app/task.py#x1B[0m:411: in __call__
    return self.run(*args, **kwargs)
#x1B[1m#x1B[.../sentry/silo/base.py#x1B[0m:148: in override
    return original_method(*args, **kwargs)
#x1B[1m#x1B[.../sentry/tasks/base.py#x1B[0m:128: in _wrapped
    result = func(*args, **kwargs)
#x1B[1m#x1B[.../sentry/tasks/base.py#x1B[0m:172: in wrapped
    current_task.retry(exc=exc)
#x1B[1m#x1B[.../sentry/silo/base.py#x1B[0m:148: in override
    return original_method(*args, **kwargs)
#x1B[1m#x1B[31m.venv/lib/python3.11.../celery/app/task.py#x1B[0m:720: in retry
    raise_with_context(exc or Retry('Task can be retried', None))
#x1B[1m#x1B[.../sentry/tasks/base.py#x1B[0m:165: in wrapped
    return func(*args, **kwargs)
#x1B[1m#x1B[.../tasks/summaries/weekly_reports.py#x1B[0m:88: in schedule_organizations
    prepare_organization_report.delay(
#x1B[1m#x1B[.../sentry/silo/base.py#x1B[0m:148: in override
    return original_method(*args, **kwargs)
#x1B[1m#x1B[31msrc/sentry/celery.py#x1B[0m:101: in delay
    return super().delay(*args, **kwargs)
#x1B[1m#x1B[31m.venv/lib/python3.11.../celery/app/task.py#x1B[0m:444: in delay
    return self.apply_async(args, kwargs)
#x1B[1m#x1B[.../sentry/silo/base.py#x1B[0m:148: in override
    return original_method(*args, **kwargs)
#x1B[1m#x1B[31msrc/sentry/celery.py#x1B[0m:114: in apply_async
    return Task.apply_async(self, *args, **kwargs)
#x1B[1m#x1B[.../testutils/pytest/stale_database_reads.py#x1B[0m:157: in apply_async
    return old_apply_async(self, args, kwargs, **options)
#x1B[1m#x1B[31m.venv/lib/python3.11........./site-packages/sentry_sdk/utils.py#x1B[0m:1718: in runner
    return original_function(*args, **kwargs)
#x1B[1m#x1B[31m.venv/lib/python3.11.../celery/app/task.py#x1B[0m:591: in apply_async
    return self.apply(args, kwargs, task_id=task_id or uuid(),
#x1B[1m#x1B[.../sentry/silo/base.py#x1B[0m:148: in override
    return original_method(*args, **kwargs)
#x1B[1m#x1B[31m.venv/lib/python3.11.../celery/app/task.py#x1B[0m:819: in apply
    ret = tracer(task_id, args, kwargs, request)
#x1B[1m#x1B[31m.venv/lib/python3.11........./site-packages/sentry_sdk/utils.py#x1B[0m:1718: in runner
    return original_function(*args, **kwargs)
#x1B[1m#x1B[31m.venv/lib/python3.11.../celery/app/trace.py#x1B[0m:490: in trace_task
    I, R, state, retval = on_error(
#x1B[1m#x1B[31m.venv/lib/python3.11.../celery/app/trace.py#x1B[0m:477: in trace_task
    R = retval = fun(*args, **kwargs)
#x1B[1m#x1B[31m.venv/lib/python3.11........./site-packages/sentry_sdk/utils.py#x1B[0m:1718: in runner
    return original_function(*args, **kwargs)
#x1B[1m#x1B[.../sentry/silo/base.py#x1B[0m:148: in override
    return original_method(*args, **kwargs)
#x1B[1m#x1B[.../sentry/tasks/base.py#x1B[0m:128: in _wrapped
    result = func(*args, **kwargs)
#x1B[1m#x1B[.../sentry/tasks/base.py#x1B[0m:172: in wrapped
    current_task.retry(exc=exc)
#x1B[1m#x1B[.../sentry/silo/base.py#x1B[0m:148: in override
    return original_method(*args, **kwargs)
#x1B[1m#x1B[31m.venv/lib/python3.11.../celery/app/task.py#x1B[0m:749: in retry
    raise ret
#x1B[1m#x1B[31mE   celery.exceptions.Retry: Retry in 180s: TemplateSyntaxError("Invalid block tag on line 1: 'endautoescape'. Did you forget to register or load this tag?")#x1B[0m

To view individual test run time comparison to the main branch, go to the Test Analytics Dashboard

@keeakita
Copy link
Member Author

Ugh, yeah, this was what I was afraid of. This breaks getsentry because of how django.contrib.humanize is loaded.

I'm just going to go through all the plaintext email templates and turn off autoescape.

@keeakita keeakita changed the title fix(email): add tests for email character escaping fix(email): don't escape strings in plaintext emails Aug 22, 2024
@keeakita
Copy link
Member Author

Ah, so there's a core issue with our email acceptance tests!

Here's what's up:

  • My changes turn autoescaping off for rendering plaintext emails. This makes sense because plaintext emails are, well, plaintext! We shouldn't escape anything.
  • However, we have some demo endpoints that render these emails inside an HTML page for debugging and acceptance tests. Handy! Except since these are HTML pages, I need to escape all the stuff I just unescaped! So I fixed that, but
  • Our acceptance tests are coded to check for the unescaped versions! Which makes sense, because we want to make sure the emails are rending without escaping. But it loads it from an HTML page, which should be escaped!

I think the real answer here is that instead of rendering an HTML page, we should render a text/plain response and use that. This will change our chromedriver code though. I will take a stab at this today.

@getsantry
Copy link
Contributor

getsantry bot commented Sep 14, 2024

This pull request has gone three weeks without activity. In another week, I will close it.

But! If you comment or otherwise update it, I will reset the clock, and if you add the label WIP, I will leave it alone unless WIP is removed ... forever!


"A weed is but an unloved flower." ― Ella Wheeler Wilcox 🥀

@getsantry getsantry bot added Stale and removed Stale labels Sep 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Scope: Backend Automatically applied to PRs that change backend components
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Plaintext emails rendered from templates contain escaping
1 participant