feat(cue): add generic webhook trigger for external events - #1303
Open
pedramamini wants to merge 2 commits into
Open
feat(cue): add generic webhook trigger for external events#1303pedramamini wants to merge 2 commits into
pedramamini wants to merge 2 commits into
Conversation
Adds a `webhook.received` Cue event type so any external service that can
send an HTTP POST - GitHub, GitLab, Slack, CI systems, ad-hoc scripts - can
drive a Cue pipeline. Deliberately generic: the payload lands in the event
and the existing filter engine decides which deliveries matter, rather than
hardcoding per-vendor handling.
Listener (src/main/cue/cue-webhook-server.ts):
- One process-wide HTTP listener shared by every webhook subscription across
every agent, refcounted so the port only exists when a webhook pipeline is
loaded.
- Binds 127.0.0.1:17997 by default (MAESTRO_CUE_WEBHOOK_PORT /
MAESTRO_CUE_WEBHOOK_HOST override). Public delivery is expected to go
through a tunnel or reverse proxy rather than a public bind.
- Every subscription must carry a secret; config validation fails loudly
without one, since an unauthenticated path is a remote trigger for an
agent running with the user's credentials.
- Two auth modes: presented secret (X-Maestro-Cue-Secret or Authorization:
Bearer) or HMAC-SHA256 over the raw body via a configured signature header,
accepting the sha256=<hex> form GitHub and GitLab send. Both compare
timing-safe.
- Auth headers are stripped before the payload is built, so secrets can't
reach a prompt, the activity log, or the Cue DB. POST only, 1 MB body cap.
Wiring: trigger source, config normalizer + validator, template context
enricher, and five {{CUE_WEBHOOK_*}} template variables. Pipeline editor gets
the trigger in the drawer, a config panel (path / secret env var / signature
header) that writes secret_env rather than literals, and lossless YAML
round-tripping for hand-written configs.
Docs: new event section in docs/maestro-cue-events.md, an in-app pattern, and
a CueHelpModal entry.
closes #858
📝 WalkthroughWalkthroughAdds the ChangesWebhook Cue trigger
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant ExternalService
participant CueWebhookServer
participant CueWebhookTriggerSource
participant CuePipeline
ExternalService->>CueWebhookServer: POST authenticated webhook payload
CueWebhookServer->>CueWebhookTriggerSource: Route and deliver sanitized payload
CueWebhookTriggerSource->>CuePipeline: Apply filter and emit webhook.received
Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 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 |
Greptile SummaryAdds authenticated generic webhook triggers to Cue.
Confidence Score: 3/5The PR should not merge until webhook editor round-tripping and exact-byte HMAC verification are fixed. Distinct webhook configurations can be silently consolidated when a pipeline is opened and saved, and valid signed binary deliveries are rejected because authentication hashes decoded text rather than the transmitted bytes. Files Needing Attention: src/renderer/components/CuePipelineEditor/utils/yamlToPipeline.ts, src/main/cue/cue-webhook-server.ts Important Files Changed
Sequence DiagramsequenceDiagram
participant Sender
participant Listener as Cue Webhook Listener
participant Trigger as Webhook Trigger Source
participant Filter as Cue Filter
participant Engine as Cue Engine
participant Agent
Sender->>Listener: POST /cue/path
Listener->>Listener: Enforce size and authenticate
Listener->>Trigger: Sanitized delivery
Trigger->>Filter: webhook.received event
alt Filter matches and Cue enabled
Filter->>Engine: Emit event
Engine->>Agent: Render and execute prompt
else Filter rejects
Filter-->>Trigger: Drop event
end
Listener-->>Sender: 202 accepted
|
![greptile-apps[bot]](https://avatars.github.com/in/867647?s=60&v=4){kind=small}
| chunks.push(chunk); | ||
| }); | ||
| req.on('end', () => { | ||
| if (!aborted) resolve(Buffer.concat(chunks).toString('utf8')); |
There was a problem hiding this comment.
When a signed webhook body contains bytes that are not valid UTF-8, this conversion replaces those bytes before authentication. Maestro therefore hashes different data than the sender signed and rejects the valid delivery with HTTP 401; retain the original Buffer for HMAC verification and decode a separate copy for payload handling.
Knowledge Base Used: Cue automation engine
| respond(res, 404, { error: 'Unknown webhook path' }); | ||
| return; | ||
| } | ||
| const path = normalizeWebhookPath(decodeURIComponent(match[1])); |
There was a problem hiding this comment.
A request path such as /cue/% or /cue/%GG makes decodeURIComponent throw. The outer handler converts this client-controlled routing error into an HTTP 500 and reports it as an application exception instead of returning a client error, adding avoidable error-tracker noise.
![coderabbitai[bot]](https://avatars.github.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
src/main/cue/cue-webhook-server.ts (1)
111-122: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winConsider a startup warning when binding beyond loopback.
getCueWebhookHost()letsMAESTRO_CUE_WEBHOOK_HOSTwiden the bind address, but nothing logs when it does - the risk is only documented in a comment. A stray0.0.0.0(or LAN IP) silently exposes an agent-triggering HTTP endpoint beyond the machine. A one-lineonLog('warn', ...)inensureServerStartedwhen the resolved host isn't loopback would give operators visibility into this footgun at effectively no cost.Also applies to: 322-355
🤖 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/main/cue/cue-webhook-server.ts` around lines 111 - 122, Update ensureServerStarted to emit a single onLog('warn', ...) startup warning when the resolved host from getCueWebhookHost is not loopback, including that the webhook endpoint is exposed beyond the local machine. Keep loopback bindings silent and preserve the existing server startup 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/main/cue/cue-webhook-server.ts`:
- Around line 214-319: The readBody function currently collapses payload-size
overruns and stream errors into the same null result, causing
handleCueWebhookRequest to return 413 for connection failures. Change readBody
to return a result that distinguishes the too-large case from stream errors,
then update handleCueWebhookRequest to send 413 only for size-limit violations
and handle generic read failures with an appropriate non-413 response.
In `@src/renderer/components/CuePipelineEditor/utils/pipelineGraph.ts`:
- Around line 63-64: Update the webhook.received branch in the pipeline graph
summary to pass config.webhook_path through the existing normalizeWebhookPath
fallback, matching TriggerConfig behavior while preserving /cue/ for an unset
path.
In `@src/renderer/components/CuePipelineEditor/utils/pipelineToYaml.ts`:
- Around line 195-214: Treat whitespace-only webhook_secret_env values as unset
in the webhook.received serialization branch. Update the condition selecting
webhook.secret_env so it requires non-whitespace content, allowing a valid
webhook_secret to be emitted through the fallback path; preserve the existing
precedence for meaningful environment values.
---
Nitpick comments:
In `@src/main/cue/cue-webhook-server.ts`:
- Around line 111-122: Update ensureServerStarted to emit a single onLog('warn',
...) startup warning when the resolved host from getCueWebhookHost is not
loopback, including that the webhook endpoint is exposed beyond the local
machine. Keep loopback bindings silent and preserve the existing server startup
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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: c17cc779-06f3-4d17-bf98-965aa0250543
📒 Files selected for processing (27)
docs/maestro-cue-events.mdsrc/__tests__/main/cue/cue-webhook-server.test.tssrc/__tests__/main/cue/cue-yaml-loader.test.tssrc/__tests__/main/cue/triggers/cue-webhook-trigger-source.test.tssrc/__tests__/renderer/components/CuePipelineEditor/drawers/TriggerDrawer.test.tsxsrc/__tests__/renderer/components/CuePipelineEditor/utils/pipelineValidation.test.tssrc/__tests__/renderer/hooks/cue/usePipelineState.test.tssrc/main/cue/config/cue-config-normalizer.tssrc/main/cue/config/cue-config-validator.tssrc/main/cue/cue-template-context-builder.tssrc/main/cue/cue-webhook-server.tssrc/main/cue/stats/cue-stats-query.tssrc/main/cue/triggers/cue-trigger-source-registry.tssrc/main/cue/triggers/cue-webhook-trigger-source.tssrc/renderer/components/CueHelpModal.tsxsrc/renderer/components/CuePipelineEditor/cueEventConstants.tssrc/renderer/components/CuePipelineEditor/drawers/TriggerDrawer.tsxsrc/renderer/components/CuePipelineEditor/panels/triggers/TriggerConfig.tsxsrc/renderer/components/CuePipelineEditor/utils/pipelineGraph.tssrc/renderer/components/CuePipelineEditor/utils/pipelineToYaml.tssrc/renderer/components/CuePipelineEditor/utils/pipelineValidation.tssrc/renderer/components/CuePipelineEditor/utils/yamlToPipeline.tssrc/renderer/constants/cuePatterns.tssrc/shared/cue-pipeline-types.tssrc/shared/cue/contracts.tssrc/shared/cue/cue-summary.tssrc/shared/templateVariables.ts
Comment on lines
+195
to
+214
| case 'webhook.received': { | ||
| // `secret` and `secret_env` are mutually exclusive in the schema, so | ||
| // emit the literal only when the trigger has no env var - that's the | ||
| // hand-written-YAML case we're preserving rather than encouraging. | ||
| const webhook: CueSubscription['webhook'] = {}; | ||
| if (triggerData.config.webhook_path) webhook.path = triggerData.config.webhook_path; | ||
| if (triggerData.config.webhook_secret_env) { | ||
| webhook.secret_env = triggerData.config.webhook_secret_env; | ||
| } else if (triggerData.config.webhook_secret) { | ||
| webhook.secret = triggerData.config.webhook_secret; | ||
| } | ||
| if (triggerData.config.webhook_signature_header) { | ||
| webhook.signature_header = triggerData.config.webhook_signature_header; | ||
| } | ||
| sub.webhook = webhook; | ||
| if (triggerData.config.filter) { | ||
| sub.filter = triggerData.config.filter; | ||
| } | ||
| break; | ||
| } |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Treat whitespace-only secret_env values as unset.
If webhook_secret_env is whitespace and webhook_secret is valid, validation permits the literal secret, but this branch serializes the whitespace env value and drops the literal. The saved YAML then fails webhook authentication validation.
Proposed fix
- if (triggerData.config.webhook_secret_env) {
- webhook.secret_env = triggerData.config.webhook_secret_env;
+ const secretEnv = triggerData.config.webhook_secret_env?.trim();
+ if (secretEnv) {
+ webhook.secret_env = secretEnv;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| case 'webhook.received': { | |
| // `secret` and `secret_env` are mutually exclusive in the schema, so | |
| // emit the literal only when the trigger has no env var - that's the | |
| // hand-written-YAML case we're preserving rather than encouraging. | |
| const webhook: CueSubscription['webhook'] = {}; | |
| if (triggerData.config.webhook_path) webhook.path = triggerData.config.webhook_path; | |
| if (triggerData.config.webhook_secret_env) { | |
| webhook.secret_env = triggerData.config.webhook_secret_env; | |
| } else if (triggerData.config.webhook_secret) { | |
| webhook.secret = triggerData.config.webhook_secret; | |
| } | |
| if (triggerData.config.webhook_signature_header) { | |
| webhook.signature_header = triggerData.config.webhook_signature_header; | |
| } | |
| sub.webhook = webhook; | |
| if (triggerData.config.filter) { | |
| sub.filter = triggerData.config.filter; | |
| } | |
| break; | |
| } | |
| case 'webhook.received': { | |
| // `secret` and `secret_env` are mutually exclusive in the schema, so | |
| // emit the literal only when the trigger has no env var - that's the | |
| // hand-written-YAML case we're preserving rather than encouraging. | |
| const webhook: CueSubscription['webhook'] = {}; | |
| if (triggerData.config.webhook_path) webhook.path = triggerData.config.webhook_path; | |
| const secretEnv = triggerData.config.webhook_secret_env?.trim(); | |
| if (secretEnv) { | |
| webhook.secret_env = secretEnv; | |
| } else if (triggerData.config.webhook_secret) { | |
| webhook.secret = triggerData.config.webhook_secret; | |
| } | |
| if (triggerData.config.webhook_signature_header) { | |
| webhook.signature_header = triggerData.config.webhook_signature_header; | |
| } | |
| sub.webhook = webhook; | |
| if (triggerData.config.filter) { | |
| sub.filter = triggerData.config.filter; | |
| } | |
| break; | |
| } |
🤖 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/renderer/components/CuePipelineEditor/utils/pipelineToYaml.ts` around
lines 195 - 214, Treat whitespace-only webhook_secret_env values as unset in the
webhook.received serialization branch. Update the condition selecting
webhook.secret_env so it requires non-whitespace content, allowing a valid
webhook_secret to be emitted through the fallback path; preserve the existing
precedence for meaningful environment values.
- HMAC now covers the received bytes, not a decoded string. Senders sign the
wire bytes, and round-tripping a non-UTF-8 body through toString('utf8')
substitutes U+FFFD, which changed the digest and rejected legitimately
signed deliveries. Body decoding moved after the signature check, and raw
body truncation now slices the buffer so it can't cut mid-codepoint.
- Trigger group key includes the webhook block. Two webhook.received subs on
different paths collapsed into one visual trigger on load, so the next
editor save rewrote both to a single path and silently broke a live
endpoint.
- readBody distinguishes a size overrun from a stream error: a dropped
connection answered 413 "Payload too large", sending operators after a
limit that was never hit. Stream errors now answer 400.
- A malformed percent-escape (/cue/%) made decodeURIComponent throw, which
surfaced as a 500 and a Sentry report for what is a bad request. Now 404.
- Warn once at startup when MAESTRO_CUE_WEBHOOK_HOST binds past loopback.
It's a supported choice, but a silent one is a footgun.
- Trigger node summary resolves an unset webhook path through the same
name-slug fallback the config panel shows, instead of rendering a bare
/cue/.
Adds regression coverage for each: raw-byte HMAC, malformed escape, stream
error vs size cap, and the two-path round trip.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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/__tests__/renderer/components/CuePipelineEditor/utils/yamlToPipeline.test.ts`:
- Around line 1131-1133: Remove the duplicated triggers.map(...) expression in
the test assertion, leaving a single webhook_path mapping passed to
expect.arrayContaining. Ensure the resulting expect(...).toEqual(...) statement
remains syntactically valid.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: c31eeddc-6431-4939-a549-66f79bcaed8c
📒 Files selected for processing (5)
src/__tests__/main/cue/cue-webhook-server.test.tssrc/__tests__/renderer/components/CuePipelineEditor/utils/yamlToPipeline.test.tssrc/main/cue/cue-webhook-server.tssrc/renderer/components/CuePipelineEditor/utils/pipelineGraph.tssrc/renderer/components/CuePipelineEditor/utils/yamlToPipeline.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- src/renderer/components/CuePipelineEditor/utils/pipelineGraph.ts
- src/main/cue/cue-webhook-server.ts
Comment on lines
+1131
to
+1133
| expect( | ||
| triggers.map((t) => (t.data as { config: { webhook_path?: string } }).config.webhook_path) | ||
| ).toEqual(expect.arrayContaining(['from-github', 'from-ci'])); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🔴 Critical | ⚡ Quick win
Remove the duplicated triggers.map(...) expression.
The supplied final snippet contains the same expression twice at Line 1132 without a separator. If both lines are checked in, the test will not compile. Keep only one expression.
🤖 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/__tests__/renderer/components/CuePipelineEditor/utils/yamlToPipeline.test.ts`
around lines 1131 - 1133, Remove the duplicated triggers.map(...) expression in
the test assertion, leaving a single webhook_path mapping passed to
expect.arrayContaining. Ensure the resulting expect(...).toEqual(...) statement
remains syntactically valid.
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
closes #858
Summary
Adds a
webhook.receivedCue event type so any external service that can send an HTTP POST - GitHub, GitLab, Slack, CI systems, ad-hoc scripts - can trigger a Cue pipeline.Kept deliberately generic as the issue asked: the delivery lands in the event payload and the existing filter engine decides which deliveries matter, rather than hardcoding per-vendor handling. One endpoint can drive several pipelines:
The listener (
src/main/cue/cue-webhook-server.ts)127.0.0.1:17997.MAESTRO_CUE_WEBHOOK_PORT/MAESTRO_CUE_WEBHOOK_HOSToverride. Public delivery is expected to go through a tunnel (ngrok, cloudflared) or reverse proxy rather than a public bind.secret/secret_envfails config validation rather than quietly starting to listen.secret_envis preferred and the two are mutually exclusive.X-Maestro-Cue-SecretorAuthorization: Bearersignature_header, accepting thesha256=<hex>form GitHub and GitLab send - so pointing a GitHub webhook at Maestro needs no glue code.Authorization,Cookie,X-Maestro-Cue-Secret, and the configured signature header are stripped before the event is built, so they can't leak into a prompt, the activity log, or the Cue DB.Wiring
Trigger source, config normalizer + validator, template-context enricher, and five
{{CUE_WEBHOOK_*}}variables (BODY,EVENT,PATH,DELIVERY_ID,HEADERS). The body is pretty-printed JSON, since senders minify and a single-line blob is harder for an agent to reason about.Pipeline editor gets the trigger in the drawer plus a config panel (path / secret env var / signature header) that writes
secret_env, never a literal. A hand-written literalsecretstill round-trips losslessly so opening and saving a pipeline in the editor can't silently strip a working secret off disk.Docs
New event section in
docs/maestro-cue-events.md, an in-app Cue pattern, and aCueHelpModalentry.Testing
enabled()gate, idempotent start/stop).webhookblock.npm run lint,lint:eslint, andprettier --checkclean.Local run is macOS only - the CI matrix still needs to confirm ubuntu and windows.
Open question for the maintainers
The listener is a standalone
http.Serverrather than a route on the existing FastifyWebServer. That keeps webhooks working without the user enabling remote web access, and keeps the auth model per-subscription instead of per-app-token. If you'd rather have this live underWebServer(inheriting its rate limiting and tunnel story), the trigger source and registration API stay unchanged - onlycue-webhook-server.tsswaps out.Summary by CodeRabbit
New Features
webhook.receivedCue event type with a local webhook listener and routing to matching subscriptions.Documentation
webhook.receiveddocumentation with security, delivery behavior, filters, and templates.Tests