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

excludedParams only works with exact match, and does not hide GET params from URL posted to HB #43

Open
benissimo opened this issue Dec 5, 2017 · 1 comment

Comments

@benissimo
Copy link

benissimo commented Dec 5, 2017

When I manually trigger an error notification to HoneyBadger (HB) like this:

        Set<String> excludedParams = new HashSet<>();
        StandardConfigContext excludedParams.add("account_number");

        configContext = new StandardConfigContext();
        configContext.setApiKey("redacted...")
                .setExcludedParams(excludedParams);

        NoticeReporter reporter = new HoneybadgerReporter(configContext);
        reporter.reportError(t, o)

If the page triggering this error has an URL such as:

/page?foo=bar&account_number=secret&foo2=baz&account_number_confirmation=secret

Then when I visit HB to see the reported error, I find two problems:

  1. (minor) account_number_confirmation is not filtered from the "params" section of HB. It would be nice if HoneybadgerReport's constructor allowed us to pass through our own exclusion strategy so that we could allow the use of regexes (or at the very least substrings) instead of only supporting exact matches.

  2. (major) The full URL, with all querystring parameters, is listed in the URL for the origin of the error on HB. ie. The GET parameters are only stripped out in the "params" section, not in the URL itself, which contains all of them.

Granted, sensitive params shouldn't be passed around via GET to begin with, but it's inconsistent to filter them out of the "params" section while leaving them intact in the URL.

@joshuap
Copy link
Member

joshuap commented Dec 7, 2017

I agree with both points. We've solved the GET URL filtering in Ruby; it's a little tricky because you have to parse out the query string and then reconstruct it with the sensitive keys filtered, but it's doable. Supporting regexps/substrings (or at least some kind of programmatic filter) would also be nice.

We don't have time to tackle this ourselves at the moment, however I'd accept a PR; otherwise we'll look at this in our next Java sprint. It may be better to start with #44 since it's easier and would at least provide a stopgap for people who don't want to risk leaking sensitive info in the query string.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants