fix(command): authoritative sudo required

[bpce-it-dis-soc-sof]

Some commands might use sudo in their options set, like for running RCEs with custom pentest tools.

E.g:

zabbix_get -k 'system.run[sudo -l]'

Currently, that would mean the system autodetect sudo in the cmd string, and pre-prends 'sudo' to the command, which is not the desired outcome in that case.

Summary by CodeRabbit

• Bug Fixes
  • Improved sudo password prompting so it now follows the command’s actual sudo requirement instead of checking for the word “sudo” in the command text.
  • Reduced unnecessary password prompts when sudo is not required, while keeping prompts enabled when security settings call for them.

[coderabbitai]

Important

Review skipped

Auto incremental reviews are disabled on this repository.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: bb6d5268-4f0b-48e9-b048-6b11f5afeafc

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

• 🔍 Trigger review

Walkthrough

The sudo-password prompting logic in Command.yielder was changed to check the self.sudo_required instance attribute instead of regex-scanning self.cmd for "sudo", gating the _prompt_sudo call accordingly when CONFIG.security.prompt_sudo_password is enabled.

Changes

Sudo Detection Update

Layer / File(s)	Summary
Sudo prompt condition secator/runners/command.py	Sudo password prompting now relies on self.sudo_required instead of a regex match against self.cmd.

Estimated code review effort: 1 (Trivial) | ~3 minutes

Possibly related PRs

• freelabz/secator#722: Both PRs update Command.yielder's sudo detection/prompting logic and conditions.
• freelabz/secator#901: Both PRs modify sudo handling behavior in Command.yielder.

Poem

A sudo check, once regex-bound,
Now trusts a flag that's simply found. 🐇
No more scanning strings so long,
Just sudo_required—clean and strong!
Hop, hop, hooray for tidier code~

🚥 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 is concise and matches the change from string-based sudo detection to using the command's explicit sudo requirement.
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 unit tests (beta)

• Create PR with unit tests

---

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.}

[ocervell]

Thanks for the PR !

@claude please check if any task, tool installs, or addon installs, or raw Command.execute() blocks could be impacted by this change.

[ocervell]

@claude it was actually self.requires_sudo. Fixed. Can you check if that impacts anything ?

[claude]

Claude finished @ocervell's task in 2m 36s —— View job

---

Checked impact of the self.requires_sudo fix

Todo list:

• Review latest diff on branch (self.sudo_required → self.requires_sudo)
• Verify requires_sudo attribute exists, how it's set, and its default value
• Check for now-unused imports/dead code (e.g. the re regex removal)
• Check other call sites of Command.execute(), task installs, addon installs for impact
• Run lint / unit tests (see note below)
• Summarize findings

---

✅ The crash is fixed

self.requires_sudo is a real, pre-existing attribute:

• Class default False at secator/runners/command.py:133
• Set True statically in secator/tasks/arp.py:16, secator/tasks/arpscan.py:15
• Set conditionally in secator/tasks/naabu.py:65
• Set dynamically from option configs ('requires_sudo': True, e.g. many nmap.py options) at secator/runners/command.py:1220-1221
• Already used elsewhere in the same file, at line 254, to prepend sudo to self.cmd during init.

So both the AttributeError (line 506) and the NameError on the orphaned local sudo_required (line 546, now correctly using self.requires_sudo) are resolved. The import re at the top of the file is still used elsewhere (lines 611, 903, 1324+), so no dead-import concern.

This also correctly fixes the original report: zabbix_get -k 'system.run[sudo -l]' no longer false-positives, since detection is now based on the task's actual requires_sudo flag instead of a regex match against the literal built command string.

⚠️ One narrower, non-crashing regression found

Command.execute() (secator/runners/command.py:391) defaults requires_sudo=False unless explicitly passed via cls_attributes. A few auto-install code paths build a literal sudo <cmd> string themselves but never set cls_attributes={'requires_sudo': True}:

• secator/installer.py:197 (PackageInstaller.install) — prepends sudo to package-manager install commands when not root, then calls Command.execute(install_cmd, cls_attributes={'shell': True}, ...) at installer.py:404 without requires_sudo.
• secator/cli.py:2681 (install_go) — wget ... | sudo sh.
• secator/cli.py:2752-2754 (install tools --cleanup) — appends sudo {distro.pm_finalizer} to the cleanup command.

