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

option to suppress db creds in error #4606

Closed
shackbarth opened this issue Feb 27, 2019 · 10 comments
Closed

option to suppress db creds in error #4606

shackbarth opened this issue Feb 27, 2019 · 10 comments
Labels
helpful info or workaround orm Related to models, datastores, orm config, Waterline, sails-hook-orm, etc. proposal

Comments

@shackbarth
Copy link

Waterline version: 0.13.6
Node version: 10.13.0
NPM version: 6.4.1
Operating system: Fedora 27


I'm in the habit of console.logging my errors, but I don't want my database password to get logged.

When I execute a waterline query against a stopped database I get an error along the lines of

A connection either could not be obtained or there was an error using the connection.
   Additional data:
   
   { error:
      { Error: connect ECONNREFUSED 127.0.0.1:5432
          at TCPConnectWrap.afterConnect [as oncomplete] (net.js:1113:14)
        errno: 'ECONNREFUSED',
        code: 'ECONNREFUSED',
        syscall: 'connect',
        address: '127.0.0.1',
        port: 5432 },
     meta:
      { adapter: 'sails-postgresql',
        url: 'postgres://me:mypreciouspassword@localhost:5432/my_db',
        ssl: false,
        identity: 'postgres' } }

Which I can trace to this line:
https://github.com/node-machine/machine/blob/850e87de1200b3985c04dadbd3554eea3c6f8613/lib/private/help-build-machine.js#L920

What I'd propose (and I'd be happy to make a PR) would be to change the previous line of

if (!rawOutputParsedAsError) {

to something like

if (!rawOutputParsedAsError && !process.env.SUPPRESS_FULL_WATERLINE_ERROR_REPORTING) {

so as to allow the ability to opt out of this level of verbosity of the error logging.

What do you think?

@sailsbot
Copy link

Hi @shackbarth! It looks like you missed a step or two when you created your issue. Please edit your comment (use the pencil icon at the top-right corner of the comment box) and fix the following:

  • Verify "My issue title is concise, on-topic and polite."

As soon as those items are rectified, post a new comment (e.g. “Ok, fixed!”) below and we'll take a look. Thanks!

*If you feel this message is in error, or you want to debate the merits of my existence (sniffle), please contact inquiries@sailsjs.com

@shackbarth
Copy link
Author

ok fixed!

@rachaelshaw rachaelshaw transferred this issue from balderdashy/waterline Mar 6, 2019
@rachaelshaw rachaelshaw added the orm Related to models, datastores, orm config, Waterline, sails-hook-orm, etc. label Mar 6, 2019
@balderdashy balderdashy deleted a comment from sailsbot Mar 6, 2019
@balderdashy balderdashy deleted a comment from sailsbot Mar 6, 2019
@raqem
Copy link
Contributor

raqem commented Mar 6, 2019

Hi @shackbarth,
If you are willing to make a PR that would be great! We could then consider it further and test any edge cases before bringing it into the project.
Also as a heads up we are currently in the process of consolidating all of Sails related issues into one repo to make it easier for our devs to keep an eye on any issues.
Thank you!

@mikermcneil
Copy link
Member

@shackbarth I like this idea.

@mikermcneil
Copy link
Member

(although really, I don't think it needs to be an option- we could just make sure to exclude it)

@mikermcneil mikermcneil added proposal what do you think? Community feedback requested labels Apr 12, 2019
@alxndrsn
Copy link

alxndrsn commented Aug 5, 2019

@shackbarth what did you do about this in the end? I think this is the same issue as #4595, which I submitted balderdashy/sails-postgresql#283 to fix. However, the PR hasn't been accepted, and I'm looking for a long-term solution without forking sails-postgresql.

@sailsbot sailsbot removed the what do you think? Community feedback requested label Aug 5, 2019
@alxndrsn
Copy link

alxndrsn commented Aug 6, 2019

I've released a node module to fix this issue at https://www.npmjs.com/package/sails-postgresql-redacted. To use:

  1. replace sails-postgresql with sails-postgresql-redacted in package.json
  2. replace sails-postgresql with sails-postgresql-redacted in datastores.*.adapter config (in config/datastores.js and/or in config/env/*.js

@mortbauer
Copy link

Is this still open?

@eashaw
Copy link
Member

eashaw commented May 13, 2021

Hi @mortbauer, we leave it to the author to close the issue.

@shackbarth
Copy link
Author

I can confirm that the latest sails-postgresql has fixed this password leak issue. Thanks @alxndrsn and the balderdash team!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
helpful info or workaround orm Related to models, datastores, orm config, Waterline, sails-hook-orm, etc. proposal
Development

No branches or pull requests

9 participants