Merge branch 'feature/import-export-security-specs' into 'master'

Import/Export security specs

Fixes https://gitlab.com/gitlab-org/gitlab-ce/issues/20857

Related: https://gitlab.com/gitlab-org/gitlab-ce/issues/20821/

See merge request !1987

spec/features/projects/import_export/export_file_spec.rb

+require 'spec_helper'
+
+# Integration test that exports a file using the Import/Export feature
+# It looks up for any sensitive word inside the JSON, so if a sensitive word is found
+# we''l have to either include it adding the model that includes it to the +safe_list+
+# or make sure the attribute is blacklisted in the +import_export.yml+ configuration
+feature 'Import/Export - project export integration test', feature: true, js: true do
+  include Select2Helper
+  include ExportFileHelper
+
+  let(:user) { create(:admin) }
+  let(:export_path) { "#{Dir::tmpdir}/import_file_spec" }
+  let(:config_hash) { YAML.load_file(Gitlab::ImportExport.config_file).deep_stringify_keys }
+
+  let(:sensitive_words) { %w[pass secret token key] }
+  let(:safe_list) do
+    {
+      token: [ProjectHook, Ci::Trigger],
+      key: [Project, Ci::Variable, :yaml_variables]
+    }
+  end
+  let(:safe_hashes) { { yaml_variables: %w[key value public] } }
+
+  let(:project) { setup_project }
+
+  background do
+    allow_any_instance_of(Gitlab::ImportExport).to receive(:storage_path).and_return(export_path)
+  end
+
+  after do
+    FileUtils.rm_rf(export_path, secure: true)
+  end
+
+  context 'admin user' do
+    before do
+      login_as(user)
+    end
+
+    scenario 'exports a project successfully' do
+      visit edit_namespace_project_path(project.namespace, project)
+
+      expect(page).to have_content('Export project')
+
+      click_link 'Export project'
+
+      visit edit_namespace_project_path(project.namespace, project)
+
+      expect(page).to have_content('Download export')
+
+      in_directory_with_expanded_export(project) do |exit_status, tmpdir|
+        expect(exit_status).to eq(0)
+
+        project_json_path = File.join(tmpdir, 'project.json')
+        expect(File).to exist(project_json_path)
+
+        project_hash = JSON.parse(IO.read(project_json_path))
+
+        sensitive_words.each do |sensitive_word|
+          found = find_sensitive_attributes(sensitive_word, project_hash)
+
+          expect(found).to be_nil, failure_message(found.try(:key_found), found.try(:parent), sensitive_word)
+        end
+      end
+    end
+
+    def failure_message(key_found, parent, sensitive_word)
+      <<-MSG
+        Found a new sensitive word <#{key_found}>, which is part of the hash #{parent.inspect}
+
+        If you think this information shouldn't get exported, please exclude the model or attribute in IMPORT_EXPORT_CONFIG.
+
+        Otherwise, please add the exception to +safe_list+ in CURRENT_SPEC using #{sensitive_word} as the key and the
+        correspondent hash or model as the value.
+
+        IMPORT_EXPORT_CONFIG: #{Gitlab::ImportExport.config_file}
+        CURRENT_SPEC: #{__FILE__}
+      MSG
+    end
+  end
+end

spec/features/projects/import_export/import_file_spec.rb

 require 'spec_helper'
 
-feature 'project import', feature: true, js: true do
+feature 'Import/Export - project import integration test', feature: true, js: true do
   include Select2Helper
 
   let(:admin) { create(:admin) }

spec/lib/gitlab/import_export/all_models.yml

