• Dylan Griffith's avatar
    Replace Vulnerabilities::Feedback.only_valid_feedback without joining ci · d73d10e1
    Dylan Griffith authored
    This method joins between `ci_*` and non `ci_*` tables and as such we
    need to replace this logic with something that does not do this join
    before we move `ci_*` tables to a different database.
    
    This method `Vulnerabilities::Feedback.only_valid_feedback` was
    introduced originally in
    https://gitlab.com/gitlab-org/security/gitlab/-/merge_requests/117 to
    solve a security vulnerability
    https://gitlab.com/gitlab-org/gitlab/-/issues/24932. The basic issue was
    that we had a controller that allowed you to create a
    `Vulnerability::Feedback` and set the `pipeline_id` of that. The problem
    is that for a window of time a user could have set the `pipeline_id` to
    be from a different project. We also had another controller that would
    present the `path` for a pipeline and as such you could learn the path
    of private projects by guessing the `pipeline_id` from other projects.
    
    The original fix used a `join ci_pipelines` to filter out any
    `Vulnerabilities::Feedback` that happened to be associated to another
    project. We can't do this join anymore. It seemed simpler to change the
    security fix to focus on not rendering the `pipeline` if the
    `project_id` was belonging to a different project. So instead we just
    don't return `pipeline` if the `project_id` is not what it should be
    (ie. they hacked the `pipeline_id` to be a different project). This may
    present some issue in the frontend but this data should only appear if
    the user in the project deliberately created this hack data so we
    shouldn't be concerned about the UX impact here.
    
    It's also possible this fix improves performance for this endpoint as it
    doesn't need to do a redundant join.
    
    Since this fixes the problem in a slightly different way it also
    required us to change the test slightly to assert the `pipeline` is
    filtered out now rather than the whole `Feedback` object.
    d73d10e1
feedback_spec.rb 10.6 KB