ouyangbo/hermes-agent
1
0
Fork 0
Code Issues Pull Requests Actions 10 Packages Projects Releases Wiki Activity

fix(hooks): single delivery_id across header+body, never follow redirects

Browse Source 
- delivery_id is now generated once per firing and used for both the
  X-Hermes-Delivery header and the signed body's delivery_id field —
  previously they were two different uuid4s, breaking receiver-side
  dedupe as documented.
- 3xx responses are no longer followed: urllib's default redirect
  handler converts a redirected POST into a body-less GET, silently
  dropping the signed payload. Redirects now log a misconfiguration
  warning and count as delivery failure (no retry).
- Docs: receiver-side replay-protection guidance (dedupe on
  delivery_id, timestamp freshness window) + redirect semantics.
- Tests: 5xx retry count, redirect-not-followed (sabotage-verified),
  header/body delivery_id equality.
This commit is contained in:
Teknium
2026-08-02 14:34:04 -07:00
parent 3829e34e23
commit 86fd6da1dc
3 changed files with 112 additions and 14 deletions
Download Patch File Download Diff File Expand all files Collapse all files
agent/outbound_webhooks.py
+42 -9
View File
@@ -383,15 +383,16 @@ def _make_callback(event: str, target: WebhookTarget):
if event in _TOOL_SCOPED_EVENTS:
if not target.matches_tool(kwargs.get("tool_name")):
return None
delivery_id = uuid.uuid4().hex
try:
body = _serialize_payload(event, kwargs)
body = _serialize_payload(event, kwargs, delivery_id)
except Exception: # defensive — a bad payload must not hurt the loop
logger.warning(
"outbound webhook payload serialization failed (event=%s "
"target=%s)", event, target.label, exc_info=True,
)
return None
_enqueue(_build_delivery(event, target, body))
_enqueue(_build_delivery(event, target, body, delivery_id))
return None
_callback.__name__ = f"outbound_webhook[{event}:{target.label}]"
@@ -399,9 +400,16 @@ def _make_callback(event: str, target: WebhookTarget):
return _callback
def _serialize_payload(event: str, kwargs: Dict[str, Any]) -> bytes:
def _serialize_payload(
event: str, kwargs: Dict[str, Any], delivery_id: str,
) -> bytes:
"""Render the POST body. Same top-level shape as shell hooks' stdin
(documented in :mod:`agent.shell_hooks`), plus delivery metadata."""
(documented in :mod:`agent.shell_hooks`), plus delivery metadata.
``delivery_id`` is shared with the ``X-Hermes-Delivery`` header so
receivers can dedupe on either — and since it (plus ``timestamp``)
lives inside the HMAC-signed body, it doubles as replay protection.
"""
extras = {k: v for k, v in kwargs.items() if k not in _TOP_LEVEL_PAYLOAD_KEYS}
try:
cwd = str(Path.cwd())
@@ -414,7 +422,7 @@ def _serialize_payload(event: str, kwargs: Dict[str, Any]) -> bytes:
"session_id": kwargs.get("session_id") or kwargs.get("parent_session_id") or "",
"cwd": cwd,
"extra": extras,
"delivery_id": uuid.uuid4().hex,
"delivery_id": delivery_id,
"timestamp": datetime.now(tz=timezone.utc)
.isoformat()
.replace("+00:00", "Z"),
@@ -423,13 +431,13 @@ def _serialize_payload(event: str, kwargs: Dict[str, Any]) -> bytes:
def _build_delivery(
event: str, target: WebhookTarget, body: bytes,
event: str, target: WebhookTarget, body: bytes, delivery_id: str,
) -> Dict[str, Any]:
headers = {
"Content-Type": "application/json",
"User-Agent": "Hermes-Agent-Outbound-Webhook",
"X-Hermes-Event": event,
"X-Hermes-Delivery": uuid.uuid4().hex,
"X-Hermes-Delivery": delivery_id,
}
if target.secret:
digest = hmac.new(
@@ -486,9 +494,26 @@ def _worker_loop() -> None:
_delivery_queue.task_done()
class _NoRedirectHandler(urlrequest.HTTPRedirectHandler):
"""Refuse to follow redirects.
urllib's default handler converts a redirected POST into a body-less
GET — the signed payload would be silently dropped and the headers
re-sent to a location the user never configured. Treat any 3xx as a
delivery failure instead (surfaced as HTTPError by returning None).
"""
def redirect_request(self, req, fp, code, msg, headers, newurl): # noqa: D102
return None
_opener = urlrequest.build_opener(_NoRedirectHandler)
def _deliver(delivery: Dict[str, Any]) -> None:
"""POST with bounded retries. Retries on connection errors and 5xx;
4xx is the receiver telling us the request itself is wrong — no retry."""
4xx is the receiver telling us the request itself is wrong — no retry.
3xx redirects are never followed (misconfiguration — fix the URL)."""
last_error = ""
for attempt in range(1, MAX_DELIVERY_ATTEMPTS + 1):
req = urlrequest.Request(
@@ -498,7 +523,7 @@ def _deliver(delivery: Dict[str, Any]) -> None:
method="POST",
)
try:
with urlrequest.urlopen(req, timeout=delivery["timeout"]) as resp:
with _opener.open(req, timeout=delivery["timeout"]) as resp:
status = getattr(resp, "status", 200)
if 200 <= status < 300:
logger.debug(
@@ -509,6 +534,14 @@ def _deliver(delivery: Dict[str, Any]) -> None:
last_error = f"HTTP {status}"
except urlerror.HTTPError as exc:
last_error = f"HTTP {exc.code}"
if 300 <= exc.code < 400:
logger.warning(
"outbound webhook target redirected (event=%s target=%s): "
"%s -> %s — redirects are not followed; update the "
"configured url", delivery["event"], delivery["label"],
last_error, exc.headers.get("Location", "?"),
)
return
if 400 <= exc.code < 500:
logger.warning(
"outbound webhook rejected (event=%s target=%s): %s — "
tests/agent/test_outbound_webhooks.py
+63 -4
View File
@@ -56,12 +56,24 @@ class _CapturingHandler(BaseHTTPRequestHandler):
self.server.captured.append( # type: ignore[attr-defined]
{
"path": self.path,
"method": "POST",
"headers": dict(self.headers),
"body": body,
}
)
status = getattr(self.server, "respond_status", 200)
self.send_response(status)
location = getattr(self.server, "respond_location", None)
if location:
self.send_header("Location", location)
self.end_headers()
def do_GET(self): # noqa: N802 — records redirect follow-ups
self.server.captured.append( # type: ignore[attr-defined]
{"path": self.path, "method": "GET", "headers": dict(self.headers),
"body": b""}
)
self.send_response(200)
self.end_headers()
def log_message(self, format, *args): # noqa: A002 — http.server naming
@@ -275,6 +287,7 @@ class TestPayload:
"status": "ok",
"duration_ms": 42,
},
"did_1234",
)
payload = json.loads(body)
assert payload["hook_event_name"] == "post_tool_call"
@@ -283,12 +296,12 @@ class TestPayload:
assert payload["session_id"] == "sess_1"
assert payload["extra"]["status"] == "ok"
assert payload["extra"]["duration_ms"] == 42
assert payload["delivery_id"]
assert payload["delivery_id"] == "did_1234"
assert payload["timestamp"].endswith("Z")
def test_unserialisable_values_stringified(self):
body = outbound_webhooks._serialize_payload(
"on_session_end", {"weird": object()}
"on_session_end", {"weird": object()}, "did_1"
)
payload = json.loads(body)
assert isinstance(payload["extra"]["weird"], str)
@@ -418,11 +431,57 @@ class TestDelivery:
url=_url(http_server), events=["on_session_end"],
)
delivery = outbound_webhooks._build_delivery(
"on_session_end", target, b"{}",
"on_session_end", target, b"{}", "did_4xx",
)
outbound_webhooks._deliver(delivery)
assert len(http_server.captured) == 1
def test_5xx_retried_once(self, http_server):
http_server.respond_status = 500
target = outbound_webhooks.WebhookTarget(
url=_url(http_server), events=["on_session_end"],
)
delivery = outbound_webhooks._build_delivery(
"on_session_end", target, b"{}", "did_5xx",
)
outbound_webhooks._deliver(delivery)
assert len(http_server.captured) == outbound_webhooks.MAX_DELIVERY_ATTEMPTS
def test_redirect_not_followed(self, http_server):
"""3xx must be treated as failure — urllib's default handler would
convert a 302'd POST into a body-less GET at the redirect target."""
http_server.respond_status = 302
http_server.respond_location = _url(http_server, "/redirected")
target = outbound_webhooks.WebhookTarget(
url=_url(http_server), events=["on_session_end"],
)
delivery = outbound_webhooks._build_delivery(
"on_session_end", target, b"{}", "did_3xx",
)
outbound_webhooks._deliver(delivery)
# One POST hit the server (the redirect response), nothing followed
# (no GET to /redirected), no retry (misconfiguration, not transient).
assert [c["method"] for c in http_server.captured] == ["POST"]
assert http_server.captured[0]["path"] == "/hook"
def test_delivery_id_matches_header_and_body(self, http_server):
"""The X-Hermes-Delivery header and the signed body's delivery_id
must be the same value, or receiver-side dedupe breaks."""
cfg = _cfg(
{"url": _url(http_server), "events": ["on_session_end"],
"secret": "s"}
)
outbound_webhooks.register_from_config(cfg)
from hermes_cli.plugins import get_plugin_manager
get_plugin_manager().invoke_hook("on_session_end", session_id="s1")
assert outbound_webhooks.flush()
req = http_server.captured[0]
payload = json.loads(req["body"])
assert payload["delivery_id"] == req["headers"]["X-Hermes-Delivery"]
def test_connection_error_does_not_raise(self):
target = outbound_webhooks.WebhookTarget(
# Port 9 (discard) — nothing listening.
@@ -431,7 +490,7 @@ class TestDelivery:
timeout=1,
)
delivery = outbound_webhooks._build_delivery(
"on_session_end", target, b"{}",
"on_session_end", target, b"{}", "did_conn",
)
# Must swallow the failure (logged), never raise into the agent loop.
outbound_webhooks._deliver(delivery)
website/docs/user-guide/features/hooks.md
+7 -1
View File
@@ -1566,7 +1566,7 @@ Headers:
|--------|-------|
| `Content-Type` | `application/json` |
| `X-Hermes-Event` | The hook event name |
| `X-Hermes-Delivery` | Unique id per delivery (for idempotency on the receiver) |
| `X-Hermes-Delivery` | Unique id per delivery — same value as `delivery_id` in the body |
| `X-Hermes-Signature-256` | `sha256=<hex>` — HMAC-SHA256 of the raw body, GitHub-style; only present when a secret is configured |
Verify the signature exactly as you would a GitHub webhook:
@@ -1579,11 +1579,17 @@ def verify(body: bytes, header: str, secret: str) -> bool:
return hmac.compare_digest(expected, header)
```
Because `delivery_id` and `timestamp` live **inside the signed body**, a verified receiver also gets replay protection for free:
- **Dedupe** on `delivery_id` (or the matching `X-Hermes-Delivery` header) — remember recently seen ids and skip duplicates. Hermes retries failed deliveries once, so the same id can legitimately arrive twice.
- **Reject stale events** by checking `timestamp` against your clock with a tolerance window (5 minutes is the common default). An attacker replaying a captured request can't forge a fresh timestamp without the secret.
### Delivery semantics
- **Fire-and-forget, off the hot path.** Events are serialized and queued instantly; a single background thread performs the HTTP POSTs. A slow or dead endpoint can never stall a tool call or an agent turn.
- **Notify-only.** Unlike shell hooks, outbound webhooks cannot block tool calls or inject context — the response body is ignored. They observe, never steer.
- **Bounded retries.** Connection errors and 5xx responses are retried once with backoff; 4xx responses are not retried (the receiver said the request itself is wrong). Failures are logged and dropped — delivery is best-effort, not guaranteed.
- **Redirects are never followed.** A 3xx response is treated as a misconfiguration and logged — following a redirected POST would silently drop the signed payload. Point the `url` at the final endpoint.
- **Bounded queue.** If the queue backs up (dead endpoint, event storm), new events are dropped with a warning rather than consuming unbounded memory.
- **No consent prompt.** Outbound targets execute no code on your machine — they receive data at a URL you configured. `HERMES_SAFE_MODE=1` still skips registration, same as plugins and shell hooks. Note that payloads include tool inputs and event metadata, so only point targets at endpoints you trust, and prefer `https://`.