Skip to content

Commit

Permalink
fix!: Return message when sql is over the obfuscation limit (#1149)
Browse files Browse the repository at this point in the history
Previously, when a SQL query with a prepended comment
exceeded the obfuscation limit, the query would be truncated without
obfuscation.

Now, when the when the obfuscation limit is hit, a message is 
returned and obfuscation is not attempted.
  • Loading branch information
kaylareopelle authored Sep 10, 2024
1 parent 012b2c2 commit 8e778ba
Show file tree
Hide file tree
Showing 6 changed files with 12 additions and 62 deletions.
2 changes: 1 addition & 1 deletion helpers/sql-obfuscation/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ end

Make sure the `Instrumentation` class for your gem contains configuration options for:

- `:obfuscation_limit`: the length at which the obfuscated SQL string will be truncated.
- `:obfuscation_limit`: the length at which the SQL string will not be obfuscated
Example: `option :obfuscation_limit, default: 2000, validate: :integer`

If you want to add support for a new adapter, update the following constants to include keys for your adapter:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -94,14 +94,16 @@ def generate_regex(dialect)
# This is a SQL obfuscation utility intended for use in database adapter instrumentation.
#
# @param sql [String] The SQL to obfuscate.
# @param obfuscation_limit [optional Integer] The maximum length of an obfuscated sql statement.
# @param obfuscation_limit [optional Integer] the length at which the SQL string will not be obfuscated
# @param adapter [optional Symbol] the type of database adapter calling the method. `:default`, `:mysql` and `:postgres` are supported.
# @return [String] The SQL query string where the values are replaced with "?". When the sql statement exceeds the obufscation limit
# the first matched pair from the SQL statement will be returned, with an appended truncation message. If trunaction is unsuccessful,
# a string describing the error will be returned.
#
# @api public
def obfuscate_sql(sql, obfuscation_limit: 2000, adapter: :default)
return "SQL not obfuscated, query exceeds #{obfuscation_limit} characters" if sql.size > obfuscation_limit

regex = case adapter
when :mysql
MYSQL_COMPONENTS_REGEX
Expand All @@ -115,7 +117,6 @@ def obfuscate_sql(sql, obfuscation_limit: 2000, adapter: :default)
# https://github.com/open-telemetry/opentelemetry-ruby-contrib/pull/160
# https://github.com/open-telemetry/opentelemetry-ruby-contrib/pull/345
sql = OpenTelemetry::Common::Utilities.utf8_encode(sql, binary: true)
return truncate_statement(sql, regex, obfuscation_limit) if sql.size > obfuscation_limit

sql = sql.gsub(regex, PLACEHOLDER)
return 'Failed to obfuscate SQL query - quote characters remained after obfuscation' if CLEANUP_REGEX[adapter].match(sql)
Expand All @@ -124,16 +125,6 @@ def obfuscate_sql(sql, obfuscation_limit: 2000, adapter: :default)
rescue StandardError => e
OpenTelemetry.handle_error(message: 'Failed to obfuscate SQL', exception: e)
end

# @api private
def truncate_statement(sql, regex, limit)
first_match_index = sql.index(regex)
truncation_message = "SQL truncated (> #{limit} characters)"
return truncation_message unless first_match_index

truncated_sql = sql[..first_match_index - 1]
"#{truncated_sql}...\n#{truncation_message}"
end
end
end
end
12 changes: 2 additions & 10 deletions helpers/sql-obfuscation/test/helpers/sql_obfuscation_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -18,17 +18,9 @@ def test_named_arg_defaults_obfuscates
assert_equal(expected, result)
end

def test_obfuscation_limit_truncates_query_after_first_match
def test_obfuscation_returns_message_when_limit_is_reached
sql = "SELECT * from users where users.id = 1 and users.email = 'test@test.com'"
expected = "SELECT * from users where users.id = ...\nSQL truncated (> 42 characters)"
result = OpenTelemetry::Helpers::SqlObfuscation.obfuscate_sql(sql, obfuscation_limit: 42)

assert_equal(expected, result)
end

def test_obfuscation_limit_truncates_when_query_not_encoded_with_utf8
sql = "SELECT * from 😄 where users.id = 1 and users.😄 = 'test@test.com'"
expected = "SELECT * from where users.id = ...\nSQL truncated (> 42 characters)"
expected = 'SQL not obfuscated, query exceeds 42 characters'
result = OpenTelemetry::Helpers::SqlObfuscation.obfuscate_sql(sql, obfuscation_limit: 42)

assert_equal(expected, result)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -236,20 +236,9 @@
describe 'with obfuscation_limit' do
let(:config) { { db_statement: :obfuscate, obfuscation_limit: 10 } }

it 'truncates SQL using config limit' do
it 'returns a message when the limit is reached' do
sql = "SELECT * from users where users.id = 1 and users.email = 'test@test.com'"
obfuscated_sql = "SELECT * from users where users.id = ...\nSQL truncated (> 10 characters)"
expect do
client.query(sql)
end.must_raise Mysql2::Error

_(span.attributes['db.statement']).must_equal obfuscated_sql
end

it 'handles regex non-matches' do
sql = 'ALTER TABLE my_table DISABLE TRIGGER ALL;'
obfuscated_sql = 'SQL truncated (> 10 characters)'

obfuscated_sql = 'SQL not obfuscated, query exceeds 10 characters'
expect do
client.query(sql)
end.must_raise Mysql2::Error
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -297,20 +297,9 @@
describe 'with obfuscation_limit' do
let(:config) { { db_statement: :obfuscate, obfuscation_limit: 10 } }

it 'truncates SQL using config limit' do
it 'returns a message when the limit is reached' do
sql = "SELECT * from users where users.id = 1 and users.email = 'test@test.com'"
obfuscated_sql = "SELECT * from users where users.id = ...\nSQL truncated (> 10 characters)"
expect do
client.exec(sql)
end.must_raise PG::UndefinedTable

_(span.attributes['db.statement']).must_equal obfuscated_sql
end

it 'handles regex non-matches' do
sql = 'ALTER TABLE my_table DISABLE TRIGGER ALL;'
obfuscated_sql = 'SQL truncated (> 10 characters)'

obfuscated_sql = 'SQL not obfuscated, query exceeds 10 characters'
expect do
client.exec(sql)
end.must_raise PG::UndefinedTable
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -323,20 +323,9 @@
describe 'with obfuscation_limit' do
let(:config) { { db_statement: :obfuscate, obfuscation_limit: 10 } }

it 'truncates SQL using config limit' do
it 'returns a message when the limit is reached' do
sql = "SELECT * from users where users.id = 1 and users.email = 'test@test.com'"
obfuscated_sql = "SELECT * from users where users.id = ...\nSQL truncated (> 10 characters)"
expect do
client.query(sql)
end.must_raise Trilogy::Error

_(span.attributes['db.statement']).must_equal obfuscated_sql
end

it 'handles regex non-matches' do
sql = 'ALTER TABLE my_table DISABLE TRIGGER ALL;'
obfuscated_sql = 'SQL truncated (> 10 characters)'

obfuscated_sql = 'SQL not obfuscated, query exceeds 10 characters'
expect do
client.query(sql)
end.must_raise Trilogy::Error
Expand Down

0 comments on commit 8e778ba

Please sign in to comment.