Improve validate.py: fake ID rejection, severity docs, and correct cel_expression+else handling - #8
Open
mraible wants to merge 9 commits into
Open
Conversation
…ion (#4) * Add Fusion SOAR JSON structure reference from Charlotte team Authoritative BPMN-based JSON schema doc covering the internal workflow model: triggers, activities, flows, gateways, submodels (loops), CEL extensions (cs.* functions), data references, custom variables, and three complete examples. Updates SKILL.md to reference the new doc when authoring workflows. * Add local structural validation to validate.py Adds a structural_check() step between pre-flight and API validation that catches schema errors without needing API credentials. Validates: action IDs are 32-char hex, actions have required id/name fields, class-based actions include version_constraint, trigger type is valid, next references resolve to defined labels, loop for.input matches a trigger parameter, and data references have balanced brackets. Also adds PyYAML to requirements.txt (needed for yaml.safe_load) and 11 new tests covering each validation rule (93 total, all passing). * Bump dependencies to latest versions crowdstrike-falconpy 1.6.1 → 1.6.2 pytest 9.0.2 → 9.0.3 PyYAML 6.0.2 → 6.0.3
* Fix README install instructions - Replace generic SKILL-NAME placeholder with actual plugin name (fusion-workflows) - Add CrowdStrike fork as primary install option with latest improvements - Keep upstream (eth0izzle) as stable alternative - Clarify that both repos provide the same plugin with two skills - Update manual install to show both repo options * Keep upstream repo URL, fix only the SKILL-NAME placeholder Revert CrowdStrike fork references. The actual bug was the generic SKILL-NAME placeholder, which is now replaced with the real plugin name (fusion-workflows) and a note about the two skills it provides. * Remove .git suffix from marketplace add command The /plugin marketplace add command takes the bare repo URL without the .git suffix. The git clone command in Manual Setup keeps it.
Real Fusion SOAR playbooks exported from the CrowdStrike Content Library with real action IDs. The import logic detects non-global actions and prompts users to configure integrations or substitute available actions. Categories: - threat-intel: VirusTotal domain/URL enrichment + Zscaler blocklist - identity-response: brute force auto-resolution, phishing with ITP - notifications: Slack messaging, endpoint containment with approval - ngsiem: duplicate detection auto-close - response-actions: 6 Palo Alto NGFW EDL/DAG playbooks - tutorials: 6 Introduction to... concept playbooks CrowdStrike-native action IDs are universal across all clouds. Plugin actions (Slack, Zscaler, PAN NGFW) require the integration installed from the CrowdStrike Store.
Two bugs in execute.py, both confirmed against live workflow executions:
- execute_workflow() assumed the execute endpoint returns resources as
a list of dicts (resources[0].get('id')), but it returns bare
execution-ID strings, crashing with 'str' object has no attribute 'get'.
Now handles both shapes.
- poll_results() checked status in ('completed','failed','error'), but
the execution-results API returns capitalized statuses
(Succeeded/Failed/Canceled/NonRecoverable/ActionRequired per
references/best-practices.md). A finished run never matched the terminal
set, so polling always ran until timeout. Now matches the documented
statuses case-insensitively.
Updated tests to use real API status casing and added coverage for the
bare-string execution ID.
CEL expressions do not support else syntax — only FQL expressions do. When a model generates a condition with both cel_expression and else, the else branch becomes unreachable, making the workflow functionally broken while still importing cleanly. Add an explicit validation error to catch this early.
mraible
marked this pull request as draft
June 12, 2026 15:07
The initial check only covered top-level conditions. Workflows with conditions nested inside loops (like the Okta session revocation workflow) were missed.
The model inconsistently uses string comparison ('Critical') vs numeric (>= 4)
because the trigger docs didn't specify the field type. Added the severity table
and correct/incorrect CEL examples so the model knows to use numeric comparison.
When the model skips action_search.py it sometimes writes all-zeros or all-f IDs that pass the hex format check but are obviously fabricated. The error message directs the model to run action_search.py to get the real ID.
mraible
changed the title
Contributor
Author
|
@eth0izzle I made the same mistake with this PR as #7. I'm happy to help resolve conflicts if necessary. FWIW, I will be on vacation from tonight - June 23rd and won't have access to my laptop until I'm back. |
mraible
marked this pull request as ready for review
June 12, 2026 17:24
The docs and validator asserted that only FQL-style `expression` supports `else` branching and that `cel_expression` + `else` is invalid. This is wrong. CEL conditions support both `else` and `else_if`, confirmed against the Fusion workflow engine's own translator test data (cel_with_else_if.yaml/.json) and by Tim Kuhlman (Principal Engineer, Fusion). Changes: - validate.py: remove the rule that rejected cel_expression + else (it would flag valid workflows as structural errors) - cel-expressions.md, yaml-schema.md: replace the "only expression supports else" claim with the correct CEL if/else-if/else syntax, and note it maps to an exclusive gateway's default flow in the underlying JSON - test_validate.py: add a regression test asserting cel_expression + else is accepted Prefer cel_expression for new workflows; FQL-style expression is legacy.
mraible
changed the title
Contributor
Author
|
Pushed a follow-up commit ( After opening this, Fusion engineering confirmed CEL conditions do support The commit removes the rule, corrects |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Two validation improvements, one documentation fix, and a correction to an earlier rule in this PR. Discovered through eval runs that generate workflows and judge them independently.
1. Correct cel_expression + else handling
An earlier commit in this PR added a rule rejecting
cel_expression+else, based on the claim in cel-expressions.md that only FQL-styleexpressionsupportselse. That claim is wrong. Fusion engineering confirmed that CEL conditions support bothelseandelse_if, and the workflow engine's own translator test data uses exactly that combination. The YAML is a conversion of the workflow JSON (which the backend processes), and theelsebranch maps to an exclusive gateway'sdefaultflow.The latest commit reverses that rule, so the validator no longer flags valid CEL workflows. It also corrects cel-expressions.md and yaml-schema.md to document the real CEL if / else-if / else syntax, and adds a regression test asserting the combination is accepted. Prefer
cel_expressionfor new workflows; FQL-styleexpressionis legacy.2. Reject all-same-character action IDs
When the model skips
action_search.pyit sometimes writes all-zeros or all-f IDs that pass the hex format check but are fabricated. The error message directs the model to runaction_search.pyto get the real ID.3. Document severity as numeric (1-5) in trigger-types.md
The EPP trigger section listed available fields but did not include
Severityor document its type. The model inconsistently used string comparison (== 'Critical') vs numeric (>= 4) because it had to guess. Added the severity value table and correct/incorrect CEL examples.Verification
pytest tests/: all passing, including a new regression test thatcel_expression+elseis acceptedpylint validate.py: 10.00/10Reviewed and merged internally before being brought back here.