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

Enable /status and /ping for php-fpm #396

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

naresh-kumar-babu
Copy link
Contributor

Partial fix for #388

@jeffw16
Copy link
Member

jeffw16 commented Apr 26, 2024

Is this accessible to the general public? Or is it an internal route?

@yaronkoren
Copy link
Member

I'm guessing it's open to the public... but does it matter?

@jeffw16
Copy link
Member

jeffw16 commented Apr 29, 2024

In theory, only synthetic monitors should have access to heartbeat endpoints, not the general public. It's best to find a way to prevent bad actors from being able to access this endpoint.

@yaronkoren
Copy link
Member

@vedmaka or @pastakhov - any thoughts on this? This came from your code. Is enabling /status and /ping a potential security issue, and if so, how can it be resolved?

@jeffw16
Copy link
Member

jeffw16 commented Jun 6, 2024

I withdraw my concern as it seems like it's working just fine for WikiTeq over the past several months. We should ideally add an option for a firewall for these endpoints in the future. I'll go ahead and give my approval.

@jeffw16 jeffw16 self-requested a review June 6, 2024 16:25
@jeffw16
Copy link
Member

jeffw16 commented Jun 6, 2024

After discussion at the last meeting, we decided we should hold off on this until we have some clarity about adding a firewall if needed. Rescinding my approval for now. Sorry :(

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