Skip to content

fix(paw-ingest): real webhook HMAC verification, fail-closed (ARN-168) - #451

Draft
rita-aga wants to merge 2 commits into
mainfrom
claude/arn-168-webhook-hmac
Draft

fix(paw-ingest): real webhook HMAC verification, fail-closed (ARN-168)#451
rita-aga wants to merge 2 commits into
mainfrom
claude/arn-168-webhook-hmac

Conversation

@rita-aga

@rita-aga rita-aga commented Jul 7, 2026

Copy link
Copy Markdown
Collaborator

Closes ARN-168 (CRITICAL) — TemperPaw Class B webhook authentication. Draft for review; do not merge.

Ownership

  • Original implementation: Claude Code (claude/arn-168-webhook-hmac)
  • Security audit, architectural repair, E2E proof, and current-main reconciliation: Codex
  • Result: shared Claude Code + Codex PR

Security model

The public webhook trigger is the single admission owner:

  1. Resolve exactly one active, governed route.
  2. Require explicit HMAC scheme, vault reference, signature/delivery headers, target capability, and budgets.
  3. Resolve the signing key through an injected tenant-scoped vault capability — never HTTP.
  4. Verify HMAC-SHA256 over the exact raw bytes using decoded hex plus Mac::verify_slice.
  5. Consume bounded rate/concurrency/body budgets and require a JSON-object body.
  6. Atomically get-or-create the deterministic event ID and validate the authoritative stored route/payload fingerprint before dispatch.
  7. Dispatch one immutable Received envelope; downstream WASM never re-reads mutable route state.

Changed content under a consumed delivery ID returns 409 without dispatch. Exact replay returns the original event as duplicate. A crash-left Created event retries only while its stored route snapshot still matches current governance.

Removed unsafe/duplicate paths

  • Deleted the downstream validate_webhook WASM verifier.
  • Removed empty/literal route secrets; all shipped routes use required vault references.
  • Removed broad Agent access to WebhookRoute/WebhookEvent; Admin and named transition modules own capabilities.
  • Removed raw-header persistence and mutable route re-read TOCTOU.
  • Reject malformed/scalar bodies, ambiguous duplicate security headers, invalid target/action/config grammar, and unbounded route-window growth.

Verification

  • cargo test -p paw-transport: 45 passed
  • strict clippy for paw-transport + temperpaw: passed
  • Cedar Admin/Agent/module-owner matrix: passed
  • route/process native + wasm32 release builds: passed
  • independent security/code review: passed, no remaining findings
  • full local HTTP → Temper entities → WASM → Patrol E2E: passed
    • forged: 401, zero persistence
    • signed malformed/scalar: 400, zero persistence
    • changed signed content with reused delivery ID: 409, no redispatch
    • exact replay: original ID, duplicate
    • four WebhookEvents Processed
    • WorkRequest + Datadog/GitHub/Discord Signals Linked
    • downstream FactoryCases/WorkCycles created

Proof report: .proofs/081-arn-168-authenticated-webhook-admission.md

Local proof bundle: /tmp/paw-patrol-webhook-smoke-proof-4531-56005

Residual

The per-route rate counter is process-local (expired entries are pruned and tracked routes are capped at 4,096). Replay integrity is durable in the shared entity store. A horizontally scaled trigger should move rate accounting into a shared Temper admission primitive.

No production deploy was performed because this PR must remain open/draft.

Greptile Summary

This PR replaces a placeholder webhook authentication path (empty secrets, WASM-side hex-string ct_eq) with real fail-closed HMAC-SHA256 admission at the raw HTTP boundary: Mac::verify_slice over decoded hex on the exact request bytes, secret resolved exclusively through an injected tenant-scoped vault closure that never touches HTTP, and an atomic get-or-create replay guard that binds delivery identity to payload and route-snapshot digests before any dispatch.

  • Admission security: rate budget is consumed only after successful HMAC verification; unsigned, forged, oversized, and malformed-but-signed requests are all rejected before entity creation is attempted; duplicate-header injection is rejected; route mutation after authentication cannot change the dispatched capability (route snapshot digest is verified pre-dispatch).
  • Cedar policy: broad unauthenticated principal access to WebhookRoute and WebhookEvent is replaced by module-scoped Agent grants; the deleted validate_webhook module is removed from all capability lists.
  • WASM dedup: the dedup_window_minutes route parameter is plumbed through the envelope but the check_dedup query in route_webhook does not apply a time-window filter, and the dedup key only matches alert_id/id fields — two functional gaps that are currently dormant because all seeded routes ship with dedup_enabled = "false".

Confidence Score: 5/5

Safe to merge — the core admission path is correctly hardened and the two findings are in the optional WASM dedup feature that ships disabled in all seeded routes.

The admission boundary is solid: HMAC verification uses decoded bytes with Mac::verify_slice, the secret resolver never crosses an HTTP boundary, replay is guarded atomically, and rate budget is consumed only after authentication. The only gaps are in the WASM-level content dedup (unbounded time window, limited key extraction) which is currently inert because every seeded route has dedup_enabled = false. No issues affect the authentication, authorization, or dispatch paths introduced by this PR.

os-apps/paw-ingest/wasm/route_webhook/src/lib.rs — the check_dedup function does not enforce dedup_window_minutes and only extracts alert_id/id as the dedup key; worth addressing before dedup is enabled on any route.

Important Files Changed

Filename Overview
crates/paw-transport/src/webhook/admission.rs New module encapsulating all webhook authentication and identity logic; HMAC-SHA256 uses Mac::verify_slice on decoded hex over raw bytes — correctly constant-time; route snapshot digest and event ID use length-prefixed SHA-256 preventing concatenation collisions; header ambiguity (duplicate headers) is rejected; input validation is strict and fail-closed
crates/paw-transport/src/webhook/trigger.rs Significant security hardening: HMAC verified against raw bytes before persistence; rate budget consumed only after auth; atomic get-or-create with stable identity fingerprint prevents TOCTOU; dispatch retry logic correctly re-reads entity status; body limit applied at Axum layer and per-route; secret resolver injected as in-process closure
crates/paw-transport/src/webhook/trigger_tests.rs Comprehensive integration test covering: unsigned/forged rejection before persistence, malformed-but-signed rejection before persistence, rate budget exhaustion, exact replay returning duplicate, changed-content conflict (409), dispatch failure and retry recovery, route mutation post-auth not changing dispatched capability, and secret-never-over-HTTP assertion
os-apps/paw-ingest/policies/webhook.cedar Cedar policy tightened significantly: removed broad unauthenticated principal access to WebhookRoute and WebhookEvent; module-scoped Agent permissions (route_webhook/process_webhook); removed validate_webhook from http_call allowlist; deleted Validated/ValidationFailed actions aligned with removal of validate_webhook WASM
os-apps/paw-ingest/wasm/route_webhook/src/lib.rs Rewritten to read from immutable admission envelope (never re-reads mutable WebhookRoute state); validates authentication_scheme == hmac-sha256; checks required fields fail-closed; but dedup_window_minutes is stored in the envelope yet not applied in check_dedup, which queries all historical entities without a time filter
crates/temperpaw/src/startup.rs Correctly threads the WebhookSecretResolver (tenant-scoped vault closure) into both the platform router's webhook handler and the standalone trigger; vault is cloned before auth setup so both auth and webhook admission share the same backing vault instance
crates/temperpaw/src/setup_api.rs Adds webhook secret readiness checks to /readyz and setup status; REQUIRED_WEBHOOK_SECRET_REFS covers all five seeded routes; missing secrets degrade /readyz to 503; test validates all five schema entries are marked required
os-apps/paw-ingest/wasm/validate_webhook/src/lib.rs Deleted — the HMAC verification logic (including the ct_eq hex-string comparison that prior reviews flagged) has been removed entirely; verification now happens at the admission boundary in Rust before any persistence
os-apps/paw-patrol/seed-data/webhook_routes.toml All five seeded routes updated from empty webhook_secret to explicit auth_scheme, secret_ref (vault reference), signature_header, delivery_id_header, and budget fields; no literal secrets remain in seed data

Sequence Diagram