+---
+issues:
+- subscriptions
+- award_emoji
+- author
+- assignee
+- updated_by
+- milestone
+- notes
+- label_links
+- labels
+- todos
+- user_agent_detail
+- moved_to
+- events
+events:
+- author
+- project
+- target
+notes:
+- award_emoji
+- project
+- noteable
+- author
+- updated_by
+- resolved_by
+- todos
+- events
+label_links:
+- target
+- label
+label:
+- subscriptions
+- project
+- lists
+- label_links
+- issues
+- merge_requests
+milestone:
+- project
+- issues
+- labels
+- merge_requests
+- participants
+- events
+snippets:
+- author
+- project
+- notes
+releases:
+- project
+project_members:
+- created_by
+- user
+- source
+- project
+merge_requests:
+- subscriptions
+- award_emoji
+- author
+- assignee
+- updated_by
+- milestone
+- notes
+- label_links
+- labels
+- todos
+- target_project
+- source_project
+- merge_user
+- merge_request_diffs
+- merge_request_diff
+- events
+merge_request_diff:
+- merge_request
+pipelines:
+- project
+- user
+- statuses
+- builds
+- trigger_requests
+statuses:
+- project
+- pipeline
+- user
+variables:
+- project
+triggers:
+- project
+- trigger_requests
+deploy_keys:
+- user
+- deploy_keys_projects
+- projects
+services:
+- project
+- service_hook
+hooks:
+- project
+protected_branches:
+- project
+- merge_access_levels
+- push_access_levels
+project:
+- taggings
+- base_tags
+- tag_taggings
+- tags
+- creator
+- group
+- namespace
+- board
+- last_event
+- services
+- campfire_service
+- drone_ci_service
+- emails_on_push_service
+- builds_email_service
+- irker_service
+- pivotaltracker_service
+- hipchat_service
+- flowdock_service
+- assembla_service
+- asana_service
+- gemnasium_service
+- slack_service
+- buildkite_service
+- bamboo_service
+- teamcity_service
+- pushover_service
+- jira_service
+- redmine_service
+- custom_issue_tracker_service
+- bugzilla_service
+- gitlab_issue_tracker_service
+- external_wiki_service
+- forked_project_link
+- forked_from_project
+- forked_project_links
+- forks
+- merge_requests
+- fork_merge_requests
+- issues
+- labels
+- events
+- milestones
+- notes
+- snippets
+- hooks
+- protected_branches
+- project_members
+- users
+- requesters
+- deploy_keys_projects
+- deploy_keys
+- users_star_projects
+- starrers
+- releases
+- lfs_objects_projects
+- lfs_objects
+- project_group_links
+- invited_groups
+- todos
+- notification_settings
+- import_data
+- commit_statuses
+- pipelines
+- builds
+- runner_projects
+- runners
+- variables
+- triggers
+- environments
+- deployments
+- project_feature
\ No newline at end of file

spec/lib/gitlab/import_export/attribute_configuration_spec.rb

+require 'spec_helper'
+
+# Part of the test security suite for the Import/Export feature
+# Checks whether there are new attributes in models that are currently being exported as part of the
+# project Import/Export feature.
+# If there are new attributes, these will have to either be added to this spec in case we want them
+# to be included as part of the export, or blacklist them using the import_export.yml configuration file.
+# Likewise, new models added to import_export.yml, will need to be added with their correspondent attributes
+# to this spec.
+describe 'Import/Export attribute configuration', lib: true do
+  include ConfigurationHelper
+
+  let(:config_hash) { YAML.load_file(Gitlab::ImportExport.config_file).deep_stringify_keys }
+  let(:relation_names) do
+    names = names_from_tree(config_hash['project_tree'])
+
+    # Remove duplicated or add missing models
+    # - project is not part of the tree, so it has to be added manually.
+    # - milestone, labels have both singular and plural versions in the tree, so remove the duplicates.
+    names.flatten.uniq - ['milestones', 'labels'] + ['project']
+  end
+
+  let(:safe_attributes_file) { 'spec/lib/gitlab/import_export/safe_model_attributes.yml' }
+  let(:safe_model_attributes) { YAML.load_file(safe_attributes_file) }
+
+  it 'has no new columns' do
+    relation_names.each do |relation_name|
+      relation_class = relation_class_for_name(relation_name)
+
+      expect(safe_model_attributes[relation_class.to_s]).not_to be_nil, "Expected exported class #{relation_class.to_s} to exist in safe_model_attributes"
+
+      current_attributes = parsed_attributes(relation_name, relation_class.attribute_names)
+      safe_attributes = safe_model_attributes[relation_class.to_s]
+      new_attributes = current_attributes - safe_attributes
+
+      expect(new_attributes).to be_empty, failure_message(relation_class.to_s, new_attributes)
+    end
+  end
+
+  def failure_message(relation_class, new_attributes)
+    <<-MSG
+      It looks like #{relation_class}, which is exported using the project Import/Export, has new attributes: #{new_attributes.join(',')}
+
+      Please add the attribute(s) to SAFE_MODEL_ATTRIBUTES if you consider this can be exported.
+      Otherwise, please blacklist the attribute(s) in IMPORT_EXPORT_CONFIG by adding it to its correspondent
+      model in the +excluded_attributes+ section.
+
+      SAFE_MODEL_ATTRIBUTES: #{File.expand_path(safe_attributes_file)}
+      IMPORT_EXPORT_CONFIG: #{Gitlab::ImportExport.config_file}
+    MSG
+  end
+
+  class Author < User
+  end
+end

