Automatic merge of gitlab-org/gitlab master

db/migrate/20220106141756_remove_lock_version_indexes.rb

+# frozen_string_literal: true
+
+class RemoveLockVersionIndexes < Gitlab::Database::Migration[1.0]
+  disable_ddl_transaction!
+
+  INDEXES = {
+    issues: 'index_issues_on_lock_version',
+    merge_requests: 'index_merge_requests_on_lock_version',
+    epics: 'index_epics_on_lock_version'
+  }
+
+  def up
+    INDEXES.each do |table, index_name|
+      remove_concurrent_index_by_name table, index_name
+    end
+  end
+
+  def down
+    INDEXES.each do |table, index_name|
+      add_concurrent_index table, :lock_version, where: "lock_version IS NULL", name: index_name
+    end
+  end
+end

db/post_migrate/20211209203820_add_tmp_index_on_report_type.rb

+# frozen_string_literal: true
+class AddTmpIndexOnReportType < Gitlab::Database::Migration[1.0]
+  # Temporary index to perform migration fixing invalid vulnerability_occurrences.raw_metadata rows
+  # Will be removed with https://gitlab.com/gitlab-org/gitlab/-/issues/349605
+  INDEX_NAME = 'tmp_idx_vulnerability_occurrences_on_id_where_report_type_7_99'
+
+  disable_ddl_transaction!
+
+  def up
+    add_concurrent_index :vulnerability_occurrences, :id, where: 'report_type IN (7, 99)', name: INDEX_NAME
+  end
+
+  def down
+    remove_concurrent_index_by_name :vulnerability_occurrences, INDEX_NAME
+  end
+end

db/post_migrate/20211209203821_convert_stringified_raw_metadata_hash_to_json.rb

+# frozen_string_literal: true
+class ConvertStringifiedRawMetadataHashToJson < Gitlab::Database::Migration[1.0]
+  MIGRATION_CLASS = Gitlab::BackgroundMigration::FixVulnerabilityOccurrencesWithHashesAsRawMetadata
+  MODEL_CLASS = MIGRATION_CLASS::Finding
+  DELAY_INTERVAL = 2.minutes
+  BATCH_SIZE = 500
+
+  disable_ddl_transaction!
+
+  def up
+    queue_background_migration_jobs_by_range_at_intervals(
+      MODEL_CLASS.by_api_report_types,
+      MIGRATION_CLASS,
+      DELAY_INTERVAL,
+      batch_size: BATCH_SIZE
+    )
+  end
+
+  def down
+    # no-op
+
+    # up fixes invalid data by updating columns in-place.
+    # It is a backwards-compatible change, and reversing it in a downgrade would not be desirable.
+  end
+end

db/schema_migrations/20211209203820

+c501bc857cef21a8657508e9a286feb3c6f5db873247707e051a8702b1e80e79
\ No newline at end of file

db/schema_migrations/20211209203821

+4e8e0917bcfcadf288425e82eeb3747d775bb301017a9b320b694cd43ed0d60a
\ No newline at end of file

db/schema_migrations/20220106141756

+cedca81a6dc1562cf0ed5f17217c52420a1cb9a74acece8d67eb1bcb948a7181
\ No newline at end of file

db/structure.sql

@@ -28023,6 +28023,8 @@ CREATE UNIQUE INDEX term_agreements_unique_index ON term_agreements USING btree
 
 CREATE INDEX tmp_idx_deduplicate_vulnerability_occurrences ON vulnerability_occurrences USING btree (project_id, report_type, location_fingerprint, primary_identifier_id, id);
 
+CREATE INDEX tmp_idx_vulnerability_occurrences_on_id_where_report_type_7_99 ON vulnerability_occurrences USING btree (id) WHERE (report_type = ANY (ARRAY[7, 99]));
+
 CREATE INDEX tmp_index_namespaces_empty_traversal_ids_with_child_namespaces ON namespaces USING btree (id) WHERE ((parent_id IS NOT NULL) AND (traversal_ids = '{}'::integer[]));
 
 CREATE INDEX tmp_index_namespaces_empty_traversal_ids_with_root_namespaces ON namespaces USING btree (id) WHERE ((parent_id IS NULL) AND (traversal_ids = '{}'::integer[]));

doc/administration/geo/index.md

@@ -120,7 +120,8 @@ The following are required to run Geo:
   The following operating systems are known to ship with a current version of OpenSSH:
   - [CentOS](https://www.centos.org) 7.4 or later
   - [Ubuntu](https://ubuntu.com) 16.04 or later
-- PostgreSQL 12 or later with [Streaming Replication](https://wiki.postgresql.org/wiki/Streaming_Replication)
+- PostgreSQL 12 with [Streaming Replication](https://wiki.postgresql.org/wiki/Streaming_Replication)
+  - PostgreSQL 13 is not supported for Geo, see [epic 3832](https://gitlab.com/groups/gitlab-org/-/epics/3832)
 - Git 2.9 or later
 - Git-lfs 2.4.2 or later on the user side when using LFS
 - All sites must run the same GitLab version.

doc/administration/package_information/postgresql_versions.md

@@ -26,7 +26,7 @@ Read more about update policies and warnings in the PostgreSQL
 
 | GitLab version | PostgreSQL versions | Default version for fresh installs | Default version for upgrades | Notes |
 | -------------- | --------------------- | ---------------------------------- | ---------------------------- | ----- |
-| 14.1 | 12.6, 13.3 | 12.6 | 12.6 | PostgreSQL 13 available for fresh installations if not using Geo or High Availability. |
+| 14.1 | 12.6, 13.3 | 12.6 | 12.6 | PostgreSQL 13 available for fresh installations if not using [Geo](../geo/index.md#requirements-for-running-geo) or [Patroni](../postgresql/index.md#postgresql-replication-and-failover-with-omnibus-gitlab).
 | 14.0 | 12.6       | 12.6 | 12.6 | HA installations with repmgr are no longer supported and will be prevented from upgrading to Omnibus GitLab 14.0 |
 | 13.8 | 11.9, 12.4 | 12.4 | 12.4 | Package upgrades automatically performed PostgreSQL upgrade for nodes that are not part of a Geo or HA cluster.). |
 | 13.7 | 11.9, 12.4 | 12.4 | 11.9 | For upgrades users can manually upgrade to 12.4 following the [upgrade docs](https://docs.gitlab.com/omnibus/settings/database.html#gitlab-133-and-later). |

doc/administration/pages/index.md

@@ -545,6 +545,14 @@ After an archive reaches `zip_cache_expiration`, it's marked as expired and remo
 
 ![ZIP cache configuration](img/zip_cache_configuration.png)
 
+### HTTP Strict Transport Security (HSTS) support
+
+HTTP Strict Transport Security (HSTS) can be enabled through the `gitlab_pages['headers']` configuration option. HSTS informs browsers that the website they are visiting should always provide its content over HTTPS to ensure that attackers cannot force subsequent connections to happen unencrypted. It can also improve loading speed of pages as it prevents browsers from attempting to connect over an unencrypted HTTP channel before being redirected to HTTPS.
+
+```ruby
+gitlab_pages['headers'] = ['Strict-Transport-Security: max-age=63072000']
+```
+
 ## Activate verbose logging for daemon
 
 Follow the steps below to configure verbose logging of GitLab Pages daemon.

ee/app/services/vulnerabilities/create_service_base.rb

@@ -112,6 +112,9 @@ module Vulnerabilities
         project_id: @project.id
       )
 
+      raw_metadata = {}
+      raw_metadata['location'] = location if location
+
       Vulnerabilities::Finding.new(
         project: @project,
         identifiers: identifiers,
@@ -122,11 +125,14 @@ module Vulnerabilities
         confidence: vulnerability.confidence,
         report_type: vulnerability.report_type,
         project_fingerprint: Digest::SHA1.hexdigest(identifiers.first.name),
+        location: location,
         location_fingerprint: loc_fingerprint,
         metadata_version: metadata_version,
-        raw_metadata: {
-          location: location
-        },
+
+        # raw_metadata is a text field rather than jsonb,
+        # so it is important to convert data to JSON.
+        # It will be removed in https://gitlab.com/groups/gitlab-org/-/epics/4239.
+        raw_metadata: raw_metadata.to_json,
         scanner: scanner,
         uuid: uuid,
         message: message,

ee/spec/services/vulnerabilities/manually_create_service_spec.rb

@@ -141,6 +141,8 @@ RSpec.describe Vulnerabilities::ManuallyCreateService do
         expect(finding.message).to eq(params.dig(:vulnerability, :message))
         expect(finding.description).to eq(params.dig(:vulnerability, :description))
         expect(finding.solution).to eq(params.dig(:vulnerability, :solution))
+        expect(finding.location).to be_empty
+        expect(finding.raw_metadata).to eq("{}")
 
         scanner = finding.scanner
         expect(scanner.name).to eq(params.dig(:vulnerability, :scanner, :name))

ee/spec/services/vulnerabilities/starboard_vulnerability_create_service_spec.rb

@@ -15,6 +15,13 @@ RSpec.describe Vulnerabilities::StarboardVulnerabilityCreateService do
         severity: 'high',
         confidence: 'unknown',
         location: {
+          image: 'alpine:latest',
+          dependency: {
+            version: '0.1.0',
+            package: {
+              name: 'libc'
+            }
+          },
           kubernetes_resource: {
             namespace: 'production',
             kind: 'deployment',
@@ -67,6 +74,19 @@ RSpec.describe Vulnerabilities::StarboardVulnerabilityCreateService do
         expect(finding.description).to eq(params.dig(:vulnerability, :description))
         expect(finding.severity).to eq(params.dig(:vulnerability, :severity))
         expect(finding.confidence).to eq(params.dig(:vulnerability, :confidence))
+        expect(finding.location['image']).to eq('alpine:latest')
+        expect(finding.location.dig('dependency', 'package', 'name')).to eq('libc')
+        expect(finding.location.dig('dependency', 'version')).to eq('0.1.0')
+        expect(finding.location['kubernetes_resource']).to eq(
+          {
+            'namespace' => 'production',
+            'kind' => 'deployment',
+            'name' => 'nginx',
+            'container' => 'nginx'
+          }
+        )
+
+        expect(finding.metadata['location']).to eq(finding.location)
 
         scanner = finding.scanner
         expect(scanner.external_id).to eq(params.dig(:scanner, :id))

lib/gitlab/background_migration/fix_vulnerability_occurrences_with_hashes_as_raw_metadata.rb

+# frozen_string_literal: true
+
+require 'parser/ruby27'
+
+module Gitlab
+  module BackgroundMigration
+    # This migration fixes raw_metadata entries which have incorrectly been passed a Ruby Hash instead of JSON data.
+    class FixVulnerabilityOccurrencesWithHashesAsRawMetadata
+      CLUSTER_IMAGE_SCANNING_REPORT_TYPE = 7
+      GENERIC_REPORT_TYPE = 99
+
+      # Type error is used to handle unexpected types when parsing stringified hashes.
+      class TypeError < ::StandardError
+        attr_reader :message, :type
+
+        def initialize(message, type)
+          @message = message
+          @type = type
+        end
+      end
+
+      # Migration model namespace isolated from application code.
+      class Finding < ActiveRecord::Base
+        include EachBatch
+
+        self.table_name = 'vulnerability_occurrences'
+
+        scope :by_api_report_types, -> { where(report_type: [CLUSTER_IMAGE_SCANNING_REPORT_TYPE, GENERIC_REPORT_TYPE]) }
+      end
+
+      def perform(start_id, end_id)
+        Finding.by_api_report_types.where(id: start_id..end_id).each do |finding|
+          next if valid_json?(finding.raw_metadata)
+
+          metadata = hash_from_s(finding.raw_metadata)
+
+          finding.update(raw_metadata: metadata.to_json) if metadata
+        end
+        mark_job_as_succeeded(start_id, end_id)
+      end
+
+      def hash_from_s(str_hash)
+        ast = Parser::Ruby27.parse(str_hash)
+
+        unless ast.type == :hash
+          ::Gitlab::AppLogger.error(message: "expected raw_metadata to be a hash", type: ast.type)
+          return
+        end
+
+        parse_hash(ast)
+      rescue Parser::SyntaxError => e
+        ::Gitlab::AppLogger.error(message: "error parsing raw_metadata", error: e.message)
+        nil
+      rescue TypeError => e
+        ::Gitlab::AppLogger.error(message: "error parsing raw_metadata", error: e.message, type: e.type)
+        nil
+      end
+
+      private
+
+      def mark_job_as_succeeded(*arguments)
+        Gitlab::Database::BackgroundMigrationJob.mark_all_as_succeeded(
+          'FixVulnerabilityOccurrencesWithHashesAsRawMetadata',
+          arguments
+        )
+      end
+
+      def valid_json?(metadata)
+        Oj.load(metadata)
+        true
+      rescue Oj::ParseError, Encoding::UndefinedConversionError
+        false
+      end
+
+      def parse_hash(hash)
+        out = {}
+        hash.children.each do |node|
+          unless node.type == :pair
+            raise TypeError.new("expected child of hash to be a `pair`", node.type)
+          end
+
+          key, value = node.children
+
+          key = parse_key(key)
+          value = parse_value(value)
+
+          out[key] = value
+        end
+
+        out
+      end
+
+      def parse_key(key)
+        case key.type
+        when :sym, :str, :int
+          key.children.first
+        else
+          raise TypeError.new("expected key to be either symbol, string, or integer", key.type)
+        end
+      end
+
+      def parse_value(value)
+        case value.type
+        when :sym, :str, :int
+          value.children.first
+        # rubocop:disable Lint/BooleanSymbol
+        when :true
+          true
+        when :false
+          false
+        # rubocop:enable Lint/BooleanSymbol
+        when :nil
+          nil
+        when :array
+          value.children.map { |c| parse_value(c) }
+        when :hash
+          parse_hash(value)
+        else
+          raise TypeError.new("value of a pair was an unexpected type", value.type)
+        end
+      end
+    end
+  end
+end

spec/lib/gitlab/background_migration/fix_vulnerability_occurrences_with_hashes_as_raw_metadata_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe Gitlab::BackgroundMigration::FixVulnerabilityOccurrencesWithHashesAsRawMetadata, schema: 20211209203821 do
+  let(:users) { table(:users) }
+  let(:namespaces) { table(:namespaces) }
+  let(:projects) { table(:projects) }
+  let(:scanners) { table(:vulnerability_scanners) }
+  let(:identifiers) { table(:vulnerability_identifiers) }
+  let(:findings) { table(:vulnerability_occurrences) }
+
+  let(:user) { users.create!(name: 'Test User', projects_limit: 10, username: 'test-user', email: '1') }
+
+  let(:namespace) do
+    namespaces.create!(
+      owner_id: user.id,
+      name: user.name,
+      path: user.username
+    )
+  end
+
+  let(:project) do
+    projects.create!(namespace_id: namespace.id, name: 'Test Project')
+  end
+
+  let(:scanner) do
+    scanners.create!(
+      project_id: project.id,
+      external_id: 'test-scanner',
+      name: 'Test Scanner',
+      vendor: 'GitLab'
+    )
+  end
+
+  let(:primary_identifier) do
+    identifiers.create!(
+      project_id: project.id,
+      external_type: 'cve',
+      name: 'CVE-2021-1234',
+      external_id: 'CVE-2021-1234',
+      fingerprint: '4c0fe491999f94701ee437588554ef56322ae276'
+    )
+  end
+
+  let(:finding) do
+    findings.create!(
+      raw_metadata: raw_metadata,
+      project_id: project.id,
+      scanner_id: scanner.id,
+      primary_identifier_id: primary_identifier.id,
+      uuid: '4deb090a-bedf-5ccc-aa9a-ac8055a1ea81',
+      project_fingerprint: '1caa750a6dad769a18ad6f40b413b3b6ab1c8d77',
+      location_fingerprint: '6d1f35f53b065238abfcadc01336ce65d112a2bd',
+      name: 'name',
+      report_type: 7,
+      severity: 0,
+      confidence: 0,
+      detection_method: 'gitlab_security_report',
+      metadata_version: 'cluster_image_scanning:1.0',
+      created_at: "2021-12-10 14:27:42 -0600",
+      updated_at: "2021-12-10 14:27:42 -0600"
+    )
+  end
+
+  subject(:perform) { described_class.new.perform(finding.id, finding.id) }
+
+  context 'with stringified hash as raw_metadata' do
+    let(:raw_metadata) do
+      '{:location=>{"image"=>"index.docker.io/library/nginx:latest", "kubernetes_resource"=>{"namespace"=>"production", "kind"=>"deployment", "name"=>"nginx", "container_name"=>"nginx", "agent_id"=>"2"}, "dependency"=>{"package"=>{"name"=>"libc"}, "version"=>"v1.2.3"}}}'
+    end
+
+    it 'converts stringified hash to JSON' do
+      expect { perform }.not_to raise_error
+
+      result = finding.reload.raw_metadata
+      metadata = Oj.load(result)
+      expect(metadata).to eq(
+        {
+          'location' => {
+            'image' => 'index.docker.io/library/nginx:latest',
+            'kubernetes_resource' => {
+              'namespace' => 'production',
+              'kind' => 'deployment',
+              'name' => 'nginx',
+              'container_name' => 'nginx',
+              'agent_id' => '2'
+            },
+            'dependency' => {
+              'package' => { 'name' => 'libc' },
+              'version' => 'v1.2.3'
+            }
+          }
+        }
+      )
+    end
+  end
+
+  context 'with valid raw_metadata' do
+    where(:raw_metadata) do
+      [
+        '{}',
+        '{"location":null}',
+        '{"location":{"image":"index.docker.io/library/nginx:latest","kubernetes_resource":{"namespace":"production","kind":"deployment","name":"nginx","container_name":"nginx","agent_id":"2"},"dependency":{"package":{"name":"libc"},"version":"v1.2.3"}}}'
+      ]
+    end
+
+    with_them do
+      it 'does not change the raw_metadata' do
+        expect { perform }.not_to raise_error
+
+        result = finding.reload.raw_metadata
+        expect(result).to eq(raw_metadata)
+      end
+    end
+  end
+
+  context 'when raw_metadata contains forbidden types' do
+    using RSpec::Parameterized::TableSyntax
+
+    where(:raw_metadata, :type) do
+      'def foo; "bar"; end'     | :def
+      '`cat somefile`'          | :xstr
+      'exec("cat /etc/passwd")' | :send
+    end
+
+    with_them do
+      it 'does not change the raw_metadata' do
+        expect(Gitlab::AppLogger).to receive(:error).with(message: "expected raw_metadata to be a hash", type: type)
+
+        expect { perform }.not_to raise_error
+
+        result = finding.reload.raw_metadata
+        expect(result).to eq(raw_metadata)
+      end
+    end
+  end
+
+  context 'when forbidden types are nested inside a hash' do
+    using RSpec::Parameterized::TableSyntax
+
+    where(:raw_metadata, :type) do
+      '{:location=>Env.fetch("SOME_VAR")}'           | :send
+      '{:location=>{:image=>Env.fetch("SOME_VAR")}}' | :send
+      # rubocop:disable Lint/InterpolationCheck
+      '{"key"=>"value: #{send}"}'                    | :dstr
+      # rubocop:enable Lint/InterpolationCheck
+    end
+
+    with_them do
+      it 'does not change the raw_metadata' do
+        expect(Gitlab::AppLogger).to receive(:error).with(
+          message: "error parsing raw_metadata",
+          error: "value of a pair was an unexpected type",
+          type: type
+        )
+
+        expect { perform }.not_to raise_error
+
+        result = finding.reload.raw_metadata
+        expect(result).to eq(raw_metadata)
+      end
+    end
+  end
+
+  context 'when key is an unexpected type' do
+    let(:raw_metadata) { "{nil=>nil}" }
+
+    it 'logs error' do
+      expect(Gitlab::AppLogger).to receive(:error).with(
+        message: "error parsing raw_metadata",
+        error: "expected key to be either symbol, string, or integer",
+        type: :nil
+      )
+
+      expect { perform }.not_to raise_error
+    end
+  end
+
+  context 'when raw_metadata cannot be parsed' do
+    let(:raw_metadata) { "{" }
+
+    it 'logs error' do
+      expect(Gitlab::AppLogger).to receive(:error).with(message: "error parsing raw_metadata", error: "unexpected token $end")
+
+      expect { perform }.not_to raise_error
+    end
+  end
+
+  describe '#hash_from_s' do
+    subject { described_class.new.hash_from_s(input) }
+
+    context 'with valid input' do
+      let(:input) { '{:location=>{"image"=>"index.docker.io/library/nginx:latest", "kubernetes_resource"=>{"namespace"=>"production", "kind"=>"deployment", "name"=>"nginx", "container_name"=>"nginx", "agent_id"=>2}, "dependency"=>{"package"=>{"name"=>"libc"}, "version"=>"v1.2.3"}}}' }
+
+      it 'converts string to a hash' do
+        expect(subject).to eq({
+          location: {
+            'image' => 'index.docker.io/library/nginx:latest',
+            'kubernetes_resource' => {
+              'namespace' => 'production',
+              'kind' => 'deployment',
+              'name' => 'nginx',
+              'container_name' => 'nginx',
+              'agent_id' => 2
+            },
+            'dependency' => {
+              'package' => { 'name' => 'libc' },
+              'version' => 'v1.2.3'
+            }
+          }
+        })
+      end
+    end
+
+    using RSpec::Parameterized::TableSyntax
+
+    where(:input, :expected) do
+      '{}'                          | {}
+      '{"bool"=>true}'              | { 'bool' => true }
+      '{"bool"=>false}'             | { 'bool' => false }
+      '{"nil"=>nil}'                | { 'nil' => nil }
+      '{"array"=>[1, "foo", nil]}'  | { 'array' => [1, "foo", nil] }
+      '{foo: :bar}'                 | { foo: :bar }
+      '{foo: {bar: "bin"}}'         | { foo: { bar: "bin" } }
+    end
+
+    with_them do
+      specify { expect(subject).to eq(expected) }
+    end
+  end
+end