%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
    participant Sender as Webhook Sender
    participant Trigger as HTTP Trigger(paw-transport)
    participant Vault as SecretResolver(in-process)
    participant Store as Temper Entity Store
    participant WASM as route_webhook WASM

    Sender->>Trigger: "POST /triggers/webhook/{route_key} + signature + delivery_id headers + body"
    Note over Trigger: Check in-flight semaphore (32)
    Trigger->>Store: "query_entities(WebhookRoutes, route_key eq ... and Status eq Active, top=2)"
    Store-->>Trigger: RouteSnapshot or empty
    Note over Trigger: Validate body size <= route budget. Extract signature + delivery_id headers (reject duplicates)
    Trigger->>Vault: resolve(secret_ref)
    Vault-->>Trigger: signing secret (in-process, never HTTP)
    Note over Trigger: HMAC-SHA256(secret, raw_bytes) Mac::verify_slice(provided_decoded_hex) 401 on mismatch
    Note over Trigger: Consume rate window (only after auth)
    Note over Trigger: Validate UTF-8 + JSON object
    Note over Trigger: Compute deterministic event_id = SHA256(v1 + tenant + route_id + delivery_id)
    Note over Trigger: Compute payload_digest + route_snapshot_digest
    Trigger->>Store: "create_entity(WebhookEvents, {Id: event_id, identity fields})"
    Store-->>Trigger: created or existing entity (atomic get-or-create)
    alt Stable identity mismatch (changed content, same delivery_id)
        Trigger-->>Sender: 409 CONFLICT
    else Entity not in Created state
        Trigger-->>Sender: 200 status duplicate
    else Route snapshot changed since delivery reservation
        Trigger-->>Sender: 409 CONFLICT
    else Fresh Created entity
        Trigger->>Store: dispatch_action(WebhookEvents, event_id, TemperPaw.Ingest.Received, immutable_envelope)
        Store-->>WASM: Received trigger
        Store-->>Trigger: OK
        Trigger-->>Sender: 200 status accepted
        WASM->>Store: create target entity + dispatch Routed
    end
Loading
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
sequenceDiagram
    participant Sender as Webhook Sender
    participant Trigger as HTTP Trigger(paw-transport)
    participant Vault as SecretResolver(in-process)
    participant Store as Temper Entity Store
    participant WASM as route_webhook WASM

    Sender->>Trigger: "POST /triggers/webhook/{route_key} + signature + delivery_id headers + body"
    Note over Trigger: Check in-flight semaphore (32)
    Trigger->>Store: "query_entities(WebhookRoutes, route_key eq ... and Status eq Active, top=2)"
    Store-->>Trigger: RouteSnapshot or empty
    Note over Trigger: Validate body size <= route budget. Extract signature + delivery_id headers (reject duplicates)
    Trigger->>Vault: resolve(secret_ref)
    Vault-->>Trigger: signing secret (in-process, never HTTP)
    Note over Trigger: HMAC-SHA256(secret, raw_bytes) Mac::verify_slice(provided_decoded_hex) 401 on mismatch
    Note over Trigger: Consume rate window (only after auth)
    Note over Trigger: Validate UTF-8 + JSON object
    Note over Trigger: Compute deterministic event_id = SHA256(v1 + tenant + route_id + delivery_id)
    Note over Trigger: Compute payload_digest + route_snapshot_digest
    Trigger->>Store: "create_entity(WebhookEvents, {Id: event_id, identity fields})"
    Store-->>Trigger: created or existing entity (atomic get-or-create)
    alt Stable identity mismatch (changed content, same delivery_id)
        Trigger-->>Sender: 409 CONFLICT
    else Entity not in Created state
        Trigger-->>Sender: 200 status duplicate
    else Route snapshot changed since delivery reservation
        Trigger-->>Sender: 409 CONFLICT
    else Fresh Created entity
        Trigger->>Store: dispatch_action(WebhookEvents, event_id, TemperPaw.Ingest.Received, immutable_envelope)
        Store-->>WASM: Received trigger
        Store-->>Trigger: OK
        Trigger-->>Sender: 200 status accepted
        WASM->>Store: create target entity + dispatch Routed
    end
Loading

Reviews (3): Last reviewed commit: "fix(paw-ingest): enforce authenticated w..." | Re-trigger Greptile

The `/triggers/webhook/{route_key}` endpoint is public, so the request
signature is the only thing authenticating an inbound webhook. The
validate_webhook WASM integration only checked that the signature *header
was present* — it never computed or compared HMAC-SHA256(secret, body) —
and returned hmac_verified:"true" whenever the header existed, dispatching
the payload regardless. Any body with a bogus X-Hub-Signature-256 was
accepted and routed into the scout/SRE/heal/patrol agents (Class B).

Fix (mirrors the kernel counterpart for ARN-171):
- Compute HMAC-SHA256(secret, raw_payload) and constant-time compare it
  (subtle::ConstantTimeEq, not ==) against the signature header.
- Accept an optional `sha256=` prefix, case-insensitively; header lookup
  is case-insensitive.
- Resolve the signing secret from the route: literal value or {secret:KEY}
  template resolved from the host secret store.
- Fail closed: a route that declares a secret but has a missing,
  unresolvable, or mismatched signature transitions the WebhookEvent to
  ValidationFailed and is never routed/processed. Routes with no secret
  keep the existing "skipped" carve-out.

Verification logic is factored into pure, host-free helpers
(classify_secret, extract_header, verify_signature, signature_matches)
covered by a red→green unit suite: a forged signature (previously accepted
as "true") and a tampered body / wrong secret / missing header are now
rejected, a correctly signed payload is accepted. Adds hmac, sha2, hex,
subtle (pure-Rust, compile for wasm32-unknown-unknown). See
os-apps/paw-ingest/adrs/001-webhook-hmac-verification.md.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@rita-aga
rita-aga marked this pull request as ready for review July 7, 2026 09:05
@rita-aga

rita-aga commented Jul 7, 2026

Copy link
Copy Markdown
Collaborator Author

@greptile review

Comment thread os-apps/paw-ingest/wasm/validate_webhook/src/lib.rs Outdated
Comment thread os-apps/paw-ingest/wasm/validate_webhook/src/lib.rs Outdated
@rita-aga

Copy link
Copy Markdown
Collaborator Author

ARN-165 / ARN-168 principle audit — request changes

The raw-byte HMAC primitive is improved, but this PR has no secure, usable production configuration and does not close ARN-168.

Blocking findings

  1. Shipped routes bypass verification. validate_webhook/src/lib.rs:107-110 treats an empty secret as skipped and then transitions to Validated. Every paw-patrol seed route uses webhook_secret = "" (lines 15/30/45/60/75), as does the deep-sci-fi route. Unsigned requests still reach WorkRequest.Submit, Signal.Ingest, or AlertCycle.Open; the checked-in smoke script intentionally posts unsigned bodies.
  2. The intended secret-reference path is denied by Cedar. {secret:KEY} calls ctx.get_secret at lib.rs:243-255, but the webhook policy permits http_call, not access_secret, for this module. A legitimately signed request using the secure-looking configuration fails.
  3. Literal secrets and route governance are insecure. webhook.cedar:57-68 permits any principal to read/create/register/update/disable/enable WebhookRoute. A low-privilege bearer can read a literal secret, set it empty, or change the target action. Raw payload/headers are also broadly readable.
  4. Replay is not prevented. Valid signed payload/header pairs are persisted and readable; verification checks authenticity only. Seed routes disable deduplication, so captured deliveries can repeat downstream effects.
  5. Validation happens after durable entity creation. Invalid unauthenticated traffic can still flood storage.
  6. Coverage tests only the pure helper. The 10 native tests pass, but there is no authorized-host/Cedar E2E, route migration proof, state-machine proof, Genesis install proof, or deployed forged-vs-signed test.

Required direction

Require an explicit provider auth scheme and nonempty governed secret reference for every public route; migrate all existing routes and fail readiness if missing. Authenticate/admit before creating WebhookEvent; restrict route governance; store only redacted references; consume provider delivery IDs uniquely before effects; apply byte/rate budgets.

The two unresolved ct_eq threads are duplicates. Use hex decoding + Mac::verify_slice, remove the extra subtle dependency, and resolve/close the duplicate after the code changes.

The PR is currently conflicting and its body says “do not merge” while GitHub marks it ready. Please return it to draft.

@nerdsane
nerdsane marked this pull request as draft July 12, 2026 03:19
@nerdsane

Copy link
Copy Markdown
Owner

@greptile review

Fresh review requested for head 37c0f98. The prior validator and ct_eq implementation were removed. Current admission verifies decoded HMAC bytes with Mac::verify_slice at the raw HTTP boundary, uses an injected tenant-vault resolver, binds replay identity to authoritative stored payload/route fingerprints, rejects malformed and ambiguous requests before persistence, and has a passing live E2E proof. Please review the entire current diff against main.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants