Skip to content

feat: pipeline approval workflows#40

Merged
TerrifiedBug merged 12 commits intomainfrom
feat/approval-workflows
Mar 7, 2026
Merged

feat: pipeline approval workflows#40
TerrifiedBug merged 12 commits intomainfrom
feat/approval-workflows

Conversation

@TerrifiedBug
Copy link
Copy Markdown
Owner

Summary

  • Add DeployRequest model with Prisma migration for tracking deploy approval requests (PENDING/APPROVED/REJECTED/CANCELLED)
  • Add requireDeployApproval toggle to Environment model and settings UI
  • Modify deploy.agent procedure to intercept deploys when approval is required and user role is EDITOR, creating a DeployRequest instead of deploying directly
  • Add listPendingRequests, approveDeployRequest, rejectDeployRequest, and cancelDeployRequest tRPC procedures
  • Update deploy dialog: shows "Request Deploy" for editors when approval is required; switches to review mode for admins with pending requests (approve/reject)
  • Add "Pending Approval" badge to flow toolbar (with cancel button for requester) and pipeline list page
  • Extend withTeamAccess and audit middleware to resolve requestId for DeployRequest entities
  • Add documentation for deploy approval workflows in pipelines and environments guides

Test plan

  • Enable "Require approval for deploys" on an environment and verify editors see "Request Deploy" instead of "Publish to Agents"
  • Submit a deploy request as an editor and verify the "Pending Approval" badge appears in the toolbar and pipeline list
  • As an admin, open the deploy dialog for a pipeline with a pending request and verify the review mode UI (requester info, diff, approve/reject buttons)
  • Approve a deploy request and verify the pipeline is deployed
  • Reject a deploy request with a note and verify the request is marked rejected
  • Cancel your own deploy request from the toolbar and verify it disappears
  • Verify an admin cannot approve their own deploy request
  • Verify admins can still deploy directly even when approval is required
  • Disable "Require approval for deploys" and verify editors can deploy directly again

@github-actions github-actions bot added feature documentation Improvements or additions to documentation and removed feature labels Mar 7, 2026
@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps bot commented Mar 7, 2026

Greptile Summary

This PR adds a deploy approval workflow to VectorFlow: when requireDeployApproval is toggled on an environment, EDITOR-role users submit deploy requests (a DeployRequest record with a frozen YAML snapshot) instead of deploying directly, and ADMINs can then review and approve or reject them from a new review pane in the deploy dialog.

Key observations:

  • Schema & migration are correct — cascade deletes, CUID PKs, and composite indexes follow established conventions.
  • Race-condition handling is well done — a Serializable transaction prevents duplicate pending requests, and an atomic updateMany prevents double-approval; failed deploys revert the request status to PENDING.
  • isAdmin check in deploy-dialog.tsx (line 207) is narrowed to "ADMIN" and excludes "SUPER_ADMIN", so super-admin users will never enter the review pane despite having backend permission to approve/reject. The backend (listPendingRequests) already handles this correctly.
  • withAudit middleware removed from deploy.agent in favour of two inline writeAuditLog(…).catch(() => {}) calls. This deviates from the project's stated security requirement that all mutations must use the audit middleware, and loses automatic input sanitization and (for consistency) before/after entity diffs that the middleware provides.

Confidence Score: 3/5

  • Two bugs should be fixed before merging — a broken SUPER_ADMIN UI path and missing audit middleware compliance — but neither causes data loss or a security bypass in typical usage.
  • Two correctness bugs are present: (1) SUPER_ADMIN users cannot access the deploy request review UI despite having backend permission, affecting any team with super admins; (2) the critical deploy.agent mutation violates the stated security requirement to use withAudit middleware on all mutations, losing automatic input sanitization. The core approval mechanics (Serializable transaction, atomic claim, status revert on failure) are solid, and other tRPC procedures correctly use audit middleware. However, these two issues should be resolved before merging.
  • src/components/flow/deploy-dialog.tsx (line 207 — SUPER_ADMIN check) and src/server/routers/deploy.ts (line 81 — missing withAudit middleware)

