feat(pii): add sensitive data scrubber#2170
Conversation
![sentry[bot]](https://avatars.githubusercontent.com/in/12637?s=60&v=4){kind=small}
| { | ||
| if (self::matchesMandatoryDenyList($key)) { | ||
| return true; | ||
| } | ||
|
|
||
| if ($behavior['mode'] === 'allowList') { | ||
| return !self::matchesAnyTerm($key, $behavior['terms'], false); | ||
| } | ||
|
|
||
| return $behavior['terms'] !== [] && self::matchesAnyTerm($key, $behavior['terms'], true); | ||
| } |
There was a problem hiding this comment.
Bug: The shouldScrubValue function doesn't handle mode = 'off'. It falls through to denyList logic, incorrectly scrubbing custom terms when the mode is "off".
Severity: MEDIUM
Suggested Fix
Add an explicit check for mode === 'off' at the beginning of the shouldScrubValue function. If the mode is 'off', the function should return false after checking the mandatory deny list, ensuring no custom terms are processed.
Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's
not valid.
Location: src/DataCollection/SensitiveDataScrubber.php#L123-L134
Potential issue: The `shouldScrubValue` function lacks explicit handling for when the
scrubbing `mode` is set to `'off'`. As a result, the logic falls through to the final
return statement on line 133, which applies `denyList` logic. If a caller sets `mode` to
`'off'` but also provides a non-empty `terms` array, those terms will be treated as a
deny list, causing data to be scrubbed. This contradicts the expected behavior of an
`'off'` mode, which should disable custom scrubbing. The mandatory deny list is still
applied, which may be intentional, but the application of custom terms is a bug. This is
not covered by any tests.
Did we get this right? 👍 / 👎 to inform future reviews.
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
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 3686aef. Configure here.
| } | ||
|
|
||
| return $behavior['terms'] !== [] && self::matchesAnyTerm($key, $behavior['terms'], true); | ||
| } |
There was a problem hiding this comment.
Unhandled off collection mode
Medium Severity
shouldScrubValue accepts KeyValueCollectionBehavior, which includes mode off, but never handles it. After the mandatory denylist check, off falls through to deny-list logic, so non-mandatory keys stay unredacted even when collection for that category is disabled.
Reviewed by Cursor Bugbot for commit 3686aef. Configure here.
1 participant
Adds a
SensitiveDataScrubberwhich can be used to perform scrubbing operations on request specific data such as headers, cookies or query strings.