Skip to content

Private project todos are not deleted after a user leaves a public group

Update:

https://gitlab.com/gitlab-org/security/gitlab/-/merge_requests/1494 prevents these todos from being visible to the user, so the security vulnerability here is resolved. However, it would be good to clean this up for data integrity and to make sure that we don't regress on this vulnerability.

Similar to https://gitlab.com/gitlab-org/gitlab/-/issues/325421, some todos are not deleted after leaving a public group.

When a user leaves a public group, we call enqueue_private_features_worker which eventually calls Todos::Destroy::PrivateFeaturesService for each sub-project in this group where the user is not a reporter.

This service just checks for the feature's availability (if it's set to "Project Members Only", "Everyone with access", etc..) but this setting only makes sense for internal / public projects. So if it's set to "Project Members Only", the todos are removed.

The problem is that with a newly created private project, issues_access_level and merge_requests_access_level are set to ENABLED which is equivalent to "Everyone with access". (In the UI, it is displayed as "Project Members Only").

In other places like the issue list, this does not cause a leak because we check project access first before checking feature availability.

To reproduce

  1. Create a public group and a private project under it
  2. Create an issue in the private project
  3. Invite a user into the public group (any role)
  4. Now that the user is invited, they can see the private project. Using this invited user, add a todo to the issue in this project
  5. As the admin, remove this user from the public group
  6. Wait 1 hour (because we delete todos 1 hour after membership removal) or manually execute Todos::Destroy::EntityLeaveService.new(user.id, group.id, 'Group').execute
  7. Even after the service is run, and the corresponding PrivateFeaturesWorker jobs finish, the todo on the issue is still visible to the user and leaks the issue title

This also applies to todos on merge requests

But if the project is private, this setting is not available and we just set this to `

Edited by Jake Lear