Before this PR, the old (buggy) regex incidentally caught these and triggered _prompt_sudo() (checks for passwordless sudo, prompts and pipes the password to the subprocess's stdin, or raises a clear "no TTY" error). After this fix, self.requires_sudo stays False for these ad-hoc instances, so _prompt_sudo is skipped entirely — sudo_password stays None, stdin isn't piped, and the child inherits the real stdin/TTY instead.

Impact is scoped (only non-root installs of certain package/source targets, not the whole install path — e.g. Docker/worker environments running as root never hit PackageInstaller's sudo branch at all), and it won't crash, but it does lose secator's own sudo-prompt/TTY-detection UX for these three call sites. If you want that back, the fix is to add 'requires_sudo': True to their cls_attributes dicts. Also worth noting: there's no existing test coverage for requires_sudo/_prompt_sudo/prompt_sudo_password in tests/, so neither the original bug nor this edge case is covered by CI.

Note: I wasn't able to run secator test lint/secator test unit in this job since no pre-installed venv/dependencies were available and a fresh install wasn't feasible in the time available — worth running those locally before merge.
(branch: sudo-fixes)

[ocervell]

@claude we do need to handle these cases by passing explicit requires_sudo attribute to the Command.execute (especially for package / source / tools installs as we don't want to break those, they are sensitive).

[claude]

Claude finished @ocervell's task in 2m 55s —— View job

---

Added explicit requires_sudo for install call sites

Todo list:

• Gather context on PR diff and current state of command.py
• Find all Command.execute() call sites for installs (package/source/tools/go)
• Add explicit requires_sudo to cls_attributes where the built command uses sudo
• Push changes
• Run lint/tests — not possible in this job (no pre-installed venv/dependencies and a fresh pip install -e ".[dev,worker]" isn't feasible in the time/tooling available here). Please run secator test lint and secator test unit locally before merging.

Changes (commit 7264a97)

The single choke point for all package/source installs is SourceInstaller.install() in secator/installer.py, which calls Command.execute(install_cmd, cls_attributes={'shell': True}, quiet=False) at what was line 404. This is reached by:

• PackageInstaller.install() (prepends sudo to the package manager command when not root, installer.py:197) → delegates to SourceInstaller.install()
• Direct source installs, including install_go() in cli.py (wget ... | sudo sh) via run_install() → SourceInstaller.install()

Fixed by detecting sudo in the built command and passing it through explicitly:

# secator/installer.py — SourceInstaller.install()
requires_sudo = bool(re.search(r'\bsudo\b', install_cmd))
ret = Command.execute(install_cmd, cls_attributes={'shell': True, 'requires_sudo': requires_sudo}, quiet=False)

This covers PackageInstaller and install_go since both funnel through this single call site — no need to touch each caller individually.

The other ad-hoc site that builds its own sudo command outside SourceInstaller is the install tools --cleanup path in cli.py (appends sudo {distro.pm_finalizer}), fixed the same way:

# secator/cli.py — install_tools(), --cleanup branch
requires_sudo = False
if distro.pm_finalizer:
    cleanup_cmds.append(f'sudo {distro.pm_finalizer}')
    requires_sudo = True
cmd = ' && '.join(cleanup_cmds)
Command.execute(cmd, cls_attributes={'shell': True, 'requires_sudo': requires_sudo}, quiet=False)

Why this is safe (no double sudo prepending): Command.execute() builds the instance first (cmd_instance = type(...)( **kwargs)), which runs __init__ — including the if self.requires_sudo: self.cmd = f'sudo {self.cmd}' step — before cls_attributes are applied via setattr afterward. Since these commands already have sudo baked into the literal string, and requires_sudo is only set after __init__ completes, the sudo prefix is never added twice; the flag only affects the later yielder() prompt/preexec_fn logic.

Other Command.execute() call sites in cli.py (worker start, http server, hatch build/publish, coverage combine, which/version checks, pipx/pip self-update) don't build sudo-containing commands, so they're unaffected.
(branch: sudo-fixes)