Commit f9dc8a61 authored by Chad Woolley's avatar Chad Woolley Committed by GitLab Release Tools Bot

Warn when snippet contains unretrievable files

Merge branch 'security-snippets-warn-unretrievable-files' into 'master'

See merge request gitlab-org/security/gitlab!2187

Changelog: security
parent ef3d007a
<script> <script>
import { GlLoadingIcon } from '@gitlab/ui'; import { GlAlert, GlLoadingIcon } from '@gitlab/ui';
import eventHub from '~/blob/components/eventhub'; import eventHub from '~/blob/components/eventhub';
import { import {
SNIPPET_MARK_VIEW_APP_START, SNIPPET_MARK_VIEW_APP_START,
...@@ -23,6 +23,7 @@ export default { ...@@ -23,6 +23,7 @@ export default {
EmbedDropdown, EmbedDropdown,
SnippetHeader, SnippetHeader,
SnippetTitle, SnippetTitle,
GlAlert,
GlLoadingIcon, GlLoadingIcon,
SnippetBlob, SnippetBlob,
CloneDropdownButton, CloneDropdownButton,
...@@ -35,6 +36,9 @@ export default { ...@@ -35,6 +36,9 @@ export default {
canBeCloned() { canBeCloned() {
return Boolean(this.snippet.sshUrlToRepo || this.snippet.httpUrlToRepo); return Boolean(this.snippet.sshUrlToRepo || this.snippet.httpUrlToRepo);
}, },
hasUnretrievableBlobs() {
return this.snippet.hasUnretrievableBlobs;
},
}, },
beforeCreate() { beforeCreate() {
performanceMarkAndMeasure({ mark: SNIPPET_MARK_VIEW_APP_START }); performanceMarkAndMeasure({ mark: SNIPPET_MARK_VIEW_APP_START });
...@@ -66,6 +70,13 @@ export default { ...@@ -66,6 +70,13 @@ export default {
data-qa-selector="clone_button" data-qa-selector="clone_button"
/> />
</div> </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 <snippet-blob
v-for="blob in blobs" v-for="blob in blobs"
:key="blob.path" :key="blob.path"
......
...@@ -17,6 +17,7 @@ export const getSnippetMixin = { ...@@ -17,6 +17,7 @@ export const getSnippetMixin = {
// Set `snippet.blobs` since some child components are coupled to this. // Set `snippet.blobs` since some child components are coupled to this.
if (!isEmpty(res)) { if (!isEmpty(res)) {
res.hasUnretrievableBlobs = res.blobs?.hasUnretrievableBlobs || false;
// It's possible for us to not get any blobs in a response. // It's possible for us to not get any blobs in a response.
// In this case, we should default to current blobs. // In this case, we should default to current blobs.
res.blobs = res.blobs ? res.blobs.nodes : blobsDefault; res.blobs = res.blobs ? res.blobs.nodes : blobsDefault;
......
...@@ -15,6 +15,7 @@ query GetSnippetQuery($ids: [SnippetID!]) { ...@@ -15,6 +15,7 @@ query GetSnippetQuery($ids: [SnippetID!]) {
sshUrlToRepo sshUrlToRepo
blobs { blobs {
__typename __typename
hasUnretrievableBlobs
nodes { nodes {
__typename __typename
binary binary
......
...@@ -12,6 +12,7 @@ query SnippetBlobContent($ids: [ID!], $rich: Boolean!, $paths: [String!]) { ...@@ -12,6 +12,7 @@ query SnippetBlobContent($ids: [ID!], $rich: Boolean!, $paths: [String!]) {
richData @include(if: $rich) richData @include(if: $rich)
plainData @skip(if: $rich) plainData @skip(if: $rich)
} }
hasUnretrievableBlobs
} }
} }
} }
......
...@@ -19,18 +19,18 @@ module Resolvers ...@@ -19,18 +19,18 @@ module Resolvers
def resolve(paths: []) def resolve(paths: [])
return [snippet.blob] if snippet.empty_repo? return [snippet.blob] if snippet.empty_repo?
if paths.empty? paths = snippet.all_files if paths.empty?
snippet.blobs blobs = snippet.blobs(paths)
else
snippet.repository.blobs_at(transformed_blob_paths(paths)) # TODO: Some blobs, e.g. those with non-utf8 filenames, are returned as nil from the
end # repository. We need to provide a flag to notify the user of this until we come up with a
end # 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
private # of https://gitlab.com/gitlab-org/gitlab/-/issues/323082, at which point this attribute may
# be removed.
def transformed_blob_paths(paths) context[:unretrievable_blobs?] = blobs.size < paths.size
ref = snippet.default_branch
paths.map { |path| [ref, path] } blobs
end end
end end
end end
......
# 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
...@@ -8,6 +8,8 @@ module Types ...@@ -8,6 +8,8 @@ module Types
description 'Represents the snippet blob' description 'Represents the snippet blob'
present_using SnippetBlobPresenter present_using SnippetBlobPresenter
connection_type_class(Types::Snippets::BlobConnectionType)
field :rich_data, GraphQL::Types::String, field :rich_data, GraphQL::Types::String,
description: 'Blob highlighted data.', description: 'Blob highlighted data.',
null: true null: true
......
...@@ -237,15 +237,19 @@ class Snippet < ApplicationRecord ...@@ -237,15 +237,19 @@ class Snippet < ApplicationRecord
end end
end end
def all_files
list_files(default_branch)
end
def blob def blob
@blob ||= Blob.decorate(SnippetBlob.new(self), self) @blob ||= Blob.decorate(SnippetBlob.new(self), self)
end end
def blobs def blobs(paths = [])
return [] unless repository_exists? return [] unless repository_exists?
files = list_files(default_branch) paths = all_files if paths.empty?
items = files.map { |file| [default_branch, file] } items = paths.map { |path| [default_branch, path] }
repository.blobs_at(items).compact repository.blobs_at(items).compact
end end
......
...@@ -7841,6 +7841,7 @@ The connection type for [`SnippetBlob`](#snippetblob). ...@@ -7841,6 +7841,7 @@ The connection type for [`SnippetBlob`](#snippetblob).
| Name | Type | Description | | Name | Type | Description |
| ---- | ---- | ----------- | | ---- | ---- | ----------- |
| <a id="snippetblobconnectionedges"></a>`edges` | [`[SnippetBlobEdge]`](#snippetblobedge) | A list of edges. | | <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="snippetblobconnectionnodes"></a>`nodes` | [`[SnippetBlob]`](#snippetblob) | A list of nodes. |
| <a id="snippetblobconnectionpageinfo"></a>`pageInfo` | [`PageInfo!`](#pageinfo) | Information to aid in pagination. | | <a id="snippetblobconnectionpageinfo"></a>`pageInfo` | [`PageInfo!`](#pageinfo) | Information to aid in pagination. |
...@@ -40768,6 +40768,9 @@ msgstr "" ...@@ -40768,6 +40768,9 @@ msgstr ""
msgid "WARNING:" msgid "WARNING:"
msgstr "" 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" msgid "Wait for the file to load to copy its contents"
msgstr "" msgstr ""
......
import { GlLoadingIcon } from '@gitlab/ui'; import { GlLoadingIcon, GlAlert } from '@gitlab/ui';
import { shallowMount } from '@vue/test-utils'; import { shallowMount } from '@vue/test-utils';
import { Blob, BinaryBlob } from 'jest/blob/components/mock_data'; import { Blob, BinaryBlob } from 'jest/blob/components/mock_data';
import EmbedDropdown from '~/snippets/components/embed_dropdown.vue'; import EmbedDropdown from '~/snippets/components/embed_dropdown.vue';
...@@ -106,6 +106,23 @@ describe('Snippet view app', () => { ...@@ -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', () => { describe('Clone button rendering', () => {
it.each` it.each`
httpUrlToRepo | sshUrlToRepo | shouldRender | isRendered httpUrlToRepo | sshUrlToRepo | shouldRender | isRendered
......
...@@ -13,11 +13,14 @@ RSpec.describe Resolvers::Snippets::BlobsResolver do ...@@ -13,11 +13,14 @@ RSpec.describe Resolvers::Snippets::BlobsResolver do
let_it_be(:current_user) { create(:user) } let_it_be(:current_user) { create(:user) }
let_it_be(:snippet) { create(:personal_snippet, :private, :repository, author: current_user) } let_it_be(:snippet) { create(:personal_snippet, :private, :repository, author: current_user) }
let(:query_context) { {} }
context 'when user is not authorized' do context 'when user is not authorized' do
let(:other_user) { create(:user) } let(:other_user) { create(:user) }
it 'redacts the field' do it 'redacts the field' do
expect(resolve_blobs(snippet, user: other_user)).to be_nil expect(resolve_blobs(snippet, user: other_user)).to be_nil
expect(query_context[:unretrievable_blobs?]).to eq(false)
end end
end end
...@@ -28,6 +31,7 @@ RSpec.describe Resolvers::Snippets::BlobsResolver do ...@@ -28,6 +31,7 @@ RSpec.describe Resolvers::Snippets::BlobsResolver do
expect(result).to match_array(snippet.list_files.map do |file| expect(result).to match_array(snippet.list_files.map do |file|
have_attributes(path: file) have_attributes(path: file)
end) end)
expect(query_context[:unretrievable_blobs?]).to eq(false)
end end
end end
...@@ -37,12 +41,14 @@ RSpec.describe Resolvers::Snippets::BlobsResolver do ...@@ -37,12 +41,14 @@ RSpec.describe Resolvers::Snippets::BlobsResolver do
path = 'CHANGELOG' path = 'CHANGELOG'
expect(resolve_blobs(snippet, paths: [path])).to contain_exactly(have_attributes(path: path)) expect(resolve_blobs(snippet, paths: [path])).to contain_exactly(have_attributes(path: path))
expect(query_context[:unretrievable_blobs?]).to eq(false)
end end
end end
context 'the argument does not match anything' do context 'the argument does not match anything' do
it 'returns an empty result' do it 'returns an empty result' do
expect(resolve_blobs(snippet, paths: ['does not exist'])).to be_empty expect(resolve_blobs(snippet, paths: ['does not exist'])).to be_empty
expect(query_context[:unretrievable_blobs?]).to eq(true)
end end
end end
...@@ -53,12 +59,15 @@ RSpec.describe Resolvers::Snippets::BlobsResolver do ...@@ -53,12 +59,15 @@ RSpec.describe Resolvers::Snippets::BlobsResolver do
expect(resolve_blobs(snippet, paths: paths)).to match_array(paths.map do |file| expect(resolve_blobs(snippet, paths: paths)).to match_array(paths.map do |file|
have_attributes(path: file) have_attributes(path: file)
end) end)
expect(query_context[:unretrievable_blobs?]).to eq(false)
end end
end end
end end
end end
def resolve_blobs(snippet, user: current_user, paths: [], args: { paths: paths }) def resolve_blobs(snippet, user: current_user, paths: [], args: { paths: paths }, has_unretrievable_blobs: false)
resolve(described_class, args: args, ctx: { current_user: user }, obj: snippet) query_context[:current_user] = user
query_context[:unretrievable_blobs?] = has_unretrievable_blobs
resolve(described_class, args: args, ctx: query_context, obj: snippet)
end end
end end
...@@ -3,6 +3,8 @@ ...@@ -3,6 +3,8 @@
require 'spec_helper' require 'spec_helper'
RSpec.describe Snippet do RSpec.describe Snippet do
include FakeBlobHelpers
describe 'modules' do describe 'modules' do
subject { described_class } subject { described_class }
...@@ -526,6 +528,21 @@ RSpec.describe Snippet do ...@@ -526,6 +528,21 @@ RSpec.describe Snippet do
end end
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 describe '#blobs' do
context 'when repository does not exist' do context 'when repository does not exist' do
let(:snippet) { create(:snippet) } let(:snippet) { create(:snippet) }
...@@ -552,6 +569,23 @@ RSpec.describe Snippet do ...@@ -552,6 +569,23 @@ RSpec.describe Snippet do
end end
end 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 end
describe '#to_json' do describe '#to_json' do
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment