Skip to content

Commit a0dcc7a

Browse files
sl0thentr0pyOpenAI
andcommitted
feat(pii): add sensitive key-value collection filter
Co-Authored-By: OpenAI <noreply@example.com>
1 parent cdc32f0 commit a0dcc7a

4 files changed

Lines changed: 199 additions & 25 deletions

File tree

sentry-ruby/lib/sentry/configuration.rb

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -244,11 +244,6 @@ class Configuration
244244
# @return [DataCollection]
245245
attr_accessor :data_collection
246246

247-
# Controls which categories of data may be collected.
248-
# Replacement for send_default_pii.
249-
# @return [DataCollection]
250-
attr_accessor :data_collection
251-
252247
# Capture queue time from X-Request-Start header set by reverse proxies.
253248
# Works with any Rack app behind Nginx, HAProxy, Heroku router, etc.
254249
# Defaults to true.
@@ -565,7 +560,6 @@ def initialize
565560
self.send_modules = true
566561
self.data_collection = DataCollection.new
567562
self.send_default_pii = false
568-
self.data_collection = DataCollection.new
569563
self.skip_rake_integration = false
570564
self.send_client_reports = true
571565
self.auto_session_tracking = true

sentry-ruby/lib/sentry/data_collection.rb

Lines changed: 2 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,7 @@
11
# frozen_string_literal: true
22

3+
require "sentry/data_collection/key_value_collection"
4+
35
module Sentry
46
class DataCollection
57
# Configuration for the categories of data collected by the SDK.
@@ -34,25 +36,6 @@ class DataCollection
3436
outgoing_response
3537
].freeze
3638

