Skip to content

feat(deploy): let workspace write access deploy workflows - #6561

Open
mzxchandra wants to merge 19 commits into
stagingfrom
feat/deploy-requires-write
Open

feat(deploy): let workspace write access deploy workflows#6561
mzxchandra wants to merge 19 commits into
stagingfrom
feat/deploy-requires-write

Conversation

@mzxchandra

@mzxchandra mzxchandra commented Aug 11, 2026

Copy link
Copy Markdown
Contributor

Why

Deploying a workflow required workspace admin, so every deploy round-tripped through an admin even though the people who build workflows hold write. This moves the deployment lifecycle to write while keeping admin on the two things that are a different risk class.

Depends on simstudioai/mothership#430 — land that first; sim's generated tool catalog is produced from the contract there and enforced at runtime by the tool executor.

Now write

Deploy, undeploy, public-API toggle, version activate + revert, chat deployments (including the per-deployment password), workflow MCP servers, the v1 API (deploy/undeploy/rollback), and the deploy tool routes.

Upstream's API refactor landed mid-branch and moved permissions out of per-route checks into a central minimumRole on each operation, so the change is now 11 declarations rather than a dozen inline gates:

  • workflows.{deploy, undeploy, chat.deploy, chat.undeploy, versions.activate, versions.revert} (public_api.update deliberately stays admin)
  • mcp_servers.workflow_deployments.{create_server, delete_server, deploy_tool, undeploy_tool} (update_server was already write)

After the change the only admin operations left across all 19 registries are api_keys.copilot.create, workflows.public_api.update, and workflows.policy.update (workflow lock). Public chat auth is gated separately on the chat routes, since it is a parameter value rather than a distinct operation.

Still admin

  • Workspace API keys. A workspace key can invoke every deployed workflow in the workspace. performCreateWorkspaceApiKey now enforces admin at the chokepoint rather than relying on each caller's own gate, so present and future callers inherit it. The MCP-servers settings surface also stopped offering key creation to editors — it passed a write-level flag into a workspace-key modal, which 403'd.
  • Custom blocks, which inject code into every workflow in the workspace.

Unauthenticated exposure stays admin

Two controls did not move to write, because both make a workflow invocable by anyone with no authentication at all — a different risk class from shipping a version. An editor deploys; only an admin exposes.

Workflow public API (workflows.public_api.update stays admin). In the UI the whole Access control in Edit API Info is disabled for non-admins with the tooltip "Only admins can change public API access". The entire ButtonGroup is gated rather than just the Public option: an already-public workflow would otherwise let an editor switch it back to API Key and hit a 403, which is exactly the client/server disagreement this is meant to avoid.

Public chat deployments (authType: 'public'). Deploying a chat stays write — an editor ships password/email/SSO chats freely — but making one public requires admin, enforced on both the create and update routes. Only the transition to public is gated: editing an already-public chat, or moving it off public, stays write, since neither increases exposure. The chat auth selector disables Public for non-admins with a tooltip, and because the create default is public, a non-admin is moved to the first mode they can actually deploy rather than hitting a wall.

The org-level disablePublicApi and allowedChatDeployAuthTypes permission-group flags still apply on top of both.

Also in this PR

A staging regression, fixed. POST /api/workflows/[id]/deployments/[version]/revert returned 400 for every numeric version — the refactor's params schema used a bare z.number(), but route params are always strings, so active was the only reachable value and "Load deployment" was broken end to end. Type-checking could not catch it: the client passes a real number that only becomes a string during URL serialization.

An unrelated flake. markdown-parse.test.ts runs ~13s alone against a 30s cap and timed out under full-suite contention. Easy to drop from this PR if you'd rather it not ride along.

Test Coverage

COVERAGE: 24/25 paths tested (96%)  |  Code: 19/20 (95%)  |  User flows: 5/5 (100%)
QUALITY: ★★★:21 ★★:3  |  GAPS: 1 (UI components — E2E-covered, no unit tests)

Tests 1885 → 1890. The audit was worth running: five operations had zero role assertions, including api_keys.copilot.create. deployment-permission-matrix.test.ts now pins the whole matrix in one place — verified to fail under mutation rather than trivially pass.

Pre-landing review

Three specialists (security, testing, red-team) found 13 issues; 6 auto-fixed, 3 deferred to TODOS.md, 2 were pre-recorded product decisions.

