Skip to content

feat(webhook): accept subscription secret via file descriptor - #94953

Open
misschloedupont wants to merge 2 commits into
NousResearch:mainfrom
misschloedupont:feat/webhook-secret-fd
Open

feat(webhook): accept subscription secret via file descriptor#94953
misschloedupont wants to merge 2 commits into
NousResearch:mainfrom
misschloedupont:feat/webhook-secret-fd

Conversation

@misschloedupont

@misschloedupont misschloedupont commented Aug 25, 2026

Copy link
Copy Markdown

What changed

  • add --secret-fd for webhook subscription secrets supplied through an already-open file descriptor
  • make --secret-fd mutually exclusive with --secret
  • accept only non-negative integer descriptors and read at most 4096 bytes
  • require strict UTF-8 input without closing the caller-owned descriptor
  • return bounded, content-free errors for invalid, closed, unreadable, oversized, or platform-out-of-range descriptors

Why

Supplying an HMAC secret with --secret places it in process arguments, where it may be visible to process inspection and command history. --secret-fd lets an operator or supervisor provide the same secret without putting it in argv.

Security impact

This PR adds a secret-ingress path. It is intentionally bounded and fail-closed:

  • no secret content is included in output or errors
  • reads stop at 4096 bytes and reject oversized input
  • decoding is strict UTF-8
  • the caller retains ownership of the descriptor
  • subscription storage remains mode 0600

Existing --secret and automatically generated-secret behavior are unchanged.

How to test

Automated:

pytest -q tests/hermes_cli/test_webhook_cli.py

Manual CLI smoke with an isolated webhook-enabled HERMES_HOME:

exec 3<<<"synthetic-subscription-secret"
hermes webhook subscribe manual-fd-smoke \
  --prompt "manual smoke" \
  --secret-fd 3
exec 3<&-

Verify that the subscription is created, the secret is absent from CLI output, and webhook_subscriptions.json has mode 0600.

Observed manual results:

  • webhook subscription created through FD input
  • secret absent from output
  • subscription file mode 0600

Platforms tested

  • Linux 6.17, aarch64, Python 3.11: automated and manual CLI smoke passed
  • macOS and Windows/WSL2: not locally available; awaiting repository CI/maintainer workflow approval

Verification

  • 30 passed in tests/hermes_cli/test_webhook_cli.py
  • Ruff, compile, and git diff --check passed

Related work and duplicate search

Searched current source plus open and closed issues/PRs before submission. No issue or PR was found that adds webhook subscription secret input through --secret-fd.

@alt-glitch alt-glitch added type/feature New feature or request P3 Low — cosmetic, nice to have comp/cli CLI entry point, hermes_cli/, setup wizard platform/webhook Webhook / API server labels Aug 25, 2026
@Enough1122

Copy link
Copy Markdown
Contributor

AI code review — automated review for reference, author can ignore or act on any point.

The fd-based handoff is implemented with real care: the read is bounded with the +1-byte oversize probe, rejects non-UTF-8 and empty-after-trim inputs, and deliberately does not take ownership of the caller's fd (webhook.py:141-166) — and the tests pin each of those properties, including the "secret never appears in stdout/stderr" negative assertions and keeping the fd open so the caller's 3< file shell construct stays correct. The strict _non_negative_fd argparse validator rejecting +1/1.0 whitespace variants is a nice bit of argv hygiene.

Two usability-scope concerns, one small robustness note:

  1. The auto-generated secret is now unlearnable through the CLI. Previously subscribe printed the secret, which is how an operator configured the external service's HMAC. This PR changes the output to "HMAC secret stored in ..." (webhook.py:249) and the docs confirm webhook list also won't show it — so the only way to retrieve an auto-generated secret is to open ~/.hermes/webhook_subscriptions.json by hand. That may be a deliberate "file is the vault" posture, but it also breaks every existing script that captured the secret from stdout, and the docs flow ("Configure your service to POST using the same secret") now has a hole for the default, no---secret-fd path. Consider a webhook list --show-secrets (or subscribe --print-secret) escape hatch, or at minimum a release-note callout since this is a CLI contract change beyond the fd addition.

  2. rstrip() silently mutates legitimate secrets. (webhook.py:158) A secret ending in spaces or tabs — unusual but valid HMAC key material — is normalized differently from what the operator's file contains, while the external service signs with the raw bytes. Stripping only \r\n (the realistic editor/pipe artifacts) would preserve the intent without the surprise. The asymmetry (trailing stripped, leading kept) also isn't documented.

  3. os.read on a non-blocking fd raises BlockingIOError as soon as the pipe is momentarily empty, surfacing as "Could not read --secret-fd." A select-or-retry isn't needed, but a clause in the help text ("fd must be blocking") would preempt the confusing failure.

Strong security posture overall — bounded reads, 0600 storage, scrubbed output, and a thorough adversarial test suite.

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 platform/webhook Webhook / API server type/feature New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants