feat(webhook): embedded receiver for CiviCRM-triggered reconciles#36
feat(webhook): embedded receiver for CiviCRM-triggered reconciles#36RyanMorash wants to merge 1 commit into
Conversation
Phase 1 of the webhook feature (door-sync side). Adds an optional Flask+waitress HTTP server that runs in a second daemon thread, verifies an HMAC-signed request (timestamp-bound for replay protection), and enqueues a ReconcileRequest onto a shared work queue. The scheduler thread drains the queue between cadence ticks, coalescing bursts into a single full reconcile — it remains the sole writer to UniFi/state/audit, so no locks are needed. orchestrator.reconcile is unchanged. - models: ReconcileRequest - config: WebhookConfig (disabled by default; secret WEBHOOK_HMAC_SECRET) - webhook.py: create_app, HMAC verify, POST /civicrm/membership-changed, /healthz - scheduler: queue-driven wake + shutdown sentinel + debounce/coalesce - __main__: start/stop the receiver in daemon mode when enabled - deploy: cloudflared.service + cloudflared-config.yml (tunnel; loopback bind) - docs + example config/env; pre-commit deps for flask/waitress The receiver binds loopback only; a Cloudflare Tunnel connects locally so the Pi exposes no public port. Verified end-to-end against the real waitress server. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
📝 WalkthroughWalkthroughAdds an optional HMAC-authenticated webhook receiver backed by Flask and waitress. Valid requests enqueue reconcile triggers, the scheduler coalesces queued events, daemon mode manages the shared queue and server lifecycle, and deployment documentation and tunnel service files are added. ChangesWebhook receiver and reconciliation flow
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant CloudflareTunnel
participant WebhookServer
participant WorkQueue
participant Scheduler
CloudflareTunnel->>WebhookServer: Forward signed webhook
WebhookServer->>WebhookServer: Verify HMAC signature and timestamp
WebhookServer->>WorkQueue: Enqueue ReconcileRequest
WebhookServer-->>CloudflareTunnel: Return 202 Accepted
Scheduler->>WorkQueue: Consume and coalesce triggers
Scheduler->>Scheduler: Run reconciliation
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Code Coverage OverviewLanguages: Python Python / code-coverage/pytestThe overall coverage in the branch is 92%. The coverage in the branch is 93%. Show a code coverage summary of the most impacted files.
Code Coverage is in Public Preview. Learn more and provide us with your feedback. |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| return Response(status=401) | ||
| contact_id = _extract_contact_id(raw) # logging only | ||
| work_queue.put(ReconcileRequest(reason="membership-changed", contact_id=contact_id)) | ||
| _logger.info("membership webhook accepted; contact_id=%s; reconcile queued", contact_id) |
There was a problem hiding this comment.
Pull request overview
Adds an optional embedded HTTP webhook receiver to trigger faster reconciles from CiviCRM events while keeping the scheduler as the sole writer to UniFi/state/audit (queue-driven wake + debounce/coalesce between cadence ticks).
Changes:
- Introduces
door_sync.webhook(Flask + waitress) with HMAC+timestamp verification and/healthz+/civicrm/membership-changedendpoints. - Extends the scheduler loop to wake early from a shared work queue, coalesce bursts, and support a shutdown sentinel.
- Adds
WebhookConfig, daemon wiring, deploy artifacts (cloudflared tunnel), documentation, and tests.
Reviewed changes
Copilot reviewed 17 out of 18 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| uv.lock | Locks new runtime dependencies (Flask/waitress and transitive deps). |
| pyproject.toml | Adds Flask and waitress as project dependencies. |
| .pre-commit-config.yaml | Ensures type-check hook has Flask/waitress available. |
| src/door_sync/webhook.py | Implements embedded receiver, HMAC verification, and server thread lifecycle. |
| src/door_sync/scheduler.py | Adds queue-driven wake/coalescing and shutdown sentinel integration. |
| src/door_sync/config.py | Adds WebhookConfig, defaults, and [webhook] validation (env secret requirement). |
| src/door_sync/models.py | Adds ReconcileRequest model for queued triggers. |
| src/door_sync/main.py | Wires webhook server start/stop and shared queue in daemon mode. |
| tests/test_webhook.py | Unit + endpoint tests for signature checking, enqueueing, size limits, and PII-safe logging. |
| tests/test_scheduler.py | Tests for signal handler sentinel, wake/coalesce behavior, and run loop integration. |
| tests/test_models.py | Tests for ReconcileRequest immutability/defaults. |
| tests/test_main.py | Tests daemon wiring to start/stop webhook and share the queue. |
| tests/test_config.py | Tests webhook config defaults/validation and env secret requirements. |
| docs/configuration.rst | Documents [webhook] configuration and required env var. |
| config.example.toml | Adds commented webhook config example. |
| .env.example | Documents WEBHOOK_HMAC_SECRET. |
| deploy/cloudflared.service | Adds systemd unit for a Cloudflare Tunnel to loopback webhook receiver. |
| deploy/cloudflared-config.yml | Adds example cloudflared tunnel config routing hostname to loopback receiver. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if shutdown_event.is_set(): | ||
| return True | ||
| try: | ||
| item = work_queue.get(timeout=cadence_seconds) | ||
| except queue.Empty: | ||
| return shutdown_event.is_set() # periodic cadence tick |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
deploy/cloudflared.service (1)
1-24: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winConsider adding restart rate limiting.
Restart=on-failurewithRestartSec=5sbut noStartLimitIntervalSec/StartLimitBurstallows systemd to restart indefinitely if cloudflared keeps failing, which can mask underlying issues and waste resources.⚙️ Suggested addition
[Service] Type=simple User=cloudflared Group=cloudflared ExecStart=/usr/local/bin/cloudflared tunnel --config /etc/cloudflared/config.yml run Restart=on-failure RestartSec=5s +StartLimitIntervalSec=60 +StartLimitBurst=5 StandardOutput=journal StandardError=journal🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@deploy/cloudflared.service` around lines 1 - 24, Add systemd restart rate limiting to the cloudflared service unit: define StartLimitIntervalSec and StartLimitBurst in the [Unit] section alongside the existing restart settings, using values that cap repeated failures while preserving normal recovery behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/door_sync/webhook.py`:
- Around line 62-65: Update _verify_signature to fail closed for malformed,
non-ASCII signature headers: compare the expected and provided signatures as
encoded bytes, or catch TypeError around hmac.compare_digest and return False.
Ensure invalid input cannot propagate an exception or produce a 500 response.
---
Nitpick comments:
In `@deploy/cloudflared.service`:
- Around line 1-24: Add systemd restart rate limiting to the cloudflared service
unit: define StartLimitIntervalSec and StartLimitBurst in the [Unit] section
alongside the existing restart settings, using values that cap repeated failures
while preserving normal recovery behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 523415b3-c5a9-4943-bc96-bffb2b3be379
⛔ Files ignored due to path filters (1)
uv.lockis excluded by!**/*.lock
📒 Files selected for processing (17)
.env.example.pre-commit-config.yamlconfig.example.tomldeploy/cloudflared-config.ymldeploy/cloudflared.servicedocs/configuration.rstpyproject.tomlsrc/door_sync/__main__.pysrc/door_sync/config.pysrc/door_sync/models.pysrc/door_sync/scheduler.pysrc/door_sync/webhook.pytests/test_config.pytests/test_main.pytests/test_models.pytests/test_scheduler.pytests/test_webhook.py
| provided = signature[len(_SIG_PREFIX) :] if signature.startswith(_SIG_PREFIX) else signature | ||
| signed = f"{ts}.".encode("ascii") + raw_body | ||
| expected = hmac.new(secret.encode("utf-8"), signed, hashlib.sha256).hexdigest() | ||
| return hmac.compare_digest(expected, provided) |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🌐 Web query:
python hmac.compare_digest TypeError non-ascii string arguments
💡 Result:
The hmac.compare_digest function in Python is designed to perform a constant-time comparison of two values to prevent timing attacks, which is essential for cryptographic verification [1][2]. If you receive a TypeError when using this function, it is typically because the arguments provided violate the strict requirements for input types [1][2]. Requirements for hmac.compare_digest: 1. Both arguments must be of the same type: either both must be strings (str) or both must be bytes-like objects [1][2]. 2. If you pass string (str) arguments, they must be ASCII-only [1][2]. This is because the function is primarily intended to handle digests like those produced by hexdigest, which are ASCII hexadecimal strings [3][2]. If you encounter an error when passing strings, it is almost certainly because at least one of the strings contains non-ASCII characters [4]. To resolve this, you should ensure that both values being compared are byte strings (bytes) [3]. You can convert a string to bytes using the.encode method (typically with 'utf-8' or 'ascii' encoding, depending on your data) before passing them to hmac.compare_digest [3]. Example of correct usage: import hmac # Ensure both inputs are bytes digest1 = b'...' digest2 = b'...' # Perform the secure comparison is_match = hmac.compare_digest(digest1, digest2)
Citations:
- 1: https://docs.python.org/3.10/library/hmac.html
- 2: https://github.com/python/cpython/blob/3.11/Doc/library/hmac.rst
- 3: https://bugs.python.org/issue21306
- 4: TypeError: comparing strings with non-ASCII characters is not supported pyauth/pyotp#22
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== file outline ==\n'
ast-grep outline src/door_sync/webhook.py --view expanded || true
printf '\n== relevant lines ==\n'
sed -n '1,180p' src/door_sync/webhook.py | cat -n
printf '\n== search for signature parsing and error handling ==\n'
rg -n "_verify_signature|compare_digest|X-Door-Sync-Signature|signature" src/door_sync/webhook.py src/door_sync -nRepository: TechValleyCenterOfGravity/door-sync
Length of output: 10584
🏁 Script executed:
python3 - <<'PY'
import hmac
cases = [
("ascii", "abc", "abc"),
("non-ascii left", "é", "e"),
("non-ascii right", "e", "é"),
("bytes", b"abc", b"abc"),
("mixed", b"abc", "abc"),
]
for name, a, b in cases:
try:
print(name, "=>", hmac.compare_digest(a, b))
except Exception as e:
print(name, "=>", type(e).__name__, e)
PYRepository: TechValleyCenterOfGravity/door-sync
Length of output: 442
Malformed signature header can raise TypeError and return 500.
In _verify_signature, hmac.compare_digest(expected, provided) compares two str values. A non-ASCII X-Door-Sync-Signature header makes compare_digest raise TypeError, so a malformed request can surface as a 500 instead of a 401. Compare bytes, or catch TypeError and fail closed.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/door_sync/webhook.py` around lines 62 - 65, Update _verify_signature to
fail closed for malformed, non-ASCII signature headers: compare the expected and
provided signatures as encoded bytes, or catch TypeError around
hmac.compare_digest and return False. Ensure invalid input cannot propagate an
exception or produce a 500 response.
Phase 1 of the CiviCRM→UniFi webhook feature (door-sync side).
Adds an optional Flask+waitress HTTP receiver in a second daemon thread. It verifies an HMAC-signed request (timestamp-bound for replay protection) and enqueues a
ReconcileRequestonto a shared work queue; the scheduler thread drains it between cadence ticks, coalescing bursts into a single full reconcile. The scheduler stays the sole writer to UniFi/state/audit, so no locks are needed andorchestrator.reconcileis unchanged.webhook.py:create_app, HMAC verify,POST /civicrm/membership-changed,/healthzscheduler: queue-driven wake + shutdown sentinel + debounce/coalesceconfig:WebhookConfig(disabled by default; secretWEBHOOK_HMAC_SECRET)__main__: start/stop the receiver in daemon mode when enabledcloudflared.service+cloudflared-config.yml(tunnel; loopback bind)config.toml/.envLoopback-only bind; a Cloudflare Tunnel connects locally so the Pi exposes no public port. 390 tests pass, pyrefly + ruff clean; verified end-to-end against the real waitress server (202 / 401 / replay / healthz / clean shutdown).
Part of a 3-repo change:
door-webhook(edge Worker) anddoor-civirules(CiviRules producer).🤖 Generated with Claude Code
Summary by CodeRabbit