feat: add sensitive data filtering methods#1330
Conversation
Semver Impact of This PR🟡 Minor (new features) 📋 Changelog PreviewThis is how your changes will appear in the changelog. This PR will not appear in the changelog. 🤖 This preview updates automatically when you update the PR. |
giortzisg
force-pushed
the
feat/data-denylist
branch
4 times, most recently
from
4e2524c to
e8e7f1e
Compare
giortzisg
changed the title
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
![sentry[bot]](https://avatars.githubusercontent.com/in/12637?s=60&v=4){kind=small}
giortzisg
force-pushed
the
feat/data-denylist
branch
from
e8e7f1e to
aed4a07
Compare
giortzisg
force-pushed
the
feat/data-denylist
branch
from
aed4a07 to
0c364f9
Compare
giortzisg
force-pushed
the
feat/data-denylist
branch
from
0c364f9 to
9f3c30a
Compare
giortzisg
force-pushed
the
feat/data-denylist
branch
from
9f3c30a to
389be77
Compare
| func isSensitiveKey(key string) bool { | ||
| return matchesDenyTerms(key, sensitiveDenyList) | ||
| } |
There was a problem hiding this comment.
This feels a bit incomplete because it's always used with !matchesDenyTerms(k, behavior.Terms) or matchesDenyTerms(k, behavior.Terms) and doesn't appear to be allowed to exist on its own.
Would it make sense to have two functions, one for
isSensitiveKey(k) || !matchesDenyTerms(k, behavior.Terms) and one with isSensitiveKey(k) || matchesDenyTerms(k, behavior.Terms) and give them descriptive names?
There was a problem hiding this comment.
In terms of separation this probably makes more sense and would be easier to distinguish, so let me change that.
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit f0b976d. Configure here.
| for _, t := range dc.HTTPBodies { | ||
| if t == bt { | ||
| return true | ||
| } | ||
| } | ||
| return false |
There was a problem hiding this comment.
could this be return dc.HTTPBodies.Contains(bt)?
|
|
||
| // matchesDenyTerms reports whether the key (case-insensitive) contains any of | ||
| // the given terms as a substring. | ||
| func matchesDenyTerms(key string, terms []string) bool { |
There was a problem hiding this comment.
nit: it's a bit confusing that the name contains deny even though it's used for allow and deny. Maybe we can call it just matchesTerms?
giortzisg
force-pushed
the
feat/data-collection
branch
from
c976792 to
a1f3a34
Compare
giortzisg
force-pushed
the
feat/data-denylist
branch
from
29a8d32 to
c0138b5
Compare
giortzisg
force-pushed
the
feat/data-collection
branch
from
a1f3a34 to
264848f
Compare
giortzisg
force-pushed
the
feat/data-denylist
branch
from
c0138b5 to
0afaade
Compare
giortzisg
force-pushed
the
feat/data-collection
branch
from
264848f to
eb379b3
Compare
giortzisg
force-pushed
the
feat/data-denylist
branch
from
0afaade to
0b51832
Compare
2 participants
Description
This PR adds the filtering methods for sensitive data. Using the new methods means that the SDK now will properly apply scrubbing to collected headers, cookies, query parameters and request bodies (the SDK previously collected raw bodies and query strings. The followup PR will take care of the plumbing and fixing all failing tests).
SendDefaultPIIalso remains supported for backwards compatibility along with the fixes on raw sensitive data.#skip-changelog
Issues
Changelog Entry Instructions
To add a custom changelog entry, uncomment the section above. Supports:
For more details: custom changelog entries
Reminders
feat:,fix:,ref:,meta:)