feat(ingester): restrict /flush and /shutdown endpoints to POST only#7674
feat(ingester): restrict /flush and /shutdown endpoints to POST only#7674Goutham-Annem wants to merge 4 commits into
Conversation
The /ingester/flush, /ingester/shutdown, /flush, and /shutdown endpoints previously accepted GET requests, making it possible to accidentally trigger a destructive ingester operation by clicking a link in a browser or by a pre-fetch proxy following links on the index page. Changes: - Route registration for flush/shutdown changed from ["GET","POST"] to ["POST"]. A browser GET (or curl without -X POST) now returns 405 Method Not Allowed. - Index page template updated to render links in the "Dangerous:" section as HTML <form method="POST"> submit buttons rather than plain <a href> anchors, preventing browser-initiated GET requests while keeping the UI functional. - tools/migrate-ingester-statefulsets.sh updated to use `wget --post-data=""` (BusyBox wget supports this flag) so the script continues to work after the GET handler is removed. - Two tests added/extended to assert the POST-form rendering for dangerous links. Fixes cortexproject#3243 Signed-off-by: Goutham Annem <gouthemannem@gmail.com>
|
this should trigger a change in the docs too https://cortexmetrics.io/docs/api/#shutdown |
Reflects the change introduced in pkg/api/api.go that removed GET from the allowed methods for /ingester/flush, /flush, /ingester/shutdown, and /shutdown. Browsers and scripts must now use POST to trigger these operations. Signed-off-by: Goutham Annem <gouthemannem@gmail.com>
|
Gentle ping — CI is passing, happy to address any feedback. |
|
Gentle ping — CI is passing and there are no conflicts. Happy to address any feedback! |
The /flush endpoint now only accepts POST (changed in feat cortexproject#3243). Update the integration test to call e2e.PostRequest instead of e2e.GetRequest so the test stays consistent with the enforced method restriction. Signed-off-by: Goutham Annem <gouthemannem@gmail.com>
friedrichg
left a comment
There was a problem hiding this comment.
I am ok with this.
But also brings the question of what should happen with /ingester/mode and /ingester/renewTokens
Signed-off-by: Goutham Annem <gouthemannem@gmail.com>
|
Good point — extended the PR to also restrict |
What this PR does:
The
/ingester/flush,/ingester/shutdown,/flush, and/shutdownendpoints previously acceptedGETrequests, making it possible to accidentally trigger a destructive ingester operation by clicking a link in a browser or via a pre-fetch proxy following the index page's links.This PR makes three coordinated changes:
Route restriction (
pkg/api/api.go): The four flush/shutdownRegisterRoutecalls now only list"POST". A browserGET(orcurlwithout-X POST) returns405 Method Not Allowed.Index page template (
pkg/api/handlers.go): Links in theDangerous:section now render as<form method="POST"><button>elements instead of<a href>anchors. This ensures that clicking in the browser issues aPOST, not aGET. Non-dangerous links continue to render as plain anchors. A template functionIsDangerous(section string) booldrives the branching.Migration script (
tools/migrate-ingester-statefulsets.sh): The BusyBoxwgetinvocation insidekubectl execis updated towget --post-data="" -T 5 .... BusyBox wget supports--post-data, so the script continues to work without requiringcurlinside the Cortex container image (which is unavailable per the original issue discussion).Which issue(s) this PR fixes:
Fixes #3243
Checklist
TestIndexPageContentextended with assertions that dangerous links producemethod="POST"forms and no<a href>; newTestIndexHandlerDangerousLinksUsePostFormscovers prefix behaviour for dangerous linksCHANGELOG.mdupdated