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

[Merged by Bors] - Add SensitiveUrl to redact user secrets from endpoints #2326

Closed
wants to merge 5 commits into from

Conversation

macladson
Copy link
Member

Issue Addressed

#2276

Proposed Changes

Add the SensitiveUrl struct which wraps Url and implements custom Display and Debug traits to redact user secrets from being logged in eth1 endpoints, beacon node endpoints and metrics.

Additional Info

This also includes a small rewrite of the eth1 crate to make requests using Url instead of &str.
Some error messages have also been changed to remove Url data.

Copy link
Member

@michaelsproul michaelsproul left a comment

Choose a reason for hiding this comment

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

This is great, really well implemented.

Just a few minor nitpicks to address and then I think we're ready to merge 🎉

beacon_node/eth1/src/service.rs Outdated Show resolved Hide resolved
common/sensitive_url/src/lib.rs Outdated Show resolved Hide resolved
beacon_node/eth1/tests/test.rs Show resolved Hide resolved
@michaelsproul michaelsproul added the waiting-on-author The reviewer has suggested changes and awaits thier implementation. label May 3, 2021
Copy link
Member

@michaelsproul michaelsproul left a comment

Choose a reason for hiding this comment

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

Ready to merge with or after #2331

@michaelsproul michaelsproul added ready-for-merge This PR is ready to merge. and removed waiting-on-author The reviewer has suggested changes and awaits thier implementation. labels May 4, 2021
@michaelsproul
Copy link
Member

bors r+

bors bot pushed a commit that referenced this pull request May 4, 2021
## Issue Addressed

#2276 

## Proposed Changes

Add the `SensitiveUrl` struct which wraps `Url` and implements custom `Display` and `Debug` traits to redact user secrets from being logged in eth1 endpoints, beacon node endpoints and metrics.

## Additional Info

This also includes a small rewrite of the eth1 crate to make requests using `Url` instead of `&str`. 
Some error messages have also been changed to remove `Url` data.
@michaelsproul
Copy link
Member

ah, merge conflict. I'll cancel this and let the audit fix go through first

bors r-

@bors
Copy link

bors bot commented May 4, 2021

Canceled.

@michaelsproul
Copy link
Member

let's try it

bors r+

bors bot pushed a commit that referenced this pull request May 4, 2021
## Issue Addressed

#2276 

## Proposed Changes

Add the `SensitiveUrl` struct which wraps `Url` and implements custom `Display` and `Debug` traits to redact user secrets from being logged in eth1 endpoints, beacon node endpoints and metrics.

## Additional Info

This also includes a small rewrite of the eth1 crate to make requests using `Url` instead of `&str`. 
Some error messages have also been changed to remove `Url` data.
@bors bors bot changed the title Add SensitiveUrl to redact user secrets from endpoints [Merged by Bors] - Add SensitiveUrl to redact user secrets from endpoints May 4, 2021
@bors bors bot closed this May 4, 2021
@macladson macladson deleted the redact-sensitive-urls branch May 5, 2021 01:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ready-for-merge This PR is ready to merge.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants