Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix: Increase Entropy Requirement for Secret Redaction to Reduce False Positives #6875

Merged
merged 2 commits into from
Feb 22, 2025

Conversation

tofarr
Copy link
Collaborator

@tofarr tofarr commented Feb 21, 2025

End-user friendly description of the problem this fixes or functionality that this introduces

If a value has less than 8 characters, it will not be considered a secret as it does not have enough entropy

  • Include this change in the Release Notes. If checked, you must provide an end-user friendly description for your change below

Give a summary of what the PR does, explaining any non-trivial design decisions
The new logging filters can sometimes lead to unusual outcomes - we look at the environment for any variable with a name containing the terms KEY, CODE, SECRET or TOKEN and replace those values with ****** in the logs - it seems in my env my AWS_ACCESS_KEY_ID was set to default, so any time the word default appeared in the logs it was redacted! By increasing the entropy requirement we remove false positives.


Link of any specific issues this addresses


To run this PR locally, use the following command:

docker run -it --rm   -p 3000:3000   -v /var/run/docker.sock:/var/run/docker.sock   --add-host host.docker.internal:host-gateway   -e SANDBOX_RUNTIME_CONTAINER_IMAGE=docker.all-hands.dev/all-hands-ai/runtime:4981063-nikolaik   --name openhands-app-4981063   docker.all-hands.dev/all-hands-ai/openhands:4981063

If a value has less than 8 characters, it cannot be a secret as it does not have enough entropy
@tofarr tofarr marked this pull request as ready for review February 21, 2025 12:28
@tofarr tofarr changed the title Fix: Secret filtering Fix: Increase Entropy Requirement for Secret Redaction Feb 21, 2025
@tofarr tofarr changed the title Fix: Increase Entropy Requirement for Secret Redaction Fix: Increase Entropy Requirement for Secret Redaction to Reduce False Positives Feb 21, 2025
@@ -221,7 +221,7 @@ def filter(self, record):
sensitive_values = []
for key, value in os.environ.items():
key_upper = key.upper()
if len(value) > 2 and any(
if len(value) >= 8 and any(
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this seems dangerous. I could see an 8 char secret, many passwords are 8 chars

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's why I picked >= 8.

@tofarr tofarr requested a review from rbren February 21, 2025 21:47
@tofarr tofarr merged commit a8bce37 into main Feb 22, 2025
14 checks passed
@tofarr tofarr deleted the refine-secret-filtering branch February 22, 2025 08:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants