From 566d3e6d536ed7451249e40ac375c7d3528f83f1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Chlo=C3=A9=20DuPont?= <321112755+misschloedupont@users.noreply.github.com> Date: Tue, 25 Aug 2026 20:17:58 +0200 Subject: [PATCH 1/2] feat(webhook): read subscription secret from fd --- hermes_cli/subcommands/webhook.py | 23 ++- hermes_cli/webhook.py | 47 ++++++- .../hermes-agent/references/webhooks.md | 9 +- tests/hermes_cli/test_webhook_cli.py | 131 +++++++++++++++++- website/docs/user-guide/messaging/webhooks.md | 11 +- .../current/user-guide/messaging/webhooks.md | 5 +- 6 files changed, 212 insertions(+), 14 deletions(-) diff --git a/hermes_cli/subcommands/webhook.py b/hermes_cli/subcommands/webhook.py index 38085141b0697..09787141403e7 100644 --- a/hermes_cli/subcommands/webhook.py +++ b/hermes_cli/subcommands/webhook.py @@ -6,9 +6,22 @@ from __future__ import annotations +import argparse from typing import Callable +def _non_negative_fd(value: str) -> int: + if not value.isascii() or not value.isdecimal(): + raise argparse.ArgumentTypeError("must be a non-negative integer") + try: + fd = int(value) + except ValueError as exc: + raise argparse.ArgumentTypeError("must be a non-negative integer") from exc + if fd < 0: + raise argparse.ArgumentTypeError("must be a non-negative integer") + return fd + + def build_webhook_parser(subparsers, *, cmd_webhook: Callable) -> None: """Attach the ``webhook`` subcommand to ``subparsers``.""" # ========================================================================= @@ -45,9 +58,17 @@ def build_webhook_parser(subparsers, *, cmd_webhook: Callable) -> None: default="", help="Target chat ID for cross-platform delivery", ) - wh_sub.add_argument( + secret_group = wh_sub.add_mutually_exclusive_group() + secret_group.add_argument( "--secret", default="", help="HMAC secret (auto-generated if omitted)" ) + secret_group.add_argument( + "--secret-fd", + type=_non_negative_fd, + default=None, + metavar="FD", + help="Read HMAC secret from FD (UTF-8, max 4096 bytes; avoids argv exposure)", + ) wh_sub.add_argument( "--deliver-only", action="store_true", diff --git a/hermes_cli/webhook.py b/hermes_cli/webhook.py index 9b9de6cd5a6c2..f14a29bc0b58d 100644 --- a/hermes_cli/webhook.py +++ b/hermes_cli/webhook.py @@ -26,6 +26,7 @@ _SUBSCRIPTIONS_FILENAME = "webhook_subscriptions.json" _SUBSCRIPTIONS_FILE_MODE = 0o600 +_MAX_SECRET_BYTES = 4096 def _hermes_home() -> Path: @@ -137,6 +138,33 @@ def _require_webhook_enabled() -> bool: return False +def _read_secret_fd(fd: int) -> str | None: + """Read and normalize a bounded UTF-8 secret without taking ownership of *fd*.""" + data = bytearray() + try: + while len(data) <= _MAX_SECRET_BYTES: + chunk = os.read(fd, min(4096, _MAX_SECRET_BYTES + 1 - len(data))) + if not chunk: + break + data.extend(chunk) + except OSError: + print("Error: Could not read --secret-fd.") + return None + + if len(data) > _MAX_SECRET_BYTES: + print(f"Error: --secret-fd input exceeds {_MAX_SECRET_BYTES} bytes.") + return None + try: + secret = data.decode("utf-8").rstrip() + except UnicodeDecodeError: + print("Error: --secret-fd input must be valid UTF-8.") + return None + if not secret: + print("Error: --secret-fd input is empty after trimming trailing whitespace.") + return None + return secret + + def webhook_command(args): """Entry point for 'hermes webhook' subcommand.""" sub = getattr(args, "webhook_action", None) @@ -165,10 +193,23 @@ def _cmd_subscribe(args): print(f"Error: Invalid name '{name}'. Use lowercase alphanumeric with hyphens/underscores.") return + secret_arg = getattr(args, "secret", "") or "" + secret_fd = getattr(args, "secret_fd", None) + if secret_arg and secret_fd is not None: + print("Error: --secret and --secret-fd are mutually exclusive.") + return + if secret_fd is not None: + if not isinstance(secret_fd, int) or isinstance(secret_fd, bool) or secret_fd < 0: + print("Error: --secret-fd must be a non-negative integer.") + return + secret = _read_secret_fd(secret_fd) + if secret is None: + return + else: + secret = secret_arg or secrets.token_urlsafe(32) + subs = _load_subscriptions() is_update = name in subs - - secret = args.secret or secrets.token_urlsafe(32) events = [e.strip() for e in args.events.split(",")] if args.events else [] route = { @@ -205,7 +246,7 @@ def _cmd_subscribe(args): print(f"\n {status} webhook subscription: {name}") print(f" URL: {base_url}/webhooks/{name}") - print(f" Secret: {secret}") + print(f" HMAC secret stored in {_subscriptions_path()} (mode 0600).") if events: print(f" Events: {', '.join(events)}") else: diff --git a/skills/autonomous-ai-agents/hermes-agent/references/webhooks.md b/skills/autonomous-ai-agents/hermes-agent/references/webhooks.md index bd52ea68f22f3..c80bf2f91861b 100644 --- a/skills/autonomous-ai-agents/hermes-agent/references/webhooks.md +++ b/skills/autonomous-ai-agents/hermes-agent/references/webhooks.md @@ -64,10 +64,15 @@ hermes webhook subscribe \ --skills "skill1,skill2" \ --deliver telegram \ --deliver-chat-id "12345" \ - --secret "optional-custom-secret" + --secret-fd 3 3< /path/to/webhook-secret ``` -Returns the webhook URL and HMAC secret. The user configures their service to POST to that URL. +`--secret-fd` reads at most 4096 bytes of UTF-8, trims trailing whitespace, +and keeps the HMAC secret out of the command line and process listings. It is +mutually exclusive with the legacy `--secret` option. If neither option is +given, Hermes continues to generate a secret automatically. Secret values are +stored in the owner-only `~/.hermes/webhook_subscriptions.json` file and are +not printed by the command or by `hermes webhook list`. ### Filter or transform payloads before the agent runs diff --git a/tests/hermes_cli/test_webhook_cli.py b/tests/hermes_cli/test_webhook_cli.py index 4fecf7f279c70..b699010ff1eae 100644 --- a/tests/hermes_cli/test_webhook_cli.py +++ b/tests/hermes_cli/test_webhook_cli.py @@ -1,11 +1,13 @@ """Tests for hermes_cli/webhook.py — webhook subscription CLI.""" +import argparse import json import os import pytest import stat from argparse import Namespace +from hermes_cli.subcommands.webhook import build_webhook_parser from hermes_cli.webhook import ( webhook_command, _get_webhook_base_url, @@ -35,6 +37,7 @@ def _make_args(**kwargs): "deliver": "log", "deliver_chat_id": "", "secret": "", + "secret_fd": None, "payload": "", "script": "", } @@ -42,6 +45,14 @@ def _make_args(**kwargs): return Namespace(**defaults) +def _webhook_parser(): + parser = argparse.ArgumentParser(prog="hermes") + build_webhook_parser( + parser.add_subparsers(dest="command"), cmd_webhook=webhook_command + ) + return parser + + @pytest.mark.parametrize("host", [None, "", "0.0.0.0", "::"]) def test_webhook_base_url_maps_wildcard_hosts_to_localhost(monkeypatch, host): monkeypatch.setattr( @@ -54,17 +65,129 @@ def test_webhook_base_url_maps_wildcard_hosts_to_localhost(monkeypatch, host): class TestSubscribe: - def test_custom_secret(self): + def test_custom_secret_is_not_echoed(self, capsys): + secret = "legacy-argv-secret" webhook_command(_make_args( - webhook_action="subscribe", name="s", secret="my-secret" + webhook_action="subscribe", name="s", secret=secret )) - assert _load_subscriptions()["s"]["secret"] == "my-secret" + assert _load_subscriptions()["s"]["secret"] == secret + assert secret not in capsys.readouterr().out - def test_auto_secret(self): + def test_auto_secret_remains_default_and_is_not_echoed(self, capsys): webhook_command(_make_args(webhook_action="subscribe", name="s")) secret = _load_subscriptions()["s"]["secret"] assert len(secret) > 20 + assert secret not in capsys.readouterr().out + + def test_secret_fd_success_strips_trailing_newline_and_keeps_fd_open(self, capsys): + read_fd, write_fd = os.pipe() + secret = b"fd-provided-value" + try: + os.write(write_fd, secret + b"\n") + os.close(write_fd) + write_fd = -1 + + webhook_command( + _make_args(webhook_action="subscribe", name="fd-route", secret_fd=read_fd) + ) + + assert _load_subscriptions()["fd-route"]["secret"] == secret.decode() + assert secret.decode() not in capsys.readouterr().out + os.fstat(read_fd) + finally: + if write_fd >= 0: + os.close(write_fd) + os.close(read_fd) + + def test_secret_and_secret_fd_are_mutually_exclusive(self, capsys): + secret = "mutual-exclusion-value" + with pytest.raises(SystemExit): + _webhook_parser().parse_args( + ["webhook", "subscribe", "route", "--secret", secret, "--secret-fd", "3"] + ) + error = capsys.readouterr().err + assert "not allowed with argument" in error + assert secret not in error + + @pytest.mark.parametrize("value", ["-1", "+1", " 1", "1.0", "not-an-integer"]) + def test_secret_fd_rejects_invalid_values(self, value, capsys): + with pytest.raises(SystemExit): + _webhook_parser().parse_args( + ["webhook", "subscribe", "route", "--secret-fd", value] + ) + assert "--secret-fd" in capsys.readouterr().err + + def test_secret_fd_rejects_closed_fd_without_persisting(self, capsys): + read_fd, write_fd = os.pipe() + os.close(read_fd) + os.close(write_fd) + + webhook_command( + _make_args(webhook_action="subscribe", name="closed", secret_fd=read_fd) + ) + + assert "closed" not in _load_subscriptions() + assert capsys.readouterr().out == "Error: Could not read --secret-fd.\n" + + def test_secret_fd_rejects_oversize_without_echoing_input(self, tmp_path, capsys): + secret_file = tmp_path / "oversize-secret" + secret_file.write_bytes(b"x" * 4097) + with secret_file.open("rb") as fh: + webhook_command( + _make_args(webhook_action="subscribe", name="oversize", secret_fd=fh.fileno()) + ) + + assert "oversize" not in _load_subscriptions() + assert "x" * 32 not in capsys.readouterr().out + + def test_secret_fd_accepts_4096_byte_limit(self, tmp_path, capsys): + secret = "x" * 4096 + secret_file = tmp_path / "maximum-size-secret" + secret_file.write_text(secret, encoding="utf-8") + with secret_file.open("rb") as fh: + webhook_command( + _make_args(webhook_action="subscribe", name="maximum", secret_fd=fh.fileno()) + ) + + assert _load_subscriptions()["maximum"]["secret"] == secret + assert secret not in capsys.readouterr().out + + def test_secret_fd_rejects_non_utf8_without_echoing_input(self, tmp_path, capsys): + secret_file = tmp_path / "malformed-secret" + secret_file.write_bytes(b"prefix-\xff-suffix") + with secret_file.open("rb") as fh: + webhook_command( + _make_args(webhook_action="subscribe", name="malformed", secret_fd=fh.fileno()) + ) + + assert "malformed" not in _load_subscriptions() + assert "prefix" not in capsys.readouterr().out + + @pytest.mark.parametrize("contents", [b"", b"\n", b" \n\t"]) + def test_secret_fd_rejects_empty_normalized_secret(self, tmp_path, contents, capsys): + secret_file = tmp_path / "empty-secret" + secret_file.write_bytes(contents) + with secret_file.open("rb") as fh: + webhook_command( + _make_args(webhook_action="subscribe", name="empty", secret_fd=fh.fileno()) + ) + + assert "empty" not in _load_subscriptions() + assert "is empty after trimming" in capsys.readouterr().out + + def test_secret_fd_call_path_keeps_secret_out_of_argv(self, tmp_path, capsys): + secret = b"not-present-in-argv" + secret_file = tmp_path / "argv-free-secret" + secret_file.write_bytes(secret) + with secret_file.open("rb") as fh: + argv = ["webhook", "subscribe", "argv-free", "--secret-fd", str(fh.fileno())] + assert secret.decode() not in argv + args = _webhook_parser().parse_args(argv) + args.func(args) + + assert _load_subscriptions()["argv-free"]["secret"] == secret.decode() + assert secret.decode() not in capsys.readouterr().out class TestList: diff --git a/website/docs/user-guide/messaging/webhooks.md b/website/docs/user-guide/messaging/webhooks.md index dd1148b0fac12..b37f65b2eb32f 100644 --- a/website/docs/user-guide/messaging/webhooks.md +++ b/website/docs/user-guide/messaging/webhooks.md @@ -409,10 +409,17 @@ hermes webhook subscribe github-issues \ --prompt "New issue #{issue.number}: {issue.title}\nBy: {issue.user.login}\n\n{issue.body}" \ --deliver telegram \ --deliver-chat-id "-100123456789" \ - --description "Triage new GitHub issues" + --description "Triage new GitHub issues" \ + --secret-fd 3 3< /path/to/webhook-secret ``` -This returns the webhook URL and an auto-generated HMAC secret. Configure your service to POST to that URL. +`--secret-fd` reads at most 4096 bytes of UTF-8, trims trailing whitespace, +and keeps the HMAC secret out of the command line and process listings. It is +mutually exclusive with the legacy `--secret` option. If neither option is +given, Hermes continues to generate a secret automatically. Secret values are +stored in the owner-only `~/.hermes/webhook_subscriptions.json` file and are +not printed by this command or by `hermes webhook list`. Configure your service +to POST to the returned URL using the same secret. ### List subscriptions diff --git a/website/i18n/zh-Hans/docusaurus-plugin-content-docs/current/user-guide/messaging/webhooks.md b/website/i18n/zh-Hans/docusaurus-plugin-content-docs/current/user-guide/messaging/webhooks.md index 82597095fa18f..c00d9be0079d9 100644 --- a/website/i18n/zh-Hans/docusaurus-plugin-content-docs/current/user-guide/messaging/webhooks.md +++ b/website/i18n/zh-Hans/docusaurus-plugin-content-docs/current/user-guide/messaging/webhooks.md @@ -407,10 +407,11 @@ hermes webhook subscribe github-issues \ --prompt "New issue #{issue.number}: {issue.title}\nBy: {issue.user.login}\n\n{issue.body}" \ --deliver telegram \ --deliver-chat-id "-100123456789" \ - --description "Triage new GitHub issues" + --description "Triage new GitHub issues" \ + --secret-fd 3 3< /path/to/webhook-secret ``` -此命令返回 webhook URL 和自动生成的 HMAC secret。将你的服务配置为 POST 到该 URL。 +`--secret-fd` 最多读取 4096 字节的 UTF-8 内容，去除末尾空白，并避免在命令行和进程列表中暴露 HMAC secret。它与旧版 `--secret` 选项互斥。如果两个选项都未提供，Hermes 仍会自动生成 secret。secret 值存储在仅所有者可读写的 `~/.hermes/webhook_subscriptions.json` 文件中，不会由此命令或 `hermes webhook list` 输出。使用同一 secret 将你的服务配置为 POST 到返回的 URL。 ### 列出订阅 From 31c219e8eaa5d2e20cea89baa52bdfc20f5bbef5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Chlo=C3=A9=20DuPont?= <321112755+misschloedupont@users.noreply.github.com> Date: Tue, 25 Aug 2026 20:17:58 +0200 Subject: [PATCH 2/2] fix(webhook): bound invalid secret descriptors --- hermes_cli/webhook.py | 2 +- tests/hermes_cli/test_webhook_cli.py | 12 ++++++++++++ 2 files changed, 13 insertions(+), 1 deletion(-) diff --git a/hermes_cli/webhook.py b/hermes_cli/webhook.py index f14a29bc0b58d..a5a3521fce0fb 100644 --- a/hermes_cli/webhook.py +++ b/hermes_cli/webhook.py @@ -147,7 +147,7 @@ def _read_secret_fd(fd: int) -> str | None: if not chunk: break data.extend(chunk) - except OSError: + except (OSError, OverflowError): print("Error: Could not read --secret-fd.") return None diff --git a/tests/hermes_cli/test_webhook_cli.py b/tests/hermes_cli/test_webhook_cli.py index b699010ff1eae..129f399917724 100644 --- a/tests/hermes_cli/test_webhook_cli.py +++ b/tests/hermes_cli/test_webhook_cli.py @@ -130,6 +130,18 @@ def test_secret_fd_rejects_closed_fd_without_persisting(self, capsys): assert "closed" not in _load_subscriptions() assert capsys.readouterr().out == "Error: Could not read --secret-fd.\n" + def test_secret_fd_rejects_out_of_platform_range_without_traceback(self, capsys): + webhook_command( + _make_args( + webhook_action="subscribe", + name="out-of-range", + secret_fd=1 << 63, + ) + ) + + assert "out-of-range" not in _load_subscriptions() + assert capsys.readouterr().out == "Error: Could not read --secret-fd.\n" + def test_secret_fd_rejects_oversize_without_echoing_input(self, tmp_path, capsys): secret_file = tmp_path / "oversize-secret" secret_file.write_bytes(b"x" * 4097)