Skip to content

fix(cli): report passwordless sudo in status - #18315

Open
rylena wants to merge 17 commits into
NousResearch:mainfrom
rylena:fix/issue-18110-status-passwordless-sudo
Open

fix(cli): report passwordless sudo in status#18315
rylena wants to merge 17 commits into
NousResearch:mainfrom
rylena:fix/issue-18110-status-passwordless-sudo

Conversation

@rylena

@rylena rylena commented May 1, 2026

Copy link
Copy Markdown
Contributor

Summary

  • detect passwordless sudo with a short sudo -n true probe when SUDO_PASSWORD is unset
  • keep the status output explicit about whether sudo is enabled via env var or passwordless sudo
  • add focused status tests for passwordless sudo, env-based sudo, missing sudo, and probe failures

Verification

  • scripts/run_tests.sh tests/hermes_cli/test_status.py tests/hermes_cli/test_status_model_provider.py tests/cli/test_cli_status_command.py
  • independent delegated code review: passed

Closes #18110

@alt-glitch alt-glitch added type/bug Something isn't working P3 Low — cosmetic, nice to have comp/cli CLI entry point, hermes_cli/, setup wizard labels May 1, 2026
@alt-glitch

Copy link
Copy Markdown
Collaborator

Related to #12431 (same scope — sudo status display when env var is empty/unset). This PR supersedes it with a full sudo -n probe approach.

@alt-glitch

Copy link
Copy Markdown
Collaborator

Related to #12431

@rylena

rylena commented May 2, 2026

Copy link
Copy Markdown
Contributor Author

Rebased this PR onto the current main via a merge commit and re-ran verification.

Verification on the refreshed branch:

  • scripts/run_tests.sh tests/hermes_cli/test_status.py tests/hermes_cli/test_status_model_provider.py tests/cli/test_cli_status_command.py
  • scripts/run_tests.sh tests/acp/test_server.py::TestSessionOps::test_send_available_commands_update tests/gateway/test_teams.py::TestTeamsSend::test_send_typing tests/plugins/test_kanban_dashboard_plugin.py::test_ws_events_rejects_when_token_required tests/run_agent/test_concurrent_interrupt.py::test_concurrent_interrupt_cancels_pending tests/run_agent/test_concurrent_interrupt.py::test_running_concurrent_worker_sees_is_interrupted tests/tools/test_dockerfile_pid1_reaping.py::test_dockerfile_installs_tui_dependencies tests/tools/test_dockerfile_pid1_reaping.py::test_dockerfile_materializes_local_tui_ink_package → same 6 failures seen on current origin/main
  • independent delegated code review: passed

I also reproduced those same 6 failures in a clean worktree at current origin/main, so they appear to be baseline failures unrelated to this PR’s sudo-status change.

@rylena

rylena commented May 5, 2026

Copy link
Copy Markdown
Contributor Author

Refreshed this PR onto the current main and kept the sudo-status change intact. I also added one extra regression test for the sudo -n true nonzero-return path so the disabled case is covered without relying only on exceptions.

Verification on the refreshed branch:

  • scripts/run_tests.sh tests/hermes_cli/test_status.py tests/hermes_cli/test_status_model_provider.py tests/cli/test_cli_status_command.py
  • scripts/run_tests.sh tests/acp/test_server.py::TestSessionOps::test_send_available_commands_update tests/gateway/test_teams.py::TestTeamsSend::test_send_typing tests/run_agent/test_concurrent_interrupt.py::test_concurrent_interrupt_cancels_pending tests/tools/test_dockerfile_pid1_reaping.py::test_dockerfile_materializes_local_tui_ink_package → 2 passed, 2 failed
  • clean baseline comparison in a detached worktree at current origin/main with the same two failing tests (tests/run_agent/test_concurrent_interrupt.py::test_concurrent_interrupt_cancels_pending and tests/tools/test_dockerfile_pid1_reaping.py::test_dockerfile_materializes_local_tui_ink_package) reproduced the same failures there as well, so they appear unrelated to this PR
  • independent delegated code review: passed

So this PR-specific change is still green on its relevant status coverage, while the remaining red checks I sampled are baseline failures on current main.

@rylena

rylena commented May 9, 2026

Copy link
Copy Markdown
Contributor Author

Refreshed this PR onto the current main and pushed the updated branch to my fork.

What I changed:

  • merged the latest origin/main into fix/issue-18110-status-passwordless-sudo
  • kept the sudo-status change itself unchanged

Verification on the refreshed branch:

  • scripts/run_tests.sh tests/hermes_cli/test_status.py tests/hermes_cli/test_status_model_provider.py tests/cli/test_cli_status_command.py
  • independent delegated code review: passed

