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

Be able to filter sensitive information #315

Closed
danielb2 opened this issue Mar 25, 2015 · 12 comments · Fixed by #327
Closed

Be able to filter sensitive information #315

danielb2 opened this issue Mar 25, 2015 · 12 comments · Fixed by #327
Assignees
Labels
Milestone

Comments

@danielb2
Copy link
Contributor

@danielb2 danielb2 commented Mar 25, 2015

Sensitive information like creditcard information or passwords is likely to not want to be logged for security reasons.

Modifying and forking reporters for each and every single case isn't practical (once for file logging, for console, for udp, etc)

How this is to be done can be discussed. Perhaps a callback filter function, or a plugin for good itself.

See https://www.npmjs.com/package/blur for reference

@AdriVanHoudt

This comment has been minimized.

Copy link
Contributor

@AdriVanHoudt AdriVanHoudt commented Mar 25, 2015

I like the filter idea, although would it also be possible to filter out certain routes from the response even for example? There are some I don't want logged in console for example

@arb

This comment has been minimized.

Copy link
Contributor

@arb arb commented Apr 9, 2015

What would the API for this look like? The problem with the expectation that you can configure everything with just JSON, so specifying a function in-line wouldn't really work. I guess you could supply a string to a module or path that gets loaded in and has a function with a to-be-determined signature.

@arb arb added the feature label Apr 9, 2015
@danielb2

This comment has been minimized.

Copy link
Contributor Author

@danielb2 danielb2 commented Apr 10, 2015

var options = {
    filter: {
        creditcard: 'creditcard',
        cvv: 'censor',
        unusedfield: 'remove'
    },
    opsInterval: 1000,
    reporters: [{
        reporter: require('good-console'),
        events: { log: '*', response: '*' }
    }, {
        reporter: require('good-file'),
        events: { ops: '*' },
        config: './test/fixtures/awesome_log'
    }, {
        reporter: 'good-http',
        events: { error: '*' },
        config: {
            endpoint: 'http://prod.logs:3000',
            wreck: {
                headers: { 'x-api-key' : 12345 }
            }
        }
    }]
};

Where the keys are fields that will be modified, and the values are modification types. The different types of modification are:

  • creditcard - This filteres all but the last four characters. For example: 1234567890 -> XXXXXX7890
  • censor - Replaces contents with [FILTERED]
  • remove - Removes from logging completely.
@arb

This comment has been minimized.

Copy link
Contributor

@arb arb commented Apr 10, 2015

I like remove and censor. I don't like creditcard as it has an extremely specific use case and if we add one super custom one like that, then we'll end up adding a bunch more.

@danielb2

This comment has been minimized.

Copy link
Contributor Author

@danielb2 danielb2 commented Apr 10, 2015

creditcard can be used for anything where it's useful to validate that the information is correct without compromising the security of what's logged. It's not restricted to creditcard. Perhaps partial can be used to describe it if that makes it feel less specific.

@arb

This comment has been minimized.

Copy link
Contributor

@arb arb commented Apr 10, 2015

Lets make this the API:

censor - replace entire value with "*"
remove - completed removes the value from the object before sending the data out to the reporters
partial - expressed via "partial:n" where n is the last n characters to remain intact.

@danielb2

This comment has been minimized.

Copy link
Contributor Author

@danielb2 danielb2 commented Apr 10, 2015

Let's make it n:partial also so it can be reversed ?

@danielb2

This comment has been minimized.

Copy link
Contributor Author

@danielb2 danielb2 commented Apr 10, 2015

Also, I think the * for censor may not be obvious enough. I like [FILTERED] better because it will be obvious for anyone reading it. * can be a valid character as well whereas [FILTERED] isn't likely to be

@gergoerdosi

This comment has been minimized.

Copy link
Contributor

@gergoerdosi gergoerdosi commented Apr 10, 2015

How about using a regex instead of n:partial or partial:n? So:

creditcard: /(\d{4})$/

We could then have a basic function that replaces the matched string with X characters, something like:

string = string.replace(regex, function ($0, $1) {

    return (new Array($1.length + 1).join('X'));
});

This can be simplified if you just want to use [FILTERED]:

string = string.replace(regex, '[FILTERED]');

Using a regex would allow more complex cases too:

  • /(\d{4})$/: 12345678 -> 1234XXXX
  • /^(\d{4})/: 12345678 -> XXXX5678
  • /(PRIVATE\d{3})/: PUBLIC-PRIVATE123-PUBLIC -> PUBLIC-XXXXXXXXXX-PUBLIC
@lloydbenson

This comment has been minimized.

Copy link
Contributor

@lloydbenson lloydbenson commented Apr 10, 2015

seems like if rather than just doing regex, you may want to it to be any function that takes in a string and outputs a string. Than you can use blur, a regex, or whatever else in order to get it to look how you want. If you want FILTERED or * it shouldnt matter.

@arb

This comment has been minimized.

Copy link
Contributor

@arb arb commented Apr 10, 2015

I think the regex is the winner. The problem with functions is you can't JSON.stringify them while you can do it with REGEX.

@arb arb self-assigned this Apr 10, 2015
@lloydbenson

This comment has been minimized.

Copy link
Contributor

@lloydbenson lloydbenson commented Apr 10, 2015

good point. regex it is.

arb added a commit to arb/good that referenced this issue Apr 10, 2015
@arb arb modified the milestone: 6.1.0 Apr 10, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants
You can’t perform that action at this time.