37-
# Configuration for key-value data collection.
38-
class KeyValueCollection
39-
# `mode` controls whether values are collected:
40-
# - `:off` disables collection.
41-
# - `:deny_list` collects values except those matching `terms`.
42-
# - `:allow_list` collects only values matching `terms`.
43-
# @return [:off, :deny_list, :allow_list]
44-
attr_accessor :mode
45-
46-
# `terms` contains the keys or patterns used by the selected mode.
47-
# @return [Array<String>, nil]
48-
attr_accessor :terms
49-
50-
def initialize(mode:, terms:)
51-
@mode = mode
52-
@terms = terms
53-
end
54-
end
55-
5639
class HttpHeaders
5740
# @return [KeyValueCollection]
5841
attr_accessor :request
Lines changed: 92 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,92 @@
1+
# frozen_string_literal: true
2+
3+
module Sentry
4+
class DataCollection
5+
# Configuration for key-value data collection.
6+
class KeyValueCollection
7+
FILTERED_VALUE = "[Filtered]"
8+
9+
# Keys from this list are ALWAYS filtered, regardless of :mode
10+
# TODO-neel-data cookies have separate list, see JS
11+
SENSITIVE_DENY_LIST = %w[
12+
auth
13+
token
14+
secret
15+
session
16+
password
17+
passwd
18+
pwd
19+
key
20+
jwt
21+
bearer
22+
sso
23+
saml
24+
csrf
25+
xsrf
26+
credentials
27+
sid
28+
identity
29+
cookie
30+
set-cookie
31+
].freeze
32+
33+
# `mode` controls whether values are collected:
34+
# - `:off` disables collection.
35+
# - `:deny_list` collects values except those matching `terms`.
36+
# - `:allow_list` collects only values matching `terms`.
37+
# @return [:off, :deny_list, :allow_list]
38+
attr_accessor :mode
39+
40+
# `terms` contains the keys or patterns used by the selected mode.
41+
# @return [Array<String>, nil]
42+
attr_reader :terms
43+
44+
def initialize(mode:, terms:)
45+
@mode = mode
46+
self.terms = terms
47+
end
48+
49+
def terms=(terms)
50+
@terms = terms&.map { |term| term.to_s.downcase }&.reject { |term| term.strip.empty? }
51+
end
52+
53+
# Applies this collection configuration without changing the input hash.
54+
# Keys are retained whenever the category is collected; values that are not
55+
# safe to send are replaced with FILTERED_VALUE.
56+
#
57+
# @param values [Hash] key-value data to filter
58+
# @return [Hash] a new filtered hash, or an empty hash when collection is off
59+
def filter(values)
60+
return {} if mode == :off
61+
62+
values.each_with_object({}) do |(key, value), filtered|
63+
filtered[key] = safe_value?(key) ? value : FILTERED_VALUE
64+
end
65+
end
66+
67+
private
68+
69+
def safe_value?(key)
70+
key_downcase = key.to_s.downcase
71+
return false if sensitive?(key_downcase)
72+
73+
case mode
74+
when :deny_list
75+
!matches_any_term?(key_downcase)
76+
when :allow_list
77+
matches_any_term?(key_downcase)
78+
else
79+
false
80+
end
81+
end
82+
83+
def sensitive?(key)
84+
SENSITIVE_DENY_LIST.any? { |term| key.include?(term) }
85+
end
86+
87+
def matches_any_term?(key)
88+
@terms&.any? { |term| key.include?(term) }
89+
end
90+
end
91+
end
92+
end
Lines changed: 105 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,105 @@
1+
# frozen_string_literal: true
2+
3+
require "sentry/data_collection/key_value_collection"
4+
5+
RSpec.describe Sentry::DataCollection::KeyValueCollection do
6+
subject(:collection) { described_class.new(mode: mode, terms: terms) }
7+
8+
let(:values) do
9+
{
10+
"Authorization" => "secret-token",
11+
"page" => "2",
12+
"display_name" => "Ada",
13+
42 => "numeric key"
14+
}
15+
end
16+
let(:mode) { :deny_list }
17+
let(:terms) { nil }
18+
19+
describe "#filter" do
20+
it "uses the collection configuration" do
21+
expect(collection.filter(values)).to eq(
22+
"Authorization" => "[Filtered]",
23+
"page" => "2",
24+
"display_name" => "Ada",
25+
42 => "numeric key"
26+
)
27+
end
28+
29+
context "when mode is :off" do
30+
let(:mode) { :off }
31+
32+
it "does not collect keys or values" do
33+
expect(collection.filter(values)).to eq({})
34+
end
35+
end
36+
37+
context "when mode is :deny_list" do
38+
it "matches sensitive terms partially and case-insensitively" do
39+
expect(described_class.new(mode: :deny_list, terms: nil).filter(
40+
{ "X-Auth-Token" => "a", "ACCESS_SECRET_VALUE" => "b", "random_field" => "c" }
41+
)).to eq(
42+
"X-Auth-Token" => "[Filtered]",
43+
"ACCESS_SECRET_VALUE" => "[Filtered]",
44+
"random_field" => "c"
45+
)
46+
end
47+
48+
it "applies additional deny terms" do
49+
expect(described_class.new(mode: :deny_list, terms: ["USER", :internal]).filter(
50+
{ "user_id" => "1", "internal" => "value" }
51+
)).to eq(
52+
"user_id" => "[Filtered]",
53+
"internal" => "[Filtered]"
54+
)
55+
end
56+
57+
it "normalizes terms assigned after initialization" do
58+
collection.terms = ["USER"]
59+
60+
expect(collection.filter("user_id" => "1")).to eq("user_id" => "[Filtered]")
61+
end
62+
63+
it "covers every built-in sensitive term" do
64+
values = Sentry::DataCollection::KeyValueCollection::SENSITIVE_DENY_LIST.to_h do |term|
65+
["prefix-#{term.upcase}-suffix", "value"]
66+
end
67+
68+
expect(collection.filter(values).values.uniq).to eq(["[Filtered]"])
69+
end
70+
end
71+
72+
context "when mode is :allow_list" do
73+
let(:mode) { :allow_list }
74+
let(:terms) { ["page", "display"] }
75+
76+
it "filters values whose keys are not allowed" do
77+
expect(collection.filter(values)).to eq(
78+
"Authorization" => "[Filtered]",
79+
"page" => "2",
80+
"display_name" => "Ada",
81+
42 => "[Filtered]"
82+
)
83+
end
84+
85+
it "still filters sensitive keys listed in the allow list" do
86+
expect(described_class.new(mode: :allow_list, terms: ["token", "public"]).filter(
87+
{ "token" => "secret", "public" => "value" }
88+
)).to eq("token" => "[Filtered]", "public" => "value")
89+
end
90+
91+
it "does not allow every key when terms include nil or blank values" do
92+
expect(described_class.new(mode: :allow_list, terms: [nil, "", " "]).filter(
93+
{ "public" => "value", "page" => "2" }
94+
)).to eq("public" => "[Filtered]", "page" => "[Filtered]")
95+
end
96+
end
97+
98+
it "does not mutate the input" do
99+
original = values.dup
100+
collection.filter(values)
101+
102+
expect(values).to eq(original)
103+
end
104+
end
105+
end

0 commit comments

Comments
 (0)