fix(worker): expose workflow variables in HTTP step conditions fixes NV-8772 - #12675
nikitagrossman wants to merge 3 commits into
Conversation
…NV-8772 HTTP Request skip conditions and Liquid templates now receive the same workflow namespace as channel steps, including workflow.workflowId mapped from the trigger identifier. Co-authored-by: Cursor <cursoragent@cursor.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
✅ Deploy Preview for dashboard-v2-novu-staging canceled.
|
There was a problem hiding this comment.
🔒 Agentic Security Review — 1 finding (HIGH)
Reviewed PR #12675: fix(worker): expose workflow variables in HTTP step conditions fixes NV-8772
Author: @nikitagrossman
Summary: The fix is correct — HTTP request steps now get the workflow namespace. However, buildWorkflowVariables spreads the entire NotificationTemplateEntity into the Liquid compile context rather than allowlisting only the five fields the dashboard variable picker advertises. For the HTTP step path this is entirely new exposure, and the spread pulls in sensitive sub-documents including the updatedBy populated UserEntity (with no field projection in findById), which contains password (bcrypt hash), resetToken (live account-takeover token), and tokens (OAuth access/refresh tokens).
Sent by Cursor Security Agent: Security Reviewer
Limit what buildWorkflowVariables exposes: add a typed IWorkflowForVariables, import SeverityLevelEnum, and return only advertised fields (workflowId, name, description, tags, severity) so internal persisted fields (ids, rawData, steps) are not leaked. Update tests to assert the exported keys and severity enum. Also reorder an export in utils index (integration-conditions moved) to keep exports consistent.
Add typed helpers and centralize control-schema handling across ConstructFrameworkWorkflow: introduce PersistedControlSchema, toFrameworkSchema, getStepTemplate and getControlSchema to remove repetitive unsafe casts and provide clearer errors for missing templates/controls. Update tests to use DeepPartial helpers and JsonSchemaTypeEnum for fixture schemas. Remove unnecessary casts for PERMISSIVE_EMPTY_SCHEMA and tighten types. In worker HTTP step, add HttpResponseBody type and type-safe tryParseJson, and add a lint ignore for cognitive complexity. These changes improve type safety and reduce repetitive casting.


What changed
HTTP Request steps now evaluate dashboard skip conditions against the same
workflownamespace as every other step type.workflow.workflowIdalso starts resolving on the Framework path.Why
HTTP Request is the only step type that evaluates
controlValues.skiplocally in the worker. Its compile context omittedworkflow, so conditions likeworkflow.name equals Pawancompared againstundefinedand the activity feed showedevaluatedValues: {}.Channel, digest, delay, and throttle steps already get
workflowfromConstructFrameworkWorkflow, but that object is the raw Mongo entity and has noworkflowIdfield — the dashboard picker advertisesworkflow.workflowIdas the trigger identifier.How
buildWorkflowVariables()spreads the workflow entity and mapstriggers[0].identifier→workflowId.fullPayloadForRender.workflowgoes through the same helper.flowchart TD Cond["Dashboard skip condition"] --> Type{"Step type"} Type -->|"email, in-app, sms, chat, push, throttle"| Bridge["SendMessage to bridge"] Type -->|"digest, delay"| AddJob["AddJob to bridge"] Type -->|"http_request"| Local["ExecuteHttpRequestStep evaluateRules"] Bridge --> Helper["buildWorkflowVariables"] AddJob --> Helper Local --> Helper Helper --> Ctx["workflow.name, workflow.workflowId, ..."]Test plan
workflow.nameworkflow.workflowId(trigger identifier)evaluatedValuesis populated instead of{}{{ workflow.name }}compiles in the HTTP request bodyworkflow.workflowIdfor channel stepsbuildWorkflowVariablesunit specOut of scope
step.filtersevaluated byConditionsFilterstill have noworkflownamespace.AddJob), not after the wait window.Fixes NV-8772
Made with Cursor
The PR appears safe to merge.
What we checked:
buildWorkflowVariablesreturns only the five dashboard fields, and the HTTP step uses that result as itsworkflowvalue.Summary
HTTP Request steps now expose the same filtered
workflowvariables as other step types when checking skip rules and rendering request bodies. A shared helper maps the first trigger identifier toworkflow.workflowId, so both worker and Framework paths use the same workflow namespace. Major changes: HTTP steps resolve workflow data before evaluating conditions; Framework steps use the shared workflow payload; tests cover names, IDs, evaluated values, and Liquid rendering.Diagram
sequenceDiagram participant Job as HTTP step job participant Worker as ExecuteHttpRequestStep participant Repo as Workflow repository participant Helper as buildWorkflowVariables participant Rules as Skip rules participant HTTP as HTTP endpoint Job->>Worker: execute(command) alt command includes workflow Worker->>Worker: use command.workflow else command has _templateId Worker->>Repo: findById(_templateId, environmentId) Repo-->>Worker: workflow end Worker->>Helper: buildWorkflowVariables(workflow) Helper-->>Worker: workflowId, name, description, tags, severity Worker->>Rules: evaluate skip with workflow context alt condition says skip Rules-->>Worker: skip Worker-->>Job: SKIPPED else condition says run Rules-->>Worker: run Worker->>HTTP: send compiled request HTTP-->>Worker: response Worker-->>Job: result endReviews (3) · Last reviewed commit: "Refactor schema typing and JSON parsing"