Do not pass params to TestHooks::ProjectService and use send method

Summary

The following discussion from !20898 (merged) should be addressed:

  • @Alexand started a discussion: (+3 comments)

    Just to confirm. Passing the trigger as a parameter could become a security concern, since we call self.__send__(trigger_data_method) here: https://gitlab.com/gitlab-org/gitlab/blob/master/app%2Fservices%2Ftest_hooks%2Fbase_service.rb#L22

    Although, we'd be safe against this threat since we only execute it agains a list of fixed possible methods here: https://gitlab.com/gitlab-org/gitlab/blob/master/app%2Fservices%2Ftest_hooks%2Fbase_service.rb#L14

    Is that correct?

Improvements

The params trigger is being passed to TestHooks::ProjectService which then uses send to dynamically call the correct method (scope). While this isn't a security concern at the moment - because we are only selecting from a predefined list of keys, it might cause issues further down the line and it's best if refactored.

Risks

Involved components

Optional: Intended side effects

Optional: Missing test coverage

Assignee Loading
Time tracking Loading