Conversation
… payloads sweeping POST /webhooks/tautulli was registered on the parent router, outside the authenticated group, so it required no credentials at all. A payload with no media id then fell through to svc.RunNow(...) -- a full repair sweep using the operator's configured settings. With destructive repair actions enabled, an unauthenticated request could therefore mass-delete. Two changes: - Mount the webhooks under a router that requires the existing API token. No new auth scheme: the standard Authorization: Bearer|Token header is accepted, and so are an X-API-Token header and a ?token= / ?apikey= query parameter, because notification agents vary in what they can attach to an outgoing request and a fix nobody can adopt is not a fix. use_auth=false is unchanged -- the whole server is intentionally open in that mode. - Reject a payload carrying no media id with 400 instead of sweeping. Nothing about a Tautulli notification implies "sweep the entire library". The check now runs before the repair service is looked up, so no path remains from an untargeted payload to a sweep. Tests cover both, plus the accepted credential forms and the use_auth=false passthrough. They construct the server with a nil manager deliberately: both behaviours must be decided before the repair service is reached, so reaching it is itself the failure. Against the previous behaviour the auth test does not merely fail -- it panics inside s.manager.Repair(), which is the defect stated as a stack trace.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What breaks:
POST /webhooks/tautullirequires no credentials, and an untargeted payload sent to it starts a full repair sweep. With destructive repair actions enabled, an unauthenticated request can mass-delete.Why: the route is registered on the parent router in
pkg/server/server.go, outside the authenticated group — every other mutating endpoint sits inside it. Separately,handleTautullitreats "no media id" as "sweep everything": it falls through tosvc.RunNow(manager.RepairRunOptions{}), which uses the operator's configured repair settings.Either alone is awkward; together they mean an anonymous
curlwith a two-field JSON body can delete library content.The fix: two changes, both small.
Authorization: Bearer|Token <token>header, anX-API-Tokenheader and a?token=/?apikey=query parameter are accepted — notification agents differ in what they can attach to an outgoing request, and requiring a form some senders cannot produce would just break existing setups.use_auth=falseis deliberately unchanged: the whole server is open in that mode and this endpoint is not special-cased around it.400instead of sweeping. Nothing about a Tautulli notification implies "sweep the entire library". The check now runs before the repair service is looked up, so there is no remaining path from an untargeted payload to a sweep.Evidence: running in production on a ~47,000-entry library. Tests cover unauthenticated and wrong-credential rejection, each accepted credential form, the targetless rejection, and the
use_auth=falsepassthrough.The tests construct the server with a nil manager on purpose: both behaviours must be decided before the repair service is ever reached, so reaching it is itself the failure. Run against the previous behaviour, the auth test doesn't merely fail — it panics inside
s.manager.Repair(), which is the defect rendered as a stack trace: an unauthenticated request arriving at the repair service.