Codex was attempted first for repo-safe PR review, but it hit sandbox / CA-bundle workspace failures and could not inspect the checkout, so Hermes handled the fallback and branch refresh directly.

I’ve pushed the refreshed branch, so CI should rerun against the current base.

@rylena

rylena commented May 10, 2026

Copy link
Copy Markdown
Contributor Author

Refreshed this PR onto the current upstream/main and pushed the updated branch to my fork.

What I changed:

  • merged the latest upstream/main into fix/issue-18110-status-passwordless-sudo
  • kept the sudo-status change itself unchanged

Verification on the refreshed branch:

  • scripts/run_tests.sh tests/hermes_cli/test_status.py tests/hermes_cli/test_status_model_provider.py tests/cli/test_cli_status_command.py
  • scripts/run_tests.sh tests/gateway/test_base_topic_sessions.py::TestBasePlatformTopicSessions::test_process_message_background_replies_in_same_topic ✅ (this was red on the stale branch before the refresh)
  • scripts/run_tests.sh tests/hermes_cli/test_tencent_tokenhub_provider.py::TestTencentTokenhubContextLength::test_hy3_preview_has_registered_context_length ✅ (this old change-detector failure is gone after refreshing to current main)
  • scripts/run_tests.sh tests/gateway/test_tts_media_routing.py::test_streaming_delivery_routes_telegram_flac_media_tag_to_document_sender still fails on the refreshed branch, and I reproduced the same failure in a clean worktree at current upstream/main, so that remaining red gateway/TTS failure appears to be a baseline failure unrelated to this PR
  • independent delegated code review: passed

Codex was attempted first for repo-safe PR review, but it hit the known sandbox / CA-bundle workspace failures and could not inspect the checkout, so Hermes handled the fallback refresh and verification directly.

@rylena

rylena commented May 10, 2026

Copy link
Copy Markdown
Contributor Author

One more note from the refreshed CI rerun: Windows footguns (blocking) is currently failing on tools/process_registry.py:588 (os.killpg(os.getpgid(proc.pid), signal.SIGKILL)). I reproduced that locally with python scripts/check-windows-footguns.py --all, and the same exact line is present on current upstream/main, so this is another baseline failure introduced by the refreshed base rather than by the sudo-status diff in this PR.

@rylena

rylena commented May 14, 2026

Copy link
Copy Markdown
Contributor Author

Refreshed this PR onto the current upstream/main and pushed the updated branch to my fork.

What I changed:

  • merged the latest upstream/main into fix/issue-18110-status-passwordless-sudo
  • kept the sudo-status change itself unchanged

Verification on the refreshed branch:

  • scripts/run_tests.sh tests/hermes_cli/test_status.py tests/hermes_cli/test_status_model_provider.py tests/cli/test_cli_status_command.py
  • python scripts/check-windows-footguns.py --all
  • independent delegated code review: passed

Codex was attempted first for repo-safe PR review, but it hit the known sandbox / CA-bundle workspace failures and could not inspect the checkout, so Hermes handled the fallback refresh and verification directly.

I’ve pushed the refreshed branch, so CI should rerun against the current base.

@rylena

rylena commented May 15, 2026

Copy link
Copy Markdown
Contributor Author

Refreshed this PR onto the current upstream/main and pushed the updated branch to my fork.

What I changed:

  • merged the latest upstream/main into fix/issue-18110-status-passwordless-sudo
  • kept the sudo-status behavior change, but relabeled successful sudo -n true detection as enabled (non-interactive) instead of enabled (passwordless) so cached sudo credentials are not misreported as true NOPASSWD configuration
  • hardened the Termux status test to explicitly treat sudo as unavailable there, so the new sudo probe does not interfere with the existing systemctl guard

Why this run was actionable:

  • the previously failing CI run on this PR was against a stale merge ref; its failing test names (test_warning_stored_for_gateway_replay, test_switch_model_preserves_config_context_length, test_matches_previous_manual_builtin_tool_set) no longer exist on current upstream/main, so a branch refresh was needed before judging the PR

Verification on the refreshed branch:

  • scripts/run_tests.sh tests/hermes_cli/test_status.py tests/hermes_cli/test_status_model_provider.py tests/cli/test_cli_status_command.py
  • independent delegated code review: passed

Codex was attempted first for repo-safe PR review, but it hit the known workspace / CA-bundle sandbox failures (Permission denied on project discovery and no native root CA certificates found), so Hermes handled the fallback refresh, fix, and verification directly.

