From 40541393153af5e36f212a39febbbb446db871ca Mon Sep 17 00:00:00 2001
From: Ramya Authappan <rauthappan@gitlab.com>
Date: Mon, 22 Jul 2019 05:20:11 +0000
Subject: Merge branch 'sl-qa-staging-65-fix-check-mentions-for-xss-spec' into
 'master'

Updates check_mentions_for_xss_spec to use admin token for creating the user

Closes gitlab-org/quality/staging#65

See merge request gitlab-org/gitlab-ce!30943

(cherry picked from commit 53547792dd7ca9e516ec756e2c0262381641d9c7)

033c1c0c Create user as admin
---
 .../browser_ui/2_plan/issue/check_mentions_for_xss_spec.rb | 14 ++++++++++++--
 1 file changed, 12 insertions(+), 2 deletions(-)

diff --git a/qa/qa/specs/features/browser_ui/2_plan/issue/check_mentions_for_xss_spec.rb b/qa/qa/specs/features/browser_ui/2_plan/issue/check_mentions_for_xss_spec.rb
index afddbff75bd..1bcd80f27af 100644
--- a/qa/qa/specs/features/browser_ui/2_plan/issue/check_mentions_for_xss_spec.rb
+++ b/qa/qa/specs/features/browser_ui/2_plan/issue/check_mentions_for_xss_spec.rb
@@ -4,14 +4,24 @@ module QA
   context 'Plan' do
     describe 'check xss occurence in @mentions in issues' do
       before do
-        Runtime::Browser.visit(:gitlab, Page::Main::Login)
-        Page::Main::Login.perform(&:sign_in_using_credentials)
+        QA::Runtime::Env.personal_access_token = QA::Runtime::Env.admin_personal_access_token
+
+        unless QA::Runtime::Env.personal_access_token
+          Runtime::Browser.visit(:gitlab, Page::Main::Login)
+          Page::Main::Login.perform(&:sign_in_using_admin_credentials)
+        end
 
         user = Resource::User.fabricate_via_api! do |user|
           user.name = "eve <img src=x onerror=alert(2)&lt;img src=x onerror=alert(1)&gt;"
           user.password = "test1234"
         end
 
+        QA::Runtime::Env.personal_access_token = nil
+
+        Page::Main::Menu.perform(&:sign_out) if Page::Main::Menu.perform { |p| p.has_personal_area?(wait: 0) }
+
+        Page::Main::Login.perform(&:sign_in_using_credentials)
+
         project = Resource::Project.fabricate_via_api! do |resource|
           resource.name = 'xss-test-for-mentions-project'
         end
-- 
cgit v1.2.1


From 3a9f91b6a00ccb00408ec9ca4e6e42aab6dc9dc8 Mon Sep 17 00:00:00 2001
From: Fatih Acet <acetfatih@gmail.com>
Date: Mon, 22 Jul 2019 12:01:43 +0000
Subject: Merge branch 'tr-embed-metrics-frontend' into 'master'

Embed metrics charts in issues

See merge request gitlab-org/gitlab-ce!29691

(cherry picked from commit 886a6957ec0d981426219f42d75e0af145a9f7cf)

80feba93 Add ability to embed metrics
b5cdde0c Integrate latest backend changes and feature flag
330b0414 Fix jest test
c260ad82 Migrate TODOs to issues
3a822d99 Seet appropriate default
57c2eb79 Put gfm rendering behind a feature flag
cfeda009 Rename link variable
81ff8d43 Move feature spec into new MR
9d08a5dd Add devensive check on `gon` features object
ca5e00cf Add w-100 style
6e63457b Condense border into shorthand
8de8b6cf Move sidebarAnimationDuration into constants
c9936bf9 Remove extraneous default export
010bb0e3 Reword conditional logic
f4ea4c81 Simplify filter logic
a2f1b4c2 Move styling from css to utility class
da9788df Tidy up component initialization
c69a119b Avoid duplication of `embedded` param supplied by backend
be4b7a11 Remove unnecessary mount
d203980b Apply suggestion to app/assets/javascripts/monitoring/components/charts/area.vue
62ebc6e3 Use object notation
26075703 Add missing class
d9464420 Make sidebarAnimationDuration match actual sidebar animation length
83014858 Remove nextTick and compute groupData object directly
259f700b Rename variable for accuracy
f34890cc Use composite key for dashboard metric groups
92ef33a0 Fix unit test to accommodate for removal of mount call
e2f10bba Remove changelog entry as feature is behind feature flag
c6d8b271 Use graph title as key for embeds
---
 .../javascripts/behaviors/markdown/render_gfm.js   |  4 +
 .../behaviors/markdown/render_metrics.js           | 24 ++++++
 .../monitoring/components/charts/area.vue          | 96 +++++++++++----------
 .../monitoring/components/dashboard.vue            |  7 +-
 .../javascripts/monitoring/components/embed.vue    | 97 ++++++++++++++++++++++
 app/assets/javascripts/monitoring/constants.js     |  2 +
 .../javascripts/monitoring/stores/actions.js       | 12 ++-
 .../monitoring/stores/mutation_types.js            |  1 +
 .../javascripts/monitoring/stores/mutations.js     |  3 +
 app/assets/javascripts/monitoring/stores/state.js  |  1 +
 app/assets/stylesheets/pages/prometheus.scss       |  5 ++
 .../behaviors/markdown/render_metrics_spec.js      | 37 +++++++++
 spec/frontend/monitoring/embed/embed_spec.js       | 78 +++++++++++++++++
 spec/frontend/monitoring/embed/mock_data.js        | 87 +++++++++++++++++++
 spec/frontend/test_setup.js                        |  6 ++
 15 files changed, 410 insertions(+), 50 deletions(-)
 create mode 100644 app/assets/javascripts/behaviors/markdown/render_metrics.js
 create mode 100644 app/assets/javascripts/monitoring/components/embed.vue
 create mode 100644 spec/frontend/behaviors/markdown/render_metrics_spec.js
 create mode 100644 spec/frontend/monitoring/embed/embed_spec.js
 create mode 100644 spec/frontend/monitoring/embed/mock_data.js

diff --git a/app/assets/javascripts/behaviors/markdown/render_gfm.js b/app/assets/javascripts/behaviors/markdown/render_gfm.js
index bfb073fdcdc..789a057caf8 100644
--- a/app/assets/javascripts/behaviors/markdown/render_gfm.js
+++ b/app/assets/javascripts/behaviors/markdown/render_gfm.js
@@ -2,6 +2,7 @@ import $ from 'jquery';
 import syntaxHighlight from '~/syntax_highlight';
 import renderMath from './render_math';
 import renderMermaid from './render_mermaid';
+import renderMetrics from './render_metrics';
 import highlightCurrentUser from './highlight_current_user';
 import initUserPopovers from '../../user_popovers';
 import initMRPopovers from '../../mr_popover';
@@ -17,6 +18,9 @@ $.fn.renderGFM = function renderGFM() {
   highlightCurrentUser(this.find('.gfm-project_member').get());
   initUserPopovers(this.find('.gfm-project_member').get());
   initMRPopovers(this.find('.gfm-merge_request').get());
+  if (gon.features && gon.features.gfmEmbeddedMetrics) {
+    renderMetrics(this.find('.js-render-metrics').get());
+  }
   return this;
 };
 
