Reject if conditions on wait, wait-all, and cancel steps

workflow-parser/src/model/convert.test.ts

@@ -868,4 +868,78 @@ jobs:
       expect(template.errors!.some(e => e.Message.includes("Nested 'parallel' blocks are not allowed"))).toBe(true);
     });
   });
+
+  describe("conditions on control-flow steps", () => {
+    const conditionError =
+      "'if' is not supported on 'wait', 'wait-all', and 'cancel' steps. These steps always run and cannot be conditional.";
+
+    it.each([
+      [
+        "wait",
+        `      - id: bg\n        run: echo hi\n        background: true\n      - if: \${{ always() }}\n        wait: bg`
+      ],
+      [
+        "wait-all",
+        `      - id: bg\n        run: echo hi\n        background: true\n      - if: \${{ always() }}\n        wait-all:`
+      ],
+      [
+        "cancel",
+        `      - id: bg\n        run: echo hi\n        background: true\n      - if: \${{ always() }}\n        cancel: bg`
+      ]
+    ])("rejects 'if' on a %s step", async (_name, stepsYaml) => {
+      const context = new TemplateContext(new TemplateValidationErrors(), getWorkflowSchema(), nullTrace);
+      context.state.featureFlags = new FeatureFlags({allowBackgroundSteps: true});
+
+      const result = parseWorkflow(
+        {
+          name: "wf.yaml",
+          content: `on: push
+jobs:
+  build:
+    runs-on: ubuntu-latest
+    steps:
+${stepsYaml}`
+        },
+        context
+      );
+
+      const template = await convertWorkflowTemplate(result.context, result.value!, undefined, {
+        errorPolicy: ErrorPolicy.TryConversion,
+        featureFlags: new FeatureFlags({allowBackgroundSteps: true})
+      });
+
+      expect(template.errors).toBeDefined();
+      expect(template.errors!.some(e => e.Message.includes(conditionError))).toBe(true);
+    });
+
+    it("rejects 'if' on a cancel step even when the if expression is invalid", async () => {
+      const context = new TemplateContext(new TemplateValidationErrors(), getWorkflowSchema(), nullTrace);
+      context.state.featureFlags = new FeatureFlags({allowBackgroundSteps: true});
+
+      const result = parseWorkflow(
+        {
+          name: "wf.yaml",
+          content: `on: push
+jobs:
+  build:
+    runs-on: ubuntu-latest
+    steps:
+      - id: bg
+        run: echo hi
+        background: true
+      - if: \${{ not-a-real-function() }}
+        cancel: bg`
+        },
+        context
+      );
+
+      const template = await convertWorkflowTemplate(result.context, result.value!, undefined, {
+        errorPolicy: ErrorPolicy.TryConversion,
+        featureFlags: new FeatureFlags({allowBackgroundSteps: true})
+      });
+
+      expect(template.errors).toBeDefined();
+      expect(template.errors!.some(e => e.Message.includes(conditionError))).toBe(true);
+    });
+  });
 });

workflow-parser/src/model/converter/steps.ts

@@ -15,6 +15,10 @@ import {handleTemplateTokenErrors} from "./handle-errors.js";
 import {IdBuilder} from "./id-builder.js";
 import {FeatureFlags} from "@actions/expressions/features";
 
+// wait, wait-all, and cancel steps always run and cannot be made conditional.
+const CONTROL_FLOW_CONDITION_ERROR =
+  "'if' is not supported on 'wait', 'wait-all', and 'cancel' steps. These steps always run and cannot be conditional.";
+
 export function convertSteps(context: TemplateContext, steps: TemplateToken): Step[] {
   if (!isSequence(steps)) {
     context.error(steps, "Invalid format for steps");
@@ -83,6 +87,7 @@ function convertStep(
   let continueOnError: boolean | ScalarToken | undefined;
   let env: MappingToken | undefined;
   let ifCondition: BasicExpressionToken | undefined;
+  let ifKeyToken: StringToken | undefined;
   for (const item of mapping) {
     const key = item.key.assertString("steps item key");
     switch (key.value) {
@@ -126,6 +131,7 @@ function convertStep(
         env = item.value.assertMapping("step env");
         break;
       case "if":
+        ifKeyToken = key;
         ifCondition = convertToIfCondition(context, item.value);
         break;
       case "continue-on-error":
@@ -162,6 +168,9 @@ function convertStep(
   }
 
   if (wait) {
+    if (ifKeyToken) {
+      context.error(ifKeyToken, CONTROL_FLOW_CONDITION_ERROR);
+    }
     return {
       id: id?.value || "",
       name: name || createSyntheticStepName("Wait"),
@@ -171,6 +180,9 @@ function convertStep(
   }
 
   if (waitAll !== undefined) {
+    if (ifKeyToken) {
+      context.error(ifKeyToken, CONTROL_FLOW_CONDITION_ERROR);
+    }
     return {
       id: id?.value || "",
       name: name || createSyntheticStepName("Wait for all"),
@@ -180,6 +192,9 @@ function convertStep(
   }
 
   if (cancel) {
+    if (ifKeyToken) {
+      context.error(ifKeyToken, CONTROL_FLOW_CONDITION_ERROR);
+    }
     return {
       id: id?.value || "",
       name: name || createSyntheticStepName("Cancel"),

workflow-parser/src/workflow-v1.0.json

@@ -2212,6 +2212,7 @@
         "properties": {
           "name": "step-name",
           "id": "step-id",
+          "if": "step-if",
           "continue-on-error": "step-continue-on-error",
           "wait": {
             "type": "step-wait-target",
@@ -2225,6 +2226,7 @@
         "properties": {
           "name": "step-name",
           "id": "step-id",
+          "if": "step-if",
           "continue-on-error": "step-continue-on-error",
           "wait-all": {
             "type": "step-wait-all-value",
@@ -2238,6 +2240,7 @@
         "properties": {
           "name": "step-name",
           "id": "step-id",
+          "if": "step-if",
           "continue-on-error": "step-continue-on-error",
           "cancel": {
             "type": "step-cancel-target",