Exposing credentials on error

kobymolcho commented 3 years ago

On console when receiving an error (connection failure and such), the adapter sails-mysql print out the user + password and all the connection information such as database, host and port

sailsbot commented 3 years ago

@kobymolcho Thanks for posting! We'll take a look as soon as possible.

In the mean time, there are a few ways you can help speed things along:

look for a workaround. (Even if it's just temporary, sharing your solution can save someone else a lot of time and effort.)
tell us why this issue is important to you and your team. What are you trying to accomplish? (Submissions with a little bit of human context tend to be easier to understand and faster to resolve.)
make sure you've provided clear instructions on how to reproduce the bug from a clean install.
double-check that you've provided all of the requested version and dependency information. (Some of this info might seem irrelevant at first, like which database adapter you're using, but we ask that you include it anyway. Oftentimes an issue is caused by a confluence of unexpected factors, and it can save everybody a ton of time to know all the details up front.)
read the code of conduct.
if appropriate, ask your business to sponsor your issue. (Open source is our passion, and our core maintainers volunteer many of their nights and weekends working on Sails. But you only get so many nights and weekends in life, and stuff gets done a lot faster when you can work on it during normal daylight hours.)
let us know if you are using a 3rd party plugin; whether that's a database adapter, a non-standard view engine, or any other dependency maintained by someone other than our core team. (Besides the name of the 3rd party package, it helps to include the exact version you're using. If you're unsure, check out this list of all the core packages we maintain.)

Please remember: never post in a public forum if you believe you've found a genuine security vulnerability. Instead, disclose it responsibly.

For help with questions about Sails, click here.

alxndrsn commented 3 years ago

Here's a similar bug for the postgres adapter: https://github.com/balderdashy/sails/issues/4595

Fix may be similar also.

eashaw commented 3 years ago

Hi @kobymolcho, Are you seeing this error message after lifting the server, and has this been an issue for your team?

kobymolcho commented 3 years ago

Hi, @eashaw we see the error both when we are lifting a new server (might be scaling up the replicas), and when the sql have a problem (like connection exhaustion). In both cases, we see the full connection info dumped to the log.

An easy solution that i can recommend, is just adding a flag "hideSecrets", which can be by default 'false', and on production systems, users can enable it. This way it won't break up the current functionality.

eashaw commented 3 years ago

Hi @kobymolcho that sounds like a good solution, and we'd be happy to take a look at a PR for this. Have you looked into seeing where in the adapter code the credentials are being logged?

kobymolcho commented 3 years ago

Hi @eashaw I'm guessing the url have it in the 'register-data-store.js', or the runner.js at the config url. than this line can be the reason: return exits.error(new Error('There was an error creating a new manager for the connection with a url of: ' + inputs.config.url + '\n\n' + e.stack));

i would have replace the throw inputs.config.url with a throw input.config.safe which output the url or the hidden url ..

eashaw commented 3 years ago

Hi @kobymolcho, I was reminded that we have a solution for this in sails-postgresql. I'll test adding that to sails-mysql, Thank you for looking into this.

kobymolcho commented 3 years ago

great @eashaw ! any updates on this one?

eashaw commented 3 years ago

Hey @kobymolcho, We just published a new version of sails-mysql that redacts passwords in error messages.

kobymol commented 3 years ago

looks great! thanks!!

KinoThe-Kafkaesque commented 1 year ago

Hi i am currently working on a codebase and currently facing this issue , i noticed that in node_modules/sails-mysql/lib/private/redact-passwords.js , the fix only works if i replace err.meta with err.raw.meta and when i try to log the err.meta it's undefined , i am using "sails": "^1.5.7", "sails-hook-grunt": "^4.0.0", "sails-hook-orm": "^2.1.1", "sails-hook-sentry": "^2.0.0-beta.2", "sails-hook-sockets": "^2.0.0", "sails-mysql": "^3.0.1",

balderdashy / sails

Exposing credentials on error #7128