Skip to content

Validate regex before enqueuing a CleanupContainerRepositoryWorker job

What does this MR do?

As described in the description of #216088 (closed), the CleanupContainerRepositoryWorker is running way too many times with invalid regex (that are received as parameters).

This MR aims to reduce and even eliminate the situations where a CleanupContainerRepositoryWorker job is enqueued with invalid regex.

The CleanupContainerRepositoryWorker can be called by two components:

  1. By ContainerExpirationPolicyService, when executing automatically container expiration policies.
  2. By API::ProjectContainerRepositories when the api receives a request to delete tags in bulk.

There is also a 3rd situation where a CleanupContainerRepositoryWorker job with invalid regex raise a validation Error -> the job is re enqueued.

To prevent these scenarios, 3 guards have been implemented:

  1. The ContainerExpirationPolicyService will now enqueue a CleanupContainerRepositoryWorker job only if the given container expiration policy is valid. In addition, if the container expiration is not valid, it will be disabled as we will don't need to schedule the next run on it.
  2. A Grape API validator based on Gitlab::UntrustedRegexp has been added. This validator has been added to API::ProjectContainerRepositories so that Grape API will automatically discard any request with invalid regex.
  3. The service used by the CleanupContainerRepositoryWorker will now return an error response upon a regex validation error instead of raising an error which would bubble up and make the worker job fail (and re enqueue).

Screenshots

Does this MR meet the acceptance criteria?

Conformity

Availability and Testing

Security

If this MR contains changes to processing or storing of credentials or tokens, authorization and authentication methods and other items described in the security review guidelines:

  • Label as security and @ mention @gitlab-com/gl-security/appsec
  • The MR includes necessary changes to maintain consistency between UI, API, email, or other methods
  • Security reports checked/validated by a reviewer from the AppSec team
Edited by David Fernandez

Merge request reports