From 5403ee9aa0f191456f2ce86118300f685e5d2387 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E9=9B=B7=E7=94=B5=E8=8A=BD=E8=A1=A3?= Date: Thu, 16 Apr 2026 23:01:23 -0400 Subject: [PATCH] fix(approval-handler): only parse workflow payload for dangerous_command type P1-22: The opportunistic JSON.parse of payloadJson ran for every approval, hoping to find workflowId/stepId. Gate on the actual approval type that carries workflow metadata (dangerous_command, as emitted by runWorkflowPipeline), and validate payload shape via zod. --- .../automation/automation-approval-handler.ts | 82 ++++++++++--------- 1 file changed, 45 insertions(+), 37 deletions(-) diff --git a/src/main/automation/automation-approval-handler.ts b/src/main/automation/automation-approval-handler.ts index d22af82c..e9341e77 100644 --- a/src/main/automation/automation-approval-handler.ts +++ b/src/main/automation/automation-approval-handler.ts @@ -1,3 +1,4 @@ +import { z } from "zod"; import type { AppDatabase } from "../database"; import type { NotificationOptions } from "../notification-service"; import { extractApprovalDocumentIds } from "@shared/approval-documents"; @@ -6,6 +7,12 @@ import { autoAssignSkillsToAgent } from "./automation-skills"; import type { AutomationEventContext } from "./types"; import { normalizeWorkflowSteps } from "./automation-workflow"; +const workflowApprovalPayloadSchema = z.object({ + workflowId: z.string().optional(), + stepId: z.string().optional(), + stepName: z.string().optional(), +}).passthrough(); + function localizeNotification(locale: string | null | undefined, en: string, zh: string): string { return locale === "zh" ? zh : en; } @@ -325,46 +332,47 @@ export function handleApprovalResolved( } } - if (approval.payloadJson) { - try { - const payload = JSON.parse(approval.payloadJson) as { workflowId?: string; stepId?: string }; - if (payload.workflowId && payload.stepId) { - const workflow = db.getWorkflowRecord(payload.workflowId, companyId); - if (workflow) { - const steps = normalizeWorkflowSteps(JSON.parse(workflow.stepsJson || "[]")); - const stepIndex = steps.findIndex((step) => step.id === payload.stepId); - if (stepIndex >= 0) { - if (decision === "approved") { - steps[stepIndex]!.status = "pending"; - db.updateWorkflowExecution({ - id: workflow.id, - companyId, - status: "paused", - currentStepIndex: stepIndex, - stepsJson: JSON.stringify(steps), - lastRunAt: workflow.lastRunAt, - }); - publishDomainChanged(); - void runWorkflowPipeline(workflow.id, companyId).catch((error) => { - logger.error(`[workflow] Failed to resume workflow ${workflow.id} after approval ${approvalId}: ${error instanceof Error ? error.message : String(error)}`); - }); - } else { - steps[stepIndex]!.status = decision === "revision_requested" ? "pending" : "failed"; - db.updateWorkflowExecution({ - id: workflow.id, - companyId, - status: decision === "revision_requested" ? "paused" : "failed", - currentStepIndex: stepIndex, - stepsJson: JSON.stringify(steps), - lastRunAt: workflow.lastRunAt, - }); - publishDomainChanged(); - } + // Workflow-step approvals are emitted by runWorkflowPipeline with + // type "dangerous_command" + workflowId/stepId in payloadJson. Gate the + // parse explicitly on that type instead of attempting a speculative + // JSON.parse for every approval. + if (approval.type === "dangerous_command" && approval.payloadJson) { + const payload = workflowApprovalPayloadSchema.safeParse(JSON.parse(approval.payloadJson)); + if (payload.success && payload.data.workflowId && payload.data.stepId) { + const { workflowId, stepId } = payload.data; + const workflow = db.getWorkflowRecord(workflowId, companyId); + if (workflow) { + const steps = normalizeWorkflowSteps(JSON.parse(workflow.stepsJson || "[]")); + const stepIndex = steps.findIndex((step) => step.id === stepId); + if (stepIndex >= 0) { + if (decision === "approved") { + steps[stepIndex]!.status = "pending"; + db.updateWorkflowExecution({ + id: workflow.id, + companyId, + status: "paused", + currentStepIndex: stepIndex, + stepsJson: JSON.stringify(steps), + lastRunAt: workflow.lastRunAt, + }); + publishDomainChanged(); + void runWorkflowPipeline(workflow.id, companyId).catch((error) => { + logger.error(`[workflow] Failed to resume workflow ${workflow.id} after approval ${approvalId}: ${error instanceof Error ? error.message : String(error)}`); + }); + } else { + steps[stepIndex]!.status = decision === "revision_requested" ? "pending" : "failed"; + db.updateWorkflowExecution({ + id: workflow.id, + companyId, + status: decision === "revision_requested" ? "paused" : "failed", + currentStepIndex: stepIndex, + stepsJson: JSON.stringify(steps), + lastRunAt: workflow.lastRunAt, + }); + publishDomainChanged(); } } } - } catch { - // Non-workflow approvals may use payloadJson for unrelated metadata. } }