Repository navigation
feat(webhooks): add timestamped HMAC verification #1660
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Aaronontheweb
merged 11 commits into
netclaw-dev:dev
from
Aaronontheweb:investigate/issue-1658
Jul 16, 2026
Merged
Changes from 10 commits
Commits
Show all changes
11 commits
Select commit
Hold shift + click to select a range
629dcec
feat(webhooks): add timestamped HMAC verification
Aaronontheweb 606d233
fix(webhooks): preserve verified route configuration
Aaronontheweb 0a2a46b
fix(webhooks): harden concurrent route updates
Aaronontheweb f90af77
fix(daemon): make restart drain tests deterministic
Aaronontheweb ea2fab2
fix: address webhook and restart review findings
Aaronontheweb 5289a44
fix: close final concurrency review gaps
Aaronontheweb bf65389
fix(webhooks): canonicalize nested route path aliases
Aaronontheweb 7a6d235
test(webhooks): accept platform-specific write errors
Aaronontheweb 984b649
docs(openspec): complete webhook verification checklist
Aaronontheweb cad8e37
fix(webhooks): honor configured HMAC algorithm
Aaronontheweb 0c50b99
Merge branch 'dev' into investigate/issue-1658
Aaronontheweb File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,5 +1,8 @@ | ||
| { | ||
| "configVersion": 1, | ||
| "Webhooks": { | ||
| "Enabled": true | ||
| }, | ||
| "Tools": { | ||
| "AudienceProfiles": { | ||
| "Personal": { | ||
|
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,2 @@ | ||
| schema: spec-driven | ||
| created: 2026-07-15 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,58 @@ | ||
| ## Context | ||
|
|
||
| Webhook routes are secret-bearing JSON files loaded independently at request time. `WebhookRequestVerifier` currently verifies either HMAC-SHA256 over the raw body or a static secret header. The endpoint already retains the raw bytes, and the daemon already registers `TimeProvider`, so timestamped verification can reuse both seams without changing actor messages, persistence, dispatch, filtering, deduplication, or rate limiting. | ||
|
|
||
| Compatibility includes existing files, CLI/tool callers, and downgrade behavior. A new daemon must not reinterpret an old route; an old daemon encountering a route that explicitly selects the new kind must fail that route closed without affecting other routes. | ||
|
|
||
| ## Goals / Non-Goals | ||
|
|
||
| **Goals:** | ||
|
|
||
| - Verify Stripe-style `t=...,v1=...` signatures over the exact `timestamp.separator.rawBody` bytes. | ||
| - Enforce a bounded replay window using injectable time. | ||
| - Support multiple signature values for sender-side secret rotation. | ||
| - Keep existing verification behavior, defaults, and route files unchanged. | ||
| - Give operators generic CLI/tool controls plus copyable Stripe and TextForge configurations. | ||
|
|
||
| **Non-Goals:** | ||
|
|
||
| - Provider-specific verifier types, key derivation, or automatic provider detection. | ||
| - Deprecating body-only HMAC or static header secrets. | ||
| - Persisting replay state beyond the existing delivery-ID deduplication behavior. | ||
| - Changing webhook sessions, endpoint responses, or ingress ordering. | ||
|
|
||
| ## Decisions | ||
|
|
||
| 1. **Add an explicit discriminator rather than auto-detection.** `HmacTimestamped` is appended to the enum. `Hmac` remains the default because senders use incompatible protocols. Auto-detection or fallback could accept a request under a weaker mode after the intended mode failed. | ||
|
|
||
| 2. **Keep raw configuration optional and resolve effective defaults at runtime.** Nullable `ToleranceSeconds`, `TimestampField`, `SignatureField`, and `SignedPayloadSeparator` fields preserve legacy JSON. Their effective values for the new kind are `300`, `t`, `v1`, and `.`. Null fields are omitted when writing routes. The v1 route schema gains only optional properties and the additive enum value. | ||
|
|
||
| 3. **Parse strictly and sign exact received bytes.** The parser accepts comma-separated `key=value` components, requires exactly one timestamp and at least one signature, and rejects malformed or ambiguous input. The numeric timestamp is used for tolerance checks, while its original text is used in the signed payload. Signature comparison decodes 32-byte SHA-256 hex values and uses fixed-time equality. | ||
|
|
||
| 4. **Use the existing `TimeProvider` registration.** `WebhookRequestVerifier` requires `TimeProvider` through DI. Tests use `FakeTimeProvider`; production uses `TimeProvider.System` already registered by the daemon. | ||
|
|
||
| 5. **Keep configuration generic at the user surface.** The CLI adds `hmac-timestamped` plus advanced optional flags. Providers still specify `SignatureHeaderName` because Stripe and TextForge use different names. Provider presets are deferred until repeated configuration demonstrates a need. | ||
|
|
||
| 6. **Preserve inactive and omitted fields.** Validation applies timestamp constraints only when `HmacTimestamped` is selected. Switching kinds does not erase dormant settings, and old kinds do not acquire new behavior. CLI and `set_webhook` updates retain optional route and verification values that the caller omits; `set_webhook` performs its read, audience authorization, patch, validation, and write under one store lock. | ||
|
|
||
| 7. **Reject unrepresentable structured-header field names before persistence.** Effective timestamp and signature field names must be distinct HTTP tokens. This excludes whitespace, delimiters, non-ASCII characters, and controls that cannot form a valid structured-header key. | ||
|
|
||
| 8. **Reject undefined numeric enum values during shared validation.** Route deserialization retains its prior ability to read numeric enum values for compatibility, but values outside the defined verifier-kind and HMAC-algorithm sets fail route validation before request handling. | ||
|
|
||
| No actor or persistence boundary changes. Verification still returns the existing in-memory result consumed by the endpoint before any session actor is created. | ||
|
|
||
| ## Risks / Trade-offs | ||
|
|
||
| - **Clock skew rejects legitimate events** → use a documented 300-second default, configurable from 1 through 3600 seconds, and expose a distinct internal rejection reason. | ||
| - **Structured header ambiguity** → reject duplicate timestamps, missing values, malformed pairs, invalid Unix timestamps, and invalid signature hex. | ||
| - **CLI output consumers break on additive fields** → emit timestamp-specific fields only for the new kind and preserve old-kind output shape. | ||
| - **Downgrade encounters the new enum** → older daemons fail that route during parsing; the route catalog removes it and emits its existing invalid-route alert. | ||
| - **Configuration files accumulate irrelevant fields** → omit nullable timestamp fields from serialization and ignore dormant values for other verifier kinds. | ||
|
|
||
| ## Migration Plan | ||
|
|
||
| No migration runs. Existing routes continue to deserialize with null timestamp settings and retain their existing discriminator. Operators opt in by changing or creating a route with `HmacTimestamped`. Rollback requires changing such routes back to a verifier supported by the older daemon before downgrading; otherwise only those routes remain unavailable. | ||
|
|
||
| ## Open Questions | ||
|
|
||
| None. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,31 @@ | ||
| ## Why | ||
|
|
||
| PRD-009 permits external services to launch webhook sessions, but the current verifier only accepts body-only HMAC signatures or static secret headers. Providers such as Stripe and TextForge sign `timestamp.rawBody` and require a timestamp tolerance, so Netclaw cannot receive their events without weakening verification outside the daemon. | ||
|
|
||
| ## What Changes | ||
|
|
||
| - Add an opt-in, generic timestamped-HMAC verification kind for structured `t=...,v1=...` signature headers. | ||
| - Verify the exact timestamp text and raw request bytes with HMAC-SHA256, accept multiple signatures for secret rotation, and reject deliveries outside a configurable replay window. | ||
| - Expose the new mode through route JSON, `netclaw webhooks`, and `set_webhook`, with Stripe-style defaults for field names, separator, and tolerance. | ||
| - Preserve body-only `Hmac` as the default and retain `HeaderSecret`; existing route files and callers require no migration. | ||
| - Update route schema, operator documentation, runtime skill guidance, and behavioral evals. | ||
|
|
||
| In scope for PRD-009 Phase 2 is generic timestamped verification and its configuration surfaces. Provider presets, provider-specific key derivation, a route-authoring TUI, and changes to webhook dispatch, filtering, deduplication, or rate limiting are out of scope. | ||
|
|
||
| ## Capabilities | ||
|
|
||
| ### New Capabilities | ||
|
|
||
| None. | ||
|
|
||
| ### Modified Capabilities | ||
|
|
||
| - `inbound-webhooks`: Add explicit timestamped-HMAC verification while preserving existing verification modes and route compatibility. | ||
|
|
||
| ## Impact | ||
|
|
||
| - Configuration: additive enum value and optional fields in the v1 webhook-route schema; no route migration. | ||
| - Runtime: one new fail-closed verifier branch using the existing raw request body and injected `TimeProvider`. | ||
| - Interfaces: additive CLI flags and optional `set_webhook` arguments; existing invocations remain valid. | ||
| - Security: timestamped routes require a valid signature and bounded timestamp; no verifier auto-detection or fallback is introduced. | ||
| - Operations: malformed or stale timestamped signatures remain ordinary `401` verification failures and use existing structured logs and counters. |
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.