spec/lib/gitlab/import_export/model_configuration_spec.rb

+require 'spec_helper'
+
+# Part of the test security suite for the Import/Export feature
+# Finds if a new model has been added that can potentially be part of the Import/Export
+# If it finds a new model, it will show a +failure_message+ with the options available.
+describe 'Import/Export model configuration', lib: true do
+  include ConfigurationHelper
+
+  let(:config_hash) { YAML.load_file(Gitlab::ImportExport.config_file).deep_stringify_keys }
+  let(:model_names) do
+    names = names_from_tree(config_hash['project_tree'])
+
+    # Remove duplicated or add missing models
+    # - project is not part of the tree, so it has to be added manually.
+    # - milestone, labels have both singular and plural versions in the tree, so remove the duplicates.
+    # - User, Author... Models we do not care about for checking models
+    names.flatten.uniq - ['milestones', 'labels', 'user', 'author'] + ['project']
+  end
+
+  let(:all_models_yml) { 'spec/lib/gitlab/import_export/all_models.yml' }
+  let(:all_models) { YAML.load_file(all_models_yml) }
+  let(:current_models) { setup_models }
+
+  it 'has no new models' do
+    model_names.each do |model_name|
+      new_models = Array(current_models[model_name]) - Array(all_models[model_name])
+      expect(new_models).to be_empty, failure_message(model_name.classify, new_models)
+    end
+  end
+
+  # List of current models between models, in the format of
+  # {model: [model_2, model3], ...}
+  def setup_models
+    all_models_hash = {}
+
+    model_names.each do |model_name|
+      model_class = relation_class_for_name(model_name)
+
+      all_models_hash[model_name] = associations_for(model_class) - ['project']
+    end
+
+    all_models_hash
+  end
+
+  def failure_message(parent_model_name, new_models)
+    <<-MSG
+      New model(s) <#{new_models.join(',')}> have been added, related to #{parent_model_name}, which is exported by
+      the Import/Export feature.
+
+      If you think this model should be included in the export, please add it to IMPORT_EXPORT_CONFIG.
+      Definitely add it to MODELS_JSON to signal that you've handled this error and to prevent it from showing up in the future.
+
+      MODELS_JSON: #{File.expand_path(all_models_yml)}
+      IMPORT_EXPORT_CONFIG: #{Gitlab::ImportExport.config_file}
+    MSG
+  end
+end

spec/lib/gitlab/import_export/safe_model_attributes.yml