@rylena

rylena commented May 16, 2026

Copy link
Copy Markdown
Contributor Author

Refreshed this PR onto the current upstream/main and pushed the updated branch to my fork.

What I changed:

  • merged the latest upstream/main into fix/issue-18110-status-passwordless-sudo
  • kept the sudo-status change itself intact

Why this run was actionable:

  • the previously failing CI on this PR was stale against an older base
  • before the refresh, I reproduced the Discord e2e failures locally on this branch
  • the same Discord e2e file passes cleanly in a detached worktree at current upstream/main, which pointed to branch drift rather than a PR-scoped sudo-status regression
  • after the merge refresh, both the previously failing Discord e2e tests and the previously failing provider-parity tests pass locally on this branch

Verification on the refreshed branch:

  • scripts/run_tests.sh tests/run_agent/test_provider_parity.py tests/e2e/test_discord_adapter.py ✅ (99 passed)
  • scripts/run_tests.sh tests/hermes_cli/test_status.py tests/hermes_cli/test_status_model_provider.py tests/cli/test_cli_status_command.py ✅ (20 passed)
  • independent delegated review: passed for the actual refreshed branch state after local verification of remotes/branch cleanliness

Codex was attempted first for repo-safe diagnosis, but it hit the known workspace / CA-bundle sandbox failures (Permission denied on repo discovery and cert bundle access), so Hermes handled the fallback refresh and verification directly.

CI should rerun now against the current base.

@rylena

rylena commented May 20, 2026

Copy link
Copy Markdown
Contributor Author

Refreshed this PR onto the current upstream/main and pushed the updated branch to my fork.

What I changed:

  • merged the latest upstream/main into fix/issue-18110-status-passwordless-sudo
  • kept the PR-specific sudo-status change intact

Why this run was actionable:

  • this PR branch was 72 commits behind upstream/main
  • the last Tests / test check on the prior head timed out and was cancelled at the old 20-minute workflow limit
  • current upstream/main raises that workflow timeout to 30 minutes and adds pytest timeouts, so refreshing to the new base was the smallest safe maintenance step

Verification on the refreshed branch:

  • scripts/run_tests.sh tests/hermes_cli/test_status.py tests/hermes_cli/test_status_model_provider.py tests/cli/test_cli_status_command.py ✅ (33 passed)
  • python scripts/check-windows-footguns.py --all
  • independent delegated code review: passed

Push / CI verification:

  • pushed head 0b0bbc17007061b11a399aaf496be1f95cc49b89 to my fork branch
  • confirmed the PR head SHA now matches that commit and new GitHub checks are queued on it

Codex was attempted first for repo-safe PR review, but it hit the known workspace / CA-bundle sandbox failures (Permission denied on repo discovery and no native root CA certificates found), so Hermes handled the fallback refresh and verification directly.

@rylena

rylena commented May 22, 2026

Copy link
Copy Markdown
Contributor Author

Refreshed this PR onto the current upstream/main and pushed the updated branch to my fork.

What I changed:

  • merged the latest upstream/main into fix/issue-18110-status-passwordless-sudo
  • kept the PR-specific sudo-status change intact

Why this run was actionable:

  • this PR branch was 111 commits behind upstream/main
  • the last Tests / test failure on the previous head was on a stale branch tip, so the smallest safe maintenance step was to refresh onto the current base and rerun PR-relevant verification

Verification on the refreshed branch:

  • scripts/run_tests.sh tests/hermes_cli/test_status.py tests/hermes_cli/test_status_model_provider.py tests/cli/test_cli_status_command.py ✅ (33 passed)
  • python scripts/check-windows-footguns.py --all
  • targeted diff hygiene for the PR-owned files: git diff --check origin/fix/issue-18110-status-passwordless-sudo...HEAD -- hermes_cli/status.py tests/hermes_cli/test_status.py
  • independent delegated review: passed

Push / CI verification:

  • pushed head 523b891767bb16d1574b4bae873de901ba28c8cd to my fork branch
  • confirmed the PR head SHA now matches that commit and fresh GitHub checks are running on it

Codex was attempted first for repo-safe PR inspection, but it hit the known hidden-workspace / CA-bundle sandbox failures (Permission denied on repo discovery and no native root CA certificates found), so Hermes handled the fallback refresh and verification directly.

@rylena

rylena commented May 23, 2026

Copy link
Copy Markdown
Contributor Author

Refreshed this PR onto the current upstream/main and pushed the updated branch to my fork.

What I changed:

  • merged the latest upstream/main into fix/issue-18110-status-passwordless-sudo
  • kept the PR-specific sudo-status change intact

