ADE-68: Orchestration & CTO: worker sandbox escape / webFetch SSRF (P1) + plan.md images, dead CTO buttons, manifest integrity, automation guard, dead webhook, decomposition by arul28 · Pull Request #496 · arul28/ADE

arul28 · 2026-05-31T23:56:48Z

Refs ADE-68

Summary

Describe the change.

What Changed

Key files and behaviors.

Validation

How you tested.

Risks

Anything to watch.

Linked Linear issues

ADE-68: Orchestration & CTO: worker sandbox escape / webFetch SSRF (P1) + plan.md images, dead CTO buttons, manifest integrity, automation guard, dead webhook, decomposition - referenced

ADE Open in ADE · ade-68-orchestration-cto-worker-sandbox-escape-webfetch-ssrf-p1-plan-md-images-dead-cto-buttons-manifest-integrity-automation-guard-dead-webhook-decomposition branch · PR #496

Greptile Summary

This PR addresses a broad set of security hardening and correctness issues across the orchestration layer, webFetch SSRF guards, worker sandbox, CTO UI, and automation scheduling. The changes are generally well-scoped and come with solid test coverage.

SSRF hardening: webFetch.ts now resolves DNS once, validates every returned address, then connects to the pinned IP via http/https.request while preserving Host/SNI — closing the DNS-rebinding window flagged in a prior review. Reserved IPv4 ranges are now blocked with a new test file covering the guard.
Worker sandbox escape: buildOrchestrationSandboxConfig strips node and tsx from the safe-command list; a new early-exit in checkWorkerSandbox blocks interpreter-payload commands when blockByDefault is true.
Heartbeat separation: Agent heartbeats are now written to a separate heartbeats.json file, eliminating etag churn and concurrency conflicts. Automation run serialization queues concurrent triggers rather than dropping them. The missing localSigningSecret = payload.signingSecret assignment in linearIngressService.ts is fixed.

Confidence Score: 5/5

Safe to merge; the changes are net-positive security hardening with good test coverage across all major paths.

The DNS-pinning SSRF fix, sandbox-escape mitigations, manifest integrity checks, and heartbeat separation are all well-implemented and covered by new tests. The one open gap (unbounded response-body buffering in requestPinnedTarget) is a latent DoS surface rather than a code-correctness regression, and the 15-second AbortController timeout bounds the worst case.

webFetch.ts — the requestPinnedTarget function buffers the full HTTP response body with no size cap.

Important Files Changed

Filename	Overview
apps/desktop/src/main/services/ai/tools/webFetch.ts	Major SSRF hardening: DNS-pinning via http/https.request with Host/SNI preservation, redirect validation, full IPv4/IPv6 block-list. One P2: response body is buffered in full with no size cap.
apps/desktop/src/main/services/ai/tools/orchestrationRuntime.ts	Filters node/tsx patterns from orchestration sandbox safe-command list to prevent interpreter-payload sandbox escapes.
apps/desktop/src/main/services/ai/tools/universalTools.ts	Adds early-exit in checkWorkerSandbox that blocks interpreter-payload commands regardless of safe-list when blockByDefault is enabled.
apps/desktop/src/main/services/automations/automationService.ts	Replaces drop-concurrent-trigger deduplication with a per-rule promise queue that serializes runs; adds 750 ms file-change debouncing with proper cleanup.
apps/desktop/src/main/services/cto/linearIngressService.ts	One-line bug fix: localSigningSecret = payload.signingSecret was missing, preventing HMAC verification of locally-delivered webhooks.
apps/desktop/src/main/services/orchestration/orchestrationService.ts	Heartbeats now written to heartbeats.json instead of manifest.json, eliminating etag/concurrency conflicts; validateManifestShape now enforced on internal patches.
apps/desktop/src/main/services/orchestration/patchPolicy.ts	Workers must now claim a task before patching it; unassigned tasks return a distinct must-claim error message.
apps/desktop/src/main/services/cto/linearDispatcherRunStore.ts	New file: extracted run/step/event persistence helpers from linearDispatcherService.ts into a dedicated store module for cleaner decomposition.
apps/desktop/src/main/services/cto/ctoPromptContent.ts	New file: dynamically generates the CTO capability manifest by reflecting on createCtoOperatorTools, replacing the static hardcoded string.
apps/desktop/src/renderer/components/cto/CtoPage.tsx	Fixes dead Wake/Edit buttons in the team grid to target the card agent.id directly; outer button changed to div[role=button] to avoid nested-button HTML violation.
apps/desktop/src/main/services/orchestration/manifestNormalization.ts	Adds validateAgents to enforce at least one agent and no duplicate sessionId values in manifest validation.
apps/desktop/src/renderer/components/orchestration/PlanMarkdown.tsx	Images in plan.md now resolve to file:// URLs via bundleAssetFileUrl instead of falling back to the raw relative path when no resolver is provided.

Sequence Diagram

