Skip to content

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

Merged
nic-6443 merged 6 commits into
apache:masterfrom
Arjen10:fix/workflow-check-schema
Sep 10, 2026
Merged

fix(workflow): reject invalid case expressions and missing action conf#13862
nic-6443 merged 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

@nic-6443
nic-6443 merged commit 9aee46a into apache:master Sep 10, 2026
16 checks passed
@Arjen10
Arjen10 deleted the fix/workflow-check-schema branch September 10, 2026 06:09
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

5 participants