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

Add middleware for conditional Scope report censoring #2515

Merged
merged 2 commits into from
Feb 26, 2019

Conversation

fbarl
Copy link

@fbarl fbarl commented Feb 25, 2019

Part of #2477 (comment) as suggested by @foot.

Depends on weaveworks/scope#3571.

@fbarl fbarl self-assigned this Feb 25, 2019
@fbarl fbarl requested a review from foot February 25, 2019 13:51
@rade
Copy link
Member

rade commented Feb 25, 2019

I really like this approach. Well done!

Copy link

@rndstr rndstr left a comment

Choose a reason for hiding this comment

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

lgtm 👌

Copy link
Contributor

@foot foot left a comment

Choose a reason for hiding this comment

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

🏄

}); err != nil {
q := r.URL.Query()
q.Add("hideCommandLineArguments", "true")
q.Add("hideEnvironmentVariables", "true")
Copy link
Contributor

@foot foot Feb 26, 2019

Choose a reason for hiding this comment

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

If a malicious user has already included a hideEnvironmentVariables=false in the querystring I wonder how the querystring parsing in scope will handle that.

Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link
Author

Choose a reason for hiding this comment

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

Thanks @foot, good catch, I'll use Set instead!

@fbarl fbarl force-pushed the 2477-scope-censor-middleware branch from 92008ac to c32f3e7 Compare February 26, 2019 11:18
@fbarl fbarl merged commit d1e80f8 into master Feb 26, 2019
@fbarl fbarl deleted the 2477-scope-censor-middleware branch February 26, 2019 11:41
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.

4 participants