sequenceDiagram
    participant Tool as webFetch tool
    participant SSRF as assertSafeWebFetchUrl
    participant DNS as dns.lookup
    participant Node as http/https.request (pinned IP)
    participant Server as Remote Server

    Tool->>SSRF: assertSafeWebFetchUrl(startUrl)
    SSRF->>DNS: "lookup(hostname, {all:true})"
    DNS-->>SSRF: [ip1, ip2, ...]
    SSRF->>SSRF: isBlockedNetworkAddress(each ip)
    SSRF-->>Tool: "{resolvedAddress, hostHeader, servername, url}"

    loop up to MAX_REDIRECTS (5)
        Tool->>Node: requestPinnedTarget(pinnedTarget, signal)
        Note over Node,Server: hostname=resolvedAddress, Host=hostHeader, SNI=servername
        Node->>Server: TCP connect to resolved IP
        Server-->>Node: HTTP response
        Node-->>Tool: Response

        alt 3xx + Location header
            Tool->>SSRF: assertSafeWebFetchUrl(redirectUrl)
            SSRF->>DNS: "lookup(newHostname, {all:true})"
            DNS-->>SSRF: [newIp, ...]
            SSRF-->>Tool: new pinnedTarget
        else non-redirect
            Tool-->>Tool: return Response
        end
    end

Comments Outside Diff (2)

apps/desktop/src/renderer/components/orchestration/PlanMarkdown.tsx, line 238-245 (link)

Mermaid SVG injected via dangerouslySetInnerHTML

The svg string produced by mermaid.render() is inserted directly into the DOM without any secondary sanitization. Mermaid's securityLevel: "strict" is the only safeguard. If a bug in mermaid's renderer allows a crafted diagram (e.g. injected via plan.md written by a prompt-injected agent) to escape the strict mode sandbox, the resulting SVG would execute arbitrary scripts in the Electron renderer process. Consider running the SVG through a DOMPurify pass after rendering (DOMPurify.sanitize(res.svg, { USE_PROFILES: { svg: true, svgFilters: true } })) before setting dangerouslySetInnerHTML.

Prompt To Fix With AI

This is a comment left during a code review.
Path: apps/desktop/src/renderer/components/orchestration/PlanMarkdown.tsx
Line: 238-245

Comment:
**Mermaid SVG injected via `dangerouslySetInnerHTML`**

The `svg` string produced by `mermaid.render()` is inserted directly into the DOM without any secondary sanitization. Mermaid's `securityLevel: "strict"` is the only safeguard. If a bug in mermaid's renderer allows a crafted diagram (e.g. injected via `plan.md` written by a prompt-injected agent) to escape the strict mode sandbox, the resulting SVG would execute arbitrary scripts in the Electron renderer process. Consider running the SVG through a DOMPurify pass after rendering (`DOMPurify.sanitize(res.svg, { USE_PROFILES: { svg: true, svgFilters: true } })`) before setting `dangerouslySetInnerHTML`.

How can I resolve this? If you propose a fix, please make it concise.

apps/desktop/src/main/services/ai/tools/workerSandboxDefaults.ts, line 44-54 (link)

node remains in the non-orchestration safeCommands allow-list

DEFAULT_WORKER_SANDBOX_CONFIG includes "^node(\\.exe)?\\s" as a safe command with blockByDefault: false. Any non-orchestration worker that uses this default config can execute arbitrary node ... commands. buildOrchestrationSandboxConfig correctly strips this for orchestration workers, but the base config is permissive. Consider whether it would be safer to flip blockByDefault to true for the base config, or to remove node from the base allow-list and add it back only where explicitly needed.

Prompt To Fix With AI

This is a comment left during a code review.
Path: apps/desktop/src/main/services/ai/tools/workerSandboxDefaults.ts
Line: 44-54

Comment:
**`node` remains in the non-orchestration `safeCommands` allow-list**

`DEFAULT_WORKER_SANDBOX_CONFIG` includes `"^node(\\.exe)?\\s"` as a safe command with `blockByDefault: false`. Any non-orchestration worker that uses this default config can execute arbitrary `node ...` commands. `buildOrchestrationSandboxConfig` correctly strips this for orchestration workers, but the base config is permissive. Consider whether it would be safer to flip `blockByDefault` to `true` for the base config, or to remove `node` from the base allow-list and add it back only where explicitly needed.

How can I resolve this? If you propose a fix, please make it concise.

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

Prompt To Fix All With AI

Fix the following 2 code review issues. Work through them one at a time, proposing concise fixes.

---

### Issue 1 of 2
apps/desktop/src/main/services/ai/tools/webFetch.ts:161-164
The response body is collected into an unbounded in-memory buffer. A server returning a multi-hundred-MB body (or an attacker deliberately slow-draining a large response) will hold that memory for the full 15 s timeout. The tool's `max_chars` limit is applied only *after* the full body is decoded, so it offers no protection here. Adding a hard cap (e.g. 10 MB) and destroying the request once it is exceeded closes this DoS path cleanly.

```suggestion
      const MAX_BODY_BYTES = 10 * 1024 * 1024; // 10 MB
      let bodyBytes = 0;
      const chunks: Buffer[] = [];
      response.on("data", (chunk) => {
        const buf = Buffer.isBuffer(chunk) ? chunk : Buffer.from(chunk);
        bodyBytes += buf.byteLength;
        if (bodyBytes > MAX_BODY_BYTES) {
          request.destroy(new Error(`Response body exceeds ${MAX_BODY_BYTES} bytes`));
          return;
        }
        chunks.push(buf);
      });
```

### Issue 2 of 2
apps/desktop/src/main/services/ai/tools/webFetch.ts:87-98
The `throw` after the loop is unreachable. The loop body either returns the response (non-redirect or no `location` header) or throws (when `redirects === MAX_REDIRECTS` with an active redirect). Control can never fall through the closing brace of the loop to this statement. A comment clarifying this avoids confusion about whether there is an unhandled exit path.

```suggestion
  for (let redirects = 0; redirects <= MAX_REDIRECTS; redirects += 1) {
    const response = await requestPinnedTarget(current, signal);
    if (response.status < 300 || response.status >= 400) return response;
    const location = response.headers.get("location");
    if (!location) return response;
    if (redirects === MAX_REDIRECTS) {
      throw new Error(`Too many redirects (>${MAX_REDIRECTS})`);
    }
    current = await assertSafeWebFetchUrl(new URL(location, current.url).toString());
    onUrl(current.url.toString());
  }
  // unreachable — loop always returns or throws on the last iteration
  throw new Error(`Too many redirects (>${MAX_REDIRECTS})`);
```

_{Reviews (5): Last reviewed commit: "ship: fix webFetch pinning and cancellat..." | Re-trigger Greptile}

linear-code · 2026-05-31T23:56:51Z

ADE-68

vercel · 2026-05-31T23:56:53Z

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment

Project	Deployment	Actions	Updated (UTC)
ade	Ignored	Preview	Jun 1, 2026 1:01am

coderabbitai · 2026-05-31T23:56:54Z

Warning

Review limit reached

@arul28, we couldn't start this review because you've reached your PR review rate limit.

More reviews will be available in 36 minutes and 5 seconds. Learn how PR review limits work.

Your organization has run out of usage credits. Purchase more in the billing tab.

⌛ How to resolve this issue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available.

Please see our Fair Usage Limits Policy for further information.

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 71964a84-8a18-4bed-be70-0e479902e69a

📥 Commits

Reviewing files that changed from the base of the PR and between c0cb436 and 9d93933.

📒 Files selected for processing (26)

apps/desktop/src/main/services/ai/tools/bashHardening.test.ts
apps/desktop/src/main/services/ai/tools/orchestrationRuntime.ts
apps/desktop/src/main/services/ai/tools/orchestrationTools.test.ts
apps/desktop/src/main/services/ai/tools/universalTools.ts
apps/desktop/src/main/services/ai/tools/webFetch.test.ts
apps/desktop/src/main/services/ai/tools/webFetch.ts
apps/desktop/src/main/services/ai/tools/workerSandboxDefaults.ts
apps/desktop/src/main/services/automations/automationService.test.ts
apps/desktop/src/main/services/automations/automationService.ts
apps/desktop/src/main/services/cto/ctoPromptContent.ts
apps/desktop/src/main/services/cto/ctoStateService.ts
apps/desktop/src/main/services/cto/linearDispatcherRunStore.ts
apps/desktop/src/main/services/cto/linearDispatcherService.ts
apps/desktop/src/main/services/cto/linearIngressService.ts
apps/desktop/src/main/services/cto/linearIntake.test.ts
apps/desktop/src/main/services/orchestration/manifestNormalization.ts
apps/desktop/src/main/services/orchestration/orchestrationService.test.ts
apps/desktop/src/main/services/orchestration/orchestrationService.ts
apps/desktop/src/main/services/orchestration/patchPolicy.test.ts
apps/desktop/src/main/services/orchestration/patchPolicy.ts
apps/desktop/src/renderer/components/cto/CtoPage.tsx
apps/desktop/src/renderer/components/cto/ctoUi.test.tsx
apps/desktop/src/renderer/components/orchestration/OrchestrationPanel.tsx
apps/desktop/src/renderer/components/orchestration/PlanMarkdown.test.tsx
apps/desktop/src/renderer/components/orchestration/PlanMarkdown.tsx
apps/desktop/src/shared/types/orchestration.ts

✨ Finishing Touches

🧪 Generate unit tests (beta)

Create PR with unit tests
Commit unit tests in branch ade-68-orchestration-cto-worker-sandbox-escape-webfetch-ssrf-p1-plan-md-images-dead-cto-buttons-manifest-integrity-automation-guard-dead-webhook-decomposition

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.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

arul28 · 2026-05-31T23:57:00Z