Skip to content

fix(workflow): reject invalid case expressions and missing action conf - #13862

Open
Arjen10 wants to merge 6 commits into
apache:masterfrom
Arjen10:fix/workflow-check-schema
Open

fix(workflow): reject invalid case expressions and missing action conf#13862
Arjen10 wants to merge 6 commits into
apache:masterfrom
Arjen10:fix/workflow-check-schema

Conversation

@Arjen10

@Arjen10 Arjen10 commented Aug 21, 2026

Copy link
Copy Markdown
Contributor

Description

check_schema inspected the wrong variable after expr.new, so illegal case values passed validation and matched every request at runtime. Missing action conf also indexed a nil value and 500'd the Admin API.

Which issue(s) this PR fixes:

Fixes #13910

Checklist

  • I have explained the need for this PR and the problem it solves
  • I have explained the changes or the new features added to this PR
  • I have added tests corresponding to this change
  • I have updated the documentation to reflect this change
  • I have verified that this change is backward compatible (If not, please discuss on the APISIX mailing list first)

check_schema inspected the wrong variable after expr.new, so illegal
case values passed validation and matched every request at runtime.
Missing action conf also indexed a nil value and 500'd the Admin API.
@dosubot dosubot Bot added size:M This PR changes 30-99 lines, ignoring generated files. bug Something isn't working labels Aug 21, 2026
@janiussyafiq

Copy link
Copy Markdown
Contributor

Thanks for fixing the CI lint, however a few points need to be addressed:

  1. CI lint. apisix/plugins/workflow.lua:144 uses the global type without local type = type, so lj-releng errors. You can run CI locally first to check if it passes.
  2. Empty actions: [] still crashes at runtime. Schema has minItems on each action but not on actions itself, so the validation loop is a no-op. Admin API returns passed, then every request on that rule gets 500 at workflow.lua:174 (rule.actions[1] is nil).
  3. Non-string action name 500s. workflow.lua:141 concatenates action[1], so [true], [{}], [null] crash with attempt to concatenate.
  4. Test gap. New tests only unit-call check_schema. No Admin-API-level error_code: 400 assertion for the issue's repro, no empty-actions case. t/plugin/workflow2.t:316 has the pattern to reuse.

Design:
One schema change beats the guard by declaring actions.items as [string, object] with minItems: 2. WDYT?

@Arjen10

Arjen10 commented Sep 4, 2026

Copy link
Copy Markdown
Contributor Author

Thanks for fixing the CI lint, however a few points need to be addressed:

  1. CI lint. apisix/plugins/workflow.lua:144 uses the global type without local type = type, so lj-releng errors. You can run CI locally first to check if it passes.
  2. Empty actions: [] still crashes at runtime. Schema has minItems on each action but not on actions itself, so the validation loop is a no-op. Admin API returns passed, then every request on that rule gets 500 at workflow.lua:174 (rule.actions[1] is nil).
  3. Non-string action name 500s. workflow.lua:141 concatenates action[1], so [true], [{}], [null] crash with attempt to concatenate.
  4. Test gap. New tests only unit-call check_schema. No Admin-API-level error_code: 400 assertion for the issue's repro, no empty-actions case. t/plugin/workflow2.t:316 has the pattern to reuse.

Design: One schema change beats the guard by declaring actions.items as [string, object] with minItems: 2. WDYT?

Thank you for the code review. i went throuth the workflow plugin doc again. i think your solution is better. i will fix it

@Arjen10

Arjen10 commented Sep 7, 2026

Copy link
Copy Markdown
Contributor Author

merge #13921

Comment thread apisix/plugins/workflow.lua

@membphis membphis left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working size:M This PR changes 30-99 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

bug: workflow check_schema accepts invalid case expressions and 500s on missing action conf

4 participants