Skip to content

Diffs included in email via Code Review comments can reveal sensitive information

IMPORTANT NOTE: Please see the additional details for information on what the delivery of this issue will and will solve.

Problem to solve

When performing a Code Review via a merge request and leaving a comment on a line of code GitLab includes a few lines of the diff in the email notification to participants. Some organizational policies treat email as a less secure system or may not control their own infrastructure for email which presents risks to IP or access control of source code.

Further details

GitLab can expose information contained within a repository through email when commenting on code. Commenting on code, for the purposes of this issue, means:

  1. Commenting on a line of code within a merge request
  2. Commenting on a line of code within a commit

Proposal

A new setting should be introduced that disables the GitLab generated diff (code) from being present in an email.

Example of how this would change emails on Code Review as below:

Before After
Screenshot_2023-01-26_at_3.02.36_PM Screenshot_2023-01-26_at_3.03.36_PM

Additional Details

This proposal will only cover GitLab Generated diffs in emails related to merge requests and commits. Users will still receive code or pieces of code in email for things like (non-exhaustive list):

  • Comments that contain a suggestion
  • Comments that contain a code block
  • Comments that contain a .patch or other attachment that could be code

Essentially, the scope of this setting will not cover user generated content which may/may-not be code. Example:

Screenshot_2023-01-19_at_9.39.26_AM

If you are seeking a solution to make sure no code is transmitted via email, this issue will not solve that completely. Consider #389491

Design notes

  • New header has been added: Email notifications
    • Disable email notifications has been moved into this new section
  • Disable email notifications has been changed to Enable email notifications
    • This should default to enabled
    • When Enable email notifications is set to unchecked, then the nested include diff setting should becom unchecked and disabled
  • Group setting should cascade to sub-groups and projects

Availability & Testing

Recommend unit tests to check snippet will be contained/not contained in email notification. If unable to be tested as a Unit Test this could be a candidate for an additional E2E test to ensure that diffs are shown/not shown in email notifications dependant on the setting. Test may have to be limited to non-live environments and would require the use of MailHog to simulate an SMTP server.

Customers

Edited by Kai Armstrong