security: configurable sudo prompt timeout + SSH config metadata redaction - #6547
security: configurable sudo prompt timeout + SSH config metadata redaction#6547win4r wants to merge 2 commits into
Conversation
…TIMEOUT The sudo password prompt timeout was hardcoded to 45 seconds in `_prompt_for_sudo_password`. In non-interactive automation (cron jobs, gateway messages, scheduled agents), an accidental sudo invocation would block the whole run for 45s before gracefully failing — wasted time on every scheduled tick. This adds `HERMES_SUDO_TIMEOUT` as an integer environment variable that controls the prompt timeout. Setting it to `0` skips the prompt entirely without blocking at all, which is the correct behaviour for cron and gateway sessions where there is no human at the keyboard anyway. - `HERMES_SUDO_TIMEOUT` unset → default 45s (existing behaviour) - `HERMES_SUDO_TIMEOUT=10` → 10s prompt - `HERMES_SUDO_TIMEOUT=0` → skip immediately, return empty password - `HERMES_SUDO_TIMEOUT=abc` → warn, fall back to 45s (typos can't silently disable the prompt) - negative values clamp to 0 The callback-based path (used by the interactive CLI) is also guarded: when timeout is 0 the callback is NOT invoked, so a `HERMES_SUDO_TIMEOUT=0` gateway session cannot accidentally prompt the user via the UI callback. Tests: `tests/tools/test_terminal_sudo_timeout.py` (12 cases) covering env parsing, fast-skip path, and callback suppression.
Reading `~/.ssh/config` via the `read_file` tool previously echoed the
entire contents verbatim, leaking:
- `IdentityFile` paths (tells an attacker exactly which key files to
steal — the single most valuable piece of info for credential theft)
- `ProxyJump` / `ProxyCommand` (enumerates internal bastion hosts and
the exact commands needed to reach protected networks)
- `IdentityAgent` / `CertificateFile` (similar metadata exposure)
PR NousResearch#4168 adds an approval-prompt pattern for `cat`/`head`/`tail` on
`.ssh/`, which covers the terminal-tool path. This patch adds the
complementary fix for the `read_file` path: sensitive directives are
masked via the existing `redact_sensitive_text` pipeline that already
handles API keys, tokens, and private key blocks.
The redaction is surgical by design. `Host`, `HostName`, `User`, and
`Port` remain visible because they are frequently needed for legitimate
debugging (and leaking them alone — without the identity file path —
does not materially help an attacker who already has read access).
Matching is case-insensitive per sshd_config(5) and multiline-aware so
each config line is evaluated independently. Values are replaced with
`***` so the structure of the config (which keywords are set, and
where) stays visible.
Tests: `tests/agent/test_redact.py::TestSSHConfigRedaction` (10 cases)
covering each directive, case-insensitivity, full-config structure
preservation, and a negative case ensuring prose mentions of the
keyword are NOT incorrectly redacted.
teknium1
left a comment
There was a problem hiding this comment.
Thanks for identifying two concrete hardening opportunities. The SSH-read premise remains live: current read_file output is redacted through tools/file_tools.py:1250 and tools/file_tools.py:1382-1385, but current agent/redact.py has no SSH-directive rule.
Problems
- The timeout implementation introduces
HERMES_SUDO_TIMEOUTattools/terminal_tool.py:129in the PR.AGENTS.mdexplicitly requires non-secret behavioral settings such as timeouts to useconfig.yaml, not new user-facingHERMES_*environment variables. - The terminal half is stale and conflicts with current architecture. Main now uses thread-local callback accessors and session-scoped password caching (
tools/terminal_tool.py:152-250), and prompts when either interactive mode or a callback is present (tools/terminal_tool.py:928-935). The PR test still patches the removed global callback attests/tools/test_terminal_sudo_timeout.py:112. - The SSH tests call the redactor directly; please cover the actual
read_fileboundary as well.
Suggested changes
- Salvage the SSH redaction as a focused, rebased change with a
read_fileregression test. - Rework any timeout configuration through
terminal.*config plumbing and the current callback/session-cache flow.
This is an automated hermes-sweeper review.
| values fall back to ``_DEFAULT_SUDO_TIMEOUT`` with a warning so a | ||
| typo in config cannot silently disable the prompt. | ||
| """ | ||
| raw = os.getenv("HERMES_SUDO_TIMEOUT", "").strip() |
There was a problem hiding this comment.
This makes a timeout a new user-facing HERMES_* setting. The repository policy requires non-secret behavioral configuration to live in config.yaml; please rework this through the terminal config path instead.
| return "should-not-be-returned" | ||
|
|
||
| import tools.terminal_tool as tt | ||
| monkeypatch.setattr(tt, "_sudo_password_callback", fake_callback) |
There was a problem hiding this comment.
Current main no longer has this module-global callback: callbacks are thread-local and registered through set_sudo_password_callback(). Rebase this test onto that public setter so it verifies the current callback path.
What does this PR do?
Two independent security hardenings surfaced by a user-authorised local security audit of
v0.8.0(13/15 attack vectors blocked; full defense-in-depth review passed).1.
fix(terminal): configurable sudo prompt timeout_prompt_for_sudo_passwordhardcoded a 45s timeout. In cron / gateway sessions an accidentalsudoinvocation would block the whole run for 45s before failing. This addsHERMES_SUDO_TIMEOUTas an integer env var:HERMES_SUDO_TIMEOUT=10HERMES_SUDO_TIMEOUT=0HERMES_SUDO_TIMEOUT=abcThe callback path (prompt_toolkit UI) is also guarded — when timeout is 0 the UI callback is NOT invoked, so a
HERMES_SUDO_TIMEOUT=0gateway session cannot accidentally prompt the user via the UI path either.2.
fix(redact): mask sensitive SSH config directives on file readReading
~/.ssh/configviaread_filepreviously echoed the entire file verbatim. That leaks:IdentityFile→ exact paths to the user's private keys (single most valuable enumeration target)ProxyJump/ProxyCommand→ internal bastion hosts and the commands needed to reach protected networksIdentityAgent/CertificateFile→ similar metadataPR #4168 (open) adds an approval-prompt pattern for
cat/head/tailon.ssh/, which covers the terminal-tool path. This patch is complementary: it adds theread_filepath by extending the existingredact_sensitive_textpipeline, which is the same layer that already masks API keys, tokens, DB connection strings, and private-key blocks.Redaction is intentionally surgical —
Host,HostName,User,Portstay visible because they are frequently needed for legitimate debugging, and leaking them without the identity-file path doesn't materially help an attacker. Matching is case-insensitive persshd_config(5).Before (today):
After:
Related Issue
No open issue — findings come from an authorised local audit. Reproduction steps are in the "How to Test" section below.
Fixes #
Type of Change
Changes Made
tools/terminal_tool.py: add_DEFAULT_SUDO_TIMEOUT,_get_sudo_timeout(), make_prompt_for_sudo_passwordhonourHERMES_SUDO_TIMEOUTagent/redact.py: add_SSH_CONFIG_SENSITIVE_REand apply it insideredact_sensitive_texttests/tools/test_terminal_sudo_timeout.py: 12 new teststests/agent/test_redact.py: 10 new tests inTestSSHConfigRedactionHow to Test
Fix #1 — sudo timeout reproduction:
Fix #2 — SSH config redaction reproduction:
Test suite:
Checklist
Code
fix(terminal):,fix(redact):)pytest tests/agent/test_redact.py tests/tools/test_terminal_*.py -qand all tests passDocumentation & Housekeeping
_prompt_for_sudo_password,_transform_sudo_commandnote the new env var;_SSH_CONFIG_SENSITIVE_REhas a full design-rationale commentCONTRIBUTING.md/AGENTS.mdunchanged, N/AHERMES_SUDO_TIMEOUTparsing is stdlib-only; the regex inredact.pyuses no platform-specific featuresRelationship to other open PRs
curl --data/cat .ssh/patterns toapproval.py. This PR is deliberately scoped to the orthogonal fixes (read-path masking + sudo timeout) so the two PRs do not conflict and can land in either order.