[orchestrator] Backport orchestrator fixes and enhancements to 1.9#2667
Merged
karthikjeeyar merged 1 commit intoworkspace/orchestratorfrom Apr 1, 2026
Merged
Conversation
…2666) * feat(orchestrator): pre-populate Execute Workflow form from URL query params (#2570) * prepopulate workflow execution page form from URL query params Signed-off-by: Karthik <karthik.jk11@gmail.com> * support enum coercison for case-insenstive match and skip invalid values * add support for fields that are defined via '$ref' * add full support for json schema fields --------- Signed-off-by: Karthik <karthik.jk11@gmail.com> * fix(orchestrator-form-react): scope async validation to active step (#2602) * fix(orchestrator-form-react): scope async validation to active step Limit validate:url requests to the active step during multi-step navigation. Made-with: Cursor * fix(orchestrator-form-react): keep full formData for async validation Pass full formData to template evaluation while scoping uiSchema traversal. Made-with: Cursor * fix(orchestrator-form-react): preserve ui:hidden on objects with properties (#2653) * fix(orchestrator): honor json schema defaults in initial formData (#2654) * fix(orchestrator): honor json schema defaults Ensure extractStaticDefaults falls back to JSON Schema defaults when ui:props fetch:response:default is absent so initial formData includes schema defaults. Made-with: Cursor * chore(changeset): document schema default fix Add changeset for orchestrator form defaults update. Made-with: Cursor * fix(orchestrator): handle root defaults Avoid setting an empty key for root schema defaults and add tests to cover root default handling. Made-with: Cursor * fix(orchestrator): document and test defaults Clarify extractStaticDefaults precedence in docs and add test coverage for default handling. Made-with: Cursor * chore(orchestrator): update yarn.lock after backport Made-with: Cursor --------- Signed-off-by: Karthik <karthik.jk11@gmail.com> Co-authored-by: Karthik Jeeyar <karthik@redhat.com>
lokanandaprabhu
requested review from
batzionb,
gciavarrini and
mareklibra
as code owners
Review Summary by QodoBackport orchestrator fixes and enhancements to 1.9
WalkthroughsDescription• Pre-populate Execute Workflow form from URL query parameters with full schema support • Scope async validation to active step in multi-step forms to reduce unnecessary requests • Preserve ui:hidden and other UI directives on object schemas with properties • Honor JSON Schema default values in initial form data alongside fetch defaults • Add comprehensive test coverage for query param coercion and schema resolution Diagramflowchart LR
A["URL Query Params"] -->|mergeQueryParamsIntoFormData| B["Form Data"]
C["JSON Schema"] -->|extractStaticDefaults| B
D["Schema Defaults"] -->|fallback| C
E["UI Schema"] -->|generateUiSchema| F["UI Directives"]
G["Multi-step Form"] -->|scope validation| H["Active Step Only"]
B --> I["Execute Workflow Page"]
F --> I
H --> I
File Changes1. workspaces/orchestrator/plugins/orchestrator/src/components/ExecuteWorkflowPage/queryParamsToFormData.ts
|
Code Review by Qodo
1. allOf paths not recognized
|
|
![rhdh-qodo-merge[bot]](https://avatars.githubusercontent.com/in/1961108?s=60&v=4){kind=small}
Comment on lines
+55
to
+86
| /** | ||
| * Returns true if the path exists in the schema's properties (recursively). | ||
| * Handles $ref, oneOf, anyOf, allOf. Used to reject unknown query params. | ||
| */ | ||
| function pathExistsInSchema( | ||
| schema: JSONSchema7, | ||
| path: string, | ||
| root: JSONSchema7, | ||
| ): boolean { | ||
| if (!path) return true; | ||
| let s: JSONSchema7 | undefined = schema; | ||
| const parts = path.split('.'); | ||
|
|
||
| for (let i = 0; i < parts.length; i++) { | ||
| const part = parts[i]; | ||
| if (!s || typeof s === 'boolean') return false; | ||
| if (s.$ref) { | ||
| s = resolveRef(root, s.$ref); | ||
| i--; | ||
| continue; | ||
| } | ||
| if (s.oneOf || s.anyOf) { | ||
| const remaining = parts.slice(i).join('.'); | ||
| return pathExistsInComposite(s, remaining, root); | ||
| } | ||
| const props = s.properties; | ||
| if (!props || typeof props !== 'object') return false; | ||
| if (!(part in props)) return false; | ||
| s = props[part] as JSONSchema7; | ||
| } | ||
| return true; | ||
| } |
There was a problem hiding this comment.
1. Allof paths not recognized 🐞 Bug ≡ Correctness
mergeQueryParamsIntoFormData can incorrectly ignore query params for schemas composed with allOf because pathExistsInSchema only branches for oneOf/anyOf and otherwise requires a direct properties object. This breaks prepopulation for workflows whose input schema uses top-level or nested allOf composition.
Agent Prompt
### Issue description
`pathExistsInSchema` rejects valid query-param paths when the schema node uses `allOf` without also having `oneOf`/`anyOf`, because it only delegates composite traversal for `oneOf`/`anyOf` and otherwise requires `properties`.
### Issue Context
This causes query-param prepopulation to silently skip fields defined via schema composition (a common JSON Schema pattern).
### Fix Focus Areas
- workspaces/orchestrator/plugins/orchestrator/src/components/ExecuteWorkflowPage/queryParamsToFormData.ts[55-86]
- workspaces/orchestrator/plugins/orchestrator/src/components/ExecuteWorkflowPage/queryParamsToFormData.ts[91-109]
### Suggested fix
Update the composite-branch condition in `pathExistsInSchema` to include `allOf` as well (e.g., `if (s.oneOf || s.anyOf || s.allOf) ...`). Consider also adding a unit test covering a root schema with `allOf: [{ type:'object', properties:{ ... }}]` to prevent regression.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
+279
to
+292
| for (const [paramKey, paramValue] of searchParams.entries()) { | ||
| if (RESERVED_QUERY_PARAMS.has(paramKey)) { | ||
| continue; | ||
| } | ||
| if (paramValue === undefined || paramValue === null) { | ||
| continue; | ||
| } | ||
|
|
||
| if (!pathExistsInSchema(schema, paramKey, schema)) continue; | ||
|
|
||
| // Build proposed data with raw value so getSchema can resolve oneOf/anyOf/if-then-else | ||
| const proposedData = cloneDeep(result) as JsonObject; | ||
| set(proposedData, paramKey, paramValue); | ||
|
|
There was a problem hiding this comment.
2. Unsafe set path validation 🐞 Bug ⛨ Security
mergeQueryParamsIntoFormData uses part in props to validate query param paths and then passes the raw paramKey into lodash/set to build proposedData; in accepts prototype-chain keys like __proto__, which can allow prototype-pollution-style paths to pass the guard and reach lodash/set. This can cause unexpected behavior and potential security issues when processing untrusted URLs.
Agent Prompt
### Issue description
The query-param merge flow can write attacker-controlled paths into objects via `lodash/set`. The current guard uses `part in props`, which considers prototype-chain keys (e.g., `__proto__`), and `set(proposedData, paramKey, paramValue)` happens before schema resolution.
### Issue Context
Even if `getSchemaAtPath` later returns undefined and the param is skipped, the intermediate `set(proposedData, ...)` has already executed.
### Fix Focus Areas
- workspaces/orchestrator/plugins/orchestrator/src/components/ExecuteWorkflowPage/queryParamsToFormData.ts[59-86]
- workspaces/orchestrator/plugins/orchestrator/src/components/ExecuteWorkflowPage/queryParamsToFormData.ts[279-299]
### Suggested fix
1) Replace `if (!(part in props))` with an own-property check:
- `if (!Object.prototype.hasOwnProperty.call(props, part)) return false;`
2) Add explicit rejection of dangerous path segments before any `set()` call (defense-in-depth):
- Reject if any segment equals `__proto__`, `prototype`, or `constructor`.
3) Consider using a safe setter (or building `proposedData` without mutating prototypes) for the `proposedData` used in schema resolution.
Add a unit test that asserts params like `__proto__=x` and `constructor=y` are ignored and do not modify object prototypes.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
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
2 participants
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.
Hey, I just made a Pull Request!
Cherrypick of below PR's
#2654
#2602
#2653
#2570
Summary
✔️ Checklist