feat(server): always disable /debug/pprof/cmdline, return 404#903
feat(server): always disable /debug/pprof/cmdline, return 404#903
Conversation
56quarters
left a comment
56quarters
left a comment
There was a problem hiding this comment.
Why is cmdline special? Why would any of /debug be public?
|
None of /debug should be public, but shit happens. When shit happens, I'd rather have someone reporting that they can see our profiles than reporting that they can see our AWS tokens. |
pracucci
left a comment
pracucci
left a comment
There was a problem hiding this comment.
Do we even need a config option to enable it? Is there any good use case for cmdline in our databases?
|
I never used it, I have to admit I didn't even know it existed, but who knows how people use dskit. I think not leaving an option is too radical. |
Options tend to proliferate with the Server in particular. If you add an option now it will never be removed because nobody will be confident enough. I'd rather explicitly disable anything we aren't expecting under |
colega
force-pushed
the
feat/disable-pprof-cmdline-by-default
branch
from
5cc5af6 to
7d56aa7
Compare
colega
changed the title
|
Okay, I removed the the option and made it respond 404 always. |
|
|
||
| func TestPprofCmdlineDisabled(t *testing.T) { | ||
| cfg := Config{} | ||
| cfg.RegisterFlags(flag.NewFlagSet("", flag.PanicOnError)) |
There was a problem hiding this comment.
This is done with flagext.DefaultValues(&cfg) elsewhere
There was a problem hiding this comment.
Actually this file doesn't use flagext.DefaultValues, but I don't think it's worth discussing this so I replaced it.
| cfg.RegisterFlags(flag.NewFlagSet("", flag.PanicOnError)) | ||
| setAutoAssignedPorts("tcp", &cfg) | ||
| cfg.Registerer = prometheus.NewPedanticRegistry() | ||
| cfg.Gatherer = prometheus.NewRegistry() |
There was a problem hiding this comment.
Why are these different objects?
There was a problem hiding this comment.
I removed them completely, the test passes without them.
Signed-off-by: Oleg Zaytsev <[email protected]>
Signed-off-by: Oleg Zaytsev <[email protected]>
4 participants
The
/debug/pprof/cmdlineendpoint exposes the full command line of the running process, which may contain sensitive information such as credentials passed as flags. This unconditionally disables the endpoint by returning 404, with no configuration option to re-enable it.Note
Low Risk
Small, localized change to debug routing plus a test; behavior change only impacts the
pprofcmdline endpoint.Overview
The server now unconditionally disables the
/debug/pprof/cmdlineendpoint by explicitly registering it to return404, while leaving other/debug/pprofroutes served fromhttp.DefaultServeMux.Adds a regression test (
TestPprofCmdlineDisabled) asserting/debug/pprof/cmdlinereturns404and that other pprof endpoints like/debug/pprof/and/debug/pprof/heapstill return200.Written by Cursor Bugbot for commit 0508ab2. This will update automatically on new commits. Configure here.