Skip to content

feat(tasks): accept client-imported MCP servers on cloud run creation#68954

Open
richardsolomou wants to merge 9 commits into
masterfrom
posthog-code/imported-mcp-servers
Open

feat(tasks): accept client-imported MCP servers on cloud run creation#68954
richardsolomou wants to merge 9 commits into
masterfrom
posthog-code/imported-mcp-servers

Conversation

@richardsolomou

@richardsolomou richardsolomou commented Jul 7, 2026

Copy link
Copy Markdown
Member

Problem

A local PostHog Code task run gets the user's own ~/.claude.json MCP servers; a cloud sandbox only gets the PostHog MCP plus MCP Store installations baked into --mcpServers at spawn. PostHog/code#3231 (client half, behind the posthog-code-local-mcp-import flag) sends the user's importable url-based servers in the run-creation payload; this PR is the backend half that validates, stores, and forwards them. Full design: docs/cloud-mcp-import.md.

Changes

  • Both run-creation endpoints (POST .../run/ and POST .../runs/) accept an optional write-only imported_mcp_servers list, already in the agent server's --mcpServers entry shape ({type: http|sse, name, url, headers: [{name, value}]}).
  • Validation: ≤20 servers, names ≤64 chars and unique, posthog reserved, header values ≤4KB, whole field ≤32KB, and a server-side is_url_allowed SSRF re-check — the client classifies public vs private hosts for UX, but the sandbox egresses from PostHog infra, so a private URL here is a user-controlled SSRF vector.
  • Stored in a new encrypted TaskRun.imported_mcp_servers column (EncryptedJSONStringField, migration 0054) rather than the plain state JSON, because header values carry credentials; never returned by any API response.
  • start_agent_server merges the stored entries into --mcpServers after the PostHog MCP and MCP Store installations; on name collision the existing server wins. Claude-only for now: codex-acp hard-fails the session on any unreachable MCP server and the sandbox does no reachability pruning.
  • The periodic mid-run MCP token refresh replaces the session's server list wholesale, so imported servers ride along there too — otherwise they'd silently vanish at the first refresh.
  • Warm-run reuse and resume paths don't apply imported servers (the sandbox is already spawned); the follow-up is pushing updates through the existing send_refresh_session.
  • MCP relay (desktop-only servers): run creation also accepts relayed_mcp_servers — names only (≤20, deduped case-insensitively, reserved names rejected, disjoint from imported names), stored in a plain TaskRun.relayed_mcp_servers JSONField (migration 0055; no secrets, nothing to encrypt). Launch passes --relayMcpServers to the agent server on both sandbox backends so it can expose loopback relay endpoints. The command endpoint gains an mcp_response method (requestId + server plus exactly one of payload/error, params ≤300KB) forwarded verbatim to the sandbox and deliberately excluded from relay telemetry — response payloads carry data returned by the user's private systems. Client half in PostHog/code#3231; design in that repo's docs/cloud-mcp-relay.md.

How did you test this code?

Automated only (I could not run the suite in this sandbox — no database — so CI is the arbiter):

  • test_create_run_endpoint_persists_imported_mcp_servers_outside_state / test_run_endpoint_persists_imported_mcp_servers: the field round-trips through the encrypted column and never lands in state or the response — no existing test touched the new column.
  • test_create_run_endpoint_rejects_private_imported_mcp_server_urls (FORCE_URL_VALIDATION=True): private/loopback URLs are rejected — catches regressions that would open the SSRF vector.
  • test_create_run_endpoint_rejects_invalid_imported_mcp_servers (parameterized): reserved name, duplicates, >20 servers, oversized header.
  • TestBuildImportedMcpServerConfigs: spawn-side merge drops name collisions and malformed stored entries instead of failing the launch.
  • test_refresh_keeps_imported_mcp_servers: a mid-run refresh_session still includes imported servers — the regression here is them disappearing minutes into a run.
  • Relay: persistence round-trips for relayed_mcp_servers on both endpoints, parameterized invalid cases (case-differing duplicates, >20, reserved, imported-name collision), mcp_response forwarding for payload and error variants plus seven invalid-shape rejections, and TestGetRelayedMcpServerNames for launch-side drift/collision handling.

ruff check and ruff format pass on the changed files.

Automatic notifications

  • Publish to changelog?
  • Alert Sales and Marketing teams?

Docs update

N/A — client-facing docs live in the PostHog/code repo.

@github-actions

github-actions Bot commented Jul 7, 2026

Copy link
Copy Markdown
Contributor

Hey @richardsolomou! 👋

It looks like your git author email on this PR isn't your @posthog.com address (richard@solomou.dev). Since you're on the PostHog team, it's worth pointing your local git author email at your @posthog.com address. Why it matters:

  • Consistent work identity in git history — internal tooling that attributes commits to team members keys off your @posthog.com address.
  • Keeps team contributions easy to tell apart from external community ones when scanning history.

You can fix it for this repo with:

git config user.email "you@posthog.com"

Or set it globally with git config --global user.email "you@posthog.com". No need to redo this PR — just a nudge for next time. 🙂

@assign-reviewers-posthog assign-reviewers-posthog Bot requested a review from a team July 7, 2026 14:27
@github-actions

github-actions Bot commented Jul 7, 2026

Copy link
Copy Markdown
Contributor

Migration SQL Changes

Hey 👋, we've detected some migrations on this PR. Here's the SQL output for each migration, make sure they make sense:

products/tasks/backend/migrations/0054_taskrun_imported_mcp_servers.py

BEGIN;
--
-- Add field imported_mcp_servers to taskrun
--
ALTER TABLE "posthog_task_run" ADD COLUMN "imported_mcp_servers" text NULL;
COMMIT;

products/tasks/backend/migrations/0055_taskrun_relayed_mcp_servers.py

BEGIN;
--
-- Add field relayed_mcp_servers to taskrun
--
ALTER TABLE "posthog_task_run" ADD COLUMN "relayed_mcp_servers" jsonb NULL;
COMMIT;

Last updated: 2026-07-09 00:40 UTC (555e3d7)

@github-actions

github-actions Bot commented Jul 7, 2026

Copy link
Copy Markdown
Contributor

🔍 Migration Risk Analysis

We've analyzed your migrations for potential risks.

Summary: 2 Safe | 0 Needs Review | 0 Blocked

✅ Safe

Brief or no lock, backwards compatible

tasks.0054_taskrun_imported_mcp_servers
  └─ #1 ✅ AddField
     Adding nullable field requires brief lock
     model: taskrun, field: imported_mcp_servers
tasks.0055_taskrun_relayed_mcp_servers
  └─ #1 ✅ AddField
     Adding nullable field requires brief lock
     model: taskrun, field: relayed_mcp_servers

📚 How to Deploy These Changes Safely

AddField:

This operation acquires a brief lock but doesn't rewrite the table.

Deployment uses lock timeouts with automatic retries, so lock contention will cause retries rather than connection pile-up.

Last updated: 2026-07-09 00:40 UTC (555e3d7)

Comment thread products/tasks/backend/presentation/serializers.py
@greptile-apps

greptile-apps Bot commented Jul 7, 2026

Copy link
Copy Markdown
Contributor

Reviews (1): Last reviewed commit: "chore: update OpenAPI generated types" | Re-trigger Greptile

Comment thread products/tasks/backend/temporal/process_task/activities/start_agent_server.py Outdated
Comment thread products/tasks/backend/tests/test_api.py
@github-actions

github-actions Bot commented Jul 7, 2026

Copy link
Copy Markdown
Contributor

🤖 CI report

⚠️ Bundle size — 🔺 +806 B (+0.0%)

Uncompressed size of every built .js bundle, compared against the base branch.

Total: 75.93 MiB · 🔺 +806 B (+0.0%)

No file changed by more than 1000 B.

Posted automatically by build-bundle-size-report · uncompressed bytes from dist-report

Eager graph — within budget

How much code each root ships on the eager path — downloaded and parsed before the surface is interactive. Measured from the esbuild output chunks (post-tree-shake, static imports only); lazy import() / React.lazy chunks are not counted.

Root Eager (shipped) Δ vs base Budget
entry (logged-out pages, app bootstrap)
src/index.tsx
1.20 MiB · 22 files no change ███░░░░░░░ 27.9% of 4.29 MiB
authenticated shell (every logged-in page)
src/scenes/AuthenticatedShell.tsx
8.04 MiB · 2,947 files no change █████████░ 86.9% of 9.25 MiB

🟢 node_modules/monaco-editor/ stays out of src/index.tsx
🟢 src/lib/components/ActivityLog/describers stays out of src/index.tsx
🟢 [object Object] stays out of src/index.tsx
🟢 [object Object] stays out of src/index.tsx
🟢 node_modules/monaco-editor/ stays out of src/scenes/AuthenticatedShell.tsx
🟢 src/lib/components/ActivityLog/describers stays out of src/scenes/AuthenticatedShell.tsx
🟢 [object Object] stays out of src/scenes/AuthenticatedShell.tsx
🟢 [object Object] stays out of src/scenes/AuthenticatedShell.tsx

Largest files eagerly shipped from src/index.tsx
Size File
126.8 KiB ../node_modules/.pnpm/react-dom@18.3.1_react@18.3.1/node_modules/react-dom/cjs/react-dom.production.min.js
24.6 KiB ../node_modules/.pnpm/buffer@6.0.3/node_modules/buffer/index.js
6.3 KiB ../node_modules/.pnpm/react@18.3.1/node_modules/react/cjs/react.production.min.js
4.5 KiB ../node_modules/.pnpm/@jspm+core@2.1.0/node_modules/@jspm/core/nodelibs/browser/process.js
3.9 KiB ../node_modules/.pnpm/scheduler@0.23.2/node_modules/scheduler/cjs/scheduler.production.min.js
1.4 KiB ../node_modules/.pnpm/base64-js@1.5.1/node_modules/base64-js/index.js
1.3 KiB src/RootErrorBoundary.tsx
912 B ../node_modules/.pnpm/ieee754@1.2.1/node_modules/ieee754/index.js
789 B src/scenes/ChunkLoadErrorBoundary.tsx
668 B src/index.tsx
Largest files eagerly shipped from src/scenes/AuthenticatedShell.tsx
Size File
277.1 KiB ../node_modules/.pnpm/posthog-js@1.398.2/node_modules/posthog-js/dist/rrweb.js
266.9 KiB ../node_modules/.pnpm/@posthog+icons@0.37.4_react-dom@18.3.1_react@18.3.1__react@18.3.1/node_modules/@posthog/icons/dist/posthog-icons.es.js
224.6 KiB src/taxonomy/core-filter-definitions-by-group.json
213.6 KiB ../node_modules/.pnpm/posthog-js@1.398.2/node_modules/posthog-js/dist/module.js
160.9 KiB src/queries/validators.js
154.0 KiB ../node_modules/.pnpm/re2js@0.4.1/node_modules/re2js/build/index.esm.js
126.8 KiB ../node_modules/.pnpm/react-dom@18.3.1_react@18.3.1/node_modules/react-dom/cjs/react-dom.production.min.js
106.1 KiB src/lib/api.ts
93.3 KiB ../node_modules/.pnpm/prosemirror-view@1.40.1/node_modules/prosemirror-view/dist/index.js
90.5 KiB ../node_modules/.pnpm/@tiptap+core@3.20.1_@tiptap+pm@3.20.1/node_modules/@tiptap/core/dist/index.js

Posted automatically by check-eager-graph · sizes are eager output bytes (shipped, post-tree-shake) from the esbuild metafile · part of #32479

⚠️ Dist folder size — 🔺 +12.1 KiB (+0.0%)

Total size of the built frontend/dist folder (all assets), compared against the base branch.

Total: 1453.96 MiB · 🔺 +12.1 KiB (+0.0%)

Comment thread products/tasks/backend/temporal/process_task/utils.py
@trunk-io

trunk-io Bot commented Jul 7, 2026

Copy link
Copy Markdown

Static BadgeStatic BadgeStatic BadgeStatic Badge

View Full Report ↗︎Docs

Comment thread products/tasks/backend/presentation/serializers.py
@veria-ai

veria-ai Bot commented Jul 7, 2026

Copy link
Copy Markdown
Contributor

PR overview

All previously flagged issues have been addressed. No open security concerns remain on this pull request.

Security review

No open security issues remain on this pull request.

Fixed/addressed: 1 · PR risk: 0/10

Comment thread products/tasks/backend/presentation/serializers.py Outdated
Backend half of PostHog/code#3231 (importing the user's local url-based
MCP servers into cloud task runs).

- Both run-creation endpoints (POST .../run/ and .../runs/) accept an
  optional write-only imported_mcp_servers list: {type: http|sse, name,
  url, headers: [{name, value}]}, already in the agent server's
  --mcpServers entry shape. Validation: <=20 servers, names <=64 chars,
  unique, "posthog" reserved, header values <=4KB, whole field <=32KB,
  and a server-side is_url_allowed SSRF re-check (the client's
  public/private classification is UX, not a security boundary).
- Stored in a new encrypted TaskRun.imported_mcp_servers column
  (EncryptedJSONStringField) rather than the plain state JSON, since
  header values carry credentials; never returned by the API.
- start_agent_server merges the stored entries into --mcpServers after
  the PostHog MCP and MCP Store installations (existing names win).
  Claude-only for now: codex-acp hard-fails on unreachable servers and
  the sandbox does no reachability pruning.

Generated-By: PostHog Code
Task-Id: 7c87c3a4-0be6-475e-a8ec-269140ded301
- Fetch the TaskRun once in _prepare_launch and reuse it for both the
  event-ingest token and the imported MCP servers, dropping the second
  query on the claude path.
- Assert the run/ endpoint response never echoes imported_mcp_servers,
  mirroring the runs/ test's credential-leak guard.

Generated-By: PostHog Code
Task-Id: 7c87c3a4-0be6-475e-a8ec-269140ded301
The periodic MCP token refresh rebuilds the session's server list as
PostHog MCP + installations only, and the agent treats a
refresh_session list as authoritative — so client-imported servers
silently vanished at the first mid-run token refresh.

- New shared get_imported_mcp_server_configs(task_run, existing_names)
  in utils.py owns the claude-only gate and collision rule; both the
  launch path and _refresh_sandbox_mcp use it now.
- build_imported_mcp_server_configs guards against non-list values in
  the encrypted column so stored drift can't break a launch.
- Tests: refresh keeps imported servers (and posthog-name collisions
  still lose); non-list guard case.

Generated-By: PostHog Code
Task-Id: 7c87c3a4-0be6-475e-a8ec-269140ded301
Address review feedback: the missing-run check was only applied on the
event-ingest path, so with ingest disabled a missing TaskRun row would
silently launch without imported MCP servers instead of failing. Check
unconditionally right after the fetch, raising SandboxExecutionError
per this activity's convention.

Generated-By: PostHog Code
Task-Id: 7c87c3a4-0be6-475e-a8ec-269140ded301
- mypy: type build_imported_mcp_server_configs' input as Any — the
  encrypted column is schemaless at read time and the guards that mypy
  flagged as unreachable exist precisely for non-conforming values;
  give SandboxExecutionError its required cause.
- test_start_agent_server: the launch test now mocks
  TaskRun.objects.filter().first() to match the hoisted single fetch
  (its fake run-id isn't a valid UUID, so the real query would raise).
- test_api: patch is_url_allowed in the two happy-path tests — CI runs
  the real SSRF guard, which does live DNS, and the example.com hosts
  don't resolve. The rejection tests keep exercising the real guard.

Generated-By: PostHog Code
Task-Id: 7c87c3a4-0be6-475e-a8ec-269140ded301
Address review feedback: reserved names were checked case-insensitively
but duplicates case-sensitively, so ["MyServer", "myserver"] slipped
through. Compare both checks on the same lowercased key — two imported
names differing only by case are far more likely a client bug than
intent. Error messages keep the original casing.

Generated-By: PostHog Code
Task-Id: 7c87c3a4-0be6-475e-a8ec-269140ded301
@github-actions

github-actions Bot commented Jul 8, 2026

Copy link
Copy Markdown
Contributor

🦔 Hogbox preview · ✅ ready

▶ Open the preview

🔑 Login test@posthog.com / 12345678 (demo data)
🧩 Running this PR's backend and frontend, on the PostHog :master base
🔗 Link stable across rebuilds — a re-push swaps the box underneath, the URL stays
🔒 Access tailnet only (PostHog VPN)
💤 Idle sleeps after ~30 min idle (snapshot to S3, zero node cost) and wakes on your next visit in ~30s, behind a brief "waking up" screen

commit 555e3d7 · box box-2752abbfa6b2 · ready in 504s (push → usable) · build log · rebuilds on every push, torn down on close

Django half of the PostHog Code MCP relay (docs/cloud-mcp-relay.md in
posthog/code): lets the creating desktop relay stdio/private-URL MCP
servers into a cloud sandbox over the existing event-stream + command
channel.

- TaskRun.relayed_mcp_servers: plain JSONField of {"name": ...} dicts —
  names only, no secrets to encrypt; migration 0055.
- Run creation accepts relayed_mcp_servers on both endpoints (≤20,
  case-insensitive dedupe, reserved names rejected, disjoint from
  imported_mcp_servers), persisted with the same single save as
  imported servers.
- Launch passes --relayMcpServers (names) to the agent server on both
  sandbox backends; name collisions with resolved MCP configs drop.
- Command endpoint allows mcp_response: requestId + server plus exactly
  one of payload/error, params ≤300KB, forwarded verbatim to the
  sandbox and excluded from relay telemetry — response payloads carry
  data from the user's private systems and must not be persisted.

Generated-By: PostHog Code
Task-Id: 7c87c3a4-0be6-475e-a8ec-269140ded301
@github-actions github-actions Bot requested a deployment to preview-pr-68954 July 9, 2026 00:24 In progress
"set_config_option, and mcp_response commands.",
strict_request_validation=True,
)
@action(

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Originally flagged line 1459; mapped to the nearest line in the diff (1371).

🟡 Unbounded relayed MCP responses can be proxied into any active sandbox

The new mcp_response command is accepted and forwarded by the generic run /command endpoint, but there is no check that the target run actually declared the named relayed_mcp_servers during creation. Any user who can control a run can now inject arbitrary mcp_response payloads into any active sandbox session, impersonating a relayed desktop MCP server and potentially steering tool results or exfiltrating agent data through a fake relay channel.

Prompt To Fix With AI
Before proxying an `mcp_response` command, verify that the target TaskRun explicitly opted into relay for the requested `params.server` name. Use the persisted `TaskRun.relayed_mcp_servers` allowlist (case-insensitive) and reject any `mcp_response` whose server name is absent. Keep this guard server-side in the Django endpoint so a client cannot inject responses for undeclared relay servers even if the sandbox agent would accept them.

Severity: medium | Confidence: 89% | React with 👍 if useful or 👎 if not

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.

1 participant