summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
-rw-r--r--app/assets/javascripts/snippets/components/show.vue13
-rw-r--r--app/assets/javascripts/snippets/mixins/snippets.js1
-rw-r--r--app/graphql/queries/snippet/snippet.query.graphql1
-rw-r--r--app/graphql/queries/snippet/snippet_blob_content.query.graphql1
-rw-r--r--app/graphql/resolvers/snippets/blobs_resolver.rb24
-rw-r--r--app/graphql/types/snippets/blob_connection_type.rb16
-rw-r--r--app/graphql/types/snippets/blob_type.rb2
-rw-r--r--app/models/snippet.rb10
-rw-r--r--config/initializers/action_mailer_hooks.rb1
-rw-r--r--doc/api/graphql/reference/index.md1
-rw-r--r--lib/gitlab/email/hook/validate_addresses_interceptor.rb32
-rw-r--r--locale/gitlab.pot3
-rw-r--r--spec/frontend/snippets/components/show_spec.js19
-rw-r--r--spec/graphql/resolvers/snippets/blobs_resolver_spec.rb13
-rw-r--r--spec/lib/gitlab/email/hook/validate_addresses_interceptor_spec.rb52
-rw-r--r--spec/models/snippet_spec.rb34
16 files changed, 204 insertions, 19 deletions
diff --git a/app/assets/javascripts/snippets/components/show.vue b/app/assets/javascripts/snippets/components/show.vue
index 35d88d5ec8e..ee8b00c1f5d 100644
--- a/app/assets/javascripts/snippets/components/show.vue
+++ b/app/assets/javascripts/snippets/components/show.vue
@@ -1,5 +1,5 @@
<script>
-import { GlLoadingIcon } from '@gitlab/ui';
+import { GlAlert, GlLoadingIcon } from '@gitlab/ui';
import eventHub from '~/blob/components/eventhub';
import {
SNIPPET_MARK_VIEW_APP_START,
@@ -23,6 +23,7 @@ export default {
EmbedDropdown,
SnippetHeader,
SnippetTitle,
+ GlAlert,
GlLoadingIcon,
SnippetBlob,
CloneDropdownButton,
@@ -35,6 +36,9 @@ export default {
canBeCloned() {
return Boolean(this.snippet.sshUrlToRepo || this.snippet.httpUrlToRepo);
},
+ hasUnretrievableBlobs() {
+ return this.snippet.hasUnretrievableBlobs;
+ },
},
beforeCreate() {
performanceMarkAndMeasure({ mark: SNIPPET_MARK_VIEW_APP_START });
@@ -66,6 +70,13 @@ export default {
data-qa-selector="clone_button"
/>
</div>
+ <gl-alert v-if="hasUnretrievableBlobs" variant="danger" class="gl-mb-3" :dismissible="false">
+ {{
+ __(
+ 'WARNING: This snippet contains hidden files which might be used to mask malicious behavior. Exercise caution if cloning and executing code from this snippet.',
+ )
+ }}
+ </gl-alert>
<snippet-blob
v-for="blob in blobs"
:key="blob.path"
diff --git a/app/assets/javascripts/snippets/mixins/snippets.js b/app/assets/javascripts/snippets/mixins/snippets.js
index b72befef56b..0b3cca4e53a 100644
--- a/app/assets/javascripts/snippets/mixins/snippets.js
+++ b/app/assets/javascripts/snippets/mixins/snippets.js
@@ -17,6 +17,7 @@ export const getSnippetMixin = {
// Set `snippet.blobs` since some child components are coupled to this.
if (!isEmpty(res)) {
+ res.hasUnretrievableBlobs = res.blobs?.hasUnretrievableBlobs || false;
// It's possible for us to not get any blobs in a response.
// In this case, we should default to current blobs.
res.blobs = res.blobs ? res.blobs.nodes : blobsDefault;
diff --git a/app/graphql/queries/snippet/snippet.query.graphql b/app/graphql/queries/snippet/snippet.query.graphql
index 24b268ec853..5c0c7ebaa1b 100644
--- a/app/graphql/queries/snippet/snippet.query.graphql
+++ b/app/graphql/queries/snippet/snippet.query.graphql
@@ -15,6 +15,7 @@ query GetSnippetQuery($ids: [SnippetID!]) {
sshUrlToRepo
blobs {
__typename
+ hasUnretrievableBlobs
nodes {
__typename
binary
diff --git a/app/graphql/queries/snippet/snippet_blob_content.query.graphql b/app/graphql/queries/snippet/snippet_blob_content.query.graphql
index 005f42ff726..4459a5e4316 100644
--- a/app/graphql/queries/snippet/snippet_blob_content.query.graphql
+++ b/app/graphql/queries/snippet/snippet_blob_content.query.graphql
@@ -12,6 +12,7 @@ query SnippetBlobContent($ids: [ID!], $rich: Boolean!, $paths: [String!]) {
richData @include(if: $rich)
plainData @skip(if: $rich)
}
+ hasUnretrievableBlobs
}
}
}
diff --git a/app/graphql/resolvers/snippets/blobs_resolver.rb b/app/graphql/resolvers/snippets/blobs_resolver.rb
index cbbc65d7263..29716ce1394 100644
--- a/app/graphql/resolvers/snippets/blobs_resolver.rb
+++ b/app/graphql/resolvers/snippets/blobs_resolver.rb
@@ -19,18 +19,18 @@ module Resolvers
def resolve(paths: [])
return [snippet.blob] if snippet.empty_repo?
- if paths.empty?
- snippet.blobs
- else
- snippet.repository.blobs_at(transformed_blob_paths(paths))
- end
- end
-
- private
-
- def transformed_blob_paths(paths)
- ref = snippet.default_branch
- paths.map { |path| [ref, path] }
+ paths = snippet.all_files if paths.empty?
+ blobs = snippet.blobs(paths)
+
+ # TODO: Some blobs, e.g. those with non-utf8 filenames, are returned as nil from the
+ # repository. We need to provide a flag to notify the user of this until we come up with a
+ # way to retrieve and display these blobs. We will be exploring a more holistic solution for
+ # this general problem of making all blobs retrievable as part
+ # of https://gitlab.com/gitlab-org/gitlab/-/issues/323082, at which point this attribute may
+ # be removed.
+ context[:unretrievable_blobs?] = blobs.size < paths.size
+
+ blobs
end
end
end
diff --git a/app/graphql/types/snippets/blob_connection_type.rb b/app/graphql/types/snippets/blob_connection_type.rb
new file mode 100644
index 00000000000..15d26af7374
--- /dev/null
+++ b/app/graphql/types/snippets/blob_connection_type.rb
@@ -0,0 +1,16 @@
+# frozen_string_literal: true
+
+module Types
+ module Snippets
+ # rubocop: disable Graphql/AuthorizeTypes
+ class BlobConnectionType < GraphQL::Types::Relay::BaseConnection
+ field :has_unretrievable_blobs, GraphQL::Types::Boolean, null: false,
+ description: 'Indicates if the snippet has unretrievable blobs.',
+ resolver_method: :unretrievable_blobs?
+
+ def unretrievable_blobs?
+ !!context[:unretrievable_blobs?]
+ end
+ end
+ end
+end
diff --git a/app/graphql/types/snippets/blob_type.rb b/app/graphql/types/snippets/blob_type.rb
index 2b9b76a6194..80702c71f63 100644
--- a/app/graphql/types/snippets/blob_type.rb
+++ b/app/graphql/types/snippets/blob_type.rb
@@ -8,6 +8,8 @@ module Types
description 'Represents the snippet blob'
present_using SnippetBlobPresenter
+ connection_type_class(Types::Snippets::BlobConnectionType)
+
field :rich_data, GraphQL::Types::String,
description: 'Blob highlighted data.',
null: true
diff --git a/app/models/snippet.rb b/app/models/snippet.rb
index 6a8123b3c08..b04fca64c87 100644
--- a/app/models/snippet.rb
+++ b/app/models/snippet.rb
@@ -237,15 +237,19 @@ class Snippet < ApplicationRecord
end
end
+ def all_files
+ list_files(default_branch)
+ end
+
def blob
@blob ||= Blob.decorate(SnippetBlob.new(self), self)
end
- def blobs
+ def blobs(paths = [])
return [] unless repository_exists?
- files = list_files(default_branch)
- items = files.map { |file| [default_branch, file] }
+ paths = all_files if paths.empty?
+ items = paths.map { |path| [default_branch, path] }
repository.blobs_at(items).compact
end
diff --git a/config/initializers/action_mailer_hooks.rb b/config/initializers/action_mailer_hooks.rb
index 46d5e387d9d..fb09ed34bf6 100644
--- a/config/initializers/action_mailer_hooks.rb
+++ b/config/initializers/action_mailer_hooks.rb
@@ -8,6 +8,7 @@ end
ActionMailer::Base.register_interceptors(
::Gitlab::Email::Hook::AdditionalHeadersInterceptor,
::Gitlab::Email::Hook::EmailTemplateInterceptor,
+ ::Gitlab::Email::Hook::ValidateAddressesInterceptor,
::Gitlab::Email::Hook::DeliveryMetricsObserver
)
diff --git a/doc/api/graphql/reference/index.md b/doc/api/graphql/reference/index.md
index 18c99b7d151..49c0361123e 100644
--- a/doc/api/graphql/reference/index.md
+++ b/doc/api/graphql/reference/index.md
@@ -7841,6 +7841,7 @@ The connection type for [`SnippetBlob`](#snippetblob).
| Name | Type | Description |
| ---- | ---- | ----------- |
| <a id="snippetblobconnectionedges"></a>`edges` | [`[SnippetBlobEdge]`](#snippetblobedge) | A list of edges. |
+| <a id="snippetblobconnectionhasunretrievableblobs"></a>`hasUnretrievableBlobs` | [`Boolean!`](#boolean) | Indicates if the snippet has unretrievable blobs. |
| <a id="snippetblobconnectionnodes"></a>`nodes` | [`[SnippetBlob]`](#snippetblob) | A list of nodes. |
| <a id="snippetblobconnectionpageinfo"></a>`pageInfo` | [`PageInfo!`](#pageinfo) | Information to aid in pagination. |
diff --git a/lib/gitlab/email/hook/validate_addresses_interceptor.rb b/lib/gitlab/email/hook/validate_addresses_interceptor.rb
new file mode 100644
index 00000000000..e63f047e63d
--- /dev/null
+++ b/lib/gitlab/email/hook/validate_addresses_interceptor.rb
@@ -0,0 +1,32 @@
+# frozen_string_literal: true
+
+module Gitlab
+ module Email
+ module Hook
+ # Check for unsafe characters in the envelope-from and -to addresses.
+ # These are passed directly as arguments to sendmail and are liable to shell injection attacks:
+ # https://github.com/mikel/mail/blob/2.7.1/lib/mail/network/delivery_methods/sendmail.rb#L53-L58
+ class ValidateAddressesInterceptor
+ UNSAFE_CHARACTERS = /(\\|[^[:print:]])/.freeze
+
+ def self.delivering_email(message)
+ addresses = Array(message.smtp_envelope_from) + Array(message.smtp_envelope_to)
+
+ addresses.each do |address|
+ next unless address.match?(UNSAFE_CHARACTERS)
+
+ Gitlab::AuthLogger.info(
+ message: 'Skipping email with unsafe characters in address',
+ address: address,
+ subject: message.subject
+ )
+
+ message.perform_deliveries = false
+
+ break
+ end
+ end
+ end
+ end
+ end
+end
diff --git a/locale/gitlab.pot b/locale/gitlab.pot
index 37f8a3fd7b4..6cdc5ba18e2 100644
--- a/locale/gitlab.pot
+++ b/locale/gitlab.pot
@@ -40768,6 +40768,9 @@ msgstr ""
msgid "WARNING:"
msgstr ""
+msgid "WARNING: This snippet contains hidden files which might be used to mask malicious behavior. Exercise caution if cloning and executing code from this snippet."
+msgstr ""
+
msgid "Wait for the file to load to copy its contents"
msgstr ""
diff --git a/spec/frontend/snippets/components/show_spec.js b/spec/frontend/snippets/components/show_spec.js
index af61f4ea54f..c73bf8f80a2 100644
--- a/spec/frontend/snippets/components/show_spec.js
+++ b/spec/frontend/snippets/components/show_spec.js
@@ -1,4 +1,4 @@
-import { GlLoadingIcon } from '@gitlab/ui';
+import { GlLoadingIcon, GlAlert } from '@gitlab/ui';
import { shallowMount } from '@vue/test-utils';
import { Blob, BinaryBlob } from 'jest/blob/components/mock_data';
import EmbedDropdown from '~/snippets/components/embed_dropdown.vue';
@@ -106,6 +106,23 @@ describe('Snippet view app', () => {
});
});
+ describe('hasUnretrievableBlobs alert rendering', () => {
+ it.each`
+ hasUnretrievableBlobs | condition | isRendered
+ ${false} | ${'not render'} | ${false}
+ ${true} | ${'render'} | ${true}
+ `('does $condition gl-alert by default', ({ hasUnretrievableBlobs, isRendered }) => {
+ createComponent({
+ data: {
+ snippet: {
+ hasUnretrievableBlobs,
+ },
+ },
+ });
+ expect(wrapper.findComponent(GlAlert).exists()).toBe(isRendered);
+ });
+ });
+
describe('Clone button rendering', () => {
it.each`
httpUrlToRepo | sshUrlToRepo | shouldRender | isRendered
diff --git a/spec/graphql/resolvers/snippets/blobs_resolver_spec.rb b/spec/graphql/resolvers/snippets/blobs_resolver_spec.rb
index ebe286769cf..b5db7ac5748 100644
--- a/spec/graphql/resolvers/snippets/blobs_resolver_spec.rb
+++ b/spec/graphql/resolvers/snippets/blobs_resolver_spec.rb
@@ -13,11 +13,14 @@ RSpec.describe Resolvers::Snippets::BlobsResolver do
let_it_be(:current_user) { create(:user) }
let_it_be(:snippet) { create(:personal_snippet, :private, :repository, author: current_user) }
+ let(:query_context) { {} }
+
context 'when user is not authorized' do
let(:other_user) { create(:user) }
it 'redacts the field' do
expect(resolve_blobs(snippet, user: other_user)).to be_nil
+ expect(query_context[:unretrievable_blobs?]).to eq(false)
end
end
@@ -28,6 +31,7 @@ RSpec.describe Resolvers::Snippets::BlobsResolver do
expect(result).to match_array(snippet.list_files.map do |file|
have_attributes(path: file)
end)
+ expect(query_context[:unretrievable_blobs?]).to eq(false)
end
end
@@ -37,12 +41,14 @@ RSpec.describe Resolvers::Snippets::BlobsResolver do
path = 'CHANGELOG'
expect(resolve_blobs(snippet, paths: [path])).to contain_exactly(have_attributes(path: path))
+ expect(query_context[:unretrievable_blobs?]).to eq(false)
end
end
context 'the argument does not match anything' do
it 'returns an empty result' do
expect(resolve_blobs(snippet, paths: ['does not exist'])).to be_empty
+ expect(query_context[:unretrievable_blobs?]).to eq(true)
end
end
@@ -53,12 +59,15 @@ RSpec.describe Resolvers::Snippets::BlobsResolver do
expect(resolve_blobs(snippet, paths: paths)).to match_array(paths.map do |file|
have_attributes(path: file)
end)
+ expect(query_context[:unretrievable_blobs?]).to eq(false)
end
end
end
end
- def resolve_blobs(snippet, user: current_user, paths: [], args: { paths: paths })
- resolve(described_class, args: args, ctx: { current_user: user }, obj: snippet)
+ def resolve_blobs(snippet, user: current_user, paths: [], args: { paths: paths }, has_unretrievable_blobs: false)
+ query_context[:current_user] = user
+ query_context[:unretrievable_blobs?] = has_unretrievable_blobs
+ resolve(described_class, args: args, ctx: query_context, obj: snippet)
end
end
diff --git a/spec/lib/gitlab/email/hook/validate_addresses_interceptor_spec.rb b/spec/lib/gitlab/email/hook/validate_addresses_interceptor_spec.rb
new file mode 100644
index 00000000000..a3f0158db40
--- /dev/null
+++ b/spec/lib/gitlab/email/hook/validate_addresses_interceptor_spec.rb
@@ -0,0 +1,52 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe Gitlab::Email::Hook::ValidateAddressesInterceptor do
+ describe 'UNSAFE_CHARACTERS' do
+ subject { described_class::UNSAFE_CHARACTERS }
+
+ it { is_expected.to match('\\') }
+ it { is_expected.to match("\x00") }
+ it { is_expected.to match("\x01") }
+ it { is_expected.not_to match('') }
+ it { is_expected.not_to match('user@example.com') }
+ it { is_expected.not_to match('foo-123+bar_456@example.com') }
+ end
+
+ describe '.delivering_email' do
+ let(:mail) do
+ ActionMailer::Base.mail(to: 'test@mail.com', from: 'info@mail.com', subject: 'title', body: 'hello')
+ end
+
+ let(:unsafe_email) { "evil+\x01$HOME@example.com" }
+
+ it 'sends emails to normal addresses' do
+ expect(Gitlab::AuthLogger).not_to receive(:info)
+ expect { mail.deliver_now }.to change(ActionMailer::Base.deliveries, :count)
+ end
+
+ [:from, :to, :cc, :bcc].each do |header|
+ it "does not send emails if the #{header.inspect} header contains unsafe characters" do
+ mail[header] = unsafe_email
+
+ expect(Gitlab::AuthLogger).to receive(:info).with(
+ message: 'Skipping email with unsafe characters in address',
+ address: unsafe_email,
+ subject: mail.subject
+ )
+
+ expect { mail.deliver_now }.not_to change(ActionMailer::Base.deliveries, :count)
+ end
+ end
+
+ [:reply_to].each do |header|
+ it "sends emails if the #{header.inspect} header contains unsafe characters" do
+ mail[header] = unsafe_email
+
+ expect(Gitlab::AuthLogger).not_to receive(:info)
+ expect { mail.deliver_now }.to change(ActionMailer::Base.deliveries, :count)
+ end
+ end
+ end
+end
diff --git a/spec/models/snippet_spec.rb b/spec/models/snippet_spec.rb
index 5d4a78bb15f..92e4bc7d1a9 100644
--- a/spec/models/snippet_spec.rb
+++ b/spec/models/snippet_spec.rb
@@ -3,6 +3,8 @@
require 'spec_helper'
RSpec.describe Snippet do
+ include FakeBlobHelpers
+
describe 'modules' do
subject { described_class }
@@ -526,6 +528,21 @@ RSpec.describe Snippet do
end
end
+ describe '#all_files' do
+ let(:snippet) { create(:snippet, :repository) }
+ let(:files) { double(:files) }
+
+ subject(:all_files) { snippet.all_files }
+
+ before do
+ allow(snippet.repository).to receive(:ls_files).with(snippet.default_branch).and_return(files)
+ end
+
+ it 'lists files from the repository with the default branch' do
+ expect(all_files).to eq(files)
+ end
+ end
+
describe '#blobs' do
context 'when repository does not exist' do
let(:snippet) { create(:snippet) }
@@ -552,6 +569,23 @@ RSpec.describe Snippet do
end
end
end
+
+ context 'when some blobs are not retrievable from repository' do
+ let(:snippet) { create(:snippet, :repository) }
+ let(:container) { double(:container) }
+ let(:retrievable_filename) { 'retrievable_file'}
+ let(:unretrievable_filename) { 'unretrievable_file'}
+
+ before do
+ allow(snippet).to receive(:list_files).and_return([retrievable_filename, unretrievable_filename])
+ blob = fake_blob(path: retrievable_filename, container: container)
+ allow(snippet.repository).to receive(:blobs_at).and_return([blob, nil])
+ end
+
+ it 'does not include unretrievable blobs' do
+ expect(snippet.blobs.map(&:name)).to contain_exactly(retrievable_filename)
+ end
+ end
end
describe '#to_json' do