Mask sensitive values

[matthewadams]

https://stackoverflow.com/search?q=%5Bnode-config%5D+mask yields no results as of this writing.
https://github.com/lorenwest/node-config/issues?q=is%3Aissue+mask yields two irrelevant results as of this writing.

I'm submitting a ...

• [ ] bug report
• [ ✅] feature request
• [ ] support request or question => Please do not submit support request or questions here, see note at the top of this template.

What is the current behavior?

const config = require('config') // includes sensitive values
console.log(config.util.toObject(config)) // provides sensitive values in the clear

What is the expected behavior?

config should mask with *** by default any values deemed "sensitive". The question is, what's the mechanism by which a user indicates to config that a value is "sensitive"?

I'm thinking that config could support another config file with a fixed name, similar to custom-environment-variables.json, that would allow a user to specify those values that are sensitive and should be masked when retrieved, unless explicitly retrieved unmasked.

In order to preserve backward compatibility, a new environment variable called NODE_CONFIG_MASK_SENSITIVES could be supported, similar to those at https://github.com/lorenwest/node-config/wiki/Environment-Variables. If set, the behavior would cause sensitive values to be masked. It would default to unset, of course.

Example:
custom-environment-variables.json:

{
  "database": {
    "url": "NODE_APP_DATABASE_URL",
    "password": "NODE_APP_DATABASE_PASSWORD"
  }
}

sensitive-variables.json:

{
  "database": {
    "password": true // default mask is "***", or a user-defined masking string could be provided
  }
}

default.json:

{
  "database": {
    "url": "mydb://foobar:3454",
    "password": ""
  }
}

Test code demonstrating requirements:

// this is a test using chai.expect
process.env.NODE_CONFIG_MASK_SENSITIVES=1

const pwd = 'J(HhxG653@;'
process.env.NODE_APP_DATABASE_PASSWORD = pwd

const config = require('config')

expect.equal(config.get('database.password'), '***')
expect.equal(config.get('database.password', true), pwd)

let plain = config.util.toObject(config)
expect.equal(plain.database.password, '***')

plain = config.util.toObject(config, true)
expect.equal(plain.database.password, pwd)

Please tell us about your environment:

• node-config version: 3.3.1
• node-version: v12.14.0

Other information

n/a

[markstos]

@matthewadams Can you say more about your threat model you have in mind where this change makes a meaningful difference in security?

In the example you gave about, the database password is in plain text in a test script. Do you have an environment where the people who have access to the test output are different that the people who have access to the tests? In my experience this always been the same group of people.

Also, doesn't it reduce security to duplicate the storage of sensitive database passwords in the system just to check to see if the live database passwords match? In our system, most developers who access to the test suite don't have access to the production database password at all because they don't need it.

In your model, how are your database passwords secured before they get into the config system?

[lorenwest]

I can attest to the value of having a generic toObject() mask sensitive values. In our production environment we log the end state of merging, and we don't want sensitive values indexed by our log indexer, for all to see.

We do this masking in our logging library, but I can see a value of it in the config library - so all general purpose output of configs have these masks.

[matthewadams]

@markstos, what @lorenwest said. This feature request aims to prevent the mindless, boneheaded propensity that some developers have to just JSON.stringify(config, undefined, 2), JSON.stringify(config.util.toObject(config), undefined, 2) or similar with some subproperty of the config object, which exposes sensitive things. I admit to doing this myself, and hating the fact that I have to go build a library that tries to layer this on top of config.

I'd rather just opt in to the new feature via NODE_CONFIG_MASK_SENSITIVES=1, identify the sensitive values, and forget about it.

[markstos]

After reading the feedback, this proposal seems sounds if someone wants to implement it.