fix(core): kill sudo/root subprocess tree on task termination (RC#7)

[ocervell]

Problem (RC#7, prod 2026-07-10)

Celery app.control.revoke(ids, terminate=True, signal="SIGKILL") reported
success and inspect().active() found all tasks, but the tasks kept
running. Tools that run as sudo <tool> (e.g. sudo nmap -sS) execute as
root. The terminate SIGKILL hits the worker's non-root Python child, which
orphans the root tool (reparented to PID 1) instead of killing it — and the
orphan then loses secator's timeout-monitor thread, so it runs to natural
completion (up to ~91h for a paranoid all-ports nmap, ignoring even the 10h
cap). sudo pkill from inside the container also fails: the container's sudoers
is passwordless for the tool only, not pkill. Only tearing down the pod
killed them.

Root cause: a non-root worker cannot signal a root process
(os.kill/os.killpg → EPERM), and the tool wasn't in its own process group
so the signal never reached the whole sudo <tool> tree.

Fix (secator-core)

All termination paths route through Command.stop_process, so the fix lives
there plus the spawn:

1. Own session. The tool is now spawned with start_new_session=True
   (setsid) in the worker/passwordless case, so stop_process can killpg the
   whole group — sudo parent and tool child — in one shot. setsid is kept
   off only for a genuine interactive sudo password prompt (detaching the
   tty breaks it, the regression fix: broken sudo prompt because of os.setsid #722 fixed); the new condition is more precise
   than the old blanket "no setsid for any sudo".
2. Sudo-aware kill. When killpg/kill raises PermissionError (root-owned
   group), stop_process escalates to sudo -n kill -<sig> -<pgid> so the root
   tool actually dies. Best-effort and idempotent — logs and moves on if not
   permitted.
3. Worker SIGTERM handler. revoke(terminate=True) sends SIGTERM to the
   pool child; without a handler it just dies and orphans the tool. A worker-only,
   main-thread-guarded SIGTERM handler now invokes the cleanup, then restores the
   previous (billiard) handler. This also fixes the timeout-monitor path that let
   the orphaned nmap run unbounded.

Required deploy follow-ups (NOT changed here — code side only)

Pick one; the code supports all:

• (recommended) Drop sudo, use Linux capabilities. Give the nmap binary
  cap_net_raw,cap_net_admin+eip (setcap at image build) and run it without
  sudo. The worker then owns the process and kills it directly — no orphan, no
  sudo-kill. Cleanest.
• Passwordless sudo kill. Add a sudoers entry allowing the worker user to
  run kill passwordless, to enable the escalation in (2).
• Backstop for SIGKILL-terminate. SIGKILL is uncatchable in-process; set a
  real Kubernetes Job activeDeadlineSeconds (and prefer terminating with
  SIGTERM, not SIGKILL) so any orphan is bounded.

Tests

tests/unit/test_command.py::TestStopProcessKillsTree spawns a real
long-running child, calls stop_process, and asserts the whole process group is
gone (and that the tool is its own group leader). A sudo-wrapped variant
asserts the escalation path and skips gracefully where passwordless sudo is
unavailable (e.g. CI).

tests/unit/test_command.py::TestStopProcessKillsTree::test_setsid_group_kill_non_sudo PASSED
tests/unit/test_command.py::TestStopProcessKillsTree::test_sudo_group_kill SKIPPED
55 passed, 1 skipped  (test_command.py + test_runners.py)

Open decisions

• SIGKILL-terminate cannot be cleaned up from inside Python by design — deploy
  backstop required (documented above). The in-process fix targets the catchable
  paths (SIGTERM terminate, timeout/memory monitor, user stop, exceptions).
• The sudo -n kill escalation is a no-op without the sudoers entry. If you go
  the capabilities route instead, it's never exercised.

🤖 Generated with Claude Code

https://claude.ai/code/session_01MtTyzcUmPYxM5nfp7MnMVd

Summary by CodeRabbit

• Bug Fixes

  • Improved command termination so stopping a task reliably terminates the complete subprocess tree.
  • Fixed orphaned processes when tasks are terminated from worker environments.
  • Improved handling of commands launched with elevated privileges, including permission-related termination failures.
  • Preserved interactive elevated-command behavior while improving cleanup for non-interactive tasks.

• Tests

  • Added coverage verifying process trees are fully terminated for standard and elevated commands.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 7d773478-67a2-42ff-a250-eee26f535ebf

📥 Commits

Reviewing files that changed from the base of the PR and between 8fcafa6 and 195b1fd.

📒 Files selected for processing (2)

• secator/runners/command.py
• tests/unit/test_command.py

---

Walkthrough

Command now creates subprocess sessions conditionally, terminates process groups or PIDs with sudo escalation, and installs worker-only SIGTERM handling. Tests cover non-sudo and sudo process-tree termination.

Changes

Command process lifecycle

Layer / File(s)	Summary
Subprocess sessions and tree termination secator/runners/command.py, tests/unit/test_command.py	Popen session setup and stop_process() now terminate subprocess trees through process groups, PID fallback, or best-effort sudo escalation; tests cover non-sudo and sudo execution.
Worker termination signal lifecycle secator/runners/command.py	Worker execution installs a SIGTERM handler that stops the tool process tree and restores the previous handler during cleanup.

Estimated code review effort: 4 (Complex) | ~45 minutes

Sequence Diagram(s)

sequenceDiagram
  participant Worker
  participant Command
  participant ProcessGroup
  Worker->>Command: terminate task
  Command->>Command: _on_worker_term
  Command->>ProcessGroup: stop_process(SIGKILL)
  ProcessGroup-->>Command: process tree terminated
  Command->>Worker: restore previous SIGTERM handler

Loading

Possibly related PRs

• freelabz/secator#1053: Updates related Command.stop_process process-tree termination behavior.

Poem

A rabbit watched the process tree sway,
Then signaled the strays to hop away.
Groups now fall, sudo roots comply,
Worker-term hops safely by.
Restore the handler—clean and bright!
The command sleeps through the night. 🐇

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly matches the main change: fixing termination of sudo/root subprocess trees during task cleanup.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch fix/revoke-kills-sudo-tasks

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}