Comments Outside Diff (2)

  1. src/components/flow/deploy-dialog.tsx, line 207-208 (link)

    isAdmin check excludes SUPER_ADMIN from review mode.

    Line 207 defines isAdmin as userRole === "ADMIN", which means users with the SUPER_ADMIN role will never enter isReviewMode, even though the backend procedures approveDeployRequest and rejectDeployRequest both use withTeamAccess("ADMIN") — which allows SUPER_ADMIN through the role hierarchy (VIEWER < EDITOR < ADMIN < SUPER_ADMIN).

    A super admin opening this dialog on a pipeline with a pending request will see the normal deploy UI instead of the review pane, with no way to approve or reject the request despite having the backend permission to do so.

    Note that listPendingRequests already handles this correctly (line 293), so the inconsistency is isolated to this component.

  2. src/server/routers/deploy.ts, line 81-82 (link)

    withAudit middleware removed from deploy.agent procedure.

    The procedure at line 73 uses .use(withTeamAccess("EDITOR")) but lacks the .use(withAudit(...)) middleware that the custom instructions require on all mutations. Instead, it uses two manual writeAuditLog(…).catch(() => {}) calls (lines 165-179 and 203-217).

    This creates gaps compared to the centralized audit middleware:

    1. No automatic input sanitization: The middleware automatically redacts sensitive fields (password, token, secret, etc.) from input metadata. The manual calls omit this sanitization — if a new sensitive field is added to the input later, it won't be redacted.
    2. No before/after entity diffs: The middleware computes and stores diffs for update operations. For consistency with other mutations in the codebase, the approach should use the established withAudit pattern.
    3. Inconsistent with project policy: The custom instructions explicitly state "Every mutation MUST use withAudit(action, entityType) middleware" — this mutation is an exception.

    Other procedures in the same router correctly use the middleware (e.g., undeploy at line 225, approveDeployRequest at line 321, rejectDeployRequest at line 382).

    Consider applying the withAudit middleware with appropriate action names for both code paths, or if that's not feasible, at minimum add input sanitization to the manual calls to align with security requirements.

    Rule Used: ## Security & Cryptography Review Rules

    When revi... (source)

Last reviewed commit: 250e53c

@TerrifiedBug TerrifiedBug force-pushed the feat/approval-workflows branch 3 times, most recently from 2c2914a to 39a84c5 Compare March 7, 2026 15:58
Add DeployRequest table to track deploy approval requests with status
tracking (PENDING/APPROVED/REJECTED/CANCELLED), requester/reviewer
relations, and environment-level requireDeployApproval toggle.
Modify the deploy.agent procedure to intercept deploys when the
environment requires approval and the user role is EDITOR. Add
listPendingRequests, approveDeployRequest, rejectDeployRequest,
and cancelDeployRequest procedures. Extend environmentInfo to
return requireDeployApproval flag. Add requestId resolver for
DeployRequest in withTeamAccess and audit middleware.
…e list

Deploy dialog shows "Request Deploy" for editors when approval is
required, and switches to review mode for admins with pending requests.
Flow toolbar displays a "Pending Approval" badge with requester info
and a cancel button for the requester. Pipeline list shows a pending
approval badge on pipelines with outstanding deploy requests.
Add a "Require approval for deploys" switch in the environment edit
form and display the approval status in the deployment overview card.
Document the deploy approval workflow in the pipelines guide and
add a deploy approval section to the environments guide covering
how to enable approval requirements and the review process.
Use useMemo instead of calling Date.now() during render to satisfy
the react-hooks/purity rule.
…pending requests

- Environment update: reject requireDeployApproval changes from EDITORs
  (only ADMINs can toggle the approval requirement)
- Deploy reject: use updateMany with status=PENDING condition to prevent
  race between concurrent approve and reject operations
- Deploy request creation: check for existing PENDING request on the same
  pipeline before creating a new one (returns CONFLICT)
- cancelDeployRequest: use updateMany with status+owner guard (atomic)
- approveDeployRequest: wrap deployAgent in try-catch, revert to PENDING on failure
- Request creation: await validateConfig before storing, fix error message formatting
- detail-panel: use storeKey instead of undefined componentKey for LiveTailPanel
…rom VIEWER responses

- Persist nodeSelector from DeployRequest to pipeline after successful
  approval deploy, matching the direct-deploy path behavior
- Replace withAudit middleware on agent mutation with manual writeAuditLog
  calls: "deploy.agent" for direct deploys, "deploy.request_submitted"
  for pending-approval requests
- Switch listPendingRequests from include to select, only exposing
  configYaml to ADMIN/SUPER_ADMIN callers (contains decrypted secrets)
- Add input.requestId to audit entity ID resolution chain so
  approve/reject/cancel log the DeployRequest ID, not the user ID
- Wrap duplicate-pending check + create in a Serializable transaction
  to prevent TOCTOU race between concurrent editors
@TerrifiedBug TerrifiedBug force-pushed the feat/approval-workflows branch from 39a84c5 to 250e53c Compare March 7, 2026 16:06
@TerrifiedBug
Copy link
Copy Markdown
Owner Author

@greptile review

@TerrifiedBug TerrifiedBug merged commit fe6c87d into main Mar 7, 2026
12 checks passed
@TerrifiedBug TerrifiedBug deleted the feat/approval-workflows branch March 7, 2026 16:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant