mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-07-24 16:54:43 +00:00
8e4b5d8774
Follow-up hardening on top of the C14 cherry-picks (#57860/#44026/#66742/#60009): - Slack file downloads (_download_slack_file/_download_slack_file_bytes) now require an https URL on a Slack CDN host (files.slack.com, *.slack.com Enterprise Grid, *.slack-files.com legacy shares) before attaching the bot token. url_private/url_private_download only ever point at the Slack CDN, so a forged file object from a malicious workspace app or compromised event stream pointing the Bearer-token download at an arbitrary PUBLIC host (token exfiltration) is now refused — a hole #44026's generic private-IP SSRF check alone could not close. - The same two download paths now use create_ssrf_safe_async_client (from #57860) so the preflight-validated hostname is resolved once, validated, and dialed by IP — closing the DNS-rebinding TOCTOU window for the token-bearing inbound fetches as well. - #60009's slack_tokens.json permission warning is generalized into utils.warn_if_credential_file_broadly_readable() (POSIX-only, fail-quiet) and wired into the other read path with the same gap: google_chat's load_user_credentials(). google_chat already writes 0o600 via _write_private_json; the read-time warning covers hand-provisioned/legacy files. Nothing in-repo writes slack_tokens.json (user/OAuth-provisioned), so there is no write path to chmod for Slack. Security tests both directions: non-CDN/lookalike/http URLs and connect-time DNS rebinds are blocked before any TCP connect; real files.slack.com, Enterprise Grid, and slack-files.com URLs still reach the network layer; 0o600 files stay silent while 0o644/0o640 warn with a chmod hint. A/B: all 10 new download-guard tests fail with the hardening reverted and pass with it applied.
100 lines
3.4 KiB
Python
100 lines
3.4 KiB
Python
"""Read-time permission warnings for on-disk credential/token files.
|
|
|
|
``utils.warn_if_credential_file_broadly_readable`` is the shared helper for
|
|
the class of bug PR #60009 reported for ``slack_tokens.json``: token files
|
|
provisioned by hand (or written by older Hermes versions without an explicit
|
|
mode) end up group/world-readable under the default umask, silently exposing
|
|
plaintext secrets to other local users. The helper warns with a remediation
|
|
hint; adapters call it on every read path.
|
|
"""
|
|
|
|
import logging
|
|
import os
|
|
|
|
import pytest
|
|
|
|
from utils import warn_if_credential_file_broadly_readable
|
|
|
|
pytestmark = pytest.mark.skipif(
|
|
os.name != "posix", reason="POSIX permission-bit semantics required"
|
|
)
|
|
|
|
|
|
class TestWarnIfCredentialFileBroadlyReadable:
|
|
def test_warns_on_world_readable(self, tmp_path, caplog):
|
|
f = tmp_path / "slack_tokens.json"
|
|
f.write_text("{}")
|
|
f.chmod(0o644)
|
|
|
|
with caplog.at_level(logging.WARNING):
|
|
warned = warn_if_credential_file_broadly_readable(f, label="[Slack]")
|
|
|
|
assert warned is True
|
|
assert "group/world-readable" in caplog.text
|
|
assert "chmod 600" in caplog.text
|
|
assert "[Slack]" in caplog.text
|
|
|
|
def test_warns_on_group_readable(self, tmp_path, caplog):
|
|
f = tmp_path / "tokens.json"
|
|
f.write_text("{}")
|
|
f.chmod(0o640)
|
|
|
|
with caplog.at_level(logging.WARNING):
|
|
assert warn_if_credential_file_broadly_readable(f) is True
|
|
|
|
def test_silent_on_0600(self, tmp_path, caplog):
|
|
f = tmp_path / "tokens.json"
|
|
f.write_text("{}")
|
|
f.chmod(0o600)
|
|
|
|
with caplog.at_level(logging.WARNING):
|
|
assert warn_if_credential_file_broadly_readable(f) is False
|
|
assert "group/world-readable" not in caplog.text
|
|
|
|
def test_silent_on_missing_file(self, tmp_path, caplog):
|
|
with caplog.at_level(logging.WARNING):
|
|
assert (
|
|
warn_if_credential_file_broadly_readable(tmp_path / "nope.json")
|
|
is False
|
|
)
|
|
assert caplog.text == ""
|
|
|
|
def test_uses_provided_logger(self, tmp_path):
|
|
f = tmp_path / "tokens.json"
|
|
f.write_text("{}")
|
|
f.chmod(0o644)
|
|
|
|
records = []
|
|
|
|
class _Sink(logging.Handler):
|
|
def emit(self, record):
|
|
records.append(record)
|
|
|
|
log = logging.getLogger("test.credfile.sink")
|
|
log.addHandler(_Sink())
|
|
try:
|
|
assert warn_if_credential_file_broadly_readable(f, log=log) is True
|
|
finally:
|
|
log.handlers.clear()
|
|
assert records and "chmod 600" in records[0].getMessage()
|
|
|
|
|
|
class TestGoogleChatReadPathWarns:
|
|
def test_load_user_credentials_warns_on_broad_perms(
|
|
self, tmp_path, monkeypatch, caplog
|
|
):
|
|
"""The google_chat legacy token read path shares the Slack fix."""
|
|
monkeypatch.setenv("HERMES_HOME", str(tmp_path))
|
|
# google-auth may not be installed in this environment; the warning
|
|
# fires before the import guard, so a None return is fine either way.
|
|
token = tmp_path / "google_chat_user_token.json"
|
|
token.write_text("{}")
|
|
token.chmod(0o644)
|
|
|
|
from plugins.platforms.google_chat.oauth import load_user_credentials
|
|
|
|
with caplog.at_level(logging.WARNING):
|
|
load_user_credentials()
|
|
|
|
assert "group/world-readable" in caplog.text
|
|
assert "chmod 600" in caplog.text
|