feat(engine): optional egress proxy for http_push delivery (use_proxy)#227
Conversation
Adds a per-node opt-in that routes an http_push webhook POST through a
deployment-configured forwarder instead of hitting the destination directly.
Motivation: some receivers sit behind Cloudflare bot protection that blocks the
engine's own network origin (Cloudflare Workers get a zone-level 403), so the
webhook can never be delivered from the Worker directly. Routing through a
non-Cloudflare proxy gets through.
- `httpDeliverySchema` gains `use_proxy?: boolean` (stored in the node's
deliveryConfig; `url` stays the real destination).
- `EngineConfig.httpPushProxy?: { url, secret }` — the deployment's single
forwarder, wired by the host adapter (env/secrets).
- `dispatchHttpPush`: when `use_proxy` is set, POST to the proxy URL with the
real target in `X-Forward-To` and the shared secret in `X-Proxy-Auth`; the
real `url` is still SSRF-checked. If `use_proxy` is set but no proxy is
configured, the delivery fails with a clear error rather than silently going
direct (which would just be blocked).
Tests: routes through the proxy with the right control headers + preserves the
node's own webhook auth header; and fails cleanly when use_proxy is set without
a configured proxy. 19/19 conformance tests pass.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
Warning Review limit reached
Next review available in: 58 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (15)
📝 WalkthroughWalkthroughAdds optional HTTP push egress proxy support: ChangesHTTP push egress proxy
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant Node
participant dispatchHttpPush
participant HttpPushProxy
participant Destination
Node->>dispatchHttpPush: delivery with use_proxy=true
alt httpPushProxy configured
dispatchHttpPush->>HttpPushProxy: POST requestUrl with X-Forward-To, X-Proxy-Auth
HttpPushProxy->>Destination: forward request
else no proxy configured
dispatchHttpPush->>dispatchHttpPush: record retryable failure, delivery stays queued
end
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 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 |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces support for routing http_push node deliveries through an optional egress proxy (httpPushProxy) when use_proxy is enabled, allowing webhook requests to bypass network origin blocks. Feedback highlights a critical security vulnerability where case-insensitive header injection (e.g., using lowercase x-forward-to) could bypass SSRF checks. It is recommended to strip any user-supplied proxy control headers case-insensitively before merging.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| // recorded as a retryable dispatch error rather than rejecting uncaught. | ||
| const headers = await buildHttpPushHeaders(config, args.eventType, args.delivery.id, body, timestamp); | ||
| const response = await globalThis.fetch(url, { | ||
| const headers = { ...(await buildHttpPushHeaders(config, args.eventType, args.delivery.id, body, timestamp)), ...proxyHeaders }; |
There was a problem hiding this comment.
Security Vulnerability: Potential SSRF / Header Injection Bypass\n\nBecause JavaScript object keys are case-sensitive, if a user registers a node with static_headers containing lowercase versions of the proxy control headers (e.g., x-forward-to or x-proxy-auth), they will not be overwritten by the uppercase X-Forward-To and X-Proxy-Auth keys in proxyHeaders.\n\nWhen globalThis.fetch sends the request, both headers will be sent. Many proxy implementations (especially those in Node.js/Express/Hono) automatically lowercase incoming headers and may concatenate duplicates or select the first occurrence. This could allow an attacker to spoof the destination URL and bypass the engine's SSRF checks, routing requests to internal services.\n\nTo prevent this, we should case-insensitively remove any user-supplied proxy control headers from the base headers before merging proxyHeaders.
const baseHeaders = await buildHttpPushHeaders(config, args.eventType, args.delivery.id, body, timestamp);\n for (const key of Object.keys(baseHeaders)) {\n const lower = key.toLowerCase();\n if (lower === 'x-forward-to' || lower === 'x-proxy-auth') {\n delete baseHeaders[key];\n }\n }\n const headers = { ...baseHeaders, ...proxyHeaders };![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: fa319c2832
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| // proxy URL is operator-configured and trusted. | ||
| let requestUrl = url; | ||
| const proxyHeaders: Record<string, string> = {}; | ||
| if (config.use_proxy === true) { |
There was a problem hiding this comment.
When a node sets use_proxy because its receiver blocks the engine origin, this branch only changes durable message dispatch. The existing http_push ephemeral path still calls postEphemeralEventToHttpPushNode from nodeDeliver/nodeContext and fetches config.url directly, so reactions, read receipts, presence, and context updates for the same proxied node continue to bypass the proxy and fail in exactly the environment this option is for. Please pass the configured proxy through that shared path or otherwise make use_proxy apply to all http_push POSTs.
Useful? React with 👍 / 👎.
| // Opt this node's webhook delivery into the deployment-configured egress proxy | ||
| // (EngineConfig.httpPushProxy). `url` stays the real destination; the engine | ||
| // forwards through the proxy at dispatch time. See dispatchHttpPush. | ||
| use_proxy: z.boolean().optional(), |
There was a problem hiding this comment.
This adds a new HTTP JSON field, but the public contract was not updated: openapi.yaml's HttpPushNodeDelivery schema and the TypeScript SDK HttpPushNodeDelivery type still describe only url/ack_mode/auth, and README examples omit the option. The repo's AGENTS.md explicitly requires updating README.md and openapi.yaml together when API behavior changes; without those updates, generated docs/clients and SDK users will not know how to opt into the proxy.
Useful? React with 👍 / 👎.
![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
🤖 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 `@packages/engine/src/routes/deliveryRouting.ts`:
- Around line 257-265: In deliveryRouting.ts, the use_proxy path in the delivery
routing logic only validates proxyCfg.url and then conditionally skips
X-Proxy-Auth when proxyCfg.secret is missing. Update the proxy handling in the
same block that sets requestUrl and proxyHeaders so it fails fast when
httpPushProxy.secret is absent, records the retry/failure, and returns failed
instead of dispatching unauthenticated through the proxy. Reference the existing
use_proxy branch, proxyCfg, and proxyHeaders setup in the delivery routing flow.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 47b1f4b4-8408-4169-83e0-98a0b7e4dfca
📒 Files selected for processing (5)
packages/engine/src/__tests__/conformance/harness.tspackages/engine/src/__tests__/conformance/nodeDeliveryContracts.test.tspackages/engine/src/ports/index.tspackages/engine/src/routes/deliveryRouting.tspackages/engine/src/routes/node.ts
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
5 issues found across 5 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="packages/engine/src/ports/index.ts">
<violation number="1" location="packages/engine/src/ports/index.ts:106">
P2: `httpPushProxy.secret` should be required whenever a proxy is configured. Current type allows `{ url }`, causing proxied deliveries to be sent without `X-Proxy-Auth` despite the proxy contract requiring authenticated forwarding.</violation>
</file>
<file name="packages/engine/src/routes/node.ts">
<violation number="1" location="packages/engine/src/routes/node.ts:52">
P2: The new `use_proxy` wire field is accepted by the API but the public contract (`openapi.yaml`) and SDK types have not been updated. Generated clients and documentation consumers won't know this option exists.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| * rejects Cloudflare Workers). Hosted adapters wire this from their secrets. | ||
| */ | ||
| httpPushProxy?: { | ||
| url?: string; |
There was a problem hiding this comment.
P2: httpPushProxy.secret should be required whenever a proxy is configured. Current type allows { url }, causing proxied deliveries to be sent without X-Proxy-Auth despite the proxy contract requiring authenticated forwarding.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/engine/src/ports/index.ts, line 106:
<comment>`httpPushProxy.secret` should be required whenever a proxy is configured. Current type allows `{ url }`, causing proxied deliveries to be sent without `X-Proxy-Auth` despite the proxy contract requiring authenticated forwarding.</comment>
<file context>
@@ -93,6 +93,19 @@ export interface EngineConfig {
+ * rejects Cloudflare Workers). Hosted adapters wire this from their secrets.
+ */
+ httpPushProxy?: {
+ url?: string;
+ secret?: string;
+ };
</file context>
| // Opt this node's webhook delivery into the deployment-configured egress proxy | ||
| // (EngineConfig.httpPushProxy). `url` stays the real destination; the engine | ||
| // forwards through the proxy at dispatch time. See dispatchHttpPush. | ||
| use_proxy: z.boolean().optional(), |
There was a problem hiding this comment.
P2: The new use_proxy wire field is accepted by the API but the public contract (openapi.yaml) and SDK types have not been updated. Generated clients and documentation consumers won't know this option exists.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/engine/src/routes/node.ts, line 52:
<comment>The new `use_proxy` wire field is accepted by the API but the public contract (`openapi.yaml`) and SDK types have not been updated. Generated clients and documentation consumers won't know this option exists.</comment>
<file context>
@@ -46,6 +46,10 @@ const httpDeliverySchema = z.object({
+ // Opt this node's webhook delivery into the deployment-configured egress proxy
+ // (EngineConfig.httpPushProxy). `url` stays the real destination; the engine
+ // forwards through the proxy at dispatch time. See dispatchHttpPush.
+ use_proxy: z.boolean().optional(),
});
</file context>
PR #227 Review —
|
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
openapi.yaml (1)
652-658: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider adding
default: falsefor consistency.Other optional fields in this schema (e.g.
ack_mode) declare an explicitdefault. Addingdefault: falsehere would make the contract clearer for API consumers, even though the backend already treatsundefinedas falsy.📝 Optional refactor
use_proxy: type: boolean + default: false description: >🤖 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 `@openapi.yaml` around lines 652 - 658, The `use_proxy` schema field is missing an explicit default, unlike nearby optional fields such as `ack_mode`. Update the `use_proxy` definition in the OpenAPI schema to declare `default: false` so the API contract is consistent and clearer for consumers.
🤖 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 `@openapi.yaml`:
- Around line 652-658: The node delivery contract in the README is missing the
new delivery.use_proxy behavior described in openapi.yaml. Update the README.md
section that documents node delivery to include delivery.use_proxy, explain that
it routes webhook delivery through the deployment-configured egress proxy
instead of posting directly to url, and note the queued/error behavior when no
proxy is configured. Keep the wording aligned with the existing delivery
contract docs and match the semantics defined around delivery.use_proxy in the
schema.
---
Nitpick comments:
In `@openapi.yaml`:
- Around line 652-658: The `use_proxy` schema field is missing an explicit
default, unlike nearby optional fields such as `ack_mode`. Update the
`use_proxy` definition in the OpenAPI schema to declare `default: false` so the
API contract is consistent and clearer for consumers.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: ec9d8c6b-e443-4560-b53c-655a8d0d7b0e
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (1)
openapi.yaml
…ephemeral path, docs Addresses PR review (Gemini/cubic/CodeRabbit/Codex): - SSRF/header-injection (HIGH): operator static_headers could inject case-variant `x-forward-to`/`x-proxy-auth` and repoint the proxy at an internal address. Add both to RELAYCAST_CONTROLLED_HEADERS so they can never be set via static_headers (case-insensitive), for durable and ephemeral paths. Test asserts a malicious `x-forward-to` static header is dropped. - Require proxy secret: `use_proxy` now fails unless BOTH proxy url and secret are configured (no unauthenticated proxy POST). Extracted a shared `resolveHttpPushProxy` helper; dispatchHttpPush uses it. Test covers the url-without-secret case. - Ephemeral path: `postEphemeralEventToHttpPushNode` now honors `use_proxy` (via the shared helper) and also fixes its own `redirect:'error'` → `'manual'` (same Workers bug). `httpPushProxy` threaded through NodeDeliverDeps/NodeContextDeps and all ephemeral deps construction sites. - Docs: document `use_proxy` in openapi.yaml (HttpPushNodeDelivery), the TS SDK type (`useProxy`), and README, per AGENTS.md. 20/20 conformance tests pass; engine + SDK tsc clean. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
Thanks all — addressed the review in SSRF / header-injection (Gemini HIGH, cubic P1) — Fixed at the root: added Require proxy secret (CodeRabbit Major, cubic P2) — Honor proxy for all http_push POSTs (Codex P2) — Document the wire field (Codex P1) — Added 20/20 conformance tests pass; engine + SDK |
1 participant
What
Adds a per-node opt-in —
delivery.use_proxy: true— that routes an http_push webhook POST through a deployment-configured forwarder instead of hitting the destination directly.Why
Some webhook receivers sit behind Cloudflare bot protection that blocks the engine's own network origin — a Cloudflare Worker gets a zone-level 403 on every request (verified exhaustively: bare GET to the receiver's homepage from a Worker → 403; identical request from curl / AWS / GCP → normal). So for those receivers the webhook can never be delivered directly from the Worker. Routing through a non-Cloudflare proxy gets through.
This generalizes the one-off proxy we stood up for a specific agent into a first-class, reusable flag any http_push node can opt into.
How
httpDeliverySchemagainsuse_proxy?: boolean(stored in the node'sdeliveryConfig;urlstays the real destination).EngineConfig.httpPushProxy?: { url, secret }— the deployment's single forwarder, wired by the host adapter from env/secrets.dispatchHttpPush: whenuse_proxyis set, POST to the proxy URL with:X-Forward-To: <real destination>X-Proxy-Auth: <shared secret>The real
urlis still SSRF-checked; the proxy URL is operator-configured and trusted. Ifuse_proxyis set but no proxy is configured, the delivery fails with a clear error (no http_push proxy configured) rather than silently going direct (which would just be blocked).The forwarder (non-CF host) validates
X-Proxy-Auth, readsX-Forward-To, and forwards the body + webhook auth headers.Tests (19/19 conformance pass)
use_proxyis set without a configured proxy (delivery stays queued, error recorded, no webhook POST).Deploy to activate (host side)
@relaycast/engine.relaycast-cloud: bump the engine, setHTTP_PUSH_PROXY_URL/HTTP_PUSH_PROXY_SECRET, wire them intoEngineConfig.httpPushProxy(adapter change staged), redeploy.delivery.use_proxy: trueand the realurl.🤖 Generated with Claude Code