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

fix: prevent postgres credentials leak #3895

Merged
merged 5 commits into from
Jul 25, 2024

Conversation

hay-kot
Copy link
Collaborator

@hay-kot hay-kot commented Jul 15, 2024

What type of PR is this?

(REQUIRED)

  • bug

What this PR does / why we need it:

I received a security report that in some cases the API could return the password of the Postgres database. This only showed up when the user was an administrator. I modified the logic of the public URL to not use replace logic and just construct a dummer representation of the db url. Additionally, if they are deploying with a custom Postgres URL we just return a place holder value. I think this is the best approach instead of trying to parse out the username and password.

Which issue(s) this PR fixes:

Internal security report

Special notes for your reviewer:

Testing

  • Manual testing

Copy link
Collaborator

@michael-genson michael-genson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a reason we even need to include this in the API? Can we just return an empty string?

Approved since I think this is fine, but it may be simpler to just return no info unless we rely on it somewhere

@hay-kot hay-kot enabled auto-merge (squash) July 25, 2024 19:58
@hay-kot hay-kot merged commit edf649d into mealie-next Jul 25, 2024
11 checks passed
@hay-kot hay-kot deleted the security/prevent-postgres-credentials-leak branch July 25, 2024 20:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants