fix(orchestrator): Address CVE-2026-3118 (#2597) - #2736
Merged
Conversation
* fix: Update grapql client * Filters, pagination and queries now use query variables fixes CVE-2026-3118 and relates to JIRA https://redhat.atlassian.net/browse/RHIDP-12388 and https://redhat.atlassian.net/browse/RHIDP-12583
lholmquist
requested review from
batzionb,
gciavarrini and
mareklibra
as code owners
April 9, 2026 13:31
Review Summary by QodoFix CVE-2026-3118 by implementing GraphQL query variables for filters and pagination
WalkthroughsDescription• Implement GraphQL query variables for filters, pagination, and queries to address CVE-2026-3118 • Update @urql/core dependency from ^4.1.4 to ^6.0.1 • Refactor filter builder to return FilterClause objects containing clause strings and variable metadata • Refactor query builder to use parameterized queries with variable substitution instead of string interpolation • Update all GraphQL queries to use gql template literals with named variables Diagramflowchart LR
A["Filter/Query Builders"] -->|"Generate FilterClause<br/>with variables"| B["FilterClause Object"]
B -->|"Contains clause string<br/>and variable metadata"| C["Query Builder"]
C -->|"Constructs parameterized<br/>GraphQL query"| D["GraphQL Query"]
D -->|"Passes variables<br/>separately"| E["URQL Client"]
E -->|"Executes safely"| F["Data Index Service"]
File Changes1. workspaces/orchestrator/plugins/orchestrator-backend/src/helpers/filterBuilder.ts
|
Code Review by Qodo
|
|
![rhdh-qodo-merge[bot]](https://avatars.githubusercontent.com/in/1961108?s=60&v=4){kind=small}
Comment on lines
+195
to
+212
| let formattedValue: any; | ||
| let paramType: string; | ||
| if (Array.isArray(binaryFilter.value)) { | ||
| formattedValue = binaryFilter.value.map(v => | ||
| formatValue(binaryFilter.field, v, fieldDef, type), | ||
| ); | ||
| paramType = isEnumFilter(binaryFilter.field, type) | ||
| ? '[ProcessInstanceState!]' | ||
| : '[String!]'; | ||
| } else { | ||
| formattedValue = formatValue( | ||
| binaryFilter.field, | ||
| binaryFilter.value, | ||
| fieldDef, | ||
| type, | ||
| ); | ||
| paramType = 'String'; | ||
| } |
There was a problem hiding this comment.
1. Enum variable typed string 🐞 Bug ≡ Correctness
handleBinaryOperator declares single-value enum filter variables as type "String" while declaring the same enum field as "[ProcessInstanceState!]" for array values, so one of those queries must be invalid against the fixed server schema type. This can cause runtime GraphQL validation errors when applying filters on the enum field.
Agent Prompt
### Issue description
`handleBinaryOperator` assigns `clauseVariableType = 'String'` for all single-value filters, even when `isEnumFilter(...)` is true. In the array branch it uses `[ProcessInstanceState!]`, meaning the same enum field will produce different GraphQL variable types depending on value cardinality, and `queryBuilder` will emit those types directly into the operation signature.
### Issue Context
GraphQL input field types are fixed in the schema; they cannot be both `String` and `ProcessInstanceState` depending on operator/value shape. With the current code, at least one of the enum filter modes will fail schema validation.
### Fix Focus Areas
- workspaces/orchestrator/plugins/orchestrator-backend/src/helpers/filterBuilder.ts[183-212]
- workspaces/orchestrator/plugins/orchestrator-backend/src/helpers/queryBuilder.ts[20-36]
### What to change
- When `isEnumFilter(binaryFilter.field, type)` is true, set the single-value `paramType` to the correct enum type (e.g., `ProcessInstanceState` or `ProcessInstanceState!`) instead of `String`.
- Keep array and single-value enum typing consistent (e.g., `ProcessInstanceState` vs `[ProcessInstanceState!]`).
- If you don’t want to hardcode enum type names, consider deriving the correct variable type from schema/introspection and mapping it to the appropriate GraphQL type string.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
+236
to
239
| ): FilterClause { | ||
| if (!filters) { | ||
| return ''; | ||
| return {} as FilterClause; | ||
| } |
There was a problem hiding this comment.
2. Empty filterclause breaks queries 🐞 Bug ☼ Reliability
buildFilterCondition/handleLogicalFilter return {} cast as FilterClause, leaving clause and
clauseVariable undefined; downstream code treats the object as truthy and interpolates undefined
into the where clause. This can produce malformed GraphQL queries (e.g., {undefined} conditions)
and/or crash when iterating clauseVariable.
Agent Prompt
### Issue description
`buildFilterCondition` (and `handleLogicalFilter` when `operator` is missing) returns `{}` cast to `FilterClause`. This violates the `FilterClause` contract and leads to `undefined` being interpolated into `whereClause` and risks runtime errors when code assumes `clauseVariable` is an array.
### Issue Context
`DataIndexService.fetchInstances`/`fetchWorkflowInfos` treat any object returned by `buildFilterCondition` as truthy and append `{${filterCondition?.clause}}` into conditions. If `clause` is missing, the query string becomes invalid.
### Fix Focus Areas
- workspaces/orchestrator/plugins/orchestrator-backend/src/helpers/filterBuilder.ts[76-92]
- workspaces/orchestrator/plugins/orchestrator-backend/src/helpers/filterBuilder.ts[231-239]
- workspaces/orchestrator/plugins/orchestrator-backend/src/service/DataIndexService.ts[235-247]
- workspaces/orchestrator/plugins/orchestrator-backend/src/service/DataIndexService.ts[312-314]
### What to change
- Stop returning `{}` for “no filter”. Prefer one of:
- Change the return type to `FilterClause | undefined` and return `undefined` for no-op cases, updating callers to check `filterCondition?.clause`.
- Or always return a valid empty clause object: `{ clause: '', clauseVariable: [] }`.
- For `handleLogicalFilter`, since `LogicalFilter.operator` is required by the API type, consider throwing if it’s missing rather than returning an empty object.
- Ensure callers only append filter conditions when `filterCondition.clause` is non-empty.
ⓘ 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
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.
cherry pick of PR #2597
fix: Update grapql client
Filters, pagination and queries now use query variables
fixes CVE-2026-3118 and relates to JIRA https://redhat.atlassian.net/browse/RHIDP-12388 and https://redhat.atlassian.net/browse/RHIDP-12583
Hey, I just made a Pull Request!
✔️ Checklist