+---
+Issue:
+- id
+- title
+- assignee_id
+- author_id
+- project_id
+- created_at
+- updated_at
+- position
+- branch_name
+- description
+- state
+- iid
+- updated_by_id
+- confidential
+- deleted_at
+- due_date
+- moved_to_id
+- lock_version
+- milestone_id
+- weight
+Event:
+- id
+- target_type
+- target_id
+- title
+- data
+- project_id
+- created_at
+- updated_at
+- action
+- author_id
+Note:
+- id
+- note
+- noteable_type
+- author_id
+- created_at
+- updated_at
+- project_id
+- attachment
+- line_code
+- commit_id
+- noteable_id
+- system
+- st_diff
+- updated_by_id
+- type
+- position
+- original_position
+- resolved_at
+- resolved_by_id
+- discussion_id
+- original_discussion_id
+LabelLink:
+- id
+- label_id
+- target_id
+- target_type
+- created_at
+- updated_at
+Label:
+- id
+- title
+- color
+- project_id
+- created_at
+- updated_at
+- template
+- description
+- priority
+Milestone:
+- id
+- title
+- project_id
+- description
+- due_date
+- created_at
+- updated_at
+- state
+- iid
+ProjectSnippet:
+- id
+- title
+- content
+- author_id
+- project_id
+- created_at
+- updated_at
+- file_name
+- type
+- visibility_level
+Release:
+- id
+- tag
+- description
+- project_id
+- created_at
+- updated_at
+ProjectMember:
+- id
+- access_level
+- source_id
+- source_type
+- user_id
+- notification_level
+- type
+- created_at
+- updated_at
+- created_by_id
+- invite_email
+- invite_token
+- invite_accepted_at
+- requested_at
+- expires_at
+User:
+- id
+- username
+- email
+MergeRequest:
+- id
+- target_branch
+- source_branch
+- source_project_id
+- author_id
+- assignee_id
+- title
+- created_at
+- updated_at
+- state
+- merge_status
+- target_project_id
+- iid
+- description
+- position
+- locked_at
+- updated_by_id
+- merge_error
+- merge_params
+- merge_when_build_succeeds
+- merge_user_id
+- merge_commit_sha
+- deleted_at
+- in_progress_merge_commit_sha
+- lock_version
+- milestone_id
+- approvals_before_merge
+- rebase_commit_sha
+MergeRequestDiff:
+- id
+- state
+- st_commits
+- merge_request_id
+- created_at
+- updated_at
+- base_commit_sha
+- real_size
+- head_commit_sha
+- start_commit_sha
+Ci::Pipeline:
+- id
+- project_id
+- ref
+- sha
+- before_sha
+- push_data
+- created_at
+- updated_at
+- tag
+- yaml_errors
+- committed_at
+- gl_project_id
+- status
+- started_at
+- finished_at
+- duration
+- user_id
+CommitStatus:
+- id
+- project_id
+- status
+- finished_at
+- trace
+- created_at
+- updated_at
+- started_at
+- runner_id
+- coverage
+- commit_id
+- commands
+- job_id
+- name
+- deploy
+- options
+- allow_failure
+- stage
+- trigger_request_id
+- stage_idx
+- tag
+- ref
+- user_id
+- type
+- target_url
+- description
+- artifacts_file
+- gl_project_id
+- artifacts_metadata
+- erased_by_id
+- erased_at
+- artifacts_expire_at
+- environment
+- artifacts_size
+- when
+- yaml_variables
+- queued_at
+Ci::Variable:
+- id
+- project_id
+- key
+- value
+- encrypted_value
+- encrypted_value_salt
+- encrypted_value_iv
+- gl_project_id
+Ci::Trigger:
+- id
+- token
+- project_id
+- deleted_at
+- created_at
+- updated_at
+- gl_project_id
+DeployKey:
+- id
+- user_id
+- created_at
+- updated_at
+- key
+- title
+- type
+- fingerprint
+- public
+Service:
+- id
+- type
+- title
+- project_id
+- created_at
+- updated_at
+- active
+- properties
+- template
+- push_events
+- issues_events
+- merge_requests_events
+- tag_push_events
+- note_events
+- pipeline_events
+- build_events
+- category
+- default
+- wiki_page_events
+- confidential_issues_events
+ProjectHook:
+- id
+- url
+- project_id
+- created_at
+- updated_at
+- type
+- service_id
+- push_events
+- issues_events
+- merge_requests_events
+- tag_push_events
+- note_events
+- pipeline_events
+- enable_ssl_verification
+- build_events
+- wiki_page_events
+- token
+- group_id
+- confidential_issues_events
+ProtectedBranch:
+- id
+- project_id
+- name
+- created_at
+- updated_at
+Project:
+- description
+- issues_enabled
+- merge_requests_enabled
+- wiki_enabled
+- snippets_enabled
+- visibility_level
+- archived
+Author:
+- name
+ProjectFeature:
+- id
+- project_id
+- merge_requests_access_level
+- issues_access_level
+- wiki_access_level
+- snippets_access_level
+- builds_access_level
+- created_at
+- updated_at
\ No newline at end of file

spec/support/import_export/configuration_helper.rb

+module ConfigurationHelper
+  # Returns a list of models from hashes/arrays contained in +project_tree+
+  def names_from_tree(project_tree)
+    project_tree.map do |branch_or_model|
+      branch_or_model =  branch_or_model.to_s if branch_or_model.is_a?(Symbol)
+
+      branch_or_model.is_a?(String) ? branch_or_model : names_from_tree(branch_or_model)
+    end
+  end
+
+  def relation_class_for_name(relation_name)
+    relation_name = Gitlab::ImportExport::RelationFactory::OVERRIDES[relation_name.to_sym] || relation_name
+    relation_name.to_s.classify.constantize
+  end
+
+  def parsed_attributes(relation_name, attributes)
+    excluded_attributes = config_hash['excluded_attributes'][relation_name]
+    included_attributes = config_hash['included_attributes'][relation_name]
+
+    attributes = attributes - JSON[excluded_attributes.to_json] if excluded_attributes
+    attributes = attributes & JSON[included_attributes.to_json] if included_attributes
+
+    attributes
+  end
+
+  def associations_for(safe_model)
+    safe_model.reflect_on_all_associations.map { |assoc| assoc.name.to_s }
+  end
+end

spec/support/import_export/export_file_helper.rb

+require './spec/support/import_export/configuration_helper'
+
+module ExportFileHelper
+  include ConfigurationHelper
+
+  ObjectWithParent = Struct.new(:object, :parent, :key_found)
+
+  def setup_project
+    project = create(:project, :public)
+
+    create(:release, project: project)
+
+    issue = create(:issue, assignee: user, project: project)
+    snippet = create(:project_snippet, project: project)
+    label = create(:label, project: project)
+    milestone = create(:milestone, project: project)
+    merge_request = create(:merge_request, source_project: project, milestone: milestone)
+    commit_status = create(:commit_status, project: project)
+
+    create(:label_link, label: label, target: issue)
+
+    ci_pipeline = create(:ci_pipeline,
+                         project: project,
+                         sha: merge_request.diff_head_sha,
+                         ref: merge_request.source_branch,
+                         statuses: [commit_status])
+
+    create(:ci_build, pipeline: ci_pipeline, project: project)
+    create(:milestone, project: project)
+    create(:note, noteable: issue, project: project)
+    create(:note, noteable: merge_request, project: project)
+    create(:note, noteable: snippet, project: project)
+    create(:note_on_commit,
+           author: user,
+           project: project,
+           commit_id: ci_pipeline.sha)
+
+    create(:event, target: milestone, project: project, action: Event::CREATED, author: user)
+    create(:project_member, :master, user: user, project: project)
+    create(:ci_variable, project: project)
+    create(:ci_trigger, project: project)
+    key = create(:deploy_key)
+    key.projects << project
+    create(:service, project: project)
+    create(:project_hook, project: project, token: 'token')
+    create(:protected_branch, project: project)
+
+    project
+  end
+
+  # Expands the compressed file for an exported project into +tmpdir+
+  def in_directory_with_expanded_export(project)
+    Dir.mktmpdir do |tmpdir|
+      export_file = project.export_project_path
+      _output, exit_status = Gitlab::Popen.popen(%W{tar -zxf #{export_file} -C #{tmpdir}})
+
+      yield(exit_status, tmpdir)
+    end
+  end
+
+  # Recursively finds key/values including +key+ as part of the key, inside a nested hash
+  def deep_find_with_parent(sensitive_key_word, object, found = nil)
+    sensitive_key_found = object_contains_key?(object, sensitive_key_word)
+
+    # Returns the parent object and the object found containing a sensitive word as part of the key
+    if sensitive_key_found && object[sensitive_key_found]
+      ObjectWithParent.new(object[sensitive_key_found], object, sensitive_key_found)
+    elsif object.is_a?(Enumerable)
+      # Recursively lookup for keys containing sensitive words in a Hash or Array
+      object_with_parent = nil
+
+      object.find do |*hash_or_array|
+        object_with_parent = deep_find_with_parent(sensitive_key_word, hash_or_array.last, found)
+      end
+
+      object_with_parent
+    end
+  end
+
+  # Return true if the hash has a key containing a sensitive word
+  def object_contains_key?(object, sensitive_key_word)
+    return false unless object.is_a?(Hash)
+
+    object.keys.find { |key| key.include?(sensitive_key_word) }
+  end
+
+  # Returns the offended ObjectWithParent object if a sensitive word is found inside a hash,
+  # excluding the whitelisted safe hashes.
+  def find_sensitive_attributes(sensitive_word, project_hash)
+    loop do
+      object_with_parent = deep_find_with_parent(sensitive_word, project_hash)
+
+      return nil unless object_with_parent && object_with_parent.object
+
+      if is_safe_hash?(object_with_parent.parent, sensitive_word)
+        # It's in the safe list, remove hash and keep looking
+        object_with_parent.parent.delete(object_with_parent.key_found)
+      else
+        return object_with_parent
+      end
+
+      nil
+    end
+  end
+
+  # Returns true if it's one of the excluded models in +safe_list+
+  def is_safe_hash?(parent, sensitive_word)
+    return false unless parent && safe_list[sensitive_word.to_sym]
+
+    # Extra attributes that appear in a model but not in the exported hash.
+    excluded_attributes = ['type']
+
+    safe_list[sensitive_word.to_sym].each do |model|
+      # Check whether this is a hash attribute inside a model
+      if model.is_a?(Symbol)
+        return true if (safe_hashes[model] - parent.keys).empty?
+      else
+        return true if safe_model?(model, excluded_attributes, parent)
+      end
+    end
+
+    false
+  end
+
+  # Compares model attributes with those those found in the hash
+  # and returns true if there is a match, ignoring some excluded attributes.
+  def safe_model?(model, excluded_attributes, parent)
+    excluded_attributes += associations_for(model)
+    parsed_model_attributes = parsed_attributes(model.name.underscore, model.attribute_names)
+
+    (parsed_model_attributes - parent.keys - excluded_attributes).empty?
+  end
+end