diff --git a/app/assets/javascripts/behaviors/markdown/render_metrics.js b/app/assets/javascripts/behaviors/markdown/render_metrics.js
new file mode 100644
index 00000000000..252b98610b6
--- /dev/null
+++ b/app/assets/javascripts/behaviors/markdown/render_metrics.js
@@ -0,0 +1,24 @@
+import Vue from 'vue';
+import Metrics from '~/monitoring/components/embed.vue';
+import { createStore } from '~/monitoring/stores';
+
+// TODO: Handle copy-pasting - https://gitlab.com/gitlab-org/gitlab-ce/issues/64369.
+export default function renderMetrics(elements) {
+  if (!elements.length) {
+    return;
+  }
+
+  elements.forEach(element => {
+    const { dashboardUrl } = element.dataset;
+    const MetricsComponent = Vue.extend(Metrics);
+
+    // eslint-disable-next-line no-new
+    new MetricsComponent({
+      el: element,
+      store: createStore(),
+      propsData: {
+        dashboardUrl,
+      },
+    });
+  });
+}
diff --git a/app/assets/javascripts/monitoring/components/charts/area.vue b/app/assets/javascripts/monitoring/components/charts/area.vue
index 454ff4f284e..edf9423c74c 100644
--- a/app/assets/javascripts/monitoring/components/charts/area.vue
+++ b/app/assets/javascripts/monitoring/components/charts/area.vue
@@ -37,7 +37,13 @@ export default {
     },
     projectPath: {
       type: String,
-      required: true,
+      required: false,
+      default: () => '',
+    },
+    showBorder: {
+      type: Boolean,
+      required: false,
+      default: () => false,
     },
     thresholds: {
       type: Array,
@@ -234,52 +240,54 @@ export default {
 </script>
 
 <template>
-  <div class="prometheus-graph col-12 col-lg-6">
-    <div class="prometheus-graph-header">
-      <h5 ref="graphTitle" class="prometheus-graph-title">{{ graphData.title }}</h5>
-      <div ref="graphWidgets" class="prometheus-graph-widgets"><slot></slot></div>
-    </div>
-    <gl-area-chart
-      ref="areaChart"
-      v-bind="$attrs"
-      :data="chartData"
-      :option="chartOptions"
-      :format-tooltip-text="formatTooltipText"
-      :thresholds="thresholds"
-      :width="width"
-      :height="height"
-      @updated="onChartUpdated"
-    >
-      <template v-if="tooltip.isDeployment">
-        <template slot="tooltipTitle">
-          {{ __('Deployed') }}
-        </template>
-        <div slot="tooltipContent" class="d-flex align-items-center">
-          <icon name="commit" class="mr-2" />
-          <gl-link :href="tooltip.commitUrl">{{ tooltip.sha }}</gl-link>
-        </div>
-      </template>
-      <template v-else>
-        <template slot="tooltipTitle">
-          <div class="text-nowrap">
-            {{ tooltip.title }}
+  <div class="col-12 col-lg-6" :class="[showBorder ? 'p-2' : 'p-0']">
+    <div class="prometheus-graph" :class="{ 'prometheus-graph-embed w-100 p-3': showBorder }">
+      <div class="prometheus-graph-header">
+        <h5 ref="graphTitle" class="prometheus-graph-title">{{ graphData.title }}</h5>
+        <div ref="graphWidgets" class="prometheus-graph-widgets"><slot></slot></div>
+      </div>
+      <gl-area-chart
+        ref="areaChart"
+        v-bind="$attrs"
+        :data="chartData"
+        :option="chartOptions"
+        :format-tooltip-text="formatTooltipText"
+        :thresholds="thresholds"
+        :width="width"
+        :height="height"
+        @updated="onChartUpdated"
+      >
+        <template v-if="tooltip.isDeployment">
+          <template slot="tooltipTitle">
+            {{ __('Deployed') }}
+          </template>
+          <div slot="tooltipContent" class="d-flex align-items-center">
+            <icon name="commit" class="mr-2" />
+            <gl-link :href="tooltip.commitUrl">{{ tooltip.sha }}</gl-link>
           </div>
         </template>
-        <template slot="tooltipContent">
-          <div
-            v-for="(content, key) in tooltip.content"
-            :key="key"
-            class="d-flex justify-content-between"
-          >
-            <gl-chart-series-label :color="isMultiSeries ? content.color : ''">
-              {{ content.name }}
-            </gl-chart-series-label>
-            <div class="prepend-left-32">
-              {{ content.value }}
+        <template v-else>
+          <template slot="tooltipTitle">
+            <div class="text-nowrap">
+              {{ tooltip.title }}
             </div>
-          </div>
+          </template>
+          <template slot="tooltipContent">
+            <div
+              v-for="(content, key) in tooltip.content"
+              :key="key"
+              class="d-flex justify-content-between"
+            >
+              <gl-chart-series-label :color="isMultiSeries ? content.color : ''">
+                {{ content.name }}
+              </gl-chart-series-label>
+              <div class="prepend-left-32">
+                {{ content.value }}
+              </div>
+            </div>
+          </template>
         </template>
-      </template>
-    </gl-area-chart>
+      </gl-area-chart>
+    </div>
   </div>
 </template>
diff --git a/app/assets/javascripts/monitoring/components/dashboard.vue b/app/assets/javascripts/monitoring/components/dashboard.vue
index 6eaced0c108..c7c55880040 100644
--- a/app/assets/javascripts/monitoring/components/dashboard.vue
+++ b/app/assets/javascripts/monitoring/components/dashboard.vue
@@ -11,10 +11,9 @@ import MonitorSingleStatChart from './charts/single_stat.vue';
 import PanelType from './panel_type.vue';
 import GraphGroup from './graph_group.vue';
 import EmptyState from './empty_state.vue';
-import { timeWindows, timeWindowsKeyNames } from '../constants';
+import { sidebarAnimationDuration, timeWindows, timeWindowsKeyNames } from '../constants';
 import { getTimeDiff } from '../utils';
 
-const sidebarAnimationDuration = 150;
 let sidebarMutationObserver;
 
 export default {
@@ -370,8 +369,8 @@ export default {
     </div>
     <div v-if="!showEmptyState">
       <graph-group
-        v-for="(groupData, index) in groupsWithData"
-        :key="index"
+        v-for="groupData in groupsWithData"
+        :key="`${groupData.group}.${groupData.priority}`"
         :name="groupData.group"
         :show-panels="showPanels"
       >
diff --git a/app/assets/javascripts/monitoring/components/embed.vue b/app/assets/javascripts/monitoring/components/embed.vue
new file mode 100644
index 00000000000..e17f03de0fd
--- /dev/null
+++ b/app/assets/javascripts/monitoring/components/embed.vue
@@ -0,0 +1,97 @@
+<script>
+import { mapActions, mapState } from 'vuex';
+import GraphGroup from './graph_group.vue';
+import MonitorAreaChart from './charts/area.vue';
+import { sidebarAnimationDuration, timeWindowsKeyNames, timeWindows } from '../constants';
+import { getTimeDiff } from '../utils';
+
+let sidebarMutationObserver;
+
+export default {
+  components: {
+    GraphGroup,
+    MonitorAreaChart,
+  },
+  props: {
+    dashboardUrl: {
+      type: String,
+      required: true,
+    },
+  },
+  data() {
+    return {
+      params: {
+        ...getTimeDiff(timeWindows[timeWindowsKeyNames.eightHours]),
+      },
+      elWidth: 0,
+    };
+  },
+  computed: {
+    ...mapState('monitoringDashboard', ['groups', 'metricsWithData']),
+    groupData() {
+      const groupsWithData = this.groups.filter(group => this.chartsWithData(group.metrics).length);
+      if (groupsWithData.length) {
+        return groupsWithData[0];
+      }
+      return null;
+    },
+  },
+  mounted() {
+    this.setInitialState();
+    this.fetchMetricsData(this.params);
+    sidebarMutationObserver = new MutationObserver(this.onSidebarMutation);
+    sidebarMutationObserver.observe(document.querySelector('.layout-page'), {
+      attributes: true,
+      childList: false,
+      subtree: false,
+    });
+  },
+  beforeDestroy() {
+    if (sidebarMutationObserver) {
+      sidebarMutationObserver.disconnect();
+    }
+  },
+  methods: {
+    ...mapActions('monitoringDashboard', [
+      'fetchMetricsData',
+      'setEndpoints',
+      'setFeatureFlags',
+      'setShowErrorBanner',
+    ]),
+    chartsWithData(charts) {
+      return charts.filter(chart =>
+        chart.metrics.some(metric => this.metricsWithData.includes(metric.metric_id)),
+      );
+    },
+    onSidebarMutation() {
+      setTimeout(() => {
+        this.elWidth = this.$el.clientWidth;
+      }, sidebarAnimationDuration);
+    },
+    setInitialState() {
+      this.setFeatureFlags({
+        prometheusEndpointEnabled: true,
+      });
+      this.setEndpoints({
+        dashboardEndpoint: this.dashboardUrl,
+      });
+      this.setShowErrorBanner(false);
+    },
+  },
+};
+</script>
+<template>
+  <div class="metrics-embed">
+    <div v-if="groupData" class="row w-100 m-n2 pb-4">
+      <monitor-area-chart
+        v-for="graphData in chartsWithData(groupData.metrics)"
+        :key="graphData.title"
+        :graph-data="graphData"
+        :container-width="elWidth"
+        group-id="monitor-area-chart"
+        :project-path="null"
+        :show-border="true"
+      />
+    </div>
+  </div>
+</template>
diff --git a/app/assets/javascripts/monitoring/constants.js b/app/assets/javascripts/monitoring/constants.js
index 26f1bf3f68d..605c95e6da5 100644
--- a/app/assets/javascripts/monitoring/constants.js
+++ b/app/assets/javascripts/monitoring/constants.js
@@ -1,5 +1,7 @@
 import { __ } from '~/locale';
 
+export const sidebarAnimationDuration = 300; // milliseconds.
+
 export const chartHeight = 300;
 
 export const graphTypes = {
diff --git a/app/assets/javascripts/monitoring/stores/actions.js b/app/assets/javascripts/monitoring/stores/actions.js
index 5b3da51e9a6..245cc2eaca3 100644
--- a/app/assets/javascripts/monitoring/stores/actions.js
+++ b/app/assets/javascripts/monitoring/stores/actions.js
@@ -44,6 +44,10 @@ export const setFeatureFlags = (
   commit(types.SET_ADDITIONAL_PANEL_TYPES_ENABLED, additionalPanelTypesEnabled);
 };
 
+export const setShowErrorBanner = ({ commit }, enabled) => {
+  commit(types.SET_SHOW_ERROR_BANNER, enabled);
+};
+
 export const requestMetricsDashboard = ({ commit }) => {
   commit(types.REQUEST_METRICS_DATA);
 };
@@ -99,7 +103,9 @@ export const fetchMetricsData = ({ state, dispatch }, params) => {
     })
     .catch(error => {
       dispatch('receiveMetricsDataFailure', error);
-      createFlash(s__('Metrics|There was an error while retrieving metrics'));
+      if (state.setShowErrorBanner) {
+        createFlash(s__('Metrics|There was an error while retrieving metrics'));
+      }
     });
 };
 
@@ -119,7 +125,9 @@ export const fetchDashboard = ({ state, dispatch }, params) => {
     })
     .catch(error => {
       dispatch('receiveMetricsDashboardFailure', error);
-      createFlash(s__('Metrics|There was an error while retrieving metrics'));
+      if (state.setShowErrorBanner) {
+        createFlash(s__('Metrics|There was an error while retrieving metrics'));
+      }
     });
 };
 
diff --git a/app/assets/javascripts/monitoring/stores/mutation_types.js b/app/assets/javascripts/monitoring/stores/mutation_types.js
index c89daba3df7..4b1aadbcf05 100644
--- a/app/assets/javascripts/monitoring/stores/mutation_types.js
+++ b/app/assets/javascripts/monitoring/stores/mutation_types.js
@@ -16,3 +16,4 @@ export const SET_ALL_DASHBOARDS = 'SET_ALL_DASHBOARDS';
 export const SET_ENDPOINTS = 'SET_ENDPOINTS';
 export const SET_GETTING_STARTED_EMPTY_STATE = 'SET_GETTING_STARTED_EMPTY_STATE';
 export const SET_NO_DATA_EMPTY_STATE = 'SET_NO_DATA_EMPTY_STATE';
+export const SET_SHOW_ERROR_BANNER = 'SET_SHOW_ERROR_BANNER';
diff --git a/app/assets/javascripts/monitoring/stores/mutations.js b/app/assets/javascripts/monitoring/stores/mutations.js
index 0104dcb867d..b19520d6638 100644
--- a/app/assets/javascripts/monitoring/stores/mutations.js
+++ b/app/assets/javascripts/monitoring/stores/mutations.js
@@ -96,4 +96,7 @@ export default {
   [types.SET_ADDITIONAL_PANEL_TYPES_ENABLED](state, enabled) {
     state.additionalPanelTypesEnabled = enabled;
   },
+  [types.SET_SHOW_ERROR_BANNER](state, enabled) {
+    state.showErrorBanner = enabled;
+  },
 };
diff --git a/app/assets/javascripts/monitoring/stores/state.js b/app/assets/javascripts/monitoring/stores/state.js
index e54bb712695..440bdc951e0 100644
--- a/app/assets/javascripts/monitoring/stores/state.js
+++ b/app/assets/javascripts/monitoring/stores/state.js
@@ -12,6 +12,7 @@ export default () => ({
   additionalPanelTypesEnabled: false,
   emptyState: 'gettingStarted',
   showEmptyState: true,
+  showErrorBanner: true,
   groups: [],
   deploymentData: [],
   environments: [],
diff --git a/app/assets/stylesheets/pages/prometheus.scss b/app/assets/stylesheets/pages/prometheus.scss
index 2d600e3aef6..05a4cc168a8 100644
--- a/app/assets/stylesheets/pages/prometheus.scss
+++ b/app/assets/stylesheets/pages/prometheus.scss
@@ -29,6 +29,11 @@
   padding: $gl-padding / 2;
 }
 
+.prometheus-graph-embed {
+  border: 1px solid $border-color;
+  border-radius: $border-radius-default;
+}
+
 .prometheus-graph-header {
   display: flex;
   align-items: center;
diff --git a/spec/frontend/behaviors/markdown/render_metrics_spec.js b/spec/frontend/behaviors/markdown/render_metrics_spec.js
new file mode 100644
index 00000000000..6db0eabc16b
--- /dev/null
+++ b/spec/frontend/behaviors/markdown/render_metrics_spec.js
@@ -0,0 +1,37 @@
+import Vue from 'vue';
+import renderMetrics from '~/behaviors/markdown/render_metrics';
+import { TEST_HOST } from 'helpers/test_constants';
+
+const originalExtend = Vue.extend;
+
+describe('Render metrics for Gitlab Flavoured Markdown', () => {
+  const container = {
+    Metrics() {},
+  };
+
+  let spyExtend;
+
+  beforeEach(() => {
+    Vue.extend = () => container.Metrics;
+    spyExtend = jest.spyOn(Vue, 'extend');
+  });
+
+  afterEach(() => {
+    Vue.extend = originalExtend;
+  });
+
+  it('does nothing when no elements are found', () => {
+    renderMetrics([]);
+
+    expect(spyExtend).not.toHaveBeenCalled();
+  });
+
+  it('renders a vue component when elements are found', () => {
+    const element = document.createElement('div');
+    element.setAttribute('data-dashboard-url', TEST_HOST);
+
+    renderMetrics([element]);
+
+    expect(spyExtend).toHaveBeenCalled();
+  });
+});
diff --git a/spec/frontend/monitoring/embed/embed_spec.js b/spec/frontend/monitoring/embed/embed_spec.js
new file mode 100644
index 00000000000..3b18a0f77c7
--- /dev/null
+++ b/spec/frontend/monitoring/embed/embed_spec.js
@@ -0,0 +1,78 @@
+import { createLocalVue, shallowMount } from '@vue/test-utils';
+import Vuex from 'vuex';
+import Embed from '~/monitoring/components/embed.vue';
+import MonitorAreaChart from '~/monitoring/components/charts/area.vue';
+import { TEST_HOST } from 'helpers/test_constants';
+import { groups, initialState, metricsData, metricsWithData } from './mock_data';
+
+const localVue = createLocalVue();
+localVue.use(Vuex);
+
+describe('Embed', () => {
+  let wrapper;
+  let store;
+  let actions;
+
+  function mountComponent() {
+    wrapper = shallowMount(Embed, {
+      localVue,
+      store,
+      propsData: {
+        dashboardUrl: TEST_HOST,
+      },
+    });
+  }
+
+  beforeEach(() => {
+    actions = {
+      setFeatureFlags: () => {},
+      setShowErrorBanner: () => {},
+      setEndpoints: () => {},
+      fetchMetricsData: () => {},
+    };
+
+    store = new Vuex.Store({
+      modules: {
+        monitoringDashboard: {
+          namespaced: true,
+          actions,
+          state: initialState,
+        },
+      },
+    });
+  });
+
+  afterEach(() => {
+    if (wrapper) {
+      wrapper.destroy();
+    }
+  });
+
+  describe('no metrics are available yet', () => {
+    beforeEach(() => {
+      mountComponent();
+    });
+
+    it('shows an empty state when no metrics are present', () => {
+      expect(wrapper.find('.metrics-embed').exists()).toBe(true);
+      expect(wrapper.find(MonitorAreaChart).exists()).toBe(false);
+    });
+  });
+
+  describe('metrics are available', () => {
+    beforeEach(() => {
+      store.state.monitoringDashboard.groups = groups;
+      store.state.monitoringDashboard.groups[0].metrics = metricsData;
+      store.state.monitoringDashboard.metricsWithData = metricsWithData;
+
+      mountComponent();
+    });
+
+    it('shows a chart when metrics are present', () => {
+      wrapper.setProps({});
+      expect(wrapper.find('.metrics-embed').exists()).toBe(true);
+      expect(wrapper.find(MonitorAreaChart).exists()).toBe(true);
+      expect(wrapper.findAll(MonitorAreaChart).length).toBe(2);
+    });
+  });
+});
diff --git a/spec/frontend/monitoring/embed/mock_data.js b/spec/frontend/monitoring/embed/mock_data.js
new file mode 100644
index 00000000000..df4acb82e95
--- /dev/null
+++ b/spec/frontend/monitoring/embed/mock_data.js
@@ -0,0 +1,87 @@
+export const metricsWithData = [15, 16];
+
+export const groups = [
+  {
+    panels: [
+      {
+        title: 'Memory Usage (Total)',
+        type: 'area-chart',
+        y_label: 'Total Memory Used',
+        weight: 4,
+        metrics: [
+          {
+            id: 'system_metrics_kubernetes_container_memory_total',
+            metric_id: 15,
+          },
+        ],
+      },
+      {
+        title: 'Core Usage (Total)',
+        type: 'area-chart',
+        y_label: 'Total Cores',
+        weight: 3,
+        metrics: [
+          {
+            id: 'system_metrics_kubernetes_container_cores_total',
+            metric_id: 16,
+          },
+        ],
+      },
+    ],
+  },
+];
+
+export const metrics = [
+  {
+    id: 'system_metrics_kubernetes_container_memory_total',
+    metric_id: 15,
+  },
+  {
+    id: 'system_metrics_kubernetes_container_cores_total',
+    metric_id: 16,
+  },
+];
+
+const queries = [
+  {
+    result: [
+      {
+        values: [
+          ['Mon', 1220],
+          ['Tue', 932],
+          ['Wed', 901],
+          ['Thu', 934],
+          ['Fri', 1290],
+          ['Sat', 1330],
+          ['Sun', 1320],
+        ],
+      },
+    ],
+  },
+];
+
+export const metricsData = [
+  {
+    queries,
+    metrics: [
+      {
+        metric_id: 15,
+      },
+    ],
+  },
+  {
+    queries,
+    metrics: [
+      {
+        metric_id: 16,
+      },
+    ],
+  },
+];
+
+export const initialState = {
+  monitoringDashboard: {},
+  groups: [],
+  metricsWithData: [],
+  useDashboardEndpoint: true,
+};
diff --git a/spec/frontend/test_setup.js b/spec/frontend/test_setup.js
index 634c78ec029..e4d62b044ca 100644
--- a/spec/frontend/test_setup.js
+++ b/spec/frontend/test_setup.js
@@ -69,3 +69,9 @@ Object.entries(jqueryMatchers).forEach(([matcherName, matcherFactory]) => {
 
 // Tech debt issue TBD
 testUtilsConfig.logModifiedComponents = false;
+
+// Basic stub for MutationObserver
+global.MutationObserver = () => ({
+  disconnect: () => {},
+  observe: () => {},
+});
-- 
cgit v1.2.1


From 2629cd9c0fdc370b981992576daccc1bef92129c Mon Sep 17 00:00:00 2001
From: Marcia Ramos <marcia@gitlab.com>
Date: Mon, 22 Jul 2019 12:58:37 +0000
Subject: Merge branch 'docs-le-for-pages-bug-warning' into 'master'

Docs: add alert for bugs - Pages' LE integration

See merge request gitlab-org/gitlab-ce!31002

(cherry picked from commit b40630e134c09e02322d9b17ddf45667d2fa6573)

69744896 Add alert for bugs
d1c583d2 Copy edit alert, add tbs steps
---
 .../lets_encrypt_integration.md                    | 23 ++++++++++++++++++++--
 1 file changed, 21 insertions(+), 2 deletions(-)

diff --git a/doc/user/project/pages/custom_domains_ssl_tls_certification/lets_encrypt_integration.md b/doc/user/project/pages/custom_domains_ssl_tls_certification/lets_encrypt_integration.md
index 7675a5dd9d4..4588375738e 100644
--- a/doc/user/project/pages/custom_domains_ssl_tls_certification/lets_encrypt_integration.md
+++ b/doc/user/project/pages/custom_domains_ssl_tls_certification/lets_encrypt_integration.md
@@ -15,6 +15,12 @@ GitLab does it for you, out-of-the-box.
 [Let's Encrypt](https://letsencrypt.org) is a free, automated, and
 open source Certificate Authority.
 
+CAUTION: **Caution:**
+This feature is in **beta** and might present bugs and UX issues
+such as [#64870](https://gitlab.com/gitlab-org/gitlab-ce/issues/64870).
+See all the related issues linked from this [issue's description](https://gitlab.com/gitlab-org/gitlab-ce/issues/28996)
+for more information.
+
 ## Requirements
 
 Before you can enable automatic provisioning of a SSL certificate for your domain, make sure you have:
@@ -55,9 +61,22 @@ associated Pages domain. It will be also renewed automatically by GitLab.
 > - If you already have SSL certificate in domain settings it
 >   will continue to work until it will be replaced by Let's Encrypt's certificate.
 
-<!-- ## Troubleshooting
+## Troubleshooting
+
+### Error "Certificate misses intermediates"
+
+If you get an error **Certificate misses intermediates** while trying to enable Let's Encrypt integration for your domain, follow the steps below:
+
+1. Go to your project's **Settings > Pages**.
+1. Turn off **Force HTTPS** if it's turned on.
+1. Click **Details** on your domain.
+1. Click the **Edit** button in the top right corner of domain details page.
+1. Enable Let's Encrypt integration.
+1. Click **Save**.
+1. Go to your project's **Settings > Pages**.
+1. Turn on **Force HTTPS**.
 
-Include any troubleshooting steps that you can foresee. If you know beforehand what issues
+<!-- Include any troubleshooting steps that you can foresee. If you know beforehand what issues
 one might have when setting this up, or when something is changed, or on upgrading, it's
 important to describe those, too. Think of things that may go wrong and include them here.
 This is important to minimize requests for support, and to avoid doc comments with
-- 
cgit v1.2.1


From 54cc84ac54c7f32b0c8a70beed53ab740c069b0e Mon Sep 17 00:00:00 2001
From: Nick Thomas <nick@gitlab.com>
Date: Mon, 22 Jul 2019 13:58:26 +0000
Subject: Merge branch 'dm-submodule-helper-routing' into 'master'

Ensure SubmoduleHelper works outside view context

Closes #64833

See merge request gitlab-org/gitlab-ce!31005

(cherry picked from commit 8b284a51c03d6ab0634f115b38014d5a87f73d57)

26ac53b3 Ensure SubmoduleHelper works outside view context
---
 app/helpers/submodule_helper.rb                    | 12 ++--
 .../unreleased/dm-submodule-helper-routing.yml     |  5 ++
 spec/helpers/submodule_helper_spec.rb              | 81 ++++++++++++----------
 3 files changed, 58 insertions(+), 40 deletions(-)
 create mode 100644 changelogs/unreleased/dm-submodule-helper-routing.yml

diff --git a/app/helpers/submodule_helper.rb b/app/helpers/submodule_helper.rb
index 35e04b0ced3..9a281065b90 100644
--- a/app/helpers/submodule_helper.rb
+++ b/app/helpers/submodule_helper.rb
@@ -34,8 +34,8 @@ module SubmoduleHelper
       project.sub!(/\.git\z/, '')
 
       if self_url?(url, namespace, project)
-        [namespace_project_path(namespace, project),
-         namespace_project_tree_path(namespace, project, submodule_item_id)]
+        [url_helpers.namespace_project_path(namespace, project),
+         url_helpers.namespace_project_tree_path(namespace, project, submodule_item_id)]
       elsif relative_self_url?(url)
         relative_self_links(url, submodule_item_id, repository.project)
       elsif github_dot_com_url?(url)
@@ -99,8 +99,8 @@ module SubmoduleHelper
 
     begin
       [
-        namespace_project_path(target_namespace_path, submodule_base),
-        namespace_project_tree_path(target_namespace_path, submodule_base, commit)
+        url_helpers.namespace_project_path(target_namespace_path, submodule_base),
+        url_helpers.namespace_project_tree_path(target_namespace_path, submodule_base, commit)
       ]
     rescue ActionController::UrlGenerationError
       [nil, nil]
@@ -118,4 +118,8 @@ module SubmoduleHelper
   rescue URI::InvalidURIError
     nil
   end
+
+  def url_helpers
+    Gitlab::Routing.url_helpers
+  end
 end
diff --git a/changelogs/unreleased/dm-submodule-helper-routing.yml b/changelogs/unreleased/dm-submodule-helper-routing.yml
new file mode 100644
index 00000000000..779d4d167df
--- /dev/null
+++ b/changelogs/unreleased/dm-submodule-helper-routing.yml
@@ -0,0 +1,5 @@
+---
+title: Fix bug that caused diffs not to show on MRs with changes to submodules
+merge_request:
+author:
+type: fixed
diff --git a/spec/helpers/submodule_helper_spec.rb b/spec/helpers/submodule_helper_spec.rb
index ea48c69e0ae..b922b910c89 100644
--- a/spec/helpers/submodule_helper_spec.rb
+++ b/spec/helpers/submodule_helper_spec.rb
@@ -3,15 +3,11 @@ require 'spec_helper'
 describe SubmoduleHelper do
   include RepoHelpers
 
-  describe 'submodule links' do
-    let(:submodule_item) { double(id: 'hash', path: 'rack') }
-    let(:config) { Gitlab.config.gitlab }
-    let(:repo) { double }
-
-    before do
-      self.instance_variable_set(:@repository, repo)
-    end
+  let(:submodule_item) { double(id: 'hash', path: 'rack') }
+  let(:config) { Gitlab.config.gitlab }
+  let(:repo) { double }
 
+  shared_examples 'submodule_links' do
     context 'submodule on self' do
       before do
         allow(Gitlab.config.gitlab).to receive(:protocol).and_return('http') # set this just to be sure
@@ -21,28 +17,28 @@ describe SubmoduleHelper do
         allow(Gitlab.config.gitlab_shell).to receive(:ssh_port).and_return(22) # set this just to be sure
         allow(Gitlab.config.gitlab_shell).to receive(:ssh_path_prefix).and_return(Settings.send(:build_gitlab_shell_ssh_path_prefix))
         stub_url([config.user, '@', config.host, ':gitlab-org/gitlab-ce.git'].join(''))
-        expect(submodule_links(submodule_item)).to eq([namespace_project_path('gitlab-org', 'gitlab-ce'), namespace_project_tree_path('gitlab-org', 'gitlab-ce', 'hash')])
+        expect(subject).to eq([namespace_project_path('gitlab-org', 'gitlab-ce'), namespace_project_tree_path('gitlab-org', 'gitlab-ce', 'hash')])
       end
 
       it 'detects ssh on non-standard port' do
         allow(Gitlab.config.gitlab_shell).to receive(:ssh_port).and_return(2222)
         allow(Gitlab.config.gitlab_shell).to receive(:ssh_path_prefix).and_return(Settings.send(:build_gitlab_shell_ssh_path_prefix))
         stub_url(['ssh://', config.user, '@', config.host, ':2222/gitlab-org/gitlab-ce.git'].join(''))
-        expect(submodule_links(submodule_item)).to eq([namespace_project_path('gitlab-org', 'gitlab-ce'), namespace_project_tree_path('gitlab-org', 'gitlab-ce', 'hash')])
+        expect(subject).to eq([namespace_project_path('gitlab-org', 'gitlab-ce'), namespace_project_tree_path('gitlab-org', 'gitlab-ce', 'hash')])
       end
 
       it 'detects http on standard port' do
         allow(Gitlab.config.gitlab).to receive(:port).and_return(80)
         allow(Gitlab.config.gitlab).to receive(:url).and_return(Settings.send(:build_gitlab_url))
         stub_url(['http://', config.host, '/gitlab-org/gitlab-ce.git'].join(''))
-        expect(submodule_links(submodule_item)).to eq([namespace_project_path('gitlab-org', 'gitlab-ce'), namespace_project_tree_path('gitlab-org', 'gitlab-ce', 'hash')])
+        expect(subject).to eq([namespace_project_path('gitlab-org', 'gitlab-ce'), namespace_project_tree_path('gitlab-org', 'gitlab-ce', 'hash')])
       end
 
       it 'detects http on non-standard port' do
         allow(Gitlab.config.gitlab).to receive(:port).and_return(3000)
         allow(Gitlab.config.gitlab).to receive(:url).and_return(Settings.send(:build_gitlab_url))
         stub_url(['http://', config.host, ':3000/gitlab-org/gitlab-ce.git'].join(''))
-        expect(submodule_links(submodule_item)).to eq([namespace_project_path('gitlab-org', 'gitlab-ce'), namespace_project_tree_path('gitlab-org', 'gitlab-ce', 'hash')])
+        expect(subject).to eq([namespace_project_path('gitlab-org', 'gitlab-ce'), namespace_project_tree_path('gitlab-org', 'gitlab-ce', 'hash')])
       end
 
       it 'works with relative_url_root' do
@@ -50,7 +46,7 @@ describe SubmoduleHelper do
         allow(Gitlab.config.gitlab).to receive(:relative_url_root).and_return('/gitlab/root')
         allow(Gitlab.config.gitlab).to receive(:url).and_return(Settings.send(:build_gitlab_url))
         stub_url(['http://', config.host, '/gitlab/root/gitlab-org/gitlab-ce.git'].join(''))
-        expect(submodule_links(submodule_item)).to eq([namespace_project_path('gitlab-org', 'gitlab-ce'), namespace_project_tree_path('gitlab-org', 'gitlab-ce', 'hash')])
+        expect(subject).to eq([namespace_project_path('gitlab-org', 'gitlab-ce'), namespace_project_tree_path('gitlab-org', 'gitlab-ce', 'hash')])
       end
 
       it 'works with subgroups' do
@@ -58,34 +54,34 @@ describe SubmoduleHelper do
         allow(Gitlab.config.gitlab).to receive(:relative_url_root).and_return('/gitlab/root')
         allow(Gitlab.config.gitlab).to receive(:url).and_return(Settings.send(:build_gitlab_url))
         stub_url(['http://', config.host, '/gitlab/root/gitlab-org/sub/gitlab-ce.git'].join(''))
-        expect(submodule_links(submodule_item)).to eq([namespace_project_path('gitlab-org/sub', 'gitlab-ce'), namespace_project_tree_path('gitlab-org/sub', 'gitlab-ce', 'hash')])
+        expect(subject).to eq([namespace_project_path('gitlab-org/sub', 'gitlab-ce'), namespace_project_tree_path('gitlab-org/sub', 'gitlab-ce', 'hash')])
       end
     end
 
     context 'submodule on github.com' do
       it 'detects ssh' do
         stub_url('git@github.com:gitlab-org/gitlab-ce.git')
-        expect(submodule_links(submodule_item)).to eq(['https://github.com/gitlab-org/gitlab-ce', 'https://github.com/gitlab-org/gitlab-ce/tree/hash'])
+        expect(subject).to eq(['https://github.com/gitlab-org/gitlab-ce', 'https://github.com/gitlab-org/gitlab-ce/tree/hash'])
       end
 
       it 'detects http' do
         stub_url('http://github.com/gitlab-org/gitlab-ce.git')
-        expect(submodule_links(submodule_item)).to eq(['https://github.com/gitlab-org/gitlab-ce', 'https://github.com/gitlab-org/gitlab-ce/tree/hash'])
+        expect(subject).to eq(['https://github.com/gitlab-org/gitlab-ce', 'https://github.com/gitlab-org/gitlab-ce/tree/hash'])
       end
 
       it 'detects https' do
         stub_url('https://github.com/gitlab-org/gitlab-ce.git')
-        expect(submodule_links(submodule_item)).to eq(['https://github.com/gitlab-org/gitlab-ce', 'https://github.com/gitlab-org/gitlab-ce/tree/hash'])
+        expect(subject).to eq(['https://github.com/gitlab-org/gitlab-ce', 'https://github.com/gitlab-org/gitlab-ce/tree/hash'])
       end
 
       it 'handles urls with no .git on the end' do
         stub_url('http://github.com/gitlab-org/gitlab-ce')
-        expect(submodule_links(submodule_item)).to eq(['https://github.com/gitlab-org/gitlab-ce', 'https://github.com/gitlab-org/gitlab-ce/tree/hash'])
+        expect(subject).to eq(['https://github.com/gitlab-org/gitlab-ce', 'https://github.com/gitlab-org/gitlab-ce/tree/hash'])
       end
 
       it 'returns original with non-standard url' do
         stub_url('http://github.com/another/gitlab-org/gitlab-ce.git')
-        expect(submodule_links(submodule_item)).to eq([repo.submodule_url_for, nil])
+        expect(subject).to eq([repo.submodule_url_for, nil])
       end
     end
 
@@ -97,39 +93,39 @@ describe SubmoduleHelper do
         allow(repo).to receive(:project).and_return(project)
 
         stub_url('./')
-        expect(submodule_links(submodule_item)).to eq(["/master-project/#{project.path}", "/master-project/#{project.path}/tree/hash"])
+        expect(subject).to eq(["/master-project/#{project.path}", "/master-project/#{project.path}/tree/hash"])
       end
     end
 
     context 'submodule on gitlab.com' do
       it 'detects ssh' do
         stub_url('git@gitlab.com:gitlab-org/gitlab-ce.git')
-        expect(submodule_links(submodule_item)).to eq(['https://gitlab.com/gitlab-org/gitlab-ce', 'https://gitlab.com/gitlab-org/gitlab-ce/tree/hash'])
+        expect(subject).to eq(['https://gitlab.com/gitlab-org/gitlab-ce', 'https://gitlab.com/gitlab-org/gitlab-ce/tree/hash'])
       end
 
       it 'detects http' do
         stub_url('http://gitlab.com/gitlab-org/gitlab-ce.git')
-        expect(submodule_links(submodule_item)).to eq(['https://gitlab.com/gitlab-org/gitlab-ce', 'https://gitlab.com/gitlab-org/gitlab-ce/tree/hash'])
+        expect(subject).to eq(['https://gitlab.com/gitlab-org/gitlab-ce', 'https://gitlab.com/gitlab-org/gitlab-ce/tree/hash'])
       end
 
       it 'detects https' do
         stub_url('https://gitlab.com/gitlab-org/gitlab-ce.git')
-        expect(submodule_links(submodule_item)).to eq(['https://gitlab.com/gitlab-org/gitlab-ce', 'https://gitlab.com/gitlab-org/gitlab-ce/tree/hash'])
+        expect(subject).to eq(['https://gitlab.com/gitlab-org/gitlab-ce', 'https://gitlab.com/gitlab-org/gitlab-ce/tree/hash'])
       end
 
       it 'handles urls with no .git on the end' do
         stub_url('http://gitlab.com/gitlab-org/gitlab-ce')
-        expect(submodule_links(submodule_item)).to eq(['https://gitlab.com/gitlab-org/gitlab-ce', 'https://gitlab.com/gitlab-org/gitlab-ce/tree/hash'])
+        expect(subject).to eq(['https://gitlab.com/gitlab-org/gitlab-ce', 'https://gitlab.com/gitlab-org/gitlab-ce/tree/hash'])
       end
 
       it 'handles urls with trailing whitespace' do
         stub_url('http://gitlab.com/gitlab-org/gitlab-ce.git  ')
-        expect(submodule_links(submodule_item)).to eq(['https://gitlab.com/gitlab-org/gitlab-ce', 'https://gitlab.com/gitlab-org/gitlab-ce/tree/hash'])
+        expect(subject).to eq(['https://gitlab.com/gitlab-org/gitlab-ce', 'https://gitlab.com/gitlab-org/gitlab-ce/tree/hash'])
       end
 
       it 'returns original with non-standard url' do
         stub_url('http://gitlab.com/another/gitlab-org/gitlab-ce.git')
-        expect(submodule_links(submodule_item)).to eq([repo.submodule_url_for, nil])
+        expect(subject).to eq([repo.submodule_url_for, nil])
       end
     end
 
@@ -137,27 +133,25 @@ describe SubmoduleHelper do
       it 'sanitizes unsupported protocols' do
         stub_url('javascript:alert("XSS");')
 
-        expect(helper.submodule_links(submodule_item)).to eq([nil, nil])
+        expect(subject).to eq([nil, nil])
       end
 
       it 'sanitizes unsupported protocols disguised as a repository URL' do
         stub_url('javascript:alert("XSS");foo/bar.git')
 
-        expect(helper.submodule_links(submodule_item)).to eq([nil, nil])
+        expect(subject).to eq([nil, nil])
       end
 
       it 'sanitizes invalid URL with extended ASCII' do
         stub_url('é')
 
-        expect(helper.submodule_links(submodule_item)).to eq([nil, nil])
+        expect(subject).to eq([nil, nil])
       end
 
       it 'returns original' do
         stub_url('http://mygitserver.com/gitlab-org/gitlab-ce')
-        expect(submodule_links(submodule_item)).to eq([repo.submodule_url_for, nil])
 
-        stub_url('http://mygitserver.com/gitlab-org/gitlab-ce.git')
-        expect(submodule_links(submodule_item)).to eq([repo.submodule_url_for, nil])
+        expect(subject).to eq([repo.submodule_url_for, nil])
       end
     end
 
@@ -168,8 +162,7 @@ describe SubmoduleHelper do
 
       def expect_relative_link_to_resolve_to(relative_path, expected_path)
         allow(repo).to receive(:submodule_url_for).and_return(relative_path)
-
-        result = submodule_links(submodule_item)
+        result = subject
 
         expect(result).to eq([expected_path, "#{expected_path}/tree/#{submodule_item.id}"])
       end
@@ -190,7 +183,7 @@ describe SubmoduleHelper do
         it 'returns nil' do
           allow(repo).to receive(:submodule_url_for).and_return('../../test.git')
 
-          result = submodule_links(submodule_item)
+          result = subject
 
           expect(result).to eq([nil, nil])
         end
@@ -200,7 +193,7 @@ describe SubmoduleHelper do
         it 'returns nil because it is not possible to have repo nested under another repo' do
           allow(repo).to receive(:submodule_url_for).and_return('./test.git')
 
-          result = submodule_links(submodule_item)
+          result = subject
 
           expect(result).to eq([nil, nil])
         end
@@ -238,6 +231,22 @@ describe SubmoduleHelper do
     end
   end
 
+  context 'as view helpers in view context' do
+    subject { helper.submodule_links(submodule_item) }
+
+    before do
+      self.instance_variable_set(:@repository, repo)
+    end
+
+    it_behaves_like 'submodule_links'
+  end
+
+  context 'as stand-alone module' do
+    subject { described_class.submodule_links(submodule_item, nil, repo) }
+
+    it_behaves_like 'submodule_links'
+  end
+
   def stub_url(url)
     allow(repo).to receive(:submodule_url_for).and_return(url)
   end
-- 
cgit v1.2.1


From b0e0d34d55bfce0316cf5f5ad172e6bb918b8ad5 Mon Sep 17 00:00:00 2001
From: Nick Thomas <nick@gitlab.com>
Date: Mon, 22 Jul 2019 15:38:08 +0000
Subject: Merge branch
 '64870-can-t-save-pages-domain-form-with-let-s-encrypt-enabled-if-current-certificate-is-outdated'
 into 'master'

Resolve "Can't save pages domain form with Let's Encrypt enabled if current certificate is outdated"

Closes #64870

See merge request gitlab-org/gitlab-ce!30995

(cherry picked from commit 8073b0554fc8f7dddc250025aaf5cedd09f423cb)

9c0f4286 Validate certificate chain only if it's changed
---
 app/models/pages_domain.rb                         |  2 +-
 ...-enabled-if-current-certificate-is-outdated.yml |  6 ++
 spec/factories/pages_domains.rb                    | 84 ++++++++++++++++++++++
 spec/models/pages_domain_spec.rb                   | 24 +++++++
 4 files changed, 115 insertions(+), 1 deletion(-)
 create mode 100644 changelogs/unreleased/64870-can-t-save-pages-domain-form-with-let-s-encrypt-enabled-if-current-certificate-is-outdated.yml

diff --git a/app/models/pages_domain.rb b/app/models/pages_domain.rb
index e6e491634ab..27c122d3559 100644
--- a/app/models/pages_domain.rb
+++ b/app/models/pages_domain.rb
@@ -22,7 +22,7 @@ class PagesDomain < ApplicationRecord
 
   validate :validate_pages_domain
   validate :validate_matching_key, if: ->(domain) { domain.certificate.present? || domain.key.present? }
-  validate :validate_intermediates, if: ->(domain) { domain.certificate.present? }
+  validate :validate_intermediates, if: ->(domain) { domain.certificate.present? && domain.certificate_changed? }
 
   attr_encrypted :key,
     mode: :per_attribute_iv_and_salt,
diff --git a/changelogs/unreleased/64870-can-t-save-pages-domain-form-with-let-s-encrypt-enabled-if-current-certificate-is-outdated.yml b/changelogs/unreleased/64870-can-t-save-pages-domain-form-with-let-s-encrypt-enabled-if-current-certificate-is-outdated.yml
new file mode 100644
index 00000000000..291901d64ed
--- /dev/null
+++ b/changelogs/unreleased/64870-can-t-save-pages-domain-form-with-let-s-encrypt-enabled-if-current-certificate-is-outdated.yml
@@ -0,0 +1,6 @@
+---
+title: Fix "Certificate misses intermediates" UI error when enabling Let's Encrypt
+  integration for pages domain
+merge_request: 30995
+author:
+type: fixed
diff --git a/spec/factories/pages_domains.rb b/spec/factories/pages_domains.rb
index 3e0baab04ce..e441dfcf229 100644
--- a/spec/factories/pages_domains.rb
+++ b/spec/factories/pages_domains.rb
@@ -166,6 +166,90 @@ pu/xO28QOG8=
 -----END CERTIFICATE-----'
     end
 
+    trait :with_trusted_expired_chain do
+      # This contains
+      # Let's Encrypt Authority X3
+      # DST Root CA X3
+      certificate '-----BEGIN CERTIFICATE-----
+MIIFSjCCBDKgAwIBAgISAw24xGWrFotvTBa6AZI/pzq1MA0GCSqGSIb3DQEBCwUA
+MEoxCzAJBgNVBAYTAlVTMRYwFAYDVQQKEw1MZXQncyBFbmNyeXB0MSMwIQYDVQQD
+ExpMZXQncyBFbmNyeXB0IEF1dGhvcml0eSBYMzAeFw0xOTAzMDcxNzU5NTZaFw0x
+OTA2MDUxNzU5NTZaMBQxEjAQBgNVBAMTCXN5dHNlLmNvbTCCASIwDQYJKoZIhvcN
+AQEBBQADggEPADCCAQoCggEBALtIpQuqeZN6OgEE+y2UoGC/31Vt9NAeQWvTuWWO
+nMn/MvDJiw8731Dx4DDbMjhF50UBE20a9iAu2nhlxcsuuIITk2MXKMEgPtqSbwM7
+Mg0/WvgrBOWnF9CpdD3qcsjtstT6Djij06VfMfUrRZzMkGgbGzudR0cShKPmkBVU
+LgB6crFmSQ/qHt5PzBivdexCUpz5WzSKU5UWYFx2UnkSLykvEJuUr3Nn4/o9oyKw
+Qoiq354S262mFuMW+s6wQdMNNkwj41OqCwAGbqq7YUYLDc8OQiRC2LcqSO5yYnnA
+0lNfbEatZ1BzHiDjTH7wMUtwcLGHsZ1C5ZmORD2s2gtGiRkCAwEAAaOCAl4wggJa
+MA4GA1UdDwEB/wQEAwIFoDAdBgNVHSUEFjAUBggrBgEFBQcDAQYIKwYBBQUHAwIw
+DAYDVR0TAQH/BAIwADAdBgNVHQ4EFgQUAMn3t1s4zXdOQbJFOP1riSwjuGkwHwYD
+VR0jBBgwFoAUqEpqYwR93brm0Tm3pkVl7/Oo7KEwbwYIKwYBBQUHAQEEYzBhMC4G
+CCsGAQUFBzABhiJodHRwOi8vb2NzcC5pbnQteDMubGV0c2VuY3J5cHQub3JnMC8G
+CCsGAQUFBzAChiNodHRwOi8vY2VydC5pbnQteDMubGV0c2VuY3J5cHQub3JnLzAU
+BgNVHREEDTALgglzeXRzZS5jb20wTAYDVR0gBEUwQzAIBgZngQwBAgEwNwYLKwYB
+BAGC3xMBAQEwKDAmBggrBgEFBQcCARYaaHR0cDovL2Nwcy5sZXRzZW5jcnlwdC5v
+cmcwggEEBgorBgEEAdZ5AgQCBIH1BIHyAPAAdQB0ftqDMa0zEJEhnM4lT0Jwwr/9
+XkIgCMY3NXnmEHvMVgAAAWlZhr4pAAAEAwBGMEQCIBEA+3oiM1UJKY1kajBO5Aoz
+9AZMMlImaR1X5hFIPr95AiBXGIACuXUDLchB0kT8VIG/jM4f9iuXMoYCoKNJggNM
+/gB3ACk8UZZUyDlluqpQ/FgH1Ldvv1h6KXLcpMMM9OVFR/R4AAABaVmGv/AAAAQD
+AEgwRgIhANeTA7H51SZUmcT2ldtumFYX6/OkOr0fdvze72U0j9U9AiEAjSOSVQmi
+ZdYK6u3JYkDVOWsEzyKwjPWod8UN5K3ej0EwDQYJKoZIhvcNAQELBQADggEBAJev
+ArtxZVVTmLghV0O7471J1mN1fVC2p6b3AsK/TqrI7aiq8XuQq76KmUsB+U05MTXH
+3sYiHm+/RJ7+ljiKVIC8ZfbQsHo5I+F1CNMo6JB6z8Z+bOeRkoves5FNYmiJnUjO
+uoGzt//CyldbX1dEPVNuU7P0s2wZ6Bubump2LoapGIiGxQJfeb0vj0TQzfRacTIZ
+x9U5E/D0y0iewX4kPHK17QDBsSL9WlqsRzFAkQjJ9XWUVn3BO7JG3WU47iOuykby
+y2HmOWUxjv1Yf/H/OYRBiuSCR4LhrE5Ze4tTo2AByrXQ5h7ezjDJQqnKBP5NuwIq
+7NuX+D2esUNos/D6uJg=
+-----END CERTIFICATE-----
+-----BEGIN CERTIFICATE-----
+MIIEkjCCA3qgAwIBAgIQCgFBQgAAAVOFc2oLheynCDANBgkqhkiG9w0BAQsFADA/
+MSQwIgYDVQQKExtEaWdpdGFsIFNpZ25hdHVyZSBUcnVzdCBDby4xFzAVBgNVBAMT
+DkRTVCBSb290IENBIFgzMB4XDTE2MDMxNzE2NDA0NloXDTIxMDMxNzE2NDA0Nlow
+SjELMAkGA1UEBhMCVVMxFjAUBgNVBAoTDUxldCdzIEVuY3J5cHQxIzAhBgNVBAMT
+GkxldCdzIEVuY3J5cHQgQXV0aG9yaXR5IFgzMIIBIjANBgkqhkiG9w0BAQEFAAOC
+AQ8AMIIBCgKCAQEAnNMM8FrlLke3cl03g7NoYzDq1zUmGSXhvb418XCSL7e4S0EF
+q6meNQhY7LEqxGiHC6PjdeTm86dicbp5gWAf15Gan/PQeGdxyGkOlZHP/uaZ6WA8
+SMx+yk13EiSdRxta67nsHjcAHJyse6cF6s5K671B5TaYucv9bTyWaN8jKkKQDIZ0
+Z8h/pZq4UmEUEz9l6YKHy9v6Dlb2honzhT+Xhq+w3Brvaw2VFn3EK6BlspkENnWA
+a6xK8xuQSXgvopZPKiAlKQTGdMDQMc2PMTiVFrqoM7hD8bEfwzB/onkxEz0tNvjj
+/PIzark5McWvxI0NHWQWM6r6hCm21AvA2H3DkwIDAQABo4IBfTCCAXkwEgYDVR0T
+AQH/BAgwBgEB/wIBADAOBgNVHQ8BAf8EBAMCAYYwfwYIKwYBBQUHAQEEczBxMDIG
+CCsGAQUFBzABhiZodHRwOi8vaXNyZy50cnVzdGlkLm9jc3AuaWRlbnRydXN0LmNv
+bTA7BggrBgEFBQcwAoYvaHR0cDovL2FwcHMuaWRlbnRydXN0LmNvbS9yb290cy9k
+c3Ryb290Y2F4My5wN2MwHwYDVR0jBBgwFoAUxKexpHsscfrb4UuQdf/EFWCFiRAw
+VAYDVR0gBE0wSzAIBgZngQwBAgEwPwYLKwYBBAGC3xMBAQEwMDAuBggrBgEFBQcC
+ARYiaHR0cDovL2Nwcy5yb290LXgxLmxldHNlbmNyeXB0Lm9yZzA8BgNVHR8ENTAz
+MDGgL6AthitodHRwOi8vY3JsLmlkZW50cnVzdC5jb20vRFNUUk9PVENBWDNDUkwu
+Y3JsMB0GA1UdDgQWBBSoSmpjBH3duubRObemRWXv86jsoTANBgkqhkiG9w0BAQsF
+AAOCAQEA3TPXEfNjWDjdGBX7CVW+dla5cEilaUcne8IkCJLxWh9KEik3JHRRHGJo
+uM2VcGfl96S8TihRzZvoroed6ti6WqEBmtzw3Wodatg+VyOeph4EYpr/1wXKtx8/
+wApIvJSwtmVi4MFU5aMqrSDE6ea73Mj2tcMyo5jMd6jmeWUHK8so/joWUoHOUgwu
+X4Po1QYz+3dszkDqMp4fklxBwXRsW10KXzPMTZ+sOPAveyxindmjkW8lGy+QsRlG
+PfZ+G6Z6h7mjem0Y+iWlkYcV4PIWL1iwBi8saCbGS5jN2p8M+X+Q7UNKEkROb3N6
+KOqkqm57TH2H3eDJAkSnh6/DNFu0Qg==
+-----END CERTIFICATE-----
+-----BEGIN CERTIFICATE-----
+MIIDSjCCAjKgAwIBAgIQRK+wgNajJ7qJMDmGLvhAazANBgkqhkiG9w0BAQUFADA/
+MSQwIgYDVQQKExtEaWdpdGFsIFNpZ25hdHVyZSBUcnVzdCBDby4xFzAVBgNVBAMT
+DkRTVCBSb290IENBIFgzMB4XDTAwMDkzMDIxMTIxOVoXDTIxMDkzMDE0MDExNVow
+PzEkMCIGA1UEChMbRGlnaXRhbCBTaWduYXR1cmUgVHJ1c3QgQ28uMRcwFQYDVQQD
+Ew5EU1QgUm9vdCBDQSBYMzCCASIwDQYJKoZIhvcNAQEBBQADggEPADCCAQoCggEB
+AN+v6ZdQCINXtMxiZfaQguzH0yxrMMpb7NnDfcdAwRgUi+DoM3ZJKuM/IUmTrE4O
+rz5Iy2Xu/NMhD2XSKtkyj4zl93ewEnu1lcCJo6m67XMuegwGMoOifooUMM0RoOEq
+OLl5CjH9UL2AZd+3UWODyOKIYepLYYHsUmu5ouJLGiifSKOeDNoJjj4XLh7dIN9b
+xiqKqy69cK3FCxolkHRyxXtqqzTWMIn/5WgTe1QLyNau7Fqckh49ZLOMxt+/yUFw
+7BZy1SbsOFU5Q9D8/RhcQPGX69Wam40dutolucbY38EVAjqr2m7xPi71XAicPNaD
+aeQQmxkqtilX4+U9m5/wAl0CAwEAAaNCMEAwDwYDVR0TAQH/BAUwAwEB/zAOBgNV
+HQ8BAf8EBAMCAQYwHQYDVR0OBBYEFMSnsaR7LHH62+FLkHX/xBVghYkQMA0GCSqG
+SIb3DQEBBQUAA4IBAQCjGiybFwBcqR7uKGY3Or+Dxz9LwwmglSBd49lZRNI+DT69
+ikugdB/OEIKcdBodfpga3csTS7MgROSR6cz8faXbauX+5v3gTt23ADq1cEmv8uXr
+AvHRAosZy5Q6XkjEGB5YGV8eAlrwDPGxrancWYaLbumR9YbK+rlmM6pZW87ipxZz
+R8srzJmwN0jP41ZL9c8PDHIyh8bwRLtTcm1D9SZImlJnt1ir/md2cXjbDaJWFBM5
+JDGFoqgCWjBH4d1QB7wCCZAA62RjYJsWvIjJEubSfZGL+T0yjWW06XyxV3bqxbYo
+Ob8VZRzI9neWagqNdwvYkQsEjgfbKbYK7p2CNTUQ
+-----END CERTIFICATE-----'
+    end
+
     trait :with_expired_certificate do
       certificate '-----BEGIN CERTIFICATE-----
 MIIBsDCCARmgAwIBAgIBATANBgkqhkiG9w0BAQUFADAeMRwwGgYDVQQDExNleHBp
diff --git a/spec/models/pages_domain_spec.rb b/spec/models/pages_domain_spec.rb
index 973c67937b7..519c519fbcf 100644
--- a/spec/models/pages_domain_spec.rb
+++ b/spec/models/pages_domain_spec.rb
@@ -127,6 +127,30 @@ describe PagesDomain do
 
       it { is_expected.not_to be_valid }
     end
+
+    context 'when certificate is expired' do
+      let(:domain) do
+        build(:pages_domain, :with_trusted_expired_chain)
+      end
+
+      context 'when certificate is being changed' do
+        it "adds error to certificate" do
+          domain.valid?
+
+          expect(domain.errors.keys).to contain_exactly(:key, :certificate)
+        end
+      end
+
+      context 'when certificate is already saved' do
+        it "doesn't add error to certificate" do
+          domain.save(validate: false)
+
+          domain.valid?
+
+          expect(domain.errors.keys).to contain_exactly(:key)
+        end
+      end
+    end
   end
 
   describe 'validations' do
-- 
cgit v1.2.1


From 7455ace0097f422f45fde5edccaf9ee739dcb735 Mon Sep 17 00:00:00 2001
From: Robert Speicher <rspeicher@gmail.com>
Date: Tue, 23 Jul 2019 15:00:11 +0000
Subject: Merge branch 'bvl-mark-remote-mirrors-as-failed-sooner' into 'master'

Mark mirrors as failed 1 hour after they started

See merge request gitlab-org/gitlab-ce!30999

(cherry picked from commit d892e80bf0161b535389c91ccb53539e4f08d790)

ab11eee1 Mark mirrors as failed 1 hour after they started
---
 app/models/remote_mirror.rb                               |  4 ++--
 .../bvl-mark-remote-mirrors-as-failed-sooner.yml          |  5 +++++
 spec/models/remote_mirror_spec.rb                         | 15 ++++++++++++---
 3 files changed, 19 insertions(+), 5 deletions(-)
 create mode 100644 changelogs/unreleased/bvl-mark-remote-mirrors-as-failed-sooner.yml

diff --git a/app/models/remote_mirror.rb b/app/models/remote_mirror.rb
index af705b29f7a..6b5605f9999 100644
--- a/app/models/remote_mirror.rb
+++ b/app/models/remote_mirror.rb
@@ -31,7 +31,7 @@ class RemoteMirror < ApplicationRecord
 
   scope :enabled, -> { where(enabled: true) }
   scope :started, -> { with_update_status(:started) }
-  scope :stuck,   -> { started.where('last_update_at < ? OR (last_update_at IS NULL AND updated_at < ?)', 1.day.ago, 1.day.ago) }
+  scope :stuck,   -> { started.where('last_update_at < ? OR (last_update_at IS NULL AND updated_at < ?)', 1.hour.ago, 3.hours.ago) }
 
   state_machine :update_status, initial: :none do
     event :update_start do
@@ -173,7 +173,7 @@ class RemoteMirror < ApplicationRecord
 
     result = URI.parse(url)
     result.password = '*****' if result.password
-    result.user = '*****' if result.user && result.user != "git" # tokens or other data may be saved as user
+    result.user = '*****' if result.user && result.user != 'git' # tokens or other data may be saved as user
     result.to_s
   end
 
diff --git a/changelogs/unreleased/bvl-mark-remote-mirrors-as-failed-sooner.yml b/changelogs/unreleased/bvl-mark-remote-mirrors-as-failed-sooner.yml
new file mode 100644
index 00000000000..1db0a4952b2
--- /dev/null
+++ b/changelogs/unreleased/bvl-mark-remote-mirrors-as-failed-sooner.yml
@@ -0,0 +1,5 @@
+---
+title: Mark push mirrors as failed after 1 hour
+merge_request: 30999
+author:
+type: changed
diff --git a/spec/models/remote_mirror_spec.rb b/spec/models/remote_mirror_spec.rb
index e14b19db915..687b0935c55 100644
--- a/spec/models/remote_mirror_spec.rb
+++ b/spec/models/remote_mirror_spec.rb
@@ -113,7 +113,7 @@ describe RemoteMirror, :mailer do
 
         remote_mirror = create(:remote_mirror)
 
-        expect(remote_mirror.remote_name).to eq("remote_mirror_secret")
+        expect(remote_mirror.remote_name).to eq('remote_mirror_secret')
       end
     end
 
@@ -201,11 +201,20 @@ describe RemoteMirror, :mailer do
   end
 
   context 'stuck mirrors' do
-    it 'includes mirrors stuck in started with no last_update_at set' do
+    it 'includes mirrors that were started over an hour ago' do
+      mirror = create_mirror(url: 'http://cantbeblank',
+                             update_status: 'started',
+                             last_update_at: 3.hours.ago,
+                             updated_at: 2.hours.ago)
+
+      expect(described_class.stuck.last).to eq(mirror)
+    end
+
+    it 'includes mirrors started over 3 hours ago for their first sync' do
       mirror = create_mirror(url: 'http://cantbeblank',
                              update_status: 'started',
                              last_update_at: nil,
-                             updated_at: 25.hours.ago)
+                             updated_at: 4.hours.ago)
 
       expect(described_class.stuck.last).to eq(mirror)
     end
-- 
cgit v1.2.1


From 4de0f5a0b9ad69a5a23ee963b49538259568eaf7 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Kamil=20Trzci=C5=84ski?= <ayufan@ayufan.eu>
Date: Tue, 23 Jul 2019 14:15:09 +0000
Subject: Merge branch 'sh-use-shared-state-cluster-pubsub' into 'master'

Use persistent Redis cluster for Workhorse pub/sub notifications

See merge request gitlab-org/gitlab-ce!30990

(cherry picked from commit 995e5b5210c1e565a7f175f67a0aca4efb1f27e9)

583c12ac Use persistent Redis cluster for Workhorse pub/sub notifications
---
 app/models/ci/runner.rb                                      | 2 +-
 changelogs/unreleased/sh-use-shared-state-cluster-pubsub.yml | 5 +++++
 lib/gitlab/workhorse.rb                                      | 2 +-
 spec/lib/gitlab/workhorse_spec.rb                            | 1 +
 spec/models/ci/runner_spec.rb                                | 4 ++--
 5 files changed, 10 insertions(+), 4 deletions(-)
 create mode 100644 changelogs/unreleased/sh-use-shared-state-cluster-pubsub.yml

diff --git a/app/models/ci/runner.rb b/app/models/ci/runner.rb
index 07d00503861..43ff874ac23 100644
--- a/app/models/ci/runner.rb
+++ b/app/models/ci/runner.rb
@@ -264,7 +264,7 @@ module Ci
     private
 
     def cleanup_runner_queue
-      Gitlab::Redis::Queues.with do |redis|
+      Gitlab::Redis::SharedState.with do |redis|
         redis.del(runner_queue_key)
       end
     end
diff --git a/changelogs/unreleased/sh-use-shared-state-cluster-pubsub.yml b/changelogs/unreleased/sh-use-shared-state-cluster-pubsub.yml
new file mode 100644
index 00000000000..5e72f23d7ad
--- /dev/null
+++ b/changelogs/unreleased/sh-use-shared-state-cluster-pubsub.yml
@@ -0,0 +1,5 @@
+---
+title: Use persistent Redis cluster for Workhorse pub/sub notifications
+merge_request: 30990
+author:
+type: fixed
diff --git a/lib/gitlab/workhorse.rb b/lib/gitlab/workhorse.rb
index 46a7b5b982a..3b77fe838ae 100644
--- a/lib/gitlab/workhorse.rb
+++ b/lib/gitlab/workhorse.rb
@@ -221,7 +221,7 @@ module Gitlab
       end
 
       def set_key_and_notify(key, value, expire: nil, overwrite: true)
-        Gitlab::Redis::Queues.with do |redis|
+        Gitlab::Redis::SharedState.with do |redis|
           result = redis.set(key, value, ex: expire, nx: !overwrite)
           if result
             redis.publish(NOTIFICATION_CHANNEL, "#{key}=#{value}")
diff --git a/spec/lib/gitlab/workhorse_spec.rb b/spec/lib/gitlab/workhorse_spec.rb
index f8332757fcd..451e18ed91b 100644
--- a/spec/lib/gitlab/workhorse_spec.rb
+++ b/spec/lib/gitlab/workhorse_spec.rb
@@ -404,6 +404,7 @@ describe Gitlab::Workhorse do
       end
 
       it 'set and notify' do
+        expect(Gitlab::Redis::SharedState).to receive(:with).and_call_original
         expect_any_instance_of(::Redis).to receive(:publish)
           .with(described_class::NOTIFICATION_CHANNEL, "test-key=test-value")
 
diff --git a/spec/models/ci/runner_spec.rb b/spec/models/ci/runner_spec.rb
index f735a89f69f..24ea059e871 100644
--- a/spec/models/ci/runner_spec.rb
+++ b/spec/models/ci/runner_spec.rb
@@ -554,7 +554,7 @@ describe Ci::Runner do
     end
 
     def expect_value_in_queues
-      Gitlab::Redis::Queues.with do |redis|
+      Gitlab::Redis::SharedState.with do |redis|
         runner_queue_key = runner.send(:runner_queue_key)
         expect(redis.get(runner_queue_key))
       end
@@ -627,7 +627,7 @@ describe Ci::Runner do
       end
 
       it 'cleans up the queue' do
-        Gitlab::Redis::Queues.with do |redis|
+        Gitlab::Redis::SharedState.with do |redis|
           expect(redis.get(queue_key)).to be_nil
         end
       end
-- 
cgit v1.2.1


From 89bc7f84120a6a15108979f07d34c14a2e70f782 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Kamil=20Trzci=C5=84ski?= <ayufan@ayufan.eu>
Date: Tue, 23 Jul 2019 14:17:05 +0000
Subject: Merge branch 'safe-archiving-for-traces' into 'master'

Extra logging for new live trace architecture

See merge request gitlab-org/gitlab-ce!30892

(cherry picked from commit 4435cdde49f6005fae2dabd7dade22ff94c8cea6)

c2e0e689 Validate the existence of archived traces before removing live trace
---
 app/models/ci/build.rb                             |  8 ++++
 app/models/ci/job_artifact.rb                      |  4 ++
 app/services/ci/archive_trace_service.rb           | 28 ++++++++++--
 app/workers/archive_trace_worker.rb                |  2 +-
 app/workers/ci/archive_traces_cron_worker.rb       |  2 +-
 .../unreleased/safe-archiving-for-traces.yml       |  5 +++
 lib/gitlab/ci/trace.rb                             | 12 ++++-
 lib/gitlab/ci/trace/chunked_io.rb                  |  7 +++
 spec/lib/gitlab/ci/trace/chunked_io_spec.rb        | 10 +++++
 spec/models/ci/build_spec.rb                       | 28 ++++++++++++
 spec/models/ci/job_artifact_spec.rb                | 25 +++++++++++
 spec/services/ci/archive_trace_service_spec.rb     | 38 +++++++++++++---
 .../shared_examples/ci_trace_shared_examples.rb    | 52 ++++++++++++++++++++++
 spec/workers/archive_trace_worker_spec.rb          |  2 +-
 spec/workers/ci/archive_traces_cron_worker_spec.rb |  7 ++-
 spec/workers/stuck_ci_jobs_worker_spec.rb          |  4 --
 16 files changed, 214 insertions(+), 20 deletions(-)
 create mode 100644 changelogs/unreleased/safe-archiving-for-traces.yml

diff --git a/app/models/ci/build.rb b/app/models/ci/build.rb
index 635fcc86166..da70cb9a9a7 100644
--- a/app/models/ci/build.rb
+++ b/app/models/ci/build.rb
@@ -531,6 +531,14 @@ module Ci
       trace.exist?
     end
 
+    def has_live_trace?
+      trace.live_trace_exist?
+    end
+
+    def has_archived_trace?
+      trace.archived_trace_exist?
+    end
+
     def artifacts_file
       job_artifacts_archive&.file
     end
diff --git a/app/models/ci/job_artifact.rb b/app/models/ci/job_artifact.rb
index f80e98e5bca..e132cb045e2 100644
--- a/app/models/ci/job_artifact.rb
+++ b/app/models/ci/job_artifact.rb
@@ -176,6 +176,10 @@ module Ci
       end
     end
 
+    def self.archived_trace_exists_for?(job_id)
+      where(job_id: job_id).trace.take&.file&.file&.exists?
+    end
+
     private
 
     def file_format_adapter_class
diff --git a/app/services/ci/archive_trace_service.rb b/app/services/ci/archive_trace_service.rb
index b5cfa1d019c..700d78361a4 100644
--- a/app/services/ci/archive_trace_service.rb
+++ b/app/services/ci/archive_trace_service.rb
@@ -2,8 +2,25 @@
 
 module Ci
   class ArchiveTraceService
-    def execute(job)
+    def execute(job, worker_name:)
+      # TODO: Remove this logging once we confirmed new live trace architecture is functional.
+      # See https://gitlab.com/gitlab-com/gl-infra/infrastructure/issues/4667.
+      unless job.has_live_trace?
+        Sidekiq.logger.warn(class: worker_name,
+                            message: 'The job does not have live trace but going to be archived.',
+                            job_id: job.id)
+        return
+      end
+
       job.trace.archive!
+
+      # TODO: Remove this logging once we confirmed new live trace architecture is functional.
+      # See https://gitlab.com/gitlab-com/gl-infra/infrastructure/issues/4667.
+      unless job.has_archived_trace?
+        Sidekiq.logger.warn(class: worker_name,
+                            message: 'The job does not have archived trace after archiving.',
+                            job_id: job.id)
+      end
     rescue ::Gitlab::Ci::Trace::AlreadyArchivedError
       # It's already archived, thus we can safely ignore this exception.
     rescue => e
@@ -11,7 +28,7 @@ module Ci
       # If `archive!` keeps failing for over a week, that could incur data loss.
       # (See more https://docs.gitlab.com/ee/administration/job_traces.html#new-live-trace-architecture)
       # In order to avoid interrupting the system, we do not raise an exception here.
-      archive_error(e, job)
+      archive_error(e, job, worker_name)
     end
 
     private
@@ -22,9 +39,12 @@ module Ci
                                 "Counter of failed attempts of trace archiving")
     end
 
-    def archive_error(error, job)
+    def archive_error(error, job, worker_name)
       failed_archive_counter.increment
-      Rails.logger.error "Failed to archive trace. id: #{job.id} message: #{error.message}" # rubocop:disable Gitlab/RailsLogger
+
+      Sidekiq.logger.warn(class: worker_name,
+        message: "Failed to archive trace. message: #{error.message}.",
+        job_id: job.id)
 
       Gitlab::Sentry
         .track_exception(error,
diff --git a/app/workers/archive_trace_worker.rb b/app/workers/archive_trace_worker.rb
index 4a9becf0ca7..66f9b8d9e80 100644
--- a/app/workers/archive_trace_worker.rb
+++ b/app/workers/archive_trace_worker.rb
@@ -7,7 +7,7 @@ class ArchiveTraceWorker
   # rubocop: disable CodeReuse/ActiveRecord
   def perform(job_id)
     Ci::Build.without_archived_trace.find_by(id: job_id).try do |job|
-      Ci::ArchiveTraceService.new.execute(job)
+      Ci::ArchiveTraceService.new.execute(job, worker_name: self.class.name)
     end
   end
   # rubocop: enable CodeReuse/ActiveRecord
diff --git a/app/workers/ci/archive_traces_cron_worker.rb b/app/workers/ci/archive_traces_cron_worker.rb
index f65ff239866..75e68d0233a 100644
--- a/app/workers/ci/archive_traces_cron_worker.rb
+++ b/app/workers/ci/archive_traces_cron_worker.rb
@@ -11,7 +11,7 @@ module Ci
       # This could happen when ArchiveTraceWorker sidekiq jobs were lost by receiving SIGKILL
       # More details in https://gitlab.com/gitlab-org/gitlab-ce/issues/36791
       Ci::Build.finished.with_live_trace.find_each(batch_size: 100) do |build|
-        Ci::ArchiveTraceService.new.execute(build)
+        Ci::ArchiveTraceService.new.execute(build, worker_name: self.class.name)
       end
     end
     # rubocop: enable CodeReuse/ActiveRecord
diff --git a/changelogs/unreleased/safe-archiving-for-traces.yml b/changelogs/unreleased/safe-archiving-for-traces.yml
new file mode 100644
index 00000000000..2b9070bacfe
--- /dev/null
+++ b/changelogs/unreleased/safe-archiving-for-traces.yml
@@ -0,0 +1,5 @@
+---
+title: Extra logging for new live trace architecture
+merge_request: 30892
+author:
+type: fixed
diff --git a/lib/gitlab/ci/trace.rb b/lib/gitlab/ci/trace.rb
index ce5857965bf..cb617080c76 100644
--- a/lib/gitlab/ci/trace.rb
+++ b/lib/gitlab/ci/trace.rb
@@ -63,7 +63,15 @@ module Gitlab
       end
 
       def exist?
-        trace_artifact&.exists? || job.trace_chunks.any? || current_path.present? || old_trace.present?
+        archived_trace_exist? || live_trace_exist?
+      end
+
+      def archived_trace_exist?
+        trace_artifact&.exists?
+      end
+
+      def live_trace_exist?
+        job.trace_chunks.any? || current_path.present? || old_trace.present?
       end
 
       def read
@@ -167,7 +175,7 @@ module Gitlab
 
       def clone_file!(src_stream, temp_dir)
         FileUtils.mkdir_p(temp_dir)
-        Dir.mktmpdir('tmp-trace', temp_dir) do |dir_path|
+        Dir.mktmpdir("tmp-trace-#{job.id}", temp_dir) do |dir_path|
           temp_path = File.join(dir_path, "job.log")
           FileUtils.touch(temp_path)
           size = IO.copy_stream(src_stream, temp_path)
diff --git a/lib/gitlab/ci/trace/chunked_io.rb b/lib/gitlab/ci/trace/chunked_io.rb
index 8c6fd56493f..e99889f4a25 100644
--- a/lib/gitlab/ci/trace/chunked_io.rb
+++ b/lib/gitlab/ci/trace/chunked_io.rb
@@ -166,6 +166,13 @@ module Gitlab
         end
 
         def destroy!
+          # TODO: Remove this logging once we confirmed new live trace architecture is functional.
+          # See https://gitlab.com/gitlab-com/gl-infra/infrastructure/issues/4667.
+          unless build.has_archived_trace?
+            Sidekiq.logger.warn(message: 'The job does not have archived trace but going to be destroyed.',
+                                job_id: build.id)
+          end
+
           trace_chunks.fast_destroy_all
           @tell = @size = 0
         ensure
diff --git a/spec/lib/gitlab/ci/trace/chunked_io_spec.rb b/spec/lib/gitlab/ci/trace/chunked_io_spec.rb
index 546a9e7d0cc..3e5cd81f929 100644
--- a/spec/lib/gitlab/ci/trace/chunked_io_spec.rb
+++ b/spec/lib/gitlab/ci/trace/chunked_io_spec.rb
@@ -442,5 +442,15 @@ describe Gitlab::Ci::Trace::ChunkedIO, :clean_gitlab_redis_cache do
 
       expect(Ci::BuildTraceChunk.where(build: build).count).to eq(0)
     end
+
+    context 'when the job does not have archived trace' do
+      it 'leaves a message in sidekiq log' do
+        expect(Sidekiq.logger).to receive(:warn).with(
+          message: 'The job does not have archived trace but going to be destroyed.',
+          job_id: build.id).and_call_original
+
+        subject
+      end
+    end
   end
 end
diff --git a/spec/models/ci/build_spec.rb b/spec/models/ci/build_spec.rb
index 78862de0657..c30cb70e1c1 100644
--- a/spec/models/ci/build_spec.rb
+++ b/spec/models/ci/build_spec.rb
@@ -692,6 +692,34 @@ describe Ci::Build do
     end
   end
 
+  describe '#has_live_trace?' do
+    subject { build.has_live_trace? }
+
+    let(:build) { create(:ci_build, :trace_live) }
+
+    it { is_expected.to be_truthy }
+
+    context 'when build does not have live trace' do
+      let(:build) { create(:ci_build) }
+
+      it { is_expected.to be_falsy }
+    end
+  end
+
+  describe '#has_archived_trace?' do
+    subject { build.has_archived_trace? }
+
+    let(:build) { create(:ci_build, :trace_artifact) }
+
+    it { is_expected.to be_truthy }
+
+    context 'when build does not have archived trace' do
+      let(:build) { create(:ci_build) }
+
+      it { is_expected.to be_falsy }
+    end
+  end
+
   describe '#has_job_artifacts?' do
     subject { build.has_job_artifacts? }
 
diff --git a/spec/models/ci/job_artifact_spec.rb b/spec/models/ci/job_artifact_spec.rb
index 1ba66565e03..1413da231e0 100644
--- a/spec/models/ci/job_artifact_spec.rb
+++ b/spec/models/ci/job_artifact_spec.rb
@@ -70,6 +70,31 @@ describe Ci::JobArtifact do
     end
   end
 
+  describe '.archived_trace_exists_for?' do
+    subject { described_class.archived_trace_exists_for?(job_id) }
+
+    let!(:artifact) { create(:ci_job_artifact, :trace, job: job) }
+    let(:job) { create(:ci_build) }
+
+    context 'when the specified job_id exists' do
+      let(:job_id) { job.id }
+
+      it { is_expected.to be_truthy }
+
+      context 'when the job does have archived trace' do
+        let!(:artifact) { }
+
+        it { is_expected.to be_falsy }
+      end
+    end
+
+    context 'when the specified job_id does not exist' do
+      let(:job_id) { 10000 }
+
+      it { is_expected.to be_falsy }
+    end
+  end
+
   describe 'callbacks' do
     subject { create(:ci_job_artifact, :archive) }
 
diff --git a/spec/services/ci/archive_trace_service_spec.rb b/spec/services/ci/archive_trace_service_spec.rb
index 44a77c29086..454db3d5a48 100644
--- a/spec/services/ci/archive_trace_service_spec.rb
+++ b/spec/services/ci/archive_trace_service_spec.rb
@@ -3,7 +3,7 @@
 require 'spec_helper'
 
 describe Ci::ArchiveTraceService, '#execute' do
-  subject { described_class.new.execute(job) }
+  subject { described_class.new.execute(job, worker_name: ArchiveTraceWorker.name) }
 
   context 'when job is finished' do
     let(:job) { create(:ci_build, :success, :trace_live) }
@@ -25,6 +25,34 @@ describe Ci::ArchiveTraceService, '#execute' do
         expect { subject }.not_to change { Ci::JobArtifact.trace.count }
       end
     end
+
+    context 'when job does not have trace' do
+      let(:job) { create(:ci_build, :success) }
+
+      it 'leaves a warning message in sidekiq log' do
+        expect(Sidekiq.logger).to receive(:warn).with(
+          class: ArchiveTraceWorker.name,
+          message: 'The job does not have live trace but going to be archived.',
+          job_id: job.id)
+
+        subject
+      end
+    end
+
+    context 'when job failed to archive trace but did not raise an exception' do
+      before do
+        allow_any_instance_of(Gitlab::Ci::Trace).to receive(:archive!) {}
+      end
+
+      it 'leaves a warning message in sidekiq log' do
+        expect(Sidekiq.logger).to receive(:warn).with(
+          class: ArchiveTraceWorker.name,
+          message: 'The job does not have archived trace after archiving.',
+          job_id: job.id)
+
+        subject
+      end
+    end
   end
 
   context 'when job is running' do
@@ -37,10 +65,10 @@ describe Ci::ArchiveTraceService, '#execute' do
               issue_url: 'https://gitlab.com/gitlab-org/gitlab-ce/issues/51502',
               extra: { job_id: job.id } ).once
 
-      expect(Rails.logger)
-        .to receive(:error)
-        .with("Failed to archive trace. id: #{job.id} message: Job is not finished yet")
-        .and_call_original
+      expect(Sidekiq.logger).to receive(:warn).with(
+        class: ArchiveTraceWorker.name,
+        message: "Failed to archive trace. message: Job is not finished yet.",
+        job_id: job.id).and_call_original
 
       expect(Gitlab::Metrics)
         .to receive(:counter)
diff --git a/spec/support/shared_examples/ci_trace_shared_examples.rb b/spec/support/shared_examples/ci_trace_shared_examples.rb
index ab0550e2613..68c2b6e10e2 100644
--- a/spec/support/shared_examples/ci_trace_shared_examples.rb
+++ b/spec/support/shared_examples/ci_trace_shared_examples.rb
@@ -720,6 +720,58 @@ shared_examples_for 'trace with enabled live trace feature' do
     end
   end
 
+  describe '#archived_trace_exist?' do
+    subject { trace.archived_trace_exist? }
+
+    context 'when trace does not exist' do
+      it { is_expected.to be_falsy }
+    end
+
+    context 'when archived trace exists' do
+      before do
+        create(:ci_job_artifact, :trace, job: build)
+      end
+
+      it { is_expected.to be_truthy }
+    end
+
+    context 'when live trace exists' do
+      before do
+        Gitlab::Ci::Trace::ChunkedIO.new(build) do |stream|
+          stream.write('abc')
+        end
+      end
+
+      it { is_expected.to be_falsy }
+    end
+  end
+
+  describe '#live_trace_exist?' do
+    subject { trace.live_trace_exist? }
+
+    context 'when trace does not exist' do
+      it { is_expected.to be_falsy }
+    end
+
+    context 'when archived trace exists' do
+      before do
+        create(:ci_job_artifact, :trace, job: build)
+      end
+
+      it { is_expected.to be_falsy }
+    end
+
+    context 'when live trace exists' do
+      before do
+        Gitlab::Ci::Trace::ChunkedIO.new(build) do |stream|
+          stream.write('abc')
+        end
+      end
+
+      it { is_expected.to be_truthy }
+    end
+  end
+
   describe '#archive!' do
     subject { trace.archive! }
 
diff --git a/spec/workers/archive_trace_worker_spec.rb b/spec/workers/archive_trace_worker_spec.rb
index 368ed3f3db1..44f7be15201 100644
--- a/spec/workers/archive_trace_worker_spec.rb
+++ b/spec/workers/archive_trace_worker_spec.rb
@@ -11,7 +11,7 @@ describe ArchiveTraceWorker do
 
       it 'executes service' do
         expect_any_instance_of(Ci::ArchiveTraceService)
-          .to receive(:execute).with(job)
+          .to receive(:execute).with(job, anything)
 
         subject
       end
diff --git a/spec/workers/ci/archive_traces_cron_worker_spec.rb b/spec/workers/ci/archive_traces_cron_worker_spec.rb
index eca6cf5235f..28381fdc3be 100644
--- a/spec/workers/ci/archive_traces_cron_worker_spec.rb
+++ b/spec/workers/ci/archive_traces_cron_worker_spec.rb
@@ -34,7 +34,7 @@ describe Ci::ArchiveTracesCronWorker do
 
     it 'executes service' do
       expect_any_instance_of(Ci::ArchiveTraceService)
-        .to receive(:execute).with(build)
+        .to receive(:execute).with(build, anything)
 
       subject
     end
@@ -60,7 +60,10 @@ describe Ci::ArchiveTracesCronWorker do
       end
 
       it 'puts a log' do
-        expect(Rails.logger).to receive(:error).with("Failed to archive trace. id: #{build.id} message: Unexpected error")
+        expect(Sidekiq.logger).to receive(:warn).with(
+          class: described_class.name,
+          message: "Failed to archive trace. message: Unexpected error.",
+          job_id: build.id)
 
         subject
       end
diff --git a/spec/workers/stuck_ci_jobs_worker_spec.rb b/spec/workers/stuck_ci_jobs_worker_spec.rb
index 72de62f1188..c3d577e2dae 100644
--- a/spec/workers/stuck_ci_jobs_worker_spec.rb
+++ b/spec/workers/stuck_ci_jobs_worker_spec.rb
@@ -7,8 +7,6 @@ describe StuckCiJobsWorker do
 
   let!(:runner) { create :ci_runner }
   let!(:job) { create :ci_build, runner: runner }
-  let(:trace_lease_key) { "trace:write:lock:#{job.id}" }
-  let(:trace_lease_uuid) { SecureRandom.uuid }
   let(:worker_lease_key) { StuckCiJobsWorker::EXCLUSIVE_LEASE_KEY }
   let(:worker_lease_uuid) { SecureRandom.uuid }
 
@@ -16,7 +14,6 @@ describe StuckCiJobsWorker do
 
   before do
     stub_exclusive_lease(worker_lease_key, worker_lease_uuid)
-    stub_exclusive_lease(trace_lease_key, trace_lease_uuid)
     job.update!(status: status, updated_at: updated_at)
   end
 
@@ -195,7 +192,6 @@ describe StuckCiJobsWorker do
     end
 
     it 'cancels exclusive leases after worker perform' do
-      expect_to_cancel_exclusive_lease(trace_lease_key, trace_lease_uuid)
       expect_to_cancel_exclusive_lease(worker_lease_key, worker_lease_uuid)
 
       worker.perform
-- 
cgit v1.2.1


From 7b2d42d3cd5cd42962c2eb0eddc1b4289f4cddd7 Mon Sep 17 00:00:00 2001
From: Thong Kuah <tkuah@gitlab.com>
Date: Wed, 24 Jul 2019 02:27:16 +0000
Subject: Merge branch '65019-auto-devops-dind-tls-fix' into 'master'

Set DOCKER_TLS_CERTDIR in Auto Dev-Ops CI template

See merge request gitlab-org/gitlab-ce!31078

(cherry picked from commit a842c3882c4b47cc8b384fbd84be5643dacc7269)

af88ccfa Set DOCKER_TLS_CERTDIR in Auto Dev-Ops CI template
---
 changelogs/unreleased/65019-auto-devops-dind-tls-fix.yml | 5 +++++
 lib/gitlab/ci/templates/Auto-DevOps.gitlab-ci.yml        | 2 ++
 2 files changed, 7 insertions(+)
 create mode 100644 changelogs/unreleased/65019-auto-devops-dind-tls-fix.yml

diff --git a/changelogs/unreleased/65019-auto-devops-dind-tls-fix.yml b/changelogs/unreleased/65019-auto-devops-dind-tls-fix.yml
new file mode 100644
index 00000000000..3eea3e551ce
--- /dev/null
+++ b/changelogs/unreleased/65019-auto-devops-dind-tls-fix.yml
@@ -0,0 +1,5 @@
+---
+title: Set DOCKER_TLS_CERTDIR in Auto Dev-Ops CI template to fix jobs using Docker-in-Docker
+merge_request: 31078
+author:
+type: fixed
diff --git a/lib/gitlab/ci/templates/Auto-DevOps.gitlab-ci.yml b/lib/gitlab/ci/templates/Auto-DevOps.gitlab-ci.yml
index cf3d261c1cb..7b9a169a91f 100644
--- a/lib/gitlab/ci/templates/Auto-DevOps.gitlab-ci.yml
+++ b/lib/gitlab/ci/templates/Auto-DevOps.gitlab-ci.yml
@@ -57,6 +57,8 @@ variables:
 
   ROLLOUT_RESOURCE_TYPE: deployment
 
+  DOCKER_TLS_CERTDIR: "" # https://gitlab.com/gitlab-org/gitlab-runner/issues/4501
+
 stages:
   - build
   - test
-- 
cgit v1.2.1


From 7443bff04a42a1e12b101e1097800ef4e52f5738 Mon Sep 17 00:00:00 2001
From: Thong Kuah <tkuah@gitlab.com>
Date: Wed, 24 Jul 2019 02:29:06 +0000
Subject: Merge branch '65019-job-templates-dind-tls-fix' into 'master'

Set DOCKER_TLS_CERTDIR in CI job templates

See merge request gitlab-org/gitlab-ce!31080

(cherry picked from commit 8768e295c3474cb3fcee9afe3b7e9a2ac1431b50)

c472bec5 Set DOCKER_TLS_CERTDIR in CI job templates
---
 changelogs/unreleased/65019-job-templates-dind-tls-fix.yml           | 5 +++++
 .../ci/templates/Jobs/Browser-Performance-Testing.gitlab-ci.yml      | 2 ++
 lib/gitlab/ci/templates/Jobs/Build.gitlab-ci.yml                     | 2 ++
 lib/gitlab/ci/templates/Jobs/Code-Quality.gitlab-ci.yml              | 1 +
 lib/gitlab/ci/templates/Security/Dependency-Scanning.gitlab-ci.yml   | 1 +
 lib/gitlab/ci/templates/Security/SAST.gitlab-ci.yml                  | 1 +
 6 files changed, 12 insertions(+)
 create mode 100644 changelogs/unreleased/65019-job-templates-dind-tls-fix.yml

diff --git a/changelogs/unreleased/65019-job-templates-dind-tls-fix.yml b/changelogs/unreleased/65019-job-templates-dind-tls-fix.yml
new file mode 100644
index 00000000000..c7c02486d61
--- /dev/null
+++ b/changelogs/unreleased/65019-job-templates-dind-tls-fix.yml
@@ -0,0 +1,5 @@
+---
+title: Set DOCKER_TLS_CERTDIR in CI job templates to fix Docker-in-Docker service
+merge_request: 31080
+author:
+type: fixed
diff --git a/lib/gitlab/ci/templates/Jobs/Browser-Performance-Testing.gitlab-ci.yml b/lib/gitlab/ci/templates/Jobs/Browser-Performance-Testing.gitlab-ci.yml
index a09217e8cf0..b0a79950667 100644
--- a/lib/gitlab/ci/templates/Jobs/Browser-Performance-Testing.gitlab-ci.yml
+++ b/lib/gitlab/ci/templates/Jobs/Browser-Performance-Testing.gitlab-ci.yml
@@ -2,6 +2,8 @@ performance:
   stage: performance
   image: docker:stable
   allow_failure: true
+  variables:
+    DOCKER_TLS_CERTDIR: ""
   services:
     - docker:stable-dind
   script:
diff --git a/lib/gitlab/ci/templates/Jobs/Build.gitlab-ci.yml b/lib/gitlab/ci/templates/Jobs/Build.gitlab-ci.yml
index 18f7290e1d9..8061da968ed 100644
--- a/lib/gitlab/ci/templates/Jobs/Build.gitlab-ci.yml
+++ b/lib/gitlab/ci/templates/Jobs/Build.gitlab-ci.yml
@@ -1,6 +1,8 @@
 build:
   stage: build
   image: "registry.gitlab.com/gitlab-org/cluster-integration/auto-build-image/master:stable"
+  variables:
+    DOCKER_TLS_CERTDIR: ""
   services:
     - docker:stable-dind
   script:
diff --git a/lib/gitlab/ci/templates/Jobs/Code-Quality.gitlab-ci.yml b/lib/gitlab/ci/templates/Jobs/Code-Quality.gitlab-ci.yml
index 005ea4b7a46..3adc6a72874 100644
--- a/lib/gitlab/ci/templates/Jobs/Code-Quality.gitlab-ci.yml
+++ b/lib/gitlab/ci/templates/Jobs/Code-Quality.gitlab-ci.yml
@@ -6,6 +6,7 @@ code_quality:
     - docker:stable-dind
   variables:
     DOCKER_DRIVER: overlay2
+    DOCKER_TLS_CERTDIR: ""
   script:
     - |
       if ! docker info &>/dev/null; then
diff --git a/lib/gitlab/ci/templates/Security/Dependency-Scanning.gitlab-ci.yml b/lib/gitlab/ci/templates/Security/Dependency-Scanning.gitlab-ci.yml
index 89eccce69f6..49f70cacab7 100644
--- a/lib/gitlab/ci/templates/Security/Dependency-Scanning.gitlab-ci.yml
+++ b/lib/gitlab/ci/templates/Security/Dependency-Scanning.gitlab-ci.yml
@@ -9,6 +9,7 @@ dependency_scanning:
   image: docker:stable
   variables:
     DOCKER_DRIVER: overlay2
+    DOCKER_TLS_CERTDIR: ""
   allow_failure: true
   services:
     - docker:stable-dind
diff --git a/lib/gitlab/ci/templates/Security/SAST.gitlab-ci.yml b/lib/gitlab/ci/templates/Security/SAST.gitlab-ci.yml
index 0a97a16b83c..4190de73e1f 100644
--- a/lib/gitlab/ci/templates/Security/SAST.gitlab-ci.yml
+++ b/lib/gitlab/ci/templates/Security/SAST.gitlab-ci.yml
@@ -9,6 +9,7 @@ sast:
   image: docker:stable
   variables:
     DOCKER_DRIVER: overlay2
+    DOCKER_TLS_CERTDIR: ""
   allow_failure: true
   services:
     - docker:stable-dind
-- 
cgit v1.2.1


From 1762341a0f10b8267c539f60249b9fe95dfed20e Mon Sep 17 00:00:00 2001
From: Nick Thomas <nick@gitlab.com>
Date: Tue, 23 Jul 2019 14:00:06 +0000
Subject: Merge branch 'bw-add-index-for-relative-position' into 'master'

Add index for issues on relative position, project, and state for manual sorting

See merge request gitlab-org/gitlab-ce!30542

(cherry picked from commit 31335ea661ea27d7bed3990346f63aabcc2140a5)

b0eafaac Add index for issues on relative position, project
---
 app/models/issue.rb                                  |  2 +-
 .../bw-add-index-for-relative-position.yml           |  5 +++++
 ...09220143_add_index_to_issues_relative_position.rb | 20 ++++++++++++++++++++
 db/schema.rb                                         |  1 +
 4 files changed, 27 insertions(+), 1 deletion(-)
 create mode 100644 changelogs/unreleased/bw-add-index-for-relative-position.yml
 create mode 100644 db/migrate/20190709220143_add_index_to_issues_relative_position.rb

diff --git a/app/models/issue.rb b/app/models/issue.rb
index 12d30389910..8c5dd5e382e 100644
--- a/app/models/issue.rb
+++ b/app/models/issue.rb
@@ -131,7 +131,7 @@ class Issue < ApplicationRecord
     when 'due_date'            then order_due_date_asc
     when 'due_date_asc'        then order_due_date_asc
     when 'due_date_desc'       then order_due_date_desc
-    when 'relative_position'   then order_relative_position_asc
+    when 'relative_position'   then order_relative_position_asc.with_order_id_desc
     else
       super
     end
diff --git a/changelogs/unreleased/bw-add-index-for-relative-position.yml b/changelogs/unreleased/bw-add-index-for-relative-position.yml
new file mode 100644
index 00000000000..80ca20992e6
--- /dev/null
+++ b/changelogs/unreleased/bw-add-index-for-relative-position.yml
@@ -0,0 +1,5 @@
+---
+title: Add index for issues on relative position, project, and state for manual sorting
+merge_request: 30542
+author:
+type: fixed
diff --git a/db/migrate/20190709220143_add_index_to_issues_relative_position.rb b/db/migrate/20190709220143_add_index_to_issues_relative_position.rb
new file mode 100644
index 00000000000..effab33ce4f
--- /dev/null
+++ b/db/migrate/20190709220143_add_index_to_issues_relative_position.rb
@@ -0,0 +1,20 @@
+# frozen_string_literal: true
+
+class AddIndexToIssuesRelativePosition < ActiveRecord::Migration[5.1]
+  include Gitlab::Database::MigrationHelpers
+
+  # Set this constant to true if this migration requires downtime.
+  DOWNTIME = false
+
+  disable_ddl_transaction!
+
+  INDEX_NAME = 'index_issues_on_project_id_and_state_and_rel_position_and_id'.freeze
+
+  def up
+    add_concurrent_index :issues, [:project_id, :state, :relative_position, :id], order: { id: :desc }, name: INDEX_NAME
+  end
+
+  def down
+    remove_concurrent_index_by_name :issues, INDEX_NAME
+  end
+end
diff --git a/db/schema.rb b/db/schema.rb
index a5079d3a5bc..f82987c2c6e 100644
--- a/db/schema.rb
+++ b/db/schema.rb
@@ -1682,6 +1682,7 @@ ActiveRecord::Schema.define(version: 2019_07_15_114644) do
     t.index ["project_id", "created_at", "id", "state"], name: "index_issues_on_project_id_and_created_at_and_id_and_state", using: :btree
     t.index ["project_id", "due_date", "id", "state"], name: "idx_issues_on_project_id_and_due_date_and_id_and_state_partial", where: "(due_date IS NOT NULL)", using: :btree
     t.index ["project_id", "iid"], name: "index_issues_on_project_id_and_iid", unique: true, using: :btree
+    t.index ["project_id", "state", "relative_position", "id"], name: "index_issues_on_project_id_and_state_and_rel_position_and_id", order: { id: :desc }, using: :btree
     t.index ["project_id", "updated_at", "id", "state"], name: "index_issues_on_project_id_and_updated_at_and_id_and_state", using: :btree
     t.index ["relative_position"], name: "index_issues_on_relative_position", using: :btree
     t.index ["state"], name: "index_issues_on_state", using: :btree
-- 
cgit v1.2.1


From a980b0dbbd6597bc73a9219f215a110ddd7a1ae2 Mon Sep 17 00:00:00 2001
From: Sean McGivern <sean@gitlab.com>
Date: Wed, 24 Jul 2019 10:41:47 +0000
Subject: Merge branch 'bw-enable-default-manual-sorting' into 'master'

Default manual_sorting feature flag to on

See merge request gitlab-org/gitlab-ce!30788

(cherry picked from commit 98152f17eaf6b9346566b64154bc2a348a1257cd)

81f10288 Default manual_sorting feature flag to on
---
 app/controllers/groups_controller.rb               | 2 +-
 app/controllers/projects/issues_controller.rb      | 2 +-
 app/views/shared/issuable/_sort_dropdown.html.haml | 2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/app/controllers/groups_controller.rb b/app/controllers/groups_controller.rb
index dbddee47997..c2517ddc10a 100644
--- a/app/controllers/groups_controller.rb
+++ b/app/controllers/groups_controller.rb
@@ -8,7 +8,7 @@ class GroupsController < Groups::ApplicationController
   include RecordUserLastActivity
 
   before_action do
-    push_frontend_feature_flag(:manual_sorting)
+    push_frontend_feature_flag(:manual_sorting, default_enabled: true)
   end
 
   respond_to :html
diff --git a/app/controllers/projects/issues_controller.rb b/app/controllers/projects/issues_controller.rb
index 228de8bc6f3..db7ca7ef0d7 100644
--- a/app/controllers/projects/issues_controller.rb
+++ b/app/controllers/projects/issues_controller.rb
@@ -11,7 +11,7 @@ class Projects::IssuesController < Projects::ApplicationController
   include RecordUserLastActivity
 
   before_action do
-    push_frontend_feature_flag(:manual_sorting)
+    push_frontend_feature_flag(:manual_sorting, default_enabled: true)
   end
 
   def issue_except_actions
diff --git a/app/views/shared/issuable/_sort_dropdown.html.haml b/app/views/shared/issuable/_sort_dropdown.html.haml
index 403e001bfe8..df0523595f5 100644
--- a/app/views/shared/issuable/_sort_dropdown.html.haml
+++ b/app/views/shared/issuable/_sort_dropdown.html.haml
@@ -1,7 +1,7 @@
 - sort_value = @sort
 - sort_title = issuable_sort_option_title(sort_value)
 - viewing_issues = controller.controller_name == 'issues' || controller.action_name == 'issues'
-- manual_sorting = viewing_issues && controller.controller_name != 'dashboard' && Feature.enabled?(:manual_sorting)
+- manual_sorting = viewing_issues && controller.controller_name != 'dashboard' && Feature.enabled?(:manual_sorting, default_enabled: true)
 
 .dropdown.inline.prepend-left-10.issue-sort-dropdown
   .btn-group{ role: 'group' }
-- 
cgit v1.2.1


From 0d9c815d5a61d0369f34adfab42fc860826100b8 Mon Sep 17 00:00:00 2001
From: Clement Ho <408677-ClemMakesApps@users.noreply.gitlab.com>
Date: Mon, 22 Jul 2019 17:12:41 +0000
Subject: Merge branch 'fix-i18n-updated-projects' into 'master'

Fix translation of 'Updated' term in projects list

See merge request gitlab-org/gitlab-ce!30958

(cherry picked from commit 3cc5535c0c7a52c93b00c1a1260eb8d828a3e9dc)

324b8109 Fix translation of 'Updated' term in projects list
---
 app/views/shared/projects/_project.html.haml        | 2 +-
 changelogs/unreleased/fix-i18n-updated-projects.yml | 5 +++++
 2 files changed, 6 insertions(+), 1 deletion(-)
 create mode 100644 changelogs/unreleased/fix-i18n-updated-projects.yml

diff --git a/app/views/shared/projects/_project.html.haml b/app/views/shared/projects/_project.html.haml
index 04f72420fe1..c4d1bdad2c4 100644
--- a/app/views/shared/projects/_project.html.haml
+++ b/app/views/shared/projects/_project.html.haml
@@ -89,4 +89,4 @@
             %span.icon-wrapper.pipeline-status
               = render 'ci/status/icon', status: project.commit.last_pipeline.detailed_status(current_user), type: 'commit', tooltip_placement: 'top', path: pipeline_path
         .updated-note
-          %span _('Updated') #{updated_tooltip}
+          %span #{_('Updated')} #{updated_tooltip}
diff --git a/changelogs/unreleased/fix-i18n-updated-projects.yml b/changelogs/unreleased/fix-i18n-updated-projects.yml
new file mode 100644
index 00000000000..408ee438480
--- /dev/null
+++ b/changelogs/unreleased/fix-i18n-updated-projects.yml
@@ -0,0 +1,5 @@
+---
+title: Properly translate term in projects list
+merge_request: 30958
+author:
+type: fixed
-- 
cgit v1.2.1


From ec1e7fa803d5e1d1e8515f563554932510522eee Mon Sep 17 00:00:00 2001
From: Stan Hu <stanhu@gmail.com>
Date: Thu, 25 Jul 2019 21:32:39 +0000
Subject: Merge branch 'print-sidekiq-class-name-in-memory-killer' into
 'master'

Logging sidekiq worker class name in SidekiqMemoryKiller

See merge request gitlab-org/gitlab-ce!30996

(cherry picked from commit 3736e1de27ab05a280cc78e6542b4e41f94aa805)

ad7acfaa Logging sidekiq worker class name in SidekiqMemoryKiller
---
 lib/gitlab/sidekiq_middleware/memory_killer.rb     | 23 +++++++++++++++-------
 .../sidekiq_middleware/memory_killer_spec.rb       |  3 +++
 2 files changed, 19 insertions(+), 7 deletions(-)

diff --git a/lib/gitlab/sidekiq_middleware/memory_killer.rb b/lib/gitlab/sidekiq_middleware/memory_killer.rb
index 671d795ec33..4b10f921ed8 100644
--- a/lib/gitlab/sidekiq_middleware/memory_killer.rb
+++ b/lib/gitlab/sidekiq_middleware/memory_killer.rb
@@ -14,9 +14,12 @@ module Gitlab
       # shut Sidekiq down
       MUTEX = Mutex.new
 
+      attr_reader :worker
+
       def call(worker, job, queue)
         yield
 
+        @worker = worker
         current_rss = get_rss
 
         return unless MAX_RSS > 0 && current_rss > MAX_RSS
@@ -25,9 +28,11 @@ module Gitlab
           # Return if another thread is already waiting to shut Sidekiq down
           next unless MUTEX.try_lock
 
-          Sidekiq.logger.warn "Sidekiq worker PID-#{pid} current RSS #{current_rss}"\
-            " exceeds maximum RSS #{MAX_RSS} after finishing job #{worker.class} JID-#{job['jid']}"
-          Sidekiq.logger.warn "Sidekiq worker PID-#{pid} will stop fetching new jobs in #{GRACE_TIME} seconds, and will be shut down #{SHUTDOWN_WAIT} seconds later"
+          warn("Sidekiq worker PID-#{pid} current RSS #{current_rss}"\
+               " exceeds maximum RSS #{MAX_RSS} after finishing job #{worker.class} JID-#{job['jid']}")
+
+          warn("Sidekiq worker PID-#{pid} will stop fetching new jobs"\
+               " in #{GRACE_TIME} seconds, and will be shut down #{SHUTDOWN_WAIT} seconds later")
 
           # Wait `GRACE_TIME` to give the memory intensive job time to finish.
           # Then, tell Sidekiq to stop fetching new jobs.
@@ -59,24 +64,28 @@ module Gitlab
       def wait_and_signal_pgroup(time, signal, explanation)
         return wait_and_signal(time, signal, explanation) unless Process.getpgrp == pid
 
-        Sidekiq.logger.warn "waiting #{time} seconds before sending Sidekiq worker PGRP-#{pid} #{signal} (#{explanation})"
+        warn("waiting #{time} seconds before sending Sidekiq worker PGRP-#{pid} #{signal} (#{explanation})", signal: signal)
         sleep(time)
 
-        Sidekiq.logger.warn "sending Sidekiq worker PGRP-#{pid} #{signal} (#{explanation})"
+        warn("sending Sidekiq worker PGRP-#{pid} #{signal} (#{explanation})", signal: signal)
         Process.kill(signal, 0)
       end
 
       def wait_and_signal(time, signal, explanation)
-        Sidekiq.logger.warn "waiting #{time} seconds before sending Sidekiq worker PID-#{pid} #{signal} (#{explanation})"
+        warn("waiting #{time} seconds before sending Sidekiq worker PID-#{pid} #{signal} (#{explanation})", signal: signal)
         sleep(time)
 
-        Sidekiq.logger.warn "sending Sidekiq worker PID-#{pid} #{signal} (#{explanation})"
+        warn("sending Sidekiq worker PID-#{pid} #{signal} (#{explanation})", signal: signal)
         Process.kill(signal, pid)
       end
 
       def pid
         Process.pid
       end
+
+      def warn(message, signal: nil)
+        Sidekiq.logger.warn(class: worker.class, pid: pid, signal: signal, message: message)
+      end
     end
   end
 end
diff --git a/spec/lib/gitlab/sidekiq_middleware/memory_killer_spec.rb b/spec/lib/gitlab/sidekiq_middleware/memory_killer_spec.rb
index 1a5a38b5d99..b451844f06c 100644
--- a/spec/lib/gitlab/sidekiq_middleware/memory_killer_spec.rb
+++ b/spec/lib/gitlab/sidekiq_middleware/memory_killer_spec.rb
@@ -45,6 +45,9 @@ describe Gitlab::SidekiqMiddleware::MemoryKiller do
       expect(subject).to receive(:sleep).with(10).ordered
       expect(Process).to receive(:kill).with('SIGKILL', pid).ordered
 
+      expect(Sidekiq.logger)
+        .to receive(:warn).with(class: 'TestWorker', message: anything, pid: pid, signal: anything).at_least(:once)
+
       run
     end
 
-- 
cgit v1.2.1


From dbbdfba9e3daa8c714c9f9509d01b8bf3134531b Mon Sep 17 00:00:00 2001
From: Nick Thomas <nick@gitlab.com>
Date: Fri, 26 Jul 2019 11:27:00 +0000
Subject: Merge branch 'dm-submodule-links-nil' into 'master'

Fix error rendering submodules in MR diffs when there is no .gitmodules

See merge request gitlab-org/gitlab-ce!31162

(cherry picked from commit 55f99e930e1c147ec191a234ff4881ea7e70ea61)

cfef1e8e Fix error rendering submodules in MR diffs when there is no .gitmodules
---
 app/helpers/submodule_helper.rb                  |  2 +
 changelogs/unreleased/dm-submodule-links-nil.yml |  5 +++
 lib/gitlab/submodule_links.rb                    | 11 ++++--
 spec/helpers/submodule_helper_spec.rb            | 13 +++++++
 spec/lib/gitlab/submodule_links_spec.rb          | 47 ++++++++++++++++++++++++
 5 files changed, 75 insertions(+), 3 deletions(-)
 create mode 100644 changelogs/unreleased/dm-submodule-links-nil.yml
 create mode 100644 spec/lib/gitlab/submodule_links_spec.rb

diff --git a/app/helpers/submodule_helper.rb b/app/helpers/submodule_helper.rb
index 9a281065b90..e683e2959d1 100644
--- a/app/helpers/submodule_helper.rb
+++ b/app/helpers/submodule_helper.rb
@@ -13,6 +13,8 @@ module SubmoduleHelper
   end
 
   def submodule_links_for_url(submodule_item_id, url, repository)
+    return [nil, nil] unless url
+
     if url == '.' || url == './'
       url = File.join(Gitlab.config.gitlab.url, repository.project.full_path)
     end
diff --git a/changelogs/unreleased/dm-submodule-links-nil.yml b/changelogs/unreleased/dm-submodule-links-nil.yml
new file mode 100644
index 00000000000..c09ca41d01d
--- /dev/null
+++ b/changelogs/unreleased/dm-submodule-links-nil.yml
@@ -0,0 +1,5 @@
+---
+title: Fix error rendering submodules in MR diffs when there is no .gitmodules
+merge_request: 31162
+author:
+type: fixed
diff --git a/lib/gitlab/submodule_links.rb b/lib/gitlab/submodule_links.rb
index a6c0369d864..18fd604a3b0 100644
--- a/lib/gitlab/submodule_links.rb
+++ b/lib/gitlab/submodule_links.rb
@@ -9,7 +9,7 @@ module Gitlab
     end
 
     def for(submodule, sha)
-      submodule_url = submodule_url_for(sha)[submodule.path]
+      submodule_url = submodule_url_for(sha, submodule.path)
       SubmoduleHelper.submodule_links_for_url(submodule.id, submodule_url, repository)
     end
 
@@ -17,10 +17,15 @@ module Gitlab
 
     attr_reader :repository
 
-    def submodule_url_for(sha)
-      strong_memoize(:"submodule_links_for_#{sha}") do
+    def submodule_urls_for(sha)
+      strong_memoize(:"submodule_urls_for_#{sha}") do
         repository.submodule_urls_for(sha)
       end
     end
+
+    def submodule_url_for(sha, path)
+      urls = submodule_urls_for(sha)
+      urls && urls[path]
+    end
   end
 end
diff --git a/spec/helpers/submodule_helper_spec.rb b/spec/helpers/submodule_helper_spec.rb
index b922b910c89..ab4ef899119 100644
--- a/spec/helpers/submodule_helper_spec.rb
+++ b/spec/helpers/submodule_helper_spec.rb
@@ -229,6 +229,19 @@ describe SubmoduleHelper do
         end
       end
     end
+
+    context 'unknown submodule' do
+      before do
+        # When there is no `.gitmodules` file, or if `.gitmodules` does not
+        # know the submodule at the specified path,
+        # `Repository#submodule_url_for` returns `nil`
+        stub_url(nil)
+      end
+
+      it 'returns no links' do
+        expect(subject).to eq([nil, nil])
+      end
+    end
   end
 
   context 'as view helpers in view context' do
diff --git a/spec/lib/gitlab/submodule_links_spec.rb b/spec/lib/gitlab/submodule_links_spec.rb
new file mode 100644
index 00000000000..a84602cd07d
--- /dev/null
+++ b/spec/lib/gitlab/submodule_links_spec.rb
@@ -0,0 +1,47 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe Gitlab::SubmoduleLinks do
+  let(:submodule_item) { double(id: 'hash', path: 'gitlab-ce') }
+  let(:repo) { double }
+  let(:links) { described_class.new(repo) }
+
+  describe '#for' do
+    subject { links.for(submodule_item, 'ref') }
+
+    context 'when there is no .gitmodules file' do
+      before do
+        stub_urls(nil)
+      end
+
+      it 'returns no links' do
+        expect(subject).to eq([nil, nil])
+      end
+    end
+
+    context 'when the submodule is unknown' do
+      before do
+        stub_urls({ 'path' => 'url' })
+      end
+
+      it 'returns no links' do
+        expect(subject).to eq([nil, nil])
+      end
+    end
+
+    context 'when the submodule is known' do
+      before do
+        stub_urls({ 'gitlab-ce' => 'git@gitlab.com:gitlab-org/gitlab-ce.git' })
+      end
+
+      it 'returns links' do
+        expect(subject).to eq(['https://gitlab.com/gitlab-org/gitlab-ce', 'https://gitlab.com/gitlab-org/gitlab-ce/tree/hash'])
+      end
+    end
+  end
+
+  def stub_urls(urls)
+    allow(repo).to receive(:submodule_urls_for).and_return(urls)
+  end
+end
-- 
cgit v1.2.1


From 3d7f98c233af59afc54618cab1fe4f5fe8591319 Mon Sep 17 00:00:00 2001
From: GitLab Release Tools Bot <robert+release-tools@gitlab.com>
Date: Fri, 26 Jul 2019 13:40:49 +0000
Subject: Merge branch 'security-bvl-filter-mr-params' into 'master'

Filter params in MR build service

Closes #2879

See merge request gitlab/gitlabhq!3237
---
 app/services/merge_requests/build_service.rb       | 28 ++++++++---
 .../unreleased/security-bvl-filter-mr-params.yml   |  5 ++
 ...ess_private_project_info_through_new_mr_spec.rb | 56 ++++++++++++++++++++++
 ...ccess_private_repository_through_new_mr_spec.rb | 37 --------------
 spec/services/merge_requests/build_service_spec.rb | 37 +++++++++++++-
 5 files changed, 119 insertions(+), 44 deletions(-)
 create mode 100644 changelogs/unreleased/security-bvl-filter-mr-params.yml
 create mode 100644 spec/features/merge_request/user_tries_to_access_private_project_info_through_new_mr_spec.rb
 delete mode 100644 spec/features/merge_request/user_tries_to_access_private_repository_through_new_mr_spec.rb

diff --git a/app/services/merge_requests/build_service.rb b/app/services/merge_requests/build_service.rb
index 109c964e577..b28f80939ae 100644
--- a/app/services/merge_requests/build_service.rb
+++ b/app/services/merge_requests/build_service.rb
@@ -11,15 +11,18 @@ module MergeRequests
       # https://gitlab.com/gitlab-org/gitlab-ce/issues/53658
       merge_quick_actions_into_params!(merge_request, only: [:target_branch])
       merge_request.merge_params['force_remove_source_branch'] = params.delete(:force_remove_source_branch) if params.has_key?(:force_remove_source_branch)
-      merge_request.assign_attributes(params)
 
+      # Assign the projects first so we can use policies for `filter_params`
       merge_request.author = current_user
+      merge_request.source_project = find_source_project
+      merge_request.target_project = find_target_project
+
+      filter_params(merge_request)
+      merge_request.assign_attributes(params.to_h.compact)
+
       merge_request.compare_commits = []
-      merge_request.source_project  = find_source_project
-      merge_request.target_project  = find_target_project
-      merge_request.target_branch   = find_target_branch
-      merge_request.can_be_created  = projects_and_branches_valid?
-      ensure_milestone_available(merge_request)
+      merge_request.target_branch = find_target_branch
+      merge_request.can_be_created = projects_and_branches_valid?
 
       # compare branches only if branches are valid, otherwise
       # compare_branches may raise an error
@@ -50,12 +53,14 @@ module MergeRequests
              to: :merge_request
 
     def find_source_project
+      source_project = project_from_params(:source_project)
       return source_project if source_project.present? && can?(current_user, :create_merge_request_from, source_project)
 
       project
     end
 
     def find_target_project
+      target_project = project_from_params(:target_project)
       return target_project if target_project.present? && can?(current_user, :create_merge_request_in, target_project)
 
       target_project = project.default_merge_request_target
@@ -65,6 +70,17 @@ module MergeRequests
       project
     end
 
+    def project_from_params(param_name)
+      project_from_params = params.delete(param_name)
+
+      id_param_name = :"#{param_name}_id"
+      if project_from_params.nil? && params[id_param_name]
+        project_from_params = Project.find_by_id(params.delete(id_param_name))
+      end
+
+      project_from_params
+    end
+
     def find_target_branch
       target_branch || target_project.default_branch
     end
diff --git a/changelogs/unreleased/security-bvl-filter-mr-params.yml b/changelogs/unreleased/security-bvl-filter-mr-params.yml
new file mode 100644
index 00000000000..4433ec73b7c
--- /dev/null
+++ b/changelogs/unreleased/security-bvl-filter-mr-params.yml
@@ -0,0 +1,5 @@
+---
+title: Filter merge request params on the new merge request page
+merge_request:
+author:
+type: security
diff --git a/spec/features/merge_request/user_tries_to_access_private_project_info_through_new_mr_spec.rb b/spec/features/merge_request/user_tries_to_access_private_project_info_through_new_mr_spec.rb
new file mode 100644
index 00000000000..1ebe9e2e409
--- /dev/null
+++ b/spec/features/merge_request/user_tries_to_access_private_project_info_through_new_mr_spec.rb
@@ -0,0 +1,56 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe 'Merge Request > User tries to access private project information through the new mr page' do
+  let(:current_user) { create(:user) }
+  let(:private_project) do
+    create(:project, :public, :repository,
+           path: 'nothing-to-see-here',
+           name: 'nothing to see here',
+           repository_access_level: ProjectFeature::PRIVATE)
+  end
+  let(:owned_project) do
+    create(:project, :public, :repository,
+           namespace: current_user.namespace,
+           creator: current_user)
+  end
+
+  context 'when the user enters the querystring info for the other project' do
+    let(:mr_path) do
+      project_new_merge_request_diffs_path(
+        owned_project,
+        merge_request: {
+          source_project_id: private_project.id,
+          source_branch: 'feature'
+        }
+      )
+    end
+
+    before do
+      sign_in current_user
+      visit mr_path
+    end
+
+    it "does not mention the project the user can't see the repo of" do
+      expect(page).not_to have_content('nothing-to-see-here')
+    end
+
+    context 'when the user enters label information from the private project in the querystring' do
+      let(:inaccessible_label) { create(:label, project: private_project) }
+      let(:mr_path) do
+        project_new_merge_request_path(
+          owned_project,
+          merge_request: {
+            label_ids: [inaccessible_label.id],
+            source_branch: 'feature'
+          }
+        )
+      end
+
+      it 'does not expose the label name' do
+        expect(page).not_to have_content(inaccessible_label.name)
+      end
+    end
+  end
+end
diff --git a/spec/features/merge_request/user_tries_to_access_private_repository_through_new_mr_spec.rb b/spec/features/merge_request/user_tries_to_access_private_repository_through_new_mr_spec.rb
deleted file mode 100644
index 9318b5f1ebb..00000000000
--- a/spec/features/merge_request/user_tries_to_access_private_repository_through_new_mr_spec.rb
+++ /dev/null
@@ -1,37 +0,0 @@
-require 'spec_helper'
-
-describe 'Merge Request > Tries to access private repo of public project' do
-  let(:current_user) { create(:user) }
-  let(:private_project) do
-    create(:project, :public, :repository,
-           path: 'nothing-to-see-here',
-           name: 'nothing to see here',
-           repository_access_level: ProjectFeature::PRIVATE)
-  end
-  let(:owned_project) do
-    create(:project, :public, :repository,
-           namespace: current_user.namespace,
-           creator: current_user)
-  end
-
-  context 'when the user enters the querystring info for the other project' do
-    let(:mr_path) do
-      project_new_merge_request_diffs_path(
-        owned_project,
-        merge_request: {
-          source_project_id: private_project.id,
-          source_branch: 'feature'
-        }
-      )
-    end
-
-    before do
-      sign_in current_user
-      visit mr_path
-    end
-
-    it "does not mention the project the user can't see the repo of" do
-      expect(page).not_to have_content('nothing-to-see-here')
-    end
-  end
-end
diff --git a/spec/services/merge_requests/build_service_spec.rb b/spec/services/merge_requests/build_service_spec.rb
index 5c3b209086c..f18239f6d39 100644
--- a/spec/services/merge_requests/build_service_spec.rb
+++ b/spec/services/merge_requests/build_service_spec.rb
@@ -1,5 +1,4 @@
 # frozen_string_literal: true
-
 require 'spec_helper'
 
 describe MergeRequests::BuildService do
@@ -225,6 +224,11 @@ describe MergeRequests::BuildService do
             let(:label_ids) { [label2.id] }
             let(:milestone_id) { milestone2.id }
 
+            before do
+              # Guests are not able to assign labels or milestones to an issue
+              project.add_developer(user)
+            end
+
             it 'assigns milestone_id and label_ids instead of issue labels and milestone' do
               expect(merge_request.milestone).to eq(milestone2)
               expect(merge_request.labels).to match_array([label2])
@@ -479,4 +483,35 @@ describe MergeRequests::BuildService do
       end
     end
   end
+
+  context 'when assigning labels' do
+    let(:label_ids) { [create(:label, project: project).id] }
+
+    context 'for members with less than developer access' do
+      it 'is not allowed' do
+        expect(merge_request.label_ids).to be_empty
+      end
+    end
+
+    context 'for users allowed to assign labels' do
+      before do
+        project.add_developer(user)
+      end
+
+      context 'for labels in the project' do
+        it 'is allowed for developers' do
+          expect(merge_request.label_ids).to contain_exactly(*label_ids)
+        end
+      end
+
+      context 'for unrelated labels' do
+        let(:project_label) { create(:label, project: project) }
+        let(:label_ids) { [create(:label).id, project_label.id] }
+
+        it 'only assigns related labels' do
+          expect(merge_request.label_ids).to contain_exactly(project_label.id)
+        end
+      end
+    end
+  end
 end
-- 
cgit v1.2.1


From b9ba0f016831c476f6316e7af22ffc3ec177bd9e Mon Sep 17 00:00:00 2001
From: GitLab Release Tools Bot <robert+release-tools@gitlab.com>
Date: Fri, 26 Jul 2019 13:40:52 +0000
Subject: Merge branch 'security-hide_moved_issue_id' into 'master'

Do not show moved issue ids for user not authorized

Closes #2878

See merge request gitlab/gitlabhq!3230
---
 app/serializers/issue_entity.rb                    |  7 ++++-
 .../unreleased/security-hide_moved_issue_id.yml    |  5 ++++
 spec/serializers/issue_entity_spec.rb              | 33 ++++++++++++++++++++++
 3 files changed, 44 insertions(+), 1 deletion(-)
 create mode 100644 changelogs/unreleased/security-hide_moved_issue_id.yml

diff --git a/app/serializers/issue_entity.rb b/app/serializers/issue_entity.rb
index 36e601f45c5..82139855760 100644
--- a/app/serializers/issue_entity.rb
+++ b/app/serializers/issue_entity.rb
@@ -16,9 +16,14 @@ class IssueEntity < IssuableEntity
   expose :discussion_locked
   expose :assignees, using: API::Entities::UserBasic
   expose :due_date
-  expose :moved_to_id
   expose :project_id
 
+  expose :moved_to_id do |issue|
+    if issue.moved_to_id.present? && can?(request.current_user, :read_issue, issue.moved_to)
+      issue.moved_to_id
+    end
+  end
+
   expose :web_url do |issue|
     project_issue_path(issue.project, issue)
   end
diff --git a/changelogs/unreleased/security-hide_moved_issue_id.yml b/changelogs/unreleased/security-hide_moved_issue_id.yml
new file mode 100644
index 00000000000..24353d797c9
--- /dev/null
+++ b/changelogs/unreleased/security-hide_moved_issue_id.yml
@@ -0,0 +1,5 @@
+---
+title: Do not show moved issue id for users that cannot read issue
+merge_request:
+author:
+type: security
diff --git a/spec/serializers/issue_entity_spec.rb b/spec/serializers/issue_entity_spec.rb
index caa3e41402b..0e05b3c84f4 100644
--- a/spec/serializers/issue_entity_spec.rb
+++ b/spec/serializers/issue_entity_spec.rb
@@ -17,4 +17,37 @@ describe IssueEntity do
   it 'has time estimation attributes' do
     expect(subject).to include(:time_estimate, :total_time_spent, :human_time_estimate, :human_total_time_spent)
   end
+
+  context 'when issue got moved' do
+    let(:public_project) { create(:project, :public) }
+    let(:member) { create(:user) }
+    let(:non_member) { create(:user) }
+    let(:issue) { create(:issue, project: public_project) }
+
+    before do
+      project.add_developer(member)
+      public_project.add_developer(member)
+      Issues::MoveService.new(public_project, member).execute(issue, project)
+    end
+
+    context 'when user cannot read target project' do
+      it 'does not return moved_to_id' do
+        request = double('request', current_user: non_member)
+
+        response = described_class.new(issue, request: request).as_json
+
+        expect(response[:moved_to_id]).to be_nil
+      end
+    end
+
+    context 'when user can read target project' do
+      it 'returns moved moved_to_id' do
+        request = double('request', current_user: member)
+
+        response = described_class.new(issue, request: request).as_json
+
+        expect(response[:moved_to_id]).to eq(issue.moved_to_id)
+      end
+    end
+  end
 end
-- 
cgit v1.2.1


From 8b43d01b5a47803daa4e9672b7c36bdfe91dba2d Mon Sep 17 00:00:00 2001
From: GitLab Release Tools Bot <robert+release-tools@gitlab.com>
Date: Fri, 26 Jul 2019 13:40:54 +0000
Subject: Merge branch 'security-60551-fix-upload-scope' into 'master'

Queries for Upload should be scoped by model

See merge request gitlab/gitlabhq!3229
---
 app/controllers/concerns/uploads_actions.rb         |  2 +-
 app/uploaders/records_uploads.rb                    |  2 +-
 .../unreleased/security-60551-fix-upload-scope.yml  |  5 +++++
 spec/controllers/groups/uploads_controller_spec.rb  |  5 +++++
 .../controllers/projects/uploads_controller_spec.rb |  5 +++++
 .../controllers/uploads_actions_shared_examples.rb  | 10 ++++++++++
 spec/uploaders/records_uploads_spec.rb              | 21 +++++++++++++++++++++
 7 files changed, 48 insertions(+), 2 deletions(-)
 create mode 100644 changelogs/unreleased/security-60551-fix-upload-scope.yml

diff --git a/app/controllers/concerns/uploads_actions.rb b/app/controllers/concerns/uploads_actions.rb
index 59f6d3452a3..f5d35379e10 100644
--- a/app/controllers/concerns/uploads_actions.rb
+++ b/app/controllers/concerns/uploads_actions.rb
@@ -90,7 +90,7 @@ module UploadsActions
     return unless uploader = build_uploader
 
     upload_paths = uploader.upload_paths(params[:filename])
-    upload = Upload.find_by(uploader: uploader_class.to_s, path: upload_paths)
+    upload = Upload.find_by(model: model, uploader: uploader_class.to_s, path: upload_paths)
     upload&.build_uploader
   end
   # rubocop: enable CodeReuse/ActiveRecord
diff --git a/app/uploaders/records_uploads.rb b/app/uploaders/records_uploads.rb
index 00b51f92b12..d7a859ebd5f 100644
--- a/app/uploaders/records_uploads.rb
+++ b/app/uploaders/records_uploads.rb
@@ -35,7 +35,7 @@ module RecordsUploads
     end
 
     def readd_upload
-      uploads.where(path: upload_path).delete_all
+      uploads.where(model: model, path: upload_path).delete_all
       upload.delete if upload
 
       self.upload = build_upload.tap(&:save!)
diff --git a/changelogs/unreleased/security-60551-fix-upload-scope.yml b/changelogs/unreleased/security-60551-fix-upload-scope.yml
new file mode 100644
index 00000000000..7d7096833a7
--- /dev/null
+++ b/changelogs/unreleased/security-60551-fix-upload-scope.yml
@@ -0,0 +1,5 @@
+---
+title: Queries for Upload should be scoped by model
+merge_request:
+author:
+type: security
diff --git a/spec/controllers/groups/uploads_controller_spec.rb b/spec/controllers/groups/uploads_controller_spec.rb
index 0f99a957581..60342bf8e3d 100644
--- a/spec/controllers/groups/uploads_controller_spec.rb
+++ b/spec/controllers/groups/uploads_controller_spec.rb
@@ -10,6 +10,11 @@ describe Groups::UploadsController do
     { group_id: model }
   end
 
+  let(:other_model) { create(:group, :public) }
+  let(:other_params) do
+    { group_id: other_model }
+  end
+
   it_behaves_like 'handle uploads' do
     let(:uploader_class) { NamespaceFileUploader }
   end
diff --git a/spec/controllers/projects/uploads_controller_spec.rb b/spec/controllers/projects/uploads_controller_spec.rb
index 776c1270977..661ed9840b1 100644
--- a/spec/controllers/projects/uploads_controller_spec.rb
+++ b/spec/controllers/projects/uploads_controller_spec.rb
@@ -10,6 +10,11 @@ describe Projects::UploadsController do
     { namespace_id: model.namespace.to_param, project_id: model }
   end
 
+  let(:other_model) { create(:project, :public) }
+  let(:other_params) do
+    { namespace_id: other_model.namespace.to_param, project_id: other_model }
+  end
+
   it_behaves_like 'handle uploads'
 
   context 'when the URL the old style, without /-/system' do
diff --git a/spec/support/shared_examples/controllers/uploads_actions_shared_examples.rb b/spec/support/shared_examples/controllers/uploads_actions_shared_examples.rb
index 59708173716..9036838e50a 100644
--- a/spec/support/shared_examples/controllers/uploads_actions_shared_examples.rb
+++ b/spec/support/shared_examples/controllers/uploads_actions_shared_examples.rb
@@ -74,6 +74,16 @@ shared_examples 'handle uploads' do
       UploadService.new(model, jpg, uploader_class).execute
     end
 
+    context 'when accessing a specific upload via different model' do
+      it 'responds with status 404' do
+        params.merge!(other_params)
+
+        show_upload
+
+        expect(response).to have_gitlab_http_status(404)
+      end
+    end
+
     context "when the model is public" do
       before do
         model.update_attribute(:visibility_level, Gitlab::VisibilityLevel::PUBLIC)
diff --git a/spec/uploaders/records_uploads_spec.rb b/spec/uploaders/records_uploads_spec.rb
index 42352f9b9f8..6134137d2b7 100644
--- a/spec/uploaders/records_uploads_spec.rb
+++ b/spec/uploaders/records_uploads_spec.rb
@@ -85,6 +85,27 @@ describe RecordsUploads do
       expect { existing.reload }.to raise_error(ActiveRecord::RecordNotFound)
       expect(Upload.count).to eq(1)
     end
+
+    it 'does not affect other uploads with different model but the same path' do
+      project = create(:project)
+      other_project = create(:project)
+
+      uploader = RecordsUploadsExampleUploader.new(other_project)
+
+      upload_for_project = Upload.create!(
+        path: File.join('uploads', 'rails_sample.jpg'),
+        size: 512.kilobytes,
+        model: project,
+        uploader: uploader.class.to_s
+      )
+
+      uploader.store!(upload_fixture('rails_sample.jpg'))
+
+      upload_for_project_fresh = Upload.find(upload_for_project.id)
+
+      expect(upload_for_project).to eq(upload_for_project_fresh)
+      expect(Upload.count).to eq(2)
+    end
   end
 
   describe '#destroy_upload callback' do
-- 
cgit v1.2.1


From 557ce163dbb648e7071ba04a58d40ed291d91fd2 Mon Sep 17 00:00:00 2001
From: GitLab Release Tools Bot <robert+release-tools@gitlab.com>
Date: Fri, 26 Jul 2019 13:40:57 +0000
Subject: Merge branch 'security-dns-ssrf-bypass' into 'master'

Server Side Request Forgery mitigation bypass

Closes #2872

See merge request gitlab/gitlabhq!3205
---
 changelogs/unreleased/security-dns-ssrf-bypass.yml |  5 +++
 lib/gitlab/url_blocker.rb                          | 13 ++++++-
 spec/lib/gitlab/url_blocker_spec.rb                | 44 ++++++++++++++++------
 spec/spec_helper.rb                                |  1 +
 4 files changed, 49 insertions(+), 14 deletions(-)
 create mode 100644 changelogs/unreleased/security-dns-ssrf-bypass.yml

diff --git a/changelogs/unreleased/security-dns-ssrf-bypass.yml b/changelogs/unreleased/security-dns-ssrf-bypass.yml
new file mode 100644
index 00000000000..e48696ce5bd
--- /dev/null
+++ b/changelogs/unreleased/security-dns-ssrf-bypass.yml
@@ -0,0 +1,5 @@
+---
+title: Fix Server Side Request Forgery mitigation bypass
+merge_request:
+author:
+type: security
diff --git a/lib/gitlab/url_blocker.rb b/lib/gitlab/url_blocker.rb
index f6b2e2acf16..cc937cbb3cf 100644
--- a/lib/gitlab/url_blocker.rb
+++ b/lib/gitlab/url_blocker.rb
@@ -85,9 +85,9 @@ module Gitlab
       # we'll be making the request to the IP address, instead of using the hostname.
       def enforce_uri_hostname(addrs_info, uri, hostname, dns_rebind_protection)
         address = addrs_info.first
-        ip_address = address&.ip_address
+        ip_address = address.ip_address
 
-        return [uri, nil] unless dns_rebind_protection && ip_address && ip_address != hostname
+        return [uri, nil] unless dns_rebind_protection && ip_address != hostname
 
         uri = uri.dup
         uri.hostname = ip_address
@@ -111,6 +111,15 @@ module Gitlab
           addr.ipv6_v4mapped? ? addr.ipv6_to_ipv4 : addr
         end
       rescue SocketError
+        # In the test suite we use a lot of mocked urls that are either invalid or
+        # don't exist. In order to avoid modifying a ton of tests and factories
+        # we allow invalid urls unless the environment variable RSPEC_ALLOW_INVALID_URLS
+        # is not true
+        return if Rails.env.test? && ENV['RSPEC_ALLOW_INVALID_URLS'] == 'true'
+
+        # If the addr can't be resolved or the url is invalid (i.e http://1.1.1.1.1)
+        # we block the url
+        raise BlockedUrlError, "Host cannot be resolved or invalid"
       end
 
       def validate_local_request(address_info:, allow_localhost:, allow_local_network:)
diff --git a/spec/lib/gitlab/url_blocker_spec.rb b/spec/lib/gitlab/url_blocker_spec.rb
index f8b0cbfb6f6..0fab890978a 100644
--- a/spec/lib/gitlab/url_blocker_spec.rb
+++ b/spec/lib/gitlab/url_blocker_spec.rb
@@ -68,6 +68,16 @@ describe Gitlab::UrlBlocker do
           expect(uri).to eq(Addressable::URI.parse('https://example.org'))
           expect(hostname).to eq(nil)
         end
+
+        context 'when it cannot be resolved' do
+          let(:import_url) { 'http://foobar.x' }
+
+          it 'raises error' do
+            stub_env('RSPEC_ALLOW_INVALID_URLS', 'false')
+
+            expect { described_class.validate!(import_url) }.to raise_error(described_class::BlockedUrlError)
+          end
+        end
       end
 
       context 'when the URL hostname is an IP address' do
@@ -79,6 +89,16 @@ describe Gitlab::UrlBlocker do
           expect(uri).to eq(Addressable::URI.parse('https://93.184.216.34'))
           expect(hostname).to be(nil)
         end
+
+        context 'when it is invalid' do
+          let(:import_url) { 'http://1.1.1.1.1' }
+
+          it 'raises an error' do
+            stub_env('RSPEC_ALLOW_INVALID_URLS', 'false')
+
+            expect { described_class.validate!(import_url) }.to raise_error(described_class::BlockedUrlError)
+          end
+        end
       end
     end
   end
@@ -180,8 +200,6 @@ describe Gitlab::UrlBlocker do
     end
 
     it 'returns true for a non-alphanumeric hostname' do
-      stub_resolv
-
       aggregate_failures do
         expect(described_class).to be_blocked_url('ssh://-oProxyCommand=whoami/a')
 
@@ -300,10 +318,6 @@ describe Gitlab::UrlBlocker do
     end
 
     context 'when enforce_user is' do
-      before do
-        stub_resolv
-      end
-
       context 'false (default)' do
         it 'does not block urls with a non-alphanumeric username' do
           expect(described_class).not_to be_blocked_url('ssh://-oProxyCommand=whoami@example.com/a')
@@ -351,6 +365,18 @@ describe Gitlab::UrlBlocker do
         expect(described_class.blocked_url?('https://git‌lab.com/foo/foo.bar', ascii_only: true)).to be true
       end
     end
+
+    it 'blocks urls with invalid ip address' do
+      stub_env('RSPEC_ALLOW_INVALID_URLS', 'false')
+
+      expect(described_class).to be_blocked_url('http://8.8.8.8.8')
+    end
+
+    it 'blocks urls whose hostname cannot be resolved' do
+      stub_env('RSPEC_ALLOW_INVALID_URLS', 'false')
+
+      expect(described_class).to be_blocked_url('http://foobar.x')
+    end
   end
 
   describe '#validate_hostname' do
@@ -382,10 +408,4 @@ describe Gitlab::UrlBlocker do
       end
     end
   end
-
-  # Resolv does not support resolving UTF-8 domain names
-  # See https://bugs.ruby-lang.org/issues/4270
-  def stub_resolv
-    allow(Resolv).to receive(:getaddresses).and_return([])
-  end
 end
diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb
index a6fb172e79b..ab04cc1dae6 100644
--- a/spec/spec_helper.rb
+++ b/spec/spec_helper.rb
@@ -3,6 +3,7 @@ SimpleCovEnv.start!
 
 ENV["RAILS_ENV"] = 'test'
 ENV["IN_MEMORY_APPLICATION_SETTINGS"] = 'true'
+ENV["RSPEC_ALLOW_INVALID_URLS"] = 'true'
 
 require File.expand_path('../config/environment', __dir__)
 require 'rspec/rails'
-- 
cgit v1.2.1


From 787688616d71e73ff3b4d6202acc61ce60f98b71 Mon Sep 17 00:00:00 2001
From: GitLab Release Tools Bot <robert+release-tools@gitlab.com>
Date: Fri, 26 Jul 2019 13:40:59 +0000
Subject: Merge branch 'security-mr-pipeline-permissions' into 'master'

MR pipeline permissions

Closes #2871

See merge request gitlab/gitlabhq!3204
---
 .../merge_requests/application_controller.rb       |  2 +-
 .../projects/merge_requests_controller.rb          |  3 +-
 .../security-mr-pipeline-permissions.yml           |  5 ++
 .../projects/merge_requests_controller_spec.rb     | 98 +++++++++++++++++++++-
 4 files changed, 102 insertions(+), 6 deletions(-)
 create mode 100644 changelogs/unreleased/security-mr-pipeline-permissions.yml

diff --git a/app/controllers/projects/merge_requests/application_controller.rb b/app/controllers/projects/merge_requests/application_controller.rb
index dcc272aecff..006731c0e66 100644
--- a/app/controllers/projects/merge_requests/application_controller.rb
+++ b/app/controllers/projects/merge_requests/application_controller.rb
@@ -45,7 +45,7 @@ class Projects::MergeRequests::ApplicationController < Projects::ApplicationCont
 
   def set_pipeline_variables
     @pipelines =
-      if can?(current_user, :read_pipeline, @project)
+      if can?(current_user, :read_pipeline, @merge_request.source_project)
         @merge_request.all_pipelines
       else
         Ci::Pipeline.none
diff --git a/app/controllers/projects/merge_requests_controller.rb b/app/controllers/projects/merge_requests_controller.rb
index 2aa2508be16..f4d381244d9 100644
--- a/app/controllers/projects/merge_requests_controller.rb
+++ b/app/controllers/projects/merge_requests_controller.rb
@@ -82,7 +82,8 @@ class Projects::MergeRequestsController < Projects::MergeRequests::ApplicationCo
   end
 
   def pipelines
-    @pipelines = @merge_request.all_pipelines.page(params[:page]).per(30)
+    set_pipeline_variables
+    @pipelines = @pipelines.page(params[:page]).per(30)
 
     Gitlab::PollingInterval.set_header(response, interval: 10_000)
 
diff --git a/changelogs/unreleased/security-mr-pipeline-permissions.yml b/changelogs/unreleased/security-mr-pipeline-permissions.yml
new file mode 100644
index 00000000000..a317c93228c
--- /dev/null
+++ b/changelogs/unreleased/security-mr-pipeline-permissions.yml
@@ -0,0 +1,5 @@
+---
+title: Use source project as permissions reference for MergeRequestsController#pipelines
+merge_request:
+author:
+type: security
diff --git a/spec/controllers/projects/merge_requests_controller_spec.rb b/spec/controllers/projects/merge_requests_controller_spec.rb
index f11880122b1..bdd7322290f 100644
--- a/spec/controllers/projects/merge_requests_controller_spec.rb
+++ b/spec/controllers/projects/merge_requests_controller_spec.rb
@@ -621,10 +621,100 @@ describe Projects::MergeRequestsController do
           format: :json
     end
 
-    it 'responds with serialized pipelines' do
-      expect(json_response['pipelines']).not_to be_empty
-      expect(json_response['count']['all']).to eq 1
-      expect(response).to include_pagination_headers
+    context 'with "enabled" builds on a public project' do
+      let(:project) { create(:project, :repository, :public) }
+
+      context 'for a project owner' do
+        it 'responds with serialized pipelines' do
+          expect(json_response['pipelines']).to be_present
+          expect(json_response['count']['all']).to eq(1)
+          expect(response).to include_pagination_headers
+        end
+      end
+
+      context 'for an unassociated user' do
+        let(:user) { create :user }
+
+        it 'responds with no pipelines' do
+          expect(json_response['pipelines']).to be_present
+          expect(json_response['count']['all']).to eq(1)
+          expect(response).to include_pagination_headers
+        end
+      end
+    end
+
+    context 'with private builds on a public project' do
+      let(:project) { create(:project, :repository, :public, :builds_private) }
+
+      context 'for a project owner' do
+        it 'responds with serialized pipelines' do
+          expect(json_response['pipelines']).to be_present
+          expect(json_response['count']['all']).to eq(1)
+          expect(response).to include_pagination_headers
+        end
+      end
+
+      context 'for an unassociated user' do
+        let(:user) { create :user }
+
+        it 'responds with no pipelines' do
+          expect(json_response['pipelines']).to be_empty
+          expect(json_response['count']['all']).to eq(0)
+          expect(response).to include_pagination_headers
+        end
+      end
+
+      context 'from a project fork' do
+        let(:fork_user)      { create :user }
+        let(:forked_project) { fork_project(project, fork_user, repository: true) } # Forked project carries over :builds_private
+        let(:merge_request)  { create(:merge_request_with_diffs, target_project: project, source_project: forked_project) }
+
+        context 'with private builds' do
+          context 'for the target project member' do
+            it 'does not respond with serialized pipelines' do
+              expect(json_response['pipelines']).to be_empty
+              expect(json_response['count']['all']).to eq(0)
+              expect(response).to include_pagination_headers
+            end
+          end
+
+          context 'for the source project member' do
+            let(:user) { fork_user }
+
+            it 'responds with serialized pipelines' do
+              expect(json_response['pipelines']).to be_present
+              expect(json_response['count']['all']).to eq(1)
+              expect(response).to include_pagination_headers
+            end
+          end
+        end
+
+        context 'with public builds' do
+          let(:forked_project) do
+            fork_project(project, fork_user, repository: true).tap do |new_project|
+              new_project.project_feature.update(builds_access_level: ProjectFeature::ENABLED)
+            end
+          end
+
+          context 'for the target project member' do
+            it 'does not respond with serialized pipelines' do
+              expect(json_response['pipelines']).to be_present
+              expect(json_response['count']['all']).to eq(1)
+              expect(response).to include_pagination_headers
+            end
+          end
+
+          context 'for the source project member' do
+            let(:user) { fork_user }
+
+            it 'responds with serialized pipelines' do
+              expect(json_response['pipelines']).to be_present
+              expect(json_response['count']['all']).to eq(1)
+              expect(response).to include_pagination_headers
+            end
+          end
+        end
+      end
     end
   end
 
-- 
cgit v1.2.1


From 273fa2cb36d01def68959957d6226d1e0ef74a06 Mon Sep 17 00:00:00 2001
From: GitLab Release Tools Bot <robert+release-tools@gitlab.com>
Date: Fri, 26 Jul 2019 13:41:02 +0000
Subject: Merge branch 'security-remove-take-trigger-ownership-feature' into
 'master'

Drop feature to take ownership of a trigger token

Closes #2868

See merge request gitlab/gitlabhq!3198
---
 app/controllers/projects/triggers_controller.rb    | 12 +--------
 app/views/projects/triggers/_trigger.html.haml     |  3 ---
 ...urity-remove-take-trigger-ownership-feature.yml |  5 ++++
 config/routes/project.rb                           |  6 +----
 doc/api/pipeline_triggers.md                       | 29 ----------------------
 doc/ci/triggers/README.md                          | 14 +----------
 doc/user/project/new_ci_build_permissions_model.md |  3 +--
 lib/api/triggers.rb                                | 21 ----------------
 locale/gitlab.pot                                  |  6 -----
 spec/features/triggers_spec.rb                     | 23 -----------------
 spec/requests/api/triggers_spec.rb                 | 28 ---------------------
 11 files changed, 9 insertions(+), 141 deletions(-)
 create mode 100644 changelogs/unreleased/security-remove-take-trigger-ownership-feature.yml

diff --git a/app/controllers/projects/triggers_controller.rb b/app/controllers/projects/triggers_controller.rb
index 284e119ca06..7159d0243a3 100644
--- a/app/controllers/projects/triggers_controller.rb
+++ b/app/controllers/projects/triggers_controller.rb
@@ -4,7 +4,7 @@ class Projects::TriggersController < Projects::ApplicationController
   before_action :authorize_admin_build!
   before_action :authorize_manage_trigger!, except: [:index, :create]
   before_action :authorize_admin_trigger!, only: [:edit, :update]
-  before_action :trigger, only: [:take_ownership, :edit, :update, :destroy]
+  before_action :trigger, only: [:edit, :update, :destroy]
 
   layout 'project_settings'
 
@@ -24,16 +24,6 @@ class Projects::TriggersController < Projects::ApplicationController
     redirect_to project_settings_ci_cd_path(@project, anchor: 'js-pipeline-triggers')
   end
 
-  def take_ownership
-    if trigger.update(owner: current_user)
-      flash[:notice] = _('Trigger was re-assigned.')
-    else
-      flash[:alert] = _('You could not take ownership of trigger.')
-    end
-
-    redirect_to project_settings_ci_cd_path(@project, anchor: 'js-pipeline-triggers')
-  end
-
   def edit
   end
 
diff --git a/app/views/projects/triggers/_trigger.html.haml b/app/views/projects/triggers/_trigger.html.haml
index 31a598ccd5e..9899cf9c6de 100644
--- a/app/views/projects/triggers/_trigger.html.haml
+++ b/app/views/projects/triggers/_trigger.html.haml
@@ -33,10 +33,7 @@
       Never
 
   %td.text-right.trigger-actions
-    - take_ownership_confirmation = "By taking ownership you will bind this trigger to your user account. With this the trigger will have access to all your projects as if it was you. Are you sure?"
     - revoke_trigger_confirmation = "By revoking a trigger you will break any processes making use of it. Are you sure?"
-    - if trigger.owner != current_user && can?(current_user, :manage_trigger, trigger)
-      = link_to 'Take ownership', take_ownership_project_trigger_path(@project, trigger), data: { confirm: take_ownership_confirmation }, method: :post, class: "btn btn-default btn-sm btn-trigger-take-ownership"
     - if can?(current_user, :admin_trigger, trigger)
       = link_to edit_project_trigger_path(@project, trigger), method: :get, title: "Edit", class: "btn btn-default btn-sm" do
         %i.fa.fa-pencil
diff --git a/changelogs/unreleased/security-remove-take-trigger-ownership-feature.yml b/changelogs/unreleased/security-remove-take-trigger-ownership-feature.yml
new file mode 100644
index 00000000000..201f66e1f18
--- /dev/null
+++ b/changelogs/unreleased/security-remove-take-trigger-ownership-feature.yml
@@ -0,0 +1,5 @@
+---
+title: Drop feature to take ownership of trigger token.
+merge_request:
+author:
+type: security
diff --git a/config/routes/project.rb b/config/routes/project.rb
index c202463dadb..1f632765317 100644
--- a/config/routes/project.rb
+++ b/config/routes/project.rb
@@ -339,11 +339,7 @@ constraints(::Constraints::ProjectUrlConstrainer.new) do
 
       resource :variables, only: [:show, :update]
 
-      resources :triggers, only: [:index, :create, :edit, :update, :destroy] do
-        member do
-          post :take_ownership
-        end
-      end
+      resources :triggers, only: [:index, :create, :edit, :update, :destroy]
 
       resource :mirror, only: [:show, :update] do
         member do
diff --git a/doc/api/pipeline_triggers.md b/doc/api/pipeline_triggers.md
index 736312df116..e207ff8e98a 100644
--- a/doc/api/pipeline_triggers.md
+++ b/doc/api/pipeline_triggers.md
@@ -120,35 +120,6 @@ curl --request PUT --header "PRIVATE-TOKEN: <your_access_token>" --form descript
 }
 ```
 
-## Take ownership of a project trigger
-
-Update an owner of a project trigger.
-
-```
-POST /projects/:id/triggers/:trigger_id/take_ownership
-```
-
-| Attribute     | Type    | required | Description              |
-|---------------|---------|----------|--------------------------|
-| `id`          | integer/string | yes      | The ID or [URL-encoded path of the project](README.md#namespaced-path-encoding) owned by the authenticated user      |
-| `trigger_id`  | integer | yes      | The trigger id           |
-
-```
-curl --request POST --header "PRIVATE-TOKEN: <your_access_token>" "https://gitlab.example.com/api/v4/projects/1/triggers/10/take_ownership"
-```
-
-```json
-{
-    "id": 10,
-    "description": "my trigger",
-    "created_at": "2016-01-07T09:53:58.235Z",
-    "last_used": null,
-    "token": "6d056f63e50fe6f8c5f8f4aa10edb7",
-    "updated_at": "2016-01-07T09:53:58.235Z",
-    "owner": null
-}
-```
-
 ## Remove a project trigger
 
 Remove a project's build trigger.
diff --git a/doc/ci/triggers/README.md b/doc/ci/triggers/README.md
index d1f9aa03b6b..2a382f18038 100644
--- a/doc/ci/triggers/README.md
+++ b/doc/ci/triggers/README.md
@@ -97,17 +97,6 @@ overview of the time the triggers were last used.
 
 ![Triggers page overview](img/triggers_page.png)
 
-## Taking ownership of a trigger
-
-> **Note**:
-GitLab 9.0 introduced a trigger ownership to solve permission problems.
-
-Each created trigger when run will impersonate their associated user including
-their access to projects and their project permissions.
-
-You can take ownership of existing triggers by clicking *Take ownership*.
-From now on the trigger will be run as you.
-
 ## Revoking a trigger
 
 You can revoke a trigger any time by going at your project's
@@ -282,8 +271,7 @@ Old triggers, created before GitLab 9.0 will be marked as legacy.
 
 Triggers with the legacy label do not have an associated user and only have
 access to the current project. They are considered deprecated and will be
-removed with one of the future versions of GitLab. You are advised to
-[take ownership](#taking-ownership-of-a-trigger) of any legacy triggers.
+removed with one of the future versions of GitLab. 
 
 [ee-2017]: https://gitlab.com/gitlab-org/gitlab-ee/merge_requests/2017
 [ee-2346]: https://gitlab.com/gitlab-org/gitlab-ee/merge_requests/2346
diff --git a/doc/user/project/new_ci_build_permissions_model.md b/doc/user/project/new_ci_build_permissions_model.md
index 494dd539da7..03ae24242e3 100644
--- a/doc/user/project/new_ci_build_permissions_model.md
+++ b/doc/user/project/new_ci_build_permissions_model.md
@@ -90,8 +90,7 @@ to steal the tokens of other jobs.
 
 Since 9.0 [pipeline triggers][triggers] do support the new permission model.
 The new triggers do impersonate their associated user including their access
-to projects and their project permissions. To migrate trigger to use new permission
-model use **Take ownership**.
+to projects and their project permissions. 
 
 ## Before GitLab 8.12
 
diff --git a/lib/api/triggers.rb b/lib/api/triggers.rb
index 0e829c5699b..eeecc390256 100644
--- a/lib/api/triggers.rb
+++ b/lib/api/triggers.rb
@@ -112,27 +112,6 @@ module API
         end
       end
 
-      desc 'Take ownership of trigger' do
-        success Entities::Trigger
-      end
-      params do
-        requires :trigger_id, type: Integer, desc: 'The trigger ID'
-      end
-      post ':id/triggers/:trigger_id/take_ownership' do
-        authenticate!
-        authorize! :admin_build, user_project
-
-        trigger = user_project.triggers.find(params.delete(:trigger_id))
-        break not_found!('Trigger') unless trigger
-
-        if trigger.update(owner: current_user)
-          status :ok
-          present trigger, with: Entities::Trigger, current_user: current_user
-        else
-          render_validation_error!(trigger)
-        end
-      end
-
       desc 'Delete a trigger' do
         success Entities::Trigger
       end
diff --git a/locale/gitlab.pot b/locale/gitlab.pot
index 7b742660a4c..a9c8174a5d6 100644
--- a/locale/gitlab.pot
+++ b/locale/gitlab.pot
@@ -11467,9 +11467,6 @@ msgstr ""
 msgid "Trigger was created successfully."
 msgstr ""
 
-msgid "Trigger was re-assigned."
-msgstr ""
-
 msgid "Trigger was successfully updated."
 msgstr ""
 
@@ -12414,9 +12411,6 @@ msgstr ""
 msgid "You could not create a new trigger."
 msgstr ""
 
-msgid "You could not take ownership of trigger."
-msgstr ""
-
 msgid "You do not have any subscriptions yet"
 msgstr ""
 
diff --git a/spec/features/triggers_spec.rb b/spec/features/triggers_spec.rb
index 7c44680e9f7..f76cecbbc40 100644
--- a/spec/features/triggers_spec.rb
+++ b/spec/features/triggers_spec.rb
@@ -81,29 +81,6 @@ describe 'Triggers', :js do
     end
   end
 
-  describe 'trigger "Take ownership" workflow' do
-    before do
-      create(:ci_trigger, owner: user2, project: @project, description: trigger_title)
-      visit project_settings_ci_cd_path(@project)
-    end
-
-    it 'button "Take ownership" has correct alert' do
-      expected_alert = 'By taking ownership you will bind this trigger to your user account. With this the trigger will have access to all your projects as if it was you. Are you sure?'
-      expect(page.find('a.btn-trigger-take-ownership')['data-confirm']).to eq expected_alert
-    end
-
-    it 'take trigger ownership' do
-      # See if "Take ownership" on trigger works post trigger creation
-      page.accept_confirm do
-        first(:link, "Take ownership").send_keys(:return)
-      end
-
-      expect(page.find('.flash-notice')).to have_content 'Trigger was re-assigned.'
-      expect(page.find('.triggers-list')).to have_content trigger_title
-      expect(page.find('.triggers-list .trigger-owner')).to have_content user.name
-    end
-  end
-
   describe 'trigger "Revoke" workflow' do
     before do
       create(:ci_trigger, owner: user2, project: @project, description: trigger_title)
diff --git a/spec/requests/api/triggers_spec.rb b/spec/requests/api/triggers_spec.rb
index f0f01e97f1d..8ea3d16a41f 100644
--- a/spec/requests/api/triggers_spec.rb
+++ b/spec/requests/api/triggers_spec.rb
@@ -270,34 +270,6 @@ describe API::Triggers do
     end
   end
 
-  describe 'POST /projects/:id/triggers/:trigger_id/take_ownership' do
-    context 'authenticated user with valid permissions' do
-      it 'updates owner' do
-        post api("/projects/#{project.id}/triggers/#{trigger.id}/take_ownership", user)
-
-        expect(response).to have_gitlab_http_status(200)
-        expect(json_response).to include('owner')
-        expect(trigger.reload.owner).to eq(user)
-      end
-    end
-
-    context 'authenticated user with invalid permissions' do
-      it 'does not update owner' do
-        post api("/projects/#{project.id}/triggers/#{trigger.id}/take_ownership", user2)
-
-        expect(response).to have_gitlab_http_status(403)
-      end
-    end
-
-    context 'unauthenticated user' do
-      it 'does not update owner' do
-        post api("/projects/#{project.id}/triggers/#{trigger.id}/take_ownership")
-
-        expect(response).to have_gitlab_http_status(401)
-      end
-    end
-  end
-
   describe 'DELETE /projects/:id/triggers/:trigger_id' do
     context 'authenticated user with valid permissions' do
       it 'deletes trigger' do
-- 
cgit v1.2.1


From 104d49d5e451ee3be2b9eec5d47cb7cb8508b877 Mon Sep 17 00:00:00 2001
From: GitLab Release Tools Bot <robert+release-tools@gitlab.com>
Date: Fri, 26 Jul 2019 13:41:08 +0000
Subject: Merge branch 'security-fix-badges-leaked-to-unauthorized-users' into
 'master'

Don't display badges when builds are restricted

Closes #2864

See merge request gitlab/gitlabhq!3175
---
 app/controllers/projects/badges_controller.rb      |   3 +-
 ...ity-fix-badges-leaked-to-unauthorized-users.yml |   5 +
 .../controllers/projects/badges_controller_spec.rb | 124 ++++++++++++++++-----
 3 files changed, 101 insertions(+), 31 deletions(-)
 create mode 100644 changelogs/unreleased/security-fix-badges-leaked-to-unauthorized-users.yml

diff --git a/app/controllers/projects/badges_controller.rb b/app/controllers/projects/badges_controller.rb
index 09a384e89ab..66b51b17790 100644
--- a/app/controllers/projects/badges_controller.rb
+++ b/app/controllers/projects/badges_controller.rb
@@ -3,7 +3,8 @@
 class Projects::BadgesController < Projects::ApplicationController
   layout 'project_settings'
   before_action :authorize_admin_project!, only: [:index]
-  before_action :no_cache_headers, except: [:index]
+  before_action :no_cache_headers, only: [:pipeline, :coverage]
+  before_action :authorize_read_build!, only: [:pipeline, :coverage]
 
   def pipeline
     pipeline_status = Gitlab::Badge::Pipeline::Status
diff --git a/changelogs/unreleased/security-fix-badges-leaked-to-unauthorized-users.yml b/changelogs/unreleased/security-fix-badges-leaked-to-unauthorized-users.yml
new file mode 100644
index 00000000000..9526f3c559f
--- /dev/null
+++ b/changelogs/unreleased/security-fix-badges-leaked-to-unauthorized-users.yml
@@ -0,0 +1,5 @@
+---
+title: Show badges if pipelines are public otherwise default to project permissions.
+erge_request:
+author:
+type: security
diff --git a/spec/controllers/projects/badges_controller_spec.rb b/spec/controllers/projects/badges_controller_spec.rb
index 5ec8d8d41d7..4ae29ba7f54 100644
--- a/spec/controllers/projects/badges_controller_spec.rb
+++ b/spec/controllers/projects/badges_controller_spec.rb
@@ -7,51 +7,115 @@ describe Projects::BadgesController do
   let!(:pipeline) { create(:ci_empty_pipeline) }
   let(:user) { create(:user) }
 
-  before do
-    project.add_maintainer(user)
-    sign_in(user)
-  end
+  shared_examples 'a badge resource' do |badge_type|
+    context 'when pipelines are public' do
+      before do
+        project.update!(public_builds: true)
+      end
+
+      context 'when project is public' do
+        before do
+          project.update!(visibility_level: Gitlab::VisibilityLevel::PUBLIC)
+        end
+
+        it "returns the #{badge_type} badge to unauthenticated users" do
+          get_badge(badge_type)
+
+          expect(response).to have_gitlab_http_status(:ok)
+        end
+      end
+
+      context 'when project is restricted' do
+        before do
+          project.update!(visibility_level: Gitlab::VisibilityLevel::INTERNAL)
+          project.add_guest(user)
+          sign_in(user)
+        end
+
+        it "returns the #{badge_type} badge to guest users" do
+          get_badge(badge_type)
+
+          expect(response).to have_gitlab_http_status(:ok)
+        end
+      end
+    end
 
-  it 'requests the pipeline badge successfully' do
-    get_badge(:pipeline)
+    context 'format' do
+      before do
+        project.add_maintainer(user)
+        sign_in(user)
+      end
 
-    expect(response).to have_gitlab_http_status(:ok)
-  end
+      it 'renders the `flat` badge layout by default' do
+        get_badge(badge_type)
 
-  it 'requests the coverage badge successfully' do
-    get_badge(:coverage)
+        expect(response).to render_template('projects/badges/badge')
+      end
 
-    expect(response).to have_gitlab_http_status(:ok)
-  end
+      context 'when style param is set to `flat`' do
+        it 'renders the `flat` badge layout' do
+          get_badge(badge_type, 'flat')
 
-  it 'renders the `flat` badge layout by default' do
-    get_badge(:coverage)
+          expect(response).to render_template('projects/badges/badge')
+        end
+      end
 
-    expect(response).to render_template('projects/badges/badge')
-  end
+      context 'when style param is set to an invalid type' do
+        it 'renders the `flat` (default) badge layout' do
+          get_badge(badge_type, 'xxx')
+
+          expect(response).to render_template('projects/badges/badge')
+        end
+      end
 
-  context 'when style param is set to `flat`' do
-    it 'renders the `flat` badge layout' do
-      get_badge(:coverage, 'flat')
+      context 'when style param is set to `flat-square`' do
+        it 'renders the `flat-square` badge layout' do
+          get_badge(badge_type, 'flat-square')
 
-      expect(response).to render_template('projects/badges/badge')
+          expect(response).to render_template('projects/badges/badge_flat-square')
+        end
+      end
     end
-  end
 
-  context 'when style param is set to an invalid type' do
-    it 'renders the `flat` (default) badge layout' do
-      get_badge(:coverage, 'xxx')
+    context 'when pipelines are not public' do
+      before do
+        project.update!(public_builds: false)
+      end
 
-      expect(response).to render_template('projects/badges/badge')
+      context 'when project is public' do
+        before do
+          project.update!(visibility_level: Gitlab::VisibilityLevel::PUBLIC)
+        end
+
+        it 'returns 404 to unauthenticated users' do
+          get_badge(badge_type)
+
+          expect(response).to have_gitlab_http_status(:not_found)
+        end
+      end
+
+      context 'when project is restricted to the user' do
+        before do
+          project.update!(visibility_level: Gitlab::VisibilityLevel::INTERNAL)
+          project.add_guest(user)
+          sign_in(user)
+        end
+
+        it 'defaults to project permissions' do
+          get_badge(:coverage)
+
+          expect(response).to have_gitlab_http_status(:not_found)
+        end
+      end
     end
   end
 
-  context 'when style param is set to `flat-square`' do
-    it 'renders the `flat-square` badge layout' do
-      get_badge(:coverage, 'flat-square')
+  describe '#pipeline' do
+    it_behaves_like 'a badge resource', :pipeline
+  end
 
-      expect(response).to render_template('projects/badges/badge_flat-square')
-    end
+  describe '#coverage' do
+    it_behaves_like 'a badge resource', :coverage
   end
 
   def get_badge(badge, style = nil)
-- 
cgit v1.2.1


From d9d2466983248809638c8ca1abf5b0f440b3568f Mon Sep 17 00:00:00 2001
From: GitLab Release Tools Bot <robert+release-tools@gitlab.com>
Date: Fri, 26 Jul 2019 13:41:11 +0000
Subject: Merge branch 'security-60143-patch-additional-xss-vector-in-wikis'
 into 'master'

Extract SanitizeNodeLink and apply to WikiLinkFilter

See merge request gitlab/gitlabhq!3143
---
 ...-60143-patch-additional-xss-vector-in-wikis.yml |  5 ++
 lib/banzai/filter/autolink_filter.rb               | 11 +--
 lib/banzai/filter/base_sanitization_filter.rb      | 32 +--------
 lib/banzai/filter/wiki_link_filter.rb              | 15 +++-
 lib/banzai/filter/wiki_link_filter/rewriter.rb     |  8 ---
 lib/gitlab/utils/sanitize_node_link.rb             | 61 +++++++++++++++++
 spec/lib/banzai/filter/wiki_link_filter_spec.rb    | 42 ------------
 spec/lib/banzai/pipeline/wiki_pipeline_spec.rb     | 79 ++++++++++++++++++++++
 spec/lib/gitlab/utils/sanitize_node_link_spec.rb   | 72 ++++++++++++++++++++
 9 files changed, 233 insertions(+), 92 deletions(-)
 create mode 100644 changelogs/unreleased/security-60143-patch-additional-xss-vector-in-wikis.yml
 create mode 100644 lib/gitlab/utils/sanitize_node_link.rb
 create mode 100644 spec/lib/gitlab/utils/sanitize_node_link_spec.rb

diff --git a/changelogs/unreleased/security-60143-patch-additional-xss-vector-in-wikis.yml b/changelogs/unreleased/security-60143-patch-additional-xss-vector-in-wikis.yml
new file mode 100644
index 00000000000..a8a26d5fc56
--- /dev/null
+++ b/changelogs/unreleased/security-60143-patch-additional-xss-vector-in-wikis.yml
@@ -0,0 +1,5 @@
+---
+title: Patch XSS issue in wiki links
+merge_request:
+author:
+type: security
diff --git a/lib/banzai/filter/autolink_filter.rb b/lib/banzai/filter/autolink_filter.rb
index 56214043d87..5f2cbc24c60 100644
--- a/lib/banzai/filter/autolink_filter.rb
+++ b/lib/banzai/filter/autolink_filter.rb
@@ -18,6 +18,7 @@ module Banzai
     #
     class AutolinkFilter < HTML::Pipeline::Filter
       include ActionView::Helpers::TagHelper
+      include Gitlab::Utils::SanitizeNodeLink
 
       # Pattern to match text that should be autolinked.
       #
@@ -72,19 +73,11 @@ module Banzai
 
       private
 
-      # Return true if any of the UNSAFE_PROTOCOLS strings are included in the URI scheme
-      def contains_unsafe?(scheme)
-        return false unless scheme
-
-        scheme = scheme.strip.downcase
-        Banzai::Filter::SanitizationFilter::UNSAFE_PROTOCOLS.any? { |protocol| scheme.include?(protocol) }
-      end
-
       def autolink_match(match)
         # start by stripping out dangerous links
         begin
           uri = Addressable::URI.parse(match)
-          return match if contains_unsafe?(uri.scheme)
+          return match unless safe_protocol?(uri.scheme)
         rescue Addressable::URI::InvalidURIError
           return match
         end
diff --git a/lib/banzai/filter/base_sanitization_filter.rb b/lib/banzai/filter/base_sanitization_filter.rb
index 420e92cb1e8..2dabca3552d 100644
--- a/lib/banzai/filter/base_sanitization_filter.rb
+++ b/lib/banzai/filter/base_sanitization_filter.rb
@@ -11,6 +11,7 @@ module Banzai
     # Extends HTML::Pipeline::SanitizationFilter with common rules.
     class BaseSanitizationFilter < HTML::Pipeline::SanitizationFilter
       include Gitlab::Utils::StrongMemoize
+      extend Gitlab::Utils::SanitizeNodeLink
 
       UNSAFE_PROTOCOLS = %w(data javascript vbscript).freeze
 
@@ -40,7 +41,7 @@ module Banzai
           # Allow any protocol in `a` elements
           # and then remove links with unsafe protocols
           whitelist[:protocols].delete('a')
-          whitelist[:transformers].push(self.class.remove_unsafe_links)
+          whitelist[:transformers].push(self.class.method(:remove_unsafe_links))
 
           # Remove `rel` attribute from `a` elements
           whitelist[:transformers].push(self.class.remove_rel)
@@ -54,35 +55,6 @@ module Banzai
       end
 
       class << self
-        def remove_unsafe_links
-          lambda do |env|
-            node = env[:node]
-
-            return unless node.name == 'a'
-            return unless node.has_attribute?('href')
-
-            begin
-              node['href'] = node['href'].strip
-              uri = Addressable::URI.parse(node['href'])
-
-              return unless uri.scheme
-
-              # Remove all invalid scheme characters before checking against the
-              # list of unsafe protocols.
-              #
-              # See https://tools.ietf.org/html/rfc3986#section-3.1
-              scheme = uri.scheme
-                           .strip
-                           .downcase
-                           .gsub(/[^A-Za-z0-9\+\.\-]+/, '')
-
-              node.remove_attribute('href') if UNSAFE_PROTOCOLS.include?(scheme)
-            rescue Addressable::URI::InvalidURIError
-              node.remove_attribute('href')
-            end
-          end
-        end
-
         def remove_rel
           lambda do |env|
             if env[:node_name] == 'a'
diff --git a/lib/banzai/filter/wiki_link_filter.rb b/lib/banzai/filter/wiki_link_filter.rb
index 1728a442533..18947679b69 100644
--- a/lib/banzai/filter/wiki_link_filter.rb
+++ b/lib/banzai/filter/wiki_link_filter.rb
@@ -8,15 +8,19 @@ module Banzai
     # Context options:
     #   :project_wiki
     class WikiLinkFilter < HTML::Pipeline::Filter
+      include Gitlab::Utils::SanitizeNodeLink
+
       def call
         return doc unless project_wiki?
 
-        doc.search('a:not(.gfm)').each { |el| process_link_attr(el.attribute('href')) }
-        doc.search('video').each { |el| process_link_attr(el.attribute('src')) }
+        doc.search('a:not(.gfm)').each { |el| process_link(el.attribute('href'), el) }
+
+        doc.search('video').each { |el| process_link(el.attribute('src'), el) }
+
         doc.search('img').each do |el|
           attr = el.attribute('data-src') || el.attribute('src')
 
-          process_link_attr(attr)
+          process_link(attr, el)
         end
 
         doc
@@ -24,6 +28,11 @@ module Banzai
 
       protected
 
+      def process_link(link_attr, node)
+        process_link_attr(link_attr)
+        remove_unsafe_links({ node: node }, remove_invalid_links: false)
+      end
+
       def project_wiki?
         !context[:project_wiki].nil?
       end
diff --git a/lib/banzai/filter/wiki_link_filter/rewriter.rb b/lib/banzai/filter/wiki_link_filter/rewriter.rb
index 77b5053f38c..f4cc8beeb52 100644
--- a/lib/banzai/filter/wiki_link_filter/rewriter.rb
+++ b/lib/banzai/filter/wiki_link_filter/rewriter.rb
@@ -4,8 +4,6 @@ module Banzai
   module Filter
     class WikiLinkFilter < HTML::Pipeline::Filter
       class Rewriter
-        UNSAFE_SLUG_REGEXES = [/\Ajavascript:/i].freeze
-
         def initialize(link_string, wiki:, slug:)
           @uri = Addressable::URI.parse(link_string)
           @wiki_base_path = wiki && wiki.wiki_base_path
@@ -37,8 +35,6 @@ module Banzai
 
         # Of the form `./link`, `../link`, or similar
         def apply_hierarchical_link_rules!
-          return if slug_considered_unsafe?
-
           @uri = Addressable::URI.join(@slug, @uri) if @uri.to_s[0] == '.'
         end
 
@@ -58,10 +54,6 @@ module Banzai
         def repository_upload?
           @uri.relative? && @uri.path.starts_with?(Wikis::CreateAttachmentService::ATTACHMENT_PATH)
         end
-
-        def slug_considered_unsafe?
-          UNSAFE_SLUG_REGEXES.any? { |r| r.match?(@slug) }
-        end
       end
     end
   end
diff --git a/lib/gitlab/utils/sanitize_node_link.rb b/lib/gitlab/utils/sanitize_node_link.rb
new file mode 100644
index 00000000000..620d71a7814
--- /dev/null
+++ b/lib/gitlab/utils/sanitize_node_link.rb
@@ -0,0 +1,61 @@
+# frozen_string_literal: true
+
+require_dependency 'gitlab/utils'
+
+module Gitlab
+  module Utils
+    module SanitizeNodeLink
+      UNSAFE_PROTOCOLS  = %w(data javascript vbscript).freeze
+      ATTRS_TO_SANITIZE = %w(href src data-src).freeze
+
+      def remove_unsafe_links(env, remove_invalid_links: true)
+        node = env[:node]
+
+        sanitize_node(node: node, remove_invalid_links: remove_invalid_links)
+
+        # HTML entities such as <video></video> have scannable attrs in
+        #   children elements, which also need to be sanitized.
+        #
+        node.children.each do |child_node|
+          sanitize_node(node: child_node, remove_invalid_links: remove_invalid_links)
+        end
+      end
+
+      # Remove all invalid scheme characters before checking against the
+      # list of unsafe protocols.
+      #
+      # See https://tools.ietf.org/html/rfc3986#section-3.1
+      #
+      def safe_protocol?(scheme)
+        return false unless scheme
+
+        scheme = scheme
+          .strip
+          .downcase
+          .gsub(/[^A-Za-z\+\.\-]+/, '')
+
+        UNSAFE_PROTOCOLS.none?(scheme)
+      end
+
+      private
+
+      def sanitize_node(node:, remove_invalid_links: true)
+        ATTRS_TO_SANITIZE.each do |attr|
+          next unless node.has_attribute?(attr)
+
+          begin
+            node[attr] = node[attr].strip
+            uri = Addressable::URI.parse(node[attr])
+
+            next unless uri.scheme
+            next if safe_protocol?(uri.scheme)
+
+            node.remove_attribute(attr)
+          rescue Addressable::URI::InvalidURIError
+            node.remove_attribute(attr) if remove_invalid_links
+          end
+        end
+      end
+    end
+  end
+end
diff --git a/spec/lib/banzai/filter/wiki_link_filter_spec.rb b/spec/lib/banzai/filter/wiki_link_filter_spec.rb
index cce1cd0b284..b9059b85fdc 100644
--- a/spec/lib/banzai/filter/wiki_link_filter_spec.rb
+++ b/spec/lib/banzai/filter/wiki_link_filter_spec.rb
@@ -70,47 +70,5 @@ describe Banzai::Filter::WikiLinkFilter do
         expect(filtered_link.attribute('href').value).to eq(invalid_link)
       end
     end
-
-    context "when the slug is deemed unsafe or invalid" do
-      let(:link) { "alert(1);" }
-
-      invalid_slugs = [
-        "javascript:",
-        "JaVaScRiPt:",
-        "\u0001java\u0003script:",
-        "javascript    :",
-        "javascript:    ",
-        "javascript    :   ",
-        ":javascript:",
-        "javascript&#58;",
-        "javascript&#0058;",
-        "javascript&#x3A;",
-        "javascript&#x003A;",
-        "java\0script:",
-        " &#14;  javascript:"
-        ]
-
-      invalid_slugs.each do |slug|
-        context "with the slug #{slug}" do
-          it "doesn't rewrite a (.) relative link" do
-            filtered_link = filter(
-              "<a href='.#{link}'>Link</a>",
-              project_wiki: wiki,
-              page_slug: slug).children[0]
-
-            expect(filtered_link.attribute('href').value).not_to include(slug)
-          end
-
-          it "doesn't rewrite a (..) relative link" do
-            filtered_link = filter(
-              "<a href='..#{link}'>Link</a>",
-              project_wiki: wiki,
-              page_slug: slug).children[0]
-
-            expect(filtered_link.attribute('href').value).not_to include(slug)
-          end
-        end
-      end
-    end
   end
 end
diff --git a/spec/lib/banzai/pipeline/wiki_pipeline_spec.rb b/spec/lib/banzai/pipeline/wiki_pipeline_spec.rb
index 64ca3ec345d..95c0c9f9a17 100644
--- a/spec/lib/banzai/pipeline/wiki_pipeline_spec.rb
+++ b/spec/lib/banzai/pipeline/wiki_pipeline_spec.rb
@@ -177,6 +177,85 @@ describe Banzai::Pipeline::WikiPipeline do
         end
       end
     end
+
+    describe "checking slug validity when assembling links" do
+      context "with a valid slug" do
+        let(:valid_slug) { "http://example.com" }
+
+        it "includes the slug in a (.) relative link" do
+          output = described_class.to_html(
+            "[Link](./alert(1);)",
+            project: project,
+            project_wiki: project_wiki,
+            page_slug: valid_slug
+          )
+
+          expect(output).to include(valid_slug)
+        end
+
+        it "includeds the slug in a (..) relative link" do
+          output = described_class.to_html(
+            "[Link](../alert(1);)",
+            project: project,
+            project_wiki: project_wiki,
+            page_slug: valid_slug
+          )
+
+          expect(output).to include(valid_slug)
+        end
+      end
+
+      context "when the slug is deemed unsafe or invalid" do
+        invalid_slugs = [
+          "javascript:",
+          "JaVaScRiPt:",
+          "\u0001java\u0003script:",
+          "javascript    :",
+          "javascript:    ",
+          "javascript    :   ",
+          ":javascript:",
+          "javascript&#58;",
+          "javascript&#0058;",
+          "javascript&#x3A;",
+          "javascript&#x003A;",
+          "java\0script:",
+          " &#14;  javascript:"
+        ]
+
+        invalid_js_links = [
+          "alert(1);",
+          "alert(document.location);"
+        ]
+
+        invalid_slugs.each do |slug|
+          context "with the invalid slug #{slug}" do
+            invalid_js_links.each do |link|
+              it "doesn't include a prohibited slug in a (.) relative link '#{link}'" do
+                output = described_class.to_html(
+                  "[Link](./#{link})",
+                  project: project,
+                  project_wiki: project_wiki,
+                  page_slug: slug
+                )
+
+                expect(output).not_to include(slug)
+              end
+
+              it "doesn't include a prohibited slug in a (..) relative link '#{link}'" do
+                output = described_class.to_html(
+                  "[Link](../#{link})",
+                  project: project,
+                  project_wiki: project_wiki,
+                  page_slug: slug
+                )
+
+                expect(output).not_to include(slug)
+              end
+            end
+          end
+        end
+      end
+    end
   end
 
   describe 'videos' do
diff --git a/spec/lib/gitlab/utils/sanitize_node_link_spec.rb b/spec/lib/gitlab/utils/sanitize_node_link_spec.rb
new file mode 100644
index 00000000000..064c2707d06
--- /dev/null
+++ b/spec/lib/gitlab/utils/sanitize_node_link_spec.rb
@@ -0,0 +1,72 @@
+require 'spec_helper'
+
+describe Gitlab::Utils::SanitizeNodeLink do
+  let(:klass) do
+    struct = Struct.new(:value)
+    struct.include(described_class)
+
+    struct
+  end
+
+  subject(:object) { klass.new(:value) }
+
+  invalid_schemes = [
+    "javascript:",
+    "JaVaScRiPt:",
+    "\u0001java\u0003script:",
+    "javascript    :",
+    "javascript:    ",
+    "javascript    :   ",
+    ":javascript:",
+    "javascript&#58;",
+    "javascript&#0058;",
+    " &#14;  javascript:"
+  ]
+
+  invalid_schemes.each do |scheme|
+    context "with the scheme: #{scheme}" do
+      describe "#remove_unsafe_links" do
+        tags = {
+          a: {
+            doc: HTML::Pipeline.parse("<a href='#{scheme}alert(1);'>foo</a>"),
+            attr: "href",
+            node_to_check: -> (doc) { doc.children.first }
+          },
+          img: {
+            doc: HTML::Pipeline.parse("<img src='#{scheme}alert(1);'>"),
+            attr: "src",
+            node_to_check: -> (doc) { doc.children.first }
+          },
+          video: {
+            doc: HTML::Pipeline.parse("<video><source src='#{scheme}alert(1);'></video>"),
+            attr: "src",
+            node_to_check: -> (doc) { doc.children.first.children.filter("source").first }
+          }
+        }
+
+        tags.each do |tag, opts|
+          context "<#{tag}> tags" do
+            it "removes the unsafe link" do
+              node = opts[:node_to_check].call(opts[:doc])
+
+              expect { object.remove_unsafe_links({ node: node }, remove_invalid_links: true) }
+                .to change { node[opts[:attr]] }
+
+              expect(node[opts[:attr]]).to be_blank
+            end
+          end
+        end
+      end
+
+      describe "#safe_protocol?" do
+        let(:doc) { HTML::Pipeline.parse("<a href='#{scheme}alert(1);'>foo</a>") }
+        let(:node) { doc.children.first }
+        let(:uri) { Addressable::URI.parse(node['href'])}
+
+        it "returns false" do
+          expect(object.safe_protocol?(scheme)).to be_falsy
+        end
+      end
+    end
+  end
+end
-- 
cgit v1.2.1