Worth calling out, all confirmed by mutation testing:

  • deployment-permission-matrix.test.ts was never staged — 12 of 13 role changes had no guard without it
  • performCreateWorkspaceApiKey's new forbidden code was wired to 403 on the REST route but not the copilot use case, so a permission denial surfaced as an opaque 500 (found independently by all three reviewers)
  • the version-coercion fix widened the union's z.input to unknown under Zod 4 — the exact drift it existed to prevent; replaced with an explicit transform
  • both chat gates and the promote/undeploy tool routes had no coverage of their required level; reverting them to admin passed the suite

Known gap (deliberate)

The registry declares workspaceApiKey: 'deny' on the deployment operations, but the v1 REST surface predates the registry and runs its own check that resolves a workspace key to its creator — so a workspace key can still deploy there. Pre-existing; this branch lowers the bar behind it from admin to write, meaning a key created by an admin later demoted to editor keeps working. Fixing it breaks anyone deploying via v1 with a workspace key today, so it is tracked as P1 in TODOS.md with its own release note rather than riding along here. The matrix test carries a scope note so it does not assert a guarantee v1 does not keep.

Verification

Type-check clean across 23 workspaces; full suite 22,858 passing; check:api-validation passing.

End-to-end run twice — once before the upstream merge and once after — on a throwaway database with three seeded accounts (admin / write / read), through real HTTP and real auth:

  • write: deploys, redeploys, activates an older version, reverts, toggles public API, creates a password-protected chat and reveals the password, creates an MCP server, undeploys
  • write in the browser: Deploy enabled with "Deploy workflow"; child-workflow badge clickable and clicking it actually deployed the child; MCP detail shows Edit/Delete/Add workflows
  • read: 403 on every mutation; Deploy disabled with "Write permissions required"; child badge not-allowed
  • workspace API keys: 403 for write and read, 200 for admin; the writer's create-key modal offers no Workspace option; the disabled button explains itself; the admin sees the "create one now" link the writer does not

🤖 Generated with Claude Code

https://claude.ai/code/session_01EhfV2XYsLhGp6rqCHVqvQN

Deploying a workflow required workspace admin, forcing every deploy to
round-trip through an admin even though the people who build workflows
hold write. Move the deployment lifecycle to write:

