Merge branch '45505-lograge_formatter_encoding' into 'master'

Enforce UTF-8 encoding on user input in LogrageWithTimestamp formatter

Closes #45505

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

4 files changed, 47 insertions, 0 deletions

diff --git a/changelogs/unreleased/45505-lograge_formatter_encoding.yml b/changelogs/unreleased/45505-lograge_formatter_encoding.yml
new file mode 100644
index 00000000000..02f4c152966
--- /dev/null
+++ b/changelogs/unreleased/45505-lograge_formatter_encoding.yml
@@ -0,0 +1,6 @@
+---
+title: Enforce UTF-8 encoding on user input in LogrageWithTimestamp formatter and
+  filter out file content from logs
+merge_request:
+author:
+type: fixed
diff --git a/config/application.rb b/config/application.rb
index 1b575f1325d..d379d611074 100644
--- a/config/application.rb
+++ b/config/application.rb
@@ -70,6 +70,7 @@ module Gitlab
     # - Webhook URLs (:hook)
     # - Sentry DSN (:sentry_dsn)
     # - Deploy keys (:key)
+    # - File content from Web Editor (:content)
     config.filter_parameters += [/token$/, /password/, /secret/]
     config.filter_parameters += %i(
       certificate
@@ -81,6 +82,7 @@ module Gitlab
       sentry_dsn
       trace
       variables
+      content
     )
 
     # Enable escaping HTML in JSON.
diff --git a/lib/gitlab/grape_logging/formatters/lograge_with_timestamp.rb b/lib/gitlab/grape_logging/formatters/lograge_with_timestamp.rb
index 1e1fdabca93..0014ce2689b 100644
--- a/lib/gitlab/grape_logging/formatters/lograge_with_timestamp.rb
+++ b/lib/gitlab/grape_logging/formatters/lograge_with_timestamp.rb
@@ -2,8 +2,12 @@ module Gitlab
   module GrapeLogging
     module Formatters
       class LogrageWithTimestamp
+        include Gitlab::EncodingHelper
+
         def call(severity, datetime, _, data)
           time = data.delete :time
+          data[:params] = utf8_encode_values(data[:params]) if data.has_key?(:params)
+
           attributes = {
             time: datetime.utc.iso8601(3),
             severity: severity,
@@ -13,6 +17,19 @@ module Gitlab
           }.merge(data)
           ::Lograge.formatter.call(attributes) + "\n"
         end
+
+        private
+
+        def utf8_encode_values(data)
+          case data
+          when Hash
+            data.merge(data) { |k, v| utf8_encode_values(v) }
+          when Array
+            data.map { |v| utf8_encode_values(v) }
+          when String
+            encode_utf8(data)
+          end
+        end
       end
     end
   end
diff --git a/spec/requests/api/commits_spec.rb b/spec/requests/api/commits_spec.rb
index 8ad19e3f0f5..7e3277c4cab 100644
--- a/spec/requests/api/commits_spec.rb
+++ b/spec/requests/api/commits_spec.rb
@@ -247,6 +247,19 @@ describe API::Commits do
           ]
         }
       end
+      let!(:valid_utf8_c_params) do
+        {
+          branch: 'master',
+          commit_message: message,
+          actions: [
+            {
+              action: 'create',
+              file_path: 'foo/bar/baz.txt',
+              content: 'puts 🦊'
+            }
+          ]
+        }
+      end
 
       it 'a new file in project repo' do
         post api(url, user), valid_c_params
@@ -257,6 +270,15 @@ describe API::Commits do
         expect(json_response['committer_email']).to eq(user.email)
       end
 
+      it 'a new file with utf8 chars in project repo' do
+        post api(url, user), valid_utf8_c_params
+
+        expect(response).to have_gitlab_http_status(201)
+        expect(json_response['title']).to eq(message)
+        expect(json_response['committer_name']).to eq(user.name)
+        expect(json_response['committer_email']).to eq(user.email)
+      end
+
       it 'returns a 400 bad request if file exists' do
         post api(url, user), invalid_c_params