From 9e7b49b400ff5a803670d3a09a4d295afd2a0496 Mon Sep 17 00:00:00 2001
From: Mehmet Emin INAC <minac@gitlab.com>
Date: Fri, 23 Oct 2020 10:04:47 +0300
Subject: [PATCH] Normalize the SQL queries before sending them to Sentry
To prevent sending some sensitive information, we need to normalize the
SQL queries before we send them to Sentry. To do so, we decided to use
the gem called `pg_query` which compiles some parts of the PostgreSQL
database to make it possible to parse SQL queries.
---
Gemfile | 3 +++
Gemfile.lock | 2 ++
lib/gitlab/error_tracking.rb | 2 +-
spec/lib/gitlab/error_tracking_spec.rb | 6 +++---
4 files changed, 9 insertions(+), 4 deletions(-)
diff --git a/Gemfile b/Gemfile
index 1ec8d53ded792..64d2179cbce63 100644
--- a/Gemfile
+++ b/Gemfile
@@ -307,6 +307,9 @@ gem 'rack-attack', '~> 6.3.0'
# Sentry integration
gem 'sentry-raven', '~> 3.0'
+# PostgreSQL query parsing
+gem 'gitlab-pg_query', '~> 1.3', require: 'pg_query'
+
gem 'premailer-rails', '~> 1.10.3'
# LabKit: Tracing and Correlation
diff --git a/Gemfile.lock b/Gemfile.lock
index 863ab0b511247..4d6496fdcf099 100644
--- a/Gemfile.lock
+++ b/Gemfile.lock
@@ -439,6 +439,7 @@ GEM
gitlab-mail_room (0.0.7)
gitlab-markup (1.7.1)
gitlab-net-dns (0.9.1)
+ gitlab-pg_query (1.3.0)
gitlab-puma (4.3.5.gitlab.3)
nio4r (~> 2.0)
gitlab-puma_worker_killer (0.1.1.gitlab.1)
@@ -1336,6 +1337,7 @@ DEPENDENCIES
gitlab-mail_room (~> 0.0.7)
gitlab-markup (~> 1.7.1)
gitlab-net-dns (~> 0.9.1)
+ gitlab-pg_query (~> 1.3)
gitlab-puma (~> 4.3.3.gitlab.2)
gitlab-puma_worker_killer (~> 0.1.1.gitlab.1)
gitlab-sidekiq-fetcher (= 0.5.2)
diff --git a/lib/gitlab/error_tracking.rb b/lib/gitlab/error_tracking.rb
index 9a9ffbeecb404..a5ace2be77398 100644
--- a/lib/gitlab/error_tracking.rb
+++ b/lib/gitlab/error_tracking.rb
@@ -153,7 +153,7 @@ def sanitize_request_parameters(parameters)
def inject_sql_query_into_extra(exception, extra)
return unless exception.is_a?(ActiveRecord::StatementInvalid)
- extra[:sql] = exception.sql
+ extra[:sql] = PgQuery.normalize(exception.sql.to_s)
end
def sentry_dsn
diff --git a/spec/lib/gitlab/error_tracking_spec.rb b/spec/lib/gitlab/error_tracking_spec.rb
index 7618f205088a4..68a46b11487bc 100644
--- a/spec/lib/gitlab/error_tracking_spec.rb
+++ b/spec/lib/gitlab/error_tracking_spec.rb
@@ -284,13 +284,13 @@
end
context 'when the error is kind of an `ActiveRecord::StatementInvalid`' do
- let(:exception) { ActiveRecord::StatementInvalid.new(sql: :foo) }
+ let(:exception) { ActiveRecord::StatementInvalid.new(sql: 'SELECT "users".* FROM "users" WHERE "users"."id" = 1 AND "users"."foo" = $1') }
- it 'injects the sql query into extra' do
+ it 'injects the normalized sql query into extra' do
track_exception
expect(Raven).to have_received(:capture_exception)
- .with(exception, a_hash_including(extra: a_hash_including(sql: :foo)))
+ .with(exception, a_hash_including(extra: a_hash_including(sql: 'SELECT "users".* FROM "users" WHERE "users"."id" = $2 AND "users"."foo" = $1')))
end
end
end
--
GitLab