- deploy/undeploy/public-API toggle, version activate + revert
- the v1 API (deploy, undeploy, rollback) and api/tools/deployments/*
- chat deployments, including the per-deployment password
- workflow MCP servers on the copilot side, which was out of step with
  its own HTTP route (already write)

Two things stay admin because they are a different risk class:

- Workspace API keys. A workspace key can invoke every deployed workflow
  in the workspace, so performCreateWorkspaceApiKey now enforces admin at
  the chokepoint rather than relying on each caller's own gate, and the
  MCP-servers settings surface no longer offers key creation to writers
  (it passed a write-level flag into a workspace-key modal, which 403'd).
- Custom blocks, which inject code into every workflow in the workspace.

The public-API toggle moves to write because whether a workspace may
expose public APIs at all already belongs to the org admin via the
permission-group disablePublicApi flag, enforced in the same handler.

Also raises an unrelated markdown property test's timeout, which runs
~13s alone and was flaking against a 30s cap under full-suite load.
staging refactored the API layer: routes became declarative
(defineInternalJsonRoute) and workspace permissions moved from inline
per-route checks to a central `minimumRole` on each operation. Our
permission change is re-expressed in that model rather than at the
callsites it originally touched:

- workflows.{deploy,undeploy,chat.deploy,chat.undeploy,public_api.update,
  versions.activate,versions.revert} -> minimumRole 'write'
- mcp_servers.workflow_deployments.{create_server,delete_server,
  deploy_tool,undeploy_tool} -> 'write' (update_server was already write)

Unchanged and still admin: api_keys.copilot.create, the workspace
api-keys HTTP route, the performCreateWorkspaceApiKey chokepoint,
custom blocks, and workflows.policy.update (workflow lock).

Conflicts taken from staging wholesale where the route or handler was
rewritten around use cases that now enforce the operation; v1
workflows/utils.ts and the deploy modal keep our change on top of
staging's rename and Button->Chip migration.

Retargets one staging test that asserted deploy rejects a write
principal; it now asserts read is rejected and adds the write-allowed
case.
Route params always arrive as strings, but
deploymentVersionOrActiveParamsSchema accepted a bare z.number(), so
every numeric version 400'd and `active` was the only reachable value —
"Load deployment" for a specific version was broken end to end.

Type-checking cannot catch this: useRevertToVersion passes a genuine
number that satisfies the contract and only becomes a string during URL
serialization, so the mismatch appears at runtime. The sibling
deploymentVersionParamsSchema already coerces.

Coercing 'active' yields NaN and fails .int(), falling through to the
literal branch, so the union ordering stays correct.
Specialist review of the deploy-permission change found six issues:

- deployment-permission-matrix.test.ts was never staged. Mutation-proven: 12
  of the 13 role changes had no guard without it.
- performCreateWorkspaceApiKey's new 'forbidden' code was wired to 403 on the
  REST route but not on the copilot use case, so an authorization denial
  surfaced as an opaque 500. Confirmed independently by three reviewers.
- The version-param coercion fix widened the union's z.input to unknown under
  Zod 4, letting a client pass anything at compile time — the exact drift the
  fix existed to prevent. Replaced with an explicit transform so the input
  stays number | string | 'active'.
- Both chat authorization gates and the promote/undeploy tool routes had zero
  coverage of their required level; reverting them to admin passed the suite.
  Added assertions, each verified to fail under mutation.
- Narrowed authorizeDeploymentWorkflow's action union, which retained an
  'admin' arm with no callers.

TODOS.md records three follow-ups, the notable one being that the v1
deployment surface does not honor the registry's workspaceApiKey: 'deny' —
pre-existing, but this branch lowers the bar it sits behind.
@vercel

vercel Bot commented Aug 11, 2026

Copy link
Copy Markdown

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

Project Deployment Actions Updated (UTC)
docs Ready Ready Preview Aug 12, 2026 2:14am

Request Review

@cursor

cursor Bot commented Aug 11, 2026

Copy link
Copy Markdown

PR Summary

High Risk
Widens who can deploy across many surfaces while relying on new admin-only gates for public exposure and API keys. Any missed gate would let editors create unauthenticated endpoints or long-lived workspace credentials.

Overview
Editors with write can now deploy — deploy, undeploy, version activate/revert, chat deployments, and workflow MCP servers no longer require workspace admin. The change is driven by operation minimumRole declarations plus matching route/UI gates.

Unauthenticated exposure stays admin-only. A new canExposePublicly helper gates transitions to public for workflow public APIs, public chats, and public MCP servers. Editors can still edit already-public deployments or move them off public. UI disables the Public option for non-admins with explanatory tooltips.

Workspace API key creation stays admin, now enforced at the performCreateWorkspaceApiKey chokepoint so every caller inherits it. Deploy UI and MCP settings stop offering key creation to editors when personal keys are disallowed.

Reviewed by Cursor Bugbot for commit 8ee0429. Bugbot is set up for automated code reviews on this repo. Configure here.

@greptile-apps

greptile-apps Bot commented Aug 11, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

The PR moves authenticated workflow deployment lifecycle operations from workspace admin to write access while retaining admin-only controls for unauthenticated public exposure and workspace-wide API-key creation.

  • Aligns deployment, rollback, chat, MCP, tool, and generated operation contracts on the write role.
  • Adds explicit admin checks for public chat and MCP exposure.
  • Centralizes workspace API-key creation authorization and corrects forbidden-result handling.
  • Restores numeric deployment-version parsing for route parameters and updates permission coverage.

Confidence Score: 5/5

The PR appears safe to merge because no blocking failure remains.

No blocking failure remains.

Important Files Changed

Filename Overview
apps/sim/lib/workflows/application/operations.ts Aligns workflow deployment operation declarations with workspace write access while retaining admin for public API exposure.
apps/sim/lib/mcp/application/operations.ts Moves workflow MCP deployment lifecycle operations to the write role.
apps/sim/lib/deployments/public-exposure.ts Centralizes the admin-only predicate and transition detection used for unauthenticated public exposure.
apps/sim/app/api/chat/route.ts Allows editors to deploy authenticated chats while requiring admin permission for public chat creation.
apps/sim/app/api/chat/manage/[id]/route.ts Requires admin only when an update transitions an existing chat to public authentication.
apps/sim/app/api/mcp/workflow-servers/route.ts Adds an admin gate when creating a publicly accessible workflow MCP server.
apps/sim/app/api/mcp/workflow-servers/[id]/route.ts Gates private-to-public MCP server transitions while preserving editor updates to already-public servers.
apps/sim/lib/api-key/application/create-api-key.ts Enforces workspace-admin authorization at the workspace API-key creation chokepoint.
apps/sim/app/api/workspaces/[id]/api-keys/route.ts Maps centralized workspace API-key authorization failures to HTTP 403.
apps/sim/lib/api/contracts/deployments.ts Coerces numeric route-parameter strings into positive integer deployment versions while preserving the active sentinel.
apps/sim/lib/core/application/deployment-permission-matrix.test.ts Pins deployment operation role declarations and retained admin-only boundaries across registries.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
  Actor[Workspace member] --> Role{Workspace role}
  Role -->|read| Deny[Reject deployment mutation]
  Role -->|write or admin| Deploy[Authenticated deployment lifecycle]
  Deploy --> Exposure{Creates new public exposure?}
  Exposure -->|no| Allow[Allow operation]
  Exposure -->|yes| Admin{Admin?}
  Admin -->|yes| Allow
  Admin -->|no| Deny
  Actor --> Key[Create workspace API key]
  Key --> KeyAdmin{Admin?}
  KeyAdmin -->|yes| Allow
  KeyAdmin -->|no| Deny
Loading

Reviews (11): Last reviewed commit: "chore: drop the version-param fix and TO..." | Re-trigger Greptile

… change

CI's tool-metadata:check failed: the three deployment tool descriptions moved
from "Requires admin permission" to "Requires write permission" but the
generated metadata they feed was not regenerated.

Scoped regeneration — exactly deployments_deploy, deployments_promote and
deployments_undeploy differ; no tools added, removed, or otherwise changed,
so staging's committed metadata was not itself stale.
@mzxchandra

Copy link
Copy Markdown
Contributor Author

@greptile

@mzxchandra

Copy link
Copy Markdown
Contributor Author

@cursor review

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

✅ Bugbot reviewed your changes and found no new issues!

Comment @cursor review or bugbot run to trigger another review on this PR

Reviewed by Cursor Bugbot for commit 55213ad. Configure here.

@mzxchandra

Copy link
Copy Markdown
Contributor Author

@greptile

@mzxchandra

Copy link
Copy Markdown
Contributor Author

@cursor review

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

✅ Bugbot reviewed your changes and found no new issues!

Comment @cursor review or bugbot run to trigger another review on this PR

Reviewed by Cursor Bugbot for commit d24962a. Configure here.

@mzxchandra

Copy link
Copy Markdown
Contributor Author

@TheodoreSpeaks

@mzxchandra
mzxchandra marked this pull request as draft August 11, 2026 23:00
Deploying moved to write, but enabling the public API is a different risk
class: it makes the workflow callable with no authentication at all,
exposing every credential and environment variable it references to
anonymous callers. An editor can ship a version; only an admin can expose
it to the internet.

- workflows.public_api.update goes back to minimumRole 'admin'
- the Access control in Edit API Info is disabled for non-admins, with a
  tooltip explaining why

The whole ButtonGroup is gated rather than just the Public option: an
already-public workflow would otherwise let an editor switch it back to
API Key and hit a 403, which is the client/server disagreement this is
meant to avoid. An editor still sees the current state, just cannot
change it either way.
@mzxchandra

Copy link
Copy Markdown
Contributor Author

@greptile

@mzxchandra

Copy link
Copy Markdown
Contributor Author

@cursor review

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

✅ Bugbot reviewed your changes and found no new issues!

Comment @cursor review or bugbot run to trigger another review on this PR

Reviewed by Cursor Bugbot for commit 2e17646. Configure here.

@mzxchandra

Copy link
Copy Markdown
Contributor Author

@greptile

Cursor Bugbot found two real gaps in the public-chat gate:

The admin check only ran on the REST chat routes. Copilot deploys through
deployWorkflowChat, which never called it and defaults authType to public
— so a write member could still ship an unauthenticated chat, which is
exactly the boundary the previous commit set out to close. The check now
runs there too.

The helper moved to lib/chat/permissions so all three deploy surfaces
share one definition; lib/ must not import from app/, and a rule
duplicated per callsite is a rule that drifts.

Second finding: a permission group restricted to public-only left a
non-admin with no selectable mode and a submit that would 403. That
combination is intentional rather than a bug — deferring to the group
would let any org grant editors public deploys by narrowing the
allow-list — so the form explains the dead end and blocks the submit
instead of widening the gate.

Adds chat-deployments.public-auth.test.ts covering the copilot path,
verified to fail when the new check is removed.
@mzxchandra

Copy link
Copy Markdown
Contributor Author

@greptile

@mzxchandra

Copy link
Copy Markdown
Contributor Author

@cursor review

Comment thread apps/sim/lib/mcp/application/operations.ts
Third instance of the same class, found by Cursor Bugbot. A public
workflow MCP server skips authentication entirely on the serve path
(api/mcp/serve/[serverId] returns early when isPublic), so anyone with
the URL can invoke every workflow published on it — the same
unauthenticated exposure already kept admin-only for the public workflow
API and public chats. create_server and update_server had moved to write
with no secondary gate on the transition to public.

Renames the helper to canExposePublicly and moves it to
lib/deployments/public-exposure: it now governs four surfaces (workflow
public API via its operation, chat REST, chat copilot, MCP servers), so a
chat-specific name and home no longer described it.

Adds a behavioral test for the MCP path, verified to fail when the gate
is removed. Also drops a cross-surface test added in this round that
grepped source text — it passed with the gate deleted, which makes it
worse than no test.
@mzxchandra

Copy link
Copy Markdown
Contributor Author

@greptile

@mzxchandra

Copy link
Copy Markdown
Contributor Author

@cursor review

…es-write

# Conflicts:
#	apps/sim/lib/api/contracts/deployments.ts
Comment thread apps/sim/lib/mcp/application/workflow-deployments.ts
Comment thread apps/sim/lib/mcp/application/workflow-deployments.ts Outdated
@mzxchandra

Copy link
Copy Markdown
Contributor Author

@greptile

@mzxchandra

Copy link
Copy Markdown
Contributor Author

@cursor review

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

✅ Bugbot reviewed your changes and found no new issues!

Comment @cursor review or bugbot run to trigger another review on this PR

Reviewed by Cursor Bugbot for commit d15ea3c. Configure here.

@mzxchandra
mzxchandra marked this pull request as ready for review August 12, 2026 01:07
The admin-only public-exposure gate lived in the workflow MCP server use
cases, but `POST /api/mcp/workflow-servers` and `PATCH .../[id]` call the
orchestration layer directly and never reach them. `withMcpAuth('write')`
was their only authorization, so a `write` member could publish an
unauthenticated MCP server through Workspace Settings — the surface the
gate was meant to cover.

Gate both routes after auth, and fix the rule while it moves: the check
tested the requested value rather than the transition, so an unchanged
`isPublic: true` — which the edit form resubmits alongside a rename —
would have 403'd a `write` member editing an already-public server. The
transition rule is now `increasesPublicExposure`, shared by the routes
and the use cases.

The settings UI offered the Access control to non-admins with nothing
disabled, so the write path led straight into a 403. Disable it with the
same tooltip the public API and chat surfaces use.

Also stop seeding a non-admin into the chat deploy form's `public`
default: the selector's fallback effect ran before the parent's form
reset and was clobbered back to public, leaving a write member on a
disabled option with Launch Chat inert.

Verified end to end against a `write` member in the browser: rename of a
public server 200s, escalation 403s on both create and update,
de-escalation and private create still work, and the admin path is
unchanged. Covered by route tests and use-case tests, both mutation-checked.

Migrating the two routes onto their use cases is filed in TODOS.md — the
gate is duplicated until then.
@mzxchandra

Copy link
Copy Markdown
Contributor Author

@greptile

@mzxchandra

Copy link
Copy Markdown
Contributor Author

@cursor review

Staging fixed the deployment version route param independently (#6560,
`z.coerce.number()`) and covers both the numeric and `active` cases in
`deployments.test.ts`, so this branch's variant and its regression test
are redundant. Revert the contract to staging's exactly, leaving this PR
scoped to the deploy permission change.

Also drop TODOS.md and rewrite the two comments that pointed at it so
each one states its own condition for removal.
@mzxchandra

Copy link
Copy Markdown
Contributor Author

@greptile

@mzxchandra

Copy link
Copy Markdown
Contributor Author

@cursor review

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

✅ Bugbot reviewed your changes and found no new issues!

Comment @cursor review or bugbot run to trigger another review on this PR

Reviewed by Cursor Bugbot for commit 8ee0429. Configure here.

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.

2 participants