Skip to content

Next

  • Projects
  • Groups
  • Snippets
  • Help
    • Loading...
    • Help
    • Support
    • Submit feedback
    • Contribute to GitLab
  • Sign in / Register
GitLab FOSS
GitLab FOSS
  • Project overview
    • Project overview
    • Details
    • Activity
    • Releases
    • Cycle Analytics
    • Insights
  • Repository
    • Repository
    • Files
    • Commits
    • Branches
    • Tags
    • Contributors
    • Graph
    • Compare
    • Charts
    • Locked Files
  • Issues 0
    • Issues 0
    • List
    • Boards
    • Labels
    • Service Desk
    • Milestones
  • Merge Requests 0
    • Merge Requests 0
  • CI / CD
    • CI / CD
    • Pipelines
    • Jobs
    • Schedules
    • Charts
  • Security & Compliance
    • Security & Compliance
    • Dependency List
  • Packages
    • Packages
    • Container Registry
  • Snippets
    • Snippets
  • Members
    • Members
  • Collapse sidebar
  • Activity
  • Graph
  • Charts
  • Create a new issue
  • Jobs
  • Commits
  • Issue Boards
  • GitLab.org
  • GitLab FOSSGitLab FOSS
  • Issues
  • #27659

Closed
Open
Opened Feb 03, 2017 by Brian Neel@briann
  • Report abuse
  • New issue
Report abuse New issue

Bug in GitHub importer causing labels to be applied to the wrong Merge Requests (Yo!)

There's a bug in the GitHub importer library that results in labels being applied to Merge Requests belonging to other projects and users, often private projects.

As seen here: https://gitlab.com/gitlab-com/infrastructure/issues/1056

and here: https://gitlab.com/gitlab-com/support-forum/issues/1512

This is likely the cause of the mysterious "Yo!" spam label appearing everywhere.

GitHub installs labels at the Pull Request level and not the Project level and considers Pull Requests a type of Issue. We import pull requests here:

lib/gitlab/github_import/importer.rb

      def import_issues
        fetch_resources(:issues, repo, state: :all, sort: :created, direction: :asc, per_page: 100) do |issues|
          issues.each do |raw|
            gh_issue = IssueFormatter.new(project, raw)

            begin
              issuable =
                if gh_issue.pull_request?
                  MergeRequest.find_by_iid(gh_issue.number) 
                else
                  gh_issue.create!
                end

              apply_labels(issuable, raw)
            rescue => e
              errors << { type: :issue, url: Gitlab::UrlSanitizer.sanitize(gh_issue.url), errors: e.message }
            end
          end
        end
      end

There's a race condition in the code above. iid is not unique to a project and if someone creates another MR with the same iid in-between when the GitHub import finishes downloading and the labels are applied the label will be applied to their MR instead. I've verified this on a test instance. The lower the iid the more likely it will be incorrectly applied to another MR.

Incidentally the label will not be applied to the imported MR so that's broken too.

We need to restrict this find to the imported project.

Related issues

  • Discussion
  • Designs
Assignee
Assign to
None
Milestone
None
Assign milestone
Time tracking
None
Due date
None
4
Labels
Platform [DEPRECATED] bug import security
Assign labels
  • View project labels
Reference: gitlab-org/gitlab-foss#27659