Why this run was actionable:

  • the previous failing Tests / test job on this PR was dominated by transcription/STT failures
  • those exact failing transcription suites now pass cleanly on current upstream/main, so the smallest safe maintenance step was to refresh this PR onto the current base and rerun the relevant coverage

Verification on the refreshed branch:

  • scripts/run_tests.sh tests/hermes_cli/test_status.py tests/hermes_cli/test_status_model_provider.py tests/cli/test_cli_status_command.py ✅ (33 passed)
  • python scripts/check-windows-footguns.py --all
  • scripts/run_tests.sh tests/tools/test_transcription.py tests/tools/test_transcription_tools.py tests/tools/test_transcription_dotenv_fallback.py ✅ (131 passed)
  • git diff --check origin/fix/issue-18110-status-passwordless-sudo...HEAD -- hermes_cli/status.py tests/hermes_cli/test_status.py
  • independent delegated code review: passed

Push / CI verification:

  • pushed head 84394b4e545d70cda0667eb736346b8ad414ccad to my fork branch
  • confirmed the PR head SHA now matches that commit and fresh GitHub checks are queued on it

Codex was attempted first for repo-safe PR inspection, but it hit the known hidden-workspace / CA-bundle sandbox failures (Permission denied on repo discovery and no native root CA certificates found), so Hermes handled the fallback refresh and verification directly.

@rylena

rylena commented May 24, 2026

Copy link
Copy Markdown
Contributor Author

Synced this PR branch with the latest upstream/main to clear stale branch drift that was causing unrelated CI noise.

What I checked:

  • inspected the failing GitHub Actions log from run 26328848939
  • confirmed the red tests were in tests/hermes_cli/test_kanban_notify.py, outside this PR's actual diff
  • confirmed this PR still only changes:
    • hermes_cli/status.py
    • tests/hermes_cli/test_status.py
  • merged current upstream/main into this branch and pushed the refresh

Local verification after the refresh:

  • scripts/run_tests.sh tests/hermes_cli/test_status.py tests/hermes_cli/test_status_model_provider.py tests/cli/test_cli_status_command.py
  • scripts/run_tests.sh tests/hermes_cli/test_kanban_notify.py
  • python scripts/check-windows-footguns.py --all

Those all passed locally, and fresh checks are now queued on the updated PR head.

@teknium1 teknium1 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for addressing the missing local sudo-status signal. The current-main premise is valid: hermes_cli/status.py:429-430 still treats only SUDO_PASSWORD as enabled.

Problems

  • hermes_cli/status.py:107 probes the host process unconditionally even though the status view has already selected a terminal backend at hermes_cli/status.py:411-415. The existing terminal contract explicitly limits this probe to local because Docker/SSH/Modal/etc. must not inherit host sudo state (tools/terminal_tool.py:680-703), with a regression test at tests/tools/test_terminal_tool.py:179-187.

Suggested changes

  • Gate this detection to the local backend (or present remote availability as unknown rather than using the host result), and add a status test that a remote backend does not call the host sudo probe.

Automated hermes-sweeper review.

Comment thread hermes_cli/status.py
)
except Exception:
return False, "disabled"

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This host subprocess probe needs a TERMINAL_ENV == "local" guard. tools/terminal_tool.py:680-703 deliberately forbids inheriting host sudo state for Docker/SSH/Modal/etc.; otherwise this status row can describe the host rather than the configured execution backend.

@teknium1 teknium1 added sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:blast-contained Sweeper blast radius: contained — one narrow path / opt-in / few users labels Jul 12, 2026
@rylena

rylena commented Jul 20, 2026

Copy link
Copy Markdown
Contributor Author

Addressed the sweeper feedback about remote terminal backends inheriting host sudo state.\n\nWhat changed:\n- _sudo_status() now only probes host sudo for the local terminal backend\n- non-local backends report sudo as unknown (remote backend) instead of reading SUDO_PASSWORD / sudo -n true from the host\n- added a regression test that a Docker backend does not call shutil.which("sudo") or subprocess.run() for the sudo line\n\nVerification run locally:\n- scripts/run_tests.sh tests/hermes_cli/test_status.py tests/hermes_cli/test_status_model_provider.py tests/cli/test_cli_status_command.py → 34 passed\n- python scripts/check-windows-footguns.py --all → passed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/cli CLI entry point, hermes_cli/, setup wizard P3 Low — cosmetic, nice to have sweeper:blast-contained Sweeper blast radius: contained — one narrow path / opt-in / few users sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

hermes status reports sudo disabled when passwordless sudo works

3 participants