fix: redact longer overlapping secrets first#4624
Closed
NikkeTryHard wants to merge 1 commit intobrowser-use:mainfrom
Closed
fix: redact longer overlapping secrets first#4624NikkeTryHard wants to merge 1 commit intobrowser-use:mainfrom
NikkeTryHard wants to merge 1 commit intobrowser-use:mainfrom
Conversation
|
|
475baa3 to
d76c94d
Compare
Member
|
Thanks for the PR and for looking into this! It looks great, but I'm closing this in favor of #4660, which centralizes the redaction logic into a shared helper to keep the code a bit cleaner. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #4609.
Summary
Fixes a partial secret leak in both AgentHistory._filter_sensitive_data_from_string and MessageManager._filter_sensitive_data when one sensitive value is a substring of another.
Previously, secrets were replaced in dict iteration order. If abc was replaced before abc123, the longer value could become partially redacted as short_token123, leaving the suffix visible.
This change sorts sensitive values by descending length before replacement so longer matches are always redacted first in both redaction paths.
The PR is intentionally kept to one atomic commit: the implementation changes and their direct regression coverage stay together for easier review and safe revert.
Tests