CallbackService.run() and arun() invoke a user-supplied script via
subprocess.run(...) / asyncio.create_subprocess_exec(...) with no timeout,
so a hung callback (e.g. a script that makes an HTTP call without its own
client-side timeout) will pin a Pulp worker indefinitely until the worker is
restarted or the process is killed externally.
Mitigations already in place:
- Callbacks are dispatched as their own Pulp tasks
(pulp_workflow.app.tasks.run_callback), so a hung callback ties up a worker
slot but does not stall the originating workflow's state machine.
- Only admins (
callbackservice_admin) can register callback scripts, so this
is a self-inflicted-foot-gun rather than an unauthenticated DoS surface.
Notes:
- This mirrors
pulpcore.app.models.SigningService.sign() / asign() exactly,
which also have no timeout. Adding a timeout knob here without doing the
same for SigningService upstream would diverge from the established
pattern.
- Suggested shape if/when this is addressed: a
WORKFLOW_CALLBACK_TIMEOUT
setting (default e.g. 300s), passed to subprocess.run(..., timeout=...) in
run() and wrapped with asyncio.wait_for(process.communicate(), ...) plus
process.kill() on TimeoutError in arun(). Ideally pushed upstream into
SigningService as well so both inherit the same behavior.
Originally raised by the Copilot reviewer on #11.
CallbackService.run()andarun()invoke a user-supplied script viasubprocess.run(...)/asyncio.create_subprocess_exec(...)with no timeout,so a hung callback (e.g. a script that makes an HTTP call without its own
client-side timeout) will pin a Pulp worker indefinitely until the worker is
restarted or the process is killed externally.
Mitigations already in place:
(
pulp_workflow.app.tasks.run_callback), so a hung callback ties up a workerslot but does not stall the originating workflow's state machine.
callbackservice_admin) can register callback scripts, so thisis a self-inflicted-foot-gun rather than an unauthenticated DoS surface.
Notes:
pulpcore.app.models.SigningService.sign()/asign()exactly,which also have no timeout. Adding a timeout knob here without doing the
same for
SigningServiceupstream would diverge from the establishedpattern.
WORKFLOW_CALLBACK_TIMEOUTsetting (default e.g. 300s), passed to
subprocess.run(..., timeout=...)inrun()and wrapped withasyncio.wait_for(process.communicate(), ...)plusprocess.kill()onTimeoutErrorinarun(). Ideally pushed upstream intoSigningServiceas well so both inherit the same behavior.Originally raised by the Copilot reviewer on #11.